Skip to content

fix(runtime): env-overridable sandbox hook/action timeout to de-flake CI (#3259)#3270

Merged
os-zhuang merged 2 commits into
mainfrom
claude/nested-write-sandbox-timeout-muw13i
Jul 19, 2026
Merged

fix(runtime): env-overridable sandbox hook/action timeout to de-flake CI (#3259)#3270
os-zhuang merged 2 commits into
mainfrom
claude/nested-write-sandbox-timeout-muw13i

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Closes #3259.

Problem

The QuickJS sandbox enforces a wall-clock deadline per hook/action invocation (250ms hooks / 5000ms actions). Every invocation compiles a fresh WASM module, and a nested hook compiles another one inside the parent's budget — so on an oversubscribed CI runner (4-vCPU, turbo --concurrency=4 + vitest workers) that fixed VM-creation cost alone intermittently tripped the 250ms hook default even while the VM was still making progress. It surfaced as hook '…' exceeded timeout of 250ms on PRs that never touched the sandbox path (the variable pump count — 215 vs 246 — before the same wall-clock deadline is the signature of a load flake, not a logic bug).

9498eb4 (#3264) bumped the individually-named tests to a 10s budget, but that's whack-a-mole: the shared 250ms floor kept resurfacing on other tests (quickjs-runner 'lvl4', per the issue comment), and the production runners in app-plugin.ts still use the stock default with no way to tune it.

Fix

Implements the issue's suggested direction #1 — make the deadline env-overridable (OS_SANDBOX_*, per the OS_{DOMAIN}_{NAME} convention / Prime Directive #9) — so the floor is raised once, deployment-wide, instead of per call site.

  • @objectstack/types — new resolveSandboxTimeoutMs(kind, fallback) (the repo's "single decision point" env pattern):
    • OS_SANDBOX_HOOK_TIMEOUT_MS — default hook budget (ms)
    • OS_SANDBOX_ACTION_TIMEOUT_MS — default action budget (ms)
    • Only a positive integer is honored; unset / empty / non-numeric / non-positive keeps the built-in fallback, so behaviour is byte-for-byte unchanged when the vars are absent.
  • @objectstack/runtimeQuickJSScriptRunner resolves its hook/action defaults through it. Precedence unchanged: explicit constructor option → env → built-in default; and a body's own declared timeoutMs still wins over the resolved default. This automatically covers the four production runners in app-plugin.ts.
  • CIOS_SANDBOX_HOOK_TIMEOUT_MS=10000 set for the whole workflow so the loaded-runner flake can't recur across any sandbox test (present or future). Genuine hangs stay bounded by each test's own vitest timeout. Production keeps 250ms/5000ms unless it opts in.
  • Tests — pinned the quickjs-runner timeout-resolution suite's runner to an explicit 250ms so its assertions are independent of the CI env var; added a dedicated env-override suite + resolveSandboxTimeoutMs unit tests.
  • Docs — corrected the QuickJSScriptRunner class docstring's stale "we pool runtimes" claim (Prime Directive chore: version packages #10, declared ≠ enforced): runtimes are deliberately per-invocation — precisely the VM-creation cost this flake is about.

Verification

  • @objectstack/types env tests — 25 passed
  • @objectstack/runtime sandbox suite — 54 passed, run both with and with OS_SANDBOX_HOOK_TIMEOUT_MS=10000 set (simulating CI); the pinned/env-managed tests behave identically either way
  • eslint clean; @objectstack/types and @objectstack/runtime builds (tsup DTS typecheck) green

Out of scope (already noted in the issue): the separate @objectstack/http-conformance vitest-worker EPIPE transient (direction #3).

🤖 Generated with Claude Code


Generated by Claude Code

… CI (#3259)

The QuickJS sandbox enforces a wall-clock deadline per hook/action invocation
(250ms hooks / 5000ms actions). Every invocation compiles a fresh WASM module,
and a nested hook compiles another inside the parent's budget, so on an
oversubscribed CI runner that fixed VM-creation cost alone intermittently
tripped the 250ms hook default even while the VM was still progressing — a load
flake ("hook '…' exceeded timeout of 250ms") on PRs unrelated to the sandbox.
9498eb4 bumped the individually-named tests to 10s, but the shared 250ms floor
kept resurfacing on other tests (quickjs-runner 'lvl4'), and the production
runners in app-plugin.ts still use the stock default.

Make the per-invocation timeout DEFAULT env-resolvable so the floor can be
raised once, deployment-wide, instead of whack-a-mole per call site:

- types: add resolveSandboxTimeoutMs(kind, fallback) — reads
  OS_SANDBOX_HOOK_TIMEOUT_MS / OS_SANDBOX_ACTION_TIMEOUT_MS (OS_{DOMAIN}_{NAME},
  DOMAIN=SANDBOX). Only a positive integer is honored; unset / empty / invalid
  keeps the built-in fallback, so behaviour is unchanged when absent.
- runtime: QuickJSScriptRunner resolves its hook/action defaults through it.
  Precedence unchanged: explicit constructor option > env > built-in default,
  and a body's own declared timeoutMs still wins over the resolved default.
- ci: set OS_SANDBOX_HOOK_TIMEOUT_MS=10000 for the whole workflow so the
  loaded-runner flake can't recur; genuine hangs stay bounded by vitest timeouts.
- quickjs-runner.test: pin the timeout-resolution suite's runner to an explicit
  250ms so its assertions are independent of the CI env var; add a dedicated
  env-override suite. Add resolveSandboxTimeoutMs unit tests.
- docs: correct the class docstring's stale "we pool runtimes" claim — runtimes
  are deliberately per-invocation (the very VM-creation cost this flake is about).

Production keeps the 250ms / 5000ms defaults unless it opts in.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011iJLjqToxNv1aYP3syQtRp
@vercel

vercel Bot commented Jul 19, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
spec Ready Ready Preview, Comment Jul 19, 2026 11:40am

Request Review

@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation ci/cd tests tooling and removed size/m labels Jul 19, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/runtime, @objectstack/types.

16 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/api/index.mdx (via @objectstack/runtime)
  • content/docs/api/wire-format.mdx (via @objectstack/runtime)
  • content/docs/automation/hook-bodies.mdx (via @objectstack/runtime)
  • content/docs/concepts/north-star.mdx (via packages/runtime)
  • content/docs/data-modeling/drivers.mdx (via @objectstack/runtime)
  • content/docs/deployment/index.mdx (via @objectstack/runtime)
  • content/docs/deployment/production-readiness.mdx (via @objectstack/runtime)
  • content/docs/deployment/single-project-mode.mdx (via @objectstack/runtime)
  • content/docs/deployment/vercel.mdx (via @objectstack/runtime)
  • content/docs/getting-started/your-first-project.mdx (via @objectstack/runtime)
  • content/docs/permissions/authentication.mdx (via @objectstack/runtime)
  • content/docs/plugins/packages.mdx (via @objectstack/runtime, @objectstack/types)
  • content/docs/protocol/kernel/http-protocol.mdx (via @objectstack/runtime)
  • content/docs/protocol/kernel/index.mdx (via @objectstack/runtime)
  • content/docs/protocol/kernel/lifecycle.mdx (via @objectstack/runtime)
  • content/docs/releases/implementation-status.mdx (via @objectstack/runtime)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

…rs (#3259)

Add the two new sandbox timeout knobs to the Tuning (advanced) table in the
canonical environment-variables reference, beside OS_ARTIFACT_FETCH_TIMEOUT_MS
(same positive-integer-only resolution). Operators land here when tuning the
sandbox deadline on a slow/loaded host.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011iJLjqToxNv1aYP3syQtRp

Copy link
Copy Markdown
Contributor Author

按 Prime Directive #10 把长期方案落成了 #3275:Phase 1 改计费语义(脚本 CPU 预算 + 宽松墙钟兜底,解开"容忍慢机器"与"约束失控脚本"共用一个旋钮的矛盾)→ Phase 2 去 asyncify(deferred+pump 重构后已无挂起 WASM 栈的需求,保留每调用独立模块)→ Phase 3 预编译 WebAssembly.Module、每次只实例化;并明确记录了拒绝共享单例模块的安全理由(共享线性内存会把 QuickJS 引擎级 bug 的爆炸半径扩大到跨租户)。

本 PR 保持最小止血不扩 scope;Phase 1 在本 PR 合并后开工。届时 CI 里的 OS_SANDBOX_HOOK_TIMEOUT_MS=10000 地板有望撤掉,env 旋钮保留作慢硬件的运维手段。


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review July 19, 2026 13:05
@os-zhuang
os-zhuang merged commit 32899e6 into main Jul 19, 2026
16 checks passed
@os-zhuang
os-zhuang deleted the claude/nested-write-sandbox-timeout-muw13i branch July 19, 2026 13:05
os-zhuang added a commit that referenced this pull request Jul 19, 2026
…ASM engine (#3275) (#3299)

Adds docs/adr/0102-sandbox-cpu-budget-and-engine-variant.md (Proposed): D1 CPU-time budget + wall ceiling, D2 per-invocation sync WASM modules (drop asyncify), D3 precompile-once (deferrable), D4 shared-module rejected on threat-model grounds. Front-loads the contract for phases #3295/#3296/#3297. Refs #3275, #3259, #3270.
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/cd documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky: nested-write.integration.test.ts sandbox hook exceeds the 250ms deadline on CI (Test Core)

2 participants