cancel all orders hanging fix

AGENTS.md

@@ -63,6 +63,13 @@ TRADER-MCP is structured in three layers:
 
 ---
 
+## Local Environment
+
+- before claiming a Python dependency is missing, source the repo-local `.venv`
+- prefer running Python, pytest, and helper scripts through the activated `.venv`
+
+---
+
 ## Interface Stability
 
 Changes affecting:

src/trader_mcp/strategy_context.py

@@ -8,6 +8,44 @@ from .news_client import call_news_tool
 from .crypto_client import call_crypto_tool
 
 
+def _order_ref(payload: Any) -> str | None:
+    if not isinstance(payload, dict):
+        return None
+    for key in ("order_id", "bitstamp_order_id", "id", "client_order_id"):
+        value = payload.get(key)
+        if value is None:
+            continue
+        text = str(value).strip()
+        if text:
+            return text
+    return None
+
+
+def _cancelled_order_ids(cancel_result: Any) -> list[str]:
+    if not isinstance(cancel_result, dict):
+        return []
+    rows = cancel_result.get("cancelled")
+    if not isinstance(rows, list):
+        return []
+    cancelled: list[str] = []
+    for row in rows:
+        if not isinstance(row, dict) or not bool(row.get("ok")):
+            continue
+        order_id = _order_ref(row)
+        if order_id is not None:
+            cancelled.append(order_id)
+    return cancelled
+
+
+def _has_inconclusive_cancel_rows(cancel_result: Any) -> bool:
+    if not isinstance(cancel_result, dict):
+        return False
+    rows = cancel_result.get("cancelled")
+    if not isinstance(rows, list):
+        return False
+    return any(isinstance(row, dict) and not bool(row.get("ok")) for row in rows)
+
+
 @dataclass(frozen=True)
 class StrategyContext:
     id: str
@@ -36,6 +74,59 @@ class StrategyContext:
     def cancel_all_orders(self) -> Any:
         return cancel_all_orders(self.account_id, self.client_id)
 
+    def cancel_all_orders_confirmed(self) -> dict[str, Any]:
+        cancel_result = None
+        cancel_error = None
+        verification_error = None
+        remaining_orders = None
+
+        try:
+            cancel_result = self.cancel_all_orders()
+        except Exception as exc:
+            cancel_error = str(exc)
+
+        try:
+            payload = self.get_open_orders()
+            if isinstance(payload, list):
+                remaining_orders = payload
+            else:
+                verification_error = f"unexpected open orders payload type: {type(payload).__name__}"
+        except Exception as exc:
+            verification_error = str(exc)
+
+        cancelled_order_ids = _cancelled_order_ids(cancel_result)
+        inconclusive_cancel = cancel_error is not None or _has_inconclusive_cancel_rows(cancel_result)
+        conclusive = (
+            not inconclusive_cancel
+            and remaining_orders is not None
+            and len(remaining_orders) == 0
+        )
+        cleanup_status = "cleanup_confirmed" if conclusive else ("cleanup_failed" if cancel_error else "cleanup_partial")
+        error = None
+        if not conclusive:
+            if cancel_error:
+                error = cancel_error
+            elif _has_inconclusive_cancel_rows(cancel_result):
+                error = "cancel-all reported uncancelled orders"
+            elif verification_error:
+                error = verification_error
+            elif remaining_orders is not None:
+                error = f"{len(remaining_orders)} open orders remain after cancel-all"
+            else:
+                error = "open order verification unavailable after cancel-all"
+
+        return {
+            "ok": cancel_error is None and verification_error is None and conclusive,
+            "conclusive": conclusive,
+            "cleanup_status": cleanup_status,
+            "cancelled_order_ids": cancelled_order_ids,
+            "remaining_orders": remaining_orders,
+            "cancel_result": cancel_result,
+            "cancel_error": cancel_error,
+            "verification_error": verification_error,
+            "error": error,
+        }
+
     def cancel_order(self, order_id: str) -> Any:
         return cancel_order(self.account_id, order_id)
 

strategies/grid_trader.py

@@ -58,6 +58,7 @@ class Strategy(Strategy):
         "seeded": {"type": "bool", "default": False},
         "last_action": {"type": "string", "default": "idle"},
         "last_error": {"type": "string", "default": ""},
+        "cleanup_status": {"type": "string", "default": ""},
         "orders": {"type": "list", "default": []},
         "order_ids": {"type": "list", "default": []},
         "debug_log": {"type": "list", "default": []},
@@ -85,6 +86,7 @@ class Strategy(Strategy):
             "seeded": False,
             "last_action": "idle",
             "last_error": "",
+            "cleanup_status": "",
             "orders": [],
             "order_ids": [],
             "debug_log": ["init cancel all orders"],
@@ -139,13 +141,73 @@ class Strategy(Strategy):
     def _trip_recovery_pause(self, seconds: float = 30.0) -> None:
         self.state["recovery_cooldown_until"] = (datetime.now(timezone.utc).timestamp() + max(seconds, 0.0))
 
+    def _tracked_order_id(self, order: dict | object) -> str:
+        if not isinstance(order, dict):
+            return ""
+        for key in ("bitstamp_order_id", "order_id", "id", "client_order_id"):
+            value = order.get(key)
+            if value is not None and str(value).strip():
+                return str(value).strip()
+        result = order.get("result")
+        if isinstance(result, dict):
+            for key in ("bitstamp_order_id", "order_id", "id", "client_order_id"):
+                value = result.get(key)
+                if value is not None and str(value).strip():
+                    return str(value).strip()
+        return ""
+
+    def _drop_tracked_orders(self, order_ids: list[str] | set[str]) -> int:
+        drop_ids = {str(order_id).strip() for order_id in (order_ids or []) if str(order_id).strip()}
+        if not drop_ids:
+            return 0
+
+        tracked_orders = list(self.state.get("orders") or [])
+        tracked_ids = [str(order_id).strip() for order_id in (self.state.get("order_ids") or []) if str(order_id).strip()]
+
+        kept_orders = [
+            order for order in tracked_orders
+            if self._tracked_order_id(order) not in drop_ids
+        ]
+        kept_ids = [order_id for order_id in tracked_ids if order_id not in drop_ids]
+        removed = len(tracked_ids) - len(kept_ids)
+
+        self.state["orders"] = kept_orders
+        self.state["order_ids"] = kept_ids
+        self.state["open_order_count"] = len(kept_ids)
+
+        return max(removed, 0)
+
+    def _cancel_all_orders_conclusive(self, failure_prefix: str) -> bool:
+        strict_cancel = getattr(self.context, "cancel_all_orders_confirmed", None)
+        if callable(strict_cancel):
+            result = strict_cancel()
+            cancelled_order_ids = result.get("cancelled_order_ids") or []
+            removed = self._drop_tracked_orders(cancelled_order_ids)
+            cleanup_status = str(result.get("cleanup_status") or ("cleanup_confirmed" if bool(result.get("conclusive")) else "cleanup_partial"))
+            self.state["cleanup_status"] = cleanup_status
+            if removed > 0:
+                self._log(f"cleanup removed tracked orders: ids={cancelled_order_ids}")
+            if bool(result.get("conclusive")):
+                return True
+            error = str(result.get("error") or "cancel-all inconclusive")
+        else:
+            try:
+                self.context.cancel_all_orders()
+                self.state["cleanup_status"] = "cleanup_confirmed"
+                return True
+            except Exception as exc:
+                self.state["cleanup_status"] = "cleanup_failed"
+                error = str(exc)
+
+        self.state["last_error"] = error
+        self._log(f"{failure_prefix}: {error}")
+        return False
+
     def _recover_grid(self, price: float) -> None:
         self._log(f"recovery mode: cancel all and rebuild from {price}")
-        try:
-            self.context.cancel_all_orders()
-        except Exception as exc:
-            self.state["last_error"] = str(exc)
-            self._log(f"recovery cancel-all failed: {exc}")
+        if not self._cancel_all_orders_conclusive("recovery cancel-all failed"):
+            self.state["last_action"] = "recovery cleanup pending"
+            return
         self.state["orders"] = []
         self.state["order_ids"] = []
         self.state["open_order_count"] = 0
@@ -812,11 +874,9 @@ class Strategy(Strategy):
             return
         current = float(self.state.get("center_price") or 0.0)
         self._log(f"{reason}: recenter from {current} to {price}")
-        try:
-            self.context.cancel_all_orders()
-        except Exception as exc:
-            self.state["last_error"] = str(exc)
-            self._log(f"{reason} cancel-all failed: {exc}")
+        if not self._cancel_all_orders_conclusive(f"{reason} cancel-all failed"):
+            self.state["last_action"] = f"{reason} cleanup pending"
+            return
         # Give the exchange a moment to release balance before we rebuild.
         time.sleep(3.0)
         self._refresh_balance_snapshot()
@@ -828,15 +888,13 @@ class Strategy(Strategy):
 
     def on_stop(self):
         self._log("stopping: cancel all open orders")
-        try:
-            self.context.cancel_all_orders()
-        except Exception as exc:
-            self.state["last_error"] = str(exc)
-            self._log(f"stop cancel-all failed: {exc}")
-        self.state["orders"] = []
-        self.state["order_ids"] = []
-        self.state["open_order_count"] = 0
-        self.state["last_action"] = "stopped"
+        if self._cancel_all_orders_conclusive("stop cancel-all failed"):
+            self.state["orders"] = []
+            self.state["order_ids"] = []
+            self.state["open_order_count"] = 0
+            self.state["last_action"] = "stopped"
+        else:
+            self.state["last_action"] = "stop cleanup pending"
 
     def _maybe_refresh_center(self, price: float) -> float:
         if price <= 0:
@@ -979,36 +1037,35 @@ class Strategy(Strategy):
         mode = self._mode()
 
         if mode != "active":
+            cleanup_pending = False
             if open_order_count > 0:
                 self._log("observe mode: cancel all open orders")
-                try:
-                    self.context.cancel_all_orders()
-                except Exception as exc:
-                    self.state["last_error"] = str(exc)
-                    self._log(f"observe cancel failed: {exc}")
-                self.state["orders"] = []
-                self.state["order_ids"] = []
-                self.state["open_order_count"] = 0
+                if self._cancel_all_orders_conclusive("observe cancel failed"):
+                    self.state["orders"] = []
+                    self.state["order_ids"] = []
+                    self.state["open_order_count"] = 0
+                else:
+                    cleanup_pending = True
 
             if not self.state.get("seeded") or not self.state.get("center_price"):
                 self.state["center_price"] = price
                 self.state["seeded"] = True
-                self.state["last_action"] = "observe monitor"
+                self.state["last_action"] = "observe cleanup pending" if cleanup_pending else "observe monitor"
                 self._log(f"observe at {price} dev 0.0000")
-                return {"action": "observe", "price": price, "deviation": 0.0}
+                return {"action": "observe", "price": price, "deviation": 0.0, "cleanup_pending": cleanup_pending}
 
             center = float(self.state.get("center_price") or price)
             recenter_pct = float(self.config.get("recenter_pct", 0.05) or 0.05)
             deviation = abs(price - center) / center if center else 0.0
             if deviation >= recenter_pct:
                 self.state["center_price"] = price
-                self.state["last_action"] = "observe monitor"
+                self.state["last_action"] = "observe cleanup pending" if cleanup_pending else "observe monitor"
                 self._log(f"observe at {price} dev {deviation:.4f}")
-                return {"action": "observe", "price": price, "deviation": deviation}
+                return {"action": "observe", "price": price, "deviation": deviation, "cleanup_pending": cleanup_pending}
 
-            self.state["last_action"] = "observe monitor"
+            self.state["last_action"] = "observe cleanup pending" if cleanup_pending else "observe monitor"
             self._log(f"observe at {price} dev {deviation:.4f}")
-            return {"action": "observe", "price": price, "deviation": deviation}
+            return {"action": "observe", "price": price, "deviation": deviation, "cleanup_pending": cleanup_pending}
 
         if stale_ids:
             self._log(f"stale live orders: {stale_ids}")

tests/test_strategies.py

@@ -93,6 +93,88 @@ def test_strategy_context_binds_identity(monkeypatch):
     assert calls["cancel_all"] == {"account_id": "acct-1", "client_id": "client-1"}
 
 
+def test_strategy_context_cancel_all_orders_confirmed_after_empty_followup(monkeypatch):
+    calls = {"open_orders": 0}
+
+    def fake_cancel_all(account_id, client_id=None):
+        calls["cancel_all"] = {"account_id": account_id, "client_id": client_id}
+        return {"ok": True, "cancelled": [{"ok": True, "order_id": "o1"}]}
+
+    def fake_open_orders(account_id, client_id=None):
+        calls["open_orders"] += 1
+        return {"orders": []}
+
+    monkeypatch.setattr("src.trader_mcp.strategy_context.cancel_all_orders", fake_cancel_all)
+    monkeypatch.setattr("src.trader_mcp.strategy_context.list_open_orders", fake_open_orders)
+
+    ctx = StrategyContext(id="inst-1", account_id="acct-1", client_id="client-1", mode="active")
+    result = ctx.cancel_all_orders_confirmed()
+
+    assert result["conclusive"] is True
+    assert result["cleanup_status"] == "cleanup_confirmed"
+    assert result["cancelled_order_ids"] == ["o1"]
+    assert result["remaining_orders"] == []
+    assert result["error"] is None
+    assert calls["cancel_all"] == {"account_id": "acct-1", "client_id": "client-1"}
+    assert calls["open_orders"] == 1
+
+
+def test_strategy_context_cancel_all_orders_confirmed_preserves_inconclusive_failure(monkeypatch):
+    calls = {"open_orders": 0}
+
+    def fake_cancel_all(account_id, client_id=None):
+        raise RuntimeError("auth breaker active")
+
+    def fake_open_orders(account_id, client_id=None):
+        calls["open_orders"] += 1
+        return {"orders": [{"id": "o1"}]}
+
+    monkeypatch.setattr("src.trader_mcp.strategy_context.cancel_all_orders", fake_cancel_all)
+    monkeypatch.setattr("src.trader_mcp.strategy_context.list_open_orders", fake_open_orders)
+
+    ctx = StrategyContext(id="inst-1", account_id="acct-1", client_id="client-1", mode="active")
+    result = ctx.cancel_all_orders_confirmed()
+
+    assert result["conclusive"] is False
+    assert result["cleanup_status"] == "cleanup_failed"
+    assert result["cancelled_order_ids"] == []
+    assert result["remaining_orders"] == [{"id": "o1"}]
+    assert result["error"] == "auth breaker active"
+    assert calls["open_orders"] == 1
+
+
+def test_strategy_context_cancel_all_orders_confirmed_keeps_partial_reply_inconclusive(monkeypatch):
+    calls = {"open_orders": 0}
+
+    def fake_cancel_all(account_id, client_id=None):
+        calls["cancel_all"] = {"account_id": account_id, "client_id": client_id}
+        return {
+            "ok": True,
+            "cancelled": [
+                {"ok": True, "order_id": "o1"},
+                {"ok": False, "order_id": "o2", "status": "deferred", "error": "auth breaker active"},
+            ],
+        }
+
+    def fake_open_orders(account_id, client_id=None):
+        calls["open_orders"] += 1
+        return {"orders": []}
+
+    monkeypatch.setattr("src.trader_mcp.strategy_context.cancel_all_orders", fake_cancel_all)
+    monkeypatch.setattr("src.trader_mcp.strategy_context.list_open_orders", fake_open_orders)
+
+    ctx = StrategyContext(id="inst-1", account_id="acct-1", client_id="client-1", mode="active")
+    result = ctx.cancel_all_orders_confirmed()
+
+    assert result["conclusive"] is False
+    assert result["cleanup_status"] == "cleanup_partial"
+    assert result["cancelled_order_ids"] == ["o1"]
+    assert result["remaining_orders"] == []
+    assert result["error"] == "cancel-all reported uncancelled orders"
+    assert calls["cancel_all"] == {"account_id": "acct-1", "client_id": "client-1"}
+    assert calls["open_orders"] == 1
+
+
 def test_stop_loss_strategy_loads_with_aligned_regime_config(tmp_path):
     original_db = strategy_store.DB_PATH
     original_dir = strategy_registry.STRATEGIES_DIR
@@ -584,6 +666,9 @@ def test_grid_recenters_exactly_on_live_price():
         def cancel_all_orders(self):
             return {"ok": True}
 
+        def cancel_all_orders_confirmed(self):
+            return {"conclusive": True, "error": None, "cleanup_status": "cleanup_confirmed", "cancelled_order_ids": []}
+
         def get_fee_rates(self, market):
             return {"maker": 0.0, "taker": 0.0}
 
@@ -601,6 +686,53 @@ def test_grid_recenters_exactly_on_live_price():
     assert strategy.state["center_price"] == 160.0
 
 
+def test_grid_recenter_preserves_tracked_orders_when_cancel_is_inconclusive():
+    class FakeContext:
+        base_currency = "XRP"
+        counter_currency = "USD"
+        market_symbol = "xrpusd"
+        minimum_order_value = 10.0
+        mode = "active"
+
+        def __init__(self):
+            self.placed_orders = []
+
+        def cancel_all_orders_confirmed(self):
+            return {
+                "conclusive": False,
+                "error": "auth breaker active",
+                "cleanup_status": "cleanup_partial",
+                "cancelled_order_ids": ["o1"],
+            }
+
+        def get_fee_rates(self, market):
+            return {"maker": 0.0, "taker": 0.0}
+
+        def suggest_order_amount(self, **kwargs):
+            return 0.1
+
+        def place_order(self, **kwargs):
+            self.placed_orders.append(kwargs)
+            return {"status": "ok", "id": "oid-1"}
+
+    ctx = FakeContext()
+    strategy = GridStrategy(ctx, {})
+    strategy.state["center_price"] = 100.0
+    strategy.state["orders"] = [{"id": "o1"}, {"id": "o2"}]
+    strategy.state["order_ids"] = ["o1", "o2"]
+    strategy.state["open_order_count"] = 2
+
+    strategy._recenter_and_rebuild_from_price(160.0, "test recenter")
+
+    assert strategy.state["center_price"] == 100.0
+    assert strategy.state["orders"] == [{"id": "o2"}]
+    assert strategy.state["order_ids"] == ["o2"]
+    assert strategy.state["open_order_count"] == 1
+    assert strategy.state["cleanup_status"] == "cleanup_partial"
+    assert strategy.state["last_action"] == "test recenter cleanup pending"
+    assert ctx.placed_orders == []
+
+
 def test_grid_stop_cancels_all_open_orders():
     class FakeContext:
         base_currency = "XRP"
@@ -616,6 +748,10 @@ def test_grid_stop_cancels_all_open_orders():
             self.cancelled = True
             return {"ok": True}
 
+        def cancel_all_orders_confirmed(self):
+            self.cancelled = True
+            return {"conclusive": True, "error": None, "cleanup_status": "cleanup_confirmed", "cancelled_order_ids": ["o1"]}
+
         def get_fee_rates(self, market):
             return {"maker": 0.0, "taker": 0.0}
 
@@ -628,9 +764,90 @@ def test_grid_stop_cancels_all_open_orders():
 
     assert strategy.context.cancelled is True
     assert strategy.state["open_order_count"] == 0
+    assert strategy.state["cleanup_status"] == "cleanup_confirmed"
     assert strategy.state["last_action"] == "stopped"
 
 
+def test_grid_stop_preserves_tracked_orders_when_cancel_is_inconclusive():
+    class FakeContext:
+        base_currency = "XRP"
+        counter_currency = "USD"
+        market_symbol = "xrpusd"
+        minimum_order_value = 10.0
+        mode = "active"
+
+        def cancel_all_orders_confirmed(self):
+            return {
+                "conclusive": False,
+                "error": "auth breaker active",
+                "cleanup_status": "cleanup_failed",
+                "cancelled_order_ids": [],
+            }
+
+        def get_fee_rates(self, market):
+            return {"maker": 0.0, "taker": 0.0}
+
+    strategy = GridStrategy(FakeContext(), {})
+    strategy.state["orders"] = [{"id": "o1"}]
+    strategy.state["order_ids"] = ["o1"]
+    strategy.state["open_order_count"] = 1
+
+    strategy.on_stop()
+
+    assert strategy.state["orders"] == [{"id": "o1"}]
+    assert strategy.state["order_ids"] == ["o1"]
+    assert strategy.state["open_order_count"] == 1
+    assert strategy.state["cleanup_status"] == "cleanup_failed"
+    assert strategy.state["last_action"] == "stop cleanup pending"
+    assert strategy.state["last_error"] == "auth breaker active"
+
+
+def test_grid_observe_mode_preserves_tracked_orders_when_cancel_is_inconclusive(monkeypatch):
+    class FakeContext:
+        base_currency = "XRP"
+        counter_currency = "USD"
+        market_symbol = "xrpusd"
+        minimum_order_value = 10.0
+        mode = "observe"
+
+        def cancel_all_orders_confirmed(self):
+            return {
+                "conclusive": False,
+                "error": "auth breaker active",
+                "cleanup_status": "cleanup_failed",
+                "cancelled_order_ids": [],
+            }
+
+        def get_fee_rates(self, market):
+            return {"maker": 0.0, "taker": 0.0}
+
+        def get_account_info(self):
+            return {"balances": [{"asset_code": "USD", "available": 50.0}, {"asset_code": "XRP", "available": 5.0}]}
+
+    strategy = GridStrategy(FakeContext(), {})
+    strategy.state["center_price"] = 1.42
+    strategy.state["seeded"] = True
+    strategy.state["orders"] = [{"id": "o1", "side": "buy", "price": 1.4, "amount": 10.0}]
+    strategy.state["order_ids"] = ["o1"]
+    strategy.state["open_order_count"] = 1
+
+    monkeypatch.setattr(strategy, "_price", lambda: 1.42)
+    monkeypatch.setattr(strategy, "_refresh_regimes", lambda: None)
+    monkeypatch.setattr(strategy, "_refresh_balance_snapshot", lambda: True)
+    monkeypatch.setattr(strategy, "_sync_open_orders_state", lambda: [{"id": "o1", "side": "buy", "price": 1.4, "amount": 10.0}])
+
+    result = strategy.on_tick({})
+
+    assert result["action"] == "observe"
+    assert result["cleanup_pending"] is True
+    assert strategy.state["orders"] == [{"id": "o1", "side": "buy", "price": 1.4, "amount": 10.0}]
+    assert strategy.state["order_ids"] == ["o1"]
+    assert strategy.state["open_order_count"] == 1
+    assert strategy.state["cleanup_status"] == "cleanup_failed"
+    assert strategy.state["last_action"] == "observe cleanup pending"
+    assert strategy.state["last_error"] == "auth breaker active"
+
+
 def test_base_strategy_report_uses_context_snapshot():
     class FakeContext:
         id = "s-1"