phase: 03_system_per_host_panels

**Phase 03 final verification pass — all 4 tasks already in `complete/`; verified criteria, fixed 3 pre-existing defects.**

- **Verified live** (headless Chromium + curl against `:8090`, all 6 sources `ok`): 3 panels × six heroes in order `cpu,mem,temp,disk,gpus,net`; charts per panel `util,net[,gpu]` (gateway: no GPU chart/hole); legends name metrics only — no legend names a foreign host; every `<svg>` `role="img"` + host-scoped `aria-label`; 2 sections, 0 `<select>`; `grep -rn "renderCores|core_usage" src/dashboards/web/` → nothing; console error-free; new shot `data/logs/final-system-panels.png`.
- **Defect fixed (credential hygiene, invariant 6)**: `collectors.py` put the *dialed* URL into every `UpstreamError`, and that string lands in the health record (`/api/health`) and the `event` table (`/api/events` → page); `/api/config` also echoed `GUFO_BASE_URL`/`VLLM_BASE_URL` verbatim. Added `config.strip_userinfo()` (now what `Target.display_url` uses) and redacted all message/config URLs. Tests: `test_a_failing_host_never_echoes_its_token_in_the_message` (×3 paths), `test_a_failing_engine_never_echoes_credentials_in_its_url`, `test_a_dead_host_never_writes_its_token_into_the_feed`, `test_engine_urls_are_redacted_the_same_way_hosts_are`, `test_strip_userinfo_is_the_one_redaction_every_url_leaves_by`. (`parse_targets` already moves `HOST_TARGETS` userinfo into a header, so this is a backstop for hosts and a real fix for engine URLs.)
- **Defect fixed (invariant 2)**: `/system` footnote printed `Δ{HOST_INTERVAL}` as the measurement; it now prints each host's measured `window_seconds` (range when they disagree) — `rates over Δ10.0s · poll 10s`. Pinned by `test_system_footnote_prints_the_window_it_actually_measured`. Also fixed the page's stale "window in each header" note and the empty-state's `--token` (argv) advice → `--token-file`.
- `bash .agents/validate.sh` → **EXIT=0**: **472 passed, 7 skipped**, coverage **96.87%** (floor 94), `ruff check src tests agent` clean, `pyright` 0 errors, `node --check` clean · `uv run pytest -m live --live` → **7 passed** · `uv pip list --outdated` empty.
- Criteria: no multi-host legend ✅ · no per-core visual ✅ · six heroes/panel incl. no-GPU `n/a` uncoloured ✅ · per-core history still queryable ✅ (354 pts) · ≥3 shape screenshots ✅ (`system-driveripper/deskwork/gateway.png` + dark/light/stale) · pytest+coverage+lint ✅ · phases 01/02 unchanged (their pin tests + `/api/summary` shape tests pass; live UPS gap in AGENTS.md re-confirmed read-only against real upsd) ✅.
- **Docs**: AGENTS.md — invariant 6 rewritten to cover message/event redaction, `collectors.py` line, new "Δ is a measurement" rule, "hosts answering total" → "nine-figure fleet-totals panel" (the subtitle still counts hosts), test count 452→472, coverage 96.8→96.9, rewrapped an over-long line. README.md — same fleet-totals wording, `RATE_WINDOW_SECONDS` now covers every measured rate (not just tokens), agent route list completed (`/`, `/schema`, `?top=N`), `LIST CMD` in the diagram, new paragraph on URL redaction.
- **Deviation**: the criterion's literal `curl …/api/system/history?host=…&metric=cpu.usage_pct@core7` uses singular params that endpoint has never accepted (it takes `hosts=`/`metrics=` and ignores unknown ones). Verified 354 points via `…system/history?hosts=driveripper&metrics=cpu.usage_pct@core7&range=1h` and `/api/history?source=host&host=…&metric=…`; I did **not** add singular param aliases (unauthorized API surface). No live infra changes made — reads only.
- **Next pending phase**: `04_llm_chart_readability`.
This commit is contained in:
2026-10-17 04:42:09 -04:00
parent 8b048599f3
commit 9757e3c947
43 changed files with 1686 additions and 464 deletions
+292 -5
View File
@@ -388,6 +388,60 @@ def test_agent_tokens_never_reach_the_browser(tmp_path: Path, monkeypatch: pytes
store.close()
def test_a_dead_host_never_writes_its_token_into_the_feed(tmp_path: Path) -> None:
"""The second path a credential can take to a browser: the failure message.
`parse_targets` moves a userinfo token out of the URL and into a bearer header, so the
documented `HOST_TARGETS` path never puts one in `Target.url` — this is the backstop for
any `Target` that does (a hand-built one, a future parser), and the message path is where
it would matter: the poller keeps an `UpstreamError` string in the health record and logs
it as an event, so a userinfo-bearing URL there would sit in SQLite and be served on
`/api/events` forever (AGENTS.md invariant 6). Asserted on every endpoint a page reads,
not just `/api/config`.
"""
db = str(tmp_path / "dead-host-token.sqlite3")
store = SampleStore(db)
settings = Settings(
db_path=db,
host_interval=0.05,
host_targets=(Target(name="driveripper", url="http://SEKRIT-AGENT-TOKEN@127.0.0.1:1"),),
)
with TestClient(create_app(settings, store)) as client:
deadline = time.time() + 10
while time.time() < deadline:
health = client.get("/api/health").json()
if health["sources"] and health["sources"][0]["error"]:
break
time.sleep(0.05)
else:
pytest.fail("the dead host never recorded an error")
for path in ("/api/health", "/api/events", "/api/system/snapshot", "/api/summary"):
body = client.get(path).text
assert "SEKRIT-AGENT-TOKEN" not in body, f"{path} leaked the agent token"
# and the outage is still legible — redaction is not the same as silence
assert "driveripper" in client.get("/api/health").text
assert "unreachable" in client.get("/api/events").text
store.close()
def test_engine_urls_are_redacted_the_same_way_hosts_are(tmp_path: Path) -> None:
"""`GUFO_BASE_URL`/`VLLM_BASE_URL` go to the browser verbatim in `/api/config`, and unlike
`HOST_TARGETS` nothing parses a userinfo token out of them into a header — so a URL that
happens to carry credentials has to be redacted where it is served (invariant 6).
"""
db = str(tmp_path / "engine-redact.sqlite3")
settings = Settings(db_path=db, gufo_url="http://SEKRIT-ENGINE-TOKEN@deskwork:8731",
vllm_url="http://driveripper:8000")
store = SampleStore(db)
with TestClient(create_app(settings, store)) as client:
body = client.get("/api/config")
assert "SEKRIT-ENGINE-TOKEN" not in body.text
llm = body.json()["llm"]
assert llm["gufo"]["base_url"] == "http://deskwork:8731"
assert llm["vllm"]["base_url"] == "http://driveripper:8000" # untouched when clean
store.close()
def wait_for_state(client: TestClient, path: str, *, timeout: float = 5.0) -> dict[str, Any]:
deadline = time.time() + timeout
while time.time() < deadline:
@@ -522,13 +576,14 @@ def test_hero_reads_the_verdict_instead_of_a_threshold(client) -> None:
def test_severity_for_is_kept_as_the_documented_fallback(client) -> None:
"""The old call sites still work until the remaining phases convert each page.
`/` was converted in phase 02: the overview colours from the `severity` verdicts
`/api/summary` already sends, so it no longer carries a threshold of its own. The
three pages still to come must keep compiling against the fallback until they are.
`/` was converted in phase 02 and `/system` in phase 03: both colour themselves from
the `severity` verdicts the API already sends, so they carry no threshold of their
own. The two pages still to come must keep compiling against the fallback until they
are converted — deleting it early would break a page mid-migration.
"""
js = client.get("/static/app.js").text
assert "FALLBACK ONLY" in js
for page in ("/static/page-system.js", "/static/page-llm.js", "/static/page-ups.js"):
for page in ("/static/page-llm.js", "/static/page-ups.js"):
assert "severityFor" in client.get(page).text, f"{page} lost its fallback call site"
@@ -539,6 +594,198 @@ def test_overview_colours_from_the_payload_not_from_js_thresholds(client) -> Non
assert "severityFor" not in js
def test_system_page_colours_from_the_payload_not_from_js_thresholds(client) -> None:
"""`page-system.js` hero cards paint payload verdicts, not thresholds of their own.
Phase 03 task 02's promise: every colour on a host card comes from a `severity` dict
`severity.py` produced (`cpu.severity.usage_pct`, `memory.severity.used_pct`,
`disks.severity.worst_used_pct`, `gpus[].severity.busy_pct`, `cpu.severity.temp_c`).
A threshold written here could disagree with the overview's alerts; a hero with no
verdict prints `n/a` uncoloured, which is `Dash.hero()`'s contract.
"""
js = client.get("/static/page-system.js").text
assert "severityFor(" not in js, "the system page re-invented a threshold"
assert "Dash.hero(" in js
for hook in ("cpu.severity.usage_pct", "memory.severity.used_pct",
"disks.severity.worst_used_pct", "gpus[].severity.busy_pct",
"cpu.severity.temp_c"):
assert hook in js, f"the card no longer names {hook} as a colour source"
def test_system_host_card_is_six_heroes_without_per_core(client) -> None:
"""Each host card leads with exactly six heroes: CPU, MEM, TEMP, DISK, GPU, NET.
Asserted on the served script because there is no JS test runner (a locked anchor).
The two dense `.metrics` rows and the per-core heat bars are exactly what made the
card unreadable, and "no per-core anywhere on /system" is a promise a stray
`renderCores` call would silently break — the series stays stored, the page just
stopped drawing it (PLAN.md *Dropped visuals*).
"""
js = client.get("/static/page-system.js").text
card = js[js.index("function hostCard("):js.index("function renderHostCard(")]
slots = re.findall(r'<div class="(?:hero|hero-slot)[^"]*" data-k="([^"]+)"', card)
assert slots == ["cpu", "mem", "temp", "disk", "gpus", "net"], \
"the hero row is no longer the six numbers in the agreed order"
# per-core visualisation is gone from the card code entirely
assert "cores" not in card, "a per-core visual came back to the card"
assert "renderCores" not in js and "core_usage" not in js
# the NET slot is a slot with two lines and no verdict: a byte rate has no good/bad
assert 'data-k="net"' in card
assert not re.search(r'data-k="net"[^>]*data-sev', card), "the NET slot invented a colour"
# what the glance leaves out stays reachable, collapsed by default
assert "More detail" in card
for key in ("load", "freq", "dr", "dw", "dutil", "uptime", "details"):
assert f'data-k="{key}"' in card, f"{key} left the page instead of the glance"
# the panel owns its charts: the slots live inside the card, not on the page
for chart in ("util", "net"):
assert f'data-chart="{chart}"' in card, f"the panel lost its {chart} chart slot"
def test_system_page_keeps_no_fleet_wide_chart_or_metric_picker(client) -> None:
"""Nothing on `/system` plots two hosts, and nothing needs a selection to make sense.
TODO.md L4 is the instruction ("The system page doesn't need shared charts for
anything") and the overview's alerts + fleet table are what made removing them safe
(phase 02). Asserted on the served bytes because the only thing that would otherwise
notice a resurrected `<select>` is a browser: there is no JS test runner (a locked
anchor), and `test_no_page_carries_an_id_nothing_reads` cannot see ids that were deleted
correctly from the page but are still queried by the script.
"""
html = _code(client.get("/system").text)
for gone in ('id="chart-cpu"', 'id="chart-mem"', 'id="chart-disk"', 'id="chart-net"',
'id="chart-gpu"', 'id="chart-temp"', 'id="chart-detail"', 'id="detail-host"',
'id="detail-metric"', 'id="f-cpu"', 'id="f-load"', 'id="f-gpu"',
"Fleet totals", "Fleet charts", "Series detail", "<select"):
assert gone not in html, f"/system still carries {gone}"
# the two panels that are left are the per-host grid and the event feed
assert html.count("<section") == 2, "/system grew a panel back on top of the host grid"
assert 'id="hosts"' in html and 'id="events"' in html
# and the script must not reach for the deleted endpoints/ids either — a `#detail-host`
# query against a removed node is a silent null, so the picker would just never appear
js = client.get("/static/page-system.js").text
for gone in ("#chart-", "detail-host", "detail-metric", "detail-note",
"renderFleet", "makeCharts", "refreshGpuChart", "populateDetailMetrics"):
assert gone not in js, f"page-system.js still references {gone}"
def test_every_system_chart_is_drawn_inside_its_own_host_panel(client) -> None:
"""One host per chart: the legends of `/system` can never list two machine names.
The construction is a factory that takes the card, so a chart cannot be built against a
page-level id; the metric list is per host, so the GPU streams cannot leak from one
panel into another; and a panel is torn down (with its charts) when its host leaves the
payload, which is the bug class a keyed-by-host chart map exists to avoid.
"""
js = client.get("/static/page-system.js").text
builds = re.findall(r"new DashCharts\.LineChart\(([^,]+),", js)
assert builds, "the system page builds no charts at all"
# every chart is built from a `[data-chart]` slot inside the host card, never from a
# page-level id — which is what makes "one host per chart" structural, not a convention
assert all(re.search(r"(?:card|slot)\.querySelector\(.\[data-chart=", b) for b in builds), \
f"a chart is built outside a host panel: {builds}"
# three construction sites, one per slot (util, net, gpu) — never one per metric
assert len(builds) == 3, f"charts are not built one per panel slot: {builds}"
assert sorted(re.findall(r'data-chart=\\?"(\w+)"', " ".join(builds))) == ["gpu", "net", "util"]
factory = js[js.index("async function renderHostCharts("):js.index("function pruneHosts(")]
assert "hosts=${encodeURIComponent(host.name)}" in factory, \
"the per-host chart query is not scoped to one host"
assert "&metrics=" in factory and "state.range" in factory, \
"a host's charts need one metric list and the selected window"
# GPU series are named from the payload's own `gpus[].index`, never discovered/guessed
assert "gpu.busy_pct@" in js and "gpu.index" in js
# the GPU chart exists only while the host reports cards
gpu_slot = js[js.index("function hostChartSet("):js.index("function seriesFor(")]
assert re.search(r"if \(!wantsGpu && set\.gpu\)", gpu_slot), \
"a host that stops reporting GPUs keeps a GPU chart it has no data for"
pruner = js[js.index("function pruneHosts("):js.index("// ---- snapshot")]
assert "state.hosts.delete(name)" in pruner and "state.charts.delete(name)" in pruner, \
"a host that left the payload keeps a live panel and drawing charts"
def test_every_system_chart_names_itself_for_assistive_tech(client) -> None:
"""A 120px SVG is silence to a screen reader unless the page names it.
The phase-03 accessibility pass (task 04) found the per-host charts drawing anonymous
plots: the legend is a sibling element and labels nothing, so the `<svg>` itself must
carry `role="img"` + `aria-label` that says which host and which metrics. Pinned on the
served bytes because there is no JS test runner (a locked anchor).
"""
charts_js = client.get("/static/charts.js").text
# the shared LineChart applies the name to the svg, and only when the caller opted in
assert "if (this.opts.ariaLabel)" in charts_js, "LineChart stopped honouring ariaLabel"
assert "setAttribute('role', 'img')" in charts_js
assert "setAttribute('aria-label', String(this.opts.ariaLabel))" in charts_js
js = client.get("/static/page-system.js").text
# all three slots name themselves, and each name starts with the host the panel belongs
# to — an aria-label that cannot say *which machine* is not an accessible name here
assert js.count("ariaLabel: `") == 3, "a per-host chart slot lost its accessible name"
for slot in ("utilisation", "network traffic", "GPU busy"):
assert f"${{host.name}} \u2014 {slot}" in js or f"${{host.name}} — {slot}" in js, \
f"the {slot} chart has no host-scoped accessible name"
def test_system_event_feed_queries_the_source_transitions_are_stored_under(client) -> None:
"""Host transitions are stored under the *dashboard* name, not the sample source.
`poller.py` logs `Event(ts, self.dashboard, …)` — host jobs write `system`, the way
the UPS job writes `ups` — while *samples* use `host`. The `/system` feed queried
`source=host` since the first commit and therefore always rendered "no transitions
yet", even with a real outage (a gateway disconnect) sitting in the store. Pinned on
the served bytes because there is no JS test runner (a locked anchor).
"""
js = client.get("/static/page-system.js").text
feed = js[js.index("async function refreshEvents("):]
url = re.search(r"fetchJSON\('(/api/events[^']*)'\)", feed)
assert url, "the /system page no longer fetches the event feed"
assert "source=system" in url.group(1), \
"the /system event feed must ask for the source the poller stores events under"
assert "source=host" not in url.group(1), \
"source=host matches no stored event — the feed is empty forever"
def test_system_footnote_prints_the_window_it_actually_measured(client) -> None:
"""`Δ<window>` is the page's promise that a rate is a measurement (invariant 2), so the
number behind it has to come from the measurement.
The `window_seconds` beside the fleet is `HOST_INTERVAL_SECONDS`, while the window behind a
rate is the estimator's `RATE_WINDOW_SECONDS` — equal at the defaults, which is why the
footnote could quote the poll interval and look right. Set `RATE_WINDOW_SECONDS=30` and it
would have labelled every rate on the page with a window it never used. The footnote now
reads each host's own measured `window_seconds`, and prints a range when they disagree.
"""
js = client.get("/static/page-system.js").text
assert "function windowLabel(" in js, "the footnote lost its measured-window helper"
footnote = js[js.index("$('#foot-note').textContent"):js.index("await Promise.all(chartJobs)")]
assert "windowLabel(" in footnote
assert "host.window_seconds" in footnote, \
"the footnote is quoting the poll interval as a measurement again"
# a footnote that says "window" and a page that says "poll" must stay two separate claims
assert "poll ${Math.round(state.pollMs / 1000)}s" in footnote
def test_no_per_core_visual_library_survives_the_page(client) -> None:
"""The per-core heat bars left `/system` and `charts.js` with them (PLAN.md *Dropped
visuals* is about what the pages draw, not what the store keeps).
`renderCores` had one caller — the hero-card per-core block deleted in task 02 — so
keeping it would be dead markup generating unstyled `.core` spans; `.cores` was the CSS
that block needed. Both are checked absent here so neither can come back as a
half-change, while `sparkline`/`LineChart` stay for phase 04 and the UPS page.
"""
assert "renderCores" not in client.get("/static/charts.js").text
css = client.get("/static/dash.css").text
assert ".cores" not in css
for asset in ASSETS:
assert "core_usage" not in _code(client.get(asset).text), f"{asset} plots per-core"
# what phase 04 and the other pages still chart from
for name in ("LineChart", "sparkline", "renderLegend"):
assert name in client.get("/static/charts.js").text
def test_overview_is_exactly_two_panels(client) -> None:
"""The landing page answers one question: what is wrong, and what is the fleet doing.
@@ -682,7 +929,14 @@ def test_parse_range_refuses_a_window_it_cannot_interpret(raw: str) -> None:
def test_system_history_charts_every_configured_host_in_one_request(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
"""N hosts x M metrics, one round trip — the chart endpoint's whole reason to exist."""
"""N hosts x M metrics, one round trip.
`/system` no longer uses the N-hosts half of this (every chart belongs to one host and
the page issues one request per host, pinned by
`test_every_system_chart_is_drawn_inside_its_own_host_panel`); the shape is kept because
the metric-list half is exactly what the per-host query costs one request with, and
because the endpoint is a public interface for programmatic consumers.
"""
with build_client(tmp_path, monkeypatch,
host_targets=(Target("driveripper", DEAD_URL),
Target("deskwork", DEAD_URL))) as test_client:
@@ -702,6 +956,39 @@ def test_system_history_charts_every_configured_host_in_one_request(
assert narrowed["hosts"] == ["deskwork"]
def test_one_request_per_host_answers_every_per_host_chart(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
"""The exact query `/system` issues per host returns every series that panel draws.
`page-system.js` sends one request per host carrying the utilisation trio
(`cpu.usage_pct`, `mem.used_pct`, `disk.used_pct`), both network rates, and one
`gpu.busy_pct@<index>` per card the payload reported — so a name missing here is a chart
that silently reads "no samples yet" in the browser, and a renamed series in
`sources/host.py` fails here rather than in a screenshot. This is the metric-name half of
the contract; `tests/test_system_contract.py` holds the payload-path half.
"""
with build_client(tmp_path, monkeypatch,
host_targets=(Target("driveripper", DEAD_URL),)) as test_client:
store = test_client.app.state.ctx.store # type: ignore[attr-defined]
now = time.time()
metrics = ["cpu.usage_pct", "mem.used_pct", "disk.used_pct",
"net.rx_bytes_per_s", "net.tx_bytes_per_s", "gpu.busy_pct@0"]
for metric in metrics:
store.add("host", "driveripper", metric, 12.5, ts=now - 60)
store.add("host", "driveripper", metric, 25.0, ts=now - 30)
body = test_client.get("/api/system/history", params={
"hosts": "driveripper", "metrics": ",".join(metrics), "range": "1h"}).json()
assert body["hosts"] == ["driveripper"], "one panel, one host"
assert [item["metric"] for item in body["series"]] == metrics
assert [len(item["points"]) for item in body["series"]] == [2] * len(metrics)
# a second card the host does not have answers empty, never zero and never 404
gap = test_client.get("/api/system/history",
params={"hosts": "driveripper", "metrics": "gpu.busy_pct@1"}).json()
assert gap["series"][0]["points"] == []
def test_llm_snapshot_keeps_an_engine_that_never_answered_in_the_list(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
"""A configured-but-dead engine is listed as unreachable, not dropped (invariant 4)."""