refactor: unified diff file fallback for all agents

Replace the Codex-specific large-diff handling with a single path
that works the same for every agent:

1. Worker writes the full diff to a snapshot file before building
   the prompt
2. Builder inlines the diff if it fits; references the file if it
   doesn't
3. No agent-specific branching, no prompt parsing, no placeholders

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

Author: wesm

internal/daemon/worker.go

@@ -422,39 +422,23 @@ func (wp *WorkerPool) processJob(workerID string, job *storage.ReviewJob) {
 		// Dirty job - use pre-captured diff
 		reviewPrompt, err = pb.BuildDirty(effectiveRepoPath, *job.DiffContent, job.RepoID, cfg.ReviewContextCount, job.Agent, job.ReviewType)
 	} else {
-		// Normal job - build prompt from git ref.
-		reviewPrompt, err = pb.Build(
-			effectiveRepoPath, job.GitRef, job.RepoID,
-			cfg.ReviewContextCount, job.Agent, job.ReviewType,
+		// Normal job - build prompt from git ref. Write the full
+		// diff to a snapshot file so the builder can reference it
+		// when the diff is too large to inline.
+		excludes := config.ResolveExcludePatterns(
+			effectiveRepoPath, cfg, job.ReviewType,
 		)
-		// If the diff was truncated for a Codex review, write a
-		// snapshot file so the sandboxed agent can read it directly
-		// instead of needing git commands.
-		if err == nil && strings.EqualFold(job.Agent, "codex") &&
-			strings.Contains(reviewPrompt, prompt.DiffTruncatedHint) {
-			excludes := config.ResolveExcludePatterns(
-				effectiveRepoPath, cfg, job.ReviewType,
-			)
-			diffFile, cleanup, diffFileErr := prepareDiffFileForCodex(
-				effectiveRepoPath, job, excludes,
-			)
-			if cleanup != nil {
-				defer cleanup()
-			}
-			if diffFile != "" {
-				reviewPrompt, err = pb.BuildWithDiffFile(
-					effectiveRepoPath, job.GitRef, job.RepoID,
-					cfg.ReviewContextCount, job.Agent,
-					job.ReviewType, diffFile,
-				)
-			} else if diffFileErr != nil &&
-				requiresSandboxedCodexDiffFile(job, cfg) {
-				err = fmt.Errorf("prepare codex diff file: %w", diffFileErr)
-			} else if diffFileErr != nil {
-				log.Printf("[%s] Warning: codex diff file: %v",
-					workerID, diffFileErr)
-			}
+		diffFile, cleanup := prepareDiffFile(
+			effectiveRepoPath, job, excludes,
+		)
+		if cleanup != nil {
+			defer cleanup()
 		}
+		reviewPrompt, err = pb.BuildWithDiffFile(
+			effectiveRepoPath, job.GitRef, job.RepoID,
+			cfg.ReviewContextCount, job.Agent,
+			job.ReviewType, diffFile,
+		)
 	}
 	if err != nil {
 		log.Printf("[%s] Error building prompt: %v", workerID, err)
@@ -1044,60 +1028,21 @@ func (wp *WorkerPool) failoverOrFail(
 func preparePrebuiltCodexPrompt(
 	repoPath string, job *storage.ReviewJob, reviewPrompt string, excludes []string,
 ) (string, func(), error) {
-	hasPlaceholder := strings.Contains(
-		reviewPrompt, prompt.CodexDiffFilePathPlaceholder,
-	)
-	// Legacy prebuilt prompts (created before diff-file support) won't
-	// have the placeholder. For those, only write a diff file when the
-	// prompt indicates the diff was truncated.
-	if !hasPlaceholder {
-		if !strings.EqualFold(job.Agent, "codex") ||
-			!strings.Contains(reviewPrompt, prompt.DiffTruncatedHint) {
-			return reviewPrompt, nil, nil
-		}
-		diffFile, cleanup, err := prepareDiffFileForCodex(
-			repoPath, job, excludes,
-		)
-		if err != nil || diffFile == "" {
-			if cleanup != nil {
-				cleanup()
-			}
-			// Best-effort for legacy prompts: return as-is so the
-			// old git-command fallback can still be attempted.
-			return reviewPrompt, nil, nil
-		}
-		reviewPrompt += fmt.Sprintf(
-			"\n\nThe full diff is also available at: `cat %s`\n",
-			shellQuoteForPrompt(diffFile),
-		)
-		return reviewPrompt, cleanup, nil
-	}
-
-	diffFile, cleanup, err := prepareDiffFileForCodex(repoPath, job, excludes)
-	if err != nil {
-		if cleanup != nil {
-			cleanup()
-		}
-		return "", nil, fmt.Errorf("prepare prebuilt codex prompt diff file: %w", err)
+	if !strings.Contains(reviewPrompt, prompt.CodexDiffFilePathPlaceholder) {
+		return reviewPrompt, nil, nil
 	}
+	diffFile, cleanup := prepareDiffFile(repoPath, job, excludes)
 	if diffFile == "" {
-		if cleanup != nil {
-			cleanup()
-		}
 		return "", nil, fmt.Errorf(
-			"prebuilt codex prompt needs diff file but none could be prepared",
+			"prebuilt prompt needs diff file but none could be prepared",
 		)
 	}
-	quotedPlaceholder := shellQuoteForPrompt(
-		prompt.CodexDiffFilePathPlaceholder,
-	)
-	reviewPrompt = strings.ReplaceAll(
-		reviewPrompt, quotedPlaceholder, shellQuoteForPrompt(diffFile),
+	replacer := strings.NewReplacer(
+		shellQuoteForPrompt(prompt.CodexDiffFilePathPlaceholder),
+		shellQuoteForPrompt(diffFile),
+		prompt.CodexDiffFilePathPlaceholder, diffFile,
 	)
-	reviewPrompt = strings.ReplaceAll(
-		reviewPrompt, prompt.CodexDiffFilePathPlaceholder, diffFile,
-	)
-	return reviewPrompt, cleanup, nil
+	return replacer.Replace(reviewPrompt), cleanup, nil
 }
 
 func shellQuoteForPrompt(s string) string {
@@ -1107,24 +1052,6 @@ func shellQuoteForPrompt(s string) string {
 	return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'"
 }
 
-func requiresSandboxedCodexDiffFile(
-	job *storage.ReviewJob, cfg *config.Config,
-) bool {
-	if !strings.EqualFold(job.Agent, "codex") ||
-		job.Agentic || agent.AllowUnsafeAgents() {
-		return false
-	}
-	// Use the same config-aware resolution as the worker to check
-	// whether Codex is the effective agent. If it falls back to
-	// another agent (e.g., Codex is unavailable or overridden via
-	// codex_cmd), the diff file is not required.
-	a, err := agent.GetAvailableWithConfig(job.Agent, cfg)
-	if err != nil {
-		return false
-	}
-	return strings.EqualFold(a.Name(), "codex")
-}
-
 // logJobFailed logs a job failure to the activity log
 func (wp *WorkerPool) logJobFailed(
 	jobID int64, workerID, agentName, errorMsg string,
@@ -1144,21 +1071,13 @@ func (wp *WorkerPool) logJobFailed(
 	)
 }
 
-// prepareDiffFileForCodex writes the full diff for a job into the
-// checkout's git dir so a sandboxed Codex agent can read it without
-// needing git commands or access outside the repository boundary.
-// Returns the file path and a cleanup function, or ("", nil) when no
-// file was written.
-func prepareDiffFileForCodex(
+// prepareDiffFile writes the full diff for a job into the checkout's
+// git dir so the prompt builder can reference it when the diff is too
+// large to inline. Returns the file path and a cleanup function, or
+// ("", nil) on failure.
+func prepareDiffFile(
 	repoPath string, job *storage.ReviewJob, excludes []string,
-) (string, func(), error) {
-	if !strings.EqualFold(job.Agent, "codex") {
-		return "", nil, nil
-	}
-	// Job-specific agentic mode keeps the existing direct-tooling path.
-	if job.Agentic {
-		return "", nil, nil
-	}
+) (string, func()) {
 	var fullDiff string
 	var err error
 	if gitpkg.IsRange(job.GitRef) {
@@ -1170,27 +1089,26 @@ func prepareDiffFileForCodex(
 			repoPath, job.GitRef, excludes...,
 		)
 	}
-	if err != nil {
-		return "", nil, fmt.Errorf("capture full diff: %w", err)
-	}
-	if fullDiff == "" {
-		return "", nil, nil
+	if err != nil || fullDiff == "" {
+		if err != nil {
+			log.Printf("Warning: capture diff for snapshot: %v", err)
+		}
+		return "", nil
 	}
 	gitDir, err := gitpkg.ResolveGitDir(repoPath)
 	if err != nil {
-		return "", nil, fmt.Errorf("resolve git dir: %w", err)
+		log.Printf("Warning: resolve git dir for diff snapshot: %v", err)
+		return "", nil
 	}
-	if info, err := os.Stat(gitDir); err != nil || !info.IsDir() {
-		if err != nil {
-			return "", nil, fmt.Errorf("stat git dir: %w", err)
-		}
-		return "", nil, fmt.Errorf("git dir %s is not a directory", gitDir)
-	}
-	diffFile := filepath.Join(gitDir, fmt.Sprintf("roborev-review-%d.diff", job.ID))
+	diffFile := filepath.Join(
+		gitDir,
+		fmt.Sprintf("roborev-review-%d.diff", job.ID),
+	)
 	if err := os.WriteFile(diffFile, []byte(fullDiff), 0o644); err != nil {
-		return "", nil, fmt.Errorf("write diff file %s: %w", diffFile, err)
+		log.Printf("Warning: write diff snapshot %s: %v", diffFile, err)
+		return "", nil
 	}
-	return diffFile, func() { os.Remove(diffFile) }, nil
+	return diffFile, func() { os.Remove(diffFile) }
 }
 
 // markCompactSourceJobs marks all source jobs as closed for a completed compact job

internal/daemon/worker_test.go

@@ -6,6 +6,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"runtime"
 
 	"github.com/roborev-dev/roborev/internal/agent"
 	"github.com/roborev-dev/roborev/internal/config"
@@ -538,9 +539,8 @@ func TestPrepareDiffFileForCodex_WritesDiffInWorktreeGitDir(t *testing.T) {
 	sha := strings.TrimSpace(string(shaBytes))
 
 	job := &storage.ReviewJob{ID: 42, Agent: "codex", GitRef: sha}
-	diffFile, cleanup, err := prepareDiffFileForCodex(worktreeDir, job, nil)
-	require.NoError(t, err)
-	require.NotEmpty(t, diffFile, "expected diff file for worktree-backed Codex review")
+	diffFile, cleanup := prepareDiffFile(worktreeDir, job, nil)
+	require.NotEmpty(t, diffFile, "expected diff file for worktree-backed review")
 	require.NotNil(t, cleanup)
 
 	gitDirBytes, err := exec.Command("git", "-C", worktreeDir, "rev-parse", "--git-dir").Output()
@@ -635,19 +635,19 @@ func TestPreparePrebuiltCodexPrompt_AllowsUnsafeModeByStillWritingDiffFile(t *te
 	cleanup()
 }
 
-func TestProcessJob_SandboxedCodexRetriesWhenDiffSnapshotCannotBeWritten(t *testing.T) {
-	prevUnsafe := agent.AllowUnsafeAgents()
-	agent.SetAllowUnsafeAgents(false)
-	t.Cleanup(func() { agent.SetAllowUnsafeAgents(prevUnsafe) })
+func TestProcessJob_DiffSnapshotFailureStillRunsReview(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("chmod does not restrict writes on Windows")
+	}
 
 	originalCodex, err := agent.Get("codex")
 	require.NoError(t, err)
 
-	agentCalled := false
+	var receivedPrompt string
 	agent.Register(&agent.FakeAgent{
 		NameStr: "codex",
-		ReviewFn: func(ctx context.Context, repoPath, commitSHA, reviewPrompt string, output io.Writer) (string, error) {
-			agentCalled = true
+		ReviewFn: func(ctx context.Context, repoPath, commitSHA, prompt string, output io.Writer) (string, error) {
+			receivedPrompt = prompt
 			return "No issues found.", nil
 		},
 	})
@@ -681,6 +681,7 @@ func TestProcessJob_SandboxedCodexRetriesWhenDiffSnapshotCannotBeWritten(t *test
 	})
 	require.NoError(t, err)
 
+	// Make git dir read-only so diff file write fails
 	gitDir, err := gitpkg.ResolveGitDir(tc.TmpDir)
 	require.NoError(t, err)
 	info, err := os.Stat(gitDir)
@@ -695,11 +696,12 @@ func TestProcessJob_SandboxedCodexRetriesWhenDiffSnapshotCannotBeWritten(t *test
 
 	tc.Pool.processJob(testWorkerID, claimed)
 
-	tc.assertJobStatus(t, job.ID, storage.JobStatusQueued)
-	assert.False(t, agentCalled, "Codex should not run when snapshot creation fails in sandboxed mode")
-	retryCount, err := tc.DB.GetJobRetryCount(job.ID)
-	require.NoError(t, err)
-	assert.Equal(t, 1, retryCount)
+	// Diff file failure is best-effort; the review still runs
+	// with a truncated-diff prompt
+	tc.assertJobStatus(t, job.ID, storage.JobStatusDone)
+	assert.NotEmpty(t, receivedPrompt, "agent should have been called")
+	assert.Contains(t, receivedPrompt, "Diff too large",
+		"prompt should indicate diff was truncated")
 }
 
 func TestWorkerPoolCancelJobFinalCheckDeadlockSafe(t *testing.T) {