diff --git a/docs/cli.md b/docs/cli.md index 33e96b6167..b81af2f5f5 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -453,7 +453,6 @@ MCP `compare_bundles.record_pairs`. Pair IDs must exist on their named sides and be one-to-one. Without pairs, one-sided membership remains unknown; REA does not infer correspondence from position or display names. - Exports preserve an existing destination unless `--overwrite` is explicit. Atomic Evidence output stages on the selected destination's filesystem. If the @@ -789,7 +788,10 @@ truncation alone does not change the diagnostic health or exit status. The default terminal format is TOON. Use `--json` when saving results for a JSON consumer. Valued output flags (`--format`, `--filter-output`, `--token-limit`, `--token-offset`) accept `--flag value` or `--flag=value`; a -missing value fails with `MISSING_OUTPUT_VALUE` before the command runs. `--json` is indented on a terminal and compact when piped or +missing value fails with `MISSING_OUTPUT_VALUE` before the command runs. +Malformed format values and token windows fail with `INVALID_OUTPUT_VALUE` in the +selected structured format. Token limits and offsets must be nonnegative safe +integers; zero selects an empty window. These checks run before command execution. `--json` is indented on a terminal and compact when piped or redirected, which keeps large results smaller for agents and scripts; pipe through `jq .` for indented files. Output selection and formatting do not change operation status. diff --git a/src/cli/cliResultOutput.ts b/src/cli/cliResultOutput.ts index 34474a63bd..a7ad793c93 100644 --- a/src/cli/cliResultOutput.ts +++ b/src/cli/cliResultOutput.ts @@ -240,8 +240,14 @@ export const createCliResultOutput = ( finishSdkOutput(exitCode) { return (sdkCompletion ??= (async () => { const parts = sdkParts.splice(0); - if (parts.length === 0) return; try { + if (parts.length === 0) { + // SDK writes outside the JSONL buffer still share this destination. + // A write callback runs after earlier queued writes have completed, + // so the launcher's explicit exit cannot discard a pending pipe tail. + await writeOutputText(destination, ""); + return; + } if (exitCode === 0) { for (const part of parts) await writeOutputText(destination, part); } else { diff --git a/src/cliOutput.ts b/src/cliOutput.ts index 1bc200ebb6..7b6e82b03a 100644 --- a/src/cliOutput.ts +++ b/src/cliOutput.ts @@ -46,8 +46,8 @@ export const renderCliDocument = ( ? [...jsonOutputParts(value, format, stdoutIsTerminal)].join("") : `${formatCliOutput(value, format)}\n`; /** - * Incur's message when its builtin pass rejects this spaced value of a valued - * output flag, or undefined when Incur accepts it. + * Validate a spaced output-flag value before Incur dispatch. REA narrows token + * coordinates to nonnegative safe integers so slice metadata remains coherent. */ export const cliOutputValueRejection = ( flag: string, @@ -59,9 +59,11 @@ export const cliOutputValueRejection = ( : `Invalid format: "${value}". Expected one of: ${[...INCUR_OUTPUT_FORMATS].join(", ")}`; if ( (flag === "--token-limit" || flag === "--token-offset") && - (!Number.isFinite(Number(value)) || value.trim() === "") + (!Number.isSafeInteger(Number(value)) || + Number(value) < 0 || + value.trim() === "") ) - return `Invalid value for ${flag}: "${value}"`; + return `Invalid value for ${flag}: "${value}". Expected a nonnegative safe integer.`; return undefined; }; @@ -73,6 +75,7 @@ export type CliOutputArgumentValidation = readonly code: | "UNSUPPORTED_OUTPUT_COMBINATION" | "INVALID_OUTPUT_FILTER" + | "INVALID_OUTPUT_VALUE" | "MISSING_OUTPUT_VALUE"; readonly message: string; }; @@ -85,7 +88,7 @@ interface CliOutputControls { readonly fullOutput: boolean; /** First valued output flag that Incur skips because it has no value. */ readonly missingValue: string | undefined; - readonly parseError: boolean; + readonly invalidValue: string | undefined; readonly tokenCount: boolean; readonly tokenWindow: boolean; readonly tokenLimit: number | undefined; @@ -109,17 +112,19 @@ export const parseCliOutputArguments = ( let filterOutput: string | undefined; let fullOutput = false; let missingValue: string | undefined; + let invalidValue: string | undefined; let tokenCount = false; let tokenLimit: number | undefined; let tokenOffset: number | undefined; - const parsed = (parseError: boolean): ParsedCliOutputArguments => { + const parsed = (): ParsedCliOutputArguments => { + const parseError = invalidValue !== undefined; const controls: CliOutputControls = { filterOutput, format, formatExplicit, fullOutput, missingValue, - parseError, + invalidValue, tokenCount, tokenWindow: !parseError && (tokenLimit !== undefined || tokenOffset !== undefined), @@ -160,12 +165,12 @@ export const parseCliOutputArguments = ( if (argument === "--format") { const value = arguments_[index + 1]; if (value) { - // Incur stops parsing at an invalid format before applying any later - // output controls. Let it report that malformed flag itself. - if (cliOutputValueRejection(argument, value) !== undefined) - return parsed(true); - format = value; - formatExplicit = true; + const rejection = cliOutputValueRejection(argument, value); + if (rejection !== undefined) invalidValue ??= rejection; + else { + format = value; + formatExplicit = true; + } index += 1; } else missingValue ??= argument; continue; @@ -180,11 +185,10 @@ export const parseCliOutputArguments = ( if (argument === "--token-limit" || argument === "--token-offset") { const value = arguments_[index + 1]; if (value) { - if (cliOutputValueRejection(argument, value) !== undefined) - return parsed(true); - const numericValue = Number(value); - if (argument === "--token-limit") tokenLimit = numericValue; - else tokenOffset = numericValue; + const rejection = cliOutputValueRejection(argument, value); + if (rejection !== undefined) invalidValue ??= rejection; + else if (argument === "--token-limit") tokenLimit = Number(value); + else tokenOffset = Number(value); index += 1; } else missingValue ??= argument; continue; @@ -192,7 +196,7 @@ export const parseCliOutputArguments = ( if (argument === "--full-output") fullOutput = true; if (argument === "--token-count") tokenCount = true; } - return parsed(false); + return parsed(); }; /** Reject slices that cannot advance within Incur's comma-delimited paths. */ @@ -222,10 +226,16 @@ const validateCliOutputControls = ({ filterOutput, format, missingValue, - parseError, + invalidValue, tokenWindow, }: CliOutputControls): CliOutputArgumentValidation => { - if (parseError) return { ok: true }; + if (invalidValue !== undefined) + return { + ok: false, + format: format === "yaml" || format === "jsonl" ? format : "json", + code: "INVALID_OUTPUT_VALUE", + message: invalidValue, + }; if (filterOutput !== undefined && hasUnclosedFilterSlice(filterOutput)) return { ok: false, @@ -260,9 +270,9 @@ const validateCliOutputControls = ({ export const renderEmptyFilteredCliOutput = ({ filterOutput, format, - parseError, + validation, }: ParsedCliOutputArguments): string | undefined => { - if (parseError) return undefined; + if (!validation.ok) return undefined; if ( filterOutput && (format === "json" || format === "jsonl" || format === "yaml") diff --git a/tests/boundary/cli/cliOutput.test.ts b/tests/boundary/cli/cliOutput.test.ts index 7696380d1d..1be3644e12 100644 --- a/tests/boundary/cli/cliOutput.test.ts +++ b/tests/boundary/cli/cliOutput.test.ts @@ -79,6 +79,73 @@ describe("CLI output argument boundary", () => { }); }); +describe("CLI output value admission", () => { + it.each(["--token-limit", "--token-offset"])( + "rejects invalid %s values before dispatch and preserves later JSON selection", + (flag) => { + for (const value of [ + "-1", + "0.5", + "NaN", + "Infinity", + "9007199254740992", + ]) { + const validation = parseCliOutputArguments([ + "providers", + flag, + value, + "--json", + ]).validation; + expect(validation).toMatchObject({ + ok: false, + format: "json", + code: "INVALID_OUTPUT_VALUE", + }); + if (!validation.ok) + expect( + JSON.parse(renderCliOutputArgumentError(validation)), + ).toMatchObject({ + ok: false, + error: { code: "INVALID_OUTPUT_VALUE" }, + }); + } + for (const value of ["0", "1", "9007199254740991"]) + expect( + parseCliOutputArguments(["providers", flag, value]).validation, + ).toEqual({ ok: true }); + }, + ); + + it("retains valid output selections on either side of a malformed value", () => { + for (const arguments_ of [ + ["--json", "--format", "invalid"], + ["--format", "invalid", "--json"], + ["--token-limit", "invalid", "--json"], + ]) + expect(parseCliOutputArguments(arguments_).validation).toMatchObject({ + ok: false, + format: "json", + code: "INVALID_OUTPUT_VALUE", + }); + expect( + parseCliOutputArguments(["--format", "jsonl", "--format", "invalid"]) + .validation, + ).toMatchObject({ + ok: false, + format: "jsonl", + code: "INVALID_OUTPUT_VALUE", + }); + expect( + parseCliOutputArguments(["--format", "yaml", "--token-offset", "-1"]) + .validation, + ).toMatchObject({ + ok: false, + format: "yaml", + code: "INVALID_OUTPUT_VALUE", + }); + }); +}); + describe("CLI output flag value forms", () => { it.each(["--filter-output", "--format", "--token-limit", "--token-offset"])( "preserves a flag-shaped spaced value owned by %s", @@ -180,8 +247,6 @@ describe("unclosed CLI output filter slices", () => { ], ["--filter-output", "capabilities[-1,].provider.id"], ["--filter-output", "capabilities[[0,1]"], - ["--format", "invalid", "--filter-output", "providers["], - ["--token-limit", "invalid", "--filter-output", "providers["], ]) expect(parseCliOutputArguments(arguments_).validation).toEqual({ ok: true, @@ -356,7 +421,10 @@ describe("CLI output valued-flag parsing", () => { ]) { const result = await cli.run({ arguments: arguments_ }); expect(result.exitCode).toBe(1); - expect(result.stdout).not.toContain("UNSUPPORTED_OUTPUT_COMBINATION"); + expect(result.json).toMatchObject({ + ok: false, + error: { code: "INVALID_OUTPUT_VALUE" }, + }); expect(result.stdout).not.toContain("[truncated:"); } }, diff --git a/tests/boundary/cli/jsonOutput.test.ts b/tests/boundary/cli/jsonOutput.test.ts index b7d1b7a372..fd9401cd0b 100644 --- a/tests/boundary/cli/jsonOutput.test.ts +++ b/tests/boundary/cli/jsonOutput.test.ts @@ -39,6 +39,42 @@ const collect = async ( return Buffer.concat(chunks).toString("utf8"); }; +describe("SDK output completion", () => { + it.each(["json", "toon"])( + "waits for pending raw %s writes before exit", + async (format) => { + const document = JSON.stringify({ + code: "VALIDATION_ERROR", + message: "x".repeat(8192), + }); + let complete = false; + const destination = new Writable({ + highWaterMark: 1, + write(chunk, _encoding, callback) { + if (chunk.length === 0) { + callback(); + return; + } + setImmediate(() => { + complete = true; + callback(); + }); + }, + }); + const output = createCliResultOutput( + parseCliOutputArguments(["--format", format]), + destination, + ); + expect(output.captureSdkOutput(document)).toBe(false); + destination.write(document); + expect(complete).toBe(false); + await output.finishSdkOutput(1); + expect(complete).toBe(true); + expect(destination.writableEnded).toBe(false); + }, + ); +}); + describe("incremental CLI JSON", () => { it.each([ null,