Skip to content

fix: reconcile terminal orders during cancel#93

Open
rupurt wants to merge 1 commit into
TexasCoding:mainfrom
rupurt:codex/reconcile-cancel-terminal-orders
Open

fix: reconcile terminal orders during cancel#93
rupurt wants to merge 1 commit into
TexasCoding:mainfrom
rupurt:codex/reconcile-cancel-terminal-orders

Conversation

@rupurt

@rupurt rupurt commented Jun 30, 2026

Copy link
Copy Markdown

What changed

  • Adds OrderManager.search_orders() for historical /Order/search queries so terminal orders can be retrieved, not only currently-open orders.
  • Updates get_order_by_id() to fall back from open-order search to historical order search.
  • Updates cancel_order() to reconcile terminal broker state after a failed cancel response:
    • already/could-have-been cancelled returns True
    • filled/expired/rejected returns False without raising a cancel submission exception
  • Narrows the cancel lock scope so reconciliation API lookups do not deadlock on order cache updates.

Why

A cancel request can race with broker-side fills or terminal status transitions. Before this change, get_order_by_id() only searched open orders, so cancel_order() could not reconcile the true terminal state and raised ProjectXOrderError from vague broker responses such as a failed cancel with no useful error detail.

Impact

Callers can now distinguish an actual cancel failure from an order that became terminal while cancellation was in flight. This preserves the existing bool return type while avoiding false hard failures for normal broker lifecycle races.

Validation

  • uv run ruff check src/project_x_py/order_manager/core.py tests/order_manager/test_order_core.py tests/order_manager/test_core_advanced.py --fix
  • uv run pytest tests/order_manager/test_order_core.py tests/order_manager/test_core_advanced.py

@rupurt rupurt marked this pull request as ready for review June 30, 2026 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant