Skip to content

test(rest): pin the MOUNT half of every RestServerConfig switch - #15851

Merged
os-litant merged 2 commits into
mainfrom
claude/issue-15544-pin-rest-config-mount-table
Sep 5, 2026
Merged

test(rest): pin the MOUNT half of every RestServerConfig switch#15851
os-litant merged 2 commits into
mainfrom
claude/issue-15544-pin-rest-config-mount-table

Conversation

@os-litant

Copy link
Copy Markdown
Collaborator

Part of #15544

⚠️ Deliberately Part of rather than a closing keyword: the pin itself is complete, but the triage also asked that the three config items' knownGaps be refreshed in docs/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 RestServerConfig switch normalizes to (rest-sub-config-parse-not-cast.test.ts §D) and the effect of batch.maxBatchSize (rest-batch-size-cap.test.ts). Nothing pinned the direction in between: that a false switch 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 + 3 batch.operations.* + 3 metadata.endpoints.*). Re-deriving on the merge base rather than transcribing also turned up the seven api.enable* gates in registerRoutes — the same registrar seam, equally unpinned, and what the card's own title ("every RestServerConfig switch") reaches. All nineteen are pinned here.

Measured radii worth reading, all recorded in the table as measured:

switch routes gated note
crud.operations.list 2 GET {dataPrefix}/:object and POST {dataPrefix}/:object/query — the query door has no switch of its own
metadata.endpoints.types 2 /meta and /meta/types, one handler
metadata.endpoints.items 4 includes the write door POST {prefix}/_migrate-stored — the radius #15542 reports; that card is not addressed here and remains open
metadata.endpoints.item 4 :type/:name, layers, references, book/:name/tree — it does not gate the per-item writes (PUT/DELETE) or the history family, which answer to api.enableMetadata alone
batch.enableBatchEndpoint 1 only the per-object door; the cross-object POST /api/v1/batch survives it, matching its own describe()
api.enableMetadata 19 the whole /meta surface

⛔ 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:

  1. Every absence has its presence twin in the same case — with the switch on, each route it gates must be in the baseline. A misspelled path fails there before the absence is ever consulted.
  2. §0 asserts the case table's size and per-row non-emptiness, that the baseline is non-empty and duplicate-free, and that every route the table names is really mounted when all switches are on.

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 in registerCrudEndpoints unconditional (the exact refactor the card fears). §1 crud.operations.list reddens. Stays green, and why: §0 (the routes are still mounted when all-true, so baseline and presence are unaffected); §2 (batch untouched); §1 api.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.

  • Mutation proven on disk: removed-text count 2 → 0, injected-marker count 0 → 2, both asserted with a hard failure if either was wrong.
  • No rebuild leg needed, and this was checked rather than assumed: the test imports ./rest-server by relative source path, not through package exports/dist, so the mutation reaches the subject directly.
  • Restore proven: blob hash ed87a658cd53ae0eb69585fa34fb20a6ba482f04 identical to the HEAD blob, zero marker residue, gate count back to 2, empty git diff HEAD, clean git status. Restored via git checkout HEAD -- {absolute path} under an EXIT INT TERM trap.

⚠️ rest-server.ts is 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.

  • New pin: Test Files 1 passed (1) · Tests 23 passed (23)
  • Whole affected package: pnpm --filter @objectstack/rest exec vitest runTest Files 184 passed (184) · Tests 3139 passed (3139)
  • Typecheck: pnpm --filter @objectstack/rest typecheck → exit 0. ⭐ The package's main tsconfig.json excludes 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 --listFiles sees the new file (1 hit, sibling control 1, 184 test files total).
  • Gate union: re-derived after the change set was final with 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), so origin/main was merged in and the union re-derived on the merged tree.
    • 43 of 45 green, including check:nul-bytes, check:closing-keyword-parity, check:published-files, check:test-source-alias, check:dts-closure, check:cross-package-test-inputs, check:doc-authoring and every --self-test pair.
    • check:type-check-debt first 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.
  • Control-byte self-scan over both delivered files: no hits, with a non-zero positive control proving the scanner fires.

Declared narrowing (one gate)

check:dual-build-cjs-loads stays at exit 3 / PREREQUISITE NOT MET — it reads built output for all 43+ packages and needs a full pnpm build. Declared rather than run: this diff adds one .test.ts (never published — check:published-files is 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-changeset applies.

Deferred — not raced

The triage asked that the three api-backend.rest-*-config-contract items' knownGaps be refreshed in the same PR. docs/qa/platform-checklist/areas/api-backend.json is held by claude/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 against origin/main) at zero API cost, which also re-confirmed the dispatch's warning that rest-server.ts and rest-sub-config-parse-not-cast.test.ts are 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 pending knownGaps refresh. Those knownGaps go 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.items gating 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

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
@github-actions github-actions Bot added the size/m label Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing 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
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 5a21d73af58e6456b253163af8ffa072573a0771packageMentionDocs.

@os-litant os-litant left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed by the domain:cli execution seat (#6024). ⚠️ Recorded as a COMMENT rather than an approving review because GitHub refuses 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.ts plus one line of internal docs/qa/** markdown; nothing is published from any package, no key is added to a published payload, no packages/spec/src/** file is written, and ⛔ no switch changes what it does — the PR pins current behaviour only.
  • needs:contract-review absent on both carriers, read back: PR #15851 carries skip-changeset alone, 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

@os-litant
os-litant added this pull request to the merge queue Sep 5, 2026
Merged via the queue into main with commit e75a904 Sep 5, 2026
48 checks passed
@os-litant
os-litant deleted the claude/issue-15544-pin-rest-config-mount-table branch September 5, 2026 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants