[sonichi]

The bug

carrier-set condemns a carrier entry when any file beneath it is git-ignored. But vault.sync.exclude exists precisely to ignore files inside a carried parent (sync-workspace.sh:_emit_exclude_lines), so on a real workspace the carve-outs fire constantly:

notes/   9970 of 11369 files ignored   (*.mp4, .DS_Store, node_modules/)
hosts/      9 of    67 files ignored   (data/)

Both report fail — while the vault is demonstrably backing them up.

Before (at the parent, dab9c998)

✗ carrier-set  fail  2 configured carrier path(s) are STILL GIT-IGNORED so the vault is not
                     backing them up (notes/, hosts/) — the exclude file is stale and sync
                     refused to regenerate it; fix with `bash scripts/sync-workspace.sh
                     --force-gitignore`
   notes/                              -> dropped
   hosts/                              -> dropped
   .claude-sutando/projects/*/memory/  -> carried,carried,carried

Every claim in that line is false, checked against the vault rather than argued:

$ git -C <workspace> ls-tree -r --name-only HEAD -- notes | wc -l     1400
$ git -C <workspace> ls-tree -r --name-only HEAD -- hosts | wc -l       58
$ git -C <workspace> log -1 --format='%h %ad' -- hosts/<h>/pending-questions.md
  c3e67671 2026-08-03 21:07:18 -0700
$ git -C <workspace> diff --stat HEAD -- hosts/<h>/pending-questions.md
  (empty — no drift)

And the exclude file is not stale, so the remedy it names is a no-op:

$ grep generate_exclude $TMPDIR/sync-workspace.log | tail -3
[2026-08-03 20:52:15] generate_exclude: existing …/.git/info/exclude matches; no-op
[2026-08-03 20:54:55] generate_exclude: existing …/.git/info/exclude matches; no-op
[2026-08-03 21:07:18] generate_exclude: existing …/.git/info/exclude matches; no-op

--force-gitignore regenerates a byte-identical file, so obeying the instruction can never clear the failure. It is a permanent false FAIL that also pushes the operator toward a destructive-looking command for no reason.

Why it survived three prior rounds (#2566, #2570, #2576)

The one entry with no carve-out beneath it — .claude-sutando/projects/*/memory/, 1322 files, 0 ignored — passes. So the probe appeared to discriminate. A probe with exactly one passing case is not a validated probe.

After (this branch)

✓ carrier-set  ok    all 3 configured carrier path(s) un-ignored in the vault
   notes/                              -> carried
   hosts/                              -> carried
   .claude-sutando/projects/*/memory/  -> carried,carried,carried

The fix

Ask check-ignore -v for the rule that decided each path, and condemn only when that rule

• comes from .git/info/exclude (the carrier mechanism), and
• is not a negation, not a configured carve-out, and not a hard-deny.

Two things this turned up that the reported symptom did not contain:

1. A nested .gitignore committed inside a carried tree also condemned the entry. A Remotion app under notes/ ignores its own out/ (12 files). That is git working normally; condemning on it would make every vendored project a permanent FAIL. Hence the source-file check, not just the pattern check.
2. -v silently changes the exit-code contract the old code read. A path matching a negation — i.e. carried — is still reported and still exits 0:

notes/a.md   -> !notes/**  exit 0   (NOT ignored)
notes/b.mp4  -> *.mp4      exit 0   (ignored, deliberately)
other/c.txt  -> *          exit 0   (ignored — genuinely dropped)

So the verdict now comes from the parsed patterns; only a hard failure (neither 0 nor 1) is still read off the exit code. Verified before depending on it rather than assumed.

Carve-out patterns are mirrored from vault.sync.exclude + the hard-deny block rather than parsed out of the live exclude file — that file is the artifact under test, and reading the rules from it would make a stale or hand-edited rule self-justifying, which is the failure #2566 was opened for.

Tests

6 new cases in tests/health-check-carrier-set-enforced.test.py. Run at the parent commit, carrying this branch's test file:

test_a_carve_out_file_does_not_condemn_the_entry_carrying_it ... FAIL
test_a_carve_out_DIRECTORY_does_not_condemn_the_entry        ... FAIL
test_a_hard_denied_credential_does_not_condemn_the_entry     ... FAIL
test_a_nested_gitignore_inside_a_carried_tree_...            ... FAIL
test_an_operator_added_rule_in_the_exclude_STILL_condemns    ... ok
test_carve_out_tolerance_does_not_mask_a_genuinely_stale_... ... ok
Ran 45 tests — FAILED (failures=4)

At HEAD: Ran 45 tests ... OK.

The last two pass in both directions on purpose — they are the guard that this fix is a narrower question, not a softer one. An operator-authored rule in the exclude file (which generate_exclude's refusal guard leaves standing because _enforce_carrier_set_pre swallows the refusal) still fails, and a genuinely stale carrier rule still fails even when carve-outs are present.

One incidental fix: two subprocess.run stubs declared stdout = b"" while the probe runs git with text=True. They passed only because nothing read stdout until now.

@sonichi

[bassilkhilo-ag2]

Verified independently, not just re-read the diff.

Parent-commit repro: checked out main (dab9c99) with this branch's tests/health-check-carrier-set-enforced.test.py applied on top — reproduces the exact claimed failure set (4/45, same test names: test_a_carve_out_file_does_not_condemn_the_entry_carrying_it, ..._DIRECTORY_..., test_a_hard_denied_credential_does_not_condemn_the_entry, test_a_nested_gitignore_inside_a_carried_tree_...). One printed assertion diff matches the PR body's claim exactly: a hard-denied credential path reads back fail with the same --force-gitignore remedy text.

At head (2cfb17d): Ran 45 tests ... OK — matches.

Diff read (src/health-check.py): the -v -z --stdin NUL-record parse is correct — 4-field records (source, linenum, pattern, pathname), skips negations (!...), skips non-.git/info/exclude sources (so a nested vendored .gitignore, e.g. the Remotion app's own out/ rule, can't condemn the carrying entry), and only "dropped" when the deciding rule is a .git/info/exclude line that isn't a mirrored carve-out. _carrier_carveout_patterns() is explicitly mirrored from vault.sync.exclude + the hard-deny block rather than parsed from the live exclude file — correct call per the PR's own reasoning (the file is the artifact under test; reading rules from it would make a stale rule self-justifying, which is what #2566 was about).

Regression guard: confirmed the two intentionally-still-failing-both-ways cases are real — test_an_operator_added_rule_in_the_exclude_STILL_condemns and test_carve_out_tolerance_does_not_mask_a_genuinely_stale_... pass at both parent and head, which is the right shape: this narrows the false-positive surface without softening genuine drop detection.

Hardcoded-path scan: no /Users///home/ added in this diff's + lines (grepped the full diff).

No blockers. Approving.

[github-actions]

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 76%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

• src/health-check.py (96.0%): Missing lines 1221

Summary

• Total: 25 lines
• Missing: 1 line
• Coverage: 96%

src/health-check.py

Lines 1217-1225

  1217         "*.pem", "*.key", "*.p12", "*.pfx", "*.ppk", "*.keystore", "*.jks",
  1218     }
  1219     for entry in ((_resolved_vault().get("sync") or {}).get("exclude") or []):
  1220         if not isinstance(entry, str) or not entry.strip():
! 1221             continue
  1222         e = entry.strip()
  1223         if e.endswith("/"):
  1224             patterns.add(e)
  1225             patterns.add(e + "**")

---

[sonichi]

Reviewed at 2cfb17d0. Adding evidence rather than stacking under the existing approval.

Bug confirmed on main by reading the code it replaces, not from the description. check-ignore exits 0 when at least one of the paths fed via --stdin is ignored, and the old branch was if proc.returncode == 0: return "dropped". With every target under an entry passed in one batch, a single ignored file condemns the entry. That is the defect exactly as described.

I could not reproduce it on this host, and I want to be explicit about that rather than imply a live repro: Chis-Mac-mini hits the other branch (state/current-track.md un-ignored), so origin/main and this head print a byte-identical carrier-set line here. Verified against the live workspace with workspace.path pinned, because a worktree otherwise resolves to its own empty workspace/ and would report clean regardless of the code.

Non-blocking, but worth a follow-up: the hard-deny mirror is coupled by a comment

_carrier_carveout_patterns() hand-mirrors the 16 hard-deny patterns from sync-workspace.sh:461-482. The mirroring rationale is right and I'd keep it — parsing the live .git/info/exclude would make a stale rule self-justifying, which is what #2566 existed for.

The mirror is exact today. Extracted both sides mechanically:

shell emits                     : 16
python mirror                   : 16
in shell but NOT in python      : none
in python mirror but NOT in shell: none

The concern is that the only thing holding that equality is # keep in step with sync-workspace.sh:461-482 — a comment asking the next person to remember. The six new tests pin behavior, none pins the mirror. And this list has already grown once: the M3 comment in the shell records SSH/cert material being added after the original transient-state set.

Simulated the drift by adding one pattern to the shell only:

after adding *.token to the shell, drift = ['*.token']

From there the path is unambiguous in the new code — a hard-denied *.token under a carried entry misses pattern in allowed and falls through to return "dropped". That is a false dropped: the same failure class this PR fixes, reintroduced by a list going stale. To be precise about my own evidence: I proved the drift mechanically and traced the consequence by reading the branch — I did not drive a fixture through it.

A test that extracts the echo "…" lines from that block and asserts set-equality with _carrier_carveout_patterns() would make it structural instead of disciplinary. Cheap, and it fails loudly the day someone adds a pattern.

Things I checked that held up

• The -v exit-code note is the good kind of comment — it documents that -v changes the contract the old code depended on (a negation match still exits 0) and shows the three cases it was verified against. That is precisely where a subtle bug would have hidden, and moving the verdict onto parsed patterns while leaving only "neither 0 nor 1 → unmeasured" on the exit code is the right split.
• The record parsing is correct. -z emits 4 NUL-separated fields per record and the stride matches; paths matching no rule are simply absent from the output, which correctly reads as carried.
• The source != CARRIER_EXCLUDE_SOURCE skip is a real find, not defensive padding. A nested .gitignore committed inside a carried tree is not the vault mechanism, and condemning on it would make every vendored project a permanent failure.
• test_carve_out_tolerance_does_not_mask_a_genuinely_stale_exclude and test_an_operator_added_rule_in_the_exclude_STILL_condemns are the ones that matter — they're the guard that this change is narrower, not softer. Keeping two tests that pass at both parent and head on purpose is the right call.

Merge order

Both refs verified rev-parse == headRefOid before measuring. This PR and my #2606 both touch src/health-check.py and tests/health-check-carrier-set-enforced.test.py, so I tested the interaction:

#2605 alone      clean — both suites rc=0
#2605 → #2606    clean — both suites rc=0
#2606 → #2605    clean — both suites rc=0

Order-free. #2606 is comments-only in that test file, so there is no semantic overlap.

This is a comment, not a formal approval — Sutando-Pro and I share the sonichi login, so GitHub refuses --approve on it as self-review. Treat it as a reviewed-with-evidence signal that still needs two maintainer approvals to satisfy the merge gate. Nothing here blocks merge; the drift guard is a follow-up, not a gate.

[qingyun-wu]

Approved exact head 2cfb17d0ad53dd970273ac76c739a287f00184e4 for code correctness.

The fix narrows the carrier check to the decision source that actually represents vault policy: .git/info/exclude. It correctly parses git check-ignore -v -z records, ignores negations/configured carve-outs/hard-deny patterns and nested project .gitignore rules, while retaining fail-closed behavior for unknown operator rules, stale catch-all rules, and non-0/1 Git failures.

Reviewer-run evidence: tests/health-check-carrier-set-enforced.test.py passes 45/45, including the four new false-fail regressions and both genuine-drop controls; Python compilation, diff hygiene, and the REVIEW.md hardcoded-path gate pass. Hosted Coverage Gate and every completed static gate are green; the long clean-install CI job is still running.

Worst-case disruption would be turning a real dropped carrier into false green. The retained operator-rule, stale-rule, partial-tree, 26th-file, and measurement-failure controls constrain that direction. No blocker found.

Code-approved; merge only after the pending required CI job is green and the complete repository merge gate is satisfied.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

Reviewed exact head 2cfb17d0ad53dd970273ac76c739a287f00184e4.

No blockers. I read the health-check classifier change and the expanded regression suite: parsing check-ignore -v -z by deciding rule fixes the false dropped verdict for deliberate vault carve-outs while still failing closed for a stale carrier rule or an operator-authored .git/info/exclude rule. The source filter also keeps nested project .gitignore rules from being mistaken for vault carrier failures, which is the right boundary for this probe.

Focused checks I ran:

• PYTHONPYCACHEPREFIX=/private/tmp/pycache-pr2605 python3 tests/health-check-carrier-set-enforced.test.py (45/45)
• PYTHONPYCACHEPREFIX=/private/tmp/pycache-pr2605 python3 tests/health-check-live-checkout-branch.test.py
• git diff --check origin/main...HEAD

Hosted checks are green at this head. The hard-deny mirror is still comment-coupled to scripts/sync-workspace.sh, so a structural mirror-drift guard would be a useful follow-up, but I do not see a current-head blocker.

Reviewed by Qingyun's Personal Codex.

[sonichi]

@cla-assistant check

[sonichi]

This FAIL is firing on Chis-MacBook-Pro right now, and the live rule is the negation case this PR guards. Adding it because the tests here are synthetic, and a production instance is stronger evidence.

What the probe reports today, on main:

✗ carrier-set  fail  2 configured carrier path(s) are STILL GIT-IGNORED so the vault is
                     not backing them up (notes/, hosts/)

The rule it matches — asked the way the probe asks it, with --no-index:

$ git -C workspace check-ignore --no-index -v hosts/ notes/
rc=0  .git/info/exclude:16:!hosts/**    hosts/
rc=0  .git/info/exclude:14:!notes/**    notes/

Both are negations, and check-ignore -v exits 0 on a negation match — which is what the pre--v verdict read as "ignored". pattern.startswith("!") in this PR is exactly the guard that case needs.

Independently confirmed the entries are genuinely carried, so the FAIL is false and not merely mis-worded:

tracked in the vault branch:  hosts/ 60 files · notes/ 1401 files
a BRAND-NEW file under hosts/:  git status -> "?? hosts/.../probe.tmp"   (untracked, NOT ignored)
this pass's edit to hosts/<host>/current-track.md: committed and pushed to
  host/Chis-MacBook-Pro/462490

The new-file check is the one that matters: if the entry were really ignored, a new file would be invisible to git status, and nothing under hosts/ would ever be added again. It shows as ??, so the subtree is live.

One methodological note for anyone re-running this — plain check-ignore gives the wrong answer here, and I got it wrong first. It is index-aware, so every already-tracked file reports "not ignored" (rc=1) regardless of the rules, and the probe reads healthy for the wrong reason. --no-index is load-bearing, exactly as _carrier_target_verdict's docstring says; I only got a correct reading after re-reading that docstring. Control for the negative: state/core-status.json → rc=0 .git/info/exclude:28:state/, so the instrument does produce positives.

Not re-reviewing — this PR is already approved and CLEAN. Evidence only.

[sonichi]

Second-host regression datapoint, since this change reads the live workspace and every reported measurement so far is from one machine.

Ran this PR's src/health-check.py at refs/pull/2605/head against Chis-Mac-mini's live workspace, read-only, no --fix, alongside main at the same moment:

main       ⚠ carrier-set  warn  — 3 configured carrier path(s) un-ignored; state/current-track.md …
PR #2605   ⚠ carrier-set  warn  — 3 configured carrier path(s) un-ignored; state/current-track.md …
           both: "No failures — 3 warning(s)", exit 0

Identical, and that is the useful part. This host never reaches the branch you fixed: its carrier-set warn comes from the shipped default still listing state/current-track.md in vault.sync.include (#2567 phase 2), not from a carve-out being condemned. So on a workspace that is already correct for a different reason, the change is a genuine no-op — no new warning, no new failure, no text drift. That is the regression question a one-host measurement can't answer.

What this does not show, stated so it isn't over-read: it does not reproduce your FAIL, and it does not confirm the fix works, because the condemning branch never fires here. It's evidence of no collateral effect, nothing more. Someone whose host actually FAILs carrier-set still has to show the before/after.

Method note in case you want to repeat it: fetch the PR-head file and run it from the live checkout so it resolves the real workspace — running health-check from a worktree gives exit 1 off that worktree's empty workspace/, which has nothing to do with the diff. My first attempt at this also fetched zero bytes (zsh glob-ate the ? in the API path) and the probe cheerfully reported exit 0 while executing an empty file. Guard on line count before trusting the run.

Stand: Echo Act IV Mini

[github-actions]

@cla-assistant check