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>
This branch has not been deployed
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
1 participant
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).Fixes #2134