Skip to content

Commit 3eb776a

Browse files
os-zhuangclaude
andauthored
chore(guard): extend the route-envelope check to the dispatcher domains (#4038 follow-up) (#4056)
The platform answers REST from two kinds of file. Route modules (*-routes.ts) call `res.json(...)`, which this scan counted. Dispatcher domains (runtime/src/domains/*.ts) RETURN `{ status, body }` for a central sender, so they never call `res.json` and were invisible to it by construction. That gap cost something real: /share-links emitted its payload under both `data` and a legacy `link`/`links` for as long as nothing looked, and it surfaced only because #3983's consumer sweep happened to walk past it (#4038, fixed in #4049). I noted the blind spot there; this closes it. The new table counts hand-built `response: { … }` literals per domain. A domain answering only through the deps.* helpers hand-builds zero and cannot drift; any hand-built one now needs a declared count plus a `note` saying why. Hand-building is not automatically wrong — four kinds show up and only the last is drift: 1. enveloped, but the helper cannot express it — deps.success hardcodes 200 so a 201 is assembled by hand, and deps.error carries no headers so a 405 with `Allow:` is too (keys.ts, share-links.ts, one in mcp.ts) 2. passthrough of a body this dispatcher does not own (mcp.ts, ai.ts) 3. a foreign wire format a client library requires — /auth answers better-auth's shapes because better-auth's client parses them (auth.ts x4) 4. drift 16 domains audited: 11 helper-only, 5 declaring hand-built responses, 1 ratcheted. The ratchet is ai.ts -> #4053, where GET /ai/agents is SDK-addressable but unenveloped and the SDK compensates by reading `.agents` off the raw body — converting it without changing cloud and the SDK together makes `client.ai.agents.list()` return an empty list silently, which is indistinguishable from the legitimate "no AI service configured" state. That warning now fails CI for whoever tries it, instead of sitting in an issue nobody reads. Five self-test assertions cover the new scan. One caught a real false positive while it was being written: matching the `response` key alone counted a PAYLOAD field named response (`deps.success({ response: … })`) as a hand-built response. It now requires `response` to be a sibling of `handled`, which is what makes the pair an HttpDispatcherResult rather than just an object with that key. Verified the guard actually bites: adding a hand-built response to a helper-only domain fails with the site and the four-kind classification prompt. Claude-Session: https://claude.ai/code/session_01CYbS3kS8xzsHNXFTzp4e2z Co-authored-by: Claude <noreply@anthropic.com>
1 parent de6daa5 commit 3eb776a

2 files changed

Lines changed: 289 additions & 6 deletions

File tree

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
---
2+
---
3+
4+
Extend `scripts/check-route-envelope.mjs` to the dispatcher domains — the REST
5+
surface it structurally could not see. Deliberately empty frontmatter: this is a
6+
CI guard, no published behaviour, type, or wire shape changes.
7+
8+
The platform answers REST from two kinds of file. Route modules (`*-routes.ts`)
9+
call `res.json(...)`, which the scan counted. Dispatcher domains
10+
(`runtime/src/domains/*.ts`) RETURN `{ status, body }` for a central sender, so
11+
they never call `res.json` and were invisible to it by construction.
12+
13+
That gap cost something real: `/share-links` emitted its payload under both `data`
14+
and a legacy `link` / `links` for as long as nothing looked, and it surfaced only
15+
because #3983's consumer sweep happened to walk past it (#4038, fixed in #4049).
16+
17+
The new table counts hand-built `response: { … }` literals per domain. A domain
18+
answering only through the `deps.success` / `deps.error` / `deps.routeNotFound` /
19+
`deps.errorFromThrown` helpers hand-builds **zero** and cannot drift; any
20+
hand-built response now has to be declared with a `note` saying why. Hand-building
21+
is not automatically wrong — four kinds show up and only the last is drift:
22+
23+
1. Enveloped, but the helper cannot express it — `deps.success` hardcodes status
24+
200, so a 201 must be assembled by hand, and `deps.error` carries no headers,
25+
so a 405 with `Allow:` must be too (`keys.ts`, `share-links.ts`, one in `mcp.ts`).
26+
2. Passthrough of a body this dispatcher does not own — an upstream Web Response,
27+
another service's result (`mcp.ts`, `ai.ts`).
28+
3. A foreign wire format a client library requires — `/auth` answers better-auth's
29+
shapes because better-auth's client parses them (`auth.ts` ×4).
30+
4. Drift.
31+
32+
Result: **16 domains audited, 11 helper-only, 5 declaring hand-built responses,
33+
1 ratcheted.** The ratchet is `ai.ts`#4053: `GET /ai/agents` is SDK-addressable
34+
but unenveloped, and the SDK compensates by reading `.agents` off the raw body, so
35+
converting it without changing cloud and the SDK in the same batch makes
36+
`client.ai.agents.list()` return an empty list silently. That warning now fails CI
37+
for whoever tries, instead of sitting in an issue.
38+
39+
Five self-test assertions cover the new scan, one of which caught a real
40+
false positive while it was being written: matching the `response` key alone
41+
counted a *payload field* named `response` (`deps.success({ response: … })`) as a
42+
hand-built response. It now requires `response` to be a sibling of `handled`
43+
that pair is the `HttpDispatcherResult`.

scripts/check-route-envelope.mjs

Lines changed: 246 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,31 @@
1818
* that was a bare string, so `body.error.message` read `undefined`
1919
* (#3636 → #3675 → #3689 → #3843).
2020
*
21+
* ## Two surfaces
22+
*
23+
* The platform answers REST from two kinds of file, and this audits both:
24+
*
25+
* 1. **Route modules** (`*-routes.ts`) — call `res.json(...)`. Counted below
26+
* in `MODULES`.
27+
* 2. **Dispatcher domains** (`runtime/src/domains/*.ts`) — RETURN
28+
* `{ status, body }` for a central sender, so they never call `res.json`
29+
* and the first scan cannot see them. Counted in `DISPATCHER_DOMAINS`.
30+
*
31+
* Surface 2 was added after that blind spot cost something real: `/share-links`
32+
* emitted its payload under both `data` and a legacy `link` / `links` for as
33+
* long as nothing looked, and it was found by hand while sweeping consumers for
34+
* #3983 rather than by any guard (#4038).
35+
*
2136
* ## Why a whole-repo scan rather than a per-module test
2237
*
23-
* The load-bearing check is structural, not per-route: it COUNTS the response
24-
* write sites. When every body in a module goes through its `sendOk` /
38+
* The load-bearing check is structural, not per-route: it COUNTS the sites where
39+
* a response gets built. When every body in a module goes through its `sendOk` /
2540
* `sendError` pair, that count is fixed at two and does not grow with the route
2641
* list — so a *new* route that hand-rolls a body moves the count and fails here,
2742
* which is the one thing a driven-body test can never cover (it can only drive
28-
* the routes that existed the day it was written).
43+
* the routes that existed the day it was written). The domain table applies the
44+
* same idea from the other end: a domain that answers only through the `deps.*`
45+
* helpers hand-builds zero responses, so any hand-built one has to be classified.
2946
*
3047
* #3675 / #3689 shipped this as a regex block copied into each converted
3148
* package. Three copies was the signal it wanted lifting (#3843 option 3), and
@@ -138,6 +155,88 @@ const MODULES = {
138155
/** Identifiers whose `.json()` READS a request rather than writing a response. */
139156
const REQUEST_RECEIVERS = new Set(['req', 'request']);
140157

158+
// ── The dispatcher's OTHER surface ───────────────────────────────────────────
159+
160+
/**
161+
* `packages/runtime/src/domains/*.ts` — the dispatcher domain handlers.
162+
*
163+
* These serve REST paths too, but they never call `res.json(...)`: they RETURN
164+
* `{ status, body }` for a central sender. So the scan above cannot see them, by
165+
* construction — which is not a theoretical gap. `/share-links` emitted the
166+
* payload under both `data` and a legacy `link` / `links` for as long as that
167+
* blind spot existed, and it was found by hand while sweeping consumers for
168+
* #3983, not by any guard (#4038).
169+
*
170+
* The structural fact here is the mirror of `responses` above. A domain that
171+
* routes every answer through the `deps.success` / `deps.error` /
172+
* `deps.routeNotFound` / `deps.errorFromThrown` helpers hand-builds **zero**
173+
* response literals, and cannot drift: the envelope lives in one place for all
174+
* of them. So this counts hand-built `response: { … }` object literals per file,
175+
* and a new one has to be classified rather than merged quietly.
176+
*
177+
* handBuilt — `response:` assigned an object literal instead of a `deps.*` call
178+
* note — why those exist. REQUIRED whenever handBuilt > 0.
179+
* ratchet — set when one of them is real, tracked drift.
180+
*
181+
* Hand-building is not automatically wrong. Four kinds show up, and only the
182+
* last is drift:
183+
*
184+
* 1. Enveloped, but the helper cannot express the response. `deps.success`
185+
* hardcodes status 200, so a 201 must be built by hand, and `deps.error`
186+
* carries no headers, so a 405 with `Allow:` must be too. These emit the
187+
* declared envelope — they just assemble it themselves.
188+
* 2. Passthrough of a body this dispatcher does not own (an upstream Web
189+
* Response, another service's result). The envelope does not govern bytes
190+
* we are only relaying.
191+
* 3. A foreign wire format a client library requires — `/auth` answers
192+
* better-auth's shapes because better-auth's client parses them.
193+
* 4. Drift.
194+
*/
195+
const DISPATCHER_DOMAIN_DIR = 'packages/runtime/src/domains';
196+
197+
const DISPATCHER_DOMAINS = {
198+
'actions.ts': { handBuilt: 0 },
199+
'analytics.ts': { handBuilt: 0 },
200+
'automation.ts': { handBuilt: 0 },
201+
'data.ts': { handBuilt: 0 },
202+
'i18n.ts': { handBuilt: 0 },
203+
'meta.ts': { handBuilt: 0 },
204+
'notifications.ts': { handBuilt: 0 },
205+
'packages.ts': { handBuilt: 0 },
206+
'security.ts': { handBuilt: 0 },
207+
'storage.ts': { handBuilt: 0 },
208+
'ui.ts': { handBuilt: 0 },
209+
210+
// Kind 1 — enveloped, hand-built only because the helper cannot say it.
211+
'keys.ts': {
212+
handBuilt: 1,
213+
note: 'POST /keys is a 201 and `deps.success` hardcodes 200; the body is the declared envelope',
214+
},
215+
'share-links.ts': {
216+
handBuilt: 1,
217+
note: 'POST /share-links is a 201, same reason as /keys (#4038 removed the duplicate key it also carried)',
218+
},
219+
220+
// Kinds 1 and 2 together.
221+
'mcp.ts': {
222+
handBuilt: 2,
223+
note: 'one 405 that must carry an `Allow:` header (`deps.error` takes none) and emits the declared envelope; one passthrough of the upstream MCP Web Response',
224+
},
225+
226+
// Kind 3 — a foreign wire format, all four in the mock/fallback path.
227+
'auth.ts': {
228+
handBuilt: 4,
229+
note: "bridges better-auth, whose client parses ITS shapes (`{ user, session }`, `{ session: null, user: null }`, `{ success: true }`) — BaseResponseSchema does not govern them",
230+
},
231+
232+
// Kind 4 — real drift, tracked.
233+
'ai.ts': {
234+
handBuilt: 2,
235+
ratchet: '#4053',
236+
note: 'one passthrough of the AI service result (kind 2); one is the unenveloped `{ agents: [] }` fallback for GET /ai/agents — an SDK-addressable route whose conversion must land with cloud and the SDK together or `ai.agents.list()` silently returns []',
237+
},
238+
};
239+
141240
/**
142241
* Count the envelope-relevant facts in one module's source.
143242
*
@@ -213,6 +312,55 @@ export function scanSource(source, fileName = 'module.ts') {
213312
return found;
214313
}
215314

315+
/**
316+
* Count hand-built response literals in one dispatcher domain's source.
317+
*
318+
* `response: { … }` is hand-built; `response: deps.success(…)` is not. Comments
319+
* and strings are not tokens, so quoting either form in prose cannot move the
320+
* count.
321+
*
322+
* Only a `response` that is a sibling of `handled` counts — that pair IS the
323+
* `HttpDispatcherResult`. A payload field that happens to be named `response`
324+
* (`deps.success({ response: … })`) is data, not a response, and the self-test
325+
* pins the difference: matching the key alone counted it.
326+
*
327+
* @param {string} source TypeScript source text.
328+
* @returns {{handBuilt: number, sites: string[]}}
329+
*/
330+
export function scanDomainSource(source, fileName = 'domain.ts') {
331+
const sf = ts.createSourceFile(fileName, source, ts.ScriptTarget.Latest, true);
332+
const found = { handBuilt: 0, sites: [] };
333+
334+
const isDispatcherResult = (obj) =>
335+
ts.isObjectLiteralExpression(obj) &&
336+
obj.properties.some(
337+
(p) => ts.isPropertyAssignment(p) && ts.isIdentifier(p.name) && p.name.text === 'handled',
338+
);
339+
340+
const visit = (node) => {
341+
if (
342+
ts.isPropertyAssignment(node) &&
343+
ts.isIdentifier(node.name) &&
344+
node.name.text === 'response' &&
345+
ts.isObjectLiteralExpression(node.initializer) &&
346+
isDispatcherResult(node.parent)
347+
) {
348+
found.handBuilt += 1;
349+
found.sites.push(`${fileName}:${sf.getLineAndCharacterOfPosition(node.getStart(sf)).line + 1}`);
350+
}
351+
ts.forEachChild(node, visit);
352+
};
353+
visit(sf);
354+
return found;
355+
}
356+
357+
/** Domain handler files, by basename. */
358+
function discoverDomains() {
359+
return readdirSync(join(ROOT, DISPATCHER_DOMAIN_DIR))
360+
.filter((n) => n.endsWith('.ts') && !n.endsWith('.d.ts') && !n.includes('.test.') && !n.includes('.conformance.'))
361+
.sort();
362+
}
363+
216364
/** Recursively collect candidate route-module paths under `packages/`. */
217365
function discover() {
218366
const out = [];
@@ -275,12 +423,57 @@ function audit() {
275423
}
276424
}
277425

426+
// ── The dispatcher domains ────────────────────────────────────────────────
427+
const domains = discoverDomains();
428+
429+
for (const name of domains) {
430+
const declared = DISPATCHER_DOMAINS[name];
431+
if (!declared) {
432+
problems.push(
433+
`${DISPATCHER_DOMAIN_DIR}/${name}\n NOT DECLARED. Add it to DISPATCHER_DOMAINS in\n` +
434+
` scripts/check-route-envelope.mjs. A domain that answers only through the\n` +
435+
` \`deps.*\` helpers declares { handBuilt: 0 }; any hand-built response needs a\n` +
436+
` \`note\` saying which of the four kinds it is.`,
437+
);
438+
continue;
439+
}
440+
const got = scanDomainSource(readFileSync(join(ROOT, DISPATCHER_DOMAIN_DIR, name), 'utf8'), name);
441+
if (got.handBuilt !== declared.handBuilt) {
442+
problems.push(
443+
`${DISPATCHER_DOMAIN_DIR}/${name}\n handBuilt: found ${got.handBuilt}, declared ${declared.handBuilt}` +
444+
(got.handBuilt > declared.handBuilt
445+
? `\n A handler is assembling its own response instead of returning a \`deps.*\`\n` +
446+
` envelope. If that is deliberate (a status or header the helper cannot\n` +
447+
` express, or a body this dispatcher only relays), raise the count and say\n` +
448+
` which in \`note\`. Sites: ${got.sites.join(', ')}`
449+
: `\n Fewer than declared — if a hand-built response moved onto \`deps.*\`, lower\n` +
450+
` the count (and drop the \`ratchet\` if that was the drift it named).`) +
451+
(declared.ratchet ? `\n (ratchet for ${declared.ratchet} — the declared count pins current drift)` : ''),
452+
);
453+
}
454+
if (declared.handBuilt > 0 && !declared.note) {
455+
problems.push(
456+
`${DISPATCHER_DOMAIN_DIR}/${name}\n declares handBuilt: ${declared.handBuilt} with no \`note\`.\n` +
457+
` A hand-built response has to say why it is not a \`deps.*\` call.`,
458+
);
459+
}
460+
}
461+
462+
for (const name of Object.keys(DISPATCHER_DOMAINS)) {
463+
if (!domains.includes(name)) {
464+
problems.push(
465+
`${DISPATCHER_DOMAIN_DIR}/${name}\n declared in DISPATCHER_DOMAINS but not found — moved or deleted?`,
466+
);
467+
}
468+
}
469+
278470
if (problems.length) {
279471
console.error('✗ Route-envelope conformance (#3843)\n');
280472
for (const p of problems) console.error(' ' + p + '\n');
281473
console.error(
282-
'Every REST body must be built by the module\'s sendOk / sendError pair, in the\n' +
283-
'envelope BaseResponseSchema declares. See scripts/check-route-envelope.mjs.',
474+
'Every REST body must be built in ONE place per surface, in the envelope\n' +
475+
'BaseResponseSchema declares — a route module\'s sendOk / sendError pair, or a\n' +
476+
'dispatcher domain\'s deps.* helpers. See scripts/check-route-envelope.mjs.',
284477
);
285478
process.exit(1);
286479
}
@@ -290,7 +483,7 @@ function audit() {
290483
const ratcheted = entries.filter(([, m]) => m.ratchet);
291484
const conformant = discovered.length - exempt.length - ratcheted.length;
292485
console.log(
293-
`✓ Route-envelope conformance — ${discovered.length} module(s) audited: ` +
486+
`✓ Route-envelope conformance — ${discovered.length} route module(s) audited: ` +
294487
`${conformant} conformant, ${ratcheted.length} ratcheted, ${exempt.length} exempt`,
295488
);
296489
for (const [file, m] of ratcheted) {
@@ -299,6 +492,18 @@ function audit() {
299492
for (const [file, m] of exempt) {
300493
console.log(` – exempt: ${file}${m.exempt}`);
301494
}
495+
496+
const dEntries = Object.entries(DISPATCHER_DOMAINS);
497+
const dRatcheted = dEntries.filter(([, m]) => m.ratchet);
498+
const helperOnly = dEntries.filter(([, m]) => m.handBuilt === 0);
499+
console.log(
500+
`✓ Dispatcher domains — ${domains.length} audited: ` +
501+
`${helperOnly.length} helper-only, ${dEntries.length - helperOnly.length} with declared hand-built responses ` +
502+
`(${dRatcheted.length} ratcheted)`,
503+
);
504+
for (const [name, m] of dRatcheted) {
505+
console.log(` ⚠ ratchet ${m.ratchet}: ${DISPATCHER_DOMAIN_DIR}/${name}${m.note}`);
506+
}
302507
}
303508

304509
// ── Self-test ────────────────────────────────────────────────────────────────
@@ -357,6 +562,41 @@ function selfTest() {
357562
`ok passed as a helper's data must not count → ${JSON.stringify(r)}`,
358563
);
359564

565+
// ── Dispatcher domains ────────────────────────────────────────────────────
566+
// A domain that answers only through the helpers hand-builds nothing.
567+
let d = scanDomainSource(`
568+
if (m === 'GET') return { handled: true, response: deps.success(rows) };
569+
if (m === 'DELETE') return { handled: true, response: deps.success({ ok: true }) };
570+
return { handled: true, response: deps.routeNotFound('/x') };
571+
`);
572+
assert(d.handBuilt === 0, `helper-only domain → ${JSON.stringify(d)}`);
573+
574+
// An assembled literal is what the count is for — this is how /share-links
575+
// carried a duplicate `link` beside `data` unseen (#4038).
576+
d = scanDomainSource(`return { handled: true, response: { status: 201, body: { success: true, data: link, link } } };`);
577+
assert(d.handBuilt === 1, `hand-built response not caught → ${JSON.stringify(d)}`);
578+
579+
// Both forms in one file, counted once each.
580+
d = scanDomainSource(`
581+
if (a) return { handled: true, response: deps.success(x) };
582+
if (b) return { handled: true, response: { status: 405, headers: { Allow: 'GET' }, body } };
583+
return { handled: true, response: { status: 200, body: { agents: [] } } };
584+
`);
585+
assert(d.handBuilt === 2, `mixed domain miscounted → ${JSON.stringify(d)}`);
586+
587+
// Prose quoting either form is not code — the note fields in the table above
588+
// quote both, and must not move anyone's count.
589+
d = scanDomainSource(`
590+
/* was: response: { status: 200, body: { agents: [] } } — see #4053 */
591+
// return { handled: true, response: { status: 201, body } };
592+
return { handled: true, response: deps.success(x) };
593+
`);
594+
assert(d.handBuilt === 0, `commented-out responses counted → ${JSON.stringify(d)}`);
595+
596+
// A nested `response` key inside a payload is not a response assignment.
597+
d = scanDomainSource(`return { handled: true, response: deps.success({ response: { status: 'queued' } }) };`);
598+
assert(d.handBuilt === 0, `a data field named response must not count → ${JSON.stringify(d)}`);
599+
360600
console.log('✓ check-route-envelope self-test passed');
361601
}
362602

0 commit comments

Comments
 (0)