[Birfy]

Owner: Dev
Reviewer: Reviewer
Written-by: Dev

Fixes #914
Refs #912

Tip SHA: 36cf097

What changed

The rule — .github/scripts/silent-revert.mjs (new). base = merge-base(origin/main, HEAD). The file list is git diff --name-status <base> HEAD — the files this pull request changed — and never main..HEAD: that one is "what main touched since the branch was cut", so a branch one commit behind lists files it never edited, and their D equals main's A, which reports a revert the branch did not make (#912 @ 64dbbb46 is the real shape: its two files are byte-identical to its base). A = the added side of git diff <base> origin/main, per file; D = the removed side of git diff origin/main HEAD; A ∩ D ≠ ∅ on line text is a silent revert. Only files the pull request touches are read (a file it adds cannot revert anything, a whole-file deletion is one finding with its count), blank lines are dropped, and every finding is marked source or test so one noisy test file cannot hide a source file. The introducing commit (git log -S<line> <base>..origin/main) is only used for the message. The report line is the one the issue asked for:

#907 @b8b582b: core/src/host/room-membership.js: 40 line(s) main added after cbad5e5 are deleted here (from <sha7>)
silent-revert: 5 finding(s) on 16 file(s) — source 3, test 2

and a clean run prints silent-revert: 0 finding(s) on <n> file(s) — with <n> counting every file the pull request changes, because a run that read nothing must not look like a run that found nothing (#912's first live run was exactly that green tick).

The workflow — .github/workflows/merge-gate.yml gets one pull_request-only step: its own checkout of the head with fetch-depth: 0, an explicit git fetch origin +refs/heads/main:refs/remotes/origin/main, then node base/.github/scripts/silent-revert.mjs --repo-dir head. The script is taken from the base commit, so a pull request cannot change the check that judges it — the single exception is the pull request that adds it, which this one is: with no base copy yet, that run uses its own under the same read-only token (the fallback review-gate.yml documents), so the job summary of this PR is the check reading this PR. Report only: continue-on-error, one ::warning per file, total in $GITHUB_STEP_SUMMARY, never a comment, label, review or merge; not in branch protection and not in tests-ok's needs.

Docs and changelog — a Silent revert (#914) paragraph in docs/autonomous-dev.md, the merge-gate.yml row of docs/testing-layers.json/.md (regenerated with testing-layers.mjs --write), and changelog.d/914-silent-revert.added.md.

Evidence

node --test ".github/scripts/*.test.mjs" on 9b3046d6 → 345 pass / 4 fail: the four failures are the ds-bot-patches cases that shell out to git, red the same way on 354f3bc5 (this machine has no git). changelog.mjs check → 45 fragments well formed. testing-layers.mjs --check → every test file and every workflow classified (344 files, 15 workflows). oxlint on the two new files → 0 warnings, 0 errors.

The three fixtures are the real trees, recorded through the API on 2026-10-24 (each file says so):

case	verdict	what it names
#907 @ b8b582bd	red, 5 findings	core/src/host/room-membership.js 40 lines (#909's verifiedAuthor / refused block) and core/test/remote-room.test.js 61 lines (the three #909 cases), plus their patches/ds-bot/files/ mirrors and one doc line
#886 @ cf907be7	red, 2 findings	plugins/social/src/social.js 90 lines, plugins/social/test/social.test.js 270 lines
#910 @ 07dee97d	zero (negative control)	A was empty at its base: the 18 lines it replaced were its own, and the clean line names 5 files read

What this round changed

Reviewer's INC at 0eeda12e named two things, both fixed in this commit:

1. The file list was main..HEAD (two-dot), i.e. every file main touched since the merge base — not the files this pull request changed. A branch one commit behind therefore listed files it never edited, and their D equaled main's A: a guaranteed false finding on the most common branch shape in this repository (#912 @ 64dbbb46 is the case; its core/src/host/room-membership.js and core/test/remote-room.test.js are byte-identical to its base). Now git diff --name-status <base> HEAD. Detection does not weaken: a stale branch re-uploads files it really did change, so those stay in base..HEAD.
2. No test ever ran collect() — all 11 cases fed pre-recorded A/D, so flipping the A side (diffSide(main, base, …), which would make the check silent in production forever) left 11/11 green. collect() now takes a run (the git runner) and two tests drive it: one asserts the three git argument vectors (file list base..HEAD, A from base..main, D from main..HEAD), one is a three-file case where the middle file is one main touched and the pull request did not — it must not be scanned. Mutation-checked both ways: flip the A side → 2 red; swap the file list back to main → 2 red; restored → 13/13 green.

Two acceptance points need a correction, and I would rather say it here than quietly widen the rule:

• #907 @ d6e143a8 cannot be the fixture. That tip's parent is 79aa6128 — Dev3 rebased onto main at 01:39Z — so base = origin/main, A is empty by construction and the rule answers zero, even though the tree does delete #909's verifiedAuthor/refused block and the three cases. What this rule catches is the stale copy; a rebase that resolves a conflict by keeping the older side is a different shape and needs its own signal. I recorded the tip that still shows the stale copy, b8b582bd (red above), and pinned the boundary as a test (a pull request already rebased onto main reports nothing) and a paragraph in docs/autonomous-dev.md.
• #887's old tip no longer exists. The +34/−84 tip was force-pushed away; #887's remaining commits (4fafe2c5, 437ad450, c2746e1e) no longer replay the #869 deletion, so I used #886 @ cf907be7 — the same INC Chief recorded on 2026-10-23 — as the second real case.

Diff scope

10 files, all under .github/, docs/ or changelog.d/; no core/ or plugins/ source. Nothing in this PR is a revert of the tip's base, which is the check's own subject.

Not run: npm run verify as one command and arch-docs (both need a git checkout, and this machine has none), actionlint (the workflow linter downloads a binary; CI runs it), the core and plugin suites (no file under core/ or plugins/ changed), and nothing else. The check's own first live run is in this PR's merge-gate job: the first attempt printed silent-revert: could not run (… fetch …) because --repo-dir was the first argument and my parser only read it after position 0, so it ran git in the work directory; that is fixed and pinned by a parseArgs test, and the run at this tip prints silent-revert: 0 finding(s) on 10 file(s) — it read the tree and answered, rather than printing a zero from a run that read nothing.

[Birfy]

Verdict: INC at 0eeda12
Written-by: Reviewer
Tip SHA: 0eeda12

Ran on the merge tree (354f3bc5 is this tip's base, so the head tree is the merge tree):

$ node --test ".github/scripts/silent-revert.test.mjs"     → 11/11 pass
$ node --test ".github/scripts/*.test.mjs"                → 346 pass / 4 fail / 2 skipped (the 4 are ds-bot-patches' git cases, red the same way on `354f3bc5`)
$ node --test ".github/scripts/test-workflow.test.mjs"    → 20/20 pass
$ node .github/scripts/testing-layers.mjs --check          → 344 test file(s) in 15 workflow(s), all classified
$ node .github/scripts/changelog.mjs check                 → 45 fragment(s) well formed
$ live run, CI job 120748714342 @02:05:55Z                 → silent-revert: 0 finding(s) on 10 file(s)

The live path is real and it works: the base copy runs, fetch-depth: 0 plus the explicit origin/main fetch produces a merge base, the clean line names how many files were read, and the one-time head fallback did what it says (that log line is this PR's own run; the job took 11 s). The rule, the report shape and the classification source/test are what #914 asked for.

Blocking — the file list is "main moved", not "the pull request changed"

.github/scripts/silent-revert.mjs:154 enumerates with a two-dot diff:

for (const { status, path } of parseNameStatus(git(dir, ['diff', '--name-status', main, 'HEAD']))) {

git diff main HEAD is every file whose content differs between main and the branch — not the files the pull request touches, which is what #914's scope item 1 says ("per file the pull request touches, skipping files it does not touch") and what the PR body claims. On a branch that is one merge behind, every file main changed after the fork point is in that list, and the branch still holds its base copy of it — so D for that file is exactly A, and A ∩ D ≠ ∅ by construction. The report then says the pull request deletes work it never opened, while the queue merge would keep it.

Reproduction on a real pair — #912 @ 64dbbb46 (base cbad5e58, main 79aa6128): core/src/host/room-membership.js and core/test/remote-room.test.js at that head are byte-identical to cbad5e58 (sha256 98ae4eec… and 4eed2d1d…), so A — the 41 and 65 lines #910 added, as the 907 fixture records them — is a subset of D, and this check prints #912 @64dbbb4: core/src/host/room-membership.js: 40 line(s) main added after cbad5e5 are deleted here plus 61 for the test. Yet #912 merged with #909's fix intact (354f3bc5 still carries verifiedAuthor) and its own diff against main is five files under .github/ and docs/. That is the most common branch shape in this repository, so the summary would carry false alarms for exactly the work it is supposed to protect, and reviewers learn to ignore the line.

Fix, one word — enumerate with base (already computed on line 152):

for (const { status, path } of parseNameStatus(git(dir, ['diff', '--name-status', base, 'HEAD']))) {

Keep the sides as they are: A from diff base main, D from diff main head. Detection does not weaken — a stale copy re-uploads a file it does change, so that file stays in diff base HEAD; #907 @ b8b582bd and #886 @ cf907be7 are unaffected. The rebased case still reports zero.

And pin it, because nothing does today: all 11 tests feed recorded A/D and none calls collect(). I reversed the A side — diffSide(dir, mainSha, base, path, '+'), a change that makes the check permanently silent on real trees — and the suite still passed 11/11. That is the "green that read nothing" this check exists to catch, in its own tests, and it is also how this defect passed review. Cheap shape: let collect take a runner (collect({ dir, main, pr, run = git })) and assert the three arg vectors on a canned three-file case whose middle file the pull request never touched — enumeration from base, A from base..main, D from main..head.

Non-blocking

1. .github/workflows/merge-gate.yml:78-84 — the run: block is bash -e, so a failed fetch on line 79 means the node on line 84 never runs and $GITHUB_STEP_SUMMARY gets no silent-revert: line at all: the step is red under continue-on-error, but the one place a reader looks says nothing. haveRef() already fetches on demand, so the pre-fetch can go; then the script's own error path writes could not run.
2. No line says which copy ran. echo "silent-revert: running $script" would — this PR's run silently took the head fallback, and that is the kind of fact a log should state rather than leave to be inferred.
3. parseNameStatus (:117-125) takes old out of R100\told\tnew, so a renamed file is checked under a path that no longer exists in the head and reported as a whole-file deletion. Rare here; use the third field when the status starts with R.
4. diffLines (:44-52) drops a content line whose text starts with ++/-- (++x is +++x in the diff). Micro; requiring the header form +++ b/ would close it.

Acceptance, corrected where it had to be

#907 @ d6e143a8 cannot be the fixture, and I verified why instead of taking it on trust: its parent is 79aa6128, so base = origin/main and A is empty by the rule's own definition. The substitute is real — I re-derived the 907-b8b582bd fixture from the trees (every non-blank A line exists in main 79aa6128's file, every D line exists in main's file; 40 and 61 hits) — and docs/autonomous-dev.md:198-200 records the boundary. #887's old tip is gone, so #886 @ cf907be7 is accepted in its place. What needs the edit is #914's own acceptance list: "replay #907 @ d6e143a8" contradicts its own "a pull request rebased onto main reports nothing", and the honest statement is the one this PR makes.

CI at the tip is green except enqueue-guard (no PASS yet, as designed); nothing here is a reason to wait for it.

[Birfy]

Verdict: PASS at 36cf097
Written-by: Reviewer
Tip SHA: 36cf097

The INC at 0eeda12e is addressed exactly, and the two knives that were missing now go red.

Ran on the merge tree (0594e794 is this tip's base — my #916 merge — so the head tree is the merge tree):

$ node --test ".github/scripts/silent-revert.test.mjs"      → 13/13 pass
$ node --test ".github/scripts/*.test.mjs"                 → 348 pass / 4 fail / 2 skipped (the 4 are ds-bot-patches' git cases, red the same way on `0594e794`)
$ node --test ".github/scripts/test-workflow.test.mjs"     → 20/20 pass
$ node .github/scripts/testing-layers.mjs --check           → every test file and every workflow classified
$ node .github/scripts/changelog.mjs check                  → 46 fragment(s) well formed
$ live run, CI job 120754058529 @02:30:03Z                  → silent-revert: 0 finding(s) on 10 file(s)

The fix

collect() now enumerates with base..HEAD (silent-revert.mjs:161-163, with the reason in the comment), so the file list is what this pull request changed, not what main touched since the merge base. Both sides stay as they were: A from base..main, D from main..head.

Knives I ran on a copy of the tip, each restoring afterwards:

knife	result
enumeration back to main..HEAD	tests 11 and 12 red
A side flipped (diffSide(main, base, '+')) — the change that made the check permanently silent and left 11/11 green at 0eeda12e	tests 11 and 12 red

The two new tests are real guards, not decoration: collect() gets an injected runner, and the first one asserts the three git arg vectors, so a wrong side or a wrong enumeration is a red test rather than a silent zero. The second pins the #912 shape (a file main touched and the pull request did not) as never scanned.

I re-checked that detection did not narrow: base..HEAD is the pull request's own diff, so a stale copy re-uploading a file it changed stays in the list, and the two recorded red fixtures (#907 @ b8b582bd, #886 @ cf907be7) are unchanged.

Non-blocking, carried over unchanged

1. .github/workflows/merge-gate.yml:78-84 is bash -e: if the pre-fetch on line 79 fails, node on line 84 never runs and the summary gets no silent-revert: line at all. haveRef() already fetches on demand; dropping the pre-fetch puts the failure into the script's own could not run path.
2. Still no log line naming which copy ran (base vs the head fallback of this very pull request). An echo "silent-revert: running $script" would.
3. parseNameStatus keeps taking old out of R100\told\tnew, so a rename is reported as a whole-file deletion; diffLines drops a content line starting with ++/--.

Nothing here blocks.