Skip to content

fix(sandbox): quarantine the workspace when the final checkpoint fails - #145

Open
tee8z wants to merge 1 commit into
mainfrom
tee8z/checkpoint-quarantine
Open

tee8z wants to merge 1 commit into
mainfrom
tee8z/checkpoint-quarantine

Conversation

@tee8z

@tee8z tee8z commented Nov 9, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

When the final checkpoint of a remote-agent Sandbox fails, the reaper holds the session for 24 hours so the workspace can be recovered from the Pod. The workspace is an emptyDir on a spot node, so the hold protects nothing:

Session (2026-11-09, staging) Final save Workspace deleted Recorded as
brandon c55728…-a76cb9c93e11 exit 11 (secret scan) at 14:54 15:07: cluster-autoscaler removed the node as "empty"; it ignores terminated Pods replaced, older turn checkpoint
tee8z 1c425d…-510f13618843 120s timeout at 20:44 21:08: spot interruption drained the node replaced, the checkpoint it was restored from; all session work lost

Behavior

After a failed final save, the harness now stores the workspace without the secret scan. The reaper records that archive and ends the session at once.

flowchart LR
    stop[Harness stops] --> save[buzz-agent-checkpoint save]
    save -->|ok| receipt[termination receipt: source receipt]
    save -->|failed| quarantine["buzz-agent-checkpoint quarantine (at most 90s, ends by stop start + 290s)"]
    quarantine -->|ok| q["S3 prefix/generation/quarantine/uuid.tar.gz"]
    quarantine -->|failed| hold[no archive: 24h hold, as before]
    q --> reaper[reaper tombstones at once: source quarantine]
    reaper --> recover[Recover from checkpoint restores it]
Loading
Area Change
checkpoint.py New quarantine action. It packs like save but runs no secret scan and no bundle validation. It keeps every excluded path out (private runtime files, .env*, PRIVATE_NAMES). A repository without a usable origin is bundled whole. One whose HEAD has no commit (new git init, orphan checkout) is kept as base unborn: its other branches and every file. No repository is dropped.
checkpoint.py restore A key under quarantine/ skips the restore-time scan. The key decides this, not the archive's manifest. A repository without a usable origin keeps its remote-tracking refs; only the temporary URL is cleared. A verified restore still refuses an empty origin or an unborn record.
buzz-acp final_save runs the quarantine only after a failed final save, never after a turn save. It reports checkpoint_quarantined or checkpoint_quarantine_failed and still exits non-zero. The quarantine gets at most 90s and must end within STOP_BUDGET (290s) of the stop's start, inside the Pod's 300s grace period. With under 10s left it is skipped and reported (reason deadline). The drains before it have no shared bound (Known Defect 7), so it takes the remaining time instead of adding a fixed term.
Reaper New Evidence::Quarantined. Precedence is receipt, quarantine, listed, inherited. A Completed session with a quarantine archive is tombstoned at once; one without an archive still holds. An S3 outage on the quarantine listing defers; it never records missing.
lifecycle.rs CHECKPOINT_SOURCE_QUARANTINE, QUARANTINE_DIR, and is_restorable_key, so a session recovered from a quarantine inherits that key.

Quarantine keys have one more / than session keys, so is_session_key keeps them out of the session listing. Clients need no change: no client reads checkpoint-source, and Recover from checkpoint restores the tombstone key. No infrastructure change: in voltage-terraform v2/environments/staging-agent, the Sandbox role may put and get <developer>/*, the reaper may list the bucket, and the unfiltered 30-day expiration rule covers quarantine archives.

Tradeoff: a quarantine archive can contain a credential that the agent wrote into a repository file; the secret scan exists to refuse exactly that. The archive stays in the developer's own prefix, expires in 30 days, and restores only through explicit recovery. voltage/remote-agents.md now says so.

Validation

  • cargo test -p buzz-acp -p buzz-sandbox-reaper -p buzz-backend-kubernetes: 1126, 143, and 225 passed. cargo clippy --all-targets -D warnings and cargo fmt are clean.
  • Falsifiability: each guard was removed in turn and its test failed (reaper quarantine probe, no-hold planning, acp fallback, inherited quarantine key, stop deadline).
  • New tests:
    • a_failed_final_save_ends_at_once_on_its_quarantine_archive replays the staging incident.
    • quarantine_archives_are_listed_apart_from_session_checkpoints covers listing, nested keys, foreign generations, and an outage.
    • a_failed_final_save_quarantines_the_workspace covers save ok, quarantine ok, quarantine failure, and a spent stop deadline.
    • checkpoint_stop.rs runs the harness binary: a failed save is followed by exactly one quarantine on the post-ready and startup-cancel stop paths, and a successful save by none.
  • test-checkpoint-storage.py (image, moto S3) now covers: SIGTERM, then a final save that the secret scan refuses, then a quarantine, then recovery that restores the leaked file, a repository without an origin, and a repository without a commit, but never .env. The same archive stored under an ordinary key is refused, because only a quarantine/ key skips the verified restore.
  • test-checkpoint.py, test-checkpoint-shallow.py, test-checkpoint-storage.py, and test-restore.py pass against a local build of this commit's agent image. CI runs them again on publish.

Limits

  • When the drains before the checkpoint overrun, the quarantine gets less time or is skipped (deadline). A spot interruption gives about 2 minutes, which a slow save can use up.
  • The quarantine still fails on a special file, an external symlink, or the 256 MiB and 50,000-file budgets. The session then falls back to the 24-hour hold, as before this change.
  • Every Sandbox of a developer can write under that developer's prefix, so a quarantine key is only as trustworthy as those Sandboxes. This matches the existing listed source.
  • A restored unborn repository has its files unstaged.
  • A session recovered from a quarantine still contains what the scan refused, so its turn saves keep failing until it is removed. Each later stop quarantines again.
  • A Completed session that quarantined no longer emits CheckpointMissing. The failed save shows as (source: quarantine) in SessionEnded and as checkpoint_failed in activity. There is no separate metric.
  • The 120s timeout of the verified save itself, and its missing step name on timeout, are not changed here.

Agent review

Two independent agent reviews ran.

  • Blocker (fixed): the first revision dropped repositories with no commit, and the reaper then ended the session at once. The unborn base keeps them, and the image test covers it.
  • Applied:
    • The stop deadline replaces a fixed-sum budget that ignored drain overruns.
    • A test proves an ordinary key takes the verified restore.
    • Restores keep remote-tracking refs.
    • Stale text is corrected.
    • The trust boundary and the recovered-session behavior are documented.
  • Declined:
    • Skipping unpackable entries would make the archive incomplete while the reaper treats it as final, so the quarantine fails and the hold applies instead.
    • A separate quarantine metric is left for later.
    • A quarantine after a save that uploaded but failed receipt validation is accepted as a rare edge case.

Human test

Prerequisite: staging runs this branch's agent and reaper images. Pins live in voltagecloud/cd (manager, gate) and voltage-terraform (worker).

  1. In a Sandbox thread, ask the agent to write github_token = "ghp_aB3dE6gH9jK2mN5pQ8sT1vW4yZ7cF0iL3oR6" into leak.txt in a cloned repository and change another file.
  2. Select End session.
  3. Expect the reaper's SessionEnded Event to show (source: quarantine) within one pass, and no CheckpointMissing Event:
    kubectl --context voltage-staging -n buzz-agent-staging-<you> get events --field-selector reason=SessionEnded
  4. Select Recover from checkpoint. Expect both files back in the new Sandbox.

Summary by CodeRabbit

  • New Features
    • If a final workspace checkpoint fails, the system can save a quarantine archive within the shutdown time limit. The archive can be used to recover work, including when no verified checkpoint is available.
    • Completed sessions with a quarantine archive can proceed without the usual 24-hour hold.
  • Documentation
    • Updated recovery guidance to explain quarantine archives, their limits, and how restoring them differs from restoring a regular checkpoint.

A session whose final save failed was held for 24 hours so its workspace
could be recovered from the Pod. The workspace is an emptyDir on a spot
node, so a spot interruption or a cluster-autoscaler scale-down (which
ignores terminated Pods) deleted it within minutes. On 2026-11-09 two
staging sessions lost their work this way.

After a failed final save, buzz-acp now runs `buzz-agent-checkpoint
quarantine`, which stores the workspace without the secret scan under
`<prefix><generation>/quarantine/`. Excluded credential paths stay out,
and no repository is dropped: one without a commit is kept as base
`unborn`. The quarantine gets at most 90s and ends within 290s of the
stop's start, inside the Pod's 300s grace period.

The reaper records that archive (checkpoint-source `quarantine`) ahead of
a listed turn checkpoint and tombstones the session at once instead of
holding it. Recover from checkpoint restores it, skipping the
restore-time scan only for quarantine keys.

Signed-off-by: Tee8z <tee8z@protonmail.com>
@tee8z tee8z self-assigned this Nov 9, 2026
@coderabbitai

coderabbitai Bot commented Nov 9, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Failed final checkpoint saves now trigger a deadline-bounded quarantine attempt. The checkpoint helper creates and restores quarantine archives, and the reaper can use quarantine evidence when reconciling completed sessions.

Changes: Workspace quarantine flow

Layer / File(s) Summary
Quarantine archive and recovery
voltage/images/buzz-agent/checkpoint.py, voltage/images/buzz-agent/test-checkpoint-storage.py, voltage/images/buzz-agent/README.md, voltage/remote-agents.md, docs/remote-agents.md
The checkpoint helper can upload unscanned quarantine archives and restore them based on the key path. The archive flow supports repositories without a usable origin or an existing commit. Tests and documentation describe archive contents, exclusions, and recovery.
Deadline-bounded final-save fallback
crates/buzz-acp/src/checkpoint.rs, crates/buzz-acp/src/lib.rs, crates/buzz-acp/src/activity.rs, crates/buzz-acp/tests/checkpoint_stop.rs, crates/buzz-backend-kubernetes/src/config.rs, docs/remote-agents.md
The ACP stop path passes deadlines to checkpoint saves. A failed final save triggers a quarantine attempt within the remaining stop budget. Activity events record quarantine outcomes, while the original save result remains the returned result.
Quarantine evidence and reconciliation
crates/buzz-backend-kubernetes/src/lifecycle.rs, crates/buzz-sandbox-reaper/src/evidence.rs, crates/buzz-sandbox-reaper/src/plan.rs, crates/buzz-sandbox-reaper/src/reconcile.rs, crates/buzz-sandbox-reaper/src/reconcile_tests.rs, docs/remote-agents.md
The reaper checks quarantine evidence when receipt evidence is missing, before falling back to listed checkpoints. Completed sessions with quarantine evidence can be tombstoned immediately with the quarantine key and source recorded.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ACP shutdown
  participant checkpoint.py CLI
  participant S3 checkpoint prefix
  participant Sandbox reaper
  ACP shutdown->>checkpoint.py CLI: Run final save with stop deadline
  checkpoint.py CLI->>checkpoint.py CLI: Run quarantine after save failure
  checkpoint.py CLI->>S3 checkpoint prefix: Upload quarantine archive
  Sandbox reaper->>S3 checkpoint prefix: List quarantine prefix
  S3 checkpoint prefix-->>Sandbox reaper: Return quarantine key
  Sandbox reaper->>Sandbox reaper: Record quarantine evidence
Loading

Possibly related PRs

  • voltagecloud/buzz#11: Adds the ACP stop-time checkpointing path that this change extends with deadlines and quarantine outcomes.
  • voltagecloud/buzz#6: Adds turn checkpoint saves and reaper recovery from listed checkpoints, which this change extends with quarantine evidence.
  • voltagecloud/buzz#78: Changes checkpoint secret scanning and failure reporting; this change adds an unscanned quarantine path that can follow a failed save.

Suggested labels: human review wanted

Merge Risk: Minimal · up to 60a59

The change adds a deadline-bounded quarantine fallback for failed final checkpoint saves and lets the reaper use that archive as checkpoint evidence. No actionable merge-blocking risk was identified, though the image tests were not independently run here.

Pre-merge checks: Passed 4 · Inconclusive 1

Failed Failed checks: 1 inconclusive

Check name Status Explanation Resolution
Docstring Coverage Inconclusive Docstring coverage is 59.09% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly summarizes the main change: quarantine the workspace when the final checkpoint fails.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 59.09% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 22 functions across 11 files. (4 skipped: 3 unsupported, 1 too large.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Generate docstrings

  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between ebb1f6a and 60a591d.

Files selected for processing: 15

  • crates/buzz-acp/src/activity.rs
  • crates/buzz-acp/src/checkpoint.rs
  • crates/buzz-acp/src/lib.rs
  • crates/buzz-acp/tests/checkpoint_stop.rs
  • crates/buzz-backend-kubernetes/src/config.rs
  • crates/buzz-backend-kubernetes/src/lifecycle.rs
  • crates/buzz-sandbox-reaper/src/evidence.rs
  • crates/buzz-sandbox-reaper/src/plan.rs
  • crates/buzz-sandbox-reaper/src/reconcile.rs
  • crates/buzz-sandbox-reaper/src/reconcile_tests.rs
  • docs/remote-agents.md
  • voltage/images/buzz-agent/README.md
  • voltage/images/buzz-agent/checkpoint.py
  • voltage/images/buzz-agent/test-checkpoint-storage.py
  • voltage/remote-agents.md

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour.

@tee8z
tee8z marked this pull request as ready for review November 10, 2026 13:23
@tee8z

tee8z commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator Author

Takeover runtime recheck of the existing checkpoint implementation: the packaged buzz-agent:quarantine helper hash exactly matches this branch's checkpoint.py (046fc60072af65eec20262114cc3662ac67499a077c687a2c347f8614b7e33ea).

Reran python3 voltage/images/buzz-agent/test-checkpoint-storage.py buzz-agent:quarantine against an isolated internal Docker network and Moto S3. It passes verified upload/checksum/recovery, foreign-prefix refusal, real harness SIGTERM/drain/final-edit preservation, failed-upload nonzero exit, scanner refusal followed by quarantine, recovery of no-origin and unborn repositories, .env exclusion, and refusal of the same unscanned archive under an ordinary key. Test containers/network were removed by the runner.

This adds runtime evidence to the prior reviews. It does not claim a staging rollout or human acceptance. The security dependency update and accepted temporary SQLite policy exception are prepared in #155.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant