diff --git a/src/main/fish-history-session.test.ts b/src/main/fish-history-session.test.ts index d1a8a7a632c..060c1f2e57b 100644 --- a/src/main/fish-history-session.test.ts +++ b/src/main/fish-history-session.test.ts @@ -6,6 +6,7 @@ import { deleteFishHistoryFile, fishHistorySessionName, isSafeFishHistorySession, + relayFishHistorySessionName, resolveFishHistoryDir, sweepOrphanedFishHistoryFiles } from './fish-history-session' @@ -195,3 +196,43 @@ describe('orphaned fish history sweep', () => { expect(sweepOrphanedFishHistoryFiles(new Set([HASH_A]), [fishDir, other, fishDir])).toBe(2) }) }) + +/** + * A relay host keyed by its CLIENT's worktree ids shares one fish data dir with + * any desktop Orca on the same machine, whose live set knows nothing of those + * ids. The name is the only thing that keeps that sweep off remote history. + */ +describe('relay fish history naming', () => { + let relayRoot: string + + beforeEach(() => { + relayRoot = mkdtempSync(join(tmpdir(), 'orca-fish-relay-')) + }) + + afterEach(() => { + rmSync(relayRoot, { recursive: true, force: true }) + }) + + it('is namespaced apart from desktop session names', () => { + expect(relayFishHistorySessionName(HASH_A)).not.toBe(fishHistorySessionName(HASH_A)) + }) + + it('is still a safe session name, so the relay can delete it by name', () => { + expect(isSafeFishHistorySession(relayFishHistorySessionName(HASH_A))).toBe(true) + }) + + it('survives a desktop sweep that knows none of the relay worktree ids', () => { + const dir = relayRoot + const relayFile = join(dir, `${relayFishHistorySessionName(HASH_A)}_history`) + const desktopFile = join(dir, `${fishHistorySessionName(HASH_B)}_history`) + writeFileSync(relayFile, 'relay') + writeFileSync(desktopFile, 'desktop') + + // A live set with neither hash in it: everything attributable is orphaned. + const removed = sweepOrphanedFishHistoryFiles(new Set(['deadbeef']), [dir]) + + expect(removed).toBe(1) + expect(existsSync(desktopFile)).toBe(false) + expect(existsSync(relayFile)).toBe(true) + }) +}) diff --git a/src/main/fish-history-session.ts b/src/main/fish-history-session.ts index 7ab56a59490..6e76f64dd33 100644 --- a/src/main/fish-history-session.ts +++ b/src/main/fish-history-session.ts @@ -16,7 +16,13 @@ import { isAbsolute, join } from 'node:path' * tool's data for the session, not just fish's history. */ const SESSION_PREFIX = 'orca_' -const SAFE_SESSION_NAME = /^orca_[0-9a-f]{1,64}$/ +const RELAY_SESSION_PREFIX = 'orca_relay_' +const SAFE_SESSION_NAME = /^orca_(?:relay_)?[0-9a-f]{1,64}$/ +/** Deliberately does NOT match the relay prefix. A relay host keyed by its + * CLIENT's worktree ids shares the one fish data dir with any desktop Orca on + * the same machine, whose live set knows nothing of those ids — so an + * attributable name is the only thing keeping that sweep off remote history. + * Relay files are removed by exact name when their worktree goes away. */ const ORCA_HISTORY_FILE = /^orca_([0-9a-f]{1,64})_history$/ export function isSafeFishHistorySession(session: unknown): session is string { @@ -27,6 +33,11 @@ export function fishHistorySessionName(worktreeHash: string): string { return `${SESSION_PREFIX}${worktreeHash}` } +/** Relay-owned counterpart, namespaced out of the desktop sweep's reach. */ +export function relayFishHistorySessionName(worktreeHash: string): string { + return `${RELAY_SESSION_PREFIX}${worktreeHash}` +} + /** The directory fish will write history into, for a given environment. * Resolve this from the PTY's SPAWN env: Orca can be launched with a different * `XDG_DATA_HOME`/`HOME` than the shells it spawns, and fish follows its own. */ diff --git a/src/main/terminal-history-gc.ts b/src/main/terminal-history-gc.ts index 2f3fdabaff1..d67e3a7dd5c 100644 --- a/src/main/terminal-history-gc.ts +++ b/src/main/terminal-history-gc.ts @@ -116,7 +116,20 @@ function gcScanRoot( export function runHistoryGc(liveWorktreeIds: Set): void { try { // Why: finish tombstones left by quit mid-rm before scanning live worktree hashes. + // Safe ahead of the guard below: these entries were already condemned by a + // completed GC, and leaving them renamed-but-present strands disk forever. schedulePendingHistoryTreeRemovals(getHistoryRoot()) + // Why refuse rather than treat every entry as orphaned: an empty live set is + // what a store that fell back to default state looks like, and it cannot be + // told apart from a user who genuinely has no worktrees — who also has no + // history to collect. So refusing costs nothing, and it is the difference + // between a recoverable bad load and every worktree's shell history being + // deleted. `sweepOrphanedFishHistoryFiles` refuses it for the same reason; + // this is the path that deletes more. + if (liveWorktreeIds.size === 0) { + console.log('[pty:history:gc] Skipped: live worktree set is empty') + return + } const main = gcScanRoot(getHistoryRoot(), liveWorktreeIds) // Also scan WSL history directories (each distro has its own subdirectory). diff --git a/src/main/terminal-history.test.ts b/src/main/terminal-history.test.ts index 535557af826..9b9a3a78ea8 100644 --- a/src/main/terminal-history.test.ts +++ b/src/main/terminal-history.test.ts @@ -645,6 +645,36 @@ describe('terminal-history', () => { ) }) + // Why: an empty live set is what a store that fell back to default state + // looks like, and it is indistinguishable from a user with no worktrees — + // who has no history to collect either. Treating it as "everything is + // orphaned" turns a recoverable bad load into deleted shell history. + it('refuses to prune anything when the live set is empty', () => { + existsSyncMock.mockImplementation((p: string) => !p.includes('terminal-history-wsl')) + readdirSyncMock.mockImplementation((dir: string) => { + if (dir.endsWith('.pending-delete')) { + return [] + } + if (dir.endsWith('terminal-history')) { + return ['dir1', 'dir2'] + } + return ['meta.json'] + }) + statSyncMock.mockReturnValue({ isDirectory: () => true, size: 100 }) + readFileSyncMock.mockReturnValue( + JSON.stringify({ + worktreeId: 'some-wt', + createdAt: new Date(Date.now() - 10 * 60 * 1000).toISOString() + }) + ) + + runHistoryGc(new Set()) + + expect(renameSyncMock).not.toHaveBeenCalled() + expect(rmSyncMock).not.toHaveBeenCalled() + expect(rmAsyncMock).not.toHaveBeenCalled() + }) + it('continues GC after one orphan tombstone fails', async () => { existsSyncMock.mockImplementation((path: string) => !path.includes('terminal-history-wsl')) readdirSyncMock.mockImplementation((dir: string) => { @@ -667,7 +697,7 @@ describe('terminal-history', () => { throw new Error('busy') }) - expect(() => runHistoryGc(new Set())).not.toThrow() + expect(() => runHistoryGc(new Set(['live-wt']))).not.toThrow() expect(renameSyncMock).toHaveBeenCalledTimes(2) expect(rmAsyncMock).toHaveBeenCalledTimes(1) await flushPendingWorktreeHistoryDeletions() @@ -692,7 +722,7 @@ describe('terminal-history', () => { JSON.stringify({ worktreeId: 'unknown-wt', createdAt: new Date().toISOString() }) ) - runHistoryGc(new Set()) + runHistoryGc(new Set(['live-wt'])) // Should NOT prune because the directory is too young expect(rmSyncMock).not.toHaveBeenCalled() @@ -701,7 +731,7 @@ describe('terminal-history', () => { it('does not throw when history root does not exist', () => { existsSyncMock.mockReturnValue(false) - expect(() => runHistoryGc(new Set())).not.toThrow() + expect(() => runHistoryGc(new Set(['live-wt']))).not.toThrow() expect(readdirSyncMock).not.toHaveBeenCalledWith('/fake/userData/terminal-history') }) @@ -722,7 +752,7 @@ describe('terminal-history', () => { tombstonePresent = false }) - runHistoryGc(new Set()) + runHistoryGc(new Set(['live-wt'])) // The tombstone queue is drained off-thread; GC must never rmSync it or count it as a worktree. expect(rmSyncMock).not.toHaveBeenCalled() diff --git a/src/main/window/history-gc-profile-worktree-ids.test.ts b/src/main/window/history-gc-profile-worktree-ids.test.ts new file mode 100644 index 00000000000..b3f0960eab9 --- /dev/null +++ b/src/main/window/history-gc-profile-worktree-ids.test.ts @@ -0,0 +1,117 @@ +/** + * Terminal history is keyed by worktree id under a root with no profile + * segment, while the Store the GC consults holds one profile's ids. Without + * these, switching profiles makes every other profile's history look orphaned. + */ +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import { folderWorkspaceKey } from '../../shared/workspace-scope' +import { getOtherProfileWorktreeIdsForHistoryGc } from './history-gc-profile-worktree-ids' + +const roots: string[] = [] + +afterEach(() => { + for (const root of roots.splice(0)) { + rmSync(root, { recursive: true, force: true }) + } +}) + +function userDataWithProfiles( + activeProfileId: string, + profiles: { id: string; state?: unknown; raw?: string }[] +): string { + const root = mkdtempSync(join(tmpdir(), 'orca-gc-profiles-')) + roots.push(root) + mkdirSync(join(root, 'profiles'), { recursive: true }) + writeFileSync( + join(root, 'orca-profile-index.json'), + JSON.stringify({ + activeProfileId, + profiles: profiles.map(({ id }) => ({ + id, + name: id, + kind: 'local', + createdAt: 0, + updatedAt: 0, + lastOpenedAt: 0, + avatar: { kind: 'initials', initials: id.slice(0, 2), color: 'neutral' } + })) + }) + ) + for (const profile of profiles) { + mkdirSync(join(root, 'profiles', profile.id), { recursive: true }) + if (profile.raw !== undefined) { + writeFileSync(join(root, 'profiles', profile.id, 'orca-data.json'), profile.raw) + } else if (profile.state !== undefined) { + writeFileSync( + join(root, 'profiles', profile.id, 'orca-data.json'), + JSON.stringify(profile.state) + ) + } + } + return root +} + +describe('getOtherProfileWorktreeIdsForHistoryGc', () => { + it('collects worktrees and folder workspaces from the inactive profiles', () => { + const root = userDataWithProfiles('active', [ + { id: 'active', state: { worktreeMeta: { 'repo::/active': {} }, folderWorkspaces: [] } }, + { + id: 'other', + state: { + worktreeMeta: { 'repo::/other': {}, 'repo::/other-2': {} }, + folderWorkspaces: [{ id: 'fw-other' }] + } + } + ]) + + const result = getOtherProfileWorktreeIdsForHistoryGc(root) + + expect(result.unreadableProfiles).toBe(0) + expect(result.ids).toEqual( + new Set(['repo::/other', 'repo::/other-2', folderWorkspaceKey('fw-other')]) + ) + }) + + // The active profile's ids come from the live Store, which is authoritative; + // re-reading its file would only race a write in progress. + it('skips the active profile', () => { + const root = userDataWithProfiles('active', [ + { id: 'active', state: { worktreeMeta: { 'repo::/active': {} } } } + ]) + + expect(getOtherProfileWorktreeIdsForHistoryGc(root).ids.size).toBe(0) + }) + + it('reports a corrupt profile data file as unreadable rather than as no worktrees', () => { + const root = userDataWithProfiles('active', [ + { id: 'active', state: {} }, + { id: 'other', raw: '{ this is not json' } + ]) + + const result = getOtherProfileWorktreeIdsForHistoryGc(root) + + expect(result.unreadableProfiles).toBe(1) + expect(result.ids.size).toBe(0) + }) + + it('reports a missing profile data file as unreadable', () => { + const root = userDataWithProfiles('active', [{ id: 'active', state: {} }, { id: 'other' }]) + + expect(getOtherProfileWorktreeIdsForHistoryGc(root).unreadableProfiles).toBe(1) + }) + + // A single-profile install must not pay for this, and no index at all is the + // pre-profiles layout rather than an error. + it('is empty and complete when there is no profile index', () => { + const root = mkdtempSync(join(tmpdir(), 'orca-gc-profiles-')) + roots.push(root) + + expect(getOtherProfileWorktreeIdsForHistoryGc(root)).toEqual({ + ids: new Set(), + unreadableProfiles: 0 + }) + }) +}) diff --git a/src/main/window/history-gc-profile-worktree-ids.ts b/src/main/window/history-gc-profile-worktree-ids.ts new file mode 100644 index 00000000000..6f82b96fc37 --- /dev/null +++ b/src/main/window/history-gc-profile-worktree-ids.ts @@ -0,0 +1,80 @@ +import { readFileSync } from 'node:fs' +import { folderWorkspaceKey } from '../../shared/workspace-scope' +import { + getOrcaProfileDataFile, + getProfileUserDataPath +} from '../orca-profiles/profile-storage-paths' +import { getOrcaProfileIndexPath, readProfileIndex } from '../orca-profiles/profile-index-store' + +/** + * Worktree ids owned by Orca profiles OTHER than the running one. + * + * Why the history GC needs these: terminal history is keyed by worktree id + * under `userData/terminal-history`, which has no profile segment, and fish + * history lands in the user's own fish data dir — but the Store the GC asks for + * live ids only ever reads the ACTIVE profile's data file. So after a profile + * switch every other profile's history looks orphaned, and the GC deletes shell + * history those profiles are still using. + * + * Reading their data files directly is deliberate: a Store per profile would + * run migrations and normalization against state another profile owns. Only the + * two id-bearing collections are read, and any unreadable profile is skipped — + * a profile whose ids cannot be established must widen the live set's + * uncertainty, never narrow it, so failure here is handled by the caller + * refusing to prune rather than by pruning more. + */ +export function getOtherProfileWorktreeIdsForHistoryGc(userDataPath = getProfileUserDataPath()): { + ids: Set + unreadableProfiles: number +} { + const ids = new Set() + const index = readProfileIndex(getOrcaProfileIndexPath(userDataPath)) + if (!index) { + return { ids, unreadableProfiles: 0 } + } + let unreadableProfiles = 0 + for (const profile of index.profiles) { + if (profile.id === index.activeProfileId) { + continue + } + const collected = readProfileWorktreeIds(getOrcaProfileDataFile(profile.id, userDataPath)) + if (!collected) { + unreadableProfiles += 1 + continue + } + for (const id of collected) { + ids.add(id) + } + } + return { ids, unreadableProfiles } +} + +function readProfileWorktreeIds(dataFile: string): Set | null { + let parsed: unknown + try { + parsed = JSON.parse(readFileSync(dataFile, 'utf8')) + } catch { + // Missing is indistinguishable from corrupt here, and both mean the same + // thing to the caller: this profile's ids are unknown. + return null + } + if (!parsed || typeof parsed !== 'object') { + return null + } + const state = parsed as { worktreeMeta?: unknown; folderWorkspaces?: unknown } + const ids = new Set() + if (state.worktreeMeta && typeof state.worktreeMeta === 'object') { + for (const id of Object.keys(state.worktreeMeta)) { + ids.add(id) + } + } + if (Array.isArray(state.folderWorkspaces)) { + for (const workspace of state.folderWorkspaces) { + const id = (workspace as { id?: unknown } | null)?.id + if (typeof id === 'string' && id) { + ids.add(folderWorkspaceKey(id)) + } + } + } + return ids +} diff --git a/src/main/window/history-gc-worktree-ids.test.ts b/src/main/window/history-gc-worktree-ids.test.ts index 2cd7ad1c425..b61d2aaca3f 100644 --- a/src/main/window/history-gc-worktree-ids.test.ts +++ b/src/main/window/history-gc-worktree-ids.test.ts @@ -7,6 +7,8 @@ import { describe, expect, it } from 'vitest' import { folderWorkspaceKey } from '../../shared/workspace-scope' import { getKnownWorktreeIdsForHistoryGc } from './history-gc-worktree-ids' +const noOtherProfiles = () => ({ ids: new Set(), unreadableProfiles: 0 }) + const store = (worktreeIds: string[], folderIds: string[]) => ({ getAllWorktreeMeta: () => Object.fromEntries(worktreeIds.map((id) => [id, {}])), @@ -15,9 +17,9 @@ const store = (worktreeIds: string[], folderIds: string[]) => describe('getKnownWorktreeIdsForHistoryGc', () => { it('includes git worktrees', () => { - expect(getKnownWorktreeIdsForHistoryGc(store(['repo-1::/path/wt'], []))).toEqual( - new Set(['repo-1::/path/wt']) - ) + expect( + getKnownWorktreeIdsForHistoryGc(store(['repo-1::/path/wt'], []), noOtherProfiles) + ).toEqual(new Set(['repo-1::/path/wt'])) }) // Why: a folder workspace's PTY carries `folder:` as its worktree id, so @@ -25,7 +27,10 @@ describe('getKnownWorktreeIdsForHistoryGc', () => { // collection, so a set built only from worktree metadata makes every live // folder workspace look orphaned and its history gets swept on a later start. it('includes folder workspaces under their PTY workspace key', () => { - const live = getKnownWorktreeIdsForHistoryGc(store(['repo-1::/path/wt'], ['fw-1', 'fw-2'])) + const live = getKnownWorktreeIdsForHistoryGc( + store(['repo-1::/path/wt'], ['fw-1', 'fw-2']), + noOtherProfiles + ) expect(live.has(folderWorkspaceKey('fw-1'))).toBe(true) expect(live.has(folderWorkspaceKey('fw-2'))).toBe(true) @@ -33,6 +38,33 @@ describe('getKnownWorktreeIdsForHistoryGc', () => { }) it('is empty only when there is genuinely nothing live', () => { - expect(getKnownWorktreeIdsForHistoryGc(store([], [])).size).toBe(0) + expect(getKnownWorktreeIdsForHistoryGc(store([], []), noOtherProfiles).size).toBe(0) + }) + + // Why: the history root has no profile segment but the store does, so the + // other profiles' worktrees own history this set would otherwise condemn the + // first time the user switches profiles. + it('includes worktrees owned by other profiles', () => { + const live = getKnownWorktreeIdsForHistoryGc(store(['repo-1::/b'], ['fw-b']), () => ({ + ids: new Set(['repo-1::/a', folderWorkspaceKey('fw-a')]), + unreadableProfiles: 0 + })) + + expect(live).toEqual( + new Set(['repo-1::/b', folderWorkspaceKey('fw-b'), 'repo-1::/a', folderWorkspaceKey('fw-a')]) + ) + }) + + // Why empty and not "just the ids we did read": an unreadable profile's + // worktrees are indistinguishable from deleted ones, and runHistoryGc refuses + // to prune on an empty set — so this reports "do not collect" rather than + // handing back a set that condemns real history. + it('reports nothing live when a profile could not be read', () => { + const live = getKnownWorktreeIdsForHistoryGc(store(['repo-1::/b'], ['fw-b']), () => ({ + ids: new Set(['repo-1::/a']), + unreadableProfiles: 1 + })) + + expect(live.size).toBe(0) }) }) diff --git a/src/main/window/history-gc-worktree-ids.ts b/src/main/window/history-gc-worktree-ids.ts index 33be4e4bb93..df614967192 100644 --- a/src/main/window/history-gc-worktree-ids.ts +++ b/src/main/window/history-gc-worktree-ids.ts @@ -1,5 +1,6 @@ import type { Store } from '../persistence' import { folderWorkspaceKey } from '../../shared/workspace-scope' +import { getOtherProfileWorktreeIdsForHistoryGc } from './history-gc-profile-worktree-ids' /** * Every workspace key that owns shell history, for the history GC's live set. @@ -12,11 +13,27 @@ import { folderWorkspaceKey } from '../../shared/workspace-scope' * and the GC then deletes history the user is still accumulating. */ export function getKnownWorktreeIdsForHistoryGc( - store: Pick + store: Pick, + readOtherProfiles = getOtherProfileWorktreeIdsForHistoryGc ): Set { const live = new Set(Object.keys(store.getAllWorktreeMeta())) for (const workspace of store.getFolderWorkspaces()) { live.add(folderWorkspaceKey(workspace.id)) } + // Why the other profiles too: the history root is not profile-scoped but this + // store is, so on its own the live set condemns every other profile's history + // the moment the user switches. An unreadable profile means those ids are + // unknown, and an incomplete live set is exactly what deletes real history — + // so report the empty set, which runHistoryGc treats as "prune nothing". + const others = readOtherProfiles() + if (others.unreadableProfiles > 0) { + console.warn( + `[pty:history:gc] Skipping GC: ${others.unreadableProfiles} profile(s) could not be read` + ) + return new Set() + } + for (const id of others.ids) { + live.add(id) + } return live } diff --git a/src/relay/terminal-history.ts b/src/relay/terminal-history.ts index 68c86737b41..37335644df1 100644 --- a/src/relay/terminal-history.ts +++ b/src/relay/terminal-history.ts @@ -12,7 +12,7 @@ import { basename, join } from 'node:path' import { hashWorktreeId } from '../main/terminal-history-id' import { deleteFishHistoryFile, - fishHistorySessionName, + relayFishHistorySessionName, resolveFishHistoryDir } from '../main/fish-history-session' @@ -124,11 +124,11 @@ export function injectRelayFishHistoryEnv(env: Record, worktreeI if (env.fish_history) { return } - env.fish_history = fishHistorySessionName(hashWorktreeId(worktreeId)) + env.fish_history = relayFishHistorySessionName(hashWorktreeId(worktreeId)) } export function deleteRelayFishHistory(worktreeId: string, env?: NodeJS.ProcessEnv): void { - deleteFishHistoryFile(fishHistorySessionName(hashWorktreeId(worktreeId)), [ + deleteFishHistoryFile(relayFishHistorySessionName(hashWorktreeId(worktreeId)), [ resolveFishHistoryDir(env) ]) }