From dcdd33a68db020735965cbd6b68ec273fcfa6bec Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Mon, 21 Sep 2026 17:59:46 -0600 Subject: [PATCH 1/2] PR checks: keep a findings/remediated table in the status comment The spelling, links, and redirects checks now post and update their summary comment through dev/upsert-report-comment.sh, which keeps a Total findings / Remediated table below the heading. The spelling summary is no longer minimized when resolved. dev/pr-check-metrics.sh sums the tables over merged PRs to report how many findings were fixed before merging and how many were merged in. Amp-Thread-ID: https://ampcode.com/threads/T-01a0c49e-51da-72ca-8a4a-6c0660c5e505 Co-authored-by: Amp --- .github/workflows/check-links.yml | 27 ++---- .github/workflows/check-redirects.yml | 27 ++---- .github/workflows/spellcheck.yml | 8 +- AGENTS.md | 5 +- dev/post-spelling-review.mjs | 114 +++++--------------------- dev/pr-check-metrics.sh | 28 +++++++ dev/upsert-report-comment.sh | 66 +++++++++++++++ dev/vercel-ignore-build.sh | 2 + 8 files changed, 137 insertions(+), 140 deletions(-) create mode 100755 dev/pr-check-metrics.sh create mode 100755 dev/upsert-report-comment.sh diff --git a/.github/workflows/check-links.yml b/.github/workflows/check-links.yml index 78629e1fc..247c26db9 100644 --- a/.github/workflows/check-links.yml +++ b/.github/workflows/check-links.yml @@ -85,29 +85,14 @@ jobs: PR_NUMBER: ${{ github.event.pull_request.number }} RESULT: ${{ steps.check.outputs.result }} RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + # The status comment keeps a running total; see dev/upsert-report-comment.sh run: | - marker='' - existing_comment=$(gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" \ - --paginate --jq ".[] | select(.body | startswith(\"$marker\")) | .id" | head -n 1) - - # Comment when there is something to report, or an earlier report to resolve - case "$RESULT" in - broken) - { echo "$marker"; cat "$RUNNER_TEMP/report.md"; } > "$RUNNER_TEMP/comment.md" ;; - crashed) - printf '%s\n### ⚠️ The links check could not run on this revision\n\nThis is a problem with the check, not with this PR; see the [job log](%s).\n' \ - "$marker" "$RUN_URL" > "$RUNNER_TEMP/comment.md" ;; - *) - [ -n "$existing_comment" ] || exit 0 - printf '%s\n### ✅ This revision introduces no broken links\n' \ - "$marker" > "$RUNNER_TEMP/comment.md" ;; - esac - - if [ -n "$existing_comment" ]; then - gh api --method PATCH "repos/$GITHUB_REPOSITORY/issues/comments/$existing_comment" \ - --field body=@"$RUNNER_TEMP/comment.md" + if [ "$RESULT" = crashed ]; then + printf '### ⚠️ The links check could not run on this revision\n\nThis is a problem with the check, not with this PR; see the [job log](%s).\n' \ + "$RUN_URL" > "$RUNNER_TEMP/report.md" + dev/upsert-report-comment.sh '' "$RUNNER_TEMP/report.md" --crashed else - gh pr comment "$PR_NUMBER" --body-file "$RUNNER_TEMP/comment.md" + dev/upsert-report-comment.sh '' "$RUNNER_TEMP/report.md" fi - name: Suggest fixes as review comments diff --git a/.github/workflows/check-redirects.yml b/.github/workflows/check-redirects.yml index ce4658f95..99d476382 100644 --- a/.github/workflows/check-redirects.yml +++ b/.github/workflows/check-redirects.yml @@ -87,29 +87,14 @@ jobs: PR_NUMBER: ${{ github.event.pull_request.number }} RESULT: ${{ steps.check.outputs.result }} RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + # The status comment keeps a running total; see dev/upsert-report-comment.sh run: | - marker='' - existing_comment=$(gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" \ - --paginate --jq ".[] | select(.body | startswith(\"$marker\")) | .id" | head -n 1) - - # Comment when there is something to report, or an earlier report to resolve - case "$RESULT" in - broken) - { echo "$marker"; cat "$RUNNER_TEMP/report.md"; } > "$RUNNER_TEMP/comment.md" ;; - crashed) - printf '%s\n### ⚠️ The redirects check could not run on this revision\n\nThis is a problem with the check, not with this PR; see the [job log](%s).\n' \ - "$marker" "$RUN_URL" > "$RUNNER_TEMP/comment.md" ;; - *) - [ -n "$existing_comment" ] || exit 0 - printf '%s\n### ✅ This revision breaks no redirects\n' \ - "$marker" > "$RUNNER_TEMP/comment.md" ;; - esac - - if [ -n "$existing_comment" ]; then - gh api --method PATCH "repos/$GITHUB_REPOSITORY/issues/comments/$existing_comment" \ - --field body=@"$RUNNER_TEMP/comment.md" + if [ "$RESULT" = crashed ]; then + printf '### ⚠️ The redirects check could not run on this revision\n\nThis is a problem with the check, not with this PR; see the [job log](%s).\n' \ + "$RUN_URL" > "$RUNNER_TEMP/report.md" + dev/upsert-report-comment.sh '' "$RUNNER_TEMP/report.md" --crashed else - gh pr comment "$PR_NUMBER" --body-file "$RUNNER_TEMP/comment.md" + dev/upsert-report-comment.sh '' "$RUNNER_TEMP/report.md" fi - name: Suggest fixes as review comments diff --git a/.github/workflows/spellcheck.yml b/.github/workflows/spellcheck.yml index 51f98bae8..606c27c8e 100644 --- a/.github/workflows/spellcheck.yml +++ b/.github/workflows/spellcheck.yml @@ -71,9 +71,13 @@ jobs: HEAD_REF: ${{ github.event.pull_request.head.ref }} RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} CRASHED: ${{ steps.check.outputs.crashed }} + # The status comment keeps a running total; see dev/upsert-report-comment.sh run: | if [ "$CRASHED" = true ]; then - node dev/post-spelling-review.mjs --crashed + printf '### ⚠️ The spell check could not run on this revision\n\nThis is a problem with the check, not with this PR; see the [job log](%s).\n' \ + "$RUN_URL" > "$RUNNER_TEMP/summary.md" + dev/upsert-report-comment.sh '' "$RUNNER_TEMP/summary.md" --crashed else - node dev/post-spelling-review.mjs --findings "$RUNNER_TEMP/spelling.json" + node dev/post-spelling-review.mjs --findings "$RUNNER_TEMP/spelling.json" --summary "$RUNNER_TEMP/summary.md" + dev/upsert-report-comment.sh '' "$RUNNER_TEMP/summary.md" fi diff --git a/AGENTS.md b/AGENTS.md index 8ae767663..2e8758739 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -74,7 +74,10 @@ gh api repos/sourcegraph/docs/pulls//comments --paginate \ Fork PRs get no comments; the findings are in the job log (`gh run view --log`). The checks resolve their own comments on the -next run. +next run: inline comments are deleted, and the summary comment is updated in +place (`dev/upsert-report-comment.sh`) with a `Total findings` / `Remediated` +table, so a merged PR's comment says how many findings it was merged with. +`dev/pr-check-metrics.sh [merged-since]` sums those tables over merged PRs. ### Vercel build failures diff --git a/dev/post-spelling-review.mjs b/dev/post-spelling-review.mjs index 15d30044c..93c133c27 100644 --- a/dev/post-spelling-review.mjs +++ b/dev/post-spelling-review.mjs @@ -1,28 +1,25 @@ #!/usr/bin/env node /** - * Reports CSpell findings on a pull request: one summary comment in the - * discussion, plus an inline review comment on each flagged line. Findings in - * the pull request's title or description (those with a `field`) have no line - * to comment on, so they appear in the summary only. Once the findings are - * fixed, the summary is minimized as resolved and the inline comments are - * deleted; the review that carried them has no body, so nothing of it remains - * visible. + * Reports CSpell findings on a pull request: writes the summary for the PR's + * status comment (posted by dev/upsert-report-comment.sh), and keeps one + * inline review comment on each flagged line. Findings in the pull request's + * title or description (those with a `field`) have no line to comment on, so + * they appear in the summary only. Once the findings are fixed, the inline + * comments are deleted; the review that carried them has no body, so nothing + * of it remains visible. * - * Usage: node dev/post-spelling-review.mjs --findings [--dry-run] - * node dev/post-spelling-review.mjs --crashed [--dry-run] + * Usage: node dev/post-spelling-review.mjs --findings --summary [--dry-run] * - * Reads the JSON written by `dev/check-spelling.mjs --format json`. With - * `--crashed`, the summary says the check could not run and links the job log - * (RUN_URL) instead; the inline comments are left as they are. + * Reads the JSON written by `dev/check-spelling.mjs --format json`. * Requires GH_TOKEN, GITHUB_REPOSITORY, PR_NUMBER, HEAD_SHA and HEAD_REF. */ -import {readFileSync} from 'fs'; +import {readFileSync, writeFileSync} from 'fs'; const args = process.argv.slice(2); const FINDINGS_FILE = args[args.indexOf('--findings') + 1]; -const CRASHED = args.includes('--crashed'); +const SUMMARY_FILE = args[args.indexOf('--summary') + 1]; const DRY_RUN = args.includes('--dry-run'); const MAX_INLINE_COMMENTS = 25; @@ -31,12 +28,10 @@ const REPOSITORY = process.env.GITHUB_REPOSITORY; const PR_NUMBER = process.env.PR_NUMBER; const HEAD_SHA = process.env.HEAD_SHA; const HEAD_REF = process.env.HEAD_REF; -const RUN_URL = process.env.RUN_URL; // Link to the PR branch, not the commit, so GitHub's edit button works from it const ALLOW_LIST_LINK = `[\`cspell-allow-list.txt\`](https://github.com/${REPOSITORY}/blob/${HEAD_REF}/cspell-allow-list.txt)`; -const SUMMARY_MARKER = ''; const INLINE_MARKER = '$/ { check = $2; sub(/-report$/, "", check) } + check && /^\| [0-9]+ \| [0-9]+ \|$/ { print check, pr, $2, $4; check = "" }' +done \ +| awk -v since="$since" ' + { prs[$1]++; total[$1] += $3; remediated[$1] += $4; merged_with[$1] += $3 - $4 } + END { + print "PRs merged since " since + printf "%-16s %8s %8s %12s %12s\n", "check", "PRs", "total", "remediated", "merged with" + for (check in prs) + printf "%-16s %8d %8d %12d %12d\n", check, prs[check], total[check], remediated[check], merged_with[check] + }' diff --git a/dev/upsert-report-comment.sh b/dev/upsert-report-comment.sh new file mode 100755 index 000000000..6b4f6ca4c --- /dev/null +++ b/dev/upsert-report-comment.sh @@ -0,0 +1,66 @@ +#!/usr/bin/env sh +# Post a check's report as the PR's status comment for that check, or update +# the one it posted earlier (found by the marker on its first line). Below the +# report's first line, the comment keeps a table the check's history can be read +# from, since inline comments are deleted once fixed and the report is replaced: +# +# | Total findings | Remediated | +# | --- | --- | +# | 12 | 8 | +# +# Total findings is the most any revision of the PR had; Remediated is how many +# of those the current revision no longer has, so a merged PR's comment says +# how many it was merged with. The report's first line must contain the +# current count as its first integer, or no integer when there are none. +# +# Nothing is posted for a PR that never had findings, to spare its author the +# notification; updating an existing comment sends none. +# +# Usage: dev/upsert-report-comment.sh '' report.md +# dev/upsert-report-comment.sh '' notice.md --crashed +# +# With --crashed, the report says the check could not run; the table carries +# over from the previous comment, if there was one. +# Needs GH_TOKEN, GITHUB_REPOSITORY, and PR_NUMBER. +set -eu +marker=$1 +report=$2 +crashed=${3:-} +comments="repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" + +existing=$(gh api "$comments" --paginate \ + --jq ".[] | select(.body | startswith(\"$marker\")) | {id, body}" | jq -c . | head -n 1) +# "total remediated" from the previous comment's table row, or "0 0" +previous=$(printf '%s' "$existing" | jq -r '.body // ""' \ + | sed -n 's/^| \([0-9]*\) | \([0-9]*\) |$/\1 \2/p' | head -n 1) +previous=${previous:-0 0} +previous_total=${previous%% *} +previous_remediated=${previous##* } + +if [ "$crashed" = --crashed ]; then + total=$previous_total + remediated=$previous_remediated +else + current=$(head -n 1 "$report" | sed -n 's/^[^0-9]*\([0-9][0-9]*\).*/\1/p') + current=${current:-0} + total=$(( current > previous_total ? current : previous_total )) + remediated=$(( total - current )) + [ -n "$existing" ] || [ "$total" -gt 0 ] || exit 0 +fi + +body=$(mktemp) +{ + echo "$marker" + head -n 1 "$report" + if [ -n "$existing" ] || [ "$crashed" != --crashed ]; then + printf '\n| Total findings | Remediated |\n| --- | --- |\n| %s | %s |\n' "$total" "$remediated" + fi + tail -n +2 "$report" +} > "$body" + +if [ -n "$existing" ]; then + gh api --method PATCH "repos/$GITHUB_REPOSITORY/issues/comments/$(printf '%s' "$existing" | jq -r .id)" \ + --field body=@"$body" > /dev/null +else + gh pr comment "$PR_NUMBER" --body-file "$body" +fi diff --git a/dev/vercel-ignore-build.sh b/dev/vercel-ignore-build.sh index 6eaaef00e..75953c9cd 100755 --- a/dev/vercel-ignore-build.sh +++ b/dev/vercel-ignore-build.sh @@ -18,4 +18,6 @@ exec git diff --quiet HEAD^ HEAD -- . \ ':(exclude)dev/report-vercel-build.mjs' \ ':(exclude)dev/verify-links-live.mjs' \ ':(exclude)dev/sync-review-comments.sh' \ + ':(exclude)dev/upsert-report-comment.sh' \ + ':(exclude)dev/pr-check-metrics.sh' \ ':(exclude)dev/slack-app-vercel-build-report.json' From 74a69b192a7275c543b386375e92ac1d7d3ae66c Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Mon, 21 Sep 2026 18:22:41 -0600 Subject: [PATCH 2/2] PR checks: count distinct findings across revisions, not the peak count The status comment keeps a hidden list of every finding identity it has seen (the checks write them with --keys, without line numbers), so Total findings is the distinct findings any revision had and Remediated is how many of those are gone, even when a revision fixes some and adds others. Amp-Thread-ID: https://ampcode.com/threads/T-01a0c49e-51da-72ca-8a4a-6c0660c5e505 Co-authored-by: Amp --- .github/workflows/check-links.yml | 5 +-- .github/workflows/check-redirects.yml | 5 +-- .github/workflows/spellcheck.yml | 7 ++-- AGENTS.md | 3 ++ dev/check-links.mjs | 14 ++++++++ dev/check-redirects.mjs | 8 ++++- dev/post-spelling-review.mjs | 26 ++++++++++++--- dev/upsert-report-comment.sh | 48 ++++++++++++++------------- 8 files changed, 81 insertions(+), 35 deletions(-) diff --git a/.github/workflows/check-links.yml b/.github/workflows/check-links.yml index 247c26db9..e02429930 100644 --- a/.github/workflows/check-links.yml +++ b/.github/workflows/check-links.yml @@ -67,6 +67,7 @@ jobs: --baseline "$RUNNER_TEMP/base-links.json" \ --diff "$RUNNER_TEMP/changes.diff" \ --review "$RUNNER_TEMP/review.json" \ + --keys "$RUNNER_TEMP/keys.txt" \ --link-base "$LINK_BASE" > "$RUNNER_TEMP/report.md"; then echo "result=clean" >> "$GITHUB_OUTPUT" elif [ -s "$RUNNER_TEMP/report.md" ]; then @@ -85,14 +86,14 @@ jobs: PR_NUMBER: ${{ github.event.pull_request.number }} RESULT: ${{ steps.check.outputs.result }} RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} - # The status comment keeps a running total; see dev/upsert-report-comment.sh + # The status comment keeps a findings/remediated table; see dev/upsert-report-comment.sh run: | if [ "$RESULT" = crashed ]; then printf '### ⚠️ The links check could not run on this revision\n\nThis is a problem with the check, not with this PR; see the [job log](%s).\n' \ "$RUN_URL" > "$RUNNER_TEMP/report.md" dev/upsert-report-comment.sh '' "$RUNNER_TEMP/report.md" --crashed else - dev/upsert-report-comment.sh '' "$RUNNER_TEMP/report.md" + dev/upsert-report-comment.sh '' "$RUNNER_TEMP/report.md" "$RUNNER_TEMP/keys.txt" fi - name: Suggest fixes as review comments diff --git a/.github/workflows/check-redirects.yml b/.github/workflows/check-redirects.yml index 99d476382..0aea9dac2 100644 --- a/.github/workflows/check-redirects.yml +++ b/.github/workflows/check-redirects.yml @@ -69,6 +69,7 @@ jobs: --baseline "$RUNNER_TEMP/base-redirects.json" \ --diff "$RUNNER_TEMP/changes.diff" \ --review "$RUNNER_TEMP/review.json" \ + --keys "$RUNNER_TEMP/keys.txt" \ --link-base "$LINK_BASE" > "$RUNNER_TEMP/report.md"; then echo "result=clean" >> "$GITHUB_OUTPUT" elif [ -s "$RUNNER_TEMP/report.md" ]; then @@ -87,14 +88,14 @@ jobs: PR_NUMBER: ${{ github.event.pull_request.number }} RESULT: ${{ steps.check.outputs.result }} RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} - # The status comment keeps a running total; see dev/upsert-report-comment.sh + # The status comment keeps a findings/remediated table; see dev/upsert-report-comment.sh run: | if [ "$RESULT" = crashed ]; then printf '### ⚠️ The redirects check could not run on this revision\n\nThis is a problem with the check, not with this PR; see the [job log](%s).\n' \ "$RUN_URL" > "$RUNNER_TEMP/report.md" dev/upsert-report-comment.sh '' "$RUNNER_TEMP/report.md" --crashed else - dev/upsert-report-comment.sh '' "$RUNNER_TEMP/report.md" + dev/upsert-report-comment.sh '' "$RUNNER_TEMP/report.md" "$RUNNER_TEMP/keys.txt" fi - name: Suggest fixes as review comments diff --git a/.github/workflows/spellcheck.yml b/.github/workflows/spellcheck.yml index 606c27c8e..c07ad3684 100644 --- a/.github/workflows/spellcheck.yml +++ b/.github/workflows/spellcheck.yml @@ -71,13 +71,14 @@ jobs: HEAD_REF: ${{ github.event.pull_request.head.ref }} RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} CRASHED: ${{ steps.check.outputs.crashed }} - # The status comment keeps a running total; see dev/upsert-report-comment.sh + # The status comment keeps a findings/remediated table; see dev/upsert-report-comment.sh run: | if [ "$CRASHED" = true ]; then printf '### ⚠️ The spell check could not run on this revision\n\nThis is a problem with the check, not with this PR; see the [job log](%s).\n' \ "$RUN_URL" > "$RUNNER_TEMP/summary.md" dev/upsert-report-comment.sh '' "$RUNNER_TEMP/summary.md" --crashed else - node dev/post-spelling-review.mjs --findings "$RUNNER_TEMP/spelling.json" --summary "$RUNNER_TEMP/summary.md" - dev/upsert-report-comment.sh '' "$RUNNER_TEMP/summary.md" + node dev/post-spelling-review.mjs --findings "$RUNNER_TEMP/spelling.json" \ + --summary "$RUNNER_TEMP/summary.md" --keys "$RUNNER_TEMP/keys.txt" + dev/upsert-report-comment.sh '' "$RUNNER_TEMP/summary.md" "$RUNNER_TEMP/keys.txt" fi diff --git a/AGENTS.md b/AGENTS.md index 2e8758739..2406d7168 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -77,6 +77,9 @@ Fork PRs get no comments; the findings are in the job log next run: inline comments are deleted, and the summary comment is updated in place (`dev/upsert-report-comment.sh`) with a `Total findings` / `Remediated` table, so a merged PR's comment says how many findings it was merged with. +The table counts distinct findings across every revision (the same broken +link or misspelled word on several lines of one file is one finding), so it +can differ from the count in the heading. `dev/pr-check-metrics.sh [merged-since]` sums those tables over merged PRs. ### Vercel build failures diff --git a/dev/check-links.mjs b/dev/check-links.mjs index 83156af84..bda27cd28 100644 --- a/dev/check-links.mjs +++ b/dev/check-links.mjs @@ -39,6 +39,8 @@ * --review Write a GitHub pull request review (JSON body for * POST /repos/{owner}/{repo}/pulls/{n}/reviews) with one * suggested-change comment per added line that has a fix + * --keys Write the findings' identities, one per line, for + * dev/upsert-report-comment.sh to count across revisions * * Exits 1 when any finding is reported. */ @@ -62,6 +64,7 @@ const LINK_BASE = flagValue('--link-base')?.replace(/\/$/, ''); const DIFF = parseDiff(flagValue('--diff')); const CHECK_EXTERNAL = args.includes('--check-external'); const REVIEW_FILE = flagValue('--review'); +const KEYS_FILE = flagValue('--keys'); if (CHECK_EXTERNAL && !DIFF) { throw new Error('--check-external needs --diff, to know which lines were added'); @@ -516,6 +519,14 @@ function findingKey({ file, url, error }) { return `${file}\n${url}\n${error}`; } +// The --keys file: one JSON string per line, so a key can hold any character, +// with `>` escaped since the lines end up inside an HTML comment on the PR +export function findingKeyLines(keys) { + return [...new Set(keys)] + .map(key => JSON.stringify(key).replaceAll('>', '\\u003e') + '\n') + .join(''); +} + function withoutBaseline(findings, baselineFile) { const baseline = new Set( JSON.parse(fs.readFileSync(baselineFile, 'utf-8')).map(findingKey) @@ -679,6 +690,9 @@ async function main() { if (REVIEW_FILE) { fs.writeFileSync(REVIEW_FILE, JSON.stringify(reviewRequest(findings), null, '\t') + '\n'); } + if (KEYS_FILE) { + fs.writeFileSync(KEYS_FILE, findingKeyLines(findings.map(findingKey))); + } process.stdout.write(format(findings)); process.exit(findings.length === 0 ? 0 : 1); } diff --git a/dev/check-redirects.mjs b/dev/check-redirects.mjs index f84d0e191..18981f99c 100644 --- a/dev/check-redirects.mjs +++ b/dev/check-redirects.mjs @@ -30,6 +30,8 @@ * diff added get a suggested change * --review Write a pull request review with one suggested * change per fixable finding to this JSON file + * --keys Write the findings' identities, one per line, for + * dev/upsert-report-comment.sh to count across revisions * * Exits 1 when any finding is reported. */ @@ -38,7 +40,7 @@ import fs from 'fs'; import path from 'path'; import vm from 'vm'; import {fileURLToPath} from 'url'; -import {extractHeadings, listFiles, routeFor} from './check-links.mjs'; +import {extractHeadings, findingKeyLines, listFiles, routeFor} from './check-links.mjs'; const __dirname = path.dirname(fileURLToPath(import.meta.url)); @@ -49,6 +51,7 @@ const BASELINE_FILE = flagValue('--baseline'); const LINK_BASE = flagValue('--link-base')?.replace(/\/$/, ''); const DIFF_FILE = flagValue('--diff'); const REVIEW_FILE = flagValue('--review'); +const KEYS_FILE = flagValue('--keys'); const REDIRECTS_PATH = 'src/data/redirects.ts'; const REDIRECTS_FILE = path.join(ROOT_DIR, REDIRECTS_PATH); @@ -490,6 +493,9 @@ function main() { JSON.stringify(reviewRequest(findings, added), null, '\t') + '\n' ); } + if (KEYS_FILE) { + fs.writeFileSync(KEYS_FILE, findingKeyLines(findings.map(findingKey))); + } // Not process.exit(): that can truncate stdout when it is a pipe process.stdout.write(format(findings)); diff --git a/dev/post-spelling-review.mjs b/dev/post-spelling-review.mjs index 93c133c27..ec95a0c04 100644 --- a/dev/post-spelling-review.mjs +++ b/dev/post-spelling-review.mjs @@ -9,9 +9,13 @@ * comments are deleted; the review that carried them has no body, so nothing * of it remains visible. * - * Usage: node dev/post-spelling-review.mjs --findings --summary [--dry-run] + * Usage: node dev/post-spelling-review.mjs --findings \ + * --summary --keys [--dry-run] * - * Reads the JSON written by `dev/check-spelling.mjs --format json`. + * Reads the JSON written by `dev/check-spelling.mjs --format json`. Writes + * the summary Markdown and, to the keys file, one identity per finding (file + * and word, since line numbers shift between revisions) for + * dev/upsert-report-comment.sh to count across revisions. * Requires GH_TOKEN, GITHUB_REPOSITORY, PR_NUMBER, HEAD_SHA and HEAD_REF. */ @@ -20,6 +24,7 @@ import {readFileSync, writeFileSync} from 'fs'; const args = process.argv.slice(2); const FINDINGS_FILE = args[args.indexOf('--findings') + 1]; const SUMMARY_FILE = args[args.indexOf('--summary') + 1]; +const KEYS_FILE = args[args.indexOf('--keys') + 1]; const DRY_RUN = args.includes('--dry-run'); const MAX_INLINE_COMMENTS = 25; @@ -152,6 +157,16 @@ function summaryBody(findings) { return lines.join('\n') + '\n'; } +// One JSON string per line, as dev/check-links.mjs writes its --keys file: +// any character fits, and `>` is escaped since the lines end up inside an +// HTML comment on the PR. Not imported from there, since this job does not +// install its github-slugger dependency +function findingKeyLines(findings) { + return [...new Set(findings.map(({file, word}) => `${file}\u0000${word}`))] + .map(key => JSON.stringify(key).replaceAll('>', '\\u003e') + '\n') + .join(''); +} + function findingKey({file, line, word}) { return `${file}:${line}:${word}`; } @@ -273,13 +288,16 @@ async function main() { throw new Error(`Missing required environment variable ${name}`); } } - if (!FINDINGS_FILE || !SUMMARY_FILE) { - throw new Error('Missing required --findings --summary '); + if (!FINDINGS_FILE || !SUMMARY_FILE || !KEYS_FILE) { + throw new Error( + 'Missing required --findings --summary --keys ' + ); } const findings = JSON.parse(readFileSync(FINDINGS_FILE, 'utf8')); console.log(`${findings.length} finding(s) to report`); writeFileSync(SUMMARY_FILE, summaryBody(findings)); + writeFileSync(KEYS_FILE, findingKeyLines(findings)); await syncInlineComments(findings); } diff --git a/dev/upsert-report-comment.sh b/dev/upsert-report-comment.sh index 6b4f6ca4c..3ac37454b 100755 --- a/dev/upsert-report-comment.sh +++ b/dev/upsert-report-comment.sh @@ -8,43 +8,42 @@ # | --- | --- | # | 12 | 8 | # -# Total findings is the most any revision of the PR had; Remediated is how many -# of those the current revision no longer has, so a merged PR's comment says -# how many it was merged with. The report's first line must contain the -# current count as its first integer, or no integer when there are none. +# Total findings counts every distinct finding any revision of the PR had; +# Remediated is how many of those the current revision no longer has, so a +# merged PR's comment says how many it was merged with. To tell a fixed finding +# from a new one, the comment ends with a hidden list of every finding seen so +# far, in the form the check writes to its --keys file (one identity per line, +# without line numbers, so the same problem on several lines of a file is one +# finding, and the table can differ from the count in the heading). # # Nothing is posted for a PR that never had findings, to spare its author the # notification; updating an existing comment sends none. # -# Usage: dev/upsert-report-comment.sh '' report.md +# Usage: dev/upsert-report-comment.sh '' report.md keys.txt # dev/upsert-report-comment.sh '' notice.md --crashed # -# With --crashed, the report says the check could not run; the table carries -# over from the previous comment, if there was one. +# With --crashed, the report says the check could not run; the table and the +# list carry over from the previous comment, if there was one. # Needs GH_TOKEN, GITHUB_REPOSITORY, and PR_NUMBER. set -eu marker=$1 report=$2 -crashed=${3:-} +keys=$3 comments="repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" existing=$(gh api "$comments" --paginate \ --jq ".[] | select(.body | startswith(\"$marker\")) | {id, body}" | jq -c . | head -n 1) -# "total remediated" from the previous comment's table row, or "0 0" -previous=$(printf '%s' "$existing" | jq -r '.body // ""' \ - | sed -n 's/^| \([0-9]*\) | \([0-9]*\) |$/\1 \2/p' | head -n 1) -previous=${previous:-0 0} -previous_total=${previous%% *} -previous_remediated=${previous##* } +previous_body=$(printf '%s' "$existing" | jq -r '.body // ""') +previous_seen=$(printf '%s\n' "$previous_body" | sed -n '/^$/p' | sed '1d;$d') -if [ "$crashed" = --crashed ]; then - total=$previous_total - remediated=$previous_remediated +if [ "$keys" = --crashed ]; then + seen=$previous_seen + table=$(printf '%s\n' "$previous_body" | sed -n 's/^| \([0-9]*\) | \([0-9]*\) |$/\1 \2/p' | head -n 1) else - current=$(head -n 1 "$report" | sed -n 's/^[^0-9]*\([0-9][0-9]*\).*/\1/p') - current=${current:-0} - total=$(( current > previous_total ? current : previous_total )) - remediated=$(( total - current )) + seen=$({ printf '%s\n' "$previous_seen"; cat "$keys"; } | grep -v '^$' | sort -u || true) + total=$(printf '%s\n' "$seen" | grep -c . || true) + current=$(sort -u "$keys" | grep -c . || true) + table="$total $((total - current))" [ -n "$existing" ] || [ "$total" -gt 0 ] || exit 0 fi @@ -52,10 +51,13 @@ body=$(mktemp) { echo "$marker" head -n 1 "$report" - if [ -n "$existing" ] || [ "$crashed" != --crashed ]; then - printf '\n| Total findings | Remediated |\n| --- | --- |\n| %s | %s |\n' "$total" "$remediated" + if [ -n "$table" ]; then + printf '\n| Total findings | Remediated |\n| --- | --- |\n| %s | %s |\n' "${table% *}" "${table#* }" fi tail -n +2 "$report" + if [ -n "$seen" ]; then + printf '\n' "$seen" + fi } > "$body" if [ -n "$existing" ]; then