diff --git a/packages/runtime/src/sandbox/body-runner.ts b/packages/runtime/src/sandbox/body-runner.ts index 82923bd873..c0cf89f483 100644 --- a/packages/runtime/src/sandbox/body-runner.ts +++ b/packages/runtime/src/sandbox/body-runner.ts @@ -71,7 +71,11 @@ export function hookBodyRunnerFactory( name: hook.name, object: typeof (hook as any).object === 'string' ? (hook as any).object : undefined, }, - timeoutMs: (body as any).timeoutMs ?? 250, + // When the body declares no timeout, leave opts.timeoutMs unset so the + // runner's own configurable hook default applies (QuickJSScriptRunner + // defaults to 250ms). Hardcoding the fallback here would make that + // constructor option dead for hooks. + timeoutMs: (body as any).timeoutMs, }); applyMutationsToInput(engineCtx, result); } catch (err: any) { @@ -123,7 +127,9 @@ export function actionBodyRunnerFactory( }); const result = await runner.run(body, sandboxCtx, { origin: { kind: 'action', name: action.name, object: action.object }, - timeoutMs: (body as any).timeoutMs ?? action.timeoutMs ?? 5000, + // As with hooks above: no declared timeout → let the runner's + // configurable action default (5000ms) apply. + timeoutMs: (body as any).timeoutMs ?? action.timeoutMs, }); return result.value; } catch (err: any) { diff --git a/packages/runtime/src/sandbox/nested-write-real-sqlite.integration.test.ts b/packages/runtime/src/sandbox/nested-write-real-sqlite.integration.test.ts index adb49c55e6..e31897f27a 100644 --- a/packages/runtime/src/sandbox/nested-write-real-sqlite.integration.test.ts +++ b/packages/runtime/src/sandbox/nested-write-real-sqlite.integration.test.ts @@ -86,7 +86,9 @@ describe('#1867 nested cross-object write — REAL SqlDriver (better-sqlite3, on engine.registerDriver(driver, true); await engine.init(); for (const o of [EXPENSE_REPORT, EXPENSE_LINE]) engine.registry.registerObject(o as any); - engine.setDefaultBodyRunner(hookBodyRunnerFactory(new QuickJSScriptRunner(), { ql: engine, appId: 'expense' })); + // Generous hook budget — the subject is the nested-write path on a real + // driver, not the 250ms default (see nested-write.integration.test.ts). + engine.setDefaultBodyRunner(hookBodyRunnerFactory(new QuickJSScriptRunner({ hookTimeoutMs: 10_000 }), { ql: engine, appId: 'expense' })); bindHooksToEngine(engine, [ROLLUP_HOOK as any], { packageId: 'expense' }); return engine; } diff --git a/packages/runtime/src/sandbox/nested-write.integration.test.ts b/packages/runtime/src/sandbox/nested-write.integration.test.ts index 53561aa077..57c143aa1d 100644 --- a/packages/runtime/src/sandbox/nested-write.integration.test.ts +++ b/packages/runtime/src/sandbox/nested-write.integration.test.ts @@ -90,8 +90,13 @@ describe('#1867 nested cross-object write from a hook (real engine + sandbox)', engine.registerDriver(driver, true); await engine.init(); for (const o of [parent, child]) engine.registry.registerObject(o as any); + // Generous hook budget: the subject here is nested-write correctness, not + // the 250ms default. Each hook invocation compiles a fresh WASM module and + // the nested parent hook compiles another inside the child's budget — on a + // loaded CI machine that overhead alone blew 250ms ("hook + // 'rollup_parent_total' exceeded timeout of 250ms"). engine.setDefaultBodyRunner( - hookBodyRunnerFactory(new QuickJSScriptRunner(), { ql: engine, appId: 'test' }), + hookBodyRunnerFactory(new QuickJSScriptRunner({ hookTimeoutMs: 10_000 }), { ql: engine, appId: 'test' }), ); }); diff --git a/packages/runtime/src/sandbox/quickjs-runner.test.ts b/packages/runtime/src/sandbox/quickjs-runner.test.ts index 7e0b25786e..a108fa42c4 100644 --- a/packages/runtime/src/sandbox/quickjs-runner.test.ts +++ b/packages/runtime/src/sandbox/quickjs-runner.test.ts @@ -4,7 +4,13 @@ import { describe, it, expect } from 'vitest'; import { QuickJSScriptRunner, SandboxError } from './quickjs-runner.js'; import type { ScriptContext, ScriptRunOptions } from './script-runner.js'; -const runner = new QuickJSScriptRunner(); +// Generous hook budget: these tests exercise sandbox *behaviour*, not the stock +// 250ms hook budget. Every invocation compiles a fresh WASM module, and nested +// hooks compile another one inside the parent's budget — on a loaded CI machine +// that fixed cost alone can blow 250ms and flake (e.g. "hook 'lvl4' exceeded +// timeout of 250ms"). Tests that ARE about the default budget use +// `defaultRunner` below. +const runner = new QuickJSScriptRunner({ hookTimeoutMs: 10_000 }); const hookOpts: ScriptRunOptions = { origin: { kind: 'hook', name: 't' } }; const actionOpts: ScriptRunOptions = { origin: { kind: 'action', name: 't' } }; @@ -364,6 +370,10 @@ describe('QuickJSScriptRunner — nested sandbox re-entrancy (#1867)', () => { // the 250ms hook default and killed mid-flight. // --------------------------------------------------------------------------- describe('QuickJSScriptRunner — timeout resolution honors body.timeoutMs (#1867)', () => { + // Stock engine defaults (250ms hooks) — these tests assert the default budget + // itself, so they must NOT use the generous shared `runner` above. + const defaultRunner = new QuickJSScriptRunner(); + it('honors a hook body timeoutMs above the 250ms hook default', async () => { // Host call settles at ~600ms — comfortably past the old 250ms hook cap but // within the body's declared 5000ms budget. Must resolve, not time out. @@ -375,7 +385,7 @@ describe('QuickJSScriptRunner — timeout resolution honors body.timeoutMs (#186 }, }), }; - const r = await runner.runScript( + const r = await defaultRunner.runScript( { language: 'js', source: "return await ctx.api.object('x').update({});", @@ -390,27 +400,26 @@ describe('QuickJSScriptRunner — timeout resolution honors body.timeoutMs (#186 it('still applies the 250ms hook default when the body declares no timeoutMs', async () => { const api = { object: () => ({ update: () => new Promise(() => {}) }) }; - const start = Date.now(); await expect( - runner.runScript( + defaultRunner.runScript( { language: 'js', source: "return await ctx.api.object('x').update({});", capabilities: ['api.write'] }, ctx({ api }), hookOpts, ), - ).rejects.toThrow(/timeout/i); - // Fired near the 250ms default, not some larger value. - expect(Date.now() - start).toBeLessThan(2000); + // The error message embeds the effective budget — asserting on it proves + // the 250ms default applied without a flaky wall-clock measurement. + ).rejects.toThrow(/timeout of 250ms/); }, 10000); it('lets a hook body LOWER its timeout below the default', async () => { const api = { object: () => ({ update: () => new Promise(() => {}) }) }; await expect( - runner.runScript( + defaultRunner.runScript( { language: 'js', source: "return await ctx.api.object('x').update({});", capabilities: ['api.write'], timeoutMs: 50 }, ctx({ api }), hookOpts, ), - ).rejects.toThrow(/timeout/i); + ).rejects.toThrow(/timeout of 50ms/); }, 10000); });