diff --git a/docs/fiqh-basis.md b/docs/fiqh-basis.md index 9122de72..c0eefa6e 100644 --- a/docs/fiqh-basis.md +++ b/docs/fiqh-basis.md @@ -306,23 +306,26 @@ Stated, not hidden — each is a place where keel's encoded behaviour could be w Whether "no underlying purpose" is disqualifying "is exactly the kind of judgement the screen defers to a human" (`docs/experiments/2026-07-20-candidate-universe.md`) — deferred, not decided. -- **The venue boundary is closed by machinery that can decline to answer (#666).** This entry - was written when the boundary was open, and #667/#668 have since closed the three paths that - were named here — the SELL is clamped to the venue's holding, rail 21 refuses an order - against an empty one, and `sweep_orphan_brackets` cancels a protective order whose position - has left. What is *not* closed is the assumption every one of them rests on. All three fail - OPEN on an unreadable balance, deliberately and correctly: refusing a SELL over a balance - endpoint that went quiet would strand positions that wanted out. So a venue that stops - answering does not produce a refusal, it produces the old behaviour — an exit sized from - keel's own ledger. - - That is survivable **only while the account cannot go short**, and keel does not check that - it cannot. `verify_cash_account` exists on the Alpaca adapter alone; Coinbase has no - equivalent read, and #666 must first establish whether Advanced Trade exposes an account - posture at all — an operator attestation, on rail 17's pattern, is a legitimate answer if it - does not. Until then the layered defence is sound whenever the venue answers, and rests on an - unverified premise whenever it does not. Named here rather than left to be inferred from - three closed issues. +- **Nothing can affirm that the account cannot go short (#666).** The three paths that could + oversell are closed — the SELL is clamped to the venue's holding, rail 21 refuses an order + against an empty one, `sweep_orphan_brackets` cancels a protective order whose position has + left — and since 2026-09-02 the coinbase adapter refuses to build a broker on an account + holding an INTX (perpetuals) portfolio, alongside the Alpaca adapter's own posture check. + + What none of that establishes is the affirmative. A probe of the live account settled why: + Coinbase exposes **no cash-versus-margin field for spot**. Every margin, borrow, leverage and + liquidation field in its SDK lives in the futures, perpetuals or derivative-order types, and + `margin_rate` on the transaction summary is present-and-NULL — it is in the response schema + for every account, so its presence signals nothing. The check can therefore **refute a cash + posture and never issue one**, which is the same shape as rail 17: silence is not evidence of + possession, and here silence is not evidence of a cash account either. + + So the layered defence is sound whenever the venue contradicts itself, and rests on the + operator's own knowledge whenever it does not. That residual is a human attestation this + repository has not yet built — the second half of the #233 pattern, where venue evidence can + refute an attestation but cannot manufacture one. Until it exists, "this account cannot go + short" is something the operator knows and keel does not. + - **ZEC and the rest of the deferrals.** The candidate-universe record lists the open questions the attestation step has to answer and "which this agent must not answer". diff --git a/keel/commands/_common.py b/keel/commands/_common.py index 26ce6b66..c6589fe1 100644 --- a/keel/commands/_common.py +++ b/keel/commands/_common.py @@ -258,6 +258,13 @@ def _build_broker(config: Config, *, timeout: int | None = None) -> Any: the real (network-free at construction) Alpaca and Coinbase classes by `tests/test_paper_equities_profile.py`. + **BOTH branches verify the account posture before returning, on different evidence and + with different confidence.** Alpaca reads `multiplier`, which IS the venue's own + classification, so it fails CLOSED on an unreadable one. Coinbase has no such field (#666): + it refuses an INTX perpetuals portfolio and otherwise passes, recording "no contradiction + found" rather than proof -- and passing on an unreadable response, because a check that can + only refute learns nothing from silence. The asymmetry is a property of the venues. + **The alpaca branch verifies the account posture before returning** (#372): one `verify_cash_account()` read of the venue's own account classification, refusing a margin-postured account (and failing closed on an unreadable one) so no engine path @@ -290,7 +297,16 @@ def _build_broker(config: Config, *, timeout: int | None = None) -> Any: ) # The registry-resolved adapter, not a hand-imported client -- the same # conformance-tested class every other venue resolves through. - return adapter_cls(transport) + broker = adapter_cls(transport) + # Cash-account posture (#666), the coinbase half of what the alpaca branch does below + # -- and REFUTE-ONLY, which the alpaca one is not. Coinbase exposes no cash-vs-margin + # field for spot, so this refuses an account holding an INTX (perpetuals) portfolio and + # otherwise records "no contradiction found", never proof of a cash posture. It passes + # on an unreadable response for that reason: failing closed would refuse a compliant + # deployment on a network blip while establishing nothing. One `get_portfolios` read + # per build. See `CoinbaseAdapter.verify_cash_account` for the probe that settled it. + broker.verify_cash_account() + return broker if module_root != "keel_broker_alpaca": raise RuntimeError( diff --git a/packages/keel-broker-coinbase/keel_broker_coinbase/adapter.py b/packages/keel-broker-coinbase/keel_broker_coinbase/adapter.py index 9522ba62..ff4c1217 100644 --- a/packages/keel-broker-coinbase/keel_broker_coinbase/adapter.py +++ b/packages/keel-broker-coinbase/keel_broker_coinbase/adapter.py @@ -12,6 +12,7 @@ from __future__ import annotations +import logging import time from decimal import Decimal from typing import Any @@ -35,6 +36,7 @@ Preview, SessionState, ) +from keel_core.telemetry import log_event, log_venue_failure from keel_core.types import Candle, Granularity from keel_broker_coinbase.translate import to_order_configuration @@ -132,6 +134,24 @@ class of order errors, and `place_order` maps those to `PlaceResult(success=Fals return body +#: Coinbase's international/perpetuals portfolio type. `DEFAULT` and `CONSUMER` are the spot +#: portfolios a probe of a live spot-only account returned on 2026-09-02; `INTX` is the one that +#: means derivatives are available on this account. +logger = logging.getLogger(__name__) + +_DERIVATIVE_PORTFOLIO_TYPES = frozenset({"INTX"}) + + +class CashAccountRequired(RuntimeError): + """This account has derivative capability keel is not scoped to (#666). + + Named to match `keel_broker_alpaca.CashAccountRequired` because `_build_broker` treats both + the same way and an operator meeting one should recognise the other. The two are raised on + DIFFERENT evidence and with different confidence -- see `verify_cash_account` -- and the + difference is a property of the venues, not of the engine. + """ + + class CoinbaseAdapter: """Implements the `Broker` port against Coinbase Advanced Trade.""" @@ -347,6 +367,60 @@ def place_order(self, spec: OrderSpec, *, idempotency_key: str | None = None) -> reason = _field(error_response, "message") or _field(error_response, "error") return PlaceResult(success=False, broker_order_id=None, reason=reason) + def verify_cash_account(self) -> None: + """Refuse an account with derivative capability. **Refutes only; never proves (#666).** + + Coinbase exposes NO cash-vs-margin field for spot. A probe of the live account on + 2026-09-02 established what it does expose: every margin/borrow/leverage/liquidation + field in the SDK lives in `futures_types`, `perpetuals_types` or the derivative order + fields, and `margin_rate` on the transaction summary is present-and-NULL -- it is in the + response schema for every account, so its presence signals nothing at all. + + What IS unambiguous is the portfolio list. `DEFAULT` and `CONSUMER` are the spot + portfolios; `INTX` is the international/perpetuals one, and its presence is derivative + capability on this account. That is the one signal strong enough to refuse on. + + **A pass means NO CONTRADICTION WAS FOUND -- it is never proof of a cash posture, and a + future reader must not take it as one.** There is no affirmative flag to read, so the + residual unknown stays with the operator's attestation on rail 17's pattern: venue + evidence can refute an attestation, it cannot issue one. + + ⚠️ **Passes on an unreadable response, which is the OPPOSITE of + `keel_broker_alpaca.verify_cash_account` and is deliberate.** Alpaca fails closed + because `multiplier` IS the classification, so a readable answer is definitive and + silence is a distinct third state worth refusing on. Here the check can only refute, so + an unreadable response proves nothing a readable one would not also have failed to + prove -- and failing closed would refuse a compliant deployment on a network blip while + establishing nothing. A gate that fires on the compliant case is the gate that gets + disabled in anger. + + One `get_portfolios` request per broker construction, alongside Alpaca's one + `/v2/account` read, well inside the venue's rate budget at this cadence. + """ + try: + response = self._require_transport().get_portfolios() + portfolios = _field(response, "portfolios", []) or [] + found = sorted( + { + str(_field(p, "type", "") or "").upper() + for p in portfolios + if str(_field(p, "type", "") or "").upper() in _DERIVATIVE_PORTFOLIO_TYPES + } + ) + except Exception: + # Every failure is the same answer: no contradiction found. See the docstring -- + # this must not become a refusal, and it must not raise into broker construction. + log_venue_failure(logger, "coinbase.portfolio_posture_unreadable") + return + if found: + raise CashAccountRequired( + f"this Coinbase account holds a {', '.join(found)} portfolio -- derivatives are " + "available on it, and keel is spot-only by charter (rails 18/19). Trade from an " + "account without one, or remove the portfolio. keel cannot verify the reverse: " + "Coinbase exposes no cash-account flag for spot, so the absence of this " + "portfolio is not proof of a cash posture and never will be." + ) + def get_fee_summary(self) -> FeeSummary: """Map Coinbase's `transaction_summary` to a `FeeSummary`. @@ -356,6 +430,24 @@ def get_fee_summary(self) -> FeeSummary: The subscription spec's §10 tracks confirming this against a live account. """ response = self._require_transport().get_transaction_summary() + # #666: surfaced HERE rather than in `verify_cash_account`, because this call already + # fetches the response -- so the warning costs no extra request. A WARNING and not a + # refusal: `margin_rate` is present-and-null on a spot-only account, so presence is not + # a signal, and a NON-null value is plausibly one but could not be verified against a + # margin-enabled account. Shipping an untested refusal branch on a compliance gate is + # how a gate ends up firing on the compliant case. + if _field(response, "margin_rate") is not None: + log_event( + logger, + logging.WARNING, + "coinbase.margin_rate_present", + detail=( + "this account's transaction summary carries a non-null `margin_rate`, " + "which may mean margin is available on it. keel does not refuse on this " + "because the signal is unverified -- confirm the account's posture and " + "your rail 17 attestation" + ), + ) fee_tier = _field(response, "fee_tier") or {} return FeeSummary( venue="coinbase", diff --git a/packages/keel-broker-coinbase/keel_broker_coinbase/transport.py b/packages/keel-broker-coinbase/keel_broker_coinbase/transport.py index bd83e8af..62e01750 100644 --- a/packages/keel-broker-coinbase/keel_broker_coinbase/transport.py +++ b/packages/keel-broker-coinbase/keel_broker_coinbase/transport.py @@ -27,6 +27,12 @@ def get_products(self, product_type: str = "SPOT", **kwargs: Any) -> Any: ... def get_accounts(self, **kwargs: Any) -> Any: ... + #: The portfolio list, for the cash-account posture check (#666). Declared here rather than + #: called off an untyped client, because this Protocol IS the adapter's statement of what it + #: needs from a transport -- a method reached without declaring it is a dependency the test + #: fakes are not obliged to satisfy and mypy cannot see. + def get_portfolios(self, **kwargs: Any) -> Any: ... + def preview_order( self, product_id: str, side: str, order_configuration: dict[str, Any], **kwargs: Any ) -> Any: ... diff --git a/tests/broker_coinbase/test_adapter.py b/tests/broker_coinbase/test_adapter.py index 98014cb6..31a8a3c6 100644 --- a/tests/broker_coinbase/test_adapter.py +++ b/tests/broker_coinbase/test_adapter.py @@ -8,6 +8,7 @@ from __future__ import annotations import json +import logging from decimal import Decimal from pathlib import Path from typing import Any @@ -25,6 +26,7 @@ SessionState, ) from keel_broker_coinbase import CoinbaseAdapter +from keel_broker_coinbase.adapter import CashAccountRequired from keel_core.types import Candle, Granularity, Side FIXTURES_DIR = Path(__file__).parent.parent / "fixtures" @@ -688,3 +690,116 @@ def test_declares_no_credential_defect_hook() -> None: opaque pair with no locally-provable shape to check, so the `getattr(..., None)` default is what the readiness derivation actually reads here, not a stub that always says "fine".""" assert getattr(CoinbaseAdapter, "credential_defect", None) is None + + +# -- cash-account posture, refute-only (#666) ----------------------------------------------------- +# +# Coinbase exposes NO cash-vs-margin field for spot. A probe against the live account on +# 2026-09-02 established what it does expose: `margin_rate` is present-and-NULL (it is in the +# response schema for every account, so its presence signals nothing), and portfolio `type` is +# `DEFAULT`/`CONSUMER` for spot with `INTX` the international/perpetuals one. +# +# So this check can only ever REFUTE. It never grants a cash posture, and the residual unknown +# stays with an operator attestation on rail 17's pattern. + + +class _PortfolioTransport(FakeTransport): + """A transport whose portfolio list the test chooses.""" + + def __init__(self, types: list[str], *, raises: Exception | None = None) -> None: + super().__init__() + self._types = types + self._raises = raises + + def get_portfolios(self, **kwargs: Any) -> Any: + if self._raises is not None: + raise self._raises + return { + "portfolios": [ + {"name": f"p{i}", "uuid": f"u{i}", "type": t} + for i, t in enumerate(self._types) + ] + } + + +def test_a_spot_only_account_passes() -> None: + """DEFAULT and CONSUMER are the spot portfolios — exactly what the live account returned.""" + CoinbaseAdapter(_PortfolioTransport(["DEFAULT", "CONSUMER"])).verify_cash_account() + + +def test_an_intx_portfolio_refuses_the_broker() -> None: + """**The one unambiguous signal.** INTX is Coinbase's international/perpetuals portfolio; + its presence is derivative capability on the account, and a spot-only engine must not build + a broker against it.""" + adapter = CoinbaseAdapter(_PortfolioTransport(["DEFAULT", "INTX"])) + + with pytest.raises(CashAccountRequired, match="INTX"): + adapter.verify_cash_account() + + +def test_the_refusal_is_case_insensitive() -> None: + """The venue's casing is not a contract keel should depend on for a compliance refusal.""" + with pytest.raises(CashAccountRequired): + CoinbaseAdapter(_PortfolioTransport(["intx"])).verify_cash_account() + + +def test_an_unreadable_portfolio_list_PASSES_and_that_is_deliberate() -> None: + """⚠️ The opposite of `keel-broker-alpaca`, and the asymmetry is the whole design. + + Alpaca fails CLOSED on an unreadable classification because `multiplier` IS the answer — a + readable response is definitive, so silence is a distinct third state worth refusing on. + + Coinbase has no such field. This check can only refute, so an unreadable response proves + nothing that a readable one would not also have failed to prove: "no contradiction found" + is the same answer either way. Failing closed here would refuse a compliant deployment on a + network blip while establishing nothing — and a gate that fires on the compliant case is + the gate that gets disabled in anger. + """ + adapter = CoinbaseAdapter(_PortfolioTransport([], raises=RuntimeError("venue unreachable"))) + + adapter.verify_cash_account() # must not raise + + +def test_a_malformed_portfolio_response_passes() -> None: + """Same reasoning: a shape keel does not recognise is not evidence of derivatives.""" + class _Malformed(FakeTransport): + def get_portfolios(self, **kwargs: Any) -> Any: + return {"unexpected": "shape"} + + CoinbaseAdapter(_Malformed()).verify_cash_account() + + +def test_passing_is_recorded_as_no_contradiction_never_as_proof() -> None: + """The docstring is load-bearing: a future reader must not take a pass here as evidence the + account is cash-only. Coinbase exposes no affirmative flag, so nothing here can grant one — + that stays with the operator attestation.""" + doc = CoinbaseAdapter.verify_cash_account.__doc__ or "" + assert "no contradiction" in doc.lower() + assert "never" in doc.lower() and "prov" in doc.lower() + + +def test_a_non_null_margin_rate_warns_without_refusing(caplog) -> None: + """`margin_rate` is present-and-NULL on a spot-only account, so PRESENCE is not a signal. + + A non-null value plausibly is one — but there is no margin-enabled account to verify that + against, so the refusal branch would ship untested. It is surfaced as a warning instead, + on the fee read that ALREADY fetches this response, so it costs no extra request. + """ + class _MarginRate(FakeTransport): + def get_transaction_summary(self, **kwargs: Any) -> Any: + base = super().get_transaction_summary(**kwargs) or {} + return {**dict(base), "margin_rate": {"value": "0.05"}} + + with caplog.at_level(logging.WARNING): + CoinbaseAdapter(_MarginRate()).get_fee_summary() + + assert any("margin_rate" in r.getMessage() for r in caplog.records) + + +def test_a_null_margin_rate_is_silent(caplog) -> None: + """The live account carries `margin_rate: null`. Warning on that would fire on every + compliant deployment, which is the failure mode this whole design avoids.""" + with caplog.at_level(logging.WARNING): + CoinbaseAdapter(FakeTransport()).get_fee_summary() + + assert not [r for r in caplog.records if "margin_rate" in r.getMessage()] diff --git a/tests/commands/test_coinbase_posture_wiring.py b/tests/commands/test_coinbase_posture_wiring.py new file mode 100644 index 00000000..58b5d0e7 --- /dev/null +++ b/tests/commands/test_coinbase_posture_wiring.py @@ -0,0 +1,86 @@ +"""`_build_broker` calls the coinbase posture check, not just the alpaca one (#666). + +The adapter having a correct `verify_cash_account` proves nothing about anything calling it — +a compliance gate wired to nothing is a gate that exists only in a docstring. Three separate +sessions this week shipped a helper whose call site was unpinned and whose removal left every +other test green; this file is the call site's pin. +""" + +from __future__ import annotations + +from typing import Any + +import pytest + +from keel.commands import _common + + +class _Adapter: + """A coinbase-shaped adapter that records whether the seam asked about its posture.""" + + __module__ = "keel_broker_coinbase.adapter" + + def __init__(self, transport: Any, refuse: Exception | None = None) -> None: + self.transport = transport + self._refuse = refuse + self.verified = False + + def verify_cash_account(self) -> None: + self.verified = True + if self._refuse is not None: + raise self._refuse + + +def _build(monkeypatch, refuse: Exception | None = None): + """Patch the two seams the coinbase branch reaches for: the entry-point registry (imported + INSIDE `_build_broker`, so it must be patched on its own module) and the SDK client it + constructs from `.env` secrets.""" + import coinbase.rest + from keel_broker_api import registry + + holder: dict[str, _Adapter] = {} + + def factory(transport: Any, **_kwargs: Any) -> _Adapter: + holder["adapter"] = _Adapter(transport, refuse) + return holder["adapter"] + + factory.__module__ = "keel_broker_coinbase.adapter" + monkeypatch.setattr(registry, "load_broker", lambda _venue: factory) + monkeypatch.setattr(coinbase.rest, "RESTClient", lambda **_kwargs: object()) + monkeypatch.setattr("keel.config.load_secrets", lambda: {"api_key": "k", "api_secret": "s"}) + return holder + + +def test_the_seam_verifies_the_coinbase_posture(monkeypatch) -> None: + """One `get_portfolios` read per build, beside Alpaca's one `/v2/account` read — the same + place, for the same reason, so no command can build a broker that skipped it.""" + holder = _build(monkeypatch) + + _common._build_broker(_config()) + + assert holder["adapter"].verified is True, ( + "_build_broker built a coinbase broker without asking about its account posture" + ) + + +def test_a_refusal_propagates_out_of_the_build(monkeypatch) -> None: + """Refusing at BUILD, not at order time: guards are broker-less by design, and a per-order + raise could fire on an exit path where a refusal traps a position.""" + from keel_broker_coinbase.adapter import CashAccountRequired + + _build(monkeypatch, refuse=CashAccountRequired("INTX portfolio")) + + with pytest.raises(CashAccountRequired, match="INTX"): + _common._build_broker(_config()) + + +def _config() -> Any: + class _Broker: + name = "coinbase" + endpoint = None + data_feed = None + + class _Config: + broker = _Broker() + + return _Config() diff --git a/tests/test_fiqh_basis.py b/tests/test_fiqh_basis.py index 50b7b965..4e568fd3 100644 --- a/tests/test_fiqh_basis.py +++ b/tests/test_fiqh_basis.py @@ -390,7 +390,9 @@ def test_the_readme_links_the_document(): _CASH_POSTURE_CHECK = "verify_cash_account" -_VENUE_BOUNDARY_PREMISE = "rests on an unverified premise whenever it does not" +#: The affirmative Coinbase does not expose. A probe of the live account on 2026-09-02 found +#: `margin_rate` present-and-NULL and no cash-versus-margin field anywhere in the spot surface. +_NO_AFFIRMATIVE = "refute a cash posture and never issue one" def test_the_long_only_ruling_is_pinned_two_sided_to_the_code_that_enforces_it(): @@ -463,36 +465,42 @@ def test_the_hadith_reference_is_marked_as_outside_the_knowledge_base(): ) -def test_the_venue_boundary_premise_is_stated_not_hidden(): - """The boundary is closed by machinery that fails OPEN, over an account nobody checked. +def test_the_unprovable_half_of_the_cash_posture_is_stated_not_hidden(): + """The venue can contradict a cash posture. It cannot confirm one, and the doc must say so. - This test's first form pinned `_sell_base_size`'s docstring as a proxy for "the SELL is not - clamped", on the theory that #667 landing would break it and force the doc to be updated - with the code. It did not: #667 clamped at intent construction and left that docstring - intact, so the pin held while the paragraph it guarded became false. **A proxy is only a pin - while the thing it stands for and the thing it matches move together**, and this one stopped. + This test has been re-pointed twice, and both moves are the record of a premise changing + rather than a sentence being reworded. - Re-anchored to the fact that actually remains open. `_clamp_to_held`, rail 21 and - `sweep_orphan_brackets` all fail open on an unreadable balance -- correctly, since refusing - a SELL over a quiet endpoint strands positions that wanted out -- so the defence rests on - the account being unable to go short, and nothing verifies that on the venue that trades - live. When #666 gives the coinbase adapter a posture read, this test fails, and it fails - because the premise changed rather than because a sentence was reworded. + Its first form pinned `_sell_base_size`'s docstring as a proxy for "the SELL is not clamped", + expecting #667 to break it. #667 clamped at intent construction and left that docstring + standing, so the pin held while the paragraph it guarded went false. + + Its second form pinned the coinbase adapter having NO posture read, expecting #666 to break + it. #666 did — and the test failing is what forced this rewrite, which is exactly what it + was for. + + What is pinned now is the thing that cannot be engineered away: Coinbase exposes no + cash-versus-margin field for spot, so the check REFUTES and never issues. Closing that needs + a human attestation, not another adapter read — and if one is ever built, this fails again. """ doc = _unwrapped(_doc()) - assert _VENUE_BOUNDARY_PREMISE in doc, ( - f"{_DOC} must state that the long-only defence rests on an unverified premise when the " - "venue does not answer -- a layered defence described without its assumption reads as " - "settled, and the assumption is the part still open" + assert _NO_AFFIRMATIVE in doc, ( + f"{_DOC} must state that the venue check can only REFUTE a cash posture. A reader who " + "takes a passing check as proof has the guarantee backwards, and that is the one " + "misreading this section exists to prevent" ) - assert "#666" in doc, f"{_DOC} must name #666 as the open venue-boundary work" + assert "#666" in doc, f"{_DOC} must name the issue the residual belongs to" + coinbase = "packages/keel-broker-coinbase/keel_broker_coinbase/adapter.py" - assert _CASH_POSTURE_CHECK not in _rel(coinbase), ( - f"{coinbase} now has a {_CASH_POSTURE_CHECK!r} read -- #666 has landed and the open " - f"question in {_DOC} is stale. Update the doc, then re-point this test." + source = _rel(coinbase) + assert _CASH_POSTURE_CHECK in source, ( + f"{coinbase} no longer has a posture check; the doc says it refuses an INTX portfolio" + ) + assert "no contradiction" in source.lower(), ( + f"{coinbase}'s check must record a pass as NO CONTRADICTION FOUND rather than as proof " + "-- the distinction is the whole of what the probe established" ) alpaca = "packages/keel-broker-alpaca/keel_broker_alpaca/adapter.py" assert _CASH_POSTURE_CHECK in _rel(alpaca), ( - f"{_DOC} says the posture check exists on the Alpaca adapter alone; {alpaca} must still " - "carry it, or the doc is describing a check no adapter has" + f"{alpaca} must still carry its own posture check -- the doc says both venues have one" )