You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
scripts/check-silent-revert.sh answers one question about a recorded incident: does the deleting
commit still produce a finding. It never asks whether the content that commit deleted is on main
today. So an incident can be detected, recorded, partially re-landed, and left permanently
incomplete while every signal stays green — the same "the audit trail looked healthy from every angle
a reader normally checks" failure issue 2691 was filed for.
Not hypothetical. It already happened, and a hand audit found it, not the canary: issue 2828 and its
PR #2829 restored plugins/disk-hygiene/README.md and plugins/disk-hygiene/skills/clean/evals/evals.json, which PR #2635 added, PR #2639's squash
deleted, and two separate re-lands (PR #2714, PR #2803) each missed. They were absent from main
for 31h28m (f603880da → 534eac138, 113298s).
Evidence
Verified against origin/main at 2de57a3798489d2c205fa21d3233b70fec5a42c9, where scripts/check-silent-revert.sh is 543 lines.
The mechanism does not exist.verify_known_incidents (scripts/check-silent-revert.sh:445-492)
reads <expectation> <sha> <note> rows, calls scan_commit, and branches on its exit status alone
(:458-459). The note is echoed verbatim, never parsed. scan_commit (:316-391) works entirely
between a commit and its own parent — git blame ... "$parent" (:306), git rev-list --first-parent -n "$WINDOW" "$parent" (:350), git diff --name-only -z --diff-filter=MD "$parent" "$sha" (:365). Nothing in the file resolves the
current tree. scripts/check-silent-revert.test.sh (531 lines) has no restoration case and contains
no git grep or git ls-files at all; its replay section (:432-476) asserts only "replay passes
when a recorded incident still fires" (:446).
The canary was green while the content was missing. At 71ca05a39 (534eac138^, PR 2830 — the
last commit before the hand fix), the eval case PR #2635 added is provably absent:
$ git grep -c "empty-directories-remain-first-class-tidiness-findings" \
71ca05a39 -- plugins/disk-hygiene/skills/clean/evals/evals.json
(no output, exit 1 — zero occurrences)
$ bash scripts/check-silent-revert.sh --verify-known-incidents
ok f603880da fires as recorded (#2639 dropped #2635 (346 lines), 13 minutes later)
ok 9239f1541 fires as recorded (#2641 dropped #2639 (451 lines), 10 minutes later)
ok cc58cbc53 fires as recorded (#2633 dropped #2632's rollups (853 lines) -- unfiled until now)
ok c8470efd0 stays clean as recorded (docs(conventions) rewrote 129 lines of a doc #2679 had just added)
Canary reproduces every recorded incident at the shipped settings.
EXIT=0
Exit 0, on a tree where the incident's content is measurably still gone. (That transcript is the
corpus as it stood at 71ca05a39; PR #2832 has since rewritten the cc58cbc53 note. The replay
still exits 0 on current main.)
Two obvious designs are already ruled out. A path-level check is a false negative on this exact
instance: 9239f1541 (PR #2641) touched both files ten minutes after the incident, so "was this path
revisited since" returns true for both. A verbatim-hunk check stays red forever on the README half:
issue 2828 records that PR #2635's README hunk was malformed and must not be restored byte-for-byte,
and PR #2829 restored the intent instead.
Content markers do work, measured on this incident. One line from PR #2635 survived PR #2829's
non-verbatim rewrite intact:
marker
a95f240f7
f603880da
71ca05a39
534eac138
2de57a379
Safe tidiness is the primary objective (README.md)
Markers must be resolved path-scoped. Both strings also occur elsewhere in the plugin on current main — the engine script, its test file, and CHANGELOG.md — so a repo-wide git grep would have
reported both files restored while they were missing.
No sibling gate covers it.scripts/check-stale-base-overlap.sh:11-19 disclaims this class by
name ("That class belongs to scripts/check-silent-revert.sh"), and its two overlap steps are if: github.event_name == 'pull_request' (.github/workflows/ci.yml:704, :709) — a pre-merge
check on one branch, structurally silent about whether an old incident's content is on main.
Why the current behavior is wrong
The incident corpus is described in the canary's own header as its "honesty proof". It proves
detection fidelity and nothing else: the incident it detected can be two-thirds fixed and the proof
still passes — exactly the property issue 2691 named as the danger, a green signal that has stopped
meaning what a reader takes it to mean.
The header rejects curated marker strings, and that rejection is right for discovery: "Nobody had
registered #2644, #2642, #2635 or #2639 -- registration happens after you already know a fix matters"
(:62-63). That objection does not apply here. For a row already in scripts/silent-revert-incidents.txt, the knowledge exists and registration costs one line. Issue
2691's own suggestion 3 proposed precisely this — "assert that a small set of marker strings from
recently-landed fixes still exist on main" — and the shipped canary implemented suggestion 2's
shape instead. The marker half was never built.
Cost note: this must not re-scan every acknowledged incident on every run. The corpus is four rows
and a marker check is one path-scoped git grep per marker, so it fits inside the existing push: main job. Anything heavier belongs in a separate on-demand mode — but an on-demand mode alone
is insufficient, because "nobody thought to check" is the failure that produced issue 2828.
Correction worth carrying: issue 2828 says f603880da "is already an acknowledged incident". It is
not — it sits in scripts/silent-revert-incidents.txt:26 with expectation fires and is absent from scripts/silent-revert-acknowledged.txt. The gap is not that the commit was muted; it is that firing
was never connected to restoration.
Acceptance criteria
scripts/silent-revert-incidents.txt rows can carry one or more human-reviewed content
markers, each bound to the path it must be found in. A marker may be dispositioned as
deliberately-not-restored only with a non-empty reason, in the same shape declares_removal()
requires of Intentional-removal: — an empty disposition is rejected, not treated as a mute.
scripts/check-silent-revert.sh gains a restoration assertion that resolves each marker
path-scoped against a caller-supplied rev, defaulting to the working tree, and exits non-zero
when a marker is absent and undispositioned.
With the fix's script and corpus, that assertion run against rev 71ca05a39 reports BOTH plugins/disk-hygiene/README.md and plugins/disk-hygiene/skills/clean/evals/evals.json as
unrestored, and exits non-zero.
A fires row carrying no marker is rejected or reported loudly, so the check cannot be
satisfied by hardcoding this one historical incident and silently covering no future one.
The assertion runs in .github/workflows/silent-revert-canary.yml, in the silent-revert-canary
job alongside the ungated "Replay the recorded incidents" step (:97-98, which already checks
out at fetch-depth: 0) — not behind a human remembering to invoke it.
scripts/check-silent-revert.test.sh covers a restored marker, an absent marker, a
dispositioned absent marker, and a marker present only outside its bound path.
--verify-known-incidents still passes unchanged at the shipped thresholds; no existing
expectation is relaxed.
Related
Refs 2828 — named this gap in prose ("The canary has no mechanism for noticing that a recorded
incident's restoration was only partial") but its acceptance criteria only restored the two files.
CLOSED as COMPLETED with no mechanism shipped.
Refs 2833 — open, same file and same function, different axis: it asks whether the DETECTOR still
reproduces the recorded attribution (culprit sha, line count), which scan_commit computes from a
fixed historical pair. Those assertions hold identically at 71ca05a39 and 534eac138, so a full
2833 implementation still exits 0 on a tree where both files are absent — zero overlap in coverage,
full overlap in the files touched. 2833 should land first and this should extend the row format
it establishes, which adds a bracketed [<culprit-sha>=<count>,...] field after the sha. That
field is a useful precondition here: it names the culprit, so a proposed marker can be validated as
a line the culprit actually added rather than an arbitrary string.
Refs 2837 — open; surveys the same issues and confirms none touches restoration.
Refs 2691 — the originating audit; its suggestion 3 is the marker design never built.
Problem
scripts/check-silent-revert.shanswers one question about a recorded incident: does the deletingcommit still produce a finding. It never asks whether the content that commit deleted is on
maintoday. So an incident can be detected, recorded, partially re-landed, and left permanently
incomplete while every signal stays green — the same "the audit trail looked healthy from every angle
a reader normally checks" failure issue 2691 was filed for.
Not hypothetical. It already happened, and a hand audit found it, not the canary: issue 2828 and its
PR #2829 restored
plugins/disk-hygiene/README.mdandplugins/disk-hygiene/skills/clean/evals/evals.json, which PR #2635 added, PR #2639's squashdeleted, and two separate re-lands (PR #2714, PR #2803) each missed. They were absent from
mainfor 31h28m (
f603880da→534eac138, 113298s).Evidence
Verified against
origin/mainat2de57a3798489d2c205fa21d3233b70fec5a42c9, wherescripts/check-silent-revert.shis 543 lines.The mechanism does not exist.
verify_known_incidents(scripts/check-silent-revert.sh:445-492)reads
<expectation> <sha> <note>rows, callsscan_commit, and branches on its exit status alone(
:458-459). Thenoteis echoed verbatim, never parsed.scan_commit(:316-391) works entirelybetween a commit and its own parent —
git blame ... "$parent"(:306),git rev-list --first-parent -n "$WINDOW" "$parent"(:350),git diff --name-only -z --diff-filter=MD "$parent" "$sha"(:365). Nothing in the file resolves thecurrent tree.
scripts/check-silent-revert.test.sh(531 lines) has no restoration case and containsno
git greporgit ls-filesat all; its replay section (:432-476) asserts only "replay passeswhen a recorded incident still fires" (
:446).The canary was green while the content was missing. At
71ca05a39(534eac138^, PR 2830 — thelast commit before the hand fix), the eval case PR #2635 added is provably absent:
Exit 0, on a tree where the incident's content is measurably still gone. (That transcript is the
corpus as it stood at
71ca05a39; PR #2832 has since rewritten thecc58cbc53note. The replaystill exits 0 on current
main.)Two obvious designs are already ruled out. A path-level check is a false negative on this exact
instance:
9239f1541(PR #2641) touched both files ten minutes after the incident, so "was this pathrevisited since" returns true for both. A verbatim-hunk check stays red forever on the README half:
issue 2828 records that PR #2635's README hunk was malformed and must not be restored byte-for-byte,
and PR #2829 restored the intent instead.
Content markers do work, measured on this incident. One line from PR #2635 survived PR #2829's
non-verbatim rewrite intact:
a95f240f7f603880da71ca05a39534eac1382de57a379Safe tidiness is the primary objective(README.md)empty-directories-remain-first-class-tidiness-findings(evals.json)Markers must be resolved path-scoped. Both strings also occur elsewhere in the plugin on current
main— the engine script, its test file, andCHANGELOG.md— so a repo-widegit grepwould havereported both files restored while they were missing.
No sibling gate covers it.
scripts/check-stale-base-overlap.sh:11-19disclaims this class byname ("That class belongs to scripts/check-silent-revert.sh"), and its two overlap steps are
if: github.event_name == 'pull_request'(.github/workflows/ci.yml:704,:709) — a pre-mergecheck on one branch, structurally silent about whether an old incident's content is on
main.Why the current behavior is wrong
The incident corpus is described in the canary's own header as its "honesty proof". It proves
detection fidelity and nothing else: the incident it detected can be two-thirds fixed and the proof
still passes — exactly the property issue 2691 named as the danger, a green signal that has stopped
meaning what a reader takes it to mean.
The header rejects curated marker strings, and that rejection is right for discovery: "Nobody had
registered #2644, #2642, #2635 or #2639 -- registration happens after you already know a fix matters"
(
:62-63). That objection does not apply here. For a row already inscripts/silent-revert-incidents.txt, the knowledge exists and registration costs one line. Issue2691's own suggestion 3 proposed precisely this — "assert that a small set of marker strings from
recently-landed fixes still exist on
main" — and the shipped canary implemented suggestion 2'sshape instead. The marker half was never built.
Cost note: this must not re-scan every acknowledged incident on every run. The corpus is four rows
and a marker check is one path-scoped
git grepper marker, so it fits inside the existingpush: mainjob. Anything heavier belongs in a separate on-demand mode — but an on-demand mode aloneis insufficient, because "nobody thought to check" is the failure that produced issue 2828.
Correction worth carrying: issue 2828 says
f603880da"is already an acknowledged incident". It isnot — it sits in
scripts/silent-revert-incidents.txt:26with expectationfiresand is absent fromscripts/silent-revert-acknowledged.txt. The gap is not that the commit was muted; it is that firingwas never connected to restoration.
Acceptance criteria
scripts/silent-revert-incidents.txtrows can carry one or more human-reviewed contentmarkers, each bound to the path it must be found in. A marker may be dispositioned as
deliberately-not-restored only with a non-empty reason, in the same shape
declares_removal()requires of
Intentional-removal:— an empty disposition is rejected, not treated as a mute.scripts/check-silent-revert.shgains a restoration assertion that resolves each markerpath-scoped against a caller-supplied rev, defaulting to the working tree, and exits non-zero
when a marker is absent and undispositioned.
71ca05a39reports BOTHplugins/disk-hygiene/README.mdandplugins/disk-hygiene/skills/clean/evals/evals.jsonasunrestored, and exits non-zero.
534eac138exits 0 — i.e. it accepts PR fix(disk-hygiene): restore the README and evals #2635 never got back (#2828) #2829'snon-verbatim README restoration rather than demanding the original hunk.
firesrow carrying no marker is rejected or reported loudly, so the check cannot besatisfied by hardcoding this one historical incident and silently covering no future one.
.github/workflows/silent-revert-canary.yml, in thesilent-revert-canaryjob alongside the ungated "Replay the recorded incidents" step (
:97-98, which already checksout at
fetch-depth: 0) — not behind a human remembering to invoke it.scripts/check-silent-revert.test.shcovers a restored marker, an absent marker, adispositioned absent marker, and a marker present only outside its bound path.
--verify-known-incidentsstill passes unchanged at the shipped thresholds; no existingexpectation is relaxed.
Related
incident's restoration was only partial") but its acceptance criteria only restored the two files.
CLOSED as COMPLETED with no mechanism shipped.
reproduces the recorded attribution (culprit sha, line count), which
scan_commitcomputes from afixed historical pair. Those assertions hold identically at
71ca05a39and534eac138, so a full2833 implementation still exits 0 on a tree where both files are absent — zero overlap in coverage,
full overlap in the files touched. 2833 should land first and this should extend the row format
it establishes, which adds a bracketed
[<culprit-sha>=<count>,...]field after the sha. Thatfield is a useful precondition here: it names the culprit, so a proposed marker can be validated as
a line the culprit actually added rather than an arbitrary string.
2de57a379.