Repository navigation
PR Review Agent — failures detected 2026-07-20 #1329
Copy link
Copy link
Closed
Labels
automated-reportCreated by automated workflowCreated by automated workflowhealth-checkAutomated health check reportAutomated health check report
Description
### 1. Executive Summary
**Status:** HEALTHY
**Period:** 2026-07-19T10:07Z – 2026-07-20T08:12Z (~22 hours)
**Result:** 2 of 100 runs failed (2.0%)
Key findings:
- Both failures (#57317, #57319) occurred in a tight cluster at ~01:58Z on 2026-07-20 — pointing to a transient event, not a systemic regression
- 5 cancellations form two distinct clusters (4× at 22:52Z, 1× at 17:50Z), consistent with expected concurrency-group preemption on rapid push events — not failures
- 7 skipped runs follow the same burst-event pattern; skips appear to be the conditional gate in `pr-review.yml` rejecting non-target triggers
Action required: Retrieve and inspect the raw logs for runs #57317 and #57319 to confirm the failure was transient; no code change is indicated yet.
---
### 2. Failure Breakdown
| Failure Category | Affected Runs | Example Error Message |
|---|---|---|
| Unknown / log not captured | #57317, #57319 | *(No log content was returned for failed runs — see §3)* |
| Concurrency preemption (cancellation) | #57258, #57298–#57301 | N/A — expected `concurrency: cancel-in-progress` behaviour |
| Conditional skip (non-target trigger) | #57228, #57231, #57234, #57279, #57308, #57310, #57318 | N/A — `if:` gate evaluated false |
---
### 3. Error Patterns
**Category: Unknown (log not captured)**
The prompt's `## Failed Run Logs` section contained no log content for runs #57317 and #57319. Without that output, the specific step and error message cannot be confirmed. The following observations narrow the likely cause:
- Both runs fired at 01:58:53Z and 01:58:54Z — a one-second spread, consistent with a single `pull_request` or `check_suite` event that fanned out two concurrent jobs before the concurrency group could cancel one.
- Run #57318 between them has status `skipped`, suggesting the conditional gate ran but the surrounding job context was unhealthy at that moment.
- No failures appear in any other hour of the 22-hour window, ruling out a persistent API, auth, or rate-limit problem.
**Most probable causes (ranked):**
1. **Transient GitHub API error** — a momentary 5xx from the `gh` CLI or REST call inside `engine.sh` / `review-one-pr.sh` at ~01:58Z.
2. **Engine (Claude/Copilot) rate limit spike** — a burst of late-night runs (six successes between 01:42Z–01:54Z) could have exhausted per-minute quota, causing the 01:58Z pair to fail.
3. **check_suite race** — a `check_suite: completed` event for a suite that contained the workflow itself arrived before the previous run finished, producing an inconsistent state that the reusable's guard condition did not handle.
**Category: Concurrency preemption (expected)**
`pr-review-trigger.yml` does not define a `concurrency:` block, but the called reusable `pr-review.yml@pr-review/next` likely does. The four-run cancellation cluster at 22:52Z (runs #57298–#57301, all within 10 seconds) followed immediately by a success (#57297 at 22:47Z) is the normal pattern when several pushes arrive in rapid succession and the concurrency group kills stale runs. This is healthy behaviour and not a defect.
---
### 4. Token Scope Analysis
The workflow source grants:
| Permission | Level | Purpose |
|---|---|---|
| `contents: read` | Present | Checkout, script access |
| `pull-requests: write` | Present | Post review comments |
| `checks: read` | Present | Read check suite status |
**Observations from available data:**
- No `gh auth status` output was included in the logs (logs were not captured), so runtime scope cannot be confirmed directly.
- The `secrets: inherit` pass-through means the called reusable receives all org/repo secrets, including `GH_PAT_DON_PETRY` (after the #1328 collapse). If that PAT lacks `read:org` or `repo` scope, intermittent auth failures on the org-level PR enumeration calls in `engine.sh` would surface as sporadic failures — matching the pattern seen here.
- No `403` or `401` signatures are visible because log content was not provided, so a scope gap cannot be confirmed or ruled out.
| Scope | Status | Recommendation |
|---|---|---|
| `repo` (full) | Assumed present (PAT) | Verify `GH_PAT_DON_PETRY` includes `repo` scope via `gh auth status -t` in a debug run |
| `read:org` | Unverified | Required if `engine.sh` calls `/orgs/{org}/members` or similar; confirm via PAT settings |
| `pull-requests: write` (GITHUB_TOKEN) | Present (workflow grants it) | No change needed |
---
### 5. Recommendations
1. **Capture failed-run logs** — Re-trigger or download artifact/log for runs #57317 and #57319 via `gh run view 57317 --log-failed` and `gh run view 57319 --log-failed`. This is the only way to confirm root cause.
- *Why:* All other analysis is speculative without the actual error line.
- *Expected impact:* Unblocks accurate diagnosis.
- *Urgency:* **HIGH** (no other action is justified until this is done)
2. **Add retry logic for transient API failures** — If the logs confirm a transient GitHub or engine API error, add a short retry loop (e.g., `for i in 1 2 3; do ... && break || sleep 15; done`) around the `gh` calls in `scripts/review-one-pr.sh` and `scripts/engine.sh`.
- *Why:* A 2% failure rate driven by transient errors is easily recoverable at the script level without re-queuing the whole workflow.
- *Expected impact:* Drops transient-error failures to near 0%.
- *Urgency:* **MEDIUM**
3. **Add a `concurrency:` block to the trigger stub** — Define a concurrency group keyed on PR number in `pr-review-trigger.yml` to prevent the double-fire pattern seen at 01:58Z:
```yaml
concurrency:
group: pr-review-${{ github.event.pull_request.number || github.event.check_suite.id || github.run_id }}
cancel-in-progress: true- Why: Two jobs firing within one second for the same event is the likely precondition for the 01:58Z failure pair.
- Expected impact: Eliminates duplicate-trigger races; reduces unnecessary run consumption.
- Urgency: MEDIUM
- Verify PAT scope post-feat: implement issue #1327 — Collapse GH_PAT_DON_PETRY_COPILOT → GH_PAT_DON_PETRY (single don-petry PAT for the Copilot engine) #1328 collapse — After collapsing
GH_PAT_DON_PETRY_COPILOT→GH_PAT_DON_PETRY(commit 298a1d2), confirm the single remaining PAT holds every scope the Copilot engine path previously used via the old token.- Why: Scope regressions from PAT consolidation are silent until a code path that needs the missing scope is exercised.
- Expected impact: Prevents latent auth failures in Copilot-engine code paths.
- Urgency: HIGH
6. Health Score
Health: 8.5/10 — Two isolated failures in a tight one-second cluster at 01:58Z are the only blemish on an otherwise clean 22-hour window; all other non-success outcomes are expected concurrency and skip behaviour.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
automated-reportCreated by automated workflowCreated by automated workflowhealth-checkAutomated health check reportAutomated health check report
Activity