From c572023b73db0dedb20d92e6b0f7b04338a28a3a Mon Sep 17 00:00:00 2001 From: jdalton Date: Sat, 10 Jan 2026 17:35:11 -0500 Subject: [PATCH 1/5] 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 --- packages/cli/package.json | 1 + packages/cli/scripts/build.mjs | 20 +++++------------- packages/cli/scripts/generate-packages.mjs | 19 +++++++++++++++++ .../cli/src/utils/cli/with-subcommands.mts | 21 ++++++++++++++++++- 4 files changed, 45 insertions(+), 16 deletions(-) create mode 100644 packages/cli/scripts/generate-packages.mjs diff --git a/packages/cli/package.json b/packages/cli/package.json index 86602abcc..dd2495e58 100644 --- a/packages/cli/package.json +++ b/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", diff --git a/packages/cli/scripts/build.mjs b/packages/cli/scripts/build.mjs index dce78cf50..17f6cb7d2 100644 --- a/packages/cli/scripts/build.mjs +++ b/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...') diff --git a/packages/cli/scripts/generate-packages.mjs b/packages/cli/scripts/generate-packages.mjs new file mode 100644 index 000000000..c6838ea00 --- /dev/null +++ b/packages/cli/scripts/generate-packages.mjs @@ -0,0 +1,19 @@ +/** + * 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) { + process.exit(result.code) + } +} diff --git a/packages/cli/src/utils/cli/with-subcommands.mts b/packages/cli/src/utils/cli/with-subcommands.mts index ee76efc3d..30d86b472 100644 --- a/packages/cli/src/utils/cli/with-subcommands.mts +++ b/packages/cli/src/utils/cli/with-subcommands.mts @@ -615,7 +615,26 @@ 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', + ]) + if ( + commandOrAliasName?.startsWith('--') && + !nodeCliFlags.has(commandOrAliasName) + ) { const pythonResult = await spawnSocketPython(argv, { stdio: 'inherit', }) From 97fc137d47482565e895b85a2b3fa299b14f40b2 Mon Sep 17 00:00:00 2001 From: jdalton Date: Sat, 10 Jan 2026 17:47:11 -0500 Subject: [PATCH 2/5] 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) --- .../cli/test/unit/shadow/npm-base.test.mts | 5 ++ .../cli/test/unit/shadow/npm/install.test.mts | 5 ++ .../test/unit/utils/fs/path-resolve.test.mts | 46 +++++++------------ 3 files changed, 26 insertions(+), 30 deletions(-) diff --git a/packages/cli/test/unit/shadow/npm-base.test.mts b/packages/cli/test/unit/shadow/npm-base.test.mts index e3bb3070a..39c715d4f 100644 --- a/packages/cli/test/unit/shadow/npm-base.test.mts +++ b/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, diff --git a/packages/cli/test/unit/shadow/npm/install.test.mts b/packages/cli/test/unit/shadow/npm/install.test.mts index a1cf1c87d..cbeeaca28 100644 --- a/packages/cli/test/unit/shadow/npm/install.test.mts +++ b/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(), diff --git a/packages/cli/test/unit/utils/fs/path-resolve.test.mts b/packages/cli/test/unit/utils/fs/path-resolve.test.mts index dad01b39b..10fe20447 100644 --- a/packages/cli/test/unit/utils/fs/path-resolve.test.mts +++ b/packages/cli/test/unit/utils/fs/path-resolve.test.mts @@ -37,6 +37,10 @@ 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 mockResolveBinPathSync = vi.hoisted(() => vi.fn((p: string) => p)) + // Mock dependencies for new tests. vi.mock('@socketsecurity/lib/bin', async () => { const actual = await vi.importActual< @@ -44,8 +48,8 @@ vi.mock('@socketsecurity/lib/bin', async () => { >('@socketsecurity/lib/bin') return { ...actual, - resolveBinPathSync: vi.fn(p => p), - whichRealSync: vi.fn(), + resolveBinPathSync: mockResolveBinPathSync, + 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') From 1e32839f8f1624c873c96d4d68bb45b058d5315a Mon Sep 17 00:00:00 2001 From: jdalton Date: Sat, 10 Jan 2026 17:51:35 -0500 Subject: [PATCH 3/5] 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) --- packages/cli/scripts/generate-packages.mjs | 3 ++- packages/cli/src/utils/cli/with-subcommands.mts | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/cli/scripts/generate-packages.mjs b/packages/cli/scripts/generate-packages.mjs index c6838ea00..7e5964fd5 100644 --- a/packages/cli/scripts/generate-packages.mjs +++ b/packages/cli/scripts/generate-packages.mjs @@ -14,6 +14,7 @@ const scripts = [ for (const script of scripts) { const result = await spawn('node', [script], { stdio: 'inherit' }) if (result.code !== 0) { - process.exit(result.code) + // Use nullish coalescing to handle signal-killed processes (code is null). + process.exit(result.code ?? 1) } } diff --git a/packages/cli/src/utils/cli/with-subcommands.mts b/packages/cli/src/utils/cli/with-subcommands.mts index 30d86b472..b250d4118 100644 --- a/packages/cli/src/utils/cli/with-subcommands.mts +++ b/packages/cli/src/utils/cli/with-subcommands.mts @@ -631,9 +631,11 @@ export async function meowWithSubcommands( '--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(commandOrAliasName) + !nodeCliFlags.has(baseFlagName ?? '') ) { const pythonResult = await spawnSocketPython(argv, { stdio: 'inherit', From 2809240dac7ab2d2f6cb0f268dfe60d8f3fe3bd9 Mon Sep 17 00:00:00 2001 From: jdalton Date: Sat, 10 Jan 2026 17:59:37 -0500 Subject: [PATCH 4/5] 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. --- packages/cli/test/unit/utils/fs/path-resolve.test.mts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cli/test/unit/utils/fs/path-resolve.test.mts b/packages/cli/test/unit/utils/fs/path-resolve.test.mts index 10fe20447..c46994277 100644 --- a/packages/cli/test/unit/utils/fs/path-resolve.test.mts +++ b/packages/cli/test/unit/utils/fs/path-resolve.test.mts @@ -39,7 +39,7 @@ const PACKAGE_JSON = 'package.json' // Hoisted mocks for better CI reliability. const mockWhichRealSync = vi.hoisted(() => vi.fn()) -const mockResolveBinPathSync = vi.hoisted(() => vi.fn((p: string) => p)) +const mockResolveRealBinSync = vi.hoisted(() => vi.fn((p: string) => p)) // Mock dependencies for new tests. vi.mock('@socketsecurity/lib/bin', async () => { @@ -48,7 +48,7 @@ vi.mock('@socketsecurity/lib/bin', async () => { >('@socketsecurity/lib/bin') return { ...actual, - resolveBinPathSync: mockResolveBinPathSync, + resolveRealBinSync: mockResolveRealBinSync, whichRealSync: mockWhichRealSync, } }) From 9521bcf71207a52dd1e32b2c826d63cb5d7f66c6 Mon Sep 17 00:00:00 2001 From: jdalton Date: Sat, 10 Jan 2026 18:05:25 -0500 Subject: [PATCH 5/5] 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. --- .github/workflows/ci.yml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4f2addfe5..4f925cad7 100644 --- a/.github/workflows/ci.yml +++ b/.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