From 9d6ab5ec5e53cca8f6e10f87a3cb67c57992eac0 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sat, 22 Aug 2026 22:09:08 -0700 Subject: [PATCH] docs(ax): a dual-auth route degrades to the other identity silently MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @sprint-review, on the comment buried in #1127's test: worth more than the bug it explains, and the third instance today of a harness manufacturing a shape production never emits. `/api/v1/tasks` dispatches on the token — `cm_agent_*` routes to agentRuntimeAuth, anything else to the human path — and the two branches produce different request shapes. The fallback is unconditional, so omitting the header does not fail; it silently succeeds as a human. A test can carry "agent" in its name, assert the right thing, go green, and be about the other identity entirely. That is how the first draft of the tests written to cover the agent shape passed while testing the human one. Filed as an agent-experience defect rather than a testing note because the false model comes from the surface: a route named `auth`, mounted once, reads as one identity contract, and nothing at the call site says it picks between two request shapes by string prefix. Companion to reviewer-checklist rule 17 in this same PR, which covers the method that missed it. Co-Authored-By: Claude Opus 5 --- docs/development/agent-experience-audit.md | 55 ++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/docs/development/agent-experience-audit.md b/docs/development/agent-experience-audit.md index 4d304fb55..c12c72a19 100644 --- a/docs/development/agent-experience-audit.md +++ b/docs/development/agent-experience-audit.md @@ -2518,3 +2518,58 @@ workflow's clock instead of the pod's, the parent commit instead of the head. before it merged — both guards appear in its 18:30 run, three hours after the retarget. Reading the check *names* answers a question the check *count* cannot. + +## 42. A dual-auth route degrades to the other identity silently, so a test can name a shape it never exercises (2026-08-22, sprint-review + pod-architect) + +> Numbering follows entry 41's caveat: 39 and 40 are still reserved by #1122 +> and #1132. Renumber this one, not those, if they land in another order. + +`/api/v1/tasks` does not take one auth middleware. It dispatches on the token: + +```ts +function auth(req: AuthReq, res: Res, next: () => void) { + const token = ((req.header?.('Authorization') || '').replace('Bearer ', '')); + if (token.startsWith('cm_agent_')) return agentRuntimeAuth(req, res, next); + return regularAuth(req, res, next); +} +``` + +The two branches produce **different request shapes** — `agentRuntimeAuth` +assigns `req.agentUser` and never `req.user`; the human path does the reverse. +And the fallback is unconditional: a request with no `cm_agent_` prefix does +not fail, it takes the human branch and succeeds. Omitting the header is +indistinguishable, from the response, from supplying it. + +**What that costs a test author.** Every case in +`tasksApi.updateRenewsLease.test.js` went through the human mock, so +`req.user` was always populated and the agent branch had never run — across +two rounds of fixes to code whose whole subject was the agent identity. The +first draft of the tests written specifically to cover the agent shape +*also* went through the human path, and passed. A test can carry "agent" in +its name, assert the right thing, go green, and be about the other identity +entirely. The only tell is a header the test does not have to set. + +**Why this is an agent-experience defect and not a testing anecdote.** The +false model is taught by the surface, not by the test: a route named `auth`, +mounted once, reads as one identity contract. Nothing at the call site says +"this endpoint has two request shapes and picks between them by string +prefix." Three of the day's four wrong conclusions trace to reading `req.user` +on a path that only ever populates `req.agentUser` — including +`resolveAgentInstanceId`, whose `req.user?.isBot` gate has been dead on the +agent path since it was written. + +**What to do.** + +- **Setting the auth header is part of naming the shape.** A test whose + subject is the agent path must send `Bearer cm_agent_*`; without it the name + is the only agent-specific thing in the test. Assert the shape arrived + (`expect(req.agentUser).toBeDefined()` in the mock, or one case that must + fail on the human path) rather than trusting the route to have chosen. +- **A silent branch needs a loud test.** Where a dispatcher falls back rather + than erroring, at least one case must prove the fallback did *not* fire. +- **Grep the middleware, not the docstring, for which property is assigned.** + `agentRuntimeAuth` assigns `req.agentUser` at `:102` and `:191` and nowhere + else — one grep, and it settles every question of this shape. +- Companion rule, on the method that missed it: reviewer-checklist rule 17 — + a mutation proves a term matters to the suite, not that the suite's shape is + real.