ci: automate PR feedback when CI lint/build/test checks fail#1664
ci: automate PR feedback when CI lint/build/test checks fail#1664hk2166 wants to merge 1 commit into
Conversation
Re-implements the PR-helper CI feedback flow as a scheduled cron driver instead of a workflow_run listener. The previous workflow_run-based approach (PR hiero-ledger#1637) was reverted in PR hiero-ledger#1638 because workflow_run was flagged as an unsafe trigger pattern. The new on-ci-result workflow runs every 10 minutes, lists open PRs, and for each one looks up the latest "PR Checks" run by head SHA. It updates the dashboard CI section, force-swaps the status label to "needs revision" on failure, and posts a one-time @mention notification on clean->failure transitions. Runs entirely in base-repo context, so fork PRs are handled without exposing any fork-controlled input plane. Signed-off-by: hk2166 <9610hemant@gmail.com>
📝 WalkthroughWalkthroughThis PR implements an automated CI result monitoring bot that runs every 10 minutes, monitors PR CI runs, classifies failures by type, updates the dashboard comment with CI status, and notifies authors when CI fails. The bot only transitions to failure notification if the prior dashboard did not already show a CI failure. ChangesAutomated CI Result Bot
Sequence DiagramsequenceDiagram
participant Cron as Cron Trigger
participant Workflow as on-ci-result Workflow
participant Driver as bot-on-ci-result.js
participant GitHub as GitHub API
participant Dashboard as PR Dashboard Comment
Cron->>Workflow: every 10 minutes
Workflow->>Driver: execute with github context
Driver->>GitHub: listAllOpenPRs (paginated)
loop For Each Open PR
Driver->>GitHub: getLatestPRChecksRun by head_sha
Driver->>GitHub: classifyWorkflowFailure from run jobs/steps
Driver->>GitHub: getBotComment (prior state)
Driver->>GitHub: runAllChecksAndComment with ci result
Driver->>Dashboard: render CI section with status
alt CI Failed
Driver->>GitHub: detect prior failure via hadPriorCIFailure
alt Transition into Failure
Driver->>GitHub: post `@mention` notification comment
end
Driver->>GitHub: force label to needs-revision
end
end
Driver->>Workflow: return {processed, success, failure, ...}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hey @hk2166 👋 thanks for the PR! This comment updates automatically as you push changes -- think of it as your PR's live scoreboard! PR Checks✅ DCO Sign-off -- All commits have valid sign-offs. Nice work! ✅ GPG Signature -- All commits have verified GPG signatures. Locked and loaded! ✅ Merge Conflicts -- No merge conflicts detected. Smooth sailing! ✅ Issue Link -- Linked to #1631 (assigned to you). 🎉 All checks passed! Your PR is ready for review. Great job! |
github-actions
Bot
added
the
status: needs review
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/on-ci-result.yaml (1)
43-50: ⚡ Quick winAdd a job timeout to prevent cron backlog stalls
This job is serialized (
concurrencywithcancel-in-progress: false). Without a timeout, one stuck run can block reconciliation for hours. Addtimeout-minutesat job level.Suggested patch
reconcile: name: Reconcile CI Results runs-on: hiero-client-sdk-linux-large + timeout-minutes: 15🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/on-ci-result.yaml around lines 43 - 50, The reconcile job is serialized and lacks a job-level timeout which can block subsequent runs; add a timeout-minutes entry under the reconcile job (next to runs-on/concurrency) such as timeout-minutes: 30 (or an org-appropriate value) so a stuck run will be automatically cancelled and not stall the concurrency group; update the job named "reconcile" to include this timeout.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/scripts/helpers/ci.js:
- Around line 168-170: The current ciFailurePattern (/ :x:.*?\*\*CI
Checks\*\*|\*\*CI Checks\*\*.*?:x:/s ) can match a :x: and a **CI Checks**
heading that occur on different lines/sections, causing false positives; update
the pattern used in ciFailurePattern to require the :x: and **CI Checks** to
appear on the same line (allowing only whitespace between them) and use
multiline mode so we only detect line-level matches against priorCommentBody —
replace the existing ciFailurePattern with a regex that anchors to a single line
(e.g. matching ":x:\s*\*\*CI Checks\*\*" or "\*\*CI Checks\*\*\s*:x:" in
multiline) so priorCommentBody is tested only for CI-line failures.
- Around line 77-83: The code reads workflowRun.html_url into runUrl before
validating workflowRun; move the access so workflowRun is validated first (use
requireObject(workflowRun, 'workflowRun') before referencing html_url) or
compute runUrl defensively (e.g., use optional chaining/fallback) so
dereferencing workflowRun.html_url cannot throw; update the symbol references:
workflowRun, runUrl, requireObject, and html_url to ensure a safe, validated
access and preserve the existing fallback return behavior.
In @.github/workflows/on-ci-result.yaml:
- Around line 57-60: The checkout step using
actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd currently persists
credentials into the repository; update the Checkout Repository step (the
actions/checkout usage) to include persist-credentials: false so the default
token is not written into .git/config, leaving the workflow able to run npm ci
and the bot without persisted auth.
---
Nitpick comments:
In @.github/workflows/on-ci-result.yaml:
- Around line 43-50: The reconcile job is serialized and lacks a job-level
timeout which can block subsequent runs; add a timeout-minutes entry under the
reconcile job (next to runs-on/concurrency) such as timeout-minutes: 30 (or an
org-appropriate value) so a stuck run will be automatically cancelled and not
stall the concurrency group; update the job named "reconcile" to include this
timeout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 781363ff-bcdf-41ea-88bf-26f1bb5f1886
📒 Files selected for processing (6)
.github/scripts/bot-on-ci-result.js.github/scripts/helpers/api.js.github/scripts/helpers/ci.js.github/scripts/helpers/comments.js.github/scripts/helpers/index.js.github/workflows/on-ci-result.yaml
| const runUrl = workflowRun.html_url; | ||
|
|
||
| try { | ||
| requireObject(workflowRun, 'workflowRun'); | ||
| requireNonEmptyString(owner, 'owner'); | ||
| requireNonEmptyString(repo, 'repo'); | ||
|
|
There was a problem hiding this comment.
Guard workflowRun before reading html_url.
Line 77 dereferences workflowRun.html_url before validation and before the try block. If workflowRun is null/undefined, this throws and skips your graceful fallback return.
Suggested fix
-async function classifyWorkflowFailure(github, workflowRun, owner, repo) {
- const runUrl = workflowRun.html_url;
-
- try {
+async function classifyWorkflowFailure(github, workflowRun, owner, repo) {
+ let runUrl = '';
+ try {
requireObject(workflowRun, 'workflowRun');
+ runUrl = workflowRun.html_url || '';
requireNonEmptyString(owner, 'owner');
requireNonEmptyString(repo, 'repo');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const runUrl = workflowRun.html_url; | |
| try { | |
| requireObject(workflowRun, 'workflowRun'); | |
| requireNonEmptyString(owner, 'owner'); | |
| requireNonEmptyString(repo, 'repo'); | |
| let runUrl = ''; | |
| try { | |
| requireObject(workflowRun, 'workflowRun'); | |
| runUrl = workflowRun.html_url || ''; | |
| requireNonEmptyString(owner, 'owner'); | |
| requireNonEmptyString(repo, 'repo'); | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/scripts/helpers/ci.js around lines 77 - 83, The code reads
workflowRun.html_url into runUrl before validating workflowRun; move the access
so workflowRun is validated first (use requireObject(workflowRun, 'workflowRun')
before referencing html_url) or compute runUrl defensively (e.g., use optional
chaining/fallback) so dereferencing workflowRun.html_url cannot throw; update
the symbol references: workflowRun, runUrl, requireObject, and html_url to
ensure a safe, validated access and preserve the existing fallback return
behavior.
| // Match either ":x: **CI Checks**" or "**CI Checks** ... :x:" | ||
| const ciFailurePattern = /:x:.*?\*\*CI Checks\*\*|\*\*CI Checks\*\*.*?:x:/s; | ||
| return ciFailurePattern.test(priorCommentBody); |
There was a problem hiding this comment.
Tighten CI failure detection regex to the CI line.
Line 169 can match a non-CI :x: earlier in the dashboard and a later **CI Checks** heading, causing false “prior CI failure” detection and suppressing first failure notifications.
Suggested fix
- const ciFailurePattern = /:x:.*?\*\*CI Checks\*\*|\*\*CI Checks\*\*.*?:x:/s;
+ const ciFailurePattern = /^:x:\s+\*\*CI Checks\*\*/m;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/scripts/helpers/ci.js around lines 168 - 170, The current
ciFailurePattern (/ :x:.*?\*\*CI Checks\*\*|\*\*CI Checks\*\*.*?:x:/s ) can
match a :x: and a **CI Checks** heading that occur on different lines/sections,
causing false positives; update the pattern used in ciFailurePattern to require
the :x: and **CI Checks** to appear on the same line (allowing only whitespace
between them) and use multiline mode so we only detect line-level matches
against priorCommentBody — replace the existing ciFailurePattern with a regex
that anchors to a single line (e.g. matching ":x:\s*\*\*CI Checks\*\*" or
"\*\*CI Checks\*\*\s*:x:" in multiline) so priorCommentBody is tested only for
CI-line failures.
| - name: Checkout Repository | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| with: | ||
| ref: ${{ github.event.repository.default_branch }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and display the referenced workflow file around the specified lines
FILE=".github/workflows/on-ci-result.yaml"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE" >&2
exit 1
fi
echo "== File: $FILE =="
nl -ba "$FILE" | sed -n '1,120p'Repository: hiero-ledger/hiero-sdk-cpp
Length of output: 180
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/on-ci-result.yaml"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE" >&2
exit 1
fi
echo "== File: $FILE =="
# Print enough context around the referenced line range
cat -n "$FILE" | sed -n '1,120p'Repository: hiero-ledger/hiero-sdk-cpp
Length of output: 3325
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/on-ci-result.yaml"
cat -n "$FILE" | sed -n '120,260p'Repository: hiero-ledger/hiero-sdk-cpp
Length of output: 52
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== actions/checkout occurrences =="
rg -n "uses:\s*actions/checkout@" -S .github/workflows
echo
echo "== persist-credentials occurrences =="
rg -n "persist-credentials" -S .github/workflows
echo
echo "== git usage in bot scripts =="
rg -n "\bgit\b" -S .github/scripts
echo
echo "== checkout ref usage in this workflow snippet (for context) =="
cat -n .github/workflows/on-ci-result.yaml | sed -n '50,110p'Repository: hiero-ledger/hiero-sdk-cpp
Length of output: 1997
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Files in .github/scripts =="
fd -t f -a .github/scripts | head -n 200
echo
echo "== Inspect bot-on-ci-result.js =="
if [ -f .github/scripts/bot-on-ci-result.js ]; then
sed -n '1,200p' .github/scripts/bot-on-ci-result.js
else
echo "Missing .github/scripts/bot-on-ci-result.js" >&2
exit 1
fi
echo
echo "== Any git commands in scripts (broader search) =="
rg -n "\bgit\b" -S .github/scripts || trueRepository: hiero-ledger/hiero-sdk-cpp
Length of output: 485
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== List .github/scripts files =="
fd -t f -a . .github/scripts | head -n 200
echo
echo "== Locate bot-on-ci-result.js =="
fd -t f -a bot-on-ci-result.js .github/scripts || true
echo
echo "== Inspect bot-on-ci-result.js (if present) =="
if [ -f .github/scripts/bot-on-ci-result.js ]; then
sed -n '1,220p' .github/scripts/bot-on-ci-result.js
else
echo "Missing .github/scripts/bot-on-ci-result.js" >&2
fi
echo
echo "== Any git commands in scripts (broader search) =="
rg -n "\bgit\b" -S .github/scripts || trueRepository: hiero-ledger/hiero-sdk-cpp
Length of output: 9024
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for child_process usage + git commands =="
rg -n "child_process|exec\(|spawn\(" .github/scripts
echo
echo "== Search for actual git command invocations =="
rg -n "(\bgit\b\s+(status|log|fetch|clone|pull|push|commit|add|checkout|rebase|merge)\b|['\"]git['\"])"
-S .github/scripts || true
echo
echo "== Search for GITHUB_TOKEN usage that might imply authenticated git operations =="
rg -n "GITHUB_TOKEN|token" .github/scripts .github/workflows/on-ci-result.yaml || trueRepository: hiero-ledger/hiero-sdk-cpp
Length of output: 3515
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/on-ci-result.yaml"
echo "== Full file (line-numbered) =="
cat -n "$FILE"
echo
echo "== Any 'git ' usage in workflow =="
rg -n "\bgit\s" "$FILE" || trueRepository: hiero-ledger/hiero-sdk-cpp
Length of output: 3345
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for subprocess invocation patterns in .github/scripts =="
rg -n "child_process|execSync|spawnSync|exec\(|spawn\(|execa|simple-git|isomorphic-git" .github/scripts || true
echo
echo "== Search for actual git CLI invocations inside scripts (not docs) =="
rg -n "\bgit\s+(status|log|fetch|clone|pull|push|commit|add|checkout|rebase|merge|remote|config)\b" .github/scripts || trueRepository: hiero-ledger/hiero-sdk-cpp
Length of output: 777
🌐 Web query:
actions/checkout persist-credentials default token stored in git config
💡 Result:
By default, actions/checkout persists the authentication token (for HTTPS) into the checked-out repository’s local git configuration (i.e., in.git/config) so that later steps can run authenticated git commands; the token is then removed during post-job cleanup. [1][2] What this means for your question - The input persist-credentials defaults to true (the README documents “persist-credentials” as “Default: true”). [2] - When persist-credentials is enabled, the auth token is configured in the local git config; when the job finishes, the post-job cleanup removes it. [1][2] - You can opt out by setting persist-credentials: false on the checkout step. [1][2] Example - uses: actions/checkout@v4 with: persist-credentials: false [1][2]
Citations:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== .github/scripts/package.json =="
cat .github/scripts/package.json
echo
echo "== package-lock.json git/github dependency patterns =="
rg -n "git\+https|git\+ssh|github\.com[:/]|source\.git|ssh://|https://github\.com" .github/scripts/package-lock.json .github/scripts/package.json || true
echo
echo "== package.json scripts (for git usage) =="
rg -n "\"scripts\"|npm|git" .github/scripts/package.json || trueRepository: hiero-ledger/hiero-sdk-cpp
Length of output: 1656
Disable persisted git credentials in checkout
actions/checkout persists the default auth token into the checked-out repo’s local .git/config (since persist-credentials isn’t set). This workflow only needs the repo contents for npm ci + running the bot, so avoid persisting credentials by adding persist-credentials: false.
Suggested patch
- name: Checkout Repository
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
ref: ${{ github.event.repository.default_branch }}
+ persist-credentials: false🧰 Tools
🪛 zizmor (1.25.2)
[warning] 57-60: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/on-ci-result.yaml around lines 57 - 60, The checkout step
using actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd currently
persists credentials into the repository; update the Checkout Repository step
(the actions/checkout usage) to include persist-credentials: false so the
default token is not written into .git/config, leaving the workflow able to run
npm ci and the bot without persisted auth.
|
@rwalworth |
1 participant
Re-implements the PR-helper CI feedback flow as a scheduled cron driver
instead of a workflow_run listener. The previous workflow_run-based approach
(PR #1637) was reverted in PR #1638 because workflow_run was flagged as an
unsafe trigger pattern.
The new on-ci-result workflow runs every 10 minutes, lists open PRs, and for
each one looks up the latest "PR Checks" run by head SHA. It updates the
dashboard CI section, force-swaps the status label to "needs revision" on
failure, and posts a one-time @mention notification on clean->failure
transitions. Runs entirely in base-repo context, so fork PRs are handled
without exposing any fork-controlled input plane.
Description:
Automate PR feedback when CI lint/build/test checks fail, using a scheduled cron driver that reconciles the dashboard, status label, and
contributor ping in base-repo context.
helpers/ci.jswithgetLatestPRChecksRun,classifyWorkflowFailure(lint / build / tests via job + step inspection), andhadPriorCIFailure(transition detection)bot-on-ci-result.jscron driver that iterates open PRs, skips drafts and bot-authored PRs, and per PR: re-renders the dashboard CI section,force-swaps the status label to
status: needs revisionon failure, and posts a one-time @mention notification only on clean->failure transitions.github/workflows/on-ci-result.yamlwithschedule: cron '*/10 * * * *'plusworkflow_dispatch, hardened runner, default-branchcheckout, and a global concurrency group so two ticks cannot double-post
helpers/comments.jswithbuildCISection,buildCIFailureNotificationComment, and a greeting that explains thestatus: needs reviewvs
status: needs revisionsemanticshelpers/index.jsrunAllChecksAndCommentinhelpers/api.jsto accept and propagate the precomputedciresult through tobuildBotCommentRelated issue(s):
Fixes #1631
Summary by CodeRabbit