Skip to content

test(service-settings): add settings-route-ledger conformance guard - #17071

Merged
os-trump merged 2 commits into
mainfrom
claude/issue-17062-settings-route-ledger-conformance
Sep 9, 2026
Merged

test(service-settings): add settings-route-ledger conformance guard#17071
os-trump merged 2 commits into
mainfrom
claude/issue-17062-settings-route-ledger-conformance

Conversation

@os-trump

@os-trump os-trump commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Fixes #17062

What

Adds packages/services/service-settings/src/settings-route-ledger.conformance.test.ts — the guard every OTHER *-route-ledger.ts in the tree already pairs itself with, missing here since SETTINGS_ROUTE_LEDGER landed at #7526. Test-only; no behaviour change.

How settings-routes.ts registers, and why this shape

registerSettingsRoutes (settings-routes.ts) is an exported, synchronous, top-level function(http, service, opts) => void — that calls http.get/put/post directly and touches neither service nor opts before a request arrives (both are only read inside handler closures). That is exactly the shape storage-routes.ts and datasource's admin-routes.ts export, and it's why their conformance tests already settled the right seam: drive the registrar against a capturing mock IHttpServer and read its recorded calls as the route set.

It is not the i18n shape — I18nServicePlugin's route registration is a private method reachable only by driving the plugin's initstartkernel:ready lifecycle. registerSettingsRoutes needs no lifecycle to reach because it's already the exported seam; driving one it doesn't require would intercept nothing a direct call doesn't. It's also not a source-scan (cli/metadata/trigger-api's second limb) — that shape earns its keep when the mounting mechanism can't be driven behind a mock; here it's an ordinary function call. So this PR copies the storage/datasource shape, chosen by mechanism rather than by proximity.

The direction that matters

packages/qa/dogfood/test/route-ledger-live-mount-parity.dogfood.test.ts (#7526) already asserts, on a real boot, that every ledgered settings row resolves — and (worth being precise about, since it changes the framing slightly from the issue) its every mounted route is ledgered assertion does fold SETTINGS_ROUTE_LEDGER into the union it checks, so a wholly new, unledgered settings route would in fact turn that dogfood test red too, today. But that coverage is (a) a cross-package aggregate — the failure names the bare route, not "service-settings' ledger drifted" — (b) gated on whatever plugins that one dogfood boot composes, and (c) lives in packages/qa/dogfood, not in this package's own pnpm test. It is not the fast, package-scoped, always-runs unit guard every sibling ledger has, and that is the gap this PR closes. settings-route-ledger.ts's own header comment previously claimed this second guard was deliberately omitted for exactly this reason; it's updated in this PR now that the guard exists.

Ledger checked for staleness — found current, not stale

Read settings-routes.ts end-to-end: it makes exactly four registration calls (GET /api/settings, GET /api/settings/:namespace, PUT /api/settings/:namespace, POST /api/settings/:namespace/:actionId), 1:1 with SETTINGS_ROUTE_LEDGER's four rows. Confirmed empirically too: the new test passes 6/6 on the unmutated tree. No data fix bundled in this PR; per the card's own fence, this would have been reported and sequenced separately had it been needed.

Both directions shown red, then restored

Predicted before running, mutated on disk, restored under a shell trap (git checkout HEAD -- PATH, PATH standing for the file's real repo-relative path), restoration proven by blob equality and an empty git diff HEAD. Both matched prediction exactly (full transcript + blob hashes in the linked issue comment / report JSON):

  1. Deleted the PUT /api/settings/:namespace ledger row (route still mounted) → red, naming PUT /api/settings/:namespace in every mounted settings route has a ledger entry (live.size 4 vs ledgerKeys().size 3).
  2. Added a mounted DELETE /api/settings/:namespace/:actionId route with no ledger row → red, naming that exact route in the same assertion (live.size 5 vs ledgerKeys().size 4) — the direction the issue flags as the one a copied test can quietly omit.

Changeset

None. packages/services/service-settings/package.json's files is ["dist", "README.md", "CHANGELOG.md"] — a *.test.ts under src/ is never published, so this diff ships nothing from any released package (skip-changeset, applied on this PR).

Gates

  • pnpm --filter '@objectstack/service-settings^...' build — clean (dependency closure).
  • pnpm --filter @objectstack/service-settings test — 578/578 passed (33 files), including the new conformance test.
  • pnpm --filter @objectstack/service-settings typecheck — clean.
  • pnpm check:route-envelope — clean (named explicitly per dispatch note; no route module touched, so N/A in substance).
  • node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 51 commands for this diff; all 51 ran (--ran reconciliation: 51 derived, 51 run, 0 NOT-MEASURED, 0 UNRUN). 49/51 exited 0. The other two are legitimate PREREQUISITE NOT MET (exit 3), not failures — check:dual-build-cjs-loads and check:type-check-debt --re-measure both refuse to run without a full pnpm build across ~50 packages' dist/ (only the service-settings dependency closure was built locally, per this lane's local-verification scope); CI's Lint & Repo Gates job builds the full closure first and will measure both for real.

registerSettingsRoutes is an exported, synchronous, top-level function
(http, service, opts) => void that calls http.get/put/post directly and
touches neither argument before a request arrives -- the same shape
storage-routes.ts and datasource's admin-routes.ts export. This mirrors
their conformance-test seam (a capturing mock IHttpServer) rather than
i18n's plugin-lifecycle shape, since no lifecycle is needed to reach an
already-exported registrar.

Both directions verified red-then-restored before landing: deleting the
PUT /api/settings/:namespace ledger row, and adding a mounted DELETE
/api/settings/:namespace/:actionId route with no row, each fail the new
test naming that exact route. The existing ledger was checked for drift
first and found current (4 registrar calls, 4 ledger rows, 1:1).

settings-route-ledger.ts's header comment previously said a per-package
guard was deliberately omitted in favour of the dogfood live-mount-parity
gate; updated now that this file exists alongside it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
@os-trump os-trump added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 9, 2026 — with Claude
@github-actions github-actions Bot added the size/m label Sep 9, 2026
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

⚠️ 1 changed file(s) yielded no anchor (packages/services/service-settings/src/settings-route-ledger.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files. Nothing else in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 1 changed package(s)).

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/services/service-settings/src/settings-route-ledger.ts) — pages documenting those are invisible to this run
  • 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 — 7 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 91f65c4ea4ce89d824e2ea64f93a956ffff13928packageMentionDocs.

@os-trump
os-trump marked this pull request as ready for review September 9, 2026 10:38
@os-trump
os-trump enabled auto-merge September 9, 2026 10:38
@os-trump
os-trump added this pull request to the merge queue Sep 9, 2026
Merged via the queue into main with commit 50b6f17 Sep 9, 2026
35 checks passed
@os-trump
os-trump deleted the claude/issue-17062-settings-route-ledger-conformance branch September 9, 2026 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m skip-changeset PR has no user-facing published change; bypasses the changeset gate tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[finding] service-settings' route ledger has no conformance guard — the pattern every sibling ledger follows is silently missing here

2 participants