From bff9989709f41539bd56e929f40f44c3c33da8cd Mon Sep 17 00:00:00 2001 From: brennanb2025 Date: Sun, 10 May 2026 21:45:32 -0700 Subject: [PATCH] fix(agent-dashboard): clean stale on-disk entries during hydrate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Defensive `lastStatusByPaneKey.clear()` at top of `hydrateLastStatusFromDisk` keeps repeat-start() calls from silently merging prior-session state. - When sanitize drops entries (drift, TTL, schema), log a single `[agent-hooks] last-status hydrate dropped N entries (kept M)` warn and synchronously rewrite the file. Pre-fix, stale entries stayed on disk until a fresh hook event triggered a debounced write — users who hadn't run an agent in 8+ days would re-drop the same entries every cold boot. - Prime `lastWrittenJson` from the raw on-disk bytes (instead of re-serializing) when hydration is lossless — robust against future shape drift in `serializeStatusFile`. - `LAST_STATUS_FILE_VERSION = 2` comment now records why v1 was skipped (in-flight branch shape). - IPC test mock uses `vi.importActual` for `isValidPaneKey` so it stays in sync with the real validator. Co-authored-by: Orca --- src/main/agent-hooks/server.ts | 38 +++++++++++++++++++++++--------- src/main/ipc/agent-hooks.test.ts | 25 ++++++++++----------- 2 files changed, 40 insertions(+), 23 deletions(-) diff --git a/src/main/agent-hooks/server.ts b/src/main/agent-hooks/server.ts index df21c063048..37f9a04b960 100644 --- a/src/main/agent-hooks/server.ts +++ b/src/main/agent-hooks/server.ts @@ -1183,9 +1183,14 @@ function getEndpointFileName(): string { // restart artifacts stay co-located. const LAST_STATUS_FILE_NAME = 'last-status.json' -// Why: bumping this rejects on-disk files written by older shapes — see the -// "Stale file from a prior Orca version" edge case in the design doc. A -// mismatched version is treated as a corrupt file (silent empty hydration). +// Why: starts at 2 (not 1) because pre-merge dev iterations of this branch +// wrote a v1 shape with no receivedAt / stateStartedAt. Bumping to 2 means a +// developer who upgrades from an in-flight branch sees an empty hydration +// once instead of partially-typed legacy entries. New file format; never +// shipped to users at v1. Bumping this rejects on-disk files written by +// older shapes — see the "Stale file from a prior Orca version" edge case +// in the design doc. A mismatched version is treated as a corrupt file +// (silent empty hydration). const LAST_STATUS_FILE_VERSION = 2 // Why: trailing-edge debounce so a burst of hook events from a multi-agent @@ -1722,6 +1727,10 @@ export class AgentHookServer { if (!this.lastStatusFilePath) { return } + // Why: defensive — keeps hydrate idempotent against repeated start() calls; + // production callers always have an empty map here, but a future re-start + // path must not silently merge prior-session state. + lastStatusByPaneKey.clear() let raw: string try { raw = readFileSync(this.lastStatusFilePath, 'utf8') @@ -1775,14 +1784,23 @@ export class AgentHookServer { dropped += 1 } } + if (dropped > 0) { + console.warn( + `[agent-hooks] last-status hydrate dropped ${dropped} entries (kept ${hydrated})` + ) + } if (hydrated > 0 && dropped === 0) { - // Why: prime lastWrittenJson so an immediate scheduleStatusPersist() - // (e.g. from a hook event that arrives before any change) does not - // re-write the file with byte-identical contents. Only prime when - // hydration was lossless — if entries were dropped during sanitize, - // the in-memory map diverges from the on-disk bytes; leaving the - // prime null forces the next write to clean up the corrupt entries. - this.lastWrittenJson = this.serializeStatusFile() + // Why: prime from the raw on-disk bytes (not a re-serialization) so the + // dedup is robust against future shape drift in serializeStatusFile. + // Only prime when hydration was lossless — if entries were dropped + // during sanitize, the in-memory map diverges from the on-disk bytes. + this.lastWrittenJson = raw + } else if (dropped > 0) { + // Why: clean the stale on-disk file now so a user who has not run an + // agent in 8+ days does not re-drop the same entries on every cold + // boot. Synchronous variant is safe at start time and avoids + // unref'd-timer-during-quit edge cases. + this.runStatusPersist() } } diff --git a/src/main/ipc/agent-hooks.test.ts b/src/main/ipc/agent-hooks.test.ts index cf7eb450607..a87ea123a37 100644 --- a/src/main/ipc/agent-hooks.test.ts +++ b/src/main/ipc/agent-hooks.test.ts @@ -1,5 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type * as AgentHookServerModule from '../agent-hooks/server' + // Why: cover the agentStatus:drop IPC handler — it must propagate the // renderer dismissal to dropStatusEntry so the on-disk last-status file // evicts the entry. @@ -24,21 +26,18 @@ vi.mock('electron', () => ({ } })) -vi.mock('../agent-hooks/server', () => ({ - agentHookServer: { - dropStatusEntry, - getStatusSnapshot - }, - // Why: matches the real isValidPaneKey shape (exactly one ':' with non-empty - // halves). The IPC handler imports this validator alongside agentHookServer. - isValidPaneKey: (value: unknown): value is string => { - if (typeof value !== 'string') { - return false +vi.mock('../agent-hooks/server', async () => { + // Why: import the real isValidPaneKey so this test stays in sync with any + // tightening of the validator (length cap, character allow-list, etc). + const actual = await vi.importActual('../agent-hooks/server') + return { + ...actual, + agentHookServer: { + dropStatusEntry, + getStatusSnapshot } - const colon = value.indexOf(':') - return colon > 0 && colon < value.length - 1 && !value.includes(':', colon + 1) } -})) +}) vi.mock('../claude/hook-service', () => ({ claudeHookService: { getStatus: vi.fn(() => ({ agent: 'claude', state: 'absent' })) }