Fix DATA-RECOVERY-001 review findings: risk-gate test coverage + honest source labeling

docs/architecture/market-data.md

@@ -130,7 +130,12 @@ any client UI -- that's `CHART-001`.
   `allMids` and merges live mid-price updates into a broadcast loop that publishes
   to connected clients every second, per tracked symbol. REST polling (10s prices,
   60s candles) remains as the source of `change24h`/`volume`/candle history, which
-  the WS channel doesn't carry -- see `getLastKnownMarketMeta` below.
+  the WS channel doesn't carry -- see `getLastKnownMarketMeta` below. The published
+  `source` field on this fast path reflects the *last REST ingestion cycle's* actual
+  source (`meta.source`), not simply "hyperliquid" -- `price` here is always a live
+  Hyperliquid WS mid, but change24h/volume are only as trustworthy as that last REST
+  cycle, which could itself have fallen back to CoinGecko even while the WS
+  connection stays healthy (independent review of PR #59, finding LA-QG-002).
 - **Reconnect/backoff/stale-execution-gating (implemented in `DATA-RECOVERY-001`).**
   The WS client reconnects with exponential backoff plus jitter on close/error, and
   detects a silent stall (connection open but no message for 30s) via a reset-able

server/src/execution/paperEngine.test.ts

@@ -4,17 +4,44 @@ import { NotFoundError, ForbiddenError } from './errors';
 
 const selectMock = vi.fn();
 const updateMock = vi.fn();
+const insertMock = vi.fn();
 
 vi.mock('../db/index', () => ({
   db: {
     select: (...args: unknown[]) => selectMock(...args),
     update: (...args: unknown[]) => updateMock(...args),
+    insert: (...args: unknown[]) => insertMock(...args),
   },
 }));
 
 // vitest hoists `vi.mock` above imports, so `./paperEngine` picks up the
 // mocked `../db/index`.
-import { cancelOrder, closePosition } from './paperEngine';
+import { cancelOrder, closePosition, submitOrder, sweepLimitOrders } from './paperEngine';
+
+const BASE_ORDER_ROW = {
+  id: 'order-1',
+  userId: 'user-a',
+  asset: 'BTC',
+  side: 'LONG' as const,
+  orderType: 'MARKET' as const,
+  quantity: '1',
+  limitPrice: null,
+  leverage: '1',
+  status: 'PENDING',
+};
+
+const RISK_LIMITS_ROW = {
+  userId: 'user-a',
+  maxPositionSize: '1000',
+  maxLeverage: '10',
+  maxOpenPositions: 5,
+  maxDailyLossPercent: '5',
+  killSwitchEnabled: false,
+};
+
+function marketRow(source: 'hyperliquid' | 'coingecko') {
+  return { symbol: 'BTC', price: '100', source, updatedAt: new Date() };
+}
 
 /**
  * Regression coverage for SEC-017: user A must never be able to cancel user
@@ -31,6 +58,7 @@ describe('cancelOrder ownership', () => {
   beforeEach(() => {
     selectMock.mockReset();
     updateMock.mockReset();
+    insertMock.mockReset();
   });
 
   it('throws NotFoundError when the order does not exist', async () => {
@@ -58,6 +86,7 @@ describe('closePosition ownership', () => {
   beforeEach(() => {
     selectMock.mockReset();
     updateMock.mockReset();
+    insertMock.mockReset();
   });
 
   it('throws NotFoundError when the position does not exist', async () => {
@@ -89,3 +118,116 @@ describe('closePosition ownership', () => {
     expect(updateMock).toHaveBeenCalledTimes(1);
   });
 });
+
+/**
+ * Regression coverage for the trustworthy-source gate (DATA-RECOVERY-001)
+ * at its actual call sites in processNewOrder/sweepLimitOrders -- flagged
+ * by independent review of PR #59 (LA-QG-001) as untested at this level:
+ * checkTrustworthySource itself and evaluateTrade's dispatch of it were
+ * unit-tested, but nothing exercised the paperEngine.ts integration where
+ * market.source is actually read from a DB row and threaded through. Each
+ * block includes a positive control (a genuinely hyperliquid-sourced order
+ * still fills) alongside the negative case, for the same reason the
+ * ownership tests above do.
+ */
+describe('processNewOrder trustworthy-source gating', () => {
+  beforeEach(() => {
+    selectMock.mockReset();
+    updateMock.mockReset();
+    insertMock.mockReset();
+  });
+
+  it('rejects a new MARKET order priced off a CoinGecko-fallback row', async () => {
+    insertMock.mockReturnValueOnce(dbChain([BASE_ORDER_ROW])); // insert(orders)
+    selectMock
+      .mockReturnValueOnce(dbChain([])) // isUserHalted -- no risk_limits row, defaults to false
+      .mockReturnValueOnce(dbChain([marketRow('coingecko')])) // getMarketSnapshot
+      .mockReturnValueOnce(dbChain([RISK_LIMITS_ROW])) // getOrCreateRiskLimits
+      .mockReturnValueOnce(dbChain([{ value: 0 }])); // countOpenPositions
+    updateMock.mockReturnValueOnce(dbChain([{ ...BASE_ORDER_ROW, status: 'REJECTED' }])); // rejectOrder -> setOrderStatus
+
+    const result = await submitOrder('user-a', {
+      asset: 'BTC',
+      side: 'LONG',
+      orderType: 'MARKET',
+      quantity: 1,
+      leverage: 1,
+      idempotencyKey: 'key-reject',
+    });
+
+    expect(result.order.status).toBe('REJECTED');
+    expect(result.fills).toEqual([]);
+    expect(insertMock).toHaveBeenCalledTimes(1); // only the initial order insert -- never reached fillOrder's insert(fills)
+  });
+
+  it('positive control: a genuinely Hyperliquid-sourced MARKET order still fills', async () => {
+    insertMock
+      .mockReturnValueOnce(dbChain([BASE_ORDER_ROW])) // insert(orders)
+      .mockReturnValueOnce(dbChain([{ id: 'fill-1', orderId: 'order-1', price: '100.05', quantity: '1' }])) // insert(fills)
+      .mockReturnValueOnce(dbChain([{ id: 'position-1', userId: 'user-a', asset: 'BTC' }])); // insert(positions)
+    selectMock
+      .mockReturnValueOnce(dbChain([])) // isUserHalted
+      .mockReturnValueOnce(dbChain([marketRow('hyperliquid')])) // getMarketSnapshot
+      .mockReturnValueOnce(dbChain([RISK_LIMITS_ROW])) // getOrCreateRiskLimits
+      .mockReturnValueOnce(dbChain([{ value: 0 }])) // countOpenPositions
+      .mockReturnValueOnce(dbChain([])) // getOpenPosition (direction-conflict check)
+      .mockReturnValueOnce(dbChain([])) // getOpenPosition again, inside fillOrder
+      .mockReturnValueOnce(dbChain([{ id: 'fill-1', orderId: 'order-1', price: '100.05', quantity: '1' }])); // select(fills) at the end of fillOrder
+    updateMock.mockReturnValueOnce(dbChain([{ ...BASE_ORDER_ROW, status: 'FILLED' }])); // setOrderStatus -> FILLED
+
+    const result = await submitOrder('user-a', {
+      asset: 'BTC',
+      side: 'LONG',
+      orderType: 'MARKET',
+      quantity: 1,
+      leverage: 1,
+      idempotencyKey: 'key-fill',
+    });
+
+    expect(result.order.status).toBe('FILLED');
+    expect(result.fills).toHaveLength(1);
+  });
+});
+
+describe('sweepLimitOrders trustworthy-source gating', () => {
+  beforeEach(() => {
+    selectMock.mockReset();
+    updateMock.mockReset();
+    insertMock.mockReset();
+  });
+
+  it('leaves a marketable resting limit order ACKNOWLEDGED (not filled) when the market is CoinGecko-fallback-sourced', async () => {
+    const restingOrder = { id: 'order-2', userId: 'user-a', asset: 'BTC', side: 'LONG', orderType: 'LIMIT', limitPrice: '100', status: 'ACKNOWLEDGED' };
+    selectMock
+      .mockReturnValueOnce(dbChain([restingOrder])) // resting LIMIT/ACKNOWLEDGED orders
+      .mockReturnValueOnce(dbChain([marketRow('coingecko')])) // getMarketSnapshot -- price 100, limitPrice 100 -> marketable
+      .mockReturnValueOnce(dbChain([])); // isUserHalted
+
+    await sweepLimitOrders();
+
+    // Blocked by the trustworthy-source gate before ever reaching fillOrder --
+    // no fill inserted, no order-status update issued.
+    expect(insertMock).not.toHaveBeenCalled();
+    expect(updateMock).not.toHaveBeenCalled();
+  });
+
+  it('positive control: a genuinely Hyperliquid-sourced marketable resting limit order fills', async () => {
+    const restingOrder = { id: 'order-2', userId: 'user-a', asset: 'BTC', side: 'LONG', orderType: 'LIMIT', limitPrice: '100', status: 'ACKNOWLEDGED' };
+    selectMock
+      .mockReturnValueOnce(dbChain([restingOrder])) // resting LIMIT/ACKNOWLEDGED orders
+      .mockReturnValueOnce(dbChain([marketRow('hyperliquid')])) // getMarketSnapshot
+      .mockReturnValueOnce(dbChain([])) // isUserHalted
+      .mockReturnValueOnce(dbChain([])) // getOpenPosition (direction-conflict check)
+      .mockReturnValueOnce(dbChain([])) // getOpenPosition again, inside fillOrder
+      .mockReturnValueOnce(dbChain([{ id: 'fill-2', orderId: 'order-2', price: '100', quantity: '1' }])); // select(fills) at the end of fillOrder
+    insertMock
+      .mockReturnValueOnce(dbChain([{ id: 'fill-2', orderId: 'order-2', price: '100', quantity: '1' }])) // insert(fills)
+      .mockReturnValueOnce(dbChain([{ id: 'position-2', userId: 'user-a', asset: 'BTC' }])); // insert(positions)
+    updateMock.mockReturnValueOnce(dbChain([{ ...restingOrder, status: 'FILLED' }])); // setOrderStatus -> FILLED
+
+    await sweepLimitOrders();
+
+    expect(updateMock).toHaveBeenCalledTimes(1);
+    expect(insertMock).toHaveBeenCalledTimes(2);
+  });
+});

server/src/server.ts

@@ -176,11 +176,15 @@ setInterval(() => {
       price: Number(price),
       change24h: Number(meta.change24h),
       volume: Number(meta.volume),
-      // A live Hyperliquid WS mid is, by definition, Hyperliquid-sourced --
-      // independent of whatever the last REST cycle's source happened to
-      // be (e.g. if REST last fell back to CoinGecko but the WS has since
-      // recovered).
-      source: 'hyperliquid',
+      // `price` here is always genuinely Hyperliquid-sourced (a live WS
+      // mid), but change24h/volume are only as fresh as the *last REST
+      // ingestion cycle*, which could itself have fallen back to
+      // CoinGecko even while this WS connection is healthy. Labeling the
+      // whole row "hyperliquid" in that window would overstate the
+      // trustworthiness of the change24h/volume figures being merged in
+      // alongside it (flagged by independent review of PR #59, LA-QG-002)
+      // -- meta.source truthfully reflects what those fields actually are.
+      source: meta.source,
       timestamp: new Date(),
     });
   }