This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow-steward.git
The following commit(s) were added to refs/heads/main by this push:
new 494217b pr-management-triage: ask author before promoting
unresolved-thread PRs (#175)
494217b is described below
commit 494217b9080e3649a68c544ffe19d14d45491a51
Author: Yeonguk Choo <[email protected]>
AuthorDate: Sun May 17 01:50:55 2026 +0900
pr-management-triage: ask author before promoting unresolved-thread PRs
(#175)
---
.claude/skills/pr-management-stats/render.md | 4 +-
.claude/skills/pr-management-triage/SKILL.md | 48 ++++--
.claude/skills/pr-management-triage/actions.md | 120 ++++++++-------
.../pr-management-triage/classify-and-act.md | 61 +++++++-
.../pr-management-triage/comment-templates.md | 83 ++++++----
.../pr-management-triage/interaction-loop.md | 38 +++--
.claude/skills/pr-management-triage/rationale.md | 168 ++++++++++++++++++---
.../skills/pr-management-triage/stale-sweeps.md | 89 ++++++++++-
.../pr-management-triage-comment-templates.md | 17 ++-
9 files changed, 480 insertions(+), 148 deletions(-)
diff --git a/.claude/skills/pr-management-stats/render.md
b/.claude/skills/pr-management-stats/render.md
index 3a5ecb2..7956461 100644
--- a/.claude/skills/pr-management-stats/render.md
+++ b/.claude/skills/pr-management-stats/render.md
@@ -188,7 +188,7 @@ The "What needs attention" panel is built from this fixed
rule set, evaluated in
| 3 | `len(stale_triaged_drafts) > 0` (drafts triaged ≥ 7d ago, no reply) |
medium | 🗑️ | `Close <N> stale-triaged drafts (≥7d, no response)` | Closure
path lives under the `stale` flow (sweep step 1a). | `/pr-management-triage
stale` |
| 4 | `len(ready_open) >= 50` | high | 📥 | `<N> PRs labeled "ready for
maintainer review"` | The `ready for maintainer review` queue is past the
triage stage; it needs maintainer review attention, not triage. |
`/pr-management-code-review ready` |
| 5 | `20 <= len(ready_open) < 50` | medium | 📥 | `<N> PRs in "ready for
maintainer review" queue` | Same trigger family as rule 4 — banded by queue
size so the priority drops once the queue is comfortable. |
`/pr-management-code-review ready` |
-| 6 | `len(responded_no_ready) > 0` (triaged + responded but not
ready-for-review) | medium | 🔄 | `<N> triaged PRs have author responses
awaiting re-triage` | These will surface as mark-ready-with-ping inside the
regular triage sweep. | `/pr-management-triage all PR issues` |
+| 6 | `len(responded_no_ready) > 0` (triaged + responded but not
ready-for-review) | medium | 🔄 | `<N> triaged PRs have author responses
awaiting re-triage` | These will surface as request-author-confirmation (first
leg of the two-sweep mark-ready gate) inside the regular triage sweep. |
`/pr-management-triage all PR issues` |
| 7 | top area's `untriaged_4w + untriaged_1_4w >= 5` | medium | 📍 | `Area
"<area>" has <total> contributor PRs (<X> untriaged >4w)` | One area is
dominating the untriaged queue; scoping a triage pass to it clears the bulk of
the load. | `/pr-management-triage label:area:<area>` |
| 8 | `velocity_drop > 30` (last_wk total - this_wk total) | low | 📉 | `PR
closure velocity dropped <N> this week` | No immediate action — re-check next
week to see if the drop persists or was a one-off. | — |
| 9 | top ready-trend area's growth in last 7d ≥ 10 PRs | low | 📈 |
`Ready-for-review queue in "<area>" grew by <N> this week` | Growth
concentrated in one area suggests it'd benefit from a focused review pass. |
`/pr-management-code-review label:area:<area>` |
@@ -248,7 +248,7 @@ Distinct from the colour scheme: these are the four
conceptual *states* a contri
| Marker | Definition | Colour | Maintainer action |
|---|---|---|---|
| `Ready for review` | `ready for maintainer review` label is present | green
| run `/maintainer-review` to actually code-review the PR |
-| `Responded` | PR is triaged AND author has commented or pushed after the
triage comment, AND not yet `Ready` | bright cyan | re-triage; may now qualify
for `mark-ready-with-ping` |
+| `Responded` | PR is triaged AND author has commented or pushed after the
triage comment, AND not yet `Ready` | bright cyan | re-triage; may now qualify
for `request-author-confirmation` (first leg of the two-sweep mark-ready gate) |
| `Waiting for Author` | PR is triaged, no author response — OR — PR is a
draft (whether triaged or not) | amber | nothing; author owns the next move
(may become a sweep candidate after 7d) |
| `Not yet triaged` | None of the above. Non-draft PR that has never received
a quality-criteria comment | blue (or grey) | run `/pr-management-triage` to
give it a first look |
diff --git a/.claude/skills/pr-management-triage/SKILL.md
b/.claude/skills/pr-management-triage/SKILL.md
index afe15c8..c6c59e3 100644
--- a/.claude/skills/pr-management-triage/SKILL.md
+++ b/.claude/skills/pr-management-triage/SKILL.md
@@ -7,9 +7,10 @@ description: |
propose a disposition, and — on the maintainer's
confirmation — carry out the action via `gh`. Disposition
options per PR: draft / comment / close / rebase / CI-rerun
- / workflow-approve / ping-stale-reviewer / mark `ready for
- maintainer review`. Does **not** perform code review — that
- lives in `pr-management-code-review`.
+ / workflow-approve / ping-stale-reviewer / request author
+ confirmation of readiness / mark `ready for maintainer
+ review`. Does **not** perform code review — that lives in
+ `pr-management-code-review`.
when_to_use: |
Invoke when a maintainer says "triage the PR queue", "go
through new contributor PRs", "run the morning triage",
@@ -148,10 +149,17 @@ bot checks (`Mergeable`, `WIP`, `DCO`, `boring-cyborg`)
while
`Tests`, `CodeQL`, and newsfragment-check sit in
`action_required`; trusting the rollup there fills the
maintainer-review queue with PRs whose real CI never ran. The
-guard applies identically to the
-[`mark-ready-with-ping`](actions.md#mark-ready-with-ping--promote-a-likely-addressed-pr--ping-reviewers)
-action — any code path that adds the `ready for maintainer
-review` label runs the REST check first.
+guard applies identically to every code path that adds the
+`ready for maintainer review` label, including the
+[`mark-ready`](actions.md#mark-ready--add-ready-for-maintainer-review-label)
+action invoked from
+[row 14a](classify-and-act.md#decision-table) after author
+confirmation. The
+[`request-author-confirmation`](actions.md#request-author-confirmation--ask-the-pr-author-whether-feedback-is-addressed)
+action itself does not add the label (it only posts a comment),
+so the REST check is not required there — but the subsequent
+sweep that promotes the PR via `mark-ready` runs the check
+exactly as documented above.
Implementation recipe: [`actions.md#mark-ready`](actions.md).
**Golden rule 2 — propose in groups, fall back to per-PR.** The
@@ -379,14 +387,20 @@ the order:
3. `deterministic_flag` with actions `draft` / `comment` /
`rebase` / `rerun` / `ping` — in that order
4. `stale_review` → `ping`
-5. `deterministic_flag` → `mark-ready-with-ping` (label-bearing
- group, presented just before plain `mark-ready` so the
- maintainer reviews all label-add proposals back-to-back)
-6. `passing` → `mark-ready`
-7. Stale sweeps (`stale_draft` → `close`, `inactive_open` →
+5. `deterministic_flag` → `request-author-confirmation`
+ (engagement heuristic fired; ask the author whether the
+ PR is ready before any label or reviewer-ping is generated
+ — first leg of the two-sweep gate)
+6. `author_confirmed_ready` → `mark-ready` (author replied
+ to a prior request; silent label apply, presented just
+ before plain `mark-ready` so the maintainer reviews all
+ label-add proposals back-to-back)
+7. `passing` → `mark-ready`
+8. Stale sweeps (`stale_draft` → `close`, `inactive_open` →
`draft`, `stale_workflow_approval` → `draft`,
`stale_ready_label` → `strip-ready-label`,
- `stale_ready_label_unhealthy` → `close`)
+ `stale_ready_label_unhealthy` → `close`,
+ `stale_author_confirm_request` → `ping`)
For each group, present one screen worth of headline info
(PR number, title, author, 1-line reason, label chips) and
@@ -447,6 +461,10 @@ When the maintainer has worked through every interactive
group
strip the label (4a — branch healthy) or propose `close`
(4b — red CI or merge conflicts). See
[`stale-sweeps.md#sweep-4--stale-ready-for-review-label`](stale-sweeps.md#sweep-4--stale-ready-for-review-label).
+- on PRs holding a pending author-confirmation request
+ (first leg of row 14c) whose author has been silent ≥ 7
+ days, propose plain `ping` to escalate. See
+
[`stale-sweeps.md#sweep-5--stale-author-confirm-request`](stale-sweeps.md#sweep-5--stale-author-confirm-request).
Each sweep emits its own group in the interaction loop (Step 3),
so the maintainer still confirms before any PR is touched.
@@ -461,8 +479,8 @@ excludes labeled PRs) — see
On exit, print a one-screen summary:
- counts of PRs handled per action (drafted, commented, closed,
- rebased, reruns triggered, marked ready, pinged, workflow
- approvals, suspicious flags)
+ rebased, reruns triggered, author-confirm requests posted,
+ marked ready, pinged, workflow approvals, suspicious flags)
- counts of PRs skipped and per-reason breakdown (already
triaged, inside grace window, bot, collaborator)
- counts of PRs left pending (reached quit, didn't finish the
diff --git a/.claude/skills/pr-management-triage/actions.md
b/.claude/skills/pr-management-triage/actions.md
index 23c4f1a..864d2a7 100644
--- a/.claude/skills/pr-management-triage/actions.md
+++ b/.claude/skills/pr-management-triage/actions.md
@@ -220,78 +220,84 @@ error; this is the only action of the skill whose sole
purpose
---
-## `mark-ready-with-ping` — promote a likely-addressed PR + ping reviewers
+## `request-author-confirmation` — ask the PR author whether feedback is
addressed
-A composite of `mark-ready` plus a `ping` comment. Used when
-the only `deterministic_flag` signal is unresolved review
-threads **and** the
+Single mutation. Used when the only `deterministic_flag` signal
+is unresolved review threads **and** the
[`unresolved_threads_only_likely_addressed`](classify-and-act.md#unresolved_threads_only_likely_addressed)
sub-flag is true (the author has engaged with every unresolved
thread via a post-comment commit or an in-thread reply).
-**Same mandatory pre-mutation guard as `mark-ready`.** Run
-the `action_required` REST check first and refuse — by
-reclassifying as `pending_workflow_approval` — if any workflow
-run is awaiting approval. The reasoning in the
-[`mark-ready`](#mark-ready--add-ready-for-maintainer-review-label)
-section above applies identically here.
-
-Order of operations: **post the comment first, then add the
-label.** The comment names the reviewers and asks them to
-re-look at the threads; the label then routes the PR into the
-review queue. If the label is added first, the reviewers see a
-"ready for maintainer review" notification before the comment
-that explains *why* it was promoted, which reads as the bot
-mark-ready'ing without context.
+The action does **not** add the `ready for maintainer review`
+label and does **not** `@`-mention the original reviewers.
+Those steps belong to the second leg of this two-sweep flow,
+gated on an explicit author confirmation
+([row 14a](classify-and-act.md#decision-table) →
+[`mark-ready`](#mark-ready--add-ready-for-maintainer-review-label)).
```bash
-# 1. Pre-check: refuse if any workflow run is awaiting approval (same as
mark-ready).
-# Runs awaiting approval surface as `status: "completed"` +
-# `conclusion: "action_required"` — `?status=action_required` matches
-# none of them, so post-filter on `conclusion`.
-head_sha=$(gh api "repos/<owner>/<repo>/pulls/<N>" --jq '.head.sha')
-pending=$(gh api
"repos/<owner>/<repo>/actions/runs?head_sha=${head_sha}&per_page=20" \
- --jq '[.workflow_runs[] | select(.conclusion == "action_required")] |
length')
-if [ "$pending" -gt 0 ]; then
- echo "refuse mark-ready-with-ping: <N> has ${pending} workflow run(s)
awaiting approval at ${head_sha}" >&2
- # Reclassify: this PR is really pending_workflow_approval.
- exit 2
-fi
-
-# 2. Post the ping comment (mark-ready-with-ping template).
-gh pr comment <N> --repo <repo> --body-file /tmp/pr-<N>-mark-ready-with-ping.md
-
-# 3. Apply the label.
-gh pr edit <N> --repo <repo> --add-label "ready for maintainer review"
+gh pr comment <N> --repo <repo> --body-file
/tmp/pr-<N>-request-author-confirmation.md
```
Body template:
-[`comment-templates.md#mark-ready-with-ping`](comment-templates.md).
-
-The body must `@`-mention every reviewer whose unresolved
-thread we believe was addressed (so they get the notification)
-and `@`-mention the PR author once at the top (so the
-contributor sees the rationale). The list of reviewers comes
-from the unresolved-thread reviewers captured during
-classification.
+[`comment-templates.md#request-author-confirmation`](comment-templates.md).
+
+The body `@`-mentions the PR author only, and **must** include
+the canonical marker string `ready for maintainer review
+confirmation` verbatim — that string is what
+[`viewer_confirmation_request_present`](classify-and-act.md#viewer_confirmation_request_present)
+searches for on subsequent sweeps to detect that a confirmation
+request is in flight. Do not paraphrase the marker.
+
+### Why no label, no reviewer mention
+
+The classifier's signal that fired this action is *engagement*
+(post-review commits, in-thread author replies), not
+*resolution*. A post-review commit does not guarantee the
+commit addresses the specific thread; an in-thread reply does
+not guarantee the reply resolves it. Adding the label and
+mentioning reviewers off the engagement signal alone pushes a
+notification framed as a stronger claim than the underlying
+evidence supports. The two-sweep gate — ask the author, wait
+for their reply, then promote — moves the resolution check to
+the only person who reliably knows: the author.
+
+The label is also intentionally absent at this stage so that
+the PR does not enter the maintainer review queue until after
+the author has confirmed. Reviewers are reached via the queue,
+not via direct `@`-mention from the bot — see
+[`rationale.md`](rationale.md) for the longer argument.
+
+### What happens next
+
+- Author replies → next sweep classifies the PR as
+
[`author_confirmation_received`](classify-and-act.md#author_confirmation_received)
+ and proposes
[`mark-ready`](#mark-ready--add-ready-for-maintainer-review-label).
+ The triaging maintainer reads the author's reply alongside
+ the proposal and confirms (or overrides to `skip` / `ping`
+ if the reply is non-affirmative).
+- Author silent → on the cooldown sweep, the PR matches the
+ [stale author-confirm-request
sweep](stale-sweeps.md#sweep-5--stale-author-confirm-request),
+ which proposes a plain `ping` (or `skip`).
+- Author pushes a new commit before replying → the
+
[`viewer_confirmation_request_present`](classify-and-act.md#viewer_confirmation_request_present)
+ precondition fails (the confirmation request now predates the
+ new head commit) and the PR drops back to row 14c / 15 for
+ re-classification against the new state.
-If the label step fails after the comment is already posted,
-**do not retry the comment** — surface the label-add error
-to the maintainer and let them apply the label manually. A
-duplicate ping comment is the worse of the two failure modes.
+### Failure handling
-If the comment step fails (network blip, rate-limit), do not
-proceed to the label — the maintainer would then see a PR
-labelled `ready for maintainer review` with no explanation of
-how it got there.
+A failed `gh pr comment` (network blip, rate-limit) is non-
+destructive — surface the error and let the maintainer retry on
+the next sweep. No partial state to clean up.
### Falling back to plain `ping`
If the post-confirmation drill-in (e.g. the maintainer pulled
-the PR out of the group with `[P]ick`) reveals that the
-threads are *not* actually addressed, the maintainer can
-override the action to `ping`. The override drops the label
-step entirely and posts the regular
+the PR out of the group with `[P]ick`) reveals that the threads
+are *not* actually addressed (the author's engagement was a
+clarifying question or a partial fix), the maintainer can
+override the action to `ping`. The override posts the regular
[`reviewer-ping`](comment-templates.md#reviewer-ping) body
instead. See
[`interaction-loop.md#group-action-override`](interaction-loop.md).
@@ -597,7 +603,7 @@ For every action that includes a comment, post the comment
| `close` | post comment → close → label |
| `flag-suspicious` | post comment → close → label *(per PR in the batch)* |
| `mark-ready` | label only |
-| `mark-ready-with-ping` | post comment → label |
+| `request-author-confirmation` | post comment only (no label) |
| `strip-ready-label` | remove-label only (no comment) |
| `rerun` | rerun (no comment) |
| `rebase` | update-branch (no comment) |
diff --git a/.claude/skills/pr-management-triage/classify-and-act.md
b/.claude/skills/pr-management-triage/classify-and-act.md
index 3fa7040..f96f809 100644
--- a/.claude/skills/pr-management-triage/classify-and-act.md
+++ b/.claude/skills/pr-management-triage/classify-and-act.md
@@ -45,7 +45,7 @@ filter is skipped silently from the main triage flow.
| F2 | Author is a known bot | login is `dependabot`, `dependabot[bot]`,
`renovate[bot]`, `github-actions`, `github-actions[bot]`, or matches `*[bot]` |
| F3 | Draft and not stale | `isDraft == true` and any activity within the
last 14 days. Stale-sweep classifications in
[`stale-sweeps.md`](stale-sweeps.md) may still pull the PR back in. |
| F4 | Already marked ready, no regression | `labels` contains `ready for
maintainer review` AND CI green AND `mergeable != CONFLICTING` AND no
unresolved threads. **Regression bypasses this filter** — any of: CI red, new
conflict, or new unresolved thread whose triggering event (failing check
`startedAt`, conflict detection, thread `createdAt`) is *after* the label-add
timestamp. The typical case is a contributor pushing a rebase or fixup commit
to a ready-for-review PR that re-introduc [...]
-| F5a | Recent collaborator comment (author cooldown) | Most recent comment
from the **union of** general-issue comments (`comments(last:10)`) and
**review-thread comments** (`reviewThreads.nodes.comments`) is by a
`COLLABORATOR`/`MEMBER`/`OWNER`, `createdAt < 72h` ago, AND posted after
`commits(last:1).committedDate`. The review-thread leg is essential — a
maintainer asking a clarifying question in-thread is just as much an active
conversation as a top-level comment, and treating only t [...]
+| F5a | Recent collaborator comment (author cooldown) | Most recent comment
from the **union of** general-issue comments (`comments(last:10)`) and
**review-thread comments** (`reviewThreads.nodes.comments`) is by a
`COLLABORATOR`/`MEMBER`/`OWNER`, `createdAt < 72h` ago, AND posted after
`commits(last:1).committedDate`. The review-thread leg is essential — a
maintainer asking a clarifying question in-thread is just as much an active
conversation as a top-level comment, and treating only t [...]
| F5b | Maintainer-to-maintainer ping unanswered | Most recent collaborator
comment from the **union** described in F5a above `@`-mentions one or more
logins other than the PR author AND none of those mentioned logins have posted
on the PR (general comments **or** review threads) or in `latestReviews` after
that comment. Team mentions (e.g. `@<upstream>-committers`) are conservatively
treated as F5b matches. |
| F6 | Maintainer co-drafted | `isDraft == true` AND any of: (a)
`latestReviews` has a node with `authorAssociation ∈ {OWNER, MEMBER,
COLLABORATOR}` AND `author.login ≠ <viewer>` AND `state ∈ {COMMENTED,
CHANGES_REQUESTED, APPROVED}` AND `submittedAt > commits(last:1).committedDate`
AND review body is non-empty (avoids the "review with only inline thread
comments and an empty top-level body" false positive — those are already
counted by row 14/15 unresolved-thread logic); (b) `comments(l [...]
@@ -85,7 +85,9 @@ Action verbs are defined in [`actions.md`](actions.md).
| 11 | [`ci_failures_only`](#ci_failures_only) AND any failure ∈
`recent_main_failures` | `deterministic_flag` | `rerun`
| K/N CI failures match recent main-branch PRs — likely systemic |
| 12 | [`ci_failures_only`](#ci_failures_only) AND every failed check is a
[static check](#static_check) | `deterministic_flag` | `comment`
| Only static-check failures — needs a code fix, not a rerun |
| 13 | [`ci_failures_only`](#ci_failures_only) AND `failed_count <= 2` AND
`commits_behind <= 50` | `deterministic_flag` | `rerun`
| N CI failure(s) on otherwise clean PR — likely flaky, suggest rerun |
-| 14 | [`unresolved_threads_only`](#unresolved_threads_only) AND
[`unresolved_threads_only_likely_addressed`](#unresolved_threads_only_likely_addressed)
| `deterministic_flag` | `mark-ready-with-ping` | K unresolved thread(s) from
<reviewers> appear addressed (post-review commits / in-thread author replies) —
promote to ready and ping reviewers to confirm |
+| 14a | [`author_confirmation_received`](#author_confirmation_received)
| `author_confirmed_ready` | `mark-ready` |
Author confirmed PR is ready for maintainer review — apply label |
+| 14b | [`pending_author_confirmation`](#pending_author_confirmation)
| `awaiting_author_confirmation` | `skip` |
Awaiting author confirmation requested M days ago |
+| 14c | [`unresolved_threads_only`](#unresolved_threads_only) AND
[`unresolved_threads_only_likely_addressed`](#unresolved_threads_only_likely_addressed)
| `deterministic_flag` | `request-author-confirmation` | K unresolved
thread(s) from <reviewers> show author engagement — ask author to confirm
readiness for maintainer review |
| 15 | [`unresolved_threads_only`](#unresolved_threads_only)
| `deterministic_flag` | `ping` |
K unresolved review thread(s) from <reviewers> — ping author + reviewers |
| 16 | No real CI ran (see [Real-CI guard](#real-ci-guard)) AND `mergeable !=
CONFLICTING` AND author NOT first-time | `deterministic_flag` | `rebase`
| No real CI checks triggered, branch mergeable — rebase to re-trigger |
| 17 | [`has_deterministic_signal`](#has_deterministic_signal) (fallback)
| `deterministic_flag` | `draft` |
Has quality issues — convert to draft with violations comment |
@@ -226,6 +228,61 @@ All of:
Heuristic, conservative on purpose. Rationale:
[`rationale.md#unresolved_threads_only_likely_addressed-heuristic-detail`](rationale.md#unresolved_threads_only_likely_addressed-heuristic-detail).
+### `viewer_confirmation_request_present`
+
+True when the viewer (the authenticated maintainer running the
+skill) has posted a comment on the PR whose body contains the
+literal marker string
+
+> `ready for maintainer review confirmation`
+
+(the canonical marker baked into the `request-author-confirmation`
+template — see
+[`comment-templates.md#request-author-confirmation`](comment-templates.md))
+AND that comment's `createdAt` is **after** the PR's head
+`committedDate`. The post-last-commit check means a fresh
+contributor push invalidates the prior confirmation request
+(new code = new threads possibly opened, our previous question
+no longer applies).
+
+Marker matching is a case-sensitive substring search on the
+comment body. The marker text is fixed in the framework template
+to make this glossary entry deterministic; adopters customising
+the wording of the rest of the body must keep this exact string
+verbatim — the same contract as the
+[`Pull Request quality criteria`](#decision-table) link text
+for `already_triaged` (rows 3–4).
+
+### `pending_author_confirmation`
+
+[`viewer_confirmation_request_present`](#viewer_confirmation_request_present)
+is true AND there is **no** comment by the PR author (issue-level
+in `comments(last:10)` **or** review-thread reply in
+`reviewThreads.nodes.comments`) with `createdAt` greater than
+the confirmation-request comment's `createdAt`.
+
+The PR is mid-cycle: we have asked, the author has not yet
+answered.
+
+### `author_confirmation_received`
+
+[`viewer_confirmation_request_present`](#viewer_confirmation_request_present)
+is true AND there **is** a comment by the PR author (issue-level
+or review-thread reply, as in
+[`pending_author_confirmation`](#pending_author_confirmation))
+with `createdAt` greater than the confirmation-request comment's
+`createdAt`.
+
+The author has responded. The reply text is **not** parsed by
+the bot — the triaging maintainer reads the reply alongside the
+proposal in
+[`interaction-loop.md`](interaction-loop.md) and decides
+whether the response is affirmative. If the maintainer reads
+the reply as a non-affirmative response ("still working on X",
+a follow-up question, etc.) they override the proposal to
+`skip` or `[O]ping`. The bot's only job is to surface the PR
+with the reply visible.
+
### `copilot_review_stale`
The PR has at least one unresolved review thread whose first
diff --git a/.claude/skills/pr-management-triage/comment-templates.md
b/.claude/skills/pr-management-triage/comment-templates.md
index b5adaa2..5cbc236 100644
--- a/.claude/skills/pr-management-triage/comment-templates.md
+++ b/.claude/skills/pr-management-triage/comment-templates.md
@@ -63,7 +63,7 @@ Rules for the footer:
- **Always include it** on every contributor-facing comment the
skill posts — `draft`, `comment-only`, `close`,
- `review-nudge`, `reviewer-ping`, `mark-ready-with-ping`,
+ `review-nudge`, `reviewer-ping`, `request-author-confirmation`,
`stale-draft-close`, `inactive-to-draft`,
`stale-workflow-approval`. The only exception is the
`suspicious-changes` template, which is short, operationally
@@ -277,50 +277,67 @@ in-thread reply.
---
-## Mark ready with ping
+## Request author confirmation
-*(`mark-ready-with-ping` — promote-and-invite-reviewers comment)*
+*(`request-author-confirmation` — ask-author-if-ready comment)*
-Used when the action is `mark-ready-with-ping` (see
-[`actions.md#mark-ready-with-ping`](actions.md)). The PR's
-only outstanding signal is unresolved review threads, the
+Used when the action is `request-author-confirmation` (see
+[`actions.md#request-author-confirmation`](actions.md)). The
+PR's only outstanding signal is unresolved review threads, the
[`unresolved_threads_only_likely_addressed`](classify-and-act.md#unresolved_threads_only_likely_addressed)
-heuristic fired, and we are promoting the PR to
-`ready for maintainer review` while inviting the original
-reviewer(s) to confirm the resolution.
+heuristic fired (engagement signal — not resolution signal),
+and we are asking the author to confirm before the PR is
+promoted into the maintainer review queue.
+
+The body **must** include the literal marker string
+`ready for maintainer review confirmation` verbatim. The
+[`viewer_confirmation_request_present`](classify-and-act.md#viewer_confirmation_request_present)
+precondition searches for that exact text on subsequent sweeps;
+paraphrasing it breaks the gated flow.
```markdown
-@<author> — Your unresolved review thread(s) from <reviewers> appear to have
been addressed (post-review commits and/or in-thread replies on every thread,
with the latest commit pushed after the most recent thread). I've added the
`ready for maintainer review` label so the PR re-enters the maintainer review
queue.
+@<author> — There are <N> unresolved review thread(s) on this PR from
<reviewers>, and you have engaged with each one (post-review commits and/or
in-thread replies). Could you confirm whether you believe the feedback is fully
addressed and the PR is ready for maintainer review confirmation?
+
+If yes, reply here (a short "yes / ready" is fine) and an <PROJECT> maintainer
will pick the PR up from the review queue on the next sweep.
-<reviewers> — could you take another look when you have a chance? If you agree
the feedback was addressed, please mark the threads as resolved so the queue
signal stays accurate. If a thread still needs work, please reply in-line —
@<author> will follow up.
+If you are still working on a thread, please reply with what is outstanding so
the threads stay unresolved on purpose.
<ai_attribution_footer>
```
Notes on the body:
-- **`@<author>` is mentioned once at the top.** They get one
- notification with the rationale (so they understand why the
- label appeared) and a clear ask if a thread comes back open.
-- **Every reviewer with an unresolved thread is `@`-mentioned
- once** in the second paragraph. They get the prompt that
- matters for them — "please re-look and resolve the threads if
- you agree".
-- **No "no rush" line.** Unlike the `draft` / `comment-only`
- templates, this one is announcing forward motion (PR
- promoted), not asking the author to fix something — the
- decompression line would read as out of place.
-- **No "thanks!"** — per the global tone rules, sign-offs are
- noise.
-- **The `<ai_attribution_footer>` still applies** so the
- reviewer knows the promotion is AI-drafted; if the
- heuristic was wrong they have a clear cue to push back
- rather than assuming a maintainer made the call.
-
-If the heuristic was wrong (the reviewer disagrees that the
-threads are addressed), the reviewer can re-open a thread,
-remove the label, or comment back — the PR re-enters the
-unresolved-threads triage path on the next sweep.
+- **`@<author>` is mentioned once at the top.** They are the
+ only person whose input we need in this step.
+- **No `<reviewers>` `@`-mention.** Reviewers are reached
+ through the `ready for maintainer review` queue once the
+ author confirms — pinging them here would push a notification
+ built on the engagement signal alone, which is the failure
+ mode this two-sweep flow exists to avoid.
+- **The marker string `ready for maintainer review
+ confirmation`** appears verbatim in the first paragraph.
+ Do not edit the wording around it in a way that breaks the
+ substring match (case-sensitive).
+- **No "no rush" line.** The author is the only one who can
+ move the PR forward at this step; framing the ask softly
+ matters more than a decompression line.
+- **The `<ai_attribution_footer>` applies** — this is a
+ contributor-facing comment drafted by the bot, and the
+ author should know that.
+
+If the author replies affirmatively, the next sweep classifies
+the PR as
+[`author_confirmation_received`](classify-and-act.md#author_confirmation_received)
+and the triaging maintainer is offered the
+[`mark-ready`](actions.md#mark-ready--add-ready-for-maintainer-review-label)
+action with the author's reply visible alongside the proposal.
+If the author replies with "still working on X" or a clarifying
+question, the maintainer overrides the proposal to `skip` or
+`[O]ping` from the group menu.
+
+If the author is silent past the cooldown, the
+[stale author-confirm-request
sweep](stale-sweeps.md#sweep-5--stale-author-confirm-request)
+takes over.
---
diff --git a/.claude/skills/pr-management-triage/interaction-loop.md
b/.claude/skills/pr-management-triage/interaction-loop.md
index 63704a3..39110f4 100644
--- a/.claude/skills/pr-management-triage/interaction-loop.md
+++ b/.claude/skills/pr-management-triage/interaction-loop.md
@@ -47,17 +47,33 @@ the groups in this fixed order:
8. `(deterministic_flag, ping)` — batchable (unresolved threads
from collaborators).
9. `(stale_review, ping)` — batchable.
-10. `(deterministic_flag, mark-ready-with-ping)` — batchable
- (unresolved threads that the heuristic believes are
- addressed; presented just before the plain mark-ready group
- because both end with the `ready for maintainer review`
- label going on).
-11. `(passing, mark-ready)` — batchable.
-12. `(stale_draft, close)` — batchable but with extra per-PR
+10. `(deterministic_flag, request-author-confirmation)` —
+ batchable (unresolved threads where the engagement
+ heuristic fired; we ask the author whether the PR is
+ ready for maintainer review before any label or reviewer
+ ping is generated — the first leg of the two-sweep gate).
+11. `(author_confirmed_ready, mark-ready)` — batchable
+ (presented just before the plain mark-ready group because
+ both end with the `ready for maintainer review` label
+ going on). The author's reply is shown in the per-PR row
+ so the maintainer can read it before confirming `[A]ll` —
+ a non-affirmative reply is the maintainer's cue to
+ `[P]ick` the PR out and override to `skip` or `ping`.
+12. `(passing, mark-ready)` — batchable.
+13. `(stale_draft, close)` — batchable but with extra per-PR
confirm inside the batch (these are rarely wrong but when
wrong they're very wrong).
-13. `(inactive_open, draft)` — batchable.
-14. `(stale_workflow_approval, draft)` — batchable.
+14. `(inactive_open, draft)` — batchable.
+15. `(stale_workflow_approval, draft)` — batchable.
+
+Skipped (filtered before group presentation) but worth noting
+for completeness: `awaiting_author_confirmation` PRs (we asked
+the author, they have not yet replied, and we are still inside
+the cooldown) classify as `skip` in
+[row 14b](classify-and-act.md#decision-table) and never form
+a group. They reappear once the author replies (→ group 11) or
+when the cooldown lapses (→ Sweep 5 in
+[`stale-sweeps.md`](stale-sweeps.md)).
The ordering is chosen so the maintainer always faces the
riskiest decisions first, while their attention is fresh. The
@@ -211,7 +227,7 @@ safe alternatives:
| `rebase` | `comment`, `skip` |
| `rerun` | `comment`, `skip` |
| `mark-ready` | `skip` |
-| `mark-ready-with-ping` | `ping` (fall back to plain reviewer ping if the
maintainer thinks the heuristic over-reached), `skip` |
+| `request-author-confirmation` | `ping` (skip the author-confirmation step
and post the plain reviewer-ping body directly if the maintainer thinks the
engagement heuristic over-reached), `skip` |
| `ping` | `comment`, `skip` |
| `close` (deterministic_flag) | — (no overrides — use `[E]` to downgrade
individually) |
| `close` (stale_draft) | `draft`, `skip` |
@@ -341,7 +357,7 @@ PRs acted on: 22
- rebased: 4
- reruns triggered: 3
- marked ready: 3
- - marked ready w/ ping: 1
+ - author-confirm requests: 1
- pings posted: 2
PRs skipped: 15 (12 already triaged / inside grace, 2 bot, 1
collaborator)
PRs left pending: 10 (reached [Q] before classifying)
diff --git a/.claude/skills/pr-management-triage/rationale.md
b/.claude/skills/pr-management-triage/rationale.md
index ba1ce21..ff1962d 100644
--- a/.claude/skills/pr-management-triage/rationale.md
+++ b/.claude/skills/pr-management-triage/rationale.md
@@ -324,29 +324,151 @@ The shape of the failures determines the action:
- Otherwise (≤ 2 failures, no conflict, branch up-to-date) →
`rerun`. Most "two-failure" cases on a clean PR are flakes.
-### Rows 14, 15 — `mark-ready-with-ping` vs `ping`
-
-Both fire when the only signal is unresolved review threads.
-Difference is what we believe about the threads:
-
-- `ping` is the safe default — threads are unresolved, may not
- yet be addressed, nudge the author to fix or explain.
-- `mark-ready-with-ping` is the optimistic path — threads are
- unresolved but the author has already engaged (post-review
- commit, or in-thread reply, *and* the latest commit post-dates
- the most recent unresolved thread). The PR is plausibly ready
- for the maintainer to take another look; we promote it with
- the `ready for maintainer review` label *and* ping the
- reviewer to confirm and resolve their threads.
-
-The `unresolved_threads_only_likely_addressed` heuristic is conservative on
-purpose. False-negative degrades to plain `ping` (cheap). False-
-positive would advance a PR that isn't actually ready, which is
-the worse failure mode. The maintainer still confirms; the
-posted comment explicitly invites the reviewer to push back if a
-thread is not actually addressed, so a heuristic false-positive
-self-corrects on the next round-trip rather than silently landing
-the PR.
+### Rows 14a, 14b, 14c, 15 — the two-sweep author-confirmation gate
+
+All four rows handle PRs whose only outstanding signal is
+unresolved review threads. The split is about *what stage of
+the gate* the PR is in:
+
+- **Row 14a — `author_confirmed_ready` → `mark-ready`.** We
+ asked the author on a prior sweep; the author replied.
+ Silently apply the `ready for maintainer review` label. No
+ comment, no reviewer `@`-mention; the label is the queue
+ signal. Reviewers reach the PR through the queue, not via
+ the bot.
+- **Row 14b — `awaiting_author_confirmation` → `skip`.** We
+ asked the author on a prior sweep; the author has not
+ replied yet and the cooldown has not elapsed. Do nothing
+ this sweep. The same row stays matched on subsequent sweeps
+ until either the author replies (row 14a takes over) or
+ Sweep 5 in [`stale-sweeps.md`](stale-sweeps.md) reroutes
+ the PR to plain `ping`.
+- **Row 14c — engagement signal present, no prior request →
+ `request-author-confirmation`.** The engagement heuristic
+ (`unresolved_threads_only_likely_addressed`) fires and we
+ have not asked the author yet. Post an author-only comment
+ asking whether the PR is ready. No label, no reviewer
+ mention.
+- **Row 15 — engagement signal absent → `ping`.** The threads
+ are unresolved and the author has not engaged with them in
+ a way the heuristic recognises. The safe default — nudge
+ the author (or, in the inspection-confirmed variant, the
+ reviewer) without making any claim about resolution.
+
+#### Why a two-sweep gate at all
+
+The earlier single-sweep `mark-ready-with-ping` action
+collapsed the entire flow into one step: heuristic match →
+maintainer confirm → label added + reviewer `@`-mentioned.
+The structural problem was that the engagement heuristic is
+an *engagement* signal, not a *resolution* signal:
+
+- A post-review commit does not guarantee the commit
+ addresses the specific thread (it can touch unrelated
+ files, fix only one of several open threads, or be a
+ follow-up to a different review).
+- An in-thread author reply does not guarantee the reply
+ resolves the thread (it can be a clarifying question, a
+ partial fix, or pushback on the reviewer's framing).
+
+The triaging maintainer's confirmation on the single-sweep
+action was an endorsement of "the heuristic looks plausible",
+not "the threads are definitively resolved". But the outgoing
+comment named the original reviewer(s) and asserted the
+threads "appear to have been addressed" — a stronger claim
+than the underlying evidence supported. False positives
+landed on the original reviewers as push notifications and
+asked them to do the verification work the heuristic could
+not.
+
+The two-sweep gate moves the verification step to the only
+party who reliably knows whether the feedback is addressed:
+the author. Trade-off is one sweep of latency in exchange for
+removing the reviewer-mention path entirely and shifting
+false-positive cost from "another maintainer's notification
+queue" to "one extra round-trip in the author/bot
+conversation".
+
+#### Why the label is silent on row 14a
+
+When row 14a fires, the label is applied with no comment.
+This is the same recipe as plain `mark-ready` (row 20). The
+reasoning:
+
+- The author already knows they have been promoted — they
+ just replied affirmatively to the bot's question on the
+ previous sweep, and the label appearing is the visible
+ confirmation that the answer was accepted.
+- Reviewers reach the PR through the `ready for maintainer
+ review` queue, which is a pull signal rather than a push
+ notification. Adding a `@`-mention here would be the
+ exact noise pattern row 14c was designed to remove,
+ just one sweep later.
+
+If reviewer attention is genuinely needed beyond the queue
+signal (e.g. a thread that the maintainer reading the
+proposal believes the author still needs to address), the
+maintainer reads the author's reply and either `[P]`-picks
+the PR out for an override to plain `ping`, or skips. The
+in-the-loop maintainer is more informed than the bot's
+heuristic and can call it correctly.
+
+#### Why row 14c posts a comment but no label
+
+The asymmetry with row 14a is deliberate. At row 14c we have
+*engagement* but not *resolution* — promoting the PR to the
+maintainer queue at this stage would re-introduce the
+single-sweep failure mode (label on a PR that may not
+actually be ready, surfaced to other maintainers who then
+discover the gap). Posting the question without the label
+keeps the PR in the author's lane until the author signals
+otherwise.
+
+The cost is one sweep of latency in the happy path. The
+benefit is that the label, when it is eventually added, is
+gated on the author's own statement that the PR is ready —
+which is the strongest cheap signal available without
+forcing the bot to read the diff or parse the reviewer's
+comments.
+
+#### Why row 14b is `skip`, not a fresh comment
+
+Once we have asked the author and we are still inside the
+cooldown, posting a second comment would be the bot
+nagging. The author has the question; they know what is
+expected; the silence is informative. Sweep 5 in
+[`stale-sweeps.md`](stale-sweeps.md) handles the escalation
+deterministically once 7 days have elapsed, so there is no
+need for the active-triage path to act in the interim.
+
+#### What happens to the `<author-reply>` body
+
+Row 14a's precondition is *any* author comment after our
+request, not "an affirmative author comment". The bot does
+not parse the reply text — natural-language affirmation
+detection is brittle and would re-introduce a heuristic
+failure mode in the second leg of the flow. Instead, the
+triaging maintainer reads the author's reply alongside the
+proposal in the
+[group screen](interaction-loop.md#group-ordering). If the
+reply is affirmative the maintainer accepts the proposal in
+one keystroke. If the reply is "actually I'm still working
+on X" the maintainer overrides to `skip` (or `[O]`-overrides
+to plain `ping` to re-surface the unresolved-thread
+conversation). The bot's job is to put the PR and the reply
+in front of a person who can read.
+
+#### `unresolved_threads_only_likely_addressed` stays conservative
+
+The heuristic still fires only when the latest commit
+post-dates the most recent unresolved thread AND every
+unresolved thread has either an in-thread author reply or a
+post-thread commit. The change is in what we *do* on
+matches, not in the matching condition. Tightening the
+heuristic further (e.g. requiring per-thread commit
+attribution) would gain little once the author-confirmation
+gate is in place: the gate already filters the
+false-positives that would otherwise reach the reviewer.
### Row 16 — no real CI ran, mergeable
diff --git a/.claude/skills/pr-management-triage/stale-sweeps.md
b/.claude/skills/pr-management-triage/stale-sweeps.md
index c41c0f0..666de62 100644
--- a/.claude/skills/pr-management-triage/stale-sweeps.md
+++ b/.claude/skills/pr-management-triage/stale-sweeps.md
@@ -5,7 +5,7 @@
The stale-sweep phase runs after the interactive triage is
done (Step 5 in [`SKILL.md`](SKILL.md)). Its job is to clear
-three categories of PRs that have gone silent:
+four categories of PRs that have gone silent:
1. **Stale drafts** — drafts that haven't moved in weeks; either
triaged and ghosted, or never triaged and drifted.
@@ -14,6 +14,11 @@ three categories of PRs that have gone silent:
3. **Stale workflow-approval PRs** — first-time-contributor
PRs awaiting workflow approval that have sat for over 4
weeks without the author pushing new commits.
+4. **Stale author-confirm-requests** — PRs where the bot asked
+ the author to confirm readiness for maintainer review (the
+ first leg of the [row 14c](classify-and-act.md#decision-table)
+ two-sweep flow) and the author has been silent past the
+ cooldown.
Each category has deterministic trigger criteria, a fixed
action, and a canned comment. They are surfaced through the
@@ -245,6 +250,80 @@ since maintainer comment, no author reply, branch has
---
+## Sweep 5 — Stale author-confirm-request
+
+When a PR has a pending author-confirmation request from a
+prior sweep (the first leg of the
+[row 14c](classify-and-act.md#decision-table) two-sweep flow)
+and the author has not replied within the cooldown, this sweep
+proposes a fallback action so the PR does not sit indefinitely
+in the "asked, awaiting reply" limbo state.
+
+### Trigger
+
+-
[`viewer_confirmation_request_present`](classify-and-act.md#viewer_confirmation_request_present)
+ is true (we posted the request, the head SHA has not advanced
+ since).
+-
[`pending_author_confirmation`](classify-and-act.md#pending_author_confirmation)
+ is true (no author reply after our request).
+- `<now> - confirmation_request_at >= 7 days`, where
+ `confirmation_request_at` is the `createdAt` of the viewer's
+ most recent comment carrying the
+ `ready for maintainer review confirmation` marker.
+
+The same 7-day window used by
+[Sweep 4](#sweep-4--stale-ready-for-review-label) applies here
+— the symmetry is intentional: in both cases the author has
+gone silent after we asked them for input on a PR they own.
+
+### Action
+
+`ping` with the
+[`reviewer-ping` author-primary body](comment-templates.md#reviewer-ping)
+(unresolved threads from collaborators). The new ping makes
+the unresolved-thread state itself the topic again — the
+confirmation request was a softer ask that did not work, so
+the next sweep escalates to the normal unresolved-thread nudge.
+
+Do **not** strip the prior confirmation-request comment — it
+remains as part of the PR history, and a contributor returning
+later can see both pings in order.
+
+**Reason string.** *"Author confirmation requested N days ago,
+no reply — escalating to plain reviewer ping"*.
+
+### Group behaviour
+
+Batchable with simple `[A]ll` — the action is a single
+non-destructive comment, recoverable by the maintainer if it
+fires on the wrong PR.
+
+### Why not `close` or `draft`?
+
+`close` would punish a contributor whose only "fault" is
+missing a confirmation question; `draft` would lose the PR's
+review-ready posture even though every other signal
+(CI green, no conflicts) is healthy. Plain `ping` is the
+minimum escalation that re-surfaces the PR to the original
+reviewer pool without prejudging where the responsibility
+lies.
+
+### Override
+
+If the maintainer reading the proposal sees the unresolved
+threads are obviously addressed (e.g. the author engaged
+substantively in-thread but never replied to our
+confirmation), they can `[O]`-override the group to
+`mark-ready` — that path skips the ping entirely and just
+applies the label, treating the author's earlier in-thread
+engagement as the implicit confirmation. The override is
+deliberately not the default because a programmatic test for
+"engagement was substantive" would re-introduce exactly the
+false-positive risk the two-sweep gate was designed to
+eliminate.
+
+---
+
## Order of sweeps
1. Sweep 1a (triaged drafts, 7d)
@@ -253,6 +332,7 @@ since maintainer comment, no author reply, branch has
4. Sweep 3 (stale WF approval, 4w)
5. Sweep 4a (stale ready-label, healthy, 7d) → strip
6. Sweep 4b (stale ready-label, rotted, 7d) → close
+7. Sweep 5 (stale author-confirm-request, 7d) → ping
Run 1a before 1b so a draft that's both "triaged 7d ago" and
"never-triaged 2w ago" (the triage comment is recent but the
@@ -264,6 +344,13 @@ the default search excluded), so there is no overlap with
the
earlier sweeps. 4a runs before 4b so the cheap label-tidying
batch lands before the per-PR-confirm `close` group.
+Sweep 5 runs last because its candidate set is also disjoint
+from the earlier sweeps — PRs holding a confirmation-request
+comment do not carry the `ready for maintainer review` label
+yet (the label is the second-leg outcome), and their author
+silence is more recent than the 4-week thresholds in Sweeps 2
+and 3.
+
---
## What the sweeps do NOT do
diff --git a/projects/_template/pr-management-triage-comment-templates.md
b/projects/_template/pr-management-triage-comment-templates.md
index 798e5e4..7ec8cbc 100644
--- a/projects/_template/pr-management-triage-comment-templates.md
+++ b/projects/_template/pr-management-triage-comment-templates.md
@@ -14,7 +14,7 @@
- [`review-nudge` (reviewer-re-review)](#review-nudge-reviewer-re-review)
- [`reviewer-ping` (author-primary)](#reviewer-ping-author-primary)
- [`reviewer-ping` (reviewer-re-review)](#reviewer-ping-reviewer-re-review)
- - [`mark-ready-with-ping`](#mark-ready-with-ping)
+ - [`request-author-confirmation`](#request-author-confirmation)
- [`stale-draft-close` (triaged)](#stale-draft-close-triaged)
- [`stale-draft-close` (untriaged)](#stale-draft-close-untriaged)
- [`inactive-to-draft`](#inactive-to-draft)
@@ -160,12 +160,21 @@ This is **not** a rejection — you're welcome to open a
new PR addressing the i
<ai_attribution_footer>
```
-### `mark-ready-with-ping`
+### `request-author-confirmation`
+
+The body **must** include the literal marker string
+`ready for maintainer review confirmation` verbatim — the
+framework's
+[`viewer_confirmation_request_present`](../../.claude/skills/pr-management-triage/classify-and-act.md#viewer_confirmation_request_present)
+precondition searches for that exact text. Do not paraphrase
+that string when adapting the rest of the body.
```markdown
-@<author> — Your unresolved review thread(s) from <reviewers> appear to have
been addressed (post-review commits and/or in-thread replies on every thread,
with the latest commit pushed after the most recent thread). I've added the
`ready for maintainer review` label so the PR re-enters the maintainer review
queue.
+@<author> — There are <N> unresolved review thread(s) on this PR, and you have
engaged with each one (post-review commits and/or in-thread replies). Could you
confirm whether you believe the feedback is fully addressed and the PR is ready
for maintainer review confirmation?
+
+If yes, reply here (a short "yes / ready" is fine) and an Apache Airflow
maintainer will pick the PR up from the review queue on the next sweep.
-<reviewers> — could you take another look when you have a chance? If you agree
the feedback was addressed, please mark the threads as resolved so the queue
signal stays accurate. If a thread still needs work, please reply in-line —
@<author> will follow up.
+If you are still working on a thread, please reply with what is outstanding so
the threads stay unresolved on purpose.
<ai_attribution_footer>
```
