fix(sdk): read MCP verdicts fail-closed; complete the prompt options - #7
Merged
Merged
Conversation
Two follow-ups from cross-checking the TypeScript SDK against the Python one.
Verdict reading was fail-open. The verdict methods returned the gateway's body
verbatim, so `if (verdict.outcome === 'deny')` was false for an empty 200, an
unknown outcome value, or a proxy error page — and the caller proceeded with the
tool call. A default-deny policy boundary has to survive the client, not just
the gateway: only an explicit allow is now an allow, and anything unparseable
denies (run steps stop) with the payload preserved under `response` so the cause
stays debuggable. Same change in the MCP server's shared helper.
A 4xx carrying no verdict still throws rather than reading as a deny. Axum
rejects a malformed body with a plain-text 422, and turning the caller's own bug
into a policy refusal would bury it. Fail-closed applies to reading a decision
the gateway actually made, not to inventing one it never sent.
Prompt options were incomplete. `missing` ("error" | "empty") was unreachable,
so callers could not opt out of the default hard failure on an unsupplied
variable; the `x-routeplane-cohort` header had no typed route, leaving sticky
A/B assignment unusable; and `complete()` threaded only `model`, so temperature,
max_tokens and the rest of the chat body were unreachable. `variables` widens to
any JSON, matching the gateway. All additive — existing call sites are
unchanged.
Verified the fail-closed tests fail against the old pass-through: reverting the
normalizers turns exactly those 12 red, and the 422-still-throws case stays
green.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #6, from cross-checking against the Python SDK (routeplane-core/routeplane-python#1). Two real gaps; one reported issue that turned out not to apply here.
1. Verdict reading was fail-open
#6 returned the gateway's body verbatim. That is fine when the gateway answers as documented, and wrong the moment it doesn't:
A default-deny boundary has to survive the client, not just the gateway. Only an explicit
outcome: 'allow' | 'deny'is honoured now; anything else denies (and run steps stop) with the unparsed payload preserved underresponseso the cause is still debuggable. Same fix in the MCP server's sharedverdict()helper.A 4xx carrying no verdict still throws. Axum rejects a malformed body with a plain-text 422; reading that as a policy deny would bury the caller's own bug. Fail-closed applies to reading a decision the gateway actually made — not to inventing one it never sent.
Verified the guard is real rather than tautological: reverting the two normalizers to the old pass-through turns exactly those 12 tests red, and the 422-still-throws case stays green.
2. Prompt options were incomplete
Checked against
prompts_api.rs/crates/prompts:missing("error" | "empty", gateway defaulterror) was unreachable — callers could not opt out of a hard failure on an unsupplied variable.x-routeplane-cohorthad no typed route, so sticky A/B assignment was unusable fromPromptResourceeven thoughcreateHeadersalready supported the header.complete()threaded onlymodel, sotemperature,max_tokens,streamand the rest of the flattened chat body were unreachable. Nowoverrides.variableswidens fromRecord<string, string>to any JSON, matching the gateway'sBTreeMap<String, Value>.All additive — existing call sites are unchanged.
3. Not applicable here
The Python SDK was sending
providerin the completions body, where the gateway flattens it into a chat request that ignores unknown fields — a silent no-op. TypeScript was already correct, sendingx-routeplane-provideras a header. Added a regression test pinning that, plus a doc note onoverridesexplaining why routing options must not go in the body.Verification
pnpm build,pnpm test,pnpm lintpass. 77 tests, up from 61: 16 new covering the fail-closed matrix across all four verdict methods, the 422-throws boundary, the payload preservation, and the three prompt-option paths.