Uh oh!
There was an error while loading. Please reload this page.
test(rest): enumerate metadata write doors on every mount base, not just /api/v1 - #12445
Merged
Merged
Conversation
The #8919 anti-drift assertion derives the metadata write-door list from the composed server's own route table, but filtered it on a literal `/api/v1/meta` prefix. `RestServer.registerRoutes` calls `registerForBase` once per base, so with `api.enableProjectScoping` on the same doors are also mounted at `/api/v1/environments/:environmentId/meta/...`, which no `/api/v1/meta` prefix can match. Because the only boot in the file was an unscoped one, the filter was complete for that boot and the blind spot was invisible from inside the assertion it narrowed. Match the `meta` path segment instead and build the expectation per mounted base, then boot the two other compositions the server can be configured into. Measured before the change, doors derived vs doors mounted: default 5/5, `auto` 5/10, `required` 0/5 — under `required` the closed-set claim was being asserted over the empty set. Adds one routing probe on the scoped mount (anonymous, capability-less, capable) so the scoped boot actually routes a request rather than booting and measuring nothing. The refusals themselves are structurally shared: `registerMetadataEndpoints` wraps the registrar around `registerMetadataEndpointsInner(basePath)`, which is the same body on both passes, so there is no seam at which the two mounts could diverge. Test-only; no runtime change. `rest-server.ts` is read, not edited. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd
…oped-meta-mount-coverage
Contributor
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
This was referenced Aug 26, 2026
os-litant
marked this pull request as ready for review
August 26, 2026 04:09
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#11473
Test-only. No runtime change.
packages/rest/src/rest-server.tsis read, never edited — it is fenced by PR #12421.The scope question the card asks first, and the answer
The card does not ask for a fixture; it asks whether one is worth having:
Measured on this checkout,
enableProjectScoping: trueappears in 22 places. Every one is either a test, a doc-comment example, or a published doc:packages/resttest filespackages/clienttest files (2 real scoped boots, 1 docstring), plus a docstring inpackages/client/src/index.tsproject(id)factory exists for scoped serverspackages/spec/src/compose-stacks-key-loss.test.tspackages/cli/src/utils/merge-boot-config.test.tsfalsepackages/cli/src/commands/serve.ts(comment)api.enableProjectScopingand forwards it to REST + dispatchercontent/docs/api/environment-routing.mdxos:checksnippetZero non-test, non-doc sites turn it on. So the flag is genuinely off-by-default here, and the standalone boot path forces it off on purpose —
createStandaloneStack()returnsapi: { enableProjectScoping: false, projectResolution: 'none' }andmergeBootConfiglets the boot builder win that key ("scoping is not the author's call on a standalone host"). A host-shaped config (isHostConfig→shouldBootWithLibraryfalse) still reaches the plugin with the author's value intact, andserve.tssays outright that "Cloud / multi-environment boot modes live in a separate distribution", which is the distribution where this is on.That measurement decides which of the card's two resolutions to take, and it decides against the more expensive one:
The defect
#8919's file declares that "the metadata write doors are a CLOSED, enumerated set", and earns that by DERIVING the door list from the composed server's own route table rather than from a hand-written list. The derivation filtered the route table on a literal/api/v1/metaprefix.RestServer.registerRoutescallsregisterForBase(...)once per base.registerMetadataEndpointsis one of the registrars it calls, so withapi.enableProjectScopingon, the whole door set is mounted a second time under/api/v1/environments/:environmentId. No scoped door path starts with/api/v1/meta.The filter's blind spot was therefore precisely the population the filter exists to enumerate — and it was invisible from inside the assertion, because the only boot in the file was an unscoped one, for which the filter is complete.
Measured, doors derived by the old prefix filter vs doors mounted, on the three compositions the server can be configured into:
enableProjectScopingabsent →false)enableProjectScoping: true,projectResolution: 'auto'enableProjectScoping: true,projectResolution: 'required'The
requiredrow is the sharp one:registerForBaseis called with the scoped base and nothing else, so a closed-set claim was being asserted over the empty set while five real write doors were live.The change
One file,
packages/rest/src/meta-write-door-capability-enumeration.test.ts:metapath segment instead of an/api/v1/metaprefix, and the expectation is built per mounted base, so the closed-set claim is checked against every mount the composition brings up. The matcher is a regular expression that acceptsmetawhen it is preceded by a slash or the start of the path and followed by a slash or the end of the path.boot()takes an optional composition, so the two scoping-on route tables can be enumerated.Why the probe is three cases and not a second copy of the matrix
registerMetadataEndpointsswaps in a guarded registrar, callsregisterMetadataEndpointsInner(basePath)inside the swap, and restores it in afinally. Both passes run that same body, and the per-door capability gates live in that same body. There is no seam at which the two mounts could carry different gates — so duplicating fifteen refusal cases onto the scoped base would buy repetition, not coverage. What is not structural is whether a request addressed to a scoped path is routed at all, and that is what the probe measures. Measured on the scoped mount, all five doors: anonymous → 401 with the flat anonymous-deny envelope, protocol call count 0; authenticated without capability → 403FORBIDDEN, protocol call count 0;manage_metadataholder → 200, protocol call count 1. The capable control is what makes the two refusals readable as decisions rather than as an unrouted path.Bounded in-place correction, named because it is not what the card asked for
The
DOORSdocstring said "The six metadata write doors". The table has held five since#12195retired the compound-name save — that card removed the entry and the comment right below it explains the removal, but the count in the sentence above was not touched. The docstring now says the count is the table's own length and records why. Evidence is the table itself and#12195's own note inside it; no behaviour is involved.LATENT, not live
enableProjectScopingdefaults tofalse(rest-server.ts:api.enableProjectScoping ?? false), so a default deployment mounts none of the scoped doors and an anonymous probe of one 404s withENDPOINT_NOT_FOUND. The filer's own words: "a coverage observation, not a measured hole." This PR closes a coverage gap in an anti-drift assertion. It does not close a reachable hole, and nothing here should be read as one.Verification
Gate union run at
d5e52b309, the final commit on this branch.Ablation — the pin fails when the thing it guards is removed.
META_SEGMENTwas reverted to the pre-change prefix form, the mutation was confirmed on disk by occurrence counts (removed-text 1 → 0, injected-text 0 → 1) and a changedgit hash-object, and the run went red in the predicted direction:The 48 that stayed green include the original
#8919default-boot assertion — which is the whole point: the prefix filter was complete for that boot, so the old assertion could not see its own blind spot. Restore wasgit checkout HEAD --against the file spelled as an absolute path under anEXIT INT TERMtrap, proven byte-identical to the HEAD blob andgit diff HEADempty. Nodist/is involved: the file under test imports./rest-server.js, resolved to source by vitest.Green run, same file, unmutated:
Test Files 1 passed (1) · Tests 50 passed (50).pnpm --filter '@objectstack/rest^...' buildpnpm --filter @objectstack/rest testTest Files 147 passed (147) · Tests 2345 passed (2345)pnpm --filter @objectstack/rest typechecktsconfig.jsonexcludes**/*.test.ts, so it says nothing about the edited filetsc --noEmit --listFilesover that packagerest-server.ts1 hit; a term known absent 0 hits). 0 errors attributed to the edited file; program total 155, equal to the recordedTEST_DEBT['@objectstack/rest'] = 155, so the shrink-only ratchet is unmovedpnpm lint(eslint . --no-inline-config, whole repo)pnpm check:nul-bytesOK (scanned 6881 text file(s) ... no raw ASCII control bytes)pnpm check:cross-package-test-inputsOK: 18 package(s) read outside themselves, all declaredpnpm check:test-source-aliasOK — 72 packages with tests scannedpnpm check:engine-double-contractOK — 416 pinned, 133 in the DEBT ledger, 2 exemptpnpm check:where-matcher303 matcher(s) discovered, 303 answer the combinator battery correctly or refuse it loudlypnpm check:type-check-coverage,check:query-options-erasure,check:slot-lookup,check:type-source-resolution,check:published-files,check:page-declaration-shape,check:dispatcher-error-vocabularynode scripts/check-ci-filter-parity.mjs,check-comment-mask-adoption.mjs,check-plugin-teardown-shape.mjsFamilies re-derived in this worktree with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackover the actual changed file, not from the dispatch list.check:type-check-debt --re-measurewas not run locally (it needs the whole workspace closure built); the only ledger entry apackages/resttest edit can move is@objectstack/rest, and the row above measures it at the recorded number with zero errors in the edited file.No changeset: this diff adds no user-visible behaviour and ships nothing — test files are not published.
skip-changeset.Generated by Claude Code