From e89e6a24de2400c9e1159f7fa20eba8f21ad9968 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 22 Oct 2026 16:18:03 -0400 Subject: [PATCH] =?UTF-8?q?fix(6245):=20bind=5Frepo(moved=5Ffrom=3D?= =?UTF-8?q?=E2=80=A6)=20moves=20the=20snippet=20locations=20recorded=20in?= =?UTF-8?q?=20the=20old=20repo?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A repo renamed on its forge kept every snippet location naming the old repo: the binding and the shape ledger moved, the snippets did not. - snippets.move_location_repo rewrites, on every live snippet of the project, each location whose repo is the old remote in any form (URL, normalized key, "owner/Repo", bare "Repo", compared without case) to the new normalized key. A snippet the caller can only read is listed in read_only and left alone. - Its loop is shared with move_locations through _rewrite_locations, which saves each changed snippet through update_snippet. - bind_repo calls it after the binding moves and reports snippets_moved and snippets_read_only. Co-Authored-By: Claude Opus 5.5 --- src/grimoire/mcp/tools/repos.py | 21 +++++-- src/grimoire/services/snippets.py | 85 ++++++++++++++++++++++------- tests/test_integration_repo_move.py | 32 +++++++++++ 3 files changed, 112 insertions(+), 26 deletions(-) diff --git a/src/grimoire/mcp/tools/repos.py b/src/grimoire/mcp/tools/repos.py index ff6255f0..a63faf39 100644 --- a/src/grimoire/mcp/tools/repos.py +++ b/src/grimoire/mcp/tools/repos.py @@ -11,6 +11,7 @@ from __future__ import annotations from grimoire.mcp._context import current_user_id from grimoire.services import projects as projects_svc from grimoire.services import repo_bindings as repo_bindings_svc +from grimoire.services import snippets as snippets_svc async def bind_repo( @@ -30,9 +31,11 @@ async def bind_repo( pass ref="-" to clear it back to the default branch. A repo renamed on its forge has a new remote, and its ledger is stored - under the old one. Pass the old remote as `moved_from` and the binding and - every shape judgment move to the new name together. Binding the new name - without it starts an empty ledger beside the old one. + under the old one. Pass the old remote as `moved_from` and the binding, + every shape judgment, and every snippet location recorded in the old repo + move to the new name together. Binding the new name without it starts an + empty ledger beside the old one. A snippet shared with you read-only keeps + its location and is listed in `snippets_read_only`; its owner moves it. Args: repo_url: the repo's git remote (e.g. the output of @@ -49,11 +52,14 @@ async def bind_repo( if project is None: raise ValueError(f"project {project_id} not found") ref_arg = None if not ref else ("" if ref.strip() == "-" else ref) - moved = None + moved = snippets = None if moved_from: _, moved = await repo_bindings_svc.move_binding( uid, moved_from, repo_url, project_id, ) + snippets = await snippets_svc.move_location_repo( + uid, project_id, moved_from, repo_url, + ) binding = await repo_bindings_svc.set_binding(uid, repo_url, project_id, ref_arg) follows = binding.ref or "the default branch" out = { @@ -70,7 +76,12 @@ async def bind_repo( old_key = repo_bindings_svc.normalize_repo_key(moved_from) out["moved_from"] = old_key out["shapes_moved"] = moved - out["message"] += f" Moved from `{old_key}` with {moved} shape(s)." + out["snippets_moved"] = snippets["moved"] + out["snippets_read_only"] = snippets["read_only"] + out["message"] += ( + f" Moved from `{old_key}` with {moved} shape(s) and " + f"{len(snippets['moved'])} snippet(s)." + ) return out diff --git a/src/grimoire/services/snippets.py b/src/grimoire/services/snippets.py index 92a4997a..b34a9f59 100644 --- a/src/grimoire/services/snippets.py +++ b/src/grimoire/services/snippets.py @@ -1280,22 +1280,15 @@ def _path_touches(recorded: str, changed: str) -> bool: return changed == recorded or changed.startswith(recorded + "/") -async def move_locations( - user_id: int, project_id: int, from_path: str, to_path: str, -) -> dict: - """Re-path the recorded locations under ``from_path`` (a file or a - directory) to the same place under ``to_path``, on every live snippet of - the project the caller can edit. +async def _rewrite_locations(user_id: int, project_id: int, rewrite) -> dict: + """Apply ``rewrite`` to every recorded location of the project's live + snippets, and save each snippet a rewrite changed. - The twin of the shape ledger's move: a snippet recorded at a path that - moved reads `missing` at its next check, and its canonical stamp no longer - covers the shape that moved. Each snippet is rewritten through - update_snippet, so its body and indexed fields stay one record. A - snippet the caller can only read is left as it is and listed in - ``read_only``. + ``rewrite(location)`` answers the new location, or None to leave it as it + is. Each snippet is saved through update_snippet, so its body and indexed + fields stay one record. A snippet the caller can only read is left as it + is and listed in ``read_only``. """ - old = (from_path or "").strip().strip("/") - new = (to_path or "").strip().strip("/") async with async_session() as session: notes = ( await session.execute( @@ -1310,15 +1303,14 @@ async def move_locations( read_only: list[int] = [] for note in notes: locations = snippet_fields(note).get("locations") or [] - if not any(_path_touches(old, loc.get("path") or "") for loc in locations): + rewritten = [rewrite(loc) for loc in locations] + if all(new is None for new in rewritten): continue - repathed = [ - {**loc, "path": new + loc["path"].strip("/")[len(old):]} - if _path_touches(old, loc.get("path") or "") else loc - for loc in locations - ] try: - await update_snippet(user_id, int(note.id), locations=repathed) + await update_snippet( + user_id, int(note.id), + locations=[new or loc for new, loc in zip(rewritten, locations)], + ) except PermissionError: read_only.append(int(note.id)) continue @@ -1326,6 +1318,57 @@ async def move_locations( return {"moved": moved, "read_only": read_only} +async def move_locations( + user_id: int, project_id: int, from_path: str, to_path: str, +) -> dict: + """Re-path the recorded locations under ``from_path`` (a file or a + directory) to the same place under ``to_path``, on every live snippet of + the project the caller can edit. + + The twin of the shape ledger's move: a snippet recorded at a path that + moved reads `missing` at its next check, and its canonical stamp no longer + covers the shape that moved. Returns {"moved": [ids], "read_only": [ids]}. + """ + old = (from_path or "").strip().strip("/") + new = (to_path or "").strip().strip("/") + + def repath(loc: dict) -> dict | None: + if not _path_touches(old, loc.get("path") or ""): + return None + return {**loc, "path": new + loc["path"].strip("/")[len(old):]} + + return await _rewrite_locations(user_id, project_id, repath) + + +async def move_location_repo( + user_id: int, project_id: int, old_repo: str, new_repo: str, +) -> dict: + """Point the project's snippet locations recorded in ``old_repo`` at + ``new_repo``, for a repo renamed on its forge. + + The twin of the binding move: a location still naming the old repo + resolves to nothing on the forge. Repo names are recorded free-form, so a + location matches when it is the old remote in any form, or a trailing + part of it ("owner/Repo", "Repo"), compared without case. A match is + rewritten to the new normalized key. Returns {"moved": [ids], + "read_only": [ids]}. + """ + from grimoire.services.repo_bindings import normalize_repo_key + + old_key = normalize_repo_key(old_repo) + new_key = normalize_repo_key(new_repo) + + def rename(loc: dict) -> dict | None: + recorded = normalize_repo_key(loc.get("repo") or "") + if not recorded or recorded == new_key: + return None + if recorded != old_key and not old_key.endswith("/" + recorded): + return None + return {**loc, "repo": new_key} + + return await _rewrite_locations(user_id, project_id, rename) + + async def invalidate_for_push( repo_key: str, changed: list[str], diff --git a/tests/test_integration_repo_move.py b/tests/test_integration_repo_move.py index 0d9b1f88..700ddad8 100644 --- a/tests/test_integration_repo_move.py +++ b/tests/test_integration_repo_move.py @@ -100,3 +100,35 @@ async def test_the_same_repo_under_both_names_is_refused(bound): await svc.move_binding( bound["owner"], OLD, "git@git.example.com:alice/oldname", bound["pid"], ) + + +@pytest.mark.integration +async def test_the_snippets_recorded_in_the_old_repo_move_with_it(bound): + from grimoire.services import snippets as snippets_svc + + owner, pid = bound["owner"], bound["pid"] + + async def snippet(name, *repos): + note = await snippets_svc.create_snippet( + owner, name=name, code=f"def {name}():\n pass\n", language="python", + project_id=pid, + locations=[{"repo": r, "path": "src/app.py", "symbol": name} for r in repos], + ) + return int(note.id) + + moved_one = await snippet("moved_one", "OldName", "alice/oldname", OLD) + mixed = await snippet("mixed", "git.example.com/alice/oldname", "elsewhere/gadget") + untouched = await snippet("untouched", "Gadget", "git.example.com/alice/oldnamesake") + + result = await snippets_svc.move_location_repo(owner, pid, OLD, NEW) + + assert sorted(result["moved"]) == sorted([moved_one, mixed]) + assert result["read_only"] == [] + + async def repos(snippet_id): + note = await snippets_svc.get_snippet(owner, snippet_id) + return [loc["repo"] for loc in snippets_svc.snippet_fields(note)["locations"]] + + assert await repos(moved_one) == [NEW_KEY, NEW_KEY, NEW_KEY] + assert await repos(mixed) == [NEW_KEY, "elsewhere/gadget"] + assert await repos(untouched) == ["Gadget", "git.example.com/alice/oldnamesake"]