Skip to content

check-links: don't fail the generated-docs sync PR over absolute self-links it can't fix - #1944

Open
marcleblanc2 wants to merge 1 commit into
mainfrom
marc/check-links/upstream-generated-files
Open

check-links: don't fail the generated-docs sync PR over absolute self-links it can't fix#1944
marcleblanc2 wants to merge 1 commit into
mainfrom
marc/check-links/upstream-generated-files

Conversation

@marcleblanc2

@marcleblanc2 marcleblanc2 commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Why

#1858 added the Check links job (--check-anchors --check-self-links), and #1899 rewrote the absolute https://sourcegraph.com/docs/… links in the generated observability/telemetry pages to relative ones. The sync bot (doc/_generated.push.sh) regenerates those files from Go sources that still contain the absolute links, so its next push to sync/generated-docs (#1883) reintroduces them, and the job flags each one as new against main.

Replaying the bot's output (#1883 head 80fb54e4, applied over main) through the job's exact command gives exit 1 with 26 findings, all "Absolute self-link" in alerts.mdx, dashboards.mdx, protocol.mdx and private-metadata-allowlist.mdx. Nothing in this repo can make that go green: the fix belongs in the generators.

What

In the files _generated.push.sh overwrites (self-hosted/observability/{alerts,dashboards}.mdx, cli/references/**, cody/capabilities/supported-models.mdx, admin/telemetry/{protocol,private-metadata-allowlist}.mdx), an absolute self-link is a finding only when its (redirect-followed) target is missing, and the message says (generated in sourcegraph/sourcegraph; fix it there). Everywhere else the check is unchanged. next build runs without --check-self-links, so deploys are unaffected.

Verification

  • Same replay with this branch: 26 → 2 findings, both real dead anchors in upstream source (monitoring/definitions/worker.go/admin/code-hosts/github#github-com-rate-limits, zoekt.go/code-search/features#shard-merging). Upstream fix: sourcegraph/sourcegraph PR to follow.
  • node dev/check-links.mjs --check-anchors --check-self-links --format json on this branch vs main: 248 findings each, 0 new, 0 removed.
  • npx tsc --noEmit passes.

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.

…ink finding

doc/_generated.push.sh in sourcegraph/sourcegraph overwrites alerts.mdx,
dashboards.mdx, cli/references/**, supported-models.mdx, protocol.mdx and
private-metadata-allowlist.mdx on every sync, with absolute
https://sourcegraph.com/docs/... links. Since #1899 made those links
relative here, the next sync PR reintroduces 26 findings and fails the
Check links job; nothing in this repo can fix that. In those files, an
absolute self-link is now only a finding when its target is missing, and
the message names the repository that owns the file.
@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 Error Error Sep 11, 2026 5:26pm 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