fix(coding-agent): repaired per-field double-encoded JSON in task tool
- Added `repairDoubleEncodedJsonString` to unescape fields double-encoded by the model (e.g. literal `\n`, `\"`, `\uXXXX` in `context`/`assignment`/`description`). - Scoped repair to natural-language fields only, leaving code-bearing tools untouched. - Applied repair on both render and execution paths in `TaskTool`.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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<TaskToolSchemaInstance, TaskToolDetai
|
||||
}
|
||||
|
||||
renderCall(args: unknown, options: Parameters<typeof renderTaskCall>[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<TaskToolSchemaInstance, TaskToolDetai
|
||||
signal?: AbortSignal,
|
||||
onUpdate?: AgentToolUpdateCallback<TaskToolDetails>,
|
||||
): Promise<AgentToolResult<TaskToolDetails>> {
|
||||
const params = rawParams as TaskParams;
|
||||
const params = repairTaskParams(rawParams as TaskParams);
|
||||
const simpleMode = this.#getTaskSimpleMode();
|
||||
const validationError = validateTaskModeParams(simpleMode, params);
|
||||
if (validationError) {
|
||||
|
||||
@@ -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 };
|
||||
}
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user