Skip to content

ci/redirects: Add Redirect check for PRs - #1935

Draft
marcleblanc2 wants to merge 3 commits into
mainfrom
marc/check-redirects
Draft

ci/redirects: Add Redirect check for PRs#1935
marcleblanc2 wants to merge 3 commits into
mainfrom
marc/check-redirects

Conversation

@marcleblanc2

@marcleblanc2 marcleblanc2 commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Linear FE-499: Fix doc site issues

Stacked on #1858 (base branch check-links-pr-check); merge that one first.

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)

Check redirects PR comment listing 8 broken redirects grouped by problem

Related

Amp threads

Merge order for the PR-check stack

Trial-merged onto main in this order with no conflicts:

  1. ci: Comment the Vercel build log on PRs whose build fails #1946 Vercel build log comment — independent; first so the other PRs' Vercel failures get a readable log
  2. check-links: one suggestion per fix, synced with findings; one fact per line in reports #1916 check-links report format — adds dev/sync-review-comments.sh, which ci/redirects: Add Redirect check for PRs #1935 calls
  3. ci/redirects: Add Redirect check for PRs #1935 redirect check — needs check-links: one suggestion per fix, synced with findings; one fact per line in reports #1916 merged first
  4. spell check: update inline comments whose text changed #1947 spell check comment updates — independent
  5. check-links: don't fail the generated-docs sync PR over absolute self-links it can't fix #1944 check-links, generated-docs sync PR — conflicts with check-links: one suggestion per fix, synced with findings; one fact per line in reports #1916 on dev/check-links.mjs; rebase after check-links: one suggestion per fix, synced with findings; one fact per line in reports #1916 merges

Squash-merge each, then rebase the next onto main.

#1948 (broken) and #1949 (fixed) are the example PRs that exercise every check; never merge, close them once the stack has landed.

marcleblanc2 and others added 3 commits September 11, 2026 05:13
dev/check-redirects.mjs checks every entry in src/data/redirects.ts:
source shadows a page, source has a #fragment, duplicate source,
/docs prefix, chained redirect, missing destination page or heading.
The workflow compares against the merge base, so only redirects a PR
breaks are reported, grouped by problem with the fix explained under
each heading, and posts one suggested change per fixable entry the PR
added (deleted again once the finding is gone).

Not part of `npm run check`: main has hundreds of pre-existing
findings.

Squash of the check-redirects branch rebased onto main; the check-links
commits it carried are already on main.

Amp-Thread-ID: https://ampcode.com/threads/T-01a08fee-74b4-76dc-aaf9-d1245d68fdc9
Co-authored-by: Amp <amp@ampcode.com>
@vercel

vercel Bot commented Sep 11, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
sourcegraph-docs Ready Ready Preview Sep 11, 2026 5:27pm UTC

Request Review

marcleblanc2 added a commit that referenced this pull request Sep 11, 2026
…er line in reports (#1916)

Follow-ups from testing the PR checks on #1913 / #1914.

- Review comments: one suggested change per finding (not one per line),
no review body. Each comment starts with an HTML marker; the workflow
deletes suggestions whose finding is gone (or that GitHub could no
longer place, `line: null`) and skips ones already posted, so resolved
suggestions disappear like the spell check's do.
- Summary comment and review comments put line, link, problem, and fix
each on their own line.
- Absolute links to this site get their own **Absolute links** section
instead of being lumped into Outbound.
- Case-mismatch findings now come with a fix (`/Code-Search/queries` →
`/code-search/queries`).
- Wording: "Write links on this site as relative paths", "fix the
inbound links on these other pages", dropped
`https://docs.sourcegraph.com/…`; reproduce command matches
`package.json` (`pnpm check links …`).

Tested locally against the `test-pr-checks-broken` branch with the CI
recipe (baseline from `origin/main`, `--diff`, `--review`); build-mode
run (`node dev/check-links.mjs`) still clean.

Trade-off: when several fixes sit on one line, applying one suggestion
outdates the others until the next run re-posts them, because GitHub
will not batch overlapping suggestions.

<!-- pr-stack-merge-order -->
## Merge order for the PR-check stack

Trial-merged onto `main` in this order with no conflicts:

1. #1946 Vercel build log comment — independent; first so the other PRs'
Vercel failures get a readable log
2. #1916 check-links report format — adds `dev/sync-review-comments.sh`,
which #1935 calls
3. #1935 redirect check — needs #1916 merged first
4. #1947 spell check comment updates — independent
5. #1944 check-links, generated-docs sync PR — conflicts with #1916 on
`dev/check-links.mjs`; rebase after #1916 merges

Squash-merge each, then rebase the next onto `main`.

#1948 (broken) and #1949 (fixed) are the example PRs that exercise every
check; never merge, close them once the stack has landed.

---------

Co-authored-by: Amp <amp@ampcode.com>
marcleblanc2 added a commit that referenced this pull request Sep 12, 2026
## Problem

- When a PR's Vercel build fails, the Vercel bot comment and the
`Vercel` check link to the Vercel inspector, which only Vercel team
members can open
- Contributors who aren't Vercel team members see a red X and a login
wall, with no hint what broke
- Example: #1913

## Solution

- On `vercel.deployment.error` (the same `repository_dispatch` channel
`preview-links.yml` uses), comment the tail of the build log on the PR
Vercel built the deployment for (`meta.githubPrId`), like the links /
redirects / CSpell checks do. Each PR gets its own deployment, so two
PRs at the same commit each get their own comment
- On the next `vercel.deployment.success`, update that comment to ✅ on
every open PR at the commit that has one. The success path only reads PR
comments; it does not touch Vercel
- Lines that look like credentials (JWTs, `vcp_` / `ghp_` /
`github_pat_` / `AKIA…` tokens, `Bearer …`, `*TOKEN=` / `*SECRET=` /
`*KEY=` assignments) are redacted before the log is written, since the
comment and artifact are public and the build gets `VERCEL_OIDC_TOKEN`,
`VERCEL_DEPLOYMENT_KEY` and `VERCEL_ENV_ENC_KEY`. The code fence is
sized longer than any backtick run in the log, so a log line cannot
close it and inject Markdown

## Token

- `VERCEL_TOKEN` repo secret is a [project-scoped
token](https://vercel.com/docs/accounts/access-tokens) for
`sourcegraph-docs` only, expiring 2026-12-10. Vercel has no read-only
scope; project scope is the narrowest it offers, and it can read and
write everything in that one project
- Enumerated with read-only calls:
- Allowed: project settings, environment variables (`/env`, including
`?decrypt=true`; the project has none), deployments list, deployment
metadata, build log events, domains, custom environments, project list
(returns only this project)
- Denied: team, team members, user, log drains, webhooks, Edge Config,
other projects, minting tokens
- Writes within the project (env vars, deployments, domains, settings)
are allowed per Vercel's docs; not exercised
- `fetch-log` asks GitHub for an open PR from this repository at the
commit before it contacts Vercel, so a dispatch for a fork PR or a stale
commit never uses the token. `repository_dispatch` runs the workflow and
script from `main`, so a PR can't change the code the token is handed to
- Anyone with write access can read any repo secret by pushing a
workflow; this token limits what that buys them to one Vercel project

## Slack

- The Vercel Slack app already posts ":red_circle: … failed to deploy …
`<short sha>` | sourcegraph-docs" to `#alerts-vercel-doc-site`. On a
failure, a last step finds that post (looking back 30 minutes, then
polling for up to 5 more since Vercel and this workflow are triggered by
the same event) and uploads the full redacted log into its thread,
linking the PR comment. `continue-on-error`, so a Slack problem can't
hide the PR comment
- Needs the `SLACK_BOT_TOKEN` repo secret and `SLACK_CHANNEL_ID` repo
variable; skips quietly without them. The bot is the app in
`dev/slack-app-vercel-build-report.json` (`channels:history` to find the
post, `files:write` to reply); it must be `/invite`d to the channel
- Not yet run end to end; the app and secret are still to be created

## Tested

- Run locally against the two example PRs, which is how the comments
there got posted (from my account, since GitHub neither delivers
`repository_dispatch` nor resolves `workflow_dispatch` for a workflow
that isn't on `main` yet):
- #1948: ❌ [build log
comment](#1948 (comment))
- #1949: ❌ posted on a broken revision, then updated to ✅ [after the
fixed revision
built](#1949 (comment))
- A PR with no failed build exits with `has no failed build to resolve`
- That test found a bug: with two open PRs at the same head SHA, only
the first PR the `commits/{sha}/pulls` API listed got the comment.
Vercel records the PR a deployment was built for (`meta.githubPrId`), so
`fetch-log` now reads it and the comment lands on that PR only. The
success path stays off Vercel and keeps the commit lookup, since it only
updates comments that already exist
- Re-ran `fetch-log` after the reorder: #1948's head →
`pull_request=1948`; a commit with no open PR stops at `No open PR with
head …` with `VERCEL_TOKEN=invalid`, proving Vercel was not contacted.
Redaction and fence sizing checked against JWT, `vcp_`, `ghp_`, `AKIA…`,
`Bearer`, `KEY: value` lines and a log line of six backticks
- After merge, re-run on a PR by hand: `gh workflow run
vercel-build-report.yml -f id=dpl_... -f state=error -f sha=<pr head
sha>`

<details><summary>Dry-run output (abridged)</summary>

> ### ❌ The Vercel build failed for this PR
>
> Vercel only shows build logs to members of its team, so here is the
end of the log.
> Run `npm run build` locally to reproduce.
>
> **Build log**
> ```
> Running build in Cleveland, USA (East) – cle1
> ...
> ❌ Found 2 dead link(s) in 1 file(s):
> 📄 docs/code-search/features.mdx
>    Line 154: /code-search/no-such-page
> ...
> ❌ Failed checks: links, filenames
>  ELIFECYCLE  Command failed with exit code 1.
> Error: Command "pnpm run build" exited with 1
> ```

</details>

## Amp thread

- [Vercel build failure
report](https://ampcode.com/threads/T-01a09014-dfa8-740c-95b4-9e28c43cae51)

<!-- pr-stack-merge-order -->
## Merge order for the PR-check stack

Trial-merged onto `main` in this order with no conflicts:

1. #1946 Vercel build log comment — independent; first so the other PRs'
Vercel failures get a readable log
2. #1916 check-links report format — adds `dev/sync-review-comments.sh`,
which #1935 calls
3. #1935 redirect check — needs #1916 merged first
4. #1947 spell check comment updates — independent
5. #1944 check-links, generated-docs sync PR — conflicts with #1916 on
`dev/check-links.mjs`; rebase after #1916 merges

Squash-merge each, then rebase the next onto `main`.

#1948 (broken) and #1949 (fixed) are the example PRs that exercise every
check; never merge, close them once the stack has landed.

---------

Co-authored-by: Amp <amp@ampcode.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant