fix(hono): hand a dispatcher result that is already a Response to the caller intact - #16680
Conversation
… caller intact
`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.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ
📓 Docs Drift CheckThis PR changes 2 package(s): 1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
What this run could not see
Coarse fallback — 1 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 4c118fd185cd21cb829d297d95a3f930c6eb5a68 && git checkout 4c118fd185cd21cb829d297d95a3f930c6eb5a68
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin ecf44b1264657f01d533210e6e71cb03b0ed0a4b 41ecd20f0ed7f949752596fd94df6666cfacd8e9 && git checkout -B drift-repro ecf44b1264657f01d533210e6e71cb03b0ed0a4b && git merge --no-ff 41ecd20f0ed7f949752596fd94df6666cfacd8e9
node scripts/docs-audit/affected-docs.mjs --json ecf44b1264657f01d533210e6e71cb03b0ed0a4b
|
Fixes #16383
toResponsereturns a dispatcher result that is already aResponseas itself — real status, real body, real headers. Thec.json(res, 200)rendering of that arm is gone.The defect, re-derived rather than inherited
HttpDispatcherResult.resultis declared for direct response objects —packages/runtime/src/http-dispatcher.ts: "For flexible return types or direct response objects (Response/NextResponse)" — and the runtime really puts one there:packages/runtime/src/domains/auth.tsforwards whatever the auth service answered as{ handled: true, result: response }.toResponsehad no arm for that. It testsresult.typefor theredirectandstreamdescriptors, a FetchResponsespells neither, and the fall-through wasc.json(res, 200): the real status replaced by a literal200, the real body byJSON.stringifyof aResponse—{}, because aResponsehas no own enumerable properties — and the producer's headers dropped.Anchor, re-derived at the moment of work, not taken from the card. On this branch's base (
813d6c5b3d) the rendering waspackages/adapters/hono/src/index.ts:297. The card carried no line number for it, so nothing had rotted there — but the rot is real and was measured on this very card: in PR #16380's tree the same statement sits at :470, and after this change it is at:337with the new guard at:298. Anchors in this file move by hundreds of lines within a day.The failure direction the pin is aimed at
A discarded status is not a missing answer — it is a wrong answer that reads as success.
res.ok,status === 200and "nothing threw" all report a refusal, a 404 or a 500 as a completed operation. Triage measured it defeating a real guard rather than merely missing one: objectui'sMePermissionsProvider.tsxrefuses onif (!data) return false, and{}is truthy — so200 {}punctures that fail-closed check.⇒ Every case added here asserts the real status and the real body. A pin that asserted only "not 200" would stay green on a repair that answered some other wrong status with the body still destroyed.
Reproduction, on a constructed real boot
@objectstack/honohas no in-repo consumer (#4117) — measured, not assumed: the only code importer of@objectstack/honoanywhere in this repo is the new conformance file added by this PR (packages/clientcarries the devDependency with zero importers). There is nothing to observe this through except a constructed boot, so this PR builds one and leaves it in the tree.A real
LiteKernelcarrying anauthservice, the realHttpDispatcherthe adapter constructs for itself,createHonoApp({ kernel, prefix: '/api/v1' }), requests injected through the returned app. Before the fix:The routing was measured rather than assumed, over four statuses × four paths. Only a 404 reaches this arm: the
${prefix}/auth/*mount answers 200 / 403 / 500 itself from its ownforwarded(), and only a 404 the auth service disclaims is yielded to the${prefix}/*catch-all, wheredispatch()→ the/authdomain →result. That is why the conformance cases assert the auth service'shandleRequestcall count beside every status:2means the arm under test ran,1means the mount answered andtoResponsewas never consulted.Two pins, deliberately not one
packages/adapters/hono/src/hono-result-response-passthrough.test.ts— this package aliases@objectstack/runtimeto a stub, which is exactly what lets these cases drive the arm over 11 statuses, a non-JSON body, a bodyless 404 and both doors intotoResponse. It also pins the arms either side (plain object → JSON 200,redirect,stream, theresponsearm) so a future widening of the passthrough fails next to the reason it must not.packages/qa/http-conformance/src/hono-dispatcher-result-response.conformance.test.ts— the real boot above. That package is the repo's cross-adapter conformance instrument and already boots real kernels; it gains@objectstack/honoas a devDependency and an anchored source alias, so its verdict is about this checkout and not about a build artifact.Ablation — the pin's RED direction, demonstrated
Implementation committed first, then the guard removed so
c.json(res, 200)is reachable again. Mutation proven on disk, never by an exit code:Both suites went RED with no rebuild between the legs, which is the measurement that the subject really resolves to source in both (relative
./indexin one, the anchored alias in the other) rather than to adist/that would have kept the ablation green. The narrowness controls stayed GREEN in both files under the mutation — 86 of 100 in the adapter, and the 403 one-call CONTROL row in the conformance file — so the ablation is specific to this arm.#16380 (card #16025) — it does not fix this, driven not assumed
PR #16380's head (
d89479dd29) was checked out over the adapter in this worktree, under a restore trap, and the conformance harness re-run against it:200where the door answered404, withhandleRequestcall count2confirming the arm ran. Its tree containsc.json(res, 200)at:470and noinstanceof Responseanywhere. That PR changes where auth is mounted; the rendering underneath is untouched.The two also merge cleanly —
git merge-treeagainst #16380's head is conflict-free, and the resulting tree carries bothgetBasePathand this guard.#15417 — a lead that got stronger, ⛔ still not a conclusion
#15417's Step 1 asks to "reproduce on a framework-side boot to confirm the mounting rather than the cloud composition is what decides it." Measured here on that boot, after this fix, with an auth service answering better-auth's honest 404:
All four are
200 {}on the pre-fix adapter. So the framework-side boot does manufacture #15417's shape, and this change removes it there. ⛔ What is not established is that #15417's cloud control-plane composition routes through@objectstack/honoat all — that is a cloud-repo question this seat did not measure. Recorded for #15417's owner (os-warren,pm:on-hold); ⛔ nothing here is filed against it and its second half (which admin endpoints actually resolve) is untouched.Scope
One branch of one function, plus its two pins and the wiring the real boot needs. ⛔ Untouched, by instruction: #16026 (which paths the dispatcher claims — landed via #16265), #16025 / #16380 (where auth is mounted), #16545 (the escaped ADR-0112 envelope — same function, different property).
Clause ② re-derived from the delivered diff:
no. No file underpackages/spec/src/**is touched, no exported type moves. A caller observes200 {}→ the door's real status, which looks like an accept/reject change, but no declared face moves:HttpDispatcherResult.resultis declared to hold direct response objects and this layer failed to honour that declaration. Making a layer honour a contract it already breaks is a defect repair.Dedupe. The card declared its own dedupe incomplete (no control could be built sharing the failing query's vocabulary). Nothing encountered while working names this mechanism: the only prior art found is
runtime/src/domains/auth-claim-segment-boundary.test.ts, whose "⛔ Not covered here" section explicitly hands this rendering off to this card.验收备注
Responsereturned from the handler still getsaccess-control-allow-origin,-credentialsand-expose-headersfrom thecors()middleware, and keeps the producer's own headers — measured withOrigin: https://app.exampleon the real boot. Thestreamarms already returned rawResponseobjects this way; this arm is not a new shape for the middleware chain.packages/clientdeclares@objectstack/honoas a devDependency with no importer anywhere in the package — a dead manifest edge that putsclient,client-react,cli,dogfood,downstream-contractand four examples intoturbo ls --affectedfor any change to this adapter. Noted, not filed (an observation, not a reproducible defect).packages/qa/http-conformance'stsconfig.jsonexcludes**/*.test.ts, sopnpm --filter @objectstack/http-conformance typecheckdoes not cover the new conformance file. That exclusion is a recorded, ledgered condition (check-type-check-coverage.mjs's@objectstack/http-conformanceentry, whose note says it cannot graduate by fixing code); the test layer is measured bycheck:type-check-debtinstead. Pre-existing, unchanged by this PR.@objectstack/honodeclares notypecheckscript at all, sopnpm --filter @objectstack/hono typecheckmatches zero scripts and exits 0 — a false green if anyone reads it as coverage. It carries a ledger entry instead;check:type-check-coverageis green here.Verification
Every reading below was taken on head
41ecd20f0e, which is this branch's final commit — nothing has moved since. Exit codes captured before any pipe throughout.pnpm --filter @objectstack/hono run test— 100 passed (100), 4 files. (86/100 before the fix, with the 14 new cases red.)pnpm --filter @objectstack/http-conformance run test— 89 passed (89), 6 files.pnpm exec eslint . --no-inline-config --format json— the whole population rather than a narrowing: 6312 files, 0 errors, 0 warnings.Gate union from
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, reconciled with--ran: 65 derived, 65 run, 0 NOT-MEASURED, 0 UNRUN — all 65 green.Two of them first came back exit 3 — PREREQUISITE NOT MET, which is NOT a pass, and both were made measurable rather than reported as a 3:
check:dual-build-cjs-loadsreads built output and no package haddist/. Ranpnpm exec turbo run build --filter='./packages/*' --filter='./packages/*/*'— exactly whatlint.ymldoes before it — then re-ran: exit 0, 104 published require entry points across 67 packages load, 620 emitted CJS files parse.check:type-check-debtthen OOM'd, and the cause was mine: its own header says the ceiling it runs tsc under is the caller'sNODE_OPTIONS, and I had capped it at 4096.lint.ymlpins this step at--max-old-space-size=6144(paired withCI_TSC_HEAP_CEILING_MBin the script). Re-ran at exactly that: exit 0 — 5 ledger entries re-measured in 108.3s, 55 raw tsc errors, none above its recorded number, surplus none.Also run, though this derivation scores them
silentbecause their rosters are lists of files that already exist:check:authz-resolver,check:error-code-casing,check:filter-alias-parity,check:auth-mount-ledger,check:route-envelope— all green.Ablation, the
#16380interaction and the#15417lead as recorded above.check:type-check-debtre-measure were taken unlocked, which is the class the lock's own--statustext putscheck:*gates, installs and dev servers in. The whole-repo build was not narrowed — it waited for the lock and got it.⛔ Draft on purpose. Do not merge, do not arm auto-merge, do not un-draft — landing is the PM's act.
🤖 Generated with Claude Code
https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ
Generated by Claude Code