diff --git a/server/src/__tests__/plugin-worker-process-registry.test.ts b/server/src/__tests__/plugin-worker-process-registry.test.ts new file mode 100644 index 00000000000..6830d7adbc9 --- /dev/null +++ b/server/src/__tests__/plugin-worker-process-registry.test.ts @@ -0,0 +1,68 @@ +import { afterEach, describe, expect, it } from "vitest"; + +import { + clearProcessPluginWorkerManager, + getProcessPluginWorkerManager, + setProcessPluginWorkerManager, +} from "../services/plugin-worker-process-registry.js"; + +/** + * Guards the fix for a bug that recurred three times: the plugin worker manager + * is process-scoped, but it was threaded as an OPTIONAL parameter through every + * runtime construction site. Any caller that omitted it built a run engine that + * could never acquire a sandbox lease, and it only surfaced when a customer hit + * that path. Observed in production on 2026-07-26 via `assignment` and + * `automation` dispatches, which reach the runtime through `heartbeatService(db)` + * called with no options at all. + */ +describe("process plugin worker manager registry", () => { + afterEach(() => { + clearProcessPluginWorkerManager(); + }); + + it("returns undefined when nothing has registered a manager", () => { + clearProcessPluginWorkerManager(); + expect(getProcessPluginWorkerManager()).toBeUndefined(); + }); + + it("hands back the manager registered for this process", () => { + const manager = { marker: "process-manager" } as never; + setProcessPluginWorkerManager(manager); + expect(getProcessPluginWorkerManager()).toBe(manager); + }); + + it("allows a test harness to replace the registration", () => { + const first = { marker: "first" } as never; + const second = { marker: "second" } as never; + setProcessPluginWorkerManager(first); + setProcessPluginWorkerManager(second); + expect(getProcessPluginWorkerManager()).toBe(second); + }); + + it("treats an undefined registration as no manager rather than throwing", () => { + // Contexts that legitimately have no worker (migrations, CLI entrypoints) + // must keep working; the sandbox driver produces the accurate error instead. + setProcessPluginWorkerManager(undefined); + expect(getProcessPluginWorkerManager()).toBeUndefined(); + }); +}); + +describe("sandbox driver manager resolution", () => { + afterEach(() => { + clearProcessPluginWorkerManager(); + }); + + it("prefers an explicitly passed manager over the process registration", () => { + // The explicit parameter has to keep winning, because tests inject their own + // manager and must not be affected by whatever the process registered. + const registered = { marker: "registered" } as never; + const explicit = { marker: "explicit" } as never; + setProcessPluginWorkerManager(registered); + + const resolve = (options: { pluginWorkerManager?: unknown }) => + options.pluginWorkerManager ?? getProcessPluginWorkerManager(); + + expect(resolve({ pluginWorkerManager: explicit })).toBe(explicit); + expect(resolve({})).toBe(registered); + }); +}); diff --git a/server/src/app.ts b/server/src/app.ts index 32e3d6e2ad7..add199ed54c 100644 --- a/server/src/app.ts +++ b/server/src/app.ts @@ -64,6 +64,7 @@ import { applyUiBranding, BRAND_DIR_PUBLIC_PATH, getBrandDir } from "./ui-brandi import { logger } from "./middleware/logger.js"; import { DEFAULT_LOCAL_PLUGIN_DIR, pluginLoader } from "./services/plugin-loader.js"; import { createPluginWorkerManager, type PluginWorkerManager } from "./services/plugin-worker-manager.js"; +import { setProcessPluginWorkerManager } from "./services/plugin-worker-process-registry.js"; import { createPluginJobScheduler } from "./services/plugin-job-scheduler.js"; import { pluginJobStore } from "./services/plugin-job-store.js"; import { createPluginToolDispatcher } from "./services/plugin-tool-dispatcher.js"; @@ -239,6 +240,11 @@ export async function createApp( const hostServicesDisposers = new Map void>(); const workerManager = opts.pluginWorkerManager ?? createPluginWorkerManager(); + // Register the manager for this process so that any code path which dispatches + // a run can find it without being handed it explicitly. Without this, a + // construction site that omits the manager builds a run engine that can never + // acquire a sandbox lease, and it only shows up when a customer hits that path. + setProcessPluginWorkerManager(workerManager); // Mount API routes const api = Router(); diff --git a/server/src/services/environment-runtime.ts b/server/src/services/environment-runtime.ts index dfca69817c4..481c43590ad 100644 --- a/server/src/services/environment-runtime.ts +++ b/server/src/services/environment-runtime.ts @@ -40,6 +40,7 @@ import { } from "./sandbox-provider-runtime.js"; import { pluginRegistryService } from "./plugin-registry.js"; import type { PluginWorkerManager } from "./plugin-worker-manager.js"; +import { getProcessPluginWorkerManager } from "./plugin-worker-process-registry.js"; import type { PluginStreamBus } from "./plugin-stream-bus.js"; import { destroyPluginEnvironmentLease, @@ -730,7 +731,15 @@ function createSandboxEnvironmentDriver( pluginWorkerReadyPollMs?: number; } = {}, ): EnvironmentRuntimeDriver { - const pluginWorkerManager = options.pluginWorkerManager; + // Fall back to the process-scoped manager when a caller did not pass one. + // Every path that dispatches a run funnels through here, so this single + // fallback covers all of them: the six runtime construction sites, and the + // `heartbeatService(db)` calls that pass no options at all (which is how + // `assignment` and `automation` dispatches used to reach a manager-less + // runtime and fail with "sandbox plugin workers are unavailable in this + // server process"). An explicitly passed manager still wins, so tests can + // inject their own. + const pluginWorkerManager = options.pluginWorkerManager ?? getProcessPluginWorkerManager(); const pluginWorkerReadyTimeoutMs = options.pluginWorkerReadyTimeoutMs ?? DEFAULT_PLUGIN_SANDBOX_WORKER_READY_TIMEOUT_MS; const pluginWorkerReadyPollMs = options.pluginWorkerReadyPollMs ?? DEFAULT_PLUGIN_SANDBOX_WORKER_READY_POLL_MS; const environmentsSvc = environmentService(db); diff --git a/server/src/services/plugin-worker-process-registry.ts b/server/src/services/plugin-worker-process-registry.ts new file mode 100644 index 00000000000..9209a68c300 --- /dev/null +++ b/server/src/services/plugin-worker-process-registry.ts @@ -0,0 +1,52 @@ +import type { PluginWorkerManager } from "./plugin-worker-manager.js"; + +/** + * Process-scoped access to the plugin worker manager. + * + * There is exactly ONE plugin worker manager per server process, which is why a + * run that cannot find one fails with "sandbox plugin workers are unavailable in + * this server process". Despite being process-scoped, it used to be threaded as + * an OPTIONAL parameter through six runtime construction sites and dozens of + * route mounts, so any caller that omitted it built a run engine that could + * never acquire a sandbox lease. The omission only surfaced when a customer hit + * that particular path, and it recurred three times: + * + * - routes mounted without it (fixed per-route in #304) + * - `heartbeatService(db)` called with no options at all, which is how + * `assignment` and `automation` dispatches reached a manager-less runtime + * + * Registering it once at boot removes the whole class: a caller can still pass + * one explicitly (tests do), but forgetting to is no longer a failure mode. + * + * Deliberately not a general service locator. It holds this one process-scoped + * dependency, because the alternative was making it a required parameter on + * every construction site, which is both a much larger change and still + * bypassable with `undefined as any`. + */ +let processPluginWorkerManager: PluginWorkerManager | undefined; + +/** + * Register the manager for this process. Called once during app construction. + * Idempotent: registering the same manager twice is a no-op, and replacing it + * is allowed so a test harness can install its own. + */ +export function setProcessPluginWorkerManager(manager: PluginWorkerManager | undefined): void { + processPluginWorkerManager = manager; +} + +/** + * The manager for this process, or undefined when nothing has registered one. + * + * Returning undefined rather than throwing is deliberate: contexts that + * legitimately have no worker (migrations, CLI entrypoints, unit tests that + * never dispatch a run) must keep working, and the sandbox driver already + * produces an accurate, non-retryable error when it genuinely has no manager. + */ +export function getProcessPluginWorkerManager(): PluginWorkerManager | undefined { + return processPluginWorkerManager; +} + +/** Test hook: drop the registration so a test can assert the no-manager path. */ +export function clearProcessPluginWorkerManager(): void { + processPluginWorkerManager = undefined; +}