From 4b2389beb5ce31e47316b7fac986004addec0f23 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Thu, 24 Sep 2026 23:14:24 -0700 Subject: [PATCH 1/3] fix(storage): prune orphaned ghost logs; protect a ghost log only while its session exists Every ghost log was protected from every retention pass as a bucket, so the ghost-logs cap could never act, and startup compaction rewrites each file, so none ever aged either. Measured on the owner's machine: 1,952 ghost logs, 2.1 GB, of which 1,935 (2,128 MB) belonged to sessions no longer in any window's workspace; only 17 (49 MB) were real recovery state. A ghost log is recovery state while its session exists: in any window's committed workspace, or running in the session manager. Main now provides that owner set to retention (read once per prune). Owned logs stay fully protected however old; orphaned logs go through TTL, cap and budget like any other debug output, still behind the 10-minute active grace (so a pane closed a moment ago can be restored by undo). With no owner set yet (the first prune runs before the workspace store opens) or a failing provider, everything stays protected as before. Tests: orphans trimmed oldest-first under the cap while an older owned log stays; an orphan ages out and an owned log does not; a fresh orphan is kept inside the grace. Unprotecting all ghost logs turns two red. Fixes #732 Co-Authored-By: Claude Opus 5.5 --- src/main/index.ts | 6 +++ src/main/storage/debugRetention.test.ts | 50 +++++++++++++++++++++ src/main/storage/debugRetention.ts | 59 ++++++++++++++++++++----- 3 files changed, 104 insertions(+), 11 deletions(-) diff --git a/src/main/index.ts b/src/main/index.ts index fb1840424..dac56ff1b 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -81,6 +81,7 @@ import { scheduleDebugStoragePrune, setDebugRetentionJournal, setLiveRecordingDirsProvider, + setGhostLogOwnersProvider, } from '@main/storage/debugRetention.js' import { cleanupClaudeImageCacheDir } from '@main/storage/claudeImageCache.js' import { @@ -1389,6 +1390,11 @@ async function startApp(): Promise { // renderer, which requires a window. const workspaceFileStore = await WorkspaceFileStore.open() shutdownWorkspaceStore = workspaceFileStore + // Ghost logs stay protected from retention only while their session exists + // somewhere: in any window's committed workspace, or running in the + // manager (a brand-new pane can run before its first save). Before this + // line runs, retention treats every ghost log as protected (#732). + setGhostLogOwnersProvider(() => new Set([...workspaceFileStore.sessionIds(), ...(manager?.list() ?? [])])) assertStartupOpen() // Conversation ledger (docs/decomposition/conversations.md, Stage 3): a // projection of every window's sessions keyed by native id, so the picker diff --git a/src/main/storage/debugRetention.test.ts b/src/main/storage/debugRetention.test.ts index 0dc710f7d..49c8b52d9 100644 --- a/src/main/storage/debugRetention.test.ts +++ b/src/main/storage/debugRetention.test.ts @@ -152,6 +152,56 @@ describe('runPrunePasses', () => { expect(result).toEqual({ removed: 1, bytesFreed: 10, remainingBytes: 40 }) }) + describe('ghost logs are recovery state only while their session exists (#732)', () => { + // Measured on the owner's machine (2026-09-25): 1,952 ghost logs, 2.1 GB, + // and 1,935 of them (2,128 MB) belonged to sessions no longer in any + // window's workspace. Every one was protected from every pass, and + // startup compaction rewrites each file, so none ever aged either. + const log = (sessionId: string, bytes: number, ageMs: number) => + artifact(`/state/ghost-logs/${sessionId}.ghost.jsonl`, 'ghost-logs', bytes, ageMs) + + it('trims orphaned logs over the cap, oldest first, and never a live session\'s', async () => { + const oldOrphan = log('gone-1', 100, 5 * HOUR) + const newerOrphan = log('gone-2', 100, 3 * HOUR) + const owned = log('live-1', 100, 6 * HOUR) + const { calls, remove } = recordingRemover() + + await runPrunePasses( + [owned, newerOrphan, oldOrphan], + policy({ caps: capsOf(1_000_000, { 'ghost-logs': 150 }), ghostLogOwners: new Set(['live-1']) }), + remove, + ) + + // 300 > 150: the two orphans go, oldest first. The live session's log is + // the OLDEST file and still stays: it is what its pane rebuilds from. + expect(calls).toEqual(['/state/ghost-logs/gone-1.ghost.jsonl', '/state/ghost-logs/gone-2.ghost.jsonl']) + }) + + it('lets an orphaned log age out, but not a live session\'s', async () => { + const orphan = log('gone-1', 10, 72 * HOUR) + const owned = log('live-1', 10, 72 * HOUR) + const { calls, remove } = recordingRemover() + + await runPrunePasses([orphan, owned], policy({ ghostLogOwners: new Set(['live-1']) }), remove) + + expect(calls).toEqual(['/state/ghost-logs/gone-1.ghost.jsonl']) + }) + + it('keeps a just-written orphan inside the active grace', async () => { + // A pane closed a minute ago can still be restored by undo. + const fresh = log('gone-1', 100, 60_000) + const { calls, remove } = recordingRemover() + + await runPrunePasses( + [fresh], + policy({ caps: capsOf(1_000_000, { 'ghost-logs': 10 }), ghostLogOwners: new Set() }), + remove, + ) + + expect(calls).toEqual([]) + }) + }) + it('cap pass trims the oldest inactive, unprotected artifacts of an over-cap bucket only', async () => { const oldest = artifact('oldest', 'proxy', 60, 5 * HOUR) const older = artifact('older', 'proxy', 60, 3 * HOUR) diff --git a/src/main/storage/debugRetention.ts b/src/main/storage/debugRetention.ts index aff9c9303..a8a7d81d6 100644 --- a/src/main/storage/debugRetention.ts +++ b/src/main/storage/debugRetention.ts @@ -1,5 +1,5 @@ import { mkdir, readFile, readdir, rm, stat, statfs } from 'node:fs/promises' -import { dirname, join, resolve } from 'node:path' +import { basename, dirname, join, resolve } from 'node:path' import { AUTOSAVE_DEBUG_BUNDLE_DIR, @@ -54,6 +54,11 @@ export type DebugStoragePrunePolicy = { activeGraceMs: number budgetBytes: number caps: Record + /** Session ids whose ghost log is still recovery state (#732): every + * session in any window's workspace, plus every running one. Absent or + * null means "not known yet" (the first prune runs before the workspace + * store opens), and then every ghost log stays protected, as before. */ + ghostLogOwners?: ReadonlySet | null } export type DebugStoragePrunePassesResult = { @@ -124,6 +129,26 @@ export function setLiveRecordingDirsProvider(fn: (() => Set) | null): vo liveRecordingDirsProvider = fn } +// Provider for the sessions whose ghost logs are still recovery state (#732). +// Same singleton shape as the recording provider above, for the same reason. +// Read ONCE per prune (into the policy), not per artifact: the workspace +// store derives it from the whole file, and a prune can see ~2,000 logs. +let ghostLogOwnersProvider: (() => ReadonlySet) | null = null + +export function setGhostLogOwnersProvider(fn: (() => ReadonlySet) | null): void { + ghostLogOwnersProvider = fn +} + +/** The owner set for one prune, or null when it cannot be known. A provider + * that throws must not unprotect anything, so failure reads as "unknown". */ +function readGhostLogOwners(): ReadonlySet | null { + try { + return ghostLogOwnersProvider ? ghostLogOwnersProvider() : null + } catch { + return null + } +} + function envNumber(name: string, fallback: number): number { const raw = process.env[name]?.trim() if (!raw) return fallback @@ -254,6 +279,7 @@ export async function pruneDebugStorage(reason: string): Promise (await removeArtifact(artifact)) > 0, ) @@ -310,12 +336,13 @@ export async function runPrunePasses( } for (const artifact of artifacts) { - // Ghost logs are recovery state. Age alone is not enough evidence that a - // log is disposable: a user can resume an older crashed session and still - // need those provisional rows during bootstrap. Keep ghost cleanup on the - // budget/cap passes below, where pressure is explicit and active recent - // files still get the activeGraceMs guard. - if (isProtectedFromDebugPrune(artifact)) continue + // A ghost log is recovery state while its session exists: a user can + // resume an older crashed session and still need its provisional rows + // during bootstrap, however old the file is. Once the session is gone from + // every workspace (and not running), nothing can rebuild from it, so it is + // ordinary debug output (#732). `isProtectedFromDebugPrune` draws that + // line; active recent files still get the activeGraceMs guard below. + if (isProtectedFromDebugPrune(artifact, policy.ghostLogOwners)) continue if (artifact.mtimeMs >= cutoff) continue await drop(artifact) } @@ -334,7 +361,7 @@ export async function runPrunePasses( // silently breaking the "keep the 50 newest runs regardless" guarantee // that collectIncidentRunDirs() establishes. (Manual debug bundles get // the same protection via the `continue` at the top of this loop.) - if (isProtectedFromDebugPrune(artifact)) continue + if (isProtectedFromDebugPrune(artifact, policy.ghostLogOwners)) continue if (artifact.mtimeMs > activeCutoff) continue bucketBytes -= await drop(artifact) } @@ -343,7 +370,7 @@ export async function runPrunePasses( let totalBytes = sumBytes([...live]) for (const artifact of [...live].sort((a, b) => a.mtimeMs - b.mtimeMs)) { if (totalBytes <= policy.budgetBytes) break - if (isProtectedFromDebugPrune(artifact)) continue + if (isProtectedFromDebugPrune(artifact, policy.ghostLogOwners)) continue if (artifact.mtimeMs > activeCutoff) continue totalBytes -= await drop(artifact) } @@ -553,7 +580,7 @@ export function legacyDebugBundleBucketForPath( : 'debug-bundles-legacy' } -function isProtectedFromDebugPrune(artifact: Artifact): boolean { +function isProtectedFromDebugPrune(artifact: Artifact, ghostLogOwners?: ReadonlySet | null): boolean { // A live session recording is protected regardless of its folder mtime — see // setLiveRecordingDirsProvider for WHY mtime is the wrong liveness oracle // here (an idle-but-open recording ages past ACTIVE_GRACE_MS while still being @@ -563,8 +590,18 @@ function isProtectedFromDebugPrune(artifact: Artifact): boolean { if (artifact.bucket === 'session-recordings' && liveRecordingDirsProvider) { if (liveRecordingDirsProvider().has(resolve(artifact.path))) return true } + // WHY ghost logs are protected per SESSION, not as a bucket (#732): the + // whole bucket used to be protected from every pass, so the per-bucket cap + // above could never act, and startup compaction rewrites each file, so + // they never aged either. Measured 2026-09-25: 1,952 logs, 2.1 GB, of which + // 1,935 (2,128 MB) belonged to sessions no longer in any workspace. A log + // is kept while its session exists; with no owner set (not known yet) all + // stay protected. + if (artifact.bucket === 'ghost-logs') { + if (!ghostLogOwners) return true + return ghostLogOwners.has(basename(artifact.path).replace(/\.ghost\.jsonl$/u, '')) + } return artifact.protected === true || - artifact.bucket === 'ghost-logs' || artifact.bucket === 'debug-bundles-manual' } From 37616282d9893ee279c391b148495f9a1d10b08e Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Thu, 24 Sep 2026 23:18:06 -0700 Subject: [PATCH 2/3] fix(storage): an unreadable workspace means unknown ghost-log owners, never none Steering review (urgent, before review): WorkspaceFileStore loads an EMPTY file and goes read-only when workspace.json is unreadable, corrupt or from a newer version, precisely so the user's real data is not overwritten. Its sessionIds() is then empty although the file on disk may own every session, and the provider handed retention that empty set as 'no owners', which would delete every ghost log in recovery mode. ghostLogOwnersFrom() now returns null (unknown, so protect all) for a read-only store, and main uses it. Tests: a read-only store yields null and an old owned log survives even an over-cap prune; a healthy store's set is its sessions plus the running ones. Removing the read-only guard turns the first red. The 1,935-orphan measurement matched every session-id-shaped UUID anywhere in workspace.json (all committed windows), not one window. Co-Authored-By: Claude Opus 5.5 --- src/main/index.ts | 3 ++- src/main/storage/debugRetention.test.ts | 25 ++++++++++++++++++++++++- src/main/storage/debugRetention.ts | 24 ++++++++++++++++++++++-- 3 files changed, 48 insertions(+), 4 deletions(-) diff --git a/src/main/index.ts b/src/main/index.ts index dac56ff1b..26ddc3ee6 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -82,6 +82,7 @@ import { setDebugRetentionJournal, setLiveRecordingDirsProvider, setGhostLogOwnersProvider, + ghostLogOwnersFrom, } from '@main/storage/debugRetention.js' import { cleanupClaudeImageCacheDir } from '@main/storage/claudeImageCache.js' import { @@ -1394,7 +1395,7 @@ async function startApp(): Promise { // somewhere: in any window's committed workspace, or running in the // manager (a brand-new pane can run before its first save). Before this // line runs, retention treats every ghost log as protected (#732). - setGhostLogOwnersProvider(() => new Set([...workspaceFileStore.sessionIds(), ...(manager?.list() ?? [])])) + setGhostLogOwnersProvider(() => ghostLogOwnersFrom(workspaceFileStore, manager?.list() ?? [])) assertStartupOpen() // Conversation ledger (docs/decomposition/conversations.md, Stage 3): a // projection of every window's sessions keyed by native id, so the picker diff --git a/src/main/storage/debugRetention.test.ts b/src/main/storage/debugRetention.test.ts index 49c8b52d9..300cc5a3c 100644 --- a/src/main/storage/debugRetention.test.ts +++ b/src/main/storage/debugRetention.test.ts @@ -4,7 +4,7 @@ import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it } from 'vitest' -import { collectSessionRecordingDirs, runPrunePasses } from './debugRetention.js' +import { collectSessionRecordingDirs, ghostLogOwnersFrom, runPrunePasses } from './debugRetention.js' import type { DebugStorageArtifact, DebugStorageBucket, @@ -187,6 +187,29 @@ describe('runPrunePasses', () => { expect(calls).toEqual(['/state/ghost-logs/gone-1.ghost.jsonl']) }) + it('protects every log when the workspace could not be read', async () => { + // A read-only store loaded an EMPTY file on purpose (unreadable, + // corrupt or newer workspace.json). Its empty session set is not + // evidence that the logs are orphans; the real file may own them all. + const readOnlyStore = { isReadOnly: () => true, sessionIds: () => new Set() } + const owners = ghostLogOwnersFrom(readOnlyStore, []) + expect(owners).toBeNull() + + const old = log('owned-by-unreadable-file', 100, 72 * HOUR) + const { calls, remove } = recordingRemover() + await runPrunePasses( + [old], + policy({ caps: capsOf(1_000_000, { 'ghost-logs': 10 }), ghostLogOwners: owners }), + remove, + ) + expect(calls).toEqual([]) + }) + + it('uses a healthy store\'s sessions plus running ones', () => { + const store = { isReadOnly: () => false, sessionIds: () => new Set(['saved']) } + expect(ghostLogOwnersFrom(store, ['running'])).toEqual(new Set(['saved', 'running'])) + }) + it('keeps a just-written orphan inside the active grace', async () => { // A pane closed a minute ago can still be restored by undo. const fresh = log('gone-1', 100, 60_000) diff --git a/src/main/storage/debugRetention.ts b/src/main/storage/debugRetention.ts index a8a7d81d6..d45cec833 100644 --- a/src/main/storage/debugRetention.ts +++ b/src/main/storage/debugRetention.ts @@ -133,12 +133,32 @@ export function setLiveRecordingDirsProvider(fn: (() => Set) | null): vo // Same singleton shape as the recording provider above, for the same reason. // Read ONCE per prune (into the policy), not per artifact: the workspace // store derives it from the whole file, and a prune can see ~2,000 logs. -let ghostLogOwnersProvider: (() => ReadonlySet) | null = null +let ghostLogOwnersProvider: (() => ReadonlySet | null) | null = null -export function setGhostLogOwnersProvider(fn: (() => ReadonlySet) | null): void { +export function setGhostLogOwnersProvider(fn: (() => ReadonlySet | null) | null): void { ghostLogOwnersProvider = fn } +/** + * Who owns ghost logs right now, or null when that cannot be known. + * + * WHY a read-only store yields null, not its (empty) session set (#1223 + * steering review): `WorkspaceFileStore` deliberately loads an EMPTY file and + * goes read-only when workspace.json is unreadable, corrupt or from a newer + * version, precisely so nothing overwrites the user's real data. Its + * `sessionIds()` is then empty although the file on disk may own every + * session. Reading that as "no owners" would declare every ghost log an + * orphan and delete the recovery state of the very workspace the store is + * protecting. Unknown means protect all. + */ +export function ghostLogOwnersFrom( + store: { isReadOnly(): boolean; sessionIds(): ReadonlySet }, + runningSessionIds: readonly string[], +): ReadonlySet | null { + if (store.isReadOnly()) return null + return new Set([...store.sessionIds(), ...runningSessionIds]) +} + /** The owner set for one prune, or null when it cannot be known. A provider * that throws must not unprotect anything, so failure reads as "unknown". */ function readGhostLogOwners(): ReadonlySet | null { From 78934bf20f8bef6aa1b9796047894f21107c02fe Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Thu, 24 Sep 2026 23:35:45 -0700 Subject: [PATCH 3/3] fix(storage): ghost-log owners are unknown unless a complete workspace.json was read Review round 1 on #1223 (2 Claude + 1 Pi): - Major (B): a MISSING or empty workspace.json is a fresh install to the store, not read-only, so sessionIds() was an empty set that retention trusted, deleting every ghost log of a workspace the user had only moved aside (this machine's state dir shows the habit). The store now records whether this run read a COMPLETE file (parser completeness, as tmuxRecovery already does), and ghostLogOwnersFrom() returns unknown otherwise: missing, empty, partial or unreadable. A later save does not upgrade it. Store tests use the real sanitized v3 workspace for the trusted case and a damaged copy for the partial one. - Major (A, test gap): the provider -> policy -> prune wiring and the throw-means-unknown guard were removable with everything green. New real-file test through pruneDebugStorage: an old orphan is deleted and an old owned log kept; unknown owners and a throwing provider delete nothing. Each mutation (wiring dropped, throw read as 'no owners', completeness ignored, trust by default) now turns a test red. - Comments (A, B): the recovery-contract wording claimed ghost logs are what a restored pane rebuilds from; the review found nothing has read one on restore since 0cb71e99 (filed as #1225), and that undo respawns under a fresh id. Comments now say what the code does. Co-Authored-By: Claude Opus 5.5 --- .../debugRetention.ghostOwners.test.ts | 83 +++++++++++++++++++ src/main/storage/debugRetention.test.ts | 10 ++- src/main/storage/debugRetention.ts | 38 ++++----- src/main/storage/workspaceFileStore.test.ts | 59 +++++++++++++ src/main/storage/workspaceFileStore.ts | 22 +++++ 5 files changed, 189 insertions(+), 23 deletions(-) create mode 100644 src/main/storage/debugRetention.ghostOwners.test.ts diff --git a/src/main/storage/debugRetention.ghostOwners.test.ts b/src/main/storage/debugRetention.ghostOwners.test.ts new file mode 100644 index 000000000..48db97c5e --- /dev/null +++ b/src/main/storage/debugRetention.ghostOwners.test.ts @@ -0,0 +1,83 @@ +import { mkdirSync, mkdtempSync, rmSync, utimesSync, writeFileSync, existsSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterAll, afterEach, describe, expect, it } from 'vitest' +import { vi } from 'vitest' + +// The production path of #732's fix (#1223 review A): provider -> policy -> +// prune, over real files. Every other test calls runPrunePasses or +// ghostLogOwnersFrom directly, so dropping the wiring (or making a throwing +// provider read as "no owners") left them all green. Only the state paths are +// redirected, into a temp dir. +const root = mkdtempSync(join(tmpdir(), 'ac-ghost-owners-')) +const ghostDir = join(root, 'ghost-logs') + +vi.mock('@main/storage/paths.js', () => ({ + AUTOSAVE_DEBUG_BUNDLE_DIR: join(root, 'bundles-autosave'), + DEBUG_BUNDLE_DIR: join(root, 'bundles'), + FEED_DEBUG_DIR: join(root, 'feed-debug'), + HEAP_SNAPSHOT_DIR: join(root, 'heap'), + INCIDENT_RUNS_DIR: join(root, 'incidents'), + MANUAL_DEBUG_BUNDLE_DIR: join(root, 'bundles-manual'), + PERFORMANCE_RUNS_DIR: join(root, 'performance'), + PROXY_EVENTS_DIR: join(root, 'proxy'), + SESSION_RECORDING_DIR: join(root, 'recordings'), + STATE_DIR: root, +})) +vi.mock('@main/ghostJournal.js', () => ({ ghostLogDir: () => ghostDir })) +vi.mock('@main/storage/debugBundleLog.js', () => ({ + DEBUG_BUNDLE_LOG_FILE: join(root, 'debug-bundles.jsonl'), + isAutosaveDebugBundleReason: () => false, +})) + +const { pruneDebugStorage, setGhostLogOwnersProvider } = await import('./debugRetention.js') + +const DAY = 24 * 60 * 60 * 1000 + +function ghostLog(sessionId: string, ageMs: number): string { + mkdirSync(ghostDir, { recursive: true }) + const path = join(ghostDir, `${sessionId}.ghost.jsonl`) + writeFileSync(path, '{"uuid":"g1"}\n') + const when = (Date.now() - ageMs) / 1000 + utimesSync(path, when, when) + return path +} + +afterEach(() => { + setGhostLogOwnersProvider(null) + rmSync(ghostDir, { recursive: true, force: true }) +}) +afterAll(() => rmSync(root, { recursive: true, force: true })) + +describe('ghost-log owners through the real prune entry point (#732, #1223)', () => { + it('deletes an old orphan and keeps an old owned log', async () => { + const orphan = ghostLog('gone-1', 3 * DAY) + const owned = ghostLog('live-1', 3 * DAY) + setGhostLogOwnersProvider(() => new Set(['live-1'])) + + await pruneDebugStorage('test') + + expect(existsSync(orphan)).toBe(false) + expect(existsSync(owned)).toBe(true) + }) + + it('deletes nothing when the owners are unknown', async () => { + const orphan = ghostLog('gone-1', 3 * DAY) + setGhostLogOwnersProvider(() => null) + + await pruneDebugStorage('test') + + expect(existsSync(orphan)).toBe(true) + }) + + it('deletes nothing when the provider throws', async () => { + // A store or manager touched during teardown can throw. That must read + // as unknown, never as "no owners". + const orphan = ghostLog('gone-1', 3 * DAY) + setGhostLogOwnersProvider(() => { throw new Error('store disposed') }) + + await pruneDebugStorage('test') + + expect(existsSync(orphan)).toBe(true) + }) +}) diff --git a/src/main/storage/debugRetention.test.ts b/src/main/storage/debugRetention.test.ts index 300cc5a3c..a60503a79 100644 --- a/src/main/storage/debugRetention.test.ts +++ b/src/main/storage/debugRetention.test.ts @@ -173,7 +173,7 @@ describe('runPrunePasses', () => { ) // 300 > 150: the two orphans go, oldest first. The live session's log is - // the OLDEST file and still stays: it is what its pane rebuilds from. + // the OLDEST file and still stays, because its session still exists. expect(calls).toEqual(['/state/ghost-logs/gone-1.ghost.jsonl', '/state/ghost-logs/gone-2.ghost.jsonl']) }) @@ -191,7 +191,7 @@ describe('runPrunePasses', () => { // A read-only store loaded an EMPTY file on purpose (unreadable, // corrupt or newer workspace.json). Its empty session set is not // evidence that the logs are orphans; the real file may own them all. - const readOnlyStore = { isReadOnly: () => true, sessionIds: () => new Set() } + const readOnlyStore = { sessionOwnershipKnown: () => false, sessionIds: () => new Set() } const owners = ghostLogOwnersFrom(readOnlyStore, []) expect(owners).toBeNull() @@ -206,12 +206,14 @@ describe('runPrunePasses', () => { }) it('uses a healthy store\'s sessions plus running ones', () => { - const store = { isReadOnly: () => false, sessionIds: () => new Set(['saved']) } + const store = { sessionOwnershipKnown: () => true, sessionIds: () => new Set(['saved']) } expect(ghostLogOwnersFrom(store, ['running'])).toEqual(new Set(['saved', 'running'])) }) it('keeps a just-written orphan inside the active grace', async () => { - // A pane closed a minute ago can still be restored by undo. + // The same active grace every bucket gets: nothing written in the last + // ten minutes is pruned. (Undo Close respawns under a fresh id and does + // not read the old log; this is the general rule, not an undo contract.) const fresh = log('gone-1', 100, 60_000) const { calls, remove } = recordingRemover() diff --git a/src/main/storage/debugRetention.ts b/src/main/storage/debugRetention.ts index d45cec833..661a9c4ad 100644 --- a/src/main/storage/debugRetention.ts +++ b/src/main/storage/debugRetention.ts @@ -142,20 +142,19 @@ export function setGhostLogOwnersProvider(fn: (() => ReadonlySet | null) /** * Who owns ghost logs right now, or null when that cannot be known. * - * WHY a read-only store yields null, not its (empty) session set (#1223 - * steering review): `WorkspaceFileStore` deliberately loads an EMPTY file and - * goes read-only when workspace.json is unreadable, corrupt or from a newer - * version, precisely so nothing overwrites the user's real data. Its - * `sessionIds()` is then empty although the file on disk may own every - * session. Reading that as "no owners" would declare every ghost log an - * orphan and delete the recovery state of the very workspace the store is - * protecting. Unknown means protect all. + * WHY "unknown" unless the store read a complete workspace.json (#1223 + * reviews): the owner set is the only thing standing between a ghost log and + * deletion, so an EMPTY set must mean "no sessions", never "could not tell". + * The store reports unknown for a read-only file (unreadable, corrupt, newer) + * and also for a missing, empty or partially decoded one: a user who moves + * workspace.json aside and relaunches would otherwise lose every ghost log of + * the workspace they put back. Unknown means protect all. */ export function ghostLogOwnersFrom( - store: { isReadOnly(): boolean; sessionIds(): ReadonlySet }, + store: { sessionOwnershipKnown(): boolean; sessionIds(): ReadonlySet }, runningSessionIds: readonly string[], ): ReadonlySet | null { - if (store.isReadOnly()) return null + if (!store.sessionOwnershipKnown()) return null return new Set([...store.sessionIds(), ...runningSessionIds]) } @@ -356,12 +355,14 @@ export async function runPrunePasses( } for (const artifact of artifacts) { - // A ghost log is recovery state while its session exists: a user can - // resume an older crashed session and still need its provisional rows - // during bootstrap, however old the file is. Once the session is gone from - // every workspace (and not running), nothing can rebuild from it, so it is - // ordinary debug output (#732). `isProtectedFromDebugPrune` draws that - // line; active recent files still get the activeGraceMs guard below. + // A ghost log is kept while its session exists. The designed use is the + // crash-resume bootstrap (docs/design/ghost-system.md, "Crash + restart"): + // a restored pane rebuilding its provisional rows under its persisted id, + // however old the file. NOTE: that reader is not wired today. Only a fresh + // spawn reads a log, under an id minted a moment earlier (#1223 review A; + // #1225). Keeping owned logs costs little and is right the day it is. Once + // the session is gone from every workspace and not running, nothing can + // use the log, so it is ordinary debug output (#732). if (isProtectedFromDebugPrune(artifact, policy.ghostLogOwners)) continue if (artifact.mtimeMs >= cutoff) continue await drop(artifact) @@ -611,9 +612,8 @@ function isProtectedFromDebugPrune(artifact: Artifact, ghostLogOwners?: Readonly if (liveRecordingDirsProvider().has(resolve(artifact.path))) return true } // WHY ghost logs are protected per SESSION, not as a bucket (#732): the - // whole bucket used to be protected from every pass, so the per-bucket cap - // above could never act, and startup compaction rewrites each file, so - // they never aged either. Measured 2026-09-25: 1,952 logs, 2.1 GB, of which + // whole bucket used to be protected from every pass, so neither the TTL nor + // the per-bucket cap above could ever act on it. Measured 2026-09-25: 1,952 logs, 2.1 GB, of which // 1,935 (2,128 MB) belonged to sessions no longer in any workspace. A log // is kept while its session exists; with no owner set (not known yet) all // stay protected. diff --git a/src/main/storage/workspaceFileStore.test.ts b/src/main/storage/workspaceFileStore.test.ts index 652c60183..f5ac6da94 100644 --- a/src/main/storage/workspaceFileStore.test.ts +++ b/src/main/storage/workspaceFileStore.test.ts @@ -1,3 +1,4 @@ +import { readFileSync as readRealFile } from 'node:fs' import { beforeEach, describe, expect, it, vi } from 'vitest' const { mkdir, readFile, writeFile, rename, unlink } = vi.hoisted(() => ({ @@ -292,3 +293,61 @@ describe('refusing to write over a file it could not read', () => { expect(writeFile).not.toHaveBeenCalled() }) }) + +describe('whether the session list can be trusted (#1223)', () => { + // Ghost-log retention deletes every log whose session is not in + // sessionIds(). That is only safe when this run read the WHOLE workspace + // file. The real, sanitized v3 workspace from 2026-09-20 is the "known" + // case; every other startup shape must say unknown. + const recorded = readRealFile( + new URL('../../../testing/fixtures/workspace-v3/2026-09-20-live-workspace.sanitized.json', import.meta.url), + 'utf8', + ) + + beforeEach(() => { + mkdir.mockReset().mockResolvedValue(undefined) + writeFile.mockReset().mockResolvedValue(undefined) + rename.mockReset().mockResolvedValue(undefined) + unlink.mockReset().mockResolvedValue(undefined) + }) + + it('trusts a complete workspace read from disk', async () => { + readFile.mockReset().mockResolvedValue(recorded) + const store = await WorkspaceFileStore.open() + expect(store.sessionOwnershipKnown()).toBe(true) + expect(store.sessionIds().size).toBeGreaterThan(0) + }) + + it('does not trust a missing file, which may have been moved aside', async () => { + readFile.mockReset().mockRejectedValue(enoent()) + const store = await WorkspaceFileStore.open() + expect(store.sessionOwnershipKnown()).toBe(false) + // Nor does saving a fresh workspace make it trustworthy: the moved-aside + // file's sessions are still not in it. + await store.saveSlice('w1', slice(['fresh']), NO_GEOMETRY) + expect(store.sessionOwnershipKnown()).toBe(false) + }) + + it('does not trust an empty file', async () => { + readFile.mockReset().mockResolvedValue(' \n') + const store = await WorkspaceFileStore.open() + expect(store.sessionOwnershipKnown()).toBe(false) + }) + + it('does not trust a partially decoded file', async () => { + // The recorded workspace with one window made unreadable: the decoder + // discards that window, so its sessions are missing from sessionIds(). + const damaged = JSON.parse(recorded) as { windows: unknown[] } + damaged.windows = [...damaged.windows, 'not-a-window'] + readFile.mockReset().mockResolvedValue(JSON.stringify(damaged)) + const store = await WorkspaceFileStore.open() + expect(store.sessionOwnershipKnown()).toBe(false) + }) + + it('does not trust a file it could not read', async () => { + readFile.mockReset().mockResolvedValue('{ not json') + const store = await WorkspaceFileStore.open() + expect(store.isReadOnly()).toBe(true) + expect(store.sessionOwnershipKnown()).toBe(false) + }) +}) diff --git a/src/main/storage/workspaceFileStore.ts b/src/main/storage/workspaceFileStore.ts index 8c6ff3bef..e2e8bf1e8 100644 --- a/src/main/storage/workspaceFileStore.ts +++ b/src/main/storage/workspaceFileStore.ts @@ -54,6 +54,20 @@ export class WorkspaceFileStore { * path, so the user is not silently working in a workspace that cannot save. */ private readOnlyReason: string | null = null + /** + * Did this run load a COMPLETE workspace.json from disk (#1223)? + * + * `sessionIds()` is the only record of which sessions exist, and ghost-log + * retention deletes what it does not list. So "which sessions exist" may be + * trusted only when the file was actually read in full at startup. A missing + * file (fresh install, OR a user who moved it aside to troubleshoot, which + * this machine's state dir shows is a habit), an empty file, a partial + * decode (discarded windows, invalid container) or an unreadable one all + * mean the real owners are unknown. A save later in the run does not upgrade + * this: the moved-aside file's sessions would still be missing from it. + * tmuxRecovery draws the same line for the same reason. + */ + private sessionOwnershipComplete = false /** * The exact bytes of an older-version file this store loaded, held until @@ -151,6 +165,7 @@ export class WorkspaceFileStore { return } this.file = parsed.file + this.sessionOwnershipComplete = parsed.completeness.kind === 'complete' if (parsed.sourceVersion < WORKSPACE_FILE_VERSION) this.preUpgradeOriginal = { text, fromVersion: parsed.sourceVersion } if (parsed.migratedFromV1) { // eslint-disable-next-line no-console @@ -182,6 +197,13 @@ export class WorkspaceFileStore { return this.readOnlyReason } + /** Whether `sessionIds()` is known to list every session the user has (see + * `sessionOwnershipComplete`). False means: do not treat absence from it + * as proof that a session is gone. */ + sessionOwnershipKnown(): boolean { + return this.readOnlyReason === null && this.sessionOwnershipComplete + } + /** Union of every window's committed session ids. */ sessionIds(): Set { return collectSessionIds(this.file)