From 8b17f0d3a17478682ba494892d122a0c9b40e61c Mon Sep 17 00:00:00 2001 From: roboomp Date: Sat, 4 Jul 2026 17:38:51 +0000 Subject: [PATCH] fix(coding-agent): scoped TTSR abort reason to matching tool call TTSR stream-interrupt aborts now carry a per-tool reason so the placeholder loop labels only the tool call whose stream matched the rule with the rule name and gives sibling committed tool calls a neutral "TTSR interrupt on another tool call" reason. Previously the single `message.errorMessage` was stamped onto every retained tool-call block, so unrelated read/edit calls read as violating a rule they never matched and misled the model's own reasoning about which call fired. Threads the matched `toolcall:` extracted from the TTSR match context through `agent.abort(...)` as a `ToolScopedAbortReason` object; the agent loop unwraps it in `emitAbortedAssistantMessage` into a `toolCallAbortMessages` map on the aborted `AssistantMessage`, and the `stopReason === "aborted"` fanout in `runAgentLoop` prefers the per-tool message when one exists. Fixes #2783 --- packages/agent/CHANGELOG.md | 4 ++ packages/agent/src/agent-loop.ts | 60 ++++++++++++++++++- packages/ai/CHANGELOG.md | 4 ++ packages/ai/src/types.ts | 2 + packages/coding-agent/CHANGELOG.md | 4 ++ .../coding-agent/src/session/agent-session.ts | 15 ++++- .../test/agent-session-concurrent.test.ts | 46 +++++++------- 7 files changed, 112 insertions(+), 23 deletions(-) diff --git a/packages/agent/CHANGELOG.md b/packages/agent/CHANGELOG.md index 26f162c85..d0f642b47 100644 --- a/packages/agent/CHANGELOG.md +++ b/packages/agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] + +### Added + +- Added per-tool abort metadata so stream-wide aborts can label matching tool-call placeholders separately from unaffected sibling calls ([#2783](https://github.com/can1357/oh-my-pi/issues/2783)). ## [16.0.1] - 2026-06-15 ### Fixed diff --git a/packages/agent/src/agent-loop.ts b/packages/agent/src/agent-loop.ts index bfff2aefe..a17798cd1 100644 --- a/packages/agent/src/agent-loop.ts +++ b/packages/agent/src/agent-loop.ts @@ -83,6 +83,26 @@ const MAX_PAUSED_TURN_CONTINUATIONS = 8; * tool's own window elapses. A cheap synchronous queue check; latency-bounded * at one tick. */ +/** + * Abort reason for a turn-wide interruption where only some tool calls caused + * the abort and sibling placeholders need neutral messages. + */ +export interface ToolScopedAbortReason { + readonly kind: "tool-scoped-abort"; + readonly message: string; + readonly toolCallMessages: Record; + readonly defaultToolCallMessage: string; +} + +/** Creates an abort reason that labels matching tool calls separately from siblings. */ +export function createToolScopedAbortReason( + message: string, + toolCallMessages: Record, + defaultToolCallMessage: string, +): ToolScopedAbortReason { + return { kind: "tool-scoped-abort", message, toolCallMessages, defaultToolCallMessage }; +} + const STEERING_INTERRUPT_POLL_MS = 250; class HarmonyLeakInterruption extends Error { @@ -158,6 +178,7 @@ function snapshotAssistantMessage(message: AssistantMessage): AssistantMessage { cost: { ...message.usage.cost }, }, disabledFeatures: message.disabledFeatures ? [...message.disabledFeatures] : undefined, + toolCallAbortMessages: message.toolCallAbortMessages ? { ...message.toolCallAbortMessages } : undefined, }; } @@ -761,7 +782,8 @@ async function runLoopBody( const toolCalls = message.content.filter((c): c is ToolCallContent => c.type === "toolCall"); const toolResults: ToolResultMessage[] = []; for (const toolCall of toolCalls) { - const result = createAbortedToolResult(toolCall, stream, message.stopReason, message.errorMessage); + const errorMessage = message.toolCallAbortMessages?.[toolCall.id] ?? message.errorMessage; + const result = createAbortedToolResult(toolCall, stream, message.stopReason, errorMessage); currentContext.messages.push(result); newMessages.push(result); toolResults.push(result); @@ -1392,6 +1414,34 @@ function emitDiscardedHarmonyPartial( }); } +function isStringRecord(value: unknown): value is Record { + if (!value || typeof value !== "object" || Array.isArray(value)) return false; + return Object.values(value).every(child => typeof child === "string"); +} + +function toolScopedAbortReason(signal: AbortSignal | undefined): ToolScopedAbortReason | undefined { + const reason = signal?.reason; + if (!reason || typeof reason !== "object") return undefined; + if (Reflect.get(reason, "kind") !== "tool-scoped-abort") return undefined; + if (typeof Reflect.get(reason, "message") !== "string") return undefined; + if (typeof Reflect.get(reason, "defaultToolCallMessage") !== "string") return undefined; + return isStringRecord(Reflect.get(reason, "toolCallMessages")) ? reason : undefined; +} + +function buildToolCallAbortMessages( + message: AssistantMessage, + reason: ToolScopedAbortReason, +): Record | undefined { + let hasToolCall = false; + const messages: Record = {}; + for (const block of message.content) { + if (block.type !== "toolCall") continue; + hasToolCall = true; + messages[block.id] = reason.toolCallMessages[block.id] ?? reason.defaultToolCallMessage; + } + return hasToolCall ? messages : undefined; +} + /** Resolve the human-readable reason an abort carried. A caller that aborts via * `AbortController.abort(reason)` with a string or a non-`AbortError` `Error` * (e.g. the coding agent's user-interrupt label) gets that text surfaced on the @@ -1399,6 +1449,8 @@ function emitDiscardedHarmonyPartial( * `signal.reason` is the default `AbortError` `DOMException`) falls back to the * generic sentinel that downstream renderers treat as "no specific reason". */ export function abortReasonText(signal: AbortSignal | undefined): string { + const scopedReason = toolScopedAbortReason(signal); + if (scopedReason) return scopedReason.message; const reason = signal?.reason; if (typeof reason === "string" && reason.trim().length > 0) return reason; if (reason instanceof Error && reason.name !== "AbortError" && reason.message.trim().length > 0) { @@ -1415,6 +1467,7 @@ export function abortReasonText(signal: AbortSignal | undefined): string { * was observed, so the block is retained and paired with a labeled placeholder; * an anonymous abort drops incomplete calls whose args may be unsafe to replay. */ function isExplicitAbortReason(signal: AbortSignal | undefined): boolean { + if (toolScopedAbortReason(signal)) return true; const reason = signal?.reason; if (typeof reason === "string") return reason.trim().length > 0; if (reason instanceof Error) return reason.name !== "AbortError" && reason.message.trim().length > 0; @@ -1456,6 +1509,11 @@ function emitAbortedAssistantMessage( // `errorMessage`; an anonymous abort still drops calls that never completed // (no `toolcall_end`), whose partial args are unsafe to replay. const retained = isExplicitAbortReason(requestSignal) ? base : retainCompletedToolCalls(base, completedToolCallIds); + const scopedAbort = toolScopedAbortReason(requestSignal); + const toolCallAbortMessages = scopedAbort ? buildToolCallAbortMessages(retained, scopedAbort) : undefined; + if (toolCallAbortMessages) { + retained.toolCallAbortMessages = toolCallAbortMessages; + } const abortedMessage = snapshotAssistantMessage(retained); if (addedPartial) { context.messages[context.messages.length - 1] = abortedMessage; diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index bb43d50d0..50758769b 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] + +### Added + +- Added `AssistantMessage.toolCallAbortMessages` for per-tool placeholder labels on aborted assistant turns ([#2783](https://github.com/can1357/oh-my-pi/issues/2783)). ## [16.0.1] - 2026-06-15 ### Added diff --git a/packages/ai/src/types.ts b/packages/ai/src/types.ts index 9bd48a5ef..9a55eef0c 100644 --- a/packages/ai/src/types.ts +++ b/packages/ai/src/types.ts @@ -495,6 +495,8 @@ export interface AssistantMessage { stopReason: StopReason; stopDetails?: StopDetails | null; errorMessage?: string; + /** Per-tool abort messages used when an aborted assistant turn needs different placeholder results per tool call. */ + toolCallAbortMessages?: Record; /** HTTP status surfaced by the provider when the request failed. Populated by every provider's catch block alongside `errorMessage` so consumers (auth retry, telemetry, UI) can branch without regex-scraping the message. */ errorStatus?: number; /** diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index ce58d520d..6142c5e8b 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] + +### Fixed + +- Fixed TTSR stream interrupts so only the tool call whose stream matched a rule receives the rule-named abort result; sibling tool-call placeholders now use a neutral abort reason ([#2783](https://github.com/can1357/oh-my-pi/issues/2783)). ## [16.0.1] - 2026-06-15 ### Breaking Changes diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index a5f30688c..3f589a3e9 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -31,6 +31,7 @@ import { AppendOnlyContextManager, type AsideMessage, type CompactionSummaryMessage, + createToolScopedAbortReason, resolveTelemetry, STREAM_INTERRUPTED_AFTER_CONTENT_STOP_DETAIL, ThinkingLevel, @@ -2946,7 +2947,8 @@ export class AgentSession { // Decide first: a non-interrupting tool-source match attaches to the // specific tool call's result instead of driving a loop-wide follow-up. const shouldInterrupt = this.#shouldInterruptForTtsrMatch(matches, matchContext); - const perToolId = shouldInterrupt ? undefined : this.#extractTtsrToolCallId(matchContext); + const matchedToolId = this.#extractTtsrToolCallId(matchContext); + const perToolId = shouldInterrupt ? undefined : matchedToolId; if (perToolId) { this.#addPerToolTtsrInjections(perToolId, matches); this.#emitSessionEvent({ type: "ttsr_triggered", rules: matches }).catch(() => {}); @@ -2962,7 +2964,16 @@ export class AgentSession { // Abort the stream immediately — do not gate on extension callbacks this.#ttsrAbortPending = true; this.#ensureTtsrResumePromise(); - this.agent.abort(this.#formatTtsrAbortReason(matches)); + const abortReason = this.#formatTtsrAbortReason(matches); + this.agent.abort( + matchedToolId + ? createToolScopedAbortReason( + abortReason, + { [matchedToolId]: abortReason }, + "TTSR interrupt on another tool call", + ) + : abortReason, + ); // Notify extensions (fire-and-forget, does not block abort) this.#emitSessionEvent({ type: "ttsr_triggered", rules: matches }).catch(() => {}); // Schedule retry after a short delay diff --git a/packages/coding-agent/test/agent-session-concurrent.test.ts b/packages/coding-agent/test/agent-session-concurrent.test.ts index cf7504b18..4d31a537b 100644 --- a/packages/coding-agent/test/agent-session-concurrent.test.ts +++ b/packages/coding-agent/test/agent-session-concurrent.test.ts @@ -755,7 +755,7 @@ describe("AgentSession TTSR resume gate", () => { expect(session.isStreaming).toBe(false); }); - it("labels aborted tool placeholders with the TTSR rule reason", async () => { + it("labels only the matching aborted tool placeholder with the TTSR rule reason", async () => { collapseSchedulerSettleDelays(); const model = getBundledModel("anthropic", "claude-sonnet-4-5")!; let streamCallCount = 0; @@ -769,7 +769,13 @@ describe("AgentSession TTSR resume gate", () => { }); ttsrManager.addRule(testRule); - const toolCallContent: ToolCall = { + const readToolCallContent: ToolCall = { + type: "toolCall", + id: "call_innocent_read", + name: "read", + arguments: { path: "history://Eval1WithSkill" }, + }; + const matchedToolCallContent: ToolCall = { type: "toolCall", id: "call_ttsr_abort_reason", name: "mock_edit", @@ -778,7 +784,7 @@ describe("AgentSession TTSR resume gate", () => { const makeToolCallMsg = (stopReason: "toolUse" | "aborted" = "toolUse"): AssistantMessage => ({ role: "assistant", - content: [toolCallContent], + content: [readToolCallContent, matchedToolCallContent], api: "anthropic-messages", provider: "anthropic", model: "mock", @@ -818,10 +824,10 @@ describe("AgentSession TTSR resume gate", () => { ); } stream.push({ type: "start", partial }); - stream.push({ type: "toolcall_start", contentIndex: 0, partial }); + stream.push({ type: "toolcall_start", contentIndex: 1, partial }); stream.push({ type: "toolcall_delta", - contentIndex: 0, + contentIndex: 1, delta: 'let val = result.unwrap("oops")', partial, }); @@ -843,22 +849,22 @@ describe("AgentSession TTSR resume gate", () => { await session.prompt("Write some Rust code"); - const toolResult = sessionManager + const toolResults = sessionManager .getEntries() - .find( - entry => - entry.type === "message" && - entry.message.role === "toolResult" && - entry.message.toolCallId === toolCallContent.id, - ); - expect(toolResult?.type).toBe("message"); - const text = - toolResult?.type === "message" && toolResult.message.role === "toolResult" - ? (toolResult.message.content.find((part): part is { type: "text"; text: string } => part.type === "text") - ?.text ?? "") - : ""; - expect(text).toContain("Tool execution was aborted: TTSR matched rule: no-unwrap"); - expect(text).not.toContain("Request was aborted"); + .filter(entry => entry.type === "message" && entry.message.role === "toolResult") + .map(entry => (entry.type === "message" && entry.message.role === "toolResult" ? entry.message : undefined)) + .filter(message => message !== undefined); + const toolResultText = (toolCallId: string): string => + toolResults + .find(message => message.toolCallId === toolCallId) + ?.content.find((part): part is { type: "text"; text: string } => part.type === "text")?.text ?? ""; + + const readText = toolResultText(readToolCallContent.id); + const matchedText = toolResultText(matchedToolCallContent.id); + expect(readText).toContain("Tool execution was aborted: TTSR interrupt on another tool call"); + expect(readText).not.toContain("TTSR matched rule: no-unwrap"); + expect(matchedText).toContain("Tool execution was aborted: TTSR matched rule: no-unwrap"); + expect(matchedText).not.toContain("Request was aborted"); }); it("relativizes the rule file path in the TTSR interrupt injection (no absolute leak)", async () => {