test: cover recent v1.x fixes - #1481
Merged
Merged
Conversation
The npm-publish workflow pins relative README assets/ refs to the release tag's raw-GitHub URL before packing (1c167f6), because registry pages render the README from the tarball where relative refs 404. Nothing exercised that script. Run .github/scripts/pin-readme-assets.mjs against temp-dir fixtures and assert the three rewrite forms (img src, srcset, markdown image), that the tag comes from package.json's version, that absolute refs are left alone, that a second run is a no-op, and that a mixed README is pinned in one pass with the base reported.
The pin script matched raw markdown with string patterns, so an assets/ ref inside a fenced code block or inline code span got rewritten even though registry pages render it as literal text, and reference-style definitions ([ref]: assets/…) were never pinned. Parse the README with mdast-util-from-markdown and derive every edit from parser-reported byte offsets: image, link, and definition nodes whose url starts with assets/ get the release-tag base inserted at the url's own range, and raw-HTML nodes are re-parsed with parse5 source locations so only real src/srcset attribute values are touched. The document is never round-tripped through a serializer, so untouched bytes stay byte-identical. Contract is unchanged: version from package.json, absolute refs untouched, idempotent, same no-op message. Both parsers are devDependencies (mdast-util-from-markdown 2.0.3, parse5 8.0.1); the script runs in CI after install and ships nothing. Tests grow from 7 to 12: reference definitions, blockquote and list refs, fenced-code and inline-code lookalikes staying as written, and a mixed document pinning the real ref while leaving the fence alone.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
GitHub and npm render the README as GFM, so the pin script now parses with micromark-extension-gfm + mdast-util-gfm and the tree matches what those pages actually show: tables, footnotes, strikethrough, and task lists parse as their real constructs instead of falling back to CommonMark paragraphs. For assets/ pinning the output is unchanged — inline refs inside those constructs already parsed at the same byte offsets — so this is about render fidelity as the script grows. A new test pins a ref inside a GFM table cell and a footnote definition.
The changelog promote located the [Unreleased] block, the next release heading, and the insertion point by scanning raw lines for '## ', and counted entries with a '- ' pattern. A '## ' or '- ' line inside a fenced code block read as structure: the promote could truncate the Unreleased block at a fence line, and a bullet lookalike in shell output counted as a real entry and suppressed the derived section. Structure now comes from the same GFM mdast parse the README pin uses: level-2 headings from parser positions, entries from real listItem nodes. Content inside code fences is content. The exported API and the promoted output for well-formed changelogs are unchanged; the two new tests fail on the line-scan version and pass here.
The entry-scripts block spawns bare 'node <entry>.mts', which only works where the runtime has native type stripping (Node 22.6+, reported by process.features.typescript). On Node 20 the spawn dies with ERR_UNKNOWN_FILE_EXTENSION before any script code runs, which made the whole block structurally red on that CI lane while 22/24 passed. Skip the spawn assertions where the runtime cannot execute .mts at all — these entries are maintainer tooling aimed at the pinned dev Node, while older lanes exist to cover the built product. The spawn assertions now also carry the child's stderr as the assertion message, so the next environmental break names itself instead of reading 'expected 1 to be +0'.
Node 20 reached end-of-life on 2026-03-24. The unit and e2e matrices now test the supported majors, 22 and 24. Whether the shipped CLI keeps claiming Node 20 support in engines is a separate product decision and is deliberately not changed here.
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
1 participant
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.
LLM Description written by Claude Code:claude-fable-5
Audit of every fix commit on v1.x since v1.1.150 for unit-test coverage of the fixed behavior, plus an owner-directed rework of the pin script itself.
test/pin-readme-assets.test.mtssrc/utils/npm-paths.test.mts(same commit)src/utils/glob-content-dedup.test.mts(same commit)handle-ci.test.mts,git.test.mts(same commit)src/shadow/npm-base.test.mts(same commit)test/dist-runtime-requires.test.mts(0c3fea0)github-errors.test.mts,create-scan-from-github.test.mts(same commit)test/script-update.test.mts(same commit)Second commit (a38c4d9) reworks
.github/scripts/pin-readme-assets.mjsfrom string patterns over raw markdown to a parser-driven pass: the README is parsed withmdast-util-from-markdownto a position-tracked mdast tree, image/link/definition nodes whose url starts withassets/get the release-tag base inserted at the parser-reported byte offset, and raw-HTML nodes are re-parsed with parse5 source locations so only realsrc/srcsetattribute values are touched. No serializer round-trip — untouched bytes stay byte-identical. Newly correct behavior, each with a test:assets/inside fenced code blocks or inline code spans stays as written (the string-pattern version rewrote it), and reference-style definitions are now pinned. Contract otherwise unchanged (version from package.json, absolute refs untouched, idempotent, same no-op message). Both parsers are devDependencies; the script runs in CI after install and ships nothing.Third commit (19707b4) parses with the GFM extensions (
micromark-extension-gfm+mdast-util-gfm) so the tree matches how GitHub and npm actually render the README; a new test pins refs inside a GFM table cell and a footnote definition.Fourth commit (b648566) ports the changelog promote (
scripts/release/changelog.mts) to the same GFM mdast parse: the[Unreleased]block range, the insertion point, and the has-entries check now come from parser-reported heading positions and reallistItemnodes instead of##/-line scans. Two new tests cover what the scan version got wrong — a##line inside a code fence no longer truncates the promoted block, and a bullet lookalike in fenced shell output no longer counts as an entry. Both fail on the line-scan implementation.Tests: 13 in
test/pin-readme-assets.test.mts(all spawning the real script against temp-dir fixtures) plus 2 new promoteChangelog cases intest/release-version.test.mts.Fifth and sixth commits fix the red Node 20 lanes:
test/script-run-main.test.mts:222failure: the block spawns barenode <entry>.mts, and Node 20 dies withERR_UNKNOWN_FILE_EXTENSIONbefore any script code runs — native TypeScript type stripping only exists from Node 22.6. The spawn assertions are now gated onprocess.features.typescript(with a why-comment) and carry the child's stderr as the assertion message so the next environmental break names itself. Under Node 20 the file reports 27 passed / 5 skipped; under newer Node all 32 pass.Open question for the maintainers: package.json
enginesstill claimsnode >=18.20.8. Whether the shipped CLI keeps claiming EOL Node 18/20 support is a product decision deliberately not made here.e2e flake note: the
cmd-fix.e2e.test.mts"django should be upgraded" failure is a known backend flake, not lane-specific — it hit the Node 20 lane on run 31068153685 (22/24 green) and the Node 22 lane on run 31069849967 (24 green), same assertion both times, and passed on rerun.Pre-existing local-only failure (not touched here):
src/commands/optimize/cmd-optimize.test.mts"should handle npm projects with cwd correctly" fails on a machine with npm 12.0.1 ("npm install failed to update package-lock.json") on an untouched origin/v1.x checkout; CI Linux lanes pass it.Full unit suite locally: 1692 passed, 2 skipped, 1 failed (the pre-existing local-only optimize failure above). All PR checks green as of c1e841c.
Note
Low Risk
Changes affect publish-time README rewriting and release changelog promotion plus CI matrix scope; behavior is tightened with broad tests and does not touch shipped CLI runtime paths.
Overview
Replaces string-replace README asset pinning with a GFM mdast + parse5 pass that inserts the release-tag base only at parser-reported offsets, so
assets/in code fences or inline code is not rewritten and reference-style definitions, tables, and footnotes are handled correctly. Addstest/pin-readme-assets.test.mts(13 cases) that run the real publish script against temp fixtures.scripts/release/changelog.mtsuses the same GFM parse for[Unreleased]bounds, insertion point, and “has entries” (reallistItemnodes), fixing false positives/negatives from##/-line scans; two newpromoteChangelogtests cover code-fence edge cases.CI unit and e2e matrices drop Node 20 (EOL 2026-03-24);
test/script-run-main.test.mtsskips barenode *.mtsspawns whenprocess.features.typescriptis absent and surfaces spawn stderr on failure. New devDependencies:mdast-util-from-markdown,mdast-util-gfm,micromark-extension-gfm,parse5,@types/mdast.Reviewed by Cursor Bugbot for commit c1e841c. Configure here.