From a2de50479a9148de50df5fa0e7a85de3d0b1aec6 Mon Sep 17 00:00:00 2001 From: momentum dev Date: Tue, 28 Jul 2026 09:11:00 -0500 Subject: [PATCH] fix: three Tradier sandbox incident fixes (2026-07-28 reject loop) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Incident: entry-timeout cancels raced lazy sandbox fills -> untracked positions; naked-stop guard then force-sold but every sell rejected ('more shares than your current long position') for hours. 1. translate_order: Tradier OTOCO/OTO parents carry no top-level symbol/side/qty (legs only) — inherit from the entry leg so symbol-scoped lookups can see brackets; get_orders symbols filter also matches by leg symbols. Without this the stop-leg search read a fully-protected position as naked. 2. close_position: cancel the symbol's resting sell orders/legs first — Tradier reserves held shares against open sells, so selling without releasing them always rejects. 3. cancel_entry (broker-agnostic): after cancelling, re-check the broker's final word by client_order_id; journal a fill for any executed shares instead of orphaning a real position (2026-07-06 class, same race exists on Alpaca). 49 tests pass incl. new coverage for all three. Co-Authored-By: Claude Fable 5 --- alpaca_paper/execution.py | 45 ++++++++++++++++++++++-- tests/test_tradier_client.py | 66 ++++++++++++++++++++++++++++++++++++ tradier_broker/client.py | 39 +++++++++++++++++++-- 3 files changed, 146 insertions(+), 4 deletions(-) diff --git a/alpaca_paper/execution.py b/alpaca_paper/execution.py index bf80291..ceb6813 100644 --- a/alpaca_paper/execution.py +++ b/alpaca_paper/execution.py @@ -251,16 +251,57 @@ class AlpacaPaperExecutor: def cancel_entry( self, order_id: str, broker_order_id: str | None, symbol: str, reason: str ) -> ExecutionResult: - """Cancel a resting (unfilled) entry order at the broker and record it.""" + """Cancel a resting (unfilled) entry order at the broker and record it. + + CANCEL-vs-FILL RACE: the entry can fill at the broker before (or as) + the cancel lands — the cancel then no-ops but journaling only a cancel + orphans a REAL position (untracked shares, the 2026-07-06 class; + happened live on Tradier sandbox 2026-07-28: lazy fills + entry + timeouts). After cancelling, re-check the broker's final word and emit + the fill for any executed shares so the position is tracked.""" try: if broker_order_id: self.client.cancel_order(broker_order_id) except AlpacaApiError as exc: # already filled/cancelled at the broker, or transient — log and - # still emit the cancel so our state stops tracking it + # continue to the fill-race check below logger.warning("cancel of %s returned: %s", order_id, exc) except Exception as exc: # noqa: BLE001 logger.warning("cancel of %s errored: %s", order_id, exc) + + filled_qty, fill_price, side = 0, None, "buy" + getter = getattr(self.client, "get_order_by_client_order_id", None) + if getter is not None: + try: + found = getter(order_id[:48]) or {} + filled_qty = int(float(found.get("filled_qty") or 0)) + fill_price = float(found.get("filled_avg_price") or 0) or None + side = found.get("side") or "buy" + except Exception: # noqa: BLE001 — lookup failure: keep cancel-only path + pass + + if filled_qty > 0: + logger.warning("cancel of %s raced a fill: %d %s executed @ %s — " + "journaling the fill", order_id, filled_qty, symbol, fill_price) + self.store.emit( + OrderFilledEvent( + timestamp=datetime.now(), + mode=self.mode, + correlation_id=self.session_id, + message=f"Filled {symbol} {filled_qty} @ {fill_price} " + f"(cancel raced fill)", + symbol=symbol, + order_id=order_id, + fill_price=fill_price or 0.0, + fill_quantity=filled_qty, + side=side, + payload={"symbol": symbol, "order_id": order_id, + "fill_price": fill_price, "fill_quantity": filled_qty, + "side": side, "cancel_raced_fill": True}, + ) + ) + return ExecutionResult(ok=True, order_id=order_id, status="filled") + self._emit_cancelled( ExecutionRequest(symbol=symbol, side="buy", quantity=0, order_id=order_id), reason, diff --git a/tests/test_tradier_client.py b/tests/test_tradier_client.py index 56a89bd..72a0720 100644 --- a/tests/test_tradier_client.py +++ b/tests/test_tradier_client.py @@ -63,6 +63,31 @@ class TranslateOrderTests(unittest.TestCase): self.assertEqual(o["legs"][0]["limit_price"], "9.0") self.assertEqual(o["legs"][0]["status"], "new") + def test_otoco_parent_inherits_symbol_from_entry_leg(self): + # Tradier bracket parents carry no top-level symbol/side/qty — the + # 2026-07-28 naked-stop false-positive + o = translate_order({ + "id": 1, "class": "otoco", "status": "open", + "leg": [ + {"id": 2, "symbol": "DAMD", "side": "buy", "quantity": 8000, + "type": "limit", "status": "filled", "exec_quantity": 8000, + "avg_fill_price": 1.96}, + {"id": 3, "symbol": "DAMD", "side": "sell", "quantity": 8000, + "type": "limit", "status": "open", "price": 2.5}, + {"id": 4, "symbol": "DAMD", "side": "sell", "quantity": 8000, + "type": "stop", "status": "open", "stop_price": 1.8}, + ], + }) + self.assertEqual(o["symbol"], "DAMD") + self.assertEqual(o["side"], "buy") + self.assertEqual(o["qty"], "8000") + self.assertEqual(o["filled_qty"], "8000") + # the stop leg is findable the way exit_manager._stop_leg looks + stops = [leg for leg in o["legs"] if leg["type"] == "stop" + and leg["side"] == "sell" and leg["symbol"] == "DAMD"] + self.assertEqual(len(stops), 1) + self.assertEqual(stops[0]["stop_price"], "1.8") + def test_single_leg_dict_normalized(self): o = translate_order({"class": "oto", "status": "open", "leg": {"id": 9, "type": "stop", "stop": 4.5, @@ -175,6 +200,17 @@ class ClientPathTests(unittest.TestCase): open_pn = client.get_orders(status="open", symbols=["PN"]) self.assertEqual([o["id"] for o in open_pn], ["1"]) + def test_symbols_filter_matches_bracket_by_legs(self): + client = self._client_with_orders({"order": [ + {"id": 1, "class": "otoco", "status": "open", + "leg": [{"id": 2, "symbol": "DAMD", "side": "buy", "status": "filled"}, + {"id": 3, "symbol": "DAMD", "side": "sell", "type": "stop", + "status": "open", "stop_price": 1.8}]}, + {"id": 9, "symbol": "MUZ", "status": "open"}, + ]}) + got = client.get_orders(status="all", symbols=["DAMD"]) + self.assertEqual([o["id"] for o in got], ["1"]) + def test_lookup_by_tag(self): client = self._client_with_orders({"order": [ {"id": 1, "symbol": "PN", "status": "open", "tag": "want"}, @@ -205,3 +241,33 @@ class ClientPathTests(unittest.TestCase): if __name__ == "__main__": unittest.main() + + +class CancelRacedFillTests(unittest.TestCase): + """cancel_entry must journal a fill the cancel raced (2026-07-28 incident).""" + + def _executor(self, found_order): + from unittest.mock import MagicMock + from alpaca_paper.execution import AlpacaPaperExecutor + client = TradierClient(SETTINGS) + client.cancel_order = lambda oid: None + client.get_order_by_client_order_id = lambda cid: found_order + store = MagicMock() + return AlpacaPaperExecutor(store, client=client), store + + def test_raced_fill_emits_fill_not_cancel(self): + ex, store = self._executor( + {"filled_qty": "8000", "filled_avg_price": "1.96", "side": "buy"}) + res = ex.cancel_entry("cid-1", "36082713", "DAMD", "entry timed out") + self.assertEqual(res.status, "filled") + emitted = [c.args[0].__class__.__name__ for c in store.emit.call_args_list] + self.assertIn("OrderFilledEvent", emitted) + self.assertNotIn("OrderCancelledEvent", emitted) + + def test_clean_cancel_still_emits_cancel(self): + ex, store = self._executor({"filled_qty": "0", "status": "canceled"}) + res = ex.cancel_entry("cid-1", "36082713", "DAMD", "entry timed out") + self.assertEqual(res.status, "cancelled") + emitted = [c.args[0].__class__.__name__ for c in store.emit.call_args_list] + self.assertIn("OrderCancelledEvent", emitted) + self.assertNotIn("OrderFilledEvent", emitted) diff --git a/tradier_broker/client.py b/tradier_broker/client.py index 1263ff1..cebc9da 100644 --- a/tradier_broker/client.py +++ b/tradier_broker/client.py @@ -92,6 +92,20 @@ def translate_order(o: dict) -> dict: legs = [translate_order(leg) for leg in _one_or_many(o, "leg")] \ if isinstance(o.get("leg"), (list, dict)) else None filled_at = o.get("transaction_date") if status == "filled" else None + if legs and not o.get("symbol"): + # Tradier OTOCO/OTO PARENT orders carry no top-level symbol/side/qty — + # only the legs do. Inherit from the entry leg (leg[0]) so symbol-scoped + # lookups (stop-leg search, naked-stop guard) can see bracket orders; + # without this the guard reads a protected position as naked and + # force-exits it (2026-07-28 sandbox incident). + entry_leg = legs[0] + o = {**o, "symbol": entry_leg.get("symbol"), + "side": o.get("side") or entry_leg.get("side"), + "quantity": o.get("quantity") or entry_leg.get("qty"), + "exec_quantity": o.get("exec_quantity") or entry_leg.get("filled_qty"), + "avg_fill_price": o.get("avg_fill_price") or entry_leg.get("filled_avg_price"), + "create_date": o.get("create_date") or entry_leg.get("submitted_at"), + "transaction_date": o.get("transaction_date") or entry_leg.get("updated_at")} return { "id": _s(o.get("id")), "client_order_id": o.get("tag") or "", @@ -401,7 +415,14 @@ class TradierClient: orders = [translate_order(o) for o in self._orders_raw()] if symbols: want = {s.upper() for s in symbols} - orders = [o for o in orders if (o.get("symbol") or "").upper() in want] + + def _matches(o: dict) -> bool: + if (o.get("symbol") or "").upper() in want: + return True + # bracket parents/legs: match if ANY leg is on a wanted symbol + return any((leg.get("symbol") or "").upper() in want + for leg in (o.get("legs") or [])) + orders = [o for o in orders if _matches(o)] if status == "open": orders = [o for o in orders if o.get("status") in _OPEN_STATUSES] elif status == "closed": @@ -488,7 +509,12 @@ class TradierClient: def close_position(self, symbol: str, qty: int | None = None, percentage: float | None = None) -> dict: - """Market-sell held shares (Tradier has no close-position endpoint).""" + """Market-sell held shares (Tradier has no close-position endpoint). + + Cancels the symbol's resting SELL orders (bracket stop/target legs) + first — Tradier reserves held shares against open sells, so selling + without releasing them rejects with 'more shares than your current + long position' (2026-07-28 sandbox incident).""" self._invalidate_cache() held = 0 for p in self.get_positions(fresh=True): @@ -497,6 +523,15 @@ class TradierClient: break if held <= 0: raise TradierApiError(404, f"no long position in {symbol}") + for o in self.get_orders(status="open", symbols=[symbol]): + for node in [o, *(o.get("legs") or [])]: + if (node.get("side") == "sell" and node.get("id") + and node.get("status") in _OPEN_STATUSES + and (node.get("symbol") or "").upper() == symbol.upper()): + try: + self.cancel_order(node["id"]) + except TradierApiError as exc: # already finalized etc. + logger.warning("leg cancel before close %s: %s", symbol, exc) sell = held if qty is not None: sell = min(int(qty), held)