🪲 [Fix]: Documentation pages with footnotes no longer fail the link check - #133
Merged
Marius Storhaug (MariusStorhaug) merged 3 commits intoAug 2, 2026
Merged
Conversation
…cker The link checker treats a GFM footnote definition as a reference-style link definition and validates the first word of the footnote prose as a relative path. These tests reproduce that, and pin the reference-style definition behavior that must keep working. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
A GFM footnote definition matched the reference-style definition pattern because the label '^1' passes the label character class. The footnote's first word was then validated as a relative path. A negative lookahead on '^' keeps footnote prose out of link validation while an inline link inside that prose is still checked. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot started reviewing on behalf of
Marius Storhaug (MariusStorhaug)
August 2, 2026 13:53
View session
There was a problem hiding this comment.
Pull request overview
This pull request updates the documentation link checker so GFM footnote definition lines ([^1]: ...) are no longer misinterpreted as reference-style link definitions, eliminating false-positive “broken link” reports while keeping real links (including inline links inside footnotes) validated.
Changes:
- Exclude footnote-definition labels from the reference-definition regex used by
Test-DocumentationLink.ps1. - Update script documentation/comments to reflect the intended footnote handling behavior.
- Add a dedicated Pester suite that exercises footnotes, reference-style definitions, and inline links inside footnotes using an out-of-process fixture repo.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
.github/scripts/Test-DocumentationLink.ps1 |
Adjusts reference-definition matching to ignore footnote definitions while preserving inline-link validation. |
tests/Test-DocumentationLink.Tests.ps1 |
Adds fixture-based Pester coverage to prevent regressions around footnotes and reference-style links. |
Marius Storhaug (MariusStorhaug)
marked this pull request as ready for review
August 2, 2026 13:56
Marius Storhaug (MariusStorhaug)
enabled auto-merge (squash)
August 2, 2026 13:56
Marius Storhaug (MariusStorhaug)
disabled auto-merge
August 2, 2026 14:03
Marius Storhaug (MariusStorhaug)
deleted the
fix-130-footnote-link-check
branch
August 2, 2026 14:07
Marius Storhaug (MariusStorhaug)
added a commit
that referenced
this pull request
Aug 2, 2026
The success message gained a scanned-file count in #134, and #133's tests were written against the message before it. Both merged green because no job ran Pester. Assert the count as well, so the check still proves files were scanned. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This was referenced Aug 2, 2026
Merged
Marius Storhaug (MariusStorhaug)
added a commit
that referenced
this pull request
Aug 2, 2026
Every pull request now runs the repository's Pester suites before review, so a change that breaks the workspace bootstrap, the documentation index generator, or the link checker fails a check instead of merging green and being discovered by whoever happens to run the tests by hand. ## New: A failing test blocks the pull request The `Docs` workflow gained a **`Test`** check that runs on every pull request and on every push to `main`, alongside `Lint`, `Build`, and `Links`. It discovers every `tests/*.Tests.ps1` suite from disk rather than from a list, so a suite added tomorrow is gated the moment it lands. `Publish` now waits for `Test` too, so the site is only deployed from a commit whose tests passed. The result is readable without opening the log. The run summary carries a per-suite table — totals, passes, failures, skips, durations — each failing test is annotated on the file and line that failed, and the job ends on one notice or error stating the verdict. ## Fixed: `main` was already failing four tests The new check found a regression on its first run. #134 changed the link checker's success message to carry a scanned-file count, #133 added tests asserting the message *without* that count, both merged green, and `main` has been failing four tests ever since — invisible, because nothing ran them. The four assertions now expect the count as well, which is the stronger check: it proves files were actually scanned, which was #134's whole point. ## New: One command runs the suites locally `CONTRIBUTING.md` now documents the same entry point CI uses, next to the existing index and link-check instructions: ```pwsh pwsh .github/scripts/Invoke-PesterSuite.ps1 ``` It installs the pinned Pester for the current user when it is missing, runs everything under `tests/`, and exits non-zero when a test fails — so the local result and the CI result come from one code path rather than two that can drift. --- <details> <summary>Technical details</summary> **Proof that the check gates, not just runs.** Three runs on this branch, in order: | Run | What it proves | | --- | --- | | [30751545862](https://github.com/MSXOrg/docs/actions/runs/30751545862) | The first run of the job. `Test` failed — 4 of 51 — on the pre-existing `main` regression described above. The gate found a real defect before it found anything else. | | [30751764554](https://github.com/MSXOrg/docs/actions/runs/30751764554) | Green after the assertion fix. Log shows `Discovered 4 suite(s)` naming all four files and `Tests Passed: 51, Failed: 0` — the same 51 that run locally, so nothing was silently skipped. Notice annotation: `51 test(s) passed in 4 suite(s) (47.8s)`. | | [30751893006](https://github.com/MSXOrg/docs/actions/runs/30751893006) | A deliberately failing test (`1 \| Should -Be 2`) was pushed as `tests/Deliberate-Failure.Tests.ps1` and then removed. `Test` went red: `Discovered 5 suite(s)`, `Tests Passed: 52, Failed: 1`, a per-test error annotation on the failing line, the job's own error annotation `1 of 53 test(s) failed, and 0 suite(s) failed to run.`, `Process completed with exit code 1`, and `Publish` skipped. The new file was picked up without editing the workflow, which is the discovery behaviour proven too. | The temporary file was removed by resetting the branch, so it is not in the diff; the run above is the durable evidence. Locally the suite is 51 passed / 0 failed in ~90 s, and `Invoke-ScriptAnalyzer -Settings .github/linters/.powershell-psscriptanalyzer.psd1` is clean on the new script. **Where the logic lives.** `.github/scripts/Invoke-PesterSuite.ps1` resolves Pester, discovers the suites, runs them, writes the job summary, emits the verdict annotation, and sets the exit code. The workflow step is a single `run:` line, so no script is embedded in a YAML string. This follows the repository's existing convention (`Test-DocumentationLink.ps1`, `Update-DocumentationIndex.ps1`, `Wait-CopilotReview.ps1` are all invoked the same way) rather than the [GitHub Actions standard's](https://msxorg.github.io/docs/Coding-Standards/GitHub-Actions/#extract-non-trivial-run-scripts-into-an-action) stricter form of a local action under `.github/actions/`. Deliberate deviation: a fourth sibling script is consistent with the three that already exist, and converting all four to local actions is a separate change. **How Pester is pinned.** Exact version `6.0.1` plus a module **GUID** check after import, so name and version both have to match. The suites' own `#Requires` lines keep their major lock (`ModuleVersion = '6.0.0'; MaximumVersion = '6.*'`) and are unchanged — [Dependencies → Update tracks](https://msxorg.github.io/docs/Coding-Standards/Dependencies/#update-tracks-who-each-is-for-and-whether-you-need-it) is explicit that the same dependency is pinned differently depending on who depends on it: a library floor-declares, while "an application or end artifact — a workflow, a deployable, a CI pipeline — pins to exact resolved versions". `6.0.1` is the newest stable `6.x` on the PowerShell Gallery. Dependabot has no PowerShell Gallery ecosystem, so this pin cannot move automatically; that gap is filed as #136 rather than left implicit, since the standard is clear that tight pinning is only safe when the updates are automated. **A green run that tested nothing is a failure.** The script exits 1 when discovery finds no `*.Tests.ps1` file, when the discovered suites contain no test, or when a suite fails to run at all (`FailedContainersCount`) — not only when an assertion fails. Without those guards a mistyped path would produce a green, worthless check. **Diagnostics.** Plain PowerShell rather than the PSModule `GitHub` module — the [documented fallback](https://msxorg.github.io/docs/Coding-Standards/GitHub-Actions/#default-to-powershell-as-the-glue-language) when the module is not warranted, and installing it plus `Markdown` into a docs repository to format one table is not. Consequently `::group::` / `::endgroup::` markers are written directly instead of through `LogGroup`; there are two of them. Per-test error annotations come from Pester's own `Output.CIFormat = 'GithubActions'`. Annotation values are percent-encoded before they reach the workflow command. **Hermeticity of `tests/Initialize-MsxWorkspace.Tests.ps1` — nothing is skipped.** The suite does real git work, so it was read before being wired in. It builds bare repositories under `[IO.Path]::GetTempPath()` and clones them over local filesystem paths, so no network fetch happens and the private `MSXOrg/memory` remote is never contacted; identity is written with repository-local `git config`, never `--global` or `--system`; `bootstrap/Initialize-MsxWorkspace.ps1` itself contains no global config write and no commit; and every run is directed away from `$HOME/.msx` by `-Root` or by `MSX_WORKSPACE_ROOT`. The seed-block tests read `MSX_DOCS_URL` / `MSX_MEMORY_URL`, which is why the documented `https://github.com/MSXOrg/...` defaults are never used. One latent dependency was checked rather than assumed: the fixture pushes `main` from a clone of an *empty* bare repository, which would break if the clone adopted a `master` HEAD — verified locally with `init.defaultBranch=master` that the clone adopts the remote's unborn `main`. Confirmed in practice by run 30751764554, where all 31 of its tests passed on `ubuntu-24.04` with no `~/.msx`, no configured global git identity, and no access to `MSXOrg/memory`. No test needed `-Skip`. **Branch name.** `maintenance-131-pester-in-ci`, not the `<type>/<issue>-<slug>` form in [Branching and Merging](https://msxorg.github.io/docs/Ways-of-Working/Branching-and-Merging/). The tooling that creates the branch strips `/` and refuses a second rename, so the slashed form is unreachable from this session. **Recommendation for the maintainer, not changed here.** `Test` is now a check run named after the job and can be required by a ruleset. [Gate merges with a named status check](https://msxorg.github.io/docs/Coding-Standards/GitHub-Actions/#gate-merges-with-a-named-status-check) says a result that should block merge has to be a required named check — and this branch demonstrates why: `main` merged two pull requests into a failing state because no check held them. Making it required is a repository setting, so it is left to the maintainer. If it is added, the name `Test` is the contract — renaming the job later would silently drop the gate. **Implementation plan progress.** #131's plan is complete, including the fifth item added while implementing it (prove the check can go red). The issue description was updated and the change recorded in [a comment](#131 (comment)). **Standards and framework alignment.** | Changed surface | Standards checked | Framework docs checked | Result | | --- | --- | --- | --- | | `.github/workflows/Docs.yml` | GitHub Actions (SHA pin, least privilege, pinned runner, named job and steps, named status check) | None (no framework-specific docs) | Aligned | | `.github/scripts/Invoke-PesterSuite.ps1` | PowerShell Scripts, Functions, Naming, Requires-Modules, Error Handling; Dependencies; GitHub Actions logging and diagnostics | None (no framework-specific docs) | Aligned — with the action-extraction deviation recorded above | | `tests/Test-DocumentationLink.Tests.ps1` | Testing (the oracle must be independent; prove the test can fail) | PowerShell Testing | Fixed in this PR | | `CONTRIBUTING.md` | Markdown, Natural Language | Contribution Workflow | Aligned | **Issue convergence sweep.** Scope: all 17 open issues in MSXOrg/docs, filtered to those touching CI, testing, or `tests/`. Only #131 is fully satisfied by this diff. #136 was opened by this work and is not closed by it. </details> <details> <summary>Relevant issues (or links)</summary> - Fixes #131 - #136 </details> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Marius Storhaug (MariusStorhaug)
added a commit
to Storhaug-ting/Kilden
that referenced
this pull request
Aug 2, 2026
…erged (#2) A table of contents can reproduce every word of the original correctly and still lead nowhere. That is exactly what happened here: 52 dead anchors, while `Update-Source.ps1` reported a match and full coverage. That check measures whether the text is reproduced faithfully — not whether the links inside it work. A second check now measures the second thing. ## New: Links and anchors are checked `./scripts/Test-MarkdownLink.ps1` walks every markdown file, checks that relative links hit a file that exists, and that every anchor matches a heading. It runs as a job of its own on every pull request, beside the source check, so the name of the red job says which of the two measurements failed. It needs nothing installed. `pwsh` is already on the runner image and already the repository's toolchain, so the check has no setup step. ## Technical Details The oracle has to come from outside the conversion. `Convert-PdfToMarkdown.py` has a `slugify` of its own, and a check built on it would only confirm itself — green no matter how the function behaved. So the anchor algorithm is written against [`github-slugger`](https://github.com/Flet/github-slugger), the library GitHub uses, and then verified against it instead of trusted: - **Every heading in the repository.** All 115 rendered headings were fed through both implementations. Zero mismatches. - **Every Unicode code point.** The .NET character class was compared against github-slugger's generated table across the whole range. They agree exactly over Basic Latin, Latin-1, Latin Extended-A and B, Greek, Cyrillic, General Punctuation, currency, letterlike and number forms, and the emoji planes. They disagree on 52 code points in the arrows and symbols blocks and 3 in CJK, where the two Unicode versions classify a character differently. That limit is written into the script's help rather than left implicit. - **Duplicate headings**, including the awkward case where a heading's own text collides with a generated `-1` suffix. Both produce `same`, `same-1`, `same-2`, `same-1-1`. - **The bug that shipped.** Run against the markdown as it stood before `a2654c7` fixed the anchors: 54 broken, exit code 1. Run against today's files: 116 links in 4 files, all resolving, exit code 0. ### Two ways this check could have lied, both closed Using the check to validate another pull request exposed the first one. The dot-directory exclusion was matched against the **full** path, and every worktree this app creates lives under `.copilot` — so the filter excluded every file in the repository. The run reported `0 link(s) checked in 0 file(s)` and exited 0. Green because it found nothing, indistinguishable from green because everything resolved. CI would never have caught it: the runner workspace has no dotted segment, so the check was green there and green locally for opposite reasons. The exclusion is now matched against the path below the root. That fixes the cause; the second commit closes the class. A run that found no markdown files now exits 1 — every link resolving is trivially true when there are no links, so an empty run is a failure rather than a pass. The same rule was adopted for the documentation link check in MSXOrg/docs#134. ### On the toolchain Written in PowerShell rather than Node. An earlier revision of this pull request added a package manifest, a lockfile, a `node_modules` ignore, a Node setup step, and a dependency, all to reach one library. The independence argument justifies not reusing the conversion's `slugify`; it does not require a JavaScript runtime. Using `github-slugger` as the reference during development, and checking against it, gets the same fidelity with no third toolchain in the repository. The approach follows [`Test-DocumentationLink.ps1`](https://github.com/MSXOrg/docs/blob/main/.github/scripts/Test-DocumentationLink.ps1) in MSXOrg/docs, adapted from the site's slug rules to GitHub's. Two things the earlier revision would have got wrong, now handled: a heading containing inline code (backticks are dropped, their content is kept, so `` ## The `slug` function `` anchors as `the-slug-function`), and reference-style link definitions, which are validated too — while footnote definitions (`[^1]: …`) are not, since their text is prose. That last distinction was missing from the MSXOrg script and is fixed upstream in MSXOrg/docs#133. The workflow file is now in English, along with the job and step names, per `AGENTS.md`: the sources keep the publisher's language, everything around them is English. The README section describing the new check is English for the same reason; the rest of that file is translated in its own pull request. **Worth knowing before this is marked ready:** GitHub Actions is disabled at repository level, so no workflow here has ever run — not this one and not the source check already on `main`. Until that setting is turned on, this pull request describes a gate rather than providing one. <details> <summary>Related issues</summary> - Fixes #7 - #5 - Storhaug-ting/S62#7 </details> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A documentation page that uses footnotes no longer fails the documentation link check. Footnote text is read as prose, so a page can cite sources without the check reporting its own sentences as broken links.
Fixed: Footnotes no longer break the link check
Previously, a page containing a footnote definition failed
Test-DocumentationLink.ps1, and the reported "broken link" was the first word of the footnote itself:The check now ignores the body of a footnote definition. Everything else is unchanged: reference-style link definitions such as
[label]: ./Real.mdare still validated and a broken one is still reported, and a real Markdown link written inside footnote prose is still checked.The footnote text above passes, and the link to
./Sources.mdinside it is still verified.Technical details
.github/scripts/Test-DocumentationLink.ps1:$refDefPatterngains a negative lookahead on the label —'^\s*\[(?!\^)[^\]]+\]:\s+(<[^>]+>|\S+)'. The old label class[^\]]+accepted the footnote label^1, so the footnote body reachedGet-LinkTargetIssueand its first whitespace-delimited token was validated as a relative path. The comment above the pattern and the.DESCRIPTIONignore list are updated to record why footnote definitions are excluded.$linkPatternis deliberately unchanged. It requires](, so a bare footnote reference such as[^1]in running text was never matched — this is now asserted by a test rather than assumed, and leaving it alone is what keeps an inline link inside footnote prose validated.tests/Test-DocumentationLink.Tests.ps1is new. It followstests/Update-DocumentationIndex.Tests.ps1: a temporary fixture repository holding a copy of the script under.github/scriptsand pages undersrc/docs, invoked out of process so assertions are on the real exit code and output. Six tests cover the footnote definition, a valid reference-style definition, a broken reference-style definition, a broken inline link inside footnote prose, a valid inline link inside footnote prose, and a bare footnote reference in running text.Broken documentation links (2)for the footnote-with-inline-link case. The two reference-style definition tests pass both before and after, which is what shows the false positive was removed rather than the feature disabled.tests/has no CI job at all, which contradicts Testing § Tests run in CI; that is filed as Run the Pester test suite in CI on every pull request #131 rather than absorbed here, because it is CI wiring across all four suites and not part of this correction..github/scripts/**(PowerShell)tests/**(Pester).github/workflows/**Verification, all on PowerShell 7.6.4:
Invoke-Pester ./tests— 51 passed, 0 failed../.github/scripts/Test-DocumentationLink.ps1against the live docs —All documentation links resolve., exit 0.Invoke-ScriptAnalyzer -Path . -Recurse -Settings .github/linters/.powershell-psscriptanalyzer.psd1— no findings at Error or Warning.Deviation worth noting: the branch is
fix-130-footnote-link-check, notfix/130-footnote-link-check. The session's branch-rename tool strips the/and may only be used once, so the slash could not be restored.Related issues