fix(runtime): sandbox budget = script CPU-time, not wall clock (ADR-0102 D1, #3295)#3301
Merged
Merged
Conversation
…102 D1, #3295) 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011iJLjqToxNv1aYP3syQtRp
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 17 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
… 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011iJLjqToxNv1aYP3syQtRp
os-zhuang
marked this pull request as ready for review
July 19, 2026 16:59
This was referenced Jul 19, 2026
Merged
os-zhuang
added a commit
that referenced
this pull request
Jul 19, 2026
…ation (#3297) (#3318) Phase 3 of #3275. Records ADR-0102 D3 as deferred: verifying quickjs-emscripten 0.32 shows no clean, portable, edge-safe way to obtain the sync variant's .wasm bytes to precompile (Node fs/fetch breaks Workers/edge-safety; replicating locateFile is fragile), and the payoff is uncertain (engines already code-cache wasm compilation). Phase 2 (#3309) already delivered the dominant win. D1 (#3301) + D2 (#3309) stand on their own; revisit criterion recorded. allow-major label bypasses the unrelated pre-existing console:major changeset on main. Closes #3297.
os-zhuang
pushed a commit
that referenced
this pull request
Jul 20, 2026
… ADR-0102 D1 #3270 raised the sandbox hook budget workflow-wide to paper over the #3259 load flake: on an oversubscribed runner the fixed per-invocation WASM-creation cost alone tripped the 250ms *wall-clock* hook deadline. ADR-0102 D1 (#3301) changed the budget to *script CPU-time* — VM creation and idle host-await time are no longer charged — so the 250ms default is meaningful again on a loaded runner and this global floor is no longer needed. Removing it also validates the fix end-to-end: the sandbox suite (incl. the nested-write integration tests, which run at the stock default) must stay green at 250ms on CI. Tests that assert a specific budget set it explicitly and are unaffected; production always kept 250ms/5000ms. The env override remains available for constrained hardware. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011iJLjqToxNv1aYP3syQtRp
os-zhuang
added a commit
that referenced
this pull request
Jul 20, 2026
… ADR-0102 D1 (#3328) #3270 raised the sandbox hook budget workflow-wide to paper over the #3259 load flake: on an oversubscribed runner the fixed per-invocation WASM-creation cost alone tripped the 250ms *wall-clock* hook deadline. ADR-0102 D1 (#3301) changed the budget to *script CPU-time* — VM creation and idle host-await time are no longer charged — so the 250ms default is meaningful again on a loaded runner and this global floor is no longer needed. Removing it also validates the fix end-to-end: the sandbox suite (incl. the nested-write integration tests, which run at the stock default) must stay green at 250ms on CI. Tests that assert a specific budget set it explicitly and are unaffected; production always kept 250ms/5000ms. The env override remains available for constrained hardware. Claude-Session: https://claude.ai/code/session_011iJLjqToxNv1aYP3syQtRp Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements Phase 1 of #3275 (ADR-0102 D1). Closes #3295.
What & why
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 (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. That was the root cause of the #3259 flake, and it also fixes the "nested hooks billed to the parent" problem. A separate, generous wall-clock ceiling (default 30s,
max(ceiling, cpuBudget)) backstops a body stuck on a host call that never settles.Changes
quickjs-runner.ts): slice-stopwatch every VM entry (evalCode/evalCodeAsync/ eachexecutePendingJobs). The interrupt handler and the post-slice checks comparecpuMsagainst the CPU budget andDate.now()against the wall deadline. Distinct errors:exceeded CPU budget of Nmsvsexceeded wall-clock ceiling of Nms while awaiting host calls. A mid-slice interrupt — which for an async body surfaces via__error, notpending.error— is mapped to the clean budget message at all three error sites.QuickJSScriptRunneroptionwallCeilingMs+ envOS_SANDBOX_WALL_CEILING_MS;resolveSandboxTimeoutMs(@objectstack/types) gains a'wallCeiling'kind. Precedence unchanged: explicit option › env › built-in default.sliceStarttostart, not0. A0init madenow - sliceStartthe full unix-epoch millis, firing the interrupt duringinstallCtxand corrupting ctx marshalling (__inputnever set) — this would have mis-fired in production, not just tests./CPU budget of Nms/; genuine stuck-host cases → dedicated small-wallCeilingMsrunners asserting/wall-clock ceiling/; added the Flaky:nested-write.integration.test.tssandbox hook exceeds the 250ms deadline on CI (Test Core) #3259 root-cause guard (idle host time resolves) and CPU-vs-ceiling coverage;resolveSandboxTimeoutMswall-ceiling unit test.nested-write*.integration: dropped the explicit 10s budget — they now pass at the stock 250ms, which is the regression guard for the nested-charging fix.environment-variables.mdxCPU-time wording +OS_SANDBOX_WALL_CEILING_MS; ADR-0102 D1 → Accepted. Changeset carries the FROM→TO semantics note.Behaviour change (see changeset)
Knob names, defaults (250ms/5000ms), and precedence are unchanged — only the dimension (wall → CPU) and the error strings change (
exceeded timeout of Nms→CPU budget/wall-clock ceiling). In practice this only loosens legitimate slow/nested work; a runaway synchronous script is still cut at the same budget.Verification
@objectstack/runtimefull suite — 574 passed (incl. the sandbox suite in ~5s with no hangs, and all production-runner consumers: app-plugin, dispatcher).@objectstack/types— 26 passed.eslint,check:doc-authoring, and@objectstack/types+@objectstack/runtimebuilds (tsup DTS typecheck) all green.Follow-ups (separate PRs, per ADR)
OS_SANDBOX_HOOK_TIMEOUT_MS=10000from.github/workflows/ci.yml— the CPU budget makes the 250ms default meaningful on loaded runners again.🤖 Generated with Claude Code
Generated by Claude Code