From a3e2c97fd4e300e8d6cb07e006b4dccbd7a7e7f5 Mon Sep 17 00:00:00 2001 From: reggi Date: Fri, 1 May 2026 22:33:57 -0400 Subject: [PATCH] fix: dynamically locate command path in argv for flags() offset Global flags before the command name (e.g., npm --registry url trust list) shifted command positions in argv, causing flags() to miscalculate the nopt offset with its fixed 2+depth formula. Now flags() accepts the full commandPath array and searches for the contiguous command sequence in argv, correctly skipping any preceding global flags. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/base-cmd.js | 33 +++++++++++++-- lib/npm.js | 4 +- test/lib/base-cmd.js | 97 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 128 insertions(+), 6 deletions(-) diff --git a/lib/base-cmd.js b/lib/base-cmd.js index 089ec680c7425..dc7cba442e3a1 100644 --- a/lib/base-cmd.js +++ b/lib/base-cmd.js @@ -276,7 +276,7 @@ class BaseCommand { this.workspacePaths = [...ws.values()] } - flags (depth = 1) { + flags (commandPath = []) { const commandDefinitions = this.constructor.definitions || [] // Build types, shorthands, and defaults from definitions @@ -315,9 +315,9 @@ class BaseCommand { const argv = this.config.argv if (argv && argv.length > 0) { // config.argv contains the full command line including node, npm, and command names - // Format: ['node', 'npm', 'command', 'subcommand', 'positional', '--flags'] - // depth tells us how many command names to skip (1 for top-level, 2 for subcommand, etc.) - const offset = 2 + depth // Skip 'node', 'npm', and all command/subcommand names + // Global flags before the command name shift command positions, so we find + // the actual command path in argv rather than assuming a fixed offset. + const offset = this.#findCommandOffset(argv, commandPath) parsed = nopt(types, cmdShorthands, argv, offset) remains = parsed.argv.remain delete parsed.argv @@ -363,6 +363,31 @@ class BaseCommand { return [{ ...defaults, ...filtered }, remains] } + // Find the offset into argv where command arguments begin (past command/subcommand names). + // Global flags before the command name (e.g., npm --registry url trust list) shift command + // positions, so we search for the contiguous commandPath sequence rather than using a fixed offset. + #findCommandOffset (argv, commandPath) { + if (commandPath.length === 0) { + // No command path provided, skip 'node', 'npm', and one command name (legacy default) + return 3 + } + // Search for the full contiguous commandPath sequence starting from index 2 (past 'node', 'npm') + for (let i = 2; i <= argv.length - commandPath.length; i++) { + let match = true + for (let j = 0; j < commandPath.length; j++) { + if (argv[i + j] !== commandPath[j]) { + match = false + break + } + } + if (match) { + return i + commandPath.length + } + } + // Fallback: skip 'node', 'npm', and assume command names start at position 2 + return 2 + commandPath.length + } + // Validate flags and throw errors for unknown flags or unexpected positionals #validateFlags (parsed, commandDefinitions, remains) { // Build a set of all valid flag names (global + command-specific + shorthands) diff --git a/lib/npm.js b/lib/npm.js index 2b4b2843e9218..211c1fe959478 100644 --- a/lib/npm.js +++ b/lib/npm.js @@ -284,8 +284,8 @@ class Npm { // Execute command with or without definitions if (Command.definitions) { // config.argv contains the full argv with flags (set by Config in production, by MockNpm in tests) - // Pass depth so flags() knows how many command names to skip - const [flags, positionalArgs] = commandInstance.flags(commandPath.length) + // Pass commandPath so flags() can locate the command names in argv dynamically + const [flags, positionalArgs] = commandInstance.flags(commandPath) return time.start(`command:${commandName}`, () => execWorkspaces ? commandInstance.execWorkspaces(positionalArgs, flags) diff --git a/test/lib/base-cmd.js b/test/lib/base-cmd.js index 41bade7298b90..8060851b71b70 100644 --- a/test/lib/base-cmd.js +++ b/test/lib/base-cmd.js @@ -677,3 +677,100 @@ t.test('flags() does not throw when positionals is null (unlimited)', async t => t.same(remains, ['pkg1', 'extra1', 'extra2'], 'all positionals are in remains') t.equal(flags.id, null, 'id flag uses default') }) + +t.test('flags() correctly skips global flags before command name', async t => { + const { npm } = await loadMockNpm(t) + + class TestCommand extends BaseCommand { + static name = 'test-command' + static description = 'Test command' + static params = ['mountain'] + + static definitions = [ + new Definition('mountain', { + type: String, + default: 'everest', + description: 'Your favorite mountain', + usage: '--mountain=', + }), + ] + + async exec () { + return this.flags(['test-command']) + } + } + + // Global flags (--registry) appear before the command name in argv + npm.config.argv = ['node', 'npm', '--registry', 'http://example.com', 'test-command', '--mountain=denali'] + + const command = new TestCommand(npm) + const [flags, remains] = await command.exec() + + t.equal(flags.mountain, 'denali', 'parses command flag correctly despite global flags before command') + t.same(remains, [], 'no unexpected positional args') +}) + +t.test('flags() correctly skips global flags before subcommand names', async t => { + const { npm } = await loadMockNpm(t) + + class TestSubCommand extends BaseCommand { + static name = 'sub' + static description = 'Test subcommand' + static params = ['mountain'] + + static definitions = [ + new Definition('mountain', { + type: String, + default: 'everest', + description: 'Your favorite mountain', + usage: '--mountain=', + }), + ] + + async exec () { + return this.flags(['parent', 'sub']) + } + } + + // Global flags push command names to higher positions + npm.config.argv = ['node', 'npm', '--registry', 'http://example.com', 'parent', 'sub', '--mountain=rainier', 'positional-arg'] + + const command = new TestSubCommand(npm) + const [flags, remains] = await command.exec() + + t.equal(flags.mountain, 'rainier', 'parses subcommand flag correctly despite global flags') + t.same(remains, ['positional-arg'], 'positional arg is preserved') +}) + +t.test('flags() handles command name appearing as a flag value', async t => { + const { npm } = await loadMockNpm(t) + + class TestCommand extends BaseCommand { + static name = 'trust' + static description = 'Test command' + static params = ['mountain'] + + static definitions = [ + new Definition('mountain', { + type: String, + default: 'everest', + description: 'Your favorite mountain', + usage: '--mountain=', + }), + ] + + async exec () { + return this.flags(['trust', 'list']) + } + } + + // 'trust' appears as both a flag value and a command name + // The contiguous sequence ['trust', 'list'] only matches at position 5 + npm.config.argv = ['node', 'npm', '--scope', 'trust', 'trust', 'list', '--mountain=k2'] + + const command = new TestCommand(npm) + const [flags, remains] = await command.exec() + + t.equal(flags.mountain, 'k2', 'finds the correct contiguous command path match') + t.same(remains, [], 'no unexpected positional args') +})