Skip to content

fix(providers): shape tool schemas to the wire so the door survives - #2227

Merged
justinhelmer merged 1 commit into
mainfrom
plan/fix-switchboard-2189-it-f3bfb8/u1
Sep 22, 2026
Merged

justinhelmer merged 1 commit into
mainfrom
plan/fix-switchboard-2189-it-f3bfb8/u1

Conversation

@justinhelmer

@justinhelmer justinhelmer commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Pointing the model defaults at first-party OpenAI took the whole door down: four of the operator's tools carry a JSON-schema pattern with a regex lookaround, the Responses API rejects the entire request with a 400 before generating anything, and every ask fell through to general. The proxy now strips constructs measured as refused and records each one; only boundary-vouched evidence naming an offered tool and keyword lets the operator re-ask without that tool.

Why: Closes #2189 and #2188. Production sits on OpenRouter today purely because of this. It is one of three fixes gating the switch to first-party OpenAI, with #2190 and #2099; it builds on the typed provider-failure seam from #2195 rather than beside it.

Where to look

  1. The per-wire shaping walks every tool on the request and recurses the whole schema, returning one typed degradation per tool and keyword. ⚠ Other wires must keep their schemas byte for byte; only Responses loses anything.
  2. What Responses refuses is the one measured mismatch, lookahead and lookbehind. ⚠ An unmeasured mismatch must recover through boundary evidence, not fall through to non_decision.
  3. The operator's evidence-gated re-ask consumes tool-and-keyword evidence minted at the model-proxy boundary and removes only that offered tool before re-asking. ⚠ A generic request-rejected 400 remains a refusal; inference from the cause alone would reopen F1.
  4. The conformance row builds the operator's real 52-tool catalogue, pins the four known lookaround carriers, and renders all three wires' native tool shapes.
  5. The re-ask test proves the event names the tool and the keyword, so a degradation is never silent.

Feedback wanted: Please scrutinize whether the boundary's exact-one quoted tool and keyword gate can misattribute provider prose, and whether four schema repairs is the right bound when evidence names an absent or repeated tool.

Risk: If the shaping is wrong the door goes down on OpenAI exactly as it does today, which is total: no review, coding or ship ask can start. If it over-strips, a tool silently loses an input constraint and falls back to its own validation. Blast radius is every operator turn on the Responses wire; rollback is a revert plus the config staying on OpenRouter, which is where it already is. 400 changed lines across 4 files, all in the two named seams — no split considered, since the proxy half and the operator half are useless apart.

Verified: 125 focused tests plus hygiene:check, specs:check and typecheck passed on the branch; full suite is CI-gated, and the live-API proof is human-gated below.

Decisions (3)
  • Shaping lives at the wire, not at the tool. The obvious fix is to rewrite REPO_SLUG in src/core/delivery.ts and move on. Measuring the emitted catalogue killed that: 52 tools, 21 patterns, and four carrying the lookaround shape — delivery_report, pulls_enqueue, pulls_merge, pulls_rebase — and one refused tool fails the whole request. A per-tool fix leaves three live and reopens the moment a fifth is written.
  • The re-ask is keyed to boundary-vouched schema evidence, not the typed cause alone. A request-rejected 400 names only the failure class. The proxy mints authenticated tool-and-keyword evidence only when provider prose identifies schema validation, exactly one offered tool and exactly one keyword in its sent schema; a generic 400 remains a refusal.
  • A degradation is recorded, never dropped silently. Losing a constraint changes what the model can be asked for. One typed degradation per tool and keyword, deduped, keeps that on the run record instead of leaving a quieter schema than the author wrote with nothing to show for it.
Validation (9 criteria)
Criterion Proof
Each wire receives the operator catalogue clean of every construct known to be refused there src/channels/modelProxy.test.ts::tool-schema conformance on each wire::the operator catalogue is clean of every construct known to be refused on each wire — builds the live 52-tool catalogue and asserts the four lookaround carriers by name
Wires other than Responses keep their schemas unchanged same test: lookaround patterns still number 4 on anthropic-messages and openai-chat, 0 on openai-responses
Native Responses patterns and property names survive byte for byte src/channels/modelProxy.test.ts::keeps native Responses patterns and property names byte-for-byte
A schema 400 on the operator's call is re-asked without the offending tool, not floored to non_decision src/core/dispatch/operator.test.ts::runOperator — the loop over a scripted model::an explicit schema rejection is re-asked without its tool even when the keyword is unmeasured
The run record names the refused tool and the schema keyword same test asserts the violation text provider rejected tool "…" schema keyword "…"; re-asked without that tool
Explicit schema repairs do not consume the ordinary structured-violation budget (F1) src/core/dispatch/operator.test.ts::runOperator — the loop over a scripted model::schema re-asks have their own budget after an ordinary structured violation
Amended model-proxy.md item 1 limits shaping claims to constructs measured as refused src/channels/modelProxy.test.ts::tool-schema conformance on each wire::the operator catalogue is clean of every construct known to be refused on each wire
Amended routing-and-config.md item 29 binds schema repair to boundary evidence and its own four-repair budget src/core/dispatch/operator.test.ts::runOperator — the loop over a scripted model::an explicit schema rejection is re-asked without its tool even when the keyword is unmeasured; ::schema re-asks have their own budget after an ordinary structured violation
The shaped catalogue is accepted by the real OpenAI Responses API human-gatedshapeToolSchemasForWire is exported for exactly this probe; no receipt yet, and it is the only pre-flip proof, since the operator resolves its model from config alone and no per-user trial reaches it
For agents

The head e922e38dc618fa2d49a8f3eb936169cde16eb8e0 is the review-reworked branch, squashed to one commit. This pull request was opened from the operator session rather than by the runner: the ship parent aborted twice without opening one — attempt 1 as #2197 (clean handoff read as "ended before ready"), attempt 2 with operator: {outcome: "non_decision", reason: "the operator failed: answer cut at the output cap (374 tokens)"}, which is #2099. Both pushes are recorded by: "salvage". No code was written or amended in this description-only refresh; the branch head was left untouched.

shapeToolSchemasForWire(shape, body) is the named export a live-API probe should import; it is a no-op for any shape other than openai-responses.

This PR does not close the OpenAI flip risk on its own. The output cap (#2099) is a separate gate: the cap is derived for answer text alone while the production openai block sets capField: max_output_tokens, which on the Responses API counts reasoning tokens too, so at effort medium a reasoning turn can spend it before emitting a tool call. On 2026-09-21 that was masked, because the schema 400 fails at request validation before generation ever starts. Do not read a green review here as the door being safe on OpenAI.

To reproduce the original defect: build the operator catalogue, send it to /v1/responses, and observe invalid_request_error / invalid_json_schema before any tokens.

@coreplane-switchboard coreplane-switchboard Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested: The retry path infers a schema rejection from an unrelated 400, and the changed wire and operator contracts are not reflected in their specs.

Warning

Changes requested · head 810f0be · 3 findings: 1 major, 2 minor

Severity Finding Where
major F1 Any HTTP 400 is misattributed to the first lookaround tool src/core/dispatch/operator.ts:1170
minor F2 Spec contradiction — model-proxy.md item 1: tool definitions are no longer forwarded byte-for-byte docs/reference/specs/model-proxy.md:11
minor F3 Spec contradiction — routing-and-config.md item 29: a schema 400 now retries instead of stopping as a typed refusal docs/reference/specs/routing-and-config.md:52
Full review

F1 — Major, high confidence. request-rejected plus status 400 does not prove a tool schema caused the failure. A 400 for an unsupported model option will remove an unrelated lookaround-bearing tool, record a false diagnosis, and retry routing with reduced capabilities. The real catalogue also contains four such tools while the retry budget is two. Carry explicit schema-rejection metadata—including tool and keyword—from the provider boundary, and retry only when that evidence exists.

F2 — Minor, high confidence. Item 1 promises tool definitions reach the provider byte-for-byte, and the Responses validation criterion repeats that guarantee. The proxy now removes unsupported pattern fields. Update the behavior and validation row to document per-wire schema shaping and bind the new conformance test.

F3 — Minor, high confidence. Item 29 explicitly says an OpenAI schema 400 renders a typed refusal and stops. The implementation now retries without a tool. Amend the behavior and validation criterion to describe the intended exception and its audit record.

@justinhelmer

Copy link
Copy Markdown
Contributor Author

Adding a fourth finding, minor, to the round at 810f0be. It is the question the description asked under Feedback wanted, which the review did not reach; a verdict that does not mention it leaves the guard's scope undocumented, so it goes on the record here rather than being dropped.

F4 — responsesRejectsPattern is a hardcoded list of one, and neither the comment nor the conformance row says so.

responsesRejectsPattern tests for lookahead and lookbehind alone. The issue asked for lookaround "and any other construct it rejects". Today that is safe: the emitted catalogue's 21 patterns carry nothing else, and the conformance row proves it against the wire's validator. But the supported subset belongs to the provider, not to us, and it can narrow without notice.

This is not a request to enumerate the Responses validator. It is a request that the code tell the truth about what it covers:

  • the comment on responsesRejectsPattern should say it is the one measured mismatch and that the operator re-ask is what covers an unmeasured one, rather than reading as a complete guard;
  • the conformance row's name and assertions should say the catalogue is clean of known-refused constructs on each wire, not that no refusable construct can exist.

The reason this stays minor rather than major is F1's mechanism: an unknown refused construct costs one failed call and one degraded tool, not the door — provided F1 is fixed so the re-ask fires on evidence. If F1's fix narrows the re-ask to explicit schema-rejection metadata from the provider boundary, please make sure an unrecognised construct still reaches that path rather than falling to non_decision, since that is the entire backstop this finding relies on.

Related, and worth folding into F1's fix: STRUCTURED_RETRIES_MAX is 2, four catalogue tools carry the lookaround shape, and the schema re-ask shares that counter with ordinary structured violations. A round that spends a violation or two before a schema 400 has no re-ask left.

@justinhelmer

Copy link
Copy Markdown
Contributor Author

Re-review requested at e922e38d — all four round-1 findings addressed.

F1 (major), the misattributed 400. The operator no longer infers the offending tool by scanning the prompt for the first lookaround carrier. The model-proxy boundary mints explicit schema-rejection evidence naming the tool and the keyword, and the re-ask fires only when that evidence exists, removing only the tool it names — so a 400 raised by something else, an unsupported model option for instance, no longer drops an unrelated tool and retries with reduced capabilities. The schema re-ask budget is separated from the structured-violation counter, so a turn that has already spent a violation still has its re-ask.

F2 and F3, the spec contradictions. docs/reference/specs/model-proxy.md item 1 no longer promises tool definitions reach the provider byte for byte; it describes per-wire shaping and binds the new conformance test. docs/reference/specs/routing-and-config.md item 29 no longer says a schema 400 renders a typed refusal and stops; it describes the evidence-gated re-ask and its audit record.

F4, the guard's narrowness. The comment states that lookaround is the one measured mismatch and that the operator re-ask covers an unmeasured one, and the conformance row asserts the catalogue is clean of known-refused constructs rather than implying none can exist. An unrecognised construct still reaches the re-ask path rather than falling to non_decision.

History and description. The branch is squashed to one commit at the same tree the fixes were validated on — git diff --stat 520e7216 e922e38d is empty — because the third commit had replaced the approach the first shipped, so the trail read as two designs rather than one. The description is rewritten against this head with every anchor regenerated at e922e38dc618fa2d49a8f3eb936169cde16eb8e0; the previous body still described the inference F1 rejected.

ci / bot is green after a retry cleared an unrelated 5-second timeout in src/memoryWorkerDiagnostics.test.ts, recorded on #2119.

@coreplane-switchboard coreplane-switchboard Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested: The model-proxy path is fixed, but the direct pi adapter can still infer and remove an unrelated tool from a tool-less schema 400.

Warning

Changes requested · head e922e38 · 1 finding: 1 major

Severity Finding Where
major F1 Tool-less schema errors still select the first matching tool src/core/provider.ts:342
Full review

F1 — The direct pi adapter still mints schemaRejection when the provider names only a schema keyword. If several offered tools contain that keyword, carrying[0] arbitrarily attributes the rejection to the first and the operator removes an unrelated tool. Require exactly one provider-named offered tool and one named keyword; otherwise keep the generic typed refusal.

Co-Authored-By: coreplane-switchboard[bot] <318072483+coreplane-switchboard[bot]@users.noreply.github.com>
@coreplane-switchboard
coreplane-switchboard Bot force-pushed the plan/fix-switchboard-2189-it-f3bfb8/u1 branch from e922e38 to 439b802 Compare September 22, 2026 20:46
@justinhelmer

Copy link
Copy Markdown
Contributor Author

Re-review requested at 439b802a — the round-2 major is addressed, CI green on every check.

F1, the tool-less schema 400 in the direct pi adapter. providerSchemaRejectionOf now mints evidence only when the provider's prose names exactly one offered tool and exactly one keyword present in that tool's own sent schema. The carrying[0] fallback is removed, along with the inference that let "regex lookaround" imply the pattern keyword and bind to the first carrier. A keyword named by several offered tools, or a 400 naming no tool at all, now yields no evidence and stays a generic typed refusal instead of dropping an unrelated tool.

The regression: src/core/harness/piAi.test.ts — "a keyword-only schema 400 shared by several offered tools stays generic and drops no tool".

Three earlier guarantees remain pinned by their own tests, so this was not bought by undoing the previous round: an explicit schema rejection is still re-asked without its tool even when the keyword is unmeasured; schema re-asks still have their own budget after an ordinary structured violation; and a provider-rejected request without explicit evidence is one typed refusal, never a re-ask or a non_decision.

One judgement call flagged rather than asserted. With the lookaround-implies-pattern inference gone, a 400 that names a keyword but no tool has no recovery path left in the adapter. The live case is covered upstream — the proxy strips lookaround before the request is sent — but that narrowing is a deliberate trade, not an oversight, and it is the thing to push back on if it reads wrong.

The branch is one commit, rebased onto current main, which now carries #2190's fix.

@coreplane-switchboard coreplane-switchboard Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM: The direct pi adapter now requires unambiguous provider-named tool and keyword evidence, and the regression covers ambiguous keyword-only refusals.

Note

Approved · head 439b802 · no findings

Full review

The prior finding is resolved, and no new issues remain.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-approved: coreplane-switchboard[bot] reviewed this PR and posted an LGTM verdict (see its review). This repository opted in through its REVIEW_BOT_LOGIN and REVIEW_BOT_ID variables.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

operator: OpenAI Responses rejects a tool schema regex lookaround, so every plain ask falls to general

1 participant