Repository navigation
feat: isolate browser code execution, add mockable runtime tests, harden idempotency and prod deploy - #25
Merged
Conversation
…den idempotency and production deploy - Isolate browser_run_playwright_code (previously raw new Function) through a new same-process node:vm sandbox (runVmSandboxedCode in plugin-sandbox.ts), matching custom_code's no-require/process/Bun guarantee where a live Page handle rules out the subprocess path. - Add a swappable streamChatImpl seam in agent-runtime.ts so agent-runtime reliability tests run against a scripted fake model instead of a real provider: planning -> execution -> completion, tool-call event persistence, approval pause/approve/reject, autonomy-budget pause, transient retry -> success, retry-exhausted -> dead_letter, non-transient failure, cancellation, and delegation depth/cycle limits. - Close a TOCTOU race in approval resolution with an atomic claimApprovalRequest CAS, and add a core-agent-engine run-transitions guard so a stale write can't revive an already-terminal agent run. - Add tasks.retry (backed by an atomic claimTaskForRetry CAS) plus a Retry action and dead_letter status handling on the task detail/runs pages. - Harden production deploys: non-root Docker users with a writable SQLite volume, HSTS/Permissions-Policy on Caddy, and a working (non-license-gated) secret-scan workflow. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CxqiVNBG8nbxwBQpWAJcZD
…leaks Bun doesn't run npm postinstall scripts by default, so playwright's own browser download never ran in CI, which broke the new browser.test.ts. Also allowlist the two files whose secret-strength tests intentionally assign secret-shaped placeholder/synthetic values to env vars, which gitleaks' first real run (post license-gate fix) correctly flagged as generic-api-key matches but are not real secrets. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CxqiVNBG8nbxwBQpWAJcZD
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
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.
Summary
Production-hardening pass across five areas:
browser_run_playwright_code(apps/server/src/tools-builtin/browser.ts) previously ran arbitrary user code via a rawnew Function("page", ...)in-process — fullprocess.env/require/Bunaccess, same threat model as pre-ADR-0007custom_code. It now runs through a newrunVmSandboxedCodeprimitive inplugin-sandbox.ts: a same-processnode:vmcontext whose only global ispage, with norequire/process/Bun/ambientfetchreachable, a timeout, and a capped/structured error — the same guaranteesrunIsolatedCustomCode's subprocess givescustom_code, minus the process boundary (a live PlaywrightPagehandle can't cross a process boundary the waycustom_code's serializablectxdoes; see the doc comment onrunVmSandboxedCodefor the exact trade-off).agent-runtime.tsnow calls every model request through a single swappablestreamChatImplreference (defaults to the realstreamChat), with test-only setters.agent-runtime.test.tsadds a scripted-model harness proving: planning → direct execution → completion, a real tool call persisted to the task-event timeline, approval pause → approve/reject (using a realfile_deletetool, not a simulated approval), an autonomy-budget pause mid-run, transient retry → success, retry-exhausted →dead_letter, a non-transient failure with no retry, cancellation mid-model-call, and delegation depth-limit/cycle-prevention. None of these hit a real local/cloud model.resolveApprovalDecisionwith an atomicclaimApprovalRequestCAS (a single conditionalUPDATE ... WHERE status = 'pending', not check-then-write), so two concurrent approve/reject calls for the same approval can't both execute the underlying action. Addedpackages/core-agent-engine/src/run-transitions.ts(assertAgentRunTransitionAllowed/isTerminalAgentRunStatus) and wired it intoapprovals.tsandagent-runtime.ts's completion write so a stale/racing write can't revive an already-terminal run.tasks.retry(backed by an atomicclaimTaskForRetryCAS, same pattern as the approval CAS) plus a "Retry" button and a dedicated failed-state callout on the task detail page. Fixed a real gap: the frontend'sAgentRunStatustype was missingdead_letterentirely, so it had no badge color anywhere it's rendered (task detail, agent detail, the cross-cutting Runs board) — now added everywhere. (The pending-approval nav badge already existed; no change needed there.)USER bunin both Dockerfiles (confirmed against the upstreamoven/bunimage source that this user exists) with the SQLite volume mount pre-chowned so it stays writable; HSTS +Permissions-Policyadded toCaddyfile; and the secret-scan workflow was actually broken —gitleaks/gitleaks-action@v2failed on every single run (12/12) with a missing-license error unrelated to any real finding, so it never completed a scan whilecontinue-on-error: truesilently kept it green. Replaced with a direct OSSgitleaksCLI download (no license gate), kept non-blocking until this fixed version's first real run is triaged.Test plan
bun install --frozen-lockfilebunx biome check .(clean on every file touched/added this session; pre-existing repo-wide formatting drift onmainis unrelated and unchanged)bun run typecheck— 0 errors across all 9 packagesbun run build— server + web build cleanbun test— 342/342 pass (30 new: agent-runtime reliability, plugin-sandbox/browser isolation, run-transitions, task-retry CAS)docker compose -f docker-compose.pc.yml config/docker-compose.server.yml config— both parse cleanlyfailed/dead_lettertask, drove the task detail page with a real browser session, clicked Retry, and confirmed via direct DB inspection that a new run was created and the atomic claim/re-execution flow behaved correctly (screenshots taken, not attached)🤖 Generated with Claude Code
https://claude.ai/code/session_01CxqiVNBG8nbxwBQpWAJcZD
Generated by Claude Code