-
Notifications
You must be signed in to change notification settings - Fork 97
ci: automate PR feedback when CI lint/build/test checks fail #1664
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
hk2166
wants to merge
1
commit into
hiero-ledger:main
Choose a base branch
from
hk2166:ci/automate-pr-feedback-cron
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,178 @@ | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
| // | ||
| // bot-on-ci-result.js | ||
| // | ||
| // Runs on a cron schedule. For each open PR: | ||
| // 1. Look up the latest "PR Checks" workflow run by head SHA. | ||
| // 2. If completed, classify the result (lint / build / tests / pass). | ||
| // 3. Re-render the dashboard comment with the CI section. | ||
| // 4. On failure, force-swap the status label to needs-revision and post a | ||
| // one-time @mention ping if the prior dashboard didn't already show a | ||
| // failure (transition into failure). | ||
| // | ||
| // This driver runs entirely in the base-repo context, so fork PRs are handled | ||
| // without any input from the fork — replacing the previous workflow_run-based | ||
| // driver that GitHub flagged as an unsafe trigger pattern. | ||
|
|
||
| const { | ||
| createLogger, | ||
| swapStatusLabel, | ||
| runAllChecksAndComment, | ||
| getBotComment, | ||
| postComment, | ||
| buildCIFailureNotificationComment, | ||
| getLatestPRChecksRun, | ||
| classifyWorkflowFailure, | ||
| hadPriorCIFailure, | ||
| MARKER, | ||
| } = require('./helpers'); | ||
|
|
||
| const logger = createLogger('on-ci-result'); | ||
|
|
||
| /** | ||
| * Lists every open PR in the repository, paginating through results. | ||
| */ | ||
| async function listAllOpenPRs(github, owner, repo) { | ||
| const prs = []; | ||
| let page = 1; | ||
| const perPage = 100; | ||
|
|
||
| while (true) { | ||
| const { data } = await github.rest.pulls.list({ | ||
| owner, | ||
| repo, | ||
| state: 'open', | ||
| per_page: perPage, | ||
| page, | ||
| }); | ||
|
|
||
| prs.push(...data); | ||
|
|
||
| if (data.length < perPage) break; | ||
| page++; | ||
| } | ||
|
|
||
| return prs; | ||
| } | ||
|
|
||
| /** | ||
| * Reconciles a single PR's CI dashboard / label / ping state. | ||
| * Returns one of: 'no-run', 'no-change', 'success', 'failure'. | ||
| */ | ||
| async function reconcilePR({ github, owner, repo, pr }) { | ||
| const prNumber = pr.number; | ||
| const headSha = pr.head?.sha; | ||
|
|
||
| if (!headSha) { | ||
| logger.log(`PR #${prNumber}: missing head SHA, skipping`); | ||
| return 'no-run'; | ||
| } | ||
|
|
||
| const workflowRun = await getLatestPRChecksRun(github, owner, repo, headSha); | ||
| if (!workflowRun) { | ||
| logger.log(`PR #${prNumber}: no completed PR Checks run for ${headSha}, skipping`); | ||
| return 'no-run'; | ||
| } | ||
|
|
||
| const ciResult = await classifyWorkflowFailure(github, workflowRun, owner, repo); | ||
| logger.log(`PR #${prNumber}: failed=${ciResult.failed}, check=${ciResult.check}`); | ||
|
|
||
| const botContext = { | ||
| github, | ||
| owner, | ||
| repo, | ||
| number: prNumber, | ||
| pr, | ||
| eventType: 'schedule', | ||
| }; | ||
|
|
||
| // Read prior dashboard before re-rendering, so we can detect a clean→failure | ||
| // transition (only point where we ping the author). | ||
| const priorComment = await getBotComment(botContext, MARKER); | ||
| const hadPriorFailure = hadPriorCIFailure(priorComment?.body); | ||
|
|
||
| const ci = ciResult.failed | ||
| ? { passed: false, check: ciResult.check, runUrl: ciResult.runUrl } | ||
| : { passed: true }; | ||
|
|
||
| await runAllChecksAndComment(botContext, { ci }); | ||
|
|
||
| if (!ciResult.failed) { | ||
| // On CI success the success-path label management is owned by | ||
| // bot-on-pr-update; we leave the label untouched here. | ||
| return 'success'; | ||
| } | ||
|
|
||
| // Force-swap to needs-revision on any CI failure, even if no prior label | ||
| // was present. | ||
| const swapResult = await swapStatusLabel(botContext, false, { force: true }); | ||
| if (!swapResult.success) { | ||
| logger.error(`PR #${prNumber}: failed to swap status label: ${swapResult.errorDetails}`); | ||
| } | ||
|
|
||
| // Only ping on the transition into failure — repeat failures stay quiet. | ||
| if (!hadPriorFailure) { | ||
| const prAuthor = pr.user?.login; | ||
| if (prAuthor) { | ||
| const body = buildCIFailureNotificationComment(prAuthor, ciResult.runUrl); | ||
| const postResult = await postComment(botContext, body); | ||
| if (!postResult.success) { | ||
| logger.error(`PR #${prNumber}: failed to post notification: ${postResult.error}`); | ||
| } else { | ||
| logger.log(`PR #${prNumber}: posted CI failure notification`); | ||
| } | ||
| } | ||
| } else { | ||
| logger.log(`PR #${prNumber}: prior CI failure detected, skipping notification`); | ||
| } | ||
|
|
||
| return 'failure'; | ||
| } | ||
|
|
||
| module.exports = async ({ github, context }) => { | ||
| try { | ||
| const owner = context.repo.owner; | ||
| const repo = context.repo.repo; | ||
|
|
||
| const allOpenPRs = await listAllOpenPRs(github, owner, repo); | ||
| logger.log(`Found ${allOpenPRs.length} open PR(s)`); | ||
|
|
||
| const counts = { processed: 0, noRun: 0, success: 0, failure: 0, skipped: 0, errors: 0 }; | ||
|
|
||
| for (const pr of allOpenPRs) { | ||
| // Skip drafts — they aren't ready for CI feedback yet. | ||
| if (pr.draft) { | ||
| counts.skipped++; | ||
| continue; | ||
| } | ||
| // Skip bot-authored PRs (matches the on-pr-update bot behaviour). | ||
| if (pr.user?.type === 'Bot') { | ||
| counts.skipped++; | ||
| continue; | ||
| } | ||
|
|
||
| try { | ||
| const outcome = await reconcilePR({ github, owner, repo, pr }); | ||
| counts.processed++; | ||
| if (outcome === 'no-run') counts.noRun++; | ||
| else if (outcome === 'success') counts.success++; | ||
| else if (outcome === 'failure') counts.failure++; | ||
| } catch (err) { | ||
| counts.errors++; | ||
| logger.error(`PR #${pr.number}: reconcile failed: ${err.message}`); | ||
| // Continue with the next PR; one bad PR shouldn't kill the tick. | ||
| } | ||
| } | ||
|
|
||
| logger.log( | ||
| `On-CI-result tick complete: processed=${counts.processed}, ` + | ||
| `noRun=${counts.noRun}, success=${counts.success}, failure=${counts.failure}, ` + | ||
| `skipped=${counts.skipped}, errors=${counts.errors}` | ||
| ); | ||
|
|
||
| return counts; | ||
| } catch (error) { | ||
| logger.error('Error:', { message: error.message }); | ||
| throw error; | ||
| } | ||
| }; |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,178 @@ | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
| // | ||
| // helpers/ci.js | ||
| // | ||
| // CI-related helpers for the cron-driven on-ci-result bot. The bot iterates | ||
| // open PRs and reconciles each one's dashboard against the latest "PR Checks" | ||
| // run for that PR's head SHA. These helpers do the lookup and classification. | ||
|
|
||
| const { getLogger } = require('./logger'); | ||
| const { requireObject, requireNonEmptyString } = require('./validation'); | ||
|
|
||
| const PR_CHECKS_WORKFLOW_NAME = 'PR Checks'; | ||
|
|
||
| /** | ||
| * Looks up the latest "PR Checks" workflow run for a given head SHA. | ||
| * Returns null if no run has been recorded yet, or if the latest run is still | ||
| * in progress / queued (the caller should skip those PRs until the run | ||
| * finishes — we don't want to flap the dashboard mid-run). | ||
| * | ||
| * Runs in the base-repository context, so this works for fork PRs without | ||
| * needing any input from the fork. | ||
| * | ||
| * @param {object} github - Octokit GitHub API client. | ||
| * @param {string} owner - Base-repo owner. | ||
| * @param {string} repo - Base-repo name. | ||
| * @param {string} headSha - PR head commit SHA. | ||
| * @returns {Promise<object|null>} - Latest completed workflow run, or null. | ||
| */ | ||
| async function getLatestPRChecksRun(github, owner, repo, headSha) { | ||
| try { | ||
| requireNonEmptyString(owner, 'owner'); | ||
| requireNonEmptyString(repo, 'repo'); | ||
| requireNonEmptyString(headSha, 'headSha'); | ||
|
|
||
| const { data } = await github.rest.actions.listWorkflowRunsForRepo({ | ||
| owner, | ||
| repo, | ||
| head_sha: headSha, | ||
| per_page: 100, | ||
| }); | ||
|
|
||
| const runs = (data.workflow_runs || []).filter( | ||
| run => run.name === PR_CHECKS_WORKFLOW_NAME | ||
| ); | ||
|
|
||
| if (runs.length === 0) { | ||
| return null; | ||
| } | ||
|
|
||
| // listWorkflowRunsForRepo returns newest-first by default, but be explicit. | ||
| runs.sort((a, b) => new Date(b.run_started_at || b.created_at) - new Date(a.run_started_at || a.created_at)); | ||
| const latest = runs[0]; | ||
|
|
||
| if (latest.status !== 'completed') { | ||
| getLogger().log(`Latest PR Checks run for ${headSha} is ${latest.status}, skipping`); | ||
| return null; | ||
| } | ||
|
|
||
| return latest; | ||
| } catch (error) { | ||
| getLogger().error(`Failed to look up PR Checks run for ${headSha}: ${error.message}`); | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Classifies a workflow run's failure by inspecting job and step results. | ||
| * Returns the type of failure (lint, build, or tests) and the run URL. | ||
| * | ||
| * @param {object} github - Octokit GitHub API client. | ||
| * @param {object} workflowRun - Workflow run object (from listWorkflowRunsForRepo). | ||
| * @param {string} owner - Repository owner. | ||
| * @param {string} repo - Repository name. | ||
| * @returns {Promise<{ failed: boolean, check: string|null, runUrl: string }>} | ||
| */ | ||
| async function classifyWorkflowFailure(github, workflowRun, owner, repo) { | ||
| const runUrl = workflowRun.html_url; | ||
|
|
||
| try { | ||
| requireObject(workflowRun, 'workflowRun'); | ||
| requireNonEmptyString(owner, 'owner'); | ||
| requireNonEmptyString(repo, 'repo'); | ||
|
|
||
| if (workflowRun.conclusion === 'success') { | ||
| return { failed: false, check: null, runUrl }; | ||
| } | ||
|
|
||
| if (workflowRun.conclusion === 'cancelled' || workflowRun.conclusion === 'skipped') { | ||
| return { failed: false, check: null, runUrl }; | ||
| } | ||
|
|
||
| const { data: jobsData } = await github.rest.actions.listJobsForWorkflowRun({ | ||
| owner, | ||
| repo, | ||
| run_id: workflowRun.id, | ||
| per_page: 100, | ||
| }); | ||
|
|
||
| const jobs = jobsData.jobs || []; | ||
| getLogger().log(`Found ${jobs.length} jobs for workflow run ${workflowRun.id}`); | ||
|
|
||
| // Lint job is its own job, named "Lint" in zxc-build-library.yaml. | ||
| const lintJob = jobs.find(job => job.name && job.name.toLowerCase().includes('lint')); | ||
| if (lintJob && lintJob.conclusion === 'failure') { | ||
| getLogger().log('Classified failure as: lint'); | ||
| return { failed: true, check: 'lint', runUrl }; | ||
| } | ||
|
|
||
| // Build job runs CMake then CTest. Distinguish build vs tests by which | ||
| // step failed. | ||
| const buildJob = jobs.find(job => | ||
| job.name && ( | ||
| job.name.toLowerCase().includes('build') || | ||
| job.name.toLowerCase().includes('code') | ||
| ) | ||
| ); | ||
|
|
||
| if (buildJob && buildJob.conclusion === 'failure') { | ||
| const buildSteps = buildJob.steps || []; | ||
|
|
||
| const buildStepFailed = buildSteps.some(step => | ||
| step.name && | ||
| (step.name.toLowerCase().includes('cmake build') || | ||
| step.name.toLowerCase().includes('build project')) && | ||
| step.conclusion === 'failure' | ||
| ); | ||
|
|
||
| if (buildStepFailed) { | ||
| getLogger().log('Classified failure as: build'); | ||
| return { failed: true, check: 'build', runUrl }; | ||
| } | ||
|
|
||
| const testStepFailed = buildSteps.some(step => | ||
| step.name && | ||
| (step.name.toLowerCase().includes('ctest') || | ||
| step.name.toLowerCase().includes('test')) && | ||
| step.conclusion === 'failure' | ||
| ); | ||
|
|
||
| if (testStepFailed) { | ||
| getLogger().log('Classified failure as: tests'); | ||
| return { failed: true, check: 'tests', runUrl }; | ||
| } | ||
|
|
||
| getLogger().log('Build job failed but step unclear, defaulting to: build'); | ||
| return { failed: true, check: 'build', runUrl }; | ||
| } | ||
|
|
||
| getLogger().log('Could not classify failure type, returning generic failure'); | ||
| return { failed: true, check: null, runUrl }; | ||
| } catch (error) { | ||
| getLogger().error(`Failed to classify workflow failure: ${error.message}`); | ||
| return { failed: true, check: null, runUrl }; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Checks if the prior dashboard comment already shows a CI failure. Used to | ||
| * detect state transitions (no failure → failure) so we only post the | ||
| * @mention notification once per failure cycle. | ||
| * | ||
| * @param {string|null} priorCommentBody - Body of the existing dashboard comment, or null. | ||
| * @returns {boolean} | ||
| */ | ||
| function hadPriorCIFailure(priorCommentBody) { | ||
| if (!priorCommentBody) return false; | ||
|
|
||
| // Match either ":x: **CI Checks**" or "**CI Checks** ... :x:" | ||
| const ciFailurePattern = /:x:.*?\*\*CI Checks\*\*|\*\*CI Checks\*\*.*?:x:/s; | ||
| return ciFailurePattern.test(priorCommentBody); | ||
|
Comment on lines
+168
to
+170
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tighten CI failure detection regex to the CI line. Line 169 can match a non-CI Suggested fix- const ciFailurePattern = /:x:.*?\*\*CI Checks\*\*|\*\*CI Checks\*\*.*?:x:/s;
+ const ciFailurePattern = /^:x:\s+\*\*CI Checks\*\*/m;🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| module.exports = { | ||
| PR_CHECKS_WORKFLOW_NAME, | ||
| getLatestPRChecksRun, | ||
| classifyWorkflowFailure, | ||
| hadPriorCIFailure, | ||
| }; | ||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard
workflowRunbefore readinghtml_url.Line 77 dereferences
workflowRun.html_urlbefore validation and before thetryblock. IfworkflowRunis null/undefined, this throws and skips your graceful fallback return.Suggested fix
📝 Committable suggestion
🤖 Prompt for AI Agents