diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 9e22dabb5..9495ccf72 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed the `task` tool mangling subagent prompts when a model double-JSON-encodes a string argument: `context` and each task's `assignment`/`description` are now repaired when they arrive uniformly double-escaped (literal `\n`, `\"`, `\uXXXX`), so the subagent receives the intended prose and the call preview renders real newlines. The repair is guarded by a JSON-string round-trip and a double-encode signature, so legitimate backslashes/quotes (Windows paths, regexes, embedded quotes) are left untouched, and it is scoped to these natural-language fields only (never code-bearing tools). + ## [15.7.4] - 2026-05-31 ### Removed diff --git a/packages/coding-agent/src/task/index.ts b/packages/coding-agent/src/task/index.ts index d831525bf..eacdf01b7 100644 --- a/packages/coding-agent/src/task/index.ts +++ b/packages/coding-agent/src/task/index.ts @@ -48,6 +48,7 @@ import { runSubprocess } from "./executor"; import { AgentOutputManager } from "./output-manager"; import { mapWithConcurrencyLimit, Semaphore } from "./parallel"; import { renderResult, renderCall as renderTaskCall } from "./render"; +import { repairTaskParams } from "./repair-args"; import { getTaskSimpleModeCapabilities, type TaskSimpleMode } from "./simple-mode"; import { applyNestedPatches, @@ -247,7 +248,7 @@ export class TaskTool implements AgentTool[1], theme: Theme) { - return renderTaskCall(args as TaskParams, options, theme); + return renderTaskCall(repairTaskParams(args as TaskParams), options, theme); } /** Dynamic description that reflects current disabled-agent settings */ @@ -292,7 +293,7 @@ export class TaskTool implements AgentTool, ): Promise> { - const params = rawParams as TaskParams; + const params = repairTaskParams(rawParams as TaskParams); const simpleMode = this.#getTaskSimpleMode(); const validationError = validateTaskModeParams(simpleMode, params); if (validationError) { diff --git a/packages/coding-agent/src/task/repair-args.ts b/packages/coding-agent/src/task/repair-args.ts new file mode 100644 index 000000000..62cc8f7a7 --- /dev/null +++ b/packages/coding-agent/src/task/repair-args.ts @@ -0,0 +1,117 @@ +/** + * Repair double-encoded JSON string arguments for the task tool. + * + * Models occasionally JSON-escape a string value twice when emitting a + * `task` tool call, so a `context`/`assignment` that should read + * + * # Role + * You are a judge … "describe this" … return — + * + * arrives — after the one JSON decode the provider already applied — as the + * literal text + * + * # Role\nYou are a judge … \"describe this\" … return \u2014 + * + * i.e. every newline, quote, and unicode character is still backslash-escaped. + * The subagent then receives that garbled prompt, and the call preview renders + * one long blob with visible `\n` / `\"` / `\uXXXX`. + * + * The *whole-arguments* form of this quirk (the entire `arguments` blob is a + * JSON string) is already auto-corrected by the validator's JSON-string + * coercion. This module handles the *per-field* form, where the object parses + * fine but an individual string value is double-encoded — the validator never + * fires there because a double-encoded string is still a structurally valid + * string. + * + * This is deliberately scoped to the task tool's natural-language fields + * (`context`, `assignment`, `description`). It is NOT applied to code-bearing + * tools (write/edit/bash/search), where a backslash or quote is load-bearing + * and a false-positive unescape would silently corrupt a file or command. + */ +import type { TaskItem, TaskParams } from "./types"; + +/** A backslash that escapes a structural char — `\"`, `\\`, `\/`, or `\uXXXX`. */ +const STRUCTURAL_ESCAPE = /\\(?:["\\/]|u[0-9a-fA-F]{4})/; + +/** + * Whether `value` carries the signature of whole-string double-encoding rather + * than an incidental escape mention. A lone `\n`/`\t` in an instruction (e.g. + * "split lines on \n") is far more likely a literal mention than a + * double-encoded document, so it is left alone; a structural escape (`\"`, + * `\\`, `\uXXXX`) or two-plus escape sequences indicates a re-escaped payload. + */ +function hasDoubleEncodeSignature(value: string): boolean { + if (STRUCTURAL_ESCAPE.test(value)) return true; + let count = 0; + for (let i = 0; i < value.length; i++) { + if (value.charCodeAt(i) === 0x5c /* \ */) { + count += 1; + if (count >= 2) return true; + i += 1; // skip the escaped char so `\\` counts once + } + } + return false; +} + +/** + * Return the once-unescaped string when `value` is uniformly double-encoded + * JSON (a well-formed JSON string body that decodes to a different string); + * otherwise return `value` unchanged. + * + * The `JSON.parse(\`"${value}"\`)` round-trip is the safety net: it only + * succeeds when *every* backslash begins a valid JSON escape and no bare + * double-quote exists — exactly the signature of double-encoding. Genuine + * prose with a Windows path (`C:\Users`), a regex (`\d+`), an embedded quote, + * or a real (already-decoded) newline makes the parse throw, so the value is + * returned untouched. + */ +export function repairDoubleEncodedJsonString(value: string): string { + // Fast path: no backslash → nothing was escaped → the parse can never differ. + if (!value.includes("\\")) return value; + if (!hasDoubleEncodeSignature(value)) return value; + let decoded: unknown; + try { + decoded = JSON.parse(`"${value}"`); + } catch { + return value; + } + return typeof decoded === "string" && decoded !== value ? decoded : value; +} + +/** Repair a single (possibly partial) task item's prose fields. */ +function repairTaskItem(task: TaskItem): TaskItem { + if (task === null || typeof task !== "object") return task; + const assignment = + typeof task.assignment === "string" ? repairDoubleEncodedJsonString(task.assignment) : task.assignment; + const description = + typeof task.description === "string" ? repairDoubleEncodedJsonString(task.description) : task.description; + if (assignment === task.assignment && description === task.description) return task; + return { ...task, assignment, description }; +} + +/** + * Repair double-encoded prose in task-tool params (`context` and each task's + * `assignment`/`description`). Returns the same reference when nothing changed + * so callers can cheaply skip work. Defensive against partially-streamed args + * (missing/undefined fields, partial task arrays) so it is safe on the render + * path as well as on execution. + */ +export function repairTaskParams(params: TaskParams): TaskParams { + if (params === null || typeof params !== "object") return params; + + const context = typeof params.context === "string" ? repairDoubleEncodedJsonString(params.context) : params.context; + + let tasks = params.tasks; + if (Array.isArray(params.tasks)) { + let changed = false; + const repaired = params.tasks.map(task => { + const next = repairTaskItem(task); + if (next !== task) changed = true; + return next; + }); + if (changed) tasks = repaired; + } + + if (context === params.context && tasks === params.tasks) return params; + return { ...params, context, tasks }; +} diff --git a/packages/coding-agent/test/tools/task-repair-args.test.ts b/packages/coding-agent/test/tools/task-repair-args.test.ts new file mode 100644 index 000000000..7979c367c --- /dev/null +++ b/packages/coding-agent/test/tools/task-repair-args.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, it } from "bun:test"; +import { repairDoubleEncodedJsonString, repairTaskParams } from "../../src/task/repair-args"; +import type { TaskParams } from "../../src/task/types"; + +describe("repairDoubleEncodedJsonString", () => { + it("decodes a uniformly double-encoded prose value", () => { + // One JSON decode already applied by the provider; the value still + // carries literal `\n`, `\"`, and `\u2014` because the model escaped twice. + const doubled = '# Role\\nYou are a judge \\"describe this\\" return \\u2014'; + expect(repairDoubleEncodedJsonString(doubled)).toBe('# Role\nYou are a judge "describe this" return —'); + }); + + it("decodes a double-encoded multi-line plain-text value", () => { + expect(repairDoubleEncodedJsonString("line one\\nline two\\nline three")).toBe("line one\nline two\nline three"); + }); + + it("preserves a Windows path (bare backslashes are not valid escapes)", () => { + expect(repairDoubleEncodedJsonString("C:\\Users\\me")).toBe("C:\\Users\\me"); + }); + + it("preserves a regex with a backslash class", () => { + expect(repairDoubleEncodedJsonString("match \\d+ digits")).toBe("match \\d+ digits"); + }); + + it("preserves text containing a bare double quote", () => { + expect(repairDoubleEncodedJsonString('she said "hi" loudly')).toBe('she said "hi" loudly'); + }); + + it("leaves a lone literal \\n mention alone (no double-encode signature)", () => { + expect(repairDoubleEncodedJsonString("split lines on \\n then count")).toBe("split lines on \\n then count"); + }); + + it("is a no-op for plain text without escapes", () => { + const plain = "just some normal instructions"; + expect(repairDoubleEncodedJsonString(plain)).toBe(plain); + }); + + it("leaves a partially-decoded value (real newline mixed with literal escape) untouched", () => { + // A real newline cannot appear inside a JSON string literal unescaped, so + // the round-trip parse throws and the value is preserved as-is. + const mixed = "real\nnewline with \\t tab"; + expect(repairDoubleEncodedJsonString(mixed)).toBe(mixed); + }); +}); + +describe("repairTaskParams", () => { + it("repairs context and each task's assignment/description, leaving ids intact", () => { + const params = { + agent: "task", + context: "# Goal\\nDo the thing \\u2014 carefully", + tasks: [ + { + id: "FirstTask", + description: 'judge \\"sketch\\" accuracy', + assignment: "Score 0-100.\\nUse the full range.\\nNo bunching.", + }, + ], + } as unknown as TaskParams; + + const repaired = repairTaskParams(params); + expect(repaired.context).toBe("# Goal\nDo the thing — carefully"); + expect(repaired.tasks[0].id).toBe("FirstTask"); + expect(repaired.tasks[0].description).toBe('judge "sketch" accuracy'); + expect(repaired.tasks[0].assignment).toBe("Score 0-100.\nUse the full range.\nNo bunching."); + }); + + it("returns the same reference when nothing needs repair", () => { + const params = { + agent: "task", + context: "plain context", + tasks: [{ id: "A", description: "label", assignment: "do work" }], + } as unknown as TaskParams; + expect(repairTaskParams(params)).toBe(params); + }); + + it("tolerates partially-streamed args without throwing", () => { + const partial = { agent: "task", tasks: [{ id: "A" }, undefined] } as unknown as TaskParams; + expect(() => repairTaskParams(partial)).not.toThrow(); + }); +});