Conversation
|
😺 Michi — all 6 tasks done, CI green, marked ready for review.
CI: Note: |
…14) Decoder.timestamp(8) read the two big-endian 32-bit words backwards: per the MessagePack spec, data64 = (nanoseconds << 34) | seconds, so the FIRST word holds the top 30 bits of nanoseconds plus the top 2 bits of seconds, and the SECOND holds the low 32 bits of seconds - the code had nanos/secs computed from the wrong word. A sub-second trade_updates timestamp using this format decoded to a date roughly a decade off. The 4-byte (whole seconds) and 12-byte forms were unaffected and already covered by tests; this adds coverage for all three timestamp-extension encodings. Michi-Issue: 14
…; fix the trading-stream codec, state-machine races, and duplicate-listen bugs (#14) Verified against the real paper API: the trading stream never authenticated. Alpaca sends the auth ack (and presumably every frame) as JSON text inside a *binary-opcode* WebSocket frame by default - "binary frames" in the docs describes the WS opcode, not the codec. The msgpack codec is opt-in via a `Content-Type` request header the standard `WebSocket` API has no way to set, so it's unreachable through this client. `decodeFrame` ran the msgpack decoder on every binary frame regardless; the leading `{` byte happens to be a valid msgpack positive-fixint, so it silently decoded to the integer 123 instead of throwing, and the client hung forever waiting for an auth message it could never recognize. Trading-stream frames now decode as UTF-8 JSON by default; `TradingStreamOptions.decode` can still opt into msgpack (now exported as `decodeMsgPack`) for a transport that does set the header. Also, per request: replaced every streaming class's `EventEmitter` consumer API with `AsyncIterable` only. Node's `EventEmitter` throws (crashing the process) when `'error'` is emitted with no listener attached - exactly the "forgot to wire something up" mistake this package's own README usage example made. An event nobody reads from an async iterator is just inert. `StreamClient`, `TradingStreamClient`, and `MarketDataStreamClient` now yield a typed `{type: ...}` event union instead. That rewrite also straightened out the state machine, fixing bugs found by review and confirmed by reproduction: - `close()` called while a reconnect was already scheduled silently no-opped (the pending-reconnect state reused the same 'closed' label close() early-returns on) and never ended the async iterator - added a distinct 'reconnecting' state so the two are no longer conflated. - `connect()`'s re-entry guard didn't cover 'closing', so connect() right after close() (before the real, async close handshake finished) could create a second socket while the first was still closing. - the idle-timeout handler closed the socket without updating `state`, so it kept reporting 'open' during the close handshake. - `TradingStreamClient` sent `listen` twice on every reconnect (once via the base client's subscription replay, once by re-calling `subscribe()` from its own auth handler) - fixed by registering it once at construction and letting the base client's replay-on-auth handle every reconnect from then on. `MarketDataStreamClient` now uses the same per-key replay mechanism (new `track()`/`forget()` on `StreamClient`) instead of its own parallel "resend on auth" bookkeeping. - `AsyncQueue` (the shared push/pull primitive) now rejects a second concurrent `next()` instead of silently splitting messages round-robin between two readers. The shared `MockSocket` test double (extracted to `mock-socket.ts`, used by all three test files) now mirrors the real async close handshake - `close()` moves `readyState` to CLOSING synchronously but fires `'close'` on a later microtask - since the previous fully-synchronous mock couldn't reproduce any of the close()/connect() races above. `engines.node: >=22` and the native `WebSocket` decision are unchanged. The `@types/node` peerDependency and `tsconfig.dts-check.json`'s `types: ["node"]` override added in the prior commit are reverted - they existed only because the old `EventEmitter`-based API leaked a `node:events` reference into the public .d.ts; that's gone now. Michi-Issue: 14
bun.lock still listed packages/core and packages/mcp at 0.1.0, stale since the "bump to 1.0.1" version commit never re-ran bun install. Pre-existing, unrelated to this PR, but release.yml installs with --frozen-lockfile, so this would have broken the next publish. Michi-Issue: 14
|
😺 Michi — post-merge-ready adversarial review + live verification turned up real bugs; fixed and pushed. After marking this ready, I ran a 4-agent adversarial review of the streaming feature and separately tested it against the real paper trading environment using local Critical (live-verified): From the adversarial review (spot-verified, not taken on faith):
Per your direction, fixed all of the above and additionally removed Commits: CI: Still ready for review: #15 |
…tion stream needs real MessagePack both ways (#14) Checked every WebSocket stream live against the real paper API (trading + all four market-data feeds), not just the mocked unit tests: - Trading, stock, crypto, and news streams all authenticate and deliver messages correctly. - The **option** data stream (`v1beta1/indicative`/`opra`) did not: it sends every frame, including the auth ack, as MessagePack inside a binary-opcode WebSocket frame - and, unlike the trading stream (JSON-as-binary-frame), it also *rejects* a JSON-text auth message outright with `{T:"error",code:400,msg:"invalid syntax"}`. It needs real MessagePack in both directions. Two changes: - `StreamClient`'s default `decode` now treats every frame (text or binary opcode) as UTF-8 JSON, instead of throwing on binary with no `decode` configured - this was already proven necessary for the trading stream; the option stream's auth/"connected" handshake messages turn out to be small JSON-shaped MessagePack maps that happened to fail differently (`JSON Parse error: Unrecognized token`) once the prior fix made decode attempt them instead of refusing outright. `trading-client.ts`'s now-redundant local JSON-decode helper is removed in favor of this. - Added a MessagePack **encoder** to `./msgpack` (previously decode-only, per the original "Alpaca accepts JSON for auth/listen/subscribe" decision - also disproven for this one feed) and wired `optionDataStream()` to use it for both directions by default, overridable via the same `encode`/ `decode` options `TradingStreamOptions` already exposes. Verified live: all five streams now connect, authenticate, subscribe, and receive messages against the real paper API. Michi-Issue: 14
|
😺 Michi — "check all websocket methods" found and fixed one more critical bug: the option data stream was also broken. Live-tested all 5 streams (trading + stock/crypto/option/news) against the real paper API end-to-end (not just mocks):
The bug: the option stream ( Fixed: added a MessagePack encoder to Re-verified live after the fix: all five streams now connect, authenticate, subscribe, and receive real messages. Added regression tests (msgpack encoder roundtrips including the exact bug — an API secret's 44 chars overflow MessagePack's fixstr limit if you don't fall back to str8; option-stream auth over real msgpack frames). Commit: Still ready for review: #15 |
…ular-reference gaps it caught (#14) Cross-checked our msgpack codec's test coverage against https://github.com/msgpack/msgpack-node/blob/master/test/lib/msgpack.js and matched its cases where they apply to a decode-everything/encode-control- messages codec (numeric edge cases incl. the 2^31/2^40 boundaries, mixed-type and nested round-trips, empty string/array/object, toJSON support, circular- reference handling). Skipped what doesn't apply here (Buffer-specific API, multi-value/remainder unpacking - every frame we handle is exactly one top-level value). Two real gaps the comparison caught, both fixed: - `encode()` had no `toJSON()` support, so a `Date` (object with no own enumerable properties) silently encoded as an empty map `{}` instead of its ISO string - same failure mode `JSON.stringify` avoids by calling `toJSON()` when present. Now `encode()` does the same, own or inherited, recursively. - `encode()` had no circular-reference detection, so a self-referential array/object would recurse until a stack overflow instead of a clear error. Tracked via a `Set` of "currently being encoded" containers, scoped per-branch so the same object appearing twice in *unrelated* branches (not circular) still encodes fine. Michi-Issue: 14
|
😺 Michi — cross-checked our msgpack codec's tests against msgpack-node's reference suite and matched its applicable coverage (numeric edge cases incl. 2^31/2^40 boundaries, mixed/nested round-trips, empty values, toJSON, circular references). Skipped what doesn't apply to a decode-everything/encode-control-messages-only codec (Buffer-specific API, multi-value unpacking). Two real gaps the comparison surfaced, both fixed:
Commit: Still ready for review: #15 |
…, LULD, corrections, cancel/error, orderbooks) (#14) Compared our streaming message types against https://github.com/alpacahq/alpaca-ts-alpha/blob/main/src/streaming/types.ts. We already typed trade/quote/bar (stocks/crypto), trade/quote (options), and news via the matching generated REST model shapes. Missing: stock trading status (T:"s"), LULD (T:"l"), trade correction (T:"c"), and cancel/error (T:"x") events, plus crypto orderbook (T:"o") - all confirmed stock-only (crypto's own subscription ack never offers statuses/lulds) except orderbook, which is crypto-only. None have a REST historical endpoint, so (unlike trade/quote/bar) there's no generated model to extend - hand-written straight from https://docs.alpaca.markets/docs/real-time-stock-pricing-data and .../real-time-crypto-pricing-data's documented field tables, each with a golden-payload test. `CryptoOrderbookMessage` does reuse the existing generated `CryptoOrderbook` REST type (a/b/t), since that one does exist (snapshot endpoint). Also ported one good idea from the reference: `TradeUpdateEvent` now ends in `| (string & {})`, so a new order-event kind Alpaca adds later still type-checks (with autocomplete preserved for the known ones) instead of rejecting `TradeUpdate.event`. Checked the reference's all-optional `RawNews` shape against Alpaca's actual real-time-news docs (which show every field but `url` as always-present, matching our existing `News & {T:"n"}` reuse) - kept ours as-is rather than copying a looser shape that the primary source doesn't back up. Michi-Issue: 14
|
😺 Michi — compared our streaming types against alpacahq/alpaca-ts-alpha's types.ts. Already covered: trade/quote/bar (stocks/crypto), trade/quote (options), trading stream's Was missing, now added:
None of the stock-only four have a REST endpoint, so unlike trade/quote/bar there was no generated model to extend — hand-written directly from Alpaca's real-time stock/crypto docs' field tables, each anchored by a golden-payload test taken straight from those docs' examples. Also ported one good idea from the reference: One thing I checked but did not copy: the reference makes every Commit: Still ready for review: #15 |
…al snapshot, not every update (#14) Live-validated every streaming type against the real paper API (60s window, all 5 streams running concurrently - confirmed first that running stock/ crypto/option/news connections at once doesn't hit any account connection limit, they all open/authenticate independently): captured 61 real crypto orderbook updates, 25 quotes, a trade, a bar, and a live news article. CryptoQuote, CryptoTrade, CryptoBar, and News matched our types exactly - no mismatches. CryptoOrderbook didn't: `r` (the full-reset/snapshot flag) was typed as always-present (matching Alpaca's one documented example, which happens to be a snapshot), but every one of the 60 incremental updates in the capture - a single bid/ask level change, the other side's array empty - omitted it entirely. `r` is now optional; added a regression test alongside the existing snapshot one. Stock trade/quote/bar/status/LULD/correction/cancel-error and option trade/ quote weren't exercised (US equity/option markets are closed after-hours; confirmed the stock stream itself authenticates and subscribes correctly, it's a market-conditions gap, not a connection issue) - same for trade_updates (no order activity on this account). Those keep their docs-table-sourced typing from the previous commit, cross-checked but not live-confirmed. Michi-Issue: 14
|
😺 Michi — live-validated every streaming type against the real paper API (60s window, all 5 streams running concurrently — first confirmed that doesn't hit any per-account connection limit, they all open/authenticate independently). Captured and matched exactly (no field mismatches): Found and fixed a real bug: Not exercised this session (and said so honestly rather than claiming false coverage): stock trade/quote/bar/status/LULD/correction/cancel-error and option trade/quote — US equity/option markets are closed after-hours (confirmed the stock stream itself connects, authenticates, and subscribes correctly — it's market conditions, not a bug); Commit: Still ready for review: #15 |
…st a real paper fill (#14) Generated real trade_updates by round-tripping 1 share of AAPL on the paper account (extended-hours limit buy, then sell to flatten) - captured pending_new/new/fill for both orders. Every event, not just fills, carries an `event_id` (unique per update) and `at` (a slightly different-precision timestamp from `timestamp`) that this type didn't declare at all; fills also carry `settle_date`. All three are now typed (optional, since only verified present - not proven required on every variant), with a regression test using the real captured fill payload. Also confirmed live: paper-account fills are purely simulated against Alpaca's paper engine and never appear on the public market-data feed - so order placement can validate `trade_updates` but cannot be used to generate real stock/option trade prints for `StockMessage`/`OptionMessage`. Tried two separate organic-listening windows (150s total) across ten liquid extended-hours symbols on the stock stream and got zero trades/quotes/bars - this paper environment's IEX extended-hours flow is apparently too sparse to catch outside regular session hours. Stock trade/quote/bar, status/LULD/ correction/cancel-error, and option trade/quote remain docs-sourced and cross-checked but not live-confirmed; status/LULD/correction/cancel-error specifically can never be generated by any account action (exchange/ regulator-only events), and the option market has no extended hours at all. Also noted, not fixed (out of scope - it's the generated Order REST model, which CLAUDE.md says never to hand-edit): the real fill's order payload included a `cancel_requested_at` field the generated `Order` type doesn't declare - an upstream OpenAPI-spec gap. Michi-Issue: 14
Pure readability pass over the base streaming client; every test (49 streaming
cases incl. the close/reconnect/idle-timeout race regressions) stays green.
- Hoist module helpers above the class and give them names: `decodeJson`
(the default frame decode, with its explanatory comment), `encodeJson`,
`asError`, and `TEXT_DECODER` (was a stray `const` dangling below the class).
- Collapse the four near-identical `events.push({type:'error', error: ...})`
sites (socket error, auth-send failure, decode failure, idle timeout) into
one `private emitError(err: unknown)`.
- Express `subscribe`/`unsubscribe` in terms of `track`/`forget` so the
subscription-map set/delete lives in exactly one place each.
- Name the `connect()` re-entry guard via a `socketInFlight` getter instead
of the inline 4-way state comparison.
- Extract the terminal close/reconnect-disabled transition (state -> 'closed'
+ end the queue), shared by `close()` and `handleClose()`, into `finalize()`.
No state-machine, timing, or public-API changes.
Michi-Issue: 14
…sy Dates (#14) Regular-hours live capture against the paper API validated the remaining streaming types. Stock trade/quote/bar and (bonus) a real LULD event all matched their types exactly - 33528 quotes, 689 trades, 12 bars, 1 LULD, zero mismatches. Options surfaced a real bug: every OptionTrade/OptionQuote failed type validation on `t`. Cause: the option data stream is msgpack, and Alpaca sends each `t` as an 8-byte msgpack timestamp extension (confirmed at the byte level - 0xd7 marker). Our decoder turned that into a JS `Date`, so option `t` was a `Date` at runtime while typed `string` (via the generated `Timestamp = string`), inconsistent with the four JSON streams whose `t` is a nanosecond ISO string - and lossy, since `Date` holds only milliseconds while Alpaca sends nanoseconds. The decoder now renders the timestamp extension as an RFC3339 string with full nanosecond precision, so `t` is uniformly a string across every stream and the existing OptionTrade/OptionQuote types are correct as-is. Re-captured live after the fix: OptionQuote 371 ok, OptionTrade 83 ok, zero mismatches, `t` e.g. "2026-07-01T17:32:44.341415523Z" (9 fractional digits preserved). Updated the three timestamp-decode tests to the string form and added one proving nanosecond precision survives (a Date would have truncated it). Live-confirmed now: stock trade/quote/bar + LULD, option trade/quote. Still not observed (rare exchange/regulator events, can't be forced): stock trading halt (status), trade correction, cancel/error. Michi-Issue: 14
|
😺 Michi — turned out the market was already open (it's a trading day now), so I captured live during regular hours instead of scheduling. Much higher liquidity closed out almost everything. Live-confirmed, zero type mismatches:
Found and fixed a real bug in the process: every OptionTrade/OptionQuote failed validation on Fixed the decoder to render timestamp extensions as RFC3339 nanosecond strings, so Still not observed — and genuinely can't be forced: stock trading halt (status), correction, and cancel/error. These are exchange/regulator-triggered, not something any account or market condition on our side produces. Their types stay docs-sourced + cross-checked. Everything else in the streaming surface is now live-validated end to end. Commit: Ready for review: #15 |
1 participant
Closes #14
Hand-written WebSocket streaming for
@alpaca-open-api/core(Phase 1 + Phase 2), living alongside the mutator seam — never undersrc/generated/.Decisions (per issue discussion): native global
WebSocket(no dep, bumpsengines.nodeto>=22); hand-rolled minimal msgpack decode for the trading stream's binary frames (no dep); consumer API is a typedEventEmitterplus async iterator. Data streams (stocks/crypto/options/news) are JSON and share one protocol; the tradingtrade_updatesstream is msgpack with its own auth/listen protocol.engines.nodeto>=22trade_updatesover msgpack frames (auth/listen protocol) + tests