From 87d39a7e9ec3e57ec4877aa37627d9dc7b2c7bd2 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 09:33:26 +0000 Subject: [PATCH] fix(plugin-auth): declare the settings ordering edge so saved auth settings apply at boot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4 --- .../auth-settings-bind-window-ordering.md | 52 ++++++ .../plugins/plugin-auth/src/auth-plugin.ts | 34 ++++ .../src/auth-settings-ordering.pin.test.ts | 166 ++++++++++++++++++ scripts/check-settings-bind-window.mjs | 10 -- 4 files changed, 252 insertions(+), 10 deletions(-) create mode 100644 .changeset/auth-settings-bind-window-ordering.md create mode 100644 packages/plugins/plugin-auth/src/auth-settings-ordering.pin.test.ts diff --git a/.changeset/auth-settings-bind-window-ordering.md b/.changeset/auth-settings-bind-window-ordering.md new file mode 100644 index 0000000000..95b24f82dd --- /dev/null +++ b/.changeset/auth-settings-bind-window-ordering.md @@ -0,0 +1,52 @@ +--- +'@objectstack/plugin-auth': patch +--- + +Apply the workspace's SAVED auth settings at boot — `AuthPlugin` now declares +the settings ordering edge instead of reading in the pre-bind window + +`SettingsServicePlugin` registers the `settings` service in `init()` but binds +its DATA ENGINE from a `kernel:ready` hook it registers in `start()`. Between +those two moments the service is resolvable and answers reads — from an empty +in-memory fallback and the manifest defaults, with `source: 'default'` — while +the deployment's real `sys_setting` rows sit unread. Nothing distinguishes that +from "no row exists". + +`AuthPlugin` was reading inside that window. Its `start()`-registered +`kernel:ready` hooks reach `getService('settings')` at depth 3 (`runBackfill` → +`ensureAuthSettingsBound` → `bindAuthSettings`) and call +`getNamespace('auth')` in the same tick. Handlers fire in registration order, +registration order is `start()` order, and `AuthPlugin` declared +`dependencies: ['com.objectstack.engine.objectql']` and nothing about settings +— so nothing ordered it after the settings plugin. + +On the shipped composition that order was not merely unconstrained, it was +**wrong**: `os serve` does `kernel.use(new AuthPlugin(...))` before the +capability loop registers `SettingsServicePlugin`, and `resolvePluginOrder` +preserves insertion order for plugins with no edge between them. So everything +`applySettings()` derives was computed from DEFAULTS at boot — the ADR-0093 +membership policy the D6 backfill runs under, and the `google_*` social-provider +config. `settings.subscribe('auth', …)` only re-applies on a *later* change, so +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 is one declaration, the same shape the three other shipped readers +(`plugin-email`, `service-sms`, `service-storage`) already carry: + +```ts +optionalDependencies = ['com.objectstack.service.settings']; +``` + +SOFT, not hard — a kernel with no settings service must still boot auth, and +`bindAuthSettings` already returns early when the service is absent. +`requiresServices` would not have done it: that asserts the service is +REGISTERED before `init()`, which it always is, and carries no `start()` +ordering. + +Enforced in both directions. `check:settings-bind-window` goes green with the +`com.objectstack.auth` entry **deleted** from its shrink-only ledger — deleting +it while the defect stood reproduces the finding, so the green is a measurement +rather than a suppression. And `auth-settings-ordering.pin.test.ts` resolves a +hostile registry that composes auth BEFORE settings, then removes the +declaration from a live instance and watches the order revert (ADR-0049: +a declaration nothing acts on is the defect, not the fix). diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index 9f2f9be7ba..b78e74c482 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -239,6 +239,40 @@ export class AuthPlugin implements Plugin { * required` comment with the machine-checked form of the same claim. */ requiresServices = ['data', 'manifest']; + /** + * `com.objectstack.service.settings` — order-if-present (ADR-0116, #10250). + * + * `start()` registers `kernel:ready` hooks that reach + * {@link ensureAuthSettingsBound} → {@link bindAuthSettings}, which resolves + * the `settings` service and reads `getNamespace('auth')`. + * `SettingsServicePlugin` binds its data engine from ITS `kernel:ready` + * hook, registered in ITS `start()`, so the plugin that starts first + * registers the earlier hook and the earlier hook runs first. Start order is + * the topological order over these declarations (`resolvePluginOrder`, used + * for BOTH phases in `ObjectKernel.bootstrap`), so declaring the edge is + * what puts the bind ahead of the read. + * + * Without it the order was WRONG on the shipped composition, not merely + * incidental: `os serve` does `kernel.use(new AuthPlugin(...))` before the + * capability loop registers `SettingsServicePlugin`, and `resolvePluginOrder` + * preserves insertion order for plugins with no edge between them — so auth + * started first and its reads landed in the pre-bind window, where + * `SettingsService`'s empty in-memory fallback answers with the manifest + * DEFAULTS and `source: 'default'` while the workspace's saved `sys_setting` + * rows sit unread. Everything `applySettings()` derives was therefore + * computed from defaults at boot — the ADR-0093 membership policy the D6 + * backfill runs under, and the `google_*` social-provider config — and + * `settings.subscribe('auth', …)` only re-applies on a LATER change, so a + * workspace that configured auth in Setup and never touched it again kept + * booting with the wrong values. + * + * SOFT, not hard: a kernel with no settings service must still boot auth — + * `bindAuthSettings` returns early when the service is absent and the + * deployment's env / options config stands. ADR-0049 — declared is enforced: + * `auth-settings-ordering.pin.test.ts` resolves a hostile registry that uses + * auth BEFORE settings and proves the DECLARATION is what moves the order. + */ + optionalDependencies = ['com.objectstack.service.settings']; private options: AuthPluginOptions; diff --git a/packages/plugins/plugin-auth/src/auth-settings-ordering.pin.test.ts b/packages/plugins/plugin-auth/src/auth-settings-ordering.pin.test.ts new file mode 100644 index 0000000000..c3a770e944 --- /dev/null +++ b/packages/plugins/plugin-auth/src/auth-settings-ordering.pin.test.ts @@ -0,0 +1,166 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `AuthPlugin`'s settings ORDERING contract (#11579) — declared, not incidental. + * + * ## What went wrong + * + * `SettingsServicePlugin` binds its data engine from a `kernel:ready` hook + * registered in its `start()`. `AuthPlugin` reaches `getService('settings')` + * from `kernel:ready` hooks registered in ITS `start()` — at depth 3, through + * `runBackfill` → `ensureAuthSettingsBound` → `bindAuthSettings` — and calls + * `settings.getNamespace('auth')` in the same tick. + * + * Hooks fire in registration order, and registration order is `start()` order, + * so whichever plugin starts first registers the earlier hook. Until this + * change **nothing constrained that order**: `AuthPlugin` declared + * `dependencies: ['com.objectstack.engine.objectql']` and nothing about + * settings, and `resolvePluginOrder` preserves insertion order for plugins + * with no edge between them. On the shipped composition that ordering was not + * merely unconstrained but WRONG — `os serve` does `kernel.use(new + * AuthPlugin(...))` before the capability loop registers + * `SettingsServicePlugin` — so at boot `getNamespace('auth')` took + * `SettingsService`'s empty in-memory fallback and answered the manifest + * DEFAULTS with `source: 'default'`, while the workspace's saved `sys_setting` + * rows sat unread. `settings.subscribe('auth', …)` only re-applies on a LATER + * change, so a deployment that configured auth in Setup and never touched it + * again kept booting on defaults. + * + * ## The division of labour with `check:settings-bind-window` + * + * Two different claims, checked in two different ways, both in CI: + * + * - **That the edge names the REAL provider, and that the read is still in + * the window at all** is `scripts/check-settings-bind-window.mjs`. It walks + * the TypeScript AST, DERIVES the provider from whoever declares + * `providesServices: ['settings']`, and fails if a `start()`-registered + * `kernel:ready` read is not covered by a declaration naming it. That is + * why `SETTINGS_PLUGIN` below is not cross-checked against the settings + * package here: a unit test comparing the constant to the same declaration + * it came from would pass on a typo. The gate is what cannot. + * - **That the declaration MOVES the order** is this file. A declaration + * nothing acts on is the defect, not the fix (ADR-0049), and the gate is + * satisfied by the declaration's presence alone. + * + * ## Resolution note + * + * `AuthPlugin` is imported from SOURCE (a relative specifier inside this + * package), which is what this file is a verdict about. `resolvePluginOrder` + * comes from `@objectstack/core`, a bare workspace specifier already listed in + * `KNOWN_UNALIASED_TEST_IMPORTS['@objectstack/plugin-auth']` + * (`scripts/check-test-source-alias.mjs`), so it resolves to that package's + * `dist/` — the ordering algorithm is the fixed instrument here, not the + * subject. + * + * The settings plugin is a NAME-ONLY stub rather than the real + * `SettingsServicePlugin`: `@objectstack/service-settings` is not a dependency + * of `@objectstack/plugin-auth`, and adding one so a test could import it + * would both create a workspace edge that exists for nothing else and force a + * new entry into the shrink-only registry above. `resolvePluginOrder` reads + * only the `OrderablePlugin` surface — `name`, `dependencies`, + * `optionalDependencies` — and the property under test is `AuthPlugin`'s + * declaration, so the stub is the whole of what the resolver would see. + */ + +import { describe, it, expect } from 'vitest'; +import { resolvePluginOrder } from '@objectstack/core'; +import type { OrderablePlugin } from '@objectstack/core'; +import { AuthPlugin } from './auth-plugin.js'; + +const SETTINGS_PLUGIN = 'com.objectstack.service.settings'; +const ENGINE_PLUGIN = 'com.objectstack.engine.objectql'; + +/** + * `AuthPlugin` declares `com.objectstack.engine.objectql` a HARD dependency, + * so every registry below has to contain it or `resolvePluginOrder` throws + * before it can order anything. Name-only: this module orders plugins by their + * declarations and never runs a lifecycle. + */ +const engineStub = (): OrderablePlugin => ({ name: ENGINE_PLUGIN }); + +/** See the resolution note in the header for why this is a stub. */ +const settingsStub = (): OrderablePlugin => ({ name: SETTINGS_PLUGIN }); + +const authPlugin = (): OrderablePlugin => + new AuthPlugin({ secret: 'test-secret-at-least-32-chars-long!!' }) as unknown as OrderablePlugin; + +/** Registry in the given insertion order — `resolvePluginOrder` preserves it + * for plugins with no edges between them, which is what makes the hostile + * order below hostile. */ +const registryOf = (...plugins: OrderablePlugin[]) => + new Map(plugins.map((p) => [p.name, p])); + +describe('AuthPlugin declares the settings ordering edge (ADR-0116, #11579)', () => { + it('1. declares `com.objectstack.service.settings` as an OPTIONAL dependency', () => { + const auth = authPlugin(); + expect(auth.optionalDependencies ?? []).toContain(SETTINGS_PLUGIN); + // Not a hard one: case 4 is the behavioural half of this, but the + // declaration site is asserted directly too, because promoting the edge to + // `dependencies` would pass case 2 and 3 while breaking every lean kernel. + expect(auth.dependencies ?? []).not.toContain(SETTINGS_PLUGIN); + // The pre-existing hard edge is untouched — this change adds an edge, it + // does not move one. + expect(auth.dependencies ?? []).toContain(ENGINE_PLUGIN); + }); + + it('2. the declaration MOVES resolution order — settings inits/starts first even when used last', () => { + // `ObjectKernel.bootstrap` and `LiteKernel.bootstrap` both iterate the SAME + // `resolvePluginOrder` output for Phase 1 (init) and Phase 2 (start), so + // this is the order the `kernel:ready` hooks get registered in. + const auth = authPlugin(); + // The shipped hostile order: `os serve` uses AuthPlugin BEFORE the + // capability loop registers the settings plugin. + const ordered = resolvePluginOrder(registryOf(engineStub(), auth, settingsStub())) + .map((p) => p.name); + expect(ordered.indexOf(SETTINGS_PLUGIN)).toBeLessThan(ordered.indexOf(auth.name)); + }); + + it('3. …and the declaration is what does it — forget it and the order reverts', () => { + // The ADR-0049 half. Case 2 alone would still pass if `resolvePluginOrder` + // happened to hoist by some other rule; this proves the DECLARATION is the + // cause by removing it from a live instance and re-resolving. + const auth = authPlugin(); + auth.optionalDependencies = (auth.optionalDependencies ?? []).filter( + (d) => d !== SETTINGS_PLUGIN, + ); + + const ordered = resolvePluginOrder(registryOf(engineStub(), auth, settingsStub())) + .map((p) => p.name); + // Insertion order is preserved for plugins with no edges between them — so + // with the declaration gone the reader is back in front, which is the + // defect this card was filed about. + expect( + ordered.indexOf(auth.name), + 'without the declaration auth must come back first — if this passes, case 2 was not measuring the declaration', + ).toBeLessThan(ordered.indexOf(SETTINGS_PLUGIN)); + }); + + it('4. the edge is SOFT — a kernel with no settings plugin still resolves', () => { + // `optionalDependencies` is order-if-present. A hard dependency here would + // refuse to boot every metadata-only / lean kernel that composes auth + // without a settings service — `bindAuthSettings` already returns early + // when the service is absent. + const auth = authPlugin(); + const registry = registryOf(engineStub(), auth); + expect(() => resolvePluginOrder(registry)).not.toThrow(); + expect(resolvePluginOrder(registry).map((p) => p.name)).toContain(auth.name); + }); + + it('5. the edge introduces no cycle — auth is not upstream of settings', () => { + // `resolvePluginOrder` throws `[Kernel] Circular dependency detected` when + // both directions are declared, and an optional edge is a real edge + // whenever both sides are composed. The settings plugin declares only + // `com.objectstack.engine.objectql`, so this direction is free — asserted + // rather than assumed, because the check that would otherwise catch it + // (`check:settings-bind-window`'s `cycle` verdict) reports it as a finding + // rather than as this plugin's failure. + const settingsWithItsRealEdge: OrderablePlugin = { + name: SETTINGS_PLUGIN, + optionalDependencies: [ENGINE_PLUGIN], + }; + const auth = authPlugin(); + const ordered = resolvePluginOrder(registryOf(engineStub(), auth, settingsWithItsRealEdge)) + .map((p) => p.name); + expect(ordered).toEqual([ENGINE_PLUGIN, SETTINGS_PLUGIN, auth.name]); + }); +}); diff --git a/scripts/check-settings-bind-window.mjs b/scripts/check-settings-bind-window.mjs index 62bf3208fe..9009565630 100644 --- a/scripts/check-settings-bind-window.mjs +++ b/scripts/check-settings-bind-window.mjs @@ -151,16 +151,6 @@ const PREFILTER_TOKENS = [...SERVICE_LOOKUP_CALLEES, 'providesServices']; * and a single "known bad" bucket would let one be closed by the other's fix. */ const KNOWN_PRE_BIND_READS = [ - { - plugin: 'com.objectstack.auth', - verdict: 'undeclared', - issue: '#11579', - note: - 'AuthPlugin reaches getService(\'settings\') at depth 3 from three start()-registered ' + - 'kernel:ready hooks and calls settings.getNamespace(\'auth\') there. In the `os serve` ' + - 'composition AuthPlugin is used() before the capability loop registers ' + - 'SettingsServicePlugin, so its hooks fire first. Repair is the #10250 declaration.', - }, { plugin: 'com.objectstack.mcp', verdict: 'unfixable-by-declaration',