Merge pull request #242 from screentinker/fix/wal-checkpoint-startup-line

The checkpointer startup line no longer describes a policy it stopped having

docs/player-parity.md

@@ -37,16 +37,18 @@ el?.addEventListener('change', () => sendCommand(device.id, cmd, { level: parseI
 | player | what the handler reads | result |
 |---|---|---|
 | Android | `payload.optDouble("level", -1.0)` | ✅ works |
-| web | `data.payload?.value ?? data.value` | 💀 `undefined` → `isFinite` fails → silent no-op |
-| Tizen | `payload.value ?? payload.volume` | 💀 `undefined` → logs `no usable value in payload` |
-| BrightSign | (the web player) | 💀 as web |
+| web | `payload.level` (fraction), `value` still read as a percentage | ✅ **fixed in 1.9.31** |
+| Tizen | `payload.level` (fraction) | ✅ fixed in 1.9.31 — but see the note below on when the baseline may move |
+| BrightSign | (the web player) | ✅ as web |
 
-Three of the four players have a complete, working volume implementation that cannot be driven,
-because nobody checked the payload key against the sender. **Fix: one line in
-`server/player/index.html` and one in `tizen/js/app.js` — accept `level` (0..1) as well.** Until
-then `audio.volume` has been removed from the `web` and `brightsign` baselines, and
-`test/player-parity-baselines.test.js` holds that as a **biconditional**: fix the player and the
-test fails, telling you to put the baseline entry back.
+Three of the four players had a complete, working volume implementation that could not be driven,
+because nobody checked the payload key against the sender. Fixed in 1.9.31: the fraction is now
+canonical everywhere, and the scale is chosen by WHICH KEY arrived rather than by the magnitude of
+the number (`1` is legal under both conventions, so guessing from the value is wrong for somebody).
+
+`audio.volume` is back in the `web` and `brightsign` baselines as of 1.9.31, and **not** in the
+`tizen` one. That asymmetry is the model, not an oversight — see
+[When a baseline may move](#when-a-baseline-may-move).
 
 ### 2. Every #161 Tier-2 command was refused for the entire fleet — FIXED here
 
@@ -98,7 +100,7 @@ describe content rendering. They are informational, and shown to the operator in
 | capability | Android | Web | Tizen | BrightSign |
 |---|---|---|---|---|
 | `audio.mute` | ✅ `device:mute-changed` → `setVideoMuted`, incl. YouTube via the IFrame bridge | ✅ | ✅ incl. YouTube via `postMessage` | ✅ as web |
-| `audio.volume` | ✅ `set_volume` reads `payload.level` | 💀 reads `payload.value`; dashboard sends `level` | 💀 handler exists (`applyVolume`, incl. `tizen.tvaudiocontrol`) and reads `payload.value` | 💀 as web |
+| `audio.volume` | ✅ `set_volume` reads `payload.level` | ✅ reads `payload.level` (1.9.31) | ✅ `applyVolume` reads `payload.level` (1.9.31, incl. `tizen.tvaudiocontrol`) | ✅ as web |
 
 ## Display
 
@@ -250,8 +252,37 @@ declares anything, every display reading a baseline is running **v1.9.28 or olde
 | `android` | **removed `display.power`** | v1.9.28 `MainActivity`: `"screen_on" -> Log.w("no privileged wake path on a non-rooted panel — no-op")`. The ON half is dead on 100% of fielded panels, and one capability renders **both** buttons. |
 | `android` | **removed `system.reboot`** | `STPolicy.reboot()` requires device owner; off-owner v1.9.28 shows the accessibility power *dialog* — which on the accessibility-enabled panels common in this fleet paints that dialog **over the signage**. Owner provisioning is unreleased (#161 / PR #168 still open), so "device owner AND pre-1.9.29" is effectively an empty set. |
 | `tizen` | **added `display.power`** | v1.9.28 `app.js` implements both halves with no signing and no panel API: `showScreenOff()` / `clearScreenOff()` + `keepAwake()`. Unlike Android, neither half is privilege-gated. Withholding it hid a working control on every Tizen panel. |
-| `web` | **removed `audio.volume`** | v1.9.28 `index.html` contains the string `set_volume` **zero** times, and HEAD's handler reads the wrong payload key. |
-| `brightsign` | **removed `audio.volume`, `display.power`, `system.reboot`, `system.restart_player`, `offline.cache`** | All five need a host bridge (`hasHost()`) or a service worker that a Supervisor-built widget refuses. `system.restart_player` is the 2026-07-28 panel-blackout path. `offline.cache` is the documented lie this whole model exists to stop. |
+| `web` | **removed `audio.volume`, then RESTORED it in 1.9.31** | Removed when v1.9.28 `index.html` contained the string `set_volume` zero times. Restored once the handler landed — this player is served by the server, so there is no fielded build to lag behind. |
+| `brightsign` | **removed `display.power`, `system.reboot`, `system.restart_player`, `offline.cache`** (and `audio.volume`, restored in 1.9.31 with `web`) | All five need a host bridge (`hasHost()`) or a service worker that a Supervisor-built widget refuses. `system.restart_player` is the 2026-07-28 panel-blackout path. `offline.cache` is the documented lie this whole model exists to stop. |
+
+### When a baseline may move
+
+A baseline describes what an **un-updated** display can do, so the question "has this shipped?"
+has two different answers depending on how the player reaches the screen.
+
+**Served by the server — `web`, `brightsign`.** The player is a document this server hands out. A
+display running against this build *is* running this build's player; there is no such thing as a
+browser panel stuck on last release's. So the baseline moves the moment the server ships the fix,
+and holding it back hides a control that already works. `test/player-parity-baselines.test.js`
+judges these two against the working tree, in **both** directions.
+
+**Shipped as a device artifact — `android`, `tizen`.** The player is an APK or a `.wgt` sitting on
+the panel. Cutting a release puts nothing on any screen; a panel updates when somebody updates it,
+and this repo cannot know how many are still back on which build. These are judged against the
+**previous release**, and only in the over-claim direction: "the baseline claims it, so the shipped
+player had better implement it" is always worth failing on, while "HEAD gained the handler, so add
+it to the baseline" is a guess about the fleet, not a fact about it. A panel that HAS updated
+declares its own capabilities and never reads the baseline at all.
+
+The cost of the one-directional rule is that a stale entry can sit here after the artifact really
+has reached the fleet. That is a judgement call about panels, so a person makes it in
+`server/lib/player-capabilities.js` and records why — which is what the Tizen `audio.volume` note
+there is doing right now.
+
+> This distinction was learned the hard way. The test used to read "shipped" as *the newest tag*,
+> which is HEAD on a release commit — so tagging 1.9.31 flipped every biconditional at once and
+> demanded a baseline change for displays that could not possibly have the fix yet. The build went
+> red naming a baseline, with nothing in the diff to explain it.
 
 ### Consequence, deliberately accepted
 
@@ -267,7 +298,7 @@ declares `system.reboot` for itself and is unaffected.
 |---|---|---|---|---|
 | `playback.*` (all 7) | ✅ | ✅ | ✅ | ✅ |
 | `audio.mute` | ✅ | ✅ | ✅ | ✅ |
-| `audio.volume` | ✅ | ❌ no handler in v1.9.28 | ❌ payload | ❌ no handler in v1.9.28 |
+| `audio.volume` | ✅ | ❌ the fielded `.wgt` has no handler | ✅ since 1.9.31 (runs the served player) | ✅ since 1.9.31 |
 | `display.rotation` | ✅ | ✅ | ⚠️ graphics only | ✅ |
 | `display.power` | ❌ `screen_on` is a no-op | ✅ | ❌ needs host | ❌ |
 | `display.brightness` | ✅ Tier 0, since v1.9.10 | ❌ | ❌ | ❌ |

server/db/wal-checkpointer.js

@@ -121,7 +121,17 @@ function startWalCheckpointer(db, dbPath) {
   try { db.pragma('wal_checkpoint(TRUNCATE)'); } catch (_) { /* best-effort */ }
 
   worker = spawnWorker();
-  console.log(`[wal-checkpoint] off-thread checkpointer started (every ${config.walCheckpointIntervalMs}ms; escalate >${config.walCheckpointHighWaterMB}MB or ${config.walCheckpointStarvationRuns} growing runs; respawn max ${config.walCheckpointRespawnMax}/${config.walCheckpointRespawnWindowMs}ms)`);
+  // #240: this line is where an operator learns the escalation policy, so it must state ALL of
+  // it. It advertised only "3 growing runs" after the size floor and the cooldown were added,
+  // which is the half that no longer holds on its own — and reading it during an incident would
+  // send you looking for a checkpoint that the gates had in fact suppressed.
+  console.log(
+    `[wal-checkpoint] off-thread checkpointer started (PASSIVE every ${config.walCheckpointIntervalMs}ms; ` +
+    `blocking TRUNCATE when the WAL exceeds ${config.walCheckpointHighWaterMB}MB, ` +
+    `or after ${config.walCheckpointStarvationRuns} growing runs but only at >=${config.walCheckpointStarvationFloorMB}MB ` +
+    `and at most once per ${Math.round(config.walCheckpointEscalateCooldownMs / 1000)}s; ` +
+    `respawn max ${config.walCheckpointRespawnMax}/${config.walCheckpointRespawnWindowMs}ms)`
+  );
   return worker;
 }
 

server/lib/player-capabilities.js

@@ -167,6 +167,14 @@ const BASELINE = {
     // gated on it, so the entry describes content rendering rather than offering a button.
     'display.rotation',
     'remote.input',
+    // RESTORED in 1.9.31 with BASELINE.web, and for the same reason plus one of its own: a
+    // BrightSign runs the web player we serve, so it gets the fixed handler the moment the server
+    // does. The unit-specific question is whether the media element is even reachable on a player
+    // that puts video on a hardware plane — and that question is already settled by `audio.mute`
+    // above, which this baseline has always claimed: set_volume reaches setMediaVolume() and
+    // device:mute-changed reaches `currentVideoEl.muted`, the same element by the same path. If hwz
+    // silently swallowed one it would swallow both, so volume is exactly as honest as mute here.
+    'audio.volume',
     'sync.clock',
     // NOT offline.cache. This is the documented case, not a hypothetical: the XT245 on alpha has
     // navigator.serviceWorker, passes every presence check, and then never fetches sw.js because
@@ -182,9 +190,9 @@ const BASELINE = {
     // NOT system.reboot / display.power / display.resolution / system.self_update: all four are
     // BrightScript calls through a bridge this unit is not known to have.
     //
-    // NOT audio.volume / remote.screenshot / remote.stream: see BASELINE.web — the volume payload
-    // never lands, and a canvas capture on a hwz player cannot read the video plane, so it returns
-    // a frame with a hole where the content is.
+    // NOT remote.screenshot / remote.stream: a canvas capture on a hwz player cannot read the video
+    // plane, so it returns a frame with a hole where the content is. (audio.volume moved INTO the
+    // list above in 1.9.31 — the payload it was waiting on now lands.)
   ],
   // A browser tab. Deliberately the smallest set: it cannot reboot its host, rotate a panel, or
   // capture anything outside its own document.
@@ -195,16 +203,22 @@ const BASELINE = {
     'display.rotation',
     'remote.screenshot', 'remote.stream', 'remote.input',
     'system.restart_player',
+    // RESTORED in 1.9.31, having been removed by the audit that found the slider dead. Both of the
+    // reasons it was removed have expired, and the second one was reasoning from the wrong artifact:
+    //   1. It read `data.payload?.value ?? data.value` while the dashboard sends `{ level: 0..1 }`,
+    //      so the number was undefined and the handler declined. Fixed in 1.9.31 — index.html now
+    //      takes the fraction as canonical (volumeLevelFromCommand) and set_volume reaches
+    //      setMediaVolume().
+    //   2. The removal cited `git show v1.9.28:server/player/index.html` having no handler at all.
+    //      But this player is SERVED BY THE SERVER: a browser panel loads it from whatever build is
+    //      running, not from the release its row was created under. There is no such thing as a
+    //      browser panel stuck on the v1.9.28 player once the server moves — which is the whole
+    //      difference between this baseline and the Android/Tizen ones below, where an un-updated
+    //      panel really is running an old artifact.
+    // So the moment the server ships the fix, an undeclared web display can be driven, and holding
+    // the entry back would hide a control that works. Released and live on prod 2026-08-06.
+    'audio.volume',
     'sync.clock', 'offline.cache',
-    // NOT audio.volume, removed after audit, and for two independent reasons:
-    //   1. `git show v1.9.28:server/player/index.html` has no set_volume handler at all — zero
-    //      occurrences of the string. The fielded browser player ignores the command outright.
-    //   2. Even at HEAD the slider cannot work: index.html reads `data.payload?.value ?? data.value`
-    //      and tizen/js/app.js reads `payload.value ?? payload.volume`, while the dashboard sends
-    //      `{ level: 0..1 }` (frontend/js/views/device-detail.js bindLevel). Only the Android
-    //      handler reads `level`. Fixing that is one line in each player, and
-    //      test/player-parity-baselines.test.js is written as a BICONDITIONAL: the moment a player
-    //      accepts `level`, the test fails and tells you to put the baseline entry back.
   ],
 };
 

server/test/player-capabilities.test.js

@@ -123,10 +123,14 @@ test('the baseline describes a FIELDED player, not the one we are about to ship'
   // working controls disappear from every legacy Tizen display.
   const tizen = { platform: 'Tizen 6.5' };
   assert.equal(caps.supports(tizen, 'audio.volume'), false, 'the slider is dead on a fielded panel');
-  // Same answer on web and BrightSign, for a second and separate reason: those players read
-  // payload.value while the dashboard sends payload.level, so even HEAD's handler never fires.
-  assert.equal(caps.supports({ android_version: 'Web/Chrome' }, 'audio.volume'), false);
-  assert.equal(caps.supports({ platform: 'brightsign' }, 'audio.volume'), false);
+  // Web and BrightSign used to answer false here too, on the grounds that those players read
+  // payload.value while the dashboard sends payload.level. 1.9.31 fixed the payload AND, more to
+  // the point, those two players are SERVED BY THIS SERVER — an undeclared browser panel runs
+  // whatever build is answering it, so there is no fielded-vs-shipping gap to describe. Tizen keeps
+  // the old answer precisely because it does have one: its .wgt sits on the panel until someone
+  // updates it, and the fix reaching this repo put nothing on any screen.
+  assert.ok(caps.supports({ android_version: 'Web/Chrome' }, 'audio.volume'), 'the web player this build serves reads payload.level');
+  assert.ok(caps.supports({ platform: 'brightsign' }, 'audio.volume'), 'BrightSign runs that same served player');
   assert.ok(caps.supports({ client_type: 'apk' }, 'audio.volume'), 'Android reads the payload it is sent');
   assert.ok(caps.supports(tizen, 'audio.mute'), 'mute does work');
   assert.ok(caps.supports(tizen, 'remote.screenshot'), 'captureAndSend exists in the shipped player');

server/test/player-parity-baselines.test.js

@@ -88,17 +88,57 @@ function readShipped(rel) {
 const needsShippedSource = (t) =>
   shippedFromTag ? false : (t.skip('no release tag available — cannot judge a baseline against shipped source'), true);
 
+/*
+ * The two families of player update by completely different physics, and a baseline that ignores
+ * the difference is wrong for half the fleet:
+ *
+ *   SERVER-SERVED (web, brightsign) — the player is a document this server hands out. A display
+ *   running against this build IS running this build's player; there is no such thing as a browser
+ *   panel stuck on an old one. So the baseline moves the moment the server ships, and the WORKING
+ *   TREE is the honest source. Anything else hides a control that already works.
+ *
+ *   DEVICE ARTIFACT (android, tizen) — the player is an APK or a .wgt sitting on the panel.
+ *   Releasing puts nothing on anything; a panel updates when someone updates it, and the repo has
+ *   no way to know how many are still back on which build. Judged against the PREVIOUS release,
+ *   and only in the over-claim direction (see below).
+ *
+ * Conflating them is what made this file demand BASELINE.web gain audio.volume at the exact moment
+ * 1.9.31 was tagged — correct conclusion, arrived at by accident, and it would have dragged Tizen
+ * along with it onto panels that have no such fix.
+ */
+const SERVER_SERVED = new Set(['web', 'brightsign']);
+
 const SRC = {
-  web: readShipped('server/player/index.html'),
+  web: read('server/player/index.html'),
   android: [
     'android/app/src/main/java/com/remotedisplay/player/MainActivity.kt',
     'android/app/src/main/java/com/remotedisplay/player/service/WebSocketService.kt',
     'android/app/src/main/java/com/remotedisplay/player/telemetry/PlayerCapabilities.kt',
   ].map(read).join('\n'),
   tizen: ['tizen/js/app.js', 'tizen/js/device-control.js', 'tizen/js/capabilities.js'].map(readShipped).join('\n'),
-  brightsign: ['brightsign/st-bridge.js', 'brightsign/autorun.brs'].map(readShipped).join('\n'),
+  brightsign: ['brightsign/st-bridge.js', 'brightsign/autorun.brs'].map(read).join('\n'),
 };
 
+/*
+ * Assert a baseline entry against what its player can actually do.
+ *
+ * Server-served: BOTH directions. Over-claiming is a dead button; under-claiming hides a working
+ * one, and both are decidable right here from the tree we are about to serve.
+ *
+ * Device artifact: the OVER-CLAIM direction only. "The baseline claims it, so the shipped player
+ * had better implement it" is always true and always worth failing on. The reverse — "HEAD gained
+ * the handler, so put it in the baseline" — is not a fact about the fleet, it is a guess about how
+ * many panels have updated, and the honest answer for an undeclared panel is the floor. A panel
+ * that HAS updated declares its own capabilities and never reads the baseline at all. The cost is
+ * that a stale entry can sit here after the artifact really has reached the fleet; that is a
+ * judgement call, so it is made by a person in player-capabilities.js and noted there.
+ */
+function assertBaseline(family, capability, playerCan, { claimedMsg, missingMsg }) {
+  const claimed = caps.BASELINE[family].includes(capability);
+  if (claimed) { assert.ok(playerCan, claimedMsg); return; }
+  if (SERVER_SERVED.has(family)) assert.ok(!playerCan, missingMsg);
+}
+
 /*
  * Which command names each player has a branch for. A command a player never names is a dead
  * button by construction — the socket delivers it and the handler falls off the end.
@@ -213,11 +253,10 @@ test('BICONDITIONAL: audio.volume in a baseline iff that player reads the payloa
   readsLevel.brightsign = readsLevel.web;   // BrightSign runs the web player
 
   for (const family of ['android', 'web', 'tizen', 'brightsign']) {
-    const claimed = caps.BASELINE[family].includes('audio.volume');
-    assert.equal(claimed, readsLevel[family],
-      claimed
-        ? `BASELINE.${family} claims audio.volume but that player never reads payload.level — the slider is dead`
-        : `the ${family} player now reads payload.level: restore 'audio.volume' to BASELINE.${family}`);
+    assertBaseline(family, 'audio.volume', readsLevel[family], {
+      claimedMsg: `BASELINE.${family} claims audio.volume but that player never reads payload.level — the slider is dead`,
+      missingMsg: `the ${family} player now reads payload.level: restore 'audio.volume' to BASELINE.${family}`,
+    });
   }
 });
 
@@ -232,8 +271,9 @@ test('audio.mute is real on all four, unlike its neighbour', () => {
   }
 });
 
-test('BICONDITIONAL: offline.cache in a baseline iff that player has a media cache', (t) => {
-  if (needsShippedSource(t)) return;
+test('BICONDITIONAL: offline.cache in a baseline iff that player has a media cache', () => {
+  // No shipped-source guard here: this one settles every family by hand below rather than by
+  // grepping a tag, so a tagless clone loses nothing by running it.
   // Tizen's media cache is a NEW file — it was not in v1.9.28 — so the baseline must not claim it
   // even though HEAD's capabilities.js declares it at runtime. BrightSign's widget is not known to
   // permit a service worker at all.

server/test/wal-checkpoint-starvation-floor.test.js

@@ -108,6 +108,28 @@ test('#240: the worker actually applies the floor it is handed', () => {
 // Measured, not assumed: with a single reader mid-transaction, TRUNCATE returns busy=1
 // after sitting on its 5s busy timeout and reclaims nothing (probe: WAL 8.8MB -> 8.8MB,
 // worst main-thread write 4,936ms). That outcome must not be logged as a success.
+// The startup line is where an operator learns the policy. It went stale the moment the gates were
+// added — it still promised "3 growing runs" with no mention of the floor or the cooldown, so it
+// described a checkpointer that no longer existed. A log line that states a rule has to state the
+// whole rule, and nothing but a test keeps the two in step.
+test('#240: the startup line states the WHOLE escalation policy', () => {
+  const ctl = fs.readFileSync(path.join(__dirname, '..', 'db', 'wal-checkpointer.js'), 'utf8');
+  // Anchor forward from the message text, not back to `return worker;` — the idempotence guard at
+  // the top of startWalCheckpointer() returns first, so slicing to it yields nothing at all.
+  const start = ctl.indexOf('off-thread checkpointer started');
+  assert.ok(start > 0, 'the startup line is gone entirely');
+  const line = ctl.slice(start, start + 800);
+  for (const knob of [
+    'walCheckpointIntervalMs',
+    'walCheckpointHighWaterMB',
+    'walCheckpointStarvationRuns',
+    'walCheckpointStarvationFloorMB',
+    'walCheckpointEscalateCooldownMs',
+  ]) {
+    assert.ok(line.includes(knob), `the startup line must report ${knob} — an operator reads it as the policy`);
+  }
+});
+
 test('#240: a TRUNCATE that reclaimed nothing says so', () => {
   const src = fs.readFileSync(path.join(__dirname, '..', 'db', 'wal-checkpointer-worker.js'), 'utf8');
   assert.match(src, /busy === 1/, 'worker must inspect the checkpoint result');