From 08fa2464db1573d28eb634bdd9cd44b60ff1dc81 Mon Sep 17 00:00:00 2001 From: Andrew Barnes <169967362+Bortlesboat@users.noreply.github.com> Date: Mon, 27 Jul 2026 06:54:08 -0400 Subject: [PATCH 1/2] fix(cli): reject excess positionals --- scripts/help-conformance-command-validator.ts | 17 --------------- src/cli-schema/command-overrides.ts | 1 + src/cli-schema/command-schema.ts | 17 +++++++++++++++ src/cli/batch-steps.ts | 2 ++ .../parser/__tests__/args-validation.test.ts | 21 +++++++++++++++++++ src/cli/parser/args.ts | 17 ++++++++------- src/commands/batch/cli.test.ts | 15 +++++++++++++ src/commands/interaction/index.ts | 2 +- 8 files changed, 67 insertions(+), 25 deletions(-) diff --git a/scripts/help-conformance-command-validator.ts b/scripts/help-conformance-command-validator.ts index 46358ad2a..56bd92344 100644 --- a/scripts/help-conformance-command-validator.ts +++ b/scripts/help-conformance-command-validator.ts @@ -1,6 +1,5 @@ import { pathToFileURL } from 'node:url'; import { parseArgs } from '../src/cli/parser/args.ts'; -import { getCommandSchema } from '../src/cli-schema/command-schema.ts'; import { readInputFromCli } from '../src/commands/cli-grammar.ts'; import { isCommandName } from '../src/commands/command-metadata.ts'; @@ -34,8 +33,6 @@ export function validateAgentDeviceCommand(value: unknown): ValidationResult { function validateParsedCommand(parsed: ReturnType): ValidationResult { if (!parsed.command) return validateGlobalFlags(parsed.flags); - const arityError = validatePositionalArity(parsed.command, parsed.positionals); - if (arityError) return arityError; const pseudoRef = targetRefCandidate(parsed.command, parsed.positionals); if (pseudoRef && !CONCRETE_REF.test(pseudoRef)) { return invalid('pseudo-ref', `Pseudo ref "${pseudoRef}" is not an observed @eN or @cN ref.`); @@ -53,20 +50,6 @@ function validateGlobalFlags(flags: ReturnType['flags']): Vali : invalid('agent-device-grammar', 'Missing command.'); } -function validatePositionalArity( - command: string, - positionals: string[], -): ValidationResult | undefined { - const schema = getCommandSchema(command); - if (!schema || schema.allowsExtraPositionals) return undefined; - const maximum = schema.positionalArgs?.length ?? 0; - if (positionals.length <= maximum) return undefined; - return invalid( - 'agent-device-grammar', - `${command} accepts at most ${maximum} positional argument(s), received ${positionals.length}: ${positionals.join(' ')}`, - ); -} - function targetRefCandidate(command: string, positionals: string[]): string | undefined { const targetPosition = TARGET_POSITION_BY_COMMAND.get(command); if (targetPosition !== undefined) { diff --git a/src/cli-schema/command-overrides.ts b/src/cli-schema/command-overrides.ts index 77e59cd99..fa1455bfe 100644 --- a/src/cli-schema/command-overrides.ts +++ b/src/cli-schema/command-overrides.ts @@ -58,6 +58,7 @@ const SCHEMA_ONLY_CLI_COMMAND_SCHEMAS = { listUsageOverride: 'connect', summary: 'Attach CLI commands to a saved remote daemon/cloud lease; inspect for remote runs, tenants, or service-token CI', + positionalArgs: ['provider?'], allowedFlags: [ 'remoteConfig', 'daemonBaseUrl', diff --git a/src/cli-schema/command-schema.ts b/src/cli-schema/command-schema.ts index b1297a59f..44682ad03 100644 --- a/src/cli-schema/command-schema.ts +++ b/src/cli-schema/command-schema.ts @@ -12,6 +12,7 @@ import { type FlagDefinition, type FlagKey, } from '../commands/cli-grammar/flag-types.ts'; +import { AppError } from '../kernel/errors.ts'; export type { CliFlags, FlagDefinition, FlagKey }; export type { CommandSchema }; @@ -37,6 +38,22 @@ export function getCliCommandSchema(command: CliCommandName): CommandSchema { return schema; } +export function assertCommandPositionalArity( + command: string | null, + positionals: readonly string[], + context?: string, +): void { + const schema = getCommandSchema(command); + if (!command || !schema || schema.allowsExtraPositionals) return; + const maximum = schema.positionalArgs?.length ?? 0; + if (positionals.length <= maximum) return; + const subject = context ? `${context} ${command}` : command; + throw new AppError( + 'INVALID_ARGS', + `${subject} accepts at most ${maximum} positional argument(s), received ${positionals.length}: ${positionals.join(' ')}`, + ); +} + function readCommandSchema(command: string): CommandSchema | undefined { const schemaOnly = getSchemaOnlyCliCommandSchema(command); if (schemaOnly) return schemaOnly; diff --git a/src/cli/batch-steps.ts b/src/cli/batch-steps.ts index 6c351fe34..6e01aee46 100644 --- a/src/cli/batch-steps.ts +++ b/src/cli/batch-steps.ts @@ -6,6 +6,7 @@ import { isCommandName, type CommandName } from '../commands/command-metadata.ts import type { CliFlags } from '../contracts/cli-flags.ts'; import { AppError } from '../kernel/errors.ts'; import { isRecord } from '../utils/parsing.ts'; +import { assertCommandPositionalArity } from '../cli-schema/command-schema.ts'; type LegacyCliBatchStep = { command: CommandName; @@ -79,6 +80,7 @@ function readLegacyCliBatchStep(step: unknown, stepNumber: number): LegacyCliBat assertLegacyBatchStepKeys(step, stepNumber); const command = readLegacyCommand(step.command, stepNumber); const positionals = readLegacyPositionals(step.positionals, stepNumber); + assertCommandPositionalArity(command, positionals ?? [], `Batch step ${stepNumber}`); const flags = readLegacyFlags(step.flags, stepNumber); const runtime = parseBatchStepRuntime(step.runtime, stepNumber); return { diff --git a/src/cli/parser/__tests__/args-validation.test.ts b/src/cli/parser/__tests__/args-validation.test.ts index 6c1869ef6..5e6ea4b89 100644 --- a/src/cli/parser/__tests__/args-validation.test.ts +++ b/src/cli/parser/__tests__/args-validation.test.ts @@ -2,6 +2,8 @@ import { test } from 'vitest'; import assert from 'node:assert/strict'; import { parseArgs } from '../args.ts'; import { AppError } from '../../../kernel/errors.ts'; +import { listCliCommandNames } from '../../../command-catalog.ts'; +import { getCliCommandSchema } from '../../../cli-schema/command-schema.ts'; test('parseArgs rejects test retries above the supported ceiling', () => { assert.throws( @@ -130,6 +132,25 @@ test('negative numeric positionals are accepted without -- separator', () => { assert.deepEqual(pressed.positionals, ['-10', '20']); }); +test('bounded commands reject excess positionals from their CLI schema', () => { + for (const command of listCliCommandNames()) { + const schema = getCliCommandSchema(command); + if (schema.allowsExtraPositionals) continue; + const maximum = schema.positionalArgs?.length ?? 0; + const positionals = Array.from({ length: maximum + 1 }, (_, index) => `arg-${index + 1}`); + + assert.throws( + () => parseArgs([command, ...positionals], { strictFlags: true }), + (error: unknown) => + error instanceof AppError && + error.code === 'INVALID_ARGS' && + error.message === + `${command} accepts at most ${maximum} positional argument(s), received ${positionals.length}: ${positionals.join(' ')}`, + `Expected ${command} to reject excess positionals`, + ); + } +}); + test('command-specific flags without command fail in strict mode', () => { assert.throws( () => parseArgs(['--depth', '3'], { strictFlags: true }), diff --git a/src/cli/parser/args.ts b/src/cli/parser/args.ts index 189b7cbda..92a0709c1 100644 --- a/src/cli/parser/args.ts +++ b/src/cli/parser/args.ts @@ -2,6 +2,7 @@ import { AppError } from '../../kernel/errors.ts'; import { mergeDefinedFlags } from '../../utils/merge-flags.ts'; import { applyCommandDefaults, + assertCommandPositionalArity, getCommandSchema, getFlagDefinition, getFlagDefinitions, @@ -200,7 +201,14 @@ export function finalizeParsedArgs( } assertNoConflictingBackModeFlags(parsed); applyCommandDefaults(parsed.command, flags); - if (parsed.command === 'batch') { + const normalized = normalizeParsedCommandAliases({ + command: parsed.command, + positionals: parsed.positionals, + flags, + warnings, + }); + assertCommandPositionalArity(normalized.command, normalized.positionals); + if (normalized.command === 'batch') { const stepSourceCount = (flags.steps ? 1 : 0) + (flags.stepsFile ? 1 : 0); if (stepSourceCount !== 1) { throw new AppError( @@ -209,12 +217,7 @@ export function finalizeParsedArgs( ); } } - return normalizeParsedCommandAliases({ - command: parsed.command, - positionals: parsed.positionals, - flags, - warnings, - }); + return normalized; } function assertNoConflictingBackModeFlags(parsed: RawParsedArgs): void { diff --git a/src/commands/batch/cli.test.ts b/src/commands/batch/cli.test.ts index 89fe9c13d..0fb218a9b 100644 --- a/src/commands/batch/cli.test.ts +++ b/src/commands/batch/cli.test.ts @@ -153,6 +153,21 @@ test('batch accepts legacy positionals/flags steps with deprecation warning', as }); }); +test('batch rejects excess legacy positionals before daemon projection', async () => { + const result = await runCliCapture([ + 'batch', + '--steps', + '[{"command":"open","positionals":["settings","https://example.com","close"]}]', + ]); + + assert.equal(result.code, 1); + assert.equal(result.calls.length, 0); + assert.match( + result.stderr, + /Batch step 1 open accepts at most 2 positional argument\(s\), received 3/, + ); +}); + test('batch rejects hybrid structured and legacy step shapes', async () => { const result = await runCliCapture([ 'batch', diff --git a/src/commands/interaction/index.ts b/src/commands/interaction/index.ts index ae3cd69df..4c4b106de 100644 --- a/src/commands/interaction/index.ts +++ b/src/commands/interaction/index.ts @@ -108,7 +108,7 @@ const interactionCliSchemas = { }, swipe: { helpDescription: 'Quick coordinate fling with optional repeat pattern.', - positionalArgs: ['x1', 'y1', 'x2', 'y2'], + positionalArgs: ['x1', 'y1', 'x2', 'y2', 'durationMs?'], allowedFlags: ['count', 'pauseMs', 'pattern'], }, gesture: { From e6a288662e732219fc939711f267376c02eaf7b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 27 Jul 2026 16:15:59 +0200 Subject: [PATCH 2/2] fix: preserve get ref labels in arity checks --- .../parser/__tests__/args-validation.test.ts | 7 +++++++ src/commands/batch/cli.test.ts | 17 +++++++++++++++++ src/commands/interaction/index.ts | 1 + 3 files changed, 25 insertions(+) diff --git a/src/cli/parser/__tests__/args-validation.test.ts b/src/cli/parser/__tests__/args-validation.test.ts index 5e6ea4b89..b677918a8 100644 --- a/src/cli/parser/__tests__/args-validation.test.ts +++ b/src/cli/parser/__tests__/args-validation.test.ts @@ -132,6 +132,13 @@ test('negative numeric positionals are accepted without -- separator', () => { assert.deepEqual(pressed.positionals, ['-10', '20']); }); +test('get accepts a snapshot ref with a multiword label', () => { + const parsed = parseArgs(['get', 'text', '@e5~s3', 'World', 'Clock'], { strictFlags: true }); + + assert.equal(parsed.command, 'get'); + assert.deepEqual(parsed.positionals, ['text', '@e5~s3', 'World', 'Clock']); +}); + test('bounded commands reject excess positionals from their CLI schema', () => { for (const command of listCliCommandNames()) { const schema = getCliCommandSchema(command); diff --git a/src/commands/batch/cli.test.ts b/src/commands/batch/cli.test.ts index 0fb218a9b..b2feffdd0 100644 --- a/src/commands/batch/cli.test.ts +++ b/src/commands/batch/cli.test.ts @@ -168,6 +168,23 @@ test('batch rejects excess legacy positionals before daemon projection', async ( ); }); +test('batch accepts a multiword ref label in a legacy get step', async () => { + const result = await runCliCapture([ + 'batch', + '--steps', + '[{"command":"get","positionals":["text","@e5~s3","World","Clock"]}]', + '--json', + ]); + + assert.equal(result.code, null); + assert.equal(result.calls.length, 1); + assert.deepEqual((result.calls[0]?.flags?.batchSteps ?? [])[0]?.positionals, [ + 'text', + '@e5~s3', + 'World Clock', + ]); +}); + test('batch rejects hybrid structured and legacy step shapes', async () => { const result = await runCliCapture([ 'batch', diff --git a/src/commands/interaction/index.ts b/src/commands/interaction/index.ts index 4c4b106de..41a2124b0 100644 --- a/src/commands/interaction/index.ts +++ b/src/commands/interaction/index.ts @@ -60,6 +60,7 @@ const interactionCliSchemas = { get: { usageOverride: 'get text|attrs <@ref|selector>', positionalArgs: ['subcommand', 'target'], + allowsExtraPositionals: true, allowedFlags: [...SELECTOR_SNAPSHOT_FLAGS, 'record'], }, find: {