From 69fed7f91007fbc2f86f58522feb79a21726b0d7 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Wed, 16 Sep 2026 23:30:04 -0600 Subject: [PATCH 1/6] ci: Do not fail or comment when a PR check crashes The redirects check on #1982 posted a comment containing only its hidden marker, then failed. The PR changed src/data/redirects.ts from module.exports to export {}, and its copy of dev/check-redirects.mjs could no longer read the base branch's file. The workflow could not tell that crash (Node exits 1) from findings (also exit 1), so it treated an empty report as a finding. - check-redirects.mjs accepts both export styles, since CI runs the PR's script against the base branch too - Both check workflows classify by output: the scripts write their result in one go at the end, so a crash leaves it empty. A crash is a workflow warning; the comment, review and fail steps only run on a real result - The comment step never posts a report it does not have Amp-Thread-ID: https://ampcode.com/threads/T-01a0adcb-1b03-716a-852a-cc3746148488 Co-authored-by: Amp --- .github/workflows/check-links.yml | 34 ++++++++++++++++++-------- .github/workflows/check-redirects.yml | 35 +++++++++++++++++++-------- AGENTS.md | 4 ++- dev/check-redirects.mjs | 3 +++ 4 files changed, 55 insertions(+), 21 deletions(-) diff --git a/.github/workflows/check-links.yml b/.github/workflows/check-links.yml index c5e4d7503..33b203006 100644 --- a/.github/workflows/check-links.yml +++ b/.github/workflows/check-links.yml @@ -43,15 +43,23 @@ jobs: git worktree add "$RUNNER_TEMP/base" "$merge_base" git diff -U0 "$merge_base" HEAD > "$RUNNER_TEMP/changes.diff" + # The script exits 1 both with findings and when it crashes. It writes + # its output in one go at the end, so a crash leaves it empty. A crash + # is a warning, not a failed check: it says nothing about the PR, and + # an empty report must not be posted as one. - name: Record broken links already present on the base branch - # Exit 1 means findings, which is expected here + id: baseline run: | node dev/check-links.mjs --check-anchors --check-self-links --format json \ - --root "$RUNNER_TEMP/base" > "$RUNNER_TEMP/base-links.json" \ - || [ $? -eq 1 ] + --root "$RUNNER_TEMP/base" > "$RUNNER_TEMP/base-links.json" || true + if ! [ -s "$RUNNER_TEMP/base-links.json" ]; then + echo "::warning::check-links crashed on the base branch, so this PR was not checked" + echo "crashed=true" >> "$GITHUB_OUTPUT" + fi - name: Find broken links introduced by this PR id: check + if: steps.baseline.outputs.crashed != 'true' env: # File links in the report open the file on the PR branch LINK_BASE: ${{ github.event.pull_request.head.repo.html_url }}/blob/${{ github.event.pull_request.head.ref }} @@ -61,26 +69,30 @@ jobs: --diff "$RUNNER_TEMP/changes.diff" \ --review "$RUNNER_TEMP/review.json" \ --link-base "$LINK_BASE" > "$RUNNER_TEMP/report.md"; then - echo "broken=false" >> "$GITHUB_OUTPUT" + echo "result=clean" >> "$GITHUB_OUTPUT" + elif [ -s "$RUNNER_TEMP/report.md" ]; then + echo "result=broken" >> "$GITHUB_OUTPUT" else - echo "broken=true" >> "$GITHUB_OUTPUT" + echo "::warning::check-links crashed, so this PR was not checked" fi cat "$RUNNER_TEMP/report.md" - name: Comment on the pull request # Fork PRs get a read-only token; the report is still in the job log - if: github.event.pull_request.head.repo.full_name == github.repository + if: >- + github.event.pull_request.head.repo.full_name == github.repository + && contains(fromJSON('["clean", "broken"]'), steps.check.outputs.result) env: GH_TOKEN: ${{ github.token }} PR_NUMBER: ${{ github.event.pull_request.number }} - BROKEN: ${{ steps.check.outputs.broken }} + RESULT: ${{ steps.check.outputs.result }} run: | marker='' existing_comment=$(gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" \ --paginate --jq ".[] | select(.body | startswith(\"$marker\")) | .id" | head -n 1) # Comment only when there is something to report, or an earlier report to resolve - if [ "$BROKEN" = true ]; then + if [ "$RESULT" = broken ]; then { echo "$marker"; cat "$RUNNER_TEMP/report.md"; } > "$RUNNER_TEMP/comment.md" elif [ -n "$existing_comment" ]; then printf '%s\n### ✅ The broken links an earlier revision of this PR introduced are fixed\n' \ @@ -99,12 +111,14 @@ jobs: - name: Suggest fixes as review comments # One suggested change per finding with a fix, kept in sync with the # findings; see dev/sync-review-comments.sh - if: github.event.pull_request.head.repo.full_name == github.repository + if: >- + github.event.pull_request.head.repo.full_name == github.repository + && contains(fromJSON('["clean", "broken"]'), steps.check.outputs.result) env: GH_TOKEN: ${{ github.token }} PR_NUMBER: ${{ github.event.pull_request.number }} run: dev/sync-review-comments.sh '' existing_comment=$(gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" \ --paginate --jq ".[] | select(.body | startswith(\"$marker\")) | .id" | head -n 1) # Comment only when there is something to report, or an earlier report to resolve - if [ "$BROKEN" = true ]; then + if [ "$RESULT" = broken ]; then { echo "$marker"; cat "$RUNNER_TEMP/report.md"; } > "$RUNNER_TEMP/comment.md" elif [ -n "$existing_comment" ]; then printf '%s\n### ✅ The redirects an earlier revision of this PR broke are fixed\n' \ @@ -100,12 +113,14 @@ jobs: - name: Suggest fixes as review comments # One suggested change per fixable entry this PR added, kept in sync # with the findings; see dev/sync-review-comments.sh - if: github.event.pull_request.head.repo.full_name == github.repository + if: >- + github.event.pull_request.head.repo.full_name == github.repository + && contains(fromJSON('["clean", "broken"]'), steps.check.outputs.result) env: GH_TOKEN: ${{ github.token }} PR_NUMBER: ${{ github.event.pull_request.number }} run: dev/sync-review-comments.sh '' existing_comment=$(gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" \ --paginate --jq ".[] | select(.body | startswith(\"$marker\")) | .id" | head -n 1) - # Comment only when there is something to report, or an earlier report to resolve - if [ "$RESULT" = broken ]; then - { echo "$marker"; cat "$RUNNER_TEMP/report.md"; } > "$RUNNER_TEMP/comment.md" - elif [ -n "$existing_comment" ]; then - printf '%s\n### ✅ The broken links an earlier revision of this PR introduced are fixed\n' \ - "$marker" > "$RUNNER_TEMP/comment.md" - else - exit 0 - fi + # 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" \ diff --git a/.github/workflows/check-redirects.yml b/.github/workflows/check-redirects.yml index c26af5258..ce4658f95 100644 --- a/.github/workflows/check-redirects.yml +++ b/.github/workflows/check-redirects.yml @@ -47,25 +47,24 @@ jobs: # The script exits 1 both with findings and when it crashes, e.g. because # this PR's copy of it cannot read the base branch's redirects.ts. It # writes its output in one go at the end, so a crash leaves it empty. - # A crash is a warning, not a failed check: it says nothing about the - # PR, and an empty report must not be posted as one. + # A crash does not fail the check, since it says nothing about the PR; + # the comment step reports it instead of posting the empty report. - name: Record broken redirects already present on the base branch - id: baseline run: | node dev/check-redirects.mjs --format json \ --root "$RUNNER_TEMP/base" > "$RUNNER_TEMP/base-redirects.json" || true - if ! [ -s "$RUNNER_TEMP/base-redirects.json" ]; then - echo "::warning::check-redirects crashed on the base branch, so this PR was not checked" - echo "crashed=true" >> "$GITHUB_OUTPUT" - fi - name: Find redirects broken by this PR id: check - if: steps.baseline.outputs.crashed != 'true' env: # Line links in the report open redirects.ts on the PR branch LINK_BASE: ${{ github.event.pull_request.head.repo.html_url }}/blob/${{ github.event.pull_request.head.ref }} run: | + if ! [ -s "$RUNNER_TEMP/base-redirects.json" ]; then + echo "::warning::check-redirects crashed on the base branch, so this PR was not checked" + echo "result=crashed" >> "$GITHUB_OUTPUT" + exit 0 + fi if node dev/check-redirects.mjs --format markdown \ --baseline "$RUNNER_TEMP/base-redirects.json" \ --diff "$RUNNER_TEMP/changes.diff" \ @@ -76,32 +75,35 @@ jobs: echo "result=broken" >> "$GITHUB_OUTPUT" else echo "::warning::check-redirects crashed, so this PR was not checked" + echo "result=crashed" >> "$GITHUB_OUTPUT" fi cat "$RUNNER_TEMP/report.md" - name: Comment on the pull request # Fork PRs get a read-only token; the report is still in the job log - if: >- - github.event.pull_request.head.repo.full_name == github.repository - && contains(fromJSON('["clean", "broken"]'), steps.check.outputs.result) + if: github.event.pull_request.head.repo.full_name == github.repository env: GH_TOKEN: ${{ github.token }} PR_NUMBER: ${{ github.event.pull_request.number }} RESULT: ${{ steps.check.outputs.result }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} run: | marker='' existing_comment=$(gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" \ --paginate --jq ".[] | select(.body | startswith(\"$marker\")) | .id" | head -n 1) - # Comment only when there is something to report, or an earlier report to resolve - if [ "$RESULT" = broken ]; then - { echo "$marker"; cat "$RUNNER_TEMP/report.md"; } > "$RUNNER_TEMP/comment.md" - elif [ -n "$existing_comment" ]; then - printf '%s\n### ✅ The redirects an earlier revision of this PR broke are fixed\n' \ - "$marker" > "$RUNNER_TEMP/comment.md" - else - exit 0 - fi + # 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" \ diff --git a/.github/workflows/spellcheck.yml b/.github/workflows/spellcheck.yml index b3daf56d0..51f98bae8 100644 --- a/.github/workflows/spellcheck.yml +++ b/.github/workflows/spellcheck.yml @@ -49,26 +49,31 @@ jobs: printf '%s\n\n%s\n' "$PR_TITLE" "$PR_BODY" > "$RUNNER_TEMP/pull-request.md" # The script exits 1 both with findings and when it crashes at import # time. It writes its output in one go at the end, so a crash leaves - # it empty. A crash is a warning, not a failed check: it says nothing - # about the PR, and an empty report must not be posted as one + # it empty. A crash does not fail the check, since it says nothing + # about the PR; the report step says so instead of posting the + # empty report node dev/check-spelling.mjs --base "$base" \ --pull-request "$RUNNER_TEMP/pull-request.md" --format json \ > "$RUNNER_TEMP/spelling.json" || true - if [ -s "$RUNNER_TEMP/spelling.json" ]; then - echo "checked=true" >> "$GITHUB_OUTPUT" - else + if ! [ -s "$RUNNER_TEMP/spelling.json" ]; then echo "::warning::check-spelling crashed, so this PR was not checked" + echo "crashed=true" >> "$GITHUB_OUTPUT" fi cat "$RUNNER_TEMP/spelling.json" - name: Report on the pull request # Fork PRs get a read-only token; the findings are still in the job log - if: >- - github.event.pull_request.head.repo.full_name == github.repository - && steps.check.outputs.checked == 'true' + if: github.event.pull_request.head.repo.full_name == github.repository env: GH_TOKEN: ${{ github.token }} PR_NUMBER: ${{ github.event.pull_request.number }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} HEAD_REF: ${{ github.event.pull_request.head.ref }} - run: node dev/post-spelling-review.mjs --findings "$RUNNER_TEMP/spelling.json" + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + CRASHED: ${{ steps.check.outputs.crashed }} + run: | + if [ "$CRASHED" = true ]; then + node dev/post-spelling-review.mjs --crashed + else + node dev/post-spelling-review.mjs --findings "$RUNNER_TEMP/spelling.json" + fi diff --git a/AGENTS.md b/AGENTS.md index 4015e4ddc..c2fd71c5a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -22,8 +22,9 @@ `src/data/redirects.ts` (CI comments on PRs that break redirects; see the script header for what it checks). Not part of `pnpm run check`: main has hundreds of pre-existing findings, and CI only reports the ones a PR adds. - When a check script crashes, CI warns and skips the check instead of failing - it or commenting: an empty report says nothing about the PR + When a check script crashes, CI comments that the check could not run, + linking the job log, instead of failing the PR: an empty report says nothing + about the PR - **Prove changed links resolve on a deploy**: `node dev/verify-links-live.mjs --site ` prints a Markdown table for the PR description diff --git a/dev/post-spelling-review.mjs b/dev/post-spelling-review.mjs index 1ec455175..15d30044c 100644 --- a/dev/post-spelling-review.mjs +++ b/dev/post-spelling-review.mjs @@ -10,8 +10,11 @@ * visible. * * Usage: node dev/post-spelling-review.mjs --findings [--dry-run] + * node dev/post-spelling-review.mjs --crashed [--dry-run] * - * Reads the JSON written by `dev/check-spelling.mjs --format json`. + * 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. * Requires GH_TOKEN, GITHUB_REPOSITORY, PR_NUMBER, HEAD_SHA and HEAD_REF. */ @@ -19,6 +22,7 @@ import {readFileSync} from 'fs'; const args = process.argv.slice(2); const FINDINGS_FILE = args[args.indexOf('--findings') + 1]; +const CRASHED = args.includes('--crashed'); const DRY_RUN = args.includes('--dry-run'); const MAX_INLINE_COMMENTS = 25; @@ -27,6 +31,7 @@ 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)`; @@ -168,7 +173,22 @@ function summaryBody(findings) { return lines.join('\n') + '\n'; } -async function upsertSummaryComment(findings) { +function crashedBody() { + return [ + SUMMARY_MARKER, + '### ⚠️ The spell check could not run on this revision', + '', + `This is a problem with the check, not with this PR; see the [job log](${RUN_URL}).`, + '' + ].join('\n'); +} + +const RESOLVED_BODY = `${SUMMARY_MARKER}\n### ✅ This revision introduces no spelling errors\n`; + +// Post the summary, or replace the earlier one. A resolved summary is kept, +// collapsed, so the discussion still shows what was flagged and fixed; it is +// reopened when there is something to say again. +async function upsertSummaryComment(body, resolved) { const comments = await githubList( `/repos/${REPOSITORY}/issues/${PR_NUMBER}/comments` ); @@ -177,27 +197,21 @@ async function upsertSummaryComment(findings) { ); if (!existing) { - if (findings.length > 0) { + // Nothing to resolve + if (!resolved) { await githubWrite( 'POST', `/repos/${REPOSITORY}/issues/${PR_NUMBER}/comments`, - {body: summaryBody(findings)} + {body} ); } return; } - // Keep the earlier report, collapsed as resolved, so the discussion still - // shows what was flagged and fixed. Reopen it when new findings appear. - const resolved = findings.length === 0; await githubWrite( 'PATCH', `/repos/${REPOSITORY}/issues/comments/${existing.id}`, - { - body: resolved - ? `${SUMMARY_MARKER}\n### ✅ The spelling errors reported on an earlier revision are fixed\n` - : summaryBody(findings) - } + {body} ); await setCommentMinimized(existing.node_id, resolved); } @@ -323,13 +337,25 @@ async function main() { throw new Error(`Missing required environment variable ${name}`); } } + if (CRASHED) { + if (!RUN_URL) { + throw new Error('Missing required environment variable RUN_URL'); + } + console.log('Reporting that the check crashed'); + await upsertSummaryComment(crashedBody(), false); + return; + } if (!FINDINGS_FILE) { throw new Error('Missing required --findings '); } const findings = JSON.parse(readFileSync(FINDINGS_FILE, 'utf8')); console.log(`${findings.length} finding(s) to report`); - await upsertSummaryComment(findings); + const resolved = findings.length === 0; + await upsertSummaryComment( + resolved ? RESOLVED_BODY : summaryBody(findings), + resolved + ); await syncInlineComments(findings); } From bb43d84c03a6f6ddac001a0666ec5786b6902575 Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Thu, 17 Sep 2026 00:11:55 -0600 Subject: [PATCH 4/6] DEMO: crash every check script at import (revert me) Amp-Thread-ID: https://ampcode.com/threads/T-01a0adcb-1b03-716a-852a-cc3746148488 Co-authored-by: Amp --- dev/check-links.mjs | 1 + dev/check-redirects.mjs | 1 + dev/check-spelling.mjs | 1 + 3 files changed, 3 insertions(+) diff --git a/dev/check-links.mjs b/dev/check-links.mjs index f9fd80857..a50ba00a2 100644 --- a/dev/check-links.mjs +++ b/dev/check-links.mjs @@ -1,4 +1,5 @@ #!/usr/bin/env node +import 'nope-this-module-does-not-exist'; /** * Dead link checker for MDX documentation files. diff --git a/dev/check-redirects.mjs b/dev/check-redirects.mjs index f84d0e191..282e53a62 100644 --- a/dev/check-redirects.mjs +++ b/dev/check-redirects.mjs @@ -1,4 +1,5 @@ #!/usr/bin/env node +import 'nope-this-module-does-not-exist'; /** * Checks redirects in src/data/redirects.ts. diff --git a/dev/check-spelling.mjs b/dev/check-spelling.mjs index 23775737f..8b3cbf37c 100644 --- a/dev/check-spelling.mjs +++ b/dev/check-spelling.mjs @@ -1,4 +1,5 @@ #!/usr/bin/env node +import 'nope-this-module-does-not-exist'; /** * Reports CSpell findings on lines added by a Git diff, and dictionary entries From 5e3ef8b6588e9d537a0ff9bb8b302aa32e3bc6ef Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Thu, 17 Sep 2026 00:14:04 -0600 Subject: [PATCH 5/6] Revert "DEMO: crash every check script at import (revert me)" This reverts commit d30e9f2f03ac98f88829c8ceace6e7f845a617f1. --- dev/check-links.mjs | 1 - dev/check-redirects.mjs | 1 - dev/check-spelling.mjs | 1 - 3 files changed, 3 deletions(-) diff --git a/dev/check-links.mjs b/dev/check-links.mjs index a50ba00a2..f9fd80857 100644 --- a/dev/check-links.mjs +++ b/dev/check-links.mjs @@ -1,5 +1,4 @@ #!/usr/bin/env node -import 'nope-this-module-does-not-exist'; /** * Dead link checker for MDX documentation files. diff --git a/dev/check-redirects.mjs b/dev/check-redirects.mjs index 282e53a62..f84d0e191 100644 --- a/dev/check-redirects.mjs +++ b/dev/check-redirects.mjs @@ -1,5 +1,4 @@ #!/usr/bin/env node -import 'nope-this-module-does-not-exist'; /** * Checks redirects in src/data/redirects.ts. diff --git a/dev/check-spelling.mjs b/dev/check-spelling.mjs index 8b3cbf37c..23775737f 100644 --- a/dev/check-spelling.mjs +++ b/dev/check-spelling.mjs @@ -1,5 +1,4 @@ #!/usr/bin/env node -import 'nope-this-module-does-not-exist'; /** * Reports CSpell findings on lines added by a Git diff, and dictionary entries From b714b50aa1f64821be5518ba351cba7398d4339e Mon Sep 17 00:00:00 2001 From: Marc LeBlanc <7050295+marcleblanc2@users.noreply.github.com> Date: Thu, 17 Sep 2026 00:14:14 -0600 Subject: [PATCH 6/6] cspell: Allow esac, the shell keyword Amp-Thread-ID: https://ampcode.com/threads/T-01a0adcb-1b03-716a-852a-cc3746148488 Co-authored-by: Amp --- cspell-allow-list.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/cspell-allow-list.txt b/cspell-allow-list.txt index c8d8eac88..13256d9ac 100644 --- a/cspell-allow-list.txt +++ b/cspell-allow-list.txt @@ -190,6 +190,7 @@ entrycommand envsubst errgroup errorf +esac esbenp Eswatini etcdctl