fix: porcelain -z parsing, setup-nodejs runner tests, parameterized solidity-review-artifacts

.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.

.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 }}

.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

actions/signed-commits/src/git/github-git/repo-status.test.ts

@@ -22,13 +22,7 @@
       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 @@
       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<string> {
@@ -161,3 +185,20 @@
 
   return repoPath;
 }
+
+async function createSpecialPathsTestRepo(name: string): Promise<string> {
+  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;
+}

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);