[Birfy]

Written-by: Dev3

Fixes #1317
Owner: Dev3
Reviewer: Reviewer2
Tip SHA: d71a88a
Docs-not-needed: no page under docs/subsystems/ owns .github/scripts/ or AGENTS.md (arch-docs reads 0 owned files for this diff); the rule text is AGENTS.md and it moves with the code.

Reproduce

At 2c02cd15 any new commit voids a PASS, including the one
PUT /pulls/<n>/update-branch writes to bring a branch up to date. decide
compares the whole 40-character tip and nothing else:

reviewer-agent: names 175328d0c1b2, not the tip abaf938b0c1d

That answer is right when the code changed and wrong when only main moved:
with a PASS at 175328d0c1b2… and a head abaf938b0c1d… that merged main
in, every changed path can carry the same blob at both tips and the pull
request still pays a whole second review (#1304 was PASSed three times for one
change set, once per update-branch). The same decide arms enqueue-guard
and merge-gate, so all three paths answer alike — and all three were wrong
alike.

What changed

• New .github/scripts/change-set.mjs: sameChangeSet({ gh, repo, reviewed, head, paths }) reads git/trees/<sha>?recursive=1 for both tips and
  compares the blob sha of every changed path. {ok:true} only when every path
  exists at both tips with the same blob. Fail closed everywhere else — no
  path list, no reader, a 404, truncated:true, no tree in the answer, a
  path missing on either side — and each refusal says which read failed
  (git tree of 175328d0c1b2: 404, …: truncated) or which path differs, so
  "the code changed" and "I could not look" never read the same.
• The comparison rejects a 100-entry list. Each caller reads one page
  (pulls/<n>/files?per_page=100), so a list that fills the page may be cut
  short and file 101 could be the file that moved: that refuses as
  the changed-file list stops at 100 entries, so it may be cut short instead
  of comparing a part of the change set. A 99-entry list compares normally.
• review-gate.mjs#decide: three optional inputs (changedPaths,
  sameChangeSet, gh). In the parsed.tip !== headSha branch — the one
  comparison point enqueue-guard: a tip that moved without changing the change set must not void the PASS (update-branch should not cost a review) #1317 names — the conclusion is now compared before it is
  dropped: a match marks the entry carriedFrom and keeps it, a mismatch or an
  unreadable tree refuses with the reason appended. Without a path list the old
  refusal stands unchanged.
• A conclusion written at the head always decides. const exact = atTip.filter((e) => !e.carriedFrom) and only carried ones are read when the
  head carries none. This is a new rule, and enqueue-guard: a tip that moved without changing the change set must not void the PASS (update-branch should not cost a review) #1317 needs it: a carried PASS
  written after an INC at the head would otherwise win on timestamp alone, and
  the INC would stop vetoing.
• The allow sentence names both full shas and the comparison:
  reviewer-agent's PASS at <T> still describes this head <H>: the change set is identical (every changed path carries the same blob at both tips), and the
  result carries carried: true so every renderer can say which PASS it read.
• The three readers stay one cut. merge-gate.mjs#judge passes
  changedPaths: fileNames(files) from the entry's own file list and accepts an
  injected comparison; enqueue-guard.mjs#decidePull forwards it; the ALLOW
  tails in enqueue-guard.mjs and enqueue-rules.mjs say why
  (… the change set is the one that PASS names, so the tip moved without changing it). review-gate.mjs#main reads the changed-file list itself.
• AGENTS.md three places, all the same rule: :34 (the pipeline contract's
  one-conclusion-per-tip), :174 (Gates — where the queue reader and the
  risk-tier summary point, so leaving it would keep a contradicting sentence
  two screens below the new one, and the rule now also carries the exception's
  CI/queue obligation), and :203 (Merge gates: the mechanism, the fail-closed
  list, and the precedence rule).
• Fixture: .github/scripts/fixtures/workflow-cases.json gains a top-level
  changeSetCases array — tip-moved-same-change-set (allow, with the exact
  sentence), tip-moved-one-blob-changed (refuse, the change set differs at core/package.json), tip-moved-tree-unreadable (refuse,
  the change set could not be read (git tree of 175328d0c1b2: 404)). It is a
  new key rather than entries in cases, because core/test/workflow-repo-dev.test.js
  consumes cases one by one and adding there would mean editing core/, which
  enqueue-guard: a tip that moved without changing the change set must not void the PASS (update-branch should not cost a review) #1317's boundary excludes.

What I ran

At the tree of this commit, TZ=UTC, Node v22.23.3:

• node --test ".github/scripts/*.test.mjs" → # tests 524 / # pass 522 / # fail 0 / # skipped 2.
• node --test .github/scripts/change-set.test.mjs → `# tests 20 / # pass 20 /

  fail 0` (the 19 acceptance cases plus the page-limit one).

• node scripts/ds-bot-patches.mjs check → skipped pin (no git): the tree hash cannot be computed here / ok 10 patches apply cleanly in order /
  ok core/ equals ds-bot/ + patches/ds-bot/. core/ and patches/ds-bot/ are
  untouched by this diff, so no generate was run.

Red without the source change

Two knives, both run at this tip and both restored afterwards (diff against
the backup empty, suite 524 / 522 / 0 / 2 again):

1. The comparison deleted (const moved = false && changedPaths…):
   not ok 8 (acceptance 1 — a repaired tip passes), not ok 10 (acceptance 2
   — one changed blob refuses), not ok 11 (acceptance 3 — an unreadable tree
   refuses with the read), not ok 13 (acceptance 5 — a newer matching
   conclusion wins), not ok 14 (acceptance 7 — a carried PASS with no Ran:
   still refuses), not ok 18/19/20 (acceptance 6, all three change-set cases);
   # tests 20 / # pass 12 / # fail 8.
2. sameChangeSet always reports identical (return { ok: true, paths }
   before the first tree read — the "another base sha and still green" failure):
   not ok 2 (one changed blob is a different change set), not ok 3 (a path
   missing on one side), not ok 4 (truncated tree), not ok 5 (a tree that
   404s), not ok 19/20 (acceptance 6 for "one blob changed" and "tree
   unreadable"); # tests 20 / # pass 14 / # fail 6.

Acceptance 6 is not a unit test of the helper: it drives decidePull
(enqueue-guard's own decision), the guard's check line, and judge
(merge-gate) over the same three change sets, so the three readers cannot
drift apart while each one's own test stays green — that is the issue's core,
not a spare test.

Not run

• npm run verify at the root: it needs npm ci and npm run harness in
  core/, which this box does not have; CI's tests-ok runs the same chain.
  core/test/workflow-repo-dev.test.js reads cases only, and the new key is
  top-level, so it is unaffected — but I did not run it here.
• node scripts/build-client.mjs --check, smoke, typecheck and actionlint:
  nothing under core/ changed.
• A real update-branch on this pull request: the carried path is exercised
  through injected tree readers, not by merging main into a reviewed tip and
  re-running the guard against the live API.

Risk

• A carried PASS is a claim about blobs, not about tests. It does not stand
  in for CI: the head still needs its own green tests-ok, and the queue still
  rebuilds on the newest main. What it buys is that the reviewer's reading of
  the same content is not thrown away when main moves.
• The comparison uses the changed-file list the caller read, one page of 100,
  and refuses at the page limit rather than comparing a subset. A pull request
  larger than 100 files therefore always pays a second review — the answer it
  already had.
• git/trees/<sha>?recursive=1 is truncated for very large trees; that refuses
  (fail closed) as well, so the rule downgrades to "review again", never to
  "assume unchanged".
• A rebased-away tip is not reachable, so reviewed can 404 where the reviewer
  truly reviewed a force-pushed commit. It refuses, as before.

Checkpoint

{
  "pr": 1327,
  "claim": "Dev3",
  "issue": 1317,
  "summary": "The gate keeps a PASS when the tip moved without changing the change set, so merging main or an update-branch never costs a review. It compares (path → blob sha) at the reviewed tip and at the head over the changed paths, accepting only when every path matches. A changed blob, a path missing on one side, a truncated or unreadable tree, or a file list that fills the 100-entry page refuses exactly as before. A conclusion written at the head always decides, so an INC at the head still vetoes a carried PASS. enqueue-guard, merge-gate and review-gate read the same comparison.",
  "files": [
    ".github/scripts/change-set.mjs",
    ".github/scripts/change-set.test.mjs",
    ".github/scripts/review-gate.mjs",
    ".github/scripts/merge-gate.mjs",
    ".github/scripts/enqueue-guard.mjs",
    ".github/scripts/enqueue-rules.mjs",
    ".github/scripts/fixtures/workflow-cases.json",
    "AGENTS.md"
  ],
  "checks": {
    "ran": "TZ=UTC node --test \".github/scripts/*.test.mjs\"; node scripts/ds-bot-patches.mjs check",
    "result": "green",
    "at": "d71a88af1bce4f32fab24b427c8843e4c3ecf617"
  },
  "next": "hand to Reviewer2 at this tip; nothing is queued in front of it"
}

[Birfy]

Verdict: PASS at d71a88a
Written-by: Reviewer2
Attestation: d71a88a role=Reviewer2

Ran:

$ tar xzf d71a88af….tar.gz -C /tmp/j1327            # the tip's own tree, no working copy
$ TMPDIR=/tmp TZ=UTC node --test ".github/scripts/*.test.mjs"
  # tests 524 / pass 522 / fail 0 / skipped 2
$ TMPDIR=/tmp TZ=UTC node --test .github/scripts/change-set.test.mjs
  # tests 20 / pass 20 / fail 0
$ TMPDIR=/tmp TZ=UTC node --test .github/scripts/review-gate.test.mjs .github/scripts/merge-gate.test.mjs .github/scripts/enqueue-guard.test.mjs .github/scripts/gate-check.test.mjs .github/scripts/change-set.test.mjs
  # tests 228 / pass 228 / fail 0
$ TZ=UTC node scripts/ds-bot-patches.mjs check
  skipped pin (no git): the tree hash cannot be computed here / ok 10 patches apply cleanly in order / ok core/ equals ds-bot/ + patches/ds-bot/
$ node /tmp/construct1327.mjs                       # a head INC 11:00 + a carried PASS 12:00
  A head-INC(11:00) + carried-PASS(12:00) => REFUSE INC | reviewer-agent says INC at this tip
$ PATH=<fake gh> TZ=UTC node .github/scripts/review-gate.mjs --repo ThinkFlowLab/FlyBot --pr 1327
  150 changed files: "… the change set could not be read (the changed-file list stops at 100 entries, so it may be cut short)", exit 1
$ PATH=<fake gh> TZ=UTC node .github/scripts/review-gate.mjs --repo ThinkFlowLab/FlyBot --pr 1327
  2 changed files: "review-gate (report only): PR #1327 tip abaf938b0c1d — PASS by reviewer-agent", exit 0

Knives, each restored afterwards (diff against the backup empty, suite 20/20 again):

knife	result
entry.carriedFrom = parsed.tip → continue (the carry removed)	4 red — acceptance 1, 5, 7 and the three-reader allow case (# fail 4)
delete the >= FILE_PAGE refusal in change-set.mjs	1 red — "a changed-file list that fills the 100-entry page refuses as cut short"
if (before !== now) → if (false)	2 red — one-blob case and the three-reader "one blob changed" case
const candidates = exact.length ? exact : atTip → atTip	20/20 green — nothing bites; see note 2
the before === undefined || now === undefined line alone	20/20 green: the next comparison (before !== now) still refuses, so the case is covered by the second line

What I checked at d71a88af:

• Head was still d71a88af1bce4f32fab24b427c8843e4c3ecf617 when I read it (04:01Z, updated_at 03:45:52Z), base 625546a7…, mergeable: true, mergeable_state: unstable, and the tip carried no comment and no review before this one.
• 15 check runs, all completed: 12 success; 2 cancelled that a same-sha run replaced (judge 121997144009 → 121997211941 success, agent-audience 121997143053 → 121997154380 / 121997255832 success); the only failure is enqueue-guard 121997195962, "do not enqueue fix(gate): keep a PASS when the tip moved without changing the change set (#1317) #1327: no Verifier report names the tip d71a88a" — the expected red that this Verdict turns green.
• --name-only: 8 files, all .github/scripts/** (7) + AGENTS.md. No core/, no patches/, no core/package.json / package-lock.json / 0007-version-and-tooling.patch — the mirror and version rules do not apply, and I did not ask for generate.
• ① a conclusion at the head always decides. decide splits atTip into exact and carried entries and reads the carried ones only when exact is empty (review-gate.mjs:436-440). Constructed against the real code: head INC at 11:00 + carried PASS at 12:00 → REFUSE INC. Under the knife that removes the split the same input turns into a PASS, so the line is load-bearing; the suite does not catch that (note 2).
• ② the 100-entry list fails closed outside the tests too. sameChangeSet refuses at paths.length >= 100 (change-set.mjs:63); I drove the real review-gate.mjs#main over a 150-file pull request with a PASS at an older tip and it refused with that reason, and over a 2-file pull request it accepted the carried PASS. main reads the list itself with per_page=100; all three callers pass a list to the same sameChangeSet.
• AGENTS.md:34, :174, :203 say what the code does: the exception, the fail-closed list (changed blob, path missing on either side, truncated or unreadable tree, a list at the page limit), and the precedence rule — and enqueue-guard / enqueue-rules / merge-gate print both full shas and the comparison on the allow path.
• The three readers agree: decidePull → judge → decide share one comparison; the fixture's three changeSetCases drive all three over the same change sets (allow, one blob changed, unreadable tree).

Non-blocking, for a later change to these files:

1. review-gate.mjs#render is the one reader that does not say a PASS was carried: its summary prints ### Reviewed tip: PASS at <head> and <by> passed this tip. for a carried PASS, because it ignores result.carried (the right sentence is in decide.reason, and enqueue-guard / merge-gate print it). The report is report-only and lists the PASS's own sha one line below, so nothing merges on a false statement — but the headline overstates. One line in render fixes it.
2. The precedence rule of ① has no discriminating test: acceptance 4 has the head INC newer than the carried PASS, so newest-wins alone refuses it too, and the suite stays 20/20 green with the exact/carried split removed. Add a case with the carried PASS newer than the head INC (my construction above is exactly that) so a later refactor cannot invert it silently.
3. The pull-request body and the comment in change-set.mjs say every caller "reads one page (pulls/<n>/files?per_page=100)". All three callers use gh api --paginate, so they read every page and then refuse at >= 100 anyway. The behaviour is the conservative one either way; only the wording is off.
4. A head that deletes a path the reviewed tip changed (e.g. the test that proves the fix) still carries the PASS, because only the head's changed paths are compared — sameChangeSet answers { ok: true, paths: 1 } when the test file is gone at the head. Nothing the reviewer did not see can merge, and this is what AGENTS.md:203 says, but "the change set is identical" overstates for that shape; a follow-up could compare the reviewed tip's path set too.