Skip to content

Commit fed1673

Browse files
ci/redirects: Add Redirect check for PRs (#1935)
Linear [FE-499: Fix doc site issues](https://linear.app/sourcegraph/issue/FE-499/fix-doc-site-issues) ## Problem - `src/data/redirects.ts` has ~900 entries that don't work: destination pages that no longer exist, `#fragments` in the source (browsers never send them, so the entry can never match), duplicates of an earlier source, chains to other redirects, `/docs/` prefixes - Nothing checks the file, so PRs keep adding broken redirects, and PRs that move a page or rename a heading silently break the redirects pointing at it ## Solution - `dev/check-redirects.mjs` validates every entry against the pages and headings in `docs/` - "Check redirects" PR check runs the script on the PR head and its merge base, and reports only what the PR introduced; the ~900 pre-existing problems are ignored - Runs in parallel with "Check links" - The comment groups findings by problem, and each section says how to fix its entries, so authors learn how to write a correct redirect: - Source overshadows a docs page that exists - Source has a `#fragment`, so this redirect can never match - Source or destination starts with `/docs` - Source already has an earlier entry, so this one is never used - Destination is another redirect (shows the final destination) - Destination page does not exist - Destination heading does not exist - Line numbers link to the entry in `redirects.ts` on the PR branch - The comment is created / updated in place, and once the PR is fixed, the PR check passes and the comment is updated to say so - A PR that never broke anything gets no comment - `pnpm check-redirects` runs it locally ## Verification PR check comment in test PR: #1896 (comment) <img width="1000" alt="Check redirects PR comment listing 8 broken redirects grouped by problem" src="https://ampcode.com/user-content/artifacts/a074bef7c771368ba56da54a9e62f9a41cef9402a064594646cd3e8b0fa6c166-file.png" /> ## Related - #1858 added the "Check links" PR check this is built on; #1916 added `dev/sync-review-comments.sh`, which this check calls ## Amp threads - [Redirect list cleanup](https://ampcode.com/threads/T-01a07e6f-db41-74af-bbeb-f8952e637289) - [Redirect check PR comment](https://ampcode.com/threads/T-01a088d9-b8fd-76fd-ba93-3a416c5829a3) --------- Co-authored-by: Amp <amp@ampcode.com>
1 parent bd1d86d commit fed1673

4 files changed

Lines changed: 621 additions & 0 deletions

File tree

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
name: Check redirects
2+
3+
# Reports redirects in src/data/redirects.ts that this PR breaks, compared with
4+
# the merge base: destinations that no longer exist, #fragments whose heading
5+
# was renamed, and new redirects that shadow an existing page. Pre-existing
6+
# broken redirects on the base branch are ignored.
7+
8+
on:
9+
pull_request:
10+
11+
# A new push supersedes the run for the previous one
12+
concurrency:
13+
group: ${{ github.workflow }}-${{ github.event.pull_request.number }}
14+
cancel-in-progress: true
15+
16+
permissions:
17+
contents: read
18+
pull-requests: write
19+
20+
jobs:
21+
check-redirects:
22+
name: Broken redirects introduced by this PR
23+
runs-on: ubuntu-latest
24+
steps:
25+
- name: Check out pull request head
26+
uses: actions/checkout@v4
27+
with:
28+
ref: ${{ github.event.pull_request.head.sha }}
29+
fetch-depth: 0
30+
31+
- name: Install github-slugger, the only dependency of dev/check-redirects.mjs
32+
# Into a scratch prefix, not the repo: `npm install <pkg>` next to
33+
# package.json would install every dependency of the site
34+
run: |
35+
npm install --prefix "$RUNNER_TEMP/deps" --no-package-lock --no-audit --no-fund \
36+
"github-slugger@$(node -p 'require("./package.json").dependencies["github-slugger"]')"
37+
ln -s "$RUNNER_TEMP/deps/node_modules" node_modules
38+
39+
- name: Check out merge base
40+
env:
41+
BASE_SHA: ${{ github.event.pull_request.base.sha }}
42+
run: |
43+
merge_base=$(git merge-base "$BASE_SHA" HEAD)
44+
git worktree add "$RUNNER_TEMP/base" "$merge_base"
45+
git diff -U0 "$merge_base" HEAD -- src/data/redirects.ts > "$RUNNER_TEMP/changes.diff"
46+
47+
- name: Record broken redirects already present on the base branch
48+
# Exit 1 means findings, which is expected here
49+
run: |
50+
node dev/check-redirects.mjs --format json \
51+
--root "$RUNNER_TEMP/base" > "$RUNNER_TEMP/base-redirects.json" \
52+
|| [ $? -eq 1 ]
53+
54+
- name: Find redirects broken by this PR
55+
id: check
56+
env:
57+
# Line links in the report open redirects.ts on the PR branch
58+
LINK_BASE: ${{ github.event.pull_request.head.repo.html_url }}/blob/${{ github.event.pull_request.head.ref }}
59+
run: |
60+
if node dev/check-redirects.mjs --format markdown \
61+
--baseline "$RUNNER_TEMP/base-redirects.json" \
62+
--diff "$RUNNER_TEMP/changes.diff" \
63+
--review "$RUNNER_TEMP/review.json" \
64+
--link-base "$LINK_BASE" > "$RUNNER_TEMP/report.md"; then
65+
echo "broken=false" >> "$GITHUB_OUTPUT"
66+
else
67+
echo "broken=true" >> "$GITHUB_OUTPUT"
68+
fi
69+
cat "$RUNNER_TEMP/report.md"
70+
71+
- name: Comment on the pull request
72+
# Fork PRs get a read-only token; the report is still in the job log
73+
if: github.event.pull_request.head.repo.full_name == github.repository
74+
env:
75+
GH_TOKEN: ${{ github.token }}
76+
PR_NUMBER: ${{ github.event.pull_request.number }}
77+
BROKEN: ${{ steps.check.outputs.broken }}
78+
run: |
79+
marker='<!-- check-redirects-report -->'
80+
existing_comment=$(gh api "repos/$GITHUB_REPOSITORY/issues/$PR_NUMBER/comments" \
81+
--paginate --jq ".[] | select(.body | startswith(\"$marker\")) | .id" | head -n 1)
82+
83+
# Comment only when there is something to report, or an earlier report to resolve
84+
if [ "$BROKEN" = true ]; then
85+
{ echo "$marker"; cat "$RUNNER_TEMP/report.md"; } > "$RUNNER_TEMP/comment.md"
86+
elif [ -n "$existing_comment" ]; then
87+
printf '%s\n### ✅ The redirects an earlier revision of this PR broke are fixed\n' \
88+
"$marker" > "$RUNNER_TEMP/comment.md"
89+
else
90+
exit 0
91+
fi
92+
93+
if [ -n "$existing_comment" ]; then
94+
gh api --method PATCH "repos/$GITHUB_REPOSITORY/issues/comments/$existing_comment" \
95+
--field body=@"$RUNNER_TEMP/comment.md"
96+
else
97+
gh pr comment "$PR_NUMBER" --body-file "$RUNNER_TEMP/comment.md"
98+
fi
99+
100+
- name: Suggest fixes as review comments
101+
# One suggested change per fixable entry this PR added, kept in sync
102+
# with the findings; see dev/sync-review-comments.sh
103+
if: github.event.pull_request.head.repo.full_name == github.repository
104+
env:
105+
GH_TOKEN: ${{ github.token }}
106+
PR_NUMBER: ${{ github.event.pull_request.number }}
107+
run: dev/sync-review-comments.sh '<!-- check-redirects-finding:' "$RUNNER_TEMP/review.json"
108+
109+
- name: Fail when this PR breaks redirects
110+
if: steps.check.outputs.broken == 'true'
111+
run: exit 1

‎AGENTS.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
- **Framework**: Next.js 16 with `--webpack` (`next-contentlayer2` has no Turbopack plugin); the request rewrite lives in `src/proxy.ts`; `/api/releases` and `/api/versions` opt into static caching with `export const dynamic = 'force-static'`
1010
- **Checks**: `npm run check` runs every `dev/check-*.mjs` (links, filenames, images); `npm run build` runs them first, so any finding fails a deploy
1111
- **Check links**: `npm run check -- links --check-anchors --check-self-links` (CI comments on PRs that break links; see `dev/check-links.mjs`; the build runs it without flags, so only dead page links fail a deploy). When moving a page or renaming a heading, update every link to it; a redirect in `src/data/redirects.ts` does not satisfy the check. Link to this site with relative paths (`/admin/config/site-config`), never `https://sourcegraph.com/docs/…` or `https://docs.sourcegraph.com/…`. To also probe the external links you added: `npm run check -- links --check-anchors --check-self-links --check-external --diff <(git diff -U0 origin/main)`
12+
- **Check redirects**: `node dev/check-redirects.mjs` reports broken entries in `src/data/redirects.ts` (CI comments on PRs that break redirects; see the script header for what it checks). Not part of `npm run check`: main has hundreds of pre-existing findings, and CI only reports the ones a PR adds
1213
- **Prove changed links resolve on a deploy**: `node dev/verify-links-live.mjs --site <vercel-preview-url>` prints a Markdown table for the PR description
1314
- **Vercel build failures**: Vercel shows build logs only to its team members, so `.github/workflows/vercel-build-report.yml` attaches the log to the Vercel Slack app's "failed to deploy" post in `#alerts-vercel-doc-site` and comments a link to it on the PR (see `dev/report-vercel-build.mjs`). The log itself never goes on the PR, since the repository is public. It reads Vercel with the `VERCEL_TOKEN` repo secret, a token scoped to the `sourcegraph-docs` project that expires 2026-12-10; mint a new one with `POST /v3/user/tokens?teamId=<team>` and `projectId` in the body. Slack needs the `SLACK_BOT_TOKEN` repo secret and `SLACK_CHANNEL_ID` repo variable. The bot is the Slack app in `dev/slack-app-vercel-build-report.json`; to recreate it, paste that manifest at <https://api.slack.com/apps?new_app=1> (From a manifest), install it, copy its Bot User OAuth Token into the secret, and `/invite @Vercel build log` to the channel
1415

0 commit comments

Comments
 (0)