From 017a220904f749eb128b929ddeb22adbfac5a08d Mon Sep 17 00:00:00 2001 From: jdalton Date: Fri, 14 Nov 2025 09:43:26 -0800 Subject: [PATCH] Fix pnpm dlx --silent flag order to prevent coana from treating it as filepath When using pnpm dlx with the --silent flag, the flag was incorrectly placed after 'dlx' in the command, causing it to be passed to the target package (e.g., @coana-tech/cli) as a positional argument. Coana's 'run' command interpreted --silent as the filepath argument instead of the actual directory. This fix moves --silent before 'dlx' so pnpm correctly interprets it: - Before: pnpm dlx --silent @coana-tech/cli run /path (incorrect) - After: pnpm --silent dlx @coana-tech/cli run /path (correct) Also adds comprehensive unit tests to prevent regression and adds .env.local to .gitignore. --- .gitignore | 1 + src/utils/dlx.mts | 10 ++- src/utils/dlx.test.mts | 182 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 189 insertions(+), 4 deletions(-) create mode 100644 src/utils/dlx.test.mts diff --git a/.gitignore b/.gitignore index b8b365731..e5349663f 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ ._.DS_Store Thumbs.db /.env +/.env.local /.nvm /.rollup.cache /.type-coverage diff --git a/src/utils/dlx.mts b/src/utils/dlx.mts index 6fb8d7225..8ee7b248b 100644 --- a/src/utils/dlx.mts +++ b/src/utils/dlx.mts @@ -115,7 +115,12 @@ export async function spawnDlx( let spawnArgs: string[] if (pm === PNPM) { - spawnArgs = ['dlx'] + spawnArgs = [] + // The --silent flag must come before dlx, not after. + if (silent) { + spawnArgs.push(FLAG_SILENT) + } + spawnArgs.push('dlx') if (force) { // For pnpm, set dlx-cache-max-age to 0 via env to force fresh download. // This ensures we always get the latest version within the range. @@ -130,9 +135,6 @@ export async function spawnDlx( }, } } - if (silent) { - spawnArgs.push(FLAG_SILENT) - } spawnArgs.push(packageString, ...args) const shadowPnpmBin = /*@__PURE__*/ require(constants.shadowPnpmBinPath) diff --git a/src/utils/dlx.test.mts b/src/utils/dlx.test.mts new file mode 100644 index 000000000..78c9c64dc --- /dev/null +++ b/src/utils/dlx.test.mts @@ -0,0 +1,182 @@ +import { createRequire } from 'node:module' + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import constants from '../constants.mts' +import { spawnDlx } from './dlx.mts' + +import type { DlxPackageSpec } from './dlx.mts' + +const require = createRequire(import.meta.url) + +describe('utils/dlx', () => { + describe('spawnDlx', () => { + let mockShadowPnpmBin: ReturnType + let mockShadowNpxBin: ReturnType + let mockShadowYarnBin: ReturnType + + beforeEach(() => { + // Create mock functions that return a promise with spawnPromise. + const createMockBin = () => + vi.fn().mockResolvedValue({ + spawnPromise: Promise.resolve({ stdout: '', stderr: '' }), + }) + + mockShadowPnpmBin = createMockBin() + mockShadowNpxBin = createMockBin() + mockShadowYarnBin = createMockBin() + + // Mock the require calls for shadow binaries. + vi.spyOn(require, 'resolve').mockImplementation((id: string) => { + if (id === constants.shadowPnpmBinPath) { + return id + } + if (id === constants.shadowNpxBinPath) { + return id + } + if (id === constants.shadowYarnBinPath) { + return id + } + throw new Error(`Unexpected require: ${id}`) + }) + + // @ts-ignore + require.cache[constants.shadowPnpmBinPath] = { + exports: mockShadowPnpmBin, + } + // @ts-ignore + require.cache[constants.shadowNpxBinPath] = { exports: mockShadowNpxBin } + // @ts-ignore + require.cache[constants.shadowYarnBinPath] = { + exports: mockShadowYarnBin, + } + }) + + afterEach(() => { + vi.restoreAllMocks() + // Clean up require cache. + // @ts-ignore + delete require.cache[constants.shadowPnpmBinPath] + // @ts-ignore + delete require.cache[constants.shadowNpxBinPath] + // @ts-ignore + delete require.cache[constants.shadowYarnBinPath] + }) + + it('should place --silent before dlx for pnpm', async () => { + const packageSpec: DlxPackageSpec = { + name: '@coana-tech/cli', + version: '~1.0.0', + } + + await spawnDlx(packageSpec, ['run', '/some/path'], { + agent: 'pnpm', + silent: true, + }) + + expect(mockShadowPnpmBin).toHaveBeenCalledTimes(1) + const [spawnArgs] = mockShadowPnpmBin.mock.calls[0] + + // Verify that --silent comes before dlx. + expect(spawnArgs[0]).toBe('--silent') + expect(spawnArgs[1]).toBe('dlx') + expect(spawnArgs[2]).toBe('@coana-tech/cli@~1.0.0') + expect(spawnArgs[3]).toBe('run') + expect(spawnArgs[4]).toBe('/some/path') + }) + + it('should not add --silent for pnpm when silent is false', async () => { + const packageSpec: DlxPackageSpec = { + name: '@coana-tech/cli', + version: '1.0.0', + } + + await spawnDlx(packageSpec, ['run', '/some/path'], { + agent: 'pnpm', + silent: false, + }) + + expect(mockShadowPnpmBin).toHaveBeenCalledTimes(1) + const [spawnArgs] = mockShadowPnpmBin.mock.calls[0] + + // Verify that --silent is not present. + expect(spawnArgs[0]).toBe('dlx') + expect(spawnArgs[1]).toBe('@coana-tech/cli@1.0.0') + expect(spawnArgs[2]).toBe('run') + expect(spawnArgs[3]).toBe('/some/path') + }) + + it('should default silent to true for pnpm when version is not pinned', async () => { + const packageSpec: DlxPackageSpec = { + name: '@coana-tech/cli', + version: '~1.0.0', + } + + await spawnDlx(packageSpec, ['run', '/some/path'], { agent: 'pnpm' }) + + expect(mockShadowPnpmBin).toHaveBeenCalledTimes(1) + const [spawnArgs] = mockShadowPnpmBin.mock.calls[0] + + // Verify that --silent is automatically added for unpinned versions. + expect(spawnArgs[0]).toBe('--silent') + expect(spawnArgs[1]).toBe('dlx') + }) + + it('should place --silent after --yes for npm', async () => { + const packageSpec: DlxPackageSpec = { + name: '@coana-tech/cli', + version: '~1.0.0', + } + + await spawnDlx(packageSpec, ['run', '/some/path'], { + agent: 'npm', + silent: true, + }) + + expect(mockShadowNpxBin).toHaveBeenCalledTimes(1) + const [spawnArgs] = mockShadowNpxBin.mock.calls[0] + + // For npm/npx, --yes comes first, then --silent. + expect(spawnArgs[0]).toBe('--yes') + expect(spawnArgs[1]).toBe('--silent') + expect(spawnArgs[2]).toBe('@coana-tech/cli@~1.0.0') + expect(spawnArgs[3]).toBe('run') + expect(spawnArgs[4]).toBe('/some/path') + }) + + it('should set npm_config_dlx_cache_max_age env var for pnpm when force is true', async () => { + const packageSpec: DlxPackageSpec = { + name: '@coana-tech/cli', + version: '1.0.0', + } + + await spawnDlx(packageSpec, ['run', '/some/path'], { + agent: 'pnpm', + force: true, + }) + + expect(mockShadowPnpmBin).toHaveBeenCalledTimes(1) + const [, options] = mockShadowPnpmBin.mock.calls[0] + + // Verify that the env var is set to force cache bypass. + expect(options.env).toBeDefined() + expect(options.env.npm_config_dlx_cache_max_age).toBe('0') + }) + + it('should handle pinned version without silent flag by default', async () => { + const packageSpec: DlxPackageSpec = { + name: '@coana-tech/cli', + version: '1.0.0', + } + + await spawnDlx(packageSpec, ['run', '/some/path'], { agent: 'pnpm' }) + + expect(mockShadowPnpmBin).toHaveBeenCalledTimes(1) + const [spawnArgs] = mockShadowPnpmBin.mock.calls[0] + + // For pinned versions, silent defaults to false. + expect(spawnArgs[0]).toBe('dlx') + expect(spawnArgs[1]).toBe('@coana-tech/cli@1.0.0') + }) + }) +})