diff --git a/docs/non-compaction-retry-policy.md b/docs/non-compaction-retry-policy.md index a6098ab4c..a66c3bff6 100644 --- a/docs/non-compaction-retry-policy.md +++ b/docs/non-compaction-retry-policy.md @@ -54,7 +54,7 @@ Current retryable categories include: The normalized classifier recognizes the transient categories above from structured flags/status and provider-aware text patterns. Classifier refusals remain a separate typed `stopDetails` decision. -Beyond `isRetryableError(...)`, empty generic aborts may enter the same retry engine when no user, dispose, or streaming-edit-guard abort is in progress. An interrupted turn whose tool calls already have matching results can also be continued safely: the failed assistant/tool-result sequence is preserved so completed side effects are not replayed. Resolved stream stalls and HTTP/2 stream resets (`NGHTTP2_INTERNAL_ERROR`, `NGHTTP2_REFUSED_STREAM`, `HTTP2StreamReset`) use the same preserve-and-continue path. Cursor idle-stall recovery still requires the exec-resolved marker (the Connect stream may still be open); an HTTP/2 RST does not, because the stream is already dead. +Beyond `isRetryableError(...)`, empty generic aborts may enter the same retry engine when no user, dispose, or streaming-edit-guard abort is in progress. An interrupted turn whose tool calls already have matching results can also be continued safely: the failed assistant/tool-result sequence is preserved so completed side effects are not replayed. Resolved stream stalls and HTTP/2 stream resets (`NGHTTP2_INTERNAL_ERROR`, `NGHTTP2_REFUSED_STREAM`, `HTTP2StreamReset`) use the same preserve-and-continue path. Cursor idle-stall recovery continues after every emitted tool call has a result; the Connect stream is already closed by the idle abort. An HTTP/2 RST is the same: the stream is already dead. Retry state is owned by `TurnRecovery`: diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 2f7b500ff..8f5c8c1fe 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Resume Cursor idle-stall turns after completed MCP/todo tool results. The watchdog already closes the Connect stream, so unmarked blocks no longer need the `exec-resolved` marker to continue. + ## [17.3.7] - 2026-08-17 ### Changed diff --git a/packages/coding-agent/src/session/turn-recovery.ts b/packages/coding-agent/src/session/turn-recovery.ts index a07cb117e..f26874691 100644 --- a/packages/coding-agent/src/session/turn-recovery.ts +++ b/packages/coding-agent/src/session/turn-recovery.ts @@ -20,7 +20,6 @@ import type { } from "@oh-my-pi/pi-ai"; import { calculateRateLimitBackoffMs, parseRateLimitReason } from "@oh-my-pi/pi-ai"; import * as AIError from "@oh-my-pi/pi-ai/error"; -import { kCursorExecResolved } from "@oh-my-pi/pi-ai/utils/block-symbols"; import { isFireworksFastModelId, toFireworksBaseModelId } from "@oh-my-pi/pi-catalog/fireworks-model-id"; import { modelsAreEqual } from "@oh-my-pi/pi-catalog/models"; import { extractRetryHint, logger, prompt } from "@oh-my-pi/pi-utils"; @@ -1177,25 +1176,15 @@ export class TurnRecovery { if (!reasonlessAbort && !streamStall && !transportReset) return undefined; if (reasonlessAbort && genericAbort) message.errorId = AIError.create(AIError.Flag.Abort); - // The Cursor server-execution marker gate applies only to the idle stream-stall - // path: an unmarked/unresolved Cursor block there means the server has not - // finished executing, so resuming would race it. A reasonless abort instead - // ends the turn and the agent loop pairs every un-run call (Cursor's unmarked - // `todo`/MCP blocks included) with a synthetic `executed: false` result, so - // the tool-result reconciliation below is the safety gate and the marker is - // irrelevant. An HTTP/2 RST_STREAM / NGHTTP2_* close also ends the Connect - // stream, so there is no in-flight server exec to race — unmarked MCP/todo - // blocks are safe to continue once every emitted call has a result. + // Idle stall and HTTP/2 RST both close the Cursor Connect stream: + // the lazy watchdog aborts the request signal, and cursor.ts then + // calls `h2Request.close()`. There is no in-flight server exec to + // race, so unmarked MCP/todo blocks can continue once every emitted + // call has a matching result. A reasonless abort ends the turn and + // the agent loop pairs leftover calls with `executed: false`. const resolvedToolCallIds: string[] = []; for (const block of message.content) { if (block.type !== "toolCall") continue; - if ( - streamStall && - message.provider === "cursor" && - (!(kCursorExecResolved in block) || block[kCursorExecResolved] !== true) - ) { - return undefined; - } resolvedToolCallIds.push(block.id); } if (resolvedToolCallIds.length === 0) return undefined; diff --git a/packages/coding-agent/test/agent-session-retry-cap.test.ts b/packages/coding-agent/test/agent-session-retry-cap.test.ts index e7902693e..52d57f851 100644 --- a/packages/coding-agent/test/agent-session-retry-cap.test.ts +++ b/packages/coding-agent/test/agent-session-retry-cap.test.ts @@ -1386,6 +1386,128 @@ describe("AgentSession retry delay cap", () => { }); }); + it("resumes a Cursor idle stall after an unmarked MCP tool result", async () => { + const stallMessage = "Provider stream stalled while waiting for the next event"; + const model = createMockModel({ + id: "composer-2.5", + provider: "cursor", + }); + authStorage.setRuntimeApiKey("cursor", "cursor-test-key"); + const toolCall: ToolCall = { + type: "toolCall", + id: "cursor-mcp-idle-1", + name: "mcp__databricks_production_execute_sql", + arguments: { query: "SELECT 1" }, + }; + const toolResult: ToolResultMessage = { + role: "toolResult", + toolCallId: toolCall.id, + toolName: toolCall.name, + content: [{ type: "text", text: "1" }], + isError: false, + timestamp: Date.now(), + }; + let streamCalls = 0; + let resumedWithToolResult = false; + const agent = new Agent({ + getApiKey: requestedModel => `${requestedModel.provider}-test-key`, + initialState: { + model, + systemPrompt: ["Test"], + tools: [], + messages: [], + }, + cursorOnToolResult: message => message, + streamFn: (_requestedModel, context, options) => { + streamCalls += 1; + if (streamCalls > 1) { + resumedWithToolResult = context.messages.some( + message => message.role === "toolResult" && message.toolCallId === toolCall.id, + ); + model.push({ content: ["Recovered after Cursor idle stall"] }); + return model.stream(model, context, options); + } + + const stream = new AssistantMessageEventStream(); + queueMicrotask(async () => { + await options?.cursorOnToolResult?.(toolResult); + const partial: AssistantMessage = { + role: "assistant", + content: [toolCall], + api: model.api, + provider: model.provider, + model: model.id, + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "stop", + timestamp: Date.now(), + }; + stream.push({ type: "start", partial }); + stream.push({ type: "toolcall_start", contentIndex: 0, partial }); + stream.push({ + type: "toolcall_delta", + contentIndex: 0, + delta: JSON.stringify(toolCall.arguments), + partial, + }); + stream.push({ type: "toolcall_end", contentIndex: 0, toolCall, partial }); + stream.push({ + type: "error", + reason: "error", + error: { + ...partial, + stopReason: "error", + errorMessage: stallMessage, + }, + }); + }); + return stream; + }, + }); + + const settings = Settings.isolated({ + "compaction.enabled": false, + "retry.baseDelayMs": 5, + "retry.maxRetries": 1, + }); + settings.setModelRole("default", `${model.provider}/${model.id}`); + session = new AgentSession({ + agent, + sessionManager: SessionManager.inMemory(), + settings, + modelRegistry, + }); + const retryStartEvents: AutoRetryStartEvent[] = []; + const retryEndEvents: AutoRetryEndEvent[] = []; + session.subscribe(event => { + if (event.type === "auto_retry_start") retryStartEvents.push(event); + if (event.type === "auto_retry_end") retryEndEvents.push(event); + }); + + await session.prompt("Run the query"); + await session.waitForIdle(); + + expect(streamCalls).toBe(2); + expect(resumedWithToolResult).toBe(true); + expect( + session.agent.state.messages.some( + message => message.role === "toolResult" && message.toolCallId === toolCall.id, + ), + ).toBe(true); + expect(retryStartEvents).toHaveLength(1); + expect(retryEndEvents).toContainEqual(expect.objectContaining({ success: true, attempt: 1 })); + expect(lastAssistant(session).content).toContainEqual({ + type: "text", + text: "Recovered after Cursor idle stall", + }); + }); + it("resumes a Cursor reasonless abort after an unmarked client-side tool call", async () => { const model = createMockModel({ id: "composer-2.5", diff --git a/packages/coding-agent/test/turn-recovery-replay-unsafe.test.ts b/packages/coding-agent/test/turn-recovery-replay-unsafe.test.ts index a880bc4e1..043b874ac 100644 --- a/packages/coding-agent/test/turn-recovery-replay-unsafe.test.ts +++ b/packages/coding-agent/test/turn-recovery-replay-unsafe.test.ts @@ -603,10 +603,10 @@ describe("TurnRecovery replay-unsafe output classification", () => { expect(recovery.classifyResolvedInterruptedToolTurn(message)).toBe("stream-stall"); }); - it("does not continue a Cursor idle stall after an unmarked MCP call", () => { + it("continues a Cursor idle stall after an unmarked MCP call", () => { const message = cursorMessage([mcpToolCall("mcp-1")], stallMessage); const recovery = recoveryForReset(message, [realResult("mcp-1", "mcp__databricks_production_execute_sql")]); - expect(recovery.classifyResolvedInterruptedToolTurn(message)).toBeUndefined(); + expect(recovery.classifyResolvedInterruptedToolTurn(message)).toBe("stream-stall"); }); it("does not continue an HTTP/2 reset whose tool call has no result", () => {