From 584e08eb36beaedb055043e8fccb58482ec3cb1c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 1 Sep 2026 13:21:20 +0000 Subject: [PATCH] fix(execution): bind parent row to request identity before child evaluation; add parent mismatch and race tests Co-authored-by: genichin <4282227+genichin@users.noreply.github.com> --- tests/adapters/broker/test_kis_cancel.py | 118 +++++++++++++++++++++ tests/application/test_execution_cancel.py | 66 ++++++++++++ trex_core/adapters/broker/kis.py | 76 ++++++++++--- 3 files changed, 245 insertions(+), 15 deletions(-) diff --git a/tests/adapters/broker/test_kis_cancel.py b/tests/adapters/broker/test_kis_cancel.py index 7ae0cc1..15e70b5 100644 --- a/tests/adapters/broker/test_kis_cancel.py +++ b/tests/adapters/broker/test_kis_cancel.py @@ -391,3 +391,121 @@ def test_ambiguous_multiple_child_cancel_rows_return_unknown(monkeypatch): assert fs.state == "unknown", f"expected unknown (ambiguous children), got {fs.state}" raw = fs.raw if isinstance(fs.raw, dict) else {} assert raw.get("reason") == "child_cancel_ambiguous" + + +# ── Regression: parent row identity mismatch with request ──────────────────── + +def test_parent_row_wrong_branch_returns_unknown(monkeypatch): + """Parent row with wrong ord_gno_brno (branch) relative to broker_order_id → unknown.""" + parent = { + "odno": ODNO, + "orgn_odno": "", + "ord_qty": "10", + "tot_ccld_qty": "0", + "avg_prvs": "0", + "cncl_yn": "", + "rjct_qty": "0", + "ord_dt": "20260901", + "ord_gno_brno": "99999", # wrong branch — request has 06010 + "pdno": "225190", + } + monkeypatch.setattr( + kis_mod.httpx, "get", _fake_get({"rt_cd": "0", "output1": [parent]}) + ) + # broker_order_id encodes branch 06010, but row has 99999 + fs = _broker().poll_fill( + f"06010:{ODNO}", side="buy", symbol="225190", order_date="20260901" + ) + assert fs.state == "unknown", f"expected unknown (wrong parent branch), got {fs.state}" + raw = fs.raw if isinstance(fs.raw, dict) else {} + assert "mismatch" in raw.get("reason", "") + + +def test_parent_row_wrong_date_returns_unknown(monkeypatch): + """Parent row with wrong ord_dt relative to order_date arg → unknown.""" + parent = { + "odno": ODNO, + "orgn_odno": "", + "ord_qty": "10", + "tot_ccld_qty": "0", + "avg_prvs": "0", + "cncl_yn": "", + "rjct_qty": "0", + "ord_dt": "20260831", # wrong date — request has 20260901 + "ord_gno_brno": "06010", + "pdno": "225190", + } + monkeypatch.setattr( + kis_mod.httpx, "get", _fake_get({"rt_cd": "0", "output1": [parent]}) + ) + fs = _broker().poll_fill( + f"06010:{ODNO}", side="buy", symbol="225190", order_date="20260901" + ) + assert fs.state == "unknown", f"expected unknown (wrong parent date), got {fs.state}" + raw = fs.raw if isinstance(fs.raw, dict) else {} + assert "mismatch" in raw.get("reason", "") + + +def test_parent_row_wrong_symbol_returns_unknown(monkeypatch): + """Parent row with wrong pdno (symbol) relative to symbol arg → unknown.""" + parent = { + "odno": ODNO, + "orgn_odno": "", + "ord_qty": "10", + "tot_ccld_qty": "0", + "avg_prvs": "0", + "cncl_yn": "", + "rjct_qty": "0", + "ord_dt": "20260901", + "ord_gno_brno": "06010", + "pdno": "000660", # wrong symbol — request has 225190 + } + monkeypatch.setattr( + kis_mod.httpx, "get", _fake_get({"rt_cd": "0", "output1": [parent]}) + ) + fs = _broker().poll_fill( + f"06010:{ODNO}", side="buy", symbol="225190", order_date="20260901" + ) + assert fs.state == "unknown", f"expected unknown (wrong parent symbol), got {fs.state}" + raw = fs.raw if isinstance(fs.raw, dict) else {} + assert "mismatch" in raw.get("reason", "") + + +def test_correction_child_wrong_identity_is_not_treated_as_correction(monkeypatch): + """Correction child with wrong branch/date/symbol must not trigger correction_child_present_unresolved.""" + parent = { + "odno": ODNO, + "orgn_odno": "", + "ord_qty": "10", + "tot_ccld_qty": "0", + "avg_prvs": "0", + "cncl_yn": "", + "rjct_qty": "0", + "ord_dt": "20260901", + "ord_gno_brno": "06010", + "pdno": "225190", + } + unrelated_correction = { + "odno": "0000199990", + "orgn_odno": ODNO, + "ord_qty": "10", + "tot_ccld_qty": "0", + "avg_prvs": "0", + "cncl_yn": "N", + "rjct_qty": "0", + "ord_dt": "20260831", # different date → not related to our request + "ord_gno_brno": "99999", # different branch + "pdno": "000660", # different symbol + } + monkeypatch.setattr( + kis_mod.httpx, + "get", + _fake_get({"rt_cd": "0", "output1": [parent, unrelated_correction]}), + ) + fs = _broker().poll_fill( + f"06010:{ODNO}", side="buy", symbol="225190", order_date="20260901" + ) + # Unrelated correction must not cause correction_child_present_unresolved + assert fs.state == "accepted", f"expected accepted (unrelated correction), got {fs.state}" + raw = fs.raw if isinstance(fs.raw, dict) else {} + assert raw.get("reason") != "correction_child_present_unresolved" diff --git a/tests/application/test_execution_cancel.py b/tests/application/test_execution_cancel.py index 1b7a7f2..e606db3 100644 --- a/tests/application/test_execution_cancel.py +++ b/tests/application/test_execution_cancel.py @@ -225,3 +225,69 @@ def test_timeout_cancel_first_attempt_with_retry_before_none(): ) execution.run_once() # should trigger timeout cancel assert broker.cancel_calls == 1 + + +# ── Regression: concurrent timeout/operator cancel race — broker call count 1 ─ + +def test_timeout_operator_race_broker_called_exactly_once(): + """Timeout cancel loop and operator cancel_pending_slot race to the same first + reservation. The repository CAS guarantees exactly one winner; broker must + receive cancel_order exactly once regardless of scheduling order. + + Uses a threading barrier so both paths enter reserve_cancel_attempt + simultaneously, making the race deterministic. + """ + import threading + + broker = ProgrammableBroker() + repo, _, intents, execution, _, _ = _wire(broker) + sid = _buy_to_holding(intents, execution) + intents.submit_sell(SellIntent(intent_id="s1", slot_id=sid)) + broker.responder = lambda side: FillStatus( + state="accepted", + raw={"order_open_confirmed": True}, + ) + execution.run_once() # dispatch sell + + # Backdate so timeout_cancel fires in the next run_once() + s = repo.fetch_slot(sid) + repo._slots[sid] = replace( + s, + order_submitted_at=datetime.now(tz=timezone.utc) - timedelta(seconds=120), + order_date="20260901", + timeout_sec=30, + ) + + barrier = threading.Barrier(2) + original_reserve = repo.reserve_cancel_attempt + + def _barrier_reserve(slot_id, broker_order_id, *, retry_before=None): + barrier.wait(timeout=5) + return original_reserve(slot_id, broker_order_id, retry_before=retry_before) + + repo.reserve_cancel_attempt = _barrier_reserve + + errors = [] + + def _operator(): + try: + execution.cancel_pending_slot(sid) + except Exception as exc: # noqa: BLE001 + errors.append(exc) + + t = threading.Thread(target=_operator) + t.start() + + # Execution loop's timeout cancel is the second concurrent caller. + try: + execution.run_once() + except Exception as exc: # noqa: BLE001 + errors.append(exc) + + t.join(timeout=5) + assert not errors, f"thread errors: {errors}" + + # CAS ensures exactly one winner — broker cancel must be called once. + assert broker.cancel_calls == 1, ( + f"expected exactly 1 broker cancel call, got {broker.cancel_calls}" + ) diff --git a/trex_core/adapters/broker/kis.py b/trex_core/adapters/broker/kis.py index eea23c2..d5a7182 100644 --- a/trex_core/adapters/broker/kis.py +++ b/trex_core/adapters/broker/kis.py @@ -413,6 +413,11 @@ class KISBroker: self._fill_poll_not_before[broker_order_id] = monotonic() + _FILL_POLL_MIN_INTERVAL_SECONDS rows = payload.get("output1") or [] normalized_odno = odno.lstrip("0") or "0" + # Canonical request identity from the caller — these are authoritative + # and must not be overridden by whatever the response row contains. + request_brno = broker_order_id.split(":")[0] if ":" in broker_order_id else "" + request_date = inquiry_date # == order_date + request_symbol = symbol row = next( ( r @@ -440,40 +445,81 @@ class KISBroker: "inquiry_date": inquiry_date, }, ) + # Validate that the matched parent row's identity agrees with the + # authoritative request identity. KIS can return stale rows for + # reused odno values; a branch, date, or symbol mismatch means this + # row does not belong to our order → fail-closed as unknown. + row_brno = str(row.get("ord_gno_brno") or "").strip() + row_dt = str(row.get("ord_dt") or "").strip() + row_pdno = str(row.get("pdno") or "").strip() + if request_brno and row_brno and row_brno != request_brno: + return FillStatus( + state="unknown", + raw={ + "reason": "parent_identity_mismatch_brno", + "order_visible": True, + "inquiry_date": inquiry_date, + }, + ) + if row_dt and row_dt != request_date: + return FillStatus( + state="unknown", + raw={ + "reason": "parent_identity_mismatch_date", + "order_visible": True, + "inquiry_date": inquiry_date, + }, + ) + if row_pdno and row_pdno != request_symbol: + return FillStatus( + state="unknown", + raw={ + "reason": "parent_identity_mismatch_symbol", + "order_visible": True, + "inquiry_date": inquiry_date, + }, + ) # dcy-013 ext: KIS sometimes records an external cancel as a separate # child row with orgn_odno= and cncl_yn="Y" rather than # setting cncl_yn on the original row. Scan sibling rows to detect # this case. # # Safety requirements: - # - Child must share the parent's canonical identity: - # ord_gno_brno (branch), ord_dt (date), pdno (symbol). - # Mismatched or ambiguous children → fail-closed as "unknown". - # - Correction (정정) children have cncl_yn != "Y"; they are not - # treated as cancels here. If a correction child is present with - # a new odno the evidence is ambiguous → "unknown" so the slot is - # not incorrectly unlocked. - parent_brno = str(row.get("ord_gno_brno") or "").strip() - parent_dt = str(row.get("ord_dt") or "").strip() - parent_pdno = str(row.get("pdno") or "").strip() + # - Child must share the AUTHORITATIVE request identity (branch, date, + # symbol from the caller), NOT the row values. Using row values as + # ground truth would let a mismatched row pass its own wrong values + # back as the filter reference. + # - Ambiguous (> 1 matching cancel) → fail-closed "unknown". + # - Correction (정정) children (cncl_yn != "Y") that match the same + # request identity are also fail-closed → "unknown". + # - Unrelated rows that only share orgn_odno but differ in branch/date/ + # symbol are excluded from both candidate lists. + def _child_matches_request(r: dict) -> bool: + """True when a child row is bound to the same order identity as the request.""" + child_brno = str(r.get("ord_gno_brno") or "").strip() + child_dt = str(r.get("ord_dt") or "").strip() + child_pdno = str(r.get("pdno") or "").strip() + if request_brno and child_brno and child_brno != request_brno: + return False + if child_dt and child_dt != request_date: + return False + return not (child_pdno and child_pdno != request_symbol) child_cancel_candidates = [ r for r in rows if (str(r.get("orgn_odno") or "").strip().lstrip("0") or "0") == normalized_odno and str(r.get("cncl_yn") or "").strip().upper() == "Y" - and str(r.get("ord_gno_brno") or "").strip() == parent_brno - and str(r.get("ord_dt") or "").strip() == parent_dt - and str(r.get("pdno") or "").strip() == parent_pdno + and _child_matches_request(r) ] - # Correction children: same orgn_odno but cncl_yn != "Y". If present, - # the situation is ambiguous — a replacement order may be live. + # Correction children: same orgn_odno + matching identity but cncl_yn != "Y". correction_candidates = [ r for r in rows if (str(r.get("orgn_odno") or "").strip().lstrip("0") or "0") == normalized_odno and str(r.get("cncl_yn") or "").strip().upper() != "Y" + and _child_matches_request(r) ] if len(child_cancel_candidates) > 1: