Skip to content

fix(api): envelope the plugin-mounted Hono error paths — six refusal bodies stop speaking the pre-#3675 dialect - #9456

Merged
hotlong merged 5 commits into
mainfrom
claude/issue-9364-plugin-route-envelope-error-paths
Aug 18, 2026
Merged

fix(api): envelope the plugin-mounted Hono error paths — six refusal bodies stop speaking the pre-#3675 dialect#9456
hotlong merged 5 commits into
mainfrom
claude/issue-9364-plugin-route-envelope-error-paths

Conversation

@os-zhuang

@os-zhuangos-zhuang commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Fixes#9364

Converts the error-path half of what #9267's third gate surface measured — the plugin-mounted Hono refusals that departed from BaseResponseSchema.

All gate results below are from e2ca3b1ea, the merge of origin/main into this branch. That merge matters: #9455 landed #9389's ruled exempt entries in the same gate script (+351/−69), so the numbers here are the post-merge ones.

Per-file counter deltas

node scripts/check-route-envelope.mjs, before and after — the acceptance instrument for this card:

filebeforeafter
packages/plugins/plugin-hono-server/src/adapter.tsunenveloped 4, stringError 4, siblingCode 1conformant ({})
packages/cli/src/commands/serve.tsunenveloped 1, stringError 1conformant ({})
packages/adapters/hono/src/index.tsunenveloped 2, errorCodeNotString 1unenveloped 2 (unchanged), retargeted to #9436

Eleven counters removed, none raised, nothing rebaselined and no exempt entry added by me. Post-merge summary line:

✓ Plugin-mounted Hono routes — 11 module(s) audited, 161 hand-built body/bodies
(count reported, NOT pinned): 7 conformant, 1 ratcheted, 3 exempt

The three exempt are #9455's, untouched and still closed at their exact ruled counts — runtime-config-plugin.ts at unenveloped 1, current-user-endpoints.ts at 9, auth-plugin.ts at 3. The one remaining ratchet is adapters/hono.

What changed on the wire

@objectstack/plugin-hono-server — the adapter's refusals, i.e. what any host using it as its transport answers for an unmatched request or a handler that produced nothing:

statuswasnow
404 unmatched{ error: 'Not found' }{ success: false, error: { code: 'ENDPOINT_NOT_FOUND', message: 'Not found' } }
405 mismatch{ error, code, message, method, path, allowed }{ success: false, error: { code: 'METHOD_NOT_ALLOWED', message, details: { method, path, allowed } } }
500 no response{ error: 'No response from handler' }{ success: false, error: { code: 'INTERNAL_ERROR', message: … } }
500 fallback threw{ error: 'Fallback handler failed' }{ success: false, error: { code: 'INTERNAL_ERROR', message: … } }

@objectstack/honoerrorJson wrote the HTTP status into error.code, where ApiErrorSchema.code declares a closed string vocabulary. It now derives the standard member from the status via resolveThrownHttpError (@objectstack/types) — the rule the REST and dispatcher doors already read (ADR-0112, #9106), so this third door does not become a fourth dialect.

@objectstack/cli — the OS_ROOT_DOMAIN unbound-hostname 404 loses its bare-string error and two stray top-level keys; hostname moves to error.details. The Accept: text/html branch still serves the styled 404 page.

All codes come from the StandardErrorCode catalog (ENDPOINT_NOT_FOUND, METHOD_NOT_ALLOWED, INTERNAL_ERROR, RESOURCE_NOT_FOUND) or are already-registered ledger members (ENVIRONMENT_NOT_FOUND), so no packages/spec edit and no new registration was needed. The 405's codevalue is unchanged — only its position moved, from beside error to inside it.

Dialect choice, verified rather than assumed

Per the #9326 / PR #9425 precedent: the ADR-0112 nested shape was chosen and then measured. Two traps were live here and both were checked against the scanner rather than reasoned about:

  • Adding success: false while leaving method/path/allowed/hostname at the top level would have moved those bodies from unenveloped into strayKeys — a net-zero trade, not a fix. They went into error.details, which ApiErrorSchema declares for exactly this. strayKeys stayed at 0 on all three files.
  • In errorJson, renaming the third parameter from code to status is load-bearing, not cosmetic: the gate flags an error.code shorthand whose identifier is the same one passed as the status argument, so keeping the old name would have left errorCodeNotString alive with the value already fixed.

The literals are deliberately written inline at each c.json(...) rather than hoisted into shared constants. The gate judges the object literal; an identifier reads to it as a relayed body it must not police — hoisting would have zeroed these counters by hiding the bodies from the scanner instead of by conforming them.

The merge with #9455, resolved by re-deriving

Both sides edit PLUGIN_ROUTE_MODULES. Rather than pick hunks — where choosing a side silently restores a counter this card removed, or drops a ruled exemption that is not mine to drop — the file was reset to main's version whole (git checkout origin/main -- …) and the three conversions re-applied onto it by exact anchor. Verified afterwards in both directions: all three of #9455's exempt entries present with their exact numbers, and all three of my conversions present.

One line of #9455's new header needed updating rather than preserving: it named adapter.ts, adapters/hono/src/index.ts and serve.ts as ordinary doors that "stay tracked drift under #9364". Post-merge two are conformant and the third is retargeted, so that sentence now describes the contrast instead — the ruling still does not reach them, and #9364 converted them, which is what a ratchet is for.

The three ratchet: '#9364 …' pointers flagged as a possible dangling carry-forward were checked on the merged ref: after #9455's restructuring they named this PR's own three entries, not the bare-payload files. All three are gone now — converted or retargeted — so the carry-forward was entirely mine and is entirely closed. The only #9364 strings left in the script are inside #9455's own synthetic self-test fixtures (x.ts / y.ts), which are test data, not ledger entries.

Consumer packages, and two declared-surface widenings

Both are mechanically forced by the conversion rather than chosen.

packages/qa/http-conformance — that package's zero-dependency NodeHttpServer mirrors the adapter's unmatched-request bodies byte-for-byte on purpose (its source says "Same body as the primary adapter" and "Both paths must produce the byte-identical answer"), and fallback-seam.conformance.test.ts runs the same cases against both adapters. Converting only the primary adapter would have made "both adapters agree" false in the suite that exists to assert it. The file is invisible to the route-envelope gate — it writes via nodeRes.end(JSON.stringify(...)), not c.json(...) — so no counter moves because of it.

Its two remaining bare 500 bodies (a handler throw, and the SSE-teardown 500) are left alone: they mirror nothing in the primary adapter, no conformance case asserts them, and no counter covers them. Fixing them would have been unbounded creep with no evidence pinning the shape.

packages/runtime — authorised by the PM after Test Core went red on the first head. That package drives the hono adapter through the dispatcher plugin, and four of its integration suites pin the adapter's refusal bodies. I confirmed these are adapter pins, not another door's contract, by reading what each case exists to prove: the two multi-tenant-endpoint cases assert "the transport's own 404 stands" as the environment-resolution decline branch; ready asserts it as an explicit anti-vacuity control that the harness mirrors prod; endpoint-fallback's shared constant is documented as "the transport's unmatched answer"; and the mcp-skill case is labelled the boundary probe for unmatchedResponse() being the correct owner of an unmounted verb. So the conversion did not over-reach — these pins had to move with it.

⚠️One thing the red build did not show, and it is the sharper half. The same file's POST/DELETE cases separated the two doors by typeof body.error !== 'string' plus three top-level not.toHaveProperty('method'/'path'/'allowed') checks. Once the adapter gained the same envelope, all four became true of the adapter answer too — so those assertions kept passing while no longer discriminating, in exactly the world the file exists to catch. They are replaced with the distinction that still holds: the domain branch fills error.httpStatus and no details, the adapter the reverse. Every touched assertion is at least as strong as before; none weakened, skipped or quarantined.

Doors that legitimately did not move, checked and left alone: packages/rest's 405 (its own OBJECT_API_METHOD_NOT_ALLOWED code), runtime's error-envelope.conformance.test.ts and meta-verb-fallthrough.test.ts (dispatcher called directly, no adapter in the path), and packages/core's QA-harness fetch mocks — whose sibling row uses { error: 'nope' }, proving the body there is an arbitrary payload and the assertion is on the status.

Verification

Full rebuild after the merge (pnpm install --frozen-lockfile, 70/70 turbo build tasks, packages/runtime/.objectstack cleared), then tests on e2ca3b1ea:

  • pnpm --filter @objectstack/runtime test167 files, 2503 tests passed (main brought 4 new ones)
  • pnpm --filter @objectstack/plugin-hono-server test18 files, 211 tests passed
  • pnpm --filter @objectstack/http-conformance test4 files, 72 tests passed (the cross-adapter suite, both adapters over real sockets)
  • pnpm --filter @objectstack/hono test2 files, 73 tests passed
  • pnpm --filter @objectstack/cli test131 files, 1412 tests passed

Refusal cases assert error.codeandstatus, per the dispatch rule — including error.details.method / .path / .allowed on the 405, asserted by value rather than by presence.

Reverse verification (run pre-merge, on the committed tree) — reverting just the 404 body to { error: 'Not found' } turned the gate red in the predicted direction, proving the banked zeros are load-bearing pins and not vacuous:

packages/plugins/plugin-hono-server/src/adapter.ts
unenveloped: found 1, declared 0 — a NEW non-conforming body.
...
Raising the declared number is not the fix.

Gate union, all green on e2ca3b1ea: check:route-envelope, check:cross-package-test-inputs, check:test-source-alias, check:type-source-resolution, check:changeset-gate-self-tests, check:objectui-changeset, check:engine-double-contract, check:where-matcher, check:query-options-erasure, check:type-check-coverage, check:type-check-debt (--re-measure, on the rebuilt closure: 33 entries, 1926 raw errors, none above its recorded number), plus check-adr-0087-registration, check-changeset-no-major, check-empty-changeset, check-cross-package-test-inputs, check-nul-bytes, check-affected-docs.

Out-of-scope findings filed


Generated by Claude Code

Six hand-built refusal bodies on plugin-mounted Hono routes departed from
BaseResponseSchema, invisible to every check in the repo until #9267 added the
route-envelope gate's third surface. This converts the error-path half:
- plugin-hono-server/src/adapter.ts — the 404, 405, and two 500 refusals now
answer { success: false, error: { code, message } }; the 405's method/path/
allowed move into error.details and its METHOD_NOT_ALLOWED code moves from
beside `error` to inside it (wire value unchanged).
- adapters/hono/src/index.ts — errorJson wrote the HTTP status into
error.code; it now derives the ADR-0112 member from the status through
resolveThrownHttpError, the rule the REST and dispatcher doors already read.
- cli/src/commands/serve.ts — the unbound-hostname 404 drops its bare-string
error and two stray top-level keys.
qa/http-conformance's NodeHttpServer mirrors the adapter's unmatched-request
bodies byte-for-byte by design and is locked to them cross-adapter by
fallback-seam.conformance.test.ts, so it moves in the same commit.
check-route-envelope.mjs counters banked: adapter.ts and serve.ts to
conformant, adapters/hono keeps unenveloped 2 (retargeted to #9436).
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012WKSnqAaoqtW3QX7SSf1Vk
@github-actions

github-actionsBot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 4 package(s): @objectstack/hono, @objectstack/cli, @objectstack/plugin-hono-server, @objectstack/http-conformance, touching 8 documentable anchor(s).

17 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive: node scripts/docs-audit/affected-docs.mjs --json origin/main.

4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)

Coarse fallback — 23 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 origin/mainpackageMentionDocs.

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 origin/main → pass the list as args.docs.

…lope (#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
…gin-route-envelope-error-paths
# Conflicts:
#	scripts/check-route-envelope.mjs
@hotlong
hotlong disabled auto-merge August 18, 2026 05:37
@hotlong
hotlong enabled auto-merge August 18, 2026 05:38
@claude

claudeBot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Heads-up before you merge — a new head is on its way, and the current one cannot satisfy the required checks.domain:cli PM seat (#6024), session session_012WKSnqAaoqtW3QX7SSf1Vk. Thanks for the review.

I saw the auto_merge_disabled (05:37:57Z) followed by your approval (05:38:47Z) and read it as you taking the landing. ⛔ I have not re-armed auto-merge and will not — undoing another party's deliberate action without knowing why is not something this seat does. Say the word if you'd rather I re-arm it instead.

The one thing worth knowing before you hit merge: this PR's current head 9cf1b4c34 predates PR #9421 (merged 04:27:08Z), so it still emits the old ESLint context and notLint & Repo Gates — which the main ruleset has required since 03:26:48Z. Verified on the head: Lint & Repo Gatesabsent, ESLint present, and the other five required contexts all green. So as it stands the PR is mergeable_state: blocked regardless of approval.

The dev on this card has already merged origin/main locally (merge commit 04eaa951c, clean tree) and is finishing its verification pass — gate re-run plus the affected suites — before pushing, which is the order this lane asks for. Once that lands, the new head will emit Lint & Repo Gates and the PR becomes mergeable.

So: either wait for that push and merge the new head, or tell me and I'll have it pushed immediately. Merging is yours; I'm only flagging that the head you approved cannot currently pass.

Context on the change itself, if useful: the card converts the plugin-mounted Hono error paths onto the declared envelope — 11 gate counters removed, none raised, post-merge gate summary 7 conformant, 1 ratcheted, 3 exempt. It also carries a conflict resolution against #9455 that was verified in both directions (that card's three exempt entries with counts 1 / 9 / 3 intact, this card's conversions present, no ratchet lifted). Full review is on #9364.


Generated by Claude Code

@hotlong
hotlong added this pull request to the merge queueAug 18, 2026
Merged via the queue into main with commit 88e1bacAug 18, 2026
26 checks passed
@hotlong
hotlong deleted the claude/issue-9364-plugin-route-envelope-error-paths branch August 18, 2026 06:18
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

20 hand-built plugin-route bodies depart from the declared envelope — the ratchet #9267's third gate surface opened with

3 participants

@os-zhuang@hotlong@claude