From cc6204526293420df6e2da83720b41c5740d4ca2 Mon Sep 17 00:00:00 2001 From: Daniil Pokrovsky Date: Thu, 27 Aug 2026 20:45:38 +0700 Subject: [PATCH] feat: implement DEV-100 - Story creation fails with 'Failed to parse story from agent output' when the agent's response isn't clean JSON --- docs/pm/configuration.md | 6 + packages/pm/agent-json.test.ts | 96 +++++++ packages/pm/chat-core.test.ts | 8 +- packages/pm/engine.test.ts | 100 ++++++++ packages/pm/index.ts | 4 +- packages/pm/lib/agent-json.ts | 422 ++++++++++++++++++++++++++++--- packages/pm/lib/chat/messages.ts | 10 +- packages/pm/lib/engine/index.ts | 98 +++++-- packages/pm/lib/engine/json.ts | 8 +- 9 files changed, 686 insertions(+), 66 deletions(-) diff --git a/docs/pm/configuration.md b/docs/pm/configuration.md index dd79c3b..28c1999 100644 --- a/docs/pm/configuration.md +++ b/docs/pm/configuration.md @@ -267,3 +267,9 @@ Update-check state (last check time, seen version) is cached per package in `~/. - Check `AGENT_HARNESS` spelling (use kebab-case, e.g. `claude-code`, `grok`, `deepseek`) - The error lists every valid harness name; pick one from that list - Ensure the matching CLI is installed and on your `PATH`, or set `AGENT_CLI_PATH` / `_CLI_PATH` + +**"Failed to parse story from agent output" / "malformed output"** + +- devpm automatically repairs common agent-output drift (markdown fences, narration, comments, trailing commas, unquoted keys, stray quotes) and re-runs the agent once with a strict-JSON reminder when repair isn't possible +- If parsing still fails, the full agent output is saved to `/tmp/devpm--.log` — check the log to see what came back, then retry +- Persistent failures usually mean the harness/model emits non-canonical JSON around long descriptions; switching harness (or pinning a different model) resolves it diff --git a/packages/pm/agent-json.test.ts b/packages/pm/agent-json.test.ts index be77f47..1c96ade 100644 --- a/packages/pm/agent-json.test.ts +++ b/packages/pm/agent-json.test.ts @@ -36,3 +36,99 @@ describe("parseAgentJson", () => { expect(() => parseAgentJson("I could not produce the story.")).toThrow(); }); }); + +describe("parseAgentJson object-literal drift repairs", () => { + test("strips line comments inside the object", () => { + const raw = [ + "{", + " // story below", + ' "summary": "S",', + ' "description": "URLs like https://x.io/a//b survive comment stripping",', + "", + " ## note", + "}", + ].join("\n"); + const parsed = parseAgentJson>(raw); + expect(parsed.summary).toBe("S"); + expect(parsed.description).toBe("URLs like https://x.io/a//b survive comment stripping"); + }); + + test("strips block comments inside the object", () => { + const raw = '{ /* draft */ "summary": "S", "description": "D" }'; + expect(parseAgentJson>(raw).summary).toBe("S"); + }); + + test("removes trailing commas", () => { + const raw = '{"summary": "S", "description": "D",}'; + expect(parseAgentJson>(raw)).toEqual({ + summary: "S", + description: "D", + }); + }); + + test("quotes unquoted JavaScript-style keys", () => { + const raw = '{ summary: "S", description: "D, with punctuation: plenty" }'; + expect(parseAgentJson>(raw)).toEqual({ + summary: "S", + description: "D, with punctuation: plenty", + }); + }); + + test("quotes single-quoted keys", () => { + const raw = `{ 'summary': "S", 'description': "D" }`; + expect(parseAgentJson>(raw)).toEqual({ + summary: "S", + description: "D", + }); + }); + + test("normalizes smart-quoted keys", () => { + const raw = '{"summary": "S", \u201cdescription\u201d: "D"}'; + expect(parseAgentJson>(raw)).toEqual({ + summary: "S", + description: "D", + }); + }); + + test("salvages values whose inner prose quotes desync every heuristic", () => { + // `Pick "one", then "two"` defeats structural quote repair because commas + // follow the quoted words — the schema-key salvage rebuilds the payload. + const raw = [ + "{", + ' "summary": "Story generation fails",', + ' "description": "Pick "one", then "two", then done"', + "}", + ].join("\n"); + const parsed = parseAgentJson>(raw); + expect(parsed.summary).toBe("Story generation fails"); + expect(parsed.description).toContain("then done"); + }); + + test("salvages a richly formatted description mixing several failure shapes", () => { + // Reconstruction of the DEV-100 report class: comments, unquoted keys, + // literal newlines, and stray quotes around long markdown descriptions. + const raw = [ + "Parsing the requirements now...", + "{", + " // generated draft", + " summary: Story creation fails when output isn't clean JSON", + ' "description": ## Problem', + "", + 'Some users see "Failed to parse story from agent output".', + "- Retry the command", + "- Or switch harness/model", + "}", + ].join("\n"); + const parsed = parseAgentJson>(raw); + expect(parsed.summary).toBe("Story creation fails when output isn't clean JSON"); + expect(parsed.description).toContain('"Failed to parse story'); + expect(parsed.description).toContain("- Or switch harness/model"); + }); + + test("does not mistake prose for keys during salvage", () => { + // No known keys anywhere -> salvage yields nothing, parse still throws. + expect(() => + parseAgentJson("I could not produce anything useful. total: 42 failure."), + ).toThrow(); + }); +}); diff --git a/packages/pm/chat-core.test.ts b/packages/pm/chat-core.test.ts index 497bbf7..aecc1a6 100644 --- a/packages/pm/chat-core.test.ts +++ b/packages/pm/chat-core.test.ts @@ -201,8 +201,14 @@ describe("messages", () => { test("renderError maps engine codes to friendly text without leaking detail", () => { const msg = renderError(new EngineError("parse-failed", "bad json", "RAW AGENT DUMP")); - expect(msg.text).toContain("couldn't parse"); + expect(msg.text).toContain("malformed output"); + expect(msg.text).toContain("switch harness/model"); expect(msg.text).not.toContain("RAW AGENT DUMP"); + expect( + renderError( + new EngineError("parse-failed", "bad json", "d", "/tmp/devpm-story-generation-parse-1.log"), + ).text, + ).toContain("/tmp/devpm-story-generation-parse-1.log"); expect(renderError(new EngineError("agent-failed", "x")).text).toContain("devpm serve"); expect(renderError(new Error("plain")).text).toContain("plain"); }); diff --git a/packages/pm/engine.test.ts b/packages/pm/engine.test.ts index 9f16aec..09eb29d 100644 --- a/packages/pm/engine.test.ts +++ b/packages/pm/engine.test.ts @@ -130,6 +130,9 @@ describe("extractJsonPayload", () => { expect(caught).toBeInstanceOf(EngineError); expect((caught as EngineError).code).toBe("parse-failed"); expect((caught as EngineError).detail).toContain("could not produce JSON"); + // Friendly, actionable headline — the low-level parser message stays out. + expect((caught as EngineError).message).toContain("malformed output"); + expect((caught as EngineError).message).toContain("Retry"); }); test("throws parse-failed with invalidMessage when fields are missing", () => { @@ -265,6 +268,103 @@ describe("createEngine", () => { expect((caught as EngineError).detail).toBe("boom"); }); + test("generateStory retries once with a corrective reminder after malformed output", async () => { + const prompts: string[] = []; + let calls = 0; + const engine = await createEngine( + stubConfig(), + { promptsDir: PROMPTS_DIR }, + { + backend: stubBackend(), + runAgent: async (_harness, _path, prompt) => { + prompts.push(prompt); + calls += 1; + return { + stdout: + calls === 1 + ? "Utter word salad, no object here." + : '{"summary": "S", "description": "D"}', + stderr: "", + exitCode: 0, + maxTurnsReached: false, + }; + }, + }, + ); + + const draft = await engine.generateStory({ + source: { type: "prompt", content: "x" }, + promptStyle: "pm", + }); + expect(draft).toEqual({ summary: "S", description: "D" }); + expect(calls).toBe(2); + expect(prompts[1]).toContain("could not be parsed as JSON"); + expect(prompts[1]).toContain(prompts[0] ?? ""); + }); + + test("generateStory reports malformed output with a dump after exhausting the retry", async () => { + let calls = 0; + const engine = await createEngine( + stubConfig(), + { promptsDir: PROMPTS_DIR }, + { + backend: stubBackend(), + runAgent: async () => { + calls += 1; + return { + stdout: `Still unparsable attempt ${calls}.`, + stderr: "", + exitCode: 0, + maxTurnsReached: false, + }; + }, + }, + ); + + let caught: unknown; + try { + await engine.generateStory({ + source: { type: "prompt", content: "x" }, + promptStyle: "pm", + }); + } catch (error) { + caught = error; + } + expect(calls).toBe(2); + expect(caught).toBeInstanceOf(EngineError); + expect((caught as EngineError).code).toBe("parse-failed"); + expect((caught as EngineError).message).toContain("malformed output"); + expect((caught as EngineError).dumpFile).toContain("devpm-story-generation-parse-"); + expect((caught as EngineError).detail).toContain("Still unparsable attempt 2."); + }); + + test("generateStory does not retry when the agent itself fails", async () => { + let calls = 0; + const engine = await createEngine( + stubConfig(), + { promptsDir: PROMPTS_DIR }, + { + backend: stubBackend(), + runAgent: async () => { + calls += 1; + return { stdout: "", stderr: "boom\n", exitCode: 1, maxTurnsReached: false }; + }, + }, + ); + + let caught: unknown; + try { + await engine.generateStory({ + source: { type: "prompt", content: "x" }, + promptStyle: "pm", + }); + } catch (error) { + caught = error; + } + expect(calls).toBe(1); + expect((caught as EngineError).code).toBe("agent-failed"); + }); + test("generateStory streams agent chunks through events", async () => { const chunks: Array<[string, string]> = []; const engine = await createEngine( diff --git a/packages/pm/index.ts b/packages/pm/index.ts index 8f962ac..b34bb0e 100755 --- a/packages/pm/index.ts +++ b/packages/pm/index.ts @@ -558,7 +558,7 @@ async function runCreateFlow(params: CreateFlowParams): Promise { return true; // continue create-another loop } console.error("\n❌ Failed to parse story requirements from Agent output"); - console.error("Error:", error.message); + console.error(error.message); console.error("Output:", error.detail); if (error.dumpFile) { console.error(`Full agent output (incl. stderr): ${error.dumpFile}`); @@ -718,7 +718,7 @@ async function runCreateFlow(params: CreateFlowParams): Promise { } if (error instanceof EngineError && error.code === "parse-failed") { console.error("\n❌ Failed to parse subtasks from Agent output"); - console.error("Error:", error.message); + console.error(error.message); console.error("Output:", error.detail); if (error.dumpFile) { console.error(`Full agent output (incl. stderr): ${error.dumpFile}`); diff --git a/packages/pm/lib/agent-json.ts b/packages/pm/lib/agent-json.ts index fefd428..7b780c0 100644 --- a/packages/pm/lib/agent-json.ts +++ b/packages/pm/lib/agent-json.ts @@ -10,6 +10,9 @@ * - a stray extra `}` after the closing brace (grok headless) * - literal `\n` text between the final value and the closing brace (grok) * - unescaped straight double quotes inside string values (`a legacy "cwd" mode`) + * - JS-object-literal drift around long rich descriptions: `//` comments, + * trailing commas, unquoted or single/smart-quoted keys — reported to users + * as "Expected double-quoted property name in JSON" style parse errors * * Try structural candidates from most to least specific, and retry each after * progressive repair passes instead of assuming one shape. @@ -67,27 +70,18 @@ function escapeControlCharsInStrings(text: string): string { } /** - * Escape unescaped double quotes inside JSON string literals. - * - * Models sometimes quote terms mid-value (`a legacy "cwd" mode`) without - * escaping, which terminates the JSON string early and corrupts everything - * after it. A closing quote is only trusted when the next non-whitespace - * character is a plausible structural token (`,`, `}`, `]`, `:`, or end of - * text); otherwise the quote is escaped. Best-effort: prose like `"a", "b"` - * can still fool it, but wrong guesses just produce another failing candidate. + * Shared quote-escape walker parameterized by when a closing double quote is + * trusted. A closing quote terminates the string only when the lookahead rule + * approves the position after it; otherwise the quote is escaped. */ -function escapeUnescapedQuotesInStrings(text: string): string { +function escapeQuotesWithRule( + text: string, + isTrustedClose: (afterQuoteIndex: number) => boolean, +): string { let result = ""; let inString = false; let escaped = false; - const isStructural = (index: number): boolean => { - let cursor = index; - while (cursor < text.length && /\s/.test(text.charAt(cursor))) cursor += 1; - if (cursor >= text.length) return true; - return ",}]: ".includes(text.charAt(cursor)); - }; - for (let index = 0; index < text.length; index += 1) { const character = text[index]; @@ -107,7 +101,7 @@ function escapeUnescapedQuotesInStrings(text: string): string { escaped = true; result += character; } else if (character === '"') { - if (isStructural(index + 1)) { + if (isTrustedClose(index + 1)) { inString = false; result += character; } else { @@ -121,6 +115,262 @@ function escapeUnescapedQuotesInStrings(text: string): string { return result; } +/** First non-whitespace character at or after `cursor`, or end sentinel. */ +function peekNonWhitespace( + text: string, + cursor: number, +): { char: string | undefined; atEnd: boolean } { + let index = cursor; + while (index < text.length && /\s/.test(text.charAt(index))) index += 1; + return { char: text[index], atEnd: index >= text.length }; +} + +/** + * Escape unescaped double quotes inside JSON string literals. + * + * Models sometimes quote terms mid-value (`a legacy "cwd" mode`) without + * escaping, which terminates the JSON string early and corrupts everything + * after it. A closing quote is only trusted when the next non-whitespace + * character is a plausible structural token (`,`, `}`, `]`, `:`); otherwise + * the quote is escaped. Best-effort: prose like `"a", "b"` can still fool it, + * but wrong guesses just produce another failing candidate. + */ +function escapeUnescapedQuotesInStrings(text: string): string { + return escapeQuotesWithRule(text, (afterQuoteIndex) => { + const { char, atEnd } = peekNonWhitespace(text, afterQuoteIndex); + if (atEnd || char === undefined) return true; + return ",}]: ".includes(char); + }); +} + +/** + * Stricter companion to {@link escapeUnescapedQuotesInStrings}: trust a close + * only when a structural token immediately follows the quote. Markdown bullet + * lines ending in a quoted word (`... say "done"\nmore prose`) mis-trip the + * lenient whitespace-skipping variant and flip string state mid-value, which + * later surfaces as "Expected double-quoted property name" parse errors. + */ +function escapeUnescapedQuotesStrictly(text: string): string { + return escapeQuotesWithRule(text, (afterQuoteIndex) => { + const char = text[afterQuoteIndex]; + return afterQuoteIndex >= text.length || char === undefined || ",}]: ".includes(char); + }); +} + +/** + * Remove `//` line comments and `/* ... *\/` block comments outside string + * literals. JavaScript-literal leakage like `{ // draft below }` makes parsers + * demand a double-quoted property name and fail. Newlines are preserved so + * position diagnostics stay meaningful. + */ +function stripJsonComments(text: string): string { + let result = ""; + let inString = false; + let escaped = false; + + for (let index = 0; index < text.length; index += 1) { + const character = text[index]; + + if (!inString) { + if (character === '"') { + inString = true; + result += character; + continue; + } + if (character === "/" && text[index + 1] === "/") { + while (index < text.length && text[index] !== "\n") index += 1; + // Keep the newline itself so line/column context stays readable. + if (index < text.length) result += "\n"; + continue; + } + if (character === "/" && text[index + 1] === "*") { + const end = text.indexOf("*/", index + 2); + index = end === -1 ? text.length : end + 1; + continue; + } + result += character; + continue; + } + + result += character; + if (escaped) { + escaped = false; + } else if (character === "\\") { + escaped = true; + } else if (character === '"') { + inString = false; + } + } + + return result; +} + +/** + * Remove trailing commas before `}` or `]` (outside strings), another + * object-literal habit models carry over from JavaScript. + */ +function removeTrailingCommas(text: string): string { + let result = ""; + let inString = false; + let escaped = false; + + for (let index = 0; index < text.length; index += 1) { + const character = text[index]; + + if (!inString) { + if (character === '"') { + inString = true; + result += character; + continue; + } + if (character === ",") { + let cursor = index + 1; + while (cursor < text.length && /\s/.test(text.charAt(cursor))) cursor += 1; + if (cursor < text.length && (text[cursor] === "}" || text[cursor] === "]")) { + continue; // drop the comma; keep following structure + } + } + result += character; + continue; + } + + result += character; + if (escaped) { + escaped = false; + } else if (character === "\\") { + escaped = true; + } else if (character === '"') { + inString = false; + } + } + + return result; +} + +const SMART_QUOTES = new Map([ + ["\u201c", '"'], + ["\u201d", '"'], +]); + +/** + * Convert smart/typographic double quotes used as delimiters into straight + * ones. Apostrophes and single quotes are left alone: they are overwhelmingly + * legitimate content inside prose values, unlike curly double quotes which no + * valid JSON contains. + */ +function normalizeSmartQuotes(text: string): string { + let changed = false; + let result = ""; + for (const character of text) { + const replacement = SMART_QUOTES.get(character); + if (replacement !== undefined) { + result += replacement; + changed = true; + } else { + result += character; + } + } + return changed ? result : text; +} + +/** + * Quote bare JavaScript-style keys (`summary:` → `"summary":`). Walks outside + * string literals only, matching an identifier (or a single/smart-quoted key) + * directly preceding a colon that follows `{` or `,` structure. + */ +function quoteObjectLiteralKeys(text: string): string { + let result = ""; + let inString = false; + let escaped = false; + + const structurallyInsideObject = (): boolean => { + // Cheap approximation: after stripping strings, is the last non-space + // structural character a `{` or `,`? Good enough for best-effort repair; + // a wrong guess yields another failing candidate, never silent corruption. + let depthSquare = 0; + for (let index = result.length - 1; index >= 0; index -= 1) { + const prior = result[index]; + if (prior === "]") depthSquare += 1; + else if (prior === "[") { + if (depthSquare > 0) depthSquare -= 1; + else return false; + } else if (prior === "{" || prior === ",") { + return true; + } else if (prior === "}") { + return false; + } + } + return false; + }; + + const collectKey = (start: number): { raw: string; quoted: string } | null => { + let cursor = start; + const wrap = (inner: string): { raw: string; quoted: string } => ({ + raw: text.slice(start, cursor), + quoted: `"${inner.replace(/"/g, '\\"')}"`, + }); + + const current = text[cursor] ?? ""; + if (current === "'") { + const close = text.indexOf("'", cursor + 1); + if (close === -1) return null; + const inner = text.slice(cursor + 1, close); + cursor = close + 1; + return { raw: text.slice(start, cursor), quoted: `"${inner.replace(/"/g, '\\"')}"` }; + } + if (SMART_QUOTES.has(current)) { + const closeSmart = text.indexOf(current, cursor + 1); + if (closeSmart === -1) return null; + const inner = text.slice(cursor + 1, closeSmart); + cursor = closeSmart + 1; + return { raw: text.slice(start, cursor), quoted: `"${inner.replace(/"/g, '\\"')}"` }; + } + + let identifierEnd = -1; + while (cursor < text.length && /[A-Za-z0-9_$]/.test(text.charAt(cursor))) { + cursor += 1; + identifierEnd = cursor; + } + if (identifierEnd === -1) return null; + return wrap(text.slice(start, identifierEnd)); + }; + + for (let index = 0; index < text.length; index += 1) { + const character = text[index] ?? ""; + + if (!inString) { + if (/[A-Za-z0-9_$'\u201c\u201d]/.test(character)) { + const key = collectKey(index); + if (key && structurallyInsideObject()) { + let afterKey = index + key.raw.length; + while (afterKey < text.length && /\s/.test(text.charAt(afterKey))) afterKey += 1; + if (text[afterKey] === ":") { + result += `${key.quoted}: `; + index = afterKey; + continue; + } + } + result += character; + continue; + } + if (character === '"') inString = true; + result += character; + continue; + } + + result += character; + if (escaped) { + escaped = false; + } else if (character === "\\") { + escaped = true; + } else if (character === '"') { + inString = false; + } + } + + return result; +} + /** * Slice out every top-level `{...}` object using brace matching that respects * string state, so trailing junk after the real closing brace (e.g. grok's @@ -171,13 +421,120 @@ function balancedObjectCandidates(text: string): string[] { return candidates; } +/** + * Ordered repair pipelines applied to every base candidate. Earlier entries + * keep today's exact behavior; later passes handle progressively stranger + * JS-object-literal drift seen in live agent output. + */ +function buildVariants(base: string): string[] { + const controlEscaped = escapeControlCharsInStrings(base); + const commentsStripped = stripJsonComments(base); + const commentsControlEscaped = escapeControlCharsInStrings(commentsStripped); + + const variants = [ + base, + controlEscaped, + escapeUnescapedQuotesInStrings(controlEscaped), + escapeUnescapedQuotesStrictly(controlEscaped), + escapeUnescapedQuotesStrictly(escapeControlCharsInStrings(normalizeSmartQuotes(base))), + removeTrailingCommas(commentsControlEscaped), + escapeUnescapedQuotesInStrings(removeTrailingCommas(commentsControlEscaped)), + quoteObjectLiteralKeys(commentsControlEscaped), + escapeUnescapedQuotesStrictly( + quoteObjectLiteralKeys(removeTrailingCommas(commentsControlEscaped)), + ), + normalizeSmartQuotes(controlEscaped), + ]; + + return [...new Set(variants)]; +} + +/** Top-level keys the engine schema actually consumes, in salvage order. */ +const KNOWN_KEYS = ["summary", "description", "subtasks"] as const; + +const KEY_LINE_PATTERN = new RegExp(`^[ \\t]{0,6}"?(?:${KNOWN_KEYS.join("|")})"?[ \\t]*:`, "m"); + +/** + * Last-resort reconstruction for outputs whose quoting is too mangled for the + * incremental repair passes (e.g. prose like `Pick "one", then "two"` inside a + * value desyncs every string-state heuristic). Slice each known top-level key's + * raw value region — from just after its colon to the next key line or closing + * brace — and re-serialize it via JSON.stringify so anything text-shaped lands + * back in a valid payload. Only runs against keys this engine actually reads; + * nonsense outputs produce no candidate at all. + */ +function salvageKnownKeysCandidate(text: string): string | null { + type Anchor = { key: string; valueStart: number }; + const anchors: Anchor[] = []; + const pattern = new RegExp(KEY_LINE_PATTERN.source, "gm"); + let match: RegExpExecArray | null; + while ((match = pattern.exec(text)) !== null) { + const rawKey = match[0].replace(/^[ \t]*"?/, "").replace(/"?[ \t]*:$/, ""); + if ((KNOWN_KEYS as readonly string[]).includes(rawKey)) { + anchors.push({ key: rawKey, valueStart: match.index + match[0].length }); + // Avoid re-matching inside a value region. + pattern.lastIndex = match.index + match[0].length; + } + } + if (anchors.length === 0) return null; + + const entries: string[] = []; + for (let i = 0; i < anchors.length; i += 1) { + const anchor = anchors[i]; + if (!anchor) continue; + let end = text.length; + const next = anchors[i + 1]; + if (next) { + // Back up to the start of the next key's line for a clean slice edge. + end = text.lastIndexOf("\n", next.valueStart - 1) + 1; + } else { + const closeBrace = text.indexOf("}", anchor.valueStart); + if (closeBrace !== -1) end = closeBrace; + } + + let valueRaw = text.slice(anchor.valueStart, end).trim(); + // Peel fence decorations and delimiter quotes from string values. + valueRaw = valueRaw + .replace(/^```(?:json)?\s*/, "") + .replace(/\s*```$/, "") + .trim(); + if (anchor.key !== "subtasks") { + // Strip structural decorations outermost-first: separators before + // delimiter quotes, so `"value",` loses both without eating content. + valueRaw = valueRaw.replace(/,\s*$/, "").trim(); + if (valueRaw.startsWith('"')) valueRaw = valueRaw.slice(1).trim(); + if (valueRaw.endsWith('"')) valueRaw = valueRaw.slice(0, -1).trim(); + if (valueRaw.length > 0) { + entries.push(`"${anchor.key}": ${JSON.stringify(valueRaw)}`); + } + continue; + } + // subtasks must stay an array; only accept it when it round-trips. + try { + const parsed = JSON.parse(valueRaw.endsWith(",") ? valueRaw.slice(0, -1) : valueRaw); + if (Array.isArray(parsed)) { + entries.push(`"${anchor.key}": ${JSON.stringify(parsed)}`); + } + } catch { + // Not salvageable as an array — omit rather than corrupt the payload. + } + } + + return entries.length >= 2 || + (entries.length === 1 && (entries[0]?.startsWith('"summary"') ?? false)) + ? `{${entries.join(",")}}` + : null; +} + /** * Parse the first JSON object found in raw agent output. * - * Candidate order per repair level (none → control chars → control chars + - * embedded quotes): fenced ```json block, each balanced `{...}` object, the - * object closed right after its last string value (tolerates junk between the - * final value and the closing brace), then the whole trimmed text. + * Candidate order per repair level: fenced ```json block, each balanced + * `{...}` object, the object closed right after its last string value + * (tolerates junk between the final value and the closing brace), then the + * whole trimmed text. Each candidate retries through the ordered repair + * variants so one malformed habit doesn't sink the whole attempt. As a final + * fallback, a payload is rebuilt from known schema keys. * * @param raw - Raw agent stdout. * @returns The parsed object. @@ -202,20 +559,21 @@ export function parseAgentJson(raw: string): T { } } - bases.push(text); + const salvaged = salvageKnownKeysCandidate(text); + if (salvaged) { + bases.push(salvaged); + } - const variants = [ - ...bases, - ...bases.map(escapeControlCharsInStrings), - ...bases.map((base) => escapeUnescapedQuotesInStrings(escapeControlCharsInStrings(base))), - ]; + bases.push(text); let lastError: unknown; - for (const candidate of variants) { - try { - return JSON.parse(candidate) as T; - } catch (error) { - lastError = error; + for (const base of bases) { + for (const candidate of buildVariants(base)) { + try { + return JSON.parse(candidate) as T; + } catch (error) { + lastError = error; + } } } throw lastError instanceof Error ? lastError : new Error(String(lastError)); diff --git a/packages/pm/lib/chat/messages.ts b/packages/pm/lib/chat/messages.ts index 8d4b020..804342c 100644 --- a/packages/pm/lib/chat/messages.ts +++ b/packages/pm/lib/chat/messages.ts @@ -89,11 +89,17 @@ export function renderCreated( */ export function renderError(error: unknown): OutgoingMessage { if (error instanceof EngineError) { + if (error.code === "parse-failed") { + const dumpHint = error.dumpFile ? ` Full output was saved to \`${error.dumpFile}\`.` : ""; + return { + text: + "⚠️ The AI agent returned malformed output I couldn't turn into a draft. " + + `Try again, or ask the host to switch harness/model.${dumpHint}`, + }; + } const text = { "agent-failed": "⚠️ The AI agent failed to run on the host machine. Check the `devpm serve` logs, then send your request again.", - "parse-failed": - "⚠️ The AI agent returned something I couldn't parse into a draft. Try rephrasing your request.", "backend-failed": `⚠️ The task tracker rejected the request: ${error.message}`, }[error.code]; return { text }; diff --git a/packages/pm/lib/engine/index.ts b/packages/pm/lib/engine/index.ts index f843460..a1f5b4b 100644 --- a/packages/pm/lib/engine/index.ts +++ b/packages/pm/lib/engine/index.ts @@ -8,6 +8,7 @@ import { runAgent as defaultRunAgent } from "../agent.js"; import { dumpAgentOutput } from "../agent-debug.js"; +import type { AgentRunResult } from "@devintern/agent-harness"; import { attachmentsGuidanceBlurb, cleanupAttachmentStaging, @@ -48,6 +49,17 @@ export type { CreatedTask, LabelListResult, LabelRef } from "../backends/index.j /** Fallback issue types when a supporting backend cannot provide a list. */ export { DEFAULT_ISSUE_TYPES, getDefaultIssueType, orderIssueTypes }; +/** + * Appended to the one corrective re-run when an agent reply fails to parse. + * Keeps the ask narrow: the same payload, strictly canonical JSON. + */ +const STRICT_JSON_REMINDER = [ + "IMPORTANT: Your previous reply could not be parsed as JSON.", + "Respond again with ONLY the JSON object this task requires — no narration,", + "no markdown fences, no comments, no trailing commas. Quote every key with", + 'double quotes and escape any double quotes inside values as \\".', +].join(" "); + export interface GenerateStoryInput { source: SourceInput; promptStyle: PromptStyle; @@ -233,38 +245,70 @@ export async function createEngine( agentFiles?: { attachmentPaths: string[]; imagePaths: string[] }, ): Promise { const onAgentChunk = events?.onAgentChunk; - const result = await runAgent(config.agent.harness, config.agent.path, prompt, { - maxTurns: 100, - skipPermissions: true, - model, - silent: true, - attachmentPaths: agentFiles?.attachmentPaths, - imagePaths: agentFiles?.imagePaths, - onStdout: onAgentChunk ? (chunk) => onAgentChunk(chunk, "stdout") : undefined, - onStderr: onAgentChunk ? (chunk) => onAgentChunk(chunk, "stderr") : undefined, - }); - const dumpContext = { harness: config.agent.harness.name, cliPath: config.agent.path }; - if (result.exitCode !== 0) { - const dumpFile = await dumpAgentOutput(label, result, dumpContext); - throw new EngineError( - "agent-failed", - failureMessage, - result.stderr.trim() || "Unknown agent error", - dumpFile ?? undefined, - ); - } + type AttemptOutcome = + | { ok: true; payload: T } + | { ok: false; stage: "agent"; error: EngineError } + | { ok: false; stage: "parse"; error: EngineError; rawResult: AgentRunResult }; + + async function attemptParse(currentPrompt: string): Promise { + const result = await runAgent(config.agent.harness, config.agent.path, currentPrompt, { + maxTurns: 100, + skipPermissions: true, + model, + silent: true, + attachmentPaths: agentFiles?.attachmentPaths, + imagePaths: agentFiles?.imagePaths, + onStdout: onAgentChunk ? (chunk) => onAgentChunk(chunk, "stdout") : undefined, + onStderr: onAgentChunk ? (chunk) => onAgentChunk(chunk, "stderr") : undefined, + }); + + if (result.exitCode !== 0) { + const dumpFile = await dumpAgentOutput(label, result, dumpContext); + return { + ok: false, + stage: "agent", + error: new EngineError( + "agent-failed", + failureMessage, + result.stderr.trim() || "Unknown agent error", + dumpFile ?? undefined, + ), + }; + } - try { - return extractJsonPayload(result.stdout, validate, invalidMessage); - } catch (error) { - if (error instanceof EngineError) { - const dumpFile = await dumpAgentOutput(`${label}-parse`, result, dumpContext); - throw new EngineError(error.code, error.message, error.detail, dumpFile ?? undefined); + try { + return { ok: true, payload: extractJsonPayload(result.stdout, validate, invalidMessage) }; + } catch (error) { + if (!(error instanceof EngineError)) throw error; + return { ok: false, stage: "parse", error, rawResult: result }; } - throw error; } + + /** + * Agent runs are by far the slowest part of a generation, so give + * malformed output one corrective re-run before giving up. The re-run + * appends a strict-output reminder to the same prompt; whatever still + * fails is dumped and raised as `parse-failed`. + */ + const first = await attemptParse(prompt); + if (first.ok) return first.payload; + if (first.stage === "agent") throw first.error; + + const retry = await attemptParse(`${prompt}\n\n${STRICT_JSON_REMINDER}`); + if (retry.ok) return retry.payload; + if (retry.stage === "agent") throw retry.error; + + // Both attempts failed to parse — surface the latest failure alongside a + // dump of the final attempt so users can inspect exactly what came back. + const dumpFile = await dumpAgentOutput(`${label}-parse`, retry.rawResult, dumpContext); + throw new EngineError( + retry.error.code, + retry.error.message, + retry.error.detail, + dumpFile ?? undefined, + ); } return { diff --git a/packages/pm/lib/engine/json.ts b/packages/pm/lib/engine/json.ts index 26dc751..2ede50b 100644 --- a/packages/pm/lib/engine/json.ts +++ b/packages/pm/lib/engine/json.ts @@ -9,7 +9,10 @@ import { EngineError } from "./types.js"; * Extract and validate a JSON payload from raw agent output. * * Delegates to {@link parseAgentJson}, which tolerates fenced ```json blocks, - * bare JSON, and prose-prefixed JSON (some agents narrate before the object). + * bare JSON, prose-prefixed JSON, and common object-literal drift (comments, + * trailing commas, unquoted keys, stray inner quotes). Failures surface as a + * friendly {@link EngineError} — the low-level parser diagnostics stay in + * `detail`, never as the user-facing headline. * * @param raw - Raw agent stdout. * @param validate - Type guard for the expected payload shape. @@ -26,9 +29,10 @@ export function extractJsonPayload( try { parsed = parseAgentJson(raw); } catch (error) { + const technical = error instanceof Error ? error.message : String(error); throw new EngineError( "parse-failed", - error instanceof Error ? error.message : String(error), + `The agent returned malformed output that could not be repaired automatically (${technical}). Retry the generation, or try another harness/model.`, raw, ); }