fix: remaining fixes from PR 1025 (#1027)

* fix: remaining fixes from PR 1025

- Add prebuild hook and generate-packages.mjs script to ensure template
  packages are generated before CLI build runs
- Always copy cli.js to dist directory (required for dist/index.js to work)
- Add Node.js CLI flags whitelist to prevent --help, --version, etc from
  being forwarded to Python CLI
- Remove unused --prod flag from build script
- Simplify cli.js copy to use copyFileSync directly instead of spawning node

* test: fix flaky shadow npm and path-resolve tests in CI

- Add isDebug mock to npm-base.test.mts and install.test.mts to ensure
  --loglevel args are consistently added (isDebug() was returning true in CI
  due to environment variables)
- Use hoisted mocks for whichRealSync in path-resolve.test.mts to ensure
  reliable mock behavior in CI (dynamic import pattern was failing)

* fix: address PR review feedback

- Handle --flag=value syntax in nodeCliFlags whitelist by extracting base flag name
- Use nullish coalescing for process.exit in generate-packages.mjs to handle
  signal-killed processes (code is null, which coerces to 0)

* fix: correct mock function name in path-resolve tests

The test was mocking `resolveBinPathSync` but the actual function is
`resolveRealBinSync`. This caused tests to use the real implementation
in CI, which resolved to the actual npm path on the runner.

* fix: always run CLI build in CI test jobs

The CI workflow was using `lookup-only: true` for cache restore, which
only checks if the cache exists but doesn't actually restore files.
This caused test jobs to skip the build step when cache existed, but
the build artifacts weren't actually present on disk.

Remove `lookup-only` and always run the build step. The build script
already has its own caching logic and will skip unnecessary work.

Author: jdalton

.github/workflows/ci.yml

@@ -108,10 +108,8 @@ jobs:
           key: cli-build-${{ runner.os }}-${{ steps.cli-cache-key.outputs.hash }}
           restore-keys: |
             cli-build-${{ runner.os }}-
-          lookup-only: true
 
       - name: Build CLI
-        if: steps.cli-build-cache.outputs.cache-hit != 'true'
         working-directory: packages/cli
         run: pnpm run build
 
@@ -155,10 +153,8 @@ jobs:
           key: cli-build-${{ runner.os }}-${{ steps.cli-cache-key.outputs.hash }}
           restore-keys: |
             cli-build-${{ runner.os }}-
-          lookup-only: true
 
       - name: Build CLI
-        if: steps.cli-build-cache.outputs.cache-hit != 'true'
         working-directory: packages/cli
         run: pnpm run build
 
@@ -315,10 +311,8 @@ jobs:
           key: cli-build-${{ runner.os }}-${{ steps.cli-cache-key.outputs.hash }}
           restore-keys: |
             cli-build-${{ runner.os }}-
-          lookup-only: true
 
       - name: Build CLI
-        if: steps.cli-build-cache.outputs.cache-hit != 'true'
         working-directory: packages/cli
         run: pnpm run build
 

packages/cli/package.json

@@ -18,6 +18,7 @@
     "logo-light.png"
   ],
   "scripts": {
+    "prebuild": "node scripts/generate-packages.mjs",
     "build": "node --max-old-space-size=8192 --import=./scripts/load.mjs scripts/build.mjs",
     "build:force": "node --max-old-space-size=8192 --import=./scripts/load.mjs scripts/build.mjs --force",
     "build:watch": "node --max-old-space-size=8192 --import=./scripts/load.mjs scripts/build.mjs --watch",

packages/cli/scripts/build.mjs

@@ -1,8 +1,9 @@
 /**
  * Build script for Socket CLI.
- * Options: --quiet, --verbose, --prod, --force, --watch
+ * Options: --quiet, --verbose, --force, --watch
  */
 
+import { copyFileSync } from 'node:fs'
 import { promises as fs } from 'node:fs'
 import path from 'node:path'
 import { fileURLToPath } from 'node:url'
@@ -97,7 +98,6 @@ async function main() {
   const verbose = isVerbose()
   const watch = process.argv.includes('--watch')
   const force = process.argv.includes('--force')
-  const prod = process.argv.includes('--prod')
 
   // Pass --force flag via environment variable.
   if (force) {
@@ -202,19 +202,6 @@ async function main() {
         command: 'node',
         args: [...NODE_MEMORY_FLAGS, '.config/esbuild.inject.config.mjs'],
       },
-      // Copy CLI to dist for production builds.
-      ...(prod
-        ? [
-            {
-              name: 'Copy CLI to dist',
-              command: 'node',
-              args: [
-                '-e',
-                'require("fs").copyFileSync("build/cli.js", "dist/cli.js")',
-              ],
-            },
-          ]
-        : []),
     ]
 
     // Run build steps sequentially.
@@ -248,6 +235,9 @@ async function main() {
       }
     }
 
+    // Copy CLI bundle to dist (required for dist/index.js to work).
+    copyFileSync('build/cli.js', 'dist/cli.js')
+
     // Post-process: Fix node-gyp strings to prevent bundler issues.
     if (!quiet && verbose) {
       log.info('Post-processing build output...')

packages/cli/scripts/generate-packages.mjs

@@ -0,0 +1,20 @@
+/**
+ * Generate template-based packages required for CLI build.
+ * Runs the package generation scripts from package-builder.
+ */
+
+import { spawn } from '@socketsecurity/lib/spawn'
+
+const scripts = [
+  '../package-builder/scripts/generate-cli-sentry-package.mjs',
+  '../package-builder/scripts/generate-socket-package.mjs',
+  '../package-builder/scripts/generate-socketbin-packages.mjs',
+]
+
+for (const script of scripts) {
+  const result = await spawn('node', [script], { stdio: 'inherit' })
+  if (result.code !== 0) {
+    // Use nullish coalescing to handle signal-killed processes (code is null).
+    process.exit(result.code ?? 1)
+  }
+}

packages/cli/src/utils/cli/with-subcommands.mts

@@ -615,7 +615,28 @@ export async function meowWithSubcommands(
 
   // If first arg is a flag (starts with --), try Python CLI forwarding.
   // This enables: socket --repo owner/repo --target-path .
-  if (commandOrAliasName?.startsWith('--')) {
+  // Exception: Don't forward Node.js CLI built-in flags (help, version, etc).
+  const nodeCliFlags = new Set([
+    '--compact-header',
+    '--config',
+    '--dry-run',
+    '--help',
+    '--help-full',
+    '--json',
+    '--markdown',
+    '--no-banner',
+    '--no-spinner',
+    '--nobanner',
+    '--org',
+    '--spinner',
+    '--version',
+  ])
+  // Extract base flag name for --flag=value syntax (e.g., '--config=/path' -> '--config').
+  const baseFlagName = commandOrAliasName?.split('=')[0]
+  if (
+    commandOrAliasName?.startsWith('--') &&
+    !nodeCliFlags.has(baseFlagName ?? '')
+  ) {
     const pythonResult = await spawnSocketPython(argv, {
       stdio: 'inherit',
     })

packages/cli/test/unit/shadow/npm-base.test.mts

@@ -117,6 +117,11 @@ vi.mock('@socketsecurity/lib/constants/node', () => ({
   supportsNodePermissionFlag: vi.fn(() => true),
 }))
 
+// Mock isDebug to always return false so --loglevel args are added.
+vi.mock('@socketsecurity/lib/debug', () => ({
+  isDebug: vi.fn(() => false),
+}))
+
 describe('shadowNpmBase', () => {
   const mockSpawnResult = Promise.resolve({
     success: true,

packages/cli/test/unit/shadow/npm/install.test.mts

@@ -95,6 +95,11 @@ vi.mock('../../../../src/constants/cli.mts', () => ({
   FLAG_LOGLEVEL: '--loglevel',
 }))
 
+// Mock isDebug to always return false so --loglevel args are added.
+vi.mock('@socketsecurity/lib/debug', () => ({
+  isDebug: vi.fn(() => false),
+}))
+
 describe('shadowNpmInstall', () => {
   const mockProcess = {
     send: vi.fn(),

packages/cli/test/unit/utils/fs/path-resolve.test.mts

@@ -37,15 +37,19 @@ import { createTestWorkspace } from '../../../helpers/workspace-helper.mts'
 
 const PACKAGE_JSON = 'package.json'
 
+// Hoisted mocks for better CI reliability.
+const mockWhichRealSync = vi.hoisted(() => vi.fn())
+const mockResolveRealBinSync = vi.hoisted(() => vi.fn((p: string) => p))
+
 // Mock dependencies for new tests.
 vi.mock('@socketsecurity/lib/bin', async () => {
   const actual = await vi.importActual<
     typeof import('@socketsecurity/lib/bin')
   >('@socketsecurity/lib/bin')
   return {
     ...actual,
-    resolveBinPathSync: vi.fn(p => p),
-    whichRealSync: vi.fn(),
+    resolveRealBinSync: mockResolveRealBinSync,
+    whichRealSync: mockWhichRealSync,
   }
 })
 
@@ -382,11 +386,8 @@ describe('Path Resolve', () => {
       vi.clearAllMocks()
     })
 
-    it('finds bin path when available', async () => {
-      const { whichRealSync } = vi.mocked(
-        await import('@socketsecurity/lib/bin'),
-      )
-      whichRealSync.mockReturnValue(['/usr/local/bin/npm'])
+    it('finds bin path when available', () => {
+      mockWhichRealSync.mockReturnValue(['/usr/local/bin/npm'])
 
       const result = findBinPathDetailsSync('npm')
 
@@ -400,10 +401,7 @@ describe('Path Resolve', () => {
     it('handles shadowed bin paths', async () => {
       const constants = await import('../../../../src/constants.mts')
       const shadowBinPath = constants.default.shadowBinPath
-      const { whichRealSync } = vi.mocked(
-        await import('@socketsecurity/lib/bin'),
-      )
-      whichRealSync.mockReturnValue([
+      mockWhichRealSync.mockReturnValue([
         `${shadowBinPath}/npm`,
         '/usr/local/bin/npm',
       ])
@@ -417,11 +415,8 @@ describe('Path Resolve', () => {
       })
     })
 
-    it('handles no bin path found', async () => {
-      const { whichRealSync } = vi.mocked(
-        await import('@socketsecurity/lib/bin'),
-      )
-      whichRealSync.mockReturnValue(null)
+    it('handles no bin path found', () => {
+      mockWhichRealSync.mockReturnValue(null)
 
       const result = findBinPathDetailsSync('nonexistent')
 
@@ -432,11 +427,8 @@ describe('Path Resolve', () => {
       })
     })
 
-    it('handles empty array result', async () => {
-      const { whichRealSync } = vi.mocked(
-        await import('@socketsecurity/lib/bin'),
-      )
-      whichRealSync.mockReturnValue([])
+    it('handles empty array result', () => {
+      mockWhichRealSync.mockReturnValue([])
 
       const result = findBinPathDetailsSync('npm')
 
@@ -447,11 +439,8 @@ describe('Path Resolve', () => {
       })
     })
 
-    it('handles single string result', async () => {
-      const { whichRealSync } = vi.mocked(
-        await import('@socketsecurity/lib/bin'),
-      )
-      whichRealSync.mockReturnValue('/usr/local/bin/npm' as any)
+    it('handles single string result', () => {
+      mockWhichRealSync.mockReturnValue('/usr/local/bin/npm' as any)
 
       const result = findBinPathDetailsSync('npm')
 
@@ -465,10 +454,7 @@ describe('Path Resolve', () => {
     it('handles only shadow bin in path', async () => {
       const constants = await import('../../../../src/constants.mts')
       const shadowBinPath = constants.default.shadowBinPath
-      const { whichRealSync } = vi.mocked(
-        await import('@socketsecurity/lib/bin'),
-      )
-      whichRealSync.mockReturnValue([`${shadowBinPath}/npm`])
+      mockWhichRealSync.mockReturnValue([`${shadowBinPath}/npm`])
 
       const result = findBinPathDetailsSync('npm')