Six defects closed from one review round — including a headline feature that was 100% broken

2026-08-08

routerapimcpbudgetinfragateway

LOCKSTEP TRACEABILITY MATRIX --- api_endpoints: ["POST /v1/chat/completions", "POST /v1/eval/datasets/{id}/items", "POST /v1/mcp/connect"] sdk_methods_updated: ["none — error-shape and routing fixes only"] mcp_tools_updated: ["none — the transport was repaired, no tool changed"] ---

What We Built

Remediation of round 1 of the stochastic review. Six defects, all found by personas doing ordinary jobs rather than by tests looking for known risks. The pooled-inference leak is written up separately in 2026-08-08-pooled-inference-leak.md.

model: "auto" was 100% broken for a correctly-configured tenant. Three of three attempts failed. Auto selected deepseek/deepseek-chat; the tenant held only OpenAI and Anthropic keys, so applyAuthOverride correctly threw ByokRequiredError rather than leak a platform credential; the BYOK cascade fell through to OpenAI and picked openai/text-embedding-3-small — an embedding model — and OpenAI answered 403 "You are not allowed to sample from this model". The cascade takes only its top candidate, so the request died there.

Embeddings are in PROVIDER_PRICING so /v1/embeddings can resolve a bare model id, but the registry builds _completion_ endpoints from the same table. They carry no MODEL_CAPABILITIES entry, so detectCapabilities returns an empty set — which passes every capability filter, because a plain text prompt requires nothing. Nothing anywhere distinguished modality.

A tenant could write into another tenant's dataset. POST /v1/eval/datasets/{id}/items inserted rows stamped with the caller's tenantId under whatever datasetId they supplied, without ever looking the parent up — while every other method on that store constrains by eq(evalDatasets.tenantId, tenantId). B could write into an object B was not allowed to read.

MCP was entirely dead. Every POST /v1/mcp/connect returned -32700 Parse error byte-identically for a valid initialize, for literal garbage, and for an empty body. The capability framework parses the request body before the handler runs, consuming the stream on c.req.raw; that spent Request went straight to the transport, which read nothing. All 121 advertised tools were unreachable, while GET on the same URL served a full manifest.

Agent spend was invisible and uncapped. Four cost surfaces, one telling the truth. GET /v1/agents/{id}/runs reported accurate per-session cost while /v1/self returned spent_usd: 0 and x-budget-remaining stayed frozen. recordSpend wrote only to the per-agent ledger, never to ${tenantId}:tenant:${period} — the key /v1/self reads and the budget middleware enforces against.

The health check could not distinguish "absent" from "working". GET /health returned db: true both when Postgres answered SELECT 1 and when Postgres was absent entirely, so a deployment that lost its DATABASE_URL would report healthy. Redis was probed via redis.status === "ready" — the client's connection state — so a connected-but-wedged server reported ok indefinitely.

Nothing paged, and it was never missing code. infra/monitoring-stack.yml defines 14 alarms, including the four reported missing. None had ever been created: the deploy workflow gated the entire stack on a Slack webhook secret that does not exist, warned, and moved on.

Why It Matters

auto is the documented default and the feature the product is named around. It failed every time for a paying, correctly-configured tenant, and it could not be discovered from a sandbox account because sandbox tenants never route at all. It took configuring real provider keys to make the failure reachable.

The cross-tenant write is the one a customer would care about most. Reads stayed tenant-filtered, so it is an integrity and quota defect rather than a disclosure one — B never sees A's rows — but B could grow A's dataset and consume A's storage.

And a governance product whose own MCP surface cannot complete a handshake, whose spend ledger reads zero, and whose alarms exist only as YAML has an evidence problem before it has a feature problem.

How It Works

Each fix is an explicit, unconditional check placed where both paths meet.

Modality. An embeddingOnly marker on the catalog entry, propagated to the endpoint and excluded in filterEndpoints — the one function shared by normal selection (model-router-select.ts:734) and the BYOK cascade (:1319), so both close together. It runs first and is not relaxable: a modality mismatch is not a preference the way circuit-breaking is.

Ownership. addItems returns null for a dataset that is missing or foreign, and both callers translate that to 404 "Dataset not found" — deliberately indistinguishable from a nonexistent id. The lookup runs inside the same transaction as the insert, so a concurrently deleted dataset cannot slip between check and write.

Body replay. A fresh Request carrying the already-parsed body, preserving method, URL and headers. Known limitation, stated rather than hidden: JSON-RPC _batch_ requests are a top-level array and this capability's object-shaped schema rejects them before the handler, so batches still do not reach the transport.

Accounting. createAgentSpendRecorder() writes the tenant ledger first, then the per-agent one, and is no longer gated on the agent budget manager existing — that gate meant a deployment without one recorded nothing anywhere.

Health. checks.db / checks.redis became ok | not_configured | fail. The booleans deliberately did not change: /health is consumed by both the ALB target group and the ECS container health check, and an unconfigured dependency must keep reading as healthy in self-hosted mode. Flipping them would make the minimal deployment kill itself. Redis now does a real PING.

Alarms. SlackWebhookArn defaults to empty with the SNS subscription behind a HasSlackWebhook condition, so all 14 alarms deploy with or without a subscriber. Failure to discover the ALB is now a hard error. The template's ECSCluster/ECSService defaults were also still the pre-migration names, so even a successful deploy would have watched a cluster that does not exist.

The Numbers

MetricBeforeAfter
model: "auto" success0 / 33 / 3
auto:fast / auto:floorboth errorboth OK
Cross-tenant write (B → A)201 Created404 (owner still 201)
MCP initialize-32700 alwaysreal handshake, 121 tools
Alarms covering the service014
Tenant spend after an agent run$0 reportedrecorded and enforced
Unit suite93609375 passing

Every fix carries two tests: one proving the behaviour, one proving it is conditional — an unconditional refusal or a hardcoded pass would satisfy the first while breaking every real caller. All were confirmed to fail with the fix reverted.

Competitive Edge

None of these were found by tests. They were found by personas doing ordinary jobs — onboarding as a new integrator, self-onboarding as an agent, going on call — and noticing that two documented surfaces disagreed. A test asserts what someone already suspected; this found what nobody had thought to suspect, including a headline feature that had been broken for an unknown length of time with no alarm, no failing test, and no report.

Postscript — two findings about the harness itself

src/gateway/health-http.test.ts sat in a brokenGatewayTests exclusion list, and src/gateway/ is excluded from the unit config — so the endpoint the load balancer uses to decide whether this service is alive had zero running coverage**. Its six failures were staleness, not rot (a mock missing socket, a renamed field, and assertions that /health 503s when it is deliberately always 200). Repaired, extended to 9 tests, and removed from the exclusion list.

Separately: pnpm test:fast -- does not filter — it runs the whole suite. Safe, but it means a "targeted" run never proves anything about the named file, which is how the health tests being disabled went unnoticed for so long.

Lockstep Checklist

  • [x] API Routes: behaviour and error-shape changes only; no path, method or request schema changed.
  • [x] TS SDK: no change required — errors surface generically, verified in packages/sdk-ts.
  • [x] Python SDK: no change required — _resource.py raises for any status_code >= 400.
  • [x] MCP Schemas: no tool added or altered; the transport was repaired.
  • [x] Master Record: n/a — defects closed in existing capabilities.