Plugin system (2/8): host foundation + plugin SDK

[ccdwyer]

What Changed

The server plugin host: a per-plugin Effect Scope lifecycle (verify → import → register → migrate → recover → mount → ready), PluginMigrator (plugin tables gated to a p_<id>_* prefix at migration time via a sqlite_master diff, core migration 034), PluginModuleLoader + an ESM resolve hook sharing the host's effect/SDK singletons, a single-writer PluginLockfileStore, and the @t3tools/plugin-sdk package.

Why

Part 2 of 8 of a runtime plugin system (see #3725 for the full overview + which parts deliver what). Builds on #3725. This is the foundation every capability and the marketplace sit on: it owns each plugin's lifecycle, isolation scope, and on-disk state.

Review — four-model tribunal (GPT-5.5 · Grok · GLM-5.2 · Fable)

• start bypassed the single-flight lock (MUST): an enable/install RPC racing startup could double-load a plugin and orphan a runtime scope → routed through the per-plugin activation lock.
• Effect.exit doesn't capture external interrupts in effect beta.78 (MUST): on real shutdown / a canceled RPC, Scope.close + the activating-marker clear never ran → wrapped the ladder in uninterruptibleMask (confirmed against the vendored runtime; verified by runtime probes since the integration test hangs in vitest under forked activation).
• SDK writeFileAtomic couldn't replace an existing file (MUST) → overwrite-permitted rename mode.
• SHOULDs: non-atomic stale-lock reclaim → atomic rename-claim + owner-token re-verify; singleton-resolution latching before/despite failure; non-crash-safe preserve-data; no host-shutdown scope-close; manifest-version validation.

UI Changes

N/A — server only.

Checklist

• This PR is small and focused
• I explained what changed and why
• Screenshots for UI changes (no UI)
• Video for animation/interaction (N/A)

https://claude.ai/code/session_011H86UHL1RPuWBX3LBTUPsk

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 6b99cbb4-d31f-41af-98ce-6147ea64f594

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[macroscopeapp]

🟠 High orchestration/decider.ts:237

decideOrchestrationCommand copies command.owner directly into the emitted thread.created event payload. Because thread.create is a client-dispatchable command type, an untrusted client can submit owner: "plugin:test", which produces a thread marked as plugin-owned. The owner-based read-model filters then hide that thread from user-facing snapshots, lists, relay, and shell streams — so clients can forge server-only plugin threads that are invisible in the UI. Consider defaulting owner to "user" unconditionally for client-dispatched thread.create commands rather than trusting command.owner.

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/orchestration/decider.ts around line 237:

`decideOrchestrationCommand` copies `command.owner` directly into the emitted `thread.created` event payload. Because `thread.create` is a client-dispatchable command type, an untrusted client can submit `owner: "plugin:test"`, which produces a thread marked as plugin-owned. The owner-based read-model filters then hide that thread from user-facing snapshots, lists, relay, and shell streams — so clients can forge server-only plugin threads that are invisible in the UI. Consider defaulting `owner` to `"user"` unconditionally for client-dispatched `thread.create` commands rather than trusting `command.owner`.

[macroscopeapp]

🟠 High src/index.ts:83

PluginHostApi declares each capability as Effect.Effect<..., PluginCapabilityUnavailable>, but the host implementation raises unavailable capabilities via Effect.die(...), producing an unrecoverable defect rather than the typed error. Any plugin that probes an unavailable capability with Effect.catchTag('PluginCapabilityUnavailable') or catchAll will still crash, because defects bypass the typed error channel. The advertised type contract is violated: plugins cannot recover from capability absence as the types promise. Either build these effects with Effect.fail(new PluginCapabilityUnavailable(...)) so they land in the typed error channel, or document that unavailable capabilities abort the fiber and should not be caught.

Also found in 1 other location(s)

apps/server/src/plugins/PluginHost.ts:128

unavailable uses Effect.die(...) to signal missing host capabilities, but PluginHostApi exposes these fields as Effect<..., PluginCapabilityUnavailable>. A plugin that tries to recover with normal typed handlers like Effect.catch/catchTag will not see this error, because defects bypass the typed error channel. Any plugin probing an optional capability will instead defect and fail activation or crash its service instead of handling PluginCapabilityUnavailable gracefully.

🤖 Copy this AI Prompt to have your agent fix this:

In file @packages/plugin-sdk/src/index.ts around line 83:

`PluginHostApi` declares each capability as `Effect.Effect<..., PluginCapabilityUnavailable>`, but the host implementation raises unavailable capabilities via `Effect.die(...)`, producing an unrecoverable defect rather than the typed error. Any plugin that probes an unavailable capability with `Effect.catchTag('PluginCapabilityUnavailable')` or `catchAll` will still crash, because defects bypass the typed error channel. The advertised type contract is violated: plugins cannot recover from capability absence as the types promise. Either build these effects with `Effect.fail(new PluginCapabilityUnavailable(...))` so they land in the typed error channel, or document that unavailable capabilities abort the fiber and should not be caught.

Also found in 1 other location(s):
- apps/server/src/plugins/PluginHost.ts:128 -- `unavailable` uses `Effect.die(...)` to signal missing host capabilities, but `PluginHostApi` exposes these fields as `Effect<..., PluginCapabilityUnavailable>`. A plugin that tries to recover with normal typed handlers like `Effect.catch`/`catchTag` will not see this error, because defects bypass the typed error channel. Any plugin probing an optional capability will instead defect and fail activation or crash its service instead of handling `PluginCapabilityUnavailable` gracefully.

[macroscopeapp]

🟡 Medium plugins/PluginHost.ts:274

The per-plugin Scope created at line 274 is only closed in the activation-failure branch (line 323). When a plugin activates successfully, nothing ever closes that scope — there is no shutdown hook or finalizer attached to the host's own lifetime. As a result, every service forked into that scope at lines 291–294 keeps running until process exit, and the scoped cleanup (interrupting background fibers, releasing resources) never runs on normal host shutdown. Consider adding a finalizer that closes each plugin's scope when the PluginHost itself shuts down, for example by forking the plugin lifecycle into a scope owned by start or by registering Scope.close(scope, Exit.interrupt(...)) as a finalizer on the host's scope.

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/plugins/PluginHost.ts around line 274:

The per-plugin `Scope` created at line 274 is only closed in the activation-failure branch (line 323). When a plugin activates successfully, nothing ever closes that scope — there is no shutdown hook or finalizer attached to the host's own lifetime. As a result, every service forked into that scope at lines 291–294 keeps running until process exit, and the scoped cleanup (interrupting background fibers, releasing resources) never runs on normal host shutdown. Consider adding a finalizer that closes each plugin's `scope` when the `PluginHost` itself shuts down, for example by forking the plugin lifecycle into a scope owned by `start` or by registering `Scope.close(scope, Exit.interrupt(...))` as a finalizer on the host's scope.

[macroscopeapp]

🟡 Medium plugins/PluginLockfileStore.ts:22

STALE_LOCK_MS = 60_000 lets a second process reclaim a lock that a still-live writer is holding. If the writer pauses longer than 60s (OS suspend, debugger stop, event-loop stall), the second process deletes and recreates the lock file, and both processes then write the lockfile concurrently because the first process still believes it owns the lock. Consider using an OS-level advisory lock (e.g. flock) instead of a pid-file with an age threshold, or document why this 60s heuristic is acceptable for the expected runtime environment.

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/plugins/PluginLockfileStore.ts around line 22:

`STALE_LOCK_MS = 60_000` lets a second process reclaim a lock that a still-live writer is holding. If the writer pauses longer than 60s (OS suspend, debugger stop, event-loop stall), the second process deletes and recreates the lock file, and both processes then write the lockfile concurrently because the first process still believes it owns the lock. Consider using an OS-level advisory lock (e.g. `flock`) instead of a pid-file with an age threshold, or document why this 60s heuristic is acceptable for the expected runtime environment.

[macroscopeapp]

🟠 High plugins/PluginMigrator.ts:96

changedObjects compares sqlite_master rows only by name, type, and sql. A migration that DROPs a non-plugin table and recreates it with identical CREATE TABLE SQL produces before/after snapshots that are identical, so removedObjects detects no removal and validateMigrationObjects accepts the migration. The original table data is permanently lost despite the migration passing validation. Consider snapshotting table row counts or contents for non-plugin objects, or blocking any DROP/CREATE on non-plugin tables regardless of whether the final schema matches.

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/plugins/PluginMigrator.ts around line 96:

`changedObjects` compares `sqlite_master` rows only by `name`, `type`, and `sql`. A migration that `DROP`s a non-plugin table and recreates it with identical `CREATE TABLE` SQL produces before/after snapshots that are identical, so `removedObjects` detects no removal and `validateMigrationObjects` accepts the migration. The original table data is permanently lost despite the migration passing validation. Consider snapshotting table row counts or contents for non-plugin objects, or blocking any `DROP`/`CREATE` on non-plugin tables regardless of whether the final schema matches.

[macroscopeapp]

🟡 Medium plugins/PluginHost.ts:321

If startup is interrupted while a plugin is activating, the activation.pipe(Scope.provide(scope), Effect.exit) call at line 321 never reaches the Scope.close(scope, exit) / updateFailure cleanup below, so the plugin scope leaks and activation.activatingSince stays set. On the next boot, reconcilePendingState sees that stale activatingSince marker, treats it as a crash, and disables the plugin after a single interrupted startup. Consider handling interruption — for example by catching the interruption cause explicitly or running activation with Effect.ensuring so the scope is closed and activatingSince is cleared even when the effect is interrupted.

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/plugins/PluginHost.ts around line 321:

If startup is interrupted while a plugin is activating, the `activation.pipe(Scope.provide(scope), Effect.exit)` call at line 321 never reaches the `Scope.close(scope, exit)` / `updateFailure` cleanup below, so the plugin scope leaks and `activation.activatingSince` stays set. On the next boot, `reconcilePendingState` sees that stale `activatingSince` marker, treats it as a crash, and disables the plugin after a single interrupted startup. Consider handling interruption — for example by catching the interruption cause explicitly or running `activation` with `Effect.ensuring` so the scope is closed and `activatingSince` is cleared even when the effect is interrupted.

[macroscopeapp]

🟡 Medium src/plugin.ts:35

RelativeEntryPath only rejects paths starting with / or \, so Windows drive-qualified absolute paths like C:\plugins\entry.js or C:/plugins/entry.js pass validation. Code that resolves these manifest entries as relative plugin paths can be redirected outside the plugin package on Windows. Consider also rejecting paths that match a Windows drive-letter pattern (e.g. /^[a-zA-Z]:[\/]/).

🤖 Copy this AI Prompt to have your agent fix this:

In file @packages/contracts/src/plugin.ts around line 35:

`RelativeEntryPath` only rejects paths starting with `/` or `\`, so Windows drive-qualified absolute paths like `C:\plugins\entry.js` or `C:/plugins/entry.js` pass validation. Code that resolves these manifest entries as relative plugin paths can be redirected outside the plugin package on Windows. Consider also rejecting paths that match a Windows drive-letter pattern (e.g. `/^[a-zA-Z]:[\/]/`).

[macroscopeapp]

🟠 High plugins/PluginMigrator.ts:164

The run function sorts migrations by version and applies every migration with migration.version > recordedHead, but it never checks that the provided versions are contiguous. When the database has recordedHead = 1 and the new bundle contains versions [1, 3], the loop skips 1 (already applied), runs migration 3, and records version 3 in plugin_migrations — even though migration 2 was never applied. Future runs treat recordedHead = 3 as up to date, so migration 2 can never be applied, leaving the schema in an inconsistent state. Consider verifying contiguity of the provided versions in validateMigrationList.

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/plugins/PluginMigrator.ts around line 164:

The `run` function sorts migrations by version and applies every migration with `migration.version > recordedHead`, but it never checks that the provided versions are contiguous. When the database has `recordedHead = 1` and the new bundle contains versions `[1, 3]`, the loop skips `1` (already applied), runs migration `3`, and records version `3` in `plugin_migrations` — even though migration `2` was never applied. Future runs treat `recordedHead = 3` as up to date, so migration `2` can never be applied, leaving the schema in an inconsistent state. Consider verifying contiguity of the provided versions in `validateMigrationList`.

[macroscopeapp]

🟡 Medium plugins/PluginHost.ts:154

upgradeLockfileEntry copies the old plugin's activation object into the upgraded version, carrying over crashCount and a non-null activatingSince. When the new version fails to activate, reconcilePendingState sees the stale crashCount and can mark it as a repeated crash after a single failure. A stale non-null activatingSince can also make the host treat the freshly upgraded plugin as if it were mid-activation. The upgraded entry should reset activation to { activatingSince: null, crashCount: 0 }.

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/plugins/PluginHost.ts around line 154:

`upgradeLockfileEntry` copies the old plugin's `activation` object into the upgraded version, carrying over `crashCount` and a non-null `activatingSince`. When the new version fails to activate, `reconcilePendingState` sees the stale `crashCount` and can mark it as a repeated crash after a single failure. A stale non-null `activatingSince` can also make the host treat the freshly upgraded plugin as if it were mid-activation. The upgraded entry should reset `activation` to `{ activatingSince: null, crashCount: 0 }`.