diff --git a/src/trader/trader-commands.test.ts b/src/trader/trader-commands.test.ts index 3002dca..465b842 100644 --- a/src/trader/trader-commands.test.ts +++ b/src/trader/trader-commands.test.ts @@ -10,6 +10,7 @@ import { parseTimeout, resolveTenantAddress, createTraderCommand, + buildCreateIntentParams, } from './trader-commands.js'; describe('parseTimeout (trader)', () => { @@ -78,9 +79,12 @@ describe('resolveTenantAddress', () => { }); describe('createTraderCommand', () => { - it('exposes all 7 expected subcommands', () => { + it('exposes the 6 controller-scoped trader subcommands', () => { const trader = createTraderCommand(); const names = trader.commands.map((c) => c.name()).sort(); + // No `status` here: STATUS is system-scoped per Unicity + // architecture and routes through the host manager via HMCP. + // Use `sphere host inspect ` for trader liveness. expect(names).toEqual([ 'cancel-intent', 'create-intent', @@ -88,7 +92,6 @@ describe('createTraderCommand', () => { 'list-intents', 'portfolio', 'set-strategy', - 'status', ]); }); @@ -120,7 +123,7 @@ describe('createTraderCommand', () => { const optionFlags = cmd!.options.map((o) => o.long); expect(optionFlags).toEqual(expect.arrayContaining([ '--direction', '--base', '--quote', '--rate-min', '--rate-max', - '--volume-min', '--volume-total', '--expiry-ms', + '--volume-min', '--volume-max', '--expiry-ms', ])); }); @@ -171,3 +174,116 @@ describe('createTraderCommand', () => { } }); }); + +describe('buildCreateIntentParams (wire shape)', () => { + // Minimal valid opts; individual tests override the field they are exercising. + const baseOpts = { + direction: 'buy', + base: 'UCT', + quote: 'USDU', + rateMin: '100', + rateMax: '200', + volumeMin: '10', + volumeMax: '100', + }; + + it('emits volume_max (not volume_total) on the wire', () => { + const result = buildCreateIntentParams(baseOpts); + expect('error' in result).toBe(false); + if ('error' in result) return; + expect(result.params['volume_max']).toBe('100'); + expect(result.params).not.toHaveProperty('volume_total'); + }); + + it('converts --expiry-ms to expiry_sec at the wire boundary (5000ms → 5s)', () => { + const result = buildCreateIntentParams({ ...baseOpts, expiryMs: '5000' }); + expect('error' in result).toBe(false); + if ('error' in result) return; + expect(result.params['expiry_sec']).toBe(5); + expect(result.params).not.toHaveProperty('expiry_ms'); + }); + + it('floors fractional seconds (90500ms → 90s)', () => { + const result = buildCreateIntentParams({ ...baseOpts, expiryMs: '90500' }); + if ('error' in result) throw new Error(result.error); + expect(result.params['expiry_sec']).toBe(90); + }); + + it('omits expiry_sec when --expiry-ms not provided', () => { + const result = buildCreateIntentParams(baseOpts); + if ('error' in result) throw new Error(result.error); + expect(result.params).not.toHaveProperty('expiry_sec'); + expect(result.params).not.toHaveProperty('expiry_ms'); + }); + + it('rejects sub-1000ms expiries with a clear CLI-layer message', () => { + const result = buildCreateIntentParams({ ...baseOpts, expiryMs: '999' }); + expect('error' in result).toBe(true); + if (!('error' in result)) return; + expect(result.error).toMatch(/at least 1000.*1 second/); + expect(result.error).toContain('999'); + }); + + it('rejects expiries greater than 7 days', () => { + const sevenDaysMs = 7 * 24 * 60 * 60 * 1000; + const result = buildCreateIntentParams({ ...baseOpts, expiryMs: String(sevenDaysMs + 1) }); + expect('error' in result).toBe(true); + if (!('error' in result)) return; + expect(result.error).toMatch(/7 days/); + }); + + it('accepts the boundary cases — exactly 1000ms and exactly 7 days', () => { + const lower = buildCreateIntentParams({ ...baseOpts, expiryMs: '1000' }); + if ('error' in lower) throw new Error(lower.error); + expect(lower.params['expiry_sec']).toBe(1); + + const sevenDaysMs = 7 * 24 * 60 * 60 * 1000; + const upper = buildCreateIntentParams({ ...baseOpts, expiryMs: String(sevenDaysMs) }); + if ('error' in upper) throw new Error(upper.error); + expect(upper.params['expiry_sec']).toBe(sevenDaysMs / 1000); + }); + + it('rejects non-numeric, zero, and negative expiry-ms', () => { + for (const bad of ['abc', '0', '-100', 'NaN']) { + const r = buildCreateIntentParams({ ...baseOpts, expiryMs: bad }); + expect('error' in r, `expected error for expiry-ms="${bad}"`).toBe(true); + } + }); + + 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. + 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/); + }); + + it('rejects invalid direction', () => { + const result = buildCreateIntentParams({ ...baseOpts, direction: 'sideways' }); + expect('error' in result).toBe(true); + if (!('error' in result)) return; + expect(result.error).toMatch(/buy.*sell/); + }); + + it('passes through bigint-string fields verbatim (no coercion)', () => { + const huge = '99999999999999999999999999'; + const result = buildCreateIntentParams({ ...baseOpts, volumeMax: huge }); + if ('error' in result) throw new Error(result.error); + // Must preserve the exact string — coercion to Number would lose + // precision and bigint serialization differs across SDKs. + expect(result.params['volume_max']).toBe(huge); + }); +}); diff --git a/src/trader/trader-commands.ts b/src/trader/trader-commands.ts index 25edafb..237c917 100644 --- a/src/trader/trader-commands.ts +++ b/src/trader/trader-commands.ts @@ -41,7 +41,7 @@ interface CreateIntentOpts { rateMin: string; rateMax: string; volumeMin: string; - volumeTotal: string; + volumeMax: string; expiryMs?: string; } @@ -204,32 +204,68 @@ function emitResult(json: boolean, response: AcpResultPayload | AcpErrorPayload) // Subcommand handlers // ============================================================================= +/** + * Build the wire payload for CREATE_INTENT from CLI options. Pure + * function so it's unit-testable without a Sphere/DM stack. + * + * Returns `{ params }` on success or `{ error }` with a human- + * readable message the caller can write to stderr. Validation here + * mirrors the trader-side ACP validation + * (trader-service/src/trader/trader-command-handler.ts:331-342) so + * the operator gets a clear diagnostic at the CLI layer instead of + * an opaque INVALID_PARAM from a remote service. + */ +export function buildCreateIntentParams( + opts: CreateIntentOpts, +): { readonly params: Record } | { readonly error: string } { + if (opts.direction !== 'buy' && opts.direction !== 'sell') { + return { error: '--direction must be "buy" or "sell"' }; + } + const params: Record = { + direction: opts.direction, + base_asset: opts.base, + quote_asset: opts.quote, + rate_min: opts.rateMin, + rate_max: opts.rateMax, + volume_min: opts.volumeMin, + 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}")` }; + } + // 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 + // flags; we convert at the wire boundary via floor(ms/1000). + if (n < 1000) { + // Sub-second expiries make no sense for trade intents and + // floor(ms/1000) would map them to 0, which the trader + // rejects with an unhelpful INVALID_PARAM. Catch here. + return { error: `--expiry-ms must be at least 1000 (1 second); got ${n}` }; + } + const sevenDaysMs = 7 * 24 * 60 * 60 * 1000; + if (n > sevenDaysMs) { + // 7-day cap matches the trader's own validation. Catch it + // here too so the operator gets the right diagnostic without + // a network round-trip. + return { error: `--expiry-ms must not exceed 7 days (${sevenDaysMs}ms); got ${n}` }; + } + params['expiry_sec'] = Math.floor(n / 1000); + } + return { params }; +} + async function handleCreateIntent(cmd: Command, opts: CreateIntentOpts): Promise { await runWithTransport(cmd, async ({ transport, json }) => { - if (opts.direction !== 'buy' && opts.direction !== 'sell') { - writeStderr('--direction must be "buy" or "sell"'); + const built = buildCreateIntentParams(opts); + if ('error' in built) { + writeStderr(built.error); process.exitCode = 1; return; } - const params: Record = { - direction: opts.direction, - base_asset: opts.base, - quote_asset: opts.quote, - rate_min: opts.rateMin, - rate_max: opts.rateMax, - volume_min: opts.volumeMin, - volume_total: opts.volumeTotal, - }; - if (opts.expiryMs !== undefined) { - const n = Number.parseInt(opts.expiryMs, 10); - if (!Number.isFinite(n) || n <= 0) { - writeStderr(`--expiry-ms must be a positive integer (got "${opts.expiryMs}")`); - process.exitCode = 1; - return; - } - params['expiry_ms'] = n; - } - const response = await transport.sendCommand('CREATE_INTENT', params); + const response = await transport.sendCommand('CREATE_INTENT', built.params); emitResult(json, response); }); } @@ -287,12 +323,16 @@ async function handlePortfolio(cmd: Command): Promise { }); } -async function handleStatus(cmd: Command): Promise { - await runWithTransport(cmd, async ({ transport, json }) => { - const response = await transport.sendCommand('STATUS', {}); - emitResult(json, response); - }); -} +// `sphere trader status` was previously wired to send STATUS over ACP +// directly to the trader. STATUS is a SYSTEM-scoped command per the +// Unicity architecture (system commands like STATUS, SHUTDOWN_GRACEFUL, +// SET_LOG_LEVEL, EXEC route through the tenant's host manager via HMCP, +// not direct controller→tenant ACP). The trader correctly rejects +// direct STATUS calls with UNAUTHORIZED. The subcommand has been +// removed from the CLI tree below; controllers should use +// `sphere host inspect ` (HMCP) to probe trader liveness, or +// rely on `sphere trader portfolio`/`list-intents` succeeding as an +// implicit liveness signal. async function handleSetStrategy(cmd: Command, opts: SetStrategyOpts): Promise { await runWithTransport(cmd, async ({ transport, json }) => { @@ -346,7 +386,7 @@ export function createTraderCommand(): Command { .requiredOption('--rate-min ', 'Minimum acceptable rate (string-encoded bigint)') .requiredOption('--rate-max ', 'Maximum acceptable rate (string-encoded bigint)') .requiredOption('--volume-min ', 'Minimum volume per match') - .requiredOption('--volume-total ', 'Total intent volume') + .requiredOption('--volume-max ', 'Total intent volume') .option('--expiry-ms ', 'Expiry duration in milliseconds (default: 24h)') .action(async function (this: Command, opts: CreateIntentOpts) { await handleCreateIntent(this, opts); @@ -385,12 +425,9 @@ export function createTraderCommand(): Command { await handlePortfolio(this); }); - trader - .command('status') - .description('Show STATUS — uptime + adapter info') - .action(async function (this: Command) { - await handleStatus(this); - }); + // `status` removed — STATUS is system-scoped and routes through the + // host manager. See the comment on the deleted handleStatus above. + // Use `sphere host inspect ` for trader liveness probes. trader .command('set-strategy') @@ -412,3 +449,5 @@ export function createTraderCommand(): Command { // Exported for unit tests. export { parseTimeout }; +// `buildCreateIntentParams` is also exported via its `export function` +// declaration above; named here for discoverability alongside parseTimeout.