From c5f71e03e2f6980c7717d8fc163e9331be92e865 Mon Sep 17 00:00:00 2001 From: t Date: Tue, 25 Aug 2026 11:41:32 -0700 Subject: [PATCH 1/6] fix(install): keep the rollback report's stderr decode inside its never-raises promise (#394) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_shield_undo_extension`'s `--unset-all` block decodes git's stderr to name why the rollback failed, guarded for `GitError` alone — so on Windows (utf-8/surrogatepass) a lone invalid byte escaped a function contracted never to raise, replacing both the activation fault and the retained-flag disclosure with the caller's generic tail reason. `OSError`/`RuntimeError` stay out: nothing in that block resolves a path. Ablation run: restored the bare `except GitError`, and test_shield_rollback_stderr_decode_fault_is_reported_not_raised errored with the injected UnicodeDecodeError escaping at install.py:1959; restored from a cp backup, re-ran green. Full suite 6699 passed / 49 skipped, pyright 0 errors, trunk check clean. --- CHANGELOG.md | 3 ++ src/bmad_loop/install.py | 12 ++++++-- tests/test_install.py | 66 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 516c58d5..c3b2c330 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,9 @@ breaking changes may land in a minor release. sitting there. Also refuse an empty or `:`-bearing session name before the probe spawns. A same-named session on a foreign server, and one differing only by whitespace the seam normalizes away, stay #531's subject (#671) +- The git-add shield's rollback report no longer raises through its own stderr decode on + Windows (#394). `_shield_undo_extension` is contracted never to raise, but the `fsdecode` of a + failing `--unset-all`'s stderr was guarded for `GitError` alone. ### Security diff --git a/src/bmad_loop/install.py b/src/bmad_loop/install.py index 87a65e78..d26616b2 100644 --- a/src/bmad_loop/install.py +++ b/src/bmad_loop/install.py @@ -1957,9 +1957,16 @@ def _shield_undo_extension(worktree: Path, git_dir: Path, common_dir: Path) -> s if undone.returncode in (0, 5): return "" detail = os.fsdecode(undone.stderr).strip() or f"git exited {undone.returncode}" - except GitError as e: + except (GitError, UnicodeError) as e: # the rollback's OWN git can time out or fail to spawn: a read-only `.git` or # a dead git fails this unset for the same reason it failed the activation. + # + # `UnicodeError` is the `fsdecode` of git's stderr one line up (#394): Windows + # decodes utf-8/surrogatepass, which REJECTS a lone invalid byte, so without it + # a codec fault escapes a function contracted never to raise (POSIX decodes + # with surrogateescape and never raises). `OSError`/`RuntimeError` are + # deliberately absent, so this is NOT a copy of the sibling scan's tuple above: + # nothing in this block resolves a path, which is what those two are there for. detail = str(e) # Both clauses HEDGE whether this shield set the flag, and must: reached from the # enable's own raise, a spawn failure can kill the enable and this unset alike, @@ -2310,7 +2317,8 @@ def _worktree_local_exclude(worktree: Path, patterns: Sequence[str]) -> str | No # `except` of its own: a non-zero rc returns the reason below, a raise lands in # this try's tail — which also catches the `UnicodeError` the `fsdecode` of # git's stderr can raise on Windows, hence that read sits inside the guard. - # #394 records that same decode escaping at a sibling block that lacks it. + # The sibling decode in `_shield_undo_extension`'s rollback carries that same + # guard, for the same reason (#394). shared_answer = git_bytes(worktree, "rev-parse", "--git-common-dir") if shared_answer.returncode != 0: detail = ( diff --git a/tests/test_install.py b/tests/test_install.py index a5a9799b..b314573c 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -6290,6 +6290,72 @@ def loop(self, *a, **kw): assert "worktreeConfig" in (repo / ".git" / "config").read_text(encoding="utf-8") +def test_shield_rollback_stderr_decode_fault_is_reported_not_raised(project, tmp_path, monkeypatch): + """The OTHER block of the same never-raises promise, and the same fault shape (#394). + + Where the sibling test above covers the dependents scan, this covers the `--unset-all` + block one down: it decodes git's stderr with `os.fsdecode` to name why the rollback + failed, and that decode was guarded for `GitError` alone — so a codec fault escaped a + function contracted never to raise, replacing BOTH the activation fault and the + retained-flag disclosure with the caller's generic tail reason. + + The fault is INJECTED rather than produced from real bytes, for the reason the issue's + testability note gives: POSIX `os.fsdecode` decodes with `surrogateescape` and cannot + raise, so the real-world trigger is Windows-only (utf-8/surrogatepass rejects a lone + invalid byte). Same justification as the sibling's injected `resolve()` fault. + + Driven at the helper rather than through `_worktree_local_exclude`, and for the same + reason: an end-to-end injection would fire on an earlier same-shaped `fsdecode` — the + caller decodes `rev-parse`'s stderr before the rollback is ever reached — and prove + something else. The `os.fsdecode` fake is predicate-scoped to the marker bytes so + every other decode in the process keeps working. + + Ablation: restore the bare `except GitError` at the `--unset-all` block and this test + errors — the injected UnicodeDecodeError escapes a function contracted never to + raise.""" + repo = project.project + wt = tmp_path / "wt" + verify.worktree_add(repo, wt, "feat", "main") + git(repo, "config", "extensions.worktreeConfig", "true") + git_dir = Path(git(wt, "rev-parse", "--absolute-git-dir")).resolve() + common = Path(git(wt, "rev-parse", "--git-common-dir")).resolve() + # the scan must find no dependent, or the helper returns before the decode under test + assert not (common / "config.worktree").exists() + marker = b"fatal: could not lock config file \xff" + real_git_bytes, real_fsdecode = install_mod.git_bytes, os.fsdecode + + def unset_fails(worktree, *args, timeout_s=None): + # An exact-argv match, and the `_is_unset` tripwire lesson does not bite here: + # a spelling drift makes this fake stop matching, the REAL unset then answers + # rc 5 for an absent key, and the helper returns "" — which reddens the clause + # assertions below rather than going quiet. + if args == ("config", "--unset-all", "extensions.worktreeConfig"): + return subprocess.CompletedProcess( + args=["git", *args], returncode=2, stdout=b"", stderr=marker + ) + return real_git_bytes(worktree, *args, timeout_s=timeout_s) + + def undecodable(value): + if value == marker: + raise UnicodeDecodeError("utf-8", b"\x62", 0, 1, "injected") + return real_fsdecode(value) + + monkeypatch.setattr(install_mod, "git_bytes", unset_fails) + monkeypatch.setattr(os, "fsdecode", undecodable) + + clause = _shield_undo_extension(wt, git_dir, common) + + # before ANY assertion: a globally patched `os.fsdecode` must not outlive the call + monkeypatch.undo() + assert isinstance(clause, str) and clause # reported, not raised... + assert "could NOT be" in clause # ...with the hedge the caller's tail cannot give + # names the injected decode as the cause, separating it from "git exited 2": the + # rc-2 branch is what the escape used to skip past + assert "injected" in clause + # the flag survives — the fake is why, and the clause is the only thing that says so + assert "worktreeConfig" in (repo / ".git" / "config").read_text(encoding="utf-8") + + def test_shield_rolls_back_inside_the_lock(project, tmp_path, monkeypatch): """The ROLLBACK has to happen while the lock is still held, and that placement is load-bearing rather than incidental: released first, a second run probes in the From 38606c2099b31a397f0d987d1d9b77b11fbe933b Mon Sep 17 00:00:00 2001 From: t Date: Tue, 25 Aug 2026 11:54:40 -0700 Subject: [PATCH 2/6] fix(install): refuse to enable worktreeConfig over an operator's explicit disable (#396) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_shield_enable_worktree_config` read `extensions.worktreeConfig` with `--type=bool` and treated anything but `true` as "needs enabling", so an operator's explicit `false` / `off` / `no` / `0` in the shared config reported `needs_enable=True`: the SUCCESS path rewrote that declaration to `true` permanently with nothing journaling what it replaced, and a failed activation's `--unset-all` then deleted the operator's own line and reported a clean rollback. Present-but-not-`true` is now a refusal arm beside the `core.bare` / `core.worktree` ones — the shield stands down and the reason quotes the raw spelling it found. Deliberate deviation from the issue's capture/restore sketch (user-approved 2026-08-25): a restore covers only the failure paths, while the success path is where the declaration is lost for good. Accepted cost, stated in the reason and in FEATURES.md: an explicitly-disabled repo gets no shield there. Refusing also puts the rollback deletion out of reach rather than trying to make it faithful. No signatures changed — the probe's 2-tuple, its caller and `_shield_undo_extension` are untouched. MEASURED, identical at both (docker ubuntu:22.04 git 2.34.1, the support floor; and local git 2.55.0): probe rc stdout --type=bool --get on = false/off/no/0/FALSE 0 false --type=bool --get doubled, true then off 0 false (LAST line answers) --type=bool --get doubled, off then true 0 true (LAST line answers) --type=bool --get doubled, junk(1st) then false 128 fatal: bad boolean config value --type=bool --get doubled, false then junk 128 fatal: bad boolean config value --type=bool --get valueless worktreeConfig 0 true (already-carried, NOT refused) raw --get on = off / no / 0 / FALSE 0 off / no / 0 / FALSE raw --get doubled, off then FALSE 0 FALSE (LAST line answers) --type=bool --get absent key 1 (empty) So a doubled disabled flag reaches the refusal, junk in any line degrades at the probe (GitError, caller's tail), a valueless line stays on the already-carried arm, and the raw re-read is what keeps the reason honest — `--type=bool` normalizes every disabled spelling to `false`, which is precisely why the issue's "restore the probed value" sketch could not work. ABLATIONS, run singly and restored from cp backups: - Deleted the `carried is not None` refusal arm: both test_shield_refuses_to_enable_over_an_operator_explicit_false and test_shield_refusal_preserves_a_doubled_disabled_flag FAILED, the enable firing the write booby-trap at install.py:2554, while test_shield_valueless_flag_counts_as_carried_not_refused stayed green — the disjointness that makes the pair mean something. - Replaced the already-carried test with a raw-truthiness read (dropping `--type=bool`): test_shield_valueless_flag_counts_as_carried_not_refused FAILED (the empty stored value reads falsy, so the valueless line is refused as a disable), and both refusal tests FAILED too (a truthy `off` reads as already-carried) — that reading is wrong in both directions at once. Full suite 6702 passed / 49 skipped, pyright 0 errors, trunk check clean. --- CHANGELOG.md | 6 ++ docs/FEATURES.md | 2 +- src/bmad_loop/install.py | 26 +++++++- tests/test_install.py | 132 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 164 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c3b2c330..2633ef16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,12 @@ breaking changes may land in a minor release. ### Fixed +- **The git-add shield refuses to enable `extensions.worktreeConfig` over an operator's + explicit disable** (#396), instead of enabling it and deleting the line on rollback. The + probe read the flag `--type=bool`, so a `false`/`off`/`no`/`0` in the shared config read + as "needs enabling": the success path rewrote that declaration to `true` permanently, and + a failed activation's `--unset-all` removed the operator's line and reported a clean + rollback. Such a repo now gets no shield there, with a reason naming the spelling found. - **TUI: a graceful-stop request that cannot be written is reported, not fatal.** The `S` worker caught only the helper's own refusals; an `OSError` from the write itself escaped, and Textual's default `exit_on_error` took the dashboard down with it. It now surfaces diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 9ee0a2a7..2cb9a5bb 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -83,7 +83,7 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se - Failed-unit forensics: a deferred/escalated unit's worktree + branch stay mounted (`keep_failed`, default on) and its full diff is preserved to `run_dir/failed//changes.patch`; `failed_diff_max_mb` caps per-file untracked-file size (oversized skipped with a marker), `failed_diff_unlimited` lifts the cap. - Config seeding: a worktree checks out _tracked_ files only, so a project's gitignored MCP/CLI configs (`.mcp.json`, `.claude/settings.json`, `.codex/config.toml`, `.gemini/settings.json`) would be missing — an isolated session couldn't reach its MCP server. With `seed_adapter_defaults` (default on) each loaded adapter's own `seed_files` are copied in from the main repo before the session launches, together with every non-hookless adapter's resolved hook `config_path` — which is gitignored for codex, so without it the project's own hook configuration never reached the worktree and the session ran against a file holding bmad-loop's relay registrations alone (#471). Seeding does not decide whether that relay hook registers — the hook-merge writes it either way, with one refusal: a seeded config that is present but **will not parse** stops provisioning outright (`GitError` → CRITICAL escalation, run paused) instead of being read as an empty document. JSON has no partial read, so an unparseable config is evidence of an earlier fault rather than a blank slate — and merging the relay into `{}` would publish a hooks-only file over the operator's allowlist, `env` and MCP entries, erasing that evidence along with them. The bytes are left exactly as found, and the refusal names whichever source actually supplies them rather than the copy it read: an escalated story re-enters the run only through a re-arm, which discards the worktree and provisions a fresh one, so a repair applied to the disposable copy is undone before the next drive. Which source that is comes from the seed bookkeeping rather than from a counterpart's existence — seeding is copy-when-absent, so a config the project TRACKS is skipped as an occupied destination and arrives with the branch checkout, and that lane is told to commit the repair on the target branch while a genuinely seeded one is sent to the main checkout. That bookkeeping is recorded per path actually written rather than per seed entry: a directory entry seeds child by child, so its presence proves that some child landed and never that this config did, and inferring from it would advise committing a gitignored settings file the checkout supplied. The remedy names the re-arm (`bmad-loop resolve --no-interactive`) because `ESCALATED` is terminal: repairing the file alone leaves the story out of the run (#592). Invalid UTF-8 takes the same lane, which before this crashed the engine instead of escalating. `worktree_seed` adds extra paths. Copy-when-absent at file granularity — a directory entry whose destination already exists (a worktree checkout carries its tracked children) still seeds the children that are missing — seeded before the hook-merge (a seeded `settings.json` keeps its content, but its relay entry is replaced rather than kept — the seeded copy names the main repo's `$CLAUDE_PROJECT_DIR`-relative relay, which resolves to the worktree, where no relay exists; left in place it would read as already registered and the session would stall silently, #352; a _tracked_ config gets the same rewrite pinned `skip-worktree` in the worktree's own index, so the machine-specific command never rides `git add -A` into a story commit — while pinned the config is orchestrator-owned, so a story's own edit to it stays session-local), and shielded from the unit's `git add -A` — in a private exclude scoped to that worktree alone (see below), never repo-wide. - Seeding is verified **result-side**, by re-probing the worktree on disk rather than trusting the copier's bookkeeping — so a user-authored `worktree_seed` rel can neither forge nor mask a report: `worktree-seed-skipped` (a seed entry that copied nothing), `worktree-seed-dropped` (a repo-carried seed that never arrived), and `worktree-module-skills-dropped` (#464 — a wheel-bundled `bmad-loop-*` skill whose content came up short, e.g. a checkout file squatting the skill's directory, which the per-file no-clobber refuses to replace) are all **informational**, journaled and never a pause. Presence is the whole contract; content is never compared, since no-clobber is per _file_ and a checkout's own divergent fork of a bundled skill is a healthy shape. Only the upstream dev/review skill and renderer completeness probes escalate, because those have a deterministic in-worktree consumer — the session dispatches them inside the worktree and stalls having written nothing — while the bundled operator/triage skills are dispatched at the **main checkout** (`/bmad-loop-sweep` from sweep triage, `bmad-loop resolve`; `bmad-loop-setup` has no session consumer at all), so their absence in a worktree stalls nothing. -- The git-add shield is scoped to the worktree and expires with it (#384). Provisioned tool files (skill trees, hook config, seeded configs) go into a private `.git/worktrees//info/exclude`, activated by a worktree-scoped `core.excludesFile` that shadows rather than concatenates with your own — so yours is copied into the private one **byte for byte**, the shield covers that unit only, `git worktree remove` takes it away, and the shared, permanent `.git/info/exclude` is never written. An explicitly empty `core.excludesFile` is honored literally — no excludes file at all — rather than read as unset, so git's XDG fallback is not consulted and there is nothing to copy. It then **proves it applies or stands down**: bmad-loop asks git which excludes file it actually resolves **and from which scope** (`git config --show-scope`), so the degrade reason names the winner rather than inferring one — ambient command-scope config (`git -c`, `GIT_CONFIG_PARAMETERS`, `GIT_CONFIG_COUNT`) is told apart from a worktree-scoped write that is not in force at all, which sends you to a different repair (#692). Anything but a confirmed byte-identical match — either of those, an unreadable excludes file, an answer naming no scope, an unanswerable probe — skips the shield with a journaled and notified reason; a match activates it whatever scope supplied the value, since the thing being proved is that git reads the file bmad-loop wrote. Two runs against one repository serialize on an exclusive lock, leaving a zero-length `.git/bmad-loop-shield.lock` — never in the working tree, so nothing your `git add -A` can see; on Windows the wait gives up after ~10s and the shield is skipped naming the lock. Caveats: its gate is the project's **git 2.34** support floor (an older git — or one that cannot say what it is — skips the shield, and the repo-format flag is deliberately not written); that is a _support_ refusal rather than a capability one, since the two things the shield is built out of, `extensions.worktreeConfig` and `git config --worktree`, have existed since git 2.20 — runs are refused outright below the floor, so what this gate still catches on a current host is a git that cannot be spawned, times out, or answers unparseably. Enabling it sets `extensions.worktreeConfig` — a **permanent** repo-format flag, rolled back wherever it could be left set without a working shield, but surviving in two cases the reason distinguishes: a sibling worktree still depends on it, or the rollback could not be made at all. Where it cannot be set safely at all (`core.bare = true` or `core.worktree` in the shared config) the shield is skipped instead. Lines an older bmad-loop wrote into `.git/info/exclude` are **not** removed for you — delete them by hand. A **file** your project tracks gets no pattern: git applies ignore rules only to untracked paths, so the pattern would shield nothing while making the file read as tracked-and-ignored to `git ls-files -ci --exclude-standard` and to repo-hygiene gates built on it (#392). A tracked **directory** gets no directory pattern either — it is replaced by one pattern per file this provisioning run actually wrote below it (#484). A dir pattern does hide new children, and no pattern shape both hides them and keeps the report clean (`dir/*` with per-child negations clears the report and leaks a new file into the commit), but over a tracked tree it bought new-child coverage alone at the price of a false tracked-and-ignored report for every tracked child — modifications to those stage regardless. The accepted trade: a file the **session** creates under such a directory can be staged, which matches your own decision to track that tree, while everything the orchestrator seeded there keeps a pattern of its own. An **untracked** tool directory is unchanged — it keeps the single `/dir` pattern. If git cannot say what a path is — or a file provisioning wrote below a tracked directory has a name no exclude line can spell (an embedded newline, a trailing carriage return), which would shield nothing and loose an orphan pattern on unrelated files — the pattern is kept in its ORIGINAL shape, dir shape included, and the reason is journaled. Patterns are escaped as they are rendered (#476): a rel holding a trailing space or a wildmatch special (`*`, `?`, `[`, `\`) otherwise names something else, leaving the seeded path unshielded while the broken character class silently hides an unrelated file the unit meant to commit. +- The git-add shield is scoped to the worktree and expires with it (#384). Provisioned tool files (skill trees, hook config, seeded configs) go into a private `.git/worktrees//info/exclude`, activated by a worktree-scoped `core.excludesFile` that shadows rather than concatenates with your own — so yours is copied into the private one **byte for byte**, the shield covers that unit only, `git worktree remove` takes it away, and the shared, permanent `.git/info/exclude` is never written. An explicitly empty `core.excludesFile` is honored literally — no excludes file at all — rather than read as unset, so git's XDG fallback is not consulted and there is nothing to copy. It then **proves it applies or stands down**: bmad-loop asks git which excludes file it actually resolves **and from which scope** (`git config --show-scope`), so the degrade reason names the winner rather than inferring one — ambient command-scope config (`git -c`, `GIT_CONFIG_PARAMETERS`, `GIT_CONFIG_COUNT`) is told apart from a worktree-scoped write that is not in force at all, which sends you to a different repair (#692). Anything but a confirmed byte-identical match — either of those, an unreadable excludes file, an answer naming no scope, an unanswerable probe — skips the shield with a journaled and notified reason; a match activates it whatever scope supplied the value, since the thing being proved is that git reads the file bmad-loop wrote. Two runs against one repository serialize on an exclusive lock, leaving a zero-length `.git/bmad-loop-shield.lock` — never in the working tree, so nothing your `git add -A` can see; on Windows the wait gives up after ~10s and the shield is skipped naming the lock. Caveats: its gate is the project's **git 2.34** support floor (an older git — or one that cannot say what it is — skips the shield, and the repo-format flag is deliberately not written); that is a _support_ refusal rather than a capability one, since the two things the shield is built out of, `extensions.worktreeConfig` and `git config --worktree`, have existed since git 2.20 — runs are refused outright below the floor, so what this gate still catches on a current host is a git that cannot be spawned, times out, or answers unparseably. Enabling it sets `extensions.worktreeConfig` — a **permanent** repo-format flag, rolled back wherever it could be left set without a working shield, but surviving in two cases the reason distinguishes: a sibling worktree still depends on it, or the rollback could not be made at all. Where it cannot be set safely at all (`core.bare = true` or `core.worktree` in the shared config) the shield is skipped instead. It is skipped the same way where the shared config already carries an explicit `extensions.worktreeConfig` that is not `true` (#396): that line is an operator's own declaration, and enabling over it would rewrite it to `true` permanently with nothing recording what stood there — so the shield stands down and the reason quotes the spelling it found, raw rather than `--type=bool`-normalized, since `false`, `off`, `no` and `0` all read back as `false`. A **valueless** `worktreeConfig` line is not that case: git reads it as `true`, so the flag counts as already carried and the shield proceeds. Lines an older bmad-loop wrote into `.git/info/exclude` are **not** removed for you — delete them by hand. A **file** your project tracks gets no pattern: git applies ignore rules only to untracked paths, so the pattern would shield nothing while making the file read as tracked-and-ignored to `git ls-files -ci --exclude-standard` and to repo-hygiene gates built on it (#392). A tracked **directory** gets no directory pattern either — it is replaced by one pattern per file this provisioning run actually wrote below it (#484). A dir pattern does hide new children, and no pattern shape both hides them and keeps the report clean (`dir/*` with per-child negations clears the report and leaks a new file into the commit), but over a tracked tree it bought new-child coverage alone at the price of a false tracked-and-ignored report for every tracked child — modifications to those stage regardless. The accepted trade: a file the **session** creates under such a directory can be staged, which matches your own decision to track that tree, while everything the orchestrator seeded there keeps a pattern of its own. An **untracked** tool directory is unchanged — it keeps the single `/dir` pattern. If git cannot say what a path is — or a file provisioning wrote below a tracked directory has a name no exclude line can spell (an embedded newline, a trailing carriage return), which would shield nothing and loose an orphan pattern on unrelated files — the pattern is kept in its ORIGINAL shape, dir shape included, and the reason is journaled. Patterns are escaped as they are rendered (#476): a rel holding a trailing space or a wildmatch special (`*`, `?`, `[`, `\`) otherwise names something else, leaving the seeded path unshielded while the broken character class silently hides an unrelated file the unit meant to commit. - Run state never moves into a worktree — `.bmad-loop/` always lives in the main repo; spec paths are persisted relative to the worktree so a kept-failed run stays portable. - The sprint board is **worktree-canonical for the duration of a story** (#350). A board your project _tracks_ needs nothing special: the story's advance is an ordinary modification of a checked-out file and rides the unit commit through the merge. A **gitignored** board is neither checked out nor delivered by one, so it is seeded into the worktree alongside the deferred-work ledger — the orchestrator writes the board through the worktree and `verify_dev` then reads the file it just wrote, which without the seed is a missing file the run dies on rather than a lost write. Its advance is re-applied to the main checkout's board **after** the merge, journaled `board-advance-carried` — or `board-advance-carry-uncommitted` where `git add` refused the ignored path, which is the ordinary outcome for such a board and not a fault (the status on disk is the value; the commit is best-effort). The carry replays from its record if a crash lands between the merge and its latch, and `sprintstatus.advance` never regresses, so a double application is a no-op. On that replay leg the merge — and with it the pre-flight above — has already happened, so the carry proves its own ownership before committing: it recomputes HEAD's content through `advance` and asks **git** whether the board holds that and nothing else, which a crashed pass's half-written advance does and an operator's edit does not. Sameness is git's own — both sides are hashed through the path's clean filter — so a board spelled CRLF by one host and LF by another still matches, where a byte compare would have to guess which spelling is on disk and would refuse a pristine board on every host that chose the other. **Both** places git holds the path are proved, because the carry's `git add` overwrites both: the working tree it copies into the commit, and the index it stages over — an edit staged and then restored in the working tree exists nowhere afterwards, so proving the working tree alone would authorize destroying it. That proof guards the **commit**, which is one write too late for the story's own row: `advance` would already have replaced an operator's status with the target, leaving exactly the bytes the proof accepts — and skipping the commit saves nothing when the value scheduling reads is the one on disk. So that one row is checked **before** the advance as well, and a status that is neither HEAD's nor this pass's own is refused there with nothing written. Both refusals journal `board-advance-carry-foreign-dirt`, and they refuse at different points with different stakes: the pre-advance row check refuses with **nothing written**, so your status is still on the board; the post-advance proof withholds only the **commit** — the advance is on disk by then — and the dirt it declined to take is still escalated by the next run's merge pre-flight. A board that is **gone** by carry time is refused before either question is asked and journaled `board-advance-carry-failed`, the same record a vanished row gets. Every probe fails closed, and git's own dirt answer decides whether either comparison is asked at all — a board nobody wrote is not their question (#618). Both comparisons apply to a board git **tracks**. A gitignored one has no baseline anywhere in git to be compared against, so a replayed carry can still overwrite a row you edited on it while the host was down; the commit half cannot arise there at all, since `git add` refuses an ignored path outright. Scheduling is what depends on it: `_pick_next` reads the **main** board, so an advance that never came back hands finished work to the next run's dev session. - Only the orchestrator writes the board — the session prompts forbid it, and `sprintstatus.advance` is the sole write path. The consequence under isolation, stated rather than left as a surprise: in a _gitignored_ board a session's edit to any OTHER story's row lives only in the worktree copy and evaporates when the worktree is removed. Only the story's own advance is carried back, because only that one is the orchestrator's own write. diff --git a/src/bmad_loop/install.py b/src/bmad_loop/install.py index d26616b2..8899dd55 100644 --- a/src/bmad_loop/install.py +++ b/src/bmad_loop/install.py @@ -1790,7 +1790,12 @@ def _shield_enable_worktree_config(worktree: Path, common_dir: Path) -> tuple[st the installer will not make behind an operator's back, so the shield degrades instead. - The version gate and both refusal probes STAY HERE, first. The gate is the + It is refused a THIRD way, which is this project's rather than git's: an + `extensions.worktreeConfig` already PRESENT but not `true` is an operator's + explicit disable, and enabling over it would rewrite that declaration + permanently — the same discipline as above, applied to the flag itself (#396). + + The version gate and the refusal probes STAY HERE, first. The gate is the PROJECT support floor (`verify.GIT_FLOOR`), not a capability threshold of this shield's own: `extensions.worktreeConfig` and `git config --worktree` arrived in git 2.20 and `--type=` in 2.18, all well below the floor, so on any supported git @@ -1857,6 +1862,25 @@ def _shield_enable_worktree_config(worktree: Path, common_dir: Path) -> tuple[st carried = _shield_shared_config(worktree, shared, "extensions.worktreeConfig", "--type=bool") if carried is not None and os.fsdecode(carried).strip() == "true": return None, False # already carried: nothing for the caller to write + if carried is not None: + # Present but NOT true: enabling over an operator's explicit disable is a stronger + # intervention than enabling from ABSENT, and it is the SUCCESS path that does the + # lasting damage — rewriting that declaration to `true` forever, unjournaled. The + # shield degrades instead, which also puts #396's rollback deletion out of reach. + # + # `--type=bool` normalized the spelling away (`off`/`no`/`0`/`FALSE` all read back + # `false`, measured at 2.34.1 and 2.55.0), so re-read it RAW for the reason and + # neutralize it as the sharedRepository arm above does. A GitError from that read + # propagates — the caller's tail degrades, and still nothing is enabled. `carried` + # stands in only if the key stops existing between the two reads. + raw = _shield_shared_config(worktree, shared, "extensions.worktreeConfig") + value = os.fsdecode(carried if raw is None else raw).removesuffix("\n") + return ( + f"skipped the git-add shield ({worktree}): the repository's shared config sets " + f"extensions.worktreeConfig = {value!r}, explicitly disabling it, and the shield " + "will not override an operator's declaration — the provisioned tool files are " + "not shielded from the unit's `git add -A`" + ), False bare = _shield_shared_config(worktree, shared, "core.bare", "--type=bool") if bare is not None and os.fsdecode(bare).strip() == "true": refused = "core.bare = true" diff --git a/tests/test_install.py b/tests/test_install.py index b314573c..31667278 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -5088,6 +5088,138 @@ def test_shield_refuses_to_enable_extension_over_core_bare(project, tmp_path): assert shared_exclude.read_bytes() == before +def _trap_worktree_config_writes(monkeypatch): + """Booby-trap every WRITE of `extensions.worktreeConfig`, leaving every read real. + + The refusal arm's whole claim is that nothing is written, and "the key still says + `off` afterwards" cannot carry that alone: `git config` rewriting a value the file + already holds leaves it byte-identical, so an enable that fired and was rolled back + reads exactly like one that never fired. + + Patches BOTH bindings for the reason `_shield_on_git` gives, and guards the argv by + PREFIX rather than exact token: `"--get" not in args` is an exact-token test, so a + `--get-all` or `--get-regexp` READ misses it and would be raised on as a write — the + `_is_unset` trap above, run in the other direction. `--unset` spellings are writes + here too, which is deliberate: the rollback deleting an operator's line is the very + thing #396 is about.""" + real = verify.git_bytes + + def no_format_change(worktree, *args, timeout_s=None): + reads = any(a.startswith("--get") for a in args) + if args[:1] == ("config",) and "extensions.worktreeConfig" in args and not reads: + raise AssertionError(f"wrote the flag over an operator's own line: {args}") + return real(worktree, *args) + + monkeypatch.setattr(verify, "git_bytes", no_format_change) + monkeypatch.setattr(install_mod, "git_bytes", no_format_change) + + +def test_shield_refuses_to_enable_over_an_operator_explicit_false(project, tmp_path, monkeypatch): + """An `extensions.worktreeConfig` the operator explicitly turned OFF is a declaration, + and the shield stands down rather than overruling it (#396). + + Before this the `--type=bool` probe read `off` as "not `true`" and reported + `needs_enable=True`: the SUCCESS path then rewrote the operator's line to `true` + permanently with nothing journaling what it replaced, and a failed activation's + `--unset-all` deleted the line outright and reported a clean rollback. + + The fixture spells it `off` rather than `false` on purpose. The reason has to quote + what is really in the file, and `--type=bool` normalizes `off`/`no`/`0`/`FALSE` all to + `false` (measured rc 0 at 2.34.1 and 2.55.0), so a reason built from the probe's own + answer would tell the operator about a line they never wrote. + + Ablation: delete the `carried is not None` refusal arm in + `_shield_enable_worktree_config` and this fails — the enable fires and the trap + raises.""" + repo = project.project + shared = repo / ".git" / "config" + # `--file` rather than a scope, and BEFORE the worktree is mounted: this is the state + # an operator's repo is already in when provisioning arrives, not one this run made. + git(repo, "config", "--file", str(shared), "extensions.worktreeConfig", "off") + wt = tmp_path / "wt" + verify.worktree_add(repo, wt, "feat", "main") + shared_exclude = repo / ".git" / "info" / "exclude" + before = shared_exclude.read_bytes() + _trap_worktree_config_writes(monkeypatch) + + reason = _worktree_local_exclude(wt, ["/probe-396"]) + + assert reason is not None and "explicitly disabling it" in reason + assert "'off'" in reason # the RAW spelling, re-read for the reason + assert "'false'" not in reason # non-vacuity: the bool probe's normalization did not win + assert "worktreeConfig = off" in shared.read_text(encoding="utf-8") + assert not _wt_private_exclude(wt).exists() + assert shared_exclude.read_bytes() == before + + +def test_shield_refusal_preserves_a_doubled_disabled_flag(project, tmp_path, monkeypatch): + """The doubled-key shape, which is where the rollback did its most visible damage: + `--unset-all` removes EVERY line, so a repo declaring the flag twice lost both. + + Written by hand rather than through `git config`, which de-duplicates. The two lines + are both disabled but spelled differently, which pins the measured reading the arm + rests on: `--type=bool --get` validates the whole file and answers the LAST line, so a + doubled disabled flag reaches the refusal at rc 0 rather than degrading, and the raw + read quotes that last spelling — `off`, not the leading `false` (2.34.1 and 2.55.0). + + Ablation: delete the `carried is not None` refusal arm and this fails — the enable + fires and the trap raises.""" + repo = project.project + shared = repo / ".git" / "config" + shared.write_text( + shared.read_text(encoding="utf-8") + + "[extensions]\n\tworktreeConfig = false\n\tworktreeConfig = off\n", + encoding="utf-8", + ) + assert shared.read_text(encoding="utf-8").count("worktreeConfig") == 2 # non-vacuity + wt = tmp_path / "wt" + verify.worktree_add(repo, wt, "feat", "main") + before = shared.read_bytes() + _trap_worktree_config_writes(monkeypatch) + + reason = _worktree_local_exclude(wt, ["/probe-396"]) + + assert reason is not None and "explicitly disabling it" in reason + assert "'off'" in reason # the LAST line's spelling, which is the one git answers + assert shared.read_bytes() == before # byte-identical: neither line was touched + assert not _wt_private_exclude(wt).exists() + + +def test_shield_valueless_flag_counts_as_carried_not_refused(project, tmp_path, monkeypatch): + """A VALUELESS `worktreeConfig` line is not an explicit disable, and the refusal above + must not swallow it: git reads a key written with no `=` as boolean TRUE (measured rc + 0, `true`, at 2.34.1 and 2.55.0), so the repo already carries the flag and the shield + proceeds with nothing to write. + + This is the claim the refusal arm rests on, which is why it is pinned rather than + argued. The arm tests the `--type=bool` probe's NORMALIZED answer instead of the + stored text precisely because the stored text here is the empty string, which any + truthiness reading calls false. + + Ablation: make the already-carried test read raw truthiness (drop `--type=bool` and + test the value) and this fails — the empty stored value reads falsy, the valueless + line is refused as a disable, and the shield stands down over a repo that carried the + flag all along.""" + repo = project.project + shared = repo / ".git" / "config" + shared.write_text( + shared.read_text(encoding="utf-8") + "[extensions]\n\tworktreeConfig\n", + encoding="utf-8", + ) + # non-vacuity, and this test's own precondition: git must read the hand-written line + # as boolean true while the RAW read answers rc 0 with nothing at all. + assert git(repo, "config", "--type=bool", "--get", "extensions.worktreeConfig") == "true" + assert git(repo, "config", "--get", "extensions.worktreeConfig") == "" + wt = tmp_path / "wt" + verify.worktree_add(repo, wt, "feat", "main") + _trap_worktree_config_writes(monkeypatch) + + reason = _worktree_local_exclude(wt, ["/probe-396"]) + + assert reason is None # already carried: the shield applied, with nothing to enable + assert _wt_private_exclude(wt).exists() + + def test_shield_refuses_a_repository_shared_between_os_users(project, tmp_path): """A repository configured as shared between OS users is not supported (#384), and is refused UP FRONT rather than shielded — loudly, and identically for every user. From 7e6395ce75c5779984c4908159a359b499e441fd Mon Sep 17 00:00:00 2001 From: t Date: Tue, 25 Aug 2026 12:07:17 -0700 Subject: [PATCH 3/6] fix(install): prefer %APPDATA%/Git/ignore on Git for Windows >= 2.46 (#403) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_shield_home_git_ignore` asked git where its `$HOME` is and stopped there. That is the whole of git's ignore fallback upstream, but not on the Git for Windows fork from 2.46 onward: the fork patches `xdg_config_home_for` (`path.c`) to prefer `%APPDATA%/Git/` whenever that file exists, and warns that it ignored the `$HOME` one when both are present. They are alternative locations, not a search path. The harm ran in both directions, and both were silent: - APPDATA file only — the returned `$HOME` path is typically not a file, so the seed came back empty with `reason is None` and the caller still activated a worktree-scoped `core.excludesFile` shadowing the file git really reads. Everything the operator globally ignores became visible to `git add -A` and was swept into the story commit. - Both files present — git reads APPDATA while the shield seeded `$HOME`, copying patterns git is not applying. The worktree over-ignored and session-created files went silently missing. Gated on the FORK STRING, not on `sys.platform` (deviation from #403's sketch, user-approved 2026-08-25). The preference is one fork's patch, not a property of the OS: Cygwin, MSYS2 and WSL gits run on Windows hardware as upstream builds and a `win32` test would hand every one of them the wrong file. install.py carries no `sys.platform` branch anywhere else, and asking git what it is keeps the tests honest — they fake a version string, never a platform, so they exercise the real path on a POSIX box. `is_file()` mirrors the fork's own `file_exists` precondition and is load-bearing; check order is cheap-first (env -> stat -> one extra spawn only when the candidate exists); every non-matching outcome, `git version` rc != 0 included, falls through to the `$HOME` probe, which is the pre-fix behavior and which raises its own GitError on a git that is truly dead. PROVENANCE: source-read of `git-for-windows/git` and `git/git` through #403, version bounds counted per tag (present 2.46.0.windows.1 and 2.55.0.windows.3, absent 2.45.0.windows.1 and 2.20.0.windows.1, absent upstream). NOT measured on a Windows machine — none was available, and Windows CI could not supply one either since its runners carry no `%APPDATA%\Git\ignore`. The tests pin our selection logic; they do not claim the fork agrees. Ablations, each run singly and restored from a post-edit `cp` snapshot with the md5 re-verified: - whole APPDATA arm deleted -> `test_shield_prefers_appdata_ignore_on_the_windows_fork` red (`appdata-junk.tmp` missing, `home-junk.tmp` seeded); the other three stay green, since pre-fix behavior is what they assert. - `git_version_at_least` conjunct deleted -> `test_shield_appdata_ignore_needs_the_246_fork` red at 2.45.0.windows.1 (APPDATA seeded off a fork without the patch). - `".windows." in reported` conjunct deleted -> `test_shield_appdata_ignore_is_the_forks_not_the_platforms` red at upstream 2.55.0. - `is_file()` precondition dropped -> `test_shield_appdata_absent_file_keeps_the_home_fallback` red with the seed reduced to the shield's own `/probe-403` pattern, i.e. EMPTY — #403's silent harm exactly. Also adds a `git version 2.46.0.windows.1` row to test_verify's floor-predicate table, pinning that `git_version_at_least`'s lookahead accepts the four-component fork spelling this gate depends on. --- CHANGELOG.md | 5 ++ src/bmad_loop/install.py | 59 +++++++++++-- tests/test_install.py | 176 +++++++++++++++++++++++++++++++++++++++ tests/test_verify.py | 4 + 4 files changed, 237 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2633ef16..2eee5fff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,11 @@ breaking changes may land in a minor release. as "needs enabling": the success path rewrote that declaration to `true` permanently, and a failed activation's `--unset-all` removed the operator's line and reported a clean rollback. Such a repo now gets no shield there, with a reason naming the spelling found. +- **The git-add shield seeds `%APPDATA%/Git/ignore` on Git for Windows >= 2.46** (#403), the + file that fork prefers over `$HOME/.config/git/ignore` whenever it exists. Seeding the + `$HOME` one there was wrong in both directions — an empty seed that let global ignores leak + into `git add -A`, or patterns git is not applying that made session files go missing. Gated + on the reported version's own `.windows.` fork string, not on the platform. - **TUI: a graceful-stop request that cannot be written is reported, not fatal.** The `S` worker caught only the helper's own refusals; an `OSError` from the write itself escaped, and Textual's default `exit_on_error` took the dashboard down with it. It now surfaces diff --git a/src/bmad_loop/install.py b/src/bmad_loop/install.py index 8899dd55..0930c082 100644 --- a/src/bmad_loop/install.py +++ b/src/bmad_loop/install.py @@ -34,7 +34,7 @@ from .platform_util import atomic_write_bytes, atomic_write_text, file_lock from .policy import POLICY_TEMPLATE from .process_host import get_process_host -from .verify import GitError, git_below_floor, git_bytes, git_floor_text +from .verify import GitError, git_below_floor, git_bytes, git_floor_text, git_version_at_least HOOK_SCRIPT_REL = ".bmad-loop/bmad_loop_hook.py" # Markers for bmad-loop-managed hook commands. RELAY_MARKER is shared by @@ -2003,6 +2003,13 @@ def _shield_undo_extension(worktree: Path, git_dir: Path, common_dir: Path) -> s ) +# The Git for Windows FORK patches `xdg_config_home_for` (`path.c`) to prefer +# `%APPDATA%/Git/` over the `$HOME/.config/git/` upstream computes, +# from this version onward (absent at 2.45, absent upstream at every version). +# `_shield_home_git_ignore` gates on it (#403). +_APPDATA_IGNORE_GIT = (2, 46) + + def _shield_home_git_ignore(worktree: Path) -> Path: """`$HOME/.config/git/ignore` — git's XDG fallback — asked of GIT, not of Python. @@ -2027,12 +2034,25 @@ def _shield_home_git_ignore(worktree: Path) -> Path: operator's config. The answer comes back NUL-terminated at rc 0; with `HOME` unset git exits 128 and applies no fallback at all. - KNOWN GAP: this answers "where is git's `$HOME`", which is the whole of the - fallback UPSTREAM but not on **Git for Windows >= 2.46**, whose fork patches - `xdg_config_home_for` to prefer `%APPDATA%/Git/` whenever that file EXISTS. - An operator there keeping global ignores at `%APPDATA%\\Git\\ignore` still gets - the wrong file seeded. Not fixed here: a downstream fork's behavior, and one - gated well above `GIT_FLOOR` at that. + That `$HOME` answer is the whole of the fallback UPSTREAM, and is not on **Git + for Windows >= 2.46** (#403). The fork patches `xdg_config_home_for` + (`git-for-windows/git`, `path.c`) to prefer `%APPDATA%/Git/` whenever that + file EXISTS, warning that it ignored the `$HOME` one when both are there. Counted + per tag: present at 2.46.0.windows.1 and 2.55.0.windows.3, absent at + 2.45.0.windows.1 and 2.20.0.windows.1, absent from upstream `git/git` entirely. + PROVENANCE: source-read through #403, **NOT measured on a Windows machine** — no + runtime observation of Git for Windows was available, and Windows CI cannot supply + one either (the runners carry no `%APPDATA%\\Git\\ignore`, so they can show only + that nothing broke). + + The APPDATA arm below closes a harm that ran in BOTH directions, each silent. + APPDATA file only: this returned a `$HOME` path that is typically not a file, the + seed came back empty with `reason is None`, and the caller then activated a + worktree-scoped `core.excludesFile` SHADOWING the file git really reads — so + everything the operator globally ignores became visible to `git add -A` and swept + into the story commit. BOTH files present: git uses the APPDATA one and says so, + while this seeded the `$HOME` one — copying patterns git is not applying, so the + worktree OVER-ignored and session-created files went silently missing instead. Raises `GitError` on any non-zero rc, INCLUDING the `HOME`-unset one. Proceeding would be a guess, and a guess here is silent: the caller seeds nothing and @@ -2041,6 +2061,31 @@ def _shield_home_git_ignore(worktree: Path) -> Path: version-dependent. A `HOME`-less environment therefore skips the shield with a reported reason; only a definite absent may be silent in this caller. """ + # Cheap-first, and every arm that does not MATCH falls through to the `$HOME` + # probe below — no APPDATA, no such file, an unanswerable `git version`, upstream + # git, a fork below 2.46. That fall-through is the conservative direction: it is + # exactly the pre-fix behavior, and a git too dead to report its version is not + # absolved by it, because the probe below raises its own `GitError` on the same + # git and the caller degrades with a reason. + appdata = os.environ.get("APPDATA") + if appdata: + candidate = Path(appdata) / "Git" / "ignore" + # LOAD-BEARING, and it mirrors the fork's own `file_exists` precondition: the + # preference applies only when that file is really there. It is also what + # keeps the cost at one stat on every other platform — only a candidate that + # exists is worth the extra spawn below. + if candidate.is_file(): + # Gated on the FORK STRING, deliberately NOT `sys.platform` (#403). The + # preference is a patch carried by one FORK, not a property of the OS: + # Cygwin, MSYS2 and WSL gits run on Windows hardware without it, and a + # `win32` test would hand them the wrong file. This module has no + # `sys.platform` branch anywhere else, and asking git what it is keeps the + # tests honest — they fake a version string, never a platform. + version = git_bytes(worktree, "version") + if version.returncode == 0: + reported = os.fsdecode(version.stdout) + if ".windows." in reported and git_version_at_least(reported, _APPDATA_IGNORE_GIT): + return candidate key = "bmadloop.xdghomeprobe" probe = git_bytes( worktree, "-c", f"{key}=~/.config/git/ignore", "config", "-z", "--type=path", "--get", key diff --git a/tests/test_install.py b/tests/test_install.py index 31667278..788efe8b 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -7335,6 +7335,182 @@ def fail_home_probe(worktree, *args): assert "probe-384" in git(wt, "status", "--porcelain", "-uall") +def _shield_on_reported_git_version(monkeypatch, reported): + """Answer `git version` with `reported`; every other call reaches the real git. + + The APPDATA arm of `_shield_home_git_ignore` asks git what it IS rather than + asking the OS what it is running on (#403), so the only thing a test has to fake + is that one answer — no `sys.platform` patch, and the rest of the shield runs + against the real repo and the real linked worktree. + + Patches BOTH bindings, like `_shield_on_git` above and for the same reason: + `install.py` does `from .verify import git_bytes`, which is a name distinct from + `verify.git_bytes`. The new arm resolves the `install` one while + `verify.git_below_floor`'s own floor probe resolves the `verify` one, and patching + either alone leaves the other live — silently, in the direction that fakes + nothing. Unlike `_shield_on_git` the config write is NOT booby-trapped: these + tests drive the shield to completion and then read the file it seeded. + + Returns the argv log, so a test can refuse to pass on a fake that was never + consulted.""" + real = verify.git_bytes + seen: list[tuple[str, ...]] = [] + + def reporting(worktree, *args, timeout_s=None): + if args == ("version",): + seen.append(args) + return subprocess.CompletedProcess( + args=["git", "version"], returncode=0, stdout=reported.encode(), stderr=b"" + ) + return real(worktree, *args, timeout_s=timeout_s) + + monkeypatch.setattr(verify, "git_bytes", reporting) + monkeypatch.setattr(install_mod, "git_bytes", reporting) + return seen + + +def _appdata_and_home_ignores(tmp_path, *, appdata_file=True): + """A `$HOME` global ignore and (optionally) an `%APPDATA%` one, distinguishable. + + Both carry a pattern of their own so that "seeded the wrong file" and "seeded + nothing" are different observations — the second is exactly the silent failure + #403 describes, so a test that cannot tell them apart proves nothing.""" + home = tmp_path / "githome" + (home / ".config" / "git").mkdir(parents=True) + (home / ".config" / "git" / "ignore").write_text("home-junk.tmp\n", encoding="utf-8") + appdata = tmp_path / "appdata" + if appdata_file: + (appdata / "Git").mkdir(parents=True) + (appdata / "Git" / "ignore").write_text("appdata-junk.tmp\n", encoding="utf-8") + else: + appdata.mkdir(parents=True) + return home, appdata + + +def _seed_with(project, tmp_path, monkeypatch, *, reported, appdata_file=True): + """Run the shield over a real worktree against a faked `git version`; return the + private exclude's lines. + + The env is pinned inside a `monkeypatch.context()` because conftest's + session-scoped `_isolate_ambient_git_ignores` pins `XDG_CONFIG_HOME` for every + test, and the caller only reaches `_shield_home_git_ignore` when that variable is + unset AND `core.excludesFile` is unset (which conftest already arranges by + pointing `GIT_CONFIG_GLOBAL` at a file that does not exist). `GIT_CONFIG_NOSYSTEM` + is deliberately NOT pinned, for the reason conftest records: it would suppress Git + for Windows' system `core.autocrlf`. A system-level `core.excludesFile` would send + the shield down the branch above instead, and every caller here asserts on BOTH a + pattern that must be seeded and one that must not, so that case fails loudly + rather than passing vacuously.""" + repo = project.project + wt = tmp_path / "wt" + verify.worktree_add(repo, wt, "feat", "main") + home, appdata = _appdata_and_home_ignores(tmp_path, appdata_file=appdata_file) + seen = _shield_on_reported_git_version(monkeypatch, reported) + + with monkeypatch.context() as pinned: + pinned.delenv("XDG_CONFIG_HOME", raising=False) + pinned.setenv("HOME", str(home)) + pinned.setenv("APPDATA", str(appdata)) + assert _worktree_local_exclude(wt, ["/probe-403"]) is None + + assert seen, "the fake never answered `git version` — the shield read a real one" + return _wt_private_exclude(wt).read_text(encoding="utf-8").splitlines() + + +def test_shield_prefers_appdata_ignore_on_the_windows_fork(project, tmp_path, monkeypatch): + """On Git for Windows >= 2.46 the operator's global ignores live at + `%APPDATA%\\Git\\ignore`, and that is the file the shield must copy (#403). + + The fork patches `xdg_config_home_for` (`git-for-windows/git`, `path.c`) to prefer + `%APPDATA%/Git/` over the `$HOME/.config/git/` upstream computes, + whenever the APPDATA one exists — and when BOTH exist it warns that the `$HOME` + one "was ignored because" the APPDATA one is there. They are alternative + locations, not a search path. + + PROVENANCE: that is a SOURCE READ of the fork (counted per tag through #403: + present at 2.46 and 2.55, absent at 2.45 and 2.20, absent upstream), **not a + measurement**. No Windows machine was available to observe it, and Windows CI + could not have supplied one either — its runners carry no `%APPDATA%\\Git\\ignore`. + So this test pins OUR selection logic against a version string we state; it cannot + and does not claim Git for Windows agrees. + + Both files exist here, which folds the mirror direction in: preferring APPDATA is + the same assertion as not seeding the `$HOME` file git itself is ignoring. Seeding + that one would copy patterns git is not applying, and the worktree would + OVER-ignore — session-created files going silently missing from `git add -A`, + #384's harm inverted. + + Ablation: delete the whole APPDATA arm and this fails — `home-junk.tmp` is seeded + and `appdata-junk.tmp` is not, which is the pre-fix behavior exactly.""" + seeded = _seed_with(project, tmp_path, monkeypatch, reported="git version 2.46.0.windows.1\n") + + assert "appdata-junk.tmp" in seeded + assert "home-junk.tmp" not in seeded + + +def test_shield_appdata_ignore_needs_the_246_fork(project, tmp_path, monkeypatch): + """2.45.0.windows.1 is the same FORK without the patch, and it reads `$HOME`. + + The preference arrived at 2.46 (`APPDATA` is absent from `path.c` at + 2.45.0.windows.1 and 2.20.0.windows.1, counted per tag through #403), far above + this project's own `GIT_FLOOR`. So "APPDATA exists, prefer it" is not enough on + its own: on an older Git for Windows it would seed a file git is not reading and + the worktree would over-ignore — the same silent loss the fix exists to stop, + aimed the other way. + + Ablation: delete the `git_version_at_least` conjunct and this fails — + `appdata-junk.tmp` is seeded off a fork that never had the patch.""" + seeded = _seed_with(project, tmp_path, monkeypatch, reported="git version 2.45.0.windows.1\n") + + assert "home-junk.tmp" in seeded + assert "appdata-junk.tmp" not in seeded + + +def test_shield_appdata_ignore_is_the_forks_not_the_platforms(project, tmp_path, monkeypatch): + """A current UPSTREAM git ignores `%APPDATA%` however new it is, so the gate reads + the fork string rather than the platform (#403). + + `APPDATA` appears nowhere in `git/git`'s `path.c` at any version, so 2.55.0 + upstream is above the 2.46 floor and still has no such preference. That is why + this gate is not `sys.platform == "win32"`: Cygwin, MSYS2 and WSL gits all run on + Windows hardware and are all upstream builds, and a platform test would hand every + one of them the wrong file. Asking git what it IS also keeps this test honest — + it fakes a version string, never a platform, and so it exercises the real code + path on the box it runs on. + + Ablation: delete the `".windows." in reported` conjunct and this fails — + `appdata-junk.tmp` is seeded off an upstream git that would never read it.""" + seeded = _seed_with(project, tmp_path, monkeypatch, reported="git version 2.55.0\n") + + assert "home-junk.tmp" in seeded + assert "appdata-junk.tmp" not in seeded + + +def test_shield_appdata_absent_file_keeps_the_home_fallback(project, tmp_path, monkeypatch): + """`%APPDATA%` set with no `Git/ignore` under it is the ORDINARY case on the fork, + and it must reach `$HOME` — the preference is conditional on the file existing. + + The `is_file()` precondition mirrors the fork's own `file_exists` guard: git does + not prefer a path it cannot read, it computes the `$HOME` one instead. Dropping it + would not merely seed the wrong file, it would seed NOTHING — a non-existent + source reads as an empty seed with `reason is None`, after which the caller + activates a worktree-scoped `core.excludesFile` that SHADOWS the operator's real + global ignores. That is #403's own harm, and it is silent. + + Ablation: drop the `is_file()` precondition and this fails — the arm returns the + absent candidate, the seed comes back empty, and `home-junk.tmp` is missing.""" + seeded = _seed_with( + project, + tmp_path, + monkeypatch, + reported="git version 2.46.0.windows.1\n", + appdata_file=False, + ) + + assert "home-junk.tmp" in seeded + assert "/probe-403" in seeded # ...and the seed is not empty for some other reason + + def test_shield_seeds_a_relative_xdg_config_home_resolved_like_git(project, tmp_path, monkeypatch): """The relative-path defect of the `core.excludesFile` branch, at the XDG fallback branch (#384). This branch exists to REPRODUCE git's fallback, so diff --git a/tests/test_verify.py b/tests/test_verify.py index 452eb357..0c2c0f32 100644 --- a/tests/test_verify.py +++ b/tests/test_verify.py @@ -5656,6 +5656,10 @@ def test_engine_written_is_keyword_only_on_all_dev_verifiers(): ("git version 2.33.8\n", False), # one minor below it ("git version 2.9.5\n", False), # numeric, not lexicographic: "9" > "34" as text ("git version 2.44.0.windows.1\n", True), + # The four-component fork spelling `_shield_home_git_ignore` gates its + # `%APPDATA%/Git/ignore` preference on (#403). The lookahead has to accept the + # `.` that continues into `0.windows.1` for that gate to read 2.46 at all. + ("git version 2.46.0.windows.1\n", True), ("git version 2.39.5 (Apple Git-154)\n", True), ("git version 3.0\n", True), ("git version 2.34\n", True), # bare major.minor: the end-of-string arm From dcb0e7c599f32892111072203a2d19a71d15fa32 Mon Sep 17 00:00:00 2001 From: t Date: Tue, 25 Aug 2026 12:16:41 -0700 Subject: [PATCH 4/6] docs(features): record the shield's %APPDATA%/Git/ignore seed (#403) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shield bullet already carried #396's explicit-disable refusal among the skip conditions; it still described git's XDG ignore fallback as if the `$HOME` answer were the whole of it. Phase 3 changed that: on the Git for Windows fork >= 2.46 the seeded file is `%APPDATA%/Git/ignore` whenever it exists. Written to match what the code actually does — the harm named in BOTH directions (an empty seed that shadows the file git really reads; patterns git is not applying that make session files go missing), the gate stated as the reported version's own `.windows.` fork string rather than the platform, the fall-through for every other outcome including an unanswerable `git version`, and the provenance kept honest: source-read from the fork, not measured on a Windows machine. Documentation only; no behavior change. --- docs/FEATURES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 2cb9a5bb..5edc8cb4 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -83,7 +83,7 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se - Failed-unit forensics: a deferred/escalated unit's worktree + branch stay mounted (`keep_failed`, default on) and its full diff is preserved to `run_dir/failed//changes.patch`; `failed_diff_max_mb` caps per-file untracked-file size (oversized skipped with a marker), `failed_diff_unlimited` lifts the cap. - Config seeding: a worktree checks out _tracked_ files only, so a project's gitignored MCP/CLI configs (`.mcp.json`, `.claude/settings.json`, `.codex/config.toml`, `.gemini/settings.json`) would be missing — an isolated session couldn't reach its MCP server. With `seed_adapter_defaults` (default on) each loaded adapter's own `seed_files` are copied in from the main repo before the session launches, together with every non-hookless adapter's resolved hook `config_path` — which is gitignored for codex, so without it the project's own hook configuration never reached the worktree and the session ran against a file holding bmad-loop's relay registrations alone (#471). Seeding does not decide whether that relay hook registers — the hook-merge writes it either way, with one refusal: a seeded config that is present but **will not parse** stops provisioning outright (`GitError` → CRITICAL escalation, run paused) instead of being read as an empty document. JSON has no partial read, so an unparseable config is evidence of an earlier fault rather than a blank slate — and merging the relay into `{}` would publish a hooks-only file over the operator's allowlist, `env` and MCP entries, erasing that evidence along with them. The bytes are left exactly as found, and the refusal names whichever source actually supplies them rather than the copy it read: an escalated story re-enters the run only through a re-arm, which discards the worktree and provisions a fresh one, so a repair applied to the disposable copy is undone before the next drive. Which source that is comes from the seed bookkeeping rather than from a counterpart's existence — seeding is copy-when-absent, so a config the project TRACKS is skipped as an occupied destination and arrives with the branch checkout, and that lane is told to commit the repair on the target branch while a genuinely seeded one is sent to the main checkout. That bookkeeping is recorded per path actually written rather than per seed entry: a directory entry seeds child by child, so its presence proves that some child landed and never that this config did, and inferring from it would advise committing a gitignored settings file the checkout supplied. The remedy names the re-arm (`bmad-loop resolve --no-interactive`) because `ESCALATED` is terminal: repairing the file alone leaves the story out of the run (#592). Invalid UTF-8 takes the same lane, which before this crashed the engine instead of escalating. `worktree_seed` adds extra paths. Copy-when-absent at file granularity — a directory entry whose destination already exists (a worktree checkout carries its tracked children) still seeds the children that are missing — seeded before the hook-merge (a seeded `settings.json` keeps its content, but its relay entry is replaced rather than kept — the seeded copy names the main repo's `$CLAUDE_PROJECT_DIR`-relative relay, which resolves to the worktree, where no relay exists; left in place it would read as already registered and the session would stall silently, #352; a _tracked_ config gets the same rewrite pinned `skip-worktree` in the worktree's own index, so the machine-specific command never rides `git add -A` into a story commit — while pinned the config is orchestrator-owned, so a story's own edit to it stays session-local), and shielded from the unit's `git add -A` — in a private exclude scoped to that worktree alone (see below), never repo-wide. - Seeding is verified **result-side**, by re-probing the worktree on disk rather than trusting the copier's bookkeeping — so a user-authored `worktree_seed` rel can neither forge nor mask a report: `worktree-seed-skipped` (a seed entry that copied nothing), `worktree-seed-dropped` (a repo-carried seed that never arrived), and `worktree-module-skills-dropped` (#464 — a wheel-bundled `bmad-loop-*` skill whose content came up short, e.g. a checkout file squatting the skill's directory, which the per-file no-clobber refuses to replace) are all **informational**, journaled and never a pause. Presence is the whole contract; content is never compared, since no-clobber is per _file_ and a checkout's own divergent fork of a bundled skill is a healthy shape. Only the upstream dev/review skill and renderer completeness probes escalate, because those have a deterministic in-worktree consumer — the session dispatches them inside the worktree and stalls having written nothing — while the bundled operator/triage skills are dispatched at the **main checkout** (`/bmad-loop-sweep` from sweep triage, `bmad-loop resolve`; `bmad-loop-setup` has no session consumer at all), so their absence in a worktree stalls nothing. -- The git-add shield is scoped to the worktree and expires with it (#384). Provisioned tool files (skill trees, hook config, seeded configs) go into a private `.git/worktrees//info/exclude`, activated by a worktree-scoped `core.excludesFile` that shadows rather than concatenates with your own — so yours is copied into the private one **byte for byte**, the shield covers that unit only, `git worktree remove` takes it away, and the shared, permanent `.git/info/exclude` is never written. An explicitly empty `core.excludesFile` is honored literally — no excludes file at all — rather than read as unset, so git's XDG fallback is not consulted and there is nothing to copy. It then **proves it applies or stands down**: bmad-loop asks git which excludes file it actually resolves **and from which scope** (`git config --show-scope`), so the degrade reason names the winner rather than inferring one — ambient command-scope config (`git -c`, `GIT_CONFIG_PARAMETERS`, `GIT_CONFIG_COUNT`) is told apart from a worktree-scoped write that is not in force at all, which sends you to a different repair (#692). Anything but a confirmed byte-identical match — either of those, an unreadable excludes file, an answer naming no scope, an unanswerable probe — skips the shield with a journaled and notified reason; a match activates it whatever scope supplied the value, since the thing being proved is that git reads the file bmad-loop wrote. Two runs against one repository serialize on an exclusive lock, leaving a zero-length `.git/bmad-loop-shield.lock` — never in the working tree, so nothing your `git add -A` can see; on Windows the wait gives up after ~10s and the shield is skipped naming the lock. Caveats: its gate is the project's **git 2.34** support floor (an older git — or one that cannot say what it is — skips the shield, and the repo-format flag is deliberately not written); that is a _support_ refusal rather than a capability one, since the two things the shield is built out of, `extensions.worktreeConfig` and `git config --worktree`, have existed since git 2.20 — runs are refused outright below the floor, so what this gate still catches on a current host is a git that cannot be spawned, times out, or answers unparseably. Enabling it sets `extensions.worktreeConfig` — a **permanent** repo-format flag, rolled back wherever it could be left set without a working shield, but surviving in two cases the reason distinguishes: a sibling worktree still depends on it, or the rollback could not be made at all. Where it cannot be set safely at all (`core.bare = true` or `core.worktree` in the shared config) the shield is skipped instead. It is skipped the same way where the shared config already carries an explicit `extensions.worktreeConfig` that is not `true` (#396): that line is an operator's own declaration, and enabling over it would rewrite it to `true` permanently with nothing recording what stood there — so the shield stands down and the reason quotes the spelling it found, raw rather than `--type=bool`-normalized, since `false`, `off`, `no` and `0` all read back as `false`. A **valueless** `worktreeConfig` line is not that case: git reads it as `true`, so the flag counts as already carried and the shield proceeds. Lines an older bmad-loop wrote into `.git/info/exclude` are **not** removed for you — delete them by hand. A **file** your project tracks gets no pattern: git applies ignore rules only to untracked paths, so the pattern would shield nothing while making the file read as tracked-and-ignored to `git ls-files -ci --exclude-standard` and to repo-hygiene gates built on it (#392). A tracked **directory** gets no directory pattern either — it is replaced by one pattern per file this provisioning run actually wrote below it (#484). A dir pattern does hide new children, and no pattern shape both hides them and keeps the report clean (`dir/*` with per-child negations clears the report and leaks a new file into the commit), but over a tracked tree it bought new-child coverage alone at the price of a false tracked-and-ignored report for every tracked child — modifications to those stage regardless. The accepted trade: a file the **session** creates under such a directory can be staged, which matches your own decision to track that tree, while everything the orchestrator seeded there keeps a pattern of its own. An **untracked** tool directory is unchanged — it keeps the single `/dir` pattern. If git cannot say what a path is — or a file provisioning wrote below a tracked directory has a name no exclude line can spell (an embedded newline, a trailing carriage return), which would shield nothing and loose an orphan pattern on unrelated files — the pattern is kept in its ORIGINAL shape, dir shape included, and the reason is journaled. Patterns are escaped as they are rendered (#476): a rel holding a trailing space or a wildmatch special (`*`, `?`, `[`, `\`) otherwise names something else, leaving the seeded path unshielded while the broken character class silently hides an unrelated file the unit meant to commit. +- The git-add shield is scoped to the worktree and expires with it (#384). Provisioned tool files (skill trees, hook config, seeded configs) go into a private `.git/worktrees//info/exclude`, activated by a worktree-scoped `core.excludesFile` that shadows rather than concatenates with your own — so yours is copied into the private one **byte for byte**, the shield covers that unit only, `git worktree remove` takes it away, and the shared, permanent `.git/info/exclude` is never written. An explicitly empty `core.excludesFile` is honored literally — no excludes file at all — rather than read as unset, so git's XDG fallback is not consulted and there is nothing to copy. Where that fallback IS consulted, the file it names is asked of git rather than computed in Python — and on **Git for Windows >= 2.46** the file is `%APPDATA%/Git/ignore` whenever that file exists, which the fork prefers over the `$HOME/.config/git/ignore` upstream resolves (#403). Seeding the `$HOME` one there was wrong in both directions: with only the APPDATA file present the seed came back empty and the activation then shadowed the ignore file git really reads, and with both present the worktree over-ignored with patterns git was not applying, so session-created files went missing instead. The gate is the reported version's own `.windows.` fork string plus that floor rather than the platform — Cygwin, MSYS2 and WSL gits run on Windows hardware without the patch — and anything else, an unanswerable `git version` included, keeps the `$HOME` answer. Read from the fork's source through #403, **not measured on a Windows machine**. It then **proves it applies or stands down**: bmad-loop asks git which excludes file it actually resolves **and from which scope** (`git config --show-scope`), so the degrade reason names the winner rather than inferring one — ambient command-scope config (`git -c`, `GIT_CONFIG_PARAMETERS`, `GIT_CONFIG_COUNT`) is told apart from a worktree-scoped write that is not in force at all, which sends you to a different repair (#692). Anything but a confirmed byte-identical match — either of those, an unreadable excludes file, an answer naming no scope, an unanswerable probe — skips the shield with a journaled and notified reason; a match activates it whatever scope supplied the value, since the thing being proved is that git reads the file bmad-loop wrote. Two runs against one repository serialize on an exclusive lock, leaving a zero-length `.git/bmad-loop-shield.lock` — never in the working tree, so nothing your `git add -A` can see; on Windows the wait gives up after ~10s and the shield is skipped naming the lock. Caveats: its gate is the project's **git 2.34** support floor (an older git — or one that cannot say what it is — skips the shield, and the repo-format flag is deliberately not written); that is a _support_ refusal rather than a capability one, since the two things the shield is built out of, `extensions.worktreeConfig` and `git config --worktree`, have existed since git 2.20 — runs are refused outright below the floor, so what this gate still catches on a current host is a git that cannot be spawned, times out, or answers unparseably. Enabling it sets `extensions.worktreeConfig` — a **permanent** repo-format flag, rolled back wherever it could be left set without a working shield, but surviving in two cases the reason distinguishes: a sibling worktree still depends on it, or the rollback could not be made at all. Where it cannot be set safely at all (`core.bare = true` or `core.worktree` in the shared config) the shield is skipped instead. It is skipped the same way where the shared config already carries an explicit `extensions.worktreeConfig` that is not `true` (#396): that line is an operator's own declaration, and enabling over it would rewrite it to `true` permanently with nothing recording what stood there — so the shield stands down and the reason quotes the spelling it found, raw rather than `--type=bool`-normalized, since `false`, `off`, `no` and `0` all read back as `false`. A **valueless** `worktreeConfig` line is not that case: git reads it as `true`, so the flag counts as already carried and the shield proceeds. Lines an older bmad-loop wrote into `.git/info/exclude` are **not** removed for you — delete them by hand. A **file** your project tracks gets no pattern: git applies ignore rules only to untracked paths, so the pattern would shield nothing while making the file read as tracked-and-ignored to `git ls-files -ci --exclude-standard` and to repo-hygiene gates built on it (#392). A tracked **directory** gets no directory pattern either — it is replaced by one pattern per file this provisioning run actually wrote below it (#484). A dir pattern does hide new children, and no pattern shape both hides them and keeps the report clean (`dir/*` with per-child negations clears the report and leaks a new file into the commit), but over a tracked tree it bought new-child coverage alone at the price of a false tracked-and-ignored report for every tracked child — modifications to those stage regardless. The accepted trade: a file the **session** creates under such a directory can be staged, which matches your own decision to track that tree, while everything the orchestrator seeded there keeps a pattern of its own. An **untracked** tool directory is unchanged — it keeps the single `/dir` pattern. If git cannot say what a path is — or a file provisioning wrote below a tracked directory has a name no exclude line can spell (an embedded newline, a trailing carriage return), which would shield nothing and loose an orphan pattern on unrelated files — the pattern is kept in its ORIGINAL shape, dir shape included, and the reason is journaled. Patterns are escaped as they are rendered (#476): a rel holding a trailing space or a wildmatch special (`*`, `?`, `[`, `\`) otherwise names something else, leaving the seeded path unshielded while the broken character class silently hides an unrelated file the unit meant to commit. - Run state never moves into a worktree — `.bmad-loop/` always lives in the main repo; spec paths are persisted relative to the worktree so a kept-failed run stays portable. - The sprint board is **worktree-canonical for the duration of a story** (#350). A board your project _tracks_ needs nothing special: the story's advance is an ordinary modification of a checked-out file and rides the unit commit through the merge. A **gitignored** board is neither checked out nor delivered by one, so it is seeded into the worktree alongside the deferred-work ledger — the orchestrator writes the board through the worktree and `verify_dev` then reads the file it just wrote, which without the seed is a missing file the run dies on rather than a lost write. Its advance is re-applied to the main checkout's board **after** the merge, journaled `board-advance-carried` — or `board-advance-carry-uncommitted` where `git add` refused the ignored path, which is the ordinary outcome for such a board and not a fault (the status on disk is the value; the commit is best-effort). The carry replays from its record if a crash lands between the merge and its latch, and `sprintstatus.advance` never regresses, so a double application is a no-op. On that replay leg the merge — and with it the pre-flight above — has already happened, so the carry proves its own ownership before committing: it recomputes HEAD's content through `advance` and asks **git** whether the board holds that and nothing else, which a crashed pass's half-written advance does and an operator's edit does not. Sameness is git's own — both sides are hashed through the path's clean filter — so a board spelled CRLF by one host and LF by another still matches, where a byte compare would have to guess which spelling is on disk and would refuse a pristine board on every host that chose the other. **Both** places git holds the path are proved, because the carry's `git add` overwrites both: the working tree it copies into the commit, and the index it stages over — an edit staged and then restored in the working tree exists nowhere afterwards, so proving the working tree alone would authorize destroying it. That proof guards the **commit**, which is one write too late for the story's own row: `advance` would already have replaced an operator's status with the target, leaving exactly the bytes the proof accepts — and skipping the commit saves nothing when the value scheduling reads is the one on disk. So that one row is checked **before** the advance as well, and a status that is neither HEAD's nor this pass's own is refused there with nothing written. Both refusals journal `board-advance-carry-foreign-dirt`, and they refuse at different points with different stakes: the pre-advance row check refuses with **nothing written**, so your status is still on the board; the post-advance proof withholds only the **commit** — the advance is on disk by then — and the dirt it declined to take is still escalated by the next run's merge pre-flight. A board that is **gone** by carry time is refused before either question is asked and journaled `board-advance-carry-failed`, the same record a vanished row gets. Every probe fails closed, and git's own dirt answer decides whether either comparison is asked at all — a board nobody wrote is not their question (#618). Both comparisons apply to a board git **tracks**. A gitignored one has no baseline anywhere in git to be compared against, so a replayed carry can still overwrite a row you edited on it while the host was down; the commit half cannot arise there at all, since `git add` refuses an ignored path outright. Scheduling is what depends on it: `_pick_next` reads the **main** board, so an advance that never came back hands finished work to the next run's dev session. - Only the orchestrator writes the board — the session prompts forbid it, and `sprintstatus.advance` is the sole write path. The consequence under isolation, stated rather than left as a surprise: in a _gitignored_ board a session's edit to any OTHER story's row lives only in the worktree copy and evaporates when the worktree is removed. Only the story's own advance is carried back, because only that one is the orchestrator's own write. From f9f9723d55146753184d4d36a6ad0d1116774b7c Mon Sep 17 00:00:00 2001 From: t Date: Tue, 25 Aug 2026 12:32:23 -0700 Subject: [PATCH 5/6] fix(install): match git's own lstat existence predicate for the APPDATA ignore (#403) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The APPDATA arm gated on `Path.is_file()` while claiming in its comment to mirror the fork's `file_exists` precondition. It does not: `xdg_config_home_for` gates on `file_exists(appdata_config)` (`path.c`), and `file_exists` is `lstat(f, &sb) == 0` (`dir.c`) — so a DIRECTORY and a BROKEN SYMLINK satisfy git's test and are selected as `excludes_file`, while `is_file()` rejects both. Rejecting what the fork accepts is not a safe fall-through: it sends the shield to the `$HOME` file git is NOT reading, seeding patterns git does not apply — #403's over-ignore direction, the one that makes session-created files go missing. `_shield_file_exists` now spells git's predicate literally. A non-regular candidate still seeds nothing, and that empty seed is the faithful answer rather than an accident of the caller: git gets no usable patterns from either shape. Traced through the fork's consumption path — a broken symlink is dropped by `access_or_warn(..., R_OK)`, whose ENOENT counts as an ignorable missing file, and a directory reaches `add_patterns_from_file_1`'s `die("cannot use %s as an exclude file")`. The caller's own `is_file()` at the seed read already produces exactly that empty seed, so no new refusal was warranted; raising here would skip the shield over an answer that is knowable. Source-read at v2.46.0.windows.1 and v2.55.0.windows.3 (both identical), absent at v2.45.0.windows.1. Still NOT measured on Windows, as the sibling record states. ABLATIONS, run singly and restored from a post-edit cp snapshot with the md5 re-verified after each restore: - `_shield_file_exists` narrowed back to `candidate.is_file()` -> test_shield_appdata_ignore_directory_is_selected_like_gits_lstat red (the arm rejects the directory, falls through, `home-junk.tmp` seeded); the other four stay GREEN, since pre-fix-compatible behavior is what they assert. - the precondition dropped entirely -> test_shield_appdata_absent_file_keeps_the_home_fallback red with the seed reduced to `/probe-403` alone, i.e. EMPTY. This re-runs a record the rename would otherwise have left stale, its docstring having named `is_file()`. Full suite 6708 passed / 49 skipped, pyright 0 errors, trunk fmt + trunk check --all clean. Raised by CodeRabbit on #724; its diagnosis held, its suggested `GitError` did not. --- src/bmad_loop/install.py | 37 ++++++++++++++++++++---- tests/test_install.py | 62 ++++++++++++++++++++++++++++++++++------ 2 files changed, 86 insertions(+), 13 deletions(-) diff --git a/src/bmad_loop/install.py b/src/bmad_loop/install.py index 0930c082..28a73535 100644 --- a/src/bmad_loop/install.py +++ b/src/bmad_loop/install.py @@ -2010,6 +2010,32 @@ def _shield_undo_extension(worktree: Path, git_dir: Path, common_dir: Path) -> s _APPDATA_IGNORE_GIT = (2, 46) +def _shield_file_exists(candidate: Path) -> bool: + """git's OWN existence predicate, which is `lstat(f, &sb) == 0` (`dir.c`). + + Deliberately NOT `Path.is_file()`. `lstat` succeeds on a DIRECTORY and on a BROKEN + SYMLINK, so `xdg_config_home_for` selects those exactly as it selects a regular + file, and a narrower test here would reject what the fork accepts — sending this + module down the `$HOME` arm git is NOT reading, which is #403's over-ignore + direction rather than a safe fall-through. + + A non-regular candidate still seeds NOTHING, because the caller applies its own + `is_file()` before reading it, and that empty seed is the faithful answer: git + applies no patterns from either shape. A broken symlink is dropped by git's own + `access_or_warn(..., R_OK)` gate, whose `ENOENT` counts as an ignorable missing + file; a directory makes git `die("cannot use %s as an exclude file")` instead, so + there are no patterns to mirror in either case. + + Swallows `OSError` alone — a candidate this process cannot stat is one the shield + must treat as absent, exactly as `file_exists` reports a failed `lstat`. + """ + try: + os.lstat(candidate) + except OSError: + return False + return True + + def _shield_home_git_ignore(worktree: Path) -> Path: """`$HOME/.config/git/ignore` — git's XDG fallback — asked of GIT, not of Python. @@ -2070,11 +2096,12 @@ def _shield_home_git_ignore(worktree: Path) -> Path: appdata = os.environ.get("APPDATA") if appdata: candidate = Path(appdata) / "Git" / "ignore" - # LOAD-BEARING, and it mirrors the fork's own `file_exists` precondition: the - # preference applies only when that file is really there. It is also what - # keeps the cost at one stat on every other platform — only a candidate that - # exists is worth the extra spawn below. - if candidate.is_file(): + # LOAD-BEARING, and it mirrors the fork's own `file_exists` precondition + # EXACTLY rather than approximately — see `_shield_file_exists` for why an + # `is_file()` here would reject a directory and a broken symlink that git + # itself selects. It is also what keeps the cost at one `lstat` on every other + # platform: only a candidate that exists is worth the extra spawn below. + if _shield_file_exists(candidate): # Gated on the FORK STRING, deliberately NOT `sys.platform` (#403). The # preference is a patch carried by one FORK, not a property of the OS: # Cygwin, MSYS2 and WSL gits run on Windows hardware without it, and a diff --git a/tests/test_install.py b/tests/test_install.py index 788efe8b..c99d7818 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -7369,7 +7369,7 @@ def reporting(worktree, *args, timeout_s=None): return seen -def _appdata_and_home_ignores(tmp_path, *, appdata_file=True): +def _appdata_and_home_ignores(tmp_path, *, appdata_file=True, appdata_dir=False): """A `$HOME` global ignore and (optionally) an `%APPDATA%` one, distinguishable. Both carry a pattern of their own so that "seeded the wrong file" and "seeded @@ -7379,7 +7379,11 @@ def _appdata_and_home_ignores(tmp_path, *, appdata_file=True): (home / ".config" / "git").mkdir(parents=True) (home / ".config" / "git" / "ignore").write_text("home-junk.tmp\n", encoding="utf-8") appdata = tmp_path / "appdata" - if appdata_file: + if appdata_dir: + # A DIRECTORY where the fork expects a file: git's `lstat` predicate selects it + # anyway, so the shield must too (see `_shield_file_exists`). + (appdata / "Git" / "ignore").mkdir(parents=True) + elif appdata_file: (appdata / "Git").mkdir(parents=True) (appdata / "Git" / "ignore").write_text("appdata-junk.tmp\n", encoding="utf-8") else: @@ -7387,7 +7391,7 @@ def _appdata_and_home_ignores(tmp_path, *, appdata_file=True): return home, appdata -def _seed_with(project, tmp_path, monkeypatch, *, reported, appdata_file=True): +def _seed_with(project, tmp_path, monkeypatch, *, reported, appdata_file=True, appdata_dir=False): """Run the shield over a real worktree against a faked `git version`; return the private exclude's lines. @@ -7404,7 +7408,9 @@ def _seed_with(project, tmp_path, monkeypatch, *, reported, appdata_file=True): repo = project.project wt = tmp_path / "wt" verify.worktree_add(repo, wt, "feat", "main") - home, appdata = _appdata_and_home_ignores(tmp_path, appdata_file=appdata_file) + home, appdata = _appdata_and_home_ignores( + tmp_path, appdata_file=appdata_file, appdata_dir=appdata_dir + ) seen = _shield_on_reported_git_version(monkeypatch, reported) with monkeypatch.context() as pinned: @@ -7486,19 +7492,59 @@ def test_shield_appdata_ignore_is_the_forks_not_the_platforms(project, tmp_path, assert "appdata-junk.tmp" not in seeded +def test_shield_appdata_ignore_directory_is_selected_like_gits_lstat( + project, tmp_path, monkeypatch +): + """A DIRECTORY at `%APPDATA%\\Git\\ignore` is SELECTED, because git's predicate is `lstat`. + + `xdg_config_home_for` gates the preference on `file_exists` (`path.c`), and + `file_exists` is `lstat(f, &sb) == 0` (`dir.c`) — so a directory satisfies it and + the fork returns that path as its `excludes_file`. A `Path.is_file()` here would + reject what git accepts and fall through to `$HOME`, seeding patterns git is not + applying: the OVER-IGNORE direction, since git never reads the `$HOME` file once it + has selected the APPDATA one. + + What the shield seeds for that shape is NOTHING, and that is the faithful answer + rather than an accident of the caller: the caller's own `is_file()` refuses to read + a directory, and git gets no usable patterns from one either — it dies on it + (`add_patterns_from_file_1`, `dir.c`). An empty inherited seed is what both ends + agree on. + + PROVENANCE: a source read of the fork, as the sibling tests record — the version + string is faked, and nothing here was measured on Windows. + + Ablation: narrow `_shield_file_exists` back to `candidate.is_file()` and this fails + — the arm rejects the directory, falls through to the `$HOME` probe, and + `home-junk.tmp` is seeded.""" + seeded = _seed_with( + project, + tmp_path, + monkeypatch, + reported="git version 2.46.0.windows.1\n", + appdata_dir=True, + ) + + assert "home-junk.tmp" not in seeded + assert "appdata-junk.tmp" not in seeded + # ...and the shield still RAN: the inherited seed is empty, not the whole file. + assert "/probe-403" in seeded + + def test_shield_appdata_absent_file_keeps_the_home_fallback(project, tmp_path, monkeypatch): """`%APPDATA%` set with no `Git/ignore` under it is the ORDINARY case on the fork, and it must reach `$HOME` — the preference is conditional on the file existing. - The `is_file()` precondition mirrors the fork's own `file_exists` guard: git does - not prefer a path it cannot read, it computes the `$HOME` one instead. Dropping it + The `_shield_file_exists` precondition mirrors the fork's own `file_exists` guard: + git does not prefer a path that is not there, it computes the `$HOME` one instead. + Dropping it would not merely seed the wrong file, it would seed NOTHING — a non-existent source reads as an empty seed with `reason is None`, after which the caller activates a worktree-scoped `core.excludesFile` that SHADOWS the operator's real global ignores. That is #403's own harm, and it is silent. - Ablation: drop the `is_file()` precondition and this fails — the arm returns the - absent candidate, the seed comes back empty, and `home-junk.tmp` is missing.""" + Ablation: drop the `_shield_file_exists` precondition and this fails — the arm + returns the absent candidate, the seed comes back empty, and `home-junk.tmp` is + missing.""" seeded = _seed_with( project, tmp_path, From ac31ca85f075a5124337f878ae1825ff5bbee7b7 Mon Sep 17 00:00:00 2001 From: t Date: Tue, 25 Aug 2026 12:49:07 -0700 Subject: [PATCH 6/6] fix(install): refuse a directory-valued APPDATA ignore instead of seeding it empty (#403) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit made the APPDATA arm select on git's own `lstat` predicate and let the caller's `is_file()` turn any non-regular candidate into an empty inherited seed. That is the faithful mirror for ONE of the two shapes and the wrong answer for the other. Traced through the fork's read path, they diverge: - BROKEN SYMLINK — `access(R_OK)` follows the link to a missing target, `ENOENT`, which `access_error_is_ok` classifies as an ignorable missing file. Git prints nothing, loads no patterns, and runs on. An empty seed mirrors that exactly. - DIRECTORY — `access(R_OK)` succeeds on a readable directory, so git goes on to `add_patterns_from_file_1` and dies: "cannot use %s as an exclude file". Git does not run at all. Modelling the second as an empty seed renders a FATAL as a permissive success. The shield's activation writes a worktree-scoped `core.excludesFile` that SHADOWS the broken path, so the unit's `git add -A` would run happily inside the worktree while the operator's own git refuses to run at all — the misconfiguration masked, and files staged where the unshielded command would have halted. A selected-but-unusable path is UNKNOWN, not empty, so it takes the reading this module already gives an unresolvable `$HOME`: raise, and let the caller degrade with a journaled reason naming the path. `is_dir()` follows the link deliberately — a symlink to a directory is the same fatal — while a broken one is not a directory and still seeds empty. Deliberately NOT a refusal of every non-regular shape: that would stand the shield down over a broken symlink git itself is perfectly happy with. Ablation B below pins that. ABLATIONS, run singly and restored from a post-edit cp snapshot with the md5 re-verified after each restore. They redden DISJOINT tests, which is what makes them proof: - `is_dir()` refusal removed -> test_shield_appdata_ignore_directory_degrades_instead_of_seeding_empty red with `reason is None` — the masked-fatal shape exactly. - refusal widened to `not candidate.is_file()` -> test_shield_appdata_ignore_broken_symlink_seeds_empty_not_a_refusal red, the shield degrading over a repo whose git runs fine. Test helpers now take an explicit `appdata_shape` (file / absent / directory / broken-symlink) and `_drive_shield` returns the degrade reason alongside the lines, since two of the four shapes are no longer seed-and-assert. The symlink test is POSIX-only, per the house `skipif(os.name == "nt")`. Full suite 6709 passed / 49 skipped, pyright 0 errors, trunk fmt + trunk check --all clean. Raised by Codex on #724. Source-read at v2.46.0.windows.1 / v2.55.0.windows.3; still not measured on Windows, as the sibling record states. --- src/bmad_loop/install.py | 36 +++++++++-- tests/test_install.py | 129 ++++++++++++++++++++++++++------------- 2 files changed, 118 insertions(+), 47 deletions(-) diff --git a/src/bmad_loop/install.py b/src/bmad_loop/install.py index 28a73535..fe6d90e4 100644 --- a/src/bmad_loop/install.py +++ b/src/bmad_loop/install.py @@ -2019,12 +2019,16 @@ def _shield_file_exists(candidate: Path) -> bool: module down the `$HOME` arm git is NOT reading, which is #403's over-ignore direction rather than a safe fall-through. - A non-regular candidate still seeds NOTHING, because the caller applies its own - `is_file()` before reading it, and that empty seed is the faithful answer: git - applies no patterns from either shape. A broken symlink is dropped by git's own - `access_or_warn(..., R_OK)` gate, whose `ENOENT` counts as an ignorable missing - file; a directory makes git `die("cannot use %s as an exclude file")` instead, so - there are no patterns to mirror in either case. + Selecting is not the same as being able to MIRROR, and the two non-regular shapes + part company right there — the caller distinguishes them: + + - a BROKEN SYMLINK is dropped by git's own `access_or_warn(..., R_OK)` gate, whose + `ENOENT` counts as an ignorable missing file, so git loads no patterns and runs + on. The caller's `is_file()` seeds nothing, which mirrors that exactly. + - a DIRECTORY passes that same `access(R_OK)` gate, so git goes on to + `add_patterns_from_file_1` and `die("cannot use %s as an exclude file")`. Git + does not run at all, and an empty seed would model a FATAL as a permissive + success — see `_shield_home_git_ignore`, which refuses that shape. Swallows `OSError` alone — a candidate this process cannot stat is one the shield must treat as absent, exactly as `file_exists` reports a failed `lstat`. @@ -2112,6 +2116,26 @@ def _shield_home_git_ignore(worktree: Path) -> Path: if version.returncode == 0: reported = os.fsdecode(version.stdout) if ".windows." in reported and git_version_at_least(reported, _APPDATA_IGNORE_GIT): + if candidate.is_dir(): + # SELECTED by git and then UNUSABLE by it: `access(R_OK)` + # succeeds on a readable directory, so git reaches + # `add_patterns_from_file_1` and dies ("cannot use %s as an + # exclude file"). Returning it would seed nothing — and an + # empty seed here is not the faithful mirror it is for a broken + # symlink, it is a FATAL rendered as a permissive success: + # activating a worktree-scoped `core.excludesFile` SHADOWS the + # broken path, so the unit's `git add -A` would run happily + # where the operator's own git refuses to run at all, and the + # misconfiguration would never surface. + # + # `is_dir()` FOLLOWS the link deliberately: a symlink to a + # directory is the same fatal. A broken one is not a directory + # and falls through to be seeded as empty, which is what git + # does with it. + raise GitError( + f"git's global ignore path is a directory ({candidate}) — " + "git for Windows selects it and then cannot read it" + ) return candidate key = "bmadloop.xdghomeprobe" probe = git_bytes( diff --git a/tests/test_install.py b/tests/test_install.py index c99d7818..d1581a5d 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -7369,7 +7369,7 @@ def reporting(worktree, *args, timeout_s=None): return seen -def _appdata_and_home_ignores(tmp_path, *, appdata_file=True, appdata_dir=False): +def _appdata_and_home_ignores(tmp_path, *, appdata_shape="file"): """A `$HOME` global ignore and (optionally) an `%APPDATA%` one, distinguishable. Both carry a pattern of their own so that "seeded the wrong file" and "seeded @@ -7379,21 +7379,26 @@ def _appdata_and_home_ignores(tmp_path, *, appdata_file=True, appdata_dir=False) (home / ".config" / "git").mkdir(parents=True) (home / ".config" / "git" / "ignore").write_text("home-junk.tmp\n", encoding="utf-8") appdata = tmp_path / "appdata" - if appdata_dir: - # A DIRECTORY where the fork expects a file: git's `lstat` predicate selects it - # anyway, so the shield must too (see `_shield_file_exists`). + # The four shapes git's `lstat` predicate can meet at `%APPDATA%/Git/ignore`. They + # are NOT interchangeable: git reads a file, silently skips a broken symlink, and + # dies on a directory, so the shield owes each a different answer. + if appdata_shape == "directory": (appdata / "Git" / "ignore").mkdir(parents=True) - elif appdata_file: + elif appdata_shape == "broken-symlink": + (appdata / "Git").mkdir(parents=True) + (appdata / "Git" / "ignore").symlink_to(tmp_path / "nowhere") + elif appdata_shape == "file": (appdata / "Git").mkdir(parents=True) (appdata / "Git" / "ignore").write_text("appdata-junk.tmp\n", encoding="utf-8") else: + assert appdata_shape == "absent", appdata_shape appdata.mkdir(parents=True) return home, appdata -def _seed_with(project, tmp_path, monkeypatch, *, reported, appdata_file=True, appdata_dir=False): - """Run the shield over a real worktree against a faked `git version`; return the - private exclude's lines. +def _drive_shield(project, tmp_path, monkeypatch, *, reported, appdata_shape="file"): + """Run the shield over a real worktree against a faked `git version`; return its + degrade reason (None when it applied) and the private exclude's lines. The env is pinned inside a `monkeypatch.context()` because conftest's session-scoped `_isolate_ambient_git_ignores` pins `XDG_CONFIG_HOME` for every @@ -7408,19 +7413,28 @@ def _seed_with(project, tmp_path, monkeypatch, *, reported, appdata_file=True, a repo = project.project wt = tmp_path / "wt" verify.worktree_add(repo, wt, "feat", "main") - home, appdata = _appdata_and_home_ignores( - tmp_path, appdata_file=appdata_file, appdata_dir=appdata_dir - ) + home, appdata = _appdata_and_home_ignores(tmp_path, appdata_shape=appdata_shape) seen = _shield_on_reported_git_version(monkeypatch, reported) with monkeypatch.context() as pinned: pinned.delenv("XDG_CONFIG_HOME", raising=False) pinned.setenv("HOME", str(home)) pinned.setenv("APPDATA", str(appdata)) - assert _worktree_local_exclude(wt, ["/probe-403"]) is None + reason = _worktree_local_exclude(wt, ["/probe-403"]) assert seen, "the fake never answered `git version` — the shield read a real one" - return _wt_private_exclude(wt).read_text(encoding="utf-8").splitlines() + private = _wt_private_exclude(wt) + lines = private.read_text(encoding="utf-8").splitlines() if private.is_file() else [] + return reason, lines + + +def _seed_with(project, tmp_path, monkeypatch, *, reported, appdata_shape="file"): + """`_drive_shield` for the APPLIED case: refuses a degrade, returns the lines.""" + reason, lines = _drive_shield( + project, tmp_path, monkeypatch, reported=reported, appdata_shape=appdata_shape + ) + assert reason is None, f"the shield degraded instead of applying: {reason}" + return lines def test_shield_prefers_appdata_ignore_on_the_windows_fork(project, tmp_path, monkeypatch): @@ -7492,42 +7506,75 @@ def test_shield_appdata_ignore_is_the_forks_not_the_platforms(project, tmp_path, assert "appdata-junk.tmp" not in seeded -def test_shield_appdata_ignore_directory_is_selected_like_gits_lstat( +def test_shield_appdata_ignore_directory_degrades_instead_of_seeding_empty( project, tmp_path, monkeypatch ): - """A DIRECTORY at `%APPDATA%\\Git\\ignore` is SELECTED, because git's predicate is `lstat`. - - `xdg_config_home_for` gates the preference on `file_exists` (`path.c`), and - `file_exists` is `lstat(f, &sb) == 0` (`dir.c`) — so a directory satisfies it and - the fork returns that path as its `excludes_file`. A `Path.is_file()` here would - reject what git accepts and fall through to `$HOME`, seeding patterns git is not - applying: the OVER-IGNORE direction, since git never reads the `$HOME` file once it - has selected the APPDATA one. - - What the shield seeds for that shape is NOTHING, and that is the faithful answer - rather than an accident of the caller: the caller's own `is_file()` refuses to read - a directory, and git gets no usable patterns from one either — it dies on it - (`add_patterns_from_file_1`, `dir.c`). An empty inherited seed is what both ends - agree on. - - PROVENANCE: a source read of the fork, as the sibling tests record — the version - string is faked, and nothing here was measured on Windows. - - Ablation: narrow `_shield_file_exists` back to `candidate.is_file()` and this fails - — the arm rejects the directory, falls through to the `$HOME` probe, and - `home-junk.tmp` is seeded.""" - seeded = _seed_with( + """A DIRECTORY at `%APPDATA%\\Git\\ignore` is selected by git and then UNUSABLE by it, + so the shield stands down rather than modelling that fatal as an empty seed. + + Git's predicate is `lstat` (`file_exists`, `dir.c`), so `xdg_config_home_for` + selects a directory exactly as it selects a file. Reading it is where the two part: + `access(R_OK)` succeeds on a readable directory, so git reaches + `add_patterns_from_file_1` and dies — "cannot use %s as an exclude file". + + Seeding nothing here would be the WRONG mirror. The shield's activation writes a + worktree-scoped `core.excludesFile`, which SHADOWS the broken path, so the unit's + `git add -A` would run happily inside the worktree where the operator's own git + refuses to run at all — a misconfiguration silently masked, and files staged where + the unshielded command would have halted. An unusable answer is UNKNOWN, not empty, + which is the same reading this module already gives an unresolvable `$HOME`. + + PROVENANCE: source read of the fork, as the sibling tests record — the version + string is faked and nothing here was measured on Windows. + + Ablation: drop the `is_dir()` refusal and this fails — the shield applies, `reason` + comes back None, and the private exclude carries `/probe-403` with no inherited + patterns, which is the masked-fatal shape exactly.""" + reason, _lines = _drive_shield( project, tmp_path, monkeypatch, reported="git version 2.46.0.windows.1\n", - appdata_dir=True, + appdata_shape="directory", ) - assert "home-junk.tmp" not in seeded - assert "appdata-junk.tmp" not in seeded + assert reason is not None + assert "is a directory" in reason + # ...and it named the path, so the operator can find the thing to repair. + assert str(tmp_path / "appdata") in reason + + +@pytest.mark.skipif(os.name == "nt", reason="POSIX symlinks") +def test_shield_appdata_ignore_broken_symlink_seeds_empty_not_a_refusal( + project, tmp_path, monkeypatch +): + """A BROKEN SYMLINK there is selected too, and unlike a directory it must NOT refuse. + + This is the discriminator for the refusal above: git's `lstat` predicate accepts + both shapes, but its READ path treats them differently. `access(R_OK)` follows the + link to a missing target, giving `ENOENT`, which `access_error_is_ok` classifies as + an ignorable missing file — so git prints nothing, loads no patterns, and runs on. + + An empty inherited seed mirrors that precisely, and refusing here instead would + stand the shield down over a configuration git itself is perfectly happy with. The + `$HOME` file must not be seeded either: git selected the APPDATA path and is not + reading `$HOME` at all. + + Ablation: widen the `is_dir()` refusal to any non-regular candidate and this fails — + the shield degrades over a repo whose git runs fine.""" + reason, lines = _drive_shield( + project, + tmp_path, + monkeypatch, + reported="git version 2.46.0.windows.1\n", + appdata_shape="broken-symlink", + ) + + assert reason is None, reason + assert "home-junk.tmp" not in lines + assert "appdata-junk.tmp" not in lines # ...and the shield still RAN: the inherited seed is empty, not the whole file. - assert "/probe-403" in seeded + assert "/probe-403" in lines def test_shield_appdata_absent_file_keeps_the_home_fallback(project, tmp_path, monkeypatch): @@ -7550,7 +7597,7 @@ def test_shield_appdata_absent_file_keeps_the_home_fallback(project, tmp_path, m tmp_path, monkeypatch, reported="git version 2.46.0.windows.1\n", - appdata_file=False, + appdata_shape="absent", ) assert "home-junk.tmp" in seeded