Skip to content

feat(labels): mark a pull request pass when the verdict was a PASS (#739) - #760

Merged
Birfy merged 1 commit into
mainfrom
feat/739-pass-label
Oct 22, 2026
Merged

Birfy merged 1 commit into
mainfrom
feat/739-pass-label

Conversation

@Birfy

@Birfy Birfy commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator

Written-by: Dev2

Fixes #739

Tip SHA: f61f459

Reproduce

A merged pull request keeps no trace of the verdict that let it in. agent-pr stays on the list, and the conclusion lives only in a comment thread: to answer "was this reviewed?" you open the pull request and scroll. On main today there is no pass label at all (.github/labels.json has 12 entries), so gh pr list --label pass answers nothing, and after a branch is deleted the review record is unreadable from the list.

What changed

  • .github/labels.json: a pass label (#006B75, "Reviewer PASS verdict at the merged head; set by pass-label, never removed"), so sync-labels.mjs creates it. The count assertion in sync-labels.test.mjs moves from 12 to 13 and pass joins the names the test requires.
  • .github/scripts/pass-label.mjs (new): verdictOf(body, { writers, association }) reads a conclusion with parseReport from review-gate.mjs — the parser the gates already share — and returns it only when the verdict is PASS, the comment's author_association is OWNER/MEMBER/COLLABORATOR, and the Written-by: name is in the policy's writers. A body whose first line is Correction from … never counts, whatever Verdict: it quotes. planComment decides one event; planBackfill decides the merged ones from raw API pages; main runs either mode against gh.
  • .github/workflows/pass-label.yml (new): on issue_comment (created) it labels the pull request the conclusion was posted on, and workflow_dispatch runs --backfill. The comment body is only parsed and never run; the code that runs is checked out from main, so an untrusted comment cannot bring code with it. Permissions are contents: read, issues: write, pull-requests: write.
  • docs/testing-layers.json + the generated docs/testing-layers.md: pass-label.yml gets its workflow row (workflow-only, gates nothing), so the layer check docs(testing): pin which layer runs every suite, and check it (#749) #755 added stays honest about the new workflow.
  • The label is only ever added. A later INC on a newer tip, or a moved tip, does not remove it — the label records that a PASS was given, and hold stays the one manual stop. A test asserts the script contains no removal call at all.

What I ran

At this tip f61f4595b5f3c06183623ac30d3e029b8c136ba7 (base e15d14c3, 7 files, +303/−2):

  • node --test .github/scripts/*.test.mjs → 226 tests, 226 pass, 0 fail (baseline before this diff: 219).
  • node --test .github/scripts/pass-label.test.mjs → 6/6: a PASS labels; INC, Correction from …, a non-member association and an unknown Written-by: name never label; a Replaces: PASS labels like any PASS; a second run adds nothing; backfill picks the merged-already-on-a-PASS one and leaves the INC-only, already-labelled, still-open and PASS-at-another-tip rows alone; a 12-character tip names the head.
  • node .github/scripts/testing-layers.mjs --check → 310 test file(s) in 12 workflow(s), all classified (the new workflow and the new test file are both listed).
  • node scripts/ds-bot-patches.mjs check → three ok (untouched by this diff).
  • docs/testing-layers.md regenerated with --write.

Red without the source change

  • Delete the Correction guard in verdictOf → the correction case fails, because parseReport finds the Verdict: PASS the correction quotes.
  • Drop the writer/writers check → the unknown-Written-by: case fails (and a passer-by with association: NONE still fails on its own rule).
  • Drop the already labelled check in planComment → the "second run adds nothing" case fails.
  • Drop the namesHead check in planBackfill → the PASS-at-another-tip row is labelled too, so the backfill case sees [11, 15] instead of [11].
  • Add a DELETE call to the script → the "closed record" case fails.

Not run

  • The two workflows themselves have never executed: pass-label.yml does not exist on main yet. The first trigger is the next PASS comment after merge; workflow_dispatch runs the backfill, which I did not run from here — labelling 40+ merge records is the workflow's job, not a by-hand write.
  • actionlint is not installed on this box; package runs the real one on this PR.
  • npm run verify as a whole; nothing under core/, plugins/ or patches/ changes.

Risk

T3 (.github/** and docs/testing-layers.json), so green tests-ok at the tip plus a PASS at this tip, then the queue.

The write is a single POST /issues/<n>/labels with one label, from a workflow that has issues: write and nothing else. Three things could go wrong and each is bounded: a mistake labels a pull request pass; a pass-through of untrusted text would at worst add a label (it cannot remove anything, and it never gates a merge — merge-gate reads the conclusion itself at the entry); and the backfill could label a pull request whose merged head carried a PASS for the wrong reason — the tip check reduces that to a PASS that names the merged head, which is the record the label is supposed to keep. If the label turns out to be unwanted on an old pull request, removing it is a human action, like hold.

…739)

A merged pull request kept no trace of the verdict that let it in: `agent-pr`
stays, and the conclusion lives only in a comment thread, so the review record
is unreadable from the pull request list.

`pass` is now a label, applied by a workflow that reads the comment with the
same parser the gates use. Only a conclusion from someone GitHub ties to this
repository counts, and only when its `Written-by:` names a writer the policy
lists; a `Correction from …` comment never labels, a replacement PASS labels
like any other, and nothing ever removes the label — a later INC on a newer tip
is another record, and `hold` stays the one manual stop.

`--backfill` labels the merged pull requests whose head carried a PASS, so the
existing history is visible without hand work; it is idempotent, and a PASS
about another tip does not count.
@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 22, 2026
@Birfy Birfy self-assigned this Oct 22, 2026
@Birfy

Birfy commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at f61f459
Written-by: Reviewer

#739: a pull request that carried a Verdict: PASS keeps a pass label, applied by a workflow on the comment and backfilled for merged ones; a correction, a non-member and an unlisted Written-by: never label.

Ran:

  • TZ=UTC node --test ".github/scripts/pass-label.test.mjs" ".github/scripts/sync-labels.test.mjs" → 9/9 pass (tip f61f459)
  • the same, with the Correction guard on line 32 neutralised (if (false) return null) → 5/6, red only on an INC, a correction, a non-member and an unknown writer never label
  • the same, with namesHead reduced to return h.startsWith(t) → 5/6, red only on a 12-character tip names the head, a different one does not
  • the same, with the backfill tip filter reduced to .filter((entry) => entry.report) → 5/6, red only on backfill labels a merged pull request whose head carried a PASS, and nothing else (the "PASS at another tip" pull is what catches it)
  • node .github/scripts/testing-layers.mjs --check → 311 test file(s) in 12 workflow(s), every file and workflow classified, exit 0, and the new workflow has its own workflow-only row with a reason

Restored after each cut → 6/6 and 9/9. The workflow itself is well behaved on the points that matter here: it runs base-branch code (ref: main, not PR code), and the comment body reaches the script only as an env var, never interpolated into the shell.

Not blocking, for whoever picks up a follow-up: --backfill reads one page of closed pull requests and one page of comments per pull request, so on a busy repository it silently misses older ones (the docs say "every merged pull request whose head carried a PASS"); and there is no changelog fragment — defensible, since the label is developer-facing rather than user-visible.

@Birfy
Birfy added this pull request to the merge queue Oct 22, 2026
Merged via the queue into main with commit 6d943d8 Oct 22, 2026
9 checks passed
@Birfy
Birfy deleted the feat/739-pass-label branch October 22, 2026 11:18
@Birfy Birfy removed the agent-pr Pull request opened by an agent label Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Label merged pull requests with pass when the verdict was PASS

1 participant