diff --git a/.github/workflows/claude-author-automerge.yml b/.github/workflows/claude-author-automerge.yml index 82b69d2..3899f93 100644 --- a/.github/workflows/claude-author-automerge.yml +++ b/.github/workflows/claude-author-automerge.yml @@ -152,7 +152,7 @@ on: type: boolean default: true findings_quiet_minutes: - description: "Quiet period (minutes) the PR must be free of new commits and review-bot comments before auto-merge is armed, after which the unaddressed-findings check runs (wxa_vpn#1392: findings posted 3-16 min after the last push merged unanswered). Applies to EVERY arm path, bypass labels included — the escape for a wrong finding is the manual merge click, which this gate never blocks. Set 0 to disable the whole gate (no wait, no findings check)." + description: "Quiet period (minutes) the PR must be free of new commits, review-bot comments and review triggers (ready_for_review / reopened) before auto-merge is armed, after which the unaddressed-findings check runs (wxa_vpn#1392: findings posted 3-16 min after the last push merged unanswered; #1512: a draft readied after 2h09m idle cleared the window without waiting at all and armed 35s before a REGRESSION verdict). Applies to EVERY arm path, bypass labels included — the escape for a wrong finding is the manual merge click, which this gate never blocks. Set 0 to disable the whole gate (no wait, no findings check)." required: false type: number default: 20 @@ -1032,10 +1032,17 @@ jobs: # 1. QUIET — wait until >= findings_quiet_minutes have passed since # the newest of: the newest AUTHORED (non-merge) commit, the # newest inline review comment (any author — an inline comment - # is a deliberate code-level finding), and the newest top-level + # is a deliberate code-level finding), the newest top-level # [bot] comment that is not this workflow's own marker comment # or pr-classify chatter (a gate that counts its own sticky - # comment as review activity never opens). Sleeping here is + # comment as review activity never opens), the newest + # `ready_for_review`/`reopened` event, and the PR's own creation + # time — the triggers that START a round of review, without + # which the clock is pre-expired on exactly the events that + # begin one (wxa_vpn#1512; see c4/c5 above read_anchor for the + # measurement — `opened` is in the same `types:` list, so a + # branch whose commits idled before the PR was opened non-draft + # is the same hole through a different door). Sleeping here is # safe: callers run concurrency cancel-in-progress per PR, so a # new push replaces this run and restarts the clock, and the # arm below binds --match-head-commit so a moved head can never @@ -1080,6 +1087,10 @@ jobs: PR: ${{ github.event.pull_request.number }} PR_URL: ${{ github.event.pull_request.html_url }} QUIET_MINUTES: ${{ inputs.findings_quiet_minutes }} + # The c5 anchor (see read_anchor). From the event payload rather + # than an API read; immutable, so a later labeled/synchronize + # re-run sees the same fixed value aged out. + PR_CREATED_AT: ${{ github.event.pull_request.created_at }} run: | set -euo pipefail retry() { local i out; for i in 1 2 3; do if out=$("$@"); then printf '%s' "$out"; return 0; fi; [ "$i" -lt 3 ] && sleep $((i * 5)); done; return 1; } @@ -1098,6 +1109,83 @@ jobs: # a --paginate stream (one top-level array per page) aggregates # ACROSS pages, not per page — the multi-page defect pinned in # test_automerge_hold_gate.sh. + # + # c4 — THE REVIEW-TRIGGER ANCHOR, and the reason this gate armed + # over a live finding on wxa_vpn#1512 (2026-08-14). The other + # three anchors all measure silence that has ALREADY ELAPSED, + # which is the right question only when the event that woke this + # workflow did not itself start a review. `ready_for_review` and + # `reopened` do exactly that: pr-review.yml, pr-codex-review.yml + # and this workflow share the same `types:` list, so readying a + # draft fires the review lanes and arms the gate IN THE SAME + # INSTANT — while every anchor the gate could read still dates + # from before the PR was drafted. + # + # Measured on #1512: the PR sat as a draft for 2h09m, so at + # ready_for_review the step logged + # Quiet: newest commit/review activity is 7757s old (>= 1200s) + # cleared the 20-minute window without sleeping once, ran the + # detector against a conversation the reviews had not joined yet, + # got a legitimate "no unaddressed findings", and armed at + # 05:38:26Z. Codex posted VERDICT: REGRESSION at 05:39:01Z — 35 + # seconds later — and the PR merged at 05:42:51Z carrying it. + # #1518 cleared the same way at 3074s (its review happened to + # come back clean, so it was a near miss rather than a hit). + # + # The fix is not a longer window — no window helps when the clock + # starts pre-expired. It is to anchor on the moment the CURRENT + # round of review was triggered, so `ready_for_review` waits + # exactly as long as a push does. That is not new policy: the + # push path already waits (wxa_vpn#1513 slept 1162s + 441s and + # merged clean), and this only stops the ready path from skipping + # a wait the push path has always taken. + # + # These are POINT-IN-TIME event timestamps, not "now", so a later + # `labeled` run re-reads the same fixed value and sees it age out + # normally — the gate cannot be held shut by its own re-runs. A + # PR opened non-draft and never reopened contributes no c4; its + # `opened` trigger is anchored by c5 (creation time) below. + # + # Fail-closed like every sibling read: `issues/{n}/timeline` is + # already load-bearing in this job (the detector fetched below + # reads issues/{n}/comments fatally), so an unreadable timeline + # is an unreadable anchor, not an absent one. What fail-closed + # covers is READABILITY — the fetch and the parse. A matching + # event that carries `created_at: null` (never observed from the + # API) is readable but UNDATABLE, and an undatable event simply + # contributes no anchor: c4 carries a timestamp, not an + # existence claim, and a clock cannot be anchored to an event + # with no time. + # + # c4 and c5 are IMMUTABLE, so they are resolved ONCE, outside the + # wait loop — historical timeline events and the PR's creation + # time cannot change while this run sleeps, and a long timeline + # costs --paginate round-trips on every wake if read in-loop. The + # one way a NEW review trigger can appear mid-sleep is a fresh + # ready_for_review event, and that event's own run replaces this + # one via the callers' cancel-in-progress concurrency groups, so + # nothing is missed by not re-reading. + # + # c5 — the PR's creation time — anchors the `opened` trigger: + # it is in the same review-lane `types:` list as ready_for_review, + # so a branch whose commits idled before the PR was opened + # NON-draft arrives with c1 pre-aged and would otherwise arm at + # the instant the reviews start — wxa_vpn#1512's hole through a + # different door. It comes from the event payload (no API read); + # an empty value (defensive — every pull_request payload carries + # created_at) simply contributes no anchor. + timeline=$(retry gh api "repos/${GITHUB_REPOSITORY}/issues/${PR}/timeline?per_page=100" --paginate) || { + echo "::error::could not read the PR timeline (the ready/reopen anchor) after retries — refusing to arm auto-merge (fail closed)." + exit 1 + } + c4=$(printf '%s' "$timeline" | jq -rs '[(add // [])[] + | select(.event == "ready_for_review" or .event == "reopened") + | .created_at] | max // empty') || { + echo "::error::could not parse the PR timeline — refusing to arm auto-merge (fail closed)." + exit 1 + } + c5="${PR_CREATED_AT:-}" + read_anchor() { local commits inline issue c1 c2 c3 commits=$(retry gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR}/commits?per_page=100" --paginate) || return 1 @@ -1109,14 +1197,14 @@ jobs: | select(((.user.login // "") | endswith("[bot]"))) | select(((.body // "") | test("