The only bound on code execution was the in-sandbox Promise.race timer. If the isolate is evicted or OOM-killed in a way that hangs the cross-isolate evaluate RPC without rejecting, that timer never fires and the host awaited the RPC forever, so the MCP client saw only SSE keepalives and never a JSON-RPC frame. Wrap the evaluate RPC in a host-side watchdog that fires after the effective in-sandbox bound plus a grace margin, delivering a descriptive SandboxHostTimeoutError (folded into the ExecuteResult.error channel like the other safe-to-report conditions) instead of open-ended silence. The watchdog is suspended whenever a tool round-trip is outstanding, so a slow tool call or a minutes-long approval pause never trips it; it fires only when the sandbox is supposed to be computing on its own and has gone unresponsive. Emits a structured mcp_execution_host_timeout event for alerting.
Unified · Split
| @@ -0,0 +1,5 @@ | |||
| 1 | --- | ||
| 2 | "executor": patch | ||
| 3 | --- | ||
| 4 | |||
| 5 | Back-stop code execution with a host-side timeout so a wedged sandbox delivers a descriptive error instead of hanging silently. | ||
| @@ -56,3 +56,26 @@ export class SandboxRuntimeError extends Data.TaggedError("SandboxRuntimeError") · 55 unmodified lines | |||
| 56 | readonly message: string; | 56 | readonly message: string; |
| 57 | readonly cause?: unknown; | 57 | readonly cause?: unknown; |
| 58 | }> {} | 58 | }> {} |
| 59 | |||
| 60 | /** | ||
| 61 | * Raised on the host side when a sandbox execution never settles within its | ||
| 62 | * effective timeout bound (plus a grace margin) AND is not merely blocked in a | ||
| 63 | * host round-trip (a tool call or an approval pause). This is the backstop for | ||
| 64 | * a wedged isolate: if the isolate is evicted / OOM-killed in a way that hangs | ||
| 65 | * the cross-isolate RPC without rejecting, the in-sandbox timer never fires and | ||
| 66 | * the host would otherwise wait forever, surfacing to the client as pure | ||
| 67 | * silence. Like `SandboxRuntimeError`, this is a safe-to-report condition (the | ||
| 68 | * execution is unrecoverable but the failure is descriptive, not an internal | ||
| 69 | * defect), so runtimes surface its `message` through the descriptive | ||
| 70 | * `ExecuteResult.error` channel rather than collapsing it to an opaque internal | ||
| 71 | * error. The host clock is suspended while the sandbox is blocked in a host | ||
| 72 | * round-trip, so a long-running tool call or a minutes-long approval pause does | ||
| 73 | * NOT trip it: it fires only when the sandbox is supposed to be computing on | ||
| 74 | * its own and has gone unresponsive. | ||
| 75 | */ | ||
| 76 | export class SandboxHostTimeoutError extends Data.TaggedError("SandboxHostTimeoutError")<{ | ||
| 77 | readonly runtime: string; | ||
| 78 | readonly message: string; | ||
| 79 | readonly timeoutMs: number; | ||
| 80 | readonly elapsedMs: number; | ||
| 81 | }> {} | ||
| @@ -56,6 +56,14 @@ export type ExecuteResult = { · 55 unmodified lines | |||
| 56 | */ | 56 | */ |
| 57 | export interface CodeExecutor<E extends Cause.YieldableError = CodeExecutionError> { | 57 | export interface CodeExecutor<E extends Cause.YieldableError = CodeExecutionError> { |
| 58 | execute(code: string, toolInvoker: SandboxToolInvoker): Effect.Effect<ExecuteResult, E>; | 58 | execute(code: string, toolInvoker: SandboxToolInvoker): Effect.Effect<ExecuteResult, E>; |
| 59 | /** | ||
| 60 | * The effective in-sandbox execution timeout, in milliseconds, that this | ||
| 61 | * runtime enforces on the code it runs. Exposed so a host can derive its own | ||
| 62 | * outer backstop (e.g. this bound plus a grace margin) for the case where the | ||
| 63 | * in-sandbox timer itself is defeated by a wedged isolate. Optional: runtimes | ||
| 64 | * that do not bound execution leave it undefined and hosts skip the backstop. | ||
| 65 | */ | ||
| 66 | readonly timeoutMs?: number; | ||
| 59 | } | 67 | } |
| 60 | 68 | ||
| 61 | /** Accept-anything schema for tools with no input validation */ | 69 | /** Accept-anything schema for tools with no input validation */ |
| @@ -16,6 +16,7 @@ import * as Effect from "effect/Effect"; · 15 unmodified lines | |||
| 16 | import { | 16 | import { |
| 17 | CodeCompilationError, | 17 | CodeCompilationError, |
| 18 | recoverExecutionBody, | 18 | recoverExecutionBody, |
| 19 | SandboxHostTimeoutError, | ||
| 19 | SandboxRuntimeError, | 20 | SandboxRuntimeError, |
| 20 | stripTypeScript, | 21 | stripTypeScript, |
| 21 | type CodeExecutor, | 22 | type CodeExecutor, |
| @@ -44,6 +45,20 @@ export type DynamicWorkerExecutorOptions = { · 22 unmodified lines | |||
| 44 | * Timeout in milliseconds for code execution. Defaults to 5 minutes. | 45 | * Timeout in milliseconds for code execution. Defaults to 5 minutes. |
| 45 | */ | 46 | */ |
| 46 | readonly timeoutMs?: number; | 47 | readonly timeoutMs?: number; |
| 48 | /** | ||
| 49 | * Extra wall-clock the host waits beyond `timeoutMs` before declaring the | ||
| 50 | * isolate wedged and delivering a `SandboxHostTimeoutError`. Defaults to 30s. | ||
| 51 | * The in-sandbox timer should always fire first for a healthy isolate; this | ||
| 52 | * margin only covers the case where that timer is defeated by a wedged RPC. | ||
| 53 | * Exposed primarily so tests can shrink the backstop; production leaves it | ||
| 54 | * unset. | ||
| 55 | */ | ||
| 56 | readonly hostTimeoutGraceMs?: number; | ||
| 57 | /** | ||
| 58 | * Poll interval for the host-timeout watchdog, in milliseconds. Defaults to | ||
| 59 | * 1s. Exposed for tests; production leaves it unset. | ||
| 60 | */ | ||
| 61 | readonly hostTimeoutPollMs?: number; | ||
| 47 | /** | 62 | /** |
| 48 | * Controls outbound network access from sandboxed code. | 63 | * Controls outbound network access from sandboxed code. |
| 49 | * - `null` (default): `fetch()` and `connect()` throw — fully isolated. | 64 | * - `null` (default): `fetch()` and `connect()` throw — fully isolated. |
| @@ -87,6 +102,14 @@ type WorkerRpcResponse = WorkerRpcSuccess | WorkerRpcFailure; · 37 unmodified lines | |||
| 87 | // --------------------------------------------------------------------------- | 102 | // --------------------------------------------------------------------------- |
| 88 | 103 | ||
| 89 | const DEFAULT_TIMEOUT_MS = 5 * 60_000; | 104 | const DEFAULT_TIMEOUT_MS = 5 * 60_000; |
| 105 | /** | ||
| 106 | * Extra wall-clock the host waits beyond the effective in-sandbox timeout | ||
| 107 | * before declaring the isolate wedged. The in-sandbox timer should always fire | ||
| 108 | * first for a healthy isolate; this margin covers scheduling jitter and the RPC | ||
| 109 | * unwind so a normally-timing-out execution reports its own descriptive timeout | ||
| 110 | * rather than this backstop. | ||
| 111 | */ | ||
| 112 | const HOST_TIMEOUT_GRACE_MS = 30_000; | ||
| 90 | const ENTRY_MODULE = "executor.js"; | 113 | const ENTRY_MODULE = "executor.js"; |
| 91 | 114 | ||
| 92 | const normalizeErrorObject = (error: Error) => ({ | 115 | const normalizeErrorObject = (error: Error) => ({ |
| @@ -467,6 +490,14 @@ export type RunPromise = <A, E>(effect: Effect.Effect<A, E>) => Promise<A>; · 374 unmodified lines | |||
| 467 | export class ToolDispatcher extends RpcTarget { | 490 | export class ToolDispatcher extends RpcTarget { |
| 468 | readonly #invoker: SandboxToolInvoker; | 491 | readonly #invoker: SandboxToolInvoker; |
| 469 | readonly #runPromise: RunPromise; | 492 | readonly #runPromise: RunPromise; |
| 493 | // Number of tool round-trips currently in flight. The host-side execution | ||
| 494 | // timeout is suspended while this is > 0: whenever the sandbox is blocked | ||
| 495 | // here it is demonstrably not wedged (the host owns the continuation and | ||
| 496 | // will resume it), so neither a slow tool call nor a minutes-long approval | ||
| 497 | // pause (which manifests as an in-flight dispatch awaiting an elicitation | ||
| 498 | // response) should count against the unresponsive-sandbox backstop. | ||
| 499 | #inFlight = 0; | ||
| 500 | #lastReturnedAt = Date.now(); | ||
| 470 | 501 | ||
| 471 | constructor(invoker: SandboxToolInvoker, runPromise: RunPromise) { | 502 | constructor(invoker: SandboxToolInvoker, runPromise: RunPromise) { |
| 472 | super(); | 503 | super(); |
| @@ -474,7 +505,32 @@ export class ToolDispatcher extends RpcTarget { · 1 unmodified line | |||
| 474 | this.#runPromise = runPromise; | 505 | this.#runPromise = runPromise; |
| 475 | } | 506 | } |
| 476 | 507 | ||
| 508 | /** True while at least one tool round-trip is awaiting the host. */ | ||
| 509 | get isDispatching(): boolean { | ||
| 510 | return this.#inFlight > 0; | ||
| 511 | } | ||
| 512 | |||
| 513 | /** | ||
| 514 | * Epoch millis when the most recent tool round-trip returned control to the | ||
| 515 | * sandbox (or dispatcher construction time if none has). Combined with | ||
| 516 | * `isDispatching`, lets the host measure only the stretches where the sandbox | ||
| 517 | * is expected to be computing on its own. | ||
| 518 | */ | ||
| 519 | get lastReturnedAt(): number { | ||
| 520 | return this.#lastReturnedAt; | ||
| 521 | } | ||
| 522 | |||
| 477 | async call(path: string, args: unknown): Promise<WorkerRpcResponse> { | 523 | async call(path: string, args: unknown): Promise<WorkerRpcResponse> { |
| 524 | this.#inFlight += 1; | ||
| 525 | try { | ||
| 526 | return await this.#dispatch(path, args); | ||
| 527 | } finally { | ||
| 528 | this.#inFlight -= 1; | ||
| 529 | this.#lastReturnedAt = Date.now(); | ||
| 530 | } | ||
| 531 | } | ||
| 532 | |||
| 533 | #dispatch(path: string, args: unknown): Promise<WorkerRpcResponse> { | ||
| 478 | return this.#runPromise( | 534 | return this.#runPromise( |
| 479 | Effect.try({ | 535 | Effect.try({ |
| 480 | try: () => rehydrateBinary(args), | 536 | try: () => rehydrateBinary(args), |
| @@ -592,13 +648,101 @@ const startDynamicWorker = ( · 111 unmodified lines | |||
| 592 | }), | 648 | }), |
| 593 | ); | 649 | ); |
| 594 | 650 | ||
| 651 | // --------------------------------------------------------------------------- | ||
| 652 | // Host-side execution timeout (unresponsive-sandbox backstop) | ||
| 653 | // --------------------------------------------------------------------------- | ||
| 654 | |||
| 655 | /** The subset of `ToolDispatcher` state the host watchdog observes. */ | ||
| 656 | export type DispatcherActivity = { | ||
| 657 | readonly isDispatching: boolean; | ||
| 658 | readonly lastReturnedAt: number; | ||
| 659 | }; | ||
| 660 | |||
| 661 | export type HostTimeoutOptions = { | ||
| 662 | /** Effective in-sandbox timeout bound the sandbox itself enforces. */ | ||
| 663 | readonly timeoutMs: number; | ||
| 664 | /** Extra wall-clock beyond the bound before declaring the isolate wedged. */ | ||
| 665 | readonly graceMs: number; | ||
| 666 | /** Dispatcher activity — the host clock is suspended while it is dispatching. */ | ||
| 667 | readonly dispatcher: DispatcherActivity; | ||
| 668 | /** Poll interval for the watchdog. Defaults to 1s. */ | ||
| 669 | readonly pollMs?: number; | ||
| 670 | /** Emitted once when the backstop fires. */ | ||
| 671 | readonly onTimeout?: (info: { readonly elapsedMs: number }) => void; | ||
| 672 | /** Clock injection for tests. Defaults to `Date.now`. */ | ||
| 673 | readonly now?: () => number; | ||
| 674 | }; | ||
| 675 | |||
| 676 | /** | ||
| 677 | * Race a (typed) evaluate Effect against a host-side watchdog that fires only | ||
| 678 | * when the sandbox has gone unresponsive: no result and no host round-trip for | ||
| 679 | * `timeoutMs + graceMs` of continuous compute time. The watchdog's reference | ||
| 680 | * point advances every time a tool round-trip returns and is held at `now` | ||
| 681 | * while a round-trip is in flight (a slow tool call or an approval pause), so | ||
| 682 | * those never trip it. A true wedge (isolate evicted / OOM, RPC hung without | ||
| 683 | * rejecting, no further dispatch) leaves the reference point stationary and the | ||
| 684 | * watchdog fires, failing with a `SandboxHostTimeoutError`. | ||
| 685 | * | ||
| 686 | * Takes the evaluate step as a pre-built Effect (not a raw promise) so the | ||
| 687 | * caller keeps its own error classification, and so a test can drive it with an | ||
| 688 | * Effect wrapping a never-settling promise (no live WorkerLoader needed). | ||
| 689 | */ | ||
| 690 | export const runEvaluateWithHostTimeout = <A, E>( | ||
| 691 | evaluated: Effect.Effect<A, E>, | ||
| 692 | options: HostTimeoutOptions, | ||
| 693 | ): Effect.Effect<A, E | SandboxHostTimeoutError> => | ||
| 694 | Effect.gen(function* () { | ||
| 695 | const now = options.now ?? Date.now; | ||
| 696 | const bound = Math.max(0, options.timeoutMs) + Math.max(0, options.graceMs); | ||
| 697 | const pollMs = Math.max(1, options.pollMs ?? 1_000); | ||
| 698 | const start = now(); | ||
| 699 | |||
| 700 | const completion = evaluated.pipe( | ||
| 701 | Effect.map((value): TimeoutRace<A> => ({ kind: "done", value })), | ||
| 702 | ); | ||
| 703 | |||
| 704 | // Poll the dispatcher's activity. `reference` is the latest instant from | ||
| 705 | // which the sandbox is expected to be computing on its own: execution | ||
| 706 | // start, bumped forward to each round-trip's return, and pinned to `now` | ||
| 707 | // for as long as a round-trip is outstanding. The watchdog fires when the | ||
| 708 | // sandbox has been on its own for `bound` ms without producing a result. | ||
| 709 | const watchdog: Effect.Effect<TimeoutRace<A>> = Effect.gen(function* () { | ||
| 710 | for (;;) { | ||
| 711 | yield* Effect.sleep(pollMs); | ||
| 712 | const current = now(); | ||
| 713 | const reference = options.dispatcher.isDispatching | ||
| 714 | ? current | ||
| 715 | : Math.max(start, options.dispatcher.lastReturnedAt); | ||
| 716 | if (current - reference >= bound) { | ||
| 717 | return { kind: "timeout", elapsedMs: current - start }; | ||
| 718 | } | ||
| 719 | } | ||
| 720 | }); | ||
| 721 | |||
| 722 | const outcome = yield* Effect.raceFirst(completion, watchdog); | ||
| 723 | if (outcome.kind === "timeout") { | ||
| 724 | options.onTimeout?.({ elapsedMs: outcome.elapsedMs }); | ||
| 725 | return yield* new SandboxHostTimeoutError({ | ||
| 726 | runtime: "dynamic-worker", | ||
| 727 | message: `execution exceeded ${bound}ms (sandbox unresponsive)`, | ||
| 728 | timeoutMs: bound, | ||
| 729 | elapsedMs: outcome.elapsedMs, | ||
| 730 | }); | ||
| 731 | } | ||
| 732 | return outcome.value; | ||
| 733 | }); | ||
| 734 | |||
| 735 | type TimeoutRace<A> = | ||
| 736 | | { readonly kind: "done"; readonly value: A } | ||
| 737 | | { readonly kind: "timeout"; readonly elapsedMs: number }; | ||
| 738 | |||
| 595 | const evaluate = ( | 739 | const evaluate = ( |
| 596 | options: DynamicWorkerExecutorOptions, | 740 | options: DynamicWorkerExecutorOptions, |
| 597 | code: string, | 741 | code: string, |
| 598 | toolInvoker: SandboxToolInvoker, | 742 | toolInvoker: SandboxToolInvoker, |
| 599 | ): Effect.Effect< | 743 | ): Effect.Effect< |
| 600 | ExecuteResult, | 744 | ExecuteResult, |
| 601 | DynamicWorkerExecutionError | CodeCompilationError | SandboxRuntimeError | 745 | DynamicWorkerExecutionError | CodeCompilationError | SandboxRuntimeError | SandboxHostTimeoutError |
| 602 | > => { | 746 | > => { |
| 603 | const timeoutMs = Math.max(100, options.timeoutMs ?? DEFAULT_TIMEOUT_MS); | 747 | const timeoutMs = Math.max(100, options.timeoutMs ?? DEFAULT_TIMEOUT_MS); |
| 604 | 748 | ||
| @@ -606,13 +750,32 @@ const evaluate = ( · 1 unmodified line | |||
| 606 | const context = yield* Effect.context<never>(); | 750 | const context = yield* Effect.context<never>(); |
| 607 | const dispatcher = new ToolDispatcher(toolInvoker, Effect.runPromiseWith(context)); | 751 | const dispatcher = new ToolDispatcher(toolInvoker, Effect.runPromiseWith(context)); |
| 608 | const entrypoint = yield* startDynamicWorker(options, code, timeoutMs); | 752 | const entrypoint = yield* startDynamicWorker(options, code, timeoutMs); |
| 609 | const response = yield* Effect.tryPromise({ | 753 | // The evaluate RPC rejects for module compile failures that escaped the |
| 754 | // strip step, non-serializable return values, isolate resource limits, and | ||
| 755 | // capacity. All are the user's concern or transient, so classify and | ||
| 756 | // surface them descriptively rather than opaquely. | ||
| 757 | const evaluated = Effect.tryPromise({ | ||
| 610 | try: () => entrypoint.evaluate(dispatcher), | 758 | try: () => entrypoint.evaluate(dispatcher), |
| 611 | // The evaluate RPC rejects for module compile failures that escaped | ||
| 612 | // the strip step, non-serializable return values, isolate resource | ||
| 613 | // limits, and capacity. All are the user's concern or transient, so | ||
| 614 | // classify and surface them descriptively rather than opaquely. | ||
| 615 | catch: toSandboxFailure, | 759 | catch: toSandboxFailure, |
| 760 | }); | ||
| 761 | const graceMs = Math.max(0, options.hostTimeoutGraceMs ?? HOST_TIMEOUT_GRACE_MS); | ||
| 762 | const response = yield* runEvaluateWithHostTimeout(evaluated, { | ||
| 763 | timeoutMs, | ||
| 764 | graceMs, | ||
| 765 | pollMs: options.hostTimeoutPollMs, | ||
| 766 | dispatcher, | ||
| 767 | onTimeout: ({ elapsedMs }) => { | ||
| 768 | // Structured, greppable event for the alerting workstream. Kept off the | ||
| 769 | // trace span (which is interrupted by the race) so it always lands. | ||
| 770 | // oxlint-disable-next-line no-console -- boundary: structured host-timeout telemetry event | ||
| 771 | console.error( | ||
| 772 | JSON.stringify({ | ||
| 773 | event: "mcp_execution_host_timeout", | ||
| 774 | elapsedMs, | ||
| 775 | timeoutMs: timeoutMs + graceMs, | ||
| 776 | }), | ||
| 777 | ); | ||
| 778 | }, | ||
| 616 | }).pipe( | 779 | }).pipe( |
| 617 | Effect.withSpan("executor.runtime.evaluate", { | 780 | Effect.withSpan("executor.runtime.evaluate", { |
| 618 | attributes: { "executor.runtime": "dynamic-worker" }, | 781 | attributes: { "executor.runtime": "dynamic-worker" }, |
| @@ -652,6 +815,13 @@ const runInDynamicWorker = ( · 33 unmodified lines | |||
| 652 | Effect.succeed({ result: null, error: error.message } satisfies ExecuteResult), | 815 | Effect.succeed({ result: null, error: error.message } satisfies ExecuteResult), |
| 653 | SandboxRuntimeError: (error) => | 816 | SandboxRuntimeError: (error) => |
| 654 | Effect.succeed({ result: null, error: error.message } satisfies ExecuteResult), | 817 | Effect.succeed({ result: null, error: error.message } satisfies ExecuteResult), |
| 818 | // A wedged isolate that the in-sandbox timer failed to catch is | ||
| 819 | // unrecoverable, but reporting it as a delivered, descriptive error beats | ||
| 820 | // the original symptom (open-ended silence). Fold it into the success | ||
| 821 | // channel like the other safe-to-report conditions so it reaches the | ||
| 822 | // model instead of collapsing to an opaque internal error. | ||
| 823 | SandboxHostTimeoutError: (error) => | ||
| 824 | Effect.succeed({ result: null, error: error.message } satisfies ExecuteResult), | ||
| 655 | }), | 825 | }), |
| 656 | Effect.withSpan("executor.code.exec.dynamic_worker", { | 826 | Effect.withSpan("executor.code.exec.dynamic_worker", { |
| 657 | attributes: { "executor.runtime": "dynamic-worker" }, | 827 | attributes: { "executor.runtime": "dynamic-worker" }, |
| @@ -667,4 +837,7 @@ export const makeDynamicWorkerExecutor = ( · 9 unmodified lines | |||
| 667 | ): CodeExecutor<DynamicWorkerExecutionError> => ({ | 837 | ): CodeExecutor<DynamicWorkerExecutionError> => ({ |
| 668 | execute: (code: string, toolInvoker: SandboxToolInvoker) => | 838 | execute: (code: string, toolInvoker: SandboxToolInvoker) => |
| 669 | runInDynamicWorker(options, code, toolInvoker), | 839 | runInDynamicWorker(options, code, toolInvoker), |
| 840 | // The effective in-sandbox bound, exposed so hosts can reason about the | ||
| 841 | // execution budget. `evaluate` clamps to a 100ms floor identically. | ||
| 842 | timeoutMs: Math.max(100, options.timeoutMs ?? DEFAULT_TIMEOUT_MS), | ||
| 670 | }); | 843 | }); |
| @@ -0,0 +1,195 @@ | |||
| 1 | // --------------------------------------------------------------------------- | ||
| 2 | // Host-side execution timeout — the unresponsive-sandbox backstop. | ||
| 3 | // | ||
| 4 | // The ONLY timeout on code execution used to be the in-SANDBOX Promise.race in | ||
| 5 | // module-template.ts. If the isolate is evicted / OOM-killed in a way that | ||
| 6 | // wedges the cross-isolate `evaluate` RPC WITHOUT rejecting, that timer never | ||
| 7 | // fires and the host side awaited the RPC forever: the client saw pure silence | ||
| 8 | // (SSE keepalives, no JSON-RPC frame) for the DO's whole running-work lease. | ||
| 9 | // | ||
| 10 | // These tests drive that exact shape through the PUBLIC executor path with a | ||
| 11 | // fake WorkerLoader whose `evaluate()` never settles. On the unpatched code the | ||
| 12 | // "delivers a typed timeout error" test HANGS and only fails via the bounded | ||
| 13 | // vitest testTimeout (red-by-timeout repro); with the host backstop it delivers | ||
| 14 | // a descriptive `ExecuteResult.error` well inside the bound. The pause-survival | ||
| 15 | // test guards the correctness risk of the change: an execution blocked in a | ||
| 16 | // host round-trip (a slow tool call or a minutes-long approval pause) must NOT | ||
| 17 | // be killed by the backstop. | ||
| 18 | // --------------------------------------------------------------------------- | ||
| 19 | |||
| 20 | import { describe, expect, it } from "@effect/vitest"; | ||
| 21 | import * as Effect from "effect/Effect"; | ||
| 22 | import * as Exit from "effect/Exit"; | ||
| 23 | import * as Fiber from "effect/Fiber"; | ||
| 24 | import * as Data from "effect/Data"; | ||
| 25 | import type { SandboxToolInvoker } from "@executor-js/codemode-core"; | ||
| 26 | |||
| 27 | import { | ||
| 28 | makeDynamicWorkerExecutor, | ||
| 29 | runEvaluateWithHostTimeout, | ||
| 30 | type DispatcherActivity, | ||
| 31 | } from "./executor"; | ||
| 32 | |||
| 33 | // A never-invoked tool invoker — the wedge cases never reach a tool call. | ||
| 34 | const idleInvoker: SandboxToolInvoker = { | ||
| 35 | invoke: () => Effect.succeed(undefined), | ||
| 36 | }; | ||
| 37 | |||
| 38 | // Build a fake WorkerLoader whose loaded worker's `evaluate()` resolves with | ||
| 39 | // `settle()` (or never, if `settle` is never called). Enough surface for | ||
| 40 | // `startDynamicWorker` -> `worker.getEntrypoint().evaluate(dispatcher)`. | ||
| 41 | const makeFakeLoader = (evaluate: () => Promise<unknown>) => { | ||
| 42 | const entrypoint = { evaluate }; | ||
| 43 | const stub = { getEntrypoint: () => entrypoint }; | ||
| 44 | // oxlint-disable-next-line executor/no-double-cast -- test double for the Cloudflare WorkerLoader binding; only get()/getEntrypoint()/evaluate() are exercised | ||
| 45 | return { get: () => stub, load: () => stub } as unknown as WorkerLoader; | ||
| 46 | }; | ||
| 47 | |||
| 48 | // A dispatcher-activity double the watchdog observes directly. | ||
| 49 | const activity = (init: Partial<DispatcherActivity> = {}): DispatcherActivity => ({ | ||
| 50 | isDispatching: false, | ||
| 51 | lastReturnedAt: 0, | ||
| 52 | ...init, | ||
| 53 | }); | ||
| 54 | |||
| 55 | class NeverError extends Data.TaggedError("NeverError")<{ readonly message: string }> {} | ||
| 56 | |||
| 57 | // A promise that never settles — the wedged RPC. | ||
| 58 | const never = <A>(): Promise<A> => new Promise<A>(() => {}); | ||
| 59 | |||
| 60 | describe("host execution timeout (public execute path)", () => { | ||
| 61 | it("delivers a descriptive timeout error when the evaluate RPC never settles", async () => { | ||
| 62 | // The wedge: evaluate() never resolves and never rejects. | ||
| 63 | const executor = makeDynamicWorkerExecutor({ | ||
| 64 | loader: makeFakeLoader(() => never()), | ||
| 65 | timeoutMs: 200, // small in-sandbox bound so the backstop (bound + grace) is quick | ||
| 66 | hostTimeoutGraceMs: 100, | ||
| 67 | hostTimeoutPollMs: 20, | ||
| 68 | }); | ||
| 69 | |||
| 70 | const started = Date.now(); | ||
| 71 | // On UNPATCHED code this Effect never completes and the test fails only via | ||
| 72 | // the bounded vitest testTimeout. With the backstop it resolves with a | ||
| 73 | // delivered, descriptive error. | ||
| 74 | const result = await Effect.runPromise(executor.execute("return 1;", idleInvoker)); | ||
| 75 | const elapsed = Date.now() - started; | ||
| 76 | |||
| 77 | expect(result.result).toBeNull(); | ||
| 78 | expect(result.error).toBeDefined(); | ||
| 79 | expect(result.error).toContain("unresponsive"); | ||
| 80 | // bound = 200 (timeout) + 100 (grace). Must have fired, not hung forever. | ||
| 81 | expect(elapsed).toBeGreaterThanOrEqual(300); | ||
| 82 | expect(elapsed).toBeLessThan(10_000); | ||
| 83 | }); | ||
| 84 | |||
| 85 | it("passes a fast result straight through without waiting on the backstop", async () => { | ||
| 86 | const executor = makeDynamicWorkerExecutor({ | ||
| 87 | loader: makeFakeLoader(() => Promise.resolve({ result: 42, logs: [] })), | ||
| 88 | timeoutMs: 200, | ||
| 89 | }); | ||
| 90 | |||
| 91 | const started = Date.now(); | ||
| 92 | const result = await Effect.runPromise(executor.execute("return 42;", idleInvoker)); | ||
| 93 | const elapsed = Date.now() - started; | ||
| 94 | |||
| 95 | expect(result.result).toBe(42); | ||
| 96 | expect(result.error).toBeUndefined(); | ||
| 97 | expect(elapsed).toBeLessThan(1_000); | ||
| 98 | }); | ||
| 99 | }); | ||
| 100 | |||
| 101 | describe("runEvaluateWithHostTimeout", () => { | ||
| 102 | it("fires when the sandbox is on its own and produces nothing", async () => { | ||
| 103 | let fired = false; | ||
| 104 | const effect = runEvaluateWithHostTimeout( | ||
| 105 | Effect.tryPromise({ | ||
| 106 | try: () => never<string>(), | ||
| 107 | catch: (c) => new NeverError({ message: String(c) }), | ||
| 108 | }), | ||
| 109 | { | ||
| 110 | timeoutMs: 50, | ||
| 111 | graceMs: 50, | ||
| 112 | pollMs: 10, | ||
| 113 | dispatcher: activity(), | ||
| 114 | onTimeout: () => { | ||
| 115 | fired = true; | ||
| 116 | }, | ||
| 117 | }, | ||
| 118 | ); | ||
| 119 | |||
| 120 | const exit = await Effect.runPromiseExit(effect); | ||
| 121 | expect(Exit.isFailure(exit)).toBe(true); | ||
| 122 | expect(fired).toBe(true); | ||
| 123 | }); | ||
| 124 | |||
| 125 | it("does NOT fire while a tool round-trip / approval pause is outstanding", async () => { | ||
| 126 | // The dispatcher reports it is dispatching for the entire window: the | ||
| 127 | // sandbox is blocked in the host (a slow tool, or an approval pause), so | ||
| 128 | // the backstop must hold its clock and never fire. The evaluate settles | ||
| 129 | // only AFTER a span far exceeding timeoutMs+graceMs. | ||
| 130 | const bound = 40; // timeoutMs 20 + graceMs 20 | ||
| 131 | let resolveEvaluate!: (value: { result: number }) => void; | ||
| 132 | const pending = new Promise<{ result: number }>((resolve) => { | ||
| 133 | resolveEvaluate = resolve; | ||
| 134 | }); | ||
| 135 | |||
| 136 | let fired = false; | ||
| 137 | const dispatcher = activity({ isDispatching: true, lastReturnedAt: Date.now() }); | ||
| 138 | const effect = runEvaluateWithHostTimeout( | ||
| 139 | Effect.tryPromise({ | ||
| 140 | try: () => pending, | ||
| 141 | catch: (c) => new NeverError({ message: String(c) }), | ||
| 142 | }), | ||
| 143 | { | ||
| 144 | timeoutMs: 20, | ||
| 145 | graceMs: 20, | ||
| 146 | pollMs: 5, | ||
| 147 | dispatcher, | ||
| 148 | onTimeout: () => { | ||
| 149 | fired = true; | ||
| 150 | }, | ||
| 151 | }, | ||
| 152 | ); | ||
| 153 | |||
| 154 | const fiber = Effect.runFork(effect); | ||
| 155 | |||
| 156 | // Wait well past the bound while "paused" (dispatching). The watchdog must | ||
| 157 | // hold: no timeout should have fired. | ||
| 158 | await new Promise((r) => setTimeout(r, bound * 6)); | ||
| 159 | expect(fired).toBe(false); | ||
| 160 | |||
| 161 | // The pause ends: the tool/approval returns, evaluate settles. Result must | ||
| 162 | // be delivered intact. | ||
| 163 | resolveEvaluate({ result: 7 }); | ||
| 164 | const value = await Effect.runPromise(Fiber.join(fiber)); | ||
| 165 | expect(value).toEqual({ result: 7 }); | ||
| 166 | expect(fired).toBe(false); | ||
| 167 | }); | ||
| 168 | |||
| 169 | it("fires after a dispatch returns and the sandbox then goes quiet", async () => { | ||
| 170 | // A round-trip returned at t0 (advancing lastReturnedAt), then the sandbox | ||
| 171 | // wedged with no further dispatch. The reference point is lastReturnedAt, | ||
| 172 | // so the backstop still fires bound ms after that return. | ||
| 173 | let fired = false; | ||
| 174 | const dispatcher = activity({ isDispatching: false, lastReturnedAt: Date.now() }); | ||
| 175 | const effect = runEvaluateWithHostTimeout( | ||
| 176 | Effect.tryPromise({ | ||
| 177 | try: () => never<string>(), | ||
| 178 | catch: (c) => new NeverError({ message: String(c) }), | ||
| 179 | }), | ||
| 180 | { | ||
| 181 | timeoutMs: 30, | ||
| 182 | graceMs: 30, | ||
| 183 | pollMs: 10, | ||
| 184 | dispatcher, | ||
| 185 | onTimeout: () => { | ||
| 186 | fired = true; | ||
| 187 | }, | ||
| 188 | }, | ||
| 189 | ); | ||
| 190 | |||
| 191 | const exit = await Effect.runPromiseExit(effect); | ||
| 192 | expect(Exit.isFailure(exit)).toBe(true); | ||
| 193 | expect(fired).toBe(true); | ||
| 194 | }); | ||
| 195 | }); | ||
| @@ -2,9 +2,12 @@ export type { CodeExecutor, ExecuteResult, SandboxToolInvoker } from "@executor- · 1 unmodified line | |||
| 2 | 2 | ||
| 3 | export { | 3 | export { |
| 4 | makeDynamicWorkerExecutor, | 4 | makeDynamicWorkerExecutor, |
| 5 | runEvaluateWithHostTimeout, | ||
| 5 | ToolDispatcher, | 6 | ToolDispatcher, |
| 6 | DynamicWorkerExecutionError, | 7 | DynamicWorkerExecutionError, |
| 7 | type DynamicWorkerExecutorOptions, | 8 | type DynamicWorkerExecutorOptions, |
| 9 | type DispatcherActivity, | ||
| 10 | type HostTimeoutOptions, | ||
| 8 | } from "./executor"; | 11 | } from "./executor"; |
| 9 | 12 | ||
| 10 | export { buildExecutorModule } from "./module-template"; | 13 | export { buildExecutorModule } from "./module-template"; |
| @@ -0,0 +1,58 @@ | |||
| 1 | // --------------------------------------------------------------------------- | ||
| 2 | // RED-ON-MAIN REPRO of the wedged-evaluate incident. | ||
| 3 | // | ||
| 4 | // A wedged `evaluate()` RPC (resolves never, rejects never) used to leave the | ||
| 5 | // host awaiting forever: `execute()` never returned, so the MCP client saw pure | ||
| 6 | // silence (SSE keepalives, no JSON-RPC frame) for the DO's whole running-work | ||
| 7 | // lease. This test drives that exact shape through the PUBLIC executor path | ||
| 8 | // with a fake WorkerLoader whose loaded worker's `evaluate` never settles. | ||
| 9 | // | ||
| 10 | // On UNPATCHED main this test HANGS and fails only via the bounded per-test | ||
| 11 | // timeout below (verified: "Test timed out in 8000ms" on c46730b5). With the | ||
| 12 | // host-timeout backstop it resolves with a delivered, descriptive error inside | ||
| 13 | // the bound. The extra host-timeout knobs are passed through a widened options | ||
| 14 | // value so this file still compiles against a checkout that predates them (they | ||
| 15 | // are simply ignored there, and the test hangs → red). | ||
| 16 | // --------------------------------------------------------------------------- | ||
| 17 | |||
| 18 | import { describe, expect, it } from "@effect/vitest"; | ||
| 19 | import * as Effect from "effect/Effect"; | ||
| 20 | import type { SandboxToolInvoker } from "@executor-js/codemode-core"; | ||
| 21 | |||
| 22 | import { makeDynamicWorkerExecutor, type DynamicWorkerExecutorOptions } from "./executor"; | ||
| 23 | |||
| 24 | const idleInvoker: SandboxToolInvoker = { invoke: () => Effect.succeed(undefined) }; | ||
| 25 | |||
| 26 | const makeFakeLoader = (evaluate: () => Promise<unknown>) => { | ||
| 27 | const entrypoint = { evaluate }; | ||
| 28 | const stub = { getEntrypoint: () => entrypoint }; | ||
| 29 | // oxlint-disable-next-line executor/no-double-cast -- test double for the Cloudflare WorkerLoader binding; only get()/getEntrypoint()/evaluate() are exercised | ||
| 30 | return { get: () => stub, load: () => stub } as unknown as WorkerLoader; | ||
| 31 | }; | ||
| 32 | |||
| 33 | const never = <A>(): Promise<A> => new Promise<A>(() => {}); | ||
| 34 | |||
| 35 | describe("wedged evaluate RPC (public execute path)", () => { | ||
| 36 | it( | ||
| 37 | "delivers a descriptive timeout error instead of hanging forever", | ||
| 38 | { timeout: 8_000 }, | ||
| 39 | async () => { | ||
| 40 | // Widened so the host-timeout knobs (absent on pre-fix checkouts) don't | ||
| 41 | // trip excess-property checks; they shrink the backstop on the fix and | ||
| 42 | // are ignored on main. | ||
| 43 | const options = { | ||
| 44 | loader: makeFakeLoader(() => never()), | ||
| 45 | timeoutMs: 200, | ||
| 46 | hostTimeoutGraceMs: 100, | ||
| 47 | hostTimeoutPollMs: 20, | ||
| 48 | } as DynamicWorkerExecutorOptions & Record<string, unknown>; | ||
| 49 | |||
| 50 | const executor = makeDynamicWorkerExecutor(options); | ||
| 51 | const result = await Effect.runPromise(executor.execute("return 1;", idleInvoker)); | ||
| 52 | |||
| 53 | expect(result.result).toBeNull(); | ||
| 54 | expect(result.error).toBeDefined(); | ||
| 55 | expect(result.error).toContain("unresponsive"); | ||
| 56 | }, | ||
| 57 | ); | ||
| 58 | }); | ||