fix: address PR review comments for workflow, runner, and cli

justin808 · claude · justin808 · commit fe15136e6341 · 2026-03-10T17:43:18.000-10:00
- Fix actions/checkout@v6 to @v4 (v6 does not exist) - Add clarifying comment about TERM trap race window in runner.rb - Remove always-true saveDir guard in --all-builds mode, add comments explaining applyDefaults guarantee and --output omission - Add cross-reference comment for duplicated validation block Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml
@@ -16,7 +16,7 @@ jobs:
 
     steps:
       - name: Checkout repository
-        uses: actions/checkout@v6
+        uses: actions/checkout@v4
         with:
           fetch-depth: 1
 
diff --git a/lib/shakapacker/runner.rb b/lib/shakapacker/runner.rb
@@ -296,7 +296,8 @@ def run
           if child_pid
             Process.kill("TERM", child_pid)
           else
-            raise SignalException, "TERM" # if there is no child_pid we never spawned the process and can quit as normal
+            # Signal arrived before spawn completed; re-raise so the process exits normally.
+            raise SignalException, "TERM"
           end
         rescue Errno::ESRCH
           nil
diff --git a/package/configExporter/cli.ts b/package/configExporter/cli.ts
@@ -712,13 +712,11 @@ async function runAllBuildsCommand(options: ExportOptions): Promise<number> {
     // Apply defaults
     const resolvedOptions = applyDefaults(options)
 
-    // Validate paths for security in all-builds mode
-    if (resolvedOptions.saveDir) {
-      // Validates for path traversal; throws on unsafe paths. Return value intentionally discarded.
-      safeResolvePath(appRoot, resolvedOptions.saveDir)
-    }
+    // Validate paths for security in all-builds mode.
+    // saveDir is always set by applyDefaults(); --output is not used in --all-builds mode.
+    safeResolvePath(appRoot, resolvedOptions.saveDir!)
 
-    // Validate annotate/format combination in all-builds mode
+    // Keep in sync with validation in run()
     if (resolvedOptions.annotate && resolvedOptions.format !== "yaml") {
       throw new Error(
         "Annotation requires YAML format. Use --no-annotate or --format=yaml."
