Conversation
|
Thanks for opening this — it has been seen, and it is queued. This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence. Current review status: working through a backlog. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
If this fixes a bug, a reproduction we can run is worth more than a description of the symptom. Thanks for contributing, and sorry in advance for the wait. |
1003528 to
d21918c
Compare
|
Thank you for the focused |
|
@vitalNohj — the behavior review is done, and the answer is accepted: both surfaces ship — the Context you deserve on why this needed a maintainer decision at all: there is a parked direction (see #573) where worktrees eventually get a cheap view keyed by the git common dir rather than nothing. Your key is opt-in-off, so it forecloses none of that — and your commondir plumbing is genuinely reusable for that family-identity work later. But "skip worktrees on purpose" and "worktrees get an overlay" are opposite product answers to the same pain, so shipping yours ahead of that decision was a call, not a formality. It is now made, in your favour. The rework map — one item is invisible and load-bearing:
The one red CI leg on your final SHA ( Take your time; the acceptance does not expire. This is a first contribution built the way we wish more were. |
077e9f1 to
6bb8ad6
Compare
Agents that use `git worktree add` to run parallel branches end up with one
indexed project per worktree. Each is a near-duplicate of the main checkout,
so the cache fills with redundant graphs and project pickers get noisy.
Add an opt-in `ignore_worktrees` config key. When enabled, a linked worktree
is left alone:
- auto-index on connect skips it (logged as autoindex.skip)
- the daemon's background auto-index skips it
- explicit index_repository refuses with an actionable message naming both
ways forward (index_worktree=true, or turn the key off)
- the hook-augment "not indexed" guidance stops telling the agent to run
index_repository in a directory where it would only be refused
Detection is git plumbing, no subprocess: <path>/.git must be a regular file
holding a "gitdir:" pointer AND that gitdir must contain a `commondir` file.
The commondir check is what separates a linked worktree from a submodule,
whose .git is also a gitlink file but whose gitdir has no commondir. The main
checkout of a repo that has worktrees is never affected.
Defaults to false, so behaviour is unchanged unless the key is set.
Signed-off-by: Nohj <vitalnohj@gmail.com>
6bb8ad6 to
0d2f840
Compare
DeusData
left a comment
There was a problem hiding this comment.
@vitalNohj, thank you, sincerely. You took a rework map with one invisible, untested trap at the top of it and came back with every item done, plus the tests we only asked for "if you can". I'm sorry, too: this landed on 09-23 and we didn't notice for two days. That's on us, not on you.
What we checked on 0d2f840c, item by item against the map:
CONFIG_KEYSrow: present insrc/cli/cli.cwith defaultfalse, mirroring the runtime reader. It's pinned by your newcli_ignore_worktrees_config_key_is_discoverable, so the enable command your docs and refusal message suggest now works.- Rebase: the branch merges cleanly with today's
main. - Default-off pin:
mcp_ignore_worktrees_default_off_still_indexes_worktreecovers the direction that was missing. We also checked the code: all four call sites (explicit call, in-process auto-index, daemon auto-index, hook guidance) short-circuit on the config read before any filesystem probe. With the key unset, today's behaviour is untouched. - Auto-index coverage,
th_mktempdir, SKIP-not-PASS: all done. Your two auto-index tests drive the realinitializepath with the count hook as the observable, which is exactly the shape we'd have asked for.
We also re-read the detector: the commondir discriminator, relative-pointer resolution, and fail-open on every probe failure. We confirmed the new read goes through cbm_fopen, that the change adds no raw allocation sites to src/, and that nothing in the diff goes beyond the feature.
Two optional wording nits, entirely up to you and fine to leave:
- The one-line help text and the docs table row say "when indexing automatically", but the key also refuses explicit
index_repositorycalls. The section below the table already explains that, so something like "Skip linked git worktrees when indexing (explicit calls need index_worktree=true)" would make the one-liner match. - The explicit default-off test asserts "not refused" rather than a successful index. The auto-index default-off test already covers the positive side, so this is only a note.
We'll run a fresh CI pass against current main and merge from there. Thank you again. This is a contribution built the way we wish more were, and it's already noted for the worktree-family work in #573.
Problem
Agent workflows increasingly use
git worktree addto run several branches of the same repo side by side. Every worktree is discovered as its own project, so the cache accumulates a near-duplicate graph per worktree and project pickers get noisy. Today there is no way to say "index the main checkout, leave the worktrees alone".Change
Adds an opt-in
ignore_worktreesconfig key. When enabled, a linked worktree is left alone across every indexing entry point:autoindex.skipwithreason=linked_worktree)index_repositoryrefuses with an actionable message naming both ways forwardindex_repositoryin a directory where it would only be refusedThe explicit call keeps an escape hatch —
index_worktree=trueindexes a worktree regardless of the setting, so the key never becomes a dead end:Default is
false, so nothing changes for existing users unless they opt in.Detection
cbm_git_is_linked_worktree()is git plumbing only, no subprocess — it runs on every session start, so shelling out the waycbm_git_context_resolve()does was not an option.<path>/.gitmust be a regular file holding agitdir:pointer and that gitdir must contain acommondirfile.That second condition is the important one: a submodule's
.gitis also a gitlink file, so a naive "is.gita regular file?" check misclassifies every submodule as a worktree. A submodule's gitdir (<super>/.git/modules/<name>) has nocommondirentry, which separates the two cleanly. The main checkout of a repo that has worktrees is never affected.Tests
tests/test_git_context.c— detection truth table: linked worktree true; the main checkout of that same repo false; a submodule false; a plain non-git dir false.tests/test_mcp.c— the config gate on explicitindex_repository: worktree refused, main checkout not refused, andindex_worktree=trueescaping the gate.Both follow the existing fixture patterns in those files (
th_mktempdir/th_rmtree,SKIP_PLATFORMon Windows, graceful skip when git is unavailable).Verification
scripts/build.sh— cleanscripts/test.sh— 6661 passed, 0 failed (116 suites)make -f Makefile.cbm security— all 8 layers passlint-format) and cppcheck — clean; clang-tidy reports 0 findings on the added linesindex_worktree=trueoverride, and unchanged default-off behaviourDocs
docs/CONFIGURATION.mdgets a table row plus a section covering motivation, the enable command, all three behaviours, and the submodule/main-checkout caveat.README.mdgets a short paragraph next toauto_watch.