(fix) adversarial-review fixes: validate after registration, await the exchange call

- Register the pair BEFORE validating supported_position_modes: bybit's
  implementation depends on the registered pair list and returns both
  modes vacuously when it is empty, so pre-registration validation
  fake-accepted HEDGE for an inverse pair and then silently did nothing —
  recreating the exact lie this PR removes. Unknown pairs are rejected 400
  and never registered (validation from the base of the stack).
- Await _execute_set_position_mode instead of the fire-and-forget
  connector.set_position_mode (spawns a background task, cannot report
  rejection, e.g. Binance -4068 with open positions), and verify the local
  trait afterwards — 502 when the exchange did not accept the switch.
- Retry position-mode adoption once on first pair registration: bitget's
  _fetch_account_position_mode returns None with an empty pair list, so
  adoption at connector init was dead code for it.
- Guard adoption with supported_position_modes (future-proofing) and
  correct the narrative in comments/docstrings: with empty pairs the old
  init call left base-class connectors at local ONEWAY but vacuously
  flipped bybit/bitget local state to HEDGE — in every case without any
  exchange call.

flake8 skipped: pre-existing violations in touched files on main.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: fengtality

services/perpetual_trading_service.py

@@ -104,42 +104,65 @@ async def set_position_mode(self, account_name: str, connector_name: str,
         """
         connector = await self._get_perpetual_connector(account_name, connector_name)
 
-        # Check if the requested position mode is supported
-        supported_modes = connector.supported_position_modes()
-        if position_mode not in supported_modes:
-            supported_values = [mode.value for mode in supported_modes]
-            raise HTTPException(
-                status_code=400,
-                detail=f"Position mode '{position_mode.value}' not supported. Supported modes: {supported_values}"
-            )
-
-        # Position-mode implementations apply the switch through the connector's
-        # trading pairs (the py-base default and e.g. bybit both log a warning and
-        # return when the list is empty), and API connectors are created with
-        # trading_pairs=[] — so without a registered pair the exchange is never
-        # called while this endpoint would report success. Register the provided
-        # pair, then refuse to proceed with an empty list rather than lie.
+        # Register the provided pair FIRST. Position-mode implementations apply
+        # the switch through the connector's trading pairs (with an empty list the
+        # base implementation warns and returns; bybit/bitget overrides vacuously
+        # "succeed" without any exchange call), and supported_position_modes() on
+        # e.g. bybit depends on the registered pair list — validating against it
+        # before registration would vacuously pass modes the actual pair cannot
+        # support. Unknown pairs are rejected with 400 and never registered.
         if trading_pair:
             from services.unified_connector_service import UnifiedConnectorService
-            await UnifiedConnectorService.sync_pair_derived_state(connector, trading_pair)
+            try:
+                await UnifiedConnectorService.sync_pair_derived_state(
+                    connector, trading_pair, refresh_rules=False)
+            except ValueError as e:
+                raise HTTPException(status_code=400, detail=str(e))
         if not getattr(connector, "trading_pairs", None):
             raise HTTPException(
                 status_code=400,
                 detail=f"No trading pairs registered on {connector_name}; pass trading_pair "
                        f"so the position mode switch can be applied on the exchange"
             )
 
+        # Validate AFTER registration, against the real pair set
+        supported_modes = connector.supported_position_modes()
+        if position_mode not in supported_modes:
+            supported_values = [mode.value for mode in supported_modes]
+            raise HTTPException(
+                status_code=400,
+                detail=f"Position mode '{position_mode.value}' not supported. Supported modes: {supported_values}"
+            )
+
         try:
-            # Try to call the method - it might be sync or async
-            result = connector.set_position_mode(position_mode)
-            # If it's a coroutine, await it
-            if asyncio.iscoroutine(result):
-                await result
+            # Await the actual exchange call. connector.set_position_mode() is
+            # fire-and-forget (it spawns _execute_set_position_mode in the
+            # background and returns immediately), which would report success
+            # before the exchange ever responds — and cannot report a rejection
+            # (e.g. Binance -4068 with open positions). _execute_set_position_mode
+            # updates the local trait only on confirmed success, so the local mode
+            # is the truth test.
+            execute = getattr(connector, "_execute_set_position_mode", None)
+            if execute is not None:
+                await execute(position_mode)
+            else:
+                result = connector.set_position_mode(position_mode)
+                if asyncio.iscoroutine(result):
+                    await result
+
+            if getattr(connector, "position_mode", position_mode) != position_mode:
+                raise HTTPException(
+                    status_code=502,
+                    detail=f"Exchange did not accept position mode {position_mode.value} on "
+                           f"{connector_name} — check for open positions/orders and connector logs"
+                )
 
             message = f"Position mode set to {position_mode.value} on {connector_name}"
             logger.info(f"Set position mode to {position_mode.value} on {connector_name} (Account: {account_name})")
             return {"status": "success", "message": message}
 
+        except HTTPException:
+            raise
         except Exception as e:
             logger.error(f"Failed to set position mode to {position_mode.value}: {e}")
             raise HTTPException(status_code=500, detail=f"Failed to set position mode: {str(e)}")

test/test_sync_pair_derived_state.py

@@ -249,6 +249,10 @@ def __init__(self):
         def trading_pairs(self):
             return self._trading_pairs or []
 
+        @property
+        def position_mode(self):
+            return self.mode_set
+
         def supported_position_modes(self):
             return [PositionMode.HEDGE, PositionMode.ONEWAY]
 
@@ -270,14 +274,113 @@ async def provider(account_name, connector_name):
         assert e.status_code == 400
     assert connector.mode_set is None
 
-    # Pair provided -> registered (with rules/throttler synced), then switched
+    # Pair provided -> registered (with throttler synced), then switched
     result = _run(service.set_position_mode(
         "master", "bybit_perpetual", PositionMode.HEDGE, trading_pair="BTC-USDT"))
     assert connector._trading_pairs == ["BTC-USDT"]
     assert connector.mode_set == PositionMode.HEDGE
     assert result["status"] == "success"
 
 
+def test_set_position_mode_validates_against_post_registration_pair_set():
+    """supported_position_modes() on bybit depends on the registered pair list
+    and returns BOTH modes vacuously when it is empty. Validating before
+    registration would fake-accept HEDGE for an inverse pair; the mode check
+    must run against the post-registration pair set."""
+    from fastapi import HTTPException
+    from hummingbot.core.data_type.common import PositionMode
+
+    from services.perpetual_trading_service import PerpetualTradingService
+
+    class BybitLikePerp(FakePairLimitsConnector):
+        INVERSE = {"BTC-USD"}
+
+        def __init__(self):
+            super().__init__(trading_pairs=[])
+            self.mode_set = None
+
+        @property
+        def trading_pairs(self):
+            return self._trading_pairs or []
+
+        @property
+        def position_mode(self):
+            return self.mode_set
+
+        def supported_position_modes(self):
+            # Faithful to bybit: vacuous both-modes on empty; inverse pairs
+            # restrict to ONEWAY.
+            if not self.trading_pairs:
+                return [PositionMode.ONEWAY, PositionMode.HEDGE]
+            if any(p in self.INVERSE for p in self.trading_pairs):
+                return [PositionMode.ONEWAY]
+            return [PositionMode.ONEWAY, PositionMode.HEDGE]
+
+        def set_position_mode(self, mode):
+            self.mode_set = mode
+
+    connector = BybitLikePerp()
+
+    async def provider(account_name, connector_name):
+        return connector
+
+    service = PerpetualTradingService(provider)
+
+    # HEDGE + inverse pair: pre-fix this vacuously passed validation and then
+    # silently did nothing; now the mode check runs after registration -> 400.
+    try:
+        _run(service.set_position_mode(
+            "master", "bybit_perpetual", PositionMode.HEDGE, trading_pair="BTC-USD"))
+        raise AssertionError("expected HTTPException for unsupported mode")
+    except HTTPException as e:
+        assert e.status_code == 400
+        assert "not supported" in e.detail
+    assert connector.mode_set is None  # exchange never touched
+
+
+def test_set_position_mode_surfaces_exchange_rejection():
+    """The endpoint must await the real exchange call and fail loudly when the
+    exchange rejects the switch (e.g. open positions) — previously the call was
+    fire-and-forget and always reported success."""
+    from fastapi import HTTPException
+    from hummingbot.core.data_type.common import PositionMode
+
+    from services.perpetual_trading_service import PerpetualTradingService
+
+    class RejectingPerp(FakePairLimitsConnector):
+        def __init__(self):
+            super().__init__(trading_pairs=["BTC-USDT"])
+            self.execute_calls = 0
+
+        @property
+        def trading_pairs(self):
+            return self._trading_pairs or []
+
+        @property
+        def position_mode(self):
+            return PositionMode.ONEWAY  # local trait never updated: rejection
+
+        def supported_position_modes(self):
+            return [PositionMode.ONEWAY, PositionMode.HEDGE]
+
+        async def _execute_set_position_mode(self, mode):
+            self.execute_calls += 1  # exchange rejected; trait stays ONEWAY
+
+    connector = RejectingPerp()
+
+    async def provider(account_name, connector_name):
+        return connector
+
+    service = PerpetualTradingService(provider)
+
+    try:
+        _run(service.set_position_mode("master", "bybit_perpetual", PositionMode.HEDGE))
+        raise AssertionError("expected HTTPException for rejected switch")
+    except HTTPException as e:
+        assert e.status_code == 502
+    assert connector.execute_calls == 1  # the exchange call was actually awaited
+
+
 def test_adopt_exchange_position_mode():
     """At init the exchange is the source of truth for position mode: its current
     mode is mirrored locally (the old HEDGE-forcing call never reached any