diff --git a/.changeset/signed-commits-porcelain-z.md b/.changeset/signed-commits-porcelain-z.md new file mode 100644 index 000000000..06c75da3e --- /dev/null +++ b/.changeset/signed-commits-porcelain-z.md @@ -0,0 +1,5 @@ +--- +"changesets-signed-commits": patch +--- + +`commitAll` now reads git status with `--porcelain=v1 -z` (NUL-delimited) instead of the line-based format. The line format C-quotes paths containing spaces or special characters (e.g. `?? "new dir/file.txt"`), and those quoted strings failed `readFileSync`, so such files were skipped from the signed commit. With `-z` paths are emitted raw and unquoted, so every readable file is committed regardless of its name. The skip-with-warning fallback remains only for genuinely unreadable entries (directories, broken symlinks). Rename parsing (`original -> new`) is unchanged. diff --git a/.github/workflows/solidity-review-artifacts.yml b/.github/workflows/solidity-review-artifacts.yml index a2c34b512..91728489f 100644 --- a/.github/workflows/solidity-review-artifacts.yml +++ b/.github/workflows/solidity-review-artifacts.yml @@ -107,6 +107,20 @@ on: type: boolean required: false default: true + node-version-file: + description: + "path to a version file for node (mise.toml, .tool-versions, .nvmrc); + empty auto-detects" + type: string + required: false + default: "" + pnpm-version: + description: + "pnpm version; empty reads it from mise.toml/.tool-versions, falling + back to ^12.0.0" + type: string + required: false + default: "" secrets: jira_username: required: false @@ -216,8 +230,8 @@ jobs: uses: smartcontractkit/.github/actions/setup-nodejs@0b8d877a32bfa479b62ce70c1f6d2493a7c93fed # v0.0.0 with: package-json-directory: ${{ inputs.contracts_directory }} - node-version-file: "" - pnpm-version: ^12.0.0 + node-version-file: ${{ inputs.node-version-file }} + pnpm-version: ${{ inputs.pnpm-version }} - name: Create code coverage directory if: ${{ inputs.generate_code_coverage == true }} @@ -327,8 +341,8 @@ jobs: uses: smartcontractkit/.github/actions/setup-nodejs@0b8d877a32bfa479b62ce70c1f6d2493a7c93fed # v0.0.0 with: package-json-directory: ${{ inputs.contracts_directory }} - node-version-file: "" - pnpm-version: ^12.0.0 + node-version-file: ${{ inputs.node-version-file }} + pnpm-version: ${{ inputs.pnpm-version }} - name: Install Foundry uses: foundry-rs/foundry-toolchain@8f1998e9878d786675189ef566a2e4bf24869773 # v1.2.0 @@ -493,8 +507,8 @@ jobs: uses: smartcontractkit/.github/actions/setup-nodejs@0b8d877a32bfa479b62ce70c1f6d2493a7c93fed # v0.0.0 with: package-json-directory: ${{ inputs.contracts_directory }} - node-version-file: "" - pnpm-version: ^12.0.0 + node-version-file: ${{ inputs.node-version-file }} + pnpm-version: ${{ inputs.pnpm-version }} - name: Setup git top level directory if: ${{ inputs.link_with_jira == true }} diff --git a/.github/workflows/test-setup-nodejs.yml b/.github/workflows/test-setup-nodejs.yml new file mode 100644 index 000000000..1d85404ce --- /dev/null +++ b/.github/workflows/test-setup-nodejs.yml @@ -0,0 +1,89 @@ +name: Test setup-nodejs + +on: + pull_request: + paths: + - "actions/setup-nodejs/**" + - ".github/workflows/test-setup-nodejs.yml" + - "mise.toml" + - ".tool-versions" + workflow_dispatch: + +permissions: {} + +jobs: + resolve-versions: + name: resolve (${{ matrix.scenario }}) + runs-on: ubuntu-latest + permissions: + contents: read + strategy: + fail-fast: false + matrix: + # Fixture versions deliberately differ from the fallbacks (node + # 24.14.1 / pnpm 10.6.5 vs pnpm ^12) so assertions prove the version + # came from the fixture, not from a default. + scenario: + - mise-bare + - mise-beats-tool-versions + - tool-versions + - nvmrc + - no-files-fallback + steps: + - name: Checkout repo (needed to reference local action) + uses: actions/checkout@v7 + with: + persist-credentials: false + + - name: Prepare fixture version files + shell: bash + run: | + set -euo pipefail + rm -f mise.toml .tool-versions .nvmrc + case "${{ matrix.scenario }}" in + mise-bare) + printf '[tools]\nnode = "24.14.1"\npnpm = "10.6.5"\n' > mise.toml + ;; + mise-beats-tool-versions) + printf '[tools]\nnode = "24.14.1"\npnpm = "10.6.5"\n' > mise.toml + printf 'nodejs 26.10.0\npnpm 12.9.1\n' > .tool-versions + ;; + tool-versions) + printf 'nodejs 24.14.1\npnpm 10.6.5\n' > .tool-versions + ;; + nvmrc) + printf '24.14.1\n' > .nvmrc + ;; + no-files-fallback) + : + ;; + esac + echo "--- fixture files:" + cat mise.toml .tool-versions .nvmrc 2>/dev/null || true + + - name: Run setup-nodejs + uses: ./actions/setup-nodejs + with: + use-cache: "false" + + - name: Assert resolved versions + shell: bash + run: | + set -euo pipefail + node_version="$(node -v)" + pnpm_version="$(pnpm -v)" + echo "resolved node: ${node_version}, pnpm: ${pnpm_version}" + case "${{ matrix.scenario }}" in + mise-bare|mise-beats-tool-versions|tool-versions) + [[ "${node_version}" == "v24.14.1" ]] + [[ "${pnpm_version}" == "10.6.5" ]] + ;; + nvmrc) + [[ "${node_version}" == "v24.14.1" ]] + [[ "${pnpm_version}" == 12.* ]] + ;; + no-files-fallback) + command -v node > /dev/null + [[ "${pnpm_version}" == 12.* ]] + ;; + esac diff --git a/actions/signed-commits/dist/index.js b/actions/signed-commits/dist/index.js index e9257c65f..6feb8a88b 100644 --- a/actions/signed-commits/dist/index.js +++ b/actions/signed-commits/dist/index.js @@ -62514,7 +62514,7 @@ async function getFileChanges(cwd) { async function getGitStatusPorcelainV1(cwd) { const stdout = await execWithOutput( "git", - ["status", "--porcelain=v1", "--untracked-files=all"], + ["status", "--porcelain=v1", "-z", "--untracked-files=all"], { cwd, notrim: true @@ -62551,16 +62551,27 @@ function listChanges(output) { }; } function parseGitStatusPorcelainOutput(output2) { - const lines = output2.split("\n"); - lines.pop(); - return lines.map((line) => { - const [status, filePath] = [ - line.substring(0, 2), - line.substring(2).trim() - ]; + const fields = output2.split("\0"); + const changes = []; + for (let i = 0; i < fields.length; i++) { + const field = fields[i] ?? ""; + if (field === "") { + continue; + } + const status = field.substring(0, 2); + let filePath = field.substring(3); const { indexStatus, workingTreeStatus } = parseStatusCode(status); - return { filePath, indexStatus, workingTreeStatus }; - }); + if (indexStatus === "R" || indexStatus === "C" || workingTreeStatus === "R" || workingTreeStatus === "C") { + const originalPath = fields[++i] ?? ""; + filePath = `${originalPath} -> ${filePath}`; + } + changes.push({ + filePath, + indexStatus, + workingTreeStatus + }); + } + return changes; } return parseGitStatusPorcelainOutput(output); } diff --git a/actions/signed-commits/src/git/github-git/repo-status.test.ts b/actions/signed-commits/src/git/github-git/repo-status.test.ts index fc1e2c270..7fc383f05 100644 --- a/actions/signed-commits/src/git/github-git/repo-status.test.ts +++ b/actions/signed-commits/src/git/github-git/repo-status.test.ts @@ -22,13 +22,7 @@ describe("repo-status", () => { const stdout = await getGitStatusPorcelainV1(repoPath); expect(stdout).toMatchInlineSnapshot(` - " M file1.txt - D file2.txt - R file3.txt -> file3renamed.txt - T file4.txt - A file5.txt - ?? untracked.txt - " + " M file1.txt\0 D file2.txt\0R file3renamed.txt\0file3.txt\0 T file4.txt\0A file5.txt\0?? untracked.txt\0" `); }); }); @@ -123,6 +117,36 @@ describe("repo-status", () => { expect(fileChanges.deletions).toEqual([{ path: "file2.txt" }]); }); }); + + describe("paths with spaces and special characters", () => { + // Regression test: the line-based porcelain format C-quotes these paths + // (e.g. `?? "new dir/untracked file.txt"`), and the quoted strings failed + // readFileSync. The -z format emits them raw and NUL-delimited. + it("parses them unquoted and commits their contents", async () => { + vi.mocked(core.warning).mockClear(); + + const repoPath = await createSpecialPathsTestRepo("special-paths"); + const stdout = await getGitStatusPorcelainV1(repoPath); + const changes = listChanges(stdout); + const { additions, deletions } = calculateAdditionsAndDeletions(changes); + const fileChanges = await calculateFileChanges( + { additions, deletions }, + repoPath, + ); + + // rename: old path deleted, new path added — both with spaces in them + expect(deletions).toContain("new dir/old file.txt"); + expect(additions).toContain("renamed dir/new name.txt"); + + // untracked paths with a space and a double quote, parsed unquoted + expect(additions).toContain("new dir/untracked file.txt"); + expect(additions).toContain('quote"name.txt'); + + // every addition is a real, readable file — none skipped, no warnings + expect(fileChanges.additions).toHaveLength(additions.length); + expect(core.warning).not.toHaveBeenCalled(); + }); + }); }); async function createCommitTestRepo(name: string): Promise { @@ -161,3 +185,20 @@ async function createCommitTestRepo(name: string): Promise { return repoPath; } + +async function createSpecialPathsTestRepo(name: string): Promise { + const repoPath = await createRepo(name); + + fs.mkdirSync(`${repoPath}/new dir`); + fs.writeFileSync(`${repoPath}/new dir/old file.txt`, "Old content"); + execSync(`cd ${repoPath} && git add . && git commit -m "Initial commit"`); + execSync( + `cd ${repoPath} && mkdir -p "renamed dir" && git mv "new dir/old file.txt" "renamed dir/new name.txt"`, + ); + + // 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"); + + return repoPath; +} diff --git a/actions/signed-commits/src/git/github-git/repo-status.ts b/actions/signed-commits/src/git/github-git/repo-status.ts index 71531d95e..12af03014 100644 --- a/actions/signed-commits/src/git/github-git/repo-status.ts +++ b/actions/signed-commits/src/git/github-git/repo-status.ts @@ -24,9 +24,13 @@ interface GitFileStatus { } export async function getGitStatusPorcelainV1(cwd?: string) { + // -z emits NUL-delimited records with raw, unquoted paths. The line-based + // format C-quotes paths containing spaces or special characters (e.g. + // `?? "new dir/file.txt"`), and those quoted paths fail readFileSync + // downstream because no file literally named `"new dir/file.txt"` exists. const stdout = await execWithOutput( "git", - ["status", "--porcelain=v1", "--untracked-files=all"], + ["status", "--porcelain=v1", "-z", "--untracked-files=all"], { cwd, notrim: true, @@ -90,23 +94,52 @@ export function listChanges(output: string): GitFileStatus[] { } /** - * Parses the output of `git status --porcelain=v1` into a list of git file statuses. - * @param output The stdout of `git status --porcelain=v1` + * Parses the output of `git status --porcelain=v1 -z` into a list of git file statuses. + * + * -z output is NUL-delimited and never quotes or escapes paths, unlike the + * line-based format which C-quotes paths containing spaces or special + * characters. Rename and copy records carry two consecutive fields: the new + * path, then the original path. The composite `original -> new` filePath + * preserved here matches the line-based format that downstream code and + * callers expect. + * @param output The stdout of `git status --porcelain=v1 -z` */ function parseGitStatusPorcelainOutput(output: string): GitFileStatus[] { - const lines = output.split("\n"); - // remove newline - lines.pop(); - return lines.map((line) => { - // only split the first two characters, as the rest is the file path - const [status, filePath] = [ - line.substring(0, 2), - line.substring(2).trim(), - ]; + const fields = output.split("\0"); + const changes: GitFileStatus[] = []; + + for (let i = 0; i < fields.length; i++) { + const field = fields[i] ?? ""; + // the trailing NUL produces one empty final field + if (field === "") { + continue; + } + // XY is the two-character status, followed by one separator space; the + // path starts at index 3. substring(3) (rather than trim) keeps any + // leading/trailing spaces that are part of the filename. + const status = field.substring(0, 2); + let filePath = field.substring(3); const { indexStatus, workingTreeStatus } = parseStatusCode(status); - return { filePath, indexStatus, workingTreeStatus } as GitFileStatus; - }); + // rename/copy records are followed by a second field: the original path + if ( + indexStatus === "R" || + indexStatus === "C" || + workingTreeStatus === "R" || + workingTreeStatus === "C" + ) { + const originalPath = fields[++i] ?? ""; + filePath = `${originalPath} -> ${filePath}`; + } + + changes.push({ + filePath, + indexStatus, + workingTreeStatus, + } as GitFileStatus); + } + + return changes; } return parseGitStatusPorcelainOutput(output);