From 26368874352ebfe1e2aadc6c47b3844859d6b053 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Sun, 9 Aug 2026 18:08:03 -0400 Subject: [PATCH] feat(tooling): read-only live probe for the robinhood adapter's fixture shapes Every test under `tests/broker_robinhood/` runs against a canned transport, so the suite proves the adapter is internally consistent with fixtures we wrote ourselves. It cannot prove that Robinhood accepts our Ed25519 signature, that the endpoint paths are real, or that the response shapes are real -- and `rh_accounts.json`'s nested `fee_tier_status` drives every number in `get_fee_summary` and every `Preview.est_fee`. `scripts/robinhood_smoke.py` closes that gap with read-only GETs: no order, no money at risk. It reports structural diffs against each committed fixture in both directions, since a key the venue sends and the fixture omits is how a capability gets missed. Read-only by construction: the guard is installed ONTO the transport's `_request` rather than wrapping it. A `__getattr__` wrapper looks correct and is not -- the transport's methods call their own `self._request` and sail straight past it, so the guarantee would hold for the tests and not for the probes. A test pins that. Prints shapes, not values, so balances never reach a terminal or a pasted log. Refs #215, #197, #198. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/robinhood_smoke.py | 288 ++++++++++++++++++++++++++ tests/scripts/__init__.py | 0 tests/scripts/test_robinhood_smoke.py | 212 +++++++++++++++++++ 3 files changed, 500 insertions(+) create mode 100644 scripts/robinhood_smoke.py create mode 100644 tests/scripts/__init__.py create mode 100644 tests/scripts/test_robinhood_smoke.py diff --git a/scripts/robinhood_smoke.py b/scripts/robinhood_smoke.py new file mode 100644 index 00000000..904568d9 --- /dev/null +++ b/scripts/robinhood_smoke.py @@ -0,0 +1,288 @@ +"""Validate the Robinhood adapter's *assumptions* against the live venue, reading only. + +Every test under `tests/broker_robinhood/` runs against a canned transport, so what the suite +proves is that the adapter is internally consistent with fixtures **we wrote ourselves**. Three +things it therefore cannot prove, and which only a real credential can: + +1. that Robinhood accepts our Ed25519 signature at all (the signing tests verify it against our + own verify-key, which is circular with respect to the venue); +2. that the endpoint paths are the real ones -- they are pinned against our *reading* of + https://docs.robinhood.com/crypto/trading/, and PR #194 showed that reading can be wrong in + both directions; +3. that the response *shapes* are real. `tests/fixtures/rh_accounts.json`'s nested + `fee_tier_status` is the load-bearing case: every number in `get_fee_summary` and every + `Preview.est_fee` is derived from a shape nothing outside this repository has corroborated. + +This script closes 1-3 without placing an order and without risking a cent. It is deliberately +NOT a conformance suite and NOT part of the shipped wheel -- it is an operator tool, run by hand +when a credential exists, and its only output is a shape report. + +## Why it cannot place an order + +`_ReadOnly` wraps the transport's request method and raises on any method other than GET, so the +guarantee does not rest on this module merely *declining* to call `create_order`. A future edit +that adds a POST fails loudly here rather than quietly placing something. That matters more than +usual: this is the one script in the repository intended to run against live credentials, and +Robinhood publishes no sandbox, so "live" is the operator's real money. + +## Why it prints shapes, not values + +The question being asked is structural ("does `fee_tier_status` exist, and what keys does it +carry"), so rendering the account's actual balances and holdings would leak private financial +data into a terminal, a CI log, or a pasted bug report to buy nothing. Every leaf is replaced by +its type. `--show-values` exists for the one case where a value IS the answer -- confirming a +`symbol` string's exact spelling, say -- and even then it never prints the credential. + +Usage:: + + uv run python scripts/robinhood_smoke.py # shape report vs the fixtures + uv run python scripts/robinhood_smoke.py --json # machine-readable, for an issue comment + +Requires `ROBINHOOD_API_KEY` and `ROBINHOOD_PRIVATE_KEY` in a git-ignored `.env`. The latter is +the base64 of the raw 32-byte Ed25519 *seed* generated locally -- NOT the base64 public key that +was pasted into Robinhood's credential page. Transposing the two produces a 401 that is +indistinguishable from a signing bug, so this script checks the key's shape before spending a +request on it. +""" + +from __future__ import annotations + +import argparse +import json +import sys +from pathlib import Path +from typing import Any + +from dotenv import dotenv_values + +REPO_ROOT = Path(__file__).resolve().parent.parent +FIXTURES = REPO_ROOT / "tests" / "fixtures" + +#: Read-only probes, in dependency order: `accounts` first because it resolves the account +#: number every later call needs, and because it is the cheapest possible proof that the +#: signature is accepted. +PROBES: tuple[tuple[str, str], ...] = ( + ("accounts", "rh_accounts.json"), + ("trading_pairs", "rh_trading_pairs.json"), + ("best_bid_ask", "rh_best_bid_ask.json"), + ("estimated_price", "rh_estimated_price.json"), + ("holdings", "rh_holdings.json"), +) + +#: The symbol every marketdata probe is run against. BTC-USD is the one pair we can be confident +#: is tradable on any Robinhood Crypto account, so a failure here is a real finding rather than +#: "that account cannot trade that asset". +PROBE_SYMBOL = "BTC-USD" + +#: A raw Ed25519 seed is 32 bytes, which is 44 base64 characters with padding. Checking this +#: before the first request turns the most likely operator error -- pasting the public key, or a +#: PEM, or a hex string -- into a precise message instead of a 401. +_SEED_B64_LEN = 44 + + +class ReadOnlyViolation(RuntimeError): + """Raised when anything in this script attempts a non-GET request.""" + + +class _ReadOnly: + """Installs a GET-only guard onto a `RobinhoodTransport` and records what it issues. + + The refusal is at the REQUEST layer, not the method layer: a method-level allowlist only + holds while nobody adds a method, whereas `_request` is the single choke point every call in + the transport passes through. + + Note the guard is **installed onto the transport instance**, not merely wrapped around it. + Wrapping with `__getattr__` alone is a trap that looks correct and is not: `get_accounts()` + would be forwarded to the wrapped object, whose body then calls its OWN `self._request`, + sailing straight past the wrapper. The guarantee would hold only for calls made directly + against the wrapper -- which is to say, for the tests and not for the probes. Rebinding the + instance attribute is what makes the transport's internal calls route through here too. + """ + + def __init__(self, transport: Any) -> None: + self._transport = transport + self.calls: list[tuple[str, str]] = [] + # Captured BEFORE rebinding, or `_request` below would recurse into itself. + self._inner = transport._request + transport._request = self._request + + def __getattr__(self, name: str) -> Any: + return getattr(self._transport, name) + + def _request(self, method: str, path: str, **kwargs: Any) -> Any: + if method.upper() != "GET": + raise ReadOnlyViolation( + f"robinhood_smoke.py attempted a {method.upper()} to {path!r}. This script is " + f"read-only by construction; it must never place, amend or cancel an order." + ) + self.calls.append((method.upper(), path)) + return self._inner(method, path, **kwargs) + + +def shape_of(value: Any) -> Any: + """Reduce a decoded JSON value to its structure, discarding every leaf. + + A list collapses to a single-element summary rather than one entry per item: the question is + what an element looks like, and a 90-pair `trading_pairs` response would otherwise bury the + answer in 90 identical copies. An empty list is reported as such, since "the venue returned + nothing" is itself a finding -- it is how an unfunded account presents, and it is what would + make a shape comparison vacuously pass. + """ + if isinstance(value, dict): + return {key: shape_of(val) for key, val in sorted(value.items())} + if isinstance(value, list): + if not value: + return [""] + return [shape_of(value[0]), f"... {len(value)} items"] + if value is None: + return "null" + return type(value).__name__ + + +def compare_shapes(live: Any, fixture: Any, path: str = "") -> list[str]: + """Return one human-readable line per structural difference, recursing into dicts. + + Differences are reported in both directions on purpose. A key the fixture invented and the + venue does not send is the dangerous one -- that is a field the adapter may already be + reading -- but a key the venue sends and the fixture omits is how a capability gets missed, + and `fees_usd` (issue #197) is exactly that shape of miss. + """ + diffs: list[str] = [] + if isinstance(fixture, dict) and isinstance(live, dict): + for key in sorted(set(fixture) | set(live)): + where = f"{path}.{key}" if path else key + if key not in live: + diffs.append(f" MISSING AT VENUE {where} (fixture has {fixture[key]!r})") + elif key not in fixture: + diffs.append(f" NEW AT VENUE {where} (venue sends {live[key]!r})") + else: + diffs.extend(compare_shapes(live[key], fixture[key], where)) + return diffs + if isinstance(fixture, list) and isinstance(live, list): + if fixture and live and fixture[0] != "" and live[0] != "": + diffs.extend(compare_shapes(live[0], fixture[0], f"{path}[]")) + return diffs + if live != fixture: + diffs.append(f" TYPE DIFFERS {path or ''} fixture={fixture!r} venue={live!r}") + return diffs + + +def load_credentials(env_path: Path) -> tuple[str, str]: + """Read the credential from `.env`, failing with instructions rather than a stack trace. + + The private key's length is checked here because the overwhelmingly likely operator error -- + pasting the base64 PUBLIC key that Robinhood's credential page asked for -- yields a 401 that + looks exactly like a signing bug, and chasing that costs far more than this check. + """ + values = dotenv_values(env_path) + api_key = (values.get("ROBINHOOD_API_KEY") or "").strip() + private_key = (values.get("ROBINHOOD_PRIVATE_KEY") or "").strip() + + missing = [ + name + for name, val in (("ROBINHOOD_API_KEY", api_key), ("ROBINHOOD_PRIVATE_KEY", private_key)) + if not val + ] + if missing: + raise SystemExit( + f"missing {' and '.join(missing)} in {env_path}.\n\n" + "Robinhood signs EVERY request, including read-only ones, so an API key alone " + "cannot make a single call.\n" + "ROBINHOOD_PRIVATE_KEY is the base64 of the raw 32-byte Ed25519 seed generated " + "locally -- not the base64 public key pasted into Robinhood's credential page.\n" + "See packages/keel-broker-robinhood/README.md for the pynacl snippet." + ) + if len(private_key) != _SEED_B64_LEN: + raise SystemExit( + f"ROBINHOOD_PRIVATE_KEY is {len(private_key)} characters; a base64-encoded 32-byte " + f"Ed25519 seed is {_SEED_B64_LEN}.\n" + "This is almost always the PUBLIC key, a PEM, or a hex string. Sending it would " + "produce a 401 indistinguishable from a signing bug." + ) + return api_key, private_key + + +def run_probes(transport: _ReadOnly, symbol: str) -> dict[str, Any]: + """Run every read-only probe, recording a per-probe result rather than aborting on the first. + + One probe failing is a finding about that endpoint, not a reason to learn nothing about the + other four -- and the first failure is usually the least informative, since a bad credential + fails all of them identically. + """ + results: dict[str, Any] = {} + calls = { + "accounts": lambda: transport.get_accounts(), + "trading_pairs": lambda: transport.get_trading_pairs(), + "best_bid_ask": lambda: transport.get_best_bid_ask(symbol), + "estimated_price": lambda: transport.get_estimated_price(symbol, "ask", "0.001"), + "holdings": lambda: transport.get_holdings(), + } + for name, call in calls.items(): + try: + results[name] = {"ok": True, "shape": shape_of(call())} + except ReadOnlyViolation: + raise + except Exception as exc: # noqa: BLE001 -- a probe report wants the failure, not a trace + results[name] = {"ok": False, "error": f"{type(exc).__name__}: {exc}"} + return results + + +def report(results: dict[str, Any], as_json: bool) -> int: + """Print the shape report and return the process exit code.""" + if as_json: + print(json.dumps(results, indent=2, sort_keys=True)) + return 0 if all(r["ok"] for r in results.values()) else 1 + + failures = 0 + for name, fixture_name in PROBES: + result = results[name] + print(f"\n=== {name} ===") + if not result["ok"]: + print(f" FAILED {result['error']}") + failures += 1 + continue + + fixture_path = FIXTURES / fixture_name + fixture_shape = shape_of(json.loads(fixture_path.read_text())) + diffs = compare_shapes(result["shape"], fixture_shape) + if not diffs: + print(f" shape matches {fixture_name}") + else: + print(f" {len(diffs)} difference(s) vs {fixture_name}:") + for line in diffs: + print(line) + failures += 1 + + print( + f"\n{len(PROBES) - failures}/{len(PROBES)} probes matched their fixture." + if failures + else f"\nall {len(PROBES)} probes matched their fixtures." + ) + return 1 if failures else 0 + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument("--json", action="store_true", help="machine-readable output") + parser.add_argument("--symbol", default=PROBE_SYMBOL, help=f"default {PROBE_SYMBOL}") + parser.add_argument("--env", type=Path, default=REPO_ROOT / ".env") + args = parser.parse_args(argv) + + api_key, private_key = load_credentials(args.env) + + # Imported here, not at module scope: `argparse --help` and the credential error above must + # work in an environment where the optional adapter is not installed. + from keel_broker_robinhood.transport import RobinhoodTransport + + transport = _ReadOnly(RobinhoodTransport(api_key=api_key, private_key_b64=private_key)) + results = run_probes(transport, args.symbol) + + print(f"issued {len(transport.calls)} request(s), all GET:") + for method, path in transport.calls: + print(f" {method} {path}") + + return report(results, args.json) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/scripts/__init__.py b/tests/scripts/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/scripts/test_robinhood_smoke.py b/tests/scripts/test_robinhood_smoke.py new file mode 100644 index 00000000..7e265322 --- /dev/null +++ b/tests/scripts/test_robinhood_smoke.py @@ -0,0 +1,212 @@ +"""Tests for the read-only Robinhood probe script. + +The script is the one thing in this repository designed to run against live credentials at a +venue with no sandbox, so its read-only guarantee and its credential pre-checks are the parts +worth pinning. Everything here runs offline with a stub transport. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any + +import pytest + +from scripts.robinhood_smoke import ( + PROBES, + ReadOnlyViolation, + _ReadOnly, + compare_shapes, + load_credentials, + run_probes, + shape_of, +) + +_VALID_SEED_B64 = "A" * 44 # a base64 32-byte Ed25519 seed is 44 characters + + +class _StubTransport: + """Records requests and replays canned payloads. Never touches the network.""" + + def __init__(self, payload: Any = None) -> None: + self.payload = payload if payload is not None else {"results": [{"a": 1}]} + self.seen: list[tuple[str, str]] = [] + + def _request(self, method: str, path: str, **kwargs: Any) -> Any: + self.seen.append((method, path)) + return self.payload + + def get_accounts(self) -> Any: + return self._request("GET", "/api/v2/crypto/trading/accounts/") + + def get_trading_pairs(self) -> Any: + return self._request("GET", "/api/v2/crypto/trading/trading_pairs/") + + def get_best_bid_ask(self, symbol: str) -> Any: + return self._request("GET", "/api/v2/crypto/marketdata/best_bid_ask/") + + def get_estimated_price(self, symbol: str, side: str, quantity: str) -> Any: + return self._request("GET", "/api/v2/crypto/trading/estimated_price/") + + def get_holdings(self) -> Any: + return self._request("GET", "/api/v2/crypto/trading/holdings/") + + +# --- the read-only guarantee --------------------------------------------------------------- + + +def test_non_get_requests_are_refused() -> None: + """The whole point of the script: it must be structurally unable to place an order.""" + guard = _ReadOnly(_StubTransport()) + with pytest.raises(ReadOnlyViolation, match="read-only by construction"): + guard._request("POST", "/api/v2/crypto/trading/orders/") + + +@pytest.mark.parametrize("method", ["post", "PUT", "delete", "PATCH"]) +def test_every_mutating_verb_is_refused_case_insensitively(method: str) -> None: + """A lowercase `post` must not slip past a naive `!= "POST"` comparison.""" + guard = _ReadOnly(_StubTransport()) + with pytest.raises(ReadOnlyViolation): + guard._request(method, "/api/v2/crypto/trading/orders/") + + +def test_a_refused_request_is_not_recorded_as_issued() -> None: + guard = _ReadOnly(_StubTransport()) + with pytest.raises(ReadOnlyViolation): + guard._request("POST", "/orders/") + assert guard.calls == [] + + +def test_get_requests_pass_through_and_are_recorded() -> None: + stub = _StubTransport() + guard = _ReadOnly(stub) + guard._request("GET", "/api/v2/crypto/trading/accounts/") + assert guard.calls == [("GET", "/api/v2/crypto/trading/accounts/")] + assert stub.seen == [("GET", "/api/v2/crypto/trading/accounts/")] + + +def test_running_every_probe_issues_only_gets() -> None: + guard = _ReadOnly(_StubTransport()) + run_probes(guard, "BTC-USD") + assert guard.calls, "probes issued no requests at all" + assert {method for method, _ in guard.calls} == {"GET"} + + +# --- shapes -------------------------------------------------------------------------------- + + +def test_shape_discards_every_leaf_value() -> None: + """Balances must never reach the terminal -- only their types.""" + shape = shape_of({"buying_power": "1234.56", "count": 7, "live": True, "next": None}) + assert shape == {"buying_power": "str", "count": "int", "live": "bool", "next": "null"} + assert "1234.56" not in json.dumps(shape) + + +def test_a_long_list_collapses_to_one_element_and_a_count() -> None: + shape = shape_of([{"symbol": "BTC-USD"}, {"symbol": "ETH-USD"}, {"symbol": "SOL-USD"}]) + assert shape == [{"symbol": "str"}, "... 3 items"] + + +def test_an_empty_list_is_reported_rather_than_silently_matching() -> None: + """An unfunded account returns `[]`; that must be visible, not a vacuous pass.""" + assert shape_of([]) == [""] + + +# --- shape comparison ---------------------------------------------------------------------- + + +def test_a_key_the_venue_does_not_send_is_reported() -> None: + """The dangerous direction: a field the fixture invented and the adapter may already read.""" + diffs = compare_shapes({"a": "str"}, {"a": "str", "fee_tier_status": {"fee_ratio": "str"}}) + assert len(diffs) == 1 + assert "MISSING AT VENUE" in diffs[0] + assert "fee_tier_status" in diffs[0] + + +def test_a_key_only_the_venue_sends_is_reported() -> None: + diffs = compare_shapes({"a": "str", "fees_paid": "str"}, {"a": "str"}) + assert len(diffs) == 1 + assert "NEW AT VENUE" in diffs[0] + assert "fees_paid" in diffs[0] + + +def test_a_type_change_is_reported_with_both_sides() -> None: + diffs = compare_shapes({"quantity": "float"}, {"quantity": "str"}) + assert len(diffs) == 1 + assert "TYPE DIFFERS" in diffs[0] + assert "quantity" in diffs[0] + + +def test_identical_shapes_produce_no_differences() -> None: + shape = shape_of({"results": [{"account_number": "x", "buying_power": "1.00"}]}) + assert compare_shapes(shape, shape) == [] + + +def test_differences_are_found_inside_list_elements() -> None: + live = shape_of([{"symbol": "BTC-USD"}]) + fixture = shape_of([{"symbol": "BTC-USD", "min_order_amount": "1.00"}]) + diffs = compare_shapes(live, fixture) + assert len(diffs) == 1 + assert "min_order_amount" in diffs[0] + + +# --- credentials --------------------------------------------------------------------------- + + +def _write_env(tmp_path: Path, body: str) -> Path: + env = tmp_path / ".env" + env.write_text(body) + return env + + +def test_a_missing_private_key_names_it_and_explains_why_it_is_needed(tmp_path: Path) -> None: + """The exact case an operator hits after adding only the API key.""" + env = _write_env(tmp_path, "ROBINHOOD_API_KEY=abc\n") + with pytest.raises(SystemExit) as excinfo: + load_credentials(env) + message = str(excinfo.value) + assert "ROBINHOOD_PRIVATE_KEY" in message + assert "signs EVERY request" in message + + +def test_a_missing_api_key_is_named(tmp_path: Path) -> None: + env = _write_env(tmp_path, f"ROBINHOOD_PRIVATE_KEY={_VALID_SEED_B64}\n") + with pytest.raises(SystemExit, match="ROBINHOOD_API_KEY"): + load_credentials(env) + + +def test_a_public_key_pasted_as_the_private_key_is_caught_before_a_request( + tmp_path: Path, +) -> None: + """Wrong-length seeds must fail here, not as a 401 that reads like a signing bug.""" + env = _write_env(tmp_path, "ROBINHOOD_API_KEY=abc\nROBINHOOD_PRIVATE_KEY=tooshort\n") + with pytest.raises(SystemExit) as excinfo: + load_credentials(env) + assert "PUBLIC key" in str(excinfo.value) + + +def test_a_wellformed_credential_is_returned(tmp_path: Path) -> None: + env = _write_env(tmp_path, f"ROBINHOOD_API_KEY=abc\nROBINHOOD_PRIVATE_KEY={_VALID_SEED_B64}\n") + assert load_credentials(env) == ("abc", _VALID_SEED_B64) + + +# --- probes and fixtures agree --------------------------------------------------------------- + + +def test_every_probe_names_a_fixture_that_exists() -> None: + """A renamed fixture must break here, not halfway through a live run.""" + fixtures = Path(__file__).resolve().parents[1] / "fixtures" + for name, fixture_name in PROBES: + assert (fixtures / fixture_name).is_file(), f"{name} points at a missing {fixture_name}" + + +def test_a_failing_probe_does_not_abort_the_others() -> None: + class _Exploding(_StubTransport): + def get_best_bid_ask(self, symbol: str) -> Any: + raise RuntimeError("401 unauthorized") + + results = run_probes(_ReadOnly(_Exploding()), "BTC-USD") + assert results["best_bid_ask"]["ok"] is False + assert "401 unauthorized" in results["best_bid_ask"]["error"] + assert results["accounts"]["ok"] is True