diff --git a/src/main.tsx b/src/main.tsx index a7832be9b6..a2a0228386 100644 --- a/src/main.tsx +++ b/src/main.tsx @@ -203,6 +203,7 @@ import { SandboxManager } from './utils/sandbox/sandbox-adapter.js'; import { fetchSession, prepareApiRequest } from './utils/teleport/api.js'; import { checkOutTeleportedSessionBranch, processMessagesForTeleportResume, teleportToRemoteWithErrorHandling, validateGitState, validateSessionRepository } from './utils/teleport.js'; import { shouldEnableThinkingByDefault, type ThinkingConfig } from './utils/thinking.js'; +import { parsePositiveAmountArg, parsePositiveIntArg } from './utils/cli/numericArgs.js'; import { initUser, resetUserCache } from './utils/user.js'; import { getTmuxInstallInstructions, isTmuxAvailable, parsePRReference } from './utils/worktree.js'; @@ -945,19 +946,7 @@ async function run(): Promise { } catch (error) { throw new InvalidArgumentError(errorMessage(error)); } - })).option('--bare', 'Minimal mode: skip hooks, LSP, plugin sync, attribution, auto-memory, background prefetches, keychain reads, and CLAUDE.md auto-discovery. Sets CLAUDE_CODE_SIMPLE=1. Anthropic auth is strictly ANTHROPIC_API_KEY or apiKeyHelper via --settings (OAuth and keychain are never read). 3P providers (Bedrock/Vertex/Foundry) use their own credentials. Skills still resolve via /skill-name. Explicitly provide context via: --system-prompt[-file], --append-system-prompt[-file], --add-dir (CLAUDE.md dirs), --mcp-config, --settings, --agents, --plugin-dir.', () => true).addOption(new Option('--init', 'Run Setup hooks with init trigger, then continue').hideHelp()).addOption(new Option('--init-only', 'Run Setup and SessionStart:startup hooks, then exit').hideHelp()).addOption(new Option('--maintenance', 'Run Setup hooks with maintenance trigger, then continue').hideHelp()).addOption(new Option('--output-format ', 'Output format (only works with --print): "text" (default), "json" (single result), or "stream-json" (realtime streaming)').choices(['text', 'json', 'stream-json'])).addOption(new Option('--json-schema ', 'JSON Schema for structured output validation. ' + 'Example: {"type":"object","properties":{"name":{"type":"string"}},"required":["name"]}').argParser(String)).option('--include-hook-events', 'Include all hook lifecycle events in the output stream (only works with --output-format=stream-json)', () => true).option('--include-partial-messages', 'Include partial message chunks as they arrive (only works with --print and --output-format=stream-json)', () => true).addOption(new Option('--input-format ', 'Input format (only works with --print): "text" (default), or "stream-json" (realtime streaming input)').choices(['text', 'stream-json'])).option('--mcp-debug', '[DEPRECATED. Use --debug instead] Enable MCP debug mode (shows MCP server errors)', () => true).option('--dangerously-skip-permissions', 'Bypass all permission checks. Recommended only for sandboxes with no internet access.', () => true).option('--allow-dangerously-skip-permissions', 'Enable bypassing all permission checks as an option, without it being enabled by default. Recommended only for sandboxes with no internet access.', () => true).addOption(new Option('--thinking ', 'Thinking mode: enabled (equivalent to adaptive), disabled').choices(['enabled', 'adaptive', 'disabled']).hideHelp()).addOption(new Option('--max-thinking-tokens ', '[DEPRECATED. Use --thinking instead for newer models] Maximum number of thinking tokens (only works with --print)').argParser(Number).hideHelp()).addOption(new Option('--max-turns ', 'Maximum number of agentic turns in non-interactive mode. This will early exit the conversation after the specified number of turns. (only works with --print)').argParser(Number).hideHelp()).addOption(new Option('--max-budget-usd ', 'Maximum dollar amount to spend on API calls (only works with --print)').argParser(value => { - const amount = Number(value); - if (isNaN(amount) || amount <= 0) { - throw new Error('--max-budget-usd must be a positive number greater than 0'); - } - return amount; - })).addOption(new Option('--task-budget ', 'API-side task budget in tokens (output_config.task_budget)').argParser(value => { - const tokens = Number(value); - if (isNaN(tokens) || tokens <= 0 || !Number.isInteger(tokens)) { - throw new Error('--task-budget must be a positive integer'); - } - return tokens; - }).hideHelp()).option('--replay-user-messages', 'Re-emit user messages from stdin back on stdout for acknowledgment (only works with --input-format=stream-json and --output-format=stream-json)', () => true).addOption(new Option('--enable-auth-status', 'Enable auth status messages in SDK mode').default(false).hideHelp()).option('--allowedTools, --allowed-tools ', 'Comma or space-separated list of tool names to allow (e.g. "Bash(git:*) Edit")').option('--tools ', 'Specify the list of available tools from the built-in set. Use "" to disable all tools, "default" to use all tools, or specify tool names (e.g. "Bash,Edit,Read").').option('--disallowedTools, --disallowed-tools ', 'Comma or space-separated list of tool names to deny (e.g. "Bash(git:*) Edit")').option('--mcp-config ', 'Load MCP servers from JSON files or strings (space-separated)').addOption(new Option('--permission-prompt-tool ', 'MCP tool to use for permission prompts (only works with --print)').argParser(String).hideHelp()).addOption(new Option('--system-prompt ', 'System prompt to use for the session').argParser(String)).addOption(new Option('--system-prompt-file ', 'Read system prompt from a file').argParser(String).hideHelp()).addOption(new Option('--append-system-prompt ', 'Append a system prompt to the default system prompt').argParser(String)).addOption(new Option('--append-system-prompt-file ', 'Read system prompt from a file and append to the default system prompt').argParser(String).hideHelp()).addOption(new Option('--permission-mode ', 'Permission mode to use for the session').argParser(String).choices(PERMISSION_MODES)) + })).option('--bare', 'Minimal mode: skip hooks, LSP, plugin sync, attribution, auto-memory, background prefetches, keychain reads, and CLAUDE.md auto-discovery. Sets CLAUDE_CODE_SIMPLE=1. Anthropic auth is strictly ANTHROPIC_API_KEY or apiKeyHelper via --settings (OAuth and keychain are never read). 3P providers (Bedrock/Vertex/Foundry) use their own credentials. Skills still resolve via /skill-name. Explicitly provide context via: --system-prompt[-file], --append-system-prompt[-file], --add-dir (CLAUDE.md dirs), --mcp-config, --settings, --agents, --plugin-dir.', () => true).addOption(new Option('--init', 'Run Setup hooks with init trigger, then continue').hideHelp()).addOption(new Option('--init-only', 'Run Setup and SessionStart:startup hooks, then exit').hideHelp()).addOption(new Option('--maintenance', 'Run Setup hooks with maintenance trigger, then continue').hideHelp()).addOption(new Option('--output-format ', 'Output format (only works with --print): "text" (default), "json" (single result), or "stream-json" (realtime streaming)').choices(['text', 'json', 'stream-json'])).addOption(new Option('--json-schema ', 'JSON Schema for structured output validation. ' + 'Example: {"type":"object","properties":{"name":{"type":"string"}},"required":["name"]}').argParser(String)).option('--include-hook-events', 'Include all hook lifecycle events in the output stream (only works with --output-format=stream-json)', () => true).option('--include-partial-messages', 'Include partial message chunks as they arrive (only works with --print and --output-format=stream-json)', () => true).addOption(new Option('--input-format ', 'Input format (only works with --print): "text" (default), or "stream-json" (realtime streaming input)').choices(['text', 'stream-json'])).option('--mcp-debug', '[DEPRECATED. Use --debug instead] Enable MCP debug mode (shows MCP server errors)', () => true).option('--dangerously-skip-permissions', 'Bypass all permission checks. Recommended only for sandboxes with no internet access.', () => true).option('--allow-dangerously-skip-permissions', 'Enable bypassing all permission checks as an option, without it being enabled by default. Recommended only for sandboxes with no internet access.', () => true).addOption(new Option('--thinking ', 'Thinking mode: enabled (equivalent to adaptive), disabled').choices(['enabled', 'adaptive', 'disabled']).hideHelp()).addOption(new Option('--max-thinking-tokens ', '[DEPRECATED. Use --thinking instead for newer models] Maximum number of thinking tokens (only works with --print)').argParser(Number).hideHelp()).addOption(new Option('--max-turns ', 'Maximum number of agentic turns in non-interactive mode. This will early exit the conversation after the specified number of turns. (only works with --print)').argParser(value => parsePositiveIntArg('--max-turns', value)).hideHelp()).addOption(new Option('--max-budget-usd ', 'Maximum dollar amount to spend on API calls (only works with --print)').argParser(value => parsePositiveAmountArg('--max-budget-usd', value))).addOption(new Option('--task-budget ', 'API-side task budget in tokens (output_config.task_budget)').argParser(value => parsePositiveIntArg('--task-budget', value)).hideHelp()).option('--replay-user-messages', 'Re-emit user messages from stdin back on stdout for acknowledgment (only works with --input-format=stream-json and --output-format=stream-json)', () => true).addOption(new Option('--enable-auth-status', 'Enable auth status messages in SDK mode').default(false).hideHelp()).option('--allowedTools, --allowed-tools ', 'Comma or space-separated list of tool names to allow (e.g. "Bash(git:*) Edit")').option('--tools ', 'Specify the list of available tools from the built-in set. Use "" to disable all tools, "default" to use all tools, or specify tool names (e.g. "Bash,Edit,Read").').option('--disallowedTools, --disallowed-tools ', 'Comma or space-separated list of tool names to deny (e.g. "Bash(git:*) Edit")').option('--mcp-config ', 'Load MCP servers from JSON files or strings (space-separated)').addOption(new Option('--permission-prompt-tool ', 'MCP tool to use for permission prompts (only works with --print)').argParser(String).hideHelp()).addOption(new Option('--system-prompt ', 'System prompt to use for the session').argParser(String)).addOption(new Option('--system-prompt-file ', 'Read system prompt from a file').argParser(String).hideHelp()).addOption(new Option('--append-system-prompt ', 'Append a system prompt to the default system prompt').argParser(String)).addOption(new Option('--append-system-prompt-file ', 'Read system prompt from a file and append to the default system prompt').argParser(String).hideHelp()).addOption(new Option('--permission-mode ', 'Permission mode to use for the session').argParser(String).choices(PERMISSION_MODES)) .option('-c, --continue', 'Continue the most recent conversation in the current directory', () => true) .option('-r, --resume [value]', 'Resume a conversation by session ID, or open interactive picker with optional search term', value => value || true) .option('--fork-session', 'When resuming, branch the conversation into a new session ID; does not create filesystem or worktree isolation (use with --resume or --continue)', () => true) diff --git a/src/utils/cli/numericArgs.test.ts b/src/utils/cli/numericArgs.test.ts new file mode 100644 index 0000000000..b63f8a9eb0 --- /dev/null +++ b/src/utils/cli/numericArgs.test.ts @@ -0,0 +1,152 @@ +import { expect, test } from 'bun:test' +import { InvalidArgumentError } from '@commander-js/extra-typings' + +import { + parsePositiveAmountArg, + parsePositiveIntArg, +} from './numericArgs.js' + +test('accepts a positive integer', () => { + expect(parsePositiveIntArg('--max-turns', '3')).toBe(3) + expect(parsePositiveIntArg('--max-turns', '100')).toBe(100) +}) + +test('rejects non-numeric input that Number() coerces to NaN', () => { + // The bug: `.argParser(Number)` turned "abc"/"10x" into NaN, and the cap + // check `maxTurns && turnCount > maxTurns` is falsy for NaN, so the agent + // ran with no turn limit at all. + for (const bad of ['abc', '10x', '', ' ']) { + expect(() => parsePositiveIntArg('--max-turns', bad)).toThrow( + InvalidArgumentError, + ) + } +}) + +test('rejects zero and negatives', () => { + // 0 is falsy (unbounded); a negative cap fires on turn 1 (immediate exit). + expect(() => parsePositiveIntArg('--max-turns', '0')).toThrow( + InvalidArgumentError, + ) + expect(() => parsePositiveIntArg('--max-turns', '-5')).toThrow( + InvalidArgumentError, + ) +}) + +test('rejects non-integers and Infinity', () => { + expect(() => parsePositiveIntArg('--max-turns', '2.5')).toThrow( + InvalidArgumentError, + ) + expect(() => parsePositiveIntArg('--max-turns', '1e999')).toThrow( + InvalidArgumentError, + ) +}) + +test('rejects integers beyond the safe range', () => { + // Number.isInteger accepts these, but they no longer round-trip: + // 9007199254740993 silently parses as 9007199254740992, and 1e308 is a cap + // the turn loop can never reach. + expect(() => parsePositiveIntArg('--max-turns', '9007199254740993')).toThrow( + InvalidArgumentError, + ) + expect(() => parsePositiveIntArg('--max-turns', '1e308')).toThrow( + InvalidArgumentError, + ) + // The boundary itself is still accepted. + expect(parsePositiveIntArg('--max-turns', '9007199254740991')).toBe( + Number.MAX_SAFE_INTEGER, + ) +}) + +test('names the option in the error message', () => { + expect(() => parsePositiveIntArg('--max-turns', 'abc')).toThrow( + '--max-turns must be a positive integer', + ) +}) + +test('parsePositiveAmountArg accepts positive amounts including decimals', () => { + expect(parsePositiveAmountArg('--max-budget-usd', '5')).toBe(5) + expect(parsePositiveAmountArg('--max-budget-usd', '0.25')).toBe(0.25) +}) + +test('parsePositiveAmountArg rejects non-numeric, zero and negative values', () => { + for (const bad of ['abc', '', ' ', '0', '-1']) { + expect(() => parsePositiveAmountArg('--max-budget-usd', bad)).toThrow( + InvalidArgumentError, + ) + } +}) + +test('parsePositiveAmountArg rejects an unbounded budget', () => { + // Number('Infinity') is not NaN and is greater than zero, so a bare isNaN + // check would accept it as a spending cap that can never be reached. + expect(() => parsePositiveAmountArg('--max-budget-usd', 'Infinity')).toThrow( + InvalidArgumentError, + ) + expect(() => parsePositiveAmountArg('--max-budget-usd', '1e999')).toThrow( + InvalidArgumentError, + ) +}) + +test('parsePositiveAmountArg rejects finite budgets the guard can never reach', () => { + // `getTotalCost() >= maxBudgetUsd` never fires for these, so they are the + // unbounded-budget hole again -- and past MAX_SAFE_INTEGER the enforced cap + // is not even the number the user supplied. + expect(() => parsePositiveAmountArg('--max-budget-usd', '1e308')).toThrow( + InvalidArgumentError, + ) + expect(() => + parsePositiveAmountArg('--max-budget-usd', '9007199254740993'), + ).toThrow(InvalidArgumentError) + // Realistic budgets, and the boundary itself, still parse. + expect(parsePositiveAmountArg('--max-budget-usd', '250.75')).toBe(250.75) + expect( + parsePositiveAmountArg('--max-budget-usd', '9007199254740991'), + ).toBe(Number.MAX_SAFE_INTEGER) +}) + +test('parsePositiveAmountArg rejects a decimal cap that loses precision', () => { + // Number('9007199254740990.5') rounds to 9007199254740990, which is under + // MAX_SAFE_INTEGER, so the earlier bound accepts it -- but the enforced cap + // is no longer the value the user typed. A double cannot hold this decimal. + expect(() => + parsePositiveAmountArg('--max-budget-usd', '9007199254740990.5'), + ).toThrow(InvalidArgumentError) + expect(() => + parsePositiveAmountArg('--max-budget-usd', '1234567890.1234567'), + ).toThrow(InvalidArgumentError) + // A decimal within a double's ~15 significant digits is fine, including one + // whose trailing/leading zeros are not significant. + expect(parsePositiveAmountArg('--max-budget-usd', '1000.50')).toBe(1000.5) + expect(parsePositiveAmountArg('--max-budget-usd', '0.000001')).toBe(0.000001) +}) + +test('parsePositiveAmountArg rejects precision-losing scientific notation', () => { + // The same value as 9007199254740990.5, spelled without a decimal point, must + // not slip past the guard just because the string carries no `.`. + expect(() => + parsePositiveAmountArg('--max-budget-usd', '90071992547409905e-1'), + ).toThrow(InvalidArgumentError) + expect(() => + parsePositiveAmountArg('--max-budget-usd', '12345678901234567e-7'), + ).toThrow(InvalidArgumentError) + // Scientific notation that denotes a representable value is still accepted. + expect(parsePositiveAmountArg('--max-budget-usd', '1.5e3')).toBe(1500) + expect(parsePositiveAmountArg('--max-budget-usd', '2.5e-1')).toBe(0.25) +}) + +test('parsePositiveAmountArg accepts the boundary integer spelled with a fraction', () => { + // 9007199254740991.0 denotes exactly MAX_SAFE_INTEGER, which round-trips; the + // precision guard must not reject an integer just because it carries a `.0`. + expect(parsePositiveAmountArg('--max-budget-usd', '9007199254740991.0')).toBe( + Number.MAX_SAFE_INTEGER, + ) + expect( + parsePositiveAmountArg('--max-budget-usd', '90071992547409910e-1'), + ).toBe(Number.MAX_SAFE_INTEGER) +}) + +test('parsePositiveAmountArg names the option in the error message', () => { + expect(() => parsePositiveAmountArg('--max-budget-usd', 'abc')).toThrow( + '--max-budget-usd must be a positive number greater than 0', + ) +}) diff --git a/src/utils/cli/numericArgs.ts b/src/utils/cli/numericArgs.ts new file mode 100644 index 0000000000..4604b71cda --- /dev/null +++ b/src/utils/cli/numericArgs.ts @@ -0,0 +1,111 @@ +import { InvalidArgumentError } from '@commander-js/extra-typings' + +/** + * Commander argParser for options that must be a positive integer. + * + * A bare `.argParser(Number)` accepts anything Number() coerces without + * complaint: "abc"/"10x" become NaN, "0" stays 0, "1e999" becomes Infinity, + * "2.5" stays a float, "-5" stays negative. For a cap like --max-turns those + * all defeat the limit — the enforcement check `maxTurns && turnCount > maxTurns` + * is falsy for NaN/0 (so the agent runs unbounded) and negatives exit + * immediately. + * + * The bound is Number.isSafeInteger, not Number.isInteger: values past 2^53-1 + * are integers but no longer round-trip, so `--max-turns 9007199254740993` + * would silently become 9007199254740992, and 1e308 would pass as a cap that + * can never be reached. Matches the maxSteps check in runAgent.ts. + */ +export function parsePositiveIntArg(name: string, value: string): number { + const parsed = Number(value) + if (!Number.isSafeInteger(parsed) || parsed <= 0) { + throw new InvalidArgumentError(`${name} must be a positive integer`) + } + return parsed +} + +/** + * Commander argParser for options that must be a positive amount, decimals + * allowed (e.g. a dollar budget). + * + * Number.isFinite rather than a bare !isNaN check: `Number('Infinity')` is not + * NaN and is greater than zero, so an unbounded budget would otherwise pass + * validation for a spending cap. + * + * Finite is not sufficient on its own though. The cap is enforced as + * `getTotalCost() >= maxBudgetUsd`, so it has to be a dollar figure real + * spending can actually reach: 1e308 is finite and positive but the guard can + * never fire, which is the unbounded-budget hole again with a different + * literal. Anything past Number.MAX_SAFE_INTEGER also stops round-tripping, + * so the enforced cap would differ from the one the user typed. Bound at the + * safe-integer range -- far above any real budget, below where either problem + * starts. + * + * The safe-integer bound alone misses one case: a fractional value just under + * it can still lose precision. `Number('9007199254740990.5')` rounds to + * 9007199254740990, which is <= MAX_SAFE_INTEGER yet no longer the value that + * was typed, so the enforced cap silently shifts. A double round-trips only + * ~15 significant decimal digits, so a fractional input carrying more than that + * is rejected. (Whole-number precision is already bounded by MAX_SAFE_INTEGER, + * which legitimately carries 16 digits, so the digit check is gated on the + * input denoting a fractional value to avoid rejecting the boundary integer + * itself.) Integrality is read from the string spelling, not from the coerced + * double: `90071992547409905e-1` and `9007199254740990.5` denote the same + * precision-losing fraction even though one carries no `.`, while + * `9007199254740991.0` denotes the boundary integer and must still pass. + */ +export function parsePositiveAmountArg(name: string, value: string): number { + const parsed = Number(value) + if ( + !Number.isFinite(parsed) || + parsed <= 0 || + parsed > Number.MAX_SAFE_INTEGER + ) { + throw new InvalidArgumentError( + `${name} must be a positive number greater than 0`, + ) + } + if (hasFractionalPart(value) && significantDigitCount(value) > 15) { + throw new InvalidArgumentError( + `${name} has more precision than can be represented exactly`, + ) + } + return parsed +} + +/** + * Whether a numeric string denotes a value with a nonzero fractional component, + * reading the digits and the decimal point's effective position (after any + * exponent) rather than trusting the coerced double, which may already have + * dropped the fraction. `9007199254740990.5` and `90071992547409905e-1` both + * report true; `9007199254740991.0`, `1.5e3`, and `90071992547409910e-1` report + * false. Used to decide whether the precision guard applies. + */ +function hasFractionalPart(value: string): boolean { + const trimmed = value.trim().replace(/^[+-]/, '') + const expMatch = trimmed.match(/[eE]([+-]?\d+)$/) + const exp = expMatch ? parseInt(expMatch[1], 10) : 0 + const mantissa = expMatch ? trimmed.slice(0, expMatch.index) : trimmed + const dot = mantissa.indexOf('.') + const intPart = dot >= 0 ? mantissa.slice(0, dot) : mantissa + const fracPart = dot >= 0 ? mantissa.slice(dot + 1) : '' + const digits = intPart + fracPart + // Index in `digits` where the decimal point lands after applying the exponent. + const pointPos = intPart.length + exp + const fractionDigits = pointPos <= 0 ? digits : digits.slice(pointPos) + return /[1-9]/.test(fractionDigits) +} + +/** + * Count the significant decimal digits in a numeric string: drop the sign, any + * exponent suffix, the decimal point, then leading and trailing zeros (neither + * of which carries precision). "1000.50" -> "10005" -> 5; "9007199254740990.5" + * -> 17. Used to reject caps a double cannot hold exactly. + */ +function significantDigitCount(value: string): number { + const mantissa = value + .trim() + .replace(/^[+-]/, '') + .replace(/[eE][+-]?\d+$/, '') + const digitsOnly = mantissa.replace('.', '') + return digitsOnly.replace(/^0+/, '').replace(/0+$/, '').length +}