fix(trader): align ACP wire shape with trader-service (expiry, volume, drop status)

src/trader/trader-commands.test.ts

@@ -10,6 +10,7 @@ import {
   parseTimeout,
   resolveTenantAddress,
   createTraderCommand,
+  buildCreateIntentParams,
 } from './trader-commands.js';
 
 describe('parseTimeout (trader)', () => {
@@ -78,17 +79,19 @@ 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 <instance>` for trader liveness.
     expect(names).toEqual([
       'cancel-intent',
       'create-intent',
       'list-deals',
       '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);
+  });
+});

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<string, unknown> } | { readonly error: string } {
+  if (opts.direction !== 'buy' && opts.direction !== 'sell') {
+    return { error: '--direction must be "buy" or "sell"' };
+  }
+  const params: Record<string, unknown> = {
+    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<void> {
   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<string, unknown> = {
-      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<void> {
   });
 }
 
-async function handleStatus(cmd: Command): Promise<void> {
-  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 <instance>` (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<void> {
   await runWithTransport(cmd, async ({ transport, json }) => {
@@ -346,7 +386,7 @@ export function createTraderCommand(): Command {
     .requiredOption('--rate-min <bigint>', 'Minimum acceptable rate (string-encoded bigint)')
     .requiredOption('--rate-max <bigint>', 'Maximum acceptable rate (string-encoded bigint)')
     .requiredOption('--volume-min <bigint>', 'Minimum volume per match')
-    .requiredOption('--volume-total <bigint>', 'Total intent volume')
+    .requiredOption('--volume-max <bigint>', 'Total intent volume')
     .option('--expiry-ms <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 <instance>` 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.