diff --git a/src/main/codex/codex-session-resume-home.test.ts b/src/main/codex/codex-session-resume-home.test.ts index f2bf77107a6..6e639e97b0e 100644 --- a/src/main/codex/codex-session-resume-home.test.ts +++ b/src/main/codex/codex-session-resume-home.test.ts @@ -5,6 +5,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { claimsCodexRolloutLayout, findTrustedCodexSessionResume, + resolveCodexSessionResumeProvenance, resolveTrustedCodexSessionResumeHome } from './codex-session-resume-home' @@ -269,3 +270,76 @@ describe('claimsCodexRolloutLayout', () => { ).toBe(false) }) }) + +describe('resolveCodexSessionResumeProvenance', () => { + function writeRollout(sessionId: string): { homePath: string; rolloutPath: string } { + const homePath = mkdtempSync(join(tmpdir(), 'orca-codex-resume-provenance-')) + tempRoots.push(homePath) + const rolloutPath = join( + homePath, + 'sessions', + '2026', + '07', + '20', + `rollout-2026-07-20T12-00-00-${sessionId}.jsonl` + ) + mkdirSync(join(rolloutPath, '..'), { recursive: true }) + writeFileSync(rolloutPath, 'rollout') + return { homePath, rolloutPath } + } + + it('starts fresh for a rollout file that really exists under a home Orca no longer trusts', async () => { + // Why: the discriminating case — the file is present, so only the trust check can + // reject it. Resuming here would run the session under the selected account. + const sessionId = '019f81b9-19a9-7651-a8d1-352d9420bd11' + const removed = writeRollout(sessionId) + const trusted = mkdtempSync(join(tmpdir(), 'orca-codex-resume-provenance-')) + tempRoots.push(trusted) + + await expect( + resolveCodexSessionResumeProvenance({ + sessionId, + transcriptPath: removed.rolloutPath, + trustedCodexHomes: [trusted] + }) + ).resolves.toEqual({ outcome: 'fresh', claimedCodexProvenance: true }) + }) + + it('resumes from the originating home when that home is still trusted', async () => { + const sessionId = '019f81b9-19a9-7651-a8d1-352d9420bd11' + const origin = writeRollout(sessionId) + + await expect( + resolveCodexSessionResumeProvenance({ + sessionId, + transcriptPath: origin.rolloutPath, + trustedCodexHomes: [origin.homePath] + }) + ).resolves.toEqual({ + outcome: 'resume', + homePath: origin.homePath, + transcriptPath: origin.rolloutPath + }) + }) + + it('starts fresh silently for cross-agent provenance on a pane relabeled codex', async () => { + await expect( + resolveCodexSessionResumeProvenance({ + sessionId: '019f81b9-19a9-7651-a8d1-352d9420bd11', + transcriptPath: '/Users/example/.claude/projects/repo/019f81b9.jsonl', + trustedCodexHomes: ['/Users/example/.codex'] + }) + ).resolves.toEqual({ outcome: 'fresh', claimedCodexProvenance: false }) + }) + + it('reports a missing rollout under a trusted home as rejected Codex provenance', async () => { + await expect( + resolveCodexSessionResumeProvenance({ + sessionId: '019f81b9-19a9-7651-a8d1-352d9420bd11', + transcriptPath: '/Users/example/.codex/sessions/2026/07/20/rollout-gone.jsonl', + trustedCodexHomes: ['/Users/example/.codex'], + fileIsRegular: () => false + }) + ).resolves.toEqual({ outcome: 'fresh', claimedCodexProvenance: true }) + }) +}) diff --git a/src/main/codex/codex-session-resume-home.ts b/src/main/codex/codex-session-resume-home.ts index 639d2620eb7..08d701bd00d 100644 --- a/src/main/codex/codex-session-resume-home.ts +++ b/src/main/codex/codex-session-resume-home.ts @@ -13,6 +13,15 @@ const ROLLOUT_RELATIVE_PATH = new RegExp(`^${DATED_ROLLOUT_TAIL}$`) // Why: case-insensitive because trusted-home matching folds Windows path case too. const CODEX_ROLLOUT_LAYOUT_PATH = new RegExp(`(?:^|/)sessions/${DATED_ROLLOUT_TAIL}$`, 'i') +/** `resume` pins CODEX_HOME to the account that owns the rollout. `fresh` means + * provenance could not be verified, so the caller drops the resume argv — an + * unverifiable rollout must never resume under whichever account is selected now. + * `claimedCodexProvenance` gates the user-facing notice: a path that claimed real + * Codex layout is worth reporting, stale cross-agent metadata is not. */ +export type CodexSessionResumePreparation = + | { outcome: 'resume'; codexHomePath: string } + | { outcome: 'fresh'; claimedCodexProvenance: boolean } + function isCodexRolloutInsideSessionsRoot(sessionsRoot: string, filePath: string): boolean { const relativePath = relativePathInsideRoot(sessionsRoot, filePath) return Boolean(relativePath && ROLLOUT_RELATIVE_PATH.test(relativePath.replace(/\\/g, '/'))) @@ -93,6 +102,27 @@ export function claimsCodexRolloutLayout(transcriptPath: string | undefined): bo return CODEX_ROLLOUT_LAYOUT_PATH.test(persistedPath.replace(/\\/g, '/')) } +/** + * Verified provenance, or an explicit fall-back to a fresh session. Never rejects: + * the caller drops the resume argv on `fresh`, so a rollout Orca cannot place under a + * trusted home resumes nowhere instead of resuming under the selected account (#10793). + */ +export async function resolveCodexSessionResumeProvenance(args: { + sessionId: string + transcriptPath: string | undefined + trustedCodexHomes: readonly string[] + fileIsRegular?: (filePath: string) => boolean + listSessionFiles?: (sessionsRoot: string) => AsyncIterable +}): Promise< + | { outcome: 'resume'; homePath: string; transcriptPath: string } + | { outcome: 'fresh'; claimedCodexProvenance: boolean } +> { + const sessionSource = await findTrustedCodexSessionResume(args) + return sessionSource + ? { outcome: 'resume', ...sessionSource } + : { outcome: 'fresh', claimedCodexProvenance: claimsCodexRolloutLayout(args.transcriptPath) } +} + export async function findTrustedCodexSessionResume(args: { sessionId: string transcriptPath: string | undefined diff --git a/src/main/codex/codex-unverified-resume-launch.ts b/src/main/codex/codex-unverified-resume-launch.ts new file mode 100644 index 00000000000..746b5a759ff --- /dev/null +++ b/src/main/codex/codex-unverified-resume-launch.ts @@ -0,0 +1,32 @@ +import { dropAgentResumeArgvFromCommand } from '../../shared/agent-resume-argv-drop' +import type { AgentProviderSessionMetadata } from '../../shared/agent-session-resume' + +export const UNVERIFIED_CODEX_RESUME_ERROR = + 'Orca could not verify the originating Codex session file, so automatic resume was stopped to avoid using a different account.' + +/** + * Launch command for a Codex resume whose originating account could not be verified: + * the resume argv is dropped so the pane starts a clean session instead of resuming + * under whichever account is selected now (#10793). Throws only when the resume + * locator is present but not strippable — launching then could still cross accounts. + */ +export function dropUnverifiedCodexResumeArgv(args: { + command: string | undefined + providerSession: AgentProviderSessionMetadata | null +}): { command: string | undefined; droppedResumeArgv: boolean } { + if (!args.command || !args.providerSession) { + return { command: args.command, droppedResumeArgv: false } + } + const drop = dropAgentResumeArgvFromCommand({ + command: args.command, + agent: 'codex', + providerSession: args.providerSession + }) + if (drop.status === 'dropped') { + return { command: drop.command, droppedResumeArgv: true } + } + if (drop.status === 'unrecognized') { + throw new Error(UNVERIFIED_CODEX_RESUME_ERROR) + } + return { command: args.command, droppedResumeArgv: false } +} diff --git a/src/main/index.ts b/src/main/index.ts index ec2dbdc1b63..5c885e8771b 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -173,8 +173,8 @@ import { createCodexSessionMigrationScheduler } from './codex/codex-session-migr import { prepareLegacySharedCodexSessionResume } from './codex/codex-legacy-session-resume' import { resolveHostCodexSessionSourceHome } from './codex/codex-session-source-home' import { - claimsCodexRolloutLayout, - findTrustedCodexSessionResume + resolveCodexSessionResumeProvenance, + type CodexSessionResumePreparation } from './codex/codex-session-resume-home' import { getSystemCodexHomePath } from './codex/codex-home-paths' import { normalizeRuntimePathForComparison } from '../shared/cross-platform-path' @@ -839,7 +839,7 @@ async function prepareCodexSessionResumeForLaunch(args: { target: CodexAccountSelectionTarget launchEnv?: NodeJS.ProcessEnv workspacePath?: string -}): Promise<{ codexHomePath: string | null } | null> { +}): Promise { if (args.target.runtime === 'wsl' || !codexRuntimeHome || !store) { return null } @@ -849,20 +849,15 @@ async function prepareCodexSessionResumeForLaunch(args: { systemHomePath, ...codexRuntimeHome.getHostCodexHomePathsForSessionDiscovery() ] - const sessionSource = await findTrustedCodexSessionResume({ + const sessionSource = await resolveCodexSessionResumeProvenance({ sessionId: args.providerSession.id, transcriptPath: args.providerSession.transcriptPath, trustedCodexHomes: trustedHomes }) - if (!sessionSource) { - // Why: an unverifiable Codex rollout still blocks; only paths that never claimed Codex - // provenance (e.g. ~/.claude/… on a pane mislabeled "codex") fall through to a normal launch. - if (claimsCodexRolloutLayout(args.providerSession.transcriptPath)) { - throw new Error( - 'Orca could not verify the originating Codex session file, so automatic resume was stopped to avoid using a different account.' - ) - } - return null + if (sessionSource.outcome === 'fresh') { + // Why: unverifiable provenance drops the resume argv instead of failing the spawn — + // with nothing to resume, cross-account resume is impossible by construction (#10793). + return sessionSource } let migrated = { useRealCodexHome: false } @@ -911,7 +906,7 @@ async function prepareCodexSessionResumeForLaunch(args: { // Why: hook repair is best-effort; session provenance must still win over the currently selected home. console.warn('[codex-hook-service] failed to prepare automatic resume home:', error) } - return { codexHomePath: resumeHome } + return { outcome: 'resume', codexHomePath: resumeHome } } // Why: restore the window the close handler may have hidden to tray, or reopen it (dock-reactivation style) if fully torn down. diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index f10355161e7..b04566a209f 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { userInfo } from 'node:os' import { delimiter, join, posix } from 'node:path' +import { resolveCodexSessionResumeProvenance } from '../codex/codex-session-resume-home' import { TERMINAL_INPUT_CHUNK_MAX_BYTES, TERMINAL_INPUT_MAX_BYTES @@ -1734,7 +1735,10 @@ describe('registerPtyHandlers', () => { it('resumes an automatic Codex session from its prepared originating home', async () => { const selectedHome = vi.fn(() => '/managed/current/home') - const prepareResume = vi.fn(async () => ({ codexHomePath: '/managed/origin/home' })) + const prepareResume = vi.fn(async () => ({ + outcome: 'resume' as const, + codexHomePath: '/managed/origin/home' + })) registerPtyHandlers( mainWindow as never, undefined, @@ -1779,7 +1783,12 @@ describe('registerPtyHandlers', () => { undefined, undefined, undefined, - { prepareCodexSessionResume: async () => ({ codexHomePath: systemHome }) } + { + prepareCodexSessionResume: async () => ({ + outcome: 'resume' as const, + codexHomePath: systemHome + }) + } ) await handlers.get('pty:spawn')!(null, { @@ -1837,6 +1846,136 @@ describe('registerPtyHandlers', () => { expect(spawnMock).not.toHaveBeenCalled() }) + describe('unverifiable Codex resume provenance', () => { + const RESUME_SESSION_ID = '019f81b9-19a9-7651-a8d1-352d9420bd11' + const ORIGIN_HOME = '/managed/origin/home' + const OTHER_HOME = '/managed/other/home' + const ORIGIN_ROLLOUT = `${ORIGIN_HOME}/sessions/2026/07/20/rollout-2026-07-20T12-00-00-${RESUME_SESSION_ID}.jsonl` + + // Why: main's real provenance rule, so these cases exercise the same decision + // prepareCodexSessionResumeForLaunch makes instead of a hand-written stub. This + // suite mocks fs, so the rollout is declared present — the only variable left is + // whether its home is trusted, which is exactly the case the guard exists for. + function registerWithTrustedHomes(trustedHomes: readonly string[], selectedHome: string) { + const selectedHomeMock = vi.fn(() => selectedHome) + registerPtyHandlers( + mainWindow as never, + undefined, + selectedHomeMock, + undefined, + undefined, + undefined, + { + prepareCodexSessionResume: async ({ providerSession }) => { + const provenance = await resolveCodexSessionResumeProvenance({ + sessionId: providerSession.id, + transcriptPath: providerSession.transcriptPath, + trustedCodexHomes: trustedHomes, + fileIsRegular: () => true + }) + return provenance.outcome === 'resume' + ? { outcome: 'resume' as const, codexHomePath: provenance.homePath } + : provenance + } + } + ) + return selectedHomeMock + } + + async function spawnCodexResume( + transcriptPath: string | undefined + ): Promise<{ agentResumeUnavailable?: true }> { + return (await handlers.get('pty:spawn')!(null, { + cols: 80, + rows: 24, + cwd: '/tmp', + command: `codex 'resume' '${RESUME_SESSION_ID}'`, + launchAgent: 'codex', + resumeProviderSession: { + key: 'session_id', + id: RESUME_SESSION_ID, + ...(transcriptPath ? { transcriptPath } : {}) + } + })) as { agentResumeUnavailable?: true } + } + + posixOnlyIt( + 'launches plain codex when a REAL rollout sits under a home Orca no longer trusts', + async () => { + // Why: the discriminating case — the rollout exists, so only the trust check can + // reject it. Falling through would resume it under the selected account. + const mockProc = createMockProc() + spawnMock.mockReturnValue(mockProc.proc) + vi.useFakeTimers() + + try { + const selectedHome = registerWithTrustedHomes([OTHER_HOME], OTHER_HOME) + const spawned = await spawnCodexResume(ORIGIN_ROLLOUT) + + await Promise.resolve() + vi.runAllTimers() + await Promise.resolve() + vi.runAllTimers() + + expect(mockProc.proc.write).toHaveBeenCalledWith('codex\n') + expect(mockProc.proc.write).not.toHaveBeenCalledWith( + expect.stringContaining(RESUME_SESSION_ID) + ) + expect(spawned.agentResumeUnavailable).toBe(true) + // The pane still runs under the selected account — but with nothing to resume. + const env = spawnMock.mock.calls.at(-1)![2].env as Record + expect(env.CODEX_HOME).toBe(OTHER_HOME) + expect(selectedHome).toHaveBeenCalled() + } finally { + vi.useRealTimers() + } + } + ) + + it('reports the dropped resume so the pane can say it started fresh', async () => { + registerWithTrustedHomes([OTHER_HOME], OTHER_HOME) + const spawned = await spawnCodexResume(ORIGIN_ROLLOUT) + + expect(spawned.agentResumeUnavailable).toBe(true) + }) + + it('stays silent for cross-agent provenance on a pane relabeled codex', async () => { + registerWithTrustedHomes([OTHER_HOME], OTHER_HOME) + + const spawned = await spawnCodexResume( + '/Users/example/.claude/projects/repo/019f81b9.jsonl' + ) + + expect(spawned.agentResumeUnavailable).toBeUndefined() + }) + + posixOnlyIt('still pins CODEX_HOME and resumes when provenance is verified', async () => { + const mockProc = createMockProc() + spawnMock.mockReturnValue(mockProc.proc) + vi.useFakeTimers() + + try { + const selectedHome = registerWithTrustedHomes([ORIGIN_HOME], OTHER_HOME) + const spawned = await spawnCodexResume(ORIGIN_ROLLOUT) + + await Promise.resolve() + vi.runAllTimers() + await Promise.resolve() + vi.runAllTimers() + + expect(mockProc.proc.write).toHaveBeenCalledWith( + `codex 'resume' '${RESUME_SESSION_ID}'\n` + ) + expect(spawned.agentResumeUnavailable).toBeUndefined() + const env = spawnMock.mock.calls.at(-1)![2].env as Record + expect(env.CODEX_HOME).toBe(ORIGIN_HOME) + expect(selectedHome).not.toHaveBeenCalled() + } finally { + vi.useRealTimers() + } + }) + }) + it('prepares Codex launch state for the workspace before spawning an interactive tab', async () => { const workspacePath = '/repo/worktrees/new-feature' const resolveHome = vi.fn( @@ -2653,7 +2792,12 @@ describe('registerPtyHandlers', () => { undefined, undefined, undefined, - { prepareCodexSessionResume: async () => ({ codexHomePath: systemHome }) } + { + prepareCodexSessionResume: async () => ({ + outcome: 'resume' as const, + codexHomePath: systemHome + }) + } ) await handlers.get('pty:spawn')!(null, { @@ -2711,7 +2855,12 @@ describe('registerPtyHandlers', () => { undefined, undefined, undefined, - { prepareCodexSessionResume: async () => ({ codexHomePath: systemHome }) } + { + prepareCodexSessionResume: async () => ({ + outcome: 'resume' as const, + codexHomePath: systemHome + }) + } ) const controller = runtime.setPtyController.mock.calls[0]?.[0] as RuntimeSpawnController diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index ff089432508..db6d5e6e626 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -163,6 +163,8 @@ import { validateTerminalViewAttributes } from '../../shared/terminal-view-attri import type { PtyModelRestoreReason } from '../../shared/pty-model-restore-marker' import type { CodexAccountSelectionTarget } from '../codex-accounts/runtime-selection' import { isCodexSystemDefaultRealHomeEnabled } from '../codex/codex-real-home-flag' +import type { CodexSessionResumePreparation } from '../codex/codex-session-resume-home' +import { dropUnverifiedCodexResumeArgv } from '../codex/codex-unverified-resume-launch' import { isHostCodexHomeForWsl, isWslCodexHomeForHost } from '../pty/codex-home-wsl-env' import { buildConfiguredProxyEnv, type NetworkProxySettings } from '../../shared/network-proxy' import { resolveSetupAgentSequenceLaunchCommand } from '../../shared/setup-agent-sequencing' @@ -787,7 +789,7 @@ export type PrepareCodexSessionResume = (args: { target: CodexAccountSelectionTarget launchEnv?: NodeJS.ProcessEnv workspacePath?: string -}) => Promise<{ codexHomePath: string | null } | null> +}) => Promise type PrepareClaudeAuth = ( target?: ClaudeAccountSelectionTarget ) => Promise @@ -3126,7 +3128,10 @@ export function registerPtyHandlers( target: CodexAccountSelectionTarget launchEnv?: NodeJS.ProcessEnv workspacePath?: string - }): Promise<{ codexHomePath: string | null } | null> | null => { + }): { + providerSession: AgentProviderSessionMetadata + preparation: Promise + } | null => { if (args.connectionId || args.launchAgent !== 'codex' || !options?.prepareCodexSessionResume) { return null } @@ -3134,14 +3139,52 @@ export function registerPtyHandlers( if (!providerSession) { return null } - return options.prepareCodexSessionResume({ + return { providerSession, - target: args.target, - launchEnv: args.launchEnv, - workspacePath: args.workspacePath - }) + preparation: options.prepareCodexSessionResume({ + providerSession, + target: args.target, + launchEnv: args.launchEnv, + workspacePath: args.workspacePath + }) + } } + type CodexResumeLaunch = { + codexResumeHome: { codexHomePath: string } | null + command: string | undefined + notifyResumeUnavailable: boolean + } + + /** Kept separate from resolveCodexResumeLaunch so non-Codex spawns never await: + * an extra tick reorders the pane-spawn reservation races this handler arbitrates. */ + const noCodexResumeLaunch = (command: string | undefined): CodexResumeLaunch => ({ + codexResumeHome: null, + command, + notifyResumeUnavailable: false + }) + + /** The command a Codex launch actually runs: unchanged when provenance is verified, + * stripped of `resume ` when it is not. */ + const resolveCodexResumeLaunch = ( + command: string | undefined, + preparation: NonNullable> + ): Promise => + preparation.preparation.then((prepared) => { + if (prepared?.outcome !== 'fresh') { + return { codexResumeHome: prepared ?? null, command, notifyResumeUnavailable: false } + } + const dropped = dropUnverifiedCodexResumeArgv({ + command, + providerSession: preparation.providerSession + }) + return { + codexResumeHome: null, + command: dropped.command, + notifyResumeUnavailable: dropped.droppedResumeArgv && prepared.claimedCodexProvenance + } + }) + // Why: route through getProviderForPty() so CLI commands work for remote PTYs too; localProvider would silently fail for them. runtime?.setPtyController({ spawn: async (args) => { @@ -3180,7 +3223,11 @@ export function registerPtyHandlers( launchEnv: args.env, workspacePath: cwd }) - const codexResumeHome = codexResumePreparation ? await codexResumePreparation : null + const codexResumeLaunch = codexResumePreparation + ? await resolveCodexResumeLaunch(args.command, codexResumePreparation) + : noCodexResumeLaunch(args.command) + const codexResumeHome = codexResumeLaunch.codexResumeHome + const launchCommand = codexResumeLaunch.command const claudeAuth = isClaudeLaunch && prepareClaudeAuth ? await prepareClaudeAuth(codexSelectionTarget) : null if (isClaudeLaunch && isClaudeAuthSwitchInProgress()) { @@ -3286,7 +3333,7 @@ export function registerPtyHandlers( skipCodexHomeEnv, stripInheritedOrcaCodexHome, githubAttributionEnabled: getSettings?.()?.enableGitHubAttribution ?? false, - launchCommand: args.command, + launchCommand, launchAgent: isTuiAgent(args.launchAgent) ? args.launchAgent : undefined, shellPath: daemonShellOverride ?? process.env.COMSPEC, isWsl: shouldSkipCodexHomeEnvForWindowsShell(daemonShellOverride, cwd), @@ -3347,8 +3394,8 @@ export function registerPtyHandlers( } deleteRequestedEnvKeys(env, spawnOptions.envToDelete) promoteAgentTeamsShimPath(env, requestedAgentTeamsPath) - if (args.command !== undefined) { - spawnOptions.command = args.command + if (launchCommand !== undefined) { + spawnOptions.command = launchCommand } if (args.commandDelivery !== undefined) { spawnOptions.commandDelivery = args.commandDelivery @@ -3742,7 +3789,7 @@ export function registerPtyHandlers( runtime?.cancelPendingPtyRegistration?.(result.id, result.incarnationId) } // Why: arms main's per-PTY Command Code output detector from the launch command (renderer startupCommand parity). - runtime?.noteTerminalSpawnCommand?.(result.id, args.command ?? null) + runtime?.noteTerminalSpawnCommand?.(result.id, launchCommand ?? null) if (isClaudeLaunch) { markClaudePtySpawned(result.id) } @@ -4400,7 +4447,11 @@ export function registerPtyHandlers( launchEnv: baseEnv, workspacePath: cwd }) - const codexResumeHome = codexResumePreparation ? await codexResumePreparation : null + const codexResumeLaunch = codexResumePreparation + ? await resolveCodexResumeLaunch(args.command, codexResumePreparation) + : noCodexResumeLaunch(args.command) + const codexResumeHome = codexResumeLaunch.codexResumeHome + const launchCommand = codexResumeLaunch.command const selectedCodexHomePath = isDaemonHostSpawn ? getCompatibleSelectedCodexHomePath( codexSelectionTarget, @@ -4444,7 +4495,7 @@ export function registerPtyHandlers( skipCodexHomeEnv, stripInheritedOrcaCodexHome, githubAttributionEnabled: getSettings?.()?.enableGitHubAttribution ?? false, - launchCommand: args.command, + launchCommand, launchAgent: isTuiAgent(args.launchAgent) ? args.launchAgent : undefined, shellPath: effectiveShellOverride ?? process.env.COMSPEC, isWsl: shouldSkipCodexHomeEnvForWindowsShell(effectiveShellOverride, cwd), @@ -4504,8 +4555,8 @@ export function registerPtyHandlers( if (combinedEnvToDelete) { spawnOptions.envToDelete = combinedEnvToDelete } - if (args.command !== undefined) { - spawnOptions.command = args.command + if (launchCommand !== undefined) { + spawnOptions.command = launchCommand } if (args.commandDelivery !== undefined) { spawnOptions.commandDelivery = args.commandDelivery @@ -4877,7 +4928,7 @@ export function registerPtyHandlers( // Why: arm main's per-PTY Command Code output detector from the launch command (startupCommand parity); banner detection covers PTYs without one. runtime?.noteTerminalSpawnCommand?.( result.id, - typeof args.command === 'string' ? args.command : null + typeof launchCommand === 'string' ? launchCommand : null ) if (isClaudeLaunch) { markClaudePtySpawned(result.id) @@ -4947,7 +4998,12 @@ export function registerPtyHandlers( ? { launchConfig: effectiveLaunchConfig } : {}), // Why: a daemon-retry race can surface isReattach even for a minted session id, and a reattach must never claim its cwd was remapped. - ...(startupCwdFallback && !result.isReattach ? { startupCwdFallback } : {}) + ...(startupCwdFallback && !result.isReattach ? { startupCwdFallback } : {}), + // Why: the pane asked to resume and got a fresh session instead; only the + // renderer can say so, and a reattach never ran this launch command. + ...(codexResumeLaunch.notifyResumeUnavailable && !result.isReattach + ? { agentResumeUnavailable: true as const } + : {}) } // Why: renderer tab state cannot reliably infer background and reattached PTYs in the daemon inventory. sendPtySpawnedToRenderer(result.id) diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 2592e08e9e6..568b060528d 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -1335,6 +1335,7 @@ export type PreloadApi = { sessionExpired?: boolean coldRestore?: { scrollback: string; cwd: string; cols?: number; rows?: number } startupCwdFallback?: { kind: 'worktree'; cwd: string } + agentResumeUnavailable?: true }> write: (id: string, data: string) => void writeAccepted: (id: string, data: string) => Promise diff --git a/src/preload/index.ts b/src/preload/index.ts index 21014fc1a35..51702c8a9f9 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -837,6 +837,7 @@ const api = { sessionExpired?: boolean coldRestore?: { scrollback: string; cwd: string; cols?: number; rows?: number } startupCwdFallback?: { kind: 'worktree'; cwd: string } + agentResumeUnavailable?: true }> => ipcRenderer.invoke('pty:spawn', opts), write: (id: string, data: string): void => { diff --git a/src/renderer/src/components/terminal-pane/SessionRestoredBanner.tsx b/src/renderer/src/components/terminal-pane/SessionRestoredBanner.tsx index 2f4c4959713..47976abc73b 100644 --- a/src/renderer/src/components/terminal-pane/SessionRestoredBanner.tsx +++ b/src/renderer/src/components/terminal-pane/SessionRestoredBanner.tsx @@ -1,15 +1,27 @@ +import type { SessionRestoredBannerReason } from './session-restored-banner-pane-state' + export const SESSION_RESTORED_BANNER_TEXT = '--- session restored ---' +export const SESSION_RESUME_UNAVAILABLE_BANNER_TEXT = + '--- previous session unavailable, started fresh ---' type SessionRestoredBannerProps = { visible: boolean + reason?: SessionRestoredBannerReason } export function SessionRestoredBanner({ - visible + visible, + reason = 'restored' }: SessionRestoredBannerProps): React.JSX.Element | null { if (!visible) { return null } - return
{SESSION_RESTORED_BANNER_TEXT}
+ return ( +
+ {reason === 'resume-unavailable' + ? SESSION_RESUME_UNAVAILABLE_BANNER_TEXT + : SESSION_RESTORED_BANNER_TEXT} +
+ ) } diff --git a/src/renderer/src/components/terminal-pane/SessionRestoredBannerPortals.tsx b/src/renderer/src/components/terminal-pane/SessionRestoredBannerPortals.tsx index ecc52befa2c..d4c7d0afba9 100644 --- a/src/renderer/src/components/terminal-pane/SessionRestoredBannerPortals.tsx +++ b/src/renderer/src/components/terminal-pane/SessionRestoredBannerPortals.tsx @@ -1,10 +1,13 @@ import { createPortal } from 'react-dom' import { SessionRestoredBanner } from './SessionRestoredBanner' -import type { SessionRestoredBannerPane } from './session-restored-banner-pane-state' +import type { + SessionRestoredBannerPane, + SessionRestoredBannerPaneReasons +} from './session-restored-banner-pane-state' type SessionRestoredBannerPortalsProps = { panes: readonly SessionRestoredBannerPane[] - paneIds: ReadonlySet + paneIds: SessionRestoredBannerPaneReasons } export function SessionRestoredBannerPortals({ @@ -14,13 +17,14 @@ export function SessionRestoredBannerPortals({ return ( <> {panes.map((pane) => { - if (!paneIds.has(pane.id)) { + const reason = paneIds.get(pane.id) + if (!reason) { return null } return createPortal( // Why: resumed TUIs repaint xterm immediately, so the wake marker // must live in that pane's chrome instead of the PTY byte stream. - , + , pane.container, `session-restored-banner-${pane.id}` ) diff --git a/src/renderer/src/components/terminal-pane/TerminalPane.tsx b/src/renderer/src/components/terminal-pane/TerminalPane.tsx index 20bc410c681..c31e6d96852 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPane.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPane.tsx @@ -76,7 +76,8 @@ import { pruneSessionRestoredBannerPaneIds, removeSessionRestoredBannerPaneId, syncSessionRestoredBannerTitleSpace, - type SessionRestoredBannerDismissEvent + type SessionRestoredBannerDismissEvent, + type SessionRestoredBannerReason } from './session-restored-banner-pane-state' import { useSystemPrefersDark } from './use-system-prefers-dark' import { useTerminalPaneGlobalEffects } from './use-terminal-pane-global-effects' @@ -790,9 +791,9 @@ export default function TerminalPane({ const [shouldMeasureHiddenStartup, setShouldMeasureHiddenStartup] = useState( () => startup !== undefined && !isVisible ) - const [sessionRestoredBannerPaneIds, setSessionRestoredBannerPaneIds] = useState>( - () => new Set() - ) + const [sessionRestoredBannerPaneIds, setSessionRestoredBannerPaneIds] = useState< + Map + >(() => new Map()) const consumeTabStartupCommand = useAppStore((store) => store.consumeTabStartupCommand) const [setupSplit] = useState(() => useAppStore.getState().pendingSetupSplitByTabId[tabId]) const consumeTabSetupSplit = useAppStore((store) => store.consumeTabSetupSplit) @@ -824,12 +825,15 @@ export default function TerminalPane({ }) }, []) - const showRestoredSessionBanner = useCallback((paneId: number): void => { - setSessionRestoredBannerPaneIds((prev) => { - const next = addSessionRestoredBannerPaneId(prev, paneId) - return next === prev ? prev : next - }) - }, []) + const showRestoredSessionBanner = useCallback( + (paneId: number, reason: SessionRestoredBannerReason = 'restored'): void => { + setSessionRestoredBannerPaneIds((prev) => { + const next = addSessionRestoredBannerPaneId(prev, paneId, reason) + return next === prev ? prev : next + }) + }, + [] + ) const dismissSessionRestoredBanner = useCallback( (event: SessionRestoredBannerDismissEvent): void => { diff --git a/src/renderer/src/components/terminal-pane/pty-connection-types.ts b/src/renderer/src/components/terminal-pane/pty-connection-types.ts index bfd2d97ee02..bcbae661a6a 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-types.ts @@ -1,4 +1,5 @@ import type { PtyTransport } from './pty-transport' +import type { SessionRestoredBannerReason } from './session-restored-banner-pane-state' import type { ReplayingPanesRef } from './replay-guard' import type { RestoredViewportBlankingPanesRef } from './terminal-restored-viewport' import type { AgentCompletionStatusSnapshot } from './agent-completion-coordinator-types' @@ -76,7 +77,7 @@ export type PtyConnectionDeps = { clearWorktreeUnread: (worktreeId: string) => void clearTerminalTabUnread: (tabId: string) => void clearTerminalPaneUnread: (paneKey: string) => void - onShowSessionRestoredBanner: (paneId: number) => void + onShowSessionRestoredBanner: (paneId: number, reason?: SessionRestoredBannerReason) => void // Why: the renderer dispatches two notification sources — BEL from the PTY // byte stream and agent-task-complete on the working→idle title transition. // shared/types.ts keeps a wider NotificationEventSource union because the diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index ca3b9cb777c..8b152c96f45 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -8102,7 +8102,7 @@ describe('connectPanePty', () => { const writeCalls = pane.terminal.write.mock.calls.map(([data]) => data) expect(writeCalls.findIndex((data) => data.includes('--- session restored ---'))).toBe(-1) expect(deps.onShowSessionRestoredBanner).toHaveBeenCalledTimes(1) - expect(deps.onShowSessionRestoredBanner).toHaveBeenCalledWith(1) + expect(deps.onShowSessionRestoredBanner).toHaveBeenCalledWith(1, 'restored') expect(transport.sendInput).not.toHaveBeenCalled() expect(transport.connect).toHaveBeenCalledWith( expect.objectContaining({ @@ -8121,6 +8121,63 @@ describe('connectPanePty', () => { expect(mockStoreState.clearSleepingAgentSession).toHaveBeenCalledWith(paneKey) }) + it('marks the pane as freshly started when main declined an unverifiable resume', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('fresh-pty') + transport.connect.mockImplementation(async ({ sessionId }: { sessionId?: string }) => { + if (sessionId) { + // Why: main dropped `resume ` because it could not verify which Codex + // account owns the rollout, so this PTY is a new session, not a restore. + return { + id: 'fresh-pty', + coldRestore: { scrollback: 'cold-payload', cwd: '/tmp/wt-1' }, + agentResumeUnavailable: true as const + } + } + return 'fresh-pty' + }) + transportFactoryQueue.push(transport) + const paneKey = makePaneKey('tab-1', LEAF_1) + mockStoreState = { + ...mockStoreState, + tabsByWorktree: { + 'wt-1': [{ id: 'tab-1', ptyId: 'lost-pty' }] + }, + settings: { + ...mockStoreState.settings, + agentCmdOverrides: {} + }, + agentStatusByPaneKey: {}, + sleepingAgentSessionsByPaneKey: { + [paneKey]: { + paneKey, + tabId: 'tab-1', + worktreeId: 'wt-1', + agent: 'codex', + providerSession: { key: 'session_id', id: 'codex-session-1' }, + prompt: 'finish the task', + state: 'working', + capturedAt: 1, + updatedAt: 1 + } + } + } as StoreState + + const pane = createPane(1) + const manager = createManager(1) + const deps = createDeps({ + restoredLeafId: LEAF_1, + restoredPtyIdByLeafId: { [LEAF_1]: 'lost-pty' } + }) + + connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks(20) + await new Promise((resolve) => setTimeout(resolve, 70)) + + expect(deps.onShowSessionRestoredBanner).toHaveBeenCalledTimes(1) + expect(deps.onShowSessionRestoredBanner).toHaveBeenCalledWith(1, 'resume-unavailable') + }) + it('resumes from an unambiguous legacy sleeping record when cold-restoring a preserved pane', async () => { const { connectPanePty } = await import('./pty-connection') const transport = createMockTransport('fresh-pty') @@ -8184,7 +8241,7 @@ describe('connectPanePty', () => { await new Promise((resolve) => setTimeout(resolve, 70)) expect(pane.terminal.write).toHaveBeenCalledWith('cold-payload', expect.any(Function)) - expect(deps.onShowSessionRestoredBanner).toHaveBeenCalledWith(1) + expect(deps.onShowSessionRestoredBanner).toHaveBeenCalledWith(1, 'restored') expect(transport.sendInput).not.toHaveBeenCalled() expect(transport.connect).toHaveBeenCalledWith( expect.objectContaining({ @@ -8694,7 +8751,7 @@ describe('connectPanePty', () => { expect.any(Function) ) expect(deps.onShowSessionRestoredBanner).toHaveBeenCalledTimes(1) - expect(deps.onShowSessionRestoredBanner).toHaveBeenCalledWith(2) + expect(deps.onShowSessionRestoredBanner).toHaveBeenCalledWith(2, 'restored') expect(mockStoreState.clearSleepingAgentSession).toHaveBeenCalledWith(paneKey) }) @@ -8797,7 +8854,7 @@ describe('connectPanePty', () => { await flushAsyncTicks(10) expect(deps.onShowSessionRestoredBanner).toHaveBeenCalledTimes(1) - expect(deps.onShowSessionRestoredBanner).toHaveBeenCalledWith(2) + expect(deps.onShowSessionRestoredBanner).toHaveBeenCalledWith(2, 'restored') expect(mockStoreState.clearSleepingAgentSession).toHaveBeenCalledWith(paneKey) }) diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index af467597005..77623c9e8cd 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -50,6 +50,7 @@ import { type HasPty } from './terminal-dead-session-reconcile' import type { PtyConnectionDeps } from './pty-connection-types' +import type { SessionRestoredBannerReason } from './session-restored-banner-pane-state' import { consumeCommittedPtyShutdownExit, deferPtyShutdownExit, @@ -4502,12 +4503,12 @@ export function connectPanePty( armStartupDraftReadinessObservation() } let sessionRestoredBannerShown = false - const showSessionRestoredBanner = (): void => { + const showSessionRestoredBanner = (reason: SessionRestoredBannerReason = 'restored'): void => { if (sessionRestoredBannerShown) { return } sessionRestoredBannerShown = true - deps.onShowSessionRestoredBanner(pane.id) + deps.onShowSessionRestoredBanner(pane.id, reason) } const getColdRestoreAgentResumePlatform = (): NodeJS.Platform => { if (projectRuntime?.status === 'repair-required') { @@ -4849,7 +4850,15 @@ export function connectPanePty( foreground: shouldWritePtyOutputForeground(deps.isVisibleRef.current) }) } - if (coldRestoreOverride?.hasSleepingRecord) { + if ( + spawnedPtyId && + typeof spawnedPtyId === 'object' && + spawnedPtyId.agentResumeUnavailable + ) { + // Why: main dropped the resume argv, so this pane is a NEW session — + // the plain restored banner would claim the old one came back. + showSessionRestoredBanner('resume-unavailable') + } else if (coldRestoreOverride?.hasSleepingRecord) { showSessionRestoredBanner() } clearSleepingRecordAfterColdRestoreSpawn(coldRestoreOverride) @@ -7443,7 +7452,11 @@ export function connectPanePty( const preparedStartup = coldRestoreStartup ?? buildColdRestoreAgentResumeStartup() const didPrepareResume = applyColdRestoreAgentResumeStartup(preparedStartup) if (didPrepareResume) { - if (preparedStartup?.hasSleepingRecord) { + if (connectResult.agentResumeUnavailable) { + // Why: main dropped the resume argv, so this pane is a NEW session — + // the plain restored banner would claim the old one came back. + showSessionRestoredBanner('resume-unavailable') + } else if (preparedStartup?.hasSleepingRecord) { showSessionRestoredBanner() } clearSleepingRecordAfterColdRestoreSpawn(preparedStartup) diff --git a/src/renderer/src/components/terminal-pane/pty-transport-types.ts b/src/renderer/src/components/terminal-pane/pty-transport-types.ts index 81cfd5575be..804655edbdf 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-types.ts @@ -63,6 +63,8 @@ export type PtyConnectResult = { coldRestore?: { scrollback: string; cwd: string; cols?: number; rows?: number } replay?: string startupCwdFallback?: { kind: 'worktree'; cwd: string } + /** Main declined an unverifiable provider-session resume and launched fresh. */ + agentResumeUnavailable?: true /** Trailing partial escape the daemon emulator held mid-parse; the reattach * replay writes it LAST (after the reset) so a racing live continuation * completes it instead of rendering literally (#7329). */ diff --git a/src/renderer/src/components/terminal-pane/pty-transport.test.ts b/src/renderer/src/components/terminal-pane/pty-transport.test.ts index 80bf875ccb5..a0bc22e229c 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.test.ts @@ -480,6 +480,30 @@ describe('createIpcPtyTransport', () => { transport.disconnect() }) + it('forwards the declined-resume signal on fresh and cold-restore spawns alike', async () => { + const { createIpcPtyTransport } = await import('./pty-transport') + const spawn = window.api.pty.spawn as unknown as ReturnType + spawn.mockResolvedValueOnce({ id: 'pty-1', agentResumeUnavailable: true }) + + const freshTransport = createIpcPtyTransport({}) + await expect(freshTransport.connect({ url: '', callbacks: {} })).resolves.toEqual({ + id: 'pty-1', + agentResumeUnavailable: true + }) + freshTransport.disconnect() + + spawn.mockResolvedValueOnce({ + id: 'pty-2', + coldRestore: { scrollback: 'recovered', cwd: '/repo/app' }, + agentResumeUnavailable: true + }) + const coldTransport = createIpcPtyTransport({}) + await expect(coldTransport.connect({ url: '', callbacks: {} })).resolves.toEqual( + expect.objectContaining({ id: 'pty-2', agentResumeUnavailable: true }) + ) + coldTransport.disconnect() + }) + it('defers title side effects until after terminal data is delivered', async () => { const { createIpcPtyTransport } = await import('./pty-transport') const onTitleChange = vi.fn() diff --git a/src/renderer/src/components/terminal-pane/pty-transport.ts b/src/renderer/src/components/terminal-pane/pty-transport.ts index 2fded96d677..1e3054f7050 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.ts @@ -804,17 +804,26 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra sessionExpired: spawnResult.sessionExpired, coldRestore: spawnResult.coldRestore, replay: spawnResult.replay, - pendingEscapeTailAnsi: spawnResult.pendingEscapeTailAnsi + pendingEscapeTailAnsi: spawnResult.pendingEscapeTailAnsi, + // Why: the cold-restore path re-runs the launch command, so it needs the + // same "main declined the resume" signal the fresh-spawn path gets. + ...(spawnResult.agentResumeUnavailable ? { agentResumeUnavailable: true as const } : {}) } satisfies PtyConnectResult } - if (resultLaunchAgent || spawnResult.launchConfig || spawnResult.startupCwdFallback) { + if ( + resultLaunchAgent || + spawnResult.launchConfig || + spawnResult.startupCwdFallback || + spawnResult.agentResumeUnavailable + ) { return { id: spawnResult.id, ...(resultLaunchAgent ? { launchAgent: resultLaunchAgent } : {}), ...(spawnResult.launchConfig ? { launchConfig: spawnResult.launchConfig } : {}), ...(spawnResult.startupCwdFallback ? { startupCwdFallback: spawnResult.startupCwdFallback } - : {}) + : {}), + ...(spawnResult.agentResumeUnavailable ? { agentResumeUnavailable: true as const } : {}) } satisfies PtyConnectResult } return spawnResult.id diff --git a/src/renderer/src/components/terminal-pane/session-restored-banner-pane-state.test.tsx b/src/renderer/src/components/terminal-pane/session-restored-banner-pane-state.test.tsx index da560dc16fb..880bbd2c098 100644 --- a/src/renderer/src/components/terminal-pane/session-restored-banner-pane-state.test.tsx +++ b/src/renderer/src/components/terminal-pane/session-restored-banner-pane-state.test.tsx @@ -3,7 +3,10 @@ import { act } from 'react' import { createRoot, type Root } from 'react-dom/client' import { afterEach, describe, expect, it } from 'vitest' -import { SESSION_RESTORED_BANNER_TEXT } from './SessionRestoredBanner' +import { + SESSION_RESTORED_BANNER_TEXT, + SESSION_RESUME_UNAVAILABLE_BANNER_TEXT +} from './SessionRestoredBanner' import { SessionRestoredBannerPortals } from './SessionRestoredBannerPortals' import { addSessionRestoredBannerPaneId, @@ -12,7 +15,8 @@ import { removeSessionRestoredBannerPaneId, seedStartupSessionRestoredBanner, syncSessionRestoredBannerTitleSpace, - type SessionRestoredBannerPane + type SessionRestoredBannerPane, + type SessionRestoredBannerPaneReasons } from './session-restored-banner-pane-state' const mountedRoots: Root[] = [] @@ -27,7 +31,7 @@ function createPane(id: number): SessionRestoredBannerPane { async function renderPortals( panes: readonly SessionRestoredBannerPane[], - paneIds: ReadonlySet + paneIds: SessionRestoredBannerPaneReasons ): Promise { const rootContainer = document.createElement('div') document.body.appendChild(rootContainer) @@ -43,6 +47,10 @@ function eventFrom(target: HTMLElement, event: KeyboardEvent | PointerEvent): ty return event } +function bannerReasons(paneIds: readonly number[]): SessionRestoredBannerPaneReasons { + return new Map(paneIds.map((paneId) => [paneId, 'restored' as const])) +} + function paneText(pane: SessionRestoredBannerPane): string { return pane.container.textContent ?? '' } @@ -60,7 +68,7 @@ describe('session restored banner pane state', () => { it('seeds sidebar startup onto the created pane and renders its overlay there', async () => { const firstPane = createPane(1) const createdPane = createPane(2) - let paneIds = new Set() + let paneIds: SessionRestoredBannerPaneReasons = new Map() seedStartupSessionRestoredBanner( { showSessionRestoredBanner: true }, @@ -71,7 +79,7 @@ describe('session restored banner pane state', () => { ) await renderPortals([firstPane, createdPane], paneIds) - expect(paneIds).toEqual(new Set([createdPane.id])) + expect(paneIds).toEqual(new Map([[createdPane.id, 'restored']])) expect(paneText(firstPane)).toBe('') expect(paneText(createdPane)).toBe(SESSION_RESTORED_BANNER_TEXT) }) @@ -84,7 +92,7 @@ describe('session restored banner pane state', () => { panes: [activePane, secondPane], paneTitles: {}, renamingPaneId: null, - sessionRestoredBannerPaneIds: new Set() + sessionRestoredBannerPaneIds: new Map() }) expect(needsFit).toBe(false) @@ -100,7 +108,7 @@ describe('session restored banner pane state', () => { panes: [titledPane, renamingPane], paneTitles: { [titledPane.id]: 'server' }, renamingPaneId: renamingPane.id, - sessionRestoredBannerPaneIds: new Set() + sessionRestoredBannerPaneIds: new Map() }) expect(needsFit).toBe(true) @@ -111,7 +119,7 @@ describe('session restored banner pane state', () => { it('renders and reserves title space only on the restored inactive split pane', async () => { const activePane = createPane(1) const inactiveRestoredPane = createPane(2) - const paneIds = new Set([inactiveRestoredPane.id]) + const paneIds = new Map([[inactiveRestoredPane.id, 'restored']]) const needsFit = syncSessionRestoredBannerTitleSpace({ panes: [activePane, inactiveRestoredPane], @@ -128,6 +136,22 @@ describe('session restored banner pane state', () => { expect(paneText(inactiveRestoredPane)).toBe(SESSION_RESTORED_BANNER_TEXT) }) + it('names a fresh session when the requested resume could not be verified', async () => { + const restoredPane = createPane(1) + const freshPane = createPane(2) + + await renderPortals( + [restoredPane, freshPane], + new Map([ + [restoredPane.id, 'restored' as const], + [freshPane.id, 'resume-unavailable' as const] + ]) + ) + + expect(paneText(restoredPane)).toBe(SESSION_RESTORED_BANNER_TEXT) + expect(paneText(freshPane)).toBe(SESSION_RESUME_UNAVAILABLE_BANNER_TEXT) + }) + it('dismisses only the interacted pane for pointer and key events', () => { const firstPane = createPane(1) const secondPane = createPane(2) @@ -137,18 +161,18 @@ describe('session restored banner pane state', () => { secondPane.container.appendChild(secondChild) const afterPointer = dismissSessionRestoredBannerPaneIds( - new Set([firstPane.id, secondPane.id]), + bannerReasons([firstPane.id, secondPane.id]), eventFrom(secondChild, new PointerEvent('pointerdown', { bubbles: true })), [firstPane, secondPane] ) const afterKey = dismissSessionRestoredBannerPaneIds( - new Set([firstPane.id, secondPane.id]), + bannerReasons([firstPane.id, secondPane.id]), eventFrom(firstChild, new KeyboardEvent('keydown', { bubbles: true })), [firstPane, secondPane] ) - expect(afterPointer).toEqual(new Set([firstPane.id])) - expect(afterKey).toEqual(new Set([secondPane.id])) + expect(afterPointer).toEqual(bannerReasons([firstPane.id])) + expect(afterKey).toEqual(bannerReasons([secondPane.id])) }) it('clears all restored banners when dismissal cannot resolve a pane', () => { @@ -158,23 +182,23 @@ describe('session restored banner pane state', () => { document.body.appendChild(outside) const afterDismiss = dismissSessionRestoredBannerPaneIds( - new Set([firstPane.id, secondPane.id]), + bannerReasons([firstPane.id, secondPane.id]), eventFrom(outside, new PointerEvent('pointerdown', { bubbles: true })), [firstPane, secondPane] ) - expect(afterDismiss).toEqual(new Set()) + expect(afterDismiss).toEqual(new Map()) }) it('clears banners for closed or removed panes', () => { const firstPane = createPane(1) const secondPane = createPane(2) - expect(removeSessionRestoredBannerPaneId(new Set([firstPane.id, secondPane.id]), 2)).toEqual( - new Set([firstPane.id]) - ) expect( - pruneSessionRestoredBannerPaneIds(new Set([firstPane.id, secondPane.id]), [firstPane]) - ).toEqual(new Set([firstPane.id])) + removeSessionRestoredBannerPaneId(bannerReasons([firstPane.id, secondPane.id]), 2) + ).toEqual(bannerReasons([firstPane.id])) + expect( + pruneSessionRestoredBannerPaneIds(bannerReasons([firstPane.id, secondPane.id]), [firstPane]) + ).toEqual(bannerReasons([firstPane.id])) }) }) diff --git a/src/renderer/src/components/terminal-pane/session-restored-banner-pane-state.ts b/src/renderer/src/components/terminal-pane/session-restored-banner-pane-state.ts index 03fd87ad600..64d2dbfd9fb 100644 --- a/src/renderer/src/components/terminal-pane/session-restored-banner-pane-state.ts +++ b/src/renderer/src/components/terminal-pane/session-restored-banner-pane-state.ts @@ -2,6 +2,12 @@ import type { ManagedPane } from '@/lib/pane-manager/pane-manager' export type SessionRestoredBannerPane = Pick +/** `resume-unavailable`: the pane asked to resume a provider session Orca could not + * verify, so it launched a fresh one — silence would read as a successful restore. */ +export type SessionRestoredBannerReason = 'restored' | 'resume-unavailable' + +export type SessionRestoredBannerPaneReasons = ReadonlyMap + export type SessionRestoredBannerStartup = | { showSessionRestoredBanner?: boolean @@ -12,36 +18,37 @@ export type SessionRestoredBannerStartup = export type SessionRestoredBannerDismissEvent = KeyboardEvent | PointerEvent export function addSessionRestoredBannerPaneId( - paneIds: ReadonlySet, - paneId: number -): Set { - if (paneIds.has(paneId)) { - return paneIds instanceof Set ? paneIds : new Set(paneIds) + paneReasons: SessionRestoredBannerPaneReasons, + paneId: number, + reason: SessionRestoredBannerReason = 'restored' +): Map { + if (paneReasons.get(paneId) === reason) { + return paneReasons instanceof Map ? paneReasons : new Map(paneReasons) } - return new Set(paneIds).add(paneId) + return new Map(paneReasons).set(paneId, reason) } export function removeSessionRestoredBannerPaneId( - paneIds: ReadonlySet, + paneReasons: SessionRestoredBannerPaneReasons, paneId: number -): Set { - if (!paneIds.has(paneId)) { - return paneIds instanceof Set ? paneIds : new Set(paneIds) +): Map { + if (!paneReasons.has(paneId)) { + return paneReasons instanceof Map ? paneReasons : new Map(paneReasons) } - const next = new Set(paneIds) + const next = new Map(paneReasons) next.delete(paneId) return next } export function pruneSessionRestoredBannerPaneIds( - paneIds: ReadonlySet, + paneReasons: SessionRestoredBannerPaneReasons, panes: readonly SessionRestoredBannerPane[] -): Set { +): Map { const livePaneIds = new Set(panes.map((pane) => pane.id)) - if ([...paneIds].every((paneId) => livePaneIds.has(paneId))) { - return paneIds instanceof Set ? paneIds : new Set(paneIds) + if ([...paneReasons.keys()].every((paneId) => livePaneIds.has(paneId))) { + return paneReasons instanceof Map ? paneReasons : new Map(paneReasons) } - return new Set([...paneIds].filter((paneId) => livePaneIds.has(paneId))) + return new Map([...paneReasons].filter(([paneId]) => livePaneIds.has(paneId))) } export function getSessionRestoredBannerDismissPaneId( @@ -62,21 +69,21 @@ export function getSessionRestoredBannerDismissPaneId( } export function dismissSessionRestoredBannerPaneIds( - paneIds: ReadonlySet, + paneReasons: SessionRestoredBannerPaneReasons, event: SessionRestoredBannerDismissEvent, panes: readonly SessionRestoredBannerPane[] -): Set { +): Map { const paneId = getSessionRestoredBannerDismissPaneId(event, panes) if (paneId === null) { - return new Set() + return new Map() } - return removeSessionRestoredBannerPaneId(paneIds, paneId) + return removeSessionRestoredBannerPaneId(paneReasons, paneId) } export function seedStartupSessionRestoredBanner( startup: SessionRestoredBannerStartup, paneId: number, - onShowSessionRestoredBanner: (paneId: number) => void + onShowSessionRestoredBanner: (paneId: number, reason?: SessionRestoredBannerReason) => void ): void { if (startup?.showSessionRestoredBanner === true) { onShowSessionRestoredBanner(paneId) @@ -87,7 +94,7 @@ export function syncSessionRestoredBannerTitleSpace(args: { panes: readonly SessionRestoredBannerPane[] paneTitles: Readonly> renamingPaneId: number | null - sessionRestoredBannerPaneIds: ReadonlySet + sessionRestoredBannerPaneIds: SessionRestoredBannerPaneReasons }): boolean { let needsFit = false for (const pane of args.panes) { diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts index 7650e870b09..dd1710dd3d2 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts @@ -131,7 +131,10 @@ import { import { acquireWebviewsDragPassthrough } from '../browser-pane/webview-registry' import { recordCreatedTerminalPaneSplit } from './terminal-pane-split-completion' import { closeTerminalTab } from '../terminal/terminal-tab-actions' -import { seedStartupSessionRestoredBanner } from './session-restored-banner-pane-state' +import { + seedStartupSessionRestoredBanner, + type SessionRestoredBannerReason +} from './session-restored-banner-pane-state' import { resolveTabTitleAfterPaneClose, shouldClearLaunchAgentForClosedPane @@ -281,7 +284,7 @@ type UseTerminalPaneLifecycleDeps = { clearWorktreeUnread: (worktreeId: string) => void clearTerminalTabUnread: (tabId: string) => void clearTerminalPaneUnread: (paneKey: string) => void - onShowSessionRestoredBanner: (paneId: number) => void + onShowSessionRestoredBanner: (paneId: number, reason?: SessionRestoredBannerReason) => void dispatchNotification: (event: { source: 'terminal-bell' | 'agent-task-complete' terminalTitle?: string diff --git a/src/shared/agent-resume-argv-drop.test.ts b/src/shared/agent-resume-argv-drop.test.ts new file mode 100644 index 00000000000..4dc71f4932e --- /dev/null +++ b/src/shared/agent-resume-argv-drop.test.ts @@ -0,0 +1,109 @@ +import { describe, expect, it } from 'vitest' +import { dropAgentResumeArgvFromCommand } from './agent-resume-argv-drop' +import { buildAgentResumeStartupPlan } from './tui-agent-startup' + +const CODEX_SESSION = { + key: 'session_id' as const, + id: '0199aa11-2b3c-4d5e-8f90-a1b2c3d4e5f6' +} + +function planFor(platform: NodeJS.Platform, agentArgs?: string): string { + const plan = buildAgentResumeStartupPlan({ + agent: 'codex', + providerSession: CODEX_SESSION, + cmdOverrides: {}, + platform, + ...(agentArgs ? { agentArgs } : {}) + }) + if (!plan) { + throw new Error('expected a codex resume plan') + } + return plan.launchCommand +} + +describe('dropAgentResumeArgvFromCommand', () => { + it('drops the resume argv the shared builder appended on every platform', () => { + for (const platform of ['darwin', 'linux', 'win32'] as const) { + const command = planFor(platform) + expect(command).toContain(CODEX_SESSION.id) + expect( + dropAgentResumeArgvFromCommand({ + command, + agent: 'codex', + providerSession: CODEX_SESSION + }) + ).toEqual({ status: 'dropped', command: 'codex' }) + } + }) + + it('keeps the user CLI args that precede the resume argv', () => { + expect( + dropAgentResumeArgvFromCommand({ + command: planFor('darwin', '--search'), + agent: 'codex', + providerSession: CODEX_SESSION + }) + ).toEqual({ status: 'dropped', command: "codex '--search'" }) + }) + + it('drops an unquoted resume argv from a legacy persisted command', () => { + expect( + dropAgentResumeArgvFromCommand({ + command: `codex resume ${CODEX_SESSION.id}`, + agent: 'codex', + providerSession: CODEX_SESSION + }) + ).toEqual({ status: 'dropped', command: 'codex' }) + }) + + it('reports absent when the command never carried the resume locator', () => { + expect( + dropAgentResumeArgvFromCommand({ + command: 'codex --search', + agent: 'codex', + providerSession: CODEX_SESSION + }) + ).toEqual({ status: 'absent' }) + }) + + it('refuses a command whose locator is not the trailing argv', () => { + // Why: the caller must fail the spawn rather than launch a resume it could not cancel. + expect( + dropAgentResumeArgvFromCommand({ + command: `codex resume ${CODEX_SESSION.id} --search`, + agent: 'codex', + providerSession: CODEX_SESSION + }) + ).toEqual({ status: 'unrecognized' }) + }) + + it('requires a whitespace boundary so a longer subcommand cannot match', () => { + expect( + dropAgentResumeArgvFromCommand({ + command: `codex xresume ${CODEX_SESSION.id}`, + agent: 'codex', + providerSession: CODEX_SESSION + }) + ).toEqual({ status: 'unrecognized' }) + }) + + it('does not strip a bare resume argv with no launch command in front of it', () => { + expect( + dropAgentResumeArgvFromCommand({ + command: `resume ${CODEX_SESSION.id}`, + agent: 'codex', + providerSession: CODEX_SESSION + }) + ).toEqual({ status: 'unrecognized' }) + }) + + it('drops the resume argv for the other resumable agents too', () => { + expect( + dropAgentResumeArgvFromCommand({ + command: `claude '--resume' '${CODEX_SESSION.id}'`, + agent: 'claude', + providerSession: CODEX_SESSION + }) + ).toEqual({ status: 'dropped', command: 'claude' }) + }) +}) diff --git a/src/shared/agent-resume-argv-drop.ts b/src/shared/agent-resume-argv-drop.ts new file mode 100644 index 00000000000..03646e83812 --- /dev/null +++ b/src/shared/agent-resume-argv-drop.ts @@ -0,0 +1,61 @@ +import { + getAgentResumeArgv, + type AgentProviderSessionMetadata, + type ResumableTuiAgent +} from './agent-session-resume' +import { quoteStartupArg, type AgentStartupShell } from './tui-agent-startup-shell' + +/** `dropped` carries the plain launch command; `unrecognized` means the resume + * locator is still in there and the caller must refuse the launch rather than + * run a resume it could not cancel. */ +export type AgentResumeArgvDrop = + | { status: 'dropped'; command: string } + | { status: 'absent' } + | { status: 'unrecognized' } + +const RESUME_ARGV_SHELLS: readonly AgentStartupShell[] = ['posix', 'powershell', 'cmd'] + +function resumeArgvSuffixCandidates(resumeArgs: readonly string[]): string[] { + const candidates = new Set([resumeArgs.join(' ')]) + for (const shell of RESUME_ARGV_SHELLS) { + candidates.add(resumeArgs.map((arg) => quoteStartupArg(arg, shell)).join(' ')) + } + return [...candidates] +} + +function stripSuffix(command: string, suffix: string): string | null { + const base = command.slice(0, command.length - suffix.length) + // Why: require a whitespace boundary so `codex resume-foo` can't match `resume`. + if (!command.endsWith(suffix) || base.length === 0 || !/\s$/.test(base)) { + return null + } + const trimmed = base.trimEnd() + return trimmed.length > 0 ? trimmed : null +} + +/** + * Remove the resume argv `buildAgentResumeStartupPlan` appended, leaving the plain + * agent launch. Main uses this when it cannot verify which account owns the session, + * so the pane starts fresh instead of resuming under whichever account is selected. + */ +export function dropAgentResumeArgvFromCommand(args: { + command: string + agent: ResumableTuiAgent + providerSession: AgentProviderSessionMetadata + ompResumeFilePath?: string | null +}): AgentResumeArgvDrop { + const argv = getAgentResumeArgv(args.agent, args.providerSession, args.ompResumeFilePath) + const resumeArgs = argv?.slice(1) ?? [] + const locator = resumeArgs.at(-1) + const command = args.command.trimEnd() + if (!locator || !command.includes(locator)) { + return { status: 'absent' } + } + for (const suffix of resumeArgvSuffixCandidates(resumeArgs)) { + const base = stripSuffix(command, suffix) + if (base !== null && !base.includes(locator)) { + return { status: 'dropped', command: base } + } + } + return { status: 'unrecognized' } +}