fix: an absent battery reading is an absent KEY, not undefined (MX-835)

.github/workflows/test.yml

@@ -0,0 +1,35 @@
+# The unit gate. `node --test` over the pure modules in lib/, no framework.
+#
+# Until MX-835 this repo had NO test file, so `typecheck` was the whole story:
+# it proved the code COMPILES and never executed a line of it. That is exactly
+# how MX-835 shipped — `batteryFromSi` returned `{ minutesRemaining: undefined }`
+# on a Mac plugged into the wall, which types perfectly under a `.optional()`
+# field and which bb's RPC serialiser then refused as "not a JSON value",
+# failing the whole `current` call and blanking the panel.
+#
+# What this job covers: lib/*.test.ts, which is the pure half of the plugin
+# (battery-state mapping). What it does NOT cover: server.ts and app.tsx, whose
+# code needs a live bb host. Green here means the pure modules behave, not that
+# the plugin works.
+#
+# `--ignore-scripts` on the install: better-sqlite3 is a devDependency needed
+# only for its types, and its native rebuild is minutes of CI for nothing.
+name: Test
+
+on:
+  pull_request:
+  push:
+    branches: [main, main-public]
+  workflow_dispatch:
+
+jobs:
+  test:
+    name: node --test lib/
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-node@v4
+        with:
+          node-version: "22"
+      - run: npm ci --no-audit --no-fund --ignore-scripts
+      - run: npm test

.github/workflows/typecheck.yml

@@ -11,9 +11,10 @@
 #
 # What a green run here covers: `tsc --noEmit` under `strict` with
 # `skipLibCheck: false`, over server.ts, app.tsx, components, lib, hooks and
-# types. What it does NOT cover: this repo has ZERO test files, so no line of
-# it is ever EXECUTED in CI. Green means it compiles, not that it works — a
-# weaker claim than the sibling plugins' Test job, and deliberately named so.
+# types. What it does NOT cover: whether any of it WORKS. Green means it
+# compiles — MX-835 was a type-correct `undefined` that bb's RPC serialiser
+# refused at runtime, and it passed this job. Execution lives in the sibling
+# `Test` job (.github/workflows/test.yml), which covers lib/ only.
 name: Typecheck
 
 on:

lib/battery.test.ts

@@ -0,0 +1,150 @@
+// The one thing this file exists to prevent: a battery state carrying a key
+// whose value is `undefined`.
+//
+// bb's RPC serialiser walks the result and refuses `undefined` — it is not a
+// JSON value — and it fails the WHOLE call, not the field. On 2026-09-09 this
+// Mac, sitting on AC power with no time estimate, produced
+//
+//   system@0.1.0  running  (rpc current failed: rpc result at
+//   $result.sample.battery.minutesRemaining is not a JSON value (undefined))
+//
+// and the System panel rendered nothing at all (MX-835). `JSON.stringify`
+// DROPS such keys, so `encodeBattery`/`decodeBattery` round-trip clean and no
+// persisted-path test could ever have caught it; the assertion below is on the
+// live object, and it is `assertJsonValue` — the serialiser's rule, not a spot
+// check on one field name, because the next field added is the next outage.
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import {
+  asMinutes,
+  asPct,
+  batteryFromRemote,
+  batteryFromSi,
+  decodeBattery,
+  encodeBattery,
+  presentBattery,
+  withBattery,
+} from "./battery.ts";
+
+/** bb's rule: every value reachable from the result must be a JSON value. */
+function assertJsonValue(value: unknown, path = "$result"): void {
+  assert.notEqual(value, undefined, `${path} is not a JSON value (undefined)`);
+  if (Array.isArray(value)) {
+    value.forEach((v, i) => assertJsonValue(v, `${path}[${i}]`));
+  } else if (value !== null && typeof value === "object") {
+    for (const [k, v] of Object.entries(value)) assertJsonValue(v, `${path}.${k}`);
+  }
+}
+
+// Measured on mgrin's MacBook 2026-09-09T01:18Z, plugged in: IOKit reports
+// timeRemaining 65535 (0xFFFF, "no estimate") and systeminformation passes it
+// through verbatim. This is the exact reading that broke `current`.
+const MAC_ON_AC = {
+  hasBattery: true,
+  cycleCount: 151,
+  isCharging: false,
+  designedCapacity: 74742,
+  maxCapacity: 63271,
+  currentCapacity: 62453,
+  voltage: 12.976,
+  capacityUnit: "mWh",
+  percent: 100,
+  timeRemaining: 65535,
+  acConnected: true,
+  type: "Li-ion",
+  model: "bq40z651",
+  manufacturer: "Apple",
+  serial: "REDACTED",
+};
+
+test("MX-835: a Mac on AC with no time estimate serialises — no undefined key", () => {
+  const state = batteryFromSi(MAC_ON_AC);
+  assertJsonValue({ sample: { battery: state } });
+  assert.equal("minutesRemaining" in (state as object), false);
+  assert.deepEqual(state, {
+    present: true,
+    charging: false,
+    acConnected: true,
+    pct: 100,
+    cycleCount: 151,
+    healthPct: 85,
+  });
+});
+
+test("a charging battery drops minutesRemaining rather than holding undefined", () => {
+  const state = batteryFromSi({ ...MAC_ON_AC, isCharging: true, timeRemaining: 42 });
+  assertJsonValue(state);
+  assert.equal("minutesRemaining" in (state as object), false);
+  assert.equal((state as { charging: boolean }).charging, true);
+});
+
+test("a discharging battery with a real estimate keeps it", () => {
+  const state = batteryFromSi({ ...MAC_ON_AC, acConnected: false, timeRemaining: 137 });
+  assertJsonValue(state);
+  assert.equal((state as { minutesRemaining?: number }).minutesRemaining, 137);
+});
+
+test("every optional reading is omitted, not undefined, when unavailable", () => {
+  const state = batteryFromSi({ hasBattery: true, acConnected: false });
+  assertJsonValue(state);
+  assert.deepEqual(state, { present: true, charging: false, acConnected: false });
+  assert.deepEqual(Object.keys(state as object).sort(), ["acConnected", "charging", "present"]);
+});
+
+test("ABSENT and UNKNOWN stay distinct, and neither is a number", () => {
+  assert.deepEqual(batteryFromSi({ hasBattery: false }), { present: false });
+  assert.equal(batteryFromSi({}), undefined); // hasBattery not a boolean => UNKNOWN
+  assert.equal(batteryFromSi(null), undefined);
+});
+
+test("the remote sampler omits minutes it did not emit", () => {
+  const state = batteryFromRemote(
+    new Map([["battery_present", "1"], ["battery_pct", "77"], ["battery_ac", "1"]]),
+  );
+  assertJsonValue({ sample: { battery: state } });
+  assert.deepEqual(state, { present: true, charging: false, acConnected: true, pct: 77 });
+});
+
+test("the remote sampler: no key is UNKNOWN, battery_present=0 is ABSENT", () => {
+  assert.equal(batteryFromRemote(new Map()), undefined);
+  assert.deepEqual(batteryFromRemote(new Map([["battery_present", "0"]])), { present: false });
+});
+
+test("withBattery omits the sample key entirely when the state is UNKNOWN", () => {
+  const base = { ts: 1, cpuPct: 3 };
+  const unknown = withBattery(base, undefined);
+  assertJsonValue({ sample: unknown });
+  assert.equal("battery" in unknown, false);
+  assert.deepEqual(withBattery(base, { present: false }), { ts: 1, cpuPct: 3, battery: { present: false } });
+});
+
+test("presentBattery never writes a key it was given as undefined", () => {
+  const state = presentBattery(
+    { charging: false, acConnected: false },
+    { pct: 5, minutesRemaining: undefined, cycleCount: undefined, healthPct: undefined },
+  );
+  assertJsonValue(state);
+  assert.deepEqual(Object.keys(state).sort(), ["acConnected", "charging", "pct", "present"]);
+});
+
+test("65535 and -1 are refused as estimates; a plausible figure is kept", () => {
+  assert.equal(asMinutes(65535), undefined);
+  assert.equal(asMinutes(-1), undefined);
+  assert.equal(asMinutes(0), undefined);
+  assert.equal(asMinutes(1439), 1439);
+  assert.equal(asMinutes(1440), undefined);
+});
+
+test("a percentage outside 0..100 is not clamped into range", () => {
+  assert.equal(asPct(-1), undefined);
+  assert.equal(asPct(101), undefined);
+  assert.equal(asPct(50.4), 50);
+});
+
+test("encode/decode round-trips the live state, UNKNOWN stays NULL", () => {
+  const state = batteryFromSi(MAC_ON_AC)!;
+  assert.deepEqual(decodeBattery(encodeBattery(state)), state);
+  assert.equal(encodeBattery(undefined), null);
+  assert.equal(decodeBattery(null), undefined);
+  assert.equal(decodeBattery('{"present":"maybe"}'), undefined); // corrupt cell => UNKNOWN
+});

lib/battery.ts

@@ -19,6 +19,19 @@
 // emitted coerces to 0, and `|| 0` collapses a genuine 0 into the same 0. A
 // battery routed through it turns a desktop into a dying laptop. Nothing here
 // uses it; absence stays `undefined` the whole way through.
+//
+// One more rule, and it is the one that took the plugin down (MX-835): an
+// optional field that has no reading is an ABSENT KEY, never a present key
+// holding `undefined`. bb's RPC serialiser refuses `undefined` — it is not a
+// JSON value — and it refuses the WHOLE call, not the field, so a Mac sitting
+// on AC power with no time estimate returned
+//   rpc current failed: $result.sample.battery.minutesRemaining is not a JSON
+//   value (undefined)
+// and the panel rendered nothing at all. The persisted path never saw it
+// because `JSON.stringify` drops such keys silently, which is exactly why the
+// bug reached a running machine: encode/decode round-trips clean, the live
+// object does not. Build the optional fields through `presentBattery` /
+// `withBattery` below rather than assigning them directly.
 import { z } from "zod";
 
 export const batteryStateShape = z.discriminatedUnion("present", [
@@ -36,6 +49,40 @@ export const batteryStateShape = z.discriminatedUnion("present", [
   }),
 ]);
 export type BatteryState = z.infer<typeof batteryStateShape>;
+export type PresentBattery = Extract<BatteryState, { present: true }>;
+
+/**
+ * The PRESENT state, carrying only the optional readings that actually have a
+ * value. A key holding `undefined` is not a JSON value and bb's RPC serialiser
+ * rejects the entire response over one — see the module header (MX-835).
+ */
+export function presentBattery(
+  base: { charging: boolean; acConnected: boolean },
+  optional: {
+    pct?: number | undefined;
+    minutesRemaining?: number | undefined;
+    cycleCount?: number | undefined;
+    healthPct?: number | undefined;
+  },
+): PresentBattery {
+  const state: PresentBattery = { present: true, ...base };
+  for (const [key, value] of Object.entries(optional)) {
+    if (value !== undefined) (state as Record<string, unknown>)[key] = value;
+  }
+  return state;
+}
+
+/**
+ * Attach a battery state to a sample, or leave the key off entirely when the
+ * state is UNKNOWN. Same rule as `presentBattery`, one level up: `battery`
+ * is optional on the sample too, so `{ battery: undefined }` fails the same way.
+ */
+export function withBattery<T extends object>(
+  base: T,
+  state: BatteryState | undefined,
+): T & { battery?: BatteryState } {
+  return state === undefined ? base : { ...base, battery: state };
+}
 
 const finite = (v: unknown): number | undefined =>
   typeof v === "number" && Number.isFinite(v) ? v : undefined;
@@ -67,19 +114,19 @@ export function batteryFromSi(raw: unknown): BatteryState | undefined {
   const charging = b.isCharging === true;
   const designed = finite(b.designedCapacity);
   const max = finite(b.maxCapacity);
-  return {
-    present: true,
-    pct: asPct(b.percent),
-    charging,
-    acConnected: b.acConnected === true,
-    // A "time remaining" while charging is an estimate of the wrong thing.
-    minutesRemaining: charging ? undefined : asMinutes(b.timeRemaining),
-    cycleCount: finite(b.cycleCount),
-    healthPct:
-      designed !== undefined && max !== undefined && designed > 0
-        ? Math.round((max / designed) * 100)
-        : undefined,
-  };
+  return presentBattery(
+    { charging, acConnected: b.acConnected === true },
+    {
+      pct: asPct(b.percent),
+      // A "time remaining" while charging is an estimate of the wrong thing.
+      minutesRemaining: charging ? undefined : asMinutes(b.timeRemaining),
+      cycleCount: finite(b.cycleCount),
+      healthPct:
+        designed !== undefined && max !== undefined && designed > 0
+          ? Math.round((max / designed) * 100)
+          : undefined,
+    },
+  );
 }
 
 /**
@@ -98,13 +145,13 @@ export function batteryFromRemote(values: Map<string, string>): BatteryState | u
     return Number.isFinite(n) ? n : undefined;
   };
   const charging = values.get("battery_charging") === "1";
-  return {
-    present: true,
-    pct: asPct(numeric("battery_pct")),
-    charging,
-    acConnected: values.get("battery_ac") === "1",
-    minutesRemaining: charging ? undefined : asMinutes(numeric("battery_minutes")),
-  };
+  return presentBattery(
+    { charging, acConnected: values.get("battery_ac") === "1" },
+    {
+      pct: asPct(numeric("battery_pct")),
+      minutesRemaining: charging ? undefined : asMinutes(numeric("battery_minutes")),
+    },
+  );
 }
 
 /** NULL in the column means UNKNOWN, which is what every pre-migration row is. */

package.json

@@ -3,6 +3,7 @@
   "version": "0.1.0",
   "type": "module",
   "scripts": {
+    "test": "node --test --experimental-strip-types --test-timeout=10000 lib/*.test.ts",
     "typecheck": "tsc --noEmit"
   },
   "engines": {

server.ts

@@ -29,6 +29,7 @@ import {
   batteryStateShape,
   decodeBattery,
   encodeBattery,
+  withBattery,
 } from "./lib/battery";
 
 const run = promisify(execFile);
@@ -56,6 +57,10 @@ const sampleShape = z.object({
   // Optional on purpose, at every layer: absent means the sampler could not
   // tell, which is a different answer from "this machine has no battery"
   // ({ present: false }) and from any number at all. See lib/battery.ts.
+  //
+  // Optional means the KEY IS ABSENT. Every construction of a Sample goes
+  // through `withBattery`, because `{ battery: undefined }` is not JSON and
+  // bb's RPC serialiser fails the whole `current` call over it (MX-835).
   battery: batteryStateShape.optional(),
 });
 type Sample = z.infer<typeof sampleShape>;
@@ -387,7 +392,7 @@ export default async function plugin(bb: BbPluginApi) {
     ]);
     const dataVol =
       fs.find((f) => f.mount === "/System/Volumes/Data") ?? fs.find((f) => f.mount === "/") ?? fs[0];
-    return {
+    return withBattery({
       ts: Date.now(),
       cpuPct: Number(load.currentLoad.toFixed(1)),
       // si reports avgLoad PER CORE; store the raw 1-minute load so the number
@@ -402,8 +407,7 @@ export default async function plugin(bb: BbPluginApi) {
       swapUsedMb: Math.round((mem.swapused ?? 0) / 1048576),
       diskTotalGb: dataVol ? Math.round(dataVol.size / 1073741824) : 0,
       diskUsedGb: dataVol ? Math.round(dataVol.used / 1073741824) : 0,
-      battery,
-    };
+    }, battery);
   }
 
   const insert = (hostId: string, s: Sample) => {
@@ -423,7 +427,7 @@ export default async function plugin(bb: BbPluginApi) {
   const rowToSample = (r: Record<string, unknown>): Sample => {
     const count = Number(r.cpu_count) || 1;
     const load1 = Number(r.load1);
-    return {
+    return withBattery({
       ts: Number(r.ts),
       // Pre-migration rows have no cpu_pct; fall back to the old (wrong but
       // present) load-derived figure so the sparkline has no hole.
@@ -442,8 +446,7 @@ export default async function plugin(bb: BbPluginApi) {
       diskUsedGb: Number(r.disk_used_gb),
       // Same shape as cpu_pct above: a missing column is a stated absence, not
       // Number(null) === 0.
-      battery: decodeBattery(r.battery),
-    };
+    }, decodeBattery(r.battery));
   };
 
   const latest = (hostId: string): Sample | null => {
@@ -579,7 +582,7 @@ export default async function plugin(bb: BbPluginApi) {
       const num = (key: string) => Number(values.get(key)) || 0;
       const memTotalMb = Math.round(num("mem_total_kb") / 1024);
       const memUsedMb = Math.round(num("mem_used_kb") / 1024);
-      const sample: Sample = {
+      const sample: Sample = withBattery({
         ts: Date.now(),
         cpuPct: Math.min(100, Math.max(0, num("cpu_pct"))),
         load1: num("load1"),
@@ -594,8 +597,7 @@ export default async function plugin(bb: BbPluginApi) {
         diskUsedGb: Math.round(num("disk_used_kb") / 1048576),
         // Deliberately NOT num(): a remote host may legitimately be a desktop,
         // and num() would render that as 0%.
-        battery: batteryFromRemote(values),
-      };
+      }, batteryFromRemote(values));
       return { sample, topCpu, topMem, uptime: values.get("uptime") ?? "" };
     } finally {
       // Always reap the one-shot terminal — success, failure, or abort. On