-
Notifications
You must be signed in to change notification settings - Fork 0
fix: add error isolation to prevent WebUI crashes, server disconnections, and agent stalling #32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -143,6 +143,15 @@ export function createEventHandler(args: { | |||||
| const lastHandledRetryStatusKey = new Map<string, string>(); | ||||||
| const lastKnownModelBySession = new Map<string, { providerID: string; modelID: string }>(); | ||||||
|
|
||||||
| const safeHookCall = async (hookName: string, fn: (() => unknown) | undefined): Promise<void> => { | ||||||
| if (!fn) return; | ||||||
| try { | ||||||
| await Promise.resolve(fn()); | ||||||
| } catch (err) { | ||||||
| log(`[event] Hook "${hookName}" threw during dispatch:`, { error: err instanceof Error ? err.message : String(err) }); | ||||||
| } | ||||||
| }; | ||||||
|
|
||||||
| const dispatchToHooks = async (input: EventInput): Promise<void> => { | ||||||
| // Invalidate session cache on session events | ||||||
| const sessionID = (input.event.properties as Record<string, unknown> | undefined)?.sessionID as string | undefined; | ||||||
|
|
@@ -154,28 +163,28 @@ export function createEventHandler(args: { | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| await Promise.resolve(hooks.autoUpdateChecker?.event?.(input)); | ||||||
| await Promise.resolve(hooks.claudeCodeHooks?.event?.(input)); | ||||||
| await Promise.resolve(hooks.backgroundNotificationHook?.event?.(input)); | ||||||
| await Promise.resolve(hooks.sessionNotification?.(input)); | ||||||
| await Promise.resolve(hooks.todoContinuationEnforcer?.handler?.(input)); | ||||||
| await Promise.resolve(hooks.unstableAgentBabysitter?.event?.(input)); | ||||||
| await Promise.resolve(hooks.contextWindowMonitor?.event?.(input)); | ||||||
| await Promise.resolve(hooks.directoryAgentsInjector?.event?.(input)); | ||||||
| await Promise.resolve(hooks.directoryReadmeInjector?.event?.(input)); | ||||||
| await Promise.resolve(hooks.rulesInjector?.event?.(input)); | ||||||
| await Promise.resolve(hooks.thinkMode?.event?.(input)); | ||||||
| await Promise.resolve(hooks.anthropicContextWindowLimitRecovery?.event?.(input)); | ||||||
| await Promise.resolve(hooks.runtimeFallback?.event?.(input)); | ||||||
| await Promise.resolve(hooks.agentUsageReminder?.event?.(input)); | ||||||
| await Promise.resolve(hooks.categorySkillReminder?.event?.(input)); | ||||||
| await Promise.resolve(hooks.interactiveBashSession?.event?.(input as EventInput)); | ||||||
| await Promise.resolve(hooks.ralphLoop?.event?.(input)); | ||||||
| await Promise.resolve(hooks.stopContinuationGuard?.event?.(input)); | ||||||
| await Promise.resolve(hooks.compactionTodoPreserver?.event?.(input)); | ||||||
| await Promise.resolve(hooks.writeExistingFileGuard?.event?.(input)); | ||||||
| await Promise.resolve(hooks.atlasHook?.handler?.(input)); | ||||||
| await Promise.resolve((hooks as any).runStateWatchdog?.event?.(input)); | ||||||
| await safeHookCall("autoUpdateChecker", () => hooks.autoUpdateChecker?.event?.(input)); | ||||||
| await safeHookCall("claudeCodeHooks", () => hooks.claudeCodeHooks?.event?.(input)); | ||||||
| await safeHookCall("backgroundNotificationHook", () => hooks.backgroundNotificationHook?.event?.(input)); | ||||||
| await safeHookCall("sessionNotification", () => hooks.sessionNotification?.(input)); | ||||||
| await safeHookCall("todoContinuationEnforcer", () => hooks.todoContinuationEnforcer?.handler?.(input)); | ||||||
| await safeHookCall("unstableAgentBabysitter", () => hooks.unstableAgentBabysitter?.event?.(input)); | ||||||
| await safeHookCall("contextWindowMonitor", () => hooks.contextWindowMonitor?.event?.(input)); | ||||||
| await safeHookCall("directoryAgentsInjector", () => hooks.directoryAgentsInjector?.event?.(input)); | ||||||
| await safeHookCall("directoryReadmeInjector", () => hooks.directoryReadmeInjector?.event?.(input)); | ||||||
| await safeHookCall("rulesInjector", () => hooks.rulesInjector?.event?.(input)); | ||||||
| await safeHookCall("thinkMode", () => hooks.thinkMode?.event?.(input)); | ||||||
| await safeHookCall("anthropicContextWindowLimitRecovery", () => hooks.anthropicContextWindowLimitRecovery?.event?.(input)); | ||||||
| await safeHookCall("runtimeFallback", () => hooks.runtimeFallback?.event?.(input)); | ||||||
| await safeHookCall("agentUsageReminder", () => hooks.agentUsageReminder?.event?.(input)); | ||||||
| await safeHookCall("categorySkillReminder", () => hooks.categorySkillReminder?.event?.(input)); | ||||||
| await safeHookCall("interactiveBashSession", () => hooks.interactiveBashSession?.event?.(input as EventInput)); | ||||||
| await safeHookCall("ralphLoop", () => hooks.ralphLoop?.event?.(input)); | ||||||
| await safeHookCall("stopContinuationGuard", () => hooks.stopContinuationGuard?.event?.(input)); | ||||||
| await safeHookCall("compactionTodoPreserver", () => hooks.compactionTodoPreserver?.event?.(input)); | ||||||
| await safeHookCall("writeExistingFileGuard", () => hooks.writeExistingFileGuard?.event?.(input)); | ||||||
| await safeHookCall("atlasHook", () => hooks.atlasHook?.handler?.(input)); | ||||||
| await safeHookCall("runStateWatchdog", () => (hooks as any).runStateWatchdog?.event?.(input)); | ||||||
| }; | ||||||
|
|
||||||
| const recentSyntheticIdles = new Map<string, number>(); | ||||||
|
|
@@ -213,7 +222,11 @@ export function createEventHandler(args: { | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| await dispatchToHooks(input); | ||||||
| try { | ||||||
| await dispatchToHooks(input); | ||||||
| } catch (err) { | ||||||
| log("[event] dispatchToHooks failed:", { error: err instanceof Error ? err.message : String(err) }); | ||||||
| } | ||||||
|
|
||||||
|
|
||||||
| const syntheticIdle = normalizeSessionStatusToIdle(input); | ||||||
|
|
@@ -259,24 +272,28 @@ export function createEventHandler(args: { | |||||
| } | ||||||
|
|
||||||
| if (event.type === "session.created") { | ||||||
| const sessionInfo = props?.info as { id?: string; title?: string; parentID?: string } | undefined; | ||||||
| try { | ||||||
| const sessionInfo = props?.info as { id?: string; title?: string; parentID?: string } | undefined; | ||||||
|
|
||||||
| if (!sessionInfo?.parentID) { | ||||||
| setMainSession(sessionInfo?.id); | ||||||
| } | ||||||
| if (!sessionInfo?.parentID) { | ||||||
| setMainSession(sessionInfo?.id); | ||||||
| } | ||||||
|
|
||||||
| const sessionID = sessionInfo?.id; | ||||||
| const sessionID = sessionInfo?.id; | ||||||
|
|
||||||
| firstMessageVariantGate.markSessionCreated(sessionInfo); | ||||||
| firstMessageVariantGate.markSessionCreated(sessionInfo); | ||||||
|
|
||||||
| await managers.tmuxSessionManager.onSessionCreated( | ||||||
| event as { | ||||||
| type: string; | ||||||
| properties?: { | ||||||
| info?: { id?: string; parentID?: string; title?: string }; | ||||||
| }; | ||||||
| }, | ||||||
| ); | ||||||
| await managers.tmuxSessionManager.onSessionCreated( | ||||||
| event as { | ||||||
| type: string; | ||||||
| properties?: { | ||||||
| info?: { id?: string; parentID?: string; title?: string }; | ||||||
| }; | ||||||
| }, | ||||||
| ); | ||||||
| } catch (err) { | ||||||
| log("[event] Error in session.created handler:", { error: err }); | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For consistency with other error logging in this file and to prevent potential issues with logging complex error objects, it's better to serialize the error to a string.
Suggested change
|
||||||
| } | ||||||
| } | ||||||
|
|
||||||
| if (event.type === "session.deleted") { | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,99 +18,103 @@ export function createToolExecuteBeforeHandler(args: { | |
| const { ctx, hooks } = args | ||
|
|
||
| return async (input, output): Promise<void> => { | ||
| await hooks.planEnforcement?.["tool.execute.before"]?.(input, output) | ||
| await hooks.semanticLoopGuard?.["tool.execute.before"]?.(input, output) | ||
| await hooks.carRuntime?.["tool.execute.before"]?.(input, output) | ||
| try { | ||
| await hooks.planEnforcement?.["tool.execute.before"]?.(input, output) | ||
| await hooks.semanticLoopGuard?.["tool.execute.before"]?.(input, output) | ||
| await hooks.carRuntime?.["tool.execute.before"]?.(input, output) | ||
|
|
||
| await hooks.writeExistingFileGuard?.["tool.execute.before"]?.(input, output) | ||
| await hooks.questionLabelTruncator?.["tool.execute.before"]?.(input, output) | ||
| await hooks.claudeCodeHooks?.["tool.execute.before"]?.(input, output) | ||
| await hooks.nonInteractiveEnv?.["tool.execute.before"]?.(input, output) | ||
| await hooks.commentChecker?.["tool.execute.before"]?.(input, output) | ||
| await hooks.directoryAgentsInjector?.["tool.execute.before"]?.(input, output) | ||
| await hooks.directoryReadmeInjector?.["tool.execute.before"]?.(input, output) | ||
| await hooks.rulesInjector?.["tool.execute.before"]?.(input, output) | ||
| await hooks.tasksTodowriteDisabler?.["tool.execute.before"]?.(input, output) | ||
| await hooks.prometheusMdOnly?.["tool.execute.before"]?.(input, output) | ||
| await hooks.sisyphusJuniorNotepad?.["tool.execute.before"]?.(input, output) | ||
| await hooks.atlasHook?.["tool.execute.before"]?.(input, output) | ||
| await hooks.writeExistingFileGuard?.["tool.execute.before"]?.(input, output) | ||
| await hooks.questionLabelTruncator?.["tool.execute.before"]?.(input, output) | ||
| await hooks.claudeCodeHooks?.["tool.execute.before"]?.(input, output) | ||
| await hooks.nonInteractiveEnv?.["tool.execute.before"]?.(input, output) | ||
| await hooks.commentChecker?.["tool.execute.before"]?.(input, output) | ||
| await hooks.directoryAgentsInjector?.["tool.execute.before"]?.(input, output) | ||
| await hooks.directoryReadmeInjector?.["tool.execute.before"]?.(input, output) | ||
| await hooks.rulesInjector?.["tool.execute.before"]?.(input, output) | ||
| await hooks.tasksTodowriteDisabler?.["tool.execute.before"]?.(input, output) | ||
| await hooks.prometheusMdOnly?.["tool.execute.before"]?.(input, output) | ||
| await hooks.sisyphusJuniorNotepad?.["tool.execute.before"]?.(input, output) | ||
| await hooks.atlasHook?.["tool.execute.before"]?.(input, output) | ||
|
|
||
| const normalizedToolName = input.tool.toLowerCase() | ||
| if ( | ||
| normalizedToolName === "question" | ||
| || normalizedToolName === "ask_user_question" | ||
| || normalizedToolName === "askuserquestion" | ||
| ) { | ||
| const sessionID = input.sessionID || getMainSessionID() | ||
| await hooks.sessionNotification?.({ | ||
| event: { | ||
| type: "tool.execute.before", | ||
| properties: { | ||
| sessionID, | ||
| tool: input.tool, | ||
| args: output.args, | ||
| const normalizedToolName = input.tool.toLowerCase() | ||
| if ( | ||
| normalizedToolName === "question" | ||
| || normalizedToolName === "ask_user_question" | ||
| || normalizedToolName === "askuserquestion" | ||
| ) { | ||
| const sessionID = input.sessionID || getMainSessionID() | ||
| await hooks.sessionNotification?.({ | ||
| event: { | ||
| type: "tool.execute.before", | ||
| properties: { | ||
| sessionID, | ||
| tool: input.tool, | ||
| args: output.args, | ||
| }, | ||
| }, | ||
| }, | ||
| }) | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| if (input.tool === "task") { | ||
| const argsObject = output.args | ||
| const category = typeof argsObject.category === "string" ? argsObject.category : undefined | ||
| const subagentType = typeof argsObject.subagent_type === "string" ? argsObject.subagent_type : undefined | ||
| const sessionId = typeof argsObject.session_id === "string" ? argsObject.session_id : undefined | ||
| if (input.tool === "task") { | ||
| const argsObject = output.args | ||
| const category = typeof argsObject.category === "string" ? argsObject.category : undefined | ||
| const subagentType = typeof argsObject.subagent_type === "string" ? argsObject.subagent_type : undefined | ||
| const sessionId = typeof argsObject.session_id === "string" ? argsObject.session_id : undefined | ||
|
|
||
| if (category) { | ||
| argsObject.subagent_type = "sisyphus-junior" | ||
| } else if (!subagentType && sessionId) { | ||
| const resolvedAgent = await resolveSessionAgent(ctx.client, sessionId) | ||
| argsObject.subagent_type = resolvedAgent ?? "continue" | ||
| if (category) { | ||
| argsObject.subagent_type = "sisyphus-junior" | ||
| } else if (!subagentType && sessionId) { | ||
| const resolvedAgent = await resolveSessionAgent(ctx.client, sessionId) | ||
| argsObject.subagent_type = resolvedAgent ?? "continue" | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (hooks.ralphLoop && input.tool === "skill") { | ||
| const rawName = typeof output.args.name === "string" ? output.args.name : undefined | ||
| const command = rawName?.replace(/^\//, "").toLowerCase() | ||
| const sessionID = input.sessionID || getMainSessionID() | ||
| if (hooks.ralphLoop && input.tool === "skill") { | ||
| const rawName = typeof output.args.name === "string" ? output.args.name : undefined | ||
| const command = rawName?.replace(/^\//, "").toLowerCase() | ||
| const sessionID = input.sessionID || getMainSessionID() | ||
|
|
||
| if (command === "ralph-loop" && sessionID) { | ||
| const rawArgs = rawName?.replace(/^\/?(ralph-loop)\s*/i, "") || "" | ||
| const parsedArguments = parseRalphLoopArguments(rawArgs) | ||
| if (command === "ralph-loop" && sessionID) { | ||
| const rawArgs = rawName?.replace(/^\/?(ralph-loop)\s*/i, "") || "" | ||
| const parsedArguments = parseRalphLoopArguments(rawArgs) | ||
|
|
||
| hooks.ralphLoop.startLoop(sessionID, parsedArguments.prompt, { | ||
| maxIterations: parsedArguments.maxIterations, | ||
| completionPromise: parsedArguments.completionPromise, | ||
| strategy: parsedArguments.strategy, | ||
| }) | ||
| } else if (command === "cancel-ralph" && sessionID) { | ||
| hooks.ralphLoop.cancelLoop(sessionID) | ||
| } else if (command === "ulw-loop" && sessionID) { | ||
| const rawArgs = rawName?.replace(/^\/?(ulw-loop)\s*/i, "") || "" | ||
| const parsedArguments = parseRalphLoopArguments(rawArgs) | ||
| hooks.ralphLoop.startLoop(sessionID, parsedArguments.prompt, { | ||
| maxIterations: parsedArguments.maxIterations, | ||
| completionPromise: parsedArguments.completionPromise, | ||
| strategy: parsedArguments.strategy, | ||
| }) | ||
| } else if (command === "cancel-ralph" && sessionID) { | ||
| hooks.ralphLoop.cancelLoop(sessionID) | ||
| } else if (command === "ulw-loop" && sessionID) { | ||
| const rawArgs = rawName?.replace(/^\/?(ulw-loop)\s*/i, "") || "" | ||
| const parsedArguments = parseRalphLoopArguments(rawArgs) | ||
|
|
||
| hooks.ralphLoop.startLoop(sessionID, parsedArguments.prompt, { | ||
| ultrawork: true, | ||
| maxIterations: parsedArguments.maxIterations, | ||
| completionPromise: parsedArguments.completionPromise, | ||
| strategy: parsedArguments.strategy, | ||
| }) | ||
| hooks.ralphLoop.startLoop(sessionID, parsedArguments.prompt, { | ||
| ultrawork: true, | ||
| maxIterations: parsedArguments.maxIterations, | ||
| completionPromise: parsedArguments.completionPromise, | ||
| strategy: parsedArguments.strategy, | ||
| }) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (input.tool === "skill") { | ||
| const rawName = typeof output.args.name === "string" ? output.args.name : undefined | ||
| const command = rawName?.replace(/^\//, "").toLowerCase() | ||
| const sessionID = input.sessionID || getMainSessionID() | ||
| if (input.tool === "skill") { | ||
| const rawName = typeof output.args.name === "string" ? output.args.name : undefined | ||
| const command = rawName?.replace(/^\//, "").toLowerCase() | ||
| const sessionID = input.sessionID || getMainSessionID() | ||
|
|
||
| if (command === "stop-continuation" && sessionID) { | ||
| hooks.stopContinuationGuard?.stop(sessionID) | ||
| hooks.todoContinuationEnforcer?.cancelAllCountdowns() | ||
| hooks.ralphLoop?.cancelLoop(sessionID) | ||
| clearBoulderState(ctx.directory) | ||
| log("[stop-continuation] All continuation mechanisms stopped", { | ||
| sessionID, | ||
| }) | ||
| if (command === "stop-continuation" && sessionID) { | ||
| hooks.stopContinuationGuard?.stop(sessionID) | ||
| hooks.todoContinuationEnforcer?.cancelAllCountdowns() | ||
| hooks.ralphLoop?.cancelLoop(sessionID) | ||
| clearBoulderState(ctx.directory) | ||
| log("[stop-continuation] All continuation mechanisms stopped", { | ||
| sessionID, | ||
| }) | ||
| } | ||
| } | ||
| } catch (err: unknown) { | ||
| log("[tool-execute-before.ts] Unhandled hook error caught:", { error: err instanceof Error ? err.message : String(err) }) | ||
| } | ||
|
Comment on lines
+116
to
118
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As noted in the PR description, this For example: } catch (err: unknown) {
if (isSafetyCriticalHookError(err)) { // `isSafetyCriticalHookError` would need to be defined/imported
throw err;
}
log("[tool-execute-before.ts] Unhandled hook error caught:", { error: err instanceof Error ? err.message : String(err) })
} |
||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Silently swallowing errors can make debugging difficult. It's better to log the error to aid in troubleshooting potential issues with notifications.
You'll also need to import the
logfunction at the top of the file:import { log } from "../shared/logger";