Skip to content

Commit c00bf2e

Browse files
fix(runs): close findings from the session-wide review (M-1 + L-1/L-2/L-3)
Four findings from the final ~5-hour holistic review (zero CRITICAL/HIGH overall): - M-1 (latent corruption): boot's fresh-number branch left a stale activeDbRunId restored from the snapshot, so a later Save would updateRunOnFinish the WRONG SQLite row. Null it when minting a fresh number (the reused branch keeps it — the content-key match proves it belongs to that run). - L-1: runRetestSkips now clears state.loadedReadonly — a retest mutates the loaded run in place, so it's no longer read-only; an interrupted retest stays resumable (and the run keeps its own number/dbRunId, so resume can't duplicate). Makes loadRunIntoState's "cleared by Retest" contract true. - L-2: boot run-number fallback uses Math.max(...numbers) instead of the last array element — robust to index.runs reordering after delete+find-or-update, matching latestRunNumber(). - L-3: /api/clear resets status='idle' so the wiped view isn't shown under a stale stopped/error overlay. Reviewed: safe to commit, no CRITICAL/HIGH/MEDIUM. Test suite green (188/31/45/35).
1 parent 140cd36 commit c00bf2e

2 files changed

Lines changed: 15 additions & 1 deletion

File tree

audit/pipeline.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1260,6 +1260,11 @@ export async function runRetestSkips(skipAddrs, state, broadcast) {
12601260
state.errorMessage = null;
12611261
state.retryCount = 0;
12621262
state.retestMode = true;
1263+
// A retest actively works the run in place (persistActiveRun writes fresh rows),
1264+
// so it's no longer a pristine read-only snapshot — clear loadedReadonly so an
1265+
// interrupted retest can be resumed (matches loadRunIntoState's documented
1266+
// "cleared by Retest" contract).
1267+
state.loadedReadonly = false;
12631268
state.retestTotal = skipAddrs.length;
12641269
state.retestTested = 0;
12651270
state.retestPassed = 0;

server.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1140,7 +1140,7 @@ function rehydrateState(results) {
11401140
// overwrite — a real saved run (the bug that corrupted Test #11).
11411141
if (state.activeRunNumber == null) {
11421142
const cand = index.activeRun != null ? index.activeRun
1143-
: (index.runs.length > 0 ? index.runs[index.runs.length - 1].number : null);
1143+
: (index.runs.length > 0 ? Math.max(...index.runs.map(r => r.number)) : null);
11441144
const candData = cand != null ? loadRun(cand) : null;
11451145
// Strengthen "is the working set the SAME run as `cand`?" beyond length:
11461146
// two DIFFERENT audits of the same chain set have identical length but
@@ -1160,6 +1160,12 @@ function rehydrateState(results) {
11601160
// could overwrite a different run. Forcing a snapshot now means the next
11611161
// boot restores this number instead of recomputing it.
11621162
if (!reused) {
1163+
// Fresh number → a NEW unsaved run with no SQLite row of its own. Drop any
1164+
// activeDbRunId the snapshot restored: it points at a DIFFERENT run, and
1165+
// keeping it would make a later Save updateRunOnFinish the wrong row
1166+
// (clobbering that run's counts/spend). Same intent as startFreshRun,
1167+
// which overwrites it via insertRun — here there's no insertRun, so null.
1168+
state.activeDbRunId = null;
11631169
try { flushStateSnapshot(); }
11641170
catch (e) { console.error('[boot] reserve run-number snapshot failed:', e.message); }
11651171
}
@@ -2744,6 +2750,9 @@ app.post('/api/clear', adminOnly, (req, res) => {
27442750
state.currentNode = null;
27452751
state.resumeHeadAddr = null;
27462752
state.activeBatchId = 0;
2753+
// Drop a stale 'stopped'/'error' status so the wiped view isn't shown under a
2754+
// paused/error overlay with zero rows.
2755+
state.status = 'idle';
27472756
saveResults(state);
27482757
broadcastStateFresh();
27492758
res.json({ ok: true });

0 commit comments

Comments
 (0)