web: a manual blocklist refresh publishes once, since refreshAll already ends in its own reload

CHANGELOG.md

@@ -4,6 +4,12 @@ All notable changes to nxdns are recorded here. The format follows [Keep a Chang
 
 Sections are written by hand. Nothing here is generated from commit messages: the point of the file is to say what changed for an operator, which a commit subject rarely does.
 
+## [Unreleased]
+
+### Fixed
+
+- **Updating the blocklists by hand builds them once.** "Update now" in the web interface, or `POST /api/blocklists/update`, rebuilt and published the blocklists a second time right after the refresh had already done so, which cost a second full build and advanced the snapshot generation twice. The scheduled refresh was not affected.
+
 ## [0.0.25] - 2026-10-22
 
 ### Fixed

specs/milestone-8.md

@@ -25,7 +25,7 @@ Ground truth: PLAN.md:610-612 (phase text), :537-550 (endpoints), §3.11/§12.1/
    > contract in this ruling is unchanged.
 10. **Repo layer**: every list row the API serves carries its row id; each mutated resource gains `getX(db, id)`, `updateX(db, id, item)`, `deleteX(db, id)` (strict: 0 rows touched → error.NotFound), written in the house repo idiom with prepared statements. Existing import-path functions stay frozen.
 11. **`GET /api/queries`**: keyset pagination `?limit` (default 100, max 1000) + `?before=<row id>`, ordered id DESC; filters `domain=` (substring), `client=` (exact), `blocked=` (bool), `since=`/`until=` (unix seconds). Response `{"queries":[...], "next_before": <id>|null}`. Row fields: id, ts, domain, client_ip, qtype, blocked, block_reason, response_time_us, cache_hit, upstream.
-12. **Live vs restart**: mutations to rules, blocklists, groups, group-sources, clients and client-prefixes call `Manager.reload(io)` and take effect live (the handler's next `acquire` sees the new snapshot). Local records and forward zones ALSO apply live: a new `src/server/local_tables.zig` RCU holder (acquire/release + swap, mirroring the manager's pattern at small scale) owns `{records, zones}`; `Handler` reads through it instead of `*const` fields; the local-records/forward-zones handlers rebuild and swap. Upstream mutations and everything in `/api/settings` are restart-required. `POST /api/blocklists/update` = `Manager.refreshAll` then reload, 202 with the status snapshot.
+12. **Live vs restart**: mutations to rules, blocklists, groups, group-sources, clients and client-prefixes call `Manager.reload(io)` and take effect live (the handler's next `acquire` sees the new snapshot). Local records and forward zones ALSO apply live: a new `src/server/local_tables.zig` RCU holder (acquire/release + swap, mirroring the manager's pattern at small scale) owns `{records, zones}`; `Handler` reads through it instead of `*const` fields; the local-records/forward-zones handlers rebuild and swap. Upstream mutations and everything in `/api/settings` are restart-required. `POST /api/blocklists/update` = `Manager.refreshAll`, which ends in its own reload, 202 with the status snapshot. Amended 2026-10-23: this ruling first said "`Manager.refreshAll` then reload", but milestone 5 already made `refreshAll` end in one reload, so every manual refresh built and published the same snapshot twice.
 13. **Stats grammar**: `period=1h|24h|7d|30d` (else 400). `/api/stats` returns totals {queries, blocked, cached, clients (distinct), avg_response_time_us} for the period. `/api/stats/timeseries` buckets: 1h→60×1m, 24h→48×30m, 7d→168×1h, 30d→120×6h; UTC; each bucket {ts, queries, blocked, cached}. SQL aggregates over query_log on the web task's own read connection.
 14. **`GET /api/lookup?domain=&group_id=`** (group optional, default group when absent): normalizes, then reports the full pipeline view: `{domain, group_id, local_records: bool, forward_zone: <zone>|null, blocked, reason, matched, source_url|null, safe_search_rewrite: <target>|null}` via `snapshot.evaluate` + records/zones lookups. Null snapshot → 503 `{"error":"no snapshot loaded"}`.
 15. **Pause API**: `GET /api/pause` → `{"paused": bool, "until": <unix s>|null}` (null while unpaused OR indefinite — disambiguated by `paused`). `POST /api/pause` body `{"paused": true, "duration_seconds": <u32, optional>}` or `{"paused": false}`.
@@ -178,7 +178,7 @@ Every handler is split into a pure core plus a thin `handle`, so the decisions a
 Owns: `src/web/handlers/` `groups.zig`, `blocklists.zig`, `rules.zig`, `local.zig`, `clients.zig`, `settings.zig`, `pause.zig`, `auth.zig` (login/logout), plus `src/server/local_tables.zig` (new, ruling 12) and the `src/server/handler.zig` edit to read records/zones through it (second mechanical handler change; coordinate: W5 already edits handler.zig — W7 runs AFTER W5 lands; sequence inside wave 2).
 
 - local_tables.zig: `LocalTables { lock: std.Io.RwLock, records, zones }` with `acquire/release` (shared) and `swap(io, new_records, new_zones)` (exclusive; frees the old under the lock after swap — no reader can hold across queries since acquire/release brackets each query). Handler: acquire in handle, release on every exit (same single defer discipline as the snapshot).
-- Handlers: rulings 9, 12, 15, 16, 17-18 behaviors; every mutation validates (shared validators — reuse validate.zig section rules by constructing the candidate row set; where validate.zig only does whole-config, extract the per-section check it already has — validate.zig is NOT in your ownership: if extraction is needed, report it and use a local check this milestone), writes via W2 repos on state's config connection, then reload/rebuild per ruling 12. blocklists/update → refreshAll + reload, 202 + status. settings PUT: merge, validate whole config, write keys, clear sessions on hash change.
+- Handlers: rulings 9, 12, 15, 16, 17-18 behaviors; every mutation validates (shared validators — reuse validate.zig section rules by constructing the candidate row set; where validate.zig only does whole-config, extract the per-section check it already has — validate.zig is NOT in your ownership: if extraction is needed, report it and use a local check this milestone), writes via W2 repos on state's config connection, then reload/rebuild per ruling 12. blocklists/update → refreshAll (it reloads itself), 202 + status. settings PUT: merge, validate whole config, write keys, clear sessions on hash change.
 - Tests: per handler with in-memory dbs and a real Manager where cheap, else assert repo effects + reload-called flag via a seam (a `reload_fn` pointer on WebState set by W9; tests inject a counter).
 
 Acceptance: fmt/ast clean; temp-root green; phase7 integration still green.

src/web/handlers/blocklists.zig

@@ -4,9 +4,9 @@
 //! an operator edits, and the counters the refresh writes ride along in the
 //! read shape so the UI can show a list's size next to its url (ruling 9).
 //!
-//! `POST /api/blocklists/update` runs `Manager.refreshAll` and then the reload
+//! `POST /api/blocklists/update` runs `Manager.refreshAll`, which publishes its
-//! seam, and answers 202 with the status of every source (ruling 12). The
+//! own snapshot, and answers 202 with the status of every source (ruling 12).
-//! refresh downloads and compiles before the response is written: 202 is
+//! The refresh downloads and compiles before the response is written: 202 is
 //! "accepted and done as far as this connection is concerned", and the status
 //! table in the body is what tells the operator which sources actually landed.
 //!
@@ -153,11 +153,9 @@ fn pruneFiles(state: *server.WebState, io: std.Io) void {
     };
 }
 
-/// Refreshes every enabled source, then applies the result (ruling 12).
+/// Refreshes every enabled source (ruling 12). The reload seam is not called:
-///
+/// `refreshAll` ends in the manager's own reload, so a second one would build
-/// `refreshAll` already ends in the manager's own reload; the seam is called
+/// and publish the same snapshot again.
-/// too, because it is how the composition root learns that a change landed and
-/// the only reload a test can observe.
 fn applyRefresh(state: *server.WebState, io: std.Io, out: []manager_mod.SourceStatus) union(enum) {
     statuses: usize,
     fail: Failure,
@@ -176,7 +174,6 @@ fn applyRefresh(state: *server.WebState, io: std.Io, out: []manager_mod.SourceSt
             return .{ .fail = .{ .internal = error.Unexpected } };
         },
     };
-    if (mutations.reload(state, io)) |failure| return .{ .fail = failure };
     return .{ .statuses = manager.statusSnapshot(io, out) };
 }
 

src/web/web_integration_test.zig

@@ -3212,6 +3212,32 @@ test "W10 a rule mutation reloads the snapshot and the change is live" {
     try bounded(env.io(), default_budget, mutationReloads, .{ env.io(), env });
 }
 
+fn refreshPublishesOnce(io: std.Io, env: *Env) anyerror!void {
+    const before = try env.generation();
+
+    var conn: Conn = undefined;
+    try conn.connect(io, env.addr);
+    defer conn.close(io);
+
+    var body_buf: [4096]u8 = undefined;
+    try conn.request("POST", "/api/blocklists/update", null, null);
+    const response = try conn.receive(&body_buf);
+    try testing.expectEqual(@as(u16, 202), response.status);
+    try testing.expectEqualStrings("{\"sources\":[]}", response.body);
+
+    try testing.expectEqual(before + 1, try env.generation());
+}
+
+test "W10 a manual refresh publishes exactly one generation" {
+    if (!build_options.integration) return error.SkipZigTest;
+
+    const gpa = testing.allocator;
+    var env = try Env.create(gpa, .{});
+    defer env.destroy();
+
+    try bounded(env.io(), default_budget, refreshPublishesOnce, .{ env.io(), env });
+}
+
 // ---------------------------------------------------------------------------
 // deleting a source takes its compiled files with it (m13 ruling F-f)
 // ---------------------------------------------------------------------------