fix(broker-alpaca): NaN-safe money coercion, host-map-only construction, quoted path segments

packages/keel-broker-alpaca/README.md

@@ -9,8 +9,10 @@ from any third-party Alpaca adapter. "Alpaca" appears here solely to identify wh
 package talks to.
 
 US equities, cash account, long-only, regular session. Paper and live are separate
-hosts selected by an explicit `endpoint` choice — there is no configuration path from a
-paper credential to `https://api.alpaca.markets`, by construction.
+hosts selected by an explicit `endpoint` choice — `transport.TRADING_HOSTS` is the only
+construction path to a trading host (no constructor parameter accepts a host URL), so
+there is no configuration path from a paper credential to
+`https://api.alpaca.markets`, by construction.
 
 ## What works
 
@@ -30,11 +32,12 @@ paper credential to `https://api.alpaca.markets`, by construction.
 Commission is $0. Sells carry regulatory pass-throughs, modelled in `fees.py` with the
 rates as provenance-commented constants:
 
-- **SEC Section 31**: $22.90 per $1,000,000 of sale proceeds — Alpaca's own
-  regulatory-fees page ($27.80 previously; the SEC adjusts the rate periodically, and
-  its advisory 2026-2 moves it to $20.60 per $1M as of 2026-04-04 — a documented
-  re-measurement point, encoded as the venue's published figure until Alpaca's page
-  moves).
+- **SEC Section 31**: $22.90 per $1,000,000 of sale proceeds — the figure Alpaca's own
+  regulatory-fees page still publishes. The SEC's advisory 2026-2 rate ($20.60 per $1M)
+  has been in force since 2026-04-04, so the venue's page is the stale side; the model
+  deliberately tracks what the venue itself charges, which over-states the statutory
+  rate by ~$0.02 per $10k (conservative for a sell preview's proceeds), and that delta
+  is the re-measurement trigger for when Alpaca's page updates.
 - **FINRA TAF**: $0.000166 per share, capped at $8.30 per trade — the cap is on
   Alpaca's page; the per-share rate is FINRA Schedule A §4(b)(7), in force since
   2021-01-01.

packages/keel-broker-alpaca/keel_broker_alpaca/adapter.py

@@ -482,13 +482,25 @@ def _decimal_or_none(value: Any) -> Decimal | None:
     transport already parses unquoted numbers as `Decimal`, and `Decimal(str(value))`
     here lands both shapes on the same exact number. `None` rather than zero: an absent
     number and a zero number must never be the same value at a preview gate.
+
+    Non-finite values are `None` too, and the check is explicit because the `except`
+    below never fires for them: JSON's `NaN`/`Infinity` tokens arrive via
+    `parse_constant` (not `parse_float`) as `float("nan")`/`float("inf")`, and
+    `Decimal(str(...))` parses BOTH without raising -- `Decimal("NaN")` then crashes any
+    ordering comparison (`bid > 0`, `min(buying_power, cash)`) and `Decimal("Infinity")`
+    compares as a real price. A non-finite number is not a money value; refusing it here
+    is what keeps the preview's "every path that could not price the order populates
+    `errors`" invariant true for these rows too.
     """
     if value is None or isinstance(value, bool):
         return None
     try:
-        return Decimal(str(value))
+        parsed = Decimal(str(value))
     except (InvalidOperation, ValueError, TypeError):
         return None
+    if not parsed.is_finite():
+        return None
+    return parsed
 
 
 def _terminal_unknown(order_id: str) -> OrderStatus:

packages/keel-broker-alpaca/keel_broker_alpaca/fees.py

@@ -9,12 +9,15 @@
 
 Provenance (all read 2026-08-17):
 
-* **SEC Section 31 fee** -- charged on SELLS, per $1,000,000 of principal. Alpaca's own
-  regulatory-fees page (https://alpaca.markets/support/regulatory-fees) states the
-  current rate as $22.90 per $1M ($27.80 previously). The SEC adjusts this rate
-  periodically by fee-rate advisory (advisory 2026-2 moves it to $20.60 per $1M as of
-  2026-04-04); the venue's published figure is the one encoded, and the drift is a
-  documented re-measurement point, not a silent correction.
+* **SEC Section 31 fee** -- charged on SELLS, per $1,000,000 of principal. The SEC's
+  advisory 2026-2 rate ($20.60 per $1M) took effect 2026-04-04 and IS the rate in force;
+  Alpaca's own regulatory-fees page (https://alpaca.markets/support/regulatory-fees)
+  still publishes $22.90 ($27.80 previously) -- the venue's page is the stale side of
+  the two. The encoded $22.90 deliberately tracks what the venue itself charges, which
+  over-states the statutory rate by ~$0.02 per $10k -- the conservative direction for a
+  sell preview's proceeds -- and that delta is the documented re-measurement trigger:
+  when Alpaca's page moves to the advisory figure, the constant and this provenance
+  move with it.
 * **FINRA Trading Activity Fee (TAF)** -- charged on SELLS, per share, capped per trade.
   The cap ($8.30 for equities) is on Alpaca's page above; the per-share rate ($0.000166)
   is FINRA's, Schedule A to the FINRA By-Laws §4(b)(7) (SR-FINRA-2020-032, in force since

packages/keel-broker-alpaca/keel_broker_alpaca/translate.py

@@ -224,8 +224,21 @@ def to_unix_seconds(value: str) -> int:
     Venue timestamps can carry fractional seconds and (per the schema) explicit offsets;
     fractional seconds truncate because `Candle.ts` is whole seconds and a bar's open
     time is second-aligned anyway.
+
+    An offset-less timestamp is REFUSED, never read as local time: without an offset
+    `fromisoformat` yields a naive datetime whose `.timestamp()` silently assumes the
+    host's zone, so the same bar would timestamp differently per machine. `ValueError`
+    is this module's refusal signal (`to_timeframe`'s), and refusing is the fail-closed
+    direction -- the venue's contract sends `Z` or an explicit offset, so anything else
+    is garbage, not a zone to guess.
     """
-    return int(datetime.fromisoformat(value.replace("Z", "+00:00")).timestamp())
+    parsed = datetime.fromisoformat(value.replace("Z", "+00:00"))
+    if parsed.tzinfo is None:
+        raise ValueError(
+            f"alpaca timestamp {value!r} carries no UTC offset; refusing to read a naive "
+            "datetime as local time"
+        )
+    return int(parsed.timestamp())
 
 
 __all__ = [

packages/keel-broker-alpaca/keel_broker_alpaca/transport.py

@@ -133,11 +133,11 @@ def get_latest_quote(self, symbol: str, feed: str) -> Any: ...
 class AlpacaTransport:
     """The live, network-backed `Transport`: header-authed JSON over HTTPS.
 
-    The trading host is derived from `endpoint` and cannot be overridden -- a paper
-    configuration must be structurally unable to reach the live venue. The `trading_host`
-    and `data_host` constructor parameters exist for tests pointing at a local recorder;
-    they are explicit escapes, not a configuration surface, and nothing in this workspace
-    passes them in production code.
+    The trading host is derived from `endpoint` and NOTHING else: the constructor accepts
+    no host URL of any kind (there were `trading_host`/`data_host` keyword escapes here
+    once -- zero callers, dead surface, removed), so `TRADING_HOSTS` is the only map from
+    an environment choice to a host and a paper credential cannot be pointed at the live
+    venue by any configuration path.
     """
 
     def __init__(
@@ -150,8 +150,6 @@ def __init__(
         timeout: float = 10.0,
         max_attempts: int = 3,
         sleep: Callable[[float], None] = time.sleep,
-        trading_host: str | None = None,
-        data_host: str | None = None,
     ) -> None:
         if endpoint not in TRADING_HOSTS:
             raise ValueError(
@@ -171,8 +169,8 @@ def __init__(
         self._timeout = timeout
         self._max_attempts = max_attempts
         self._sleep = sleep
-        self._trading_host = trading_host if trading_host is not None else TRADING_HOSTS[endpoint]
-        self._data_host = data_host if data_host is not None else DATA_HOST
+        self._trading_host = TRADING_HOSTS[endpoint]
+        self._data_host = DATA_HOST
 
     @property
     def endpoint(self) -> str:
@@ -273,6 +271,9 @@ def _request_json(
         URL per request, so a recorded call is exactly what the venue received. Like the
         Robinhood transport, `quote_via=quote` (not `quote_plus`) so a `+` in a value is
         never decoded server-side as a space, and `safe=""` so nothing rides unencoded.
+        The same discipline covers PATH segments: callers percent-encode every
+        interpolated id/symbol with `quote(..., safe="")`, so a `/` or `?` inside one can
+        never reshape the request into a different resource.
 
         `parse_float=Decimal`, never `response.json()`: this venue mixes quoted and
         unquoted money fields (see the module docstring), and the parser is the only
@@ -313,7 +314,7 @@ def get_order(self, order_id: str) -> Any:
         rather than launder a network blip into "this order does not exist".
         """
         try:
-            return self._request_json("GET", f"/v2/orders/{order_id}")
+            return self._request_json("GET", f"/v2/orders/{quote(order_id, safe='')}")
         except AlpacaAPIError as exc:
             if exc.status_code == 404:
                 return None
@@ -328,7 +329,8 @@ def cancel_order(self, order_id: str) -> Any:
         filled) are returned as statuses rather than raised because both are ordinary
         answers the adapter maps to `False`; every other failure raises.
         """
-        response = self._send("DELETE", f"{self._trading_host}/v2/orders/{order_id}")
+        path = f"/v2/orders/{quote(order_id, safe='')}"
+        response = self._send("DELETE", f"{self._trading_host}{path}")
         status = int(getattr(response, "status_code", 0))
         if status < 400 or status in (404, 422):
             return status
@@ -355,7 +357,10 @@ def get_bars(
         if page_token is not None:
             params["page_token"] = page_token
         return self._request_json(
-            "GET", f"/v2/stocks/{symbol}/bars", host=self._data_host, params=params
+            "GET",
+            f"/v2/stocks/{quote(symbol, safe='')}/bars",
+            host=self._data_host,
+            params=params,
         )
 
     def get_latest_quote(self, symbol: str, feed: str) -> Any:
@@ -365,7 +370,10 @@ def get_latest_quote(self, symbol: str, feed: str) -> Any:
         signal the adapter treats as "no book on that side", not a price of zero.
         """
         return self._request_json(
-            "GET", f"/v2/stocks/{symbol}/quotes/latest", host=self._data_host, params={"feed": feed}
+            "GET",
+            f"/v2/stocks/{quote(symbol, safe='')}/quotes/latest",
+            host=self._data_host,
+            params={"feed": feed},
         )
 
 

tests/broker_alpaca/test_adapter.py

@@ -260,6 +260,18 @@ def test_an_unknown_endpoint_is_refused_at_construction(self) -> None:
             with pytest.raises(ValueError, match="endpoint"):
                 AlpacaAdapter(endpoint=bad)
 
+    def test_no_constructor_parameter_accepts_a_host_url(self) -> None:
+        """The `trading_host`/`data_host` keyword escapes are GONE, so the documented
+        endpoint-to-host map is the only construction path to a trading host: no
+        parameter accepts a host URL at all, which is what makes the README's "no
+        configuration path from a paper credential to the live host, by construction"
+        literally true (FR-11). `TypeError` is Python's own "no such keyword" answer --
+        there is nothing to validate because there is nothing to pass."""
+        with pytest.raises(TypeError):
+            AlpacaTransport("key-id", "secret", trading_host=LIVE_TRADING_HOST)  # type: ignore[call-arg]
+        with pytest.raises(TypeError):
+            AlpacaTransport("key-id", "secret", data_host="https://example.test")  # type: ignore[call-arg]
+
     def test_the_data_tier_is_a_declared_choice_not_an_assumption(self) -> None:
         """IEX (free) vs SIP is a declared capability (FR-5): the adapter names its feed
         on every market-data request instead of letting the venue default it, because the
@@ -327,6 +339,22 @@ def test_a_short_position_row_is_not_reported_as_a_holding(self) -> None:
         balances = AlpacaAdapter(transport).get_balances()
         assert [b.currency for b in balances] == ["USD"]
 
+    def test_a_nonfinite_buying_power_is_handled_not_a_crash(self) -> None:
+        """A NaN `buying_power` arrives as `float("nan")` (the `parse_constant` path, not
+        `parse_float`), and `min()` over a NaN `Decimal` raises. The existing
+        balances convention for a money field that cannot be parsed is the same as an
+        absent one -- read as zero via the `or Decimal("0")` every balance field carries
+        -- so a garbage spendable figure never reaches a `Balance` row and nothing
+        raises."""
+        account = load_fixture("alpaca_account.json")
+        account["buying_power"] = float("nan")
+
+        balances = AlpacaAdapter(FakeTransport(account=account)).get_balances()
+
+        usd = {b.currency: b for b in balances}["USD"]
+        assert usd.available == Decimal("0"), "an unparseable buying power reads as zero"
+        assert usd.total == Decimal("102086.50"), "the parseable cash figure is untouched"
+
 
 # ---------------------------------------------------------------------------------------------
 # Candles (FR-5, FR-10's adjusted/raw policy)
@@ -367,6 +395,24 @@ def test_every_unsupported_granularity_is_refused(self) -> None:
             with pytest.raises(ValueError, match="timeframe"):
                 adapter.get_candles(_PRODUCT, granularity, 0, 86_400)
 
+    def test_a_nonfinite_bar_value_is_never_stored_in_a_candle(self) -> None:
+        """A NaN high arrives as `float("nan")`, and `Decimal("NaN")` is TRUTHY -- so
+        without an explicit finiteness check the `or Decimal("0")` fallback never fires
+        and the NaN rides into `Candle.high` silently, poisoning every indicator that
+        touches the series. The module's existing convention for an unparseable bar leaf
+        is the same as an absent one: read as zero, never as the venue's garbage."""
+        bars = load_fixture("alpaca_bars_page1.json")
+        bars["bars"][0]["h"] = float("nan")
+        bars["next_page_token"] = None
+
+        candles = AlpacaAdapter(FakeTransport(bars_pages=[bars])).get_candles(
+            _PRODUCT, Granularity.ONE_DAY, 1_700_000_000, 1_700_086_400
+        )
+
+        assert len(candles) == 2, "both fixture bars survive; only the NaN leaf changes"
+        assert candles[0].high == Decimal("0"), "the unparseable-leaf-reads-as-zero rule"
+        assert candles[0].close == Decimal("131.9"), "parseable leaves are untouched"
+
 
 # ---------------------------------------------------------------------------------------------
 # Preview: synthesized from the book (FR-4, FR-7)
@@ -447,6 +493,26 @@ def test_a_quote_with_no_active_ask_leaves_the_buy_unpriced_and_says_so(self) ->
         assert preview.errors, "an unpriced leg must appear in errors"
         assert any("ask" in e for e in preview.errors)
 
+    def test_a_nonfinite_quote_side_is_unpriced_and_reported_never_a_crash(self) -> None:
+        """JSON `NaN`/`Infinity` tokens ride `parse_constant`, not `parse_float`, so they
+        reach the adapter as `float("nan")`/`float("inf")` -- and `Decimal(str(...))`
+        parses BOTH without raising, which means the `except` in `_decimal_or_none` never
+        fires. `Decimal("NaN")` then crashes the `bid > 0` comparison and `Decimal(
+        "Infinity")` compares `> 0` as a real price; a non-finite side must land in the
+        same "no active side" path as a zero one -- `errors` says so, nothing raises --
+        for the preview docstring's "every path that could not price the order populates
+        `errors`" invariant to hold."""
+        quote = json.loads('{"quote": {"bp": NaN, "ap": Infinity}}', parse_float=Decimal)
+        preview = AlpacaAdapter(FakeTransport(quote=quote)).preview_order(
+            MarketIOCByQuote(product_id=_PRODUCT, side=Side.BUY, quote_size=Decimal("100"))
+        )
+
+        assert preview.est_base_size == Decimal("0")
+        assert preview.errors, "a non-finite quote side must appear in errors"
+        assert any("ask" in e for e in preview.errors)
+        assert preview.detail["best_bid"] == "none"
+        assert preview.detail["best_ask"] == "none"
+
     def test_a_non_usd_product_is_refused_before_any_request_is_made(self) -> None:
         transport = _full_transport()
         with pytest.raises(UnsupportedOrder, match="USD"):

tests/broker_alpaca/test_translate.py

@@ -208,3 +208,13 @@ def test_rfc3339_and_epoch_round_trip() -> None:
     assert to_unix_seconds("2026-08-14T14:30:00.999Z") == ts
     # ...and an explicit offset must parse too, not only a trailing Z.
     assert to_unix_seconds("2026-08-14T10:30:00-04:00") == ts
+
+
+def test_an_offset_less_timestamp_is_refused_never_read_as_local_time() -> None:
+    """`fromisoformat` without an offset yields a NAIVE datetime, and `.timestamp()`
+    silently assumes the host's local zone -- the same bar would timestamp differently
+    per machine. The fail-closed rule (the venue contract sends `Z` or an explicit
+    offset, so anything else is garbage): refuse with `ValueError`, this module's
+    refusal signal (`to_timeframe`'s), rather than guess the zone."""
+    with pytest.raises(ValueError, match="offset"):
+        to_unix_seconds("2026-08-14T14:30:00")

tests/broker_alpaca/test_transport.py

@@ -243,6 +243,42 @@ def test_get_positions_and_get_clock_use_their_documented_paths(http: Any) -> No
     assert recorder.calls[1]["url"] == f"{PAPER_TRADING_HOST}/v2/clock"
 
 
+# ---------------------------------------------------------------------------------------------
+# Path-segment encoding: a symbol or order id is always ONE segment
+# ---------------------------------------------------------------------------------------------
+
+
+def test_a_symbol_containing_a_path_separator_stays_one_segment(http: Any) -> None:
+    """A malformed product id (`A/B-USD`) survives `to_symbol` as `A/B`, and an unquoted
+    `/` would reshape `/v2/stocks/A/B/bars` into a DIFFERENT resource. Path segments are
+    percent-encoded with `safe=""` exactly like the query string, so the symbol rides as
+    `A%2FB` -- one segment, whatever it contains."""
+    recorder = http([_FakeResponse(payload={"bars": []}), _FakeResponse(payload={"quote": {}})])
+    transport = _transport()
+
+    transport.get_bars("A/B", "1Day", "2026-08-01T00:00:00Z", "2026-08-14T00:00:00Z", "iex")
+    transport.get_latest_quote("A/B", "iex")
+
+    assert urlsplit(recorder.calls[0]["url"]).path == "/v2/stocks/A%2FB/bars"
+    assert urlsplit(recorder.calls[1]["url"]).path == "/v2/stocks/A%2FB/quotes/latest"
+
+
+def test_an_order_id_with_query_or_traversal_characters_stays_one_segment(http: Any) -> None:
+    """A `?` in an order id would open a query string and `../` would walk out of
+    `/v2/orders/`; both are data in an opaque id, so both ride percent-encoded (`%3F`,
+    `%2F`) and the request is never reshaped -- there is no query part at all."""
+    recorder = http([_FakeResponse(payload={"id": "o1"}), _FakeResponse(204)])
+    transport = _transport()
+
+    transport.get_order("o1?x=../evil")
+    transport.cancel_order("o1?x=../evil")
+
+    expected = f"{PAPER_TRADING_HOST}/v2/orders/o1%3Fx%3D..%2Fevil"
+    assert recorder.calls[0]["url"] == expected
+    assert recorder.calls[1]["url"] == expected
+    assert all(urlsplit(call["url"]).query == "" for call in recorder.calls)
+
+
 # ---------------------------------------------------------------------------------------------
 # Response handling: money as Decimal, the 404 sentinel, cancel statuses
 # ---------------------------------------------------------------------------------------------