From 8aa11497603e1718c29ad53e66e2d875c541feb4 Mon Sep 17 00:00:00 2001 From: Ryan Brodkin <236564+brodkin@users.noreply.github.com> Date: Sat, 9 May 2026 15:53:46 -0700 Subject: [PATCH 1/2] fix(pepper): mark PRs as pepper-cooking during review [DEV-237] Pepper was adding the new outcome label on a re-review but leaving the prior one in place, so PRs ended up with both pepper-approved and pepper-changes-requested simultaneously and the label-based filters lost meaning. Move label lifecycle ownership from the LLM into the workflow: - A new pre-review step (review mode only) snapshots current PR labels, clears any prior outcome labels actually present, and applies pepper-cooking as a "currently being reviewed" marker. Removes are gated on presence so a fresh repo's first review doesn't error on gh pr edit --remove-label for a label not in the repo yet. - Pepper's final action swaps pepper-cooking for the chosen outcome in a single gh pr edit call (--add-label "" --remove-label "pepper-cooking"). The four label-application points in the prompt (auto-fail, both intent-verification halts, and the central section) all use the swap form. - A new always-on cleanup step removes pepper-cooking regardless of how the job ended (Pepper success, Pepper failure, prompt-build error, AWS-creds error, runner timeout, manual cancellation), so the marker only ever appears on a PR that is actively being reviewed. Failed runs are surfaced via the workflow's run status, not via a stale label. Net behavior: re-reviews end with exactly one outcome label on the PR; in-flight reviews carry pepper-cooking; nothing else combines. Refs: DEV-237 --- .github/workflows/pepper-pr-review.yml | 44 ++++++++++++++++++++++++++ prompts/pr-review-default.md | 14 +++++--- 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/.github/workflows/pepper-pr-review.yml b/.github/workflows/pepper-pr-review.yml index 69e7848..2b0299b 100644 --- a/.github/workflows/pepper-pr-review.yml +++ b/.github/workflows/pepper-pr-review.yml @@ -130,6 +130,31 @@ jobs: echo "Resolved mode: ${MODE}" echo "mode=${MODE}" >> "${GITHUB_OUTPUT}" + # Mark this PR as under active Pepper review and clear any prior + # outcome labels in a single API call. The remove side is gated on + # labels actually being on the PR — `gh pr edit --remove-label` + # errors if a label doesn't exist in the repo at all (cold-start + # case for a fresh repo's first review). Pepper's final action + # swaps `pepper-cooking` for the chosen outcome label, and the + # always-on cleanup at the end of the job removes the marker if + # Pepper never finished. Net: `pepper-cooking` only appears on a + # PR that is currently being reviewed, never as a stale leftover. + - name: Mark PR as cooking (review mode only) + if: steps.mode.outputs.mode == 'review' + env: + GH_TOKEN: ${{ steps.app-token.outputs.token }} + PR_NUMBER: ${{ github.event.pull_request.number || github.event.issue.number }} + run: | + set -euo pipefail + CURRENT=$(gh pr view "${PR_NUMBER}" --json labels --jq '[.labels[].name] | join(",")') + REMOVES=() + for label in pepper-approved pepper-changes-requested pepper-needs-review; do + case ",${CURRENT}," in + *,"${label}",*) REMOVES+=(--remove-label "${label}") ;; + esac + done + gh pr edit "${PR_NUMBER}" --add-label "pepper-cooking" "${REMOVES[@]}" + # `issue_comment` events check out the default branch by default. For on-demand # mode (Pepper making edits the user asked for) we need to be on the PR's head # branch so commits land on the right ref. Fork PRs aren't supported in @@ -350,3 +375,22 @@ jobs: --model ${{ inputs.model }} --allowedTools "${{ steps.tools.outputs.allowed }}" ${{ steps.mcp.outputs.path != '' && format('--mcp-config {0}', steps.mcp.outputs.path) || '' }} + + # Always-on cleanup — `pepper-cooking` is purely a "currently being + # reviewed" marker, so it must come off no matter how this job ended + # (Pepper success, Pepper failure, prompt-build error, AWS-creds + # error, runner timeout, manual cancellation). Failed runs are + # surfaced via the workflow's run status; we don't need a stale + # label to encode the same signal. The check-before-remove handles + # the corner case where the cooking step itself failed and the + # label is not in the repo (would otherwise error on --remove-label). + - name: Clear pepper-cooking marker + if: always() && steps.mode.outputs.mode == 'review' + env: + GH_TOKEN: ${{ steps.app-token.outputs.token }} + PR_NUMBER: ${{ github.event.pull_request.number || github.event.issue.number }} + run: | + set -uo pipefail + if gh pr view "${PR_NUMBER}" --json labels --jq '.labels[].name' 2>/dev/null | grep -Fxq "pepper-cooking"; then + gh pr edit "${PR_NUMBER}" --remove-label "pepper-cooking" + fi diff --git a/prompts/pr-review-default.md b/prompts/pr-review-default.md index 5001ad4..98fa3ab 100644 --- a/prompts/pr-review-default.md +++ b/prompts/pr-review-default.md @@ -66,7 +66,7 @@ Patterns: **Action when any pattern matches:** 1. `gh pr review {{PR_NUMBER}} --request-changes --body ''` -2. `gh pr edit {{PR_NUMBER}} --add-label "pepper-changes-requested"` +2. `gh pr edit {{PR_NUMBER}} --add-label "pepper-changes-requested" --remove-label "pepper-cooking"` Then end your turn. @@ -88,7 +88,7 @@ The Linear MCP allowlist is read-only by design. If a tool is not allowed, the s **Halt the review if a parseable ID was found but the fetch failed** (MCP error or `null`, `gh issue view` non-zero, issue inaccessible). The PR's intent depends on an issue you cannot read; a partial review is worse than escalating clearly. Do not classify, do not keep inspecting files. Run exactly: 1. `gh pr review {{PR_NUMBER}} --comment --body '<5–8 lines: which issue ID was parsed, which tracker, the exact failure (e.g., "mcp__linear__get_issue returned null for DEV-212" / "gh issue view #14 exited 1: not found"), and that escalation is required because intent cannot be verified>'` -2. `gh pr edit {{PR_NUMBER}} --add-label "pepper-needs-review"` +2. `gh pr edit {{PR_NUMBER}} --add-label "pepper-needs-review" --remove-label "pepper-cooking"` 3. Assign the default reviewer per the recipe in ``. Then end your turn. @@ -96,7 +96,7 @@ Then end your turn. **Block the PR if no Linear or GitHub Issue ID is found and the PR is not a chore.** An unsupported-tracker reference (non-Linear Jira key, GitLab/Asana/internal-tracker URL) does not satisfy the policy — treat it as if no ID was found. Run exactly: 1. `gh pr review {{PR_NUMBER}} --request-changes --body ''` -2. `gh pr edit {{PR_NUMBER}} --add-label "pepper-changes-requested"` +2. `gh pr edit {{PR_NUMBER}} --add-label "pepper-changes-requested" --remove-label "pepper-cooking"` Then end your turn. Do not classify, do not continue review. @@ -253,7 +253,13 @@ Action: -Apply exactly one outcome label: `pepper-approved`, `pepper-changes-requested`, or `pepper-needs-review`. Apply `area:*` labels matching modified paths only if the repo has an existing area-labeling convention you can identify from past PRs — don't invent a vocabulary. +The workflow has applied `pepper-cooking` to mark this PR as under active review and cleared any prior outcome labels. Your final action is to swap `pepper-cooking` for the outcome label in a single `gh pr edit` call: + +- `gh pr edit {{PR_NUMBER}} --add-label "pepper-approved" --remove-label "pepper-cooking"` (when approving) +- `gh pr edit {{PR_NUMBER}} --add-label "pepper-changes-requested" --remove-label "pepper-cooking"` (when requesting changes) +- `gh pr edit {{PR_NUMBER}} --add-label "pepper-needs-review" --remove-label "pepper-cooking"` (when escalating to a human) + +Apply `area:*` labels matching modified paths only if the repo has an existing area-labeling convention you can identify from past PRs — don't invent a vocabulary. From ecdb8fef335193a9f34dd742c853dfcea08bdebc Mon Sep 17 00:00:00 2001 From: Ryan Brodkin <236564+brodkin@users.noreply.github.com> Date: Sat, 9 May 2026 16:41:32 -0700 Subject: [PATCH 2/2] fix(pepper): build cooking-step argv as single non-empty array [DEV-237] Pepper auto-review flagged an empty-array footgun: under `set -euo pipefail` on bash 3.2 (macOS default) or bash 4.3 and earlier, `"${REMOVES[@]}"` on an empty array errors as "unbound variable" before gh ever runs. GitHub runners on bash 5.x are fine, but the form is brittle. Build a single ARGS array that always carries the --add-label pair so the expansion is never empty. Refs: DEV-237 --- .github/workflows/pepper-pr-review.yml | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/.github/workflows/pepper-pr-review.yml b/.github/workflows/pepper-pr-review.yml index 2b0299b..7e3dc8a 100644 --- a/.github/workflows/pepper-pr-review.yml +++ b/.github/workflows/pepper-pr-review.yml @@ -146,14 +146,18 @@ jobs: PR_NUMBER: ${{ github.event.pull_request.number || github.event.issue.number }} run: | set -euo pipefail + # Build the gh-pr-edit argv as a single array so we never expand + # an empty array under `set -u` (bash 3.2 / 4.3 footgun: empty + # `"${arr[@]}"` errors as unbound variable). ARGS always carries + # the --add-label pair, so the expansion is always safe. CURRENT=$(gh pr view "${PR_NUMBER}" --json labels --jq '[.labels[].name] | join(",")') - REMOVES=() + ARGS=(--add-label "pepper-cooking") for label in pepper-approved pepper-changes-requested pepper-needs-review; do case ",${CURRENT}," in - *,"${label}",*) REMOVES+=(--remove-label "${label}") ;; + *,"${label}",*) ARGS+=(--remove-label "${label}") ;; esac done - gh pr edit "${PR_NUMBER}" --add-label "pepper-cooking" "${REMOVES[@]}" + gh pr edit "${PR_NUMBER}" "${ARGS[@]}" # `issue_comment` events check out the default branch by default. For on-demand # mode (Pepper making edits the user asked for) we need to be on the PR's head