From afd6ac60919167528506c36d23398434c9ac51a1 Mon Sep 17 00:00:00 2001 From: Builder Date: Sun, 13 Sep 2026 13:56:38 +0800 Subject: [PATCH] fix(api): refuse to start a second trading engine (BUG-04) `POST /api/trading/start` built and started a new engine unconditionally. `_live_engine` was only ever overwritten: engine = LiveTradingEngine(...) engine.start() _live_engine = engine # replaces, never checks A second call therefore orphaned the first engine: - its TradingScheduler thread and its broker kept running - no handle remained, so `POST /api/trading/stop` could never reach it - both engines wrote positions / P&L / orders into the same store, and `positions` is keyed on `code` alone, so their rows overwrote each other - its kill switch became unreachable too, because `_core_risk` was rebound `LiveTradingEngine.start()` has its own `if self._running: return` guard, but that is per-instance and cannot see the new object. Realistic trigger: a double-clicked button, two browser tabs, or a script -- anything that sends the request twice before the first response returns. Now returns 409 with an explicit message, and the guard keys on the engine's `_running` flag rather than on the global merely being set, so a stopped engine does not block a restart. Tests (tests/test_api/test_trading_start_guard.py): - a second start while one is running -> 409 - a stopped engine does not block a restart - repeated attempts stay refused Verified by removing the guard: the first and third fail there, the second still passes (it is the one asserting the guard does not over-block). 1326 passed, 1 skipped. Co-Authored-By: Claude Opus 4.8 (1M context) --- api/routes.py | 18 +++++ tests/test_api/test_trading_start_guard.py | 79 ++++++++++++++++++++++ 2 files changed, 97 insertions(+) create mode 100644 tests/test_api/test_trading_start_guard.py diff --git a/api/routes.py b/api/routes.py index 2101e49..ec9b31f 100644 --- a/api/routes.py +++ b/api/routes.py @@ -2333,6 +2333,24 @@ async def trading_start(req: dict): global _live_engine, _core_sm, _core_risk + # Refuse to start a second engine. + # + # `_live_engine` is only ever overwritten, so a second start orphaned the + # first: its scheduler thread and broker kept running with no handle left + # to stop them, both engines wrote positions/P&L/orders into the same + # store, and the orphan's kill switch became unreachable because + # `_core_risk` was rebound too. LiveTradingEngine.start()'s own + # `if self._running: return` guard is per-instance and cannot help. + existing = _get_live_engine() + if existing is not None and getattr(existing, "_running", False): + raise HTTPException( + status_code=409, + detail=( + "A trading engine is already running. Call POST /api/trading/stop " + "before starting another." + ), + ) + broker_type = req.get("broker", "simulated") initial_cash = req.get("initial_cash", 1_000_000) n_stocks = req.get("n_stocks", 30) diff --git a/tests/test_api/test_trading_start_guard.py b/tests/test_api/test_trading_start_guard.py new file mode 100644 index 0000000..adefe23 --- /dev/null +++ b/tests/test_api/test_trading_start_guard.py @@ -0,0 +1,79 @@ +"""Regression tests for the trading-engine start guard (BUG-04). + +`POST /api/trading/start` used to build and start a new engine unconditionally; +`_live_engine` was only ever overwritten. A second start therefore orphaned the +first engine -- its scheduler thread and broker kept running with no handle left +to stop them, both engines wrote positions/P&L/orders into the same store, and +the orphan's kill switch became unreachable because `_core_risk` was rebound +too. `LiveTradingEngine.start()`'s own `if self._running: return` guard is +per-instance and cannot prevent this. +""" + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from quant_platform.api import routes + + +class _FakeEngine: + def __init__(self, running): + self._running = running + + +@pytest.fixture +def client(): + app = FastAPI() + app.include_router(routes.router) + return TestClient(app) + + +def test_a_second_start_is_refused_while_one_is_running(client, monkeypatch): + monkeypatch.setattr(routes, "_live_engine", _FakeEngine(running=True)) + + resp = client.post("/api/trading/start", json={"broker": "simulated"}) + + assert resp.status_code == 409 + assert "already running" in resp.json()["detail"] + + +def test_a_stopped_engine_does_not_block_a_restart(client, monkeypatch): + """The guard keys on the engine's running flag, not on the global merely + being set -- otherwise a stopped engine could never be restarted.""" + monkeypatch.setattr(routes, "_live_engine", _FakeEngine(running=False)) + + started = {} + + class _StubEngine: + def __init__(self, **kwargs): + self._running = False + + def set_universe(self, codes): + started["universe"] = codes + + def start(self): + self._running = True + started["started"] = True + + import quant_platform.trading.engine as engine_module + import quant_platform.trading.realtime as realtime_module + + monkeypatch.setattr(engine_module, "LiveTradingEngine", _StubEngine) + + def _no_market(): + raise RuntimeError("no network in tests") + + monkeypatch.setattr(realtime_module, "RealTimeMarket", _no_market) + + resp = client.post("/api/trading/start", json={"broker": "simulated"}) + + assert resp.status_code == 200, resp.text + assert started.get("started") is True + + +def test_the_orphaned_engine_is_the_one_the_guard_protects(client, monkeypatch): + """Starting twice must not leave two engines started.""" + monkeypatch.setattr(routes, "_live_engine", _FakeEngine(running=True)) + + for _ in range(3): + assert client.post("/api/trading/start", json={}).status_code == 409