diff --git a/.agents/phases/todo/12_nut_self_test_verdict/00_phase.md b/.agents/phases/todo/12_nut_self_test_verdict/00_phase.md new file mode 100644 index 0000000..86f1b18 --- /dev/null +++ b/.agents/phases/todo/12_nut_self_test_verdict/00_phase.md @@ -0,0 +1,55 @@ +# Phase 12 — A passed battery self-test is news, not a warning + +**Source:** `TODO.md` L3 — ""Last self-test: Done and passed" still shows up as a warning in the overview and UPS section" +**Story:** n/a +**Context:** `src/dashboards/sources/nut.py` (`assess()` — the explicit `health.checks[]` list, the "Last self-test" check at the `test_result` branch), `src/dashboards/alerts.py` (the overview alert rule that reads `health.checks[]`), `tests/test_nut.py` (the two existing "Last self-test" assertions, the `SimUps` test-result tests), `tests/fixtures/nut_apc.vars` / `nut_cyberpower.vars` (verbatim `LIST VAR` captures), `src/dashboards/sources/nut_sim.py` (the simulator's `PASSED`/`FAILED` window), `src/dashboards/web/pages/ups.html` + `src/dashboards/web/static/page-ups.js` (the check list the `/ups` card renders), `data/logs/` (screenshots) + +## Objective + +The `Last self-test` check grades the driver's own word about the last battery test, and a +passed test — the real drivers answer **`Done and passed`** — must read `ok`, not `warn`. +The current word lists (`"ok", "passed", "pass", "completed"` / `"failed", "fail", "error", +"aborted", "test incomplete"`) let the one phrasing this fleet's hardware actually uses fall +through to `warn`, so a *healthy* UPS carries a warning on the overview and the UPS page. +After this phase the verdict follows the test's outcome: pass → `ok`, fail → `fail`, +never-tested / unset → `unknown`, and the overview's alert feed stops naming it. + +## Dependencies + +- `05_ups_hero_battery_glyph` (complete) — the `health.checks[]` contract and the `/ups` + check list this phase changes. +- `11_admin_login` (todo) — **no shared files**: 11 touches `app.py`/`config.py`/`store.py`/ + the auth surface; this phase touches `sources/nut.py` + its tests + fixtures. Either order + runs; the only coupling is the shared `tests/test_nut.py` — apply whichever runs second on + top of the file state at that time. + +## Tasks + +1. `01_pin_self_test_wording.md` — capture the real drivers' `ups.test.result` values and pin the current (wrong) verdict in a failing test, so the fix has a before/after. +2. `02_verdict_map_for_test_results.md` — the `assess()` word map: pass phrasings → `ok`, fail phrasings → `fail`, "No test initiated"/unset → `unknown` (never `warn`); fixtures + `tests/test_nut.py` + the simulator's wording stay consistent. +3. `03_no_false_warning_overview_and_ups.md` — end-to-end: a passed test produces no alert on `/api/summary` and an `ok` chip on `/ups`; pinned by API test and a live screenshot. + +## Testing & Quality + +- Unit: `tests/test_nut.py` — the full word map (each pass phrasing `ok`, each fail phrasing + `fail`, `No test initiated` and unset `unknown`, case/whitespace-insensitive), driven from + the verbatim fixture wording where it exists and synthetic `UpsReading`s elsewhere. +- Integration: `tests/test_api.py` — a `health.checks[]` entry with a passed self-test + produces no alert in the `/api/summary` payload; a failed one does. +- The `SimUps` test-result tests (`tests/test_nut.py`) keep passing: the simulator's + `PASSED`/`FAILED` window words were already in the right tiers. +- Coverage: **>90%** on new/modified code; the overall gate (94%) must hold. + +## Completion Criteria + +- [ ] `printf 'LIST VAR apc ups.test.result\n' | nc 3051` (run from a host + that can reach upsd — the laptop still cannot, per the fleet facts) captured and + recorded in `tests/test_nut.py` / the fixture; the captured word asserts `ok` +- [ ] the "Last self-test" check renders `ok` (dimmed, no warning class) on `/ups` and does + not appear in the overview's Warnings panel; screenshots in `data/logs/` + (`self-test-passed-overview.png`, `self-test-passed-ups.png`) +- [ ] `uv run pytest` green, `uv run pytest --cov --cov-fail-under=94` green, + `uv run ruff check src tests agent && uv run pyright` clean, `bash .agents/validate.sh` + green +- [ ] no behavior change in completed phases: every other `health.checks[]` check (power + path, charge, runtime, battery age/temperature, load, alarms) grades exactly as before \ No newline at end of file diff --git a/.agents/phases/todo/12_nut_self_test_verdict/01_pin_self_test_wording.md b/.agents/phases/todo/12_nut_self_test_verdict/01_pin_self_test_wording.md new file mode 100644 index 0000000..d2b6aab --- /dev/null +++ b/.agents/phases/todo/12_nut_self_test_verdict/01_pin_self_test_wording.md @@ -0,0 +1,50 @@ +# Task 01 — Pin the self-test word the real drivers use + +**Phase:** `12_nut_self_test_verdict` · **Source:** `TODO.md:3` — ""Last self-test: Done and passed" still shows up as a warning in the overview and UPS section" + +## Objective + +Prove the bug in a test before touching the fix: the `Last self-test` check in +`assess()` must be shown to grade the drivers' real wording (`Done and passed`) as +`warn`, and the verbatim `ups.test.result` value must be captured from the running +upsd so the fixture/test asserts the hardware's actual sentence, not a memory of it +(AGENTS.md rule 7 — verify against the deployed upstream; the factory state both UPSes +answer is `No test initiated`, pinned in `AGENTS.md`'s fleet facts). + +## Work + +1. `tests/test_nut.py` — add the pinning test **first**: build the `UpsReading` the way + the two existing "Last self-test" tests do (the `SimUps`/`vars` helpers in the file), + with `test_result="Done and passed"`, and assert the current behaviour + (`check["status"] == "warn"`) with a comment that the *fix* flips it to `ok`. This is + the red test the whole phase turns green. +2. Capture the real value — from a host that can reach upsd (the laptop cannot yet, + per the fleet facts; `driveripper` itself, or the deployed dashboard's network): + `printf 'LIST VAR apc ups.test.result\nLIST VAR cyberpower ups.test.result\n' | nc + 3051` — record the exact words in `tests/test_nut.py` (an assertion per + UPS where the value is a stable driver constant) and note the capture date. If a test + has been run on the hardware, the value is the driver's pass word; if not, it is + `No test initiated` — either way it goes into the test, because the verdict map must + cover both. +3. `tests/fixtures/nut_apc.vars` / `nut_cyberpower.vars` — if the live `ups.test.result` + line differs from the captured fixture (a test has since run), re-capture with + `./scripts/capture-fixtures.sh` (the script scrubs serials; the test-result line is + not a secret) so the fixtures stay verbatim ground truth. +4. Confirm the symptom end-to-end while it is live: `curl -s + http://127.0.0.1:8090/api/summary` (signed in, per phase 11) shows the alert row for + the `Last self-test` check, and `/api/ups/snapshot`'s `health.checks[]` carries the + `warn` entry — the two surfaces the TODO names. + +## Testing & Quality + +- Unit: the pinning test (red now, green after task 02). +- Coverage: **>90%** on this task's new/modified code. + +## Completion Criteria + +- [ ] `uv run pytest tests/test_nut.py -k self_test` shows the new test **failing** on + the current code (or passing if the wording already graded `ok` — in which case + record that and task 02 narrows to the `No test initiated` branch only) +- [ ] the real `ups.test.result` word is captured from the running upsd and recorded in + the test suite with its capture date +- [ ] fixtures re-captured if the live value differs from the recorded one diff --git a/.agents/phases/todo/12_nut_self_test_verdict/02_verdict_map_for_test_results.md b/.agents/phases/todo/12_nut_self_test_verdict/02_verdict_map_for_test_results.md new file mode 100644 index 0000000..6931c18 --- /dev/null +++ b/.agents/phases/todo/12_nut_self_test_verdict/02_verdict_map_for_test_results.md @@ -0,0 +1,62 @@ +# Task 02 — Grade the test outcome, not the phrasing + +**Phase:** `12_nut_self_test_verdict` · **Source:** `TODO.md:3` — ""Last self-test: Done and passed" still shows up as a warning in the overview and UPS section" + +## Objective + +The `Last self-test` branch in `assess()` maps the driver's word to a verdict with two +hardcoded tuples, and any phrasing outside both falls through to `warn` — which is how +`Done and passed` (the real drivers' pass word) and `No test initiated` (the factory +state) both read as warnings. Rewrite the branch as an explicit outcome map: pass +phrasings → `ok`, fail phrasings → `fail`, "no test has run" phrasings and unset → +`unknown`. A warning for a *passed* test is the bug; a warning for a machine that never +ran a test is noise this phase removes too. + +## Work + +1. `src/dashboards/sources/nut.py` — the `test_result` branch in `assess()`: + - pass tier: the existing `("ok", "passed", "pass", "completed")` **plus** + `done and passed` (the live word captured in task 01, kept lowercase/trimmed — + keep the existing `strip().lower()` normalisation); + - fail tier: unchanged (`"failed", "fail", "error", "aborted", "test incomplete"`); + - new `unknown` tier: `no test initiated` (and the empty-string case) — reuse the + existing `None`-branch sentence ("no self-test result reported — run a battery + test") so the two "never tested" states say the same thing; + - the fall-through for any *other* phrasing stays `warn` (an unrecognised word is + the one thing worth a warning — a driver inventing a new word is drift worth + noticing), and the check keeps printing the raw word plus the `test_date` suffix. + - ASSUMPTION: `No test initiated` (the factory state on both UPSes, per AGENTS.md + fleet facts) maps to `unknown`, not `warn` — a pack that has never been + self-tested is a suggestion to run one, not a fault; the `None` branch already + says exactly that, so the two states read identically. +2. `tests/test_nut.py` — the task-01 red test now asserts `ok`; add the full map: + every pass phrasing (including the captured live word) → `ok`, every fail phrasing + → `fail`, `No test initiated` / empty / unset → `unknown` with the run-a-test + sentence, an unknown phrasing (e.g. `mystery`) → `warn`; case and whitespace + variations of the live word. +3. `tests/fixtures/nut_apc.vars` / `nut_cyberpower.vars` — if the captured live value + (task 01) is the factory `No test initiated`, the fixtures already carry it; if a + test has run, the fixture carries `Done and passed` — either way the fixture-driven + tests in `tests/test_ups_contract.py` / `test_nut.py` must pass against the new map + (the fixture's word is part of the hero contract's check list). +4. `src/dashboards/sources/nut_sim.py` — no code change expected: the simulator's + `PASSED` / `FAILED` window words are already in the pass/fail tiers; run the + `SimUps` test-result tests to prove it. If the simulator ever answers a *third* + word, add it to the map — do not special-case the simulator. + +## Testing & Quality + +- Unit: the full word-map table in `tests/test_nut.py` (the tiers above, plus + `test_date` suffix still printed on the pass/fail detail line). +- Regression: every pre-existing `test_nut.py` / `test_ups_contract.py` test passes + unmodified in intent (the fixtures' words land in the right tier). +- Coverage: **>90%** on this task's new/modified code. + +## Completion Criteria + +- [ ] `uv run pytest tests/test_nut.py tests/test_ups_contract.py` green, including the + task-01 test now asserting `ok` for `Done and passed` +- [ ] no phrasing the two real drivers answer (verified against the task-01 capture) + falls into `warn` +- [ ] the `unknown` tier's detail sentence is the existing "run a battery test" one, so + unset and never-initiated read the same diff --git a/.agents/phases/todo/12_nut_self_test_verdict/03_no_false_warning_overview_and_ups.md b/.agents/phases/todo/12_nut_self_test_verdict/03_no_false_warning_overview_and_ups.md new file mode 100644 index 0000000..23834c7 --- /dev/null +++ b/.agents/phases/todo/12_nut_self_test_verdict/03_no_false_warning_overview_and_ups.md @@ -0,0 +1,60 @@ +# Task 03 — No false warning on the overview and the UPS page + +**Phase:** `12_nut_self_test_verdict` · **Source:** `TODO.md:3` — ""Last self-test: Done and passed" still shows up as a warning in the overview and UPS section" + +## Objective + +Prove the fix end-to-end on the two surfaces the TODO names: a passed self-test must +produce **no alert** in `/api/summary` (the overview's Warnings panel) and an `ok` +entry in `/api/ups/snapshot`'s `health.checks[]` (the `/ups` check list), while a +*failed* test still alerts — the check keeps its reason for existing. + +## Work + +1. `tests/test_api.py` — the alert-absence pin: an app (the shared `create_app` + test harness, `auth_contract` seeding) whose UPS payload carries a `Last self-test` + check with a passed word answers `/api/summary` with **no** alert row naming it + (the `alerts` list is the thing the overview renders — a row there is the warning + the TODO sees); the same harness with a `fail`-tier word **does** produce the + alert, so the test pins both directions. +2. `src/dashboards/web/static/page-ups.js` / `src/dashboards/web/pages/ups.html` — no + code change expected (the check list renders `health.checks[]` verdicts verbatim); + verify the `ok` chip styling for the "Last self-test" row is the existing dim path + (`ok` stays quiet ink, per the shared verdict vocabulary). If the row needs no + markup, say so in the completion note — do not restyle. +3. Live verification (AGENTS.md invariant 10 — UI changes are screenshotted): the dev + server (`./scripts/dev-server.sh status` first; the source must be `ok`), signed in + with the dev password: + - `curl -s http://127.0.0.1:8090/api/summary` — no `Last self-test` alert while the + live UPSes report a passed (or never-initiated) result; + - the `/` overview: Warnings & errors panel shows no self-test row; + - the `/ups` page: the "Last self-test" row renders the driver's word with the `ok` + (or `unknown`) class. +4. `data/logs/` — `self-test-passed-overview.png` and `self-test-passed-ups.png` + (the overview with the Warnings panel visible, and the `/ups` card with the check + list visible). If the live UPSes are at the factory `No test initiated`, the + screenshots show the `unknown` dim state — the *absence of the warning* is the + point, in either state. + - ASSUMPTION: no battery test is run on the live hardware to force a `Done and + passed` state — a self-test puts real load on a battery things are plugged into, + which stays the operator's call (the AGENTS.md known-gaps rule). The hermetic + `fail`/`ok` pins in `tests/test_api.py` carry the behaviour; the live screenshots + carry whichever state the hardware currently reports. + +## Testing & Quality + +- Integration: the two-direction alert pin in `tests/test_api.py`. +- Live: the two surfaces checked per step 3, screenshots recorded. +- Coverage: **>90%** on this task's new/modified code (mostly test code — the overall + 94% gate must hold). + +## Completion Criteria + +- [ ] `curl -s http://127.0.0.1:8090/api/summary | grep -i "self-test"` returns nothing + while the live `health.checks[]` entry is `ok`/`unknown`, and returns the alert + row when a `fail`-tier word is injected (the `tests/test_api.py` pin) +- [ ] `/` renders no self-test row in Warnings & errors; `/ups` renders the check with + its verdict class — both screenshotted in `data/logs/` +- [ ] `uv run pytest` green, `bash .agents/validate.sh` green +- [ ] the `Dev server stays up` rule held: `./scripts/dev-server.sh status` shows the + `ups` source `ok` before and after the screenshots diff --git a/.agents/phases/todo/13_llm_page_and_decode_rate/00_phase.md b/.agents/phases/todo/13_llm_page_and_decode_rate/00_phase.md new file mode 100644 index 0000000..6502514 --- /dev/null +++ b/.agents/phases/todo/13_llm_page_and_decode_rate/00_phase.md @@ -0,0 +1,60 @@ +# Phase 13 — The LLM page: fleet spacing and an honest decode tok/s + +**Source:** `TODO.md` L4–L5 — "In the LLM engines page the 'fleet' section has no spacing between it and the engine summaries" / "Decode tok/s sometimes jumps unrealistically high, could be a calculation error" +**Story:** n/a +**Context:** `src/dashboards/web/pages/llm.html` (the `#banners`/`#fleet`/`#strip`/engines section stack), `src/dashboards/web/static/dash.css` (`.strip`, the `.fleet` block, the shared `.85rem` rhythm), `src/dashboards/rates.py` (`RateEstimator` — the one piece of mutable state in the app), `src/dashboards/sources/llm.py` (`parse_gufo_metrics`/`parse_vllm_metrics`, the `measured()` Δcounter/Δt cells), `src/dashboards/prom.py` (`scalar(agg="sum")` over label sets), `tests/test_rates.py` / `tests/test_llm_source.py`, `tests/fixtures/metrics_gufo.txt` / `metrics_vllm.txt`, `data/logs/` (findings + screenshots) + +## Objective + +Two defects on the `/llm` page, one phase. (1) The `#fleet` aggregate block and the +`#strip` engine summaries sit flush — the page's shared `.85rem` vertical rhythm skips +the two sections that have no inline margin, so the page reads as one unbroken wall +above the engine panels. (2) The `decode tok/s` hero/chart occasionally prints values +far above anything the engines can produce: the measured Δcounter/Δt path is keyed +correctly by *name* (rule 2), but the *value* it diffs is a **sum over the label +set**, and the estimator's window-empty fallback divides by a `dt` it never bounds — +so a counter's carried value can leak into one interval's delta, or a stall can +rescale the window. The fix keeps every number measured and labelled (`Δ`), +just makes the measurement honest. + +## Dependencies + +- `07_llm_monitor_style_pages` (complete) — the fleet block, the strip, and the + one-`/api/llm/history`-request-per-refresh contract this phase must not break. +- `04_llm_chart_readability` (complete) — the hero row and the `provenance()` tags. +- `11_admin_login` (todo) — no shared files (this phase touches `dash.css`, `llm.html` + optionally, `rates.py`, `sources/llm.py`, `prom.py` and their tests; 11 touches + `app.py`/`config.py`/`auth.py`). Either order runs. + +## Tasks + +1. `01_fleet_strip_spacing.md` — restore the shared `.85rem` rhythm between `#fleet` and `#strip` (and above `#fleet` when no banners are shown) in `dash.css`; screenshot. +2. `02_pin_decode_spike_mechanism.md` — diagnose the spike against the live engines and the stored history (AGENTS.md rule 7 — never guess the trigger); written findings in `data/logs/`. +3. `03_bounded_estimator_and_per_series_rates.md` — the `rates.py`/`sources/llm.py` fix: per-label-series diffing plus a bounded effective window, so a label-set change contributes zero and a stall contributes an `n/a`, never a spike. +4. `04_live_stable_verification.md` — the dev-server proof: sustained watch of the live `decode tok/s` with its `Δ` label, screenshots, fixtures re-captured if the live shape changed. + +## Testing & Quality + +- Unit: `tests/test_rates.py` — the new window/label-set behaviours (see task 03); the + existing pinned behaviour (`test_window_is_labelled_with_the_span_actually_used`) + keeps passing — the labelled-window contract is load-bearing. +- Unit/integration: `tests/test_llm_source.py` — a metrics text whose label set gains a + series between two polls produces no spike in `decode_tokens_per_second`; the + fixture-driven parse tests pass unmodified in intent. +- Page: `tests/test_api.py` — the served `llm.html` + `dash.css` pin the spacing rule + (the same device that pins the other dash.css invariants). +- Coverage: **>90%** on new/modified code; the 94% gate must hold. + +## Completion Criteria + +- [ ] `/llm` shows the shared gap between the fleet block and the engine summaries, and + between the page head and the fleet block when no banners render — screenshots in + `data/logs/` (`llm-fleet-spacing.png`) +- [ ] the task-02 findings file exists and names the confirmed mechanism +- [ ] `tests/test_rates.py` + `tests/test_llm_source.py` pin: no spike on a label-set + change, no unbounded `dt`, the `Δ` label always the window actually used +- [ ] a sustained live watch shows no `decode tok/s` value above the confirmed + physical ceiling for the fleet's cards (the task-02 note records the ceiling) +- [ ] `uv run pytest` green, `bash .agents/validate.sh` green +- [ ] no behavior change in completed phases: the strip, the fleet block, the slot grid + and the trend charts render exactly as phase 07/04 shipped them, apart from the gap diff --git a/.agents/phases/todo/13_llm_page_and_decode_rate/01_fleet_strip_spacing.md b/.agents/phases/todo/13_llm_page_and_decode_rate/01_fleet_strip_spacing.md new file mode 100644 index 0000000..a4973e9 --- /dev/null +++ b/.agents/phases/todo/13_llm_page_and_decode_rate/01_fleet_strip_spacing.md @@ -0,0 +1,59 @@ +# Task 01 — The gap between the fleet block and the engine summaries + +**Phase:** `13_llm_page_and_decode_rate` · **Source:** `TODO.md:4` — "In the LLM engines page the 'fleet' section has no spacing between it and the engine summaries" + +## Objective + +`#llm`'s top-level sections break the page's shared `.85rem` vertical rhythm: +`#banners` ends with `margin-bottom:.85rem`, the engine section below `#strip` carries +an inline `margin-top:.85rem`, but `#fleet` and `#strip` carry **no** margin at all — +so the fleet block and the engine summaries sit flush (and, with no banners, the fleet +block also sits flush against the page head). Restore the rhythm with two CSS rules; +no markup or JS change. + +## Work + +1. `src/dashboards/web/static/dash.css` — in the fleet/strip section: + - `#strip { margin-top: .85rem; }` — the gap the TODO names (fleet → engine + summaries). Unconditional on purpose: when `#fleet` is `hidden` (the + below-two-engines case) the strip is the first visible section and still needs + the same top gap. + - `#banners:empty ~ #fleet { margin-top: .85rem; }` — the gap above the fleet block + *only* when no banner occupies the slot above it; with a banner present, the + banner's own `margin-bottom:.85rem` already is the gap (never two stacked gaps). + - The engine section's existing inline `margin-top:.85rem` stays (it already is the + strip → panels gap). +2. `src/dashboards/web/pages/llm.html` — no change expected; if the executor finds the + sections are not siblings the way the markup suggests, fix the CSS selectors to the + actual structure rather than re-flowing the markup (the section order is a phase-07 + contract). +3. `tests/test_api.py` — pin the two rules on the served bytes (the existing dash.css + pin device: a test that fails if the rules are missing from the served stylesheet), + so a later "cleanup" that drops them fails a test. +4. Screenshot (invariant 10): dev server up, `/llm` with **≥2 engines answering** (the + fleet block visible — below two engines it stays hidden and the fix is invisible), + full page top-to-bottom: `data/logs/llm-fleet-spacing.png`. If the live fleet cannot + show two engines (the transport gap in AGENTS.md's fleet facts), pin the spacing on + the simulated path is **not** an option for a layout claim — instead screenshot the + one-engine page (strip visible, `#fleet` hidden, strip's `margin-top` doing its job) + and record in the completion note that the two-engine gap was verified by the CSS + rule + the served-bytes test, with the live two-engine capture deferred to the + operator's engine restore. + - ASSUMPTION: the fix is CSS-only in `dash.css`, not an HTML wrapper change — the + page's section stack is a phase-07 layout contract and the page already uses + inline `margin-top:.85rem` for the same rhythm, so a rule (not a restructure) is + the change in the page's own vocabulary. + +## Testing & Quality + +- Page: the served-bytes pin in `tests/test_api.py`. +- Live: the screenshot per step 4. +- Coverage: **>90%** on new/modified code (CSS + test — the 94% gate must hold). + +## Completion Criteria + +- [ ] `#strip` has a `.85rem` top gap in all states (fleet visible, fleet hidden); + `#fleet` has the gap above it exactly when no banner is shown +- [ ] no double gap (banner + rule) and no flush pair anywhere in the section stack +- [ ] `tests/test_api.py` pin green; screenshot in `data/logs/` +- [ ] `bash .agents/validate.sh` green diff --git a/.agents/phases/todo/13_llm_page_and_decode_rate/02_pin_decode_spike_mechanism.md b/.agents/phases/todo/13_llm_page_and_decode_rate/02_pin_decode_spike_mechanism.md new file mode 100644 index 0000000..d353aa0 --- /dev/null +++ b/.agents/phases/todo/13_llm_page_and_decode_rate/02_pin_decode_spike_mechanism.md @@ -0,0 +1,72 @@ +# Task 02 — Pin the decode-tok/s spike against the live upstream + +**Phase:** `13_llm_page_and_decode_rate` · **Source:** `TODO.md:5` — "Decode tok/s sometimes jumps unrealistically high, could be a calculation error" + +## Objective + +Rule 7: never guess the trigger. Before touching `rates.py`, confirm **which** mechanism +produces the spike, from the stored history and the live engines, and record the +findings in a file the fix task (03) and the verification task (04) both read. + +## Work + +1. Find the spikes in the stored history (the dev DB is the source of truth — AGENTS.md + known gap 4 records it at ~610 MB with a year of raw/1m/1h rollups): + `sqlite3 data/… "" "SELECT ts, value FROM sample WHERE metric LIKE + 'decode_tokens_per_second%' ORDER BY ts DESC LIMIT 40000"` — list the top-N values + and their timestamps. Note for each: which engine (`source`/`host`), and what the + neighbouring values look like (a one-poll spike vs a sustained step). +2. For each spike, read the **provenance** of the cell that produced it — the payload + carries it (rule 2): `computed:Δ/Δt` + `Δ` (our estimator) vs the + engine's own gauge name (the native `decode_tokens_per_second_native` source string). + A spike from the *engine's* gauge is the engine's rolling average, not our math — + the fix there is a label, not a code change. `sqlite3` the engine's native gauge + series alongside to separate the two paths. +3. For the estimator path, inspect the live exposure (from a host that can reach the + engines — the laptop can reach both, per the fleet facts): + - `curl -s /metrics | grep -E "tokens_(predicted|generated)_total"` and the + vLLM equivalent — are the counters **plain** (as in `tests/fixtures/`), or + **labelled per slot/engine** in the live build? A label set that changes between + polls is the leading suspect: `prom.Prom.scalar(agg="sum")` sums whatever label + sets exist, so a series that appears with a carried cumulative value adds that + whole value to one interval's delta (a `value < samples[-1]` decrease would reset + the window — an *increase* from a new series does not). + - Cross-check the spike timestamps against `GET /slots` history (which slots were + starting/turning over) and the event feed (`/api/events`) for engine restarts in + the same minutes. +4. Record the findings in `data/logs/NOTES-decode-spike.md`: the confirmed mechanism + (one of the candidates below, or a new one), the evidence (values, timestamps, + captures), and the fleet's physical decode ceiling for the task-04 comparison — + the observed max of the native gauge over 90d **and** the cards' spec ceiling + (R9700 / Strix Halo 8060S), so "unrealistically high" has a number attached. + +Candidates, in suspected order (the findings file must name the one that fired): + - (a) **label-set sum**: `scalar(agg="sum")` over a changing set of labelled + series — a new series' carried counter enters the delta as a one-interval spike; + - (b) **window-empty fallback**: `rates.py`'s `base = samples[-2]` when fewer than + two samples sit inside the span — the effective window is the gap to the previous + sample, unbounded above by 2×span (the trim keeps the last two however old); + - (c) **a lineage jump that is not a decrease**: a counter that restarts *above* + zero (a renamed/relabelled series resolving under the same key, a pre-loaded + counter) — `value < samples[-1]` never fires, so the carried value is diffed; + - (d) **the engine's own native gauge** (vLLM's `avg_*_throughput` rolling average) — + not our math; label-level fix only. + +## Testing & Quality + +- No code changes in this task — diagnostics only, with the findings file as the + artifact. If a candidate is confirmed hermetically reproducible, add the red test + here (a `tests/test_llm_source.py` / `tests/test_rates.py` test that fails on the + current code) so task 03 turns it green. +- Coverage: **>90%** on any test added (the 94% gate must hold). + +## Completion Criteria + +- [ ] `data/logs/NOTES-decode-spike.md` exists: confirmed mechanism, evidence, and the + physical ceiling with its two sources (observed native-gauge max + card spec) +- [ ] every spike in the stored top-N is classified (our estimator / the engine's + gauge / a real burst) — a spike that cannot be classified is a finding, not a + skipped row +- [ ] a red test exists for the confirmed mechanism if it is hermetically reproducible +- [ ] the fixtures were compared against the live exposure and any shape drift is + recorded (drift → `./scripts/capture-fixtures.sh` re-capture noted for task 04) diff --git a/.agents/phases/todo/13_llm_page_and_decode_rate/03_bounded_estimator_and_per_series_rates.md b/.agents/phases/todo/13_llm_page_and_decode_rate/03_bounded_estimator_and_per_series_rates.md new file mode 100644 index 0000000..f2210fb --- /dev/null +++ b/.agents/phases/todo/13_llm_page_and_decode_rate/03_bounded_estimator_and_per_series_rates.md @@ -0,0 +1,85 @@ +# Task 03 — A rate that cannot be a carried counter + +**Phase:** `13_llm_page_and_decode_rate` · **Source:** `TODO.md:5` — "Decode tok/s sometimes jumps unrealistically high, could be a calculation error" + +## Objective + +Fix the confirmed mechanism from task 02, plus the two generic weaknesses that make +any mechanism possible: a summed counter whose *label set* changes between polls, and +an effective window the estimator never bounds. After this task, a delta that includes +a carried counter value is impossible by construction — a new series contributes +nothing until it has a baseline, a stalled window answers `n/a` for one poll instead of +rescaling, and a jump that looks like a lineage break is confirmed over two polls +before it is published. Every published number still carries its `Δ` label and +`source` string (rule 2). + +## Work + +1. `src/dashboards/prom.py` — expose the **per-label-set** view the fix needs (the + `series` map already holds them; `by_label` shows the shape): a `values_by_series` + (or reuse `values` with labels) accessor that returns `[(labels, value)]` per + resolved name, so the source can diff each labelled series individually. Keep + `scalar` untouched (the gauges still sum/avg/max the same way — a *gauge's* sum is + a real aggregate; only *counter* deltas may not mix series). +2. `src/dashboards/sources/llm.py` — the `measured()` path (gufo's + `tokens_generated_total`/`tokens_prompt_total` and vLLM's + `generation_tokens_total`/`prompt_tokens_total`): resolve the per-label-set values + and run **each series through the estimator under its own key** (the key is the + resolved name *plus* its label set — `llamacpp:tokens_predicted_total{slot="2"}`), + then `rate = Σ per-series rates`, with the cell's `window` the max of the + contributing series' windows (the window label says the widest window actually + used). A series with no previous sample contributes 0 tokens this interval — never + its carried value. The `Δ` label and the `computed:Δ…/Δt` source string stay + in the `provenance()` format the page already prints (the source string names the + resolved series, as today). + - ASSUMPTION: per-series diffing is the fix for candidates (a) and (c) — a + label-set change then changes *nothing* in the delta until the new series has two + samples, which is exactly the "a renamed counter starts a fresh window" doctrine + the estimator docstring already states, extended from names to label sets. +3. `src/dashboards/rates.py` — the two generic bounds: + - **stalled-window bound**: in the `len(inside) < 2` fallback, accept `samples[-2]` + as the base only while its age is ≤ 1.5 × `span`; older than that, return + `(None, None)` for this poll — one `n/a` on the poll after a stall is the honest + answer (a 20-second window on a 10-second panel is a different claim), and the + next poll recovers normally. The existing `test_window_is_labelled_with_the_span_actually_used` + (a 9s fallback) keeps passing: 9s ≤ 15s. + - **jump confirmation**: a published rate ≥ 8× the previous poll's published rate + (previous > 0) is held — the previous value is repeated for one more poll, and the + jump publishes only if *confirmed* by the next interval; an unconfirmed jump is + dropped (the cell prints the held value, its `Δ` label unchanged). This + suppresses a single-interval artifact without flattening a real burst: a genuine + step-up is confirmed one interval later and then publishes, and the confirmation + state is per-key in the estimator. + - ASSUMPTION: the 8×-over-previous threshold and the one-poll hold are the right + confirmation rule — a real LLM decode step (a batch of long outputs starting) + persists across a 10s interval, while a carried-counter artifact does not; the + thresholds are named constants at the top of the file so they can move without a + logic change. +4. `tests/test_rates.py` — the pins: a stalled window (previous sample > 1.5×span old) + answers `(None, None)` for one poll and recovers; a jump ≥ 8× holds for one poll + and publishes when confirmed / drops when not; the pinned pre-existing tests all + pass unmodified. +5. `tests/test_llm_source.py` — the label-set pin: two parses of a metrics text whose + counter series set differs between polls (a `{slot="2"}` series appears carrying + 50 000) — the delta of the second poll contains **no** 50 000; the flat-counter and + counter-reset tests keep passing. + +## Testing & Quality + +- Unit: `tests/test_rates.py` + `tests/test_llm_source.py` per the work list. +- Regression: every pre-existing `test_llm_source.py` / `test_rates.py` test passes + unmodified in intent; the fixture-driven parse tests (`metrics_gufo.txt`, + `metrics_vllm.txt`) are the shape pins. +- Coverage: **>90%** on this task's new/modified code; the 94% gate must hold. + +## Completion Criteria + +- [ ] no delta can contain a value that was not generated inside the window: the + label-set test, the reset test and the stall test all green +- [ ] a real step-up (sustained over two polls) still publishes — the confirmation + path is tested, not just the suppression path +- [ ] every published rate still carries `Δ` (the window actually used, possibly + a sum-of-windows label) and its `source` string — `tests/test_llm_source.py` + asserts the provenance shape +- [ ] the task-02 red test is green +- [ ] `bash .agents/validate.sh` green diff --git a/.agents/phases/todo/13_llm_page_and_decode_rate/04_live_stable_verification.md b/.agents/phases/todo/13_llm_page_and_decode_rate/04_live_stable_verification.md new file mode 100644 index 0000000..1dcae7f --- /dev/null +++ b/.agents/phases/todo/13_llm_page_and_decode_rate/04_live_stable_verification.md @@ -0,0 +1,50 @@ +# Task 04 — Live proof: a decode rate that stays under its ceiling + +**Phase:** `13_llm_page_and_decode_rate` · **Source:** `TODO.md:5` — "Decode tok/s sometimes jumps unrealistically high, could be a calculation error" + +## Objective + +The dev-server proof that the fix holds on the real engines: a sustained watch of the +live `decode tok/s` (payload cell + stored history) with its `Δ` label, every +observed value under the physical ceiling task 02 recorded, and the page itself +screenshotted with the task-01 spacing in view. + +## Work + +1. Dev server up and the LLM sources `ok` (`./scripts/dev-server.sh status`); record + the start time. Watch for at least one full poll-cycle cluster with traffic if the + engines are idle (the operator's own requests are the traffic — do **not** + generate load: a live test that puts real work on the engines' batteries is not a + test, the AGENTS.md live-suite rule): + - `curl -s http://127.0.0.1:8090/api/llm/snapshot` every poll for ≥ 10 minutes: + log `decode_tokens_per_second` (value, `source`, `window`) per engine; + - then `sqlite3` the `decode_tokens_per_second` raw series for the same period and + confirm no stored value exceeds the task-02 ceiling. +2. `data/logs/llm-decode-stable.png` — the `/llm` page showing the hero row with the + `Δ` provenance tag, the strip card, and (when ≥ 2 engines answer) the fleet + block with the task-01 gap; a second screenshot if a real burst was observed + mid-watch (the confirmed-and-published path — the burst's `Δ` label visible). +3. `./scripts/capture-fixtures.sh` — re-capture **only if** task 02 recorded a live + shape drift (a label set the fixtures lack); otherwise leave the fixtures + untouched and say so. +4. `uv run pytest -m live --live` — the opt-in live suite (read-only per its rule; + the LLM engine tests skip when their source is unset). Record the tally in the + completion note (the fleet's transport gap means the host/disks live tests may + still fail — the LLM and UPS ones are this task's bar). + +## Testing & Quality + +- Live: the watch + `sqlite3` comparison per step 1, screenshots per step 2. +- No code changes expected in this task — if the watch finds a residual spike the + fix missed, the findings go back into `data/logs/NOTES-decode-spike.md` and task 03 + is re-opened (record the re-open in the completion note rather than fixing inline). + +## Completion Criteria + +- [ ] ≥ 10 minutes of live polls: no `decode_tokens_per_second` value (payload or + stored) above the task-02 ceiling; the `Δ` label present on every + non-`n/a` value +- [ ] `data/logs/llm-decode-stable.png` (and the burst screenshot if one occurred) +- [ ] fixture re-capture decision recorded; `bash .agents/validate.sh` green +- [ ] the live tally recorded, with the known transport gap acknowledged as the reason + any host/disks live failures are out of scope here diff --git a/.agents/phases/todo/14_store_performance/00_phase.md b/.agents/phases/todo/14_store_performance/00_phase.md new file mode 100644 index 0000000..804cbea --- /dev/null +++ b/.agents/phases/todo/14_store_performance/00_phase.md @@ -0,0 +1,66 @@ +# Phase 14 — The store: fast reads, short writes, no "database is locked" + +**Source:** `TODO.md` L7 — "Pages take forever to load and seem to lock up the database. Fix that." +**Story:** n/a +**Context:** `src/dashboards/store.py` (the `sample`/`event`/`meta` schema, `pick_res`, `history`, `series_names`, `stats`, `maintain`, the single `self._lock`, `busy_timeout=5000`), `src/dashboards/api.py` (the N-hosts × M-metrics history loops, `/api/metrics`, the polled `/api/health`), `src/dashboards/poller.py` (`add_many` on every successful poll — the write side that loses the `busy_timeout` race), `tests/test_store.py`, `tests/test_api.py`, `data/logs/` (the ~610 MB dev DB per AGENTS.md known gap 4 — the `store-error: OperationalError: database is locked` line) + +## Objective + +The page load time and the `database is locked` lines are one disease: the hot read +paths of a 610 MB / 140 MB-WAL database are **full-table operations on every request** +(`series_names()` scans for `DISTINCT metric`, the browser-polled `/api/health` runs +`stats()` which counts the whole `sample` table, `pick_res()` pays three COUNT +round-trips per series), while `maintain()`'s rollups are long whole-table write +transactions that hold the WAL write lock long enough for a poller's `add_many` to +outlive `busy_timeout=5000` and surface as the reported `store-error` (AGENTS.md +known gap 4). This phase makes every hot path bounded (indexes, one grouped query, +short-TTL caches) and every write short (windowed rollups, WAL checkpoint tuning), +with timing budgets pinned as tests so a regression is a red test, not a slow page. + +## Dependencies + +- the foundation (complete) — `store.py`/`api.py`/`poller.py` as shipped; the rollup + and retention contract in AGENTS.md invariant 9. +- `11_admin_login` (todo) — **shared files, run-order note**: 11 rewrites `api.py` + (the session gate) and adds `get_meta`/`set_meta`/`delete_meta` to `store.py`. Apply + this phase's `store.py`/`api.py` changes on top of the file state at the time it + runs; never restore a line the other phase owns. The two phases touch disjoint + *regions* (11: the meta helpers + the auth routes; 14: the query paths + the + maintain rollups) — the coupling is the file, not the logic. + +## Tasks + +1. `01_baseline_store_costs.md` — a production-shaped seeded store + the timing/EXPLAIN baseline for every hot path, recorded in `data/logs/`. +2. `02_no_full_scan_on_hot_paths.md` — `series_names`/`stats` off the per-request full scan (bounded caches), `pick_res` to one grouped query, the N×M history endpoints to one grouped round trip. +3. `03_windowed_rollups_and_wal_tuning.md` — `maintain()` rolls up only the window since `last_maintain`; WAL checkpoint + journal-size tuning so the 140 MB WAL stops growing between checkpoints. +4. `04_budget_gates_and_live_proof.md` — the timing budgets pinned as tests, the live dev-server proof (page load measured, zero `store-error` over a watch window), docs updated. + +## Testing & Quality + +- Unit/integration: `tests/test_store.py` — the budget tests (a seeded + production-shaped store: the task-01 recipe), the cache-invalidation behaviour, the + windowed-rollup correctness (a rollup run twice rolls the new window only, and the + 1m/1h values match a whole-table rollup on the same data — the windowing must not + change a single averaged value). +- Integration: `tests/test_api.py` — the N×M history endpoints' response shape is + byte-identical to today (only the cost changes), under the new budget. +- Live: the dev-server watch per task 04 (AGENTS.md invariant 10 — the dev server stays + up while work happens; invariant 9 — the DB stays bounded, on a volume in + deployment). +- Coverage: **>90%** on new/modified code; the 94% gate must hold. + +## Completion Criteria + +- [ ] the task-01 baseline and the task-04 "after" numbers are both in + `data/logs/NOTES-store-perf.md`, side by side +- [ ] the budget tests are green: the full `/system` history request, `/api/metrics`, + `/api/health` and a 90d `history()` call all inside their pinned budgets on the + seeded store +- [ ] a 25-minute dev-server watch (the task-04 recipe) shows **zero** `store-error` + events and page loads inside the budget — the known-gap-4 failure mode is gone, + not hidden +- [ ] `maintain()`'s write transaction is measurably shorter (the task-03 note) and + the WAL stays bounded across two maintain cycles +- [ ] `uv run pytest` green, `bash .agents/validate.sh` green +- [ ] no behavior change in completed phases: every history payload, rollup value and + event answer is identical to today's shape (the `tests/test_api.py` pins) diff --git a/.agents/phases/todo/14_store_performance/01_baseline_store_costs.md b/.agents/phases/todo/14_store_performance/01_baseline_store_costs.md new file mode 100644 index 0000000..9d2109e --- /dev/null +++ b/.agents/phases/todo/14_store_performance/01_baseline_store_costs.md @@ -0,0 +1,68 @@ +# Task 01 — What the hot paths actually cost + +**Phase:** `14_store_performance` · **Source:** `TODO.md:7` — "Pages take forever to load and seem to lock up the database. Fix that." + +## Objective + +A before/after exists only if the "before" is measured, not remembered. Seed a +production-shaped store, time every hot path of `store.py`/`api.py` against it, +`EXPLAIN QUERY PLAN` the queries that will change, and record the baseline in +`data/logs/NOTES-store-perf.md` — the numbers task 04 compares against. + +## Work + +1. `tests/test_store.py` — the seeded-store recipe (a fixture the budget tests of this + phase all reuse): an on-disk SQLite file (WAL, like production — the in-memory DB + hides the lock behaviour) seeded to the dev DB's *shape and retention semantics* + (`raw_keep_hours=48`, `minute_keep_days=30`, `hour_keep_days=400` — the + `SampleStore` defaults, i.e. what a 90 d chart actually reads: 1m/1h, never raw + beyond 48 h): + - ~3 hosts × ~30 series per host, 10 s cadence for the 48 h raw window; + - their 1m rollups for the 30 d window and 1h rollups for the 400 d window + (generated by calling `maintain()` — a few hundred MB, the dev DB's order of + magnitude, a temp file, never the dev DB itself: tests stay hermetic per the + AGENTS.md testing policy); + - ~2 000 event rows and the `meta` rows the app writes (`last_maintain` included). + The test asserts the row counts, not the wall time of seeding. + - ASSUMPTION: "production-shaped" means the dev DB's *shape and magnitude* (~610 MB, + per AGENTS.md known gap 4), reproduced in a temp file — the dev DB itself is + never opened by a test (tests stay hermetic, the AGENTS.md testing policy), and + a temp DB of the same shape is the honest substitute. +2. Measure (a script or the test's `--store-perf` marker; record in + `data/logs/NOTES-store-perf.md` with the machine's specs): + - `store.series_names()` (what `/api/metrics` runs per call) — wall time + + `EXPLAIN QUERY PLAN` (expected: a full scan of `sample`); + - `store.stats()` (what the browser-polled `/api/health` runs) — wall time + + EXPLAIN (expected: a full COUNT scan of `sample` + `event`); + - `store.pick_res()` × 3 + `store.history()` for one 90 d window (what one chart + series costs) — wall time + EXPLAIN; + - the full `/api/system/history` request shape (3 hosts × 9 series — the + `tests/test_system_contract.py` table, 27 `_history` calls) through the test app + — total wall time; + - a `maintain()` run on the seeded store — the write transaction's wall time + (the lock-holding window; note the `to_hour` step already scans the *entire* + 30 d 1m window on every run — the suspected long write) and the WAL file's + size before/after; +3. Record the baseline table in `data/logs/NOTES-store-perf.md`: path, wall time, + EXPLAIN shape, and the wall time of the poller's `add_many` (a 30-sample insert) + issued **concurrently** with a `stats()`/`series_names()` call — the race known gap + 4 describes, made visible as a number (how often does the insert wait, and does it + ever outlive `busy_timeout=5000`? run the race loop ~100× and count the waits). + +## Testing & Quality + +- Unit: the seeded-store fixture in `tests/test_store.py` (asserts row counts and the + rollup values match a second `maintain()` pass — determinism check). +- No behaviour changes in this task — measurement only. +- Coverage: **>90%** on the fixture code; the 94% gate must hold. + +## Completion Criteria + +- [ ] `data/logs/NOTES-store-perf.md` exists: the full baseline table (wall times, + EXPLAIN shapes, the concurrent-insert wait count), the machine's specs, and the + seeded DB's row counts +- [ ] the baseline confirms (or refutes — record the refutation) the three suspect + mechanisms: `series_names` full scan, `stats` full scan, `maintain`'s long write + window +- [ ] the fixture is green and deterministic (two runs, same rollup values) +- [ ] `bash .agents/validate.sh` green diff --git a/.agents/phases/todo/14_store_performance/02_no_full_scan_on_hot_paths.md b/.agents/phases/todo/14_store_performance/02_no_full_scan_on_hot_paths.md new file mode 100644 index 0000000..4c10516 --- /dev/null +++ b/.agents/phases/todo/14_store_performance/02_no_full_scan_on_hot_paths.md @@ -0,0 +1,70 @@ +# Task 02 — No full table scan on a per-request path + +**Phase:** `14_store_performance` · **Source:** `TODO.md:7` — "Pages take forever to load and seem to lock up the database. Fix that." + +## Objective + +Three per-request paths pay a whole-table price on a 610 MB database: +`series_names()` (a `SELECT DISTINCT metric` scan, on every `/api/metrics` call), +`stats()` (a `COUNT(*) GROUP BY res` over all of `sample` plus `event`, on every +browser-polled `/api/health`), and the N-hosts × M-metrics history endpoints +(`pick_res()` is three COUNT round-trips per series, so one `/system` chart request +is ~108 serialized round-trips through the single `self._lock`). Make every hot path +bounded: short-TTL caches for the two scanned answers, one grouped query where three +stand, and one grouped round trip where N×M stand. The response shapes are byte- +identical — only the cost moves. + +## Work + +1. `src/dashboards/store.py` — `series_names()`: an in-memory cache of the distinct + metric set (per `source`/`host` argument tuple, a 60 s TTL, invalidated on + `add`/`add_many`/`delete_series` via a dirty flag — a cache that outlives a write + answers "why is this chart empty" with yesterday's truth). `stats()`: cache the + `sample`/`event` counts the same way (30 s TTL) — the `/api/health` poll (the + browser's `app.js` masthead loop) must never run a full COUNT scan; `last_maintain` + stays read through `get_meta()` as today (the meta table is tiny; do not cache + what a three-row table costs nothing to read). +2. `src/dashboards/store.py` — `pick_res()`: replace the three per-`res` COUNT queries + with **one** `SELECT res, COUNT(*) … WHERE … ts >= ? GROUP BY res` (the three + resolutions in a single index-range pass), same decision logic, same return value. +3. `src/dashboards/api.py` — the four history endpoints' loops (system/disks/llm/ups — + the `[host, metric]` nesting in each) call `store.history()` per pair, which is + `pick_res()` (now 1 query) + the thinning query. Add the batched door in + `store.py`: `pick_res_many(hosts, metrics, span, max_points)` and + `history_many(hosts, metrics, span, max_points)` — one grouped COUNT over the whole + `host IN (…) AND metric IN (…) AND res IN (…) AND ts >= ?` range, and one + per-resolution thinning query (the per-series `LIMIT ?` tail-thinning kept by a + window function or a per-series subquery — the shape of each series' answer must + stay identical: ascending `(ts, value)`, `tail=True` semantics preserved). The + endpoints' response dicts are unchanged (the same series, the same order); the + `tests/test_api.py` pins are the contract. + - ASSUMPTION: one grouped round trip (plus one thinning query per resolution + present) is the target for the N×M endpoints — the alternative (N×M separate + queries, each now cheaper) leaves ~100 serialized round-trips on a 3-host request, + which on a 610 MB file is still the page-load tail; the grouped shape is the fix + the TODO asks for. The per-series thinning keeps `tail=True` (the "the chart must + have 'now'" docstring) exactly as today. +4. `tests/test_store.py` — the pins: `series_names()`/`stats()` answer from cache + within the TTL (a write after the read invalidates `series_names`; `stats` expires + on the clock); `pick_res_many`/`history_many` equal the per-series answers on the + seeded store (the byte-identical-shape test); a cache hit makes a second + `series_names()` call measurably not scan (assert via a query-counter hook or the + wall-time budget of task 04). + +## Testing & Quality + +- Unit/integration: per the work list, on the task-01 seeded store. +- Regression: every pre-existing `tests/test_store.py` / `tests/test_api.py` test + passes unmodified in intent — the response shapes are the contract. +- Coverage: **>90%** on this task's new/modified code; the 94% gate must hold. + +## Completion Criteria + +- [ ] no per-request path in `api.py` triggers a full-table scan of `sample` or + `event` (the task-04 budgets are the proof; `EXPLAIN QUERY PLAN` of the grouped + queries shows index-range access, recorded in the notes file) +- [ ] the N×M endpoints' answers are byte-identical to the per-series path on the + seeded store (the equality test) +- [ ] a fresh `series_names()`/`stats()` call costs the full scan (documented — the + first poll after a process start pays it once), a repeated call costs the cache +- [ ] `bash .agents/validate.sh` green diff --git a/.agents/phases/todo/14_store_performance/03_windowed_rollups_and_wal_tuning.md b/.agents/phases/todo/14_store_performance/03_windowed_rollups_and_wal_tuning.md new file mode 100644 index 0000000..33aac61 --- /dev/null +++ b/.agents/phases/todo/14_store_performance/03_windowed_rollups_and_wal_tuning.md @@ -0,0 +1,78 @@ +# Task 03 — A rollup that holds the write lock for seconds, not minutes + +**Phase:** `14_store_performance` · **Source:** `TODO.md:7` — "Pages take forever to load and seem to lock up the database. Fix that." + +## Objective + +`maintain()` is "safe to call every minute" and is idempotent — but on a real-sized +store the `to_hour` step's `SELECT … GROUP BY` scans the **entire 30-day 1m window** +on every run (and `to_minute` the entire 48 h raw window), and that GROUP BY is +inside the one `with self._lock, self._db:` transaction: a multi-second write +transaction that holds the WAL write lock long enough for a poller's `add_many` to +outlive `busy_timeout=5000` and surface as the `store-error: OperationalError: +database is locked` line (AGENTS.md known gap 4). Window each step to *what became +due since the last run* — the idempotent `INSERT OR REPLACE` makes a slightly wider +re-scan safe, so the window is a cost bound, not a correctness device — and tune the +WAL so its 140 MB growth stops being a per-run surprise. + +## Work + +1. `src/dashboards/store.py` — `maintain()`, both rollup steps: + - read `last_maintain` through `get_meta()` **before** the transaction (the + existing `stats()` note applies: the meta read is outside the non-reentrant + lock); + - `to_minute`: the raw scan bound moves from `ts >= now - raw_keep_seconds` to + `ts >= max(now - raw_keep_seconds, last_maintain - MINUTE - 60)` — steady state + (a run per minute) scans ~one minute of raw rows; a missed interval scans the + catch-up window; a first run on a fresh DB (no `last_maintain` row) scans the + full retention window exactly as today; + - `to_hour`: the same pattern against the 1m rows (`ts >= max(now - + minute_keep_seconds, last_maintain - HOUR - 60)`). The `ts < now - {MINUTE|HOUR} + - 60` incompleteness guard stays — a bucket is still only rolled once its whole + window is in the past. + - the `DELETE … pruned` step is unchanged (the retention window is the correct + bound for a prune; it is `DELETE`s on indexed ranges, not a GROUP BY). +2. `src/dashboards/store.py` — the WAL tuning in `__init__` (after the existing + pragmas): `PRAGMA wal_autocheckpoint = 1000` (the default 1000 pages ≈ 4 MB + auto-checkpoint is already the default — the 140 MB WAL means checkpoints lose to + the write rate; the real lever is the journal-size cap) and + `PRAGMA journal_size_limit = 67108864` (64 MB — the WAL stops growing past it; + checkpoints then truncate it). Keep `journal_mode=WAL` and + `synchronous=NORMAL` (the "a lost sample is fine, corruption is not" comment + stands); keep `busy_timeout=5000` — the fix is a shorter write, not a longer wait. + - ASSUMPTION: 64 MB is the journal-size cap — the dev DB's 140 MB WAL is the + measured "too big" (AGENTS.md known gap 4), 64 MB is the nearest power of two + below it, and the cap is a pragma, not a migration; if the live watch (task 04) + shows checkpoint truncation starving writers, the cap moves (named constant in + `__init__`, one-line change). +3. `tests/test_store.py` — the pins: + - **windowing correctness**: seed a store, run `maintain()` (full window, no + `last_maintain` yet), advance time and add new 1m buckets, run `maintain()` + again — the second run's `to_hour`/`to_minute` scan (assert via the EXPLAIN/row + count hook, or a `sqlite3` trace) touches only the since-`last_maintain` window, + **and** the rolled 1m/1h values are byte-identical to a reference whole-window + rollup on the same data (the windowing must not change a single averaged value — + `INSERT OR REPLACE` idempotency is what makes this safe, prove it); + - **catch-up**: no `maintain` for 10 minutes, then one run — the 10 minutes of + buckets are all rolled (the wider window, not a gap); + - **the lock race**: the task-01 concurrent loop (a poller-style `add_many` during + a `maintain()` run) now completes with **zero** `OperationalError`s over the + 100× loop (the baseline's wait count is the before, this is the after). + +## Testing & Quality + +- Unit: per the work list, on the task-01 seeded store. +- Regression: every pre-existing `tests/test_store.py` test passes unmodified in + intent (the idempotency/retention tests are the load-bearing ones). +- Coverage: **>90%** on this task's new/modified code; the 94% gate must hold. + +## Completion Criteria + +- [ ] the second `maintain()` run's write transaction is measured shorter than the + task-01 baseline (both numbers in `data/logs/NOTES-store-perf.md`), and the + rollup values are unchanged (the equality test) +- [ ] the 100× concurrent `add_many`-during-`maintain` loop: zero `OperationalError` + (baseline wait count vs after, in the notes file) +- [ ] two `maintain()` cycles on the seeded store: the WAL file size stays bounded by + the 64 MB cap (before/after sizes recorded) +- [ ] `bash .agents/validate.sh` green diff --git a/.agents/phases/todo/14_store_performance/04_budget_gates_and_live_proof.md b/.agents/phases/todo/14_store_performance/04_budget_gates_and_live_proof.md new file mode 100644 index 0000000..1fd61eb --- /dev/null +++ b/.agents/phases/todo/14_store_performance/04_budget_gates_and_live_proof.md @@ -0,0 +1,78 @@ +# Task 04 — The budgets, and the dev server that proves them + +**Phase:** `14_store_performance` · **Source:** `TODO.md:7` — "Pages take forever to load and seem to lock up the database. Fix that." + +## Objective + +The "after" numbers, pinned as tests so a regression is red instead of slow: the +task-01 baseline vs the fixed store, side by side in `data/logs/NOTES-store-perf.md`, +plus the live dev-server proof on the real 610 MB DB — page loads inside budget, zero +`store-error` events over a watch window (the known-gap-4 failure mode gone, not +hidden), and the docs updated. + +## Work + +1. `tests/test_store.py` / `tests/test_api.py` — the budget tests (the task-01 recipe + on the seeded store; budgets are the task-01 baseline's *target* — record the + chosen numbers in the notes file with the reasoning): + - the full `/api/system/history` request (3 hosts × 9 series, 1 h and 90 d): + wall time budget (the 90 d case reads the 1m/1h rollups — the one that used to + be the page-load tail); + - `store.series_names()` / `store.stats()` second call (cache hit): wall time + budget, asserted to be a fraction of the first call's (the scan is gone); + - one 90 d `store.history()` call: wall time budget; + - the budgets are generous (2–3× the measured "after"), so a slower CI machine does + not flake red — the notes file records both the machine's specs and the margin. +2. The live proof (AGENTS.md invariant 10 — the dev server stays up while work + happens, and a fix is screenshotted, not claimed; invariant 9 — the DB is the + volume's business in deployment, the dev DB is the test bench): + - `./scripts/dev-server.sh restart` (its `stop` waits for the old pid to die — + known gap 4's "two servers on one file" cause stays fixed), `status` shows the + sources; + - page loads: `curl -o /dev/null -s -w '%{time_total}'` on `/` and on the + history-heavy pages' API calls (signed in, per phase 11), five runs each, + recorded; the browser's `/llm` or `/system` page with a 90 d window selected is + the worst case — screenshot `data/logs/store-perf-live.png` (the page loaded, + the charts drawn, the masthead dots `ok`); + - the watch: ≥ 25 minutes of `./scripts/dev-server.sh logs` (longer than the + gap-4 baseline's "a few times an hour" rate) with **zero** + `store-error: OperationalError: database is locked` lines; the `store.stats()` + `event` count for `kind='store-error'` over the window is 0 (the `sqlite3` + check on the dev DB — read-only); + - the WAL file's size after the watch (the cap from task 03, in the real DB). + - ASSUMPTION: the watch window is 25 minutes against a "few times an hour" + baseline rate — enough to cover > 0.4 of the baseline's expected occurrences at + the mean rate; if the baseline was at the mean, a zero-count 25-minute window + already moves the needle, and the `sqlite3` event-table check over *the whole + dev DB's* `store-error` rows (not just the window) shows when the last + occurrence predates this phase. +3. Docs: `README.md` — the *Configuration*/*Development* notes gain the two new + `PRAGMA`s' one-line "why" (the journal cap, and that it is a cap not a target); + `AGENTS.md` — known gap 4's entry is updated to record the fix (the windowed + rollups, the journal cap, the budget tests) and the gap's "what remains" line + (checkpoint starvation was never shown to be a cause — the controlled 25 s read + transaction produced none — so the entry's honest wording stays, with the + `database is locked` line now attributed to the rollup's write window). (The + AGENTS.md/README update is the phase's *normal work* per the repo's definition of + done — this instruction is for the phase's executor, not for the conversion.) + +## Testing & Quality + +- Unit/integration: the budget tests per step 1. +- Live: the step-2 watch, screenshots and log evidence in `data/logs/`. +- Coverage: **>90%** on new/modified code; the 94% gate must hold. + +## Completion Criteria + +- [ ] `data/logs/NOTES-store-perf.md` has the baseline and the after table side by + side (every task-01 path, wall time before/after, EXPLAIN shapes) +- [ ] the budget tests are green with the recorded margins; `bash .agents/validate.sh` + green +- [ ] the 25-minute watch: zero `store-error` lines in the dev-server logs and zero + new `store-error` rows in the dev DB's `event` table; `data/logs/` holds the + page-load numbers and `store-perf-live.png` +- [ ] the WAL's post-watch size is inside the 64 MB cap on the real DB +- [ ] `README.md`/`AGENTS.md` carry the updated, accurate wording (gap 4: fixed, with + its "what remains" honest) +- [ ] no behavior change in completed phases: the `tests/test_api.py` payload pins all + pass — the answers are the same, only faster diff --git a/.agents/phases/todo/15_nut_tls_credentials/00_phase.md b/.agents/phases/todo/15_nut_tls_credentials/00_phase.md new file mode 100644 index 0000000..7d80fd4 --- /dev/null +++ b/.agents/phases/todo/15_nut_tls_credentials/00_phase.md @@ -0,0 +1,90 @@ +# Phase 15 — The upsd wire: a managed root CA, STARTTLS, and the account gate + +**Source:** `TODO.md` L6 — "For privileged actions (UPS commands) the exporter should be installed with password protection and that password must be used to register it with the collector. Effectively the exporters become protected web servers. Additionally, the collector should generate and manage a root certificate that needs to be installed with each exporter. In fact, just use the certs as auth rather than a password." +**Story:** n/a +**Context:** `src/dashboards/sources/nut.py` (the ~10-line `NutClient` wire protocol, `login()`'s `USERNAME`/`PASSWORD` verbs pinned by `test_login_uses_the_verbs_upsd_2_8_5_answers_to`, `assess()`/`COMMANDS` risk tiers), `src/dashboards/collectors.py` (`NutMonitor`, the redacted-URL error doctrine), `src/dashboards/api.py` (the one `POST /api/ups/command` and its four gates), `src/dashboards/config.py` (env-only settings), `tests/test_nut.py` (the fake upsd), `tests/fixtures/nut_apc.vars` / `nut_cyberpower.vars` (the wire-verb ground truth, incl. `HELP`'s advertised verbs: `GET LIST SET INSTCMD LOGIN LOGOUT USERNAME PASSWORD STARTTLS`), `scripts/`, `README.md` → *NUT* + +## Objective + +The privileged action in this app is `POST /api/ups/command`, and its security today +rests on the `upsd.users` account (the password the operator installed on the +exporter, carried in the collector's config and used at `LOGIN` before every +`INSTCMD` — the "register with the collector" half is already true) plus the four +gates. What it does **not** have is transport security: the account password and the +command travel in cleartext on the wire. This phase gives the wire what the TODO asks +for, in the form NUT 2.8.5 can express: the collector **generates and manages a root +CA**, issues a server certificate for the exporter (installed on it by the operator) +and a client certificate for itself; the `NutClient` upgrades the connection with +`STARTTLS`, verifies the exporter's certificate against the local CA (the collector +dials the real upsd, not a lookalike) and presents its own client certificate. +- ASSUMPTION (the load-bearing one): "just use the certs as auth rather than a + password" is implemented as **certs authenticate the transport, the account + password still gates the command** — the deployed upsd 2.8.5's command gate *is* the + `upsd.users` account (`SET`/`INSTCMD` still need it, per AGENTS.md's verified fleet + facts; NUT has no wire-level client-certificate check for commands, and its own + `HELP` verb list shows `USERNAME`/`PASSWORD` as the auth pair). A client + certificate presented to a server that does not check it is a trust slot for the + next NUT, not the gate today; the gate that exists today is made *usable only over + TLS* in the documented deployment, which is the protection the TODO is after + (password + commands off the cleartext wire, both endpoints authenticated). The + `available_commands()` limitation (the wire cannot tell what the account is + granted, AGENTS.md known gap 1) is unchanged — the risk tiers and the typed + `DANGER` stay load-bearing. + +## Dependencies + +- `05_ups_hero_battery_glyph` (complete) — the `NutClient`/`NutMonitor`/`COMMANDS` + surface this phase extends. +- `01…04` (complete) — `collectors.py`'s redacted-URL error doctrine (invariant 6) + that the TLS error messages must follow. +- `11_admin_login` (todo) — **shared file `config.py`**: 11 appends the four `DASH_*` + auth settings, this phase appends the `NUT_TLS*` block; whichever runs second + applies on top of the file state at that time, never restoring the other's lines. + `api.py` is also shared (11's gate vs this phase's none — this phase adds no route + and changes no route; the one POST stays the one POST). + +## Tasks + +1. `01_ca_and_certificate_tooling.md` — the collector's root CA: generation script, the 0600 material layout, and the upsd-side install steps verified against the deployed upsd 2.8.5 (rule 7: no guessed `upsd.conf` line). +2. `02_nutclient_starttls.md` — `STARTTLS` in the `NutClient`: stdlib `ssl`, server verified against the local CA, client certificate presented, the rest of the protocol byte-identical, and the cleartext path unchanged when TLS is off. +3. `03_config_gates_and_redaction.md` — the `NUT_TLS*` settings (off by default), the one-POST-four-gates story made explicit (the account password *is* the registration credential, unchanged), and invariant 6: no key material in anything that leaves the process. +4. `04_hermetic_tls_tests.md` — the loopback-TLS fake upsd, the `STARTTLS` framing pin (a server without the branch fails exactly as the `USER`/`PASS` regression does), wrong-CA rejection, the gate over TLS, the no-leak test, and the read-only `live` test. + +## Testing & Quality + +- Unit: `tests/test_nut.py` — `STARTTLS` framing (the fake upsd's `STARTTLS` branch), + server-cert verification (a self-signed server the local CA did not issue → + refused, with a redacted message), client-certificate presentation (the loopback + server with `CERT_REQUIRED` sees the dashboard's client cert), the + `USERNAME`/`PASSWORD` login **over TLS** (the gate still works through the + upgrade), and the cleartext path unmodified (`NUT_TLS` off → no `STARTTLS` sent, + every pre-existing test passes unmodified in intent). +- Unit: the no-leak test — a TLS failure's message, the health record it becomes and + the `event` row that carries it contain no key material and no `NUT_TLS_DIR` + contents (the URL redaction doctrine, extended to key material). +- Live (`-m live --live`, read-only per the AGENTS.md live rule): a + `STARTTLS`-capable probe of the real upsd (does the deployed 2.8.5 answer the + verb — `HELP` says it does; the probe records the answer), **no `INSTCMD`** sent + from the test (a battery test on hardware something is plugged into is not a + test). +- Coverage: **>90%** on new/modified code; the 94% gate must hold. + +## Completion Criteria + +- [ ] `scripts/nut-tls-certs.sh` (or the Python equivalent in `scripts/`) generates + the CA + both certificates on a clean machine; the material is 0600, outside + git, and the README's *NUT* section walks the operator through installing the + server certificate on the exporter (verified against the deployed upsd 2.8.5 — + the exact config in `data/logs/`, not a memory of a man page) +- [ ] with `NUT_TLS=1` and the material in place, `NutClient` completes `STARTTLS`, + verifies the server, and the `LOGIN` + a `GET` reading succeed over the + upgraded socket (the hermetic loopback; the `live` probe for the real verb + answer) +- [ ] with `NUT_TLS` unset (the default), every pre-existing `tests/test_nut.py` + behaviour is byte-identical — no `STARTTLS` on the wire, cleartext reads + unchanged +- [ ] no key material in any health record, event row, error message or `/api/config` + answer (the no-leak test + the redaction suite) +- [ ] `uv run pytest` green, `bash .agents/validate.sh` green +- [ ] `README.md`'s NUT section documents the CA, the install steps, the + password-still-gates-the-command reality, and why (the NUT 2.8.5 constraint) diff --git a/.agents/phases/todo/15_nut_tls_credentials/01_ca_and_certificate_tooling.md b/.agents/phases/todo/15_nut_tls_credentials/01_ca_and_certificate_tooling.md new file mode 100644 index 0000000..64e93c0 --- /dev/null +++ b/.agents/phases/todo/15_nut_tls_credentials/01_ca_and_certificate_tooling.md @@ -0,0 +1,77 @@ +# Task 01 — The collector's root CA: tooling and the verified upsd-side install + +**Phase:** `15_nut_tls_credentials` · **Source:** `TODO.md:6` — "the collector should generate and manage a root certificate that needs to be installed with each exporter" + +## Objective + +The certificate side of "the collector manages the root certificate": a script that +generates the local CA, the exporter's server certificate and the collector's client +certificate with sane defaults, the material layout on disk, and — per rule 7 +(verify against the deployed upstream, never guess) — the **actual** upsd 2.8.5 +server-side configuration, verified on the host and recorded, not a man-page memory. +No key material ever enters a tracked file, and the generator is re-runnable (a +rotation is a re-run, not a migration). + +## Work + +1. `scripts/nut-tls-certs.sh` (bash + `openssl`, stdlib tooling — no new dependency, + the locked-decisions doctrine): generate, under `NUT_TLS_DIR` (default + `~/.config/dashboards/nut-tls/`, mode `0700`, files `0600`): + - `ca.key`/`ca.crt` — a local root CA (RSA 4096 or Ed25519, 20 y, `CA:TRUE`); + - `upsd.key`/`upsd.crt` — the exporter's server certificate, signed by the CA + (`CN` + `subjectAltName` for the address the collector dials — + `driveripper.reeselink.com` and `10.1.x.x` per the fleet facts; the SAN list is + a script argument, defaulting to the `NUT_HOST` value); + - `dashboard.key`/`dashboard.crt` — the collector's client certificate (the one the + `NutClient` will present); + - re-runnable: existing material is verified (the certs chain to the CA) and + left alone; `--rotate` regenerates (a rotation re-issues all three — the old + server cert stops verifying, which is the intended eviction, like the + session-secret rotation in phase 11). +2. Verify the upsd-side configuration **on the deployed upsd** (from + `driveripper`, where `sudo` grants the `/etc/ups` reads per the fleet facts): + read the running `upsd`'s TLS capability — `upsd -V`, its conf's current `LISTEN` + lines, and the 2.8.5 documentation *as installed on the host* (`man upsd` / the + package's docs) — and record the exact configuration that serves `STARTTLS` on + 3051 with the server certificate, in `data/logs/NOTES-upsd-tls.md` (the exact + directive name(s), the cert/key paths upsd expects, and whether the 2.8.5 build + verifies *client* certificates at all — the phase-15 ASSUMPTION says it does not; + the note records the evidence either way). If the installed 2.8.5 **cannot** + serve `STARTTLS` (a `--without-tls` build — check `upsd -V`'s feature list), + record that and the phase's task 02/04 narrow to "the client is ready, the + server-side is the operator's upgrade call" — do **not** install a different NUT + (a fleet change is the operator's action, the AGENTS.md fleet-facts doctrine). + - ASSUMPTION: the script uses `openssl` (present on every fleet host, the + `deploy/install-agent.sh` doctrine — stdlib/tooling only, no new package) and + the default `NUT_TLS_DIR` lives in the operator's home config tree, not the + repo (invariant 6: the material is a credential, never tracked). +3. `README.md` → *NUT*: the operator walkthrough — run the script on the machine that + dials upsd, copy `upsd.crt`/`upsd.key` to the exporter, apply the task-1-recorded + upsd configuration (the `sudo` grant for the `/etc/ups` writes exists per the + fleet facts; the write itself stays the operator's action, like every other + `upsd.users` edit in this repo — the known-gap-1 doctrine: never grant `actions = + -`, never write it blind), restart `nut-server`, and confirm the collector + connects (task 04's `live` probe). + +## Testing & Quality + +- Script test (the repo's `tests/test_deploy_scripts.py` device — it already fails + the day a script regresses): the script's output layout (file names, modes + 0700/0600, the certs chain to the CA via `openssl verify`), re-run idempotency, + and `--rotate` changing the key fingerprints; no tracked file receives key material + (the test asserts the generated dir is the only place the material is). +- Coverage: **>90%** on new/modified code (script + its test; the 94% gate must + hold). + +## Completion Criteria + +- [ ] a clean run of the script produces the CA + both certificates with the pinned + layout/modes; `openssl verify -CAfile ca.crt upsd.crt` passes; a re-run is a + no-op; `--rotate` changes fingerprints +- [ ] `data/logs/NOTES-upsd-tls.md` records the deployed upsd 2.8.5's actual TLS + capability and the exact server-side configuration (or the finding that this + build cannot serve `STARTTLS` — the note, not the assumption, decides task + 02's scope) +- [ ] `tests/test_deploy_scripts.py` pin green; `bash .agents/validate.sh` green +- [ ] the README walkthrough exists and names the operator actions that stay the + operator's (the upsd config write, the restart) diff --git a/.agents/phases/todo/15_nut_tls_credentials/02_nutclient_starttls.md b/.agents/phases/todo/15_nut_tls_credentials/02_nutclient_starttls.md new file mode 100644 index 0000000..ecce84e --- /dev/null +++ b/.agents/phases/todo/15_nut_tls_credentials/02_nutclient_starttls.md @@ -0,0 +1,82 @@ +# Task 02 — `STARTTLS` in the NutClient: verified server, presented client + +**Phase:** `15_nut_tls_credentials` · **Source:** `TODO.md:6` — "just use the certs as auth rather than a password" (implemented per the phase ASSUMPTION: certs authenticate the transport; the `upsd.users` account still gates the command) + +## Objective + +The `NutClient` upgrades the connection with the verb the server's own `HELP` +advertises (`STARTTLS` — pinned in `tests/test_nut.py`'s verb-list test): after the +upgrade the socket is wrapped in stdlib `ssl` (no new dependency), the exporter's +certificate is verified against the local CA (the collector dials the real upsd, not +a lookalike), the collector's client certificate is presented (the trust slot for a +server that will check it), and **every other byte of the ~10-line protocol is +unchanged** — the `USERNAME`/`PASSWORD` login, the `LIST`/`GET` reads, the +`INSTCMD` write. When `NUT_TLS` is off (the default, and the only state the current +fleet can use — the laptop cannot reach 3051 at all, per the fleet facts), the +client behaves exactly as today: no `STARTTLS` sent, cleartext, every pre-existing +test passing unmodified in intent. + +## Work + +1. `src/dashboards/sources/nut.py` — `NutClient` (the socket half, where the + existing `connect`/`_read_line`/`login`/`instcmd` live): + - a `tls: bool` + `ca_path`/`cert_path`/`key_path` constructor path (built by + `NutMonitor` from the task-03 settings; the client itself stays testable with + explicit paths); + - on connect, when `tls`: send `STARTTLS\n`, read the OK line (the framing the + fake upsd in `tests/test_nut.py` already models for the other verbs — the same + one-line-per-command discipline), then wrap the **existing** socket with + `ssl.SSLContext(verify_mode=CERT_REQUIRED, purpose=server_auth)`: + `load_verify_locations(ca)`, `load_cert_chain(cert, key)` (the client + certificate), `server_hostname` from the dialled host — the wrap happens on the + connected socket, so `_read_line`/the writers are unchanged after the upgrade + (they already read/write `self._sock`); + - when `tls` is off: the constructor path is not even built (no `SSLContext` + object, no file reads — the cleartext path touches no TLS code at runtime); + - the `LOGIN`/`LOGOUT`/`GET`/`LIST`/`INSTCMD` sequence is unchanged in both modes + (the login happens after the upgrade — the password is the first thing that + travels over the encrypted socket, which is the point). + - The scope guard from task 01: if `data/logs/NOTES-upsd-tls.md` recorded that + the deployed 2.8.5 build **cannot** serve `STARTTLS`, this task still ships + the client (the hermetic loopback proves it; the README says the server side is + the operator's upgrade call) — the client capability is independent of one + host's build. +2. `src/dashboards/collectors.py` — `NutMonitor` takes the task-03 settings and + builds the TLS half (one place, the `from_env()` single-source doctrine `nut_sim` + already follows: the assembly and the model cannot disagree about what is + enabled). A TLS failure (refused upgrade, unverified server, expired cert) is an + `UpstreamError` like any other dial failure — the source goes `DOWN` with the + real reason (invariant 4), and the message follows the existing redacted-URL + doctrine (the dialled address redacted, and **no key material** — task 04's + no-leak test pins it). +3. `src/dashboards/sources/nut_sim.py` — no change expected (the simulator is a + labelled fake, invariant 3/its own doctrine); the `SimUps` tests keep passing — + TLS is a transport property the simulator does not model, and that is correct. + +## Testing & Quality + +- Unit: `tests/test_nut.py` — the fake upsd's `STARTTLS` branch (a scripted + loopback `ssl` server, the task-01-generated certificates in a temp dir — the + test generates its own material with the task-01 script, never a tracked + fixture): the upgrade framing (`STARTTLS` → OK → the wrapped socket answers the + same verbs), the `LOGIN` over TLS, a `GET` reading over TLS, and the + `CERT_REQUIRED` rejection: a loopback server with a certificate the local CA did + not issue → the connection is refused with an `UpstreamError` whose message + names the redacted host and **no** key material. +- Regression: `NUT_TLS` off (the default in every pre-existing test) → every + pre-existing `test_nut.py` / `test_collectors.py` / `test_api.py` behaviour + passes unmodified in intent; no `STARTTLS` string ever appears on the cleartext + wire (the fake upsd's scripted log asserts it was never sent). +- Coverage: **>90%** on this task's new/modified code; the 94% gate must hold. + +## Completion Criteria + +- [ ] the loopback-TLS fake upsd: `STARTTLS` framing, verified server, presented + client certificate (the server with `CERT_REQUIRED` sees it), login + read + over the upgraded socket — all green +- [ ] a wrong-CA server is refused with a redacted, key-free `UpstreamError`; the + source state path (task 04's poller test) shows `DOWN` with the reason, last + good payload kept (invariant 4) +- [ ] the cleartext path is byte-identical: the pre-existing suite passes and the + fake upsd records no `STARTTLS` verb in cleartext mode +- [ ] `bash .agents/validate.sh` green diff --git a/.agents/phases/todo/15_nut_tls_credentials/03_config_gates_and_redaction.md b/.agents/phases/todo/15_nut_tls_credentials/03_config_gates_and_redaction.md new file mode 100644 index 0000000..c577a03 --- /dev/null +++ b/.agents/phases/todo/15_nut_tls_credentials/03_config_gates_and_redaction.md @@ -0,0 +1,84 @@ +# Task 03 — `NUT_TLS*` settings, the registration story, and the no-leak boundary + +**Phase:** `15_nut_tls_credentials` · **Source:** `TODO.md:6` — "the exporter should be installed with password protection and that password must be used to register it with the collector" + +## Objective + +The config surface: `NUT_TLS` (off by default — the current fleet cannot reach upsd +from the laptop at all, and the deployed dashboard reads cleartext today; enabling +is the operator's action per the README walkthrough) and `NUT_TLS_DIR` (the +0600 material tree task 01 lays out). Plus the "registration" half made explicit in +code and docs: the `upsd.users` account password the operator installed on the +exporter is carried in the collector's env (the existing `NUT_USER`/`NUT_PASSWORD` +settings — the `.env.dev` already holds the credentials per known gap 1c) and is +used at `LOGIN` before every `INSTCMD`; TLS changes *how* it travels, never *that* +it is the gate. And invariant 6 extended: a key is a credential, so no key material +may appear in anything that leaves the process (health records, event rows, error +messages, `/api/config`). + +## Work + +1. `src/dashboards/config.py` — append the block (the phase-11 `DASH_*` block is the + pattern; whichever phase runs second appends on top, never restoring the + other's lines): `NUT_TLS` (the `_env()` truthy rule, same as `NUT_SIMULATE`'s + `from_env()` doctrine — one place the rule lives, the settings object carries + the parsed flag) and `NUT_TLS_DIR` (path, default `~/.config/dashboards/nut-tls`, + expanded). The `Target`/redaction half is unchanged: the TLS settings are not + URLs and carry no userinfo. + - ASSUMPTION: `NUT_TLS_DIR` is a *directory* (the script's layout: + `ca.crt`/`dashboard.crt`/`dashboard.key` inside it), not three path variables — + one knob, one layout, the rotation is a re-run of the script (task 01), and a + mistyped path is a clear `UpstreamError` ("TLS material not found in …"), not + three silent defaults. +2. `src/dashboards/sources/nut.py` — the no-leak boundary (invariant 6, the existing + doctrine's exact pattern: `strip_userinfo`/`Target.display_url` for URL tokens): + - every `UpstreamError` message built in the TLS path is checked against a + **key-free list** — the message may name the redacted dial address, the verb, + and a fixed failure kind (`tls: server certificate not signed by the local + CA`, `tls: client refused the certificate`, `tls: material not found in `) + — nothing interpolated from the key files themselves (they are opened for the + `SSLContext` and never read into a string; the one place a path may appear is + `NUT_TLS_DIR`'s *path*, which is not secret material); + - the `tests/test_auth.py` device for fixed-text raise/log sites (the structural + pin that fails the day someone interpolates a credential) is extended to the + `nut.py` TLS error-construction sites — the same structural test shape, + applied to the new code. +3. `.env.example` / `.env.dev.example` — the `NUT_TLS*` block (commented, off, the + walkthrough pointer to the README's NUT section); the existing + `NUT_USER`/`NUT_PASSWORD` comments gain the one line that names the registration + story: "this is the account the exporter was installed with — it is used at + `LOGIN` before every `INSTCMD`; with `NUT_TLS=1` it travels over the encrypted + socket, without it the wire is cleartext (the firewall, not the password, is + what keeps strangers off 3051 today — the fleet-facts doctrine: the gate is the + account + the four gates on `POST /api/ups/command`, and `NUT_ALLOW_ACTIONS` + stays the first of them)." +4. `README.md` → *NUT*: the settings table rows + the one-paragraph "how the + privileged action is protected" that states the whole story in operator words: + the account gates the command (the upsd side), the four gates gate the app side + (`NUT_ALLOW_ACTIONS`, the account actually having the command, the UI + confirmation, the on-battery refusal), TLS (when enabled) keeps the password and + the command off the cleartext wire and authenticates both ends by certificate — + and says *why the password remains the gate* (the NUT 2.8.5 constraint, the + phase ASSUMPTION, with the task-01 note as the evidence). + +## Testing & Quality + +- Unit: `tests/test_config.py` — the `NUT_TLS` truthy rule (the `NUT_SIMULATE` + test's shape), the `NUT_TLS_DIR` default/expansion, and the "unset means + cleartext" default (an app built with no TLS settings never opens a key file — + a `monkeypatch` on the file reads asserts zero opens in the default suite). +- Unit: the structural no-leak pin (the `test_auth.py` device, task-02's error + sites) — a regression that interpolates a key file's *contents* into a message + fails the test, not a log line. +- Coverage: **>90%** on new/modified code; the 94% gate must hold. + +## Completion Criteria + +- [ ] `NUT_TLS` unset (the default in every pre-existing test and in both env + templates): no key file is opened, no `STARTTLS` is sent — the pre-existing + suite is byte-identical in behaviour +- [ ] `NUT_TLS=1` + the task-01 layout: the settings resolve to the three files; + a missing file is a clear `UpstreamError` naming the directory, nothing more +- [ ] the env templates + README carry the registration story and the NUT 2.8.5 + "why the password remains the gate" paragraph +- [ ] `bash .agents/validate.sh` green diff --git a/.agents/phases/todo/15_nut_tls_credentials/04_hermetic_tls_tests.md b/.agents/phases/todo/15_nut_tls_credentials/04_hermetic_tls_tests.md new file mode 100644 index 0000000..06e61a8 --- /dev/null +++ b/.agents/phases/todo/15_nut_tls_credentials/04_hermetic_tls_tests.md @@ -0,0 +1,85 @@ +# Task 04 — The loopback-TLS suite, the no-leak pin, and the read-only live probe + +**Phase:** `15_nut_tls_credentials` · **Source:** `TODO.md:6` — "For privileged actions (UPS commands) the exporter should be installed with password protection…" (the whole phase, proven) + +## Objective + +The proof layer: a hermetic loopback TLS fake upsd (stdlib `ssl`, the task-01 +script's certificates in a temp dir — never tracked), the `STARTTLS` framing pin +that fails the day the server-side verb stops being answered (the exact device +`test_login_uses_the_verbs_upsd_2_8_5_answers_to` uses for the `USER`/`PASS` +regression), the wrong-CA rejection, the client-certificate presentation, the +`LOGIN` gate **over TLS**, the no-leak pin for key material, and the read-only +`live` probe of the deployed upsd's `STARTTLS` answer. No `INSTCMD` from any test +(a battery test on hardware something is plugged into is not a test — the +AGENTS.md live rule). + +## Work + +1. `tests/test_nut.py` — the fake upsd's TLS half (the existing fake's scripted- + reply doctrine, one socket upgrade away): + - a loopback `ssl.SSLContext` server (the task-01 `upsd.crt`/`upsd.key`, + `CERT_REQUIRED` + `load_verify_locations(dashboard.crt)` so it **demands** the + client certificate) answering the same one-line-per-command script: `STARTTLS` + → OK, then the post-upgrade verbs; + - `test_starttls_upgrade_framing` — the client sends `STARTTLS` exactly once, + before the `LOGIN`, and the post-upgrade `LIST`/`GET` answers are parsed + identically to the cleartext suite (the `UpsReading` contract, + `tests/test_ups_contract.py`, holds over TLS); + - `test_client_certificate_is_presented` — the server context's + `CERT_REQUIRED` passes only because the client presented `dashboard.crt` + (the test asserts the server saw a verified peer cert, not just a completed + handshake); + - `test_unverified_server_is_refused` — a second loopback server with a + self-signed certificate the local CA did not issue → `UpstreamError`, the + message redacted and key-free (asserted against the no-leak list), the + connection closed, no `LOGIN` sent (the password never travelled); + - `test_login_gate_over_tls` — a wrong `PASSWORD` over the upgraded socket + still answers the upsd's `ERR` exactly as the cleartext suite pins it (the + account gate is TLS-agnostic — the phase ASSUMPTION's core, proven). +2. `tests/test_collectors.py` — the poller half (invariant 4's exact doctrine, the + existing `store-error`/stale device): a `NutMonitor` whose loopback server + dies mid-session (the TLS handshake succeeds, then the socket closes) → the + source is `DOWN` with the real reason, the last good payload kept, one + transition event, no exception ends the task; a TLS-missing-material start + (task 03's `NUT_TLS_DIR` with no files) → the same `DOWN` path with the clear + reason, **without dialling** (invariant 3's shape: a source that cannot even + build its material does not pretend to be configured-and-dead — the event says + the material is missing, once, not per poll). +3. `tests/test_api.py` — the no-leak pin at the boundary that leaves the process: + with a TLS failure injected, `/api/health`'s job record, the `event` table row + and `/api/events`' answer contain none of the key material (the temp-dir + `dashboard.key`'s contents, the `ca.key`'s contents, and any PEM marker are all + absent) — the same test shape that pins the URL-token redaction, applied to + key material. +4. `tests/test_live_fleet.py` — the `live` probe (skipped unless `--live` and + `NUT_TLS`-capable material exists; **read-only**): the deployed upsd's + `STARTTLS` answer — `HELLO`-style: open a connection, send `STARTTLS`, record + the one-line answer (OK or the 2.8.5 build's refusal — both are findings for + `data/logs/NOTES-upsd-tls.md`'s "what the live server does" line, the task-01 + note's running update), **no `LOGIN` and no `INSTCMD`** (the account password + is not a test input; the known-gap-1 doctrine: a live test never writes). + +## Testing & Quality + +- Unit: per the work list — framing, presentation, refusal, the gate over TLS, + the poller's `DOWN` path, the no-leak pin. +- Live: the read-only probe, its answer recorded (the tally in the completion + note, the known transport gap acknowledged: from the laptop the probe skips on + the 3051 unreachability, per the fleet facts). +- Coverage: **>90%** on new/modified code; the 94% gate must hold. + +## Completion Criteria + +- [ ] `uv run pytest tests/test_nut.py tests/test_collectors.py tests/test_api.py` + green — the whole task-02 matrix (framing/presentation/refusal/gate) plus + the poller `DOWN` path plus the no-leak pin +- [ ] no `INSTCMD` string in any `live` test (the suite's read-only rule, greppable + and pinned by the `test_live_fleet.py` convention the existing live tests + follow) +- [ ] `uv run pytest -m live --live` run (skips recorded for the unreachable + paths); the `STARTTLS` live answer recorded in `data/logs/NOTES-upsd-tls.md` + when reachable +- [ ] `bash .agents/validate.sh` green — the phase's final pass: the whole suite + 100% (the phase-11 completion doctrine: "Complete" only when the entire + project test suite passes, including the opt-in live run) diff --git a/TODO.md b/TODO.md index 2e7ac81..4640904 100644 --- a/TODO.md +++ b/TODO.md @@ -1,6 +1 @@ # TODO - -- [ ] "Last self-test: Done and passed" still shows up as a warning in the overview and UPS section -- [ ] In the LLM engines page the "fleet" section has no spacing between it and the engine summaries -- [ ] Decode tok/s sometimes jumps unrealistically high, could be a calculation error -- [ ] For privileged actions (UPS commands) the exporter should be installed with password protection and that password must be used to register it with the collector. Effectively the exporters become protected web servers. Additionally, the collector should generate and manage a root certificate that needs to be installed with each exporter. In fact, just use the certs as auth rather than a password. \ No newline at end of file