feat(disk-hygiene): hygiene.py handoff-verify — deterministic revalidation for the manual lane (F2)#1145
Conversation
…lane revalidation (F2) dh 0.8.0 — closes #1109. New read-only subcommand bringing snapshot binding to the platforms where engine apply is unsupported, without adding an engine deletion lane: - handoff-verify takes --snapshot plus --paths (the human-approved exact path list; same containment rules as plan candidates) and reruns the identity/reparse/protection/descendant/VCS/handle checks per path against live state, emitting machine-readable verdicts: clear / drifted / gone / contested. No deletion capability. Exit 0 all-clear, 3 otherwise. - Every unverifiable condition (handle tooling, unreadable state, truncated coverage) fails closed into contested; platform execution blockers deliberately do not apply. - Target-root gate is shared with preview (new resolve_snapshot_target helper); handoff-verify uses the engine's established directory object-identity check (device/inode/type) for the root instead of full stat identity, because the manual lane deletes one root-level item at a time and re-verifies — the root's own mtime churn must not invalidate the next item's verify. A replaced root still refuses. - Bash guard admits the exact handoff-verify shape as read-only, including in audit-only mode (the kill switch keeps blocking every deletion lane; verification is reporting). Malformed shapes stay denied. - Clean SKILL.md manual-handoff lane now writes handoff-paths.json, runs handoff-verify immediately before deletion, and acts only on verdict-clear paths; owner-process absence stays a model-side check (snapshot entries carry no owner claim). - Safety model documents the verdict vocabulary, fail-closed mapping, and emission-time-only validity; #1116's affirmation paragraph updated from "planned" to landed. Tests: 12 new (9 engine verdict/validation/exit-code cases including the changed-since-approval path, 3 guard shape cases). Suite: 163 passing locally on Windows. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
Warning Automated security review did not complete — this is an infrastructure failure, not a review verdict. Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."
Re-running the job, or pushing a new commit, will retry the review. |
|
Warning Automated review did not complete — this is an infrastructure failure, not a review verdict. Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."
Re-running the job, or pushing a new commit, will retry the review. |
…ed test coverage, safety-model precision Addresses the local security + code review of the handoff-verify PR (CI review bots infra-fail per #1122, so both reviews ran as local agents): - Unstatable-path branch now splits PermissionError into the established needs-elevation token and other OSError into filesystem-state-unverified, instead of a bare "state-unverified: <exc>" string that broke the <domain>-state-unverified vocabulary and hid elevation cases. - safety-model.md no longer claims the target root is re-validated "exactly as preview does": it names the tolerant device/inode/type root identity and why (per-item manual deletions churn the root's own mtime). - Comment documents why descendant-walk failures treat the live set as unknown rather than empty (contested, not provably drifted). - Five new tests pin the previously untested fail-closed branches: root-mtime churn from a prior deletion does not refuse (the motivating scenario), a replaced root still refuses, PermissionError on lstat maps to needs-elevation, unreadable descendants are contested without a spurious changed-since-scan, and a truncated path can never verdict clear. Suite: 168 passing locally on Windows. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
Claude encountered an error after 1s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 1s —— View job I'll analyze this and get back to you. |
|
Local review compensation for #1122 (claude-review + security-review workflows infra-failed in 0s on this PR — their gate checks pass without a real review): both reviews were run as local agents against the full diff instead.
Suite after fixes: 168 tests passing locally (Windows); this push retries the CI review workflows. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 330a2b8bef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… handoff-verify Codex review P1: a multi-path handoff-verify run lets the earliest path's checks age while every later path is still walked and probed, so its clear verdict is already stale at emission — and staler after each intervening deletion. The manual lane now pairs each deletion with a fresh single-path verify (verify one, delete that one, then the next); the multi-path form is reserved for reporting. Engine note and safety-model verdict table say the same. No engine logic change; suite 168 passing. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
Claude encountered an error after 1s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 1s —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95e68b44b2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…cendant stat is contested, not drifted Two Codex P2 findings on handoff-verify, both valid: - tracked_blocker raising subprocess.TimeoutExpired (hung git) escaped to main()'s generic handler, aborting the run with a single filesystem-state-unverified payload and zero verdicts — breaking the per-approved-path verdict contract. The per-path VCS probe now catches OSError/SubprocessError and degrades that one path to contested vcs-state-unverified. - A denied lstat on a captured descendant flowed through same_identity's blanket OSError->False into changed-since-scan/drifted, telling the lane to rescan when the remedy is resolving access. The per-entry check now stats explicitly: FileNotFoundError -> drifted, PermissionError -> contested needs-elevation, other OSError -> contested filesystem-state-unverified. Two new tests pin both (multi-path timeout still yields all verdicts; denied descendant is contested without changed-since-scan). Suite: 170 passing locally on Windows. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c296a8f7f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… not a whole-run abort Codex P2 (same class as the VCS-timeout finding): lsof vanishing between shutil.which() and subprocess.run() — or a ctypes load error on the Windows probe — raised through candidate_handle_state to main()'s generic handler, emitting one filesystem-state-unverified payload with zero verdicts. The per-path handle probe now catches OSError/SubprocessError and contests that path as handle-state-unverified: handle-probe-failed. Multi-path test pins full verdict coverage under a raising probe. Suite: 171 passing locally on Windows. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
Claude encountered an error after 1s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
Summary
dh 0.8.0 — the disk-hygiene consumer-audit item's largest remaining finding (F2 ambitious path). Adds a read-only
hygiene.py handoff-verifysubcommand that brings snapshot binding to Windows/macOS — the platforms where engineapplyis unsupported — without adding an engine deletion lane. Captures most of the declined Windows-execution-lane (F12) value at a fraction of the risk; #1116's affirmation records this as the intended alternative.Engine (
hygiene.py):handoff-verify --snapshot <s> --paths <p> [--data-root <d>]takes the snapshot plus the human-approved exact path list ({"version": 1, "paths": [...]}— validated with the same containment rules as plan candidates: relative, non-root, no traversal, present in snapshot, non-overlapping) and reruns identity/reparse/protection/descendant/VCS/handle checks per path against live state.clear/drifted(identity or captured descendant set changed) /gone(no longer present) /contested(protection, VCS, live handle, elevation, or unverifiable state). Fail-closed: every unverifiable condition lands incontested, neverclear. Exit 0 all-clear, 3 otherwise. No deletion capability exists in the subcommand.execution-platform-unsupportedblocks the engine lane.resolve_snapshot_target, shared withpreview(behavior there unchanged, full stat identity — the approval token binds one immutable state). handoff-verify passesstrict_root_stat=False: the root is checked with the engine's established directory object-identity (device/inode/type — the same checkapplyuses for directories) because the manual lane deletes one root-level item at a time with a re-verify between items, and the root's own mtime churn must not invalidate the next verify. A replaced root still refuses.Guard (
destructive_guard.py): admits the exacthandoff-verifytoken shape as a read-only invocation — including in audit-only mode (the kill switch continues to block every deletion lane; verification is reporting). Malformed shapes (wrong flag order, missing/undeclared flags, trailing tokens) stay denied. No change to the deny-by-default posture.Docs: clean SKILL.md's manual-handoff lane now writes
handoff-paths.json, runs handoff-verify immediately before deletion, and acts only on verdict-clearpaths (per-item, under the PowerShell guard's final prompt); owner-process absence stays a model-side check since snapshot entries carry no owner claim.reference/safety-model.mdgains a handoff-verify section with the verdict vocabulary table, fail-closed mapping, and emission-time-only validity ofclear.Contract decisions recorded (resolved loudly, not silently): the four verdict classes come from #1109's acceptance criteria; the mapping choices made here are (a) unverifiable →
contested, (b) drifted takes precedence over contested when both apply, (c) root metadata churn tolerated via object identity while root replacement refuses. All three documented in safety-model.md and the CHANGELOG.Test plan
main()exit codes 0/3) + 3 guard (exact shape allowed, allowed in audit-only mode, malformed shapes denied)hygiene.test.shwrapper → OK;check-changelog-parity.sh --check-bump→ pass;check-changed-skills.sh→ CHECK-SKILL clean: PASSRelated
Closes #1109. Source: handoff-inbox item
20260723-021058-disk-hygiene-0-6-4-consumer-audit, finding F2 (ambitious path). Relates to #1116 (F12 decline affirmation — this PR is the recorded alternative) and #1108/PR #1118 (manual-lane TOCTOU discipline this extends).🤖 Generated with Claude Code