Skip to content

Commit fc0a783

Browse files
claude[bot]claude
andauthored
fix(hono): hand a dispatcher result that is already a Response to the caller intact (#16680)
`HttpDispatcherResult.result` is declared for direct response objects ("For flexible return types or direct response objects (Response/NextResponse)") and the runtime really puts one there — `runtime/src/domains/auth.ts` returns `{ handled: true, result: response }` with whatever the auth service answered. The adapter's `toResponse` had no arm for that. It tests `result.type` for the `redirect` and `stream` descriptors, a `Response` spells neither, and the fall-through was `c.json(res, 200)`: the real status replaced by a literal 200 and the real body by `JSON.stringify` of a `Response`, which is `{}` because it has no own enumerable properties. Measured on a real boot through this adapter (a real kernel, the real dispatcher, prefix `/api/v1`), an auth service answering an honest 404 on a path it does not serve: GET /api/v1/auth/me/permissions the door answered : 404 {"message":"Not found","code":"NOT_FOUND"} the caller read : 200 {} A discarded status is not a missing answer, it is a wrong one that reads as success, and it defeats fail-closed guards rather than missing them: objectui's `MePermissionsProvider.tsx` refuses on `if (!data) return false`, and `{}` is truthy. The check is `instanceof Response` and nothing else — the descriptor arms, the plain-object rendering after them and the separate `response` arm are unchanged. Two pins, deliberately not one. `@objectstack/hono` has no in-repo consumer, and its own suite aliases `@objectstack/runtime` to a stub, so it cannot reach the real dispatcher: the adapter-local file drives the arm over every status and body shape against that stub, and a new conformance file in `packages/qa/http-conformance` boots the real stack — a real `LiteKernel`, the real `HttpDispatcher`, the real `/auth` domain — and reads the answer off the wire. That package now carries `@objectstack/hono` as a devDependency with an anchored source alias, so its verdict is about this checkout and not about a build artifact. Claude-Session: https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ Co-authored-by: Claude <noreply@anthropic.com>
1 parent fc015bc commit fc0a783

7 files changed

Lines changed: 448 additions & 0 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
"@objectstack/hono": patch
3+
---
4+
5+
`createHonoApp` no longer discards the status and body of a dispatcher result that is already a `Response` — it hands the object on unchanged.
6+
7+
`HttpDispatcherResult.result` is declared for direct response objects ("For flexible return types or direct response objects (Response/NextResponse)"), and the runtime really puts one there: the `/auth` domain returns whatever the auth service answered as `{ handled: true, result: response }`. The adapter's `toResponse` had no arm for that. It tested `result.type` for the `redirect` and `stream` descriptors, a `Response` spells neither, and the fall-through was `c.json(res, 200)` — so the real status was replaced by a literal `200` and the real body by `JSON.stringify` of a `Response`, which is `{}` because a `Response` has no own enumerable properties.
8+
9+
Measured on a real boot through this adapter (a real kernel, the real dispatcher, `prefix: '/api/v1'`), an auth service answering an honest 404 on a path it does not serve:
10+
11+
```
12+
GET /api/v1/auth/me/permissions
13+
the door answered : 404 {"message":"Not found","code":"NOT_FOUND"}
14+
the caller read : 200 {}
15+
```
16+
17+
A discarded status is not a missing answer, it is a wrong one that reads as success: `res.ok`, `status === 200` and "nothing threw" all report a refusal, a 404 or a 500 as a completed operation, and a fail-closed guard written as `if (!data) return false` does not fire on `{}` because `{}` is truthy. Callers embedding this adapter now see the status and the body the door actually produced, along with its headers, and a non-JSON body arrives byte-identical instead of being re-serialized.
18+
19+
The check is `instanceof Response` and nothing else: the `redirect` and `stream` descriptor arms, the plain-object rendering after them, and the separate `response` arm all behave exactly as before.
Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,214 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#16383] `toResponse` returns a `HttpDispatcherResult.result` that IS a
5+
* `Response` unchanged — its real status, its real body, its real headers.
6+
*
7+
* ## The defect
8+
*
9+
* `HttpDispatcherResult.result` is DECLARED for direct response objects
10+
* (`packages/runtime/src/http-dispatcher.ts`: "For flexible return types or
11+
* direct response objects (Response/NextResponse)"), and the runtime really
12+
* puts one there — `runtime/src/domains/auth.ts` hands back whatever the auth
13+
* service answered as `{ handled: true, result: response }`.
14+
*
15+
* `toResponse` had no arm for that. It tested `result.type === 'redirect'` and
16+
* `result.type === 'stream'`, and everything else fell into `c.json(res, 200)`.
17+
* A Fetch `Response` has no own enumerable properties, so `JSON.stringify` of
18+
* one is `{}`, and the `200` was a literal:
19+
*
20+
* door answers 404 {"message":"Not found","code":"NOT_FOUND"}
21+
* caller reads 200 {}
22+
*
23+
* ⭐ The failure direction is what makes this a p1 rather than a cosmetic loss.
24+
* A discarded status is not a missing answer, it is a WRONG answer that reads
25+
* as success — `res.ok`, `status === 200` and "nothing threw" all report a
26+
* refusal as a completed operation — and it DEFEATS fail-closed guards instead
27+
* of merely missing them: objectui's `MePermissionsProvider.tsx` refuses on
28+
* `if (!data) return false`, and `{}` is truthy.
29+
*
30+
* ⇒ Every case below asserts the real status AND the real body. A pin that
31+
* asserted only "not 200" would stay green on a repair that answered some other
32+
* wrong status with the body still destroyed.
33+
*
34+
* ## What this file is, and what its sibling is
35+
*
36+
* This package's vitest config aliases `@objectstack/runtime` to a stub, so the
37+
* dispatcher here is a fixture — which is exactly what lets these cases drive
38+
* `toResponse`'s `result` arm over statuses and body shapes the real
39+
* composition cannot reach on demand. The other half is a REAL boot, in
40+
* `packages/qa/http-conformance/src/hono-dispatcher-result-response.conformance.test.ts`:
41+
* a real `LiteKernel`, the real `HttpDispatcher`, the real `/auth` domain, one
42+
* wire reading. `@objectstack/hono` has no in-repo consumer (#4117), so that
43+
* boot is the only thing there is to observe this through; neither file
44+
* replaces the other.
45+
*
46+
* ⛔ Not this card, deliberately untouched: which paths the dispatcher CLAIMS
47+
* (#16026), WHERE auth is mounted (#16025), and the escaped ADR-0112 envelope
48+
* on the same function's error exit (#16545).
49+
*/
50+
51+
import { describe, it, expect, vi, beforeEach } from 'vitest';
52+
import type { Hono } from 'hono';
53+
54+
const mockDispatcher = {
55+
getDiscoveryInfo: vi.fn().mockReturnValue({ version: '1.0', routes: {} }),
56+
handleAuth: vi.fn(),
57+
dispatch: vi.fn(),
58+
};
59+
60+
vi.mock('@objectstack/runtime', () => ({
61+
HttpDispatcher: function HttpDispatcher() { return mockDispatcher; },
62+
}));
63+
64+
import { createHonoApp } from './index';
65+
66+
const PREFIX = '/api/v1';
67+
/** A path no explicit mount claims, so it lands on the `${prefix}/*` catch-all. */
68+
const PATH = `${PREFIX}/data/thing`;
69+
70+
const kernel = { name: 'test-kernel' } as any;
71+
const bootApp = (): Hono => createHonoApp({ kernel, prefix: PREFIX });
72+
73+
const jsonResponse = (status: number, body: unknown, headers: Record<string, string> = {}) =>
74+
new Response(JSON.stringify(body), {
75+
status,
76+
headers: { 'Content-Type': 'application/json', ...headers },
77+
});
78+
79+
describe('#16383: toResponse passes a `result` that is already a Response through', () => {
80+
beforeEach(() => {
81+
vi.clearAllMocks();
82+
mockDispatcher.handleAuth.mockResolvedValue({ handled: false });
83+
});
84+
85+
// The statuses a door really produces. 200 is carried too: a repair that
86+
// special-cased "non-200" would leave the success path rebuilt and its body
87+
// re-serialized, which is the same defect wearing the other sign.
88+
it.each([200, 201, 302, 400, 401, 403, 404, 409, 422, 500, 503])(
89+
'a %i Response reaches the caller with that status and its own body',
90+
async (status) => {
91+
const body = { message: `answer-${status}`, code: 'DOOR_SAID_SO' };
92+
mockDispatcher.dispatch.mockResolvedValue({
93+
handled: true,
94+
result: jsonResponse(status, body, { 'X-Door': 'dispatcher' }),
95+
});
96+
97+
const res = await bootApp().request(`http://localhost${PATH}`, { redirect: 'manual' });
98+
99+
expect(res.status).toBe(status);
100+
// ⭐ The body half. `{}` is what the defect produced, and it is TRUTHY —
101+
// asserting the status alone would pass on a door that still destroys it.
102+
await expect(res.clone().json()).resolves.toEqual(body);
103+
await expect(res.clone().text()).resolves.not.toBe('{}');
104+
expect(res.headers.get('x-door')).toBe('dispatcher');
105+
},
106+
);
107+
108+
it('does not re-serialize — a non-JSON body arrives byte-identical', async () => {
109+
// `c.json(res, 200)` could not have produced this at all: the body is not
110+
// JSON and its content-type is not `application/json`. A repair that
111+
// rebuilt the Response from a parsed body would corrupt both.
112+
const payload = 'id,name\n1,ada\n';
113+
mockDispatcher.dispatch.mockResolvedValue({
114+
handled: true,
115+
result: new Response(payload, {
116+
status: 418,
117+
headers: { 'Content-Type': 'text/csv; charset=utf-8' },
118+
}),
119+
});
120+
121+
const res = await bootApp().request(`http://localhost${PATH}`);
122+
123+
expect(res.status).toBe(418);
124+
expect(res.headers.get('content-type')).toBe('text/csv; charset=utf-8');
125+
await expect(res.text()).resolves.toBe(payload);
126+
});
127+
128+
it('a bodyless refusal stays bodyless — no `{}` is invented for it', async () => {
129+
// better-call answers an unrouted path exactly this way, and it is the
130+
// shape `hono-auth-owned-404.test.ts` calls `unrouted404`.
131+
mockDispatcher.dispatch.mockResolvedValue({
132+
handled: true,
133+
result: new Response(null, { status: 404, statusText: 'Not Found' }),
134+
});
135+
136+
const res = await bootApp().request(`http://localhost${PATH}`);
137+
138+
expect(res.status).toBe(404);
139+
await expect(res.text()).resolves.toBe('');
140+
});
141+
142+
it('the auth mount\'s dispatcher fallback passes one through too', async () => {
143+
// The second door into `toResponse`: `${prefix}/auth/*` with no auth
144+
// service on the kernel falls back to `dispatcher.handleAuth`, and
145+
// `runtime/src/domains/auth.ts` is the very producer that puts a `Response`
146+
// in `result`. Both callers must render it the same way.
147+
mockDispatcher.handleAuth.mockResolvedValue({
148+
handled: true,
149+
result: jsonResponse(401, { message: 'Unauthorized', code: 'UNAUTHENTICATED' }),
150+
});
151+
152+
const res = await bootApp().request(`http://localhost${PREFIX}/auth/get-session`);
153+
154+
expect(res.status).toBe(401);
155+
await expect(res.json()).resolves.toEqual({ message: 'Unauthorized', code: 'UNAUTHENTICATED' });
156+
});
157+
158+
describe('⛔ the arms either side of it are untouched', () => {
159+
it('a plain object result is still rendered as JSON with 200', async () => {
160+
// The narrowness control. This is `hono.test.ts`'s "generic result
161+
// objects with 200 status" case, restated here so a future widening of
162+
// the passthrough (`typeof res === 'object'`, say) fails in THIS file,
163+
// next to the reason it must not.
164+
mockDispatcher.dispatch.mockResolvedValue({ handled: true, result: { foo: 'bar' } });
165+
166+
const res = await bootApp().request(`http://localhost${PATH}`);
167+
168+
expect(res.status).toBe(200);
169+
await expect(res.json()).resolves.toEqual({ foo: 'bar' });
170+
});
171+
172+
it('a redirect descriptor still redirects', async () => {
173+
mockDispatcher.dispatch.mockResolvedValue({
174+
handled: true,
175+
result: { type: 'redirect', url: 'https://example.com' },
176+
});
177+
178+
const res = await bootApp().request(`http://localhost${PATH}`, { redirect: 'manual' });
179+
180+
expect(res.status).toBe(302);
181+
expect(res.headers.get('location')).toBe('https://example.com');
182+
});
183+
184+
it('a stream descriptor still streams', async () => {
185+
mockDispatcher.dispatch.mockResolvedValue({
186+
handled: true,
187+
result: {
188+
type: 'stream',
189+
events: (async function* () { yield { tick: 1 }; })(),
190+
contentType: 'text/event-stream',
191+
},
192+
});
193+
194+
const res = await bootApp().request(`http://localhost${PATH}`);
195+
196+
expect(res.status).toBe(200);
197+
expect(res.headers.get('content-type')).toContain('text/event-stream');
198+
await expect(res.text()).resolves.toContain('data: {"tick":1}');
199+
});
200+
201+
it('the `response` arm — status + body + headers — is unchanged', async () => {
202+
mockDispatcher.dispatch.mockResolvedValue({
203+
handled: true,
204+
response: { status: 201, body: { id: 1 }, headers: { 'X-Custom': 'yes' } },
205+
});
206+
207+
const res = await bootApp().request(`http://localhost${PATH}`);
208+
209+
expect(res.status).toBe(201);
210+
expect(res.headers.get('x-custom')).toBe('yes');
211+
await expect(res.json()).resolves.toEqual({ id: 1 });
212+
});
213+
});
214+
});

packages/adapters/hono/src/index.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,46 @@ export function createHonoApp(options: ObjectStackHonoOptions): Hono {
258258
}
259259
if (result.result) {
260260
const res = result.result;
261+
/**
262+
* [#16383] A `result` that IS a `Response` is the answer — hand it on.
263+
*
264+
* `HttpDispatcherResult.result` is DECLARED for exactly this ("For
265+
* flexible return types or direct response objects
266+
* (Response/NextResponse)"), and the runtime really puts one there:
267+
* `runtime/src/domains/auth.ts` returns `{ handled: true, result:
268+
* response }` with whatever the auth service answered.
269+
*
270+
* This function had no arm for it. The two below test `res.type`, a
271+
* `Response` never spells `'redirect'` or `'stream'` there, and the
272+
* fall-through was `c.json(res, 200)` — so the real status was replaced
273+
* by a literal `200` and the real body by `JSON.stringify` of a
274+
* `Response`, which is `{}` because it has no own enumerable
275+
* properties. Measured on a real boot through this adapter (a real
276+
* kernel, the real dispatcher, `prefix: '/api/v1'`), an auth service
277+
* answering an honest 404:
278+
*
279+
* GET /api/v1/auth/me/permissions
280+
* the door answered : 404 {"message":"Not found","code":"NOT_FOUND"}
281+
* the caller read : 200 {} <- manufactured here
282+
*
283+
* ⭐ That is not a missing answer, it is a WRONG one that reads as
284+
* success, and it defeats fail-closed guards rather than missing them:
285+
* objectui's `MePermissionsProvider.tsx` refuses on `if (!data) return
286+
* false`, and `{}` is truthy. `res.ok`, `status === 200` and "nothing
287+
* threw" all report a refusal as a completed operation.
288+
*
289+
* ⛔ Narrow on purpose — `instanceof Response`, not "looks like one".
290+
* The arms below and the plain-object rendering after them are other
291+
* producers' contracts and are unchanged; `hono.test.ts` and
292+
* `hono-result-response-passthrough.test.ts` pin both sides of that
293+
* line. Returning the object itself rather than rebuilding it is what
294+
* keeps the body byte-identical (a CSV, an empty 404) and the
295+
* producer's headers attached; the `stream` arms below already return a
296+
* `Response` this way.
297+
*/
298+
if (res instanceof Response) {
299+
return res;
300+
}
261301
if (res.type === 'redirect' && res.url) {
262302
return c.redirect(res.url);
263303
}

packages/qa/http-conformance/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
},
1515
"devDependencies": {
1616
"@objectstack/driver-sqlite-wasm": "workspace:*",
17+
"@objectstack/hono": "workspace:*",
1718
"@objectstack/objectql": "workspace:*",
1819
"@objectstack/plugin-hono-server": "workspace:*",
1920
"@objectstack/runtime": "workspace:*",

0 commit comments

Comments
 (0)