perf: checkpoint daemon PTYs only when dirty (#2593)

src/main/daemon/daemon-pty-adapter.test.ts

@@ -523,6 +523,62 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => {
       }
     })
 
+    it('does not schedule a checkpoint timer until a session is dirty', async () => {
+      const adapterClass = DaemonPtyAdapter as unknown as { CHECKPOINT_INTERVAL_MS: number }
+      const previousInterval = adapterClass.CHECKPOINT_INTERVAL_MS
+      const setTimeoutSpy = vi.spyOn(globalThis, 'setTimeout')
+      adapterClass.CHECKPOINT_INTERVAL_MS = 10_000
+
+      try {
+        historyAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir })
+        await historyAdapter.spawn({
+          cols: 80,
+          rows: 24,
+          cwd: '/home/user',
+          sessionId: 'idle-checkpoint'
+        })
+
+        expect(setTimeoutSpy.mock.calls.some(([, delay]) => delay === 10_000)).toBe(false)
+
+        lastSubprocess._simulateData('dirty after idle\r\n')
+        await waitFor(() => setTimeoutSpy.mock.calls.some(([, delay]) => delay === 10_000))
+      } finally {
+        adapterClass.CHECKPOINT_INTERVAL_MS = previousInterval
+        setTimeoutSpy.mockRestore()
+      }
+    })
+
+    it('clears a pending checkpoint timer when the last dirty session closes', async () => {
+      const adapterClass = DaemonPtyAdapter as unknown as { CHECKPOINT_INTERVAL_MS: number }
+      const previousInterval = adapterClass.CHECKPOINT_INTERVAL_MS
+      const clearTimeoutSpy = vi.spyOn(globalThis, 'clearTimeout')
+      adapterClass.CHECKPOINT_INTERVAL_MS = 10_000
+
+      try {
+        historyAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir })
+        const { id } = await historyAdapter.spawn({
+          cols: 80,
+          rows: 24,
+          cwd: '/home/user',
+          sessionId: 'close-dirty-checkpoint'
+        })
+        const internals = historyAdapter as unknown as {
+          dirtySessionVersions: Map<string, number>
+        }
+
+        lastSubprocess._simulateData('dirty before close\r\n')
+        await waitFor(() => internals.dirtySessionVersions.has(id))
+        const callsBeforeClose = clearTimeoutSpy.mock.calls.length
+
+        await historyAdapter.shutdown(id, { immediate: true })
+
+        expect(clearTimeoutSpy.mock.calls.length).toBeGreaterThan(callsBeforeClose)
+      } finally {
+        adapterClass.CHECKPOINT_INTERVAL_MS = previousInterval
+        clearTimeoutSpy.mockRestore()
+      }
+    })
+
     it('writes meta.json with endedAt on exit', async () => {
       historyAdapter = new DaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir })
 

src/main/daemon/daemon-pty-adapter.ts

@@ -66,7 +66,7 @@ export class DaemonPtyAdapter implements IPtyProvider {
   private coldRestoreCache = new Map<string, { scrollback: string; cwd: string }>()
   private activeSessionIds = new Set<string>()
   private dirtySessionVersions = new Map<string, number>()
-  private checkpointInterval: ReturnType<typeof setInterval> | null = null
+  private checkpointTimer: ReturnType<typeof setTimeout> | null = null
   private checkpointInFlight: Promise<void> | null = null
   // Why: checkpoint-based persistence requires the getSnapshot RPC (v4+).
   // Legacy daemons reject it, causing noisy log spam every 5 seconds.
@@ -246,6 +246,7 @@ export class DaemonPtyAdapter implements IPtyProvider {
     await this.client.request('kill', { sessionId: id })
     this.activeSessionIds.delete(id)
     this.dirtySessionVersions.delete(id)
+    this.stopCheckpointTimerIfIdle()
     this.initialCwds.delete(id)
     // Why: history removal is for the "user explicitly closed this terminal"
     // path. Sleep also calls shutdown but expects scrollback to survive — wake
@@ -407,6 +408,7 @@ export class DaemonPtyAdapter implements IPtyProvider {
     const ids = [...this.activeSessionIds]
     this.activeSessionIds.clear()
     this.dirtySessionVersions.clear()
+    this.stopCheckpointTimer()
     for (const id of ids) {
       // Why: listener throws are intentionally *not* caught — matches the
       // natural onExit fanout in setupEventRouting, so synthetic exits don't
@@ -462,10 +464,7 @@ export class DaemonPtyAdapter implements IPtyProvider {
   }
 
   dispose(): void {
-    if (this.checkpointInterval) {
-      clearInterval(this.checkpointInterval)
-      this.checkpointInterval = null
-    }
+    this.stopCheckpointTimer()
     this.dirtySessionVersions.clear()
     this.removeEventListener?.()
     this.removeEventListener = null
@@ -487,10 +486,7 @@ export class DaemonPtyAdapter implements IPtyProvider {
   // We write a final checkpoint before disconnecting so that if the daemon
   // later crashes while Orca is closed, checkpoint.json has recovery data.
   async disconnectOnly(): Promise<void> {
-    if (this.checkpointInterval) {
-      clearInterval(this.checkpointInterval)
-      this.checkpointInterval = null
-    }
+    this.stopCheckpointTimer()
     // Why: wait for any in-flight timer pass to finish before starting
     // the final checkpoint. Otherwise both passes race on the shared tmp
     // file, risking ENOENT on rename and disabling future writes.
@@ -512,23 +508,47 @@ export class DaemonPtyAdapter implements IPtyProvider {
   private async ensureConnected(): Promise<void> {
     await this.client.ensureConnected()
     this.setupEventRouting()
-    this.startCheckpointTimer()
+    this.scheduleCheckpointTimer()
   }
 
-  private startCheckpointTimer(): void {
-    if (this.checkpointInterval || !this.historyManager || !this.supportsCheckpoints) {
+  private stopCheckpointTimer(): void {
+    if (!this.checkpointTimer) {
       return
     }
-    this.checkpointInterval = setInterval(() => {
+    clearTimeout(this.checkpointTimer)
+    this.checkpointTimer = null
+  }
+
+  private stopCheckpointTimerIfIdle(): void {
+    if (this.dirtySessionVersions.size === 0) {
+      this.stopCheckpointTimer()
+    }
+  }
+
+  private scheduleCheckpointTimer(): void {
+    if (
+      this.checkpointTimer ||
+      !this.historyManager ||
+      !this.supportsCheckpoints ||
+      this.dirtySessionVersions.size === 0
+    ) {
+      return
+    }
+    // Why: checkpointing is only needed after terminal data/resize/write marks
+    // a session dirty. A permanent interval woke the main process every 5s for
+    // idle daemon-backed terminals just to discover there was nothing to write.
+    this.checkpointTimer = setTimeout(() => {
+      this.checkpointTimer = null
       // Why: if the previous pass is still in-flight (slow RPC or disk),
-      // skip this tick. Overlapping passes race on the shared tmp file
-      // in checkpoint(), and a lost rename triggers handleWriteError which
-      // permanently disables the session's history writes.
+      // retry later instead of overlapping checkpoint() writes to the same tmp
+      // file, which can lose a rename and disable future history writes.
       if (this.checkpointInFlight) {
+        this.scheduleCheckpointTimer()
         return
       }
       this.checkpointInFlight = this.checkpointDirtySessions().finally(() => {
         this.checkpointInFlight = null
+        this.scheduleCheckpointTimer()
       })
     }, DaemonPtyAdapter.CHECKPOINT_INTERVAL_MS)
   }
@@ -538,6 +558,7 @@ export class DaemonPtyAdapter implements IPtyProvider {
       return
     }
     this.dirtySessionVersions.set(sessionId, (this.dirtySessionVersions.get(sessionId) ?? 0) + 1)
+    this.scheduleCheckpointTimer()
   }
 
   private async checkpointDirtySessions(): Promise<void> {
@@ -552,6 +573,8 @@ export class DaemonPtyAdapter implements IPtyProvider {
       [...this.dirtySessionVersions].filter(([sessionId]) => this.activeSessionIds.has(sessionId))
     )
     if (versions.size === 0) {
+      this.dirtySessionVersions.clear()
+      this.stopCheckpointTimer()
       return
     }
     const completed = await this.checkpointSessions(versions.keys())
@@ -560,6 +583,7 @@ export class DaemonPtyAdapter implements IPtyProvider {
         this.dirtySessionVersions.delete(sessionId)
       }
     }
+    this.stopCheckpointTimerIfIdle()
   }
 
   // Why: the adapter runs in the Electron main process and does not have direct
@@ -650,6 +674,7 @@ export class DaemonPtyAdapter implements IPtyProvider {
       } else if (event.event === 'exit') {
         this.activeSessionIds.delete(event.sessionId)
         this.dirtySessionVersions.delete(event.sessionId)
+        this.stopCheckpointTimerIfIdle()
         if (this.historyManager) {
           void this.historyManager
             .closeSession(event.sessionId, event.payload.code)