Target check timeout is 80% of the refresh interval (closes #78)

Each target check now times out after 80% of the refresh interval, 24
seconds at 30 seconds, where it was capped at 3 seconds, so slow, far
targets are recorded with their real time. A round asked for while the
last one's checks are still waiting, after an interval change or by the
recovery probe, is skipped, so rounds never overlap; the recovery probe
starts no new checks while its last ones wait.

The frontend has its first unit tests, run by script/frontend-test with
Node's built-in test runner. index.html now links src/styles.css, which
src/main.js imported, since Node cannot import CSS.

Model: opus-5-5

README.md

@@ -52,8 +52,8 @@ halves, so the root `make check` fails if either one is broken. We provide:
 - `script/fmt` — format all files (writes): prettier, then gofmt over `backend/`
 - `script/fmt-check` — check formatting (read-only): prettier, then gofmt
 - `script/check` — run test, lint, and fmt-check
-- `script/frontend-test` — run the production build as the frontend's test (no
-  unit tests yet)
+- `script/frontend-test` — run the unit tests in `test/unit/` with Node's
+  built-in test runner, then the production build
 - `script/frontend-lint` — run prettier in check mode
 - `script/frontend-fmt` — format everything prettier understands (writes)
 - `script/frontend-fmt-check` — check prettier formatting (read-only)
@@ -136,8 +136,11 @@ Local hosts are tracked separately from WAN stats.
 ### Latency measurement
 
 HEAD requests with `mode: 'no-cors'` and `cache: 'no-store'`, timed with
-`performance.now()`. 1-second timeout; anything over 1000ms is clamped to
-unreachable. IPv4 only.
+`performance.now()`. Each check times out after 80% of the refresh interval (24
+seconds at 30 seconds) and is then recorded as a timeout, so a round's checks
+have all finished before the next round is due. A round due while the last one
+is still waiting, which happens only after an interval change or when the
+recovery probe starts one, is skipped. IPv4 only.
 
 ### Color coding
 
@@ -216,10 +219,8 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for netwatch needs:
     - `CORS_ALLOWED_ORIGINS`, default empty: other origins whose pages may call
       the API
     - `DEBUG`, default `false`: debug logging
-    - `DATA_DIR`, default `/data/reports`: the directory the reports are kept
-      in: `/data` or a path below it, with no `.` or `..` part and no extra `/`.
-      The container also stops if a part of the path that exists, `/data`
-      included, is a symbolic link
+    - `DATA_DIR`, default `/data/reports`: leave unset; reports kept outside
+      `/data` do not survive a redeploy
     - `TRUSTED_PROXIES`, default empty: set it to the address the reverse proxy
       in front of the container connects from, as an IP address or CIDR; several
       are separated by commas. nginx takes the client address from

TODO.md

@@ -23,14 +23,13 @@ latest run passes.
 
 # Completed Steps
 
-- 2026-09-29: `bin/entrypoint.sh` checks `DATA_DIR` in full before it acts on it
-  as root (issue #80): `DATA_DIR` must be `/data` or a path below it with no
-  `.`, `..` or empty part, and no part of it that exists, `/data` included, may
-  be a symbolic link; anything else stops the start with a message naming
-  `DATA_DIR`. Only then is `DATA_DIR` created and `/data` given to `netwatch`,
-  so a refused start no longer creates directories outside `/data`, and
-  `DATA_DIR=/etc` no longer gives `/etc` to `netwatch`. The `README.md` section
-  "Running under upaas" says which values are accepted
+- 2026-09-29: each target check times out after 80% of the refresh interval
+  (issue #78), 24 seconds at 30 seconds, where it was capped at 3 seconds. A
+  round due while the last one's checks are still waiting is skipped, so rounds
+  never overlap, and the recovery probe starts no new checks while its last ones
+  are waiting. The frontend has its first unit tests, run by
+  `script/frontend-test` with Node's built-in test runner; for them,
+  `index.html` now links `src/styles.css`, which `src/main.js` used to import
 - 2026-09-29: the container sets up its own data directory (issue #75):
   `bin/entrypoint.sh`, still as root, creates `DATA_DIR` if missing and gives it
   and `/data` to the `netwatch` user with mode 750 before starting the backend

bin/entrypoint.sh

@@ -63,43 +63,25 @@ done > /etc/nginx/trusted-proxies.conf
 
 # netwatch-server keeps its report files in DATA_DIR, on the /data
 # volume, which may be a host directory owned by root or by another
-# uid. /data and everything in it are given to the netwatch user here,
-# and /data and DATA_DIR get the mode the server gives a directory it
-# creates, so the host directory needs no preparing.
+# uid. Both are given to the netwatch user here, with the mode the
+# server gives a directory it creates, so the host directory needs no
+# preparing.
 #
-# This runs as root, so nothing is created or changed until DATA_DIR is
-# known to be /data or a path below it, with no '.', '..' or empty
-# part, and no part of it that exists, /data included, is a symbolic
-# link: the netwatch user can put one in /data, and root would follow
-# it anywhere in the container. Nothing else runs in the container yet,
-# so no link can appear after the check.
+# chown and chmod, run as root, change whatever a symbolic link on the
+# path points to, anywhere in the container, and the netwatch user can
+# put one in /data. So the start stops unless readlink -f, which
+# follows every link on a path, gives /data and DATA_DIR back as they
+# are. It also writes a path in full, so a DATA_DIR with '.', '..' or
+# an extra '/' in it is refused too.
 export DATA_DIR="${DATA_DIR:-/data/reports}"
-data_dir_ok() {
-    # With a / added at the end, a last part of '.' or '..', and a / at
-    # the end, match these patterns too.
-    case "$DATA_DIR/" in
-        */./* | */../* | *//*) return 1 ;;
-        /data/*) ;;
-        *) return 1 ;;
-    esac
-    # Each part from DATA_DIR up to /data. [ -L ] is false for a part
-    # that does not exist.
-    dir="$DATA_DIR"
-    while [ "$dir" != /data ]; do
-        [ -L "$dir" ] && return 1
-        dir="${dir%/*}"
-    done
-    [ ! -L /data ]
-}
-if ! data_dir_ok; then
-    echo "entrypoint: DATA_DIR must be /data or a path below it, with no" \
-        "'.', '..', extra '/' or symbolic link on it, not '$DATA_DIR'" >&2
+mkdir -p "$DATA_DIR" || exit 1
+if [ "$(readlink -f /data)" != /data ] ||
+    [ "$(readlink -f "$DATA_DIR")" != "$DATA_DIR" ]; then
+    echo "entrypoint: DATA_DIR must be a full path with no '.', '..'," \
+        "extra '/' or symbolic link on it or on /data, not '$DATA_DIR'" >&2
     exit 1
 fi
-mkdir -p "$DATA_DIR" || exit 1
-# -h: a symbolic link in /data is itself given to netwatch, not what it
-# points to.
-chown -R -h netwatch:netwatch /data || exit 1
+chown -R netwatch:netwatch /data "$DATA_DIR" || exit 1
 chmod 750 /data "$DATA_DIR" || exit 1
 
 # A stop signal is only noted here; the loop below acts on it.

index.html

@@ -9,6 +9,9 @@
             type="image/svg+xml"
             href="data:image/svg+xml,<svg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 100 100'><text y='.9em' font-size='90'>📡</text></svg>"
         />
+        <!-- Linked here, not imported by src/main.js, so the unit tests can
+             import that module in Node, which cannot import CSS. -->
+        <link rel="stylesheet" href="/src/styles.css" />
     </head>
     <body class="bg-gray-900 text-white min-h-screen">
         <div id="app"></div>

script/frontend-test

@@ -1,13 +1,14 @@
 #!/bin/sh
-# script/frontend-test: run the frontend test suite. The frontend has no
-# unit tests; the production build serves as the test (fails on broken
-# code).
+# script/frontend-test: run the frontend test suite: the unit tests in
+# test/unit/ with Node's built-in test runner, then the production
+# build, which fails on broken code.
 set -eu
 
 ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
 
 main() {
     cd "$ROOT"
+    timeout 30 node --test test/unit/*.test.js
     timeout 30 yarn build
 }
 

src/main.js

@@ -1,14 +1,14 @@
-import "./styles.css";
-
 // --- Configuration -----------------------------------------------------------
 
-// Timing, axis labels, and display constants. Latency above maxLatency is
-// clamped to "unreachable". The sparkline Y-axis is capped at
+// Timing, axis labels, and display constants. A target check times out
+// after requestTimeout, 80% of updateInterval, so a round's checks have
+// all finished before the next round is due; latency above maxLatency is
+// recorded as a timeout. The sparkline Y-axis is capped at
 // graphMaxLatency — values above it pin to the top of the chart but still
 // display their real value in the latency figure. The history buffer holds
 // maxHistoryPoints samples (historyDuration / updateInterval).
 // reportInterval is how often collected samples are POSTed to the backend.
-const CONFIG = {
+export const CONFIG = {
     updateInterval: 3000,
     maxHistoryPoints: 100,
     reportInterval: 60000,
@@ -16,7 +16,7 @@ const CONFIG = {
         return (this.maxHistoryPoints * this.updateInterval) / 1000;
     },
     get requestTimeout() {
-        return Math.min(this.updateInterval - 100, 3000);
+        return this.updateInterval * 0.8;
     },
     get maxLatency() {
         return this.requestTimeout;
@@ -503,7 +503,7 @@ class Reporter {
 
 // --- Latency Measurement -----------------------------------------------------
 
-async function measureLatency(url) {
+export async function measureLatency(url) {
     const controller = new AbortController();
     const timeoutId = setTimeout(
         () => controller.abort(),
@@ -1181,11 +1181,16 @@ function startRecoveryProbe(state, triggerTick) {
     log.notice(
         `Recovery probe started (${canaries.map((h) => h.name).join(", ")})`,
     );
+    // A check can wait up to CONFIG.requestTimeout, far longer than
+    // 500ms, so no new checks start while the last ones are waiting.
+    let checking = false;
     state._recoveryProbeId = setInterval(async () => {
-        if (state.paused) return;
+        if (state.paused || checking) return;
+        checking = true;
         const results = await Promise.all(
             canaries.map((h) => measureLatency(h.url)),
         );
+        checking = false;
         if (results.some((r) => r.error === null)) {
             log.notice("Recovery probe: connectivity detected");
             stopRecoveryProbe(state);
@@ -1374,8 +1379,18 @@ async function init() {
     updateClocks();
     setInterval(updateClocks, 1000);
 
-    function doTick() {
-        tick(state, () => startRecoveryProbe(state, doTick));
+    // A round waits up to CONFIG.requestTimeout for its checks. A round
+    // asked for while one is still waiting, after an interval change or
+    // by the recovery probe, is skipped, so rounds never overlap.
+    let roundRunning = false;
+    async function doTick() {
+        if (roundRunning) return;
+        roundRunning = true;
+        try {
+            await tick(state, () => startRecoveryProbe(state, doTick));
+        } finally {
+            roundRunning = false;
+        }
     }
 
     doTick();
@@ -1444,7 +1459,7 @@ async function init() {
 
 // Bootstrap only when loaded as the page: a real DOM containing the #app
 // mount point this module renders into. Importing the module in a unit test
-// (which has no #app) runs nothing, so buildReport can be tested in isolation.
+// (which has no #app) runs nothing, so its exports can be tested in isolation.
 if (typeof document !== "undefined" && document.getElementById("app")) {
     if (document.readyState === "loading") {
         document.addEventListener("DOMContentLoaded", init);

test/unit/main.test.js

@@ -0,0 +1,52 @@
+// Unit tests for src/main.js, run by script/frontend-test with Node's
+// built-in test runner. Importing the module does not start the page.
+
+import { after, before, test } from "node:test";
+import assert from "node:assert/strict";
+import { createServer } from "node:http";
+import { CONFIG, measureLatency } from "../../src/main.js";
+
+// measureLatency writes timeouts to the debug log, which looks for its
+// panel in the page. There is no page here.
+globalThis.document = { getElementById: () => null };
+
+// A target that answers after the number of milliseconds in the path,
+// e.g. /600.
+let server;
+let target;
+
+before(async () => {
+    server = createServer((req, res) => {
+        const delay = Number(new URL(req.url, "http://x").pathname.slice(1));
+        setTimeout(() => res.end(), delay);
+    });
+    await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve));
+    target = `http://127.0.0.1:${server.address().port}`;
+});
+
+after(() => {
+    server.closeAllConnections();
+    server.close();
+});
+
+test("the timeout is 80% of the refresh interval", () => {
+    CONFIG.updateInterval = 30000;
+    assert.equal(CONFIG.requestTimeout, 24000);
+    CONFIG.updateInterval = 3000;
+    assert.equal(CONFIG.requestTimeout, 2400);
+});
+
+test("an answer within the timeout is recorded with its real time, a later one as a timeout", async () => {
+    // 600ms is past the 400ms timeout of a 500ms interval...
+    CONFIG.updateInterval = 500;
+    assert.deepEqual(await measureLatency(`${target}/600`), {
+        latency: null,
+        error: "timeout",
+    });
+
+    // ...and within the 1200ms timeout of a 1500ms interval.
+    CONFIG.updateInterval = 1500;
+    const { latency, error } = await measureLatency(`${target}/600`);
+    assert.equal(error, null);
+    assert.ok(latency >= 600 && latency < 1200, `latency ${latency}ms`);
+});