Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-auth): declare the settings ordering edge so saved auth settings apply at boot - #11619
Conversation
…ttings apply at boot
`AuthPlugin` reached `getService('settings')` from `kernel:ready` hooks
registered in its own `start()` — at depth 3 through `runBackfill` →
`ensureAuthSettingsBound` → `bindAuthSettings` — and called
`getNamespace('auth')` in the same tick. `SettingsServicePlugin` binds its
data engine from ITS `start()`-registered `kernel:ready` hook, handlers fire
in registration order, and `AuthPlugin` declared nothing about settings, so
nothing ordered it after the provider.
On the shipped composition that order was wrong, not merely unconstrained:
`os serve` uses `AuthPlugin` before the capability loop registers
`SettingsServicePlugin`, so at boot `getNamespace('auth')` took the empty
in-memory fallback and answered manifest DEFAULTS with `source: 'default'`
while the workspace's `sys_setting` rows went unread — the ADR-0093
membership policy the D6 backfill runs under, and the `google_*`
social-provider config, both computed from defaults. `subscribe('auth', …)`
only re-applies on a later change, so a workspace configured once in Setup
kept booting wrong.
Repair is one declaration, the #10250 shape the three other shipped readers
already carry: `optionalDependencies = ['com.objectstack.service.settings']`.
Soft, not hard — `bindAuthSettings` already returns early with no service.
The `com.objectstack.auth` entry is deleted from `KNOWN_PRE_BIND_READS` in
`scripts/check-settings-bind-window.mjs`; that ledger is shrink-only and
errors on a stale entry, so the deletion is part of the repair. Deleting it
while the defect stood reproduces the finding, so the green is a measurement
rather than a suppression.
`auth-settings-ordering.pin.test.ts` is the ADR-0049 half: it resolves a
hostile registry composing auth BEFORE settings, then strips the declaration
from a live instance and watches the order revert.
Scope note: `packages/mcp` carries the same pre-bind class and is a separate
lane — untouched here, and #11580 remains open for it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4📓 Docs Drift CheckThis PR changes 1 package(s): 10 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 — 11 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 8c2800ff720252a3345270e84e056348f134e340 && git checkout 8c2800ff720252a3345270e84e056348f134e340
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin d15ddba02c698ec4116b2f032bc99c235095bad4 87d39a7e9ec3e57ec4877aa37627d9dc7b2c7bd2 && git checkout -B drift-repro d15ddba02c698ec4116b2f032bc99c235095bad4 && git merge --no-ff 87d39a7e9ec3e57ec4877aa37627d9dc7b2c7bd2
node scripts/docs-audit/affected-docs.mjs --json d15ddba02c698ec4116b2f032bc99c235095bad4
|
Uh oh!
There was an error while loading. Please reload this page.
Conflict: KNOWN_PRE_BIND_READS in scripts/check-settings-bind-window.mjs. Both sides DELETE a different adjacent entry — main removed com.objectstack.auth (#11579's fix, PR #11619), this branch removes com.objectstack.mcp. Both deletions are correct and both survive, so the shrink-only ledger is now empty: the baseline is fully burned down. Verified rather than assumed, because an empty ledger is a code path that could have gone unexercised: ✓ settings bind-window guard self-test: all cases pass. ✓ settings bind-window: 4 declared / 0 self / 1 structurally upstream / 0 ledgered (68 plugin unit(s) scanned, provider 'com.objectstack.service.settings'). The ledger docblock is updated in the same stroke: it described 'the two live readers below' in the present tense, which the empty array makes false. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR
Fixes#11579
The defect
SettingsServicePluginregisters thesettingsservice ininit()but binds its data engine from akernel:readyhook it registers instart(). Between those two moments the service is resolvable and answers reads — from an empty in-memory fallback and the manifest defaults, withsource: 'default'— while the deployment's realsys_settingrows sit unread. Nothing distinguishes that from "no row exists".AuthPluginwas reading inside that window. Itsstart()-registeredkernel:readyhooks reachgetService('settings')at depth 3 (runBackfill→ensureAuthSettingsBound→bindAuthSettings) and callgetNamespace('auth')in the same tick. Handlers fire in registration order, registration order isstart()order, andAuthPlugindeclareddependencies: ['com.objectstack.engine.objectql']and nothing about settings.On the shipped composition the resulting order was wrong, not merely unconstrained:
os servedoeskernel.use(new AuthPlugin(...))before the capability loop registersSettingsServicePlugin, andresolvePluginOrderpreserves insertion order for plugins with no edge between them. So everythingapplySettings()derives was computed from DEFAULTS at boot — the ADR-0093 membership policy the D6 backfill runs under, and thegoogle_*social-provider config — andsettings.subscribe('auth', …)only re-applies on a later change. A workspace that configured auth in Setup and never touched it again kept booting with the wrong values: authored, stored, and silently not applied.The repair
One declaration, the #10250 shape the three other shipped readers (
plugin-email,service-sms,service-storage) already carry:SOFT, not hard — a kernel with no settings service must still boot auth, and
bindAuthSettingsalready returns early when the service is absent.requiresServiceswould not have done it: it asserts the service is REGISTERED beforeinit(), which it always is, and carries nostart()ordering.The acceptance test: the gate's own ledger shrinks
KNOWN_PRE_BIND_READSinscripts/check-settings-bind-window.mjsis shrink-only and errors on a stale entry, so deleting thecom.objectstack.authrow is part of the repair rather than follow-up. No entry was added, re-added or weakened — thecom.objectstack.mcprow (#11580, a different lane) is untouched.The green is a measurement, not a suppression, because the red was measured first — the entry deleted with the defect still standing:
With the declaration in place, at
87d39a7e9e:3 declared / 2 ledgered→4 declared / 1 ledgered: auth moved from the ledger into the declared column.--self-testalso passes (its case 2b is literally this fixed shape).ADR-0049 — declared is enforced
packages/plugins/plugin-auth/src/auth-settings-ordering.pin.test.ts, 5 cases, all green:optionalDependenciesrather thandependencies;com.objectstack.engine.objectql, so this direction is free.The settings plugin is a name-only stub there (
resolvePluginOrderreads only theOrderablePluginsurface); thatcom.objectstack.service.settingsis the real provider id ischeck:settings-bind-window's job, which DERIVES the provider fromprovidesServices: ['settings']. The header states that division of labour so neither claim is assumed to be the other's.Verification
Union re-run after the final commit, at
87d39a7e9e:check:settings-bind-window✓ 4 declared / 0 self / 1 structurally upstream / 1 ledgeredcheck:settings-bind-window --self-test✓ all cases pass@objectstack/plugin-authsuiteTest Files 73 passed (73) · Tests 1480 passed (1480)@objectstack/plugin-authtypecheckVERDICT command-exit 0(after building the package's owndist/)check:nul-bytesOK (scanned 6501 text file(s) ... no raw ASCII control bytes)check:test-source-aliasOK — 72 packages with tests scanned(no registry change needed)check:slot-lookup✓ ratchet holds: 107 unswept site(s), none newcheck:query-options-erasure✓ ratchet holds: 67 unswept non-test site(s), none newcheck:engine-double-contract379 (file, verb) row(s) held by the RETAINED ledgercheck:where-matcher✓ 292 matcher(s) discovered, 292 conformantcheck:type-check-coverageOK — 65/78 packages type-checked, 13 in DEBTcheck:cross-package-test-inputsOK: 16 package(s) read outside themselves, all declaredAlso green before the commit, on the same content:
check:auth-mount-ledger,check:route-envelope,check:plugin-teardown-shape,check:published-files,check:entry-guard,check:parse-guard,check:pnpm-filter-targets,check:ci-filter-parity,check:type-source-resolution, the changeset family (check-empty-changeset,check-changeset-no-major,check-adr-0087-registration,check:objectui-changeset,check:changeset-gate-self-tests,release-rehearsal-clone --self-test) anddocs-audit/check-affected-docs. Gate list derived withnode scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackfrom the real change set, not from a hand-written diff.The zero-margin ratchet this change could have moved
TEST_DEBT['@objectstack/plugin-auth']records 97 raw tsc errors with no margin ("the next new error here goes red immediately"), and this PR adds a test file. The package's owntsconfig.jsonexcludes**/*.test.ts, so the green package typecheck does not cover the new file — measuring it needed the test exclusion lifted, which is whatmeasureTestDebtdoes. Mirrored locally with a temp in-package project (removed afterwards, verified absent on disk) against the built dependency closure:97 is the ledger's recorded number exactly, which is what makes the instrument credible rather than merely quiet — the new file contributes zero.
Lint: a declared narrowing, not a repo-wide run
Repo-wide
pnpm lintwas not run locally; the shared verify lock had a holder plus a waiter and queuing it risked the foreground cap. Narrowed instead, with the three pieces of evidence a narrowing needs:.changeset/…mdas "File ignored because no matching configuration was supplied", i.e. eslint decided the population, not I;--format json— 4 results, 3 linted,errorCount=0,warningCount=0on all three code files;eslint.config.mjsstates in its own docblock that this repo "runs oneeslint.config.mjs, which never enables type-aware linting (noparserOptions.project, no typed@typescript-eslintrules) for ANY file", and this diff changes no eslint config, so no untouched file's verdict can move.CI runs the farm exactly once regardless.
Not measured
check:type-check-debt --re-measurewas not run whole (it refuses without the entire ledgered closure built). The one entry this change could move was measured directly, above.pnpm lint, per the declared narrowing.resolvePluginOrder, and the ordering claim is pinned by the new test rather than by a boot.Scope
packages/mcpcarries the same pre-bind class in a different lane and is untouched here; #11580 remains open for it. Nopackages/spec/**, nocontent/docs/releases/**, no governed surfaces. Not flipped ready, not enqueued, no auto-merge.Generated by Claude Code