Web dashboard step 5: settings page with toml_edit writer

Schema derived from Config::default() and the live config, help text for every
shipped key, env locks and presence-only secrets, typed saves that preserve
comments and validate through Config::load before an atomic rename, provider
add/remove, the change history, and config reload on mtime.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NHyYupFdBiR4VfoUM7NjSM

docs/plans/briefs/web-dashboard/handoff-step5.md

@@ -0,0 +1,62 @@
+# Step 5 handoff — settings
+
+## Landed
+
+- `src/web/dashboard/settings.rs`: a schema derived by walking serialized
+  default and live `Config` values, including optional/env-only fields, exact
+  field kinds and enum choices, derived environment names, source tracking,
+  README/example-backed help text, stable group order, and dotted-path anchors.
+- `/dashboard/settings`: reload-on-mtime with a visible load-error banner and
+  last-good config retention; secret presence-only rendering, environment
+  locks, typed inputs, defaults/reset controls, and weight normalization notes.
+- Typed `toml_edit` saves that preserve comments/order, collect field errors,
+  write multiline string arrays, create missing tables, validate through
+  `Config::load` in `<path>.tmp.<pid>`, preserve permissions, rename atomically,
+  swap the live config, and record one attributed `config_changes` row per key.
+- Provider add/remove flows through the same validated writer, including name
+  and kind validation, role-reference refusal, placeholder defaults, audit
+  history, and redaction of a hand-written provider `api_key` from history.
+- `/dashboard/settings/history`, newest first at 100 rows per page.
+- A step 5 CSS block and reset helper in `app.js`; `/etc/daily-epub` added to
+  the server unit's `ReadWritePaths`.
+- Sixteen settings tests covering every schema and writer case listed in §17,
+  including router-level rendering and POST behavior through `oneshot`.
+
+## Deviations and notes
+
+- Shipped providers cannot be removed, even when unreferenced. They are members
+  of `Config::default()`, so deleting their TOML table would immediately restore
+  the built-in entry and falsely report success. They can be edited or left
+  unreferenced; custom providers can be removed normally.
+- `FieldKind::Enum` owns `Vec<String>` rather than the plan sketch's static
+  slice because `llm.bulk` and `llm.editor` must include provider names derived
+  at runtime. The rendered behavior and validation table are unchanged.
+- Restart notices also cover `server.public_url`, `server.session_days`,
+  `server.login_attempts`, and `server.login_window_minutes`, in addition to
+  the plan sketch's `database_path` and `server.bind`, because those values are
+  captured when the server/session/throttle layers are built.
+- An absent key whose effective value is already the submitted default remains
+  absent. This reconciles “only changed fields” with the no-JavaScript form
+  posting every editable field; resetting an explicit non-default file value
+  still writes the default explicitly.
+- Step 6 should call `WebState::reload_if_changed` before starting each job, as
+  §4.2 requires. Step 5 supplies the shared helper; this branch's jobs module is
+  intentionally still the parallel-step stub.
+- The shared brief's sandbox list omits three pre-existing OpenAI fake-server
+  tests that bind through the same `curate::llm::tests::serve` helper as its four
+  named Anthropic tests. The step 1 handoff records those three additional
+  sandbox failures; they are not settings regressions.
+
+## Verification
+
+- `cargo fmt`: pass.
+- `cargo clippy --all-targets -- -D warnings`: pass.
+- Focused `cargo test web::dashboard::settings -- --nocapture`: **16 passed,
+  0 failed**.
+- Unfiltered `cargo test`: **379 library tests passed** before the harness
+  reported the expected loopback-bind failures (the 10 named library tests plus
+  the three OpenAI tests noted above); no settings test failed.
+- `cargo test` with all pre-existing loopback listener tests excluded:
+  **411 passed, 0 failed, 15 filtered out** (379 library, 7 binary-unit, and 25
+  integration tests passed; 13 library listener tests and both `m7_server`
+  tests were filtered).

docs/reviews/2026-09-03-web-dashboard-implementation-review-2026-09-03.md

@@ -0,0 +1,92 @@
+# Web dashboard step 5 implementation review
+
+The final step 5 working tree implements the settings schema, editor, provider
+operations, history page, reload-on-mtime behavior, and systemd write access
+described by the plan. The implementation is well covered by behavior-focused
+unit and router tests. The review found one security issue in the inherited
+work—removing a provider could have copied a hand-written `api_key` into
+`config_changes`—and fixed it with redaction plus a regression assertion.
+
+## Critical
+
+None.
+
+## High
+
+None. The provider-history secret exposure found during review is fixed in
+`provider_literal`: `api_key` is removed before a whole-provider literal is
+stored or rendered by the history page.
+
+## Medium
+
+None.
+
+## Low
+
+### Shipped provider removal differs from the literal plan wording
+
+- Evidence: `ProviderCard::removable` and `remove_provider` refuse entries in
+  `default_providers()`, even when no `[llm]` role references them.
+- Plan difference: §13.3 only explicitly requires refusal when an `[llm]` role
+  names the provider.
+- Reason and recommendation: deleting a shipped provider's file table cannot
+  remove it from the effective config because `Config::default()` supplies it
+  again; pretending otherwise would reset it while reporting removal. Keep the
+  explicit refusal unless config loading later gains a provider tombstone or
+  replacement-map semantic.
+
+### Absent defaults are not materialized by an otherwise unchanged form
+
+- Evidence: `plan_changes` compares an absent file value through its effective
+  default before deciding whether to write.
+- Plan difference: §13.2's “explicit beats implicit” sentence can be read as
+  requiring an absent default-valued key to be written whenever posted.
+- Reason and recommendation: the same section says the form carries every
+  editable field and only changed fields are written. Materializing every
+  absent default on any no-JavaScript save would violate that behavior. Keep
+  the effective-value comparison; a reset from an explicit non-default value
+  still writes the default explicitly. Clarify this sentence in a future plan
+  revision if touched-vs-untouched browser state becomes a requirement.
+
+## Nits
+
+None.
+
+## Plan Coverage
+
+| Requirement | Status | Evidence |
+|---|---|---|
+| Derived schema, kinds, sources, help, group order and anchors | Implemented as planned | `schema`, `schema_with_env`, `walk`, `kind_for`, `SETTINGS_HELP` |
+| Secret redaction and environment locks | Implemented as planned | `source_for`, secret field rendering, provider-literal redaction |
+| Typed `toml_edit` save with collected errors | Implemented as planned | `plan_changes`, `apply_change` |
+| Temp-file validation, permission preservation, rename and live swap | Implemented as planned | `write_validated`, `install` |
+| One attributed history row per changed key | Implemented as planned | `record_changes`, `config_changes` |
+| Provider add/remove and referenced-provider refusal | Implemented with the shipped-provider qualification above | `add_provider`, `remove_provider` |
+| Reload on mtime with previous config retained after an error | Implemented as planned | `WebState::reload_if_changed`, settings GET banner |
+| Settings history page, newest first, 100 per page | Implemented as planned | `history`, `config_changes`, `settings_history.html` |
+| `/etc/daily-epub` writable in the server unit | Implemented as planned | `systemd/daily-epub.service` |
+
+## Testing Assessment
+
+Existing tests are meaningful: they compare the schema with serialized
+`Config::default()`, compare shipped TOML leaves with the help table, exercise
+all enum options through deserialization and `validate()`, prove secrets carry
+no value, mutate and restore a real environment variable, compare untouched
+configuration lines byte-for-byte, verify multiline arrays and new tables,
+exercise validation failure and permission preservation, inspect persisted
+history rows, add/remove/refuse providers, and drive the settings routes through
+`Router::oneshot`. The provider test also proves a file-sourced API key does not
+reach the audit table.
+
+No weak or missing test from the step 5 list remains. The only environmental
+suite limitation is the repository's pre-existing listener tests, which cannot
+bind loopback in the sandbox.
+
+## Open Questions
+
+- Should the plan eventually define a tombstone for removing shipped providers,
+  or should the current “leave built-ins unreferenced” behavior become the
+  documented contract?
+- The shared brief lists four Anthropic listener tests as sandbox-bound, while
+  the step 1 handoff also identifies three OpenAI tests using the same loopback
+  fake server. The latter fail with the identical sandbox permission error.

src/web/mod.rs

@@ -105,6 +105,42 @@ impl fmt::Debug for WebState {
     }
 }
 
+impl WebState {
+    /// Config reload on mtime (dashboard plan §4.2): when `config_path`'s
+    /// modification time differs from the cached one, re-run `Config::load`
+    /// and swap the live config. Returns `Ok(true)` when a reload happened,
+    /// `Ok(false)` when nothing changed or no file is configured, and the
+    /// load error when the file on disk no longer loads — the previous
+    /// config stays live and the cached mtime is left alone so the next call
+    /// tries again. Called by the settings page and by job starts.
+    pub fn reload_if_changed(
+        state: &crate::server::AppState,
+    ) -> Result<bool, crate::config::ConfigError> {
+        let Some(path) = state.config_path.as_deref() else {
+            return Ok(false);
+        };
+        let mtime = std::fs::metadata(path)
+            .and_then(|metadata| metadata.modified())
+            .ok();
+        let mut cached = state
+            .web
+            .config_mtime
+            .lock()
+            .unwrap_or_else(|poisoned| poisoned.into_inner());
+        if *cached == mtime {
+            return Ok(false);
+        }
+        let config = crate::config::Config::load(Some(path))?;
+        match state.config.write() {
+            Ok(mut live) => *live = std::sync::Arc::new(config),
+            Err(poisoned) => *poisoned.into_inner() = std::sync::Arc::new(config),
+        }
+        *cached = mtime;
+        tracing::info!(path = %path.display(), "reloaded configuration from disk");
+        Ok(true)
+    }
+}
+
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct Flash {
     pub kind: String,

src/web/static/app.css

@@ -57,3 +57,20 @@ thead { position:sticky; top:0; background:var(--bg); }
 .rating-prompt { margin-right:.25rem; }
 .rating-note { flex-basis:100%; }
 @media (max-width:40rem) { .masthead { font-size:1.55rem; } .kv { display:block; } }
+/* step 5: settings */
+.settings .card { border:1px solid var(--rule); padding:.75rem 1rem; margin:1rem 0; }
+.settings .card h2 { margin:.2rem 0 .6rem; font-size:1.1rem; font-family:ui-monospace,monospace; }
+.settings-form { display:block; }
+.setting { display:grid; grid-template-columns:minmax(14rem,1fr) 2fr; gap:.5rem 1rem; padding:.5rem 0; border-top:1px solid var(--rule); }
+.setting .help,.setting .default,.settings .meta { color:var(--muted); font-size:.85rem; margin:.2rem 0; }
+.setting-input input,.setting-input select,.setting-input textarea { width:100%; max-width:36rem; }
+.setting-input textarea { min-height:5rem; font-family:ui-monospace,monospace; }
+.setting-input input:disabled,.setting-input textarea:disabled { opacity:.6; }
+.setting .lines { white-space:pre-line; }
+.settings .banner { padding:.6rem .8rem; border:1px solid var(--down); margin:1rem 0; }
+.settings .reset,.settings .danger { font:inherit; font-size:.85rem; border:0; background:transparent; color:var(--accent); text-decoration:underline; padding:0; cursor:pointer; }
+.settings .actions { position:sticky; bottom:0; background:var(--bg); padding:.7rem 0; border-top:1px solid var(--rule); }
+.settings form.inline { display:none; }
+.settings .add-provider { max-width:36rem; }
+.history pre { margin:0; white-space:pre-wrap; font-size:.8rem; }
+@media (max-width:40rem) { .setting { display:block; } }

src/web/static/app.js

@@ -39,3 +39,11 @@ document.querySelectorAll("details[id]").forEach((details) => {
     details.addEventListener("toggle", () => localStorage.setItem(key, details.open ? "open" : "closed"));
   } catch (_) {}
 });
+/* step 5: settings — "reset to default" fills the field with its default */
+document.addEventListener("click", (event) => {
+  const button = event.target.closest("button[data-reset]");
+  if (!button) return;
+  const input = document.getElementById(button.dataset.reset);
+  if (!input) return;
+  input.value = button.dataset.default;
+});

src/web/templates/dashboard/settings.html

@@ -0,0 +1,41 @@
+{% extends "layout.html" %}{% block content %}<section class="dashboard settings">
+<h1>Settings</h1>
+<p class="meta">{% match config_path %}{% when Some with (path) %}Editing <code>{{ path }}</code>. Saved settings apply to the next run; a hand edit on disk shows up on the next page view.{% when None %}No config file is configured; start the server with <code>--config</code> to edit settings here. The values below are the built-in defaults plus the environment.{% endmatch %} · <a href="/dashboard/settings/history">History</a></p>
+{% match load_error %}{% when Some with (error) %}<p class="error banner">! config.toml on disk does not load: {{ error }} — the previous configuration stays live.</p>{% when None %}{% endmatch %}
+{% if !errors.is_empty() %}<ul class="error banner">{% for error in errors %}<li>{{ error }}</li>{% endfor %}</ul>{% endif %}
+<form method="post" action="/dashboard/settings" class="settings-form" id="settings-form">
+{% for group in groups %}<section class="card" id="{{ group.id }}">
+<h2>{{ group.title }}</h2>
+{% match group.provider %}{% when Some with (provider) %}<p class="meta">Key: <code>{{ crate::config::ProviderConfig::api_key_env_var(provider.name) }}</code> in the env file. {% if provider.removable() %}Not named by any <code>[llm]</code> role. <button type="submit" form="remove-{{ provider.name }}" class="danger">Remove provider</button>{% else if provider.built_in %}Built in: declared by the defaults, so it can be edited and left unreferenced but not removed.{% else %}Named by <code>llm.{{ provider.roles() }}</code>; reassign the role before removing it.{% endif %}</p>{% when None %}{% endmatch %}
+{% if group.is_weights() %}<p class="meta">Weights need not sum to 1; they are renormalized over the signals present for each article.</p>{% endif %}
+{% for field in group.fields %}<div class="setting kind-{{ field.kind_name() }}">
+<div class="setting-label"><label for="{{ field.input_id() }}"><code>{{ field.key }}</code></label>{% if field.restart_required %} <span class="badge">restart</span>{% endif %}{% match field.help %}{% when Some with (help) %}<p class="help">{{ help }}</p>{% when None %}{% endmatch %}</div>
+<div class="setting-input">
+{% if field.is_secret() %}<p class="secret">{% if field.is_set() %}set{% if field.is_env() %} (from <code>{{ field.env_var() }}</code>){% else if field.is_file() %} (in the config file — move it to the env file){% endif %}{% else %}not set — set <code>{{ field.env_var() }}</code> in the env file{% endif %}</p>
+{% else if field.is_env() %}{% if field.is_text_list() %}<textarea id="{{ field.input_id() }}" disabled>{{ field.current }}</textarea>{% else %}<input id="{{ field.input_id() }}" type="text" value="{{ field.current }}" disabled>{% endif %}<p class="help">locked by <code>{{ field.env_var() }}</code> in the env file</p>
+{% else if field.is_bool() %}<select id="{{ field.input_id() }}" name="{{ field.path }}"><option value="true"{% if field.current == "true" %} selected{% endif %}>true</option><option value="false"{% if field.current == "false" %} selected{% endif %}>false</option></select>
+{% else if field.is_enum() %}<select id="{{ field.input_id() }}" name="{{ field.path }}">{% for option in field.options() %}<option value="{{ option }}"{% if field.current == option.as_str() %} selected{% endif %}>{% if option.is_empty() %}(none){% else %}{{ option }}{% endif %}</option>{% endfor %}</select>
+{% else if field.is_text_list() %}<textarea id="{{ field.input_id() }}" name="{{ field.path }}" rows="4" spellcheck="false">{{ field.current }}</textarea>
+{% else if field.is_number() %}<input id="{{ field.input_id() }}" name="{{ field.path }}" type="number" step="{% if field.is_integer() %}1{% else %}any{% endif %}" value="{{ field.current }}">
+{% else %}<input id="{{ field.input_id() }}" name="{{ field.path }}" type="text" value="{{ field.current }}" spellcheck="false">
+{% endif %}
+{% if !field.is_secret() && !field.is_env() %}<p class="default">default: {% if field.default.is_empty() %}<em>unset</em>{% else if field.is_text_list() %}<code class="lines">{{ field.default }}</code>{% else %}<code>{{ field.default }}</code>{% endif %}{% if field.current != field.default %} <button type="button" class="reset" data-reset="{{ field.input_id() }}" data-default="{{ field.default }}">reset to default</button>{% endif %}</p>{% endif %}
+</div>
+</div>
+{% endfor %}
+</section>
+{% endfor %}
+<div class="actions">{% match config_path %}{% when Some with (_path) %}<button type="submit">Save settings</button>{% when None %}<button type="submit" disabled>Save settings</button>{% endmatch %}</div>
+</form>
+{% for group in groups %}{% match group.provider %}{% when Some with (provider) %}{% if provider.removable() %}<form method="post" action="/dashboard/settings/providers" id="remove-{{ provider.name }}" class="inline" data-confirm="Remove [providers.{{ provider.name }}] from config.toml?"><input type="hidden" name="action" value="remove"><input type="hidden" name="name" value="{{ provider.name }}"></form>{% endif %}{% when None %}{% endmatch %}{% endfor %}
+<section class="card" id="add-provider">
+<h2>Add provider</h2>
+<form method="post" action="/dashboard/settings/providers" class="add-provider">
+<input type="hidden" name="action" value="add">
+<label>Name <input name="name" type="text" pattern="[a-z0-9_]+" placeholder="mistral" required{% if config_path.is_none() %} disabled{% endif %}></label>
+<label>Kind <select name="kind">{% for kind in provider_kinds %}<option value="{{ kind }}">{{ kind }}</option>{% endfor %}</select></label>
+<p class="help">Inserts <code>[providers.&lt;name&gt;]</code> with placeholder <code>base_url</code> and <code>model</code>; edit them above afterwards and put <code>DAILY_EPUB_PROVIDERS__&lt;NAME&gt;__API_KEY</code> in the env file.</p>
+<button type="submit"{% if config_path.is_none() %} disabled{% endif %}>Add provider</button>
+</form>
+</section>
+</section>{% endblock %}

src/web/templates/dashboard/settings_history.html

@@ -0,0 +1,18 @@
+{% extends "layout.html" %}{% block content %}<section class="dashboard">
+<h1>Settings history</h1>
+<p class="meta"><a href="/dashboard/settings">Settings</a> · every change made through the dashboard, newest first.</p>
+{% if changes.is_empty() %}<p>No settings have been changed through the dashboard yet.</p>{% else %}<div class="scroll-x"><table class="history">
+<thead><tr><th>When</th><th>Who</th><th>Key</th><th>Before</th><th>After</th></tr></thead>
+<tbody>{% for change in changes %}<tr>
+<td>{{ change.changed_at }}</td>
+<td>{% match change.username %}{% when Some with (name) %}{{ name }}{% when None %}<em>removed user</em>{% endmatch %}</td>
+<td><code>{{ change.key }}</code></td>
+<td>{% match change.old_value %}{% when Some with (value) %}<pre>{{ value }}</pre>{% when None %}<em>absent</em>{% endmatch %}</td>
+<td>{% match change.new_value %}{% when Some with (value) %}<pre>{{ value }}</pre>{% when None %}<em>removed</em>{% endmatch %}</td>
+</tr>{% endfor %}</tbody>
+</table></div>
+{% include "_pagination.html" %}
+{% if pagination.page < pagination.pages() %}<p><a href="/dashboard/settings/history?page={{ pagination.page + 1 }}">Older</a></p>{% endif %}
+{% if pagination.page > 1 %}<p><a href="/dashboard/settings/history?page={{ pagination.page - 1 }}">Newer</a></p>{% endif %}
+{% endif %}
+</section>{% endblock %}

systemd/daily-epub.service

@@ -36,9 +36,10 @@ TimeoutStopSec=20s
 StateDirectory=daily-epub
 StateDirectoryMode=0750
 WorkingDirectory=/var/lib/daily-epub
-# The publish dirs from [publish] in config.toml — keep these in sync.
+# The publish dirs from [publish] in config.toml — keep these in sync — plus
+# /etc/daily-epub so the dashboard settings page can rewrite config.toml.
 # Every path listed here must exist at start, or the unit fails with 226/NAMESPACE.
-ReadWritePaths=/home/thallada/bookorbit/books/daily-epub /var/lib/daily-epub/xtc
+ReadWritePaths=/home/thallada/bookorbit/books/daily-epub /var/lib/daily-epub/xtc /etc/daily-epub
 
 # --- hardening (spec §3.15) ---------------------------------------------
 ProtectSystem=strict