An unreadable repo skill dir aborts provision_worktree mid-loop and leaves the worktree short

[pbean]

Found while reviewing PR #406 (the skills seed gate). Not a regression from that PR — the copy loop is byte-identical on release/0.9.x. Sibling of #416, same loop.

What happens

provision_worktree's BASE_SKILLS copy loop calls _copy_traversable(src, dst) with no IO error handling. A repo-side skill dir the process cannot read raises out of provisioning, part-way through the loop.

Measured

Repo skill dir chmod 000 on bmad-review-edge-case-hunter, everything else normal:

provision_worktree -> RAISED PermissionError: [Errno 13] Permission denied:
    .../repo/.claude/skills/bmad-review-edge-case-hunter
worktree tree after the raise -> [bmad-build-auto, bmad-dev-auto, bmad-loop-resolve,
    bmad-loop-setup, bmad-loop-sweep, bmad-review-adversarial-general,
    bmad-review-edge-case-hunter]
BASE_SKILLS expected      -> [bmad-build-auto, bmad-dev-auto, bmad-review,
    bmad-review-adversarial-general, bmad-review-edge-case-hunter,
    bmad-review-verification-gap]

Two distinct problems in that output:

1. The loop aborts, so bmad-review and bmad-review-verification-gap — later in iteration order, nothing wrong with either — are never copied.
2. The victim is left half-copied. bmad-review-edge-case-hunter exists in the worktree as a directory whose contents are incomplete. Every seed check in this area asks is_dir(), so a partial skill reads as present: base_skills_seed_incomplete cannot report it, and the run-start preflight already passed against the repo. That is the shape the renderer-scripts sentinel exists to catch for _bmad/scripts/ — a partial unit being worse than an absent one.

Why it matters

Same as #416: this escapes the worktree-seed-skipped channel entirely, so the engine never gets to choose escalate-vs-defer, and it is an environment fault — the seed reads the same repo for every story, so no repair session fixes it and no later story escapes it.

Unreadable skill trees are not exotic: a shared machine-wide BMad install owned by another user, or a root-owned install read by an unprivileged run, produces exactly this.

Shape of a fix

Decide the doctrine deliberately — this is the "observation degrades, repair raises" question. If provisioning is repair, the raise wants to be a typed fault the engine can journal and pause on rather than a bare OSError through the state machine; if it degrades, the per-skill failure wants to land in the skipped-seed channel and let the existing gate name it. Either way the half-copied destination should not be left where an is_dir() probe reads it as complete.

[coderabbitai]

🔗 Related PRs

#130 - fix: preserve seed file modes + pause on verify env faults (rc 126/127) [merged]
#233 - fix: report worktree_seed entries skipped as no-ops [merged]
#257 - feat(worktree): seed the absent children of an existing seed dir (#235) [merged]
#283 - fix(validate): accept merged bmad-review, drop unreleased verification-gap requirement (#260) [merged]
#373 - fix(install): guard the worktree exclude's filesystem tail (#359) [merged]

---

📝 Issue Planner

_{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

• Create Plan

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!

[pbean]

Problem 2 is closed by PR #406; problem 1 is not. Keeping this open for problem 1.

The worktree skill copy is now a per-FILE merge (_merge_traversable) and
base_skills_seed_incomplete asks per required file instead of is_dir(), so the
"a partial skill reads as present" half of this issue no longer holds — measured
against the new code:

shape	before	after
worktree carries a half-copied skill dir, repo has it whole	dir-level dst.exists() skipped it whole; gate's is_dir() called it complete	the merge fills in every absent file; provision_worktree -> [] because there is nothing left short
worktree carries a skill dir short of a required file (SKILL.md, or a BASE_SKILLS marker) that the repo has and the merge could not deliver	gate returned []	gate returns the file, e.g. ['.claude/skills/bmad-build-auto/customize.toml'], and the engine escalates before dispatch

So the is_dir() blindness this issue names is gone twice over: the ordinary partial
dir is repaired rather than reported, and one that genuinely cannot be delivered is
named per file.

Bounded blind spot, stated deliberately. The required surface is the one the
run-start preflight already asserts — SKILL.md plus that skill's BASE_SKILLS
markers. REVIEW_HUNTER_SKILLS markers are (), so for a hunter SKILL.md is the
whole required surface: a hunter dir that is short of some other file is still not
reported. That is the accepted cost of not using rglob-parity-with-the-repo as the
gate's surface (parity halts the whole backlog over a repo-only README.md, with no
remedy available to the operator). The remedy for a file that turns out to matter is
to add it to the marker tuple, where both layers pick it up at once.

Problem 1 is unchanged. Re-measured on the new code, repo skill dir chmod 000:

provision_worktree -> RAISED PermissionError: [Errno 13] Permission denied:
    .../repo/.claude/skills/bmad-review-edge-case-hunter

_merge_traversable reaches src.iterdir() on the unreadable dir exactly as
_copy_traversable did, so the loop still aborts part-way and the later skills are
never copied. No typed fault, no worktree-seed-skipped entry, still out through the
state machine. The doctrine question this issue poses — observation degrades, repair
raises — is untouched.

One incidental note for whoever fixes it: the gate's new precondition stats a path
inside the repo's skill dir (repo_skill / "SKILL.md") rather than the dir itself.
Path.is_file() swallows the PermissionError and answers False, so an unreadable
repo skill dir now reads to the gate as "the repo does not have this skill" and is
skipped silently — but the copy loop raises on the identical condition first, so
nothing reaches that branch today. A fix for problem 1 that makes provisioning degrade
instead of raise needs to decide what that gate should then answer.