feat(gates): enable the silent-revert guard#6157
Merged
Merged
Conversation
A change that byte-exactly restores the pre-landing bytes of every file a recent commit touched reads as ordinary work in review — a plausible diff, not an undo — so nothing currently catches it. It has landed on main and silently erased a real change. A stale worktree, a mis-resolved merge, or `git add -A` over old content all produce the shape. The classifier existed but was never wired to anything, and two defects would have kept it dark even if it had been: * Its adapter walked `git log --merges`. Squash-merged PRs are SINGLE-parent commits, and squash is how nearly every PR lands here — so the walk could not see the very shape it was built for. Replayed against the real incident in this repo's history, the as-built adapter returns 9 records, none of them the landing, and does not fire. It now walks --first-parent. * Its window was `--since="<float> hours ago"`. git's approxidate accepts that and silently means something else: measured here, "72.0 hours ago" matches all 4367 commits (no filter at all) while "120.0 hours ago" matches zero — taking the guard offline with no error. Only the integer form parses as intended. The window is now an absolute ISO cutoff. Adds the two things needed to actually run it: the proposed-change computation, measured from the merge-base rather than tip-vs-tip (tip-vs-tip would flag every branch that is merely behind the base, since its blobs at paths landed since it was cut ARE the pre-landing bytes), and revert-intent detection, without which every legitimate `git revert` would be blocked. Candidate landings are path-filtered to those sharing a path with the push — a superset of what can fire, since a wholesale revert must restore every path its landing touched — which keeps it to a handful of git calls. Local only: pre-push and gates.sh, pure blob-sha comparison against local git, no build and no network. Degrades to a pass when git cannot answer. A deliberate revert passes via a `Revert "..."` subject, a `Revert-Of:` trailer, or `Silent-Revert: skip reason="..."`. Version-Bump: skip reason="local gate tooling only; no SDK surface, API, or runtime behavior change"
…e in time The guard billed itself as zero-cost, but measured 18s on a real 5-file push. The path filter narrowed which landings were candidates; it did not stop each one from costing two blob reads per file it touched. Two candidates touching 561 files between them meant ~1124 git invocations. A landing can only fire if the push restores EVERY path it touched, so one whose paths are not all present in the push is already a guaranteed False. Checking that against the name listing alone — before reading any blob — rejects the large, unrelated landings immediately. Same verdicts, 18s -> 0.27s. Also threads `now` through check_push. Without it the recency window is always wall-clock, so a historical incident can only ever pass — outside the window, for the wrong reason, and indistinguishable from the guard being broken. The end-to-end test now replays the real incident at the moment it happened, and separately asserts the same commits pass once they are old. Version-Bump: skip reason="local gate tooling only; no SDK surface, API, or runtime behavior change"
The originating branch of the silent revert is still on the remote, which turns the load-bearing claim from an inference into a measurement. Replaying it against main's tip as it stood when it was pushed: its merge-base with the landing IS the landing, so it was up to date, and the reverting bytes were already in the branch. A pre-push check can therefore see them — which is what justifies putting the guard there rather than at merge time. It blocks, and names all five files it would have erased. Without this the suite proved only that the guard recognizes the shape once it is already on main, which is a post-mortem rather than a gate. Skips cleanly if the branch is ever deleted; a fabricated stand-in would prove nothing. Version-Bump: skip reason="test-only; no SDK surface, API, or runtime behavior change"
The approxidate trap was pinned by asserting `_since_arg`'s return value. That is white-box in the one place it cannot afford to be: it cannot fail if the walk stops passing the cutoff to git at all, and it cannot fail in the direction that actually matters. Reverting the fix, the end-to-end tests stay GREEN — the float form matches the ENTIRE history, so the landing is still found and the guard still blocks, for the wrong reason. A window that matches everything and a window that works are indistinguishable from the block direction alone. Drives the real walk over the real landing and moves only `now`: inside the cutoff it is found, 73 hours later it must be gone. Reverting the fix now fails this behaviorally (the landing is returned regardless of `now`), which is the same shape as the guard silently returning zero records and going offline. Also stop the ISO assertion from raising. `fromisoformat` on a non-absolute cutoff — precisely what this test exists to catch — threw ValueError and aborted the suite at that point, hiding all eleven later tests. It now reports a clean FAIL, so a red-before stays readable exactly when the rest of the coverage is the thing being consulted. Version-Bump: skip reason="test-only; no SDK surface, API, or runtime behavior change"
The gate rejects a push at the pre-push lane, so the skill that owns "a gate rejected my push" has to be able to explain it — otherwise the first agent to hit it learns the shape by guessing, and the cheapest wrong guess is the skip trailer, which is the one response that defeats the gate's whole purpose. Names the real cause it is usually reporting (a stale branch — rebase), the two ways a deliberate revert states intent, and the fact that it is local-only and degrades to a pass, so nobody debugs it as a CI failure.
… signing config A throwaway fixture repo inherits commit.gpgsign from the global config. Where signing is on and the key is unusable, every commit in the fixture dies and the whole suite aborts on a traceback instead of reporting a result — an environment-dependent red on the required gate, since this test is registered there. Every sibling temp-repo test already disables signing; this one did not. Also drop the bare issue numbers from two test names. The lint is comment-aware, so a Python function name slips past it — the narrative belongs in the commit message, not the test name.
The silent-revert-guard selftest's approxidate sub-check runs a live `git log --first-parent --since=...` comparison to demonstrate that a float duration silently matches the entire history. CI's build job checks out at the default fetch-depth=1, which grafts the first-parent walk at the tip, so every --since window returns a single commit and the "float matches everything, int does not" assertion cannot hold (float=1 int=1 total=1). The three arithmetic assertions above already pin the _since_arg contract; skip only the live demonstration when the repository is shallow, mirroring the existing "history not present" skip. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017CdzQZrrtCETcDYH9MABhR
danielraffel
force-pushed
the
feature/enable-silent-revert-guard
branch
from
July 22, 2026 00:28
924b965 to
33108a1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Enables the already-built silent-revert guard: a byte-exact revert of a recent landing — bundled with real work, which is what makes the shape hard to catch in review — is now caught pre-push.
Also fixes a latent red on the required gate. The fixture repo inherited the developer's global
commit.gpgsign. Where signing is on and the key is unusable, every commit in the fixture dies and the whole suite aborts on a traceback instead of reporting a result — and this test is registered on the required macOS gate. Every sibling temp-repo test already disables signing; this one did not. Proven red-before/green-after (aborts on a traceback without the fix; 58 assertions pass with it).Two test names carrying bare issue numbers were renamed — the docs-noise lint is comment-aware, so a Python function name slips past it. That narrative belongs in the commit message.
Scope note — the bundle was one item, not three. The shipyard-lane flaky retry is already on main (2026-07-15), and the stale
ship/<PR>.lockcleanup is a reasoned won't-build: the only safe scope races a concurrent ship start for marginal gain, and it belongs to the shipyard repo, not pulp.🤖 Generated with Claude Code
https://claude.ai/code/session_018qWM833UhHu4DFu5fXcFnF
🔎 Provenance
claudem3SL/Volumes/Workshop/Code/pulpe135c3c1-8355-4030-aa59-05f60adaf16cResume
Jump to this tab
Relaunch (any agent)
Restore URL — https://claude.ai/code/session_018qWM833UhHu4DFu5fXcFnF
stamped 2026-07-16 18:23 UTC
Summary by cubic
Enable the new pre-push silent-revert guard to block branches that byte-exactly undo a recent landing, with fast local blob-SHA checks and clear escape hatches for intentional reverts. Wired into pre-push and
gates.sh, with full tests (including a real-incident replay) and docs.New Features
tools/scripts/silent_revert_guard.py; integrated in.githooks/pre-pushandtools/scripts/gates.sh(report/hint modes).Revert "...",Revert-Of:, orSilent-Revert: skip reason="...".--first-parent, use absolute ISO--since, path-filter candidates before blob reads; degrades to pass if git can’t answer.tools/scripts/test_silent_revert_guard.py(real-incident and e2e), registered intest/cmake/quality_tests.cmake; updated CI skill docs; skip live approxidate demo on shallow clones to avoid false CI failures.Bug Fixes
commit.gpgsign=falsein temp fixture repos.Written for commit 924b965. Summary will update on new commits.