diff --git a/skills/pr-management-quick-merge/candidate-rules.md b/skills/pr-management-quick-merge/candidate-rules.md index e2c3eb129..15cf7b651 100644 --- a/skills/pr-management-quick-merge/candidate-rules.md +++ b/skills/pr-management-quick-merge/candidate-rules.md @@ -125,8 +125,8 @@ G2/G3 already established every check is green and done, so a `blocked` state here is review-required in the normal case. Where an adopter's branch protection makes a *non*-CI context required, confirm with `gh pr view --json reviewDecision` — `REVIEW_REQUIRED` ⇒ a missing approval -is the blocker (route to the approval bucket); any other decision ⇒ drop, the -block is not something an approval clears. +is the blocker (route to the approval bucket); any other decision ⇒ drop as +`gate:G5-blocked`, the block is not something an approval clears. --- @@ -235,14 +235,20 @@ Every screened-out PR carries exactly one drop reason, surfaced in the | `path-denied` | a changed file matched `deny_globs` (consequential area) | | `path-unmatched` | a changed file matched no allow glob (unknown area) | | `gate:G5-conflict` | Stage-3 live re-poll: genuine merge conflict (`mergeable == false` / `dirty`) | +| `gate:G5-blocked` | Stage-3 live re-poll: merges cleanly but `blocked` by a required context an approval does not clear (`reviewDecision != REVIEW_REQUIRED`) | | `gate:G5-unknown` | Stage-3 live re-poll: mergeability still uncomputed after the direct call — dropped this run, qualifies next | +Note that the Stage-3 codes are deliberately suffixed. A bare `gate:G5` means +"failed **Stage-1** gate G5" — the batch-level *obviously conflicting* cull — so +reusing it for a Stage-3 outcome would give one code two meanings. + `gate:*` and `gate:G5-unknown` drops are reported as a single count each (the maintainer rarely cares which one a non-ready-looking PR hit); `too-large`, -`path-denied`, `path-unmatched`, and `gate:G5-conflict` are reported with PR -numbers, because those are the "so-close" PRs a maintainer may want to glance at -or hand to `pr-management-code-review`. **Note:** a `BLOCKED` live state is -**not** a drop reason — it is the *approval* bucket (see +`path-denied`, `path-unmatched`, `gate:G5-conflict`, and `gate:G5-blocked` are +reported with PR numbers, because those are the "so-close" PRs a maintainer may +want to glance at or hand to `pr-management-code-review`. **Note:** a `BLOCKED` +live state with `reviewDecision == REVIEW_REQUIRED` is **not** a drop reason — +it is the *approval* bucket (see [Stage 3](#stage-3--live-merge-readiness)), the skill's primary output. --- diff --git a/tools/skill-evals/evals/pr-management-quick-merge/README.md b/tools/skill-evals/evals/pr-management-quick-merge/README.md index 2e18ad31e..3e4016b32 100644 --- a/tools/skill-evals/evals/pr-management-quick-merge/README.md +++ b/tools/skill-evals/evals/pr-management-quick-merge/README.md @@ -5,13 +5,13 @@ Behavioral evals for the `pr-management-quick-merge` skill. -## Suites (20 cases total) +## Suites (21 cases total) | Suite | Step | Cases | What it covers | |---|---|---|---| | stage-1-quality-gate | Steps 1–2 (Stage 1 quality gates) | 8 | G2 (CI failure), G2 (real-CI not ran / bot-only SUCCESS), G3 (pending check), G4 (action_required workflow), G6 (unresolved collaborator thread), G7 (non-stale changes-requested), all-gates-pass, injection attempt ignored | | stage-2-triviality | Step 2 (Stage 2 triviality + tier) | 8 | Tier A (docs only), Tier B (tests only), Tier B (mixed docs+tests), too-large churn, too-many files, path-denied (.github/**), path-unmatched (unknown source path), deny overrides allow | -| stage-3-merge-readiness | Step 2 (Stage 3 live re-poll) | 4 | ready (clean state), needs-approval (blocked + REVIEW_REQUIRED), drop (conflict / dirty), drop (unknown / still computing) | +| stage-3-merge-readiness | Step 2 (Stage 3 live re-poll) | 5 | ready (clean state), needs-approval (blocked + REVIEW_REQUIRED), drop (blocked by a non-review requirement), drop (conflict / dirty), drop (unknown / still computing) | ## Run @@ -42,6 +42,7 @@ uv run --project tools/skill-evals skill-eval \ dropped as `path-denied`. - `stage-3-merge-readiness` exercises the live re-poll classification. The `blocked + REVIEW_REQUIRED` row (`case-2`) is the skill's primary case: it - routes to the `[A]pprove` action rather than dropping. `blocked` alone - (without `REVIEW_REQUIRED`) would be `gate:G5-conflict` but that case is - covered implicitly by `case-3` (`dirty`/`mergeable=false`). + routes to the `[A]pprove` action rather than dropping. `blocked` without + `REVIEW_REQUIRED` is explicitly covered by `case-5` and drops as + `gate:G5-blocked`; `case-3` reserves `gate:G5-conflict` for + `dirty`/`mergeable=false`. diff --git a/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/case-5-blocked-non-review-drop/expected.json b/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/case-5-blocked-non-review-drop/expected.json new file mode 100644 index 000000000..d0454d24e --- /dev/null +++ b/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/case-5-blocked-non-review-drop/expected.json @@ -0,0 +1,5 @@ +{ + "bucket": "drop", + "drop_reason": "gate:G5-blocked", + "reason": "mergeable=true, mergeable_state=blocked, reviewDecision=APPROVED — the branch merges cleanly but the remaining block is not cleared by another approval." +} diff --git a/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/case-5-blocked-non-review-drop/report.md b/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/case-5-blocked-non-review-drop/report.md new file mode 100644 index 000000000..757596225 --- /dev/null +++ b/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/case-5-blocked-non-review-drop/report.md @@ -0,0 +1,9 @@ + + +PR #69001 +Live GET /repos/apache/airflow/pulls/69001: + mergeable: true + mergeable_state: blocked + reviewDecision: APPROVED + approvals: 1 diff --git a/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/system-prompt.md b/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/system-prompt.md index a73c01aec..fe0222f2e 100644 --- a/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/system-prompt.md +++ b/tools/skill-evals/evals/pr-management-quick-merge/stage-3-merge-readiness/fixtures/system-prompt.md @@ -18,7 +18,7 @@ Classify each candidate by the live `(mergeable, mergeable_state)` pair: | `mergeable == true`, `mergeable_state ∈ {clean, has_hooks}` | **ready** — surface with the merge command | | `mergeable == true`, `mergeable_state ∈ {unstable, behind}` | **ready** — note the state; `unstable` means a non-required check is still running but every required check is green; `behind` is a stale-but-clean branch GitHub will fast-forward | | `mergeable == true`, `mergeable_state == blocked` **and** `reviewDecision == REVIEW_REQUIRED` | **needs-approval** — the branch merges cleanly but a required committer review is missing; route to the `[A]pprove` action | -| `mergeable == true`, `mergeable_state == blocked` **and** `reviewDecision != REVIEW_REQUIRED` | **drop** — the block is not cleared by an approval; reason `gate:G5-conflict` (a non-approval required check is the blocker) | +| `mergeable == true`, `mergeable_state == blocked` **and** `reviewDecision != REVIEW_REQUIRED` | **drop** — the block is not cleared by an approval; reason `gate:G5-blocked` (a non-approval required context is the blocker) | | `mergeable == false` **or** `mergeable_state == dirty` | **drop** — genuine merge conflict; reason `gate:G5-conflict` | | `mergeable == null` **or** `mergeable_state == unknown` | **drop** — mergeability still computing; reason `gate:G5-unknown`; conservative per Golden rule 4. It will settle on the next run. | @@ -34,8 +34,8 @@ Return ONLY valid JSON: } ``` -- `drop_reason` is one of `gate:G5-conflict`, `gate:G5-unknown`, - or `null` when `bucket != "drop"`. +- `drop_reason` is one of `gate:G5-blocked`, `gate:G5-conflict`, + `gate:G5-unknown`, or `null` when `bucket != "drop"`. - `reason` is one concise sentence naming the classification outcome. For a `gate:G5-conflict` drop on a genuine merge conflict (`mergeable=false` / `mergeable_state=dirty`), the sentence must also name