From d3a8eb8a1c58f31a1cf83fff576bc2fe2c5ab97e Mon Sep 17 00:00:00 2001 From: Ayaan Zaidi Date: Sat, 11 Jul 2026 13:11:35 +0530 Subject: [PATCH] refactor(agents): converge CLI tool terminal reason onto run-termination One shared resolveCliToolTerminalReason owns the timed_out/cancelled/failed decision for one-shot and live CLI runners; abort-signal lifecycle fields are authoritative, error shape is the fallback. Deletes both local derivations. Fixes live-path drift where a timeout abort delivered as a generic AbortError was reported as cancelled instead of timed_out. Part of #104219 (seam 3). --- src/agents/cli-runner/claude-live-session.ts | 19 +++- src/agents/cli-runner/execute.ts | 16 +--- src/agents/run-termination.test.ts | 96 ++++++++++++++++++++ src/agents/run-termination.ts | 25 ++++- 4 files changed, 139 insertions(+), 17 deletions(-) diff --git a/src/agents/cli-runner/claude-live-session.ts b/src/agents/cli-runner/claude-live-session.ts index d5511c30747b..9a05c88c5005 100644 --- a/src/agents/cli-runner/claude-live-session.ts +++ b/src/agents/cli-runner/claude-live-session.ts @@ -40,6 +40,7 @@ import { } from "../cli-output.js"; import { classifyFailoverReason } from "../embedded-agent-helpers.js"; import { FailoverError, isTimeoutError, resolveFailoverStatus } from "../failover-error.js"; +import { resolveCliToolTerminalReason } from "../run-termination.js"; import { prepareCliBundleMcpCaptureAttempt } from "./bundle-mcp.js"; import { buildClaudeOwnerKey } from "./helpers.js"; import { cliBackendLog, formatCliBackendOutputDigest } from "./log.js"; @@ -52,6 +53,8 @@ type ManagedRun = Awaited>; type ClaudeLiveTurn = { backend: CliBackendConfig; diagnosticRefs: ClaudeLiveDiagnosticRefs; + /** Enclosing run abort signal; authoritative for tool terminal reason on turn failure. */ + abortSignal?: AbortSignal; outputLimits: ClaudeLiveOutputLimits; startedAtMs: number; rawLines: string[]; @@ -670,11 +673,16 @@ function markClaudeLiveToolDenied(turn: ClaudeLiveTurn, tool: CliToolUseStartDel } function failActiveClaudeLiveTools(turn: ClaudeLiveTurn, error: unknown): void { - const timedOut = - isTimeoutError(error) || (error instanceof FailoverError && error.reason === "timeout"); - const aborted = error instanceof Error && error.name === "AbortError"; - const errorCategory = timedOut ? "timeout" : aborted ? "aborted" : "error"; - const terminalReason = timedOut ? "timed_out" : aborted ? "cancelled" : "failed"; + const terminalReason = resolveCliToolTerminalReason({ + error, + abortSignal: turn.abortSignal, + }); + const errorCategory = + terminalReason === "timed_out" + ? "timeout" + : terminalReason === "cancelled" + ? "aborted" + : "error"; for (const activeTool of turn.activeTools.values()) { const event: Omit = { @@ -1188,6 +1196,7 @@ function createTurn(params: { ...(params.context.params.sessionKey ? { sessionKey: params.context.params.sessionKey } : {}), ...(params.context.params.agentId ? { agentId: params.context.params.agentId } : {}), }, + abortSignal: params.context.params.abortSignal, outputLimits: resolveCliStreamJsonOutputLimits(params.context.preparedBackend.backend), startedAtMs: Date.now(), rawLines: [], diff --git a/src/agents/cli-runner/execute.ts b/src/agents/cli-runner/execute.ts index 082f23e9c209..cf84771f50a0 100644 --- a/src/agents/cli-runner/execute.ts +++ b/src/agents/cli-runner/execute.ts @@ -69,7 +69,7 @@ import { } from "../embedded-agent-subscribe.tools.js"; import { FailoverError, resolveFailoverStatus } from "../failover-error.js"; import { applyPluginTextReplacements } from "../plugin-text-transforms.js"; -import { resolveAgentRunAbortLifecycleFields } from "../run-termination.js"; +import { resolveCliToolTerminalReason } from "../run-termination.js"; import { prepareCliBundleMcpCaptureAttempt } from "./bundle-mcp.js"; import { rotateClaudeLiveMcpCaptureKeyForContext, @@ -845,15 +845,6 @@ export async function executePreparedCliRun( : {}), }; }; - const resolveToolTerminalReason = (error?: unknown) => { - const abortFields = resolveAgentRunAbortLifecycleFields(params.abortSignal); - if (abortFields.aborted) { - return abortFields.stopReason === "timeout" ? "timed_out" : "cancelled"; - } - return error instanceof FailoverError && error.reason === "timeout" - ? "timed_out" - : "failed"; - }; let finalizeParsedTools = () => {}; try { cliBackendLog.info( @@ -1301,7 +1292,10 @@ export async function executePreparedCliRun( : undefined; const terminalReason = trustedTerminalReason ?? - resolveToolTerminalReason(event.incomplete ? runError : undefined); + resolveCliToolTerminalReason({ + error: event.incomplete ? runError : undefined, + abortSignal: params.abortSignal, + }); // Incomplete client/MCP tools inherit the enclosing failed run even when // the loopback disconnect is ambiguous. Server-native tools do not. const useEnclosingTerminalReason = diff --git a/src/agents/run-termination.test.ts b/src/agents/run-termination.test.ts index 0b2fe523a293..c6aa65579cc2 100644 --- a/src/agents/run-termination.test.ts +++ b/src/agents/run-termination.test.ts @@ -7,8 +7,104 @@ import { isAbortedAgentStopReason, resolveAgentRunAbortLifecycleFields, resolveAgentRunErrorLifecycleFields, + resolveCliToolTerminalReason, } from "./run-termination.js"; +describe("resolveCliToolTerminalReason", () => { + it.each([ + { + name: "abort-timeout", + setup: () => { + const controller = new AbortController(); + const timeout = new Error("timed out"); + timeout.name = "TimeoutError"; + controller.abort(timeout); + return { abortSignal: controller.signal, error: new Error("other") }; + }, + expected: "timed_out", + }, + { + name: "abort-cancel", + setup: () => { + const controller = new AbortController(); + controller.abort(); + return { abortSignal: controller.signal, error: undefined }; + }, + expected: "cancelled", + }, + { + name: "restart-abort reason", + setup: () => { + const controller = new AbortController(); + controller.abort(createAgentRunRestartAbortError()); + return { abortSignal: controller.signal, error: undefined }; + }, + expected: "cancelled", + }, + { + name: "FailoverError timeout", + setup: () => ({ + error: new FailoverError("CLI timed out", { reason: "timeout" }), + }), + expected: "timed_out", + }, + { + name: "isTimeoutError error", + setup: () => { + const error = new Error("request timed out"); + error.name = "TimeoutError"; + return { error }; + }, + expected: "timed_out", + }, + { + name: "AbortError", + setup: () => { + const error = new Error("CLI run aborted"); + error.name = "AbortError"; + return { error }; + }, + expected: "cancelled", + }, + { + name: "plain Error", + setup: () => ({ error: new Error("tool failed") }), + expected: "failed", + }, + { + name: "undefined error", + setup: () => ({ error: undefined }), + expected: "failed", + }, + { + name: "timeout abort wins over generic AbortError", + setup: () => { + const controller = new AbortController(); + const timeout = new Error("timed out"); + timeout.name = "TimeoutError"; + controller.abort(timeout); + const error = new Error("CLI run aborted"); + error.name = "AbortError"; + return { abortSignal: controller.signal, error }; + }, + expected: "timed_out", + }, + { + name: "cancel abort wins over timeout-shaped error", + setup: () => { + const controller = new AbortController(); + controller.abort(); + const error = new Error("request timed out"); + error.name = "TimeoutError"; + return { abortSignal: controller.signal, error }; + }, + expected: "cancelled", + }, + ] as const)("$name", ({ setup, expected }) => { + expect(resolveCliToolTerminalReason(setup())).toBe(expected); + }); +}); + describe("resolveAgentRunAbortLifecycleFields", () => { it("classifies generic cancellation as aborted", () => { const controller = new AbortController(); diff --git a/src/agents/run-termination.ts b/src/agents/run-termination.ts index 453ec618565a..b9a278d22828 100644 --- a/src/agents/run-termination.ts +++ b/src/agents/run-termination.ts @@ -1,4 +1,4 @@ -import { isFailoverError } from "./failover-error.js"; +import { isFailoverError, isTimeoutError } from "./failover-error.js"; import type { AgentRunTimeoutPhase } from "./run-timeout-attribution.js"; /** @@ -124,3 +124,26 @@ export function isAbortedAgentStopReason( ): value is typeof AGENT_RUN_ABORTED_STOP_REASON | typeof AGENT_RUN_RESTART_ABORT_STOP_REASON { return value === AGENT_RUN_ABORTED_STOP_REASON || value === AGENT_RUN_RESTART_ABORT_STOP_REASON; } + +/** + * CLI tool terminal reason for one-shot and live runners. + * Abort-signal lifecycle is authoritative so a timeout abort stays timed_out + * even when the delivered error is a generic AbortError. + */ +export function resolveCliToolTerminalReason(params: { + error?: unknown; + abortSignal?: AbortSignal; +}): "timed_out" | "cancelled" | "failed" { + const abortFields = resolveAgentRunAbortLifecycleFields(params.abortSignal); + if (abortFields.aborted) { + return abortFields.stopReason === "timeout" ? "timed_out" : "cancelled"; + } + const { error } = params; + if (isTimeoutError(error) || (isFailoverError(error) && error.reason === "timeout")) { + return "timed_out"; + } + if (error instanceof Error && error.name === "AbortError") { + return "cancelled"; + } + return "failed"; +}