Merge branch 'step6' into web-dashboard

# Conflicts: # src/web/static/app.css
2026-09-03 18:01:36 +00:00
parent d404946c78 aa3de51d9c
commit f6e317a9fe
21 changed files with 2962 additions and 112 deletions
@@ -0,0 +1,73 @@
+# Step 6 handoff — jobs and stats
+
+## Landed
+
+- Added `src/jobs.rs`: the fixed, regex-constrained job catalogue; jobs-table
+  query/lifecycle helpers; single-line terminal messages; `SystemdRunner` with
+  ten-second command timeouts, captured stderr and systemd status parsing.
+- Added `daily-epub job run <name>`. It uses the existing command lock policy,
+  claims or creates the requested row, calls the same in-process helpers as the
+  ordinary CLI commands, records `ok`/`failed`, stores the generated run id,
+  and propagates failures to a non-zero process exit. Existing profile,
+  features and social CLI output retains its prior line content.
+- Finalized `JobRunner`: production `serve` installs `SystemdRunner` when
+  `server.jobs_enabled` is true; disabled servers use `DisabledRunner`;
+  `AppState::new` keeps its existing test behavior; `MockRunner` records calls
+  and scripts starts, statuses and logs.
+- Added the admin Jobs list/start/detail routes and templates: catalogue cards,
+  dated generation, job history, duplicate conflict handling, start-failure
+  persistence, live unit fields, configured journal tail, 30-second
+  pre-claim failure detection, disabled-state messaging and five-second active
+  refresh.
+- Added the systemd job template, the §14.3 polkit rule verbatim, and
+  `SupplementaryGroups=systemd-journal` on the server unit.
+- Split stats into `stats_data` plus `render_stats_text` while pinning the
+  complete previous CLI output. Added the 14/30/90-day stats page, aggregate
+  tables, retriever yield, per-run table, daily provider costs and the three
+  requested SVG charts.
+- Replaced the overview placeholder with cost-per-run, selected-per-run and
+  generation-time sparklines over the last 30 finished non-dry runs. SVGs use
+  presentation attributes and CSS classes only; no inline styles or template
+  `safe` filters were added.
+- Added/expanded tests for the catalogue, job lifecycle and run link, CLI lock
+  mapping, in-process job success/failure, polkit/unit files, runner status
+  parsing, every jobs route behavior, exact stats text/data, stats routes and
+  both sparkline forms.
+
+## Deviations and integration notes
+
+- Per the parallel-step constraint, job start contains exactly the requested
+  `// TODO(step 5 merge): reload_if_changed` marker and does not duplicate step
+  5's helper. The step-5 merge must replace that marker with its helper call.
+- The offline in-process job test uses `features-prune`; generate's `run_id`
+  persistence is tested separately through the same lifecycle helper because a
+  full generate would require Miniflux/network fixtures. Production generate
+  and dry-run mappings both store `GenerateOutcome.run_id`.
+- The server unit intentionally does not add `/etc/daily-epub` to
+  `ReadWritePaths` here; step 5 owns that additive unit change.
+- `StatsData.durations` retains finished dry runs because that is what the old
+  CLI calculation included and the text must remain byte-identical. Dashboard
+  per-run series and overview sparklines exclude dry runs as specified.
+
+## Left for later
+
+- Step-5 merge: call its `reload_if_changed` implementation at the marked job
+  start site and combine its `/etc/daily-epub` unit path change.
+- Step 7 owns user-facing documentation and deployment/runbook polish.
+
+## Verification
+
+- `cargo fmt`: pass.
+- `cargo clippy --all-targets -- -D warnings`: pass.
+- Raw `cargo test`: **406 passed, 13 failed** in the library before Cargo
+  stopped; every failure was a sandbox-denied loopback listener (four named
+  Anthropic tests, three pre-existing OpenAI fake-server tests documented in
+  the step-1 handoff, the named extractor test and five named server tests).
+- `cargo test` with those listener tests and both `tests/m7_server.rs` listener
+  tests skipped: **440 passed, 0 failed, 15 filtered out** across all targets.
+- Focused `cargo test jobs::`, `cargo test job_run`, and `cargo test stats`:
+  pass.
+- `node --check` on the polkit JavaScript (via stdin): pass.
+- `systemd-analyze verify systemd/daily-epub-job@.service`: no diagnostics for
+  the job unit.
+
@@ -0,0 +1,64 @@
+# Web dashboard step 6 implementation review
+
+The finished step-6 implementation matches the jobs and stats brief. The fixed
+job catalogue, CLI lifecycle, systemd runner and files, dashboard routes,
+stats-data refactor, SVG sparklines, production runner wiring, and required
+tests are all present. The review found and corrected two faulty test
+assertions, restored the polkit file to the brief's verbatim rule, made job
+messages reliably one-line, and strengthened the duplicate-start test to use a
+genuinely running row. No outstanding correctness finding remains.
+
+## Critical
+
+None.
+
+## High
+
+None.
+
+## Medium
+
+None.
+
+## Low
+
+None.
+
+## Nits
+
+None.
+
+## Plan Coverage
+
+| Requirement | Status | Evidence |
+|---|---|---|
+| Fixed job catalogue and validated names | Implemented as planned | `src/jobs.rs`: `Job`, `parse`, `name`, `unit`, `description`, `takes_lock`, `dangerous` and traversal/uppercase/unknown-name tests. |
+| `daily-epub job run <name>` lifecycle | Implemented as planned | `src/main.rs`: CLI dispatch, existing lock path, in-process command mapping, requested-row claim, terminal update, failure propagation and tests. `src/jobs.rs` owns row lifecycle helpers. |
+| Unit template, polkit rule and journal group | Implemented as planned | `systemd/daily-epub-job@.service`, verbatim `systemd/50-daily-epub.rules`, and `SupplementaryGroups=systemd-journal` in `systemd/daily-epub.service`; repository-content tests cover their security-sensitive strings. |
+| Production/test/disabled runners | Implemented as planned | `src/jobs.rs::SystemdRunner` uses bounded `tokio::process::Command`; `src/web/mod.rs` contains scripted `MockRunner` and `DisabledRunner`; `src/server.rs::serve` selects the production runner only when jobs are enabled while `AppState::new` remains test-compatible. |
+| Jobs list/start/detail pages | Implemented as planned | `src/web/dashboard/jobs.rs` and the two jobs templates implement catalogue cards, date selection, history, duplicate refusal, failed-start persistence, live status, journal tail, the 30-second failure rule, authorization tests and five-second refresh. |
+| Config reload integration boundary | Partially implemented by design | The required `// TODO(step 5 merge): reload_if_changed` is at the start site. Step 5 owns the helper and merge-time call per the brief's parallel-work constraint. |
+| Stats data/text split | Implemented as planned | `src/curate/telemetry.rs` exposes `StatsData`, `stats_data`, and `render_stats_text`; seeded fixtures pin the complete legacy output byte-for-byte. |
+| Stats page | Implemented as planned | `src/web/dashboard/stats.rs` and `dashboard/stats.html` provide the three allowed windows, tables, retriever yield, per-run data and the three requested charts. |
+| Overview sparklines | Implemented as planned | `src/web/dashboard/mod.rs` loads the last 30 finished non-dry runs and renders cost, selected count and duration through `_sparkline.html`. |
+| CSP-safe rendering | Implemented as planned | SVG geometry uses presentation attributes and stylesheet classes; changed templates contain no inline `style` attributes and no `safe` filter. |
+
+## Testing Assessment
+
+Meaningful tests cover every catalogue form and rejection case, job-row claim
+and completion (including run-id persistence), real router authorization and
+POST behavior with `MockRunner`, requested/running duplicate refusal, failed
+starts, the unit-exited grace rule, disabled jobs, systemd output parsing,
+polkit/unit contents, in-process job success and failure, exact
+legacy stats text, stats aggregation, SVG geometry, page output, and overview
+series.
+
+The tests deliberately do not execute systemd or the network. The mapped
+generate branch is therefore covered by its option construction and the
+separate job run-id lifecycle test rather than a live pipeline invocation;
+this follows the brief's offline-test rule. No additional step-6 test is needed.
+
+## Open Questions
+
+- During the step-5 merge, replace the required job-start TODO with the single
+  `reload_if_changed` call supplied by step 5; do not duplicate that helper.