diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 6826ecfb731..c44b2abaa56 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -270,7 +270,10 @@ jobs: src/main/providers/local-pty-shell-ready-zsh-zdotdir-normalization.test.ts \ src/main/providers/__tests__/shell-ready-framework-example.test.ts \ src/main/pty/omp-shell-wrapper.node-pty.test.ts \ + src/main/terminal-history-fish-session.node-pty.test.ts \ + src/main/zsh-scoped-histfile.live-shell.test.ts \ src/renderer/src/components/terminal-pane/fish-color-scheme-child-stdin.node-pty.test.ts \ + src/shared/startup-shell-portability.live-shell.test.ts \ src/shared/posix-command-path-lookup.test.ts test: @@ -309,7 +312,10 @@ jobs: --exclude=src/main/providers/local-pty-shell-ready-zsh-zdotdir-normalization.test.ts \ --exclude=src/main/providers/__tests__/shell-ready-framework-example.test.ts \ --exclude=src/main/pty/omp-shell-wrapper.node-pty.test.ts \ + --exclude=src/main/terminal-history-fish-session.node-pty.test.ts \ + --exclude=src/main/zsh-scoped-histfile.live-shell.test.ts \ --exclude=src/renderer/src/components/terminal-pane/fish-color-scheme-child-stdin.node-pty.test.ts \ + --exclude=src/shared/startup-shell-portability.live-shell.test.ts \ --exclude=src/shared/posix-command-path-lookup.test.ts \ --exclude=tests/e2e/cross-version-wire/** \ --shard=${{ matrix.shard }}/${{ matrix.shard_total }} diff --git a/config/scripts/pr-workflow-parallelism.test.mjs b/config/scripts/pr-workflow-parallelism.test.mjs index e934d1d836d..536059fe4cc 100644 --- a/config/scripts/pr-workflow-parallelism.test.mjs +++ b/config/scripts/pr-workflow-parallelism.test.mjs @@ -15,6 +15,7 @@ const shellContractFiles = [ 'src/main/providers/local-pty-shell-ready-zsh-zdotdir-discovery.test.ts', 'src/main/providers/local-pty-shell-ready-zsh-zdotdir-normalization.test.ts', 'src/main/providers/__tests__/shell-ready-framework-example.test.ts', + 'src/main/zsh-scoped-histfile.live-shell.test.ts', 'src/shared/posix-command-path-lookup.test.ts' ] const patchedNodePtyContractFiles = [ diff --git a/src/main/daemon/daemon-pty-adapter.ts b/src/main/daemon/daemon-pty-adapter.ts index aad29987053..7d2063e2a7d 100644 --- a/src/main/daemon/daemon-pty-adapter.ts +++ b/src/main/daemon/daemon-pty-adapter.ts @@ -66,6 +66,9 @@ import { recognizeAgentProcessFromCommandLine } from '../../shared/agent-process import { shouldUseShellReadyStartupDelivery } from '../../shared/codex-startup-delivery' import type { PtyIncarnationId } from '../../shared/pty-incarnation' import { resolveSafePtyDefaultCwd } from '../providers/pty-default-cwd' +import { resolveUnixShellPath } from '../providers/local-pty-utils' +import { injectHistoryEnv, injectWslFishHistoryEnv, logHistoryInjection } from '../terminal-history' +import { addWslEnvKeys } from '../wsl-env' import { PtyWriteUnavailableError } from '../providers/pty-write-unavailable-error' import { ColdRestorePayloadCache, type ColdRestorePayload } from './cold-restore-payload-cache' import { CheckpointSessionQueue } from './daemon-checkpoint-session-queue' @@ -425,7 +428,8 @@ export class DaemonPtyAdapter implements IPtyProvider { } async spawn(opts: PtySpawnOptions): Promise { - const sessionId = opts.sessionId ?? mintPtySessionId(opts.worktreeId) + const spawnOpts = this.withHistoryIsolation(opts) + const sessionId = spawnOpts.sessionId ?? mintPtySessionId(spawnOpts.worktreeId) const operation = { exitsBySessionId: new Map(), ignoredExitIncarnationIds: new Set(), @@ -444,7 +448,9 @@ export class DaemonPtyAdapter implements IPtyProvider { } try { return await this.withHistorySpawnLock(sessionId, () => - this.withDaemonRetry(() => this.doSpawn({ ...opts, sessionId }, operation, historyRecovery)) + this.withDaemonRetry(() => + this.doSpawn({ ...spawnOpts, sessionId }, operation, historyRecovery) + ) ) } finally { if (historyRecovery.freeze) { @@ -458,6 +464,44 @@ export class DaemonPtyAdapter implements IPtyProvider { } } + private withHistoryIsolation(opts: PtySpawnOptions): PtySpawnOptions { + const wslContext = resolveWslSessionContext({ + cwd: opts.cwd, + sessionId: opts.sessionId, + shellOverride: opts.shellOverride, + terminalWindowsWslDistro: opts.terminalWindowsWslDistro + }) + if ( + opts.attachOnly === true || + (opts.sessionId !== undefined && opts.isNewSession !== true) || + !opts.worktreeId || + opts.historyIsolationEnabled !== true || + (process.platform === 'win32' && !wslContext) + ) { + return opts + } + const env = { ...opts.env } + const preferredShell = wslContext + ? 'bash' + : opts.shellOverride || env.SHELL || process.env.SHELL || '/bin/zsh' + const shellPath = resolveUnixShellPath(preferredShell) + const historyArgs = [ + env, + opts.worktreeId, + shellPath, + opts.cwd ?? resolveSafePtyDefaultCwd() + ] as const + const result = wslContext + ? injectHistoryEnv(...historyArgs, { wslDistro: wslContext.distro }) + : injectHistoryEnv(...historyArgs) + if (wslContext) { + injectWslFishHistoryEnv(env, opts.worktreeId, wslContext.distro) + addWslEnvKeys(env, ['HISTFILE', 'fish_history']) + } + logHistoryInjection(opts.worktreeId, result) + return { ...opts, env } + } + private async doSpawn( opts: PtySpawnOptions, operation: PendingDaemonSpawnOperation, diff --git a/src/main/daemon/daemon-zsh-shell-ready-rcfile.ts b/src/main/daemon/daemon-zsh-shell-ready-rcfile.ts index 4e5d4242d81..1b436bb1ea5 100644 --- a/src/main/daemon/daemon-zsh-shell-ready-rcfile.ts +++ b/src/main/daemon/daemon-zsh-shell-ready-rcfile.ts @@ -1,6 +1,10 @@ import { getPosixOmpShellWrapper } from '../pty/omp-shell-wrapper' import { getPosixCodexShellLaunchPreflight } from '../pty/codex-shell-launch-preflight' -import { getZshFinalZdotdirRestoreBlock, getZshStartupFileSourceBlock } from '../shell-templates' +import { + getZshFinalZdotdirRestoreBlock, + getZshStartupFileSourceBlock, + ZSH_HISTFILE_RESTORE_BLOCK +} from '../shell-templates' export function getDaemonZshShellReadyRcfileContent(): string { return `# Orca daemon zsh shell-ready wrapper @@ -23,6 +27,7 @@ if [[ ! -o login ]]; then [[ -n "\${ORCA_MIMOCODE_HOME:-}" ]] && export MIMOCODE_HOME="\${ORCA_MIMOCODE_HOME}" ${getPosixOmpShellWrapper()} [[ -n "\${ORCA_CODEX_HOME:-}" ]] && export CODEX_HOME="\${ORCA_CODEX_HOME}" +${ZSH_HISTFILE_RESTORE_BLOCK} ${getPosixCodexShellLaunchPreflight()} fi __orca_osc133_precmd() { diff --git a/src/main/daemon/shell-ready.ts b/src/main/daemon/shell-ready.ts index 7989401e4d0..0f9b2d65885 100644 --- a/src/main/daemon/shell-ready.ts +++ b/src/main/daemon/shell-ready.ts @@ -16,7 +16,8 @@ import { getZshEnvTemplate, getZshFinalZdotdirRestoreBlock, getZshShellReadyMarkerRegistrationBlock, - getZshStartupFileSourceBlock + getZshStartupFileSourceBlock, + ZSH_HISTFILE_RESTORE_BLOCK } from '../shell-templates' import { SHELL_READY_MARKER } from './daemon-shell-ready-marker' import { getDaemonBashShellReadyRcfileContent } from './daemon-bash-shell-ready-rcfile' @@ -121,6 +122,7 @@ __orca_restore_agent_teams_path [[ -n "\${ORCA_MIMOCODE_HOME:-}" ]] && export MIMOCODE_HOME="\${ORCA_MIMOCODE_HOME}" ${getPosixOmpShellWrapper()} [[ -n "\${ORCA_CODEX_HOME:-}" ]] && export CODEX_HOME="\${ORCA_CODEX_HOME}" +${ZSH_HISTFILE_RESTORE_BLOCK} ${getPosixCodexShellLaunchPreflight()} ${getZshShellReadyMarkerRegistrationBlock(SHELL_READY_MARKER)} ${getZshFinalZdotdirRestoreBlock()} diff --git a/src/main/fish-history-session.test.ts b/src/main/fish-history-session.test.ts new file mode 100644 index 00000000000..060c1f2e57b --- /dev/null +++ b/src/main/fish-history-session.test.ts @@ -0,0 +1,238 @@ +import { existsSync, mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { + deleteFishHistoryFile, + fishHistorySessionName, + isSafeFishHistorySession, + relayFishHistorySessionName, + resolveFishHistoryDir, + sweepOrphanedFishHistoryFiles +} from './fish-history-session' + +const HASH_A = 'a1b2c3d4e5f60718' +const HASH_B = '00112233445566ff' +const SESSION_A = fishHistorySessionName(HASH_A) + +describe('fish history session naming', () => { + it('mints a session name that is a valid fish variable value', () => { + // fish falls back to the shared default for anything that is not a valid + // variable name, which would silently un-isolate the worktree. + expect(SESSION_A).toBe('orca_a1b2c3d4e5f60718') + expect(SESSION_A).toMatch(/^[A-Za-z_][A-Za-z0-9_]*$/) + }) + + it.each([ + ['orca_deadbeef', true], + ['orca_', false], + ['fish_history', false], + ['orca_../../etc/passwd', false], + ['orca_DEADBEEF', false], + ['', false], + [undefined, false], + [42, false] + ])('accepts %s as a safe session name: %s', (value, expected) => { + expect(isSafeFishHistorySession(value)).toBe(expected) + }) +}) + +describe('fish history directory resolution', () => { + // Why join rather than literals: the separator is platform-specific, and on + // Windows `isAbsolute('/data')` is also true, so a hardcoded '/data/fish' + // fails there for a reason that has nothing to do with the behavior tested. + it('follows XDG_DATA_HOME when it is absolute', () => { + expect(resolveFishHistoryDir({ XDG_DATA_HOME: '/data', HOME: '/home/me' })).toBe( + join('/data', 'fish') + ) + }) + + it('ignores a relative XDG_DATA_HOME the way fish does', () => { + expect(resolveFishHistoryDir({ XDG_DATA_HOME: 'relative', HOME: '/home/me' })).toBe( + join('/home/me', '.local', 'share', 'fish') + ) + }) + + it('falls back to HOME when XDG_DATA_HOME is unset', () => { + expect(resolveFishHistoryDir({ HOME: '/home/me' })).toBe( + join('/home/me', '.local', 'share', 'fish') + ) + }) +}) + +describe('fish history deletion', () => { + let root: string + let fishDir: string + + beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'orca-fish-history-')) + fishDir = join(root, 'fish') + mkdirSync(fishDir, { recursive: true }) + }) + + afterEach(() => { + rmSync(root, { recursive: true, force: true }) + }) + + const historyFile = (session: string): string => join(fishDir, `${session}_history`) + + it('deletes the session file rather than truncating it', () => { + writeFileSync(historyFile(SESSION_A), '- cmd: ls\n') + + expect(deleteFishHistoryFile(SESSION_A, [fishDir])).toBe(true) + expect(existsSync(historyFile(SESSION_A))).toBe(false) + }) + + it('tries every candidate directory, since the PTY may not share this env', () => { + const other = join(root, 'other-xdg', 'fish') + mkdirSync(other, { recursive: true }) + writeFileSync(join(other, `${SESSION_A}_history`), '- cmd: ls\n') + + expect(deleteFishHistoryFile(SESSION_A, [fishDir, other])).toBe(true) + expect(existsSync(join(other, `${SESSION_A}_history`))).toBe(false) + }) + + it('refuses a session name it did not mint', () => { + const foreign = join(fishDir, 'fish_history') + writeFileSync(foreign, 'the user’s real history\n') + + expect(deleteFishHistoryFile('fish_history', [fishDir])).toBe(false) + expect(existsSync(foreign)).toBe(true) + }) + + // Why skipped on Windows: symlinkSync needs Developer Mode or elevation there. + it.skipIf(process.platform === 'win32')( + 'refuses to follow a symlink out of the fish data dir', + () => { + const outside = join(root, 'precious.txt') + writeFileSync(outside, 'keep me\n') + symlinkSync(outside, historyFile(SESSION_A)) + + expect(deleteFishHistoryFile(SESSION_A, [fishDir])).toBe(false) + expect(existsSync(outside)).toBe(true) + } + ) + + it('reports false when there is nothing to delete', () => { + expect(deleteFishHistoryFile(SESSION_A, [fishDir])).toBe(false) + }) +}) + +describe('orphaned fish history sweep', () => { + let root: string + let fishDir: string + + beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'orca-fish-sweep-')) + fishDir = join(root, 'fish') + mkdirSync(fishDir, { recursive: true }) + }) + + afterEach(() => { + rmSync(root, { recursive: true, force: true }) + }) + + it('removes files for dead worktrees and keeps live ones', () => { + writeFileSync(join(fishDir, `${fishHistorySessionName(HASH_A)}_history`), 'live\n') + writeFileSync(join(fishDir, `${fishHistorySessionName(HASH_B)}_history`), 'dead\n') + + expect(sweepOrphanedFishHistoryFiles(new Set([HASH_A]), [fishDir])).toBe(1) + expect(existsSync(join(fishDir, `${fishHistorySessionName(HASH_A)}_history`))).toBe(true) + expect(existsSync(join(fishDir, `${fishHistorySessionName(HASH_B)}_history`))).toBe(false) + }) + + it("never touches the user's own history files", () => { + // The whole safety of the sweep rests on the orca__ prefix. + const untouched = ['fish_history', 'work_history', 'orca_history', 'orca_nothex_history'] + for (const name of untouched) { + writeFileSync(join(fishDir, name), 'mine\n') + } + + expect(sweepOrphanedFishHistoryFiles(new Set([HASH_A]), [fishDir])).toBe(0) + for (const name of untouched) { + expect(existsSync(join(fishDir, name))).toBe(true) + } + }) + + it('refuses to sweep on an empty live set, which cannot be told from a cold store', () => { + writeFileSync(join(fishDir, `${fishHistorySessionName(HASH_B)}_history`), 'dead\n') + + expect(sweepOrphanedFishHistoryFiles(new Set(), [fishDir])).toBe(0) + expect(existsSync(join(fishDir, `${fishHistorySessionName(HASH_B)}_history`))).toBe(true) + }) + + it('spares a file younger than the age guard, since the live set is a snapshot', () => { + // The reachable race: a worktree created after the snapshot but before the + // sweep looks dead while it is actively writing the history it owns. + const fresh = join(fishDir, `${fishHistorySessionName(HASH_B)}_history`) + writeFileSync(fresh, 'just created\n') + + expect(sweepOrphanedFishHistoryFiles(new Set([HASH_A]), [fishDir], 5 * 60 * 1000)).toBe(0) + expect(existsSync(fresh)).toBe(true) + }) + + it('sweeps a file older than the age guard', () => { + const stale = join(fishDir, `${fishHistorySessionName(HASH_B)}_history`) + writeFileSync(stale, 'long dead\n') + const wellPast = Date.now() + 60 * 60 * 1000 + + expect( + sweepOrphanedFishHistoryFiles(new Set([HASH_A]), [fishDir], 5 * 60 * 1000, wellPast) + ).toBe(1) + expect(existsSync(stale)).toBe(false) + }) + + it('tolerates a directory that does not exist', () => { + expect(sweepOrphanedFishHistoryFiles(new Set([HASH_A]), [join(root, 'absent')])).toBe(0) + }) + + it('sweeps every candidate directory once', () => { + const other = join(root, 'other', 'fish') + mkdirSync(other, { recursive: true }) + for (const dir of [fishDir, other]) { + writeFileSync(join(dir, `${fishHistorySessionName(HASH_B)}_history`), 'dead\n') + } + + 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 new file mode 100644 index 00000000000..6e76f64dd33 --- /dev/null +++ b/src/main/fish-history-session.ts @@ -0,0 +1,139 @@ +import { lstatSync, readdirSync, rmSync } from 'node:fs' +import { homedir } from 'node:os' +import { isAbsolute, join } from 'node:path' + +/** + * Per-worktree fish history. + * + * fish ignores HISTFILE entirely — history lives at `/_history` + * where `` comes only from the `fish_history` variable, and there is no + * custom-directory knob. So the session NAME is the only isolation lever, and + * the file necessarily lands in the user's fish data dir rather than inside + * Orca's history tree. That is why deletion needs its own path here instead of + * riding the tree tombstone the way bash/zsh history does. + * + * `XDG_DATA_HOME` is deliberately not redirected: that would move every other + * tool's data for the session, not just fish's history. + */ +const SESSION_PREFIX = 'orca_' +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 { + return typeof session === 'string' && SAFE_SESSION_NAME.test(session) +} + +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. */ +export function resolveFishHistoryDir(env: NodeJS.ProcessEnv = process.env): string { + const xdg = env.XDG_DATA_HOME?.trim() + const dataHome = + xdg && isAbsolute(xdg) ? xdg : join(env.HOME?.trim() || homedir(), '.local', 'share') + return join(dataHome, 'fish') +} + +/** Removes one history file, refusing anything that is not a regular file. + * Why lstat: a symlink here would take `rmSync` out of the fish data dir. */ +function removeHistoryFile(path: string): boolean { + try { + if (!lstatSync(path).isFile()) { + return false + } + rmSync(path) + return true + } catch { + return false + } +} + +/** Delete one worktree's fish history file, trying every directory it could be in. + * Callers pass the directory recorded at spawn time plus this process's own, since + * the two disagree whenever Orca's environment differs from the PTY's. */ +export function deleteFishHistoryFile(session: string, dirs: Iterable): boolean { + if (!isSafeFishHistorySession(session)) { + return false + } + let removed = false + for (const dir of new Set(dirs)) { + removed = removeHistoryFile(join(dir, `${session}_history`)) || removed + } + return removed +} + +/** + * Delete every Orca-minted fish history file whose worktree is gone. + * + * Why a sweep and not just per-worktree deletion: the history file outlives the + * directory that names it. A crash between tombstone and removal, a hand-deleted + * history dir, or a file written by an older build all orphan one, and only a + * scan of the data dir can find those. Matching on Orca's own `orca__` + * prefix means the user's real `fish_history` is never a candidate. + * + * `minAgeMs` mirrors the age guard the history-tree GC already applies: the + * live-worktree set is a snapshot, so a worktree created between the snapshot + * and this sweep would otherwise look dead and lose the history it is actively + * writing. A live session's file keeps a recent mtime, so this doubles as a + * second check on the set itself. + * + * Refuses to run at all on an empty live set: that cannot be told apart from a + * store that failed to hydrate, and unlike the tree GC this deletes files + * outside Orca's own directory. + */ +export function sweepOrphanedFishHistoryFiles( + liveWorktreeHashes: ReadonlySet, + dirs: Iterable, + minAgeMs = 0, + now = Date.now() +): number { + if (liveWorktreeHashes.size === 0) { + return 0 + } + let removed = 0 + for (const dir of new Set(dirs)) { + let entries: string[] + try { + entries = readdirSync(dir) + } catch { + continue + } + for (const entry of entries) { + const hash = ORCA_HISTORY_FILE.exec(entry)?.[1] + if (!hash || liveWorktreeHashes.has(hash)) { + continue + } + const path = join(dir, entry) + if (minAgeMs > 0 && !isOlderThan(path, minAgeMs, now)) { + continue + } + if (removeHistoryFile(path)) { + removed += 1 + } + } + } + return removed +} + +/** Treats an unreadable file as too young: never delete on a failed stat. */ +function isOlderThan(path: string, minAgeMs: number, now: number): boolean { + try { + return now - lstatSync(path).mtimeMs >= minAgeMs + } catch { + return false + } +} diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index ba1255c058e..376b705ef05 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -4762,7 +4762,8 @@ export function registerPtyHandlers( rows: args.rows, cwd, env, - ...(isNewDaemonSession ? { isNewSession: true } : {}) + ...(isNewDaemonSession ? { isNewSession: true } : {}), + historyIsolationEnabled: getSettings?.()?.terminalScopeHistoryByWorktree ?? true } if (!args.connectionId && !isDaemonHostSpawn) { spawnOptions.codexHomePathOverride = { value: selectedCodexHomePath } @@ -6479,7 +6480,8 @@ export function registerPtyHandlers( cwd, ...(prevalidatedCwd && !isDaemonHostSpawn ? { prevalidatedCwd } : {}), env: spawnEnv, - ...(isMintedSessionId ? { isNewSession: true } : {}) + ...(isMintedSessionId ? { isNewSession: true } : {}), + historyIsolationEnabled: getSettings?.()?.terminalScopeHistoryByWorktree ?? true } if (!args.connectionId && !isDaemonHostSpawn) { spawnOptions.codexHomePathOverride = { value: selectedCodexHomePath } diff --git a/src/main/ipc/worktrees.ts b/src/main/ipc/worktrees.ts index beebaf70058..c7ec109e2e6 100644 --- a/src/main/ipc/worktrees.ts +++ b/src/main/ipc/worktrees.ts @@ -265,6 +265,7 @@ import { removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval, recoverLocalWindowsWorktreeRemoval } from '../local-worktree-removal-recovery' +import { deleteRemoteWorktreeHistory } from '../remote-worktree-history-cleanup' const NullableWorkspaceLinkedItemSchema = WorkspaceLinkedItemSchema.nullable() const NullableTaskSourceContextSchema = TaskSourceContextSchema.nullable() @@ -2493,15 +2494,15 @@ export function registerWorktreeHandlers( 'Cannot delete the project root workspace. Remove the folder project instead.' ) } + const ownerHost = parseExecutionHostId(removalHostId) + const sshPtyProvider = + ownerHost?.kind === 'ssh' ? getSshPtyProvider(ownerHost.targetId) : undefined // Why: folder workspaces share one root, so there's no Git remove step to close shells; sweep PTYs before dropping metadata. await withWorktreeRemoveStageSpan('pty_sweep', 'folder', async () => { // Folder projects can be SSH-backed, so fence the sweep to the owning host exactly // like the git paths — the local inventory must never reach a remote workspace's id. // The resolved repo is authoritative here: path-derived metadata is shared by // same-id host copies and can describe a different owner's workspace. - const ownerHost = parseExecutionHostId(removalHostId) - const sshPtyProvider = - ownerHost?.kind === 'ssh' ? getSshPtyProvider(ownerHost.targetId) : undefined const externalHost = ownerHost?.kind === 'ssh' || ownerHost?.kind === 'runtime' await killAllProcessesForWorktree(args.worktreeId, { runtime, @@ -2524,6 +2525,7 @@ export function registerWorktreeHandlers( }) }) await withWorktreeRemoveStageSpan('metadata_purge', 'folder', async () => { + await deleteRemoteWorktreeHistory(sshPtyProvider, args.worktreeId) removeWorktreeMetadataAndTransientState( store, args.worktreeId, @@ -2612,6 +2614,14 @@ export function registerWorktreeHandlers( } finally { await removalGate.finish(removalCompleted) } + // Why history first: the worktree is already gone from git and + // disk by here, so a rejecting push-target cleanup must not be + // able to skip history removal and leave the user's commands on + // the remote host. + await deleteRemoteWorktreeHistory( + getSshPtyProvider(repo.connectionId), + args.worktreeId + ) await cleanupUnusedWorktreePushTargetRemoteSsh( provider!, repo.path, @@ -2720,6 +2730,14 @@ export function registerWorktreeHandlers( } // Why: a manually deleted worktree is already gone; persisted metadata proves it was an Orca-known row, so no force is needed. if (repo.connectionId) { + // Why history first: the worktree is already gone from git and + // disk by here, so a rejecting push-target cleanup must not be + // able to skip history removal and leave the user's commands on + // the remote host. + await deleteRemoteWorktreeHistory( + getSshPtyProvider(repo.connectionId), + args.worktreeId + ) await cleanupUnusedWorktreePushTargetRemoteSsh( provider!, repo.path, @@ -2898,6 +2916,10 @@ export function registerWorktreeHandlers( removedPushTarget, store ) + await deleteRemoteWorktreeHistory( + getSshPtyProvider(remoteConnectionId), + args.worktreeId + ) rememberPreservedBranchCleanupTarget( args.worktreeId, removalHostId, diff --git a/src/main/providers/local-pty-provider.ts b/src/main/providers/local-pty-provider.ts index af0e17656bd..cf98ca7cf0b 100644 --- a/src/main/providers/local-pty-provider.ts +++ b/src/main/providers/local-pty-provider.ts @@ -19,8 +19,10 @@ import { splitWorktreeIdForFilesystem } from '../../shared/worktree/id' import { isBracketedPasteSafeShell } from '../../shared/startup-command-submission' import { injectHistoryEnv, - updateHistFileForFallback, - logHistoryInjection + injectWslFishHistoryEnv, + updateHistoryEnvForFallback, + logHistoryInjection, + type HistoryInjectionResult } from '../terminal-history' import type { IPtyProvider, PtyProcessInfo, PtySpawnOptions, PtySpawnResult } from './types' import { @@ -872,7 +874,15 @@ export class LocalPtyProvider implements IPtyProvider { historyResult = injectHistoryEnv(finalEnv, worktreeId, effectiveShellPath, cwd, { wslDistro: launchWslDistro }) + if (isWslTerminal && launchWslDistro) { + injectWslFishHistoryEnv(finalEnv, worktreeId, launchWslDistro) + addWslEnvKeys(finalEnv, ['HISTFILE', 'fish_history']) + } logHistoryInjection(worktreeId, historyResult) + } else { + // Why: injectHistoryEnv is what normally clears it, so when history is off + // an inherited ORCA_HISTFILE would still reach the wrapper. Credit: #11146. + delete finalEnv.ORCA_HISTFILE } await prepareLocalPtySpawn(id) @@ -895,8 +905,9 @@ export class LocalPtyProvider implements IPtyProvider { ptySpawn: pty.spawn, getShellReadyConfig: getFallbackShellReadyConfig, // Why: on zsh→bash fallback HISTFILE still points to zsh_history; update before spawn so the child inherits it (design doc §8). - onBeforeFallbackSpawn: historyResult?.histFile - ? (env, fallbackShell) => updateHistFileForFallback(env, fallbackShell) + onBeforeFallbackSpawn: historyResult?.historyDir + ? (env, fallbackShell) => + updateHistoryEnvForFallback(env, fallbackShell, historyResult as HistoryInjectionResult) : undefined, windowsFallbackAttempts }) diff --git a/src/main/providers/local-pty-shell-ready-wrapper-generation.ts b/src/main/providers/local-pty-shell-ready-wrapper-generation.ts index 29863d0a169..7c32a283aa2 100644 --- a/src/main/providers/local-pty-shell-ready-wrapper-generation.ts +++ b/src/main/providers/local-pty-shell-ready-wrapper-generation.ts @@ -11,7 +11,8 @@ import { getZshEnvTemplate, getZshFinalZdotdirRestoreBlock, getZshShellReadyMarkerRegistrationBlock, - getZshStartupFileSourceBlock + getZshStartupFileSourceBlock, + ZSH_HISTFILE_RESTORE_BLOCK } from '../shell-templates' import { getBashShellReadyRcfileContent } from './local-pty-shell-ready-bash-rcfile' import { @@ -44,6 +45,7 @@ if [[ ! -o login ]]; then ${getPosixOmpShellWrapper()} # Why: Codex must keep using Orca's runtime CODEX_HOME after rc files. [[ -n "\${ORCA_CODEX_HOME:-}" ]] && export CODEX_HOME="\${ORCA_CODEX_HOME}" +${ZSH_HISTFILE_RESTORE_BLOCK} ${getPosixCodexShellLaunchPreflight()} fi __orca_osc133_precmd() { @@ -96,6 +98,7 @@ __orca_restore_agent_teams_path [[ -n "\${ORCA_MIMOCODE_HOME:-}" ]] && export MIMOCODE_HOME="\${ORCA_MIMOCODE_HOME}" ${getPosixOmpShellWrapper()} [[ -n "\${ORCA_CODEX_HOME:-}" ]] && export CODEX_HOME="\${ORCA_CODEX_HOME}" +${ZSH_HISTFILE_RESTORE_BLOCK} ${getPosixCodexShellLaunchPreflight()} ${getZshShellReadyMarkerRegistrationBlock(SHELL_READY_MARKER_ESCAPED)} ${getZshFinalZdotdirRestoreBlock()} diff --git a/src/main/providers/pty-provider-contract.ts b/src/main/providers/pty-provider-contract.ts index 43bbb15a125..3ebdb6827fb 100644 --- a/src/main/providers/pty-provider-contract.ts +++ b/src/main/providers/pty-provider-contract.ts @@ -70,6 +70,8 @@ export type PtySpawnOptions = { * Existing-session attach paths must stay false so recovery checks do not * replace the daemon out from under a still-live PTY. */ isNewSession?: boolean + /** Host setting forwarded additively to the process owner; old owners ignore it. */ + historyIsolationEnabled?: boolean /** Attach the named session atomically or fail without creating a process. */ attachOnly?: boolean /** Exact persisted owner expected by an attach-only routing decision. */ @@ -119,6 +121,8 @@ export type IPtyProvider = { /** Re-probes a degraded durable host before main commits to fallback spawn semantics. */ recoverFreshSpawnRouting?: () => Promise spawn(opts: PtySpawnOptions): Promise + /** Process-owner cleanup for history stored outside the workspace tree. */ + deleteWorktreeHistory?: (worktreeId: string) => Promise /** Whether this spawn target can append the Git guard after its final env merge. */ supportsGitCredentialGuardHost?: (sessionId?: string) => boolean /** Explicit false selects pre-claim legacy spawn for a preserved old daemon. */ diff --git a/src/main/providers/ssh-pty-provider.test.ts b/src/main/providers/ssh-pty-provider.test.ts index 40301906f52..b3b64be7a71 100644 --- a/src/main/providers/ssh-pty-provider.test.ts +++ b/src/main/providers/ssh-pty-provider.test.ts @@ -369,14 +369,16 @@ describe('SshPtyProvider', () => { }) }) - it('forwards explicit shellOverride and terminalWindowsWslDistro to the relay mux', async () => { + it('forwards shell, WSL, and scoped-history options to the relay mux', async () => { mux.request.mockResolvedValue({ id: 'pty-2' }) await provider.spawn({ cols: 120, rows: 40, shellOverride: 'powershell.exe', - terminalWindowsWslDistro: 'Ubuntu' + terminalWindowsWslDistro: 'Ubuntu', + worktreeId: 'repo-1::/remote/wt', + historyIsolationEnabled: true }) expectRequest(mux.request, 'pty.spawn', { @@ -385,7 +387,9 @@ describe('SshPtyProvider', () => { cwd: undefined, env: { [POWERLEVEL10K_WIZARD_DISABLE_ENV]: 'true' }, shellOverride: 'powershell.exe', - terminalWindowsWslDistro: 'Ubuntu' + terminalWindowsWslDistro: 'Ubuntu', + worktreeId: 'repo-1::/remote/wt', + historyIsolationEnabled: true }) }) diff --git a/src/main/providers/ssh-pty-provider.ts b/src/main/providers/ssh-pty-provider.ts index eb24ffdbb8b..99df854f4d0 100644 --- a/src/main/providers/ssh-pty-provider.ts +++ b/src/main/providers/ssh-pty-provider.ts @@ -14,7 +14,7 @@ import { spawnFreshSshPty } from './ssh-agent-session-create-operation' import { mapSshPtyProcessList } from './ssh-agent-session-process-list' import { requestSshPtyAttach, - reattachSshPtySessionWithExitFence, + reattachSshPtySessionForSpawn, type PtySourceRecoveryRequest, type SshPtyAttachResult } from './ssh-pty-session-reattach' @@ -22,7 +22,6 @@ import { buildSshPtySpawnRequest } from './ssh-pty-spawn-request' import { SshPtySpawnExitRaceTracker } from './ssh-pty-spawn-exit-race' import { SshAgentSessionCapabilities } from './ssh-agent-session-capabilities' import type { PtyProcessInspection } from './pty-process-inspection' -import { SSH_SESSION_EXPIRED_ERROR } from './ssh-pty-errors' import { writeToSshPty, writeToSshPtyWithSettlement } from './ssh-pty-write' // Why: sequential relay teardown calls share one absolute budget; convert to the mux-relative timeout only at dispatch. @@ -91,36 +90,18 @@ export class SshPtyProvider implements IPtyProvider { } } if (opts.sessionId) { - let result: Awaited> | undefined - try { - result = await reattachSshPtySessionWithExitFence({ - mux: this.mux, - connectionId: this.connectionId, - sessionId: opts.sessionId, - options: opts, - exitRaceTracker: this.spawnExitRaces, - installSourceActivation: (relayPtyId, activation) => - this.outputState.installReceivingActivation(relayPtyId, activation), - rememberPtyIncarnation: (relayPtyId, incarnationId) => - this.outputState.rememberPtyIncarnation(relayPtyId, incarnationId) - }) - if (result.sourceRecovery?.status === 'restoreRequired') { - throw new Error( - `${SSH_SESSION_EXPIRED_ERROR}: ${toRelaySshPtyId(this.connectionId, result.id)}` - ) - } - this.livePtyIds.add(result.id) - result.sourceActivationLease?.commit() - const { - sourceActivationLease: _lease, - sourceRecovery: _sourceRecovery, - ...spawnResult - } = result - return spawnResult - } catch (error) { - result?.sourceActivationLease?.rollback() - throw error - } + return await reattachSshPtySessionForSpawn({ + mux: this.mux, + connectionId: this.connectionId, + sessionId: opts.sessionId, + options: opts, + exitRaceTracker: this.spawnExitRaces, + installSourceActivation: (relayPtyId, activation) => + this.outputState.installReceivingActivation(relayPtyId, activation), + rememberPtyIncarnation: (relayPtyId, incarnationId) => + this.outputState.rememberPtyIncarnation(relayPtyId, incarnationId), + acceptLivePty: (relayPtyId) => this.livePtyIds.add(relayPtyId) + }) } const supportsCreateOperation = opts.agentSessionCreateOperationId @@ -151,6 +132,10 @@ export class SshPtyProvider implements IPtyProvider { }) } + async deleteWorktreeHistory(worktreeId: string): Promise { + await this.mux.request('pty.deleteWorktreeHistory', { worktreeId }) + } + async supportsAgentSessionClaims(options: { signal?: AbortSignal } = {}): Promise { return await this.agentSessionCapabilities.supportsClaims(options) } diff --git a/src/main/providers/ssh-pty-session-reattach.ts b/src/main/providers/ssh-pty-session-reattach.ts index 34974940caf..06e4c9ae7c5 100644 --- a/src/main/providers/ssh-pty-session-reattach.ts +++ b/src/main/providers/ssh-pty-session-reattach.ts @@ -255,3 +255,39 @@ export async function reattachSshPtySessionWithExitFence( args.exitRaceTracker.finish(operation) } } + +/** + * The full reattach path a spawn takes when it carries a sessionId: fence the + * exit race, reject a session the relay can no longer restore, and commit or + * roll back the source-activation lease. + * + * Lives here rather than in SshPtyProvider.spawn so the lease's commit and + * rollback stay in one place — a caller that only wrapped the fence could + * return without committing and silently leak the activation. + */ +export async function reattachSshPtySessionForSpawn( + args: Parameters[0] & { + acceptLivePty: (relayPtyId: string) => void + } +): Promise { + let result: SshPtyReattachResult | undefined + try { + result = await reattachSshPtySessionWithExitFence(args) + if (result.sourceRecovery?.status === 'restoreRequired') { + throw new Error( + `${SSH_SESSION_EXPIRED_ERROR}: ${toRelaySshPtyId(args.connectionId, result.id)}` + ) + } + args.acceptLivePty(result.id) + result.sourceActivationLease?.commit() + const { + sourceActivationLease: _lease, + sourceRecovery: _sourceRecovery, + ...spawnResult + } = result + return spawnResult + } catch (error) { + result?.sourceActivationLease?.rollback() + throw error + } +} diff --git a/src/main/providers/ssh-pty-spawn-request.ts b/src/main/providers/ssh-pty-spawn-request.ts index 889c0813ee9..9f493d9e778 100644 --- a/src/main/providers/ssh-pty-spawn-request.ts +++ b/src/main/providers/ssh-pty-spawn-request.ts @@ -22,6 +22,10 @@ export function buildSshPtySpawnRequest(args: { // Why: the relay needs launch identity for plugin env overlays and provider-side delivery. ...(options.command ? { command: options.command } : {}), ...(options.launchAgent ? { launchAgent: options.launchAgent } : {}), + ...(options.worktreeId ? { worktreeId: options.worktreeId } : {}), + ...(options.historyIsolationEnabled !== undefined + ? { historyIsolationEnabled: options.historyIsolationEnabled } + : {}), ...(options.shellOverride !== undefined ? { shellOverride: options.shellOverride } : {}), ...(options.terminalWindowsWslDistro !== undefined ? { terminalWindowsWslDistro: options.terminalWindowsWslDistro } diff --git a/src/main/remote-worktree-history-cleanup.test.ts b/src/main/remote-worktree-history-cleanup.test.ts new file mode 100644 index 00000000000..ce0f23a60f9 --- /dev/null +++ b/src/main/remote-worktree-history-cleanup.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it, vi } from 'vitest' +import { deleteRemoteWorktreeHistory } from './remote-worktree-history-cleanup' + +describe('deleteRemoteWorktreeHistory', () => { + it('repeats idempotent cleanup through the PTY owner', async () => { + const deleteWorktreeHistory = vi.fn().mockResolvedValue(undefined) + const provider = { deleteWorktreeHistory } as never + + await deleteRemoteWorktreeHistory(provider, 'repo-1::/remote/wt') + await deleteRemoteWorktreeHistory(provider, 'repo-1::/remote/wt') + + expect(deleteWorktreeHistory).toHaveBeenCalledTimes(2) + }) + + it('degrades safely when an old relay does not expose cleanup', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const provider = { + deleteWorktreeHistory: vi.fn().mockRejectedValue(new Error('unknown request method')) + } as never + + try { + await expect( + deleteRemoteWorktreeHistory(provider, 'repo-1::/remote/wt') + ).resolves.toBeUndefined() + expect(warn).toHaveBeenCalledWith(expect.stringContaining('Remote cleanup unavailable')) + } finally { + warn.mockRestore() + } + }) +}) diff --git a/src/main/remote-worktree-history-cleanup.ts b/src/main/remote-worktree-history-cleanup.ts new file mode 100644 index 00000000000..8842758d15e --- /dev/null +++ b/src/main/remote-worktree-history-cleanup.ts @@ -0,0 +1,15 @@ +import type { IPtyProvider } from './providers/pty-provider-contract' + +/** Best-effort cleanup through the PTY owner; older relays may not expose the method. */ +export async function deleteRemoteWorktreeHistory( + provider: IPtyProvider | undefined, + worktreeId: string +): Promise { + try { + await provider?.deleteWorktreeHistory?.(worktreeId) + } catch (error) { + console.warn( + `[pty:history] Remote cleanup unavailable: ${error instanceof Error ? error.message : String(error)}` + ) + } +} diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 7bd76f0d316..17117479f80 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -6465,7 +6465,8 @@ describe('OrcaRuntimeService', () => { worktreeId: `${TEST_REPO_ID}::/remote/feature` } ]), - shutdown: vi.fn().mockResolvedValue(undefined) + shutdown: vi.fn().mockResolvedValue(undefined), + deleteWorktreeHistory: vi.fn().mockResolvedValue(undefined) } const runtime = new OrcaRuntimeService(remoteStore as never, undefined, { getSshProvider: () => ptyProvider as never @@ -6482,6 +6483,9 @@ describe('OrcaRuntimeService', () => { 'pty-remote', expect.objectContaining({ immediate: true }) ) + expect(ptyProvider.deleteWorktreeHistory).toHaveBeenCalledWith( + `${TEST_REPO_ID}::/remote/feature` + ) expect(ptyProvider.shutdown.mock.invocationCallOrder[0]).toBeLessThan( gitProvider.removeWorktree.mock.invocationCallOrder[0] ) @@ -33158,7 +33162,7 @@ describe('OrcaRuntimeService', () => { expect(spawn).not.toHaveBeenCalled() }) - it('uses POSIX quoting for mobile agent launch commands in WSL project runtimes', async () => { + it('uses portable Unix quoting for mobile agent launch commands in WSL project runtimes', async () => { await withPlatform('win32', async () => { const spawn = vi.fn().mockResolvedValue({ id: 'pty-agent' }) const runtime = new OrcaRuntimeService({ @@ -33196,7 +33200,7 @@ describe('OrcaRuntimeService', () => { expect(spawn).toHaveBeenCalledWith( expect.objectContaining({ - command: "command-code --profile mobile '--note' 'can'\\''t'", + command: `command-code --profile mobile '--note' 'can'"'"'t'`, cwd: TEST_WORKTREE_PATH, worktreeId: TEST_WORKTREE_ID }) @@ -48435,6 +48439,145 @@ describe('OrcaRuntimeService', () => { } }) + it('routes already-missing SSH runtime history cleanup through the PTY owner', async () => { + const repo = { + id: 'repo-runtime-ssh', + path: '/remote/repo', + displayName: 'ssh', + badgeColor: 'blue', + addedAt: 1, + connectionId: 'ssh-1' + } + const worktreeId = `${repo.id}::/remote/already-deleted` + const metaById: Record = { + [worktreeId]: makeWorktreeMeta({ hostId: 'ssh:ssh-1', orcaCreationSource: 'ssh' }) + } + const removeWorktreeMeta = vi.fn((id: string) => { + delete metaById[id] + }) + const runtimeStore = { + ...store, + getRepos: () => [repo], + getRepo: (id: string) => (id === repo.id ? repo : undefined), + getAllWorktreeMeta: () => metaById, + getWorktreeMeta: (id: string) => metaById[id], + removeWorktreeMeta + } + const gitProvider = { + listWorktrees: vi.fn().mockResolvedValue([ + { + path: repo.path, + head: 'main', + branch: 'refs/heads/main', + isBare: false, + isMainWorktree: true + } + ]) + } + const fsProvider = { + stat: vi.fn().mockRejectedValue(Object.assign(new Error('missing'), { code: 'ENOENT' })) + } + const deleteWorktreeHistory = vi.fn().mockResolvedValue(undefined) + const ptyProvider = { deleteWorktreeHistory } as never + registerSshGitProvider(repo.connectionId, gitProvider as never) + registerSshFilesystemProvider(repo.connectionId, fsProvider as never) + const runtime = new OrcaRuntimeService(runtimeStore as never, undefined, { + getSshProvider: () => ptyProvider + }) + + try { + await expect(runtime.removeManagedWorktree(`id:${worktreeId}`)).resolves.toEqual({}) + } finally { + unregisterSshGitProvider(repo.connectionId) + unregisterSshFilesystemProvider(repo.connectionId) + } + + expect(deleteWorktreeHistory).toHaveBeenCalledWith(worktreeId) + expect(deleteWorktreeHistory.mock.invocationCallOrder[0]).toBeLessThan( + removeWorktreeMeta.mock.invocationCallOrder[0] + ) + expect(removeWorktreeMeta).toHaveBeenCalledWith(worktreeId, 'ssh:ssh-1') + }) + + it('routes SSH runtime orphan-directory history cleanup through the PTY owner', async () => { + const repo = { + id: 'repo-runtime-ssh', + path: '/remote/repo', + displayName: 'ssh', + badgeColor: 'blue', + addedAt: 1, + connectionId: 'ssh-1' + } + const worktreePath = '/remote/orphan' + const worktreeId = `${repo.id}::${worktreePath}` + const metaById: Record = { + [worktreeId]: makeWorktreeMeta({ + hostId: 'ssh:ssh-1', + orcaCreatedAt: Date.now(), + orcaCreationSource: 'ssh' + }) + } + const removeWorktreeMeta = vi.fn((id: string) => { + delete metaById[id] + }) + const runtimeStore = { + ...store, + getRepos: () => [repo], + getRepo: (id: string) => (id === repo.id ? repo : undefined), + getAllWorktreeMeta: () => metaById, + getWorktreeMeta: (id: string) => metaById[id], + removeWorktreeMeta + } + const gitProvider = { + listWorktrees: vi.fn().mockResolvedValue([ + { + path: repo.path, + head: 'main', + branch: 'refs/heads/main', + isBare: false, + isMainWorktree: true + } + ]) + } + const fsProvider = { + lstat: vi.fn(async (path: string) => ({ + type: path === `${worktreePath}/.git` ? 'file' : 'directory' + })), + readFile: vi.fn(async (path: string) => ({ + isBinary: false, + content: + path === `${worktreePath}/.git` + ? `gitdir: ${repo.path}/.git/worktrees/orphan\n` + : `${worktreePath}/.git\n` + })), + deletePath: vi.fn().mockResolvedValue(undefined) + } + const deleteWorktreeHistory = vi.fn().mockResolvedValue(undefined) + const ptyProvider = { + listProcesses: vi.fn().mockResolvedValue([]), + shutdown: vi.fn().mockResolvedValue(undefined), + deleteWorktreeHistory + } + registerSshGitProvider(repo.connectionId, gitProvider as never) + registerSshFilesystemProvider(repo.connectionId, fsProvider as never) + const runtime = new OrcaRuntimeService(runtimeStore as never, undefined, { + getSshProvider: () => ptyProvider as never + }) + + try { + await expect(runtime.removeManagedWorktree(`id:${worktreeId}`, true)).resolves.toEqual({}) + } finally { + unregisterSshGitProvider(repo.connectionId) + unregisterSshFilesystemProvider(repo.connectionId) + } + + expect(fsProvider.deletePath).toHaveBeenCalledWith(worktreePath, true) + expect(deleteWorktreeHistory).toHaveBeenCalledWith(worktreeId) + expect(deleteWorktreeHistory.mock.invocationCallOrder[0]).toBeLessThan( + removeWorktreeMeta.mock.invocationCallOrder[0] + ) + }) + it('force-removes a legacy Orca-created runtime orphaned worktree directory after Git tracking is gone', async () => { const parentDir = await mkdtemp(join(tmpdir(), 'orca-runtime-orphan-')) const repoPath = join(parentDir, 'repo') diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 9cae4ee4798..fb490109950 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -1039,6 +1039,7 @@ import { resolveWorktreeSharedDirectories } from '../git/worktree-shared-directories' import { deleteWorktreeHistoryDir } from '../terminal-history-deletion' +import { deleteRemoteWorktreeHistory } from '../remote-worktree-history-cleanup' import { cleanupUnusedWorktreePushTargetRemote, cleanupUnusedWorktreePushTargetRemoteSsh, @@ -26204,6 +26205,7 @@ export class OrcaRuntimeService { .then((gate) => gate.finish(false)) .catch(() => {}) } + await deleteRemoteWorktreeHistory(sshPtyProvider, removalTarget.id) this.clearOptimisticReconcileToken(removalTarget.id) this.removeWorktreeMetadataAndHistory(store, removalTarget.id) this.preservedBranchCleanupByScope.delete(cleanupScopeKey) @@ -26248,6 +26250,7 @@ export class OrcaRuntimeService { console.warn(`[worktree-teardown] failed for ${removalTarget.id}:`, err) }) } + await deleteRemoteWorktreeHistory(folderSshPtyProvider, removalTarget.id) this.removeWorktreeMetadataAndHistory(store, removalTarget.id) this.preservedBranchCleanupByScope.delete(cleanupScopeKey) this.invalidateResolvedWorktreeCache() @@ -26332,6 +26335,10 @@ export class OrcaRuntimeService { removedPushTarget, store ) + await deleteRemoteWorktreeHistory( + this.getSshProviderFn?.(repo.connectionId), + removalTarget.id + ) } else { const removalGate = await this.acquireFileWatcherRemoval(removalTarget.path) let removalCompleted = false @@ -26437,6 +26444,12 @@ export class OrcaRuntimeService { store, localWorktreeGitOptions )) + if (repo.connectionId) { + await deleteRemoteWorktreeHistory( + this.getSshProviderFn?.(repo.connectionId), + removalTarget.id + ) + } this.clearOptimisticReconcileToken(removalTarget.id) this.removeWorktreeMetadataAndHistory(store, removalTarget.id) this.preservedBranchCleanupByScope.delete(cleanupScopeKey) @@ -26530,6 +26543,10 @@ export class OrcaRuntimeService { removedPushTarget, store ) + await deleteRemoteWorktreeHistory( + this.getSshProviderFn?.(repo.connectionId), + removalTarget.id + ) this.rememberPreservedBranchCleanupTarget( removalTarget.id, cleanupHostId, diff --git a/src/main/shell-templates.ts b/src/main/shell-templates.ts index 97288a2b971..8748303c5c6 100644 --- a/src/main/shell-templates.ts +++ b/src/main/shell-templates.ts @@ -103,6 +103,29 @@ unset _orca_spawn_orig_zdotdir _orca_user_zdotdir _orca_zshenv_source_dir _orca_ ` } +/** + * Restores the worktree-scoped HISTFILE that macOS `/etc/zshrc` destroys. + * + * That file assigns `HISTFILE=${ZDOTDIR:-$HOME}/.zsh_history` with no + * check-before-set, and it runs before any wrapper file Orca controls — so the + * injected value is already gone, and because ZDOTDIR still points at Orca's + * wrapper dir the replacement lands INSIDE it. Per-worktree history was a + * silent no-op on the primary platform as a result (#11044). + * + * Emitted after the user's own startup file has been sourced, so it is the last + * word, exactly like the CODEX_HOME/OPENCODE_CONFIG_DIR restores beside it. + */ +export const ZSH_HISTFILE_RESTORE_BLOCK = `if [[ -n "\${ORCA_HISTFILE:-}" ]]; then + HISTFILE="$ORCA_HISTFILE" +elif [[ "\${HISTFILE:-}" == "$ZDOTDIR/.zsh_history" ]]; then + # Why also when Orca injected nothing: /etc/zshrc derived this from Orca's + # wrapper ZDOTDIR, so history would accumulate INSIDE the wrapper dir and the + # user's real history would be invisible — the plain #11044 bug, with no + # per-worktree scoping involved. Matching the exact clobbered value means a + # HISTFILE the user set deliberately is never touched. + HISTFILE="\${ORCA_ORIG_ZDOTDIR:-$HOME}/.zsh_history" +fi` + export function getZshStartupFileSourceBlock(options: { fileName: '.zprofile' | '.zshrc' | '.zlogin' homeExpression?: string @@ -168,11 +191,13 @@ fi ` } -// Why: fish has no ZDOTDIR-style wrapper dir, so the marker rides `--init-command` -// and fires on fish_prompt — the earliest event fish exposes (STA-3417). Unlike zsh's -// zle-line-init this lands just *before* fish arms `?2004h`, which PostReadyFlushGate -// absorbs. `builtin printf` so a user-defined printf can't silently swallow the marker -// and send every launch to the ready timeout. +// Why: fish has no ZDOTDIR-style wrapper dir, so the marker rides `--init-command`, +// which fish runs AFTER config.fish (verified on 4.7.1) — the same last-word +// guarantee the zsh/bash wrapper files rely on. It fires on fish_prompt, the +// earliest event fish exposes (STA-3417). Unlike zsh's zle-line-init this lands +// just *before* fish arms `?2004h`, which PostReadyFlushGate absorbs. `builtin +// printf` so a user-defined printf can't silently swallow the marker and send +// every launch to the ready timeout. export function getFishShellReadyInitCommand(escapedMarker: string): string { return `if test "$ORCA_SHELL_READY_MARKER" = 1 function __orca_shell_ready_marker --on-event fish_prompt diff --git a/src/main/terminal-history-deletion.ts b/src/main/terminal-history-deletion.ts index b17eaa7ba70..1fea1128ebe 100644 --- a/src/main/terminal-history-deletion.ts +++ b/src/main/terminal-history-deletion.ts @@ -1,24 +1,39 @@ -import { basename, join } from 'node:path' +import { basename, dirname, join } from 'node:path' import { existsSync, mkdirSync, readdirSync, renameSync } from 'node:fs' import { removeHostTree } from './host-tree-removal' +import { deleteFishHistoryFile, resolveFishHistoryDir } from './fish-history-session' +import { readHistoryMeta } from './terminal-history' import { getHistoryRoot, - hashWorktreeId, listWslHistoryRoots, PENDING_DELETE_DIR_NAME } from './terminal-history-paths' +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] const historyTreeRemovalAttempts = new Map() const historyTreeRemovalRetryTimers = new Map>() +const wslDistroByTombstone = new Map() + +function wslDistroForHistoryRoot(historyRoot: string): string | undefined { + return basename(dirname(historyRoot)) === 'terminal-history-wsl' + ? basename(historyRoot) + : undefined +} 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)) { @@ -51,6 +66,7 @@ function scheduleHistoryTreeRemovalRetry(dir: string): void { if (retryDelayMs === undefined) { // Out of in-process attempts: the tombstone stays on disk and the next startup drain re-queues it. historyTreeRemovalAttempts.delete(dir) + wslDistroByTombstone.delete(dir) return } historyTreeRemovalAttempts.set(dir, attempt + 1) @@ -62,19 +78,46 @@ function scheduleHistoryTreeRemovalRetry(dir: string): void { historyTreeRemovalRetryTimers.set(dir, timer) } -function scheduleHistoryTreeRemoval(dir: string): void { +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 } - const removal = removeHostTree(dir) + if (wslDistro) { + wslDistroByTombstone.set(dir, wslDistro) + } + let removalSucceeded = false + const cleanupDistro = wslDistroByTombstone.get(dir) + const meta = cleanupDistro ? readHistoryMeta(dir) : null + const cleanup = + cleanupDistro && meta?.fishSession + ? // Why swallow rather than rethrow: fish history cleanup is best effort + // and runs `wsl.exe --exec fish`, which fails outright on a distro that + // has no fish — which is most of them. Rethrowing chained that failure + // into removeHostTree below, so those users' history trees were never + // reclaimed at all. + deleteWslFishHistoryFile(cleanupDistro, meta.fishSession).catch((err: unknown) => { + console.warn( + `[pty:history] Failed to delete WSL fish history: ${err instanceof Error ? err.message : String(err)}` + ) + }) + : null + const removal = (cleanup ? cleanup.then(() => removeHostTree(dir)) : removeHostTree(dir)) .then(() => { + removalSucceeded = true historyTreeRemovalAttempts.delete(dir) + wslDistroByTombstone.delete(dir) }) .catch((err: unknown) => { console.warn( @@ -86,6 +129,9 @@ function scheduleHistoryTreeRemoval(dir: string): void { if (pendingHistoryTreeRemovals.get(dir) === removal) { pendingHistoryTreeRemovals.delete(dir) } + if (removalSucceeded) { + schedulePendingHistoryTreeRemovals(historyRootForTombstone(dir)) + } }) pendingHistoryTreeRemovals.set(dir, removal) } @@ -93,11 +139,23 @@ function scheduleHistoryTreeRemoval(dir: string): void { /** Tombstone one history tree and queue its recursive removal off the caller's critical path. * Returns false when the rename failed, leaving the tree for a later GC pass to reclaim. */ export function scheduleWorktreeHistoryTreeDeletion(dir: string, historyRoot: string): boolean { + // Why first: fish keeps its history in the user's fish data dir, outside this tree, + // so the meta.json naming the session must still be readable when we look it up. + const meta = readHistoryMeta(dir) + const wslDistro = wslDistroForHistoryRoot(historyRoot) + if (meta?.fishSession && !wslDistro) { + // Why both directories: the recorded one is what the PTY's fish saw, this + // process's own is the fallback when meta.json predates that field. + deleteFishHistoryFile(meta.fishSession, [ + ...(meta.fishHistoryDir ? [meta.fishHistoryDir] : []), + resolveFishHistoryDir() + ]) + } const tombstone = tombstoneHistoryTree(dir, historyRoot) if (!tombstone) { return false } - scheduleHistoryTreeRemoval(tombstone) + scheduleHistoryTreeRemoval(tombstone, wslDistro) return true } @@ -109,7 +167,7 @@ export function schedulePendingHistoryTreeRemovals(historyRoot: string): void { } try { for (const entry of readdirSync(pendingRoot)) { - scheduleHistoryTreeRemoval(join(pendingRoot, entry)) + scheduleHistoryTreeRemoval(join(pendingRoot, entry), wslDistroForHistoryRoot(historyRoot)) } } catch { // Non-fatal. @@ -131,6 +189,7 @@ export function cancelPendingHistoryTreeRemovalRetries(): void { } historyTreeRemovalRetryTimers.clear() historyTreeRemovalAttempts.clear() + wslDistroByTombstone.clear() } /** Drain every history root's tombstones and await the in-flight removals. Tests only: production diff --git a/src/main/terminal-history-fish-session.node-pty.test.ts b/src/main/terminal-history-fish-session.node-pty.test.ts new file mode 100644 index 00000000000..f4d7b3cf404 --- /dev/null +++ b/src/main/terminal-history-fish-session.node-pty.test.ts @@ -0,0 +1,145 @@ +/** + * Real-fish proof for worktree-scoped fish history. + * + * fish IGNORES HISTFILE, so the directory+filename mechanism bash and zsh use does + * not transfer: history lives at `$XDG_DATA_HOME/fish/${fish_history}_history` and + * the only isolation knob is the session NAME. This suite pins the two facts + * `injectHistoryEnv` bets on — that fish picks up `fish_history` from the spawn + * environment (it imports env vars as global variables at startup), and that the + * file it then writes is the one `resolveFishHistoryDir` points at. + * + * Interactive is mandatory: fish writes no history in non-interactive mode, so the + * PTY and the typed line are the test, not scaffolding. DA1/CPR/OSC-11 probes are + * answered here because no real xterm is attached — without them fish stalls ~10s + * on its DA1 read sentinel before painting a prompt. + */ +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import path from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import { fishRequirementViolation, resolveFishBinary } from '../shared/fish-binary-requirement' +import { fishHistorySessionName, resolveFishHistoryDir } from './fish-history-session' + +const FISH = resolveFishBinary(4) +const itWithFish = FISH.available ? it : it.skip + +const PROMPT_MARK = 'ORCAHIST> ' +const WORKTREE_HASH = 'deadbeefdeadbeef' +const MARKER = 'echo orca-worktree-scoped-history' + +const sleep = (ms: number): Promise => new Promise((resolve) => setTimeout(resolve, ms)) + +async function waitUntil(predicate: () => boolean, timeoutMs: number): Promise { + const deadline = Date.now() + timeoutMs + while (Date.now() < deadline) { + if (predicate()) { + return true + } + await sleep(20) + } + return false +} + +describe('fish keeps per-worktree history under the session Orca names', () => { + let home: string | null = null + + // Always runs, so the CI lane cannot report green with the regression below skipped. + it('has the fish this suite needs when CI requires one', () => { + expect(fishRequirementViolation(FISH)).toBeNull() + }) + + afterEach(() => { + if (home) { + rmSync(home, { recursive: true, force: true }) + home = null + } + }) + + itWithFish( + 'writes an interactive command to $XDG_DATA_HOME/fish/_history, not the shared file', + async () => { + const nodePty = await import('node-pty') + + home = mkdtempSync(path.join(tmpdir(), 'orca-fish-history-')) + const dataHome = path.join(home, 'data') + mkdirSync(path.join(home, 'fish'), { recursive: true }) + writeFileSync( + path.join(home, 'fish/config.fish'), + [ + 'set -g fish_greeting ""', + `function fish_prompt; printf '${PROMPT_MARK}'; end`, + 'function fish_right_prompt; end', + '' + ].join('\n') + ) + + const session = fishHistorySessionName(WORKTREE_HASH) + const term = nodePty.spawn(FISH.path as string, ['-l', '-i'], { + name: 'xterm-256color', + cols: 120, + rows: 30, + cwd: home, + // Fully pinned: no ambient HOME/XDG_* reaches fish, so this cannot pass + // only on a machine whose real fish config happens to cooperate. + env: { + PATH: process.env.PATH ?? '/usr/bin:/bin', + HOME: home, + TERM: 'xterm-256color', + // LC_ALL wins over any LANG/LC_* a host might contribute, pinning fish's locale. + LANG: 'en_US.UTF-8', + LC_ALL: 'en_US.UTF-8', + XDG_CONFIG_HOME: home, + XDG_DATA_HOME: dataHome, + // The production injection under test (terminal-history.ts). + fish_history: session + } + }) + + let rendered = '' + term.onData((chunk) => { + rendered += chunk + if (chunk.includes('\x1b[0c') || chunk.includes('\x1b[c')) { + term.write('\x1b[?62;4;6;22c') + } + if (chunk.includes('\x1b[6n')) { + term.write('\x1b[1;1R') + } + if (chunk.includes('\x1b]10;?') || chunk.includes('\x1b]11;?')) { + term.write('\x1b]11;rgb:1e1e/1e1e/1e1e\x1b\\') + } + }) + let exited = false + term.onExit(() => { + exited = true + }) + + expect(await waitUntil(() => rendered.includes(PROMPT_MARK), 15_000)).toBe(true) + term.write(`${MARKER}\r`) + expect(await waitUntil(() => rendered.includes('orca-worktree-scoped-history'), 5_000)).toBe( + true + ) + // fish flushes history on exit, so the read must wait for the process to go. + term.write('exit\r') + expect(await waitUntil(() => exited, 10_000)).toBe(true) + try { + term.kill() + } catch { + // already gone + } + + const scopedPath = path.join( + resolveFishHistoryDir({ XDG_DATA_HOME: dataHome }), + `${session}_history` + ) + expect(scopedPath).toBe(path.join(dataHome, 'fish', `${session}_history`)) + const scoped = readFileSync(scopedPath as string, 'utf8') + // YAML-ish records, not one line per command — any reader must handle this shape. + expect(scoped).toContain(`- cmd: ${MARKER}`) + expect(scoped).toMatch(/^ {2}when: \d+$/m) + + // Isolation: the default session file fish would otherwise have used is absent. + expect(() => readFileSync(path.join(dataHome, 'fish', 'fish_history'), 'utf8')).toThrow() + }, + 40_000 + ) +}) diff --git a/src/main/terminal-history-gc.ts b/src/main/terminal-history-gc.ts index 2a6d2280945..d67e3a7dd5c 100644 --- a/src/main/terminal-history-gc.ts +++ b/src/main/terminal-history-gc.ts @@ -1,5 +1,5 @@ import { join } from 'node:path' -import { existsSync, readFileSync, readdirSync, statSync } from 'node:fs' +import { existsSync, readdirSync, statSync } from 'node:fs' import { getHistoryRoot, listWslHistoryRoots, @@ -9,6 +9,9 @@ import { schedulePendingHistoryTreeRemovals, scheduleWorktreeHistoryTreeDeletion } from './terminal-history-deletion' +import { readHistoryMeta } from './terminal-history' +import { resolveFishHistoryDir, sweepOrphanedFishHistoryFiles } from './fish-history-session' +import { hashWorktreeId } from './terminal-history-id' // Why 5 minutes: GC runs ~10s after startup, and the live-worktree snapshot is // taken just before. A worktree created between the snapshot and GC execution @@ -25,8 +28,21 @@ let historyGcRunning = false function gcScanRoot( root: string, liveWorktreeIds: Set -): { totalDirs: number; orphaned: number; pruned: number; totalSizeKB: number } { - const result = { totalDirs: 0, orphaned: 0, pruned: 0, totalSizeKB: 0 } +): { + totalDirs: number + orphaned: number + pruned: number + totalSizeKB: number + /** Every fish data dir a meta.json in this root names, for the orphan sweep. */ + fishHistoryDirs: Set +} { + const result = { + totalDirs: 0, + orphaned: 0, + pruned: 0, + totalSizeKB: 0, + fishHistoryDirs: new Set() + } if (!existsSync(root)) { return result } @@ -61,11 +77,11 @@ function gcScanRoot( continue } - const meta = JSON.parse(readFileSync(metaPath, 'utf-8')) as { - worktreeId?: string - createdAt?: string + const meta = readHistoryMeta(entryPath) + if (meta?.fishHistoryDir) { + result.fishHistoryDirs.add(meta.fishHistoryDir) } - if (!meta.worktreeId) { + if (!meta?.worktreeId) { continue } @@ -100,11 +116,25 @@ 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). const wslTotals = { totalDirs: 0, orphaned: 0, pruned: 0, totalSizeKB: 0 } + const liveFishHistoryDirs = new Set(main.fishHistoryDirs) for (const distroRoot of listWslHistoryRoots()) { schedulePendingHistoryTreeRemovals(distroRoot) const r = gcScanRoot(distroRoot, liveWorktreeIds) @@ -112,6 +142,27 @@ export function runHistoryGc(liveWorktreeIds: Set): void { wslTotals.orphaned += r.orphaned wslTotals.pruned += r.pruned wslTotals.totalSizeKB += r.totalSizeKB + for (const dir of r.fishHistoryDirs) { + liveFishHistoryDirs.add(dir) + } + } + + // Why a sweep on top of per-worktree deletion: a fish history file lives in + // the user's fish data dir, so it outlives the directory that names it. A + // crash between tombstone and removal, or a hand-deleted history dir, leaves + // one with nothing left to point at it. Collecting the dirs the live meta + // files name covers a machine whose XDG_DATA_HOME changed between runs. + const fishDirs = new Set([resolveFishHistoryDir()]) + for (const dir of liveFishHistoryDirs) { + fishDirs.add(dir) + } + const fishOrphans = sweepOrphanedFishHistoryFiles( + new Set([...liveWorktreeIds].map(hashWorktreeId)), + fishDirs, + GC_MIN_AGE_MS + ) + if (fishOrphans > 0) { + console.log(`[pty:history:gc] Swept ${fishOrphans} orphaned fish history file(s)`) } const totalDirs = main.totalDirs + wslTotals.totalDirs diff --git a/src/main/terminal-history-id.ts b/src/main/terminal-history-id.ts new file mode 100644 index 00000000000..38454baa29e --- /dev/null +++ b/src/main/terminal-history-id.ts @@ -0,0 +1,6 @@ +import { createHash } from 'node:crypto' + +/** First 16 hex chars of SHA-256 of the worktreeId. */ +export function hashWorktreeId(worktreeId: string): string { + return createHash('sha256').update(worktreeId).digest('hex').slice(0, 16) +} diff --git a/src/main/terminal-history-paths.ts b/src/main/terminal-history-paths.ts index e942dd8bc3f..263f39c8c35 100644 --- a/src/main/terminal-history-paths.ts +++ b/src/main/terminal-history-paths.ts @@ -1,4 +1,3 @@ -import { createHash } from 'node:crypto' import { existsSync, readdirSync } from 'node:fs' import { join } from 'node:path' import { app } from 'electron' @@ -8,10 +7,7 @@ const HISTORY_DIR_NAME_WSL = 'terminal-history-wsl' // Why: rename live history out of the way first so a quit mid-rm still leaves a durable tombstone GC can finish. export const PENDING_DELETE_DIR_NAME = '.pending-delete' -/** First 16 hex chars of SHA-256 of the worktreeId. */ -export function hashWorktreeId(worktreeId: string): string { - return createHash('sha256').update(worktreeId).digest('hex').slice(0, 16) -} +export { hashWorktreeId } from './terminal-history-id' export function getHistoryRoot(): string { return join(app.getPath('userData'), HISTORY_DIR_NAME) diff --git a/src/main/terminal-history-tombstone-retry.test.ts b/src/main/terminal-history-tombstone-retry.test.ts index 195ee82d9f3..67cb9926322 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,118 @@ 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 }) + // Why removeHostTree and not the fish cleanup: a failing fish cleanup no + // longer blocks tree removal (most distros have no fish), so the tree + // removal itself has to be the thing that fails to exercise the cap. + removeHostTreeMock.mockRejectedValue(new Error('EBUSY')) + 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(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) + removeHostTreeMock.mockReset() + 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) + }) + + it('still removes the history tree when WSL fish cleanup fails', async () => { + // Why this matters: `injectWslFishHistoryEnv` records a fishSession for EVERY + // WSL terminal, and cleanup shells out to `wsl.exe --exec fish`. On a distro + // with no fish that always fails — which used to chain into removeHostTree and + // leak the bash/zsh history tree for those users permanently. + const distroRoot = join(userDataDir, 'terminal-history-wsl', 'Ubuntu') + mkdirSync(distroRoot, { recursive: true }) + deleteWslFishHistoryFileMock.mockRejectedValue(new Error('fish: command not found')) + removeHostTreeMock.mockImplementation(async (dir) => { + rmSync(dir, { recursive: true, force: true }) + }) + const worktreeId = 'repo-1::/path/no-fish' + 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 flushPendingWorktreeHistoryDeletions() + + expect(deleteWslFishHistoryFileMock).toHaveBeenCalled() + expect(removeHostTreeMock).toHaveBeenCalled() + 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 c1900e3d0b5..9b9a3a78ea8 100644 --- a/src/main/terminal-history.test.ts +++ b/src/main/terminal-history.test.ts @@ -7,23 +7,39 @@ const { mkdirSyncMock, writeFileSyncMock, readFileSyncMock, + lstatSyncMock, rmSyncMock, renameSyncMock, readdirSyncMock, statSyncMock, + openSyncMock, + fstatSyncMock, + closeSyncMock, getPathMock, - rmAsyncMock + rmAsyncMock, + deleteWslFishHistoryFileMock } = vi.hoisted(() => ({ existsSyncMock: vi.fn(), mkdirSyncMock: vi.fn(), writeFileSyncMock: vi.fn(), readFileSyncMock: vi.fn(), + lstatSyncMock: vi.fn(), rmSyncMock: vi.fn(), 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) + rmAsyncMock: vi.fn(async () => undefined), + deleteWslFishHistoryFileMock: vi.fn(async () => undefined) })) vi.mock('fs', () => ({ @@ -31,10 +47,14 @@ vi.mock('fs', () => ({ mkdirSync: mkdirSyncMock, writeFileSync: writeFileSyncMock, readFileSync: readFileSyncMock, + lstatSync: lstatSyncMock, 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 @@ -60,21 +80,31 @@ vi.mock('./wsl', () => ({ toLinuxPath: toLinuxPathMock })) +vi.mock('./wsl-fish-history-cleanup', () => ({ + deleteWslFishHistoryFile: deleteWslFishHistoryFileMock +})) + import { resolveShellKind, ensureHistoryDir, injectHistoryEnv, - updateHistFileForFallback + MAX_HISTORY_META_BYTES, + updateHistoryEnvForFallback, + type HistoryInjectionResult } from './terminal-history' +import { fishHistorySessionName } from './fish-history-session' import { hashWorktreeId } from './terminal-history-paths' import { + cancelPendingHistoryTreeRemovalRetries, deleteWorktreeHistoryDir, flushPendingWorktreeHistoryDeletions } from './terminal-history-deletion' + import { runHistoryGc, scheduleHistoryGc } from './terminal-history-gc' describe('terminal-history', () => { afterEach(() => { + cancelPendingHistoryTreeRemovalRetries() vi.useRealTimers() }) @@ -82,9 +112,30 @@ describe('terminal-history', () => { vi.clearAllMocks() // 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, + birthtimeNs: 3n, + size: 100, + isDirectory: () => true, + isFile: () => true, + isSymbolicLink: () => false + }) }) describe('resolveShellKind', () => { @@ -228,12 +279,103 @@ describe('terminal-history', () => { expect(result.histFile).toBeNull() }) - it('does not inject HISTFILE for fish (Phase 2)', () => { + it('scopes fish by session name instead of HISTFILE, which fish ignores', () => { const env: Record = {} const result = injectHistoryEnv(env, 'repo-1::/path/wt', '/usr/bin/fish', '/path/wt') expect(env.HISTFILE).toBeUndefined() expect(result.shell).toBe('fish') + expect(result.fishSession).toBe(fishHistorySessionName(hashWorktreeId('repo-1::/path/wt'))) + expect(env.fish_history).toBe(result.fishSession) + // Deletion can only find the out-of-tree fish file through this record. + expect(writeFileSyncMock).toHaveBeenCalledWith( + expect.stringContaining('meta.json'), + expect.stringContaining(`"fishSession":"${result.fishSession}"`), + expect.anything() + ) + }) + + it('records the history DIRECTORY from the SPAWN env, not this process env', () => { + // The file is written by the PTY's fish, and the two envs disagree whenever + // Orca was launched with a different XDG_DATA_HOME than the shells it spawns. + // Pinned so the assertion cannot accidentally read the developer's own value. + const originalDataHome = process.env.XDG_DATA_HOME + process.env.XDG_DATA_HOME = ['', 'main', 'process', 'data'].join(sep) + try { + const env: Record = { XDG_DATA_HOME: ['', 'spawn', 'data'].join(sep) } + injectHistoryEnv(env, 'repo-1::/path/wt', '/usr/bin/fish', '/path/wt') + const meta = JSON.parse(writeFileSyncMock.mock.calls.at(-1)?.[1] as string) as Record< + string, + string + > + + expect(meta.fishHistoryDir).toBe(['', 'spawn', 'data', 'fish'].join(sep)) + } finally { + if (originalDataHome === undefined) { + delete process.env.XDG_DATA_HOME + } else { + process.env.XDG_DATA_HOME = originalDataHome + } + } + }) + + it('replaces oversized metadata without parsing it', () => { + statSyncMock.mockReturnValue({ isDirectory: () => true, size: MAX_HISTORY_META_BYTES + 1 }) + const env = { XDG_DATA_HOME: ['', 'active'].join(sep) } + + injectHistoryEnv(env, 'repo-1::/path/wt', '/usr/bin/fish', '/path/wt') + + expect(readFileSyncMock).not.toHaveBeenCalled() + const meta = JSON.parse(writeFileSyncMock.mock.calls.at(-1)?.[1] as string) as Record< + string, + string + > + expect(meta.fishHistoryDir).toBe(['', 'active', 'fish'].join(sep)) + }) + + it('gives two fish worktrees different history sessions', () => { + const envA: Record = {} + injectHistoryEnv(envA, 'repo-1::/path/wt-a', '/usr/bin/fish', '/path/wt-a') + const envB: Record = {} + injectHistoryEnv(envB, 'repo-1::/path/wt-b', '/usr/bin/fish', '/path/wt-b') + + expect(envA.fish_history).not.toBe(envB.fish_history) + }) + + it('drops an ORCA_HISTFILE inherited from a parent Orca PTY', () => { + // Why: an Orca terminal opened from inside another Orca terminal inherits + // it, and the zsh wrapper would then re-export the PARENT worktree's + // history path here. Credit: caught by @innocarpe in #11146. + const env: Record = { + ORCA_HISTFILE: ['', 'other', 'wt', 'zsh_history'].join(sep) + } + + injectHistoryEnv(env, 'repo-1::/path/wt', '/bin/zsh', '/path/wt') + + expect(env.ORCA_HISTFILE).toBe(env.HISTFILE) + expect(env.ORCA_HISTFILE).not.toContain('other') + }) + + it('drops an inherited ORCA_HISTFILE even when it injects nothing', () => { + // The dangerous variant: the early return would otherwise leave the stale + // value pointing the wrapper at another worktree, overriding HISTFILE. + const env: Record = { + HISTFILE: ['', 'mine', 'zsh_history'].join(sep), + ORCA_HISTFILE: ['', 'other', 'wt', 'zsh_history'].join(sep) + } + + injectHistoryEnv(env, 'repo-1::/path/wt', '/bin/zsh', '/path/wt') + + expect(env.HISTFILE).toBe(['', 'mine', 'zsh_history'].join(sep)) + expect(env.ORCA_HISTFILE).toBeUndefined() + }) + + it('preserves a caller-supplied fish_history', () => { + const env: Record = { fish_history: 'mine' } + const result = injectHistoryEnv(env, 'repo-1::/path/wt', '/usr/bin/fish', '/path/wt') + + expect(env.fish_history).toBe('mine') + expect(result.fishSession).toBeNull() }) it('degrades gracefully when directory creation fails', () => { @@ -249,12 +391,19 @@ describe('terminal-history', () => { }) }) - describe('updateHistFileForFallback', () => { + describe('updateHistoryEnvForFallback', () => { + const zshInjection = (): HistoryInjectionResult => ({ + shell: 'zsh', + histFile: '/fake/userData/terminal-history/abc123/zsh_history', + fishSession: null, + historyDir: '/fake/userData/terminal-history/abc123' + }) + it('updates HISTFILE to match fallback shell', () => { const env: Record = { HISTFILE: '/fake/userData/terminal-history/abc123/zsh_history' } - updateHistFileForFallback(env, '/bin/bash') + updateHistoryEnvForFallback(env, '/bin/bash', zshInjection()) expect(env.HISTFILE).toBe('/fake/userData/terminal-history/abc123/bash_history') }) @@ -262,18 +411,45 @@ describe('terminal-history', () => { const env: Record = { HISTFILE: '/fake/userData/terminal-history/abc123/zsh_history' } - updateHistFileForFallback(env, '/bin/sh') + updateHistoryEnvForFallback(env, '/bin/sh', zshInjection()) expect(env.HISTFILE).toBeUndefined() }) - it('is a no-op when HISTFILE is not set', () => { + it('is a no-op when nothing was injected', () => { const env: Record = {} - updateHistFileForFallback(env, '/bin/bash') + updateHistoryEnvForFallback(env, '/bin/bash', { + shell: 'unknown', + histFile: null, + fishSession: null, + historyDir: null + }) expect(env.HISTFILE).toBeUndefined() }) + + it('swaps an injected fish session for the fallback shell HISTFILE', () => { + const env: Record = { fish_history: 'orca_abc123' } + updateHistoryEnvForFallback(env, '/bin/bash', { + shell: 'fish', + histFile: null, + fishSession: 'orca_abc123', + historyDir: '/fake/userData/terminal-history/abc123' + }) + expect(env.fish_history).toBeUndefined() + expect(env.HISTFILE).toBe('/fake/userData/terminal-history/abc123/bash_history') + }) + + it('keeps a caller-supplied fish_history the injection never claimed', () => { + const env: Record = { fish_history: 'mine' } + updateHistoryEnvForFallback(env, '/bin/bash', zshInjection()) + expect(env.fish_history).toBe('mine') + }) }) describe('deleteWorktreeHistoryDir', () => { + const worktreeId = 'repo-1::/path/wt' + const session = fishHistorySessionName(hashWorktreeId(worktreeId)) + const historyFilename = `${session}_history` + it('tombstones then async-removes the history directory without recursive rmSync', async () => { existsSyncMock.mockReturnValue(true) deleteWorktreeHistoryDir('repo-1::/path/wt') @@ -286,6 +462,84 @@ describe('terminal-history', () => { await flushPendingWorktreeHistoryDeletions() }) + it('deletes the fish history file in the directory meta.json recorded', async () => { + const recordedDir = ['', 'spawn', 'data', 'fish'].join(sep) + existsSyncMock.mockReturnValue(true) + lstatSyncMock.mockReturnValue({ isFile: () => true }) + readFileSyncMock.mockReturnValue( + JSON.stringify({ worktreeId, fishSession: session, fishHistoryDir: recordedDir }) + ) + + deleteWorktreeHistoryDir('repo-1::/path/wt') + + expect(rmSyncMock).toHaveBeenCalledWith([recordedDir, historyFilename].join(sep)) + await flushPendingWorktreeHistoryDeletions() + }) + + it('also tries this process fish dir, for meta written before that field', async () => { + const originalDataHome = process.env.XDG_DATA_HOME + process.env.XDG_DATA_HOME = ['', 'main', 'data'].join(sep) + try { + existsSyncMock.mockReturnValue(true) + lstatSyncMock.mockReturnValue({ isFile: () => true }) + readFileSyncMock.mockReturnValue(JSON.stringify({ worktreeId, fishSession: session })) + + deleteWorktreeHistoryDir('repo-1::/path/wt') + + expect(rmSyncMock).toHaveBeenCalledWith( + ['', 'main', 'data', 'fish', historyFilename].join(sep) + ) + } finally { + if (originalDataHome === undefined) { + delete process.env.XDG_DATA_HOME + } else { + process.env.XDG_DATA_HOME = originalDataHome + } + } + await flushPendingWorktreeHistoryDeletions() + }) + + it('ignores a fishSession that does not belong to this history directory', async () => { + // Why: the session name is re-derived from the directory's own hash, so a + // tampered meta.json naming someone else's session cannot steer the delete. + existsSyncMock.mockReturnValue(true) + lstatSyncMock.mockReturnValue({ isFile: () => true }) + readFileSyncMock.mockReturnValue( + JSON.stringify({ worktreeId, fishSession: 'orca_deadbeefdeadbeef' }) + ) + + deleteWorktreeHistoryDir('repo-1::/path/wt') + + expect(rmSyncMock).not.toHaveBeenCalledWith(expect.stringContaining('orca_deadbeefdeadbeef')) + await flushPendingWorktreeHistoryDeletions() + }) + + it('refuses to unlink a fish history path that is not a regular file', async () => { + existsSyncMock.mockReturnValue(true) + lstatSyncMock.mockReturnValue({ isFile: () => false }) + readFileSyncMock.mockReturnValue(JSON.stringify({ worktreeId, fishSession: session })) + + deleteWorktreeHistoryDir('repo-1::/path/wt') + + expect(rmSyncMock).not.toHaveBeenCalled() + 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) => { + if (path.endsWith('terminal-history-wsl')) { + return ['Ubuntu'] + } + return [] + }) + + deleteWorktreeHistoryDir(worktreeId) + await flushPendingWorktreeHistoryDeletions() + + expect(deleteWslFishHistoryFileMock).toHaveBeenCalledWith('Ubuntu', session) + }) + it('leaves the live directory alone when the tombstone rename fails', async () => { existsSyncMock.mockReturnValue(true) renameSyncMock.mockImplementation(() => { @@ -309,6 +563,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')) { @@ -321,11 +576,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'), @@ -388,6 +645,64 @@ 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) => { + if (dir.endsWith('.pending-delete')) { + return [] + } + if (dir.endsWith('terminal-history')) { + return ['broken', 'healthy'] + } + return ['meta.json'] + }) + statSyncMock.mockReturnValue({ isDirectory: () => true, size: 100 }) + readFileSyncMock.mockReturnValue( + JSON.stringify({ + worktreeId: 'orphan', + createdAt: new Date(Date.now() - 10 * 60 * 1000).toISOString() + }) + ) + renameSyncMock.mockImplementationOnce(() => { + throw new Error('busy') + }) + + expect(() => runHistoryGc(new Set(['live-wt']))).not.toThrow() + expect(renameSyncMock).toHaveBeenCalledTimes(2) + expect(rmAsyncMock).toHaveBeenCalledTimes(1) + await flushPendingWorktreeHistoryDeletions() + }) + it('skips recently-created directories to avoid TOCTOU race', () => { existsSyncMock.mockImplementation((p: string) => { if (p.includes('terminal-history-wsl')) { @@ -407,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() @@ -416,15 +731,16 @@ 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(() => runHistoryGc(new Set(['live-wt']))).not.toThrow() + 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'] @@ -432,8 +748,11 @@ describe('terminal-history', () => { return ['meta.json'] }) statSyncMock.mockReturnValue({ isDirectory: () => true, size: 100 }) + rmAsyncMock.mockImplementation(async () => { + 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/terminal-history.ts b/src/main/terminal-history.ts index 38dd7968264..c15f903c2de 100644 --- a/src/main/terminal-history.ts +++ b/src/main/terminal-history.ts @@ -1,10 +1,18 @@ import { join, basename } from 'node:path' -import { mkdirSync, existsSync, writeFileSync } from 'node:fs' +import { mkdirSync, existsSync, readFileSync, statSync, writeFileSync } from 'node:fs' +import { + fishHistorySessionName, + isSafeFishHistorySession, + resolveFishHistoryDir +} from './fish-history-session' import { parseWslPath, toLinuxPath } from './wsl' -import { getHistoryRoot, getHistoryRootWsl, hashWorktreeId } from './terminal-history-paths' +import { getHistoryRoot, getHistoryRootWsl } from './terminal-history-paths' +import { hashWorktreeId } from './terminal-history-id' type ShellKind = 'zsh' | 'bash' | 'fish' | 'pwsh' | 'powershell' | 'cmd' | 'unknown' +export const MAX_HISTORY_META_BYTES = 32 * 1024 + // ─── Shell Detection ─────────────────────────────────────────────── /** Resolve the shell kind from a shell binary path. @@ -33,14 +41,16 @@ export function resolveShellKind(shellPath: string): ShellKind { return 'unknown' } -/** Map shell kind to the filename used inside the history directory. */ +/** Map shell kind to the filename used inside the history directory. + * fish is absent on purpose: it ignores HISTFILE and keeps history in its own + * data dir keyed by session name (see fish-history-session.ts). */ function historyFilename(shell: ShellKind): string | null { switch (shell) { case 'zsh': return 'zsh_history' case 'bash': return 'bash_history' - // Phase 2: fish and PowerShell use different mechanisms + // Phase 2: PowerShell and cmd use different mechanisms case 'fish': case 'pwsh': case 'powershell': @@ -68,17 +78,81 @@ export function ensureHistoryDir(worktreeHash: string, wslDistro?: string): stri } } -/** Write meta.json alongside history files for debuggability. */ -function writeMetaFile(dir: string, worktreeId: string): void { +/** Write meta.json alongside history files, for debuggability and for GC. + * `fishSession` is load-bearing, not diagnostic: fish history lives outside + * this directory, so deletion can only find it by the name recorded here. + * `fishHistoryDir` is resolved from the SPAWN env, which is the one fish + * follows — this process's own may differ. */ +function writeMetaFile( + dir: string, + worktreeId: string, + fish?: { session: string; historyDir?: string } +): void { try { const metaPath = join(dir, 'meta.json') - if (!existsSync(metaPath)) { - writeFileSync(metaPath, JSON.stringify({ worktreeId, createdAt: new Date().toISOString() }), { - mode: 0o600 - }) + const existing = existsSync(metaPath) ? readHistoryMeta(dir) : null + if ( + existing && + (!fish || + (existing.fishSession === fish.session && existing.fishHistoryDir === fish.historyDir)) + ) { + return + } + writeFileSync( + metaPath, + JSON.stringify({ + worktreeId, + createdAt: existing?.createdAt ?? new Date().toISOString(), + ...(fish ? { fishSession: fish.session } : {}), + ...(fish?.historyDir ? { fishHistoryDir: fish.historyDir } : {}) + }), + { mode: 0o600 } + ) + } catch { + // Non-fatal — a missing meta.json only costs GC attribution. + } +} + +export type HistoryDirMeta = { + worktreeId?: string + createdAt?: string + /** fish session name whose history file lives in the user's fish data dir. */ + fishSession?: string + /** Directory that session's history file was written to, as the PTY saw it. */ + fishHistoryDir?: string +} + +/** Read one history directory's meta.json, or null when it is absent or unparseable. */ +export function readHistoryMeta(dir: string): HistoryDirMeta | null { + try { + const metaPath = join(dir, 'meta.json') + if (statSync(metaPath).size > MAX_HISTORY_META_BYTES) { + return null + } + const raw: unknown = JSON.parse(readFileSync(metaPath, 'utf-8')) + if (!raw || typeof raw !== 'object' || Array.isArray(raw)) { + return null + } + const record = raw as Record + // Why re-derive: the session name is a pure function of this directory's own + // hash, so a meta.json naming someone else's session cannot steer deletion. + const expectedFishSession = fishHistorySessionName(basename(dir).split('.')[0]) + const fishSession = + isSafeFishHistorySession(record.fishSession) && record.fishSession === expectedFishSession + ? record.fishSession + : undefined + const fishHistoryDir = + fishSession && typeof record.fishHistoryDir === 'string' && record.fishHistoryDir + ? record.fishHistoryDir + : undefined + return { + ...(typeof record.worktreeId === 'string' ? { worktreeId: record.worktreeId } : {}), + ...(typeof record.createdAt === 'string' ? { createdAt: record.createdAt } : {}), + ...(fishSession ? { fishSession } : {}), + ...(fishHistoryDir ? { fishHistoryDir } : {}) } } catch { - // Non-fatal — meta.json is purely for diagnostics. + return null } } @@ -87,6 +161,10 @@ function writeMetaFile(dir: string, worktreeId: string): void { export type HistoryInjectionResult = { shell: ShellKind histFile: string | null + /** fish session name exported as `fish_history`; null when fish is not the shell. */ + fishSession: string | null + /** Worktree history dir as the spawned shell sees it (Linux-visible under WSL). */ + historyDir: string | null } /** Build shell-specific history env overrides for a PTY spawn. @@ -103,18 +181,31 @@ export function injectHistoryEnv( cwd: string, options: { wslDistro?: string | null } = {} ): HistoryInjectionResult { + // Why unconditionally first: ORCA_HISTFILE is Orca-owned, and an Orca PTY + // launched from inside another Orca PTY inherits the parent's. Left in place, + // the zsh wrapper would re-export a PREVIOUS worktree's history path into this + // shell — the cross-worktree leak this feature exists to prevent — and it would + // also override a caller-supplied HISTFILE on the early return below. + // Credit: caught by @innocarpe in #11146. + delete spawnEnv.ORCA_HISTFILE + const shell = resolveShellKind(shellPath) - const result: HistoryInjectionResult = { shell, histFile: null } + const result: HistoryInjectionResult = { + shell, + histFile: null, + fishSession: null, + historyDir: null + } const filename = historyFilename(shell) - if (!filename) { - // Unknown shell or Phase 2 shell (fish, pwsh, cmd) — leave unchanged. + if (!filename && shell !== 'fish') { + // Unknown shell or Phase 2 shell (pwsh, cmd) — leave unchanged. return result } - // Check-before-set: if the caller already provided HISTFILE, preserve it. - // This follows the pattern used by Ghostty, Kitty, and VS Code (§6). - if (spawnEnv.HISTFILE) { + // Check-before-set: if the caller already provided the shell's history knob, + // preserve it. Same pattern Ghostty, Kitty, and VS Code use for HISTFILE (§6). + if (shell === 'fish' ? spawnEnv.fish_history : spawnEnv.HISTFILE) { return result } @@ -130,46 +221,92 @@ export function injectHistoryEnv( return result } + if (!filename) { + // fish: the directory holds no history, only the meta.json that lets deletion + // find the session file fish keeps in its own data dir. fish never runs as the + // inner WSL shell, so histDir needs no /mnt conversion here. + const session = fishHistorySessionName(worktreeHash) + // Resolve from the SPAWN env: that is the XDG_DATA_HOME/HOME fish will see, + // which need not match the one this process was launched with. + writeMetaFile(histDir, worktreeId, { session, historyDir: resolveFishHistoryDir(spawnEnv) }) + spawnEnv.fish_history = session + result.fishSession = session + result.historyDir = histDir + return result + } + writeMetaFile(histDir, worktreeId) const histFilePath = join(histDir, filename) // For WSL, convert the Windows path to a Linux-visible path. spawnEnv.HISTFILE = wslDistro ? toLinuxPath(histFilePath) : histFilePath + // Why a second variable: macOS `/etc/zshrc` assigns HISTFILE unconditionally + // (`HISTFILE=${ZDOTDIR:-$HOME}/.zsh_history`) and runs before Orca's wrapper + // .zshrc, so by then the injected value is gone from HISTFILE itself. The + // wrapper restores it from here once the user's own config has loaded (#11044). + spawnEnv.ORCA_HISTFILE = spawnEnv.HISTFILE result.histFile = spawnEnv.HISTFILE + result.historyDir = spawnEnv.HISTFILE.replace(/[/\\][^/\\]+$/, '') return result } -/** Update HISTFILE in spawnEnv when shell fallback changes the shell kind. - * For example, if zsh fails and bash takes over, the HISTFILE should point - * to bash_history instead of zsh_history. */ -export function updateHistFileForFallback( +/** WSL's outer executable hides the login shell, so carry both history knobs. */ +export function injectWslFishHistoryEnv( spawnEnv: Record, - fallbackShellPath: string + worktreeId: string, + wslDistro: string +): string | null { + if (spawnEnv.fish_history) { + return null + } + const worktreeHash = hashWorktreeId(worktreeId) + const historyDir = ensureHistoryDir(worktreeHash, wslDistro) + if (!historyDir) { + return null + } + const session = fishHistorySessionName(worktreeHash) + // Why no historyDir: this session's file lives inside the WSL distro, so a + // path resolved from THIS process's Windows environment names an unrelated + // host directory — which the host GC sweep would then scan. WSL cleanup goes + // through `deleteWslFishHistoryFile`, which resolves the path in the distro. + writeMetaFile(historyDir, worktreeId, { session }) + spawnEnv.fish_history = session + return session +} + +/** Re-point the history env when shell fallback changes the shell kind — e.g. zsh + * fails and bash takes over, so HISTFILE must name bash_history. A fish primary + * injected `fish_history` instead, which the fallback shell cannot use. */ +export function updateHistoryEnvForFallback( + spawnEnv: Record, + fallbackShellPath: string, + injected: HistoryInjectionResult ): void { - if (!spawnEnv.HISTFILE) { + // Only ever undo what this spawn injected; a caller-supplied value stays. + if (injected.fishSession && spawnEnv.fish_history === injected.fishSession) { + delete spawnEnv.fish_history + } + if (!injected.historyDir) { return } - const newShell = resolveShellKind(fallbackShellPath) - const newFilename = historyFilename(newShell) + const newFilename = historyFilename(resolveShellKind(fallbackShellPath)) if (!newFilename) { - // Fallback to an unknown shell — remove HISTFILE override entirely - // so the shell uses its own default. + // Fallback to an unknown shell — drop the override so it uses its own default. delete spawnEnv.HISTFILE + delete spawnEnv.ORCA_HISTFILE return } - - // Replace the filename portion of the HISTFILE path. - const dir = spawnEnv.HISTFILE.replace(/[/\\][^/\\]+$/, '') - spawnEnv.HISTFILE = `${dir}/${newFilename}` + spawnEnv.HISTFILE = `${injected.historyDir}/${newFilename}` + spawnEnv.ORCA_HISTFILE = spawnEnv.HISTFILE } /** Log the history injection result for diagnostics. */ export function logHistoryInjection(worktreeId: string, result: HistoryInjectionResult): void { const truncatedId = worktreeId.length > 60 ? `${worktreeId.slice(0, 60)}...` : worktreeId console.log( - `[pty:history] worktreeId=${truncatedId} shell=${result.shell} histFile=${result.histFile ?? 'none'}` + `[pty:history] worktreeId=${truncatedId} shell=${result.shell} histFile=${result.histFile ?? 'none'} fishSession=${result.fishSession ?? 'none'}` ) } 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 1aca7d585b0..b61d2aaca3f 100644 --- a/src/main/window/history-gc-worktree-ids.test.ts +++ b/src/main/window/history-gc-worktree-ids.test.ts @@ -1,18 +1,70 @@ -import { describe, expect, it, vi } from 'vitest' +/** + * The GC live set decides what history is "orphaned". Anything missing from it + * gets deleted, so a workspace kind that owns history and is absent here is a + * data-loss bug — that is what reverted #14863 (#14975). + */ +import { describe, expect, it } from 'vitest' +import { folderWorkspaceKey } from '../../shared/workspace-scope' import { getKnownWorktreeIdsForHistoryGc } from './history-gc-worktree-ids' -describe('getKnownWorktreeIdsForHistoryGc', () => { - it('uses persisted metadata keys without probing repo paths', () => { - const store = { - getAllWorktreeMeta: vi.fn(() => ({ - 'repo-1::/worktree-a': {}, - 'repo-2::/worktree-b': {} - })) - } +const noOtherProfiles = () => ({ ids: new Set(), unreadableProfiles: 0 }) - expect(getKnownWorktreeIdsForHistoryGc(store as never)).toEqual( - new Set(['repo-1::/worktree-a', 'repo-2::/worktree-b']) +const store = (worktreeIds: string[], folderIds: string[]) => + ({ + getAllWorktreeMeta: () => Object.fromEntries(worktreeIds.map((id) => [id, {}])), + getFolderWorkspaces: () => folderIds.map((id) => ({ id })) + }) as never + +describe('getKnownWorktreeIdsForHistoryGc', () => { + it('includes git worktrees', () => { + 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 + // injectHistoryEnv mints history under that key. They live in a separate store + // 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']), + noOtherProfiles ) - expect(store.getAllWorktreeMeta).toHaveBeenCalledTimes(1) + + expect(live.has(folderWorkspaceKey('fw-1'))).toBe(true) + expect(live.has(folderWorkspaceKey('fw-2'))).toBe(true) + expect(live.size).toBe(3) + }) + + it('is empty only when there is genuinely nothing live', () => { + 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 beec2f9cfc1..df614967192 100644 --- a/src/main/window/history-gc-worktree-ids.ts +++ b/src/main/window/history-gc-worktree-ids.ts @@ -1,7 +1,39 @@ 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. + * + * Why folder workspaces are included: a folder workspace's PTY carries + * `folder:` as its worktree id (see folder-workspace-composer-submit.ts), + * so `injectHistoryEnv` mints history under that key exactly as it does for a + * git worktree. They live in a separate store collection, so a set built only + * from `getAllWorktreeMeta()` makes every live folder workspace look orphaned — + * and the GC then deletes history the user is still accumulating. + */ export function getKnownWorktreeIdsForHistoryGc( - store: Pick + store: Pick, + readOtherProfiles = getOtherProfileWorktreeIdsForHistoryGc ): Set { - return new Set(Object.keys(store.getAllWorktreeMeta())) + 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/main/wsl-fish-history-cleanup.test.ts b/src/main/wsl-fish-history-cleanup.test.ts new file mode 100644 index 00000000000..fd8db40f01d --- /dev/null +++ b/src/main/wsl-fish-history-cleanup.test.ts @@ -0,0 +1,89 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + __resetWslFishHistoryCleanups, + deleteWslFishHistoryFile, + flushWslFishHistoryCleanups +} from './wsl-fish-history-cleanup' + +// Why reset on BOTH sides: the cleanup queue is module state, so one test's +// pending work would otherwise serialize behind — or deadlock — the next. +beforeEach(() => { + __resetWslFishHistoryCleanups() +}) + +afterEach(async () => { + await flushWslFishHistoryCleanups() + __resetWslFishHistoryCleanups() +}) + +describe('deleteWslFishHistoryFile', () => { + it('uses direct argv and bounds a distro cleanup subprocess', async () => { + const run = vi.fn().mockResolvedValue({ stdout: '', stderr: '' }) + + await deleteWslFishHistoryFile('Ubuntu Test', 'orca_0123456789abcdef', run) + + expect(run).toHaveBeenCalledWith( + 'wsl.exe', + [ + '--distribution', + 'Ubuntu Test', + '--exec', + 'fish', + '--command', + expect.stringContaining('orca_0123456789abcdef_history') + ], + { timeout: 5_000, windowsHide: true } + ) + }) + + it('rejects an unsafe session before spawning', async () => { + const run = vi.fn() + + await deleteWslFishHistoryFile('Ubuntu', '../../user-history', run) + + expect(run).not.toHaveBeenCalled() + }) + + it('coalesces concurrent requests for the same distro and session', async () => { + // Why: the GC sweep and an explicit worktree delete can queue the same + // tombstone, and one wsl.exe launch per cleanup is enough. + let settle!: () => void + const run = vi.fn().mockReturnValue( + new Promise((resolve) => { + settle = () => resolve({ stdout: '', stderr: '' }) + }) + ) + const session = 'orca_0123456789abcdef' + + const first = deleteWslFishHistoryFile('Ubuntu', session, run) + const second = deleteWslFishHistoryFile('Ubuntu', session, run) + + expect(second).toBe(first) + expect(run).toHaveBeenCalledTimes(1) + settle() + await Promise.all([first, second]) + }) + + it('keeps distinct distros independent of each other', async () => { + const run = vi.fn().mockResolvedValue({ stdout: '', stderr: '' }) + + await Promise.all([ + deleteWslFishHistoryFile('Ubuntu', 'orca_0123456789abcdef', run), + deleteWslFishHistoryFile('Debian', 'orca_0123456789abcdef', run) + ]) + + expect(run).toHaveBeenCalledTimes(2) + }) + + it('permits a retry once a failed cleanup settles', async () => { + const run = vi.fn().mockRejectedValueOnce(new Error('distro offline')).mockResolvedValue({ + stdout: '', + stderr: '' + }) + const session = 'orca_0123456789abcdef' + + await expect(deleteWslFishHistoryFile('Ubuntu', session, run)).rejects.toThrow('distro offline') + await expect(deleteWslFishHistoryFile('Ubuntu', session, 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 new file mode 100644 index 00000000000..18aebb650a2 --- /dev/null +++ b/src/main/wsl-fish-history-cleanup.ts @@ -0,0 +1,81 @@ +import { execFile } from 'node:child_process' +import { promisify } from 'node:util' +import { isSafeFishHistorySession } from './fish-history-session' + +const execFileAsync = promisify(execFile) + +/** Deduped by distro+session, so a rescan that re-queues the same tombstone + * joins the running call instead of spawning a second `wsl.exe`. + * + * Not otherwise rate-limited: the caller admits at most + * MAX_PENDING_HISTORY_TREE_REMOVALS tombstones at a time, so fanout is bounded + * but can still be dozens of `wsl.exe` launches during a startup GC drain on a + * machine with many dead WSL fish worktrees. Narrowing that belongs with the + * tombstone scheduler that owns the admission, not here. */ +const cleanupsInFlight = new Map>() + +/** Why run this INSIDE the distro: the history file lives on the distro's own + * filesystem under its `$XDG_DATA_HOME`/`$HOME`, which only a shell in there + * can resolve. `string match -qr '^/'` rejects a relative XDG_DATA_HOME the + * same way fish itself does before falling back. */ +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, + session: string, + run: typeof execFileAsync = execFileAsync +): Promise { + if (!distro.trim() || !isSafeFishHistorySession(session)) { + return Promise.resolve() + } + const key = `${distro}\0${session}` + const existing = cleanupsInFlight.get(key) + if (existing) { + return existing + } + const cleanup = runCleanup(distro, session, run).finally(() => { + cleanupsInFlight.delete(key) + }) + cleanupsInFlight.set(key, cleanup) + return cleanup +} + +async function runCleanup( + distro: string, + session: string, + run: typeof execFileAsync +): Promise { + await run( + 'wsl.exe', + ['--distribution', distro, '--exec', 'fish', '--command', fishCleanupScript(session)], + { timeout: 5_000, windowsHide: true } + ) +} + +/** Test-only: drop in-flight state so one test's pending work cannot reach the next. */ +export function __resetWslFishHistoryCleanups(): void { + cleanupsInFlight.clear() +} + +/** Await every in-flight cleanup; for deterministic shutdown and tests. + * Why snapshot-and-drain rather than re-reading the map in the loop condition: + * an entry that has settled but whose `.finally` has not yet removed it would + * make `while (size > 0)` re-await a resolved promise forever, starving the + * event loop instead of returning. */ +export async function flushWslFishHistoryCleanups(): Promise { + const awaited = new Set>() + let pending = [...cleanupsInFlight.values()] + while (pending.length > 0) { + for (const cleanup of pending) { + awaited.add(cleanup) + } + await Promise.allSettled(pending) + pending = [...cleanupsInFlight.values()].filter((cleanup) => !awaited.has(cleanup)) + } +} diff --git a/src/main/zsh-scoped-histfile.live-shell.test.ts b/src/main/zsh-scoped-histfile.live-shell.test.ts new file mode 100644 index 00000000000..98af8914ec6 --- /dev/null +++ b/src/main/zsh-scoped-histfile.live-shell.test.ts @@ -0,0 +1,106 @@ +/** + * Real-zsh proof that a worktree-scoped HISTFILE survives shell startup. + * + * macOS `/etc/zshrc` assigns `HISTFILE=${ZDOTDIR:-$HOME}/.zsh_history` with no + * check-before-set, and it runs before every wrapper file Orca controls. So the + * value `injectHistoryEnv` put in the spawn env is already gone by the time the + * user reaches a prompt — and because ZDOTDIR still points at Orca's wrapper + * dir, the replacement lands inside it. Per-worktree history was therefore a + * silent no-op on the primary platform (#11044). + * + * Only a real zsh can show this: the string the wrapper emits looks correct + * either way, and the whole bug lives in what /etc/zshrc does between the spawn + * env and the first prompt. + */ +import { execFileSync, spawnSync } from 'node:child_process' +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' +import { getZshShellReadyRcfileContent } from './providers/local-pty-shell-ready-wrapper-generation' +import { getZshEnvTemplate, ZSH_HISTFILE_RESTORE_BLOCK } from './shell-templates' + +// Why probe and execute the same binary: guarding on `zsh` from PATH but then +// running a hardcoded `/bin/zsh` lets the guard pass on a host that installs zsh +// elsewhere, and the test fails for a missing binary rather than a wrapper +// defect. The absolute path is resolved once so the sandboxed PATH below cannot +// lose it. +const hasZsh = process.platform !== 'win32' && spawnSync('zsh', ['--version']).status === 0 +const ZSH_PATH = hasZsh + ? (spawnSync('sh', ['-c', 'command -v zsh'], { encoding: 'utf8' }).stdout || '').trim() + : '' +const itWithZsh = hasZsh ? it : it.skip + +function runLoginZsh(home: string, zdotdir: string, env: Record): string { + // -o noglobalrcs is deliberately NOT passed: /etc/zshrc is the thing under test. + return execFileSync(ZSH_PATH, ['-li', '-c', 'echo "RESULT=$HISTFILE"'], { + encoding: 'utf8', + timeout: 20_000, + env: { + PATH: '/usr/bin:/bin', + HOME: home, + ZDOTDIR: zdotdir, + ORCA_ORIG_ZDOTDIR: home, + ORCA_ZSHENV_SOURCE_DIR: home, + ...env + } + }) +} + +describe('worktree-scoped HISTFILE survives zsh startup', () => { + const withWrapper = (run: (home: string, zdotdir: string) => void): void => { + const home = mkdtempSync(join(tmpdir(), 'orca-scoped-histfile-')) + const zdotdir = join(home, 'shell-ready', 'zsh') + mkdirSync(zdotdir, { recursive: true }) + writeFileSync(join(zdotdir, '.zshenv'), getZshEnvTemplate(zdotdir)) + writeFileSync(join(zdotdir, '.zshrc'), getZshShellReadyRcfileContent()) + writeFileSync(join(zdotdir, '.zlogin'), `${ZSH_HISTFILE_RESTORE_BLOCK}\n`) + try { + run(home, zdotdir) + } finally { + rmSync(home, { recursive: true, force: true }) + } + } + + itWithZsh('keeps the injected path that a system zshrc would otherwise clobber', () => { + withWrapper((home, zdotdir) => { + const scoped = join(home, 'orca-history', 'zsh_history') + + const output = runLoginZsh(home, zdotdir, { HISTFILE: scoped, ORCA_HISTFILE: scoped }) + + expect(output).toContain(`RESULT=${scoped}`) + }) + }) + + itWithZsh('never leaves history inside Orca’s own wrapper directory', () => { + withWrapper((home, zdotdir) => { + const scoped = join(home, 'orca-history', 'zsh_history') + + const output = runLoginZsh(home, zdotdir, { HISTFILE: scoped, ORCA_HISTFILE: scoped }) + + // The exact failure mode of #11044: history written into shell-ready/zsh. + expect(output).not.toContain(zdotdir) + }) + }) + + itWithZsh('leaves HISTFILE exactly as an unwrapped zsh would when Orca injects nothing', () => { + // Why compared against an unwrapped run rather than asserted non-empty: what + // zsh defaults to is platform-specific. macOS `/etc/zshrc` assigns HISTFILE, + // so it is always set there; a stock Ubuntu zsh has no such file and leaves + // it EMPTY. The contract is that Orca's wrapper does not change it either + // way, which is the same assertion on both. + withWrapper((home, zdotdir) => { + const wrapped = runLoginZsh(home, zdotdir, {}) + const unwrapped = execFileSync(ZSH_PATH, ['-li', '-c', 'echo "RESULT=$HISTFILE"'], { + encoding: 'utf8', + timeout: 20_000, + env: { PATH: '/usr/bin:/bin', HOME: home } + }) + + const histfileOf = (output: string): string => + /^RESULT=(.*)$/m.exec(output)?.[1]?.trim() ?? '' + expect(histfileOf(wrapped)).toBe(histfileOf(unwrapped)) + expect(wrapped).not.toContain('ORCA_HISTFILE') + }) + }) +}) diff --git a/src/relay/pty-handler.ts b/src/relay/pty-handler.ts index c077378ebda..16d256e72a9 100644 --- a/src/relay/pty-handler.ts +++ b/src/relay/pty-handler.ts @@ -2,7 +2,7 @@ import type { IPty } from 'node-pty' import type * as NodePty from 'node-pty' import { existsSync } from 'node:fs' -import { join } from 'node:path' +import { basename, join } from 'node:path' import { randomUUID } from 'node:crypto' import { resolveWindowsGitBashShellPath } from '../main/git-bash' import { WINDOWS_GIT_BASH_SHELL } from '../shared/windows-terminal-shell' @@ -79,6 +79,12 @@ import { type AgentSessionOwnerBinding } from '../shared/agent-session-host-authority' import { createPtySlaveEchoProbe, readPtySlavePath } from '../shared/pty-slave-line-discipline-echo' +import { + deleteRelayFishHistory, + deleteRelayHistory, + injectRelayFishHistoryEnv, + injectRelayHistoryEnv +} from './terminal-history' // Why: only Linux compiles node-pty (no prebuilt), so the build-tools remedy is a closable setup gap // there and wrong advice anywhere node-pty ships one. The relay only sees an unloadable binding, never @@ -158,6 +164,7 @@ type ManagedPty = { shellPathEnv?: string envToDelete: string[] gitCredentialPromptGuarded: boolean + historyIsolationEnabled?: boolean startupCommand?: ManagedStartupCommand physicalExit?: PhysicalExitTracker forceKillSent?: boolean @@ -327,6 +334,8 @@ type SerializedPtyEntry = { envToDelete?: string[] /** Optional for state serialized by relays predating the credential guard. */ gitCredentialPromptGuarded?: boolean + /** Optional for state serialized by relays predating scoped history. */ + historyIsolationEnabled?: boolean agentSessionOwners?: AgentSessionOwnerBinding[] } @@ -860,6 +869,13 @@ export class PtyHandler { this.dispatcher.onRequest('pty.serialize', (p) => this.serialize(p)) this.dispatcher.onRequest('pty.revive', (p) => this.revive(p)) this.dispatcher.onRequest('pty.getProfiles', async () => listShellProfiles()) + this.dispatcher.onRequest('pty.deleteWorktreeHistory', async (p) => { + if (typeof p.worktreeId === 'string') { + deleteRelayFishHistory(p.worktreeId) + deleteRelayHistory(p.worktreeId) + } + return { ok: true } + }) this.dispatcher.onRequest('pty.closeStartupQueryAuthority', (p) => this.closeStartupQueryAuthority(p) ) @@ -1387,7 +1403,8 @@ export class PtyHandler { context?: RequestContext ): Promise { const env = params.env as Record | undefined - const worktreeId = env?.ORCA_WORKTREE_ID + const worktreeId = + typeof params.worktreeId === 'string' ? params.worktreeId : env?.ORCA_WORKTREE_ID const worktreePath = worktreeId ? splitWorktreeId(worktreeId)?.worktreePath : undefined const cwd = typeof params.cwd === 'string' ? params.cwd : resolveDefaultCwd() const finishCreation = this.beginPtyCreation([worktreePath, cwd]) @@ -1503,7 +1520,9 @@ export class PtyHandler { const shellOverride = typeof params.shellOverride === 'string' ? params.shellOverride.trim() : '' const resolvedShellOverride = resolvePtyShellOverride(shellOverride) - const shell = resolvedShellOverride || resolveDefaultShell() + const requestedEnvShell = + process.platform !== 'win32' && typeof env?.SHELL === 'string' ? env.SHELL.trim() : '' + const shell = resolvedShellOverride || requestedEnvShell || resolveDefaultShell() let id: string do { id = `pty-${this.nextId++}` @@ -1525,6 +1544,15 @@ export class PtyHandler { { id, paneKey, shell, command, launchAgent }, envToDelete ) + const worktreeId = + typeof params.worktreeId === 'string' ? params.worktreeId : env?.ORCA_WORKTREE_ID + const historyIsolationEnabled = params.historyIsolationEnabled === true + if (historyIsolationEnabled && worktreeId && basename(shell).toLowerCase().startsWith('fish')) { + injectRelayFishHistoryEnv(spawnEnv, worktreeId) + } + if (historyIsolationEnabled && worktreeId) { + injectRelayHistoryEnv(spawnEnv, worktreeId, shell) + } const launchCommandHint = resolveSetupAgentSequenceLaunchCommand(spawnEnv, command) // Why: SSH PTYs bypass main's host-env builder, so apply the guard after the relay merges its authoritative env. const gitCredentialPromptGuarded = applyTerminalGitCredentialPromptGuard(spawnEnv, { @@ -1591,7 +1619,6 @@ export class PtyHandler { paneKey: typeof params.paneKey === 'string' ? params.paneKey : paneKey, tabId: typeof params.tabId === 'string' ? params.tabId : tabId } - const worktreeId = typeof env?.ORCA_WORKTREE_ID === 'string' ? env.ORCA_WORKTREE_ID : undefined const startupIngressIntent = params.startupIngressVersion === PTY_STARTUP_INGRESS_VERSION ? parsePtyStartupIngressIntent(params.startupIngress) @@ -1612,6 +1639,7 @@ export class PtyHandler { ...(explicitTerm !== undefined ? { explicitTerm } : {}), envToDelete, gitCredentialPromptGuarded, + ...(historyIsolationEnabled ? { historyIsolationEnabled: true } : {}), shellPath: shell, shellCwd: cwd, shellPathEnv: spawnEnv.PATH, @@ -2034,6 +2062,7 @@ export class PtyHandler { ...(managed.explicitTerm !== undefined ? { explicitTerm: managed.explicitTerm } : {}), envToDelete: managed.envToDelete, gitCredentialPromptGuarded: managed.gitCredentialPromptGuarded, + ...(managed.historyIsolationEnabled ? { historyIsolationEnabled: true } : {}), ...(managed.terminalHandle ? { terminalHandle: managed.terminalHandle } : {}) }) } @@ -2097,11 +2126,22 @@ export class PtyHandler { // Why: serialized state may come from an older/untrusted client; reapply fresh-spawn bounds. const envToDelete = sanitizeEnvToDelete(entry.envToDelete) const shell = resolveDefaultShell() + const historyIsolationEnabled = entry.historyIsolationEnabled === true const spawnEnv = this.buildSpawnEnv( revivedEnv, { id: entry.id, paneKey: entry.paneKey, shell }, envToDelete ) + if ( + historyIsolationEnabled && + entry.worktreeId && + basename(shell).toLowerCase().startsWith('fish') + ) { + injectRelayFishHistoryEnv(spawnEnv, entry.worktreeId) + } + if (historyIsolationEnabled && entry.worktreeId) { + injectRelayHistoryEnv(spawnEnv, entry.worktreeId, shell) + } // Why: revive lacks the original launch command, so reuse the fresh-spawn guard decision (legacy defaults to unguarded). const gitCredentialPromptGuarded = entry.gitCredentialPromptGuarded === true if (gitCredentialPromptGuarded) { @@ -2137,6 +2177,7 @@ export class PtyHandler { ...(explicitTerm !== undefined ? { explicitTerm } : {}), envToDelete, gitCredentialPromptGuarded, + ...(historyIsolationEnabled ? { historyIsolationEnabled: true } : {}), ownerBackend: resolvePtyOwnerBackend({ platform: process.platform, shellPath: shell diff --git a/src/relay/pty-shell-launch.ts b/src/relay/pty-shell-launch.ts index eaa9ac15e4c..1af77e87b7c 100644 --- a/src/relay/pty-shell-launch.ts +++ b/src/relay/pty-shell-launch.ts @@ -1,28 +1,14 @@ -import { chmodSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs' import { homedir } from 'node:os' -import { dirname, join } from 'node:path' -import { getPosixOmpShellWrapper } from '../main/pty/omp-shell-wrapper' -import { - BASH_PROMPT_COMMAND_COMPOSITION_BLOCK, - getZshFinalZdotdirRestoreBlock, - getZshShellReadyMarkerRegistrationBlock, - SHELL_STARTUP_IDENTITY_MARKER_BLOCK, - getZshStartupFileSourceBlock -} from '../main/shell-templates' - +import { join } from 'node:path' +import { ensureOverlayRestoreWrappers } from './pty-shell-overlay-wrappers' const RELAY_SHELL_READY_DIR = '.orca-relay/shell-ready' const POSIX_LOGIN_ARGS = ['-l'] -const SHELL_READY_MARKER_ESCAPED = '\\033]777;orca-shell-ready\\007' export type RelayShellLaunchConfig = { args: string[] env: Record } -function quotePosixSingle(value: string): string { - return `'${value.replace(/'/g, `'\\''`)}'` -} - function shellBasename(shellPath: string): string { return shellPath.replace(/\\/g, '/').split('/').pop()?.toLowerCase() ?? '' } @@ -81,189 +67,6 @@ function resolveOriginalZdotdir(env: Record): string { ) } -function ensureOverlayRestoreWrappers(root: string): void { - const zshDir = join(root, 'zsh') - const bashDir = join(root, 'bash') - - const zshEnv = `# Orca relay zsh overlay wrapper -${SHELL_STARTUP_IDENTITY_MARKER_BLOCK} -export ORCA_ORIG_ZDOTDIR="\${ORCA_ORIG_ZDOTDIR:-$HOME}" -case "\${ORCA_ORIG_ZDOTDIR%/}" in - */shell-ready/zsh) export ORCA_ORIG_ZDOTDIR="$HOME" ;; -esac -[[ -f "$ORCA_ORIG_ZDOTDIR/.zshenv" ]] && source "$ORCA_ORIG_ZDOTDIR/.zshenv" -export ORCA_USER_ZDOTDIR="\${ZDOTDIR:-\${ORCA_ORIG_ZDOTDIR:-$HOME}}" -case "\${ORCA_USER_ZDOTDIR%/}" in - */shell-ready/zsh) export ORCA_USER_ZDOTDIR="$HOME" ;; -esac -export ZDOTDIR=${quotePosixSingle(zshDir)} -` - const zshProfile = `# Orca relay zsh overlay wrapper -${getZshStartupFileSourceBlock({ - fileName: '.zprofile', - homeExpression: '"${ORCA_USER_ZDOTDIR:-${ORCA_ORIG_ZDOTDIR:-$HOME}}"' -})} -` - const zshRc = `# Orca relay zsh overlay wrapper -${getZshStartupFileSourceBlock({ - fileName: '.zshrc', - homeExpression: '"${ORCA_USER_ZDOTDIR:-${ORCA_ORIG_ZDOTDIR:-$HOME}}"', - interactiveOnly: true -})} -if [[ ! -o login ]]; then - # Why: remote startup files can re-export user defaults after relay spawn. - [[ -n "\${ORCA_OPENCODE_CONFIG_DIR:-}" ]] && export OPENCODE_CONFIG_DIR="\${ORCA_OPENCODE_CONFIG_DIR}" - [[ -n "\${ORCA_MIMOCODE_HOME:-}" ]] && export MIMOCODE_HOME="\${ORCA_MIMOCODE_HOME}" - [[ -n "\${ORCA_REMOTE_CLI_BIN_DIR:-}" ]] && case ":$PATH:" in *:"\${ORCA_REMOTE_CLI_BIN_DIR}":*) ;; *) export PATH="\${ORCA_REMOTE_CLI_BIN_DIR}:$PATH" ;; esac - ${getPosixOmpShellWrapper()} -fi -if [[ ! -o login ]]; then -${getZshFinalZdotdirRestoreBlock('"${ORCA_USER_ZDOTDIR:-${ORCA_ORIG_ZDOTDIR:-$HOME}}"')} -fi -` - const zshLogin = `# Orca relay zsh overlay wrapper -${getZshStartupFileSourceBlock({ - fileName: '.zlogin', - homeExpression: '"${ORCA_USER_ZDOTDIR:-${ORCA_ORIG_ZDOTDIR:-$HOME}}"', - interactiveOnly: true -})} -# Why: .zlogin is the final zsh login startup file before the prompt. -[[ -n "\${ORCA_OPENCODE_CONFIG_DIR:-}" ]] && export OPENCODE_CONFIG_DIR="\${ORCA_OPENCODE_CONFIG_DIR}" -[[ -n "\${ORCA_MIMOCODE_HOME:-}" ]] && export MIMOCODE_HOME="\${ORCA_MIMOCODE_HOME}" -[[ -n "\${ORCA_REMOTE_CLI_BIN_DIR:-}" ]] && case ":$PATH:" in *:"\${ORCA_REMOTE_CLI_BIN_DIR}":*) ;; *) export PATH="\${ORCA_REMOTE_CLI_BIN_DIR}:$PATH" ;; esac -${getPosixOmpShellWrapper()} -${getZshFinalZdotdirRestoreBlock('"${ORCA_USER_ZDOTDIR:-${ORCA_ORIG_ZDOTDIR:-$HOME}}"')} -${getZshShellReadyMarkerRegistrationBlock(SHELL_READY_MARKER_ESCAPED)} -` - const bashRc = `# Orca relay bash overlay wrapper -${SHELL_STARTUP_IDENTITY_MARKER_BLOCK} -[[ -f /etc/profile ]] && source /etc/profile -if [[ -f "$HOME/.bash_profile" ]]; then - source "$HOME/.bash_profile" -elif [[ -f "$HOME/.bash_login" ]]; then - source "$HOME/.bash_login" -elif [[ -f "$HOME/.profile" ]]; then - source "$HOME/.profile" -fi -# Why: enable bracketed paste so Orca can deliver a multiline startup prompt as -# a single literal paste (ESC[200~…ESC[201~); without it, older readline builds -# treat each embedded newline as Enter and mangle the prompt into PS2 -# continuation. Modern readline defaults this on; force it for the rest. -[[ $- == *i* ]] && bind 'set enable-bracketed-paste on' 2>/dev/null -# Why: remote startup files can re-export user defaults after relay spawn. -[[ -n "\${ORCA_OPENCODE_CONFIG_DIR:-}" ]] && export OPENCODE_CONFIG_DIR="\${ORCA_OPENCODE_CONFIG_DIR}" -[[ -n "\${ORCA_MIMOCODE_HOME:-}" ]] && export MIMOCODE_HOME="\${ORCA_MIMOCODE_HOME}" -[[ -n "\${ORCA_REMOTE_CLI_BIN_DIR:-}" ]] && case ":$PATH:" in *:"\${ORCA_REMOTE_CLI_BIN_DIR}":*) ;; *) export PATH="\${ORCA_REMOTE_CLI_BIN_DIR}:$PATH" ;; esac -${getPosixOmpShellWrapper()} -# Why: SSH bash sessions need the same command lifecycle markers as local -# bash so agent rows stop showing "working" when the foreground command exits. -__orca_initializing_wrapper=1 -__orca_osc133_precmd() { - local exit_code=$? - __orca_in_prompt_command=1 - if [[ -n "\${__orca_in_command:-}" ]]; then - printf "\\033]133;D;%s\\007" "$exit_code" - unset __orca_in_command - fi - printf "\\033]133;A\\007" - return "$exit_code" -} -__orca_osc133_prompt_done() { - unset __orca_in_prompt_command; __orca_adopt_outer_debug_trap - trap '__orca_osc133_preexec' DEBUG -} -__orca_osc133_preexec() { - if [[ -n "\${__orca_prompt_status_capture_command:-}" && "$BASH_COMMAND" == "$__orca_prompt_status_capture_command" ]]; then - unset __orca_initial_prompt - __orca_in_legacy_prompt_wrapper=1 - return 0 - fi - if [[ -n "\${__orca_initializing_wrapper:-}\${__orca_in_debug_capture:-}\${__orca_initial_prompt:-}\${__orca_in_prompt_dispatch:-}\${__orca_in_legacy_prompt_wrapper:-}\${__orca_in_prompt_command:-}" ]]; then - [[ -z "\${__orca_initializing_wrapper:-}\${__orca_in_debug_capture:-}" ]] || return 0 - if [[ -n "\${__orca_initial_prompt:-}" && "$BASH_COMMAND" == "__orca_osc133_precmd" ]]; then - unset __orca_initial_prompt; return 0 - fi - if [[ -n "\${__orca_in_prompt_dispatch:-}" ]]; then - [[ -n "\${__orca_dispatching_user_prompt_command:-}" ]] || return 0 - if [[ "\${FUNCNAME[1]:-}" == "__orca_run_prompt_command_array" ]]; then - case "$BASH_COMMAND" in - '(( __orca_exit_code == 0 ))'|'__orca_restore_prompt_status "$__orca_exit_code"'|'eval "$__orca_prompt_part"'|'eval "$__orca_final_prompt_command"'|__orca_dispatching_user_prompt_command=*|__orca_osc133_precmd|__orca_osc133_prompt_done|__orca_prompt_mark) return 0 ;; - esac - fi - elif [[ "\${FUNCNAME[1]:-}" == "__orca_run_prompt_command_array" || "$BASH_COMMAND" == "__orca_run_prompt_command_array" ]]; then - return 0 - fi - [[ -z "\${__orca_in_legacy_prompt_wrapper:-}" || -n "\${__orca_dispatching_user_prompt_command:-}" ]] || return 0 - if [[ -n "\${__orca_in_prompt_command:-}" && "$BASH_COMMAND" == "__orca_in_debug_capture=1" ]]; then - return 0 - fi - fi - case "\${FUNCNAME[1]:-}" in __orca_osc133_*|__orca_prompt_mark|__orca_restore_prompt_status) return 0 ;; esac - case "$BASH_COMMAND" in __orca_osc133_precmd|__orca_osc133_prompt_done|__orca_prompt_mark) return 0 ;; esac - __orca_run_user_debug_trap - [[ -z "\${__orca_in_prompt_command:-}" ]] || return 0 - [[ -z "\${__orca_in_command:-}" ]] || return 0 - printf "\\033]133;C\\007" - __orca_in_command=1 -} -${BASH_PROMPT_COMMAND_COMPOSITION_BLOCK} -__orca_prepend_prompt_command "__orca_osc133_precmd" -# Why: SSH startup commands are renderer-delivered; emit the same internal -# readiness marker as local shells only when that delivery mode asks for it. -if [[ "\${ORCA_SHELL_READY_MARKER:-0}" == "1" ]]; then - __orca_prompt_mark() { - printf "${SHELL_READY_MARKER_ESCAPED}" - } - __orca_append_prompt_command "__orca_prompt_mark" -fi -__orca_append_prompt_command '__orca_in_debug_capture=1; __orca_prompt_had_functrace=""; if [[ -o functrace ]]; then __orca_prompt_had_functrace=1; set +T; fi; __orca_outer_debug_trap_spec="$(trap -p DEBUG)"; [[ -z "$__orca_prompt_had_functrace" ]] || set -T; unset __orca_prompt_had_functrace __orca_in_debug_capture' -__orca_append_prompt_command "__orca_osc133_prompt_done" -__orca_had_functrace="" -[[ -o functrace ]] && __orca_had_functrace=1 -set +T -__orca_debug_trap_spec="$(trap -p DEBUG)" -[[ -z "$__orca_had_functrace" ]] || set -T -if [[ -n "$__orca_debug_trap_spec" && "$__orca_debug_trap_spec" != "trap -- '__orca_osc133_preexec' DEBUG" ]]; then - __orca_debug_trap_command="\${__orca_debug_trap_spec#trap -- }" - __orca_debug_trap_command="\${__orca_debug_trap_command% DEBUG}" - eval "__orca_user_debug_trap=$__orca_debug_trap_command" -fi -unset __orca_debug_trap_spec __orca_debug_trap_command __orca_had_functrace -unset -f __orca_normalize_prompt_command_part __orca_normalize_prompt_command __orca_prepend_prompt_command __orca_append_prompt_command -unset __orca_prompt_command_normalized -# Why: arm DEBUG after wrapper setup so the relay rcfile itself does not emit -# fake command-start/end markers before the first prompt. -__orca_initial_prompt=1 -trap '__orca_osc133_preexec' DEBUG -unset __orca_initializing_wrapper -` - - const files = [ - [join(zshDir, '.zshenv'), zshEnv], - [join(zshDir, '.zprofile'), zshProfile], - [join(zshDir, '.zshrc'), zshRc], - [join(zshDir, '.zlogin'), zshLogin], - [join(bashDir, 'rcfile'), bashRc] - ] as const - - for (const [path, content] of files) { - mkdirSync(dirname(path), { recursive: true }) - let existing: string | null = null - try { - existing = readFileSync(path, 'utf8') - } catch { - existing = null - } - // Why: relay wrapper files persist under ~/.orca-relay across app - // upgrades. Existence alone is not enough; stale wrappers would miss - // later fixes such as preserving post-.zshenv ZDOTDIR. - if (existing !== content) { - writeFileSync(path, content, 'utf8') - } - chmodSync(path, 0o644) - } -} - export function getRelayShellLaunchConfig( shellPath: string, env: Record, diff --git a/src/relay/pty-shell-overlay-wrappers.ts b/src/relay/pty-shell-overlay-wrappers.ts new file mode 100644 index 00000000000..1fa2e9b821a --- /dev/null +++ b/src/relay/pty-shell-overlay-wrappers.ts @@ -0,0 +1,207 @@ +import { chmodSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs' +import { dirname, join } from 'node:path' +import { getPosixOmpShellWrapper } from '../main/pty/omp-shell-wrapper' +import { + BASH_PROMPT_COMMAND_COMPOSITION_BLOCK, + getZshFinalZdotdirRestoreBlock, + getZshShellReadyMarkerRegistrationBlock, + SHELL_STARTUP_IDENTITY_MARKER_BLOCK, + ZSH_HISTFILE_RESTORE_BLOCK, + getZshStartupFileSourceBlock +} from '../main/shell-templates' + +/** Writes the zsh/bash overlay wrapper files a relay-spawned shell sources. + * Split from pty-shell-launch.ts so the launch-config decisions stay readable + * next to each other rather than buried under ~150 lines of shell templates. */ + +const SHELL_READY_MARKER_ESCAPED = '\\033]777;orca-shell-ready\\007' + +function quotePosixSingle(value: string): string { + return `'${value.replace(/'/g, `'\\''`)}'` +} + +export function ensureOverlayRestoreWrappers(root: string): void { + const zshDir = join(root, 'zsh') + const bashDir = join(root, 'bash') + + const zshEnv = `# Orca relay zsh overlay wrapper +${SHELL_STARTUP_IDENTITY_MARKER_BLOCK} +export ORCA_ORIG_ZDOTDIR="\${ORCA_ORIG_ZDOTDIR:-$HOME}" +case "\${ORCA_ORIG_ZDOTDIR%/}" in + */shell-ready/zsh) export ORCA_ORIG_ZDOTDIR="$HOME" ;; +esac +[[ -f "$ORCA_ORIG_ZDOTDIR/.zshenv" ]] && source "$ORCA_ORIG_ZDOTDIR/.zshenv" +export ORCA_USER_ZDOTDIR="\${ZDOTDIR:-\${ORCA_ORIG_ZDOTDIR:-$HOME}}" +case "\${ORCA_USER_ZDOTDIR%/}" in + */shell-ready/zsh) export ORCA_USER_ZDOTDIR="$HOME" ;; +esac +export ZDOTDIR=${quotePosixSingle(zshDir)} +` + const zshProfile = `# Orca relay zsh overlay wrapper +${getZshStartupFileSourceBlock({ + fileName: '.zprofile', + homeExpression: '"${ORCA_USER_ZDOTDIR:-${ORCA_ORIG_ZDOTDIR:-$HOME}}"' +})} +` + const zshRc = `# Orca relay zsh overlay wrapper +${getZshStartupFileSourceBlock({ + fileName: '.zshrc', + homeExpression: '"${ORCA_USER_ZDOTDIR:-${ORCA_ORIG_ZDOTDIR:-$HOME}}"', + interactiveOnly: true +})} +if [[ ! -o login ]]; then + # Why: remote startup files can re-export user defaults after relay spawn. + [[ -n "\${ORCA_OPENCODE_CONFIG_DIR:-}" ]] && export OPENCODE_CONFIG_DIR="\${ORCA_OPENCODE_CONFIG_DIR}" + [[ -n "\${ORCA_MIMOCODE_HOME:-}" ]] && export MIMOCODE_HOME="\${ORCA_MIMOCODE_HOME}" + [[ -n "\${ORCA_REMOTE_CLI_BIN_DIR:-}" ]] && case ":$PATH:" in *:"\${ORCA_REMOTE_CLI_BIN_DIR}":*) ;; *) export PATH="\${ORCA_REMOTE_CLI_BIN_DIR}:$PATH" ;; esac + ${getPosixOmpShellWrapper()} +${ZSH_HISTFILE_RESTORE_BLOCK} +fi +if [[ ! -o login ]]; then +${getZshFinalZdotdirRestoreBlock('"${ORCA_USER_ZDOTDIR:-${ORCA_ORIG_ZDOTDIR:-$HOME}}"')} +fi +` + const zshLogin = `# Orca relay zsh overlay wrapper +${getZshStartupFileSourceBlock({ + fileName: '.zlogin', + homeExpression: '"${ORCA_USER_ZDOTDIR:-${ORCA_ORIG_ZDOTDIR:-$HOME}}"', + interactiveOnly: true +})} +# Why: .zlogin is the final zsh login startup file before the prompt. +[[ -n "\${ORCA_OPENCODE_CONFIG_DIR:-}" ]] && export OPENCODE_CONFIG_DIR="\${ORCA_OPENCODE_CONFIG_DIR}" +[[ -n "\${ORCA_MIMOCODE_HOME:-}" ]] && export MIMOCODE_HOME="\${ORCA_MIMOCODE_HOME}" +[[ -n "\${ORCA_REMOTE_CLI_BIN_DIR:-}" ]] && case ":$PATH:" in *:"\${ORCA_REMOTE_CLI_BIN_DIR}":*) ;; *) export PATH="\${ORCA_REMOTE_CLI_BIN_DIR}:$PATH" ;; esac +${getPosixOmpShellWrapper()} +${ZSH_HISTFILE_RESTORE_BLOCK} +${getZshFinalZdotdirRestoreBlock('"${ORCA_USER_ZDOTDIR:-${ORCA_ORIG_ZDOTDIR:-$HOME}}"')} +${getZshShellReadyMarkerRegistrationBlock(SHELL_READY_MARKER_ESCAPED)} +` + const bashRc = `# Orca relay bash overlay wrapper +${SHELL_STARTUP_IDENTITY_MARKER_BLOCK} +[[ -f /etc/profile ]] && source /etc/profile +if [[ -f "$HOME/.bash_profile" ]]; then + source "$HOME/.bash_profile" +elif [[ -f "$HOME/.bash_login" ]]; then + source "$HOME/.bash_login" +elif [[ -f "$HOME/.profile" ]]; then + source "$HOME/.profile" +fi +# Why: enable bracketed paste so Orca can deliver a multiline startup prompt as +# a single literal paste (ESC[200~…ESC[201~); without it, older readline builds +# treat each embedded newline as Enter and mangle the prompt into PS2 +# continuation. Modern readline defaults this on; force it for the rest. +[[ $- == *i* ]] && bind 'set enable-bracketed-paste on' 2>/dev/null +# Why: remote startup files can re-export user defaults after relay spawn. +[[ -n "\${ORCA_OPENCODE_CONFIG_DIR:-}" ]] && export OPENCODE_CONFIG_DIR="\${ORCA_OPENCODE_CONFIG_DIR}" +[[ -n "\${ORCA_MIMOCODE_HOME:-}" ]] && export MIMOCODE_HOME="\${ORCA_MIMOCODE_HOME}" +[[ -n "\${ORCA_REMOTE_CLI_BIN_DIR:-}" ]] && case ":$PATH:" in *:"\${ORCA_REMOTE_CLI_BIN_DIR}":*) ;; *) export PATH="\${ORCA_REMOTE_CLI_BIN_DIR}:$PATH" ;; esac +${getPosixOmpShellWrapper()} +${ZSH_HISTFILE_RESTORE_BLOCK} +# Why: SSH bash sessions need the same command lifecycle markers as local +# bash so agent rows stop showing "working" when the foreground command exits. +__orca_initializing_wrapper=1 +__orca_osc133_precmd() { + local exit_code=$? + __orca_in_prompt_command=1 + if [[ -n "\${__orca_in_command:-}" ]]; then + printf "\\033]133;D;%s\\007" "$exit_code" + unset __orca_in_command + fi + printf "\\033]133;A\\007" + return "$exit_code" +} +__orca_osc133_prompt_done() { + unset __orca_in_prompt_command; __orca_adopt_outer_debug_trap + trap '__orca_osc133_preexec' DEBUG +} +__orca_osc133_preexec() { + if [[ -n "\${__orca_prompt_status_capture_command:-}" && "$BASH_COMMAND" == "$__orca_prompt_status_capture_command" ]]; then + unset __orca_initial_prompt + __orca_in_legacy_prompt_wrapper=1 + return 0 + fi + if [[ -n "\${__orca_initializing_wrapper:-}\${__orca_in_debug_capture:-}\${__orca_initial_prompt:-}\${__orca_in_prompt_dispatch:-}\${__orca_in_legacy_prompt_wrapper:-}\${__orca_in_prompt_command:-}" ]]; then + [[ -z "\${__orca_initializing_wrapper:-}\${__orca_in_debug_capture:-}" ]] || return 0 + if [[ -n "\${__orca_initial_prompt:-}" && "$BASH_COMMAND" == "__orca_osc133_precmd" ]]; then + unset __orca_initial_prompt; return 0 + fi + if [[ -n "\${__orca_in_prompt_dispatch:-}" ]]; then + [[ -n "\${__orca_dispatching_user_prompt_command:-}" ]] || return 0 + if [[ "\${FUNCNAME[1]:-}" == "__orca_run_prompt_command_array" ]]; then + case "$BASH_COMMAND" in + '(( __orca_exit_code == 0 ))'|'__orca_restore_prompt_status "$__orca_exit_code"'|'eval "$__orca_prompt_part"'|'eval "$__orca_final_prompt_command"'|__orca_dispatching_user_prompt_command=*|__orca_osc133_precmd|__orca_osc133_prompt_done|__orca_prompt_mark) return 0 ;; + esac + fi + elif [[ "\${FUNCNAME[1]:-}" == "__orca_run_prompt_command_array" || "$BASH_COMMAND" == "__orca_run_prompt_command_array" ]]; then + return 0 + fi + [[ -z "\${__orca_in_legacy_prompt_wrapper:-}" || -n "\${__orca_dispatching_user_prompt_command:-}" ]] || return 0 + if [[ -n "\${__orca_in_prompt_command:-}" && "$BASH_COMMAND" == "__orca_in_debug_capture=1" ]]; then + return 0 + fi + fi + case "\${FUNCNAME[1]:-}" in __orca_osc133_*|__orca_prompt_mark|__orca_restore_prompt_status) return 0 ;; esac + case "$BASH_COMMAND" in __orca_osc133_precmd|__orca_osc133_prompt_done|__orca_prompt_mark) return 0 ;; esac + __orca_run_user_debug_trap + [[ -z "\${__orca_in_prompt_command:-}" ]] || return 0 + [[ -z "\${__orca_in_command:-}" ]] || return 0 + printf "\\033]133;C\\007" + __orca_in_command=1 +} +${BASH_PROMPT_COMMAND_COMPOSITION_BLOCK} +__orca_prepend_prompt_command "__orca_osc133_precmd" +# Why: SSH startup commands are renderer-delivered; emit the same internal +# readiness marker as local shells only when that delivery mode asks for it. +if [[ "\${ORCA_SHELL_READY_MARKER:-0}" == "1" ]]; then + __orca_prompt_mark() { + printf "${SHELL_READY_MARKER_ESCAPED}" + } + __orca_append_prompt_command "__orca_prompt_mark" +fi +__orca_append_prompt_command '__orca_in_debug_capture=1; __orca_prompt_had_functrace=""; if [[ -o functrace ]]; then __orca_prompt_had_functrace=1; set +T; fi; __orca_outer_debug_trap_spec="$(trap -p DEBUG)"; [[ -z "$__orca_prompt_had_functrace" ]] || set -T; unset __orca_prompt_had_functrace __orca_in_debug_capture' +__orca_append_prompt_command "__orca_osc133_prompt_done" +__orca_had_functrace="" +[[ -o functrace ]] && __orca_had_functrace=1 +set +T +__orca_debug_trap_spec="$(trap -p DEBUG)" +[[ -z "$__orca_had_functrace" ]] || set -T +if [[ -n "$__orca_debug_trap_spec" && "$__orca_debug_trap_spec" != "trap -- '__orca_osc133_preexec' DEBUG" ]]; then + __orca_debug_trap_command="\${__orca_debug_trap_spec#trap -- }" + __orca_debug_trap_command="\${__orca_debug_trap_command% DEBUG}" + eval "__orca_user_debug_trap=$__orca_debug_trap_command" +fi +unset __orca_debug_trap_spec __orca_debug_trap_command __orca_had_functrace +unset -f __orca_normalize_prompt_command_part __orca_normalize_prompt_command __orca_prepend_prompt_command __orca_append_prompt_command +unset __orca_prompt_command_normalized +# Why: arm DEBUG after wrapper setup so the relay rcfile itself does not emit +# fake command-start/end markers before the first prompt. +__orca_initial_prompt=1 +trap '__orca_osc133_preexec' DEBUG +unset __orca_initializing_wrapper +` + + const files = [ + [join(zshDir, '.zshenv'), zshEnv], + [join(zshDir, '.zprofile'), zshProfile], + [join(zshDir, '.zshrc'), zshRc], + [join(zshDir, '.zlogin'), zshLogin], + [join(bashDir, 'rcfile'), bashRc] + ] as const + + for (const [path, content] of files) { + mkdirSync(dirname(path), { recursive: true }) + let existing: string | null = null + try { + existing = readFileSync(path, 'utf8') + } catch { + existing = null + } + // Why: relay wrapper files persist under ~/.orca-relay across app + // upgrades. Existence alone is not enough; stale wrappers would miss + // later fixes such as preserving post-.zshenv ZDOTDIR. + if (existing !== content) { + writeFileSync(path, content, 'utf8') + } + chmodSync(path, 0o644) + } +} diff --git a/src/relay/terminal-history.test.ts b/src/relay/terminal-history.test.ts new file mode 100644 index 00000000000..b6c59e332d3 --- /dev/null +++ b/src/relay/terminal-history.test.ts @@ -0,0 +1,55 @@ +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' +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') +const historyPrefix = hashWorktreeId(worktreeId) + +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) => { + const env: Record = {} + const dir = injectRelayHistoryEnv(env, worktreeId, shell) + expect(dir).toBe(historyDir) + expect(env.HISTFILE).toBe( + join( + historyDir, + `${historyPrefix}-${shell.endsWith('bash') ? 'bash_history' : 'zsh_history'}` + ) + ) + + const custom = { HISTFILE: '/custom/history' } + expect(injectRelayHistoryEnv(custom, worktreeId, shell)).toBeNull() + expect(custom.HISTFILE).toBe('/custom/history') + }) + + it('does not scope unsupported shells and cleans up idempotently', () => { + const env: Record = {} + expect(injectRelayHistoryEnv(env, worktreeId, '/bin/fish')).toBeNull() + deleteRelayHistory(worktreeId) + deleteRelayHistory(worktreeId) + expect(existsSync(join(historyDir, `${historyPrefix}-bash_history`))).toBe(false) + }) + + it.skipIf(process.platform === 'win32')('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 new file mode 100644 index 00000000000..37335644df1 --- /dev/null +++ b/src/relay/terminal-history.ts @@ -0,0 +1,134 @@ +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' +import { + deleteFishHistoryFile, + relayFishHistorySessionName, + resolveFishHistoryDir +} from '../main/fish-history-session' + +const HISTORY_ROOT = join(homedir(), '.orca-remote', 'terminal-history') + +function historyFilename(shell: string): string | null { + const name = basename(shell).toLowerCase() + if (name.startsWith('bash')) { + return 'bash_history' + } + if (name.startsWith('zsh')) { + return 'zsh_history' + } + return null +} + +export function injectRelayHistoryEnv( + env: Record, + worktreeId: string, + shell: string +): string | null { + // Why first: same reason as the desktop path — an inherited ORCA_HISTFILE + // would otherwise survive every early return below and let the remote wrapper + // re-export another worktree's history path. + delete env.ORCA_HISTFILE + if (env.HISTFILE) { + return null + } + const filename = historyFilename(shell) + if (!filename) { + return null + } + try { + 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 + // Why a second variable: a remote macOS `/etc/zshrc` assigns HISTFILE + // unconditionally before the wrapper runs, so the injected value is gone by + // the first prompt. The wrapper restores it from here (#11044) — the same + // contract the desktop PTY path uses. + env.ORCA_HISTFILE = path + return HISTORY_ROOT + } catch { + return null + } +} + +export function deleteRelayHistory(worktreeId: string): void { + try { + 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)}` + ) + } +} + +/** fish keeps history in its own data dir under a session NAME, so the relay + * isolates it the same way the desktop app does and deletes by that name. + * No metadata file is needed: the name is a pure function of the worktree id. */ +export function injectRelayFishHistoryEnv(env: Record, worktreeId: string): void { + if (env.fish_history) { + return + } + env.fish_history = relayFishHistorySessionName(hashWorktreeId(worktreeId)) +} + +export function deleteRelayFishHistory(worktreeId: string, env?: NodeJS.ProcessEnv): void { + deleteFishHistoryFile(relayFishHistorySessionName(hashWorktreeId(worktreeId)), [ + resolveFishHistoryDir(env) + ]) +} diff --git a/src/renderer/src/components/settings/CliSkillRuntimeSetup.tsx b/src/renderer/src/components/settings/CliSkillRuntimeSetup.tsx index 913b6356322..4206fb46c87 100644 --- a/src/renderer/src/components/settings/CliSkillRuntimeSetup.tsx +++ b/src/renderer/src/components/settings/CliSkillRuntimeSetup.tsx @@ -229,9 +229,8 @@ function wrapWindowsSkillCommandWithNpxPrerequisite( } function isPosixFamilyWindowsShellConfigured(): boolean { - return ( - resolveWindowsShellStartupFamily(useAppStore.getState().settings?.terminalWindowsShell) === - 'posix' + return ['posix', 'unix'].includes( + resolveWindowsShellStartupFamily(useAppStore.getState().settings?.terminalWindowsShell) ) } diff --git a/src/renderer/src/components/sidebar/folder-workspace-composer-submit.test.ts b/src/renderer/src/components/sidebar/folder-workspace-composer-submit.test.ts index 749038df3e4..a96564b6652 100644 --- a/src/renderer/src/components/sidebar/folder-workspace-composer-submit.test.ts +++ b/src/renderer/src/components/sidebar/folder-workspace-composer-submit.test.ts @@ -588,7 +588,7 @@ describe('submitFolderWorkspaceCreate', () => { 'folder-workspace-1', expect.objectContaining({ startup: expect.objectContaining({ - command: "claude 'Use Bob'\\''s POSIX startup'" + command: `claude 'Use Bob'"'"'s POSIX startup'` }) }) ) diff --git a/src/renderer/src/components/terminal-pane/pty-connection-cold-restore-agent-resume.test.ts b/src/renderer/src/components/terminal-pane/pty-connection-cold-restore-agent-resume.test.ts index 8e29afb8763..35b37855238 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-cold-restore-agent-resume.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-cold-restore-agent-resume.test.ts @@ -298,8 +298,7 @@ describe('connectPanePty', () => { expect(transport.connect).toHaveBeenCalledWith( expect.objectContaining({ sessionId: 'lost-pty', - command: - "codex '--dangerously-bypass-approvals-and-sandbox' 'resume' 'codex-session-1'\\''s'", + command: `codex '--dangerously-bypass-approvals-and-sandbox' 'resume' 'codex-session-1'"'"'s'`, env: expect.objectContaining({ ORCA_PANE_KEY: paneKey, ORCA_TAB_ID: 'tab-1', diff --git a/src/renderer/src/lib/ai-vault-resume-command.test.ts b/src/renderer/src/lib/ai-vault-resume-command.test.ts index f89f4054bf4..57dd3499bc5 100644 --- a/src/renderer/src/lib/ai-vault-resume-command.test.ts +++ b/src/renderer/src/lib/ai-vault-resume-command.test.ts @@ -306,7 +306,7 @@ describe('ai vault resume command runtime', () => { } }) ).toBe( - "unset CODEX_HOME; unset ORCA_CODEX_HOME; cd '/home/alice/repo' && codex 'resume' 'session one'" + `cd '/home/alice/repo' && env -u CODEX_HOME -u ORCA_CODEX_HOME codex 'resume' 'session one'` ) }) @@ -598,7 +598,7 @@ describe('ai vault resume command runtime', () => { }) expect(command).toBe( - "unset CODEX_HOME; unset ORCA_CODEX_HOME; cd '/home/alice/repo' && codex 'resume' 'session one'" + `cd '/home/alice/repo' && env -u CODEX_HOME -u ORCA_CODEX_HOME codex 'resume' 'session one'` ) expect(command).not.toContain('/retired/shared-home') }) diff --git a/src/renderer/src/lib/ai-vault-resume-command.ts b/src/renderer/src/lib/ai-vault-resume-command.ts index 92e826f83a9..45943e8f9fa 100644 --- a/src/renderer/src/lib/ai-vault-resume-command.ts +++ b/src/renderer/src/lib/ai-vault-resume-command.ts @@ -16,7 +16,6 @@ import { } from '../../../shared/tui-agent-launch-defaults' import { parseWslUncPath } from '../../../shared/wsl-paths' import type { AgentStartupShell } from '../../../shared/tui-agent-startup-shell' -import { clearEnvCommand, commandSeparator } from '../../../shared/tui-agent-startup-shell' import type { AppState } from '@/store/types' import type { AiVaultSessionDragPayload } from '@/lib/ai-vault-session-drag' import { getLocalProjectExecutionRuntimeContext } from '@/lib/local-preflight-context' @@ -64,16 +63,16 @@ type AiVaultResumeWorktreeArgs = { } export function buildAiVaultResumeCopyCommandForWorktree(args: AiVaultResumeWorktreeArgs): string { - const command = buildAiVaultResumeForWorktree(args, true).command - if (args.session.agent !== 'codex' || args.session.codexHome !== null) { - return command - } - const shell = resolveAiVaultResumeShell(args) - const separator = commandSeparator(shell) - const clearHomes = ['CODEX_HOME', 'ORCA_CODEX_HOME'] - .map((name) => clearEnvCommand(name, shell)) - .join(separator) - return `${clearHomes}${separator}${command}` + // Why an `env -u` prefix on the agent rather than a preceding clear statement: + // this text is COPIED, so it runs in a shell Orca never spawned and cannot + // seed. A clear statement has to test `$fish_pid`, an unbound expansion that + // aborts the line under `set -u` — and because the clear came first, it took + // the agent launch down with it (the regression that reverted #14863). + const clearEnvNames = + args.session.agent === 'codex' && args.session.codexHome === null + ? (['CODEX_HOME', 'ORCA_CODEX_HOME'] as const) + : undefined + return buildAiVaultResumeForWorktree(args, true, clearEnvNames).command } export function buildAiVaultResumeStartupForWorktree( @@ -121,7 +120,10 @@ export function buildAiVaultDropRepinStartup(args: { function buildAiVaultResumeForWorktree( args: AiVaultResumeWorktreeArgs, - embedCwd: boolean + embedCwd: boolean, + /** Copy-path only: names the pasted line must strip off the agent itself. + * Spawned startups drop them through `envToDelete` instead. */ + clearEnvNames?: readonly string[] ): AiVaultResumeStartup { const providerSession = getAiVaultAgentProviderSession(args.session) if ( @@ -189,14 +191,16 @@ function buildAiVaultResumeForWorktree( platform, commandOverride: startupPlan.launchConfig.agentCommand, codexHome, - shell: liveShell + shell: liveShell, + clearEnvNames }) : buildAiVaultResumeShellCommand({ resumeCommand: startupPlan.launchCommand, cwd, platform, codexHome, - shell: liveShell + shell: liveShell, + clearEnvNames }), ...(startupPlan.env ? { env: startupPlan.env } : {}), ...realHomeCodexResumeEnvDeletion(args.session), @@ -221,7 +225,8 @@ function buildAiVaultResumeForWorktree( codexHome, // Why: non-resumable agents queue through this fallback too, so it must // quote for the live Windows shell like the startup-plan branch above. - shell: liveShell + shell: liveShell, + clearEnvNames }), ...startupCwd, ...realHomeCodexResumeEnvDeletion(args.session) @@ -241,32 +246,10 @@ function resolveAiVaultResumeShell(args: AiVaultResumeWorktreeArgs): AgentStartu state: args.state, worktreeId: args.worktreeId, platform, - isLocalSession, - parsedByClientLoginShell: isLocalSession && runsOnClientLoginShell(args, platform) + isLocalSession }) } -/** - * Whether the resume line is handed to THIS machine's login shell. - * - * Why not `isLocalSession`: that only says the session file was scanned locally - * (no executionHostId), which is still true when the worktree lives on an SSH - * or runtime host — the command then goes to that host's shell. The WSL case is - * caught by the platform mismatch (a WSL worktree resolves to 'linux' on win32). - */ -function runsOnClientLoginShell( - args: AiVaultResumeWorktreeArgs, - platform: NodeJS.Platform -): boolean { - const executionHost = parseExecutionHostId( - getExecutionHostIdForWorktree(args.state, args.worktreeId ?? args.state.activeWorktreeId) - ) - if (executionHost?.kind === 'ssh' || executionHost?.kind === 'runtime') { - return false - } - return platform === CLIENT_PLATFORM -} - export function getAiVaultAgentProviderSession( session: Pick & { filePath?: string } ): AgentProviderSessionMetadata | null { diff --git a/src/renderer/src/lib/ai-vault-resume-shell.test.ts b/src/renderer/src/lib/ai-vault-resume-shell.test.ts index e2ab4b9a173..2fff99a44dc 100644 --- a/src/renderer/src/lib/ai-vault-resume-shell.test.ts +++ b/src/renderer/src/lib/ai-vault-resume-shell.test.ts @@ -51,45 +51,33 @@ function withLoginShell(shell: string, run: () => T): T { } describe('resolveAiVaultResumeStartupShell', () => { - it('reports the fish dialect for a local session under a fish login shell', () => { - expect( - withLoginShell('/opt/homebrew/bin/fish', () => - resolveAiVaultResumeStartupShell({ - state: makeState(), - worktreeId: 'repo-1::worktree-1', - platform: 'darwin', - isLocalSession: true, - parsedByClientLoginShell: true - }) - ) - ).toBe('fish') - }) + // Why no login-shell cases: the Unix branch emits quoting and env clearing that + // are correct in sh and fish alike, so it no longer probes $SHELL at all — see + // startup-shell-portability.live-shell.test.ts for the proof it holds. + it.each(['/opt/homebrew/bin/fish', '/bin/zsh', '/bin/bash'])( + 'reports one Unix dialect regardless of the login shell (%s)', + (loginShell) => { + expect( + withLoginShell(loginShell, () => + resolveAiVaultResumeStartupShell({ + state: makeState(), + worktreeId: 'repo-1::worktree-1', + platform: 'darwin', + isLocalSession: true + }) + ) + ).toBe('posix') + } + ) - it('stays on sh for zsh users', () => { - expect( - withLoginShell('/bin/zsh', () => - resolveAiVaultResumeStartupShell({ - state: makeState(), - worktreeId: 'repo-1::worktree-1', - platform: 'darwin', - isLocalSession: true, - parsedByClientLoginShell: true - }) - ) - ).toBe('posix') - }) - - it('stays on sh for a LOCAL session whose command a remote host parses', () => { - // The reachable case: a locally scanned session has no executionHostId, so - // isLocalSession stays true while the command is bound for an SSH host. + it('stays on the Unix dialect for a LOCAL session whose command a remote host parses', () => { expect( withLoginShell('/opt/homebrew/bin/fish', () => resolveAiVaultResumeStartupShell({ state: makeState(), worktreeId: 'repo-1::worktree-1', platform: 'linux', - isLocalSession: true, - parsedByClientLoginShell: false + isLocalSession: true }) ) ).toBe('posix') @@ -104,23 +92,7 @@ describe('copied real-home Codex resume command', () => { codexHome: null } - it('clears inherited Codex homes with fish syntax under a fish login shell', () => { - expect( - withLoginShell('/opt/homebrew/bin/fish', () => - buildAiVaultResumeCopyCommandForWorktree({ - state: makeState(), - worktreeId: 'repo-1::worktree-1', - session - }) - ) - ).toBe( - "set -e CODEX_HOME; set -e ORCA_CODEX_HOME; cd '/home/alice/repo' && codex 'resume' 'session one'" - ) - }) - - it('keeps `unset` when a fish client targets an SSH worktree', () => { - // The session was scanned locally (no executionHostId), but the worktree lives - // on an SSH host: `set -e CODEX_HOME` would enable errexit there, not clear it. + it('clears inherited Codex homes for a worktree on an SSH host', () => { expect( withLoginShell('/opt/homebrew/bin/fish', () => buildAiVaultResumeCopyCommandForWorktree({ @@ -130,11 +102,13 @@ describe('copied real-home Codex resume command', () => { }) ) ).toBe( - "unset CODEX_HOME; unset ORCA_CODEX_HOME; cd '/home/alice/repo' && codex 'resume' 'session one'" + `cd '/home/alice/repo' && env -u CODEX_HOME -u ORCA_CODEX_HOME codex 'resume' 'session one'` ) }) - it('keeps `unset` for sh-family login shells', () => { + it('emits the same self-contained teardown under an sh-family login shell', () => { + // Why identical to the fish case: this text is COPIED, so it may be pasted + // into any shell — it carries its own fish/sh branch instead of guessing. expect( withLoginShell('/bin/bash', () => buildAiVaultResumeCopyCommandForWorktree({ @@ -144,7 +118,7 @@ describe('copied real-home Codex resume command', () => { }) ) ).toBe( - "unset CODEX_HOME; unset ORCA_CODEX_HOME; cd '/home/alice/repo' && codex 'resume' 'session one'" + `cd '/home/alice/repo' && env -u CODEX_HOME -u ORCA_CODEX_HOME codex 'resume' 'session one'` ) }) }) diff --git a/src/renderer/src/lib/ai-vault-resume-shell.ts b/src/renderer/src/lib/ai-vault-resume-shell.ts index 9b6214fb247..3d36b28c07f 100644 --- a/src/renderer/src/lib/ai-vault-resume-shell.ts +++ b/src/renderer/src/lib/ai-vault-resume-shell.ts @@ -4,11 +4,9 @@ import { CLIENT_PLATFORM } from '@/lib/new-workspace' import { resolveLocalWindowsTerminalShellOverrideForTab } from '../../../shared/local-windows-terminal-runtime' import { resolveWindowsShellStartupFamily } from '../../../shared/windows-terminal-shell' import { - resolveLoginShellStartupDialect, resolveStartupShell, type AgentStartupShell } from '../../../shared/tui-agent-startup-shell' -import { getClientLoginShell } from '@/lib/client-login-shell' import { parseWorkspaceKey } from '../../../shared/workspace-scope' import { parseWslUncPath } from '../../../shared/wsl-paths' @@ -28,19 +26,12 @@ export function resolveAiVaultResumeStartupShell(args: { worktreeId?: string | null platform: NodeJS.Platform isLocalSession: boolean - /** - * True only when this machine's own login shell parses the command. A local - * session is NOT enough: a locally scanned session carries no executionHostId - * yet can target an SSH/runtime/WSL worktree, whose shell is unrelated. - */ - parsedByClientLoginShell?: boolean }): AgentStartupShell { - // Why: fish rejects `unset`, so the client's login shell decides the dialect — - // but only when it is the shell that reads the line; otherwise it stays sh. + // Why no login-shell probe: everything this command is built from — quoting + // and env clearing — is emitted in a form that is correct in sh and fish + // alike, so the Unix branch never has to know which one reads the line. if (args.platform !== 'win32') { - return args.parsedByClientLoginShell - ? resolveLoginShellStartupDialect(getClientLoginShell()) - : 'posix' + return 'posix' } const projectRuntime = args.isLocalSession ? getLocalProjectExecutionRuntimeContext(args.state, args.worktreeId, CLIENT_PLATFORM) diff --git a/src/renderer/src/lib/launch-agent-background-session.test.ts b/src/renderer/src/lib/launch-agent-background-session.test.ts index 78fd3a91f6e..5dc076afc1b 100644 --- a/src/renderer/src/lib/launch-agent-background-session.test.ts +++ b/src/renderer/src/lib/launch-agent-background-session.test.ts @@ -332,7 +332,7 @@ describe('launchAgentBackgroundSession', () => { expect(mockSpawn).toHaveBeenCalledWith( expect.objectContaining({ cwd: 'C:\\Users\\jinwo\\repo\\feature', - command: "claude '--dangerously-skip-permissions' 'don'\\''t use powershell quoting'", + command: `claude '--dangerously-skip-permissions' 'don'"'"'t use powershell quoting'`, connectionId: null, worktreeId: 'wt-1', tabId: expect.stringMatching(UUID_RE) diff --git a/src/renderer/src/lib/launch-agent-in-new-tab-windows-quoting.test.ts b/src/renderer/src/lib/launch-agent-in-new-tab-windows-quoting.test.ts index c4034ba5bee..4d91f86a632 100644 --- a/src/renderer/src/lib/launch-agent-in-new-tab-windows-quoting.test.ts +++ b/src/renderer/src/lib/launch-agent-in-new-tab-windows-quoting.test.ts @@ -238,7 +238,7 @@ describe('launchAgentInNewTab Windows shell quoting', () => { expect(mockQueueTabStartupCommand).toHaveBeenCalledWith( 'tab-1', expect.objectContaining({ - command: "claude '--dangerously-skip-permissions' --prefill 'review Bob'\\''s change'" + command: `claude '--dangerously-skip-permissions' --prefill 'review Bob'"'"'s change'` }) ) }) @@ -293,7 +293,7 @@ describe('launchAgentInNewTab Windows shell quoting', () => { expect(mockQueueTabStartupCommand).toHaveBeenCalledWith( 'tab-1', expect.objectContaining({ - command: "claude '--dangerously-skip-permissions' --prefill 'review Bob'\\''s change'" + command: `claude '--dangerously-skip-permissions' --prefill 'review Bob'"'"'s change'` }) ) }) @@ -326,7 +326,7 @@ describe('launchAgentInNewTab Windows shell quoting', () => { launchAgentInNewTab({ agent: 'codex', worktreeId: 'wt-1' }) const queued = mockQueueTabStartupCommand.mock.calls.at(-1)?.[1] as { command: string } - expect(queued.command).toContain("'don'\\''t'") + expect(queued.command).toContain(`'don'"'"'t'`) expect(queued.command).not.toContain("'don''t'") }) }) diff --git a/src/renderer/src/lib/launch-work-item-direct.test.ts b/src/renderer/src/lib/launch-work-item-direct.test.ts index 52243fcc679..e70526aa3a7 100644 --- a/src/renderer/src/lib/launch-work-item-direct.test.ts +++ b/src/renderer/src/lib/launch-work-item-direct.test.ts @@ -688,7 +688,9 @@ describe('launchWorkItemDirect', () => { expect(mocks.activateAndRevealWorktree).toHaveBeenCalled() const activationOptions = mocks.activateAndRevealWorktree.mock.calls.at(-1)?.[1] - expect(activationOptions.startup.command).toContain('unset ORCA_PI_PREFILL') + expect(activationOptions.startup.command).toContain( + `command test -n "$fish_pid" && set --erase -g ORCA_PI_PREFILL; command test -z "$fish_pid" && unset ORCA_PI_PREFILL; true` + ) expect(activationOptions.startup.command).not.toContain('Remove-Item Env:ORCA_PI_PREFILL') }) @@ -729,7 +731,9 @@ describe('launchWorkItemDirect', () => { expect(mocks.ensureRemoteDetectedAgents).toHaveBeenCalledWith('ssh-1') expect(mocks.ensureDetectedAgents).not.toHaveBeenCalled() const activationOptions = mocks.activateAndRevealWorktree.mock.calls.at(-1)?.[1] - expect(activationOptions.startup.command).toContain('unset ORCA_PI_PREFILL') + expect(activationOptions.startup.command).toContain( + `command test -n "$fish_pid" && set --erase -g ORCA_PI_PREFILL; command test -z "$fish_pid" && unset ORCA_PI_PREFILL; true` + ) }) it('plans direct local Windows-path launches with POSIX startup for WSL project runtime', async () => { diff --git a/src/renderer/src/lib/resume-sleeping-agent-session.test.ts b/src/renderer/src/lib/resume-sleeping-agent-session.test.ts index c4f219f1524..c90aff730c4 100644 --- a/src/renderer/src/lib/resume-sleeping-agent-session.test.ts +++ b/src/renderer/src/lib/resume-sleeping-agent-session.test.ts @@ -885,7 +885,7 @@ describe('resumeSleepingAgentSessionsForWorktree', () => { const resumedTab = state.tabsByWorktree['wt-1']?.[0] expect(resumedTab?.launchAgent).toBe('claude') expect(state.pendingStartupByTabId[resumedTab!.id]?.command).toContain( - "'--resume' 'sess-1'\\''s'" + `'--resume' 'sess-1'"'"'s'` ) }) }) diff --git a/src/renderer/src/lib/tui-agent-startup.test.ts b/src/renderer/src/lib/tui-agent-startup.test.ts index 74dba64f02c..768a5363c85 100644 --- a/src/renderer/src/lib/tui-agent-startup.test.ts +++ b/src/renderer/src/lib/tui-agent-startup.test.ts @@ -352,7 +352,7 @@ describe('buildAgentDraftLaunchPlan', () => { }) ).toEqual({ agent: 'pi', - launchCommand: 'pi; unset ORCA_PI_PREFILL', + launchCommand: `pi; command test -n "$fish_pid" && set --erase -g ORCA_PI_PREFILL; command test -z "$fish_pid" && unset ORCA_PI_PREFILL; true`, expectedProcess: 'pi', env: { ORCA_PI_PREFILL: 'https://github.com/acme/repo/issues/42' }, launchConfig: emptyLaunchConfig('pi') diff --git a/src/shared/agent-resume-launch-command.ts b/src/shared/agent-resume-launch-command.ts index 1093735d266..cda40878c28 100644 --- a/src/shared/agent-resume-launch-command.ts +++ b/src/shared/agent-resume-launch-command.ts @@ -1,5 +1,6 @@ import type { ResumableTuiAgent } from './agent-session-resume' import { + isPosixStartupShell, quoteStartupArg, tokenizeStartupCommand, type AgentStartupShell @@ -37,9 +38,9 @@ function findClaudeExecutableIndex(tokens: readonly string[], shell: AgentStartu return i } if ( - // Why: `NAME=value cmd` is posix-only syntax; on cmd/PowerShell such a - // token is just a bogus executable name, not a prefix to skip. - (shell === 'posix' && /^[A-Za-z_][A-Za-z0-9_]*=/.test(token)) || + // Why: `NAME=value cmd` is sh-family syntax (fish included, 3.1+); on + // cmd/PowerShell such a token is a bogus executable name, not a prefix. + (isPosixStartupShell(shell) && /^[A-Za-z_][A-Za-z0-9_]*=/.test(token)) || (shell === 'powershell' && token === '&' && i === 0) ) { continue diff --git a/src/shared/ai-vault-resume-command.test.ts b/src/shared/ai-vault-resume-command.test.ts index a111b9eed35..2ebb5d59794 100644 --- a/src/shared/ai-vault-resume-command.test.ts +++ b/src/shared/ai-vault-resume-command.test.ts @@ -1,6 +1,9 @@ import { describe, expect, it } from 'vitest' -import { buildAiVaultResumeCommand } from './ai-vault-resume-command' +import { + buildAiVaultResumeCommand, + buildAiVaultResumeShellCommand +} from './ai-vault-resume-command' describe('buildAiVaultResumeCommand', () => { it('uses Antigravity conversation ids instead of Gemini resume flags', () => { @@ -150,3 +153,75 @@ describe('buildAiVaultResumeCommand', () => { ).toBe("cd '/Users/ada/repo' && prime-agent --resume 'dddddddd-eeee-4fff-8aaa-111111111111'") }) }) + +describe('buildAiVaultResumeShellCommand env removal', () => { + const base = { + resumeCommand: "codex 'resume' 'sid'", + cwd: '/repo', + clearEnvNames: ['CODEX_HOME', 'ORCA_CODEX_HOME'] + } + + it('carries the removal on the agent under a POSIX shell', () => { + expect(buildAiVaultResumeShellCommand({ ...base, platform: 'darwin' })).toBe( + "cd '/repo' && env -u CODEX_HOME -u ORCA_CODEX_HOME codex 'resume' 'sid'" + ) + }) + + // Why: `env -u` strips what the assignment just set, so an unfiltered list + // would silently resume against the real home instead of the pinned one. + it('keeps a pinned CODEX_HOME authoritative instead of stripping it', () => { + const command = buildAiVaultResumeShellCommand({ + ...base, + platform: 'darwin', + codexHome: '/home/a/.codex-work' + }) + + expect(command).toBe( + "cd '/repo' && CODEX_HOME='/home/a/.codex-work' env -u ORCA_CODEX_HOME codex 'resume' 'sid'" + ) + expect(command).not.toContain('-u CODEX_HOME') + }) + + it('keeps a pinned CODEX_HOME authoritative under a git-bash shell too', () => { + expect( + buildAiVaultResumeShellCommand({ + ...base, + platform: 'win32', + shell: 'posix', + codexHome: '/c/users/a/.codex-work' + }) + ).toBe( + "cd '/repo' && CODEX_HOME='/c/users/a/.codex-work' env -u ORCA_CODEX_HOME codex 'resume' 'sid'" + ) + }) + + // Why: the shell decides the grammar, not the host. Keying placement on the + // platform emitted POSIX `env -u` into a PowerShell line. + it('uses PowerShell grammar for a PowerShell shell on a non-Windows host', () => { + const command = buildAiVaultResumeShellCommand({ + ...base, + platform: 'linux', + shell: 'powershell' + }) + + expect(command).not.toContain('env -u') + expect(command).toBe( + 'Remove-Item Env:CODEX_HOME -ErrorAction SilentlyContinue; ' + + 'Remove-Item Env:ORCA_CODEX_HOME -ErrorAction SilentlyContinue; ' + + "Set-Location -LiteralPath '/repo'; codex 'resume' 'sid'" + ) + }) + + it('keeps the cmd clear ahead of the cd so a failed cd cannot launch the agent', () => { + expect( + buildAiVaultResumeShellCommand({ + ...base, + cwd: 'C:\\repo', + platform: 'win32', + shell: 'cmd' + }) + ).toBe( + 'set "CODEX_HOME=" & set "ORCA_CODEX_HOME=" & cd /d "C:\\repo" && codex \'resume\' \'sid\'' + ) + }) +}) diff --git a/src/shared/ai-vault-resume-command.ts b/src/shared/ai-vault-resume-command.ts index 13c33b8145e..26c5b637e82 100644 --- a/src/shared/ai-vault-resume-command.ts +++ b/src/shared/ai-vault-resume-command.ts @@ -3,10 +3,12 @@ // and (when known) the live tab's shell. import { TUI_AGENT_CONFIG } from './tui-agent-config' import { + clearEnvCommand, commandSeparator, isPosixStartupShell, quoteStartupArg, - type AgentStartupShell + type AgentStartupShell, + withoutEnvCommand } from './tui-agent-startup-shell' import type { AiVaultAgent, AiVaultSession } from './ai-vault-types' @@ -19,6 +21,7 @@ export function buildAiVaultResumeCommand(args: { codexHome?: string | null resumeFilePath?: string | null shell?: AgentStartupShell + clearEnvNames?: readonly string[] }): string { const { agent, sessionId, cwd, platform, commandOverride, codexHome, resumeFilePath, shell } = args @@ -45,7 +48,8 @@ export function buildAiVaultResumeCommand(args: { cwd, platform, codexHome, - shell + shell, + clearEnvNames: args.clearEnvNames }) } @@ -54,42 +58,69 @@ export function buildAiVaultResumeShellCommand(args: { cwd: string | null platform: NodeJS.Platform codexHome?: string | null + /** Env names the agent must not inherit. Applied as a prefix on the agent + * itself, never on the whole `cd … && agent` chain — `cd` is a shell builtin + * that `env` cannot run, and a child `cd` would not move the agent anyway. */ + clearEnvNames?: readonly string[] // Why: the QUEUED resume command is typed into the live tab shell, so its // cd/env prefix must match that shell. Shell-less persisted commands keep the // legacy self-contained `cmd /d /s /c` wrapper. shell?: AgentStartupShell }): string { - const { cwd, platform, codexHome, shell } = args + const { cwd, platform, codexHome, shell, clearEnvNames } = args // Why: shell-aware commands are parsed by a known running shell, while // shell-less persisted commands keep the legacy self-contained cmd wrapper. - if (platform === 'win32' && shell && shell !== 'cmd') { + // PowerShell routes here whatever the platform: it is the shell, not the + // host, that decides which grammar the line has to be written in. + if (shell === 'powershell' || (platform === 'win32' && shell && shell !== 'cmd')) { return buildResumeShellCommandForShell({ resumeCommand: args.resumeCommand, cwd, codexHome: codexHome?.trim() || null, - shell + shell, + clearEnvNames }) } - const resumeCommand = `${codexHomeEnvPrefix(codexHome?.trim() || null, platform)}${ - args.resumeCommand + const resolvedCodexHome = codexHome?.trim() || null + // Why filter: the prefix and the removal name the same variable, and `env -u` + // strips what the assignment just set, so an unfiltered list would silently + // resume against the real home. Keeping the assignment authoritative matches + // the old `clear…; CODEX_HOME=x agent` ordering. + const clearNames = resolvedCodexHome + ? clearEnvNames?.filter((name) => name !== 'CODEX_HOME') + : clearEnvNames + // Why the two placements differ: `set -u` aborts on the unbound `$fish_pid` + // the POSIX clear statement has to test, so there it must not precede the + // agent — `env -u` carries the removal on the agent itself. cmd has no such + // hazard, and keeping its clear ahead of the `cd` preserves `cd … && agent`, + // so a failed `cd` still cannot run the agent in the wrong directory. + // Keyed on the shell, not the platform: the shell is what picks the grammar. + const dialect = shell ?? (platform === 'win32' ? 'cmd' : 'posix') + const clearsOnAgent = clearNames?.length && isPosixStartupShell(dialect) + const resumeCommand = `${codexHomeEnvPrefix(resolvedCodexHome, platform, shell)}${ + clearsOnAgent ? withoutEnvCommand(clearNames, args.resumeCommand, dialect) : args.resumeCommand }` + const clearPrefix = + clearNames?.length && !clearsOnAgent + ? `${clearEnvCommand(clearNames, dialect)}${commandSeparator(dialect)}` + : '' if (platform === 'win32' && shell === 'cmd') { // Why: an interactive cmd splits the doubled quotes required by a nested // `cmd /s /c` wrapper, so queued commands must use direct cmd syntax. - return cwd ? `cd /d ${quoteWindowsCmdArg(cwd)} && ${resumeCommand}` : resumeCommand + return `${clearPrefix}${cwd ? `cd /d ${quoteWindowsCmdArg(cwd)} && ${resumeCommand}` : resumeCommand}` } if (!cwd) { - return resumeCommand + return `${clearPrefix}${resumeCommand}` } if (platform === 'win32') { - const inner = `cd /d ${quoteWindowsCmdArg(cwd)} && ${resumeCommand}` + const inner = `${clearPrefix}cd /d ${quoteWindowsCmdArg(cwd)} && ${resumeCommand}` return `cmd /d /s /c ${quoteWindowsCmdArg(inner)}` } - return `cd ${quoteShellArg(cwd, platform)} && ${resumeCommand}` + return `cd ${quoteResumeArg(cwd, platform, shell)} && ${resumeCommand}` } function buildResumeShellCommandForShell(args: { @@ -97,18 +128,33 @@ function buildResumeShellCommandForShell(args: { cwd: string | null codexHome: string | null shell: Exclude + clearEnvNames?: readonly string[] }): string { - const { cwd, codexHome, shell } = args + const { cwd, codexHome, shell, clearEnvNames } = args if (isPosixStartupShell(shell)) { // Why: git-bash on a Windows host runs a POSIX shell, so reuse the same // inline-env + `cd ''` prefix as the non-Windows path. const envPrefix = codexHome ? `CODEX_HOME=${quoteStartupArg(codexHome, shell)} ` : '' - const command = `${envPrefix}${args.resumeCommand}` + // Why filter: see the twin in buildAiVaultResumeShellCommand — `env -u` + // would strip the home the prefix just set. + const clearNames = codexHome + ? clearEnvNames?.filter((name) => name !== 'CODEX_HOME') + : clearEnvNames + const command = `${envPrefix}${ + clearNames?.length + ? withoutEnvCommand(clearNames, args.resumeCommand, shell) + : args.resumeCommand + }` return cwd ? `cd ${quoteStartupArg(cwd, shell)} && ${command}` : command } const separator = commandSeparator(shell) const segments: string[] = [] + // Why ahead of Set-Location: PowerShell has no `set -u` expansion hazard, so + // the removal keeps its original leading position. + if (clearEnvNames?.length) { + segments.push(clearEnvCommand(clearEnvNames, shell)) + } if (cwd) { segments.push(`Set-Location -LiteralPath ${quoteStartupArg(cwd, shell)}`) } @@ -184,21 +230,37 @@ function buildAgentResumeInvocation( } } -function codexHomeEnvPrefix(codexHome: string | null, platform: NodeJS.Platform): string { +function codexHomeEnvPrefix( + codexHome: string | null, + platform: NodeJS.Platform, + shell?: AgentStartupShell +): string { if (!codexHome) { return '' } if (platform === 'win32') { return `set ${quoteWindowsCmdArg(`CODEX_HOME=${codexHome}`)} && ` } - return `CODEX_HOME=${quoteShellArg(codexHome, platform)} ` + // fish accepts the `NAME=value cmd` prefix (3.1+), but not sh's quoting. + return `CODEX_HOME=${quoteResumeArg(codexHome, platform, shell)} ` } +/** Quotes for the live shell when one is known, else for the platform's default. */ +function quoteResumeArg( + value: string, + platform: NodeJS.Platform, + shell?: AgentStartupShell +): string { + return shell ? quoteStartupArg(value, shell) : quoteShellArg(value, platform) +} + +/** Why not the sh `'\''` idiom here: this is the same resume command the shell + * branch above builds, and fish reads that idiom differently — it would halve + * backslashes in a path and reject a trailing one. Deferring to + * `quoteStartupArg` keeps one spelling regardless of whether a caller happened + * to pass a shell. */ function quoteShellArg(value: string, platform: NodeJS.Platform): string { - if (platform === 'win32') { - return quoteWindowsCmdArg(value) - } - return `'${value.replace(/'/g, `'\\''`)}'` + return platform === 'win32' ? quoteWindowsCmdArg(value) : quoteStartupArg(value, 'posix') } function quoteWindowsCmdArg(value: string): string { diff --git a/src/shared/global-settings-types.ts b/src/shared/global-settings-types.ts index 1e41228f37e..6c331594e37 100644 --- a/src/shared/global-settings-types.ts +++ b/src/shared/global-settings-types.ts @@ -274,7 +274,8 @@ export type GlobalSettings = { claudeManagedAccounts: ClaudeManagedAccount[] activeClaudeManagedAccountId: string | null activeClaudeManagedAccountIdsByRuntime?: ClaudeManagedAccountRuntimeSelection - /** Per-worktree shell history file so ArrowUp doesn't surface other worktrees' commands. Defaults to true. */ + /** Per-worktree shell history so ArrowUp doesn't surface other worktrees' commands (a HISTFILE for + * bash/zsh, a `fish_history` session name for fish). Defaults to true. */ terminalScopeHistoryByWorktree: boolean /** Kill switch for hidden terminal view parking: unmount long-hidden panes while a pane-less watcher keeps PTY side effects alive. */ terminalHiddenViewParking?: boolean diff --git a/src/shared/hermes-startup-query.ts b/src/shared/hermes-startup-query.ts index a120305e1ee..a8536f91601 100644 --- a/src/shared/hermes-startup-query.ts +++ b/src/shared/hermes-startup-query.ts @@ -159,7 +159,11 @@ function buildQueryCommand(argv: string[], shell: AgentStartupShell): string { // Why: a fixed single-quote-safe wrapper parses in POSIX shells and pwsh; // the dynamic argv is decoded only after entering the known `sh` grammar. const script = `${POSIX_QUERY_VARIABLE}="\${${ORCA_HERMES_STARTUP_QUERY_ENV}}"; unset ${ORCA_HERMES_STARTUP_QUERY_ENV}; eval "$(printf %b "${encodedInvocation}")"` - return `sh -c ${quoteStartupArg(script, 'posix')}` + // Why `shell` and not 'posix': the body runs under `sh`, but the quoting around + // it is parsed by the shell that types the line. Today's payload survives sh + // quoting in fish only because its escapes happen to be `\0NNN` and never `\\` + // — quote for the real dialect so that stays an implementation detail. + return `sh -c ${quoteStartupArg(script, shell)}` } export function planHermesStartupQuery(args: { @@ -181,7 +185,10 @@ export function planHermesStartupQuery(args: { return null } const command = buildQueryCommand(argv, args.shell) - const env = { ...args.agentEnv, [ORCA_HERMES_STARTUP_QUERY_ENV]: args.prompt } + const env = { + ...args.agentEnv, + [ORCA_HERMES_STARTUP_QUERY_ENV]: args.prompt + } const envSize = Object.entries(env).reduce((total, [key, value]) => { if (args.platform === 'win32') { return total + key.length + value.length + 2 diff --git a/src/shared/startup-shell-portability.live-shell.test.ts b/src/shared/startup-shell-portability.live-shell.test.ts new file mode 100644 index 00000000000..b72a60ec075 --- /dev/null +++ b/src/shared/startup-shell-portability.live-shell.test.ts @@ -0,0 +1,266 @@ +/** + * Proves the central claim of the POSIX startup dialect against real shells: + * ONE emitted string is correct in sh, bash, zsh, dash and fish alike, so Orca + * never has to detect which shell will parse a queued command line. + * + * The shells are the oracle. Each case is handed to a real shell, which echoes + * the value back through `printf`, and the test compares bytes — a hand-written + * expectation would only re-state the implementation's own assumptions. + * + * Two of the quoting cases are regression pins with teeth: `\\server\share` and + * a trailing backslash are exactly what plain sh `'\''` quoting gets wrong when + * fish reads it (silently dropped backslashes, and a hard syntax error), and + * `command eval` is what zsh gets wrong — its `command` resolves external + * binaries only, so it cannot run a builtin. + */ +import { execFileSync } from 'node:child_process' +import { existsSync, mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import path from 'node:path' +import { afterAll, describe, expect, it } from 'vitest' +import { fishRequirementViolation, resolveFishBinary } from './fish-binary-requirement' +import { clearEnvCommand, quoteStartupArg, withoutEnvCommand } from './tui-agent-startup-shell' + +const FISH = resolveFishBinary(4) + +type LiveShell = { name: string; path: string } + +/** Every Unix shell on this machine a queued Orca command line could land in. */ +function discoverShells(): LiveShell[] { + const shells: LiveShell[] = [] + for (const path of [ + '/bin/sh', + '/bin/bash', + '/bin/zsh', + '/bin/dash', + '/usr/bin/dash', + '/bin/ksh' + ]) { + if (existsSync(path) && !shells.some((shell) => shell.name === basename(path))) { + shells.push({ name: basename(path), path }) + } + } + if (FISH.available) { + // Why absolute: runInShell scrubs PATH, so a bare `fish` would not resolve. + const path = FISH.path.includes('/') + ? FISH.path + : execFileSync('command', ['-v', FISH.path], { encoding: 'utf8', shell: true }).trim() + shells.push({ name: 'fish', path }) + } + return shells +} + +function basename(path: string): string { + return path.split('/').pop() ?? path +} + +// Why a real (empty) HOME rather than a bogus one: fish needs a writable config +// dir to hold universal variables, and warns loudly on every launch without it. +const SANDBOX_HOME = mkdtempSync(path.join(tmpdir(), 'orca-shell-portability-')) + +/** Env with no user shell config reachable, so only Orca's own text is exercised. */ +function sandboxEnv(): NodeJS.ProcessEnv { + return { + PATH: '/usr/bin:/bin:/usr/sbin:/sbin', + HOME: SANDBOX_HOME, + XDG_CONFIG_HOME: path.join(SANDBOX_HOME, 'config'), + XDG_DATA_HOME: path.join(SANDBOX_HOME, 'data') + } +} + +/** Runs one line in a real shell with no user config reachable. */ +function runInShell(shell: LiveShell, script: string): string { + return execFileSync(shell.path, ['-c', script], { + encoding: 'utf8', + timeout: 20_000, + env: sandboxEnv() + }) +} + +const SHELLS = discoverShells() + +const QUOTING_CASES: readonly string[] = [ + 'plain', + '', + 'with space', + "it's", + 'a\\b', + '\\d+', + '\\\\server\\share', + 'C:\\Users\\foo', + 'ends\\', + '\\', + '$HOME', + '`whoami`', + '$(whoami)', + '${HOME}', + '(paren)', + '{a,b}', + '*glob?', + '[bracket]', + '#hash', + 'a\nb', + 'a\tb', + 'semi;colon', + 'pipe|and&', + 'redirect>out { + rmSync(SANDBOX_HOME, { recursive: true, force: true }) +}) + +// Why skipIf: on Windows discoverShells() finds nothing, so the suite would +// fail for want of a POSIX shell rather than for any defect it tests. +describe.skipIf(process.platform === 'win32')( + 'one POSIX startup dialect is correct in every Unix shell', + () => { + // Always runs, so a CI lane cannot report green with the shell cases skipped. + it('has the fish this suite needs when CI requires one', () => { + expect(fishRequirementViolation(FISH)).toBeNull() + }) + + it('found shells to test against', () => { + expect(SHELLS.map((shell) => shell.name)).toContain('sh') + }) + + describe.each(SHELLS)('$name', (shell) => { + it.each(QUOTING_CASES)('quotes %j so the shell yields it back verbatim', (value) => { + expect(runInShell(shell, `printf '%s' ${quoteStartupArg(value, 'posix')}`)).toBe(value) + }) + + it('quotes a whole argv so every argument survives independently', () => { + const argv = QUOTING_CASES.filter((value) => value !== '') + const quoted = argv.map((value) => quoteStartupArg(value, 'posix')).join(' ') + // Why NUL: it is the one byte no case above can contain, so it cannot be + // forged by a value that was mis-split into two arguments. + const output = runInShell(shell, `printf '%s\\0' ${quoted}`) + expect(output.split('\0').slice(0, -1)).toEqual(argv) + }) + + // Why `set -u` gets its own case: the copied resume command puts the clear + // BEFORE the agent, so an aborted line takes the launch down with it. That + // is the regression that reverted #14863 (#14975). + it('runs the agent under set -u, with the vars removed', () => { + // The child must read its own environment, the way the agent binary does; + // expanding it in the caller would just echo the caller's value back. + const launch = withoutEnvCommand( + ['CODEX_HOME', 'ORCA_CODEX_HOME'], + `sh -c 'printf "LAUNCHED:%s" "\${CODEX_HOME-unset}"'`, + 'posix' + ) + const probe = + shell.name === 'fish' + ? `set -gx CODEX_HOME /bad; ${launch}` + : `set -u; CODEX_HOME=/bad; export CODEX_HOME; ${launch}` + + expect(runInShell(shell, probe)).toContain('LAUNCHED:unset') + }) + + it('clears an exported variable with no wrapper installed', () => { + const clear = clearEnvCommand('ORCA_PI_PREFILL', 'posix') + const probe = + shell.name === 'fish' + ? `set -gx ORCA_PI_PREFILL draft; ${clear}; set -q ORCA_PI_PREFILL; and echo STILL; or echo CLEARED` + : `ORCA_PI_PREFILL=draft; export ORCA_PI_PREFILL; ${clear}; echo "\${ORCA_PI_PREFILL:+STILL}\${ORCA_PI_PREFILL:-CLEARED}"` + expect(runInShell(shell, probe).trim()).toBe('CLEARED') + }) + + it('clears several variables in one statement', () => { + const clear = clearEnvCommand(['ORCA_A', 'ORCA_B'], 'posix') + const probe = + shell.name === 'fish' + ? `set -gx ORCA_A 1; set -gx ORCA_B 2; ${clear}; set -q ORCA_A; or set -q ORCA_B; and echo STILL; or echo CLEARED` + : `ORCA_A=1 ORCA_B=2; export ORCA_A ORCA_B; ${clear}; echo "\${ORCA_A:+STILL}\${ORCA_B:+STILL}\${ORCA_A:-CLEARED}"` + expect(runInShell(shell, probe).trim()).toBe('CLEARED') + }) + + // Why this case: fish's `set -e` returns non-zero for a variable that is + // already unset. An `A && B || C` spelling would fall through to the sh + // branch and print `Unknown command: unset` — the exact bug being fixed. + it.each([ + ['already set', true], + ['already unset', false] + ])('exits 0 and writes nothing to stderr when the variable is %s', (_label, preset) => { + const clear = clearEnvCommand('ORCA_PI_PREFILL', 'posix') + const setUp = preset + ? shell.name === 'fish' + ? 'set -gx ORCA_PI_PREFILL draft; ' + : 'ORCA_PI_PREFILL=draft; export ORCA_PI_PREFILL; ' + : '' + const stderr = execFileSync(shell.path, ['-c', `${setUp}${clear} 2>&1 1>/dev/null`], { + encoding: 'utf8', + timeout: 20_000, + env: sandboxEnv() + }) + expect(stderr).toBe('') + }) + + // Why INTERACTIVE (-i, script on stdin): aliases are only expanded by an + // interactive shell, which is the mode Orca types into — a `-c` run cannot + // see this class of bug at all. A user with `alias test=…` would otherwise + // silently skip both branches and keep the prefill exported. + it.runIf(['bash', 'zsh'].includes(shell.name))( + 'clears even when `test` is aliased away in an interactive shell', + () => { + const clear = clearEnvCommand('ORCA_PI_PREFILL', 'posix') + const script = [ + 'alias test=false', + 'ORCA_PI_PREFILL=draft; export ORCA_PI_PREFILL', + clear, + 'echo "RESULT=${ORCA_PI_PREFILL:+STILL}${ORCA_PI_PREFILL:-CLEARED}"' + ].join('\n') + const out = execFileSync(shell.path, ['-i'], { + input: `${script}\n`, + encoding: 'utf8', + timeout: 20_000, + env: sandboxEnv(), + stdio: ['pipe', 'pipe', 'ignore'] + }) + expect(out).toContain('RESULT=CLEARED') + } + ) + + // Why: `$fish_pid` is a heuristic — any non-empty value takes the fish + // branch. `set -e NAME` there would enable errexit, silently changing the + // semantics of every command after it for the rest of the session. + // `set --erase` cannot: `--` ends option parsing, so the worst a misfire + // costs is a usage message and the positional parameters (which an + // interactive shell does not meaningfully use). + // Scoped to bash/zsh because `set` is a POSIX special builtin: in sh, dash + // and ksh a misfire aborts a NON-interactive shell before anything can be + // observed. Interactively — the mode Orca types into — they survive without + // errexit too, but that is not scriptable here. bash and zsh report `$-` + // either way, and both DID silently enable errexit under the old `set -e`. + // Why `-g` is not optional: without it, a name that exists ONLY as a + // universal — `set -Ux CODEX_HOME …`, a normal thing for a fish user to + // have — is permanently deleted from every future session. Reachable from + // the clipboard command, which may run with no injected value at all. + it.runIf(shell.name === 'fish')('never deletes a lone universal variable', () => { + const clear = clearEnvCommand('ORCA_PI_PREFILL', 'posix') + const probe = `set -Ux ORCA_PI_PREFILL persisted; ${clear}; set -q ORCA_PI_PREFILL; and echo "RESULT=$ORCA_PI_PREFILL"; or echo RESULT=DESTROYED; set -Ue ORCA_PI_PREFILL` + expect(runInShell(shell, probe)).toContain('RESULT=persisted') + }) + + // A universal shadowed by the injected global: the global goes, theirs + // comes back. That is the wanted outcome, not a missed erase. + it.runIf(shell.name === 'fish')('reveals a shadowed universal instead of deleting it', () => { + const clear = clearEnvCommand('ORCA_PI_PREFILL', 'posix') + const probe = `set -Ux ORCA_PI_PREFILL mine; set -gx ORCA_PI_PREFILL injected; ${clear}; echo "RESULT=$ORCA_PI_PREFILL"; set -Ue ORCA_PI_PREFILL` + expect(runInShell(shell, probe)).toContain('RESULT=mine') + }) + + it.runIf(['bash', 'zsh'].includes(shell.name))( + 'never enables errexit when $fish_pid misfires', + () => { + const clear = clearEnvCommand('ORCA_PI_PREFILL', 'posix') + const probe = `fish_pid=1; export fish_pid; ${clear} 2>/dev/null; printf '%s' "$-"` + expect(runInShell(shell, probe).trim()).not.toContain('e') + } + ) + }) + } +) diff --git a/src/shared/tui-agent-startup-session-options.test.ts b/src/shared/tui-agent-startup-session-options.test.ts index cd47d81b971..45d83900b86 100644 --- a/src/shared/tui-agent-startup-session-options.test.ts +++ b/src/shared/tui-agent-startup-session-options.test.ts @@ -125,7 +125,7 @@ describe('tui agent startup session options', () => { allowEmptyPromptLaunch: true, sessionOptions: { model: "team's-model", effort: 'high' } }) - expect(plan?.launchCommand).toContain("'team'\\''s-model'") + expect(plan?.launchCommand).toContain(`'team'"'"'s-model'`) }) it('threads options through native draft launches', () => { diff --git a/src/shared/tui-agent-startup-shell.test.ts b/src/shared/tui-agent-startup-shell.test.ts index 2d155063462..1698e86b08b 100644 --- a/src/shared/tui-agent-startup-shell.test.ts +++ b/src/shared/tui-agent-startup-shell.test.ts @@ -5,7 +5,6 @@ import { commandSeparator, isPosixStartupShell, quoteStartupArg, - resolveLoginShellStartupDialect, tokenizeStartupCommand } from './tui-agent-startup-shell' import { buildAgentDraftLaunchPlan } from './tui-agent-startup' @@ -66,49 +65,86 @@ describe('tokenizeStartupCommand spans (windows shells)', () => { }) }) -describe('fish startup shell dialect', () => { - it('clears variables with fish syntax instead of the sh `unset` fish rejects', () => { - expect(clearEnvCommand('CODEX_HOME', 'fish')).toBe('set -e CODEX_HOME') - expect(clearEnvCommand('CODEX_HOME', 'posix')).toBe('unset CODEX_HOME') +describe('one Unix startup dialect', () => { + it('clears variables with a self-contained branch, not a per-shell builtin', () => { + // Why not `unset`/`set -e` alone, and why not a wrapper-defined helper: + // Orca only wraps zsh/bash/fish, so an `sh`/`dash`/`ksh` login shell — and + // any shell the user pastes copied text into — would not have the helper. + // startup-shell-portability.live-shell.test.ts proves this form works in + // real sh/bash/zsh/dash/ksh/fish. + expect(clearEnvCommand('CODEX_HOME', 'posix')).toBe( + `command test -n "$fish_pid" && set --erase -g CODEX_HOME; command test -z "$fish_pid" && unset CODEX_HOME; true` + ) + expect(clearEnvCommand(['A', 'B'], 'posix')).toBe( + `command test -n "$fish_pid" && set --erase -g A B; command test -z "$fish_pid" && unset A B; true` + ) expect(clearEnvCommand('CODEX_HOME', 'cmd')).toBe('set "CODEX_HOME="') expect(clearEnvCommand('CODEX_HOME', 'powershell')).toBe( 'Remove-Item Env:CODEX_HOME -ErrorAction SilentlyContinue' ) }) - it('shares POSIX quoting, tokenizing and chaining', () => { - expect(quoteStartupArg("it's", 'fish')).toBe(quoteStartupArg("it's", 'posix')) - expect(buildShellCommandFromArgv(['codex', 'resume', 'a b'], 'fish')).toBe( - buildShellCommandFromArgv(['codex', 'resume', 'a b'], 'posix') - ) - expect(tokenizeStartupCommand('codex --arg "a b"', 'fish')).toEqual( - tokenizeStartupCommand('codex --arg "a b"', 'posix') - ) - expect(commandSeparator('fish')).toBe('; ') - expect(isPosixStartupShell('fish')).toBe(true) + it.each([ + 'FOO; rm -rf /tmp/x', + 'FOO BAR', + '', + '1LEADING_DIGIT', + 'has-dash', + '$FOO', + 'PATH\nHOME' + ])('refuses %j as an environment variable name', (bad) => { + // Why: the name is interpolated straight into a shell line. Anything but an + // identifier is a command injection, and in fish `set --erase -g` would + // really delete whatever it names — PATH or HOME included. + expect(() => clearEnvCommand(bad, 'posix')).toThrow(/not an environment variable name/) + expect(() => clearEnvCommand(['OK', bad], 'posix')).toThrow(/not an environment variable name/) }) - it('maps login shell paths to their dialect', () => { - expect(resolveLoginShellStartupDialect('/opt/homebrew/bin/fish')).toBe('fish') - expect(resolveLoginShellStartupDialect('/usr/local/bin/FISH')).toBe('fish') - expect(resolveLoginShellStartupDialect('/bin/zsh')).toBe('posix') - expect(resolveLoginShellStartupDialect('/bin/bash')).toBe('posix') - expect(resolveLoginShellStartupDialect('')).toBe('posix') - expect(resolveLoginShellStartupDialect(undefined)).toBe('posix') + it('quotes so fish reads back the same bytes sh does', () => { + // fish single quotes are NOT literal — `\\` and `\'` are escapes inside them — + // so the sh `'\''` idiom would halve these backslashes when fish read them, + // and a trailing backslash would be a hard syntax error. + expect(quoteStartupArg("it's", 'posix')).toBe(`'it'"'"'s'`) + expect(quoteStartupArg(String.raw`\\server\share`, 'posix')).toBe( + `"\\\\""\\\\"'server'"\\\\"'share'` + ) + expect(quoteStartupArg('ends\\', 'posix')).toBe(`'ends'"\\\\"`) + expect(quoteStartupArg('', 'posix')).toBe(`''`) }) - // Contract-only: no POSIX caller threads a shell into buildAgentDraftLaunchPlan yet - // (`selectedRepoStartupShell` is Windows-only), so fish users still get `unset` here. - it('clears an agent draft prefill variable with fish syntax', () => { + it('keeps the sh-family grammar claims that do hold for fish', () => { + expect(commandSeparator('posix')).toBe('; ') + expect(isPosixStartupShell('posix')).toBe(true) + expect(isPosixStartupShell('powershell')).toBe(false) + expect(buildShellCommandFromArgv(['codex', 'resume', 'a b'], 'posix')).toBe( + `'codex' 'resume' 'a b'` + ) + }) + + it('round-trips an adversarial argument through quote then tokenize', () => { + for (const value of [ + String.raw`use \d+ and \\server\share`, + "it's mine", + 'ends\\', + 'a "b" c', + '$PATH *.ts' + ]) { + const tokenized = tokenizeStartupCommand(quoteStartupArg(value, 'posix'), 'posix') + expect(tokenized.ok && tokenized.tokens).toEqual([value]) + } + }) + + it('clears an agent draft prefill variable with the portable teardown', () => { const plan = buildAgentDraftLaunchPlan({ agent: 'pi', draft: 'hello', cmdOverrides: {}, - platform: 'darwin', - shell: 'fish' + platform: 'darwin' }) - expect(plan?.launchCommand).toBe('pi; set -e ORCA_PI_PREFILL') + expect(plan?.launchCommand).toBe( + `pi; command test -n "$fish_pid" && set --erase -g ORCA_PI_PREFILL; command test -z "$fish_pid" && unset ORCA_PI_PREFILL; true` + ) expect(plan?.env?.ORCA_PI_PREFILL).toBe('hello') }) }) diff --git a/src/shared/tui-agent-startup-shell.ts b/src/shared/tui-agent-startup-shell.ts index ef23b837e0b..94df0f80ac5 100644 --- a/src/shared/tui-agent-startup-shell.ts +++ b/src/shared/tui-agent-startup-shell.ts @@ -1,28 +1,26 @@ import { tokenizeCustomCommandTemplate, type CommandTokenSpan } from './commit-message-prompt' -// Why: fish shares POSIX word splitting, quoting and `;` chaining, so it is a -// separate dialect only where its grammar actually diverges (env clearing). -export type AgentStartupShell = 'posix' | 'fish' | 'powershell' | 'cmd' +/** + * `'posix'` covers every Unix shell Orca can type into, fish included — not + * because they agree, but because everything this module emits for it is built + * to be correct in all of them (see quoteStartupArg and clearEnvCommand). There + * is deliberately no fish member: a dialect Orca has to detect is a dialect it + * can get wrong, and it cannot detect one reliably for a remote or WSL host. + */ +export type AgentStartupShell = 'posix' | 'powershell' | 'cmd' type WindowsStartupShell = Extract -/** True for shells parsed with POSIX quoting/word rules (sh family + fish). */ +/** True for the sh-family grammar: `NAME=value cmd` prefixes, `sh -c` wrapping + * and `;` chaining. fish parses all three, so this holds for fish too. */ export function isPosixStartupShell(shell: AgentStartupShell): boolean { - return shell === 'posix' || shell === 'fish' + return shell === 'posix' } function isWindowsStartupShell(shell: AgentStartupShell): shell is WindowsStartupShell { return shell === 'powershell' || shell === 'cmd' } -/** Maps a POSIX login-shell path (`$SHELL`) to the dialect that parses queued commands. */ -export function resolveLoginShellStartupDialect( - loginShell: string | null | undefined -): AgentStartupShell { - const basename = loginShell?.trim().replaceAll('\\', '/').split('/').pop()?.toLowerCase() ?? '' - return basename === 'fish' ? 'fish' : 'posix' -} - export type StartupCommandTokens = | { ok: true; tokens: string[]; spans: CommandTokenSpan[] } | { ok: false; error: string } @@ -154,6 +152,11 @@ export function tokenizeStartupCommand( value: string, shell: AgentStartupShell ): StartupCommandTokens { + // Why one Unix parse: the input is a string the user typed into an Orca + // settings field, and the shell never parses it — every token is re-quoted by + // quoteStartupArg before the line is built. Parsing it differently per shell + // would make the same setting mean different things in different workspaces. + // (Windows is genuinely different: cmd/PowerShell re-parse the built line.) return isWindowsStartupShell(shell) ? tokenizeWindowsStartupCommand(value, shell) : tokenizeCustomCommandTemplate(value) @@ -166,6 +169,52 @@ export function resolveStartupShell( return shell ?? (platform === 'win32' ? 'powershell' : 'posix') } +/** + * Quotes one argument so the SAME text is literal in every Unix shell Orca can + * be typing into — sh, bash, zsh, dash and fish. + * + * Why not plain sh quoting: fish's single quotes are not literal. Inside `'…'` + * it treats `\\` and `\'` as escapes, so the sh `'\''` idiom collapses every + * backslash (a `\d+` regex, a `\\server\share` UNC path) and a trailing + * backslash is a hard syntax error that kills the launch outright. + * + * Both shell families agree on two things: a single-quoted run is literal apart + * from those escapes, and `"\\"` is one backslash. So emitting backslashes as + * `"\\"` and apostrophes as `"'"` between single-quoted runs round-trips in all + * of them, and Orca never has to know which shell will read the line. + * + * Verified against sh, bash, zsh, dash and fish 4.7.1 over regex escapes, UNC + * and drive paths, trailing/lone backslashes, mixed quotes, `$`/backtick/`$()` + * expansions, globs, braces, operators, comments, newlines, tabs and non-BMP + * unicode — see fish-startup-arg-quoting.live-fish.test.ts. + */ +function quotePortableUnixArg(value: string): string { + if (!value) { + return "''" + } + const parts: string[] = [] + let literal = '' + const flushLiteral = (): void => { + if (literal) { + parts.push(`'${literal}'`) + literal = '' + } + } + for (const char of value) { + if (char === "'") { + flushLiteral() + parts.push(`"'"`) + } else if (char === '\\') { + flushLiteral() + parts.push(`"\\\\"`) + } else { + literal += char + } + } + flushLiteral() + return parts.join('') +} + export function quoteStartupArg(value: string, shell: AgentStartupShell): string { if (shell === 'powershell') { return `'${value.replace(/'/g, "''")}'` @@ -173,7 +222,7 @@ export function quoteStartupArg(value: string, shell: AgentStartupShell): string if (shell === 'cmd') { return `"${value.replace(/([\^&|<>()%!"])/g, '^$1')}"` } - return `'${value.replace(/'/g, `'\\''`)}'` + return quotePortableUnixArg(value) } export function buildShellCommandFromArgv( @@ -187,19 +236,131 @@ export function buildShellCommandFromArgv( return command } -export function clearEnvCommand(name: string, shell: AgentStartupShell): string { +/** + * Clears one or more environment variables, in a form correct in every shell a + * queued command line can land in. + * + * Why it carries its own fish/sh branch rather than a single builtin: `unset` + * does not exist in fish, and `set -e` in bash enables errexit rather than + * clearing anything, so neither spelling is safe alone. + * + * Why not a helper function defined by Orca's shell wrappers: Orca only wraps + * zsh, bash and fish. A login shell of `sh`, `dash` or `ksh` launches + * UNWRAPPED, and this text is also copied to the clipboard and pasted into + * shells Orca never spawned — in all of those a helper would be `command not + * found`, which is the exact failure this exists to avoid. + * + * Why two statements rather than `A && B || C`: in fish, `set -e` on a variable + * that is already unset returns non-zero, so an `||` fallback would run the sh + * branch too and print `Unknown command: unset`. Each branch is guarded by its + * own test, so exactly one runs and the other is only parsed. + * + * The trailing `true` pins the exit status at 0: the guard that does NOT fire + * leaves a non-zero status behind, and this is the last statement of a launch + * line, so that status is what the user's prompt would render. + * + * `command test` rather than `test`, because an interactive shell expands + * aliases and `alias test=...` would otherwise skip both branches silently. + * + * `set --erase` rather than `set -e`, because `$fish_pid` is a heuristic: any + * non-empty value takes the fish branch. `set -e NAME` in the sh family enables + * errexit and replaces the positional parameters, silently changing the + * semantics of everything after it. `set --erase NAME` is the same erase in + * fish, but in sh `--` ends option parsing, so a misfire is a usage message on + * stderr and nothing else. + * + * `$fish_pid` is set by fish 3.0+ and by nothing in the sh family, and fish + * does not export it, so a fish parent cannot make a bash child misread itself. + * + * `-g` is not optional: it scopes the erase to fish's GLOBAL scope, which is + * where an inherited environment variable lands. Without it, a name that exists + * only as a UNIVERSAL — `set -Ux CODEX_HOME …`, a perfectly normal thing for a + * fish user to have — is permanently deleted from every future session. That is + * real data loss to undo one launch's injection, and it is reachable from the + * clipboard command, which a user may run with no Orca-injected value at all. + * With `-g`, a universal shadowed by an injected global is revealed again + * instead, which is the wanted outcome. + * + * KNOWN LIMIT: under `set -u` the sh side aborts on the unset `$fish_pid` and + * the variable survives. `${fish_pid-}` would be nounset-safe but is a fish + * parse error, and so is `set +u`. + * + * The one spelling that does satisfy both is a fish-builtin probe — e.g. + * `math 1 >/dev/null 2>&1 && … || …` — because it references no variable at + * all. Rejected deliberately: it EXECUTES whatever that name resolves to on + * the user's PATH, twice. `math` is a real binary (Wolfram Mathematica ships + * `/usr/local/bin/math`); measured with one on PATH, clearing a single variable + * took 10.4s and then did not clear it, because a probe that exits 0 is an + * unconditional false positive. Trading a rare uncleared prefill for an + * arbitrary program execution on the launch path is the wrong direction. + * + * Verified to clear the variables, write nothing to stderr and exit 0 — whether + * they were set or already unset — in sh, bash, zsh, dash, ksh and fish. + */ +const ENV_VAR_NAME = /^[A-Za-z_][A-Za-z0-9_]*$/ + +export function clearEnvCommand( + name: string | readonly string[], + shell: AgentStartupShell +): string { + const names = typeof name === 'string' ? [name] : [...name] + // Why assert rather than escape: these names are interpolated straight into a + // shell line, so anything but an identifier is both a command injection and — + // in fish, where `-g` erases for real — a way to delete `PATH`/`HOME` out of + // the user's session. Every caller passes a literal from a fixed table, so + // this cannot fire today; it exists so it stays that way. + for (const each of names) { + if (!ENV_VAR_NAME.test(each)) { + throw new Error( + `clearEnvCommand: ${JSON.stringify(each)} is not an environment variable name` + ) + } + } if (shell === 'powershell') { - return `Remove-Item Env:${name} -ErrorAction SilentlyContinue` + return names.map((each) => `Remove-Item Env:${each} -ErrorAction SilentlyContinue`).join('; ') } if (shell === 'cmd') { - return `set "${name}="` + return names.map((each) => `set "${each}="`).join(' & ') } - // Why: fish has no `unset`; the sh spelling errors out and silently leaves - // the variable exported (e.g. an account-routed CODEX_HOME survives). - if (shell === 'fish') { - return `set -e ${name}` + const joined = names.join(' ') + return ( + `command test -n "$fish_pid" && set --erase -g ${joined}; ` + + `command test -z "$fish_pid" && unset ${joined}; true` + ) +} + +/** + * Prefix that runs `command` with `names` removed from its environment. + * + * Why this and not `clearEnvCommand` for a copied command: `clearEnvCommand` + * mutates the *calling* shell, so it needs a shell-specific branch, and every + * spelling of that branch touches `$fish_pid` — an unbound expansion that aborts + * the whole line under `set -u`, taking the agent launch with it. This prefix + * only has to change the CHILD's environment, which `env -u` does with no shell + * syntax and no variable expansion at all. Verified identical in sh, bash, zsh, + * dash, ksh and fish, including under `set -u`. + */ +export function withoutEnvCommand( + names: readonly string[], + command: string, + shell: AgentStartupShell +): string { + if (names.length === 0) { + return command } - return `unset ${name}` + if (isWindowsStartupShell(shell)) { + // Windows has no `env -u`; those shells clear in-place, and neither has a + // nounset mode that could abort the line. + return `${clearEnvCommand(names, shell)}${commandSeparator(shell)}${command}` + } + for (const name of names) { + if (!ENV_VAR_NAME.test(name)) { + throw new Error( + `withoutEnvCommand: ${JSON.stringify(name)} is not an environment variable name` + ) + } + } + return `env ${names.map((name) => `-u ${name}`).join(' ')} ${command}` } export function commandSeparator(shell: AgentStartupShell): string { diff --git a/src/shared/tui-agent-startup.test.ts b/src/shared/tui-agent-startup.test.ts index b3220956677..12c9f56e270 100644 --- a/src/shared/tui-agent-startup.test.ts +++ b/src/shared/tui-agent-startup.test.ts @@ -28,6 +28,26 @@ function unwrapPowerShellScript(command: string | undefined): string { return Buffer.from(encoded!, 'base64').toString('utf16le') } +describe('draft prefill teardown ordering (#14975)', () => { + // Why pinned: the teardown mutates the calling shell, so it must reference + // $fish_pid, which aborts the line under `set -u`. That is survivable ONLY + // because it runs AFTER the agent — the agent is already up. Moving it before + // the command would make an aborted line a blocked launch, which is exactly + // what reverted #14863. + it('runs the clear after the agent command, never before it', () => { + const plan = buildAgentDraftLaunchPlan({ + agent: 'pi', + draft: 'hello', + cmdOverrides: {}, + platform: 'darwin' + }) + + const command = plan?.launchCommand ?? '' + expect(command.indexOf('pi')).toBeLessThan(command.indexOf('fish_pid')) + expect(command).toMatch(/^pi;/) + }) +}) + describe('tui agent startup plans', () => { it.each(['powershell', 'cmd'] as const)( 'keeps the established invalid-quote error on %s', @@ -47,7 +67,7 @@ describe('tui agent startup plans', () => { platform: 'linux' }) - expect(plan?.launchCommand).toBe("claude 'fix Bob'\\''s branch'") + expect(plan?.launchCommand).toBe("claude 'fix Bob'\"'\"'s branch'") }) it('uses PowerShell quoting by default when the target shell is Windows', () => { @@ -184,9 +204,12 @@ describe('tui agent startup plans', () => { }) expect(plan?.launchCommand).toMatch(/^sh -c /) - expect(plan?.launchCommand).toMatch(/\\0[0-7]{3}/) expect(plan?.launchCommand).not.toContain("'sh' '-c'") - expect(plan?.launchCommand).not.toContain("'\\''") + // Why parse rather than string-match: the octal escapes must survive the + // OUTER quoting to reach the inner sh, and portable quoting emits a + // backslash as `"\\"` rather than leaving it inside a single-quoted run. + const tokens = tokenizeStartupCommand(plan?.launchCommand ?? '', 'posix') + expect(tokens.ok && tokens.tokens.at(-1)).toMatch(/\\0[0-7]{3}/) }) it('moves Hermes command override flags after the chat subcommand', () => { @@ -856,7 +879,9 @@ describe('tui agent startup plans', () => { expect(plan).not.toBeNull() expect(plan?.env).toEqual({ ORCA_OMP_PREFILL: 'fix the omp regression' }) expect(plan?.expectedProcess).toBe('omp') - expect(plan?.launchCommand).toBe('omp; unset ORCA_OMP_PREFILL') + expect(plan?.launchCommand).toBe( + `omp; command test -n "$fish_pid" && set --erase -g ORCA_OMP_PREFILL; command test -z "$fish_pid" && unset ORCA_OMP_PREFILL; true` + ) }) it('returns null for oversized Windows flag drafts so callers paste after ready', () => {