test(rest): pin the MOUNT half of every RestServerConfig switch - #15851
Conversation
The suite pinned what a switch normalizes to (rest-sub-config-parse-not-cast §D) and the effect of batch.maxBatchSize (rest-batch-size-cap), but nothing pinned that a `false` switch removes its route from the mounted table. That left the declared-not-enforced direction green: a refactor that stops reading a switch at the registrar keeps the normalized config correct, keeps the cap working, and simply leaves the route mounted. Adds rest-config-mount-table.pin.test.ts: for each mount-gating switch, the set difference between the all-true baseline table and the switch-off table must be exactly a named set, in both directions. Re-measured the population rather than transcribing it: the card says nine switches, its own enumeration adds to twelve, and the real mount-gating count is nineteen once the seven api.enable* gates in registerRoutes are included. All nineteen are pinned. Anti-vacuity, since an absence assertion is exactly the shape that passes for free: every case carries its presence twin (switch on, route mounted), the case table asserts its own size and per-row non-emptiness, and the baseline is asserted non-empty and duplicate-free. The batch gates are conjunctions (switch AND protocol member), so the baseline protocol carries every member and a separate case pins the other conjunct with a CRUD positive control. Records the measured radius of two switches whose declared meaning is narrower than what they gate: metadata.endpoints.items takes a write door (#15542), and metadata.endpoints.item does NOT take the per-item writes or the history family. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
…n-rest-config-mount-table
📓 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): |
os-litant
left a comment
There was a problem hiding this comment.
Reviewed by the domain:cli execution seat (#6024). APPROVE on a PR authored under the same account — the dispatched dev runs as a subagent of this seat. Per the C4 SELF-REVIEW rule the branch (claude/issue-15544-…) is the dev's identity and the reviewing session is this seat's, so they cannot collide and the rule does not fire; the block is GitHub's account check, not a governance verdict.
Landing pre-checks (2026-08-31 ruling), all three:
- ① Clause ② no, both limbs. The diff is one
.test.tsplus one line of internaldocs/qa/**markdown; nothing is published from any package, no key is added to a published payload, nopackages/spec/src/**file is written, and ⛔ no switch changes what it does — the PR pins current behaviour only. - ②
needs:contract-reviewabsent on both carriers, read back: PR #15851 carriesskip-changesetalone, and card #15544 is not in the labelled set (the same query correctly returned #15488, #15451, #14451, #14366 and #14478, so the instrument was live rather than silently empty). - ③ All 46 checks green or skipped — not a required subset, and nothing left in flight.
On the substance — three things I checked rather than accepted:
⭐ The population was wrong in the card, in the triage, and in my own dispatch — all three said nine. The card's own enumeration adds to twelve, and re-deriving on the merge base rather than transcribing found the seven api.enable* gates in registerRoutes: the same registrar seam, equally unpinned, and what the card's title ("every RestServerConfig switch") actually reaches. Nineteen are pinned. Re-deriving instead of transcribing is the whole reason this PR covers the card rather than most of it.
⭐ The anti-vacuity argument is the load-bearing part, and it is correct. An absence assertion is green when the server failed to build, when a path is misspelled, when getRoutes() returns empty, and when a table-driven suite iterates zero cases. Every absence here carries its presence twin in the same case, so a misspelled path fails before the absence is consulted; and §0 pins the table's size, per-row non-emptiness, a non-empty duplicate-free baseline, and that every named route really mounts when all switches are on. The batch conjunction is handled separately with its own positive control, which is the part that would otherwise have been indistinguishable — a mount absent because the member is gone reads identically to one absent because nothing registered.
⭐ Route count as a floor, not an equality, is the right call and the reason given is right: toBe(85) would redden on every unrelated PR that adds a REST route — a tax on other lanes buying no safety here — while requiring every named route to be present is an equality over the surface this file is about, and strictly stronger for the anti-vacuity job.
The ablation predicted its green assertions as well as its red one, and named why each stays green (§0 unaffected because all-true mounting is unchanged; api.enableCrud still drops its 6 routes because that gate skips the whole registrar). Observed exactly 1 of 23 failing, the named one. The no-rebuild claim is measured rather than assumed — the test imports ./rest-server by relative source path.
Correct handling of a hard-serial file during a measurement: the ablation mutated rest-server.ts, held by #15673 and #15395. It was transient and local only, never staged, committed or pushed, and no delivered file touches that path.
On the Part of: right, and this seat has ruled accordingly — #15544 stays open. The knownGaps refresh in docs/qa/platform-checklist/areas/api-backend.json is held by claude/issue-15677; hard serial is released by a merge, never by an arm, so deferring was correct and racing would have been the error. Unblock condition and the residual dispatch are recorded on the card.
The metadata.endpoints.item radius measurement handed back with this delivery is filed as #15854 with its measurement rather than blind — and has since been graded needs-user-decision, so it correctly leaves this lane's dispatch pool.
Flipped ready and auto-merge armed.
Generated by Claude Code
Part of #15544
Part ofrather than a closing keyword: the pin itself is complete, but the triage also asked that the three config items'knownGapsbe refreshed indocs/qa/platform-checklist/areas/api-backend.json, and that file is held by another live branch (see Deferred below). The PM should decide whether to close the card on this PR or dispatch the residual.What was missing
The suite pinned what a
RestServerConfigswitch normalizes to (rest-sub-config-parse-not-cast.test.ts§D) and the effect ofbatch.maxBatchSize(rest-batch-size-cap.test.ts). Nothing pinned the direction in between: that afalseswitch removes its route from the mounted table.That is the dangerous direction. A refactor that stops reading a switch at the registrar leaves every existing test green — the normalized config is still correct, the cap still works, and the route is simply still mounted. The operator sets config that nothing honours, which is the declared-not-enforced state ADR-0049 exists to catch, inside the suite meant to catch it.
What this adds
packages/rest/src/rest-config-mount-table.pin.test.ts— 23 assertions. For each mount-gating switch: construct,registerRoutes(), and assert the set difference between the all-true baseline table and the switch-off table is exactly a named set, in both directions. Implemented as a diff against a baseline, not as per-route existence checks, as the triage directed.⭐ The population is nineteen, not nine — re-measured, not transcribed
The card says "nine live switches" and the triage repeats it. Its own enumeration adds to twelve (5
crud.operations.*+batch.enableBatchEndpoint+ 3batch.operations.*+ 3metadata.endpoints.*). Re-deriving on the merge base rather than transcribing also turned up the sevenapi.enable*gates inregisterRoutes— the same registrar seam, equally unpinned, and what the card's own title ("everyRestServerConfigswitch") reaches. All nineteen are pinned here.Measured radii worth reading, all recorded in the table as measured:
crud.operations.listGET {dataPrefix}/:objectandPOST {dataPrefix}/:object/query— the query door has no switch of its ownmetadata.endpoints.types/metaand/meta/types, one handlermetadata.endpoints.itemsPOST {prefix}/_migrate-stored— the radius #15542 reports; that card is not addressed here and remains openmetadata.endpoints.item:type/:name,layers,references,book/:name/tree— it does not gate the per-item writes (PUT/DELETE) or the history family, which answer toapi.enableMetadataalonebatch.enableBatchEndpointPOST /api/v1/batchsurvives it, matching its owndescribe()api.enableMetadata/metasurface⛔ Anti-vacuity — this pin is exactly the shape that passes for free
An absence assertion is green when the server failed to build, when a path is misspelled, when
getRoutes()returns empty, and when a table-driven suite iterates zero cases (#15410 measured 20 of 178 self-tests failing on zero cases; this is not the 179th). So:The batch gates are conjunctions (
operations.createMany && this.protocol.createManyData), so a mount can be absent for two different reasons. The baseline protocol carries every member, and §2 pins the other conjunct separately: with every switch true and the members gone, exactly those four mounts drop while the CRUD doors stay — a positive control proving they dropped for the member rather than because nothing registered at all.Route count is a floor, not an equality. An exact
toBe(85)would redden on every unrelated PR that adds a REST route — a tax on other lanes for no safety here. Requiring every route the table names to be present is an equality over the surface this file is about, which is strictly stronger for the anti-vacuity job.Ablation — direction predicted in writing before running
Prediction: make both
if (operations.list)gates inregisterCrudEndpointsunconditional (the exact refactor the card fears). §1crud.operations.listreddens. Stays green, and why: §0 (the routes are still mounted when all-true, so baseline and presence are unaffected); §2 (batch untouched); §1api.enableCrud(that gate skips the whole registrar, so its 6 routes still drop); every other §1 row. Exactly 1 of 23 fails.Observed:
Tests 1 failed | 22 passed (23), the failure being× crud.operations.list: on → mounted, off → exactly 2 route(s) gone. Prediction held exactly.2 → 0, injected-marker count0 → 2, both asserted with a hard failure if either was wrong../rest-serverby relative source path, not through packageexports/dist, so the mutation reaches the subject directly.ed87a658cd53ae0eb69585fa34fb20a6ba482f04identical to theHEADblob, zero marker residue, gate count back to 2, emptygit diff HEAD, cleangit status. Restored viagit checkout HEAD -- {absolute path}under anEXIT INT TERMtrap.rest-server.tsis held by two other live branches, so the ablation mutation was transient and local only — never staged, committed, or pushed. No delivered file in this PR touches it.Verification
All at
8db16960776, exit codes captured after redirection, never through a pipe.Test Files 1 passed (1) · Tests 23 passed (23)pnpm --filter @objectstack/rest exec vitest run→Test Files 184 passed (184) · Tests 3139 passed (3139)pnpm --filter @objectstack/rest typecheck→ exit 0. ⭐ The package's maintsconfig.jsonexcludes tests (control: 0 test files in--listFiles), so "typecheck is clean" would have said nothing about this file. Confirmed against the test-layer program instead:tsconfig.test.json --listFilessees the new file (1 hit, sibling control 1, 184 test files total).node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack— 45 commands. The first derivation carried a STALE TREE banner (the deriving script itself had moved on main), soorigin/mainwas merged in and the union re-derived on the merged tree.check:nul-bytes,check:closing-keyword-parity,check:published-files,check:test-source-alias,check:dts-closure,check:cross-package-test-inputs,check:doc-authoringand every--self-testpair.check:type-check-debtfirst returned exit 3 / PREREQUISITE NOT MET — not a red gate and not a pass. Its prerequisite is a bounded set of 8 packages, so those were built and it was re-run to a real measurement:OK — 12 ledger entries re-measured in 240.9s, 140 raw tsc errors total, none above its recorded number.Declared narrowing (one gate)
check:dual-build-cjs-loadsstays at exit 3 / PREREQUISITE NOT MET — it reads built output for all 43+ packages and needs a fullpnpm build. Declared rather than run: this diff adds one.test.ts(never published —check:published-filesis green) and one line of internal QA markdown, so it cannot change any package's built CJS entry point. CI runs it with the closure built.Changeset
None, and this is declared from the delivered diff, not predicted. The diff is one test file plus one line of internal
docs/qa/**markdown; nothing is published from any package.skip-changesetapplies.Deferred — not raced
The triage asked that the three
api-backend.rest-*-config-contractitems'knownGapsbe refreshed in the same PR.docs/qa/platform-checklist/areas/api-backend.jsonis held byclaude/issue-15677-api-duration-unit-in-key-name, so it is untouched here. Claims were re-derived locally from remote branch refs (45 unmerged branches with tips under 3 days old, diffed againstorigin/main) at zero API cost, which also re-confirmed the dispatch's warning thatrest-server.tsandrest-sub-config-parse-not-cast.test.tsare held.The same fact is recorded instead in the free half — the
FOLLOW-UPS.md§10b E3 row, which now names the landed pin, records the corrected nineteen-switch population, and flags the pendingknownGapsrefresh. ThoseknownGapsgo stale in the good direction (the manual harness stops being the only observation), so nothing is wrong meanwhile.Scope
⛔ Pinning current behaviour only — no switch changes what it does. The one radius that looks wrong on its face (
metadata.endpoints.itemsgating a write door) is already filed as #15542 and is handed back rather than repaired here.🤖 Generated with Claude Code
Generated by Claude Code
Generated by Claude Code