Skip to content

SHARK-3629/3635: serve the chain reads from the management endpoint, and stop paying for a tokenizer nobody reached - #36

Merged
mikhailak merged 6 commits into
integ/mcp-prod-readinessfrom
feature/SHARK-3619-3622-mgmt-truthfulness
Aug 9, 2026
Merged

SHARK-3629/3635: serve the chain reads from the management endpoint, and stop paying for a tokenizer nobody reached#36
mikhailak merged 6 commits into
integ/mcp-prod-readinessfrom
feature/SHARK-3619-3622-mgmt-truthfulness

Conversation

@mikhailak

@mikhailak mikhailak commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Follows PR #34 on the same branch, which is already merged. Targets integ/mcp-prod-readiness so it reaches #28.

SHARK-3629: the default entry point had 75 management tools and zero chain reads

Asked for an address balance, an agent connected to /mcp had no tool for it. The data tools existed on a second server behind a raw key in a header, and reaching them cost a second MCP entry, a hand-pasted credential and a client restart. Measured end to end on 2026-08-07; the restart is what actually stopped people.

/mcp now registers the same sixteen data tools /rpc does. registerDataTools is imported rather than copied, so the two endpoints cannot drift into different surfaces. The key they spend is resolved SERVER-SIDE from the account the session is already signed in as: slot 0, the Default project key, or whichever slot mgmt_select_key names. Nothing is pasted and no credential enters the conversation.

The default selection moves from core to core plus data. It only ever ADDS tools, so nothing anyone depended on moved; the entry cost goes from ~2.4k to 8,823 o200k tokens against an 8,900 ceiling the test asserts. ?toolsets=core is how you ask for the old listing by name.

Two classification suites had to narrow their scope rather than grow their lists

This is the part worth reviewing carefully, because "add the names to the list" was the wrong fix and looked like the right one.

mgmt-annotations (SHARK-3540) and mgmt-role-capabilities (SHARK-3553) both enumerate everything registered on the management server and partition it. Both rulebooks are about acting on an ACCOUNT. A chain read is read-only and answers to a different contract, held in test/annotations.test.ts — so getAccountBalance was being told its readOnlyHint should be false, which is the wrong complaint about the right code.

The boundary is now DATA_TOOL_NAMES in src/server.ts, exported beside the registrar and held equal to the live /rpc surface by test/data-tool-surface.test.ts. Verified rather than asserted: dropping one name from that list fires all four gates at once, so a data tool cannot slip past either classification by claiming to belong to the other.

mgmt_select_key is a management tool that rides with the group, so it stays in both partitions. It registers through the account-scope wrapper (naming slot 4 while the session is aimed somewhere you did not expect is exactly the mistake worth refusing) and takes JwtManagerRead, the capability the key LISTING takes: a seat that may not see which projects exist has no business naming one by index.

Four defects found in review before this shipped

What Why it mattered
the resolved token was cached by SLOT ALONE mgmt_select_account moves a session between accounts, and slot 4 of one team is a different key from slot 4 of another. Every later chain read went out on the previous account's key, including the slot-0 default nobody selects. The session reported one account and billed another
the chain tools' contracts were not delivered on /mcp SHARK-3599 lifted that prose OUT of the 16 descriptions because instructions carry it, which is only true where the instructions do. The RAW BASE UNITS rule lives nowhere else, so an agent decoding a transfer would report an amount wrong by 10^decimals with nothing contradicting it. Contracts 2 to 5 are now a shared DATA_TOOL_CONTRACTS; contract 1 stays per-endpoint, because the bound key is the one thing the two planes genuinely disagree about
mgmt_select_key could name a key the account no longer has in that slot a session can delete and recreate a slot without leaving, and the cache sees neither write. It now always re-reads
the deferred client Proxy was THENABLE it answered every property with a function, so one await or Promise.resolve near it would call then(resolve, reject), resolve the real client, find no then, and never call either callback: a permanent hang on a request path with nothing in a log

SHARK-3635: the management binary loaded gpt-tokenizer at boot for sessions that never read a chain

Importing the data plane brought gpt-tokenizer with it: 65 MB and ~390 ms, paid at boot by every pod against a 512Mi limit and a 5 s HEALTHCHECK, including one serving ?toolsets=core, which has no chain tool in it at all.

The load now happens on first use, warmed by registerDataTools: the function that puts chain tools on a server is exactly the event after which a token count becomes reachable.

scenario                     RSS                gpt-tokenizer
import src/mgmt/server.ts    118 MB (was 177)   not loaded
?toolsets=core session       104 MB (was 176)   not loaded
default core+data session    176 MB             loaded
/rpc createServer            170 MB             loaded, as before

Those RSS figures are one run on one machine and are NOT what the test asserts. The first version of this branch did gate on them, with a 140 MB ceiling on the cold cases, and CI failed at 149 MB for a process that reads 107-118 MB locally — while the module registry correctly reported the tokenizer absent. Baseline RSS moves with the Node build, the GC and the transform cache, and at 149 cold against 176 warm the bands overlap across environments, so no portable ceiling separates them. The test asserts the MODULE LOAD, which is exact and identical everywhere, and prints the RSS for information.

createRequire rather than await import(), because countTokensDetailed is called synchronously from every tool response and an async deferral would ripple through tokenMeta and fourteen call sites for no behavioural gain. No fallback to chars/4 on failure: it throws, because silently reverting to the 40-60% understatement SHARK-3525 removed is worse than a loud failure.

And this is why mgmt_list_toolsets KEEPS its chars/4 estimate. The two are one decision: that tool is in core, i.e. on every session including the narrowest, so counting for real would pull the 65 MB straight back into exactly the connections this relieved.

Verification

typecheck, lint, format, build clean. 1742/1742 tests on the merged tree. Coverage: global 90/80/85 and mgmt-scoped 80/75/80, both pass. CI gate and Codacy both green.

Mutation (Stryker, repo config). toolsets.ts 97.17%, keySession.ts 95.45%, tokens.ts run over the changed logic. It earned its keep twice:

  • keySession.ts scored 68% on the first pass with fourteen survivors, and one showed that my own account-switch test was driving select, which always re-reads — so it would have passed against a cache keyed on the wrong thing entirely. Also that the provider accessor, which hands the AAPI client to eight registered tools, was reachable by no test.
  • On toolsets.ts, listPhrase was untested at three names, where slice(0, -1) and slice(0, 1) stop agreeing.
  • A pre-existing vacuous assertion in the >256 KB extrapolation path: it compared meta.token_count with d.tokens, i.e. the computation with itself, so two arithmetic mutations survived the whole suite.

Load-bearing gates re-verified by hand mutation with the restore checked by md5sum rather than git diff.

Merge notes for the reviewer

One real conflict in the merge from integ, and it was semantic rather than textual: SHARK-3607 added instrumentToolCalls, which must patch registerTool BEFORE any tool registers, while SHARK-3629 moved the registrations out of createServer into registerDataTools. Resolved by keeping both and ordering them.

That interaction leaves a gap neither side had on its own, and it is NOT fixed here. instrumentToolCalls is applied only in createServer, so the chain tools the management plane now serves are not counted by mcp_ankr_tool_calls_total. The same tool name is counted from /rpc and not from /mcp, so a per-tool rate read off that family understates real usage silently. Closing it is a one-line application of the same helper to the management server, left out because widening a metric's coverage changes what every existing dashboard and alert on those names means. Recorded at the call site and in DEPLOY-MGMT.md.

The e2e parity group will be red against the deployed build until this rolls out, which is what that group is for: it compares the deployment with this checkout, so red there reads as "deploy", not "fix".

integ/mcp-prod-readiness is 0 commits behind main, so #28 stays conflict-free.

mikhailak and others added 5 commits August 7, 2026 20:43
… MFA-gated synthetic one

findKeyBySlot read index 0 as the account's own synthetic key whenever no team
account was selected, so every key-addressed tool refused the slot a user can
see in their own listing, explaining itself with a second factor that has
nothing to do with it. Measured on prod: mgmt_list_api_keys shows
`index 0: Default`, and mgmt_get_api_key_status(index 0) answers "Slot 0 is not
a project key".

Two different routes were being treated as one. GET /auth/jwt/all is the project
listing and carries slot 0; GET /auth/jwt/getMySyntheticJwt is the account's own
key, MFA-gated and deliberately never wrapped here. A personal account's slot 0
now resolves through the listing like slots 1 and up, and the synthetic route is
still not reached on any path -- asserted, not assumed.

mgmt_reveal_api_key keeps its refusal for this slot. It shares the resolver, so
without an explicit guard this change would have silently reopened what
SHARK-3567 closed on purpose: a tool whose job is handing over a credential
makes a different trade from one that operates on a key without disclosing it.

One existing pin flipped rather than being deleted: the fixture it used has no
slot 0, so it now pins the refusal that stays true for it, an empty slot naming
the slots that exist, with no claim about second factors.

Gates: typecheck, lint, format, 1644 tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… by default

The advertised OAuth endpoint served 75 account-administration tools and not one
chain read. Asked for an address balance, an agent connected to /mcp had no tool
for it: the data tools lived on a second server behind a raw key in a header, and
reaching them cost a second MCP entry, a hand-pasted credential and a client
restart. The restart is what actually stopped people.

/mcp now registers the same sixteen data tools /rpc does -- registerDataTools is
imported rather than copied, so the two endpoints cannot drift into different
surfaces -- and the key they spend is resolved server-side from the account the
session is already signed in as: slot 0, the Default project key, or whichever
slot mgmt_select_key names. Nothing is pasted and no credential enters the
conversation. The default selection moves from `core` to `core` plus `data`,
which only ever adds tools; the entry cost goes from ~2.4k to 8,823 o200k tokens
against an 8,900 ceiling the test asserts.

TWO CLASSIFICATION SUITES HAD TO NARROW THEIR SCOPE RATHER THAN GROW THEIR LISTS.
mgmt-annotations (SHARK-3540) and mgmt-role-capabilities (SHARK-3553) enumerate
everything registered on the management server and partition it, and both rules
are about acting on an ACCOUNT. A chain read is read-only and answers to a
different contract, held in test/annotations.test.ts -- so getAccountBalance was
being told its readOnlyHint should be false, which is the wrong complaint about
the right code. The boundary is now DATA_TOOL_NAMES in src/server.ts, exported
beside the registrar and held equal to the live /rpc surface by
test/data-tool-surface.test.ts. Verified rather than asserted: dropping one name
from that list fires all four gates at once, so a data tool cannot slip past
either classification by claiming to belong to the other.

mgmt_select_key is a management tool that rides with the group, so it stays in
both partitions. It registers through the account-scope wrapper -- naming slot 4
while the session is aimed somewhere you did not expect is exactly the mistake
worth refusing -- and takes JwtManagerRead, the capability the key LISTING takes:
a seat that may not see which projects exist has no business naming one by index.

FOUR DEFECTS FOUND IN REVIEW BEFORE THIS SHIPPED, all pinned by a test:

  - The resolved token was cached by SLOT ALONE. mgmt_select_account moves a
    session between the accounts a login holds a seat on, and slot 4 of one team
    is a different key from slot 4 of another. The first resolution won, so every
    later chain read went out on the previous account's key -- including the
    slot-0 default nobody selects. The session would report one account through
    mgmt_whoami and the account line every wrapped tool prints, while the reads
    were billed to another. The cache key is now the account plus the slot.
  - The chain tools' own contracts were not delivered on this endpoint. SHARK-3599
    lifted that prose OUT of the 16 tool descriptions because instructions carry
    it, which is only true where the instructions do; here they did not, and the
    RAW BASE UNITS rule lives nowhere else, so an agent decoding a transfer would
    have reported an amount wrong by 10^decimals with nothing contradicting it.
    Contracts 2 to 5 are now a shared DATA_TOOL_CONTRACTS both planes compose
    from; contract 1 stays per-endpoint, because the bound key is the one thing
    the two genuinely disagree about.
  - mgmt_select_key could name a key the account no longer has in that slot: a
    session can delete and recreate a slot without leaving, and the cache sees
    neither write. It now always re-reads, which is one gateway read and one
    worker exchange, the same as the equivalent key tool pays.
  - The deferred client Proxy answered every property with a function, which made
    it THENABLE. One `await` or `Promise.resolve` near it would call
    `then(resolve, reject)`, and the handler would resolve the real client, find
    no `then`, return undefined and never call either callback -- a permanent
    hang on a request path with nothing in a log. `then` is now absent.

MUTATION TESTING FOUND WHAT COVERAGE COULD NOT, on both files it ran over. The
new key session scored 68% on its first pass with fourteen survivors, and they
were not noise: nothing proved the cache was a cache, nothing proved a failed
resolution was retried rather than remembered as broken, nothing drove the
refusal path of a switch at all, and the account-switch test above turned out to
be passing through `select`, which always re-reads -- so it would have passed
against a cache keyed on the wrong thing. It also found that the `provider`
accessor, which hands the AAPI client to eight registered tools, was reachable
by no test: replacing it with one that yields undefined survived the whole
suite. Six tests later the file is at 95.45%, with two survivors that are
genuinely equivalent. Over toolsets.ts (97.17%) two more real gaps: listPhrase
was untested at three names, where slice(0, -1) and slice(0, 1) stop agreeing,
and the guarantee that a session carries `core` however it was built was never
exercised on a core-less input.

KNOWN, NOT FIXED, AND NOW STATED WHERE IT WAS PREVIOUSLY DENIED. Importing the
data plane here pulls gpt-tokenizer into the management binary at boot: measured
RSS 79 -> 189 MB and 1.04 s for src/mgmt/server.ts, the tokenizer being 65 MB and
386 ms of it, against a 512Mi pod. Two comments asserted the opposite ("the
management binary carries no tokenizer on purpose") and are corrected rather than
left to be believed. Deferring the import is not cheap -- `data` is in the
default and the group thunks run inside registerAsOneChange's synchronous window
-- so the two real options, a real token count in mgmt_list_toolsets and a lazy
tokenizer in torpc/tokens.ts, are recorded as open decisions.

Gates: typecheck, lint, format, 1662 tests, coverage (global 90/80/85 and
mgmt-scoped 80/75/80), build, mutation (toolsets.ts 97.17%, keySession.ts
95.45%).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…n the module does

SHARK-3629 made the management server import registerDataTools, which reaches
torpc/tokens.ts, which imported gpt-tokenizer at the top level. So every
management process paid 65 MB and 386 ms at boot for a tokenizer it might never
reach: `import src/mgmt/server.ts` went RSS 79 -> 189 MB and took 1.04 s, against
a 512Mi pod and a 5 s HEALTHCHECK, and a `?toolsets=core` session -- which has no
chain tool in it at all -- paid the same as one serving the whole data plane.

The load now happens on first use, and the warm-up moved to registerDataTools.
That is the honest place for it: it is the function that puts chain tools on a
server, so it is exactly the event after which a token count becomes reachable.
Measured per process, one scenario each:

  import src/mgmt/server.ts     118 MB (was 177)   not loaded
  ?toolsets=core session        104 MB (was 176)   not loaded
  default core+data session     176 MB             loaded
  /rpc createServer             170 MB             loaded, as before

A cold process is not at the 82 MB management-only baseline, and that is the
change's boundary rather than a shortfall: the AAPI client and the sixteen tool
modules are still statically imported. The tokenizer is the single largest piece
and the only one a chain-free session provably never needs.

createRequire RATHER THAN `await import()`. countTokensDetailed is called
synchronously from every tool's response path, so making the deferral async would
ripple through tokenMeta and all fourteen call sites for no behavioural gain. In
an ESM package createRequire is how a synchronous deferral is spelled. There is
deliberately NO fallback: if the module cannot be loaded this throws rather than
quietly reverting to chars/4, which is the 40-60% understatement SHARK-3525
removed and would be worse arriving silently.

AND THIS IS WHY mgmt_list_toolsets KEEPS ITS chars/4 ESTIMATE. The two halves are
one decision. mgmt_list_toolsets is in `core`, i.e. on every session including
the narrowest, so counting for real would pull those 65 MB straight back into
exactly the connections this relieved -- undoing the change through the one tool
that reports the numbers. Two comments claimed the binary "carries no tokenizer
on purpose", which SHARK-3629 had falsified and d860d75 corrected to say so; they
now state the posture that is actually true again.

Pinned in test/tokenizer-lazy.test.ts, one CHILD PROCESS per scenario. A module
registry is per process and write-once, so in-process the answer to "was it
loaded?" would depend on test order -- the shape of a test that passes for the
wrong reason. Both directions are asserted: the cold cases prove the deferral,
and the warm ones prove it is a deferral and not a removal. Verified by hand
mutation: deleting the warmTokenizer() call fails both warm tests.

Also closes a vacuous assertion the mutation run exposed in the pre-existing
>256 KB path. The extrapolation test asserted `meta.token_count === d.tokens`,
which compares the computation with itself, so replacing `(tokens / counted) *
text.length` with `/ text.length` or `tokens * counted` survived the whole suite.
A uniform payload of twice the limit must extrapolate to about twice the count of
one at the limit, which is a reference the function did not produce.

Gates: typecheck, lint, format, 1668 tests, coverage (global 90/80/85 and
mgmt-scoped 80/75/80), build, mutation (tokens.ts 82.35% before the added test;
remaining survivors are the Math.min cost bound, which only a timing assertion
could kill, and two equivalents).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Brings the observability work (#29), the live e2e suite (#32) and the docs
commits onto this branch. One real conflict, in src/server.ts, and it was
semantic rather than textual: SHARK-3607 added instrumentToolCalls, which must
patch registerTool BEFORE any tool registers, while SHARK-3629 moved the
registrations out of createServer into registerDataTools. Resolved by keeping
both and ordering them: instrument the server, then register.

The interaction leaves a gap neither side had on its own. instrumentToolCalls is
applied only in createServer, so the chain tools the MANAGEMENT plane now serves
are not counted by mcp_tool_calls_total. Recorded at the call site and in
DEPLOY-MGMT.md rather than fixed here.

Gates on the merged tree: typecheck, lint, format, 1742 tests, coverage (global
and mgmt-scoped), build.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…gement plane

instrumentToolCalls is applied only in createServer, so mcp_ankr_tool_calls_total
and mcp_ankr_tool_call_duration_seconds carry nothing from /mcp. That was
invisible while the planes served disjoint surfaces; SHARK-3629 put the sixteen
chain reads on both, so the same tool name is now counted from /rpc and not from
/mcp, and a per-tool rate read off those families understates real usage
silently.

Not fixed here on purpose: widening a metric's coverage changes what every
existing dashboard and alert on those names means, which belongs to whoever owns
them rather than to a merge.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

CI failed the tokenizer test at 149 MB against a 140 MB ceiling, for a cold
process that is 107-118 MB here, while the module registry correctly reported the
tokenizer absent. The threshold was measuring the runner, not the change.

The file's own comment already said RSS is noisy and the registry probe is exact,
and the ceiling was added anyway on the argument that RSS is the number the
ticket is about. It is, and that is an argument for reporting it, not for gating
on it: baseline RSS moves with the Node build, the GC and the transform cache,
and at 149 cold against 176 warm the bands overlap across environments, so no
portable ceiling separates them. Locally the same scenario read 118 MB and then
107 MB on consecutive runs.

The registry assertion, which is exact and environment independent, is unchanged
and is what proved the change works. Each scenario now prints its RSS instead, so
the figure stays visible without the gate depending on the machine.

Gates: typecheck, lint, format, 1742 tests, both coverage scripts including the
one CI failed on.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@mikhailak
mikhailak merged commit 9c79f20 into integ/mcp-prod-readiness Aug 9, 2026
2 checks passed
mikhailak added a commit that referenced this pull request Aug 9, 2026
Comments across src/ recorded what the code USED TO be before they said what
it is, so a reader met the archaeology first. This removes that layer without
losing what it carried.

The rule applied per comment: it stays if it explains something a competent
reader cannot get from the code (a constraint, a non-obvious "why", a trap, an
external contract, a security consequence); it goes if it only records that
things were once different, which is what git history and REVIEW-READY.md are
for. Where a history note carried a live lesson it is restated as a rule or a
counterfactual, so the warning survives without the narrative.

Notable, beyond the mechanical pass:
- groupScope.ts explained the same trap four times (absence from the route set
  is NOT an opt-out; `resolveGroup` still inherits the selection). Stated once
  in the header, back-referenced from the four sites.
- rpcCall.ts: the debug_ history becomes a better live warning, that the
  mutating word sits mid-camelCase so the verb rule cannot reach it.
- buildInfo.ts asserted the deployment runs image tag `latest`. It does not:
  both planes pin a full git sha (REVIEW-READY 4b). Removed rather than reworded.
- session-store.ts carried a comment about a REMOVED field, labelled as kept
  for history, ending in a truncated sentence fragment.

Comment markers in src/ comments: 164 -> 19, of which 11 are in files owned by
the open PR #36 and 8 are idioms ("can be used to", "the old one" meaning the
previous ticket). The 26 marker hits inside string literals are product text
under the SHARK-3599 token budget and were deliberately not touched.

No behaviour change, proven rather than asserted: for each of the 46 changed
files, comments were stripped from the committed and working versions via the
TypeScript AST printer and compared byte-for-byte. All 46 are identical.
Gate: typecheck, lint, format:check green; 1714 tests pass, 0 fail (unchanged).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mikhailak added a commit that referenced this pull request Aug 9, 2026
… files

Two groups.

FOUR STALE COMMENTS THE FIRST SWEEP MISSED, because none of them contains a
history marker word — they simply assert something the code stopped doing. Each
was confirmed by executing the code, not by reading it:

- oauth-provider.ts:704 said the nonce is "re-checked at /callback". Nothing
  reads `ankrState` on the callback path, and the block seventy lines below
  (added by the previous commit) says the check cannot fire and must not be
  re-added. The file contradicted itself on a CSRF control.
- oauth-provider.ts:1097 likewise said "only the nonce is trusted at /callback".
  What is trusted there is the one-time state key and the APPROVAL_COOKIE nonce.
- rpcCall.ts:286 said the txpool boundary is "those three NAMES". Executed:
  txpool_flushPending and txpool_anythingElse are FORWARDED. With the read
  allowlist gone there is no name boundary, only the write rules.
- rpcCall.ts:206 listed debug_chaindbCompact among methods the VERB rule
  refuses. Executed: hasMutatingVerb("debug_chaindbcompact") is false, because
  `_chaindbcompact` is not `_compact`. Only the debug_ namespace rule reaches it
  — which is what the header now says, so the file contradicted itself here too.

A RESTORED WARNING. The previous commit dropped the `argocd-mrpc` note from
DEPLOY-RUNBOOK.md as archaeology. That was wrong by this cleanup's own rule: the
name is still live in deploy/README.md and both ingress.yaml headers on the
`deploy/mcp-helm` branch, which is the branch the same table sends a reader to
for the chart, and the repository 404s. It is back as a table row.

The second group applies the same rule to the eleven files PR #36 owns, which
the first pass deliberately skipped to avoid conflicting with it. The
toolsets.ts deny-list-vs-allow-list reasoning is kept in full: the
`Set.prototype.forEach` third-argument hole is a live reason for the shape, not
a story about a previous one.

Archaeology markers in src/ comments: 164 -> 7, and all 7 are idioms ("can be
used to", "a ref that no longer resolves", "the old one" meaning the previous
ticket). The 26 hits inside string literals are product text and untouched.

Comments-only across all 52 changed files, proven by AST-printer comparison.
Gate: 1742 tests pass, 0 fail; typecheck, lint, format:check green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mikhailak added a commit that referenced this pull request Aug 9, 2026
… files

Two groups.

FOUR STALE COMMENTS THE FIRST SWEEP MISSED, because none of them contains a
history marker word: they simply assert something the code stopped doing. Each
was confirmed by executing the code, not by reading it:

- oauth-provider.ts:704 said the nonce is "re-checked at /callback". Nothing
  reads `ankrState` on the callback path, and the block seventy lines below
  (added by the previous commit) says the check cannot fire and must not be
  re-added. The file contradicted itself on a CSRF control.
- oauth-provider.ts:1097 likewise said "only the nonce is trusted at /callback".
  What is trusted there is the one-time state key and the APPROVAL_COOKIE nonce.
- rpcCall.ts:286 said the txpool boundary is "those three NAMES". Executed:
  txpool_flushPending and txpool_anythingElse are FORWARDED. With the read
  allowlist gone there is no name boundary, only the write rules.
- rpcCall.ts:206 listed debug_chaindbCompact among methods the VERB rule
  refuses. Executed: hasMutatingVerb("debug_chaindbcompact") is false, because
  `_chaindbcompact` is not `_compact`. Only the debug_ namespace rule reaches it
  which is what the header now says, so the file contradicted itself here too.

A RESTORED WARNING. The previous commit dropped the `argocd-mrpc` note from
DEPLOY-RUNBOOK.md as archaeology. That was wrong by this cleanup's own rule: the
name is still live in deploy/README.md and both ingress.yaml headers on the
`deploy/mcp-helm` branch, which is the branch the same table sends a reader to
for the chart, and the repository 404s. It is back as a table row.

The second group applies the same rule to the eleven files PR #36 owns, which
the first pass deliberately skipped to avoid conflicting with it. The
toolsets.ts deny-list-vs-allow-list reasoning is kept in full: the
`Set.prototype.forEach` third-argument hole is a live reason for the shape, not
a story about a previous one.

Archaeology markers in src/ comments: 164 -> 7, and all 7 are idioms ("can be
used to", "a ref that no longer resolves", "the old one" meaning the previous
ticket). The 26 hits inside string literals are product text and untouched.

Comments-only across all 52 changed files, proven by AST-printer comparison.
Gate: 1742 tests pass, 0 fail; typecheck, lint, format:check green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant