diff --git a/internal/agent/codex.go b/internal/agent/codex.go index 421e6294..0ac81423 100644 --- a/internal/agent/codex.go +++ b/internal/agent/codex.go @@ -7,10 +7,7 @@ import ( "fmt" "io" "log" - "os" "os/exec" - "path/filepath" - "regexp" "strings" "sync" ) @@ -128,7 +125,7 @@ func (a *CodexAgent) buildArgs( agenticMode: agenticMode, autoApprove: autoApprove, sandboxBroken: sandboxBroken, - addDirs: codexDiffSnapshotDirs(prompt), + addDirs: diffSnapshotDirs(prompt), }) } @@ -185,44 +182,6 @@ func (a *CodexAgent) commandArgs(opts codexArgOptions) []string { return args } -var codexDiffSnapshotRE = regexp.MustCompile("`([^`]*roborev-snapshot-[^`]*\\.diff)`") - -func codexDiffSnapshotDirs(reviewPrompt string) []string { - matches := codexDiffSnapshotRE.FindAllStringSubmatch(reviewPrompt, -1) - if len(matches) == 0 { - return nil - } - seen := make(map[string]struct{}, len(matches)) - var dirs []string - for _, match := range matches { - if len(match) < 2 { - continue - } - path := filepath.Clean(match[1]) - if !filepath.IsAbs(path) { - continue - } - base := filepath.Base(path) - if !strings.HasPrefix(base, "roborev-snapshot-") || filepath.Ext(base) != ".diff" { - continue - } - dir := filepath.Dir(path) - if !strings.HasPrefix(filepath.Base(dir), "roborev-snapshot-") { - continue - } - info, err := os.Stat(path) - if err != nil || info.IsDir() { - continue - } - if _, ok := seen[dir]; ok { - continue - } - seen[dir] = struct{}{} - dirs = append(dirs, dir) - } - return dirs -} - func codexSupportsDangerousFlag(ctx context.Context, command string) (bool, error) { if cached, ok := codexDangerousSupport.Load(command); ok { return cached.(bool), nil diff --git a/internal/agent/gemini.go b/internal/agent/gemini.go index 8a03f91e..d2042963 100644 --- a/internal/agent/gemini.go +++ b/internal/agent/gemini.go @@ -90,17 +90,17 @@ func (a *GeminiAgent) CommandName() string { func (a *GeminiAgent) CommandLine() string { agenticMode := a.Agentic || AllowUnsafeAgents() - args := a.buildArgs(agenticMode) + args := a.buildArgs(agenticMode, "") return a.Command + " " + strings.Join(args, " ") } -func (a *GeminiAgent) buildArgs(agenticMode bool) []string { - return a.buildArgsWithModel(a.Model, agenticMode) +func (a *GeminiAgent) buildArgs(agenticMode bool, prompt string) []string { + return a.buildArgsWithModel(a.Model, agenticMode, prompt) } func (a *GeminiAgent) Review(ctx context.Context, repoPath, commitSHA, prompt string, output io.Writer) (string, error) { agenticMode := a.Agentic || AllowUnsafeAgents() - args := a.buildArgs(agenticMode) + args := a.buildArgs(agenticMode, prompt) result, stderrStr, err := a.runGemini(ctx, repoPath, prompt, args, output) if err != nil && a.Model == defaultGeminiModel && isModelNotFoundError(stderrStr) { @@ -109,7 +109,7 @@ func (a *GeminiAgent) Review(ctx context.Context, repoPath, commitSHA, prompt st // its own default. Non-default models (set via WithModel / // config) fail fast so config errors are surfaced. log.Printf("gemini: model %q not found, retrying without -m flag", a.Model) - noModelArgs := a.buildArgsWithModel("", agenticMode) + noModelArgs := a.buildArgsWithModel("", agenticMode, prompt) result, _, err = a.runGemini(ctx, repoPath, prompt, noModelArgs, output) } return result, err @@ -117,7 +117,7 @@ func (a *GeminiAgent) Review(ctx context.Context, repoPath, commitSHA, prompt st // buildArgsWithModel builds CLI args with an explicit model override // (empty string omits the -m flag entirely). -func (a *GeminiAgent) buildArgsWithModel(model string, agenticMode bool) []string { +func (a *GeminiAgent) buildArgsWithModel(model string, agenticMode bool, prompt string) []string { args := []string{"--output-format", "stream-json"} if model != "" { @@ -129,6 +129,12 @@ func (a *GeminiAgent) buildArgsWithModel(model string, agenticMode bool) []strin } else { args = append(args, "--approval-mode", "plan") } + + // Expose roborev diff snapshot dirs referenced in the prompt so + // gemini can read large diff files that live outside the repo. + for _, dir := range diffSnapshotDirs(prompt) { + args = append(args, "--include-directories", dir) + } return args } diff --git a/internal/agent/gemini_test.go b/internal/agent/gemini_test.go index 012886fe..ceb69587 100644 --- a/internal/agent/gemini_test.go +++ b/internal/agent/gemini_test.go @@ -69,7 +69,7 @@ func TestGeminiBuildArgs(t *testing.T) { assert := assert.New(t) a := NewGeminiAgent("gemini") - args := a.buildArgs(tc.agentic) + args := a.buildArgs(tc.agentic, "") // Check standalone boolean flags for _, flag := range tc.wantFlags { @@ -89,6 +89,33 @@ func TestGeminiBuildArgs(t *testing.T) { } } +func TestGeminiBuildArgsAddsDiffSnapshotDirectory(t *testing.T) { + a := NewGeminiAgent("gemini") + snapshotDir, err := os.MkdirTemp("", "roborev-snapshot-*") + require.NoError(t, err) + t.Cleanup(func() { os.RemoveAll(snapshotDir) }) + diffFile := filepath.Join(snapshotDir, "roborev-snapshot-content.diff") + require.NoError(t, os.WriteFile(diffFile, []byte("diff --git a/x b/x\n"), 0o600)) + + args := a.buildArgs(false, "Read the diff from: `"+diffFile+"`") + + idx := slices.Index(args, "--include-directories") + require.NotEqual(t, -1, idx, "expected --include-directories in args: %v", args) + require.Less(t, idx+1, len(args), "expected --include-directories value in args: %v", args) + assert.Equal(t, snapshotDir, args[idx+1]) +} + +func TestGeminiBuildArgsIgnoresDiffSnapshotOutsidePrivateDirectory(t *testing.T) { + a := NewGeminiAgent("gemini") + diffFile, err := os.CreateTemp("", "roborev-snapshot-*.diff") + require.NoError(t, err) + t.Cleanup(func() { os.Remove(diffFile.Name()) }) + + args := a.buildArgs(false, "Read the diff from: `"+diffFile.Name()+"`") + + assert.NotContains(t, args, "--include-directories") +} + func assertFlagValue(t *testing.T, args []string, flag, expectedVal string) { t.Helper() assert := assert.New(t) diff --git a/internal/agent/snapshot_dirs.go b/internal/agent/snapshot_dirs.go new file mode 100644 index 00000000..198d50d2 --- /dev/null +++ b/internal/agent/snapshot_dirs.go @@ -0,0 +1,51 @@ +package agent + +import ( + "os" + "path/filepath" + "regexp" + "strings" +) + +var diffSnapshotRE = regexp.MustCompile("`([^`]*roborev-snapshot-[^`]*\\.diff)`") + +// diffSnapshotDirs scans a review prompt for backtick-quoted paths to +// roborev diff snapshots and returns the unique parent directories. +// Only paths that point to an existing file inside a "roborev-snapshot-" +// directory are returned, so sandboxed agents can expose just those +// private directories instead of the whole system temp dir. +func diffSnapshotDirs(reviewPrompt string) []string { + matches := diffSnapshotRE.FindAllStringSubmatch(reviewPrompt, -1) + if len(matches) == 0 { + return nil + } + seen := make(map[string]struct{}, len(matches)) + var dirs []string + for _, match := range matches { + if len(match) < 2 { + continue + } + path := filepath.Clean(match[1]) + if !filepath.IsAbs(path) { + continue + } + base := filepath.Base(path) + if !strings.HasPrefix(base, "roborev-snapshot-") || filepath.Ext(base) != ".diff" { + continue + } + dir := filepath.Dir(path) + if !strings.HasPrefix(filepath.Base(dir), "roborev-snapshot-") { + continue + } + info, err := os.Stat(path) + if err != nil || info.IsDir() { + continue + } + if _, ok := seen[dir]; ok { + continue + } + seen[dir] = struct{}{} + dirs = append(dirs, dir) + } + return dirs +}