Skip to content

docs(cli): the unknown-hostname-guard's install-order rationale names Phase 1 init() (#9745) - #9821

Merged
huangyiirene merged 1 commit into
mainfrom
claude/issue-9745-serve-guard-rationale-phase1
Aug 19, 2026
Merged

docs(cli): the unknown-hostname-guard's install-order rationale names Phase 1 init() (#9745)#9821
huangyiirene merged 1 commit into
mainfrom
claude/issue-9745-serve-guard-rationale-phase1

Conversation

@huangyiirene

Copy link
Copy Markdown
Collaborator

Fixes#9745

The guard is installed correctly and this PR changes nothing it accepts or refuses. What was wrong is the rationale in its doc comment — the part the next author copies:

Hono's app.use('*') is order-independent for matching, so as long as the middleware is added before kernel:listening fires, it intercepts every request regardless of which plugin registered its handler.

Premise re-verified on this ref, not inherited

  • The comment block is still present in packages/cli/src/commands/serve.ts and still says what the card quotes. The card cites :3429-3435; on origin/main at 5cc8a3b it sits at :3428-3435 — located by text, never by line number.
  • LiteKernel.bootstrap (packages/core/src/lite-kernel.ts) and ObjectKernel.bootstrap (packages/core/src/kernel.ts) each run Phase 1 init() for every plugin, then Phase 2 start() for every plugin, and only then trigger kernel:readykernel:bootstrappedkernel:listening. All three hooks fire strictly after Phase 2.
  • Route registration begins in Phase 2 and continues past it: createConsoleStaticPlugin mounts the Console in start() (packages/cli/src/utils/console.ts), HonoServerPlugin registers the current-user endpoints from a kernel:ready hook, and plugin-auth defers its terminal /api/v1/auth/* mount to kernel:ready as well. Phase 1 init() is ahead of all of it — which is what the code already does.

Verdict: the premise holds and the emphasised sentence is false. "Before kernel:listening" admits install points that sit behind every route.

What changed

1. packages/cli/src/commands/serve.ts — comment only. Names Phase 1 init() as the requirement, states the true mechanism (Hono composes the handlers a request matched in registration order, so a route registered ahead of the middleware answers and never calls next()), and drops the "order-independent for matching" clause — the phrase that made the false generalisation sound principled. The correct first sentence is kept; the bypass matrix and the inline-object-literal note are untouched.

2. packages/cli/src/commands/serve-unknown-hostname-guard.test.ts — one new test pinning the corrected sentence. Every other test in that file mounts the guard first via mountGuard, which is precisely the case that works, so none of them can fail when the false claim comes back.

The new test registers /beforeahead of the guard's init() and /afterbehind it, on one real HonoHttpServer, then drives one unmapped hostname at both: /before answers 200 and the env-registry is never consulted; /after is refused with 404 and ENVIRONMENT_NOT_FOUND. Registration order is the only difference between those two requests.

Reverse verification

Prediction stated before running: moving /before's registration to after init() turns the test red on the pass-through leg. Measured, from the committed state:

- 200
+ 404
❯ src/commands/serve-unknown-hostname-guard.test.ts:456:27
Tests 1 failed | 34 passed (35)

Restored with git checkout HEAD -- ..., re-run: Tests 35 passed (35). No rebuild is involved on either leg — the subject is imported from ./serve.js, a same-package source specifier vitest resolves out of src/, and the mutation lived inside the test file itself.

Scope boundary

packages/runtime/src/http-metrics-inbound-coverage.hono.integration.test.ts — the card's evidence table — was read, never written. The pin above needed no part of that harness: packages/cli already boots a real Hono adapter in its own guard suite. No Blocked-by: was wired, per triage.

Changeset: deliberately none — skip-changeset instead

A judgment call, not an assumption. The serve.ts diff is prose inside a JSDoc block: nothing that executes changes, no public surface moves, and there is no user-visible behaviour for a release note to describe. The test file never reaches the published tarball at all — packages/cli/tsconfig.build.json carries "exclude": ["src/**/*.test.ts", "src/**/*.spec.ts", "src/**/__tests__/**"], so it is not compiled and files: ["dist"] cannot ship it. Precedent landed on main today: #9802 corrected prose inside @objectstack/lint's published src/** and merged with no changeset.

Gates run, all at 218e2be

Derived with node scripts/pm/dispatch-gates.mjs (no paths passed — the script took the change set from the merge base 5cc8a3b itself), then run:

gateresult
pnpm --filter '@objectstack/cli^...' builddependency closure built first, in this fresh worktree
pnpm lintclean, no findings (run regardless — the deriver does not name it)
pnpm --filter @objectstack/cli testTest Files 134 passed (134), Tests 1462 passed (1462)
pnpm --filter @objectstack/cli typechecktsc --noEmit, clean
pnpm check:route-envelope11 modules audited, 7 conformant / 1 ratcheted / 3 exempt — unchanged
pnpm check:cross-package-test-inputs33 self-test cases pass; 12 packages read outside themselves, all declared
pnpm check:engine-double-contractOK — 321 pinned, 133 ledgered, 2 exempt
pnpm check:where-matcher257 matchers, 0 silently-wrong, 0 new
pnpm check:query-options-erasureratchet holds, none new
pnpm check:nul-bytesOK, 6265 text files, no raw control bytes
node scripts/docs-audit/check-affected-docs.mjsself-test 242 cases pass

check:type-check-coverage / check:type-check-debt were not run locally, and the reason is the measurement rather than the cost: packages/cli declares a typecheck script and its tsconfig.json includes src — only tsconfig.build.json excludes tests — so the new test file already sits inside a tsc program and the package carries no TEST_DEBT entry to re-measure. The ratchet half needs the whole workspace closure built; CI runs that farm. tsc --noEmit over the package is the local half.


Generated by Claude Code

…le (#9745)
The guard is installed correctly; the stated rationale was not. Name Phase 1
init() as the sufficient condition and drop the "order-independent for
matching" clause, and pin the ordering property locally.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WeN7F6jQFpcqW2BN56RdPa
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/cli, touching 2 documentable anchor(s).

17 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 2a9752cf0d48cd31f238e3eb3d3681e10e46658a.

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

What this run could not see

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 2a9752cf0d48cd31f238e3eb3d3681e10e46658apackageMentionDocs.

Which tree this was computed on

This run read content/docs from 4345b820c5f9d489bb030dfe8253e8ce4d5ea585 — the merge of head 218e2be7bcfbe69ca6df685c3394b08dd26382c2 into base 2a9752cf0d48cd31f238e3eb3d3681e10e46658a, 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 4345b820c5f9d489bb030dfe8253e8ce4d5ea585 && git checkout 4345b820c5f9d489bb030dfe8253e8ce4d5ea585
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 2a9752cf0d48cd31f238e3eb3d3681e10e46658a 218e2be7bcfbe69ca6df685c3394b08dd26382c2 && git checkout -B drift-repro 2a9752cf0d48cd31f238e3eb3d3681e10e46658a && git merge --no-ff 218e2be7bcfbe69ca6df685c3394b08dd26382c2
node scripts/docs-audit/affected-docs.mjs --json 2a9752cf0d48cd31f238e3eb3d3681e10e46658a

⚠️ 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 2a9752cf0d48cd31f238e3eb3d3681e10e46658a → pass the list as
args.docs, on the commit named under Which tree this was computed on.

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

Labels

size/mskip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

2 participants

@huangyiirene@claude