diff --git a/server/src/__tests__/plugin-worker-manager.test.ts b/server/src/__tests__/plugin-worker-manager.test.ts index 216313a7d0..451e7d1c60 100644 --- a/server/src/__tests__/plugin-worker-manager.test.ts +++ b/server/src/__tests__/plugin-worker-manager.test.ts @@ -13,6 +13,7 @@ import { appendStderrExcerpt, createPluginWorkerHandle, formatWorkerFailureMessage, + resolveRpcCallTimeoutMs, } from "../services/plugin-worker-manager.js"; const FIXTURES_DIR = path.join(path.dirname(fileURLToPath(import.meta.url)), "fixtures"); @@ -35,6 +36,56 @@ const TEST_MANIFEST: PaperclipPluginManifestV1 = { entrypoints: { worker: "dist/worker.js" }, }; +describe("resolveRpcCallTimeoutMs", () => { + const MAX_RPC_TIMEOUT_MS = 15 * 60 * 1_000; + const MAX_NODE_TIMER_TIMEOUT_MS = 2_147_483_647; + const DEFAULT_RPC_TIMEOUT_MS = 30_000; + + it("honors an explicit timeout above the 15-minute default ceiling", () => { + // The sandbox environment driver requests ~4h + 30s buffer for + // environmentExecute; this must not be clamped to 15 minutes. + const fourHoursPlusBuffer = 4 * 60 * 60 * 1_000 + 30_000; + expect(resolveRpcCallTimeoutMs(fourHoursPlusBuffer, DEFAULT_RPC_TIMEOUT_MS)).toBe( + fourHoursPlusBuffer, + ); + }); + + it("honors an explicit timeout below the ceiling", () => { + expect(resolveRpcCallTimeoutMs(100, DEFAULT_RPC_TIMEOUT_MS)).toBe(100); + expect(resolveRpcCallTimeoutMs(MAX_RPC_TIMEOUT_MS - 1, DEFAULT_RPC_TIMEOUT_MS)).toBe( + MAX_RPC_TIMEOUT_MS - 1, + ); + }); + + it("truncates fractional explicit timeouts", () => { + expect(resolveRpcCallTimeoutMs(1_000.9, DEFAULT_RPC_TIMEOUT_MS)).toBe(1_000); + }); + + it("normalizes explicit timeouts to Node's timer-safe range", () => { + expect(resolveRpcCallTimeoutMs(0.5, DEFAULT_RPC_TIMEOUT_MS)).toBe(1); + expect(resolveRpcCallTimeoutMs(MAX_NODE_TIMER_TIMEOUT_MS + 1, DEFAULT_RPC_TIMEOUT_MS)).toBe( + MAX_NODE_TIMER_TIMEOUT_MS, + ); + }); + + it("uses the default timeout when no explicit timeout is provided", () => { + expect(resolveRpcCallTimeoutMs(undefined, DEFAULT_RPC_TIMEOUT_MS)).toBe( + DEFAULT_RPC_TIMEOUT_MS, + ); + }); + + it("clamps only the default path to the 15-minute ceiling", () => { + expect(resolveRpcCallTimeoutMs(undefined, 24 * 60 * 60 * 1_000)).toBe(MAX_RPC_TIMEOUT_MS); + }); + + it("falls back to the clamped default for unusable explicit timeouts", () => { + for (const bad of [0, -1, Number.NaN, Number.POSITIVE_INFINITY]) { + expect(resolveRpcCallTimeoutMs(bad, DEFAULT_RPC_TIMEOUT_MS)).toBe(DEFAULT_RPC_TIMEOUT_MS); + } + expect(resolveRpcCallTimeoutMs(Number.NaN, 24 * 60 * 60 * 1_000)).toBe(MAX_RPC_TIMEOUT_MS); + }); +}); + describe("plugin-worker-manager stderr failure context", () => { it("appends worker stderr context to failure messages", () => { expect( diff --git a/server/src/services/plugin-worker-manager.ts b/server/src/services/plugin-worker-manager.ts index 25e6882a67..af2c6b78ba 100644 --- a/server/src/services/plugin-worker-manager.ts +++ b/server/src/services/plugin-worker-manager.ts @@ -61,9 +61,23 @@ import { logger } from "../middleware/logger.js"; /** Default timeout for RPC calls in milliseconds. */ const DEFAULT_RPC_TIMEOUT_MS = 30_000; -/** Hard upper bound for any RPC timeout (15 minutes). Prevents unbounded waits. */ +/** + * Upper bound for the *default* RPC timeout path (15 minutes). Explicit + * caller-supplied timeouts are not subject to this cap: execute-class RPCs such + * as `environmentExecute` run entire sandboxed agent sessions in one call and + * their callers deliberately request multi-hour budgets (see + * `resolvePluginExecuteRpcTimeoutMs` in plugin-environment-driver.ts). + * Clamping those explicit budgets here killed long sandboxed runs mid-work. + */ const MAX_RPC_TIMEOUT_MS = 15 * 60 * 1_000; +/** + * Maximum delay accepted by Node timers before Node clamps the timeout to 1ms. + * Keep accepted explicit RPC budgets inside this range before calling + * setTimeout, otherwise a huge timeout can expire almost immediately. + */ +const MAX_NODE_TIMER_TIMEOUT_MS = 2_147_483_647; + /** Timeout for the initialize RPC call. */ const INITIALIZE_TIMEOUT_MS = 15_000; @@ -154,6 +168,30 @@ export function formatWorkerFailureMessage(message: string, stderrExcerpt: strin return `${message}\n\nWorker stderr:\n${excerpt}`; } +/** + * Resolve the effective timeout for an RPC call. + * + * An explicit, positive, finite caller-supplied timeout bypasses the 15-minute + * RPC cap after normalization to Node's timer-safe integer range. Callers that + * pass one (e.g. the environment driver for `environmentExecute`) own their + * budget, and independent inactivity/safety guards bound hung runs. Only the + * default path (no usable explicit timeout) is clamped to MAX_RPC_TIMEOUT_MS so + * ordinary plugin calls stay bounded. + */ +export function resolveRpcCallTimeoutMs( + explicitTimeoutMs: number | undefined, + defaultTimeoutMs: number, +): number { + if ( + explicitTimeoutMs !== undefined && + Number.isFinite(explicitTimeoutMs) && + explicitTimeoutMs > 0 + ) { + return Math.min(Math.max(Math.trunc(explicitTimeoutMs), 1), MAX_NODE_TIMER_TIMEOUT_MS); + } + return Math.min(defaultTimeoutMs, MAX_RPC_TIMEOUT_MS); +} + /** * Options for starting a worker process. */ @@ -1232,7 +1270,7 @@ export function createPluginWorkerHandle( } const id = nextRequestId++; - const timeout = Math.min(timeoutMs ?? rpcTimeoutMs, MAX_RPC_TIMEOUT_MS); + const timeout = resolveRpcCallTimeoutMs(timeoutMs, rpcTimeoutMs); const invocationScope = deriveInvocationScope(method, params); const invocation = invocationScope ? registerInvocation(invocationScope) : null; @@ -1357,6 +1395,9 @@ export function createPluginWorkerHandle( notify(method: string, params: unknown) { if (status !== "running") return; const invocationScope = deriveInvocationScope(method, params); + // Notifications have no response to settle on, so the invocation scope + // is GC'd by TTL. Call-path invocations are registered without a TTL and + // cleared on settlement, so they survive arbitrarily long call timeouts. const invocation = invocationScope ? registerInvocation(invocationScope, MAX_RPC_TIMEOUT_MS) : null; try { sendMessage({