From b3b4f382aca96eb9d63c1da7b6d45fafeab69800 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 06:21:44 +0000 Subject: [PATCH] =?UTF-8?q?ci(docs):=20check-doc-links=20=E6=8F=90?= =?UTF-8?q?=E5=8D=87=E4=B8=BA=E6=97=A0=E8=B7=AF=E5=BE=84=E8=BF=87=E6=BB=A4?= =?UTF-8?q?=E7=9A=84=E7=8B=AC=E7=AB=8B=20workflow,=E7=BA=AF=20docs=20PR=20?= =?UTF-8?q?=E4=B9=9F=E6=8B=A6=E5=BE=97=E4=BD=8F=E6=96=AD=E9=93=BE=20(#3448?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `check-doc-links.mjs` 由 PR #3450 接进 CI 时,落点是 `ci.yml` 的 `docs` job —— 而 `ci.yml` 的 `paths-ignore` 列着 `'**/*.md'`、`content/**`、`docs/**`、 `apps/site/**`。GitHub 的 `paths-ignore` 语义是「改动文件全部命中即整个 workflow 不启动」,且没有 per-job path filter;站点文档全在 `content/docs/**`。所以**只改 文档的 PR 根本不会启动 ci.yml**,这道门禁看不见的恰恰是最可能改坏内链的那一类 PR。它此前只覆盖「文档+代码」的混合 PR 和 push 到 main —— 坏链能经纯 docs PR 合 进 main,直到下一个无关作者推代码时才把 main 弄红,归因还错人。 `control-bytes.yml` 撞过同一堵墙,头注写着结论:看不见 markdown-only PR 的门禁 "rebuilds the hole it exists to close";`changeset-guard.yml` 是同一形状的第二例。 本次是第三例。 改动: • 新增 `.github/workflows/docs-links.yml`,镜像 control-bytes.yml 的形状: push/PR to main+develop + workflow_dispatch,**无 paths / 无 paths-ignore**, checkout + setup-node + 一行 `node scripts/check-doc-links.mjs`,无 install 无网络。头注写清为什么不能加路径过滤。 • **从 ci.yml 的 docs job 删掉重复步骤**(最小重复原则):新 workflow 的触发集 是该 job 的严格超集,留着只会为同一条坏链多出一个红勾和一处会忘记同步的副本。 原地留注释说明它去哪了、为什么别加回来。 • 新增 `scripts/__tests__/docs-links-workflow.test.ts` 钉住形状:workflow 必须 存在、必须门禁 PR、必须既无 `paths` 也无 `paths-ignore`、必须是**唯一**跑该 脚本的 workflow;并以不变式表述「跑这个脚本的 workflow 必须是纯 docs PR 能 启动的」,即使将来改名搬家也成立。扫描前先剥掉整行注释 —— ci.yml 的说明注释 里仍然提到脚本名,不剥会误判成重复门禁。 • `content/docs/guide/ci-cd-pipeline.md`:这一条不是顺手改文档,是 `scripts/__tests__/ci-cd-pipeline-doc.test.ts` 的机械要求 —— 只加 workflow 不加章节,该测试立刻红,报「docs-links.yml 无对应标题」。补:清单表一行、新章节 (含为什么无路径过滤)、ci.yml 任务表里 docs 行的更正(它不再跑链接检查)、 以及 Link Checking 章节里 #3448 那个「已知缺口」的收口。 反向验证(方向先判后跑):把删掉的步骤加回 ci.yml —— 新测试如期两处红: "is the only workflow that runs the link checker"(ci.yml + docs-links.yml)与 "every workflow that runs it is one a docs-only PR can start"(ci.yml 有 paths-ignore)。给 docs-links.yml 加 `paths: content/**` —— "carries NO path filter" 如期红。 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01GTRjn8xBqp75dk7kFupVRt --- .github/workflows/ci.yml | 36 ++---- .github/workflows/docs-links.yml | 61 ++++++++++ content/docs/guide/ci-cd-pipeline.md | 57 +++++++-- scripts/__tests__/docs-links-workflow.test.ts | 112 ++++++++++++++++++ 4 files changed, 235 insertions(+), 31 deletions(-) create mode 100644 .github/workflows/docs-links.yml create mode 100644 scripts/__tests__/docs-links-workflow.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6e7c69fba..ce5c56477 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -320,30 +320,18 @@ jobs: node-version: '22.x' cache: 'pnpm' - # `scripts/check-doc-links.mjs` existed and worked, but nothing under - # `.github/` ever called it, so it had never run in CI — it exited 1 on an - # untouched `main` for a broken link that had been sitting there (#3213, - # #3292). It resolves every `/docs/...` markdown link against - # `content/docs/` on the filesystem: no install and no network, so it runs - # here — after Node is set up, before the expensive install + site build — - # and a bad link fails in seconds instead of after a full Next.js build. - # - # KNOWN GAP — read before assuming docs links are fully gated: `ci.yml` - # lists `content/**` and `'**/*.md'` under `paths-ignore`, and GitHub has - # no per-job path filter, so a docs-ONLY pull request never starts this - # workflow and is never link-checked. This step therefore covers pull - # requests that touch docs alongside code, plus pushes to `main` — not the - # pure-docs pull request, which is the likeliest way to break a link. - # - # `control-bytes.yml` hit this same wall and answered it by being its own - # workflow with no path filters, for a gate that likewise needs no install - # and no network; its header explains the reasoning, and - # `scripts/__tests__/check-control-bytes.test.ts` pins it. Moving this - # check to that shape is the known fix, but it changes CI triggering - # policy, so it is left to the maintainer — tracked in #3448. - - name: Check docs links - if: steps.docs-changes.outputs.should_run == 'true' - run: node scripts/check-doc-links.mjs + # The docs *link* check is deliberately NOT here any more — do not add it + # back. `node scripts/check-doc-links.mjs` ran in this job from #3213 / + # #3292 (PR #3450) until #3448 promoted it to its own workflow, + # `docs-links.yml`, with no path filters. The reason is this workflow's own + # `paths-ignore`: it lists `content/**` and `'**/*.md'`, and GitHub has no + # per-job path filter, so a docs-ONLY pull request never started `ci.yml` + # at all and the link check never saw the class of PR most likely to break + # a link. It is removed rather than kept in both places: the standalone + # workflow's trigger set is a strict superset of this job's, so a copy here + # could only ever add a second red check for the same broken link, and a + # second place to forget. `scripts/__tests__/docs-links-workflow.test.ts` + # pins the gate to exactly one home. - name: Turbo Cache if: steps.docs-changes.outputs.should_run == 'true' diff --git a/.github/workflows/docs-links.yml b/.github/workflows/docs-links.yml new file mode 100644 index 000000000..c9611010a --- /dev/null +++ b/.github/workflows/docs-links.yml @@ -0,0 +1,61 @@ +name: Docs Links + +# Why this is its own workflow instead of a step in `ci.yml`'s `docs` job, which +# is where this check lived when it first reached CI (#3213 / #3292, PR #3450): +# `ci.yml` lists `'**/*.md'`, `content/**`, `docs/**` and `apps/site/**` under +# `paths-ignore`, and GitHub has no per-job path filter. The published site is +# built from `content/docs/**`, so a docs-ONLY pull request matched every ignore +# pattern, started no workflow at all, and was never link-checked — while a +# docs-only PR is the likeliest way an internal link breaks in the first place. +# The step therefore only ever saw PRs that touched docs *alongside code*, plus +# pushes to `main`; a broken link could land through a pure-docs PR and only turn +# `main` red later, under an unrelated author (#3448). +# +# `control-bytes.yml` hit the same wall and its header names the consequence: a +# gate that cannot see a markdown-only PR "rebuilds the hole it exists to close". +# `changeset-guard.yml` is the second instance of the shape in this repo. +# +# Hence: no `paths` and no `paths-ignore` here, deliberately. +# `scripts/__tests__/docs-links-workflow.test.ts` fails if either is ever added. +# A `paths: content/**` filter would look tighter and buy nothing measurable: the +# whole run is a checkout plus one `node` call — no install, no network, a few +# seconds — and the filter would be a second, drift-prone copy of the script's +# own scan surface. Keep it that way if you add checks here. + +on: + pull_request: + branches: [main, develop] + push: + branches: [main, develop] + workflow_dispatch: + +concurrency: + group: docs-links-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + docs-links: + name: Internal Docs Link Check + runs-on: ubuntu-latest + timeout-minutes: 5 + + steps: + - name: Checkout code + uses: actions/checkout@v7 + + - name: Setup Node.js + uses: actions/setup-node@v7 + with: + node-version: '22.x' + + # Resolves every relative / `/docs/...` markdown link found in + # `content/docs/**` against the files actually on disk. Reads the checkout + # and nothing else, so no install is required. External URLs are a + # different problem with a different tool — Lychee, in `check-links.yml`, + # which is manual-dispatch only and scans `docs/` rather than + # `content/docs/` (#3449). + - name: Check internal docs links + run: node scripts/check-doc-links.mjs diff --git a/content/docs/guide/ci-cd-pipeline.md b/content/docs/guide/ci-cd-pipeline.md index 5fc89a657..b6ee4234b 100644 --- a/content/docs/guide/ci-cd-pipeline.md +++ b/content/docs/guide/ci-cd-pipeline.md @@ -27,6 +27,7 @@ one has its own section below. | `lint.yml` | Lint | Push / PR to `main`, `develop`; manual | **Yes** — ESLint **errors** only | | `changeset-guard.yml` | Changeset Bump Policy | PR / push touching `.changeset/**` | **Yes** | | `control-bytes.yml` | Control Byte Scan | Push / PR to `main`, `develop` — **no path filter**; manual | **Yes** | +| `docs-links.yml` | Internal Docs Link Check | Push / PR to `main`, `develop` — **no path filter**; manual | **Yes** | | `performance-budget.yml` | Bundle Analysis | Push / PR touching `packages/**`, `apps/console/**`, `pnpm-lock.yaml` | **Yes** — the console entry gzip budget | | `live-e2e.yml` | Live E2E (informational) | PR to `main`, `develop` (code paths); nightly cron `30 6 * * *`; manual | No — informational lane, `continue-on-error` | | `labeler.yml` | Auto Label PRs | PR `opened`, `synchronize`, `reopened` | No | @@ -39,13 +40,16 @@ one has its own section below. | `shadcn-check.yml` | Check Shadcn Components | Weekly cron `0 9 * * 1`; manual | n/a | | `check-links.yml` | Check Links | Manual dispatch only | n/a | -Two path-filter facts explain most "why did nothing run on my PR?" questions: +The path filters explain most "why did nothing run on my PR?" questions: - `ci.yml` and `lint.yml` both list `**/*.md`, `content/**`, `docs/**` and `.changeset/**` under `paths-ignore` (`ci.yml` also ignores `apps/site/**`). A docs-only or changeset-only PR starts neither of them. - `changeset-guard.yml` carries the inverse filter — it runs *only* when `.changeset/**` changes, which is precisely why it is a separate workflow instead of a job inside `ci.yml`. +- `control-bytes.yml` and `docs-links.yml` carry **no** filter of any kind, which is equally + deliberate: both guard markdown, and a gate that a markdown-only PR cannot start is no gate on + the change most likely to trip it. Both cost a checkout plus one `node` call. ## Core CI Workflow (`ci.yml`) @@ -61,7 +65,7 @@ Seven jobs, all parallel — there are no `needs:` edges between them: | `test` | Test (shard N/4) | `pnpm test --shard=N/4` across a 4-runner matrix with `fail-fast: false`, so every shard reports its own failures. No coverage instrumentation — v8 adds 40–100% overhead. | **Pull requests only** | | `test-coverage` | Test (coverage) | One unsharded `pnpm test:coverage`, uploaded to Codecov. Nothing blocks on it, which is why it is not sharded. | **Push only** | | `e2e` | Build & E2E | Builds the console with `vite build` (`VITE_BASE_PATH=/console/`), verifies the artifact, then `pnpm test:e2e --project=chromium`. Uploads the Playwright report on failure. | Every run | -| `docs` | Build Docs | `scripts/check-doc-links.mjs` (resolves every `/docs/...` markdown link against `content/docs/` — no install, no network), then `turbo run build --filter='@object-ui/site'`. On a PR it first diffs against the base and skips both when nothing under `apps/site/` or `content/` changed. | Every run (steps themselves conditional) | +| `docs` | Build Docs | `turbo run build --filter='@object-ui/site'`. On a PR it first diffs against the base and skips the build when nothing under `apps/site/` or `content/` changed. It does **not** check docs links any more — that moved to `docs-links.yml` (#3448), because this workflow's `paths-ignore` hides exactly the docs-only PRs a link check needs to see. | Every run (build itself conditional) | | `dev-server` | Dev-server fixture build | `pnpm --filter @object-ui/dev-server build` — guards `apps/dev-server`'s `objectstack.config.ts` against fixture / `@objectstack/spec` drift. | Every run | Uses: Node 22.x, pnpm via `corepack`, `actions/cache` over `.turbo/cache`. @@ -213,6 +217,45 @@ Playwright report and job summary. Backend pins live in `e2e/live/ci/backend.env` and must match the `@objectstack/spec` version in `pnpm-lock.yaml` — bump both in the same PR, or the run proves nothing. +## Internal Docs Links (`docs-links.yml`) + +**Triggers:** Push and PR to `main`/`develop`, plus manual dispatch — with **no path filter at +all**, which is the point of the workflow. It appears in the checks list as **Internal Docs Link +Check**. + +Runs `scripts/check-doc-links.mjs`, which walks every `.md` / `.mdx` file under `content/docs/` and +resolves each internal markdown link against the files actually on disk (`/docs/foo` must have a +`foo.md`, `foo.mdx` or `foo/index.md*` under `content/docs/`). External `http(s)` and `mailto:` +links and bare `#anchors` are skipped — those belong to Lychee, below. No install, no build, no +network: a checkout and one `node` call. + +**Why it blocks a merge.** A broken internal link is a 404 on the published site, and nothing else +in CI sees it: the site build succeeds with a dead link in it. The script itself is older than its +gate — it existed, worked, and was wired to nothing under `.github/`, so it had never run in CI at +all, and `main` sat with a broken link it would have caught (objectui#3213, objectui#3292). + +**Why it is a separate workflow.** This is the second instance of the lesson `control-bytes.yml` +records, and it was found by the PR that first put this check into CI. That PR added it as a step +in `ci.yml`'s `docs` job — where it could never see the PRs that matter. `ci.yml` lists +`'**/*.md'`, `content/**`, `docs/**` and `apps/site/**` under `paths-ignore`, GitHub's +`paths-ignore` skips the *whole workflow* when every changed file matches, and GitHub has no +per-job path filter. So a **docs-only** PR — the likeliest way an internal link breaks — started no +workflow at all, and the check only ever ran on PRs that touched docs alongside code, plus pushes +to `main`. A bad link could merge through a pure-docs PR and turn `main` red later under an +unrelated author (objectui#3448). + +The step was **removed** from `ci.yml` in the same change rather than left in place. This +workflow's trigger set is a strict superset of that job's, so keeping both would only add a second +red check for one broken link, and a second place to forget. +`scripts/__tests__/docs-links-workflow.test.ts` pins all of it: the workflow must exist, must gate +pull requests, must carry neither `paths` nor `paths-ignore`, and must remain the only workflow +that runs the script. + +**If it fails:** it prints every offending `file -> href`. Either the link is misspelled, or the +page it points at has moved or been renamed — fix the link, or restore the target. Links are +checked as *routes*, so `/docs/guide/foo` is what belongs in the markdown, not +`content/docs/guide/foo.md`. Run it locally with `pnpm docs:check-links`. + ## Link Checking (`check-links.yml`) **Trigger:** Manual workflow dispatch (`workflow_dispatch`). @@ -221,17 +264,17 @@ There are **two** link checkers, and they cover different things (objectui#3213) | | Covers | Network | Runs | |---|---|---|---| -| `scripts/check-doc-links.mjs` | **Internal** `/docs/...` routes, resolved against `content/docs/` | No | In `ci.yml`'s `docs` job — see the job table above | +| `scripts/check-doc-links.mjs` | **Internal** `/docs/...` routes, resolved against `content/docs/` | No | `docs-links.yml` — every push and PR, no path filter (previous section) | | Lychee (this workflow) | **External** URLs in `docs/` and `README.md` | Yes | Manual dispatch only | Note the asymmetry in what Lychee scans: `docs/` holds internal material (ADRs, audits, architecture notes), while the published site is built from `content/docs/`. Lychee therefore does not currently see the site's own pages. -Two known gaps are tracked rather than silently lived with: `ci.yml` lists `content/**` under -`paths-ignore` and GitHub has no per-job path filter, so a **docs-only** PR does not start `ci.yml` -and is not link-checked (objectui#3448); and Lychee's scan scope predates the move to -`content/docs/` (objectui#3449). +One known gap remains tracked rather than silently lived with: Lychee's scan scope predates the +move to `content/docs/` (objectui#3449), so **external** URLs on the published site's own pages are +checked by nothing. The gap that used to sit beside it — docs-only PRs never being link-checked, +because `ci.yml` ignores `content/**` — is closed: that check is now `docs-links.yml` (objectui#3448). Uses [Lychee](https://github.com/lycheeverse/lychee) with configuration from `lychee.toml`: - Scans markdown files in `docs/` and `README.md` diff --git a/scripts/__tests__/docs-links-workflow.test.ts b/scripts/__tests__/docs-links-workflow.test.ts new file mode 100644 index 000000000..269487bd4 --- /dev/null +++ b/scripts/__tests__/docs-links-workflow.test.ts @@ -0,0 +1,112 @@ +import { describe, expect, it } from 'vitest'; +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +/** + * objectui#3448 — the wiring test for `scripts/check-doc-links.mjs`. + * + * The script is older than its gate. It existed, worked, and nothing under + * `.github/` called it, so it had never run in CI at all and `main` carried a + * broken link it would have caught (#3213, #3292). PR #3450 fixed that by adding + * it as a step in `ci.yml`'s `docs` job — and rebuilt half the hole in the + * process: `ci.yml` lists every markdown path, plus `content/**`, `docs/**` and + * `apps/site/**`, under `paths-ignore`; `paths-ignore` skips the *entire* + * workflow when every changed file matches it, and GitHub has no per-job path + * filter. A docs-only PR therefore started no workflow, so the one class of + * change most likely to break an internal link was the one class the link check + * could never see. + * + * `control-bytes.yml` hit this exact wall first and its header states the + * consequence: a gate that cannot see a markdown-only PR "rebuilds the hole it + * exists to close". `changeset-guard.yml` is the second instance. This is the + * third, and the test below is what keeps it from regressing — the failure mode + * is silent by construction, because a path-filtered gate looks green and + * configured while simply never running. + * + * The last assertion pins the de-duplication decision, not just the shape: the + * check has exactly ONE home. Re-adding a copy to a path-filtered workflow is + * the specific mistake that would restore the illusion of coverage. + */ + +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); +const workflowDir = path.join(repoRoot, '.github/workflows'); +const workflowPath = path.join(workflowDir, 'docs-links.yml'); +const SCRIPT = 'scripts/check-doc-links.mjs'; + +/** + * A workflow's YAML with whole-line comments removed. + * + * Required, not cosmetic: `ci.yml` still *names* the script in the comment that + * explains why the step was removed, and this workflow's own header discusses + * `paths` / `paths-ignore` in prose. A scan that counted those would report a + * duplicate gate and a path filter that neither file has. + */ +function withoutComments(yaml: string): string { + return yaml + .split('\n') + .filter((line) => !/^\s*#/.test(line)) + .join('\n'); +} + +const workflowFiles = fs.readdirSync(workflowDir).filter((f) => f.endsWith('.yml')); + +describe('docs-links.yml — the internal link gate is reachable', () => { + it('exists at all', () => { + expect(fs.existsSync(workflowPath), 'a check nothing runs is not a gate').toBe(true); + expect(fs.existsSync(path.join(repoRoot, SCRIPT)), `${SCRIPT} must exist for the workflow to run it`).toBe(true); + }); + + it('runs the link checker', () => { + expect(withoutComments(fs.readFileSync(workflowPath, 'utf8'))).toMatch( + new RegExp(`run:\\s*node\\s+${SCRIPT.replace(/[.]/g, '\\.')}`), + ); + }); + + it('gates pull requests, not just pushes', () => { + const yaml = withoutComments(fs.readFileSync(workflowPath, 'utf8')); + expect(yaml).toMatch(/^\s*pull_request:/m); + expect(yaml).toMatch(/^\s*push:/m); + }); + + it('carries NO path filter of any kind', () => { + // The whole reason this is its own workflow. `paths-ignore` would recreate + // #3448 verbatim; a `paths: content/**` filter would look tighter and buy + // nothing — the run is a checkout plus one `node` call, and the filter would + // be a second copy of the script's scan surface, free to drift from it. + const yaml = withoutComments(fs.readFileSync(workflowPath, 'utf8')); + expect(yaml).not.toMatch(/paths-ignore:/); + expect(yaml).not.toMatch(/^\s+paths:/m); + }); + + it('is the only workflow that runs the link checker', () => { + // Not tidiness: a second copy living in a path-filtered workflow is exactly + // how a gate ends up looking covered while the docs-only PR still slips + // past. One gate, one home — and one place to fix when it needs changing. + const runners = workflowFiles.filter((f) => + withoutComments(fs.readFileSync(path.join(workflowDir, f), 'utf8')).includes(SCRIPT), + ); + expect(runners).toEqual(['docs-links.yml']); + }); + + it('every workflow that runs it is one a docs-only PR can start', () => { + // States the invariant rather than the file name, so it still holds if the + // gate is ever moved or renamed: whoever runs this check must be startable + // by a PR that touches nothing but markdown. + expect(workflowFiles.length, 'the workflow directory scan returned implausibly few files').toBeGreaterThan(5); + + for (const file of workflowFiles) { + const yaml = withoutComments(fs.readFileSync(path.join(workflowDir, file), 'utf8')); + if (!yaml.includes(SCRIPT)) continue; + expect(yaml, `${file} runs ${SCRIPT} behind a paths-ignore — a docs-only PR would not start it`).not.toMatch( + /paths-ignore:/, + ); + expect(yaml, `${file} runs ${SCRIPT} behind a paths filter — see objectui#3448`).not.toMatch(/^\s+paths:/m); + } + }); + + it('is runnable locally under the name the docs give', () => { + const pkg = JSON.parse(fs.readFileSync(path.join(repoRoot, 'package.json'), 'utf8')); + expect(pkg.scripts['docs:check-links']).toBe(`node ${SCRIPT}`); + }); +});