fix(rest): a scoped /discovery advertises routes.auth on the unscoped base - #16957
Conversation
No test had ever touched `routes.auth` on a scoped `/discovery` document. The three existing pins live in `packages/objectql/src/protocol-discovery.test.ts` and measure the PRODUCER, which never sees a base path — so the REST projection's scoped branch was unmeasured, and that is why the defect was green. This commit adds the failing case only; the repair follows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
…#16538) `registerDiscoveryEndpoints`' auth branch stripped only the retired `/projects/:environmentId`, while `isScoped` — the condition guarding that branch — is true only for `/environments/:environmentId`. The replace could therefore never match where it ran: it returned `basePath` unchanged, so a scoped `/discovery` advertised `routes.auth` as `/api/v1/environments/:environmentId/auth` — keeping both the scope the comment two lines above says to drop ("Auth is a control-plane concern, so use the unscoped base") and a literal, unsubstituted route parameter. The repair is the sibling MCP block's own regex, in the same handler and for the same reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
…epair (#16538) Measured rather than assumed: `@objectstack/rest` is public (npm 17.3.0) and ships `files: ["dist", ...]`; the changed regex is present twice in both `dist/index.js` and `dist/index.cjs` — the sibling MCP strip as the positive control, and the auth strip this change moved — while the retired-only spelling has zero occurrences there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
📓 Docs Drift CheckThis PR changes 1 package(s): 8 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 13 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 745bcabfbb090489f9e836af47665d57c2c8553e && git checkout 745bcabfbb090489f9e836af47665d57c2c8553e
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 97adce2faa9d27d7811f1f299f5ed806a467f624 d5d014784f861eade5c9a4fb8d1344a430b7e93a && git checkout -B drift-repro 97adce2faa9d27d7811f1f299f5ed806a467f624 && git merge --no-ff d5d014784f861eade5c9a4fb8d1344a430b7e93a
node scripts/docs-audit/affected-docs.mjs --json 97adce2faa9d27d7811f1f299f5ed806a467f624
|
PM review — accepted, armingCI. 33 check-run rows on head Clause-② re-run by me. ⭐ The RED leg is genuine, and I checked the thing that would have made it fake. A RED/GREEN pair only means something if the RED commit truly lacked the fix. Verified:
On the declared deviation — accepted, and the defective input was mine. Ruled in full on the card (#16538, comment ⭐ And that yields a fact worth keeping: the reason no test had ever covered this was deeper than the card said. The card said the existing pins all sit on the unscoped base; the stronger truth is that they sit on an object with no scoped/unscoped notion at all. Docs drift — one gap closed by measurement rather than assumption. The bot computed on the merge commit (base What I accept on the delivering seat's measurement rather than re-measuring: the Marking ready for review, then arming auto-merge, in that order. Generated by Claude Code |
Fixes #16538
Clause-②: no
— re-declared from the delivered diff, not inherited. The diff changes one regex alternation inside a private method body, adds no member to any schema, export, config key or wire field, and widens no authorable surface. The value it replaces,
/api/v1/environments/:environmentId/auth, is not a URL: it carries an unsubstituted route parameter, so no consumer can be following it usefully. The only other files are a newdescribeblock in an existing test and apatchchangeset.The defect
registerDiscoveryEndpointsholds two siblingunscopedBasecomputations eleven lines apart. Both strip the scoped segment offbasePath; only one of them named the current spelling.isScopedis true only for/environments/:environmentId, so the auth strip named the one spelling that can never appear on the branch it guarded.replacereturnedbasePathunchanged:The contract it violates is stated two lines above the computation, and the repair needed no invention — the sibling MCP block, same handler and same purpose, already carried it:
AFTER — the auth branch now names both spellings, exactly as the MCP sibling does:
The deliverable is the test that did not exist — RED, then GREEN
Triage was explicit that the regex alone is not acceptance:
The new case serves the real scoped route through the router the server registered, and carries its own control so a green cannot come from the unscoped branch having been taken instead.
RED, at
9dcb38cf2c(test committed, fix not yet applied), on this harness:GREEN, at
3e13dc14a4(same file, same harness, same command):Command both times:
pnpm --filter @objectstack/rest exec vitest run --maxWorkers=2 src/discovery-per-request-protocol.test.ts.The second
itin the block pins the unscoped document's auth route. It is green on both sides and is labelled in-file as a regression guard rather than the reproduction — it exists so the repair cannot be paid for out of the other branch of the same computation.The dispatch fenced the file surface to
packages/rest/src/rest-server.tsandpackages/objectql/src/protocol-discovery.test.ts, and asked that anything else be reported before it was written. This reports it: the named test file cannot host the required test, measured, and the scoped pin went intopackages/rest/src/discovery-per-request-protocol.test.tsinstead.The premise behind that file choice is that the three existing
routes.authpins sit where a scoped pin could join them. They do not measure the same thing:protocol-discovery.test.ts:47,:100,:461) exercise the producer,ObjectStackProtocolImplementation.getDiscovery(), which is handed no base path at all and has no scoped/unscoped notion. That is a second reason no test had ever touchedroutes.authon a scoped document, beyond the one the card named.rest-server.ts.@objectstack/restis not reachable frompackages/objectql:require.resolve('@objectstack/rest', { paths: ['packages/objectql/src'] })answersMODULE_NOT_FOUND, andpackages/objectql/node_modules/@objectstack/holdscore formula metadata metadata-core metadata-protocol spec typesand norest. Declaring the dependency would need a third file (packages/objectql/package.json) and would close a workspace cycle, since@objectstack/restalready devDepends on@objectstack/objectql.The chosen home is in the same package as the fix, is the file that already boots a
RestServerwithenableProjectScoping: trueand serves the scoped/discoveryroute, and #9292's half is untouched: the diff on it is 62 insertions and 0 deletions, all three#9292markers still present.packages/objectql/src/protocol-discovery.test.tswas not modified.#15488
Read, as asked. Nothing to batch — it has already landed, so there was no honest way to fix both here and the surface was not widened for it. On current
main:packages/runtime/src/http-dispatcher.tsstrips with/^\/environments\/[^/]+(\/.*)?$/, not the/projects/form the card recorded./^(?:\/environments\/[^/]+)?\/mcp(?:[/?]|$)/, under a comment naming that repair.completed(2026-09-05).Changeset — measured, not guessed
A
patchchangeset for@objectstack/restis included.skip-changesetwould have been wrong in both halves of the test:@objectstack/restis published —privateis absent, and the npm registry answers200withdist-tags.latest = 17.3.0across 133 versions.files: ["dist", "README.md", "CHANGELOG.md"]actually ships. Afterpnpm --filter @objectstack/rest build, bothdist/index.jsanddist/index.cjscontain the changed alternation twice, and the shipped lines read:Positive control: the first of those two is the pre-existing MCP strip — untouched by this diff, already published, and it proves the grep and the
files[]path are live rather than silently matching nothing. Negative: the retired-only spelling has 0 occurrences indist/index.js.And this changes behaviour, not only bytes: what a scoped
/discoveryreturns forroutes.authis different after this diff.Docs drift — re-derived, and hand-swept against a control
Tool run, from a clean worktree —
computedOn.dirty: false, headd5d014784f, diffBase5abca1792e:node scripts/docs-audit/affected-docs.mjs --jsonlists 9 docs / 4 anchors (1 symbol, 3 route). This is the wide list the dispatch predicted, and all 9 rows are broad-anchor artifacts. Per page:api/environment-routing.mdx/environments/:environmentId+/projects/:environmentIdauthroute and no discovery document.concepts/north-star.mdx/environments/:environmentIddeployment/publish-and-preview.mdx/environments/:environmentIddeployment/single-project-mode.mdx/environments/:environmentIdprotocol/kernel/http-protocol.mdx/environments/:environmentIdGET /api/v1/discovery) and both show"auth": "/api/v1/auth"— the value this fix now also delivers on the scoped route. Not falsified; agreed with before and after.protocol/kernel/metadata-service.mdx/environments/:environmentIdprotocol/kernel/plugin-spec.mdx/projects/:environmentIdui/forms.mdx/environments/:environmentIdreleases/implementation-status.mdxregisterDiscoveryEndpoints(symbol) +/environments/:environmentId/api/v1/discoveryand makes no claim about the scoped auth route. Not falsified, so nothing filed and nothing edited.Hand sweep of
content/, because a tool list is not a clean bill either way. Positive control first:/api/v1/discoveryhits 9 files, so the sweep is live.routes.auth— 2 hits, and neither is in the tool's list, which is exactly the anchor blind spot to expect here: neither page names a changed route literal or symbol. Read both.kernel/services-checklist.mdx:609states- `routes.auth` → `"/api/v1/auth"` appears in routes— the doc already described the post-fix behaviour, and the code was the half that disagreed.permissions/authentication.mdx:1284discussesroutes.authbeing absent when no auth service is registered, which this diff does not touch.environmentId— 46 hits;unscoped— 47 hits. Both scanned; none states a scoped auth route.environments/.../authandprojects/.../authhave 0 occurrences anywhere incontent/, and every"routes"sample in the tree is an unscoped document. So no page documented the behaviour this diff changes.Conclusion: zero pages falsified, zero doc edits, nothing filed against the release-owned page.
Verification
All at head
d5d014784funless noted.node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack→ 57 commands; all 57 run; reconciled with--ran:✓ 57 derived famil(ies) accounted for — 57 run, 0 NOT-MEASURED. 55 exit 0.PREREQUISITE NOT MET— the gates' own word for "nothing was measured", not a finding. Both need the whole-package build closure thatlint.ymlbuilds before its step, which is CI's farm run and is declared to it here:pnpm check:dual-build-cjs-loads— "this gate reads built output, and some package has no dist/ … ⛔ This is NOT a pass: nothing was measured." (43 packages unbuilt.) Its--self-testleg passed, 93 cases.pnpm check:type-check-debt— its coverage half passed (OK — 76/80 workspace packages type-checked); only the--re-measureleg refused, needing 6 dependencies' builtdist/*.d.ts.pnpm --filter @objectstack/rest typecheck→ exit 0, and itscheck:test-typecheckleg confirms the test layer itself compiles (0 file(s) / 0 error(s)), so the new test is not outside the type program.pnpm --filter @objectstack/rest test→ 184 files / 3059 tests passed,VERDICT command-exit 0.eslint . --no-inline-config --format json→ 6384 files linted, 0 errors, 0 warnings, exit 0 (2m14s). The narrowed run over the two touched source files is 2 files, 0/0. The repo enables no type-aware linting anywhere (eslint.config.mjssays so in as many words: "this repo runs oneeslint.config.mjs, which never enables type-aware linting (noparserOptions.project, no typed@typescript-eslintrules) for ANY file"), so this diff could not have moved a verdict on an untouched file — but the whole population was measured regardless.grep -naPover the C0/DEL ranges → 0 hits.Acceptance notes
routes.authon a scoped deployment; triage froze the card at p3 with a written re-grade trigger. Nothing in this PR measured that either, in either direction.Generated by Claude Code