| 1 | --- |
| 2 | name: pr-reviews |
| 3 | description: Address pull-request comments and reviews iteratively until the PR is clean — fetch all comments with paranoid pagination, classify by author (AI bot vs human), verify each finding, address it, find similar patterns, reply per-thread, resolve threads, check CI before pushing, retrigger AI reviewers (cubic-dev-ai, copilot), and wait for new feedback. Use when the user says "address PR comments", "look at the reviews on PR N", "deal with the bot comments", "iterate on PR N until clean", or anything mentioning PR comments / reviews / cubic / copilot. |
| 4 | --- |
| 5 | |
| 6 | # PR review handler skill |
| 7 | |
| 8 | This skill iterates a PR through review/comment cycles until there is nothing |
| 9 | left to address. |
| 10 | |
| 11 | ## Your role on a PR |
| 12 | |
| 13 | When this skill is in use, the agent's job is to **bring the PR into |
| 14 | merge-ready shape**: solve the original problem the PR was opened for, |
| 15 | **and** address every legitimate finding the PR has accumulated, from |
| 16 | every source. Reviewers, linters, and CI all matter. Comments are the |
| 17 | loudest source but they are not the only source -- you must proactively |
| 18 | pull findings from every channel that reports on the PR, not wait for |
| 19 | something to surface as a chat message. |
| 20 | |
| 21 | Sources of findings, in priority order: |
| 22 | |
| 23 | 1. **Human review comments** -- maintainers / devs / community. |
| 24 | 2. **AI bot review comments** -- cubic-dev-ai, copilot, etc. |
| 25 | 3. **SonarCloud PR findings** -- new code-smell / vulnerability / |
| 26 | security-hotspot issues introduced by this PR. SonarCloud does NOT |
| 27 | post these as inline GitHub review-comments; only a QualityGate |
| 28 | summary is posted to GitHub. The actual findings live behind the |
| 29 | SonarCloud API and must be pulled explicitly. |
| 30 | 4. **CI failures relevant to this PR** -- shellcheck, codeql, build / |
| 31 | test failures caused by the PR's changes. |
| 32 | 5. **Anything else this repo configures** (Codacy, custom workflows, ...). |
| 33 | |
| 34 | A finding is "relevant to this PR" if its existence (or its line |
| 35 | location) is plausibly caused by the PR's diff. CI failures unrelated to |
| 36 | this PR (a flaky test on an unrelated module, an infra outage) are NOT |
| 37 | in scope -- note them, surface to the user at the end, do not fix them |
| 38 | here. |
| 39 | |
| 40 | The bar is the project's performance, stability, and long-term |
| 41 | maintainability. Don't dismiss findings because they look minor. |
| 42 | |
| 43 | ## MANDATORY rules |
| 44 | |
| 45 | These are non-negotiable. Skipping any of them will cost the user time. |
| 46 | |
| 47 | 1. **Pagination paranoia.** Do not stop at round numbers. If a fetch returns |
| 48 | exactly 100 / 200 / 300 items, the round count is suspicious -- GitHub |
| 49 | pagination defaults to 100, and round-multiples almost always mean there |
| 50 | is a next page that the previous client missed. Always re-probe with an |
| 51 | explicit `page=N+1` request. `fetch-all.sh` does this automatically. |
| 52 | 2. **Accept all comments and address them all.** No exceptions. No "this is |
| 53 | minor, skip it." The bar is: Netdata's performance, stability, and |
| 54 | long-term maintainability. |
| 55 | 3. **Verify every comment properly.** No shortcuts. Read the code, follow |
| 56 | the trace, confirm the claim. AI bots produce false positives -- judge |
| 57 | each one on its merits. |
| 58 | 4. **Reply per-thread, one by one.** No bulk replies. No mechanical "fixed" |
| 59 | answers. Each thread gets a substantive reply that explains what you |
| 60 | did or why the comment doesn't apply. |
| 61 | 5. **Don't dismiss comments because they look minor.** Even style nits |
| 62 | compound. The goal is for the project to thrive. |
| 63 | 6. **Help bots when they're confused.** AI reviewers sometimes flag false |
| 64 | positives because the surrounding code is ambiguous. Add a short |
| 65 | comment in the source that clarifies the intent -- it helps the next |
| 66 | reviewer (human or bot). |
| 67 | 7. **Check CI BEFORE every push, but never WAIT for CI between iterations.** |
| 68 | Waiting for CI between bot-review cycles destroys throughput -- a CI |
| 69 | run can take 30+ minutes, and during that time the AI reviewers are |
| 70 | idle. The right cadence is: |
| 71 | - Before each push: run `ci-status.sh`. If there are FAILURES, fix |
| 72 | them and bundle into the same push. If checks are still running, |
| 73 | that's fine -- ignore them and push anyway. The next push triggers |
| 74 | fresh CI on the new code, which is what we actually care about. |
| 75 | - After each push: re-trigger the bots, then `wait-for-activity.sh` |
| 76 | for new comments (NOT for CI). |
| 77 | - If `wait-for-activity.sh` times out (30 min, no new comments): |
| 78 | re-check `ci-status.sh`. If checks are still running, that's normal, |
| 79 | surface to the user. If there are failures, fix and iterate. |
| 80 | 8. **Re-trigger AI reviewers explicitly.** They do NOT react to thread |
| 81 | replies or pushed commits the way humans do. |
| 82 | - Copilot: re-add as a requested reviewer (`trigger-copilot.sh`). |
| 83 | - cubic-dev-ai: post a new top-level comment mentioning it |
| 84 | (`trigger-cubic.sh`). |
| 85 | 9. **Don't loop forever on silent bots.** Some assistants stop responding. |
| 86 | That's fine. Use `wait-for-activity.sh` with the 30-min timeout and |
| 87 | move on if nothing changes. |
| 88 | 10. **When a bot finds a legit issue, search the WHOLE PR for similar |
| 89 | issues.** This is the most expensive rule to ignore. AI reviewers |
| 90 | surface their top 3-7 findings, not the full set. If you fix only the |
| 91 | ones they pointed at, you'll spend dozens of round-trips discovering |
| 92 | the rest one at a time. Every round-trip is 30+ minutes of bot |
| 93 | review latency. **The fix for one issue means a full re-audit of the |
| 94 | PR for the same class of issue.** Do that before pushing. |
| 95 | 11. **Don't trust linters alone -- smoke-test every fix.** Static |
| 96 | analyzers (shellcheck, etc.) verify a property of the code; they |
| 97 | don't verify behavior. A "correct per the linter" fix can change |
| 98 | runtime behavior in subtle ways (e.g. a printf format-string fix |
| 99 | that stops escape-sequence interpretation, breaking colored output |
| 100 | that the linter never knew about). After every fix, run the |
| 101 | affected script (or the smallest invocation that exercises the |
| 102 | change) and verify the output looks right. "Linter green" is not |
| 103 | the same as "still works." |
| 104 | 12. **Before every push, spawn a subagent for a holistic PR review.** |
| 105 | See Step 4a in the workflow. The orchestrator's context is biased |
| 106 | toward the fixes it just made; a clean-context subagent re-reviewing |
| 107 | the WHOLE diff is what catches the issues the orchestrator and the |
| 108 | AI reviewers missed. Skipping this turns each iteration into a |
| 109 | 30-minute round-trip to discover issues that could have been found |
| 110 | in 2 minutes locally. |
| 111 | 13. **Before every push, re-fetch all finding sources one last time.** |
| 112 | See Step 4-pre. Reviewers post in parallel; if findings arrive |
| 113 | while you're addressing the current batch, they belong in THIS |
| 114 | push, not the next one. Without this sync barrier, you and the |
| 115 | reviewers stay one round out of sync forever -- the next iteration |
| 116 | is always "fixing" issues that no longer apply. |
| 117 | |
| 118 | ## Author classes -- different handling per class |
| 119 | |
| 120 | - **AI bots** (`cubic-dev-ai[bot]`, `copilot[bot]` and variants): handle |
| 121 | autonomously. Verify the finding, fix or push back with reasoning, reply |
| 122 | in-thread, resolve thread. |
| 123 | - **Informational bots** (`sonarqubecloud[bot]`, `github-actions[bot]`, |
| 124 | `netdata-bot[bot]`, `coderabbitai[bot]`): read for signal (e.g. quality |
| 125 | gate status). They don't usually require a reply. |
| 126 | - **Humans** (developers, maintainers, community): consult the user. |
| 127 | Maintainer comments matter most -- in this project, we are usually |
| 128 | contributors, they are the project owners. Do not respond on the user's |
| 129 | behalf without their direction. Surface human comments to the user with |
| 130 | a recommendation, then act per their instruction. |
| 131 | |
| 132 | ## Setup |
| 133 | |
| 134 | `gh` CLI authenticated for the repo. Nothing else. |
| 135 | |
| 136 | The skill reads `upstream` (or `origin`) from git remotes to derive the |
| 137 | repo slug. Override with `PR_REPO_SLUG=owner/repo` if working cross-repo. |
| 138 | |
| 139 | State for each PR is cached under `<repo-root>/.local/audits/pr-reviews/pr-<N>/`: |
| 140 | |
| 141 | - `pr.json` -- top-level PR metadata |
| 142 | - `issue-comments.json` -- top-level PR comments (REST) |
| 143 | - `review-comments.json` -- inline review comments (REST) |
| 144 | - `reviews.json` -- review submissions with body (REST) |
| 145 | - `review-threads.json` -- per-thread, with `isResolved` (GraphQL) |
| 146 | - `summary.txt` -- human-readable triage summary |
| 147 | |
| 148 | ## Workflow |
| 149 | |
| 150 | The order is: **gather all findings -> address them per-thread / per-finding |
| 151 | -> check CI for failures the PR caused -> push -> retrigger -> wait -> loop.** |
| 152 | |
| 153 | ### 1a. Fetch all comments (paranoid) |
| 154 | |
| 155 | ``` |
| 156 | bash .agents/skills/pr-reviews/scripts/fetch-all.sh <PR_NUMBER> |
| 157 | ``` |
| 158 | |
| 159 | Tail-prints a `summary.txt` that shows the per-author count and the list of |
| 160 | open review threads. Use this as the input to the rest of the cycle. |
| 161 | |
| 162 | ### 1b. Fetch SonarCloud PR findings |
| 163 | |
| 164 | ``` |
| 165 | bash .agents/skills/pr-reviews/scripts/fetch-sonar-findings.sh <PR_NUMBER> |
| 166 | ``` |
| 167 | |
| 168 | SonarCloud findings are NOT delivered as inline GitHub comments -- only a |
| 169 | QualityGate summary is. The actual issue list lives behind the SonarCloud |
| 170 | API. This script writes: |
| 171 | - `.local/audits/pr-reviews/pr-<N>/sonar-issues.json` |
| 172 | - `.local/audits/pr-reviews/pr-<N>/sonar-hotspots.json` |
| 173 | - a brief summary to stdout (counts by rule and severity). |
| 174 | |
| 175 | Requires the same `.env` config the `sonarqube-audit` skill uses |
| 176 | (`SONAR_TOKEN`, `SONAR_HOST_URL`, `SONAR_PROJECT`). If `.env` is missing, |
| 177 | the script prints what's needed and exits. |
| 178 | |
| 179 | ### 1c. Note the CI signal as a third source |
| 180 | |
| 181 | Run `bash .agents/skills/pr-reviews/scripts/ci-status.sh <PR>` once early to capture which checks are failing |
| 182 | **right now**. You're looking for failures caused by the current PR |
| 183 | (typo in a YAML file you added, a script that doesn't pass shellcheck, |
| 184 | a build that breaks because of the diff). DO NOT fix CI yet -- just note |
| 185 | the failures as input alongside review comments and Sonar findings. They |
| 186 | all get addressed in the same iteration so a single push covers them. |
| 187 | |
| 188 | ### 2. List open threads (and Sonar findings) |
| 189 | |
| 190 | ``` |
| 191 | bash .agents/skills/pr-reviews/scripts/list-open-threads.sh <PR_NUMBER> # full bodies |
| 192 | bash .agents/skills/pr-reviews/scripts/list-open-threads.sh <PR_NUMBER> --short # one line per thread |
| 193 | ``` |
| 194 | |
| 195 | The "short" output is a table: `thread-id | path:line | author`. The full |
| 196 | form prints every comment in each thread. |
| 197 | |
| 198 | ### 3. For each open thread, ONE AT A TIME |
| 199 | |
| 200 | **This is per-thread, not batched.** Do not prepare a list of replies and |
| 201 | fire them in a loop. Do not post all replies first and resolve all later. |
| 202 | Walk one thread at a time: |
| 203 | |
| 204 | For thread N: |
| 205 | |
| 206 | 1. **Read the comment carefully.** What is the bot/dev claiming? |
| 207 | 2. **Open the file at the line and verify.** Does the claim hold against |
| 208 | the current code? Is it valid in context? |
| 209 | 3. **Search the whole PR diff (and adjacent code) for the same class of |
| 210 | issue.** Rule #10 -- this is mandatory. (You only do this sweep once, |
| 211 | on the first thread of a class -- subsequent threads in the same |
| 212 | class share the same fix.) |
| 213 | 4. **Decide**: |
| 214 | - If valid -> fix it AND every similar instance you found. |
| 215 | - If invalid -> understand why the bot got confused. Often a small |
| 216 | source comment clarifying the intent will help the next reviewer. |
| 217 | 5. **Reply in the thread.** |
| 218 | ``` |
| 219 | bash .agents/skills/pr-reviews/scripts/reply-thread.sh <PR> <comment-id> "<reply>" |
| 220 | ``` |
| 221 | `<comment-id>` is the `databaseId` of the FIRST comment in the thread |
| 222 | (from `review-threads.json` -> `.[].comments.nodes[0].databaseId`). |
| 223 | 6. **Resolve the thread immediately after the reply succeeds.** |
| 224 | ``` |
| 225 | bash .agents/skills/pr-reviews/scripts/resolve-thread.sh <thread-id> |
| 226 | ``` |
| 227 | `<thread-id>` is the GraphQL node id (`review-threads.json` -> `.[].id`, |
| 228 | starts with `PRRT_`). Resolving immediately after replying takes the |
| 229 | thread out of the "needs attention" view; leaving threads open without |
| 230 | resolution accumulates noise. |
| 231 | |
| 232 | Then move to thread N+1. Reply-and-resolve, reply-and-resolve. Never |
| 233 | queue them up. |
| 234 | |
| 235 | The reason: the order makes intent visible to humans watching the PR -- |
| 236 | they see "agent posted reply, agent resolved" as one motion per thread, |
| 237 | not "agent dumped 14 replies, then dumped 14 resolves". Bulk operations |
| 238 | look mechanical and erode trust in the address pass. |
| 239 | |
| 240 | ### 3b. Address each Sonar finding |
| 241 | |
| 242 | For each issue in `sonar-issues.json` and each hotspot in |
| 243 | `sonar-hotspots.json`: |
| 244 | |
| 245 | 1. **Read the rule and the message.** What is Sonar claiming? |
| 246 | 2. **Open the file at the line and verify.** Does the claim hold against |
| 247 | the current code? |
| 248 | 3. **Search the whole PR diff (and adjacent code) for the same class of |
| 249 | issue.** Same rule #10 as for review comments. If Sonar flagged one |
| 250 | instance of S131 (case without default), sweep all case statements. |
| 251 | If Sonar flagged S2245 (insecure RNG), sweep all `random()` callsites. |
| 252 | 4. **Decide**: |
| 253 | - If valid -> fix it AND every similar instance you found in the |
| 254 | project where the same reasoning applies (within the diff, plus |
| 255 | nearby code in files this PR already touches). |
| 256 | - If invalid -> the corresponding `sonarqube-audit` skill provides |
| 257 | `sonar-mark.sh fp <KEY> "<reason>"` to mark it False Positive |
| 258 | directly in SonarCloud. Comments are ASCII-only (Cloudflare). |
| 259 | 5. **Repeat until `sonar-issues.json` has zero issues that we caused.** |
| 260 | |
| 261 | For Sonar there is no "thread reply" -- you address the issue with |
| 262 | either a code fix or a `sonar-mark.sh` action. There's nothing to |
| 263 | resolve in GitHub for Sonar findings. |
| 264 | |
| 265 | ### 4-pre. Before pushing -- MANDATORY final-fetch sync barrier |
| 266 | |
| 267 | Reviewers run in parallel. Multiple bots and humans can be appending |
| 268 | findings WHILE you're addressing the current batch. If you push the |
| 269 | moment your queue is empty, the findings that arrived during this |
| 270 | iteration get attributed to your fresh commit instead of the previous |
| 271 | one -- and on the next round you end up "fixing" findings that no |
| 272 | longer apply because you addressed them implicitly with the next push. |
| 273 | The result: chronic desync, where your commit and the reviewers' |
| 274 | findings are always one round apart. |
| 275 | |
| 276 | The fix: a sync barrier immediately before push. Re-fetch ALL sources |
| 277 | (comments, Sonar, CI) one more time. If ANY new finding has arrived |
| 278 | since you last looked, loop back to step 2 -- address those new |
| 279 | findings in the SAME upcoming push -- then re-fetch again. Only push |
| 280 | when a fresh fetch comes back with no new findings against the current |
| 281 | HEAD. This guarantees you and the reviewers are synchronized. |
| 282 | |
| 283 | ``` |
| 284 | bash .agents/skills/pr-reviews/scripts/fetch-all.sh <PR_NUMBER> |
| 285 | bash .agents/skills/pr-reviews/scripts/fetch-sonar-findings.sh <PR_NUMBER> |
| 286 | bash .agents/skills/pr-reviews/scripts/ci-status.sh <PR_NUMBER> |
| 287 | ``` |
| 288 | |
| 289 | The `ci-status.sh` line is the third source: a CI failure that is |
| 290 | CAUSED by this PR's changes (added a script that doesn't pass |
| 291 | shellcheck, broke a YAML parse, etc.) is in scope and must be folded |
| 292 | in. CI failures unrelated to this PR are noted, surfaced to the user |
| 293 | at the end, but not fixed here. |
| 294 | |
| 295 | If `summary.txt` shows any new open thread or `sonar-issues.json` shows |
| 296 | any new issue you haven't addressed yet, **do NOT push**. Loop back to |
| 297 | step 2 and address them first. Then re-run the fetch. Only push when |
| 298 | the fetch is clean. |
| 299 | |
| 300 | The same loop applies during the iteration: if you re-fetched while |
| 301 | addressing the previous batch and saw new findings drop in, fold them |
| 302 | into the same push rather than dispatching a half-done batch. |
| 303 | |
| 304 | ### 4a. Before pushing -- MANDATORY holistic PR review via subagent |
| 305 | |
| 306 | This is the most important pre-push step. Skipping it is what makes |
| 307 | review cycles last for hours. |
| 308 | |
| 309 | After you have made all the fixes for the current iteration's findings |
| 310 | but BEFORE running `git push`, spawn a subagent to re-review the WHOLE |
| 311 | PR diff (not the small change you just made). The orchestrator's |
| 312 | context is already loaded with the recent fixes; the subagent's clean |
| 313 | context is what gives an honest second look. |
| 314 | |
| 315 | Why this is non-negotiable: |
| 316 | |
| 317 | - AI reviewers (cubic-dev-ai, copilot, sonarqube) only surface their |
| 318 | top 3-7 findings. The full set of similar issues remains hidden. |
| 319 | - Each fix can introduce its own new problems (a printf format-string |
| 320 | fix that breaks color rendering, a portability fix that drops a |
| 321 | feature, an input-validation fix that rejects valid inputs). |
| 322 | - Without a holistic pre-push review, every iteration takes ~30 min of |
| 323 | bot review latency just to discover problems the orchestrator could |
| 324 | have spotted in 2 minutes by re-reading the diff. |
| 325 | |
| 326 | How to invoke: |
| 327 | |
| 328 | Use the orchestrator's Agent / subagent tool (whatever the harness |
| 329 | provides). Pass the subagent the PR diff (or the list of touched files) |
| 330 | and ask it to: |
| 331 | |
| 332 | - Verify each fix in this iteration solves the original finding without |
| 333 | side effects. |
| 334 | - Sweep the touched files for similar patterns the original findings |
| 335 | did not point at, but which the same reasoning would flag. |
| 336 | - Sweep the touched files for NEW issues the fixes themselves may have |
| 337 | introduced (broken behavior, lost features, regressions). |
| 338 | - Report findings as a flat list -- file:line + class + suggested fix. |
| 339 | |
| 340 | Then the orchestrator addresses every finding the subagent returns |
| 341 | BEFORE push. Loop the subagent if necessary until it returns a clean |
| 342 | review. Only then proceed to step 4b. |
| 343 | |
| 344 | A good subagent prompt template: |
| 345 | |
| 346 | > Re-review PR <N> end-to-end. The current diff is on branch <X>; the |
| 347 | > base is <master|...>. Recent fixes addressed: <list>. For the WHOLE |
| 348 | > diff (not just the recent fixes), find: |
| 349 | > 1. Similar patterns to the ones recently fixed that were NOT pointed |
| 350 | > at by reviewers but where the same reasoning applies. |
| 351 | > 2. Issues the recent fixes may have introduced (regressions, broken |
| 352 | > behavior, dropped features). |
| 353 | > 3. Anything in the diff that does not match the project's |
| 354 | > conventions (AGENTS.md, sibling files, the rest of the repo). |
| 355 | > Report a flat list of file:line + class + suggested fix. Be |
| 356 | > exhaustive; do not stop at 3-7 findings. |
| 357 | |
| 358 | ### 4b. Before pushing -- check CI for FAILURES (don't wait) |
| 359 | |
| 360 | ``` |
| 361 | bash .agents/skills/pr-reviews/scripts/ci-status.sh <PR_NUMBER> |
| 362 | ``` |
| 363 | |
| 364 | Exit codes: |
| 365 | - `0` -- all green, safe to push |
| 366 | - `2` -- runs in progress -- IGNORE this; push anyway. Waiting for CI |
| 367 | between iterations destroys throughput. The new push triggers fresh CI |
| 368 | on the new code, which is what matters. |
| 369 | - `3` -- runs failing -- fix the failures and bundle them into the push. |
| 370 | |
| 371 | CI failures unrelated to this PR (a flaky test on a different module, an |
| 372 | infra outage) are NOT in scope for this PR -- note them, surface to the |
| 373 | user, move on. Do not make drive-by fixes here. |
| 374 | |
| 375 | ### 5. Push, then re-trigger reviewers |
| 376 | |
| 377 | After pushing the fix commit(s): |
| 378 | |
| 379 | ``` |
| 380 | bash .agents/skills/pr-reviews/scripts/trigger-copilot.sh <PR_NUMBER> |
| 381 | bash .agents/skills/pr-reviews/scripts/trigger-cubic.sh <PR_NUMBER> |
| 382 | ``` |
| 383 | |
| 384 | Copilot re-runs when re-requested as a reviewer. cubic re-reviews when |
| 385 | mentioned in a new top-level PR comment. |
| 386 | |
| 387 | ### 6. Wait for new activity |
| 388 | |
| 389 | ``` |
| 390 | bash .agents/skills/pr-reviews/scripts/wait-for-activity.sh <PR_NUMBER> |
| 391 | ``` |
| 392 | |
| 393 | Default timeout 30 min, poll every 30 s. Returns 0 on new activity, 124 on |
| 394 | timeout. Both bots typically post a "no new findings" comment when they |
| 395 | have nothing left, so the loop ends naturally on a clean PR. |
| 396 | |
| 397 | What counts as "new activity": |
| 398 | - New issue comment / review comment / review on the PR. |
| 399 | - New commit pushed to the PR head. |
| 400 | - A review thread getting resolved or unresolved (often by a bot saying |
| 401 | "addressed; resolving" -- without this signal we'd miss thread state |
| 402 | flips and time out spuriously). |
| 403 | |
| 404 | ### 7. Loop |
| 405 | |
| 406 | Go back to step 1. Continue until ALL of these are true: |
| 407 | |
| 408 | - `fetch-all.sh` reports all review threads resolved. |
| 409 | - `fetch-sonar-findings.sh` reports zero open issues / hotspots that |
| 410 | this PR introduced (or the remaining ones are explicitly marked FP / |
| 411 | WontFix). |
| 412 | - The AI bots have posted a "no new findings" or equivalent comment |
| 413 | after their most recent re-trigger. |
| 414 | - `ci-status.sh` reports no failures caused by this PR (failures |
| 415 | unrelated to the PR are noted, surfaced to the user, but not fixed). |
| 416 | |
| 417 | When `wait-for-activity.sh` times out (30 min): |
| 418 | - Re-run `ci-status.sh`. If checks are still running, surface to the |
| 419 | user and stop -- the PR is in a clean intermediate state. |
| 420 | - If there are CI failures attributable to the PR, treat them as a new |
| 421 | finding and iterate (commit -> ci-check -> push -> retrigger -> wait). |
| 422 | - If the failures are unrelated, note them in the final report. |
| 423 | |
| 424 | ### 8. Final report |
| 425 | |
| 426 | When the loop ends, summarize for the user: |
| 427 | - Findings addressed (count by source: review threads, Sonar, CI). |
| 428 | - Any unrelated CI failures observed but not fixed (with check name + URL). |
| 429 | - Any human comments that need their attention. |
| 430 | - Current PR state (mergeable / blocked, decision, head SHA). |
| 431 | |
| 432 | ## Commit message hygiene |
| 433 | |
| 434 | Commit messages on the address-the-comments cycle should describe **the |
| 435 | change**, not the reviewer or the cycle: |
| 436 | |
| 437 | - BAD: "address copilot comments" |
| 438 | - BAD: "fix bot review feedback" |
| 439 | - GOOD: "scripts: fix dry-run env var name and printf format-string usage" |
| 440 | |
| 441 | Never reference an AI tool by name in commit messages or PR bodies. The |
| 442 | work matters; the tool that flagged it does not. |
| 443 | |
| 444 | (Comments on the PR are an exception when they're operational mentions |
| 445 | required by the bot itself: `@cubic-dev-ai please review again` is a |
| 446 | direct trigger for that bot, and the trigger script enforces it. Outside |
| 447 | operational triggers, the same rule applies to comments.) |
| 448 | |
| 449 | ## Replying to bots -- tone |
| 450 | |
| 451 | Be substantive but brief. The bot's prompt-text is verbose; your reply |
| 452 | doesn't have to be. Examples: |
| 453 | |
| 454 | - For a valid fix: "Fixed in <sha-or-paragraph>: <one-sentence what changed>." |
| 455 | - For a false positive: "False positive -- <one-sentence why>: <evidence |
| 456 | citation>." Add a code comment if it'll help future reviewers. |
| 457 | - For a partial fix: "Partial -- fixed the immediate case at <line>, but the |
| 458 | related <other-line> is intentional because <reason>." |
| 459 | |
| 460 | ## Replying to humans -- consult the user |
| 461 | |
| 462 | Maintainer / dev / community comments go to the user FIRST. Your message |
| 463 | should: |
| 464 | 1. Quote the relevant part of their comment. |
| 465 | 2. State your read of what they're asking for. |
| 466 | 3. Propose 1-3 options if it's a design call, or one option if obvious. |
| 467 | 4. Wait for the user's decision. |
| 468 | |
| 469 | Then act per their direction. Do not respond to humans on the user's |
| 470 | behalf without explicit direction. |
| 471 | |
| 472 | ## Bot directory |
| 473 | |
| 474 | | Bot | Role | Re-trigger | |
| 475 | |------------------------------|--------------------------------------------|--------------------------------------------------| |
| 476 | | `cubic-dev-ai[bot]` | Line-level code review | New PR comment mentioning `@cubic-dev-ai` | |
| 477 | | `copilot[bot]` | Line-level code review | Re-add as requested reviewer (`gh pr edit`) | |
| 478 | | `sonarqubecloud[bot]` | Quality-gate status | Auto, on each scan run -- read its issue comment | |
| 479 | | `github-actions[bot]` | CI status / labels | Auto, on each workflow run | |
| 480 | | `netdata-bot[bot]` | Repo automation (labels, etc.) | Auto | |
| 481 | |
| 482 | If a new AI reviewer appears in the project, classify it by adding to |
| 483 | `PR_AI_BOT_RE` in `_lib.sh` so the skill recognizes it. |
| 484 | |
| 485 | ## Failure modes -- quick diagnosis |
| 486 | |
| 487 | | Symptom | Likely cause | |
| 488 | |--------------------------------------------------------|----------------------------------------------------------------------| |
| 489 | | `fetch-all.sh` returns suspiciously round counts | Pagination missed pages. Re-run; fetch-all auto-probes when count is a multiple of 100. | |
| 490 | | `reply-thread.sh` -> 404 | Wrong comment id (use `databaseId` from `review-threads.json`, not the GraphQL node id). | |
| 491 | | `resolve-thread.sh` -> "thread not found" | Used REST id instead of GraphQL node id. | |
| 492 | | `trigger-copilot.sh` succeeds but no new review | Reviewer was already requested -- script removes-then-adds to force a fresh run. If still nothing, copilot may be quota-limited; wait. | |
| 493 | | `trigger-cubic.sh` succeeds but no new review | cubic ignores comments without an explicit `@cubic-dev-ai` mention. The script always prepends it. | |
| 494 | | `ci-status.sh` exits 2 (running) | CI hasn't finished. Push anyway -- waiting on CI between iterations destroys throughput. The next push triggers a fresh CI run on the new code, which is what matters. (See Step 4b.) | |
| 495 | | Bot keeps re-flagging the same line after a fix push | The bot didn't see the new commit because it wasn't re-triggered. | |
| 496 | | `wait-for-activity.sh` 124 timeout | Bots are silent -- could be done, could be quota-limited. Check `summary.txt`; if all threads resolved, you're done. | |
| 497 | |
| 498 | ## MANDATORY -- keep this skill alive |
| 499 | |
| 500 | If you (the agent) discover a new pattern, gotcha, working flow, correction, |
| 501 | or any piece of knowledge while running this skill -- update this `SKILL.md` |
| 502 | AND commit it BEFORE proceeding. Knowledge that isn't committed is lost. |
| 503 | |
| 504 | Examples of things to capture: |
| 505 | - A new AI reviewer bot that appears in the project (add to the directory + `PR_AI_BOT_RE`) |
| 506 | - A new common false-positive pattern that warrants a clarifying source comment |
| 507 | - A new GitHub API quirk (rate limits, undocumented response shapes, pagination edge cases) |
| 508 | - A retrigger mechanism that changed (e.g. copilot's re-request behavior) |