From 9ab5b6f60cf444f68e49006eb50980d4785f8207 Mon Sep 17 00:00:00 2001 From: prklm10 Date: Fri, 10 Jul 2026 17:50:03 +0530 Subject: [PATCH 1/2] fix(cli-app): parse app:exec loosely when command separator is missing Some environments strip the `--` separator before argv reaches the CLI (notably npm's npx PowerShell shim on Windows), which made `percy app:exec -- dotnet test` fail with "ParseError: Unexpected argument 'dotnet'" while the equivalent `percy exec` invocation only warned and ran the command. Reuse the exec command's loose definition for app:exec so both commands degrade the same way when the separator is missing. Co-Authored-By: Claude Fable 5 --- packages/cli-app/src/exec.js | 4 ++++ packages/cli-app/test/exec.test.js | 11 ++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/cli-app/src/exec.js b/packages/cli-app/src/exec.js index 10b54de21..e7fa6aec1 100644 --- a/packages/cli-app/src/exec.js +++ b/packages/cli-app/src/exec.js @@ -189,6 +189,10 @@ export const exec = command('exec', { // grouped flags are built-in flags .flags.filter(f => !f.group), + // some environments (e.g. npx PowerShell shims) strip the `--` separator; + // fall back to loose parsing like `percy exec` instead of hard failing + loose: ExecPlugin.default.definition.loose, + percy: { server: true, projectType: 'app', diff --git a/packages/cli-app/test/exec.test.js b/packages/cli-app/test/exec.test.js index b114676af..93082175e 100644 --- a/packages/cli-app/test/exec.test.js +++ b/packages/cli-app/test/exec.test.js @@ -28,12 +28,21 @@ describe('percy app:exec', () => { }); it('does not accept asset discovery options', async () => { + // with loose parsing, unrecognized exec options are treated as the command await expectAsync(exec(['--allowed-hostname', 'percy.io'])) - .toBeRejectedWithError("Unknown option '--allowed-hostname'"); + .toBeRejectedWithError('Command not found "--allowed-hostname"'); await expectAsync(start(['--network-idle-timeout', '500'])) .toBeRejectedWithError("Unknown option '--network-idle-timeout'"); }); + it('parses loosely when the command separator is missing', async () => { + // some environments (e.g. npx PowerShell shims) strip the `--` separator; + // the command should warn and run instead of throwing a parse error + expect(exec.definition.loose).toEqual(ExecPlugin.default.definition.loose); + await expectAsync(exec(['dotnet', 'test'])).not.toBeRejectedWithError( + "Unexpected argument 'dotnet'"); + }); + describe('maybeInjectMaestroServer', () => { function ctxFor(argv, addr = 'http://localhost:5338') { return { From 0fa31ddae6878f1b0d821757543440468c273d9a Mon Sep 17 00:00:00 2001 From: prklm10 Date: Tue, 14 Jul 2026 13:26:22 +0530 Subject: [PATCH 2/2] =?UTF-8?q?test(cli-app):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20hermetic=20loose-parsing=20spec,=20usage=20text?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Mock which.sync in the loose-parsing spec so no real binary is resolved or spawned; assert the loose warning is logged and that the argv is parsed as a command (rejected with 'Command not found "dotnet"' rather than a parse error), so the spec actually guards the regression. - Split the exec case out of 'does not accept asset discovery options' into its own spec named for what it now verifies: unrecognized exec options degrade into command lookup under loose parsing. - Update usage text to '[options] [--] ' for exec and app:exec since the separator is now optional (degrades to a warning). Co-Authored-By: Claude Fable 5 --- packages/cli-app/src/exec.js | 2 +- packages/cli-app/test/exec.test.js | 32 ++++++++++++++++++++++++------ packages/cli-exec/src/exec.js | 2 +- 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/packages/cli-app/src/exec.js b/packages/cli-app/src/exec.js index e7fa6aec1..e440566a3 100644 --- a/packages/cli-app/src/exec.js +++ b/packages/cli-app/src/exec.js @@ -182,7 +182,7 @@ export function maybeInjectScreenshotDir(ctx, log) { export const exec = command('exec', { description: 'Start and stop Percy around a supplied command for native apps', - usage: '[options] -- ', + usage: '[options] [--] ', commands: [start, stop, ping], flags: ExecPlugin.default.definition diff --git a/packages/cli-app/test/exec.test.js b/packages/cli-app/test/exec.test.js index 93082175e..44b036dc2 100644 --- a/packages/cli-app/test/exec.test.js +++ b/packages/cli-app/test/exec.test.js @@ -1,7 +1,7 @@ import fs from 'fs'; import os from 'os'; import path from 'path'; -import { setupTest } from '@percy/cli-command/test/helpers'; +import { logger, setupTest } from '@percy/cli-command/test/helpers'; import * as ExecPlugin from '@percy/cli-exec'; import { exec, start, stop, ping, @@ -28,19 +28,39 @@ describe('percy app:exec', () => { }); it('does not accept asset discovery options', async () => { - // with loose parsing, unrecognized exec options are treated as the command - await expectAsync(exec(['--allowed-hostname', 'percy.io'])) - .toBeRejectedWithError('Command not found "--allowed-hostname"'); await expectAsync(start(['--network-idle-timeout', '500'])) .toBeRejectedWithError("Unknown option '--network-idle-timeout'"); }); + it('degrades unrecognized exec options into the command (loose parsing trade-off)', async () => { + // with loose parsing, exec no longer hard-rejects unknown flags with + // "Unknown option" — a typo like --allowed-hostname falls through to + // command lookup and fails there instead + let { default: which } = await import('which'); + spyOn(which, 'sync').and.returnValue(null); + + await expectAsync(exec(['--allowed-hostname', 'percy.io'])) + .toBeRejectedWithError('Command not found "--allowed-hostname"'); + }); + it('parses loosely when the command separator is missing', async () => { // some environments (e.g. npx PowerShell shims) strip the `--` separator; // the command should warn and run instead of throwing a parse error expect(exec.definition.loose).toEqual(ExecPlugin.default.definition.loose); - await expectAsync(exec(['dotnet', 'test'])).not.toBeRejectedWithError( - "Unexpected argument 'dotnet'"); + + // stub command resolution so no real binary is looked up or spawned; the + // "Command not found" rejection proves `dotnet` was parsed as the command + // (not rejected as an unexpected argument) before reaching the lookup + let { default: which } = await import('which'); + spyOn(which, 'sync').and.returnValue(null); + + await expectAsync(exec(['dotnet', 'test'])) + .toBeRejectedWithError('Command not found "dotnet"'); + + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + `[percy] ${ExecPlugin.default.definition.loose}`, + '[percy] Error: Command not found "dotnet"' + ])); }); describe('maybeInjectMaestroServer', () => { diff --git a/packages/cli-exec/src/exec.js b/packages/cli-exec/src/exec.js index be4850e86..cfa6d81ac 100644 --- a/packages/cli-exec/src/exec.js +++ b/packages/cli-exec/src/exec.js @@ -8,7 +8,7 @@ import { waitForTimeout } from '@percy/client/utils'; export const exec = command('exec', { description: 'Start and stop Percy around a supplied command', - usage: '[options] -- ', + usage: '[options] [--] ', commands: [start, stop, ping, replay], flags: [{