fix(execution): cancel a resting SELL whose position has left the account (#668)

docs/fiqh-basis.md

@@ -182,6 +182,14 @@ refused BUY costs nothing; a refused SELL strands a position that wanted out. An
 balance is not evidence the position is gone, and this is the one place in the engine where
 "unknown is a rejection" would do more harm than the hole it closes.
 
+Beside the rail, one sweep with the same warrant and no number: `sweep_orphan_brackets`
+(#668) cancels a resting SELL whose position has left the account. It is not a rail because it
+runs on the reconciliation pass rather than before an order, and it is order-driven where every
+other sweep is position-driven -- a protective order whose position is gone has no tranche to be
+found from. The trigger is the venue's own statement, never keel's ledger: cancelling a stop
+over a position that really exists strips a live holding of its only protection, and the ledger
+can be stale in exactly that direction.
+
 What remains open at the venue boundary is #666: on a cash account every case above is a
 rejected order rather than a short, and keel has no cash-account posture check on Coinbase —
 `verify_cash_account` exists only on the Alpaca adapter.

keel/agent.py

@@ -1648,6 +1648,16 @@ def run_once(
         # row, so the pass above cannot see it (issue #195).
         reconcile.reconcile_unbracketed_positions(broker, repo, config, now_ts)
 
+        # Then cancel any resting SELL the account can no longer honour (#668). LAST of the
+        # three, and the order is load-bearing: the pass above heals a tranche that has no
+        # bracket, and for a tranche whose position left the account out of band it will place
+        # one -- against nothing. Sweeping afterwards makes this the cycle's final word, so
+        # nothing re-creates within the same cycle what it just cancelled. (#667's rail 21 stops
+        # that re-place at source, which makes the churn rare rather than making the order
+        # optional: the rail refuses to PLACE into an empty holding, and this cancels what is
+        # already resting. Neither subsumes the other.)
+        reconcile.sweep_orphan_brackets(broker, repo, now_ts)
+
         # Rail 11's inputs, refreshed BEFORE any entry this cycle. `poll_once` above has already
         # written the candles, so every product's latest price is readable here -- and it has to
         # be done now, not after the loop: `guards.check` reads these scalars from inside

keel/commands/doctor.py

@@ -813,6 +813,45 @@ def balance_drift_findings(records: dict[str, Any]) -> list[Finding]:
     ]
 
 
+def orphan_bracket_findings(records: dict[str, Any]) -> list[Finding]:
+    """Resting SELLs the orphan sweep cancelled because the account no longer held them (#668).
+
+    The cancel already resolved the ORDER -- there is nothing left at the venue to trigger
+    against nothing. What it did not resolve is why keel was protecting a position the account
+    says is gone, and that question outlives the order: the tranche is still open in the ledger,
+    deliberately, because closing it would book a realized outcome at a price nobody observed.
+
+    WARN. The sweep did the safe thing and did it automatically; this is the record that it had
+    to, which is a state a human should look at once rather than an ongoing fault.
+    """
+    orphans = sorted((p, r) for p, r in records.items() if isinstance(r, dict))
+    if not orphans:
+        return [
+            Finding(
+                "bracket.orphan",
+                OK,
+                "no orphaned protective orders",
+                "every resting SELL stands against a position the venue confirms",
+                "-",
+            )
+        ]
+    described = ", ".join(
+        f"{product}: order {record.get('order_id')} cancelled, venue held {record.get('held')}"
+        for product, record in orphans
+    )
+    return [
+        Finding(
+            "bracket.orphan",
+            WARN,
+            f"{len(orphans)} product(s) had a protective order with nothing behind it",
+            f"{described} -- cancelled before the market could trigger it; the tranche is "
+            "still open in the ledger",
+            "reconcile the position: an out-of-band sale or transfer, or an exit that "
+            "already executed at the venue",
+        )
+    ]
+
+
 def unbooked_exit_findings(
     open_positions: list[dict[str, Any]], orders: list[dict[str, Any]]
 ) -> list[Finding]:
@@ -985,6 +1024,7 @@ def gather_findings(repo: Any, config: Any, log_lines: Iterable[str], now_ts: in
     from keel.commands._products import _default_sim_products
     from keel.execution import executor as executor_mod
     from keel.execution import guards
+    from keel.execution import reconcile as reconcile_mod
 
     findings: list[Finding] = []
 
@@ -1037,6 +1077,12 @@ def gather_findings(repo: Any, config: Any, log_lines: Iterable[str], now_ts: in
             for key in repo.get_state_keys(executor_mod.BALANCE_DRIFT_PREFIX)
         }
     )
+    findings += orphan_bracket_findings(
+        {
+            key[len(reconcile_mod.ORPHAN_BRACKET_PREFIX) :]: repo.get_state(key)
+            for key in repo.get_state_keys(reconcile_mod.ORPHAN_BRACKET_PREFIX)
+        }
+    )
     # #639: modes are POOLED here, unlike the partial-fill sweep above -- the ledger
     # invariant belongs to `agent._open_tranche`, which writes it for paper and live alike.
     findings += unbooked_exit_findings(repo.get_open_positions(), repo.get_orders())

keel/execution/reconcile.py

@@ -29,6 +29,14 @@
 recorded before this change, which is the gap itself. That rarity changes the priority of
 auto-remediation, not the validity of recognizing the state.
 
+ORPHANED PROTECTIVE ORDERS (#668) are swept here too, and by a different kind of pass. Every
+other reconciliation in this module and in `executor` is POSITION-DRIVEN: it starts from a
+tranche and asks what protects it. `sweep_orphan_brackets` runs the other way -- it starts from
+keel's own resting SELLs and asks whether the account still holds what each one stands over --
+because a protective order whose position has left the account has no tranche to be walked from,
+and nothing else in the engine can reach it. Left alone, the market reverses through the stop and
+the venue is asked to sell an asset that is not there.
+
 What is deliberately NOT done here: resizing or amending the bracket when a partially-filled
 entry leaves it oversized for what is held. The port migration is done and the
 cancel-and-replace is expressible (`BracketGTC` since #569; `executor._roll_stop` performs
@@ -316,6 +324,149 @@ def reconcile_unbracketed_positions(
     return healed
 
 
+#: `agent_state` key prefix for a resting SELL this sweep cancelled because the account no
+#: longer held the product (#668). Read by `keel doctor`: the cancel resolves the ORDER, but the
+#: divergence that produced it -- keel holding a protective order for something the venue says is
+#: gone -- is a fact about the deployment that outlives the order and nobody else reports.
+ORPHAN_BRACKET_PREFIX = "orphan_bracket:"
+
+
+def sweep_orphan_brackets(broker: Any, repo: Repository, now_ts: int) -> list[int]:
+    """Cancel every resting SELL for a product the venue says the account no longer holds (#668).
+
+    **The gap this closes is structural, not incidental.** `_clear_resting_bracket` fails closed
+    before an exit, and `reconcile_unbracketed_positions` heals a position with no bracket. Both
+    are POSITION-DRIVEN -- they start from a tranche and ask what protects it. A resting SELL
+    whose position is gone has no tranche to be walked from, so neither can reach it, and nothing
+    else looks.
+
+    How one appears, since the shape is not obvious on a venue with native brackets:
+
+    * the operator sells or transfers the asset at the venue themselves -- keel's bracket keeps
+      resting against inventory that left without telling it;
+    * `scale_out` and `_roll_stop` both cancel before they place, and a process that dies inside
+      that window can leave a replacement resting with the position already resolved;
+    * a venue without a native trigger-bracket needs two legs, and a filled target then leaves a
+      live stop behind (the classic spot-OCO collision -- not reachable on the venue that trades
+      live today, where `BracketGTC` is one order, and reachable the moment one is not).
+
+    The market then reverses through the stop and the venue sells an asset that is not held. On a
+    cash account that is a rejection; on a margin-enabled one (#666) it is a short, produced
+    entirely by a missing cancel -- *bay' ma la yamlik* with nobody in the loop.
+
+    **The VENUE decides, never the ledger, and that asymmetry is a safety property.** Cancelling
+    a protective order on a position that really exists strips a live holding of its stop, so the
+    trigger must be the account's own statement about itself. keel's ledger can be stale in the
+    dangerous direction -- it says zero while the venue holds the position -- and a sweep driven
+    from it would cancel exactly the brackets that were doing their job. An unreadable balance
+    therefore cancels NOTHING; the sweep simply did not run for that product this cycle.
+
+    `Balance.total`, not `Balance.available`, for the same reason as #667's clamp: a resting SELL
+    holds the base it commits, so `available` reads ~0 for precisely the products this sweep is
+    looking at, and cancelling on it would cancel every bracket keel has ever placed.
+
+    Dust below the venue's own `base_increment` counts as nothing held -- a residue the venue
+    cannot even express as a size is not a position, and a bracket protecting it is an orphan
+    with a rounding error attached.
+
+    Returns the local ids it cancelled. **Never raises into the cycle**: an unreachable venue
+    means the sweep did not run, which is the honest state, and the next cycle retries. The
+    tranche is deliberately LEFT ALONE -- closing it would book a realized outcome at a price
+    nobody observed, and this function's warrant covers the order, not the ledger.
+    """
+    cancelled: list[int] = []
+    rows = [r for r in _polled_rows(repo) if str(r["side"]).upper() == Side.SELL.value.upper()]
+    if not rows:
+        return cancelled
+
+    # One balance read per PRODUCT, not per row: a product with two resting legs asks the same
+    # question twice otherwise, and this runs on every cycle.
+    held_by_product: dict[str, Decimal | None] = {}
+    for row in rows:
+        product_id = row["product_id"]
+        if product_id not in held_by_product:
+            held_by_product[product_id] = _orphan_threshold_held(broker, repo, product_id, now_ts)
+        held = held_by_product[product_id]
+        if held is None:
+            continue
+
+        try:
+            executor._cancel_at_exchange(broker, repo, row)
+        except executor.CancelPending:
+            # The venue took it and settles asynchronously. Not a failure and not a retry:
+            # `reconcile_open_orders` reads the terminal state next cycle, and this sweep will
+            # see the row gone from `_polled_rows` when it does.
+            log_event(
+                logger,
+                logging.INFO,
+                "reconcile.orphan_cancel_pending",
+                product=product_id,
+                order_id=row["id"],
+            )
+            continue
+        except Exception:
+            # Includes `CancelUnavailable` -- an order the venue refuses to cancel may be one it
+            # already filled, which is a fact the next status poll will settle. Per-row
+            # isolation, like every other venue call in this module: one refusal must not
+            # abandon the remaining orphans.
+            log_exception(
+                logger,
+                "reconcile.orphan_cancel_failed",
+                product=product_id,
+                order_id=row["id"],
+            )
+            continue
+
+        repo.update_order(row["id"], status="canceled", updated_at=now_ts)
+        repo.set_state(
+            f"{ORPHAN_BRACKET_PREFIX}{product_id}",
+            {"order_id": row["id"], "held": str(held), "cancelled_at": now_ts},
+        )
+        cancelled.append(row["id"])
+        log_event(
+            logger,
+            logging.WARNING,
+            "reconcile.orphan_bracket_cancelled",
+            product=product_id,
+            order_id=row["id"],
+            held=str(held),
+            detail=(
+                "a resting SELL was protecting a position the venue reports the account no "
+                "longer holds -- cancelled before the market could trigger it against nothing. "
+                "The tranche is left open deliberately: closing it would book an outcome at a "
+                "price nobody observed"
+            ),
+        )
+
+    return cancelled
+
+
+def _orphan_threshold_held(
+    broker: Any, repo: Repository, product_id: str, now_ts: int
+) -> Decimal | None:
+    """The venue's holding when it is small enough to orphan a bracket, else `None`.
+
+    `None` is the DO-NOTHING answer and covers two different situations on purpose: the balance
+    could not be read (so nothing may be cancelled), and the account holds a real position (so
+    nothing should be). Collapsing them is safe precisely because the action is the same and the
+    dangerous mistake -- cancelling a live position's only stop -- is impossible from either.
+    """
+    held = executor._held_base(broker, product_id)
+    if held is None:
+        return None
+    increment = executor._base_increment_for(broker, repo, product_id, now_ts)
+    if increment is not None and increment > 0:
+        # Dust the venue cannot express as a size is not a position. Strictly BELOW the
+        # increment: a holding of exactly one increment is the smallest real position there is,
+        # and a bracket over it is doing its job.
+        return held if held < increment else None
+    # No increment known, so there is no dust threshold to apply and none may be invented --
+    # only an affirmative nothing counts. `_base_increment_for` returns None for a venue error
+    # as well as for a genuinely unknown product, and guessing a floor from either would cancel
+    # protective orders over a number nobody supplied.
+    return held if held <= 0 else None
+
+
 def _has_resting_bracket(repo: Repository, position: dict[str, Any]) -> bool:
     """Whether this tranche still has a bracket working at the exchange."""
     bracket_id = position.get("bracket_order_id")

tests/commands/test_doctor.py

@@ -29,6 +29,7 @@
     doctor_exit_code,
     doctor_lines,
     gather_findings,
+    orphan_bracket_findings,
     partial_fill_findings,
     rail_state_findings,
     render_json,
@@ -582,6 +583,7 @@ def test_gather_findings_covers_every_check_over_a_seeded_db(tmp_path, valid_con
         "veto.recent",
         "fill.partial",
         "balance.drift",
+        "bracket.orphan",
         "ledger.unbooked_exit",
         "data.missing",
         "data.stale",
@@ -834,3 +836,50 @@ def test_gather_findings_surfaces_a_recorded_drift(tmp_path, valid_config_path)
     (drift,) = [f for f in findings if f.name == "balance.drift"]
     assert drift.status == "warn"
     assert "BTC-USD" in drift.detail
+
+
+# -- orphaned protective orders (#668) -----------------------------------------------------------
+
+
+def test_orphan_bracket_findings_is_ok_when_none_were_swept() -> None:
+    (finding,) = orphan_bracket_findings({})
+
+    assert finding.name == "bracket.orphan"
+    assert finding.status == "ok"
+
+
+def test_orphan_bracket_findings_warns_and_says_the_tranche_is_still_open() -> None:
+    """The cancel resolved the order. The ledger row it stood over is deliberately untouched.
+
+    An operator reading only "cancelled" would assume the position was closed out; it was not,
+    and the detail has to say so or the finding is misleading in the direction that matters.
+    """
+    (finding,) = orphan_bracket_findings(
+        {"BTC-USD": {"order_id": 7, "held": "0", "cancelled_at": NOW}}
+    )
+
+    assert finding.status == "warn"
+    assert "still open in the ledger" in finding.detail
+
+
+def test_orphan_bracket_findings_ignores_a_malformed_record() -> None:
+    (finding,) = orphan_bracket_findings({"BTC-USD": "not a record"})
+
+    assert finding.status == "ok"
+
+
+def test_gather_findings_surfaces_a_swept_orphan(tmp_path, valid_config_path) -> None:
+    """The wiring, keyed on the prefix the sweep actually writes."""
+    from keel.execution.reconcile import ORPHAN_BRACKET_PREFIX
+
+    repo = _seeded_repo(tmp_path / "keel.db")
+    repo.set_state(
+        f"{ORPHAN_BRACKET_PREFIX}BTC-USD", {"order_id": 7, "held": "0", "cancelled_at": NOW}
+    )
+    config = load_config(valid_config_path)
+
+    findings = gather_findings(repo, config, [], NOW)
+
+    (orphan,) = [f for f in findings if f.name == "bracket.orphan"]
+    assert orphan.status == "warn"
+    assert "BTC-USD" in orphan.detail