Skip to content

fix(mcp): resolve index_repository's repo_path from a known project name - #1258

Open
AmirF194 wants to merge 1 commit into
DeusData:mainfrom
AmirF194:fix/1211-index-repository-project-name
Open

fix(mcp): resolve index_repository's repo_path from a known project name#1258
AmirF194 wants to merge 1 commit into
DeusData:mainfrom
AmirF194:fix/1211-index-repository-project-name

Conversation

@AmirF194

@AmirF194 AmirF194 commented Jul 25, 2026

Copy link
Copy Markdown

What does this PR do?

handle_index_repository requires repo_path and errors with "repo_path is required" when it is absent. list_projects only ever advertises the project NAME, never repo_path, and every read tool accepts that name back through get_project_arg's project/project_name/project_id/projectName aliases (mcp.c:1385), but index_repository did not, so re-indexing an already-indexed project by the name list_projects gave you had no resolution path and failed immediately, even though the project's root was already on file.

Fix: added resolved_repo_path_from_project_arg(), a query-only lookup of the project's own stored root_path, tried when repo_path is absent before giving up. It never touches srv->store or srv->current_project, so it cannot disturb whatever project the server already has cached, and an unresolvable name still falls through to the original, correct "repo_path is required" error.

Verified in a clean ubuntu:24.04 container (TEST_SUITES=mcp make -f Makefile.cbm test-focused):

  • New test tool_index_repository_resolves_root_path_from_project_name_issue1211 fails on unpatched main (strstr(resp, "repo_path is required") is not NULL) and passes with the fix.
  • Companion test tool_index_repository_unknown_project_name_still_requires_repo_path guards the fallback: a name with no stored project still gets the exact original error, not a resolver crash or silent no-op.
  • Full mcp suite: 179 passed on both sides, one pre-existing, unrelated failure (tool_detect_changes_contained_commands_clean_up_error_and_success, test_mcp.c:5814) reproducing identically on unmodified origin/main, so not introduced by this change.
  • scripts/lint.sh --ci CLANG_FORMAT=clang-format-20 and scripts/security-audit.sh both clean.

Refs #1211 (this PR fixes only the repo_path-required/name-resolution bug the issue was filed for; a separate, unrelated defect was reported in the thread afterward: CBM_ALLOWED_ROOT refusals get mis-reported as a worker crash. Leaving the issue open rather than auto-closing it on merge.)

Checklist

  • Every commit is signed off (git commit -s)
  • Tests pass locally (make -f Makefile.cbm test, focused on the mcp suite)
  • Lint passes (make -f Makefile.cbm lint-ci)
  • New behavior is covered by a test (reproduce-first for bug fixes)

handle_index_repository requires repo_path and errors with
"repo_path is required" if it is absent. list_projects only ever
advertises the project NAME, never repo_path, and every read tool
accepts that name back through get_project_arg's project/project_name/
project_id/projectName aliases -- but index_repository did not, so
re-indexing an already-indexed project by the name list_projects gave
you had no resolution path and failed immediately.

Add resolved_repo_path_from_project_arg(): a query-only lookup of the
project's own stored root_path, tried when repo_path is absent before
giving up. It never touches srv->store or srv->current_project, so it
cannot disturb whatever project the server already has cached, and an
unresolvable name still falls through to the original, correct
"repo_path is required" error.

Verified in a clean ubuntu:24.04 container (TEST_SUITES=mcp make -f
Makefile.cbm test-focused): the new
tool_index_repository_resolves_root_path_from_project_name_issue1211
fails on unpatched main (strstr(resp, "repo_path is required") is
not NULL) and passes with the fix; the companion
tool_index_repository_unknown_project_name_still_requires_repo_path
guards the fallback stays exact for a name that was never indexed.
Full mcp suite: 179 passed on both sides, with one pre-existing,
unrelated failure (tool_detect_changes_contained_commands_clean_up_error_and_success,
test_mcp.c:5814) reproducing identically on unmodified origin/main.
scripts/lint.sh --ci and scripts/security-audit.sh both clean.

Fixes DeusData#1211

Signed-off-by: Amir Fathi <amirfathi.me@gmail.com>
@AmirF194
AmirF194 requested a review from DeusData as a code owner July 25, 2026 07:18
@DeusData DeusData added bug Something isn't working editor/integration Editor compatibility and CLI integration ux/behavior Display bugs, docs, adoption UX priority/normal Standard review queue; useful PR with ordinary maintainer urgency. labels Jul 28, 2026
@DeusData DeusData added this to the 0.9.1-rc milestone Jul 28, 2026
@DeusData

DeusData commented Jul 28, 2026

Copy link
Copy Markdown
Owner

Thank you for the contribution and for isolating the list_projects and index_repository mismatch with regression coverage. This is now triaged as a 0.9.1-rc integration bug at normal priority. Our community PR queue is currently quite full, so it may take a little time before we can complete the review and, if approved, merge it. We are doing our best to support community contributions and will return with code-grounded feedback as capacity opens.

@AmirF194

Copy link
Copy Markdown
Author

Thanks for the update, no rush. Happy to adjust the fix if the 0.9.1-rc review turns up anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working editor/integration Editor compatibility and CLI integration priority/normal Standard review queue; useful PR with ordinary maintainer urgency. ux/behavior Display bugs, docs, adoption UX

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants