ci(disk-hygiene): run the full test_hygiene module in CI

[kyle-sexton]

Closes #2871

Summary

test_hygiene.py has ten TestCase classes. The required Windows lane invoked only GuardTests. This PR adds a dedicated Ubuntu job that always provisions Python and runs the whole module.

Fix

• New disk-hygiene-tests job on ubuntu-24.04: python -m unittest -v test_hygiene, listed in ci-status.needs. plugin-gate already reaches the same module via hygiene.test.sh on non-docs-only diffs; this job is a fast required signal that does not wait on plugin-gate's Node/ShellCheck/full-plugin matrix and cannot take the wrapper's SKIP-when-Python-absent path.
• disk-hygiene-guard-windows stays test_hygiene.GuardTests. A one-line flip of that job to the whole module landed red on windows-2025 (315/317, run 32477560257). The workflow comment names the two failures rather than silently omitting classes:
  • HygieneTests.test_preview_allows_root_children_os_managed_snapshot — expects ready-for-explicit-approval, Windows returns blocked
  • HandoffVerifyTests.test_stash_must_exist_in_an_independent_checkout — path-string compare; 8.3 RUNNER~1 != long runneradmin
• Those two are green on Linux (317/317). The Windows job remains GuardTests-only also because GuardTests is the NT allowlist coverage Linux cannot exercise (disk-hygiene: read-only Bash allowlist is inert on Windows (0 commands accepted); engine-gate hard-allow leak on Linux #2774).

Classes now required in CI via disk-hygiene-tests: HygieneTests, ChildrenRollupTests, VersionFloorTests, StandingPolicyTests, OsAutocleanAdvisoryTests, LeastObservableEnginePathTests, TargetRootIdentityTests, HandoffVerifyTests, GuardTests, DirectReadKillSwitchTests. GuardTests also remain on Windows.

Verification

• Local Linux: python -m unittest -v test_hygiene — 317 tests, OK.
• First CI push: Windows full-module run FAILED the two tests named above. Linux disk-hygiene-tests is green (317 tests, all ten classes collected).
• scripts/check-lane-coverage.sh --check — every lane reachable from ci-status.needs.
• Windows GuardTests lane left as a required NT-specific check.

Related

Refs #2774 (Windows GuardTests lane; NT allowlist). Refs #2840 (fixture git isolation). Refs #2856 / #2862 (same family: coverage that existed but did not gate).

[claude]

Claude finished @kyle-sexton's task in 2m 39s —— View job

---

Claude finished the code review

• Gather context (diff, workflow file, issue test_hygiene.py: nine of ten TestCase classes never run in CI #2871)
• Run /review:code-review per repo instructions
• Post findings as inline PR comments
• Post summary comment

Summary

Reviewed the one changed file, .github/workflows/ci.yml (13/-4), which expands the disk-hygiene-guard-windows job's test invocation from test_hygiene.GuardTests to the whole test_hygiene module, per the PR's stated goal of #2871.

Blocking finding (posted inline on .github/workflows/ci.yml#L348): I checked this PR's own live CI run rather than relying on the author's described local test plan, and the disk-hygiene-guard-windows job is currently failing on commit d124e7c — run 32477560257 / job 96756961862, FAILED (failures=2, skipped=6):

• HandoffVerifyTests.test_stash_must_exist_in_an_independent_checkout — path list mismatch (RUNNER~1 8.3 short name vs. runneradmin long name), a Windows-only path-normalization bug.
• HygieneTests.test_preview_allows_root_children_os_managed_snapshot — 'ready-for-explicit-approval' != 'blocked'.

The PR description states these exact two tests "are already green on current origin/main; no test-file repair was needed" — that was measured on Linux only. Since disk-hygiene-guard-windows is a required lane (ci-status.needs, line 1404), this PR as submitted breaks a required check on its own branch. test_hygiene.py needs a Windows-safe fix for at least the short/long-path comparison before the command-widening change here can land green.

No other issues found in the diff itself — the workflow YAML syntax, step naming, and comment update are otherwise fine, and the approach (widen the command, not swap to a Linux-safe subset) matches the stated intent of catching real coverage gaps.
· Branch

[claude]

Claude finished @kyle-sexton's task in 44s —— View job

---

Security review

• Gather context (PR diff, CLAUDE.md)
• Apply skip gate
• Review changed workflow file for security-relevant issues
• Post findings

No security issues found.

This PR's only change is in .github/workflows/ci.yml: it broadens the disk-hygiene-guard-windows job's test invocation from test_hygiene.GuardTests to the full test_hygiene module, plus a comment explaining why.

• No changes to permissions:, triggers, secrets handling, or the github context inside run: blocks.
• actions/checkout still uses persist-credentials: false; action pins are unchanged.
• The added run: line is a static string (python -m unittest -v test_hygiene) — no interpolation of PR-controlled input, so no script-injection surface.
• This runs unit tests only (per the PR description: tempdir-fixture based, not host-destructive), broadening test coverage rather than introducing new execution surface.

Skip-gate condition 2 applies (no security-relevant surface after reading the diff) — stopping here per the review lane's criteria.

[github-actions]

Last security-reviewed head: d124e7cbab3c4d79b5e863936fa128705f6b539e. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

[github-actions]

Claude has reviewed this PR 5 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[claude]

Claude finished @cursor[bot]'s task in 5m 14s —— View job

---

Code review

• Gather context (diff, CLAUDE.md/REVIEW.md, related scripts)
• Apply skip gate — no reason to skip; job wiring is meaningful review surface
• Review the new disk-hygiene-tests job and its ci-status wiring
• Cross-check against existing coverage (plugin-gate, run-plugin-tests.sh, hygiene.test.sh)
• Post findings

Findings

Posted one inline comment on the substantive issue: the new disk-hygiene-tests job (.github/workflows/ci.yml:359-373) looks like it duplicates coverage plugin-gate already provides. plugin-gate's run-plugin-tests.sh step discovers and runs every plugins/**/*.test.sh, including hygiene.test.sh, which unconditionally runs python -m unittest -v test_hygiene.py (the whole module) once Python ≥3.11 is on PATH — and plugin-gate explicitly installs Python 3.14 for any diff not confined to the docs-only allowlist (.github/workflows/** isn't in that allowlist, so this applies here too). plugin-gate is already required via ci-status.needs. If that reading holds, the "other nine classes never gated" premise doesn't apply to realistic changes today, and the new job re-runs identical coverage under a new name rather than closing a gap. See the inline comment for the full trace and a possible reconciliation (drop the job, or restate the comment's rationale as "faster dedicated signal" rather than "closes a gap").

Other notes (not blocking, FYI only)

• The PR description says the fix "changes the Windows disk-hygiene step from test_hygiene.GuardTests to test_hygiene," but the code at head (9bb0aea) leaves disk-hygiene-guard-windows unchanged (still GuardTests-only) and instead adds the new disk-hygiene-tests Linux job. The description reads like it documents an earlier commit (d124e7c) rather than the current approach — worth updating so reviewers relying on the PR body aren't misled.
• Everything else checked out: disk-hygiene-tests is correctly added to ci-status.needs (.github/workflows/ci.yml:1431), matching scripts/check-lane-coverage.sh's requirement that every job be reachable from that aggregate. test_hygiene.py's platform-specific tests are properly guarded with unittest.skipUnless (POSIX-only Bash export, POSIX path case, etc.), consistent with the PR's claim that the suite isn't host-destructive on Linux. Test-method count (317) matches the file.
• I could not independently execute python -m unittest or scripts/check-lane-coverage.sh --check in this sandbox (command execution is blocked here) — the PR's local-run and lane-coverage verification claims are author-reported and unverified by me; the redundancy finding above is based on static reading of scripts/run-plugin-tests.sh, hygiene.test.sh, and scripts/docs-only-paths.txt, not on running anything.

[claude]

Claude finished @cursor[bot]'s task in 2m 42s —— View job

---

Code review

• Gather context (diff, prior review rounds, CLAUDE.md)
• Apply skip gate — head SHA changed since the last review and it responds to prior findings, so re-review applies
• Review the new commit (44a75e4) against the two prior review findings
• Cross-check the updated rationale against plugin-gate's actual behavior (scripts/run-plugin-tests.sh, hygiene.test.sh, scripts/docs-only-paths.txt)
• Post findings

Findings

Prior finding #1 (Windows job failing) — resolved. Commit 9bb0aea kept disk-hygiene-guard-windows on test_hygiene.GuardTests only (never widened to the full module), so the two Windows-only failures from the first commit (d124e7c) don't apply anymore. Confirmed both disk-hygiene-guard-windows and the new disk-hygiene-tests are green on this PR's own head, run 32478931898.

Prior finding #2 (duplicates plugin-gate coverage) — not resolved, restated in an inline comment. Commit 44a75e4 only reworded the workflow comments in response; the underlying issue is unchanged. .github/workflows/** is not in scripts/docs-only-paths.txt's allowlist, so this PR's own diff is non-docs-only — meaning plugin-gate (.github/workflows/ci.yml:1021-1052) already sets up Python and, via scripts/run-plugin-tests.sh's discovery of every plugins/**/*.test.sh, already runs hygiene.test.sh → the full test_hygiene module on this PR's own head (plugin-gate passed in the same run). The comment's "docs-only gated" justification invokes a scenario that can't occur for a change that actually matters to test_hygiene.py: the allowlist is defined (in its own header) to "feed NO code lane," so a genuinely docs-only diff can't be one where test_hygiene.py coverage is at stake. On every diff where this test module's coverage actually matters, plugin-gate already runs it, and disk-hygiene-tests is duplicate execution rather than gap-closing. Detail and a fix-it link are on the inline comment at .github/workflows/ci.yml#L366.

No other issues found in the diff (YAML syntax, job wiring into ci-status.needs at line 1437, action pins) — this review was scoped to the one changed file per the PR's stated diff.
· Branch

[claude]

Claude finished @cursor[bot]'s task in 1m 32s —— View job

---

Code review

• Gather context (diff, prior review rounds, hygiene.test.sh, docs-only-paths.txt)
• Apply skip gate
• Compare current head against prior findings
• Post summary

Skip gate applied

This trigger fired on a synchronize (new head 8daed61), but the only content added since the last full review round (commit 44a75e4, reviewed at 11:59 UTC) is a merge of origin/main that touches unrelated lines (fixture-git-isolation-gate's comment, from #3089) — confirmed via git diff 44a75e4..8daed61 -- .github/workflows/ci.yml. The disk-hygiene-tests / disk-hygiene-guard-windows job bodies and their comments are byte-identical to what was already reviewed. Per the skip-gate criterion ("this head already has a successful review from this lane that still applies"), this round adds no new inline comment.

Prior findings, current status

1. Windows job failing on its own commit — resolved (commit 9bb0aea). disk-hygiene-guard-windows reverted to test_hygiene.GuardTests only; both it and the new disk-hygiene-tests job are green on this PR's own runs.
2. disk-hygiene-tests duplicates plugin-gate's coverage for this PR's own (non-docs-only) diff — still open. The comment at ci.yml#L356-L365 still argues disk-hygiene-tests is "not a second copy of plugin-gate" via a docs-only-skip scenario, but .github/workflows/** isn't in scripts/docs-only-paths.txt's allowlist, so this PR's own diff isn't docs-only — plugin-gate already installs Python and runs the full test_hygiene module via hygiene.test.sh on this PR's own head (confirmed green in prior runs). The author's rebuttal (PR comment, 11:49 UTC) is that the value is a non-skippable signal (hygiene.test.sh SKIPs exit 0 if Python is absent/below MIN_PYTHON, and plugin-gate short-circuits on docs-only diffs) — that's a real distinction in the abstract, but the comment text still frames it as closing a coverage gap rather than stating that narrower rationale, which was the substance of the prior finding. Not re-anchoring a duplicate inline comment since the code is unchanged from the last round; flagging it here for author/maintainer resolution rather than blocking further reviews from this lane on it.

No new issues found in the diff itself.
· Branch

[claude]

Claude finished @cursor[bot]'s task in 1m 53s —— View job

---

Code review

• Gather context (diff since last reviewed head 8daed61, prior review rounds, CLAUDE.md)
• Apply skip gate — new commit 00bf0e7 since last full review; not skipped
• Review the new commit against prior findings
• Post findings

Skip gate

The last full review round covered head 44a75e4 / merge 8daed61. Since then, commit 00bf0e7 rewrote both comment blocks in .github/workflows/ci.yml to state the "dedicated fast signal" rationale instead of the "closes a coverage gap" framing the prior two rounds flagged — a substantive response to open findings, so this round reviews it in full. Commit 575316e is a no-op merge of origin/main (confirmed git diff 00bf0e7..575316e -- .github/workflows/ci.yml is empty).

Prior findings, current status

1. Windows job failing on its own commit (d124e7c) — resolved since 9bb0aea; disk-hygiene-guard-windows stayed on test_hygiene.GuardTests only.
2. disk-hygiene-tests duplicates plugin-gate coverage / comment overstates the rationale — substantially addressed by 00bf0e7. The job's own comment (.github/workflows/ci.yml:354-362) and the second half of the Windows-job comment (:332-334) now correctly state the real rationale (a fast, always-on, non-skippable signal that doesn't wait on plugin-gate's Node/ShellCheck/full-plugin matrix and can't take hygiene.test.sh's SKIP-when-Python-absent path) rather than claiming a coverage gap plugin-gate doesn't have.

New finding this round

Posted inline — 00bf0e7 rewrote lines 332-334 and 354-362 of this same PR-added block but left lines 323-324 with the old, unqualified framing: "the full module … is the required non-skippable gate, and it lives on disk-hygiene-tests" — the exact "closes a gap" framing the last two rounds flagged, now sitting eight lines above the corrected hedge that says the opposite (disk-hygiene-tests is a dedicated fast signal; plugin-gate also runs the module on non-docs-only diffs). Since the whole block is new in this PR, it's in scope to make internally consistent — reword 323-324 to match the corrected framing already used just below it and in the disk-hygiene-tests job's own comment.

No other issues found — job wiring into ci-status.needs (.github/workflows/ci.yml:1434) is correct, action pins/Python version match the sibling disk-hygiene-guard-windows job, and the run: step is unchanged from prior rounds.
· Branch