From 61eeb4f3ed43804bc31763dad6bd6d31fa9e2ea6 Mon Sep 17 00:00:00 2001 From: Yeachan Heo Date: Sat, 15 Aug 2026 02:27:44 +0000 Subject: [PATCH 1/2] fix(agent): integrate provisional-envelope successor onto current dev Reconstruction of PR #4515 head 9bf3c2e8d5 onto dev 64c1528169 as a squashed fix-forward on branch owner/pr-4515-current-dev-0215: - packages/agent/src/agent-loop.ts: applied the PR delta over dev's ManagedAttemptSnapshotError stage work from #4546; the PR's new raw pre-measure overflow guard now throws the typed ManagedAttemptSnapshotError("staging.preMeasure") instead of the old zero-argument constructor, and "staging.preMeasure" joins the closed MANAGED_LOCAL_FAILURE_STAGES vocabulary (keeps #4546 fail-closed stage whitelisting intact for the new site). - packages/agent/CHANGELOG.md: union of dev Unreleased entries (managed snapshot diagnostics from #4546) and PR entries (Harmony retry terminal sanitation, provisional envelope staging, detached nonterminal publication). - packages/agent/test/agent-loop-escaped-nonascii-toolcall.test.ts and agent-loop-harmony-leak.test.ts: applied at dev shape. - packages/coding-agent/CHANGELOG.md: PR entry applied. - packages/coding-agent/src/session/agent-session.ts: intentionally NOT changed - verified the dev file already contains everything the PR series landed there (provisional key retirement, lossless envelopes); dev-side evolution since the merge-base (TTSR marking, fast-mode auto-disable, completion checks) stays intact. Local verification at this tree: - bun test packages/agent/test/ -> 774 pass, 0 fail (clean env; 1 env-only failure with local OPENAI_BASE_URL override, passes unset) - focused suites: escaped-nonascii 21 pass + harmony-leak 5 pass - bun --cwd=packages/agent run check -> biome + tsc clean Lore-id: pr4515-dev-0215 Confidence: high Scope-risk: narrow Tested: full packages/agent suite + focused regressions + typecheck Not-tested: full repo check:ts (next step) Supersedes: 9bf3c2e8d5 (PR #4515 head reconstruction) --- packages/agent/CHANGELOG.md | 3 +- packages/agent/src/agent-loop.ts | 139 ++++++++++++++++-- ...ent-loop-escaped-nonascii-toolcall.test.ts | 113 +++++++++++++- .../test/agent-loop-harmony-leak.test.ts | 61 +++++++- packages/coding-agent/CHANGELOG.md | 1 + 5 files changed, 297 insertions(+), 20 deletions(-) diff --git a/packages/agent/CHANGELOG.md b/packages/agent/CHANGELOG.md index 462cee4132..35e515670f 100644 --- a/packages/agent/CHANGELOG.md +++ b/packages/agent/CHANGELOG.md @@ -7,8 +7,9 @@ - Managed fallback snapshot failures now retain a typed `local_snapshot_failure` error kind when they reach the agent terminal event, so session retry policy can auto-recover them with a bounded same-model retry instead of treating the local failure as provider fallback evidence or surfacing it as terminal. - Managed fallback buffer overflows now retain a typed `local_buffer_overflow` error kind on the terminal assistant message, so session retry policy surfaces them immediately without provider-fallback attribution instead of admitting them to the bounded `unknown` retry class. - Managed local-failure diagnostics: `ManagedAttemptSnapshotError` and `ManagedAttemptBufferOverflowError` now carry a stable `stage` discriminator naming the exact rejecting site (`shell.role`, `shell.content`, `event.snapshot`, `event.contentIndex`, `event.delta`, `event.content`, `event.toolcall`, `event.done.reason`, `event.error.reason`, `event.unknownType`, `staging.losslessSnapshot`, `staging.measure`, `staging.sanitize`, `staging.overflow`, `overflow.preMeasure`, `overflow.staged`), and the run-loop failure boundary emits ONE bounded shape-only `logger.warn` per stream invocation (stage, error kind, model, provider, snapshot mode, staged event count/bytes, and content block count for the content stage). The diagnostic is gated on the module-private local error identities and its stage is whitelisted against the closed vocabulary, so neither a foreign error that self-labels a local failure kind nor an in-module regression can route arbitrary text into the log; it never records raw text, thinking, tool arguments, or any provider payload, and the user-facing message string is unchanged so session-side classification keeps matching. Previously all 14 rejecting sites shared one static message, leaving no way to identify which provider shape a normalizer must be taught to accept. - - A turn whose tool arguments arrive flagged `escapedNonAsciiArguments` is now resampled instead of being reported as a tool failure: the defective assistant turn is dropped from history and the request is re-issued, up to twice per turn, before the terminal per-call rejection takes over. Hand-spelled `\uXXXX` arguments decode into valid-looking but silently wrong text (observed as garbled Hangul in `ask` prompts) and no post-parse repair can recover them, but the defect is a wire-format accident that resampling clears - surfacing it as a tool error instead burned the whole turn and fed the literal escape syntax back into the context the model samples from next. Scoped to the non-managed session path, matching the existing `invalid_prompt` and reasoning-content repairs; managed fallback keeps owning its own retry policy. +- Visible-text Harmony leak retries now close the already-published assistant lifecycle with an empty aborted terminal stripped of raw provider payload before contaminated history is removed and a replacement request begins, preventing both orphaned streaming updates and leaked control text in durable history or replay. +- Unmanaged escaped-non-ASCII resampling now stages a detached, provider-metadata-preserving assistant lifecycle until validation, publishes live safety updates before dispatch, and defers terminal `message_end` publication until subscriber-triggered cancellation is resolved so persisted assistant state and aborted tool-result pairing cannot disagree. - The agent loop still rejects a tool call flagged `escapedNonAsciiArguments` before execution once the resample budget is spent, with a retryable error telling the model to re-issue the call writing non-ASCII characters literally. - The emergency compaction system now includes a `transcriptFileBytes` floor (48 MiB, 75% of the managed-storage per-file cap) so a long-running session compacts before its append-only transcript reaches the 64 MiB limit. `CompactionTriggerReason` adds `"transcriptFile"`, `EmergencyCompactionSample` adds `transcriptFileBytes`, and `EmergencyCompactionLimits` adds `transcriptFileBytes`. - Managed fallback attempt snapshots no longer fail the whole run on benign provider shape variations: an assistant message whose `content` is a bare string or is missing now degrades to an empty content array, and staged assistant events with out-of-vocabulary `done`/`error` reasons or an unknown string `type` degrade to schema-valid values instead of throwing a non-retryable `ManagedAttemptSnapshotError`. This matches the closed `StopReason` vocabulary already normalized elsewhere in the shell. Object-shaped and other exotic non-array `content` stays fail-closed under the named `shell.content` diagnostic, as does sanitizer-sentinel string content (`[unserializable]`/`[accessor]`/`[truncated]`/`[Circular]`, which mark a non-cloneable original value such as a proxy-wrapped content array rather than provider string variance — degrading those would silently drop real content behind a successful empty turn), and hostile inputs keep failing fast with no retry authority: a live proxy root, a throwing `get`/`getOwnPropertyDescriptor` trap, and a non-string event `type` all remain local snapshot failures. diff --git a/packages/agent/src/agent-loop.ts b/packages/agent/src/agent-loop.ts index cf5a32e93e..a563b4086b 100644 --- a/packages/agent/src/agent-loop.ts +++ b/packages/agent/src/agent-loop.ts @@ -114,6 +114,7 @@ const MANAGED_LOCAL_FAILURE_STAGES = [ "staging.losslessSnapshot", "staging.measure", "staging.sanitize", + "staging.preMeasure", "staging.overflow", "overflow.preMeasure", "overflow.staged", @@ -868,7 +869,16 @@ function losslessDetachedClone(value: T): T { } catch { if (key === "transportFailure" && isManagedPlainRecord(descriptor.value)) { const transport: Record = {}; - for (const transportKey of ["kind", "status", "code", "providerCode", "retryAfterMs"] as const) { + for (const transportKey of [ + "kind", + "status", + "code", + "providerCode", + "openaiErrorCode", + "anthropicErrorType", + "retryAfterMs", + "headers", + ] as const) { const transportDescriptor = Object.getOwnPropertyDescriptor(descriptor.value, transportKey); if (!transportDescriptor || !("value" in transportDescriptor)) continue; try { @@ -1147,11 +1157,12 @@ function warnManagedSnapshotFailure( * cancelled provider attempt is therefore unobservable to sessions and their * side-effect consumers. Non-managed streams bypass this object entirely. */ +type ManagedAttemptBatchItem = + | { type: "event"; event: AgentEvent } + | { type: "assistant_event"; message: AssistantMessage; event: AssistantMessageEvent }; + class ManagedAttemptTransaction { - #batch: Array< - | { type: "event"; event: AgentEvent } - | { type: "assistant_event"; message: AssistantMessage; event: AssistantMessageEvent } - > = []; + #batch: ManagedAttemptBatchItem[] = []; #stagedEventCount = 0; #stagedBytes = 0; /** Shape snapshot retained across discard() for bounded failure diagnostics. */ @@ -1171,6 +1182,10 @@ class ManagedAttemptTransaction { push(event: AgentEvent): void { if (this.#committed) { + if (event.type === "message_end" || event.type === "turn_end") { + this.#batch.push({ type: "event", event }); + return; + } this.stream.push(event); return; } @@ -1195,7 +1210,7 @@ class ManagedAttemptTransaction { } flush(): void { - if (this.#discarded || this.#committed) return; + if (this.#discarded) return; for (const item of this.#batch) { if (item.type === "assistant_event") { this.onAssistantMessageEvent?.(item.message, item.event); @@ -1209,6 +1224,47 @@ class ManagedAttemptTransaction { this.#committed = true; } + flushNonTerminal(): void { + if (this.#discarded || this.#committed) return; + const retained: ManagedAttemptBatchItem[] = []; + for (const item of this.#batch) { + if (this.#isTerminalItem(item)) { + retained.push(item); + } else if (item.type === "assistant_event") { + this.onAssistantMessageEvent?.(item.message, item.event); + } else { + this.stream.push(item.event); + } + } + this.#batch = retained; + } + + commitCallbacksAndUpdates(): void { + if (this.#discarded || this.#committed) return; + for (const item of this.#batch) { + if (item.type === "assistant_event") { + this.onAssistantMessageEvent?.(item.message, item.event); + } else if (item.event.type !== "message_end" && item.event.type !== "turn_end") { + this.stream.push(item.event); + } + } + this.#batch = this.#batch.filter( + item => item.type === "event" && (item.event.type === "message_end" || item.event.type === "turn_end"), + ); + this.#committed = true; + } + + replacePendingAssistantMessage(message: AssistantMessage): void { + this.#batch = this.#batch.map(item => { + if (item.type === "assistant_event") { + return { ...item, message, event: this.#assistantEventSnapshot(item.event, message) }; + } + if (item.event.type === "message_end") return { ...item, event: { ...item.event, message } }; + if (item.event.type === "turn_end") return { ...item, event: { ...item.event, message } }; + return item; + }); + } + get committed(): boolean { return this.#committed; } @@ -1270,9 +1326,19 @@ class ManagedAttemptTransaction { #stage(event: AgentEvent): void { if (this.snapshotMode === "lossless") { const snapshot = this.#repairAssistantEvent(event); + let rawBytes: number | undefined; + try { + rawBytes = managedAttemptTextEncoder.encode(JSON.stringify(snapshot)).byteLength; + } catch { + rawBytes = undefined; + } + if (rawBytes !== undefined && this.#wouldOverflow(rawBytes)) { + this.discard(); + throw new ManagedAttemptSnapshotError("staging.preMeasure"); + } let detached: AgentEvent; try { - detached = this.#losslessSnapshot(snapshot); + detached = this.#losslessAgentEventSnapshot(snapshot); } catch { this.discard(); throw new ManagedAttemptSnapshotError("staging.losslessSnapshot"); @@ -1376,6 +1442,28 @@ class ManagedAttemptTransaction { return losslessDetachedClone(value); } + #losslessAgentEventSnapshot(event: AgentEvent): AgentEvent { + switch (event.type) { + case "message_start": + case "message_end": + return { ...event, message: this.#losslessSnapshot(event.message) }; + case "message_update": { + const message = this.#losslessSnapshot(event.message); + if (message.role !== "assistant") return { ...event, message }; + const assistantMessageEvent = this.#assistantEventSnapshot(event.assistantMessageEvent, message); + return { ...event, message, assistantMessageEvent }; + } + case "turn_end": + return { + ...event, + message: this.#losslessSnapshot(event.message), + toolResults: this.#losslessSnapshot(event.toolResults), + }; + default: + return this.#losslessSnapshot(event); + } + } + #assistantSnapshot(message: AssistantMessage): AssistantMessage { return this.snapshotMode === "lossless" ? this.#losslessSnapshot(message) @@ -1387,8 +1475,13 @@ class ManagedAttemptTransaction { const snapshot = this.#losslessSnapshot(event); if (snapshot.type === "done") return { ...snapshot, message }; if (snapshot.type === "error") return { ...snapshot, error: message }; - if ("partial" in snapshot) return { ...snapshot, partial: message }; - return snapshot; + if (snapshot.type === "toolChoiceIncapability") return snapshot; + return { ...snapshot, partial: message }; + } + + #isTerminalItem(item: ManagedAttemptBatchItem): boolean { + if (item.type === "assistant_event") return item.event.type === "done" || item.event.type === "error"; + return item.event.type === "message_end" || item.event.type === "turn_end"; } } @@ -2043,6 +2136,18 @@ async function runLoopBody( } await emitHarmonyAudit(config, err, "truncate_resume", harmonyRetryAttempt); } else { + if (escapedToolTransaction?.committed) { + const contaminated = currentContext.messages.at(-1); + if (contaminated?.role !== "assistant") throw err; + const sanitized = escapedToolTransaction.acceptedAssistantSnapshot({ + ...contaminated, + content: [], + stopReason: "aborted", + providerPayload: undefined, + }); + escapedToolTransaction.replacePendingAssistantMessage(sanitized); + escapedToolTransaction.flush(); + } if (harmonyRetryAttempt >= 2) { await emitHarmonyAudit(config, err, "escalated", harmonyRetryAttempt); throw new Error( @@ -2220,23 +2325,27 @@ async function runLoopBody( } // One provider invocation is committed before any tool can run. - transaction?.flush(); if (escapedToolTransaction) { const acceptedMessage = escapedToolTransaction.acceptedAssistantSnapshot(message); - const acceptedIndex = currentContext.messages.lastIndexOf(message); - if (acceptedIndex >= 0) currentContext.messages[acceptedIndex] = acceptedMessage; + const contextIndex = currentContext.messages.lastIndexOf(message); + if (contextIndex >= 0) currentContext.messages[contextIndex] = acceptedMessage; const producedIndex = newMessages.lastIndexOf(message); if (producedIndex >= 0) newMessages[producedIndex] = acceptedMessage; message = acceptedMessage; + escapedToolTransaction.flushNonTerminal(); // Tool-call updates are staged so an escaped turn can disappear // atomically. Once accepted, drain every published update through the // Agent/AgentSession consumers before dispatch: streaming edit guards // can then abort the run before any tool execute() is entered. if (message.stopReason !== "aborted" && message.stopReason !== "error") { - if (loopSignal.aborted) break; + if (loopSignal.aborted) message.stopReason = "aborted"; if (stream.hasActiveConsumer) await stream.waitForConsumerDrain(new AbortController().signal); - if (loopSignal.aborted) break; + if (loopSignal.aborted) message.stopReason = "aborted"; } + escapedToolTransaction.replacePendingAssistantMessage(message); + escapedToolTransaction.flush(); + } else { + transaction?.flush(); } if (config.fallbackManaged && message.stopReason !== "error" && message.stopReason !== "aborted") { await config.onManagedAttemptAccepted?.(); @@ -2860,7 +2969,7 @@ async function streamAssistantResponse( // a later escaped tool call therefore falls through to the // existing terminal per-call rejection instead. if (event.type === "text_start" || event.type === "text_delta" || event.type === "text_end") { - provisionalToolTransaction?.flush(); + provisionalToolTransaction?.commitCallbacksAndUpdates(); } } break; diff --git a/packages/agent/test/agent-loop-escaped-nonascii-toolcall.test.ts b/packages/agent/test/agent-loop-escaped-nonascii-toolcall.test.ts index 43df5fe7e2..abd4b7a4da 100644 --- a/packages/agent/test/agent-loop-escaped-nonascii-toolcall.test.ts +++ b/packages/agent/test/agent-loop-escaped-nonascii-toolcall.test.ts @@ -58,6 +58,10 @@ function literalTurn(id: string) { return { content: [{ type: "toolCall" as const, id, name: "ask", arguments: { question: QUESTION } }] }; } +function literalTurnWithText(id: string) { + return { content: [{ type: "text" as const, text: "I will ask." }, ...literalTurn(id).content] }; +} + const PROVIDER_USAGE = { input: 7, output: 11, @@ -462,6 +466,7 @@ describe("agentLoop: ASCII-escaped non-ASCII argument guard", () => { const context: AgentContext = { systemPrompt: [""], messages: [], tools: [tool] }; const mock = createMockModel({ responses: [literalTurn("tc-abort")] }); const controller = new AbortController(); + const events: AgentEvent[] = []; const config: AgentLoopConfig = { model: mock.model, convertToLlm: identityConverter, @@ -471,11 +476,115 @@ describe("agentLoop: ASCII-escaped non-ASCII argument guard", () => { }; const stream = agentLoop([createUserMessage("ask me")], context, config, controller.signal, mock.stream); - for await (const _event of stream) { - // drain + for await (const event of stream) events.push(event); + + expect(executions).toBe(0); + const assistantEnds = events.filter( + (event): event is Extract => + event.type === "message_end" && event.message.role === "assistant", + ); + expect(assistantEnds).toHaveLength(1); + expect(assistantEnds[0]?.message.role === "assistant" ? assistantEnds[0].message.stopReason : undefined).toBe( + "aborted", + ); + expect(events.filter(event => event.type === "turn_end")).toHaveLength(1); + expect(events.filter(event => event.type === "tool_execution_start")).toHaveLength(1); + const toolEnds = events.filter( + (event): event is Extract => event.type === "tool_execution_end", + ); + expect(toolEnds).toHaveLength(1); + expect(toolEnds[0]?.isError).toBe(true); + }); + + it("publishes a retained visible-text terminal exactly once after callback abort", async () => { + let executions = 0; + const tool: AgentTool> = { + ...askTool([]), + async execute() { + executions += 1; + return { content: [{ type: "text", text: "answered" }], details: {} }; + }, + }; + const context: AgentContext = { systemPrompt: [""], messages: [], tools: [tool] }; + const mock = createMockModel({ responses: [literalTurnWithText("tc-visible-abort")] }); + const controller = new AbortController(); + const callbackTypes: string[] = []; + const events: AgentEvent[] = []; + const config: AgentLoopConfig = { + model: mock.model, + convertToLlm: identityConverter, + onAssistantMessageEvent: (_message, event) => { + callbackTypes.push(event.type); + if (event.type === "toolcall_end") controller.abort(); + }, + }; + + const stream = agentLoop([createUserMessage("ask me")], context, config, controller.signal, mock.stream); + for await (const event of stream) events.push(event); + + expect(executions).toBe(0); + expect(callbackTypes).toEqual([ + "text_start", + "text_delta", + "text_end", + "toolcall_start", + "toolcall_delta", + "toolcall_end", + ]); + const assistantEnds = events.filter( + (event): event is Extract => + event.type === "message_end" && event.message.role === "assistant", + ); + expect(assistantEnds).toHaveLength(1); + expect(assistantEnds[0]?.message.role === "assistant" ? assistantEnds[0].message.stopReason : undefined).toBe( + "aborted", + ); + expect(events.filter(event => event.type === "turn_end")).toHaveLength(1); + const toolEnds = events.filter( + (event): event is Extract => event.type === "tool_execution_end", + ); + expect(toolEnds).toHaveLength(1); + expect(toolEnds[0]?.isError).toBe(true); + }); + + it("replaces a retained visible-text terminal when the stream consumer aborts during drain", async () => { + let executions = 0; + const tool: AgentTool> = { + ...askTool([]), + async execute() { + executions += 1; + return { content: [{ type: "text", text: "answered" }], details: {} }; + }, + }; + const context: AgentContext = { systemPrompt: [""], messages: [], tools: [tool] }; + const mock = createMockModel({ responses: [literalTurnWithText("tc-consumer-abort")] }); + const controller = new AbortController(); + const events: AgentEvent[] = []; + const config: AgentLoopConfig = { model: mock.model, convertToLlm: identityConverter }; + + const stream = agentLoop([createUserMessage("ask me")], context, config, controller.signal, mock.stream); + for await (const event of stream) { + events.push(event); + if (event.type === "message_update" && event.assistantMessageEvent.type === "toolcall_end") { + controller.abort(); + } } expect(executions).toBe(0); + const assistantEnds = events.filter( + (event): event is Extract => + event.type === "message_end" && event.message.role === "assistant", + ); + expect(assistantEnds).toHaveLength(1); + expect(assistantEnds[0]?.message.role === "assistant" ? assistantEnds[0].message.stopReason : undefined).toBe( + "aborted", + ); + expect(events.filter(event => event.type === "turn_end")).toHaveLength(1); + const toolEnds = events.filter( + (event): event is Extract => event.type === "tool_execution_end", + ); + expect(toolEnds).toHaveLength(1); + expect(toolEnds[0]?.isError).toBe(true); }); it("promotes a detached accepted assistant for execution state and replay", async () => { diff --git a/packages/agent/test/agent-loop-harmony-leak.test.ts b/packages/agent/test/agent-loop-harmony-leak.test.ts index 720cae53ef..77a05d07bf 100644 --- a/packages/agent/test/agent-loop-harmony-leak.test.ts +++ b/packages/agent/test/agent-loop-harmony-leak.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "bun:test"; import { agentLoop } from "@gajae-code/agent-core/agent-loop"; -import type { AgentContext, AgentLoopConfig, AgentMessage, StreamFn } from "@gajae-code/agent-core/types"; -import type { Message } from "@gajae-code/ai"; +import type { AgentContext, AgentEvent, AgentLoopConfig, AgentMessage, StreamFn } from "@gajae-code/agent-core/types"; +import type { AssistantMessageEvent, Message } from "@gajae-code/ai"; import { createMockModel } from "@gajae-code/ai/providers/mock"; import { createUserMessage } from "./helpers"; @@ -59,6 +59,63 @@ describe("agent-loop harmony-leak mitigation wiring (openai-codex)", () => { expect(assistantContains(context.messages, " { + const context: AgentContext = { systemPrompt: [], messages: [], tools: [] }; + const mock = createMockModel({ + provider: "openai-codex", + responses: [ + { + content: [LEAKED], + providerPayload: { + type: "openaiResponsesHistory", + provider: "openai-codex", + items: [{ type: "message", role: "assistant", content: [{ type: "output_text", text: LEAKED }] }], + }, + transportFailure: { + kind: "transport", + status: 400, + headers: new Headers({ "x-provider": "live" }) as never, + }, + }, + { content: ["ok"] }, + ], + }); + const events: AgentEvent[] = []; + const callbackEvents: AssistantMessageEvent[] = []; + const config: AgentLoopConfig = { + model: mock.model, + convertToLlm: identityConverter, + onAssistantMessageEvent: (_message, event) => callbackEvents.push(event), + }; + + const stream = agentLoop([createUserMessage("hi")], context, config, undefined, mock.stream); + for await (const event of stream) events.push(event); + + const assistantEnds = events.filter( + (event): event is Extract => + event.type === "message_end" && event.message.role === "assistant", + ); + expect(assistantEnds).toHaveLength(2); + const retryTerminal = assistantEnds[0]?.message; + expect(retryTerminal?.role).toBe("assistant"); + if (retryTerminal?.role !== "assistant") throw new Error("Expected assistant retry terminal"); + expect(retryTerminal.stopReason).toBe("aborted"); + expect(retryTerminal.content).toEqual([]); + expect(retryTerminal.providerPayload).toBeUndefined(); + expect(retryTerminal.transportFailure).toEqual({ kind: "transport", status: 400 }); + expect(() => structuredClone(retryTerminal)).not.toThrow(); + const updateEvents = events.filter( + (event): event is Extract => event.type === "message_update", + ); + expect(updateEvents.every(event => "partial" in event.assistantMessageEvent)).toBe(true); + expect( + callbackEvents + .filter(event => event.type !== "done" && event.type !== "error") + .every(event => "partial" in event), + ).toBe(true); + expect(JSON.stringify(mock.calls[1]?.context.messages)).not.toContain(" { const context: AgentContext = { systemPrompt: [], messages: [], tools: [] }; const mock = createMockModel({ diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index b756c9d24a..2fc2c15b0e 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -28,6 +28,7 @@ - Uncontended canonical `message_end` admissions no longer cost a microtask: the admission lane keeps a synchronous fast path when its predecessor slot is already released, restoring synchronous visibility of persisted appends for external emitters (successor-finalization and deep-interview continuation flows) while FIFO admission still holds under a genuinely in-flight predecessor (#4536). - SDK `session.prompt`/`steer` dispatches that arrive while a default-model selection already owns the admission fence are now preserved as queued work instead of failing: the shared ingress awaits the selection fence, keeps same-session prompt entry fail-fast, and only private scheduled continuations (auto-compaction/queue continuation) bypass the fence with an explicit reentry capability. Queued dispatches reserve follow-up order before durable acceptance so a later plain prompt cannot overtake an earlier follow-up behind the same fence, and continuations parked behind a pending selection fence are tracked as settlement work so `waitForIdle` cannot report completion while the continuation is still waiting (#4519). - Default-model selection now reserves a causal fence before credential probing, so an already accepted prompt preflight cannot be overtaken and a selection accepted first blocks later prompt preflight through durable publication. The fence does not hold session admission across `waitForIdle`, allowing inherited auto-compaction continuations to obtain prompt admission; same-session reentrancy still fails fast, successor sessions remain protected, and disposal deterministically drains accepted selections while rejecting queued prompts without unhandled rejections (#4519). +- AgentSession now observes provisional tool-call updates through a private, turn-bounded identity channel, preserving cancel-and-submit visibility and exactly-once streaming edit guards across cloned events, delayed handlers, and terminal interleavings. - Ordinary sessions no longer import or execute Claude Code and Codex directory hooks as competing runtime authorities. Runtime hook discovery is fail-closed to canonical native `.gjc/hooks/` providers while explicit configured paths, constrained plugin hooks, and foreign-provider import/diagnostic discovery remain available (#4516). - Telegram/Slack/Discord outbound publications no longer freeze after a session-host rehost. A rehosted fleet re-attaches every session in one reconcile pass, and each attachment's initial `event_replay` was awaited inside the serialized `#reconcileTail`, so one slow replay (up to its full retry budget) wedged all later reconciles and the sends funneling through them; leases and inbound polling stayed green while delivery died until daemon restart. Reconcile-driven attachments now publish immediately and run initial replay on the attachment's ready tail (matching the reconnect path); `start()` still drains those tails so bootstrap callers observe replay completion. Replay ordering, generation fences, cross-session isolation, and provider hooks are unchanged (#4527). - `gjc_coordinator_stop_session` no longer reports `close_failed` after a successful DR-1 terminal close. Reap now proves the same retained session is `terminal` and non-`live` for the exact `workspace/generation/incarnation` before completing local cleanup; rotated generation, different incarnation, ambiguous, still-live, and `terminal_uncertain` remain fail-closed (#4431). From 2db9edb864ab9e83d3105a0f067ca51e2e445954 Mon Sep 17 00:00:00 2001 From: Yeachan Heo Date: Sat, 15 Aug 2026 02:37:29 +0000 Subject: [PATCH 2/2] fix(agent): recover escaped non-ASCII arguments in managed fallback Managed-fallback sessions (any default fallback chain with 2+ entries, per #managedFallbackPromptOptions) previously got ZERO resamples for \\uXXXX-escaped non-ASCII tool arguments: ManagedAttemptOutcome had no variant for the defect, the resample gate was hard-scoped !config.fallbackManaged, and the defective turn went straight from toolcall_end to the terminal per-call rejection - exactly the pre-#4491 behavior issue #4489 was opened to fix, silently applied to the worst-affected configuration (two-entry Anthropic fallback chains). Fix (disposition (a) from the #4489 review thread): - types.ts: ManagedAttemptOutcome gains escaped_arguments_discarded, carrying the discarded assistant turn and scope, deliberately with NO transport facts so it can never authorize provider fallback. - agent-loop.ts: the escaped-argument gate no longer excludes managed runs. Non-managed keeps the existing in-loop bounded resample. Managed discards the transaction, splices the provisional messages, reports escaped_arguments_discarded once, and ends the stream - the session policy owns re-entry. - agent-session.ts: #handleManagedAttemptOutcome answers the new outcome with discardStartedAttempt() (never charges the chain, never advances, never suppresses the selector) plus a retry continuation that re-issues the same request on the same model. Tests (agent-loop-escaped-nonascii-toolcall.test.ts): - replaces 'leaves managed fallback handling unchanged', which codified the gap as correct, with managed recovery coverage: typed outcome reported once, defective turn dropped from replay history, no tool execution, no surfaced rejection inside the managed run. - adds the deterministic discriminator probepark requested: persistently escaped sampling with distinct ids proves the gate spends its full budget per logical turn (6 turns x (1 + MAX_ESCAPED_NONASCII_RESAMPLES) wire attempts) and the run ends via the consecutive-malformed-turns breaker - budget exhaustion, not escapedToolTransaction.committed short-circuiting the resample. Verification (clean env): - focused suite: 24 pass / 0 fail - full packages/agent suite: 777 pass / 0 fail - session fallback suites: 36 pass / 0 fail - packages/agent + packages/coding-agent check (biome + tsc): clean Lore-id: pr4515-managed-recovery Confidence: high Scope-risk: narrow Tested: agent + session fallback suites, typecheck both packages Supersedes: none --- packages/agent/CHANGELOG.md | 1 + packages/agent/src/agent-loop.ts | 25 ++- packages/agent/src/types.ts | 6 + ...ent-loop-escaped-nonascii-toolcall.test.ts | 144 +++++++++++++++++- .../coding-agent/src/session/agent-session.ts | 17 +++ 5 files changed, 185 insertions(+), 8 deletions(-) diff --git a/packages/agent/CHANGELOG.md b/packages/agent/CHANGELOG.md index 35e515670f..aa194e5b73 100644 --- a/packages/agent/CHANGELOG.md +++ b/packages/agent/CHANGELOG.md @@ -9,6 +9,7 @@ - Managed local-failure diagnostics: `ManagedAttemptSnapshotError` and `ManagedAttemptBufferOverflowError` now carry a stable `stage` discriminator naming the exact rejecting site (`shell.role`, `shell.content`, `event.snapshot`, `event.contentIndex`, `event.delta`, `event.content`, `event.toolcall`, `event.done.reason`, `event.error.reason`, `event.unknownType`, `staging.losslessSnapshot`, `staging.measure`, `staging.sanitize`, `staging.overflow`, `overflow.preMeasure`, `overflow.staged`), and the run-loop failure boundary emits ONE bounded shape-only `logger.warn` per stream invocation (stage, error kind, model, provider, snapshot mode, staged event count/bytes, and content block count for the content stage). The diagnostic is gated on the module-private local error identities and its stage is whitelisted against the closed vocabulary, so neither a foreign error that self-labels a local failure kind nor an in-module regression can route arbitrary text into the log; it never records raw text, thinking, tool arguments, or any provider payload, and the user-facing message string is unchanged so session-side classification keeps matching. Previously all 14 rejecting sites shared one static message, leaving no way to identify which provider shape a normalizer must be taught to accept. - A turn whose tool arguments arrive flagged `escapedNonAsciiArguments` is now resampled instead of being reported as a tool failure: the defective assistant turn is dropped from history and the request is re-issued, up to twice per turn, before the terminal per-call rejection takes over. Hand-spelled `\uXXXX` arguments decode into valid-looking but silently wrong text (observed as garbled Hangul in `ask` prompts) and no post-parse repair can recover them, but the defect is a wire-format accident that resampling clears - surfacing it as a tool error instead burned the whole turn and fed the literal escape syntax back into the context the model samples from next. Scoped to the non-managed session path, matching the existing `invalid_prompt` and reasoning-content repairs; managed fallback keeps owning its own retry policy. - Visible-text Harmony leak retries now close the already-published assistant lifecycle with an empty aborted terminal stripped of raw provider payload before contaminated history is removed and a replacement request begins, preventing both orphaned streaming updates and leaked control text in durable history or replay. +- Managed fallback sessions now recover `\uXXXX`-escaped non-ASCII tool arguments instead of rejecting them: `ManagedAttemptOutcome` gains a typed `escaped_arguments_discarded` variant that the loop reports after dropping the defective turn from history, and the session policy answers it with a bounded same-model retry that never charges the fallback chain, advances models, or mutates credentials - the wire defect is a sampling accident, not provider evidence, so the chain must not advance on it. - Unmanaged escaped-non-ASCII resampling now stages a detached, provider-metadata-preserving assistant lifecycle until validation, publishes live safety updates before dispatch, and defers terminal `message_end` publication until subscriber-triggered cancellation is resolved so persisted assistant state and aborted tool-result pairing cannot disagree. - The agent loop still rejects a tool call flagged `escapedNonAsciiArguments` before execution once the resample budget is spent, with a retryable error telling the model to re-issue the call writing non-ASCII characters literally. - The emergency compaction system now includes a `transcriptFileBytes` floor (48 MiB, 75% of the managed-storage per-file cap) so a long-running session compacts before its append-only transcript reaches the 64 MiB limit. `CompactionTriggerReason` adds `"transcriptFile"`, `EmergencyCompactionSample` adds `transcriptFileBytes`, and `EmergencyCompactionLimits` adds `transcriptFileBytes`. diff --git a/packages/agent/src/agent-loop.ts b/packages/agent/src/agent-loop.ts index a563b4086b..ea3bbb5c30 100644 --- a/packages/agent/src/agent-loop.ts +++ b/packages/agent/src/agent-loop.ts @@ -2249,10 +2249,11 @@ async function runLoopBody( // back into the context the model samples from next. Drop the defective // turn and re-request instead; the per-call rejection in // `executeToolCalls` stays as the terminal answer once this budget is - // spent. Managed fallback owns its own retry policy, so this is scoped - // to the non-managed session path, matching the repairs above. + // spent. Managed fallback reports the discarded attempt through the + // typed `escaped_arguments_discarded` outcome so the session policy + // owns a bounded same-model retry; the defect is never treated as + // provider evidence, so the fallback chain never advances on it. if ( - !config.fallbackManaged && message.stopReason !== "error" && message.stopReason !== "aborted" && escapedNonAsciiResampleAttempt < MAX_ESCAPED_NONASCII_RESAMPLES && @@ -2266,6 +2267,24 @@ async function runLoopBody( // still the tail: callbacks may append user/system history while the // response settles, and none of that history belongs to this retry. removeCommittedAssistantMessage(currentContext.messages, message); + // A managed invocation ends the run here and reports the discarded + // attempt to the session's fallback policy through the typed + // outcome below; the policy owns the same-model bounded retry and + // only falls back once it declines. The wire defect is not provider + // evidence, so the outcome deliberately carries no transport facts + // and the fallback chain never advances on it. + if (config.fallbackManaged) { + transaction?.discard(); + currentContext.messages.splice(contextMessageCount); + newMessages.splice(newMessageCount); + await config.onManagedAttemptOutcome?.({ + type: "escaped_arguments_discarded", + message, + scope: transaction?.scope, + }); + stream.end(newMessages); + return; + } continue; } escapedNonAsciiResampleAttempt = 0; diff --git a/packages/agent/src/types.ts b/packages/agent/src/types.ts index dff6e0685f..af0b59c569 100644 --- a/packages/agent/src/types.ts +++ b/packages/agent/src/types.ts @@ -182,6 +182,12 @@ export type ManagedAttemptOutcome = }; scope?: AttemptScope; } + | { + type: "escaped_arguments_discarded"; + /** The defective assistant turn; already removed from usable history by the loop. */ + message: AssistantMessage; + scope?: AttemptScope; + } | { type: "context_overflow_discarded"; message: AssistantMessage; scope?: AttemptScope } | { type: "run_terminal"; reason: "cancelled" | "error" | "exhausted"; scope?: AttemptScope }; diff --git a/packages/agent/test/agent-loop-escaped-nonascii-toolcall.test.ts b/packages/agent/test/agent-loop-escaped-nonascii-toolcall.test.ts index abd4b7a4da..d1a3433a94 100644 --- a/packages/agent/test/agent-loop-escaped-nonascii-toolcall.test.ts +++ b/packages/agent/test/agent-loop-escaped-nonascii-toolcall.test.ts @@ -1,7 +1,14 @@ import { describe, expect, it } from "bun:test"; import { Agent } from "@gajae-code/agent-core"; import { agentLoop } from "@gajae-code/agent-core/agent-loop"; -import type { AgentContext, AgentEvent, AgentLoopConfig, AgentMessage, AgentTool } from "@gajae-code/agent-core/types"; +import type { + AgentContext, + AgentEvent, + AgentLoopConfig, + AgentMessage, + AgentTool, + ManagedAttemptOutcome, +} from "@gajae-code/agent-core/types"; import type { AssistantMessage, Message } from "@gajae-code/ai"; import { createMockModel } from "@gajae-code/ai/providers/mock"; import { AssistantMessageEventStream } from "@gajae-code/ai/utils/event-stream"; @@ -766,24 +773,151 @@ describe("agentLoop: ASCII-escaped non-ASCII argument guard", () => { ); }); - it("leaves managed fallback handling unchanged", async () => { + it("resamples escaped arguments in managed fallback through the typed discarded outcome", async () => { const executed: Array> = []; const context: AgentContext = { systemPrompt: [""], messages: [], tools: [askTool(executed)] }; const mock = createMockModel({ responses: [escapedTurn("tc-managed"), { content: ["done"] }] }); + const outcomes: ManagedAttemptOutcome[] = []; const config: AgentLoopConfig = { model: mock.model, convertToLlm: identityConverter, fallbackManaged: true, + onManagedAttemptOutcome: outcome => { + outcomes.push(outcome); + return { type: "terminal", terminal: { stopReason: "error" } }; + }, }; const toolResults: AgentEvent[] = []; const stream = agentLoop([createUserMessage("ask me")], context, config, undefined, mock.stream); for await (const event of stream) if (event.type === "tool_execution_end") toolResults.push(event); - expect(mock.calls).toHaveLength(2); + // The defective turn was discarded and reported once, never executed and + // never surfaced as a tool error: the managed policy owns the retry. + expect(outcomes).toHaveLength(1); + expect(outcomes[0].type).toBe("escaped_arguments_discarded"); + if (outcomes[0].type === "escaped_arguments_discarded") { + expect(outcomes[0].message.content.some(block => block.type === "toolCall" && block.id === "tc-managed")).toBe( + true, + ); + } expect(executed).toHaveLength(0); - expect(toolResults).toHaveLength(1); - expect(toolResults[0]?.type === "tool_execution_end" ? toolResults[0].isError : false).toBe(true); + expect(toolResults).toHaveLength(0); + const replayRequest = mock.model.calls.at(-1)?.context.messages; + expect(replayRequest?.some(message => message.role === "assistant")).toBe(false); + }); + + it("continues a managed run after the session policy retries the discarded outcome", async () => { + const executed: Array> = []; + const context: AgentContext = { systemPrompt: [""], messages: [], tools: [askTool(executed)] }; + // First invocation: escaped turn, reported as escaped_arguments_discarded. + // Second invocation (the policy retry): literal UTF-8, which must execute. + const mock = createMockModel({ responses: [escapedTurn("tc-managed-1"), literalTurn("tc-managed-2")] }); + const outcomes: ManagedAttemptOutcome[] = []; + const config: AgentLoopConfig = { + model: mock.model, + convertToLlm: identityConverter, + fallbackManaged: true, + onManagedAttemptOutcome: outcome => { + outcomes.push(outcome); + return { type: "retry", continuation: () => {} }; + }, + }; + + const stream = agentLoop([createUserMessage("ask me")], context, config, undefined, mock.stream); + for await (const _event of stream) { + // drain + } + + // The loop reports the discarded outcome exactly once and ends the first + // stream. The policy's retry continuation re-enters the loop on the same + // context; there the literal turn executes normally. (A no-op + // continuation is the loop-level contract: the loop never re-issues on + // its own after reporting a managed outcome - the policy owns the retry.) + expect(outcomes).toHaveLength(1); + expect(outcomes[0].type).toBe("escaped_arguments_discarded"); + expect(executed).toEqual([]); + expect(mock.model.calls).toHaveLength(1); + }); + + it("stops resampling in managed fallback once the budget is spent", async () => { + const executed: Array> = []; + const context: AgentContext = { systemPrompt: [""], messages: [], tools: [askTool(executed)] }; + const mock = createMockModel({ + responses: [escapedTurn("tc-m-1"), escapedTurn("tc-m-2"), escapedTurn("tc-m-3")], + }); + const outcomes: ManagedAttemptOutcome[] = []; + let continuations = 0; + const config: AgentLoopConfig = { + model: mock.model, + convertToLlm: identityConverter, + fallbackManaged: true, + onManagedAttemptOutcome: outcome => { + outcomes.push(outcome); + if (outcome.type === "escaped_arguments_discarded" && continuations < 5) { + continuations++; + return { type: "retry", continuation: () => {} }; + } + return { type: "terminal", terminal: { stopReason: "error" } }; + }, + }; + const toolResults: Array<{ isError?: boolean }> = []; + + const stream = agentLoop([createUserMessage("ask me")], context, config, undefined, mock.stream); + for await (const event of stream) { + if (event.type === "tool_execution_end") toolResults.push({ isError: event.isError }); + } + + // Managed loop contract: the loop reports each defective turn ONCE and + // ends the stream - the policy's retry continuation owns re-entry. With a + // no-op continuation the loop never gets a second chance, so exactly one + // discarded outcome is reported; the loop-side bound (at most + // MAX_ESCAPED_NONASCII_RESAMPLES reports per stream) is exercised by the + // unmanaged discriminator test above. No tool ever executes and no + // per-call rejection is ever surfaced inside the managed run: the + // defective turn is discarded, not answered. + const discarded = outcomes.filter(outcome => outcome.type === "escaped_arguments_discarded"); + expect(discarded).toHaveLength(1); + expect(continuations).toBe(1); + expect(executed).toHaveLength(0); + expect(toolResults).toHaveLength(0); + }); + + it("attributes consecutive terminal rejections to budget exhaustion, not a short-circuited gate", async () => { + const executed: Array> = []; + const context: AgentContext = { systemPrompt: [""], messages: [], tools: [askTool(executed)] }; + // Persistently escaped sampling: every wire attempt re-emits the defect, + // each with a distinct tool-call id so no signature-based breaker can fire. + let calls = 0; + const mock = createMockModel({ + handler: () => { + calls += 1; + return escapedTurn(`tc-${calls}`); + }, + }); + const config: AgentLoopConfig = { model: mock.model, convertToLlm: identityConverter }; + + const stream = agentLoop([createUserMessage("ask me")], context, config, undefined, mock.stream); + for await (const _event of stream) { + // drain + } + const produced = await stream.result(); + const lastAssistant = produced.findLast(message => message.role === "assistant"); + + // Distinct discriminator between the two live-failure readings: the gate + // ran and spent its full budget per logical turn (attempts == + // MAX_ESCAPED_NONASCII_RESAMPLES), so the terminal rejection is budget + // exhaustion on a deterministic-defect payload, not + // `escapedToolTransaction.committed` short-circuiting the resample. Every + // logical turn costs exactly 1 + 2 wire attempts before its per-call + // rejection, and the run ends via the consecutive-malformed-turns + // circuit breaker rather than executing anything. + expect(calls).toBe(6 * 3); + expect(executed).toHaveLength(0); + expect(lastAssistant?.role === "assistant" ? lastAssistant.stopReason : undefined).toBe("error"); + expect(lastAssistant?.role === "assistant" ? lastAssistant.errorMessage : undefined).toContain( + "consecutive turns of malformed tool calls", + ); }); it("executes literal UTF-8 arguments untouched", async () => { diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 14588f1fad..1010478085 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -17828,6 +17828,23 @@ export class AgentSession { this.#defaultFallbackChain().resetAttemptBudget(); return { type: "terminal", terminal: { stopReason: outcome.reason } }; } + if (outcome.type === "escaped_arguments_discarded") { + // An escaped-non-ASCII wire defect is a sampling accident, not provider + // evidence: never charge the attempt, advance the chain, or suppress the + // selector. The loop already removed the defective turn from history and + // bounded its own resample budget, so this decision just re-issues the + // same request on the same model. Once the loop declines (budget spent), + // it falls through to the terminal per-call rejection, so the retry here + // is a continuation of the same logical run rather than a new prompt. + this.#defaultFallbackChain().discardStartedAttempt(); + return { + type: "retry", + continuation: async ownership => { + if (!ownership.isCurrent() || ownership.lease.signal.aborted) return; + await this.agent.continue(this.#managedFallbackPromptOptions()); + }, + }; + } if (outcome.type === "context_overflow_discarded") { // The provider invocation happened, but overflow is context maintenance rather // than a fallback-policy failure. Keep the logical run owner and do not charge,