From 8a2c206732083b4b9bf9c44704321604bf1d7532 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Fri, 29 May 2026 01:47:34 +0200 Subject: [PATCH] fix(pr-management-triage): guard Sweep 4 against freshly-promoted PRs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sweep 4 (stale ready-for-review label) in stale-sweeps.md proposes `strip-ready-label` (4a, healthy branch) or `close` (4b, rotted branch) when a PR carries the `ready for maintainer review` label, a maintainer commented ≥ 7 days ago, and the author has been silent since. The "maintainer commented ≥ 7 days ago" check was naive: it counted ANY maintainer comment older than 7 days, including ones that predate the `ready for maintainer review` label-add. So a PR that was promoted to ready on day N could be closed by Sweep 4b on day N+2 — because the maintainer comment that initially nudged the contributor (and that the contributor's eventual fix addressed, leading to the promotion) was now > 7 days old and the contributor was, by definition, "silent" since. This is backwards. When the `ready for maintainer review` label is added, the queue moves to the maintainers; the author has nothing they need to reply to. Pre-label maintainer comments are part of the conversation that *got* the PR to ready, not evidence of silence on something the author still owes. == Fix == Sweep 4's "Common trigger" now requires the qualifying maintainer comment to have come AFTER the label was added: last_maintainer_comment_at > ready_label_added_at `ready_label_added_at` is the most recent `LabeledEvent { label.name == "ready for maintainer review" }` timestamp from the PR's `timelineItems`. If no maintainer has commented post-promotion, the sweep doesn't fire at all — the queue is on the maintainers, not the contributor. The new guard mirrors the "after the label-add timestamp" pattern that classify-and-act.md row F4 already uses for regression detection, so the convention is consistent across the skill. == Live incident motivating the fix == apache/airflow#66642 — contributor was promoted to `ready for maintainer review` on 2026-05-24, closed by this skill on 2026-05-26 with *"no author response for 15 days since the last maintainer comment"*. The cited maintainer comment was from 2026-05-11, well before the promotion. Contributor pushed back politely; PR reopened 2026-05-28; this is the skill-side fix. == Verification == `skill-and-tool-validate` exits 0. Generated-by: Claude Code (Opus 4.7) --- .../pr-management-triage/stale-sweeps.md | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/.claude/skills/pr-management-triage/stale-sweeps.md b/.claude/skills/pr-management-triage/stale-sweeps.md index 666de62a..d251695d 100644 --- a/.claude/skills/pr-management-triage/stale-sweeps.md +++ b/.claude/skills/pr-management-triage/stale-sweeps.md @@ -205,13 +205,33 @@ The label name comes from ### Common trigger (4a and 4b) - The PR carries the `ready for maintainer review` label. -- `last_maintainer_comment_at` is not null and - ` - last_maintainer_comment_at >= 7 days`. +- The `ready for maintainer review` label was added ≥ 7 days ago + (` - ready_label_added_at >= 7 days`, where + `ready_label_added_at` is the most recent + `LabeledEvent { label.name == "ready for maintainer review" }` + timestamp from the PR's `timelineItems`). +- The most recent maintainer comment came **after** the label + was added (`last_maintainer_comment_at > ready_label_added_at`) + AND ` - last_maintainer_comment_at >= 7 days`. - `last_author_activity_at` is null **or** `last_author_activity_at <= last_maintainer_comment_at`. -The last condition makes this sweep about *author silence*, -not label age — a "still working on it" reply resets the clock. +The "maintainer comment after label-add" condition is the +load-bearing guard against the freshly-promoted misfire: when a +maintainer just added the `ready for maintainer review` label, +the queue moves to the maintainers, not the author. A pre-label +maintainer comment is part of the conversation that *got* the PR +to ready; counting it as proof of author silence would close PRs +the moment they get promoted, which is the opposite of what +`ready for maintainer review` means. This guard mirrors the +"after the label-add timestamp" pattern row F4 already uses for +regression detection (see +[`classify-and-act.md#decision-table`](classify-and-act.md), F4 +row). + +The author-activity condition makes this sweep about *author +silence*, not label age — a "still working on it" reply resets +the clock. ### 4a — Branch healthy → strip label