update: the automatic full-rebuild fallback bypasses the root-mismatch guard and can replace another repository's graph

[tirth8205]

Found in a read-only audit of v2.3.8. Verified against current main.

What happens

incremental_update refuses to reconcile a graph whose stored File nodes all live under a different root (code_review_graph/incremental.py:1372, the guard added for #889). The update tool does not always reach it.

code_review_graph/tools/build.py resolves an automatic diff base first, and when no usable anchor exists it sets full_rebuild = True and calls full_build() instead. full_build reconciles stale files with no root check, so a graph anchored to another repository is emptied and replaced rather than refused.

Reproduction

Build repository A into a shared data directory, then run an ordinary update for repository B with no usable stored SHA. The graph goes from holding only A/a.py to holding only B/b.py, with no mismatch error.

Why it matters

This is the same data-loss shape #889 fixed, reachable through the default path rather than an unusual spelling. The loss is regenerable graph data, never source files, which is why this is high and not critical.

Suggested fix

Apply the same guard before the fallback rebuild — a full build that is chosen automatically (rather than requested with --full) should refuse a graph anchored elsewhere, exactly as the incremental path does. An explicit full_rebuild=True from the user can keep its current wipe-and-rebuild behavior.

[lavaxun]

Adding our own stakes on this, since it's currently the one thing blocking us from adopting
crg-daemon: the full-rebuild fallback being able to replace another repository's graph
(rather than only the one that triggered the fallback) is a much bigger risk for us than it
might look in isolation, because of how we're set up.

We run a multi-repo registry (~/.code-review-graph/registry.json) with one row per checkout,
and we routinely run many checkouts at once — every in-flight branch gets its own git worktree, each registered separately. code-review-graph register defaults a repo's alias to
basename(repo_root), and our worktree tooling derives worktree directory names from a branch
or task name, so nothing in our setup guarantees those basenames stay distinct — two checkouts
named for the same task, or two different repositories sharing a basename, collide by
construction. Our registry currently holds 11 rows with 11 distinct aliases, so we are not
looking at a live collision today; we treat it as a structural property rather than a current
incident. It was still enough for us to make our own registry sweep unregister strictly by
absolute path, never by alias.

That matters here because the registry is genuinely multi-repository — ours holds rows for
several unrelated projects alongside worktrees of our monorepo. A fallback path that resolves a
repository ambiguously is therefore choosing among real, unrelated graphs for us, not among
variants of one.

Until this is closed, we're staying on the per-invocation update/build model (git hooks
calling the CLI directly, one call per repo per event) instead of the daemon — the risk of a
fallback rebuild silently clobbering a sibling checkout's graph outweighs what the daemon would
save us. Happy to share more detail on our registration/alias setup if it helps pin down the
resolution path, or to test a fix against our multi-repo registry once one lands.

[tirth8205]

@lavaxun that is a useful escalation and it changes how I read this bug, so thanks for the detail. Two separate things in your comment.

On this issue. The guard is still missing on staging. full_build() reconciles stale files with no root check, and tools/build.py can reach it automatically when no usable diff anchor exists, so the #889 guard at incremental.py:1372 is bypassed on the default path. PR #924 (@yzxcj797) implements the fix: apply the same root check before an automatically chosen full rebuild, while an explicit --full keeps its wipe-and-rebuild behaviour. It is open and mergeable but behind staging, so it needs a rebase before it can land. That is the whole remaining distance on this one.

Your offer to test against a real multi-repo registry is worth taking up. When #924 lands on staging I will ping this thread, because the shape you describe (11 rows, unrelated projects plus worktrees of one monorepo) exercises it in a way my single-repo fixtures do not.

On the alias collision. That is a different defect and it deserves its own issue rather than being absorbed into this one. register defaulting an alias to basename(repo_root) with no uniqueness check means two worktrees named for the same task, or two unrelated repos with the same basename, collide by construction. Your workaround of unregistering strictly by absolute path is the right one. If you file it with the registry layout you described, I will treat it as a separate bug; if you would rather I file it, say so and I will.

Staying on per-invocation update/build until #924 lands is the right call.