From 92403d823c599db79dc721d0e337793319c16228 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 19 Jul 2026 16:44:12 +0000 Subject: [PATCH 1/2] fix(runtime): sandbox budget = script CPU-time, not wall clock (ADR-0102 D1, #3295) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1 of #3275. The QuickJS sandbox now meters each hook/action invocation against VM-active (CPU) time, not wall clock. Idle host-await time and a nested hook's own execution (host-side, while the caller VM is parked) are no longer charged to the caller — so a slow/loaded host or a deep nested-write chain can't trip the budget while a script is merely waiting (the #3259 flake root cause). A separate wall-clock ceiling (default 30s, max(ceiling, cpuBudget)) backstops a body stuck on a never-settling host call. - runner: slice-stopwatch every VM entry (evalCode / evalCodeAsync / each executePendingJobs); the interrupt handler and post-slice checks compare against the CPU budget and the wall deadline; distinct errors "exceeded CPU budget of Nms" vs "exceeded wall-clock ceiling of Nms while awaiting host calls". A mid-slice interrupt — which for an async body surfaces via __error, not pending.error — is mapped to the clean budget message at all three sites. - new knobs: QuickJSScriptRunner wallCeilingMs + env OS_SANDBOX_WALL_CEILING_MS (resolveSandboxTimeoutMs gains a 'wallCeiling' kind). Precedence unchanged: explicit option > env > built-in default. - fix: init sliceStart to `start`, not 0 — a 0 init made `now - sliceStart` the full epoch millis, firing the interrupt during installCtx and corrupting ctx marshalling (__input never set). - tests: rewrite never-settling-host timeout tests to CPU-burn bodies (assert /CPU budget of Nms/) + dedicated small-wallCeilingMs runners for the genuine stuck-host cases (assert /wall-clock ceiling/); add the #3259 root-cause guard (idle host time resolves) and CPU-vs-ceiling coverage. - nested-write*.integration: drop the explicit 10s budget — they pass at the stock 250ms now, the nested-charging-fix regression guard. - docs: environment-variables.mdx CPU-time wording + OS_SANDBOX_WALL_CEILING_MS; ADR-0102 D1 → Accepted. Changeset carries the FROM→TO semantics note. 574 runtime tests + 26 types tests green. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_011iJLjqToxNv1aYP3syQtRp --- .changeset/sandbox-cpu-budget.md | 39 +++++ .../docs/deployment/environment-variables.mdx | 5 +- ...2-sandbox-cpu-budget-and-engine-variant.md | 4 +- ...sted-write-real-sqlite.integration.test.ts | 6 +- .../sandbox/nested-write.integration.test.ts | 12 +- .../src/sandbox/quickjs-runner.test.ts | 141 +++++++++++------- .../runtime/src/sandbox/quickjs-runner.ts | 120 ++++++++++++--- packages/types/src/env.test.ts | 15 ++ packages/types/src/env.ts | 48 +++--- 9 files changed, 284 insertions(+), 106 deletions(-) create mode 100644 .changeset/sandbox-cpu-budget.md diff --git a/.changeset/sandbox-cpu-budget.md b/.changeset/sandbox-cpu-budget.md new file mode 100644 index 0000000000..86c8ec0fb9 --- /dev/null +++ b/.changeset/sandbox-cpu-budget.md @@ -0,0 +1,39 @@ +--- +'@objectstack/runtime': minor +'@objectstack/types': minor +--- + +feat(runtime): sandbox budget is script CPU-time, not wall clock (ADR-0102 D1, #3295) + +The QuickJS sandbox now meters each hook/action invocation against how much +**VM-active (CPU) time** the body burns, not wall clock. Idle host-await time and +a nested hook's own execution (which runs host-side while the caller's VM is +parked) are no longer charged to the caller — so a slow/loaded host or a deep +nested-write chain can't trip the budget while a script is merely waiting (the +root cause of the #3259 CI flake). A separate, generous **wall-clock ceiling** +(default 30s, `max(ceiling, cpuBudget)`) remains as the backstop for a body stuck +on a host call that never settles. + +What changes for consumers (behaviour, not API signatures): + +- **Meaning of the timeout knobs.** `body.timeoutMs`, the `hookTimeoutMs` / + `actionTimeoutMs` runner options, and `OS_SANDBOX_HOOK_TIMEOUT_MS` / + `OS_SANDBOX_ACTION_TIMEOUT_MS` keep their **names, defaults (250ms / 5000ms), + and precedence** — but now bound CPU-time instead of wall-clock. In practice + this only *loosens* legitimate slow/nested work; a runaway synchronous script + is still cut at the same budget. +- **Error messages.** `exceeded timeout of Nms` → either `exceeded CPU budget of + Nms` (script burned its CPU budget) or `exceeded wall-clock ceiling of Nms + while awaiting host calls` (stuck on a never-settling host call). Update any + code/tests matching the old string. + +New knobs (additive): + +- `QuickJSScriptRunner` option `wallCeilingMs` and env `OS_SANDBOX_WALL_CEILING_MS` + — tune the wall ceiling (explicit option › env › 30s). +- `resolveSandboxTimeoutMs` (`@objectstack/types`) gains a `'wallCeiling'` kind. + +Also fixes a latent init bug in the new accounting where the interrupt handler +could fire during `installCtx` and corrupt ctx marshalling. The nested-write +integration suites now run at the stock 250ms budget (previously forced to 10s), +which is itself the regression guard for the nested-charging fix. diff --git a/content/docs/deployment/environment-variables.mdx b/content/docs/deployment/environment-variables.mdx index 2228444b7c..6c8cf36c5a 100644 --- a/content/docs/deployment/environment-variables.mdx +++ b/content/docs/deployment/environment-variables.mdx @@ -331,8 +331,9 @@ the hosted ObjectOS Cloud control plane. | `OS_ENV_CACHE_TTL_MS` | number | `300000` | Env-record cache TTL in ms. | | `OS_ARTIFACT_CACHE_TTL_MS` | number | `300000` | Artifact-record cache TTL in ms. | | `OS_ARTIFACT_FETCH_TIMEOUT_MS` | number | `60000` | Timeout for remote artifact fetches. Only a positive value is honored — an unset, non-numeric, or `0` value falls back to the 60s default (pass `fetchTimeoutMs: 0` in `artifactSource` config to actually disable the timeout). | -| `OS_SANDBOX_HOOK_TIMEOUT_MS` | number | `250` | Default wall-clock budget for a sandboxed **hook** body (QuickJS). Each invocation compiles a fresh WASM module and a nested hook compiles another inside the parent's budget, so a loaded/slow host (e.g. an oversubscribed CI runner) may need a larger floor. Only a positive integer is honored; unset / non-numeric / non-positive keeps the 250ms default. A hook body's own declared `timeoutMs` still wins over this. | -| `OS_SANDBOX_ACTION_TIMEOUT_MS` | number | `5000` | Default wall-clock budget for a sandboxed **action** body (QuickJS). Same resolution rules as the hook variant above (positive integer only; an action body's own `timeoutMs` still wins). | +| `OS_SANDBOX_HOOK_TIMEOUT_MS` | number | `250` | Default **CPU-time** budget for a sandboxed **hook** body (QuickJS, ADR-0102): how much *VM-active* time a body may burn — idle host-await time and a nested hook's own run are NOT charged. A loaded/slow host rarely needs to raise this now (it is not wall-clock), but the knob remains. Only a positive integer is honored; unset / non-numeric / non-positive keeps the 250ms default. A hook body's own declared `timeoutMs` still wins over this. | +| `OS_SANDBOX_ACTION_TIMEOUT_MS` | number | `5000` | Default **CPU-time** budget for a sandboxed **action** body (QuickJS). Same resolution rules as the hook variant above (positive integer only; an action body's own `timeoutMs` still wins). | +| `OS_SANDBOX_WALL_CEILING_MS` | number | `30000` | Wall-clock ceiling (ADR-0102) — the backstop that cuts a hook/action body stuck on a host call that never settles (which burns no CPU, so the CPU budget alone would never fire). The effective ceiling is `max(this, cpuBudget)`, so it can never cut a body still inside its CPU budget. Positive integer only; unset keeps 30s. | | `OS_INLINE_SEED_BUDGET_MS` | number | `8000` | Time budget for synchronous seed execution at boot before deferring to a worker. | | `OS_TENANT_AUDIT` | flag | `1` | Set to `0` to silence the tenant-isolation audit warnings emitted by the SQL driver. | diff --git a/docs/adr/0102-sandbox-cpu-budget-and-engine-variant.md b/docs/adr/0102-sandbox-cpu-budget-and-engine-variant.md index f69f226e04..96949ef785 100644 --- a/docs/adr/0102-sandbox-cpu-budget-and-engine-variant.md +++ b/docs/adr/0102-sandbox-cpu-budget-and-engine-variant.md @@ -1,6 +1,6 @@ # ADR-0102: Sandbox Execution Budget & Engine Variant — CPU-time budget with a wall ceiling; per-invocation sync WASM modules, precompiled -**Status**: Proposed (2026-07-19) — spec'd and API-verified in framework#3275; lands as Phase 1 #3295 (D1), Phase 2 #3296 (D2), Phase 3 #3297 (D3). Flips to **Accepted** when Phase 1 merges; D3 is explicitly deferrable without weakening D1/D2. +**Status**: Accepted (2026-07-19) — **D1 landed** with Phase 1 (#3295): CPU-time budget + wall ceiling in `QuickJSScriptRunner`, nested-write integration tests now green at the stock 250ms budget. D2 (#3296, drop asyncify) and D3 (#3297, precompile) remain **Proposed**; D3 is explicitly deferrable without weakening D1/D2. **Deciders**: ObjectStack Protocol Architects **Builds on**: the #1867 sandbox redesign (deferred-promise host calls + pump loop) — the mechanism that both retired asyncify's only remaining justification (D2) and created the discrete VM-entry points D1 meters. No earlier ADR covers the sandbox runner; prior art is code history (#3232 honor body `timeoutMs`, #3264 test de-flake, #3270 env-overridable defaults). **Tracking**: framework#3275 (implementation-ready spec) · #3295 / #3296 / #3297 (phases) · #3259 (motivating CI flake, closed by the #3270 stopgap) @@ -41,7 +41,7 @@ The value resolved by `resolveTimeout()` (body `timeoutMs` › enclosing `opts.t Consequence for the knobs: names, defaults (250ms hooks / 5000ms actions), and precedence are unchanged; only the *dimension* changes. Raising the CPU budget no longer trades away host-latency tolerance, and vice versa. This restores meaning to the 250ms default on any hardware — which is what lets CI drop its 10s floor (a stopgap that today also loosens the runaway bound for every test). -Open point to settle at Phase 1 review: whether the ceiling also gets `OS_SANDBOX_WALL_CEILING_MS` (recommended — symmetric with the CPU knobs, wanted by slow-IO deployments) or stays constructor-only. +Settled (Phase 1): the ceiling is both a constructor option (`wallCeilingMs`, needed for fast tests) AND env-tunable via `OS_SANDBOX_WALL_CEILING_MS` — symmetric with the CPU knobs and wanted by slow-IO deployments. Precedence: explicit option › env › built-in 30s. ### D2 — Drop asyncify: per-invocation *sync* modules (isolation unchanged) 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 e31897f27a..2ab14a6438 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,9 +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); - // 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' })); + // Runs at the STOCK 250ms CPU-time budget (ADR-0102 D1): idle host-await time + // and the nested rollup's own work are not charged (see nested-write.integration.test.ts). + engine.setDefaultBodyRunner(hookBodyRunnerFactory(new QuickJSScriptRunner(), { 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 57c143aa1d..c40ff813b5 100644 --- a/packages/runtime/src/sandbox/nested-write.integration.test.ts +++ b/packages/runtime/src/sandbox/nested-write.integration.test.ts @@ -90,13 +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"). + // Runs at the STOCK 250ms budget on purpose (ADR-0102 D1): the budget is now + // CPU-time, so the child hook parking on its nested `parent.update(...)` — and + // the parent hook that fires host-side inside it — is not charged to the + // child. If this suite ever needs a generous budget again, that is a + // regression in CPU accounting, not an inherent cost of nested writes. engine.setDefaultBodyRunner( - hookBodyRunnerFactory(new QuickJSScriptRunner({ hookTimeoutMs: 10_000 }), { ql: engine, appId: 'test' }), + hookBodyRunnerFactory(new QuickJSScriptRunner(), { 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 2e78a2e0bc..9e6f9516ea 100644 --- a/packages/runtime/src/sandbox/quickjs-runner.test.ts +++ b/packages/runtime/src/sandbox/quickjs-runner.test.ts @@ -53,7 +53,7 @@ describe('QuickJSScriptRunner — L2 hook script', () => { expect(r.value).toEqual({ ok: true, doubled: 42 }); }); - it('respects the timeoutMs cap', async () => { + it('respects the timeoutMs cap (CPU budget)', async () => { await expect( runner.runScript( { @@ -65,7 +65,7 @@ describe('QuickJSScriptRunner — L2 hook script', () => { ctx(), hookOpts, ), - ).rejects.toThrow(); + ).rejects.toThrow(/CPU budget of 50ms/); }); it('rejects use of api.read without capability', async () => { @@ -402,43 +402,41 @@ describe('QuickJSScriptRunner — timeout resolution honors body.timeoutMs (#186 expect(r.value).toEqual({ ok: true }); }, 10000); - it('still applies the 250ms hook default when the body declares no timeoutMs', async () => { - const api = { object: () => ({ update: () => new Promise(() => {}) }) }; + it('applies the 250ms default CPU budget when the body declares no timeoutMs', async () => { + // A synchronous busy loop burns CPU (unlike a never-settling host call, which + // is idle and would hit the wall ceiling instead). The error embeds the + // effective budget — asserting on it proves the 250ms default applied. await expect( defaultRunner.runScript( - { language: 'js', source: "return await ctx.api.object('x').update({});", capabilities: ['api.write'] }, - ctx({ api }), + { language: 'js', source: 'while (true) {}', capabilities: [] }, + ctx(), hookOpts, ), - // 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/); + ).rejects.toThrow(/CPU budget of 250ms/); }, 10000); - it('lets a hook body LOWER its timeout below the default', async () => { - const api = { object: () => ({ update: () => new Promise(() => {}) }) }; + it('lets a hook body LOWER its CPU budget below the default', async () => { await expect( defaultRunner.runScript( - { language: 'js', source: "return await ctx.api.object('x').update({});", capabilities: ['api.write'], timeoutMs: 50 }, - ctx({ api }), + { language: 'js', source: 'while (true) {}', capabilities: [], timeoutMs: 50 }, + ctx(), hookOpts, ), - ).rejects.toThrow(/timeout of 50ms/); + ).rejects.toThrow(/CPU budget of 50ms/); }, 10000); }); // --------------------------------------------------------------------------- -// Env-overridable timeout defaults (#3259). Every invocation compiles a fresh -// WASM module, and a nested hook compiles another inside the parent's budget, so -// on a loaded/slow host (an oversubscribed CI runner) the 250ms hook default can -// trip even while the VM is making progress. `OS_SANDBOX_HOOK_TIMEOUT_MS` lets an -// operator raise the FALLBACK default without a code change; an explicit -// constructor option still wins over the env, and a body's own timeoutMs still -// wins over the resolved default. Each test constructs its runner AFTER setting -// the env (the constructor reads it once), and the env is saved/restored so a -// CI-wide value doesn't leak in or out. +// Env-overridable CPU-budget defaults (#3259 / ADR-0102 D1). `OS_SANDBOX_HOOK_TIMEOUT_MS` +// sets the FALLBACK per-invocation CPU budget for hooks; an explicit constructor +// option still wins over the env, and a body's own timeoutMs still wins over the +// resolved default. A synchronous busy loop (`while(true){}`) burns CPU, so the +// error embeds the RESOLVED budget — asserting on it proves which value applied +// without a flaky wall-clock measurement. Each test constructs its runner AFTER +// setting the env (the constructor reads it once), and the env is saved/restored +// so a CI-wide value doesn't leak in or out. // --------------------------------------------------------------------------- -describe('QuickJSScriptRunner — env-overridable timeout defaults (#3259)', () => { +describe('QuickJSScriptRunner — env-overridable CPU-budget defaults (#3259)', () => { const HOOK_ENV = 'OS_SANDBOX_HOOK_TIMEOUT_MS'; let saved: string | undefined; beforeEach(() => { @@ -450,34 +448,68 @@ describe('QuickJSScriptRunner — env-overridable timeout defaults (#3259)', () else process.env[HOOK_ENV] = saved; }); - // A never-settling host call forces the deadline path; the SandboxError embeds - // the RESOLVED budget, so asserting on the message proves which timeout applied - // without a flaky wall-clock measurement. - const neverSettles = { object: () => ({ update: () => new Promise(() => {}) }) }; - const runHook = (r: QuickJSScriptRunner) => - r.runScript( - { language: 'js', source: "return await ctx.api.object('x').update({});", capabilities: ['api.write'] }, - ctx({ api: neverSettles }), - hookOpts, - ); + const runBurn = (r: QuickJSScriptRunner) => + r.runScript({ language: 'js', source: 'while (true) {}', capabilities: [] }, ctx(), hookOpts); - it('falls back to the built-in 250ms hook default when the env var is unset', async () => { - await expect(runHook(new QuickJSScriptRunner())).rejects.toThrow(/timeout of 250ms/); + it('falls back to the built-in 250ms CPU budget when the env var is unset', async () => { + await expect(runBurn(new QuickJSScriptRunner())).rejects.toThrow(/CPU budget of 250ms/); }, 10000); it('uses OS_SANDBOX_HOOK_TIMEOUT_MS as the default when set', async () => { process.env[HOOK_ENV] = '150'; - await expect(runHook(new QuickJSScriptRunner())).rejects.toThrow(/timeout of 150ms/); + await expect(runBurn(new QuickJSScriptRunner())).rejects.toThrow(/CPU budget of 150ms/); }, 10000); it('lets an explicit constructor option win over the env var', async () => { process.env[HOOK_ENV] = '150'; - await expect(runHook(new QuickJSScriptRunner({ hookTimeoutMs: 50 }))).rejects.toThrow(/timeout of 50ms/); + await expect(runBurn(new QuickJSScriptRunner({ hookTimeoutMs: 50 }))).rejects.toThrow(/CPU budget of 50ms/); }, 10000); it('ignores a non-numeric / non-positive env value and keeps the built-in default', async () => { process.env[HOOK_ENV] = 'not-a-number'; - await expect(runHook(new QuickJSScriptRunner())).rejects.toThrow(/timeout of 250ms/); + await expect(runBurn(new QuickJSScriptRunner())).rejects.toThrow(/CPU budget of 250ms/); + }, 10000); +}); + +// --------------------------------------------------------------------------- +// CPU budget vs wall ceiling (ADR-0102 D1). The two bounds are distinct: CPU +// time bounds a runaway *script*; the wall ceiling bounds a body stuck on a host +// call that never settles. Critically, idle host-await time is NOT charged to the +// CPU budget — the regression that caused the #3259 flake. +// --------------------------------------------------------------------------- +describe('QuickJSScriptRunner — CPU budget vs wall ceiling (ADR-0102)', () => { + it('does NOT charge idle host-await time to the CPU budget (#3259 root cause)', async () => { + // Host settles ~500ms — well past the 250ms CPU budget — but the VM burns ~no + // CPU while awaiting, so it MUST resolve. The old wall-clock 250ms killed this. + const api = { + object: () => ({ + update: async () => { + await new Promise((resolve) => setTimeout(resolve, 500)); + return { ok: true }; + }, + }), + }; + const r = new QuickJSScriptRunner({ hookTimeoutMs: 250 }); + const out = await r.runScript( + { language: 'js', source: "return await ctx.api.object('x').update({});", capabilities: ['api.write'] }, + ctx({ api }), + hookOpts, + ); + expect(out.value).toEqual({ ok: true }); + }, 10000); + + it('cuts a body stuck on a never-settling host call at the wall ceiling', async () => { + // CPU budget 250ms is never reached (the VM is idle); wallCeilingMs 300ms — + // effective ceiling max(300, 250) = 300 — is the backstop that fires. + const api = { object: () => ({ update: () => new Promise(() => {}) }) }; + const r = new QuickJSScriptRunner({ hookTimeoutMs: 250, wallCeilingMs: 300 }); + await expect( + r.runScript( + { language: 'js', source: "return await ctx.api.object('x').update({});", capabilities: ['api.write'] }, + ctx({ api }), + hookOpts, + ), + ).rejects.toThrow(/wall-clock ceiling of 300ms/); }, 10000); }); @@ -558,26 +590,26 @@ describe('QuickJSScriptRunner — long-running async host work (pump budget)', ( expect(calls).toBe(N); }, 40000); - it('still enforces the timeout on a host call that never settles', async () => { + it('still enforces the wall ceiling on a host call that never settles', async () => { const api = { object: () => ({ - // Never resolves — must be killed by the deadline, not hang forever. + // Never resolves — an idle body burns no CPU, so it is the wall ceiling + // (not the CPU budget) that must kill it rather than hang forever. update: () => new Promise(() => {}), }), }; - + const r = new QuickJSScriptRunner({ actionTimeoutMs: 300, wallCeilingMs: 300 }); await expect( - runner.runScript( + r.runScript( { language: 'js', source: "return await ctx.api.object('wid').update({ id: 'x' });", capabilities: ['api.write'], - timeoutMs: 300, }, ctx({ api }), actionOpts, ), - ).rejects.toThrow(/timeout/i); + ).rejects.toThrow(/wall-clock ceiling/i); }, 10000); }); @@ -733,12 +765,12 @@ describe('QuickJSScriptRunner — ctx.api.transaction', () => { ).rejects.toThrow(/api\.transaction/); }, 30000); - it('rolls back a transaction the body leaves open when the deadline fires', async () => { + it('rolls back a transaction the body leaves open when the wall ceiling fires', async () => { const events: Array<{ op: string; tx: number | null }> = []; let nextTx = 0; const api = { object: () => ({ - // never settles — the in-tx op stalls until the deadline cuts in + // never settles — the in-tx op stalls until the wall ceiling cuts in insert: () => new Promise(() => {}), }), beginTransaction: async () => { @@ -750,8 +782,10 @@ describe('QuickJSScriptRunner — ctx.api.transaction', () => { rollbackTransaction: async (h: number) => { events.push({ op: 'rollback', tx: h }); }, }; + // Idle in-tx op → the wall ceiling (not the CPU budget) is the backstop. + const r = new QuickJSScriptRunner({ wallCeilingMs: 300 }); await expect( - runner.runScript( + r.runScript( { language: 'js', source: ` @@ -765,9 +799,9 @@ describe('QuickJSScriptRunner — ctx.api.transaction', () => { ctx({ api }), actionOpts, ), - ).rejects.toThrow(/timeout/i); + ).rejects.toThrow(/ceiling/i); - // begin happened, the op stalled, deadline fired → finally rolled it back. + // begin happened, the op stalled, wall ceiling fired → finally rolled it back. expect(events.map((e) => e.op)).toEqual(['begin', 'rollback']); }, 10000); @@ -808,11 +842,12 @@ describe('QuickJSScriptRunner — ctx.api.transaction', () => { // --------------------------------------------------------------------------- describe('QuickJSScriptRunner — idle pump backoff (#3233)', () => { it('does not busy-spin while idle-waiting on a slow host call — pump count stays bounded', async () => { - // A host call that never settles; the deadline cuts it off. Under the old + // A host call that never settles; the wall ceiling cuts it off. Under the old // unconditional setImmediate yield this reported ~50k pump iterations for a // ~250ms wait; the adaptive backoff keeps it to a small bounded number. const api = { object: () => ({ update: () => new Promise(() => {}) }) }; - const err = await runner + const r = new QuickJSScriptRunner({ wallCeilingMs: 300 }); + const err = await r .runScript( { language: 'js', source: "return await ctx.api.object('x').update({});", capabilities: ['api.write'], timeoutMs: 300 }, ctx({ api }), @@ -821,7 +856,7 @@ describe('QuickJSScriptRunner — idle pump backoff (#3233)', () => { .then(() => null, (e) => e as SandboxError); expect(err).toBeInstanceOf(SandboxError); const m = /after (\d+) pump iterations/.exec(err!.message); - expect(m, `timeout message should report the pump count: ${err?.message}`).toBeTruthy(); + expect(m, `ceiling message should report the pump count: ${err?.message}`).toBeTruthy(); const pumps = Number(m![1]); // ~300ms at an ≤8ms idle poll ≈ tens of pumps, not tens of thousands. expect(pumps).toBeLessThan(1000); diff --git a/packages/runtime/src/sandbox/quickjs-runner.ts b/packages/runtime/src/sandbox/quickjs-runner.ts index 68e8133570..433b7cba72 100644 --- a/packages/runtime/src/sandbox/quickjs-runner.ts +++ b/packages/runtime/src/sandbox/quickjs-runner.ts @@ -21,12 +21,13 @@ * fresh WASM module via `newAsyncContext()` and disposes it on settle (see * {@link QuickJSScriptRunner.execute}). Runtimes are deliberately NOT pooled — * a shared module hit HostRef double-free crashes when contexts were disposed - * concurrently, and the per-invocation isolate sidesteps that. The cost is - * real, though: a nested hook compiles a SECOND module inside the parent's - * budget, so on a loaded/slow host the fixed compile cost alone can trip the - * hook timeout. The per-invocation timeout default is therefore env-overridable - * via `OS_SANDBOX_HOOK_TIMEOUT_MS` / `OS_SANDBOX_ACTION_TIMEOUT_MS` - * (framework#3259) so an operator can raise the floor without a code change. + * concurrently, and the per-invocation isolate sidesteps that (ADR-0102 D4). + * - Budgeting is CPU-time, not wall-clock (ADR-0102 D1): the per-invocation + * `timeoutMs` bounds VM-active time (env defaults `OS_SANDBOX_HOOK_TIMEOUT_MS` / + * `OS_SANDBOX_ACTION_TIMEOUT_MS`; `OS_SANDBOX_WALL_CEILING_MS` the wall + * backstop), so host-await time and a nested hook's own run are not charged to + * the caller — the load flake that motivated this (framework#3259) cannot recur + * while a script is merely waiting on the host. * - Memory caps are advisory under quickjs (engine has no hard MB cap); the * runner uses `setMemoryLimit(memoryMb * 1MB)` which is best-effort. */ @@ -49,12 +50,23 @@ import type { const DEFAULT_HOOK_TIMEOUT_MS = 250; const DEFAULT_ACTION_TIMEOUT_MS = 5000; const DEFAULT_MEMORY_MB = 32; +// Wall-clock backstop (ADR-0102 D1): the CPU budget bounds VM-active time, but a +// body parked forever on a host call that never settles burns no CPU — the +// interrupt handler can't fire while no VM code runs — so a separate, generous +// wall ceiling cuts it off. 30s matches the spec cap on `ScriptBody.timeoutMs`. +const DEFAULT_WALL_CEILING_MS = 30_000; export interface QuickJSScriptRunnerOptions { - /** Default per-invocation timeout for hooks (ms). */ + /** Default per-invocation **CPU-time** budget for hooks (ms). */ hookTimeoutMs?: number; - /** Default per-invocation timeout for actions (ms). */ + /** Default per-invocation **CPU-time** budget for actions (ms). */ actionTimeoutMs?: number; + /** + * Wall-clock ceiling (ms) — the backstop for a body stuck on a never-settling + * host call. Effective ceiling is `max(this, cpuBudget)`, so it can never cut + * a body still inside its CPU budget. Default 30_000. + */ + wallCeilingMs?: number; /** Default memory cap in MB. */ memoryMb?: number; } @@ -72,6 +84,7 @@ export class QuickJSScriptRunner implements ScriptRunner { this.opts = { hookTimeoutMs: opts.hookTimeoutMs ?? resolveSandboxTimeoutMs('hook', DEFAULT_HOOK_TIMEOUT_MS), actionTimeoutMs: opts.actionTimeoutMs ?? resolveSandboxTimeoutMs('action', DEFAULT_ACTION_TIMEOUT_MS), + wallCeilingMs: opts.wallCeilingMs ?? resolveSandboxTimeoutMs('wallCeiling', DEFAULT_WALL_CEILING_MS), memoryMb: opts.memoryMb ?? DEFAULT_MEMORY_MB, }; } @@ -160,9 +173,46 @@ export class QuickJSScriptRunner implements ScriptRunner { runtime.setMemoryLimit(args.memoryMb * 1024 * 1024); runtime.setMaxStackSize(512 * 1024); + // Budget accounting (ADR-0102 D1). `args.timeoutMs` is a CPU-time budget: the + // sum of VM-active slices (the initial eval + each `executePendingJobs`), NOT + // wall clock. Idle pump yields, host-promise settle time, and nested-hook + // execution (which runs host-side while this VM is parked) are excluded — so a + // slow/loaded host or a deep nested-write chain no longer trips the budget + // while the script is merely waiting. A separate wall ceiling bounds a body + // stuck forever on a host call that never settles. + const cpuBudget = args.timeoutMs; + const wallCeiling = Math.max(this.opts.wallCeilingMs, cpuBudget); const start = Date.now(); - const deadline = start + args.timeoutMs; - runtime.setInterruptHandler(() => Date.now() > deadline); + const wallDeadline = start + wallCeiling; + let cpuMs = 0; // sum of completed VM slices + // Init to `start`, NOT 0: the interrupt handler reads `now - sliceStart`, and a + // 0 init makes that the full unix-epoch millis — firing the interrupt during + // `installCtx` (before the first real slice), which corrupts ctx marshalling + // (the host-side setup evalCodes get interrupted). Elapsed-since-start is ~0 + // here, so the handler stays quiet until a real slice begins. + let sliceStart = start; + // Fires only while VM code runs, so `now - sliceStart` is the in-flight slice; + // added to finished `cpuMs` it is total CPU. Cuts a runaway synchronous loop + // the instant the CPU budget (or the wall ceiling) is exceeded. + runtime.setInterruptHandler( + () => cpuMs + (Date.now() - sliceStart) > cpuBudget || Date.now() > wallDeadline, + ); + // Map a settled-VM state to the right budget error, or null if the body is + // simply still progressing. Callers add the just-run slice to `cpuMs` first, + // so `cpuMs` here is inclusive. + const budgetError = (pumps: number): SandboxError | null => { + if (cpuMs > cpuBudget) { + return new SandboxError( + `${args.origin.kind} '${args.origin.name}' exceeded CPU budget of ${cpuBudget}ms (after ${pumps} pump iterations)`, + ); + } + if (Date.now() > wallDeadline) { + return new SandboxError( + `${args.origin.kind} '${args.origin.name}' exceeded wall-clock ceiling of ${wallCeiling}ms while awaiting host calls (after ${pumps} pump iterations)`, + ); + } + return null; + }; // Shared, per-invocation transaction state. `ctx.api.transaction(fn)` opens // it (routing subsequent ctx.api ops through the tx-scoped context) and @@ -177,8 +227,15 @@ export class QuickJSScriptRunner implements ScriptRunner { // L1 expressions are pure-sync: evaluate and read __result. if (args.isExpression) { const wrapped = `globalThis.__result = JSON.stringify((function(){ return (${args.source}); })());`; + sliceStart = Date.now(); const result = vm.evalCode(wrapped); + cpuMs += Date.now() - sliceStart; if (result.error) { + const budget = budgetError(0); + if (budget) { + result.error.dispose(); + throw budget; + } const err = vm.dump(result.error); result.error.dispose(); throw new SandboxError( @@ -213,8 +270,15 @@ export class QuickJSScriptRunner implements ScriptRunner { function(e){ globalThis.__error = (e && e.message) ? (e.name + ': ' + e.message) : String(e); } );`; + sliceStart = Date.now(); const evalRes = await vm.evalCodeAsync(wrapped); + cpuMs += Date.now() - sliceStart; if (evalRes.error) { + const budget = budgetError(0); + if (budget) { + evalRes.error.dispose(); + throw budget; + } const err = vm.dump(evalRes.error); evalRes.error.dispose(); throw new SandboxError( @@ -227,12 +291,13 @@ export class QuickJSScriptRunner implements ScriptRunner { // Drive the script's async continuations to completion. Each iteration // yields to the host event loop (so in-flight host promises settle and // resolve their VM-side deferred handles) and then drains the QuickJS job - // queue. The ONLY bound on how long we wait is the deadline: a slow but - // progressing script — many sequential host writes, or one write that - // synchronously drives a downstream record-change automation — must be - // allowed to finish within its timeout, and a stuck / never-settling host - // call is cut off here (the QuickJS interrupt handler can't fire while we - // are parked on a host promise, so this deadline check is the backstop). + // queue. Two bounds apply (ADR-0102 D1): the CPU budget (VM-active time, + // via `budgetError` below) and the wall ceiling. A slow but progressing + // script — many sequential host writes, or one write that synchronously + // drives a downstream record-change automation — burns little CPU per pump + // and must be allowed to finish; a stuck / never-settling host call (the + // interrupt handler can't fire while we are parked on a host promise) is + // cut off by the wall ceiling, which this loop's `budgetError` checks. // The previous fixed `pumps < 1000` cap fired in ~tens of ms on legitimate // work and surfaced as "did not resolve after 1000 pump iterations". // Adaptive idle backoff (#3233): while the script is progressing we pump @@ -256,8 +321,18 @@ export class QuickJSScriptRunner implements ScriptRunner { await new Promise((resolve) => setTimeout(resolve, backoffMs)); } + sliceStart = Date.now(); const pending = runtime.executePendingJobs(); + cpuMs += Date.now() - sliceStart; if (pending.error) { + // An interrupt (CPU budget / wall ceiling hit mid-slice) surfaces here + // as an error too — map it to the clean budget message rather than + // dumping `InternalError: interrupted`. + const budget = budgetError(pumps); + if (budget) { + pending.error.dispose(); + throw budget; + } const err = vm.dump(pending.error); pending.error.dispose(); throw new SandboxError( @@ -270,6 +345,12 @@ export class QuickJSScriptRunner implements ScriptRunner { const errStr = vm.dump(errH); errH.dispose(); if (errStr) { + // An interrupt (CPU budget / wall ceiling) raised inside the async body + // rejects its promise, surfacing here as __error rather than + // pending.error — map it to the budget message instead of the raw + // "InternalError: interrupted". + const budget = budgetError(pumps); + if (budget) throw budget; throw new SandboxError( `${args.origin.kind} '${args.origin.name}' threw: ${errStr}`, userFacingMessage(String(errStr)), @@ -286,11 +367,8 @@ export class QuickJSScriptRunner implements ScriptRunner { return { value, mutatedInput, durationMs: Date.now() - start }; } - if (Date.now() > deadline) { - throw new SandboxError( - `${args.origin.kind} '${args.origin.name}' exceeded timeout of ${args.timeoutMs}ms (after ${pumps} pump iterations)`, - ); - } + const budget = budgetError(pumps); + if (budget) throw budget; // Progress this pump → reset to the fast path; genuinely idle → ramp the // counter so the next yield backs off. idle = pending.value > 0 ? 0 : idle + 1; diff --git a/packages/types/src/env.test.ts b/packages/types/src/env.test.ts index 37f77e8637..a0c4590822 100644 --- a/packages/types/src/env.test.ts +++ b/packages/types/src/env.test.ts @@ -242,13 +242,17 @@ describe('MCP switches — HTTP surface vs stdio auto-start are decoupled (#3167 describe('resolveSandboxTimeoutMs (#3259)', () => { const HOOK = 'OS_SANDBOX_HOOK_TIMEOUT_MS'; const ACTION = 'OS_SANDBOX_ACTION_TIMEOUT_MS'; + const WALL = 'OS_SANDBOX_WALL_CEILING_MS'; const origHook = process.env[HOOK]; const origAction = process.env[ACTION]; + const origWall = process.env[WALL]; afterEach(() => { if (origHook === undefined) delete process.env[HOOK]; else process.env[HOOK] = origHook; if (origAction === undefined) delete process.env[ACTION]; else process.env[ACTION] = origAction; + if (origWall === undefined) delete process.env[WALL]; + else process.env[WALL] = origWall; }); it('returns the fallback unchanged when the var is unset', () => { @@ -283,4 +287,15 @@ describe('resolveSandboxTimeoutMs (#3259)', () => { process.env[HOOK] = '3000ms'; expect(resolveSandboxTimeoutMs('hook', 250)).toBe(3000); }); + + it("resolves the wall-ceiling kind from OS_SANDBOX_WALL_CEILING_MS (ADR-0102)", () => { + delete process.env[WALL]; + expect(resolveSandboxTimeoutMs('wallCeiling', 30_000)).toBe(30_000); // unset → fallback + process.env[WALL] = '60000'; + expect(resolveSandboxTimeoutMs('wallCeiling', 30_000)).toBe(60_000); + // Independent of the hook/action vars. + process.env[HOOK] = '111'; + process.env[ACTION] = '222'; + expect(resolveSandboxTimeoutMs('wallCeiling', 30_000)).toBe(60_000); + }); }); diff --git a/packages/types/src/env.ts b/packages/types/src/env.ts index ad17986c73..f317ef7faf 100644 --- a/packages/types/src/env.ts +++ b/packages/types/src/env.ts @@ -240,32 +240,42 @@ export function resolveSearchPinyinEnabled(opts?: { locales?: readonly string[] } /** - * SINGLE decision point for the sandbox script-runner's DEFAULT per-invocation - * timeout (ms), for a given origin kind, from the environment (framework#3259). + * SINGLE decision point for a sandbox script-runner DEFAULT (ms), resolved from + * the environment (framework#3259 / ADR-0102). * - * The QuickJS sandbox enforces a wall-clock deadline on every hook/action - * invocation. The built-in defaults (250ms hooks / 5000ms actions) suit a warm, - * idle host — but every invocation compiles a fresh WASM module, and a nested - * hook compiles ANOTHER one inside the parent's budget, so on a heavily loaded - * or slow host (an oversubscribed CI runner, constrained production hardware) - * that fixed VM-creation cost alone can trip the hook default even while the VM - * is still making progress. That surfaced as an intermittent CI flake ("hook - * '…' exceeded timeout of 250ms"). This lets an operator raise the floor once, - * deployment-wide, instead of re-tuning every call site. + * The QuickJS sandbox meters each hook/action invocation against a per-invocation + * budget. Two dimensions are env-tunable: + * - the **CPU-time budget** for hooks / actions — how much *VM-active* time a + * body may burn (built-in 250ms hooks / 5000ms actions); and + * - the **wall-clock ceiling** — the backstop bounding a body parked forever on + * a host call that never settles (built-in 30_000ms). + * + * The built-in defaults suit a warm, idle host; a heavily loaded or slow host + * (an oversubscribed CI runner, constrained production hardware) may need a + * higher floor. This lets an operator raise it once, deployment-wide, instead of + * re-tuning every call site. * * Canonical vars (OS_{DOMAIN}_{NAME}, DOMAIN=SANDBOX): - * - hook → `OS_SANDBOX_HOOK_TIMEOUT_MS` - * - action → `OS_SANDBOX_ACTION_TIMEOUT_MS` + * - hook → `OS_SANDBOX_HOOK_TIMEOUT_MS` + * - action → `OS_SANDBOX_ACTION_TIMEOUT_MS` + * - wallCeiling → `OS_SANDBOX_WALL_CEILING_MS` * * Only a positive integer is honored; unset / empty / non-numeric / non-positive * falls back to `fallback`, so behaviour is byte-for-byte unchanged when the var - * is absent. This is a FALLBACK default ONLY: an explicit `hookTimeoutMs` / - * `actionTimeoutMs` passed to the runner constructor still wins over it, and a - * body's own declared `timeoutMs` still wins over the resolved default per the - * runner's timeout-resolution rule (the smaller of the explicit values). + * is absent. This is a FALLBACK default ONLY: an explicit constructor option + * still wins over it, and (for the CPU budget) a body's own declared `timeoutMs` + * still wins over the resolved default per the runner's resolution rule. */ -export function resolveSandboxTimeoutMs(kind: 'hook' | 'action', fallback: number): number { - const name = kind === 'hook' ? 'OS_SANDBOX_HOOK_TIMEOUT_MS' : 'OS_SANDBOX_ACTION_TIMEOUT_MS'; +export function resolveSandboxTimeoutMs( + kind: 'hook' | 'action' | 'wallCeiling', + fallback: number, +): number { + const name = + kind === 'hook' + ? 'OS_SANDBOX_HOOK_TIMEOUT_MS' + : kind === 'action' + ? 'OS_SANDBOX_ACTION_TIMEOUT_MS' + : 'OS_SANDBOX_WALL_CEILING_MS'; const raw = readEnvWithDeprecation(name, [], { silent: true }); if (raw == null || String(raw).trim() === '') return fallback; const n = Number.parseInt(String(raw).trim(), 10); From 0dc44cfe039ed91e802803be8e12923605c19c06 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 19 Jul 2026 16:46:25 +0000 Subject: [PATCH 2/2] docs(automation): hook-bodies budget is CPU-time, nested rollups need no larger timeout (ADR-0102) hook-bodies.mdx said per-invocation "timeouts" are wall-clock and told authors to raise `timeoutMs` for deeper/slower rollup chains. Under ADR-0102 D1 the budget is script CPU-time: awaiting host calls and nested-hook execution are not charged, so the stock 250ms default covers deep rollups and the guidance to bump `timeoutMs` no longer applies. Notes the 30s wall ceiling + OS_SANDBOX_* knobs. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_011iJLjqToxNv1aYP3syQtRp --- content/docs/automation/hook-bodies.mdx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/content/docs/automation/hook-bodies.mdx b/content/docs/automation/hook-bodies.mdx index 55df9dcecc..02b0bc7fe8 100644 --- a/content/docs/automation/hook-bodies.mdx +++ b/content/docs/automation/hook-bodies.mdx @@ -129,11 +129,11 @@ Hooks mutate `ctx.input`/`ctx.result`; actions return their output value explici The sandbox engine is **`quickjs-emscripten`** — pure-WASM, runs on every JS host. We considered `isolated-vm` but its native dependency disqualifies edge targets. The choice is hidden behind the `ScriptRunner` interface in `packages/runtime/src/sandbox/`, so a node-only deployment can swap in a faster engine later without touching call sites. -Per-invocation timeouts default to **250ms** for hooks and **5000ms** for actions; per-invocation memory caps at **32 MB**. Both are overridable per body. +Per-invocation budgets default to **250ms** (hooks) / **5000ms** (actions) of **script CPU time** — VM-active time, *not* wall clock (ADR-0102): time spent awaiting host calls, or running a nested hook, is not charged. A separate **30s wall-clock ceiling** backstops a body stuck on a host call that never settles. Per-invocation memory caps at **32 MB**. All are overridable per body and deployment-wide via `OS_SANDBOX_HOOK_TIMEOUT_MS` / `OS_SANDBOX_ACTION_TIMEOUT_MS` / `OS_SANDBOX_WALL_CEILING_MS`. ### Nested cross-object writes -A body may write *other* objects — e.g. `await ctx.api.object('parent').update({ ... })` from a child's `afterInsert`/`afterUpdate` (requires `api.write`). The target's own hooks fire too: the nested write runs in a **fresh sandbox VM** while the calling body is suspended, and this composes to any depth. This is the natural "when a child changes, roll the total up to the parent" automation — it does **not** need a denormalized, hand-maintained mirror field. The 250ms default suits a single quick write; give a deeper or slower rollup chain headroom by declaring a larger `timeoutMs` on the outer body (up to 30_000ms), so the whole chain settles within budget. +A body may write *other* objects — e.g. `await ctx.api.object('parent').update({ ... })` from a child's `afterInsert`/`afterUpdate` (requires `api.write`). The target's own hooks fire too: the nested write runs in a **fresh sandbox VM** while the calling body is suspended, and this composes to any depth. This is the natural "when a child changes, roll the total up to the parent" automation — it does **not** need a denormalized, hand-maintained mirror field. Because each body's budget is **CPU time** (ADR-0102), the caller is **not** charged for the nested write's own run — so the stock 250ms default comfortably covers deep rollup chains, and you rarely need to raise `timeoutMs` (the spec still permits up to 30_000ms for a genuinely CPU-heavy body). ## L3 — Compiled modules (intentionally disabled)