diff --git a/REVIEW-READY.md b/REVIEW-READY.md index 198ff18..6fd7127 100644 --- a/REVIEW-READY.md +++ b/REVIEW-READY.md @@ -5,10 +5,23 @@ this branch, so it states the decisions and their reasoning rather than assuming them. Section 4 is the one to read if you only read one: it holds the tradeoffs and the -limits, including three that are real in production today and are not this +limits, including ones that are real in production today and are not this branch's to fix. Section 4b is the one to read next: it records what the cluster -actually runs, which is not what the manifests in this repository describe, and -it corrects two claims made elsewhere in these notes. +actually runs, and corrects claims made elsewhere in these notes. + +**Read these two subsections before disagreeing with anything, because both are +reversals that were argued out already and the reasoning is not obvious from the +diff:** 4.5, where `rpcCall`'s guard stopped being a read allowlist and became a +write denylist, and 4.1, where a blank allowlist stopped falling back to the +built-in default and started refusing to serve. 4.1 reverses a decision taken +INSIDE this merge, not just an older one. + +**Status as of 2026-08-07.** This branch is in production. Both planes were +rolled to builds of it (data `e9a0b57`, mgmt `9176d12`, chart `0.4.0-rc.1`) via +`infrastructure-k8s` PR #2093, merged 09:34Z, ArgoCD synced 09:36Z, both +applications Synced and Healthy. A live end-to-end suite (PR #32) runs **26/26** +against that deployment. Six of the seven questions section 6 used to hold are +answered; section 6 now carries two. --- @@ -77,15 +90,21 @@ The two branch-coverage figures move by a few hundredths between runs (timing dependent branches: the session sweeper's interval, the child-process polls), so read them as the measurement they are rather than as constants. -| Gate | Command | Result | -| ---------------- | ------------------------------------------------------------------ | --------------------------------------------------- | -| Types | `pnpm typecheck` (`tsc --noEmit` plus `tsc -p tsconfig.test.json`) | clean | -| Lint | `pnpm lint` | clean | -| Format | `pnpm format:check` | clean | -| Tests | `pnpm test` | **1559 pass, 0 fail** (1545 after the review round) | -| Coverage, global | `pnpm test:coverage` (thresholds 90 / 80 / 85) | **98.63 lines, 88.38 branches, 95.10 functions** | -| Coverage, mgmt | `pnpm test:coverage:mgmt` (thresholds 80 / 75 / 80) | **99.01 lines, 88.71 branches, 96.17 functions** | -| Build | `pnpm build` | clean | +| Gate | Command | Result | +| ---------------- | ------------------------------------------------------------------ | --------------------------------------------------------------------------------------------- | +| Types | `pnpm typecheck` (`tsc --noEmit` plus `tsc -p tsconfig.test.json`) | clean | +| Lint | `pnpm lint` | clean | +| Format | `pnpm format:check` | clean | +| Tests | `pnpm test` | **1627 pass, 0 fail** (1559 when this section was first written, 1545 after the review round) | +| Coverage, global | `pnpm test:coverage` (thresholds 90 / 80 / 85) | **98.63 lines, 88.38 branches, 95.10 functions** | +| Coverage, mgmt | `pnpm test:coverage:mgmt` (thresholds 80 / 75 / 80) | **99.01 lines, 88.71 branches, 96.17 functions** | +| Build | `pnpm build` | clean | +| Advisories | `pnpm audit --prod` | no known vulnerabilities | +| Live e2e | `pnpm test:e2e` (PR #32, outside CI on purpose) | **26 pass, 0 fail** against the deployment in 4b | + +The live e2e row is the only one in this table that can go red without any code +here being wrong. It talks to `mcp.ankr.com`, so a red parity test means the +deployment is behind and the remedy is to deploy, not to edit. Mutation testing is scoped per file (`pnpm mutation:file ''`), because `coverageAnalysis` is off in this repo so every mutant costs a full suite run. @@ -486,33 +505,43 @@ Finding 3 above is closely related but is not the same thing: it removed the pat by which a stranger could inflict that failure on you at will. The redeploy path is untouched and needs the same shared store as 4.2. -### 4.8 SHARK-3592: the control-plane limiter's production behaviour is unexplained +### 4.8 SHARK-3592: the control-plane limiter DOES limit, and the ticket is closed -The management plane has an in-app per-IP token bucket on its control-plane -routes. Its behaviour in production has not been explained, and the reproduction -against the deployed build has not been done. +**Rewritten 2026-08-07. The heading used to read "the control-plane limiter's +production behaviour is unexplained", and the observability shipped in SHARK-3607 +explained it.** The limiter refuses in production, and the counter that says so +is now a metric anyone can read: -One thing is settled and worth recording so nobody repeats it: **the +``` +mcp_ankr_refusals_total{job="agent-rpc-mgmt-mcp", reason="bucket_empty"} +``` + +It stood at 150 on 2026-08-07. SHARK-3592 is closed in Jira. What made the +original observation ("250 requests in 2s, zero 429") look like a broken limiter +was that nothing in the process could be asked whether the bucket had ever +emptied, so the only evidence was the absence of a status code at the client. + +The competing explanation is also ruled out now. It required the requests to have +been spread over several independent buckets, which required several replicas. +There is exactly one pod per plane: `up{job="agent-rpc-mgmt-mcp"}` returns one +series, and so does the data plane's. One process, one bucket. + +One thing stays settled and is worth keeping so nobody repeats it: **the `TRUST_PROXY_HOPS` fix applied on 2026-08-04 was a no-op.** The code already defaults to 1 (`app.set("trust proxy", intEnv(process.env.TRUST_PROXY_HOPS, 1))` -on both planes), so setting the variable to 1 changed nothing, and whatever was -observed in production has another cause. The two open questions in section 6 are -the ones that would narrow it. - -`deploy/mgmt/ingress.yaml` also carries no nginx `limit-rps` or -`limit-connections` annotations, unlike the data plane's, so on the management -plane the in-app bucket is the only limiter. - -**Correction (2026-08-06): that comparison is void, and the conclusion it drew is -now true of BOTH planes.** Neither Ingress in this repository is applied to the -cluster; production routes through Istio, which does not read ingress-nginx -annotations (section 4b). So the data plane's `limit-rps: 20` is not in force -either, and the in-app bucket on the management plane and the batch cap on the -data plane are the only limits that exist anywhere in front of either service. -There is also no CDN or DDoS layer in front of `mcp.ankr.com`. This does not -explain SHARK-3592 by itself, since the in-app bucket is in the process and does -not depend on the ingress, but it does mean the question "what bounds this -endpoint" currently has the answer "one in-process bucket, and nothing else". +on both planes), so setting the variable to 1 changed nothing. The per-cluster +values in `infrastructure-k8s` now set it explicitly to `"1"` anyway, which +documents the intent even though it does not change behaviour. + +What is NOT closed is the layer question, and it is a decision rather than a +finding. Neither Ingress in this repository is applied to the cluster; +production routes through Istio, which does not read ingress-nginx annotations +(section 4b). So the data plane's `limit-rps: 20` is not in force either, and +there is no CDN or DDoS layer in front of `mcp.ankr.com`. **The in-app bucket on +the management plane and the 20-message batch cap on the data plane are the only +bounds that exist anywhere in front of either service.** Whether that is +acceptable, and whether the answer should be an Istio local rate limit rather +than more app code, is item 1 of section 6. ### 4.9 The payment initiators carry no second factor, and that is the decision @@ -562,11 +591,20 @@ gateway-verified code when the account has one enrolled. --- -## 4b. What production actually runs (read on 2026-08-06) +## 4b. What production actually runs (read 2026-08-06, re-read 2026-08-07) `deploy/` and the two Helm charts on the `deploy/*` branches describe three different deployments, and production is none of them. Read this before believing -any deployment claim in this repository, including two these notes made. +any deployment claim in this repository, including several these notes made. + +**What changed on 2026-08-07, and it changes the shape of this section.** The +routing, the TLS certificate and the signing secret are no longer undocumented: +they are committed YAML in `w3tech/infrastructure-k8s`, and they are quoted below +rather than described. So the old summary "production matches no manifest +anywhere" is now half wrong. It matches manifests, just not the ones in THIS +repository. What survives, and is stated precisely at the end of this section, is +that this repository still ships three artifacts that describe deployments nobody +runs, and two of them carry values that would be wrong if anyone ever ran them. **What is deployed.** Two ArgoCD applications in project `aapi-production`, both Synced and Healthy: @@ -590,6 +628,40 @@ The source of truth is **`w3tech/infrastructure-k8s`**, at `common/common.values.yaml` plus a per-cluster directory (`do-fra1-03`). This repository does not reference it once. +**The routing, quoted rather than described (read 2026-08-07).** Both files live +under those paths, in `do-fra1-03/certs/istio.yaml`: + +- One **Gateway**, `aapi-mcp-server-gateway`, owned by the data-plane app: HTTPS + on 443 for host `mcp.ankr.com`, TLS mode SIMPLE, credential `mcp-ankr-com-tls`, + which a cert-manager `Certificate` in `istio-ingress` issues off the Route53 + cluster issuer. The management app deliberately defines neither, and says so in + a comment. +- **Two VirtualServices on that one Gateway and one host.** `aapi-mcp-server` + matches `uri.prefix: /rpc` and routes to `agent-rpc-mcp:3000`. + `aapi-mgmt-mcp-server` has NO match block at all and routes everything to + `agent-rpc-mgmt-mcp:3100`. There is no rewrite on either, which is why the app + serves `/rpc` unmodified. + +**A risk that follows from that shape, and that nobody has confirmed either way.** +Two VirtualServices binding the same host and gateway are merged by Istio, and +the order of routes contributed by separate resources is not something either +file states. Today the specific `/rpc` prefix wins, which is why the data plane +answers at all. If the merge order were ever to put the catch-all first, every +`/rpc` request would land on the management plane and answer 401, and nothing in +either file would look wrong. **The cheap fix is to stop relying on the answer: +either express both routes in ONE VirtualService, where the order is the order +they are written in, or give the management route an explicit match set instead +of making it a catch-all.** Until then this is a live single point of failure with +no test behind it, and it is worth an operator's opinion during review. + +**The signing key is fixed, and the manifest says how.** `do-fra1-03/secrets/external-secret.yaml` +reads property `gateway-jwt-private-key` from ClusterSecretStore +`vault-k8s-kv-store` at `aapi/mgmt-mcp-server`, `refreshInterval: 1h`. It READS a +stored value; it does not generate one. So the key survives a deploy and a +resync, and the failure mode section 6 used to worry about (every live session +dying at once on a rotation that looks like an auth bug) does not arise unless +the Vault value itself is changed. + **CORRECTION, same day, second pass. An earlier version of this section named `argocd-mrpc`. There is no such repository** (the GitHub API answers 404). That name came from comments inside the Helm charts on the `deploy/*-helm` branches @@ -603,22 +675,28 @@ favour rather than against us:** `image.tag` to a full git sha on both planes, overriding the chart default, with a comment saying the chart's `latest` "is a placeholder, not something to run in production as-is". So a rollback has a target and a rollout is - verifiable. What is still missing is on OUR side: the build does not pass - `--build-arg BUILD_COMMIT`, so the served version is a bare `0.2.0` and the sha - lives only in the registry tag. + verifiable. - **The 128Mi memory request was already corrected.** `K8S-1107` on 2026-08-06 bumped the data plane to 256Mi in the deploy values, citing the tokenizer measurement. -**And this branch is already in production.** On 2026-08-06 at 12:37Z, K8S-1107 -pinned both planes to builds of this branch; ArgoCD synced the data plane at -12:43:56Z and the management plane at 12:46:35Z, both Healthy. The data-plane -image (`8c53c58e`) contains every commit on this branch including the review -fixes; the management-plane image (`883fac3d`) was cut from `f71f30b` and does -NOT, so the two planes are currently running from different source states. That -is benign today, because nothing in the newer commits changes shared runtime -behaviour, but it is the drift the runbook exists to prevent and the next roll -should bring them back together. +**And this branch is already in production.** First on 2026-08-06 via K8S-1107, +then rolled forward on 2026-08-07 by `infrastructure-k8s` PR #2093 (merged +09:34Z, ArgoCD synced 09:36Z, both applications Synced and Healthy): + +| Plane | Image tag = commit | Branch | Chart | +| ----- | ------------------ | -------------------------------- | ------------ | +| data | `e9a0b572…` | `deploy/aapi-mcp-server-helm-rc` | `0.4.0-rc.1` | +| mgmt | `9176d12c…` | `deploy/mgmt-mcp-helm-rc` | `0.4.0-rc.1` | + +**The two planes are back on one source state.** `src/` is byte-identical +between the two RC branches; they differ only in `charts/`. The 08-06 drift, +where the management image was cut from `f71f30b` and lacked the review fixes, is +closed. Note that production is pinned to a chart version that says +`-rc.1`, which is a deliberate pre-review state and not an accident, but it does +mean a release decision is pending: cut `0.4.0` when #30 and #31 merge and re-pin +`helmChartVersion` in `infrastructure-k8s`, or record that an rc chart is what +production runs. The practical consequence for a reviewer: this PR is being reviewed AFTER its contents reached production, so `main` is behind what is serving. @@ -628,61 +706,72 @@ Ingresses, marked DRAFT for PlatEng. `charts/aapi-mcp-server` (branch `deploy/aapi-mcp-server-helm`) and `charts/agent-rpc-mgmt-mcp` (branch `deploy/mgmt-mcp-helm`) are Traefik, and neither chart is on this branch at all. -**Live behaviour, measured against `mcp.ankr.com` on 2026-08-06:** - -| Request | Result | -| --------------- | ---------------------------------------- | -| `POST /rpc` | 200, the data plane answers `initialize` | -| `POST /rpc/mcp` | 404 | -| `POST /mcp` | 401, the management plane's OAuth gate | -| `GET /healthz` | 404 | - -Three consequences, each a thing to fix rather than a thing to note: - -1. **The data-plane chart would break the public URL.** It sets - `pathPrefix: /rpc` with a stripPrefix middleware, so it expects callers at - `mcp.ankr.com/rpc/mcp`. Live, that path is 404 and `/rpc` is the one that - answers. Applying that chart as written moves every existing client onto a path - that does not exist. -2. **The data-plane chart requests the wrong memory.** It asks 128Mi. - `deploy/deployment.yaml` raised the request to 256Mi because the o200k - tokenizer measures 111 MB steady and 146 MB peak. The artifact that would - actually deploy carries the number that was measured to be wrong. -3. **No edge limiting is in force on either plane.** See the corrections in - finding 2 and in 4.8. - -**Build identity: half solved, and the missing half is ours.** The REGISTRY side -is fine: the deploy values pin a full git sha per plane, so the image is -identifiable and a rollback has a target. The WIRE side is not: `serverInfo.version` -was the constant `"0.2.0"` in `src/server.ts` and is now `buildVersion()`, but the -build does not pass `--build-arg BUILD_COMMIT`, so it still answers a bare -`0.2.0`. Confirmed live on 2026-08-06 against a deployed build that already -contains `src/buildInfo.ts`. - -So the remaining work is one line in the build workflow on the `deploy/*-helm` -branches, not a change to how the deployment references images. That is item 2 in -section 6 and it is smaller than it was first written. +**Live behaviour, measured against `mcp.ankr.com`:** + +| Request | 2026-08-06 | 2026-08-07 | +| --------------- | ---------------------------------------- | ------------------------ | +| `POST /rpc` | 200, the data plane answers `initialize` | unchanged | +| `POST /rpc/mcp` | 404 | unchanged | +| `POST /mcp` | 401, the management plane's OAuth gate | unchanged | +| `GET /healthz` | 404 | **200** | +| `GET /readyz` | not present | **200** | +| `GET /metrics` | not present | 404, and that is correct | + +`/healthz` answering 404 was the readiness gap SHARK-3607 closed; both probes now +answer. `/metrics` is served on its own listener on port 9464 and is deliberately +NOT routed by the VirtualService, so a 404 from the public host is the intended +result rather than a missing endpoint: the scrape reaches it inside the cluster +through a `VMServiceScrape`, and `up` is 1 for both planes. + +**The three consequences this section used to list, re-checked on 2026-08-07:** + +1. **The data-plane chart still carries the routing that would break the public + URL, and it is disarmed rather than fixed.** The chart sets + `ingress.enabled: true`, `className: traefik`, `pathPrefix: /rpc` and a + stripPrefix middleware. Production is unaffected only because the per-cluster + values in `infrastructure-k8s` set `ingress.enabled: false` on both planes and + route through Istio instead. So the mine is armed and stepped around, not + removed: a cluster added without that per-cluster override would get a Traefik + Ingress that strips `/rpc` before the app sees it, and the app serves `/mcp` + and `/rpc`, so the stripped path would 404. +2. **The data-plane chart still requests 128Mi**, and the 256Mi that the + tokenizer measurement justified lives only in the per-cluster values. Same + shape as the point above: correct in production, wrong in the artifact. +3. **No edge limiting is in force on either plane.** Unchanged. See 4.8. + +**Build identity: now solved on both halves.** The registry side was already +fine. The wire side shipped in the RC: the image build passes +`--build-arg BUILD_COMMIT`, and `initialize` answers +`0.2.0+e9a0b572d3715155dcca8805008391489bec68bf` on the data plane and +`0.2.0+9176d12cc62ea48e0a83fe037239ab6bb6f59e38` on the management plane. The +same fact is on the metric, `mcp_ankr_build_info{commit=…,version=…}`, so "which +build is running" is answerable from a dashboard and from an MCP `initialize` +without cluster access. Verified live 2026-08-07 by the e2e suite (PR #32), which +now also FAILS if the suffix ever disappears. --- ## 5. Deliberately not in this branch -| Not here | Ticket | -| --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------- | -| `.strict()` on the management plane's tool schemas, and the `blockchain` to `chain` alias on `getTokenPrice`. Smaller than the ticket says: PR #25 already made all 16 data-plane tools strict, so what remains is the management surface plus the alias | SHARK-3596 | -| Mutation gate on `session-store` and `deleteApiKey`. A run against `src/mgmt/auth/session-store.ts` (91 mutants) was started and stopped at 7 of 91, roughly 90 minutes short: at the pinned concurrency of 2 with `coverageAnalysis` off, each mutant costs a full suite run. Its early numbers are not quoted anywhere here, because 6 of those 7 were timeouts recorded while another suite was running on the same machine, which makes them a measurement of the load and not of the tests. The hand-mutation evidence in finding 3 stands in for it: restoring the FIFO eviction turns 4 of the 5 new tests red | SHARK-3588 | -| Per-route limits on the control plane, multi-replica safety, and the reproduction against the deployed build | SHARK-3592 | -| An end-to-end run of the user-story suite against a deployed build of THIS branch. Everything above was verified locally and, for finding 1 and finding 2, against a locally running data plane | (part of the release checklist, not a code ticket) | -| Anything that makes either plane safe to scale past one replica | see 4.2 | +| Not here | Ticket | +| --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------- | +| ~~`.strict()` on the management plane's tool schemas, and the `blockchain` to `chain` alias on `getTokenPrice`~~ **LANDED, this row is stale.** The management tool schemas carry `.strict()`, and `getTokenPrice` takes `chain` with `blockchain` kept as a deprecated alias that is documented as such and refused when both are given at once, so a caller cannot name the chain twice and get an answer about one of them | SHARK-3596, in review | +| Mutation gate on `session-store` and `deleteApiKey`. A run against `src/mgmt/auth/session-store.ts` (91 mutants) was started and stopped at 7 of 91, roughly 90 minutes short: at the pinned concurrency of 2 with `coverageAnalysis` off, each mutant costs a full suite run. Its early numbers are not quoted anywhere here, because 6 of those 7 were timeouts recorded while another suite was running on the same machine, which makes them a measurement of the load and not of the tests. The hand-mutation evidence in finding 3 stands in for it: restoring the FIFO eviction turns 4 of the 5 new tests red | SHARK-3588 | +| Per-route limits on the control plane, and multi-replica safety. The third item this row used to carry, the reproduction against the deployed build, is done: the limiter refuses in production and the refusal is on a metric (4.8), and SHARK-3592 is closed | SHARK-3592, closed | +| ~~An end-to-end run against a deployed build of THIS branch~~ **LANDED as PR #32.** `pnpm test:e2e` runs against a live target and scores 26/26 against the deployment described in 4b. It is outside CI on purpose: it needs a credential and costs real requests | (release checklist, not a code ticket) | +| Anything that makes either plane safe to scale past one replica | see 4.2 | --- ## 6. What is needed from Aleksandr Balev -**Rewritten on 2026-08-06, after he had already done half of it.** K8S-1107 that -day pinned both planes to builds of this branch, bumped the data plane to 256Mi, -and synced both applications. Four of the seven items this section used to carry -are therefore closed, and two of them were closed by facts rather than by work: +**Rewritten again on 2026-08-07. This section held seven items on 06 August and +holds two now.** Some were closed by his work, some by the observability that +shipped in SHARK-3607, and some turned out to be answerable by reading a file +rather than by asking anyone, which is the least flattering way for a question to +close and worth recording as such. + +Closed, with what closed it: - ~~pin the deploy to an immutable image~~ ALREADY TRUE. `common.values.yaml` pins a full git sha per plane and its own comment says the chart's `latest` is @@ -690,56 +779,58 @@ are therefore closed, and two of them were closed by facts rather than by work: - ~~correct the 128Mi memory request~~ DONE in K8S-1107. - ~~tell us the deployment path~~ FOUND: `w3tech/infrastructure-k8s`, `argocd/apps/aapi/resources/{aapi-mcp-server,aapi-mgmt-mcp-server}/`. -- ~~read the pod `imageID`~~ MOSTLY MOOT. With a unique sha tag, +- ~~read the pod `imageID`~~ MOOT. With a unique sha tag, `imagePullPolicy: IfNotPresent` cannot serve a stale image, which is what that reading existed to rule out. - -What is still open: - -```sh -kubectl -n agent-rpc-mcp get deploy agent-rpc-mgmt-mcp \ - -o jsonpath='{.spec.replicas}{" "}{.status.readyReplicas}{"\n"}' - -kubectl -n agent-rpc-mcp get destinationrule -o yaml | grep -A5 consistentHash -``` - -1. **Replica count on the management plane.** Outstanding since 3 August and - still the main one: every store on that plane is per process, and the - SHARK-3592 diagnosis turns on this number. A bucket of 60 gives zero 429s over - 250 requests only if those requests were spread over at least five independent - buckets. -2. **Pass `--build-arg BUILD_COMMIT` in the image build.** One line in - `build-and-push.yml` on the `deploy/*-helm` branches. The app half shipped: the - deployed build already carries `src/buildInfo.ts`, and without the build arg it - answers a bare `0.2.0`, so the sha exists only in the registry tag and not on - the wire. With it, `initialize` answers `0.2.0+` and "which build is - running" becomes a question anyone can answer without cluster access. -3. **`consistentHash` in the DestinationRules.** If sessions are sticky by - cookie, the browser login, callback and approve flow pins to one pod while a - `curl` without a cookie spreads across all of them, which would invalidate the - reasoning "approve succeeded first try, therefore there is one pod". -4. **The Gateway and VirtualService as applied.** We know where they live now but - not what they say. Two properties specifically: the route timeout, because - `GET /rpc` and `GET /mcp` are long-lived SSE streams and a default Istio - timeout would cut them mid-stream, with an agent going quiet rather than - erroring; and how `X-Forwarded-For` reaches the pod, because both planes run - `trust proxy` with a hop count of 1 and every per-IP bound is only as correct - as that number. -5. **Edge rate limiting: does any exist, and where should it live.** Neither - nginx Ingress in this repository is applied, so no `limit-rps` or - `limit-connections` is in force, and there is no CDN in front of - `mcp.ankr.com`. The in-app bucket on the management plane and the 20-message - batch cap on the data plane are the only bounds anywhere. If the answer is an - Istio local rate limit, we would rather have it there than grow app code that - duplicates it. -6. **Confirm the mgmt `ExternalSecret` holds a FIXED signing key.** - `gateway-jwt-private-key` mints the shim's own bearers. If it is regenerated - on a deploy or a resync, every live session dies at once and the symptom looks - like an auth bug rather than a rotation. -7. **Bring the two planes back to one commit.** The data-plane image contains - this whole branch; the management-plane image was cut from `f71f30b` and does - not. Benign today, because nothing in the newer commits changes shared runtime - behaviour, but the two are meant to roll together and currently do not. +- ~~pass `--build-arg BUILD_COMMIT`~~ SHIPPED in the RC. Both planes answer + `0.2.0+` on `initialize` and carry the same fact on + `mcp_ankr_build_info`. See 4b. +- ~~replica count on the management plane~~ ANSWERED, and not by kubectl: + `up{job="agent-rpc-mgmt-mcp"}` returns exactly one series, and so does the data + plane's. One pod each. This is the number the SHARK-3592 diagnosis turned on, + and it removes the "spread over five buckets" explanation for good (4.8). +- ~~`consistentHash` in the DestinationRules~~ ANSWERED. Neither ArgoCD + application manages a DestinationRule at all, and none exists in the repository + paths above, so there is no consistent-hash stickiness to reason about. With one + pod per plane the question is moot in both directions. +- ~~confirm the mgmt `ExternalSecret` holds a FIXED signing key~~ ANSWERED by + reading it. It reads property `gateway-jwt-private-key` from ClusterSecretStore + `vault-k8s-kv-store` at `aapi/mgmt-mcp-server` on a 1h refresh. It reads a + stored value rather than generating one, so the key survives deploys and + resyncs (4b). +- ~~bring the two planes back to one commit~~ DONE. `src/` is byte-identical + between the two RC branches, and both images were rolled together in + `infrastructure-k8s` PR #2093. +- ~~the Gateway and VirtualService as applied~~ HALF ANSWERED, and the half that + is answered is now quoted in 4b rather than described. `X-Forwarded-For` is + settled enough to stop asking: the per-cluster values set `TRUST_PROXY_HOPS: "1"` + explicitly, which matches the code default. The route-timeout half is item 2 + below, because it is still genuinely unknown. + +What is still open, and both are decisions rather than lookups: + +1. **Edge rate limiting: should any exist, and where should it live.** Nothing + bounds either plane at the edge. No nginx Ingress in this repository is + applied, there is no CDN in front of `mcp.ankr.com`, and the only bounds + anywhere are the in-process bucket on the management plane and the 20-message + batch cap on the data plane. Both are per process, which is exactly enough for + one replica and stops being enough the moment 4.2's allowance expires. If the + answer is an Istio local rate limit, we would rather have it there than grow + app code that duplicates it. +2. **The route timeout on long-lived streams.** Neither VirtualService sets + `timeout`, so whatever the mesh defaults to is what applies, and nobody has + read it. `GET /rpc` and `GET /mcp` are SSE streams that can sit idle between + messages; a stream cut by a default timeout looks to an agent like the server + going quiet rather than like an error, which is the worst shape a failure can + take here. This is cheap to settle either by reading the mesh config or by + holding a stream open past the suspected boundary and watching, and it should + be settled before this is announced to anyone. + +One more thing to put in front of an operator, new on 2026-08-07 and not +previously on this list: **two VirtualServices share one host and one gateway, +and the management one is a catch-all.** The data plane answers only because the +specific `/rpc` prefix is evaluated first. See 4b for why that is worth removing +rather than relying on. ## 7. Reproducing any of this locally @@ -750,6 +841,10 @@ pnpm test:coverage pnpm test:coverage:mgmt pnpm build pnpm mutation:file 'src/bodyLimit.ts' # ONE path per invocation + +# And, against the deployment rather than this checkout (PR #32): +ANKR_RPC_KEY= pnpm test:e2e +E2E_EXPECT_COMMIT= ANKR_RPC_KEY= pnpm test:e2e # pin what should be serving ``` Two traps worth knowing before you spend time on them, both paid for already: diff --git a/USER-STORIES.md b/USER-STORIES.md index b78e03b..01cc187 100644 --- a/USER-STORIES.md +++ b/USER-STORIES.md @@ -35,6 +35,21 @@ Status legend: **DONE** verified by test or live run · **PARTIAL** works with a stated limit · **GAP** not implemented · **N/A** cannot exist here, with the reason. +**What a status here does and does not claim (added 2026-08-07).** DONE means the +capability is verified against THIS code. It is not a claim that the capability +works for every account in production today: a row can be DONE and still fail +upstream, and row 4.5 is exactly that case. Where the two diverge the row says so +in its own cell rather than leaving a reader to discover it. Two facts to hold +while reading, both from `REVIEW-READY.md`: + +- **This branch is in production.** Both planes serve builds of it (data + `e9a0b57`, mgmt `9176d12`, chart `0.4.0-rc.1`), so these rows describe what is + serving, not what is proposed. A live suite scores 26/26 against it. +- **Every store is per process and there is one pod per plane.** A deploy drops + live MCP sessions, pending confirmations and registered OAuth clients. That is + a stated allowance, not a defect, and it is the reason rows 6.4 and 7.5 read + the way they do. See `REVIEW-READY.md` 4.2. + --- ## 1. Keys and projects @@ -76,14 +91,14 @@ reason. ## 4. Balance and payments -| # | Story | Status | Serving tool / note | -| --- | --------------------------------------- | -------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| 4.1 | See balance, level and estimated runway | **DONE** | `mgmt_get_balance`, `mgmt_get_days_estimate` | -| 4.2 | Top up with a card | **DONE** | `mgmt_deposit_with_card` returns a Stripe Checkout URL, `mgmt_card_payment_eligibility` says up front whether the account may use one. **Correction (SHARK-3571): it said the opposite of the truth to every account until this ticket.** The route answers `{isEligible}` (protojson default names) and the shim read `is_eligible`, so the flag was never true and the tool replied "This account is NOT eligible for card (Stripe) payment" to everybody. It is normalised at the client boundary now, both spellings accepted, and an ABSENT flag is a third answer rather than a NO: the tool says the gateway did not report it instead of telling a paying customer they cannot pay. The agent cannot charge anything; a human completes payment. Matches QuickNode and Alchemy, neither exposes autonomous fiat | -| 4.3 | Start or read a subscription | **DONE** | `mgmt_subscribe_recurrent`, `mgmt_get_subscription_prices` (which had the same wire-shape defect as row 4.2 and answered "No subscription prices available" whatever the gateway held; the reply is `{productPrices: [...]}` with `intervalCount` as a protojson string, and it is normalised at the client boundary now), and, since SHARK-3571, the BUNDLE half of the same capability: `mgmt_list_bundles` (the catalog, with the product and price ids a purchase needs) and `mgmt_subscribe_to_bundle` (a Stripe Checkout link, HITL-gated exactly like the recurring initiator). `mgmt_get_subscriptions` READS BOTH KINDS and labels each row with which it is. **Correction (SHARK-3571): this row said DONE while half of it was missing, and the missing half was reported to customers as a fact about their account.** The listing read only `GET /auth/payment/getMySubscriptions`, which never contains bundles (the gateway serves those from `GET /auth/myBundles`, and `bundle_controller.go` resolves it against the same account), so a bundle holder was told they had no subscriptions. That is the failure mode the second rule at the top of this file is about: nobody had read the bundle route inventory. Two honest limits are stated rather than papered over. A list that FAILS to read is reported as unreadable next to the list that did read, because "the bundle route is down" and "you hold no bundles" are different answers and only one of them can be true; and `SubscribeToBundleRequest.resubscribe` is sent as `false` and is NOT exposed as an input, because what the gateway does with `true` is not documented anywhere we have read and a guessed flag on a payment is worse than an absent one. Renewing an existing bundle therefore stays a console action, recorded here as a known limit rather than attributed to a ticket nobody has filed **Correction (SHARK-3571, found by this branch's own adversarial review): the sentence above was true of the intent and false of the code.** `mgmt_get_subscriptions` rendered `No active subscriptions or bundles.` whenever nothing was held, which included the bundle list having failed, the recurring list having failed, and BOTH having failed. The unreadable-list qualifier was appended AFTER it, so the absence was still asserted first, in the words a customer reads, which is the SHARK-3571 defect standing in a different place. The absence is now scoped to the lists that actually answered (`absenceSentence`): one list dead names only the kind that WAS read, and both dead says the answer is empty because neither list could be read rather than because the account holds nothing. `_meta.unreadable` carries the same fact as a list of kinds, for a client that branches on flags rather than on prose, which is the reason the sibling notification work carries `_meta.connected`. Separately, a money AMOUNT that arrived as a JSON number was DROPPED by the wire readers and rendered as `?`, and the cancel approval page said `an unreported amount`: `optString` accepted only strings while `interval_count` in the same object literal accepted both encodings. The amount on a subscription, on a catalogue price and on a bundle offer, plus the ledger's `amount_usd` and `amount_ankr`, now read through `optWireString`, which takes either encoding and returns a string's exact characters so the decimals are never reformatted | -| 4.4 | Cancel a subscription | **DONE** | Ships in SHARK-3546. `mgmt_cancel_subscription` wraps `POST /auth/payment/cancelSubscription` (body `{subscription_id}`), HITL-gated. The old GAP's stated reason was wrong in the way the second rule at the top of this file warns about: no server-verified TOTP path is needed, because the gateway is the MFA authority and the shim simply FORWARDS `totp` as `x-ankr-totp-token`, exactly as it already does on the other MFA-gated routes. **Correction (SHARK-3584): there are SIX such routes, not three, and this row used to name two.** The list was read straight off the gateway's `mfa.go` `targetList` instead of being inferred from the console's client: `DELETE /auth/jwt`, `PATCH /auth/whitelist`, this route, `POST /auth/token/custom/new`, `POST /auth/token/custom/delete` and, added by SHARK-3578, `POST /auth/abstractBindings/unbind`. (The count read FIVE here until SHARK-3570; the sixth had shipped in `MFA_GATED_ACTIONS` and was recorded only in row 6.8.) The code also no longer has to come from the caller: on an account with 2FA the approval page asks the human for it (row 6.6). SHARK-3392 stands unchanged: the shim neither mandates nor verifies the code, and an account without 2FA is let through by the gateway. The approval page states WHAT stops being charged (that subscription's amount, currency and billing period, read from the account's own record rather than from the caller's arguments) and FROM WHEN (no further payment for THAT subscription; the period already paid for runs to its `current_period_end` and is not refunded), plus what does NOT stop (other subscriptions, and pay-as-you-go usage). Two honest limits, both stated to the caller instead of guessed: the route answers with an EMPTY body, so the reply reports that the request was ACCEPTED and points at `mgmt_get_subscriptions` rather than claiming Stripe's resulting state, and nothing tells us whether the gateway ends access at once or lets the paid period run out. An id the account does not hold is refused before any human is asked to approve anything; one that disappears between the approval and the call is refused rather than reported as cancelled **Extended (SHARK-3571): it cancels BOTH kinds, and it picks the route on evidence.** The pre-flight now reads both lists and the id is cancelled through the route whose list it was actually in: `POST /auth/myBundles/unsubscribe` for a bundle, `POST /auth/payment/cancelSubscription` for a recurring one, which is the same branch the console makes in `useSubscription.ts`. Worth recording because it reads as stronger than it is: at multirpc-accounting-gateway 470f9a4 `router.go` points BOTH paths at `paymentController.CancelSubscription`, with the same body and the same acl roles, so sending a bundle to the payment route would not today cancel the wrong object. The evidence-based pick is there because the shim must not tell a customer "bundle" while calling the payment route, and because the two routes are free to diverge. The refusal that opened SHARK-3571 is gone in both directions: an id in NEITHER list is still refused before any human is asked to approve anything (and the refusal now lists both kinds' ids), while an id that is merely unfindable BECAUSE a list could not be read is no longer refused at all: the gateway, which can see both lists, is the authority. The approval page names which kind it is only when the lookup found it; when it did not, the wording stays neutral rather than guessing "recurring" at somebody holding a bundle. `_meta.subscription_kind` carries the same answer for a machine reader. **Correction (SHARK-3571 follow-up, found by this branch's own adversarial review): the unreadable-list reason was computed and DISCARDED.** `findSubscription` built a " list: " string for the case where a list could not be read, and no caller ever read it, which is why mutation could delete it, empty its `.map` and drop its `join("; ")` with the suite green: there was no surface to observe it on. It now reaches the APPROVAL PAGE, which is where it belongs. A human is being asked to approve a cancel on an object this shim could not identify, and "its amount and billing period could not be read" does not tell them whether that is an empty account or a gateway that is down. The tool still does not refuse in that case, which is unchanged and deliberate: the gateway can see both lists and is the authority. Separately, the page's five effect statements are now pinned WHOLE by a test rather than matched with loose alternations, because this row is DONE almost entirely on what that page states and mutation showed every sentence of it, including the widened UNAFFECTED_EFFECT, could be emptied one at a time without a failure | -| 4.5 | Read invoices | **DONE** | `mgmt_list_transactions` + `mgmt_get_invoice_details`. **Correction (SHARK-3575): this row said DONE while the tool it named could not be called.** `mgmt_get_invoice_details` requires a `txId`, `GET /auth/transactionHistory` was not wrapped, and no other tool in the set returns a transaction id, so the only way to reach the invoice read was to find the id in the console, where the document is one click away anyway. A capability that needs an argument nothing can produce is not shipped, and the row is the second rule at the top of this file failing in the other direction: nobody had walked the chain. `mgmt_list_transactions` wraps that route and closes it. It lists the account's billing ledger over a window with the paging the route supports (cursor plus limit), and renders each row as the thing a customer recognises: date, kind, amount and currency, plus the chain and the free-text reason where the route carries them. Three things are read off the route rather than assumed. It has no currency FIELD, so which of `amount_usd` / `amount_ankr` is populated is the currency, and both are shown when both are; its `type` is a proto enum that arrives as a member name from one responder and as an ordinal from another, so both are decoded and an ordinal outside the set is reported as unknown rather than mapped onto the enum's own `UNKNOWN` member; and it carries NO API key or project, so the listing does not pretend to attribute a charge to one. `from` and `to` are the route's only required parameters, so the tool defaults a 30-day window and always states the window it sent, in ISO and in raw milliseconds, which is what makes an empty page diagnosable instead of reading as an account with no history. The `type`, `order_by` and `sort` filters exist on the route and are deliberately NOT plumbed: nothing we have read says whether `type` wants `DEPOSIT` or `TRANSACTION_TYPE_DEPOSIT`, and a filter that silently matches nothing would report an empty ledger to a customer who has one, which is the failure this ticket is about. One known limit, stated in the tool text rather than returned as a blank that reads like an error: a card payment has Stripe documents behind its transaction id, and a crypto deposit has none. The gateway generates that one through `GET /auth/document/invoice/cryptoDeposit`, which requires the on-chain transaction hash and a billing name; `proto.Transaction` carries neither, so it cannot be driven from a listed row and stays a console action. When both URLs are absent `mgmt_get_invoice_details` now says which situations produce that (a crypto deposit, or a card payment whose documents Stripe has not published yet) and that the gateway did answer **Correction (SHARK-3575, found by this branch's own adversarial review): the chain was closed for ONE of the two document types.** `mgmt_get_invoice_details` takes `txId` AND `txType`; the listing produced only the id, and the listing's own guidance hardcoded `(txType DEPOSIT)` for every row. A bundle purchase reaches this ledger as a `DEDUCTION` and its Stripe document is filed under `BUNDLE`, so following that guidance answered "no Stripe document, probably a crypto deposit" while the invoice existed one enum value away. The two vocabularies are unrelated and nothing we have read maps between them: the ledger's `kind` is `proto.TransactionType` (DEPOSIT, DEDUCTION, WITHDRAW, BONUS, COMPENSATION, VOUCHER__, WITHDRAW__, with NO `BUNDLE` member) while the document selector is `StripeDocumentType` = DEPOSIT or BUNDLE, so deriving one from the other would be exactly the guess the second rule at the top of this file forbids. `txType` is therefore OPTIONAL and the tool SEARCHES: omitted, it asks for DEPOSIT and then, only if that answered with no document, for BUNDLE, and it names the type that held the document both in the reply and in `_meta.tx_type`. A probe that FAILS is not a verdict on its type, so the other one is still tried; when NEITHER type answers at all the reply is an error rather than a claim that no document exists. The empty-result note is now honest in both directions: after a search it states that both types were asked and the type is therefore not the reason, and when the caller pinned a type it names the THIRD situation the shipped wording omitted (the document may be filed under the other type) together with the value to pass instead | -| 4.6 | Pay with crypto / on-chain PAYG | **GAP** | The console does this through wallet contracts (`PAYGContractManager`). Server-side equivalent would need custody; x402 is the intended path. SHARK-3550 | +| # | Story | Status | Serving tool / note | +| --- | --------------------------------------- | -------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 4.1 | See balance, level and estimated runway | **DONE** | `mgmt_get_balance`, `mgmt_get_days_estimate` | +| 4.2 | Top up with a card | **DONE** | `mgmt_deposit_with_card` returns a Stripe Checkout URL, `mgmt_card_payment_eligibility` says up front whether the account may use one. **Correction (SHARK-3571): it said the opposite of the truth to every account until this ticket.** The route answers `{isEligible}` (protojson default names) and the shim read `is_eligible`, so the flag was never true and the tool replied "This account is NOT eligible for card (Stripe) payment" to everybody. It is normalised at the client boundary now, both spellings accepted, and an ABSENT flag is a third answer rather than a NO: the tool says the gateway did not report it instead of telling a paying customer they cannot pay. The agent cannot charge anything; a human completes payment. Matches QuickNode and Alchemy, neither exposes autonomous fiat | +| 4.3 | Start or read a subscription | **DONE** | `mgmt_subscribe_recurrent`, `mgmt_get_subscription_prices` (which had the same wire-shape defect as row 4.2 and answered "No subscription prices available" whatever the gateway held; the reply is `{productPrices: [...]}` with `intervalCount` as a protojson string, and it is normalised at the client boundary now), and, since SHARK-3571, the BUNDLE half of the same capability: `mgmt_list_bundles` (the catalog, with the product and price ids a purchase needs) and `mgmt_subscribe_to_bundle` (a Stripe Checkout link, HITL-gated exactly like the recurring initiator). `mgmt_get_subscriptions` READS BOTH KINDS and labels each row with which it is. **Correction (SHARK-3571): this row said DONE while half of it was missing, and the missing half was reported to customers as a fact about their account.** The listing read only `GET /auth/payment/getMySubscriptions`, which never contains bundles (the gateway serves those from `GET /auth/myBundles`, and `bundle_controller.go` resolves it against the same account), so a bundle holder was told they had no subscriptions. That is the failure mode the second rule at the top of this file is about: nobody had read the bundle route inventory. Two honest limits are stated rather than papered over. A list that FAILS to read is reported as unreadable next to the list that did read, because "the bundle route is down" and "you hold no bundles" are different answers and only one of them can be true; and `SubscribeToBundleRequest.resubscribe` is sent as `false` and is NOT exposed as an input, because what the gateway does with `true` is not documented anywhere we have read and a guessed flag on a payment is worse than an absent one. Renewing an existing bundle therefore stays a console action, recorded here as a known limit rather than attributed to a ticket nobody has filed **Correction (SHARK-3571, found by this branch's own adversarial review): the sentence above was true of the intent and false of the code.** `mgmt_get_subscriptions` rendered `No active subscriptions or bundles.` whenever nothing was held, which included the bundle list having failed, the recurring list having failed, and BOTH having failed. The unreadable-list qualifier was appended AFTER it, so the absence was still asserted first, in the words a customer reads, which is the SHARK-3571 defect standing in a different place. The absence is now scoped to the lists that actually answered (`absenceSentence`): one list dead names only the kind that WAS read, and both dead says the answer is empty because neither list could be read rather than because the account holds nothing. `_meta.unreadable` carries the same fact as a list of kinds, for a client that branches on flags rather than on prose, which is the reason the sibling notification work carries `_meta.connected`. Separately, a money AMOUNT that arrived as a JSON number was DROPPED by the wire readers and rendered as `?`, and the cancel approval page said `an unreported amount`: `optString` accepted only strings while `interval_count` in the same object literal accepted both encodings. The amount on a subscription, on a catalogue price and on a bundle offer, plus the ledger's `amount_usd` and `amount_ankr`, now read through `optWireString`, which takes either encoding and returns a string's exact characters so the decimals are never reformatted | +| 4.4 | Cancel a subscription | **DONE** | Ships in SHARK-3546. `mgmt_cancel_subscription` wraps `POST /auth/payment/cancelSubscription` (body `{subscription_id}`), HITL-gated. The old GAP's stated reason was wrong in the way the second rule at the top of this file warns about: no server-verified TOTP path is needed, because the gateway is the MFA authority and the shim simply FORWARDS `totp` as `x-ankr-totp-token`, exactly as it already does on the other MFA-gated routes. **Correction (SHARK-3584): there are SIX such routes, not three, and this row used to name two.** The list was read straight off the gateway's `mfa.go` `targetList` instead of being inferred from the console's client: `DELETE /auth/jwt`, `PATCH /auth/whitelist`, this route, `POST /auth/token/custom/new`, `POST /auth/token/custom/delete` and, added by SHARK-3578, `POST /auth/abstractBindings/unbind`. (The count read FIVE here until SHARK-3570; the sixth had shipped in `MFA_GATED_ACTIONS` and was recorded only in row 6.8.) The code also no longer has to come from the caller: on an account with 2FA the approval page asks the human for it (row 6.6). SHARK-3392 stands unchanged: the shim neither mandates nor verifies the code, and an account without 2FA is let through by the gateway. The approval page states WHAT stops being charged (that subscription's amount, currency and billing period, read from the account's own record rather than from the caller's arguments) and FROM WHEN (no further payment for THAT subscription; the period already paid for runs to its `current_period_end` and is not refunded), plus what does NOT stop (other subscriptions, and pay-as-you-go usage). Two honest limits, both stated to the caller instead of guessed: the route answers with an EMPTY body, so the reply reports that the request was ACCEPTED and points at `mgmt_get_subscriptions` rather than claiming Stripe's resulting state, and nothing tells us whether the gateway ends access at once or lets the paid period run out. An id the account does not hold is refused before any human is asked to approve anything; one that disappears between the approval and the call is refused rather than reported as cancelled **Extended (SHARK-3571): it cancels BOTH kinds, and it picks the route on evidence.** The pre-flight now reads both lists and the id is cancelled through the route whose list it was actually in: `POST /auth/myBundles/unsubscribe` for a bundle, `POST /auth/payment/cancelSubscription` for a recurring one, which is the same branch the console makes in `useSubscription.ts`. Worth recording because it reads as stronger than it is: at multirpc-accounting-gateway 470f9a4 `router.go` points BOTH paths at `paymentController.CancelSubscription`, with the same body and the same acl roles, so sending a bundle to the payment route would not today cancel the wrong object. The evidence-based pick is there because the shim must not tell a customer "bundle" while calling the payment route, and because the two routes are free to diverge. The refusal that opened SHARK-3571 is gone in both directions: an id in NEITHER list is still refused before any human is asked to approve anything (and the refusal now lists both kinds' ids), while an id that is merely unfindable BECAUSE a list could not be read is no longer refused at all: the gateway, which can see both lists, is the authority. The approval page names which kind it is only when the lookup found it; when it did not, the wording stays neutral rather than guessing "recurring" at somebody holding a bundle. `_meta.subscription_kind` carries the same answer for a machine reader. **Correction (SHARK-3571 follow-up, found by this branch's own adversarial review): the unreadable-list reason was computed and DISCARDED.** `findSubscription` built a " list: " string for the case where a list could not be read, and no caller ever read it, which is why mutation could delete it, empty its `.map` and drop its `join("; ")` with the suite green: there was no surface to observe it on. It now reaches the APPROVAL PAGE, which is where it belongs. A human is being asked to approve a cancel on an object this shim could not identify, and "its amount and billing period could not be read" does not tell them whether that is an empty account or a gateway that is down. The tool still does not refuse in that case, which is unchanged and deliberate: the gateway can see both lists and is the authority. Separately, the page's five effect statements are now pinned WHOLE by a test rather than matched with loose alternations, because this row is DONE almost entirely on what that page states and mutation showed every sentence of it, including the widened UNAFFECTED_EFFECT, could be emptied one at a time without a failure | +| 4.5 | Read invoices | **DONE** | `mgmt_list_transactions` + `mgmt_get_invoice_details`. **Correction (SHARK-3575): this row said DONE while the tool it named could not be called.** `mgmt_get_invoice_details` requires a `txId`, `GET /auth/transactionHistory` was not wrapped, and no other tool in the set returns a transaction id, so the only way to reach the invoice read was to find the id in the console, where the document is one click away anyway. A capability that needs an argument nothing can produce is not shipped, and the row is the second rule at the top of this file failing in the other direction: nobody had walked the chain. `mgmt_list_transactions` wraps that route and closes it. It lists the account's billing ledger over a window with the paging the route supports (cursor plus limit), and renders each row as the thing a customer recognises: date, kind, amount and currency, plus the chain and the free-text reason where the route carries them. Three things are read off the route rather than assumed. It has no currency FIELD, so which of `amount_usd` / `amount_ankr` is populated is the currency, and both are shown when both are; its `type` is a proto enum that arrives as a member name from one responder and as an ordinal from another, so both are decoded and an ordinal outside the set is reported as unknown rather than mapped onto the enum's own `UNKNOWN` member; and it carries NO API key or project, so the listing does not pretend to attribute a charge to one. `from` and `to` are the route's only required parameters, so the tool defaults a 30-day window and always states the window it sent, in ISO and in raw milliseconds, which is what makes an empty page diagnosable instead of reading as an account with no history. The `type`, `order_by` and `sort` filters exist on the route and are deliberately NOT plumbed: nothing we have read says whether `type` wants `DEPOSIT` or `TRANSACTION_TYPE_DEPOSIT`, and a filter that silently matches nothing would report an empty ledger to a customer who has one, which is the failure this ticket is about. One known limit, stated in the tool text rather than returned as a blank that reads like an error: a card payment has Stripe documents behind its transaction id, and a crypto deposit has none. The gateway generates that one through `GET /auth/document/invoice/cryptoDeposit`, which requires the on-chain transaction hash and a billing name; `proto.Transaction` carries neither, so it cannot be driven from a listed row and stays a console action. When both URLs are absent `mgmt_get_invoice_details` now says which situations produce that (a crypto deposit, or a card payment whose documents Stripe has not published yet) and that the gateway did answer **Correction (SHARK-3575, found by this branch's own adversarial review): the chain was closed for ONE of the two document types.** `mgmt_get_invoice_details` takes `txId` AND `txType`; the listing produced only the id, and the listing's own guidance hardcoded `(txType DEPOSIT)` for every row. A bundle purchase reaches this ledger as a `DEDUCTION` and its Stripe document is filed under `BUNDLE`, so following that guidance answered "no Stripe document, probably a crypto deposit" while the invoice existed one enum value away. The two vocabularies are unrelated and nothing we have read maps between them: the ledger's `kind` is `proto.TransactionType` (DEPOSIT, DEDUCTION, WITHDRAW, BONUS, COMPENSATION, VOUCHER__, WITHDRAW__, with NO `BUNDLE` member) while the document selector is `StripeDocumentType` = DEPOSIT or BUNDLE, so deriving one from the other would be exactly the guess the second rule at the top of this file forbids. `txType` is therefore OPTIONAL and the tool SEARCHES: omitted, it asks for DEPOSIT and then, only if that answered with no document, for BUNDLE, and it names the type that held the document both in the reply and in `_meta.tx_type`. A probe that FAILS is not a verdict on its type, so the other one is still tried; when NEITHER type answers at all the reply is an error rather than a claim that no document exists. The empty-result note is now honest in both directions: after a search it states that both types were asked and the type is therefore not the reason, and when the caller pinned a type it names the THIRD situation the shipped wording omitted (the document may be filed under the other type) together with the value to pass instead **Production caveat (SHARK-3593), and it is not this branch's to fix.** This row is DONE on the legend's terms, pinned by test and by live run, and it still FAILS in production on at least one account: the gateway answers 504 deadline_exceeded on `GET /auth/transactionHistory` for a large ledger, whatever window is asked for. So `mgmt_list_transactions` returns the route's error rather than a ledger, and the invoice read behind it is unreachable for that account. A gateway problem, tracked as SHARK-3593; read it as an upstream limit rather than as a defect in this code, and see REVIEW-READY section 4.6 | +| 4.6 | Pay with crypto / on-chain PAYG | **GAP** | The console does this through wallet contracts (`PAYGContractManager`). Server-side equivalent would need custody; x402 is the intended path. SHARK-3550 | ## 5. Notifications