Skip to content

Commit 2efa1e1

Browse files
os-samclaude
andauthored
fix(plugin-auth): declare the settings ordering edge so saved auth settings apply at boot (#11619)
`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. Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4 Co-authored-by: Claude <noreply@anthropic.com>
1 parent bde3072 commit 2efa1e1

4 files changed

Lines changed: 252 additions & 10 deletions

File tree

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
---
2+
'@objectstack/plugin-auth': patch
3+
---
4+
5+
Apply the workspace's SAVED auth settings at boot — `AuthPlugin` now declares
6+
the settings ordering edge instead of reading in the pre-bind window
7+
8+
`SettingsServicePlugin` registers the `settings` service in `init()` but binds
9+
its DATA ENGINE from a `kernel:ready` hook it registers in `start()`. Between
10+
those two moments the service is resolvable and answers reads — from an empty
11+
in-memory fallback and the manifest defaults, with `source: 'default'` — while
12+
the deployment's real `sys_setting` rows sit unread. Nothing distinguishes that
13+
from "no row exists".
14+
15+
`AuthPlugin` was reading inside that window. Its `start()`-registered
16+
`kernel:ready` hooks reach `getService('settings')` at depth 3 (`runBackfill`
17+
`ensureAuthSettingsBound``bindAuthSettings`) and call
18+
`getNamespace('auth')` in the same tick. Handlers fire in registration order,
19+
registration order is `start()` order, and `AuthPlugin` declared
20+
`dependencies: ['com.objectstack.engine.objectql']` and nothing about settings
21+
— so nothing ordered it after the settings plugin.
22+
23+
On the shipped composition that order was not merely unconstrained, it was
24+
**wrong**: `os serve` does `kernel.use(new AuthPlugin(...))` before the
25+
capability loop registers `SettingsServicePlugin`, and `resolvePluginOrder`
26+
preserves insertion order for plugins with no edge between them. So everything
27+
`applySettings()` derives was computed from DEFAULTS at boot — the ADR-0093
28+
membership policy the D6 backfill runs under, and the `google_*` social-provider
29+
config. `settings.subscribe('auth', …)` only re-applies on a *later* change, so
30+
a workspace that configured auth in Setup and never touched it again kept
31+
booting with the wrong values: authored, stored, and silently not applied.
32+
33+
The repair is one declaration, the same shape the three other shipped readers
34+
(`plugin-email`, `service-sms`, `service-storage`) already carry:
35+
36+
```ts
37+
optionalDependencies = ['com.objectstack.service.settings'];
38+
```
39+
40+
SOFT, not hard — a kernel with no settings service must still boot auth, and
41+
`bindAuthSettings` already returns early when the service is absent.
42+
`requiresServices` would not have done it: that asserts the service is
43+
REGISTERED before `init()`, which it always is, and carries no `start()`
44+
ordering.
45+
46+
Enforced in both directions. `check:settings-bind-window` goes green with the
47+
`com.objectstack.auth` entry **deleted** from its shrink-only ledger — deleting
48+
it while the defect stood reproduces the finding, so the green is a measurement
49+
rather than a suppression. And `auth-settings-ordering.pin.test.ts` resolves a
50+
hostile registry that composes auth BEFORE settings, then removes the
51+
declaration from a live instance and watches the order revert (ADR-0049:
52+
a declaration nothing acts on is the defect, not the fix).

packages/plugins/plugin-auth/src/auth-plugin.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,40 @@ export class AuthPlugin implements Plugin {
239239
* required` comment with the machine-checked form of the same claim.
240240
*/
241241
requiresServices = ['data', 'manifest'];
242+
/**
243+
* `com.objectstack.service.settings` — order-if-present (ADR-0116, #10250).
244+
*
245+
* `start()` registers `kernel:ready` hooks that reach
246+
* {@link ensureAuthSettingsBound} → {@link bindAuthSettings}, which resolves
247+
* the `settings` service and reads `getNamespace('auth')`.
248+
* `SettingsServicePlugin` binds its data engine from ITS `kernel:ready`
249+
* hook, registered in ITS `start()`, so the plugin that starts first
250+
* registers the earlier hook and the earlier hook runs first. Start order is
251+
* the topological order over these declarations (`resolvePluginOrder`, used
252+
* for BOTH phases in `ObjectKernel.bootstrap`), so declaring the edge is
253+
* what puts the bind ahead of the read.
254+
*
255+
* Without it the order was WRONG on the shipped composition, not merely
256+
* incidental: `os serve` does `kernel.use(new AuthPlugin(...))` before the
257+
* capability loop registers `SettingsServicePlugin`, and `resolvePluginOrder`
258+
* preserves insertion order for plugins with no edge between them — so auth
259+
* started first and its reads landed in the pre-bind window, where
260+
* `SettingsService`'s empty in-memory fallback answers with the manifest
261+
* DEFAULTS and `source: 'default'` while the workspace's saved `sys_setting`
262+
* rows sit unread. Everything `applySettings()` derives was therefore
263+
* computed from defaults at boot — the ADR-0093 membership policy the D6
264+
* backfill runs under, and the `google_*` social-provider config — and
265+
* `settings.subscribe('auth', …)` only re-applies on a LATER change, so a
266+
* workspace that configured auth in Setup and never touched it again kept
267+
* booting with the wrong values.
268+
*
269+
* SOFT, not hard: a kernel with no settings service must still boot auth —
270+
* `bindAuthSettings` returns early when the service is absent and the
271+
* deployment's env / options config stands. ADR-0049 — declared is enforced:
272+
* `auth-settings-ordering.pin.test.ts` resolves a hostile registry that uses
273+
* auth BEFORE settings and proves the DECLARATION is what moves the order.
274+
*/
275+
optionalDependencies = ['com.objectstack.service.settings'];
242276

243277

244278
private options: AuthPluginOptions;
Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* `AuthPlugin`'s settings ORDERING contract (#11579) — declared, not incidental.
5+
*
6+
* ## What went wrong
7+
*
8+
* `SettingsServicePlugin` binds its data engine from a `kernel:ready` hook
9+
* registered in its `start()`. `AuthPlugin` reaches `getService('settings')`
10+
* from `kernel:ready` hooks registered in ITS `start()` — at depth 3, through
11+
* `runBackfill` → `ensureAuthSettingsBound` → `bindAuthSettings` — and calls
12+
* `settings.getNamespace('auth')` in the same tick.
13+
*
14+
* Hooks fire in registration order, and registration order is `start()` order,
15+
* so whichever plugin starts first registers the earlier hook. Until this
16+
* change **nothing constrained that order**: `AuthPlugin` declared
17+
* `dependencies: ['com.objectstack.engine.objectql']` and nothing about
18+
* settings, and `resolvePluginOrder` preserves insertion order for plugins
19+
* with no edge between them. On the shipped composition that ordering was not
20+
* merely unconstrained but WRONG — `os serve` does `kernel.use(new
21+
* AuthPlugin(...))` before the capability loop registers
22+
* `SettingsServicePlugin` — so at boot `getNamespace('auth')` took
23+
* `SettingsService`'s empty in-memory fallback and answered the manifest
24+
* DEFAULTS with `source: 'default'`, while the workspace's saved `sys_setting`
25+
* rows sat unread. `settings.subscribe('auth', …)` only re-applies on a LATER
26+
* change, so a deployment that configured auth in Setup and never touched it
27+
* again kept booting on defaults.
28+
*
29+
* ## The division of labour with `check:settings-bind-window`
30+
*
31+
* Two different claims, checked in two different ways, both in CI:
32+
*
33+
* - **That the edge names the REAL provider, and that the read is still in
34+
* the window at all** is `scripts/check-settings-bind-window.mjs`. It walks
35+
* the TypeScript AST, DERIVES the provider from whoever declares
36+
* `providesServices: ['settings']`, and fails if a `start()`-registered
37+
* `kernel:ready` read is not covered by a declaration naming it. That is
38+
* why `SETTINGS_PLUGIN` below is not cross-checked against the settings
39+
* package here: a unit test comparing the constant to the same declaration
40+
* it came from would pass on a typo. The gate is what cannot.
41+
* - **That the declaration MOVES the order** is this file. A declaration
42+
* nothing acts on is the defect, not the fix (ADR-0049), and the gate is
43+
* satisfied by the declaration's presence alone.
44+
*
45+
* ## Resolution note
46+
*
47+
* `AuthPlugin` is imported from SOURCE (a relative specifier inside this
48+
* package), which is what this file is a verdict about. `resolvePluginOrder`
49+
* comes from `@objectstack/core`, a bare workspace specifier already listed in
50+
* `KNOWN_UNALIASED_TEST_IMPORTS['@objectstack/plugin-auth']`
51+
* (`scripts/check-test-source-alias.mjs`), so it resolves to that package's
52+
* `dist/` — the ordering algorithm is the fixed instrument here, not the
53+
* subject.
54+
*
55+
* The settings plugin is a NAME-ONLY stub rather than the real
56+
* `SettingsServicePlugin`: `@objectstack/service-settings` is not a dependency
57+
* of `@objectstack/plugin-auth`, and adding one so a test could import it
58+
* would both create a workspace edge that exists for nothing else and force a
59+
* new entry into the shrink-only registry above. `resolvePluginOrder` reads
60+
* only the `OrderablePlugin` surface — `name`, `dependencies`,
61+
* `optionalDependencies` — and the property under test is `AuthPlugin`'s
62+
* declaration, so the stub is the whole of what the resolver would see.
63+
*/
64+
65+
import { describe, it, expect } from 'vitest';
66+
import { resolvePluginOrder } from '@objectstack/core';
67+
import type { OrderablePlugin } from '@objectstack/core';
68+
import { AuthPlugin } from './auth-plugin.js';
69+
70+
const SETTINGS_PLUGIN = 'com.objectstack.service.settings';
71+
const ENGINE_PLUGIN = 'com.objectstack.engine.objectql';
72+
73+
/**
74+
* `AuthPlugin` declares `com.objectstack.engine.objectql` a HARD dependency,
75+
* so every registry below has to contain it or `resolvePluginOrder` throws
76+
* before it can order anything. Name-only: this module orders plugins by their
77+
* declarations and never runs a lifecycle.
78+
*/
79+
const engineStub = (): OrderablePlugin => ({ name: ENGINE_PLUGIN });
80+
81+
/** See the resolution note in the header for why this is a stub. */
82+
const settingsStub = (): OrderablePlugin => ({ name: SETTINGS_PLUGIN });
83+
84+
const authPlugin = (): OrderablePlugin =>
85+
new AuthPlugin({ secret: 'test-secret-at-least-32-chars-long!!' }) as unknown as OrderablePlugin;
86+
87+
/** Registry in the given insertion order — `resolvePluginOrder` preserves it
88+
* for plugins with no edges between them, which is what makes the hostile
89+
* order below hostile. */
90+
const registryOf = (...plugins: OrderablePlugin[]) =>
91+
new Map<string, OrderablePlugin>(plugins.map((p) => [p.name, p]));
92+
93+
describe('AuthPlugin declares the settings ordering edge (ADR-0116, #11579)', () => {
94+
it('1. declares `com.objectstack.service.settings` as an OPTIONAL dependency', () => {
95+
const auth = authPlugin();
96+
expect(auth.optionalDependencies ?? []).toContain(SETTINGS_PLUGIN);
97+
// Not a hard one: case 4 is the behavioural half of this, but the
98+
// declaration site is asserted directly too, because promoting the edge to
99+
// `dependencies` would pass case 2 and 3 while breaking every lean kernel.
100+
expect(auth.dependencies ?? []).not.toContain(SETTINGS_PLUGIN);
101+
// The pre-existing hard edge is untouched — this change adds an edge, it
102+
// does not move one.
103+
expect(auth.dependencies ?? []).toContain(ENGINE_PLUGIN);
104+
});
105+
106+
it('2. the declaration MOVES resolution order — settings inits/starts first even when used last', () => {
107+
// `ObjectKernel.bootstrap` and `LiteKernel.bootstrap` both iterate the SAME
108+
// `resolvePluginOrder` output for Phase 1 (init) and Phase 2 (start), so
109+
// this is the order the `kernel:ready` hooks get registered in.
110+
const auth = authPlugin();
111+
// The shipped hostile order: `os serve` uses AuthPlugin BEFORE the
112+
// capability loop registers the settings plugin.
113+
const ordered = resolvePluginOrder(registryOf(engineStub(), auth, settingsStub()))
114+
.map((p) => p.name);
115+
expect(ordered.indexOf(SETTINGS_PLUGIN)).toBeLessThan(ordered.indexOf(auth.name));
116+
});
117+
118+
it('3. …and the declaration is what does it — forget it and the order reverts', () => {
119+
// The ADR-0049 half. Case 2 alone would still pass if `resolvePluginOrder`
120+
// happened to hoist by some other rule; this proves the DECLARATION is the
121+
// cause by removing it from a live instance and re-resolving.
122+
const auth = authPlugin();
123+
auth.optionalDependencies = (auth.optionalDependencies ?? []).filter(
124+
(d) => d !== SETTINGS_PLUGIN,
125+
);
126+
127+
const ordered = resolvePluginOrder(registryOf(engineStub(), auth, settingsStub()))
128+
.map((p) => p.name);
129+
// Insertion order is preserved for plugins with no edges between them — so
130+
// with the declaration gone the reader is back in front, which is the
131+
// defect this card was filed about.
132+
expect(
133+
ordered.indexOf(auth.name),
134+
'without the declaration auth must come back first — if this passes, case 2 was not measuring the declaration',
135+
).toBeLessThan(ordered.indexOf(SETTINGS_PLUGIN));
136+
});
137+
138+
it('4. the edge is SOFT — a kernel with no settings plugin still resolves', () => {
139+
// `optionalDependencies` is order-if-present. A hard dependency here would
140+
// refuse to boot every metadata-only / lean kernel that composes auth
141+
// without a settings service — `bindAuthSettings` already returns early
142+
// when the service is absent.
143+
const auth = authPlugin();
144+
const registry = registryOf(engineStub(), auth);
145+
expect(() => resolvePluginOrder(registry)).not.toThrow();
146+
expect(resolvePluginOrder(registry).map((p) => p.name)).toContain(auth.name);
147+
});
148+
149+
it('5. the edge introduces no cycle — auth is not upstream of settings', () => {
150+
// `resolvePluginOrder` throws `[Kernel] Circular dependency detected` when
151+
// both directions are declared, and an optional edge is a real edge
152+
// whenever both sides are composed. The settings plugin declares only
153+
// `com.objectstack.engine.objectql`, so this direction is free — asserted
154+
// rather than assumed, because the check that would otherwise catch it
155+
// (`check:settings-bind-window`'s `cycle` verdict) reports it as a finding
156+
// rather than as this plugin's failure.
157+
const settingsWithItsRealEdge: OrderablePlugin = {
158+
name: SETTINGS_PLUGIN,
159+
optionalDependencies: [ENGINE_PLUGIN],
160+
};
161+
const auth = authPlugin();
162+
const ordered = resolvePluginOrder(registryOf(engineStub(), auth, settingsWithItsRealEdge))
163+
.map((p) => p.name);
164+
expect(ordered).toEqual([ENGINE_PLUGIN, SETTINGS_PLUGIN, auth.name]);
165+
});
166+
});

scripts/check-settings-bind-window.mjs

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -151,16 +151,6 @@ const PREFILTER_TOKENS = [...SERVICE_LOOKUP_CALLEES, 'providesServices'];
151151
* and a single "known bad" bucket would let one be closed by the other's fix.
152152
*/
153153
const KNOWN_PRE_BIND_READS = [
154-
{
155-
plugin: 'com.objectstack.auth',
156-
verdict: 'undeclared',
157-
issue: '#11579',
158-
note:
159-
'AuthPlugin reaches getService(\'settings\') at depth 3 from three start()-registered ' +
160-
'kernel:ready hooks and calls settings.getNamespace(\'auth\') there. In the `os serve` ' +
161-
'composition AuthPlugin is used() before the capability loop registers ' +
162-
'SettingsServicePlugin, so its hooks fire first. Repair is the #10250 declaration.',
163-
},
164154
{
165155
plugin: 'com.objectstack.mcp',
166156
verdict: 'unfixable-by-declaration',

0 commit comments

Comments
 (0)