SHARK-3619/3620/3622: stop the key-lifecycle writes describing a world the data plane does not produce - #34
Merged
mikhailak merged 3 commits intoAug 7, 2026
Conversation
… world the data plane does not produce Three defects found by walking create -> call -> restrict -> freeze against prod on 2026-08-07, all the same shape: a reply or a description asserts something the proxy contradicts. SHARK-3619. The create reply said the ready URL "works immediately". A newly minted key answers -32050 "API key not found" under HTTP 401 for roughly 60 to 90 seconds first, and every plausible reading of that error is wrong: create it again, escalate, or report MCP key creation as broken. The claim about TIME is gone; the claim about SETUP (no session, no header) survives, because that is what makes the URL the shortest path to a first call. The create path adds a measured propagation note naming the code, the words the proxy uses, and the verdict. Reveal does not carry it: its key already exists. SHARK-3620. The tool description said "The secret key material is never returned in the tool output" while the reply carries the endpoint token in full plus a URL with it embedded. The approval page was already accurate, so the description now IS that sentence rather than a second wording of it, shared as one constant and pinned by identity rather than by two regexes that can drift apart. Found while wiring that pairing: the approval page CLIPS every effect at 200 characters, and three lines were over it -- create's credential disclosure (cut at "which are live credentials", losing where they land), the platform-key mint's "without asking a human to approve anything and without a second factor", and the bulk logout's blast radius, whose truncation depended on how many sessions the account had. All three now sit under the bound, and no gated page may ship a truncated consequence again. SHARK-3622. freeze neither read the resulting state back nor mentioned the lag, while the allowlist write on the same key one minute earlier did both. It now reads the status back from the control plane -- authoritative immediately, unlike the data-plane propagation the allowlist writes rightly refuse to race -- and states the measured lag per direction: 10 to 21 seconds for freeze, unmeasured for unfreeze rather than borrowed from it. A read-back that disagrees with the request is reported without isError, because a separate route can trail a write and "retry me" here costs a second human approval. Gates: format, lint, typecheck, build clean; 1639/1639 tests; coverage on the changed files 98.3% lines / 90.5% branches / 100% functions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…not only the disagreeing one A client branching on the field would otherwise have to read `undefined` as agreement, which is the trap writeOutcome.ts closed for `observed`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…them my own vacuous assertion Mutation run over freezeApiKey.ts (Stryker, repo config, 132 mutants) scored 67.42 and surfaced two survivors worth acting on. 1. The date assertion on the freeze propagation note was VACUOUS. The fixture key was named "acceptance-2026-08-07", so /2026-08-07/ matched the key's LABEL in the reply and would have passed with the date deleted from the note entirely -- which is exactly what the surviving mutant did. The fixture key is now "acceptance-run" and the assertion tests the note it was written for. 2. Flipping `||` to `&&` in the read-back guard survived: with an absent body the mutated guard throws reading `.frozen`, the throw is caught one frame later, and the result is still "accepted, not observed" -- passing every assertion. The unobserved test now pins the REASON, and a new case covers the other half of the guard: a body that exists but whose `frozen` is not a boolean, which must not render as `frozen: undefined`. Both re-verified by hand mutation: each mutant now fails, and the source was restored byte-for-byte (md5sum checked, not `git diff`). 1640/1640 tests; format, lint, typecheck, build clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
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.
Closes the three management-plane defects from the 2026-08-07 acceptance run against prod
mcp.ankr.com(chart0.4.0-rc.1, mgmt9176d12). All three are the same shape: a reply or a description asserts something the proxy contradicts. The fourth ticket from that run, SHARK-3621, is in shark-proxy and is not in this PR.SHARK-3619 the create reply promised immediate usability
A newly minted key answers
-32050 API key not foundunder HTTP 401 for roughly 60 to 90 seconds, and the reply said the URL "works immediately". Every plausible reaction to that error is wrong: create the key again (another human approval, mints nothing), escalate, or report MCP key creation as broken.The claim about TIME is gone. The claim about SETUP survives, because that is the useful half and it is true on both paths: the URL needs no session, no header, no client. The create path then adds a measured note naming the code, the words the proxy uses, and the verdict that this is not a failed creation.
mgmt_reveal_api_keyrenders the same surface and does NOT carry the note, since its key already exists and is already known to the proxy: the renderer is shared, so the caveat is a parameter rather than an edit to the shared string.The number carries its date, so a future edit has to change both.
SHARK-3620 the description denied a disclosure the reply performs
mgmt_create_api_keysaid "The secret key material is never returned in the tool output" while the reply carries the endpoint token in full plus a ready-to-call URL with it embedded. The approval page for the same call was already accurate and already separated the two secrets.So the description now IS that sentence, exported as one constant used by both call sites, and the test asserts IDENTITY rather than matching two regexes: two wordings that agree today are two wordings that can drift, and each side reads fine on its own while drifting.
Found while wiring that pairing: the approval page stores every effect through
clip(e, 200), silently, and three lines were over the bound:mgmt_create_api_keymgmt_create_platform_api_keymgmt_logout_other_sessionsThe truncation was known:
mgmt-sessions.test.tscarried a comment saying the store "CLIPS a long effect and a clipped assertion would only cover its prefix", and pinned the SOURCE instead. That is the workaround, not the fix. All three lines are split under the bound, andmgmt-gated-display.test.tsnow fails any gated page whose stored effect arrives truncated, for every call site rather than the ones that happened to be caught.SHARK-3622 freeze neither read back nor warned
mgmt_set_blockchain_allowliston the same key, one minute earlier, reads the resulting state back and warns that enforcement lags. freeze did neither: it told the caller to go and runmgmt_get_api_key_statusthemselves.It now performs that read. This is NOT the read-back the allowlist writes deliberately refuse: theirs would race the 45-100 second DATA-plane propagation and manufacture false failures, while this asks the CONTROL plane what it now holds, which is the same store the write just went to. Three outcomes, each stating exactly what it knows:
_meta.observed = true,matchesRequest = true;frozen, or a throw): today's "ACCEPTED, NOT observed" wording plus the reason,_meta.observed = false;isError. A separate route can trail a write, so the shim cannot tell "did not apply" from "read too early", andisErrorreads as "retry me" to most agents, where a retry costs a second human approval for a change that may already be in place. The disagreement travels in_meta.matchesRequest.The lag is stated per DIRECTION and never averaged: freeze was measured at 10 to 21 seconds on 2026-08-07 (faster than the allowlist's window, which is worth knowing before anyone writes a wait into a runbook), and the unfreeze direction says it has not been measured rather than borrowing the number.
Verification
format,lint,typecheck,buildclean; 1640/1640 tests. Coverage on the changed files 98.3% lines / 90.5% branches / 100% functions.G9, executed rather than transcribed. The wording the create note now publishes was checked against the live endpoint, not against memory:
Mutation (Stryker, repo config, 132 mutants on
freezeApiKey.ts): 67.42%, above the configured break threshold of 60. Two survivors were worth acting on and both are fixed in the last commit:acceptance-2026-08-07, so the regex matched the key's LABEL in the reply and would have passed with the date deleted from the note entirely;||to&&in the read-back guard survived, because the mutated guard throws, the throw is caught one frame later, and the result is still "accepted, not observed". The test now pins the REASON, and a new case covers a body whosefrozenis not a boolean, which must never render asfrozen: undefined.Both re-verified by hand mutation, with the restore checked by
md5sumrather thangit diff. The remaining survivors are unpinned wording fragments in the tool description and the pre-existing gateway-throws catch path, neither introduced here.Merge order. Targets
integ/mcp-prod-readiness, independent of #32.