refactor(scripts): build script hardening (SL-006, 008-013) - #477
Conversation
…, 012, 013) - SL-006 (highest-risk item in this wave): split gen-api-index.js's 829 lines along its existing section-comment boundaries into scripts/lib/api-index/extract.js (annotations, previous-index reading, banner/comment parsing, bundle mapping, token/class extraction and merging) and render.js (the Markdown companion). gen-api-index.js is now just path constants, the write-time regression guard, and orchestration in main(). One small behavior-neutral cleanup along the way: replaced the hoisted-closure `bundlesFor_` mutable-variable hack with a normal parameter, since extractClassesFromFile now takes bundlesFor directly. No logic changed otherwise - verified by diffing docs/api-index.json, docs/api-index.md, docs/token-index.json, and token-registry.json byte-for-byte against a pre-refactor snapshot (empty diff on all four, both from a single `node scripts/gen-api-index.js` run and from the full `npm run docs` pipeline). - SL-010 + SL-012: add regression guards before the three fs.writeFileSync sites that downstream tooling trusts (gen-api-index.js, gen-token-registry.js, gen-token-index.js) - a >20% entry-count drop (or, for the append-only token-registry.json, any drop at all) now throws instead of silently writing a truncated artifact. No new dependency; hand-rolled against each file's existing "read the previous version" logic. - SL-008/009/011/013: new scripts/README.md documenting the intentionally-unenforced error-handling convention, the hand-rolled CLI flag parsing, the 3 environment variables the scripts read (SLASHED_ROOT, GITHUB_REF/GITHUB_REF_NAME, GITHUB_BASE_REF), and the execSync(shell:true) pattern in check-artifacts.js/changelog-release.js (config-sourced today, not attacker input, but worth flagging for future artifacts.json entries). Verified: npm run docs before/after diff empty on every generated artifact; check:macros, check:registry, check:llm-guide, audit:check, lint:css, build, test:unit (44/44) all pass.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)❌ Error creating Unit Test PR.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Summary by QodoHarden docs build scripts: split API index generator and add regression guards
AI Description
Diagram
High-Level Assessment
Files changed (6)
|
|
Note Unit test generation is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
Request timed out after 900000ms (requestId=4016eea8-7ecb-4c47-8615-b12f8eb7b006) |
6310621
into
claude/pr-469-audit-rebase-ggp0e4

PR3 from the technical-debt audit remediation plan (#469) — the plan's own "highest technical risk" item in this wave. Targets the integration branch, same stacking model as PR1/PR2.
Findings addressed
gen-api-index.js's 829 lines along its existing section-comment boundaries intoscripts/lib/api-index/extract.js(annotations, previous-index reading, banner/comment parsing, bundle mapping, token/class extraction and merging) andrender.js(the Markdown companion).gen-api-index.jsis now just path constants, the write-time regression guard, and orchestration inmain(). One small behavior-neutral cleanup along the way: replaced a hoisted-closure mutable-variable hack (bundlesFor_) with a normal function parameter — same value, same call order, just less confusing to read.fs.writeFileSyncsites that downstream tooling trusts (gen-api-index.js,gen-token-registry.js,gen-token-index.js). A >20% entry-count drop (or, for the append-onlytoken-registry.json, any drop at all) now throws instead of silently writing a truncated artifact.scripts/README.mddocumenting the (intentionally unenforced) error-handling convention, the hand-rolled CLI flag parsing, the 3 environment variables the scripts read, and theexecSync(shell:true)pattern.Verification (this is the load-bearing check for SL-006)
Diffed
docs/api-index.json,docs/api-index.md,docs/token-index.json, andtoken-registry.jsonbyte-for-byte against a pre-refactor snapshot — empty diff on all four, both from a standalonenode scripts/gen-api-index.jsrun and from the fullnpm run docspipeline.npm run check:macros,check:registry,check:llm-guide,audit:check— all passnpm run lint:css,npm run build— cleannpm run test:unit— 44/44 passA process note
While verifying this PR I hit the exact bug Qodo's review just flagged on PR #476:
npm run test:unitfails ifbadges/isn't freshly built, and that's not obvious from the script name. I'll fix that documentation gap over on #476.Generated by Claude Code