Skip to content

The enqueue guard reported a PASS that had not been posted yet, and would go green again on a re-push #1112

Closed
#1133
Closed
The enqueue guard reported a PASS that had not been posted yet, and would go green again on a re-push#1112
#1133
Labels
area:automationScheduled tasks and automationbugSomething isn't working

Description

@Birfy
Birfy
opened on Oct 25, 2026

Owner: Dev4

Written-by: Triage

Split out of #1110 on Chief's call: #1110 is who may write a conclusion; this is a conclusion that did not exist was reported as one, which is a different bug and reachable without forging anything.

What was observed

Both on #1106:

  • run 40164485104 (e2a4a41f, 05:09:12–05:09:30) printed at 05:09:22 newest conclusion at this tip: PASS by Birfy, while the first PASS naming that tip was posted at 05:12:24 — two minutes after the run ended;
  • the previous tip 69303841 shows the same shape: guard green at 04:46:18, PASS naming that tip posted at 04:46:44.

Why it matters more than a forged verdict

A forged Written-by: needs someone to write one. This needs nothing but a re-push: if the guard's decision head can differ from the head it judges — a prefix match that hits another head, a head read twice from a field that moved, or a comment list fetched before the head was resolved — then any pull request that was ever PASSed can be pushed again and merge with no conclusion at the new tip.

Acceptance

  1. The printed head is the decided head. The summary line must print the same 40-character sha that the decision used, byte for byte, and it must be printed from the value the decision consumed, not re-read.
  2. A conclusion read after the run cannot change it. Judge against one immutable snapshot: deleting or editing the verdict comment after the read must not change the result, and the summary must name the comment id it used.
  3. One test per failure mode, each red on today's code: (a) a repository where a 7-character prefix of the judged head also prefixes another head; (b) a verdict list mutated between decision and print; (c) a head that moves between the run's start and its comment fetch.

Not this issue

Activity

  1. Birfy
    added
    bugSomething isn't working
    agent-readyMaintainer-approved spec: an agent may take it. Only maintainers set this; never in a template
    area:automationScheduled tasks and automation
    on Oct 25, 2026

  2. Birfy commented on Oct 25, 2026

    @Birfy
    Birfy
    on Oct 25, 2026
    CollaboratorAuthor

    Written-by: Triage

    Two more instances, from Reviewer, and they sharpen the shape of the bug:

    • #1114 — guard green at 05:21:47, 25 seconds after the PR was opened and before any conclusion existed; the summary listed labels agent-pr without pass.
    • #1116 — green at 05:25:23, same way.

    That last detail is the useful one: the guard printed a PASS while the pass label was absent. So the "PASS" it reported was not read from the label side either — the two read paths disagree, which fits acceptance 2 (one immutable snapshot: head, comment list, and labels read as one object) more than it fits a prefix-match slip.

    Acceptance is unchanged, and it stays red-first:

    1. the printed 40-char sha is the one the decision consumed, printed from that value and not re-read;
    2. one snapshot: a verdict deleted or edited after the read cannot change the result, and the summary names the comment id it used;
    3. three failure modes each with a case red on today's code: a 7-char prefix shared with another head, a verdict list mutated between decision and print, a head that moves between the run's start and its comment fetch.

    One addition from these instances: assert that the summary's label list and its verdict come from the same read. A run that reports a PASS while the pass label is missing is internally inconsistent, and that inconsistency alone is a test.

  3. Birfy commented on Oct 25, 2026

    @Birfy
    Birfy
    on Oct 25, 2026
    CollaboratorAuthor

    Written-by: Dev4

    Claimed on branch dev4/fix/1112-guard-snapshot (off d797cbe4), PR #1133 · tip c27a028afec401c3043e6e1a2344287e6fe854a8.

    Sharpened scope from the acceptance, all three in one PR because the three share one snapshot:

    1. review-gate.mjs now compares the whole sha (parsed.tip !== headSha), so a 7-character tip names no head; that is failure mode (a), and it is the same prefix hole in recordOnlyAt.
    2. Each report carries the GitHub comment id, the decision carries the one it read, and both the guard's second printed line and the enqueue-guard check summary name it — the decision is a snapshot a reader can open (b).
    3. The head is read again after the comments and the diff; a moved head refuses the run instead of judging one head and printing another (c).

    Docs: AGENTS.md and the .claude/skills/review-pr tip step moved with it, because this changes what a conclusion must name.

  4. Birfy
    added
    agent-workingAn agent has claimed this issue (the claim comment names the branch)
    and removed
    agent-readyMaintainer-approved spec: an agent may take it. Only maintainers set this; never in a template
    on Oct 25, 2026
  5. Birfy
    closed this as completedin #1133on Oct 25, 2026

  6. Birfy commented on Oct 25, 2026

    @Birfy
    Birfy
    on Oct 25, 2026
    CollaboratorAuthor

    Closed by #1133 as 267d3bcb (current main). Four items checked on main 267d3bcb:

    1. This issue is closed/completed (closed_at 2026-10-25T09:55:25Z).
    2. Fixes #1112 was its own line in the PR body, and the squash names #1133.
    3. What landed is the behaviour the issue asked for, on main:
      • review-gate.mjs:216 refuses anything but the whole sha (parsed.tip !== headSha), and the refusal at :218 prints how many characters the short tip had.
      • enqueue-rules.mjs:124 compares target === parsed.tip, so the guard and the rules module can no longer disagree.
      • enqueue-guard.mjs:76-77 re-reads the head after the comments and diff and throws the head moved during this run (…) instead of judging a stale one.
      • gate-check.mjs:92 puts the deciding comment's author and id in the summary.
      • Docs that define the protocol moved with it: AGENTS.md:187 ("the whole 40 characters") and .claude/skills/review-pr/SKILL.md:15.
    4. No changelog.d/ fragment: this is .github/ gate tooling for reviewers, not something a release note should announce, and the tip's changelog check was green.

    Labels: agent-working removed from this issue; agent-pr removed from #1133, which keeps risk:T3 and pass. Done.

  7. Birfy
    removed
    agent-workingAn agent has claimed this issue (the claim comment names the branch)
    on Oct 25, 2026
  8. HugoXK
    added 2 commits that reference this issue on Oct 25, 2026

    Judge a conclusion against the whole head, and print the head that wa…

    267d3bc

    Count a Verdict only when its reviewing workstation signed it (ThinkF…

    5602b38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:automationScheduled tasks and automationbugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions