diff --git a/src/main/roster.ts b/src/main/roster.ts index 16b8751d1..ea4fd4e2a 100644 --- a/src/main/roster.ts +++ b/src/main/roster.ts @@ -70,7 +70,10 @@ function entryCount(s: RosterSnapshot): number { * A class rather than free functions because the empty-guard needs to know * whether THIS run has written yet, and a module-level flag would be invisible * shared state that no test could reset. One instance per process in `index.ts`; - * tests make their own. + * tests make their own. The instance lives in MAIN, so a renderer reload reuses + * it: the guard's state survives reloads (the 2026-08-16 incident was two + * refusal-worthy writes in one run, minutes apart) and re-arms only on a fresh + * app launch. */ export class RosterStore { /** Set once this store has written successfully. The empty-guard applies only @@ -109,11 +112,14 @@ export class RosterStore { * THE EMPTY-GUARD. The dangerous sequence is: open the packaged build for the * first time, its localStorage is empty (different origin), the store boots * with zero agents, and the first mirror write flattens a file that holds a - * real roster. So the first write of a run is refused when it would replace a - * non-empty roster with an empty one. Later writes go through — by then an - * empty roster means the user actually removed their agents, and refusing it - * would make deletion impossible. The previous file is backed up either way, - * so even a wrong call here is recoverable. + * real roster. So an empty write is refused until this run has landed a + * NON-empty write — only then has the renderer proven it actually holds a + * roster, and a later empty write means the user really removed their agents + * (refusing those would make deletion impossible). The guard must survive a + * refusal: a renderer reload used to re-send the same empty snapshot, and + * because the first refusal disarmed the guard, the second empty write went + * through and flattened the file (seen live 2026-08-16 20:40:03). The previous + * file is backed up either way, so even a wrong call here is recoverable. */ write(snap: unknown): RosterWriteResult { const home = this.home(); @@ -126,9 +132,9 @@ export class RosterStore { if (!this.wrote && existing && entryCount(existing) > 0 && entryCount(snap) === 0) { // Back it up anyway: what is on disk right now is exactly what we are - // protecting, and a copy of it costs nothing. + // protecting, and a copy of it costs nothing. `wrote` stays false: the + // guard disarms only when a non-empty write lands, never on a refusal. this.backup(home, p, 'declined'); - this.wrote = true; console.warn('[roster] refused to overwrite a non-empty roster with an empty one'); return { ok: false, skipped: 'empty-first-write' }; } diff --git a/test/roster.test.cjs b/test/roster.test.cjs index b48a181f9..554193b5c 100644 --- a/test/roster.test.cjs +++ b/test/roster.test.cjs @@ -98,6 +98,27 @@ test('a refused write still backs up whatever was on disk', () => { assert.ok(backupsIn(home).some((f) => f.includes('declined'))); }); +test('a refusal does not disarm the guard: repeated empty writes stay refused', () => { + // Seen live 2026-08-16: the first empty write of a run was declined, but the + // decline set the has-written flag — so when a renderer reload re-sent the + // same empty snapshot, the second write went through and flattened the file. + // The guard must hold until a NON-empty write proves the renderer has a roster. + const home = tmpHome(); + storeAt(home).write(snapshot([{ id: 'a', name: 'Dwight' }])); + + const nextRun = storeAt(home); + assert.equal(nextRun.write(snapshot([])).skipped, 'empty-first-write'); + const second = nextRun.write(snapshot([])); + assert.equal(second.ok, false); + assert.equal(second.skipped, 'empty-first-write'); + assert.equal(nextRun.read().agents.length, 1, 'the roster on disk survived both'); + + // A non-empty write disarms it; a later empty write is then a real deletion. + assert.equal(nextRun.write(snapshot([{ id: 'a', name: 'Dwight' }])).ok, true); + assert.equal(nextRun.write(snapshot([])).ok, true); + assert.equal(nextRun.read().agents.length, 0); +}); + test('a corrupt file reads as "no opinion" rather than as an empty roster', () => { // The renderer treats null as "use localStorage". Returning an empty roster // instead would show a blank floor and then mirror that blank back to disk.