diff --git a/src/config/schema.test.ts b/src/config/schema.test.ts index bbf3a50b0..bb4f3d581 100644 --- a/src/config/schema.test.ts +++ b/src/config/schema.test.ts @@ -5,6 +5,7 @@ import { BrowserAutomationProviderSchema, BuiltinCategoryNameSchema, CategoryConfigSchema, + ExperimentalConfigSchema, OhMyOpenCodeConfigSchema, } from "./schema" @@ -606,3 +607,59 @@ describe("OhMyOpenCodeConfigSchema - browser_automation_engine", () => { expect(result.data?.browser_automation_engine).toBeUndefined() }) }) + +describe("ExperimentalConfigSchema feature flags", () => { + test("accepts plugin_load_timeout_ms as number", () => { + //#given + const config = { plugin_load_timeout_ms: 5000 } + + //#when + const result = ExperimentalConfigSchema.safeParse(config) + + //#then + expect(result.success).toBe(true) + if (result.success) { + expect(result.data.plugin_load_timeout_ms).toBe(5000) + } + }) + + test("rejects plugin_load_timeout_ms below 1000", () => { + //#given + const config = { plugin_load_timeout_ms: 500 } + + //#when + const result = ExperimentalConfigSchema.safeParse(config) + + //#then + expect(result.success).toBe(false) + }) + + test("accepts safe_hook_creation as boolean", () => { + //#given + const config = { safe_hook_creation: false } + + //#when + const result = ExperimentalConfigSchema.safeParse(config) + + //#then + expect(result.success).toBe(true) + if (result.success) { + expect(result.data.safe_hook_creation).toBe(false) + } + }) + + test("both fields are optional", () => { + //#given + const config = {} + + //#when + const result = ExperimentalConfigSchema.safeParse(config) + + //#then + expect(result.success).toBe(true) + if (result.success) { + expect(result.data.plugin_load_timeout_ms).toBeUndefined() + expect(result.data.safe_hook_creation).toBeUndefined() + } + }) +}) diff --git a/src/config/schema.ts b/src/config/schema.ts index 3b70a31fd..8990ea1b0 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -268,6 +268,10 @@ export const ExperimentalConfigSchema = z.object({ dynamic_context_pruning: DynamicContextPruningConfigSchema.optional(), /** Enable experimental task system for Todowrite disabler hook */ task_system: z.boolean().optional(), + /** Timeout in ms for loadAllPluginComponents during config handler init (default: 10000, min: 1000) */ + plugin_load_timeout_ms: z.number().min(1000).optional(), + /** Wrap hook creation in try/catch to prevent one failing hook from crashing the plugin (default: true at call site) */ + safe_hook_creation: z.boolean().optional(), }) export const SkillSourceSchema = z.union([ diff --git a/src/index.ts b/src/index.ts index ee210e667..aa0c6e73c 100644 --- a/src/index.ts +++ b/src/index.ts @@ -109,6 +109,7 @@ import { injectServerAuthIntoClient, } from "./shared"; import { filterDisabledTools } from "./shared/disabled-tools"; +import { safeCreateHook } from "./shared/safe-create-hook"; import { loadPluginConfig } from "./plugin-config"; import { createModelCacheState } from "./plugin-state"; import { createConfigHandler } from "./plugin-handlers"; @@ -135,21 +136,22 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => { agent_pane_min_width: pluginConfig.tmux?.agent_pane_min_width ?? 40, } as const; const isHookEnabled = (hookName: HookName) => !disabledHooks.has(hookName); + const safeHookEnabled = pluginConfig.experimental?.safe_hook_creation ?? true; const modelCacheState = createModelCacheState(); const contextWindowMonitor = isHookEnabled("context-window-monitor") - ? createContextWindowMonitorHook(ctx) + ? safeCreateHook("context-window-monitor", () => createContextWindowMonitorHook(ctx), { enabled: safeHookEnabled }) : null; const preemptiveCompaction = isHookEnabled("preemptive-compaction") && pluginConfig.experimental?.preemptive_compaction - ? createPreemptiveCompactionHook(ctx) + ? safeCreateHook("preemptive-compaction", () => createPreemptiveCompactionHook(ctx), { enabled: safeHookEnabled }) : null; const sessionRecovery = isHookEnabled("session-recovery") - ? createSessionRecoveryHook(ctx, { + ? safeCreateHook("session-recovery", () => createSessionRecoveryHook(ctx, { experimental: pluginConfig.experimental, - }) + }), { enabled: safeHookEnabled }) : null; // Check for conflicting notification plugins before creating session-notification @@ -166,17 +168,17 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => { allPlugins: externalNotifier.allPlugins, }); } else { - sessionNotification = createSessionNotification(ctx); + sessionNotification = safeCreateHook("session-notification", () => createSessionNotification(ctx), { enabled: safeHookEnabled }); } } const commentChecker = isHookEnabled("comment-checker") - ? createCommentCheckerHooks(pluginConfig.comment_checker) + ? safeCreateHook("comment-checker", () => createCommentCheckerHooks(pluginConfig.comment_checker), { enabled: safeHookEnabled }) : null; const toolOutputTruncator = isHookEnabled("tool-output-truncator") - ? createToolOutputTruncatorHook(ctx, { + ? safeCreateHook("tool-output-truncator", () => createToolOutputTruncatorHook(ctx, { experimental: pluginConfig.experimental, - }) + }), { enabled: safeHookEnabled }) : null; // Check for native OpenCode AGENTS.md injection support before creating hook let directoryAgentsInjector = null; @@ -195,18 +197,18 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => { }, ); } else { - directoryAgentsInjector = createDirectoryAgentsInjectorHook(ctx); + directoryAgentsInjector = safeCreateHook("directory-agents-injector", () => createDirectoryAgentsInjectorHook(ctx), { enabled: safeHookEnabled }); } } const directoryReadmeInjector = isHookEnabled("directory-readme-injector") - ? createDirectoryReadmeInjectorHook(ctx) + ? safeCreateHook("directory-readme-injector", () => createDirectoryReadmeInjectorHook(ctx), { enabled: safeHookEnabled }) : null; const emptyTaskResponseDetector = isHookEnabled( "empty-task-response-detector", ) - ? createEmptyTaskResponseDetectorHook(ctx) + ? safeCreateHook("empty-task-response-detector", () => createEmptyTaskResponseDetectorHook(ctx), { enabled: safeHookEnabled }) : null; - const thinkMode = isHookEnabled("think-mode") ? createThinkModeHook() : null; + const thinkMode = isHookEnabled("think-mode") ? safeCreateHook("think-mode", () => createThinkModeHook(), { enabled: safeHookEnabled }) : null; const claudeCodeHooks = createClaudeCodeHooksHook( ctx, { @@ -219,84 +221,84 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => { const anthropicContextWindowLimitRecovery = isHookEnabled( "anthropic-context-window-limit-recovery", ) - ? createAnthropicContextWindowLimitRecoveryHook(ctx, { + ? safeCreateHook("anthropic-context-window-limit-recovery", () => createAnthropicContextWindowLimitRecoveryHook(ctx, { experimental: pluginConfig.experimental, - }) + }), { enabled: safeHookEnabled }) : null; const rulesInjector = isHookEnabled("rules-injector") - ? createRulesInjectorHook(ctx) + ? safeCreateHook("rules-injector", () => createRulesInjectorHook(ctx), { enabled: safeHookEnabled }) : null; const autoUpdateChecker = isHookEnabled("auto-update-checker") - ? createAutoUpdateCheckerHook(ctx, { + ? safeCreateHook("auto-update-checker", () => createAutoUpdateCheckerHook(ctx, { showStartupToast: isHookEnabled("startup-toast"), isSisyphusEnabled: pluginConfig.sisyphus_agent?.disabled !== true, autoUpdate: pluginConfig.auto_update ?? true, - }) + }), { enabled: safeHookEnabled }) : null; const keywordDetector = isHookEnabled("keyword-detector") - ? createKeywordDetectorHook(ctx, contextCollector) + ? safeCreateHook("keyword-detector", () => createKeywordDetectorHook(ctx, contextCollector), { enabled: safeHookEnabled }) : null; const contextInjectorMessagesTransform = createContextInjectorMessagesTransformHook(contextCollector); const agentUsageReminder = isHookEnabled("agent-usage-reminder") - ? createAgentUsageReminderHook(ctx) + ? safeCreateHook("agent-usage-reminder", () => createAgentUsageReminderHook(ctx), { enabled: safeHookEnabled }) : null; const nonInteractiveEnv = isHookEnabled("non-interactive-env") - ? createNonInteractiveEnvHook(ctx) + ? safeCreateHook("non-interactive-env", () => createNonInteractiveEnvHook(ctx), { enabled: safeHookEnabled }) : null; const interactiveBashSession = isHookEnabled("interactive-bash-session") - ? createInteractiveBashSessionHook(ctx) + ? safeCreateHook("interactive-bash-session", () => createInteractiveBashSessionHook(ctx), { enabled: safeHookEnabled }) : null; const thinkingBlockValidator = isHookEnabled("thinking-block-validator") - ? createThinkingBlockValidatorHook() + ? safeCreateHook("thinking-block-validator", () => createThinkingBlockValidatorHook(), { enabled: safeHookEnabled }) : null; let categorySkillReminder: ReturnType | null = null; const ralphLoop = isHookEnabled("ralph-loop") - ? createRalphLoopHook(ctx, { + ? safeCreateHook("ralph-loop", () => createRalphLoopHook(ctx, { config: pluginConfig.ralph_loop, checkSessionExists: async (sessionId) => sessionExists(sessionId), - }) + }), { enabled: safeHookEnabled }) : null; const editErrorRecovery = isHookEnabled("edit-error-recovery") - ? createEditErrorRecoveryHook(ctx) + ? safeCreateHook("edit-error-recovery", () => createEditErrorRecoveryHook(ctx), { enabled: safeHookEnabled }) : null; const delegateTaskRetry = isHookEnabled("delegate-task-retry") - ? createDelegateTaskRetryHook(ctx) + ? safeCreateHook("delegate-task-retry", () => createDelegateTaskRetryHook(ctx), { enabled: safeHookEnabled }) : null; const startWork = isHookEnabled("start-work") - ? createStartWorkHook(ctx) + ? safeCreateHook("start-work", () => createStartWorkHook(ctx), { enabled: safeHookEnabled }) : null; const prometheusMdOnly = isHookEnabled("prometheus-md-only") - ? createPrometheusMdOnlyHook(ctx) + ? safeCreateHook("prometheus-md-only", () => createPrometheusMdOnlyHook(ctx), { enabled: safeHookEnabled }) : null; const sisyphusJuniorNotepad = isHookEnabled("sisyphus-junior-notepad") - ? createSisyphusJuniorNotepadHook(ctx) + ? safeCreateHook("sisyphus-junior-notepad", () => createSisyphusJuniorNotepadHook(ctx), { enabled: safeHookEnabled }) : null; const tasksTodowriteDisabler = isHookEnabled("tasks-todowrite-disabler") - ? createTasksTodowriteDisablerHook({ + ? safeCreateHook("tasks-todowrite-disabler", () => createTasksTodowriteDisablerHook({ experimental: pluginConfig.experimental, - }) + }), { enabled: safeHookEnabled }) : null; const questionLabelTruncator = createQuestionLabelTruncatorHook(); const subagentQuestionBlocker = createSubagentQuestionBlockerHook(); const writeExistingFileGuard = isHookEnabled("write-existing-file-guard") - ? createWriteExistingFileGuardHook(ctx) + ? safeCreateHook("write-existing-file-guard", () => createWriteExistingFileGuardHook(ctx), { enabled: safeHookEnabled }) : null; const taskResumeInfo = createTaskResumeInfoHook(); const anthropicEffort = isHookEnabled("anthropic-effort") - ? createAnthropicEffortHook() + ? safeCreateHook("anthropic-effort", () => createAnthropicEffortHook(), { enabled: safeHookEnabled }) : null; const tmuxSessionManager = new TmuxSessionManager(ctx, tmuxConfig); @@ -333,28 +335,28 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => { ); const atlasHook = isHookEnabled("atlas") - ? createAtlasHook(ctx, { directory: ctx.directory, backgroundManager }) + ? safeCreateHook("atlas", () => createAtlasHook(ctx, { directory: ctx.directory, backgroundManager }), { enabled: safeHookEnabled }) : null; initTaskToastManager(ctx.client); const stopContinuationGuard = isHookEnabled("stop-continuation-guard") - ? createStopContinuationGuardHook(ctx) + ? safeCreateHook("stop-continuation-guard", () => createStopContinuationGuardHook(ctx), { enabled: safeHookEnabled }) : null; const compactionContextInjector = isHookEnabled("compaction-context-injector") - ? createCompactionContextInjector() + ? safeCreateHook("compaction-context-injector", () => createCompactionContextInjector(), { enabled: safeHookEnabled }) : null; const todoContinuationEnforcer = isHookEnabled("todo-continuation-enforcer") - ? createTodoContinuationEnforcer(ctx, { + ? safeCreateHook("todo-continuation-enforcer", () => createTodoContinuationEnforcer(ctx, { backgroundManager, isContinuationStopped: stopContinuationGuard?.isStopped, - }) + }), { enabled: safeHookEnabled }) : null; const unstableAgentBabysitter = isHookEnabled("unstable-agent-babysitter") - ? createUnstableAgentBabysitterHook( + ? safeCreateHook("unstable-agent-babysitter", () => createUnstableAgentBabysitterHook( { directory: ctx.directory, client: { @@ -382,7 +384,7 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => { backgroundManager, config: pluginConfig.babysitting, }, - ) + ), { enabled: safeHookEnabled }) : null; if (sessionRecovery && todoContinuationEnforcer) { @@ -393,7 +395,7 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => { } const backgroundNotificationHook = isHookEnabled("background-notification") - ? createBackgroundNotificationHook(backgroundManager) + ? safeCreateHook("background-notification", () => createBackgroundNotificationHook(backgroundManager), { enabled: safeHookEnabled }) : null; const backgroundTools = createBackgroundTools(backgroundManager, ctx.client); @@ -490,7 +492,7 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => { }); categorySkillReminder = isHookEnabled("category-skill-reminder") - ? createCategorySkillReminderHook(ctx, availableSkills) + ? safeCreateHook("category-skill-reminder", () => createCategorySkillReminderHook(ctx, availableSkills), { enabled: safeHookEnabled }) : null; const skillMcpManager = new SkillMcpManager(); @@ -515,7 +517,7 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => { }); const autoSlashCommand = isHookEnabled("auto-slash-command") - ? createAutoSlashCommandHook({ skills: mergedSkills }) + ? safeCreateHook("auto-slash-command", () => createAutoSlashCommandHook({ skills: mergedSkills }), { enabled: safeHookEnabled }) : null; const configHandler = createConfigHandler({ @@ -610,7 +612,7 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => { await stopContinuationGuard?.["chat.message"]?.(input); await keywordDetector?.["chat.message"]?.(input, output); - await claudeCodeHooks["chat.message"]?.(input, output); + await claudeCodeHooks?.["chat.message"]?.(input, output); await autoSlashCommand?.["chat.message"]?.(input, output); await startWork?.["chat.message"]?.(input, output); @@ -700,7 +702,7 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => { event: async (input) => { await autoUpdateChecker?.event(input); - await claudeCodeHooks.event(input); + await claudeCodeHooks?.event?.(input); await backgroundNotificationHook?.event(input); await sessionNotification?.(input); await todoContinuationEnforcer?.handler(input); @@ -800,10 +802,10 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => { }, "tool.execute.before": async (input, output) => { - await subagentQuestionBlocker["tool.execute.before"]?.(input, output); + await subagentQuestionBlocker?.["tool.execute.before"]?.(input, output); await writeExistingFileGuard?.["tool.execute.before"]?.(input, output); - await questionLabelTruncator["tool.execute.before"]?.(input, output); - await claudeCodeHooks["tool.execute.before"](input, output); + await questionLabelTruncator?.["tool.execute.before"]?.(input, output); + await claudeCodeHooks?.["tool.execute.before"]?.(input, output); await nonInteractiveEnv?.["tool.execute.before"](input, output); await commentChecker?.["tool.execute.before"]?.(input, output); await directoryAgentsInjector?.["tool.execute.before"]?.(input, output); @@ -909,7 +911,7 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => { } } - await claudeCodeHooks["tool.execute.after"](input, output); + await claudeCodeHooks?.["tool.execute.after"]?.(input, output); await toolOutputTruncator?.["tool.execute.after"](input, output); await preemptiveCompaction?.["tool.execute.after"](input, output); await contextWindowMonitor?.["tool.execute.after"](input, output); @@ -924,7 +926,7 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => { await editErrorRecovery?.["tool.execute.after"](input, output); await delegateTaskRetry?.["tool.execute.after"](input, output); await atlasHook?.["tool.execute.after"]?.(input, output); - await taskResumeInfo["tool.execute.after"](input, output); + await taskResumeInfo?.["tool.execute.after"]?.(input, output); }, "experimental.session.compacting": async ( diff --git a/src/plugin-handlers/config-handler.test.ts b/src/plugin-handlers/config-handler.test.ts index 4839e5531..d33f37184 100644 --- a/src/plugin-handlers/config-handler.test.ts +++ b/src/plugin-handlers/config-handler.test.ts @@ -642,3 +642,123 @@ describe("Deadlock prevention - fetchAvailableModels must not receive client", ( fetchSpy.mockRestore?.() }) }) + +describe("config-handler plugin loading error boundary (#1559)", () => { + test("returns empty defaults when loadAllPluginComponents throws", async () => { + //#given + ;(pluginLoader.loadAllPluginComponents as any).mockRestore?.() + spyOn(pluginLoader, "loadAllPluginComponents" as any).mockRejectedValue(new Error("crash")) + const pluginConfig: OhMyOpenCodeConfig = {} + const config: Record = { + model: "anthropic/claude-opus-4-6", + agent: {}, + } + const handler = createConfigHandler({ + ctx: { directory: "/tmp" }, + pluginConfig, + modelCacheState: { + anthropicContext1MEnabled: false, + modelContextLimitsCache: new Map(), + }, + }) + + //#when + await handler(config) + + //#then + expect(config.agent).toBeDefined() + }) + + test("returns empty defaults when loadAllPluginComponents times out", async () => { + //#given + ;(pluginLoader.loadAllPluginComponents as any).mockRestore?.() + spyOn(pluginLoader, "loadAllPluginComponents" as any).mockImplementation( + () => new Promise(() => {}) + ) + const pluginConfig: OhMyOpenCodeConfig = { + experimental: { plugin_load_timeout_ms: 100 }, + } + const config: Record = { + model: "anthropic/claude-opus-4-6", + agent: {}, + } + const handler = createConfigHandler({ + ctx: { directory: "/tmp" }, + pluginConfig, + modelCacheState: { + anthropicContext1MEnabled: false, + modelContextLimitsCache: new Map(), + }, + }) + + //#when + await handler(config) + + //#then + expect(config.agent).toBeDefined() + }, 5000) + + test("logs error when loadAllPluginComponents fails", async () => { + //#given + ;(pluginLoader.loadAllPluginComponents as any).mockRestore?.() + spyOn(pluginLoader, "loadAllPluginComponents" as any).mockRejectedValue(new Error("crash")) + const logSpy = shared.log as ReturnType + const pluginConfig: OhMyOpenCodeConfig = {} + const config: Record = { + model: "anthropic/claude-opus-4-6", + agent: {}, + } + const handler = createConfigHandler({ + ctx: { directory: "/tmp" }, + pluginConfig, + modelCacheState: { + anthropicContext1MEnabled: false, + modelContextLimitsCache: new Map(), + }, + }) + + //#when + await handler(config) + + //#then + const logCalls = logSpy.mock.calls.map((c: unknown[]) => c[0]) + const hasPluginFailureLog = logCalls.some( + (msg: string) => typeof msg === "string" && msg.includes("Plugin loading failed") + ) + expect(hasPluginFailureLog).toBe(true) + }) + + test("passes through plugin data on successful load (identity test)", async () => { + //#given + ;(pluginLoader.loadAllPluginComponents as any).mockRestore?.() + spyOn(pluginLoader, "loadAllPluginComponents" as any).mockResolvedValue({ + commands: { "test-cmd": { description: "test", template: "test" } }, + skills: {}, + agents: {}, + mcpServers: {}, + hooksConfigs: [], + plugins: [{ name: "test-plugin", version: "1.0.0" }], + errors: [], + }) + const pluginConfig: OhMyOpenCodeConfig = {} + const config: Record = { + model: "anthropic/claude-opus-4-6", + agent: {}, + } + const handler = createConfigHandler({ + ctx: { directory: "/tmp" }, + pluginConfig, + modelCacheState: { + anthropicContext1MEnabled: false, + modelContextLimitsCache: new Map(), + }, + }) + + //#when + await handler(config) + + //#then + const commands = config.command as Record + expect(commands["test-cmd"]).toBeDefined() + }) +}) diff --git a/src/plugin-handlers/config-handler.ts b/src/plugin-handlers/config-handler.ts index 36191fafa..41adbaf20 100644 --- a/src/plugin-handlers/config-handler.ts +++ b/src/plugin-handlers/config-handler.ts @@ -25,7 +25,7 @@ import { loadMcpConfigs } from "../features/claude-code-mcp-loader"; import { loadAllPluginComponents } from "../features/claude-code-plugin-loader"; import { createBuiltinMcps } from "../mcp"; import type { OhMyOpenCodeConfig } from "../config"; -import { log, fetchAvailableModels, readConnectedProvidersCache, resolveModelPipeline } from "../shared"; +import { log, fetchAvailableModels, readConnectedProvidersCache, resolveModelPipeline, addConfigLoadError } from "../shared"; import { getOpenCodeConfigPaths } from "../shared/opencode-config-dir"; import { migrateAgentConfig } from "../shared/permission-compat"; import { AGENT_NAME_MAP } from "../shared/migration"; @@ -104,19 +104,44 @@ export function createConfigHandler(deps: ConfigHandlerDeps) { } } - const pluginComponents = (pluginConfig.claude_code?.plugins ?? true) - ? await loadAllPluginComponents({ - enabledPluginsOverride: pluginConfig.claude_code?.plugins_override, - }) - : { - commands: {}, - skills: {}, - agents: {}, - mcpServers: {}, - hooksConfigs: [], - plugins: [], - errors: [], - }; + const emptyPluginDefaults = { + commands: {}, + skills: {}, + agents: {}, + mcpServers: {}, + hooksConfigs: [] as { hooks?: Record }[], + plugins: [] as { name: string; version: string }[], + errors: [] as { pluginKey: string; installPath: string; error: string }[], + }; + + let pluginComponents: typeof emptyPluginDefaults; + const pluginsEnabled = pluginConfig.claude_code?.plugins ?? true; + + if (pluginsEnabled) { + const timeoutMs = pluginConfig.experimental?.plugin_load_timeout_ms ?? 10000; + try { + let timeoutId: ReturnType; + const timeoutPromise = new Promise((_, reject) => { + timeoutId = setTimeout( + () => reject(new Error(`Plugin loading timed out after ${timeoutMs}ms`)), + timeoutMs, + ); + }); + pluginComponents = await Promise.race([ + loadAllPluginComponents({ + enabledPluginsOverride: pluginConfig.claude_code?.plugins_override, + }), + timeoutPromise, + ]).finally(() => clearTimeout(timeoutId)); + } catch (error) { + const errorMessage = error instanceof Error ? error.message : String(error); + log("[config-handler] Plugin loading failed", { error: errorMessage }); + addConfigLoadError({ path: "plugin-loading", error: errorMessage }); + pluginComponents = emptyPluginDefaults; + } + } else { + pluginComponents = emptyPluginDefaults; + } if (pluginComponents.plugins.length > 0) { log(`Loaded ${pluginComponents.plugins.length} Claude Code plugins`, { diff --git a/src/shared/index.ts b/src/shared/index.ts index 93163dcc0..44d6b1f58 100644 --- a/src/shared/index.ts +++ b/src/shared/index.ts @@ -41,3 +41,4 @@ export * from "./tmux" export * from "./model-suggestion-retry" export * from "./opencode-server-auth" export * from "./port-utils" +export * from "./safe-create-hook" diff --git a/src/shared/safe-create-hook.test.ts b/src/shared/safe-create-hook.test.ts new file mode 100644 index 000000000..72c326a66 --- /dev/null +++ b/src/shared/safe-create-hook.test.ts @@ -0,0 +1,73 @@ +import { describe, test, expect, spyOn, afterEach } from "bun:test" +import * as shared from "./logger" +import { safeCreateHook } from "./safe-create-hook" + +afterEach(() => { + ;(shared.log as any)?.mockRestore?.() +}) + +describe("safeCreateHook", () => { + test("returns hook object when factory succeeds", () => { + //#given + const hook = { handler: () => {} } + const factory = () => hook + + //#when + const result = safeCreateHook("test-hook", factory) + + //#then + expect(result).toBe(hook) + }) + + test("returns null when factory throws", () => { + //#given + spyOn(shared, "log").mockImplementation(() => {}) + const factory = () => { + throw new Error("boom") + } + + //#when + const result = safeCreateHook("test-hook", factory) + + //#then + expect(result).toBeNull() + }) + + test("logs error when factory throws", () => { + //#given + const logSpy = spyOn(shared, "log").mockImplementation(() => {}) + const factory = () => { + throw new Error("boom") + } + + //#when + safeCreateHook("my-hook", factory) + + //#then + expect(logSpy).toHaveBeenCalled() + const callArgs = logSpy.mock.calls[0] + expect(callArgs[0]).toContain("my-hook") + expect(callArgs[0]).toContain("Hook creation failed") + }) + + test("propagates error when enabled is false", () => { + //#given + const factory = () => { + throw new Error("boom") + } + + //#when + #then + expect(() => safeCreateHook("test-hook", factory, { enabled: false })).toThrow("boom") + }) + + test("returns null for factory returning undefined", () => { + //#given + const factory = () => undefined as any + + //#when + const result = safeCreateHook("test-hook", factory) + + //#then + expect(result).toBeNull() + }) +}) diff --git a/src/shared/safe-create-hook.ts b/src/shared/safe-create-hook.ts new file mode 100644 index 000000000..1ef3c9eea --- /dev/null +++ b/src/shared/safe-create-hook.ts @@ -0,0 +1,24 @@ +import { log } from "./logger" + +interface SafeCreateHookOptions { + enabled?: boolean +} + +export function safeCreateHook( + name: string, + factory: () => T, + options?: SafeCreateHookOptions, +): T | null { + const enabled = options?.enabled ?? true + + if (!enabled) { + return factory() ?? null + } + + try { + return factory() ?? null + } catch (error) { + log(`[safe-create-hook] Hook creation failed: ${name}`, { error }) + return null + } +}