diff --git a/src/trader/trader-commands.test.ts b/src/trader/trader-commands.test.ts index 465b842..12c74bb 100644 --- a/src/trader/trader-commands.test.ts +++ b/src/trader/trader-commands.test.ts @@ -250,25 +250,11 @@ describe('buildCreateIntentParams (wire shape)', () => { } }); - it('documents parseInt prefix-truncation behavior on scientific notation', () => { - // parseInt('1e6', 10) returns 1 (parses '1', stops at 'e'). - // The user wanted 1_000_000 ms (≈1000s ≈ 16.7m) but got the - // truncated value 1, then our < 1000ms guard fires and rejects. - // Counterintuitive but at least the value is REJECTED rather - // than silently misused. This test pins the surprising - // behavior so a future strict-parse refactor (e.g., using - // `Number(s)` which rejects 'e' in this context) is tracked - // as a deliberate change rather than an accidental drift. - // - // FIXME: harden numeric parsing across this file with a - // strict-positive-int helper. Same pattern affects --limit, - // --max-concurrent, --timeout. Out of scope for this PR. + it('rejects scientific notation for expiry-ms without prefix truncation', () => { const result = buildCreateIntentParams({ ...baseOpts, expiryMs: '1e6' }); expect('error' in result).toBe(true); if (!('error' in result)) return; - // The error message reports the parsed value (1), not the - // user's original input (1e6) — that's the surprising part. - expect(result.error).toMatch(/got 1\b/); + expect(result.error).toContain('1e6'); }); it('rejects invalid direction', () => { diff --git a/src/trader/trader-commands.ts b/src/trader/trader-commands.ts index 237c917..6c9b9a7 100644 --- a/src/trader/trader-commands.ts +++ b/src/trader/trader-commands.ts @@ -116,6 +116,17 @@ function printJson(value: unknown): void { process.stdout.write(`${JSON.stringify(value, null, 2)}\n`); } +function parsePositiveInteger(raw: string, flag: string): number | string { + if (!/^\d+$/.test(raw)) { + return `${flag} must be a positive integer (got "${raw}")`; + } + const n = Number(raw); + if (!Number.isSafeInteger(n) || n <= 0) { + return `${flag} must be a positive integer (got "${raw}")`; + } + return n; +} + // ============================================================================= // Core runner // ============================================================================= @@ -231,10 +242,11 @@ export function buildCreateIntentParams( volume_max: opts.volumeMax, }; if (opts.expiryMs !== undefined) { - const n = Number.parseInt(opts.expiryMs, 10); - if (!Number.isFinite(n) || n <= 0) { - return { error: `--expiry-ms must be a positive integer (got "${opts.expiryMs}")` }; + const parsed = parsePositiveInteger(opts.expiryMs, '--expiry-ms'); + if (typeof parsed === 'string') { + return { error: parsed }; } + const n = parsed; // Trader's ACP CREATE_INTENT param is `expiry_sec` (validated // as a finite positive integer ≤ 7 days). The CLI flag stays // in milliseconds for ergonomic consistency with other timeout @@ -282,13 +294,13 @@ async function handleListIntents(cmd: Command, opts: ListIntentsOpts): Promise = {}; if (opts.state !== undefined) params['state'] = opts.state; if (opts.limit !== undefined) { - const n = Number.parseInt(opts.limit, 10); - if (!Number.isFinite(n) || n <= 0) { - writeStderr(`--limit must be a positive integer (got "${opts.limit}")`); + const parsed = parsePositiveInteger(opts.limit, '--limit'); + if (typeof parsed === 'string') { + writeStderr(parsed); process.exitCode = 1; return; } - params['limit'] = n; + params['limit'] = parsed; } const response = await transport.sendCommand('LIST_INTENTS', params); emitResult(json, response); @@ -300,13 +312,13 @@ async function handleListDeals(cmd: Command, opts: ListDealsOpts): Promise const params: Record = {}; if (opts.state !== undefined) params['state'] = opts.state; if (opts.limit !== undefined) { - const n = Number.parseInt(opts.limit, 10); - if (!Number.isFinite(n) || n <= 0) { - writeStderr(`--limit must be a positive integer (got "${opts.limit}")`); + const parsed = parsePositiveInteger(opts.limit, '--limit'); + if (typeof parsed === 'string') { + writeStderr(parsed); process.exitCode = 1; return; } - params['limit'] = n; + params['limit'] = parsed; } // Trader exposes the swap-set via LIST_SWAPS; alias it as `list-deals` // because operators think in deal language. Spec also accepts LIST_SWAPS