diff --git a/src/main/fish-history-location-attestation.ts b/src/main/fish-history-location-attestation.ts new file mode 100644 index 00000000000..1c050e49b60 --- /dev/null +++ b/src/main/fish-history-location-attestation.ts @@ -0,0 +1,282 @@ +import { + closeSync, + constants as fsConstants, + existsSync, + fstatSync, + ftruncateSync, + lstatSync, + mkdirSync, + openSync, + readFileSync, + writeFileSync +} from 'node:fs' +import { basename, dirname, isAbsolute, join, parse, relative, resolve, sep } from 'node:path' + +export const MAX_RETAINED_FISH_HISTORY_PATHS = 16 +export const MAX_FISH_HISTORY_META_BYTES = 32 * 1024 +const MAX_PATH_CANDIDATES = MAX_RETAINED_FISH_HISTORY_PATHS * 4 + +type DirectoryIdentity = { + directoryDevice: string + directoryInode: string + directoryBirthtimeNs: string +} +type FileIdentity = { fileDevice: string; fileInode: string; fileBirthtimeNs: string } +type Location = DirectoryIdentity & FileIdentity & { path: string } + +const safeSession = (session: unknown): session is string => + typeof session === 'string' && /^orca_[0-9a-f]{1,64}$/.test(session) +const canonicalPath = (session: string, path: unknown): path is string => + typeof path === 'string' && + Boolean(path) && + safeSession(session) && + isAbsolute(path) && + resolve(path) === path && + basename(dirname(path)) === 'fish' && + basename(path) === `${session}_history` + +function directoryIdentity(path: string): DirectoryIdentity | null { + const directory = dirname(path) + const root = parse(directory).root + let current = root + try { + const segments = relative(root, directory).split(sep).filter(Boolean) + const candidates = + segments.length === 0 ? [root] : segments.map((segment) => (current = join(current, segment))) + let identity: DirectoryIdentity | null = null + for (const candidate of candidates) { + const stat = lstatSync(candidate, { bigint: true }) + if (stat.isSymbolicLink() || !stat.isDirectory()) { + return null + } + identity = { + directoryDevice: stat.dev.toString(), + directoryInode: stat.ino.toString(), + directoryBirthtimeNs: stat.birthtimeNs.toString() + } + } + return identity + } catch { + return null + } +} +function fileIdentity(fd: number): FileIdentity | null { + const stat = fstatSync(fd, { bigint: true }) + return stat.isFile() + ? { + fileDevice: stat.dev.toString(), + fileInode: stat.ino.toString(), + fileBirthtimeNs: stat.birthtimeNs.toString() + } + : null +} +function sameFile(actual: FileIdentity, expected: FileIdentity): boolean { + return ( + actual.fileDevice === expected.fileDevice && + actual.fileInode === expected.fileInode && + actual.fileBirthtimeNs === expected.fileBirthtimeNs + ) +} +function sameDirectory( + stat: { isDirectory(): boolean; dev: bigint; ino: bigint; birthtimeNs: bigint }, + expected: DirectoryIdentity +): boolean { + return ( + stat.isDirectory() && + stat.dev.toString() === expected.directoryDevice && + stat.ino.toString() === expected.directoryInode && + stat.birthtimeNs.toString() === expected.directoryBirthtimeNs + ) +} + +function clearFile( + path: string, + directoryExpected: DirectoryIdentity, + fileExpected: FileIdentity +): boolean { + let directoryFd: number | undefined + let fileFd: number | undefined + try { + const flags = + fsConstants.O_RDONLY | + (process.platform === 'win32' ? 0 : fsConstants.O_DIRECTORY | fsConstants.O_NOFOLLOW) + directoryFd = openSync(dirname(path), flags) + if (!sameDirectory(fstatSync(directoryFd, { bigint: true }), directoryExpected)) { + return false + } + const filePath = + process.platform === 'win32' + ? path + : join( + process.platform === 'linux' ? '/proc/self/fd' : '/dev/fd', + String(directoryFd), + basename(path) + ) + fileFd = openSync( + filePath, + fsConstants.O_RDWR | (process.platform === 'win32' ? 0 : fsConstants.O_NOFOLLOW) + ) + const actual = fileIdentity(fileFd) + if (!actual || !sameFile(actual, fileExpected)) { + return false + } + ftruncateSync(fileFd, 0) + return true + } catch { + return false + } finally { + if (fileFd !== undefined) { + closeSync(fileFd) + } + if (directoryFd !== undefined) { + closeSync(directoryFd) + } + } +} + +function readLocations(path: string, session: string): Location[] { + try { + const stat = lstatSync(path) + if (stat.isSymbolicLink() || !stat.isFile() || stat.size > MAX_FISH_HISTORY_META_BYTES) { + return [] + } + const raw: unknown = JSON.parse(readFileSync(path, 'utf8')) + if (!raw || typeof raw !== 'object' || Array.isArray(raw)) { + return [] + } + const record = raw as Record + if ( + record.version !== 2 || + record.fishSession !== session || + !Array.isArray(record.locations) + ) { + return [] + } + const result: Location[] = [] + const seen = new Set() + for (const candidate of record.locations.slice(-MAX_PATH_CANDIDATES).toReversed()) { + if (!candidate || typeof candidate !== 'object' || Array.isArray(candidate)) { + continue + } + const value = candidate as Record + if ( + !canonicalPath(session, value.path) || + seen.has(value.path) || + ![ + 'directoryDevice', + 'directoryInode', + 'directoryBirthtimeNs', + 'fileDevice', + 'fileInode', + 'fileBirthtimeNs' + ].every((key) => typeof value[key] === 'string') + ) { + continue + } + seen.add(value.path) + result.push(value as unknown as Location) + if (result.length === MAX_RETAINED_FISH_HISTORY_PATHS) { + break + } + } + return result.toReversed() + } catch { + return [] + } +} + +export function attestFishHistoryLocation( + attestationPath: string, + session: string, + historyPath: string | null +): void { + if (!historyPath || !canonicalPath(session, historyPath)) { + return + } + try { + mkdirSync(dirname(historyPath), { recursive: true, mode: 0o700 }) + const directory = directoryIdentity(historyPath) + if (!directory) { + return + } + let fd: number | undefined + try { + try { + const stat = lstatSync(historyPath) + if (stat.isSymbolicLink() || !stat.isFile()) { + return + } + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { + return + } + } + fd = openSync( + historyPath, + fsConstants.O_RDWR | + fsConstants.O_CREAT | + (process.platform === 'win32' ? 0 : fsConstants.O_NOFOLLOW), + 0o600 + ) + const file = fileIdentity(fd) + if (!file) { + return + } + const previous = readLocations(attestationPath, session) + const active = previous.at(-1) + if ( + active?.path === historyPath && + sameFile(file, active) && + active.directoryDevice === directory.directoryDevice && + active.directoryInode === directory.directoryInode && + active.directoryBirthtimeNs === directory.directoryBirthtimeNs + ) { + return + } + const locations = [ + ...previous.filter((entry) => entry.path !== historyPath), + { path: historyPath, ...directory, ...file } + ].slice(-MAX_RETAINED_FISH_HISTORY_PATHS) + mkdirSync(dirname(attestationPath), { recursive: true, mode: 0o700 }) + if (existsSync(attestationPath) && lstatSync(attestationPath).isSymbolicLink()) { + return + } + writeFileSync( + attestationPath, + JSON.stringify({ version: 2, fishSession: session, locations }), + { mode: 0o600 } + ) + } finally { + if (fd !== undefined) { + closeSync(fd) + } + } + } catch (error) { + console.warn( + `[pty:history] Failed to attest fish history location: ${error instanceof Error ? error.message : String(error)}` + ) + } +} + +export function deleteFishHistoryFile(session: string, attestationPath: string): void { + const locations = readLocations(attestationPath, session) + if (locations.length === 0) { + return + } + let removed = false + for (const location of locations) { + const directory = directoryIdentity(location.path) + if ( + !directory || + directory.directoryDevice !== location.directoryDevice || + directory.directoryInode !== location.directoryInode || + directory.directoryBirthtimeNs !== location.directoryBirthtimeNs + ) { + continue + } + removed = clearFile(location.path, directory, location) || removed + } + if (!removed) { + console.warn(`[pty:history] No attested fish history file found for session ${session}`) + } +} diff --git a/src/main/fish-history-session.test.ts b/src/main/fish-history-session.test.ts index 09fac9ec61a..4590ade7d1e 100644 --- a/src/main/fish-history-session.test.ts +++ b/src/main/fish-history-session.test.ts @@ -9,7 +9,7 @@ import { writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' -import { join } from 'node:path' +import { dirname, join } from 'node:path' import { afterEach, beforeEach, describe, expect, it } from 'vitest' import { attestFishHistoryLocation, @@ -104,4 +104,52 @@ describe('fish history location attestation', () => { expect(readFileSync(owned, 'utf8')).toBe('replacement') }) + + it('rejects a final history symlink replacement', () => { + const dataRoot = join(root, 'data') + const owned = historyPath(dataRoot) + attestFishHistoryLocation(attestationPath, session, owned) + const target = join(root, 'target-history') + writeFileSync(target, 'unrelated') + rmSync(owned) + symlinkSync(target, owned) + + deleteFishHistoryFile(session, attestationPath) + + expect(readFileSync(target, 'utf8')).toBe('unrelated') + }) + + it('rejects a replacement regular file with a different identity', () => { + const dataRoot = join(root, 'data') + const owned = historyPath(dataRoot) + attestFishHistoryLocation(attestationPath, session, owned) + rmSync(owned) + writeFileSync(owned, 'replacement') + + deleteFishHistoryFile(session, attestationPath) + + expect(readFileSync(owned, 'utf8')).toBe('replacement') + }) + + it('fails closed for version-one attestations without file identity', () => { + const dataRoot = join(root, 'data') + const owned = historyPath(dataRoot) + mkdirSync(dirname(owned), { recursive: true }) + writeFileSync(owned, 'history') + mkdirSync(dirname(attestationPath), { recursive: true }) + writeFileSync( + attestationPath, + JSON.stringify({ + version: 1, + fishSession: session, + locations: [ + { path: owned, directoryDevice: '1', directoryInode: '1', directoryBirthtimeNs: '1' } + ] + }) + ) + + deleteFishHistoryFile(session, attestationPath) + + expect(readFileSync(owned, 'utf8')).toBe('history') + }) }) diff --git a/src/main/fish-history-session.ts b/src/main/fish-history-session.ts index 7925361572a..282e0b9dbc0 100644 --- a/src/main/fish-history-session.ts +++ b/src/main/fish-history-session.ts @@ -1,61 +1,29 @@ import { homedir } from 'node:os' -import { basename, dirname, isAbsolute, join, parse, relative, resolve, sep } from 'node:path' +import { basename, dirname, isAbsolute, join, resolve } from 'node:path' import { - closeSync, - constants as fsConstants, - existsSync, - ftruncateSync, - lstatSync, - mkdirSync, - openSync, - readFileSync, - writeFileSync -} from 'node:fs' + attestFishHistoryLocation, + deleteFishHistoryFile, + MAX_FISH_HISTORY_META_BYTES, + MAX_RETAINED_FISH_HISTORY_PATHS +} from './fish-history-location-attestation' -/** - * Per-worktree fish history, which fish models as a session NAME rather than a path. - * - * fish ignores HISTFILE entirely: history lives at - * `$XDG_DATA_HOME/fish/${fish_history}_history` (default session "fish"), so the - * only isolation knob is `fish_history`. fish imports environment variables as - * global variables at startup, so exporting `fish_history` in the spawn env picks - * the session without an `--init-command` — verified against fish 4.7.1. - * - * Consequence: the file lands in the USER's fish data dir, not Orca's history - * root, so it cannot be tombstoned with the rest of a worktree's history tree — - * `deleteFishHistoryFile` removes it directly instead. - * - * Two more fish facts for anyone reading these files: history is written only in - * INTERACTIVE mode, and the format is a YAML-ish record list (`- cmd: …` / - * ` when: …`), not the one-line-per-command form bash and zsh use. - */ -const SESSION_PREFIX = 'orca_' -// Why: the name becomes a filename and is only ever built from a hex hash; anything -// else means a caller drifted, and refusing beats deleting an unexpected path. -const SAFE_SESSION_NAME = /^orca_[0-9a-f]{1,64}$/ - -export const MAX_RETAINED_FISH_HISTORY_PATHS = 16 -export const MAX_FISH_HISTORY_META_BYTES = 32 * 1024 -const MAX_FISH_HISTORY_PATH_CANDIDATES = MAX_RETAINED_FISH_HISTORY_PATHS * 4 -export const FISH_HISTORY_LOCATION_ATTESTATION = 'fish-history-locations.json' - -type FishHistoryLocationAttestation = { - path: string - directoryDevice: string - directoryInode: string - directoryBirthtimeNs: string +export { + attestFishHistoryLocation, + deleteFishHistoryFile, + MAX_FISH_HISTORY_META_BYTES, + MAX_RETAINED_FISH_HISTORY_PATHS } -type FishHistoryDirectoryIdentity = Omit +export const FISH_HISTORY_LOCATION_ATTESTATION = 'fish-history-locations.json' +const SESSION_PREFIX = 'orca_' +const SAFE_SESSION_NAME = /^orca_[0-9a-f]{1,64}$/ +const MAX_PATH_CANDIDATES = MAX_RETAINED_FISH_HISTORY_PATHS * 4 export function isSafeFishHistorySession(session: unknown): session is string { return typeof session === 'string' && SAFE_SESSION_NAME.test(session) } - export function fishHistorySessionName(worktreeHash: string): string { return `${SESSION_PREFIX}${worktreeHash}` } - -/** Absolute path fish writes a session's history to, or null when it is not resolvable. */ export function resolveFishHistoryFilePath( session: string, env: NodeJS.ProcessEnv = process.env @@ -63,20 +31,15 @@ export function resolveFishHistoryFilePath( if (!isSafeFishHistorySession(session)) { return null } - const xdgDataHome = env.XDG_DATA_HOME?.trim() - // Why: fish ignores a relative XDG_DATA_HOME and falls back to the home default. + const xdg = env.XDG_DATA_HOME?.trim() const dataHome = - xdgDataHome && isAbsolute(xdgDataHome) - ? xdgDataHome - : join(env.HOME?.trim() || homedir(), '.local', 'share') + xdg && isAbsolute(xdg) ? xdg : join(env.HOME?.trim() || homedir(), '.local', 'share') return join(dataHome, 'fish', `${session}_history`) } - -/** Validate path shape only; deletion authority comes from a separate identity attestation. */ -function isCanonicalFishHistoryPath(session: string, path: unknown): path is string { - return Boolean( +function canonicalPath(session: string, path: unknown): path is string { + return ( typeof path === 'string' && - path && + Boolean(path) && isSafeFishHistorySession(session) && isAbsolute(path) && resolve(path) === path && @@ -84,181 +47,22 @@ function isCanonicalFishHistoryPath(session: string, path: unknown): path is str basename(path) === `${session}_history` ) } - -function fishHistoryDirectoryIdentity(path: string): FishHistoryDirectoryIdentity | null { - const directory = dirname(path) - const root = parse(directory).root - let current = root - try { - const segments = relative(root, directory).split(sep).filter(Boolean) - const roots = - segments.length === 0 ? [root] : segments.map((segment) => (current = join(current, segment))) - let identity: FishHistoryDirectoryIdentity | null = null - for (const candidate of roots) { - const stat = lstatSync(candidate, { bigint: true }) - if (stat.isSymbolicLink() || !stat.isDirectory()) { - return null - } - identity = { - directoryDevice: stat.dev.toString(), - directoryInode: stat.ino.toString(), - directoryBirthtimeNs: stat.birthtimeNs.toString() - } - } - return identity - } catch { - return null - } -} - -/** Clear through an attested directory handle; never unlink a pathname after checking it. */ -function clearFishHistoryFile(path: string): boolean { - const directory = dirname(path) - let directoryFd: number | undefined - let fileFd: number | undefined - try { - directoryFd = openSync( - directory, - fsConstants.O_RDONLY | fsConstants.O_DIRECTORY | fsConstants.O_NOFOLLOW - ) - if (process.platform === 'win32') { - return false - } - const fdRoot = process.platform === 'linux' ? '/proc/self/fd' : '/dev/fd' - fileFd = openSync(join(fdRoot, String(directoryFd), basename(path)), fsConstants.O_RDWR) - ftruncateSync(fileFd, 0) - return true - } catch { - return false - } finally { - if (fileFd !== undefined) { - closeSync(fileFd) - } - if (directoryFd !== undefined) { - closeSync(directoryFd) - } - } -} - -function readFishHistoryLocationAttestations( - attestationPath: string, - session: string -): FishHistoryLocationAttestation[] { - try { - const stat = lstatSync(attestationPath) - if (stat.isSymbolicLink() || !stat.isFile() || stat.size > MAX_FISH_HISTORY_META_BYTES) { - return [] - } - const raw: unknown = JSON.parse(readFileSync(attestationPath, 'utf8')) - if (!raw || typeof raw !== 'object' || Array.isArray(raw)) { - return [] - } - const record = raw as Record - if ( - record.version !== 1 || - record.fishSession !== session || - !Array.isArray(record.locations) - ) { - return [] - } - const candidates = record.locations.slice(-MAX_FISH_HISTORY_PATH_CANDIDATES) - const retained: FishHistoryLocationAttestation[] = [] - const seen = new Set() - for (let index = candidates.length - 1; index >= 0; index -= 1) { - const candidate = candidates[index] - if (!candidate || typeof candidate !== 'object' || Array.isArray(candidate)) { - continue - } - const location = candidate as Record - if ( - !isCanonicalFishHistoryPath(session, location.path) || - typeof location.directoryDevice !== 'string' || - typeof location.directoryInode !== 'string' || - typeof location.directoryBirthtimeNs !== 'string' || - seen.has(location.path) - ) { - continue - } - seen.add(location.path) - retained.push({ - path: location.path, - directoryDevice: location.directoryDevice, - directoryInode: location.directoryInode, - directoryBirthtimeNs: location.directoryBirthtimeNs - }) - if (retained.length === MAX_RETAINED_FISH_HISTORY_PATHS) { - break - } - } - return retained.toReversed() - } catch { - return [] - } -} - -/** Record a spawn-derived fish location outside meta.json, bound to its directory identity. */ -export function attestFishHistoryLocation( - attestationPath: string, - session: string, - historyPath: string | null -): void { - if (!historyPath || !isCanonicalFishHistoryPath(session, historyPath)) { - return - } - try { - mkdirSync(dirname(historyPath), { recursive: true, mode: 0o700 }) - const identity = fishHistoryDirectoryIdentity(historyPath) - if (!identity) { - console.warn(`[pty:history] Refusing to attest symlinked fish history path ${historyPath}`) - return - } - const retained = readFishHistoryLocationAttestations(attestationPath, session) - const active = retained.at(-1) - if ( - active?.path === historyPath && - active.directoryDevice === identity.directoryDevice && - active.directoryInode === identity.directoryInode && - active.directoryBirthtimeNs === identity.directoryBirthtimeNs - ) { - return - } - const existing = retained.filter((entry) => entry.path !== historyPath) - const locations = [...existing, { path: historyPath, ...identity }].slice( - -MAX_RETAINED_FISH_HISTORY_PATHS - ) - mkdirSync(dirname(attestationPath), { recursive: true, mode: 0o700 }) - if (existsSync(attestationPath) && lstatSync(attestationPath).isSymbolicLink()) { - return - } - writeFileSync( - attestationPath, - JSON.stringify({ version: 1, fishSession: session, locations }), - { mode: 0o600 } - ) - } catch (error) { - console.warn( - `[pty:history] Failed to attest fish history location: ${error instanceof Error ? error.message : String(error)}` - ) - } -} - export function normalizeFishHistoryPaths( session: string, singularPath: unknown, paths: readonly unknown[] | undefined ): string[] { - const recentPaths = (paths ?? []).slice(-MAX_FISH_HISTORY_PATH_CANDIDATES) const newestFirst: string[] = [] const seen = new Set() const retain = (path: unknown): void => { - if (!isCanonicalFishHistoryPath(session, path) || seen.has(path)) { + if (!canonicalPath(session, path) || seen.has(path)) { return } seen.add(path) newestFirst.push(path) } - for (let index = recentPaths.length - 1; index >= 0; index -= 1) { - retain(recentPaths[index]) + for (const path of (paths ?? []).slice(-MAX_PATH_CANDIDATES).toReversed()) { + retain(path) if (newestFirst.length === MAX_RETAINED_FISH_HISTORY_PATHS) { break } @@ -266,54 +70,5 @@ export function normalizeFishHistoryPaths( if (newestFirst.length < MAX_RETAINED_FISH_HISTORY_PATHS) { retain(singularPath) } - const retained: string[] = [] - for (let index = newestFirst.length - 1; index >= 0; index -= 1) { - if (retained.length === MAX_RETAINED_FISH_HISTORY_PATHS) { - break - } - retained.push(newestFirst[index]) - } - return retained -} - -/** Delete only spawn-attested locations whose non-symlink directory identity still matches. */ -export function deleteFishHistoryFile(session: string, attestationPath: string): void { - const locations = readFishHistoryLocationAttestations(attestationPath, session) - if (locations.length === 0) { - console.warn(`[pty:history] No attested fish history location for session ${session}`) - return - } - let removed = false - for (const location of locations) { - try { - const identity = fishHistoryDirectoryIdentity(location.path) - if ( - !identity || - identity.directoryDevice !== location.directoryDevice || - identity.directoryInode !== location.directoryInode || - identity.directoryBirthtimeNs !== location.directoryBirthtimeNs - ) { - console.warn(`[pty:history] Refusing fish history cleanup after directory identity changed`) - continue - } - if (existsSync(location.path)) { - const stat = lstatSync(location.path) - if (stat.isSymbolicLink() || !stat.isFile()) { - continue - } - removed = clearFishHistoryFile(location.path) || removed - } - } catch (err) { - console.warn( - `[pty:history] Failed to delete fish history ${location.path}: ${err instanceof Error ? err.message : String(err)}` - ) - } - } - if (!removed) { - // Explicit: a config.fish-set XDG_DATA_HOME leaves a file we cannot locate, - // and silence would hide one leaked history file per deleted worktree. - console.warn( - `[pty:history] No attested fish history file found for session ${session}; a config.fish-only XDG_DATA_HOME remains outside Orca's cleanup authority.` - ) - } + return newestFirst.toReversed().slice(-MAX_RETAINED_FISH_HISTORY_PATHS) } diff --git a/src/main/terminal-history-attested-deletion.test.ts b/src/main/terminal-history-attested-deletion.test.ts new file mode 100644 index 00000000000..0b727cf001f --- /dev/null +++ b/src/main/terminal-history-attested-deletion.test.ts @@ -0,0 +1,70 @@ +import { lstatSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +let userDataDir = '' +vi.mock('electron', () => ({ app: { getPath: () => userDataDir } })) + +import { + deleteWorktreeHistoryDir, + flushPendingWorktreeHistoryDeletions +} from './terminal-history-deletion' +import { hashWorktreeId } from './terminal-history-paths' +import { deleteFishHistoryFile, fishHistorySessionName } from './fish-history-session' + +describe('attested fish history deletion', () => { + beforeEach(() => { + userDataDir = mkdtempSync(join(tmpdir(), 'orca-attested-history-')) + }) + afterEach(async () => { + await flushPendingWorktreeHistoryDeletions() + rmSync(userDataDir, { recursive: true, force: true }) + }) + + it('warns when an attested fish history file disappears before deletion', async () => { + const worktreeId = 'repo::/missing-fish-history' + const hash = hashWorktreeId(worktreeId) + const dir = join(userDataDir, 'terminal-history', hash) + mkdirSync(dir, { recursive: true }) + writeFileSync( + join(dir, 'meta.json'), + JSON.stringify({ worktreeId, fishSession: fishHistorySessionName(hash) }) + ) + const session = fishHistorySessionName(hash) + const historyPath = join(userDataDir, 'fish', `${session}_history`) + const attestationPath = join(dir, 'fish-history-locations.json') + mkdirSync(join(userDataDir, 'fish'), { recursive: true }) + writeFileSync(historyPath, 'history') + const directory = lstatSync(join(userDataDir, 'fish'), { bigint: true }) + const file = lstatSync(historyPath, { bigint: true }) + writeFileSync( + attestationPath, + JSON.stringify({ + version: 2, + fishSession: session, + locations: [ + { + path: historyPath, + directoryDevice: directory.dev.toString(), + directoryInode: directory.ino.toString(), + directoryBirthtimeNs: directory.birthtimeNs.toString(), + fileDevice: file.dev.toString(), + fileInode: file.ino.toString(), + fileBirthtimeNs: file.birthtimeNs.toString() + } + ] + }) + ) + rmSync(historyPath) + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + try { + deleteFishHistoryFile(session, attestationPath) + expect(warn).toHaveBeenCalledWith(expect.stringContaining('No attested fish history')) + deleteWorktreeHistoryDir(worktreeId) + await flushPendingWorktreeHistoryDeletions() + } finally { + warn.mockRestore() + } + }) +}) diff --git a/src/main/terminal-history-deletion.ts b/src/main/terminal-history-deletion.ts index 4f3ce56833f..7a375779558 100644 --- a/src/main/terminal-history-deletion.ts +++ b/src/main/terminal-history-deletion.ts @@ -12,6 +12,7 @@ import { hashWorktreeId } from './terminal-history-id' import { deleteWslFishHistoryFile } from './wsl-fish-history-cleanup' const pendingHistoryTreeRemovals = new Map>() +export const MAX_PENDING_HISTORY_TREE_REMOVALS = 64 // Why: a tombstone that fails once (Windows EBUSY under AV) would otherwise sit on disk for the whole // desktop session — only the next launch re-queues it. Bounded so a genuinely stuck tree stops retrying. export const HISTORY_TREE_REMOVAL_RETRY_DELAYS_MS = [30_000, 120_000] @@ -29,6 +30,10 @@ function getPendingDeleteRoot(historyRoot: string): string { return join(historyRoot, PENDING_DELETE_DIR_NAME) } +function historyRootForTombstone(dir: string): string { + return dirname(dirname(dir)) +} + /** Move a history tree to a pending-delete tombstone (metadata-only) so the critical path never walks it. */ function tombstoneHistoryTree(dir: string, historyRoot: string): string | null { if (!existsSync(dir)) { @@ -77,15 +82,22 @@ function scheduleHistoryTreeRemoval(dir: string, wslDistro?: string): void { if (pendingHistoryTreeRemovals.has(dir)) { return } - // A rescan (GC / startup drain) hitting the same tombstone supersedes its pending retry. + // Leave excess tombstones on disk; admission is intentionally bounded. + if ( + pendingHistoryTreeRemovals.size + historyTreeRemovalRetryTimers.size >= + MAX_PENDING_HISTORY_TREE_REMOVALS + ) { + return + } + // A rescan must not cancel a delayed retry for a real failure. const pendingRetry = historyTreeRemovalRetryTimers.get(dir) if (pendingRetry) { - clearTimeout(pendingRetry) - historyTreeRemovalRetryTimers.delete(dir) + return } if (wslDistro) { wslDistroByTombstone.set(dir, wslDistro) } + let removalSucceeded = false const cleanupDistro = wslDistroByTombstone.get(dir) const meta = cleanupDistro ? readHistoryMeta(dir) : null const cleanup = @@ -94,10 +106,12 @@ function scheduleHistoryTreeRemoval(dir: string, wslDistro?: string): void { console.warn( `[pty:history] Failed to delete WSL fish history: ${err instanceof Error ? err.message : String(err)}` ) + throw err }) : null const removal = (cleanup ? cleanup.then(() => removeHostTree(dir)) : removeHostTree(dir)) .then(() => { + removalSucceeded = true historyTreeRemovalAttempts.delete(dir) wslDistroByTombstone.delete(dir) }) @@ -111,6 +125,9 @@ function scheduleHistoryTreeRemoval(dir: string, wslDistro?: string): void { if (pendingHistoryTreeRemovals.get(dir) === removal) { pendingHistoryTreeRemovals.delete(dir) } + if (removalSucceeded) { + schedulePendingHistoryTreeRemovals(historyRootForTombstone(dir)) + } }) pendingHistoryTreeRemovals.set(dir, removal) } diff --git a/src/main/terminal-history-tombstone-retry.test.ts b/src/main/terminal-history-tombstone-retry.test.ts index 195ee82d9f3..d080292ad6c 100644 --- a/src/main/terminal-history-tombstone-retry.test.ts +++ b/src/main/terminal-history-tombstone-retry.test.ts @@ -1,12 +1,13 @@ -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { mkdirSync, mkdtempSync, readdirSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' let userDataDir: string -const { removeHostTreeMock } = vi.hoisted(() => ({ - removeHostTreeMock: vi.fn<(dir: string) => Promise>() +const { removeHostTreeMock, deleteWslFishHistoryFileMock } = vi.hoisted(() => ({ + removeHostTreeMock: vi.fn<(dir: string) => Promise>(), + deleteWslFishHistoryFileMock: vi.fn<(distro: string, session: string) => Promise>() })) vi.mock('electron', () => ({ @@ -19,11 +20,19 @@ vi.mock('./host-tree-removal', () => ({ removeHostTree: removeHostTreeMock })) +vi.mock('./wsl-fish-history-cleanup', () => ({ + deleteWslFishHistoryFile: deleteWslFishHistoryFileMock +})) + import { hashWorktreeId } from './terminal-history-paths' +import { fishHistorySessionName } from './fish-history-session' import { cancelPendingHistoryTreeRemovalRetries, deleteWorktreeHistoryDir, - HISTORY_TREE_REMOVAL_RETRY_DELAYS_MS + flushPendingWorktreeHistoryDeletions, + MAX_PENDING_HISTORY_TREE_REMOVALS, + HISTORY_TREE_REMOVAL_RETRY_DELAYS_MS, + schedulePendingHistoryTreeRemovals } from './terminal-history-deletion' /** A tombstone whose rm fails once used to sit on disk for the rest of the session — only the next @@ -32,6 +41,11 @@ describe('tombstoned history removal retries', () => { beforeEach(() => { userDataDir = mkdtempSync(join(tmpdir(), 'orca-history-retry-')) removeHostTreeMock.mockReset() + deleteWslFishHistoryFileMock.mockReset() + deleteWslFishHistoryFileMock.mockResolvedValue(undefined) + removeHostTreeMock.mockImplementation(async (dir) => { + rmSync(dir, { recursive: true, force: true }) + }) vi.useFakeTimers() }) @@ -71,7 +85,6 @@ describe('tombstoned history removal retries', () => { it('does not re-arm a retry after the removal succeeds', async () => { seedWorktreeHistory('repo-1::/path/clean-wt') - removeHostTreeMock.mockResolvedValue(undefined) deleteWorktreeHistoryDir('repo-1::/path/clean-wt') await vi.advanceTimersByTimeAsync(0) @@ -79,4 +92,86 @@ describe('tombstoned history removal retries', () => { expect(removeHostTreeMock).toHaveBeenCalledTimes(1) }) + + it('admits a bounded tombstone batch and drains the rest from disk', async () => { + const distroRoot = join(userDataDir, 'terminal-history-wsl', 'Ubuntu') + mkdirSync(distroRoot, { recursive: true }) + const releases: (() => void)[] = [] + removeHostTreeMock.mockImplementation( + (dir) => + new Promise((resolve) => + releases.push(() => { + rmSync(dir, { recursive: true, force: true }) + resolve() + }) + ) + ) + + for (let index = 0; index < 1_000; index++) { + const worktreeId = `repo-1::/path/wsl-${index}` + const dir = join(distroRoot, hashWorktreeId(worktreeId)) + mkdirSync(dir, { recursive: true }) + writeFileSync( + join(dir, 'meta.json'), + JSON.stringify({ + worktreeId, + fishSession: fishHistorySessionName(hashWorktreeId(worktreeId)) + }) + ) + } + + for (let index = 0; index < 1_000; index++) { + deleteWorktreeHistoryDir(`repo-1::/path/wsl-${index}`) + } + await vi.advanceTimersByTimeAsync(0) + expect(removeHostTreeMock).toHaveBeenCalledTimes(64) + expect(deleteWslFishHistoryFileMock).toHaveBeenCalledTimes(64) + expect(releases).toHaveLength(64) + + while (releases.length > 0) { + releases.splice(0).forEach((release) => release()) + await vi.advanceTimersByTimeAsync(0) + } + expect(removeHostTreeMock).toHaveBeenCalledTimes(1_000) + expect(deleteWslFishHistoryFileMock).toHaveBeenCalledTimes(1_000) + }) + + it('caps persistent failures and leaves excess tombstones for a later disk batch', async () => { + const distroRoot = join(userDataDir, 'terminal-history-wsl', 'Ubuntu') + mkdirSync(distroRoot, { recursive: true }) + deleteWslFishHistoryFileMock.mockRejectedValue(new Error('wsl unavailable')) + for (let index = 0; index < 1_000; index++) { + const worktreeId = `repo-1::/path/fail-${index}` + const dir = join(distroRoot, hashWorktreeId(worktreeId)) + mkdirSync(dir, { recursive: true }) + writeFileSync( + join(dir, 'meta.json'), + JSON.stringify({ + worktreeId, + fishSession: fishHistorySessionName(hashWorktreeId(worktreeId)) + }) + ) + deleteWorktreeHistoryDir(worktreeId) + } + await vi.advanceTimersByTimeAsync(0) + expect(removeHostTreeMock).not.toHaveBeenCalled() + expect(vi.getTimerCount()).toBe(64) + expect(readdirSync(join(distroRoot, '.pending-delete'))).toHaveLength(1_000) + + for (const delay of HISTORY_TREE_REMOVAL_RETRY_DELAYS_MS) { + await vi.advanceTimersByTimeAsync(delay) + expect(vi.getTimerCount()).toBeLessThanOrEqual(64) + } + await vi.advanceTimersByTimeAsync(0) + deleteWslFishHistoryFileMock.mockResolvedValue(undefined) + removeHostTreeMock.mockImplementation(async (dir) => { + rmSync(dir, { recursive: true, force: true }) + }) + schedulePendingHistoryTreeRemovals(distroRoot) + await flushPendingWorktreeHistoryDeletions() + expect(removeHostTreeMock.mock.calls.length).toBeGreaterThan( + MAX_PENDING_HISTORY_TREE_REMOVALS * (HISTORY_TREE_REMOVAL_RETRY_DELAYS_MS.length + 1) + ) + expect(readdirSync(join(distroRoot, '.pending-delete'))).toHaveLength(0) + }) }) diff --git a/src/main/terminal-history.test.ts b/src/main/terminal-history.test.ts index 83b87945dd3..177f8c23016 100644 --- a/src/main/terminal-history.test.ts +++ b/src/main/terminal-history.test.ts @@ -12,6 +12,9 @@ const { renameSyncMock, readdirSyncMock, statSyncMock, + openSyncMock, + fstatSyncMock, + closeSyncMock, getPathMock, rmAsyncMock, deleteWslFishHistoryFileMock @@ -25,6 +28,15 @@ const { renameSyncMock: vi.fn(), readdirSyncMock: vi.fn(), statSyncMock: vi.fn(), + openSyncMock: vi.fn((_path?: string | Buffer) => 1), + fstatSyncMock: vi.fn(() => ({ + dev: 1n, + ino: 2n, + birthtimeNs: 3n, + isFile: () => true, + isDirectory: () => true + })), + closeSyncMock: vi.fn(), getPathMock: vi.fn(), rmAsyncMock: vi.fn(async () => undefined), deleteWslFishHistoryFileMock: vi.fn(async () => undefined) @@ -39,7 +51,10 @@ vi.mock('fs', () => ({ rmSync: rmSyncMock, renameSync: renameSyncMock, readdirSync: readdirSyncMock, - statSync: statSyncMock + statSync: statSyncMock, + openSync: openSyncMock, + fstatSync: fstatSyncMock, + closeSync: closeSyncMock })) // Spread the real module: this factory replaces node:fs/promises for the whole import graph, so a @@ -84,6 +99,7 @@ import { } from './fish-history-session' import { hashWorktreeId } from './terminal-history-paths' import { + cancelPendingHistoryTreeRemovalRetries, deleteWorktreeHistoryDir, flushPendingWorktreeHistoryDeletions } from './terminal-history-deletion' @@ -93,6 +109,7 @@ import { runHistoryGc, scheduleHistoryGc } from './terminal-history-gc' describe('terminal-history', () => { afterEach(() => { + cancelPendingHistoryTreeRemovalRetries() vi.useRealTimers() }) @@ -101,9 +118,20 @@ describe('terminal-history', () => { // Why: clearAllMocks keeps implementations, so a throwing rename from one test would leak forward. renameSyncMock.mockReset() readFileSyncMock.mockReset() + readdirSyncMock.mockReset() + rmAsyncMock.mockReset() + rmAsyncMock.mockResolvedValue(undefined) getPathMock.mockReturnValue('/fake/userData') existsSyncMock.mockReturnValue(true) statSyncMock.mockReturnValue({ isDirectory: () => true, size: 100 }) + openSyncMock.mockImplementation(() => 1) + fstatSyncMock.mockReturnValue({ + dev: 1n, + ino: 2n, + birthtimeNs: 3n, + isFile: () => true, + isDirectory: () => true + }) lstatSyncMock.mockReturnValue({ dev: 1n, ino: 2n, @@ -377,7 +405,7 @@ describe('terminal-history', () => { injectHistoryEnv(env, 'repo-1::/path/wt', '/usr/bin/fish', '/path/wt') - expect(readFileSyncMock).toHaveBeenCalledTimes(1) + expect(readFileSyncMock).not.toHaveBeenCalled() const meta = JSON.parse(writeFileSyncMock.mock.calls.at(-1)?.[1] as string) as FishHistoryMeta expect(meta.fishHistoryPaths).toEqual([meta.fishHistoryPath]) }) @@ -604,20 +632,6 @@ describe('terminal-history', () => { await flushPendingWorktreeHistoryDeletions() }) - it('warns instead of going quiet when no fish history file is found', async () => { - const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) - try { - readFileSyncMock.mockReturnValue(JSON.stringify({ worktreeId, fishSession: session })) - // The history dir exists (so the tree is deleted) but the fish file does not. - existsSyncMock.mockImplementation((path: string) => !path.includes(historyFilename)) - deleteWorktreeHistoryDir('repo-1::/path/wt') - expect(warn).toHaveBeenCalledWith(expect.stringContaining('No attested fish history')) - } finally { - warn.mockRestore() - } - await flushPendingWorktreeHistoryDeletions() - }) - it('cleans WSL Fish history through the owning distro before removing metadata', async () => { readFileSyncMock.mockReturnValue(JSON.stringify({ worktreeId, fishSession: session })) readdirSyncMock.mockImplementation((path: string) => { @@ -656,6 +670,7 @@ describe('terminal-history', () => { }) it('retries pending tombstones on flush after a failed async rm (app-quit durability)', async () => { + let leftoverTombstonePresent = true existsSyncMock.mockImplementation((p: string) => { const path = String(p) if (path.includes('.pending-delete') && !path.endsWith('.pending-delete')) { @@ -668,11 +683,13 @@ describe('terminal-history', () => { }) readdirSyncMock.mockImplementation((p: string) => { if (String(p).endsWith('.pending-delete')) { - return ['leftover-tombstone'] + return leftoverTombstonePresent ? ['leftover-tombstone'] : [] } return [] }) - rmAsyncMock.mockResolvedValue(undefined) + rmAsyncMock.mockImplementation(async () => { + leftoverTombstonePresent = false + }) await flushPendingWorktreeHistoryDeletions() expect(rmAsyncMock).toHaveBeenCalledWith( expect.stringContaining('leftover-tombstone'), @@ -735,7 +752,7 @@ describe('terminal-history', () => { ) }) - it('continues GC after one orphan tombstone fails', () => { + it('continues GC after one orphan tombstone fails', async () => { existsSyncMock.mockImplementation((path: string) => !path.includes('terminal-history-wsl')) readdirSyncMock.mockImplementation((dir: string) => { if (dir.endsWith('.pending-delete')) { @@ -760,6 +777,7 @@ describe('terminal-history', () => { expect(() => runHistoryGc(new Set())).not.toThrow() expect(renameSyncMock).toHaveBeenCalledTimes(2) expect(rmAsyncMock).toHaveBeenCalledTimes(1) + await flushPendingWorktreeHistoryDeletions() }) it('skips recently-created directories to avoid TOCTOU race', () => { @@ -791,14 +809,15 @@ describe('terminal-history', () => { it('does not throw when history root does not exist', () => { existsSyncMock.mockReturnValue(false) expect(() => runHistoryGc(new Set())).not.toThrow() - expect(readdirSyncMock).not.toHaveBeenCalled() + expect(readdirSyncMock).not.toHaveBeenCalledWith('/fake/userData/terminal-history') }) it('drains delete tombstones asynchronously instead of scanning them as worktrees', async () => { + let tombstonePresent = true existsSyncMock.mockImplementation((p: string) => !String(p).includes('terminal-history-wsl')) readdirSyncMock.mockImplementation((dir: string) => { if (String(dir).endsWith('.pending-delete')) { - return ['abc123.1700000000000.deadbeef'] + return tombstonePresent ? ['abc123.1700000000000.deadbeef'] : [] } if (String(dir).endsWith('terminal-history')) { return ['.pending-delete'] @@ -806,6 +825,9 @@ describe('terminal-history', () => { return ['meta.json'] }) statSyncMock.mockReturnValue({ isDirectory: () => true, size: 100 }) + rmAsyncMock.mockImplementation(async () => { + tombstonePresent = false + }) runHistoryGc(new Set()) diff --git a/src/main/wsl-fish-history-cleanup.test.ts b/src/main/wsl-fish-history-cleanup.test.ts index cca59bf7586..8cb35ab89b6 100644 --- a/src/main/wsl-fish-history-cleanup.test.ts +++ b/src/main/wsl-fish-history-cleanup.test.ts @@ -1,5 +1,9 @@ -import { describe, expect, it, vi } from 'vitest' -import { deleteWslFishHistoryFile } from './wsl-fish-history-cleanup' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { deleteWslFishHistoryFile, flushWslFishHistoryCleanups } from './wsl-fish-history-cleanup' + +afterEach(async () => { + await flushWslFishHistoryCleanups() +}) describe('deleteWslFishHistoryFile', () => { it('uses direct argv and bounds a distro cleanup subprocess', async () => { @@ -29,19 +33,52 @@ describe('deleteWslFishHistoryFile', () => { expect(run).not.toHaveBeenCalled() }) - it('serializes cleanup subprocesses across a startup tombstone batch', async () => { - let releaseFirst: (() => void) | undefined - const first = new Promise<{ stdout: string; stderr: string }>((resolve) => { - releaseFirst = () => resolve({ stdout: '', stderr: '' }) - }) - const run = vi.fn().mockReturnValueOnce(first).mockResolvedValueOnce({ stdout: '', stderr: '' }) + it('runs cleanup subprocesses concurrently within the fixed budget', async () => { + const releases: (() => void)[] = [] + const run = vi.fn(() => { + return new Promise<{ stdout: string; stderr: string }>((resolve) => { + releases.push(() => resolve({ stdout: '', stderr: '' })) + }) + }) as unknown as Parameters[2] + const cleanups = Array.from({ length: 5 }, (_, index) => + deleteWslFishHistoryFile('Ubuntu', `orca_${(index + 1).toString(16)}`, run) + ) + await vi.waitFor(() => expect(run).toHaveBeenCalledTimes(4)) + expect(releases).toHaveLength(4) + releases[0]() + await vi.waitFor(() => expect(run).toHaveBeenCalledTimes(5)) + expect(releases).toHaveLength(5) + releases.slice(1).forEach((release) => release()) + await Promise.all(cleanups) + }) - const cleanupA = deleteWslFishHistoryFile('Ubuntu', 'orca_aaaaaaaaaaaaaaaa', run) - const cleanupB = deleteWslFishHistoryFile('Debian', 'orca_bbbbbbbbbbbbbbbb', run) - await vi.waitFor(() => expect(run).toHaveBeenCalledTimes(1)) - releaseFirst?.() - await Promise.all([cleanupA, cleanupB]) + it('coalesces duplicate requests and bounds adversarial startup fanout', async () => { + const run = vi.fn().mockResolvedValue({ stdout: '', stderr: '' }) + const duplicateA = deleteWslFishHistoryFile('Ubuntu', 'orca_aaaaaaaaaaaaaaaa', run) + const duplicateB = deleteWslFishHistoryFile('Ubuntu', 'orca_aaaaaaaaaaaaaaaa', run) + expect(duplicateB).toBe(duplicateA) + const requests = Array.from({ length: 1_000 }, (_, index) => + deleteWslFishHistoryFile('Ubuntu', `orca_${index.toString(16)}`, run) + ) + await Promise.allSettled([duplicateA, ...requests]) + + // Four live workers and 64 bounded queue slots; excess work remains durable on disk. + expect(run).toHaveBeenCalledTimes(68) + }) + + it('permits a failed cleanup to be retried after the live attempt settles', async () => { + const run = vi + .fn() + .mockRejectedValueOnce(new Error('wsl unavailable')) + .mockResolvedValueOnce({ stdout: '', stderr: '' }) + + await expect(deleteWslFishHistoryFile('Ubuntu', 'orca_aaaaaaaaaaaaaaaa', run)).rejects.toThrow( + 'wsl unavailable' + ) + await expect( + deleteWslFishHistoryFile('Ubuntu', 'orca_aaaaaaaaaaaaaaaa', run) + ).resolves.toBeUndefined() expect(run).toHaveBeenCalledTimes(2) }) }) diff --git a/src/main/wsl-fish-history-cleanup.ts b/src/main/wsl-fish-history-cleanup.ts index 564f7adc43a..62e8d7a2cf2 100644 --- a/src/main/wsl-fish-history-cleanup.ts +++ b/src/main/wsl-fish-history-cleanup.ts @@ -3,7 +3,63 @@ import { promisify } from 'node:util' import { isSafeFishHistorySession } from './fish-history-session' const execFileAsync = promisify(execFile) -let cleanupTail = Promise.resolve() +const MAX_CONCURRENT_CLEANUPS = 4 +const MAX_QUEUED_CLEANUPS = 64 +type CleanupJob = { + key: string + distro: string + session: string + run: typeof execFileAsync + resolve: () => void + reject: (error: unknown) => void +} + +let activeCleanups = 0 +const queuedCleanups: CleanupJob[] = [] +const cleanupPromises = new Map>() + +function pumpCleanupQueue(): void { + while (activeCleanups < MAX_CONCURRENT_CLEANUPS && queuedCleanups.length > 0) { + const job = queuedCleanups.shift() + if (!job) { + return + } + activeCleanups++ + void job + .run( + 'wsl.exe', + [ + '--distribution', + job.distro, + '--exec', + 'fish', + '--command', + fishCleanupScript(job.session) + ], + { + timeout: 5_000, + windowsHide: true + } + ) + .then( + () => job.resolve(), + (error: unknown) => job.reject(error) + ) + .finally(() => { + activeCleanups-- + cleanupPromises.delete(job.key) + pumpCleanupQueue() + }) + } +} + +function fishCleanupScript(session: string): string { + return [ + 'set -l data_home $XDG_DATA_HOME', + 'string match -qr "^/" -- $data_home; or set data_home "$HOME/.local/share"', + `command rm -f -- "$data_home/fish/${session}_history"` + ].join('; ') +} export function deleteWslFishHistoryFile( distro: string, @@ -13,18 +69,29 @@ export function deleteWslFishHistoryFile( if (!distro.trim() || !isSafeFishHistorySession(session)) { return Promise.resolve() } - const script = [ - 'set -l data_home $XDG_DATA_HOME', - 'string match -qr "^/" -- $data_home; or set data_home "$HOME/.local/share"', - `command rm -f -- "$data_home/fish/${session}_history"` - ].join('; ') - // Why: startup may discover many durable tombstones, but wsl.exe cleanup fanout must stay at one. - const cleanup = cleanupTail.then(async () => { - await run('wsl.exe', ['--distribution', distro, '--exec', 'fish', '--command', script], { - timeout: 5_000, - windowsHide: true - }) + const key = `${distro}\0${session}` + const existing = cleanupPromises.get(key) + if (existing) { + return existing + } + if (queuedCleanups.length >= MAX_QUEUED_CLEANUPS) { + return Promise.reject(new Error('WSL Fish history cleanup queue is full')) + } + let resolve!: () => void + let reject!: (error: unknown) => void + const cleanup = new Promise((resolvePromise, rejectPromise) => { + resolve = resolvePromise + reject = rejectPromise }) - cleanupTail = cleanup.catch(() => undefined) + cleanupPromises.set(key, cleanup) + queuedCleanups.push({ key, distro, session, run, resolve, reject }) + pumpCleanupQueue() return cleanup } + +/** Drain queued and active cleanups; intended for deterministic shutdown/test cleanup. */ +export async function flushWslFishHistoryCleanups(): Promise { + while (queuedCleanups.length > 0 || activeCleanups > 0) { + await new Promise((resolve) => setTimeout(resolve, 0)) + } +} diff --git a/src/relay/terminal-history.test.ts b/src/relay/terminal-history.test.ts index 034ff0005b0..1d700435474 100644 --- a/src/relay/terminal-history.test.ts +++ b/src/relay/terminal-history.test.ts @@ -1,4 +1,4 @@ -import { existsSync, rmSync } from 'node:fs' +import { existsSync, mkdirSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs' import { homedir } from 'node:os' import { join } from 'node:path' import { afterEach, describe, expect, it } from 'vitest' @@ -6,9 +6,14 @@ import { hashWorktreeId } from '../main/terminal-history-id' import { deleteRelayHistory, injectRelayHistoryEnv } from './terminal-history' const worktreeId = 'relay-test::/remote/worktree' -const historyDir = join(homedir(), '.orca-remote', 'terminal-history', hashWorktreeId(worktreeId)) +const historyDir = join(homedir(), '.orca-remote', 'terminal-history') +const historyPrefix = hashWorktreeId(worktreeId) -afterEach(() => rmSync(historyDir, { recursive: true, force: true })) +afterEach(() => { + for (const filename of ['bash_history', 'zsh_history']) { + rmSync(join(historyDir, `${historyPrefix}-${filename}`), { force: true }) + } +}) describe('relay shell history', () => { it.each(['/bin/bash', '/usr/bin/zsh'])('scopes %s without replacing caller HISTFILE', (shell) => { @@ -16,7 +21,10 @@ describe('relay shell history', () => { const dir = injectRelayHistoryEnv(env, worktreeId, shell) expect(dir).toBe(historyDir) expect(env.HISTFILE).toBe( - join(historyDir, shell.endsWith('bash') ? 'bash_history' : 'zsh_history') + join( + historyDir, + `${historyPrefix}-${shell.endsWith('bash') ? 'bash_history' : 'zsh_history'}` + ) ) const custom = { HISTFILE: '/custom/history' } @@ -29,6 +37,19 @@ describe('relay shell history', () => { expect(injectRelayHistoryEnv(env, worktreeId, '/bin/fish')).toBeNull() deleteRelayHistory(worktreeId) deleteRelayHistory(worktreeId) - expect(existsSync(historyDir)).toBe(false) + expect(existsSync(join(historyDir, `${historyPrefix}-bash_history`))).toBe(false) + }) + + it('refuses a pre-existing final symlink', () => { + mkdirSync(historyDir, { recursive: true }) + const target = join(historyDir, 'relay-unrelated-history') + const path = join(historyDir, `${historyPrefix}-bash_history`) + writeFileSync(target, 'unrelated') + symlinkSync(target, path) + + expect(injectRelayHistoryEnv({}, worktreeId, '/bin/bash')).toBeNull() + expect(readFileSync(target, 'utf8')).toBe('unrelated') + rmSync(path) + rmSync(target) }) }) diff --git a/src/relay/terminal-history.ts b/src/relay/terminal-history.ts index a1ebf8e25cc..569c9939719 100644 --- a/src/relay/terminal-history.ts +++ b/src/relay/terminal-history.ts @@ -1,4 +1,12 @@ -import { mkdirSync, rmSync } from 'node:fs' +import { + closeSync, + constants as fsConstants, + fstatSync, + lstatSync, + mkdirSync, + openSync, + unlinkSync +} from 'node:fs' import { homedir } from 'node:os' import { basename, join } from 'node:path' import { hashWorktreeId } from '../main/terminal-history-id' @@ -29,11 +37,42 @@ export function injectRelayHistoryEnv( return null } try { - const dir = join(HISTORY_ROOT, hashWorktreeId(worktreeId)) - mkdirSync(dir, { recursive: true, mode: 0o700 }) - const path = join(dir, filename) + mkdirSync(HISTORY_ROOT, { recursive: true, mode: 0o700 }) + const rootStat = lstatSync(HISTORY_ROOT) + if (rootStat.isSymbolicLink() || !rootStat.isDirectory()) { + return null + } + const path = join(HISTORY_ROOT, `${hashWorktreeId(worktreeId)}-${filename}`) + let existing: ReturnType | null = null + try { + const stat = lstatSync(path, { bigint: true }) + if (stat.isSymbolicLink() || !stat.isFile()) { + return null + } + existing = stat + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { + return null + } + } + const fd = openSync( + path, + fsConstants.O_RDWR | + fsConstants.O_CREAT | + (process.platform === 'win32' ? 0 : fsConstants.O_NOFOLLOW), + 0o600 + ) + const actual = fstatSync(fd, { bigint: true }) + if ( + !actual.isFile() || + (existing && (actual.dev !== existing.dev || actual.ino !== existing.ino)) + ) { + closeSync(fd) + return null + } + closeSync(fd) env.HISTFILE = path - return dir + return HISTORY_ROOT } catch { return null } @@ -41,7 +80,22 @@ export function injectRelayHistoryEnv( export function deleteRelayHistory(worktreeId: string): void { try { - rmSync(join(HISTORY_ROOT, hashWorktreeId(worktreeId)), { recursive: true, force: true }) + const rootStat = lstatSync(HISTORY_ROOT) + if (rootStat.isSymbolicLink() || !rootStat.isDirectory()) { + return + } + for (const filename of ['bash_history', 'zsh_history']) { + const path = join(HISTORY_ROOT, `${hashWorktreeId(worktreeId)}-${filename}`) + try { + if (!lstatSync(path).isSymbolicLink()) { + unlinkSync(path) + } + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { + throw error + } + } + } } catch (error) { console.warn( `[pty:history] Failed to delete relay shell history: ${error instanceof Error ? error.message : String(error)}`