Tighten helper binstub parity and PATH lookup (#1128)

## Summary

Adds focused parity coverage for the helper binstub contract introduced
in #1123.

The PR keeps the checked-in Ruby helper wrappers, the dummy app wrapper,
and the `createBinStub` TypeScript template aligned so future helper
drift fails CI quickly.

## Changes

- Export `createBinStub` from `package/configExporter/cli.ts` for test
use.
- Add `test/configExporter/createBinStub.test.js`, which generates
`shakapacker-config` and `diff-bundler-config` and byte-compares them
with `lib/install/bin/*`.
- Assert generated helper binstubs are executable.
- Keep `mkdirSync` and `chmodSync` in the top-level `fs` import instead
of using inline `require("fs")`.
- Improve `spec/shakapacker/binstub_sync_spec.rb` failure guidance so
contributors are told to update all synchronized copies.

## Review follow-up

Addressed the current actionable review threads:

- Switched to flat `test.each(["shakapacker-config",
"diff-bundler-config"])`.
- Removed the dead `if (tmp)` cleanup guard.
- Added a note explaining why `diff-bundler-config` is covered even
though production init only generates `shakapacker-config`.
- Added executable-permission coverage for generated binstubs.
- Expanded the Ruby sync failure message to name all synchronized
copies.
- Moved `mkdirSync` and `chmodSync` into the module-level `fs` import.
- Fixed leading and trailing empty `PATH` segment handling so helper
binstubs honor the current directory during Node lookup, and added RSpec
coverage for both helper commands.

Discussion/advice:

- The generated-user-binstub comment concern is intentionally left as
discussion-only for this PR. This parity test is meant to prove the
generated template matches the checked-in install files byte-for-byte;
changing generated comments separately would weaken that contract.
- Empty `PATH` segment handling turned out to be a real edge case: Ruby
`File.join("", "node")` checks `/node`, not the current directory, and
Ruby `String#split` drops trailing empty fields unless called with a
negative limit. The helper now preserves trailing entries and maps empty
segments to `Dir.pwd` before joining, preserving normal PATH semantics
even though the binstub later `chdir`s to the app root.

## CI note

This branch is rebased onto current `origin/main`.

The earlier `Test Both Bundlers` failure was Bundler frozen-mode
lockfile drift in `spec/dummy/Gemfile.lock`: the lockfile still recorded
`shakapacker (10.1.0.rc.1)` while the current gemspec resolves to
`10.1.0`. That lockfile fix is now already included in current
`origin/main`, and this branch has been rebased on top of it, so the PR
diff no longer carries a lockfile change.

The previous `claude-review` failure was due to the external Claude
weekly limit, not the repository diff.

## Test plan

- [x] `bundle exec rspec spec/shakapacker/helper_binstubs_spec.rb:211
--format documentation` - 4 empty-`PATH` examples pass for leading and
trailing entries across both helper commands
- [x] `bundle exec rspec spec/shakapacker/binstub_sync_spec.rb
spec/shakapacker/helper_binstubs_spec.rb` - 21 examples, 0 failures
- [x] `yarn jest test/configExporter/createBinStub.test.js` - 2 tests
passed
- [x] `yarn eslint package/configExporter/cli.ts
test/configExporter/createBinStub.test.js`
- [x] `bundle exec rubocop spec/shakapacker/helper_binstubs_spec.rb
spec/shakapacker/binstub_sync_spec.rb` - no offenses
- [x] `git diff --check origin/main...HEAD`

Refs #1123.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> Changes are confined to install/helper binstub templates and test
tooling; behavior fix is narrow (PATH parsing) with new regression
coverage.
> 
> **Overview**
> Tightens **helper binstub** parity and fixes **Node lookup** when
`PATH` contains empty segments (leading/trailing `:`).
> 
> **PATH lookup:** `shakapacker_find_executable` now splits `PATH` with
a negative limit so trailing empty entries are kept, and treats an empty
segment as **`Dir.pwd`** instead of joining to `/node`. The same Ruby
logic is updated in `lib/install/bin/*`,
`spec/dummy/bin/shakapacker-config`, and the **`createBinStub`**
template in `package/configExporter/cli.ts`.
> 
> **Parity & CI:** **`createBinStub`** is exported (test-only) and uses
top-level `fs` imports; **`test/configExporter/createBinStub.test.js`**
byte-compares generated `shakapacker-config` / `diff-bundler-config`
stubs to `lib/install/bin/*` and checks execute bits.
**`binstub_sync_spec`** failure text lists all four sync targets.
**`helper_binstubs_spec`** adds RSpec for leading/trailing empty `PATH`
entries, refactors `real_node_path`, and wraps some cases in
**`Bundler.with_unbundled_env`**.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
88a44c9. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: justin808

lib/install/bin/diff-bundler-config

@@ -28,9 +28,11 @@ def shakapacker_executable_candidates(executable)
 end
 
 def shakapacker_find_executable(executable)
-  ENV.fetch("PATH", "").split(File::PATH_SEPARATOR).each do |path|
+  ENV.fetch("PATH", "").split(File::PATH_SEPARATOR, -1).each do |path|
+    search_path = path.empty? ? Dir.pwd : path
+
     shakapacker_executable_candidates(executable).each do |candidate|
-      executable_path = File.join(path, candidate)
+      executable_path = File.join(search_path, candidate)
       return executable_path if File.file?(executable_path) && File.executable?(executable_path)
     end
   end

lib/install/bin/shakapacker-config

@@ -28,9 +28,11 @@ def shakapacker_executable_candidates(executable)
 end
 
 def shakapacker_find_executable(executable)
-  ENV.fetch("PATH", "").split(File::PATH_SEPARATOR).each do |path|
+  ENV.fetch("PATH", "").split(File::PATH_SEPARATOR, -1).each do |path|
+    search_path = path.empty? ? Dir.pwd : path
+
     shakapacker_executable_candidates(executable).each do |candidate|
-      executable_path = File.join(path, candidate)
+      executable_path = File.join(search_path, candidate)
       return executable_path if File.file?(executable_path) && File.executable?(executable_path)
     end
   end

package/configExporter/cli.ts

@@ -1,7 +1,13 @@
 // This will be a substantial file - the main CLI entry point
 // Originally migrated from bin/export-bundler-config, now bin/shakapacker-config
 
-import { existsSync, readFileSync, writeFileSync } from "fs"
+import {
+  chmodSync,
+  existsSync,
+  mkdirSync,
+  readFileSync,
+  writeFileSync
+} from "fs"
 import { resolve, dirname, sep, delimiter, basename } from "path"
 import { inspect } from "util"
 import { load as loadYaml } from "js-yaml"
@@ -510,10 +516,15 @@ function runInitCommand(options: ExportOptions): number {
   return 0
 }
 
-function createBinStub(binStubPath: string): void {
+/**
+ * Exported for test use only: verifies generated content matches
+ * lib/install/bin/* binstubs. Not part of the public API.
+ *
+ * @internal
+ */
+export function createBinStub(binStubPath: string): void {
   const binDir = dirname(binStubPath)
   const packageScript = `${basename(binStubPath)}.cjs`
-  const { mkdirSync, chmodSync } = require("fs")
 
   // Ensure bin directory exists
   if (!existsSync(binDir)) {
@@ -550,9 +561,11 @@ def shakapacker_executable_candidates(executable)
 end
 
 def shakapacker_find_executable(executable)
-  ENV.fetch("PATH", "").split(File::PATH_SEPARATOR).each do |path|
+  ENV.fetch("PATH", "").split(File::PATH_SEPARATOR, -1).each do |path|
+    search_path = path.empty? ? Dir.pwd : path
+
     shakapacker_executable_candidates(executable).each do |candidate|
-      executable_path = File.join(path, candidate)
+      executable_path = File.join(search_path, candidate)
       return executable_path if File.file?(executable_path) && File.executable?(executable_path)
     end
   end

spec/dummy/bin/shakapacker-config

@@ -28,9 +28,11 @@ def shakapacker_executable_candidates(executable)
 end
 
 def shakapacker_find_executable(executable)
-  ENV.fetch("PATH", "").split(File::PATH_SEPARATOR).each do |path|
+  ENV.fetch("PATH", "").split(File::PATH_SEPARATOR, -1).each do |path|
+    search_path = path.empty? ? Dir.pwd : path
+
     shakapacker_executable_candidates(executable).each do |candidate|
-      executable_path = File.join(path, candidate)
+      executable_path = File.join(search_path, candidate)
       return executable_path if File.file?(executable_path) && File.executable?(executable_path)
     end
   end

spec/shakapacker/binstub_sync_spec.rb

@@ -55,7 +55,9 @@
 
     expect(dummy_content).to eq(install_content),
       "spec/dummy/bin/shakapacker-config and lib/install/bin/shakapacker-config have diverged. " \
-      "Update both files to keep them in sync."
+      "All four copies must stay byte-for-byte identical — update each one: " \
+      "lib/install/bin/shakapacker-config, lib/install/bin/diff-bundler-config, " \
+      "spec/dummy/bin/shakapacker-config, and the createBinStub template in package/configExporter/cli.ts."
   end
 
   # lib/install/bin/diff-bundler-config and lib/install/bin/shakapacker-config share

spec/shakapacker/helper_binstubs_spec.rb

@@ -38,6 +38,12 @@ def install_fake_node_script(app_path, command)
     FileUtils.chmod(0o755, script_path)
   end
 
+  def real_node_path
+    ENV.fetch("PATH", "").split(File::PATH_SEPARATOR)
+      .map { |dir| File.join(dir, "node") }
+      .find { |candidate| File.file?(candidate) && File.executable?(candidate) }
+  end
+
   %w[shakapacker-config diff-bundler-config].each do |command|
     it "runs #{command} through a CommonJS package script when the app is ESM" do
       Dir.mktmpdir("shakapacker-binstub-") do |app_path|
@@ -160,11 +166,9 @@ def install_fake_node_script(app_path, command)
         FileUtils.mkdir_p(File.join(app_path, "bin"))
         install_fake_node_script(app_path, command)
 
-        real_node_path = ENV.fetch("PATH").split(File::PATH_SEPARATOR).map { |path| File.join(path, "node") }.find do |path|
-          File.file?(path) && File.executable?(path)
-        end
+        node_path = real_node_path
 
-        skip "node not found in PATH" unless real_node_path
+        skip "node not found in PATH" unless node_path
 
         fake_bin_path = File.join(app_path, "fake-bin")
         FileUtils.mkdir_p(fake_bin_path)
@@ -176,7 +180,7 @@ def install_fake_node_script(app_path, command)
             echo probed >> "$SHAKAPACKER_NODE_PROBE_OUTPUT"
             exit 0
           fi
-          exec #{real_node_path.shellescape} "$@"
+          exec #{node_path.shellescape} "$@"
         SH
         FileUtils.chmod(0o755, fake_node_path)
 
@@ -200,6 +204,55 @@ def install_fake_node_script(app_path, command)
       end
     end
 
+    {
+      "leading" => "#{File::PATH_SEPARATOR}/nonexistent",
+      "trailing" => "/nonexistent#{File::PATH_SEPARATOR}"
+    }.each do |position, path_value|
+      it "honors a #{position} empty PATH entry as the current directory for #{command}" do
+        Dir.mktmpdir("shakapacker-binstub-") do |app_path|
+          File.write(File.join(app_path, "Gemfile"), "")
+          FileUtils.mkdir_p(File.join(app_path, "bin"))
+          install_fake_node_script(app_path, command)
+
+          node_path = real_node_path
+          skip "node not found in PATH" unless node_path
+
+          launch_path = File.join(app_path, "launch")
+          FileUtils.mkdir_p(launch_path)
+          fake_node_path = File.join(launch_path, "node")
+          File.write(fake_node_path, <<~SH)
+            #!/bin/sh
+            exec #{node_path.shellescape} "$@"
+          SH
+          FileUtils.chmod(0o755, fake_node_path)
+
+          binstub_path = File.join(app_path, "bin", command)
+          FileUtils.cp(File.join(gem_root, "lib", "install", "bin", command), binstub_path)
+          FileUtils.chmod(0o755, binstub_path)
+
+          output_path = File.join(app_path, "binstub-output.json")
+          _stdout, stderr, status = Bundler.with_unbundled_env do
+            Open3.capture3(
+              {
+                "BUNDLE_GEMFILE" => nil,
+                "PATH" => path_value,
+                "RUBYOPT" => nil,
+                "SHAKAPACKER_BINSTUB_OUTPUT" => output_path
+              },
+              RbConfig.ruby,
+              binstub_path,
+              chdir: launch_path
+            )
+          end
+
+          expect(status).to be_success, stderr
+          expect(JSON.parse(File.read(output_path))).to include(
+            "cwd" => File.realpath(app_path)
+          )
+        end
+      end
+    end
+
     it "exits with an error when #{command}'s package script is missing" do
       Dir.mktmpdir("shakapacker-binstub-") do |app_path|
         File.write(File.join(app_path, "Gemfile"), "")
@@ -232,12 +285,14 @@ def install_fake_node_script(app_path, command)
         FileUtils.cp(File.join(gem_root, "lib", "install", "bin", command), binstub_path)
         FileUtils.chmod(0o755, binstub_path)
 
-        _stdout, stderr, status = Open3.capture3(
-          { "BUNDLE_GEMFILE" => nil, "RUBYOPT" => nil, "PATH" => "/nonexistent" },
-          RbConfig.ruby,
-          binstub_path,
-          chdir: app_path
-        )
+        _stdout, stderr, status = Bundler.with_unbundled_env do
+          Open3.capture3(
+            { "BUNDLE_GEMFILE" => nil, "RUBYOPT" => nil, "PATH" => "/nonexistent" },
+            RbConfig.ruby,
+            binstub_path,
+            chdir: app_path
+          )
+        end
 
         expect(status.exitstatus).to eq(1)
         expect(stderr).to include('[Shakapacker] Could not find Node.js executable "node"')

test/configExporter/createBinStub.test.js

@@ -0,0 +1,51 @@
+const {
+  accessSync,
+  constants,
+  mkdtempSync,
+  readFileSync,
+  rmSync
+} = require("fs")
+const { join, resolve } = require("path")
+const { tmpdir } = require("os")
+const { createBinStub } = require("../../package/configExporter/cli")
+
+const gemRoot = resolve(__dirname, "../..")
+
+// The Ruby logic in lib/install/bin/shakapacker-config and
+// lib/install/bin/diff-bundler-config is also duplicated inside the
+// `createBinStub` template in package/configExporter/cli.ts. The Ruby spec
+// (spec/shakapacker/binstub_sync_spec.rb) keeps the three checked-in copies
+// (install template, install diff template, and the dummy app's binstub)
+// honest, but it cannot reach into the JS template. This test closes that
+// gap by invoking createBinStub for both helper names and asserting the
+// generated content matches the corresponding lib/install/bin/* file.
+describe("createBinStub template parity", () => {
+  let tmp
+
+  beforeEach(() => {
+    tmp = mkdtempSync(join(tmpdir(), "shakapacker-createBinStub-"))
+  })
+
+  afterEach(() => {
+    rmSync(tmp, { recursive: true, force: true })
+  })
+
+  // diff-bundler-config is a white-box parity case; production init only
+  // generates shakapacker-config.
+  test.each(["shakapacker-config", "diff-bundler-config"])(
+    "generates lib/install/bin/%s byte-for-byte",
+    (binstubName) => {
+      const generatedPath = join(tmp, "bin", binstubName)
+      createBinStub(generatedPath)
+
+      const generated = readFileSync(generatedPath, "utf8")
+      const installed = readFileSync(
+        join(gemRoot, "lib", "install", "bin", binstubName),
+        "utf8"
+      )
+
+      expect(generated).toBe(installed)
+      expect(() => accessSync(generatedPath, constants.X_OK)).not.toThrow()
+    }
+  )
+})