hiero-ledger / hiero-ledger/hiero-sdk-cpp

[Intermediate]: Automate PR feedback when CI lint/build/test checks fail

Open
#1,631 16 comments 0 reactions 0 assignees View on GitHub
priority: high scope: ci skill: intermediate status: ready for dev
Dominant language
C++
Stars
42
Forks
108
Avg merge
11h 45m
Merged PRs (30d)
2

Description

### 🧩 Intermediate Friendly

This issue is a good fit for contributors who are already familiar with the Hiero C++ SDK and feel comfortable navigating the codebase.

Intermediate Issues often involve:
- Exploring existing implementations
- Understanding how different components work together
- Making thoughtful changes that follow established patterns

The goal is to support deeper problem-solving while keeping the task clear, focused, and enjoyable to work on.

> [!IMPORTANT]
> ### 🧭 About Intermediate Issues
>
> Intermediate Issues are a great next step for contributors who enjoy digging into the codebase and reasoning about how things work.
>
> These issues often:
> - Involve multiple related files or components
> - Encourage investigation and understanding of existing behavior
> - Leave room for thoughtful implementation choices
> - Stay focused on a clearly defined goal
>
> Other kinds of contributions — from beginner-friendly tasks to large system-level changes — are just as valuable and use different labels.

### 👾 Description of the Task

When a maintainer runs CI on a PR (the `PR Checks` workflow defined in `.github/workflows/flow-pull-request-checks.yaml`), one of the lint, build, or test jobs may fail. Today, surfacing those failures to the contributor — flipping the status label and explaining what failed — is a manual maintainer chore. We want this automated, consistent with the rest of our PR Helper Bot.

The desired contributor experience is:

1. Contributor pushes commits. The existing `bot-on-pr-update` flow runs the dashboard checks (DCO, GPG, merge conflicts, issue link).
2. If those pass, the PR gets `status: needs review`. A maintainer reviews the code for security and runs CI.
3. When CI completes:
- **On success** — the dashboard's new CI section shows ✅ and the label is left as `status: needs review`.
- **On failure** — the dashboard's CI section is updated with the failure details and a link to the failing run, the label is force-swapped to `status: needs revision`, and a small transient comment is posted that `@mentions` the contributor (so they actually get a notification, since GitHub doesn't notify on edits).

The race between `bot-on-pr-update` (which may set `status: needs review` based on its 4 checks) and this new flow (which may flip it back on CI failure) is intentional: the temporary `needs review` confirms the PR-helper checks pass and signals to maintainers that CI can be run.

Relevant files:

```
.github/workflows/flow-pull-request-checks.yaml # The "PR Checks" workflow whose completion we listen for
.github/workflows/zxc-build-library.yaml # Reusable workflow with lint + build (and CTest) jobs
.github/scripts/bot-on-pr-update.js # Existing pattern to mirror
.github/scripts/helpers/comments.js # Dashboard comment builder (add a new CI section here)
.github/scripts/helpers/api.js # swapStatusLabel, postOrUpdateComment, etc.
```

### 💡 Proposed Approach

Add a new workflow + bot script that listens for the `PR Checks` workflow completing, and integrate the result into the existing dashboard comment.

**Trigger.** A new workflow `.github/workflows/on-ci-result.yaml` listening for:

```yaml
on:
workflow_run:
workflows: ["PR Checks"] # filter by workflow name — does NOT fire on every workflow
types: [completed]
```

This avoids a feedback loop: reusable workflows invoked via `workflow_call` don't fire `workflow_run` themselves (only their top-level caller does), and a `workflow_run`-triggered workflow can't trigger itself. The job-level guard `if: github.event.workflow_run.conclusion != 'cancelled'` keeps cancelled runs cheap.

**Bot script.** A new `.github/scripts/bot-on-ci-result.js` that:

1. Resolves the PR number. For same-repo PRs read `workflow_run.pull_requests[0]`. For fork PRs that array is empty, so fall back to `pulls.list({ head: \`${owner}:${branch}\` })` keyed off `workflow_run.head_sha`/`head_branch`.
2. Calls `actions.listJobsForWorkflowRun` and inspects job + step results to classify the failure as **Lint**, **Build**, or **Tests**. Lint is its own job; the build job in `zxc-build-library.yaml` runs CMake configure/build and then `Start CTest suite (Debug)` — distinguish "build failed" from "tests failed" by which step has `conclusion === 'failure'`.
3. Re-renders the dashboard comment via `runAllChecksAndComment` with a new `ci` precomputed result, so the dashboard always reflects the latest CI state.
4. On a state transition (the prior dashboard comment had no CI failure but the new one does), posts a separate short notification comment that `@mentions` the PR author and links to the failing run. Detect the transition by reading the existing dashboard via `getBotComment(botContext, MARKER)` *before* re-rendering and checking whether its CI section contained `:x:`. This is the same pattern as `buildMergeConflictNotificationComment` in [helpers/comments.js](.github/scripts/helpers/comments.js).
5. On any CI failure, force-swaps the status label: `swapStatusLabel(botContext, false, { force: true })`. On CI success, leave the label alone — `bot-on-pr-update` already manages success-path labelling.

**Dashboard changes.** In `.github/scripts/helpers/comments.js`:

- Add a `buildCISection(ci)` builder following the same shape as the existing four (`pass` / `fail` / `error` rendering via `buildSection`). On failure include the failing run URL and the human check name.
- Wire it into `buildChecksSection` and `allChecksPassed` so the dashboard renders 5 sections instead of 4.
- Update the greeting in `buildBotComment` to explain the labelling so contributors know to watch for `status: needs revision`. Keep it short — one short paragraph.

**Permissions.** The new workflow needs `pull-requests: write`, `issues: write` (for labels/comments), and `actions: read` (to list jobs of the triggering run). Because `workflow_run` runs in the base repo context, these tokens work for fork PRs too.

**Tone of bot output.** Match the existing dashboard voice — friendly, second-person, with a concrete next step. The transient ping example body:

> Hi @author 👋 — the **** check just failed on this PR. Take a look at the [failing run](url) and push a fix when you're ready. Feel free to ping the maintainers if you need a hand.

### 👩‍💻 Implementation Steps

- [ ] Read `bot-on-pr-update.js`, `helpers/comments.js`, and `helpers/api.js` to understand the dashboard/swap patterns. The merge-conflict notification flow is the closest analogue for the transient-ping logic.
- [ ] Add `buildCISection(ci)` in `helpers/comments.js`. Section states: `pass` (✅, "All CI checks passed."), `fail` (❌, lists the failing check name + run URL), `error` (⚠️, internal error path with maintainer @mention). Wire it into `buildChecksSection` and `allChecksPassed`.
- [ ] Update the dashboard greeting in `buildBotComment` to explain the status-label semantics — `status: needs review` means all checks pass; `status: needs revision` means something needs attention.
- [ ] Add a helper (in `helpers/api.js` or a new `helpers/ci.js`) to:
- Resolve PR number from a `workflow_run` payload (handles fork case).
- Classify a workflow run's failure into `{ failed: boolean, check: 'lint' | 'build' | 'tests' | null, runUrl: string }` by listing jobs/steps.
- [ ] Create `.github/scripts/bot-on-ci-result.js` mirroring the structure of `bot-on-pr-update.js` (build context → run logic → log + return).
- [ ] In the new bot script, detect the failure transition by reading the prior dashboard comment body and checking whether its CI section already showed a failure. If transitioning into failure, post the @mention ping comment.
- [ ] On CI failure, force-swap the status label to `status: needs revision`.
- [ ] Create `.github/workflows/on-ci-result.yaml` with:
```yaml
on:
workflow_run:
workflows: ["PR Checks"]
types: [completed]
permissions:
contents: read
pull-requests: write
issues: write
actions: read
jobs:
notify:
if: ${{ github.event.workflow_run.conclusion != 'cancelled' }}
runs-on: hiero-client-sdk-linux-large
# ...harden runner, checkout, setup-node, github-script
```
- [ ] Add unit tests in `.github/scripts/tests/` covering: the new CI section renderer, fork-PR resolution, job-failure classification (lint vs build vs tests via step inspection), and the transition-detection branch (prior dashboard with/without `:x:` in the CI section).
- [ ] Run the existing bot test suite to ensure nothing else regressed: `cd .github/scripts && npm test`.
- [ ] Manually verify on a draft PR by intentionally failing each check (push a clang-format violation, a build break, and a test break) and confirm the comment, label, and ping all behave as expected.

### ✔️ Acceptance Criteria

- [ ] The new workflow only fires on completion of `PR Checks` (verified by running another workflow and observing it does not trigger).
- [ ] On `lint` failure, the dashboard CI section reports the lint failure with a run URL, label is `status: needs revision`, and a single `@mention` ping comment is posted.
- [ ] On `build` failure, the dashboard reports a build failure (not a test failure) with a run URL, and labels/ping behave the same.
- [ ] On `tests` failure (CTest step inside the `build` job), the dashboard reports a test failure with a run URL, and labels/ping behave the same.
- [ ] On CI success after a prior failure, the dashboard CI section flips back to ✅ and no ping is posted; the label is left alone (existing `bot-on-pr-update` flow handles success-path labelling).
- [ ] The transient ping comment is posted only on a transition into failure — re-running the same failing CI does not post another ping.
- [ ] Fork PRs are handled correctly (PR number resolved via head SHA fallback).
- [ ] The dashboard greeting now explains what `status: needs review` and `status: needs revision` mean.
- [ ] All existing bot unit tests continue to pass and new tests cover the added behavior.
- [ ] No public SDK API or behavior is changed.

### 🤔 Additional Information

- `workflow_run` reference: .
- The `pull_requests` array on `workflow_run` is only populated for same-repo PRs — fork PRs require a head-SHA lookup. This is a known GitHub quirk and is the main reason this needs a deliberate resolver helper.
- Coordination with `bot-on-pr-update`: the brief window where a PR can show `status: needs review` before CI completes is intentional. It signals to maintainers that the PR-helper checks pass and CI can be run; the new flow flips the label back if CI fails. No locking or coordination logic is needed.
- A short Discord announcement to contributors will accompany the rollout, but the dashboard greeting carries the long-term explanation for new contributors who join later.

---

### 📋 Step-by-Step Contribution Guide

To help keep contributions consistent and easy to review, we recommend following these steps:

- [ ] Comment `/assign` to request the issue
- [ ] Wait for assignment
- [ ] Fork the repository and create a branch
- [ ] Set up the project using the instructions in `README.md`
- [ ] Make the requested changes
- [ ] Sign each commit using `-s -S`
- [ ] Push your branch and open a pull request

Read [Workflow Guide](https://github.com/hiero-ledger/hiero-sdk-cpp/blob/main/docs/training/workflow.md) for step-by-step workflow guidance.
Read [README.md](https://github.com/hiero-ledger/hiero-sdk-cpp/blob/main/README.md) for setup instructions.

❗ Pull requests **cannot be merged** without `S` and `s` signed commits.
See the [Signing Guide](https://github.com/hiero-ledger/hiero-sdk-cpp/blob/main/docs/training/signing.md).

### 🤔 Additional Information

- `workflow_run` reference: .
- The `pull_requests` array on `workflow_run` is only populated for same-repo PRs — fork PRs require a head-SHA lookup. This is a known GitHub quirk and is the main reason this needs a deliberate resolver helper.
- Coordination with `bot-on-pr-update`: the brief window where a PR can show `status: needs review` before CI completes is intentional. It signals to maintainers that the PR-helper checks pass and CI can be run; the new flow flips the label back if CI fails. No locking or coordination logic is needed.
- A short Discord announcement to contributors will accompany the rollout, but the dashboard greeting carries the long-term explanation for new contributors who join later.

Contributor guide

Open the contributing guide

Research direction

Read .github/scripts/bot-on-pr-update.js, helpers/comments.js, helpers/api.js, and .github/workflows/flow-pull-request-checks.yaml first, using the merge-conflict notification flow as the closest analogue. Add the CI-result workflow and bot, CI dashboard section, failure classification, label and notification behavior, plus tests in .github/scripts/tests/. Run `cd .github/scripts && npm test`; done means the listed lint, build, test, fork, transition, and success cases pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, github-actions, javascript, node.js
Domain
ci-cd, devops, tooling
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.