@cryptotaxi247 / netdata-1 / commits / d1d46100b

agents: add coverity-audit, sonarqube-audit, graphql-audit skills (#22296)

* agents: add coverity-audit, sonarqube-audit, graphql-audit skills Adds three repo-scoped AI agent skills under .agents/skills/ for the recurring static-analysis triage workflows: coverity-audit/ - Coverity Scan defect triage sonarqube-audit/ - SonarCloud findings triage graphql-audit/ - GitHub Code Scanning (CodeQL) triage Each skill is self-contained: a SKILL.md with frontmatter (name, description) plus a scripts/ directory with the shell helpers that drive each platform's API. Scripts auto-detect the repo root and read per-user secrets from the gitignored /.env at the repo root. They write run-time artifacts under the gitignored /.local/audits/ tree. AGENTS.md gains three new sections documenting the conventions: - AI agent skills: where skills live and the rule that any new knowledge must be committed back into SKILL.md - Local-only directory: the gitignored /.local/ for runtime artifacts - Per-user secrets: the gitignored /.env for tokens, cookies, keys The placeholder .agents/skills/.gitkeep is removed since the directory now has real content. The shell scripts share several conventions: - ASCII-only enforcement on comment bodies (Coverity and SonarCloud sit behind Cloudflare, which 403s non-ASCII payloads) - Repo-relative paths via git rev-parse, so scripts work from any cwd - Token / cookie masking in transparency output - Idempotent fetch loops (skip outputs that already exist on disk) - Read-only search/list helpers separated from write actions For Coverity specifically, the keepalive.sh script exits non-zero the moment a ping fails so the orchestrator's background-task notification fires immediately and the agent can recapture the cookie. * agents: auto-create .local/audits/<skill>/ on access The *_audit_dir() helpers in each skill's _lib.sh now run mkdir -p before returning the path. Callers can redirect output into subpaths (e.g. .local/audits/sonarqube/queue.json) without having to mkdir the parent first. * agents/coverity-audit: capture full operational knowledge The first cut of this skill was thin -- it had operational scaffolding but lost the API knowledge accumulated over hundreds of CIDs. This rewrite captures it. SKILL.md additions: - Server-side view-state cursor: documented, plus the explicit warning that the user must NOT touch the Coverity UI while a fetch runs (they would move the cursor out from under the scripts). - Coverity views (queue scopes): how to find a viewId, what the common view types mean (Outstanding, All in project, Dismissed, Fixed, Unclassified non-outstanding), how to switch without confusing the cursor. - Coverity attribute reference: classification (3), severity (1), action (2), external-reference (4) ID tables -- previously only in inline script comments. - displayImpact -> severity-ID mapping. - Suggested verdict vocabulary (TRUE_BUG_*, FALSE_POSITIVE_*, COSMETIC, CODE_GONE, NEEDS_HUMAN) with the classification/action it maps to. - CID vs defectInstanceId distinction and the resolve-cid-to-diid.sh primitive. - Project-specific FP guardrails (z-allocators, freez(NULL), DOUBLE_LINKED_LIST_*, buffer_*, STRING, ARAL, DICTIONARY, custom locks, glibc/musl, gcc/clang, daemon/plugin/streaming trust model). - Input trust boundaries table for FALSE_POSITIVE_TRUSTED_INPUT decisions. - Per-defect workdir convention. - Failure-mode quick-diagnosis table. The skill explicitly does NOT prescribe a review pipeline. Reviews are adhoc -- agree the approach with the user up front. For small batches (1-3 defects, the common case) a single agent or the user reading the bundle directly is usually fastest. Multi-model setups make sense only for large batches; the user picks the actual CLIs/models. New scripts (model-agnostic primitives): - resolve-cid-to-diid.sh -- CID -> defectInstanceId via /reports/defects.json - prepare-defect.sh -- bundle one defect (summary + details + source context + scratch TODO) under .local/audits/coverity/triage/<scope>/cid-<N>/ * agents: add pr-reviews skill for iterating PR comments Adds a fourth repo-scoped skill under .agents/skills/pr-reviews/ for the iterative PR comment / review handling workflow. The skill captures the rules that experience showed are non-negotiable: - Pagination paranoia. GitHub paginates everything; round-number counts (100, 200, 300) almost always mean the previous client missed the next page. fetch-all.sh auto-probes for an extra page when the count is a multiple of 100. - Address every comment, no exceptions. The bar is project performance, stability, and long-term maintainability. - Reply per-thread, one by one. No bulk replies, no mechanical 'fixed'. - When a bot finds a legit issue, search the whole PR for similar issues before pushing. AI reviewers surface their top 3-7 findings, not the full set; fixing only what they pointed at means dozens of round-trips. - Check CI before every push but never wait for CI between iterations. Throughput matters; CI lag does not block AI reviewer round-trips. - Re-trigger AI reviewers explicitly. They do not react to thread replies or pushed commits. Copilot needs a re-add as reviewer; cubic-dev-ai needs a top-level mention comment. - Some bots stop responding -- accept it, do not loop forever. Scripts: - fetch-all.sh -- paranoid-paginated fetch of issue comments, review comments, reviews, and review threads (GraphQL for thread isResolved). Writes a summary.txt with per-author counts and the open thread list. - list-open-threads.sh -- read from cached fetch; show open threads either full-body or as a one-line table. - reply-thread.sh -- post a reply inside an existing thread. - resolve-thread.sh -- mark a thread resolved (GraphQL). - ci-status.sh -- check current CI for failures before pushing. - trigger-cubic.sh -- post the @cubic-dev-ai mention comment. - trigger-copilot.sh -- re-add @copilot as a requested reviewer. - wait-for-activity.sh -- block until new activity (new comment / new review / new push) or 30-min timeout. - _lib.sh -- shared helpers, author classification regex. Author classes are handled differently: - AI bots -- iterate autonomously. - Informational bots (sonarqubecloud quality gate, etc.) -- read for signal, no reply needed. - Humans -- consult the user; do not reply on the user's behalf. AGENTS.md is updated to list the new skill alongside the other three. * agents/skills: portability, input validation, doc consistency, shellcheck Bundled fixes across the four skills, addressing both the cubic-dev-ai findings on PR 22296 and a sweep for similar patterns elsewhere. Portability (BSD/macOS): - Replace 'grep -qP "[^\\x00-\\x7F]"' with a portable 'tr -d "\\000-\\177"' pipeline in three places: coverity _lib.sh, sonarqube _lib.sh, and coverity update-triage.sh. 'grep -P' is GNU-only and breaks ASCII validation on BSD/macOS. - Replace 'mktemp --tmpdir' (GNU-only) with 'mktemp "${TMPDIR:-/tmp}/..."' in three places: coverity finalize-defect.sh, coverity update-triage.sh, pr-reviews fetch-all.sh. Input validation: - Add cov_require_numeric_cid() helper in coverity _lib.sh; call it from finalize-defect.sh, prepare-defect.sh, update-triage.sh, resolve-cid-to-diid.sh before interpolating CID into jq filters, URL params, or paths. Earlier versions trusted raw CLI input. - Switch jq filters in finalize-defect.sh and prepare-defect.sh from raw '${CID}' interpolation to jq '--argjson cid' for defense-in-depth even after the numeric guard. Repo-slug parsing: - The previous gh_repo_slug regex used '[^/.]+' which truncated repo names containing dots ('my.repo'). Replace with bash parameter expansion in graphql-audit _lib.sh and pr-reviews _lib.sh -- handles dots correctly and avoids regex altogether. Wildcard merge: - coverity fetch-table.sh combined pages with '${PREFIX}-page*.json', which would silently include stale page files from a prior larger run (e.g. PAGES=5 after a previous PAGES=7). Enumerate the requested page files explicitly. sonar-mark.sh: - list_open_hotspots_for_rule() embedded ${rule} inside a Python -c script via single-quoted bash interpolation -- shell metacharacters in the rule id could break parsing. Replace with jq '--arg rule' for the filter; consistent with the rest of the script's jq usage. - list_open_issues_for_rule() and list_open_hotspots_for_rule() were capped at the first 500 findings (a single ps=500 page) -- rules with more than 500 open findings would be silently truncated. Both now paginate via Sonar's p=N until paging.total is reached. - URL-encode the rule id before placing it in the issue-search query string (rule ids contain ':' which must be %3A). Documentation: - sonarqube SKILL.md documented 'SONAR_MARK_DRY_RUN' but the script actually reads 'SONAR_DRY_RUN'. The shown dry-run command would have performed real mutations. Renamed in the doc (two locations) to match the script. - sonarqube SKILL.md dry-run section now clarifies which calls are suppressed in dry-run (writes) vs not (reads -- needed so family-mode can show what it would have acted on). - coverity SKILL.md finalize-defect.sh invocation used '<SCOPE>' as the third arg placeholder where the script read '<phase>'. Renamed PHASE -> SCOPE in finalize-defect.sh for consistency with prepare-defect.sh (which already used 'scope') and updated the doc to match. shellcheck: - Mark color vars with 'shellcheck disable=SC2034' across all four _lib.sh files (they are referenced from sourcing scripts; shellcheck cannot see that). - Restructure 'set -a; source ...; set +a' across two lines so the shellcheck source-disable directive has a clear target line. - Drop unused 'http' and 'DIR' locals. - Replace 'printf >&2 "${COLOR}"' (variables in the format string) with 'printf >&2 "%s" "${COLOR}"' in sonarqube _lib.sh. shellcheck --severity=warning is now clean across all skill scripts. * agents/pr-reviews: emphasize per-thread reply-then-resolve cadence The 'one by one' rule is in the mandatory rules list, but the workflow section was loose enough that batch replies (prepare 14 reply texts, loop) felt acceptable. Tighten the wording: walk threads one at a time, reply, resolve, then move on. Bulk reply+resolve looks mechanical and erodes trust in the address pass. * agents/pr-reviews: pull SonarCloud findings, address all PR finding sources The pr-reviews skill was missing SonarCloud as a finding source and was not explicit about its job being broader than just review-comment threads. SonarCloud only posts a QualityGate summary to GitHub -- the actual issue list lives behind /api/issues/search?pullRequest=<N>. Without an explicit fetch, the agent has no way to see them. SKILL.md changes: - New top-level 'Your role on a PR' section that states the agent's job: bring the PR into merge-ready shape, addressing every legitimate finding from every source, not just GitHub review comments. Lists the five sources in priority order: humans, AI bot review comments, SonarCloud PR findings, CI failures relevant to the PR, anything else (Codacy, custom workflows). Documents the 'unrelated CI failures get noted but not fixed' rule. - New Step 1b 'Fetch SonarCloud PR findings' invokes the new fetch-sonar-findings.sh; new Step 1c notes CI signal as a third source. - New Step 3b walks each Sonar issue / hotspot, with a similar-pattern sweep and the option to mark FP via sonar-mark.sh from the sonarqube-audit skill (cross-skill integration). - Loop completion criteria now include zero PR-introduced Sonar findings and no CI failures attributable to the PR. - Final-report step lists what to summarize for the user, including any unrelated CI failures noted but not fixed. New script: - pr-reviews/scripts/fetch-sonar-findings.sh Pulls /api/issues/search and /api/hotspots/search filtered by pullRequest=<N>. Paginated up to total. Reuses SONAR_TOKEN / SONAR_HOST_URL / SONAR_PROJECT from .env (same vars sonarqube-audit uses). Writes sonar-issues.json and sonar-hotspots.json under .local/audits/pr-reviews/pr-<N>/, plus a stdout summary by rule and severity. coverity finalize-defect.sh: - Add explicit '*) ;;' default branch to the early-exit case statement for VERDICT. Sonar S131 (CRITICAL) flagged this. The default arm is a no-op; the next case statement handles all other verdicts. Documented the fall-through with a comment. sonarqube _lib.sh: - printf format-string fix from the earlier shellcheck pass dropped the ANSI escape interpretation; the literal backslash-octal-three-three was being printed verbatim instead of becoming ESC. Switch to %b (interprets backslash escapes) -- still safe with respect to SC2059 because the format string is a constant literal. The two Sonar S1135 'TODO' findings on prepare-defect.sh:18 and :109 were marked False Positive directly on SonarCloud (not in this commit): both reference the 'TODO.md' filename convention used by prepare-defect to scope per-defect notes inside the defect directory, not literal unfinished-task markers. * agents: holistic pre-push review fixes Second iteration after addressing the 14 cubic-dev-ai threads + the 3 SonarCloud findings on PR 22296. The skill's mandatory pre-push subagent review surfaced these: - pr-reviews now mandates a subagent holistic review BEFORE every push (Step 4a, plus rule #12 in the mandatory rules list). The skill context is biased toward the recent fix; a clean-context subagent re-reviewing the WHOLE diff is what catches similar patterns and new issues introduced by the fixes themselves. - pr-reviews scripts now validate PR numbers via pr_require_numeric() before interpolating them into REST paths or gh arguments. Eight scripts wired: fetch-all, fetch-sonar-findings, ci-status, list-open-threads, reply-thread, trigger-cubic, trigger-copilot, wait-for-activity. - graphql codeql-dismiss validates the alert number is a positive integer. - coverity update-triage.sh switched --arg cid + |tonumber to --argjson cid (consistent with finalize-defect.sh / prepare-defect.sh now); same for projectId. Numeric-validation guards added. - coverity _lib.sh:cov_load_env now validates COVERITY_PROJECT_ID is a positive integer at load time. - All numeric guards use ^[1-9][0-9]*$ rather than ^[0-9]+$ -- the error message says 'positive integer' so reject zero accordingly. - Color vars across all four _lib.sh redefined with $'...' so they hold real ESC bytes -- printf '%s' works and shellcheck SC2059 stays happy. Earlier change to printf '%s' had silently broken color rendering; rule #11 (smoke-test every fix, do not trust the linter alone) added to the mandatory rules. - sonarqube SKILL.md: replaced the 'shelldre / godre' language list with the actual SonarCloud language keys (c, cpp, go, javascript, py, shell, ...) and clarified that rule-id namespaces (the prefix before the colon in rule keys) are different from language keys -- the qualityprofile API takes the language key, not the rule-id namespace. - sonarqube _lib.sh: SONAR_ORG no longer required at env-load (no script uses it today; documented why it stays as an optional default). - Documented the audit-dir naming convention in AGENTS.md (skill '<topic>-audit/' writes to '.local/audits/<topic>/'). New tooling: - pr-reviews/scripts/fetch-sonar-findings.sh -- pulls PR-specific SonarCloud findings via /api/issues/search?pullRequest=<N> and /api/hotspots/search?pullRequest=<N>. Paginated. Writes sonar-issues.json + sonar-hotspots.json under the per-PR cache. The pr-reviews SKILL.md gained a 'Your role on a PR' section listing the five finding sources (humans, AI bots, Sonar, CI, others) and the rule that the agent's job is to bring the PR to merge-ready shape by addressing legitimate findings from every channel. * agents: address iteration-3 cubic findings + 4 holistic-review rounds Iteration 3 of the address-review cycle on PR 22296. Cubic-dev-ai returned 13 new findings on the previous push and the pre-push holistic subagent reviews (rounds 3 + 4) found another 8 real bugs the AI bots had not surfaced. Bundled fixes: Pagination correctness (the largest class): - pr-reviews/fetch-all.sh fetch_paranoid: 'gh api --paginate' writes per-page arrays back-to-back, NOT a single JSON array; 'jq length' on that input only counted the first page. Pipe through 'jq -s "add // []"' so all pages slurp into one array. Same fix applied to graphql-audit/codeql-list.sh which had the identical pattern. - pr-reviews/wait-for-activity.sh snapshot: switch from per-page '--jq length' summed via awk to defensive 'if type=="array" then length else 0 end' so a malformed page (object instead of array) does not get its key-count inflated into the sum. - pr-reviews/wait-for-activity.sh snapshot also now cursor-paginates the GraphQL reviewThreads connection -- PRs with >100 threads (the topology-maps PR has hundreds) would otherwise miss resolve/unresolve transitions on every thread past the first page. - sonarqube-audit: introduced sq_paginate helper that walks /api/*/search until paging.total. Refactored sonar-search.sh, sonar-mark.sh family-mode helpers, and pr-reviews/fetch-sonar-findings.sh to use it. fetch-sonar-findings.sh now sources sonarqube-audit/_lib.sh for cross-skill helper reuse rather than duplicating the paginator. - pr-reviews/fetch-all.sh GraphQL nested comments: bumped first:50 -> first:100 + added pageInfo and totalCount; logs a warning naming any thread whose comments truncated. Robustness: - sq_paginate validates each page is a JSON object with .paging and a recognised array key (issues/hotspots/components/rules/users); bails loudly on unknown payload rather than silently returning zero rows. - sq_run_read added: read-only API call helper that masks the token in the transparency log but does NOT skip in dry-run (read-only enumeration must run so the caller sees what would be acted on). - pr-reviews/wait-for-activity.sh GraphQL block always emits a threads_resolved=...open=... line, falling back to ERR placeholders on transient failure. Without this the line vanished intermittently and the snapshot diff falsely flagged 'new activity' on each return. - pr-reviews/wait-for-activity.sh diff in success branch wrapped with '|| true' so the diff exit-1-on-difference does not trip pipefail. Input validation: - pr-reviews/_lib.sh + graphql-audit/_lib.sh: the gh_repo_slug / pr_repo_slug helpers now return empty for non-github.com remotes (the skills are GitHub-only). PR_REPO_SLUG override validated as ^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$. - pr_require_slug + gh_require_slug helpers added; all 8 sites that derive a slug now use them so the failure mode is one consistent error. - coverity-audit/fetch-table.sh validates VIEW_ID and PAGES as positive integers before the loop; otherwise non-numeric PAGES would make seq produce no output and the final jq merge would block on stdin. - coverity-audit/prepare-defect.sh: source-context check now requires non-empty displayFile AND a regular file at ${ROOT}/${repo_file}; previously an empty displayFile resolved to ${ROOT}/ which IS a readable directory (the -r test passed) and awk would crash. - pr-reviews/reply-thread.sh: empty-body guard strips ALL whitespace ([[:space:]]) not just spaces. - sonarqube-audit/sonar-search.sh: --resolved and --status whitelisted. URL encoding consistency: - sq_url_encode lifted out of sonar-mark.sh into _lib.sh. sonar-search.sh now uses it on --rule values too -- previously the rule string was concatenated into the URL raw, so any reserved character (& = % space) could inject extra query parameters or be rejected by Cloudflare. ci-status.sh: - Header comment + WARNING text were saying 'pushing now will lose CI results' which contradicted the SKILL.md policy ('push anyway, fresh CI runs on the new code'). Reworded as informational. Doc fixes: - pr-reviews/SKILL.md: 'no AI tool names in commit messages / PR bodies' rule clarified to NOT ban operational mentions like @cubic-dev-ai (which the trigger script enforces). - pr-reviews/SKILL.md: ci-status exit-2 entry in the failure-modes table reconciled with the don't-wait policy. - pr-reviews/SKILL.md: wait-for-activity.sh now also detects review thread resolve/unresolve transitions (documented). - sonarqube-audit/SKILL.md: removed stale 'family-mode hits ps=500 ceiling' workaround (sq_paginate handles it). Style/naming: - local_cursor_arg renamed to cursor_arg (top-level scope; misleading prefix). - reviewer name fallback in pr.json summary: '.login // .name // "?"'. - trigger-copilot.sh comment about remove-reviewer rephrased to describe what we actually rely on. * agents: address iteration-4 cubic findings (3 P2/P3) + similar-pattern sweep Cubic-dev-ai's review of the previous push surfaced three more findings: - sonar-mark.sh confirm_family used ${ans,,} which is a bash 4+ feature and breaks on macOS bash 3.2. Replaced with portable 'tr [:upper:] [:lower:]' lowercase conversion. - coverity fetch-details.sh interpolated 'cid' (read from input JSON) into the output filename without numeric validation. Corrupted/manipulated input could escape OUT_DIR. cid + defectInstanceId now validated as positive integers before path construction; non-numeric rows are skipped with a clear log line. - coverity prepare-defect.sh: SCOPE arg landed in a path component (.local/audits/coverity/triage/<scope>/cid-N/) without validation. Added regex check ^[a-z][a-z0-9_-]*$ -- matches every documented scope (outstanding, dismissed, fixed, unclassified, all-in-project) and rejects path-escaping inputs. Cross-skill sweeps (rule #10): - No other ${var,,} / ${var,^^} sites anywhere in .agents/skills/. - No other JSON-string-to-filesystem-path interpolation outside the one fetched-details.sh site that was just fixed. - CLI-arg-to-path: SCOPE in prepare-defect.sh was the only validation gap; OUT_DIR/PREFIX are by-design user-controlled paths. Cubic finding on fetch-sonar-findings.sh:62 'group_by needs sort_by' verified false positive: jq's group_by sorts internally (see https://stedolan.github.io/jq/manual/#group_by(path_expression)). The thread reply documents this with the spec citation; no code change. Holistic pre-push review #5 ran on the resulting tree and recommended SHIP -- all CLI/JSON path sinks now validated, all bash 4+ idioms removed, all documented scopes still parse. * agents: iteration-5 + iteration-6 hardening (input validation, portability, doc fixes) Bundled fixes from cubic-dev-ai's last two review rounds, the holistic-review subagents, and the cross-skill sweeps. All addressed in one push per the new sync-barrier rule (Step 4-pre): re-fetch all finding sources immediately before push, fold in anything that arrived during the iteration, only push when fetches are clean. Pre-push sync barrier added to pr-reviews/SKILL.md (Step 4-pre + rule #13). Without it, reviewers post in parallel and findings that arrive mid-iteration get attributed to the next push, leaving the orchestrator and reviewers chronically one round out of sync. Hardening: - All 7 ' -r ' / '! -r ' path tests across the 4 skills upgraded to ' -f ... && -r ... ' / '! -f ... || ! -r ... ', so a directory or symlink-to-directory cannot pass the readability gate and crash on the read. - pr_repo_slug + gh_repo_slug: tightened the github.com host check from substring '*github.com*' (matched 'notgithub.com' and 'github.com.attacker.example.com') to three explicit prefixes covering SCP-style ssh, URL-style ssh, anonymous https, and credentialed https with x-access-token. Verified against fake-remote smoke tests. - pr-reviews/_lib.sh: PR_REPO_SLUG override now validated as ^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$ so an env var can't smuggle whitespace or shell metacharacters. - coverity/keepalive.sh: COVERITY_VIEW_OUTSTANDING validated as a positive integer before going into PING_URL. - coverity/fetch-details.sh: cid + defectInstanceId from input JSON validated as positive integers before path/URL interpolation. Non-numeric rows are skipped with a clear log line, counted in failed total. - coverity/fetch-table.sh: GET response now validated to be JSON with '.resultSet.results | type=="array"' before being treated as cached. A 200-with-HTML-body (Cloudflare challenge) no longer caches as 'valid' to confuse subsequent runs. - coverity/prepare-defect.sh: repo_file (from Coverity displayFile) now also rejects '..' segments to prevent path traversal escaping the repo root. - pr-reviews/list-open-threads.sh + reply-thread.sh: input file checks now require ' -f -r -s '. reply-thread.sh comment-id validated as positive integer; @file body must be readable non-empty regular file. - pr-reviews/resolve-thread.sh: thread-node-id validated against '^PRRT_[A-Za-z0-9_-]+$' before being passed to the GraphQL mutation. - pr-reviews/wait-for-activity.sh: timeout + poll args validated as positive integers. Pagination + concatenation correctness: - pr-reviews/fetch-all.sh: 'gh api --paginate' writes per-page arrays back-to-back, NOT one JSON array; previous 'jq length' on the concatenated stream only saw the first page. Now piped through 'jq -s "add // []"' to slurp + concatenate. - graphql-audit/codeql-list.sh: same fix applied (had identical bug). - pr-reviews/wait-for-activity.sh snapshot: switched to per-page '--jq length' summed via awk; defensive 'if type=="array" then length else 0 end' so a malformed page doesn't get key-counted. - pr-reviews/wait-for-activity.sh: GraphQL reviewThreads now cursor-paginated; 'threads_resolved=N_open=M' line emitted unconditionally with ERR fallback so transient GraphQL failure doesn't drop the line and falsely flag 'new activity'. - pr-reviews/fetch-all.sh GraphQL nested comments: bumped first:50 to first:100 + added pageInfo.hasNextPage + totalCount; warning logged per truncated thread. Sonar pagination + URL safety: - sonarqube-audit/_lib.sh: introduced sq_paginate that walks every page until paging.total, validates each page is an object with .paging and a recognised array key (issues/hotspots/components/ rules/users), bails loudly on unknown payload. Used by sonar-mark.sh family-mode helpers, sonar-search.sh, and pr-reviews/ fetch-sonar-findings.sh (which sources sonarqube-audit/_lib.sh cross-skill rather than duplicating). - sonarqube-audit/_lib.sh: added sq_run_read for read-only API calls (token-masked log line, no dry-run skip) so reads in family-mode still execute under SONAR_DRY_RUN. - sonarqube-audit/_lib.sh: lifted _url_encode out of sonar-mark.sh into the lib as sq_url_encode. sonar-search.sh now URL-encodes --rule values; previously a rule with reserved characters (':', '&', '=') could inject extra params or trigger Cloudflare. - sonar-search.sh: --rule arg now requires a value (was crashing under set -u when missing); positional params copied to local vars (Sonar S7679); --resolved/--status whitelisted. Portability: - sonar-mark.sh confirm_family: replaced ${ans,,} (bash 4+) with 'tr [:upper:] [:lower:]'; works on macOS bash 3.2. Doc fixes: - AGENTS.md /.local/ convention text + naming rule reconciled into a single statement (was written twice with inconsistent placeholders). - pr-reviews/SKILL.md: ci-status.sh referenced with the explicit repo-relative path. - graphql-audit/SKILL.md: codeql-list.sh / codeql-dismiss.sh examples use the explicit 'bash .agents/skills/.../...' paths instead of bare script names that assume PATH setup. - sonarqube-audit/SKILL.md: removed leading '+' that was rendering as a markdown list bullet. - pr-reviews/SKILL.md: failure-modes table entry for ci-status exit 2 reconciled with the don't-wait policy. - pr-reviews/SKILL.md: 'no AI tool names in commit messages / PR bodies' clarified to NOT ban operational mentions like @cubic-dev-ai (which the trigger script always prepends). - pr-reviews/SKILL.md: wait-for-activity.sh now lists thread-count as a tracked signal. - sonarqube-audit/SKILL.md: removed stale 'family-mode hits ps=500 ceiling' workaround; sq_paginate handles it. - coverity-audit/SKILL.md: finalize-defect.sh invocation uses <scope> consistently (matches prepare-defect.sh terminology); the script's third arg renamed PHASE -> SCOPE so doc and code agree. Style/structure: - Color vars across all four _lib.sh redefined with $'...' so they hold real ESC bytes; printf '%s' renders correctly without violating shellcheck SC2059. The earlier change to bare '%s' had silently broken color rendering -- rule #11 (smoke-test every fix) added. - shellcheck SC2034 silenced for color vars; SC1090 directive moved to its own line; unused locals dropped. - Naming: local_cursor_arg renamed to cursor_arg. - Reviewer name fallback in pr.json summary uses '.login // .name // "?"'. - finalize-defect.sh early-exit case has explicit '*) ;;' default (Sonar S131); two false-positive S1135 'TODO' findings on prepare-defect.sh marked False Positive directly on SonarCloud. * agents: clean shellcheck info-level findings + 2 cubic round-7 fixes CI's reviewdog/action-shellcheck reporter (github-pr-check) fails the check on ANY shellcheck finding regardless of severity, so info-level findings block the PR. Local shellcheck --severity=warning was clean, but CI was failing on: - SC1091 'Not following: ./_lib.sh' across 16 source-loading scripts. The 'shellcheck source=./_lib.sh' directive helps shellcheck try to follow but the relative path doesn't resolve under CI's working dir. Added an explicit 'shellcheck disable=SC1091' next to each source directive so the warning is suppressed without losing the source-path hint for local developers running 'shellcheck -x'. - SC2016 'Expressions don't expand in single quotes' on three GraphQL queries (fetch-all.sh, wait-for-activity.sh, resolve-thread.sh). The single-quoted body contains $owner / $name / $number / $threadId / $after as GraphQL placeholders, NOT shell variables. Suppressed with 'shellcheck disable=SC2016' on its own line above each call (combined comment+directive trips SC1072). Plus two findings cubic raised after the previous push: - pr-reviews/scripts/resolve-thread.sh: added 'pr_require_slug >/dev/null' for fail-fast on non-GitHub remotes. The GraphQL mutation operates by node-id and doesn't need the slug, but failing entry-point checks catch misconfiguration earlier than letting the mutation hit a non-github API. - pr-reviews/SKILL.md Step 4-pre (sync barrier): the prose mentioned 'comments, Sonar, CI' as the three sources to re-check, but only showed fetch-all.sh + fetch-sonar-findings.sh commands. Added ci-status.sh to the command list with a note that it's the third source and that in-scope CI failures (caused by the PR's diff) get folded into the same push, while unrelated CI failures are noted but not fixed. Sync barrier ran one more time before this push -- confirmed no new findings beyond the two above.

Costa Tsaousis committed Apr 28, 2026 at 08:11 UTC d1d46100b3b5e298eef7c05a3cebfb5d8f5265b2
31 files changed +3702
.agents/skills/.gitkeep
.agents/skills/coverity-audit/SKILL.md new
+473
@@ -0,0 +1,473 @@
1 +---
2 +name: coverity-audit
3 +description: Triage Coverity Scan defects (https://scan.coverity.com) for this project — fetch defect lists, fetch per-defect details, and apply triage decisions (Bug / FalsePositive / Intentional with severity, action, and a comment). Use when the user asks to "review Coverity defects", "triage Coverity findings", "fetch Coverity outstanding", or anything mentioning Coverity Scan, CIDs, or scan.coverity.com.
4 +---
5 +
6 +# Coverity Scan triage skill
7 +
8 +This skill drives the Coverity Scan unofficial JSON API (the public site has
9 +no documented API — the scripts mimic what the browser does) for the project
10 +configured in `.env`. Scripts auto-detect the repo root and write all
11 +artifacts under `<repo-root>/.local/audits/coverity/`.
12 +
13 +The skill captures **operational knowledge** (how the API works, where it
14 +trips up, what the data means). It does NOT prescribe a review pipeline —
15 +how you actually triage each defect (single model, multi-model, manual) is
16 +adhoc; agree the approach with the user up front.
17 +
18 +## MANDATORY — keep this skill alive
19 +
20 +If you (the agent) discover a new pattern, gotcha, working flow, correction,
21 +or any piece of knowledge while running this skill — update this `SKILL.md`
22 +AND commit it BEFORE proceeding. Knowledge that isn't committed is lost.
23 +
24 +Examples of things to capture:
25 +- New view IDs encountered (and what each represents)
26 +- New API endpoint or parameter behavior
27 +- New failure mode (Cloudflare quirks, session expiry signals, rate limits)
28 +- New FP guardrail you found via the codebase (idiom that Coverity mismodels)
29 +- New severity / classification mapping detail learned the hard way
30 +
31 +---
32 +
33 +## MANDATORY — startup sequence when this skill is invoked
34 +
35 +Do these steps in this order. Skipping any step costs the user their session.
36 +
37 +### Step 1 — agree the triage approach with the user
38 +
39 +Coverity reviews are usually 1-3 defects. Sometimes hundreds. Ask the user:
40 +
41 +- "How many defects are we looking at — one specific CID, or a sweep?"
42 +- "How do you want to review them — you read each one, you spawn one agent
43 + per defect, you want me to use multiple models for cross-checking, …?"
44 +
45 +Do NOT default to a heavy multi-stage pipeline; that's only worth setting up
46 +when there are tens of defects to crunch. For small batches a single agent
47 +or the user reading the bundle directly is usually faster.
48 +
49 +If the user does want a multi-model approach, a sensible (not prescribed)
50 +shape is: cheap models surface ideas, the strongest coding model produces
51 +the actual analysis + fix, the strongest reviewing model sanity-checks the
52 +result. The user picks the actual CLIs/models — this skill does not assume
53 +any specific tool.
54 +
55 +### Step 2 — ask the user for a fresh cookie
56 +
57 +Coverity Scan auth is cookie-based and tied to a live browser session. Give
58 +the user the exact recipe:
59 +
60 +> 1. Open https://scan.coverity.com/projects/<owner>-<repo>?tab=overview
61 +> (replace `<owner>-<repo>` with the project slug, e.g. `netdata-netdata`).
62 +> 2. Click **"View Defects"** at the top right -- a new tab opens to
63 +> https://scan4.scan.coverity.com/# (or `scanN.scan.coverity.com` --
64 +> Coverity load-balances; use whichever URL you land on as `COVERITY_HOST`).
65 +> 3. **Keep that tab open the whole time we work.** Closing it kills the
66 +> session immediately.
67 +> 4. Press F12 to open DevTools, switch to the Network tab.
68 +> 5. Click any defect in the list -- 3-4 requests appear in the Network tab.
69 +> 6. Right-click any of those requests, select Copy > **Copy as cURL**.
70 +> 7. Paste the entire curl command back to me.
71 +
72 +If the curl the user pastes does NOT contain a `-b 'cookie=...'` line, ask
73 +again -- they probably picked "Copy as fetch" or "Copy as Node.js fetch".
74 +
75 +### Step 3 — save the cookie in `.env`
76 +
77 +Extract the value of the `-b` argument from the user's curl and write/update
78 +`<repo-root>/.env` with:
79 +
80 +```bash
81 +COVERITY_COOKIE='<paste-the-entire-cookie-blob-here>'
82 +COVERITY_PROJECT_ID=<numeric projectId from the URL or table.json query>
83 +COVERITY_VIEW_OUTSTANDING=<viewId of the Outstanding view>
84 +COVERITY_HOST=https://scan4.scan.coverity.com
85 +```
86 +
87 +`.env` is gitignored. The cookie blob must include both `COVJSESSIONID-build`
88 +and `XSRF-TOKEN`. The scripts extract `XSRF-TOKEN` automatically.
89 +
90 +### Step 4 — start the keepalive (background)
91 +
92 +```
93 +Bash tool with run_in_background=true,
94 +command="bash .agents/skills/coverity-audit/scripts/keepalive.sh"
95 +```
96 +
97 +The keepalive **exits non-zero the moment a ping fails** (session expired,
98 +browser tab closed, cookie went bad). The orchestrator's background-task
99 +notification fires immediately so you know to ask for a fresh cookie and
100 +restart.
101 +
102 +**Stop the keepalive at the end of the triage session** by killing its
103 +background task.
104 +
105 +### Step 5 — proceed with the actual triage work
106 +
107 +Now (and only now) is it safe to fetch tables, fetch details, prepare
108 +defect bundles, finalize verdicts.
109 +
110 +---
111 +
112 +## CRITICAL — server-side view state
113 +
114 +Coverity's table API has a **stateful, server-side view cursor**. The
115 +`/views/table.json` POST changes server-state (which page is "current"),
116 +then the `/reports/table.json` GET reads whatever the current state is.
117 +
118 +This has two consequences:
119 +
120 +### 1. Pagination is two-step, not one-shot
121 +
122 +Per page:
123 +1. `POST /views/table.json {projectId, viewId, pageNum}` -- moves the cursor
124 +2. `GET /reports/table.json?projectId=...&viewId=...` -- reads the page
125 +
126 +`fetch-table.sh` handles both steps and the page loop.
127 +
128 +### 2. **The user MUST NOT touch the Coverity UI while a fetch is running**
129 +
130 +If the user clicks a different view, scrolls to a different page, sorts a
131 +column, applies a filter — the server-side cursor moves under our scripts'
132 +feet. The scripts will then fetch garbage (rows from whatever view the user
133 +just opened, not the view the script asked for).
134 +
135 +Tell the user explicitly: "I'm about to fetch the defect list. Don't click
136 +in the Coverity tab until I say I'm done."
137 +
138 +If the user accidentally interferes, re-run the fetch script — it's
139 +idempotent on cached pages but not on already-corrupted ones, so delete the
140 +output JSONs and re-fetch from page 1.
141 +
142 +---
143 +
144 +## Coverity views (queue scopes)
145 +
146 +Coverity organizes defects into named **views**, each with a numeric
147 +`viewId`. The skill operates per-view.
148 +
149 +### How to find a view's ID
150 +
151 +In the Coverity UI:
152 +- Click the project's defects view, then the view selector dropdown.
153 +- Each view has a URL like `https://scan4.scan.coverity.com/#viewId=NNNNN`.
154 +- That `NNNNN` is the value to put in `.env` (or pass to `fetch-table.sh`).
155 +
156 +### Common view types you'll encounter
157 +
158 +The default project ships with these (the IDs are project-specific):
159 +
160 +- **Outstanding** — the live queue: defects neither classified nor dismissed.
161 + Where day-to-day triage happens. Save its viewId as
162 + `COVERITY_VIEW_OUTSTANDING` (also used by the keepalive).
163 +- **All in project** — every defect, including already-classified ones.
164 + Useful for batch rescans or full audits.
165 +- **Dismissed** — defects classified as False Positive / Intentional and
166 + ignored. Useful for re-review when the underlying code changed.
167 +- **Fixed** — defects Coverity now considers fixed. Verification queue.
168 +- **Unclassified non-outstanding** — corner cases.
169 +
170 +When a user asks you to operate on a non-default view, ask them to give you
171 +its viewId from the UI. You can have multiple `COVERITY_VIEW_*` env vars.
172 +
173 +### Switching views without confusing the cursor
174 +
175 +The view-state cursor described above is **per-session**. If you want to
176 +fetch view A then view B, do them sequentially (not concurrently): each
177 +`fetch-table.sh` call sends its own POST that resets the cursor. There is
178 +no need to "go back" -- just call it again with the next viewId.
179 +
180 +---
181 +
182 +## Coverity attribute reference
183 +
184 +Triage values are sent as integer **attribute IDs**, not names:
185 +
186 +### Classification (attribute 3)
187 +
188 +| ID | Name |
189 +|----|----------------|
190 +| 20 | Unclassified |
191 +| 21 | Pending |
192 +| 22 | False Positive |
193 +| 23 | Intentional |
194 +| 24 | Bug |
195 +
196 +### Severity (attribute 1)
197 +
198 +| ID | Name |
199 +|----|-------------|
200 +| 10 | Unspecified |
201 +| 11 | Major |
202 +| 12 | Moderate |
203 +| 13 | Minor |
204 +
205 +### Action (attribute 2)
206 +
207 +| ID | Name |
208 +|----|-------------------|
209 +| 1 | Undecided |
210 +| 2 | Fix Required |
211 +| 3 | Fix Submitted |
212 +| 4 | Modeling Required |
213 +| 5 | Ignore |
214 +
215 +### External reference (attribute 4)
216 +
217 +Free-form string. The scripts always send `null`.
218 +
219 +### Sensible (verdict → attributes) mappings
220 +
221 +These are what `finalize-defect.sh` applies when given a verdict from the
222 +suggested vocabulary below:
223 +
224 +| Outcome | classification (3) | action (2) |
225 +|-------------------|--------------------|------------|
226 +| Real bug, fixed | 24 Bug | 3 Fix Submitted |
227 +| False positive | 22 False Positive | 5 Ignore |
228 +| Cosmetic / harmless | 23 Intentional | 5 Ignore |
229 +
230 +### Coverity displayImpact → severity ID
231 +
232 +`finalize-defect.sh` maps `defect-summary.json#displayImpact` to severity:
233 +
234 +| displayImpact | severity ID | name |
235 +|---------------|-------------|-------------|
236 +| `High` | 11 | Major |
237 +| `Medium` | 12 | Moderate |
238 +| `Low` | 13 | Minor |
239 +| `null` / missing | 10 | Unspecified |
240 +
241 +---
242 +
243 +## Suggested verdict vocabulary
244 +
245 +This is the taxonomy used historically; you can reuse it or replace it. The
246 +`finalize-defect.sh` script understands these names directly:
247 +
248 +### Real bugs (classification = 24 Bug, action = 3 Fix Submitted)
249 +
250 +| Verdict | When to use |
251 +|-------------------------------|----------------------------------------------------------------------|
252 +| `TRUE_BUG_MEMORY_CORRUPTION` | OOB read/write, UAF, double-free, type confusion, stack-escape UAF. |
253 +| `TRUE_BUG_CRASH` | Reachable NULL deref / div-by-zero / assert / fatal. No corruption. |
254 +| `TRUE_BUG_RESOURCE_LEAK` | fd / memory / lock / refcount leak that accumulates on a reachable path. |
255 +| `TRUE_BUG_LOGIC` | Wrong result, wrong metric, wrong stored/transmitted data. No crash. |
256 +| `TRUE_BUG_UB` | Spec-UB (signed overflow, strict aliasing) compiles today but latent.|
257 +
258 +### False positives (classification = 22 FP, action = 5 Ignore)
259 +
260 +| Verdict | When to use |
261 +|----------------------------------|--------------------------------------------------------------|
262 +| `FALSE_POSITIVE_GUARD_EXISTS` | The code has a check Coverity failed to track. |
263 +| `FALSE_POSITIVE_UNREACHABLE` | The flagged path cannot be reached under any realistic state.|
264 +| `FALSE_POSITIVE_TRUSTED_INPUT` | Tainted source is actually trusted (root-owned local file). |
265 +| `FALSE_POSITIVE_TOOL_MODEL` | Coverity's semantic model is wrong (e.g. doesn't know `mallocz` cannot return NULL). |
266 +| `IMPOSSIBLE_CONDITIONS` | Combination of states existing invariants prevent. |
267 +
268 +### Cosmetic (classification = 23 Intentional, action = 5 Ignore)
269 +
270 +| Verdict | When to use |
271 +|-------------|--------------------------------------------------------------|
272 +| `COSMETIC` | Not a bug: unused value, dead branch, redundant expression. |
273 +
274 +### Bookkeeping (no Coverity update)
275 +
276 +| Verdict | When to use |
277 +|---------------|------------------------------------------------------------------------|
278 +| `CODE_GONE` | The flagged file/function no longer exists. (`finalize-defect.sh` skips.) |
279 +| `NEEDS_HUMAN` | Genuinely unsure after reasonable investigation. (`finalize-defect.sh` skips.) |
280 +
281 +---
282 +
283 +## Per-defect workdir convention
284 +
285 +`prepare-defect.sh` creates a per-defect directory under
286 +`.local/audits/coverity/triage/<scope>/cid-<N>/` with:
287 +
288 +- `defect-summary.json` — the row from the table dump
289 +- `defect-details.json` — the per-defect details (event trace, CWE, checker)
290 +- `source-context.c` — ~150 lines of source around the main event
291 +- `TODO.md` — per-defect notes; keep all per-defect artifacts inside
292 +
293 +Whatever review approach the user picks, write its outputs here too (e.g.
294 +analyzer reports, decider reasoning, commit message draft, build-verify
295 +log). Don't pile per-defect stuff at the repo root.
296 +
297 +---
298 +
299 +## CID vs defectInstanceId
300 +
301 +Two IDs for one defect, both flying around the API:
302 +
303 +- **`cid`** is stable across scans. Use it for tracking, comments, your
304 + permanent records.
305 +- **`defectInstanceId`** is per-scan. The `defectdetails.json` endpoint
306 + takes a `defectInstanceId`, NOT a `cid`. Each `table.json` row carries the
307 + current `lastDefectInstanceId` for its CID.
308 +
309 +When all you have is a CID (e.g. you're acting on an old list, or working
310 +from external triage notes), use:
311 +
312 +```
313 +bash .agents/skills/coverity-audit/scripts/resolve-cid-to-diid.sh <cid>
314 +# prints the current defectInstanceId, or "GONE" if Coverity no longer reports it
315 +```
316 +
317 +Internally it queries `/reports/defects.json?cid=N` and parses
318 +`defectInstanceId` out of the returned `.url` field.
319 +
320 +---
321 +
322 +## Workflow
323 +
324 +### Fetch a view's table
325 +
326 +```
327 +bash .agents/skills/coverity-audit/scripts/fetch-table.sh \
328 + "${COVERITY_VIEW_OUTSTANDING}" 7 .local/audits/coverity/raw/outstanding
329 +```
330 +
331 +Produces `.local/audits/coverity/raw/outstanding-page1.json` ... and a
332 +combined flat array at `.local/audits/coverity/raw/outstanding-all.json`.
333 +Pass the right page count (visible in the UI) for the view.
334 +
335 +**Reminder**: the user must not touch the UI during a fetch.
336 +
337 +### Fetch per-defect details
338 +
339 +```
340 +bash .agents/skills/coverity-audit/scripts/fetch-details.sh \
341 + .local/audits/coverity/raw/outstanding-all.json \
342 + .local/audits/coverity/details/outstanding
343 +```
344 +
345 +One file per CID at `<details>/cid-<N>.json`. Idempotent.
346 +
347 +### Bundle a defect for review
348 +
349 +```
350 +bash .agents/skills/coverity-audit/scripts/prepare-defect.sh <CID> outstanding
351 +```
352 +
353 +Creates `.local/audits/coverity/triage/outstanding/cid-<N>/` with the bundle.
354 +The actual review (single-model, multi-model, human) is **adhoc** — agree
355 +the approach with the user.
356 +
357 +### Apply a verdict
358 +
359 +After review and (if needed) a fix commit:
360 +
361 +```
362 +bash .agents/skills/coverity-audit/scripts/finalize-defect.sh \
363 + <CID> <VERDICT> <scope> .local/audits/coverity/triage/<scope>/cid-<N>/comment.txt \
364 + [<commit-sha>]
365 +```
366 +
367 +`<scope>` is the same name `prepare-defect.sh` uses (e.g. `outstanding`,
368 +`dismissed`, `fixed`, `unclassified`).
369 +
370 +This:
371 +- Skips silently for `NEEDS_HUMAN` and `CODE_GONE`.
372 +- Reads `displayImpact` from the table dump to derive severity.
373 +- Appends `Fix commit: <sha>` to the comment when a SHA is given.
374 +- Posts JSON to `/sourcebrowser/updatedefecttriage.json`.
375 +
376 +For scopes other than "outstanding" (re-triaging dismissed/fixed/etc.),
377 +finalize-defect prints a warning -- the caller is asserting that the new
378 +verdict disagrees with the existing classification. It still applies.
379 +
380 +---
381 +
382 +## ASCII-only comments — non-negotiable
383 +
384 +Coverity Scan sits behind Cloudflare. The WAF rejects bodies containing
385 +non-ASCII bytes (em-dashes, smart quotes, accented letters) with a 403
386 +Cloudflare challenge that looks like an expired-session error but isn't.
387 +
388 +- Use `--` instead of em-dash (U+2014).
389 +- Use straight quotes `"` `'` instead of smart quotes.
390 +- The scripts reject non-ASCII before the network round-trip.
391 +
392 +---
393 +
394 +## Project-specific FP guardrails (Netdata)
395 +
396 +Most of the cost of a Coverity audit is rejecting false positives. Before
397 +calling something a bug in this codebase, rule out these idioms — Coverity
398 +mis-models all of them:
399 +
400 +1. **`z`-suffix allocators never fail.** `mallocz`, `callocz`, `reallocz`,
401 + `strdupz`, `strndupz`, `mallocz_flex` call `fatal()` on OOM. They cannot
402 + return NULL. Any "possible NULL deref after `mallocz`" warning is FP.
403 +2. **`freez(NULL)` is safe.** Same for `string_freez`, etc.
404 +3. **`DOUBLE_LINKED_LIST_*` macros** (`libnetdata/linked-lists.h`) manage
405 + prev/next with their own invariants. Raw pointer manipulation inside
406 + them is expected.
407 +4. **`buffer_*` API** (`libnetdata/buffer/`) auto-grows the underlying
408 + `buffer->buffer[]` on write. Direct indexing of `buffer->buffer[N]` from
409 + callers is the risk, not the API.
410 +5. **`STRING` is refcounted and interned** (`libnetdata/string/`).
411 + `string_strdupz` increments refcount on an existing intern;
412 + `string_dup` acquires a new reference. Mixing them is a refcount bug.
413 +6. **`ARAL` is a slab allocator** (`libnetdata/aral/`). Objects are reused
414 + — UAF looks different here: the same address is re-issued. A pointer
415 + that "still works" after `aral_freez` may be a reused object.
416 +7. **`DICTIONARY`** has `dictionary_acquired_item_get` / `_release` with
417 + refcounts. Raw access bypasses the refcount.
418 +8. **Custom locks** (`spinlock_lock`, `rw_spinlock_*`) — not pthread.
419 + Coverity MISSING_LOCK warnings often misunderstand them.
420 +9. **Platform**: glibc + musl. Watch glibc-only assumptions
421 + (e.g. `strerror_r` signature).
422 +10. **Compilers**: gcc + clang both must compile.
423 +11. **Process model**: long-running daemon, spawns plugin subprocesses over
424 + a line-based stdin/stdout protocol. Plugin stdin is typically trusted;
425 + streaming peers are UNTRUSTED remote peers over TCP.
426 +
427 +---
428 +
429 +## Input trust boundaries (Netdata)
430 +
431 +Used to decide whether a tainted-data path is `FALSE_POSITIVE_TRUSTED_INPUT`:
432 +
433 +| Source | Trust | Module |
434 +|------------------------------|--------------------------------|------------------------------------|
435 +| Local `/proc`, `/sys` | Trusted (root-owned) | `collectors/proc.plugin/`, `cgroups.plugin/` |
436 +| Plugin stdin (our plugins) | Trusted | `plugins.d/`, `collectors/*.plugin/` |
437 +| Streaming peer (remote agent)| **UNTRUSTED** | `streaming/`, `stream-*` |
438 +| HTTP request (dashboard API) | Semi-trusted (usually localhost) | `web/api/`, `web/server/` |
439 +| MCP request | Semi-trusted (localhost) | `web/mcp/` |
440 +| Cloud (aclk) | Trusted (TLS + token to Netdata Cloud) | `aclk/` |
441 +| Config files | Trusted (root-owned) | `daemon/config/`, `health/` |
442 +
443 +---
444 +
445 +## Failure modes — quick diagnosis
446 +
447 +| Symptom | Likely cause |
448 +|------------------------------------------------------|-------------------------------------------------------------|
449 +| HTTP 401 / 403 / 302, response is HTML | Session expired. Recapture cookie from browser. |
450 +| HTTP 403 with Cloudflare challenge HTML | Either non-ASCII in comment, or browser tab closed. |
451 +| keepalive.sh exits non-zero | Same as above. Ask user for a fresh cookie. |
452 +| HTTP 200 but `defectStatus` empty | XSRF token stale. Recapture cookie. |
453 +| `Could not parse session from .env` | Wrong cookie format. Paste the FULL `-b 'k=v; ...'` string. |
454 +| Fetched rows look wrong (different view) | User clicked in the UI mid-fetch. Delete output JSONs and re-fetch. |
455 +| `.url` field missing in `/reports/defects.json` reply | Coverity no longer reports this CID -- treat as `CODE_GONE`. |
456 +| Cookie expires in mid-run despite keepalive | Browser tab was closed. Reopen the tab and recapture cookie. |
457 +
458 +---
459 +
460 +## Recurring tips
461 +
462 +- The `lastDefectInstanceId` field, NOT `cid`, is what `defectdetails.json` wants.
463 +- Coverity caches results aggressively; if a CID disappears from "Outstanding"
464 + immediately after finalize, a fresh fetch can take a few seconds to reflect.
465 +- Idempotence: `fetch-details.sh` skips files already present, so partial runs
466 + are safe to re-invoke.
467 +- `prepare-defect.sh` uses Coverity's `displayFile` and the main-event line
468 + to extract source context. If Coverity's line numbers are stale (after a
469 + refactor), the context may not center on the current code -- use it as a
470 + hint, not an authoritative location.
471 +- Coverity load-balances across `scanN.scan.coverity.com` hosts. Whatever
472 + hostname your browser landed on must be the one in `COVERITY_HOST`;
473 + cookies are per-host.
.agents/skills/coverity-audit/scripts/_lib.sh new
+108
@@ -0,0 +1,108 @@
1 +#!/usr/bin/env bash
2 +# Common helpers for coverity-audit scripts.
3 +# Sourced from the per-action scripts; not executed directly.
4 +
5 +set -euo pipefail
6 +
7 +# ANSI colors for transparent output (per the project's run() pattern).
8 +#
9 +# IMPORTANT: define with $'...' so the variables contain real ESC bytes,
10 +# not the literal four-character string "\033". This way both `echo -e
11 +# "${COV_RED}..."` and `printf '%s' "${COV_RED}..."` render correctly --
12 +# without forcing every printf format string to be the variable itself
13 +# (which trips shellcheck SC2059) or %b (which adds inconsistency).
14 +#
15 +# Color vars are referenced by sourcing scripts; shellcheck cannot see that.
16 +# shellcheck disable=SC2034
17 +COV_RED=$'\033[0;31m'
18 +# shellcheck disable=SC2034
19 +COV_GREEN=$'\033[0;32m'
20 +# shellcheck disable=SC2034
21 +COV_YELLOW=$'\033[1;33m'
22 +# shellcheck disable=SC2034
23 +COV_GRAY=$'\033[0;90m'
24 +# shellcheck disable=SC2034
25 +COV_NC=$'\033[0m'
26 +
27 +# Locate the repo root by walking up from the script directory.
28 +# This way the scripts work no matter where the user runs them from.
29 +cov_repo_root() {
30 + git -C "$(dirname "${BASH_SOURCE[0]}")" rev-parse --show-toplevel
31 +}
32 +
33 +# Source `<repo-root>/.env` if it exists. .env is the user's local-only
34 +# secrets file (gitignored). It must export at least:
35 +# COVERITY_COOKIE — the full Cookie header value pasted from a curl
36 +# copy-as-cURL captured in DevTools
37 +# COVERITY_PROJECT_ID — Coverity Scan numeric projectId (constant per project)
38 +# COVERITY_HOST — defaults to https://scan4.scan.coverity.com
39 +# Optional:
40 +# COVERITY_VIEW_OUTSTANDING — viewId of the Outstanding view
41 +# COVERITY_USER_AGENT — overridable UA string
42 +cov_load_env() {
43 + local root env
44 + root="$(cov_repo_root)"
45 + env="${root}/.env"
46 + if [[ ! -f "${env}" || ! -r "${env}" ]]; then
47 + echo -e "${COV_RED}[ERROR]${COV_NC} Missing ${env}. See SKILL.md for the .env template." >&2
48 + return 1
49 + fi
50 + set -a
51 + # shellcheck disable=SC1090
52 + source "${env}"
53 + set +a
54 +
55 + : "${COVERITY_COOKIE:?COVERITY_COOKIE is empty in .env — paste a fresh cookie from the browser}"
56 + : "${COVERITY_PROJECT_ID:?COVERITY_PROJECT_ID is empty in .env}"
57 + if [[ ! "${COVERITY_PROJECT_ID}" =~ ^[1-9][0-9]*$ ]]; then
58 + echo -e "${COV_RED}[ERROR]${COV_NC} COVERITY_PROJECT_ID must be a positive integer, got: '${COVERITY_PROJECT_ID}'" >&2
59 + return 1
60 + fi
61 + : "${COVERITY_HOST:=https://scan4.scan.coverity.com}"
62 + : "${COVERITY_USER_AGENT:=Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/147.0.0.0 Safari/537.36}"
63 + export COVERITY_COOKIE COVERITY_PROJECT_ID COVERITY_HOST COVERITY_USER_AGENT
64 +
65 + # Extract XSRF-TOKEN from the cookie string.
66 + COVERITY_XSRF="$(printf '%s' "${COVERITY_COOKIE}" | sed -n 's/.*XSRF-TOKEN=\([^;]*\).*/\1/p')"
67 + if [[ -z "${COVERITY_XSRF}" ]]; then
68 + echo -e "${COV_RED}[ERROR]${COV_NC} XSRF-TOKEN not found inside COVERITY_COOKIE. Did you paste the full Cookie header?" >&2
69 + return 1
70 + fi
71 + export COVERITY_XSRF
72 +}
73 +
74 +# Audit artifacts go under .local/audits/coverity/ at the repo root.
75 +# .local/ is gitignored -- see AGENTS.md for the convention.
76 +# Creates the directory on first call so callers can redirect output into
77 +# subpaths without thinking about it.
78 +cov_audit_dir() {
79 + local root dir
80 + root="$(cov_repo_root)"
81 + dir="${root}/.local/audits/coverity"
82 + mkdir -p "${dir}"
83 + echo "${dir}"
84 +}
85 +
86 +# Reject non-ASCII bytes in a string. Coverity's edge (Cloudflare) rejects
87 +# em-dashes and smart quotes with a 403 challenge; far better to fail before
88 +# the network round-trip than to debug a Cloudflare block.
89 +#
90 +# `tr -d '\000-\177'` deletes ALL ASCII bytes; anything left is non-ASCII.
91 +# This is portable across GNU and BSD/macOS (unlike `grep -P`, which is GNU-only).
92 +cov_require_ascii() {
93 + local s="$1"
94 + if LC_ALL=C printf '%s' "${s}" | LC_ALL=C tr -d '\000-\177' | grep -q .; then
95 + echo -e "${COV_RED}[ERROR]${COV_NC} Comment contains non-ASCII characters. Cloudflare blocks them. Replace em-dashes with '--' and curly quotes with straight quotes." >&2
96 + return 1
97 + fi
98 +}
99 +
100 +# CID validation: Coverity CIDs are positive integers (>= 1). Reject anything
101 +# else before interpolating into jq filters, URLs, or paths.
102 +cov_require_numeric_cid() {
103 + local cid="$1"
104 + if [[ ! "${cid}" =~ ^[1-9][0-9]*$ ]]; then
105 + echo -e "${COV_RED}[ERROR]${COV_NC} CID must be a positive integer, got: '${cid}'" >&2
106 + return 1
107 + fi
108 +}
.agents/skills/coverity-audit/scripts/fetch-details.sh new
+86
@@ -0,0 +1,86 @@
1 +#!/usr/bin/env bash
2 +# Fetch per-defect details from Coverity Scan for each CID in a table.json dump.
3 +#
4 +# Usage:
5 +# fetch-details.sh <input-defects.json> <output-dir>
6 +#
7 +# Reads cookies/XSRF/UA from .env via _lib.sh.
8 +# For each row, calls /sourcebrowser/defectdetails.json?defectInstanceId=<lastDefectInstanceId>.
9 +# Skips rows whose output file already exists (idempotent; safe to re-run).
10 +
11 +set -euo pipefail
12 +
13 +# shellcheck source=./_lib.sh
14 +# shellcheck disable=SC1091
15 +source "$(dirname "$0")/_lib.sh"
16 +cov_load_env
17 +
18 +INPUT="${1:?usage: $0 <input-defects.json> <output-dir>}"
19 +OUT_DIR="${2:?usage: $0 <input-defects.json> <output-dir>}"
20 +
21 +if [[ ! -f "${INPUT}" || ! -r "${INPUT}" ]]; then
22 + echo -e "${COV_RED}[ERROR]${COV_NC} input file not a readable regular file: '${INPUT}'" >&2
23 + exit 1
24 +fi
25 +
26 +mkdir -p "${OUT_DIR}"
27 +
28 +TOTAL="$(jq 'length' "${INPUT}")"
29 +echo -e "${COV_GRAY}Fetching details for ${TOTAL} defects into ${OUT_DIR}${COV_NC}" >&2
30 +
31 +i=0
32 +fetched=0
33 +skipped=0
34 +failed=0
35 +
36 +while IFS=$'\t' read -r cid defect_instance_id; do
37 + i=$((i + 1))
38 + # cid comes from the input JSON (Coverity-supplied) and lands in the
39 + # output filename. Validate numeric before path construction so a
40 + # corrupted/manipulated INPUT cannot write outside OUT_DIR.
41 + if [[ ! "${cid}" =~ ^[1-9][0-9]*$ ]]; then
42 + failed=$((failed + 1))
43 + echo -e "${COV_RED}[${i}/${TOTAL}] non-numeric cid in input: '${cid}' -- skipping${COV_NC}" >&2
44 + continue
45 + fi
46 + if [[ ! "${defect_instance_id}" =~ ^[1-9][0-9]*$ ]]; then
47 + failed=$((failed + 1))
48 + echo -e "${COV_RED}[${i}/${TOTAL}] cid=${cid} non-numeric defectInstanceId: '${defect_instance_id}' -- skipping${COV_NC}" >&2
49 + continue
50 + fi
51 + out_file="${OUT_DIR}/cid-${cid}.json"
52 + if [[ -s "${out_file}" ]]; then
53 + skipped=$((skipped + 1))
54 + continue
55 + fi
56 + sleep 0.3
57 +
58 + http_code="$(curl -sS --max-time 30 \
59 + -w '%{http_code}' \
60 + -o "${out_file}" \
61 + -H "accept: application/json, text/plain, */*" \
62 + -H "referer: ${COVERITY_HOST}/" \
63 + -H "user-agent: ${COVERITY_USER_AGENT}" \
64 + -H "x-xsrf-token: ${COVERITY_XSRF}" \
65 + -b "${COVERITY_COOKIE}" \
66 + "${COVERITY_HOST}/sourcebrowser/defectdetails.json?projectId=${COVERITY_PROJECT_ID}&defectInstanceId=${defect_instance_id}" || echo "000")"
67 +
68 + if [[ "${http_code}" != "200" ]]; then
69 + failed=$((failed + 1))
70 + echo -e "${COV_RED}[${i}/${TOTAL}] cid=${cid} diid=${defect_instance_id} HTTP=${http_code}${COV_NC}" >&2
71 + if [[ "${http_code}" == "401" || "${http_code}" == "403" ]]; then
72 + echo -e "${COV_RED}Session likely expired. Recapture cookie from browser, update .env, retry.${COV_NC}" >&2
73 + rm -f "${out_file}"
74 + exit 2
75 + fi
76 + rm -f "${out_file}"
77 + continue
78 + fi
79 +
80 + fetched=$((fetched + 1))
81 + if (( i % 20 == 0 )); then
82 + echo -e "${COV_GRAY}[${i}/${TOTAL}] fetched=${fetched} skipped=${skipped} failed=${failed}${COV_NC}" >&2
83 + fi
84 +done < <(jq -r '.[] | "\(.cid)\t\(.lastDefectInstanceId)"' "${INPUT}")
85 +
86 +echo -e "${COV_GREEN}Done. fetched=${fetched} skipped=${skipped} failed=${failed} total=${TOTAL}${COV_NC}" >&2
.agents/skills/coverity-audit/scripts/fetch-table.sh new
+107
@@ -0,0 +1,107 @@
1 +#!/usr/bin/env bash
2 +# Fetch all pages of a Coverity Scan view table.
3 +#
4 +# Usage:
5 +# fetch-table.sh <viewId> <pages> <output-prefix>
6 +# Example:
7 +# fetch-table.sh 41549 7 .local/audits/coverity/raw/outstanding
8 +#
9 +# Coverity's UI uses two steps per page:
10 +# 1) POST /views/table.json {projectId, viewId, pageNum} -- updates server view state
11 +# 2) GET /reports/table.json?projectId=X&viewId=Y -- fetches rows for current state
12 +#
13 +# The script combines all pages into <prefix>-all.json (a flat array).
14 +# Pages already on disk are skipped (idempotent).
15 +
16 +set -euo pipefail
17 +
18 +# shellcheck source=./_lib.sh
19 +# shellcheck disable=SC1091
20 +source "$(dirname "$0")/_lib.sh"
21 +cov_load_env
22 +
23 +VIEW_ID="${1:?usage: $0 <viewId> <pages> <output-prefix>}"
24 +PAGES="${2:?usage: $0 <viewId> <pages> <output-prefix>}"
25 +PREFIX="${3:?usage: $0 <viewId> <pages> <output-prefix>}"
26 +
27 +# View IDs and page counts are positive integers. Reject anything else
28 +# before the loop -- otherwise non-numeric PAGES would make `seq` produce
29 +# no output, and the final jq merge would block waiting on stdin.
30 +if [[ ! "${VIEW_ID}" =~ ^[1-9][0-9]*$ ]]; then
31 + echo -e "${COV_RED}[ERROR]${COV_NC} viewId must be a positive integer, got: '${VIEW_ID}'" >&2
32 + exit 1
33 +fi
34 +if [[ ! "${PAGES}" =~ ^[1-9][0-9]*$ ]]; then
35 + echo -e "${COV_RED}[ERROR]${COV_NC} pages must be a positive integer, got: '${PAGES}'" >&2
36 + exit 1
37 +fi
38 +
39 +mkdir -p "$(dirname "${PREFIX}")"
40 +
41 +for page in $(seq 1 "${PAGES}"); do
42 + out="${PREFIX}-page${page}.json"
43 + if [[ -s "${out}" ]]; then
44 + echo -e "${COV_GRAY}[page ${page}/${PAGES}] cached ${out}${COV_NC}" >&2
45 + continue
46 + fi
47 +
48 + # Step 1: set server-side page state.
49 + post_body="{\"projectId\":${COVERITY_PROJECT_ID},\"viewId\":${VIEW_ID},\"pageNum\":${page}}"
50 + post_http=$(curl -sS -o /dev/null -w '%{http_code}' -X POST \
51 + -H "accept: application/json, text/plain, */*" \
52 + -H "content-type: application/json" \
53 + -H "origin: ${COVERITY_HOST}" \
54 + -H "referer: ${COVERITY_HOST}/" \
55 + -H "user-agent: ${COVERITY_USER_AGENT}" \
56 + -H "x-xsrf-token: ${COVERITY_XSRF}" \
57 + -b "${COVERITY_COOKIE}" \
58 + --data-raw "${post_body}" \
59 + "${COVERITY_HOST}/views/table.json")
60 + if [[ "${post_http}" != "200" ]]; then
61 + echo -e "${COV_RED}[page ${page}] POST failed HTTP=${post_http}${COV_NC}" >&2
62 + exit 2
63 + fi
64 +
65 + # Step 2: fetch the now-current page.
66 + # Capture exit status of curl explicitly so a transport failure (timeout,
67 + # connection reset, DNS) doesn't leave a non-empty partial file behind
68 + # that the next run would treat as cached.
69 + get_http=$(curl -sS -o "${out}" -w '%{http_code}' \
70 + -H "accept: application/json, text/plain, */*" \
71 + -H "referer: ${COVERITY_HOST}/" \
72 + -H "user-agent: ${COVERITY_USER_AGENT}" \
73 + -b "${COVERITY_COOKIE}" \
74 + "${COVERITY_HOST}/reports/table.json?projectId=${COVERITY_PROJECT_ID}&viewId=${VIEW_ID}" \
75 + || echo "000")
76 + if [[ "${get_http}" != "200" ]]; then
77 + echo -e "${COV_RED}[page ${page}] GET failed HTTP=${get_http}${COV_NC}" >&2
78 + rm -f "${out}"
79 + exit 3
80 + fi
81 + # Validate response is JSON with the expected shape -- a Cloudflare
82 + # challenge or a 200-with-HTML-body would otherwise be cached as
83 + # 'valid' and confuse subsequent runs.
84 + if ! jq -e '.resultSet.results | type == "array"' "${out}" >/dev/null 2>&1; then
85 + echo -e "${COV_RED}[page ${page}] response not in expected shape; first 200 chars:${COV_NC}" >&2
86 + head -c 200 "${out}" >&2; echo >&2
87 + rm -f "${out}"
88 + exit 3
89 + fi
90 +
91 + count=$(jq '.resultSet.results | length' "${out}")
92 + total=$(jq '.resultSet.totalCount' "${out}")
93 + echo -e "${COV_GRAY}[page ${page}/${PAGES}] fetched ${count} rows (total=${total})${COV_NC}" >&2
94 + sleep 0.3
95 +done
96 +
97 +combined="${PREFIX}-all.json"
98 +# Build the explicit page-file list. A glob (`-page*.json`) would pick up
99 +# stale page files from a previous larger fetch (e.g. running with PAGES=5
100 +# after a prior run with PAGES=7 would merge 7 pages into "all"). Enumerate
101 +# only the pages we asked for in this invocation.
102 +page_files=()
103 +for page in $(seq 1 "${PAGES}"); do
104 + page_files+=("${PREFIX}-page${page}.json")
105 +done
106 +jq -s '[.[].resultSet.results[]]' "${page_files[@]}" > "${combined}"
107 +echo -e "${COV_GREEN}Combined $(jq 'length' "${combined}") defects into ${combined}${COV_NC}" >&2
.agents/skills/coverity-audit/scripts/finalize-defect.sh new
+107
@@ -0,0 +1,107 @@
1 +#!/usr/bin/env bash
2 +# Apply a verdict to one Coverity defect (high-level wrapper around update-triage.sh).
3 +#
4 +# Usage:
5 +# finalize-defect.sh <cid> <verdict> <scope> <comment-file> [commit-sha]
6 +#
7 +# Verdicts:
8 +# TRUE_BUG_MEMORY_CORRUPTION, TRUE_BUG_CRASH, TRUE_BUG_RESOURCE_LEAK,
9 +# TRUE_BUG_LOGIC, TRUE_BUG_UB -> Bug + Fix Submitted
10 +# FALSE_POSITIVE_GUARD_EXISTS, FALSE_POSITIVE_UNREACHABLE,
11 +# FALSE_POSITIVE_TRUSTED_INPUT,
12 +# FALSE_POSITIVE_TOOL_MODEL,
13 +# IMPOSSIBLE_CONDITIONS -> False Positive + Ignore
14 +# COSMETIC -> Intentional + Ignore
15 +# NEEDS_HUMAN, CODE_GONE -> NO-OP (skipped, exit 0)
16 +#
17 +# Scope:
18 +# "outstanding" default; applied unconditionally
19 +# anything else ("dismissed", "fixed", "unclassified", ...) -- caller
20 +# asserts the new verdict disagrees with the existing
21 +# Coverity classification; the script will warn but proceed.
22 +#
23 +# Severity is mapped from Coverity's displayImpact field. The script reads it
24 +# from .local/audits/coverity/raw/outstanding-all.json or
25 +# .local/audits/coverity/raw/all-in-project-all.json (fallback).
26 +# If neither file exists, severity defaults to Unspecified.
27 +#
28 +# If a commit SHA is given, the script appends "Fix commit: <sha>" to the
29 +# comment before posting.
30 +
31 +set -euo pipefail
32 +
33 +# shellcheck source=./_lib.sh
34 +# shellcheck disable=SC1091
35 +source "$(dirname "$0")/_lib.sh"
36 +
37 +CID="${1:?usage: $0 <cid> <verdict> <scope> <comment-file> [commit-sha]}"
38 +VERDICT="${2:?usage}"
39 +SCOPE="${3:?usage}"
40 +COMMENT_FILE="${4:?usage}"
41 +COMMIT_SHA="${5:-}"
42 +
43 +cov_require_numeric_cid "${CID}"
44 +
45 +# Verdicts that never touch the UI.
46 +# Anything other than NEEDS_HUMAN / CODE_GONE falls through to the next
47 +# case statement which decides classification/action; the *) here just
48 +# documents that explicitly.
49 +case "${VERDICT}" in
50 + NEEDS_HUMAN|CODE_GONE)
51 + echo -e "${COV_YELLOW}Skipping Coverity update for CID ${CID} -- verdict=${VERDICT}.${COV_NC}" >&2
52 + exit 0
53 + ;;
54 + *)
55 + ;;
56 +esac
57 +
58 +# Verdict -> (classification, action).
59 +case "${VERDICT}" in
60 + TRUE_BUG_MEMORY_CORRUPTION|TRUE_BUG_CRASH|TRUE_BUG_RESOURCE_LEAK|TRUE_BUG_LOGIC|TRUE_BUG_UB)
61 + CLASS_ID=24; ACT_ID=3 ;;
62 + FALSE_POSITIVE_GUARD_EXISTS|FALSE_POSITIVE_UNREACHABLE|FALSE_POSITIVE_TRUSTED_INPUT|FALSE_POSITIVE_TOOL_MODEL|IMPOSSIBLE_CONDITIONS)
63 + CLASS_ID=22; ACT_ID=5 ;;
64 + COSMETIC)
65 + CLASS_ID=23; ACT_ID=5 ;;
66 + *)
67 + echo -e "${COV_RED}Unknown verdict: ${VERDICT}${COV_NC}" >&2; exit 1 ;;
68 +esac
69 +
70 +# Severity from displayImpact. CID is validated numeric above, so embedding
71 +# it in the jq filter is safe (cov_require_numeric_cid rejects anything else).
72 +audit="$(cov_audit_dir)"
73 +impact=""
74 +for f in "${audit}/raw/outstanding-all.json" "${audit}/raw/all-in-project-all.json"; do
75 + if [[ -f "${f}" && -r "${f}" ]]; then
76 + impact="$(jq -r --argjson cid "${CID}" '.[] | select(.cid==$cid) | .displayImpact' "${f}" 2>/dev/null || true)"
77 + [[ -n "${impact}" && "${impact}" != "null" ]] && break
78 + fi
79 +done
80 +
81 +case "${impact}" in
82 + High) SEV_ID=11 ;;
83 + Medium) SEV_ID=12 ;;
84 + Low) SEV_ID=13 ;;
85 + *) SEV_ID=10 ;;
86 +esac
87 +
88 +echo -e "${COV_GRAY}CID ${CID}: verdict=${VERDICT} -> class=${CLASS_ID} sev=${SEV_ID} (impact=${impact:-unknown}) act=${ACT_ID}${COV_NC}" >&2
89 +
90 +# If a commit SHA is given, append it to the comment before posting.
91 +if [[ -n "${COMMIT_SHA}" ]]; then
92 + tmp_comment="$(mktemp "${TMPDIR:-/tmp}/cov-comment-XXXXXX.txt")"
93 + trap 'rm -f "${tmp_comment}"' EXIT
94 + {
95 + cat "${COMMENT_FILE}"
96 + printf '\nFix commit: %s\n' "${COMMIT_SHA}"
97 + } > "${tmp_comment}"
98 + effective="${tmp_comment}"
99 +else
100 + effective="${COMMENT_FILE}"
101 +fi
102 +
103 +if [[ "${SCOPE}" != "outstanding" ]]; then
104 + echo -e "${COV_YELLOW}Scope=${SCOPE}: applying ONLY because caller asserts verdict disagrees with the existing classification.${COV_NC}" >&2
105 +fi
106 +
107 +"$(dirname "$0")/update-triage.sh" "${CID}" "${CLASS_ID}" "${SEV_ID}" "${ACT_ID}" "${effective}"
.agents/skills/coverity-audit/scripts/keepalive.sh new
+83
@@ -0,0 +1,83 @@
1 +#!/usr/bin/env bash
2 +# Keep the Coverity Scan session warm during a triage run.
3 +#
4 +# DESIGN — the script EXITS NON-ZERO the moment a ping fails. That fires the
5 +# orchestrator's background-task completion notification, so the agent learns
6 +# *immediately* that the cookie went bad and can ask the user to recapture it.
7 +# DO NOT silently retry — silent retries hide the failure and waste a triage
8 +# session's worth of work.
9 +#
10 +# The Coverity session cookie expires after a few minutes of inactivity AND
11 +# requires the user's browser tab on https://scan.coverity.com to be open.
12 +# Closing the browser tab kills the session immediately; pings stop working.
13 +#
14 +# Usage (from an orchestrator agent):
15 +# Bash tool with run_in_background=true,
16 +# command="bash .agents/skills/coverity-audit/scripts/keepalive.sh"
17 +#
18 +# Stop the background task at the end of the triage session.
19 +#
20 +# Environment overrides:
21 +# PING_INTERVAL seconds between pings (default 300 = 5 min)
22 +
23 +set -euo pipefail
24 +
25 +# shellcheck source=./_lib.sh
26 +# shellcheck disable=SC1091
27 +source "$(dirname "$0")/_lib.sh"
28 +cov_load_env
29 +
30 +PING_INTERVAL="${PING_INTERVAL:-300}"
31 +
32 +# Pick a cheap, authenticated endpoint. /reports/table.json with the configured
33 +# Outstanding view is ideal — it returns proper JSON when authenticated and
34 +# HTML (Cloudflare challenge or login redirect) when not.
35 +if [[ -z "${COVERITY_VIEW_OUTSTANDING:-}" ]]; then
36 + echo -e "${COV_RED}[ERROR]${COV_NC} COVERITY_VIEW_OUTSTANDING is not set in .env -- keepalive needs a viewId to ping. See SKILL.md." >&2
37 + exit 1
38 +fi
39 +if [[ ! "${COVERITY_VIEW_OUTSTANDING}" =~ ^[1-9][0-9]*$ ]]; then
40 + echo -e "${COV_RED}[ERROR]${COV_NC} COVERITY_VIEW_OUTSTANDING must be a positive integer (got: '${COVERITY_VIEW_OUTSTANDING}')" >&2
41 + exit 1
42 +fi
43 +
44 +PING_URL="${COVERITY_HOST}/reports/table.json?projectId=${COVERITY_PROJECT_ID}&viewId=${COVERITY_VIEW_OUTSTANDING}"
45 +
46 +ping_once() {
47 + local body rc
48 + # Capture both body and HTTP code in one go.
49 + body="$(curl -sS --max-time 30 \
50 + -H "accept: application/json, text/plain, */*" \
51 + -H "user-agent: ${COVERITY_USER_AGENT}" \
52 + -H "referer: ${COVERITY_HOST}/" \
53 + -b "${COVERITY_COOKIE}" \
54 + "${PING_URL}" 2>/dev/null)" || {
55 + rc=$?
56 + echo "[$(date -Iseconds)] FAIL curl rc=${rc}" >&2
57 + return 1
58 + }
59 +
60 + if [[ -z "${body}" ]]; then
61 + echo "[$(date -Iseconds)] FAIL empty response -- session likely expired" >&2
62 + return 1
63 + fi
64 +
65 + # Validate the JSON shape -- a Cloudflare challenge or login redirect
66 + # returns HTML; we want the structured response.
67 + if ! printf '%s' "${body}" | jq -e '.resultSet.results' >/dev/null 2>&1; then
68 + echo "[$(date -Iseconds)] FAIL response shape invalid (first 80 chars: '${body:0:80}') -- session likely expired or browser tab closed" >&2
69 + return 1
70 + fi
71 +
72 + echo "[$(date -Iseconds)] OK" >&2
73 + return 0
74 +}
75 +
76 +# First ping immediately -- if we're already dead, fail fast.
77 +ping_once || exit 1
78 +echo "[$(date -Iseconds)] keepalive running (interval=${PING_INTERVAL}s)" >&2
79 +
80 +while true; do
81 + sleep "${PING_INTERVAL}"
82 + ping_once || exit 1
83 +done
.agents/skills/coverity-audit/scripts/prepare-defect.sh new
+152
@@ -0,0 +1,152 @@
1 +#!/usr/bin/env bash
2 +# Bundle one Coverity defect into a per-CID working directory.
3 +#
4 +# Usage:
5 +# prepare-defect.sh [--force] <cid> [<scope>]
6 +#
7 +# <scope> defaults to "outstanding" -- it's only used as a subdirectory name
8 +# for organizing artifacts, not a Coverity API parameter.
9 +#
10 +# Inputs (must already exist; produced by fetch-table.sh + fetch-details.sh):
11 +# .local/audits/coverity/raw/<scope>-all.json
12 +# .local/audits/coverity/details/<scope>/cid-<N>.json (or details/cid-<N>.json)
13 +#
14 +# Outputs (under <audit-dir>/triage/<scope>/cid-<N>/):
15 +# defect-summary.json -- the row from the table dump
16 +# defect-details.json -- the per-defect details
17 +# source-context.c -- ~150 lines around the main event in the flagged file
18 +# TODO.md -- per-defect scratch (so review work doesn't pile at repo root)
19 +#
20 +# Idempotent. Re-running keeps existing files; pass --force to regenerate.
21 +#
22 +# This script does NOT prescribe a review pipeline. After preparing the bundle,
23 +# how you triage it (single model, multiple models, manual review, etc.) is
24 +# adhoc and should be agreed with the user.
25 +
26 +set -euo pipefail
27 +
28 +# shellcheck source=./_lib.sh
29 +# shellcheck disable=SC1091
30 +source "$(dirname "$0")/_lib.sh"
31 +
32 +FORCE=0
33 +if [[ "${1:-}" == "--force" ]]; then
34 + FORCE=1; shift
35 +fi
36 +
37 +CID="${1:?usage: $0 [--force] <cid> [<scope>]}"
38 +SCOPE="${2:-outstanding}"
39 +
40 +cov_require_numeric_cid "${CID}"
41 +
42 +# Scope is used as a path component (.../triage/<scope>/cid-N/...). Reject
43 +# anything that could path-escape; allow only simple lowercase identifiers.
44 +if [[ ! "${SCOPE}" =~ ^[a-z][a-z0-9_-]*$ ]]; then
45 + echo -e "${COV_RED}[ERROR]${COV_NC} scope must match ^[a-z][a-z0-9_-]*\$ (got: '${SCOPE}')" >&2
46 + exit 1
47 +fi
48 +
49 +ROOT="$(cov_repo_root)"
50 +AUDIT="$(cov_audit_dir)"
51 +
52 +OUT_DIR="${AUDIT}/triage/${SCOPE}/cid-${CID}"
53 +ROW_FILE="${AUDIT}/raw/${SCOPE}-all.json"
54 +DETAILS_SRC="${AUDIT}/details/${SCOPE}/cid-${CID}.json"
55 +
56 +if [[ ( ! -f "${DETAILS_SRC}" || ! -r "${DETAILS_SRC}" ) \
57 + && -f "${AUDIT}/details/cid-${CID}.json" \
58 + && -r "${AUDIT}/details/cid-${CID}.json" ]]; then
59 + DETAILS_SRC="${AUDIT}/details/cid-${CID}.json"
60 +fi
61 +
62 +if [[ ! -f "${ROW_FILE}" || ! -r "${ROW_FILE}" ]]; then
63 + echo -e "${COV_RED}Missing ${ROW_FILE}. Run fetch-table.sh first.${COV_NC}" >&2
64 + exit 1
65 +fi
66 +if [[ ! -f "${DETAILS_SRC}" || ! -r "${DETAILS_SRC}" ]]; then
67 + echo -e "${COV_RED}Missing ${DETAILS_SRC}. Run fetch-details.sh first.${COV_NC}" >&2
68 + exit 1
69 +fi
70 +
71 +mkdir -p "${OUT_DIR}"
72 +
73 +# defect-summary.json -- CID is validated numeric above, --argjson is safe.
74 +summary="${OUT_DIR}/defect-summary.json"
75 +if [[ ! -s "${summary}" || "${FORCE}" == "1" ]]; then
76 + jq --argjson cid "${CID}" '.[] | select(.cid==$cid)' "${ROW_FILE}" > "${summary}"
77 + if [[ ! -s "${summary}" ]]; then
78 + echo -e "${COV_RED}CID ${CID} not found in ${ROW_FILE}. Wrong scope, or table is stale?${COV_NC}" >&2
79 + rm -f "${summary}"
80 + exit 2
81 + fi
82 +fi
83 +
84 +# defect-details.json
85 +details="${OUT_DIR}/defect-details.json"
86 +if [[ ! -s "${details}" || "${FORCE}" == "1" ]]; then
87 + cp "${DETAILS_SRC}" "${details}"
88 +fi
89 +
90 +# source-context.c
91 +display_file="$(jq -r '.displayFile // ""' "${summary}")"
92 +repo_file="${display_file#/}" # strip leading slash; paths are repo-relative
93 +main_line="$(jq -r '
94 + (.occurrences[0].eventSets[0].eventTree | map(select(.main==true))[0]
95 + // .occurrences[0].eventSets[0].eventTree[-1])
96 + | .lineNumber // 1
97 +' "${details}")"
98 +
99 +ctx_file="${OUT_DIR}/source-context.c"
100 +if [[ ! -s "${ctx_file}" || "${FORCE}" == "1" ]]; then
101 + # Validation:
102 + # 1. Non-empty: an empty displayFile would resolve `${ROOT}/${repo_file}`
103 + # to `${ROOT}/`, which IS a readable directory.
104 + # 2. No `..`: prevent path traversal escaping the repo. Coverity's
105 + # displayFile is its source-tree path; legitimate values never
106 + # contain `..`. Reject anything that does.
107 + # 3. Regular file + readable.
108 + repo_file_ok=1
109 + [[ -z "${repo_file}" ]] && repo_file_ok=0
110 + [[ "${repo_file}" == *..* ]] && repo_file_ok=0
111 + [[ -f "${ROOT}/${repo_file}" && -r "${ROOT}/${repo_file}" ]] || repo_file_ok=0
112 + if (( repo_file_ok )); then
113 + start=$((main_line - 100))
114 + (( start < 1 )) && start=1
115 + end=$((main_line + 50))
116 + {
117 + printf '// Extracted from %s (lines %d..%d; main event at line %d).\n' \
118 + "${repo_file}" "${start}" "${end}" "${main_line}"
119 + printf '// Line numbers are the original file line numbers.\n\n'
120 + awk -v s="${start}" -v e="${end}" 'NR>=s && NR<=e {printf "%5d %s\n", NR, $0}' \
121 + "${ROOT}/${repo_file}"
122 + } > "${ctx_file}"
123 + else
124 + echo -e "${COV_YELLOW}Source ${repo_file} not in tree -- CODE_GONE candidate.${COV_NC}" >&2
125 + printf '// Source file %s not present in the current tree.\n// Candidate CODE_GONE.\n' \
126 + "${repo_file}" > "${ctx_file}"
127 + fi
128 +fi
129 +
130 +# TODO.md (so review work doesn't create a TODO at repo root)
131 +per_todo="${OUT_DIR}/TODO.md"
132 +if [[ ! -s "${per_todo}" || "${FORCE}" == "1" ]]; then
133 + short_type="$(jq -r '.displayType // "unknown"' "${summary}")"
134 + impact="$(jq -r '.displayImpact // "unspecified"' "${summary}")"
135 + cat > "${per_todo}" <<EOF
136 +# CID ${CID} -- ${short_type} (${impact} impact)
137 +
138 +File: ${repo_file}
139 +Line: ${main_line}
140 +
141 +Bundle:
142 +- defect-summary.json
143 +- defect-details.json
144 +- source-context.c
145 +
146 +(Use this file for per-defect notes, plan, decisions. Keep all per-defect
147 +artifacts inside this directory.)
148 +EOF
149 +fi
150 +
151 +echo -e "${COV_GREEN}Prepared ${OUT_DIR}${COV_NC}" >&2
152 +ls -1 "${OUT_DIR}"
.agents/skills/coverity-audit/scripts/resolve-cid-to-diid.sh new
+51
@@ -0,0 +1,51 @@
1 +#!/usr/bin/env bash
2 +# Resolve a CID to its current defectInstanceId via /reports/defects.json.
3 +#
4 +# Why: many Coverity endpoints (defectdetails.json, the source-browser deep
5 +# links) take a defectInstanceId, NOT a CID. The CID is stable across runs;
6 +# the defectInstanceId is per-scan. When you only have a CID (e.g. you're
7 +# acting on a stale list, or processing per-cid output from a diff tool),
8 +# you need to look up the current defectInstanceId.
9 +#
10 +# Usage:
11 +# resolve-cid-to-diid.sh <cid>
12 +#
13 +# Prints the defectInstanceId on stdout if resolved, or "GONE" if Coverity has
14 +# no current defect instance for this CID (the underlying code was removed or
15 +# the scan no longer reports it).
16 +#
17 +# This call is INDEPENDENT of view state -- it queries the defect by CID
18 +# directly, so it does not interact with the server-side view pagination.
19 +
20 +set -euo pipefail
21 +
22 +# shellcheck source=./_lib.sh
23 +# shellcheck disable=SC1091
24 +source "$(dirname "$0")/_lib.sh"
25 +cov_load_env
26 +
27 +CID="${1:?usage: $0 <cid>}"
28 +cov_require_numeric_cid "${CID}"
29 +
30 +url="$(curl -sS --max-time 30 \
31 + -H "accept: application/json, text/plain, */*" \
32 + -H "referer: ${COVERITY_HOST}/" \
33 + -H "user-agent: ${COVERITY_USER_AGENT}" \
34 + -b "${COVERITY_COOKIE}" \
35 + "${COVERITY_HOST}/reports/defects.json?projectId=${COVERITY_PROJECT_ID}&cid=${CID}" \
36 + | jq -r '.url // ""')"
37 +
38 +if [[ -z "${url}" ]]; then
39 + echo "GONE"
40 + exit 0
41 +fi
42 +
43 +# The .url field looks like:
44 +# /reports.htm#v70389/p15826/defectInstanceId=14451237&fileInstanceId=...&mergedDefectId=...
45 +diid="$(printf '%s' "${url}" | sed -n 's/.*defectInstanceId=\([0-9]*\).*/\1/p')"
46 +if [[ -z "${diid}" ]]; then
47 + echo "GONE"
48 + exit 0
49 +fi
50 +
51 +echo "${diid}"
.agents/skills/coverity-audit/scripts/update-triage.sh new
+100
@@ -0,0 +1,100 @@
1 +#!/usr/bin/env bash
2 +# Update Coverity Scan triage for one CID (low-level — usually called by finalize-defect.sh).
3 +#
4 +# Usage:
5 +# update-triage.sh <cid> <classification> <severity> <action> <comment-file>
6 +#
7 +# Coverity attribute IDs (not names):
8 +# classification (attr 3): 20=Unclassified 21=Pending 22=FalsePositive 23=Intentional 24=Bug
9 +# severity (attr 1): 10=Unspecified 11=Major 12=Moderate 13=Minor
10 +# action (attr 2): 1=Undecided 2=FixRequired 3=FixSubmitted 4=ModelingRequired 5=Ignore
11 +# external ref (attr 4): always sent null here
12 +#
13 +# Comment is read from <comment-file>. MUST be ASCII (Cloudflare blocks em-dashes
14 +# and smart quotes with a 403 Cloudflare challenge — see SKILL.md).
15 +
16 +set -euo pipefail
17 +
18 +# shellcheck source=./_lib.sh
19 +# shellcheck disable=SC1091
20 +source "$(dirname "$0")/_lib.sh"
21 +cov_load_env
22 +
23 +CID="${1:?usage: $0 <cid> <classification> <severity> <action> <comment-file>}"
24 +CLASS_ID="${2:?usage}"
25 +SEV_ID="${3:?usage}"
26 +ACT_ID="${4:?usage}"
27 +COMMENT_FILE="${5:?usage}"
28 +
29 +cov_require_numeric_cid "${CID}"
30 +for v in CLASS_ID SEV_ID ACT_ID; do
31 + if [[ ! "${!v}" =~ ^[1-9][0-9]*$ ]]; then
32 + echo -e "${COV_RED}[ERROR]${COV_NC} ${v} must be a positive integer, got: '${!v}'" >&2
33 + exit 1
34 + fi
35 +done
36 +
37 +if [[ ! -f "${COMMENT_FILE}" || ! -r "${COMMENT_FILE}" ]]; then
38 + echo -e "${COV_RED}Comment file not readable: ${COMMENT_FILE}${COV_NC}" >&2
39 + exit 1
40 +fi
41 +
42 +# ASCII-only check on the comment body. `tr -d '\000-\177'` is portable across
43 +# GNU and BSD/macOS (`grep -P` is GNU-only).
44 +if LC_ALL=C tr -d '\000-\177' < "${COMMENT_FILE}" | grep -q .; then
45 + echo -e "${COV_RED}[ERROR]${COV_NC} ${COMMENT_FILE} contains non-ASCII bytes. Cloudflare will block. Replace em-dashes (--) and smart quotes." >&2
46 + exit 1
47 +fi
48 +
49 +# cid + project are numeric (validated above); attribute values must be
50 +# strings per the API.
51 +payload="$(jq -n \
52 + --argjson cid "${CID}" \
53 + --arg class "${CLASS_ID}" \
54 + --arg sev "${SEV_ID}" \
55 + --arg act "${ACT_ID}" \
56 + --argjson project "${COVERITY_PROJECT_ID}" \
57 + --rawfile comment "${COMMENT_FILE}" \
58 + '{
59 + triageValues: [
60 + {attributeId: 3, attributeValue: $class},
61 + {attributeId: 1, attributeValue: $sev},
62 + {attributeId: 2, attributeValue: $act},
63 + {attributeId: 4, attributeValue: null}
64 + ],
65 + comment: $comment,
66 + mergedDefectIds: [$cid],
67 + ownerId: -1,
68 + projectId: $project,
69 + triageStoreIds: [],
70 + type: "apply"
71 + }')"
72 +
73 +echo -e "${COV_GRAY}Posting triage update for CID ${CID} (class=${CLASS_ID} sev=${SEV_ID} act=${ACT_ID})...${COV_NC}" >&2
74 +
75 +response_file="$(mktemp "${TMPDIR:-/tmp}/cov-triage-XXXXXX.json")"
76 +trap 'rm -f "${response_file}"' EXIT
77 +
78 +http=$(curl -sS -X POST -o "${response_file}" -w '%{http_code}' \
79 + -H "accept: application/json, text/plain, */*" \
80 + -H "content-type: application/json" \
81 + -H "origin: ${COVERITY_HOST}" \
82 + -H "referer: ${COVERITY_HOST}/" \
83 + -H "user-agent: ${COVERITY_USER_AGENT}" \
84 + -H "x-xsrf-token: ${COVERITY_XSRF}" \
85 + -b "${COVERITY_COOKIE}" \
86 + --data-raw "${payload}" \
87 + "${COVERITY_HOST}/sourcebrowser/updatedefecttriage.json")
88 +
89 +if [[ "${http}" != "200" ]]; then
90 + echo -e "${COV_RED}HTTP ${http} -- update failed${COV_NC}" >&2
91 + head -c 500 "${response_file}" >&2; echo >&2
92 + if [[ "${http}" == "401" || "${http}" == "403" || "${http}" == "302" ]]; then
93 + echo -e "${COV_RED}Session likely expired. Recapture cookie from browser and update .env.${COV_NC}" >&2
94 + fi
95 + exit 2
96 +fi
97 +
98 +echo -e "${COV_GREEN}OK -- triage updated for CID ${CID}.${COV_NC}" >&2
99 +jq -c '{defectStatus, lastTriaged, updatedValuesByCid}' "${response_file}" 2>/dev/null || head -c 300 "${response_file}"
100 +echo
.agents/skills/graphql-audit/SKILL.md new
+162
@@ -0,0 +1,162 @@
1 +---
2 +name: graphql-audit
3 +description: Triage GitHub Code Scanning alerts (CodeQL with security-extended suite) for this repository — list open alerts, dismiss as false positive / won't fix / used in tests, query via GitHub REST + GraphQL. Use when the user asks to "review GitHub security alerts", "check CodeQL findings", "triage code scanning", or anything mentioning Code Scanning, CodeQL, security-extended, or github.com/$repo/security/code-scanning.
4 +---
5 +
6 +# GitHub Code Scanning triage skill
7 +
8 +This skill drives the GitHub Code Scanning API (REST + GraphQL via `gh`) for
9 +the repository's CodeQL alerts. The repo's CI runs the **security-extended**
10 +CodeQL suite, which surfaces a wider set of findings than the default suite.
11 +
12 +The skill operates on whatever repo this checkout points at — it derives the
13 +`owner/repo` from the `upstream` (or `origin`) git remote. Auth uses the
14 +`gh` CLI's stored credentials; no token is needed in `.env`.
15 +
16 +## MANDATORY — keep this skill alive
17 +
18 +**If you (the agent) discover a new pattern, gotcha, working flow, correction,
19 +or any piece of knowledge while running this skill — update this `SKILL.md`
20 +AND commit it BEFORE proceeding. Knowledge that isn't committed is lost.**
21 +
22 +Examples of things to capture:
23 +- A CodeQL rule with a known FP pattern + the canonical comment to use
24 +- A query (GraphQL or REST) that returns useful aggregate views not available in the UI
25 +- An alert format change or new field that started appearing
26 +- A way to bulk-dismiss without hitting REST rate limits
27 +
28 +## Setup
29 +
30 +### Prerequisite — `gh` CLI authenticated
31 +
32 +```
33 +gh auth status
34 +```
35 +
36 +Must show authentication for github.com. If not, run `gh auth login`.
37 +
38 +The `gh` user needs **write** scope on the repository to dismiss alerts;
39 +read scope is enough for listing.
40 +
41 +### .env entries
42 +
43 +None required for this skill. `gh` handles auth.
44 +
45 +(Optional `GITHUB_TOKEN` could go in `.env` if you ever need to bypass `gh`
46 +and call REST/GraphQL directly via curl, e.g. from a script that runs
47 +without an authenticated `gh` session.)
48 +
49 +## Triage decision matrix
50 +
51 +GitHub Code Scanning alerts have three lifecycle states: `open`, `dismissed`,
52 +`fixed` (auto-detected when the underlying code changes). Manual triage uses
53 +`dismissed` with one of three reasons:
54 +
55 +| Decision | API value | When to use |
56 +|------------------|-------------------|---------------------------------------------------------------------|
57 +| False Positive | `false positive` | CodeQL is wrong (path is unreachable, type is wider, guard exists) |
58 +| Won't Fix | `won't fix` | Real but acceptable risk; not addressing in this codebase |
59 +| Used in Tests | `used in tests` | Alert is in test fixtures / vendored test code, not production |
60 +
61 +A dismissed alert can be **reopened** later (manually in the UI or via
62 +`PATCH /alerts/<n>` with `state=open`); the dismissal history is preserved.
63 +
64 +## Workflow
65 +
66 +### Step 1 — see what's open
67 +
68 +```
69 +bash .agents/skills/graphql-audit/scripts/codeql-list.sh
70 +```
71 +
72 +Default output is a count-by-rule summary for **open** alerts:
73 +
74 +```
75 + 42 cpp/integer-overflow|warning
76 + 17 cpp/uninitialized-local|error
77 + ...
78 +```
79 +
80 +Filters:
81 +```
82 +bash .agents/skills/graphql-audit/scripts/codeql-list.sh --state=open --severity=high
83 +bash .agents/skills/graphql-audit/scripts/codeql-list.sh --tool=CodeQL --severity=critical
84 +bash .agents/skills/graphql-audit/scripts/codeql-list.sh --raw # full JSON
85 +```
86 +
87 +### Step 2 — inspect a single alert
88 +
89 +```
90 +gh api /repos/<owner>/<repo>/code-scanning/alerts/<n>
91 +```
92 +
93 +Or visit `https://github.com/<owner>/<repo>/security/code-scanning/<n>` in
94 +the browser for the full data-flow view.
95 +
96 +### Step 3 — dismiss
97 +
98 +```
99 +bash .agents/skills/graphql-audit/scripts/codeql-dismiss.sh \
100 + <alert_number> "false positive" "<short ASCII comment>"
101 +```
102 +
103 +Comments are stored verbatim; keep them short, factual, and ASCII.
104 +
105 +### Step 4 — bulk dismissal
106 +
107 +For a known-FP rule pattern (e.g., `cpp/uninitialized-local` always firing
108 +on a particular header), GitHub does NOT have a REST bulk endpoint. The
109 +working pattern is:
110 +
111 +```bash
112 +bash .agents/skills/graphql-audit/scripts/codeql-list.sh --raw \
113 + | jq -r '.[] | select(.rule.id=="cpp/uninitialized-local") | .number' \
114 + | while read n; do
115 + bash .agents/skills/graphql-audit/scripts/codeql-dismiss.sh \
116 + "$n" "false positive" "FP: rule firing on a stub model not real code"
117 + done
118 +```
119 +
120 +Throttle if you have many — the REST API tolerates ~10 req/s for a single
121 +user.
122 +
123 +## What this skill does NOT do
124 +
125 +- **CodeQL query authoring**: this skill triages results, not the queries
126 + that produce them. To suppress a class of FPs at the source, edit
127 + `.github/codeql/` config or the queries themselves.
128 +- **Workflow management**: enabling/disabling CodeQL runs is in
129 + `.github/workflows/codeql.yml` — not here.
130 +- **Other Advanced Security features** (secret scanning alerts, dependabot)
131 + use sibling APIs (`/secret-scanning/alerts`, `/dependabot/alerts`). They
132 + could be added to this skill if needed.
133 +
134 +## REST vs GraphQL
135 +
136 +CodeQL alerts are exposed via REST (`/repos/{owner}/{repo}/code-scanning/...`).
137 +GraphQL (`gh api graphql`) is useful for cross-repo queries or reaching
138 +combined data (e.g., alert + commit history in one call):
139 +
140 +```
141 +gh api graphql -f query='
142 + query($owner:String!,$name:String!) {
143 + repository(owner:$owner, name:$name) {
144 + vulnerabilityAlerts(first: 100, states:OPEN) {
145 + nodes { securityAdvisory { ghsaId, severity } }
146 + }
147 + }
148 + }' -F owner=netdata -F name=netdata
149 +```
150 +
151 +The above is for **Dependabot** advisories, not CodeQL — CodeQL alerts
152 +remain REST-only at the time of writing.
153 +
154 +## Failure modes — quick diagnosis
155 +
156 +| Symptom | Likely cause |
157 +|------------------------------------------|---------------------------------------------------------|
158 +| `HTTP 403 Resource not accessible by integration` | gh token lacks `security_events` scope |
159 +| `HTTP 404` on the alerts endpoint | Code Scanning not enabled for the repo, or wrong slug |
160 +| `gh: not found` | gh CLI not installed |
161 +| Empty output, no error | No alerts in that state — try `--state=dismissed` to confirm gh is reaching the API |
162 +| Pagination cuts off at 100 | Use `--paginate` (already in `codeql-list.sh`) |
.agents/skills/graphql-audit/scripts/_lib.sh new
+83
@@ -0,0 +1,83 @@
1 +#!/usr/bin/env bash
2 +# Common helpers for graphql-audit scripts.
3 +# Sourced from the per-action scripts; not executed directly.
4 +
5 +set -euo pipefail
6 +
7 +# IMPORTANT: define with $'...' so the variables contain real ESC bytes,
8 +# not the literal four-character string "\033". This way both `echo -e
9 +# "${GH_RED}..."` and `printf '%s' "${GH_RED}..."` render correctly --
10 +# without forcing every printf format string to be the variable itself
11 +# (which trips shellcheck SC2059) or %b (which adds inconsistency).
12 +#
13 +# Color vars are referenced by sourcing scripts; shellcheck cannot see that.
14 +# shellcheck disable=SC2034
15 +GH_RED=$'\033[0;31m'
16 +# shellcheck disable=SC2034
17 +GH_GREEN=$'\033[0;32m'
18 +# shellcheck disable=SC2034
19 +GH_YELLOW=$'\033[1;33m'
20 +# shellcheck disable=SC2034
21 +GH_GRAY=$'\033[0;90m'
22 +# shellcheck disable=SC2034
23 +GH_NC=$'\033[0m'
24 +
25 +gh_repo_root() {
26 + # Walk up from this _lib.sh; that's stable regardless of caller layout.
27 + git -C "$(dirname "${BASH_SOURCE[0]}")" rev-parse --show-toplevel
28 +}
29 +
30 +gh_repo_slug() {
31 + # Owner/repo of the upstream remote (or origin if no upstream).
32 + # Uses bash parameter expansion so repo names containing dots
33 + # (e.g. "my.repo", "kubernetes-sigs/cluster-api-provider-aws.git") parse
34 + # correctly. The previous regex `[^/.]+` truncated names with dots.
35 + # Returns empty for non-github.com remotes (this skill is GitHub-only).
36 + local root url
37 + root="$(gh_repo_root)"
38 + url="$(git -C "${root}" config --get remote.upstream.url 2>/dev/null \
39 + || git -C "${root}" config --get remote.origin.url)"
40 + # Strict github.com host match. `*github.com*` substring would
41 + # accept `notgithub.com` or `github.com.attacker.example.com`.
42 + # Three accepted forms cover SCP-style ssh, URL-style ssh, anonymous
43 + # https, and credentialed https (`x-access-token:TOK@github.com/...`).
44 + if [[ "${url}" != *@github.com:* \
45 + && "${url}" != *://github.com/* \
46 + && "${url}" != *@github.com/* ]]; then
47 + echo ""
48 + return
49 + fi
50 + url="${url%.git}" # strip trailing .git, if any
51 + url="${url#*github.com[:/]}" # strip everything up to and including github.com:/
52 + echo "${url}"
53 +}
54 +
55 +# Resolve and validate the repo slug. Returns "owner/repo" on stdout or
56 +# exits non-zero if no slug could be derived.
57 +gh_require_slug() {
58 + local slug
59 + slug="$(gh_repo_slug)"
60 + if [[ -z "${slug}" || "${slug}" != */* ]]; then
61 + echo -e "${GH_RED}[ERROR]${GH_NC} could not derive owner/repo from git remotes (got: '${slug}'). Fix the upstream/origin remote URL." >&2
62 + return 1
63 + fi
64 + printf '%s' "${slug}"
65 +}
66 +
67 +gh_audit_dir() {
68 + local root dir
69 + root="$(gh_repo_root)"
70 + dir="${root}/.local/audits/graphql"
71 + mkdir -p "${dir}"
72 + echo "${dir}"
73 +}
74 +
75 +# Run gh against the GitHub API. Authentication comes from `gh auth status`.
76 +# No token is required in .env when using the gh CLI directly.
77 +gh_api() {
78 + if ! command -v gh >/dev/null; then
79 + echo -e "${GH_RED}[ERROR]${GH_NC} 'gh' CLI is not installed. Install from https://cli.github.com/." >&2
80 + return 1
81 + fi
82 + gh "$@"
83 +}
.agents/skills/graphql-audit/scripts/codeql-dismiss.sh new
+46
@@ -0,0 +1,46 @@
1 +#!/usr/bin/env bash
2 +# Dismiss one GitHub Code Scanning alert.
3 +#
4 +# Usage:
5 +# codeql-dismiss.sh <alert_number> <reason> "<comment>"
6 +#
7 +# Reasons (per GitHub API):
8 +# false positive -- alert is incorrect
9 +# won't fix -- alert is correct but won't be fixed
10 +# used in tests -- alert appears only in test code
11 +#
12 +# This script is WRITE-side: it changes the state of an alert. Use codeql-list.sh
13 +# (read-only) to find alert numbers first.
14 +
15 +set -euo pipefail
16 +
17 +# shellcheck source=./_lib.sh
18 +# shellcheck disable=SC1091
19 +source "$(dirname "$0")/_lib.sh"
20 +
21 +NUMBER="${1:?usage: $0 <alert_number> <reason> '<comment>'}"
22 +REASON="${2:?usage}"
23 +COMMENT="${3:?usage}"
24 +
25 +if [[ ! "${NUMBER}" =~ ^[1-9][0-9]*$ ]]; then
26 + echo -e "${GH_RED}[ERROR]${GH_NC} alert_number must be a positive integer, got: '${NUMBER}'" >&2
27 + exit 1
28 +fi
29 +
30 +case "${REASON}" in
31 + "false positive"|"won't fix"|"used in tests") ;;
32 + *)
33 + echo -e "${GH_RED}[ERROR]${GH_NC} Reason must be one of: 'false positive', \"won't fix\", 'used in tests'." >&2
34 + exit 2
35 + ;;
36 +esac
37 +
38 +slug="$(gh_require_slug)"
39 +echo -e "${GH_GRAY}> Dismissing alert #${NUMBER} on ${slug}: ${REASON}${GH_NC}" >&2
40 +echo -e "${GH_GRAY} comment: ${COMMENT}${GH_NC}" >&2
41 +
42 +gh_api api --method PATCH "/repos/${slug}/code-scanning/alerts/${NUMBER}" \
43 + -f state=dismissed \
44 + -f dismissed_reason="${REASON}" \
45 + -f dismissed_comment="${COMMENT}" \
46 + --jq '{number, state, dismissed_reason, dismissed_comment, html_url}'
.agents/skills/graphql-audit/scripts/codeql-list.sh new
+62
@@ -0,0 +1,62 @@
1 +#!/usr/bin/env bash
2 +# List GitHub Code Scanning alerts (CodeQL with security-extended) for the repo.
3 +#
4 +# Usage:
5 +# codeql-list.sh # all OPEN alerts (default)
6 +# codeql-list.sh --state=open|fixed|dismissed
7 +# codeql-list.sh --severity=critical|high|medium|low|warning|note|error
8 +# codeql-list.sh --tool=CodeQL # filter by tool name
9 +# codeql-list.sh --raw # emit raw JSON instead of summary
10 +#
11 +# Authentication uses the `gh` CLI's stored credentials.
12 +# This is a READ-ONLY script.
13 +
14 +set -euo pipefail
15 +
16 +# shellcheck source=./_lib.sh
17 +# shellcheck disable=SC1091
18 +source "$(dirname "$0")/_lib.sh"
19 +
20 +state="open"
21 +severity=""
22 +tool=""
23 +raw=0
24 +
25 +while [[ $# -gt 0 ]]; do
26 + case "$1" in
27 + --state=*) state="${1#*=}"; shift ;;
28 + --severity=*) severity="${1#*=}"; shift ;;
29 + --tool=*) tool="${1#*=}"; shift ;;
30 + --raw) raw=1; shift ;;
31 + -h|--help)
32 + sed -n '2,12p' "$0"; exit 0 ;;
33 + *) echo "Unknown arg: $1" >&2; exit 2 ;;
34 + esac
35 +done
36 +
37 +slug="$(gh_require_slug)"
38 +
39 +# Build the API path.
40 +path="/repos/${slug}/code-scanning/alerts?state=${state}&per_page=100"
41 +[[ -n "${severity}" ]] && path="${path}&severity=${severity}"
42 +[[ -n "${tool}" ]] && path="${path}&tool_name=${tool}"
43 +
44 +# `gh api --paginate` writes the per-page JSON arrays back-to-back
45 +# (e.g. `[a,b,c][d,e]`), which is NOT a single valid JSON array. Pipe
46 +# through `jq -s 'add'` to slurp the multiple top-level values into one
47 +# array. Without this, downstream `jq '.[]'` only sees the first page.
48 +echo -e "${GH_GRAY}> gh api --paginate ${path}${GH_NC}" >&2
49 +data="$(gh_api api --paginate "${path}" | jq -s 'add // []')"
50 +
51 +if (( raw )); then
52 + printf '%s\n' "${data}"
53 + exit 0
54 +fi
55 +
56 +# Compact summary: count by rule.
57 +printf '%s\n' "${data}" \
58 + | jq -r '.[] | "\(.rule.id)|\(.rule.severity)|\(.most_recent_instance.location.path):\(.most_recent_instance.location.start_line)"' \
59 + | awk -F'|' '{c[$1"|"$2]++} END{for (k in c) print c[k], k}' \
60 + | sort -rn \
61 + | head -40 \
62 + | column -t -s'|'
.agents/skills/pr-reviews/SKILL.md new
+508
@@ -0,0 +1,508 @@
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)
.agents/skills/pr-reviews/scripts/_lib.sh new
+144
@@ -0,0 +1,144 @@
1 +#!/usr/bin/env bash
2 +# Common helpers for pr-reviews scripts.
3 +# Sourced from the per-action scripts; not executed directly.
4 +
5 +set -euo pipefail
6 +
7 +# IMPORTANT: define with $'...' so the variables contain real ESC bytes,
8 +# not the literal four-character string "\033". This way both `echo -e
9 +# "${PR_RED}..."` and `printf '%s' "${PR_RED}..."` render correctly --
10 +# without forcing every printf format string to be the variable itself
11 +# (which trips shellcheck SC2059) or %b (which adds inconsistency).
12 +#
13 +# Color vars are referenced by sourcing scripts; shellcheck cannot see that.
14 +# shellcheck disable=SC2034
15 +PR_RED=$'\033[0;31m'
16 +# shellcheck disable=SC2034
17 +PR_GREEN=$'\033[0;32m'
18 +# shellcheck disable=SC2034
19 +PR_YELLOW=$'\033[1;33m'
20 +# shellcheck disable=SC2034
21 +PR_GRAY=$'\033[0;90m'
22 +# shellcheck disable=SC2034
23 +PR_NC=$'\033[0m'
24 +
25 +pr_repo_root() {
26 + git -C "$(dirname "${BASH_SOURCE[0]}")" rev-parse --show-toplevel
27 +}
28 +
29 +# Owner/repo of the upstream remote (or origin if no upstream).
30 +# Override with PR_REPO_SLUG=owner/repo for cross-repo work.
31 +# Uses bash parameter expansion so repo names containing dots parse correctly.
32 +# Returns empty if the URL is not a github.com remote (this skill only
33 +# supports GitHub).
34 +pr_repo_slug() {
35 + if [[ -n "${PR_REPO_SLUG:-}" ]]; then
36 + # Validate the override too so callers can't smuggle whitespace or
37 + # shell metacharacters in via env. Allowed: owner/repo with
38 + # alphanumerics, dot, underscore, hyphen.
39 + if [[ ! "${PR_REPO_SLUG}" =~ ^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$ ]]; then
40 + echo "" >&2
41 + return
42 + fi
43 + echo "${PR_REPO_SLUG}"
44 + return
45 + fi
46 + local root url
47 + root="$(pr_repo_root)"
48 + url="$(git -C "${root}" config --get remote.upstream.url 2>/dev/null \
49 + || git -C "${root}" config --get remote.origin.url)"
50 + # Match github.com only as a host -- a substring match would accept
51 + # `notgithub.com`, `github.com.attacker.example.com`, etc.
52 + # Accepted forms (covering all common gh / git remote outputs):
53 + # git@github.com:owner/repo[.git] (SCP-style ssh)
54 + # ssh://git@github.com/owner/repo[.git] (URL-style ssh)
55 + # https://github.com/owner/repo[.git] (anonymous https)
56 + # https://x-access-token:TOK@github.com/... (credentialed https, gh auth)
57 + if [[ "${url}" != *@github.com:* \
58 + && "${url}" != *://github.com/* \
59 + && "${url}" != *@github.com/* ]]; then
60 + echo ""
61 + return
62 + fi
63 + url="${url%.git}" # strip trailing .git
64 + url="${url#*github.com[:/]}" # strip everything up to and including github.com:/
65 + echo "${url}"
66 +}
67 +
68 +# Audit/state directory for pr-reviews artifacts.
69 +pr_audit_dir() {
70 + local root dir
71 + root="$(pr_repo_root)"
72 + dir="${root}/.local/audits/pr-reviews"
73 + mkdir -p "${dir}"
74 + echo "${dir}"
75 +}
76 +
77 +# Per-PR working directory.
78 +pr_state_dir() {
79 + local pr="${1:?usage: pr_state_dir <pr-number>}"
80 + local dir
81 + dir="$(pr_audit_dir)/pr-${pr}"
82 + mkdir -p "${dir}"
83 + echo "${dir}"
84 +}
85 +
86 +# Verify gh is available and authenticated. Bail loudly otherwise.
87 +pr_require_gh() {
88 + if ! command -v gh >/dev/null; then
89 + echo -e "${PR_RED}[ERROR]${PR_NC} 'gh' CLI not installed. https://cli.github.com/" >&2
90 + return 1
91 + fi
92 + if ! gh auth status >/dev/null 2>&1; then
93 + echo -e "${PR_RED}[ERROR]${PR_NC} 'gh' is not authenticated. Run 'gh auth login'." >&2
94 + return 1
95 + fi
96 +}
97 +
98 +# Bot logins recognized by the skill. The first regex matches the AI reviewers
99 +# the skill iterates with autonomously; the second matches CI/quality bots
100 +# whose comments are informational (sonar quality gate, etc.).
101 +PR_AI_BOT_RE='^(cubic-dev-ai|copilot|copilot-pull-request-reviewer|github-copilot)\[bot\]$'
102 +PR_INFO_BOT_RE='^(sonarqubecloud|netdata-bot|github-actions|coderabbitai)\[bot\]$'
103 +
104 +# Classify a login -> "ai_bot" | "info_bot" | "human".
105 +pr_classify_author() {
106 + local login="$1"
107 + if [[ "${login}" =~ ${PR_AI_BOT_RE} ]]; then
108 + echo "ai_bot"
109 + elif [[ "${login}" =~ ${PR_INFO_BOT_RE} ]]; then
110 + echo "info_bot"
111 + else
112 + echo "human"
113 + fi
114 +}
115 +
116 +# Pretty timestamp in UTC.
117 +pr_now_utc() {
118 + date -u +%Y-%m-%dT%H:%M:%SZ
119 +}
120 +
121 +# PR numbers are positive integers (GitHub assigns 1+). Reject anything
122 +# else before interpolating into REST paths or `gh` arguments. Even though
123 +# URL interpolation isn't a shell-injection vector, malformed input causes
124 +# confusing API errors that look like permission/auth issues.
125 +pr_require_numeric() {
126 + local n="$1" name="${2:-PR}"
127 + if [[ ! "${n}" =~ ^[1-9][0-9]*$ ]]; then
128 + echo -e "${PR_RED}[ERROR]${PR_NC} ${name} must be a positive integer, got: '${n}'" >&2
129 + return 1
130 + fi
131 +}
132 +
133 +# Resolve and validate the repo slug. Returns "owner/repo" on stdout or
134 +# exits non-zero if no slug could be derived (no remotes configured, or
135 +# the URL didn't match github.com).
136 +pr_require_slug() {
137 + local slug
138 + slug="$(pr_repo_slug)"
139 + if [[ -z "${slug}" || "${slug}" != */* ]]; then
140 + echo -e "${PR_RED}[ERROR]${PR_NC} could not derive owner/repo from git remotes (got: '${slug}'). This skill only supports github.com remotes; set PR_REPO_SLUG=owner/repo or fix the remote URL." >&2
141 + return 1
142 + fi
143 + printf '%s' "${slug}"
144 +}
.agents/skills/pr-reviews/scripts/ci-status.sh new
+69
@@ -0,0 +1,69 @@
1 +#!/usr/bin/env bash
2 +# Report current CI status for a PR head commit.
3 +#
4 +# Usage:
5 +# ci-status.sh <pr-number> # human-readable summary
6 +# ci-status.sh <pr-number> --json # raw JSON
7 +#
8 +# This is read-only. Use it BEFORE every push to find FAILURES that must
9 +# be addressed in the same push. Do NOT wait for in-progress checks
10 +# between iterations -- the next push will trigger a fresh CI run on the
11 +# new code, which is what matters. See SKILL.md Step 4b for the policy.
12 +
13 +set -euo pipefail
14 +
15 +# shellcheck source=./_lib.sh
16 +# shellcheck disable=SC1091
17 +source "$(dirname "$0")/_lib.sh"
18 +pr_require_gh
19 +
20 +PR="${1:?usage: $0 <pr-number> [--json]}"
21 +pr_require_numeric "${PR}"
22 +JSON=0
23 +[[ "${2:-}" == "--json" ]] && JSON=1
24 +
25 +SLUG="$(pr_require_slug)"
26 +
27 +# pr view --json statusCheckRollup gives the per-check breakdown.
28 +data="$(gh pr view "${PR}" --repo "${SLUG}" --json statusCheckRollup,headRefOid,mergeStateStatus)"
29 +
30 +if (( JSON )); then
31 + printf '%s\n' "${data}"
32 + exit 0
33 +fi
34 +
35 +head_sha="$(jq -r '.headRefOid' <<< "${data}" | head -c 10)"
36 +merge_state="$(jq -r '.mergeStateStatus' <<< "${data}")"
37 +
38 +echo "Head: ${head_sha} Merge state: ${merge_state}"
39 +echo
40 +
41 +# Group checks by conclusion / status.
42 +jq -r '
43 + .statusCheckRollup
44 + | map(. + {key: ((.conclusion // .status) // "PENDING")})
45 + | group_by(.key)
46 + | map({key: .[0].key, count: length, names: [.[].name // .[].context]})
47 + | sort_by(.key)
48 + | .[]
49 + | "\(.key)\t\(.count)\t\((.names | sort | unique | join(", ")[0:200]))"
50 +' <<< "${data}" | column -t -s $'\t'
51 +
52 +echo
53 +total=$(jq '.statusCheckRollup | length' <<< "${data}")
54 +fail=$(jq '[.statusCheckRollup[] | select((.conclusion // "")|test("FAILURE|TIMED_OUT|CANCELLED"))] | length' <<< "${data}")
55 +running=$(jq '[.statusCheckRollup[] | select((.status // "")=="IN_PROGRESS" or (.status // "")=="QUEUED")] | length' <<< "${data}")
56 +echo "Total checks: ${total} Failing: ${fail} Running: ${running}"
57 +
58 +if (( running > 0 )); then
59 + # Exit 2 is informational, not a "do not push" signal. The next push
60 + # will start a fresh CI run on top of the new code -- that's what we
61 + # actually want to verify. The skill's policy is to push anyway.
62 + echo -e "${PR_GRAY}Note: ${running} check(s) still running; the next push will start a fresh CI run.${PR_NC}" >&2
63 + exit 2
64 +fi
65 +if (( fail > 0 )); then
66 + echo -e "${PR_RED}WARNING: ${fail} check(s) failing. Address these BEFORE pushing.${PR_NC}" >&2
67 + exit 3
68 +fi
69 +exit 0
.agents/skills/pr-reviews/scripts/fetch-all.sh new
+231
@@ -0,0 +1,231 @@
1 +#!/usr/bin/env bash
2 +# Fetch ALL comments / reviews / review threads for a PR, with paranoid pagination.
3 +#
4 +# Usage:
5 +# fetch-all.sh <pr-number>
6 +#
7 +# Outputs (under .local/audits/pr-reviews/pr-<N>/):
8 +# pr.json -- gh pr view dump (state, head sha, etc.)
9 +# issue-comments.json -- /repos/{slug}/issues/{n}/comments (top-level PR comments)
10 +# review-comments.json -- /repos/{slug}/pulls/{n}/comments (line-level inline comments)
11 +# reviews.json -- /repos/{slug}/pulls/{n}/reviews (review submissions with body)
12 +# review-threads.json -- GraphQL reviewThreads (per-thread isResolved + comments[])
13 +# summary.txt -- human-readable triage summary
14 +#
15 +# Pagination paranoia:
16 +# GitHub paginates everything. Default page size is 30, max 100. The skill's
17 +# #1 rule is: "do not stop at the pagination boundary -- if you see exactly
18 +# 100/200/300 items the round number is suspect; fetch one more page even
19 +# when the Link header says no more, just to confirm."
20 +#
21 +# This script:
22 +# 1. Uses --paginate (gh follows Link rel="next" automatically).
23 +# 2. Re-checks each result count against round-number multiples of 100.
24 +# If suspicious, explicitly requests page=N+1 and merges.
25 +# 3. Logs the final count for each source so the caller can verify.
26 +
27 +set -euo pipefail
28 +
29 +# shellcheck source=./_lib.sh
30 +# shellcheck disable=SC1091
31 +source "$(dirname "$0")/_lib.sh"
32 +pr_require_gh
33 +
34 +PR="${1:?usage: $0 <pr-number>}"
35 +pr_require_numeric "${PR}"
36 +SLUG="$(pr_require_slug)"
37 +DIR="$(pr_state_dir "${PR}")"
38 +
39 +echo -e "${PR_GRAY}[fetch-all] PR ${SLUG}#${PR} -> ${DIR}${PR_NC}" >&2
40 +
41 +# --- pr.json (state, head sha, draft, requested reviewers, ...) ------------
42 +gh pr view "${PR}" --repo "${SLUG}" --json \
43 + number,title,state,isDraft,headRefName,headRefOid,baseRefName,baseRefOid,reviewDecision,reviewRequests,mergeable,mergeStateStatus,statusCheckRollup,labels,createdAt,updatedAt,author \
44 + > "${DIR}/pr.json"
45 +
46 +# --- Helper: fetch a paginated REST endpoint with paranoia -----------------
47 +# Args: <api-path> <output-file> <kind-label>
48 +fetch_paranoid() {
49 + local path="$1" out="$2" kind="$3"
50 + # Ask for max page size and let gh follow rel=next.
51 + local sep
52 + if [[ "${path}" == *\?* ]]; then sep='&'; else sep='?'; fi
53 + # `gh api --paginate` writes the per-page JSON arrays back-to-back
54 + # (e.g. `[a,b,c][d,e]`), which is NOT a single valid JSON array. Pipe
55 + # through `jq -s 'add'` to slurp the multiple top-level values and
56 + # concatenate them into one array. (`gh api --paginate --jq '.[]'`
57 + # would emit JSONL but loses the array shape we need for the rest of
58 + # the loop.)
59 + gh api --paginate "${path}${sep}per_page=100" | jq -s 'add // []' > "${out}"
60 +
61 + # Did we end up with a JSON array?
62 + if ! jq -e 'type=="array"' "${out}" >/dev/null 2>&1; then
63 + echo -e "${PR_RED}[fetch-all] ${kind}: response is not a JSON array. Auth? Rate limit?${PR_NC}" >&2
64 + head -c 200 "${out}" >&2; echo >&2
65 + return 1
66 + fi
67 +
68 + local n
69 + n="$(jq 'length' "${out}")"
70 + echo -e "${PR_GRAY}[fetch-all] ${kind}: ${n} items${PR_NC}" >&2
71 +
72 + # Paranoia check: round multiples of 100 are suspicious. Explicitly
73 + # request page=N+1 to confirm we've reached the end.
74 + if (( n > 0 && n % 100 == 0 )); then
75 + local next_page=$(( n / 100 + 1 ))
76 + echo -e "${PR_YELLOW}[fetch-all] ${kind}: count is exactly ${n} (multiple of 100). Verifying with explicit page=${next_page}...${PR_NC}" >&2
77 +
78 + local probe
79 + probe="$(gh api "${path}${sep}per_page=100&page=${next_page}" 2>/dev/null || echo '[]')"
80 + local extra
81 + extra="$(jq 'length' <<< "${probe}")"
82 + if (( extra > 0 )); then
83 + echo -e "${PR_YELLOW}[fetch-all] ${kind}: page ${next_page} had ${extra} more items! Merging.${PR_NC}" >&2
84 + # Merge and continue probing further pages until empty.
85 + jq -s '.[0] + .[1]' "${out}" <(printf '%s' "${probe}") > "${out}.merged"
86 + mv "${out}.merged" "${out}"
87 + local p=$(( next_page + 1 ))
88 + while true; do
89 + probe="$(gh api "${path}${sep}per_page=100&page=${p}" 2>/dev/null || echo '[]')"
90 + extra="$(jq 'length' <<< "${probe}")"
91 + (( extra == 0 )) && break
92 + echo -e "${PR_YELLOW}[fetch-all] ${kind}: page ${p} had ${extra} more.${PR_NC}" >&2
93 + jq -s '.[0] + .[1]' "${out}" <(printf '%s' "${probe}") > "${out}.merged"
94 + mv "${out}.merged" "${out}"
95 + p=$(( p + 1 ))
96 + done
97 + n="$(jq 'length' "${out}")"
98 + echo -e "${PR_GREEN}[fetch-all] ${kind}: final count after probing: ${n}${PR_NC}" >&2
99 + else
100 + echo -e "${PR_GRAY}[fetch-all] ${kind}: page ${next_page} empty -- ${n} confirmed.${PR_NC}" >&2
101 + fi
102 + fi
103 +}
104 +
105 +# --- Three REST sources ----------------------------------------------------
106 +fetch_paranoid "/repos/${SLUG}/issues/${PR}/comments" "${DIR}/issue-comments.json" "issue-comments"
107 +fetch_paranoid "/repos/${SLUG}/pulls/${PR}/comments" "${DIR}/review-comments.json" "review-comments"
108 +fetch_paranoid "/repos/${SLUG}/pulls/${PR}/reviews" "${DIR}/reviews.json" "reviews"
109 +
110 +# --- GraphQL reviewThreads (resolved state + thread IDs) -------------------
111 +# REST does not expose review-thread IDs or isResolved -- we need GraphQL for
112 +# resolve-thread.sh. Pagination uses cursors here, not pages.
113 +echo -e "${PR_GRAY}[fetch-all] review-threads (GraphQL)${PR_NC}" >&2
114 +owner="${SLUG%%/*}"
115 +name="${SLUG##*/}"
116 +
117 +threads_tmp="$(mktemp "${TMPDIR:-/tmp}/pr-threads-XXXXXX.json")"
118 +trap 'rm -f "${threads_tmp}"' EXIT
119 +
120 +cursor=""
121 +echo '[]' > "${DIR}/review-threads.json"
122 +while true; do
123 + cursor_args=()
124 + if [[ -n "${cursor}" ]]; then
125 + cursor_args+=(-F "after=${cursor}")
126 + fi
127 + # The single-quoted GraphQL string contains $owner/$name/$number as
128 + # GraphQL placeholders, not shell variables; SC2016 is expected here.
129 + # shellcheck disable=SC2016
130 + gh api graphql -F owner="${owner}" -F name="${name}" -F number="${PR}" "${cursor_args[@]}" -f query='
131 + query($owner:String!, $name:String!, $number:Int!, $after:String) {
132 + repository(owner:$owner, name:$name) {
133 + pullRequest(number:$number) {
134 + reviewThreads(first:100, after:$after) {
135 + pageInfo { hasNextPage endCursor }
136 + nodes {
137 + id
138 + isResolved
139 + isOutdated
140 + path
141 + line
142 + comments(first:100) {
143 + pageInfo { hasNextPage endCursor }
144 + totalCount
145 + nodes {
146 + id
147 + databaseId
148 + body
149 + author { login }
150 + createdAt
151 + url
152 + }
153 + }
154 + }
155 + }
156 + }
157 + }
158 + }
159 + ' > "${threads_tmp}"
160 +
161 + page_nodes="$(jq '.data.repository.pullRequest.reviewThreads.nodes' "${threads_tmp}")"
162 + n_page="$(jq 'length' <<< "${page_nodes}")"
163 + # Append to running file
164 + jq -s '.[0] + .[1]' "${DIR}/review-threads.json" <(printf '%s' "${page_nodes}") > "${DIR}/review-threads.json.merged"
165 + mv "${DIR}/review-threads.json.merged" "${DIR}/review-threads.json"
166 +
167 + # Warn if any thread on this page has more than 100 comments -- the
168 + # inner connection is fetched first:100 only, so the tail is cut.
169 + truncated_threads="$(jq -r '
170 + [.data.repository.pullRequest.reviewThreads.nodes[]
171 + | select(.comments.pageInfo.hasNextPage)
172 + | .id] | join(", ")
173 + ' "${threads_tmp}")"
174 + if [[ -n "${truncated_threads}" ]]; then
175 + echo -e "${PR_YELLOW}[fetch-all] review-threads: nested comments truncated (>100) for: ${truncated_threads}${PR_NC}" >&2
176 + fi
177 +
178 + has_next="$(jq -r '.data.repository.pullRequest.reviewThreads.pageInfo.hasNextPage' "${threads_tmp}")"
179 + cursor="$(jq -r '.data.repository.pullRequest.reviewThreads.pageInfo.endCursor' "${threads_tmp}")"
180 + echo -e "${PR_GRAY}[fetch-all] review-threads: +${n_page} (hasNext=${has_next})${PR_NC}" >&2
181 + [[ "${has_next}" == "true" ]] || break
182 +done
183 +n_threads="$(jq 'length' "${DIR}/review-threads.json")"
184 +echo -e "${PR_GRAY}[fetch-all] review-threads: ${n_threads} threads total${PR_NC}" >&2
185 +
186 +# --- summary.txt -----------------------------------------------------------
187 +{
188 + echo "PR ${SLUG}#${PR} -- snapshot $(pr_now_utc)"
189 + state=$(jq -r '.state' "${DIR}/pr.json")
190 + is_draft=$(jq -r '.isDraft' "${DIR}/pr.json")
191 + head_oid=$(jq -r '.headRefOid' "${DIR}/pr.json")
192 + head_ref=$(jq -r '.headRefName' "${DIR}/pr.json")
193 + base_oid=$(jq -r '.baseRefOid' "${DIR}/pr.json")
194 + base_ref=$(jq -r '.baseRefName' "${DIR}/pr.json")
195 + decision=$(jq -r '.reviewDecision // "none"' "${DIR}/pr.json")
196 + mergeable=$(jq -r '.mergeable' "${DIR}/pr.json")
197 + merge_state=$(jq -r '.mergeStateStatus' "${DIR}/pr.json")
198 + [[ "${is_draft}" == "true" ]] && state="${state} (draft)"
199 + printf 'State: %s\nHead: %s on %s\nBase: %s on %s\nDecision: %s\nMerge: %s / %s\n' \
200 + "${state}" "${head_oid:0:10}" "${head_ref}" "${base_oid:0:10}" "${base_ref}" \
201 + "${decision}" "${mergeable}" "${merge_state}"
202 + echo
203 + echo "Reviewers requested:"
204 + jq -r '(.reviewRequests // [])[] | " - " + (.login // .name // "?")' "${DIR}/pr.json"
205 + echo
206 + echo "Counts:"
207 + printf ' issue-comments : %d\n' "$(jq 'length' "${DIR}/issue-comments.json")"
208 + printf ' review-comments : %d\n' "$(jq 'length' "${DIR}/review-comments.json")"
209 + printf ' reviews : %d\n' "$(jq 'length' "${DIR}/reviews.json")"
210 + n_resolved=$(jq '[.[] | select(.isResolved)] | length' "${DIR}/review-threads.json")
211 + n_open=$(jq '[.[] | select(.isResolved | not)] | length' "${DIR}/review-threads.json")
212 + printf ' review-threads : %d (resolved: %d, open: %d)\n' "${n_threads}" "${n_resolved}" "${n_open}"
213 + echo
214 + echo "Authors involved (count of items per author across all sources):"
215 + {
216 + jq -r '.[] | .user.login' "${DIR}/issue-comments.json"
217 + jq -r '.[] | .user.login' "${DIR}/review-comments.json"
218 + jq -r '.[] | .user.login' "${DIR}/reviews.json"
219 + } | sort | uniq -c | sort -rn | sed 's/^/ /'
220 + echo
221 + echo "Open review threads (need attention):"
222 + jq -r '.[] | select(.isResolved | not)
223 + | " THREAD " + .id
224 + + " " + .path + ":" + ((.line // "?") | tostring)
225 + + " comments=" + ((.comments.nodes | length) | tostring)
226 + + " by=" + (.comments.nodes[0].author.login // "?")' \
227 + "${DIR}/review-threads.json"
228 +} > "${DIR}/summary.txt"
229 +
230 +echo
231 +cat "${DIR}/summary.txt"
.agents/skills/pr-reviews/scripts/fetch-sonar-findings.sh new
+75
@@ -0,0 +1,75 @@
1 +#!/usr/bin/env bash
2 +# Fetch SonarCloud findings introduced by a specific pull request.
3 +#
4 +# Usage:
5 +# fetch-sonar-findings.sh <pr-number>
6 +#
7 +# SonarCloud does NOT post per-finding inline comments on the GitHub PR --
8 +# only a QualityGate summary comment is delivered. The actual issue list
9 +# lives behind /api/issues/search?pullRequest=<N> and /api/hotspots/search.
10 +# This script pulls both, so the PR-reviews loop can address them.
11 +#
12 +# Outputs (under .local/audits/pr-reviews/pr-<N>/):
13 +# sonar-issues.json -- all open issues this PR introduced
14 +# sonar-hotspots.json -- all open security hotspots this PR introduced
15 +#
16 +# Reads SONAR_TOKEN, SONAR_HOST_URL, SONAR_PROJECT from <repo-root>/.env --
17 +# the same .env entries the sonarqube-audit skill uses. If they're missing,
18 +# this script prints what's needed and exits 1.
19 +#
20 +# Sourcing strategy: this script is part of pr-reviews, but it leans on
21 +# the sonarqube-audit skill's `_lib.sh` helpers (sq_load_env, sq_paginate)
22 +# so we get a single paginator + token-masking implementation, rather
23 +# than duplicating the loop and risking drift.
24 +
25 +set -euo pipefail
26 +
27 +# shellcheck source=./_lib.sh
28 +# shellcheck disable=SC1091
29 +source "$(dirname "$0")/_lib.sh"
30 +
31 +# shellcheck disable=SC1091
32 +source "$(dirname "$0")/../../sonarqube-audit/scripts/_lib.sh"
33 +
34 +PR="${1:?usage: $0 <pr-number>}"
35 +pr_require_numeric "${PR}"
36 +
37 +# sq_load_env reads <repo-root>/.env; same .env the pr-reviews scripts use.
38 +sq_load_env
39 +
40 +DIR="$(pr_state_dir "${PR}")"
41 +
42 +echo -e "${PR_GRAY}[fetch-sonar] PR ${PR} -> ${DIR}${PR_NC}" >&2
43 +
44 +# Build per-source path. sq_paginate streams one JSON page per line; jq -s
45 +# slurps them and concatenates the array under each result key.
46 +sq_paginate "/api/issues/search?componentKeys=${SONAR_PROJECT}&pullRequest=${PR}&resolved=false" \
47 + | jq -s '[.[].issues[]]' > "${DIR}/sonar-issues.json"
48 +
49 +sq_paginate "/api/hotspots/search?projectKey=${SONAR_PROJECT}&pullRequest=${PR}&status=TO_REVIEW" \
50 + | jq -s '[.[].hotspots[]]' > "${DIR}/sonar-hotspots.json"
51 +
52 +# Summary
53 +n_issues="$(jq 'length' "${DIR}/sonar-issues.json")"
54 +n_hotspots="$(jq 'length' "${DIR}/sonar-hotspots.json")"
55 +
56 +echo
57 +echo "SonarCloud findings on PR ${PR}:"
58 +echo " issues: ${n_issues}"
59 +echo " hotspots: ${n_hotspots}"
60 +echo
61 +if (( n_issues > 0 )); then
62 + echo "Issues by severity / rule:"
63 + jq -r 'group_by(.severity + " " + .rule) | .[] | " \(.[0].severity) \(.[0].rule) x\(length)"' \
64 + "${DIR}/sonar-issues.json"
65 + echo
66 + echo "Issue details:"
67 + jq -r '.[] | " \(.key) \(.severity) \(.rule) \(.component | sub("^[^:]+:"; ""))\(if .line then ":" + (.line | tostring) else "" end)\n \(.message)"' \
68 + "${DIR}/sonar-issues.json"
69 +fi
70 +if (( n_hotspots > 0 )); then
71 + echo
72 + echo "Hotspots:"
73 + jq -r '.[] | " \(.key) \(.vulnerabilityProbability) \(.ruleKey) \(.component | sub("^[^:]+:"; ""))\(if .line then ":" + (.line | tostring) else "" end)\n \(.message)"' \
74 + "${DIR}/sonar-hotspots.json"
75 +fi
.agents/skills/pr-reviews/scripts/list-open-threads.sh new
+41
@@ -0,0 +1,41 @@
1 +#!/usr/bin/env bash
2 +# List open (unresolved) review threads on a PR with the comments inside each.
3 +# Usage:
4 +# list-open-threads.sh <pr-number> # full bodies
5 +# list-open-threads.sh <pr-number> --short # one-line per thread
6 +#
7 +# Reads from the cached fetch-all.sh dump. Run fetch-all.sh first.
8 +
9 +set -euo pipefail
10 +
11 +# shellcheck source=./_lib.sh
12 +# shellcheck disable=SC1091
13 +source "$(dirname "$0")/_lib.sh"
14 +
15 +PR="${1:?usage: $0 <pr-number> [--short]}"
16 +pr_require_numeric "${PR}"
17 +SHORT=0
18 +[[ "${2:-}" == "--short" ]] && SHORT=1
19 +
20 +DIR="$(pr_state_dir "${PR}")"
21 +FILE="${DIR}/review-threads.json"
22 +if [[ ! -f "${FILE}" || ! -r "${FILE}" || ! -s "${FILE}" ]]; then
23 + echo -e "${PR_RED}Missing ${FILE}. Run fetch-all.sh ${PR} first.${PR_NC}" >&2
24 + exit 1
25 +fi
26 +
27 +if (( SHORT )); then
28 + jq -r '
29 + .[]
30 + | select(.isResolved | not)
31 + | "\(.id) | \(.path):\(.line // "?") | \(.comments.nodes[0].author.login)"
32 + ' "${FILE}" | column -t -s '|'
33 +else
34 + jq -r '
35 + .[]
36 + | select(.isResolved | not)
37 + | "================================================================\nTHREAD: \(.id)\nFile: \(.path):\(.line // "?")\nOutdated: \(.isOutdated)\n----------------------------------------------------------------\n" + (
38 + [.comments.nodes[] | "[\(.author.login) at \(.createdAt)]\n\(.body)\n -> \(.url)\n"] | join("\n")
39 + )
40 + ' "${FILE}"
41 +fi
.agents/skills/pr-reviews/scripts/reply-thread.sh new
+53
@@ -0,0 +1,53 @@
1 +#!/usr/bin/env bash
2 +# Reply inside an existing review-comment thread.
3 +#
4 +# Usage:
5 +# reply-thread.sh <pr-number> <comment-id> <body-text>
6 +# reply-thread.sh <pr-number> <comment-id> @<file>
7 +#
8 +# <comment-id> is the REST databaseId of any comment in the thread (e.g. one
9 +# of `databaseId` from review-threads.json -> .comments.nodes[]). The new
10 +# reply is anchored under that thread automatically.
11 +#
12 +# To resolve the thread after replying, call resolve-thread.sh with the
13 +# thread's GraphQL node id.
14 +
15 +set -euo pipefail
16 +
17 +# shellcheck source=./_lib.sh
18 +# shellcheck disable=SC1091
19 +source "$(dirname "$0")/_lib.sh"
20 +pr_require_gh
21 +
22 +PR="${1:?usage: $0 <pr-number> <comment-id> <body>}"
23 +pr_require_numeric "${PR}"
24 +COMMENT_ID="${2:?usage}"
25 +BODY_ARG="${3:?usage}"
26 +
27 +pr_require_numeric "${COMMENT_ID}" "comment-id"
28 +
29 +if [[ "${BODY_ARG}" == @* ]]; then
30 + body_file="${BODY_ARG#@}"
31 + if [[ ! -f "${body_file}" || ! -r "${body_file}" || ! -s "${body_file}" ]]; then
32 + echo -e "${PR_RED}[ERROR]${PR_NC} body file not a readable, non-empty regular file: '${body_file}'" >&2
33 + exit 1
34 + fi
35 + BODY="$(cat "${body_file}")"
36 +else
37 + BODY="${BODY_ARG}"
38 +fi
39 +
40 +# Strip ALL whitespace (spaces, tabs, newlines, carriage returns) before
41 +# the empty-body check; previous version stripped spaces only and would
42 +# accept a tab-only or newline-only body.
43 +if [[ -z "${BODY//[[:space:]]/}" ]]; then
44 + echo -e "${PR_RED}[ERROR]${PR_NC} Empty body (whitespace-only)." >&2
45 + exit 2
46 +fi
47 +
48 +SLUG="$(pr_require_slug)"
49 +echo -e "${PR_GRAY}[reply-thread] PR ${SLUG}#${PR} reply to comment ${COMMENT_ID}${PR_NC}" >&2
50 +
51 +gh api --method POST "/repos/${SLUG}/pulls/${PR}/comments/${COMMENT_ID}/replies" \
52 + -f body="${BODY}" \
53 + --jq '"posted reply id=\(.id) url=\(.html_url)"'
.agents/skills/pr-reviews/scripts/resolve-thread.sh new
+43
@@ -0,0 +1,43 @@
1 +#!/usr/bin/env bash
2 +# Resolve a review thread on a PR (marks the conversation as done in the UI).
3 +#
4 +# Usage:
5 +# resolve-thread.sh <thread-node-id>
6 +#
7 +# <thread-node-id> is the GraphQL id from review-threads.json -> .[].id.
8 +# It looks like "PRRT_kwDO..." -- not the numeric REST id.
9 +#
10 +# Resolving a thread does NOT require a reply, but the convention is to reply
11 +# first then resolve. See SKILL.md.
12 +
13 +set -euo pipefail
14 +
15 +# shellcheck source=./_lib.sh
16 +# shellcheck disable=SC1091
17 +source "$(dirname "$0")/_lib.sh"
18 +pr_require_gh
19 +
20 +THREAD_ID="${1:?usage: $0 <thread-node-id>}"
21 +
22 +# Review-thread node IDs follow the pattern `PRRT_<base64ish>`. Validate
23 +# the prefix + the body charset so a malformed value can't reach the API.
24 +if [[ ! "${THREAD_ID}" =~ ^PRRT_[A-Za-z0-9_-]+$ ]]; then
25 + echo -e "${PR_RED}[ERROR]${PR_NC} thread-node-id must look like 'PRRT_...' (got: '${THREAD_ID}')" >&2
26 + exit 1
27 +fi
28 +
29 +# Fail fast on non-GitHub remotes. The mutation operates by node ID and
30 +# does not need the slug, but this script is GitHub-only -- making that
31 +# explicit at entry catches misconfiguration earlier than letting the
32 +# mutation hit a non-github API.
33 +pr_require_slug >/dev/null
34 +
35 +# The single-quoted GraphQL string has $threadId as a placeholder.
36 +# shellcheck disable=SC2016
37 +gh api graphql -F threadId="${THREAD_ID}" -f query='
38 + mutation($threadId:ID!) {
39 + resolveReviewThread(input: {threadId: $threadId}) {
40 + thread { id isResolved }
41 + }
42 + }
43 +' --jq '.data.resolveReviewThread.thread | "resolved=\(.isResolved) id=\(.id)"'
.agents/skills/pr-reviews/scripts/trigger-copilot.sh new
+30
@@ -0,0 +1,30 @@
1 +#!/usr/bin/env bash
2 +# Re-request the GitHub Copilot reviewer on a PR.
3 +#
4 +# Copilot does not react to comments. It re-runs only when re-added as a
5 +# requested reviewer. Calling this after pushing is the right way to get a
6 +# follow-up Copilot review.
7 +#
8 +# Usage:
9 +# trigger-copilot.sh <pr-number>
10 +
11 +set -euo pipefail
12 +
13 +# shellcheck source=./_lib.sh
14 +# shellcheck disable=SC1091
15 +source "$(dirname "$0")/_lib.sh"
16 +pr_require_gh
17 +
18 +PR="${1:?usage: $0 <pr-number>}"
19 +pr_require_numeric "${PR}"
20 +SLUG="$(pr_require_slug)"
21 +
22 +echo -e "${PR_GRAY}[trigger-copilot] PR ${SLUG}#${PR}: re-add @copilot as reviewer${PR_NC}" >&2
23 +
24 +# Force a fresh review run by removing then re-adding the reviewer.
25 +# `--remove-reviewer @copilot` returns non-zero when copilot is NOT
26 +# currently a requested reviewer (nothing to remove); we ignore that exit
27 +# so the subsequent `--add-reviewer` always runs and triggers the review.
28 +gh pr edit "${PR}" --repo "${SLUG}" --remove-reviewer "@copilot" >/dev/null 2>&1 || true
29 +gh pr edit "${PR}" --repo "${SLUG}" --add-reviewer "@copilot"
30 +echo -e "${PR_GREEN}[trigger-copilot] @copilot re-requested.${PR_NC}" >&2
.agents/skills/pr-reviews/scripts/trigger-cubic.sh new
+32
@@ -0,0 +1,32 @@
1 +#!/usr/bin/env bash
2 +# Re-trigger cubic-dev-ai to re-review the PR.
3 +#
4 +# Cubic does not react to comments inside threads. It re-reviews when a NEW
5 +# top-level PR comment mentions it. Posting the comment is the trigger.
6 +#
7 +# Usage:
8 +# trigger-cubic.sh <pr-number> [<extra-message>]
9 +#
10 +# Default body is "@cubic-dev-ai please review again". Override by passing a
11 +# second argument; the @cubic-dev-ai mention is always prepended so the bot
12 +# is guaranteed to see it.
13 +
14 +set -euo pipefail
15 +
16 +# shellcheck source=./_lib.sh
17 +# shellcheck disable=SC1091
18 +source "$(dirname "$0")/_lib.sh"
19 +pr_require_gh
20 +
21 +PR="${1:?usage: $0 <pr-number> [<extra-message>]}"
22 +pr_require_numeric "${PR}"
23 +EXTRA="${2:-please review again}"
24 +
25 +SLUG="$(pr_require_slug)"
26 +BODY="@cubic-dev-ai ${EXTRA}"
27 +
28 +echo -e "${PR_GRAY}[trigger-cubic] PR ${SLUG}#${PR}: ${BODY}${PR_NC}" >&2
29 +
30 +gh api --method POST "/repos/${SLUG}/issues/${PR}/comments" \
31 + -f body="${BODY}" \
32 + --jq '"posted comment id=\(.id) url=\(.html_url)"'
.agents/skills/pr-reviews/scripts/wait-for-activity.sh new
+132
@@ -0,0 +1,132 @@
1 +#!/usr/bin/env bash
2 +# Block until new activity appears on a PR (new comment, new review, new
3 +# commit), or until the timeout fires.
4 +#
5 +# Usage:
6 +# wait-for-activity.sh <pr-number> [<timeout-seconds>] [<poll-interval-seconds>]
7 +#
8 +# Defaults: timeout=1800 (30 min), poll=30s.
9 +#
10 +# Establishes a baseline by reading the cached fetch-all.sh dump (or fetching
11 +# fresh if missing). Then polls every <poll-interval> seconds for changes.
12 +# Exits 0 when something new is found; exits 124 on timeout.
13 +#
14 +# "New" means any of:
15 +# - issue-comments count changed
16 +# - review-comments count changed
17 +# - reviews count changed
18 +# - PR head sha changed (new push)
19 +# - reviewThreads isResolved transitions
20 +#
21 +# Both cubic-dev-ai and copilot post a comment when they have nothing new
22 +# to add -- so this loop ends naturally on a clean re-review.
23 +#
24 +# Note about Costa's rule: "the PR should never be left with unaddressed
25 +# comments". After this returns, run fetch-all.sh again, classify the new
26 +# activity, and address it.
27 +
28 +set -euo pipefail
29 +
30 +# shellcheck source=./_lib.sh
31 +# shellcheck disable=SC1091
32 +source "$(dirname "$0")/_lib.sh"
33 +pr_require_gh
34 +
35 +PR="${1:?usage: $0 <pr-number> [<timeout-seconds>] [<poll-interval-seconds>]}"
36 +pr_require_numeric "${PR}"
37 +TIMEOUT="${2:-1800}"
38 +POLL="${3:-30}"
39 +
40 +pr_require_numeric "${TIMEOUT}" "timeout-seconds"
41 +pr_require_numeric "${POLL}" "poll-interval-seconds"
42 +
43 +SLUG="$(pr_require_slug)"
44 +
45 +snapshot() {
46 + # Quick snapshot for change detection -- not a full fetch.
47 + #
48 + # `gh api --paginate ... --jq '.field'` emits one value per page (JSONL),
49 + # so we sum them with awk. `gh api --paginate ... | jq 'length'` would
50 + # NOT work here -- the concatenated arrays from paginate are not a
51 + # single JSON value.
52 + gh pr view "${PR}" --repo "${SLUG}" --json headRefOid \
53 + --jq '"head=\(.headRefOid)"'
54 + gh api --paginate "/repos/${SLUG}/issues/${PR}/comments?per_page=100" --jq 'if type=="array" then length else 0 end' 2>/dev/null \
55 + | awk 'BEGIN{t=0} {t+=$1} END{print "n_issue="t}'
56 + gh api --paginate "/repos/${SLUG}/pulls/${PR}/comments?per_page=100" --jq 'if type=="array" then length else 0 end' 2>/dev/null \
57 + | awk 'BEGIN{t=0} {t+=$1} END{print "n_review_comment="t}'
58 + gh api --paginate "/repos/${SLUG}/pulls/${PR}/reviews?per_page=100" --jq 'if type=="array" then length else 0 end' 2>/dev/null \
59 + | awk 'BEGIN{t=0} {t+=$1} END{print "n_review="t}'
60 + # Track resolve/unresolve transitions via the GraphQL reviewThreads
61 + # connection -- counts of resolved/open threads. A thread getting
62 + # resolved/unresolved is real PR activity that the REST counts above
63 + # would miss.
64 + #
65 + # IMPORTANT: this line MUST always be present in the snapshot, even on
66 + # transient GraphQL failure. If it disappears intermittently, the
67 + # snapshot diff would falsely show "new activity" each time it returns.
68 + # We collect the GraphQL output into a variable, fall back to a fixed
69 + # literal on any failure, and emit one canonical line.
70 + local owner name graphql_out
71 + owner="${SLUG%%/*}"
72 + name="${SLUG##*/}"
73 + # Cursor-paginate threads so PRs with >100 threads are tracked correctly.
74 + local cursor='' resolved=0 open=0
75 + while :; do
76 + local cursor_args=()
77 + [[ -n "${cursor}" ]] && cursor_args+=(-F "after=${cursor}")
78 + # GraphQL string has $owner/$name/$number/$after as placeholders.
79 + # shellcheck disable=SC2016
80 + graphql_out="$(gh api graphql -F owner="${owner}" -F name="${name}" -F number="${PR}" \
81 + "${cursor_args[@]}" -f query='
82 + query($owner:String!, $name:String!, $number:Int!, $after:String) {
83 + repository(owner:$owner, name:$name) {
84 + pullRequest(number:$number) {
85 + reviewThreads(first:100, after:$after) {
86 + pageInfo { hasNextPage endCursor }
87 + nodes { isResolved }
88 + }
89 + }
90 + }
91 + }' 2>/dev/null)" || { echo "threads_resolved=ERR_open=ERR"; return 0; }
92 + local r o
93 + r=$(jq -r '[.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved)] | length' <<< "${graphql_out}" 2>/dev/null) || r=0
94 + o=$(jq -r '[.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved | not)] | length' <<< "${graphql_out}" 2>/dev/null) || o=0
95 + resolved=$(( resolved + r ))
96 + open=$(( open + o ))
97 + local hasnext nextcur
98 + hasnext=$(jq -r '.data.repository.pullRequest.reviewThreads.pageInfo.hasNextPage // false' <<< "${graphql_out}" 2>/dev/null)
99 + nextcur=$(jq -r '.data.repository.pullRequest.reviewThreads.pageInfo.endCursor // ""' <<< "${graphql_out}" 2>/dev/null)
100 + [[ "${hasnext}" == "true" ]] || break
101 + cursor="${nextcur}"
102 + done
103 + echo "threads_resolved=${resolved}_open=${open}"
104 +}
105 +
106 +echo -e "${PR_GRAY}[wait] PR ${SLUG}#${PR} timeout=${TIMEOUT}s poll=${POLL}s${PR_NC}" >&2
107 +
108 +baseline="$(snapshot)"
109 +echo -e "${PR_GRAY}[wait] baseline:${PR_NC}" >&2
110 +echo "${baseline}" | sed 's/^/ /' >&2
111 +
112 +start=$(date +%s)
113 +while true; do
114 + sleep "${POLL}"
115 + now=$(date +%s)
116 + elapsed=$(( now - start ))
117 + if (( elapsed >= TIMEOUT )); then
118 + echo -e "${PR_YELLOW}[wait] timeout after ${elapsed}s -- no new activity${PR_NC}" >&2
119 + exit 124
120 + fi
121 + current="$(snapshot)"
122 + if [[ "${current}" != "${baseline}" ]]; then
123 + echo -e "${PR_GREEN}[wait] new activity detected after ${elapsed}s:${PR_NC}" >&2
124 + # diff exits 1 when inputs differ (that's the success case here);
125 + # set -euo pipefail would trip on it. Force exit 0 from the
126 + # pipeline so the script proper can return 0 cleanly.
127 + { diff <(printf '%s' "${baseline}") <(printf '%s' "${current}") || true; } \
128 + | sed 's/^/ /' >&2
129 + exit 0
130 + fi
131 + echo -e "${PR_GRAY}[wait] ${elapsed}s elapsed, no change yet...${PR_NC}" >&2
132 +done
.agents/skills/sonarqube-audit/SKILL.md new
+190
@@ -0,0 +1,190 @@
1 +---
2 +name: sonarqube-audit
3 +description: Triage SonarCloud findings (issues, hotspots, code smells, vulnerabilities) for this project — search what's open, mark False Positive / Won't Fix / Confirm / Safe / Acknowledged / Fixed, batch-mark whole rule families. Use when the user asks to "review Sonar findings", "triage SonarCloud", "mark False Positive on Sonar", or anything mentioning sonarqube/sonarcloud, S2259, S5008, code smells, security hotspots, or sonarcloud.io.
4 +---
5 +
6 +# SonarCloud triage skill
7 +
8 +This skill drives the SonarCloud Web API
9 +(<https://docs.sonarsource.com/sonarcloud/api/>) to enumerate findings and apply
10 +triage decisions: False Positive / Won't Fix / Confirmed for issues; Reviewed
11 +with one of Safe / Acknowledged / Fixed for hotspots. Family-mode lets
12 +you mark every open finding for a rule in one go.
13 +
14 +The skill operates on the project configured in `.env` (see Setup). Scripts
15 +auto-detect the repo root and write all artifacts under `<repo-root>/.local/`.
16 +
17 +## MANDATORY — keep this skill alive
18 +
19 +**If you (the agent) discover a new pattern, gotcha, working flow, correction,
20 +or any piece of knowledge while running this skill — update this `SKILL.md`
21 +AND commit it BEFORE proceeding. Knowledge that isn't committed is lost.**
22 +
23 +Examples of things to capture:
24 +- New rule with a known FP pattern (and the exact comment to use)
25 +- A bulk-FP family that's safe to apply project-wide
26 +- A SonarCloud API quirk (rate limits, undocumented response shapes)
27 +- A new path/issue exclusion that's safer than per-finding marking
28 +
29 +## Setup
30 +
31 +### .env entries
32 +
33 +```bash
34 +# SonarCloud
35 +SONAR_TOKEN='<paste your token from https://sonarcloud.io/account/security>'
36 +SONAR_HOST_URL=https://sonarcloud.io
37 +SONAR_PROJECT=<project_key, e.g. netdata_netdata>
38 +SONAR_ORG=<organization_key, e.g. netdata>
39 +```
40 +
41 +The token is used as **HTTP Basic auth username with empty password**:
42 +`-u "$SONAR_TOKEN:"` (note the trailing colon).
43 +
44 +No browser tab is required — token-based auth is stable across sessions.
45 +
46 +## Triage decision matrix
47 +
48 +### Issues (Bug, Vulnerability, Code Smell)
49 +
50 +| Decision | API transition | When to use |
51 +|--------------|-----------------|--------------------------------------------------------------------|
52 +| Confirm | `confirm` | Sonar is right, we're going to fix it |
53 +| Won't Fix | `wontfix` | Real but acceptable — won't fix (e.g., legacy code being deleted) |
54 +| False Positive | `falsepositive` | Sonar is wrong (guard exists, unreachable, tool model error) |
55 +
56 +### Security Hotspots
57 +
58 +Hotspots have a separate state machine. They go from `TO_REVIEW` to
59 +`REVIEWED` with one of three resolutions:
60 +
61 +| Resolution | When to use |
62 +|---------------|---------------------------------------------------------------|
63 +| `SAFE` | Hotspot reviewed, code is fine as-is (no risk in context) |
64 +| `ACKNOWLEDGED`| Risk understood, no immediate action — leave for future review |
65 +| `FIXED` | Hotspot reviewed and the code was changed to remove the risk |
66 +
67 +## ASCII-only comments — non-negotiable
68 +
69 +`api.sonarcloud.io` sits behind Cloudflare, which rejects bodies containing
70 +non-ASCII bytes (em-dashes, smart quotes) with a 403 challenge. The scripts
71 +fail before the network round-trip if non-ASCII is detected.
72 +
73 +- Use `--` instead of em-dash (U+2014).
74 +- Use straight quotes `"` `'` instead of smart quotes.
75 +
76 +## Workflow
77 +
78 +### Step 1 — see what's open
79 +
80 +```
81 +bash .agents/skills/sonarqube-audit/scripts/sonar-search.sh summary
82 +```
83 +
84 +Prints per-rule counts of open issues + open hotspots. Use this to spot
85 +high-volume rules that are candidates for family-mode bulk marking, and
86 +project-wide quality-profile or exclusion changes.
87 +
88 +### Step 2 — search for specific rule's findings
89 +
90 +Issues:
91 +```
92 +bash .agents/skills/sonarqube-audit/scripts/sonar-search.sh issues --rule cpp:S5827
93 +```
94 +
95 +Hotspots:
96 +```
97 +bash .agents/skills/sonarqube-audit/scripts/sonar-search.sh hotspots --status=TO_REVIEW \
98 + | jq '.hotspots[] | select(.ruleKey=="c:S5443")'
99 +```
100 +
101 +### Step 3 — triage
102 +
103 +#### Single finding
104 +
105 +```
106 +bash .agents/skills/sonarqube-audit/scripts/sonar-mark.sh fp <ISSUE_KEY> "<COMMENT>"
107 +bash .agents/skills/sonarqube-audit/scripts/sonar-mark.sh wontfix <ISSUE_KEY> "<COMMENT>"
108 +bash .agents/skills/sonarqube-audit/scripts/sonar-mark.sh confirm <ISSUE_KEY> "<COMMENT>"
109 +
110 +bash .agents/skills/sonarqube-audit/scripts/sonar-mark.sh safe <HOTSPOT_KEY> "<COMMENT>"
111 +bash .agents/skills/sonarqube-audit/scripts/sonar-mark.sh ack <HOTSPOT_KEY> "<COMMENT>"
112 +bash .agents/skills/sonarqube-audit/scripts/sonar-mark.sh fixed <HOTSPOT_KEY> "<COMMENT>"
113 +```
114 +
115 +#### Family mode (every open finding for a rule)
116 +
117 +```
118 +bash .agents/skills/sonarqube-audit/scripts/sonar-mark.sh family-fp <RULE_ID> "<COMMENT>"
119 +bash .agents/skills/sonarqube-audit/scripts/sonar-mark.sh family-safe <RULE_ID> "<COMMENT>"
120 +```
121 +
122 +Family mode prints all matched keys and prompts before acting unless
123 +`SONAR_MARK_YES=1` is set.
124 +
125 +### Step 4 — dry runs
126 +
127 +```
128 +SONAR_DRY_RUN=1 bash .agents/skills/sonarqube-audit/scripts/sonar-mark.sh fp KEY "Comment"
129 +```
130 +
131 +In dry-run mode, **write** API calls (mark issues, change hotspot status,
132 +add comments) are printed but not executed. **Read** API calls (issue
133 +search, hotspot search used to enumerate findings in family mode) still
134 +run -- otherwise family mode could not show what it would have acted on.
135 +
136 +## What this skill does NOT do
137 +
138 +- **Disable rules**: use `api/qualityprofiles/deactivate_rule` directly, or do
139 + it in the SonarCloud UI under Quality Profiles.
140 +- **Configure issue exclusions**: use Project Settings -> Analysis Scope ->
141 + Issue Exclusions in the UI.
142 +- **Rule-tuning audit**: when you want a per-rule KEEP/DISABLE/NARROW decision
143 + log, document that separately (it's project-wide policy, not per-finding
144 + triage).
145 +
146 +## Project-wide quality profile / exclusion configuration
147 +
148 +Effective profile lookup:
149 +```
150 +GET /api/qualityprofiles/search?project=$SONAR_PROJECT&organization=$SONAR_ORG
151 +```
152 +
153 +To make project-wide changes (deactivate a rule or override severity):
154 +1. Copy the inherited profile (`api/qualityprofiles/copy`)
155 +2. Make your edits there
156 +3. Assign the project to the new profile (`api/qualityprofiles/add_project`)
157 +
158 +This is a one-shot operation per language. SonarCloud language keys are:
159 +`c`, `cpp`, `go`, `javascript`, `py`, `shell`, `plsql`, `docker`, `css`,
160 +`ipynb`, `php` (and others depending on the project). Note the rule-id
161 +namespaces in `api/issues/search` results may differ from the language
162 +keys -- e.g. shell rules use the `shelldre:` prefix, Go rules can use
163 +either `go:` or `godre:` depending on which analyzer fired -- so the
164 +language argument to qualityprofile APIs is the SHORT key (`shell`,
165 +`go`), not the rule-namespace prefix.
166 +
167 +Keep a record of profile decisions in a project-local doc under
168 +`.local/audits/sonarqube/`.
169 +
170 +## Failure modes — quick diagnosis
171 +
172 +| Symptom | Likely cause |
173 +|----------------------------------------|-------------------------------------------------------------|
174 +| HTTP 401 / 403 with HTML body | Token wrong/expired, or Cloudflare blocking non-ASCII |
175 +| Token works for issues but not hotspots| Hotspot endpoints have separate auth checks — token must have `Browse` permission |
176 +| Family-mode appears to stop at 500 | Outdated -- `sonar-mark.sh` family-mode now paginates transparently via `sq_paginate`. If you still see truncation, check `sq_paginate`'s array-key recognition list. |
177 +| `falsepositive` transition rejected | Issue is not in `OPEN` or `CONFIRMED` state — check current status |
178 +| Hotspot transition rejected | Hotspot already in `REVIEWED` state — re-check before retry |
179 +
180 +## Recurring tips
181 +
182 +- `api/issues/search` is paged at `ps=500` max. The `sq_paginate` helper
183 + in `_lib.sh` walks every page until `paging.total`; use it from any
184 + new script instead of re-implementing the loop.
185 +- Hotspot `ruleKey` filtering is client-side (search only filters by
186 + status/project), so the family-mode helper does it in Python.
187 +- An issue may be transitioned only between certain states; if you get
188 + "Cannot do transition from STATUS X to Y", it's already past that state.
189 +- `SONAR_DRY_RUN=1` is the right knob when iterating on comments
190 + before committing to a bulk operation.
.agents/skills/sonarqube-audit/scripts/_lib.sh new
+188
@@ -0,0 +1,188 @@
1 +#!/usr/bin/env bash
2 +# Common helpers for sonarqube-audit scripts.
3 +# Sourced from the per-action scripts; not executed directly.
4 +
5 +set -euo pipefail
6 +
7 +# IMPORTANT: define with $'...' so the variables contain real ESC bytes,
8 +# not the literal four-character string "\033". This way both `echo -e
9 +# "${SQ_RED}..."` and `printf '%s' "${SQ_RED}..."` render correctly --
10 +# without forcing every printf format string to be the variable itself
11 +# (which trips shellcheck SC2059) or %b (which adds inconsistency).
12 +#
13 +# Color vars are referenced by sourcing scripts; shellcheck cannot see that.
14 +# shellcheck disable=SC2034
15 +SQ_RED=$'\033[0;31m'
16 +# shellcheck disable=SC2034
17 +SQ_GREEN=$'\033[0;32m'
18 +# shellcheck disable=SC2034
19 +SQ_YELLOW=$'\033[1;33m'
20 +# shellcheck disable=SC2034
21 +SQ_GRAY=$'\033[0;90m'
22 +# shellcheck disable=SC2034
23 +SQ_NC=$'\033[0m'
24 +
25 +sq_repo_root() {
26 + git -C "$(dirname "${BASH_SOURCE[0]}")" rev-parse --show-toplevel
27 +}
28 +
29 +sq_load_env() {
30 + local root env
31 + root="$(sq_repo_root)"
32 + env="${root}/.env"
33 + if [[ ! -f "${env}" || ! -r "${env}" ]]; then
34 + echo -e "${SQ_RED}[ERROR]${SQ_NC} Missing ${env}. See SKILL.md for the .env template." >&2
35 + return 1
36 + fi
37 + set -a
38 + # shellcheck disable=SC1090
39 + source "${env}"
40 + set +a
41 +
42 + : "${SONAR_TOKEN:?SONAR_TOKEN is empty in .env}"
43 + : "${SONAR_HOST_URL:=https://sonarcloud.io}"
44 + : "${SONAR_PROJECT:?SONAR_PROJECT is empty in .env (e.g. netdata_netdata)}"
45 + # SONAR_ORG is optional today -- the existing scripts don't pass it to
46 + # the API, but qualityprofile management calls (documented in SKILL.md)
47 + # require it. Default empty; consumers should fail loudly if they need it.
48 + : "${SONAR_ORG:=}"
49 + export SONAR_TOKEN SONAR_HOST_URL SONAR_PROJECT SONAR_ORG
50 +}
51 +
52 +sq_audit_dir() {
53 + local root dir
54 + root="$(sq_repo_root)"
55 + dir="${root}/.local/audits/sonarqube"
56 + mkdir -p "${dir}"
57 + echo "${dir}"
58 +}
59 +
60 +# Cloudflare in front of api.sonarcloud.io rejects non-ASCII bodies.
61 +# Fail before the network round-trip rather than debug a 403 challenge.
62 +#
63 +# `tr -d '\000-\177'` deletes ALL ASCII bytes; anything left is non-ASCII.
64 +# This is portable across GNU and BSD/macOS (unlike `grep -P`, which is GNU-only).
65 +sq_require_ascii() {
66 + local s="$1"
67 + if LC_ALL=C printf '%s' "${s}" | LC_ALL=C tr -d '\000-\177' | grep -q .; then
68 + echo -e "${SQ_RED}[ERROR]${SQ_NC} Comment contains non-ASCII characters. Cloudflare blocks them. Replace em-dashes with '--' and curly quotes with straight quotes." >&2
69 + return 1
70 + fi
71 +}
72 +
73 +# Print a curl invocation with the token masked (for transparency without leaking).
74 +# In SONAR_DRY_RUN mode the command is printed but not executed -- this only
75 +# affects calls routed through sq_run, which is the WRITE path (api_post).
76 +# Read-only API calls (issue/hotspot search used to enumerate findings) still
77 +# run in dry-run so the caller can see what would be acted on.
78 +sq_run() {
79 + local arg
80 + # Color vars contain real ESC bytes (defined with $'...'), so '%s' is
81 + # both safe (no SC2059) and correctly renders the colors.
82 + printf >&2 '%s> %s' "${SQ_GRAY}" "${SQ_YELLOW}"
83 + for arg in "$@"; do
84 + if [[ "${arg}" == "${SONAR_TOKEN}:" ]]; then
85 + printf >&2 '%q ' '<TOKEN>:'
86 + else
87 + printf >&2 '%q ' "${arg}"
88 + fi
89 + done
90 + printf >&2 '%s\n' "${SQ_NC}"
91 + if [[ "${SONAR_DRY_RUN:-0}" == "1" ]]; then
92 + return 0
93 + fi
94 + "$@"
95 +}
96 +
97 +# URL-encode a string for inclusion in a Sonar API query parameter.
98 +# Sonar rule IDs (`c:S2245`, `shelldre:S131`, etc.) contain `:` which must
99 +# become `%3A`; some rule namespaces also contain other reserved chars.
100 +# Limitation: emits the codepoint, not UTF-8 bytes, for non-ASCII -- safe
101 +# for Sonar rule IDs (always ASCII) but do NOT use this for arbitrary
102 +# user-supplied strings without auditing.
103 +sq_url_encode() {
104 + local s="$1" out="" i ch
105 + for (( i=0; i<${#s}; i++ )); do
106 + ch="${s:$i:1}"
107 + case "${ch}" in
108 + [a-zA-Z0-9._~-]) out+="${ch}" ;;
109 + *) out+="$(printf '%%%02X' "'${ch}")" ;;
110 + esac
111 + done
112 + printf '%s' "${out}"
113 +}
114 +
115 +# Read-only Sonar API call: prints the masked curl line (transparency)
116 +# but always executes (dry-run only suppresses WRITE calls). Use this for
117 +# enumeration calls that drive subsequent decisions (issue/hotspot search).
118 +sq_run_read() {
119 + local arg
120 + printf >&2 '%s> %s' "${SQ_GRAY}" "${SQ_YELLOW}"
121 + for arg in "$@"; do
122 + if [[ "${arg}" == "${SONAR_TOKEN}:" ]]; then
123 + printf >&2 '%q ' '<TOKEN>:'
124 + else
125 + printf >&2 '%q ' "${arg}"
126 + fi
127 + done
128 + printf >&2 '%s\n' "${SQ_NC}"
129 + "$@"
130 +}
131 +
132 +# Paginate a Sonar API listing endpoint until paging.total is reached.
133 +# Emits each page's body to stdout (one JSON value per page); the caller
134 +# composes them with `jq -s 'reduce .[] as $p (...)'` to sum or merge.
135 +#
136 +# Args:
137 +# $1 = path with all query params EXCEPT ps and p (e.g.
138 +# "/api/issues/search?componentKeys=...&resolved=false")
139 +#
140 +# Sonar's max page size is 500; we use it. The function uses sq_run_read
141 +# so the masked curl is logged but execution is not skipped in dry-run
142 +# (read-only enumeration should still happen so the caller sees what
143 +# would be acted on).
144 +sq_paginate() {
145 + local path="$1"
146 + local sep
147 + if [[ "${path}" == *\?* ]]; then sep='&'; else sep='?'; fi
148 + local page=1 total=-1 fetched=0
149 + while :; do
150 + local resp
151 + resp="$(sq_run_read curl --fail --silent --show-error -u "${SONAR_TOKEN}:" \
152 + "${SONAR_HOST_URL}${path}${sep}ps=500&p=${page}")"
153 +
154 + # Validate the response is a JSON object with a .paging field --
155 + # otherwise we can't decide when to stop and would loop forever
156 + # (or terminate prematurely on 0). Bail loudly.
157 + if ! jq -e 'type=="object" and has("paging")' >/dev/null 2>&1 <<< "${resp}"; then
158 + echo -e "${SQ_RED}[sq_paginate]${SQ_NC} response missing .paging on ${path} page ${page}; first 200 chars:" >&2
159 + head -c 200 <<< "${resp}" >&2; echo >&2
160 + return 1
161 + fi
162 +
163 + printf '%s\n' "${resp}"
164 + # Sonar wraps results either under .issues / .hotspots / .components
165 + # / .rules / .users; .paging is consistent across endpoints. Reject
166 + # any response whose array key we don't recognise so the caller is
167 + # forced to add support rather than silently get zero rows.
168 + local array_key in_page
169 + array_key=$(jq -r '
170 + (.issues|values|"issues") //
171 + (.hotspots|values|"hotspots") //
172 + (.components|values|"components") //
173 + (.rules|values|"rules") //
174 + (.users|values|"users") //
175 + empty
176 + ' <<< "${resp}")
177 + if [[ -z "${array_key}" ]]; then
178 + echo -e "${SQ_RED}[sq_paginate]${SQ_NC} unrecognized payload from ${path}; expected one of issues/hotspots/components/rules/users." >&2
179 + return 1
180 + fi
181 + in_page=$(jq -r --arg k "${array_key}" '.[$k] | length' <<< "${resp}")
182 + total=$(jq -r '.paging.total' <<< "${resp}")
183 + fetched=$(( fetched + in_page ))
184 + (( fetched >= total || in_page == 0 )) && break
185 + page=$(( page + 1 ))
186 + done
187 +}
188 +
.agents/skills/sonarqube-audit/scripts/sonar-mark.sh new
+177
@@ -0,0 +1,177 @@
1 +#!/usr/bin/env bash
2 +# Apply per-finding triage decisions on SonarCloud:
3 +# issues -> falsepositive | wontfix | confirm
4 +# hotspots -> REVIEWED + (SAFE | ACKNOWLEDGED | FIXED)
5 +#
6 +# AUTHENTICATION:
7 +# Reads SONAR_TOKEN, SONAR_HOST_URL, SONAR_PROJECT, SONAR_ORG from <repo-root>/.env.
8 +# Token is sent as basic-auth username with empty password.
9 +#
10 +# COMMENTS MUST BE ASCII-ONLY:
11 +# Cloudflare's WAF in front of api.sonarcloud.io blocks non-ASCII bodies
12 +# (em-dashes, smart quotes, accented characters). Stick to "--", '"', etc.
13 +#
14 +# USAGE:
15 +# sonar-mark.sh fp <ISSUE_KEY> <COMMENT> # Bug/Vuln -> False Positive
16 +# sonar-mark.sh wontfix <ISSUE_KEY> <COMMENT> # Bug/Vuln -> Won't Fix
17 +# sonar-mark.sh confirm <ISSUE_KEY> [COMMENT] # Bug/Vuln -> Confirmed (real, will fix)
18 +# sonar-mark.sh safe <HOTSPOT_KEY> <COMMENT> # Hotspot -> REVIEWED + SAFE
19 +# sonar-mark.sh ack <HOTSPOT_KEY> <COMMENT> # Hotspot -> REVIEWED + ACKNOWLEDGED
20 +# sonar-mark.sh fixed <HOTSPOT_KEY> <COMMENT> # Hotspot -> REVIEWED + FIXED
21 +#
22 +# FAMILY MODE (acts on every open finding for a rule):
23 +# sonar-mark.sh family-fp <RULE_ID> <COMMENT> # e.g. go:S2077
24 +# sonar-mark.sh family-safe <RULE_ID> <COMMENT> # e.g. c:S5443
25 +#
26 +# Family mode prints the matched keys and prompts before acting unless
27 +# SONAR_MARK_YES=1 is set in the environment.
28 +#
29 +# DRY RUN:
30 +# Set SONAR_DRY_RUN=1 to print the curl commands without executing.
31 +
32 +set -euo pipefail
33 +
34 +# shellcheck source=./_lib.sh
35 +# shellcheck disable=SC1091
36 +source "$(dirname "$0")/_lib.sh"
37 +sq_load_env
38 +
39 +api_post() {
40 + local path="$1"; shift
41 + sq_run curl --fail --silent --show-error \
42 + -u "${SONAR_TOKEN}:" \
43 + -X POST "${SONAR_HOST_URL}${path}" "$@"
44 +}
45 +
46 +issue_add_comment() {
47 + local key="$1" text="$2"
48 + sq_require_ascii "${text}"
49 + api_post "/api/issues/add_comment" \
50 + --data-urlencode "issue=${key}" \
51 + --data-urlencode "text=${text}" \
52 + -o /dev/null
53 +}
54 +
55 +issue_transition() {
56 + local key="$1" transition="$2"
57 + api_post "/api/issues/do_transition" \
58 + --data-urlencode "issue=${key}" \
59 + --data-urlencode "transition=${transition}" \
60 + -o /dev/null
61 +}
62 +
63 +mark_issue() {
64 + local transition="$1" key="$2" comment="${3:-}"
65 + if [[ -n "${comment}" ]]; then
66 + issue_add_comment "${key}" "${comment}"
67 + fi
68 + issue_transition "${key}" "${transition}"
69 + echo -e "${SQ_GREEN}[OK]${SQ_NC} issue ${key} -> ${transition}" >&2
70 +}
71 +
72 +hotspot_change_status() {
73 + local key="$1" resolution="$2" comment="$3"
74 + sq_require_ascii "${comment}"
75 + # Add the comment first so it persists even if the transition fails.
76 + api_post "/api/hotspots/add_comment" \
77 + --data-urlencode "hotspot=${key}" \
78 + --data-urlencode "comment=${comment}" \
79 + -o /dev/null
80 + api_post "/api/hotspots/change_status" \
81 + --data-urlencode "hotspot=${key}" \
82 + --data-urlencode "status=REVIEWED" \
83 + --data-urlencode "resolution=${resolution}" \
84 + -o /dev/null
85 + echo -e "${SQ_GREEN}[OK]${SQ_NC} hotspot ${key} -> REVIEWED/${resolution}" >&2
86 +}
87 +
88 +list_open_issues_for_rule() {
89 + # Sonar caps page size at 500; sq_paginate walks every page until
90 + # paging.total. Token is masked in transparency log.
91 + local rule="$1" rule_enc
92 + rule_enc="$(sq_url_encode "${rule}")"
93 + sq_paginate "/api/issues/search?componentKeys=${SONAR_PROJECT}&rules=${rule_enc}&resolved=false" \
94 + | jq -r '.issues[].key'
95 +}
96 +
97 +list_open_hotspots_for_rule() {
98 + # Sonar's hotspot search does not accept a rule filter -- we have to
99 + # fetch all TO_REVIEW hotspots and filter client-side. Pass the rule
100 + # via jq's --arg so values containing colons / quotes / shell
101 + # metacharacters cannot inject into the filter.
102 + local rule="$1"
103 + sq_paginate "/api/hotspots/search?projectKey=${SONAR_PROJECT}&status=TO_REVIEW" \
104 + | jq -r --arg rule "${rule}" '.hotspots[] | select(.ruleKey == $rule) | .key'
105 +}
106 +
107 +confirm_family() {
108 + local rule="$1" count="$2" action="$3"
109 + if [[ "${SONAR_MARK_YES:-0}" == "1" ]]; then
110 + return 0
111 + fi
112 + echo -e "${SQ_YELLOW}About to ${action} ${count} finding(s) for rule ${rule}.${SQ_NC}" >&2
113 + echo -en "${SQ_YELLOW}Proceed? [y/N] ${SQ_NC}" >&2
114 + local ans
115 + read -r ans
116 + # Lowercase via tr -- bash 4+ has ${var,,} but macOS ships bash 3.2.
117 + local ans_lc
118 + ans_lc=$(printf '%s' "${ans}" | tr '[:upper:]' '[:lower:]')
119 + [[ "${ans_lc}" == "y" || "${ans_lc}" == "yes" ]]
120 +}
121 +
122 +family_fp() {
123 + local rule="$1" comment="$2"
124 + sq_require_ascii "${comment}"
125 + local keys
126 + keys="$(list_open_issues_for_rule "${rule}")"
127 + local count
128 + count="$(printf '%s\n' "${keys}" | grep -c . || true)"
129 + if [[ "${count}" == "0" ]]; then
130 + echo -e "${SQ_YELLOW}No open issues for rule ${rule}.${SQ_NC}" >&2
131 + return 0
132 + fi
133 + echo "${keys}" >&2
134 + confirm_family "${rule}" "${count}" "mark as False Positive" || { echo "Aborted." >&2; return 1; }
135 + while IFS= read -r key; do
136 + [[ -z "${key}" ]] && continue
137 + mark_issue "falsepositive" "${key}" "${comment}"
138 + done <<< "${keys}"
139 +}
140 +
141 +family_safe() {
142 + local rule="$1" comment="$2"
143 + sq_require_ascii "${comment}"
144 + local keys
145 + keys="$(list_open_hotspots_for_rule "${rule}")"
146 + local count
147 + count="$(printf '%s\n' "${keys}" | grep -c . || true)"
148 + if [[ "${count}" == "0" ]]; then
149 + echo -e "${SQ_YELLOW}No open hotspots for rule ${rule}.${SQ_NC}" >&2
150 + return 0
151 + fi
152 + echo "${keys}" >&2
153 + confirm_family "${rule}" "${count}" "mark REVIEWED/SAFE" || { echo "Aborted." >&2; return 1; }
154 + while IFS= read -r key; do
155 + [[ -z "${key}" ]] && continue
156 + hotspot_change_status "${key}" "SAFE" "${comment}"
157 + done <<< "${keys}"
158 +}
159 +
160 +usage() {
161 + sed -n '4,33p' "$0"
162 + exit 2
163 +}
164 +
165 +cmd="${1:-}"; shift || true
166 +case "${cmd}" in
167 + fp) mark_issue "falsepositive" "${1:?key required}" "${2:?comment required}" ;;
168 + wontfix) mark_issue "wontfix" "${1:?key required}" "${2:?comment required}" ;;
169 + confirm) mark_issue "confirm" "${1:?key required}" "${2:-}" ;;
170 + safe) hotspot_change_status "${1:?key required}" "SAFE" "${2:?comment required}" ;;
171 + ack) hotspot_change_status "${1:?key required}" "ACKNOWLEDGED" "${2:?comment required}" ;;
172 + fixed) hotspot_change_status "${1:?key required}" "FIXED" "${2:?comment required}" ;;
173 + family-fp) family_fp "${1:?rule id required (e.g. go:S2077)}" "${2:?comment required}" ;;
174 + family-safe) family_safe "${1:?rule id required (e.g. c:S5443)}" "${2:?comment required}" ;;
175 + ""|-h|--help|help) usage ;;
176 + *) echo -e "${SQ_RED}[ERROR]${SQ_NC} Unknown subcommand: ${cmd}" >&2; usage ;;
177 +esac
.agents/skills/sonarqube-audit/scripts/sonar-search.sh new
+115
@@ -0,0 +1,115 @@
1 +#!/usr/bin/env bash
2 +# Search SonarCloud findings for the configured project.
3 +#
4 +# Usage:
5 +# sonar-search.sh issues [--rule RULE_ID] [--resolved=false|true]
6 +# sonar-search.sh hotspots [--status=TO_REVIEW|REVIEWED]
7 +# sonar-search.sh summary # rule + count for open issues + hotspots
8 +#
9 +# Output: a single merged JSON object on stdout (.issues / .hotspots is the
10 +# concatenation of all pages). Always paginated to .paging.total -- a `--ps`
11 +# arg is no longer accepted because it was a footgun (only the first page
12 +# was ever returned).
13 +#
14 +# This is a READ-ONLY script -- it does not mutate Sonar state. Safe to
15 +# run anytime to inspect what's outstanding.
16 +
17 +set -euo pipefail
18 +
19 +# shellcheck source=./_lib.sh
20 +# shellcheck disable=SC1091
21 +source "$(dirname "$0")/_lib.sh"
22 +sq_load_env
23 +
24 +cmd="${1:-summary}"; shift || true
25 +
26 +# Whitelist common URL-param values to avoid raw user input ending up in
27 +# the URL. Sonar would reject malformed params anyway, but the error
28 +# messages are confusing -- fail-fast locally instead.
29 +_validate_resolved() {
30 + local v="$1"
31 + case "${v}" in true|false) ;; *)
32 + echo -e "${SQ_RED}[ERROR]${SQ_NC} --resolved must be 'true' or 'false', got: '${v}'" >&2
33 + return 1 ;;
34 + esac
35 +}
36 +_validate_status() {
37 + local v="$1"
38 + case "${v}" in TO_REVIEW|REVIEWED) ;; *)
39 + echo -e "${SQ_RED}[ERROR]${SQ_NC} --status must be 'TO_REVIEW' or 'REVIEWED', got: '${v}'" >&2
40 + return 1 ;;
41 + esac
42 +}
43 +
44 +case "${cmd}" in
45 + issues)
46 + rule=""
47 + resolved="false"
48 + while [[ $# -gt 0 ]]; do
49 + arg="$1"
50 + case "${arg}" in
51 + --rule)
52 + if [[ $# -lt 2 ]]; then
53 + echo -e "${SQ_RED}[ERROR]${SQ_NC} --rule requires a value (e.g. --rule c:S2245)" >&2
54 + exit 2
55 + fi
56 + rule="$2"
57 + shift 2
58 + ;;
59 + --resolved=*) resolved="${arg#*=}"; shift ;;
60 + *) echo "Unknown arg: ${arg}" >&2; exit 2 ;;
61 + esac
62 + done
63 + _validate_resolved "${resolved}"
64 + path="/api/issues/search?componentKeys=${SONAR_PROJECT}&resolved=${resolved}"
65 + # URL-encode the rule id so values containing `:` (always),
66 + # spaces, or other reserved chars don't inject extra params.
67 + [[ -n "${rule}" ]] && path="${path}&rules=$(sq_url_encode "${rule}")"
68 + sq_paginate "${path}" \
69 + | jq -s '{paging: .[0].paging, issues: [.[].issues[]]}'
70 + ;;
71 +
72 + hotspots)
73 + status="TO_REVIEW"
74 + while [[ $# -gt 0 ]]; do
75 + arg="$1"
76 + case "${arg}" in
77 + --status=*) status="${arg#*=}"; shift ;;
78 + *) echo "Unknown arg: ${arg}" >&2; exit 2 ;;
79 + esac
80 + done
81 + _validate_status "${status}"
82 + path="/api/hotspots/search?projectKey=${SONAR_PROJECT}&status=${status}"
83 + sq_paginate "${path}" \
84 + | jq -s '{paging: .[0].paging, hotspots: [.[].hotspots[]]}'
85 + ;;
86 +
87 + summary)
88 + echo "=== Open issues by rule ===" >&2
89 + # Issue facets are computed server-side and returned on every page;
90 + # the first page's facet totals reflect ALL matching issues, so a
91 + # single fetch is correct here.
92 + sq_run_read curl --fail --silent --show-error -u "${SONAR_TOKEN}:" \
93 + "${SONAR_HOST_URL}/api/issues/search?componentKeys=${SONAR_PROJECT}&resolved=false&ps=1&facets=rules" \
94 + | jq -r '.facets[] | select(.property=="rules") | .values[] | " \(.val) (\(.count))"' \
95 + | sort -k2 -t'(' -nr | head -30
96 +
97 + echo >&2
98 + echo "=== Open hotspots by rule ===" >&2
99 + # Hotspot search has no facets, so we have to walk every page.
100 + sq_paginate "/api/hotspots/search?projectKey=${SONAR_PROJECT}&status=TO_REVIEW" \
101 + | jq -s '[.[].hotspots[].ruleKey]
102 + | group_by(.) | map({rule: .[0], count: length})
103 + | sort_by(-.count) | .[] | " \(.rule) (\(.count))"' -r \
104 + | head -30
105 + ;;
106 +
107 + "")
108 + echo "usage: $0 issues|hotspots|summary [args...]" >&2
109 + exit 2
110 + ;;
111 + *)
112 + echo "Unknown command: ${cmd}" >&2
113 + exit 2
114 + ;;
115 +esac
.gitignore
+4
@@ -2,6 +2,10 @@
2 gcs-credentials.json
3 /.env
4
5 +# Per-user local working directory (audits, scratch, drafts, temp artifacts).
6 +# AI agents that follow AGENTS.md treat this as the place to write run-time data.
7 +/.local/
8 +
9 # Cross-tool AI agent private overrides (per-user, not for sharing)
10 **/AGENTS.local.md
11
AGENTS.md
+50
@@ -47,3 +47,53 @@ These files MUST be consistent with each other. For example:
47 - "Netdata Agent" (capitalized) when referring to the product
48 - "`netdata`" (lowercase, code-formatted) when referring to the process
49 - See DICTIONARY.md for precise terminology
50 +
51 +## AI agent skills
52 +
53 +Repo-scoped skills for AI agents live under `.agents/skills/<skill-name>/`.
54 +Each skill is self-contained: a `SKILL.md` with frontmatter (`name`,
55 +`description`) plus its own `scripts/` directory. Skills carry the operational
56 +knowledge for tasks that recur across sessions (Coverity triage, SonarCloud
57 +triage, GitHub Code Scanning triage, etc.).
58 +
59 +When an agent learns something new while running a skill (a new gotcha, a
60 +working API call, a corrected workflow) it MUST update the skill's
61 +`SKILL.md` and commit it before proceeding. Knowledge that isn't committed
62 +is lost.
63 +
64 +Currently available skills:
65 +- `.agents/skills/coverity-audit/` - Coverity Scan defect triage
66 +- `.agents/skills/sonarqube-audit/` - SonarCloud findings triage
67 +- `.agents/skills/graphql-audit/` - GitHub Code Scanning (CodeQL) triage
68 +- `.agents/skills/pr-reviews/` - PR comment / review iteration loop
69 +
70 +## Local-only working directory
71 +
72 +`/.local/` at the repo root is gitignored and reserved for per-user runtime
73 +artifacts: audit reports, fetched API data, scratch notes, queue files,
74 +intermediate triage decisions. Agents writing skill output should default to
75 +`<repo-root>/.local/audits/<topic>/...` -- where `<topic>` is the skill
76 +name with any trailing `-audit` suffix removed (so `coverity-audit/`
77 +writes under `coverity/`, `pr-reviews/` writes under `pr-reviews/`).
78 +
79 +Convention:
80 +- `/.local/audits/coverity/` - Coverity raw fetches, per-defect details, triage decisions
81 +- `/.local/audits/sonarqube/` - Sonar finding queues, FP comment templates
82 +- `/.local/audits/graphql/` - GitHub Code Scanning fetches and dismissals
83 +- `/.local/audits/pr-reviews/`- Per-PR comment / review caches
84 +
85 +Naming: each skill `<topic>-audit/` writes to `.local/audits/<topic>/`
86 +(the `-audit` suffix is dropped from the directory name so the URL-style
87 +path stays short). Skills without the `-audit` suffix keep their full
88 +name (e.g. `pr-reviews/` writes to `.local/audits/pr-reviews/`). When
89 +adding a new skill, follow this convention.
90 +
91 +Nothing under `/.local/` is committed. Treat the directory as ephemeral
92 +between users and machines, not as a shared source of truth.
93 +
94 +## Per-user secrets via `.env`
95 +
96 +`/.env` at the repo root is gitignored and holds per-user secrets and
97 +endpoint configuration consumed by skill scripts: API tokens, session
98 +cookies, project keys. Each skill's `SKILL.md` documents the variables it
99 +needs. Never commit secrets; never hard-code tokens in scripts.