Skip to content

Deduplicate project indexes by git/path identity#754

Closed
Andy11-cpu wants to merge 1 commit into
DeusData:mainfrom
Andy11-cpu:fix/project-dedup
Closed

Deduplicate project indexes by git/path identity#754
Andy11-cpu wants to merge 1 commit into
DeusData:mainfrom
Andy11-cpu:fix/project-dedup

Conversation

@Andy11-cpu

Copy link
Copy Markdown
Contributor

Summary

  • Add project_resolve module to match repos by git canonical root or canonical path
  • Reuse existing cached project names when the same repo is opened from a different path
  • MCP session detection and pipeline creation both use the resolver

Motivation

Split from #625 — prevents duplicate .db files when the same repository is indexed from multiple paths.

Test plan

  • test_project_resolve.c (canonicalize, identity key, find existing, pipeline reuse)
  • make -f Makefile.cbm test (CI)

Signed-off-by: Andy11-cpu canada11@duck.com

@Andy11-cpu Andy11-cpu requested a review from DeusData as a code owner July 2, 2026 01:56
@DeusData DeusData added bug Something isn't working ux/behavior Display bugs, docs, adoption UX stability/performance Server crashes, OOM, hangs, high CPU/memory priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. labels Jul 3, 2026
@DeusData

DeusData commented Jul 3, 2026

Copy link
Copy Markdown
Owner

Thanks for splitting out project identity deduping. Triage: high-priority path/project identity fix, but the PR is currently conflicting.

Please rebase on current main before deeper review. The key review point will be avoiding project-name drift while still deduplicating symlink/path/git-root identities, especially around relative paths and existing cache entries.

@DeusData

DeusData commented Jul 3, 2026

Copy link
Copy Markdown
Owner

Thank you — the problem is real (duplicate project indexes for the same repo) and splitting this out of #625 was the right move. After a careful review we can't merge the current identity key, though, and want to be specific about why: canonical_root is derived from git's common-dir and is deliberately identical for a main repo and all its linked worktrees (that's what it was built for in #749 — branch-level identity). Used as project identity, indexing a worktree resolves to the main repo's project and reuses/overwrites its DB — silently breaking multi-worktree workflows (which we use heavily ourselves). Two more issues: identity_nested in the reverse direction means opening a monorepo root adopts a child project's name and its incomplete graph; and the scan opens every cached .db + runs a git subprocess per project on every pipeline creation and session-detect, which is costly and invasive (read-write opens of other projects' stores). CI is currently red on all five platforms and the branch conflicts with main. Concrete path to merge: (1) key identity per-worktree — canonicalized worktree root, or (canonical_root, worktree_path) pair — so worktrees stay distinct projects; (2) drop the reverse-nesting adoption; (3) cache the identity scan (or restrict it to index-time, not every session-detect). Also note #625 declares itself gated on this PR, so it inherits the same rework. If you'd rather not carry the redesign we completely understand — say the word and we'll distill the per-worktree variant with credit to you. Thanks for pushing on a real pain point.

rarepops pushed a commit to rarepops/codebase-memory-mcp that referenced this pull request Jul 4, 2026
…on (default on)

Add the auto_watch config key (default: true) and route both session
watcher-registration sites in mcp.c (autoindex completion and the
already-indexed connect path) through a register_watcher_if_enabled()
helper. When disabled, the skip is logged as a short structured line
(watcher.register.skipped reason=auto_watch_off).

Default is TRUE (opt-out): existing users keep background auto-sync
unchanged; the knob exists for multi-project users who want each
session contained to explicit indexing.

Distilled from the auto_watch half of PR DeusData#625. The PR's bundled
project-index dedup commit is excluded here — it belongs to DeusData#754's
own review. Unlike the original PR, no watch registration is added
to the index_repository handler (none exists on main; adding one
would be new behavior beyond the gate), and the default is flipped
from off to on to preserve current behavior.

Tests: default-unset registers the watcher on connect (guards the
no-behavior-change promise); auto_watch=false registers nothing —
verified RED against the ungated production code, GREEN with the gate.

Co-authored-by: Andy11-cpu <canada11@duck.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Reuse an existing cached project when the same repository is opened from a different path. Match by git canonical root or canonical filesystem path so duplicate .db files are not created.

Signed-off-by: Andy11-cpu <canada11@duck.com>
@Andy11-cpu Andy11-cpu force-pushed the fix/project-dedup branch from f7b112e to 9376ca1 Compare July 4, 2026 15:40
@Andy11-cpu

Copy link
Copy Markdown
Contributor Author

Rebased on current main and reworked per your review. Summary of what changed:

1. Per-worktree identity (not canonical_root)
cbm_project_identity_key now uses the canonicalized git worktree root (rev-parse --show-toplevel + realpath/_fullpath). Linked worktrees keep distinct identity keys even when canonical_root matches. Symlinks / alternate paths into the same worktree still dedupe.

2. Dropped reverse nesting
cbm_find_existing_project_name only matches when the query path equals or is a subdirectory of an indexed root. Opening a monorepo root no longer adopts a child project name/DB.

3. Cached identity scan
The cache-dir scan runs once (read-only via cbm_store_open_path_query) and is held in memory until cbm_project_identity_cache_invalidate(). Invalidated after a successful full or incremental cbm_pipeline_run. Session detect still calls the resolver but hits the cache after the first load.

4. CI / conflict fixes

  • Resolved Makefile.cbm conflict (test_index_resilience.c + test_project_resolve.c both kept).
  • Fixed test_project_resolve.c compile errors (helpers, no ASSERT in void callbacks).
  • Added project_resolve_worktrees_distinct test.

Commit: 9376ca1 on fix/project-dedup. CI should be re-running now.

@DeusData

DeusData commented Jul 4, 2026

Copy link
Copy Markdown
Owner

Thank you for tackling this — index duplication is a real annoyance and the git-worktree-identity idea is a good instinct. After a careful review we're going to close this one, and I want to be transparent about why:

  1. The safe cases are already handled. Normalize project paths for ADR lookups #780 (merged) canonicalizes the repo path inside project-name derivation (realpath), so path-spelling variants (trailing slash, ./, ../) and symlinked spellings already resolve to the same project + .db — no duplicate index.

  2. The one genuinely-new behavior carries a data-loss risk. The remaining novel case here is 'indexing a subdirectory of an already-indexed repo reuses the whole-repo project.' Via identity_is_child, indexing a nested/submodule repo (e.g. /repo/packages/api under an indexed /repo) reuses the parent's project name → the new repo resolves onto the parent's .db → the incremental-mismatch path cbm_unlinks and rebuilds it, destroying the original project's index. It's enabled by default with no nested-.git guard, and it also blocks a user from deliberately indexing a subdirectory as its own scope.

  3. There's also a build issue (project_resolve.c:40 uses an undeclared buffer and writes to the wrong variable, so dedup is a no-op on Linux/macOS; CI's test job was skipped behind the failing lint/security gates).

Given #780 already covers the safe dedup and the new behavior is the risky part, we don't want to ship the auto-merge-and-delete identity model. If you'd like to pursue a safe version — detect a likely-duplicate index and warn (or offer an explicit, opt-in merge) rather than silently merging + unlinking — that'd be a welcome, much lower-risk direction and we'd happily review it. Really appreciate the effort here.

@DeusData DeusData closed this Jul 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. stability/performance Server crashes, OOM, hangs, high CPU/memory ux/behavior Display bugs, docs, adoption UX

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants