diff --git a/app/sidecar/__tests__/message-processor.test.ts b/app/sidecar/__tests__/message-processor.test.ts index 3f4b29028..20dfc7941 100644 --- a/app/sidecar/__tests__/message-processor.test.ts +++ b/app/sidecar/__tests__/message-processor.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, beforeEach } from "vitest"; import { MessageProcessor } from "../message-processor.js"; -import { extractResultMarkdown, tryParseJsonFromText } from "../lib/result-extraction.js"; +import { extractResultMarkdown } from "../lib/result-extraction.js"; import type { DisplayItem, DisplayItemEnvelope } from "../display-types.js"; import type { AgentEventEnvelope } from "../agent-events.js"; @@ -651,42 +651,11 @@ describe("MessageProcessor", () => { }); // ========================================================================= - // tryParseJsonFromText helper + // resultMarkdown is derived only from structured output // ========================================================================= - describe("tryParseJsonFromText", () => { - it("parses plain JSON string", () => { - const result = tryParseJsonFromText('{"status":"ok","count":3}'); - expect(result).toEqual({ status: "ok", count: 3 }); - }); - - it("strips ```json code fence before parsing", () => { - const text = "```json\n{\"status\":\"validation_complete\",\"validation_log_markdown\":\"# Log\"}\n```"; - const result = tryParseJsonFromText(text) as Record; - expect(result.status).toBe("validation_complete"); - expect(result.validation_log_markdown).toBe("# Log"); - }); - - it("strips plain ``` code fence before parsing", () => { - const text = "```\n{\"key\":\"value\"}\n```"; - expect(tryParseJsonFromText(text)).toEqual({ key: "value" }); - }); - - it("returns undefined for non-JSON text", () => { - expect(tryParseJsonFromText("just some plain text")).toBeUndefined(); - }); - - it("returns undefined for empty string", () => { - expect(tryParseJsonFromText("")).toBeUndefined(); - }); - }); - - // ========================================================================= - // resultMarkdown fallback from output text block - // ========================================================================= - - describe("resultMarkdown fallback from output text", () => { - it("extracts resultMarkdown from last output text when structuredOutput is absent", () => { + describe("resultMarkdown without structured output", () => { + it("does not extract resultMarkdown from last output text when structuredOutput is absent", () => { const jsonText = JSON.stringify({ status: "validation_complete", validation_log_markdown: "# Validation Log\n\nAll good.", @@ -707,12 +676,11 @@ describe("MessageProcessor", () => { const items = extractDisplayItems(out); const resultItem = items.find((i) => i.type === "result"); - expect(resultItem?.resultMarkdown).toContain("# Validation Log"); - expect(resultItem?.resultMarkdown).toContain("# Tests"); - expect(resultItem?.structuredOutput).toMatchObject({ status: "validation_complete" }); + expect(resultItem?.resultMarkdown).toBeUndefined(); + expect(resultItem?.structuredOutput).toBeUndefined(); }); - it("extracts resultMarkdown from output text with ```json code fence", () => { + it("does not extract resultMarkdown from fenced JSON output text", () => { const jsonText = "```json\n" + JSON.stringify({ status: "validation_complete", validation_log_markdown: "# Log", @@ -730,7 +698,8 @@ describe("MessageProcessor", () => { const items = extractDisplayItems(out); const resultItem = items.find((i) => i.type === "result"); - expect(resultItem?.resultMarkdown).toBe("# Log"); + expect(resultItem?.resultMarkdown).toBeUndefined(); + expect(resultItem?.structuredOutput).toBeUndefined(); }); it("does not override structuredOutput when already present", () => { diff --git a/app/sidecar/__tests__/result-extraction.test.ts b/app/sidecar/__tests__/result-extraction.test.ts index 2ac10a5fa..d122e93ce 100644 --- a/app/sidecar/__tests__/result-extraction.test.ts +++ b/app/sidecar/__tests__/result-extraction.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from "vitest"; -import { extractResultMarkdown, tryParseJsonFromText } from "../lib/result-extraction"; +import { extractResultMarkdown } from "../lib/result-extraction.js"; // --------------------------------------------------------------------------- // extractResultMarkdown @@ -60,81 +60,3 @@ describe("extractResultMarkdown", () => { expect(result).toBe("included"); }); }); - -// --------------------------------------------------------------------------- -// tryParseJsonFromText -// --------------------------------------------------------------------------- - -describe("tryParseJsonFromText", () => { - it("parses valid JSON directly", () => { - expect(tryParseJsonFromText('{"key": "value"}')).toEqual({ key: "value" }); - }); - - it("parses JSON wrapped in code fences", () => { - const text = "```json\n{\"key\": \"value\"}\n```"; - expect(tryParseJsonFromText(text)).toEqual({ key: "value" }); - }); - - it("parses JSON wrapped in plain code fences (no language tag)", () => { - const text = "```\n{\"key\": \"value\"}\n```"; - expect(tryParseJsonFromText(text)).toEqual({ key: "value" }); - }); - - it("returns undefined for invalid JSON", () => { - expect(tryParseJsonFromText("not json at all")).toBeUndefined(); - }); - - it("returns undefined for empty string", () => { - expect(tryParseJsonFromText("")).toBeUndefined(); - }); - - it("parses JSON arrays", () => { - expect(tryParseJsonFromText("[1, 2, 3]")).toEqual([1, 2, 3]); - }); - - it("handles whitespace around fences", () => { - const text = "```json\n { \"a\": 1 } \n``` "; - expect(tryParseJsonFromText(text)).toEqual({ a: 1 }); - }); - - it("parses primitive JSON values", () => { - expect(tryParseJsonFromText('"hello"')).toBe("hello"); - expect(tryParseJsonFromText("42")).toBe(42); - expect(tryParseJsonFromText("true")).toBe(true); - expect(tryParseJsonFromText("null")).toBeNull(); - }); - - it("extracts JSON from text with preamble before code fence", () => { - const text = 'Here is the result:\n\n```json\n{"status": "research_complete", "count": 3}\n```'; - expect(tryParseJsonFromText(text)).toEqual({ status: "research_complete", count: 3 }); - }); - - it("extracts JSON from text with preamble and postamble around code fence", () => { - const text = 'I completed the research.\n\n```json\n{"status": "done"}\n```\n\nLet me know if you need more.'; - expect(tryParseJsonFromText(text)).toEqual({ status: "done" }); - }); - - it("extracts JSON object from plain text without code fences", () => { - const text = 'Now I have all the outputs. Let me consolidate:\n\n{"status": "research_complete", "dimensions_selected": 3}'; - expect(tryParseJsonFromText(text)).toEqual({ status: "research_complete", dimensions_selected: 3 }); - }); - - it("handles nested braces in brace-matching extraction", () => { - const text = 'Result:\n{"outer": {"inner": "value"}, "count": 1}'; - expect(tryParseJsonFromText(text)).toEqual({ outer: { inner: "value" }, count: 1 }); - }); - - it("handles strings with braces in brace-matching extraction", () => { - const text = 'Output:\n{"message": "use {name} placeholder", "ok": true}'; - expect(tryParseJsonFromText(text)).toEqual({ message: "use {name} placeholder", ok: true }); - }); - - it("returns undefined when no valid JSON exists anywhere", () => { - expect(tryParseJsonFromText("Just some text with no JSON at all")).toBeUndefined(); - }); - - it("returns undefined for malformed JSON in code fence", () => { - const text = '```json\n{invalid json}\n```'; - expect(tryParseJsonFromText(text)).toBeUndefined(); - }); -}); diff --git a/app/sidecar/__tests__/sdk-output-format.integration.test.ts b/app/sidecar/__tests__/sdk-output-format.integration.test.ts new file mode 100644 index 000000000..1e9939a44 --- /dev/null +++ b/app/sidecar/__tests__/sdk-output-format.integration.test.ts @@ -0,0 +1,125 @@ +/** + * Integration test: verifies whether the SDK populates structured_output + * when outputFormat is passed with a nested JSON schema. + * + * This test guards the previously observed upstream SDK bug: + * anthropics/claude-agent-sdk-typescript#277 + * + * Run with ANTHROPIC_API_KEY set: + * ANTHROPIC_API_KEY=sk-... npx vitest run __tests__/sdk-output-format.integration.test.ts + * + * If the SDK regresses, the "nested schema" test will fail because the app + * requires structured_output for outputFormat runs. + */ +import { describe, it, expect } from "vitest"; +import { query } from "@anthropic-ai/claude-agent-sdk"; + +const HAS_API_KEY = !!process.env.ANTHROPIC_API_KEY; + +/** Flat schema — no nested objects. Used as the control. */ +const FLAT_SCHEMA = { + type: "object" as const, + properties: { + status: { type: "string" as const }, + count: { type: "number" as const }, + }, + required: ["status", "count"], + additionalProperties: false, +}; + +/** Nested schema — contains an object-typed property inside the top-level object. + * This shape previously triggered the SDK bug for non-trivial schemas. */ +const NESTED_SCHEMA = { + type: "object" as const, + properties: { + status: { type: "string" as const }, + metadata: { + type: "object" as const, + properties: { + count: { type: "number" as const }, + tags: { + type: "array" as const, + items: { type: "string" as const }, + }, + }, + required: ["count", "tags"], + additionalProperties: false, + }, + items: { + type: "array" as const, + items: { + type: "object" as const, + properties: { + id: { type: "string" as const }, + value: { type: "number" as const }, + }, + required: ["id", "value"], + additionalProperties: false, + }, + }, + }, + required: ["status", "metadata", "items"], + additionalProperties: false, +}; + +async function runQuery(schema: Record, prompt: string): Promise<{ subtype: string; structured_output: unknown; result: unknown }> { + let resultMsg: Record | null = null; + + for await (const msg of query({ + prompt, + options: { + model: "claude-haiku-4-5-20251001", + outputFormat: { type: "json_schema", schema }, + maxTurns: 2, + permissionMode: "bypassPermissions" as const, + }, + })) { + const m = msg as Record; + if (m.type === "result") resultMsg = m; + } + + if (!resultMsg) throw new Error("No result message from SDK"); + + return { + subtype: String(resultMsg.subtype ?? ""), + structured_output: resultMsg.structured_output, + result: resultMsg.result, + }; +} + +describe.skipIf(!HAS_API_KEY)("SDK outputFormat — structured_output presence (VU-1015)", () => { + it("populates structured_output for a flat (non-nested) schema", async () => { + const { subtype, structured_output } = await runQuery( + FLAT_SCHEMA, + 'Return JSON with status "ok" and count 1.', + ); + + expect(subtype).toBe("success"); + // If this assertion fails, the SDK is broken even for flat schemas. + expect(structured_output).not.toBeNull(); + expect(structured_output).not.toBeUndefined(); + expect(structured_output).toMatchObject({ status: expect.any(String), count: expect.any(Number) }); + }, 30_000); + + it("populates structured_output for a nested schema", async () => { + const { subtype, structured_output } = await runQuery( + NESTED_SCHEMA, + 'Return JSON: status "complete", metadata with count 2 and tags ["a","b"], items [{id:"x",value:1},{id:"y",value:2}].', + ); + + expect(subtype).toBe("success"); + // This is the canary for the upstream SDK bug (anthropics/claude-agent-sdk-typescript#277). + // If structured_output is null/undefined here, the SDK has regressed and outputFormat + // runs must fail rather than parse text fallback. + expect(structured_output).not.toBeNull(); + expect(structured_output).not.toBeUndefined(); + expect(structured_output).toMatchObject({ + status: expect.any(String), + metadata: { + count: expect.any(Number), + tags: expect.any(Array), + }, + items: expect.any(Array), + }); + }, 30_000); +}); diff --git a/app/sidecar/__tests__/structured-output-required.test.ts b/app/sidecar/__tests__/structured-output-required.test.ts new file mode 100644 index 000000000..06b55c452 --- /dev/null +++ b/app/sidecar/__tests__/structured-output-required.test.ts @@ -0,0 +1,217 @@ +/** + * Tests for VU-1015: SDK outputFormat should provide structured_output + * for nested schemas (anthropics/claude-agent-sdk-typescript#277). + * + * When outputFormat is configured, MessageProcessor requires SDK + * structured_output and hard-fails if it is missing. Text parsing fallback + * belongs outside this path now that the SDK canary passes for nested schemas. + */ +import { describe, it, expect, beforeEach } from "vitest"; +import { MessageProcessor } from "../message-processor.js"; +import type { DisplayItem, DisplayItemEnvelope } from "../display-types.js"; + +function extractDisplayItems(output: Record[]): DisplayItem[] { + return output + .filter((o) => o.type === "display_item") + .map((o) => (o as DisplayItemEnvelope).item); +} + +describe("structured_output required outputFormat path (VU-1015)", () => { + describe("with hasOutputFormat: true (requireStructuredOutput)", () => { + let processor: MessageProcessor; + + beforeEach(() => { + processor = new MessageProcessor({ hasOutputFormat: true, pluginSlug: "test" }); + }); + + it("uses SDK structured_output when present (happy path)", () => { + const schema = { step_summary: "All steps complete", artifacts: ["a.ts", "b.ts"] }; + const raw = { + type: "result", + subtype: "success", + structured_output: schema, + result: "some text summary", + usage: { input_tokens: 100, output_tokens: 50 }, + }; + + const out = processor.process(raw); + const items = extractDisplayItems(out); + const result = items.find((i) => i.type === "result"); + + expect(result?.resultStatus).toBe("success"); + expect(result?.structuredOutput).toEqual(schema); + }); + + it("hard-fails when SDK omits structured_output even if result text is JSON", () => { + const schema = { step_summary: "Research complete", artifacts: ["plan.md"] }; + const raw = { + type: "result", + subtype: "success", + // structured_output intentionally absent. + result: JSON.stringify(schema), + usage: { input_tokens: 100, output_tokens: 50 }, + }; + + const out = processor.process(raw); + const items = extractDisplayItems(out); + const result = items.find((i) => i.type === "result"); + + expect(result?.resultStatus).toBe("error"); + expect(result?.errorSubtype).toBe("structured_output_missing"); + expect(result?.structuredOutput).toBeUndefined(); + }); + + it("hard-fails when SDK returns structured_output: null even if result text is JSON", () => { + const schema = { step_summary: "Done", next_steps: ["deploy"] }; + const raw = { + type: "result", + subtype: "success", + structured_output: null, + result: JSON.stringify(schema), + usage: { input_tokens: 100, output_tokens: 50 }, + }; + + const out = processor.process(raw); + const items = extractDisplayItems(out); + const result = items.find((i) => i.type === "result"); + + expect(result?.resultStatus).toBe("error"); + expect(result?.errorSubtype).toBe("structured_output_missing"); + expect(result?.structuredOutput).toBeUndefined(); + }); + + it("hard-fails when SDK omits structured_output even if result is an object", () => { + const raw = { + type: "result", + subtype: "success", + result: { step_summary: "Done" }, + usage: { input_tokens: 100, output_tokens: 50 }, + }; + + const out = processor.process(raw); + const items = extractDisplayItems(out); + const result = items.find((i) => i.type === "result"); + + expect(result?.resultStatus).toBe("error"); + expect(result?.errorSubtype).toBe("structured_output_missing"); + expect(result?.structuredOutput).toBeUndefined(); + }); + + it("hard-fails instead of parsing last assistant JSON text", () => { + const schema = { step_summary: "Done", nested: { level: 2, items: [1, 2, 3] } }; + + processor.process({ + type: "assistant", + message: { + content: [{ type: "text", text: JSON.stringify(schema) }], + usage: { input_tokens: 10, output_tokens: 5 }, + }, + }); + + const raw = { + type: "result", + subtype: "success", + structured_output: null, + result: "I have completed the research phase.", + usage: { input_tokens: 200, output_tokens: 80 }, + }; + + const out = processor.process(raw); + const items = extractDisplayItems(out); + const result = items.find((i) => i.type === "result"); + + expect(result?.resultStatus).toBe("error"); + expect(result?.errorSubtype).toBe("structured_output_missing"); + expect(result?.structuredOutput).toBeUndefined(); + }); + + it("hard-fails instead of parsing fenced JSON from last assistant text", () => { + const schema = { step_summary: "Done", code_files: ["lib/a.ts", "lib/b.ts"] }; + const fencedJson = "```json\n" + JSON.stringify(schema) + "\n```"; + + processor.process({ + type: "assistant", + message: { + content: [{ type: "text", text: fencedJson }], + usage: { input_tokens: 10, output_tokens: 5 }, + }, + }); + + const raw = { + type: "result", + subtype: "success", + structured_output: null, + result: "Task complete.", + usage: { input_tokens: 200, output_tokens: 80 }, + }; + + const out = processor.process(raw); + const items = extractDisplayItems(out); + const result = items.find((i) => i.type === "result"); + + expect(result?.resultStatus).toBe("error"); + expect(result?.errorSubtype).toBe("structured_output_missing"); + expect(result?.structuredOutput).toBeUndefined(); + }); + + it("hard-fails with structured_output_missing when no JSON is recoverable", () => { + // No structured_output, no JSON in result text, no prior assistant text. + const raw = { + type: "result", + subtype: "success", + structured_output: null, + result: "I completed the task but forgot to output JSON.", + usage: { input_tokens: 100, output_tokens: 50 }, + }; + + const out = processor.process(raw); + const items = extractDisplayItems(out); + const result = items.find((i) => i.type === "result"); + + expect(result?.resultStatus).toBe("error"); + expect(result?.errorSubtype).toBe("structured_output_missing"); + }); + + it("hard-fails when structured_output is null and result is empty", () => { + const raw = { + type: "result", + subtype: "success", + structured_output: null, + usage: { input_tokens: 100, output_tokens: 50 }, + }; + + const out = processor.process(raw); + const items = extractDisplayItems(out); + const result = items.find((i) => i.type === "result"); + + expect(result?.resultStatus).toBe("error"); + expect(result?.errorSubtype).toBe("structured_output_missing"); + }); + }); + + describe("without hasOutputFormat (default)", () => { + let processor: MessageProcessor; + + beforeEach(() => { + processor = new MessageProcessor(); + }); + + it("succeeds even when structured_output is null and result has no JSON", () => { + // Without hasOutputFormat, missing structured_output is not an error. + const raw = { + type: "result", + subtype: "success", + structured_output: null, + result: "Task complete with no structured output.", + usage: { input_tokens: 100, output_tokens: 50 }, + }; + + const out = processor.process(raw); + const items = extractDisplayItems(out); + const result = items.find((i) => i.type === "result"); + + expect(result?.resultStatus).toBe("success"); + expect(result?.structuredOutput).toBeUndefined(); + }); + }); +}); diff --git a/app/sidecar/lib/result-extraction.ts b/app/sidecar/lib/result-extraction.ts index 351609dc8..961a90ad3 100644 --- a/app/sidecar/lib/result-extraction.ts +++ b/app/sidecar/lib/result-extraction.ts @@ -1,5 +1,5 @@ /** - * Pure utility functions for extracting structured results from agent output. + * Pure utility functions for rendering structured results. * * @module result-extraction */ @@ -17,80 +17,3 @@ export function extractResultMarkdown(structuredOutput: unknown): string | undef .map(([, val]) => val as string); return sections.length > 0 ? sections.join("\n\n---\n\n") : undefined; } - -/** - * Attempts to parse JSON from a text block. Tries in order: - * 1. Direct JSON.parse (text is pure JSON) - * 2. Strip wrapping code fences (```json ... ``` or ``` ... ```) - * 3. Extract a fenced JSON code block from surrounding text (e.g. preamble - * before ```json\n{...}\n```) — handles the common case where the agent - * wraps structured output in explanatory markdown - * 4. Extract the first top-level JSON object ({...}) from the text - * - * Returns the parsed value or undefined if no valid JSON is found. - */ -export function tryParseJsonFromText(text: string): unknown { - // 1. Direct parse - try { - return JSON.parse(text.trim()); - } catch { - // continue - } - - // 2. Entire text is a code-fenced block - const stripped = text.replace(/^```(?:json)?\s*\n?/, "").replace(/\n?```\s*$/, "").trim(); - if (stripped !== text.trim()) { - try { - return JSON.parse(stripped); - } catch { - // continue - } - } - - // 3. Extract fenced JSON block from surrounding text - const fenceMatch = text.match(/```(?:json)?\s*\n([\s\S]*?)\n```/); - if (fenceMatch) { - try { - return JSON.parse(fenceMatch[1].trim()); - } catch { - // continue - } - } - - // 4. Extract first top-level JSON object by brace-matching - const startIdx = text.indexOf("{"); - if (startIdx !== -1) { - let depth = 0; - let inString = false; - let escape = false; - for (let i = startIdx; i < text.length; i++) { - const ch = text[i]; - if (escape) { - escape = false; - continue; - } - if (ch === "\\") { - escape = true; - continue; - } - if (ch === '"') { - inString = !inString; - continue; - } - if (inString) continue; - if (ch === "{") depth++; - else if (ch === "}") { - depth--; - if (depth === 0) { - try { - return JSON.parse(text.slice(startIdx, i + 1)); - } catch { - break; - } - } - } - } - } - - return undefined; -} diff --git a/app/sidecar/message-processor.ts b/app/sidecar/message-processor.ts index 715ba53b1..b7fef2fee 100644 --- a/app/sidecar/message-processor.ts +++ b/app/sidecar/message-processor.ts @@ -25,7 +25,7 @@ import { truncate, computeToolSummary } from "./tool-summaries.js"; import { RESULT_ERROR_LABELS, ASSISTANT_ERROR_LABELS } from "./error-labels.js"; import { RunMetadataAccumulator } from "./run-metadata-accumulator.js"; import type { RequestContext } from "./run-metadata-accumulator.js"; -import { extractResultMarkdown, tryParseJsonFromText } from "./lib/result-extraction.js"; +import { extractResultMarkdown } from "./lib/result-extraction.js"; // --------------------------------------------------------------------------- // MessageProcessor @@ -41,7 +41,7 @@ export class MessageProcessor { /** True once processResultMessage has emitted a run_result event. */ private resultEmitted = false; - /** Last top-level output text block — used as fallback for structured output extraction. */ + /** Last top-level output text block — used for non-structured run summaries. */ private lastOutputText: string | undefined; /** ID of the last top-level output item — used to emit a replacement when consumed by result. */ @@ -662,42 +662,21 @@ export class MessageProcessor { if ("structured_output" in raw && raw.structured_output != null) { structuredOutput = raw.structured_output; - } else if ("result" in raw && raw.result != null && typeof raw.result !== "string") { + } else if (!this.requireStructuredOutput && "result" in raw && raw.result != null && typeof raw.result !== "string") { structuredOutput = raw.result; } - // When structured_output is absent, try to parse JSON from the result text. - // The SDK silently drops outputFormat enforcement for non-trivial schemas - // (known bug: anthropics/claude-agent-sdk-typescript#277), but prompt - // directives make the model return raw JSON in the result text field. - // Rust serde validates the parsed JSON downstream — this is a best-effort - // extraction, not a schema validation step. - if (structuredOutput == null) { - // Try parsing result text as JSON (covers both outputFormat and non-outputFormat paths). - const textToParse = (typeof raw.result === "string" ? raw.result : null) ?? this.lastOutputText; - if (textToParse) { - const parsed = tryParseJsonFromText(textToParse); - if (parsed != null && typeof parsed === "object") { - structuredOutput = parsed; - if (this.requireStructuredOutput) { - process.stderr.write( - `[message-processor] event=structured_output_fallback subtype=${subtype ?? "none"} ` + - `source=result_text — SDK did not populate structured_output, parsed from result text\n` - ); - } - } - } - - // If outputFormat was configured and we still have nothing: hard fail. - if (structuredOutput == null && this.requireStructuredOutput) { - resultStatus = "error"; - errorSubtype = "structured_output_missing"; - outputText = "Structured output missing — the agent did not return JSON matching the required schema. Retry the step."; - process.stderr.write( - `[message-processor] event=structured_output_missing subtype=${subtype ?? "none"} ` + - `result_type=${typeof raw.result} result_length=${typeof raw.result === "string" ? raw.result.length : 0}\n` - ); - } + // outputFormat is an SDK contract: when configured, the SDK must provide + // structured_output. If it is absent, fail clearly instead of recovering + // from display text. + if (structuredOutput == null && this.requireStructuredOutput) { + resultStatus = "error"; + errorSubtype = "structured_output_missing"; + outputText = "Structured output missing — the agent did not return JSON matching the required schema. Retry the step."; + process.stderr.write( + `[message-processor] event=structured_output_missing subtype=${subtype ?? "none"} ` + + `result_type=${typeof raw.result} result_length=${typeof raw.result === "string" ? raw.result.length : 0}\n` + ); } const resultMarkdown = extractResultMarkdown(structuredOutput); diff --git a/app/src-tauri/schemas-review/test-sdk-multiturn.mjs b/app/src-tauri/schemas-review/test-sdk-multiturn.mjs index a4ac9df91..a45596a13 100644 --- a/app/src-tauri/schemas-review/test-sdk-multiturn.mjs +++ b/app/src-tauri/schemas-review/test-sdk-multiturn.mjs @@ -2,10 +2,11 @@ * Test: does the Agent SDK return structured_output for each step's inline schema? * * Pass criteria (in order): - * 1. structured_output is present → PASS (SDK enforced) - * 2. structured_output is null → parse result text with Rust serde (validate-output binary) - * - Rust serde succeeds → PASS (prompt directives + Rust validation worked) - * - Rust serde fails → FAIL + * 1. structured_output is present (SDK enforced) + * 2. Rust serde accepts that structured_output for the step contract + * + * Missing structured_output is a failure; the app no longer parses result text + * as a recovery path for outputFormat runs. * * Usage: * cd app @@ -210,29 +211,26 @@ for (const step of toRun) { } const hasStructured = result.structured_output != null; - console.log(` structured_output: ${hasStructured ? "present (SDK enforced)" : "null (SDK bug)"}`); + console.log(` structured_output: ${hasStructured ? "present (SDK enforced)" : "missing"}`); console.log(` subtype: ${result.subtype}`); - if (hasStructured) { - // Best case: SDK enforced the schema - console.log(` \x1b[32mPASS\x1b[0m structured_output present`); - writeFileSync(join(outDir, `step-${step.id}.json`), JSON.stringify(result.structured_output, null, 2)); - passed++; + if (!hasStructured) { + console.log(` \x1b[31mFAIL\x1b[0m structured_output missing`); + writeFileSync(join(outDir, `step-${step.id}-failed.txt`), String(result.result ?? "")); + failed++; } else { - // Fallback: validate result text with Rust serde - const text = String(result.result ?? "").trim(); - console.log(` result text (first 120): ${text.slice(0, 120)}...`); - console.log(` Validating with Rust serde...`); + const jsonText = JSON.stringify(result.structured_output, null, 2); + console.log(` Validating structured_output with Rust serde...`); - const validation = validateWithRust(step.id, text); + const validation = validateWithRust(step.id, jsonText); if (validation.ok) { - console.log(` \x1b[32mPASS\x1b[0m Rust serde deserialized successfully`); - writeFileSync(join(outDir, `step-${step.id}.json`), text); + console.log(` \x1b[32mPASS\x1b[0m structured_output present and valid`); + writeFileSync(join(outDir, `step-${step.id}.json`), jsonText); passed++; } else { - console.log(` \x1b[31mFAIL\x1b[0m Rust serde rejected the output`); + console.log(` \x1b[31mFAIL\x1b[0m Rust serde rejected structured_output`); console.log(` ${validation.error.trim()}`); - writeFileSync(join(outDir, `step-${step.id}-failed.txt`), text); + writeFileSync(join(outDir, `step-${step.id}-failed.json`), jsonText); failed++; } } diff --git a/docs/design/agent-specs/README.md b/docs/design/agent-specs/README.md index fd0b8b6d0..e83af69f7 100644 --- a/docs/design/agent-specs/README.md +++ b/docs/design/agent-specs/README.md @@ -37,8 +37,8 @@ One agent delegates to a plugin-internal skill: JSON contract write path: - Steps 0, 1, and 2 return structured payloads. Steps 1 and 2 run as direct agents (not subagent relay) so `outputFormat` applies to the producing agent. -- SDK `outputFormat` is set with inline JSON Schema generated from Rust structs (no `$ref`). Due to a known SDK bug ([anthropics/claude-agent-sdk-typescript#277](https://github.com/anthropics/claude-agent-sdk-typescript/issues/277)), `structured_output` is not populated for nested schemas. As a workaround, agents include prompt directives ("CRITICAL — raw JSON only") and reference generated JSON schema files. The sidecar parses JSON from the `result` text field when `structured_output` is absent. -- Rust deserializes the extracted JSON into typed contract structs (`ResearchStepOutput`, `DetailedResearchOutput`, `DecisionsOutput`) — this is the authoritative validation. +- SDK `outputFormat` is set with inline JSON Schema generated from Rust structs (no `$ref`). `structured_output` is required for these runs; if it is absent, the sidecar emits `structured_output_missing` instead of parsing JSON from text. +- Rust deserializes `structured_output` into typed contract structs (`ResearchStepOutput`, `DetailedResearchOutput`, `DecisionsOutput`) — this is the authoritative validation. - `answer-evaluator` follows the same structured-output pattern for `answer-evaluation.json`. - Agent-facing schema references are at `agent-sources/plugins/skill-content-researcher/shared/schemas.md` (semantic rules) and `shared/output-schemas/` (generated JSON Schema files agents can Read). @@ -78,8 +78,8 @@ Written by `answer-evaluator` as a gate check before advancing from steps 0 and | Structured-output materialization | Steps 0/1/2 + gate evaluator validated/written by backend | This page now documents backend materialization path | aligned | | Rust contract structs | All workflow output types defined in `contracts/` with Specta + Schemars derives | `canonical-format.md` references Rust as canonical source | aligned | | Codegen pipeline | `cargo run --bin codegen` generates TS types + inline JSON Schema | Enforcement layers table documents freshness check | aligned | -| SDK outputFormat | Inline JSON Schema passed for steps 0-2; `structured_output` not populated (SDK bug) | Documented with workaround (prompt directives + result text fallback) | known issue | -| Sidecar fallback | `tryParseJsonFromText` extracts JSON from `result` text when `structured_output` absent | `canonical-format.md` documents extraction flow | aligned | +| SDK outputFormat | Inline JSON Schema passed for steps 0-2; `structured_output` required | Documented with integration canary for nested schemas | aligned | +| Sidecar missing-output handling | Emits `structured_output_missing` when `outputFormat` was configured but SDK omits `structured_output` | `canonical-format.md` documents extraction flow | aligned | | Agent prompt directives | Agent `.md` files reference generated `output-schemas/` and include "raw JSON only" instructions | `schemas.md` path updated to `shared/` | aligned | | Workflow step outputs | Step 3 writes `SKILL.md`, `references/`, `context/evaluations.md` | Workflow table includes all three outputs | aligned | | `answer-evaluation.json` consumers | Used by `detailed-research` | Infrastructure note reflects `detailed-research` only | aligned | @@ -90,4 +90,4 @@ Written by `answer-evaluator` as a gate check before advancing from steps 0 and 1. Normalize remaining legacy `clarifications.md` / `decisions.md` wording in sidecar mock transcript templates. 2. Add/extend tests to flag new legacy transcript references automatically. 3. Keep this page and `canonical-format.md` synchronized whenever agent I/O contracts change. -4. When SDK bug ([anthropics/claude-agent-sdk-typescript#277](https://github.com/anthropics/claude-agent-sdk-typescript/issues/277)) is fixed: `structured_output` becomes the primary path. The `result` text fallback remains permanent (covers errors and missing `structured_output`). Prompt directives can be relaxed but not removed. +4. Keep the nested-schema SDK canary ([anthropics/claude-agent-sdk-typescript#277](https://github.com/anthropics/claude-agent-sdk-typescript/issues/277)) green. If the SDK omits `structured_output`, fail the run rather than parsing result text. Prompt directives can be relaxed but not removed. diff --git a/docs/design/agent-specs/canonical-format.md b/docs/design/agent-specs/canonical-format.md index b5bfe1b4e..6f5c4690d 100644 --- a/docs/design/agent-specs/canonical-format.md +++ b/docs/design/agent-specs/canonical-format.md @@ -29,9 +29,9 @@ When changing any format in this file, run all applicable checks before merge: | Structural (static) | Prompt inventory, frontmatter/model tiers, anti-pattern bans, and key policy-text invariants | `cd app && npm run test:agents:structural` | | Unit parser checks | App-side parsing stays compatible with canonical artifacts | `cd app && npm run test:unit` | | Codegen freshness | Generated TypeScript types and JSON Schema match Rust contract structs | `cd app && npm run codegen && git diff --exit-code src/generated/ sidecar/generated/ src-tauri/src/generated/` | -| SDK outputFormat | Inline JSON Schema passed to Agent SDK `outputFormat` for constrained decoding (steps 0-2). Currently non-functional for nested schemas due to SDK bug ([anthropics/claude-agent-sdk-typescript#277](https://github.com/anthropics/claude-agent-sdk-typescript/issues/277)). | Verified by `app/src-tauri/schemas-review/test-sdk-multiturn.mjs` | -| Prompt directives | Agent `.md` files include "CRITICAL — raw JSON only" directives and reference generated JSON schema files at `shared/output-schemas/`. Workaround for SDK bug — model returns valid JSON in `result` text. | Manual / smoke tests | -| Sidecar extraction | When `structured_output` is absent (SDK bug), sidecar parses JSON from `result` text via `tryParseJsonFromText`. Hard-fails only if both `structured_output` and text parsing fail. | `cd app/sidecar && npx vitest run` | +| SDK outputFormat | Inline JSON Schema passed to Agent SDK `outputFormat` for constrained decoding (steps 0-2). `structured_output` is required. | Verified by `app/src-tauri/schemas-review/test-sdk-multiturn.mjs` and `app/sidecar/__tests__/sdk-output-format.integration.test.ts` | +| Prompt directives | Agent `.md` files include "CRITICAL — raw JSON only" directives and reference generated JSON schema files at `shared/output-schemas/`. These reinforce the JSON-only contract but are not the extraction path. | Manual / smoke tests | +| Sidecar extraction | Sidecar uses SDK `structured_output` directly. If `outputFormat` was configured and `structured_output` is absent, sidecar emits `structured_output_missing`; it does not parse JSON from `result` text as fallback. | `cd app/sidecar && npx vitest run` | | Rust serde (final) | Typed deserialization into contract structs (`ResearchStepOutput`, `DetailedResearchOutput`, `DecisionsOutput`). Rejects missing required fields, wrong types, invalid enum values. This is the authoritative validation layer. | `cd app && cargo test --manifest-path src-tauri/Cargo.toml` | | Promptfoo smoke (live) | End-to-end behavior still produces contract-compliant outputs in representative scenarios | `cd app && FORCE_PLUGIN_TESTS=1 npm run test:agents:smoke` | @@ -272,13 +272,12 @@ The research planner now represents the research plan **inside the `research_out The precise field‑level schema for `research_output` is defined in the Rust contract struct `app/src-tauri/src/contracts/clarifications.rs` (canonical type definition) and the agent-facing reference at `agent-sources/plugins/skill-content-researcher/shared/schemas.md`. Generated JSON Schema files are at `agent-sources/plugins/skill-content-researcher/shared/output-schemas/` (inline, no `$ref`) and `output-deep-schemas/` (with `$defs`/`$ref` for readability). Runtime enforcement uses Serde deserialization against the Rust types. If `schemas.md` and this example diverge, treat the Rust contract types as authoritative. -### Structured output extraction flow +## Structured Output Extraction Flow 1. SDK `outputFormat` passes inline JSON Schema for constrained decoding (steps 0-2). 2. If `structured_output` is present on the SDK result → use it directly. -3. If `structured_output` is absent (known SDK bug for nested schemas) → sidecar parses JSON from the `result` text field via `tryParseJsonFromText`. -4. If text parsing also fails → sidecar emits `structured_output_missing` error. -5. Rust deserializes the extracted JSON into typed contract structs — this is the final validation. +3. If `structured_output` is absent on an `outputFormat` run → sidecar emits `structured_output_missing`. +4. Rust deserializes `structured_output` into typed contract structs — this is the final validation. Legacy `research-plan.md` markdown output is no longer part of the app ↔ agent contract; it may still be generated for human‑readable views but must be derived from `research_output.metadata.research_plan`. diff --git a/docs/design/backend-design/agent-event-contracts.md b/docs/design/backend-design/agent-event-contracts.md index 61753e6c1..58fb60dc6 100644 --- a/docs/design/backend-design/agent-event-contracts.md +++ b/docs/design/backend-design/agent-event-contracts.md @@ -172,16 +172,17 @@ This means frontend payload types have the shape When a workflow step completes, the SDK result message may include: -- `structured_output` — parsed JSON object (when SDK constrained decoding works) +- `structured_output` — parsed JSON object from SDK constrained decoding - `result` — text field containing the agent's final output -Due to a known SDK bug ([anthropics/claude-agent-sdk-typescript#277](https://github.com/anthropics/claude-agent-sdk-typescript/issues/277)), `structured_output` is not populated for nested JSON schemas. In this case, the sidecar extracts JSON from the `result` text field via `tryParseJsonFromText`. If both are absent, the sidecar emits `status: "error"` with `errorSubtype: "structured_output_missing"`. +For `outputFormat` runs, the sidecar requires `structured_output`. If it is absent, the sidecar emits `status: "error"` with `errorSubtype: "structured_output_missing"`; it does not parse JSON from `result` text as fallback. Regression coverage for the previously observed nested-schema SDK issue lives in `app/sidecar/__tests__/sdk-output-format.integration.test.ts`. The `result` text field is used when: -- `structured_output` is absent (SDK bug workaround) + +- The run has no structured-output contract - The agent returned an error (non-JSON output) -Rust is the final validator — it deserializes the extracted JSON into typed contract structs. +Rust is the final validator — it deserializes `structured_output` into typed contract structs. --- diff --git a/docs/design/data-contracts/README.md b/docs/design/data-contracts/README.md index c7698f10c..9585fb721 100644 --- a/docs/design/data-contracts/README.md +++ b/docs/design/data-contracts/README.md @@ -28,7 +28,7 @@ No hand-maintained type mirrors. No untyped `serde_json::Value` escape hatches. ## Architecture -``` +```text ┌─────────────────────────────────────────────────┐ │ Rust Contract Structs │ │ app/src-tauri/src/contracts/ │ @@ -77,7 +77,7 @@ Wired into `app/scripts/dev.mjs` before `sidecar:build`. CI freshness check: `np ### Clarifications (`contracts/clarifications.rs`) -``` +```text ClarificationsFile ├── version: String ("1") ├── metadata: ClarificationsMetadata @@ -97,7 +97,7 @@ ClarificationsFile ### Decisions (`contracts/decisions.rs`) -``` +```text DecisionsOutput ├── version: String ├── metadata: DecisionsMetadata @@ -110,7 +110,7 @@ DecisionsOutput ### Agent Events (`contracts/agent_events.rs`) -``` +```text AgentEventEnvelope ├── type: "agent_event" ├── event: AgentEvent (tagged union by "type") @@ -132,15 +132,14 @@ AgentEventEnvelope The SDK's `outputFormat` gets **inline** JSON Schema generated from Rust structs — all `$ref` resolved, `additionalProperties: false` on every object, no `$schema`/`definitions` block. This is required because the SDK silently ignores schemas with `$ref`. -**Known SDK bug** ([anthropics/claude-agent-sdk-typescript#277](https://github.com/anthropics/claude-agent-sdk-typescript/issues/277)): The SDK does not populate `structured_output` for nested schemas. It returns `subtype: "success"` with `structured_output: undefined`. Constrained decoding is not enforced. +The sidecar requires SDK `structured_output` for `outputFormat` runs. If the SDK omits `structured_output`, the sidecar emits `structured_output_missing`; it does not parse JSON from `result` text as a recovery path. -**Workaround (temporary)**: 1. Agent `.md` files include strong prompt directives ("CRITICAL — raw JSON only, no markdown fences") and reference generated JSON schema files at `shared/output-schemas/`. -2. The sidecar tries `structured_output` first; if absent, parses JSON from the `result` text field via `tryParseJsonFromText`. -3. If both are absent, the sidecar emits `structured_output_missing` error. -4. Rust serde deserializes the extracted JSON into typed contract structs — this is the authoritative validation. +2. The sidecar reads `structured_output` from the SDK result. +3. If `structured_output` is absent, the sidecar emits `structured_output_missing`. +4. Rust serde deserializes `structured_output` into typed contract structs — this is the authoritative validation. -When the SDK bug is fixed, `structured_output` will be the primary path. The `result` text fallback remains permanent — it covers error cases and any future scenario where `structured_output` is absent. Prompt directives can be relaxed but not removed, as they reinforce the JSON-only contract. +Regression coverage for the previously observed nested-schema SDK issue ([anthropics/claude-agent-sdk-typescript#277](https://github.com/anthropics/claude-agent-sdk-typescript/issues/277)) lives in `app/sidecar/__tests__/sdk-output-format.integration.test.ts` and `app/src-tauri/schemas-review/test-sdk-multiturn.mjs`. ### Recursive Question type @@ -199,7 +198,7 @@ Semantic validation: `validate_business_rules()` methods on contract structs for | `agent-sources/.../shared/output-schemas/` | Generated inline JSON Schema files (agents Read at runtime) | | `agent-sources/.../shared/output-deep-schemas/` | Deep JSON Schema with `$ref`/`$defs` (human-readable) | | `agent-sources/.../shared/schemas.md` | Semantic rules supplement (what JSON Schema cannot express) | -| `app/sidecar/message-processor.ts` | Structured output extraction + fallback logic | -| `app/sidecar/lib/result-extraction.ts` | `tryParseJsonFromText` — JSON parsing from `result` text | +| `app/sidecar/message-processor.ts` | Structured output extraction + missing-output error handling | +| `app/sidecar/lib/result-extraction.ts` | Display markdown extraction from structured output | | `app/src-tauri/schemas-review/test-sdk-multiturn.mjs` | SDK structured output test script | | `.claude/rules/codegen.md` | Agent rule: run codegen when modifying contracts | diff --git a/docs/design/sdk-agent-options/README.md b/docs/design/sdk-agent-options/README.md index 596dcf4df..8e5487d6e 100644 --- a/docs/design/sdk-agent-options/README.md +++ b/docs/design/sdk-agent-options/README.md @@ -170,8 +170,8 @@ Decision candidate: **keep** (`effort` configurable, `fallbackModel` derived). - Workflow non-contract agents: `skill-creator:generate-skill` (step 3, flat schema) - Refine conversational flow: `rewrite-skill` - Test conversational/text agents: `test-plan-with`, `test-plan-without`, `test-evaluator` -- **Known SDK bug** ([anthropics/claude-agent-sdk-typescript#277](https://github.com/anthropics/claude-agent-sdk-typescript/issues/277)): SDK returns `subtype: "success"` with `structured_output: undefined` for nested schemas. The SDK does not enforce constrained decoding. Workaround: prompt directives force raw JSON output; sidecar parses JSON from `result` text field; Rust serde validates the final structure. -- When the SDK bug is fixed, `structured_output` will be the primary path. The `result` text fallback remains permanent — it covers error cases and any future scenario where `structured_output` is absent. +- `structured_output` is required for `outputFormat` runs. The sidecar does not parse JSON from `result` text as a recovery path; if the SDK omits `structured_output`, the run emits `structured_output_missing`. +- Regression coverage for the previously observed nested-schema SDK issue ([anthropics/claude-agent-sdk-typescript#277](https://github.com/anthropics/claude-agent-sdk-typescript/issues/277)) lives in `app/sidecar/__tests__/sdk-output-format.integration.test.ts`. Decision candidate: **keep selective** (avoid forcing JSON on conversational/text agents). diff --git a/docs/design/workflow-state/README.md b/docs/design/workflow-state/README.md index 39209bfbc..5bb7fe9e7 100644 --- a/docs/design/workflow-state/README.md +++ b/docs/design/workflow-state/README.md @@ -60,7 +60,7 @@ Valid statuses: `pending | in_progress | waiting_for_user | completed | error` | Subtype | Cause | Recovery | |---|---|---| -| `structured_output_missing` | Agent completed (`subtype: "success"`) but neither `structured_output` nor parseable JSON in `result` text was available. Caused by SDK bug + model returning non-JSON text. | Retry the step. | +| `structured_output_missing` | Agent completed an `outputFormat` run but the SDK result did not include `structured_output`. | Retry the step. | | *(general)* | Agent runtime error, timeout, or sidecar failure. | Retry or reset the step. | ---