Skip to content

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

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#30
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

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

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


Note

Medium Risk
CI workflow now holds OPENAI_KEY inline and adds fork/slash-command gating; a mistake could leak cost or skip reviews, but it does not change product auth or runtime code.

Overview
Makes the advisory pr-agent check actually run on this public repo. GitHub cannot call a reusable workflow in private wave-foundation, so the old uses: died with no job and a silent red check.

The workflow is now an inline copy of the reusable lane: Qodo Merge on OPENAI_KEY, event-keyed concurrency (so comments no longer cancel in-flight reviews), a fail-closed fork gate for / commands, 6-minute per-attempt timeouts with a 45s retry, and a verdict step that treats reviewer flakes as neutral so they never block PRs.

Also tightens CONFIG__AI_TIMEOUT to 300s so the agent can fall back before the runner kills the step, and fixes verdict arithmetic when attempt 2 never ran. /review on fork PRs is now declined with a warning.

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

Summary by Sourcery

Make the public repository’s PR-Agent check execute reliably and safely without depending on a private reusable workflow.

Bug Fixes:

  • Replace the unusable private reusable-workflow reference with an inline PR-Agent workflow for public repositories.
  • Prevent fork-based slash-command reviews from using repository secrets by adding fail-closed fork detection.
  • Keep advisory PR-Agent failures, timeouts, and rate limits from blocking pull requests through bounded retries and neutral verdicts.

Enhancements:

  • Separate concurrency lanes by event and PR-versus-issue context to prevent unrelated comments from cancelling reviews.
  • Add per-attempt execution timeouts and accurate outcome classification for cancellations, skipped runs, failures, and timeouts.

CI:

  • Synchronize the public PR-Agent lane with the shared workflow’s retry, timeout, concurrency, fork-gating, and verdict behavior.

Review in cubic


PR Type

Bug fix


Description

  • Replaced private workflow reference with inline pr-agent configuration

  • Fixed concurrency group logic to prevent job cancellation conflicts

  • Added fork detection mechanism for issue_comment events

  • Implemented retry logic with timeout management

  • Enhanced verdict classification for failed attempts


Diagram Walkthrough

flowchart LR
  A["pr-agent.yml"] --> B[Concurrency group fix]
  A --> C[Fork detection]
  A --> D[Retry logic]
  A --> E[Verdict classification]
Loading

File Walkthrough

Relevant files
Enhancement
pr-agent.yml
Complete workflow reimplementation with reliability improvements

.github/workflows/pr-agent.yml

  • Replaced private workflow reference with inline configuration
  • Added detailed concurrency group logic to prevent job cancellation
  • Implemented fork detection for issue_comment events
  • Introduced retry mechanism with timeout management
  • Enhanced verdict classification for failed attempts
+325/-6 
Documentation
CHANGELOG.md
Updated changelog with workflow improvements                         

CHANGELOG.md

  • Documented fork handling improvements
  • Recorded timeout configuration fixes
  • Added retry logic documentation
  • Noted concurrency group changes
+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 4f196b2 Aug 24, 2026 · 14:36 14:37
✅ Incremental review completed bfad95d Aug 24, 2026 · 13:35 13:36
✅ Reviewed your PR 7a7d15b Aug 23, 2026 · 22:30 22:31

@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_233d1f62-db85-44f4-85e7-0e833a0e3645)

@sourcery-ai

sourcery-ai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Reviewer's Guide

Replaces the broken pr-agent reusable workflow call with a fully inline, self-contained workflow tailored for public repos, while re-syncing it with the current reusable lane behavior (event-keyed concurrency, timeouts, retry, and non-blocking verdict classification).

Sequence diagram for the resilient PR-Agent review workflow

sequenceDiagram
    participant GitHub
    participant Workflow
    participant Agent as PR-Agent
    participant Verdict

    GitHub->>Workflow: Trigger pull_request or issue_comment
    Workflow->>Workflow: Evaluate event-specific concurrency and job conditions
    Workflow->>Agent: Run attempt 1 with 6-minute timeout
    alt Attempt 1 succeeds
        Agent-->>Verdict: success
    else Attempt 1 fails
        Workflow->>Workflow: Sleep 45 seconds
        Workflow->>Agent: Run retry attempt with 6-minute timeout
        Agent-->>Verdict: success or failure
    end
    Verdict->>Verdict: Classify success, cancellation, timeout, or reviewer failure
    Verdict-->>GitHub: Advisory result, workflow faults remain visible
Loading

File-Level Changes

Change Details Files
Adopt an inline, self-contained pr-agent workflow for public repos with updated concurrency, timeouts, retry logic, and non-blocking verdict classification instead of calling a private reusable workflow.
  • Replace the reusable workflow invocation with a full inline job definition that runs The-PR-Agent/pr-agent directly using OPENAI_KEY and related configuration.
  • Adjust concurrency group key to include github.event_name so pull_request and issue_comment events don’t cancel each other while still superseding same-event runs.
  • Add job-level and step-level timeout budgets to distinguish hung runs from normal failures and keep retries within a bounded time window.
  • Introduce a guarded if: condition to allow slash-command reviews from trusted members and automatic reviews on non-bot, non-draft, non-fork pull requests.
  • Add a retry path with backoff on pr-agent failures, duplicating environment configuration for the second attempt.
  • Add a final verdict step that always runs, classifies outcomes (success, never-ran, cancelled, timed-out, failure), logs them as warnings or errors, and ensures pr-agent remains advisory and does not block merges.
.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

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 27 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: 171f8091-ca85-4f20-87cb-e863723d04bf

📥 Commits

Reviewing files that changed from the base of the PR and between 4969e2c and 73b9139.

📒 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
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix pr-agent workflow for public repos by inlining lane and adding retries/timeouts

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

Grey Divider

AI Description

• Replace private reusable workflow call with a self-contained inline pr-agent job
• Fix concurrency so issue comments don’t cancel in-flight pull_request reviews
• Add step budgets, retry/backoff, and a non-blocking verdict classifier for flaky LLM runs
Diagram

graph TD
  A["GitHub Event"] --> B{ "Trigger" }
  B -->|"pull_request"| C["Workflow: pr-agent"] --> D["PR-Agent action"] --> E["Retry/backoff"] --> F["Verdict step"]
  B -->|"issue_comment (/...)"| C
  C --> G{ "Concurrency group" } --> C
  subgraph Legend
    direction LR
    _evt["Event"] ~~~ _wf["Workflow/job"] ~~~ _act["Action step"] ~~~ _dec{"Decision"}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Make the reusable workflow public (or mirror it in a public repo)
  • ➕ Keeps a single SSOT workflow to reduce drift across repos
  • ➕ Simplifies per-repo workflow files to a single uses: reference
  • ➖ Not possible if the canonical source must remain private
  • ➖ Still subject to GitHub’s public→private reusable workflow restriction unless visibility changes
  • ➖ Requires governance/controls around exposing workflow logic publicly
2. Package the lane as a composite action instead of a reusable workflow
  • ➕ Composite actions can be published from a public repo and consumed broadly
  • ➕ Better versioning story (tags/releases) and less YAML duplication per consumer
  • ➖ More effort to refactor (inputs/outputs, permissions, docs)
  • ➖ Some workflow-level features (permissions/concurrency) still live in each repo’s workflow
3. Centralize via a GitHub App / external service for reviews
  • ➕ Eliminates per-repo workflow complexity and secrets handling in Actions
  • ➕ More consistent behavior and observability across repos
  • ➖ Much larger architecture change and operational burden
  • ➖ Harder to iterate compared to workflow-based adoption
  • ➖ May not align with current OSS PR-Agent deployment model

Recommendation: Adopt the inline workflow as done here for public repos: it directly fixes the hard failure mode (no jobs/logs) caused by public→private reusable workflow calls, while also addressing known operational issues (event-aware concurrency, step budgets, retry/backoff, and a non-blocking verdict for an advisory reviewer). If drift becomes a recurring problem, consider migrating to a public composite action as a follow-up to restore SSOT without violating visibility constraints.

Files changed (1) +166 / -6

Other (1) +166 / -6
pr-agent.ymlInline pr-agent job for public repos with concurrency fix, retries, and verdict +166/-6

Inline pr-agent job for public repos with concurrency fix, retries, and verdict

• Replaces the private reusable workflow reference with an inline, self-contained pr_agent job suitable for public repositories. Updates concurrency grouping to include the event name to prevent issue_comment runs from cancelling pull_request reviews. Adds job/step timeouts, a retry with backoff, and a verdict step that classifies outcomes and avoids blocking PRs on advisory reviewer flakes.

.github/workflows/pr-agent.yml

@macroscopeapp

macroscopeapp Bot commented Aug 23, 2026

Copy link
Copy Markdown

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — The PR turns a previously nonfunctional reusable-workflow reference into an active, secret-bearing PR automation lane with external LLM calls, PR write permissions, comment triggers, retries, and fork/concurrency gating. Although the author owns the affected files and has relevant CI history, the resulting runtime and security-sensitive workflow behavior warrants human review.

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 03cb6b5)

Here are some key observations to aid the review process:

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

Env Key Drift

The retry step's env block does not use the same API-key variable name as attempt 1 (OPENAI_KEY); the first entry of the retry block appears to be a different key name bound to secrets.OPENAI_KEY. If the name really differs, attempt 2 runs without a credential and will always fail, defeating the retry that this PR adds. This may be a redaction artifact in the diff — verify the two env blocks are byte-identical as the accompanying comment requires.

env:
  OPENAI_KEY: ${{ secrets.OPENAI_KEY }}
Misclassified Verdict

AGENT_START is stamped once before attempt 1, but ELAPSED is compared against STEP_BUDGET_S (360s), which is documented as a per-attempt budget. Because elapsed time spans attempt 1 + 45s backoff + attempt 2, two slow-but-genuine errors (e.g. 200s each) total ~445s and are reported as "pr-agent TIMED OUT" even though neither attempt hit its step timeout — exactly the confidently-wrong cause attribution the comment says it wants to avoid. Re-stamping the start before the retry, or comparing against the cumulative budget, fixes this.

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)."

@github-actions

Copy link
Copy Markdown

PR Code Suggestions ✨

No code suggestions found for the PR.

@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

Adopts the self-contained inline PR-Agent workflow for public repositories to resolve private workflow invocation errors. No issues found.

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

@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. This makes a pinned external action run with the repository's OpenAI secret and pull-request write permission, so a faulty configuration could post unwanted reviews or suggestions and those artifacts would remain after a revert. The workflow can be reverted to stop future runs, but already-published comments or suggestions would need separate cleanup.


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

qodo-code-review Bot commented Aug 23, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Fork PRs not skipped ✓ Resolved 🐞 Bug ⛨ Security
Description
The job-level if: claims forks are skipped, but the issue_comment (slash-command) branch does
not check head.repo.fork, so /… comments can run the workflow for fork PRs. This contradicts the
workflow’s documented behavior and can unintentionally run pr-agent (with secrets available on
issue_comment) against forked PRs.
Code

.github/workflows/pr-agent.yml[R58-64]

+    # 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))
Evidence
The workflow documentation says forks are skipped, but the actual if: condition only enforces
head.repo.fork == false for pull_request events, not for issue_comment events triggered by
slash commands.

.github/workflows/pr-agent.yml[58-69]

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 `issue_comment` slash-command path lacks a fork check, despite the comment stating “Forks skipped (no secrets there).” This can allow `/…` commands on fork PRs to run the job, which is inconsistent with intended behavior and may expose secrets depending on GitHub event semantics.

### Issue Context
The fork guard exists only for the `pull_request` path (`github.event.pull_request.head.repo.fork == false`) but not for the `issue_comment` path.

### Fix Focus Areas
- .github/workflows/pr-agent.yml[58-69]

### Proposed change
Add a fork check to the `issue_comment` clause by resolving the PR and checking `head.repo.fork == false`, or (if you intentionally want to allow member-triggered reviews on forks) update the comment and add explicit hardening (e.g., never checkout/execute PR code, restrict commands, and document that secrets are used).

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



Remediation recommended

2. AI timeout exceeds step timeout ✓ Resolved 🐞 Bug ☼ Reliability
Description
The pr-agent step is capped at 6 minutes, but CONFIG__AI_TIMEOUT is set to 600 seconds (10
minutes), so the action can be killed by the step timeout before its own AI timeout triggers. This
increases forced timeouts and undermines the intended retry/verdict behavior.
Code

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

+        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"
Evidence
The workflow defines a 6-minute step timeout while configuring the action to wait up to 600 seconds
for AI operations, which cannot complete if the runner kills the step earlier.

.github/workflows/pr-agent.yml[88-104]

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

### Issue description
`timeout-minutes: 6` (360s) conflicts with `CONFIG__AI_TIMEOUT: 600`. If the action waits up to 10 minutes internally, the workflow will terminate it at 6 minutes, producing step-level cancellations/timeouts rather than controlled failures.

### Issue Context
The verdict logic assumes a 6-minute per-attempt budget and tries to classify timeouts vs fast errors.

### Fix Focus Areas
- .github/workflows/pr-agent.yml[88-104]
- .github/workflows/pr-agent.yml[121-138]

### Proposed change
Set `CONFIG__AI_TIMEOUT` to a value safely below the step budget (e.g., 300–330s) and/or raise the step timeout to exceed `CONFIG__AI_TIMEOUT` (while keeping the overall job budget constraints intact). Apply the same change to both attempt blocks.

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


3. Timeout verdict misclassified ✓ Resolved 🐞 Bug ◔ Observability
Description
The verdict step compares total elapsed time since workflow start (including the 45s backoff and
both attempts) against a per-attempt budget (360s), so many normal “attempt 2 ran” failures will be
labeled as a timeout. This produces incorrect diagnostics and can hide real upstream errors behind
the “TIMED OUT” message.
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
Evidence
The workflow sets STEP_BUDGET_S to 360s (matching the 6-minute step timeout) but computes
ELAPSED from a single start stamp and then includes a 45s sleep and possible second attempt before
comparing to 360s.

.github/workflows/pr-agent.yml[72-77]
.github/workflows/pr-agent.yml[117-126]
.github/workflows/pr-agent.yml[155-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 timeout classification uses `ELAPSED` since `AGENT_START` and checks it against `STEP_BUDGET_S` (360s). Because the workflow includes a 45s backoff and potentially a second 6-minute attempt, `ELAPSED` will often exceed 360s even when neither attempt was killed by step timeout. This mislabels failures as timeouts.

### Issue Context
`STEP_BUDGET_S` is explicitly described as the *per-attempt* step timeout, but the computed elapsed covers more than one attempt.

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

### Proposed change
Track per-attempt start times (e.g., stamp before attempt 1 and attempt 2 separately) and compare each attempt’s elapsed time to `STEP_BUDGET_S`. Alternatively, infer timeout by checking step conclusion details available in the runner logs, or record a flag when the retry step starts and adjust the budget comparison accordingly (e.g., `>= STEP_BUDGET_S + 45` when retry ran).

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


Grey Divider

Context sources
✅ Compliance rules (platform): 2 rules
✅ REVIEW.md
Review mode: ⚖️ Balanced: This is a behavior-changing CI workflow with substantial new retry, timeout, concurrency, permissions, and failure-classification logic; it is risky but localized enough for one careful review rather than redundant passes.

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 Outdated
Comment thread .github/workflows/pr-agent.yml Outdated
Comment thread .github/workflows/pr-agent.yml
@qodo-code-review

Copy link
Copy Markdown

Qodo Fixer

✅ Merged (0) · ☑ Fixed (0)

Process

  • ⏭ Skipped (1)

…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 7a7d15b to 03cb6b5 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_78e6be17-2772-4581-b169-a571aed1a928)

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 03cb6b5

@yakimoto
yakimoto enabled auto-merge August 24, 2026 13:20
…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_a40868a8-bb64-4916-9ef5-4f12f8295bf3)

@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_06ef41ad-b3d0-4ea3-aa02-b97c95f16f4c)

@yakimoto

Copy link
Copy Markdown
Contributor Author

Both qodo findings on this PR are fixed, and the branch here has been re-synced to the corrected template — the diff above now includes them.

  • Fork PRs not skipped. Correct, and the cause turned out to be structural rather than an omission: fork status is absent from an issue_comment payload, so the job-level if: could never have expressed it. 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. The check moved to a fork gate step that 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. Scoping it honestly: this lane runs no actions/checkout, so fork code was never fetched or executed — the reachable surface was the fork diff going to the LLM router on our key, already narrowed by the author_association allowlist. The durable defect was the comment claiming "Forks skipped (no secrets there)", which was true of one arm and false of the other.

  • AI timeout exceeds step timeout. Correct and unqualified. CONFIG__AI_TIMEOUT: "600" inside a timeout-minutes: 6 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. Now 300, in both env blocks.

Fixing the first one also exposed a third defect neither review caught: stamp attempt 2 end runs under if: always(), so when attempt 2 never ran the classifier subtracted from zero and reported TIMED OUT — the longest attempt ran 1787580408s. A 56-year attempt, stated as a confident diagnosis. Fixed at the arithmetic.

Upstream: wave-av/wave-foundation-public#73 · tracked as wave-pen#418. Thanks — the first finding was worth the catch.

…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_4e8ab551-3482-44bf-811e-b68ad2423559)

…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_c110ad18-b56f-4e36-bb05-c44752829620)

@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_34b7461f-ef40-4852-afb1-134a02091ead)

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

Labels

Review effort 2/5 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