fix: use read-only sandbox for Codex reviews with diff file snapshots

[wesm]

Summary

• Revert Codex sandbox from --sandbox danger-full-access back to --sandbox read-only. With full access, Codex was scanning /home, /nix/store, and /root when investigating large diffs.
• For large diffs that don't fit inline in the prompt, the worker writes the full diff to a file in the repo's git dir (resolved via git rev-parse --git-dir) and references the absolute path in the prompt so sandboxed Codex can read it directly.
• CI prebuilt prompts that contain a diff file placeholder are resolved at job time via preparePrebuiltCodexPrompt.
• Exclude patterns are applied consistently to both inline and file-based diffs.
• Codex review prompt now instructs the agent not to search or read files outside the repository checkout.

🤖 Generated with Claude Code

[roborev-ci]

roborev: Combined Review (5b233a6)

Verdict: Changes are not ready to merge; there are 2 medium-severity correctness issues in the new sandboxed Codex diff handling.

Medium

1. Sandboxed prebuilt Codex reviews can fail outright in unsafe/full-access mode
   Location: internal/daemon/worker.go around preparePrebuiltCodexPrompt and prepareDiffFileForCodex
preparePrebuiltCodexPrompt now treats a missing generated diff file as fatal, but prepareDiffFileForCodex intentionally returns no file when agent.AllowUnsafeAgents() is enabled. That makes prebuilt Codex reviews fail instead of falling back to the original git-based prompt in unsafe/full-access mode.

2. Diff snapshot creation failures are silently downgraded into unusable sandboxed reviews
   Location: internal/daemon/worker.go around the new prepareDiffFileForCodex call in processJob
   On the normal review path, errors creating the diff snapshot are converted into diffFile == "" and the code falls back to pb.Build(...). For oversized Codex diffs, that fallback still tells the agent to inspect git state, but the review now runs with --sandbox read-only, so those instructions are not usable. The result can be a broken or no-op review instead of a clean retry/failure path.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (5b233a6)

Summary verdict: 2 medium-severity issues need attention before merge.

Medium

1. Sandboxed Codex reviews can fail outright in unsafe/full-access mode

   • Location: internal/daemon/worker.go around preparePrebuiltCodexPrompt and prepareDiffFileForCodex
   • Issue: Prebuilt Codex prompts now always include the diff-file placeholder, but preparePrebuiltCodexPrompt treats a missing generated diff file as fatal. prepareDiffFileForCodex intentionally returns no file when agent.AllowUnsafeAgents() is enabled, so prebuilt Codex reviews fail instead of falling back to the original git-based prompt.
   • Suggested fix: Only inject the placeholder when the review is actually sandboxed, or allow preparePrebuiltCodexPrompt to preserve the stored prompt when unsafe/full-access mode is active.

2. Diff snapshot failures are silently downgraded into a broken sandboxed fallback

   • Location: internal/daemon/worker.go around the new prepareDiffFileForCodex call in processJob
   • Issue: On the normal review path, failures creating the diff snapshot are converted into diffFile == "", and execution falls back to pb.Build(...). For oversized Codex diffs, that fallback still tells the agent to inspect git state, but the agent is launched with --sandbox read-only, so those instructions are unusable and the review can degrade into a no-op or incomplete analysis instead of failing cleanly.
   • Suggested fix: Propagate diff snapshot creation failures for sandboxed Codex reviews so the job retries/fails explicitly, or retain a full-access fallback when no readable diff snapshot can be prepared.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (7b40338)

Verdict: Changes are directionally correct, but there are 3 medium-severity issues that should be fixed before merge.

Medium

• Stale diff snapshot path can be persisted into retried Codex jobs
  Location: internal/daemon/worker.go:427
  For oversized non-prebuilt Codex reviews, processJob rebuilds the prompt with a concrete snapshot path via BuildWithDiffFile(...) and then deletes that file afterward. If the prompt is persisted for retry/failover, later retries can reuse a prompt that points at a deleted file because preparePrebuiltCodexPrompt only rewrites the placeholder sentinel, not an already-materialized path.
  Fix: Persist a placeholder-based prompt and substitute the real snapshot path only in memory right before invoking Codex, or regenerate the prompt on each retry.

• Dirty-review path does not get the new diff-file fallback
  Location: internal/daemon/worker.go:406
  The new oversized-diff fallback only applies to the git-backed Build(...) path. Jobs using job.DiffContent != nil still go through BuildDirty(...) without snapshot support, so large dirty Codex reviews can still fall back to truncated inline instructions even though review mode now forces --sandbox read-only.
  Fix: Add the same snapshot-file flow for dirty reviews by writing job.DiffContent to a temporary diff file and teaching the dirty prompt builder to reference it when inline diff content overflows.

• git rev-parse --git-dir output may include a trailing newline and break os.Stat
  Location: internal/git/git.go:278
cmd.Output() preserves the trailing newline from git rev-parse --git-dir. Unless normalizeMSYSPath already strips whitespace, passing that through filepath.Clean can leave the newline intact and cause later os.Stat(gitDir) calls to fail with ENOENT.
  Fix: Trim the command output before normalization/cleaning, e.g. strings.TrimSpace(...).

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (e3a75ee)

Verdict: Changes are not ready to merge due to one high-severity build break and one medium-severity review regression.

High

• Compilation failure at internal/daemon/worker.go:1167
  prepareDiffFileForCodex calls gitpkg.GetDiff / gitpkg.GetRangeDiff with excludes..., but the current function signatures in internal/git/git.go only accept two string arguments. This will not compile (too many arguments in call).
  Fix: Either update internal/git/git.go so those helpers accept excludes ...string, or remove the extra arguments if they are not needed.

Medium

• Dirty-review fallback is incomplete at internal/daemon/worker.go:406
  The new diff-snapshot flow only covers stored prompts and the normal job.GitRef path. Dirty reviews still use BuildDirty(...) unchanged, so large uncommitted diffs can still be truncated in the prompt without a file-based fallback. With non-agentic Codex now running under --sandbox read-only, Codex can no longer recover omitted diff content itself, which is a review-quality regression.
  Fix: Add the same snapshot/file-reference mechanism for dirty reviews, or keep dirty Codex reviews on the previous sandbox mode until oversized dirty diffs are supplied out-of-band.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

[roborev-ci]

roborev: Combined Review (209273e)

Verdict: One medium-severity regression needs to be addressed; no higher-severity issues were identified.

Medium

• Retry path persists stale oversized-diff instructions for some Codex reviews
  Location: internal/daemon/worker.go, internal/daemon/worker.go
  Large non-agentic Codex reviews built through the normal worker path still save the original pb.Build(...) prompt before the diff-file rewrite happens. On retry, that leaves the stored prompt with the old git-based oversized-diff fallback instead of CodexDiffFilePathPlaceholder, so preparePrebuiltCodexPrompt can only append a cat <diff> note rather than replacing the stale instructions. In read-only sandbox mode, retries can therefore contain contradictory guidance and point the model back toward commands expected to fail.
  Suggested fix: When truncation is detected, persist a placeholder-based prompt for Codex reviews by building with prompt.CodexDiffFilePathPlaceholder, then replace that placeholder at execution time.

---

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)