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 <noreply@anthropic.com>
This commit is contained in:
1 parent
d16eb85ee9
commit
119e4c0ad1
4 files changed
+168
-2
No files matched your search
+17
-1
@@ -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)
|
||||
|
||||
@@ -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")
|
||||
|
||||
|
||||
@@ -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")
|
||||
@@ -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"}]
|
||||
Reference in new issue
Block a user