fix: preserve Topstep fill metadata and persist Questrade token rotation

CHANGELOG.md

@@ -2,6 +2,18 @@
 
 All notable changes to `@luxalgo/broker-sdk` are documented here.
 
+## Unreleased
+
+These schema additions are intended for the next minor release; the package version is unchanged until release preparation.
+
+- Trade schema: optional native fill/instrument identity, a deterministic numeric tie-break key (`executionTieBreak`, not a chronology guarantee), and provider-reported realized gross P&L.
+- Topstep: preserve fill IDs, full contract IDs, and sub-millisecond UTC timestamps; include reported fees plus commissions and distinguish explicit zero from unknown costs; expose `profitAndLoss` as `reportedGrossPnl`; reject `success: false` JSON responses even with HTTP 200. Provider array order and automatic discovery of all returned accounts are preserved.
+- Topstep: malformed JSON bodies raise typed request errors. Failed searches abort the snapshot instead of appearing as empty positions/history, preventing false position-closure events. Both cost fields exist in the published API schema; their additive interpretation remains an accounting assumption.
+- Topstep compatibility: timestamps without `Z` or a numeric offset now leave `executedAt` absent instead of using the host's local timezone. A negative `fees` or `commissions` component omits the entire `fee`, even when the other component is positive, because negative component semantics are undocumented.
+- Sync: prefer native fill IDs within each account. Match pre-upgrade Topstep snapshots using millisecond timestamps and occurrence counts, reserving exact matches first. Old fills with identical legacy fields cannot be individually identified during migration.
+- Stats: compare mixed-length fractional timestamps chronologically without losing sub-millisecond digits. Topstep fraction parsing no longer relies on engines accepting more than three digits in `Date.parse`.
+- Questrade: update in-memory credentials and await persistence immediately after single-use token exchange, before account/history requests. Persistence failures propagate and stop further requests. Direct adapter consumers can use `FetchContext.onCredentialsRotated`; consumers without it retain `rotatedCredentials` on success.
+
 ## 0.5.1
 
 - IBKR Flex XML: optional `statementTimeZone` on `connect()` and `parseFlexStatement()` interprets timestamps without offsets in the selected IANA timezone, including daylight saving. Existing callers retain the UTC default. Explicit UTC/numeric offsets are honored; invalid, ambiguous, or nonexistent local timestamps leave `executedAt` absent.

README.md

@@ -56,10 +56,29 @@ type Account = {
   cash?: number;       // when the broker reports it separately
   environment?: "live" | "paper";
   positions: { symbol: string; quantity: number; marketValue?: number }[];
-  trades: { symbol: string; side: "buy" | "sell"; quantity: number; price: number; fee?: number; executedAt?: string }[];
+  trades: {
+    symbol: string;
+    side: "buy" | "sell";
+    quantity: number;
+    price: number;
+    fee?: number;
+    executedAt?: string;
+    executionId?: string;
+    instrumentId?: string;
+    executionTieBreak?: number;
+    reportedGrossPnl?: number;
+  }[];
 };
 ```
 
+Topstep fills retain native fill IDs, full futures contract IDs, and UTC
+timestamps with source fractional precision. `fee` includes reported fees
+plus commissions (including explicit zero); `reportedGrossPnl` carries the
+provider's realized gross P&L. Numeric fill IDs provide `executionTieBreak` as a
+deterministic tie-break for equal timestamps, without a guarantee of actual
+chronology. See [the schema](docs/SCHEMA.md#trade) for cost assumptions and consumer
+limitations. All returned provider accounts are imported automatically.
+
 ### IBKR Flex statement timezone
 
 For Flex XML timestamps without an offset, pass the timezone used by the report:
@@ -166,7 +185,7 @@ const positions = positionsFromTrades(trades);
 
 - **Your keys stay yours.** The SDK runs where your code runs. There is no LuxAlgo server in the path, no telemetry, no phoning home. We don't want your keys.
 - **Read-only by default.** The root export reads accounts, balances, positions, and history. Nothing in it can place an order.
-- **Credential rotation is first-class.** Brokers with single-use tokens (Questrade) hand the rotated credentials back through `onCredentialsRotated` so you can persist them before the old ones die.
+- **Credential rotation is first-class.** Questrade updates in-memory credentials and awaits `onCredentialsRotated` before account/history requests. Persist the replacement there; callback errors abort further requests. See [the rotation recipe](docs/recipes.md#persist-snapshots-and-survive-token-rotation).
 - **Fail-soft, never fabricate.** A position the broker can't price has no `marketValue` rather than a made-up one. A history row that can't be read is skipped and counted, not guessed.
 
 ## Give your AI agent portfolio access
@@ -197,6 +216,11 @@ npx broker-sync --interval 300 --webhook-url https://example.com/hooks/portfolio
 
 Events are `trade_executed`, `balance_changed`, `position_opened`, `position_closed`, `position_changed`, `broker_error`, and `sync_completed`, delivered per sweep as one JSON batch. When a secret is configured, every webhook request carries `X-BrokerSync-Signature`, the lowercase hex HMAC-SHA256 of the raw body. State persists to a local JSON file before delivery, so a crash can drop a batch but never replay one. Programmatic use lives at `@luxalgo/broker-sdk/sync`.
 
+The CLI keeps rotated credentials in memory while running, but never stores them
+in snapshot state or updates its environment/config file. For Questrade tokens
+that must survive restarts, supply SDK connections with a persistence callback
+to the programmatic daemon; see [the rotation recipe](docs/recipes.md#persist-snapshots-and-survive-token-rotation).
+
 ## Place orders (experimental)
 
 The write layer lives in a deliberately separate module. Importing the SDK never gives code trading capability by accident:

conformance/vectors/topstep.json

@@ -1,6 +1,6 @@
 {
   "broker": "topstep",
-  "description": "Futures prop account: contract ids like CON.F.US.EP.U26 normalize to the symbol segment, position type 2 means short, voided executions drop out.",
+  "description": "Futures prop account: native fill/contract identity, numeric ID tie-break key, reported gross P&L, fees plus commissions, short positions, and voided execution filtering.",
   "raw": {
     "accounts": [
       {
@@ -17,13 +17,16 @@
         ],
         "trades": [
           {
+            "id": 101,
             "contractId": "CON.F.US.EP.U26",
             "side": 1,
             "size": 1,
             "price": 5600.25,
             "fees": 1.4,
             "creationTimestamp": "2026-08-10T15:00:00Z",
-            "voided": false
+            "voided": false,
+            "commissions": 0.5,
+            "profitAndLoss": 32.5
           },
           {
             "contractId": "CON.F.US.EP.U26",
@@ -52,12 +55,16 @@
       ],
       "trades": [
         {
+          "executionId": "101",
+          "instrumentId": "CON.F.US.EP.U26",
           "symbol": "EP",
           "side": "sell",
           "quantity": 1,
           "price": 5600.25,
-          "fee": 1.4,
-          "executedAt": "2026-08-10T15:00:00.000Z"
+          "fee": 1.9,
+          "executedAt": "2026-08-10T15:00:00.000Z",
+          "executionTieBreak": 101,
+          "reportedGrossPnl": 32.5
         }
       ]
     }

docs/SCHEMA.md

@@ -32,11 +32,35 @@ One shape for everything, regardless of broker. This document is the human-reada
 | Field | Type | Notes |
 | --- | --- | --- |
 | `symbol` | `string` | Same normalization as positions. |
+| `executionId?` | `string` | Native fill identifier, scoped to the broker account; never substituted with an order ID. |
+| `instrumentId?` | `string` | Native instrument identifier, such as Topstep's full futures contract ID, including expiry. |
+| `executionTieBreak?` | `number` | Deterministic numeric tie-break key for equal timestamps. Does not imply chronological sequence unless the provider documents that guarantee. |
+| `reportedGrossPnl?` | `number` | Provider-reported realized gross P&L on this fill, in account currency, before fees/commissions. Omitted when unknown; zero and losses are preserved. |
 | `side` | `"buy" \| "sell"` | Always from the account holder's side. |
 | `quantity` | `number` | Always positive — `side` carries direction. |
 | `price` | `number` | Per unit, in the account's `currency`. |
-| `fee?` | `number` | Commission/fee, absolute value, only when reported and non-zero. |
-| `executedAt?` | `string` | ISO 8601, only when the source carried a parseable timestamp. |
+| `fee?` | `number` | Non-negative reported costs. Topstep sums available non-negative `fees` and `commissions`, preserving explicit zero; omitted when neither is known or either is negative. Other adapters may omit zero costs. |
+| `executedAt?` | `string` | ISO 8601, only when the source carried a parseable timestamp. Topstep normalizes offsets to UTC while retaining fractional precision beyond milliseconds. |
+
+Topstep's `executionTieBreak` uses non-negative safe-integer numeric fill IDs;
+the [API reference](https://gateway.docs.projectx.com/docs/api-reference/trade/trade-search/)
+does not guarantee their chronology. String IDs have no inferred ordering key;
+unsafe numeric IDs are omitted. Provider array order is preserved. Stats compares
+precise instants, preserving input order for ties; it does not use this key,
+contract IDs, or reported P&L to reconstruct futures results.
+
+Keep precise timestamps as strings: `Date` discards sub-millisecond digits.
+Sync uses native fill IDs and occurrence counts within each account. On upgrade,
+old Topstep fills without native metadata can match by the original trade fields
+and millisecond timestamp. Exact matches take precedence. Fills indistinguishable
+in the old data remain ambiguous during that transition. Other timestamp-keyed
+consumers must handle the precision change themselves.
+
+The [published Topstep API schema](https://api.topstepx.com/swagger/v1/swagger.json)
+declares both `fees` and optional `commissions`, but does not describe whether
+`fees` is inclusive. The adapter adds these fields as separate components;
+confirm totals against broker statements when accounting accuracy is required.
+Negative component semantics are also unspecified, so their total is omitted.
 
 ### Bar
 

docs/recipes.md

@@ -67,13 +67,26 @@ import { connect } from "@luxalgo/broker-sdk";
 const connection = connect({
   broker: "questrade",
   credentials: { refreshToken: loadToken() },
-  // Questrade tokens are single-use: persist the rotation or the next fetch dies.
+  // Awaited immediately after exchange, before any account/history requests.
   onCredentialsRotated: (next) => saveToken(next.refreshToken),
 });
 const snapshot = await connection.fetchSnapshot();
 await db.upsert("accounts", snapshot.accounts); // Account.id is a stable upsert key
 ```
 
+`saveToken` must return its persistence promise so the SDK can await it.
+Questrade's old refresh token is invalid after exchange. The replacement is
+already in `connection.credentials` even if persistence or a later request
+fails; a persistence error stops further provider requests. Avoid concurrent
+fetches sharing a single-use token.
+
+Direct `questrade.fetchRaw(credentials, { fetch, onCredentialsRotated })`
+consumers can supply the same awaited callback, which receives the rotated
+credential fields. Without it, successful calls retain the legacy
+`rotatedCredentials` result, but a later request failure prevents that result
+from being returned. With a callback, rotation is delivered only through the
+callback. Other adapters retain their existing rotation timing.
+
 ## Give your agent read access (MCP)
 
 No code — run [`@luxalgo/mcp`](https://github.com/LuxAlgo/luxalgo-mcp-server) locally with your read-only keys as `BROKERS_*` env vars in your MCP client config, then ask Claude "how's my portfolio doing?".

src/adapters/questrade.ts

@@ -7,9 +7,10 @@ import type { BrokerAdapter, Credentials, FetchContext } from "./types.js";
   Questrade (Canada), read-only via the user's own API refresh token
   (generated self-serve in Questrade's API centre). The token is single-use:
   every exchange returns a fresh access token AND a fresh refresh token, so
-  the rotated pair is handed back through rotatedCredentials — persist it
-  before the old one dies. Accounts, balances, positions, and the last month
-  of trade activities.
+  the replacement refresh token must be persisted immediately, before any
+  later request can fail. Direct callers without a rotation callback receive
+  rotatedCredentials on success. Accounts, balances, positions, and the last
+  month of trade activities.
 */
 
 const QUESTRADE_LOGIN = "https://login.questrade.com";
@@ -123,6 +124,10 @@ const fetchRaw = async (credentials: Credentials, ctx: FetchContext) => {
     throw new BrokerRequestError("questrade", "Questrade returned an incomplete token exchange");
   }
 
+  // History requests can fail after this single-use token has been consumed.
+  // Persist the replacement before making any of those requests.
+  await ctx.onCredentialsRotated?.({ refreshToken: tokens.refresh_token });
+
   const get = async <T>(path: string): Promise<T> => {
     const response = await ctx.fetch(`${apiServer}v1${path}`, {
       headers: { Authorization: `Bearer ${accessToken}` },
@@ -156,8 +161,8 @@ const fetchRaw = async (credentials: Credentials, ctx: FetchContext) => {
 
   return {
     raw: { accounts },
-    // The old refresh token is now dead — persist the rotated one.
-    rotatedCredentials: { refreshToken: tokens.refresh_token },
+    // Compatibility for direct adapter callers without the immediate callback.
+    ...(ctx.onCredentialsRotated ? {} : { rotatedCredentials: { refreshToken: tokens.refresh_token } }),
   };
 };
 
@@ -166,7 +171,7 @@ export const questrade: BrokerAdapter<QuestradeRaw> = {
   displayName: "Questrade",
   credentials: [{ key: "refreshToken", label: "API refresh token", secret: true }],
   readOnlySetup:
-    "Generate a refresh token in Questrade's API centre. Tokens are single-use and rotate on every fetch — persist rotatedCredentials (see onCredentialsRotated) or the connection dies.",
+    "Generate a refresh token in Questrade's API centre. Tokens are single-use and rotate on every fetch — persist them with onCredentialsRotated or the connection dies.",
   fetchRaw,
   normalize,
 };

src/adapters/topstep.ts

@@ -1,6 +1,7 @@
 import { BrokerAuthError, BrokerRequestError, MissingCredentialsError } from "../errors.js";
 import type { Account, Position, Trade } from "../schema.js";
-import { asFiniteNumber, asIsoTimestamp, rejectResponse } from "./http.js";
+import { asPreciseIsoTimestamp } from "../timestamps.js";
+import { asFiniteNumber, rejectResponse } from "./http.js";
 import type { BrokerAdapter, Credentials, FetchContext } from "./types.js";
 
 /*
@@ -15,10 +16,13 @@ const TOPSTEP_API = "https://api.topstepx.com/api";
 const TRADE_WINDOW_DAYS = 30;
 
 type TopstepTradeRow = {
+  id?: number | string;
   contractId?: string;
   creationTimestamp?: string;
   price?: number;
   fees?: number;
+  commissions?: number;
+  profitAndLoss?: number | null;
   side?: number;
   size?: number;
   voided?: boolean;
@@ -50,14 +54,32 @@ export const parseTopstepTrades = (trades: TopstepTradeRow[] | undefined): Trade
     const quantity = Math.abs(trade.size ?? 0);
     const price = trade.price ?? 0;
     if (!trade.contractId || !side || quantity <= 0 || price <= 0) continue;
-    const fee = Math.abs(trade.fees ?? 0);
-    const executedAt = asIsoTimestamp(trade.creationTimestamp);
+    const fees = asFiniteNumber(trade.fees);
+    const commissions = asFiniteNumber(trade.commissions);
+    // The API does not specify negative component semantics; don't invent a charge.
+    const fee = (fees !== undefined || commissions !== undefined)
+      && (fees ?? 0) >= 0 && (commissions ?? 0) >= 0
+      ? (fees ?? 0) + (commissions ?? 0)
+      : undefined;
+    const reportedGrossPnl = asFiniteNumber(trade.profitAndLoss);
+    const executedAt = asPreciseIsoTimestamp(trade.creationTimestamp);
     mapped.push({
+      ...(typeof trade.id === "string" && trade.id.length > 0
+        ? { executionId: trade.id }
+        : typeof trade.id === "number" && Number.isSafeInteger(trade.id)
+          ? { executionId: String(trade.id) }
+          : {}),
+      // Numeric IDs give a repeatable tie-break, not a documented execution sequence.
+      ...(typeof trade.id === "number" && Number.isSafeInteger(trade.id) && trade.id >= 0
+        ? { executionTieBreak: trade.id }
+        : {}),
+      ...(reportedGrossPnl !== undefined ? { reportedGrossPnl } : {}),
+      instrumentId: trade.contractId,
       symbol: topstepSymbol(trade.contractId),
       side,
       quantity,
       price,
-      ...(fee > 0 ? { fee } : {}),
+      ...(fee !== undefined ? { fee } : {}),
       ...(executedAt ? { executedAt } : {}),
     });
   }
@@ -110,7 +132,19 @@ const fetchRaw = async (credentials: Credentials, ctx: FetchContext) => {
       body: JSON.stringify(body),
     });
     if (!response.ok) rejectResponse("topstep", "Topstep", response);
-    return (await response.json()) as T;
+    const result: unknown = await response.json();
+    if (typeof result !== "object" || result === null || Array.isArray(result)) {
+      throw new BrokerRequestError("topstep", `Topstep returned an invalid response for ${path}`);
+    }
+    const envelope = result as { success?: boolean; errorMessage?: string | null };
+    if (envelope.success === false) {
+      const message = envelope.errorMessage || `Topstep rejected ${path}`;
+      // Before authentication, rejection is an auth error; endpoint business
+      // error codes are endpoint-specific and must not be guessed from text.
+      if (!token) throw new BrokerAuthError("topstep", message);
+      throw new BrokerRequestError("topstep", message);
+    }
+    return result as T;
   };
 
   type TopstepLogin = { token?: string; success?: boolean; errorMessage?: string | null };

src/adapters/types.ts

@@ -10,13 +10,19 @@ export type NormalizationContext = {
 /** Injected IO — lets callers supply a custom fetch (proxies, tests). */
 export type FetchContext = {
   fetch: typeof globalThis.fetch;
+  /**
+   * Awaited by supporting adapters immediately after rotation, before further IO.
+   * Receives rotated fields to merge into existing credentials. Rejection aborts the fetch.
+   */
+  onCredentialsRotated?: (credentials: Credentials) => void | Promise<void>;
 };
 
 export type AdapterFetchResult<Raw> = {
   raw: Raw;
   /**
-   * Rotated credentials the caller must persist before the old ones die
-   * (e.g. Questrade refresh tokens are single-use).
+   * Rotated credentials the caller must persist. Questrade returns this
+   * only when no onCredentialsRotated callback is supplied; its old token
+   * is already invalid after exchange.
    */
   rotatedCredentials?: Credentials;
 };