chore(tooling): enumerate namespace-claiming wildcard mounts and require each to be classified (#4116) - #4122
Conversation
…ire each to be classified (#4116) A handler mounted on `<prefix>/*` claims an entire namespace, and Hono runs every handler matching a path in registration order with the first Response winning. So a TERMINAL wildcard — one that always answers, never yields — makes every other route under that prefix reachable only when it happens to register first, and plugin registration order is not a contract anyone declares or tests. That shape has now cost four fixes, and every one was found by hand, after the fact, by someone reading the code for an unrelated reason: - #2567 — anonymous-deny on raw `/data` held only while REST registered first, so a load-order change silently reopened anonymous data access. - #4018 — the standalone `/discovery` sat behind the dispatcher's, so a third publisher drifted unnoticed. - #4088 / #4092 — AuthPlugin's `/api/v1/auth/*` swallowed plugin-hono-server's `/auth/me/permissions`, the console's entire permission layer. - cloud#923 — the same shape in AuthProxyPlugin, which cloud had ALREADY hit in staging (its apps/*/server/index.ts record the Console 404ing on that path). Nothing in CI noticed any of them, and nothing would notice the fifth. The driven tests #4092 and cloud#923 shipped each pin ONE catch-all and cannot cover the next wildcard someone mounts — a test only drives the routes that existed when it was written. What never existed is the ENUMERATION. This adds it, following check-route-envelope.mjs (#3843) including the part that matters most: a site the scan finds but the ledger does not declare is an ERROR, not a default. Three states — `yields` (verified from the AST, so an entry cannot rot: delete the next() and it fails), `exempt` with a reason for a wildcard that is supposed to own its namespace, `ratchet` naming the issue for one that is terminal and should not be. No fourth state where nobody looked. AST rather than regex because "does this call next()?" is a question about the handler's parameter binding: a comment or string mentioning next(), or an inner closure's own next, all fool a textual match in the direction that PASSES while the defect ships, and the parameter is not always named `next`. Handlers passed by identifier are resolved too — cloud#923 mounts its handler that way, and a scan that only understood inline arrows would have called it terminal. Found 13 namespace-claiming mounts where a manual grep had found 3, and with them two real, previously untracked instances of the #4088 defect in `packages/adapters/hono` (`${prefix}/auth/*`, `${prefix}/storage/*`) — ratcheted under #4117. It also separates that file's deliberate `${prefix}/*` ownership (ADR-0076 OQ#9) from the two that merely forgot to yield; in source they look identical. Verified both directions by hand: reverting #4092's fix fails with the terminal diagnostic, and adding a new undeclared `${prefix}/reports/*` catch-all fails as NOT DECLARED. Self-test covers 15 cases, including the three textual-match failure modes and identifier handler resolution. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013VZLsKypjrGhiLpio2uWKu
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
…ails on main
`scripts/check-wildcard-fallthrough.mjs` declared the hono adapter's
`${prefix}/storage/*` mount, which no longer exists:
✗ wildcard fall-through guard (#4116)
packages/adapters/hono/src/index.ts:all `${prefix}/storage/*`
DECLARED but not found by the scan. Moved, renamed or deleted? Update MOUNTS.
#4112 removed that mount outright — the adapter now carries a "deliberately NOT
mounted (#4087)" note in its place — because the wildcard claimed the whole
`/storage` subtree and answered its own 404 for every path `service-storage`
registers under it. That is the #4088 failure actually happening, so removal was
the fix rather than a ratchet.
#4112 and #4122 (which added this scan) were each green on their own base and
landed in that order, leaving the entry declaring a deleted mount. Reproduced on
pristine origin/main, so it is not specific to any branch — and since the guard
runs on every PR, it blocks all of them:
git worktree add /tmp/main-check origin/main
cd /tmp/main-check && node scripts/check-wildcard-fallthrough.mjs # 1 problem(s)
Removes the stale entry, which is what the guard's own message asks for. The
sibling `${prefix}/auth/*` entry stays: that mount is still there
(packages/adapters/hono/src/index.ts:279), so its ratchet is still live. The
comment above them said "Both are the #4088 shape" and is now singular, with the
storage mount's removal recorded so the next reader does not have to reconstruct
why one of a documented pair disappeared.
Verified: `pnpm check:wildcard-fallthrough` self-test 15 cases, then
6 yielding / 1 ratcheted / 5 exempt across 12 namespace-claiming mounts. The two
steps that follow it in the same CI job and never got to run — check:release-notes,
check:node-version — pass too.
Refs #4116, #4117, #4112
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HrRNgrWaRtggzmrHpbomyh
…oes not own, and unbreak the #4116 ledger (#4117) (#4133) Two things, and the second is urgent. ## The fix (#4117) `app.all(`${prefix}/auth/*`)` claimed a whole namespace and was TERMINAL: it returned the auth service's response unconditionally, including better-auth's 404 for a path it does not implement, and the legacy `handleAuth` bridge's own `handled: false` 404. That is #4088's shape. #4116's scan found it; the manual greps that preceded that scan had not. A 404 from better-auth, or `handled: false` from the dispatcher, now means "not this mount's path" and the handler yields. The predicate is the dispatcher's OWN `handled` flag wherever one exists — an explicit ownership answer beats inferring one from a status; only the better-auth hand-off lacks such a flag, and there the 404 is the signal, exactly as in #4092. What this buys here is narrower than #4092 and the code says so: it does NOT make a later-registered Hono route reachable, because the `${prefix}/*` dispatcher catch-all below is deliberately terminal (ADR-0076 OQ#9, #3576/#3608) and would swallow one either way. It means an unowned `/auth/*` path now reaches that gated `dispatch()` instead of dead-ending in a 404 built one mount earlier, so a domain handler registered for it becomes reachable — which is this adapter's actual extension mechanism. The first draft of the tests asserted the #4092 outcome and failed, which is how the distinction got established rather than assumed. ## Unbreaking main #4122 shipped the ledger declaring TWO ratcheted mounts here. Between its base (974c6d4) and its merge, #4087/#4112 landed and DELETED the `/storage/*` bridge — so the squash produced a main where the ledger declares a mount that no longer exists, and `check:wildcard-fallthrough` fails on main with "DECLARED but not found". That is my merge race, and this commit removes the entry. Worth recording why the guard behaved well here: the stale-entry arm is what caught it, immediately, on the first run against the new main. And #4112 reached the same verdict about that wildcard independently, from the other direction — "the wildcard was wider than the two routes it served". Two independent reads landing on one defect is the argument for enumerating the shape rather than finding it by eye each time. Also extends `callsContinuation`: handing the continuation to a helper (`yieldUnowned(c, next, …)`) now counts as yielding. Without that the checker called this fix terminal — a false negative that would have pushed the compose subtlety toward being duplicated at each call site instead of written once. Guard: 7 yielding / 0 ratcheted / 5 exempt over 12 mounts. Self-test 17 cases. adapters/hono 73 passed. Verified both defences bite: reverting the fix fails the guard with the TERMINAL diagnostic and fails 2 of the 6 new tests. The 3 `tsc` errors and 2 `eslint` rule-definition errors in this package are pre-existing, measured identical on a clean tree. Claude-Session: https://claude.ai/code/session_013VZLsKypjrGhiLpio2uWKu Co-authored-by: Claude <noreply@anthropic.com>
|
Heads-up: Reproduced on pristine Cause — a semantic conflict with #4112, not a bug in the guard. #4112 ("retire the Fix: drop the I've included that one-line removal in #4124 (unrelated PR, but it was blocked behind this and the fix is unambiguous) — happy to pull it out if you'd rather land it yourself. After it: Generated by Claude Code |
…moved NOT part of this branch's work — `main` is red on it and every open PR fails the same check, so this unblocks rather than waits. `origin/main` @ 857a6cf fails `check:wildcard-fallthrough` on its own, verified in a detached worktree with no branch involved: ✗ packages/adapters/hono/src/index.ts:all `${prefix}/storage/*` DECLARED but not found by the scan. Semantic conflict between two commits that both landed, not a bug in the guard. #4112 retired that mount outright — the adapter now carries a comment where `app.all(prefix + '/storage/*')` was, because the handler spoke a storage contract that does not exist and the wildcard claimed the whole `/storage` subtree. #4116's MOUNTS ledger (#4122) was written against a tree that still had it and landed after the removal, so the entry was stale on arrival. Git merged both cleanly because they touch different lines; the guard is what noticed, which is it working on day one. Dropping the entry is the whole fix — there is nothing left to ratchet when the mount it names does not exist. Its `${prefix}/auth/*` sibling is untouched and still correctly ratcheted to #4117. After: 6 yielding / 1 ratcheted / 5 exempt (12 namespace-claiming mounts). Flagged on #4122 as well, so whoever owns it can take it back if they would rather land it themselves. Also enumerated the ESLint job properly this time — it runs ELEVEN checks, not the six I had been running (`doc-authoring`, `authz-resolver`, `release-notes` and `node-version` were the ones I was missing, plus `pnpm lint` itself). All eleven pass, and `pnpm lint` is clean once `.cache/` — the objectui console build artifact an earlier `.objectui-sha` bump left in this worktree — is removed. Those 61 "errors" were entirely that directory; CI checks out fresh and never had them. Full suite: 129/129 packages. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CYbS3kS8xzsHNXFTzp4e2z
…d envelope (#4053) (#4124) * fix(runtime): the /ai/agents degraded fallback answers in the declared envelope (#4053) The last unenveloped SDK-addressable route. The framework's degraded fallback — what an open-source runtime with no service-ai answers — now returns `{ success: true, data: { agents: [] } }` via `deps.success`, and the guard's last ratchet retires with it: 0 ratcheted on both surfaces. ## Why `data: { agents }` and not `data: []` #3983 set the precedent that `data` carries the payload directly, and following it here would have looked consistent. It would also have been wrong, silently. `AiAgentsResponseSchema` is a DECLARED payload schema; share-links' `{ links }` was an ad-hoc wrapper with none. So this is the #3843 relocation — the declared payload moves under `data` unchanged, as SettingsNamespacePayload did — not a reshape. That decides the blast radius. `unwrapResponse` returns `body.data` when a body has a boolean `success` AND a `data` key, so `data: { agents }` keeps `client.ai.agents.list()` reading `.agents` off it, while `data: [...]` would make `.agents` undefined and the method answer `[]`. An empty list is not a visible failure on this route: `useAiSurfaceEnabled` gates the ENTIRE AI surface on `agents.length > 0`, and an empty catalog is the correct answer for a seat-less user (ADR-0068) or a Community-Edition deployment. Broken and legitimate look identical — no error, no 403, no log. ## Consequence: no lockstep Because the SDK reads both shapes identically, each surface converts on its own schedule. Cloud's service-ai still answers unenveloped and keeps working unchanged; objectui already reads all four shapes (objectui#2992). The "three repos in one batch" framing #4053 opened with does not apply to this variant — which is the finding, not a shortcut around it. Five tests in @objectstack/client pin both shapes, including the road not taken: the flattened body asserts [], so the cost of choosing it is recorded rather than rediscovered. The dispatcher's own fallback test now asserts the envelope and that no `agents` key survives at the top level. runtime 914, client 205, rest 505, spec 6987. All ten typecheck-job gates and the six check scripts pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CYbS3kS8xzsHNXFTzp4e2z * fix(runtime): re-point #4058's new /ai/agents assertion at the enveloped body Semantic conflict, not a textual one. #4058 step 2 (#4086) landed on main while this branch was open and added `http-dispatcher.test.ts`'s "a stub slot 404s per route and keeps the /ai/agents empty list", which asserts the body is exactly `{ agents: [] }` — the shape #4053 enveloped one commit earlier. The two merge cleanly and the test fails, which is why CI caught it and the merge did not. The courtesy that test pins is unchanged: a stub-occupied AI slot still answers an empty list rather than a fault. The list just travels under `data` now, and the assertion says so — plus that no `agents` key survives at the top level, so a revert to the bare body fails here too. Also swept the repo for any other reader of the old shape. Three hits, all correct: the two negative assertions above, and `client/src/index.ts`'s `body?.agents` — which is exactly what the relocation variant keeps working. Merged main in rather than rebasing, per the branch's history so far. Verified against the full suite the way Test Core runs it — `turbo run test --filter='!@objectstack/dogfood'`: 129/129 packages, runtime 924. Two worktree-staleness traps on the way, both reading as code failures: - `plugin-auth` could not resolve `hono` — the merge moved the lockfile and I had not reinstalled. `pnpm install --frozen-lockfile` fixed it. - `runtime` failed under turbo but passed standalone — the gitignored `.objectstack/data/memory-driver.json` accumulating rows again. Neither is CI-visible; CI installs clean and checks out fresh. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CYbS3kS8xzsHNXFTzp4e2z * fix(tooling): drop the wildcard ledger's entry for the mount #4112 removed NOT part of this branch's work — `main` is red on it and every open PR fails the same check, so this unblocks rather than waits. `origin/main` @ 857a6cf fails `check:wildcard-fallthrough` on its own, verified in a detached worktree with no branch involved: ✗ packages/adapters/hono/src/index.ts:all `${prefix}/storage/*` DECLARED but not found by the scan. Semantic conflict between two commits that both landed, not a bug in the guard. #4112 retired that mount outright — the adapter now carries a comment where `app.all(prefix + '/storage/*')` was, because the handler spoke a storage contract that does not exist and the wildcard claimed the whole `/storage` subtree. #4116's MOUNTS ledger (#4122) was written against a tree that still had it and landed after the removal, so the entry was stale on arrival. Git merged both cleanly because they touch different lines; the guard is what noticed, which is it working on day one. Dropping the entry is the whole fix — there is nothing left to ratchet when the mount it names does not exist. Its `${prefix}/auth/*` sibling is untouched and still correctly ratcheted to #4117. After: 6 yielding / 1 ratcheted / 5 exempt (12 namespace-claiming mounts). Flagged on #4122 as well, so whoever owns it can take it back if they would rather land it themselves. Also enumerated the ESLint job properly this time — it runs ELEVEN checks, not the six I had been running (`doc-authoring`, `authz-resolver`, `release-notes` and `node-version` were the ones I was missing, plus `pnpm lint` itself). All eleven pass, and `pnpm lint` is clean once `.cache/` — the objectui console build artifact an earlier `.objectui-sha` bump left in this worktree — is removed. Those 61 "errors" were entirely that directory; CI checks out fresh and never had them. Full suite: 129/129 packages. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CYbS3kS8xzsHNXFTzp4e2z --------- Co-authored-by: Claude <noreply@anthropic.com>
Closes #4116。#4088 / #4092 与 cloud#923 收尾后的通用护栏。
问题
挂在
<prefix>/*上的 handler 声明整个命名空间,而 Hono 对同一路径按注册顺序执行、先产出 Response 的赢。所以终结式通配(总是应答、从不让路)会让同前缀下其他路由只有"恰好注册得更早"才可达 —— 而插件注册顺序既没人声明、也没人测试。这个形状已付四次代价,四次都靠人手在为别的事读代码时发现:
/data匿名拒绝只在 REST 先注册时成立 —— 加载顺序一变,匿名数据访问被静默重开/discovery被 dispatcher 遮蔽,第三份供给长期漂移无人察觉AuthPlugin的/api/v1/auth/*吞掉/auth/me/permissions(console 整个权限层)AuthProxyPlugin同形状;cloud 已真的踩过,staging 上 Console 404CI 一个都没拦住,第五个也不会。
为什么不是再写一个 per-plugin 测试
#4092 与 cloud#923 各带一个驱动测试,钉住那一个 catch-all。它们值得有,但结构上覆盖不了下一个通配 —— 测试只能驱动它被写下那天存在的路由。从来不存在的是枚举:"本仓库哪些 handler 声明了命名空间、每个是不是终结式"一直没有答案。
做法
照
check-route-envelope.mjs(#3843)的既有形状,含最要紧的那条:扫到但未登记 = 错误,不是默认放行。三态:
yields—— handler 收next并调用它,由 AST 验证,所以条目不会腐烂:把next()删掉就失败;exempt—— 附理由,给本就应独占命名空间的(传输适配器单一入口、SPA 子树、第三方中间件工厂);ratchet—— 当前终结式、被跟踪、未被祝福,记下将修它的 issue。必须 AST 而非正则:"这个 handler 调了
next()吗"是关于参数绑定的问题。注释里提到next()、字符串里提到、内层闭包自己的next—— 三者都能骗过文本匹配,而且朝"通过"的方向骗;参数也不一定叫next。按标识符传入的 handler 也做解析 —— cloud#923 正是那样挂的,只认内联箭头的扫描会把它误报为终结式。扫出来的东西
13 处命名空间挂载(手工 grep 当初只找到 3 处):6 yielding / 2 ratcheted / 5 exempt。
其中两处是真实的、此前无人跟踪的 #4088 同类缺陷 ——
packages/adapters/hono的${prefix}/auth/*与${prefix}/storage/*,已登记为 ratchet 并立 #4117(该适配器全仓无消费者,所以今天没坏东西,但它是发布出去的包,而 ADR-0076 的触发场景本身就是外部嵌入方)。顺带一个副产品:同一文件里 line 382 那个刻意独占
${prefix}/*的(ADR-0076 OQ#9 / #3576 / #3608,拆成 per-prefix 会绕过闸门阶段)与两个忘了让路的,在源码里长得一模一样;ledger 第一次把它们区分开。验证
两个方向都手工验过:
declared { yields: true } but the handler never calls its continuation — it is TERMINAL;${prefix}/reports/*catch-all → 报NOT DECLARED并给出三态该怎么选。两次都在验证后完整还原,工作树干净。
--self-test覆盖 15 例,其中包括三种文本匹配失效模式(注释提到next()、字符串提到、内层同名遮蔽 —— 最后一条最初确实咬到了我自己的实现,callsContinuation当时没有作用域意识,已修)以及按标识符传入 handler 的解析。eslintclean。已接入pnpm check:wildcard-fallthrough与lint.yml(紧随两个同类 ratchet 之后,同样先跑自检)。changeset 为 release-nothing(空 frontmatter)—— 只加脚本与 CI 步骤,不动任何包代码。
Generated by Claude Code