[erikdarlingdata]

Claude posting for Erik Darling.

Part of #6656.

Problem

A push to a pull request starts pr.yml. Marking the PR ready (or pushing again) starts a second run, and cancel-in-progress cancels the first. The cancelled run's verdict job still runs (job-level if: always() && ...) and its "Set CI result" step posted CI result = failure ("CI failed: plan (cancelled)") on the head. Seen on #6637, run 40033237161. A second, related defect: a push to a draft and gh pr ready seconds apart start the push run last; it cancels the ready run and skips as a draft from its stale event payload, so the final head got no CI result at all.

Why round 1 failed

Round 1 put if: ${{ !cancelled() }} on the step. In a step's if, cancelled() reports the job's own state, not the workflow's, so it was true in run 40033984860 and the step still posted failure. A needs.*.result == 'cancelled' test is wrong too: a test leg that hits timeout-minutes is also reported as cancelled and that must still post failure.

What this PR changes (.github/workflows/pr.yml only)

• API lookup (verdict job, "Set CI result"): on pull_request events, before it posts a failure, it asks the API whether a newer run of pr.yml exists for this pull request. If one does, it prints a ::notice::, writes empty state= and failed= outputs and exits 0 without posting; the newer run sets the status. If the call fails or returns a non-count, it posts as before. Success is posted as before. A test leg that hits timeout-minutes has no newer run, so its failure still posts.
• Scoped to this pull request: gh api -X GET .../actions/workflows/pr.yml/runs -f event=pull_request -f branch="$HEAD_REF" -f per_page=20 (the -f form URL-encodes a branch name with #, & or ?; mirrors claude-review.yml). A run counts when its id is above GITHUB_RUN_ID, its head_repository.full_name equals this PR's head repo (HEAD_REPO env var; a fork's runs list no pull_requests, so this is what separates two forks that both use patch-1), and its pull_requests is empty or names this PR's number (PR_NUMBER). I ran the filter against this repo's real run list: 8 matches for this repo and PR, 0 for a different head repo.
• Live draft read (plan job): before calling ci-plan.py, on pull_request events, reads gh api repos/$R/pulls/<number> --jq .draft and uses it in place of the payload's IS_DRAFT. If it differs from the payload, a notice says so; if the call fails, the payload value is used and a ::notice:: says so. pull-requests: read was already granted.
• Adds actions: read to permissions: (the lookup needs it; round 3 of Part of #6656: smaller build artifact, Lite reads classes on Darling-only runs, legs fit the free runners (CI speed) #6668 had removed it with the leg cap).
• The comment at the top of the file now says the draft state is read live, and why.

The dev is red step is unchanged: it runs only on push events.

Reproduction runs (UTC, 2026-10-23/24)

Earlier lane, fix at ec46ece (no head-repo scoping, no live draft read):

Run	Head	Result
40035554053	ec46ece	cancelled; verdict printed the "newer run" notice
40035586184	ec46ece	cancelled, no jobs
40035588687	1fdbd56	success, skip-draft from the stale payload: no CI result on the head (the race this round fixes)
40036369361	192cdd8	cancelled; verdict printed the notice
40036385426	192cdd8	newer run; cancelled when this round pushed

This round (fix at d47de35):

Run	Head	Result
40040414151	d47de35	cancelled by the next push (draft PR)
40040455139	dc648f5	success, PR was a draft, so no status set (expected)
40041348424	dc648f5	PR made ready; cancelled by the next push. Its verdict log: "A newer run of this workflow exists on this pull request, so it sets CI result and this run (plan (cancelled)) posts no failure." No failure on the head.
40041541464	253a5cd	newer run; plan and build success, 2 legs success, 16 legs cancelled by the race push below, its verdict job still queued when I stopped. CI result on 253a5cd = pending, no failure.
40044093305	253a5cd	the ready_for_review run of the race cycle, cancelled
40044095342	5709f55	the push run of the race cycle (final head), status pending waiting for the concurrency group, no jobs yet when I stopped

Race cycle: PR set back to draft, empty commit 5709f55 pushed, gh pr ready run right after (about 2 seconds later).

NOT PROVEN (runner queue)

The GitHub-hosted runners were queued for over 30 minutes on both cycles, and the old run's queued verdict job still holds the PR's concurrency group, so run 40044095342 had not started when my 60 minutes ran out. Still to confirm on the final head 5709f55: that run's plan prints the "Draft state read live" notice, sets CI result pending, and the result ends in success. Check gh api repos/erikdarlingdata/PerformanceMonitor/commits/5709f5542/statuses. Open run IDs: 40041541464 (queued verdict), 40044095342 (pending). Update at 00:37Z: the pending run did start. Its plan job completed success and CI result on the final head 5709f55 is pending (set by that plan), so the push-then-ready race no longer skips the head as a draft. I could not tell from the log whether the live-read notice printed, because the run was still in progress. I then ran gh pr ready --undo: the PR is a draft again. Build and the test legs were still queued; the final success is not yet seen.

No actionlint was installed locally; I checked the jq filter against the live API and the YAML edits by reading them.

Bot review round 2 (changes requested)

• Body described if: !cancelled() as the change and "Not a fix yet": rewritten above to describe the API lookup at HEAD; title updated.
• Two commits after ec46ece (1fdbd56, 192cdd8) had no run IDs or results: listed above.
• Lookup keyed on the branch name alone (pr.yml ~379): now scoped to this PR by head repository and by pull_requests (empty or this number).
• Branch name interpolated unescaped into the query string: now passed with -f branch=....
• Live check on a cancelled run that CI result ends correct: runs 40041348424 and 40036369361 show the notice and no failure; the final success is NOT PROVEN yet (above).

Considered, not added

• A newer run that is itself skipped as a draft (the PR turned draft after the push) would leave the older cancelled run quiet and the older SHA without a status. That SHA is no longer the head, so no merge depends on it.
• Retrying the live draft read: a failed call falls back to the payload value with a notice; the trigger for a failure is a GitHub API outage, which also breaks the files call just below it.

Test plan

• Cancelled run's verdict step prints the notice and posts no failure (40041348424, 40036369361)
• jq filter returns 8 for this repo/PR and 0 for another head repo on this repo's real runs
• Final head 5709f55 got a CI result (pending) from the push run after the ready race
• Final head 5709f55 CI result ends success (run 40044095342 build and legs still queued)
• actionlint (not installed)
• No local build or test: one workflow file

Bot review round 3 (changes requested)

• Body claim corrected: "Considered, not added" no longer lists head-repository filtering as left out. The filter is in the newer-run lookup (.github/workflows/pr.yml 400-404), described under "What this PR changes": it counts only runs from this pull request's head repository whose pull_requests list is empty or names this pull request.
• Body claim corrected: the push and gh pr ready race is no longer described as untouched. The plan job's live draft read (pr.yml 68-80), described under "What this PR changes", re-reads the pull request's draft state from the API instead of the event payload, so a run that starts after gh pr ready sees the pull request as ready. It is meant to close that race; the final-head result is still NOT PROVEN (see above).
• CRLF risk on the multi-line --jq filter (pr.yml 400-404) checked: the coordinator ran the exact filter with CRLF and with LF line ends against the real runs of Darling README: plain uninstall-darling.ps1 also refuses on an MSI install #6691 (cancelled run 40049568380, newer run 40049596679), and both returned 1, so jq treats the CR bytes as whitespace.

[claude]

Verdict: LGTM

Claims

• VERIFIED — the verdict job's job-level if is always() && (...), so it still runs when upstream jobs are cancelled/failed — matches .github/workflows/pr.yml:335 (pre-existing, unchanged by this diff)
• VERIFIED — "the step posts nothing" when the run is cancelled — matches the new if: ${{ !cancelled() }} at .github/workflows/pr.yml:342
• VERIFIED — "a test leg that times out does not cancel the workflow, so the step still posts failure" — a timed-out Run test leg step (.github/workflows/pr.yml:302-309, timeout-minutes: 25) makes the test job's own conclusion failure, not cancelled; that is a different job's result flowing into TEST_RESULT, not a cancellation signal on the verdict job itself, so cancelled() stays false and the step still runs
• VERIFIED — "the dev is red step after it already requires steps.verdict.outputs.state != '', so it skips when the verdict step is skipped" — matches .github/workflows/pr.yml:392
• VERIFIED — "the verdict step is the job's first step" — confirmed structurally, it's the first entry under the job's steps: (.github/workflows/pr.yml:338-339)
• UNVERIFIED — "a push to a draft PR... Marking the PR ready about seven seconds later starts a second run on the same head" — a live timing claim about two specific Actions runs; not checkable from a read-only tree, would need the Actions run history/timestamps for #6637
• UNVERIFIED — "Seen 2026-10-23 on #6637, run 40033237161" — a specific historical run id; would need to query that run via the Actions API/UI

Riskiest lines

1. .github/workflows/pr.yml:342 — the new guard itself; if it over-fires it would silently swallow a genuine failure, if it under-fires the original bug (a cancelled run posting a bogus failure) persists — UNCOVERED (no behaviour test can exercise GitHub Actions' own cancellation semantics from this repo; the PR body gives the one-sentence reason: "proven by CI runs on this PR," and the pattern is already established elsewhere in this repo — build.yml and wpf-smoke.yml both use if: ${{ !cancelled() }} the same way)
2. .github/workflows/pr.yml:386 — the gh api .../statuses call the new guard gates; a wrong guard surfaces here as a wrongly-posted or wrongly-withheld commit status — UNCOVERED, same reasoning as above
3. .github/workflows/pr.yml:392 — unchanged downstream step whose steps.verdict.outputs.state != '' check is the safety net the PR body leans on if the new guard skips the step; re-read and confirmed it still holds (the output simply stays unset when the step is skipped) — not itself at risk from this diff, but it's the line that makes the change safe

Traps near this diff

• "dorny/paths-filter v4 evaluates each pattern as an INDEPENDENT predicate..." — does not apply: no paths-filter usage anywhere in this diff; the touched step has no file-pattern filtering.
• "Under set -euo pipefail, a bare $(gh api …) assignment that fails kills the step with one context-free stderr line" — does not apply: the gated script still uses set -uo pipefail (no -e, pre-existing and unchanged at .github/workflows/pr.yml:356), and its gh api call (:386) is piped to /dev/null, not captured into a variable via $(...).
• "gh api --paginate --jq runs the filter once PER PAGE" — does not apply: the changed step's gh api call (:386) uses neither --paginate nor --jq.

Divergence (blind read vs PR body)

None — the body's account matches the diff on every claim graded above. The body is also appropriately conservative: its test-plan checkboxes are left unchecked ("Run IDs and statuses to be filled in") rather than claiming verification that a read-only tree (or the author) hadn't yet produced.

Reviewer checklist

• Confirm on the live PR that the second ("empty... to reproduce") commit actually produced two pr.yml runs on one head, that the first run's verdict job skipped "Set CI result," and that the head's CI result status history shows no spurious failure from the cancelled run.

[claude]

Verdict: LGTM

Claims

• VERIFIED — the verdict job's job-level if is always() && (...), so it still runs when upstream jobs are cancelled/failed — matches .github/workflows/pr.yml:335 (pre-existing, unchanged by this diff)
• VERIFIED — "the step posts nothing" when the run is cancelled — matches the new if: ${{ !cancelled() }} at .github/workflows/pr.yml:342
• VERIFIED — "a test leg that times out does not cancel the workflow, so the step still posts failure" — a timed-out Run test leg step (.github/workflows/pr.yml:302-309, timeout-minutes: 25) makes the test job's own conclusion failure, not cancelled; that's a different job's result flowing into TEST_RESULT, not a cancellation signal on the verdict job itself, so cancelled() stays false and the step still runs
• VERIFIED — "the dev is red step after it already requires steps.verdict.outputs.state != '', so it skips when the verdict step is skipped" — matches .github/workflows/pr.yml:392
• VERIFIED — "the verdict step is the job's first step" — confirmed structurally, it's the first entry under the job's steps: (.github/workflows/pr.yml:338-339)
• UNVERIFIED — "a push to a draft PR... Marking the PR ready about seven seconds later starts a second run on the same head" — a live timing claim about two specific Actions runs; not checkable from a read-only tree, would need the Actions run history/timestamps for #6637
• UNVERIFIED — "Seen 2026-10-23 on #6637, run 40033237161" — a specific historical run id; would need to query that run via the Actions API/UI

Riskiest lines

1. .github/workflows/pr.yml:342 — the new guard itself; if it over-fires it would silently swallow a genuine failure, if it under-fires the original bug (a cancelled run posting a bogus failure) persists — UNCOVERED (no behaviour test can exercise GitHub Actions' own cancellation semantics from this repo; the PR body gives the one-sentence reason: "proven by CI runs on this PR," and the pattern is already established elsewhere in this repo — build.yml and wpf-smoke.yml both use if: ${{ !cancelled() }} the same way)
2. .github/workflows/pr.yml:386 — the gh api .../statuses call the new guard gates; a wrong guard surfaces here as a wrongly-posted or wrongly-withheld commit status — UNCOVERED, same reasoning as above
3. .github/workflows/pr.yml:392 — unchanged downstream step whose steps.verdict.outputs.state != '' check is the safety net the PR body leans on if the new guard skips the step; re-read and confirmed it still holds (the output simply stays unset when the step is skipped) — not itself at risk from this diff, but it's the line that makes the change safe

Traps near this diff

• "dorny/paths-filter v4 evaluates each pattern as an INDEPENDENT predicate..." — does not apply: no paths-filter usage anywhere in this diff; the touched step has no file-pattern filtering.
• "Under set -euo pipefail, a bare $(gh api …) assignment that fails kills the step with one context-free stderr line" — does not apply: the gated script still uses set -uo pipefail (no -e, pre-existing and unchanged at .github/workflows/pr.yml:356), and its gh api call (:386) is piped to /dev/null, not captured into a variable via $(...).
• "gh api --paginate --jq runs the filter once PER PAGE" — does not apply: the changed step's gh api call (:386) uses neither --paginate nor --jq.

Divergence (blind read vs PR body)

None — the body's account matches the diff on every claim graded above. The body is also appropriately conservative: its test-plan checkboxes are left unchecked ("Run IDs and statuses to be filled in") rather than claiming verification that a read-only tree (or the author) hadn't yet produced.

Reviewer checklist

• Confirm on the live PR that the second ("empty... to reproduce") commit actually produced two pr.yml runs on one head, that the first run's verdict job skipped "Set CI result," and that the head's CI result status history shows no spurious failure from the cancelled run.

[claude]

branch=${HEAD_REF} filters GitHub's "list workflow runs" by head_branch alone. That field is the unqualified branch name — it is not qualified by head_repository/fork owner and is not the same thing as "a newer run of this workflow for this pull request." Two different contributors (especially forks) can share a branch name — GitHub's own web editor suggests patch-1, patch-2, … to every first-time contributor, so collisions on exactly that name are common for a repo that accepts outside PRs (the IS_FORK handling a few lines below shows this repo does).

Concretely: if PR A (this run) legitimately fails, and an unrelated PR B — from a different fork, branch also named patch-1 — happens to have a newer run of pr.yml, this check sees newer > 0, logs the "left to a newer run" notice, and exits 0 without setting state/failed. But run B belongs to a different PR and will set the commit status on a different SHA, never PR A's. PR A's CI result stays stuck at pending (set earlier in the plan job) with no failure ever posted and no further run to correct it — a quieter failure mode than the bug this PR is fixing, but still wrong.

The run objects returned by this same endpoint carry head_repository.full_name, which — combined with the branch name — uniquely identifies the PR. Matching on that (or on .pull_requests[].number for same-repo runs) before counting a run as "newer" would close this gap.

[claude]

The PR title and body ("Not a fix yet... It stays a draft and must not be merged as a fix", "What this PR changes: ... if: ${{ !cancelled() }} and a one-line comment above it. Nothing else.") describe the b56611dc7 commit, the attempt that the body itself says failed to reproduce the fix.

That is not what HEAD (ec46ece6) contains. The current diff removes no !cancelled() guard (there isn't one — grep -n "cancelled()" .github/workflows/pr.yml only matches a comment) and instead adds a gh api .../actions/workflows/pr.yml/runs lookup plus an actions: read permission — a different mechanism, added by a later commit (ec46ece6, "Leave CI result to a newer run when the API shows one, instead of cancelled()"), that the body never describes or evaluates. Two more "empty commit to reproduce" commits were pushed after ec46ece6, so testing of this approach may be in progress, but nothing in the visible record (title, body, test-plan checkboxes) says whether it passed, failed, or was even run. Please update the title/body (or close out the lane notes) before this is read as current — right now a reader has no way to tell from the PR narrative whether the code at HEAD works.

See the inline comment on pr.yml:379 for a correctness gap I found in that new mechanism itself (branch-name-only matching can pick up an unrelated PR/fork's run).

[claude]

Verdict: CHANGES REQUESTED

Claims

• VERIFIED — the new permission actions: read is present and the comment explains it's for the verdict job's newer-run lookup — pr.yml:25, used at pr.yml:379
• VERIFIED — the gh api lookup's failure is caught (2>/dev/null) || newer="") and falls through to posting as before, as the inline comment above the code claims ("If the call fails, post as before") — pr.yml:379-385; the regex guard [[ "${newer}" =~ ^[1-9][0-9]*$ ]] is false for an empty string, so the existing failure path at pr.yml:387 still runs
• VERIFIED — the suppression is gated to pull_request events only, so the dev-push "dev is red" issue logic (pr.yml:404-465, gated on github.event_name == 'push') is unaffected — confirmed by re-reading that step's condition
• REFUTED — PR body: "What this PR changes: ... if: ${{ !cancelled() }} and a one-line comment above it. Nothing else." — HEAD (ec46ece6) contains no if: ${{ !cancelled() }} guard anywhere (grep -n "cancelled()" .github/workflows/pr.yml matches only a comment at line 374); the actual diff adds a permission, an env var, and a ~13-line gh api lookup, none of which the body mentions
• REFUTED (as a description of HEAD) — PR body/title: "Not a fix yet... must not be merged as a fix" — this describes the abandoned b56611dc7 attempt's failure to reproduce; it is not an evaluation of the current ec46ece6 mechanism, which the body never discusses, tests, or claims to have verified
• UNVERIFIED — whether the ec46ece6 approach itself was reproduced successfully: two further "empty commit to reproduce" commits exist after it (1fdbd56a, 192cdd8e), but no run IDs, statuses, or updated test-plan checkboxes are given for them

Riskiest lines

1. pr.yml:379 — branch=${HEAD_REF} filters GitHub's workflow-run list by the unqualified branch name only, not by PR number or head_repository, so a different PR (e.g. an unrelated fork also named patch-1) with a newer run id can be mistaken for "a newer run of this workflow for this pull request" — covered by no behaviour test (exercises GitHub Actions' own cross-PR run listing, not reachable from this repo's test suite) | UNCOVERED
2. pr.yml:380-385 — the exit-0 suppression branch; if line 379 ever mismatches as above, this PR's CI result stays stuck at the pending set in the plan job forever, since the run that line 379 found "newer" belongs to a different PR and will never write this SHA's status — UNCOVERED, same reasoning
3. pr.yml:374-377 — the comment's claim that a timed-out test leg "also reports cancelled, but no newer run exists then, so that failure still posts" depends entirely on the branch-scoped lookup at line 379 correctly excluding unrelated runs; if it does not (finding above), this line's stated guarantee doesn't hold — UNCOVERED

Traps near this diff

• "Under set -euo pipefail, a bare $(gh api …) assignment that fails kills the step with one context-free stderr line... Every lookup goes through a wrapper that names the endpoint and decides whether a failure is a verdict." — does not apply: the step uses set -uo pipefail (no -e, pre-existing, unchanged), and the new gh api call explicitly catches failure with 2>/dev/null) || newer="" and falls through to the existing failure path — pr.yml:356, 379
• "gh api --paginate --jq runs the filter once PER PAGE" — does not apply: the new call uses neither --paginate nor pagination; it's a single per_page=20 request — pr.yml:379
• "dorny/paths-filter v4 evaluates each pattern as an INDEPENDENT predicate..." — does not apply: no paths-filter usage in this diff

Divergence (blind read vs PR body)

Substantial. The PR title and body describe and disclaim the b56611dc7 commit's if: ${{ !cancelled() }} attempt ("Not a fix yet... must not be merged as a fix"), which is no longer in the tree. HEAD is actually the later ec46ece6 commit, a different mechanism (an actions: read-gated gh api lookup for a newer run) that the body never mentions, describes, or claims to have validated. A reader relying on the PR's own narrative would not know this mechanism exists, let alone whether it works. Separately, that mechanism has the branch-matching gap in Riskiest line 1 above, which the body — being silent on the new code — obviously does not address either.

Reviewer checklist

• Update the PR title/body (or lane notes) to describe and evaluate the ec46ece6 mechanism actually at HEAD, including results of the two reproduction commits pushed after it (1fdbd56a, 192cdd8e)
• Reproduce the fork/branch-name-collision scenario (or confirm it can't occur under this repo's contribution model) before relying on branch=${HEAD_REF} to scope the newer-run check to this pull request specifically
• Confirm live, on an actual cancelled run, that CI result ends up success/correct rather than stuck pending

[claude]

branch=${HEAD_REF} scopes this "newer run exists" lookup by branch name only, not by pull request. This repository already has the correct pattern for exactly this problem in claude-review.yml:301-306: it filters primarily on .pull_requests[].number == PR (via env.PR), falling back to branch-only matching only once a pull request closes and that field goes empty — with an explicit comment explaining why: "A run is this pull request's when its list names the number; a run with none listed... counts through the branch."

This line skips that primary filter and uses only the fallback. Two concurrently open pull requests that happen to share a head branch name — including GitHub's own auto-generated patch-1 for a one-line web-UI edit, a very plausible collision in a public repo — would make this check see an unrelated PR's/fork's newer run and conclude (wrongly) that this run's result has been "superseded." That causes state=/failed= to be reset to empty (pr.yml:382-383) and the step to exit before ever calling gh api .../statuses (pr.yml:399), so this run's actual head SHA gets no CI result status at all — not a wrong status, but a silently-missing one, left to a "newer run" that has nothing to do with this PR and will never write this SHA's status.

Suggest adding PR: ${{ github.event.pull_request.number }} to this step's env: and filtering the --jq on .pull_requests[] containing that number, the same way claude-review.yml already does, rather than relying on branch name alone.

[claude]

The PR title and body describe and disclaim the b56611dc7 commit's if: ${{ !cancelled() }} attempt ("Not a fix yet... It stays a draft and must not be merged as a fix", "What this PR changes: ... if: ${{ !cancelled() }} and a one-line comment above it. Nothing else.").

That is not what HEAD (192cdd8e) contains. grep -n "cancelled()" .github/workflows/pr.yml matches only an explanatory comment, not a guard. A later commit, ec46ece6 ("Leave CI result to a newer run when the API shows one, instead of cancelled()"), replaced that approach entirely with a new actions: read permission plus a gh api .../actions/workflows/pr.yml/runs lookup — a different mechanism the body never describes or evaluates. Two more "empty commit to reproduce" commits (1fdbd56a, 192cdd8e) were pushed after ec46ece6, so testing of this approach may be in progress, but nothing in the visible record (title, body, test-plan checkboxes) says whether it passed, failed, or was even run.

See the inline comment on pr.yml:379 for a correctness gap in that new mechanism itself (branch-name-only matching can pick up an unrelated PR's/fork's run). Please update the title/body before this is read as current — right now a reader has no way to tell from the PR narrative whether the code at HEAD works.

[claude]

Verdict: CHANGES REQUESTED

Claims

• VERIFIED — the comment "the verdict job asks whether a newer run of this workflow exists for the pull request (#6656)" (pr.yml:25) — matches the lookup's actual use at pr.yml:379
• VERIFIED — the inline comment's claim "If the call fails, post as before" (pr.yml:377) — newer=$(gh api ... 2>/dev/null) || newer="" catches a failed call, and the guard [[ "${newer}" =~ ^[1-9][0-9]*$ ]] is false for an empty or non-numeric value, so execution falls through to the pre-existing failure-posting path at pr.yml:387-392
• REFUTED — PR body: "What this PR changes: .github/workflows/pr.yml, verdict job, step "Set CI result" (id verdict): if: ${{ !cancelled() }} and a one-line comment above it. Nothing else." — HEAD (192cdd8e, same tree as ec46ece6) contains no such guard anywhere; grep -n "cancelled()" .github/workflows/pr.yml matches only the explanatory comment at pr.yml:374-377. The actual diff adds a new actions: read permission (pr.yml:25), a HEAD_REF env var (pr.yml:354), and a ~13-line gh api lookup (pr.yml:378-386) — a different mechanism added by the later commit ec46ece6, which the body never mentions
• REFUTED (as a description of HEAD) — title/body: "Not a fix yet... It stays a draft and must not be merged as a fix" — this evaluates the abandoned b56611dc7 attempt's failure to reproduce the fix; it says nothing about the ec46ece6 mechanism actually in the tree
• UNVERIFIED — whether the ec46ece6 mechanism was itself validated: two further "empty commit to reproduce" commits exist after it (1fdbd56a, 192cdd8e), but no run IDs, statuses, or updated test-plan checkboxes are given for either

Riskiest lines

1. pr.yml:379 — branch=${HEAD_REF} scopes the "a newer run exists" lookup by branch name alone, not by pull request. claude-review.yml:301-306 already solves this exact problem in this repo by filtering primarily on .pull_requests[].number == PR, falling back to branch-only matching only once a PR closes; this new code skips the primary filter and uses only the fallback. Two open PRs sharing a head branch name (e.g. GitHub's auto-generated patch-1) would make this see an unrelated PR's newer run as this one's — UNCOVERED, no behaviour test in this repo exercises GitHub's cross-PR workflow-run listing
2. pr.yml:382-384 — downstream of the above: a false match resets state=/failed= to empty and exits before the gh api .../statuses call (pr.yml:399), so this run's own head SHA gets no CI result status at all, left to a "newer run" that belongs to a different PR and will never write this SHA's status — UNCOVERED
3. pr.yml:379 — ${HEAD_REF} is interpolated into the query string unescaped; a branch name containing #, &, or ? (all valid in git ref names) can corrupt the query string or change which runs the filter matches — UNCOVERED

Traps near this diff

• "Under set -euo pipefail, a bare $(gh api …) assignment that fails kills the step with one context-free stderr line... Every lookup goes through a wrapper that names the endpoint and decides whether a failure is a verdict." — does not apply: the script uses set -uo pipefail (no -e, pr.yml:356, pre-existing and unchanged), and the new $(gh api ...) assignment at pr.yml:379 is explicitly guarded with 2>/dev/null) || newer="" plus a regex check before use
• "gh api --paginate --jq runs the filter once PER PAGE: a max_by or last is per-page, not global" — does not apply: pr.yml:379 uses neither --paginate nor multiple pages, just a single per_page=20 request with a plain length count
• "dorny/paths-filter v4 evaluates each pattern as an INDEPENDENT predicate..." — does not apply: no paths-filter usage anywhere in this diff
• "Lite and Darling ship in parity (CONTRIBUTING's two-store rule)..." — does not apply: this is a CI workflow file with no Lite/Darling SKU counterpart requiring parity

Divergence (blind read vs PR body)

Substantial. The title and body describe and disclaim the b56611dc7 commit's if: ${{ !cancelled() }} attempt ("Not a fix yet... must not be merged as a fix"), which is no longer in the tree. HEAD actually carries the later ec46ece6 commit's different mechanism (an actions: read-gated gh api lookup for a newer run), which the body never mentions, describes, or claims to have validated — a reader relying on the PR's own narrative would not know this mechanism exists. That mechanism also has the branch-matching gap in Riskiest line 1 above, which the body — being silent on the new code — does not address.

Reviewer checklist

• Update the PR title/body to describe and evaluate the ec46ece6 mechanism actually at HEAD, including results of the two reproduction commits pushed after it (1fdbd56a, 192cdd8e)
• Reproduce the fork/branch-name-collision scenario (or confirm it can't occur under this repo's contribution model) before relying on branch=${HEAD_REF} alone to scope the newer-run check to this pull request
• Confirm live, on an actual cancelled run, that CI result ends up correct rather than stuck unset

[claude]

Follow-up pass over 192cdd8e..253a5cda1. The only authored commit in that range is d47de35a0 ("Scope the newer-run lookup to this pull request and read the live draft state"), touching only .github/workflows/pr.yml; the rest of the range is the 3d76e2cc5 merge of origin/dev, which only brought in base-branch content (the Darling/Storage test and compose-layer churn) and is not this author's change.

d47de35a0 is good — both pieces do what their headline says:

• The new gh api repos/.../pulls/${PR_NUMBER} --jq '.draft' read in the plan job (pr.yml:71-80) replaces the stale event-payload IS_DRAFT with the live value, directly closing the push/gh pr ready race the file's own top comment now documents (pr.yml:9-12).
• The verdict job's "newer run" lookup (pr.yml:399-411) now requires .head_repository.full_name == env.HEAD_REPO and an empty-or-matching .pull_requests list, not just branch=${HEAD_REF}. That's exactly the gap flagged on this PR's two earlier full-pass reviews (head 192cdd8e and ec46ece6): branch-name-only matching could pick up a different PR's/fork's newer run (e.g. two forks both on patch-1).

One divergence worth a body update, though: the PR description's own "Considered, not added" section currently says:

Filtering the runs by head repository so a fork PR with the same branch name does not count: it would only matter for a fork PR sharing a branch name with another PR that has a newer run, and the result is a pending status (fail-safe), not a green. No concrete trigger seen.

d47de35a0 adds exactly that filtering (pr.yml:402), reversing the "not added" decision, and the commit message gives no reason for the reversal. The body also says nothing about the live-draft-read mechanism at all. As written, the body describes a decision and a scope that this commit's own diff already contradicts — worth syncing before this is read as current, same as the two prior passes asked for the title/body to track whichever mechanism was actually at HEAD.

[claude]

Verdict: CHANGES REQUESTED

Claims

• VERIFIED — commit headline "Scope the newer-run lookup to this pull request" — the verdict job's lookup now filters on .head_repository.full_name == env.HEAD_REPO and requires .pull_requests to be empty or name env.PR_NUMBER, not branch name alone — .github/workflows/pr.yml:399-404
• VERIFIED — commit headline "read the live draft state" — the plan job now queries gh api repos/.../pulls/${PR_NUMBER} --jq '.draft' and overwrites IS_DRAFT with the live value before it feeds ci-plan.py --draft, instead of trusting the event payload's draft field — .github/workflows/pr.yml:71-80
• REFUTED — PR body, "Considered, not added": "Filtering the runs by head repository so a fork PR with the same branch name does not count: ... No concrete trigger seen." — d47de35a0 adds exactly this filtering (pr.yml:402), reversing the stated decision; the body is unedited since and still describes the old, narrower scope as current

Riskiest lines

1. .github/workflows/pr.yml:402 — the new head_repository/pull_requests scoping itself; if a newer run is wrongly excluded (an edge case in how the Actions API populates head_repository or pull_requests), this run posts a real failure instead of deferring to the newer one — if wrongly included, a genuine failure is silently swallowed — UNCOVERED, no behaviour test in this repo can exercise GitHub Actions' own cross-run listing (same limitation the two earlier reviews on this PR already noted for the branch-only version of this lookup)
2. .github/workflows/pr.yml:72 — the live-draft gh api call; a failure here (rate limit, transient 5xx) falls back to the stale payload value via the else branch, silently reopening the exact race this commit fixes for that one run — UNCOVERED, same reasoning
3. .github/workflows/pr.yml:76 — IS_DRAFT="${live_draft}" is the only place the live value reaches ci-plan.py; ci-plan-test.py only exercises cp.plan()'s handling of a --draft flag it's handed directly (ci-plan-test.py:132-151), so it cannot and does not cover whether the bash correctly computes that flag from the API in the first place — UNCOVERED

Traps near this diff

• "Under set -euo pipefail, a bare $(gh api …) assignment that fails kills the step with one context-free stderr line ... Every lookup goes through a wrapper that names the endpoint and decides whether a failure is a verdict." — does not apply: the live-draft read (pr.yml:72) sits inside an if condition under set -euo pipefail (pr.yml:66), so bash's errexit rules exempt it even on failure; the newer-run read (pr.yml:400) runs under set -uo pipefail (no -e, pr.yml:374, pre-existing) with an explicit 2>/dev/null) || newer="" fallback
• "gh api --paginate --jq runs the filter once PER PAGE: a max_by or last is per-page, not global." — does not apply: neither new call uses --paginate; both are single requests (per_page=20 at pr.yml:400, a single PR lookup at pr.yml:72)
• "A summary helper reading "$1" truncates every message whose echo wraps with a trailing backslash." — does not apply: the new ::notice:: lines (pr.yml:74, 78) are each one physical line, no backslash continuation
• "dorny/paths-filter v4 evaluates each pattern as an INDEPENDENT predicate..." — does not apply: no paths-filter usage anywhere in this diff

Divergence (blind read vs PR body)

The body's "Considered, not added" section claims head-repository filtering was deliberately left out; d47de35a0 adds it, reversing that call with no explanation in the commit message or an updated body. The body also never mentions the live-draft-read mechanism this commit adds. Posted as a separate comment on the PR with the specifics; this is the same category of body/HEAD drift the two earlier full-pass reviews on this PR flagged for the commits before this range.

Reviewer checklist

• Update the PR body's "Considered, not added" section (or remove it) now that head-repository/PR-number scoping is implemented, and add a line describing the live-draft-read fix
• Reproduce the push-to-draft-then-gh pr ready race live once more against this exact commit (the PR's own reproduction table covers ec46ece6/1fdbd56a/192cdd8e, not d47de35a0) and confirm the final head's CI result lands success, not stuck pending

[claude]

Divergence from the PR body. The body's "Considered, not added" section says:

Filtering the runs by head repository so a fork PR with the same branch name does not count: it would only matter for a fork PR sharing a branch name with another PR that has a newer run, and the result is a pending status (fail-safe), not a green. No concrete trigger seen.

But this is exactly what's shipped here: .head_repository.full_name == env.HEAD_REPO (line 402) plus the pull_requests membership check (lines 403-404) are the head-repository filter the body says was considered and left out. That filter landed in the later commit d47de35 ("Scope the newer-run lookup to this pull request and read the live draft state"), after the body's reproduction section was written, and the body was never updated to match — it still describes the pre-d47de35 version of this block.

Net effect: none of the reproduction cycles in the body (all against ec46ece6/1fdbd56a/192cdd8e) exercise this scoping filter at all. It may well be correct — the logic reads consistently (null-safe on a deleted fork's head_repository, matches only this PR's number or an empty pull_requests list) — but it's unverified by anything the body presents as evidence, and the body actively asserts the opposite of what's in the diff.

[claude]

Undocumented in the PR body, and it directly contradicts a claim there. This live-draft re-read isn't mentioned anywhere in "What this PR changes" or the reproduction table. Worse, the body's "What I could not reproduce" section says, about the push/gh pr ready ordering race:

That is an event-ordering race between the push and ready events, not something this change touches. Worth a look in a separate issue; I did not file one.

That's the exact race this block fixes: instead of trusting the pull_request payload's stale draft flag (captured when the event fired, not when the plan job runs), it re-reads .draft live from the API so a run that lands after gh pr ready sees the current state rather than skipping as a draft. This landed in the same later commit (d47de35) as the head-repository scoping flagged in the other comment — after the body's text was written, with no update to reflect it.

No reproduction cycle in the body exercises this path (the cycles described predate the commit that added it), so the claim that the race is untouched is simply false for the code actually shipped, and the fix for it is correspondingly unverified by anything presented.

[claude]

Review note

This is a workflow-only change (.github/workflows/pr.yml): a live re-read of the PR's draft state in the plan job, and a check in the verdict job for a newer pr.yml run before posting a CI failure. Mechanically, both pieces read as sound on a blind walk through the shell: the gh api calls that can fail are either inside an if condition (exempt from set -e) or guarded by || newer="" under set -uo pipefail, the .head_repository/pull_requests scoping is null-safe, and the new actions: read permission matches what the verdict job's new lookup needs.

The finding is in how the PR body accounts for its own commits, posted inline on the two blocks it affects:

• The body's "Considered, not added" section says head-repository filtering for forks was left out. The shipped code adds exactly that filter (.head_repository.full_name == env.HEAD_REPO plus the pull_requests membership check).
• The body's "What I could not reproduce" section says the push/gh pr ready ordering race is "not something this change touches." The shipped code's live-draft re-read in the plan job is a direct fix for that race.

Both landed in the last substantive commit (d47de35, "Scope the newer-run lookup to this pull request and read the live draft state"), after the reproduction section was written. The body was never updated, so every reproduction cycle it presents as evidence predates both pieces — neither the fork-scoping filter nor the live-draft fix has been exercised by anything the body shows.

No correctness bug found in the shipped logic itself from a read-only tree; the issue is that the body's account of what was tested and what was deliberately left out no longer matches what's in the diff.

[claude]

Verdict: CHANGES REQUESTED

Claims

• VERIFIED — "Removes round 1's if: ${{ !cancelled() }} and its comment." — no !cancelled() line exists anywhere in the final pr.yml (confirmed by grep across the file); consistent with it being added and removed across the PR's own commit history.
• VERIFIED — "Adds actions: read to permissions: (the call needs it...)" — pr.yml:27.
• VERIFIED — "The dev is red step is unchanged: it runs only on push events, where this check does not apply." — pr.yml:429 onward shows no diff to that step; its if stays gated on github.event_name == 'push'.
• VERIFIED (incompletely described) — "asks the API whether a newer pr.yml run exists for this branch... any run with an id above this run's" — the id check exists (pr.yml:401), but the description omits the head_repository and pull_requests scoping that the shipped filter also applies (pr.yml:402-404) — see REFUTED claim below, which is the other half of this same filter.
• REFUTED — "Considered, not added: Filtering the runs by head repository so a fork PR with the same branch name does not count" — the shipped filter adds exactly this: .head_repository.full_name == env.HEAD_REPO plus a pull_requests membership check (pr.yml:394-404), landed in commit d47de35 after the body's "Considered, not added" text was written.
• REFUTED — "[the push/gh pr ready ordering race is] not something this change touches. Worth a look in a separate issue; I did not file one." — the same commit d47de35 adds a live re-read of the PR's draft state in the plan job (pr.yml:68-80) specifically to close that race (reading current state instead of the stale event payload), directly contradicting the claim that this change doesn't touch it.

Riskiest lines

1. .github/workflows/pr.yml:394-404 — the head_repository/pull_requests scoping filter the body says wasn't added; no reproduction cycle in the body exercises it — UNCOVERED (no behaviour test exists for workflow-YAML bash in this repo; GitHub Actions logic isn't unit-tested here)
2. .github/workflows/pr.yml:68-80 — the live draft-state re-read, which is the actual fix for the race the body disclaims fixing; untested by anything the body presents — UNCOVERED
3. .github/workflows/pr.yml:400-404 — the multi-line --jq filter, the first instance in this codebase of a jq program spanning physical lines inside a single-quoted argument in a file the repo's .gitattributes checks out as CRLF (confirmed via file .github/workflows/pr.yml); relies on jq treating the embedded \r bytes between tokens as insignificant whitespace — UNCOVERED, and unlike the other two risks this one isn't addressed by the body at all

Traps near this diff

• "Under set -euo pipefail, a bare $(gh api …) assignment that fails kills the step with one context-free stderr line... Every lookup goes through a wrapper that names the endpoint and decides whether a failure is a verdict." (#2309) — does not apply: the plan job's live-draft read sits inside an if live_draft=$(...) && [[ ... ]]; then condition (pr.yml:72), which bash's -e does not trigger on; the verdict job's lookup runs under set -uo pipefail with no -e (pr.yml:374) and falls back explicitly via || newer="" (pr.yml:404).
• "gh api --paginate --jq runs the filter once PER PAGE: a max_by or last is per-page, not global" — does not apply: neither new gh api call uses --paginate (pr.yml:72, 400).
• ".gitattributes says text=auto eol=crlf... A diff that touches every line of a file is a line-ending conversion until proven otherwise" — does not apply: gh pr diff shows isolated hunks (pr.yml:7-12, 22-27, 65-80, 367-411), not a whole-file rewrite; the file's pre-existing CRLF endings are unchanged background, not something this diff did.

Divergence (blind read vs PR body)

The PR body's "Considered, not added" and "What I could not reproduce" sections describe the pre-d47de35 version of this diff. The final commit adds exactly the two things the body says were left out or untouched: the fork/head-repository scoping, and the live-draft re-read that fixes the push/gh pr ready race. The body was written against an earlier state of the branch and never updated after the last substantive commit, so its reproduction evidence (all against ec46ece6/1fdbd56a/192cdd8e) validates none of what's actually shipped in d47de35. The shipped mechanism reads as internally consistent on a blind walk, but nothing in the body demonstrates it, and parts of the body assert the opposite of what's in the tree.

Reviewer checklist

• Confirm live that a draft PR pushed-to and then gh pr ready'd within seconds (the exact race in cycle 1 of the body) now gets a non-pending CI result on its final head, now that the live-draft read and the newer-run check both ship together.

Claude posting for Erik Darling. Addressed: the body now matches the shipped filter and the plan job's live draft read, and the CRLF jq filter was run locally with CRLF and LF line endings on #6691's runs and returned the same result.

[claude]

Non-blocking observation: build.yml's gate job, wpf-smoke.yml's draft check, and claude-review.yml's draft gate all read github.event.pull_request.draft straight from the event payload, with no live re-read — the same staleness this PR fixes for pr.yml's plan job. The failure mode differs there (build.yml's declined-run path fails its required checks loudly instead of leaving them unset, so it isn't the silent "stuck pending forever" shape #6656 describes for pr.yml), and this PR is explicitly scoped to pr.yml only, so I'm not asking for it here — just flagging it in case the same push-then-gh pr ready race is worth closing in those workflows too.

[claude]

Verdict: LGTM

Claims

• VERIFIED — "before it posts a failure, it asks the API whether a newer run of pr.yml exists for this pull request... writes empty state= and failed= outputs and exits 0" — pr.yml:399-410
• VERIFIED — "If the call fails or returns a non-count, it posts as before." — pr.yml:404-405 (|| newer="" plus the ^[1-9][0-9]*$ guard before use)
• VERIFIED — "gh api -X GET .../actions/workflows/pr.yml/runs -f event=pull_request -f branch=$HEAD_REF -f per_page=20... mirrors claude-review.yml" — pr.yml:400 and claude-review.yml:303 (-f branch="$HEAD_REF")
• VERIFIED — "A run counts when its id is above GITHUB_RUN_ID, its head_repository.full_name equals this PR's head repo, and its pull_requests is empty or names this PR's number." — pr.yml:401-404
• VERIFIED — "Live draft read (plan job): reads gh api repos/$R/pulls/ --jq .draft... uses it in place of the payload's IS_DRAFT... notice on mismatch... fallback with notice on failure." — pr.yml:71-80
• VERIFIED — "pull-requests: read was already granted." — pr.yml:26 (unchanged context line)
• VERIFIED — "Adds actions: read to permissions" — pr.yml:27
• VERIFIED — "The dev is red step is unchanged: it runs only on push events." — pr.yml:430 (unchanged context, if: ... github.event_name == 'push' ...)
• UNVERIFIED — "Seen on #6637, run 40033237161" and the whole reproduction-run table (run IDs, cancellation outcomes) — live GitHub Actions run history, not inspectable from a read-only tree
• UNVERIFIED — "I ran the filter against this repo's real run list: 8 matches for this repo and PR, 0 for a different head repo." — live API measurement
• UNVERIFIED — "the coordinator ran the exact filter with CRLF and with LF line ends against the real runs of #6691... both returned 1" — live measurement; independently, the file's .gitattributes-driven CRLF is normalized away by the YAML parser before gh api --jq ever sees the block scalar's content, so the practical conclusion holds either way

Riskiest lines

1. pr.yml:399-410 — if the "newer run exists" lookup finds a run that never itself reaches verdict (e.g. it too gets cancelled, or sits outside the 20-run/per_page window), this run abstains and nothing ever sets CI result, reproducing the original stuck-pending shape for a different reason — UNCOVERED (CI workflow logic; no behaviour test in this repo's suite can exercise GitHub Actions run sequencing, and the PR body's live reproduction runs stand in for one)
2. pr.yml:71-80 — a misread live .draft value would push a ready PR back into skip-draft or run a draft early; the ^(true|false)$ regex guard makes this narrow, but it's unverifiable here — UNCOVERED, same reason as above
3. pr.yml:400-404 — the jq filter's head_repository.full_name/pull_requests matching is the only thing stopping two different PRs that share a fork and branch name from being conflated (e.g. the same fork branch opened against both main and dev) — UNCOVERED, same reason as above

Traps near this diff

• "Under set -euo pipefail, a bare $(gh api …) assignment that fails kills the step with one context-free stderr line... Every lookup goes through a wrapper that names the endpoint and decides whether a failure is a verdict." — does not apply — both new gh api calls are guarded: pr.yml:72 sits as an if condition (so set -e does not fire on its failure) with an explicit else branch, and pr.yml:400/404 pairs the assignment with || newer="" and a regex check before use
• "dorny/paths-filter v4 evaluates each pattern as an INDEPENDENT predicate under predicate-quantifier: some..." — does not apply — pr.yml contains no paths-filter usage anywhere (grep -n "paths-filter" .github/workflows/pr.yml is empty)
• "gh api --paginate --jq runs the filter once PER PAGE..." — does not apply — neither new call passes --paginate (pr.yml:72 fetches a single resource; pr.yml:400 fetches one page of 20)
• ".gitattributes says text=auto eol=crlf: ... A diff that touches every line of a file is a line-ending conversion until proven otherwise" — does not apply — gh pr diff shows only the added/modified hunks, not a whole-file rewrite; the pre-existing CRLF endings are unchanged by this PR

Divergence (blind read vs PR body)

None — the body's account matches the diff on every claim graded above. (Posted a separate non-blocking comment noting that build.yml, wpf-smoke.yml and claude-review.yml have the same stale-payload draft read this PR fixes in pr.yml's plan job; the PR explicitly scopes itself to pr.yml only, so this is an observation, not a divergence.)

Reviewer checklist

• Confirm on GitHub that the final head from the PR's race-cycle test, commit 5709f554, finished with CI result = success (the PR body's own "NOT PROVEN" item at the time of writing)