thalladaandClaude Fable 5.1
aa3de51d9c
Job catalogue and daily-epub job run, the systemd job unit and polkit rule, SystemdRunner/MockRunner, the Jobs pages with status and journal tail, the stats_data refactor with a byte-identical CLI, the stats page and the overview sparklines. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NHyYupFdBiR4VfoUM7NjSM
65 lines
4.0 KiB
Markdown
65 lines
4.0 KiB
Markdown
# 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.
|