feat(validate): warn on legacy shared-exclude pollution (#384)

CHANGELOG.md

@@ -9,6 +9,12 @@ breaking changes may land in a minor release.
 
 ### Added
 
+- **`validate` warns when an old bmad-loop left shield patterns in `.git/info/exclude` (#384).**
+  The writer is fixed, but nothing removes the lines already there — and they hide every new file
+  under paths like `.claude/skills` from `git add -A` in that checkout. `git.exclude-legacy-pollution`
+  names the file and the exact lines. Warn only, never auto-removed: the same line could be the
+  project's own, so only exact matches are reported and deleting them stays a manual call.
+
 - **A park travels with its story's commit, so `bmad-loop confirm` works from any clone (#356).**
   Each parked story now writes one committed JSON record to `.bmad-loop/operator/<key>.json`, inside
   the story's own commit window, so the record rides the park's commit — through the worktree
@@ -260,6 +266,13 @@ story <id>`, the same annotation a sweep bundle writes. Both sprint and stories
 
 ### Fixed
 
+- **`scm.worktree_seed` entries are validated at policy load (#384).** An empty entry made worktree
+  provisioning resolve its copy source to the repo **root**, copying the whole project — gitignored
+  and untracked files included — into the worktree, then render as the exclude pattern `/`, which hid
+  the entire worktree from the unit's `git add -A` so nothing was ever committed. Entries must be
+  project-relative, the same rule adapter profiles and plugin manifests already apply to their own
+  seed lists; `worktree_seed` was the only unchecked source.
+
 - **The worktree git-add shield no longer hides new files in your own checkout (#384).** Worktree
   provisioning shielded the tool files it writes (`.claude/skills`, the per-CLI hook config, seeded
   configs) by appending to `.git/info/exclude` — a file shared with the main checkout and every

docs/FEATURES.md

@@ -183,6 +183,7 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se
 
 - `bmad-loop init` installs the three `bmad-loop-*` skills (`bmad-loop-setup`, `bmad-loop-resolve`, `bmad-loop-sweep`, into `.claude/skills/` and/or `.agents/skills/`), the hook relay, `.bmad-loop/policy.toml`, and a gitignore covering the runs dir, plugin caches, and policy.toml itself (per-machine config). Flags: `--cli` (repeatable), `--no-skills`, `--force-skills`.
 - `bmad-loop validate` preflights every prerequisite: BMAD config, sprint-status, git, the selected terminal-multiplexer backend (listing all detected when more than one is registered), CLI binary, hook registration, and the review skills the installed `bmad-dev-auto` actually invokes — derived from its `customize.toml` review layers (or from `step-04-review.md` on releases that name reviewers inline), so both the merged `bmad-review` topology and the standalone-hunter one validate, and configured layers naming an uninstalled skill are caught — plus its `customize.toml`.
+- `validate` also warns (`git.exclude-legacy-pollution`) when the repository-wide `.git/info/exclude` still carries git-add shield patterns a pre-#384 bmad-loop appended to it. Those lines name paths projects legitimately track, so every **new** file under them stays invisible to `git status` and `git add -A` in that checkout until they are removed. The warning names the file and the exact lines and stops there — it never edits the file, because a line like `/.claude/skills` is indistinguishable from one the project wrote for itself, and only exact matches against the shield's own pattern set are reported.
 - Non-invasive: drives the upstream `bmad-dev-auto` skill unmodified — there is no fork to keep in sync — and review is just a re-invocation of it on the `done` spec. Your standard BMAD install is never modified.
 
 ### Command reference

src/bmad_loop/checks.py

@@ -56,6 +56,7 @@
         "queue.stories-manifest",
         "git.worktree-clean",
         "git.probe",
+        "git.exclude-legacy-pollution",
         "hooks.config-parse",
         "hooks.registered",
         "mux.backend",

src/bmad_loop/cli.py

@@ -185,7 +185,7 @@ def _reconcile_stale(project: Path, paths: bmadconfig.ProjectPaths, pol) -> None
 
 
 def cmd_validate(args: argparse.Namespace) -> int:
-    from .install import relay_registered
+    from .install import legacy_exclude_pollution, relay_registered
 
     project = _project(args)
     report = ValidationReport()
@@ -267,6 +267,30 @@ def cmd_validate(args: argparse.Namespace) -> int:
     except verify.GitError as e:
         report.fail("git.probe", f"git check failed: {e}")
 
+    # #384: an older bmad-loop wrote the worktree git-add shield's patterns into the
+    # repository-wide .git/info/exclude. The writer is fixed and never touches that
+    # file again, but nothing removes the lines it already wrote — and they name
+    # paths projects TRACK, so new files under them stay unstageable in this
+    # checkout until someone deletes them. Warn only, never repair: the file is
+    # hand-editable and its lines carry no authorship (see the helper's docstring).
+    seed_rels: list[str] = []
+    if pol is not None:
+        if pol.scm.seed_adapter_defaults:
+            for profile in profiles:
+                seed_rels.extend(profile.seed_files)
+        seed_rels.extend(pol.scm.worktree_seed)
+    pollution = legacy_exclude_pollution(project, profiles, seed_rels)
+    if pollution is not None:
+        report.warn(
+            "git.exclude-legacy-pollution",
+            f"{pollution.path} carries git-add shield patterns written by an older "
+            f"bmad-loop: {', '.join(pollution.lines)} — new files under those paths never "
+            f"appear in `git status` or `git add -A` in this checkout, however long ago the "
+            f"run that wrote them finished. bmad-loop no longer writes this file (#384); "
+            f"review those lines and delete them by hand if your project tracks those paths.",
+            {"exclude": str(pollution.path), "lines": pollution.lines},
+        )
+
     report.extend(_platform_preflight())
 
     # #231: notify.desktop defaults to true but only fires when a platform notifier

src/bmad_loop/install.py

@@ -1064,6 +1064,103 @@ def _worktree_local_exclude(worktree: Path, patterns: Sequence[str]) -> str | No
     return None
 
 
+class LegacyExcludePollution(NamedTuple):
+    """Where the repository-wide exclude is, and which shield patterns it carries.
+
+    Returned only when `lines` is non-empty, so a caller that has one of these has
+    something to report and a file to name in the report.
+    """
+
+    path: Path
+    lines: list[str]
+
+
+def legacy_exclude_pollution(
+    project: Path, profiles: Sequence[CLIProfile], seed_rels: Sequence[str]
+) -> LegacyExcludePollution | None:
+    """Git-add shield patterns an OLDER bmad-loop left in `.git/info/exclude`.
+
+    Read-only detection for the residue of #384. Until that fix, the worktree
+    shield appended its patterns to the repository-wide exclude — shared with the
+    main checkout, permanent, and unversioned. `_worktree_local_exclude` no longer
+    writes that file at all, but nothing removes what it already wrote, and the
+    patterns name paths projects legitimately TRACK (`.claude/skills`,
+    `.claude/settings.json`). So in a repo provisioned by an older version the
+    tracked files keep diffing normally while every NEW file under those paths
+    stays invisible to `git status` and `git add -A` — the reporter lost 51 files
+    and three whole skills that way, across two commits, with nothing in either
+    diff able to reveal the cause.
+
+    Detection only, never repair, and that is a decision rather than an omission:
+    `.git/info/exclude` is a hand-editable file whose lines carry no authorship. A
+    project that genuinely wants `/.claude/skills` ignored writes the identical
+    line, and there is no way to tell the two apart after the fact — so removing
+    one is a change only the operator can be sure is right. `validate` warns and
+    names the lines; deleting them is theirs.
+
+    EXACT matches only, for the same reason. The candidate set is rebuilt the way
+    `provision_worktree` builds it, and a line counts only if it is byte-identical
+    to a pattern that set contains. No prefix, substring or path-containment
+    matching: `/.claude/skills-of-my-own` and a bare `.claude/skills` are the
+    operator's lines, not ours, and a preflight that flagged them would be telling
+    someone to delete their own gitignore rules.
+
+    `--git-path info/exclude` names git's ONE shared exclude from either vantage:
+    a main checkout answers relative (`.git/info/exclude`), a linked worktree
+    answers with the MAIN repo's absolute path (verified, git 2.55) — which is the
+    same file, and the file #384 is about. Nothing here reads the private
+    per-worktree exclude the current shield writes; that one is supposed to carry
+    these patterns.
+
+    Silent on every fault — not a repo, git missing or too old, an unreadable or
+    undecodable exclude — because this is preflight observation, and a repo whose
+    exclude cannot be read is not thereby a repo with a problem to report.
+    """
+    # Mirror provision_worktree's pattern build (worktree_flow.py). Kept in step by
+    # construction rather than by a shared constant: the shield's set is derived
+    # per-run from what it actually wrote, and this one is a superset guess about
+    # runs that are long over.
+    candidates = {f"/{p.skill_tree}" for p in profiles}
+    candidates |= {f"/{p.hooks.config_path}" for p in profiles if not p.hookless}
+    candidates |= {f"/{rel}" for rel in seed_rels}
+    # Unconditional, unlike the writer's `if customize_seeded`: whether some past
+    # run seeded _bmad/custom is not knowable from here, and the pattern is ours
+    # either way.
+    candidates.add(f"/{CUSTOMIZE_DIR.as_posix()}")
+    # A bare "/" is never a shield pattern, and it is the candidate that could do
+    # harm: it is a real (if odd) line for an operator to have written, and this
+    # function's output tells them to delete what it names.
+    #
+    # No validated caller can produce it any more — every seed source is checked
+    # at its own boundary (`skill_tree` and profile `seed_files` in
+    # adapters/profile.py, plugin seeds in plugins/manifest.py, and
+    # `scm.worktree_seed` in policy.py, which was the hole), and the writer skips
+    # hookless config_paths for this same reason. This stays as the helper's own
+    # contract: it takes `seed_rels` from its caller, and a detector that tells
+    # someone to delete a line should not depend on validation happening upstream.
+    candidates.discard("/")
+    try:
+        answered = git_bytes(project, "rev-parse", "--git-path", "info/exclude")
+        if answered.returncode != 0:
+            return None  # not a repo, or a git too old to answer
+        # fsdecode for the same reason _worktree_local_exclude uses it: a non-UTF-8
+        # repo path must round-trip back to the filesystem rather than fault.
+        exclude = Path(os.fsdecode(answered.stdout).strip())
+        if not exclude.is_absolute():
+            exclude = project / exclude
+        present = exclude.read_text(encoding="utf-8").splitlines()
+    # GitError for the chokepoint's two raised faults (a timeout, and a spawn
+    # failure as GitSpawnError) — this is a `validate` warning, and a git that
+    # cannot answer is not a finding. OSError and UnicodeError stay for the read.
+    except (GitError, OSError, UnicodeError):
+        return None
+    # Set intersection IS the exact-match rule — no normalization, no strip(): a
+    # leading space is a meaningful part of a gitignore pattern, so a line that
+    # carries one is not a line this code wrote.
+    hits = sorted(candidates.intersection(present))
+    return LegacyExcludePollution(exclude, hits) if hits else None
+
+
 def _copy_skills(project: Path, trees: Sequence[str], force: bool) -> bool:
     """Install the bundled bmad-loop-* skills into each project skill tree.
 

src/bmad_loop/policy.py

@@ -19,7 +19,7 @@
 from pathlib import Path
 from typing import Any
 
-from .platform_util import atomic_replace
+from .platform_util import atomic_replace, has_parent_ref, is_absolute_path
 
 POLICY_FILE = Path(".bmad-loop") / "policy.toml"
 
@@ -957,6 +957,25 @@ def loads(text: str, plugin_schemas: dict[str, Any] | None = None) -> Policy:
         )
     if scm.failed_diff_max_mb < 1:
         raise PolicyError(f"scm.failed_diff_max_mb must be >= 1: got {scm.failed_diff_max_mb}")
+    # The same rule the other two seed sources already apply to their own entries
+    # (adapters/profile.py `seed_files`, plugins/manifest.py `_check_relative_paths`).
+    # All three feed one list into provision_worktree's seed loop, and this was the
+    # only one arriving unvalidated.
+    #
+    # The empty entry is why this is a guard rather than a tidy-up: `""` makes that
+    # loop resolve src to the repo ROOT and dst to the worktree, both of which pass
+    # its `is_relative_to` containment checks — so it copies the entire repo into
+    # the worktree, gitignored and untracked files included, and then renders as the
+    # exclude pattern "/", which git-excludes the whole worktree so the unit's
+    # `git add -A` stages nothing at all. Absolute and `..` entries are already
+    # contained by those same checks (silently skipped); they are rejected here for
+    # consistency with the sibling sources, and because a silently-inert seed entry
+    # reads as applied configuration when it is not.
+    for seed in scm.worktree_seed:
+        if not seed or is_absolute_path(seed) or has_parent_ref(seed):
+            raise PolicyError(
+                f"scm.worktree_seed entries must be project-relative paths: got {seed!r}"
+            )
     cleanup = CleanupPolicy(
         run_retention=int(cleanup_d.get("run_retention", CleanupPolicy.run_retention)),
         retention_days=int(cleanup_d.get("retention_days", CleanupPolicy.retention_days)),

tests/test_cli.py

@@ -4162,6 +4162,80 @@ def test_dry_run_stories_relativizes_absolute_folder(project, capsys):
     assert f"Spec folder: {abs_folder}" not in out  # not the raw absolute path
 
 
+# ------------- #384: legacy shared-exclude pollution is a validate warning -----
+
+
+def _pollute_exclude(project, *lines: str):
+    """Append `lines` to the repository-wide exclude, the way a pre-#384 run did.
+
+    Called AFTER the fixture's own commit: these patterns name paths the fixture
+    tracks, and an exclude in place earlier would change what `git add -A` stages
+    — which is the very bug, and would make the fixture, not the gate, the thing
+    under test."""
+    exclude = project.project / ".git" / "info" / "exclude"
+    exclude.parent.mkdir(parents=True, exist_ok=True)
+    with exclude.open("a", encoding="utf-8") as fh:
+        fh.write("".join(f"{line}\n" for line in lines))
+    return exclude
+
+
+def test_validate_warns_on_legacy_shared_exclude_pollution(project, capsys, monkeypatch):
+    """#384's residue: lines an older bmad-loop left in `.git/info/exclude` hide
+    every NEW file under those paths from this checkout's `git add -A`, forever.
+    validate names the file and the exact lines."""
+    _make_validate_pass(project, monkeypatch, capsys)
+    exclude = _pollute_exclude(project, "/.claude/skills", "/_bmad/custom")
+
+    doc = machine_json(["validate", "--project", str(project.project), "--json"], capsys)
+
+    warned = [f for f in doc["findings"] if f["check"] == "git.exclude-legacy-pollution"]
+    assert len(warned) == 1, "the pollution gate fires exactly once"
+    assert warned[0]["severity"] == "warning"
+    assert warned[0]["detail"]["lines"] == ["/.claude/skills", "/_bmad/custom"]
+    assert warned[0]["detail"]["exclude"] == str(exclude)
+    # a warning is not a verdict: the operator is told, not blocked
+    assert doc["ok"] is True
+
+
+def test_validate_pollution_warning_prints_the_lines_in_text_mode(project, capsys, monkeypatch):
+    """The remedy is manual, so the human-readable output has to carry enough to
+    act on: which file, which lines, and what the symptom looks like."""
+    _make_validate_pass(project, monkeypatch, capsys)
+    exclude = _pollute_exclude(project, "/.claude/skills")
+
+    cli.main(["validate", "--project", str(project.project)])
+
+    text = _validate_output(capsys)
+    assert "/.claude/skills" in text
+    # the file, by its real path rather than a POSIX-shaped substring: on Windows
+    # this renders with backslashes, so asserting "info/exclude" passed nowhere
+    assert str(exclude).lower() in text
+    assert "git add -a" in text  # the symptom, lowercased by _validate_output
+    assert "delete them by hand" in text
+
+
+def test_validate_does_not_flag_an_operator_authored_exclude_line(project, capsys, monkeypatch):
+    """A line for any path that is not a shield pattern is the operator's own, and
+    telling them to delete it would be wrong. Exact match only — never substring
+    or containment (ablation: widen the match and this fails)."""
+    _make_validate_pass(project, monkeypatch, capsys)
+    _pollute_exclude(project, "/scratch", "/.claude/skills-mine", ".claude/skills")
+
+    doc = machine_json(["validate", "--project", str(project.project), "--json"], capsys)
+
+    assert not [f for f in doc["findings"] if f["check"] == "git.exclude-legacy-pollution"]
+
+
+def test_validate_silent_when_the_exclude_file_is_absent(project, capsys, monkeypatch):
+    """A repo never provisioned by an old bmad-loop gets no warning at all."""
+    _make_validate_pass(project, monkeypatch, capsys)
+    (project.project / ".git" / "info" / "exclude").unlink(missing_ok=True)
+
+    doc = machine_json(["validate", "--project", str(project.project), "--json"], capsys)
+
+    assert not [f for f in doc["findings"] if f["check"] == "git.exclude-legacy-pollution"]
+
+
 # --------------- `bmad-loop mux`: backend listing + persisted choice (issue #87) ----