feat(codexrunner): carry the Codex rollout in durable checkpoints - #179
Conversation
A checkpoint held only a thread ID and a model. That is enough to resume on the box that produced it, because "codex exec resume" resolves a thread from the local Codex session store — and enough for nothing at all on a replacement sandbox, where that store is empty and the thread ID names a rollout no process can reach. Checkpoint now carries RolloutPath, the local rollout backing its thread, so a host can upload it and restore it into a rebuilt sandbox before resuming. The package locates the file and stops there: it never reads, ships, or logs a rollout, and it learns nothing about artifact storage. Options.Rollouts selects the locator; CodexHomeLocator resolves $CODEX_HOME, then $HOME/.codex. Adding the field is not sufficient on its own — two sites rebuilt Checkpoint as a literal and would have dropped it: - Runner.handleEvent emitted the sink checkpoint from the thread ID and model, so Result never saw the rollout it had just located. - Continuity.Run adopted the post-turn checkpoint from Result's thread ID and model, zeroing the rollout on every turn after the first. Both now carry it, and Result grew the mirroring field so a caller adopting a Result as durable state keeps what the sink already received. Continuity.Restore validates the rollout the way it already validates the thread ID and the model: a relative path is refused as ErrInvalidRolloutPath, and a rollout absent from this box as ErrRolloutUnavailable, without installing the checkpoint. That second error is the sandbox handover made loud — callers recover by reporting the reason and restoring with RolloutPath cleared, which starts a fresh thread instead of resuming into an empty one that looks live. Digest verification stays with the host: the checkpoint struct records no hash, and the contract's rollout_ref carries sha256 alongside the reference. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014apoNDacRcYKGdxtLaauu1
Sigilix OverviewEffort: 3/5 (medium) Quality gates
Summary — latest pushExtends the Important files
Confidence: 4/5The change is well-scoped with comprehensive test coverage including mutation-checked regression tests, and the validation logic correctly prevents silent data loss on sandbox handover.
Suggested labels:
|
![qualitymaxapp[bot]](https://avatars.githubusercontent.com/in/2748338?s=60&v=4){kind=small}
There was a problem hiding this comment.
QualityMax Review — canonical overview updated; inline findings are attached to this review.
😸 1 finding(s) auto-dismissed by the Finding Verifier
llm-path-traversalcodexrunner/rollout_test.go:10 — false positive: The code is in a test file using hardcoded test constants, not actual user-controlled input, so it poses no production risk.
Auto-dismissals are advisory. If a dismissal looks wrong, treat the finding as blocking and request a verifier review.
| "os" | ||
| "path/filepath" | ||
| "testing" | ||
| ) |
There was a problem hiding this comment.
P3 ·
insecure-configuration· MODEL-ONLY
File permissions 0o700 for directory creation are overly permissive (owner read/write/execute), which could expose sensitive session data
Evidence: codexrunner/rollout_test.go:9
Impact / next step: Use more restrictive permissions like 0o750 (owner rwx, group rx) or 0o700 only if absolutely necessary. Consider the principle of least privilege for file permissions.
Detailed reasoning
- Root cause: File permissions 0o700 for directory creation are overly permissive (owner read/write/execute), which could expose sensitive session data
- Threat model: Review callers that can reach
codexrunner/rollout_test.go:9. - Existing protection: Reported by llm; confidence: medium.
- Alternatives considered: Preserve the current interface; prefer the smallest safe change.
- Severity calibration: P3 based on the scanner severity.
Prompt to fix with AI
Fix this insecure-configuration finding in codexrunner/rollout_test.go:9. Evidence: File permissions 0o700 for directory creation are overly permissive (owner read/write/execute), which could expose sensitive session data Requested outcome: Use more restrictive permissions like 0o750 (owner rwx, group rx) or 0o700 only if absolutely necessary. Consider the principle of least privilege for file permissions.. Preserve existing behavior and add focused coverage.
QualityMax · proof: MODEL-ONLY · served model: mistral-large-latest · requested: gemini-3.1-pro-preview
|
|
||
| func writeRollout(t *testing.T, home, threadID string) string { | ||
| t.Helper() | ||
| directory := filepath.Join(home, "sessions", "2026", "08", "27") |
There was a problem hiding this comment.
P3 ·
insecure-configuration· MODEL-ONLY
File permissions 0o600 for rollout file creation are correct but should be documented as a security requirement
Evidence: codexrunner/rollout_test.go:13
Impact / next step: While 0o600 is appropriate (owner read/write), add comments explaining the security rationale for these permissions to prevent future changes that might weaken security.
Detailed reasoning
- Root cause: File permissions 0o600 for rollout file creation are correct but should be documented as a security requirement
- Threat model: Review callers that can reach
codexrunner/rollout_test.go:13. - Existing protection: Reported by llm; confidence: medium.
- Alternatives considered: Preserve the current interface; prefer the smallest safe change.
- Severity calibration: P3 based on the scanner severity.
Prompt to fix with AI
Fix this insecure-configuration finding in codexrunner/rollout_test.go:13. Evidence: File permissions 0o600 for rollout file creation are correct but should be documented as a security requirement Requested outcome: While 0o600 is appropriate (owner read/write), add comments explaining the security rationale for these permissions to prevent future changes that might weaken security.. Preserve existing behavior and add focused coverage.
QualityMax · proof: MODEL-ONLY · served model: mistral-large-latest · requested: gemini-3.1-pro-preview
QualityMax ReviewVerdict: COMMENT · Confidence: evidence-backed scan Files eligible: 6 · Files reviewed: 6 · Files with findings: 1 · Findings: 2 · Inline cards: 2 Priority findings
Review gates
Important files
Change diagram — Flowflowchart TD
A[Continuity.Run] -->|thread.started| B[Checkpoint Update]
B --> C[RolloutLocator.LocateRollout]
C -->|path| D[Checkpoint.RolloutPath]
D --> E[Continuity.Restore]
E --> F{validateRolloutPath}
F -->|valid| G[Install Checkpoint]
F -->|invalid| H[ErrRolloutUnavailable]
H --> I[Clear RolloutPath]
I --> J[Start Fresh Thread]
Review lifecycleUse the inline cards to inspect evidence and suggested remediation. Re-run the QualityMax review after pushing a fix; unchanged cards are identified by their stable finding marker. Dismiss with a reason through the existing QualityMax/GitHub review feedback flow. 0 prior card(s) are stale/resolved on this head. Proof legend: VERIFIED independently judged patch · REPRODUCED verified finding · GROUNDED deterministic evidence · MODEL-ONLY model judgment. QualityMax project results are available in the configured project. Receipt · commit |
![sigilix[bot]](https://avatars.githubusercontent.com/in/3558711?s=60&v=4){kind=small}
|
| Gate | Result |
|---|---|
| 🔍 AI diff review | ✅ Clean · mistral-large-latest · completed · 5 eligible / 4 reviewed · mistral-large-latest |
| 🔍 SAST | completed · 6 eligible / 6 reviewed · mistral-large-latest |
| 🔍 Canonical PR review delivery | completed · 0 eligible / 0 reviewed · exact-head review #5040525808 and overview #5438808909 confirmed |
| 🧪 Repo Tests | ✅ 675/675 passed (go) |
| 🤖 AI Tests |
Powered by QualityMax — AI-Powered Test Automation
2 participants
Implements Ask A of the Cross-Sandbox Continuity handoff: let a checkpoint carry the rollout, not just its id.
The problem
Checkpointheld a thread ID and a model. That is enough to resume on the box that produced it, becausecodex exec resumeresolves a thread from the local Codex session store — and enough for nothing at all on a replacement sandbox, where that store is empty and the thread ID names a rollout no process can reach.What changed
CheckpointgainsRolloutPath, the local rollout backing its thread, so a host can upload it and restore it into a rebuilt sandbox before resuming. The package locates the file and stops there: it never reads, ships, or logs a rollout, and it learns nothing about artifact storage.Options.Rolloutsselects the locator;CodexHomeLocatorresolves$CODEX_HOME, then$HOME/.codex, and matchessessions/<yyyy>/<mm>/<dd>/rollout-*-<thread_id>.jsonl.Adding the field is not sufficient on its own. Two sites rebuilt
Checkpointas a literal and would have dropped it silently:Runner.handleEvent(runner.go:324) emitted the sink checkpoint from the thread ID and model, soResultnever saw the rollout it had just located.Continuity.Run(continuity.go:51) adopted the post-turn checkpoint fromResult's thread ID and model, zeroing the rollout on every turn after the first.Both now carry it, and
Resultgrew the mirroring field so a caller adopting aResultas durable state keeps what the sink already received.Continuity.Restorevalidates the rollout the way it already validates the thread ID and the model:RolloutPathErrInvalidRolloutPath, not installedErrRolloutUnavailable, not installedErrRolloutUnavailableis the sandbox handover made loud. Callers recover by reporting the reason and restoring withRolloutPathcleared, which starts a fresh thread rather than resuming into an empty one that looks live.Scope
Digest verification stays with the host: the checkpoint struct records no hash, and the contract's
rollout_refcarriessha256alongside the reference. Asks B (rehydrate$CODEX_HOMEbefore resuming) and C (snapshot the working tree) land on the worker and are untouched here.Testing
go build ./... && go vet ./... && go test ./...all pass.New
codexrunner/rollout_test.gocovers the locator (exact-thread match, unknown thread, non-canonical thread ID, absent store,$CODEX_HOME), the checkpoint sink receiving the path atthread.started, a turn succeeding when no rollout is locatable, retention across a resumed turn, all threeRestoreoutcomes, and the documented fresh-thread recovery.Both regression tests were mutation-checked: reverting the two literal sites and the
Restorevalidation failsTestContinuityRetainsRolloutAcrossTurnsandTestContinuityRestoreValidatesRolloutrespectively, and nothing else.Generated by Claude Code