diff --git a/keel/web/render.py b/keel/web/render.py index fbd8419..36a6158 100644 --- a/keel/web/render.py +++ b/keel/web/render.py @@ -24,8 +24,20 @@ from collections.abc import Iterable, Sequence from decimal import Decimal from typing import Any +from urllib.parse import quote + +#: The published documentation root. Spelled here and in `static/js/docs.js`, and +#: `tests/web/test_doc_links.py` pins that the two agree. +DOCS_URL = "https://keeltrading.com/en/docs/" #: Nav order, and the labels. `/` first because the status page is the answer to "is it alive". +#: +#: **The eighth entry is an OUTBOUND link, and used to be a page (#539).** `/glossary` rendered +#: `docs/glossary.md` read from the working directory -- which no installed deployment has, since +#: `docs/` sits at the repository root, outside the `keel/` module `uv_build` packages. Every +#: install therefore rendered an empty glossary, and `help_console.load_glossary`'s docstring said +#: so. It is a link now, for the same reason the client's is: linking is the only form of this +#: that reaches an installed deployment at all. NAV: tuple[tuple[str, str], ...] = ( ("/", "Status"), ("/setup", "Setup"), @@ -34,7 +46,7 @@ ("/rules", "Rules"), ("/venues", "Venues"), ("/gates", "Gates"), - ("/glossary", "Glossary"), + (DOCS_URL, "Docs"), ) _STYLE = """ @@ -304,6 +316,7 @@ def page( path: str, body: str, build: str = "", + version: str = "", refresh_sec: int | None = None, ) -> str: """The document shell. `refresh_sec` emits a `` -- a zero-JS @@ -313,7 +326,29 @@ def page( nav_items = [] for href, label in NAV: on = ' class="on"' if href == path else "" - nav_items.append(f'{esc(label)}') + # An outbound entry is navigation, not a subresource, so `default-src 'none'` does not + # reach it. `noopener` still does matter: a new tab opened without it holds a + # `window.opener` handle back to a trading console on a token-bearing origin. + # + # `?v=` carries the running build, exactly as `static/js/docs.js` does for the client: + # the site pins `main` while an operator runs a tagged release, so a linked page can + # describe behaviour their build does not have. That skew is made VISIBLE rather than + # solved -- the build ends up in the URL bar of the page they are reading. `quote`, not + # an f-string, because a full version is `0.11.2+c1634a3fa17f` and a raw `+` in a query + # string decodes to a space. + # + # `version`, NOT `build`, and the two are different strings: `build` is the footer's + # human-readable LINE -- `keel 0.11.2+c1634a3fa17f (DIRTY) [checkout]` -- and the first + # spelling of this used it, putting that whole sentence percent-encoded into the query. + # Caught by looking at the rendered href, not by a test: both forms are non-empty + # strings and every assertion about "the link carries a version" passed. + away = "" + target = href + if href.startswith("https://"): + away = ' target="_blank" rel="noopener noreferrer"' + if version: + target = href + "?v=" + quote(version, safe="") + nav_items.append(f'{esc(label)}') nav = "".join(nav_items) meta_refresh = ( f'' if refresh_sec else "" @@ -948,34 +983,5 @@ def render_gates(gates: Sequence[Any], capabilities: Sequence[Any]) -> str: return "".join(parts) -def render_glossary(terms: Sequence[Any]) -> str: - parts = [ - '

Glossary

keel\'s vocabulary, and the fiqh terms it anchors to

', - '
', - ] - for term in terms: - marker = ' fiqh' if term.fiqh else "" - if term.fiqh and not term.stated: - marker = ' not stated in fiqh-basis' - parts.append(f"
{esc(term.term)}{marker}
") - parts.append(f"
{esc(term.definition)}
") - source = term.citation or term.source - if source: - parts.append(f'
{esc(source)}
') - parts.append("
") - if not terms: - # The normal state of an INSTALLED deployment, not a bug: `docs/glossary.md` is read from - # the working directory, and a deployment folder is a config, a database and an .env -- - # there is no docs checkout beside them. The TUI's help screen shows the same empty state - # for the same reason. Packaging the docs inside the artifact is D5's business (#438); - # until then this says which file is missing rather than implying the glossary is empty. - parts.append( - '

No glossary here. keel reads docs/glossary.md from ' - "the folder it is run in, and an installed deployment has no docs checkout beside " - "its config and database.

" - ) - return "".join(parts) - - def render_message(heading: str, detail: str) -> str: return f'

{esc(heading)}

{esc(detail)}

' diff --git a/keel/web/server.py b/keel/web/server.py index 65acdc8..4031740 100644 --- a/keel/web/server.py +++ b/keel/web/server.py @@ -280,17 +280,11 @@ def page_gates(_cfg: ServeConfig, _query: dict[str, list[str]]) -> tuple[str, st return "Gates", render.render_gates(GATES, CAPABILITIES), None -def page_glossary(_cfg: ServeConfig, _query: dict[str, list[str]]) -> tuple[str, str, int | None]: - from keel.commands.help_console import load_glossary - - return "Glossary", render.render_glossary(load_glossary()), None - - ROUTES: dict[str, Callable[[ServeConfig, dict[str, list[str]]], tuple[str, str, int | None]]] = { # First-run detection (#437): every page that reads the database serves the checklist when # there is no database to read, rather than a 500 whose real cause is that the user has not - # set anything up yet. `/venues`, `/gates` and `/glossary` are not wrapped -- none of them - # touches the deployment, and all three are useful before one exists. + # set anything up yet. `/venues` and `/gates` are not wrapped -- neither touches the + # deployment, and both are useful before one exists. "/": needs_database(page_status), "/setup": page_setup, "/activity": needs_database(page_activity), @@ -298,7 +292,6 @@ def page_glossary(_cfg: ServeConfig, _query: dict[str, list[str]]) -> tuple[str, "/rules": needs_database(page_rules), "/venues": page_venues, "/gates": page_gates, - "/glossary": page_glossary, } @@ -466,6 +459,22 @@ def run_setup_action(cfg: ServeConfig, key: str, form: dict[str, str]) -> Any: _JSON_CONTENT_TYPE = "application/json; charset=utf-8" +def _docs_version(cfg: ServeConfig) -> str: + """The build the nav's documentation link should report, or `""`. + + Read off `build_info` rather than off `cfg.build`, because those are different strings and + only one of them is a version: `cfg.build` is the footer's human-readable LINE + (`keel 0.11.2+c1634a3fa17f (DIRTY) [checkout]`), and putting it in a query string produced + `?v=keel%200.11.2%2B...%20%28DIRTY%29%20%5Bcheckout%5D`. This is the same field `/api/config` + hands the client for the same purpose (`payload.config_document`'s `"build"`), so both + front-ends report the identical string while both exist. + + `""` when there is no build info at all -- an unversioned link is honest, and a link claiming + `?v=unknown` is not. + """ + return str(getattr(cfg.build_info, "full_version", "") or "") + + 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` @@ -571,6 +580,7 @@ def _refuse(self, code: int, heading: str, detail: str) -> None: path="", body=render.render_message(heading, detail), build=self.cfg.build, + version=_docs_version(self.cfg), ), ) @@ -906,6 +916,7 @@ def do_GET(self) -> None: # noqa: N802 - stdlib's naming, not ours path=parsed.path, body=body, build=self.cfg.build, + version=_docs_version(self.cfg), refresh_sec=refresh, ), ) diff --git a/keel/web/static/css/keel.css b/keel/web/static/css/keel.css index db9962e..de68061 100644 --- a/keel/web/static/css/keel.css +++ b/keel/web/static/css/keel.css @@ -181,6 +181,22 @@ h2 { font-size: 1.05rem; margin: 2rem 0 0.6rem; } } .kv .v { font-size: 1.05rem; font-variant-numeric: tabular-nums; } +/* #539: a label that names a documented term links out to its definition. + `--muted`, NOT `--accent`: the label is the quietest text on the card and a link colour there + would pull the eye off the figure beside it, which is the thing the operator came to read. The + dotted underline is what marks it as a link -- and it is an underline rather than colour alone, + because #532's whole finding was that this palette had been separating meanings by hue. + `text-decoration-thickness` is set because a dotted border-bottom would sit below the descenders + and read as a divider instead. */ +.kv .k .doclink { + color: inherit; + text-decoration: underline dotted; + text-underline-offset: 0.2em; + text-decoration-thickness: 1px; +} +.kv .k .doclink:hover, +.kv .k .doclink:focus-visible { color: var(--fg); text-decoration-style: solid; } + .tablewrap { overflow-x: auto; /* A scrollable region must be reachable by keyboard. `tabindex="0"` on the wrapper (set in diff --git a/keel/web/static/index.html b/keel/web/static/index.html index 52b1923..f6ec75c 100644 --- a/keel/web/static/index.html +++ b/keel/web/static/index.html @@ -71,6 +71,23 @@
  • Rules
  • Venues
  • Gates
    • + +
  • Docs
    • diff --git a/keel/web/static/js/docs.js b/keel/web/static/js/docs.js new file mode 100644 index 0000000..adeb0e1 --- /dev/null +++ b/keel/web/static/js/docs.js @@ -0,0 +1,141 @@ +// @ts-check +/** + * Outbound documentation links (#539). **The app fetches, bundles and caches nothing.** + * + * ── WHY LINKING IS THE ONLY OPTION, NOT THE CHEAP ONE ─────────────────────────────────────── + * `docs/` lives at the REPOSITORY root, outside `keel/`, and `uv_build` packages the module root + * -- so no wheel has ever carried it. Every installed deployment, the signed desktop bundle + * included, renders an empty glossary today, and `keel/commands/help_console.py` says so in its + * own docstring: "an installed deployment has no docs/ checkout, and the help screen renders + * that notice as its empty state." + * + * That is not fixable by adding a packaging glob -- it is structural. Measured at #535: building + * with `artifacts` set and with `artifacts = []` produces byte-identical wheels, because + * `uv_build` ships the whole module root regardless of the key. Linking out is the only option + * that reaches an installed deployment at all. + * + * ── NO OFFLINE FALLBACK, DELIBERATELY ─────────────────────────────────────────────────────── + * No inline definitions, no cached snapshot, no entry in the service worker's `PRECACHE`. An + * operator running a trading engine has network by definition, and #538's whole argument is that + * a cached copy of something authoritative is worse than no copy: a definition that has since + * changed, presented as current, with nothing on screen to say which it is. + * + * ── THE ANCHOR CONTRACT, AND WHY A TEST HOLDS IT ──────────────────────────────────────────── + * `docs/glossary.md` states its own rule -- "Each entry is a `## term` heading, a definition, + * and a `Source:` line" -- and Astro emits kebab-cased IDs for those headings, so the anchor for + * a term is its heading kebab-cased. Nothing in either repository enforces that from the other + * side: a heading renamed upstream would break every deep link here **silently**, because a bad + * fragment is not an error, it is a page that opens at the top. + * + * `tests/web/test_doc_links.py` closes that by parsing this table and asserting every anchor + * exists as a heading in the named document, in this repository, where `docs/` is the source. + */ + +/** The published documentation root. One string, spelled once. */ +const SITE = "https://keeltrading.com/en/docs/"; + +/** + * The running build, for `?v=`. + * + * **Version skew is made VISIBLE here, not solved.** The site pins `main` while an operator runs + * a tagged release, so a linked page can describe behaviour their build does not have. Per- + * version documentation paths were rejected: `keeltrading.com/en/docs/v0.11.0/glossary` 404s + * today, and building versioned trees is work in the other repository plus a retention policy, + * across three languages and a sitemap. Carrying the version in the query string costs nothing + * and puts the operator's build in the URL bar of the page they are reading. + * + * Module state, written exactly once, at boot, by `main.js` -- the alternative is threading a + * version string through every render function to reach the four places that build a link. + */ +let version = ""; + +/** + * Record the build every documentation link should carry. Called once from `main.js`, from the + * same `/api/config` read that fills the footer. + * + * @param {string} build + */ +export function rememberVersion(build) { + version = typeof build === "string" ? build : ""; +} + +/** + * The URL for one document, optionally at one anchor. + * + * The trailing slash on the slug is not cosmetic: the site serves `…/docs/glossary/index.html`, + * and the un-slashed form is a redirect that some browsers resolve by dropping the fragment -- + * a deep link that lands at the top of the page, which is exactly the failure this module's + * anchor table exists to prevent. + * + * @param {string} slug a document slug, or `""` for the documentation index. + * @param {string} anchor a heading anchor, or `""` for the top of the page. + * @returns {string} + */ +export function documentUrl(slug, anchor) { + let url = SITE; + if (slug) url = url + slug + "/"; + if (version) url = url + "?v=" + encodeURIComponent(version); + if (anchor) url = url + "#" + anchor; + return url; +} + +/** The documentation index, for the header's outbound link. @returns {string} */ +export function indexUrl() { + return documentUrl("", ""); +} + +/** + * The labels this client puts on screen that name a term the documentation defines, mapped to + * where it is defined. + * + * **Keyed by the LABEL, not by the term.** The alternative -- tagging each call site with a term + * name -- spreads the decision across six views and makes "which words on this screen are + * defined somewhere" unanswerable without reading all of them. Keyed by label, the whole answer + * is this table, and `kv` consults it for every pair it builds, so a label that names a term is + * a link wherever it appears without a call site knowing. + * + * **Deliberately NOT exhaustive, and the omissions are the point.** `mode` reads `paper` or + * `live` and would need two different targets for one label; `evidence required` sits on the + * CAPABILITY gates (`keel.capabilities.GATES`), not the promotion gate, and linking it to + * `#promotion-gate` would be confidently wrong. A missing link costs a reader one search. A + * wrong one costs them their trust in every other link on the page. + * + * @type {Record} + */ +export const TERMS = { + autonomy: { slug: "glossary", anchor: "autonomy" }, + "autonomy configured": { slug: "glossary", anchor: "autonomy" }, + "autonomy lapses": { slug: "glossary", anchor: "autonomy" }, + "kill switch": { slug: "glossary", anchor: "kill-switch" }, + "rail 11": { slug: "glossary", anchor: "rail" }, + "withdrawal attestation (rail 17)": { slug: "glossary", anchor: "attestation" }, + "market session": { slug: "glossary", anchor: "market-clock" }, + session: { slug: "glossary", anchor: "session-bound-venue" }, + "paper stage": { slug: "glossary", anchor: "paper-mode" }, +}; + +/** + * A label as an outbound link to its definition, or `null` if the label names no term. + * + * `rel="noopener"` with `target="_blank"`: a new tab opened without it gets a `window.opener` + * handle back to this page, and this page is a trading console on a token-bearing origin. + * `noreferrer` too -- the server already sends `Referrer-Policy: no-referrer`, and a link that + * states it as well is one that keeps holding if this markup is ever read somewhere the header + * is not sent. + * + * @param {string} label + * @returns {HTMLAnchorElement | null} + */ +export function termLink(label) { + const target = Object.prototype.hasOwnProperty.call(TERMS, label) ? TERMS[label] : null; + if (!target) return null; + const anchor = document.createElement("a"); + anchor.className = "doclink"; + anchor.textContent = label; + anchor.href = documentUrl(target.slug, target.anchor); + anchor.target = "_blank"; + anchor.rel = "noopener noreferrer"; + // Named for a reader who arrives on the link out of context, and hears only the link text. + anchor.title = "Definition on keeltrading.com — opens in a new tab"; + return anchor; +} diff --git a/keel/web/static/js/main.js b/keel/web/static/js/main.js index be84587..bbd9eb2 100644 --- a/keel/web/static/js/main.js +++ b/keel/web/static/js/main.js @@ -34,6 +34,7 @@ */ import { read } from "./api.js"; +import { indexUrl, rememberVersion } from "./docs.js"; import { available, subscribe } from "./live.js"; import { activityView, @@ -127,6 +128,8 @@ const engineNode = must("engine"); const contentNode = must("content"); /** @type {HTMLElement} */ const buildNode = must("build"); +/** The header's outbound documentation link (#539); its href gains `?v=` once the build is known. */ +const docsNode = /** @type {HTMLAnchorElement} */ (must("docs-link")); /** * An element that `index.html` guarantees. Throwing beats rendering half a page: the two files @@ -619,16 +622,26 @@ function registerWorker(config) { } /** - * The footer's build line, read once. + * The build, read once, and the three things that depend on it. * * Once, not per poll: `/api/config` describes the binary that is answering, and that cannot * change without the process restarting -- at which point the session token is new, every fetch * is a 403, and the banner says so. A version string re-read four times a minute would be four * times a minute spent confirming a constant. + * + * **The first view is painted from INSIDE this callback (#539), and that ordering is deliberate.** + * Every documentation link carries `?v=` (`docs.rememberVersion`), and a link built before + * the build is known would carry no version until the next poll -- fifteen seconds of links that + * quietly do not say which build the reader is running, on exactly the first screen they see. + * `/api/config` is the one endpoint that opens no database, so this costs a single round trip on + * a loopback socket, and it cannot hang the app: `api.read` resolves with a stopped reading + * rather than rejecting, so `show` runs even with nothing listening on the port. */ void read("config").then((reading) => { - buildLine(buildNode, reading.data); - registerWorker(reading.data); + const config = reading.data; + rememberVersion((config && (config.build || config.version)) || ""); + docsNode.href = indexUrl(); + buildLine(buildNode, config); + registerWorker(config); + show(booted, false); }); - -show(booted, false); diff --git a/keel/web/static/js/render.js b/keel/web/static/js/render.js index 64f7d11..dd7efe2 100644 --- a/keel/web/static/js/render.js +++ b/keel/web/static/js/render.js @@ -56,6 +56,7 @@ */ import { equityChart } from "./chart.js"; +import { termLink } from "./docs.js"; import { instant } from "./format.js"; /** @@ -158,7 +159,14 @@ function plain(value) { */ function kv(label, value) { const wrap = el("div", "kv"); - wrap.append(el("span", "k", label)); + // #539: a label that names a documented term becomes an outbound link to its definition, + // here rather than at each call site. `docs.TERMS` is the whole answer to "which words on + // this screen are defined somewhere", and a label it does not know stays plain text. + const linked = termLink(label); + const key = el("span", "k"); + if (linked) key.append(linked); + else key.textContent = label; + wrap.append(key); const holder = el("span", "v"); if (value instanceof Node) holder.append(value); else if (typeof value === "string") holder.textContent = value; diff --git a/keel/web/static/sw.js b/keel/web/static/sw.js index 82507f5..a7c463c 100644 --- a/keel/web/static/sw.js +++ b/keel/web/static/sw.js @@ -70,6 +70,7 @@ const PRECACHE = [ `${BASE}css/keel.css`, `${BASE}js/api.js`, `${BASE}js/chart.js`, + `${BASE}js/docs.js`, `${BASE}js/format.js`, `${BASE}js/live.js`, `${BASE}js/main.js`, diff --git a/tests/commands/test_console_thinness.py b/tests/commands/test_console_thinness.py index 0491a49..51cbc01 100644 --- a/tests/commands/test_console_thinness.py +++ b/tests/commands/test_console_thinness.py @@ -102,10 +102,17 @@ def _console_module_paths() -> list[str]: #: hand-rolling percent-decoding on attacker-influenced input, which is a strictly worse trade #: than one named, scoped allowance. #: -#: Scoped to the module and the exact import, so it cannot widen: `urllib.request` in either file +#: `render` joined them at #539, for the same reason in the other direction: it BUILDS a query +#: string rather than splitting one. The nav's documentation link carries `?v=`, a full +#: version is `0.11.2+c1634a3fa17f`, and a raw `+` in a query string decodes to a space -- so the +#: choice was `quote(build, safe="")` or a hand-rolled encoder for "the characters a version +#: string might contain", which is a guess about an alphabet rather than a rule about one. +#: (`render` is deleted at #540 and this entry goes with it.) +#: +#: Scoped to the module and the exact import, so it cannot widen: `urllib.request` in any of them #: still fails, and `urllib.parse` anywhere else still fails. RULE5_IMPORT_ALLOWLIST: frozenset[tuple[str, str]] = frozenset( - {("server", "urllib.parse"), ("staticfiles", "urllib.parse")} + {("server", "urllib.parse"), ("staticfiles", "urllib.parse"), ("render", "urllib.parse")} ) diff --git a/tests/web/test_client_assets.py b/tests/web/test_client_assets.py index 29d80b8..22e46f1 100644 --- a/tests/web/test_client_assets.py +++ b/tests/web/test_client_assets.py @@ -52,17 +52,16 @@ #: appearing under `js/` without a test author noticing fails `test_the_client_ships_exactly_the_ #: declared_modules` rather than shipping unexamined. #: -#: `chart.js` and `live.js` arrived at #537, which is what the previous revision of this line -#: predicted ("a fifth module is a design decision (#537 adds `chart`, `live`, `docs`, `sw`) and -#: should arrive with the list updated, not silently"). `docs` (#539) is still to come, and its -#: absence here is what will make it arrive the same way. +#: `chart.js` and `live.js` arrived at #537 and `docs.js` at #539, which is what the first +#: revision of this line predicted ("a fifth module is a design decision (#537 adds `chart`, +#: `live`, `docs`, `sw`) and should arrive with the list updated, not silently"). #: #: **`sw` arrived at #538 and is deliberately NOT in this list**, because it is not under `js/`: #: a service worker's scope is its own directory, so `js/sw.js` would be scoped to `/static/js/` #: and could not answer a navigation to `/static/insights`. It sits at the static root instead, #: and `tests/web/test_pwa.py::test_the_worker_is_served_from_the_scope_it_must_control` asserts #: that placement rather than leaving it to whoever next reads the spec's file list. -_MODULES = ("main.js", "api.js", "render.js", "chart.js", "live.js", "format.js") +_MODULES = ("main.js", "api.js", "render.js", "chart.js", "live.js", "format.js", "docs.js") #: The modules held to "no arithmetic, no judgement, no derived display string", and the ONLY two. #: @@ -567,9 +566,31 @@ def test_the_client_loads_nothing_from_a_third_party_origin() -> None: a browser. This asserts we never ship the attempt, so the header is never the only thing standing between the page and an external request.""" html = _markup_only(_INDEX.read_text(encoding="utf-8")) - remote = re.findall(r'(?:src|href)="((?:https?:)?//[^"]*)"', html) + + # **`` is navigation, not a load, and #539 makes that distinction load-bearing.** The + # documentation link opens keeltrading.com in a new tab; nothing is fetched into this page, + # nothing is bundled and nothing is cached, so `default-src 'self'` is untouched by it (the + # spec: "Outbound links are navigation, not connections"). Every OTHER way a URL can appear in + # this markup is a subresource, and those stay same-origin: `src` on a script or an image, + # and `href` on a `` -- the stylesheet, the manifest and the icons. + # + # The check is narrowed rather than dropped. An earlier spelling of this test matched `href` + # anywhere, which would now pass only by listing the docs URL as an exception -- and an + # exception list is what turns a rule into a habit of adding to a list. + loads = re.findall(r'<(?:script|img|iframe|source|embed)\b[^>]*\bsrc="([^"]*)"', html) + loads += re.findall(r']*\bhref="([^"]*)"', html) + remote = [url for url in loads if url.startswith(("http://", "https://", "//"))] assert remote == [], f"index.html loads from another origin: {remote}" + # And what IS allowed outbound is exactly one link, to the documentation, opened safely. + anchors = re.findall(r"]*>", html) + outbound = [tag for tag in anchors if re.search(r'href="(?:https?:)?//', tag)] + assert len(outbound) == 1, f"expected one outbound link, found {len(outbound)}: {outbound}" + assert 'href="https://keeltrading.com/en/docs/"' in outbound[0], outbound[0] + # `noopener` is the one that matters: a tab opened without it holds a `window.opener` handle + # back to a trading console on a token-bearing origin. + assert 'rel="noopener noreferrer"' in outbound[0], outbound[0] + css = _CSS.read_text(encoding="utf-8") assert "@import" not in css, "a stylesheet that imports another can import a remote one" # `url()` covers fonts, background images and cursors in one check. There are none today, and diff --git a/tests/web/test_doc_links.py b/tests/web/test_doc_links.py new file mode 100644 index 0000000..ba5ece4 --- /dev/null +++ b/tests/web/test_doc_links.py @@ -0,0 +1,335 @@ +"""Documentation is linked, never embedded (#539) -- and the links are checked against `docs/`. + +**Why this module exists at all.** A broken deep link is not an error. `…/glossary/#rial` opens +the glossary at the top of the page, looking exactly like a link to a term that happens not to +scroll, and nothing anywhere reports it. keel's `docs/` is the SOURCE and keeltrading.com is the +mirror (`engine-docs.manifest.json` pins `CodeGateSoftware/keel@main`), so this repository is the +one place where a rename and the links that depend on it can be compared at all -- and this is +the comparison. + +The anchor contract is the one `docs/glossary.md` states about itself: "Each entry is a `## term` +heading, a definition, and a `Source:` line." Astro's slugger kebab-cases those headings into +ids, and `_slug` below reproduces that transformation. If the site ever changes slugger, this +module is what fails. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest + +from keel.web import render, staticfiles + +_REPO = Path(__file__).resolve().parents[2] +_DOCS = _REPO / "docs" +_DOCS_JS = staticfiles.STATIC_ROOT / "js" / "docs.js" +_RENDER_JS = staticfiles.STATIC_ROOT / "js" / "render.js" +_INDEX = staticfiles.STATIC_ROOT / "index.html" + +#: Slug -> the document in THIS repository it is published from. Mirrors +#: `keeltrading.com/engine-docs.manifest.json`, which is the site's own pin of the same pairs. +#: Only the slugs the app links to need an entry; a link to a slug absent here fails below, +#: which is the correct outcome for a link to a document nobody has confirmed is published. +_PUBLISHED: dict[str, str] = { + "glossary": "glossary.md", + "fiqh-basis": "fiqh-basis.md", + "operator-runbook": "operator-runbook.md", + "go-live-runbook": "go-live-runbook.md", +} + + +def _slug(heading: str) -> str: + """A `## heading` as the id the built site gives it. + + Lowercase, drop everything that is not a letter, digit, space or hyphen, then spaces to + hyphens -- GitHub's slugger, which is what `rehype-slug` implements and what the built site + was verified to emit (`id="rail"`, `id="instrument-attestation"`, `id="kill-switch"`). + """ + text = heading.strip().lower() + text = re.sub(r"[^\w\s-]", "", text, flags=re.UNICODE) + return re.sub(r"[\s_]+", "-", text).strip("-") + + +def _anchors(document: str) -> set[str]: + """Every anchor a document offers, from its `##`-and-deeper headings.""" + source = (_DOCS / document).read_text(encoding="utf-8") + return {_slug(match) for match in re.findall(r"^#{2,6}\s+(.+)$", source, re.MULTILINE)} + + +def _terms() -> dict[str, tuple[str, str]]: + """`docs.js`'s `TERMS`, as `{label: (slug, anchor)}`. + + Parsed rather than imported, for the same reason `test_client_assets.py` parses `main.js`'s + route table: there is no JavaScript runtime here. The parse is narrow on purpose, and + `test_the_term_parser_actually_found_something` is what stops a rewritten table from silently + matching nothing. + """ + source = _DOCS_JS.read_text(encoding="utf-8") + block = re.search(r"export const TERMS = \{(.*?)\n\};", source, re.DOTALL) + assert block is not None, "TERMS is not in the shape this parser understands" + found: dict[str, tuple[str, str]] = {} + pattern = re.compile( + r'^\s*(?:"(?P[^"]+)"|(?P[A-Za-z_][\w$]*))\s*:\s*' + r'\{\s*slug:\s*"(?P[^"]+)"\s*,\s*anchor:\s*"(?P[^"]+)"\s*\}\s*,\s*$' + ) + for line in block.group(1).splitlines(): + if not line.strip() or line.strip().startswith("//"): + continue + match = pattern.match(line) + assert match is not None, f"unparsed TERMS entry: {line}" + label = match.group("quoted") or match.group("bare") + found[label] = (match.group("slug"), match.group("anchor")) + return found + + +# -- the acceptance criterion ---------------------------------------------------------------------- + + +def test_every_anchor_the_app_emits_exists_in_the_source_document() -> None: + """**The acceptance criterion, and the reason a rename upstream cannot break a link quietly.** + + Each `(slug, anchor)` the client can emit is resolved to a document in `docs/` and checked + against the headings that document actually has. + """ + for label, (slug, anchor) in sorted(_terms().items()): + assert slug in _PUBLISHED, ( + f"{label!r} links to slug {slug!r}, which is not a published document -- add it to " + "_PUBLISHED here and to keeltrading.com's engine-docs.manifest.json, or link " + "somewhere that exists" + ) + available = _anchors(_PUBLISHED[slug]) + assert anchor in available, ( + f"{label!r} links to #{anchor} in docs/{_PUBLISHED[slug]}, which has no such heading. " + f"A heading was probably renamed; the link would open the page at the top and report " + f"nothing. Closest available: {sorted(a for a in available if anchor[:4] in a)}" + ) + + +def test_the_anchor_check_would_notice_a_renamed_heading() -> None: + """Mutation: the assertion above compares against real headings, not against anything. + + Without this, a `_anchors` that returned everything -- or a regex that matched nothing and so + made the set empty in a way `in` happened to tolerate -- would leave the criterion green and + meaningless. + """ + real = _anchors("glossary.md") + assert "kill-switch" in real, "the heading parser found no known term" + assert "kill-switch-renamed-upstream" not in real + + +def test_the_term_parser_actually_found_something() -> None: + """Guards every assertion that iterates `TERMS` against an empty parse.""" + terms = _terms() + assert len(terms) >= 5, terms + assert terms["kill switch"] == ("glossary", "kill-switch") + + +def test_the_slugger_matches_the_ids_the_built_site_emits() -> None: + """The transformation, pinned against ids observed in the built site rather than assumed. + + `dist/en/docs/glossary/index.html` was checked at #531 and carries `id="rail"`, + `id="attestation"`, `id="instrument-attestation"`, `id="kill-switch"`, `id="qabd"`, + `id="riba"`. These are those cases run backwards through `_slug`. + """ + assert _slug("rail") == "rail" + assert _slug("instrument attestation") == "instrument-attestation" + assert _slug("kill switch") == "kill-switch" + assert _slug("qabd") == "qabd" + assert _slug("DCA benchmark") == "dca-benchmark" + assert _slug("session-bound venue") == "session-bound-venue" + + +# -- the table cannot rot ---------- + + +def test_every_linked_label_is_a_label_the_client_actually_puts_on_screen() -> None: + """The other direction, and the one that keeps the table honest as views change. + + `kv` links a label by looking it up, so a label renamed in `render.js` does not break -- it + just silently stops being a link, and the entry here becomes an entry for a label that no + longer exists. This is what turns that into a failure. + """ + source = _RENDER_JS.read_text(encoding="utf-8") + emitted = set(re.findall(r'kv\("([^"]*)"', source)) + assert emitted, "no kv labels found -- this test would prove nothing" + for label in sorted(_terms()): + assert label in emitted, ( + f"{label!r} is in docs.TERMS but no view emits it as a kv label; it was probably " + "renamed in render.js, where the link would have vanished without a word" + ) + + +def test_the_ambiguous_labels_are_deliberately_absent() -> None: + """A wrong link costs more than a missing one, and these two are the wrong ones. + + `mode` reads `paper` or `live` -- one label, two definitions, and no way to pick. `evidence + required` sits on the CAPABILITY gates (`keel.capabilities.GATES`), not the promotion gate, so + `#promotion-gate` would be confidently and invisibly wrong. Pinned so that "the table looks + incomplete" never becomes a reason to complete it. + """ + terms = _terms() + assert "mode" not in terms + assert "evidence required" not in terms + + +# -- nothing is fetched, bundled or cached ---------- + + +def test_the_app_fetches_no_documentation() -> None: + """`docs.js` builds URLs and returns anchors. It opens no connection. + + `test_client_assets.py::test_fetch_appears_in_exactly_one_client_module` already pins that + `fetch` lives only in `api.js`; this is the narrower statement the issue asks for -- the + documentation module in particular never reads a document. + """ + source = _DOCS_JS.read_text(encoding="utf-8") + code = "\n".join( + line for line in source.splitlines() if not line.lstrip().startswith(("*", "/*", "//")) + ) + for forbidden in ("fetch(", "XMLHttpRequest", "EventSource", "import("): + assert forbidden not in code, f"docs.js reaches the network via {forbidden}" + + +def test_no_documentation_is_precached() -> None: + """#538's worker must not hold a copy either. A cached definition that has since changed, + presented as current, is the same failure as a cached balance in a milder register.""" + precache = (staticfiles.STATIC_ROOT / "sw.js").read_text(encoding="utf-8") + assert "keeltrading.com" not in precache + for name in sorted(p.name for p in _DOCS.glob("*.md")): + assert name not in precache, f"{name} is precached; documentation is linked, not shipped" + + +def test_no_documentation_prose_ships_inside_the_client() -> None: + """The definitions themselves stay in `docs/`. A copy in the client is a second source that + drifts, and drifts silently, because nothing compares them.""" + glossary = (_DOCS / "glossary.md").read_text(encoding="utf-8") + definitions = [ + line.strip() + for line in glossary.splitlines() + if len(line.strip()) > 60 and not line.startswith(("#", "Source:", "-", ">")) + ] + assert definitions, "no definitions found in the glossary -- this test would prove nothing" + client = "\n".join( + path.read_text(encoding="utf-8") + for path in sorted(staticfiles.STATIC_ROOT.rglob("*")) + if path.is_file() and path.suffix in (".js", ".html", ".css") + ) + for definition in definitions: + assert definition not in client, ( + f"a glossary definition is embedded in the client: {definition[:60]!r}" + ) + + +# -- the version, and the one URL ---------- + + +def test_links_carry_the_running_version() -> None: + """Version skew is made visible rather than solved: the site pins `main` while an operator + runs a tagged release, so the build goes in the URL bar of the page they are reading.""" + source = _DOCS_JS.read_text(encoding="utf-8") + assert 'url = url + "?v=" + encodeURIComponent(version)' in source + main = (staticfiles.STATIC_ROOT / "js" / "main.js").read_text(encoding="utf-8") + assert "rememberVersion(" in main, "nothing ever tells docs.js which build is running" + + +def test_the_first_paint_already_knows_the_version() -> None: + """`show` is called from inside the `/api/config` callback, so no link is ever built before + the build is known -- otherwise the first screen an operator sees carries links that do not + say which build they are reading about, for a whole poll interval.""" + main = (staticfiles.STATIC_ROOT / "js" / "main.js").read_text(encoding="utf-8") + config_block = main[main.index('void read("config")') :] + assert "show(booted, false);" in config_block, ( + "the first paint no longer waits for the build; documentation links would be unversioned " + "until the first poll" + ) + assert main.count("show(booted, false);") == 1, "the first paint happens twice" + + +def test_the_rendered_pages_version_their_documentation_link_too(running) -> None: # type: ignore[no-untyped-def] + """Both front-ends exist until #540, and the criterion is not "the client carries `?v=`". + + `quote(..., safe="")` rather than an f-string: a full version is `0.11.2+c1634a3fa17f`, and a + raw `+` in a query string decodes to a space. + """ + from tests.web.test_server import _request, _session + + _status, _headers, body = _request(running, "/", cookie=_session(running)) + assert render.DOCS_URL in body + # The fixture's build may be empty, in which case no `?v=` is correct -- an empty version is + # not a version to report. Asserted through the renderer instead, where a build is present. + versioned = render.page( + title="t", + path="/", + body="", + # The two are DIFFERENT strings, and passing both here is the point of this assertion: + # `build` is the footer's human-readable line and the first spelling of this feature put + # it in the query, yielding `?v=keel%200.11.2%2B...%20%28DIRTY%29%20%5Bcheckout%5D`. + build="keel 0.11.2+c1634a3fa17f (DIRTY) [checkout]", + version="0.11.2+c1634a3fa17f", + ) + assert render.DOCS_URL + "?v=0.11.2%2Bc1634a3fa17f" in versioned, ( + "the rendered nav's documentation link carries no version, or carries a raw `+`" + ) + assert "keel%200.11.2" not in versioned, "the build LINE leaked into the query string" + assert 'rel="noopener noreferrer"' in versioned + + +def test_the_documentation_root_is_spelled_the_same_in_both_front_ends() -> None: + """`render.py` and `docs.js` both link out while both front-ends exist. Two spellings is two + things to update at #540, and one of them would be missed.""" + js = _DOCS_JS.read_text(encoding="utf-8") + site = re.search(r'const SITE = "([^"]+)"', js) + assert site is not None + assert site.group(1) == render.DOCS_URL + assert render.DOCS_URL in _INDEX.read_text(encoding="utf-8") + + +# -- the deletions ---------- + + +def test_the_glossary_page_is_gone() -> None: + """`/glossary` rendered a file no installed deployment has. A link replaced it.""" + from keel.web import server + + assert "/glossary" not in server.ROUTES + assert not hasattr(render, "render_glossary") + assert not hasattr(server, "page_glossary") + + +def test_the_web_layer_no_longer_reads_the_glossary_file() -> None: + """`help_console.load_glossary` stays for the TUI until #541, but nothing under `keel/web/` + calls it any more -- the whole point being that the file is not there to read. + + Read through `ast` rather than by substring, and the difference is not fastidiousness: the + NAV comment in `render.py` explains this deletion by NAMING `load_glossary`, and a substring + scan would fail on the prose that documents the change. An AST sees identifiers, so a + docstring can say the word and only a call can fail the test. + """ + import ast + + for path in sorted((_REPO / "keel" / "web").rglob("*.py")): + tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) + used = { + node.id if isinstance(node, ast.Name) else node.attr + for node in ast.walk(tree) + if isinstance(node, (ast.Name, ast.Attribute)) + } + used |= { + alias.name + for node in ast.walk(tree) + if isinstance(node, ast.ImportFrom) + for alias in node.names + } + assert "load_glossary" not in used, path + assert "parse_glossary" not in used, path + + +@pytest.mark.parametrize("path", ["/glossary", "/static/glossary"]) +def test_the_glossary_path_is_a_404_on_both_front_ends(path: str, running) -> None: # type: ignore[no-untyped-def] + """Over the wire, on the rendered pages and on the client's own prefix.""" + from tests.web.test_server import _request, _session + + status, _headers, _body = _request(running, path, cookie=_session(running)) + assert status == 404, path diff --git a/tests/web/test_pwa.py b/tests/web/test_pwa.py index 9653268..e4e08f8 100644 --- a/tests/web/test_pwa.py +++ b/tests/web/test_pwa.py @@ -332,12 +332,13 @@ def test_the_client_registers_the_worker_only_after_a_successful_config_read() - leave the installed worker alone: it is the thing letting the operator read the page. """ main = (_STATIC / "js" / "main.js").read_text(encoding="utf-8") - assert "registerWorker(reading.data)" in main, ( + config_block = main[main.index('void read("config")') :] + assert "registerWorker(config)" in config_block, ( "the worker is not registered from the config read" ) assert re.search( r"const build = \(config && \(config\.build \|\| config\.version\)\) \|\| \"\";", main - ) + ), "registerWorker no longer derives the build from the config document" assert "if (!build) return;" in main, "a failed config read must register nothing" diff --git a/tests/web/test_render.py b/tests/web/test_render.py index f2999e2..a8f3cb4 100644 --- a/tests/web/test_render.py +++ b/tests/web/test_render.py @@ -11,7 +11,6 @@ from decimal import Decimal from keel.commands.brokers import BrokerInfo -from keel.commands.help_console import GlossaryTerm from keel.web import render XSS = '' @@ -70,20 +69,13 @@ def test_a_failed_adapter_row_shows_the_error_and_not_the_placeholders() -> None assert "WIRED" not in html -def test_a_fiqh_term_that_fiqh_basis_does_not_state_says_so() -> None: - """`stated=False` means fiqh-basis does not define the term and the glossary entry says that - rather than substituting a help-authored summary. Losing that marker in a new front-end would - turn a disclaimed gap into an apparent citation.""" - term = GlossaryTerm( - term="something", - definition="fiqh-basis does not state this.", - source="", - citation=None, - fiqh=True, - stated=False, - ) - html = render.render_glossary([term]) - assert "not stated in fiqh-basis" in html +# `test_a_fiqh_term_that_fiqh_basis_does_not_state_says_so` lived here and went with +# `render_glossary` at #539. The property it protected did NOT go: the "not stated" disclaimer is +# written into the definition text in `docs/glossary.md` itself -- which is why +# `help_console.parse_glossary` can DERIVE `stated` from it (`stated = "not stated" not in +# source.lower()`), and why `tests/commands/test_help_console.py` asserts it on the gharar entry. +# A reader following the deep link lands on that prose. What was deleted is a renderer for a file +# no installed deployment has ever had. def test_utc_is_used_and_a_broken_timestamp_does_not_raise() -> None: diff --git a/tests/web/test_server.py b/tests/web/test_server.py index fa2051d..714f1a7 100644 --- a/tests/web/test_server.py +++ b/tests/web/test_server.py @@ -35,7 +35,6 @@ "/rules", "/venues", "/gates", - "/glossary", ) @@ -969,10 +968,19 @@ def test_the_printed_url_is_the_one_that_carries_the_token( def test_the_nav_and_the_routing_table_agree() -> None: """A page with no nav entry is unreachable; a nav entry with no page is a 404 the user is - invited to click. Neither is caught by testing either side alone.""" + invited to click. Neither is caught by testing either side alone. + + **The nav has one entry that is deliberately not a route (#539).** `Docs` links out to + keeltrading.com, because `docs/` has never shipped inside a wheel and the page that used to + render it was empty in every installed deployment. It is separated here by its scheme rather + than by its label, so a second outbound entry needs no edit and an internal entry that loses + its route still fails.""" from keel.web import render - assert {href for href, _label in render.NAV} == set(web_server.ROUTES) + internal = {href for href, _label in render.NAV if not href.startswith("https://")} + outbound = {href for href, _label in render.NAV if href.startswith("https://")} + assert internal == set(web_server.ROUTES) + assert outbound == {render.DOCS_URL}, "an unexpected outbound nav entry" assert set(ROUTES) == set(web_server.ROUTES), "this test module's list drifted from the server"