diff --git a/src/main/codex/codex-session-resume-preparation.test.ts b/src/main/codex/codex-session-resume-preparation.test.ts new file mode 100644 index 00000000000..7eaf7add123 --- /dev/null +++ b/src/main/codex/codex-session-resume-preparation.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, it, vi } from 'vitest' +import { prepareCodexSessionResume } from './codex-session-resume-preparation' + +const SESSION_ID = '019f81b9-19a9-7651-a8d1-352d9420bd11' +const ORIGIN_HOME = '/managed/origin/home' +const ORIGIN_ROLLOUT = `${ORIGIN_HOME}/sessions/2026/07/20/rollout-2026-07-20T12-00-00-${SESSION_ID}.jsonl` + +function prepare(args: { + transcriptPath: string | undefined + trustedCodexHomes: readonly string[] + resolveVerifiedResumeHome?: (source: { homePath: string }) => Promise +}) { + return prepareCodexSessionResume({ + sessionId: SESSION_ID, + transcriptPath: args.transcriptPath, + trustedCodexHomes: args.trustedCodexHomes, + fileIsRegular: () => true, + resolveVerifiedResumeHome: args.resolveVerifiedResumeHome ?? (async (source) => source.homePath) + }) +} + +describe('prepareCodexSessionResume', () => { + it('pins the verified origin home the caller resolved', async () => { + const resolveVerifiedResumeHome = vi.fn(async () => '/managed/migrated/home') + + await expect( + prepare({ + transcriptPath: ORIGIN_ROLLOUT, + trustedCodexHomes: [ORIGIN_HOME], + resolveVerifiedResumeHome + }) + ).resolves.toEqual({ outcome: 'resume', codexHomePath: '/managed/migrated/home' }) + expect(resolveVerifiedResumeHome).toHaveBeenCalledWith({ + homePath: ORIGIN_HOME, + transcriptPath: ORIGIN_ROLLOUT + }) + }) + + it('falls back to a fresh session when the rollout home is not trusted', async () => { + // Why: returning `resume` here is exactly the #10793 bug — the pane would resume + // under whichever account is selected now. + const resolveVerifiedResumeHome = vi.fn(async (source: { homePath: string }) => source.homePath) + + await expect( + prepare({ + transcriptPath: ORIGIN_ROLLOUT, + trustedCodexHomes: ['/managed/other/home'], + resolveVerifiedResumeHome + }) + ).resolves.toEqual({ outcome: 'fresh', claimedCodexProvenance: true }) + // Migration, project trust and hook repair must not run without a verified home. + expect(resolveVerifiedResumeHome).not.toHaveBeenCalled() + }) + + it('marks cross-agent metadata as never having claimed Codex provenance', async () => { + await expect( + prepare({ + transcriptPath: '/Users/example/.claude/projects/repo/x.jsonl', + trustedCodexHomes: [ORIGIN_HOME] + }) + ).resolves.toEqual({ outcome: 'fresh', claimedCodexProvenance: false }) + }) + + it('marks a resume with no transcript path as unclaimed but still fresh', async () => { + await expect( + prepare({ transcriptPath: undefined, trustedCodexHomes: [ORIGIN_HOME] }) + ).resolves.toEqual({ outcome: 'fresh', claimedCodexProvenance: false }) + }) +}) diff --git a/src/main/codex/codex-session-resume-preparation.ts b/src/main/codex/codex-session-resume-preparation.ts new file mode 100644 index 00000000000..0069343c455 --- /dev/null +++ b/src/main/codex/codex-session-resume-preparation.ts @@ -0,0 +1,39 @@ +import { + resolveCodexSessionResumeProvenance, + type CodexSessionResumePreparation +} from './codex-session-resume-home' + +export type VerifiedCodexResumeSource = { homePath: string; transcriptPath: string } + +/** + * The launch decision for an automatic Codex resume: pin the verified origin home, or + * fall back to a fresh session whose resume argv the PTY layer drops (#10793). The + * caller supplies `resolveVerifiedResumeHome` because migration, project trust and hook + * repair only run once provenance is verified — a fresh outcome must skip all of them. + */ +export async function prepareCodexSessionResume(args: { + sessionId: string + transcriptPath: string | undefined + trustedCodexHomes: readonly string[] + resolveVerifiedResumeHome: (source: VerifiedCodexResumeSource) => Promise + fileIsRegular?: (filePath: string) => boolean + listSessionFiles?: (sessionsRoot: string) => AsyncIterable +}): Promise { + const provenance = await resolveCodexSessionResumeProvenance({ + sessionId: args.sessionId, + transcriptPath: args.transcriptPath, + trustedCodexHomes: args.trustedCodexHomes, + ...(args.fileIsRegular ? { fileIsRegular: args.fileIsRegular } : {}), + ...(args.listSessionFiles ? { listSessionFiles: args.listSessionFiles } : {}) + }) + if (provenance.outcome === 'fresh') { + return provenance + } + return { + outcome: 'resume', + codexHomePath: await args.resolveVerifiedResumeHome({ + homePath: provenance.homePath, + transcriptPath: provenance.transcriptPath + }) + } +} diff --git a/src/main/codex/codex-unverified-resume-launch.ts b/src/main/codex/codex-unverified-resume-launch.ts index 746b5a759ff..f90c222c657 100644 --- a/src/main/codex/codex-unverified-resume-launch.ts +++ b/src/main/codex/codex-unverified-resume-launch.ts @@ -7,12 +7,15 @@ export const UNVERIFIED_CODEX_RESUME_ERROR = /** * 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. + * under whichever account is selected now (#10793). Throws only when metadata claimed + * real Codex layout and the locator is present but not strippable — launching then + * could still cross accounts. Metadata that never claimed Codex provenance (e.g. + * `~/.claude/…` on a pane mislabeled "codex") launches unchanged, as it did before. */ export function dropUnverifiedCodexResumeArgv(args: { command: string | undefined providerSession: AgentProviderSessionMetadata | null + claimedCodexProvenance: boolean }): { command: string | undefined; droppedResumeArgv: boolean } { if (!args.command || !args.providerSession) { return { command: args.command, droppedResumeArgv: false } @@ -25,7 +28,7 @@ export function dropUnverifiedCodexResumeArgv(args: { if (drop.status === 'dropped') { return { command: drop.command, droppedResumeArgv: true } } - if (drop.status === 'unrecognized') { + if (drop.status === 'unrecognized' && args.claimedCodexProvenance) { 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 5c885e8771b..fad6ea73686 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -172,10 +172,8 @@ import { startCodexSessionIndexHealInBackground } from './codex/codex-session-in import { createCodexSessionMigrationScheduler } from './codex/codex-session-migration-scheduler' import { prepareLegacySharedCodexSessionResume } from './codex/codex-legacy-session-resume' import { resolveHostCodexSessionSourceHome } from './codex/codex-session-source-home' -import { - resolveCodexSessionResumeProvenance, - type CodexSessionResumePreparation -} from './codex/codex-session-resume-home' +import type { CodexSessionResumePreparation } from './codex/codex-session-resume-home' +import { prepareCodexSessionResume } from './codex/codex-session-resume-preparation' import { getSystemCodexHomePath } from './codex/codex-home-paths' import { normalizeRuntimePathForComparison } from '../shared/cross-platform-path' import type { AgentProviderSessionMetadata } from '../shared/agent-session-resume' @@ -849,64 +847,63 @@ async function prepareCodexSessionResumeForLaunch(args: { systemHomePath, ...codexRuntimeHome.getHostCodexHomePathsForSessionDiscovery() ] - const sessionSource = await resolveCodexSessionResumeProvenance({ + const settingsStore = store + // Why: a `fresh` outcome must skip migration, trust and hook repair entirely — there is + // no verified origin home to prepare, so the PTY layer drops the resume argv (#10793). + return prepareCodexSessionResume({ sessionId: args.providerSession.id, transcriptPath: args.providerSession.transcriptPath, - trustedCodexHomes: trustedHomes - }) - 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 } - try { - migrated = await prepareLegacySharedCodexSessionResume( - { - agent: 'codex', - executionHostId: 'local', - filePath: sessionSource.transcriptPath, - codexHome: sessionSource.homePath - }, - { - isHostSystemDefaultRealHome: () => codexRuntimeHome!.isHostSystemDefaultRealHome(), - systemCodexHomePath: systemHomePath + trustedCodexHomes: trustedHomes, + resolveVerifiedResumeHome: async (sessionSource) => { + let migrated = { useRealCodexHome: false } + try { + migrated = await prepareLegacySharedCodexSessionResume( + { + agent: 'codex', + executionHostId: 'local', + filePath: sessionSource.transcriptPath, + codexHome: sessionSource.homePath + }, + { + isHostSystemDefaultRealHome: () => codexRuntimeHome!.isHostSystemDefaultRealHome(), + systemCodexHomePath: systemHomePath + } + ) + } catch (error) { + // Why: migration is a compatibility repair; its failure must not prevent the PTY from resuming from its trusted origin home. + console.warn( + '[codex-session-resume] Legacy rollout migration failed; using origin home:', + error + ) } - ) - } catch (error) { - // Why: migration is a compatibility repair; its failure must not prevent the PTY from resuming from its trusted origin home. - console.warn( - '[codex-session-resume] Legacy rollout migration failed; using origin home:', - error - ) - } - const resumeHome = migrated.useRealCodexHome ? systemHomePath : sessionSource.homePath + const resumeHome = migrated.useRealCodexHome ? systemHomePath : sessionSource.homePath - if (args.workspacePath) { - try { - markCodexProjectTrusted(args.workspacePath) - } catch (error) { - console.warn('[codex-project-trust] failed to pre-mark resumed workspace:', error) + if (args.workspacePath) { + try { + markCodexProjectTrusted(args.workspacePath) + } catch (error) { + console.warn('[codex-project-trust] failed to pre-mark resumed workspace:', error) + } + } + const isSystemHome = + normalizeRuntimePathForComparison(resumeHome) === + normalizeRuntimePathForComparison(systemHomePath) + const hooksEnabled = isAgentStatusHooksEnabled(settingsStore.getSettings()) + try { + if (isSystemHome) { + ensureRealHomeCodexHookState({ hooksEnabled, userDataPath: app.getPath('userData') }) + } else if (hooksEnabled) { + codexHookService.install(resumeHome) + } else { + codexHookService.refreshRuntimeUserHooks(resumeHome) + } + } catch (error) { + // 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 resumeHome } - } - const isSystemHome = - normalizeRuntimePathForComparison(resumeHome) === - normalizeRuntimePathForComparison(systemHomePath) - const hooksEnabled = isAgentStatusHooksEnabled(store.getSettings()) - try { - if (isSystemHome) { - ensureRealHomeCodexHookState({ hooksEnabled, userDataPath: app.getPath('userData') }) - } else if (hooksEnabled) { - codexHookService.install(resumeHome) - } else { - codexHookService.refreshRuntimeUserHooks(resumeHome) - } - } catch (error) { - // 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 { 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 b04566a209f..39752745d4a 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -2,7 +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 { prepareCodexSessionResume } from '../codex/codex-session-resume-preparation' import { TERMINAL_INPUT_CHUNK_MAX_BYTES, TERMINAL_INPUT_MAX_BYTES @@ -215,7 +215,8 @@ import { unregisterSshPtyProvider, getLocalPtyProvider, isCurrentPtyExit, - restorePtyIncarnation + restorePtyIncarnation, + type PrepareCodexSessionResume } from './pty' import { _resetLocalPtyProviderStateForTest } from '../providers/local-pty-provider' import { resetMacosLoginShellPreflightForTests } from '../providers/macos-tcc-login-shell' @@ -1852,10 +1853,21 @@ describe('registerPtyHandlers', () => { 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 + // Why: main's real provenance rule via the same prepareCodexSessionResume that + // index.ts calls, so the outcome wiring is exercised rather than restated. 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. + const prepareResumeWithTrustedHomes = + (trustedHomes: readonly string[]): PrepareCodexSessionResume => + ({ providerSession }) => + prepareCodexSessionResume({ + sessionId: providerSession.id, + transcriptPath: providerSession.transcriptPath, + trustedCodexHomes: trustedHomes, + fileIsRegular: () => true, + resolveVerifiedResumeHome: async (source) => source.homePath + }) + function registerWithTrustedHomes(trustedHomes: readonly string[], selectedHome: string) { const selectedHomeMock = vi.fn(() => selectedHome) registerPtyHandlers( @@ -1865,31 +1877,21 @@ describe('registerPtyHandlers', () => { 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 - } - } + { prepareCodexSessionResume: prepareResumeWithTrustedHomes(trustedHomes) } ) return selectedHomeMock } async function spawnCodexResume( - transcriptPath: string | undefined + transcriptPath: string | undefined, + overrides: { command?: string; env?: Record } = {} ): Promise<{ agentResumeUnavailable?: true }> { return (await handlers.get('pty:spawn')!(null, { cols: 80, rows: 24, cwd: '/tmp', - command: `codex 'resume' '${RESUME_SESSION_ID}'`, + command: overrides.command ?? `codex 'resume' '${RESUME_SESSION_ID}'`, + ...(overrides.env ? { env: overrides.env } : {}), launchAgent: 'codex', resumeProviderSession: { key: 'session_id', @@ -1899,6 +1901,36 @@ describe('registerPtyHandlers', () => { })) as { agentResumeUnavailable?: true } } + /** A daemon-shaped provider: the only local path that reports reattach results and + * the only one that surfaces the resolved spawn options this guard rewrites. */ + function setupResumeDaemonProvider(spawnResult: { isReattach?: true } = {}) { + const daemonSpawn = vi.fn( + async (options: { + sessionId?: string + command?: string + env: Record + }) => { + void options + return { id: options.sessionId ?? 'daemon-pty', ...spawnResult } + } + ) + setLocalPtyProvider({ + spawn: daemonSpawn, + supportsGitCredentialGuardHost: () => true, + supportsAgentSessionClaims: () => true, + supportsAgentSessionCreateOperations: () => true, + write: vi.fn(), + resize: vi.fn(), + kill: vi.fn(), + shutdown: vi.fn(), + onData: vi.fn(() => vi.fn()), + onExit: vi.fn(() => vi.fn()), + listProcesses: vi.fn(async () => []), + getForegroundProcess: vi.fn(async () => null) + } as never) + return daemonSpawn + } + posixOnlyIt( 'launches plain codex when a REAL rollout sits under a home Orca no longer trusts', async () => { @@ -1974,6 +2006,150 @@ describe('registerPtyHandlers', () => { vi.useRealTimers() } }) + + posixOnlyIt('reports a dropped resume that carried no transcript path at all', async () => { + // Why: legacy sleeping-agent and relay records persist only the session id. The + // argv still gets stripped, so silence would leave an empty pane unexplained. + const mockProc = createMockProc() + spawnMock.mockReturnValue(mockProc.proc) + vi.useFakeTimers() + + try { + registerWithTrustedHomes([OTHER_HOME], OTHER_HOME) + const spawned = await spawnCodexResume(undefined) + + await Promise.resolve() + vi.runAllTimers() + await Promise.resolve() + vi.runAllTimers() + + expect(mockProc.proc.write).toHaveBeenCalledWith('codex\n') + expect(spawned.agentResumeUnavailable).toBe(true) + } finally { + vi.useRealTimers() + } + }) + + it('refuses an unstrippable resume whose metadata claimed Codex layout', async () => { + // Why: the locator survives in the command, so launching could still cross accounts. + registerWithTrustedHomes([OTHER_HOME], OTHER_HOME) + + await expect( + spawnCodexResume(ORIGIN_ROLLOUT, { + command: `codex 'resume' '${RESUME_SESSION_ID}' --sandbox` + }) + ).rejects.toThrow(/could not verify the originating Codex session file/) + }) + + posixOnlyIt( + 'launches an unstrippable resume unchanged when metadata never claimed Codex layout', + async () => { + // Why: a pane mislabeled "codex" carrying ~/.claude metadata launched fine before + // this guard existed; refusing its spawn would be a new hard failure. + const mockProc = createMockProc() + spawnMock.mockReturnValue(mockProc.proc) + vi.useFakeTimers() + + try { + registerWithTrustedHomes([OTHER_HOME], OTHER_HOME) + const command = `cd '/tmp/${RESUME_SESSION_ID}' && codex 'resume' '${RESUME_SESSION_ID}'` + const spawned = await spawnCodexResume('/Users/example/.claude/projects/repo/x.jsonl', { + command + }) + + await Promise.resolve() + vi.runAllTimers() + await Promise.resolve() + vi.runAllTimers() + + expect(mockProc.proc.write).toHaveBeenCalledWith(`${command}\n`) + expect(spawned.agentResumeUnavailable).toBeUndefined() + } finally { + vi.useRealTimers() + } + } + ) + + it('strips the resume argv from the sequenced startup command too', async () => { + // Why: buildPtyHostEnv prefers this env var over the launch command and the + // sequenced wrapper `eval`s it, so leaving it intact resumes under the wrong account. + const daemonSpawn = setupResumeDaemonProvider() + registerWithTrustedHomes([OTHER_HOME], OTHER_HOME) + + await spawnCodexResume(ORIGIN_ROLLOUT, { + env: { ORCA_SEQUENCED_STARTUP_COMMAND: `codex 'resume' '${RESUME_SESSION_ID}'` } + }) + + const spawnOptions = daemonSpawn.mock.calls.at(-1)![0] + expect(spawnOptions.env.ORCA_SEQUENCED_STARTUP_COMMAND).toBe('codex') + expect(spawnOptions.command).toBe('codex') + }) + + it('leaves the sequenced startup command alone when provenance is verified', async () => { + const daemonSpawn = setupResumeDaemonProvider() + registerWithTrustedHomes([ORIGIN_HOME], OTHER_HOME) + const sequenced = `codex 'resume' '${RESUME_SESSION_ID}'` + + await spawnCodexResume(ORIGIN_ROLLOUT, { + env: { ORCA_SEQUENCED_STARTUP_COMMAND: sequenced } + }) + + expect(daemonSpawn.mock.calls.at(-1)![0].env.ORCA_SEQUENCED_STARTUP_COMMAND).toBe(sequenced) + }) + + it('omits the notice on a reattach that never ran this launch command', async () => { + setupResumeDaemonProvider({ isReattach: true }) + registerWithTrustedHomes([OTHER_HOME], OTHER_HOME) + + const spawned = await spawnCodexResume(ORIGIN_ROLLOUT) + + expect(spawned.agentResumeUnavailable).toBeUndefined() + }) + + it('drops the resume argv on the runtime controller spawn path', async () => { + // Why: the runtime/relay controller is a second spawn entry point; the invariant + // has to hold there too even though it has no channel for the notice. + const daemonSpawn = setupResumeDaemonProvider() + const runtime = { + setPtyController: vi.fn(), + registerPty: vi.fn(), + noteTerminalSpawnCommand: vi.fn(), + onPtySpawned: vi.fn(), + onPtyExit: vi.fn(), + onPtyData: vi.fn() + } + handlers.clear() + registerPtyHandlers( + mainWindow as never, + runtime as never, + vi.fn(() => OTHER_HOME), + undefined, + undefined, + undefined, + { prepareCodexSessionResume: prepareResumeWithTrustedHomes([OTHER_HOME]) } + ) + const controller = runtime.setPtyController.mock.calls[0]?.[0] as { + spawn(args: Record): Promise<{ id: string }> + } + + await controller.spawn({ + cols: 80, + rows: 24, + command: `codex 'resume' '${RESUME_SESSION_ID}'`, + env: { ORCA_SEQUENCED_STARTUP_COMMAND: `codex 'resume' '${RESUME_SESSION_ID}'` }, + launchAgent: 'codex', + resumeProviderSession: { + key: 'session_id', + id: RESUME_SESSION_ID, + transcriptPath: ORIGIN_ROLLOUT + } + }) + + const spawnOptions = daemonSpawn.mock.calls.at(-1)![0] + expect(spawnOptions.command).toBe('codex') + expect(spawnOptions.env.ORCA_SEQUENCED_STARTUP_COMMAND).toBe('codex') + expect(runtime.noteTerminalSpawnCommand).toHaveBeenCalledWith(expect.any(String), 'codex') + }) }) it('prepares Codex launch state for the workspace before spawning an interactive tab', async () => { diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index db6d5e6e626..ffbe1b528cd 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -167,7 +167,11 @@ import type { CodexSessionResumePreparation } from '../codex/codex-session-resum 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' +import { + resolveSetupAgentSequenceLaunchCommand, + SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV +} from '../../shared/setup-agent-sequencing' +import { dropAgentResumeArgvFromCommand } from '../../shared/agent-resume-argv-drop' import { parseWorkspaceKey } from '../../shared/workspace-scope' import { getStartupTerminalColorQueryReplyColors } from './terminal-startup-color-query-replies' import { @@ -3154,6 +3158,8 @@ export function registerPtyHandlers( codexResumeHome: { codexHomePath: string } | null command: string | undefined notifyResumeUnavailable: boolean + droppedResumeArgv: boolean + providerSession: AgentProviderSessionMetadata | null } /** Kept separate from resolveCodexResumeLaunch so non-Codex spawns never await: @@ -3161,7 +3167,9 @@ export function registerPtyHandlers( const noCodexResumeLaunch = (command: string | undefined): CodexResumeLaunch => ({ codexResumeHome: null, command, - notifyResumeUnavailable: false + notifyResumeUnavailable: false, + droppedResumeArgv: false, + providerSession: null }) /** The command a Codex launch actually runs: unchanged when provenance is verified, @@ -3171,20 +3179,54 @@ export function registerPtyHandlers( preparation: NonNullable> ): Promise => preparation.preparation.then((prepared) => { + const providerSession = preparation.providerSession if (prepared?.outcome !== 'fresh') { - return { codexResumeHome: prepared ?? null, command, notifyResumeUnavailable: false } + return { + codexResumeHome: prepared ?? null, + command, + notifyResumeUnavailable: false, + droppedResumeArgv: false, + providerSession + } } const dropped = dropUnverifiedCodexResumeArgv({ command, - providerSession: preparation.providerSession + providerSession, + claimedCodexProvenance: prepared.claimedCodexProvenance }) return { codexResumeHome: null, command: dropped.command, - notifyResumeUnavailable: dropped.droppedResumeArgv && prepared.claimedCodexProvenance + // Why: staying silent only makes sense for metadata that positively belongs to + // another agent; a resume with no transcript path at all still owes the user a notice. + notifyResumeUnavailable: + dropped.droppedResumeArgv && + (prepared.claimedCodexProvenance || !providerSession.transcriptPath), + droppedResumeArgv: dropped.droppedResumeArgv, + providerSession } }) + /** Why: buildPtyHostEnv prefers ORCA_SEQUENCED_STARTUP_COMMAND over the launch command + * and the sequenced wrapper `eval`s it, so a dropped resume argv has to go there too. */ + const stripSequencedStartupResumeArgv = | undefined>( + env: T, + launch: CodexResumeLaunch + ): T => { + const sequenced = env?.[SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV] + if (!env || !sequenced || !launch.droppedResumeArgv || !launch.providerSession) { + return env + } + const drop = dropAgentResumeArgvFromCommand({ + command: sequenced, + agent: 'codex', + providerSession: launch.providerSession + }) + return drop.status === 'dropped' + ? { ...env, [SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV]: drop.command } + : env + } + // Why: route through getProviderForPty() so CLI commands work for remote PTYs too; localProvider would silently fail for them. runtime?.setPtyController({ spawn: async (args) => { @@ -3227,6 +3269,8 @@ export function registerPtyHandlers( ? await resolveCodexResumeLaunch(args.command, codexResumePreparation) : noCodexResumeLaunch(args.command) const codexResumeHome = codexResumeLaunch.codexResumeHome + // Why: the drop still applies here, but this controller's result has no field for + // notifyResumeUnavailable — runtime/relay panes start fresh without the notice. const launchCommand = codexResumeLaunch.command const claudeAuth = isClaudeLaunch && prepareClaudeAuth ? await prepareClaudeAuth(codexSelectionTarget) : null @@ -3296,6 +3340,7 @@ export function registerPtyHandlers( ? { ...sshScopedEnv, ...claudeAuth.envPatch } : sshScopedEnv const requestedAgentTeamsPath = env?.ORCA_AGENT_TEAMS_TEAM_ID ? env.PATH : undefined + env = stripSequencedStartupResumeArgv(env, codexResumeLaunch) if (args.preAllocatedHandle) { env = { ...env, ORCA_TERMINAL_HANDLE: args.preAllocatedHandle } } @@ -4452,6 +4497,7 @@ export function registerPtyHandlers( : noCodexResumeLaunch(args.command) const codexResumeHome = codexResumeLaunch.codexResumeHome const launchCommand = codexResumeLaunch.command + baseEnv = stripSequencedStartupResumeArgv(baseEnv, codexResumeLaunch) const selectedCodexHomePath = isDaemonHostSpawn ? getCompatibleSelectedCodexHomePath( codexSelectionTarget, diff --git a/src/renderer/src/assets/terminal.css b/src/renderer/src/assets/terminal.css index c7e11730266..ab45df2cc73 100644 --- a/src/renderer/src/assets/terminal.css +++ b/src/renderer/src/assets/terminal.css @@ -321,8 +321,14 @@ left: 8px; z-index: 5; height: var(--orca-pane-title-height); - display: flex; - align-items: center; + /* Why: block + line-height (not flex) so the single line can ellipsize like + .pane-title-text — an overflowing banner would wrap onto the terminal grid. */ + display: block; + line-height: var(--orca-pane-title-height); + max-width: calc(100% - 16px); + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; font-family: var(--font-mono); font-size: 12px; color: var(--orca-pane-title-fg); 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 8b152c96f45..ce43e690be2 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -8858,6 +8858,150 @@ describe('connectPanePty', () => { expect(mockStoreState.clearSleepingAgentSession).toHaveBeenCalledWith(paneKey) }) + it('says the fresh-spawn resume started fresh when main declined it', async () => { + // Why: the primary #10757 path — the sidebar resumes a sleeping agent into a new tab, + // so there is no restored PTY and the spawn is fresh, not a cold restore. + const { connectPanePty } = await import('./pty-connection') + const spawn = createDeferred<{ id: string; agentResumeUnavailable: true }>() + const transport = createMockTransport() + transport.connect.mockImplementation(() => spawn.promise) + transportFactoryQueue.push(transport) + const paneKey = makePaneKey('tab-1', LEAF_2) + mockStoreState = { + ...mockStoreState, + tabsByWorktree: { + 'wt-1': [{ id: 'tab-1', ptyId: null }] + }, + ptyIdsByTabId: { + 'tab-1': [] + }, + terminalLayoutsByTabId: { + 'tab-1': { + root: { type: 'leaf', leafId: LEAF_2 }, + activeLeafId: LEAF_2, + expandedLeafId: null, + ptyIdsByLeafId: {} + } + }, + 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(2) + const manager = createManager(2) + const deps = createDeps({ + restoredLeafId: LEAF_2, + restoredPtyIdByLeafId: {} + }) + + connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks(4) + + expect(transport.connect).toHaveBeenCalledWith( + expect.not.objectContaining({ sessionId: expect.any(String) }) + ) + + spawn.resolve({ id: 'fresh-pty', agentResumeUnavailable: true }) + await flushAsyncTicks(10) + + expect(deps.onShowSessionRestoredBanner).toHaveBeenCalledTimes(1) + expect(deps.onShowSessionRestoredBanner).toHaveBeenCalledWith(2, 'resume-unavailable') + }) + + it('lets a declined resume replace an already-shown restored banner', async () => { + // Why: the banner latch is one-shot per pane. A pane that showed "session restored" + // and then respawned into a session main declined must stop claiming the restore. + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport() + let connectCount = 0 + transport.connect.mockImplementation(async () => { + connectCount += 1 + // The wake respawn is the one main declines; the first spawn is an ordinary resume. + const spawnedPtyId = connectCount === 1 ? 'fresh-pty' : 'woken-pty' + transport.getPtyId.mockReturnValue(spawnedPtyId) + return connectCount === 1 + ? spawnedPtyId + : { id: spawnedPtyId, agentResumeUnavailable: true as const } + }) + transportFactoryQueue.push(transport) + const paneKey = makePaneKey('tab-1', LEAF_2) + const sleepingRecord = { + paneKey, + tabId: 'tab-1', + worktreeId: 'wt-1', + agent: 'codex' as const, + providerSession: { key: 'session_id' as const, id: 'codex-session-1' }, + prompt: 'finish the task', + state: 'done' as const, + capturedAt: 1, + updatedAt: 1, + origin: 'worktree-sleep' as const + } + mockStoreState = { + ...mockStoreState, + tabsByWorktree: { + 'wt-1': [{ id: 'tab-1', ptyId: null }] + }, + ptyIdsByTabId: { 'tab-1': [] }, + terminalLayoutsByTabId: { + 'tab-1': { + root: { type: 'leaf', leafId: LEAF_2 }, + activeLeafId: LEAF_2, + expandedLeafId: null, + ptyIdsByLeafId: {} + } + }, + settings: { ...mockStoreState.settings, agentCmdOverrides: {} }, + agentStatusByPaneKey: {}, + sleepingAgentSessionsByPaneKey: { [paneKey]: sleepingRecord }, + suppressedPtyExitIds: { 'fresh-pty': true } + } as StoreState + + const pane = createPane(2) + const manager = createManager(2) + const deps = createDeps({ + restoredLeafId: LEAF_2, + restoredPtyIdByLeafId: {}, + consumeSuppressedPtyExit: vi.fn(() => true), + isVisibleRef: { current: false } + }) + + const binding = connectPanePty(pane as never, manager as never, deps as never) as unknown as { + noteVisibilityResume: () => void + } + await flushAsyncTicks(10) + + expect(deps.onShowSessionRestoredBanner).toHaveBeenCalledWith(2, 'restored') + + // Hibernate the pane, then reveal it so the wake respawns the recorded session. + // Hibernation writes the sleeping record the first spawn consumed. + mockStoreState.sleepingAgentSessionsByPaneKey[paneKey] = sleepingRecord + const onPtyExit = createdTransportOptions[0]?.onPtyExit as ((ptyId: string) => void) | undefined + onPtyExit?.('fresh-pty') + await flushAsyncTicks(10) + binding.noteVisibilityResume() + await flushAsyncTicks(10) + + expect(connectCount).toBeGreaterThan(1) + expect(deps.onShowSessionRestoredBanner).toHaveBeenLastCalledWith(2, 'resume-unavailable') + }) + it('keeps sleeping resume record when fresh cold-restore spawn fails', async () => { const { connectPanePty } = await import('./pty-connection') const staleSessionId = 'wt-1@@stale-session' diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 77623c9e8cd..6d17ec03fc4 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -4502,12 +4502,17 @@ export function connectPanePty( if (ownsStartupDraftPaste && !connectionId && !shouldDeliverStartupViaTerminalPaste) { armStartupDraftReadinessObservation() } - let sessionRestoredBannerShown = false + let sessionRestoredBannerShown: SessionRestoredBannerReason | null = null const showSessionRestoredBanner = (reason: SessionRestoredBannerReason = 'restored'): void => { - if (sessionRestoredBannerShown) { + // Why: a plain 'restored' banner must not latch out the later 'resume-unavailable' + // upgrade — the pane would keep claiming a session it never got back. + if ( + sessionRestoredBannerShown === reason || + sessionRestoredBannerShown === 'resume-unavailable' + ) { return } - sessionRestoredBannerShown = true + sessionRestoredBannerShown = reason deps.onShowSessionRestoredBanner(pane.id, reason) } const getColdRestoreAgentResumePlatform = (): NodeJS.Platform => { 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 880bbd2c098..1a6abf8e3cf 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 @@ -152,6 +152,17 @@ describe('session restored banner pane state', () => { expect(paneText(freshPane)).toBe(SESSION_RESUME_UNAVAILABLE_BANNER_TEXT) }) + it('upgrades a restored pane to resume-unavailable and keeps identity otherwise', () => { + // Why: the reason must win over the earlier one — a pane that turned out to be a fresh + // session cannot keep claiming a restore. Identity is what stops a needless re-render. + const restored = new Map([[1, 'restored']]) + + expect(addSessionRestoredBannerPaneId(restored, 1, 'resume-unavailable')).toEqual( + new Map([[1, 'resume-unavailable']]) + ) + expect(addSessionRestoredBannerPaneId(restored, 1, 'restored')).toBe(restored) + }) + it('dismisses only the interacted pane for pointer and key events', () => { const firstPane = createPane(1) const secondPane = createPane(2) diff --git a/src/shared/agent-resume-argv-drop.test.ts b/src/shared/agent-resume-argv-drop.test.ts index 4dc71f4932e..aec709e411d 100644 --- a/src/shared/agent-resume-argv-drop.test.ts +++ b/src/shared/agent-resume-argv-drop.test.ts @@ -97,6 +97,30 @@ describe('dropAgentResumeArgvFromCommand', () => { ).toEqual({ status: 'unrecognized' }) }) + it('drops a resume argv left with trailing whitespace by a persisted command', () => { + // Why: the suffix match is anchored to the end, so an unnormalized command would + // fall through to `unrecognized` and refuse an otherwise strippable launch. + expect( + dropAgentResumeArgvFromCommand({ + command: `codex 'resume' '${CODEX_SESSION.id}' \n`, + agent: 'codex', + providerSession: CODEX_SESSION + }) + ).toEqual({ status: 'dropped', command: 'codex' }) + }) + + it('refuses when the locator also appears outside the trailing resume argv', () => { + // Why: stripping the suffix would leave the id behind in the cwd, so the launch is + // not actually resume-free and must not be reported as dropped. + expect( + dropAgentResumeArgvFromCommand({ + command: `cd '/tmp/${CODEX_SESSION.id}' && codex '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({ diff --git a/src/shared/agent-resume-argv-drop.ts b/src/shared/agent-resume-argv-drop.ts index 03646e83812..ae393d0897e 100644 --- a/src/shared/agent-resume-argv-drop.ts +++ b/src/shared/agent-resume-argv-drop.ts @@ -26,7 +26,7 @@ function resumeArgvSuffixCandidates(resumeArgs: readonly string[]): string[] { 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)) { + if (!command.endsWith(suffix) || !/\s$/.test(base)) { return null } const trimmed = base.trimEnd() @@ -42,9 +42,8 @@ export function dropAgentResumeArgvFromCommand(args: { command: string agent: ResumableTuiAgent providerSession: AgentProviderSessionMetadata - ompResumeFilePath?: string | null }): AgentResumeArgvDrop { - const argv = getAgentResumeArgv(args.agent, args.providerSession, args.ompResumeFilePath) + const argv = getAgentResumeArgv(args.agent, args.providerSession) const resumeArgs = argv?.slice(1) ?? [] const locator = resumeArgs.at(-1) const command = args.command.trimEnd()