|
| 1 | +"""v1.108.164: a path-shaped `repo` arg no longer escapes as a raw storage ValueError. |
| 2 | +
|
| 3 | +Origin: #376 (split out of #375), reported from a daily Ubuntu install. An agent that |
| 4 | +doesn't have an owner/name id at hand guesses a path. A two-segment path split on the |
| 5 | +FIRST separator left a `name` still carrying one, which the store rejects at |
| 6 | +`_safe_repo_component` — outside every caller's handler, so it surfaced as a hard |
| 7 | +`ValueError: Path separator in name` instead of a routed tool error. |
| 8 | +""" |
| 9 | + |
| 10 | +from types import SimpleNamespace |
| 11 | + |
| 12 | +import pytest |
| 13 | + |
| 14 | +from jcodemunch_mcp.tools import _utils |
| 15 | +from jcodemunch_mcp.tools import resolve_repo as rr |
| 16 | + |
| 17 | + |
| 18 | +def _store_with(repos=(), present=False): |
| 19 | + return SimpleNamespace( |
| 20 | + inspect_index=lambda o, n: SimpleNamespace(index_present=present), |
| 21 | + list_repos=lambda: list(repos), |
| 22 | + ) |
| 23 | + |
| 24 | + |
| 25 | +class TestMultiSegmentRepoArg: |
| 26 | + def test_file_path_raises_actionable_error_not_storage_valueerror( |
| 27 | + self, monkeypatch |
| 28 | + ): |
| 29 | + monkeypatch.setattr(rr, "_compute_repo_id", lambda p, store=None: "") |
| 30 | + monkeypatch.setattr( |
| 31 | + _utils, "IndexStore", lambda base_path=None: _store_with() |
| 32 | + ) |
| 33 | + with pytest.raises(ValueError) as exc: |
| 34 | + _utils.resolve_repo("src/auth/service.ts") |
| 35 | + message = str(exc.value) |
| 36 | + assert "Path separator in name" not in message |
| 37 | + assert "is a path, not a repository id" in message |
| 38 | + assert "resolve_repo" in message and "index_folder" in message |
| 39 | + |
| 40 | + def test_file_shaped_arg_names_file_pattern(self, monkeypatch): |
| 41 | + monkeypatch.setattr(rr, "_compute_repo_id", lambda p, store=None: "") |
| 42 | + monkeypatch.setattr( |
| 43 | + _utils, "IndexStore", lambda base_path=None: _store_with() |
| 44 | + ) |
| 45 | + with pytest.raises(ValueError) as exc: |
| 46 | + _utils.resolve_repo("src/auth/service.ts") |
| 47 | + assert "file_pattern='src/auth/service.ts'" in str(exc.value) |
| 48 | + |
| 49 | + def test_directory_shaped_arg_omits_the_file_pattern_hint(self, monkeypatch): |
| 50 | + # No suffix means the agent meant a checkout, not a file to scope to. |
| 51 | + monkeypatch.setattr(rr, "_compute_repo_id", lambda p, store=None: "") |
| 52 | + monkeypatch.setattr( |
| 53 | + _utils, "IndexStore", lambda base_path=None: _store_with() |
| 54 | + ) |
| 55 | + with pytest.raises(ValueError) as exc: |
| 56 | + _utils.resolve_repo("local/foo/bar") |
| 57 | + assert "file_pattern" not in str(exc.value) |
| 58 | + |
| 59 | + def test_backslash_tail_also_routed(self, monkeypatch): |
| 60 | + monkeypatch.setattr(rr, "_compute_repo_id", lambda p, store=None: "") |
| 61 | + monkeypatch.setattr( |
| 62 | + _utils, "IndexStore", lambda base_path=None: _store_with() |
| 63 | + ) |
| 64 | + with pytest.raises(ValueError) as exc: |
| 65 | + _utils.resolve_repo("local/foo\\bar") |
| 66 | + assert "is a path, not a repository id" in str(exc.value) |
| 67 | + |
| 68 | + def test_relative_path_that_is_indexed_resolves(self, monkeypatch, tmp_path): |
| 69 | + # 'apps/web/src' is too conservative for _looks_like_path, but if it really |
| 70 | + # is an indexed checkout we should resolve it rather than error. |
| 71 | + sub = tmp_path / "apps" / "web" / "src" |
| 72 | + sub.mkdir(parents=True) |
| 73 | + monkeypatch.chdir(tmp_path) |
| 74 | + monkeypatch.setattr(rr, "_compute_repo_id", lambda p, store=None: "") |
| 75 | + monkeypatch.setattr( |
| 76 | + _utils, |
| 77 | + "IndexStore", |
| 78 | + lambda base_path=None: _store_with( |
| 79 | + repos=[{"repo": "local/web", "source_root": str(sub)}] |
| 80 | + ), |
| 81 | + ) |
| 82 | + assert tuple(_utils.resolve_repo("apps/web/src")) == ("local", "web") |
| 83 | + |
| 84 | + |
| 85 | +class TestNoRegressionOnRealIds: |
| 86 | + def test_owner_name_id_untouched(self): |
| 87 | + assert tuple(_utils.resolve_repo("local/gin")) == ("local", "gin") |
| 88 | + |
| 89 | + def test_single_slash_missing_repo_keeps_its_own_error_path(self, monkeypatch): |
| 90 | + # One separator is a well-formed id shape; it must NOT be re-routed as a |
| 91 | + # path, so the caller still gets the index-not-loadable error downstream. |
| 92 | + monkeypatch.setattr( |
| 93 | + _utils, "IndexStore", lambda base_path=None: _store_with() |
| 94 | + ) |
| 95 | + assert tuple(_utils.resolve_repo("tools/agents.ts")) == ( |
| 96 | + "tools", |
| 97 | + "agents.ts", |
| 98 | + ) |
| 99 | + |
| 100 | + |
| 101 | +class TestSearchSymbolsSurfacesItInBand: |
| 102 | + def test_path_shaped_repo_returns_error_dict(self, monkeypatch): |
| 103 | + from jcodemunch_mcp.tools.search_symbols import search_symbols |
| 104 | + |
| 105 | + monkeypatch.setattr(rr, "_compute_repo_id", lambda p, store=None: "") |
| 106 | + monkeypatch.setattr( |
| 107 | + _utils, "IndexStore", lambda base_path=None: _store_with() |
| 108 | + ) |
| 109 | + result = search_symbols(repo="src/auth/service.ts", query="login") |
| 110 | + assert isinstance(result, dict) |
| 111 | + assert "is a path, not a repository id" in result["error"] |
0 commit comments