diff --git a/keel/commands/serve.py b/keel/commands/serve.py index 8d19a7e..0964f95 100644 --- a/keel/commands/serve.py +++ b/keel/commands/serve.py @@ -15,6 +15,7 @@ from __future__ import annotations import webbrowser +from typing import Any import click @@ -57,13 +58,19 @@ def serve_cmd(ctx: click.Context, host: str, port: int, open_browser: bool) -> N readable by anyone who can reach the port, with a cleartext token as the only obstacle. """ obj = ctx.obj or {} + # Resolved ONCE, here, and carried on the config in both its forms. `build_info()` shells out + # to git twice, and `/api/config` is polled by a service worker (#538) -- an endpoint that + # forks a subprocess to answer "which build is this" would make the cheapest question on the + # server the most expensive one. + build = _build_info() cfg = ServeConfig( host=host, port=port, token=new_session_token(), db_path=obj.get("db_path") or default_db_path(), config_path=obj.get("config_path") or default_config_path(), - build=_build_line(), + build=_build_line(build), + build_info=build, ) if open_browser: @@ -78,12 +85,24 @@ def serve_cmd(ctx: click.Context, host: str, port: int, open_browser: bool) -> N ctx.exit(serve(cfg, echo=click.echo)) -def _build_line() -> str: - """The build identity in the page footer, so a screenshot of the UI says which build produced - it. Best-effort: a footer is not worth failing a server start over.""" +def _build_info() -> Any: + """The running build, resolved once, or `None`. + + Best-effort: a build identity is not worth failing a server start over, and `None` is a state + the consumers already handle -- the footer renders empty and `/api/config` reports the version + absent rather than inventing one. Split out of `_build_line` when `/api/config` (#534) needed + the same object as STRUCTURE rather than as a sentence, so the two can never describe different + builds: parsing `describe()`'s output back into fields would be a display string being read as + data.""" try: from keel.version import build_info - return build_info().describe() + return build_info() except Exception: # pragma: no cover - metadata absent in odd environments - return "" + return None + + +def _build_line(build: Any) -> str: + """The build identity in the page footer, so a screenshot of the UI says which build produced + it.""" + return "" if build is None else str(build.describe()) diff --git a/keel/web/__init__.py b/keel/web/__init__.py index 5a236ea..0233b44 100644 --- a/keel/web/__init__.py +++ b/keel/web/__init__.py @@ -6,8 +6,22 @@ macOS app launched from Finder has no controlling terminal at all, and the GUI human gate (#436) needs somewhere to live that is architecturally distinct from `_is_interactive`. -**This package has no write surface, and that is structural rather than a matter of discipline.** -The request handler implements `do_GET` and `do_HEAD` and nothing else, so every other method is -refused by `BaseHTTPRequestHandler` before any keel code runs. Write actions land in D3, behind a -gate of their own; until then there is no code path here that could grow one by accident. +**This package's write surface is a closed set, and that is structural rather than a matter of +discipline.** + +The sentence that used to be here -- "the request handler implements `do_GET` and `do_HEAD` and +nothing else" -- stopped being true at #437 and is corrected rather than deleted, because +`server.do_POST` still points a reader at this file for the rule it must not break. The guarantee +now is stronger than the one it replaced, and it is the one anyone actually cares about: + +* `do_POST` exists, and it routes ONLY through `keel.commands.setup.ACTIONS` -- idempotent, + non-destructive steps -- so a first-run user on a machine with no terminal can create a + deployment. "No POST at all" was a clean property that was also satisfied by a server which + could not set anything up. +* **Not one of the eleven capability-increasing actions in `keel/capabilities.py` is reachable + from this package**, asserted by a test that scans this source rather than by inspection. The + server cannot arm, release or spend. Attesting, promoting, releasing a halt and arming autonomy + remain CLI-only, behind the TTY gate; D3 (#436) is where a browser gate for those would go. +* The JSON API (#534, `keel/web/api.py`) is **reads only**. Every route in its table answers a GET + and 404s a POST, and it did not widen `API_PREFIX`'s existing `X-Keel-Client` gate by one byte. """ diff --git a/keel/web/api.py b/keel/web/api.py new file mode 100644 index 0000000..2ffd909 --- /dev/null +++ b/keel/web/api.py @@ -0,0 +1,601 @@ +"""The JSON API (#534) -- routing, one bounded read per endpoint, and no rendering at all. + +This module is the READ half of `keel/web/`: which report an endpoint needs, which rows a query +string asks to be ordered by, and what a client is told when there is no deployment to read. It +formats nothing. Every string a user will see was written by `keel/web/payload.py`, which is the +one place a `Decimal` becomes text and the one place the closed `state` vocabulary is spelled -- +`tests/web/test_api.py::test_the_routing_layer_formats_nothing` fails the build on a `format(...)` +call or a format spec appearing here, because the second money renderer is never the one with +`_plain`'s no-exponent guarantee in it. + +**Reads only.** Not one route below answers a POST. The write surface is still +`keel.commands.setup.ACTIONS` reached through `SETUP_ACTION_PREFIX`, still behind +`X-Keel-Client: 1` for anything under `API_PREFIX`, and this issue added nothing to either -- +`test_no_api_route_answers_a_post` asserts that for every route in the table below rather than for +a sample of them. + +**Why the deployment's state is read on EVERY endpoint, including the three that do not need it.** +`engine` is a uniform field so that #536's single `fetch` wrapper can render "keel isn't running" +from whatever response it happens to hold, without a table of which endpoints carry the word. +Measured cost: `keel.commands.setup.inspect` is 3.6 ms per call on this machine against a migrated +database, against 0.07 ms for `gather_status` -- so the liveness probe is roughly fifty times the +report it accompanies, and still small next to a loopback HTTP round trip. If a client is ever +written that polls `/api/config` in a loop, the fix is to cache the `DeploymentState` for the life +of one response (it already is) or per second -- not to make the field optional, because an +optional field puts the branch back on the client. + +**Where the sort is, and where it is not.** Deciding WHICH column a query string asked for, and +refusing one that does not exist, is routing and lives here. The ordering itself -- `Decimal` +comparison over `Field.value` -- is `payload.order_rows`, next to the code that wrote those +strings and under the same contract. A `float()` on a wire value would be the whole money +guarantee dying in one expression, and `test_rule_6_holds_in_the_api_layer_too` runs #533's own +AST scan over this file for exactly that reason. +""" + +from __future__ import annotations + +import time +from collections.abc import Callable, Mapping, Sequence +from dataclasses import dataclass, field +from typing import TYPE_CHECKING, Any + +from keel.web import payload + +if TYPE_CHECKING: # pragma: no cover - typing only + from keel.web.server import ServeConfig + +#: Journal rows served when a caller names no `limit`. The same cap the HTML insights page uses, +#: so the two front-ends answer "how has this been going" with the same amount of history. +DEFAULT_JOURNAL_LIMIT = 50 + +#: The ceiling on `?limit=`. A caller-supplied row count is a memory and time primitive against a +#: server with no proxy in front of it, and a journal is not a bulk export -- `keel insights +#: journal` is, and it runs in the operator's own process against their own machine's limits. +MAX_JOURNAL_LIMIT = 1000 + +#: The two directions, and no third spelling. `desc`/`descending`/`down` would all have to be +#: accepted forever once accepted once, and a client reading `sort.direction` back off the +#: response needs one word to compare against. +DIRECTIONS: tuple[str, ...] = ("asc", "desc") + + +# -- the reads ------------------------------------------------------------------------------------ +# +# These four moved here from `keel/web/server.py` when this module arrived, unchanged. They are +# the seam BOTH front-ends read keel through -- the HTML pages and the JSON endpoints -- and a +# copy in each would be two places deciding whether a page may migrate a live database. The +# direction of the dependency is one-way on purpose: `server` imports `api`, `api` imports nothing +# from `server` at runtime, so there is no cycle to reason about. + + +def open_repo(db_path: str) -> Any: + """A plain connection -- deliberately WITHOUT `migrate`. + + Every CLI command migrates on the way in, which is right for a command: it runs once, and a + schema behind the code is a thing to fix rather than to fail on. It is wrong here. These + pages auto-reload every 15 seconds, so migrating per request would have a view that calls + itself read-only take a write lock on the deployment database four times a minute -- against + a database the agent may be mid-cycle on. `server.ensure_schema` does it ONCE, at bind time, + before anything is served.""" + from keel.data.db import connect + from keel.data.repository import Repository + + return Repository(connect(db_path)) + + +def load_config(config_path: str) -> Any: + """`load_config` only -- deliberately NOT `_common._load_cfg`, which also calls + `configure_logging` and `bind_venue`. Those are process-entry side effects; re-applying them + on every page load would have the web UI quietly reconfiguring the running deployment's + logging.""" + from keel.config import load_config as _load + + return _load(config_path) + + +def deployment_state(cfg: ServeConfig) -> Any: + from keel.commands.setup import inspect + + return inspect(cfg.config_path, cfg.db_path) + + +def close_repo(repo: Any) -> None: + conn = getattr(repo, "conn", None) + if conn is not None: + try: + conn.close() + except Exception: # pragma: no cover - a close that fails leaks nothing that matters + pass + + +# -- refusals ------------------------------------------------------------------------------------- + + +class ApiRefusal(Exception): + """A request this API understood and declined -- a column that does not exist, a limit out of + range. Carried as an exception rather than returned, so a reader can refuse from wherever it + discovers the problem without every caller in between having to pass a failure along. + + Distinct from an unexpected exception, which becomes a 500: this one is the API stating a rule + the caller broke, and its `detail` is written to be shown to whoever wrote the caller.""" + + def __init__(self, status: int, title: str, detail: str) -> None: + super().__init__(detail) + self.status = status + self.title = title + self.detail = detail + + +# -- the readers ---------------------------------------------------------------------------------- + + +def _status_report(cfg: ServeConfig, now_ts: int) -> Any: + """`gather_status`'s frozen report, over its own connection. + + A named module-level function rather than an inline call inside `read_status` so a test can + replace it -- pinning that a report which cannot be built becomes a stated 500 rather than a + 200 with an empty payload, which is the failure mode this whole envelope exists to prevent.""" + from keel.commands.status import gather_status + + repo = open_repo(cfg.db_path) + try: + return gather_status(repo, load_config(cfg.config_path), now_ts=now_ts) + finally: + close_repo(repo) + + +def read_config(cfg: ServeConfig, _query: Query, _state: Any, _now_ts: int) -> dict[str, Any]: + """The running build. Reads no deployment and no database -- it describes the BINARY that is + answering, which is why it still has data to serve on a machine with nothing set up.""" + return payload.config_payload(cfg.build_info, describe=cfg.build) + + +def read_status(cfg: ServeConfig, _query: Query, _state: Any, now_ts: int) -> dict[str, Any]: + return payload.status_payload(_status_report(cfg, now_ts)) + + +def read_setup(cfg: ServeConfig, _query: Query, state: Any, _now_ts: int) -> dict[str, Any]: + """The first-run checklist -- the ONE deployment-reading endpoint that must answer when there + is no deployment, because it is the thing that says how to make one. `needs_database=False` + for that reason, and `server.needs_database` serves the same page in HTML for the same one. + + `state` is the `DeploymentState` the envelope already read for `engine`, passed in rather than + re-inspected: one 3.6 ms probe per response, not two.""" + from keel.commands import jobs + from keel.commands.setup import ACTIONS, NOT_AUTOMATED_YET + + return payload.setup_payload( + state, actions=ACTIONS, not_automated=NOT_AUTOMATED_YET, job=jobs.status() + ) + + +def read_activity(cfg: ServeConfig, query: Query, _state: Any, _now_ts: int) -> dict[str, Any]: + """The engine's own log, scoped. + + An unrecognised `?scope=` is NORMALISED rather than refused, unlike an unrecognised `?sort=`, + and the difference is not an inconsistency. `normalise_scope` is the activity SERVICE's own + function with the CLI's own default behind it, so refusing here would mean the browser and the + terminal disagreeing about the same input -- and the resolved value is echoed back in + `data.scope`, so a client can see that its input was changed. A sort column has no service, no + default and no echo that would make a silent substitution visible, so it is refused.""" + from keel.commands.activity import ( + apply_scope, + feed_from_lines, + normalise_scope, + read_log_window, + resolve_log_path, + ) + + scope = normalise_scope(_first(query, "scope")) + path = resolve_log_path(load_config(cfg.config_path)) + window = read_log_window(path) + feed = feed_from_lines(window.lines, source=str(path), truncated=window.truncated) + if window.status != "ok" and feed.status == "empty": + # A read that failed and a window that held nothing are different facts; the reader's + # status is the more specific one and must not be flattened into "empty". + feed = feed_from_lines((), source=str(path)) + return payload.activity_payload(apply_scope(feed, scope, now_ts=time.time())) + + +def read_insights(cfg: ServeConfig, _query: Query, _state: Any, now_ts: int) -> dict[str, Any]: + """The per-rule track records and the promotion-gate distances. + + The journal the HTML `/insights` page renders below these is `/api/journal` instead of a second + table here. One sortable collection per endpoint keeps `?sort=` unambiguous without a + `?table=` beside it, and it gives the journal somewhere to carry its own `?limit=` -- the cap + the HTML page apologises for in a comment ("a cap, not a paginator").""" + from keel.commands.insights import build_insights_report + + repo = open_repo(cfg.db_path) + try: + config = load_config(cfg.config_path) + report = build_insights_report(repo, config, _status_report(cfg, now_ts), now_ts) + finally: + close_repo(repo) + return payload.insights_payload(report) + + +def read_journal(cfg: ServeConfig, query: Query, _state: Any, now_ts: int) -> dict[str, Any]: + from keel.commands.insights import build_journal_report + + limit = _journal_limit(query) + repo = open_repo(cfg.db_path) + try: + report = build_journal_report( + repo, _status_report(cfg, now_ts), now_ts, limit=limit + ) + finally: + close_repo(repo) + return payload.journal_payload(report) + + +def read_rules(cfg: ServeConfig, _query: Query, _state: Any, _now_ts: int) -> dict[str, Any]: + repo = open_repo(cfg.db_path) + try: + rows = repo.get_rules(None) + finally: + close_repo(repo) + return payload.rules_payload(rows) + + +def read_venues(_cfg: ServeConfig, _query: Query, _state: Any, _now_ts: int) -> dict[str, Any]: + from keel.commands.brokers import list_installed_brokers + + return payload.venues_payload(list_installed_brokers()) + + +def read_gates(_cfg: ServeConfig, _query: Query, _state: Any, _now_ts: int) -> dict[str, Any]: + """Read from `keel.capabilities`, which is a pure declaration -- no config, no database, no + network. It describes the binary that is serving the response.""" + from keel.capabilities import CAPABILITIES, GATES + + return payload.gates_payload(GATES, CAPABILITIES) + + +# -- the route table ------------------------------------------------------------------------------- + +#: A parsed query string, as `urllib.parse.parse_qs` returns it. +Query = dict[str, list[str]] + +Reader = Callable[["ServeConfig", Query, Any, int], "dict[str, Any]"] + + +@dataclass(frozen=True) +class ApiRoute: + """One `GET /api/*` endpoint. + + `html_route` names the rendered page this endpoint carries the data for, so + `test_the_api_routes_cover_every_html_route_that_reads` can compare the two tables mechanically + -- an HTML route added without a JSON counterpart fails that test rather than being discovered + by a client that cannot render it. `""` means there is no rendered counterpart at all, which + today is only `/api/config`. + + `needs_database` is the JSON counterpart of `server.needs_database`: rather than serving the + setup checklist in place of a broken page, it answers `data: null` with `engine: "stopped"`, + which is the same statement in the shape a client can act on. + + `collection` and `sortable` are the sort surface. `collection` is the key of the one list in + `data` that `?sort=` orders; `sortable` is the closed set of columns it may be ordered by, and + it is a hand-written list on purpose -- deriving it from the first row would make the API's + answer to `?sort=x` depend on whether any rows exist, which is the sort of behaviour that shows + up only on an empty deployment. + """ + + html_route: str + read: Reader + needs_database: bool = True + collection: str = "" + sortable: tuple[str, ...] = field(default=()) + + +#: THE READ SURFACE, in full. Every one of the rendered routes in `server.ROUTES` appears here as +#: its `html_route` except `/glossary`, which gets no counterpart: it becomes an outbound +#: keeltrading.com link in #539 and `render_glossary` is deleted in #540, so an `/api/glossary` +#: would be a surface built in order to be removed -- and any client written against it would break +#: on the release that removes it. +API_ROUTES: dict[str, ApiRoute] = { + "/api/config": ApiRoute( + html_route="", read=read_config, needs_database=False + ), + "/api/status": ApiRoute( + html_route="/", + read=read_status, + collection="open_positions", + # The one collection on this payload with money in it. `rule_counts` is already ordered by + # status to match `render_human`, `data_freshness` follows the config's product order and + # `live_rules` the ledger's -- three intrinsic orders that a display sort would destroy + # rather than improve. + sortable=( + "id", + "product_id", + "rule_name", + "qty", + "entry_price", + "opened_at", + "bracket", + ), + ), + "/api/setup": ApiRoute( + html_route="/setup", + read=read_setup, + # No sortable collection: the steps are in RUNBOOK order, and that order is the + # information. A checklist sorted by title is a checklist you cannot work down. + needs_database=False, + ), + "/api/activity": ApiRoute( + html_route="/activity", + read=read_activity, + collection="cycles", + sortable=( + "key", + "cycle_id", + "started_at", + "ended_at", + "mode", + "signals", + "blocked", + "entered", + "exited", + "errors", + "events_dropped", + ), + ), + "/api/insights": ApiRoute( + html_route="/insights", + read=read_insights, + collection="rules", + sortable=( + "rule_name", + "status", + "promotion_class", + "n_trades", + "win_rate", + "avg_win", + "avg_loss", + "realized_rr", + "expectancy", + "profit_factor", + "max_drawdown", + ), + ), + "/api/journal": ApiRoute( + html_route="/insights", + read=read_journal, + collection="entries", + sortable=( + "closed_at", + "opened_at", + "rule_name", + "product_id", + "qty", + "entry_fill", + "exit_fill", + "pnl", + "fees", + "r_multiple", + "outcome", + ), + ), + "/api/rules": ApiRoute( + html_route="/rules", + read=read_rules, + collection="rules", + sortable=("id", "kind", "status", "created_at", "promoted_at", "demoted_at"), + ), + "/api/venues": ApiRoute( + html_route="/venues", + read=read_venues, + needs_database=False, + collection="venues", + sortable=("name", "venue", "deployment", "package_version"), + ), + "/api/gates": ApiRoute( + html_route="/gates", + read=read_gates, + needs_database=False, + # No sortable collection: a gate's rows are the actions it covers, nested one level down, + # and `?sort=` names a column of ONE top-level list. Sorting the gates themselves would + # order a tuple that has one member. + ), +} + +#: `/api/*` -> the rendered route it carries the data for. Derived rather than written twice, so +#: the two cannot disagree. +HTML_ROUTE_FOR: dict[str, str] = {path: route.html_route for path, route in API_ROUTES.items()} + + +# -- the query surface ----------------------------------------------------------------------------- + + +def _first(query: Query, name: str) -> str: + """The first value for `name`, or `""`. Repeated parameters take the first rather than the last + or a refusal: `?sort=a&sort=b` is a client bug, not an attack, and picking one deterministically + is a better answer than a 400 the client's author will read as "sorting is broken".""" + values = query.get(name) or [""] + return values[0] + + +def _journal_limit(query: Query) -> int: + """How many journal rows to build, from `?limit=`. + + Bounded at both ends. A zero or a negative would reach `build_journal_report` as a slice bound + and answer an empty journal that looks like an empty ledger, and an unbounded upper end is a + caller choosing how much of this machine's memory to spend.""" + raw = _first(query, "limit") + if not raw: + return DEFAULT_JOURNAL_LIMIT + try: + value = int(raw) + except ValueError as exc: + raise ApiRefusal( + 400, "Bad limit", f"limit={raw!r} is not a whole number." + ) from exc + if value < 1 or value > MAX_JOURNAL_LIMIT: + raise ApiRefusal( + 400, + "Bad limit", + f"limit={raw!r} is outside 1..{MAX_JOURNAL_LIMIT}.", + ) + return value + + +def _sort_request(path: str, route: ApiRoute, query: Query) -> tuple[str, bool]: + """`(column, descending)` from `?sort=` and `?dir=`, or a refusal. + + **Two parameters rather than one signed token.** `?sort=-expectancy` was the alternative and it + needs a grammar -- what a leading `-` means, what happens to a column whose name starts with + one, how a client strips it before comparing against `sort.columns`. Two orthogonal facts, two + parameters, no parsing beyond a table lookup. + + **An unknown column is REFUSED, never ignored.** Silently ignoring one is how a client ships a + sort header that does nothing and nobody notices for a release; the refusal names the columns + that do exist, so the fix is in the response that reported the problem. + """ + column = _first(query, "sort") + direction = _first(query, "dir") or DIRECTIONS[0] + if direction not in DIRECTIONS: + raise ApiRefusal( + 400, + "No such direction", + f"dir={direction!r} is not a direction; use one of: {', '.join(DIRECTIONS)}.", + ) + if not column: + return "", False + if not route.sortable: + raise ApiRefusal( + 400, + "Nothing to sort", + f"{path} serves no sortable table -- its rows carry an intrinsic order.", + ) + if column not in route.sortable: + raise ApiRefusal( + 400, + "No such column", + f"sort={column!r} is not a column of {path}; sortable columns are: " + f"{', '.join(route.sortable)}.", + ) + return column, direction == DIRECTIONS[1] + + +def _sort_echo(route: ApiRoute, column: str, descending: bool) -> dict[str, Any] | None: + """What was sorted, and what could be. + + `columns` is echoed even when nothing was sorted, so #537 renders sortable headers by reading + the response rather than by holding a second copy of the list above -- a column added there + reaches the interface without a second edit, and one removed there cannot leave a header that + sorts by nothing.""" + if not route.sortable: + return None + return { + "column": column, + "direction": DIRECTIONS[1] if descending else DIRECTIONS[0], + "columns": list(route.sortable), + } + + +def _apply_sort( + data: dict[str, Any], route: ApiRoute, column: str, descending: bool +) -> dict[str, Any]: + rows = data.get(route.collection) + if not isinstance(rows, list): # pragma: no cover - a declared collection is always a list + return data + ordered = payload.order_rows(rows, column=column, descending=descending) + return {**data, route.collection: ordered} + + +# -- the response ---------------------------------------------------------------------------------- + + +def respond(cfg: ServeConfig, path: str, query: Query) -> tuple[int, dict[str, Any]]: + """One `GET /api/*`, as `(status, document)`. Never raises; never renders. + + The order of the steps is the contract: + + 1. **Unmapped path first**, so an unknown endpoint costs no database read at all. + 2. **The deployment probe**, which fills `engine` on every success. A probe that itself fails + reports a STOPPED engine rather than a 500: `inspect` is documented as read-only and total, + and if it ever is not, the honest reading of "we could not tell whether keel is set up" is + the same one the rest of this file takes -- do not show figures. + 3. **The query surface**, before the read. A bad `?sort=` should not cost a report build, and + more importantly it should not be able to arrive AFTER one and discard it. + 4. **The read**, then the sort, then the envelope. + + A stopped engine answers **200**, not a 4xx. #538's service worker and #536's `fetch` wrapper + both read a non-ok status as "the server is unreachable", which would put a first-run user into + an outage state when their actual position is that they have not set anything up yet -- and the + two need different words on screen. The request succeeded; the answer is that there is nothing + to report. + """ + now_ts = int(time.time()) + route = API_ROUTES.get(path) + if route is None: + return 404, payload.error_envelope( + now_ts, + status=404, + title="No such endpoint", + detail=f"Nothing is served at {path}.", + ) + + try: + state: Any = deployment_state(cfg) + running = bool(state.has_usable_database) + except Exception: # pragma: no cover - `inspect` is total; this is the belt to its braces + state, running = None, False + + try: + column, descending = _sort_request(path, route, query) + except ApiRefusal as refusal: + return refusal.status, payload.error_envelope( + now_ts, status=refusal.status, title=refusal.title, detail=refusal.detail + ) + + if route.needs_database and not running: + return 200, payload.envelope( + now_ts, + running=False, + data=None, + sort=_sort_echo(route, column, descending), + ) + + try: + data = route.read(cfg, query, state, now_ts) + except ApiRefusal as refusal: + return refusal.status, payload.error_envelope( + now_ts, status=refusal.status, title=refusal.title, detail=refusal.detail + ) + except Exception as exc: + # A broken report is a stated failure, never a 200 with an empty payload -- that shape is + # precisely the "blank view, or worse, stale figures" this envelope exists to prevent. The + # detail carries the exception's TYPE and message and no traceback: a traceback in a + # browser is a stack of file paths from someone else's machine. + return 500, payload.error_envelope( + now_ts, + status=500, + title="That report could not be built", + detail=f"{type(exc).__name__}: {exc}", + ) + + if column: + data = _apply_sort(data, route, column, descending) + return 200, payload.envelope( + now_ts, + running=running, + data=data, + sort=_sort_echo(route, column, descending), + ) + + +def refusal_document(status: int, title: str, detail: str) -> dict[str, Any]: + """A refusal raised BEFORE routing -- a failed host check, a missing session cookie. + + Lives here rather than in the handler so that every `/api/*` body, admitted or not, is built by + one function against one contract. It carries no `engine`, and that is the reason the success + and failure documents are told apart by HTTP status rather than by a field: these refusals + happen before admission, and filling in `engine` would mean an unauthenticated request reading + the deployment state off disk.""" + return payload.error_envelope(int(time.time()), status=status, title=title, detail=detail) + + +def sortable_columns() -> Mapping[str, Sequence[str]]: + """The declared sort surface, for a test to read rather than restate.""" + return {path: route.sortable for path, route in API_ROUTES.items() if route.sortable} diff --git a/keel/web/payload.py b/keel/web/payload.py index 72889c0..5c63fa3 100644 --- a/keel/web/payload.py +++ b/keel/web/payload.py @@ -78,6 +78,18 @@ would mean accepting that branch on the client, which is the thing this contract exists to remove. +WHAT #534 ADDED, AND WHY IT IS HERE RATHER THAN IN THE ROUTING LAYER. Three things: the +`envelope`/`error_envelope` pair that wraps every `GET /api/*` response, the `engine_state` word +that lets a client say "keel isn't running" instead of rendering a blank view, and `order_rows`, +the server-side sort. All three touch the WIRE VOCABULARY -- the closed `state` words, the +no-exponent guarantee that makes `Field.value` re-parseable, the rule that a number crosses as a +string -- and a second file holding half of that vocabulary is how the two halves drift. The +routing layer's job is deciding WHICH rows and WHICH column from a query string; the ordering +itself sits next to the `_plain` that wrote the strings being ordered. + +Note what `order_rows` does NOT do: it does not add the numeric `sort` companion field described +above. It re-parses `Field.value`, which is exactly what that value is for. + WHAT IS *NOT* A FIELD. Identifiers and enum words -- `product_id`, `rule_name`, `mode`, `as_of` -- cross as bare JSON strings. They carry no precision hazard, no rounding decision and no judgement, and wrapping them would be ceremony rather than contract. Bare JSON *numbers* @@ -89,7 +101,7 @@ import json import time -from collections.abc import Mapping +from collections.abc import Mapping, Sequence from decimal import Decimal, InvalidOperation from typing import TYPE_CHECKING, Any, TypedDict @@ -1025,3 +1037,518 @@ def activity_payload(feed: ActivityFeed) -> dict[str, Any]: ), "cycles": [_cycle_payload(c) for c in feed.cycles], } + + +# -- the envelope (#534) ------------------------------------------------------------------------- +# +# Every `GET /api/*` success is wrapped in the same four keys, so #536's single `fetch` wrapper +# needs no per-endpoint branch and no knowledge of which endpoints can report a dead engine. + + +#: The closed `engine` vocabulary. Two words, and deliberately not three. +#: +#: A third word for "the report raised" was drafted and dropped. The CLIENT behaviour required by +#: a stopped engine and by an unbuildable report is identical -- show no figures, say why -- so a +#: third word would force every view to write three branches to get two behaviours, and the +#: difference between the two is already carried by the HTTP status and by `error.detail`. Add one +#: the day a client would DO something different with it. +ENGINE_STATES: frozenset[str] = frozenset({"running", "stopped"}) + +RUNNING = "running" +STOPPED = "stopped" + +#: What `engine: "stopped"` says when the caller has nothing more specific. The wording has to be +#: true both for a machine with no deployment on it and for one whose report could not be built, +#: because those are the two ways this value is reached. +_STOPPED_DISPLAY = "keel isn't running here — there is no deployment to read" +_RUNNING_DISPLAY = "keel is set up on this machine" + + +def engine_state(*, running: bool, detail: str = "") -> Field: + """Whether there is a keel deployment behind this response, as a judged field. + + **What this does NOT answer, on purpose: "did the agent run recently".** That question needs a + THRESHOLD -- how many hours of silence is too many -- and this file holds no thresholds by + design (`_freshness_payload` refuses the same temptation and says so). The evidence for it is + already on the wire in two places a client can render without arithmetic: `data_freshness` + carries each product's candle age in the CLI's own words, and the activity feed carries + `last_cycle_before_scope`, which exists precisely so an empty view can say *when keel last + ran*. The day a report builder holds an agent heartbeat WITH its staleness verdict, this + vocabulary gains a third word and the verdict is copied here, never computed here. + + `warn` rather than `bad` for a stopped engine: on the commonest path to this value nothing is + broken at all -- it is a first run, and the correct next action is the setup checklist, not an + incident. A caller that knows better passes its own `detail`. + """ + if running: + return {"value": RUNNING, "display": _RUNNING_DISPLAY, "state": GOOD} + return {"value": STOPPED, "display": detail or _STOPPED_DISPLAY, "state": WARN} + + +def envelope( + now_ts: float | int | None, + *, + running: bool, + data: dict[str, Any] | None, + detail: str = "", + sort: dict[str, Any] | None = None, +) -> dict[str, Any]: + """One `GET /api/*` success. + + `data` is `null` -- never `{}` -- when the endpoint could not be answered because there is no + deployment. An empty object is a payload in which every figure is missing, and a view handed + one renders zeros; `null` cannot be rendered by accident, which is the whole requirement. + + The key set is CONSTANT across every endpoint and every state, `sort` included on endpoints + that sort nothing. A client that has to test whether a key is present is branching on payload + SHAPE, which Rule 3 rejects for `state` and which is rejected here for the same reason. + + `as_of` is the instant this response was built, and it is present even when `data` is not -- + that pairing is the requirement: a client showing "keel isn't running" should be able to say + since when it was looking. + """ + return { + "as_of": iso(now_ts), + "engine": engine_state(running=running, detail=detail), + "data": data, + "sort": sort, + } + + +def error_envelope( + now_ts: float | int | None, *, status: int, title: str, detail: str +) -> dict[str, Any]: + """One `GET /api/*` refusal or failure. + + **The discriminator between this document and `envelope` is the HTTP STATUS, deliberately, and + not a field.** A uniform envelope with a nullable `error` was the first shape and it does not + survive contact with admission: most refusals happen BEFORE the session cookie is checked, and + filling in `engine` there would mean an unauthenticated request reading the deployment state + off disk. `res.ok` is a check every `fetch` client already makes. + + `status` crosses as a STRING like every other number here. It would survive JSON's number type + intact, and that is exactly the argument `count` refuses: a payload with "only a few" numbers + in it needs a per-field rule about which ones, and that rule is what rots. + + `data` is present and `null` so that a client reading `.data` on any response gets a value + rather than `undefined` -- the key set stays constant across the two documents for the same + reason it stays constant across endpoints. + """ + return { + "as_of": iso(now_ts), + "data": None, + "error": {"status": str(status), "title": title, "detail": detail}, + } + + +# -- ordering (#534) ----------------------------------------------------------------------------- +# +# Server-side sort, ordered with `Decimal`. The spec's own words: "On loopback the round trip is +# sub-millisecond, so there is nothing to optimise and no client arithmetic to audit." + + +def _cell_text(row: Mapping[str, Any], column: str) -> str: + """The exact, ungrouped text a row carries in `column`, or `""` for nothing to order by. + + Reads `Field.value` for a field and the string itself for a bare identifier, which is exactly + what `Field.value` is documented to be: "machine input (exact, ungrouped, `Decimal`-parseable + for figures and ISO-8601 for instants)". A list, an object or a `None` yields `""` and sorts + with the absent rows -- an endpoint should not be declaring such a column sortable, and the + routing layer refuses a column it does not declare, but a total function here means a mistake + there costs an ordering rather than a 500. + """ + cell = row.get(column) + if isinstance(cell, Mapping): + raw = cell.get("value", "") + return raw if isinstance(raw, str) else "" + if isinstance(cell, str): + return cell + return "" + + +def _finite_decimal(text: str) -> Decimal | None: + """`text` as a finite `Decimal`, or `None` for anything that cannot be ordered as a number. + + Non-finite is `None` on purpose, and it is not hypothetical: `stringify` renders a + `Decimal("NaN")` sentinel as `"NaN"`, and `Decimal("NaN")` PARSES. Ordering by it would make + the result depend on the comparison order the sort happened to use, because NaN compares false + to everything including itself -- so a column containing one is ordered as text instead, as a + whole column, and the ordering stays deterministic. + """ + if not text: + return None + try: + candidate = Decimal(text) + except (InvalidOperation, ValueError, TypeError): + return None + return candidate if candidate.is_finite() else None + + +def _ordering_key(pair: tuple[Any, Mapping[str, Any]]) -> Any: + """The first element of a (key, row) pair. + + Sorting is keyed on this rather than on the pair, because the second element is a `dict` and + comparing two of them raises the moment two keys tie -- which is exactly the case a + tie-breaking sort reaches.""" + return pair[0] + + +def order_rows( + rows: Sequence[Mapping[str, Any]], *, column: str, descending: bool = False +) -> list[Mapping[str, Any]]: + """`rows` ordered by `column`, with `Decimal` wherever the column is numeric. + + **Why `Decimal` and not `float`, with the number in it.** Two ERC-20 quantities one wei apart + -- `0.100000000000000001` and `0.100000000000000002`, both legal at an 18-decimal base + increment -- map to the SAME IEEE-754 double, because adjacent doubles near 0.1 are about + 1.4e-17 apart. A float-keyed sort cannot separate them, so their order becomes whatever order + they arrived in; `Decimal` orders them. The same collapse takes the last cent of a large + notional. `tests/web/test_api.py` asserts that disagreement directly, before the endpoint test + relies on it, so the endpoint test cannot be green for the trivial reason that sorting sorts. + + **The SERIALISED rows are sorted, not the source dataclasses.** The alternative -- ordering + `report.open_positions` before serialising -- needs a map from every payload key back to the + report attribute it came from, held in a second file, and that map rots the first time a key + is renamed here. Sorting the output means `?sort=qty` names exactly the field the client can + see in the response, and `_plain` guarantees `value` re-parses to the `Decimal` it was written + from, for any input, so the round trip loses nothing. + + **Numeric or text is decided per COLUMN, never per row.** A per-row decision would put a + `Decimal` and a `str` in one comparison, which raises, and a per-row fallback would make the + order depend on which values happened to look like numbers. So: numeric only when every + present value parses finite, text otherwise. Instants land in the text branch and are still + chronological, because `moment().value` is fixed-width ISO-8601 UTC. + + **An absent value sorts last in BOTH directions.** `None` means "not recorded" and `0` means + "recorded as zero"; collapsing the first into the second is the shape of the always-passing fee + rail (#198). Giving an absent cell a numeric key would make it the largest thing in a + descending sort, which is that same collapse wearing a different hat. + """ + present: list[tuple[str, Mapping[str, Any]]] = [] + missing: list[Mapping[str, Any]] = [] + for row in rows: + text = _cell_text(row, column) + if text: + present.append((text, row)) + else: + missing.append(row) + + figures = [_finite_decimal(text) for text, _row in present] + keyed: list[tuple[Any, Mapping[str, Any]]] + if all(figure is not None for figure in figures): + keyed = [(figure, row) for figure, (_text, row) in zip(figures, present, strict=True)] + else: + keyed = [(text.casefold(), row) for text, row in present] + + # `list.sort` is stable in both directions, so rows that tie keep the order the report built + # them in -- which for `open_positions` is FIFO, the attribution order a later exit uses, and + # therefore an order that must not be scrambled by a display sort. + keyed.sort(key=_ordering_key, reverse=descending) + return [row for _key, row in keyed] + missing + + +# -- config (#534) ------------------------------------------------------------------------------- + + +def config_payload(build: Any, *, describe: str = "") -> dict[str, Any]: + """The running build, for the two consumers that need it by name. + + `version` is what #539 carries as `?v=` on a documentation link, so version skew between the + engine and the docs is visible rather than silent. `build` is `full_version` -- the version + bound to the commit -- and is what #538 keys its service-worker cache name to, because a + version alone is ambiguous (many commits share one between bumps) and a cache key that does + not move when the code does is exactly how an upgraded engine gets met by a stale shell + holding an older contract. + + `build` arrives already RESOLVED, from `ServeConfig`, rather than being looked up here. + `keel.version.build_info()` shells out to git twice, and an endpoint a service worker polls + must not fork a subprocess to answer. `None` -- no package metadata and no git, the same + environment in which the page footer is already empty -- reports absent rather than inventing + a version: a cache key of `""` still keys a cache, whereas a wrong version in a `?v=` link + would send a reader to documentation for a build that does not exist. + """ + return { + "version": str(getattr(build, "version", "") or ""), + "build": str(getattr(build, "full_version", "") or ""), + "commit": str(getattr(build, "commit", "") or ""), + "source": str(getattr(build, "source", "") or ""), + "describe": describe, + # keel's central honesty signal, and the one judgement this payload carries: `False` means + # the running code corresponds to no commit (a dirty tree, or no idea), and `keel.version` + # treats saying so as more important than looking tidy. + "reproducible": flag( + None if build is None else bool(getattr(build, "is_reproducible", False)), + on="reproducible", + off="NOT reproducible — this build corresponds to no commit", + on_state=GOOD, + off_state=WARN, + ), + } + + +# -- setup (#534) -------------------------------------------------------------------------------- + +#: `StepKind`, judged -- and the judgement is "can a machine do this for you", never "is something +#: wrong". `judgement` warns because a human must decide it and keel must never decide it for +#: them; `off_venue` warns because keel can neither perform nor VERIFY it, and `render.py`'s own +#: note is the reason: "a green check that verifies nothing turns an open risk into a false +#: assurance". +_STEP_KIND_STATES: Mapping[str, str] = { + "mechanical": NEUTRAL, + "operator_input": NEUTRAL, + "judgement": WARN, + "off_venue": WARN, +} + +#: `JobStatus.state`, judged. +_JOB_STATES: Mapping[str, str] = {"running": WARN, "done": GOOD, "failed": BAD} + + +def _step_payload(item: Any) -> dict[str, Any]: + """One checklist step. + + `done` is three-valued and stays three-valued: `None` means "could not be determined", which is + NOT `False`. An unreadable database is not an unseeded one, and reporting it as incomplete + would send an operator to re-run a step that may already be done -- `StepState.done`'s own + comment. `flag(None)` renders it absent, so the browser shows a dash where the CLI shows + `[?]`, and neither claims to know.""" + return { + "key": item.step.key, + "title": item.step.title, + "kind": label( + item.step.kind.value, state=_STEP_KIND_STATES.get(item.step.kind.value, NEUTRAL) + ), + "stage": item.step.stage.value, + "why": item.step.why, + "how": item.step.how, + "done": flag(item.done, on="done", off="outstanding", on_state=GOOD, off_state=WARN), + "detail": item.detail, + } + + +def _action_input_payload(field: Any) -> dict[str, Any]: + return { + "name": field.name, + "label": field.label, + "hint": field.hint, + # `secret=True` is the difference between a `password` input and a `text` one, and between + # a form that can be submitted safely and one that leaks its own contents into browser + # history. The client is told the answer rather than deriving it from the field's name. + "secret": flag(field.secret, on="never echoed back", off="shown as typed"), + # A closed set of answers, rendered with NOTHING pre-selected: an action that could fill in + # a field the operator left blank is one that could record something they never supplied. + "choices": list(field.choices), + } + + +def _action_payload(action: Any) -> dict[str, Any]: + return { + "key": action.key, + "title": action.title, + "detail": action.detail, + "needs_input": flag( + action.needs_input, on="needs your input", off="keel can do this unaided" + ), + "inputs": [_action_input_payload(field) for field in action.inputs], + } + + +def _job_payload(job: Any) -> dict[str, Any]: + """A background setup job (`keel.commands.jobs`), as JSON. + + `int(job.elapsed_sec)` is the one integer conversion in this file outside `_gmt`, and it is on + SECONDS, which have no cent to lose -- the same argument Rule 6b's timestamp allowance makes. + `duration` takes whole seconds because `_human_age`, the CLI's own phrasing, does. A failure + stays in the payload rather than being cleared: the whole point of running something in the + background is that nobody was watching when it broke.""" + return { + "key": job.key, + "state": label(job.state, state=_JOB_STATES.get(job.state, NEUTRAL)), + "started_at": moment(job.started_ts), + "finished_at": moment(job.finished_ts), + "elapsed": duration(int(job.elapsed_sec)), + "running": flag(job.is_running, on="running", off="finished"), + "error": job.error or "", + # Newest last, unscrolled, exactly as the CLI prints them: an operator who has run `keel + # fetch` in a terminal should recognise what they are looking at rather than have to learn + # a second vocabulary for the same thing. + "lines": list(job.lines), + } + + +def setup_payload( + state: Any, + *, + actions: Sequence[Any] = (), + not_automated: Mapping[str, str] | None = None, + job: Any = None, +) -> dict[str, Any]: + """`keel.commands.setup.inspect`'s `DeploymentState`, as JSON. + + **No CSRF token, and that is the point rather than an omission.** `render_setup` takes one + because it emits `
`; this issue ships reads only, and minting a live write + credential into a GET response would put it into every cached copy, every proxy log and every + paste of "here is what the API returned". The token is `csrf_token(session)` and it stays where + the write is. + + `actions` and `not_automated` are read from `keel.commands.setup`'s own closed registries and + copied, never filtered here: an action appears only where the registry carries one, which is + what stops "attest this asset" appearing because somebody edited a front-end. + """ + from keel.commands.setup import Stage + + return { + "root": str(state.root), + "config_path": str(state.config_path), + "db_path": str(state.db_path), + "is_new": flag( + state.is_new, + on="nothing set up here yet", + off="a deployment exists here", + on_state=WARN, + off_state=NEUTRAL, + ), + "has_usable_database": flag( + state.has_usable_database, + on="readable", + off="no schema here yet", + on_state=GOOD, + off_state=WARN, + ), + # `ready_for(LIVE)` is deliberately absent: `OFF_VENUE` steps can never be OBSERVED, so the + # answer would be a permanent `False` that says nothing about the deployment. The honest + # last word on going live belongs to the operator who checked the venue dashboard. + "ready_for_paper": flag( + state.ready_for(Stage.PAPER), + on="ready to run in paper", + off="not yet ready for paper", + on_state=GOOD, + off_state=WARN, + ), + "next_step": state.next_step.step.key if state.next_step is not None else "", + "steps": [_step_payload(item) for item in state.states], + "actions": [_action_payload(action) for action in actions], + # Mechanical steps deliberately NOT offered as one-click actions, and why. Carried as data + # rather than omitted silently, so a gap is visible to the next person rather than looking + # like an oversight -- and empty is a fine value. + "not_automated": [ + {"key": key, "why": why} for key, why in sorted((not_automated or {}).items()) + ], + "job": _job_payload(job) if job is not None else None, + } + + +# -- rules (#534) -------------------------------------------------------------------------------- + + +def _rule_row_payload(row: Mapping[str, Any]) -> dict[str, Any]: + return { + # An identifier, so a bare string -- and one that still sorts numerically, because + # `order_rows` reads a column as `Decimal` when every value in it parses as one. + "id": str(row.get("id", "")), + "kind": str(row.get("kind") or ""), + "status": label(str(row.get("status") or "")), + "created_at": moment(row.get("created_at")), + "promoted_at": moment(row.get("promoted_at")), + "demoted_at": moment(row.get("demoted_at")), + # Operator-supplied and open-ended (any rule kind may invent its own), so they cross as + # strings for the same reason `ActivityEvent.fields` does. + "params": { + str(key): stringify(value) for key, value in sorted((row.get("params") or {}).items()) + }, + } + + +def rules_payload(rows: Sequence[Mapping[str, Any]]) -> dict[str, Any]: + """`Repository.get_rules(None)`'s rows, as JSON. + + Read-only, and the page it mirrors says so out loud: promotion happens in the CLI, behind the + TTY gate, and nothing this API serves can change a rule's status.""" + return {"rules": [_rule_row_payload(row) for row in rows]} + + +# -- venues (#534) ------------------------------------------------------------------------------- + + +def _venue_payload(info: Any) -> dict[str, Any]: + """One installed adapter's DECLARED capabilities. + + What the adapter says it can do, never an inference about the operator's keys: a row here is + not a claim that the venue is configured or reachable (#233). An adapter that failed to + construct still gets a row, with the failure judged `bad` -- a missing row would read as "not + installed", which is a different fact and a worse one to be wrong about.""" + return { + "name": info.name, + "venue": info.venue, + "deployment": info.deployment, + "asset_classes": list(info.asset_classes), + "supported_orders": list(info.supported_orders), + "quote_currencies": list(info.quote_currencies), + "supported_data_feeds": list(info.supported_data_feeds), + "declared_endpoints": list(info.declared_endpoints), + "package_version": info.package_version or "", + "preview": info.preview, + "session_bound": flag(info.session_bound, on="session-bound", off="stateless"), + "supports_fee_summary": flag( + info.supports_fee_summary, on="fee summary", off="no fee summary" + ), + # Not `absent()` when there is no error: absent means "not recorded", and "constructed + # cleanly" is a positive observation. Spelling it out is the same choice + # `_subscription_payload` makes for an unlimited cap. + "error": ( + label(info.error, state=BAD) + if info.error + else label("none", display="constructed cleanly", state=GOOD) + ), + } + + +def venues_payload(infos: Sequence[Any]) -> dict[str, Any]: + """`keel.commands.brokers.list_installed_brokers`'s rows, as JSON.""" + return {"venues": [_venue_payload(info) for info in infos]} + + +# -- gates (#534) -------------------------------------------------------------------------------- + + +def _capability_payload(capability: Any) -> dict[str, Any]: + return { + "surface": capability.surface, + "invocation": capability.invocation, + "increases": capability.increases, + "call_site": f"{capability.module}.{capability.function}", + "mirrors": ( + f"{capability.mirrors[0]}.{capability.mirrors[1]}" if capability.mirrors else "" + ), + } + + +def gates_payload(gates: Sequence[Any], capabilities: Sequence[Any]) -> dict[str, Any]: + """`keel/capabilities.py`'s declaration, as JSON. + + A pure declaration -- no config, no database, no network. It describes the BINARY that is + answering, which is why this endpoint has data to serve on a machine with no deployment on it. + + **No `count` of covered actions**, unlike the HTML page's `esc(len(covered))`. Rule 6e of + `test_console_thinness.py` bans `len()` in this module, because a count on the wire must be one + the report already holds -- and `Gate` holds none. Unlike `JournalReport.shown_count` there is + no report builder to add it to either, `keel/capabilities.py` being a declaration rather than a + report. A client renders `actions.length`, which is a list length in the language that owns the + list, not a figure this layer invented. + """ + return { + "gates": [ + { + "name": gate.name, + "evidence": gate.evidence, + "fails_closed_against": gate.fails_closed_against, + "implementation": gate.implementation, + "actions": [ + _capability_payload(capability) + for capability in capabilities + if capability.gate == gate.name + ], + } + for gate in gates + ] + } diff --git a/keel/web/server.py b/keel/web/server.py index 1e306d5..2ef60ca 100644 --- a/keel/web/server.py +++ b/keel/web/server.py @@ -1,14 +1,20 @@ -"""The loopback HTTP server behind `keel serve` -- routing, one bounded read per page, no writes. - -**Why the standard library and not a web framework.** This surface is six read-only pages served -to one person on one machine. FastAPI/uvicorn would bring a dependency subtree (pydantic, -starlette, anyio, h11, ...) into a wheel that today depends on `click` and its own workspace -siblings -- and D5 has to freeze that tree into a signed, notarised app bundle, where every -dynamic import is a hook to write and every megabyte is download the user waits through. It would -also enlarge the supply-chain surface of a project whose proposition is auditability, to buy -routing for six paths and a templating engine used zero times. `http.server` is the smaller, -more honest answer here, and it is genuinely the wrong answer the moment this serves more than -one local user -- at which point the framework, not this module, is the thing to reach for. +"""The loopback HTTP server behind `keel serve` -- routing, one bounded read per response. + +Four surfaces, and the split is the whole of this module's job: the rendered HTML pages in +`ROUTES`, the static assets under `staticfiles.STATIC_PREFIX` (#535), the JSON read API under +`API_PREFIX` (#534, routed by `keel/web/api.py`), and the one closed write surface under +`SETUP_ACTION_PREFIX` (#437). Each gets its own header set, because they need different values for +the SAME header rather than merely different extra ones. + +**Why the standard library and not a web framework.** This surface is a handful of read-only pages +and endpoints served to one person on one machine. FastAPI/uvicorn would bring a dependency +subtree (pydantic, starlette, anyio, h11, ...) into a wheel that today depends on `click` and its +own workspace siblings -- and D5 has to freeze that tree into a signed, notarised app bundle, where +every dynamic import is a hook to write and every megabyte is download the user waits through. It +would also enlarge the supply-chain surface of a project whose proposition is auditability, to buy +routing for a couple of dozen paths and a templating engine used zero times. `http.server` is the +smaller, more honest answer here, and it is genuinely the wrong answer the moment this serves more +than one local user -- at which point the framework, not this module, is the thing to reach for. **The write surface is a closed set, and that is a better guarantee than the one it replaced.** This handler used to implement `do_GET`/`do_HEAD` and nothing else, so a POST died in the stdlib. @@ -35,6 +41,7 @@ from __future__ import annotations import functools +import json import socket import sys import time @@ -45,7 +52,7 @@ from typing import Any from urllib.parse import parse_qs, quote, urlsplit -from keel.web import render, staticfiles +from keel.web import api, render, staticfiles from keel.web.security import ( SESSION_COOKIE, HostPolicy, @@ -83,7 +90,18 @@ class ServeConfig: token: str db_path: str config_path: str + #: The build identity as one human line, for the page footer: `keel 0.1.0+9f2c1a [checkout]`. build: str = "" + #: The same build as STRUCTURE -- a `keel.version.BuildInfo`, or `None` where one could not be + #: resolved. `/api/config` needs the version and the commit as separate fields (#538 keys a + #: service-worker cache to one, #539 puts the other in a `?v=`), and parsing them back out of + #: `build` would be a display string being read as data. + #: + #: Resolved ONCE by `serve_cmd`, never per request: `keel.version.build_info()` shells out to + #: git twice, and an endpoint a service worker polls must not fork a subprocess to answer. + #: `Any` rather than the real type for the same reason `api.load_config` returns `Any` -- this + #: module names service objects loosely so that importing `keel/web/` stays cheap. + build_info: Any = None @property def host_policy(self) -> HostPolicy: @@ -99,21 +117,12 @@ def url(self) -> str: # Each of these is a thin adapter over `keel/commands/*`. Nothing below computes anything: the # service layer returns a frozen report and `keel/web/render.py` turns it into HTML. That is the # seam `tests/commands/test_console_thinness.py` pins, now extended over this package. - - -def _open_repo(db_path: str) -> Any: - """A plain connection -- deliberately WITHOUT `migrate`. - - Every CLI command migrates on the way in, which is right for a command: it runs once, and a - schema behind the code is a thing to fix rather than to fail on. It is wrong here. These - pages auto-reload every 15 seconds, so migrating per request would have a view that calls - itself read-only take a write lock on the deployment database four times a minute -- against - a database the agent may be mid-cycle on. `ensure_schema` does it ONCE, at bind time, before - anything is served.""" - from keel.data.db import connect - from keel.data.repository import Repository - - return Repository(connect(db_path)) +# +# `open_repo`, `load_config`, `deployment_state` and `close_repo` moved to `keel/web/api.py` when +# the JSON endpoints arrived (#534), unchanged and with their reasoning intact. Both front-ends +# read keel through them, and a copy in each file would be two places deciding whether a view may +# migrate a live database. The dependency runs one way -- this module imports `api`, `api` imports +# nothing from this one at runtime -- so there is no cycle to reason about. def ensure_schema(db_path: str) -> None: @@ -141,22 +150,6 @@ def ensure_schema(db_path: str) -> None: conn.close() -def _load_config(config_path: str) -> Any: - """`load_config` only -- deliberately NOT `_common._load_cfg`, which also calls - `configure_logging` and `bind_venue`. Those are process-entry side effects; re-applying them - on every page load would have the web UI quietly reconfiguring the running deployment's - logging.""" - from keel.config import load_config - - return load_config(config_path) - - -def _deployment_state(cfg: ServeConfig) -> Any: - from keel.commands.setup import inspect - - return inspect(cfg.config_path, cfg.db_path) - - def page_setup(cfg: ServeConfig, query: dict[str, list[str]]) -> tuple[str, str, int | None]: from keel.commands import jobs from keel.commands.setup import ACTIONS, NOT_AUTOMATED_YET @@ -165,7 +158,7 @@ def page_setup(cfg: ServeConfig, query: dict[str, list[str]]) -> tuple[str, str, return ( "Setup", render.render_setup( - _deployment_state(cfg), + api.deployment_state(cfg), actions=ACTIONS, not_automated=NOT_AUTOMATED_YET, csrf=csrf_token(cfg.token), @@ -196,7 +189,7 @@ def needs_database( @functools.wraps(page) def guarded(cfg: ServeConfig, query: dict[str, list[str]]) -> tuple[str, str, int | None]: - if not _deployment_state(cfg).has_usable_database: + if not api.deployment_state(cfg).has_usable_database: # The full setup page, not a bare checklist: someone who lands here has nothing set # up, and the actions are the reason they are being shown this instead of a 500. return page_setup(cfg, query) @@ -208,12 +201,12 @@ def guarded(cfg: ServeConfig, query: dict[str, list[str]]) -> tuple[str, str, in def page_status(cfg: ServeConfig, _query: dict[str, list[str]]) -> tuple[str, str, int | None]: from keel.commands.status import gather_status - repo = _open_repo(cfg.db_path) + repo = api.open_repo(cfg.db_path) try: - config = _load_config(cfg.config_path) + config = api.load_config(cfg.config_path) report = gather_status(repo, config, now_ts=int(time.time())) finally: - _close(repo) + api.close_repo(repo) return "Status", render.render_status(report), _REFRESH_SEC @@ -227,7 +220,7 @@ def page_activity(cfg: ServeConfig, query: dict[str, list[str]]) -> tuple[str, s ) scope = normalise_scope((query.get("scope") or [""])[0]) - config = _load_config(cfg.config_path) + config = api.load_config(cfg.config_path) path = resolve_log_path(config) window = read_log_window(path) feed = feed_from_lines(window.lines, source=str(path), truncated=window.truncated) @@ -252,24 +245,24 @@ def page_insights(cfg: ServeConfig, _query: dict[str, list[str]]) -> tuple[str, from keel.commands.insights import build_insights_report, build_journal_report from keel.commands.status import gather_status - repo = _open_repo(cfg.db_path) + repo = api.open_repo(cfg.db_path) try: - config = _load_config(cfg.config_path) + config = api.load_config(cfg.config_path) now_ts = int(time.time()) status_report = gather_status(repo, config, now_ts=now_ts) insights = build_insights_report(repo, config, status_report, now_ts) journal = build_journal_report(repo, status_report, now_ts, limit=_JOURNAL_LIMIT) finally: - _close(repo) + api.close_repo(repo) return "Insights", render.render_insights(insights, journal), None def page_rules(cfg: ServeConfig, _query: dict[str, list[str]]) -> tuple[str, str, int | None]: - repo = _open_repo(cfg.db_path) + repo = api.open_repo(cfg.db_path) try: rows = repo.get_rules(None) finally: - _close(repo) + api.close_repo(repo) return "Rules", render.render_rules(rows), None @@ -313,10 +306,12 @@ def page_glossary(_cfg: ServeConfig, _query: dict[str, list[str]]) -> tuple[str, #: is no other way into this handler, and no other verb. SETUP_ACTION_PREFIX = "/setup/" -#: Reserved for #533/#534's JSON API and #536's fetch()-based client -- nothing is mapped under -#: it yet, so a POST here is a 404 like any other unmapped path. Named now because #535's third -#: CSRF layer (`X-Keel-Client`, checked in `_client_header_ok`) is scoped to it specifically: see -#: that method's docstring for why it must NOT also gate `SETUP_ACTION_PREFIX`. +#: The JSON API (#534). `GET` under this prefix routes through `keel/web/api.py`'s own table -- +#: reads only, one bounded read per endpoint. `POST` under it is unchanged from #535: it clears +#: `_api_client_header_ok` (the third CSRF layer, scoped to this prefix specifically -- see that +#: method's docstring for why it must NOT also gate `SETUP_ACTION_PREFIX`) and then meets the same +#: 404 every unmapped path gets, because there is still no JSON write surface and this issue added +#: none. API_PREFIX = "/api/" @@ -336,15 +331,6 @@ def run_setup_action(cfg: ServeConfig, key: str, form: dict[str, str]) -> Any: return action.run(Path(cfg.config_path), Path(cfg.db_path), values) -def _close(repo: Any) -> None: - conn = getattr(repo, "conn", None) - if conn is not None: - try: - conn.close() - except Exception: # pragma: no cover - a close that fails leaks nothing that matters - pass - - # -- the handler ------------------------------------------------------------------------------- #: Sent on every response, success or refusal. @@ -440,6 +426,34 @@ def _close(repo: Any) -> None: # wrong: forcing HTTPS at an address that was never issued a certificate. +#: The header set for `/api/*` (#534). The same three unconditional headers the static route +#: sends, plus the `no-store` that matters more here than anywhere else on this server. +#: +#: **`Cache-Control: no-store` is the layer BELOW the service worker's promise.** The design spec +#: routes `/api/*` as `NetworkOnly`, "no exceptions", because "opening the app to last week's +#: equity styled as current is worse than an error" -- but a service worker is a thing that may +#: not be installed, may have been unregistered, or may be a version behind. `no-store` on the +#: response means the browser's ordinary HTTP cache cannot hold an account balance either, whether +#: or not any worker is in the picture. +#: +#: **No CSP, deliberately**, for the reason `_STATIC_BASE_HEADERS` already records: CSP is a +#: response header with no defined meaning outside a browsing context, and `application/json` is +#: not one. `nosniff` is what carries the weight for this content type instead -- a JSON body a +#: browser is free to sniff as HTML is a stored-XSS primitive wearing a `Content-Type`. +_API_HEADERS: tuple[tuple[str, str], ...] = ( + ("X-Content-Type-Options", "nosniff"), + ("X-Frame-Options", "DENY"), + ("Referrer-Policy", "no-referrer"), + ("Cache-Control", "no-store, max-age=0"), +) + +#: What every `/api/*` response is labelled. `charset=utf-8` explicitly, even though JSON's +#: default encoding is UTF-8 by RFC 8259: the payload carries `—`, `▲`, `▼` and `−` (#532's +#: non-colour gain/loss signal), and a client that guessed Latin-1 would render the whole contract +#: as mojibake. +_JSON_CONTENT_TYPE = "application/json; charset=utf-8" + + def _static_headers(content_type: str) -> tuple[tuple[str, str], ...]: """`_STATIC_BASE_HEADERS` plus CSP, but ONLY when `content_type` is one of `_CSP_CONTENT_TYPES` -- see the comments on `_STATIC_BASE_HEADERS` and `_CSP_CONTENT_TYPES` @@ -492,7 +506,52 @@ def _send( if self.command != "HEAD": self.wfile.write(payload) + def _send_json(self, code: int, document: dict[str, Any]) -> None: + """One JSON response, with its own headers. + + Writes them itself rather than going through `_send` for the same reason `_serve_static` + does: `_send` puts `_SECURITY_HEADERS` on every response, and one of those is a CSP that + has no meaning on `application/json` (see `_API_HEADERS`). Sharing the method would have + meant a parameter with a default, and a default on a shared sender is how a header set + silently changes for a route nobody was thinking about. + + A plain `json.dumps`: `keel/web/payload.py` normalises every leaf to a string before it + gets here, so there is nothing for an encoder to convert -- and the encoder a hurried + author reaches for is `default=float`, which is the whole money contract dying in one + keyword. Rule 6d of `test_console_thinness.py` fails the build on it in the serialiser; + there is no `default=` here for the same reason. + + `ensure_ascii=False` because the payload is UTF-8 and the glyphs carrying #532's + non-colour gain/loss signal have no business becoming escape sequences. + """ + body = json.dumps(document, ensure_ascii=False) + payload = body.encode("utf-8") + self.send_response(code) + self.send_header("Content-Type", _JSON_CONTENT_TYPE) + self.send_header("Content-Length", str(len(payload))) + for name, value in _API_HEADERS: + self.send_header(name, value) + self.end_headers() + if self.command != "HEAD": + self.wfile.write(payload) + def _refuse(self, code: int, heading: str, detail: str) -> None: + """A refusal, in the media type the caller asked for by the path it used. + + **Path-scoped, not method-scoped, and not content-negotiated.** An HTML error page handed + to a `fetch()` client's `res.json()` is a parse error in the client, which is a strictly + worse diagnostic than the 403 it is hiding -- so everything under `API_PREFIX` refuses in + JSON, including the POST that still 404s there. The GATE in front of that POST + (`_api_client_header_ok`) is untouched by this: same trigger, same status, same ordering; + only the body's media type follows the path. + + `Accept`-based negotiation was the alternative and it is worse here: a client that forgets + the header would get HTML from a JSON endpoint, and the one thing this server can be + certain about is which path was requested. + """ + if urlsplit(self.path).path.startswith(API_PREFIX): + self._send_json(code, api.refusal_document(code, heading, detail)) + return self._send( code, render.page( @@ -733,6 +792,20 @@ def do_GET(self) -> None: # noqa: N802 - stdlib's naming, not ours if not self._admitted(): return + if parsed.path.startswith(API_PREFIX): + # The JSON API (#534). Reads only: `api.respond` maps a path to one bounded read and + # returns `(status, document)` -- it never raises, so a broken report becomes a stated + # 500 with a JSON body rather than an HTML error page a `fetch()` client cannot parse, + # and never an empty payload that a view would render as zeros. + # + # Same admission as every rendered page, checked above and never weakened: an API is + # not exempt from the loopback-plus-session model for being machine-readable. What it + # does NOT additionally require is `X-Keel-Client` -- that header gates POSTs, and its + # docstring explains why a GET is not the gap it closes. + code, document = api.respond(self.cfg, parsed.path, query) + self._send_json(code, document) + return + if parsed.path.startswith(staticfiles.STATIC_PREFIX): # Same admission as every rendered page (never weakened): a static asset is not # exempted from the loopback-plus-session model just because it holds no secrets @@ -807,6 +880,7 @@ def serve(cfg: ServeConfig, *, echo: Callable[[str], None] = print) -> int: db_path=cfg.db_path, config_path=cfg.config_path, build=cfg.build, + build_info=cfg.build_info, ) server.RequestHandlerClass.cfg = running # type: ignore[attr-defined] diff --git a/tests/web/test_api.py b/tests/web/test_api.py new file mode 100644 index 0000000..f3c9a5f --- /dev/null +++ b/tests/web/test_api.py @@ -0,0 +1,873 @@ +"""The JSON API (#534), over a real bound server. + +Driven against an actual `ThreadingHTTPServer` rather than a hand-built handler for the same +reason `tests/web/test_server.py` is: the properties worth pinning here -- that a read needs no +write header, that a refusal is still JSON, that `Cache-Control: no-store` reaches the wire on +every `/api/*` response -- are properties of the BYTES, and a test against a handler object could +pass while the served response said otherwise. + +**This module deliberately does not reuse `tests/web/test_server.py`'s `_request`.** That helper +defaults `client_header="1"` on every POST, which is right for the module it lives in and wrong +here: the central question below is whether a *GET* is answered by a client that sends no custom +header at all (a `curl`, an address bar, a service worker's `NetworkOnly` fetch), so the helper +these tests need is one that sends nothing it was not asked to send. Sharing the other one would +have meant a default quietly answering the question the test is asking -- see `_get`'s docstring. + +Five things are pinned, and each exists because a downstream issue (#536-#540) consumes it: + +* **Every HTML read has a JSON counterpart**, `/glossary` excepted -- it becomes an outbound link + in #539 and its renderer is deleted in #540, so an `/api/glossary` would be a surface built to + be removed. +* **`GET /api/config` returns the running version**, because #538 keys a service-worker cache to + it and #539 carries it as `?v=` on documentation links. +* **Sorting is a query parameter ordered with `Decimal`**, with the float-collision case that + makes the choice of `Decimal` load-bearing rather than decorative. +* **A stopped engine is a stated fact, not an empty payload** -- `engine.value == "stopped"` with + `data: null`, at HTTP 200, so a client renders "keel isn't running" instead of a blank view. +* **The write surface did not move.** `POST /api/*` still 404s behind the `X-Keel-Client` gate, + and this issue adds no route to it. +""" + +from __future__ import annotations + +import ast +import http.client +import json +import sqlite3 +import threading +from collections.abc import Iterator +from decimal import Decimal +from pathlib import Path +from typing import Any + +import pytest + +from keel.data.db import connect, migrate +from keel.web import api as web_api +from keel.web import payload +from keel.web import server as web_server +from keel.web.security import SESSION_COOKIE, csrf_token, new_session_token +from tests.conftest import VALID_CONFIG_YAML + +#: Every read the HTML routes perform, as its JSON counterpart. `/glossary` is absent on purpose +#: and `test_the_glossary_has_no_api_counterpart` states that absence as an assertion, so deleting +#: it here later cannot silently mean "we forgot". +API_ROUTES = ( + "/api/config", + "/api/status", + "/api/setup", + "/api/activity", + "/api/insights", + "/api/journal", + "/api/rules", + "/api/venues", + "/api/gates", +) + + +# -- a real server ------------------------------------------------------------------------------ + + +def _bind(db_path: str, config_path: str, **extra: Any) -> Iterator[web_server.ServeConfig]: + cfg = web_server.ServeConfig( + host="127.0.0.1", + port=0, + token=new_session_token(), + db_path=db_path, + config_path=config_path, + **extra, + ) + server = web_server.build_server(cfg) + bound = web_server.ServeConfig( + host=cfg.host, + port=int(server.server_address[1]), + token=cfg.token, + db_path=db_path, + config_path=config_path, + **extra, + ) + server.RequestHandlerClass.cfg = bound # type: ignore[attr-defined] + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + yield bound + finally: + server.shutdown() + server.server_close() + thread.join(timeout=5) + + +@pytest.fixture +def deployment(tmp_path: Path) -> tuple[str, str]: + db_path = tmp_path / "keel.db" + conn = connect(str(db_path)) + migrate(conn) + conn.close() + config_path = tmp_path / "config.yaml" + config_path.write_text(VALID_CONFIG_YAML) + return str(db_path), str(config_path) + + +@pytest.fixture +def running(deployment: tuple[str, str]) -> Iterator[web_server.ServeConfig]: + db_path, config_path = deployment + yield from _bind(db_path, config_path) + + +@pytest.fixture +def empty_machine(tmp_path: Path) -> Iterator[web_server.ServeConfig]: + """Nothing set up at all -- no config, no database. The first-run state, and the one an API + client must be told about in words rather than left to infer from an empty list.""" + yield from _bind(str(tmp_path / "keel.db"), str(tmp_path / "config.yaml")) + + +def _get( + cfg: web_server.ServeConfig, + path: str, + *, + method: str = "GET", + cookie: str | None = None, + host: str | None = None, + headers: dict[str, str] | None = None, +) -> tuple[int, dict[str, str], str]: + """A request that sends NOTHING it was not asked to send. + + No `X-Keel-Client`, no `Sec-Fetch-Site`, no `Origin` unless a test names them. That is the + whole point of not sharing `test_server.py`'s helper: a default header here would answer + `test_a_read_needs_no_client_header` on the test's behalf, and the shipped consumers of this + API include a `curl` and a browser address bar, neither of which sends one.""" + conn = http.client.HTTPConnection(cfg.host, cfg.port, timeout=10) + sent = {"Host": host if host is not None else f"{cfg.host}:{cfg.port}"} + if cookie: + sent["Cookie"] = cookie + sent.update(headers or {}) + try: + conn.request(method, path, headers=sent) + response = conn.getresponse() + body = response.read().decode("utf-8", "replace") + return response.status, dict(response.getheaders()), body + finally: + conn.close() + + +def _session(cfg: web_server.ServeConfig) -> str: + return f"{SESSION_COOKIE}={cfg.token}" + + +def _json(cfg: web_server.ServeConfig, path: str) -> tuple[int, dict[str, str], Any]: + status, headers, body = _get(cfg, path, cookie=_session(cfg)) + return status, headers, json.loads(body) + + +# -- seeding ------------------------------------------------------------------------------------ + + +def _seed_positions(db_path: str, rows: tuple[tuple[str, str, str], ...]) -> None: + """Open tranches, written straight into the table `gather_status` reads. + + `qty` and `entry_fill` are TEXT columns holding exact decimal strings, which is what makes the + float-collision case below reachable end to end rather than only in a unit test: the value the + API sorts is the value the ledger stored, character for character.""" + conn = sqlite3.connect(db_path) + try: + for index, (product_id, qty, entry_fill) in enumerate(rows): + conn.execute( + "INSERT INTO positions (product_id, rule_name, opened_at, qty, entry_fill, " + "entry_fee, status) VALUES (?, ?, ?, ?, ?, ?, 'open')", + (product_id, f"rule-{index}", 1_700_000_000 + index, qty, entry_fill, "0"), + ) + conn.commit() + finally: + conn.close() + + +def _seed_rules(db_path: str, kinds: tuple[str, ...]) -> None: + conn = sqlite3.connect(db_path) + try: + for index, kind in enumerate(kinds): + conn.execute( + "INSERT INTO rules (kind, params, status, created_at) VALUES (?, ?, ?, ?)", + (kind, json.dumps({"product_id": "BTC-USD"}), "candidate", 1_700_000_000 + index), + ) + conn.commit() + finally: + conn.close() + + +# -- every read is available as JSON -------------------------------------------------------------- + + +@pytest.mark.parametrize("path", API_ROUTES) +def test_every_route_answers_json_with_the_envelope( + running: web_server.ServeConfig, path: str +) -> None: + """The uniform half of the contract: one shape, so #536's single `fetch` wrapper needs no + per-endpoint branch. `as_of` and `engine` are present on EVERY success, including the three + endpoints that describe the binary rather than the deployment -- a client that had to know + which endpoints carry the liveness word would be branching on payload shape, which is the + thing `payload.py`'s Rule 3 exists to remove.""" + status, headers, document = _json(running, path) + + assert status == 200, (path, document) + assert headers["Content-Type"] == "application/json; charset=utf-8" + assert set(document) >= {"as_of", "engine", "data"} + assert document["as_of"].endswith("Z") + assert document["engine"]["value"] in payload.ENGINE_STATES + assert document["data"] is not None + + +def test_the_glossary_has_no_api_counterpart(running: web_server.ServeConfig) -> None: + """Stated as an assertion rather than left as an omission. `/glossary` becomes an outbound + keeltrading.com link in #539 and `render_glossary` is deleted in #540, so an `/api/glossary` + would be a surface built in order to be removed -- and a client written against it would + break on the release that removes it.""" + status, headers, document = _json(running, "/api/glossary") + + assert status == 404 + assert headers["Content-Type"] == "application/json; charset=utf-8" + assert document["error"]["status"] == "404" + + +def test_the_api_routes_cover_every_html_route_that_reads( + running: web_server.ServeConfig, +) -> None: + """The pin that keeps the two surfaces in step. Read off both route tables, so an HTML route + added without a JSON counterpart fails here rather than being noticed when a client cannot + render it.""" + html = set(web_server.ROUTES) - {"/glossary"} + covered = {web_api.HTML_ROUTE_FOR[name] for name in web_api.API_ROUTES} + + assert html <= covered, html - covered + + +# -- /api/config ---------------------------------------------------------------------------------- + + +def test_config_returns_the_running_version(deployment: tuple[str, str]) -> None: + """#538 keys a service-worker cache name to `build`, and #539 carries `version` as `?v=` on + every documentation link. Both consumers need the value to be the RUNNING build's, resolved + once at start-up rather than re-derived per request -- `keel.version.build_info()` shells out + to git, and a service worker polling an endpoint that forks a subprocess is a bad trade.""" + db_path, config_path = deployment + fake = _FakeBuild() + for cfg in _bind(db_path, config_path, build="keel 9.9.9+abc [checkout]", build_info=fake): + status, _headers, document = _json(cfg, "/api/config") + + assert status == 200 + assert document["data"]["version"] == "9.9.9" + assert document["data"]["build"] == "9.9.9+abc123456789" + assert document["data"]["source"] == "checkout" + assert document["data"]["describe"] == "keel 9.9.9+abc [checkout]" + assert document["data"]["reproducible"]["value"] == "true" + + +def test_config_survives_a_build_it_could_not_resolve(running: web_server.ServeConfig) -> None: + """An environment with no package metadata and no git is not an error worth a 500 for: the + footer already degrades to an empty string there (`serve.py::_build_line`), and a cache key + of `"unknown"` still keys a cache. Reported as absent rather than invented.""" + status, _headers, document = _json(running, "/api/config") + + assert status == 200 + assert document["data"]["version"] == "" + assert document["data"]["reproducible"]["state"] == "unknown" + + +class _FakeBuild: + """A resolved `keel.version.BuildInfo`, without shelling out to git in a test.""" + + version = "9.9.9" + commit = "abc123456789" + dirty = False + source = "checkout" + full_version = "9.9.9+abc123456789" + is_reproducible = True + + +# -- money crosses the wire as a string ------------------------------------------------------------ + + +def _walk(node: Any, path: str = "$") -> list[tuple[str, Any]]: + if isinstance(node, dict): + out: list[tuple[str, Any]] = [] + for key, value in node.items(): + out.extend(_walk(value, f"{path}.{key}")) + return out + if isinstance(node, list): + out = [] + for index, value in enumerate(node): + out.extend(_walk(value, f"{path}[{index}]")) + return out + return [(path, node)] + + +def _json_numbers(document: Any) -> list[str]: + """`bool` first and deliberately: Python's `bool` is a subclass of `int`, so an `isinstance` + test alone would flag every JSON `true`. Duplicated from `test_payload.py` rather than + imported so that weakening one guard cannot weaken the other.""" + return [ + path + for path, leaf in _walk(document) + if not isinstance(leaf, bool) and isinstance(leaf, (int, float)) + ] + + +@pytest.mark.parametrize("path", API_ROUTES) +def test_no_wire_value_from_any_endpoint_is_ever_a_json_number( + running: web_server.ServeConfig, path: str +) -> None: + """#533's rule, asserted over the bytes each ENDPOINT actually serves rather than over the + serialiser's return value. The two are not the same statement: the envelope, the sort echo and + every refusal body are written by the routing layer, and a plain `int` in any of them is a + double in a browser exactly as a mis-serialised price would be.""" + db_path = running.db_path + _seed_positions(db_path, (("BTC-USD", "0.01", "50000"),)) + _seed_rules(db_path, ("breakout",)) + + _status, _headers, document = _json(running, path) + + assert _json_numbers(document) == [], path + + +def test_the_number_walker_is_proven_false_capable() -> None: + """The guard's positive control. A walker that matched nothing would make every parametrised + case above green over a payload full of doubles.""" + assert _json_numbers({"a": [{"b": 0.1}], "ok": True, "text": "1"}) == ["$.a[0].b"] + + +def test_even_a_refusal_carries_no_json_number(running: web_server.ServeConfig) -> None: + """The HTTP status crosses as `"404"`, not `404`. It would survive JSON's number type intact, + and it still goes as a string: a payload with "only a few" numbers in it needs a per-field rule + about which ones, and that rule is what rots -- `payload.count`'s docstring makes the same + argument for a trade count.""" + _status, _headers, document = _json(running, "/api/nope") + + assert _json_numbers(document) == [] + + +# -- server-side sort ------------------------------------------------------------------------------ + + +def test_sorting_is_a_query_parameter(running: web_server.ServeConfig) -> None: + _seed_rules(running.db_path, ("momentum", "breakout", "reversal")) + + _status, _headers, ascending = _json(running, "/api/rules?sort=kind") + _status, _headers, descending = _json(running, "/api/rules?sort=kind&dir=desc") + + assert [row["kind"] for row in ascending["data"]["rules"]] == [ + "breakout", + "momentum", + "reversal", + ] + assert [row["kind"] for row in descending["data"]["rules"]] == [ + "reversal", + "momentum", + "breakout", + ] + + +def test_the_sort_is_echoed_so_a_client_needs_no_hardcoded_column_list( + running: web_server.ServeConfig, +) -> None: + """#537 renders sortable headers. It reads the column list off the response rather than + holding a copy, so a column added here reaches the interface without a second edit -- and a + column removed here cannot leave a header that sorts by nothing.""" + _status, _headers, document = _json(running, "/api/rules?sort=kind&dir=desc") + + assert document["sort"]["column"] == "kind" + assert document["sort"]["direction"] == "desc" + assert "kind" in document["sort"]["columns"] + + +def test_an_unknown_sort_column_is_refused_not_ignored(running: web_server.ServeConfig) -> None: + """Silently ignoring an unknown column is how a client ships a sort that does nothing and + nobody notices for a release. The refusal names the columns that do exist, so the fix is in + the response.""" + status, _headers, document = _json(running, "/api/rules?sort=expectancy") + + assert status == 400 + assert "expectancy" in document["error"]["detail"] + assert "kind" in document["error"]["detail"] + + +def test_an_unknown_sort_direction_is_refused(running: web_server.ServeConfig) -> None: + status, _headers, document = _json(running, "/api/rules?sort=kind&dir=sideways") + + assert status == 400 + assert "sideways" in document["error"]["detail"] + + +def test_an_endpoint_with_nothing_to_sort_says_so(running: web_server.ServeConfig) -> None: + """`/api/setup`'s steps are in runbook order -- the order IS the information, since a checklist + sorted by title is a checklist you cannot work down -- so it declares no sortable columns, and + a `?sort=` against it is refused rather than quietly obeyed.""" + status, _headers, refused = _json(running, "/api/setup?sort=title") + _status, _headers, plain = _json(running, "/api/setup") + + assert status == 400 + assert "no sortable table" in refused["error"]["detail"] + # And the successful response says the same thing in the shape a client reads: `sort` is + # `null`, not an object with an empty column list, so "this endpoint does not sort" and "this + # endpoint sorts but you have not asked it to" stay distinguishable. + assert plain["sort"] is None + + +def test_an_endpoint_that_sorts_echoes_its_columns_before_anything_is_sorted( + running: web_server.ServeConfig, +) -> None: + """The other half of the distinction above.""" + _status, _headers, document = _json(running, "/api/rules") + + assert document["sort"] == { + "column": "", + "direction": "asc", + "columns": list(web_api.API_ROUTES["/api/rules"].sortable), + } + + +def test_every_declared_sort_column_is_a_column_the_rows_actually_have( + running: web_server.ServeConfig, +) -> None: + """The guard against the one way a hand-written column list rots: a key renamed in + `payload.py` leaves a `sortable` entry that names nothing, and `?sort=` by it would be accepted + and then order every row identically -- an accepted request that silently does nothing, which + is exactly what refusing an unknown column exists to prevent. + + Checked over the two endpoints this test can seed rows into. A collection with no rows proves + nothing here, which is why the seeding is not optional.""" + _seed_positions(running.db_path, (("BTC-USD", "0.01", "50000"),)) + _seed_rules(running.db_path, ("breakout",)) + + for path, collection in (("/api/status", "open_positions"), ("/api/rules", "rules")): + _status, _headers, document = _json(running, path) + rows = document["data"][collection] + + assert rows, path + for row in rows: + missing = set(web_api.API_ROUTES[path].sortable) - set(row) + assert not missing, (path, missing) + + +# -- Decimal ordering, and the case where a float would differ ------------------------------------- + +#: Three quantities in the ledger's own text form. The first two differ by 1e-18 -- one wei, the +#: base increment of any 18-decimal ERC-20 -- and `float()` maps BOTH to the same IEEE-754 double, +#: because at 0.1 the gap between adjacent doubles is about 1.4e-17. +WEI_APART = ( + "0.100000000000000002", + "0.100000000000000001", + "0.099000000000000000", +) + + +def test_float_and_decimal_orderings_of_the_same_column_genuinely_differ() -> None: + """The premise of the test below, asserted rather than assumed. + + If `float` and `Decimal` agreed on these three strings, the endpoint test would be green for + no reason -- it would prove only that sorting sorts. So this states the disagreement first: + the two finest values are DISTINCT as `Decimal` and EQUAL as `float`, which means a float-keyed + sort cannot separate them and leaves them in whatever order they arrived in.""" + finer, coarser = Decimal(WEI_APART[1]), Decimal(WEI_APART[0]) + + assert finer < coarser + assert float(WEI_APART[1]) == float(WEI_APART[0]) + # Ascending by float keeps the input order of the tied pair; ascending by Decimal swaps it. + assert sorted(WEI_APART, key=float) == [WEI_APART[2], WEI_APART[0], WEI_APART[1]] + assert sorted(WEI_APART, key=Decimal) == [WEI_APART[2], WEI_APART[1], WEI_APART[0]] + + +def test_the_endpoint_orders_with_decimal_not_float(running: web_server.ServeConfig) -> None: + """The end-to-end half: the same three quantities through the real table, the real report + builder, the real serialiser and the real sort. + + A float-keyed implementation would answer `[0.099, ...002, ...001]` here -- the tied pair left + in insertion order, because the comparison cannot tell them apart. `Decimal` answers + `[0.099, ...001, ...002]`. The difference is one wei on one row, which is exactly the size of + error that survives review and shows up in a reconciliation.""" + _seed_positions( + running.db_path, tuple((f"TKN{i}-USD", qty, "50000") for i, qty in enumerate(WEI_APART)) + ) + + _status, _headers, unsorted = _json(running, "/api/status") + _status, _headers, document = _json(running, "/api/status?sort=qty") + + as_built = [row["qty"]["value"] for row in unsorted["data"]["open_positions"]] + ordered = [row["qty"]["value"] for row in document["data"]["open_positions"]] + + assert ordered == [WEI_APART[2], WEI_APART[1], WEI_APART[0]] + # The comparison that makes this test say something: the SAME rows, in the order the report + # built them, keyed by `float` instead. Python's sort is stable, so the tied pair keeps its + # arrival order and the answer differs from the one above by one row. + assert sorted(as_built, key=float) == [WEI_APART[2], WEI_APART[0], WEI_APART[1]] + assert ordered != sorted(as_built, key=float) + + +def test_a_missing_figure_sorts_last_in_both_directions() -> None: + """A row with no recorded value is not a row with a zero -- that collapse is the shape of the + always-passing fee rail (#198), and `payload.absent` exists to keep the two apart. So an absent + cell trails the ordered rows whichever way they run, rather than heading a descending sort by + being the largest thing in it.""" + rows = [ + {"id": "a", "n": payload.money(Decimal("2"))}, + {"id": "b", "n": payload.absent()}, + {"id": "c", "n": payload.money(Decimal("1"))}, + ] + + ascending = payload.order_rows(rows, column="n", descending=False) + descending = payload.order_rows(rows, column="n", descending=True) + + assert [row["id"] for row in ascending] == ["c", "a", "b"] + assert [row["id"] for row in descending] == ["a", "c", "b"] + + +def test_a_column_that_is_not_numeric_orders_as_text() -> None: + """One column, one ordering. A column whose values do not ALL parse as finite `Decimal`s is + ordered as text -- deciding per ROW would put a `Decimal` and a `str` in the same comparison, + which raises, and falling back per row would make the order depend on which values happened to + look like numbers.""" + rows = [ + {"outcome": payload.label("win")}, + {"outcome": payload.label("dca")}, + {"outcome": payload.label("loss")}, + ] + + ordered = payload.order_rows(rows, column="outcome", descending=False) + + assert [row["outcome"]["value"] for row in ordered] == ["dca", "loss", "win"] + + +def test_an_instant_orders_chronologically_through_its_iso_string() -> None: + """`moment().value` is ISO-8601 UTC with a fixed width, so lexicographic order IS chronological + order and no date parsing happens in the sort. This is the reason `moment` puts ISO in `value` + rather than epoch seconds -- `payload.moment`'s docstring records the other.""" + rows = [ + {"at": payload.moment(1_700_000_100)}, + {"at": payload.moment(1_700_000_000)}, + {"at": payload.moment(1_700_000_200)}, + ] + + ordered = payload.order_rows(rows, column="at", descending=True) + + assert [row["at"]["value"] for row in ordered] == [ + "2023-11-14T22:16:40Z", + "2023-11-14T22:15:00Z", + "2023-11-14T22:13:20Z", + ] + + +def test_a_non_finite_value_does_not_drag_a_column_into_numeric_ordering() -> None: + """`Decimal("NaN")` and `Decimal("Infinity")` both PARSE, and both are unorderable against a + real figure -- `NaN` compares false to everything, which would make the sort's output depend on + the comparison order the algorithm happened to use. Non-finite means "not a number this column + can be ordered by", so the column falls to text ordering as a whole.""" + rows = [ + {"n": {"value": "Infinity", "display": "inf", "state": "unknown"}}, + {"n": {"value": "2", "display": "2", "state": "neutral"}}, + ] + + ordered = payload.order_rows(rows, column="n", descending=False) + + assert [row["n"]["value"] for row in ordered] == ["2", "Infinity"] + + +# -- the engine's state is a fact, not an inference ------------------------------------------------ + + +def test_a_stopped_engine_says_so_rather_than_answering_an_empty_payload( + empty_machine: web_server.ServeConfig, +) -> None: + """THE requirement the service worker enforces from the other side (#538): a client must be + able to say "keel isn't running" rather than render a blank view or, worse, a stale figure. + + `data` is `null` and not `{}`: an empty object is a payload with every figure missing, and a + view given one renders zeros. `null` cannot be rendered by accident.""" + status, _headers, document = _json(empty_machine, "/api/status") + + assert status == 200 + assert document["engine"]["value"] == "stopped" + assert document["data"] is None + assert document["as_of"].endswith("Z") + assert document["engine"]["display"] + assert document["engine"]["state"] in {"warn", "bad"} + + +def test_a_stopped_engine_is_a_200_not_an_error(empty_machine: web_server.ServeConfig) -> None: + """Reported at 200 on purpose. A 4xx/5xx is what #538's service worker and #536's `fetch` + wrapper both read as "the server is unreachable", which would put a first-run user in an + outage state when their actual position is that they have not set anything up yet -- and the + two need different words on screen.""" + status, _headers, _document = _json(empty_machine, "/api/status") + + assert status == 200 + + +def test_a_stopped_engine_still_answers_the_endpoints_that_describe_the_binary( + empty_machine: web_server.ServeConfig, +) -> None: + """`/api/config`, `/api/venues` and `/api/gates` read no deployment at all -- they describe the + binary that is answering -- so they carry data even with nothing set up. They still report + `engine`, because a client showing the "keel isn't running" banner should not have to fetch a + different endpoint to know whether to show it.""" + for path in ("/api/config", "/api/venues", "/api/gates"): + status, _headers, document = _json(empty_machine, path) + + assert status == 200, path + assert document["engine"]["value"] == "stopped", path + assert document["data"] is not None, path + + +def test_setup_is_answerable_with_nothing_set_up(empty_machine: web_server.ServeConfig) -> None: + """The one deployment-reading endpoint that must work when there is no deployment: it is the + checklist that says how to make one. `needs_database`'s HTML counterpart serves this same page + for the same reason.""" + status, _headers, document = _json(empty_machine, "/api/setup") + + assert status == 200 + assert document["engine"]["value"] == "stopped" + assert document["data"]["is_new"]["value"] == "true" + assert document["data"]["steps"] + assert document["data"]["actions"] + + +def test_setup_carries_no_csrf_token(empty_machine: web_server.ServeConfig) -> None: + """`render_setup` takes a `csrf` argument; this payload does not, and that is deliberate. A + CSRF token authorises a WRITE, this issue ships reads only, and minting one into a read + response would put a live write credential into every cached and logged copy of a GET. + + Asserted against the token's VALUE and against the key names, not against the substring + `"csrf"` in the whole document: the payload echoes `config_path` and `db_path`, and a + deployment living in a directory whose name happens to contain those four letters would fail a + substring check for a reason that has nothing to do with the contract.""" + _status, _headers, document = _json(empty_machine, "/api/setup") + + assert csrf_token(empty_machine.token) not in json.dumps(document) + assert not [path for path, _leaf in _walk(document) if "csrf" in path.lower()] + + +def test_a_report_that_cannot_be_built_is_reported_not_swallowed( + running: web_server.ServeConfig, monkeypatch: pytest.MonkeyPatch +) -> None: + """A broken read is a 500 with a JSON body, never an HTML error page a `fetch()` client would + fail to parse and never a 200 with empty data. `engine` reads `stopped` alongside it: from a + client's point of view a report that cannot be built and an engine that is not there require + the SAME behaviour -- show no figures -- and the difference is already carried by the status + code and `error.detail`.""" + monkeypatch.setattr( + web_api, "_status_report", _raise, raising=True + ) + + status, headers, document = _json(running, "/api/status") + + assert status == 500 + assert headers["Content-Type"] == "application/json; charset=utf-8" + assert document["data"] is None + assert "RuntimeError" in document["error"]["detail"] + + +def _raise(*_args: Any, **_kwargs: Any) -> Any: + raise RuntimeError("the ledger is on fire") + + +# -- headers and caching --------------------------------------------------------------------------- + + +@pytest.mark.parametrize("path", API_ROUTES) +def test_no_api_response_may_be_cached(running: web_server.ServeConfig, path: str) -> None: + """The design spec's service-worker table says `/api/*` is `NetworkOnly`, no exceptions, + because "opening the app to last week's equity styled as current is worse than an error". This + is the layer BELOW that promise: a `no-store` on the response means the HTTP cache cannot hold + a balance either, whether or not a service worker is installed.""" + _status, headers, _document = _json(running, path) + + assert headers["Cache-Control"] == "no-store, max-age=0" + + +@pytest.mark.parametrize("path", API_ROUTES) +def test_json_is_served_with_nosniff_and_no_csp( + running: web_server.ServeConfig, path: str +) -> None: + """`nosniff` matters more here than on the HTML: a JSON body a browser is free to sniff as + HTML is a stored-XSS primitive wearing a `Content-Type`. CSP is absent for the reason + `_static_headers` already records -- it is a response header with no defined meaning outside a + browsing context, and `application/json` is not one.""" + _status, headers, _document = _json(running, path) + + assert headers["X-Content-Type-Options"] == "nosniff" + assert headers["X-Frame-Options"] == "DENY" + assert headers["Referrer-Policy"] == "no-referrer" + assert "Content-Security-Policy" not in headers + + +def test_head_returns_the_headers_and_no_body(running: web_server.ServeConfig) -> None: + status, headers, body = _get( + running, "/api/status", method="HEAD", cookie=_session(running) + ) + + assert status == 200 + assert headers["Content-Type"] == "application/json; charset=utf-8" + assert body == "" + + +# -- admission ------------------------------------------------------------------------------------- + + +def test_a_read_needs_no_client_header(running: web_server.ServeConfig) -> None: + """`X-Keel-Client` gates `POST /api/*` and deliberately not `GET`. + + The header buys one thing: it forces a CORS preflight a hostile origin cannot satisfy, closing + the plain-form-POST gap that `SameSite=Strict` and the HMAC token both assume shut. A GET is + not that gap -- a cross-origin read cannot see this response at all without CORS headers this + server never sends, and `SameSite=Strict` denies the cookie to the cross-site request in the + first place, so the read is refused at admission before any of this matters. + + Requiring it anyway would cost the thing §4 of the design philosophy is about: `curl + http://127.0.0.1:8765/api/status` and a browser address bar are how an operator checks that + the interface is telling the truth, and neither can set a header. Reverse this the day a GET + can change something.""" + status, _headers, document = _json(running, "/api/status") + + assert status == 200 + assert document["data"] is not None + + +def test_a_read_without_the_session_cookie_is_refused_in_json( + running: web_server.ServeConfig, +) -> None: + """Same admission as every rendered page -- never weakened for being an API -- but the refusal + speaks the caller's language. An HTML error page handed to `res.json()` is a parse error in + the client, which is a worse diagnostic than the 403 it is hiding.""" + status, headers, body = _get(running, "/api/status") + + assert status == 403 + assert headers["Content-Type"] == "application/json; charset=utf-8" + assert json.loads(body)["error"]["status"] == "403" + + +def test_a_read_from_a_foreign_host_header_is_refused_in_json( + running: web_server.ServeConfig, +) -> None: + """DNS rebinding lands here exactly as it does for a page: the packet arrived on loopback, so + the bind check passed, and only the header tells the truth about who the browser thinks it is + talking to.""" + status, headers, body = _get( + running, "/api/status", cookie=_session(running), host="keel.example.com" + ) + + assert status == 403 + assert headers["Content-Type"] == "application/json; charset=utf-8" + assert "Refused" in json.loads(body)["error"]["title"] + + +def test_an_html_route_still_refuses_in_html(running: web_server.ServeConfig) -> None: + """The other half of the same statement: making `/api/*` speak JSON did not make the rendered + pages speak it. A person who opens the URL without the token still gets a readable page.""" + status, headers, body = _get(running, "/insights") + + assert status == 403 + assert headers["Content-Type"].startswith("text/html") + assert "

" in body + + +# -- the write surface did not move ---------------------------------------------------------------- + + +@pytest.mark.parametrize("path", API_ROUTES) +def test_no_api_route_answers_a_post(running: web_server.ServeConfig, path: str) -> None: + """This issue ships READS. Every route added here is a 404 for a POST -- reached only after + the `X-Keel-Client` gate, which is unchanged -- so nothing below widened what the browser may + do.""" + status, _headers, body = _get( + running, + path, + method="POST", + cookie=_session(running), + headers={"X-Keel-Client": "1", "Content-Length": "0"}, + ) + + assert status == 404, path + assert json.loads(body)["error"]["status"] == "404" + + +def test_the_client_header_gate_still_guards_every_api_post( + running: web_server.ServeConfig, +) -> None: + """#535's third CSRF layer, still in front of `/api/*` writes and still refusing before the + 404 -- unchanged by this issue, and asserted here as well as in `test_server.py` because the + route table under that prefix is no longer empty.""" + status, _headers, _body = _get( + running, "/api/status", method="POST", cookie=_session(running), + headers={"Content-Length": "0"}, + ) + + assert status == 403 + + +def test_the_handler_still_declares_exactly_three_verbs() -> None: + verbs = { + name + for klass in web_server.KeelHandler.__mro__ + for name in vars(klass) + if name.startswith("do_") + } + assert verbs == {"do_GET", "do_HEAD", "do_POST"} + + +# -- the routing layer computes nothing ------------------------------------------------------------ + + +def test_the_api_module_is_inside_the_thinness_scan() -> None: + """`test_console_thinness.py` globs `keel/web/*.py`, so the API layer is covered by Rules 1-5 + by construction rather than by anyone remembering to list it. Asserted here, from this side, + because that file names its web modules explicitly for exactly this reason and a module that + dropped out of the glob would leave the rules green over less code.""" + from tests.commands.test_console_thinness import _console_module_paths + + stems = {Path(p).stem for p in _console_module_paths()} + + assert "api" in stems + + +def test_rule_6_holds_in_the_api_layer_too() -> None: + """Rule 6 is scoped by stem to the serialiser, which is right -- it is the module whose output + is the money contract. But the routing layer sits directly on top of that output and re-parses + it to sort, so the same five spellings of "a Decimal became a double" are reachable there: + `float(row["value"])` as a sort key is the whole failure in one expression. + + Run here, over `keel/web/api.py`, rather than by adding a stem to `SERIALISER_STEMS`: the pin + in that file is #533's contract and it stays as it was written, while this states the extra + thing #534 needs. If the API layer ever needs an allowance, it gets one HERE, named.""" + from tests.commands.test_console_thinness import ( + _collect_aliases, + _enclosing_functions, + _rule6_findings, + ) + + source = Path(web_api.__file__).read_text(encoding="utf-8") + tree = ast.parse(source) + findings = _rule6_findings("api", tree, _collect_aliases(tree), _enclosing_functions(tree)) + + assert findings.get("rule6_serialisation", []) == [] + + +def test_the_routing_layer_formats_nothing() -> None: + """Every displayable string in an API response was written by `payload.py`. + + Checked by shape rather than by reading: a `format(...)` call or an f-string carrying a format + spec in the routing layer is how a second money renderer starts, and the second one is never + the one with `_plain`'s no-exponent guarantee in it.""" + tree = ast.parse(Path(web_api.__file__).read_text(encoding="utf-8")) + formatting = [ + node.lineno + for node in ast.walk(tree) + if _is_format_call(node) + or (isinstance(node, ast.FormattedValue) and node.format_spec is not None) + ] + + assert formatting == [] + + +def _is_format_call(node: ast.AST) -> bool: + return ( + isinstance(node, ast.Call) + and isinstance(node.func, ast.Name) + and node.func.id == "format" + ) diff --git a/tests/web/test_payload.py b/tests/web/test_payload.py index ec6be35..b29610d 100644 --- a/tests/web/test_payload.py +++ b/tests/web/test_payload.py @@ -31,11 +31,14 @@ from __future__ import annotations import json +from dataclasses import dataclass from decimal import Decimal +from pathlib import Path from typing import Any import pytest +from keel.capabilities import CAPABILITIES, GATES from keel.commands.activity import ActivityCycle, ActivityEvent, ActivityFeed from keel.commands.insights import ( AccountSummary, @@ -45,6 +48,8 @@ JournalReport, RuleTrackRecord, ) +from keel.commands.jobs import JobStatus +from keel.commands.setup import ACTIONS, STEPS, DeploymentState, StepState from keel.commands.status import ( AutonomyStatus, MarketSessionStatus, @@ -287,17 +292,107 @@ def _activity_feed(**overrides: Any) -> ActivityFeed: return ActivityFeed(**base) +def _deployment_state() -> DeploymentState: + """A half-built deployment: one step done, one outstanding, one that could not be determined. + + All three `done` values on purpose. `None` is NOT `False` -- an unreadable database is not an + unseeded one -- and a fixture carrying only booleans would let the three-valued field be + quietly collapsed into two without any guard here noticing.""" + observed = (True, False, None) + return DeploymentState( + root=Path("/tmp/keel"), + config_path=Path("/tmp/keel/config.yaml"), + db_path=Path("/tmp/keel/keel.db"), + states=tuple( + StepState(step=step, done=observed[index % 3], detail=f"observed {step.key}") + for index, step in enumerate(STEPS) + ), + ) + + +@dataclass(frozen=True) +class _BrokerInfo: + """The `BrokerInfo` shape `venues_payload` reads, without importing an adapter into a + serialisation test. Two rows are built from it below: one healthy, one that failed to + construct -- the second is a row rather than an omission, because a missing row would read as + "not installed", which is a different fact and a worse one to be wrong about.""" + + name: str = "coinbase" + venue: str = "coinbase" + deployment: str = "spot" + session_bound: bool = False + quote_currencies: tuple[str, ...] = ("USD",) + asset_classes: tuple[str, ...] = ("crypto",) + supported_orders: tuple[str, ...] = ("market", "limit") + preview: str = "spot, USD quotes" + supports_fee_summary: bool = True + declared_endpoints: tuple[str, ...] = ("https://api.coinbase.com",) + supported_data_feeds: tuple[str, ...] = ("candles",) + package_version: str | None = "0.1.0" + error: str | None = None + + def _every_payload() -> dict[str, Any]: - """All four payload builders at once. The guards below run over the whole surface, because a - contract that holds for `status` and leaks on `journal` is not a contract.""" + """Every payload builder at once. The guards below run over the whole surface, because a + contract that holds for `status` and leaks on `journal` is not a contract. + + #534 added five entries here -- the endpoints behind `/api/config`, `/api/setup`, `/api/rules`, + `/api/venues` and `/api/gates` -- and that is the point of the helper being shared: a new + serialiser joins the money-as-strings walk, the `state`-vocabulary check and the + no-custom-encoder check by being listed once, rather than by its author remembering three + separate guards. `gates` reads the REAL `keel/capabilities.py` registry rather than a fixture, + because that module is a declaration with no external inputs and a fixture of it would be a + second copy to drift.""" return { "status": payload.status_payload(_status_report()), "insights": payload.insights_payload(_insights_report()), "journal": payload.journal_payload(_journal_report()), "activity": payload.activity_payload(_activity_feed()), + "config": payload.config_payload(_build_info(), describe="keel 0.1.0+abc [checkout]"), + "setup": payload.setup_payload( + _deployment_state(), + actions=ACTIONS, + not_automated={"market_data": "runs for minutes; see keel fetch"}, + job=JobStatus( + key="market_data", + state="failed", + started_ts=NOW_TS - 90, + finished_ts=NOW_TS, + lines=("fetching BTC-USD",), + error="RuntimeError: no credential", + ), + ), + "rules": payload.rules_payload( + [ + { + "id": 1, + "kind": "breakout", + "status": "candidate", + "created_at": NOW_TS, + "promoted_at": None, + "demoted_at": None, + "params": {"product_id": "BTC-USD", "lookback": 20, "risk": Decimal("0.01")}, + } + ] + ), + "venues": payload.venues_payload( + [_BrokerInfo(), _BrokerInfo(name="broken", error="ImportError: no module")] + ), + "gates": payload.gates_payload(GATES, CAPABILITIES), } +class _build_info: # noqa: N801 - a stand-in for `keel.version.BuildInfo`, not a public type + """A resolved build, without shelling out to git in a serialisation test.""" + + version = "0.1.0" + commit = "abc123456789" + dirty = False + source = "checkout" + full_version = "0.1.0+abc123456789" + is_reproducible = True + + # -- recursive walkers ---------------------------------------------------------------------------