From 650fc184ca9457b7ddddd408d6fd96efef55fb67 Mon Sep 17 00:00:00 2001 From: Dayoooun Date: Mon, 17 Aug 2026 21:57:29 +0900 Subject: [PATCH] fix(compaction): stop null persisted tool arguments from killing the turn Sessions written by an earlier cold-spill eviction path persist `toolCall.arguments` as `null` where the spill sentinel belongs. The compaction pruning pass dereferenced those arguments unguarded, so reloading such a session threw TypeError: null is not an object (evaluating 'args.path') which surfaced as a turn-killing provider error rather than a skipped call. 43 sessions in a live store reproduce it. `ToolCall.arguments` is typed non-nullable, so the type system never flagged the gap. Route every read of a persisted argument bag through a `toolArguments()` guard that treats a non-object payload as absent: path/file_path/filePath extraction, apply_patch header parsing, idempotent-bash key building, and search target keys. Also guard the `/copy` last-bash-command lookup, which had the same shape. Data is not lost: the eviction marker still names the blob, and rehydration restores the original arguments (verified across the same 43 sessions, 5,175 cold-spilled argument payloads restored, zero still null). --- packages/agent/CHANGELOG.md | 1 + packages/agent/src/compaction/pruning.ts | 38 ++++-- .../agent/test/pruning-null-arguments.test.ts | 120 ++++++++++++++++++ packages/coding-agent/CHANGELOG.md | 1 + .../modes/controllers/command-controller.ts | 2 +- 5 files changed, 152 insertions(+), 10 deletions(-) create mode 100644 packages/agent/test/pruning-null-arguments.test.ts diff --git a/packages/agent/CHANGELOG.md b/packages/agent/CHANGELOG.md index c54d70f695..6c2aa0c6ec 100644 --- a/packages/agent/CHANGELOG.md +++ b/packages/agent/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## [Unreleased] +- Compaction pruning no longer kills the turn when a persisted `toolCall.arguments` is `null`. Sessions written by an earlier cold-spill eviction path store `null` where the spill sentinel belongs, and the staleness index dereferenced that payload unguarded, so reloading such a session threw `null is not an object (evaluating 'args.path')` as a turn-fatal error instead of skipping the one unusable call. `ToolCall.arguments` is typed non-nullable, so no type check flagged the gap. Every read of a persisted argument bag — path extraction, `apply_patch` header parsing, idempotent-bash keys, and search target keys — now treats a non-object payload as absent. The original arguments are not lost: the eviction marker still names the blob and rehydration restores them. ## [0.14.0] - 2026-08-17 diff --git a/packages/agent/src/compaction/pruning.ts b/packages/agent/src/compaction/pruning.ts index 78c92df048..646cc15501 100644 --- a/packages/agent/src/compaction/pruning.ts +++ b/packages/agent/src/compaction/pruning.ts @@ -233,9 +233,25 @@ interface PrunedToolArgumentsSentinel { const EDIT_TOOL_NAMES = new Set(["edit", "write", "apply_patch", "ast_edit"]); +/** + * A tool call's arguments, or `undefined` when the persisted payload is not an + * object. + * + * `ToolCall.arguments` is typed non-nullable, but sessions written by an older + * cold-spill eviction path carry `arguments: null` where the spill sentinel + * should be. Reading `.path` off that null threw a TypeError that surfaced as + * `null is not an object (evaluating 'args.path')` and killed the turn, so + * every reader of persisted arguments must treat them as untrusted. + */ +function toolArguments(call: ToolCall): Record | undefined { + const args = call.arguments; + return typeof args === "object" && args !== null ? args : undefined; +} + /** Extract the file-path argument from a tool call, when the tool has one. */ function toolCallPath(call: ToolCall): string | undefined { - const args = call.arguments; + const args = toolArguments(call); + if (!args) return undefined; const path = args.path ?? args.file_path ?? args.filePath; return typeof path === "string" && path.length > 0 ? path : undefined; } @@ -260,7 +276,7 @@ const APPLY_PATCH_HEADER = /^\*\*\* (?:((?:Add|Update|Delete) File)|(Move to)): function editToolPathGroups(call: ToolCall): string[][] { const path = toolCallPath(call); if (path !== undefined) return [[path]]; - const input = call.arguments.input; + const input = toolArguments(call)?.input; if (typeof input !== "string") return []; const groups: string[][] = []; for (const match of input.matchAll(APPLY_PATCH_HEADER)) { @@ -422,11 +438,13 @@ const IDEMPOTENT_BASH_COMMAND = function normalizedIdempotentBashCommand(call: ToolCall): string | undefined { if (call.name !== "bash") return undefined; - const command = call.arguments.command; + const args = toolArguments(call); + if (!args) return undefined; + const command = args.command; if (typeof command !== "string") return undefined; const normalized = command.trim().replace(/\s+/g, " "); if (/[;&|]/.test(normalized) || !IDEMPOTENT_BASH_COMMAND.test(normalized)) return undefined; - return JSON.stringify([normalized, typeof call.arguments.cwd === "string" ? call.arguments.cwd : undefined]); + return JSON.stringify([normalized, typeof args.cwd === "string" ? args.cwd : undefined]); } function toolTargetKey(call: ToolCall): string | undefined { @@ -434,13 +452,15 @@ function toolTargetKey(call: ToolCall): string | undefined { if (path !== undefined) return JSON.stringify([call.name, "path", path]); const command = normalizedIdempotentBashCommand(call); if (command !== undefined) return JSON.stringify([call.name, "command", command]); - const pattern = call.arguments.pattern; + const args = toolArguments(call); + if (!args) return undefined; + const pattern = args.pattern; if (typeof pattern === "string" && pattern.length > 0) { - const paths = call.arguments.paths; + const paths = args.paths; const pathList = Array.isArray(paths) ? paths.filter((p): p is string => typeof p === "string") : []; - const skip = typeof call.arguments.skip === "number" ? call.arguments.skip : 0; - const caseInsensitive = call.arguments.i === true; - const gitignore = call.arguments.gitignore !== false; + const skip = typeof args.skip === "number" ? args.skip : 0; + const caseInsensitive = args.i === true; + const gitignore = args.gitignore !== false; return JSON.stringify([call.name, "pattern", pattern, pathList, skip, caseInsensitive, gitignore]); } return undefined; diff --git a/packages/agent/test/pruning-null-arguments.test.ts b/packages/agent/test/pruning-null-arguments.test.ts new file mode 100644 index 0000000000..11d068ba07 --- /dev/null +++ b/packages/agent/test/pruning-null-arguments.test.ts @@ -0,0 +1,120 @@ +import { describe, expect, it } from "bun:test"; +import type { ToolResultMessage } from "@gajae-code/ai"; +import type { SessionEntry, SessionMessageEntry } from "../src/compaction/entries"; +import { DEFAULT_PRUNE_CONFIG, pruneAssistantToolArguments } from "../src/compaction/pruning"; +import { applyToolOutputPrune as pruneToolOutputs } from "./pruning-test-utils"; + +/** + * A persisted assistant `toolCall` block can carry `arguments: null`. Live + * sessions on disk contain thousands of them, every one paired with a + * `message.content.N.arguments` cold-spill payload ref: the eviction pass moved + * the real arguments to a blob and the sentinel that should have replaced them + * did not survive persistence. Reloading such a session must not crash the + * pruning pass — `args.path` on a null `arguments` throws a TypeError that + * surfaces to the user as `null is not an object (evaluating 'args.path')` and + * kills the turn. + */ + +let idCounter = 0; + +function assistantCallEntry(callId: string, toolName: string, args: unknown): SessionEntry { + idCounter++; + return { + type: "message", + id: `a-${idCounter}`, + parentId: null, + timestamp: new Date(idCounter).toISOString(), + message: { + role: "assistant", + content: [{ type: "toolCall", id: callId, name: toolName, arguments: args }], + api: "anthropic-messages", + provider: "anthropic", + model: "m", + stopReason: "toolUse", + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + timestamp: idCounter, + }, + } as SessionEntry; +} + +function toolResultEntry(callId: string, toolName: string, sizeChars = 8000, isError = false): SessionMessageEntry { + idCounter++; + return { + type: "message", + id: `r-${idCounter}`, + parentId: null, + timestamp: new Date(idCounter).toISOString(), + message: { + role: "toolResult", + toolCallId: callId, + toolName, + content: [{ type: "text", text: `result-${callId} ${"x ".repeat(Math.floor(sizeChars / 2))}` }], + isError, + timestamp: idCounter, + } as ToolResultMessage, + } as SessionMessageEntry; +} + +function pair( + entries: SessionEntry[], + callId: string, + toolName: string, + args: unknown, + sizeChars = 8000, +): SessionMessageEntry { + entries.push(assistantCallEntry(callId, toolName, args)); + const result = toolResultEntry(callId, toolName, sizeChars); + entries.push(result); + return result; +} + +describe("pruning tolerates persisted null tool arguments", () => { + it("does not throw when an edit-class call carries null arguments", () => { + const entries: SessionEntry[] = []; + pair(entries, "c1", "write", null); + expect(() => pruneAssistantToolArguments(entries, { ...DEFAULT_PRUNE_CONFIG, protectTokens: 0 })).not.toThrow(); + }); + + it("does not throw when tool-output pruning indexes a null-argument call", () => { + const entries: SessionEntry[] = []; + pair(entries, "c1", "write", null); + pair(entries, "c2", "bash", null); + pair(entries, "c3", "search", null); + expect(() => + pruneToolOutputs(entries, { + protectTokens: 0, + minimumSavings: 0, + protectedTools: ["skill", "read"], + staleOverridableTools: ["read"], + }), + ).not.toThrow(); + }); + + it("still prunes a real stale edit when a null-argument call sits in the same history", () => { + const entries: SessionEntry[] = []; + pair(entries, "c0", "write", null); + const staleEdit = assistantCallEntry("c1", "edit", { + path: "src/a.ts", + input: "x".repeat(2_000), + }); + entries.push(staleEdit, toolResultEntry("c1", "edit", 100)); + pair(entries, "c2", "write", { path: "src/a.ts", content: "new" }, 100); + + const result = pruneAssistantToolArguments(entries, { + protectTokens: 0, + minimumSavings: 0, + protectedTools: ["skill", "read"], + staleOverridableTools: ["read"], + }); + + expect(result.argumentPrunedCount).toBe(1); + expect(result.prunedEntries.map(entry => entry.id)).toEqual([staleEdit.id]); + }); +}); diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 1304bbe295..92535d2748 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -13,6 +13,7 @@ - Every session start under a non-writable cwd (e.g. a Windows console defaulting to `C:\Windows\System32`) no longer dies with an uncaught `EPERM` before any output: `FileGateStore.beginRuntimeInstance` no longer flushes at construction when the store holds no gates and no counters (the runtime instance id rides along with the first real mutation, preserving the documented lazy first-write contract), and `flushState` now runs `mkdirSync` inside the typed write boundary so a genuinely unwritable directory surfaces as `GateStoreWriteError` instead of a raw `ErrnoException` escaping the store abstraction (#4568). - Fixed ACP `session/new` failing with `lost exact Router authority` for every symlinked workspace cwd. The broker's session index stores the lifecycle caller's lexical cwd in `locator.repo` (`reconcileReadyScope` re-scopes only that field) while `locator.stateRoot` stays the host process's physical path, because the host derives it from `process.cwd()`, which resolves symlinks; `SessionRouter#readEndpoint` derived the expected state root from `repo` and compared spellings with a plain `path.resolve` equality, so a symlinked cwd (`/home/jun/desk -> /data/Lina-Desk`, macOS `/var -> /private/var`) made the scope test fail on every reconcile, the adopted attachment was retired, and the ACP agent tore down a healthy host after its publication poll expired. The scope test now compares path identity through `resolveEquivalentPath` — the same symlink-equivalent comparison `sameResumeLocator` and the index fence-row predicate already use — so equivalent spellings resolve to `default`/`chat` and the attachment publishes. - A cold-spilled tool call whose payload can no longer be recovered no longer breaks the whole session. `rehydrateColdSpillRef` reports an unreadable, hash-mismatched, or unparseable blob by returning the human-readable `[Cold-spill blob unavailable: …]` sentence, and that string landed directly on `toolCall.arguments`. Providers forward it verbatim — Anthropic serializes it into `tool_use.input` and rejects the entire request with `tool_use.input: Input should be a valid dictionary` — so one missing blob made a compacted session permanently unresumable, with an error naming a message index rather than the lost payload. Rehydration now enforces the object invariant at the session boundary and degrades just that call to the existing `incompleteArguments` / `"malformed"` contract, preserving the recovered text under `recoveryNotice`, so the agent loop refuses that one call with retryable guidance while the rest of the transcript still loads. +- `/copy` no longer throws when the most recent assistant turn carries a persisted `toolCall.arguments` of `null` (written by an earlier cold-spill eviction path). The last-bash-command lookup read `arguments.command` unguarded, the same shape that made compaction pruning fatal. ## [0.14.0] - 2026-08-17 - Documented how to run GJC inside external agent shells. `README.md` gains a support-rated integration table for [Paseo](https://paseo.sh) (★★★★★ — `gjc setup paseo` writes a conformance-tested ACP provider), [Orca](https://onorca.dev) (★★★★ — GJC runs as a custom CLI agent per worktree), and [T3 Code](https://t3.codes) (★★★ experimental — no GJC harness exists upstream yet), and [`docs/terminal-app-integrations.md`](../../docs/terminal-app-integrations.md) carries the per-host setup, verification, cancel-semantics, and troubleshooting detail. The T3 Code row is deliberately marked unsupported rather than advertising an install command for a bridge that does not exist. diff --git a/packages/coding-agent/src/modes/controllers/command-controller.ts b/packages/coding-agent/src/modes/controllers/command-controller.ts index 9aea32a4ec..4fe3032206 100644 --- a/packages/coding-agent/src/modes/controllers/command-controller.ts +++ b/packages/coding-agent/src/modes/controllers/command-controller.ts @@ -398,7 +398,7 @@ export class CommandController { const toolCalls = msg.content.filter((c): c is ToolCall => c.type === "toolCall"); for (let j = toolCalls.length - 1; j >= 0; j--) { const tc = toolCalls[j]; - if (tc.name === "bash" && typeof tc.arguments.command === "string") { + if (tc.name === "bash" && typeof tc.arguments?.command === "string") { this.#doCopy(tc.arguments.command, "Copied last bash command to clipboard"); return; }