Agent RPC MCP: merge the data and management planes, and make the branch production-ready (SHARK-3524, SHARK-3373)

[mikhailak]

Merges the data plane (#25) and the management plane (#6) into one branch, plus the rounds of fixes that only became possible once the trees were together.

REVIEW-READY.md on this branch is the review document. It is written for someone who was in none of the sessions that produced this, and it states the decisions and their reasoning rather than assuming them. Section 4 is the one to read if you read one; section 4b is the one to read next.

This PR replaces #25 and #6, which are closed in its favour.

Why the two had to merge before either could ship

Not tidiness. Both PRs had rewritten the same security bootstrap in src/http.ts, independently, and each carried controls the other lacked:

• only on feat(mgmt): Management MCP control plane (SHARK-3373) — rebased onto main, audit fixes landed #6: the bounded session registry, the deployment posture resolved once at construction, the loopback-with-port origin carve-out, JSON-RPC error shapes for an over-limit body, and intEnv;
• only on Agent RPC MCP data plane: audit fixes (SHARK-3524/3525/3526/3527) #25: guardHotPath, installLastResortHandlers, the fail-closed AllowlistConfigError, the origin allowlist passed to the transport, and transport.close() on a failed initialize.

Shipping either alone would have shipped a regression against the other in the file that holds the public endpoint's security bootstrap.

Gates

Gate	Result
pnpm typecheck (src + tests)	clean
pnpm lint, pnpm format:check	clean
pnpm test	1583 pass, 0 fail
pnpm test:coverage (90/80/85)	98.69 lines, 88.61 branches, 95.11 functions
pnpm test:coverage:mgmt (80/75/80)	99.02, 88.97, 96.17
pnpm build	clean

Both coverage gates now run in CI. They did not before, so the thresholds constrained nothing and the numbers in the review notes were hand-measured.

Mutation is scoped per file by hand (coverageAnalysis is off, so every mutant costs a full suite run). src/bodyLimit.ts 94.44, src/buildInfo.ts 94.74, session-store 35.59 to 86.96, deleteApiKey 58.00 to 78.00.

What the review round closed

Nine findings survived adversarial verification, four high, all fixed. Each names its evidence in REVIEW-READY.md section 3. The four high ones:

1. rpcCall admitted 21 state-changing methods, two of which broadcast. bumpfee and psbtbumpfee cleared a substring allowlist on the fee token and were verified reaching upstream against a running data plane.
2. One unauthenticated request could fan out to thousands of upstream calls. Measured: 2000 tools/call entries in one 327 KB body, answered in 0.53 s. Now capped at 20 messages per request.
3. Unauthenticated DCR registrations evicted other people's OAuth clients. Reproduced end to end; the victim's next /authorize answered 400 invalid_client.
4. buildProvider's wiring to guardProvider was untested: removing the guard left 1521 of 1521 tests passing while putting an incident string back into agent-visible text and removing the only AAPI deadline.

The largest behaviour change

rpcCall stopped deciding which reads exist (section 4.5). It was a default-deny read allowlist in front of the write rules; it is now a write denylist only, and anything it does not refuse is forwarded. The read half failed in both directions at once, and which reads exist is decided by the chain's schema and the caller's tenant, both current by construction. 109 string literals removed.

The risk, stated plainly: a method no write rule recognises now reaches the endpoint. Shark's tx_broadcasting profile forwards broadcasts, so for the WRITE class this guard remains the only chokepoint, and nothing here touches that half.

What production actually runs (section 4b)

Read from ArgoCD on 2026-08-06, and it corrects two claims these notes had made. Production is Istio (Gateway plus VirtualService) via ArgoCD out of argocd-mrpc, with an ExternalSecret and ECR images. The deploy/*.yaml in this repo are ingress-nginx and are not applied anywhere, so their limit-rps annotations are not in force: there is no edge rate limiting on either plane and no CDN. DEPLOY-RUNBOOK.md is new and is the single current deploy document.

Also: nothing identified the running build. serverInfo.version was a constant that differed between the two planes and from package.json, and both deployments run latest with IfNotPresent. It is now <version>+<commit> from a BUILD_COMMIT build arg (SHARK-3606).

Recorded decisions rather than open findings

• Payment initiators carry no second factor, deliberately (section 4.9). They create a Stripe hosted Checkout session and return a URL; no money moves until a human completes Stripe's own form, and both sit behind a HITL gate that needs a fresh interactive login. A TOTP would gate the making of a URL, lock out every account without 2FA enrolled, and put a live second factor in an agent transcript. SHARK-3392's other half, the /confirm self-approval path, shipped.
• Single replica with in-memory state is an accepted allowance (section 4.2), with its costs enumerated and a stated expiry: externalise before GA, or before a deploy-time drop is visible to a customer.

Known gaps, tracked

Gap	Ticket
No metrics, no structured logs	SHARK-3607
No dashboard, no alerts, no status test	SHARK-3608
Immutable image reference instead of latest	SHARK-3606 (our half done)
Control-plane limiter unexplained in prod	SHARK-3592
Gateway 504 on transactionHistory, breaks the invoice chain	SHARK-3593
DCR registry does not survive a redeploy	SHARK-3547
End-to-end run against a deployed build of THIS branch	SHARK-3460, SHARK-3461

Two checks need one real interactive login and are listed in SHARK-3461: whether UAuth echoes ankrState to /callback (then MGMT_REQUIRE_ANKR_NONCE becomes mandatory), and whether the login token and the exchanged session token carry the same unique_id. If they differ, every HITL-gated write is permanently unapprovable, silently.

Reproducing locally

pnpm install --frozen-lockfile
pnpm typecheck && pnpm lint && pnpm format:check && pnpm test
pnpm test:coverage && pnpm test:coverage:mgmt && pnpm build
pnpm mutation:file 'src/bodyLimit.ts'   # ONE path per invocation

Never raise Stryker's concurrency above the pinned 2 and never run it unscoped: an unbounded run put a 20-core machine at load average 252.

[mikhailak]

Cleanup pass before review: 4 commits, no behaviour change

These four commits change comments and documentation only. They exist because the
code and docs told you what they USED to be before they told you what they are,
so a reader met the archaeology first.

The rule applied per comment. It stays if it explains something you 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.

Archaeology markers in src/ comments: 170 down to 7, and all 7 are idioms
("can be used to", "a ref that no longer resolves"). The 26 marker hits inside
string literals are untouched: those are tool descriptions and error messages under
the SHARK-3599 token budget, i.e. product, not comments.

Proof that behaviour did not move, since "comments only" is easy to claim:

• For each of the 52 changed .ts files, comments were stripped from both
  revisions with the TypeScript AST printer and compared byte for byte. All 52 are
  identical. The printer, not transpileModule: the latter erases type-only
  imports, which would make a type edit invisible to the check.
• Full gate green: 1748 tests, 0 failures, plus typecheck, lint, format, build and
  pnpm audit --prod.
• Live e2e against production: 26/26, 0 tool contract differences across all 16
  tools, identical session instructions, and the auth refusal byte identical at 296
  characters. That is the run that would catch a cleanup clipping tools/list text.

Four stale claims found and fixed, none of which contained a history marker
word, so they were caught by review rather than by the sweep. Each was confirmed by
executing the code:

• oauth-provider.ts said the nonce is "re-checked at /callback". Nothing reads
  ankrState there, and the block below says the check cannot fire and must not be
  re-added. The file contradicted itself on a CSRF control.
• rpcCall.ts 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 listed debug_chaindbCompact among methods the verb rule refuses.
  Executed: it does not, because _chaindbcompact is not _compact. Only the
  debug_ namespace rule reaches it.
• buildInfo.ts asserted the deployment runs image tag latest. Both planes pin a
  git sha (4b).

Docs. DEPLOY.md is folded into DEPLOY-RUNBOOK.md and deleted, so deployment
is described once. DEPLOY-MGMT.md told whoever provisions the deploy to generate
a fresh signing key, and twenty lines later said not to, about a key whose
regeneration invalidates every live session; that is now one instruction. Stale
facts corrected against 4b: one ArgoCD application rather than two, one source of
truth path, one VirtualService, and /healthz answering 200.

I did not create a docs/ directory, though the brief suggested it as an option.
test/doc-gates.test.ts reads README.md, DEPLOY-MGMT.md and USER-STORIES.md
by name from the repo root and asserts their claims against the code, and those two
docs are referenced from 8 source files and 9 test files. Moving them means
rewiring a doc honesty gate and 17 references on a branch already in production, to
gain a directory. Happy to do it as a follow-up with the gate updated in the same
change if you would rather have it.

Two things worth your judgement:

1. The comment ratio barely moved: 40 files above 40% before and after,
   groupScope.ts 82% to 82%. The archaeology was spread thin across many files
   rather than concentrated in the dense ones, and those dense files are evidence
   bases (per route citations into the gateway source) where deleting text would
   delete knowledge. I did not chase the ratio.
2. I removed the argocd-mrpc note as archaeology and that was wrong: the name is
   still live in three files on deploy/mcp-helm, the branch the same table sends
   you to for the chart, and the repository 404s. It is restored as a table row.