Repository navigation
Conversation
The docs check had no tests, so a refactor of packages/cli/src/cli.ts could make it pass vacuously. Extract the command/docs comparison into checkDocsCliCommands(cliSource, docsText) and cover the pass, missing command, missing command-finder section and missing commandLoaders cases with node:test. The CLI entry keeps the previous output and exit code, and the new test file runs in test:scripts.
…, record duplicate rule Review PR heygen-com#7304-heygen-com#7335: 10 merged already adopted or deferred; 10 open or draft PRs pinned (heygen-com#7304, heygen-com#7308, heygen-com#7311, heygen-com#7315, heygen-com#7321, heygen-com#7330-heygen-com#7334); open duplicates of pinned heygen-com#7281/heygen-com#7285/heygen-com#7286/heygen-com#7296 are tracked by those pins. strict exit 0; monitored 110 -> 120. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jrusso1020
left a comment
There was a problem hiding this comment.
Reviewed at 66d747e10b289ff2e0cf617da938c56d6d11ec65.
The existing docs checker is now importable without executing its CLI entry (scripts/check-docs-cli-commands.mjs:24,60). The tested missing/malformed command-loader declarations fail loudly, and the new tests are wired into the root runner. Compared with #7335, this modifies the actual CI script and uses the correct test location.
Validation at this head: node --test scripts/check-docs-cli-commands.test.mjs → 5 passed. Running the real entry prints All 55 CLI commands are listed in the docs. Fresh independent source review found no Important defect; no live browser or native Windows run. No prior substantive reviews or inline comments were present.
Need: yes, requested starter issue · Size: ok · Reuse: existing CLI policy, color and checker helpers · Simpler: already minimal · Structure: fine
Risk: low — bounded CLI output/checker change
— hf-review
Verdict: APPROVE
Reasoning: The requested behavior is covered by focused tests and reuses the existing contract. No Important blocker found.
Summary
scripts/check-docs-cli-commands.mjsruns in CI (docs.yml) and enforces that every CLI command is documented, but it has no tests — a refactor ofpackages/cli/src/cli.tscould make it pass vacuously.This extracts the comparison into
checkDocsCliCommands(cliSource, docsText), keeps the CLI entry behaviour identical (same message, same exit code), and addsscripts/check-docs-cli-commands.test.mjstotest:scripts.Change
scripts/check-docs-cli-commands.mjscheckDocsCliCommands(cliSource, docsText)→{ commands, missing }const commandLoaders = { ... };" error when the declaration or its closing brace is missing (previously a missing closing brace sliced to the end of the file silently)main()keeps the previous read/console/exit behaviour and runs only when the file is executed directlyscripts/check-docs-cli-commands.test.mjs— pass, missing command, missing command-finder section, missingcommandLoaders, unclosedcommandLoadersblockpackage.json— added the test file totest:scriptsVerification
node --test scripts/check-docs-cli-commands.test.mjs→ 5 pass / 0 failnode scripts/check-docs-cli-commands.mjs→All 55 CLI commands are listed in the docs.— same command count as the previous implementation (55 commands, 0 missing)bun run test:scriptslocally (it needs the full monorepo install); the new file uses the samenode --testrunner the existing entries use, so CI will cover the whole list.Fixes #7278
Prepared with AI assistance (implementation and tests); I ran the commands above locally and reviewed the diff.