From 7a19205c63a9d7de83a55eeb7b808070400ab4bb Mon Sep 17 00:00:00 2001 From: Polloinfilzato Date: Tue, 21 Jul 2026 18:02:44 +0200 Subject: [PATCH] fix: report worktree_seed entries skipped as no-ops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit provision_worktree returns the seed_files entries that named a real source but were skipped because the destination already existed, and the engine journals them as worktree-seed-skipped. Copy-when-absent is right for a file the checkout legitimately carries. For a DIRECTORY entry it is skipped whole the moment any child is tracked — and a worktree checks out tracked files, so `worktree_seed = ["_bmad"]` where _bmad/custom is tracked copies nothing at all, including the children that are absent and would clobber nothing. Nothing reported it: the local `seeded` list only feeds the git-exclude patterns. Reporting rather than printing because provision_worktree is quiet by contract (it runs under the TUI), so the journal is where the signal belongs. Glob-expanded matches are deliberately excluded: a plugin's glob is expected to hit paths the checkout already carries, so that skip is routine. Behaviour is unchanged — nothing new is copied. Fixes #230 --- src/bmad_loop/engine.py | 11 ++++++++++- src/bmad_loop/install.py | 24 +++++++++++++++++++++--- tests/test_install.py | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 4 deletions(-) diff --git a/src/bmad_loop/engine.py b/src/bmad_loop/engine.py index 603d535e..690677f5 100644 --- a/src/bmad_loop/engine.py +++ b/src/bmad_loop/engine.py @@ -598,13 +598,22 @@ def _run_isolated(self, task: StoryTask, drive: Callable[[StoryTask], None]) -> # config so the worktree's Editor MCP is reachable. Aggregate every loaded # plugin's declared seeds. seeds.extend(self._registry.seed_files()) - provision_worktree( + skipped_seeds = provision_worktree( unit.path, profiles, self.paths.repo_root, seed_files=list(dict.fromkeys(seeds)), # dedupe, preserve order seed_globs=self._registry.seed_globs(), ) + if skipped_seeds: + # A seed entry whose destination already exists is a no-op. Harmless for + # a file the checkout legitimately carries, but a directory entry is + # skipped WHOLE the moment any child is tracked — so a `worktree_seed` + # that looks applied can be copying nothing. Journal it; provision is + # quiet by contract (it runs under the TUI). + self.journal.append( + "worktree-seed-skipped", story_key=task.story_key, entries=skipped_seeds + ) self.journal.append( "worktree-opened", story_key=task.story_key, branch=unit.branch, path=str(unit.path) ) diff --git a/src/bmad_loop/install.py b/src/bmad_loop/install.py index ccb5b24c..24375f18 100644 --- a/src/bmad_loop/install.py +++ b/src/bmad_loop/install.py @@ -486,7 +486,7 @@ def provision_worktree( repo_root: Path, seed_files: Sequence[str] = (), seed_globs: Sequence[str] = (), -) -> None: +) -> list[str]: """Make a freshly-created git worktree a self-sufficient bmad-loop project. A worktree checks out tracked files only, but the skill trees (.claude/skills, @@ -519,9 +519,14 @@ def provision_worktree( seed_files are copied BEFORE the hook step so a seeded settings file that is also a hook config_path (.claude/settings.json, .gemini/settings.json) keeps its real content and just gets the Stop hook merged in, rather than being created empty. + + Returns the `seed_files` entries that existed in the repo but were skipped + because the destination already existed — copy-when-absent turned them into + no-ops. The caller journals them: a user-authored `worktree_seed` entry that + silently copies nothing reads as applied configuration and is not. """ if not profiles and not seed_files and not seed_globs: - return + return [] worktree = worktree.resolve() repo_root = repo_root.resolve() relay = repo_root / HOOK_SCRIPT_REL @@ -530,12 +535,24 @@ def provision_worktree( # project gitignored MCP/CLI configs: copy from the main repo when absent. # Resolve-and-contain guards against an `..`/absolute entry escaping either tree. seeded: list[str] = [] + # Entries that named a real source but whose destination already exists, so + # copy-when-absent made them a no-op. Reported to the caller (this function is + # quiet by contract — it runs under a TUI) because for a DIRECTORY entry the + # no-op is silent and total: a worktree checks out tracked files, so a seed dir + # with any tracked child always exists and the whole entry is skipped, including + # the children that are absent and would clobber nothing. Glob-expanded matches + # are deliberately not reported: a plugin's glob is expected to hit paths the + # checkout already carries, so that skip is routine rather than a misconfiguration. + skipped: list[str] = [] for rel in seed_files: src = (repo_root / rel).resolve() dst = (worktree / rel).resolve() if not src.is_relative_to(repo_root) or not dst.is_relative_to(worktree): continue - if not src.exists() or dst.exists(): + if not src.exists(): + continue + if dst.exists(): + skipped.append(str(rel)) continue dst.parent.mkdir(parents=True, exist_ok=True) _copy_traversable(src, dst) @@ -614,6 +631,7 @@ def provision_worktree( patterns |= {f"/{p.hooks.config_path}" for p in profiles if not p.hookless} patterns |= {f"/{rel}" for rel in seeded} _worktree_local_exclude(worktree, sorted(patterns)) + return skipped def _warn_if_policy_tracked(project: Path) -> None: diff --git a/tests/test_install.py b/tests/test_install.py index 839050f8..686e2e56 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -804,6 +804,43 @@ def test_provision_worktree_seed_does_not_clobber_existing(tmp_path): assert dst.read_text() == "IN_WORKTREE" +def test_provision_worktree_reports_seed_skipped_as_noop(tmp_path): + """A seed entry left untouched because the destination exists is REPORTED, so a + `worktree_seed` that copies nothing cannot look like applied configuration.""" + wt, repo = tmp_path / "wt", tmp_path / "repo" + repo.mkdir() + (repo / ".mcp.json").write_text("FROM_REPO", encoding="utf-8") + dst = wt / ".mcp.json" + dst.parent.mkdir(parents=True) + dst.write_text("IN_WORKTREE", encoding="utf-8") + assert provision_worktree(wt, [], repo, seed_files=[".mcp.json"]) == [".mcp.json"] + + +def test_provision_worktree_reports_seed_dir_skipped_whole(tmp_path): + """The case that motivated the report: a worktree checks out tracked files, so a + seed DIRECTORY with any tracked child already exists and the whole entry is + skipped — including children that are absent and would clobber nothing.""" + wt, repo = tmp_path / "wt", tmp_path / "repo" + (repo / "_bmad" / "custom").mkdir(parents=True) # tracked child + (repo / "_bmad" / "bmm").mkdir() # gitignored sibling, absent from the checkout + (repo / "_bmad" / "bmm" / "config.yaml").write_text("SEED ME", encoding="utf-8") + (wt / "_bmad" / "custom").mkdir(parents=True) # what `git worktree add` lays down + + assert provision_worktree(wt, [], repo, seed_files=["_bmad"]) == ["_bmad"] + assert not (wt / "_bmad" / "bmm").exists() # documents today's behaviour + + +def test_provision_worktree_reports_nothing_when_seeding_succeeds(tmp_path): + """A seed that actually copies is not reported — the signal stays specific to + entries that silently did nothing. A missing source is also not a no-op report: + it is already covered as its own case.""" + wt, repo = tmp_path / "wt", tmp_path / "repo" + repo.mkdir() + (repo / ".mcp.json").write_text("FROM_REPO", encoding="utf-8") + assert provision_worktree(wt, [], repo, seed_files=[".mcp.json", "absent.json"]) == [] + assert (wt / ".mcp.json").read_text() == "FROM_REPO" + + def test_provision_worktree_seed_rejects_escaping_path(tmp_path): """A seed entry resolving outside the repo/worktree is skipped — never copies a file from outside the project tree into the worktree."""