From 06ae7be38415d29b31ff882e02f12d09847fd1a8 Mon Sep 17 00:00:00 2001 From: Bulat Yapparov Date: Sat, 10 Oct 2026 13:18:02 +0100 Subject: [PATCH 1/3] feat(run): StructuredOutput publishes the validated result file (#140) BREAKING CHANGE: in --output-schema mode the StructuredOutput tool takes only {"path": ""}. The model writes its final result as JSON to a file in the working directory and passes the path; result objects passed as tool arguments are rejected. The CLI resolves the path with realpath, keeps it inside the working directory, caps it at 2 MiB, reads it through one handle, validates it with the shared Ajv validator and publishes the parsed file. Only {path} reaches the provider, so provider schema conversion cannot drop result fields (Gemini emptied map-typed fields, #139), and the model no longer re-emits the whole result. File rejections are counted corrective turns that ask the model to fix the file at that path. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019gASjVHuvN9AxK5WT8jzsN --- CHANGELOG.md | 4 + EVENTS.md | 18 +- README.md | 2 + packages/cli/README.md | 19 ++ packages/cli/src/cli/cmd/run.ts | 6 +- packages/cli/src/session/output-schema.ts | 54 ++++ packages/cli/src/session/prompt.ts | 50 ++- .../cli/test/cli/run-output-schema.test.ts | 11 +- packages/cli/test/fixture/output-schema.ts | 10 + .../cli/test/session/output-schema.test.ts | 45 +-- .../cli/test/session/prompt-tool-loop.test.ts | 3 +- .../test/session/structured-output.test.ts | 299 ++++++++---------- 12 files changed, 305 insertions(+), 216 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e515e9..24826b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Breaking changes + +- **StructuredOutput publishes the validated result file** — In `--output-schema` mode the `StructuredOutput` tool now takes only `{ "path": "" }`. The model writes its final result as JSON to a file in the working directory and passes the path; the CLI follows symlinks, keeps the file inside the working directory, caps it at 2 MiB, validates it with Ajv and publishes the parsed file. Passing the result object as tool arguments is no longer accepted. Callers whose prompts say "call StructuredOutput with the result" must say to write a file and pass its path. Only `{ path }` reaches the provider, so provider schema conversion can no longer drop result fields (Gemini emptied map-typed fields, #139), and the model no longer re-emits the whole result as output tokens. Rejected files are counted corrective turns that ask the model to fix the file. (#140) + ### Fixes - **Provider error finishes fail headless execution** — Normal streams ending in `error` or `content-filter` now persist a structured provider failure, emit consistent failure events, and exit nonzero while retaining partial output and usage. Empty successful responses remain successful. (#108) diff --git a/EVENTS.md b/EVENTS.md index 99563e8..0cd914e 100644 --- a/EVENTS.md +++ b/EVENTS.md @@ -541,6 +541,9 @@ does not coerce types, insert defaults or remove extra properties. Serialized schemas are limited to 64 KiB, depth 64 and 10,000 nested objects. The schema file is trusted instruction input: annotations (`description`, `title`, `examples`) are shown to the model in the StructuredOutput tool description. +StructuredOutput takes only `{ "path": "" }`: the model writes the result as +JSON to a file inside the working directory and passes its path. The CLI publishes +the parsed file contents (#140). `--output-schema-retries ` permits N additional corrective turns (0–10, default 2). Zero permits only the initial turn; N permits the initial turn and at most N @@ -557,8 +560,8 @@ value in the terminal event below. ### `structured_output_rejected` -Emitted once per rejected StructuredOutput attempt, including unparseable JSON -and a model turn that finishes in prose without a final tool call: +Emitted once per rejected StructuredOutput attempt, including an unusable result +file and a model turn that finishes in prose without a final tool call: ```json { @@ -577,13 +580,18 @@ is under 2 KB and never includes submitted values or raw JSON arguments. A `path can contain property names submitted by the model (for example, in map or `patternProperties` schemas), never their values. Each path segment is clipped to 64 characters; paths also have an overall length bound. -Validation feedback goes to the model in the same session through the `invalid` tool. +File diagnostics use keyword `file` (`file not found`, `path must stay inside the +working directory`, `path must be a regular file`, a size over 2 MiB) or `parse` +(`file must contain complete, valid JSON`); schema diagnostics use Ajv keywords. +Feedback for a rejected file goes to the model as the StructuredOutput tool error and +asks it to fix the file at that path. Malformed tool arguments (not a `{ path }` +object) go to the model through the `invalid` tool. A prose-only finish emits the fixed diagnostic `{ "path": "", "keyword": "missing", "message": "call StructuredOutput with the final result" }` and receives an ephemeral reminder before the next turn while budget remains. Other tools remain usable until a valid final result; their outputs are not final -results. Provider-compatible tool schemas may be transformed, while local -validation always uses the canonical schema. +results. Only the `{ path }` tool schema is sent to the provider; local validation +of the file always uses the canonical schema. ### `structured_output` diff --git a/README.md b/README.md index e90d8f6..cc746d9 100644 --- a/README.md +++ b/README.md @@ -54,6 +54,8 @@ aictrl run --format json "review this PR" | jq '.type' `aictrl run --output-schema ` validates the final JSON result. Use `--output-schema-retries ` for additional corrective turns (0–10, default 2) and `--output-result ` to atomically write the accepted value. +The agent writes the result to a JSON file in the working directory and calls +`StructuredOutput` with its path; the CLI validates and publishes that file. Zero retries permits only the initial turn; N permits the initial turn and at most N corrective turns. Serialized schemas are limited to 64 KiB, depth 64 and 10,000 nested objects. diff --git a/packages/cli/README.md b/packages/cli/README.md index 812e2c4..29275ae 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -55,6 +55,25 @@ aictrl run --output-schema result.schema.json --output-schema-retries 2 \ Relative `--output-schema` and `--output-result` paths resolve against `--dir` when it is given (as `--file` paths do); configuration errors print the resolved path. +**StructuredOutput contract (#140).** The model writes its final result as JSON +to a file in the working directory (`--dir`, or the attached server's session +directory), then calls `StructuredOutput` with +`{ "path": "" }`. The tool takes only that argument; the canonical schema is +text in the tool description, so provider schema conversion cannot drop result +fields. The CLI reads the file and publishes its parsed contents, not a re-emitted +copy. The file must: + +- resolve inside the working directory after following symlinks (relative paths + resolve against it; absolute paths inside it are accepted); +- be a regular file of at most 2 MiB; +- contain complete JSON that the schema accepts. + +A file that fails any of these is a counted rejection. The model is told to fix the +file at that path and call `StructuredOutput` again with its path. Calling +`StructuredOutput` with the result object as arguments is no longer supported. +Callers whose prompts say to pass the result as tool arguments must say to write a +file and pass its path instead. + Ajv strict mode rejects unknown schema keywords and unregistered `format` values such as `date-time` and `uri` as configuration errors (exit **2**). The result parent directory must exist and be writable at configuration time. diff --git a/packages/cli/src/cli/cmd/run.ts b/packages/cli/src/cli/cmd/run.ts index 3296655..8f68ee4 100644 --- a/packages/cli/src/cli/cmd/run.ts +++ b/packages/cli/src/cli/cmd/run.ts @@ -349,7 +349,11 @@ export const RunCommand = cmd({ default: "default", describe: "format: default (formatted) or json (raw JSON events)", }) - .option("output-schema", { type: "string", describe: "JSON Schema file for the final result (object root)" }) + .option("output-schema", { + type: "string", + describe: + "JSON Schema file for the final result (object root); the agent writes the result to a JSON file in the working directory and passes its path to StructuredOutput", + }) .option("output-schema-retries", { type: "number", describe: "additional corrective turns (0-10, default: 2)" }) .option("output-result", { type: "string", diff --git a/packages/cli/src/session/output-schema.ts b/packages/cli/src/session/output-schema.ts index 0a6c05b..c290e47 100644 --- a/packages/cli/src/session/output-schema.ts +++ b/packages/cli/src/session/output-schema.ts @@ -2,6 +2,8 @@ import Ajv, { type ErrorObject } from "ajv" import Ajv2020 from "ajv/dist/2020" import z from "zod" import { createHash } from "crypto" +import path from "path" +import fs from "fs/promises" export namespace OutputSchema { export const Diagnostic = z.object({ path: z.string(), keyword: z.string(), message: z.string() }) @@ -88,6 +90,58 @@ export namespace OutputSchema { } } + // Path form (#140): the model writes its result to a file and passes the path, so the + // published value is the file it validated, never a provider-schema-constrained re-emission. + export const PATH_SCHEMA: Record = { + type: "object", + properties: { + path: { + type: "string", + maxLength: 1024, + description: "Path of the JSON result file, relative to the working directory", + }, + }, + required: ["path"], + additionalProperties: false, + } + const MAX_FILE_BYTES = 2 * 1024 * 1024 + + export async function file( + root: string, + input: string, + validate: ReturnType, + ): Promise<{ value: unknown } | { errors: Diagnostic[] }> { + const fail = (keyword: string, message: string) => ({ errors: [{ path: "", keyword, message }] }) + const base = await fs.realpath(root) + // realpath follows symlinks, so a link pointing outside the root is rejected too. + const target = await fs.realpath(path.resolve(base, input)).catch(() => undefined) + if (!target) return fail("file", "file not found") + const relative = path.relative(base, target) + if (relative.startsWith("..") || path.isAbsolute(relative)) + return fail("file", "path must stay inside the working directory") + // One handle for stat and read, so the checked file is the file read. + const handle = await fs.open(target, "r").catch(() => undefined) + if (!handle) return fail("file", "file not readable") + try { + const stat = await handle.stat() + if (!stat.isFile()) return fail("file", "path must be a regular file") + if (stat.size > MAX_FILE_BYTES) return fail("file", `file must not exceed ${MAX_FILE_BYTES} bytes`) + const text = await handle.readFile("utf8") + const value = (() => { + try { + return { parsed: JSON.parse(text) as unknown } + } catch { + return undefined + } + })() + if (!value) return fail("parse", "file must contain complete, valid JSON") + if (!validate(value.parsed)) return { errors: diagnostics(validate.errors) } + return { value: value.parsed } + } finally { + await handle.close() + } + } + export function summary(error: unknown) { const value = error && typeof error === "object" ? error : {} return { diff --git a/packages/cli/src/session/prompt.ts b/packages/cli/src/session/prompt.ts index 9e8bb72..35a9934 100644 --- a/packages/cli/src/session/prompt.ts +++ b/packages/cli/src/session/prompt.ts @@ -50,15 +50,15 @@ import { Truncate } from "@/tool/truncation" // @ts-ignore globalThis.AI_SDK_LOG_WARNINGS = false -const STRUCTURED_OUTPUT_DESCRIPTION = `Use this tool to return your final response in the requested structured format. +const STRUCTURED_OUTPUT_DESCRIPTION = `Use this tool to submit your final response as a JSON file. IMPORTANT: -- Call this tool at the end of your response; repair rejected arguments within the allowed budget -- The input must be valid JSON matching the required schema -- Complete all necessary research and tool calls BEFORE calling this tool -- This tool provides your final answer - no further actions are taken after calling it` +- First write the final result as JSON to a file in the working directory, matching the schema below +- Then call this tool with that file's path; the file is validated and published as written +- If the file is rejected, fix the file and call this tool again with its path +- Complete all necessary research and tool calls BEFORE calling this tool` -const STRUCTURED_OUTPUT_SYSTEM_PROMPT = `IMPORTANT: The user has requested structured output. You MUST use the StructuredOutput tool to provide your final response. Do NOT respond with plain text - you MUST call the StructuredOutput tool with your answer formatted according to the schema.` +const STRUCTURED_OUTPUT_SYSTEM_PROMPT = `IMPORTANT: The user has requested structured output. Write your final result as JSON to a file in the working directory, then call the StructuredOutput tool with that file's path. Do NOT respond with plain text.` export namespace SessionPrompt { const log = Log.create({ service: "session.prompt" }) @@ -738,7 +738,8 @@ export namespace SessionPrompt { const reject = (raw: string) => { if (!contract) throw new Error("Missing output schema") - return rejection(OutputSchema.parse(raw, contract.validate)) + // Only the {path} arguments reach the SDK boundary; file content errors come from execute. + return rejection(OutputSchema.parse(raw, OutputSchema.compile(OutputSchema.PATH_SCHEMA))) } const available = !!contract && attempts < contract.format.retryCount + 1 @@ -748,6 +749,8 @@ export namespace SessionPrompt { schema: format.schema, validate: contract.validate, model, + root: Instance.directory, + onReject: rejection, async onSuccess(output) { if (structuredOutput !== undefined) return if (!available) throw new Error("StructuredOutput corrective turn budget exhausted") @@ -806,7 +809,8 @@ export namespace SessionPrompt { ? [ { role: "user" as const, - content: "call StructuredOutput with the final result", + content: + "write the final result as JSON to a file in the working directory, then call StructuredOutput with its path", }, ] : []), @@ -1121,6 +1125,9 @@ export namespace SessionPrompt { schema: Record validate?: ReturnType model?: Provider.Model + /** The tool takes {path} to the JSON result file, which must stay inside this directory (#140). */ + root: string + onReject?: (errors: OutputSchema.Diagnostic[]) => Promise onSuccess: (output: unknown) => void | Promise }): AITool & { id: "StructuredOutput" } { OutputSchema.canonical(input.schema) @@ -1131,18 +1138,29 @@ export namespace SessionPrompt { canonical.bytes <= 8 * 1024 ? `Canonical JSON Schema: ${canonical.text}` : `Canonical JSON Schema (truncated; the validator enforces the full schema): ${new TextDecoder().decode(Buffer.from(canonical.text).subarray(0, 8 * 1024))}` + const shape = OutputSchema.compile(OutputSchema.PATH_SCHEMA) const result = tool({ description: `${STRUCTURED_OUTPUT_DESCRIPTION}\n\n${description}`, - inputSchema: jsonSchema(input.model ? ProviderTransform.schema(input.model, schema) : schema, { - validate(value) { - return validate(value) - ? { success: true, value } - : { success: false, error: OutputSchema.error(OutputSchema.diagnostics(validate.errors)) } + // Only {path} reaches the provider, so provider schema conversion cannot drop result fields. + inputSchema: jsonSchema<{ path: string }>( + input.model ? ProviderTransform.schema(input.model, OutputSchema.PATH_SCHEMA) : OutputSchema.PATH_SCHEMA, + { + validate(value) { + return shape(value) + ? { success: true, value: value as { path: string } } + : { success: false, error: OutputSchema.error(OutputSchema.diagnostics(shape.errors)) } + }, }, - }), + ), async execute(args) { - if (!validate(args)) throw OutputSchema.error(OutputSchema.diagnostics(validate.errors)) - await input.onSuccess(args) + const loaded = await OutputSchema.file(input.root, args.path, validate) + if ("errors" in loaded) { + const message = input.onReject + ? await input.onReject(loaded.errors) + : OutputSchema.error(loaded.errors).message + throw new Error(`Fix the file at ${args.path}, then call StructuredOutput again with its path. ${message}`) + } + await input.onSuccess(loaded.value) return { output: "Structured output captured successfully.", title: "Structured Output", diff --git a/packages/cli/test/cli/run-output-schema.test.ts b/packages/cli/test/cli/run-output-schema.test.ts index 7882c4c..a7f3f25 100644 --- a/packages/cli/test/cli/run-output-schema.test.ts +++ b/packages/cli/test/cli/run-output-schema.test.ts @@ -2,7 +2,7 @@ import { expect, test } from "bun:test" import path from "path" import fs from "fs/promises" import { tmpdir } from "../fixture/fixture" -import { schema } from "../fixture/output-schema" +import { schema, pathArgs } from "../fixture/output-schema" import { outputResult } from "../../src/cli/cmd/run.output" const entry = path.resolve(import.meta.dir, "../../src/index.ts") @@ -73,7 +73,10 @@ async function run( type: "function", function: options.denied ? { name: "read", arguments: JSON.stringify({ filePath: "/outside-fixture.txt" }) } - : { name: "StructuredOutput", arguments: args }, + : { + name: "StructuredOutput", + arguments: pathArgs(tmp.path, args, `so_${requests.length}.json`), + }, }, ], }, @@ -381,7 +384,9 @@ for (const json of [true, false]) { "required", { type: "function", function: { name: "StructuredOutput" } }, ]) - expect(JSON.stringify(result.requests[1].messages)).toContain("call StructuredOutput with the final result") + expect(JSON.stringify(result.requests[1].messages)).toContain( + "write the final result as JSON to a file in the working directory, then call StructuredOutput with its path", + ) expect(result.result).toBe(JSON.stringify({ result: "accepted" }, null, 2) + "\n") if (!json) return expect(result.events.filter((event) => event.type === "structured_output_rejected")).toMatchObject([ diff --git a/packages/cli/test/fixture/output-schema.ts b/packages/cli/test/fixture/output-schema.ts index b78610c..5ba71d0 100644 --- a/packages/cli/test/fixture/output-schema.ts +++ b/packages/cli/test/fixture/output-schema.ts @@ -1,3 +1,6 @@ +import fs from "fs" +import path from "path" + export const schema = { type: "object", properties: { @@ -27,3 +30,10 @@ export const corpus = [ [], "value", ] + +// StructuredOutput takes the path of a result file (#140): fixture providers write the +// result text to a file in the run directory and call the tool with its path. +export function pathArgs(directory: string, args: string, name: string) { + fs.writeFileSync(path.join(directory, name), args) + return JSON.stringify({ path: name }) +} diff --git a/packages/cli/test/session/output-schema.test.ts b/packages/cli/test/session/output-schema.test.ts index b010a59..add10e9 100644 --- a/packages/cli/test/session/output-schema.test.ts +++ b/packages/cli/test/session/output-schema.test.ts @@ -4,6 +4,7 @@ import { LLM } from "../../src/session/llm" import { PermissionNext } from "../../src/permission/next" import Ajv from "ajv" import Ajv2020 from "ajv/dist/2020" +import path from "path" import { Instance } from "../../src/project/instance" import { Session } from "../../src/session" import { SessionPrompt } from "../../src/session/prompt" @@ -11,7 +12,7 @@ import { Bus } from "../../src/bus" import { MessageV2 } from "../../src/session/message-v2" import { OutputSchema } from "../../src/session/output-schema" import { tmpdir } from "../fixture/fixture" -import { schema, corpus } from "../fixture/output-schema" +import { schema, corpus, pathArgs } from "../fixture/output-schema" async function run( inputs: string[], @@ -86,7 +87,9 @@ async function run( index, id: `call_${requests.length}_${index}`, type: "function", - function: { name: options.invalid ? "invalid" : "StructuredOutput", arguments: args }, + function: options.invalid + ? { name: "invalid", arguments: args } + : { name: "StructuredOutput", arguments: pathArgs(directory, args, `so_${requests.length}_${index}.json`) }, })) if ((options.queued || options.denied) && requests.length === 1) { calls[0].function = { @@ -331,7 +334,7 @@ test("truncated arguments are a counted rejected attempt", async () => { sessionID: result.info.sessionID, attempt: 1, maxAttempts: 3, - errors: [{ path: "", keyword: "parse", message: "arguments must be complete, valid JSON" }], + errors: [{ path: "", keyword: "parse", message: "file must contain complete, valid JSON" }], }, ]) }, 15_000) @@ -363,13 +366,8 @@ test("repair diagnostics sent to model are bounded and never echo submitted valu .map((message) => JSON.stringify(message.content)) .join("") expect(Buffer.byteLength(feedback)).toBeLessThan(2048) - const calls = (request.messages as { tool_calls?: { function: { name: string; arguments: string } }[] }[]) - .flatMap((message) => message.tool_calls ?? []) - .filter((call) => call.function.name === "invalid") - expect(calls.length).toBeGreaterThan(0) - calls.forEach((call) => { - expect(Buffer.byteLength(call.function.arguments)).toBeLessThan(2048) - }) + // Path form: file content errors return as the StructuredOutput tool error, not an `invalid` repair call. + expect(feedback).toContain("Fix the file at so_1_0.json") expect(JSON.stringify(request)).not.toContain("submitted-secret-") expect(feedback).toContain("additionalProperties") expect(result.info.structured).toEqual({ result: "repaired" }) @@ -448,15 +446,20 @@ test("draft 2020-12 uses Ajv2020 with default strict validation", () => { for (const value of corpus) expect(validate(value)).toBe(reference(value)) }) -test("execute defence in depth rejects invalid input before capture", async () => { +test("execute defence in depth rejects an invalid result file before capture", async () => { + await using tmp = await tmpdir() + await Bun.write(path.join(tmp.path, "result.json"), "{}") const captured: unknown[] = [] const tool = SessionPrompt.createStructuredOutputTool({ schema, + root: tmp.path, onSuccess: (value) => { captured.push(value) }, }) - await expect(tool.execute!({}, { toolCallId: "fixture", messages: [] })).rejects.toThrow("StructuredOutput rejected") + await expect(tool.execute!({ path: "result.json" }, { toolCallId: "fixture", messages: [] })).rejects.toThrow( + "StructuredOutput rejected", + ) expect(captured).toEqual([]) }) @@ -482,9 +485,10 @@ test("Gemini tool schema is transformed but canonical numeric enum remains autho expect(result.info.structured).toEqual({ result: 1 }) expect(result.rejected).toHaveLength(1) const tools = result.requests[0].tools as { function: { name: string; parameters: unknown } }[] - expect(tools.find((tool) => tool.function.name === "StructuredOutput")?.function.parameters).toMatchObject({ - properties: { result: { type: "string", enum: ["1", "2"] } }, - }) + // Path form: only {path} reaches the provider; the canonical schema is enforced on the file. + const parameters = tools.find((tool) => tool.function.name === "StructuredOutput")?.function.parameters + expect(parameters).toMatchObject({ properties: { path: { type: "string" } }, required: ["path"] }) + expect(Object.keys((parameters as { properties: object }).properties)).toEqual(["path"]) expect(canonical.properties.result).toEqual({ type: "integer", enum: [1, 2] }) }, 15_000) @@ -503,7 +507,9 @@ test("prose-only finish is a counted corrective attempt before a valid result", errors: [{ path: "", keyword: "missing", message: "call StructuredOutput with the final result" }], }, ]) - expect(JSON.stringify(result.requests[1].messages)).toContain("call StructuredOutput with the final result") + expect(JSON.stringify(result.requests[1].messages)).toContain( + "write the final result as JSON to a file in the working directory, then call StructuredOutput with its path", + ) expect(result.outcomes).toEqual([{ status: "accepted", attempts: 2, value: { result: "repaired" } }]) }, 15_000) @@ -585,7 +591,9 @@ for (const parallel of [ expect(result.info.error).toBeUndefined() expect(result.info.structured).toEqual({ result: "accepted" }) expect(result.outcomes).toEqual([{ status: "accepted", attempts: 2, value: { result: "accepted" } }]) - expect(result.rejected).toMatchObject([{ attempt: parallel[0] === "{}" ? 1 : 2, maxAttempts: 1 }]) + // Path form: both calls execute concurrently, so the rejected attempt number follows file I/O order. + expect(result.rejected).toMatchObject([{ maxAttempts: 1 }]) + expect([1, 2]).toContain(result.rejected[0].attempt) }, 15000) } @@ -700,7 +708,7 @@ for (const [name, description] of [ ]) { test(`tool description caps ${name} schema at 8 KiB and explains truncation`, () => { const canonical = { type: "object", description } - const tool = SessionPrompt.createStructuredOutputTool({ schema: canonical, onSuccess() {} }) + const tool = SessionPrompt.createStructuredOutputTool({ schema: canonical, root: import.meta.dir, onSuccess() {} }) const text = JSON.stringify(canonical) const expected = new TextDecoder().decode(Buffer.from(text).subarray(0, 8 * 1024)) expect(tool.description).toContain("truncated; the validator enforces the full schema") @@ -723,6 +731,7 @@ for (const [name, schema, reason] of [ SessionPrompt.createStructuredOutputTool({ schema, validate: OutputSchema.compile({ type: "object" }), + root: import.meta.dir, onSuccess() {}, }), ).toThrow(reason) diff --git a/packages/cli/test/session/prompt-tool-loop.test.ts b/packages/cli/test/session/prompt-tool-loop.test.ts index 58a10d7..0d2640b 100644 --- a/packages/cli/test/session/prompt-tool-loop.test.ts +++ b/packages/cli/test/session/prompt-tool-loop.test.ts @@ -145,7 +145,7 @@ describe("session prompt tool-call continuation", () => { async fetch(request) { requests.push((await request.json()) as Record) const call = requests.length === 1 ? "read" : "StructuredOutput" - const args = requests.length === 1 ? { filePath: "aictrl.json" } : { result: "follow-up reached" } + const args = requests.length === 1 ? { filePath: "aictrl.json" } : { path: "result.json" } return stream([ { id: `chatcmpl-${requests.length}`, @@ -185,6 +185,7 @@ describe("session prompt tool-call continuation", () => { try { await using tmp = await tmpdir({ init: async (dir) => { + await Bun.write(path.join(dir, "result.json"), JSON.stringify({ result: "follow-up reached" })) await Bun.write( path.join(dir, "aictrl.json"), JSON.stringify({ diff --git a/packages/cli/test/session/structured-output.test.ts b/packages/cli/test/session/structured-output.test.ts index de8f5cb..82fd503 100644 --- a/packages/cli/test/session/structured-output.test.ts +++ b/packages/cli/test/session/structured-output.test.ts @@ -2,6 +2,9 @@ import { describe, expect, test } from "bun:test" import { asSchema } from "ai" import { MessageV2 } from "../../src/session/message-v2" import { SessionPrompt } from "../../src/session/prompt" +import fs from "fs/promises" +import path from "path" +import { tmpdir } from "../fixture/fixture" describe("structured-output.OutputFormat", () => { test("parses text format", () => { @@ -157,200 +160,152 @@ describe("structured-output.AssistantMessage", () => { }) describe("structured-output.createStructuredOutputTool", () => { - test("creates tool with correct id", () => { - const tool = SessionPrompt.createStructuredOutputTool({ - schema: { type: "object", properties: { name: { type: "string" } } }, - onSuccess: () => {}, + const schema = { + type: "object", + properties: { elements: { type: "object", additionalProperties: { type: "string" }, minProperties: 1 } }, + required: ["elements"], + } + const options = { toolCallId: "call", messages: [], abortSignal: undefined as any } + const create = (root: string, input: Partial[0]> = {}) => + SessionPrompt.createStructuredOutputTool({ schema, root, onSuccess: () => {}, ...input }) + // Rejections report diagnostics through onReject, as the prompt loop does. + const rejecting = (root: string, rejected: unknown[], input: { schema?: Record } = {}) => + create(root, { + ...input, + onReject: async (errors) => { + rejected.push(errors) + return `StructuredOutput rejected: ${JSON.stringify(errors)}` + }, + onSuccess: () => { + throw new Error("rejected file captured") + }, }) + test("creates tool with correct id", async () => { + await using tmp = await tmpdir() // AI SDK tool type doesn't expose id, but we set it internally - expect((tool as any).id).toBe("StructuredOutput") + expect((create(tmp.path) as any).id).toBe("StructuredOutput") }) - test("creates tool with description", () => { - const tool = SessionPrompt.createStructuredOutputTool({ - schema: { type: "object" }, - onSuccess: () => {}, - }) - - expect(tool.description).toContain("structured format") + test("only {path} is the tool input schema; the canonical schema is description text", async () => { + await using tmp = await tmpdir() + const tool = create(tmp.path, { schema: { $schema: "http://json-schema.org/draft-07/schema#", ...schema } }) + const input = (tool.inputSchema as any).jsonSchema + expect(Object.keys(input.properties)).toEqual(["path"]) + expect(input.required).toEqual(["path"]) + expect(input.additionalProperties).toBe(false) + expect(tool.description).toContain("write the final result as JSON to a file") + expect(tool.description).toContain(`Canonical JSON Schema: ${JSON.stringify(schema)}`) + expect(tool.description).not.toContain("$schema") }) - test("creates tool with schema as inputSchema", () => { - const schema = { - type: "object", - properties: { - company: { type: "string" }, - founded: { type: "number" }, - }, - required: ["company"], - } - - const tool = SessionPrompt.createStructuredOutputTool({ - schema, - onSuccess: () => {}, - }) - - // AI SDK wraps schema in { jsonSchema: {...} } - expect(tool.inputSchema).toBeDefined() - const inputSchema = tool.inputSchema as any - expect(inputSchema.jsonSchema?.properties?.company).toBeDefined() - expect(inputSchema.jsonSchema?.properties?.founded).toBeDefined() + test("AI SDK boundary accepts only a bounded {path}", async () => { + await using tmp = await tmpdir() + const validate = asSchema(create(tmp.path).inputSchema).validate! + expect(await validate({ path: "result.json" })).toMatchObject({ success: true }) + for (const value of [{}, { path: 1 }, { path: "result.json", elements: {} }, { path: "a".repeat(1025) }]) + expect(await validate(value)).toMatchObject({ success: false }) }) - test("strips $schema property from inputSchema", () => { - const schema = { - $schema: "http://json-schema.org/draft-07/schema#", - type: "object", - properties: { name: { type: "string" } }, + test("publishes the validated file contents", async () => { + await using tmp = await tmpdir() + const value = { elements: { "queued->running": "dispatch", "TASKS.org_id": "fk", "1": "first" } } + await Bun.write(path.join(tmp.path, "out", "result.json"), JSON.stringify(value)) + const captured: unknown[] = [] + const tool = create(tmp.path, { onSuccess: (output) => void captured.push(output) }) + for (const file of ["out/result.json", path.join(tmp.path, "out", "result.json")]) { + const result = await tool.execute!({ path: file }, options) + expect(result.output).toBe("Structured output captured successfully.") + expect(result.metadata.valid).toBe(true) } - - const tool = SessionPrompt.createStructuredOutputTool({ - schema, - onSuccess: () => {}, - }) - - // AI SDK wraps schema in { jsonSchema: {...} } - const inputSchema = tool.inputSchema as any - expect(inputSchema.jsonSchema?.$schema).toBeUndefined() - expect(tool.description).toContain(`Canonical JSON Schema: ${JSON.stringify(inputSchema.jsonSchema)}`) - expect(tool.description).not.toContain("$schema") - }) - - test("execute calls onSuccess with valid args", async () => { - let capturedOutput: unknown - - const tool = SessionPrompt.createStructuredOutputTool({ - schema: { type: "object", properties: { name: { type: "string" } } }, - onSuccess: (output) => { - capturedOutput = output - }, - }) - - expect(tool.execute).toBeDefined() - const testArgs = { name: "Test Company" } - const result = await tool.execute!(testArgs, { - toolCallId: "test-call-id", - messages: [], - abortSignal: undefined as any, - }) - - expect(capturedOutput).toEqual(testArgs) - expect(result.output).toBe("Structured output captured successfully.") - expect(result.metadata.valid).toBe(true) + expect(captured).toEqual([value, value]) }) - test("AI SDK boundary rejects missing required fields", async () => { - const tool = SessionPrompt.createStructuredOutputTool({ - schema: { - type: "object", - properties: { name: { type: "string" }, age: { type: "number" } }, - required: ["name", "age"], - }, - onSuccess: () => { - throw new Error("Invalid input captured") - }, - }) - expect(await asSchema(tool.inputSchema).validate!({})).toMatchObject({ success: false }) + test("rejects paths that escape the working directory, directly or through a symlink", async () => { + await using outside = await tmpdir() + await using tmp = await tmpdir() + await Bun.write(path.join(outside.path, "result.json"), JSON.stringify({ elements: { a: "b" } })) + await fs.symlink(path.join(outside.path, "result.json"), path.join(tmp.path, "link.json")) + const rejected: unknown[] = [] + const tool = rejecting(tmp.path, rejected) + for (const file of [ + path.join(outside.path, "result.json"), + "../" + path.basename(outside.path) + "/result.json", + "link.json", + ]) + await expect(tool.execute!({ path: file }, options)).rejects.toThrow("Fix the file at") + expect(rejected).toEqual( + Array(3).fill([{ path: "", keyword: "file", message: "path must stay inside the working directory" }]), + ) }) - test("AI SDK boundary rejects wrong scalar types", async () => { - const tool = SessionPrompt.createStructuredOutputTool({ - schema: { type: "object", properties: { count: { type: "number" } }, required: ["count"] }, - onSuccess: () => { - throw new Error("Invalid input captured") - }, - }) - expect(await asSchema(tool.inputSchema).validate!({ count: "wrong" })).toMatchObject({ success: false }) + test("rejects missing, non-regular, oversized and unparseable files", async () => { + await using tmp = await tmpdir() + await fs.mkdir(path.join(tmp.path, "dir")) + await Bun.write(path.join(tmp.path, "big.json"), JSON.stringify({ elements: { a: "x".repeat(2 * 1024 * 1024) } })) + await Bun.write(path.join(tmp.path, "truncated.json"), '{"elements": {"a": "submitted-secret-') + const rejected: unknown[] = [] + const tool = rejecting(tmp.path, rejected) + for (const file of ["missing.json", "dir", "big.json", "truncated.json"]) + await expect(tool.execute!({ path: file }, options)).rejects.toThrow(`Fix the file at ${file}`) + expect(rejected).toEqual([ + [{ path: "", keyword: "file", message: "file not found" }], + [{ path: "", keyword: "file", message: "path must be a regular file" }], + [{ path: "", keyword: "file", message: `file must not exceed ${2 * 1024 * 1024} bytes` }], + // JSON parser errors can echo the file contents, so the diagnostic is fixed. + [{ path: "", keyword: "parse", message: "file must contain complete, valid JSON" }], + ]) }) - test("execute handles nested objects", async () => { - let capturedOutput: unknown - - const tool = SessionPrompt.createStructuredOutputTool({ - schema: { - type: "object", - properties: { - user: { - type: "object", - properties: { - name: { type: "string" }, - email: { type: "string" }, - }, - required: ["name"], - }, - }, - required: ["user"], - }, - onSuccess: (output) => { - capturedOutput = output - }, - }) - - // Valid nested object - AI SDK validates before calling execute() - const validResult = await tool.execute!( - { user: { name: "John", email: "john@test.com" } }, - { - toolCallId: "test-call-id", - messages: [], - abortSignal: undefined as any, - }, + test("invalid file content is a counted rejection with a fix-the-file corrective message", async () => { + await using tmp = await tmpdir() + await Bun.write(path.join(tmp.path, "result.json"), JSON.stringify({ elements: {} })) + const rejected: unknown[] = [] + const error = await rejecting(tmp.path, rejected).execute!({ path: "result.json" }, options).catch( + (error: Error) => error, ) - - expect(capturedOutput).toEqual({ user: { name: "John", email: "john@test.com" } }) - expect(validResult.metadata.valid).toBe(true) - - // Verify schema has correct nested structure - const inputSchema = tool.inputSchema as any - expect(inputSchema.jsonSchema?.properties?.user?.type).toBe("object") - expect(inputSchema.jsonSchema?.properties?.user?.properties?.name?.type).toBe("string") - expect(inputSchema.jsonSchema?.properties?.user?.required).toContain("name") + expect((error as Error).message).toBe( + 'Fix the file at result.json, then call StructuredOutput again with its path. StructuredOutput rejected: [{"path":"/elements","keyword":"minProperties","message":"must NOT have fewer than 1 properties"}]', + ) + expect(rejected).toHaveLength(1) }) - test("execute handles arrays", async () => { - let capturedOutput: unknown - - const tool = SessionPrompt.createStructuredOutputTool({ - schema: { - type: "object", - properties: { - tags: { - type: "array", - items: { type: "string" }, - }, - }, - required: ["tags"], - }, - onSuccess: (output) => { - capturedOutput = output - }, - }) - - // Valid array - AI SDK validates before calling execute() - const validResult = await tool.execute!( - { tags: ["a", "b", "c"] }, - { - toolCallId: "test-call-id", - messages: [], - abortSignal: undefined as any, + test("file content is validated against nested objects, arrays and scalar types", async () => { + await using tmp = await tmpdir() + const nested = { + type: "object", + properties: { + user: { type: "object", properties: { name: { type: "string" } }, required: ["name"] }, + tags: { type: "array", items: { type: "string" } }, + count: { type: "number" }, }, + required: ["user", "tags", "count"], + } + const value = { user: { name: "John" }, tags: ["a", "b"], count: 1 } + await Bun.write(path.join(tmp.path, "valid.json"), JSON.stringify(value)) + await Bun.write(path.join(tmp.path, "invalid.json"), JSON.stringify({ user: {}, tags: [1], count: "1" })) + const captured: unknown[] = [] + await create(tmp.path, { schema: nested, onSuccess: (output) => void captured.push(output) }).execute!( + { path: "valid.json" }, + options, ) - - expect(capturedOutput).toEqual({ tags: ["a", "b", "c"] }) - expect(validResult.metadata.valid).toBe(true) - - // Verify schema has correct array structure - const inputSchema = tool.inputSchema as any - expect(inputSchema.jsonSchema?.properties?.tags?.type).toBe("array") - expect(inputSchema.jsonSchema?.properties?.tags?.items?.type).toBe("string") + expect(captured).toEqual([value]) + const rejected: unknown[] = [] + await expect( + rejecting(tmp.path, rejected, { schema: nested }).execute!({ path: "invalid.json" }, options), + ).rejects.toThrow("Fix the file at invalid.json") + expect(rejected).toEqual([ + [ + { path: "/user", keyword: "required", message: "must have required property 'name'" }, + { path: "/tags/0", keyword: "type", message: "must be string" }, + { path: "/count", keyword: "type", message: "must be number" }, + ], + ]) }) - test("toModelOutput returns text value", () => { - const tool = SessionPrompt.createStructuredOutputTool({ - schema: { type: "object" }, - onSuccess: () => {}, - }) - + test("toModelOutput returns text value", async () => { + await using tmp = await tmpdir() + const tool = create(tmp.path) expect(tool.toModelOutput).toBeDefined() const modelOutput = tool.toModelOutput!({ output: "Test output", @@ -363,5 +318,5 @@ describe("structured-output.createStructuredOutputTool", () => { }) // The prompt loop owns the corrective attempt budget; this tool enforces the - // canonical validator before capture, including direct execute() calls. + // canonical validator on the file before capture, including direct execute() calls. }) From f72bd0bca59ae2892753a6bc9cd0d7e138f7bff2 Mon Sep 17 00:00:00 2001 From: Bulat Yapparov Date: Sat, 10 Oct 2026 13:46:04 +0100 Subject: [PATCH 2/3] fix(run): harden StructuredOutput result-file reads (#142 review) - open with O_NOFOLLOW (ELOOP is the containment rejection) and re-check that the opened inode is still the file the path resolves to inside the working directory, so a final-component or parent-directory symlink swap after the check cannot publish an outside file - open with O_NONBLOCK so a FIFO is rejected as non-regular instead of hanging the run, and read at most 2 MiB + 1 bytes so a file that grows after stat is rejected by size - containment rejects only real parent traversal, so in-root names such as `..result.json` are accepted - an unavailable working directory or a failed stat/read is a fixed, counted diagnostic; close errors are swallowed - onReject is required; PATH_SCHEMA compiles once; the corrective message JSON-encodes the submitted path - the prose-only `missing` diagnostic uses the file form, like the reminder; EVENTS.md lists every fixed file diagnostic verbatim - live integration tests run in a temporary directory and check the published value is the file passed by path Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019gASjVHuvN9AxK5WT8jzsN --- CHANGELOG.md | 2 +- EVENTS.md | 22 ++- packages/cli/src/session/output-schema.ts | 53 +++++-- packages/cli/src/session/prompt.ts | 28 ++-- .../cli/test/cli/run-output-schema.test.ts | 30 +++- .../cli/test/session/output-schema.test.ts | 29 +++- .../structured-output-integration.test.ts | 22 ++- .../test/session/structured-output.test.ts | 136 +++++++++++++++++- 8 files changed, 274 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 24826b0..21d1ad4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Breaking changes -- **StructuredOutput publishes the validated result file** — In `--output-schema` mode the `StructuredOutput` tool now takes only `{ "path": "" }`. The model writes its final result as JSON to a file in the working directory and passes the path; the CLI follows symlinks, keeps the file inside the working directory, caps it at 2 MiB, validates it with Ajv and publishes the parsed file. Passing the result object as tool arguments is no longer accepted. Callers whose prompts say "call StructuredOutput with the result" must say to write a file and pass its path. Only `{ path }` reaches the provider, so provider schema conversion can no longer drop result fields (Gemini emptied map-typed fields, #139), and the model no longer re-emits the whole result as output tokens. Rejected files are counted corrective turns that ask the model to fix the file. (#140) +- **StructuredOutput publishes the validated result file** — In `--output-schema` mode the `StructuredOutput` tool now takes only `{ "path": "" }`. The model writes its final result as JSON to a file in the working directory and passes the path; the CLI follows symlinks, keeps the file inside the working directory, caps it at 2 MiB, validates it with Ajv and publishes the parsed file. Passing the result object as tool arguments is no longer accepted. Callers whose prompts say "call StructuredOutput with the result" must say to write a file and pass its path. Only `{ path }` reaches the provider, so provider schema conversion can no longer drop result fields (Gemini emptied map-typed fields, #139), and the model no longer re-emits the whole result as output tokens. Rejected files are counted corrective turns that ask the model to fix the file; their fixed diagnostics are listed in EVENTS.md. The prose-only `missing` diagnostic and reminder now read "write the final result as JSON to a file in the working directory, then call StructuredOutput with its path". (#140) ### Fixes diff --git a/EVENTS.md b/EVENTS.md index 0cd914e..bb423ae 100644 --- a/EVENTS.md +++ b/EVENTS.md @@ -580,14 +580,24 @@ is under 2 KB and never includes submitted values or raw JSON arguments. A `path can contain property names submitted by the model (for example, in map or `patternProperties` schemas), never their values. Each path segment is clipped to 64 characters; paths also have an overall length bound. -File diagnostics use keyword `file` (`file not found`, `path must stay inside the -working directory`, `path must be a regular file`, a size over 2 MiB) or `parse` -(`file must contain complete, valid JSON`); schema diagnostics use Ajv keywords. +A result file that cannot be used emits one fixed diagnostic with `path: ""`: + +| `keyword` | `message` | +|---|---| +| `file` | `working directory unavailable` | +| `file` | `file not found` | +| `file` | `path must stay inside the working directory` | +| `file` | `file not readable` | +| `file` | `path must be a regular file` | +| `file` | `file must not exceed 2097152 bytes` | +| `parse` | `file must contain complete, valid JSON` | + +A file that parses but fails the schema emits Ajv diagnostics as above. Feedback for a rejected file goes to the model as the StructuredOutput tool error and -asks it to fix the file at that path. Malformed tool arguments (not a `{ path }` -object) go to the model through the `invalid` tool. +asks it to fix the file at the JSON-encoded path. Malformed tool arguments (not a +`{ path }` object) go to the model through the `invalid` tool. A prose-only finish emits the fixed diagnostic -`{ "path": "", "keyword": "missing", "message": "call StructuredOutput with the final result" }` +`{ "path": "", "keyword": "missing", "message": "write the final result as JSON to a file in the working directory, then call StructuredOutput with its path" }` and receives an ephemeral reminder before the next turn while budget remains. Other tools remain usable until a valid final result; their outputs are not final results. Only the `{ path }` tool schema is sent to the provider; local validation diff --git a/packages/cli/src/session/output-schema.ts b/packages/cli/src/session/output-schema.ts index c290e47..eeebb85 100644 --- a/packages/cli/src/session/output-schema.ts +++ b/packages/cli/src/session/output-schema.ts @@ -104,32 +104,61 @@ export namespace OutputSchema { required: ["path"], additionalProperties: false, } + export const shape = compile(PATH_SCHEMA) const MAX_FILE_BYTES = 2 * 1024 * 1024 + const OUTSIDE = "path must stay inside the working directory" + function contains(base: string, target: string) { + const relative = path.relative(base, target) + return relative !== ".." && !relative.startsWith(".." + path.sep) && !path.isAbsolute(relative) + } + + // Reads at most buffer.length bytes, however much the file grows while it is read. + async function fill(handle: fs.FileHandle, buffer: Buffer, offset = 0): Promise { + if (offset === buffer.length) return offset + const read = await handle.read(buffer, offset, buffer.length - offset, offset) + if (!read.bytesRead) return offset + return fill(handle, buffer, offset + read.bytesRead) + } + + // Every failure is a fixed diagnostic, so it counts as a rejection and never echoes OS error text. export async function file( root: string, input: string, validate: ReturnType, ): Promise<{ value: unknown } | { errors: Diagnostic[] }> { const fail = (keyword: string, message: string) => ({ errors: [{ path: "", keyword, message }] }) - const base = await fs.realpath(root) + const base = await fs.realpath(root).catch(() => undefined) + if (!base) return fail("file", "working directory unavailable") // realpath follows symlinks, so a link pointing outside the root is rejected too. const target = await fs.realpath(path.resolve(base, input)).catch(() => undefined) if (!target) return fail("file", "file not found") - const relative = path.relative(base, target) - if (relative.startsWith("..") || path.isAbsolute(relative)) - return fail("file", "path must stay inside the working directory") - // One handle for stat and read, so the checked file is the file read. - const handle = await fs.open(target, "r").catch(() => undefined) - if (!handle) return fail("file", "file not readable") + if (!contains(base, target)) return fail("file", OUTSIDE) + // O_NOFOLLOW refuses a final component swapped for a symlink after the check (ELOOP). + // O_NONBLOCK makes opening a FIFO return at once; fstat then rejects it as non-regular. + const handle = await fs + .open(target, fs.constants.O_RDONLY | fs.constants.O_NOFOLLOW | fs.constants.O_NONBLOCK) + .catch((error: NodeJS.ErrnoException) => (error.code === "ELOOP" ? OUTSIDE : "file not readable")) + if (typeof handle === "string") return fail("file", handle) try { - const stat = await handle.stat() + // One handle for stat and read, so the checked file is the file read. + const stat = await handle.stat().catch(() => undefined) + if (!stat) return fail("file", "file not readable") if (!stat.isFile()) return fail("file", "path must be a regular file") - if (stat.size > MAX_FILE_BYTES) return fail("file", `file must not exceed ${MAX_FILE_BYTES} bytes`) - const text = await handle.readFile("utf8") + // A parent directory swapped for a symlink after the check is followed by open, so the + // opened file must still be the file the path resolves to inside the working directory. + const again = await fs.realpath(target).catch(() => undefined) + const current = again && contains(base, again) ? await fs.stat(again).catch(() => undefined) : undefined + if (!current || current.dev !== stat.dev || current.ino !== stat.ino) return fail("file", OUTSIDE) + const size = `file must not exceed ${MAX_FILE_BYTES} bytes` + if (stat.size > MAX_FILE_BYTES) return fail("file", size) + const buffer = Buffer.alloc(MAX_FILE_BYTES + 1) + const length = await fill(handle, buffer).catch(() => undefined) + if (length === undefined) return fail("file", "file not readable") + if (length > MAX_FILE_BYTES) return fail("file", size) const value = (() => { try { - return { parsed: JSON.parse(text) as unknown } + return { parsed: JSON.parse(buffer.subarray(0, length).toString("utf8")) as unknown } } catch { return undefined } @@ -138,7 +167,7 @@ export namespace OutputSchema { if (!validate(value.parsed)) return { errors: diagnostics(validate.errors) } return { value: value.parsed } } finally { - await handle.close() + await handle.close().catch(() => undefined) } } diff --git a/packages/cli/src/session/prompt.ts b/packages/cli/src/session/prompt.ts index 35a9934..3fbe6a7 100644 --- a/packages/cli/src/session/prompt.ts +++ b/packages/cli/src/session/prompt.ts @@ -58,6 +58,9 @@ IMPORTANT: - If the file is rejected, fix the file and call this tool again with its path - Complete all necessary research and tool calls BEFORE calling this tool` +const STRUCTURED_OUTPUT_MISSING = + "write the final result as JSON to a file in the working directory, then call StructuredOutput with its path" + const STRUCTURED_OUTPUT_SYSTEM_PROMPT = `IMPORTANT: The user has requested structured output. Write your final result as JSON to a file in the working directory, then call the StructuredOutput tool with that file's path. Do NOT respond with plain text.` export namespace SessionPrompt { @@ -739,7 +742,7 @@ export namespace SessionPrompt { const reject = (raw: string) => { if (!contract) throw new Error("Missing output schema") // Only the {path} arguments reach the SDK boundary; file content errors come from execute. - return rejection(OutputSchema.parse(raw, OutputSchema.compile(OutputSchema.PATH_SCHEMA))) + return rejection(OutputSchema.parse(raw, OutputSchema.shape)) } const available = !!contract && attempts < contract.format.retryCount + 1 @@ -809,8 +812,7 @@ export namespace SessionPrompt { ? [ { role: "user" as const, - content: - "write the final result as JSON to a file in the working directory, then call StructuredOutput with its path", + content: `${STRUCTURED_OUTPUT_MISSING}`, }, ] : []), @@ -873,7 +875,7 @@ export namespace SessionPrompt { reminder = !!contract && (await missingStructuredOutput(processor.message, () => MessageV2.parts(processor.message.id))) if (reminder) { - await rejection([{ path: "", keyword: "missing", message: "call StructuredOutput with the final result" }]) + await rejection([{ path: "", keyword: "missing", message: STRUCTURED_OUTPUT_MISSING }]) } if (contract && !processor.message.error && rejected > contract.format.retryCount) { @@ -1127,7 +1129,8 @@ export namespace SessionPrompt { model?: Provider.Model /** The tool takes {path} to the JSON result file, which must stay inside this directory (#140). */ root: string - onReject?: (errors: OutputSchema.Diagnostic[]) => Promise + /** Counts the rejection against the corrective budget and returns the feedback text. */ + onReject: (errors: OutputSchema.Diagnostic[]) => Promise onSuccess: (output: unknown) => void | Promise }): AITool & { id: "StructuredOutput" } { OutputSchema.canonical(input.schema) @@ -1138,7 +1141,6 @@ export namespace SessionPrompt { canonical.bytes <= 8 * 1024 ? `Canonical JSON Schema: ${canonical.text}` : `Canonical JSON Schema (truncated; the validator enforces the full schema): ${new TextDecoder().decode(Buffer.from(canonical.text).subarray(0, 8 * 1024))}` - const shape = OutputSchema.compile(OutputSchema.PATH_SCHEMA) const result = tool({ description: `${STRUCTURED_OUTPUT_DESCRIPTION}\n\n${description}`, // Only {path} reaches the provider, so provider schema conversion cannot drop result fields. @@ -1146,20 +1148,18 @@ export namespace SessionPrompt { input.model ? ProviderTransform.schema(input.model, OutputSchema.PATH_SCHEMA) : OutputSchema.PATH_SCHEMA, { validate(value) { - return shape(value) + return OutputSchema.shape(value) ? { success: true, value: value as { path: string } } - : { success: false, error: OutputSchema.error(OutputSchema.diagnostics(shape.errors)) } + : { success: false, error: OutputSchema.error(OutputSchema.diagnostics(OutputSchema.shape.errors)) } }, }, ), async execute(args) { const loaded = await OutputSchema.file(input.root, args.path, validate) - if ("errors" in loaded) { - const message = input.onReject - ? await input.onReject(loaded.errors) - : OutputSchema.error(loaded.errors).message - throw new Error(`Fix the file at ${args.path}, then call StructuredOutput again with its path. ${message}`) - } + if ("errors" in loaded) + throw new Error( + `Fix the file at ${JSON.stringify(args.path)}, then call StructuredOutput again with its path. ${await input.onReject(loaded.errors)}`, + ) await input.onSuccess(loaded.value) return { output: "Structured output captured successfully.", diff --git a/packages/cli/test/cli/run-output-schema.test.ts b/packages/cli/test/cli/run-output-schema.test.ts index a7f3f25..6711aa8 100644 --- a/packages/cli/test/cli/run-output-schema.test.ts +++ b/packages/cli/test/cli/run-output-schema.test.ts @@ -393,7 +393,14 @@ for (const json of [true, false]) { { attempt: 1, maxAttempts: 3, - errors: [{ path: "", keyword: "missing", message: "call StructuredOutput with the final result" }], + errors: [ + { + path: "", + keyword: "missing", + message: + "write the final result as JSON to a file in the working directory, then call StructuredOutput with its path", + }, + ], }, ]) expect(result.events.filter((event) => event.type === "structured_output")).toMatchObject([ @@ -415,7 +422,14 @@ test("prose finish with zero retries fails after exactly one CLI provider reques { attempt: 1, maxAttempts: 1, - errors: [{ path: "", keyword: "missing", message: "call StructuredOutput with the final result" }], + errors: [ + { + path: "", + keyword: "missing", + message: + "write the final result as JSON to a file in the working directory, then call StructuredOutput with its path", + }, + ], }, ]) expect(result.events.filter((event) => event.type === "structured_output")).toMatchObject([ @@ -531,7 +545,17 @@ test("schema attach repairs a prose finish through the HTTP session boundary", a ]) expect(result.result).toBe(JSON.stringify({ result: "accepted" }, null, 2) + "\n") expect(result.events.filter((event) => event.type === "structured_output_rejected")).toMatchObject([ - { attempt: 1, errors: [{ path: "", keyword: "missing", message: "call StructuredOutput with the final result" }] }, + { + attempt: 1, + errors: [ + { + path: "", + keyword: "missing", + message: + "write the final result as JSON to a file in the working directory, then call StructuredOutput with its path", + }, + ], + }, ]) expect(result.events.filter((event) => event.type === "structured_output")).toMatchObject([ { status: "accepted", attempts: 2, value: { result: "accepted" } }, diff --git a/packages/cli/test/session/output-schema.test.ts b/packages/cli/test/session/output-schema.test.ts index add10e9..9effc99 100644 --- a/packages/cli/test/session/output-schema.test.ts +++ b/packages/cli/test/session/output-schema.test.ts @@ -367,7 +367,7 @@ test("repair diagnostics sent to model are bounded and never echo submitted valu .join("") expect(Buffer.byteLength(feedback)).toBeLessThan(2048) // Path form: file content errors return as the StructuredOutput tool error, not an `invalid` repair call. - expect(feedback).toContain("Fix the file at so_1_0.json") + expect(feedback).toContain('Fix the file at \\\"so_1_0.json\\\"') expect(JSON.stringify(request)).not.toContain("submitted-secret-") expect(feedback).toContain("additionalProperties") expect(result.info.structured).toEqual({ result: "repaired" }) @@ -388,7 +388,14 @@ test("model finish without StructuredOutput has an explicit missing outcome", as sessionID: result.info.sessionID, attempt: 1, maxAttempts: 1, - errors: [{ path: "", keyword: "missing", message: "call StructuredOutput with the final result" }], + errors: [ + { + path: "", + keyword: "missing", + message: + "write the final result as JSON to a file in the working directory, then call StructuredOutput with its path", + }, + ], }, ]) expect(result.info.structured).toBeUndefined() @@ -453,6 +460,7 @@ test("execute defence in depth rejects an invalid result file before capture", a const tool = SessionPrompt.createStructuredOutputTool({ schema, root: tmp.path, + onReject: async (errors) => OutputSchema.error(errors).message, onSuccess: (value) => { captured.push(value) }, @@ -504,7 +512,14 @@ test("prose-only finish is a counted corrective attempt before a valid result", sessionID: result.info.sessionID, attempt: 1, maxAttempts: 2, - errors: [{ path: "", keyword: "missing", message: "call StructuredOutput with the final result" }], + errors: [ + { + path: "", + keyword: "missing", + message: + "write the final result as JSON to a file in the working directory, then call StructuredOutput with its path", + }, + ], }, ]) expect(JSON.stringify(result.requests[1].messages)).toContain( @@ -708,7 +723,12 @@ for (const [name, description] of [ ]) { test(`tool description caps ${name} schema at 8 KiB and explains truncation`, () => { const canonical = { type: "object", description } - const tool = SessionPrompt.createStructuredOutputTool({ schema: canonical, root: import.meta.dir, onSuccess() {} }) + const tool = SessionPrompt.createStructuredOutputTool({ + schema: canonical, + root: import.meta.dir, + onReject: async () => "", + onSuccess() {}, + }) const text = JSON.stringify(canonical) const expected = new TextDecoder().decode(Buffer.from(text).subarray(0, 8 * 1024)) expect(tool.description).toContain("truncated; the validator enforces the full schema") @@ -732,6 +752,7 @@ for (const [name, schema, reason] of [ schema, validate: OutputSchema.compile({ type: "object" }), root: import.meta.dir, + onReject: async () => "", onSuccess() {}, }), ).toThrow(reason) diff --git a/packages/cli/test/session/structured-output-integration.test.ts b/packages/cli/test/session/structured-output-integration.test.ts index c9c5436..2669f85 100644 --- a/packages/cli/test/session/structured-output-integration.test.ts +++ b/packages/cli/test/session/structured-output-integration.test.ts @@ -5,21 +5,35 @@ import { SessionPrompt } from "../../src/session/prompt" import { Log } from "../../src/util/log" import { Instance } from "../../src/project/instance" import { MessageV2 } from "../../src/session/message-v2" +import { tmpdir } from "../fixture/fixture" -const projectRoot = path.join(__dirname, "../..") Log.init({ print: false }) // Skip tests if no API key is available const hasApiKey = !!process.env.ANTHROPIC_API_KEY -// Helper to run test within Instance context +// Helper to run test within Instance context. The model writes its result file into the +// working directory (#140), so each test gets a temporary one rather than the repository. async function withInstance(fn: () => Promise): Promise { + await using tmp = await tmpdir({ git: true }) return Instance.provide({ - directory: projectRoot, + directory: tmp.path, fn, }) } +// The published value is the file the model passed to StructuredOutput by path (#140). +async function expectPublishedFile(sessionID: string, structured: unknown) { + const calls = (await Session.messages({ sessionID })) + .flatMap((message) => message.parts) + .filter((part) => part.type === "tool" && part.tool === "StructuredOutput" && part.state.status === "completed") + expect(calls).toHaveLength(1) + const call = calls[0] + if (call.type !== "tool" || call.state.status !== "completed") throw new Error("unreachable") + expect(Object.keys(call.state.input)).toEqual(["path"]) + expect(await Bun.file(path.resolve(Instance.directory, call.state.input.path)).json()).toEqual(structured) +} + describe("StructuredOutput Integration", () => { test.skipIf(!hasApiKey)( "produces structured output with simple schema", @@ -57,6 +71,7 @@ describe("StructuredOutput Integration", () => { const output = result.info.structured as any expect(output.answer).toBe(4) + await expectPublishedFile(session.id, result.info.structured) // Verify no error was set expect(result.info.error).toBeUndefined() @@ -120,6 +135,7 @@ describe("StructuredOutput Integration", () => { if (output.products) { expect(Array.isArray(output.products)).toBe(true) } + await expectPublishedFile(session.id, result.info.structured) // Verify no error was set expect(result.info.error).toBeUndefined() diff --git a/packages/cli/test/session/structured-output.test.ts b/packages/cli/test/session/structured-output.test.ts index 82fd503..287a5d9 100644 --- a/packages/cli/test/session/structured-output.test.ts +++ b/packages/cli/test/session/structured-output.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, test } from "bun:test" +import { describe, expect, spyOn, test } from "bun:test" import { asSchema } from "ai" import { MessageV2 } from "../../src/session/message-v2" import { SessionPrompt } from "../../src/session/prompt" @@ -167,7 +167,13 @@ describe("structured-output.createStructuredOutputTool", () => { } const options = { toolCallId: "call", messages: [], abortSignal: undefined as any } const create = (root: string, input: Partial[0]> = {}) => - SessionPrompt.createStructuredOutputTool({ schema, root, onSuccess: () => {}, ...input }) + SessionPrompt.createStructuredOutputTool({ + schema, + root, + onReject: async (errors) => JSON.stringify(errors), + onSuccess: () => {}, + ...input, + }) // Rejections report diagnostics through onReject, as the prompt loop does. const rejecting = (root: string, rejected: unknown[], input: { schema?: Record } = {}) => create(root, { @@ -247,7 +253,7 @@ describe("structured-output.createStructuredOutputTool", () => { const rejected: unknown[] = [] const tool = rejecting(tmp.path, rejected) for (const file of ["missing.json", "dir", "big.json", "truncated.json"]) - await expect(tool.execute!({ path: file }, options)).rejects.toThrow(`Fix the file at ${file}`) + await expect(tool.execute!({ path: file }, options)).rejects.toThrow(`Fix the file at ${JSON.stringify(file)}`) expect(rejected).toEqual([ [{ path: "", keyword: "file", message: "file not found" }], [{ path: "", keyword: "file", message: "path must be a regular file" }], @@ -265,7 +271,7 @@ describe("structured-output.createStructuredOutputTool", () => { (error: Error) => error, ) expect((error as Error).message).toBe( - 'Fix the file at result.json, then call StructuredOutput again with its path. StructuredOutput rejected: [{"path":"/elements","keyword":"minProperties","message":"must NOT have fewer than 1 properties"}]', + 'Fix the file at "result.json", then call StructuredOutput again with its path. StructuredOutput rejected: [{"path":"/elements","keyword":"minProperties","message":"must NOT have fewer than 1 properties"}]', ) expect(rejected).toHaveLength(1) }) @@ -293,7 +299,7 @@ describe("structured-output.createStructuredOutputTool", () => { const rejected: unknown[] = [] await expect( rejecting(tmp.path, rejected, { schema: nested }).execute!({ path: "invalid.json" }, options), - ).rejects.toThrow("Fix the file at invalid.json") + ).rejects.toThrow('Fix the file at "invalid.json"') expect(rejected).toEqual([ [ { path: "/user", keyword: "required", message: "must have required property 'name'" }, @@ -303,6 +309,126 @@ describe("structured-output.createStructuredOutputTool", () => { ]) }) + test("accepts in-root names that start with two dots", async () => { + await using tmp = await tmpdir() + const value = { elements: { a: "b" } } + await Bun.write(path.join(tmp.path, "..result.json"), JSON.stringify(value)) + await Bun.write(path.join(tmp.path, "..out", "result.json"), JSON.stringify(value)) + const captured: unknown[] = [] + const tool = create(tmp.path, { onSuccess: (output) => void captured.push(output) }) + await tool.execute!({ path: "..result.json" }, options) + await tool.execute!({ path: "..out/result.json" }, options) + expect(captured).toEqual([value, value]) + }) + + // The swaps below run inside fs.open, after the containment check and before the open: + // the window a background process started by the model could race. + test("a result file swapped for an outside symlink after the check is rejected", async () => { + await using outside = await tmpdir() + await using tmp = await tmpdir() + await Bun.write(path.join(outside.path, "secret.json"), JSON.stringify({ elements: { secret: "outside" } })) + await Bun.write(path.join(tmp.path, "result.json"), JSON.stringify({ elements: { a: "b" } })) + const open = fs.open + const spy = spyOn(fs, "open").mockImplementation(async (...args: Parameters) => { + await fs.rm(path.join(tmp.path, "result.json")) + await fs.symlink(path.join(outside.path, "secret.json"), path.join(tmp.path, "result.json")) + return open(...args) + }) + try { + const rejected: unknown[] = [] + const tool = rejecting(tmp.path, rejected) + await expect(tool.execute!({ path: "result.json" }, options)).rejects.toThrow("Fix the file at") + expect(rejected).toEqual([ + [{ path: "", keyword: "file", message: "path must stay inside the working directory" }], + ]) + } finally { + spy.mockRestore() + } + }) + + test("a parent directory swapped for an outside symlink after the check is rejected", async () => { + await using outside = await tmpdir() + await using tmp = await tmpdir() + await Bun.write(path.join(outside.path, "result.json"), JSON.stringify({ elements: { secret: "outside" } })) + await Bun.write(path.join(tmp.path, "out", "result.json"), JSON.stringify({ elements: { a: "b" } })) + const open = fs.open + const spy = spyOn(fs, "open").mockImplementation(async (...args: Parameters) => { + await fs.rename(path.join(tmp.path, "out"), path.join(tmp.path, "moved")) + await fs.symlink(outside.path, path.join(tmp.path, "out")) + return open(...args) + }) + try { + const rejected: unknown[] = [] + const tool = rejecting(tmp.path, rejected) + await expect(tool.execute!({ path: "out/result.json" }, options)).rejects.toThrow("Fix the file at") + expect(rejected).toEqual([ + [{ path: "", keyword: "file", message: "path must stay inside the working directory" }], + ]) + } finally { + spy.mockRestore() + } + }) + + test("a FIFO is rejected without blocking", async () => { + if (process.platform === "win32") return + await using tmp = await tmpdir() + await Bun.$`mkfifo ${path.join(tmp.path, "result.json")}`.quiet() + const rejected: unknown[] = [] + const tool = rejecting(tmp.path, rejected) + const outcome = await Promise.race([ + tool.execute!({ path: "result.json" }, options).catch((error: Error) => error.message), + Bun.sleep(2_000).then(() => "blocked"), + ]) + expect(outcome).toContain("Fix the file at") + expect(rejected).toEqual([[{ path: "", keyword: "file", message: "path must be a regular file" }]]) + }) + + test("a file that grows past the cap after stat is rejected without reading it all", async () => { + await using tmp = await tmpdir() + const target = path.join(tmp.path, "result.json") + await Bun.write(target, JSON.stringify({ elements: { a: "b" } })) + const open = fs.open + const spy = spyOn(fs, "open").mockImplementation(async (...args: Parameters) => { + const handle = await open(...args) + const stat = handle.stat.bind(handle) + handle.stat = (async (...options: Parameters) => { + const result = await stat(...options) + await fs.appendFile(target, " ".repeat(3 * 1024 * 1024)) + return result + }) as typeof handle.stat + return handle + }) + try { + const rejected: unknown[] = [] + const tool = rejecting(tmp.path, rejected) + await expect(tool.execute!({ path: "result.json" }, options)).rejects.toThrow("Fix the file at") + expect(rejected).toEqual([ + [{ path: "", keyword: "file", message: `file must not exceed ${2 * 1024 * 1024} bytes` }], + ]) + } finally { + spy.mockRestore() + } + }) + + test("an unavailable working directory is a counted rejection, not a raw OS error", async () => { + await using tmp = await tmpdir() + const rejected: unknown[] = [] + const tool = rejecting(path.join(tmp.path, "removed"), rejected) + await expect(tool.execute!({ path: "result.json" }, options)).rejects.toThrow("Fix the file at") + expect(rejected).toEqual([[{ path: "", keyword: "file", message: "working directory unavailable" }]]) + }) + + test("the corrective message JSON-encodes the submitted path", async () => { + await using tmp = await tmpdir() + const rejected: unknown[] = [] + const error = await rejecting(tmp.path, rejected).execute!({ path: "a\nb\u001b[31m.json" }, options).catch( + (error: Error) => error.message, + ) + expect(error).toStartWith( + 'Fix the file at "a\\nb\\u001b[31m.json", then call StructuredOutput again with its path.', + ) + }) + test("toModelOutput returns text value", async () => { await using tmp = await tmpdir() const tool = create(tmp.path) From b80b533d39e5d1db2b62927106d75d1a0db28066 Mon Sep 17 00:00:00 2001 From: Bulat Yapparov Date: Sat, 10 Oct 2026 14:17:55 +0100 Subject: [PATCH 3/3] fix(run): path description, Windows flags, unzeroed read buffer (#142 review 2) - the provider-facing path description says relative or absolute inside the working directory, matching file() and the README - O_NOFOLLOW/O_NONBLOCK default to 0 where undefined (win32); containment there rests on the post-open inode re-check, now pinned by a test that opens without the POSIX flags - the 2 MiB + 1 read buffer is allocUnsafe: only the filled prefix is parsed and each call has its own buffer Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019gASjVHuvN9AxK5WT8jzsN --- packages/cli/src/session/output-schema.ts | 12 ++++++--- .../test/session/structured-output.test.ts | 25 +++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/session/output-schema.ts b/packages/cli/src/session/output-schema.ts index eeebb85..162c711 100644 --- a/packages/cli/src/session/output-schema.ts +++ b/packages/cli/src/session/output-schema.ts @@ -98,7 +98,7 @@ export namespace OutputSchema { path: { type: "string", maxLength: 1024, - description: "Path of the JSON result file, relative to the working directory", + description: "Path of the JSON result file in the working directory (relative, or absolute inside it)", }, }, required: ["path"], @@ -107,6 +107,11 @@ export namespace OutputSchema { export const shape = compile(PATH_SCHEMA) const MAX_FILE_BYTES = 2 * 1024 * 1024 const OUTSIDE = "path must stay inside the working directory" + // POSIX only; Windows defines neither flag. There, containment rests on the post-open + // inode re-check below, and named pipes live outside the filesystem (\\.\pipe\), so an + // in-directory path cannot open one. + const NOFOLLOW = fs.constants.O_NOFOLLOW ?? 0 + const NONBLOCK = fs.constants.O_NONBLOCK ?? 0 function contains(base: string, target: string) { const relative = path.relative(base, target) @@ -137,7 +142,7 @@ export namespace OutputSchema { // O_NOFOLLOW refuses a final component swapped for a symlink after the check (ELOOP). // O_NONBLOCK makes opening a FIFO return at once; fstat then rejects it as non-regular. const handle = await fs - .open(target, fs.constants.O_RDONLY | fs.constants.O_NOFOLLOW | fs.constants.O_NONBLOCK) + .open(target, fs.constants.O_RDONLY | NOFOLLOW | NONBLOCK) .catch((error: NodeJS.ErrnoException) => (error.code === "ELOOP" ? OUTSIDE : "file not readable")) if (typeof handle === "string") return fail("file", handle) try { @@ -152,7 +157,8 @@ export namespace OutputSchema { if (!current || current.dev !== stat.dev || current.ino !== stat.ino) return fail("file", OUTSIDE) const size = `file must not exceed ${MAX_FILE_BYTES} bytes` if (stat.size > MAX_FILE_BYTES) return fail("file", size) - const buffer = Buffer.alloc(MAX_FILE_BYTES + 1) + // Unzeroed per call: only the filled prefix is parsed, and calls never share a buffer. + const buffer = Buffer.allocUnsafe(MAX_FILE_BYTES + 1) const length = await fill(handle, buffer).catch(() => undefined) if (length === undefined) return fail("file", "file not readable") if (length > MAX_FILE_BYTES) return fail("file", size) diff --git a/packages/cli/test/session/structured-output.test.ts b/packages/cli/test/session/structured-output.test.ts index 287a5d9..1f093b2 100644 --- a/packages/cli/test/session/structured-output.test.ts +++ b/packages/cli/test/session/structured-output.test.ts @@ -200,6 +200,7 @@ describe("structured-output.createStructuredOutputTool", () => { expect(Object.keys(input.properties)).toEqual(["path"]) expect(input.required).toEqual(["path"]) expect(input.additionalProperties).toBe(false) + expect(input.properties.path.description).toContain("relative, or absolute inside it") expect(tool.description).toContain("write the final result as JSON to a file") expect(tool.description).toContain(`Canonical JSON Schema: ${JSON.stringify(schema)}`) expect(tool.description).not.toContain("$schema") @@ -346,6 +347,30 @@ describe("structured-output.createStructuredOutputTool", () => { } }) + test("without O_NOFOLLOW (Windows), the inode re-check still rejects a swapped result file", async () => { + await using outside = await tmpdir() + await using tmp = await tmpdir() + await Bun.write(path.join(outside.path, "secret.json"), JSON.stringify({ elements: { secret: "outside" } })) + await Bun.write(path.join(tmp.path, "result.json"), JSON.stringify({ elements: { a: "b" } })) + const open = fs.open + // Drops the POSIX-only flags, as on win32 where fs.constants defines neither. + const spy = spyOn(fs, "open").mockImplementation(async (...args: Parameters) => { + await fs.rm(path.join(tmp.path, "result.json")) + await fs.symlink(path.join(outside.path, "secret.json"), path.join(tmp.path, "result.json")) + return open(args[0], fs.constants.O_RDONLY) + }) + try { + const rejected: unknown[] = [] + const tool = rejecting(tmp.path, rejected) + await expect(tool.execute!({ path: "result.json" }, options)).rejects.toThrow("Fix the file at") + expect(rejected).toEqual([ + [{ path: "", keyword: "file", message: "path must stay inside the working directory" }], + ]) + } finally { + spy.mockRestore() + } + }) + test("a parent directory swapped for an outside symlink after the check is rejected", async () => { await using outside = await tmpdir() await using tmp = await tmpdir()