Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions .changeset/adr-0116-followups.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
---
"@objectstack/plugin-auth": patch
"@objectstack/plugin-webhooks": patch
---

fix(plugin-auth,plugin-webhooks): retire a dead degrade branch and an implicit transitive dependency (ADR-0116 follow-ups, #4187)

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('No data
engine service found - 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
even earlier with `Dependency … not found`. The branch is removed and the real
contract is declared — `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
`com.objectstack.service.messaging`, which in turn depends on ObjectQL, the
actual provider. That works today and would have broken silently the day
messaging stopped depending on the engine — surfacing 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 now checked rather than
inherited.

Docs: `content/docs/plugins/anatomy.mdx` gains the three ADR-0116 fields and
the decision rule for resolving a service inside `init()` (hard dependency vs
`optionalDependencies` + `requiresServices`), including the two traps behind
these fixes — don't rely on a transitive provider, and don't write an
`if (!svc)` fallback after a bare `getService`. The api-registry example
declares the contract on all seven of its plugins instead of relying on
`kernel.use()` order.
64 changes: 64 additions & 0 deletions content/docs/plugins/anatomy.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,32 @@ export interface Plugin {
* Dependencies (Optional)
* List of other plugin names that this plugin depends on.
* The kernel ensures these plugins are initialized before this one.
* A name that is not composed on the kernel fails the boot.
*/
dependencies?: string[];

/**
* Soft dependencies (Optional) — order-if-present, ADR-0116.
* Hoisted ahead exactly like `dependencies` when composed, silently
* skipped when absent. For plugins that degrade without the dependency
* but must never initialize before it.
*/
optionalDependencies?: string[];

/**
* Services this plugin resolves SYNCHRONOUSLY during init() (ADR-0116).
* Validated before Phase 1 and again immediately before this plugin's
* init, so a misordered composition fails with a named error instead of
* crashing inside your init.
*/
requiresServices?: string[];

/**
* Services this plugin's init() registers UNCONDITIONALLY (ADR-0116).
* Never declare an option-gated registration.
*/
providesServices?: string[];

/**
* Init Phase (REQUIRED)
* Called when the kernel is initializing. Use this to:
Expand Down Expand Up @@ -242,6 +265,47 @@ Plugins follow a strict three-phase lifecycle managed by the kernel:
2. **start()** — Called after *all* plugins have initialized. Start servers, connect to databases, or execute main logic.
3. **destroy()** — Called during shutdown, in reverse order. Clean up connections, timers, and resources.

### Resolving a service inside `init()` — declare it

`kernel.use()` **registration order is not a contract**. Init order comes from
the dependency graph, so "it works because I registered it later" is luck, and
it is the recipe behind two production bugs (a server that booted with no
tables, then [#4085](https://github.com/objectstack-ai/objectstack/issues/4085)).
Anything you resolve in `start()` needs no declaration — Phase 1 completes
before any `start()` runs. For `init()`, pick one of two:

| Your plugin… | Declare |
| :--- | :--- |
| **cannot work** without the provider | `dependencies: ['<provider plugin>']` — plus `requiresServices` to say which service, and why |
| **degrades** without it, but must init after it when present | `optionalDependencies: ['<provider plugin>']` + `requiresServices: ['<service>']` |

Resolving a service in `init()` while declaring **neither** is the bug shape:
it works until someone composes the stack in a different order, and then fails
inside your `init()` with an error that names the service but not the cause.

```ts
class MyPlugin implements Plugin {
// Hard: the engine is always composed with this plugin.
dependencies = ['com.objectstack.engine.objectql'];
requiresServices = ['manifest']; // what the dependency is FOR

async init(ctx: PluginContext) {
ctx.getService<{ register(m: any): void }>('manifest').register(mySchema);
}
}
```

Two rules worth stating explicitly, because both have already caused bugs:

- **Declare the requirement directly, not transitively.** Depending on a plugin
that *happens* to depend on the real provider works until that plugin's own
dependencies change, and the breakage then surfaces in your `init()`.
- **A service you probe behind `try`/`catch` is not a requirement.** Put only
hard, no-fallback needs in `requiresServices` — and if you write a fallback
branch, make sure it can actually run: `getService` **throws** for a missing
service, it never returns `undefined`, so `if (!svc) { /* degrade */ }` after
a bare `getService` is dead code advertising a mode you do not have.

## Next Steps

- **[Plugin System](/docs/plugins)** — Module overview: architecture, configuration, and built-in plugins
Expand Down
18 changes: 18 additions & 0 deletions packages/core/examples/api-registry-example.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ async function example1_BasicApiRegistration() {
const customerPlugin: Plugin = {
name: 'customer-plugin',
version: '1.0.0',
// init() resolves `api-registry` synchronously with no fallback, so the
// ordering requirement is DECLARED rather than left to the `kernel.use()`
// order above (ADR-0116). Registration order is not a contract — the
// kernel orders from this graph.
dependencies: ['com.objectstack.core.api-registry'],
requiresServices: ['api-registry'],
init: async (ctx) => {
const registry = ctx.getService<ApiRegistry>('api-registry');

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

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

Expand Down Expand Up @@ -282,6 +292,8 @@ async function example3_ConflictResolution() {
// Core Plugin - High priority
const corePlugin: Plugin = {
name: 'core-plugin',
dependencies: ['com.objectstack.core.api-registry'],
requiresServices: ['api-registry'],
init: async (ctx) => {
const registry = ctx.getService<ApiRegistry>('api-registry');

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

Expand Down Expand Up @@ -361,6 +375,8 @@ async function example4_CustomProtocol() {

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

Expand Down Expand Up @@ -431,6 +447,8 @@ async function example5_DynamicSchemas() {

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

Expand Down
31 changes: 25 additions & 6 deletions packages/plugins/plugin-auth/src/auth-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,19 @@ export class AuthPlugin implements Plugin {
providesServices = ['auth', 'tenancy'];
type = 'standard';
version = '1.0.0';
dependencies: string[] = ['com.objectstack.engine.objectql']; // manifest service required

dependencies: string[] = ['com.objectstack.engine.objectql'];
/**
* What that dependency is FOR, stated so the kernel can check it (#4187).
* `init()` resolves both synchronously with no fallback: `data` builds the
* AuthManager config, `manifest` registers the auth objects. ObjectQL
* registers both unconditionally and the hard dependency above hoists it
* ahead, so this never changes whether the boot succeeds — it changes what
* a broken composition SAYS, and replaces the trailing `// manifest service
* required` comment with the machine-checked form of the same claim.
*/
requiresServices = ['data', 'manifest'];


private options: AuthPluginOptions;
private authManager: AuthManager | null = null;
/** ADR-0093 D4 — the tenancy service registered in init(); reused at kernel:ready. */
Expand Down Expand Up @@ -199,11 +210,19 @@ export class AuthPlugin implements Plugin {
throw new Error('AuthPlugin: secret is required');
}

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

const authConfig: AuthManagerOptions & AuthPluginOptions = {
...this.options,
Expand Down
10 changes: 10 additions & 0 deletions packages/plugins/plugin-webhooks/src/webhook-outbox-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,16 @@ export class WebhookOutboxPlugin implements Plugin {
version = '2.0.0';
type = 'standard' as const;
dependencies = ['com.objectstack.service.messaging'];
/**
* `init()` registers this plugin's schema through `manifest` with no
* fallback. Until #4187 that was safe only TRANSITIVELY — messaging happens
* to depend on ObjectQL, which provides `manifest` — so the guarantee would
* have evaporated silently the day messaging stopped depending on the
* engine, and the failure would have surfaced as an unrelated plugin's
* init crash. Declaring the requirement directly makes the kernel check it
* regardless of what messaging depends on.
*/
requiresServices = ['manifest'];

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