Skip to content

test: check-links on a PR that adds absolute self-links and a dead external link (do not merge) - #1900

Closed
marcleblanc2 wants to merge 23 commits into
mainfrom
test-check-links-breaks-link
Closed

test: check-links on a PR that adds absolute self-links and a dead external link (do not merge)#1900
marcleblanc2 wants to merge 23 commits into
mainfrom
test-check-links-breaks-link

Conversation

@marcleblanc2

Copy link
Copy Markdown
Contributor

Exercises the #1858 workflow: two absolute self-links (one to a moved page, one with an anchor) and one dead external link. Expect a failing check, a report comment, and one review suggestion. Will be closed, not merged.

marcleblanc2 and others added 18 commits September 10, 2026 00:49
Add a pull_request workflow that runs dev/check-links.mjs --check-anchors
on both the PR head and its merge base, and reports only the findings the
PR introduces: outbound links from changed pages, and inbound links from
other pages to a page or heading the PR removed or renamed. Pre-existing
broken anchors on main are ignored. The job comments on the PR and fails
when new breakage is found.

dev/check-links.mjs gains --root, --format (text|json|markdown) and
--baseline to support that diff, plus case-mismatch detection for routes
(links that resolve on macOS but 404 on Linux) and scanning of *.md files.
Only *.mdx files count as routes, matching contentlayer's filePathPattern.

Amp-Thread-ID: https://ampcode.com/threads/T-01a0753f-0f4f-7478-b36c-87466e7c0261
Co-authored-by: Amp <amp@ampcode.com>
Generated pages such as admin/telemetry/protocol.mdx define anchors with
<a name="..."> rather than headings. Drops 132 false positives on main
(431 -> 299 with --check-anchors).

Amp-Thread-ID: https://ampcode.com/threads/T-01a0753f-0f4f-7478-b36c-87466e7c0261
Co-authored-by: Amp <amp@ampcode.com>
Replace the fs.existsSync asset checks (case-insensitive on macOS, so they
hid links that 404 on the Linux build) with an enumerated lowercase -> real
path map of files under public/ and docs/, mirroring the route check.

Also register docs/index.mdx as the / route. It was never in the path map
(the /index strip needed a leading slash), and existsSync('public/') was
masking that by accepting any '/' link. Six pre-existing broken /#anchor
links on the homepage are now reported.

Amp-Thread-ID: https://ampcode.com/threads/T-01a07597-43c0-751b-8c49-6e5809e714d2
Co-authored-by: Amp <amp@ampcode.com>
A green run posts nothing. If an earlier run left a report and the PR has
since been fixed, delete that comment instead of editing it to a checkmark.

Amp-Thread-ID: https://ampcode.com/threads/T-01a07597-43c0-751b-8c49-6e5809e714d2
Co-authored-by: Amp <amp@ampcode.com>
The checkmark comment only ever appears after an earlier run reported
breakage, so word it that way instead of "introduces no broken links".
Keeps the comment (reverts the delete from the previous commit).

Amp-Thread-ID: https://ampcode.com/threads/T-01a07597-43c0-751b-8c49-6e5809e714d2
Co-authored-by: Amp <amp@ampcode.com>
The checker resolves links against docs/**/*.mdx routes only and never
reads src/data/redirects.ts, so a redirect added alongside a page move
still leaves inbound links reported. Say so in the PR comment and in
AGENTS.md so contributors update the links instead.

Amp-Thread-ID: https://ampcode.com/threads/T-01a085bf-fe07-77a1-a4e2-55bff4679bcc
Co-authored-by: Amp <amp@ampcode.com>
…the source view

Drop glob in favour of fs.readdirSync(recursive) so the PR check needs one
package instead of the whole site, and skip the pnpm/setup-node steps.
Every finding now links to the file and line on the PR branch, in the
?plain=1 code view where #L anchors work.

Amp-Thread-ID: https://ampcode.com/threads/T-01a088d9-b8fd-76fd-ba93-3a416c5829a3
Co-authored-by: Amp <amp@ampcode.com>
Outbound findings are in files the PR changed (it added or edited a bad
link); inbound ones are elsewhere (the PR renamed or removed a link
target). The workflow passes git diff --name-only against the merge base.
… probe external links on added lines

Absolute links to this site (https://sourcegraph.com/docs/..., the legacy
docs.sourcegraph.com host, http://, //, www.) are findings even when the target
exists: they leave preview deployments and local dev, and hide moved pages
behind redirects. The finding proposes the relative link, following
src/data/redirects.ts when the page moved. Version-pinned links (/@5.1/...) stay
external.

--diff replaces --changed-files and scopes two new features to added lines:
--check-external requests each external link (HEAD, then GET on an error status,
following redirects) and reports 404/410 only; --review writes a GitHub review
with one suggested change per line, which the workflow posts, skipping
suggestions already on the PR.

Amp-Thread-ID: https://ampcode.com/threads/T-01a08a01-44c1-775b-84d0-d67ff9501905
Co-authored-by: Amp <amp@ampcode.com>
@vercel

vercel Bot commented Sep 10, 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 10, 2026 7:44am UTC

Request Review

@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

❌ This PR introduces 5 broken link(s)

Outbound

Your PR includes links to pages or anchors that do not exist, or absolute links to this site.

docs/code-search/features.mdx

  • line 152: https://sourcegraph.com/docs/admin/config/site_config — Absolute self-link to a moved page; use "/admin/config/site-config" instead
  • line 152: //www.sourcegraph.com/docs/code-search/ — Absolute self-link; use "/code-search" instead
  • line 152: http://docs.sourcegraph.com/admin/observability/tracing#sampling — Absolute self-link to a moved page; "/self-hosted/observability/tracing#sampling" replaced it, but anchor "sampling" not found in "/self-hosted/observability/tracing"
  • line 152: https://github.com/sourcegraph/docs/blob/main/this-file-does-not-exist.md — External link returns HTTP 404
  • line 152: https://sourcegraph.com/docs/@5.1/no-such-page — External link returns HTTP 404

Write links to this site as relative paths (/admin/config/site-config), not https://sourcegraph.com/docs/… or https://docs.sourcegraph.com/…: absolute links leave the preview deployment and local dev server, and hide moved pages behind redirects.

Reproduce locally with pnpm check-links --check-anchors (see dev/check-links.mjs).

Adding a redirect in src/data/redirects.ts does not satisfy this check, because it’s a workaround instead of a fix.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested fixes for the links this PR adds; details in the check-links comment.

Comment thread docs/code-search/features.mdx Outdated
- When viewing a file or directory, press the `y` key to expand the URL to its canonical form (with the full 40-character Git commit SHA).
- To share a link to multi-line range in a file, click on the starting line number and shift-click on the ending line number (in the left-hand gutter).

Test links for the check-links workflow (will be reverted): [site config](https://sourcegraph.com/docs/admin/config/site_config), [tracing](http://docs.sourcegraph.com/admin/observability/tracing#sampling), [dead external](https://github.com/sourcegraph/docs/blob/main/this-file-does-not-exist.md), [live external](https://github.com/sourcegraph/docs).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Absolute self-link to a moved page; use "/admin/config/site-config" instead
Suggested change
Test links for the check-links workflow (will be reverted): [site config](https://sourcegraph.com/docs/admin/config/site_config), [tracing](http://docs.sourcegraph.com/admin/observability/tracing#sampling), [dead external](https://github.com/sourcegraph/docs/blob/main/this-file-does-not-exist.md), [live external](https://github.com/sourcegraph/docs).
Test links for the check-links workflow (will be reverted): [site config](/admin/config/site-config), [tracing](http://docs.sourcegraph.com/admin/observability/tracing#sampling), [dead external](https://github.com/sourcegraph/docs/blob/main/this-file-does-not-exist.md), [live external](https://github.com/sourcegraph/docs).

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested fixes for the links this PR adds; details in the check-links comment.

- When viewing a file or directory, press the `y` key to expand the URL to its canonical form (with the full 40-character Git commit SHA).
- To share a link to multi-line range in a file, click on the starting line number and shift-click on the ending line number (in the left-hand gutter).

Test links for the check-links workflow (will be reverted): [site config](https://sourcegraph.com/docs/admin/config/site_config), [search](//www.sourcegraph.com/docs/code-search/), [tracing](http://docs.sourcegraph.com/admin/observability/tracing#sampling), [dead external](https://github.com/sourcegraph/docs/blob/main/this-file-does-not-exist.md), [dead pinned](https://sourcegraph.com/docs/@5.1/no-such-page), [live pinned](https://sourcegraph.com/docs/@5.1/admin/search), [live external](https://github.com/sourcegraph/docs).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • https://sourcegraph.com/docs/admin/config/site_config: Absolute self-link to a moved page; use "/admin/config/site-config" instead
  • //www.sourcegraph.com/docs/code-search/: Absolute self-link; use "/code-search" instead
  • http://docs.sourcegraph.com/admin/observability/tracing#sampling: Absolute self-link to a moved page; "/self-hosted/observability/tracing#sampling" replaced it, but anchor "sampling" not found in "/self-hosted/observability/tracing" (not fixed by this suggestion)
  • https://github.com/sourcegraph/docs/blob/main/this-file-does-not-exist.md: External link returns HTTP 404 (not fixed by this suggestion)
  • https://sourcegraph.com/docs/@5.1/no-such-page: External link returns HTTP 404 (not fixed by this suggestion)
Suggested change
Test links for the check-links workflow (will be reverted): [site config](https://sourcegraph.com/docs/admin/config/site_config), [search](//www.sourcegraph.com/docs/code-search/), [tracing](http://docs.sourcegraph.com/admin/observability/tracing#sampling), [dead external](https://github.com/sourcegraph/docs/blob/main/this-file-does-not-exist.md), [dead pinned](https://sourcegraph.com/docs/@5.1/no-such-page), [live pinned](https://sourcegraph.com/docs/@5.1/admin/search), [live external](https://github.com/sourcegraph/docs).
Test links for the check-links workflow (will be reverted): [site config](/admin/config/site-config), [search](/code-search), [tracing](http://docs.sourcegraph.com/admin/observability/tracing#sampling), [dead external](https://github.com/sourcegraph/docs/blob/main/this-file-does-not-exist.md), [dead pinned](https://sourcegraph.com/docs/@5.1/no-such-page), [live pinned](https://sourcegraph.com/docs/@5.1/admin/search), [live external](https://github.com/sourcegraph/docs).

@marcleblanc2 marcleblanc2 changed the title test: check-links on a PR that adds absolute self-links and a dead external link test: check-links on a PR that adds absolute self-links and a dead external link (do not merge) Sep 10, 2026
@marcleblanc2
marcleblanc2 force-pushed the test-check-links-breaks-link branch from f64089c to 67f39d4 Compare September 10, 2026 07:42
marcleblanc2 added a commit that referenced this pull request Sep 11, 2026
Linear [FE-499: Fix doc site
issues](https://linear.app/sourcegraph/issue/FE-499/fix-doc-site-issues)

## Problem

- Our docs site has hundreds of broken links
- `dev/check-links.mjs` finds broken internal links and anchors, but it
isn't run automatically, so PRs can easily break links (renaming a
heading, moving or deleting a page) without anyone noticing

## Solution

- Updated the script to also work as a PR check, with additional
functions beyond what's run when used as a CI test in Vercel builds
- PR check to run the script and report if the PR breaks links
- It runs the script (with `--check-anchors`) on both the PR head and
its merge base, and diffs the findings
- This catches both directions:

- **Outbound**: a changed page links to a page or `#heading` that
doesn't exist
- **Inbound**: the PR renames a heading or removes/moves a page that
other, unchanged pages link to — those show up as findings in files the
PR didn't touch

- Pre-existing broken links are ignored by the PR check
- 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

## Verification

PR check comment in test PR:
#1895 (comment)

### Broken links found

<img width="1826" height="1628" alt="Screenshot 2026-09-09 at 20 05 31"
src="https://github.com/user-attachments/assets/930cde1f-50b1-46c0-9421-bd75a257c17d"
/>

### Broken links fixed

<img width="910" height="168" alt="Screenshot 2026-09-09 at 20 06 31"
src="https://github.com/user-attachments/assets/1cc26276-5cba-4595-85fc-8e10955aabab"
/>

## Absolute self-links and external links

- Absolute links to this site (`https://sourcegraph.com/docs/…`,
`http://…`, `//…`, `www.`, the legacy `https://docs.sourcegraph.com/…`)
fail the check even when the target exists: they leave the Vercel
preview and local dev, and hide moved pages behind redirects. The
finding names the relative link, following `src/data/redirects.ts` when
the page moved. Version-pinned links (`/@5.1/…`) stay external
- External links on lines this PR added are requested (HEAD, then GET on
an error status, following redirects); only 404 and 410 are findings, so
rate limits, bot blocks, 5xx and network errors never fail a PR.
Placeholder hosts (`*.example.com`, `localhost`, templated `<host>`) are
skipped
- Findings with a fix become one suggested-change review comment per
line, which the author can apply from the PR. Suggestions already on the
PR are not posted again
- #1899 clears the 67 existing absolute self-links so this check starts
from zero

Test PR: #1900 (report comment + one review suggestion; the `#sampling`
anchor deliberately does not exist, so that link gets no suggestion; a
second run posted nothing new)

## Related

- Draft PR #1562 proposes a daily Slack digest with a separate
reimplementation of this script
  - Instead, this PR improves on the existing script, and gates PRs
- PR #1860 enabled external link
checkers to find broken links again

## Amp threads

- [Broken link PR
check](https://ampcode.com/threads/T-01a0753f-0f4f-7478-b36c-87466e7c0261)
- [Asset case
mismatch](https://ampcode.com/threads/T-01a07597-43c0-751b-8c49-6e5809e714d2)
- [Docs - Fix broken heading
links](https://ampcode.com/threads/T-01a07623-9d65-7356-96b8-2bebb31ffa5a)
- [Self-links and external
links](https://ampcode.com/threads/T-01a08a01-44c1-775b-84d0-d67ff9501905)

---------

Co-authored-by: Amp <amp@ampcode.com>
Base automatically changed from check-links-pr-check to main September 11, 2026 01:36
@marcleblanc2

Copy link
Copy Markdown
Contributor Author

Moved to #1940 (branch renamed to marc/test-check-links-breaks-link).

@marcleblanc2
marcleblanc2 deleted the test-check-links-breaks-link branch September 11, 2026 17:27
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