feat(disk-hygiene): accept_unpublished lane for throwaway repos (#4227)

[kyle-sexton]

Closes #4227

Summary

Adds the accept_unpublished lane the #4227 decision chose. A throwaway local Git checkout (no remote, untracked files, zero commits) can now verify clear in handoff-verify once the operator records an acknowledgement for that exact approved path. Before this, such a checkout could only be deleted outside the engine, with every check skipped. This PR replaces the earlier "contested throwaways stay contested" text, which documented the opposite of the decision.

Fix

• Engine (handoff-verify VCS evidence mode): a vcs-evidence.json repository entry may carry "accept_unpublished": true with a non-empty "reason". The entry is accepted only when its path equals an approved path exactly, so a nested repository or a pattern cannot carry it. The value must be the literal true, and the two keys must appear together.
• With the acknowledgement, gate 1 (empty porcelain) and gate 2 (local heads on the github.com remote) report accepted-unpublished instead of failing. A status or head probe that fails to run still fails closed. Gate 3 (stashes), the repository-set and Git-boundary checks, and every link, mount, handle, identity, and descendant check still apply.
• The verdict's vcs_evidence.accept_unpublished lists each acknowledgement with its reason. Without an acknowledgement, verdicts are unchanged.
• SKILL.md, reference/safety-model.md, and reference/unsupported-platform-handoff.md now require the acknowledgement plus handoff-verify before any deletion the operator overrides, forbid deleting outside the engine, and require warning the operator that unpushed commits and untracked or ignored files will be lost.
• disk-hygiene 0.28.14 (one patch above main), with a CHANGELOG Added entry.

Verification

• New HandoffVerifyTests cases, written first (red, then green):
  • an acknowledged zero-commit repo and an acknowledged committed repo with no remote each verify clear, and the verdict shows the acknowledgement and its reason;
  • the same repo without the acknowledgement stays contested;
  • with the acknowledgement, a symlink, a mount point, an unduplicated stash, a live handle, a late descendant, and a rewritten file each still block;
  • SKILL.md requires handoff-verify before an operator-overridden VCS deletion, forbids out-of-engine deletion, and requires the loss warning (text assertion);
  • in one run, the acknowledgement covers only its own path and not a sibling;
  • the acknowledgement does not cover a dirty nested repository;
  • the validator rejects the acknowledgement on a nested or wildcard path, a non-true value, a blank reason, or a reason without the flag (or the flag without a reason).
• hygiene.test.sh: 492 tests. The only failures are the 11 test_guard_allows_literal_readonly_supporting_bash_commands subtests, which also fail on main on this machine.
• scripts/check-changed-skills.sh origin/main shows those same 11 failures and nothing else.
• check-changelog-parity.sh passes --check, --check-order, --check-bump origin/main, and --check-preserved origin/main.
• check-stale-base-overlap.sh --check origin/main, check-purged-em-dashes.sh, markdownlint-cli2, typos, and scripts/run-ruff.sh check plugins/disk-hygiene all pass.

Related

• disk-hygiene: throwaway local repos cannot be cleared, so they get deleted with no checks #4227 decision comment: the acknowledgement relaxes gates 1-2 only, for one exact approved path.
• disk-hygiene: build an engine-native Windows Recycle Bin apply instead of model-composed deletion commands #4007: the same argument, that without an engine lane the deletion happens anyway with fewer checks.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PT4esxbdC7eQiwQxy35Mie

[github-actions]

PR body contract — issue linkage

This PR body conforms to the issue-linkage contract. Nothing to do.

[claude]

Claude finished @kyle-sexton's task in 3m 16s —— View job

---

Security review complete

• Gather context (PR diff, changed files)
• Review changed files for security-relevant issues
• Post findings (inline or summary)
• Final summary

Scope: plugins/disk-hygiene — hygiene.py's new accept_unpublished acknowledgement (waives the porcelain-status and local-heads-on-remote gates for a throwaway Git checkout), its validation in validate_vcs_evidence, the corresponding gate logic in verify_vcs_checkout_evidence, plus doc/test/changelog/version updates. No .github/workflows files are touched, so nothing routes to the zizmor lane.

No security issues found.

Notes from the review (informational, not findings):

• validate_vcs_evidence (hygiene.py) requires accept_unpublished to be exactly True, reason to be a non-empty string, and path to be an exact member of the operator-approved path list — a nested repository or pattern path is rejected before the flag can attach (covered by test_accept_unpublished_does_not_cover_a_nested_repository).
• verify_vcs_checkout_evidence only relaxes the two documented gates (git-status-porcelain-empty, all-local-heads-on-remote) for the exact acknowledged repository; a failed status/head probe still fails closed (accepted only short-circuits after the git command itself succeeds), and the git-boundary, repository-set, stash, link/mount/handle, and identity/descendant checks are all untouched and still categorical (covered by test_accept_unpublished_leaves_every_other_check_categorical).
• The acknowledgement is scoped per-path (test_accept_unpublished_covers_only_its_own_path) and doesn't weaken validation for any other evidence entry in the same file.
• reason and path values are only ever stored/echoed in JSON output, never interpolated into a shell command or git argv, so there's no injection surface there; the existing remote validation (starts-with "-", no whitespace) guarding git invocation is unchanged.

This is a narrow, well-tested relaxation of an internal safety gate, gated by exact-path matching and a mandatory human-authored reason, with no injection, auth-bypass, or secret-handling issues in the diff.

[claude]

Claude finished @kyle-sexton's task in 4m 20s —— View job

---

/review:code-review findings

Scope: origin/main...HEAD (5 commits, 7 files) at 00ae9614.

1. PR title/description contradicts the shipped diff

The PR title ("contested throwaways stay contested") and the ## Decision section claim:

gates 1–2 stay categorical … There is no accept_unpublished relaxation. … Option A is the safer default (forbid the bypass) rather than an unpaid engine acknowledgement.

But the three most recent commits on this branch (caca1bf, ee0ccea, 00ae961) do the opposite: they add accept_unpublished, a per-path acknowledgement that waives gates 1–2 (git-status-porcelain-empty and all-local-heads-on-remote) for an exact approved path. This is implemented and exercised end-to-end:

• Waiver logic: hygiene.py#L2730-L2763
• Schema validation for the new field: hygiene.py#L2240-L2254
• Documented as intentional: safety-model.md#L185-L194
• Released: CHANGELOG.md#L6-L20 (0.28.8, "Added accept_unpublished acknowledgement…")

So the code, the reference docs, and the changelog all agree with each other on the new decision (allow a scoped, acknowledged bypass) — only the PR's own title and ## Decision block still assert the old decision (forbid any bypass). Read on its own, the PR description tells a reviewer the opposite of what the diff does, and it will become a permanently wrong decision record once merged (this repo tracks these via the four-part claim/basis/as-of/recheck convention, and the ## Decision here states a claim the diff itself falsifies).

Given the PR body's own ## Recheck clause ("the owner funds a per-path unpublished acknowledgement … ") appears to be exactly what triggered the newer commits, this looks like the description simply wasn't updated after the decision was reversed mid-PR, rather than a code bug. Recommend rewriting the title and ## Decision/## Summary to describe the accept_unpublished acknowledgement that was actually shipped (0.28.8), not the 0.27.4 "forbid" decision it superseded.

Fix this →

Code-level review

Walked the accept_unpublished implementation in hygiene.py (schema validation, gate waiver in verify_vcs_checkout_evidence, gate_status helper) against test_hygiene.py's new cases (throwaway clear/contested, per-path scoping, nested-repo exclusion, every other categorical check still enforced, and the validator's malformed-input matrix). The probe-failure paths (non-zero git status/head-lookup errors) still fail closed even when accept_unpublished is set, the acknowledgement is rejected unless it names an exact approved path (not a nested repo or pattern), and the new tests cover those boundaries. No correctness bugs found in this part of the diff.

(Test plan's check-changelog-parity.sh run is author-claimed; not independently re-run here.)

No inline comments posted — the one finding above is about the PR description vs. the diff as a whole, not a specific changed line.

[claude]

Claude finished @kyle-sexton's task in 1m 47s —— View job

---

Security review — skipped (unchanged diff)

• Gather context (PR diff, changed files)
• Determine whether a deep review is needed
• Post result

Skip gate: this head already has a successful security review that still applies.

The three-dot diff origin/main...HEAD at 7568adf9 touches the same 7 files, with the same
line counts, as the diff already reviewed at 00ae9614 (the run linked in the comment above this
one). Diffing origin/main...HEAD against origin/main...00ae961 shows the accept_unpublished
logic itself — validate_vcs_evidence's exact-path/flag/reason checks and
verify_vcs_checkout_evidence's gate-1/gate-2 waiver via the new gate_status helper — is
byte-for-byte unchanged; only the merge commit's unrelated origin/main content (version-bump
baseline, engine_grammar.py, destructive_guard.py, and other plugins) came along for the ride,
none of which is part of this PR's own diff.

No new security-relevant surface to review. The prior finding — no security issues found —
still stands; see that run for the full analysis.

[claude]

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

---

/review:code-review findings

Scope: origin/main...HEAD at de4be60d05f79149e9f9a30773a0dc62070ac59b (7 files, +351/-18), diffed against a1733b03054a6bad14b9362d713a622621e6626f (the current origin/main tip, confirmed as the exact merge-base and matching the PR's own stated diff stats).

• Gather context (PR diff, changed files, prior review comments)
• Check skip gate
• Review changed files for correctness/maintainability issues
• Post findings
• Final summary

Skip-gate check: this lane already reviewed head 00ae9614 with no findings. Diffing that reviewed commit against the current head (isolated to the PR's own changes, not the unrelated origin/main content pulled in by the two merge commits since) shows hygiene.py, SKILL.md, reference/safety-model.md, and reference/unsupported-platform-handoff.md are byte-for-byte unchanged. The only substantive addition is in test_hygiene.py:

• verify_throwaway gained a mounted parameter that mocks the pre-existing hygiene.mount_state, feeding a new "mount" subtest in test_accept_unpublished_leaves_every_other_check_categorical (L5299) that asserts accept_unpublished does not waive the unrelated nested-mount-point check (still drifted).
• A new test_skill_requires_handoff_verify_and_loss_warning pins the three SKILL.md phrases the doc update introduced.

I reviewed both additions directly: the mount_state mock signature (path, known_linux_mounts=None) -> (bool, str | None)) matches the lambda's (path, *_) shape, and nullcontext is already imported at the top of the file (used elsewhere), so nothing here is a dangling reference. The CHANGELOG.md/plugin.json delta is just the 0.28.13 → 0.28.14 re-bump plus a changelog entry restating the shipped accept_unpublished behavior — consistent with the code.

Also confirmed: the PR title and body now match the shipped diff — no repeat of the earlier "contested throwaways stay contested" / "no accept_unpublished relaxation" mismatch a prior review pass on this PR flagged; that appears to have been fixed already.

No new findings. The accept_unpublished gate-waiver logic itself (validate_vcs_evidence, verify_vcs_checkout_evidence, gate_status) is unchanged from the already-reviewed commit and still holds up on inspection: a failed probe (status.returncode != 0) fails closed regardless of the acknowledgement, the acknowledgement is scoped to an exact approved path only (validated before it reaches the gate logic), and an unborn repo (heads empty) never sets heads_accepted, matching the "passed" vs "accepted-unpublished" distinction the tests check for.

(Test-suite/lint commands in the PR's Verification section are author-claimed and were not independently re-run here.)
· branch

[claude]

Claude finished @kyle-sexton's task in 1m 53s —— View job

---

Security review — skipped (unchanged diff)

• Gather context (PR diff, changed files)
• Invoke /review:security-review and follow it exactly
• Post result
• Final summary

Skip gate: this head already has a successful security review that still applies (skip-gate condition 3).

de4be60 is a merge of origin/main (a1733b0) into the previously-reviewed head 7568adf9, done to re-bump disk-hygiene past intervening main releases. Diffing 7568adf9..de4be60 on the PR's own files shows the merge brought in unrelated main content only — additional_protected_path_globs object/absolute-glob support, stdlib_shadowing advisories, the Windows temp-zone size measurement, and the corresponding CHANGELOG.md/version bump to 0.28.14 (from issues #4228, #4217, #4225, #4010, #3855) — none of which is part of this PR's own diff (GitHub's merge-base diff still lists the same 7 files as before). A direct search of that same range for accept_unpublished returns nothing:

git diff 7568adf9..de4be60 -- plugins/disk-hygiene/skills/clean/scripts/hygiene.py \
  plugins/disk-hygiene/skills/clean/scripts/test_hygiene.py \
  plugins/disk-hygiene/skills/clean/reference/safety-model.md \
  plugins/disk-hygiene/skills/clean/reference/unsupported-platform-handoff.md \
  plugins/disk-hygiene/skills/clean/SKILL.md | grep -i accept_unpublished
# (no output)

The accept_unpublished acknowledgement logic (schema validation, exact-path matching, and the gate-1/gate-2 waiver in verify_vcs_checkout_evidence) is byte-for-byte unchanged from the version already reviewed and cleared — see the "No security issues found" review at 00ae9614 and the follow-up skip confirmation at 7568adf9 above.

No new security-relevant surface to review at de4be60. The prior finding stands: no security issues found.