feat(worktree): seed the absent children of an existing seed dir (#235)

[Polloinfilzato]

What

worktree_seed entries naming a directory now seed the children the worktree is missing,
instead of skipping the entry whole. Option B from #230, as scoped in #235.

Why

A worktree checks out tracked files, so a seed directory with any tracked child always exists —
and copy-when-absent then skipped the entry entirely, including the gitignored children that are
absent and would clobber nothing. #233 made that skip visible; this makes it stop happening.

How

_copy_traversable gains an opt-in skip_existing guard, so no-clobber holds at file
granularity, and returns whether it wrote anything.

Opt-in rather than baked into the helper because install_into --force-skills rmtrees the
destination precisely to overwrite it (install.py:455); a guard in the helper would have
silently regressed it. Only the seeding call passes the flag — the other four call sites are
byte-identical, and test_install_skills_force still pins the force path. The guard sits ahead
of both write branches, so it holds for the zip-imported Traversable source too, not just
shutil.copy2.

Reporting follows the rule from #233 one level down:

• an entry that seeded even one child is applied configuration → not reported;
• an entry that copied nothing → still reported (still a silent no-op);
• per-child skips are not reported — the checkout is expected to carry its tracked children,
  the same routine-noise argument that excluded globs in fix: report worktree_seed entries skipped as no-ops #233;
• a partially-seeded entry still reaches patterns, so the children just written stay out of the
  unit's git add -A. Excluding the whole dir is safe: an exclude does not untrack the tracked
  children that were already there.

test_provision_worktree_reports_seed_dir_skipped_whole carried a # documents today's behaviour assertion; it is inverted and renamed here, which is the behaviour change #235 asked
for. test_provision_worktree_seed_does_not_clobber_existing passes unmodified.

Testing

• Full suite green: 2701 passed, 23 skipped (run under LC_ALL=C LANG=C).
• trunk fmt && trunk check --no-progress clean.
• Each new test was proved to bite: with install.py stashed, 5 of the 5 new tests fail (the
  6th, reports_seed_dir_with_nothing_to_copy, passes either way by design — it pins the
  unchanged case).
• New coverage: absent children seeded; no-clobber at file granularity including a nested child;
  total no-op still reported; exclude pattern written for a partial seed in a real git worktree;
  skip_existing on the zip Traversable branch; the False return on a total no-op.

Note, not addressed here

seed_globs has the same whole-entry behaviour for a directory match (install.py:572). I left
it alone deliberately — #235 scopes this to worktree_seed, and changing the glob path would
alter behaviour for plugin-authored seeds that were not part of the report. Happy to extend it
in a follow-up if you'd rather the two paths share one semantic.

Also carrying forward your non-blocking note on #233: the skipped-entry signal is not specific to
hand-written worktree_seed entries, since the seed list also carries adapter-profile
seed_files. I have not repeated that claim here.

Fixes #235.

Summary by CodeRabbit

• Bug Fixes

  • Improved worktree configuration seeding to copy missing files and directories even when their parent directory already exists.
  • Existing files are preserved, and conflicting file/directory paths are skipped safely without overwriting.
  • Seeded files remain excluded from later bulk staging operations.
  • Updated skipped-seed reporting to distinguish fully skipped entries from partially applied ones.

• Documentation

  • Clarified seeding behavior, sequencing, and staging protection for isolated worktrees.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 587d3b9b-83d2-4b42-b37d-6d346109bf00

📥 Commits

Reviewing files that changed from the base of the PR and between 1524b45 and 4acc64b.

📒 Files selected for processing (4)

• docs/FEATURES.md
• src/bmad_loop/install.py
• src/bmad_loop/worktree_flow.py
• tests/test_install.py

---

Walkthrough

Directory seeding now copies missing children beneath existing directories without clobbering existing content. Provisioning distinguishes partial applications from total no-ops, preserves git exclusion behavior, and documents the updated sequencing and staging protection.

Changes

Worktree seeding

Layer / File(s)	Summary
No-clobber traversable copying src/bmad_loop/install.py, tests/test_install.py	_copy_traversable skips existing paths when requested, recursively copies missing children, returns whether it wrote data, and handles filesystem and zip sources.
Partial seed provisioning and reporting src/bmad_loop/worktree_flow.py, tests/test_install.py, docs/FEATURES.md	Existing seed directories are populated child by child; total no-ops remain skipped, partial seeds are applied and shielded from staging, and the behavior is documented and tested.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant provision_worktree
  participant _copy_traversable
  participant WorktreeFilesystem
  participant LocalGitExclude
  provision_worktree->>_copy_traversable: seed existing directory
  _copy_traversable->>WorktreeFilesystem: copy absent children
  _copy_traversable-->>provision_worktree: report copied or total no-op
  provision_worktree->>LocalGitExclude: shield partially seeded path

Loading

Possibly related PRs

• bmad-code-org/bmad-loop#130: Also changes _copy_traversable copy behavior.
• bmad-code-org/bmad-loop#233: Also changes existing-destination seed reporting.
• bmad-code-org/bmad-loop#265: Also changes directory-recursion and no-op semantics in worktree seeding.

Suggested reviewers: pbean

Poem

I hop through folders, quiet and light,
Copying missing leaves just right.
Old files stay safe where they lay,
New seeds hide from staging’s way.
A carrot twirls—hooray!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: seeding absent children of an existing directory entry.
Linked Issues check	✅ Passed	The code and tests implement #235's directory-recursion, no-clobber, reporting, and exclude-pattern requirements.
Out of Scope Changes check	✅ Passed	The changes stay focused on worktree seeding behavior, docs, helper logic, and regression tests.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/bmad_loop/install.py`:
- Around line 403-410: Update _copy_traversable’s directory branch to initialize
copied based on whether dst existed before mkdir, so creating a missing empty
directory returns True; retain child propagation for non-empty directories. Add
a regression test covering an empty child directory created during
provision_worktree and verify it is reported as applied rather than skipped.
- Around line 578-585: The _copy_traversable call for directory sources must
skip existing file targets without recursing. Before recursing, distinguish
destination types: treat an existing non-directory dst as a skipped no-op, and
invoke _copy_traversable only when dst is absent or dst.is_dir(), preserving
skip_existing no-clobber behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 79007fc3-586d-4d5e-af10-87d922570307

📥 Commits

Reviewing files that changed from the base of the PR and between dc71927 and 1524b45.

📒 Files selected for processing (3)

• docs/FEATURES.md
• src/bmad_loop/install.py
• tests/test_install.py

[pbean]

@Polloinfilzato thanks — reviewed, and this is exactly the shape #235 asked for: file-granular no-clobber with the pinned invariant test untouched, the #233 reporting rule carried one level down, the exclude still written for partial seeds, and the opt-in flag keeping --force-skills intact.

To get it merged promptly I pushed two commits to your branch (maintainer edits) rather than round-tripping:

• 8e67b6b — merge of main. refactor(engine): extract worktree isolation flow into WorktreeFlow (#244, PR 1/2) #265 moved provision_worktree out of install.py into the new worktree_flow.py after you branched, which is where the conflict came from. Your _copy_traversable changes stay in install.py (the helper still lives there and worktree_flow imports it); the seeding-loop half of your change is re-applied verbatim in worktree_flow.py.
• 4acc64b — the two CodeRabbit findings. The Major was real: a dir entry whose destination is a file crashed on mkdir. The suggested call-site check alone misses the same crash one level down (child dir in the repo vs file in the checkout), so the helper's dir branch also skips a file-typed dst under skip_existing. The Minor (a created empty dir not counted as written) is fixed by initializing copied from whether dst existed. Four regression tests, each proven to bite with the fix hunks stashed.

Full suite green locally (2772 passed / 7 skipped), trunk fmt && trunk check --no-fix clean. Nothing else in your work changed — shout if you'd have done any of it differently.

[pbean]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.