Repository navigation
fix(mcp): reuse the project that owns a root on unnamed re-index (#2134) - #2344
Merged
Merged
Conversation
index_repository without `name` derived the project key from the path alone. When the root had first been indexed under an explicit name, a later unnamed re-index forked a second index (same root_path, path-derived name) and left the original untouched, so the stale index kept being served while the call reported success. Resolve the key from the indexes on disk before any work starts: - the path-derived project exists -> keep it (unchanged behavior); - exactly one other project owns the canonical root -> adopt its name and update it in place, so existing indexes are never orphaned; - several projects own the root -> fail loudly, name them and ask for `name` instead of guessing or forking again; - none -> first index under the derived name, as before. The resolved name is written into the args handed to the daemon job coordinator and the supervised worker, so the project lease key and the index actually written stay the same project. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The memory-core ratchet flagged src/mcp/mcp.c growing by 10 raw allocator sites (784 -> 794) from index_root_owner_resolve and its caller. The owner list buffer, which this code allocates and frees itself, now uses cbm_realloc/cbm_free (class other). Strings handed over by other APIs (cbm_project_name_from_path, heap_strdup, the index argument strings) are released through safe_free, as elsewhere in the file, so no memory the core never counted reaches cbm_free. Behaviour is unchanged; mcp.c is back at its baseline of 784. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
…e test path Linux, MSan, TSan and diag CI failed in mcp_mutation_guard: tool_cross_repo_missing_inputs_fail_without_creating_ghost_databases ASSERT(source_failed). handle_index_repository resolved the root owner before dispatching cross-repo-intelligence mode. That test registers its named target with the source's own root, so the unindexed source adopted existing-cross-target as its project (log: index.root_owner_reused project=existing-cross-target), and the missing source was no longer reported. Cross-repo mode reads an existing source index and never writes one, so the #2134 reuse now runs only on the re-index path, after that dispatch. macOS CI stayed green because cbm_tmpdir() is /tmp, a symlink to /private/tmp there, so the canonical repo_path never equalled the fixture's stored root; with the fixture path canonicalized, the failure reproduced 3/3 on macOS and passes 3/3 with this change. The #2134 test itself called realpath() into a 1 KiB buffer. glibc's FORTIFY check aborts on that ("buffer overflow detected", a SIGABRT of the mcp suite on the ubuntu 2/3 shards), and MinGW has no realpath at all (Windows compile error). It now uses cbm_canonical_path with a 4 KiB buffer, the same canonicalizer production uses. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
tool_index_repository_reuses_existing_project_for_root_issue2134 counts how often the canonical repo root occurs in list_projects. On Windows, cbm_canonical_path returns "C:\Users\...", while the stored root_path is "C:/Users/...", so the count was 0 and the test failed on the CLANG64 leg (test_mcp.c:8448: owners == 0, expected 1). Normalize the expected root with cbm_normalize_path_sep, a no-op on POSIX, before counting. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
With #2134, an unnamed index_repository of a root that another indexed project already owns adopts that owner's name, and the daemon keys the job by it (application_index_project_key reads the forwarded "name"). The unnamed status poll that the #2144 async reply asks for ("the same repo_path and status: true") still derived its key from the path alone in index_status_project_key. For an owned root it never found the job it had just started and answered "no index job or index is known for project '<path-derived>'"; an ambiguous root got that same misleading answer instead of the loud error the start returns. index_status_project_key now runs the start path's index_root_owner_resolve on the same canonical repo_path: an explicit name still wins, a single other owner is the key, several owners fail with the same error naming them, otherwise the path-derived key stays. The status error buffer grows to hold that owner list. The daemon's other readers of the key (job dedupe in application_job_subscribe_locked, the synchronous cut-short marker set and taken around application_index_run_sync) derive it from the forwarded args, which already carry the owner name, so they agree with the start; status was the only reader that diverged. Refs #2134, #2144 Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
DeusData
force-pushed
the
fix/issue-2134
branch
from
September 30, 2026 22:30
b205f0b to
41b1aea
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
index_repositorywithoutnamederived the project key from the path only. So re-indexing a root first indexed under an explicit name forked a second index with the same root_path, and the original stayed stale and was still served. The key is now resolved from the indexes on disk:name;The resolved name is written into the daemon/worker args, so the lease key and the written index match.
Test:
mcp::tool_index_repository_reuses_existing_project_for_root_issue2134(fails before the fix, passes after, fails again with the fix reverted; also covers the ambiguous case). Proven end-to-end with the CLI binary: one project and one .db after an unnamed re-index, where there used to be two.Notes for review:
handle_cross_repo_modenow also gets the resolved owner name whennameis missing (previously always the path-derived name).srv->session_project, set from the path-derived name) is not changed here; applying the same owner lookup there is a possible follow-up.src/mcp/mcp.calongside fix(mcp): resolve projects by their non-ASCII folder name (#1827) #2331 and fix(mcp): translate Git-root paths for subdirectory projects in detect_changes (#1951) #2334 (different functions).Follow-up for #2144 (rebased onto main after it merged): the unnamed
index_repository(status: true)poll now resolves its key the same way as the unnamed start, owner first, then path-derived. For a root owned under another name, polling with the samerepo_pathnow finds the async job that was just started. An ambiguous root gets the same "several indexed projects share root_path" error on both calls.Fixes #2134