feat(FR-3773): validate WEBUIHelpButton manual anchors in verify.sh - #9269
Open
yomybaby wants to merge 3 commits into
Open
feat(FR-3773): validate WEBUIHelpButton manual anchors in verify.sh#9269yomybaby wants to merge 3 commits into
yomybaby wants to merge 3 commits into
Conversation
The header's "?" button opened a hard-coded manual `page#anchor` picked from
two literal tables inside WEBUIHelpButton.tsx. Nothing tied those tables to
the manual, so renaming a heading in packages/backend.ai-webui-docs silently
turned the button into a no-op scroll — the page still loaded, the anchor just
resolved to nothing.
Data move. URLMatchingTable and TabMatchingTable become one flat, typed list
in react/src/helper/helpAnchors.json ({ path, tab?, docPage, anchor? }), read
by react/src/helper/helpAnchors.ts. The button now calls
`resolveHelpDocPath(menuKey, activeTab)`; lookup order is unchanged (a
tab-specific entry wins, then the page-level entry, then the index page), and
helpAnchors.test.ts pins that behaviour. JSON rather than a TS literal so the
checker can read the data with plain `node`, no transpile step.
The gate. scripts/check-help-anchors.mjs resolves every entry against the
ENGLISH manual sources: chapter slugs come from book.config.yaml's `navigation`
paths, in-page ids from each chapter's headings and its explicit `<a id="…">`
markers. It reports both dead anchors and docPage values that map to no source
file, and exits 1. verify.sh runs it as "Help anchors (user manual)" next to
the other hard gates.
backend.ai-docs-toolkit owns the slug rules but is TypeScript with no build
output in this workspace, so `slugify`, `slugFromNavPath`, `stripHtmlTags` and
`decodeHtmlEntities` are ported line-for-line from markdown-processor.ts /
markdown-extensions.ts. check-help-anchors.test.ts pins that parity against
real headings from the manual, plus a deliberately broken entry and an unknown
page.
Anchors fixed: one, the known dead one. `credential?tab=credentials` pointed at
`admin_menu.html#admin_menu-manage-user39s-keypairs`, a pre-FR-2825 id from
before the toolkit decoded `'` ahead of slugifying; the live heading
"Manage user's keypairs" is `admin_menu-manage-users-keypairs`. No entry had to
be dropped — every other page and anchor still resolves.
CI teeth. verify.sh is the local/agent harness and no workflow runs it, so the
gate is also wired into the two workflows that fire on the inputs that can
break it: docs-checks.yml runs the checker on any manual PR, and
vitest-root.yml's path filter gains helpAnchors.json so check-help-anchors.test.ts
resolves the table whenever it is edited.
The `slugFromNavPath` port now really is line-for-line: it was missing the
toolkit's dash-collapse guard, so a nav path with `--` would have keyed the page
`foo--bar.html` while the site serves `foo-bar.html`. The empty-basename throw is
restored too, and the reserved `index` home slug is rejected in buildManualIndex
(where website-generator.ts checks it), not inside the slug function.
collectAnchorIds also indexes setext headings, which the web build gives ids —
missing them made the checker hard-fail a live anchor.
helpAnchors.ts keyed its tab lookup with a literal NUL in a template string,
which made the whole module a binary blob to git and grep. The two tables are a
nested Map now, so the file is text and reviewable.
Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_018BG9zkxxCdqjzDgBgfhXtW
Contributor
Coverage Report for root-coverage
File CoverageNo changed files found. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
Coverage Report for react-coverage (./react)
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
Contributor
There was a problem hiding this comment.
Pull request overview
Extracts help-anchor mappings into validated shared data and adds CI safeguards against dead manual links.
Changes:
- Adds typed help-anchor resolution backed by JSON.
- Adds anchor validation and regression tests.
- Integrates validation into local and CI checks.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
scripts/verify.sh |
Adds the help-anchor gate. |
scripts/check-help-anchors.mjs |
Validates manual pages and anchors. |
scripts/check-help-anchors.test.ts |
Tests checker behavior and slug rules. |
react/src/helper/helpAnchors.json |
Stores route-to-manual mappings. |
react/src/helper/helpAnchors.ts |
Provides typed mapping resolution. |
react/src/helper/helpAnchors.test.ts |
Tests lookup precedence and data shape. |
react/src/components/WEBUIHelpButton.tsx |
Uses the extracted resolver. |
.github/workflows/vitest-root.yml |
Triggers tests for mapping changes. |
.github/workflows/docs-checks.yml |
Validates links after documentation changes. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
…tions The parity tests only compared the checker's copied slug rules against hard-coded outputs, and the "real headings" test built its index with that same copy, so a change to the canonical toolkit logic left both green. The new "parity with the docs toolkit source" suite imports `slugify` / `slugFromNavPath` from `packages/backend.ai-docs-toolkit/src/markdown-processor.ts` and `stripHtmlTags` / `decodeHtmlEntities` from `markdown-extensions.ts` directly (vitest transpiles the toolkit TypeScript that the plain-node checker cannot load) and compares every port — and the composed heading-id pipeline — against the toolkit over every ATX/setext heading and every navigation path of the English manual, plus the marked-escaped form of each heading so the entity-decoding branch runs on real input. The corpus must be non-trivial (> 20 paths, > 100 headings, at least one `'` and one `&`) so an empty corpus cannot pass silently. The hard-coded cases stay; the file and checker header comments now describe the suite accurately. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01HCRVJAbdPPFsqDM4BXxis3
yomybaby
marked this pull request as ready for review
August 31, 2026 03:07
CodeQL (default setup) failed the required check on this PR with its one new
high-severity alert:
check-help-anchors.mjs:47 js/incomplete-multi-character-sanitization,
"This string may still contain <script"
The query is syntactic about the strip itself: any single-pass
`.replace(/<[^>]+>/g, "")` is reported, regardless of what happens
downstream. Here nothing can survive anyway -- the next `.replace` drops
every character that is not a letter, digit, whitespace or dash, and the
output is used as a URL fragment id, never interpolated into HTML. So this
was never an injection path; the code shape is what has to change.
`stripTagSpans` restates that regex's match rule as an index scan: a match
can only start at `<`, `[^>]+` can never cross a `>`, so the span ends at
the first `>` after the `<` and is dropped only when that `>` is at least
two characters away (`[^>]+` cannot match empty). Removing the regex clears
the alert -- the query anchors on the StringReplaceCall -- and incidentally
removes a genuine quadratic: '<'.repeat(50000) went from 3649 ms to 0.01 ms.
This is the same helper, with the same equivalence argument, that
FR-3762 applies to the `slug.ts` port of the same function, so the two ports
stay recognisably identical. Equivalence was proven, not assumed: 0 output
differences over an exhaustive enumeration of all 797,161 strings up to
length 12 on {<,>,a}, all 120 manual files / 1,504 headings / 30,577 lines
in every language, and 400k fuzz inputs.
The toolkit original is untouched -- and check-help-anchors.test.ts, which
pins these ports against the toolkit's own functions over every heading and
navigation path of the manual, still passes 33/33.
Verified: 33/33 pinning tests, `node scripts/check-help-anchors.mjs` resolves
all 59 entries against 28 pages, prettier.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Y3Ky5jYEW7Uk4ybDKnCxoe
agatha197
requested changes
Aug 31, 2026
agatha197
left a comment
agatha197
left a comment
Contributor
There was a problem hiding this comment.
please resolve conflicts
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.
Resolves #9262 (FR-3773)
Part of the bai-agent page→feature map plan (epic FR-3740, step A): make the page→manual link map a validated data source.
What changed
WEBUIHelpButton's two inline tables (URLMatchingTable,TabMatchingTable) move intoreact/src/helper/helpAnchors.json+ a typedhelpAnchors.ts(resolveHelpDocPath(menuKey, activeTab)); the button's behaviour is unchanged (helpAnchors.test.tspins the lookup order).scripts/check-help-anchors.mjsresolves everydocPage#anchoragainst the English manual sources using a line-for-line port of the docs toolkit's slug rules (slugify/slugFromNavPath, setext + ATX headings, explicit<a id>markers); parity is pinned byscripts/check-help-anchors.test.tsagainst real headings and the toolkit's own functions.scripts/verify.shgains a hardHelp anchors (user manual)gate;docs-checks.ymlruns the checker in CI andvitest-root.ymlwatches the data file.admin_menu-manage-user39s-keypairs→admin_menu-manage-users-keypairs(the toolkit decodes'before slugifying). Every other entry resolves.Verification
node scripts/check-help-anchors.mjs→Checked 59 help-anchor entries against 28 en manual pages.(exit 0); a deliberately broken entry exits 1 with the dead target listed.src/helper581 passed.bash scripts/verify.sh→=== ALL PASS ===🤖 Generated with Claude Code
https://claude.ai/code/session_018BG9zkxxCdqjzDgBgfhXtW
Follow-up (2026-08-31, review):
scripts/check-help-anchors.test.tsnow imports the toolkit source (slugify,slugFromNavPath,stripHtmlTags,decodeHtmlEntities) and compares the ports over every English manual heading (357, raw and marked-escaped) and navigation path (28); the earlier version only checked hard-coded outputs.pnpm exec vitest run scripts/check-help-anchors.test.ts33/33.