Skip to content

fix(hono): hand a dispatcher result that is already a Response to the caller intact - #16680

Merged
os-sales merged 2 commits into
mainfrom
claude/issue-16383-toresponse-preserve-response
Sep 7, 2026
Merged

fix(hono): hand a dispatcher result that is already a Response to the caller intact#16680
os-sales merged 2 commits into
mainfrom
claude/issue-16383-toresponse-preserve-response

Conversation

@claude

@claude claude Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Fixes #16383

toResponse returns a dispatcher result that is already a Response as itself — real status, real body, real headers. The c.json(res, 200) rendering of that arm is gone.

The defect, re-derived rather than inherited

HttpDispatcherResult.result is 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.ts forwards whatever the auth service answered as { handled: true, result: response }.

toResponse had no arm for that. It tests result.type for the redirect and stream descriptors, a Fetch Response spells neither, and the fall-through was c.json(res, 200): the real status replaced by a literal 200, the real body by JSON.stringify of a Response{}, because a Response has 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 was packages/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 :337 with 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 === 200 and "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's MePermissionsProvider.tsx refuses on if (!data) return false, and {} is truthy — so 200 {} 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/hono has no in-repo consumer (#4117) — measured, not assumed: the only code importer of @objectstack/hono anywhere in this repo is the new conformance file added by this PR (packages/client carries 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 LiteKernel carrying an auth service, the real HttpDispatcher the adapter constructs for itself, createHonoApp({ kernel, prefix: '/api/v1' }), requests injected through the returned app. Before the fix:

GET /api/v1/auth/me/permissions      (auth service answers 404 + a real body)
  the door answered : 404 {"message":"Not found","code":"NOT_FOUND","hint":"no such endpoint"}
  the caller read   : 200 {}      X-Door header gone

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 own forwarded(), and only a 404 the auth service disclaims is yielded to the ${prefix}/* catch-all, where dispatch() → the /auth domain → result. That is why the conformance cases assert the auth service's handleRequest call count beside every status: 2 means the arm under test ran, 1 means the mount answered and toResponse was never consulted.

Two pins, deliberately not one

  • packages/adapters/hono/src/hono-result-response-passthrough.test.ts — this package aliases @objectstack/runtime to 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 into toResponse. It also pins the arms either side (plain object → JSON 200, redirect, stream, the response arm) 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/hono as 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:

LEG 0  guard marker `if (res instanceof Response)` : 1   blob 558a001549d930737db2ad1a1f475d76f9bb4519
LEG 1  guard marker                                : 0   blob 0b6d795ef85e06adb6790188cf07555d1211e578
       git diff HEAD --stat -> 1 file changed, 3 deletions(-)

ablated  @objectstack/hono            : exit 1   14 failed | 86 passed (100)
ablated  the conformance file          : exit 1    2 failed |  1 passed (3)
restore  on-disk blob == HEAD blob, `git diff HEAD` empty, guard marker back to 1

Both suites went RED with no rebuild between the legs, which is the measurement that the subject really resolves to source in both (relative ./index in one, the anchored alias in the other) rather than to a dist/ 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: 200 where the door answered 404, with handleRequest call count 2 confirming the arm ran. Its tree contains c.json(res, 200) at :470 and no instanceof Response anywhere. That PR changes where auth is mounted; the rendering underneath is untouched.

The two also merge cleanly — git merge-tree against #16380's head is conflict-free, and the resulting tree carries both getBasePath and 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:

POST /api/v1/auth/admin/definitely-not-a-route-1989 -> 404 {"message":"Not found","code":"NOT_FOUND"}
GET  /api/v1/auth/admin/list-users                  -> 404  (same)
POST /api/v1/auth/admin/update-user                 -> 404  (same)
POST /api/v1/auth/admin/set-role                    -> 404  (same)

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/hono at 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 under packages/spec/src/** is touched, no exported type moves. A caller observes 200 {} → the door's real status, which looks like an accept/reject change, but no declared face moves: HttpDispatcherResult.result is 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.

验收备注

  • CORS survives the passthrough. A raw Response returned from the handler still gets access-control-allow-origin, -credentials and -expose-headers from the cors() middleware, and keeps the producer's own headers — measured with Origin: https://app.example on the real boot. The stream arms already returned raw Response objects this way; this arm is not a new shape for the middleware chain.
  • packages/client declares @objectstack/hono as a devDependency with no importer anywhere in the package — a dead manifest edge that puts client, client-react, cli, dogfood, downstream-contract and four examples into turbo ls --affected for any change to this adapter. Noted, not filed (an observation, not a reproducible defect).
  • packages/qa/http-conformance's tsconfig.json excludes **/*.test.ts, so pnpm --filter @objectstack/http-conformance typecheck does not cover the new conformance file. That exclusion is a recorded, ledgered condition (check-type-check-coverage.mjs's @objectstack/http-conformance entry, whose note says it cannot graduate by fixing code); the test layer is measured by check:type-check-debt instead. Pre-existing, unchanged by this PR.
  • @objectstack/hono declares no typecheck script at all, so pnpm --filter @objectstack/hono typecheck matches zero scripts and exits 0 — a false green if anyone reads it as coverage. It carries a ledger entry instead; check:type-check-coverage is 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 test100 passed (100), 4 files. (86/100 before the fix, with the 14 new cases red.)

  • pnpm --filter @objectstack/http-conformance run test89 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-loads reads built output and no package had dist/. Ran pnpm exec turbo run build --filter='./packages/*' --filter='./packages/*/*' — exactly what lint.yml does before it — then re-ran: exit 0, 104 published require entry points across 67 packages load, 620 emitted CJS files parse.
    • check:type-check-debt then OOM'd, and the cause was mine: its own header says the ceiling it runs tsc under is the caller's NODE_OPTIONS, and I had capped it at 4096. lint.yml pins this step at --max-old-space-size=6144 (paired with CI_TSC_HEAP_CEILING_MB in 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 silent because 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 #16380 interaction and the #15417 lead as recorded above.

⚠️ The shared verify lock was held continuously by sibling agents for much of this round (four queue timeouts, exit 99 each — NOT MEASURED, never a red). Two narrowings are declared rather than hidden: the ablation's two single-file vitest runs and the check:type-check-debt re-measure were taken unlocked, which is the class the lock's own --status text puts check:* 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

… 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
@github-actions github-actions Bot added the size/m label Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/hono, @objectstack/http-conformance, touching 3 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/qa/http-conformance/vitest.config.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/kernel/runtime-services/sharing-service.mdx (via NOT_FOUND (literal, a string literal in createHonoApp))
What this run could not see
  • 1 changed file(s) yielded no anchor (packages/qa/http-conformance/vitest.config.ts) — pages documenting those are invisible to this run
  • 1 anchor(s) matched too much of the corpus to be a work list: /api/v1 (route, 82 pages)
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 1 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json ecf44b1264657f01d533210e6e71cb03b0ed0a4bpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 4c118fd185cd21cb829d297d95a3f930c6eb5a68 — the merge of head 41ecd20f0ed7f949752596fd94df6666cfacd8e9 into base ecf44b1264657f01d533210e6e71cb03b0ed0a4b, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# 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

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs ecf44b1264657f01d533210e6e71cb03b0ed0a4b → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants