From bf023b273dd453febf6697362864d7a9b0d80ece Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 2 Nov 2026 22:37:36 -0800 Subject: [PATCH] fix(sandbox): keep a workbench timeout report when its stop settles after the call deadline A workbench run stopped by its own timeout reports its output and files, but both run paths checked the call signal before the report. When the provider's stop round trip outlasted the report reserve, the call deadline had already passed and the run returned a bare timeout with its output dropped. - The cancellation check moves into throwIfRunTimedOut: a passed deadline keeps the report, a cancellation at any point stays a cancellation - One-shot sandboxes and runs that did not time out keep their checks --- .../sim/lib/execution/remote-sandbox/index.ts | 26 +++++++---- .../remote-sandbox/session-sandbox.test.ts | 46 +++++++++++++++++++ 2 files changed, 63 insertions(+), 9 deletions(-) diff --git a/apps/sim/lib/execution/remote-sandbox/index.ts b/apps/sim/lib/execution/remote-sandbox/index.ts index 3b9c35f5d3d..32d378621f2 100644 --- a/apps/sim/lib/execution/remote-sandbox/index.ts +++ b/apps/sim/lib/execution/remote-sandbox/index.ts @@ -308,10 +308,17 @@ function throwIfSandboxTimedOut(result: { timedOut?: boolean }): void { /** Bounds the output-directory listing a timeout report adds; a slow listing reports no files. */ const TIMED_OUT_OUTPUT_LISTING_MS = 3_000 +/** Throws when the call was cancelled; a passed deadline is not a cancellation. */ +function throwIfCancelled(signal: AbortSignal): void { + if (!isTimeoutAbortReason(signal.reason)) throwIfAborted(signal) +} + /** - * A workbench run stopped by its own timeout reports what it produced, because the workbench and - * its files outlive the call and the next call can resume from them. A one-shot sandbox is torn - * down with everything in it, so it keeps the plain timeout. + * Settles a finished run against the call's signal. A workbench run stopped by its own timeout + * reports what it produced, because the workbench and its files outlive the call and the next + * call can resume from them; the call's deadline passing while that stop settles keeps the + * report, and a cancellation at any point stays a cancellation. A one-shot sandbox is torn down + * with everything in it, so it keeps the plain timeout. */ async function throwIfRunTimedOut( result: { timedOut?: boolean }, @@ -324,8 +331,12 @@ async function throwIfRunTimedOut( signal: AbortSignal } ): Promise { - if (!result.timedOut) return - if (!run.lease.session) throw new DOMException('timeout', 'AbortError') + if (!result.timedOut || !run.lease.session) { + throwIfAborted(run.signal) + throwIfSandboxTimedOut(result) + return + } + throwIfCancelled(run.signal) const { outputSandboxDir, sandbox } = run // Half of what the call has left, so the report lands before the call's deadline. const listingMs = Math.min( @@ -339,8 +350,7 @@ async function throwIfRunTimedOut( run.signal ).catch(() => []) : [] - // A cancellation that arrives while listing stays a cancellation. - throwIfAborted(run.signal) + throwIfCancelled(run.signal) throw new SandboxExecutionTimeoutError(sandboxTimeoutProgress(streams, run.timeoutMs, listed)) } @@ -1092,7 +1102,6 @@ async function executeInSandboxWithinBudget( throwIfAborted(signal) throw error } - throwIfAborted(signal) await throwIfRunTimedOut(execution, [execution.text, execution.stdout, execution.stderr], { sandbox, lease, @@ -1316,7 +1325,6 @@ async function executeShellInSandboxWithinBudget( throwIfAborted(signal) throw error } - throwIfAborted(signal) await throwIfRunTimedOut(result, [result.stdout, result.stderr], { sandbox, lease, diff --git a/apps/sim/lib/execution/remote-sandbox/session-sandbox.test.ts b/apps/sim/lib/execution/remote-sandbox/session-sandbox.test.ts index 0cb4b375993..ae079b41f6d 100644 --- a/apps/sim/lib/execution/remote-sandbox/session-sandbox.test.ts +++ b/apps/sim/lib/execution/remote-sandbox/session-sandbox.test.ts @@ -1167,6 +1167,52 @@ describe('session sandbox lease', () => { } }) + it('reports a short run whose stop settles after the call deadline', async () => { + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout', 'Date'] }) + try { + const { handle } = fakeSandbox('timeout-slow-stop') + mockFindSessionSandbox.mockResolvedValue(handle) + handle.listFiles = async () => [] + // The provider's own timeout stops the command, and that stop settles after the call's + // deadline; an abort before the stop interrupts the command. + handle.runCommand = (_command, options) => + new Promise((resolve, reject) => { + let stopping = false + const stop = () => { + stopping = true + setTimeout( + () => resolve({ stdout: 'step 1', stderr: '', exitCode: 124, timedOut: true }), + 3_000 + ) + } + setTimeout(() => setTimeout(stop, options.timeoutMs), 300) + options.signal?.addEventListener( + 'abort', + () => { + if (!stopping) reject(options.signal?.reason) + }, + { once: true } + ) + }) + const pending = executeShellInSandbox({ + ...CODE_REQUEST, + timeoutMs: 10_000, + envs: {}, + session: { key: 'timeout-slow-stop' }, + outputSandboxDir: OUTPUT_DIR, + }).then( + () => undefined, + (rejection: unknown) => rejection + ) + await vi.runAllTimersAsync() + const error = await pending + if (!(error instanceof SandboxExecutionTimeoutError)) throw error + expect(error.progress.output).toBe('step 1') + } finally { + vi.useRealTimers() + } + }) + it('keeps a cancellation that arrives while listing the outputs a cancellation', async () => { const { handle } = fakeSandbox('timeout-cancelled') mockFindSessionSandbox.mockResolvedValue(handle)