fix: resolve node_modules bins when running scripts inside .faststore

packages/cli/src/commands/build.ts

@@ -5,6 +5,7 @@ import { existsSync } from 'node:fs'
 import fsExtra from 'fs-extra'
 import path from 'node:path'
 import { fileURLToPath, pathToFileURL } from 'node:url'
+import { withNodeModulesBins } from '../utils/binPaths'
 import { getPreferredPackageManager } from '../utils/commands'
 import { checkDeprecatedSecretFiles } from '../utils/deprecations'
 import { getBasePath, withBasePath } from '../utils/directory'
@@ -94,6 +95,7 @@ export default class Build extends Command {
       shell: true,
       cwd: tmpDir,
       stdio: 'inherit',
+      env: withNodeModulesBins(tmpDir),
     })
 
     if (scriptResult.status && scriptResult.status !== 0) {

packages/cli/src/commands/dev.ts

@@ -7,6 +7,7 @@ import dotenv from 'dotenv'
 import { cpSync, existsSync, readFileSync } from 'node:fs'
 import path from 'node:path'
 import { fileURLToPath } from 'node:url'
+import { withNodeModulesBins } from '../utils/binPaths'
 import { getPreferredPackageManager } from '../utils/commands'
 import { checkDeprecatedSecretFiles } from '../utils/deprecations'
 import { getBasePath, withBasePath } from '../utils/directory'
@@ -68,6 +69,7 @@ async function storeDev(
     errorMessage: `The "predev" step ("${packageManager} predev") failed inside the ".faststore" directory. See the error output below for details.`,
     throws: 'error',
     cwd: tmpDir,
+    env: withNodeModulesBins(tmpDir),
   })
 
   const { success } = copyGenerated(
@@ -89,10 +91,10 @@ async function storeDev(
     cwd: tmpDir,
     signal: devAbortController.signal,
     stdio: ['inherit', 'pipe', 'inherit'],
-    env: {
+    env: withNodeModulesBins(tmpDir, {
       ...process.env,
       ...envVars,
-    },
+    }),
   })
 
   let nextStdout = ''

packages/cli/src/commands/test.ts

@@ -2,6 +2,7 @@ import { Args, Command } from '@oclif/core'
 import { spawn } from 'node:child_process'
 import chokidar from 'chokidar'
 
+import { withNodeModulesBins } from '../utils/binPaths'
 import { getPreferredPackageManager } from '../utils/commands'
 import { getBasePath, withBasePath } from '../utils/directory'
 import { generate } from '../utils/generate'
@@ -37,6 +38,7 @@ async function storeTest(tmpDir: string) {
     cwd: tmpDir,
     signal: testAbortController.signal,
     stdio: 'inherit',
+    env: withNodeModulesBins(tmpDir),
   })
 
   testProcess.on('close', () => {

packages/cli/src/utils/binPaths.test.ts

@@ -0,0 +1,146 @@
+import { spawnSync } from 'node:child_process'
+import fs from 'node:fs'
+import os from 'node:os'
+import path from 'node:path'
+import { afterAll, beforeAll, describe, expect, it } from 'vitest'
+
+import { withNodeModulesBins } from './binPaths'
+
+// mimics a hoisted monorepo: bins live at the workspace root, the store has its
+// own node_modules, and `.faststore` has none
+let workspaceRoot: string
+let storeDir: string
+let tmpDir: string
+
+beforeAll(() => {
+  workspaceRoot = fs.realpathSync(
+    fs.mkdtempSync(path.join(os.tmpdir(), 'bin-paths-'))
+  )
+  storeDir = path.join(workspaceRoot, 'stores', 'my-store')
+  tmpDir = path.join(storeDir, '.faststore')
+
+  fs.mkdirSync(path.join(workspaceRoot, 'node_modules', '.bin'), {
+    recursive: true,
+  })
+  fs.mkdirSync(path.join(storeDir, 'node_modules', '.bin'), { recursive: true })
+  fs.mkdirSync(tmpDir, { recursive: true })
+})
+
+afterAll(() => {
+  fs.rmSync(workspaceRoot, { recursive: true, force: true })
+})
+
+describe('withNodeModulesBins', () => {
+  it('prepends the bin directories of every ancestor, nearest first', () => {
+    const { PATH } = withNodeModulesBins(tmpDir, { PATH: '/usr/bin' })
+    const entries = PATH?.split(path.delimiter) ?? []
+
+    expect(entries.slice(0, 2)).toEqual([
+      path.join(storeDir, 'node_modules', '.bin'),
+      path.join(workspaceRoot, 'node_modules', '.bin'),
+    ])
+    expect(entries.at(-1)).toBe('/usr/bin')
+  })
+
+  it('skips ancestors without a node_modules/.bin', () => {
+    const { PATH } = withNodeModulesBins(tmpDir, { PATH: '/usr/bin' })
+
+    expect(PATH).not.toContain(
+      path.join(workspaceRoot, 'stores', 'node_modules', '.bin')
+    )
+  })
+
+  it('keeps the other environment variables untouched', () => {
+    const env = withNodeModulesBins(tmpDir, {
+      PATH: '/usr/bin',
+      VTEX_ACCOUNT: 'storeframework',
+    })
+
+    expect(env.VTEX_ACCOUNT).toBe('storeframework')
+  })
+
+  it('reuses the existing PATH key casing, as used on Windows', () => {
+    const env = withNodeModulesBins(tmpDir, { Path: 'C:\\Windows\\system32' })
+
+    expect(env.PATH).toBeUndefined()
+    expect(env.Path).toContain(path.join(storeDir, 'node_modules', '.bin'))
+    expect(env.Path).toContain('C:\\Windows\\system32')
+  })
+
+  it('moves a bin directory already in PATH to its nearest-first position instead of duplicating it', () => {
+    const storeBinDir = path.join(storeDir, 'node_modules', '.bin')
+    const { PATH } = withNodeModulesBins(tmpDir, {
+      PATH: [storeBinDir, '/usr/bin'].join(path.delimiter),
+    })
+    const entries = PATH?.split(path.delimiter) ?? []
+
+    expect(entries).toEqual([
+      storeBinDir,
+      path.join(workspaceRoot, 'node_modules', '.bin'),
+      '/usr/bin',
+    ])
+  })
+
+  it('does not drop the bin directories when PATH is empty', () => {
+    const { PATH } = withNodeModulesBins(tmpDir, {})
+
+    expect(PATH?.split(path.delimiter).slice(0, 2)).toEqual([
+      path.join(storeDir, 'node_modules', '.bin'),
+      path.join(workspaceRoot, 'node_modules', '.bin'),
+    ])
+  })
+
+  it('returns the environment untouched when no ancestor has a node_modules/.bin', () => {
+    // Sibling of the fixture rather than a descendant: the walk from here goes
+    // straight up the OS temp dir, where no node_modules/.bin exists.
+    const binlessDir = fs.realpathSync(
+      fs.mkdtempSync(path.join(os.tmpdir(), 'bin-paths-binless-'))
+    )
+    const env = { PATH: '/usr/bin', VTEX_ACCOUNT: 'storeframework' }
+
+    try {
+      expect(withNodeModulesBins(binlessDir, env)).toEqual(env)
+    } finally {
+      fs.rmSync(binlessDir, { recursive: true, force: true })
+    }
+  })
+
+  // The tests above only inspect the object we build. This one spawns a real
+  // child process from `.faststore`, the way `dev` and `build` do, to check
+  // that the environment is what makes a bare binary name resolve. Windows
+  // would need a `.cmd` shim for the fixture, so it stays on POSIX.
+  it.skipIf(process.platform === 'win32')(
+    'lets a child process resolve a binary living in an ancestor bin directory',
+    () => {
+      const probe = path.join(
+        storeDir,
+        'node_modules',
+        '.bin',
+        'faststore-bin-probe'
+      )
+      fs.writeFileSync(probe, '#!/bin/sh\necho resolved\n')
+      fs.chmodSync(probe, 0o755)
+
+      // no `shell: true`: the OS resolves the bare name from the environment
+      // we hand it, which is the behaviour under test, and it keeps a shell
+      // interpreter out of the test
+      const run = (env: NodeJS.ProcessEnv) =>
+        spawnSync('faststore-bin-probe', {
+          cwd: tmpDir,
+          encoding: 'utf-8',
+          env,
+        })
+
+      // A PATH that cannot resolve the probe on its own, so that a passing
+      // assertion below can only come from the directories we added.
+      const barePath = { PATH: path.join(workspaceRoot, 'nowhere') }
+
+      expect(run(barePath).status).not.toBe(0)
+
+      const resolved = run(withNodeModulesBins(tmpDir, barePath))
+
+      expect(resolved.status).toBe(0)
+      expect(resolved.stdout.trim()).toBe('resolved')
+    }
+  )
+})

packages/cli/src/utils/binPaths.ts

@@ -0,0 +1,65 @@
+import fsExtra from 'fs-extra'
+import path from 'node:path'
+
+const { existsSync } = fsExtra
+
+function collectBinDirs(fromDir: string) {
+  const binDirs: string[] = []
+  let currentDir = path.resolve(fromDir)
+
+  while (true) {
+    const binDir = path.join(currentDir, 'node_modules', '.bin')
+
+    if (existsSync(binDir)) {
+      binDirs.push(binDir)
+    }
+
+    const parentDir = path.dirname(currentDir)
+
+    if (parentDir === currentDir) {
+      return binDirs
+    }
+
+    currentDir = parentDir
+  }
+}
+
+// Environment variables are case-insensitive on Windows, where the key is
+// usually `Path`. Reusing the existing key avoids handing the child process
+// both `Path` and `PATH`.
+function getPathKey(env: NodeJS.ProcessEnv) {
+  return Object.keys(env).find((key) => key.toUpperCase() === 'PATH') ?? 'PATH'
+}
+
+/**
+ * Prepends the `node_modules/.bin` of `fromDir` and of every ancestor
+ * directory to `PATH`.
+ *
+ * The generated `.faststore` package has no `node_modules` of its own, so the
+ * binaries its scripts call (`na`, `next`) live higher up the tree: in the
+ * store root or, on hoisted monorepos, the workspace root.
+ */
+export function withNodeModulesBins(
+  fromDir: string,
+  env: NodeJS.ProcessEnv = process.env
+): NodeJS.ProcessEnv {
+  const binDirs = collectBinDirs(fromDir)
+
+  if (binDirs.length === 0) {
+    return { ...env }
+  }
+
+  const pathKey = getPathKey(env)
+  const currentEntries = env[pathKey]?.split(path.delimiter) ?? []
+  // Package managers already put some of these directories in PATH when they
+  // run a script, but not necessarily nearest first. Dropping their copies
+  // before prepending `binDirs` keeps the nearest ancestor in charge instead
+  // of letting a farther one that was missing from PATH jump ahead of it.
+  const binDirSet = new Set(binDirs)
+  const otherEntries = currentEntries.filter((entry) => !binDirSet.has(entry))
+
+  return {
+    ...env,
+    [pathKey]: [...binDirs, ...otherEntries].join(path.delimiter),
+  }
+}

packages/cli/src/utils/runCommandSync.test.ts

@@ -0,0 +1,47 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+import { runCommandSync } from './runCommandSync'
+
+const execSyncMock = vi.hoisted(() => vi.fn())
+
+vi.mock('node:child_process', () => ({ execSync: execSyncMock }))
+
+const optionsOfLastCall = () => execSyncMock.mock.calls.at(-1)?.[1]
+
+describe('runCommandSync', () => {
+  beforeEach(() => {
+    execSyncMock.mockClear()
+  })
+
+  it('forwards a custom environment to the child process', () => {
+    const env = {
+      PATH: '/store/node_modules/.bin',
+      VTEX_ACCOUNT: 'storeframework',
+    }
+
+    runCommandSync({
+      cmd: 'yarn predev',
+      errorMessage: 'predev failed',
+      throws: 'error',
+      cwd: '/store/.faststore',
+      env,
+    })
+
+    expect(optionsOfLastCall()).toMatchObject({
+      cwd: '/store/.faststore',
+      env,
+    })
+  })
+
+  // execSync treats an undefined env as "inherit process.env", which is what
+  // the call sites that never pass one rely on.
+  it('leaves the environment to the child when the caller does not pass one', () => {
+    runCommandSync({
+      cmd: 'yarn generate',
+      errorMessage: 'generate failed',
+      throws: 'error',
+    })
+
+    expect(optionsOfLastCall()?.env).toBeUndefined()
+  })
+})

packages/cli/src/utils/runCommandSync.ts

@@ -46,12 +46,14 @@ export const runCommandSync = ({
   errorMessage,
   throws,
   cwd,
+  env,
   interactive = false,
 }: {
   cmd: string
   errorMessage: string
   throws: 'warning' | 'error'
   cwd?: string
+  env?: NodeJS.ProcessEnv
   interactive?: boolean
 }) => {
   try {
@@ -70,6 +72,7 @@ export const runCommandSync = ({
     const res = execSync(interactive ? cmd : `${cmd} 2>&1`, {
       stdio: interactive ? 'inherit' : 'pipe',
       cwd,
+      env,
     })
     logger.log(`[STATUS] ${res?.toString() ?? 'Unknown'}`)
     logger.log(`[FINISHED] ${cmd}`)