Skip to content

ci: adopt the inline pr-agent lane — a public repo cannot call a private reusable workflow - #24

Open
yakimoto wants to merge 6 commits into
mainfrom
ci/adopt-inline-pr-agent
Open

ci: adopt the inline pr-agent lane — a public repo cannot call a private reusable workflow#24
yakimoto wants to merge 6 commits into
mainfrom
ci/adopt-inline-pr-agent

Conversation

@yakimoto

@yakimoto yakimoto commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

User description

User description

User description

User description

User description

Part of the fan-out tracked in wave-pen#388, proven first on wave-certify#44 where pr_agent returned success.

The defect

pr-agent.yml here calls wave-av/wave-foundation/.github/workflows/reusable-pr-agent.yml@main, 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 at all.

That is worse than a normal failure. There is nothing to click through to. Every PR on this repo has been carrying a red check that reports nothing, and external contributors see it.

Measured on this repo today — the last 5 pr-agent runs:

failure, failure, failure, failure, failure

Measured across the org on 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 were each tested and refuted — missing OPENAI_KEY (present in both populations), a dead pinned ref (150ffae2 resolves, file exists at it), and @main vs a pinned sha (wave-realtime-edge pins @main and fails, wave-pen pins @main and works).

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. Its own header says it was written for exactly this. This PR adopts it verbatim.

So this is an adoption gap, not a design gap.

Why now, and not when #388 was filed

#388 named two blockers, and both are cleared as of wave-foundation-public#71:

  1. The shared concurrency key. The template carried pr-agent-${{ github.event.pull_request.number || … }}, shared between pull_request and issue_comment, so any bot comment cancelled a live review ~10s in (wave-pen#386). It now keys on github.event_name.
  2. Missing step-level timeouts. The template now carries 6.

Fanning out before those landed would have traded a red-with-no-log lane for a cancelled-on-every-comment lane — a different failure, not a fix.

Verified before opening this

  • The template is genuinely self-contained. Its only two wave-foundation/ mentions are in comments, not in a uses:. Checked rather than assumed, since that is the whole property this depends on.
  • The job id stays pr_agent. A job's id is its check-run context and branch protection matches on (context, app_id), so nothing needs touching on the protection side.
  • The workflow parses, and the source was read from a fresh clone of wave-foundation-public's default branch — not from a local checkout that might be parked on another branch.

The receipt is this PR, not the diff

A red lane and a working lane are indistinguishable until one actually runs — that is the whole reason 176 failures went unexamined. So the proof is pr-agent going green on this PR. If it does, the remaining 27 repos get the same change with evidence behind it. If it does not, we learn that here, on one low-traffic repo, instead of across the org's entire public surface.

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. The other repos were not changed on hope.

Refs wave-pen#388


Note

Medium Risk
CI-only, but the job holds OPENAI_KEY and PR write permissions; fork gating and fail-closed shell logic are security-sensitive even though there is no checkout of fork code.

Overview
Replaces the broken uses: of private wave-foundation reusable workflow with a self-contained pr-agent.yml. Public repos cannot call private reusable workflows, so the old lane failed with zero jobs and no logs.

The inline lane keys concurrency on event name plus pr-vs-issue so comments no longer cancel in-flight reviews. Slash commands stay PR-only and member-gated. Fork PRs are refused on pull_request in the job if:, and on issue_comment via a fail-closed gh api fork gate (only a literal false proceeds).

Adds a 6-minute per-attempt timeout, CONFIG__AI_TIMEOUT of 300s, one retry after 45s, and a verdict step that classifies success, skip, cancel, hang, and rate-limit without blocking the PR. Changelog documents the fork-gate, timeout, and stamp-arithmetic fixes.

Reviewed by Cursor Bugbot for commit 55f9b69. Bugbot is set up for automated code reviews on this repo. Configure here.

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Summary by Sourcery

Restore a reliable, advisory-only PR-Agent lane for this public repository.

Bug Fixes:

  • Make the PR-Agent check run successfully in this public repository by removing its dependency on a private reusable workflow.
  • Prevent issue comments and fork pull requests from triggering unsafe or conflicting advisory reviews.
  • Ensure reviewer failures, timeouts, cancellations, and rate limits are classified appropriately without blocking pull requests.

Enhancements:

  • Keep pull-request and slash-command reviews independent while allowing successive updates to supersede earlier reviews.
  • Add a retry attempt with bounded execution time for transient reviewer failures.

CI:

  • Inline and update the PR-Agent workflow with event-aware concurrency, fork safeguards, retries, per-attempt timeouts, and non-blocking verdict reporting.

Review in cubic


PR Type

Bug fix, Enhancement


Description

  • Replaced private workflow reference with inline PR agent configuration

  • Fixed fork handling for slash command reviews

  • Added timeout budgeting and retry logic for reliability

  • Improved error classification and reporting


Diagram Walkthrough

flowchart TD
  A["pr-agent.yml"] --> B["Fork Gate Check"]
  B --> C{Fork?}
  C -->|Yes| D["Skip with warning"]
  C -->|No| E["Attempt 1 (6min)"]
  E --> F["Stamp Start"]
  E --> G["Run PR Agent"]
  G --> H{"Success?"}
  H -->|Yes| I["Exit"]
  H -->|No| J["Retry after 45s"]
  J --> K["Attempt 2 (6min)"]
  K --> L["Stamp End"]
  L --> M["Verdict Classification"]
Loading

File Walkthrough

Relevant files
Bug fix
pr-agent.yml
Inline PR agent workflow with enhanced reliability             

.github/workflows/pr-agent.yml

  • Replaced private workflow reference with inline configuration
  • Added comprehensive fork safety checks for slash commands
  • Implemented per-attempt timeout budgeting (360s) with retry logic
  • Enhanced error classification with detailed status reporting
+325/-6 
Documentation
CHANGELOG.md
Updated changelog with workflow improvements                         

CHANGELOG.md

  • Documented fix for fork-triggered review handling
  • Added timeout budgeting improvements
  • Recorded retry logic enhancements
  • Noted safety changes for forked PRs
+26/-0   

@codeant-ai

codeant-ai Bot commented Aug 23, 2026

Copy link
Copy Markdown

🤖 CodeAnt AI — Review Status

Status Commit Started (UTC) Finished (UTC)
✅ Incremental review completed 359e066 Aug 24, 2026 · 14:37 14:37
✅ Incremental review completed 6a0b59e Aug 24, 2026 · 13:36 13:36
✅ Reviewed your PR 3818080 Aug 23, 2026 · 22:34 22:35

@sourcery-ai

sourcery-ai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Reviewer's Guide

Replaces the broken reusable pr-agent workflow call with a fully inline, self-contained workflow definition tailored for public repos, and aligns it with the current reusable lane behavior (concurrency, retries, timeouts, and advisory verdict handling).

Sequence diagram for PR-Agent retry and advisory verdict

sequenceDiagram
    participant G as GitHub Actions
    participant A as PR-Agent action
    participant L as LLM router
    participant V as Verdict step

    G->>A: Run attempt 1
    A->>L: Review pull request
    alt attempt 1 succeeds
        A-->>G: success
        G->>V: Evaluate outcomes
        V-->>G: exit 0
    else attempt 1 fails
        A-->>G: failure
        G->>G: sleep 45 seconds
        G->>A: Run retry attempt 2
        A->>L: Review pull request
        A-->>G: success or failure
        G->>V: Evaluate both outcomes
        alt either attempt succeeds
            V-->>G: exit 0
        else reviewer failure or timeout
            V-->>G: advisory warning, exit 0
        end
    end
Loading

Flow diagram for public PR-Agent event handling

flowchart TD
    E[Workflow event] --> K[Concurrency key includes event name]
    K --> T{Eligible event and PR?}
    T -->|pull_request: non-draft, non-fork, non-bot| R[Run inline reviewer]
    T -->|issue_comment: trusted slash command| R
    T -->|otherwise| S[Skip job]
    R --> B[6-minute attempt timeout]
    B --> F{Attempt succeeds?}
    F -->|yes| V[Verdict exits 0]
    F -->|no| D[45-second backoff]
    D --> X[Retry with 6-minute timeout]
    X --> V
    V --> O[Advisory result; reviewer failures do not block PR]
Loading

File-Level Changes

Change Details Files
Inline and modernize the pr-agent CI workflow for public repository compatibility and more robust, non-blocking behavior.
  • Replace the reusable workflow call with an inline job definition that runs The-PR-Agent directly with the required OPENAI and GitHub configuration.
  • Update concurrency grouping to key on the event name plus PR/issue/ref so slash-command runs no longer cancel pull_request reviews.
  • Add job- and step-level timeouts plus a start-time stamp to distinguish hangs, cancellations, and normal failures and keep retries within the overall budget.
  • Introduce a guarded if: on the job to limit slash commands to trusted members and to only review non-draft, non-bot, non-fork pull requests.
  • Add a second PR-Agent attempt with fixed backoff and identical environment to handle transient upstream failures (e.g., 429s).
  • Add a final verdict step that always runs, classifies outcomes into explicit buckets (never blocking the PR), and emits informative warnings/errors instead of hard failures in advisory scenarios.
.github/workflows/pr-agent.yml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@cursor

cursor Bot commented Aug 23, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_23a3491f-7f08-4d72-8e19-85cd2be6c3c8)

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 26 minutes.

View limit details

Limit details: You’ve used the included review currently available. Your 91 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 0e2c304e-1138-4ff3-a3e0-d253f7be3458

📥 Commits

Reviewing files that changed from the base of the PR and between ae89f11 and 55f9b69.

📒 Files selected for processing (2)
  • .github/workflows/pr-agent.yml
  • CHANGELOG.md

Comment @coderabbitai help to get the list of available commands.

@codeant-ai codeant-ai Bot added the size:L This PR changes 100-499 lines, ignoring generated files label Aug 23, 2026
@macroscopeapp

macroscopeapp Bot commented Aug 23, 2026

Copy link
Copy Markdown

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR replaces a small reusable-workflow reference with a credential-bearing, 345-line GitHub Actions implementation that changes PR automation, concurrency, retries, fork handling, and write behavior. An unresolved bot-filtering defect and security-sensitive review concern warrant human validation.

Not approved because:

  • Credit balance exhausted. Approvability relies on correctness review in order to determine eligibility

Review your spending limits in Billing settings. You can add or adjust custom eligibility rules. Learn more.

@github-actions

github-actions Bot commented Aug 23, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit 7518c97)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
⚡ Recommended focus areas for review

Misclassification

ELAPSED is measured from the single AGENT_START stamp taken before attempt 1, so it
accumulates attempt 1 + the 45s backoff + attempt 2. A genuine error path (e.g. attempt 1
errors after ~300s, backoff 45s, attempt 2 errors after 20s) totals >360s and gets reported
as "TIMED OUT ... A hang, NOT a rate limit" — precisely the confidently-wrong cause the
comment says it wants to avoid. Re-stamping the start before each attempt (or comparing
per-attempt elapsed) would make the discriminator match the per-attempt budget.

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
Ordering Bug

The empty-outcome branch is evaluated before the cancelled branch. If the run is superseded
by the concurrency group (or hits the job timeout) before the agent step starts,
AGENT_OUTCOME is empty rather than cancelled, so the verdict emits ::error::pr-agent never ran — an earlier step failed and exits 1, blaming a workflow fault for what is
actually a supersession. Checking

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've reviewed your changes and they look great!

Sourcery assessment

Needs a human reviewer. If the workflow’s trust checks or pinned action usage are wrong, an untrusted pull request or comment could cause a secret-bearing reviewer to run with write-capable GitHub credentials, or could generate and commit unwanted code changes. Reverting the workflow stops future runs, but any exposed secret or already-created commit would require separate remediation.


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

CI: inline PR-Agent workflow so public repos avoid private reusable workflow

🐞 Bug fix ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Replace private reusable workflow call with a self-contained inline pr-agent job.
• Prevent issue_comment runs from canceling pull_request reviews by scoping concurrency to event.
• Add step timeouts, retry/backoff, and a verdict step to keep this reviewer advisory.
Diagram

graph TD
  evt(("PR events")) --> conc["Concurrency group\n(event-scoped)"] --> gate{"Job gate\n(trusted + non-fork)"}
  gate -->|"run"| job["Job: pr_agent"] --> a1["PR-Agent attempt 1"] --> a2["Backoff + attempt 2\n(on failure)"] --> verdict["Verdict step\n(neutralize advisory failures)"]
  gate -->|"skip"| skip["Job skipped"]
  subgraph Legend
    direction LR
    _evt(("Event")) ~~~ _dec{"Decision"} ~~~ _box["Job/Step"]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Move reusable workflow to a public “shared workflows” repo
  • ➕ Restores true single-source-of-truth while keeping public repos compliant
  • ➕ Avoids copy/paste drift across many repos
  • ➖ Requires org-level coordination and migration across all repos
  • ➖ Still needs careful handling of secrets/permissions for public consumers
2. Package PR-Agent wrapper as a composite action
  • ➕ Versioned, reusable logic without workflow-level coupling
  • ➕ Easier to unit-test and reuse across workflows
  • ➖ More work up front than copying a workflow
  • ➖ Still requires keeping workflow wiring (triggers/concurrency/permissions) consistent per repo

Recommendation: Adopting the inline workflow is the right immediate fix because GitHub forbids public→private reusable-workflow calls and this restores a visible, debuggable check run. Longer-term, consider centralizing the lane into a public shared-workflows repo (or a composite action) to reduce ongoing drift risk across the org.

Files changed (1) +166 / -6

Other (1) +166 / -6
pr-agent.ymlInline PR-Agent lane with event-scoped concurrency, retries, and neutral verdicts +166/-6

Inline PR-Agent lane with event-scoped concurrency, retries, and neutral verdicts

• Replaces the private reusable-workflow 'uses:' invocation with an inline 'pr_agent' job so the workflow can run in a public repository. Updates concurrency to include 'github.event_name' to prevent issue_comment runs from cancelling pull_request reviews, and adds step-level timeouts plus a retry/backoff and verdict step to keep advisory PR-Agent failures from blocking merges.

.github/workflows/pr-agent.yml

Comment thread .github/workflows/pr-agent.yml
Comment thread .github/workflows/pr-agent.yml
@gitar-bot

gitar-bot Bot commented Aug 23, 2026

Copy link
Copy Markdown

Note

Automatic reviews are paused because your team has used its included automatic processing for this billing period (headroom scales with your seat count). You can still comment "Gitar review" to run one anytime, and automatic reviews resume on their own by September 1. Add seats for more headroom.
Learn more

Code Review ✅ Approved 2 resolved / 2 findings

Adopts an inline PR-Agent workflow to fix private reusable workflow failures in public repositories, but the bot-login substring check misses non-[bot]-suffixed bot accounts and the issue_comment slash-command lacks a fork check.

✅ 2 resolved
Bug: Bot-login substring check misses non-[bot]-suffixed bot accounts

📄 .github/workflows/pr-agent.yml:66
!contains(github.event.pull_request.user.login, '[bot]') (pr-agent.yml:66) only excludes logins that literally contain the string [bot]. Bot/automation accounts that don't follow that naming convention (e.g. many GitHub Apps or custom automation users) will slip through and trigger the paid LLM review, while a legitimate human account whose login happens to contain that substring would be wrongly excluded. Use github.event.pull_request.user.type == 'Bot', which is the field GitHub sets for actual bot/App-authored actors, instead of a substring match on login.

Security: issue_comment slash-command branch has no fork check for the target PR

📄 .github/workflows/pr-agent.yml:59-69
The pull_request branch of the if: guard excludes forked-repo PRs (head.repo.fork == false), but the issue_comment branch (pr-agent.yml:61-64) only checks author_association, with no equivalent check that the PR being commented on isn't from a fork. A trusted member commenting /review on a fork-originated PR will still run this job with OPENAI_KEY/GITHUB_TOKEN (write) exposed. The-PR-Agent action reads PR data via the GitHub API rather than checking out fork code, which limits the blowup radius, but the diff/PR content from the fork is still sent to the LLM backend under these credentials — consider also gating the comment branch on the underlying PR not being a fork if that risk matters here.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@github-actions

Copy link
Copy Markdown

PR Code Suggestions ✨

No code suggestions found for the PR.

@qodo-code-review

qodo-code-review Bot commented Aug 23, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. AI timeout exceeds step ✓ Resolved 🐞 Bug ☼ Reliability
Description
The PR-Agent step has timeout-minutes: 6 (360s) but sets CONFIG__AI_TIMEOUT: "600", so the
action can legitimately wait longer than the step budget and get killed by the runner, producing a
cancelled outcome and preventing the retry (which only runs on failure). This will reduce
successful reviews during slow upstream periods and can silently turn recoverable slowness into a
cancellation path.
Code

.github/workflows/pr-agent.yml[R100-103]

+          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"
Relevance

●●● Strong

Recent workflow review history accepts concrete CI reliability fixes; timeout mismatch directly
undermines the documented retry design.

PR-#23

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow config sets a 6-minute step timeout but independently configures the agent to allow 600
seconds for AI operations; the retry step only triggers on failure, so a cancellation caused by
the step timeout won’t retry.

.github/workflows/pr-agent.yml[78-112]
.github/workflows/pr-agent.yml[121-147]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`CONFIG__AI_TIMEOUT` (600s) is larger than the step timeout (6 minutes / 360s). This makes it possible for pr-agent to still be waiting on its own timeout when the runner kills the step, yielding a `cancelled` outcome and skipping the retry.

## Issue Context
- Attempt 1 has `timeout-minutes: 6` and `CONFIG__AI_TIMEOUT: "600"`.
- Attempt 2 repeats the same pattern.
- Retry is gated by `if: steps.agent.outcome == 'failure'`, so cancelled attempts won’t retry.

## Fix Focus Areas
- .github/workflows/pr-agent.yml[78-112]
- .github/workflows/pr-agent.yml[121-147]

## Suggested change
Either:
- Reduce `CONFIG__AI_TIMEOUT` to <= 360 (preferably a bit under, e.g. 330) so the action times out before the step budget, OR
- Increase the step timeout (and job timeout) to safely exceed the configured AI timeout (and keep the two-attempt math consistent).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Timeout verdict misclassified ✓ Resolved 🐞 Bug ◔ Observability
Description
The verdict’s timeout detection compares ELAPSED since a single job-level stamp against a
*per-attempt* budget (STEP_BUDGET_S=360), but ELAPSED includes the backoff sleep and the retry
attempt. This can incorrectly label a genuine (non-timeout) failure as a timeout whenever
attempt1+sleep+attempt2 exceeds 360s, producing misleading warnings and hiding the real failure
mode.
Code

.github/workflows/pr-agent.yml[R177-180]

+          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
Relevance

●●● Strong

This is a deterministic elapsed-time accounting bug: job-wide elapsed time is compared against a
per-attempt budget.

PR-#23

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The code stamps AGENT_START once, sleeps 45s on failure, and potentially runs a second attempt,
yet the verdict compares total elapsed time to STEP_BUDGET_S which is annotated as the per-attempt
timeout (360s). That logic can exceed 360s without any step hitting its 6-minute timeout.

.github/workflows/pr-agent.yml[75-77]
.github/workflows/pr-agent.yml[117-126]
.github/workflows/pr-agent.yml[153-183]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The verdict step computes `ELAPSED` from a single timestamp set before attempt 1, then compares it to `STEP_BUDGET_S` (documented as the 6-minute *per-attempt* timeout). Because `ELAPSED` includes attempt 1 + 45s sleep + attempt 2, it can exceed 360s without any step timing out, causing misclassification.

## Issue Context
- `AGENT_START` is stamped once before attempt 1.
- There is a 45s backoff and potentially a second attempt.
- Verdict compares total elapsed time to the per-attempt budget.

## Fix Focus Areas
- .github/workflows/pr-agent.yml[75-77]
- .github/workflows/pr-agent.yml[117-126]
- .github/workflows/pr-agent.yml[153-183]

## Suggested change
Options:
1) Stamp separate start times (e.g., `AGENT1_START`, `AGENT2_START`) before each attempt and compute elapsed per attempt; only classify timeout if the corresponding attempt elapsed approaches the step budget.
2) Remove the elapsed heuristic and classify timeouts/cancellations based on step outcomes only, and adjust messaging to avoid claiming a timeout when you can’t distinguish it.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

3. Env duplication drift risk 🐞 Bug ⚙ Maintainability
Description
The full pr-agent env: block is duplicated for attempt 1 and attempt 2, so any future tweak can
accidentally apply to only one attempt and change behavior under retry conditions. This is an
ongoing maintenance risk in a file explicitly described as a “mirror” that can drift.
Code

.github/workflows/pr-agent.yml[R130-133]

+        env:
+          OPENAI_KEY: ${{ secrets.OPENAI_KEY }}
+          CONFIG__MODEL: "openai/claude-opus-5"
+          CONFIG__MODEL_WEAK: "openai/qwen3.7-flash"
Relevance

●● Moderate

Duplication is explicitly intentional due to GitHub Actions limitations; maintainability risk is
subjective without close precedent.

PR-#23

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Both attempt steps define the same model/router/token configuration via duplicated env: blocks,
making it easy for the two attempts to diverge unintentionally over time.

.github/workflows/pr-agent.yml[95-112]
.github/workflows/pr-agent.yml[130-147]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Attempt 1 and attempt 2 duplicate a large `env:` block. This increases the chance of configuration drift between attempts.

## Issue Context
GitHub Actions supports `env:` at the job level, which can be inherited by all steps, reducing duplication.

## Fix Focus Areas
- .github/workflows/pr-agent.yml[70-112]
- .github/workflows/pr-agent.yml[121-147]

## Suggested change
Define the shared `env:` once at `jobs.pr_agent.env:` (or at least at `steps` via a composite action), and keep only the minimal per-step differences (e.g., `id`, `if`, `timeout-minutes`) in each attempt step.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
Review mode: ⚖️ Balanced: This materially changes a public repository's GitHub Actions workflow, including permissions, event filters, concurrency, retries, timeouts, secrets, and failure classification; it is behaviorally and operationally risky but localized enough for one careful review.

Grey Divider

Tip of the day
💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread .github/workflows/pr-agent.yml
Comment thread .github/workflows/pr-agent.yml Outdated
…ate reusable workflow

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) <noreply@anthropic.com>
@yakimoto
yakimoto force-pushed the ci/adopt-inline-pr-agent branch from 3818080 to 7518c97 Compare August 23, 2026 22:39
@cursor

cursor Bot commented Aug 23, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_3c532a69-f777-4f5c-a4cc-c31d488bca0d)

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 7518c97

@qodo-code-review

Copy link
Copy Markdown

Qodo Fixer

✅ Merged (0) · ☑ Fixed (0)

Process

  • No fixes were applied (no_fixes_applied)

…t classification

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
@cursor

cursor Bot commented Aug 24, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_95c6c927-f406-4eb7-b945-a00dd0357114)

@codeant-ai codeant-ai Bot added size:L This PR changes 100-499 lines, ignoring generated files and removed size:L This PR changes 100-499 lines, ignoring generated files labels Aug 24, 2026
…s step cap

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/<n>.pull_request` carries exactly [diff_url, html_url, merged_at,
   patch_url, url], while `pulls/<n>.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) <noreply@anthropic.com>
@cursor

cursor Bot commented Aug 24, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_abbf5067-bc19-4b5b-be6e-7535c1bcbee2)

…ce of a true

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) <noreply@anthropic.com>
@cursor

cursor Bot commented Aug 24, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_fd0b5eac-ecc9-4e5e-812c-ec9b62410397)

…mber space

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) <noreply@anthropic.com>
@cursor

cursor Bot commented Aug 24, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_663b918b-a090-48d3-b012-0a501cf20ab0)

@codeant-ai codeant-ai Bot added size:L This PR changes 100-499 lines, ignoring generated files and removed size:L This PR changes 100-499 lines, ignoring generated files labels Aug 24, 2026
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) <noreply@anthropic.com>
@cursor

cursor Bot commented Aug 24, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_9c68c375-8539-4962-8408-f0372f2d4b8d)

@yakimoto

Copy link
Copy Markdown
Contributor Author

Review disposition — every finding on this PR, actioned or refuted with evidence

This branch has been updated. Reviews across this wave raised nine distinct findings; four were real and are fixed, three are working-as-intended, two are refuted with measurements. Nothing was silently skipped.

Source of truth for the template: wave-av/wave-foundation-public#73. Tracked as wave-pen#418.

Fixed — the reviewers were right

finding disposition
Fork gate fails openif: … != 'true' grants on an empty output Accepted. The gate wrote an output on every path, and a gate step that errors is caught by the consumer's implicit success(), so it did not fail open in practice — but the safety rested on an argument rather than the structure. The gate now writes once, from a variable that starts at the refusing value, and the consumer requires == 'false'. Two independent changes, so neither carries the invariant alone.
issue_comment has no fork check / secrets on fork PRs Accepted, and the cause is structural. Fork status is absent from an issue_comment payload — issues/<n>.pull_request carries exactly [diff_url, html_url, merged_at, patch_url, url], while pulls/<n>.head.repo.fork answers. A fork gate step now asks the pulls endpoint.
AI timeout exceeds step timeout Accepted. CONFIG__AI_TIMEOUT: "600" inside a 360s step is unreachable — the runner killed the step first, so pr-agent never fell back to CONFIG__FALLBACK_MODELS. Now 300, in both env blocks.
issue_comment concurrency collision (PR #N vs Issue #N) Accepted, and it is a genuinely separate axis from wave-pen#386. PRs and Issues share one number sequence, and concurrency is evaluated before the job if: — so a comment on Issue #30 cancelled a /review on PR #30 and then skipped itself. The key now carries a pr/issue discriminator.
Verdict ELAPSED is cumulative, not per-attempt Already fixed in wave-foundation-public#72, before this wave. Fixing the fork gate then exposed a fourth defect in the same step, see below.
Missing CHANGELOG entry Accepted — and widened. Raised on one repo; 25 of the 28 in this wave share the Keep-a-Changelog convention, so the entry landed in all 25. Fixing the reported instance and leaving the class is the pattern this wave keeps undoing.

Refuted — with the measurement, not an opinion

The-PR-Agent/pr-agent is an unverified org / possible typosquat. The best-reasoned finding here, and it inverts on checking. All three names are one repository:

$ gh api repos/qodo-ai/pr-agent   --jq .full_name    ->  The-PR-Agent/pr-agent
$ gh api repos/Codium-ai/pr-agent --jq .full_name    ->  The-PR-Agent/pr-agent
   stars=12688  created=2023-07-05  fork=false  parent=none
   description: "PR Agent: The Original Open-Source PR Reviewer. This project is not the Qodo free tier."

GitHub is following an org rename (Codium-ai → qodo-ai → The-PR-Agent) transparently. fork: false with no parent rules out a fork; 12.7k stars and a 2023 creation date rule out a fresh typosquat. The pinned SHA resolves to the same object through either name.

The suggested remedy would make things worse: qodo-ai/pr-agent is a stale name that resolves only via redirect, and a released org name can be re-registered by anyone. Pinning to the current name plus a commit SHA is the stronger position. Keeping as is.

A2 arithmetic is broken — bare ATTEMPT2_START is a literal token. Not so; POSIX arithmetic expansion evaluates a bare name as a variable:

$ ATTEMPT2_END=1000 ATTEMPT2_START=820 bash -c 'echo $(( ${ATTEMPT2_END:-0} - ATTEMPT2_START ))'
180
$ ATTEMPT2_END=1000 ATTEMPT2_START=820 bash -c 'echo $(( ${ATTEMPT2_END:-0} - ${ATTEMPT2_START:-0} ))'
180

Normalised anyway. An expression that reads wrong on 27 repos gets re-filed on 27 repos, so consistency is worth more than being right about it.

Committable suggestions lack contents: write. Not reproduced. Committable suggestions are GitHub ```suggestion blocks posted through the pull-requests API — the human clicks "Commit suggestion" and GitHub commits under their identity; the workflow never pushes. pull-requests: write is granted. Checked against a live run rather than argued: api-spec run `32733642988` ran with `Contents: read` and `commitable_code_suggestions: true`, concluded success, posted 3 comments, and its log contains no permission error (the one `403` substring match is inside a docker layer digest).

Working as intended

  • No actions/checkout. Deliberate, and load-bearing: it is exactly why the fork finding is defence-in-depth rather than a live exfiltration path. Fork code is never fetched or executed.
  • A neutral message with a success exit. pr-agent is an advisory reviewer — it annotates, it never gates correctness — so a flaked reviewer must not block a PR. That is wave-foundation-public#3128's whole point.
  • A gh api rate limit treated the same as a real fork. That is the fail-closed design. "I could not tell" must not reach the same answer as "not a fork" on the arm holding OPENAI_KEY; the cost of erring this way is one skipped advisory review.

One defect no reviewer found, surfaced by fixing the first

stamp attempt 2 end runs under if: always(), so when attempt 2 never ran the verdict subtracted from zero:

::warning::pr-agent TIMED OUT — the longest attempt ran 1787580408s against a 360s per-attempt budget

A 56-year attempt, stated as a confident diagnosis. Latent since #72; the fork gate is simply the first path that reaches it. Fixed at the arithmetic, and the verdict gained an explicit skipped branch.

Receipts

actionlint clean · zizmor --persona=regular clean · shellcheck clean on both new run: blocks. The gate was driven through all six branches plus the no-output scenario — only a literal false reaches AGENT RUNS. The verdict was executed old-vs-new across six states; the five that already worked are byte-identical. Live: wave-av/api-spec merged an earlier revision and its pull_request run executed fork gate (issue_comment only) -> success in production and then ran the agent, so the gate does not wrongly refuse a legitimate same-repo PR.

@yakimoto

Copy link
Copy Markdown
Contributor Author

One correction to the disposition comment above: it tabulated eight findings and omitted one that was raised on this PR — gitar-bot's "Bot-login substring check misses non-[bot]-suffixed bot accounts". I resolved that thread along with the others, which was wrong of me: a resolved thread with no recorded verdict is a finding deleted, not a finding answered.

It is filed as wave-pen#419 with the reasoning, so it survives this PR merging.

Short version: real, pre-existing, and deliberately not fixed here. !contains(user.login, '[bot]') matches the suffix GitHub appends to App identities, not a plain machine user account. But the guard is a cost/noise control, not a security one — the lane runs no actions/checkout, and a non-[bot] automation account that can open PRs here is already inside the author_association allowlist the issue_comment arm trusts. So the failure mode is a wasted review.

The issue also records the trap: gating on user.type == 'Bot' looks like the principled fix and is behaviourally equivalent to the substring — a machine account still reports User. The recommendation is to measure how many non-[bot] automation PRs the public fleet actually sees over 30 days before changing anything, and to close it as a non-finding if that number is ~0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant