Skip to content

Commit bb1ce2e

Browse files
authored
fix(plugin-auth,plugin-webhooks): retire a dead degrade branch and an implicit transitive dependency (#4187) (#4211)
Two concrete findings from the ADR-0116 consumer-side audit, plus the authoring rule that would have prevented both. plugin-auth claimed a fallback it did not have: init() ran `const dataEngine = ctx.getService('data'); if (!dataEngine) { warn(... "auth will use in-memory storage") }`. That branch could never execute — getService THROWS for an unregistered service rather than returning undefined, and this plugin declares a hard dependency on ObjectQL (which registers `data` unconditionally), so a kernel without the engine fails earlier still with "Dependency ... not found". The branch is removed and the real contract declared as requiresServices: ['data', 'manifest'], which also replaces a trailing `// manifest service required` comment with the machine-checked form of the same claim. AuthManager keeps its own optional dataEngine guards — it is usable outside the plugin. plugin-webhook-outbox was protected only transitively: it resolves `manifest` in init() with no fallback while depending on messaging, which in turn depends on ObjectQL, the actual provider. That would have broken silently the day messaging stopped depending on the engine, and surfaced as a crash inside an unrelated plugin's init. It now declares requiresServices: ['manifest'] directly. Neither change alters ordering or boot outcomes on any current composition — both plugins were already ordered correctly. What changes is what a broken composition says, and that the guarantees are checked rather than inherited. Docs: anatomy.mdx gains the three ADR-0116 fields and the decision rule for resolving a service inside init(), including the two traps behind these fixes. The api-registry example declares the contract on all seven of its plugins instead of relying on kernel.use() order. Refs #4187
1 parent 6f23667 commit bb1ce2e

5 files changed

Lines changed: 159 additions & 6 deletions

File tree

.changeset/adr-0116-followups.md

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
"@objectstack/plugin-auth": patch
3+
"@objectstack/plugin-webhooks": patch
4+
---
5+
6+
fix(plugin-auth,plugin-webhooks): retire a dead degrade branch and an implicit transitive dependency (ADR-0116 follow-ups, #4187)
7+
8+
Two concrete findings from the ADR-0116 consumer-side audit, plus the
9+
authoring rule that would have prevented both.
10+
11+
**`plugin-auth` claimed a fallback it did not have.** `init()` ran
12+
`const dataEngine = ctx.getService('data'); if (!dataEngine) { warn('No data
13+
engine service found - auth will use in-memory storage') }`. That branch could
14+
never execute: `getService` **throws** for an unregistered service rather than
15+
returning `undefined`, and this plugin declares a hard dependency on ObjectQL
16+
(which registers `data` unconditionally), so a kernel without the engine fails
17+
even earlier with `Dependency … not found`. The branch is removed and the real
18+
contract is declared — `requiresServices: ['data', 'manifest']` — which also
19+
replaces a trailing `// manifest service required` comment with the
20+
machine-checked form of the same claim. `AuthManager` keeps its own optional
21+
`dataEngine` guards: it is usable outside the plugin.
22+
23+
**`plugin-webhook-outbox` was protected only transitively.** It resolves
24+
`manifest` in `init()` with no fallback while depending on
25+
`com.objectstack.service.messaging`, which in turn depends on ObjectQL, the
26+
actual provider. That works today and would have broken silently the day
27+
messaging stopped depending on the engine — surfacing as a crash inside an
28+
unrelated plugin's init. It now declares `requiresServices: ['manifest']`
29+
directly.
30+
31+
Neither change alters ordering or boot outcomes on any current composition:
32+
both plugins were already ordered correctly. What changes is what a broken
33+
composition *says*, and that the guarantees are now checked rather than
34+
inherited.
35+
36+
Docs: `content/docs/plugins/anatomy.mdx` gains the three ADR-0116 fields and
37+
the decision rule for resolving a service inside `init()` (hard dependency vs
38+
`optionalDependencies` + `requiresServices`), including the two traps behind
39+
these fixes — don't rely on a transitive provider, and don't write an
40+
`if (!svc)` fallback after a bare `getService`. The api-registry example
41+
declares the contract on all seven of its plugins instead of relying on
42+
`kernel.use()` order.

content/docs/plugins/anatomy.mdx

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,9 +131,32 @@ export interface Plugin {
131131
* Dependencies (Optional)
132132
* List of other plugin names that this plugin depends on.
133133
* The kernel ensures these plugins are initialized before this one.
134+
* A name that is not composed on the kernel fails the boot.
134135
*/
135136
dependencies?: string[];
136137

138+
/**
139+
* Soft dependencies (Optional) — order-if-present, ADR-0116.
140+
* Hoisted ahead exactly like `dependencies` when composed, silently
141+
* skipped when absent. For plugins that degrade without the dependency
142+
* but must never initialize before it.
143+
*/
144+
optionalDependencies?: string[];
145+
146+
/**
147+
* Services this plugin resolves SYNCHRONOUSLY during init() (ADR-0116).
148+
* Validated before Phase 1 and again immediately before this plugin's
149+
* init, so a misordered composition fails with a named error instead of
150+
* crashing inside your init.
151+
*/
152+
requiresServices?: string[];
153+
154+
/**
155+
* Services this plugin's init() registers UNCONDITIONALLY (ADR-0116).
156+
* Never declare an option-gated registration.
157+
*/
158+
providesServices?: string[];
159+
137160
/**
138161
* Init Phase (REQUIRED)
139162
* Called when the kernel is initializing. Use this to:
@@ -242,6 +265,47 @@ Plugins follow a strict three-phase lifecycle managed by the kernel:
242265
2. **start()** — Called after *all* plugins have initialized. Start servers, connect to databases, or execute main logic.
243266
3. **destroy()** — Called during shutdown, in reverse order. Clean up connections, timers, and resources.
244267

268+
### Resolving a service inside `init()` — declare it
269+
270+
`kernel.use()` **registration order is not a contract**. Init order comes from
271+
the dependency graph, so "it works because I registered it later" is luck, and
272+
it is the recipe behind two production bugs (a server that booted with no
273+
tables, then [#4085](https://github.com/objectstack-ai/objectstack/issues/4085)).
274+
Anything you resolve in `start()` needs no declaration — Phase 1 completes
275+
before any `start()` runs. For `init()`, pick one of two:
276+
277+
| Your plugin… | Declare |
278+
| :--- | :--- |
279+
| **cannot work** without the provider | `dependencies: ['<provider plugin>']` — plus `requiresServices` to say which service, and why |
280+
| **degrades** without it, but must init after it when present | `optionalDependencies: ['<provider plugin>']` + `requiresServices: ['<service>']` |
281+
282+
Resolving a service in `init()` while declaring **neither** is the bug shape:
283+
it works until someone composes the stack in a different order, and then fails
284+
inside your `init()` with an error that names the service but not the cause.
285+
286+
```ts
287+
class MyPlugin implements Plugin {
288+
// Hard: the engine is always composed with this plugin.
289+
dependencies = ['com.objectstack.engine.objectql'];
290+
requiresServices = ['manifest']; // what the dependency is FOR
291+
292+
async init(ctx: PluginContext) {
293+
ctx.getService<{ register(m: any): void }>('manifest').register(mySchema);
294+
}
295+
}
296+
```
297+
298+
Two rules worth stating explicitly, because both have already caused bugs:
299+
300+
- **Declare the requirement directly, not transitively.** Depending on a plugin
301+
that *happens* to depend on the real provider works until that plugin's own
302+
dependencies change, and the breakage then surfaces in your `init()`.
303+
- **A service you probe behind `try`/`catch` is not a requirement.** Put only
304+
hard, no-fallback needs in `requiresServices` — and if you write a fallback
305+
branch, make sure it can actually run: `getService` **throws** for a missing
306+
service, it never returns `undefined`, so `if (!svc) { /* degrade */ }` after
307+
a bare `getService` is dead code advertising a mode you do not have.
308+
245309
## Next Steps
246310

247311
- **[Plugin System](/docs/plugins)** — Module overview: architecture, configuration, and built-in plugins

packages/core/examples/api-registry-example.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,12 @@ async function example1_BasicApiRegistration() {
2424
const customerPlugin: Plugin = {
2525
name: 'customer-plugin',
2626
version: '1.0.0',
27+
// init() resolves `api-registry` synchronously with no fallback, so the
28+
// ordering requirement is DECLARED rather than left to the `kernel.use()`
29+
// order above (ADR-0116). Registration order is not a contract — the
30+
// kernel orders from this graph.
31+
dependencies: ['com.objectstack.core.api-registry'],
32+
requiresServices: ['api-registry'],
2733
init: async (ctx) => {
2834
const registry = ctx.getService<ApiRegistry>('api-registry');
2935

@@ -163,6 +169,8 @@ async function example2_MultiPluginDiscovery() {
163169
// Data Plugin - REST APIs
164170
const dataPlugin: Plugin = {
165171
name: 'data-plugin',
172+
dependencies: ['com.objectstack.core.api-registry'],
173+
requiresServices: ['api-registry'],
166174
init: async (ctx) => {
167175
const registry = ctx.getService<ApiRegistry>('api-registry');
168176

@@ -212,6 +220,8 @@ async function example2_MultiPluginDiscovery() {
212220
// Analytics Plugin - Beta API
213221
const analyticsPlugin: Plugin = {
214222
name: 'analytics-plugin',
223+
dependencies: ['com.objectstack.core.api-registry'],
224+
requiresServices: ['api-registry'],
215225
init: async (ctx) => {
216226
const registry = ctx.getService<ApiRegistry>('api-registry');
217227

@@ -282,6 +292,8 @@ async function example3_ConflictResolution() {
282292
// Core Plugin - High priority
283293
const corePlugin: Plugin = {
284294
name: 'core-plugin',
295+
dependencies: ['com.objectstack.core.api-registry'],
296+
requiresServices: ['api-registry'],
285297
init: async (ctx) => {
286298
const registry = ctx.getService<ApiRegistry>('api-registry');
287299

@@ -310,6 +322,8 @@ async function example3_ConflictResolution() {
310322
// Custom Plugin - Medium priority
311323
const customPlugin: Plugin = {
312324
name: 'custom-plugin',
325+
dependencies: ['com.objectstack.core.api-registry'],
326+
requiresServices: ['api-registry'],
313327
init: async (ctx) => {
314328
const registry = ctx.getService<ApiRegistry>('api-registry');
315329

@@ -361,6 +375,8 @@ async function example4_CustomProtocol() {
361375

362376
const websocketPlugin: Plugin = {
363377
name: 'websocket-plugin',
378+
dependencies: ['com.objectstack.core.api-registry'],
379+
requiresServices: ['api-registry'],
364380
init: async (ctx) => {
365381
const registry = ctx.getService<ApiRegistry>('api-registry');
366382

@@ -431,6 +447,8 @@ async function example5_DynamicSchemas() {
431447

432448
const dynamicPlugin: Plugin = {
433449
name: 'dynamic-plugin',
450+
dependencies: ['com.objectstack.core.api-registry'],
451+
requiresServices: ['api-registry'],
434452
init: async (ctx) => {
435453
const registry = ctx.getService<ApiRegistry>('api-registry');
436454

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

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,19 @@ export class AuthPlugin implements Plugin {
144144
providesServices = ['auth', 'tenancy'];
145145
type = 'standard';
146146
version = '1.0.0';
147-
dependencies: string[] = ['com.objectstack.engine.objectql']; // manifest service required
148-
147+
dependencies: string[] = ['com.objectstack.engine.objectql'];
148+
/**
149+
* What that dependency is FOR, stated so the kernel can check it (#4187).
150+
* `init()` resolves both synchronously with no fallback: `data` builds the
151+
* AuthManager config, `manifest` registers the auth objects. ObjectQL
152+
* registers both unconditionally and the hard dependency above hoists it
153+
* ahead, so this never changes whether the boot succeeds — it changes what
154+
* a broken composition SAYS, and replaces the trailing `// manifest service
155+
* required` comment with the machine-checked form of the same claim.
156+
*/
157+
requiresServices = ['data', 'manifest'];
158+
159+
149160
private options: AuthPluginOptions;
150161
private authManager: AuthManager | null = null;
151162
/** ADR-0093 D4 — the tenancy service registered in init(); reused at kernel:ready. */
@@ -199,11 +210,19 @@ export class AuthPlugin implements Plugin {
199210
throw new Error('AuthPlugin: secret is required');
200211
}
201212

202-
// Get data engine service for database operations
213+
// Get data engine service for database operations. This plugin declares a
214+
// hard dependency on ObjectQL (which registers `data` unconditionally), so
215+
// the service is always present here.
216+
//
217+
// There used to be an `if (!dataEngine) warn('…auth will use in-memory
218+
// storage')` guard under this line. It could never fire and the fallback it
219+
// named did not exist: `getService` THROWS for an unregistered service, it
220+
// does not return undefined, and the hard dependency means a kernel without
221+
// the engine fails earlier still with "Dependency … not found". A branch
222+
// that advertises a tolerance the composition forbids is worse than no
223+
// branch — it reads as a supported degraded mode (#4187). AuthManager keeps
224+
// its own `dataEngine?` guards because it is usable outside this plugin.
203225
const dataEngine = ctx.getService<any>('data');
204-
if (!dataEngine) {
205-
ctx.logger.warn('No data engine service found - auth will use in-memory storage');
206-
}
207226

208227
const authConfig: AuthManagerOptions & AuthPluginOptions = {
209228
...this.options,

packages/plugins/plugin-webhooks/src/webhook-outbox-plugin.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,16 @@ export class WebhookOutboxPlugin implements Plugin {
6262
version = '2.0.0';
6363
type = 'standard' as const;
6464
dependencies = ['com.objectstack.service.messaging'];
65+
/**
66+
* `init()` registers this plugin's schema through `manifest` with no
67+
* fallback. Until #4187 that was safe only TRANSITIVELY — messaging happens
68+
* to depend on ObjectQL, which provides `manifest` — so the guarantee would
69+
* have evaporated silently the day messaging stopped depending on the
70+
* engine, and the failure would have surfaced as an unrelated plugin's
71+
* init crash. Declaring the requirement directly makes the kernel check it
72+
* regardless of what messaging depends on.
73+
*/
74+
requiresServices = ['manifest'];
6575

6676
private autoEnqueuer: AutoEnqueuer | undefined;
6777
/** Engine the provenance hook was bound to, so `dispose()` can unbind it. */

0 commit comments

Comments
 (0)