From 4de9b79d30757542906ca22f748eccdcee264991 Mon Sep 17 00:00:00 2001 From: Ayaan Zaidi Date: Sun, 31 May 2026 17:04:33 +0530 Subject: [PATCH] refactor(agents): simplify stale cli retry cleanup --- src/agents/cli-runner.ts | 100 ++++++++++-------------- src/agents/command/attempt-execution.ts | 42 ++++------ test/scripts/lint-suppressions.test.ts | 1 - 3 files changed, 59 insertions(+), 84 deletions(-) diff --git a/src/agents/cli-runner.ts b/src/agents/cli-runner.ts index 4cea6931b4d..3586e8cb3e7 100644 --- a/src/agents/cli-runner.ts +++ b/src/agents/cli-runner.ts @@ -61,16 +61,19 @@ function shouldRetryFreshCliSessionAfterFailover(params: { error: FailoverError; hasHistoryPrompt: boolean; }): boolean { - // These reasons can mean only the resumed CLI session is bad. Auth, billing, - // rate-limit, and provider/config errors should clear stale state but surface. if (!params.hasHistoryPrompt) { return false; } - return ( - params.error.reason === "session_expired" || - (params.error.reason === "unknown" && params.error.code === "cli_unknown_empty_failure") || - (params.error.reason === "timeout" && params.error.code === "cli_no_output_timeout") - ); + switch (params.error.reason) { + case "session_expired": + return true; + case "unknown": + return params.error.code === "cli_unknown_empty_failure"; + case "timeout": + return params.error.code === "cli_no_output_timeout"; + default: + return false; + } } export async function isCliBindingFlushed( @@ -666,7 +669,6 @@ export async function runPreparedCliAgent( }; }; - // Try with the provided CLI session ID first try { await bootstrapHarnessContextEngine({ hadSessionFile: context.hadSessionFile, @@ -686,6 +688,35 @@ export async function runPreparedCliAgent( config: params.config, }) : []; + const finishCliAttempt = async ( + result: Awaited>, + fallbackCliSessionId?: string, + ) => { + const { output, lastAssistant } = result; + const assistantText = output.text.trim(); + const effectiveCliSessionId = output.sessionId ?? fallbackCliSessionId; + await finalizeCliContextEngineTurn({ + context, + historyMessages: context.contextEngine ? contextEngineHistoryMessages : historyMessages, + assistantText, + output, + }); + const bindingFlushOk = await isCliBindingFlushed( + effectiveCliSessionId, + params.provider, + context.cwd ?? context.workspaceDir, + ); + await runCliAgentEndHook(params, { + event: { + messages: buildAgentEndMessages(lastAssistant), + success: true, + durationMs: Date.now() - context.started, + }, + ctx: hookContext, + hookRunner, + }); + return buildCliRunResult({ output, effectiveCliSessionId, bindingFlushOk }); + }; if (hasBeforeAgentRunHooks && hookRunner) { let beforeRunResult: @@ -747,32 +778,10 @@ export async function runPreparedCliAgent( hookRunner, }); try { - const { output, lastAssistant } = await executeCliAttempt( + return await finishCliAttempt( + await executeCliAttempt(context.reusableCliSession.sessionId), context.reusableCliSession.sessionId, ); - const assistantText = output.text.trim(); - const effectiveCliSessionId = output.sessionId ?? context.reusableCliSession.sessionId; - await finalizeCliContextEngineTurn({ - context, - historyMessages: context.contextEngine ? contextEngineHistoryMessages : historyMessages, - assistantText, - output, - }); - const bindingFlushOk = await isCliBindingFlushed( - effectiveCliSessionId, - params.provider, - context.cwd ?? context.workspaceDir, - ); - await runCliAgentEndHook(params, { - event: { - messages: buildAgentEndMessages(lastAssistant), - success: true, - durationMs: Date.now() - context.started, - }, - ctx: hookContext, - hookRunner, - }); - return buildCliRunResult({ output, effectiveCliSessionId, bindingFlushOk }); } catch (err) { if (isFailoverError(err)) { const retryableSessionId = context.reusableCliSession.sessionId; @@ -802,32 +811,7 @@ export async function runPreparedCliAgent( cliBackendLog.warn( `cli session recovery retry: provider=${params.provider} reason=${err.reason} sessionKey=${params.sessionKey}`, ); - const { output, lastAssistant } = await executeCliAttempt(undefined, retryTimeoutMs); - const assistantText = output.text.trim(); - const effectiveCliSessionId = output.sessionId; - await finalizeCliContextEngineTurn({ - context, - historyMessages: context.contextEngine - ? contextEngineHistoryMessages - : historyMessages, - assistantText, - output, - }); - const bindingFlushOk = await isCliBindingFlushed( - effectiveCliSessionId, - params.provider, - context.cwd ?? context.workspaceDir, - ); - await runCliAgentEndHook(params, { - event: { - messages: buildAgentEndMessages(lastAssistant), - success: true, - durationMs: Date.now() - context.started, - }, - ctx: hookContext, - hookRunner, - }); - return buildCliRunResult({ output, effectiveCliSessionId, bindingFlushOk }); + return await finishCliAttempt(await executeCliAttempt(undefined, retryTimeoutMs)); } catch (retryErr) { const retryMessage = formatErrorMessage(retryErr); await runCliAgentEndHook(params, { diff --git a/src/agents/command/attempt-execution.ts b/src/agents/command/attempt-execution.ts index b86a52c74c9..5d536458c81 100644 --- a/src/agents/command/attempt-execution.ts +++ b/src/agents/command/attempt-execution.ts @@ -509,6 +509,14 @@ export function runAgentAttempt(params: { if (!isRawModelRun && isCliProvider(cliExecutionProvider, params.cfg)) { const cliSessionBinding = getCliSessionBinding(params.sessionEntry, cliExecutionProvider); const cliProcessCwd = params.cwd ? resolveUserPath(params.cwd) : params.workspaceDir; + const mutableCliSessionStore = + params.sessionKey && params.sessionStore && params.storePath + ? { + sessionKey: params.sessionKey, + sessionStore: params.sessionStore, + storePath: params.storePath, + } + : undefined; const resolveReusableCliSessionBinding = async () => { if ( !isClaudeCliProvider(cliExecutionProvider) || @@ -525,21 +533,16 @@ export function runAgentAttempt(params: { `cli session reset: provider=${sanitizeForLog(cliExecutionProvider)} reason=transcript-missing sessionKey=${params.sessionKey ?? params.sessionId}`, ); - if (params.sessionKey && params.sessionStore && params.storePath) { + if (mutableCliSessionStore) { params.sessionEntry = (await clearCliSessionInStore({ provider: cliExecutionProvider, - sessionKey: params.sessionKey, - sessionStore: params.sessionStore, - storePath: params.storePath, + ...mutableCliSessionStore, })) ?? params.sessionEntry; } return undefined; }; - const canClearCliSessionBeforeRetry = Boolean( - params.sessionKey && params.sessionStore && params.storePath, - ); const runCliWithSession = ( nextCliSessionId: string | undefined, activeCliSessionBinding = cliSessionBinding, @@ -581,28 +584,21 @@ export function runAgentAttempt(params: { toolsAllow: params.opts.toolsAllow, cleanupBundleMcpOnRunEnd: params.opts.cleanupBundleMcpOnRunEnd, cleanupCliLiveSessionOnRunEnd: params.opts.cleanupCliLiveSessionOnRunEnd, - ...(canClearCliSessionBeforeRetry + ...(mutableCliSessionStore ? { onBeforeFreshCliSessionRetry: async (retry) => { - if ( - retry.sessionId !== activeCliSessionBinding?.sessionId || - !params.sessionKey || - !params.sessionStore || - !params.storePath - ) { + if (retry.sessionId !== activeCliSessionBinding?.sessionId) { return false; } log.warn( - `CLI session failed, clearing before fresh retry: provider=${sanitizeForLog(cliExecutionProvider)} sessionKey=${params.sessionKey} reason=${sanitizeForLog(retry.reason)}`, + `CLI session failed, clearing before fresh retry: provider=${sanitizeForLog(cliExecutionProvider)} sessionKey=${mutableCliSessionStore.sessionKey} reason=${sanitizeForLog(retry.reason)}`, ); params.sessionEntry = (await clearCliSessionInStore({ provider: cliExecutionProvider, - sessionKey: params.sessionKey, - sessionStore: params.sessionStore, - storePath: params.storePath, + ...mutableCliSessionStore, })) ?? params.sessionEntry; return true; }, @@ -617,20 +613,16 @@ export function runAgentAttempt(params: { isClaudeCliProvider(cliExecutionProvider) && shouldClearReusedCliSessionAfterError(err) && activeCliSessionBinding?.sessionId && - params.sessionKey && - params.sessionStore && - params.storePath + mutableCliSessionStore ) { log.warn( - `CLI session cleared after failed reused turn: provider=${sanitizeForLog(cliExecutionProvider)} sessionKey=${params.sessionKey} reason=${sanitizeForLog(resolveClearedCliSessionReason(err))}`, + `CLI session cleared after failed reused turn: provider=${sanitizeForLog(cliExecutionProvider)} sessionKey=${mutableCliSessionStore.sessionKey} reason=${sanitizeForLog(resolveClearedCliSessionReason(err))}`, ); params.sessionEntry = (await clearCliSessionInStore({ provider: cliExecutionProvider, - sessionKey: params.sessionKey, - sessionStore: params.sessionStore, - storePath: params.storePath, + ...mutableCliSessionStore, })) ?? params.sessionEntry; } throw err; diff --git a/test/scripts/lint-suppressions.test.ts b/test/scripts/lint-suppressions.test.ts index a226b7cce18..0f5df1f7eab 100644 --- a/test/scripts/lint-suppressions.test.ts +++ b/test/scripts/lint-suppressions.test.ts @@ -108,7 +108,6 @@ function collectProductionLintSuppressionsFromGit(): SuppressionEntry[] | null { "git", [ "grep", - "--cached", "-n", "-E", String.raw`(oxlint|eslint)-disable(-next-line)?[[:space:]]+[@/[:alnum:]_-]+`,