feat(port): session-aware venues — market clock in the port, closed-session semantics in the engine (#370 B1) - #385
Merged
Conversation
…ession semantics in the engine (#370 B1) FR-9 (PRD §3, keel-broker-alpaca): equities are not 24/7. Until now the feed-staleness rails and the agent's cycle semantics assumed a 24/7 venue, so a weekend or market holiday would report a closed equities feed as STALE (the crypto semantics false-positiving) and cycles would log staleness-gated noise. The port gains a first-class session concept: - `BrokerCapabilities.session_bound: bool` — REQUIRED, not defaulted. Crypto venues (fake/coinbase/robinhood) declare False; alpaca declares True. A default would be an answer to a question only the venue knows, and the likeliest default (24/7) is exactly the one that reads a closed equities venue as a stale feed. - `Broker.market_clock() -> SessionState` (`open` | `closed` | `clock_unavailable`). The alpaca implementation REUSES Phase A's `/v2/clock` read (`is_market_open()` now delegates to it, so the two can never disagree); the 24/7 adapters answer `OPEN` as a constant with no network call (proven by the no-transport adapter tests). - The conformance suite holds all four adapters to the new surface. Fail-closed clock rule: a session-bound venue's clock that cannot be read (transport error, missing transport, no clock in the response) answers `clock_unavailable` — never an exception, never a guess of open. The engine belt-and-braces the same way: a third-party adapter that raises instead of answering still fails closed; the cycle never crashes on a clock read. Engine consumption (`keel/agent.py::run_once`): - The session gate sits directly after the kill-switch and skips EXACTLY like it — no poll, no evaluation, no orders — logging `agent.cycle_skipped` with `reason: market_closed` (INFO; a weekend is expected) or the distinct `reason: market_clock_unavailable` (WARNING; "could not know" and "we know it is shut" are different operator facts). The kill-switch still outranks. - Each cycle records the venue's answer into `agent_state` (`market_session`/`market_session_ts`), so the broker-free surfaces can render it without a clock call of their own. - `fetch --check` (still offline) reads that recording via `agent.recorded_market_closed`: a stale-but-present series renders as the distinct `CLOSED` state and does NOT count as missing/stale for the exit code — a weekend must not page an operator. `missing` still alerts (a closed venue serves history), `clock_unavailable` does NOT defuse alerts (fail-closed for trading, fail-loud for alerting), and the recording is trusted only for rail 12's feed-heartbeat window (`interval_sec * FEED_STALENESS_CYCLES`) so a dead agent cannot silence alerts forever. - `keel status`/`keel tui` render a distinct session line under the kill-switch line (the #345 rail-17 precedent; no paper carve-out — the session gate skips paper cycles too). Crypto unchanged, guaranteed at every seam: a broker that does not implement the port (today's `CoinbaseClient`) or declares `session_bound=False` is never asked its clock, writes no session state, and every existing behavior, output, and test stays byte-identical. Test-first: the conformance, adapter, agent, freshness, fetch-CLI, status and TUI tests were written red first (collection failures on the missing `SessionState`/`session_bound`/`market_clock`/`MarketSessionStatus` plus failing CLOSED/exit-code assertions) and drive the implementation. Refs #370
…fail-loud clocks Ten adversarial-review findings on PR #385 (branch feat/370-session-awareness): - Record the venue session (state/ts/effective interval) BEFORE the kill-switch return: a halted-but-healthy deployment keeps tracking the venue clock, so a weekend under a kill switch no longer false-positives STALE and a Friday pre-close halt no longer freezes market_session=open all weekend. - Namespace the session record by venue (market_session:{venue} plus ts and interval twins); recorded_market_closed takes the venue it is answering for, so one venue's CLOSED never defuses another's staleness and two deployments with different cadences carry their own trust windows. - Alpaca market_clock: a 2xx body without a usable boolean is_open is CLOCK_UNAVAILABLE, never CLOSED (closed defuses alerting forever; a malformed body must not buy that silence). Only an actual bool answers OPEN/CLOSED. - _venue_session validates market_clock()'s answer is a SessionState: a None-returning third-party adapter fails closed with the distinct market_clock_unavailable skip instead of AttributeError killing the loop. - The trust window derives from the RECORDED cycle interval (loop/monitor --interval overrides included), falling back to config only when absent. - TUI freshness styling is session-aware: closed AND inside the trust window renders behind series muted, not warn -- same source of truth as the session line; no-data cells keep warn (a closed venue still serves history). - recorded_market_closed documents the pre-close staleness attenuation (bounded silence until reopen) with an arc test: alerts pre-close, silenced during closure, re-alerts once the record expires post-reopen. - Trust-window boundary tests (age == window defuses, window+1 does not, fresh-nonzero-age defuses, junk ts does not) with a frozen clock. - Truthful fetch wording: the summary says 'market closed -- staleness does not alert' when closed-explained staleness exists, and a plain fetch no longer calls a behind cache 'all series current' to justify the skip. - keel monitor --loop is session-aware: closed venues skip polling (logged once per state change) and record the session so fetch --check stays quiet over the weekend; crypto venues poll exactly as before.
eaitbrahim
added a commit
that referenced
this pull request
Aug 20, 2026
Minor, not patch. Three things since v0.9.3 change what an implementor or a deployment can rely on: * A NEW DISTRIBUTION. `keel-broker-alpaca` (#382, #384) plus the paper-equities profile that selects it (#386), so a deployment can now be US equities via the broker port rather than crypto only. * THE PORT CONTRACT MOVED TWICE. `market_clock`/`market_schedule` made venues session-aware (#385), and `place_order` gained `idempotency_key` (#419). Both carry defaults so no CALLER breaks, but a third-party adapter that does not accept them is no longer a `Broker` -- the conformance suite now says so. That is exactly the kind of change a patch bump must not hide. * THE OPERATOR CONSOLE. The TUI became keel's console across #399-#408, and `keel update` (#415/#417) makes a deployment self-updating. Every pinned sibling moves with it. The four production distributions are required `==` at this exact version (`RELEASING.md`, "Release assets"), so a bump that missed one would install a mixed set -- the `keel-trader 0.5.7` against `keel-core 0.5.5` failure `keel versions` exists to catch, and which `~/keel` actually ran across two releases. Also in this window, on the Robinhood adapter: the best_bid_ask fixture corrected against the live venue (#414), a credential guard that catches the error it only claimed to (#416), pre-flight sizing reported on the preview (#418), transport backoff (#420), and the fenced one-order probe (#421). Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Phase B1 of #370 (FR-9, PRD
docs/superpowers/specs/2026-08-18-keel-broker-alpaca-prd.md): the broker port gains a first-class session concept, and the engine consumes it — a closed equities market reads market closed, never feed stale.Equities are not 24/7. keel's feed-staleness rails and the agent's cycle semantics assumed a 24/7 venue: on a weekend or market holiday, an equities feed that is simply closed would be reported STALE (the crypto semantics false-positiving), and cycles would log staleness-gated noise.
The port surface
BrokerCapabilities.session_bound: bool— REQUIRED, not defaulted (packages/keel-broker-api/keel_broker_api/capabilities.py). fake/coinbase/robinhood declareFalse(24/7); alpaca declaresTrue. A default would be an answer to a question only the venue knows, and the likeliest default (24/7) is exactly the one that reads a closed equities venue as a stale feed.Broker.market_clock() -> SessionState(port.py;SessionStateinresults.py:open|closed|clock_unavailable). Alpaca reuses Phase A's/v2/clockread —is_market_open()now delegates tomarket_clock()so the two can never disagree. The 24/7 adapters answerOPENas a constant with no network call (pinned by the no-transport adapter tests).conformance/suite.py).FR-9 mapping (engine)
keel/agent.py::run_once): the session gate sits directly after the kill-switch and skips exactly like it — no poll, no evaluation, no orders — loggingagent.cycle_skippedwithreason: market_closed(INFO; a weekend is expected) or the distinctreason: market_clock_unavailable(WARNING; "could not know" and "we know it is shut" are different operator facts). The kill-switch still outranks.clock_unavailable— never an exception, never a guess of open. The engine belt-and-braces the same way (a third-party adapter that raises instead of answering still fails closed); the cycle never crashes on a clock read.agent_state(market_session/market_session_ts).fetch --check(still offline) reads that recording viaagent.recorded_market_closed: stale-but-present series render as the distinctCLOSEDstate and do NOT count as missing/stale for the exit code — a weekend must not page an operator. Deliberate carve-outs:missingstill alerts (a closed venue serves history);clock_unavailabledoes not defuse alerts (fail-closed for trading, fail-loud for alerting); the recording is trusted only for rail 12's feed-heartbeat window (interval_sec * FEED_STALENESS_CYCLES) so a dead agent cannot silence alerts forever.keel status/keel tui: a distinct session line directly under the kill-switch line (the feat(status): surface rail-17 attestation expiry before it vetoes, and a rules enable verb (#340) #345 rail-17 line precedent; no paper carve-out — the session gate skips paper cycles too, so the same line is truthful in every mode).Crypto unchanged — guaranteed at every seam
A broker that does not implement the port (today's
CoinbaseClient) or declaressession_bound=Falseis never asked its clock, writes no session state, and every existing behavior, output, and test stays byte-identical (pinned by engine tests at both seams: no-port broker and port-but-24/7 broker).Test-first
Conformance, adapter, agent, freshness, fetch-CLI, status, and TUI tests were written red first — collection failures on the missing
SessionState/session_bound/market_clock/MarketSessionStatus, plus failingCLOSED-label and exit-code assertions — and drove the implementation.Gates
uv run pytest -q— 3161 passed, 2 skipped (both expected skips: session-bound venue in the 24/7 clock test; the pre-existing no-granularity skip)uv run ruff check keel tests packages— cleanuv run mypy— clean (249 files)Refs #370 (Phase B continues — not a fix)