Conversation
Re-syncs this repo to wave-foundation-public#72, which landed after the inline lane was adopted here. THE DEFECT. The adopted template stamped AGENT_START once, before attempt 1, then compared TOTAL job time — attempt 1 + the 45s backoff + attempt 2 — against STEP_BUDGET_S=360, a budget its own comment calls PER-ATTEMPT. Two healthy-but-slow attempts (~180s each, ~405s together) therefore reported "pr-agent TIMED OUT ... A hang, NOT a rate limit." sending the next reader to debug a hang that never happened; the else-branch lied the other way, asserting 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 before acting. THE FIX. Stamp each attempt separately and classify on the LONGEST attempt, with if: always() end stamps so an attempt killed BY its step timeout still records one — exactly the case the classifier exists to catch. Total wall time is still reported as context but no longer decides the verdict. NOT URGENT, NOT IGNORABLE. The defect is in a MESSAGE, not in behaviour: the lane still retries, still renders NEUTRAL, still never blocks a PR. But that verdict step exists precisely because "a confidently wrong cause is worse than no cause", so shipping a classifier that can misname a hang defeats its purpose. Job id pr_agent and every on: trigger unchanged — the job id is the check-run context and branch protection matches on it. Refs wave-av/wave-pen#417, wave-av/wave-pen#388
🤖 CodeAnt AI — Review Status
|
Bugbot couldn't run - usage limit reachedBugbot 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_a168353a-1914-4225-bc65-a195dc3753ef) |
|
Warning Review limit reachedNext included review available in 28 minutes. View limit detailsLimit 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. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Comment |
codeant-ai
Bot
added
the
size:M
ApprovabilityVerdict: Would Approve Macroscope's review found this PR approvable — This is a one-file CI diagnostic fix that records per-attempt timings and corrects timeout messaging while preserving retry behavior, action configuration, triggers, and neutral non-blocking handling. Not approved because:
Review your spending limits in Billing settings. You can add or adjust custom eligibility rules. Learn more. |
PR Summary by QodoFix pr-agent timeout classification to be per-attempt (not total job time)
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
|
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. Code Review ✅ ApprovedRefactors PR-agent timeout classification to measure duration per attempt rather than total job time, preventing false hang diagnoses on slow retries. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
Code Review by Qodo
1. Missing pr-agent changelog entry
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+216
to
+217
| 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)." |
There was a problem hiding this comment.
1. Missing pr-agent changelog entry 📘 Rule violation ⚙ Maintainability
This PR changes the pr_agent check’s user-visible warning classification/text for timeout vs failure in .github/workflows/pr-agent.yml, but CHANGELOG.md has no corresponding entry under `## [Unreleased]`. This violates the requirement to document user-facing changes in the changelog.
Agent Prompt
## Issue description
The PR introduces a user-visible change to PR check output/classification (`pr_agent` timeout vs failure messaging) but does not add an entry under `## [Unreleased]` in `CHANGELOG.md`.
## Issue Context
The compliance checklist requires updating `CHANGELOG.md` for user-facing changes.
## Fix Focus Areas
- CHANGELOG.md[7-29]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+207
to
+208
| A1=$(( ${ATTEMPT1_END:-0} - ${ATTEMPT1_START:-0} )) | ||
| A2=$(( ${ATTEMPT2_END:-0} - ${ATTEMPT2_START:-0} )) |
There was a problem hiding this comment.
2. Attempt2 duration inflated 🐞 Bug ≡ Correctness
The workflow stamps ATTEMPT2_END unconditionally but only stamps ATTEMPT2_START when the retry is scheduled, so A2 can become a large positive value (END-0) if attempt 2 never actually started. That can drive LONGEST above the timeout threshold and incorrectly emit the "TIMED OUT" classification for non-timeout failures.
Agent Prompt
## Issue description
`ATTEMPT2_END` is stamped with `if: always()` even when attempt 2 never runs, while `ATTEMPT2_START` is only stamped when the retry is scheduled. In the verdict step, `A2=$((ATTEMPT2_END - ATTEMPT2_START))` then becomes a large positive value when `ATTEMPT2_START` is unset (defaults to 0), which can incorrectly make `LONGEST` exceed the step budget and misclassify a non-timeout failure as `TIMED OUT`.
## Issue Context
- `stamp attempt 2 start` is conditional on `steps.agent.outcome == 'failure'`.
- `stamp attempt 2 end` currently runs unconditionally (`if: always()`), so it can set `ATTEMPT2_END` even when attempt 2 never started.
- Verdict arithmetic defaults missing vars to `0`, which turns the missing-start case into a huge positive duration.
## Fix Focus Areas
- .github/workflows/pr-agent.yml[136-169]
- .github/workflows/pr-agent.yml[206-217]
## Suggested changes
1) Make `stamp attempt 2 end` conditional on attempt 2 being intended (while still using `always()`):
- Example: `if: always() && steps.agent.outcome == 'failure'`
2) Defensively compute `A2` as `0` unless `ATTEMPT2_START` is set to a non-zero epoch value (or unless `AGENT_RETRY_OUTCOME` is non-empty):
- Example:
- `A2=0`
- `if [ "${ATTEMPT2_START:-0}" -gt 0 ] && [ "${ATTEMPT2_END:-0}" -gt 0 ]; then A2=$((ATTEMPT2_END-ATTEMPT2_START)); fi`
This keeps `LONGEST` meaningful and prevents false timeout classification when the retry didn’t actually run.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Qodo Fixer✅ Merged (0) · ☑ Fixed (0) Process
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Reviewer's GuideUpdates the pr-agent workflow to distinguish per-attempt timeouts from failures across the complete retry sequence by timestamping each attempt, classifying on the longest attempt with runner-timeout slack, and retaining total wall time only for context. Retry behavior and advisory, non-blocking job semantics remain unchanged. Sequence diagram for per-attempt pr-agent timeout classificationsequenceDiagram
participant Workflow
participant Agent as PR-Agent
participant Verdict
Workflow->>Workflow: stamp attempt 1 start
Workflow->>Agent: run attempt 1
Workflow->>Workflow: stamp attempt 1 end
alt attempt 1 fails
Workflow->>Workflow: sleep 45s
Workflow->>Workflow: stamp attempt 2 start
Workflow->>Agent: run attempt 2
Workflow->>Workflow: stamp attempt 2 end
end
Workflow->>Verdict: compare longest attempt with STEP_BUDGET_S - 15s
alt longest attempt reaches timeout threshold
Verdict-->>Workflow: render NEUTRAL: TIMED OUT
else neither attempt reaches threshold
Verdict-->>Workflow: render NEUTRAL: failed, likely rate limit
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Re-syncs this repo to wave-foundation-public#72, which landed after the inline pr-agent lane was adopted here. Tracked as wave-pen#417.
The defect
The adopted template stamped
AGENT_STARTonce, before attempt 1, then compared total job time — attempt 1 + the 45s backoff + attempt 2 — againstSTEP_BUDGET_S=360, a budget its own comment calls per-attempt.Two healthy-but-slow attempts (~180s each, ~405s together) therefore reported:
…sending the next reader to debug a hang that never happened. The else-branch lied the other way, asserting 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 before acting.
The fix
Stamp each attempt separately and classify on the longest attempt, with
if: always()end stamps so an attempt killed by its step timeout still records one — exactly the case the classifier exists to catch. Total wall time is still reported as context but no longer decides the verdict.failed after 2 attempts✅TIMED OUT✅Not urgent, not ignorable
The defect is in a message, not behaviour — the lane still retries, still renders NEUTRAL, still never blocks a PR. But that verdict step exists precisely because "a confidently wrong cause is worse than no cause", so a classifier that can misname a hang defeats its own purpose.
Job id
pr_agentand everyon:trigger unchanged — the job id is the check-run context and branch protection matches on it.Refs wave-av/wave-pen#417, wave-av/wave-pen#388
Note
Low Risk
CI classifier/messaging only; the job still retries, stays advisory, and never blocks PRs. No application or security logic.
Overview
Fixes the pr-agent verdict step so hang vs rate-limit is decided from per-attempt duration, not wall-clock time across both tries plus backoff.
Previously a single
AGENT_STARTstamp made two slow-but-healthy ~180s attempts (~405s together) look like a step timeout against the 360s per-attempt budget. The workflow now stamps start/end for each attempt (if: always()so a killed step still records an end), classifies on the longest attempt with 15s slack, and still reports total wall time only as context. Retry, NEUTRAL rendering, and job id are unchanged.Reviewed by Cursor Bugbot for commit 64a39a9. Bugbot is set up for automated code reviews on this repo. Configure here.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by Sourcery
Classify pr-agent timeouts using per-attempt duration so retries and backoff no longer cause healthy slow runs to be misreported as hangs.
Bug Fixes:
Enhancements:
CI: