Skip to content

Commit b378fbe

Browse files
committed
test(runtime): move the consumer pins onto the converted refusal envelope (#9364)
packages/runtime drives the hono adapter through the dispatcher plugin, and its integration tests pin the adapter's refusal bodies. Those pins still asserted the pre-conversion shapes. Adapter pins, moved — verified as adapter pins rather than another door's contract by reading what each case exists to prove: - dispatcher-plugin.multi-tenant-endpoint (2 sites) and dispatcher-plugin.ready — both assert "the transport's own 404 stands", one as the decline branch of environment resolution, one as an explicit anti-vacuity control that the harness mirrors prod. - dispatcher-plugin.endpoint-fallback — BARE_NOT_FOUND renamed TRANSPORT_NOT_FOUND (the name described the old shape) across 6 uses, plus the 405 case whose code/allowed moved into error / error.details. - mcp-skill-method-not-allowed — the boundary case now reads error.details.allowed, asserted by value (the four-verb set #7649's own fix produces) rather than by presence. ⚠️ The conversion also drained the POST/DELETE cases in that last file of their discriminating power without failing: they separated the two doors by `typeof body.error !== 'string'` and three top-level not.toHaveProperty checks, all four of which became true of the ADAPTER answer too once it gained the same envelope. Replaced with the distinction that still holds — the domain fills error.httpStatus and no details, the adapter the reverse — so the suite still fails in the world it exists to catch. Assertions are at least as strong as before; none weakened or skipped, no ratchet touched. check-route-envelope counters unchanged. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012WKSnqAaoqtW3QX7SSf1Vk
1 parent d2ccb2f commit b378fbe

4 files changed

Lines changed: 86 additions & 26 deletions

‎packages/runtime/src/dispatcher-plugin.endpoint-fallback.integration.test.ts‎

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@
1919
*
2020
* The load-bearing assertion in most of these is a NEGATIVE one: that adding
2121
* this seam changed nothing for anybody who did not ask for it. The unmatched
22-
* answers — the bare 404 and the 405 + `Allow` — must come back byte for byte
23-
* for every path no declaration owns. Since the #5040 E7 publish flip that is
22+
* answers — the transport's 404 and the 405 + `Allow` — must come back byte for
23+
* byte for every path no declaration owns. Since the #5040 E7 publish flip that is
2424
* the assertion's whole weight: stacks CAN declare endpoints now, so "the
2525
* fallback stays silent unless a declaration matches" is a promise to live
2626
* deployments rather than a property of a surface nothing could reach.
@@ -123,8 +123,18 @@ async function shutdown(kernel: LiteKernel | undefined) {
123123
]);
124124
}
125125

126-
/** The transport's unmatched answer, captured from a boot with no seam armed. */
127-
const BARE_NOT_FOUND = { error: 'Not found' };
126+
/**
127+
* The transport's unmatched answer, captured from a boot with no seam armed.
128+
*
129+
* The hono adapter's `unmatchedResponse()` — the declared refusal envelope
130+
* since #9364, previously the bare `{ error: 'Not found' }`. Every use below
131+
* is asserting "the transport answered, the seam did not", so the value has to
132+
* track the adapter; what those cases pin is the ROUTING, not the wording.
133+
*/
134+
const TRANSPORT_NOT_FOUND = {
135+
success: false,
136+
error: { code: 'ENDPOINT_NOT_FOUND', message: 'Not found' },
137+
};
128138

129139
describe('metadata slot carries no matchEndpoint — a fully working passthrough', () => {
130140
let kernel: LiteKernel;
@@ -139,7 +149,7 @@ describe('metadata slot carries no matchEndpoint — a fully working passthrough
139149
it('answers an endpoint-shaped path with the transport\'s own 404, unchanged', async () => {
140150
const res = await fetch(`${baseUrl}/api/v1/apps/showcase/tasks`);
141151
expect(res.status).toBe(404);
142-
expect(await res.json()).toEqual(BARE_NOT_FOUND);
152+
expect(await res.json()).toEqual(TRANSPORT_NOT_FOUND);
143153
});
144154

145155
it('serves the dispatcher\'s own routes normally', async () => {
@@ -158,7 +168,7 @@ describe('no metadata service at all', () => {
158168
it('answers 404 rather than failing the request', async () => {
159169
const res = await fetch(`${baseUrl}/api/v1/apps/showcase/tasks`);
160170
expect(res.status).toBe(404);
161-
expect(await res.json()).toEqual(BARE_NOT_FOUND);
171+
expect(await res.json()).toEqual(TRANSPORT_NOT_FOUND);
162172
});
163173
});
164174

@@ -198,15 +208,15 @@ describe('matcher present — the endpoint dispatch step (#5090)', () => {
198208
it('answers a MISS under the mount with the transport\'s 404, unchanged', async () => {
199209
const res = await fetch(`${baseUrl}/api/v1/apps/showcase/nope`);
200210
expect(res.status).toBe(404);
201-
expect(await res.json()).toEqual(BARE_NOT_FOUND);
211+
expect(await res.json()).toEqual(TRANSPORT_NOT_FOUND);
202212
expect(queries).toContainEqual({ path: '/api/v1/apps/showcase/nope', method: 'GET' });
203213
});
204214

205215
it('never consults the matcher for a path outside the mount', async () => {
206216
queries.length = 0;
207217
const res = await fetch(`${baseUrl}/api/v1/nope`);
208218
expect(res.status).toBe(404);
209-
expect(await res.json()).toEqual(BARE_NOT_FOUND);
219+
expect(await res.json()).toEqual(TRANSPORT_NOT_FOUND);
210220
const notFoundOnRoot = await fetch(`${baseUrl}/totally/unrouted`);
211221
expect(notFoundOnRoot.status).toBe(404);
212222
expect(queries, 'the endpoint step ran for a path outside `/api/v1/apps/`').toEqual([]);
@@ -231,9 +241,16 @@ describe('matcher present — the endpoint dispatch step (#5090)', () => {
231241
const res = await fetch(`${baseUrl}/api/v1/apps/showcase/tasks`, { method: 'DELETE' });
232242
expect(res.status).toBe(405);
233243
expect(res.headers.get('Allow')).toBe('PUT');
234-
const body = await res.json() as { code: string; allowed: string[] };
235-
expect(body.code).toBe('METHOD_NOT_ALLOWED');
236-
expect(body.allowed).toEqual(['PUT']);
244+
// The adapter's 405, in the declared envelope since #9364 — `code` and
245+
// the `allowed` hint moved from top-level siblings into `error` and
246+
// `error.details` respectively. Same two facts, one level in.
247+
const body = await res.json() as {
248+
success: boolean;
249+
error: { code: string; details: { allowed: string[] } };
250+
};
251+
expect(body.success).toBe(false);
252+
expect(body.error.code).toBe('METHOD_NOT_ALLOWED');
253+
expect(body.error.details.allowed).toEqual(['PUT']);
237254
});
238255

239256
it('answers an execution result, never the "nothing was wired" 501', async () => {
@@ -518,11 +535,11 @@ describe('the wired chain — policies, then the real pipeline (#5129)', () => {
518535
queries.length = 0;
519536
const outside = await fetch(`${baseUrl}/api/v1/nope`);
520537
expect(outside.status).toBe(404);
521-
expect(await outside.json()).toEqual(BARE_NOT_FOUND);
538+
expect(await outside.json()).toEqual(TRANSPORT_NOT_FOUND);
522539

523540
const missUnderMount = await fetch(`${baseUrl}/api/v1/apps/showcase/not-declared`);
524541
expect(missUnderMount.status).toBe(404);
525-
expect(await missUnderMount.json()).toEqual(BARE_NOT_FOUND);
542+
expect(await missUnderMount.json()).toEqual(TRANSPORT_NOT_FOUND);
526543

527544
expect(queries).toEqual([{ path: '/api/v1/apps/showcase/not-declared', method: 'GET' }]);
528545
});

‎packages/runtime/src/dispatcher-plugin.multi-tenant-endpoint.integration.test.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,11 +256,14 @@ describe('#5040 E5b — multi-tenant resolution finds no environment: decline +
256256
// single-environment host that never had a resolver.
257257
expect(resolverCalls).toEqual([undefined]);
258258

259-
// Declining writes nothing, so the bare Hono 404 stands. The host
259+
// Declining writes nothing, so the transport's own 404 stands. The host
260260
// DECLARES this path and could have served it: without the branch this
261261
// is a 200 carrying `servedBy: 'host'`.
262262
expect(res.status).toBe(404);
263-
expect(await res.json()).toEqual({ error: 'Not found' });
263+
expect(await res.json()).toEqual({
264+
success: false,
265+
error: { code: 'ENDPOINT_NOT_FOUND', message: 'Not found' },
266+
});
264267

265268
// The decline happened BEFORE the probe. A declaration must never be
266269
// consulted for a request that was not placed in an environment.
@@ -284,7 +287,10 @@ describe('#5040 E5b — multi-tenant resolution finds no environment: decline +
284287

285288
expect(resolverCalls).toEqual(['env-does-not-exist']);
286289
expect(res.status).toBe(404);
287-
expect(await res.json()).toEqual({ error: 'Not found' });
290+
expect(await res.json()).toEqual({
291+
success: false,
292+
error: { code: 'ENDPOINT_NOT_FOUND', message: 'Not found' },
293+
});
288294
expect(matchQueries).toEqual([]);
289295
}, 60_000);
290296
});

‎packages/runtime/src/dispatcher-plugin.ready.integration.test.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,17 @@ describe('GET /ready over a real HTTP server (integration)', () => {
5959
expect(body.data.status).toBe('ok');
6060
});
6161

62-
it('proves the harness mirrors prod: an unmounted path 404s with the Hono not-found body', async () => {
62+
it('proves the harness mirrors prod: an unmounted path 404s with the adapter not-found body', async () => {
6363
// This is the exact response /ready produced BEFORE the fix. Asserting it
6464
// here shows the test would have failed against the old code (the /ready
6565
// assertion above would have returned this body), not passed vacuously.
6666
const res = await fetch(`${baseUrl}/api/v1/this-route-does-not-exist`);
6767
expect(res.status).toBe(404);
6868
const body = await res.json();
69-
expect(body).toEqual({ error: 'Not found' });
69+
expect(body).toEqual({
70+
success: false,
71+
error: { code: 'ENDPOINT_NOT_FOUND', message: 'Not found' },
72+
});
7073
});
7174

7275
it('returns 503 while the kernel is shutting down (drain signal)', async () => {

‎packages/runtime/src/mcp-skill-method-not-allowed.hono.integration.test.ts‎

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,15 @@ import { createDispatcherPlugin } from './dispatcher-plugin.js';
2525
* `{success:false, error:{code, message, httpStatus}}` carrying the documented
2626
* message "Method not allowed — use GET".
2727
*
28+
* ⚠️ The quoted body above is the shape MEASURED IN #7627, and the adapter no
29+
* longer emits it: #9364 converted `unmatchedResponse()` onto the same declared
30+
* envelope, nesting `method`/`path`/`allowed` under `error.details`. The defect
31+
* this file guards is unchanged — the wrong DOOR answering — but the two doors
32+
* are no longer told apart by envelope shape, only by which optional member
33+
* each fills (`httpStatus` for the domain, `details` for the adapter). See the
34+
* note on the negative case below; several assertions here had to move for
35+
* exactly that reason, and would otherwise have gone quietly vacuous.
36+
*
2837
* ## Why the defect was invisible to the existing tests
2938
*
3039
* The 405 branch is NOT missing. `handleMcpSkillRequest` has had one since
@@ -146,13 +155,23 @@ describe('POST /api/v1/mcp/skill answers the standard 405 envelope (integration,
146155
it('POST does not answer with `unmatchedResponse()`\'s shape', async () => {
147156
const { body } = await call('POST');
148157

149-
// The four keys that identify the adapter's unmatched-route answer.
150-
// `error` as a STRING is the tell — the standard envelope nests an
151-
// object there, so this assertion cannot be satisfied by both shapes.
152-
expect(typeof body.error).not.toBe('string');
153-
expect(body).not.toHaveProperty('method');
154-
expect(body).not.toHaveProperty('path');
155-
expect(body).not.toHaveProperty('allowed');
158+
// ⚠️ The tell CHANGED with #9364, and the old one would now pass
159+
// vacuously. Before that card the adapter's 405 answered
160+
// `error: 'Method Not Allowed'` (a STRING) with `method`/`path`/
161+
// `allowed` as TOP-LEVEL siblings, so `typeof body.error !== 'string'`
162+
// and three `not.toHaveProperty` checks genuinely separated the two
163+
// doors. #9364 put the adapter's refusal into the same declared
164+
// envelope, nesting its context under `error.details` — so all four of
165+
// those assertions are now true of the ADAPTER answer as well, and
166+
// would keep this test green in exactly the world it exists to catch.
167+
//
168+
// What still separates the doors is which OPTIONAL member each fills:
169+
// the dispatcher's `buildApiError` carries `httpStatus` and no
170+
// `details`; the adapter's `unmatchedResponse` carries
171+
// `details.{method,path,allowed}` and no `httpStatus`. Both halves are
172+
// asserted here so this case discriminates on its own.
173+
expect(body.error).not.toHaveProperty('details');
174+
expect(body.error.httpStatus).toBe(405);
156175
});
157176

158177
// The `Allow` header CHANGES with this fix, which is worth stating exactly
@@ -201,6 +220,21 @@ describe('POST /api/v1/mcp/skill answers the standard 405 envelope (integration,
201220
it('PUT — unmounted — still falls through to the adapter (boundary, not a regression)', async () => {
202221
const { res, body } = await call('PUT');
203222
expect(res.status).toBe(405);
204-
expect(body).toHaveProperty('allowed');
223+
224+
// The adapter's own 405. Since #9364 it speaks the SAME declared
225+
// envelope as the domain branch, so `success`/`error.code` no longer
226+
// tell the two apart — `error.details.allowed` is what does, and it is
227+
// the direct successor of the top-level `allowed` this case used to
228+
// read. Asserted by VALUE, not by presence — and the value is the
229+
// whole four-verb set because #7649's fix mounts GET/POST/DELETE, with
230+
// Hono registering HEAD implicitly alongside GET. That is precisely
231+
// why PUT is the boundary probe: it is the one verb left unmounted.
232+
expect(body.error.code).toBe('METHOD_NOT_ALLOWED');
233+
expect(body.error.details.allowed).toEqual(['DELETE', 'GET', 'HEAD', 'POST']);
234+
expect(body.error.details.method).toBe('PUT');
235+
expect(body.error.details.path).toBe(SKILL_PATH);
236+
// …and it is NOT the domain branch's answer, which fills `httpStatus`
237+
// and no `details`.
238+
expect(body.error).not.toHaveProperty('httpStatus');
205239
});
206240
});

0 commit comments

Comments
 (0)