[Hackathon] fix: reject empty and whitespace ids with VALIDATION_ERROR in read-by-id commands

src/commands/project.ts

@@ -724,9 +724,6 @@ export async function runDelete(
   deps: ProjectDeps = {},
 ): Promise<CliDeleteProjectResponse> {
   assertIdempotencyKey(opts.idempotencyKey);
-  if (opts.projectId === undefined || opts.projectId.trim().length === 0) {
-    throw localValidationError('<project-id> is required');
-  }
 
   if (!opts.confirm && !opts.dryRun) {
     throw ApiError.fromEnvelope({

src/commands/test.ts

@@ -2012,7 +2012,6 @@ export async function runPlanPut(
   deps: TestDeps = {},
 ): Promise<CliPutPlanStepsResponse> {
   assertIdempotencyKey(opts.idempotencyKey);
-  requireNonEmpty('test-id', opts.testId);
   requireNonEmpty('steps', opts.stepsFile);
 
   if (
@@ -2425,7 +2424,6 @@ export async function runUpdate(
   deps: TestDeps = {},
 ): Promise<CliUpdateTestResponse> {
   assertIdempotencyKey(opts.idempotencyKey);
-  requireNonEmpty('test-id', opts.testId);
   // P1-3: client-side length checks matching server limits.
   if (opts.name !== undefined && opts.name.trim().length === 0) {
     throw localValidationError(
@@ -2568,7 +2566,6 @@ export async function runDelete(
   deps: TestDeps = {},
 ): Promise<CliDeleteTestResponse> {
   assertIdempotencyKey(opts.idempotencyKey);
-  requireNonEmpty('test-id', opts.testId);
 
   if (!opts.confirm && !opts.dryRun) {
     throw ApiError.fromEnvelope({
@@ -5801,7 +5798,6 @@ export async function runCodePut(
   deps: TestDeps = {},
 ): Promise<CliPutTestCodeResponse> {
   assertIdempotencyKey(opts.idempotencyKey);
-  requireNonEmpty('test-id', opts.testId);
   requireNonEmpty('code-file', opts.codeFile);
   assertPythonCodeFile(opts.codeFile);
 
@@ -16226,9 +16222,6 @@ export async function runFlaky(
   const stderrFn = deps.stderr ?? ((line: string) => process.stderr.write(`${line}\n`));
   const out = makeOutput(opts.output, deps);
 
-  if (typeof opts.testId !== 'string' || opts.testId.length === 0) {
-    throw localValidationError('test-id', 'is required');
-  }
   if (!Number.isInteger(opts.runs) || opts.runs < 1 || opts.runs > MAX_FLAKY_RUNS) {
     throw localValidationError('runs', `must be an integer between 1 and ${MAX_FLAKY_RUNS}`);
   }

src/index.ts

@@ -22,6 +22,7 @@ import { createUsageCommand } from './commands/usage.js';
 import { resolveProfileName } from './lib/config.js';
 import { classifyBillingRefusal } from './lib/billing-refusal.js';
 import { TARGETS, type AgentTarget } from './lib/agent-targets.js';
+import { validatePositionalArgs } from './lib/args.js';
 import {
   ApiError,
   CLIError,
@@ -285,6 +286,13 @@ function telemetryGlobals(): {
 // action; the helper self-gates (text-only, non-dry-run, a small command
 // allowlist, opt-out via TESTSPRITE_NO_SKILL_WARNING) and never throws.
 program.hook('preAction', (_thisCommand, actionCommand) => {
+  const commandPath = commandPathOf(actionCommand);
+  // Record which leaf command ran, for the telemetry emit around parseAsync.
+  ranCommandPath = commandPath;
+
+  // Validate positional arguments across all commands before executing their actions.
+  validatePositionalArgs(actionCommand);
+
   const globals = actionCommand.optsWithGlobals() as {
     output?: string;
     profile?: string;
@@ -295,9 +303,6 @@ program.hook('preAction', (_thisCommand, actionCommand) => {
     local?: string;
     localHost?: string;
   };
-  const commandPath = commandPathOf(actionCommand);
-  // Record which leaf command ran, for the telemetry emit around parseAsync.
-  ranCommandPath = commandPath;
   // Preserve explicit --local attempts, including refusals and borrowed tunnels;
   // only flag presence is recorded. Automatic tunnel paths record the same fact.
   if (globals.local !== undefined) recordTelemetryExtras({ local: true });

src/lib/args.test.ts

@@ -0,0 +1,163 @@
+import { Command, Argument } from 'commander';
+import { describe, expect, it, vi } from 'vitest';
+import { formatArgumentName, validatePositionalArgs } from './args.js';
+import { createProjectCommand } from '../commands/project.js';
+import { createTestCommand } from '../commands/test.js';
+
+describe('formatArgumentName', () => {
+  it('formats required positional argument as <name>', () => {
+    const arg = new Argument('<project-id>');
+    expect(formatArgumentName(arg)).toBe('<project-id>');
+  });
+
+  it('formats optional positional argument as [name]', () => {
+    const arg = new Argument('[environment]');
+    expect(formatArgumentName(arg)).toBe('[environment]');
+  });
+
+  it('formats required variadic argument as <name...>', () => {
+    const arg = new Argument('<run-id...>');
+    expect(formatArgumentName(arg)).toBe('<run-id...>');
+  });
+
+  it('formats optional variadic argument as [name...]', () => {
+    const arg = new Argument('[test-ids...]');
+    expect(formatArgumentName(arg)).toBe('[test-ids...]');
+  });
+});
+
+describe('validatePositionalArgs with throwaway Commander program', () => {
+  function makeProgram(actionFn = vi.fn()) {
+    const program = new Command();
+    program.exitOverride();
+    program.hook('preAction', (_thisCmd, actionCmd) => {
+      validatePositionalArgs(actionCmd);
+    });
+
+    program.command('test-req <id>').action(actionFn);
+
+    program.command('test-opt [opt]').action(actionFn);
+
+    program.command('test-var <first> [rest...]').action(actionFn);
+
+    return { program, actionFn };
+  }
+
+  it('passes when valid positional arguments are provided', async () => {
+    const { program, actionFn } = makeProgram();
+    await program.parseAsync(['node', 'test', 'test-req', 'valid-id']);
+    expect(actionFn).toHaveBeenCalledWith('valid-id', expect.anything(), expect.anything());
+  });
+
+  it('rejects empty string required positional with VALIDATION_ERROR (exit 5)', async () => {
+    const { program, actionFn } = makeProgram();
+    await expect(program.parseAsync(['node', 'test', 'test-req', ''])).rejects.toMatchObject({
+      code: 'VALIDATION_ERROR',
+      exitCode: 5,
+      nextAction: 'Argument `<id>` is invalid: cannot be empty or whitespace-only.',
+      details: {
+        field: '<id>',
+        reason: 'cannot be empty or whitespace-only',
+      },
+    });
+    expect(actionFn).not.toHaveBeenCalled();
+  });
+
+  it('rejects whitespace-only required positional with VALIDATION_ERROR (exit 5)', async () => {
+    const { program, actionFn } = makeProgram();
+    await expect(program.parseAsync(['node', 'test', 'test-req', '   \t  '])).rejects.toMatchObject(
+      {
+        code: 'VALIDATION_ERROR',
+        exitCode: 5,
+        nextAction: 'Argument `<id>` is invalid: cannot be empty or whitespace-only.',
+      },
+    );
+    expect(actionFn).not.toHaveBeenCalled();
+  });
+
+  it('passes when optional positional is omitted', async () => {
+    const { program, actionFn } = makeProgram();
+    await program.parseAsync(['node', 'test', 'test-opt']);
+    expect(actionFn).toHaveBeenCalled();
+  });
+
+  it('rejects empty string optional positional when passed explicitly', async () => {
+    const { program, actionFn } = makeProgram();
+    await expect(program.parseAsync(['node', 'test', 'test-opt', ''])).rejects.toMatchObject({
+      code: 'VALIDATION_ERROR',
+      exitCode: 5,
+      nextAction: 'Argument `[opt]` is invalid: cannot be empty or whitespace-only.',
+    });
+    expect(actionFn).not.toHaveBeenCalled();
+  });
+
+  it('rejects whitespace-only optional positional when passed explicitly', async () => {
+    const { program, actionFn } = makeProgram();
+    await expect(program.parseAsync(['node', 'test', 'test-opt', '   '])).rejects.toMatchObject({
+      code: 'VALIDATION_ERROR',
+      exitCode: 5,
+      nextAction: 'Argument `[opt]` is invalid: cannot be empty or whitespace-only.',
+    });
+    expect(actionFn).not.toHaveBeenCalled();
+  });
+
+  it('rejects empty or whitespace elements in variadic positional arguments', async () => {
+    const { program, actionFn } = makeProgram();
+    await expect(
+      program.parseAsync(['node', 'test', 'test-var', 'ok-first', 'valid-item', '   ']),
+    ).rejects.toMatchObject({
+      code: 'VALIDATION_ERROR',
+      exitCode: 5,
+      nextAction: 'Argument `[rest...]` is invalid: cannot be empty or whitespace-only.',
+    });
+    expect(actionFn).not.toHaveBeenCalled();
+  });
+});
+
+describe('command validation via parseAsync with real commands', () => {
+  function makeCliProgram(cmd: Command) {
+    const program = new Command();
+    program.option('--dry-run', 'dry run mode');
+    program.hook('preAction', (_thisCmd, actionCmd) => {
+      validatePositionalArgs(actionCmd);
+    });
+    program.addCommand(cmd);
+
+    function applyExitOverride(c: Command): void {
+      c.exitOverride();
+      for (const sub of c.commands) {
+        applyExitOverride(sub);
+      }
+    }
+    applyExitOverride(program);
+    return program;
+  }
+
+  it('rejects project get with whitespace project id', async () => {
+    const program = makeCliProgram(createProjectCommand());
+
+    await expect(
+      program.parseAsync(['node', 'testsprite', 'project', 'get', '   ']),
+    ).rejects.toMatchObject({
+      code: 'VALIDATION_ERROR',
+      exitCode: 5,
+      nextAction: 'Argument `<project-id>` is invalid: cannot be empty or whitespace-only.',
+      details: {
+        field: '<project-id>',
+        reason: 'cannot be empty or whitespace-only',
+      },
+    });
+  });
+
+  it('rejects test get with empty test id under --dry-run', async () => {
+    const program = makeCliProgram(createTestCommand());
+
+    await expect(
+      program.parseAsync(['node', 'testsprite', 'test', 'get', '', '--dry-run']),
+    ).rejects.toMatchObject({
+      code: 'VALIDATION_ERROR',
+      exitCode: 5,
+      nextAction: 'Argument `<test-id>` is invalid: cannot be empty or whitespace-only.',
+    });
+  });
+});

src/lib/args.ts

@@ -0,0 +1,64 @@
+import type { Argument, Command } from 'commander';
+import { localValidationError } from './errors.js';
+
+/**
+ * Returns the formatted argument label matching Commander's display convention:
+ * `<name>` for required arguments, `[name]` for optional arguments, with `...`
+ * appended for variadic arguments.
+ */
+export function formatArgumentName(arg: Argument): string {
+  const name = arg.name() + (arg.variadic ? '...' : '');
+  return arg.required ? `<${name}>` : `[${name}]`;
+}
+
+/**
+ * Validates positional arguments on a Commander command before its action executes.
+ * Rejects any empty string or whitespace-only positional argument (including variadics)
+ * with exit 5 VALIDATION_ERROR.
+ */
+export function validatePositionalArgs(command: Command): void {
+  const registeredArgs = command.registeredArguments;
+  if (!registeredArgs || registeredArgs.length === 0) {
+    return;
+  }
+  const processedArgs = command.processedArgs;
+  if (!processedArgs || processedArgs.length === 0) {
+    return;
+  }
+
+  for (let i = 0; i < registeredArgs.length; i++) {
+    const argDef = registeredArgs[i];
+    if (!argDef) {
+      continue;
+    }
+    const val = processedArgs[i];
+
+    if (val === undefined) {
+      continue;
+    }
+
+    const argLabel = formatArgumentName(argDef);
+
+    if (typeof val === 'string') {
+      if (val.trim().length === 0) {
+        throw localValidationError(
+          argLabel,
+          'cannot be empty or whitespace-only',
+          undefined,
+          'argument',
+        );
+      }
+    } else if (Array.isArray(val)) {
+      for (const item of val) {
+        if (typeof item === 'string' && item.trim().length === 0) {
+          throw localValidationError(
+            argLabel,
+            'cannot be empty or whitespace-only',
+            undefined,
+            'argument',
+          );
+        }
+      }
+    }
+  }
+}

src/lib/errors.test.ts

@@ -602,6 +602,52 @@ describe('localValidationError', () => {
     expect(err.nextAction).toBe('Flag `--page-size` is invalid: must be a positive integer.');
     expect(err.nextAction.endsWith('..')).toBe(false);
   });
+
+  it('kind=argument formats bare positional name wrapped in angle brackets', () => {
+    const err = localValidationError(
+      'projectId',
+      'cannot be empty or whitespace-only',
+      undefined,
+      'argument',
+    );
+    expect(err.code).toBe('VALIDATION_ERROR');
+    expect(err.exitCode).toBe(5);
+    expect(err.nextAction).toBe(
+      'Argument `<project-id>` is invalid: cannot be empty or whitespace-only.',
+    );
+  });
+
+  it('kind=argument preserves existing bracketed argument names', () => {
+    const required = localValidationError(
+      '<project-id>',
+      'cannot be empty or whitespace-only',
+      undefined,
+      'argument',
+    );
+    expect(required.nextAction).toBe(
+      'Argument `<project-id>` is invalid: cannot be empty or whitespace-only.',
+    );
+
+    const optional = localValidationError(
+      '[output-path]',
+      'cannot be empty or whitespace-only',
+      undefined,
+      'argument',
+    );
+    expect(optional.nextAction).toBe(
+      'Argument `[output-path]` is invalid: cannot be empty or whitespace-only.',
+    );
+
+    const variadic = localValidationError(
+      '<run-id...>',
+      'cannot be empty or whitespace-only',
+      undefined,
+      'argument',
+    );
+    expect(variadic.nextAction).toBe(
+      'Argument `<run-id...>` is invalid: cannot be empty or whitespace-only.',
+    );
+  });
 });
 
 describe('ApiError.getDetail', () => {

src/lib/errors.ts

@@ -396,12 +396,18 @@ export function localValidationError(
   field: string,
   reason: string,
   accepted?: unknown,
-  kind: 'flag' | 'field' = 'flag',
+  kind: 'flag' | 'field' | 'argument' = 'flag',
 ): ApiError {
+  let argName = field;
+  if (kind === 'argument' && !field.startsWith('<') && !field.startsWith('[')) {
+    argName = `<${field.replace(/[A-Z]/g, ch => `-${ch.toLowerCase()}`)}>`;
+  }
   const subject =
     kind === 'flag'
       ? `Flag \`--${field.replace(/[A-Z]/g, ch => `-${ch.toLowerCase()}`)}\``
-      : `Field \`${field}\``;
+      : kind === 'argument'
+        ? `Argument \`${argName}\``
+        : `Field \`${field}\``;
   const punctuatedReason = reason.endsWith('.') ? reason : `${reason}.`;
   return ApiError.fromEnvelope({
     error: {