Repository navigation
Conversation
…ommit The line-based porcelain format C-quotes paths containing spaces or special characters (e.g. `?? "new dir/file.txt"`), and the quoted strings failed readFileSync, so those files were silently skipped from the signed commit (PR review: repo-status.ts:122). Switch to -z (NUL-delimited, never quoted or escaped) and reconstruct the 'original -> new' composite for renames, so every readable file commits regardless of its name. The skip-with-warning fallback stays for genuinely unreadable entries (directories, broken symlinks). Adds a regression test with spaces and a double quote in file names.
…rtifacts Mirror reusable-docusaurus: expose node-version-file and pnpm-version as workflow_call inputs (auto-detect / file-driven defaults) instead of hardcoding them at three setup-nodejs call sites. Resolves the review thread asking this reusable workflow to match reusable-docusaurus; behavior is unchanged for existing callers.
resolve-versions.test.sh covers the script in isolation, but nothing exercised the full action wiring (mise-action, resolve script, setup-node, pnpm/action-setup, PATH ordering) on a real runner — the exact surface that broke CI when .tool-versions was deleted. Matrix over mise / tool-versions / nvmrc / no-files / mise-precedence fixtures with versions that differ from the fallbacks, asserting on the installed node and pnpm versions.
kalverra
added this pull request to stack #1694
October 14, 2026 20:55
Contributor
🦋 Changeset is good to goLatest commit: 92d9d5d We got this. Not sure what this means? Click here to learn what changesets are. |
kalverra
changed the title
kalverra
marked this pull request as ready for review
October 14, 2026 20:56
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const repoPath = await createRepo(name); | ||
|
|
||
| fs.mkdirSync(`${repoPath}/new dir`); | ||
| fs.writeFileSync(`${repoPath}/new dir/old file.txt`, "Old content"); |
| ); | ||
|
|
||
| // untracked paths whose names trigger C-quoting in the line-based format | ||
| fs.writeFileSync(`${repoPath}/new dir/untracked file.txt`, "Untracked"); |
|
|
||
| // untracked paths whose names trigger C-quoting in the line-based format | ||
| fs.writeFileSync(`${repoPath}/new dir/untracked file.txt`, "Untracked"); | ||
| fs.writeFileSync(`${repoPath}/quote"name.txt`, "Quoted name"); |
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
2 participants
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.
Review fixes from the DX-5569 dependency-modernization stack review. Refs DX-5569. Builds on #1695.
Why
The stack review surfaced two unresolved review threads, a correctness bug in
signed-commitsporcelain parsing, and a class of integration bug invisible to the existing unit tests.Notes
signed-commitsnow readsgit status --porcelain=v1 -z(NUL-delimited, never quoted) instead of the line-based format, which C-quotes paths containing spaces or special characters (e.g.?? "new dir/file.txt"). Those quoted strings failedreadFileSync, so such files were silently skipped from the signed commit — resolves the review thread on [DX-5569] Dependency Updates! #1688 (repo-status.ts:122). The skip-with-warning fallback remains only for genuinely unreadable entries (directories, broken symlinks).test-setup-nodejs.ymlexercises the full action wiring (mise-action →resolve-versions.sh→ setup-node → pnpm/action-setup → PATH) on a real runner over a fixture matrix.resolve-versions.test.shcovers the script in isolation; nothing covered the runner integration — the exact surface that broke feat: asdf -> mise #1695's CI.solidity-review-artifacts.ymlnow exposesnode-version-file/pnpm-versionas workflow-call inputs mirroringreusable-docusaurus, instead of hardcoding them at three call sites — resolves the review thread on [DX-5569] Dependency Updates! #1688. Behavior is unchanged for existing callers.Changes
signed-commitsparses status with-zgetGitStatusPorcelainV1+ parser rewritten for NUL-delimited records; rename records (new\0old) reconstruct theoriginal -> newcomposite downstream code expects.Runner-level matrix test for
setup-nodejs.tool-versions(mise wins),.tool-versions,.nvmrc, no files (pnpm^12fallback). Fixture versions (node 24.14.1 / pnpm 10.6.5) differ from fallbacks so assertions prove provenance.solidity-review-artifactsnode/pnpm inputsnode-version-file/pnpm-versionworkflow-call inputs (auto-detect / file-driven defaults); the threesetup-nodejscall sites now pass them through.Merge Danger
-zfix is one module + its test + dist.signed-commitsdist changes the signed-commit path for every consumer; new workflow is repo-only.