Repository navigation
build(docs): escape NUL stash placeholders in docs site builder - #1001
Conversation
scripts/build-docs-site.mjs carried four literal NUL bytes in the inline() renderer's inline-code stash placeholder (one template literal, one regex literal). The first sat at byte 13289, past Git's 8000-byte binary sniff, so Git still diffed the file as text, but the autoreview helper scans the whole file and refused every diff touching it as a binary change. Spell them as \u0000 escapes instead. Runtime strings and regex semantics are unchanged: the generated _site output is byte-identical across all 79 files. Add a standalone guard test that the docs-site sources contain no literal NUL bytes, run test:docs-site over tests/docs-site-*.test.mjs, and have macOS CI call pnpm run test:docs-site. Those two wiring edits match open PR #992 byte-for-byte, so either PR can land first without conflicts.
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
PR SummaryLow Risk Overview Adds Reviewed by Cursor Bugbot for commit bf49af9. Bugbot is set up for automated code reviews on this repo. Configure here. |
clawsweeper
Bot
added
P2
|
Codex review: needs maintainer review before merge. Reviewed October 7, 2026, 1:44 AM ET / 05:44 UTC. ClawSweeper reviewWhat this changesThe branch escapes literal NUL bytes in the documentation renderer, adds a source-byte regression guard, and includes all documentation-site tests in the shared command and macOS CI. Merge readiness✅ Ready for maintainer review This PR remains useful: current main and v4.8.0 retain the literal NUL bytes, and the related renderer PR does not replace this repair. No actionable correctness or security finding was identified. Priority: P2 Review scores
Verification
How this fits togetherPeekaboo’s documentation builder converts Markdown into the published HTML site. Its inline renderer temporarily replaces code spans with placeholders before restoring escaped code; CI checks the builder and its supporting sources. flowchart LR
A[Markdown documentation] --> B[Inline code placeholders]
B --> C[Text formatting and escaping]
C --> D[Restored code spans]
D --> E[Generated HTML site]
F[Source byte guard] --> G[Shared documentation tests]
G --> H[macOS CI]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Keep the renderer’s output unchanged while representing its placeholders as text escapes and guarding the source files against literal NUL bytes. Do we have a high-confidence way to reproduce the issue? Yes, source inspection establishes the byte-level defect: pinned main contains four literal NUL bytes and the branch contains none. The reported autoreview rejection was not independently executed. Is this the best way to solve the issue? Yes, escaping the existing values is a narrow semantics-preserving repair, and the guard integrates with the existing shared test command. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against a0df5621dcdc. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
1 participant
Why
scripts/build-docs-site.mjscarried four literal NUL bytes (0x00) in theinline()renderer's inline-code stash placeholder: one template literal and one regex literal. The first sits at byte 13289, past Git's 8000-byte binary sniff, so Git still diffed the file as text. The autoreview helper scans the whole file and refused every diff touching it as a binary change, which blocked reviews of the docs PRs #931, #939, #940 and #971.What
\u0000escapes. Runtime strings and regex semantics are unchanged.tests/docs-site-source-bytes.test.mjs, a guard that the docs-site sources (build-docs-site.mjs,docs-site-assets.mjs,docs-site-toc.mjs) contain no literal NUL bytes.test:docs-siteovertests/docs-site-*.test.mjs, and have macOS CI callpnpm run test:docs-siteso the new test runs there.The two wiring edits are byte-identical to the same edits in #992 (same resulting blobs). Simulated both landing orders locally: either way the merge is conflict-free, the builder ends with zero NULs, and all 25 docs-site tests pass.
Proof
diff -rover all 79 generated files is identical (869 inline<code>spans exercise the placeholder path).pnpm run test:docs-site: 3/3 pass.node scripts/build-docs-site.mjs: OK.git diff --numstatreports normal text lines; autoreview accepted the diff and returned scoped-clean.No changelog entry: internal tooling, no user-visible behavior change.