Repository navigation
feat: implement issue #2235 — CI Failure Analyst never runs for failed GitHub Actions jobs: the check_run trigger only fires for third-party checks - #2308
Conversation
…d GitHub Actions jobs: the check_run trigger only fires for third-party checks
don-petry
added
the
auto-rebase:ready
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for the workflow_run event in the CI Failure Analyst, allowing it to diagnose failed GitHub Actions runs in addition to third-party check_run events. It adds a new payload resolution script scripts/ci-failure-analyst-resolve.sh, updates documentation and test scenarios, and implements a comprehensive BATS test suite. The review feedback suggests improving the test setup and teardown by adding error handling to the mktemp -d command and guarding the rm -rf cleanup command with a non-empty check to prevent accidental deletions.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2eb942a6c0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
CodeAnt Nitpicks2 code suggestions1. This payload path accepts closed PRs, unlike the open-only fallback. If a check finishes after its PR is merged, the analyst can post a diagnostic comment on the closed PR.Incorrect condition logic · 2. If the comments API request fails,
|
This comment has been minimized.
This comment has been minimized.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
QA Lead — test-risk advisoryRisk tier: MEDIUM. This is shared workflow automation used by every caller stub. If the new checkout or output wiring is wrong, the analyst breaks quietly in every repo once What I'd shore up (highest leverage first):
Escalate? no Advisory only — I comment, I do not change code. Ground: BMAD Test Architecture (bmad-tea v1.19.0). Opt out on this item with the qa-lead:hands-off label. |
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-10-11T21:17:24Z. |
Dev-Lead — review-changes (applied)Changes committed and pushed. Requested items addressed:
|
|
No description provided. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43241964c4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - name: Checkout analyst tooling | ||
| uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| with: | ||
| repository: petry-projects/.github-private |
There was a problem hiding this comment.
When this reusable is invoked from any consumer repository, ${{ github.token }} is scoped to that caller, so actions/checkout cannot clone the separate private petry-projects/.github-private repository (despite the comment claiming it is public). The new checkout therefore fails before payload resolution, disabling both existing check_run analysis and future workflow_run analysis for downstream callers once the channel is promoted. Keep the resolver inside the reusable workflow or supply a credential with read access to this repository.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
No change: this repo is public on purpose (see CLAUDE.md), so actions/checkout with the caller's github.token can read it; no credential is needed. Leaving open for a human to confirm.
| payload_field() { | ||
| local event_path="${GITHUB_EVENT_PATH:-}" | ||
| [ -n "$event_path" ] && [ -s "$event_path" ] || return 0 | ||
| jq -r "$1 // empty" "$event_path" 2>/dev/null || true |
There was a problem hiding this comment.
When GITHUB_EVENT_PATH is missing, empty, malformed, unreadable, or jq itself fails, this suppression returns an empty value and resolve subsequently emits skip=true with exit 0 as though the payload simply contained no supported event. That turns an inability to inspect the trigger into a successful no-op, so the CI failure receives no diagnostic and the run can be counted as a healthy sample; distinguish a valid unsupported payload from a payload-read failure and fail loudly.
AGENTS.md reference: AGENTS.md:L168-L170
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
No change: the existing test "zero-byte, non-JSON and missing payload files all skip with exit 0" in tests/ci_failure_analyst_resolve.bats deliberately encodes skip-with-exit-0 for unreadable payloads, so this suggestion contradicts it. Leaving open for a human to decide.
|
No description provided. |
- Fix Issue 1: Filter payload PRs by open state, matching the fallback behavior (lines 73-74). Accepts only open PRs from the webhook payload, consistent with the commits-to-pulls API fallback which also filters by state == "open". - Fix Issue 2: Remove error suppression in idempotency function (line 118). Changed '|| true' to '|| return' so API failures propagate instead of silently returning empty, which could cause duplicate comment posts. - Update test fixtures to include PR state field matching real GitHub webhook payloads that include state information. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
|
Fixed in two places:
Both test fixtures and the test cases have been updated to include the field that real GitHub webhook payloads provide. |
This comment has been minimized.
This comment has been minimized.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
|
No description provided. |
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Dev-Lead — fix-reviews (partial)A commit was pushed, but not every requested change was applied. Per requested item:
The unaddressed items above still need work. |
|
No description provided. |
CI Failure: CodeQLStep: CodeQL analysis (code scanning alert, not a workflow step log) CodeQL reported 1 new medium-severity alert, "Checkout of untrusted code in a non-privileged context", in Suggested fix: In |
This comment has been minimized.
This comment has been minimized.
|
SonarCloud Quality Gate Passed: scanned PR diff for security hotspots per RSPEC guidelines — no script injection (curl|bash), hardcoded credentials, unsafe eval/exec, or insecure downloads found. The reported '10 New issues' appear to be code-style warnings in BATS test code (SC2030/SC2031 subshell var modifications), which are expected for BATS test structure and not security or functional issues. Quality Gate passed status confirmed. |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
This comment has been minimized.
This comment has been minimized.
|
No description provided. |
CI Failure: CodeQLStep: CodeQL analysis (code scanning alert, not a workflow job step) CodeQL reported 1 new medium-severity alert: "Checkout of untrusted code in a non-privileged context" in Suggested fix: In |
|
No description provided. |
|
CodeRabbit summary received, but the review was rate-limited and contains no actionable code findings. The summary describes the changes accurately, and pre-merge checks all pass. No code changes required. |
|
SonarCloud Quality Gate passed. The 10 new issues reported do not include security hotspots and are not blockers. Scanned changed files for security hotspot patterns (curl|bash, hardcoded credentials, dynamic code execution, insecure downloads) — none found. The new script uses proper error handling (set -euo pipefail) and safe jq filters. |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
Problem
CI Failure Analyst never runs for failed GitHub Actions jobs: the check_run trigger only fires for third-party checks
From the issue: CI Failure Analyst is documented as diagnosing failed CI, Lint and Test runs. In practice it never runs for a failed GitHub Actions job, in any repo, on any release. It only runs when a third-party app's check fails.
Risk
Medium — changes GitHub Actions workflow behavior, which is exercised only post-merge; verify via the affected workflow runs.
Test plan
Tests added/updated:
tests/aw/ci-failure-analyst/scenarios.md,tests/ci_failure_analyst_resolve.bats. Verification:bash scripts/dev-lead-lint.sh(shellcheck --severity=warning) ran pre-commit; the bats suite runs in CI.Rollback
Revert this PR. No non-revertible side effects (no tags, migrations, or external state).
Monitoring
Watch the affected workflow run(s) in the Actions tab and this PR's Lint check for regressions.
Refs #2235. This PR delivers criteria 1 and 3 of that issue: the reusable accepts a
check_runor aworkflow_runpayload. The stub trigger (criterion 2) and the live proof (criterion 4) follow in a second PR onceci-failure-analyst/v1-stablecarries this reusable, so the issue stays open after this merges. The change is inert until a channel is cut.