From 119e4c0ad1fd87ea2f3f8b32d94199319bbcfc3a Mon Sep 17 00:00:00 2001 From: momentum dev Date: Mon, 13 Jul 2026 22:24:14 -0400 Subject: [PATCH] money-path: reconcile a lost order-submit by client_order_id before journaling cancel Plan 003 step 4. A transport error AFTER the POST landed leaves a live order resting at the broker; journaling it 'cancelled' breeds the duplicate/naked order (2026-07-06 incident). Add client.get_order_by_client_order_id + an after= passthrough on get_orders; on the generic-exception path, look up the order by the client_order_id we already send and, if live, emit the normal submitted sequence (ok=True) instead of cancelled. AlpacaApiError (real rejection) is NOT reconciled. Fills the empty test_alpaca_client.py. Co-Authored-By: Claude Fable 5 --- alpaca_paper/client.py | 18 +++++- alpaca_paper/execution.py | 37 ++++++++++- .../alpaca_paper/test_execution_and_sync.py | 52 +++++++++++++++ tests/unit/test_alpaca_client.py | 63 +++++++++++++++++++ 4 files changed, 168 insertions(+), 2 deletions(-) diff --git a/alpaca_paper/client.py b/alpaca_paper/client.py index b2d2169..00bb492 100644 --- a/alpaca_paper/client.py +++ b/alpaca_paper/client.py @@ -183,9 +183,23 @@ class AlpacaPaperClient: return val return self._cached("positions", lambda: self._trading("GET", "/positions")) + def get_order_by_client_order_id(self, client_order_id: str) -> dict | None: + """The single order carrying this client_order_id, or None if unknown. + + Used to reconcile a LOST submit response (network drop after the POST + left): the order may actually be resting at the broker, so we look it up + by the client_order_id we sent before assuming it was cancelled. A 404 + (never landed) surfaces as an AlpacaApiError the caller treats as 'not + found'. `_trading` already prefixes /v2; the literal ':' in the path is + Alpaca's by-client-id sub-resource, not a typo.""" + return self._trading( + "GET", "/orders:by_client_order_id", + params={"client_order_id": client_order_id}, + ) + def get_orders( self, status: str = "all", limit: int = 500, nested: bool = True, - symbols: list[str] | None = None, + symbols: list[str] | None = None, after: str | None = None, ) -> list[dict]: # nested=true includes bracket child legs (stop/target) so working risk # isn't under-reported in the orders snapshot. @@ -199,6 +213,8 @@ class AlpacaPaperClient: # the exit manager / safety checks must do. params: dict = {"status": status, "limit": min(limit, 500), "nested": str(nested).lower(), "direction": "desc"} + if after: # only orders submitted after this ISO timestamp (exclusive) — the cage-count reconcile scopes to today + params["after"] = after if symbols: # scoped to a few names — well under the cap, one page is complete params["symbols"] = ",".join(symbols) diff --git a/alpaca_paper/execution.py b/alpaca_paper/execution.py index 992db11..5d2c23b 100644 --- a/alpaca_paper/execution.py +++ b/alpaca_paper/execution.py @@ -122,12 +122,47 @@ class AlpacaPaperExecutor: ok=False, order_id=request.order_id, error=str(exc) ) except Exception as exc: + # LOST RESPONSE: the POST may have LANDED before the connection dropped, + # leaving a real order resting at the broker. Reconcile by the + # client_order_id we sent before assuming it was cancelled — journalling + # a live order as 'cancelled' is how a duplicate/naked order is born + # (the 2026-07-06 incident). Only the generic/transport error path + # reconciles; an AlpacaApiError above is a genuine broker rejection. logger.exception("alpaca order failed") - self._emit_cancelled(request, f"submit error: {exc}") + recovered = self._reconcile_lost_submit(request) + if recovered is not None: + logger.warning("recovered lost submit for %s via client_order_id " + "(broker status=%s)", request.symbol, recovered.get("status")) + return self._finalize(request, recovered) + self._emit_cancelled(request, f"submit error (reconcile: not found): {exc}") return ExecutionResult( ok=False, order_id=request.order_id, error=str(exc) ) + return self._finalize(request, raw) + + _LIVE_STATUSES = {"new", "accepted", "pending_new", "partially_filled", + "filled", "held", "accepted_for_bidding"} + + def _reconcile_lost_submit(self, request: "ExecutionRequest") -> dict | None: + """Look up the order by the client_order_id we sent; return it only if it + is actually LIVE at the broker (so a lost response isn't journaled as a + cancel). Returns None on 404/lookup error — then the caller emits cancelled.""" + getter = getattr(self.client, "get_order_by_client_order_id", None) + if getter is None: + return None + try: + found = getter(request.order_id[:48]) + except Exception: # noqa: BLE001 — 404 (never landed) or transport error + return None + if found and str(found.get("status")) in self._LIVE_STATUSES: + return found + return None + + def _finalize(self, request: "ExecutionRequest", raw: dict) -> ExecutionResult: + """Record a submission's outcome from the broker response (or a recovered + order) and return the ExecutionResult — one event sequence for the normal + and the reconcile-recovered paths alike.""" broker_order_id = raw.get("id") status = raw.get("status") diff --git a/tests/unit/alpaca_paper/test_execution_and_sync.py b/tests/unit/alpaca_paper/test_execution_and_sync.py index ab09915..f11b404 100644 --- a/tests/unit/alpaca_paper/test_execution_and_sync.py +++ b/tests/unit/alpaca_paper/test_execution_and_sync.py @@ -259,3 +259,55 @@ def test_post_not_retried_on_urlerror(monkeypatch): with pytest.raises(urllib.error.URLError): cli._request("GET", "https://x/v2/account") assert calls["n"] > 1, "idempotent GET should still retry" + +# --- lost-response reconcile (plan 003 step 4) ------------------------------- + +def test_lost_submit_recovered_via_client_order_id(store): + """A transport error AFTER the POST landed: the order is live at the broker. + Reconcile by client_order_id -> emit submitted (not cancelled), ok=True.""" + class LostThenFound(FakeClient): + def submit_order(self, **kw): + raise ConnectionError("connection reset after POST") + def get_order_by_client_order_id(self, coid): + return {"id": "broker-999", "status": "accepted", "client_order_id": coid} + executor = AlpacaPaperExecutor(store, client=LostThenFound(), session_id="t") + result = executor.execute(ExecutionRequest(symbol="GOOD", side="buy", quantity=5, + entry_price=14.0, stop_loss_price=13.45)) + assert result.ok and result.broker_order_id == "broker-999" + assert len(store.query_events(event_type="order_submitted")) == 1 + assert store.query_events(event_type="order_cancelled") == [] + + +def test_lost_submit_not_found_emits_cancelled(store): + """Transport error and the order never landed (404 -> lookup raises): emit + cancelled, ok=False (unchanged behavior, now with an honest reason).""" + class LostNotFound(FakeClient): + def submit_order(self, **kw): + raise ConnectionError("connection reset before POST") + def get_order_by_client_order_id(self, coid): + raise AlpacaApiError(404, "order not found") + executor = AlpacaPaperExecutor(store, client=LostNotFound(), session_id="t") + result = executor.execute(ExecutionRequest(symbol="GOOD", side="buy", quantity=5)) + assert not result.ok + assert store.query_events(event_type="order_cancelled") + assert store.query_events(event_type="order_submitted") == [] + + +def test_broker_rejection_not_reconciled(store): + """An AlpacaApiError is a genuine rejection, NOT a lost response — it must not + trigger a reconcile lookup (regression guard on the two-branch split).""" + class Rejected(FakeClient): + def __init__(self): + super().__init__() + self.lookup_calls = 0 + def submit_order(self, **kw): + raise AlpacaApiError(422, "insufficient buying power") + def get_order_by_client_order_id(self, coid): + self.lookup_calls += 1 + return {"id": "x", "status": "accepted"} + client = Rejected() + executor = AlpacaPaperExecutor(store, client=client, session_id="t") + result = executor.execute(ExecutionRequest(symbol="GOOD", side="buy", quantity=5)) + assert not result.ok + assert client.lookup_calls == 0 + assert store.query_events(event_type="order_cancelled") diff --git a/tests/unit/test_alpaca_client.py b/tests/unit/test_alpaca_client.py index e69de29..3cd1449 100644 --- a/tests/unit/test_alpaca_client.py +++ b/tests/unit/test_alpaca_client.py @@ -0,0 +1,63 @@ +"""AlpacaPaperClient request-shaping tests (plan 003 step 4 + client hardening). + +The client's trickiest logic — by-client-order-id lookup, the `after=` passthrough, +and get_orders scoping — only bites in live trading, so pin the request shapes here +with a stubbed transport (no network). +""" + +import pytest + +from alpaca_paper.client import AlpacaApiError, AlpacaPaperClient + + +class _RecordingClient(AlpacaPaperClient): + """Bypass __init__/network: capture (method, url, params) from _request and + return a scripted response, so we can assert the exact request shape.""" + def __init__(self, response=None, raise_status=None): + self.calls = [] + self._response = response if response is not None else [] + self._raise_status = raise_status + + class _S: + trading_base_url = "https://paper-api.alpaca.markets" + data_base_url = "https://data.alpaca.markets" + self.settings = _S() + + def _request(self, method, url, params=None, **kw): + self.calls.append({"method": method, "url": url, "params": params or {}}) + if self._raise_status: + raise AlpacaApiError(self._raise_status, "err") + return self._response + + +def test_get_order_by_client_order_id_request_shape(): + c = _RecordingClient(response={"id": "b1", "status": "accepted"}) + out = c.get_order_by_client_order_id("my-coid-123") + assert out == {"id": "b1", "status": "accepted"} + call = c.calls[-1] + assert call["method"] == "GET" + assert call["url"].endswith("/v2/orders:by_client_order_id") + assert call["params"] == {"client_order_id": "my-coid-123"} + + +def test_get_order_by_client_order_id_404_raises(): + c = _RecordingClient(raise_status=404) + with pytest.raises(AlpacaApiError): + c.get_order_by_client_order_id("missing") + + +def test_get_orders_after_passthrough(): + c = _RecordingClient(response=[]) + c.get_orders(status="all", symbols=["AAA"], after="2026-07-13T00:00:00-04:00") + params = c.calls[-1]["params"] + assert params["after"] == "2026-07-13T00:00:00-04:00" + assert params["symbols"] == "AAA" # scoped query, single page + assert params["status"] == "all" + + +def test_get_orders_scoped_is_single_page(): + # a symbols= query is scoped under the cap: exactly one request, no pagination + c = _RecordingClient(response=[{"id": "1"}, {"id": "2"}]) + out = c.get_orders(symbols=["AAA", "BBB"]) + assert len(c.calls) == 1 + assert out == [{"id": "1"}, {"id": "2"}]