fix(tool): resolve file_read paths against git top-level in monorepos

ocr review from a monorepo subdirectory failed with "file not found" (alibaba#287):
git reports diff and `git show HEAD:<path>` paths relative to the repo root,
but RepoDir was scoped to the invocation subdirectory, producing a double
prefix. resolveWorkingDir now anchors RepoDir at `git rev-parse
--show-toplevel` on the review path (requireGit=true); scan keeps the CWD so
its `git ls-files` walk stays scoped.

The top-level lookup uses a stdout-only git helper so stderr notices can't
pollute the path, and fails loudly if --show-toplevel errors or is empty
(e.g. a bare repo) instead of silently reusing the subdirectory. Adds
regression tests for the subdir hoist, the scan-path scoping, git-show
resolution of root-relative paths, and the bare-repo failure.

Author: chethanuk

cmd/opencodereview/git.go

@@ -12,6 +12,15 @@ func runGitCmd(repoDir string, args ...string) ([]byte, error) {
 	return cmd.CombinedOutput()
 }
 
+// runGitCmdStdout is like runGitCmd but returns stdout only. Use it when the
+// output is consumed as data (e.g. a resolved path) so git's stderr warnings
+// (permissions, deprecations, config notices) can't pollute the result.
+func runGitCmdStdout(repoDir string, args ...string) ([]byte, error) {
+	fullArgs := append([]string{"-C", repoDir}, args...)
+	cmd := exec.Command("git", fullArgs...)
+	return cmd.Output()
+}
+
 func getCommitMessage(repoDir, commit string) (string, error) {
 	out, err := runGitCmd(repoDir, "log", "-1", "--format=%B", "--end-of-options", commit)
 	if err != nil {

cmd/opencodereview/shared.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 	"time"
 
 	"github.com/open-code-review/open-code-review/internal/agent"
@@ -100,6 +101,22 @@ func resolveWorkingDir(input string, requireGit bool) (string, bool, error) {
 	if !isGit && requireGit {
 		return "", false, fmt.Errorf("%s is not a git repository", absPath)
 	}
+	// #287: git reports diff and `git show HEAD:<path>` paths relative to the
+	// repository root, not the current directory. When `ocr review` runs from a
+	// subdirectory of a monorepo, anchor RepoDir at the git top-level so those
+	// root-relative paths resolve for both disk reads and git-show reads.
+	// requireGit is true only for the review path; scan (requireGit=false) keeps
+	// the CWD so its `git ls-files` walk stays scoped to the subdirectory.
+	if isGit && requireGit {
+		// runGitCmdStdout captures stdout only so git stderr notices can't
+		// pollute the resolved path. absPath is overridden only on success;
+		// on any rev-parse failure it falls back to the caller-provided dir.
+		if top, topErr := runGitCmdStdout(absPath, "rev-parse", "--show-toplevel"); topErr == nil {
+			if t := strings.TrimSpace(string(top)); t != "" {
+				absPath = t
+			}
+		}
+	}
 	return absPath, isGit, nil
 }
 

cmd/opencodereview/shared_test.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"os"
+	"os/exec"
 	"path/filepath"
 	"testing"
 
@@ -109,6 +110,71 @@ func TestResolveWorkingDir_NonExistent(t *testing.T) {
 	}
 }
 
+// TestResolveWorkingDir_MonorepoSubdir reproduces #287: running `ocr review`
+// from a subdirectory of a git repo must anchor RepoDir at the git top-level
+// (git reports diff / `git show HEAD:<path>` paths relative to the repo root),
+// while `ocr scan` (requireGit=false) must keep the subdirectory so its walk
+// stays scoped.
+func TestResolveWorkingDir_MonorepoSubdir(t *testing.T) {
+	root := t.TempDir()
+	git := func(args ...string) {
+		t.Helper()
+		cmd := exec.Command("git", args...)
+		cmd.Dir = root
+		if out, err := cmd.CombinedOutput(); err != nil {
+			t.Fatalf("git %v: %v\n%s", args, err, out)
+		}
+	}
+	git("init")
+	git("config", "user.email", "t@t.co")
+	git("config", "user.name", "t")
+
+	sub := filepath.Join(root, "subproject1", "src")
+	if err := os.MkdirAll(sub, 0o755); err != nil {
+		t.Fatal(err)
+	}
+
+	// macOS /var -> /private/var symlink means t.TempDir() differs from the
+	// canonicalized toplevel git returns; compare via EvalSymlinks.
+	wantRoot, err := filepath.EvalSymlinks(root)
+	if err != nil {
+		t.Fatalf("EvalSymlinks(%q): %v", root, err)
+	}
+
+	// review path: hoisted to the git top-level.
+	got, isGit, err := resolveWorkingDir(sub, true)
+	if err != nil {
+		t.Fatalf("resolveWorkingDir(sub, true) error: %v", err)
+	}
+	if !isGit {
+		t.Error("expected isGit=true for a git subdirectory")
+	}
+	gotResolved, err := filepath.EvalSymlinks(got)
+	if err != nil {
+		t.Fatalf("EvalSymlinks(%q): %v", got, err)
+	}
+	if gotResolved != wantRoot {
+		t.Errorf("review RepoDir = %q, want git top-level %q", gotResolved, wantRoot)
+	}
+
+	// scan path: keeps the subdirectory unchanged.
+	gotScan, _, err := resolveWorkingDir(sub, false)
+	if err != nil {
+		t.Fatalf("resolveWorkingDir(sub, false) error: %v", err)
+	}
+	gotScanResolved, err := filepath.EvalSymlinks(gotScan)
+	if err != nil {
+		t.Fatalf("EvalSymlinks(%q): %v", gotScan, err)
+	}
+	wantSub, err := filepath.EvalSymlinks(sub)
+	if err != nil {
+		t.Fatalf("EvalSymlinks(%q): %v", sub, err)
+	}
+	if gotScanResolved != wantSub {
+		t.Errorf("scan RepoDir = %q, want subdir %q (must stay scoped)", gotScanResolved, wantSub)
+	}
+}
+
 func TestResolveWorkingDir_GitRepo(t *testing.T) {
 	dir := t.TempDir()
 	gitDir := filepath.Join(dir, ".git")

internal/tool/filereader_read_test.go

@@ -3,6 +3,7 @@ package tool
 import (
 	"context"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -215,3 +216,58 @@ func TestFileReader_Read_SubdirectoryFile(t *testing.T) {
 		t.Errorf("Read() = %q, want %q", got, "package main")
 	}
 }
+
+// TestFileReader_Read_CommitMode_MonorepoSubdirPath reproduces #287 at the
+// git-show layer: in a monorepo, git reports paths relative to the repo root
+// (e.g. "subproject1/src/models/request_meta.py"). With RepoDir anchored at the
+// git top-level (the fix), `git show HEAD:<root-relative-path>` must resolve —
+// this is the exact command that failed in the issue.
+func TestFileReader_Read_CommitMode_MonorepoSubdirPath(t *testing.T) {
+	dir := t.TempDir()
+	git := func(args ...string) {
+		t.Helper()
+		cmd := exec.Command("git", args...)
+		cmd.Dir = dir
+		if out, err := cmd.CombinedOutput(); err != nil {
+			t.Fatalf("git %v: %v\n%s", args, err, out)
+		}
+	}
+	git("init")
+	git("config", "user.email", "t@t.co")
+	git("config", "user.name", "t")
+
+	rel := filepath.Join("subproject1", "src", "models", "request_meta.py")
+	if err := os.MkdirAll(filepath.Join(dir, filepath.Dir(rel)), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	content := "class RequestMeta:\n    id = 1\n"
+	if err := os.WriteFile(filepath.Join(dir, rel), []byte(content), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	git("add", ".")
+	git("commit", "-m", "init")
+	commit := getHeadCommit(t, dir)
+
+	// Use the git-style forward-slash path the diff/LLM would supply.
+	gitPath := "subproject1/src/models/request_meta.py"
+
+	// git-show (commit mode): the exact path from the issue error.
+	frShow := &FileReader{RepoDir: dir, Mode: ModeCommit, Ref: commit}
+	got, err := frShow.Read(context.Background(), gitPath)
+	if err != nil {
+		t.Fatalf("commit-mode Read(%q) error: %v", gitPath, err)
+	}
+	if got != content {
+		t.Errorf("commit-mode Read = %q, want %q", got, content)
+	}
+
+	// disk (workspace mode): same root-relative path resolves too.
+	frDisk := &FileReader{RepoDir: dir, Mode: ModeWorkspace}
+	gotDisk, err := frDisk.Read(context.Background(), gitPath)
+	if err != nil {
+		t.Fatalf("workspace-mode Read(%q) error: %v", gitPath, err)
+	}
+	if gotDisk != content {
+		t.Errorf("workspace-mode Read = %q, want %q", gotDisk, content)
+	}
+}