From 0a2d11973a6936876e825d6bf439aa747bfd03f6 Mon Sep 17 00:00:00 2001 From: "wave-av-bot[bot]" Date: Sun, 23 Aug 2026 18:39:00 -0400 Subject: [PATCH 1/6] =?UTF-8?q?ci:=20adopt=20the=20inline=20pr-agent=20lan?= =?UTF-8?q?e=20=E2=80=94=20a=20public=20repo=20cannot=20call=20a=20private?= =?UTF-8?q?=20reusable=20workflow?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This repo's pr-agent.yml calls `wave-av/wave-foundation/.github/workflows/reusable-pr-agent.yml`, and wave-foundation is PRIVATE. GitHub does not permit a PUBLIC repository to call a reusable workflow from a private one, so the `uses:` never resolves: the run dies before any job is created — conclusion: failure, total_count: 0, no log, and no check run on the head sha to read. Every PR here has carried a red check that reports nothing, and external contributors see it. Measured across the org 2026-08-22: 7 public repos / 176 runs / 100% failure; 9 private repos / zero failures — a clean 16/16 split on visibility alone. Three competing hypotheses (missing OPENAI_KEY, dead pinned ref, @main vs a pinned sha) were each tested and refuted. THE FIX already existed and was never adopted: wave-foundation-public/.github/workflows/pr-agent.yml is an INLINE copy of the same lane with no reference to the private repo. This adopts it verbatim. PROVEN BEFORE FANNING OUT. wave-certify#44 took this exact change first and its pr_agent run returned SUCCESS on the pull_request event — a job with a real log, where the broken form produced no job at all. 27 repos were not changed on hope. Two prerequisites named in wave-pen#388 are cleared as of wave-foundation-public#71: the shared concurrency key that let any bot comment cancel a live review ~10s in (wave-pen#386) now keys on github.event_name, and the lane carries step-level timeouts. The job id stays `pr_agent`, so the check-run context is unchanged and no branch protection rule needs touching. Refs wave-pen#388 Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/pr-agent.yml | 172 +++++++++++++++++++++++++++++++-- 1 file changed, 166 insertions(+), 6 deletions(-) diff --git a/.github/workflows/pr-agent.yml b/.github/workflows/pr-agent.yml index 5d4ce23..abb7b31 100644 --- a/.github/workflows/pr-agent.yml +++ b/.github/workflows/pr-agent.yml @@ -1,5 +1,21 @@ -# pr-agent (OSS) caller — adopts the wave-foundation reusable lane. -# Engine: OSS Qodo Merge on our OPENAI_KEY (trial-independent). SSOT pinned by SHA. +# pr-agent (OSS) — INLINE form for public repos (no references to private wave-foundation). +# Mirrors wave-foundation/.github/workflows/reusable-pr-agent.yml. Engine: OSS Qodo Merge on OPENAI_KEY. +# +# WHY AN INLINE COPY EXISTS AT ALL, restated because 28 repos got this wrong. +# GitHub does not let a PUBLIC repository call a reusable workflow from a +# PRIVATE one. wave-foundation is private, so a public repo pointing at +# `wave-av/wave-foundation/.github/workflows/reusable-pr-agent.yml@…` never +# resolves: the run dies before any job is created — `conclusion: failure`, +# zero jobs, no log, and no check run on the head sha to read. Measured +# 2026-08-22 across 16 repos, a clean 16/16 split on visibility alone: +# 7 public repos, 176 runs, 100% failure; 9 private repos, zero failures. +# THIS FILE is what a public repo adopts instead. See wave-pen#388. +# +# IT IS A MIRROR, AND MIRRORS DRIFT. As of 2026-08-22 this copy had fallen a +# long way behind the reusable lane it claims to mirror — no retry, no verdict +# step, no continue-on-error, so an upstream 429 turned the check RED on an +# ADVISORY reviewer. Everything below re-syncs it and carries today's two fixes. +# When the reusable lane changes, this changes with it or it lies. name: pr-agent (OSS) on: pull_request: @@ -12,12 +28,156 @@ permissions: pull-requests: write contents: read +# The group key carries the EVENT NAME (wave-pen#386). +# +# Without it, `pull_request` and `issue_comment` share one group per PR number, +# so any comment on the PR cancels a review already in flight. Measured on +# wave-pen over 30 runs: 15 skipped, 15 cancelled, ZERO reviews ever completed. +# 14 of the 15 cancelled died at ~12 SECONDS, inside GitHub's docker build, +# each followed 10-12s later by an `issue_comment` run that entered this group, +# cancelled it, then skipped ITSELF on the job `if:` below because the comment +# did not start with `/`. A run that will not review took the lane from the run +# that would have: +# +# 17:43:14 cancelled pull_request <- review starts +# 17:43:26 skipped issue_comment <- +12s, kills it, then skips itself +# +# Keying on event name preserves what this block is FOR — two pushes still share +# `pr-agent-pull_request-` and still supersede each other — while giving a +# slash-command comment its own lane, which is correct: an explicit `/review` is +# a new request, not a supersession of a push review. Proven live on wave-pen: +# the first successful run in that repo's history, with the comment run skipping +# harmlessly beside it. concurrency: - group: pr-agent-${{ github.event.pull_request.number || github.event.issue.number || github.ref }} + group: pr-agent-${{ github.event_name }}-${{ github.event.pull_request.number || github.event.issue.number || github.ref }} cancel-in-progress: true jobs: pr_agent: - uses: wave-av/wave-foundation/.github/workflows/reusable-pr-agent.yml@150ffae24f63e207ab81430fa64cb2b1e5c01546 # post-#1191 - secrets: - OPENAI_KEY: ${{ secrets.OPENAI_KEY }} + timeout-minutes: 15 + # Slash commands: PR-only + trusted members (cost-abuse guard). Forks skipped (no secrets there). + if: >- + ${{ + (github.event_name == 'issue_comment' + && github.event.issue.pull_request + && startsWith(github.event.comment.body, '/') + && contains(fromJSON('["MEMBER","OWNER","COLLABORATOR"]'), github.event.comment.author_association)) + || (github.event_name == 'pull_request' + && !contains(github.event.pull_request.user.login, '[bot]') + && github.event.pull_request.draft == false + && github.event.pull_request.head.repo.fork == false) + }} + runs-on: ubuntu-latest + steps: + # Stamped so the verdict step can tell a TIMED-OUT attempt from a fast + # upstream error. Both arrive as outcome == 'failure' and GitHub exposes no + # step-level "timed_out", so elapsed time is the only discriminator there is. + - name: stamp attempt start + run: echo "AGENT_START=$(date +%s)" >> "$GITHUB_ENV" + + - name: PR-Agent (OSS qodo-merge) + id: agent + # STEP-level budget, under the job's 15 (wave-pen#386). + # + # Without it one hung attempt consumes the WHOLE job budget: the job hits + # `timeout-minutes: 15`, which GitHub renders as `cancelled` — the same + # word a concurrency supersede produces, so a 12-second supersede and a + # 15-minute hang are indistinguishable in a run list. And the retry below + # becomes unreachable on exactly the path it would help. + # + # 6 is measured, not picked. Two observed successful reviews: 64s + # (wave-foundation run 32539972644) and 180s on a 314-file PR (wave-pen + # run 32589642731). 360s is 2x the larger. 6 + 0.75 + 6 = 12.75 keeps both + # attempts inside the job's 15. + timeout-minutes: 6 + uses: The-PR-Agent/pr-agent@f6af7d77554ff8d26adffded077e6461329e92fa # v0.42.0 + continue-on-error: true # outcome is classified by the verdict step below (#3128) + env: + OPENAI_KEY: ${{ secrets.OPENAI_KEY }} + CONFIG__MODEL: "openai/claude-opus-5" + CONFIG__MODEL_WEAK: "openai/qwen3.7-flash" + CONFIG__MODEL_REASONING: "openai/claude-sonnet-5" + OPENAI_API_BASE: "https://api.wave.online/v1/dispatch" + CONFIG__CUSTOM_MODEL_MAX_TOKENS: "32000" + CONFIG__AI_TIMEOUT: "600" + CONFIG__FALLBACK_MODELS: "openai/qwen3-coder:30b" + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + github_action_config.auto_review: "true" + github_action_config.auto_describe: "false" + github_action_config.auto_improve: "true" + github_action_config.handle_push_trigger: "true" + pr_code_suggestions.commitable_code_suggestions: "true" + pr_code_suggestions.suggestions_score_threshold: "7" + pr_code_suggestions.num_code_suggestions: "6" + + # Red-while-silent (#3128), which this mirror was missing entirely: upstream + # 429s from the LLM router rendered this check RED with no retry, and + # pr-agent is an ADVISORY reviewer — it annotates, it never gates + # correctness — so a flaked reviewer must never block a PR. + - name: backoff before retry + if: steps.agent.outcome == 'failure' + run: sleep 45 + + - name: PR-Agent retry (attempt 2) + id: agent_retry + if: steps.agent.outcome == 'failure' + timeout-minutes: 6 # same budget as attempt 1 — see its comment + uses: The-PR-Agent/pr-agent@f6af7d77554ff8d26adffded077e6461329e92fa # v0.42.0 + continue-on-error: true + # Duplicated, not aliased: GitHub Actions does not support YAML anchors. + # Keep both blocks identical — a drift here silently reviews attempt 2 + # against a different model than attempt 1. + env: + OPENAI_KEY: ${{ secrets.OPENAI_KEY }} + CONFIG__MODEL: "openai/claude-opus-5" + CONFIG__MODEL_WEAK: "openai/qwen3.7-flash" + CONFIG__MODEL_REASONING: "openai/claude-sonnet-5" + OPENAI_API_BASE: "https://api.wave.online/v1/dispatch" + CONFIG__CUSTOM_MODEL_MAX_TOKENS: "32000" + CONFIG__AI_TIMEOUT: "600" + CONFIG__FALLBACK_MODELS: "openai/qwen3-coder:30b" + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + github_action_config.auto_review: "true" + github_action_config.auto_describe: "false" + github_action_config.auto_improve: "true" + github_action_config.handle_push_trigger: "true" + pr_code_suggestions.commitable_code_suggestions: "true" + pr_code_suggestions.suggestions_score_threshold: "7" + pr_code_suggestions.num_code_suggestions: "6" + + # FOUR outcomes, not three (wave-pen#386). Branching on {success, failure, + # empty} alone sweeps everything else into "most commonly an upstream 429", + # so a job timeout and a concurrency supersede both report a rate limit that + # never happened. A confidently wrong cause is worse than no cause: it sends + # the next reader to the LLM router to debug a workflow defect. + - name: verdict (classify, never block) + if: always() + env: + AGENT_OUTCOME: ${{ steps.agent.outcome }} + AGENT_RETRY_OUTCOME: ${{ steps.agent_retry.outcome }} + STEP_BUDGET_S: "360" # the 6-minute step timeout above, in seconds + run: | + if [ "$AGENT_OUTCOME" = "success" ] || [ "$AGENT_RETRY_OUTCOME" = "success" ]; then + exit 0 + fi + if [ -z "$AGENT_OUTCOME" ]; then + # The reviewer never ran: an earlier step failed or the job was cancelled. + # A WORKFLOW fault, not a reviewer flake — stays loud. + echo "::error::pr-agent never ran — an earlier step failed or the job was cancelled." + exit 1 + fi + if [ "$AGENT_OUTCOME" = "cancelled" ] || [ "$AGENT_RETRY_OUTCOME" = "cancelled" ]; then + # `continue-on-error` does NOT catch a cancellation, so the retry never + # fired either. Two causes, both workflow-level, neither the reviewer's: + # a newer run superseded this one via the concurrency group above, or + # the job hit timeout-minutes. Named rather than guessed between. + echo "::warning::pr-agent was CANCELLED, not failed — a newer run superseded this one via the concurrency group, or the job hit timeout-minutes. Not a reviewer or rate-limit fault (wave-pen#386)." + exit 0 + fi + ELAPSED=$(( $(date +%s) - ${AGENT_START:-$(date +%s)} )) + if [ "$ELAPSED" -ge "$STEP_BUDGET_S" ]; then + echo "::warning::pr-agent TIMED OUT — ${ELAPSED}s against a ${STEP_BUDGET_S}s per-attempt budget, so an attempt was killed by its step timeout rather than returning an error. A hang, NOT a rate limit. Rendering NEUTRAL: an advisory reviewer must not block the PR (#3128)." + exit 0 + fi + echo "::warning::pr-agent failed after 2 attempts (45s backoff, ${ELAPSED}s total — well inside the ${STEP_BUDGET_S}s budget, so it returned an error rather than hanging) — most commonly an upstream 429/rate-limit from the LLM router. Rendering NEUTRAL: an advisory reviewer must not block the PR (#3128)." + exit 0 From 21acf4b3c4a95b0515723012c5883bbd8db64070 Mon Sep 17 00:00:00 2001 From: "wave-av-bot[bot]" Date: Mon, 24 Aug 2026 09:36:06 -0400 Subject: [PATCH 2/6] =?UTF-8?q?fix(pr-agent):=20re-sync=20to=20the=20corre?= =?UTF-8?q?cted=20template=20=E2=80=94=20per-ATTEMPT=20timeout=20classific?= =?UTF-8?q?ation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Picks up wave-foundation-public#72, which landed after this PR was opened. The template this PR originally copied classified timeouts on TOTAL job time (attempt 1 + 45s backoff + attempt 2) against STEP_BUDGET_S=360, a PER-ATTEMPT budget. Two healthy-but-slow attempts (~180s each) were therefore reported as "TIMED OUT ... A hang, NOT a rate limit", and the else-branch claimed the run was "well inside the budget" from the same misused total. Found by qodo review on wave-monitor#48 and confirmed against the file. Now stamps each attempt separately and classifies on the LONGEST attempt, with if: always() end stamps so an attempt killed BY its step timeout still records one. Verified by dry-running both cases before the template landed. Updated in place rather than as a follow-up PR because this has not merged yet — cheaper, and it keeps the repo from ever carrying the defective version. Refs wave-av/wave-pen#417, wave-av/wave-pen#388 --- .github/workflows/pr-agent.yml | 50 ++++++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/.github/workflows/pr-agent.yml b/.github/workflows/pr-agent.yml index abb7b31..b12f4d1 100644 --- a/.github/workflows/pr-agent.yml +++ b/.github/workflows/pr-agent.yml @@ -72,8 +72,17 @@ jobs: # Stamped so the verdict step can tell a TIMED-OUT attempt from a fast # upstream error. Both arrive as outcome == 'failure' and GitHub exposes no # step-level "timed_out", so elapsed time is the only discriminator there is. - - name: stamp attempt start - run: echo "AGENT_START=$(date +%s)" >> "$GITHUB_ENV" + # + # PER-ATTEMPT, and that is the whole fix. This used to be one AGENT_START + # stamped before attempt 1, with the verdict comparing TOTAL job time + # against STEP_BUDGET_S — a budget its own comment calls per-attempt. Two + # slow-but-healthy attempts (~180s each) plus the 45s backoff total ~405s + # and were reported as "TIMED OUT … a hang, NOT a rate limit", sending the + # next reader to debug a hang that never happened. The else-branch was + # equally wrong the other way, asserting the run was "well inside the + # budget" from a total that spans both attempts. + - name: stamp attempt 1 start + run: echo "ATTEMPT1_START=$(date +%s)" >> "$GITHUB_ENV" - name: PR-Agent (OSS qodo-merge) id: agent @@ -114,10 +123,20 @@ jobs: # 429s from the LLM router rendered this check RED with no retry, and # pr-agent is an ADVISORY reviewer — it annotates, it never gates # correctness — so a flaked reviewer must never block a PR. + # `if: always()` so an attempt KILLED by its step timeout still records an + # end stamp — that is precisely the case the classifier needs to see. + - name: stamp attempt 1 end + if: always() + run: echo "ATTEMPT1_END=$(date +%s)" >> "$GITHUB_ENV" + - name: backoff before retry if: steps.agent.outcome == 'failure' run: sleep 45 + - name: stamp attempt 2 start + if: steps.agent.outcome == 'failure' + run: echo "ATTEMPT2_START=$(date +%s)" >> "$GITHUB_ENV" + - name: PR-Agent retry (attempt 2) id: agent_retry if: steps.agent.outcome == 'failure' @@ -145,6 +164,10 @@ jobs: pr_code_suggestions.suggestions_score_threshold: "7" pr_code_suggestions.num_code_suggestions: "6" + - name: stamp attempt 2 end + if: always() + run: echo "ATTEMPT2_END=$(date +%s)" >> "$GITHUB_ENV" + # FOUR outcomes, not three (wave-pen#386). Branching on {success, failure, # empty} alone sweeps everything else into "most commonly an upstream 429", # so a job timeout and a concurrency supersede both report a rate limit that @@ -174,10 +197,25 @@ jobs: echo "::warning::pr-agent was CANCELLED, not failed — a newer run superseded this one via the concurrency group, or the job hit timeout-minutes. Not a reviewer or rate-limit fault (wave-pen#386)." exit 0 fi - ELAPSED=$(( $(date +%s) - ${AGENT_START:-$(date +%s)} )) - if [ "$ELAPSED" -ge "$STEP_BUDGET_S" ]; then - echo "::warning::pr-agent TIMED OUT — ${ELAPSED}s against a ${STEP_BUDGET_S}s per-attempt budget, so an attempt was killed by its step timeout rather than returning an error. A hang, NOT a rate limit. Rendering NEUTRAL: an advisory reviewer must not block the PR (#3128)." + # PER-ATTEMPT durations, not total job time. STEP_BUDGET_S is the + # per-attempt step timeout; comparing it against a total spanning + # attempt 1 + 45s backoff + attempt 2 misclassified two healthy-but-slow + # attempts (~180s each, ~405s together) as a hang. Every value is + # defaulted so the arithmetic can never fail this step and turn the + # classifier into an error of its own. + NOW=$(date +%s) + A1=$(( ${ATTEMPT1_END:-0} - ${ATTEMPT1_START:-0} )) + A2=$(( ${ATTEMPT2_END:-0} - ${ATTEMPT2_START:-0} )) + [ "$A1" -lt 0 ] && A1=0 + [ "$A2" -lt 0 ] && A2=0 + LONGEST=$A1; [ "$A2" -gt "$LONGEST" ] && LONGEST=$A2 + ELAPSED=$(( NOW - ${ATTEMPT1_START:-$NOW} )) + # SLACK because a step killed AT its timeout records a hair under the + # budget — the runner's kill is not instantaneous. + SLACK=15 + if [ "$LONGEST" -ge $(( STEP_BUDGET_S - SLACK )) ]; then + echo "::warning::pr-agent TIMED OUT — the longest attempt ran ${LONGEST}s against a ${STEP_BUDGET_S}s per-attempt budget (attempt 1 ${A1}s, attempt 2 ${A2}s), so it was killed by its step timeout rather than returning an error. A hang, NOT a rate limit. Rendering NEUTRAL: an advisory reviewer must not block the PR (#3128)." exit 0 fi - echo "::warning::pr-agent failed after 2 attempts (45s backoff, ${ELAPSED}s total — well inside the ${STEP_BUDGET_S}s budget, so it returned an error rather than hanging) — most commonly an upstream 429/rate-limit from the LLM router. Rendering NEUTRAL: an advisory reviewer must not block the PR (#3128)." + echo "::warning::pr-agent failed after 2 attempts (attempt 1 ${A1}s, attempt 2 ${A2}s, ${ELAPSED}s wall including the 45s backoff — NEITHER attempt reached the ${STEP_BUDGET_S}s per-attempt budget, so it returned an error rather than hanging) — most commonly an upstream 429/rate-limit from the LLM router. Rendering NEUTRAL: an advisory reviewer must not block the PR (#3128)." exit 0 From 59288f01b2eda1c3411af7a9818db564079e63cd Mon Sep 17 00:00:00 2001 From: "wave-av-bot[bot]" Date: Mon, 24 Aug 2026 10:11:30 -0400 Subject: [PATCH 3/6] =?UTF-8?q?fix(pr-agent):=20re-sync=20=E2=80=94=20fork?= =?UTF-8?q?=20gate=20for=20/commands,=20AI=5FTIMEOUT=20under=20its=20step?= =?UTF-8?q?=20cap?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-syncs this PR to the hardened template before it merges, so it cannot land carrying the two defects it was opened with (wave-pen#418, wave-foundation-public#73). 1. Fork status is now RESOLVED, not assumed. The job-level `if:` refuses forks on the `pull_request` arm; it structurally cannot on `issue_comment`, because fork status is absent from that payload — measured, with a positive control: `issues/.pull_request` carries exactly [diff_url, html_url, merged_at, patch_url, url], while `pulls/.head.repo.fork` answers. A `fork gate` step asks the pulls endpoint and FAILS CLOSED: only a literal `false` proceeds; a 404, a revoked token, a rate limit and `.head.repo = null` all skip. Scope: this lane runs no `actions/checkout`, so fork code is never fetched or executed and no exfiltration path existed. The durable defect was the comment claiming "Forks skipped (no secrets there)" — true of one arm, false of the other, and exactly what would mislead whoever adds a checkout step later. 2. CONFIG__AI_TIMEOUT 600 -> 300, in both env blocks. 600s inside a 360s step is unreachable: the runner killed the step first, so pr-agent never reached its own timeout and never fell back to CONFIG__FALLBACK_MODELS. 3. A latent classifier bug the gate exposed: `stamp attempt 2 end` runs under `if: always()`, so when attempt 2 never ran the arithmetic subtracted from zero and reported a 1787580408-second attempt as a confident TIMED OUT. Fixed at the arithmetic; the verdict also gains an explicit `skipped` branch. The job id stays `pr_agent`, so the check-run context is unchanged and no branch protection rule needs touching. Refs wave-pen#418, wave-pen#417, wave-pen#388 Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/pr-agent.yml | 105 +++++++++++++++++++++++++++++++-- 1 file changed, 101 insertions(+), 4 deletions(-) diff --git a/.github/workflows/pr-agent.yml b/.github/workflows/pr-agent.yml index b12f4d1..bb2c5eb 100644 --- a/.github/workflows/pr-agent.yml +++ b/.github/workflows/pr-agent.yml @@ -55,7 +55,24 @@ concurrency: jobs: pr_agent: timeout-minutes: 15 - # Slash commands: PR-only + trusted members (cost-abuse guard). Forks skipped (no secrets there). + # Slash commands: PR-only + trusted members (cost-abuse guard). + # + # FORKS ARE REFUSED ON ONE ARM HERE, NOT BOTH — say it accurately, because the + # comment this replaces said "Forks skipped (no secrets there)" and that was + # true of `pull_request` and false of `issue_comment` (wave-pen#418). The + # `issue_comment` arm cannot test fork status: its payload does not carry it. + # Measured, with a positive control: + # + # GET /repos/{o}/{r}/issues/47 -> .pull_request keys are exactly + # [diff_url, html_url, merged_at, patch_url, url] + # GET /repos/{o}/{r}/pulls/47 -> .head.repo.fork = false + # + # No `head`, no `repo` — so there is nothing to write in this expression. The + # check moves to the `fork gate` STEP below, which asks the pulls endpoint. + # A false comment is worse than a missing check: it tells the next editor the + # guard is already here, and the day someone adds `actions/checkout` to this + # lane that belief is what makes it a real exfiltration path rather than a + # theoretical one. if: >- ${{ (github.event_name == 'issue_comment' @@ -69,6 +86,57 @@ jobs: }} runs-on: ubuntu-latest steps: + # The fork check the job-level `if:` structurally cannot make (wave-pen#418). + # + # This job holds `OPENAI_KEY` and `pull-requests: write`. On the + # `pull_request` arm the gate above already refused forks. On the + # `issue_comment` arm it could not, because fork status is absent from that + # payload — so it is resolved here, from the pulls endpoint, which does + # carry it. + # + # FAILS CLOSED, and that is the point of the `*` case rather than an + # `if [ "$fork" = true ]`. A 404, a revoked token, a rate limit and a + # renamed field all produce an empty `fork`, and "I could not tell" must not + # arrive at the same answer as "not a fork" on the arm that carries the key. + # The cost of being wrong in that direction is a skipped advisory review. + # + # SCOPE, stated so nobody reads more into it than is there: this lane runs + # NO `actions/checkout`, so fork code is never fetched or executed and there + # is no exfiltration path to close today. What a `/review` on a fork PR + # actually reaches is the fork's diff, sent to the LLM router on our key — + # cost surface, already narrowed by the author_association allowlist above. + # This is defence in depth, and it is what makes the corrected comment true. + - name: fork gate (issue_comment only) + id: gate + env: + # Every event-derived value crosses into the shell through env, never + # through `${{ }}` in the script body — a `run:` block is a template, + # and a PR title is attacker-controlled text. + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + EVENT_NAME: ${{ github.event_name }} + REPO: ${{ github.repository }} + PR_NUMBER: ${{ github.event.issue.number }} + run: | + if [ "$EVENT_NAME" != "issue_comment" ]; then + # The job-level `if:` already proved `head.repo.fork == false`. + echo "fork=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + fork=$(gh api "repos/$REPO/pulls/$PR_NUMBER" --jq '.head.repo.fork' 2>/dev/null || echo "") + case "$fork" in + false) + echo "fork=false" >> "$GITHUB_OUTPUT" + ;; + true) + echo "fork=true" >> "$GITHUB_OUTPUT" + echo "::warning::Slash command ignored: this PR is from a fork, and this lane carries OPENAI_KEY. Forks are reviewed by a maintainer, not by the agent lane (wave-pen#418)." + ;; + *) + echo "fork=true" >> "$GITHUB_OUTPUT" + echo "::warning::Could not determine whether PR #${PR_NUMBER} is from a fork (the pulls endpoint returned nothing usable). Treating it AS a fork and skipping — an unreadable answer is not a negative one (wave-pen#418)." + ;; + esac + # Stamped so the verdict step can tell a TIMED-OUT attempt from a fast # upstream error. Both arrive as outcome == 'failure' and GitHub exposes no # step-level "timed_out", so elapsed time is the only discriminator there is. @@ -86,6 +154,7 @@ jobs: - name: PR-Agent (OSS qodo-merge) id: agent + if: steps.gate.outputs.fork != 'true' # STEP-level budget, under the job's 15 (wave-pen#386). # # Without it one hung attempt consumes the WHOLE job budget: the job hits @@ -108,7 +177,12 @@ jobs: CONFIG__MODEL_REASONING: "openai/claude-sonnet-5" OPENAI_API_BASE: "https://api.wave.online/v1/dispatch" CONFIG__CUSTOM_MODEL_MAX_TOKENS: "32000" - CONFIG__AI_TIMEOUT: "600" + # MUST stay under the step's 360s, or it is not a budget (wave-pen#418). + # At 600 the runner killed the step first, so pr-agent never reached its + # own timeout, never fell back to CONFIG__FALLBACK_MODELS, and returned + # no error the retry could classify. 300 leaves 60s of headroom and sits + # above both observed successful reviews (64s, 180s). + CONFIG__AI_TIMEOUT: "300" CONFIG__FALLBACK_MODELS: "openai/qwen3-coder:30b" GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} github_action_config.auto_review: "true" @@ -153,7 +227,12 @@ jobs: CONFIG__MODEL_REASONING: "openai/claude-sonnet-5" OPENAI_API_BASE: "https://api.wave.online/v1/dispatch" CONFIG__CUSTOM_MODEL_MAX_TOKENS: "32000" - CONFIG__AI_TIMEOUT: "600" + # MUST stay under the step's 360s, or it is not a budget (wave-pen#418). + # At 600 the runner killed the step first, so pr-agent never reached its + # own timeout, never fell back to CONFIG__FALLBACK_MODELS, and returned + # no error the retry could classify. 300 leaves 60s of headroom and sits + # above both observed successful reviews (64s, 180s). + CONFIG__AI_TIMEOUT: "300" CONFIG__FALLBACK_MODELS: "openai/qwen3-coder:30b" GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} github_action_config.auto_review: "true" @@ -183,6 +262,13 @@ jobs: if [ "$AGENT_OUTCOME" = "success" ] || [ "$AGENT_RETRY_OUTCOME" = "success" ]; then exit 0 fi + if [ "$AGENT_OUTCOME" = "skipped" ]; then + # The fork gate refused this run and already said why. Distinct from + # the empty case below: `skipped` means a guard declined deliberately, + # `""` means the step never existed to have an outcome. + echo "::notice::pr-agent did not run — declined by the fork gate above (wave-pen#418)." + exit 0 + fi if [ -z "$AGENT_OUTCOME" ]; then # The reviewer never ran: an earlier step failed or the job was cancelled. # A WORKFLOW fault, not a reviewer flake — stays loud. @@ -205,7 +291,18 @@ jobs: # classifier into an error of its own. NOW=$(date +%s) A1=$(( ${ATTEMPT1_END:-0} - ${ATTEMPT1_START:-0} )) - A2=$(( ${ATTEMPT2_END:-0} - ${ATTEMPT2_START:-0} )) + # A2 is computed ONLY from a real start stamp. `stamp attempt 2 end` + # carries `if: always()`, so it fires even when attempt 2 never ran — + # and `END - ${START:-0}` then subtracts from ZERO, yielding a ~1.7e9 + # "duration" that clears any budget and reports a TIMED OUT that never + # happened. Latent since the stamps landed (#72); the fork gate is just + # the first path that reaches it. Fixed at the arithmetic, not by + # special-casing the caller (wave-pen#418). + if [ -n "${ATTEMPT2_START:-}" ] && [ "${ATTEMPT2_START:-0}" -gt 0 ]; then + A2=$(( ${ATTEMPT2_END:-0} - ATTEMPT2_START )) + else + A2=0 + fi [ "$A1" -lt 0 ] && A1=0 [ "$A2" -lt 0 ] && A2=0 LONGEST=$A1; [ "$A2" -gt "$LONGEST" ] && LONGEST=$A2 From 00e5536c22479a8635288177053c6236a3a4655f Mon Sep 17 00:00:00 2001 From: "wave-av-bot[bot]" Date: Mon, 24 Aug 2026 10:28:33 -0400 Subject: [PATCH 4/6] fix(pr-agent): gate positively on an explicit false, not on the absence of a true MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of this wave found the fail-closed gate had a fail-OPEN consumer. Two reviewers flagged it independently, on two different repos, and they were right. if: steps.gate.outputs.fork != 'true' # grants when the output is EMPTY The gate could only fail closed if it always wrote an output. It did, on every path — so this did not fail open today, and the implicit success() on the consumer covers a gate that errors outright. But the safety rested on an argument rather than on the structure, and it is the very argument this change exists to delete: absence must not read as permission. Two independent changes, so neither carries the invariant alone: - the gate now assigns a shell variable that STARTS at `true` and writes ONCE at the end, so no future edit adding an early exit can emit nothing; - the consumer requires `== 'false'`, an explicit affirmative, so an empty or missing output skips the agent. Also braces both sides of the A2 subtraction in the verdict step. The bare `ATTEMPT2_START` was CORRECT — POSIX arithmetic expansion evaluates a bare name as a variable, verified identical (180 == 180) — but a reviewer read it as a literal token and filed it High. An expression that reads wrong on 27 repos gets re-filed on 27 repos, so it is normalised rather than defended. RECEIPTS. actionlint clean; zizmor clean; shellcheck clean. The gate was driven through all six branches plus the reviewers' no-output scenario: only a literal `false` reaches AGENT RUNS. The verdict was re-run across all six states and is unchanged on the five that already worked. LIVE: wave-av/api-spec merged the previous revision and its pull_request run executed `fork gate (issue_comment only) -> success` in production, then ran the agent — so the gate does not wrongly refuse a legitimate same-repo PR. Upstream: wave-av/wave-foundation-public#73. Refs wave-pen#418, wave-pen#417. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/pr-agent.yml | 51 ++++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/.github/workflows/pr-agent.yml b/.github/workflows/pr-agent.yml index bb2c5eb..11e59ab 100644 --- a/.github/workflows/pr-agent.yml +++ b/.github/workflows/pr-agent.yml @@ -117,25 +117,31 @@ jobs: REPO: ${{ github.repository }} PR_NUMBER: ${{ github.event.issue.number }} run: | + # ONE write, at the end, from a variable that STARTS at the refusing + # value. An earlier revision wrote `fork=` separately on each branch, + # which meant a future edit adding an early `exit 0` would emit no + # output at all — and the consumer's `!= 'true'` then read that silence + # as permission. Two reviewers flagged it independently. Structured so + # the refusal is the default rather than one branch among several. + fork=true if [ "$EVENT_NAME" != "issue_comment" ]; then # The job-level `if:` already proved `head.repo.fork == false`. - echo "fork=false" >> "$GITHUB_OUTPUT" - exit 0 + fork=false + else + answer=$(gh api "repos/$REPO/pulls/$PR_NUMBER" --jq '.head.repo.fork' 2>/dev/null || echo "") + case "$answer" in + false) + fork=false + ;; + true) + echo "::warning::Slash command ignored: this PR is from a fork, and this lane carries OPENAI_KEY. Forks are reviewed by a maintainer, not by the agent lane (wave-pen#418)." + ;; + *) + echo "::warning::Could not determine whether PR #${PR_NUMBER} is from a fork (the pulls endpoint returned nothing usable). Treating it AS a fork and skipping — an unreadable answer is not a negative one (wave-pen#418)." + ;; + esac fi - fork=$(gh api "repos/$REPO/pulls/$PR_NUMBER" --jq '.head.repo.fork' 2>/dev/null || echo "") - case "$fork" in - false) - echo "fork=false" >> "$GITHUB_OUTPUT" - ;; - true) - echo "fork=true" >> "$GITHUB_OUTPUT" - echo "::warning::Slash command ignored: this PR is from a fork, and this lane carries OPENAI_KEY. Forks are reviewed by a maintainer, not by the agent lane (wave-pen#418)." - ;; - *) - echo "fork=true" >> "$GITHUB_OUTPUT" - echo "::warning::Could not determine whether PR #${PR_NUMBER} is from a fork (the pulls endpoint returned nothing usable). Treating it AS a fork and skipping — an unreadable answer is not a negative one (wave-pen#418)." - ;; - esac + echo "fork=$fork" >> "$GITHUB_OUTPUT" # Stamped so the verdict step can tell a TIMED-OUT attempt from a fast # upstream error. Both arrive as outcome == 'failure' and GitHub exposes no @@ -154,7 +160,11 @@ jobs: - name: PR-Agent (OSS qodo-merge) id: agent - if: steps.gate.outputs.fork != 'true' + # POSITIVELY gated: runs only on an explicit `false`, never on the mere + # absence of a `true`. `!= 'true'` would grant on an empty output, i.e. + # treat "the gate said nothing" as "the gate said yes" — the exact shape + # this whole change exists to remove, reproduced one layer down. + if: steps.gate.outputs.fork == 'false' # STEP-level budget, under the job's 15 (wave-pen#386). # # Without it one hung attempt consumes the WHOLE job budget: the job hits @@ -299,7 +309,12 @@ jobs: # the first path that reaches it. Fixed at the arithmetic, not by # special-casing the caller (wave-pen#418). if [ -n "${ATTEMPT2_START:-}" ] && [ "${ATTEMPT2_START:-0}" -gt 0 ]; then - A2=$(( ${ATTEMPT2_END:-0} - ATTEMPT2_START )) + # Braced on BOTH sides. The bare `ATTEMPT2_START` this replaces was + # correct — POSIX arithmetic expansion evaluates a bare name as a + # variable, verified identical (180 == 180) — but a reviewer read it + # as a literal token and filed it High, and an expression that reads + # wrong on 27 repos will be re-filed on 27 repos. + A2=$(( ${ATTEMPT2_END:-0} - ${ATTEMPT2_START:-0} )) else A2=0 fi From d80d077212e4350c893ace75a55c3339404495ad Mon Sep 17 00:00:00 2001 From: "wave-av-bot[bot]" Date: Mon, 24 Aug 2026 10:37:05 -0400 Subject: [PATCH 5/6] =?UTF-8?q?fix(pr-agent):=20key=20concurrency=20on=20p?= =?UTF-8?q?r-vs-issue=20too=20=E2=80=94=20they=20share=20one=20number=20sp?= =?UTF-8?q?ace?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found a SECOND concurrency collision, on a different axis from the one this template already documents (wave-pen#386). `issue_comment` fires for ISSUES as well as PRs, and GitHub draws both from ONE number sequence. So a comment on Issue #30 and a `/review` on PR #30 entered the same concurrency group. Concurrency is evaluated at WORKFLOW level, BEFORE the job-level `if:` runs — so the Issue comment cancelled the PR review already in flight, and was then skipped itself, having done nothing. That is the identical shape as the #386 defect the block above exists to fix, one axis over: a run that will not review taking the lane from the run that would have. #386 separated the two EVENTS; it did not separate the two number spaces inside one event. pull_request PR 433 -> pr-agent-pull_request-pr-433 issue_comment on PR 30 -> pr-agent-issue_comment-pr-30 issue_comment on ISSUE 30 -> pr-agent-issue_comment-issue-30 The last two used to be one group. actionlint and zizmor clean. Upstream: wave-av/wave-foundation-public#73. Refs wave-pen#418, wave-pen#417. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/pr-agent.yml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pr-agent.yml b/.github/workflows/pr-agent.yml index 11e59ab..230ef7d 100644 --- a/.github/workflows/pr-agent.yml +++ b/.github/workflows/pr-agent.yml @@ -48,8 +48,17 @@ permissions: # a new request, not a supersession of a push review. Proven live on wave-pen: # the first successful run in that repo's history, with the comment run skipping # harmlessly beside it. +# +# THE KEY ALSO CARRIES pr-VS-issue, and that is a second, separate collision +# (wave-pen#418). `issue_comment` fires for ISSUES as well as PRs, and GitHub +# draws both from ONE number sequence — so a comment on Issue #30 and a +# `/review` on PR #30 landed in the same group. Concurrency is evaluated at +# WORKFLOW level, before the job-level `if:` runs, so the Issue comment cancels +# the PR's in-flight review and is then skipped itself, doing nothing. That is +# the identical shape as the #386 defect above, one axis over: a run that will +# not review taking the lane from the run that would have. concurrency: - group: pr-agent-${{ github.event_name }}-${{ github.event.pull_request.number || github.event.issue.number || github.ref }} + group: pr-agent-${{ github.event_name }}-${{ (github.event.pull_request.number || github.event.issue.pull_request) && 'pr' || 'issue' }}-${{ github.event.pull_request.number || github.event.issue.number || github.ref }} cancel-in-progress: true jobs: From 6ef66e1b9d6507478e7c293581cc16a0fd489207 Mon Sep 17 00:00:00 2001 From: "wave-av-bot[bot]" Date: Mon, 24 Aug 2026 10:41:25 -0400 Subject: [PATCH 6/6] docs(changelog): record the pr-agent fork gate and timeout fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A reviewer flagged the missing entry on wave-modules#41. 25 of the 28 repos in this wave keep the same Keep-a-Changelog convention, so the entry lands in all of them rather than only the repo whose review happened to catch it — fixing the reported instance and leaving the class is the pattern this wave keeps undoing. The change IS user-visible, which is why it belongs here: a maintainer's `/review` on a fork PR is now declined with a warning instead of silently running, so contributors on forks see different behaviour. Refs wave-pen#418, wave-av/wave-foundation-public#73 Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f99a84..69a9b7d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,3 +5,29 @@ All notable changes to this project are documented here. The format is based on [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] + +### Fixed + +- `pr-agent` lane: fork-triggered `/` commands are now refused, and the AI + call's budget fits inside its step. Three defects, one of them only visible + once the first was fixed. + + The job-level `if:` refused forks on the `pull_request` arm and could not on + `issue_comment` — fork status is absent from that payload, so there was never + an expression to write. A `fork gate` step now asks the pulls endpoint and + fails closed: only a literal `false` proceeds, so a 404, a rate limit or a + deleted fork all skip. The lane runs no `actions/checkout`, so fork code was + never executed and no exfiltration path existed; what this closes is the + comment claiming forks were already skipped, which was true of one arm only. + + `CONFIG__AI_TIMEOUT` was 600s inside a 360s step, so the runner killed the + step before pr-agent could reach its own timeout or fall back to a secondary + model. Now 300s. + + Fixing the first exposed a third: `stamp attempt 2 end` runs under + `if: always()`, so when attempt 2 never ran the verdict subtracted from zero + and reported a 1787580408-second attempt as a confident TIMED OUT. + + Contributors on forks are affected: a maintainer's `/review` on a fork PR is + now declined with a warning rather than silently running. + (wave-av/wave-foundation-public#73)