fix(runtime): the dispatcher's scope strip matches /environments/, the prefix its own hint parser reads - #15859
Conversation
…the prefix its own hint parser reads
`HttpDispatcher.dispatch()` reads one scoped-URL convention in three places:
`extractEnvironmentIdFromPath` (the environment-id hint), the
`acceptOAuthAccessToken` test, and the scope strip that lets
`DomainHandlerRegistry` match the remainder. Only the first had been moved to
the ADR-0006 `/environments/` spelling. The strip's comment already claimed
`/environments/:environmentId`; its regex matched `/projects/`.
Driven through the real `@objectstack/hono` catch-all — the entry cloud hosts
mount, and the only one that hands `dispatch()` a still-scoped path — the
strip never fired, and the registry matches from the head of the path:
GET /api/v1/environments/env_alpha/health 404 ROUTE_NOT_FOUND -> 200
GET /api/v1/environments/env_alpha/data/task 404 ROUTE_NOT_FOUND -> reaches /data
GET /api/v1/health (control) 200 -> 200
GET /api/v1/data/task (control) reaches /data, unchanged
GET /api/v1/no-such-domain (negative) 404 -> 404
The legacy spelling is not kept as an alias. Nothing parses `/projects/<id>`,
so stripping it discarded the only place the request named an environment and
served it from the host default; ADR-0006 D2 retired `project` on the API
surface with no aliases, and `content/docs/api/environment-routing.mdx` tells
callers to replace it. It now answers 404, which is the honest response.
The OAuth-on-MCP gate moves in the same change because repairing the strip is
what makes it reachable: left behind, a scoped `/mcp` caller would reach the
domain with its OAuth 2.1 access token refused. The orphaned pre-rename
docblock stacked above `extractEnvironmentIdFromPath` is deleted — it named a
"project UUID" and the retired URL form, and it is the shape of prose this
card exists to remove.
Part of #15488
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
…rip actually removes `routeObjectFromPath` documents what it expects of the dispatcher's cleaned path, and said `/projects/:environmentId` prefix stripped. Same sentence, same strip, same retired spelling as the regex this branch repaired — a comment that teaches the next reader the wrong invariant is the whole subject of this card, so leaving one of them standing one file over would only defer it. Prose only; `routeObjectFromPath` itself matches `/^\/data\/([^/?#]+)/` and is unchanged. Part of #15488 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
📓 Docs Drift CheckThis PR changes 1 package(s): 18 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 1 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 24 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 ec9586bf0667ffbc762b1f7a22ed7cc0ebfe1d80 && git checkout ec9586bf0667ffbc762b1f7a22ed7cc0ebfe1d80
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 59953d5a3fe247f738a446f30bb7eb9f239a17dd 4538d6289d5b4ef1a4d8365680ad58e1e5befa40 && git checkout -B drift-repro 59953d5a3fe247f738a446f30bb7eb9f239a17dd && git merge --no-ff 4538d6289d5b4ef1a4d8365680ad58e1e5befa40
node scripts/docs-audit/affected-docs.mjs --json 59953d5a3fe247f738a446f30bb7eb9f239a17dd
|
os-litant
left a comment
There was a problem hiding this comment.
Contract review (clause ②) — in-seat, contract-review tier
VERDICT: PASS
REVIEWED-HEAD: 4538d6289d5 (PR #15859)
CLAUSE-2-PATH: no
CLAUSE-2-CONTENT: yes
Implemented-by: `claude/issue-15488-scope-strip-prefix`
Reviewed-by: `session_01D47qPfEWVPmhguWgBZCi5N`
FINDINGS: 0 blocking. 2 citation-precision notes (non-blocking, wording only). 1 residual reading that only the cloud repository can answer (NOT MEASURED here, already carried by #15861).
Why this is a COMMENT and not an approval. GitHub refuses APPROVE on a pull request authored under the same account, and agent seats do not submit approving reviews in any case. The verdict is the fenced block above; the dispatching seat performs the landing steps. The Implemented-by: identity is the dev's branch (a subagent dev has no session of its own; the branch is the one its claim comment on #15488 names), and Reviewed-by: is this seat's session — two disjoint grammars, so this is not a self-review under the C4 rule. Tier: get_session reads the dispatching session, not this reviewer, so the reading that counts is the harness model stamp in this run's own transcript (subagents/agent-a0cb91b66ad655b2b.jsonl, 60 stamps, all equal to CONTRACT_REVIEW_TIER as exported by scripts/pm/dispatch-gates.mjs). The seat is to verify that transcript before adopting this verdict, per the transcript-verification rule.
Clause ②, re-judged from the delivered diff
Path limb — no. Four files: .changeset/dispatcher-scope-strip-environments-prefix.md, packages/runtime/src/http-dispatcher.ts, packages/runtime/src/http-dispatcher.scoped-url-strip.test.ts, packages/runtime/src/security/permission-denied-envelope.ts (docblock only). No governed surface is touched: docs/adr/** is read, not changed. No new exported symbol, no new key on a published payload.
Conformance limb — yes, and correctly declared. The diff changes what the door accepts, in four derived judgments, each checked:
GET /api/v1/environments/<id>/<domain>through the@objectstack/honocatch-all now reaches<domain>(was 404ROUTE_NOT_FOUND). Correct. It is the documented surface (content/docs/api/environment-routing.mdx, the 6.0.0 changelog migration entry, thedispatcher-plugin.tsscoped mounts, the hint parser) and the catch-all is the only entry that handsdispatch()a still-scoped path (packages/adapters/hono/src/index.ts:subPath = c.req.path.substring(prefix.length), context{ request: c.req.raw }— no routerparams).GET /api/v1/projects/<id>/<domain>now answers 404 (was stripped and served). Correct, and the pre-change behaviour was a defect, not a feature — see the alias derivation below.- OAuth 2.1 access tokens are now honoured on
/environments/<id>/mcp(were not). Correct and non-deferrable — see the OAuth section. - OAuth tokens are no longer honoured on
/projects/<id>/mcp(were, on the host-default environment). Consistent with 2: that path no longer reaches any domain.
Semver. patch on @objectstack/runtime is consistent with the repo rule (a bug fix in a released package takes a patch) and with the fact that the scoped-route rename was already published as a breaking migration entry in the runtime 6.0.0 changelog ("Scoped routes: /api/v1/projects/:projectId/... → /api/v1/environments/:environmentId/..."). The changeset body carries the FROM → TO line regardless.
Boundary flag. The dev's one open question (does a cloud host still emit the retired prefix?) was ruled C by the seat and is carried by #15861. Not re-opened here; a sharpening is noted under residual risk.
Source readings — do they hold?
ADR-0006 v4 — holds in substance, misattributed in citation. Read at docs/adr/0006-project-environment-split.v4.md, unchanged by this PR:
- The second addendum (2026-08-28, #12867) did execute D2 with no alias ("no deprecated forwarder, no compatibility getter, nothing"; "no dual-key emission and no compatibility window"). But D2's three surfaces are D1's three: the SDK
projectsnamespace, the control-plane response keys, and the SDK JSDoc. The scoped URL prefix is not among them. It was renamed in the v5.0 rename itself — commit944f18758ef(2026-05-24, "rename project to environment throughout codebase"), the same commit that flippedextractEnvironmentIdFromPathandrest-server.tsto/environments/and left the strip (from276b94284a6, 2026-04-23) and the OAuth gate behind. That rename is recorded in the ADR's section "The v5.0 rename and its no-alias decision" (#12747), which is also where the quoted sentence lives — "no gradualism, no dual-spelling interval, no single release carrying both" (lines 303–305), not in the second addendum. - Consequence for the verdict: none. The applicable rule is the general one and it is stronger than the PR states — the prefix was never granted a retention window at all, and AGENTS.md's "No aliases. See ADR-0006." points at the same record. The "one spelling, not an alternation" argument stands at source.
- Consequence for the prose: the strip comment, the test docblock and the changeset all say "ADR-0006 D2 retired
projecton the API surface with no aliases". A reader who follows that to D2 finds three other surfaces. Non-blocking wording note: cite the v5.0 no-alias section for the prefix, and D2 only as the later confirmation that the rule reached even the last retained surfaces without an alias. Fold in or file, the seat's call; it does not change the acceptance set.
content/docs/api/environment-routing.mdx — item 1 holds verbatim; the second quote belongs to item 5. Migration checklist item 1 reads exactly "Replace /api/v1/projects/:projectId/... with /api/v1/environments/:environmentId/...". The sentence "there is no alias, so the old spelling does not resolve" is checklist item 5, about the SDK client.project(id) method, not about the URL prefix; the page does not literally say the old URL prefix does not resolve. The in-code comment cites only item 1 (accurate); the PR body juxtaposes the two as if both concern the prefix. Non-blocking: the caller instruction the PR relies on is item 1, and the 6.0.0 changelog entry says the same about the prefix. (Out of scope, handed to the seat: the same page's "Client SDK" section still says the client "keeps the legacy factory name project(id)" while item 5 says it was retired at D2 — a docs inconsistency for a docs card, not this PR.)
The crux — was /projects/<id> ever a working alias? No, not in this repository.
Re-derived, not accepted. Three independent measurements, each with a control:
(a) Source derivation on both trees. I extracted the three literal regexes (hint parser, OAuth gate, strip) from the merge-base dispatcher (5a21d73) and from the head dispatcher (4538d62) and evaluated them on ten URLs with the registry's match rules (exact/segment/prefix, startsWith from the head of the path). Control axis: same inputs, same evaluator, only the prefix spelling differs between the two trees; the unscoped forms (/health → /health, /data/task → /data) and the negative control (/no-such-domain → no domain) read identically on both. Merge-base:
/api/v1/projects/env_alpha/data/task urlEnvironmentId=undefined oauth=false after-strip=/data/task domain=/data
/api/v1/projects/env_alpha/mcp urlEnvironmentId=undefined oauth=true after-strip=/mcp domain=/mcp
/api/v1/environments/env_alpha/data/task urlEnvironmentId=env_alpha oauth=false after-strip=/environments/env_alpha/data/task domain=NONE (404)
Head: the two rows swap — /environments/… parses, strips and reaches its domain with OAuth true on /mcp; /projects/… parses nothing, strips nothing, reaches nothing. /cloud/environments/<id>/mcp is excluded on both trees (anchored regexes plus the control-plane guard). So on the merge-base the retired prefix was stripped by a reading that nobody paired with a parser: urlEnvironmentId stayed undefined (the hint regex is /\/environments\/([^/?#]+)/; the context.request?.params?.environmentId fallback is absent on the catch-all's raw Request), and the host resolver went on to hostname / header / session / default. The request named an environment and was served from a different one, silently. One fact the PR body did not print: /projects/<id>/mcp reached the MCP domain with OAuth honoured, on that default environment. The 404 is a wrong answer becoming a refusal; the risk reading in #15861 is the right way round.
(b) The pin, and an ablation I ran myself. In a fresh worktree at 4538d6289d5 (dependency closure built; no packages/runtime/dist present):
vitest run src/http-dispatcher.scoped-url-strip.test.ts→ exit 0, 7 passed / 7.- Same pin with the merge-base dispatcher blob swapped in (
git cat-file blob 5a21d73:packages/runtime/src/http-dispatcher.ts; pre-change spelling on disk = 1, head spelling = 0) → exit 1, 4 failed / 3 passed: the two scoped-routing assertions and both OAuth assertions red; the two controls and the "parses the id off the SAME prefix it strips" assertion green — exactly the split the drift predicts, since the hint parser was already correct. With no runtimedistin the tree, a red ablation is proof the pin resolves source. - Restored from the head blob, verified by hash (
bdab9bf2fc1…on disk =HEAD:…/http-dispatcher.ts),git diff --quietclean.
(c) No other reader, no in-repo emitter. Full-scope git grep over packages/ (non-test, non-dist): no reader of context.routePath / urlEnvironmentId outside the dispatcher, no in-repo KernelResolver implementation (the only in-repo resolveKernel site is a consumer in plugin-hono-server/current-user-endpoints.ts), and no emitter of the /projects/:id scoped prefix — the hits are changelog migration entries, a REST README example in which projects is an object name, the two rest-server.ts docblocks already filed as #15858, and a storage bucket prefix. Every scoped mount in dispatcher-plugin.ts dispatches a pre-stripped literal subpath with router params, which is why the standalone server never showed the defect.
The acceptOAuthAccessToken site — the reasoning holds, and the repaired gate is correct, not merely consistent
Non-deferrability holds. With the flag false, resolve-execution-context.ts never extracts a JWT bearer (if (opts.acceptOAuthAccessToken) { … }), so the request falls to the better-auth session path, which does not recognise the JWT, and resolves as a guest. Before the repair a scoped /mcp URL never reached the MCP domain, so the flag's value there was unobservable; repairing the strip alone would have made /environments/<id>/mcp reach the MCP domain with the token unread — a scoped MCP caller admitted to the domain as a guest. The two lines had to move together, and the ablation confirms the pin discriminates them (both OAuth assertions red under the swap).
Correctness of the repaired gate, judged against the strip and the domain rather than against itself:
- Same anchor, same id segment:
^(?:\/environments\/[^/]+)?\/mcp(?:[/?]|$)versus the strip^\/environments\/[^/]+(\/.*)?$. Whatever the gate accepts as a scoped MCP path, the strip reduces to/mcp…; the MCP domain is registered{ prefix: '/mcp', match: 'segment' }, and the gate's(?:[/?]|$)boundary is the same segment boundary —/mcpfoois refused by both. - Both run on the same
cleanPathstring (dispatch()passes it toresolveRequestScopebefore the strip reassigns it), so there is no second normalisation to drift. - The direction is not a widening. The gate re-enables the #2698 contract as originally written ("the plain and scoped route forms"); the token is verified against the per-request kernel's auth service (
resolveService(requestKernel(context), 'auth', context.environmentId)), i.e. the environment the URL names, and the failure mode is unchanged and fail-closed (a JWT-shaped bearer that does not verify yields hard anonymous, no cookie fallback). - No second gate is left in the same half-state:
enforceAuthGateclassifies by allow-list (isAuthGateAllowlisted,/auth/segment,/health-style suffixes) and applies to every other path regardless of head segment, andenforceProjectMembershipkeys on the resolvedenvironmentId, not on the path head — so a scoped data/MCP request that now reaches its domain is gated by both.
Verification run for this review (exit codes captured before any pipe)
- Pin at head: exit 0, 7/7. Ablation (merge-base blob): exit 1, 4 failed / 3 passed. Restore: blob-hash equal, tree clean.
- Runtime regression at head, same selection the dev ran (
http-dispatcher*,dispatcher-plugin*,security/*,error-envelope.conformance,auth-unknown-subpath.hono.integration): exit 0, 39 files / 714 tests passed. @objectstack/honoadapter suite at head: exit 0, 2 files / 74 tests passed (run afterturbo run build --filter=@objectstack/runtime, 30/30 tasks, since the adapter resolves runtime throughdist).- A first pin attempt in the fresh worktree answered "no tests" (
@objectstack/observabilityhad nodist): recorded as PREREQUISITE NOT MET, not as a result; the dependency closure was built (turbo run build --filter='@objectstack/runtime^...', 29/29 tasks) and the pin re-run. - CI on
4538d6289d5: all 30 completed check runs are success or skipped; the six required checks (TypeScript Type Check, Lint & Repo Gates, Test Core, Dogfood Regression Gate, Build Core, Temporal Conformance) are green. This is a reading of CI, not a re-run.
NOT MEASURED (reported as such, not as a pass):
- Whether any
objectstack-ai/cloudKernelResolverparsescontext.routePathfor/projects/.routePathis handed to the host resolver unstripped, so a cloud-side parser is the one reader outside this repository that could make the retired prefix a working alias on those hosts. Recommend #15861 ask that question explicitly, alongside "does any host still emit it". Per ADR-0006 the answer cannot re-introduce an alias here, so it is residual risk for the cloud side, not an input to this PR's shape. - The dev's end-to-end probe through
createHonoApp+app.request(): not re-run. The catch-all's single contributing line was verified by reading and the pin reproduces it exactly. - The 53-family gate union and the
tsc -p tsconfig.test.json --listFilesinclusion of the new test file: not re-run here; CI type-check and lint are green on the head.
Residual risk, not re-litigated
The cross-repo emitter question is ruled (C, #15861). What this review adds is only the sharper reading above, and the observation that any cloud host still emitting the retired prefix is already served from the wrong environment today.
Generated by Claude Code
Fixes #15488
The card asked which of three readings is true: stale prose, a live routing defect, or a strip that should accept both spellings. It was driven, not read.
The answer: reading 1 — a live routing defect
Measured through the real
@objectstack/honocatch-all (createHonoApp, unmocked, realHttpDispatcher,app.request()), oncc5b3dd0c27, before any edit. Ten requests sent; the probe printed its own send count so an empty run could not read as a clean one.GET /api/v1/health— CONTROLGET /api/v1/data/task— CONTROLSERVICE_UNAVAILABLE(the/datadomain ran)GET /api/v1/no-such-domain— NEGATIVE CONTROLROUTE_NOT_FOUNDGET /api/v1/environments/env_alpha/healthROUTE_NOT_FOUNDGET /api/v1/environments/env_alpha/readyROUTE_NOT_FOUNDGET /api/v1/environments/env_alpha/data/taskROUTE_NOT_FOUND/dataGET /api/v1/projects/env_alpha/healthROUTE_NOT_FOUNDThe negative control matters: an unclaimed path and a scoped path answered the same 404 shape before the repair, which is what "matches no dispatcher domain" looks like from outside.
A second probe read the mutated context back, and it is the sharper half — the two spellings fail in opposite directions:
So
dispatch()'s own two readings of one convention disagreed with each other, in this repository, with no cloud-side wiring needed to decide it.extractEnvironmentIdFromPathparsed/environments/; the strip removed/projects/. The card's reading 2 ("dead legacy, fix the comment") is refuted, and so is the triage comment's reading 3 ("both spellings are meant to work") — see below.Why the catch-all specifically. It is the only entry that hands
dispatch()a still-scoped path:const subPath = c.req.path.substring(prefix.length). Every scoped mount indispatcher-plugin.tspasses a pre-stripped subpath —registerAutomationRoutesmounts${prefix}/environments/:environmentId/automationand dispatches the literal/automation— which is why the standalone server never showed this.The legacy prefix was not a working alias in exchange. Nothing parses
/projects/:id, so stripping it discarded the only place the request named an environment: it was served from the host default, not the environment its own URL named. A 404 is the honest answer, and that is what it answers now.Why one spelling, not an alternation
The triage comment's reading 3 would widen the regex to accept either prefix. That is exactly what the standing ruling forbids, and the four axes agree, led by long-term rationality:
environment/environmentswith no aliases, and the ADR states the posture in as many words: "no gradualism, no dual-spelling interval, no single release carrying both." An alternation here is precisely the alias it exists to prevent./projects/:idAPI URL survives: the client SDK builds scoped bases as.../environments/:id,rest-server.tsmounts/environments/:environmentIdat every one of itsisScopedsites, anddispatcher-plugin.tsmounts the same. The one remaining hit is a UI-location fixture in an unrelated websocket test.content/docs/api/environment-routing.mdxalready tells callers "Replace/api/v1/projects/:projectId/...with/api/v1/environments/:environmentId/..." and "there is no alias, so the old spelling does not resolve." Honouring the published checklist costs nothing; contradicting it costs a compatibility surface.The repair — four sites, all located by content
Three in
packages/runtime/src/http-dispatcher.ts, one in a sibling:/^\/projects\/[^/]+(\/.*)?$/becomes/^\/environments\/[^/]+(\/.*)?$/. The card's subject.acceptOAuthAccessTokentested the still-scoped path against the same retired prefix. This had to move with the strip, and could not have been left for a follow-up: unrepaired, a scoped/mcpURL never reached the MCP domain at all, so the flag was unobservable; repairing the strip is exactly what makes the line reachable, and left behind it would have started admitting scoped MCP callers to the domain with their OAuth 2.1 access tokens refused. A new defect manufactured by the fix.extractEnvironmentIdFromPath, describing a "project UUID" and the retired URL form. Deleted; the live docblock immediately beneath it already says the right thing.packages/runtime/src/security/permission-denied-envelope.ts—routeObjectFromPath's docblock states what it expects of the dispatcher's cleaned path, and said "/projects/:environmentIdprefix stripped". Prose only; the function's own/^\/data\/([^/?#]+)/is untouched.Sites 2 to 4 are declared in-scope deliberately rather than swept in quietly: same defect class, mechanical, the correct spelling pinned by the ADR and by site 1, no new gate family (re-derived and diffed — identical list). The claim comment on the card names the file surface.
What this branch did NOT touch.
packages/rest/src/rest-server.tscarries two more docblocks naming the retired form, and that file is held by two open pull requests — #15673 and #15395. The sweep stopped there and filed the finding as #15858 instead of reaching into a file two branches are rewriting.The pin, and the ablation that proves it can fail
New:
packages/runtime/src/http-dispatcher.scoped-url-strip.test.ts— 7 assertions, including the two controls and the negative control from the table above, and an observation seam that captures theacceptOAuthAccessTokendecision.Both ablation limbs had their direction and their green assertions predicted in writing first, then measured. Each mutation was proven on disk by removed-text and injected-marker counts before the run, and restored under a trap via
git checkout HEAD -- <absolute path>, proven by blob-hash equality against the HEAD blob and an emptygit diff HEAD.⭐ The assertion that deliberately stays GREEN under limb A is the finding restated as a test: "parses the environment id off the SAME prefix it strips" keeps answering
env_alphawhile the strip is broken, becauseextractEnvironmentIdFromPathis a separate reading. That independence is exactly how the two drifted apart unnoticed for a release, and an ablation that reddened it would be measuring the wrong thing.No rebuild was needed, and that is proven rather than assumed. The pin imports
./http-dispatcher.js— a relative import inside the package, resolved to source. To make the claim falsifiable, each limb printeddist/index.jsstill holding the repaired spelling at mutation time: a RED pin therefore proves source resolution, and a GREEN one would have proved the suite was reading dist.Verification — all on the final head
4538d6289d5node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commandson that head with no STALE TREE banner, re-derived after the change set was final and diffed against the executed list (identical). Two of them (check:dual-build-cjs-loads,check:type-check-debt) first answeredPREREQUISITE NOT MET/ exit 3 — read as NOT MEASURED, not as a pass — and were re-run green afterturbo run buildover the workspace closure. Exit codes captured after redirection, never through a pipe.http-dispatcher.*,dispatcher-plugin,error-envelope.conformance,auth-unknown-subpath.hono.integrationandsecurity/*suites in@objectstack/runtime, plus@objectstack/hono(2 files / 74).@objectstack/runtimegreen, and the new test file was confirmed inside the type-checked program viatsc -p tsconfig.test.json --listFiles(1 hit, 0 errors attributable to either changed file) rather than inferred from a green script.pnpm lint=eslint . --no-inline-config, green in 90s — no narrowing claimed or needed.For triage
The triage comment set a promotion rule: "if reading 1 or 3 is established, re-grade to p2 immediately — no new card, no re-argument." Reading 1 is established by driven measurement, so that rule has fired.
The same comment's boundary test applies too: this changes what the door accepts — requests that reached no dispatcher domain now reach one, and the retired spelling now refuses — so it reads as clause ② YES and a manual floor. ⛔ Left as a draft for that judgement rather than pre-labelled.
One observation handed back rather than acted on: the card called this "the third independent reason the dispatcher cannot serve the scoped
/packagespath", alongside two measured by #14503's census. This branch removes this one — a scoped/packagesURL now reaches the/packagesdomain through the catch-all — which may change how #14503's other two findings read. That is #14503's question, not this card's.🤖 Generated with Claude Code
https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
Generated by Claude Code