fix: Codex sandbox compatibility and command line accuracy (#633)

## Summary

- **Inline snapshot diffs for sandboxed Codex**: Codex `--sandbox
read-only` blocks `.git/` access, making large-diff snapshot files
unreadable. The worker now reads the `.git/` snapshot and inlines the
diff content into the prompt sent via stdin. The DB keeps the truncated
prompt; only the agent sees the full diff. Non-Codex agents continue
using `.git/` snapshots directly.
- **`disable_codex_sandbox` config option**: On systems where bwrap
fails (missing unprivileged user namespace support), add a global config
toggle that switches Codex from `--sandbox read-only` to
`--dangerously-bypass-approvals-and-sandbox`. Hot-reloadable via config
watcher.
- **Store actual command line on jobs**: The daemon worker now saves the
real `CommandLine()` to the job record when starting each run. The TUI
displays this stored value instead of reconstructing it client-side,
which missed daemon-only config like `disable_codex_sandbox`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: wesm

cmd/roborev/tui/render_queue.go

@@ -725,6 +725,11 @@ func commandLineForJob(job *storage.ReviewJob) string {
 	if job == nil {
 		return ""
 	}
+	// Prefer the actual command line saved by the daemon worker.
+	if job.CommandLine != "" {
+		return stripControlChars(job.CommandLine)
+	}
+	// Fallback: reconstruct locally (may not reflect daemon config).
 	a, err := agent.Get(job.Agent)
 	if err != nil {
 		return ""

internal/agent/agent.go

@@ -99,6 +99,7 @@ var (
 )
 
 var allowUnsafeAgents atomic.Bool
+var codexSandboxDisabled atomic.Bool
 var anthropicAPIKey atomic.Value
 
 func AllowUnsafeAgents() bool {
@@ -109,6 +110,18 @@ func SetAllowUnsafeAgents(allow bool) {
 	allowUnsafeAgents.Store(allow)
 }
 
+// CodexSandboxDisabled returns true when the Codex bwrap sandbox
+// should be skipped (e.g. on machines without unprivileged user
+// namespace support). When true, Codex runs with --full-auto
+// instead of --sandbox read-only.
+func CodexSandboxDisabled() bool {
+	return codexSandboxDisabled.Load()
+}
+
+func SetCodexSandboxDisabled(v bool) {
+	codexSandboxDisabled.Store(v)
+}
+
 // AnthropicAPIKey returns the configured Anthropic API key, or empty string if not set
 func AnthropicAPIKey() string {
 	if v := anthropicAPIKey.Load(); v != nil {

internal/agent/codex.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"log"
 	"os/exec"
 	"strings"
 	"sync"
@@ -106,26 +107,32 @@ func (a *CodexAgent) CommandName() string {
 func (a *CodexAgent) CommandLine() string {
 	agenticMode := a.Agentic || AllowUnsafeAgents()
 	args := a.commandArgs(codexArgOptions{
-		agenticMode: agenticMode,
-		autoApprove: !agenticMode,
-		preview:     true,
+		agenticMode:   agenticMode,
+		autoApprove:   !agenticMode,
+		sandboxBroken: CodexSandboxDisabled(),
+		preview:       true,
 	})
 	return a.Command + " " + strings.Join(args, " ")
 }
 
-func (a *CodexAgent) buildArgs(repoPath string, agenticMode, autoApprove bool) []string {
+func (a *CodexAgent) buildArgs(
+	repoPath string,
+	agenticMode, autoApprove, sandboxBroken bool,
+) []string {
 	return a.commandArgs(codexArgOptions{
-		repoPath:    repoPath,
-		agenticMode: agenticMode,
-		autoApprove: autoApprove,
+		repoPath:      repoPath,
+		agenticMode:   agenticMode,
+		autoApprove:   autoApprove,
+		sandboxBroken: sandboxBroken,
 	})
 }
 
 type codexArgOptions struct {
-	repoPath    string
-	agenticMode bool
-	autoApprove bool
-	preview     bool
+	repoPath      string
+	agenticMode   bool
+	autoApprove   bool
+	sandboxBroken bool
+	preview       bool
 }
 
 func (a *CodexAgent) commandArgs(opts codexArgOptions) []string {
@@ -141,10 +148,13 @@ func (a *CodexAgent) commandArgs(opts codexArgOptions) []string {
 		args = append(args, codexDangerousFlag)
 	}
 	if opts.autoApprove {
-		// Use read-only sandbox for review mode. The worker
-		// writes the diff to a file that Codex can read within
-		// the sandbox, removing the need for git commands.
-		args = append(args, "--sandbox", "read-only")
+		if opts.sandboxBroken {
+			// --full-auto still uses bwrap internally, so we
+			// need the full bypass flag on broken systems.
+			args = append(args, codexDangerousFlag)
+		} else {
+			args = append(args, "--sandbox", "read-only")
+		}
 	}
 	if !opts.preview {
 		args = append(args, "-C", opts.repoPath)
@@ -226,7 +236,11 @@ func (a *CodexAgent) Review(ctx context.Context, repoPath, commitSHA, prompt str
 
 	// Use codex exec with --json for JSONL streaming output
 	// The prompt is piped via stdin using "-" to avoid command line length limits on Windows
-	args := a.buildArgs(repoPath, agenticMode, autoApprove)
+	sandboxBroken := CodexSandboxDisabled()
+	if sandboxBroken && autoApprove {
+		log.Printf("codex: sandbox disabled via config, using %s", codexAutoApproveFlag)
+	}
+	args := a.buildArgs(repoPath, agenticMode, autoApprove, sandboxBroken)
 
 	runResult, runErr := runStreamingCLI(ctx, streamingCLISpec{
 		Name:         "codex",

internal/agent/codex_test.go

@@ -29,6 +29,7 @@ func TestCodex_buildArgs(t *testing.T) {
 		name             string
 		agentic          bool
 		autoApprove      bool
+		sandboxBroken    bool
 		wantFlags        []string
 		wantMissingFlags []string
 	}{
@@ -46,11 +47,19 @@ func TestCodex_buildArgs(t *testing.T) {
 			wantFlags:        []string{codexDangerousFlag, "--json"},
 			wantMissingFlags: []string{codexAutoApproveFlag},
 		},
+		{
+			name:             "SandboxBrokenFallback",
+			agentic:          false,
+			autoApprove:      true,
+			sandboxBroken:    true,
+			wantFlags:        []string{codexDangerousFlag, "--json"},
+			wantMissingFlags: []string{"--sandbox", codexAutoApproveFlag},
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			args := a.buildArgs("/repo", tt.agentic, tt.autoApprove)
+			args := a.buildArgs("/repo", tt.agentic, tt.autoApprove, tt.sandboxBroken)
 
 			for _, flag := range tt.wantFlags {
 				assert.Contains(t, args, flag, "buildArgs() missing expected flag %q, args: %v", flag, args)
@@ -68,7 +77,7 @@ func TestCodex_buildArgs(t *testing.T) {
 func TestCodexBuildArgsWithSessionResume(t *testing.T) {
 	a := NewCodexAgent("codex").WithSessionID("session-123").(*CodexAgent)
 
-	args := a.buildArgs("/repo", false, true)
+	args := a.buildArgs("/repo", false, true, false)
 
 	require.GreaterOrEqual(t, len(args), 3)
 	assert.Equal(t, "exec", args[0])
@@ -81,7 +90,7 @@ func TestCodexBuildArgsWithSessionResume(t *testing.T) {
 func TestCodexBuildArgsRejectsInvalidSessionResume(t *testing.T) {
 	a := NewCodexAgent("codex").WithSessionID("-bad-session").(*CodexAgent)
 
-	args := a.buildArgs("/repo", false, true)
+	args := a.buildArgs("/repo", false, true, false)
 
 	require.GreaterOrEqual(t, len(args), 2)
 	assert.Equal(t, "exec", args[0])

internal/config/config.go

@@ -144,8 +144,9 @@ type Config struct {
 	SecurityBackupModel string `toml:"security_backup_model"`
 	DesignBackupModel   string `toml:"design_backup_model"`
 
-	AllowUnsafeAgents  *bool `toml:"allow_unsafe_agents"`  // nil = not set, allows commands to choose their own default
-	ReuseReviewSession *bool `toml:"reuse_review_session"` // nil = not set; when true, reuse prior branch review sessions when possible
+	AllowUnsafeAgents   *bool `toml:"allow_unsafe_agents"`   // nil = not set, allows commands to choose their own default
+	DisableCodexSandbox bool  `toml:"disable_codex_sandbox"` // use --full-auto instead of --sandbox read-only (for systems where bwrap is broken)
+	ReuseReviewSession  *bool `toml:"reuse_review_session"`  // nil = not set; when true, reuse prior branch review sessions when possible
 
 	// Agent commands
 	CodexCmd      string `toml:"codex_cmd"`

internal/daemon/config_watcher.go

@@ -210,6 +210,7 @@ func (cw *ConfigWatcher) reloadConfig() {
 
 	// Update global agent settings
 	agent.SetAllowUnsafeAgents(newCfg.AllowUnsafeAgents != nil && *newCfg.AllowUnsafeAgents)
+	agent.SetCodexSandboxDisabled(newCfg.DisableCodexSandbox)
 	agent.SetAnthropicAPIKey(newCfg.AnthropicAPIKey)
 
 	// Log what changed (for debugging)

internal/daemon/server.go

@@ -55,6 +55,7 @@ type Server struct {
 func NewServer(db *storage.DB, cfg *config.Config, configPath string) *Server {
 	// Always set for deterministic state - default to false (conservative)
 	agent.SetAllowUnsafeAgents(cfg.AllowUnsafeAgents != nil && *cfg.AllowUnsafeAgents)
+	agent.SetCodexSandboxDisabled(cfg.DisableCodexSandbox)
 	agent.SetAnthropicAPIKey(cfg.AnthropicAPIKey)
 	broadcaster := NewBroadcaster()
 

internal/daemon/worker.go

@@ -7,6 +7,7 @@ import (
 	"io"
 	"log"
 	"os"
+	"regexp"
 	"strings"
 	"sync"
 	"sync/atomic"
@@ -501,6 +502,21 @@ func (wp *WorkerPool) processJob(workerID string, job *storage.ReviewJob) {
 		log.Printf("[%s] Agent %s not available, using %s", workerID, job.Agent, agentName)
 	}
 
+	// Store the actual command line so the TUI displays what the
+	// daemon executes, not a client-side reconstruction.
+	if err := wp.db.SaveJobCommandLine(job.ID, a.CommandLine()); err != nil {
+		log.Printf("[%s] Error saving command line: %v", workerID, err)
+	}
+
+	// Codex --sandbox read-only cannot read files inside .git/.
+	// When the prompt references a snapshot file (oversized diff),
+	// read the file and inline its content so Codex gets the full
+	// diff via stdin without needing filesystem access. The DB
+	// keeps the truncated prompt; only the agent prompt expands.
+	if agentName == "codex" && !agentic {
+		reviewPrompt = expandSnapshotInline(reviewPrompt)
+	}
+
 	// Use the effective worktree path for events (empty when worktree is gone or not a worktree job).
 	eventWorktreePath := ""
 	if effectiveRepoPath != job.RepoPath {
@@ -1047,6 +1063,63 @@ func preparePrebuiltPrompt(
 	return replacer.Replace(reviewPrompt), cleanup, nil
 }
 
+// snapshotFileRefRE matches the file-reference lines emitted by
+// diffFileFallbackVariants in the prompt builder when a diff is too
+// large to inline. Example:
+//
+//	Read the diff from: `/path/to/.git/roborev-snapshot-1234.diff`
+var snapshotFileRefRE = regexp.MustCompile(
+	"`([^`]+roborev-snapshot-[^`]+\\.diff)`",
+)
+
+// expandSnapshotInline reads snapshot files referenced in a prompt
+// and replaces the file-reference section with the actual diff
+// content. This allows sandboxed agents that cannot access .git/ to
+// receive the full diff via stdin.
+func expandSnapshotInline(reviewPrompt string) string {
+	m := snapshotFileRefRE.FindStringSubmatch(reviewPrompt)
+	if m == nil {
+		return reviewPrompt
+	}
+	snapshotPath := m[1]
+	data, err := os.ReadFile(snapshotPath)
+	if err != nil {
+		// File unreadable — return the prompt as-is; the agent
+		// will see the truncated reference (same as before).
+		log.Printf("expand snapshot inline: read %s: %v", snapshotPath, err)
+		return reviewPrompt
+	}
+
+	// Replace the entire "(Diff too large …) … Read the diff from …"
+	// block with an inline diff fence.
+	inline := "```diff\n" + string(data)
+	if len(data) > 0 && data[len(data)-1] != '\n' {
+		inline += "\n"
+	}
+	inline += "```\n"
+
+	// The fallback block always starts with "(Diff too large" and
+	// ends after the file-reference line. Replace that span.
+	start := strings.Index(reviewPrompt, "(Diff too large")
+	if start < 0 {
+		// Dirty-review fallback uses different wording.
+		start = strings.Index(reviewPrompt, "The full diff is also available at:")
+		if start < 0 {
+			return reviewPrompt
+		}
+	}
+	refEnd := strings.Index(reviewPrompt[start:], m[0])
+	if refEnd < 0 {
+		return reviewPrompt
+	}
+	// Include the trailing backtick and any newlines after it.
+	end := start + refEnd + len(m[0])
+	for end < len(reviewPrompt) && reviewPrompt[end] == '\n' {
+		end++
+	}
+	return reviewPrompt[:start] + inline + reviewPrompt[end:]
+}
+
 func shellQuoteForPrompt(s string) string {
 	if s == "" {
 		return "''"

internal/daemon/worker_test.go

@@ -751,6 +751,49 @@ func TestProcessJob_LargeDiffRetriesWhenSnapshotFails(t *testing.T) {
 	assert.False(t, agentCalled, "agent should not run when large diff has no snapshot")
 }
 
+func TestExpandSnapshotInline_ReplacesFileRef(t *testing.T) {
+	// Write a temp snapshot file
+	f, err := os.CreateTemp(t.TempDir(), "roborev-snapshot-*.diff")
+	require.NoError(t, err)
+	_, err = f.WriteString("diff --git a/file.go b/file.go\n+added line\n")
+	require.NoError(t, err)
+	require.NoError(t, f.Close())
+
+	prompt := fmt.Sprintf(
+		"### Diff\n\n"+
+			"(Diff too large to include inline)\n\n"+
+			"The full diff has been written to a file for review.\n"+
+			"Read the diff from: `%s`\n\n"+
+			"Review the actual diff before writing findings.\n"+
+			"\n## Summary\n",
+		f.Name(),
+	)
+
+	result := expandSnapshotInline(prompt)
+
+	assert := assert.New(t)
+	assert.NotContains(result, "Diff too large")
+	assert.NotContains(result, "roborev-snapshot-")
+	assert.Contains(result, "```diff")
+	assert.Contains(result, "+added line")
+	assert.Contains(result, "## Summary")
+}
+
+func TestExpandSnapshotInline_NoRef(t *testing.T) {
+	prompt := "### Diff\n\n```diff\n+inline diff\n```\n"
+	result := expandSnapshotInline(prompt)
+	assert.Equal(t, prompt, result)
+}
+
+func TestExpandSnapshotInline_MissingFile(t *testing.T) {
+	prompt := "### Diff\n\n" +
+		"(Diff too large to include inline)\n\n" +
+		"Read the diff from: `/nonexistent/roborev-snapshot-123.diff`\n"
+	result := expandSnapshotInline(prompt)
+	// Should return unchanged when file doesn't exist
+	assert.Equal(t, prompt, result)
+}
+
 func TestWorkerPoolCancelJobFinalCheckDeadlockSafe(t *testing.T) {
 	tc := newWorkerTestContext(t, 1)
 	job := tc.createAndClaimJob(t, "deadlock-test", testWorkerID)

internal/storage/db.go

@@ -775,6 +775,18 @@ func (db *DB) migrate() error {
 		}
 	}
 
+	// Migration: add command_line column to review_jobs if missing
+	err = db.QueryRow(`SELECT COUNT(*) FROM pragma_table_info('review_jobs') WHERE name = 'command_line'`).Scan(&count)
+	if err != nil {
+		return fmt.Errorf("check command_line column: %w", err)
+	}
+	if count == 0 {
+		_, err = db.Exec(`ALTER TABLE review_jobs ADD COLUMN command_line TEXT`)
+		if err != nil {
+			return fmt.Errorf("add command_line column: %w", err)
+		}
+	}
+
 	// Run sync-related migrations
 	if err := db.migrateSyncColumns(); err != nil {
 		return err