feat(cli): add --json-schema for structured output in headless mode (#3598)

* feat(cli): add --json-schema for structured output in headless mode

Registers a synthetic `structured_output` tool whose parameter schema IS the
user-supplied JSON Schema. In headless mode (`qwen -p`), the first successful
call terminates the session and exposes the validated payload via the result
message's `structured_result` field. Invalid schemas are rejected at CLI parse
time via a new strict Ajv compile helper so they can't silently no-op at
runtime.

* fix(cli): honour "first structured_output call ends session" + reject non-object root schemas

Two review fixes for the `--json-schema` feature:

1. `runNonInteractive` now breaks out of the tool-call loop as soon as the
   first successful `structured_output` invocation is captured, rather than
   continuing to execute any trailing tool calls the model emitted in the
   same turn. This restores the documented single-shot contract and prevents
   side-effecting tools from running after the final answer has already
   been accepted.

2. `resolveJsonSchemaArg` rejects schemas whose root `type` is anything
   other than "object" (or a type array including "object"). Function-
   calling APIs require tool arguments to be JSON objects, so a schema
   like `{"type": "array"}` would have registered an unusable synthetic
   tool the model could never satisfy. Absent `type` and `type: "object"`
   remain accepted.

Adds tests for both paths and updates the existing Ajv-compile test to
exercise that path without tripping the new root-type guard first.

* fix(cli): also reject root anyOf/oneOf schemas whose branches can't accept objects

Addresses a review follow-up: the previous root-object check only inspected
the top-level `type` keyword, so a schema like
`{"anyOf":[{"type":"array"},{"type":"string"}]}` slipped through even though
none of its branches can ever validate the object-shaped arguments that
function-calling APIs send.

Replace the single `type` check with `schemaRootAcceptsObject`, which
recursively walks root-level anyOf/oneOf branches and requires at least one
to accept objects. Absent `type`, `type: "object"`, `type: ["object", ...]`,
and mixed anyOf branches where one accepts object all still pass. `allOf`
is left to Ajv's runtime behaviour — guessing intent across contradictory
allOf branches at parse time is fragile.

* fix(cli): propagate exitCode from --json-schema failure path + tests

Address two PR-3598 review findings:

1. gemini.tsx unconditionally called process.exit(0) after
   runNonInteractive/runNonInteractiveStreamJson, clobbering the
   process.exitCode = 1 set by nonInteractiveCli.ts when the model
   emits plain text instead of the structured_output tool. Switch
   both call sites to process.exit(process.exitCode ?? 0) so CI can
   detect the failure via the exit code.

2. nonInteractiveCli.test.ts: strengthen the structured-output
   success path to assert registry.abortAll() is called and that the
   stdout result envelope carries the JSON-stringified args under
   `result` plus the raw object under `structured_result`. Add a
   retry-path test that mocks executeToolCall to return an error on
   the first structured_output call, then verifies sendMessageStream
   is called a second time so the model can retry rather than the
   session terminating early.

* fix(cli): suppress non-structured tool calls when structured_output is in the same turn

When --json-schema is active and the model emits a batch like
[write_file(...), structured_output(...)], the previous implementation
ran the leading side-effecting tool before accepting the structured
result, violating the "structured_output is the terminal contract"
guarantee. The trailing-only break also let an invalid first
structured_output fall through to subsequent tools before the retry
turn.

Pre-scan the batch: if a structured_output request is present, execute
ONLY the first one and skip everything else (leading and trailing).
This is consistent with the existing terminal-path semantics — the
suppressed tool_use blocks lack a matching tool_result, the same way
max-turns / cancellation leave the stream.

Adds a test covering the reverse-order [side_effect, structured_output]
case alongside the existing trailing-suppression and retry tests.

* fix(cli): tighten --json-schema root validation per review feedback

Three small holes flagged in the latest pass:

1. `schemaRootAcceptsObject` returned early when a root `type` keyword
   was present, ignoring sibling `anyOf`/`oneOf`. JSON Schema applies
   keywords at the same level conjunctively, so e.g.
   `{type:"object", anyOf:[{type:"string"}]}` is unsatisfiable for any
   value but used to pass. Now both `type` AND any sibling
   `anyOf`/`oneOf` must independently admit object.

2. The FatalConfigError text said "Every branch of a root anyOf/oneOf
   must be satisfiable by an object", but the actual logic only
   requires *at least one* branch (and tests still accept
   `anyOf:[object, string]`). Reworded to "at least one branch" so the
   message matches the behaviour.

3. `compileStrict` used `typeof schema !== 'object'` to gate input,
   which lets arrays through (`typeof [] === 'object'`). The contract
   says "schema must be a JSON object", so add an `Array.isArray`
   check so array input gets the intended error rather than a less
   helpful Ajv compile message.

Tests cover the new rejection paths and the array case.

* fix(cli): handle root $ref and allOf in --json-schema accept-object check

`schemaRootAcceptsObject` previously only inspected `type`, `anyOf`,
and `oneOf` at the root, so a couple of unsatisfiable shapes still
slipped through:

1. `{"$ref":"#/$defs/Foo","$defs":{"Foo":{"type":"array"}}}` would be
   accepted because we don't follow $refs, but registers a synthetic
   tool whose params resolve to "array" — the model can never produce
   a valid object. Now reject any root $ref unless the user adds a
   sibling `type:"object"` as an explicit anchor.

2. `allOf` was deferred to Ajv runtime, but allOf is conjunctive at
   the same level as `type` / `anyOf` / `oneOf`, so an entry like
   `{"allOf":[{"type":"object"},{"type":"string"}]}` is unsatisfiable
   for any value. Walk it like the others, requiring every branch to
   admit object.

Tests cover the new $ref-rejected / $ref+anchor-accepted paths and the
allOf reject/accept paths.

* fix(cli): explicit exit code from runNonInteractive + pair suppressed tool calls

Three review threads on the structured-output flow:

1. The break that ends the for-loop on a successful structured_output
   call sat *before* the responseParts.push and modelOverride capture.
   SyntheticOutputTool currently returns neither, so it was safe today
   — but anyone wiring extra signals into the synthetic tool later
   would see them silently dropped. Move the break after both captures
   so the contract is explicit, not implicit.

2. The failure path used to set process.exitCode = 1 and return void,
   relying on global mutable state across an async boundary. Any
   cleanup task between runNonInteractive and process.exit could
   silently turn the structured-output failure into exit 0. Switch
   runNonInteractive to Promise<number>, return 0 / 1 directly from
   each function-level exit, and have gemini.tsx use the captured
   return value.

3. The pre-scan from the prior commit suppresses sibling tool calls
   when structured_output is in the same turn. On the retry path —
   when structured_output fails validation — the next-turn payload
   has tool_result for structured_output but no entry for the
   suppressed siblings, leaving the prior assistant turn's tool_use
   blocks unpaired. Anthropic and OpenAI both reject that batch
   shape, so the retry would surface as an opaque provider error.
   Synthesize a "skipped" functionResponse for every suppressed call
   so every tool_use in the prior assistant message has a matching
   tool_result.

Tests cover the new retry-pairing contract and update the existing
plain-text-failure test to assert on the return value rather than
process.exitCode.

* fix: address Copilot follow-up review on --json-schema scaffolding

Five small but real findings flagged on the latest pass:

1. core/src/index.ts re-exported `SyntheticOutputTool` via `export type`,
   but it's a runtime class — that erased it from the emitted JS and
   would break value imports. Split into a value `export { ... }` and a
   `export type { StructuredOutputParams }`.

2. The structured-output success path returned without flushing
   `localQueue` notifications or finalising one-shot monitors. If a
   background task had already emitted `task_started`, exiting here
   could drop its paired `task_notification` and leave SDK consumers
   with unpaired lifecycle events. Mirror the regular terminal path's
   `flushQueuedNotificationsToSdk` + `finalizeOneShotMonitors` calls
   before `emitResult`.

3. `schemaRootAcceptsObject` ignored the `not` keyword, so
   `{not:{type:"object"}}` (which forbids every object value) slipped
   through. Add a best-effort `not` check that rejects when
   `not.type` directly excludes object. Deeper negated patterns still
   fall through to Ajv at runtime.

4. `compileStrict`'s JSDoc claimed it errored on "Ajv versions we can't
   support", but the function doesn't actually check Ajv versions. Reword
   to "malformed or uses unsupported draft/features for our Ajv
   configuration" so the contract matches the implementation.

5. The pre-scan suppressed sibling tool calls but only synthesised
   tool_result events for them on the retry path — the success path
   left those tool_use blocks unpaired in the emitted JSONL/stream-json
   event log. Move the synthesis after the for loop so it runs for both
   the success break and the validation-failure fall-through; the event
   log is now consistent regardless of which path the run takes.

Tests cover the new \`not\`-rejection paths, the success-path tool_result
synthesis, and the existing retry-pairing test still passes against the
restructured emit ordering.

* fix(cli): tighten --json-schema parse-time gate per Copilot review

Two more shapes that used to slip through:

1. `schemaRootAcceptsObject` defaulted to true when no narrowing
   keyword was present, so root-value constraints like `{const: 1}`
   or `{enum: [1, 2]}` registered an unsatisfiable structured_output
   contract — the model could never produce a value matching the
   tool's parameter schema, and the run would loop on validation
   failures until max-turns. Reject `const` whose literal isn't an
   object, and `enum` whose members include no object.

2. The yargs check rejected `--json-schema` with `-i` and with no
   prompt, but not with `--input-format stream-json`. Stream-json
   keeps the process open waiting for protocol messages, so
   "terminate on the first valid structured_output call" silently
   drops everything queued after that point. Refuse the combination
   at parse time so the contradiction surfaces immediately.

Tests cover the new const/enum reject and accept paths.

* fix(cli): handle empty/boolean subschemas + allow stdin-only prompt

Three more shapes flagged on the latest review pass:

1. `schemaRootAcceptsObject` treated an empty root `anyOf`/`oneOf`
   as "no constraint" (skipped when length === 0), but per JSON
   Schema an empty union is unsatisfiable — no value can match a
   member of the empty set. Reject those at parse time so users
   get a clear parse error instead of an opaque runtime
   never-validates loop.

2. JSON Schema (draft-06+) allows boolean subschemas anywhere a
   schema is accepted: `true` matches every value, `false` matches
   nothing. The `anyOf`/`oneOf`/`allOf` walks were rejecting
   booleans via the typeof-object guard, which incorrectly
   rejected `{anyOf:[true]}` and `{allOf:[true,{type:"object"}]}`
   while letting `{anyOf:[false]}` slip through. Replace the
   per-branch object guard with a `variantAcceptsObject` helper
   that treats `true` as accepting and `false` as rejecting, then
   recurses on object subschemas.

3. The yargs `.check` rejected `--json-schema` when no `-p` /
   positional prompt was given, but the headless CLI also reads
   the prompt from stdin (`cat prompt.txt | qwen --json-schema
   '...'`) — a legit usage pattern that was being blocked. Drop
   the parse-time no-prompt rejection; the existing runtime "No
   input provided via stdin..." error in gemini.tsx still catches
   genuinely empty input.

Tests cover the empty-union, all-`false`, mixed-boolean accept,
and `false`-in-allOf reject paths. Live-verified against the
bundled CLI: `echo "..." | qwen --json-schema '...'` now reaches
the model call, and the four schema edge cases all surface the
expected error text or proceed past parse time.

* docs(core): note SyntheticOutputTool as the value-export exception

The block comment above the lazy-load type re-exports said tool classes
"are now lazy-loaded and are not exported as values from the package
root", but `SyntheticOutputTool` was just promoted to a runtime export
in 62038527c so the CLI's `--json-schema` flow can construct it from
the package root. Document that exception inline so downstream
consumers reading the comment don't get told the wrong story.

* fix(cli): try every structured_output in a same-turn batch in order

The pre-scan used to pick only the FIRST structured_output call from a
turn and suppress everything else, even other structured_output calls.
That created two avoidable failure modes:

1. `[structured_output(bad), structured_output(good)]` would attempt
   only the bad one, fail validation, and force a full retry turn.
   The model already produced a valid structured payload — we should
   try it before asking again.

2. The trailing structured_output's tool_result was synthesised with
   the "Skipped: structured_output was also requested in this turn..."
   message, which is misleading because that call WAS the structured
   output we should have tried.

Filter `requestsToExecute` to ALL structured_output calls (in original
order) when --json-schema is active, and let the existing loop break
on the first success. Track an `executedCallIds` set, then synthesise
tool_result + retry parts after the loop for every tool_use the model
emitted that we never actually executed — covering both non-structured
siblings (always suppressed) and any structured_output left over after
the success break (only one terminal contract per turn).

Reworded the synthesised "skipped" output to "this turn's
structured_output contract took precedence" so it reads correctly
regardless of whether the suppressed call was structured or not.

Tests cover the multi-structured retry-free success path; the
existing single-structured retry and trailing/leading suppression
tests still pass against the updated emit ordering.

* fix: address gpt-5.5 review on --json-schema (privacy + $ref + core-tools)

Three findings, three changes:

1. Reject every root `$ref` in --json-schema, even with a sibling
   `type: "object"` anchor. Ajv applies `$ref` conjunctively with
   sibling keywords, so the previous "accept when type:object is
   present" carve-out was unsound: `{type:"object",$ref:"#/$defs/Foo",
   $defs:{Foo:{type:"array"}}}` parsed fine but no object value can
   satisfy both at runtime, leaving the model to loop until maxTurns.
   Updated docstring + test cases (replaced the accept-with-anchor
   case with a reject case for both anchored and well-formed $ref
   shapes — users wanting composition should inline at the root).

2. Redact `function_args` for structured_output in ToolCallEvent.
   The args ARE the user's structured payload (already emitted via
   stdout `result` / `structured_result`); recording them again as
   ordinary tool-call function_args duplicates that data into OTLP
   exports, QwenLogger, ui-telemetry, and the chat-recording UI
   event mirror — surfaces that can leak off-device. Replace with a
   stable `__redacted` placeholder so consumers still see the call
   happened (duration, success, decision metrics preserved) but the
   payload itself doesn't ride along. Two new uiTelemetry tests
   cover the redacted vs non-redacted paths.

3. Document and test that structured_output bypasses the --core-tools
   allowlist (same as agent / skill / exit_plan_mode / ask_user_question
   etc.). The synthetic tool only exists when --json-schema is set,
   so adding it to CORE_TOOLS would let `--core-tools read_file
   --json-schema X` silently drop the terminal contract and loop the
   model until maxTurns — bypass is intentional. Expanded the
   CORE_TOOLS docstring to enumerate the synthetic-tool exclusions
   and added a permission-manager test mirroring the pattern used
   for agent / skill / exit_plan_mode.

* fix(cli): apply structured_output terminal handling to drain turns

The synthetic structured_output tool is registered for the entire
headless session, so it can be invoked from EITHER the main
assistant-turn loop OR from a drain turn (queued cron-job /
notification reply). The drain path (drainOneItem) was treating it
like any other tool: execute, append the response back into
itemMessages, keep going. The submitted args were never captured and
no structured_result envelope was emitted, so a run that legitimately
satisfied --json-schema mid-drain ended up failing the contract with
"Model produced plain text..." anyway.

Apply the same terminal handling to drain turns:

- Hoist `structuredSubmission` to session scope so both paths write
  to one variable.
- In `drainOneItem`, run the same pre-scan: when --json-schema is
  active and structured_output is in the batch, execute every
  structured_output in original order until one succeeds; suppress
  every non-structured sibling. Synthesise tool_results for any
  unexecuted tool_use the model emitted, mirroring the main path.
- On capture, return early from drainOneItem so the drained item's
  inner while loop stops.
- `drainLocalQueue` short-circuits when a captured submission is in
  flight, so subsequent queued items don't run.
- The cron `checkCronDone` watches the same flag and stops the
  scheduler immediately on capture, releasing the surrounding
  `await new Promise(...)`.
- The final holdback loop bails out on capture so monitor lifecycle
  doesn't extend past the structured submission.
- After the holdback, before the existing failure / regular-success
  emit, emit the structured success envelope and return 0.

Adds a focused unit test that drives the drain path end-to-end via a
synchronously-fired monitor notification: main turn produces plain
text, the drain reply calls structured_output, and the test asserts
exit 0 + structured_result populated + no "Model produced plain
text..." error.

* fix(cli): address gpt-5.5 review follow-ups on --json-schema scaffolding

Six review findings, six small fixes:

1. **Nested $ref incorrectly rejected.** `schemaRootAcceptsObject`
   recurses into anyOf/oneOf/allOf branches and used to apply the
   root-only $ref rejection at every level, blocking common
   composition shapes like
   `{anyOf:[{$ref:"#/$defs/Foo"},{type:"string"}]}`. Add an
   `isRoot=true` parameter; non-root recursion treats `$ref` as
   opaque and defers to Ajv at runtime. Tests cover nested refs in
   anyOf / oneOf / allOf.

2. **Inaccurate package-root export comment.** `core/src/index.ts`
   claimed `SyntheticOutputTool` was exported as a runtime value
   for the CLI's --json-schema flow, but the only construction is
   inside `Config.registerLazy` via a relative dynamic import — no
   value consumer reaches into `@qwen-code/qwen-code-core`. Revert
   to a type-only re-export so `SyntheticOutputTool` lines up with
   every other lazy-loaded tool class.

3. **Unused constructor parameter.** `SyntheticOutputTool` took
   `(_config: Config, schema)` but never read `_config`. Drop the
   parameter (and the corresponding pass-through at the registration
   call site) so readers don't wonder why a Config is being threaded
   through.

4. **Tool description claimed "exactly once".** The retry path
   explicitly tolerates multiple calls until one validates, so
   "Call this tool exactly once" is misleading to a model that
   tried twice. Reword to "Call this tool to deliver the final
   result; the first call with valid arguments ends the session" so
   the description matches the actual contract.

5. **Asymmetric shutdown on the structured-output success path.**
   The regular terminal path waits in a holdback loop until
   `hasUnfinalizedTasks()` is false; the structured-output path
   used to call `abortAll()` and flush immediately, dropping the
   matching `task_notification` for any agent whose natural handler
   hadn't yet enqueued it. Add a bounded holdback (capped at 500ms
   via STRUCTURED_SHUTDOWN_HOLDBACK_MS) — long enough for typical
   abort callbacks to enqueue, short enough that a hung agent can't
   block exit.

6. **gemini.tsx exit-code asymmetry.** `runNonInteractive` returns
   an explicit exit code, but `runNonInteractiveStreamJson` still
   reads `process.exitCode` after `runExitCleanup`. Currently safe
   because the yargs `.check` rejects --json-schema with stream-json
   input, but a future stream-json equivalent of structured output
   would need to plumb the exit code through the return value too.
   Document this in a comment so the constraint is visible at the
   call site.

Plus: strengthen `synthesises tool_result for suppressed sibling
calls when structured_output fails validation` to assert the failed
structured_output's `functionResponse.response` carries the actual
validation error string ("args invalid"), not the synthesised
"Skipped:" prose — a regression that overwrote it would otherwise
slip past the existing pairing assertion.

* fix(cli): close --json-schema gaps surfaced in self-audit + review

Five fixes layered onto the same robustness pass over the
`--json-schema` flow:

1. **bare-mode registration** (`packages/core/src/config/config.ts`):
   `qwen --bare --json-schema X -p "..."` previously skipped the
   synthetic `structured_output` registration entirely (the
   registration block lives below the bare-mode early-return), so the
   model had no way to terminate and the run looped to
   `maxSessionTurns`. Register the synthetic tool inside the bare
   branch too.

2. **TTY interactive rejection** (`packages/cli/src/gemini.tsx`):
   `qwen --json-schema X` on a TTY with no `-p` and no piped stdin
   routes to `isInteractive=true` (priority-3 fallback) and would
   launch the TUI, where `structured_output` is just an inert tool
   that prints "accepted" and lets the chat continue. Parse-time
   gating can't catch this (stdin isn't probed yet at parse time), so
   reject at runtime before the UI launches; runs `runExitCleanup`
   first so MCP subprocesses get torn down.

3. **drain-turn structured-success flush**
   (`packages/cli/src/nonInteractiveCli.ts`): when a drain turn
   captures `structured_output`, `drainLocalQueue` returns early,
   leaving any items the drain didn't process in `localQueue`. The
   prior emit path then ran `registry.abortAll()` + `emitResult`
   without flushing — stream-json consumers saw `task_started` events
   without paired `task_notification`. Add the same 500ms holdback +
   `flushQueuedNotificationsToSdk` the main-turn structured-success
   path uses, so the two paths agree.

4. **ACP mutual-exclusion** (`packages/cli/src/config/config.ts`):
   `--acp` runs an independent `runAcpAgent` turn loop that doesn't
   honour the synthetic-tool terminal contract, so `--acp
   --json-schema X` would register the tool but never terminate. Add
   a yargs `.check` rejection covering both `--acp` and the
   deprecated `--experimental-acp` alias.

5. **max-turns + Skipped wording** (review comments
   #3198579251/#3198579389/#3198579567 from yiliang114):
   - `handleMaxTurnsExceededError` now appends a `--json-schema`-
     specific hint pointing at the common stuck-run causes
     (structured_output denied by `permissions.deny` /
     `--exclude-tools`, unsatisfiable schema, prompt didn't
     instruct the model). Without this, three different failures
     all surfaced as the same generic "increase maxSessionTurns"
     line.
   - The synthesised "Skipped:" tool_result for suppressed sibling
     calls drops the trailing "Re-issue this call in a separate
     turn if needed." sentence on the success path, where the
     session terminates immediately and no consumer (model or SDK)
     can act on the advice. Retry path keeps the sentence — the
     model is about to receive these parts and may legitimately
     re-issue.

Tests cover each fix: bare-mode registration order, ACP / experimental-
acp rejection (×2), `--json-schema` hint in both text and JSON max-turns
output, and explicit Skipped-text assertions on the success and retry
paths.

* fix: address 9 self-qreview comments on --json-schema PR

Folds the 9 Suggestion-level comments from the previous /qreview pass
into code/test fixes. Each one is a real issue, but mostly defensive —
none changes the user-visible happy path.

**Refactors (F4/F5/F6 — code-quality)**

- F4 `nonInteractiveCli.ts`: extract `SUPPRESSED_OUTPUT_SUCCESS` /
  `SUPPRESSED_OUTPUT_RETRY` module-level constants and a
  `suppressedOutputBody(structuredCaptured)` helper. Both the main-turn
  and drain-turn synthesis sites previously had a 4-way duplicated
  ternary; future wording changes can no longer drift between them.
- F5 `nonInteractiveCli.ts`: extract `emitStructuredSuccess()` closure
  inside `runNonInteractive`. The "abortAll → bounded holdback → flush
  → finalize one-shot monitors → emitResult → return 0" terminal block
  is now defined once and called from both the main-turn and drain-turn
  success paths. `finalizeOneShotMonitors` is idempotent
  (`oneShotMonitorsFinalized` guard) so the unconditional invocation is
  safe even when the drain-turn already finalized monitors before
  reaching the helper.
- F6 `core/config/config.ts`: extract
  `registerStructuredOutputIfRequested()` helper. The synthetic-tool
  registration block is no longer duplicated between the bare-mode
  early-return branch and the regular registration branch.

**Tests (F7/F8/F9 — pin existing behaviour)**

- F7 `nonInteractiveCli.test.ts`: new test "holds back for in-flight
  background tasks before emitting structured success" — flips
  `hasUnfinalizedTasks: true → false` mid-poll so the holdback `while`
  body actually executes; spies on `abortAll` and asserts ordering of
  `task_notification` (must precede the result envelope) and the
  bounded elapsed-time cap. None of the existing structured-output
  success tests entered this branch (they all pinned
  `hasUnfinalizedTasks: () => false`).
- F8 `gemini.test.tsx`: new test "rejects --json-schema when running
  in interactive (TUI) mode" — pins the TUI guard at gemini.tsx:694,
  asserting the headless-only stderr message AND the exact ordering
  `writeStderrLine → runExitCleanup → process.exit(1)` so a future
  refactor can't swap any of those steps.
- F9 `cli/config.test.ts`: pin the two previously-untested
  `--json-schema` mutual-exclusion branches: `-i`/`--prompt-interactive`
  and `--input-format stream-json`. The stream-json check is
  load-bearing — `gemini.tsx:768` explicitly relies on this rejection
  holding (the parse-time `process.exitCode ?? 0` plumbing in the
  stream-json branch is only safe because `--json-schema` can't reach
  it).

**Behaviour fixes (F1/F2/F15 — privacy / security / correctness)**

- F1 `core/core/geminiChat.ts`: redact `functionCall.args` for
  `structured_output` tool calls before passing them to
  `chatRecordingService.recordAssistantTurn`. Without this, the
  user's structured payload (already emitted on stdout via
  `result` / `structured_result`) was persisted verbatim to
  `<projectDir>/chats/<sessionId>.jsonl` and re-fed into model
  context on `--continue` / `--resume`, contradicting the privacy
  contract documented next to the existing `ToolCallEvent` redaction.
  Each validation-failure retry was also recorded. Now mirrors the
  same `__redacted` placeholder. Helper extracted as
  `redactStructuredOutputArgsForRecording` so it's unit-testable.
- F2 `cli/config/config.ts`: `resolveJsonSchemaArg`'s `@path` reader
  now (a) `fs.statSync`s first to refuse non-regular files (FIFOs,
  character devices like `/dev/zero`, directories), (b) caps the
  schema file at 1 MiB so an attacker who can influence the path
  through a wrapping process can't OOM the run, and (c) on JSON
  parse failure for `@path` source emits a generic "content of
  <path> is not valid JSON" instead of echoing the SyntaxError —
  Node ≥18's SyntaxError embeds a ~10-char file-content prefix in
  its message, which would otherwise ride out on stderr through any
  wrapper that surfaces the error. Inline (non-`@path`) JSON keeps
  the SyntaxError detail because the user is the source.
- F15 `core/tools/tool-registry.ts`: `registerTool` now also checks
  the lazy `factories` map for name collisions, not just the eager
  `tools` map. An MCP server registering a tool whose name shadows a
  built-in lazy factory (e.g. `structured_output`) now gets
  auto-qualified to `mcp__<server>__<name>`, instead of silently
  winning the resolution. The synthetic structured-output tool no
  longer needs renaming for the corner case to be safe.

Targeted suite (13 changed-area test files): 883/886 pass — 3
pre-existing skips. Typecheck clean on both packages.

* fix: address 3 deepseek-v4-pro qreview comments on --json-schema PR

Three Suggestion-level comments from the latest /qreview pass.

**N1 — `schemaRootAcceptsObject` skips `if/then/else`** (cli/config/config.ts):
A schema like `{"if": true, "then": {"type": "string"}}` passed parse-time
gating but is unsatisfiable for object-typed tool args at runtime — the
model would loop until maxSessionTurns. Add a best-effort check for the
two decidable shapes:
- `if: true` → object MUST match `then`; if `then` excludes objects
  (boolean `false`, non-object `type`, etc.), reject at parse time.
- `if: false` → object MUST match `else` (`true` if absent); same check.
Object-schema `if` cases stay runtime-decidable and fall through to Ajv,
matching the existing best-effort scope on `not`. 4 new test cases pin
both reject and accept paths.

**N2 — subagent registries register `structured_output` too** (core/config/config.ts,
core/tools/agent/agent.ts, core/agents/backends/InProcessBackend.ts):
`createApprovalModeOverride` and `buildSubagentContextOverride` rebuild
the tool registry on a `Object.create(base)` config. `this.jsonSchema`
propagates through the prototype chain, so
`registerStructuredOutputIfRequested` was firing for every subagent
registry rebuild — but only `runNonInteractive`'s main / drain loops
detect a successful `structured_output` call as terminal. A subagent
that called the tool would receive "Session will end now" and then keep
running because its own loop has no terminator: wasted tokens, no
structured payload on stdout.

Add a `forSubAgent: true` option to `createToolRegistry` (alongside the
existing `skipDiscovery`), and propagate it from both subagent rebuild
sites. The structured-output registration helper short-circuits when
the flag is set. Bare-mode init does NOT set the flag, preserving the
F6 fix where `qwen --bare --json-schema X -p "..."` still gets the
synthetic tool. New test asserts the registry rebuilt with
`forSubAgent: true` registers READ_FILE / EDIT / SHELL but NOT
STRUCTURED_OUTPUT.

**N3 — TEXT-mode `structuredResult` not integration-tested** (nonInteractiveCli.test.ts):
All 8 existing `--json-schema` tests pin `OutputFormat.JSON` or
`STREAM_JSON`. TEXT (the default for `qwen -p ...`) has no integration
coverage, so a regression in
`BaseJsonOutputAdapter.buildResultMessage`'s
`hasStructured ? JSON.stringify(structuredResult) : resultText`
contract or in `JsonOutputAdapter.emitResult`'s text-mode
`process.stdout.write(`${result}\n`)` path would only surface to plain
`qwen -p` users. New test pins TEXT-mode behaviour: stdout is exactly
`${JSON.stringify(structuredArgs)}\n` — no JSON envelope, no event
log.

Targeted suite (13 spec files): 945/948 pass — 3 pre-existing skips.
Typecheck clean on both packages.

* fix(cli): narrow `not` rejection in schemaRootAcceptsObject

Address Critical review comment #3216123734.

`schemaRootAcceptsObject`'s `not` handler previously rejected any schema
whose `not.type` included `"object"`, regardless of what other
constraints `not` had. That's a false positive for schemas where the
extra constraints NARROW what `not` excludes:

    { "not": { "type": "object", "required": ["error"] } }

excludes only objects with an `error` key — the value `{}` satisfies
this schema fine, but the old check rejected it at parse time with
"--json-schema root must accept object-typed values".

Fix: only reject when `not` is exactly `{type: ...}` with no narrowing
siblings (the unambiguous "every object is excluded" case). When other
keywords are present (`required`, `properties`, `minProperties`,
`enum`, etc.), defer to Ajv at runtime — same best-effort scope as the
sibling `anyOf`/`oneOf`/`allOf` deep-content checks.

3 new test cases pin the fixed accept paths
(`{not:{type:"object",required:[...]}}`,
`{not:{type:"object",properties:...,required:[...]}}`,
`{not:{type:"object",minProperties:1}}`). The existing reject test for
bare `{not:{type:"object"}}` still passes.

* refactor: dedupe structured_output handling per qreview C1/C2/C3

Three Suggestion-level review comments from the latest /qreview pass.

**C1 — main-turn / drain-turn `structured_output` dispatch was
duplicated ~120 lines** (`nonInteractiveCli.ts`)

The two batch-handling sites had near-identical bodies (filter
`structured_output` from the batch when `--json-schema` is active →
iterate with `executeToolCall` → write to `structuredSubmission` on
first valid call → synthesise tool_result events for suppressed
siblings). The only meaningful difference was which `modelOverride`
binding the loop wrote to (session-scoped `modelOverride` for the
main turn vs per-drain-item `itemModelOverride`). Extracted
`processToolCallBatch(batchRequests, setModelOverride)` defined
inside `runNonInteractive`:

- Closes over session-scoped state (`adapter`, `config`,
  `abortController`, `options`, `structuredSubmission`,
  `executeToolCall`, `handleToolError`, `suppressedOutputBody`,
  the progress-handler helpers).
- Takes the `modelOverride` setter as the one call-site-specific
  parameter so the main turn binds to the session var and the drain
  binds to the per-item var.

Main-turn body went from ~120 lines to a single call; drain-turn body
likewise. Net file shrink ~80 lines, no behaviour change. All 42
existing structured-output tests still pass (including
`stops executing remaining tool calls...`,
`tries multiple structured_output calls in the same turn...`,
`synthesises tool_result for suppressed sibling calls...`,
`captures structured_output emitted from a drain-turn (queued notification)`).

**C2 + C3 — `{__redacted: '…'}` placeholder duplicated in two files**
(`telemetry/types.ts` + `core/geminiChat.ts`)

The `ToolCallEvent` constructor (for telemetry surfaces — OTLP /
QwenLogger / ui-telemetry / chat-recording UI event mirror) and
`redactStructuredOutputArgsForRecording` (for the on-disk
chat-recording JSONL) each had a verbatim copy of:

    { __redacted: 'structured_output payload (see stdout result)' }

If the redaction wording (or the `__redacted` key, or the placeholder
text) ever drifted between the two surfaces, the privacy contract
would be subtly broken on one and not the other.

Hoisted to `STRUCTURED_OUTPUT_REDACTED_ARGS` exported from
`packages/core/src/tools/syntheticOutput.ts`, imported in both sites.
The constant carries its rationale in a JSDoc block so future readers
see both call sites at once.

Targeted suite (13 spec files): 961/964 pass — 3 pre-existing skips.
Typecheck clean on both packages.

---------

Co-authored-by: wenshao <[email protected]>
This commit is contained in:
Shaojin Wenandwenshao authored and GitHub committed 2026-05-11 14:21:55 +08:00
1 parent e2f7661ef1
commit 84ecb5b8a3
25 files changed
+2653 -786

No files matched your search

+117
View File
@@ -254,6 +254,123 @@ describe('parseArguments', () => {
expect(argv.prompt).toBeUndefined();
});
it('rejects --json-schema combined with --acp', async () => {
// ACP runs an independent turn loop (runAcpAgent) that doesn't honour
// the synthetic structured_output terminal contract. The yargs check
// must reject the combination at parse time so users get an actionable
// error instead of silently watching the run never terminate.
process.argv = [
'node',
'script.js',
'--acp',
'--json-schema',
'{"type":"object"}',
];
const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => {
throw new Error('process.exit called');
});
mockWriteStderrLine.mockClear();
await expect(parseArguments()).rejects.toThrow('process.exit called');
expect(mockWriteStderrLine).toHaveBeenCalledWith(
expect.stringContaining('--json-schema cannot be used with --acp'),
);
mockExit.mockRestore();
});
it('rejects --json-schema combined with --experimental-acp (deprecated alias)', async () => {
// --experimental-acp is the deprecated alias; the same mutual-
// exclusion logic must apply or users get the silent-no-terminate
// behaviour the --acp check was added to prevent.
process.argv = [
'node',
'script.js',
'--experimental-acp',
'--json-schema',
'{"type":"object"}',
];
const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => {
throw new Error('process.exit called');
});
mockWriteStderrLine.mockClear();
await expect(parseArguments()).rejects.toThrow('process.exit called');
expect(mockWriteStderrLine).toHaveBeenCalledWith(
expect.stringContaining('--json-schema cannot be used with --acp'),
);
mockExit.mockRestore();
});
it('rejects --json-schema combined with --prompt-interactive (-i)', async () => {
// The interactive flow doesn't honour the synthetic-tool terminal
// contract — `structured_output` would just print "accepted" and
// leave the chat alive. The yargs check must reject this at parse
// time so users get an actionable message instead of a silently
// misbehaving run.
process.argv = [
'node',
'script.js',
'-i',
'do work then submit',
'--json-schema',
'{"type":"object"}',
];
const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => {
throw new Error('process.exit called');
});
mockWriteStderrLine.mockClear();
await expect(parseArguments()).rejects.toThrow('process.exit called');
expect(mockWriteStderrLine).toHaveBeenCalledWith(
expect.stringContaining(
'structured output only terminates the non-interactive flow',
),
);
mockExit.mockRestore();
});
it('rejects --json-schema combined with --input-format stream-json', async () => {
// The "first valid structured_output call ends the session"
// contract is incompatible with the long-lived stream-json input
// protocol. Also load-bearing: gemini.tsx's
// `process.exit(process.exitCode ?? 0)` plumbing in the stream-json
// branch explicitly relies on this rejection holding. Pair with
// --output-format stream-json because input/output formats must
// match (a separate yargs check fires first otherwise).
process.argv = [
'node',
'script.js',
'--input-format',
'stream-json',
'--output-format',
'stream-json',
'--json-schema',
'{"type":"object"}',
];
const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => {
throw new Error('process.exit called');
});
mockWriteStderrLine.mockClear();
await expect(parseArguments()).rejects.toThrow('process.exit called');
expect(mockWriteStderrLine).toHaveBeenCalledWith(
expect.stringContaining('first structured_output call ends the session'),
);
mockExit.mockRestore();
});
it('should parse --system-prompt', async () => {
process.argv = [
'node',
+239 -37
View File
@@ -30,9 +30,9 @@ import {
NativeLspService,
isBareMode,
isToolEnabled,
SchemaValidator,
type ConfigParameters,
type MCPServerConfig,
SchemaValidator,
} from '@qwen-code/qwen-code-core';
import { extensionsCommand } from '../commands/extensions.js';
import { hooksCommand } from '../commands/hooks.js';
@@ -170,6 +170,181 @@ export interface CliArgs {
inputFile?: string | undefined;
}
/**
* Returns true if the root of the given schema can accept a JSON object.
*
* JSON Schema applies sibling keywords conjunctively, so `type`, `anyOf`,
* `oneOf`, and `allOf` at the same level must EACH allow an object — they
* can't rescue one another. For example, `{type:"object", anyOf:[{type:"string"}]}`
* is unsatisfiable for any value because `type` requires object while
* `anyOf` requires string. Walk all four rather than returning on the
* first hit.
*
* For `anyOf` / `oneOf`, at least one branch must admit object (a value
* only has to match one branch). For `allOf`, every branch must admit
* object (a value has to match all of them). Root `$ref` is rejected
* unconditionally — Ajv applies `$ref` conjunctively with sibling
* keywords, so even `{type:"object", $ref:"#/$defs/Foo"}` is
* unsatisfiable when `Foo` resolves to a non-object schema. We don't
* follow refs ourselves (local-only resolution would still need to
* handle remote / recursive refs) so users wanting composition should
* inline the schema at the root or use `allOf`.
*
* The `$ref` rejection is **root-only**. Sub-schemas inside `anyOf` /
* `oneOf` / `allOf` recurse with `isRoot=false`, where a `$ref` is
* treated as opaque (assume-object-compatible) and deferred to Ajv at
* runtime — otherwise common composition shapes like
* `{anyOf:[{$ref:"#/$defs/Foo"}, {type:"string"}]}` would be wrongly
* rejected at parse time even though Ajv can resolve them.
*/
function schemaRootAcceptsObject(
schema: Record<string, unknown>,
isRoot = true,
): boolean {
if (isRoot && typeof schema['$ref'] === 'string') {
// Reject any root `$ref`. The previous "accept when sibling
// `type:"object"` is present" carve-out was unsound: Ajv applies
// both keywords, so `{type:"object", $ref:"#/$defs/Foo",
// $defs:{Foo:{type:"array"}}}` parses fine but no object argument
// can satisfy both at runtime — the model would loop forever on
// validation failures.
return false;
}
const rawType = schema['type'];
const typeIncludesObject =
rawType !== undefined &&
(Array.isArray(rawType) ? rawType : [rawType]).includes('object');
if (rawType !== undefined && !typeIncludesObject) {
return false;
}
// Root `const` / `enum` pin the value to specific literals. If those
// literals can never be a JSON object (e.g. `{const: 1}` or
// `{enum: ["a", "b"]}`), no object satisfies the schema — reject.
if ('const' in schema) {
const constVal = schema['const'];
if (
typeof constVal !== 'object' ||
constVal === null ||
Array.isArray(constVal)
) {
return false;
}
}
const enumVal = schema['enum'];
if (Array.isArray(enumVal)) {
const anyObjectMember = enumVal.some(
(v) => typeof v === 'object' && v !== null && !Array.isArray(v),
);
if (!anyObjectMember) return false;
}
// JSON Schema (draft-06+) treats `true` and `false` as valid subschemas
// for any keyword that accepts a schema: `true` matches every value,
// `false` matches nothing. Honour those alongside object subschemas so
// shapes like `{anyOf:[true]}` or `{allOf:[true,{type:"object"}]}` pass
// and `{anyOf:[false]}` is rejected.
const variantAcceptsObject = (v: unknown): boolean => {
if (v === true) return true;
if (v === false) return false;
if (typeof v === 'object' && v !== null && !Array.isArray(v)) {
// isRoot=false: nested branches don't trigger the root-only `$ref`
// rejection — the parent's keyword scope already pins the
// sub-schema's role to "candidate value type", and Ajv will
// resolve the ref at runtime.
return schemaRootAcceptsObject(v as Record<string, unknown>, false);
}
return false;
};
for (const key of ['anyOf', 'oneOf'] as const) {
const variants = schema[key];
if (Array.isArray(variants)) {
// Empty anyOf/oneOf is unsatisfiable per JSON Schema — no value can
// match a member of an empty union. Reject rather than treating it
// as "no constraint".
if (variants.length === 0) return false;
if (!variants.some(variantAcceptsObject)) return false;
}
}
const allOf = schema['allOf'];
if (Array.isArray(allOf) && allOf.length > 0) {
// allOf is conjunctive — `false` in any branch makes the schema
// unsatisfiable, `true` is neutral.
if (!allOf.every(variantAcceptsObject)) return false;
}
// Best-effort `not` handling: when `not` directly forbids object via its
// own `type` keyword (e.g. `{not:{type:"object"}}` or
// `{not:{type:["object","null"]}}`), the schema can never be satisfied
// by an object — reject. We don't try to do full satisfiability analysis
// for arbitrary `not` schemas (e.g. `not:{const:"foo"}` is fine, but
// `not:{anyOf:[{type:"object"},…]}` would also reject objects); those
// fall through to Ajv at runtime.
const notSchema = schema['not'];
if (
typeof notSchema === 'object' &&
notSchema !== null &&
!Array.isArray(notSchema)
) {
const notRecord = notSchema as Record<string, unknown>;
const notType = notRecord['type'];
if (notType !== undefined) {
const types = Array.isArray(notType) ? notType : [notType];
// If `not` is JUST `{type: "object"[…]}` (no additional keywords),
// every object value matches the `not` subschema and so gets
// excluded — schema is unsatisfiable for objects, reject.
//
// If `not` has additional constraints alongside `type` (e.g.
// `{not:{type:"object",required:["error"]}}`), those constraints
// NARROW what `not` excludes: only objects matching ALL of `not`'s
// keywords are rejected, so objects that fail any of the
// narrowing constraints survive. Example: `{}` satisfies
// `{not:{type:"object",required:["error"]}}` because the value
// lacks the `error` key. Rejecting at parse time would be a
// false positive — defer to Ajv at runtime.
if (types.includes('object') && Object.keys(notRecord).length === 1) {
return false;
}
}
}
// Best-effort `if/then/else` handling for the decidable cases. The
// semantics: if the value matches `if`, it must match `then`; otherwise
// it must match `else` (defaults to `true`). For root-acceptance we can
// only decide statically when `if` is itself a constant boolean
// subschema:
// `if: true` → every object matches `if`, so it MUST match `then`.
// `if: false` → no value matches `if`, so it must match `else`.
// Other shapes for `if` (object schemas) depend on the candidate value
// and fall through to Ajv at runtime — we can't decide acceptance
// without seeing the value.
if ('if' in schema) {
const ifSchema = schema['if'];
if (ifSchema === true) {
// Object MUST match `then` (if absent, defaults to `true`, no
// constraint on root acceptance).
const thenSchema = schema['then'];
if (thenSchema !== undefined && !variantAcceptsObject(thenSchema)) {
return false;
}
} else if (ifSchema === false) {
// Object MUST match `else` (if absent, defaults to `true`).
const elseSchema = schema['else'];
if (elseSchema !== undefined && !variantAcceptsObject(elseSchema)) {
return false;
}
}
// ifSchema is an object schema — runtime Ajv decides; do nothing.
}
// No narrowing at the root — lenient default, treated as object-compatible.
return true;
}
/** 4 MiB — well above any real schema, well below an accidental
* gigabyte-sized file that would OOM `fs.readFileSync` + `JSON.parse`.
*/
@@ -194,16 +369,25 @@ export function resolveJsonSchemaArg(
}
let payload: string;
let payloadSource: 'inline' | 'file' = 'inline';
let payloadSourcePath: string | undefined;
if (trimmed.startsWith('@')) {
const resolvedPath = resolvePath(trimmed.slice(1));
payloadSource = 'file';
payloadSourcePath = resolvedPath;
try {
// Cap the read size at 4 MiB. Real-world JSON schemas are well
// under this (the spec encourages decomposition via `$ref`); any
// multi-MiB file is almost certainly a wrong-path mistake or a
// typo'd argument. Without a cap, a multi-GB file (e.g. accidental
// `--json-schema @./node_modules/.cache/*.json`) loads into memory
// before `JSON.parse` even runs and OOMs the CLI.
// Stat first so we can refuse non-regular files (directories,
// character devices like `/dev/zero`, FIFOs that would block
// synchronously) and cap by size before pulling bytes into memory.
// The cap (`MAX_JSON_SCHEMA_FILE_BYTES`) is set well above any real
// schema and well below an accidental gigabyte-sized file that
// would OOM `fs.readFileSync` + `JSON.parse`.
const stat = fs.statSync(resolvedPath);
if (!stat.isFile()) {
throw new FatalConfigError(
`--json-schema "@${resolvedPath}" must be a regular file.`,
);
}
if (stat.size > MAX_JSON_SCHEMA_FILE_BYTES) {
throw new FatalConfigError(
`--json-schema file "${resolvedPath}" is ${stat.size} bytes ` +
@@ -230,6 +414,16 @@ export function resolveJsonSchemaArg(
try {
parsed = JSON.parse(payload);
} catch (err) {
// For inline JSON the user IS the source — echoing the SyntaxError
// (which on Node ≥18 embeds a 10-char input snippet) is fine. For
// @path, the error message would leak a prefix of the file's bytes
// through stderr to whatever wrapping process surfaces it; emit a
// generic message instead.
if (payloadSource === 'file') {
throw new FatalConfigError(
`--json-schema content of "${payloadSourcePath}" is not valid JSON.`,
);
}
throw new FatalConfigError(
`--json-schema is not valid JSON: ${
err instanceof Error ? err.message : String(err)
@@ -243,6 +437,23 @@ export function resolveJsonSchemaArg(
);
}
// The schema will be installed as a TOOL PARAMETER schema. All function-
// calling APIs (Gemini/OpenAI/Anthropic) require tool arguments to be a
// JSON object, so a schema that cannot accept objects registers an
// unusable synthetic tool the model could never satisfy. `schemaRootAcceptsObject`
// walks `type`/`const`/`enum`/`anyOf`/`oneOf`/`allOf`/`not`/`if` (with
// best-effort decidable cases for the harder shapes); the strict Ajv
// compile below catches structural validity. The two together cover both
// "schema can be parsed" and "schema can be satisfied by an object value".
if (!schemaRootAcceptsObject(parsed as Record<string, unknown>)) {
throw new FatalConfigError(
'--json-schema root must accept object-typed values (tool parameters ' +
'are always JSON objects). At least one branch of a root anyOf/oneOf ' +
'must be satisfiable by an object, and a root `type` (when present) ' +
'must include "object".',
);
}
// Ajv compile-time validation. SchemaValidator.validate is deliberately
// lenient at runtime (falls back to no-op on compile failure to support
// exotic MCP schemas) — but `--json-schema` is explicit user intent, so
@@ -254,30 +465,6 @@ export function resolveJsonSchemaArg(
);
}
// The schema becomes the parameter schema of the synthetic
// structured_output tool, and tool-call arguments are object-shaped.
// A schema like `{"type":"string"}` would compile fine but be
// unsatisfiable as a tool-call argument — fail at parse time so the
// user sees the contract violation immediately instead of at runtime.
//
// We only check the direct `type` field (or array of types). Deeper
// analysis of `anyOf`/`oneOf`/`not` is intentionally not done here:
// the strict-Ajv compile is the right place for full structural
// validation, and a partial check would either give false reassurance
// or wrongly reject valid composed schemas. Schemas with no top-level
// `type` are allowed through (covers `{}`, plain `properties`, etc).
const schemaType = (parsed as { type?: unknown }).type;
const isObjectAllowed =
schemaType === undefined ||
schemaType === 'object' ||
(Array.isArray(schemaType) && schemaType.includes('object'));
if (!isObjectAllowed) {
throw new FatalConfigError(
`--json-schema top-level type must include "object" (got ${JSON.stringify(schemaType)}); ` +
'wrap your value under an object property if you need a non-object payload.',
);
}
return parsed as Record<string, unknown>;
}
@@ -729,12 +916,24 @@ export async function parseArguments(): Promise<CliArgs> {
if (argv['promptInteractive']) {
return '--json-schema cannot be used with --prompt-interactive (-i); structured output only terminates the non-interactive flow.';
}
// Stream-json input runs through runNonInteractiveStreamJson,
// which doesn't honor the structured-output termination
// contract. Reject the combination explicitly so users see
// the mismatch at parse time instead of confusion at runtime.
if (argv['inputFormat'] === 'stream-json') {
return '--json-schema cannot be used with --input-format stream-json; structured output is a single-shot contract incompatible with stream-json input.';
// The "first valid structured_output call ends the session"
// contract assumes a single one-shot prompt. Stream-json
// input keeps the process open waiting for more protocol
// messages, so terminating on the first call would silently
// drop subsequent prompts. Refuse the combination here
// rather than letting the run race to whichever message
// wins.
return '--json-schema cannot be used with --input-format stream-json; the "first structured_output call ends the session" contract is incompatible with the long-lived stream-json input protocol.';
}
if (argv['acp'] || argv['experimentalAcp']) {
// ACP runs an external IDE/Zed protocol on its own turn loop
// (runAcpAgent), which doesn't honour the synthetic
// structured_output contract. Without this check the tool
// would register but its "session ends now" llmContent would
// just be relayed back into the ACP chat, leaving the run
// open and silently ignoring --json-schema.
return '--json-schema cannot be used with --acp; structured output is only honoured by the headless non-interactive flow.';
}
const hasPrompt = !!argv['prompt'];
const query = argv['query'] as string | string[] | undefined;
@@ -744,7 +943,10 @@ export async function parseArguments(): Promise<CliArgs> {
// Allow stdin piping (`echo "..." | qwen --json-schema ...`):
// when stdin is not a TTY, the prompt is supplied via the pipe
// and headless mode runs normally. Only reject true interactive
// invocations with neither flag nor positional nor pipe.
// invocations with neither flag nor positional nor pipe — the
// synthetic tool's "session ends now" llmContent has no
// termination handler in the TUI loop, so silently launching
// the TUI would strand the run.
const stdinIsPiped = !process.stdin.isTTY;
if (!hasPrompt && !hasPositionalQuery && !stdinIsPiped) {
return '--json-schema only applies to non-interactive mode; pass a prompt via -p, as a positional argument, or piped via stdin.';
+360 -23
View File
@@ -58,11 +58,89 @@ describe('resolveJsonSchemaArg', () => {
).toThrow(/could not read/);
});
it('rejects @path that resolves to a directory', () => {
// stat-based "must be a regular file" guard. Without this, a path
// pointing at a directory would surface a less-specific Node EISDIR
// error from the readFileSync call (or worse, on systems where
// readFileSync on a directory does not error).
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'qwen-schema-dir-'));
try {
expect(() => resolveJsonSchemaArg(`@${tmp}`)).toThrow(
/must be a regular file/,
);
} finally {
fs.rmSync(tmp, { recursive: true, force: true });
}
});
it('rejects @path schema files that exceed the size cap', () => {
// Defence against a wrapper that forwards a user-supplied path into
// `qwen --json-schema "$X"` where X is e.g. `@/dev/zero` or any
// pathologically large file. We pre-check size via fs.statSync so the
// huge buffer never gets allocated.
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'qwen-schema-big-'));
const file = path.join(tmp, 'huge.json');
// Cap is 4 MiB; write 4 MiB + 1 byte to trip it.
fs.writeFileSync(file, Buffer.alloc(4 * 1024 * 1024 + 1, 0x20));
try {
expect(() => resolveJsonSchemaArg(`@${file}`)).toThrow(
/Refusing to read/,
);
} finally {
fs.rmSync(tmp, { recursive: true, force: true });
}
});
it('does not echo the JSON parse error message for @path source', () => {
// The Node ≥18 SyntaxError message for `JSON.parse('hello world…')`
// embeds a ~10-char prefix of the input. For inline JSON that's
// fine — the user typed it themselves — but for @path it would leak
// a prefix of the referenced file through stderr to any wrapper
// that surfaces qwen's error output. Sanitise by emitting a generic
// "content of <path> is not valid JSON" instead.
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'qwen-schema-bad-'));
const file = path.join(tmp, 'leaky.txt');
const secretContent = 'SECRET_TOKEN_PREFIX hello world';
fs.writeFileSync(file, secretContent);
try {
let caught: Error | undefined;
try {
resolveJsonSchemaArg(`@${file}`);
} catch (err) {
caught = err as Error;
}
expect(caught).toBeDefined();
expect(caught!.message).toMatch(/is not valid JSON/);
// The file's contents must NOT appear in the error message.
expect(caught!.message).not.toContain('SECRET_TOKEN_PREFIX');
expect(caught!.message).not.toContain('hello worl');
} finally {
fs.rmSync(tmp, { recursive: true, force: true });
}
});
it('still echoes JSON parse error detail for inline (non-@path) source', () => {
// Inline JSON is the user's own input — keeping the SyntaxError detail
// is helpful for debugging typos, and there's no third-party file
// content to leak.
let caught: Error | undefined;
try {
resolveJsonSchemaArg('{"foo":}');
} catch (err) {
caught = err as Error;
}
expect(caught).toBeDefined();
expect(caught!.message).toMatch(/--json-schema is not valid JSON/);
// Should mention the JSON parser detail (echoes SyntaxError text).
expect(caught!.message).not.toBe('--json-schema is not valid JSON: ');
});
it('throws when schema is syntactically JSON but invalid as a JSON Schema', () => {
// `type` must be a string or array, not a number.
expect(() => resolveJsonSchemaArg('{"type": 42}')).toThrow(
/not a valid JSON Schema/,
);
// The root-type check fires first for an integer `type`; drop type
// entirely to exercise the Ajv compile-path rejection instead.
expect(() =>
resolveJsonSchemaArg('{"properties":{"foo":{"type":42}}}'),
).toThrow(/not a valid JSON Schema/);
});
it('accepts a minimal empty-object schema', () => {
@@ -77,34 +155,293 @@ describe('resolveJsonSchemaArg', () => {
expect(schema).toBeDefined();
});
it('rejects schemas whose top-level type is not "object"', () => {
// The schema becomes structured_output's parameter schema; tool args
// are object-shaped, so non-object roots can never be satisfied.
expect(() => resolveJsonSchemaArg('{"type":"string"}')).toThrow(
/top-level type must include "object"/,
);
it('rejects a schema whose root type is not object', () => {
expect(() => resolveJsonSchemaArg('{"type":"array"}')).toThrow(
/top-level type must include "object"/,
/must accept object-typed values/,
);
expect(() => resolveJsonSchemaArg('{"type":"string"}')).toThrow(
/must accept object-typed values/,
);
});
it('accepts type-arrays that include "object"', () => {
// `{"type":["object","null"]}` is a common nullable-object pattern.
// The earlier guard rejected anything that wasn't exactly the string
// "object", which broke this idiom.
it('accepts a schema whose type array includes "object"', () => {
// Rare but valid; don't over-restrict nullable object roots.
const schema = resolveJsonSchemaArg('{"type":["object","null"]}');
expect(schema).toEqual({ type: ['object', 'null'] });
});
it('accepts a schema without an explicit root type', () => {
// Absent type is tolerated — Ajv treats it as "anything" which covers
// the object case the model will actually submit.
const schema = resolveJsonSchemaArg('{"properties":{"foo":{}}}');
expect(schema).toBeDefined();
});
it('rejects root anyOf where no branch accepts object', () => {
expect(() =>
resolveJsonSchemaArg('{"anyOf":[{"type":"array"},{"type":"string"}]}'),
).toThrow(/must accept object-typed values/);
});
it('rejects root oneOf where no branch accepts object', () => {
expect(() =>
resolveJsonSchemaArg('{"oneOf":[{"type":"number"},{"type":"boolean"}]}'),
).toThrow(/must accept object-typed values/);
});
it('accepts root anyOf when at least one branch accepts object', () => {
const schema = resolveJsonSchemaArg(
'{"anyOf":[{"type":"object"},{"type":"string"}]}',
);
expect(schema).toBeDefined();
});
it('accepts nested anyOf/oneOf chains where a deep branch accepts object', () => {
// The recursion should see through one level of nesting.
const schema = resolveJsonSchemaArg(
'{"anyOf":[{"oneOf":[{"type":"object"}]},{"type":"string"}]}',
);
expect(schema).toBeDefined();
});
it('rejects type:"object" combined with an anyOf that excludes object', () => {
// type and anyOf are AND'd at the same level — type:"object" alone is
// not enough if a sibling anyOf forbids every object branch. Without
// this check the synthetic tool would register an unsatisfiable schema.
expect(() =>
resolveJsonSchemaArg(
'{"type":"object","anyOf":[{"type":"string"},{"type":"number"}]}',
),
).toThrow(/must accept object-typed values/);
});
it('accepts type:"object" combined with anyOf where one branch admits object', () => {
const schema = resolveJsonSchemaArg(
'{"type":"object","anyOf":[{"type":"object","properties":{"a":{"type":"string"}}},{"type":"object","properties":{"b":{"type":"number"}}}]}',
);
expect(schema).toBeDefined();
});
it('rejects any root $ref, even with a sibling type:"object" anchor', () => {
// Ajv applies `$ref` conjunctively with sibling keywords, so a sibling
// `type:"object"` is NOT enough to make the schema satisfiable — when
// the referenced subschema is non-object, the resulting AND is
// unsatisfiable at runtime. We reject root `$ref` outright rather than
// following the reference ourselves (local-only resolution would still
// have to handle remote / recursive refs).
expect(() =>
resolveJsonSchemaArg(
'{"$ref":"#/$defs/Foo","$defs":{"Foo":{"type":"array"}}}',
),
).toThrow(/must accept object-typed values/);
expect(() =>
resolveJsonSchemaArg(
'{"type":"object","$ref":"#/$defs/Foo","$defs":{"Foo":{"type":"array"}}}',
),
).toThrow(/must accept object-typed values/);
// Even when the referenced schema IS object-shaped, we still reject —
// the contract for `--json-schema` is "the root schema describes the
// tool args directly", not "follow these refs". Users wanting
// composition should inline at the root or use `allOf`.
expect(() =>
resolveJsonSchemaArg(
'{"type":"object","$ref":"#/$defs/Foo","$defs":{"Foo":{"type":"object","properties":{"a":{"type":"string"}}}}}',
),
).toThrow(/must accept object-typed values/);
});
it('rejects allOf where any branch forbids object at the root', () => {
// allOf is conjunctive — every branch must accept object. A schema
// like `allOf:[{type:"object"}, {type:"string"}]` is unsatisfiable.
expect(() =>
resolveJsonSchemaArg('{"allOf":[{"type":"object"},{"type":"string"}]}'),
).toThrow(/must accept object-typed values/);
});
it('accepts allOf where every branch admits object', () => {
const schema = resolveJsonSchemaArg(
'{"allOf":[{"type":"object","properties":{"a":{"type":"string"}}},{"type":"object","required":["a"]}]}',
);
expect(schema).toBeDefined();
});
it('rejects a root `not` that directly forbids object', () => {
// `not:{type:"object"}` excludes every object value, so the schema is
// unsatisfiable for tool-call args. Best-effort check — only inspects
// `not.type`; deeper negated patterns fall through to Ajv at runtime.
expect(() => resolveJsonSchemaArg('{"not":{"type":"object"}}')).toThrow(
/must accept object-typed values/,
);
expect(() =>
resolveJsonSchemaArg('{"not":{"type":["object","null"]}}'),
).toThrow(/must accept object-typed values/);
});
it('accepts a root `not` whose negated type does not exclude object', () => {
// `not:{type:"string"}` only forbids strings — objects are still fine.
const schema = resolveJsonSchemaArg('{"not":{"type":"string"}}');
expect(schema).toBeDefined();
});
it('accepts root `not:{type:"object", ...narrowing}` because narrowing keywords leave some objects satisfiable', () => {
// `not:{type:"object",required:["error"]}` only excludes objects
// that have an `error` key. An object like `{}` is NOT excluded
// (it doesn't match the `required` constraint), so the schema is
// satisfiable for at least one object value.
//
// The previous parse-time check looked only at `not.type` and
// rejected this as "must accept object-typed values" — a false
// positive. The fix: only reject when `not` is exactly
// `{type: ...}` with no narrowing siblings; otherwise defer to
// Ajv at runtime.
expect(
resolveJsonSchemaArg('{"not":{"type":"object","required":["error"]}}'),
).toBeDefined();
expect(
resolveJsonSchemaArg(
'{"type":["object","null"],"properties":{"x":{"type":"string"}}}',
'{"not":{"type":"object","properties":{"k":{"type":"string"}},"required":["k"]}}',
),
).toEqual({
type: ['object', 'null'],
properties: { x: { type: 'string' } },
});
).toBeDefined();
expect(
resolveJsonSchemaArg('{"not":{"type":"object","minProperties":1}}'),
).toBeDefined();
});
it('rejects type-arrays that do not include "object"', () => {
expect(() => resolveJsonSchemaArg('{"type":["string","number"]}')).toThrow(
/top-level type must include "object"/,
it('rejects a root `const` whose value is not an object', () => {
expect(() => resolveJsonSchemaArg('{"const":1}')).toThrow(
/must accept object-typed values/,
);
expect(() => resolveJsonSchemaArg('{"const":"hello"}')).toThrow(
/must accept object-typed values/,
);
expect(() => resolveJsonSchemaArg('{"const":[1,2]}')).toThrow(
/must accept object-typed values/,
);
});
it('accepts a root `const` whose value is an object', () => {
const schema = resolveJsonSchemaArg(
'{"const":{"summary":"hello","risk":"low"}}',
);
expect(schema).toBeDefined();
});
it('rejects a root `enum` with no object members', () => {
expect(() => resolveJsonSchemaArg('{"enum":[1,2,"three"]}')).toThrow(
/must accept object-typed values/,
);
// Empty enum admits nothing — also reject.
expect(() => resolveJsonSchemaArg('{"enum":[]}')).toThrow(
/must accept object-typed values/,
);
});
it('accepts a root `enum` when at least one member is an object', () => {
const schema = resolveJsonSchemaArg(
'{"enum":[{"summary":"a","risk":"low"},{"summary":"b","risk":"high"}]}',
);
expect(schema).toBeDefined();
});
it('rejects an empty root anyOf / oneOf as unsatisfiable', () => {
expect(() => resolveJsonSchemaArg('{"anyOf":[]}')).toThrow(
/must accept object-typed values/,
);
expect(() => resolveJsonSchemaArg('{"oneOf":[]}')).toThrow(
/must accept object-typed values/,
);
});
it('accepts boolean subschemas in anyOf where any branch is true', () => {
// `true` matches every value (per JSON Schema 2019-09+), so it admits
// objects. `{anyOf:[true]}` should pass.
const a = resolveJsonSchemaArg('{"anyOf":[true]}');
expect(a).toBeDefined();
const b = resolveJsonSchemaArg('{"anyOf":[false,true]}');
expect(b).toBeDefined();
const c = resolveJsonSchemaArg('{"anyOf":[true,{"type":"string"}]}');
expect(c).toBeDefined();
});
it('rejects anyOf where every branch is `false`', () => {
// `false` matches nothing, so an anyOf of all-false is unsatisfiable.
expect(() => resolveJsonSchemaArg('{"anyOf":[false]}')).toThrow(
/must accept object-typed values/,
);
expect(() => resolveJsonSchemaArg('{"anyOf":[false,false]}')).toThrow(
/must accept object-typed values/,
);
});
it('accepts $ref nested inside anyOf / oneOf / allOf branches', () => {
// Root $ref is rejected unconditionally (Ajv applies it conjunctively
// with siblings), but $ref *inside* a composition branch is opaque
// at parse time — Ajv will resolve it at runtime. Refusing nested
// refs would block common $defs/$ref composition shapes.
const a = resolveJsonSchemaArg(
'{"anyOf":[{"$ref":"#/$defs/Foo"},{"type":"string"}],"$defs":{"Foo":{"type":"object"}}}',
);
expect(a).toBeDefined();
const b = resolveJsonSchemaArg(
'{"oneOf":[{"$ref":"#/$defs/A"},{"$ref":"#/$defs/B"}],"$defs":{"A":{"type":"object"},"B":{"type":"object"}}}',
);
expect(b).toBeDefined();
const c = resolveJsonSchemaArg(
'{"allOf":[{"$ref":"#/$defs/Bar"},{"type":"object"}],"$defs":{"Bar":{"type":"object"}}}',
);
expect(c).toBeDefined();
});
it('handles boolean subschemas in allOf', () => {
// `true` is neutral in allOf, `false` makes the whole schema unsatisfiable.
const ok = resolveJsonSchemaArg('{"allOf":[true,{"type":"object"}]}');
expect(ok).toBeDefined();
expect(() =>
resolveJsonSchemaArg('{"allOf":[false,{"type":"object"}]}'),
).toThrow(/must accept object-typed values/);
expect(() => resolveJsonSchemaArg('{"allOf":[false]}')).toThrow(
/must accept object-typed values/,
);
});
it('rejects if/then/else when the decidable branch admits no objects', () => {
// `if: true` reduces root acceptance to `then`'s acceptance.
// `if: false` reduces it to `else`'s acceptance. Object schemas in
// `if` are runtime-decidable only and fall through to Ajv.
expect(() =>
resolveJsonSchemaArg('{"if":true,"then":{"type":"string"}}'),
).toThrow(/must accept object-typed values/);
expect(() => resolveJsonSchemaArg('{"if":true,"then":false}')).toThrow(
/must accept object-typed values/,
);
expect(() =>
resolveJsonSchemaArg('{"if":false,"else":{"type":"array"}}'),
).toThrow(/must accept object-typed values/);
expect(() => resolveJsonSchemaArg('{"if":false,"else":false}')).toThrow(
/must accept object-typed values/,
);
});
it('accepts if/then/else when the decidable branch admits objects', () => {
// `if: true` + object-compatible `then` passes (parse-time
// schemaRootAcceptsObject reduces to checking `then`).
expect(
resolveJsonSchemaArg('{"if":true,"then":{"type":"object"}}'),
).toBeDefined();
// `if: false` + object-compatible `else`.
expect(
resolveJsonSchemaArg('{"if":false,"else":{"type":"object"}}'),
).toBeDefined();
// Object schema for `if` — runtime-decidable; defer to Ajv. We
// accept at parse time even when `then` excludes object, because
// an object value may not match `if` and so isn't bound by `then`.
expect(
resolveJsonSchemaArg(
'{"if":{"type":"object","properties":{"k":{"const":"x"}}},"then":{"type":"object","properties":{"v":{"type":"string"}}}}',
),
).toBeDefined();
// (The degenerate `{if:true}` / `{if:false}` shapes — no `then` and
// no `else` — are rejected by Ajv strict mode as meaningless rather
// than by schemaRootAcceptsObject; that's fine.)
});
});
+96 -1
View File
@@ -279,7 +279,7 @@ describe('gemini.tsx main function', () => {
vi.mocked(loadSandboxConfig).mockResolvedValue(undefined);
vi.mocked(relaunchAppInChildProcess).mockResolvedValue(undefined);
vi.mocked(loadCliConfig).mockResolvedValue(configStub);
vi.spyOn(nonInteractiveModule, 'runNonInteractive').mockResolvedValue();
vi.spyOn(nonInteractiveModule, 'runNonInteractive').mockResolvedValue(0);
try {
await main();
@@ -711,6 +711,101 @@ describe('gemini.tsx main function kitty protocol', () => {
processOnceSpy.mockRestore();
processExitSpy.mockRestore();
});
it('rejects --json-schema when running in interactive (TUI) mode', async () => {
// The synthetic structured_output tool only terminates the run inside
// runNonInteractive. In TUI mode it's an inert tool that prints
// "accepted" and leaves the chat alive — silently stranding the run.
// gemini.tsx must reject this combination at runtime (parse-time
// gating can't catch the no-prompt-on-TTY case because stdin
// availability isn't probed yet at parse time).
const { loadCliConfig, parseArguments } = await import(
'./config/config.js'
);
const { loadSettings } = await import('./config/settings.js');
const cleanupModule = await import('./utils/cleanup.js');
const callOrder: string[] = [];
const exitCodes: Array<string | number | null | undefined> = [];
mockWriteStderrLine.mockClear();
mockWriteStderrLine.mockImplementation(() => {
callOrder.push('writeStderrLine');
});
const runExitCleanupMock = vi.mocked(cleanupModule.runExitCleanup);
runExitCleanupMock.mockReset();
runExitCleanupMock.mockImplementation(async () => {
callOrder.push('runExitCleanup');
});
const processExitSpy = vi.spyOn(process, 'exit').mockImplementation(((
code?: string | number | null,
) => {
callOrder.push('processExit');
exitCodes.push(code);
throw new MockProcessExitError(code);
}) as unknown as typeof process.exit);
vi.mocked(loadCliConfig).mockResolvedValue({
isInteractive: () => true,
getJsonSchema: () => ({ type: 'object' }),
getQuestion: () => '',
getSandbox: () => false,
getDebugMode: () => false,
getListExtensions: () => false,
getMcpServers: () => ({}),
initialize: vi.fn(),
getIdeMode: () => false,
getExperimentalZedIntegration: () => false,
getScreenReader: () => false,
getGeminiMdFileCount: () => 0,
getWarnings: () => [],
getModelsConfig: () => ({ getCurrentAuthType: () => null }),
getUsageStatisticsEnabled: () => true,
getSessionId: () => 'test-session-id',
shutdown: vi.fn(),
} as unknown as Config);
vi.mocked(loadSettings).mockReturnValue({
errors: [],
merged: {
advanced: {},
security: { auth: {} },
ui: {},
},
setValue: vi.fn(),
forScope: () => ({ settings: {}, originalSettings: {}, path: '' }),
migrationWarnings: [],
getUserHooks: () => undefined,
getProjectHooks: () => undefined,
} as never);
vi.mocked(parseArguments).mockResolvedValue({} as never);
try {
await main();
} catch (e) {
if (!(e instanceof MockProcessExitError)) throw e;
}
// The headless-only message must reach stderr…
expect(mockWriteStderrLine).toHaveBeenCalledWith(
expect.stringContaining('--json-schema is a headless-only flag'),
);
// …runExitCleanup must run before exit so MCP subprocesses /
// telemetry exporters registered earlier get torn down…
expect(runExitCleanupMock).toHaveBeenCalledTimes(1);
// …and exit must be 1, not 0.
expect(processExitSpy).toHaveBeenCalledWith(1);
// Order: stderr → cleanup → exit. A regression that swapped any of
// these (cleanup before stderr; exit without cleanup; exit 0
// instead of 1) would silently strand TUI users.
expect(callOrder).toEqual([
'writeStderrLine',
'runExitCleanup',
'processExit',
]);
expect(exitCodes).toEqual([1]);
processExitSpy.mockRestore();
});
});
describe('validateDnsResolutionOrder', () => {
+39 -9
View File
@@ -711,6 +711,24 @@ export async function main() {
finalizeStartupProfile(config.getSessionId());
if (config.isInteractive()) {
// --json-schema is a headless-only contract: the synthetic
// structured_output tool only terminates the run inside
// runNonInteractive's main/drain loops. In TUI mode the same call
// would just emit "Structured output accepted." and keep the chat
// alive, which silently strands the user's run. Parse-time gating
// can't catch this case (`qwen --json-schema '...'` on a TTY with
// no prompt routes to interactive only after stdin TTY detection),
// so reject here before the UI launches.
if (config.getJsonSchema?.()) {
writeStderrLine(
'Error: --json-schema is a headless-only flag. Provide a one-shot prompt via -p / --prompt or pipe one in via stdin.',
);
// Run cleanup so MCP subprocesses + telemetry exporters that the
// earlier initializeApp() / loadCliConfig() registered get shut
// down — process.exit() doesn't drain them on its own.
await runExitCleanup();
process.exit(1);
}
// Need kitty detection to be complete before we can start the interactive UI.
await kittyProtocolDetectionComplete;
// Drain the auto-theme probe before render so the OSC 11 response is
@@ -773,9 +791,15 @@ export async function main() {
settings,
);
await runExitCleanup();
// Honor any exitCode set by the run (e.g. --json-schema plain-text
// path sets it to 1). Hardcoding 0 here would silently mask non-zero
// shell exits so the caller can't detect failures.
// `runNonInteractiveStreamJson` doesn't return an explicit exit
// code yet, so a cleanup task that mutates `process.exitCode`
// could clobber a non-zero failure signal. This is currently safe
// because `--json-schema` is rejected at parse time when combined
// with `--input-format stream-json` (see the yargs `.check` in
// resolveCliGenerationConfig), so structured-output failures
// never reach this branch. If a future stream-json equivalent of
// structured output is added, plumb the exit code through the
// function's return value the way `runNonInteractive` below does.
process.exit(process.exitCode ?? 0);
}
@@ -797,13 +821,19 @@ export async function main() {
debugLogger.debug(`Session ID: ${config.getSessionId()}`);
await runNonInteractive(nonInteractiveConfig, settings, input, prompt_id);
// Call cleanup before process.exit, which causes cleanup to not run
const exitCode = await runNonInteractive(
nonInteractiveConfig,
settings,
input,
prompt_id,
);
// Call cleanup before process.exit, which causes cleanup to not run.
// Capture the exit code BEFORE cleanup so any cleanup task that
// mutates process.exitCode can't silently turn a structured-output
// failure (or other explicit non-zero return from runNonInteractive)
// into a zero exit.
await runExitCleanup();
// Honor any exitCode set by the run (e.g. --json-schema plain-text
// path sets it to 1). Hardcoding 0 here would silently mask non-zero
// shell exits so the caller can't detect failures.
process.exit(process.exitCode ?? 0);
process.exit(exitCode);
}
}
File diff suppressed because it is too large. Load diff
+342 -232
View File
@@ -43,6 +43,43 @@ import {
handleCancellationError,
handleMaxTurnsExceededError,
} from './utils/errors.js';
const debugLogger = createDebugLogger('NON_INTERACTIVE_CLI');
/**
* Maximum wait, in milliseconds, for in-flight background tasks to emit
* their terminal `task_notification` after `abortAll()` on the
* structured-output success path. Tasks are marked cancelled
* synchronously by `abortAll`, but the natural task handler emits the
* notification on a later microtask — without a brief holdback the
* structured-output run would silently drop those events. Capped so a
* slow agent can't block exit indefinitely.
*/
const STRUCTURED_SHUTDOWN_HOLDBACK_MS = 500;
/**
* Body of the synthesised `tool_result` for a `tool_use` block that was
* suppressed because a sibling `structured_output` call took precedence
* as the terminal output for the same turn.
*
* Two variants — the success-path body drops the trailing "Re-issue this
* call in a separate turn if needed." sentence because the session
* terminates immediately after synthesis (no model or SDK consumer can
* act on the advice). The retry-path body keeps it: when the structured
* call failed validation, the model is about to receive these parts in
* the next turn and may legitimately re-issue the suppressed call.
*
* Shared between the main-turn and drain-turn synthesis sites so a
* future wording change can't desync them.
*/
const SUPPRESSED_OUTPUT_SUCCESS =
"Skipped: this turn's structured_output contract took precedence as the terminal output.";
const SUPPRESSED_OUTPUT_RETRY = `${SUPPRESSED_OUTPUT_SUCCESS} Re-issue this call in a separate turn if needed.`;
function suppressedOutputBody(structuredCaptured: boolean): string {
return structuredCaptured
? SUPPRESSED_OUTPUT_SUCCESS
: SUPPRESSED_OUTPUT_RETRY;
}
import {
normalizePartList,
extractPartsFromUserMessage,
@@ -52,8 +89,6 @@ import {
computeUsageFromMetrics,
} from './utils/nonInteractiveHelpers.js';
const debugLogger = createDebugLogger('NON_INTERACTIVE_CLI');
// Human-readable labels for the detectors that can fire mid-stream.
// Surfaced to stderr in TEXT mode so a headless run that halts on a loop
// doesn't exit with empty stdout and no explanation — see PR #3236 review.
@@ -156,8 +191,8 @@ export async function runNonInteractive(
input: string,
prompt_id: string,
options: RunNonInteractiveOptions = {},
): Promise<void> {
return promptIdContext.run(prompt_id, async () => {
): Promise<number> {
return promptIdContext.run(prompt_id, async (): Promise<number> => {
// Create output adapter based on format
let adapter: JsonOutputAdapterInterface;
const outputFormat = config.getOutputFormat();
@@ -289,7 +324,7 @@ export async function runNonInteractive(
config,
startTimeMs: startTime,
});
return;
return slashCommandResult.messageType === 'error' ? 1 : 0;
}
case 'stream_messages':
throw new FatalInputError(
@@ -303,7 +338,7 @@ export async function runNonInteractive(
config,
startTimeMs: startTime,
});
return;
return 1;
}
case 'no_command':
break;
@@ -417,12 +452,237 @@ export async function runNonInteractive(
let isFirstTurn = true;
let modelOverride: string | undefined;
// Session-scoped because the synthetic `structured_output` tool can
// be invoked from EITHER the main assistant-turn loop or from a
// drain-turn (queued notification / cron prompt); whichever fires
// first wins, and both paths need to surface the same structured
// result envelope.
let structuredSubmission: unknown = undefined;
// Captures the first ~200 chars of model-emitted plain text across
// turns. Used only to enrich the --json-schema "produced plain
// text" error: the user/operator gets a hint of what the model
// actually said instead of a static, context-free message.
let plainTextPreview = '';
const PLAIN_TEXT_PREVIEW_LIMIT = 200;
// Shared terminal block for the structured-output success
// contract. Both the main-turn loop and the drain-turn post-loop
// previously reproduced this block verbatim
// (`registry.abortAll()` → bounded holdback for in-flight
// background-task `task_notification` events → flush localQueue →
// finalize one-shot monitors → `adapter.emitResult` → return 0).
// `finalizeOneShotMonitors` is idempotent (the
// `oneShotMonitorsFinalized` guard makes the second call a
// no-op), so unconditional invocation is safe even when the drain
// path already finalized monitors before reaching here.
const emitStructuredSuccess = async (): Promise<0> => {
registry.abortAll();
// `abortAll()` marks each task `cancelled` synchronously, but
// the matching `task_notification` is emitted later by the
// task's natural handler. Hold back briefly (capped at
// STRUCTURED_SHUTDOWN_HOLDBACK_MS) so consumers see every
// `task_started` paired with its terminal notification, without
// blocking exit on a slow agent that the user has already
// declared done.
const holdbackDeadline = Date.now() + STRUCTURED_SHUTDOWN_HOLDBACK_MS;
while (
Date.now() < holdbackDeadline &&
registry.hasUnfinalizedTasks()
) {
await new Promise((r) => setTimeout(r, 50));
}
flushQueuedNotificationsToSdk(localQueue);
finalizeOneShotMonitors();
const metrics = uiTelemetryService.getMetrics();
const usage = computeUsageFromMetrics(metrics);
const stats =
outputFormat === OutputFormat.JSON
? uiTelemetryService.getMetrics()
: undefined;
adapter.emitResult({
isError: false,
durationMs: Date.now() - startTime,
apiDurationMs: totalApiDurationMs,
numTurns: turnCount,
usage,
stats,
structuredResult: structuredSubmission,
});
return 0;
};
/**
* Shared per-turn tool-call dispatch for the main-turn loop and
* `drainOneItem`. Both call sites used to reproduce ~120 lines of
* near-identical logic that filtered `structured_output` to its
* own pre-scan when `--json-schema` is active, executed each
* request through `executeToolCall`, captured the `structured_output`
* args into the session-scoped `structuredSubmission`, and
* synthesised `tool_result` events for every suppressed sibling
* `tool_use`. The two blocks differed only by variable name
* prefixes (`requestsToExecute` vs `itemRequestsToExecute`, etc.)
* and which scope's `modelOverride` to update — passed in as
* `setModelOverride` so the caller controls binding.
*
* The helper mutates the closure-captured `structuredSubmission`
* directly (it's session-scoped on purpose: whichever turn
* captures it terminates the run). The caller is responsible for
* acting on a non-undefined `structuredSubmission` after the
* helper returns (main-turn → emitStructuredSuccess(); drain-turn
* → return so the post-drain code emits success).
*/
const processToolCallBatch = async (
batchRequests: ToolCallRequestInfo[],
setModelOverride: (override: string | undefined) => void,
): Promise<Part[]> => {
const toolResponseParts: Part[] = [];
// Pre-scan: when --json-schema is active and the model emitted
// a `structured_output` call alongside other tools in the same
// turn, the structured call is the terminal contract. Execute
// every structured_output in original order until one succeeds,
// suppress every non-structured sibling. See the multi-shape
// examples in the main loop's prior comment for the
// [bad/good/side-effect] permutations.
let requestsToExecute = batchRequests;
if (
config.getJsonSchema() &&
batchRequests.some((r) => r.name === ToolNames.STRUCTURED_OUTPUT)
) {
requestsToExecute = batchRequests.filter(
(r) => r.name === ToolNames.STRUCTURED_OUTPUT,
);
}
const executedCallIds = new Set<string>();
for (const requestInfo of requestsToExecute) {
executedCallIds.add(requestInfo.callId);
const inputFormat =
typeof config.getInputFormat === 'function'
? config.getInputFormat()
: InputFormat.TEXT;
const toolCallUpdateCallback =
inputFormat === InputFormat.STREAM_JSON && options.controlService
? options.controlService.permission.getToolCallUpdateCallback()
: undefined;
// Build outputUpdateHandler for this tool call. Agent tool
// has its own complex handler (subagent messages). All other
// tools with canUpdateOutput=true (e.g., MCP tools) get a
// generic handler that emits progress via the adapter.
const isAgentTool = requestInfo.name === 'agent';
const { handler: outputUpdateHandler } = isAgentTool
? createAgentToolProgressHandler(
config,
requestInfo.callId,
adapter,
)
: createToolProgressHandler(requestInfo, adapter);
const toolResponse = await executeToolCall(
config,
requestInfo,
abortController.signal,
{
outputUpdateHandler,
...(toolCallUpdateCallback && {
onToolCallsUpdate: toolCallUpdateCallback,
}),
},
);
if (toolResponse.error) {
// In JSON/STREAM_JSON mode, tool errors are tolerated and
// formatted as tool_result blocks. handleToolError detects
// mode from config and allows the session to continue so
// the LLM can decide what to do next. In text mode, we
// still log the error.
handleToolError(
requestInfo.name,
toolResponse.error,
config,
toolResponse.errorType || 'TOOL_EXECUTION_ERROR',
typeof toolResponse.resultDisplay === 'string'
? toolResponse.resultDisplay
: undefined,
);
}
adapter.emitToolResult(requestInfo, toolResponse);
config
.getGeminiClient()
.recordCompletedToolCall(
requestInfo.name,
requestInfo.args as Record<string, unknown>,
);
if (toolResponse.responseParts) {
toolResponseParts.push(...toolResponse.responseParts);
}
// Capture model override from skill tool results.
// Use `in` so that undefined (from inherit/no-model skills)
// clears a prior override, while non-skill tools (field
// absent) leave the current override intact.
if ('modelOverride' in toolResponse) {
setModelOverride(toolResponse.modelOverride);
}
if (
requestInfo.name === ToolNames.STRUCTURED_OUTPUT &&
!toolResponse.error
) {
// Honour the "first valid call ends the session" contract.
// The break is after the responseParts/modelOverride capture
// above so future changes to SyntheticOutputTool can't
// silently drop those signals. structuredSubmission is the
// session-scoped binding from the enclosing scope.
structuredSubmission = requestInfo.args;
break;
}
}
// Synthesise tool_result events + retry parts for every
// tool_use block from the prior assistant message that we did
// NOT actually execute — non-structured siblings that were
// suppressed up front, plus any structured_output calls left
// unexecuted after an earlier one in the batch already
// succeeded. Runs for both the success and retry paths so the
// emitted event log pairs every tool_use with a tool_result
// AND the retry-turn payload (when reached) doesn't leave
// Anthropic / OpenAI staring at unpaired tool_use blocks.
const unexecutedCalls = batchRequests.filter(
(r) => !executedCallIds.has(r.callId),
);
if (unexecutedCalls.length > 0) {
const skippedOutput = suppressedOutputBody(
structuredSubmission !== undefined,
);
for (const call of unexecutedCalls) {
const responseParts: Part[] = [
{
functionResponse: {
id: call.callId,
name: call.name,
response: { output: skippedOutput },
},
},
];
adapter.emitToolResult(call, {
callId: call.callId,
responseParts,
resultDisplay: skippedOutput,
error: undefined,
errorType: undefined,
});
toolResponseParts.push(...responseParts);
}
}
return toolResponseParts;
};
while (true) {
turnCount++;
if (
@@ -496,162 +756,31 @@ export async function runNonInteractive(
totalApiDurationMs += Date.now() - apiStartTime;
if (toolCallRequests.length > 0) {
const toolResponseParts: Part[] = [];
// When --json-schema is active, the first successful call to the
// synthetic structured_output tool terminates the session with the
// submitted args as the structured result. A separate boolean
// tracks whether a submission happened, since `args` itself may
// legitimately be undefined or any falsy value (an empty schema
// `{}` accepts any payload, including no fields at all).
let structuredSubmission: unknown = undefined;
let hasStructuredSubmission = false;
// Dispatch the per-turn tool-call batch through the shared
// helper (see processToolCallBatch above). The helper handles
// the `--json-schema` pre-scan, executes each request, writes
// the first valid `structured_output` call's args into the
// session-scoped `structuredSubmission`, and synthesises
// tool_result events for every suppressed sibling. The
// `modelOverride` setter is the only call-site-specific
// binding — the main turn updates the session-scoped
// `modelOverride` so the next turn's sendMessageStream sees
// it; the drain turn updates a per-item `itemModelOverride`
// scoped to that drain item.
const toolResponseParts = await processToolCallBatch(
toolCallRequests,
(override) => {
modelOverride = override;
},
);
// Pre-scan: when --json-schema is active and the model emitted
// structured_output alongside other tools in the same turn,
// execute structured_output FIRST so its terminal-flag wins
// before sibling tools' side effects (write_file, shell, …)
// get a chance to persist. If structured_output succeeds the
// loop breaks immediately and siblings are skipped — no
// tool_result is emitted for them; the session terminates via
// the emitResult call below so the missing function_response
// entries cause no API protocol issue (there is no next turn).
// If structured_output fails (validation), `hasStructuredSubmission`
// stays false and the siblings still run via the normal loop
// body — same behavior as a turn that didn't issue
// structured_output at all.
//
// Without this, [write_file, structured_output] runs write_file
// first (irreversible), THEN structured_output sets the flag,
// and the user gets back a "successful" structured result with
// unrelated side-effects already on disk.
let orderedToolCallRequests = toolCallRequests;
if (config.getJsonSchema()) {
const structIdx = orderedToolCallRequests.findIndex(
(r) => r.name === ToolNames.STRUCTURED_OUTPUT,
);
if (structIdx > 0) {
orderedToolCallRequests = [
orderedToolCallRequests[structIdx],
...orderedToolCallRequests.slice(0, structIdx),
...orderedToolCallRequests.slice(structIdx + 1),
];
}
}
for (const requestInfo of orderedToolCallRequests) {
const finalRequestInfo = requestInfo;
const inputFormat =
typeof config.getInputFormat === 'function'
? config.getInputFormat()
: InputFormat.TEXT;
const toolCallUpdateCallback =
inputFormat === InputFormat.STREAM_JSON && options.controlService
? options.controlService.permission.getToolCallUpdateCallback()
: undefined;
// Build outputUpdateHandler for this tool call.
// Agent tool has its own complex handler (subagent messages).
// All other tools with canUpdateOutput=true (e.g., MCP tools)
// get a generic handler that emits progress via the adapter.
const isAgentTool = finalRequestInfo.name === 'agent';
const { handler: outputUpdateHandler } = isAgentTool
? createAgentToolProgressHandler(
config,
finalRequestInfo.callId,
adapter,
)
: createToolProgressHandler(finalRequestInfo, adapter);
const toolResponse = await executeToolCall(
config,
finalRequestInfo,
abortController.signal,
{
outputUpdateHandler,
...(toolCallUpdateCallback && {
onToolCallsUpdate: toolCallUpdateCallback,
}),
},
);
if (toolResponse.error) {
// In JSON/STREAM_JSON mode, tool errors are tolerated and formatted
// as tool_result blocks. handleToolError will detect JSON/STREAM_JSON mode
// from config and allow the session to continue so the LLM can decide what to do next.
// In text mode, we still log the error.
handleToolError(
finalRequestInfo.name,
toolResponse.error,
config,
toolResponse.errorType || 'TOOL_EXECUTION_ERROR',
typeof toolResponse.resultDisplay === 'string'
? toolResponse.resultDisplay
: undefined,
);
}
adapter.emitToolResult(finalRequestInfo, toolResponse);
config
.getGeminiClient()
.recordCompletedToolCall(
finalRequestInfo.name,
finalRequestInfo.args as Record<string, unknown>,
);
if (
finalRequestInfo.name === ToolNames.STRUCTURED_OUTPUT &&
!toolResponse.error &&
!hasStructuredSubmission
) {
structuredSubmission = finalRequestInfo.args;
hasStructuredSubmission = true;
}
if (toolResponse.responseParts) {
toolResponseParts.push(...toolResponse.responseParts);
}
// Capture model override from skill tool results.
// Use `in` so that undefined (from inherit/no-model skills) clears a prior override,
// while non-skill tools (field absent) leave the current override intact.
if ('modelOverride' in toolResponse) {
modelOverride = toolResponse.modelOverride;
}
// Single-shot contract: structured_output is terminal.
// The pre-scan above hoists it to the front of the batch,
// so once it succeeds the remaining (now reordered)
// entries are guaranteed to be siblings the model
// intended for THIS turn — break and let the terminal
// emitResult fire below. Unpaired tool_use entries in
// the model's record are harmless because no next API
// call happens (the session is over).
if (hasStructuredSubmission) {
break;
}
}
if (hasStructuredSubmission) {
// Abort any in-flight background agents so they don't race the
// terminal emitResult; structured-output mode is a single-shot
// contract and the caller expects a deterministic shutdown.
registry.abortAll();
const metrics = uiTelemetryService.getMetrics();
const usage = computeUsageFromMetrics(metrics);
const stats =
outputFormat === OutputFormat.JSON
? uiTelemetryService.getMetrics()
: undefined;
adapter.emitResult({
isError: false,
durationMs: Date.now() - startTime,
apiDurationMs: totalApiDurationMs,
numTurns: turnCount,
usage,
stats,
structuredResult: structuredSubmission,
});
return;
if (structuredSubmission !== undefined) {
// Single-shot terminal contract; aborts in-flight background
// agents, holds back briefly for their terminal
// task_notification events to land, then emits the
// structured success envelope. Same helper as the drain-turn
// post-loop branch — see emitStructuredSuccess above.
return emitStructuredSuccess();
}
currentMessages = [{ role: 'user', parts: toolResponseParts }];
} else {
@@ -672,15 +801,6 @@ export async function runNonInteractive(
await handleMaxTurnsExceededError(config);
}
const inputFormat =
typeof config.getInputFormat === 'function'
? config.getInputFormat()
: InputFormat.TEXT;
const toolCallUpdateCallback =
inputFormat === InputFormat.STREAM_JSON && options.controlService
? options.controlService.permission.getToolCallUpdateCallback()
: undefined;
let itemMessages: Content[] = [
{ role: 'user', parts: [{ text: item.modelText }] },
];
@@ -742,57 +862,23 @@ export async function runNonInteractive(
totalApiDurationMs += Date.now() - itemApiStartTime;
if (itemToolCallRequests.length > 0) {
const itemToolResponseParts: Part[] = [];
// Same shared dispatch as the main-turn loop. The only
// call-site difference is `itemModelOverride` is local to
// the drain item (so the next iteration's
// sendMessageStream picks up the per-item override),
// while the main loop binds to the session-scoped
// `modelOverride`.
const itemToolResponseParts = await processToolCallBatch(
itemToolCallRequests,
(override) => {
itemModelOverride = override;
},
);
for (const requestInfo of itemToolCallRequests) {
const isAgentTool = requestInfo.name === 'agent';
const { handler: outputUpdateHandler } = isAgentTool
? createAgentToolProgressHandler(
config,
requestInfo.callId,
adapter,
)
: createToolProgressHandler(requestInfo, adapter);
const toolResponse = await executeToolCall(
config,
requestInfo,
abortController.signal,
{
outputUpdateHandler,
...(toolCallUpdateCallback && {
onToolCallsUpdate: toolCallUpdateCallback,
}),
},
);
if (toolResponse.error) {
handleToolError(
requestInfo.name,
toolResponse.error,
config,
toolResponse.errorType || 'TOOL_EXECUTION_ERROR',
typeof toolResponse.resultDisplay === 'string'
? toolResponse.resultDisplay
: undefined,
);
}
adapter.emitToolResult(requestInfo, toolResponse);
config
.getGeminiClient()
.recordCompletedToolCall(
requestInfo.name,
requestInfo.args as Record<string, unknown>,
);
if (toolResponse.responseParts) {
itemToolResponseParts.push(...toolResponse.responseParts);
}
if ('modelOverride' in toolResponse) {
itemModelOverride = toolResponse.modelOverride;
}
if (structuredSubmission !== undefined) {
// Stop processing further turns for this drain item;
// the post-drain code will emit the terminal result.
return;
}
itemMessages = [{ role: 'user', parts: itemToolResponseParts }];
} else {
@@ -813,6 +899,10 @@ export async function runNonInteractive(
if (drainPromise) return drainPromise;
const p = (async () => {
while (localQueue.length > 0) {
// Stop draining once a queued item's structured_output
// call captured the terminal contract — no point running
// more queued prompts that can't influence the result.
if (structuredSubmission !== undefined) return;
await drainOneItem();
}
})();
@@ -846,6 +936,16 @@ export async function runNonInteractive(
});
const checkCronDone = () => {
// A drain-turn structured_output makes the rest of the
// cron schedule moot: we already have a terminal result
// and the post-drain emit is about to fire. Stop the
// scheduler so no further jobs enqueue.
if (structuredSubmission !== undefined) {
abortController.signal.removeEventListener('abort', onAbort);
scheduler.stop();
resolve();
return;
}
if (scheduler.size === 0 && !drainPromise) {
abortController.signal.removeEventListener('abort', onAbort);
scheduler.stop();
@@ -897,6 +997,10 @@ export async function runNonInteractive(
// through the model, but later monitor output is SDK-only.
captureMonitorTurnsInLocalQueue = false;
await drainLocalQueue();
// A drain-turn structured_output captured the terminal
// contract — bail out of the holdback loop early and let the
// post-loop code emit the success result.
if (structuredSubmission !== undefined) break;
// Wait for every background task's terminal notification, not
// just the running ones: cancel() marks status 'cancelled'
// synchronously but the notification is emitted later by the
@@ -925,12 +1029,22 @@ export async function runNonInteractive(
? uiTelemetryService.getMetrics()
: undefined;
// A drain-turn structured_output captured the terminal contract
// — emit the structured success envelope rather than falling
// through to the "Model produced plain text..." failure path.
// Same helper as the main-turn path; recomputes its own
// metrics snapshot after the holdback so any task notifications
// that landed during shutdown contribute to the totals.
if (structuredSubmission !== undefined) {
return emitStructuredSuccess();
}
// --json-schema contract: the model MUST terminate via the
// structured_output tool. Reaching this branch means it emitted
// plain text instead — surface as an error rather than silently
// returning whatever free-form summary the adapter collected.
// Setting exitCode + returning (rather than throwing) avoids the
// outer catch re-emitting the result a second time.
// Returning a non-zero exit code (rather than throwing) avoids
// the outer catch re-emitting the result a second time.
if (config.getJsonSchema()) {
// Enrich the static contract message with diagnostic context:
// turn count (how many tries the model got) + a preview of
@@ -955,15 +1069,7 @@ export async function runNonInteractive(
usage,
stats,
});
// Adapter handles user-visible feedback per output format:
// - TEXT: writes errorMessage to stderr (JsonOutputAdapter
// emitResult, line ~70).
// - JSON / STREAM_JSON: emits the structured result with
// is_error=true.
// No extra stderr write here — duplicating in TEXT mode
// produced two copies of the same line in headless runs.
process.exitCode = 1;
return;
return 1;
}
adapter.emitResult({
@@ -974,7 +1080,7 @@ export async function runNonInteractive(
usage,
stats,
});
return;
return 0;
}
}
} catch (error) {
@@ -1050,5 +1156,9 @@ export async function runNonInteractive(
await shutdownTelemetry();
}
}
// Unreachable in practice: the catch block awaits handleError() which
// returns Promise<never> (it always exits the process or rethrows).
// This return exists only so TS sees the function as total.
return 1;
});
}
+47
View File
@@ -725,6 +725,53 @@ describe('errors', () => {
);
});
});
describe('with --json-schema active', () => {
// When the structured-output run hits maxSessionTurns the generic
// "increase maxSessionTurns" message can be misleading: the real
// cause is usually that structured_output never got called (denied
// by permissions, unsatisfiable schema, prompt didn't instruct the
// model). Append a contextual hint so users debugging a stuck
// --json-schema run know where to look.
beforeEach(() => {
(mockConfig as unknown as { getJsonSchema: Mock }).getJsonSchema = vi
.fn()
.mockReturnValue({ type: 'object' });
});
it('appends a json-schema-specific hint in text mode', async () => {
(
mockConfig.getOutputFormat as ReturnType<typeof vi.fn>
).mockReturnValue(OutputFormat.TEXT);
await expect(handleMaxTurnsExceededError(mockConfig)).rejects.toThrow(
'process.exit called with code: 53',
);
const written = mockWriteStderrLine.mock.calls[0]?.[0] as string;
expect(written).toMatch(/Reached max session turns for this session\./);
expect(written).toMatch(/--json-schema is active/);
expect(written).toMatch(/permissions\.deny.*--exclude-tools/);
});
it('appends a json-schema-specific hint inside the JSON error message', async () => {
(
mockConfig.getOutputFormat as ReturnType<typeof vi.fn>
).mockReturnValue(OutputFormat.JSON);
await expect(handleMaxTurnsExceededError(mockConfig)).rejects.toThrow(
'process.exit called with code: 53',
);
const written = mockWriteStderrLine.mock.calls[0]?.[0] as string;
const parsed = JSON.parse(written) as {
error: { type: string; message: string; code: number };
};
expect(parsed.error.type).toBe('FatalTurnLimitedError');
expect(parsed.error.code).toBe(53);
expect(parsed.error.message).toMatch(/--json-schema is active/);
});
});
});
describe('cleanup-before-exit invariant', () => {
+14 -3
View File
@@ -246,13 +246,24 @@ export async function handleCancellationError(config: Config): Promise<never> {
/**
* Handles max session turns exceeded consistently.
*
* When `--json-schema` is active the error gets an extra hint pointing at the
* common reasons a structured-output run never terminated: the model never
* called `structured_output`, the tool was denied by `permissions.deny` /
* `--exclude-tools`, or the schema is unsatisfiable. Without this, all three
* failure modes surface as the same generic "increase maxSessionTurns" line
* even though the fix is a permissions / schema change, not a turns bump.
*/
export async function handleMaxTurnsExceededError(
config: Config,
): Promise<never> {
const maxTurnsError = new FatalTurnLimitedError(
'Reached max session turns for this session. Increase the number of turns by specifying maxSessionTurns in settings.json.',
);
const baseMessage =
'Reached max session turns for this session. Increase the number of turns by specifying maxSessionTurns in settings.json.';
const jsonSchemaActive = config.getJsonSchema?.() !== undefined;
const message = jsonSchemaActive
? `${baseMessage}\nNote: --json-schema is active. If the model never called structured_output, verify it isn't denied by permissions.deny / --exclude-tools and that the schema is satisfiable.`
: baseMessage;
const maxTurnsError = new FatalTurnLimitedError(message);
if (config.getOutputFormat() === OutputFormat.JSON) {
const formatter = new JsonFormatter();
@@ -413,7 +413,7 @@ async function createPerAgentConfig(
const agentRegistry: ToolRegistry = await override.createToolRegistry(
undefined,
{ skipDiscovery: true },
{ skipDiscovery: true, forSubAgent: true },
);
agentRegistry.copyDiscoveredToolsFrom(base.getToolRegistry());
override.getToolRegistry = () => agentRegistry;
+76
View File
@@ -1376,6 +1376,82 @@ describe('Server Config (config.ts)', () => {
).toEqual([ToolNames.READ_FILE, ToolNames.EDIT, ToolNames.SHELL]);
});
it('registers structured_output in bare mode when jsonSchema is set', async () => {
// Bare mode strips the toolset to READ_FILE/EDIT/SHELL, but the
// synthetic structured_output tool is the terminal contract for
// --json-schema runs. Without it the model loops until
// maxSessionTurns and exits via the "plain text" failure path —
// expensive in tokens for what's almost always a CI use case. The
// synthetic tool must be registered alongside the bare three.
const config = new Config({
...baseParams,
bareMode: true,
jsonSchema: { type: 'object', properties: { ok: { type: 'boolean' } } },
});
await config.initialize();
const registerToolMock = (
(await vi.importMock('../tools/tool-registry')) as {
ToolRegistry: { prototype: { registerFactory: Mock } };
}
).ToolRegistry.prototype.registerFactory;
expect(
(registerToolMock as Mock).mock.calls.map((call) => call[0]),
).toEqual([
ToolNames.READ_FILE,
ToolNames.EDIT,
ToolNames.SHELL,
ToolNames.STRUCTURED_OUTPUT,
]);
});
it('does NOT register structured_output when createToolRegistry is called with forSubAgent=true', async () => {
// Subagent overrides reuse the parent Config via prototype
// delegation (createApprovalModeOverride / buildSubagentContextOverride
// → Object.create(base)) and rebuild the tool registry with
// `forSubAgent: true`. Even though `this.jsonSchema` propagates
// through the prototype chain, the synthetic tool MUST NOT register
// in the subagent registry: only runNonInteractive's main / drain
// loops detect a successful structured_output call as terminal, so
// a subagent calling the tool would receive "Session will end now"
// and then keep running because its own loop has no terminator —
// wasted tokens and no structured payload on stdout.
const config = new Config({
...baseParams,
bareMode: true,
jsonSchema: { type: 'object', properties: { ok: { type: 'boolean' } } },
});
await config.initialize();
const registerToolMock = (
(await vi.importMock('../tools/tool-registry')) as {
ToolRegistry: { prototype: { registerFactory: Mock } };
}
).ToolRegistry.prototype.registerFactory;
// Initial bare init registers READ_FILE / EDIT / SHELL /
// STRUCTURED_OUTPUT (asserted by the test above). Reset so we can
// observe ONLY the forSubAgent rebuild's calls.
(registerToolMock as Mock).mockClear();
// Rebuild registry as if for a subagent override.
await config.createToolRegistry(undefined, {
skipDiscovery: true,
forSubAgent: true,
});
const registeredNames = (registerToolMock as Mock).mock.calls.map(
(call) => call[0],
);
expect(registeredNames).not.toContain(ToolNames.STRUCTURED_OUTPUT);
// The bare three still register so the subagent has its toolset.
expect(registeredNames).toEqual([
ToolNames.READ_FILE,
ToolNames.EDIT,
ToolNames.SHELL,
]);
});
it('should register a tool if coreTools contains an argument-specific pattern', async () => {
const params: ConfigParameters = {
...baseParams,
+36 -10
View File
@@ -2899,7 +2899,7 @@ export class Config {
async createToolRegistry(
sendSdkMcpMessage?: SendSdkMcpMessage,
options?: { skipDiscovery?: boolean },
options?: { skipDiscovery?: boolean; forSubAgent?: boolean },
): Promise<ToolRegistry> {
const registry = new ToolRegistry(
this,
@@ -2933,6 +2933,37 @@ export class Config {
}
};
// The synthetic structured_output tool is the terminal contract for
// --json-schema runs. It must be registered in BOTH the bare-mode
// branch and the regular branch — without it the model can't finish
// a structured run, so omitting either branch causes
// `qwen [--bare] --json-schema X -p "..."` to loop until
// maxSessionTurns and exit via the "plain text" failure path. Hoisted
// out of the two branches so the dynamic-import factory shape stays
// in sync between them.
//
// Skipped when building a subagent-context registry. `this.jsonSchema`
// propagates to subagent overrides via prototype delegation
// (`Object.create(base)` in `createApprovalModeOverride` /
// `buildSubagentContextOverride`), but only `runNonInteractive`'s main
// and drain loops detect a successful structured_output call as
// terminal. A subagent that called the tool would receive the
// "Session will end now" llmContent, then keep running because its
// own loop has no termination handler — wasted tokens with no
// structured payload surfacing on stdout. Strip the registration in
// those contexts.
const registerStructuredOutputIfRequested = async (): Promise<void> => {
if (!this.jsonSchema) return;
if (options?.forSubAgent) return;
const schema = this.jsonSchema;
await registerLazy(ToolNames.STRUCTURED_OUTPUT, async () => {
const { SyntheticOutputTool } = await import(
'../tools/syntheticOutput.js'
);
return new SyntheticOutputTool(schema);
});
};
if (this.getBareMode()) {
await registerLazy(ToolNames.READ_FILE, async () => {
const { ReadFileTool } = await import('../tools/read-file.js');
@@ -2946,6 +2977,7 @@ export class Config {
const { ShellTool } = await import('../tools/shell.js');
return new ShellTool(this);
});
await registerStructuredOutputIfRequested();
this.debugLogger.debug(
`ToolRegistry created: ${JSON.stringify(registry.getAllToolNames())} (${registry.getAllToolNames().length} tools)`,
);
@@ -3063,15 +3095,9 @@ export class Config {
// Register synthetic structured-output tool when --json-schema is set.
// The tool's parameter schema IS the user-supplied JSON Schema, so the
// model's arguments must match it (Ajv-validated in BaseDeclarativeTool).
if (this.jsonSchema) {
const schema = this.jsonSchema;
await registerLazy(ToolNames.STRUCTURED_OUTPUT, async () => {
const { SyntheticOutputTool } = await import(
'../tools/syntheticOutput.js'
);
return new SyntheticOutputTool(this, schema);
});
}
// Same helper as the bare-mode branch above to keep the registration
// shape and permission gating in sync between the two paths.
await registerStructuredOutputIfRequested();
// Register cron tools unless disabled
if (this.isCronEnabled()) {
+70
View File
@@ -15,6 +15,7 @@ import { AuthType, type ContentGenerator } from '../core/contentGenerator.js';
import {
GeminiChat,
InvalidStreamError,
redactStructuredOutputArgsForRecording,
StreamEventType,
type StreamEvent,
} from './geminiChat.js';
@@ -3322,6 +3323,75 @@ describe('GeminiChat', async () => {
});
});
describe('redactStructuredOutputArgsForRecording', () => {
// The chat-recording JSONL persists assistant turns to disk and re-feeds
// them on `--continue` / `--resume`. For `--json-schema` runs the
// structured_output args ARE the user's structured payload, already
// emitted on stdout; recording them verbatim here would silently
// contradict the redaction the ToolCallEvent telemetry path applies.
// These tests pin the helper that scrubs them.
it('replaces args on a structured_output functionCall with the placeholder', () => {
const result = redactStructuredOutputArgsForRecording({
functionCall: {
id: 'call-1',
name: 'structured_output',
args: {
extracted: 'sensitive answer',
score: 0.9,
details: { token: 'shhhh' },
},
},
});
expect(result).not.toBeNull();
expect(result!.functionCall.name).toBe('structured_output');
expect(result!.functionCall.id).toBe('call-1');
expect(result!.functionCall.args).toEqual({
__redacted: 'structured_output payload (see stdout result)',
});
// The original payload must NOT survive in any field of the output.
expect(JSON.stringify(result)).not.toContain('sensitive answer');
expect(JSON.stringify(result)).not.toContain('shhhh');
});
it('passes non-structured_output functionCalls through untouched', () => {
const original = {
id: 'call-2',
name: 'write_file',
args: { path: '/tmp/x', content: 'hello' },
};
const result = redactStructuredOutputArgsForRecording({
functionCall: original,
});
expect(result).not.toBeNull();
expect(result!.functionCall).toEqual(original);
// Reference identity not required, but the args object must equal
// the input (no redaction applied).
expect(result!.functionCall.args).toEqual({
path: '/tmp/x',
content: 'hello',
});
});
it('returns null for parts with no functionCall', () => {
expect(redactStructuredOutputArgsForRecording({ text: 'hi' })).toBeNull();
expect(redactStructuredOutputArgsForRecording({})).toBeNull();
});
it('does not mutate the input part', () => {
const original = {
functionCall: {
id: 'call-3',
name: 'structured_output',
args: { ok: true, data: [1, 2, 3] },
},
};
const snapshot = JSON.parse(JSON.stringify(original));
redactStructuredOutputArgsForRecording(original);
expect(original).toEqual(snapshot);
});
});
// Compression logic is tested in chatCompressionService.test.ts; this
// suite covers per-chat state on GeminiChat: hasFailedCompressionAttempt
// stickiness, token-count mutation, history replacement, and conditional
+44 -2
View File
@@ -34,6 +34,8 @@ import {
tokenLimit,
} from './tokenLimits.js';
import { hasCycleInSchema } from '../tools/tools.js';
import { ToolNames } from '../tools/tool-names.js';
import { STRUCTURED_OUTPUT_REDACTED_ARGS } from '../tools/syntheticOutput.js';
import type { StructuredError } from './turn.js';
import {
logContentRetry,
@@ -54,6 +56,41 @@ import { getContextLengthExceededInfo } from '../utils/contextLengthError.js';
const debugLogger = createDebugLogger('QWEN_CODE_CHAT');
/**
* Replaces the args on a `structured_output` `functionCall` with the
* same `__redacted` placeholder used by `ToolCallEvent` telemetry
* (`packages/core/src/telemetry/types.ts`).
*
* The chat-recording JSONL (`<projectDir>/chats/<sessionId>.jsonl`)
* persists assistant turns to disk and re-feeds them on
* `--continue` / `--resume`. For `--json-schema` runs the tool args
* ARE the user's structured payload — already emitted on stdout via
* `result` / `structured_result`. Recording them verbatim here would
* mean the same payload (and every validation-failure retry along the
* way) sits on disk indefinitely, contradicting the privacy contract
* documented next to the telemetry redaction. Mirror the placeholder
* here so the chat-recording surface matches.
*
* Non-`structured_output` `functionCall`s pass through untouched.
*
* Exported for tests; callers should prefer the inline use inside
* `recordAssistantTurn` invocation below.
*/
export function redactStructuredOutputArgsForRecording(
part: Part,
): { functionCall: NonNullable<Part['functionCall']> } | null {
if (!part.functionCall) return null;
if (part.functionCall.name !== ToolNames.STRUCTURED_OUTPUT) {
return { functionCall: part.functionCall };
}
return {
functionCall: {
...part.functionCall,
args: { ...STRUCTURED_OUTPUT_REDACTED_ARGS },
},
};
}
function isCompressionFailureStatus(status: CompressionStatus): boolean {
return (
status === CompressionStatus.COMPRESSION_FAILED_INFLATED_TOKEN_COUNT ||
@@ -1257,8 +1294,13 @@ export class GeminiChat {
...(contentText ? [{ text: contentText }] : []),
...(hasToolCall
? contentParts
.filter((part) => part.functionCall)
.map((part) => ({ functionCall: part.functionCall }))
.map(redactStructuredOutputArgsForRecording)
.filter(
(
p,
): p is { functionCall: NonNullable<Part['functionCall']> } =>
p !== null,
)
: []),
],
tokens: usageMetadata,
@@ -1592,6 +1592,20 @@ describe('PermissionManager', () => {
expect(await pm.isToolEnabled('ask_user_question')).toBe(true);
});
it('structured_output tool bypasses coreTools allowlist check', async () => {
// structured_output is a synthetic tool that only exists when the
// user opts into --json-schema. Treating it like agent/skill/
// exit_plan_mode (bypass the coreTools allowlist) is the right
// default: a run that combines `--json-schema X --core-tools read_file`
// intends "restrict the model's pluggable toolbelt to read_file"
// while still receiving the structured payload — silently dropping
// structured_output here would leave --json-schema with no terminal
// contract, so the run would loop until maxTurns.
pm = new PermissionManager(makeConfig({ coreTools: ['read_file'] }));
pm.initialize();
expect(await pm.isToolEnabled('structured_output')).toBe(true);
});
it('Non-core tools still respect deny rules', async () => {
pm = new PermissionManager(
makeConfig({
@@ -427,7 +427,17 @@ export class PermissionManager {
/**
* Core tools that are subject to the coreTools allowlist check.
* Tools not in this set (MCP, Skill, Agent, etc.) bypass the check.
*
* Tools NOT in this set bypass the check. Two categories live outside:
* - Dynamically discovered tools (MCP, Skill).
* - Synthetic system tools that the framework injects when a feature is
* opted into and that have no meaning when missing — `agent`,
* `exit_plan_mode`, `ask_user_question`, `task_stop`, `send_message`,
* `structured_output` (registered only when `--json-schema` is set).
* Excluding `structured_output` from `--core-tools` would leave a
* `--json-schema` run with no terminal contract, so the synthetic
* tool stays available regardless of the allowlist (deny rules still
* apply).
*/
private static readonly CORE_TOOLS = new Set([
'read_file',
+16 -1
View File
@@ -19,6 +19,7 @@ import type { FileOperation } from './metrics.js';
export { ToolCallDecision };
import type { OutputFormat } from '../output/types.js';
import { ToolNames } from '../tools/tool-names.js';
import { STRUCTURED_OUTPUT_REDACTED_ARGS } from '../tools/syntheticOutput.js';
import type { SkillTool } from '../tools/skill.js';
import type { AgentTool } from '../tools/agent/agent.js';
@@ -189,7 +190,21 @@ export class ToolCallEvent implements BaseTelemetryEvent {
this['event.name'] = 'tool_call';
this['event.timestamp'] = new Date().toISOString();
this.function_name = call.request.name;
this.function_args = call.request.args;
// structured_output args ARE the user's final structured payload (the
// command's actual answer, already emitted in stdout `result` /
// `structured_result`). Recording them again as ordinary tool-call
// function_args duplicates that data into telemetry surfaces (OTLP
// exports, QwenLogger, ui-telemetry stream, the chat-recording UI
// event mirror) where it can leak off-device. Replace with a shared
// placeholder constant so consumers still see the call happened —
// duration, success, decision metrics are preserved — but the
// payload itself doesn't ride along. The same constant is used by
// `redactStructuredOutputArgsForRecording` in `core/geminiChat.ts`
// for the on-disk JSONL surface so neither side can silently drift.
this.function_args =
call.request.name === ToolNames.STRUCTURED_OUTPUT
? { ...STRUCTURED_OUTPUT_REDACTED_ARGS }
: call.request.args;
this.duration_ms = call.durationMs ?? 0;
this.status = call.status;
this.success = call.status === 'success'; // Keep for backward compatibility
@@ -737,6 +737,65 @@ describe('UiTelemetryService', () => {
expect(tools.byName['tool_A'].count).toBe(1);
expect(tools.byName['tool_B'].count).toBe(1);
});
it('redacts function_args for structured_output calls while preserving metrics', () => {
const toolCall = createFakeCompletedToolCall(
'structured_output',
true,
250,
ToolConfirmationOutcome.ProceedOnce,
);
// The fake helper hardcodes args to { foo: 'bar' }; in the real
// structured-output flow this would be the user's extracted payload.
// ToolCallEvent must not pass that through to telemetry.
(toolCall.request as { args: Record<string, unknown> }).args = {
secret: 'extracted private value',
};
const event = new ToolCallEvent(toolCall);
expect(event.function_name).toBe('structured_output');
expect(event.function_args).not.toHaveProperty('secret');
expect(event.function_args).toEqual({
__redacted: 'structured_output payload (see stdout result)',
});
// Metrics still flow through normally — duration, success, decision.
service.addEvent({
...structuredClone(event),
'event.name': EVENT_TOOL_CALL,
} as ToolCallEvent & { 'event.name': typeof EVENT_TOOL_CALL });
const { tools } = service.getMetrics();
expect(tools.totalCalls).toBe(1);
expect(tools.totalSuccess).toBe(1);
expect(tools.totalDurationMs).toBe(250);
expect(tools.byName['structured_output']).toMatchObject({
count: 1,
success: 1,
durationMs: 250,
});
});
it('does not redact function_args for non-structured_output tools', () => {
const toolCall = createFakeCompletedToolCall(
'write_file',
true,
100,
ToolConfirmationOutcome.ProceedOnce,
);
(toolCall.request as { args: Record<string, unknown> }).args = {
path: '/tmp/x',
content: 'hello',
};
const event = new ToolCallEvent(toolCall);
expect(event.function_args).toEqual({
path: '/tmp/x',
content: 'hello',
});
});
});
describe('resetLastPromptTokenCount', () => {
+1
View File
@@ -281,6 +281,7 @@ export async function rebuildToolRegistryOnOverride(
const ov = override as any;
const agentRegistry = await ov.createToolRegistry(undefined, {
skipDiscovery: true,
forSubAgent: true,
});
agentRegistry.copyDiscoveredToolsFrom(base.getToolRegistry());
ov.getToolRegistry = () => agentRegistry;
@@ -4,14 +4,12 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, it, expect, vi } from 'vitest';
import { describe, it, expect } from 'vitest';
import { SyntheticOutputTool } from './syntheticOutput.js';
import { ToolNames } from './tool-names.js';
import type { Config } from '../config/config.js';
function makeTool(schema: Record<string, unknown>): SyntheticOutputTool {
const mockConfig = { isInteractive: vi.fn().mockReturnValue(false) } as unknown as Config;
return new SyntheticOutputTool(mockConfig, schema);
return new SyntheticOutputTool(schema);
}
describe('SyntheticOutputTool', () => {
@@ -37,9 +35,7 @@ describe('SyntheticOutputTool', () => {
it('accepts args that match the user schema', () => {
const tool = makeTool(objectSchema);
expect(
tool.validateToolParams({ summary: 'ok', score: 1 }),
).toBeNull();
expect(tool.validateToolParams({ summary: 'ok', score: 1 })).toBeNull();
});
it('rejects args missing required fields', () => {
@@ -68,6 +64,11 @@ describe('SyntheticOutputTool', () => {
});
it('is always loaded (never hidden behind ToolSearch)', () => {
// The synthetic terminal tool MUST be visible to the model from the
// very first turn. If ToolSearch's deferred-load logic ever hid it,
// the structured-output contract would silently break (the model
// wouldn't know the tool exists, would emit plain text, and the run
// would exit via the "Model produced plain text..." failure path).
const tool = makeTool(objectSchema);
expect(tool.alwaysLoad).toBe(true);
expect(tool.shouldDefer).toBe(false);
+30 -3
View File
@@ -6,17 +6,39 @@
import type { ToolResult } from './tools.js';
import { BaseDeclarativeTool, BaseToolInvocation, Kind } from './tools.js';
import type { Config } from '../config/config.js';
import { ToolDisplayNames, ToolNames } from './tool-names.js';
const structuredOutputDescription = `Submit your final answer as structured JSON that conforms to the provided schema.
CRITICAL: In structured-output mode, this is the ONLY way to deliver the final result. Call this tool exactly once when you are ready to finish. Do not emit the final answer as plain text — it will be discarded. Use other tools (Read, Grep, etc.) to gather the information you need before calling this tool.
CRITICAL: In structured-output mode, this is the ONLY way to deliver the final result. Call this tool to deliver the final result; the first call with valid arguments ends the session. Do not emit the final answer as plain text — it will be discarded. Use other tools (Read, Grep, etc.) to gather the information you need before calling this tool.
The arguments you pass MUST validate against the tool's parameter schema. If validation fails you will receive the error and may retry with corrected fields.`;
export type StructuredOutputParams = Record<string, unknown>;
/**
* Placeholder that replaces a `structured_output` tool call's `args` on
* every surface that would otherwise persist or re-broadcast the user's
* structured payload.
*
* Two on-device surfaces redact via this constant:
* 1. `ToolCallEvent` in `telemetry/types.ts` — keeps the payload out
* of OTLP exports / QwenLogger / ui-telemetry stream / chat-recording
* UI event mirror.
* 2. `redactStructuredOutputArgsForRecording` in `core/geminiChat.ts`
* — keeps the payload out of the on-disk chat-recording JSONL
* (which gets re-fed into model context on `--continue` /
* `--resume`).
*
* Shared so both sites can't drift. The args ARE the user's final
* structured payload; they're already on stdout via `result` /
* `structured_result`, so persisting them again is duplication that
* leaks payload data into long-lived storage.
*/
export const STRUCTURED_OUTPUT_REDACTED_ARGS = {
__redacted: 'structured_output payload (see stdout result)',
} as const;
/**
* Synthetic tool that is registered only when the user passes --json-schema.
* The parameter schema of the tool IS the user-provided JSON Schema, so the
@@ -25,6 +47,11 @@ export type StructuredOutputParams = Record<string, unknown>;
*
* The caller (nonInteractiveCli) recognizes a successful invocation of this
* tool and ends the session, using request.args as the structured result.
*
* Wired into the ToolSearch infrastructure with `alwaysLoad: true` so the
* tool is never hidden behind on-demand schema loading — the model has to
* see this tool in its function-declaration list from the very first turn,
* otherwise the structured-output contract can't be honored at all.
*/
export class SyntheticOutputTool extends BaseDeclarativeTool<
StructuredOutputParams,
@@ -32,7 +59,7 @@ export class SyntheticOutputTool extends BaseDeclarativeTool<
> {
static readonly Name: string = ToolNames.STRUCTURED_OUTPUT;
constructor(_config: Config, userSchema: Record<string, unknown>) {
constructor(userSchema: Record<string, unknown>) {
super(
SyntheticOutputTool.Name,
ToolDisplayNames.STRUCTURED_OUTPUT,
+2 -2
View File
@@ -38,9 +38,9 @@ export const ToolNames = {
CRON_DELETE: 'cron_delete',
TASK_STOP: 'task_stop',
SEND_MESSAGE: 'send_message',
STRUCTURED_OUTPUT: 'structured_output',
MONITOR: 'monitor',
TOOL_SEARCH: 'tool_search',
STRUCTURED_OUTPUT: 'structured_output',
} as const;
/**
@@ -69,9 +69,9 @@ export const ToolDisplayNames = {
CRON_DELETE: 'CronDelete',
TASK_STOP: 'TaskStop',
SEND_MESSAGE: 'SendMessage',
STRUCTURED_OUTPUT: 'StructuredOutput',
MONITOR: 'Monitor',
TOOL_SEARCH: 'ToolSearch',
STRUCTURED_OUTPUT: 'StructuredOutput',
} as const;
// Migration from old tool names to new tool names
@@ -153,6 +153,48 @@ describe('ToolRegistry', () => {
toolRegistry.registerTool(tool);
expect(toolRegistry.getTool('mock-tool')).toBe(tool);
});
it('renames an MCP tool whose name shadows a registered lazy factory', async () => {
// The synthetic `structured_output` tool registers via
// `registerFactory` (lazy). Without this guard, an MCP server
// discovering a tool named `structured_output` would silently
// shadow the factory: `tools.has(name)` is false (factories live
// in a separate map), the MCP tool registers as-is, and the next
// `ensureTool('structured_output')` resolves from the eager map
// and discards the factory. Same for any other lazy built-in.
// The fix folds factory collisions into the same auto-rename
// path MCP tools already get for eager-tool collisions.
toolRegistry.registerFactory(
'structured_output',
async () => new MockTool({ name: 'structured_output' }),
);
const mockCallable = {} as CallableTool;
const collidingMcp = new DiscoveredMCPTool(
mockCallable,
'rogue-server',
'structured_output',
'description',
{},
);
toolRegistry.registerTool(collidingMcp);
// The MCP tool must have been auto-qualified and live under its
// namespaced name, not under `structured_output`.
const renamed = toolRegistry.getTool(
'mcp__rogue-server__structured_output',
);
expect(renamed).toBeDefined();
expect(renamed).toBeInstanceOf(DiscoveredMCPTool);
// The factory must still be the canonical owner of
// `structured_output` — `ensureTool` resolves it without going
// through the MCP tool.
const resolved = await toolRegistry.ensureTool('structured_output');
expect(resolved).toBeDefined();
expect(resolved).not.toBeInstanceOf(DiscoveredMCPTool);
expect(resolved!.name).toBe('structured_output');
});
});
describe('getAllTools', () => {
+15 -2
View File
@@ -209,11 +209,24 @@ export class ToolRegistry {
* @param tool - The tool object containing schema and execution logic.
*/
registerTool(tool: AnyDeclarativeTool): void {
if (this.tools.has(tool.name)) {
// A name collision can happen against either the eager `tools` map
// (already-instantiated tools) or the lazy `factories` map (registered
// but not yet constructed — `structured_output` lives here when
// `--json-schema` is set, but the same is true for every other lazy
// built-in). Without considering factories, an MCP server registering
// a tool with a name that shadows a built-in factory would silently
// win: `tools.has(name)` returns false, no rename happens, then the
// first `ensureTool(name)` resolves from `tools` and the factory is
// discarded. For MCP tools we resolve this by appending the server-
// qualified suffix; for other internal callers we keep the existing
// overwrite-with-warning behaviour for parity with the eager-only
// path.
const collidesWithEager = this.tools.has(tool.name);
const collidesWithFactory = this.factories.has(tool.name);
if (collidesWithEager || collidesWithFactory) {
if (tool instanceof DiscoveredMCPTool) {
tool = tool.asFullyQualifiedTool();
} else {
// Decide on behavior: throw error, log warning, or allow overwrite
debugLogger.warn(
`Tool with name "${tool.name}" is already registered. Overwriting.`,
);
+7 -4
View File
@@ -75,10 +75,13 @@ function getValidator(schema: AnySchema): Ajv {
export class SchemaValidator {
/**
* Strictly compiles a schema. Returns an error message if the schema is
* malformed or uses an Ajv version we can't support. Returns null on
* success. Unlike {@link validate}, this does NOT silently skip on
* compile failure — callers (e.g. the CLI's `--json-schema` parser) need
* to surface invalid schemas instead of letting them no-op at runtime.
* malformed or uses unsupported draft/features for our Ajv configuration
* (see {@link getValidator} — `$schema` selects between draft-07 and
* draft-2020-12; anything else falls through to draft-07's compiler).
* Returns null on success. Unlike {@link validate}, this does NOT
* silently skip on compile failure — callers (e.g. the CLI's
* `--json-schema` parser) need to surface invalid schemas instead of
* letting them no-op at runtime.
*/
static compileStrict(schema: unknown): string | null {
if (!schema || typeof schema !== 'object' || Array.isArray(schema)) {