fix(cleanup/db): harden raw_json offload, reclaim orphan raw dirs, keep all unfinished batches, correct char-vs-byte labels

Closes the 4 defects from the adversarial review of the DB slim-down /
disk-hardening work:

FIX 1 (MED) — cleanup.mjs raw_json backfill now validates a JSON round-trip
before NULLing the column. Action 1 previously only stat-checked size>0; a
non-empty file can still be truncated/corrupt. It now reads the written file
back and JSON.parse()s it, only adding the id to the to-be-NULLed set when the
parse SUCCEEDS. Any read/parse failure leaves the column intact, logs a warning
naming run_id/result_id, and counts toward the skipped total reported in fix
output.

FIX 2 (MED/LOW) — orphan raw dirs are now reclaimable. Added a namespace-correct
orphan-raw-dir sweep to cleanup.mjs (new report section 7 + a --fix reclaim
gated on the existing backup+checkpoint guard): run-<id> dirs whose <id> has no
row in the runs table are orphans; reported with reclaimable bytes and rm -rf'd
under --fix. Handles RAW_DIR absent; stray non-run-<int> entries are reported
but never deleted. deleteRun() in server.js is deliberately UNCHANGED (only a
clarifying comment added) — it keys on the file-index run NUMBER, a different
namespace from the runs-table id that keys raw dirs, and no mapping holds for
every path (dbRunId is null for legacy runs and absent for the
interrupted/brand-new run case, where the only id available is the LIVE run's).
A wrong-id rm would destroy live raw data; correctness over completeness.

FIX 3 (LOW) — pruneBatchResults() now keeps ALL unfinished batches, not just
getActiveBatch's single newest one. Keep-set step (a) selects every id from
batches WHERE finished_at IS NULL, so a second unfinished batch's batch_results
can no longer be stripped (which would orphan its parent row, since the parent
DELETE guards finished_at IS NOT NULL). Added a :memory: test asserting both
unfinished batches survive while old finished ones are pruned, with no orphaned
children and a clean foreign_key_check.

FIX 4 (LOW) — corrected "16KB"/"16 KB"/byte wording to "16384 chars (UTF-8 byte
size varies)" at every mislabeled site in core/db.js (insertErrorLog cap site +
v11 migration block) and scripts/cleanup.mjs (LOG_SNIPPET_CAP + report/fix
messages). Numeric value and behavior are unchanged.

The :memory: raw_json INLINE behavior, TEST RUN paths, and ETA code are
untouched. npm test + npm run test:integration both fully green.

Author: Sentinel-Bluebuilder

core/db.js

@@ -477,13 +477,14 @@ function runMigrations(db) {
 
   if (current < 11) {
     db.transaction(() => {
-    // ── Migration v11: cap error_logs.log_snippet at 16 KB ──────────────────
-    // The inline log_snippet column historically stored up to 512 KB per
-    // failure, bloating audit.db. The full log already lives on disk in the
-    // per-run audit-*.log file, so the DB only needs a readable snippet for
-    // the copy-failure UX. Going forward insertErrorLog() caps at 16384 chars
-    // from the tail; this migration truncates any legacy oversized rows to
-    // match. substr(s, length(s) - 16384 + 1) keeps the LAST 16384 chars,
+    // ── Migration v11: cap error_logs.log_snippet at 16384 chars ────────────
+    // (16384 chars; UTF-8 byte size varies). The inline log_snippet column
+    // historically stored up to ~512K chars per failure, bloating audit.db. The
+    // full log already lives on disk in the per-run audit-*.log file, so the DB
+    // only needs a readable snippet for the copy-failure UX. Going forward
+    // insertErrorLog() caps at 16384 chars from the tail; this migration
+    // truncates any legacy oversized rows to match.
+    // substr(s, length(s) - 16384 + 1) keeps the LAST 16384 chars,
     // mirroring the `.slice(-16384)` tail semantics. Bounded fast UPDATE —
     // safe on the boot path (no file I/O here).
     db.prepare(`
@@ -976,7 +977,7 @@ export function getRunStats(runId, which) {
  * @param {string}  opts.stage       - 'handshake'|'session'|'speedtest'|'wallet'|'rpc'|'other'
  * @param {string}  [opts.error_code]
  * @param {string}  [opts.error_message]
- * @param {string}  [opts.log_snippet] - a readable snippet of log context (capped at 16 KB)
+ * @param {string}  [opts.log_snippet] - a readable snippet of log context (capped at 16384 chars; UTF-8 byte size varies)
  * @returns {number} inserted id
  */
 export function insertErrorLog({
@@ -987,8 +988,8 @@ export function insertErrorLog({
   log_snippet = null,
 }, which) {
   const db = getDb(which);
-  // Store only a readable snippet of the per-node tester log (capped at 16 KB
-  // = 16384 chars from the tail). The FULL log is persisted to disk in the
+  // Store only a readable snippet of the per-node tester log (capped at 16384
+  // chars from the tail; UTF-8 byte size varies). The FULL log is persisted to disk in the
   // per-run audit-*.log file; the DB column exists purely to power the
   // copy-failure UX, so a bounded tail is all we need. Production callers
   // already pre-truncate to ~4 KB via pipeline.js (_sanitizeSnippet /
@@ -1694,10 +1695,14 @@ export function getLastBatch(which) {
  * re-tests every node and writes a full set of rows.
  *
  * KEEP-SET RATIONALE — three readers must never lose their data:
- *   (a) the ACTIVE batch (finished_at IS NULL): the continuous-loop resume path
- *       re-seeds already-tested addresses from its batch_results (getActiveBatch
- *       + raw SELECTs in audit/continuous.js). Pruning it mid-flight would lose
- *       the resume seed.
+ *   (a) ALL active/unfinished batches (finished_at IS NULL): the continuous-loop
+ *       resume path re-seeds already-tested addresses from its batch_results
+ *       (getActiveBatch + raw SELECTs in audit/continuous.js). Pruning one
+ *       mid-flight would lose the resume seed. We keep EVERY unfinished batch,
+ *       not just getActiveBatch's single newest one — if two+ batches have
+ *       finished_at IS NULL, dropping the older one's batch_results would orphan
+ *       its parent row (the parent DELETE guards finished_at IS NOT NULL, so the
+ *       childless row would survive).
  *   (b) the single most-recent FINISHED batch (getLastBatch): drives the /live
  *       last-completed snapshot. Pruning it would blank /live between runs.
  *   (c) the last K batches overall (by recency): preserves recent history for
@@ -1734,9 +1739,14 @@ export function pruneBatchResults({ keepBatches = DEFAULT_BATCH_RETENTION } = {}
       keepIds.add(row.id);
     }
 
-    // (a) the active batch (finished_at IS NULL).
-    const active = getActiveBatch(which);
-    if (active?.batch?.id != null) keepIds.add(active.batch.id);
+    // (a) ALL active/unfinished batches (finished_at IS NULL). Keeping only
+    // getActiveBatch's single newest unfinished batch would let the child DELETE
+    // strip a second unfinished batch's batch_results, orphaning its parent row.
+    for (const row of db.prepare(
+      'SELECT id FROM batches WHERE finished_at IS NULL',
+    ).all()) {
+      keepIds.add(row.id);
+    }
 
     // (b) the most-recent finished batch.
     const last = getLastBatch(which);

scripts/cleanup.mjs

@@ -23,8 +23,9 @@
  *                                   – offload inline results.raw_json blobs to
  *                                     results/raw/run-<run_id>/<id>.json, then NULL
  *                                     the column (write+verify BEFORE null = no loss)
- *                                   – cap legacy error_logs.log_snippet at 16 KB tail
- *                                     (idempotent backstop to migration v11)
+ *                                   – cap legacy error_logs.log_snippet at the
+ *                                     16384-char tail (UTF-8 byte size varies;
+ *                                     idempotent backstop to migration v11)
  *                                   – prune batch_results to DEFAULT_BATCH_RETENTION
  *   --purge-orphan-logs       (with --fix) quarantine orphan audit logs into
  *                             results/.cleanup-trash/<ts>/ (move, not delete;
@@ -44,7 +45,7 @@
  * boot-reconcile, continuous-loop and retest paths and could fabricate values.
  */
 import Database from 'better-sqlite3';
-import { readFileSync, writeFileSync, existsSync, readdirSync, statSync, renameSync, mkdirSync, copyFileSync } from 'fs';
+import { readFileSync, writeFileSync, existsSync, readdirSync, statSync, renameSync, mkdirSync, copyFileSync, rmSync } from 'fs';
 import path from 'path';
 import { fileURLToPath } from 'url';
 import { RAW_DIR, DEFAULT_BATCH_RETENTION } from '../core/constants.js';
@@ -64,7 +65,7 @@ const MATCH_TOLERANCE_MS = 15_000;
 const BACKFILL_TOLERANCE_MS = 60_000; // wider dedup window for backfill (save-time skew)
 const RECENT_LOG_MS = 24 * 60 * 60 * 1000;       // never purge a log written in the last day
 const RUNAWAY_NOTES = 'continuous-loop iteration%';
-const LOG_SNIPPET_CAP = 16384;                    // bytes — mirrors core/db.js insertErrorLog + migration v11
+const LOG_SNIPPET_CAP = 16384;                    // 16384 chars (UTF-8 byte size varies) — mirrors core/db.js insertErrorLog + migration v11
 
 // Human-readable byte size for the slim-down report.
 const humanBytes = n => {
@@ -228,13 +229,14 @@ if (existsSync(RESULTS_DIR)) {
 
 // ─── 6. DB slim-down (one-time backfill for legacy data) ──────────────────────
 // These mirror the forward-going behaviour that core/db.js now applies on every
-// insert: raw_json blobs offloaded to per-run files, log_snippet capped at 16 KB,
-// batch_results bounded. Existing rows from before those commits still carry the
+// insert: raw_json blobs offloaded to per-run files, log_snippet capped at 16384
+// chars (UTF-8 byte size varies), batch_results bounded. Existing rows from
+// before those commits still carry the
 // inline blob / oversized snippet / unbounded batch history; this section reports
 // (and, under --fix, performs) the one-time migration of that legacy data.
 section('6. DB slim-down (legacy backfill)');
 let rawJsonRows = 0;          // results rows still holding an inline raw_json blob
-let logSnippetRows = 0;       // error_logs rows whose snippet exceeds the 16 KB cap
+let logSnippetRows = 0;       // error_logs rows whose snippet exceeds the 16384-char cap (UTF-8 byte size varies)
 let batchCount = 0;           // total batches
 let batchResultRows = 0;      // total batch_results rows
 if (rdb) {
@@ -249,7 +251,7 @@ if (rdb) {
   try {
     logSnippetRows = rdb.prepare(`SELECT COUNT(*) AS c FROM error_logs WHERE log_snippet IS NOT NULL AND length(log_snippet) > ${LOG_SNIPPET_CAP}`).get().c;
     logSnippetRows > 0
-      ? repairable(`${logSnippetRows.toLocaleString()} error_logs row(s) exceed the 16 KB log_snippet cap (would truncate to tail)`)
+      ? repairable(`${logSnippetRows.toLocaleString()} error_logs row(s) exceed the 16384-char log_snippet cap (UTF-8 byte size varies; would truncate to tail)`)
       : ok('no oversized log_snippet rows (migration v11 already applied)');
   } catch (e) { hard(`log_snippet scan failed: ${e.message}`); }
 
@@ -263,6 +265,59 @@ if (rdb) {
   } catch (e) { hard(`batch_results scan failed: ${e.message}`); }
 }
 
+// ─── 7. Orphan raw_json dirs (results/raw/run-<id>) ───────────────────────────
+// Each result's raw_json blob is offloaded to results/raw/run-<id>/<id>.json,
+// keyed by the SQLite runs-table id. When a run row is deleted (runaway prune,
+// backfill churn, manual cleanup) its raw dir is left behind — server.js
+// deleteRun() operates on the file-index run NUMBER, a DIFFERENT namespace from
+// the runs-table id that keys these dirs, so it can't safely reclaim them (a
+// wrong-id rm would destroy a LIVE run's raw data). This sweep is the
+// namespace-correct reclaimer: a run-<id> dir whose <id> has no row in `runs`
+// is an orphan. Stray entries that aren't run-<int> dirs are reported but never
+// deleted.
+section('7. Orphan raw dirs (results/raw)');
+const orphanRawDirs = [];   // { name, dir, bytes }
+let orphanRawBytes = 0;
+if (!existsSync(RAW_DIR)) {
+  ok(`no raw dir at ${path.relative(ROOT, RAW_DIR)} (ok — nothing offloaded yet)`);
+} else if (!rdb) {
+  info('raw dir present but audit.db unreadable — cannot determine orphans (skipped)');
+} else {
+  // Recursively sum a directory's byte size for the reclaimable-bytes report.
+  const dirBytes = (d) => {
+    let total = 0;
+    let entries;
+    try { entries = readdirSync(d, { withFileTypes: true }); }
+    catch (e) { console.error('[cleanup] raw dir size scan failed:', e.message); return 0; }
+    for (const ent of entries) {
+      const p = path.join(d, ent.name);
+      if (ent.isDirectory()) total += dirBytes(p);
+      else { try { total += statSync(p).size; } catch (e) { console.error('[cleanup] raw file stat failed:', e.message); } }
+    }
+    return total;
+  };
+  const runExists = rdb.prepare('SELECT 1 FROM runs WHERE id = ? LIMIT 1');
+  let names;
+  try { names = readdirSync(RAW_DIR, { withFileTypes: true }); }
+  catch (e) { hard(`raw dir listing failed: ${e.message}`); names = []; }
+  for (const ent of names) {
+    const m = /^run-(\d+)$/.exec(ent.name);
+    if (!ent.isDirectory() || !m) { info(`unexpected entry in raw dir (left untouched): ${ent.name}`); continue; }
+    const id = parseInt(m[1], 10);
+    let present;
+    try { present = !!runExists.get(id); }
+    catch (e) { hard(`runs lookup for raw dir ${ent.name} failed: ${e.message}`); continue; }
+    if (present) continue; // live/known run — keep
+    const dir = path.join(RAW_DIR, ent.name);
+    const bytes = dirBytes(dir);
+    orphanRawBytes += bytes;
+    orphanRawDirs.push({ name: ent.name, dir, bytes });
+    repairable(`orphan raw dir ${ent.name} (no runs row for id=${id}, ${humanBytes(bytes)} reclaimable)`);
+  }
+  if (!orphanRawDirs.length) ok('no orphan raw dirs (every run-<id> maps to a runs row)');
+  else { repairable(`${orphanRawDirs.length} orphan raw dir(s), ${humanBytes(orphanRawBytes)} total reclaimable`); if (!FIX) console.log(C.dim('    (pass --fix to rm -rf these orphan dirs)')); }
+}
+
 // Close our OWN read-only handle before mutating: if it stays open, the WAL
 // checkpoint below can report `busy` and falsely abort the whole --fix on a
 // perfectly healthy, server-stopped box.
@@ -379,11 +434,14 @@ if (FIX) {
           try {
             mkdirSync(dir, { recursive: true });
             writeFileSync(file, row.raw_json);
-            // VERIFY before we agree to null: file exists and is non-empty.
-            const st = statSync(file);
-            if (st.size > 0) ready.push(row.id);
-            else { skippedWrite++; hard(`raw_json offload: ${file} wrote 0 bytes — left column intact for result ${row.id}`); }
-          } catch (e) { skippedWrite++; hard(`raw_json offload: write failed for result ${row.id} — left column intact: ${e.message}`); }
+            // VERIFY before we agree to null: read the file back and JSON.parse
+            // it. A non-empty file can still be truncated/corrupt — size>0 is not
+            // enough. Only NULL the column when the round-trip parse SUCCEEDS;
+            // any read/parse failure leaves the column intact (no data loss).
+            const back = readFileSync(file, 'utf8');
+            JSON.parse(back); // throws on truncated/corrupt JSON
+            ready.push(row.id);
+          } catch (e) { skippedWrite++; hard(`raw_json offload: round-trip verify failed for run-${row.run_id}/result ${row.id} (${file}) — left column intact: ${e.message}`); }
         }
         if (ready.length) {
           sdb.transaction(() => { for (const id of ready) { nullStmt.run(id); offloaded++; } })();
@@ -402,7 +460,7 @@ if (FIX) {
         `UPDATE error_logs SET log_snippet = substr(log_snippet, length(log_snippet) - ${LOG_SNIPPET_CAP} + 1) WHERE log_snippet IS NOT NULL AND length(log_snippet) > ${LOG_SNIPPET_CAP}`,
       ).run();
       capRes.changes > 0
-        ? fixd(`capped ${capRes.changes.toLocaleString()} oversized log_snippet row(s) to 16 KB tail`)
+        ? fixd(`capped ${capRes.changes.toLocaleString()} oversized log_snippet row(s) to the 16384-char tail (UTF-8 byte size varies)`)
         : console.log(`  ${C.dim('↳ log_snippet cap: nothing to truncate (v11 already applied)')}`);
 
       // Action 3: prune batch_results to retention (reuse core/db.js keep-set).
@@ -417,6 +475,40 @@ if (FIX) {
     } catch (e) { hard(`DB slim-down failed (restore audit.db from the backup): ${e.message}`); }
   } else console.log(`  ${C.dim('↳ slim-down skipped (DB writes disabled)')}`);
 
+  // ── Orphan raw-dir reclaim ────────────────────────────────────────────────
+  // Gated on the SAME backup+checkpoint guard as the DB actions (dbWriteOk).
+  // Re-scan against the CURRENT runs table (the runaway prune above may have
+  // just orphaned more dirs) so one --fix pass reclaims them all. Namespace
+  // note: these dirs are keyed by the runs-table id, NOT the file-index run
+  // number — see the section-7 comment for why deleteRun() can't reclaim them.
+  if (dbWriteOk && existsSync(RAW_DIR)) {
+    try {
+      const { getDb } = await import('../core/db.js');
+      const odb = getDb();
+      const runExists = odb.prepare('SELECT 1 FROM runs WHERE id = ? LIMIT 1');
+      let removed = 0, reclaimed = 0;
+      let names;
+      try { names = readdirSync(RAW_DIR, { withFileTypes: true }); }
+      catch (e) { hard(`raw dir listing failed: ${e.message}`); names = []; }
+      for (const ent of names) {
+        const m = /^run-(\d+)$/.exec(ent.name);
+        if (!ent.isDirectory() || !m) continue; // never delete non-run-<int> entries
+        const id = parseInt(m[1], 10);
+        let present;
+        try { present = !!runExists.get(id); }
+        catch (e) { hard(`runs lookup for raw dir ${ent.name} failed: ${e.message}`); continue; }
+        if (present) continue;
+        const dir = path.join(RAW_DIR, ent.name);
+        const known = orphanRawDirs.find(o => o.name === ent.name);
+        const bytes = known ? known.bytes : 0;
+        try { rmSync(dir, { recursive: true, force: true }); removed++; reclaimed += bytes; fixd(`removed orphan raw dir ${ent.name} (${humanBytes(bytes)})`); }
+        catch (e) { hard(`could not remove orphan raw dir ${ent.name}: ${e.message}`); }
+      }
+      if (!removed) console.log(`  ${C.dim('↳ orphan raw dirs: none to reclaim')}`);
+      else fixd(`reclaimed ${removed} orphan raw dir(s), ${humanBytes(reclaimed)} freed`);
+    } catch (e) { hard(`orphan raw-dir reclaim failed: ${e.message}`); }
+  } else if (!dbWriteOk) console.log(`  ${C.dim('↳ orphan raw-dir reclaim skipped (DB writes disabled)')}`);
+
   if (PURGE_LOGS && orphanLogs.length && !abortAll) {
     const trash = path.join(RESULTS_DIR, '.cleanup-trash', String(ts));
     try {

server.js

@@ -1075,6 +1075,17 @@ function deleteRun(num) {
   if (i !== -1) index.runs.splice(i, 1);
   if (index.activeRun === num) index.activeRun = null;
   saveRunsIndex(index);
+  // NOTE: we deliberately do NOT delete this run's raw_json dir
+  // (results/raw/run-<id>) here. That dir is keyed by the SQLite runs-table id,
+  // whereas deleteRun operates on the file-index run NUMBER — a DIFFERENT
+  // namespace. The entry MAY carry a `dbRunId`, but it is null for legacy runs
+  // (pre-dbRunId field) and entirely absent for the interrupted/brand-new run
+  // case (i === -1, entry === null), where the only id available is
+  // state.activeDbRunId — i.e. the LIVE run's raw data. There is no mapping that
+  // holds for every code path, and a wrong-id `rm -rf` would destroy a live
+  // run's raw blobs. Correctness over completeness: the raw dir is reclaimed
+  // (namespace-correctly) by scripts/cleanup.mjs's orphan-raw-dir sweep, which
+  // matches run-<id> dirs against the runs table directly.
   // Remove the snapshot dir (results.json, summary.txt, failures.jsonl, audit.log).
   const runDir = path.join(RUNS_DIR, `test-${String(num).padStart(3, '0')}`);
   if (_ex(runDir)) {