From 2fa0fa747b262e299b7f9feb2608b7c7f965234e Mon Sep 17 00:00:00 2001 From: Willem Date: Fri, 21 Aug 2026 10:19:32 +0200 Subject: [PATCH] fix(store): do not rewrite the whole store on thread select (#636) threads:get stamped lastVisitedAt via store.save(), which armed a debounced flush of the entire ~180 MB JSON blob. Clicking through idle threads wrote gigabytes for one timestamp. Visit now calls markDirty(): the stamp stays in memory, the exit hook covers quit, and the next real flush persists it. Hard crash: unread-dot is a click stale. --- electron/services.js | 9 ++++++--- electron/store.js | 23 +++++++++++++++++------ electron/test/last-visited.test.js | 27 +++++++++++++++++++++++++++ electron/test/store.test.js | 10 ++++++++++ 4 files changed, 60 insertions(+), 9 deletions(-) diff --git a/electron/services.js b/electron/services.js index ff0cfe3d6..6b9a712db 100644 --- a/electron/services.js +++ b/electron/services.js @@ -3413,8 +3413,11 @@ function searchThreads(store, input) { * Full thread detail for the renderer. * * Selecting a thread is visiting it: when markVisited is true (default), stamp - * lastVisitedAt = Date.now() and persist WITHOUT bumping updatedAt (visiting is - * not activity; bumping would re-unread the thread and re-sort the sidebar). + * lastVisitedAt = Date.now() WITHOUT bumping updatedAt (visiting is not + * activity; bumping would re-unread the thread and re-sort the sidebar). + * Do not store.save() here: a visit stamp must not schedule a whole-store + * rewrite (#636). markDirty lets the field ride the next real flush; the + * exit hook covers quit. Hard crash: unread-dot is a click stale. * * Callers (audit before changing stamp rules): * - electron/ipc.js threads:get — user selection; markVisited true (default) @@ -3436,7 +3439,7 @@ function getThreadDetail(store, threadId, workflow = null, opts) { if (markVisited) { // No touch: visiting must not bump updatedAt. store.updateThread(threadId, { lastVisitedAt: Date.now() }); - store.save(); + store.markDirty(); } const current = store.getThread(threadId) || thread; return { diff --git a/electron/store.js b/electron/store.js index 4c064f832..e03dbb558 100644 --- a/electron/store.js +++ b/electron/store.js @@ -1122,6 +1122,22 @@ class Store { return cloneEmpty(); } + /** + * Remember in-memory mutations without scheduling a flush. The next + * save() coalesces them; the exit hook writes if we quit first. Use for + * cheap bookkeeping (lastVisitedAt) that must not rewrite the whole + * store on every call (#636). Not `touch`: that already means bump + * updatedAt on updateThread. + */ + markDirty() { + this._dirty = true; + // At most one exit hook no matter how often markDirty()/save() run. + if (!this._exitHookArmed) { + this._exitHookArmed = true; + process.once("exit", this._flushOnExit); + } + } + /** * Mark dirty and coalesce writes: the whole store is one JSON blob, so a * per-stream-event save would re-stringify everything every time. The @@ -1130,13 +1146,8 @@ class Store { * Callers that need the bytes on disk right now use saveNow(). */ save() { - this._dirty = true; + this.markDirty(); this._scheduleFlush(); - // At most one exit hook no matter how often save() is called. - if (!this._exitHookArmed) { - this._exitHookArmed = true; - process.once("exit", this._flushOnExit); - } } _scheduleFlush() { diff --git a/electron/test/last-visited.test.js b/electron/test/last-visited.test.js index cb955ccb4..1c075158f 100644 --- a/electron/test/last-visited.test.js +++ b/electron/test/last-visited.test.js @@ -72,6 +72,33 @@ describe("lastVisitedAt (round 43)", () => { assert.equal(store.getThread(thread.id).lastVisitedAt, thread.createdAt); }); + it("getThreadDetail (threads:get) does not schedule a flush (#636)", () => { + const thread = services.createThread(store, { + projectId: project.id, + title: "Idle click", + }); + store.updateThread(thread.id, { lastVisitedAt: 1 }); + store.saveNow(); + assert.equal(store._timer, null, "precondition: no flush armed"); + + const detail = services.getThreadDetail(store, thread.id); + + assert.ok( + detail.thread.lastVisitedAt > 1, + "selection still stamps lastVisitedAt in memory", + ); + assert.equal( + store._timer, + null, + "visit stamp must not arm a whole-store rewrite", + ); + assert.equal( + store._dirty, + true, + "stamp must stay dirty so the exit hook / next real flush persist it", + ); + }); + it("getThreadDetail stamps lastVisitedAt and does NOT bump updatedAt", async () => { const thread = services.createThread(store, { projectId: project.id, diff --git a/electron/test/store.test.js b/electron/test/store.test.js index 1b6a81231..38156e785 100644 --- a/electron/test/store.test.js +++ b/electron/test/store.test.js @@ -361,6 +361,16 @@ describe("Store", () => { assert.ok(SAVE_DEBOUNCE_MAX_MS >= SAVE_DEBOUNCE_MS * 2); }); + it("markDirty remembers mutations without scheduling a flush (#636)", () => { + const store = new Store(filePath); + store.setProjects([{ id: "p", slug: "a/b", name: "b", path: "/x" }]); + store.markDirty(); + assert.equal(store._dirty, true); + assert.equal(store._timer, null, "must not arm the debounce timer"); + store.saveNow(); + assert.equal(new Store(filePath).getProjects()[0].id, "p"); + }); + it("saveNow() flushes a pending debounced save immediately", () => { const store = new Store(filePath); store.setProjects([{ id: "p", slug: "a/b", name: "b", path: "/x" }]);