feat(llm): honour supportsTools: "strict" on the OpenAI tools payload

[plombeer31]

What was wrong

Discord #feedback-and-bugs, thegreatteacher, 2026-09-10 07:43 UTC — testing mercury 2.5 (Inception Labs) with atomic-agent: the model "makes a lot of tool calling mistakes without "strict": true option. Is it possible to set "strict": true argument through config file or do you need to add provider specific patch?" The maintainer's answer was that a patch would be needed.

The awkward part: "strict" has been a documented, parseable value of supportsTools all along — llm-provider.ts:35, model-resolver.ts:12, model-catalog-entry.ts:20, provider-types.ts:71, validated out of user config in llm-config.ts:486 — and nothing consumed it. descriptorsToOpenAiTools emitted {type:"function", function:{name, description, parameters}} and never a strict field, so an operator who set the level got silence. extraBody is not a workaround either: it merges at the top level of the request body and can never reach a per-tool function.strict.

What this does

When the resolved model declares supportsTools: "strict", the native-tools request marks function definitions strict: true and hands them a strict-compatible schema. Opt-in, default-off: no wire change for anyone who does not set the level, and the flag-off payload is byte-for-byte what origin/main emits.

"llm": { "providers": [{ "id": "inception", "kind": "openai-compatible",
  "userModels": [{ "id": "mercury-2.5", "kind": "chat", "supportsTools": "strict" }] }] }

Strict decoding also turns parallel calls off. OpenAI documents Structured Outputs as not compatible with parallel function calls — a parallel call generated under strict mode "may not match supplied schemas" — and says to send parallel_tool_calls: false. Marking every convertible tool strict: true and leaving that flag at its default true buys best-effort adherence, i.e. the exact symptom this feature exists to cure. This is the one thing the parallel PR #402 got right and this branch had missed outright; the reasoning and the fix are taken from there. Implemented here off the emitted tools array rather than off the config level, because strict is granted per tool: an adapter can ignore the option and a descriptor set can convert nothing, and neither should silently lose parallel calls for a request that is not constrained at all. One predicate, hasStrictFunctionTools, read in two places that therefore cannot disagree — buildLlmStreamParams makes the decision, so the CompletionRequest is honest about it, and buildOpenAiChatBody is the floor under every other caller of the body builder. The executor's own maxParallelToolCalls batching is untouched: a model that emits several calls anyway is planned and run exactly as before.

The hazard, and how it is engineered around. Strict schemas are much narrower than this repo's descriptors: every object must carry additionalProperties: false, every property must appear in required (optionals as a null union), and the bounds keywords are rejected outright. Getting that wrong is a 400 on every request once the flag is on — worse for the user than the bug being fixed. So the new strict-tool-schema.ts converts per tool, against a keyword allowlist, and refuses per tool; descriptorsToOpenAiTools marks strict only on what converted and leaves the rest byte-identical to today. A mixed tools array is legal, and a partial win beats a request that cannot be served.

Retracted this round: the coverage expansion. An earlier commit on this branch stripped value-range bounds instead of refusing over them, taking coverage from 76 to 80 of the 82 emitted functions by recovering reply, vision.describe, fusion.delegate and os.fs.archive.extract. It has been reverted, and coverage is back to 76.

The reason is specific. Those last two are the only built-ins whose strict schema contains a nested object (fusion.delegate.tasks[], os.fs.archive.extract.limits), and indexOfferedTools — the fix described under tagged-tool providers below — narrows the top-level required and nothing else. So with both converted, a realistic <tool_call> for either one (a fusion.delegate fan-out that does not fill deliverable and files on every task, say) failed the offered-tool check and the whole call collapsed back into prose on a qwen-openai-compatible link — where it worked before the expansion. That is the exact failure the tagged fix exists to prevent, re-opened one level down.

Teaching indexOfferedTools to recurse into nested objects and array items is a real fix and a reasonable future change. It is deliberately not this branch's change. A fixup on this chain has introduced a new problem in four consecutive review rounds, and trading a converter that four rounds have now signed off for four more tools is not a trade worth making again. The branch is back to refuse-on-unsupported-keyword, which is the shape that was verified.

What survives from #402 is the other port, parallel_tool_calls: false above — that one is a correctness fix and is not implicated in any of this.

Coverage, measured here over the real DEFAULT_TOOL_DESCRIPTORS rather than a sample: 76 of the 82 emitted functions carry strict: true (77 of the 82 registered schemas convert; the emitted figure is one lower because reply's hand-tuned minLength: 1 schema in the adapter, not its registry schema, is the one that ships). The six refusals:

tool	why
os.http.request	headers is a typed open map; body may be any object
mcp.prompt.get	arguments is a map of arbitrary string keys
os.fs.archive.extract	a nested limits that declares properties and says additionalProperties: true
fusion.delegate	minItems/maxItems/minimum on the task list
vision.describe	maxItems on paths
reply	minLength: 1 on text
any descriptor with no schema	the {properties:{}, additionalProperties:true} fallback has no strict form short of making the tool zero-argument

In the first two the map is the payload, so closing it would delete the tool's arguments. The other four are the bounds-and-open-object refusals restored by the revert above.

Nesting is bounded separately, at five levels. The allowlist bounds what a node may say; nothing bounded how many of them there are. Object properties, array items and anyOf branches all count as a level. Built-in descriptors reach two (os.shell.run), so only a third-party inputSchema gets near it — and a schema past a strict compiler's ceiling is rejected with the whole request, every other tool's definition with it, which is the one outcome the per-tool refusal exists to avoid. The same bound answers a self-referential schema — through properties, items or anyOf alike — with a refusal instead of a RangeError escaping descriptorsToOpenAiTools.

The keyword allowlist is the safety property, per node shape. Every emitted node is a spread of the node that came in, so an allowlisted keyword the node's shape has no rule for would ride out unconverted inside a schema we then mark strict — enum on an object node emitting its raw sub-objects verbatim is the sharp case; items on an object, or the object keywords on an array or a union, are the same bug with different keys. Each shape refuses its own strays.

Third-party MCP schemas. The converter has to survive whatever an MCP server ships, and be worth something there rather than just not crashing. All three spellings of nullable are accepted and never widened twice — type: ["string","null"], an anyOf with a {type:"null"} branch, and a bare {type:"null"} — which makes the conversion idempotent (feed its own output back in, nothing changes) and makes a pydantic/FastMCP Optional[str] convert. default, $schema and $comment are accepted and dropped: a strict decode has no absent key for a default to fill, and the key is deleted again on the way in, so the server applies its own default exactly as today. Still refused, deliberately: $defs/$ref (a pydantic model nested in another one — resolving references faithfully is a separate change), any bound or format, and an object with no additionalProperties at all. Absent means open in JSON Schema; closing it would forbid arguments the server accepts today, and would publish a zero-property MCP tool as a zero-argument tool. Our own descriptors spell additionalProperties: false out on every object, so requiring it costs the built-ins nothing.

The one non-cosmetic rewrite is optionality, and its undo is per argument. An optional property is unioned with null and moved into required, so the model now answers "cwd": null where it used to omit the key. That is not harmless: memory.profile.set (pinned, keywords), memory.notes.recall (id) and os.git.init (userName/userEmail) branch on rawArgs.x !== undefined and would take a branch a literal null does not mean. So openAiToolCallsToBatch drops top-level nulls again — but only for the arguments the converter actually moved, via the strictWidenedArgs map (escaped function name → the argument names whose optionality the rewrite erased) that the adapter reports for the same descriptors.

Two kinds of null therefore survive, and both have to:

• every argument of a tool whose schema was refused. It went out byte-identical to the flag-off payload, so its nulls are the model answering the tool's own schema.
• an argument of a tool that did convert but was already required. The converter only widens what it moves, so such an argument is also emitted byte-identical — and if it is also nullable (z.string().nullable() through the official MCP SDK: anyOf: [{string},{null}], listed in required) the model means the null literally, and deleting the key would hand that server a call missing a required field.

Nested nulls are left alone in every case — a null deeper inside an argument is data the model meant to send. That is safe because of a premise, not because nesting is handled: no schema this converts contains a nested object. The two built-ins that nest are both refused (see the table above). The premise is load-bearing twice — the tagged-call narrowing below walks the top level too — so it is now pinned directly over the real emitted payload rather than asserted in a comment, and the pin fails if either nesting tool is ever converted.

Tagged-tool providers read the strict payload differently. A qwen-openai-compatible link answers with <tool_call> prose that we parse ourselves; no strict decoder is involved, so nothing stops the model omitting an optional. Read literally, the strict payload's inflated required made coerceArguments reject every realistic tagged call for a missing parameter, and the whole tool call collapsed into text — turning the level on would have made tool calling strictly worse there. indexOfferedTools now reads a strict: true function's required the way strict means it: a listed property that admits null is optional. That costs only a presence check the tool's own validator makes again, and it holds on whichever link the fallback chain serves. It rewrites the top-level required only, which is as far as it needs to reach while nothing convertible nests an object — the premise pinned above. A built-in that nested one would have to teach indexOfferedTools to recurse in the same change; that is what the retraction at the top of this description is about.

Plumbing mirrors supportsParallelTools exactly: modelWantsStrictTools (its own module, per-model rather than a provider capability, re-read per inference so a hot-swapped model is seen immediately) → AgentLoopDependencies.strictTools / ResolvedTurnLlmSlice → StepDependencies.strictTools → both directions of the adapter, the incoming one keyed by what the outgoing one actually changed. Both directions share one build of the function definitions rather than converting every registered schema twice per inference.

Test evidence

• npx vitest run src/llm src/config src/agent → 113 files, 1661 tests, all pass (1653 before this round; the 8 net new ones are listed below)
• npx vitest run src/llm/provider/openai → 14 files, 193 tests, all pass (188 before this round). The previous revision of this description said 193 while the head at the time emitted 197; that head has been reverted and 193 is the measured figure now.
• npm run lint (tsc --noEmit) → clean
• The three earlier rounds' scenarios re-run against the new head, not just the new tests: a required-nullable argument of a converted tool keeps its null while the widened optional beside it is dropped; a refused tool's deliberate body: null survives untouched; a <tool_call> tagged response that omits optionals still parses against the strict payload; a 12-deep and a self-referential schema are both refused rather than emitted or blowing the stack; and the flag off still emits no strict marking anywhere.
• Flag-off byte identity is checked, not asserted. descriptorsToOpenAiTools on the branch and on origin/main produce identical JSON over the built-in schemas, with the option absent, {}, and {strict:false}.
• Checked the tests fail without the src change. Reverting only openai-tool-call-adapter.ts and re-running its suite fails exactly the behavioural cases while the two control cases ("changes nothing when the option is off", "leaves nested nulls alone") keep passing. Each individual fix was re-checked the same way by mutating it back: batch-level null drop → the adapter's required-nullable case and the step-executor wiring case both fail; per-tool (rather than per-argument) keying → 2 failures; reading the tagged payload's required literally → the qwen strict case fails; absent additionalProperties treated as closed → 4; out[name] = ... for __proto__ → 1; no type-array support → 4; no annotation stripping → 2; non-idempotent nullable() → 2.

New this round, net of the revert: four parallel_tool_calls cases in openai-build-body.test.ts, three in step-executor.test.ts (including the end-to-end one that runs the captured step params through buildOpenAiChatBody and reads parallel_tool_calls off the wire body), and one pin in openai-tool-call-adapter.test.ts — "emits no nested object inside a function it marked strict" — which walks every emitted strict schema through properties, items and anyOf. That pin is the control for the retraction: restoring the reverted converter makes it fail with the two nested objects named above, so a future attempt at the same coverage fails in the suite rather than on somebody's tagged link.

Existing coverage: strict-tool-schema.test.ts (the promotions, enum widening, union widening, recursion, non-mutation, the three nullable spellings, idempotence over every convertible built-in schema, dropped annotations, __proto__, which properties the rewrite widened, and the refusals — bounds, map objects, $ref, $defs, oneOf, anyOf beside a sibling type, open objects, per-shape stray keywords, over-deep and cyclic schemas through both the properties and the anyOf spelling, plus a pinned refusal list and count over all 82 registered schemas); strict cases in openai-tool-call-adapter.test.ts (flag-off byte identity, arbitrary MCP inputSchemas refused rather than blowing up, a pydantic-shaped one converting, a refused tool keeping its nulls, and a converted tool keeping the null of an argument that was already required); model-strict-tools.test.ts for the config leg; a strict block in qwen-tagged-tool-response-adapter.test.ts; and end-to-end cases in step-executor.test.ts that run a native-tools step with the level on and check every half of the contract on the real wiring.

What this does not cover

• Untested against a live strict endpoint. Every assertion here is about the payload we emit; nobody has run it against mercury-2.5, or against OpenAI itself. The reporter is the natural first tester. The shape most likely to draw a provider's first 400 is the widened enum — type: ["string","null"] with null appended to enum, on 18 emitted properties (os.fs.list.kind, os.fs.hash.algorithm, github.pr.list.state, ...) against 2 for the anyOf + {type:"null"} branch. If a provider rejects it, the fix is to tighten the converter (refuse an optional enum outright) — not to loosen anything. The one preflight that could have caught it cannot: run-contract-probe.ts builds its own request body and sends a single unmarked tool, bypassing buildOpenAiChatBody, so no probe has ever put a strict function, a widened enum or the parallel_tool_calls floor in front of a real endpoint. Routing the probe through the body builder is where a first 400 would be cheapest to find, and it is the obvious next change — out of scope here.
• The strict decision is resolved before the fallback chain picks a link. It comes from the active (or pinned) provider's model, exactly as supportsParallelTools does, so a cross-provider fallover ships the strict-marked payload to a link whose model never declared the level. It is inert on a provider that ignores strict, and the tagged-decoder path above is handled wherever the payload lands, but a provider that rejects the field would 400 the fallover. Re-resolving per link means rebuilding tools inside llm-fallback-seam.ts, which has neither the descriptors nor the adapter — the same gap supportsParallelTools has today, and a maintainer call to fix for both at once.
• A genuinely-required nullable argument is optional to the tagged decoder. Strict cannot distinguish "required and nullable" from "optional", so the qwen-tagged narrowing above treats both as optional. That is the lenient direction: worst case a tagged call missing a required argument reaches the tool, whose own validator rejects it — against the alternative, which was every tagged call disappearing into prose.
• $defs/$ref MCP schemas do not convert. A pydantic model nested inside another one is the common shape that still ships non-strict.
• The refusals are not a fixed list, and the pin over them is narrower than it looks. A new default tool with a map-shaped or $ref argument, or a bound, silently joins the refusals. The pinned test in strict-tool-schema.test.ts iterates DEFAULT_TOOL_NAMES, a hand-written literal array of the 82 names (DEFAULT_TOOL_ARGS_SCHEMAS is not exported, so it cannot be reflected) — so it catches a bound added to one of those 82, or one of them disappearing, and it does not catch a brand-new tool, which is the case that sentence used to claim. Verified: adding minLength to finish.text fails the pin; adding a whole new entry leaves the suite green. Coverage can therefore drift down in two directions without a red test — the other being reply's hand-written adapter schema, since the emitted-side count pin went out with the revert. It degrades gracefully in both (the tool just ships non-strict), but it is not the guarantee the old wording promised.
• A bound costs the tool strict mode. Four built-ins are refused over bounds the strict compiler would have ignored anyway, so they ship unconstrained and unbounded. Recovering them is what the retraction above is about, and doing it safely means the indexOfferedTools recursion first — not the strip on its own.
• Nested optionals in third-party MCP schemas. No built-in schema that converts has a nested object (pinned), but a third-party MCP schema can. If one converts and nests, its inner optionals arrive as explicit null and reach the server that way, since the null-drop is top-level only — and on a tagged link the same nesting is outside what indexOfferedTools narrows. Both walks would need the same recursion.
• No TUI surface. The level is set by hand in the config file; the Providers tab does not offer it.
• Nothing here touches the grammar/local path — llama-server ignores tools entirely.

On PR #402

For whoever is choosing between the two branches: #402 reaches its higher coverage — 80 of 82, the figure this branch has just given up — by exactly the strip-don't-refuse rule retracted above. Its openai-strict-tools.ts strips minItems, maxItems, minimum, minLength, maxLength, pattern, format, default, examples, uniqueItems and const, which is what makes fusion.delegate and os.fs.archive.extract convert there too. #402 also enables the flag on qwen-openai-compatible (one of its five provider kinds) and does not touch qwen-tagged-tool-response-adapter.ts at all — it has no equivalent of the indexOfferedTools narrowing, at any level. Verified read-only against #402's diff at 0fd12f11 (14 changed files); nothing on that PR or its branch was touched.