Skip to content

fix: honor --repo pointing at a module's own .code-review-graph dir#701

Open
Khusan-Khujakeldiev wants to merge 1 commit into
tirth8205:mainfrom
Khusan-Khujakeldiev:fix/repo-flag-monorepo-graph-tools
Open

fix: honor --repo pointing at a module's own .code-review-graph dir#701
Khusan-Khujakeldiev wants to merge 1 commit into
tirth8205:mainfrom
Khusan-Khujakeldiev:fix/repo-flag-monorepo-graph-tools

Conversation

@Khusan-Khujakeldiev

Copy link
Copy Markdown

Fixes #697.

Problem

search/query/impact (and the rest of _GRAPH_TOOL_COMMANDS) resolve an
explicit --repo via find_project_root(), which only walks up looking for
.git/.svn. In a monorepo with a single .git at the root and independently
build/register-ed module subdirectories (each carrying its own
.code-review-graph marker but no .git of their own — e.g. llvm-project's
llvm/ and clang/), this silently climbs past the module's own graph to the
monorepo root and reports "No graph found" even though the graph exists
exactly where --repo pointed.

Fix

When --repo points directly at a directory that already has its own
.code-review-graph subdirectory, honor that path as-is instead of using it
only as a starting point for the upward git-root walk. This mirrors how
build/register/status already treat .code-review-graph as sufficient
evidence of a project root via tools._common._validate_repo_root.

When --repo has no .code-review-graph of its own, behavior is unchanged
(falls through to the existing find_project_root walk), so the existing
"nested subdirectory of a single repo" case is unaffected.

Testing

  • Added test_tool_command_honors_repo_flag_over_ancestor_git_in_monorepo,
    which reproduces the monorepo shape (one .git at the top, a module
    subdirectory with its own .code-review-graph/graph.db) and asserts
    search --repo <module> resolves to <module>, not the ancestor with
    .git. Verified it fails on main and passes with this fix.
  • Ran the full test suite locally on Windows before and after the change;
    no new failures (only pre-existing, unrelated Windows-only failures around
    chmod/symlink privileges and git submodule fixtures remain, present on
    main too).
  • Verified against the real-world case that surfaced this: a 21-module,
    ~180K-file llvm-project checkout built/registered module-by-module with
    CRG, where search/query callers_of/impact --files now return correct
    results with --repo pointing at a module (e.g. the largest module,
    llvm/, ~80K files).

search/query/impact (and other _GRAPH_TOOL_COMMANDS) resolved an explicit
--repo via find_project_root(), which only walks *up* looking for .git/.svn.
In a monorepo with a single .git at the root and independently
build/register-ed module subdirectories (each carrying its own
.code-review-graph marker but no .git of their own, e.g. llvm-project's
llvm/ and clang/), this silently climbed past the module's own graph to
the monorepo root and reported 'No graph found' even though the graph
existed exactly where --repo pointed.

Now, when --repo points directly at a directory that already has its own
.code-review-graph subdirectory, that path is honored as-is instead of
being used only as a starting point for the upward git-root walk. This
mirrors how build/register/status already treat .code-review-graph as
sufficient evidence of a project root via tools._common._validate_repo_root.

No change when --repo has no .code-review-graph of its own (falls through
to the existing find_project_root walk), so existing subdirectory-of-a-
single-repo behavior is unaffected.

Adds a regression test that reproduces the monorepo shape and asserts
search --repo <module> resolves to <module>, not an ancestor with .git.

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

search/query/impact ignore explicit --repo inside a monorepo (git-root walk overrides it)

1 participant