diff --git a/.github/workflows/check-links.yml b/.github/workflows/check-links.yml index 78629e1fc..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,29 +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 findings/remediated table; 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" "$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 ce4658f95..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,29 +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 findings/remediated table; 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" "$RUNNER_TEMP/keys.txt" fi - name: Suggest fixes as review comments diff --git a/.github/workflows/spellcheck.yml b/.github/workflows/spellcheck.yml index 51f98bae8..c07ad3684 100644 --- a/.github/workflows/spellcheck.yml +++ b/.github/workflows/spellcheck.yml @@ -71,9 +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 findings/remediated table; 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" --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 8ae767663..2406d7168 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -74,7 +74,13 @@ 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. +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 15d30044c..ec95a0c04 100644 --- a/dev/post-spelling-review.mjs +++ b/dev/post-spelling-review.mjs @@ -1,28 +1,30 @@ #!/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 --keys [--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`. 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. */ -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 KEYS_FILE = args[args.indexOf('--keys') + 1]; const DRY_RUN = args.includes('--dry-run'); const MAX_INLINE_COMMENTS = 25; @@ -31,12 +33,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..3ac37454b --- /dev/null +++ b/dev/upsert-report-comment.sh @@ -0,0 +1,68 @@ +#!/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 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 keys.txt +# dev/upsert-report-comment.sh '' notice.md --crashed +# +# 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 +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) +previous_body=$(printf '%s' "$existing" | jq -r '.body // ""') +previous_seen=$(printf '%s\n' "$previous_body" | sed -n '/^$/p' | sed '1d;$d') + +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 + 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 + +body=$(mktemp) +{ + echo "$marker" + head -n 1 "$report" + 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 + 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'