diff --git a/src/main/codex/codex-shared-server-pane.test.ts b/src/main/codex/codex-shared-server-pane.test.ts index ef05bf5f006..14d76f7a425 100644 --- a/src/main/codex/codex-shared-server-pane.test.ts +++ b/src/main/codex/codex-shared-server-pane.test.ts @@ -29,9 +29,9 @@ vi.mock('../pty/shell-startup-env', () => ({ })) import { - findPaneCodexCommandLine, + findPaneCodex, + findPaneCodexOnSharedServer, isCodexPaneOnOrcaMirrorHome, - isPaneCodexOnSharedServer, resolveCodexPaneHome } from './codex-shared-server-pane' @@ -46,16 +46,17 @@ beforeEach(() => { mocks.isShellStartupEnvProbeSupported.mockReturnValue(true) }) -describe('findPaneCodexCommandLine', () => { +describe('findPaneCodex', () => { it('takes the launcher line, which carries the argv, over its native child', () => { const rows = [ row(SHELL, 1, '-bash'), row(101, SHELL, 'node /usr/lib/node_modules/@openai/codex/bin/codex.js --no-daemon'), row(102, 101, '/usr/lib/node_modules/@openai/codex/vendor/codex --no-daemon') ] - expect(findPaneCodexCommandLine(rows, SHELL)).toBe( - 'node /usr/lib/node_modules/@openai/codex/bin/codex.js --no-daemon' - ) + expect(findPaneCodex(rows, SHELL)).toEqual({ + command: 'node /usr/lib/node_modules/@openai/codex/bin/codex.js --no-daemon', + shell: 'bash' + }) }) it('ignores the shared server a Windows Codex spawns as its own child', () => { @@ -65,12 +66,29 @@ describe('findPaneCodexCommandLine', () => { row(102, 101, '"C:\\h\\codex.exe" app-server --listen unix:// --managed-daemon'), row(103, 101, '"C:\\h\\codex.exe" app-server daemon pid-update-loop') ] - expect(findPaneCodexCommandLine(rows, SHELL)).toBe('C:\\npm\\codex.exe') + expect(findPaneCodex(rows, SHELL)).toEqual({ command: 'C:\\npm\\codex.exe', shell: 'cmd' }) + }) + + it.each([ + ['a login shell', row(SHELL, 1, '-zsh'), 'zsh'], + ['a shell path', row(SHELL, 1, '/opt/homebrew/bin/fish -l'), 'fish'], + [ + 'a Windows image name over its command line', + { ...row(SHELL, 1, ''), name: 'pwsh.exe' }, + 'pwsh' + ] + ])('names the shell Codex was typed into from %s', (_label, shell, name) => { + expect(findPaneCodex([shell, row(101, SHELL, 'codex')], SHELL)?.shell).toBe(name) + }) + + it('leaves fish without config unnamed, since it never loads the codex function', () => { + const fish = row(SHELL, 1, '/opt/homebrew/bin/fish -l -N') + expect(findPaneCodex([fish, row(101, SHELL, 'codex')], SHELL)?.shell).toBeNull() }) it('ignores Codex outside this pane and non-Codex children', () => { const rows = [row(SHELL, 1, '-zsh'), row(101, SHELL, 'vim'), row(201, 1, 'codex')] - expect(findPaneCodexCommandLine(rows, SHELL)).toBeNull() + expect(findPaneCodex(rows, SHELL)).toBeNull() }) }) @@ -148,7 +166,7 @@ describe('isCodexPaneOnOrcaMirrorHome', () => { }) }) -describe('isPaneCodexOnSharedServer', () => { +describe('findPaneCodexOnSharedServer', () => { beforeEach(() => { mocks.getCodexPaneAccount.mockReturnValue({ selectionKey: 'host', @@ -161,20 +179,23 @@ describe('isPaneCodexOnSharedServer', () => { mocks.readWindowsProcessTable.mockResolvedValue(rows) }) - it('is true for a typed codex while its home has a live server', async () => { - await expect(isPaneCodexOnSharedServer('pty', SHELL)).resolves.toBe(true) + it('finds a typed codex while its home has a live server', async () => { + await expect(findPaneCodexOnSharedServer('pty', SHELL)).resolves.toEqual({ + command: 'codex', + shell: 'bash' + }) expect(mocks.probeCodexSharedServer).toHaveBeenCalledWith('/home/me/.codex') }) it.each(['absent', 'unknown'] as const)( - 'is false when the pane home server is %s', + 'finds none when the pane home server is %s', async (state) => { mocks.probeCodexSharedServer.mockResolvedValue(state) - await expect(isPaneCodexOnSharedServer('pty', SHELL)).resolves.toBe(false) + await expect(findPaneCodexOnSharedServer('pty', SHELL)).resolves.toBeNull() } ) - it('is false when Codex runs with --no-daemon, without probing', async () => { + it('finds none when Codex runs with --no-daemon, without probing', async () => { mocks.getProcessTableSnapshot.mockResolvedValue([ row(SHELL, 1, '-bash'), row(101, SHELL, 'codex --no-daemon') @@ -183,13 +204,13 @@ describe('isPaneCodexOnSharedServer', () => { row(SHELL, 1, 'cmd.exe'), row(101, SHELL, 'codex --no-daemon') ]) - await expect(isPaneCodexOnSharedServer('pty', SHELL)).resolves.toBe(false) + await expect(findPaneCodexOnSharedServer('pty', SHELL)).resolves.toBeNull() expect(mocks.probeCodexSharedServer).not.toHaveBeenCalled() }) - it('is false when the pane home cannot be named, without reading processes', async () => { + it('finds none when the pane home cannot be named, without reading processes', async () => { mocks.getCodexPaneAccount.mockReturnValue(null) - await expect(isPaneCodexOnSharedServer('pty', SHELL)).resolves.toBe(false) + await expect(findPaneCodexOnSharedServer('pty', SHELL)).resolves.toBeNull() expect(mocks.getProcessTableSnapshot).not.toHaveBeenCalled() expect(mocks.readWindowsProcessTable).not.toHaveBeenCalled() }) diff --git a/src/main/codex/codex-shared-server-pane.ts b/src/main/codex/codex-shared-server-pane.ts index 3f010995dbb..9cfd9ac18fb 100644 --- a/src/main/codex/codex-shared-server-pane.ts +++ b/src/main/codex/codex-shared-server-pane.ts @@ -1,3 +1,4 @@ +import { win32 as pathWin32 } from 'node:path' import { recognizeAgentProcessFromCommandLine } from '../../shared/agent-process-recognition' import { codexCommandLineJoinsSharedServer } from '../../shared/codex-shared-server-command' import { @@ -8,23 +9,41 @@ import { import { getProcessTableSnapshot } from '../../shared/process-table-snapshot-reader' import { readWindowsProcessTable } from '../windows/windows-process-table' import { isShellStartupEnvProbeSupported } from '../pty/shell-startup-env' +import { fishArgsSkipConfig } from '../fish-xdg-data-dirs-handoff' import { getSystemCodexHomePath, resolveOrcaManagedCodexHomePath } from './codex-home-paths' import { getCodexPaneAccount } from './codex-pane-account-registry' import { probeCodexSharedServer } from './codex-shared-server-probe' -type CommandRow = ProcessIdentityRow & { command: string } +type CommandRow = ProcessIdentityRow & { command: string; name?: string } + +export type PaneCodexProcess = { + command: string + /** Lowercase name of the process that launched Codex, e.g. `zsh` or `pwsh`; null when absent. */ + shell: string | null +} + +// Why name first: Windows rows carry the image name, and their command line may be empty. +function executableName(row: CommandRow): string { + const executable = row.name || row.command.trim().split(/\s+/)[0] || '' + return pathWin32 + .basename(executable) + .replace(/^-/, '') + .replace(/\.exe$/i, '') + .toLowerCase() +} /** - * The outermost Codex under the pane's shell. Outermost because a launcher - * (`node …/codex.js`) carries the argv, and on Windows the shared server it - * starts is its own child. + * The outermost Codex under the pane's shell, and the shell it was typed into. + * Outermost because a launcher (`node …/codex.js`) carries the argv, and on + * Windows the shared server it starts is its own child. */ -export function findPaneCodexCommandLine( +export function findPaneCodex( rows: readonly CommandRow[], rootPid: number -): string | null { +): PaneCodexProcess | null { + const index = getProcessTableIndex(rows) let outermost: (CommandRow & { depth: number }) | null = null - for (const row of collectDescendantsFromIndex(getProcessTableIndex(rows), rootPid)) { + for (const row of collectDescendantsFromIndex(index, rootPid)) { if ( (!outermost || row.depth < outermost.depth) && recognizeAgentProcessFromCommandLine(row.command, { includeHeadlessOneShot: true })?.agent === @@ -33,7 +52,15 @@ export function findPaneCodexCommandLine( outermost = row } } - return outermost?.command ?? null + if (!outermost) { + return null + } + const parent = index.byPid.get(outermost.ppid) + const shell = parent ? executableName(parent) : null + // Why: fish without config never loads Orca's codex function, so it counts as unwrapped. + const unwrappedFish = + shell === 'fish' && fishArgsSkipConfig(parent?.command.trim().split(/\s+/).slice(1) ?? []) + return { command: outermost.command, shell: unwrappedFish ? null : shell } } /** @@ -70,18 +97,21 @@ export function isCodexPaneOnOrcaMirrorHome(ptyId: string): boolean { ) } -/** Whether the Codex running in this local pane is a client of Codex's shared server. */ -export async function isPaneCodexOnSharedServer(ptyId: string, rootPid: number): Promise { +/** This local pane's Codex when it is a client of Codex's shared server; otherwise null. */ +export async function findPaneCodexOnSharedServer( + ptyId: string, + rootPid: number +): Promise { const codexHome = resolveCodexPaneHome(ptyId) if (!codexHome) { - return false + return null } const rows: readonly CommandRow[] = process.platform === 'win32' ? await readWindowsProcessTable() : await getProcessTableSnapshot() - const commandLine = findPaneCodexCommandLine(rows, rootPid) - return ( - commandLine !== null && - codexCommandLineJoinsSharedServer(commandLine) && + const codex = findPaneCodex(rows, rootPid) + return codex && + codexCommandLineJoinsSharedServer(codex.command) && (await probeCodexSharedServer(codexHome)) === 'live' - ) + ? codex + : null } diff --git a/src/main/daemon/daemon-protocol-version.ts b/src/main/daemon/daemon-protocol-version.ts index 42f87382d62..7f9f911c647 100644 --- a/src/main/daemon/daemon-protocol-version.ts +++ b/src/main/daemon/daemon-protocol-version.ts @@ -1,6 +1,7 @@ // Why: daemons survive app updates, so wire behavior must be version-gated. // v39 gives plain fish panes Orca's codex function through XDG_DATA_DIRS; older owners stay attachable. export const PROTOCOL_VERSION = 39 +export const CODEX_FISH_SHELL_FUNCTION_DAEMON_PROTOCOL_VERSION = 39 // Why: older daemons reject `setColorQueryReplyColors` as an unknown request type. export const COLOR_QUERY_REPLY_COLORS_DAEMON_PROTOCOL_VERSION = 38 export const CODEX_NO_DAEMON_SHELL_LAUNCH_DAEMON_PROTOCOL_VERSION = 37 diff --git a/src/main/daemon/daemon-provider-routing.test.ts b/src/main/daemon/daemon-provider-routing.test.ts new file mode 100644 index 00000000000..721f1e52c06 --- /dev/null +++ b/src/main/daemon/daemon-provider-routing.test.ts @@ -0,0 +1,9 @@ +import { describe, expect, it } from 'vitest' +import { getLegacyDaemonAdapters } from './daemon-provider-routing' +import { LocalPtyProvider } from '../providers/local-pty-provider' + +describe('getLegacyDaemonAdapters', () => { + it('lists none for the in-process fallback provider', () => { + expect(getLegacyDaemonAdapters(new LocalPtyProvider())).toEqual([]) + }) +}) diff --git a/src/main/daemon/daemon-provider-routing.ts b/src/main/daemon/daemon-provider-routing.ts index 0198bf2893c..4a2e648c37e 100644 --- a/src/main/daemon/daemon-provider-routing.ts +++ b/src/main/daemon/daemon-provider-routing.ts @@ -1,6 +1,7 @@ import { DegradedDaemonPtyProvider } from './degraded-daemon-pty-provider' import type { DaemonPtyAdapter } from './daemon-pty-adapter' import { DaemonPtyRouter } from './daemon-pty-router' +import type { IPtyProvider } from '../providers/types' export type DaemonProvider = DaemonPtyRouter | DaemonPtyAdapter | DegradedDaemonPtyProvider @@ -11,7 +12,7 @@ export function getCurrentDaemonAdapter(provider: DaemonProvider): DaemonPtyAdap return provider } -export function getLegacyDaemonAdapters(provider: DaemonProvider): DaemonPtyAdapter[] { +export function getLegacyDaemonAdapters(provider: IPtyProvider): DaemonPtyAdapter[] { if (provider instanceof DaemonPtyRouter || provider instanceof DegradedDaemonPtyProvider) { return [...provider.getLegacyAdapters()] } diff --git a/src/main/fish-xdg-data-dirs-handoff.ts b/src/main/fish-xdg-data-dirs-handoff.ts index bec47294044..e5be3ce6ab8 100644 --- a/src/main/fish-xdg-data-dirs-handoff.ts +++ b/src/main/fish-xdg-data-dirs-handoff.ts @@ -22,7 +22,7 @@ export function getFishVendorConfSnippetPath(wrapperRoot: string): string { // Why: -N/--no-config (also abbreviated or in a flag cluster) skips vendor_conf.d, // so the snippet could never undo the env. Over-matching only costs the codex hook. -function fishArgsSkipConfig(fishArgs: readonly string[]): boolean { +export function fishArgsSkipConfig(fishArgs: readonly string[]): boolean { return fishArgs.some( (arg) => /^-[^-]*N/.test(arg) || (arg.length > 5 && '--no-config'.startsWith(arg)) ) diff --git a/src/main/ipc/pty/ipc/codex-shared-server.test.ts b/src/main/ipc/pty/ipc/codex-shared-server.test.ts index 94715870d3c..2c457df160b 100644 --- a/src/main/ipc/pty/ipc/codex-shared-server.test.ts +++ b/src/main/ipc/pty/ipc/codex-shared-server.test.ts @@ -1,18 +1,22 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -type Handler = (event: unknown, args: { id: string }) => Promise +type Handler = (event: unknown, args: { id: string }) => Promise type Session = { id: string; rootProcessId?: number; wslDistro?: string } const mocks = vi.hoisted(() => ({ handlers: new Map(), sessions: new Array(), hasProvider: vi.fn<(id: string) => boolean>(), - isPaneCodexOnSharedServer: vi.fn<(id: string, rootPid: number) => Promise>(), + findPaneCodexOnSharedServer: + vi.fn< + (id: string, rootPid: number) => Promise<{ command: string; shell: string | null } | null> + >(), resolveCodexPaneHome: vi.fn<(id: string) => string | null>(), isOnOrcaMirror: vi.fn<(id: string) => boolean>(), disable: vi.fn<(home: string) => Promise>(), disableOnMirror: vi.fn<(home: string) => Promise>(), - stop: vi.fn<(home: string) => Promise>() + stop: vi.fn<(home: string) => Promise>(), + legacyAdapters: new Array<{ protocolVersion: number; hasPty: (id: string) => boolean }>() })) vi.mock('../../pty-host-bindings', () => ({ getPtyIpc: () => ({ @@ -21,7 +25,7 @@ vi.mock('../../pty-host-bindings', () => ({ })) vi.mock('../../../codex/codex-shared-server-pane', () => ({ isCodexPaneOnOrcaMirrorHome: mocks.isOnOrcaMirror, - isPaneCodexOnSharedServer: mocks.isPaneCodexOnSharedServer, + findPaneCodexOnSharedServer: mocks.findPaneCodexOnSharedServer, resolveCodexPaneHome: mocks.resolveCodexPaneHome })) vi.mock('../../../codex/codex-shared-server-fix', () => ({ @@ -29,6 +33,9 @@ vi.mock('../../../codex/codex-shared-server-fix', () => ({ disableCodexSharedServerAutoStartOnOrcaMirror: mocks.disableOnMirror, stopCodexSharedServer: mocks.stop })) +vi.mock('../../../daemon/daemon-provider-routing', () => ({ + getLegacyDaemonAdapters: () => mocks.legacyAdapters +})) vi.mock('../provider/registry', () => ({ hasPtyProviderForInspection: mocks.hasProvider, getProviderForPty: () => ({ listProcesses: () => Promise.resolve(mocks.sessions) }) @@ -38,13 +45,14 @@ import { toAppSshPtyId } from '../../../providers/ssh-pty-id' import { ptyOwnership } from '../provider/ownership-state' import { installPtyCodexSharedServerIpcHandler } from './codex-shared-server' +// Each channel with its answer for a local pane and its answer when it refuses one. const CHANNELS = [ - 'pty:isCodexOnSharedServer', - 'pty:disableCodexSharedServerAutoStart', - 'pty:stopCodexSharedServer' + ['pty:isCodexOnSharedServer', { joined: true, openedBeforeWrapper: false }, { joined: false }], + ['pty:disableCodexSharedServerAutoStart', true, false], + ['pty:stopCodexSharedServer', true, false] ] as const -function invoke(channel: string, id: string): Promise { +function invoke(channel: string, id: string): Promise { const handler = mocks.handlers.get(channel) if (!handler) { throw new Error(`missing ${channel}`) @@ -58,18 +66,50 @@ beforeEach(() => { ptyOwnership.clear() mocks.sessions = [{ id: 'local-1', rootProcessId: 100 }] mocks.hasProvider.mockReturnValue(true) - mocks.isPaneCodexOnSharedServer.mockResolvedValue(true) + mocks.findPaneCodexOnSharedServer.mockResolvedValue({ command: 'codex', shell: 'zsh' }) mocks.resolveCodexPaneHome.mockReturnValue('/home/me/.codex') mocks.isOnOrcaMirror.mockReturnValue(false) mocks.disable.mockResolvedValue(true) mocks.disableOnMirror.mockResolvedValue(true) mocks.stop.mockResolvedValue(true) + mocks.legacyAdapters = [] installPtyCodexSharedServerIpcHandler({ getLocalPtyProviderStartupPromise: () => undefined }) }) describe('Codex shared-server IPC', () => { - it.each(CHANNELS)('%s answers for a local pane', async (channel) => { - expect(await invoke(channel, 'local-1')).toBe(true) + it.each(CHANNELS)('%s answers for a local pane', async (channel, answer) => { + expect(await invoke(channel, 'local-1')).toEqual(answer) + }) + + const ownedBy = (protocolVersion: number, ptyId = 'local-1') => ({ + protocolVersion, + hasPty: (id: string) => id === ptyId + }) + + it.each([ + ['zsh on a v36 daemon', 'zsh', [ownedBy(36)], true], + ['zsh on a v37 daemon', 'zsh', [ownedBy(37)], false], + ['fish on a v37 daemon', 'fish', [ownedBy(37)], true], + ['fish on a v38 daemon', 'fish', [ownedBy(38)], true], + ['fish on a v39 daemon', 'fish', [ownedBy(39)], false], + ['cmd.exe, which never gets the codex function', 'cmd', [ownedBy(36)], false], + ['an unknown shell', null, [ownedBy(36)], false], + ['zsh where the older daemon does not hold this pane', 'zsh', [ownedBy(36, 'other')], false] + ])('tells whether a new terminal would fix %s', async (_label, shell, adapters, old) => { + mocks.findPaneCodexOnSharedServer.mockResolvedValue({ command: 'codex', shell }) + mocks.legacyAdapters = adapters + expect(await invoke('pty:isCodexOnSharedServer', 'local-1')).toEqual({ + joined: true, + openedBeforeWrapper: old + }) + }) + + it('reads no daemon owner for a pane that has not joined', async () => { + const hasPty = vi.fn(() => true) + mocks.legacyAdapters = [{ protocolVersion: 36, hasPty }] + mocks.findPaneCodexOnSharedServer.mockResolvedValue(null) + expect(await invoke('pty:isCodexOnSharedServer', 'local-1')).toEqual({ joined: false }) + expect(hasPty).not.toHaveBeenCalled() }) it('runs the fix against the pane home', async () => { @@ -104,17 +144,18 @@ describe('Codex shared-server IPC', () => { ['a pane no provider holds', 'local-1', () => mocks.hasProvider.mockReturnValue(false)] ] - it.each(CHANNELS.flatMap((channel) => refusals.map((refusal) => [channel, ...refusal] as const)))( - '%s refuses %s', - async (channel, _label, id, arrange) => { - arrange() - expect(await invoke(channel, id)).toBe(false) - expect(mocks.isPaneCodexOnSharedServer).not.toHaveBeenCalled() - expect(mocks.disable).not.toHaveBeenCalled() - expect(mocks.disableOnMirror).not.toHaveBeenCalled() - expect(mocks.stop).not.toHaveBeenCalled() - } - ) + it.each( + CHANNELS.flatMap(([channel, , refused]) => + refusals.map((refusal) => [channel, ...refusal, refused] as const) + ) + )('%s refuses %s', async (channel, _label, id, arrange, refused) => { + arrange() + expect(await invoke(channel, id)).toEqual(refused) + expect(mocks.findPaneCodexOnSharedServer).not.toHaveBeenCalled() + expect(mocks.disable).not.toHaveBeenCalled() + expect(mocks.disableOnMirror).not.toHaveBeenCalled() + expect(mocks.stop).not.toHaveBeenCalled() + }) it.each(CHANNELS.slice(1))('%s runs nothing when the pane has no Codex home', async (channel) => { mocks.resolveCodexPaneHome.mockReturnValue(null) diff --git a/src/main/ipc/pty/ipc/codex-shared-server.ts b/src/main/ipc/pty/ipc/codex-shared-server.ts index 33d30d314a9..57221284fa1 100644 --- a/src/main/ipc/pty/ipc/codex-shared-server.ts +++ b/src/main/ipc/pty/ipc/codex-shared-server.ts @@ -1,8 +1,8 @@ import { getPtyIpc } from '../../pty-host-bindings' import { parseAppSshPtyId } from '../../../providers/ssh-pty-id' import { + findPaneCodexOnSharedServer, isCodexPaneOnOrcaMirrorHome, - isPaneCodexOnSharedServer, resolveCodexPaneHome } from '../../../codex/codex-shared-server-pane' import { @@ -10,9 +10,25 @@ import { disableCodexSharedServerAutoStartOnOrcaMirror, stopCodexSharedServer } from '../../../codex/codex-shared-server-fix' +import { getLegacyDaemonAdapters } from '../../../daemon/daemon-provider-routing' +import { + CODEX_FISH_SHELL_FUNCTION_DAEMON_PROTOCOL_VERSION, + CODEX_NO_DAEMON_SHELL_LAUNCH_DAEMON_PROTOCOL_VERSION +} from '../../../daemon/daemon-protocol-version' +import type { CodexSharedServerStatus } from '../../../../shared/codex-shared-server-command' import { ptyOwnership } from '../provider/ownership-state' import { getProviderForPty, hasPtyProviderForInspection } from '../provider/registry' +// Why per shell: a new terminal fixes Codex only where this build's daemon gives that shell Orca's +// codex function; cmd.exe and unrecognized shells never get one. +const CODEX_SHELL_FUNCTION_PROTOCOL_BY_SHELL: ReadonlyMap = new Map([ + ['zsh', CODEX_NO_DAEMON_SHELL_LAUNCH_DAEMON_PROTOCOL_VERSION], + ['bash', CODEX_NO_DAEMON_SHELL_LAUNCH_DAEMON_PROTOCOL_VERSION], + ['powershell', CODEX_NO_DAEMON_SHELL_LAUNCH_DAEMON_PROTOCOL_VERSION], + ['pwsh', CODEX_NO_DAEMON_SHELL_LAUNCH_DAEMON_PROTOCOL_VERSION], + ['fish', CODEX_FISH_SHELL_FUNCTION_DAEMON_PROTOCOL_VERSION] +]) + type Deps = { getLocalPtyProviderStartupPromise: () => Promise | undefined } /** The pane's root pid when it is a local, non-WSL pane; otherwise null. */ @@ -37,21 +53,40 @@ async function findLocalPaneRootPid(deps: Deps, id: unknown): Promise( deps: Deps, channel: string, - run: (id: string, rootPid: number) => Promise + run: (id: string, rootPid: number) => Promise, + refused: T ): void { - getPtyIpc().handle(channel, async (_event, args: { id: string }): Promise => { + getPtyIpc().handle(channel, async (_event, args: { id: string }): Promise => { try { const rootPid = await findLocalPaneRootPid(deps, args?.id) - return rootPid === null ? false : await run(args.id, rootPid) + return rootPid === null ? refused : await run(args.id, rootPid) } catch { - return false + return refused } }) } +async function readPaneSharedServerStatus( + id: string, + rootPid: number +): Promise { + const codex = await findPaneCodexOnSharedServer(id, rootPid) + if (!codex) { + return { joined: false } + } + // Why: the pane's own daemon predates its shell's codex function, which a new terminal has. + const shellFunctionProtocol = CODEX_SHELL_FUNCTION_PROTOCOL_BY_SHELL.get(codex.shell ?? '') + const openedBeforeWrapper = + shellFunctionProtocol !== undefined && + getLegacyDaemonAdapters(getProviderForPty(id)).some( + (adapter) => adapter.hasPty(id) && adapter.protocolVersion < shellFunctionProtocol + ) + return { joined: true, openedBeforeWrapper } +} + /** Runs a fix command against the pane's own CODEX_HOME, never a guessed one. */ function runForPaneHome(fix: (codexHome: string) => Promise) { return async (id: string): Promise => { @@ -71,7 +106,7 @@ function disableForPane(id: string): Promise { // Why its own read: only a pane already showing Codex asks, so no cadence poll pays for argv. export function installPtyCodexSharedServerIpcHandler(deps: Deps): void { - handleLocalPane(deps, 'pty:isCodexOnSharedServer', isPaneCodexOnSharedServer) - handleLocalPane(deps, 'pty:disableCodexSharedServerAutoStart', disableForPane) - handleLocalPane(deps, 'pty:stopCodexSharedServer', runForPaneHome(stopCodexSharedServer)) + handleLocalPane(deps, 'pty:isCodexOnSharedServer', readPaneSharedServerStatus, { joined: false }) + handleLocalPane(deps, 'pty:disableCodexSharedServerAutoStart', disableForPane, false) + handleLocalPane(deps, 'pty:stopCodexSharedServer', runForPaneHome(stopCodexSharedServer), false) } diff --git a/src/preload/api/pty-api.ts b/src/preload/api/pty-api.ts index c7c46c7b004..4cfb5572776 100644 --- a/src/preload/api/pty-api.ts +++ b/src/preload/api/pty-api.ts @@ -18,6 +18,7 @@ import type { TerminalViewAttributes } from '../../shared/terminal-view-attribut import type { TuiAgent } from '../../shared/tui-agent' import type { PtyManagementApi } from './pty-management-api' import type { TerminalProcessInspection } from '../../shared/terminal-process-inspection' +import type { CodexSharedServerStatus } from '../../shared/codex-shared-server-command' export type PtyApi = { spawn: (opts: { @@ -126,8 +127,8 @@ export type PtyApi = { } ) => Promise confirmForegroundProcess: (id: string) => Promise - /** Local panes only; false for any other pane. */ - isCodexOnSharedServer: (id: string) => Promise + /** Local panes only; never joined for any other pane. */ + isCodexOnSharedServer: (id: string) => Promise /** Runs the fix with the pane's own Codex; true only once verified. Local panes only. */ disableCodexSharedServerAutoStart: (id: string) => Promise stopCodexSharedServer: (id: string) => Promise diff --git a/src/preload/api/pty-bridge-stream-and-serialization.ts b/src/preload/api/pty-bridge-stream-and-serialization.ts index ec5b232acb5..e0b2e7f2bf6 100644 --- a/src/preload/api/pty-bridge-stream-and-serialization.ts +++ b/src/preload/api/pty-bridge-stream-and-serialization.ts @@ -3,6 +3,7 @@ import type { PtyModelRestoreNeededEvent } from '../../shared/pty-model-restore- import type { TerminalSideEffectBatch } from '../../shared/terminal-side-effect-facts' import type { PreloadApi } from '../api-types' import type { TerminalProcessInspection } from '../../shared/terminal-process-inspection' +import type { CodexSharedServerStatus } from '../../shared/codex-shared-server-command' export const ptyStreamAndSerializationApi = { inspectProcess: ( @@ -16,7 +17,7 @@ export const ptyStreamAndSerializationApi = { ipcRenderer.invoke('pty:inspectProcess', { id, ...options }), confirmForegroundProcess: (id: string): Promise => ipcRenderer.invoke('pty:confirmForegroundProcess', { id }), - isCodexOnSharedServer: (id: string): Promise => + isCodexOnSharedServer: (id: string): Promise => ipcRenderer.invoke('pty:isCodexOnSharedServer', { id }), disableCodexSharedServerAutoStart: (id: string): Promise => ipcRenderer.invoke('pty:disableCodexSharedServerAutoStart', { id }), diff --git a/src/renderer/src/components/activity/activity-thread-actions.ts b/src/renderer/src/components/activity/activity-thread-actions.ts index c2d99c23fc2..84d9d55ea21 100644 --- a/src/renderer/src/components/activity/activity-thread-actions.ts +++ b/src/renderer/src/components/activity/activity-thread-actions.ts @@ -1,6 +1,5 @@ import { activateTabAndFocusPane } from '@/lib/activate-tab-and-focus-pane' -import { TOGGLE_FLOATING_TERMINAL_EVENT } from '@/lib/floating-terminal' -import { isFloatingWorkspacePanelVisible } from '@/lib/floating-workspace-terminal-actions' +import { revealFloatingWorkspacePanel } from '@/lib/floating-workspace-panel-reveal' import { activateStructuredAgentSessionTab } from '@/lib/structured-agent-session-tab-activation' import { activateAndRevealWorkspace } from '@/lib/worktree-activation' import { jumpToWorktreeFromSidebar } from '@/lib/worktree-jump-navigation' @@ -48,25 +47,6 @@ export function hasActivityThreadWorkspace( ) } -function toggleFloatingWorkspacePanelIfHidden(): void { - if (!isFloatingWorkspacePanelVisible()) { - window.dispatchEvent(new Event(TOGGLE_FLOATING_TERMINAL_EVENT)) - } -} - -// Why enable first: floating tabs outlive a feature disable, and the panel ignores the toggle -// while disabled, so a live floating agent row would otherwise be a silent no-op. -function revealFloatingWorkspacePanel(state: AppState): void { - if (state.settings?.floatingTerminalEnabled === true) { - toggleFloatingWorkspacePanelIfHidden() - return - } - void state.updateSettings({ floatingTerminalEnabled: true }).then(() => { - // Why deferred a frame: the panel only honors the toggle once the enabled flag has reached React. - requestAnimationFrame(toggleFloatingWorkspacePanelIfHidden) - }) -} - export function createActivityThreadActions({ getMarkAllReadThreads, acknowledgeAgents, diff --git a/src/renderer/src/components/terminal-pane/CodexOldTerminalDialog.tsx b/src/renderer/src/components/terminal-pane/CodexOldTerminalDialog.tsx new file mode 100644 index 00000000000..60238ca3832 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/CodexOldTerminalDialog.tsx @@ -0,0 +1,54 @@ +import { Button } from '@/components/ui/button' +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle +} from '@/components/ui/dialog' +import { translate } from '@/i18n/i18n' + +/** Learn more for a terminal opened before Orca gave each Codex its own server. */ +export function CodexOldTerminalDialog({ + open, + onOpenChange, + onOpenNewTerminal +}: { + open: boolean + onOpenChange: (open: boolean) => void + onOpenNewTerminal: () => void +}): React.JSX.Element { + return ( + + + + + {translate( + 'terminal.codexSharedServerBanner.oldTerminalDialogTitle', + 'Why this Codex shares a server' + )} + + + {translate( + 'terminal.codexSharedServerBanner.oldTerminalDialogRisk', + 'Codex sessions that share a server can end together, and agent status can be wrong.' + )} + + +

+ {translate( + 'terminal.codexSharedServerBanner.oldTerminalDialogCause', + 'Older terminals still share a Codex server. Open a new terminal to run Codex on a separate server.' + )} +

+ {/* Why no global fix: a new terminal already runs Codex on its own server here. */} + + + +
+
+ ) +} diff --git a/src/renderer/src/components/terminal-pane/CodexSharedServerBanner.tsx b/src/renderer/src/components/terminal-pane/CodexSharedServerBanner.tsx index 434c05b0e85..94264705465 100644 --- a/src/renderer/src/components/terminal-pane/CodexSharedServerBanner.tsx +++ b/src/renderer/src/components/terminal-pane/CodexSharedServerBanner.tsx @@ -4,6 +4,14 @@ import { Button } from '@/components/ui/button' import { translate } from '@/i18n/i18n' import { useAppStore } from '@/store' import { isCodexSharedServerWarningEnabled } from '../../../../shared/codex-terminal-server-isolation' +import type { CodexSharedServerStatus } from '../../../../shared/codex-shared-server-command' +import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants' +import { makePaneKey } from '../../../../shared/stable-pane-id' +import { createFloatingWorkspaceTerminalTab } from '@/lib/floating-workspace-tab-creation' +import { revealFloatingWorkspacePanel } from '@/lib/floating-workspace-panel-reveal' +import { activateAndRevealWorkspace } from '@/lib/worktree-activation' +import { useModalReturnFocus } from '@/hooks/useModalReturnFocus' +import { CodexOldTerminalDialog } from './CodexOldTerminalDialog' import { CodexSharedServerFixDialog } from './CodexSharedServerFixDialog' import { retireCodexTerminalServerIsolationNotice } from './codex-terminal-server-isolation-notice' @@ -14,22 +22,22 @@ const dismissedPtyIds = new Set() /** Asks on the ladder until an answer is yes; returns a cancel that drops any later answer. */ function askUntilOnSharedServer( - ask: (ptyId: string) => Promise, + ask: (ptyId: string) => Promise, ptyId: string, - onJoined: () => void + onJoined: (status: CodexSharedServerStatus) => void ): () => void { let cancelled = false let timer: ReturnType | undefined const schedule = (attempt: number): void => { timer = setTimeout(() => { void ask(ptyId) - .catch(() => false) - .then((joined) => { + .catch((): CodexSharedServerStatus => ({ joined: false })) + .then((status) => { if (cancelled) { return } - if (joined) { - onJoined() + if (status.joined) { + onJoined(status) } else if (attempt + 1 < CHECK_DELAYS_MS.length) { schedule(attempt + 1) } @@ -43,21 +51,47 @@ function askUntilOnSharedServer( } } -function usePaneCodexOnSharedServer(ptyId: string, enabled: boolean, recheck: number): boolean { - const [joined, setJoined] = useState(false) +function usePaneCodexSharedServerStatus( + ptyId: string, + enabled: boolean, + recheck: number +): CodexSharedServerStatus | null { + const [status, setStatus] = useState(null) useEffect(() => { if (!enabled) { return } - const cancel = askUntilOnSharedServer(window.api.pty.isCodexOnSharedServer, ptyId, () => - setJoined(true) - ) + const cancel = askUntilOnSharedServer(window.api.pty.isCodexOnSharedServer, ptyId, setStatus) return () => { cancel() - setJoined(false) + setStatus(null) } }, [enabled, ptyId, recheck]) - return joined + return status +} + +/** Opens a new terminal where this tab lives; a new terminal's shell has Orca's codex wrapper. */ +function openTerminalBesideTab(terminalTabId: string): boolean { + const state = useAppStore.getState() + const tab = Object.values(state.unifiedTabsByWorktree) + .flat() + .find( + (candidate) => candidate.contentType === 'terminal' && candidate.entityId === terminalTabId + ) + if (!tab) { + return false + } + if (tab.worktreeId === FLOATING_TERMINAL_WORKTREE_ID) { + revealFloatingWorkspacePanel(state) + void createFloatingWorkspaceTerminalTab(state) + return true + } + // Why always: Activity can show this pane behind its own view even when its workspace is active. + if (activateAndRevealWorkspace(tab.worktreeId) === false) { + return false + } + void useAppStore.getState().openNewTerminalTabInActiveWorkspace(tab.groupId) + return true } /** Reserves the banner's height at the top of its pane so the terminal refits below it. */ @@ -85,11 +119,14 @@ function useReservePaneTopSpace(): React.RefObject { export function CodexSharedServerBanner({ ptyId, - paneKey + tabId, + leafId }: { ptyId: string - paneKey: string + tabId: string + leafId: string }): React.JSX.Element | null { + const paneKey = makePaneKey(tabId, leafId) const [dismissed, setDismissed] = useState(() => dismissedPtyIds.has(ptyId)) const warningEnabled = useAppStore( (state) => state.settings !== null && isCodexSharedServerWarningEnabled(state.settings) @@ -102,45 +139,123 @@ export function CodexSharedServerBanner({ ) // Why a recheck: after the fix stops the server, the banner hides unless a new one is joined. const [recheck, setRecheck] = useState(0) - const joined = usePaneCodexOnSharedServer( + const status = usePaneCodexSharedServerStatus( ptyId, warningEnabled && codexInPane && !dismissed, recheck ) - const [fixOpen, setFixOpen] = useState(false) - // Why fixOpen keeps it: stopping the server ends this pane's Codex, which must not close the dialog. - if (!joined && !fixOpen) { + const [openDialog, setOpenDialog] = useState<'fix' | 'oldTerminal' | null>(null) + // Why: neither dialog has a Radix trigger, so closing it would leave focus on document.body. + const { captureReturnFocus, skipReturnFocus } = useModalReturnFocus(openDialog !== null) + const showDialog = (dialog: 'fix' | 'oldTerminal'): void => { + captureReturnFocus() + setOpenDialog(dialog) + } + // Why an open dialog keeps it: this pane's Codex can end mid-dialog (Stop server does it), and + // the dialog must still close normally so focus returns. + if (!status && openDialog === null) { return null } + const { body, primaryAction } = + status?.joined && status.openedBeforeWrapper + ? { + body: ( + <> + {translate( + 'terminal.codexSharedServerBanner.openedBeforeUpdateBody', + 'This terminal was opened before Orca started giving each Codex its own server.' + )}{' '} + showDialog('oldTerminal')} /> + + ), + primaryAction: ( + + ) + } + : { + body: ( + <> + {translate( + 'terminal.codexSharedServerBanner.body', + 'Sessions may end unexpectedly, and agent status may be wrong.' + )}{' '} + showDialog('fix')} /> + + ), + primaryAction: ( + + ) + } + const closeDialog = (open: boolean): void => { + if (!open) { + setOpenDialog(null) + } + } return ( - setRecheck((count) => count + 1)} - onDismiss={() => { - dismissedPtyIds.add(ptyId) - setDismissed(true) - }} - onDontShowAgain={() => - void useAppStore.getState().updateSettings({ codexSharedServerWarning: false }) - } - /> + <> + {status ? ( + { + dismissedPtyIds.add(ptyId) + setDismissed(true) + }} + onDontShowAgain={() => + void useAppStore.getState().updateSettings({ codexSharedServerWarning: false }) + } + /> + ) : null} + { + // Why: a new terminal takes focus; only when none opens does focus return to this pane. + if (openTerminalBesideTab(tabId)) { + skipReturnFocus() + } + setOpenDialog(null) + }} + /> + setRecheck((count) => count + 1)} + /> + ) } -function CodexSharedServerBannerContent({ - ptyId, - fixOpen, - onFixOpenChange, - onServerStopped, +function LearnMoreLink({ onClick }: { onClick: () => void }): React.JSX.Element { + return ( + + ) +} + +function CodexSharedServerBannerFrame({ + body, + primaryAction, onDismiss, onDontShowAgain }: { - ptyId: string - fixOpen: boolean - onFixOpenChange: (open: boolean) => void - onServerStopped: () => void + body: React.ReactNode + primaryAction: React.ReactNode onDismiss: () => void onDontShowAgain: () => void }): React.JSX.Element { @@ -169,25 +284,11 @@ function CodexSharedServerBannerContent({ 'This Codex is sharing a server with your other Codex tabs' )}

-

- {translate( - 'terminal.codexSharedServerBanner.body', - 'Sessions may end unexpectedly, and agent status may be wrong.' - )}{' '} - -

+

{body}

- + {primaryAction} @@ -196,12 +297,6 @@ function CodexSharedServerBannerContent({
- ) } diff --git a/src/renderer/src/components/terminal-pane/TerminalPaneRuntimePortals.tsx b/src/renderer/src/components/terminal-pane/TerminalPaneRuntimePortals.tsx index ff587ab8b95..0d6d2d53056 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPaneRuntimePortals.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPaneRuntimePortals.tsx @@ -1,7 +1,6 @@ import { createPortal } from 'react-dom' import CodexRestartChip from '../CodexRestartChip' import { CodexSharedServerBanner } from './CodexSharedServerBanner' -import { makePaneKey } from '../../../../shared/stable-pane-id' import { TerminalSshReconnectOverlay } from './TerminalSshReconnectOverlay' import { TerminalRemoteRuntimeReconnectBanner } from './TerminalRemoteRuntimeReconnectBanner' import { TerminalProcessExitOverlay } from './TerminalProcessExitOverlay' @@ -40,7 +39,8 @@ export function TerminalPaneCodexRestartPortals({ , pane.container, diff --git a/src/renderer/src/components/terminal-pane/codex-shared-server-banner.test.tsx b/src/renderer/src/components/terminal-pane/codex-shared-server-banner.test.tsx index 2f81ae371e0..1855062a1df 100644 --- a/src/renderer/src/components/terminal-pane/codex-shared-server-banner.test.tsx +++ b/src/renderer/src/components/terminal-pane/codex-shared-server-banner.test.tsx @@ -6,20 +6,47 @@ import { toast } from 'sonner' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { useAppStore } from '@/store' import type { GlobalSettings } from '../../../../shared/global-settings-types' -import { getDefaultSettings } from '../../../../shared/constants' +import { FLOATING_TERMINAL_WORKTREE_ID, getDefaultSettings } from '../../../../shared/constants' import { CodexSharedServerBanner } from './CodexSharedServerBanner' import { CODEX_DISABLE_AUTO_START_COMMAND, - CODEX_STOP_SHARED_SERVER_COMMAND + CODEX_STOP_SHARED_SERVER_COMMAND, + type CodexSharedServerStatus } from '../../../../shared/codex-shared-server-command' +import { makePaneKey } from '../../../../shared/stable-pane-id' +import type { Tab } from '../../../../shared/tab-types' + +const routing = vi.hoisted(() => ({ + activateAndRevealWorkspace: vi.fn<(id: string) => unknown>(), + revealFloatingWorkspacePanel: vi.fn(), + createFloatingWorkspaceTerminalTab: vi.fn() +})) +vi.mock('@/lib/worktree-activation', () => ({ + activateAndRevealWorkspace: routing.activateAndRevealWorkspace +})) +vi.mock('@/lib/floating-workspace-panel-reveal', () => ({ + revealFloatingWorkspacePanel: routing.revealFloatingWorkspacePanel +})) +vi.mock('@/lib/floating-workspace-tab-creation', () => ({ + createFloatingWorkspaceTerminalTab: routing.createFloatingWorkspaceTerminalTab +})) globalThis.IS_REACT_ACT_ENVIRONMENT = true -const PANE_KEY = 'tab-1:leaf-1' +const TAB_ID = 'tab-1' +const LEAF_ID = '11111111-1111-4111-8111-111111111111' +const PANE_KEY = makePaneKey(TAB_ID, LEAF_ID) +const JOINED: CodexSharedServerStatus = { joined: true, openedBeforeWrapper: false } +const NOT_JOINED: CodexSharedServerStatus = { joined: false } +const OLD_TAB_JOINED: CodexSharedServerStatus = { joined: true, openedBeforeWrapper: true } const TITLE = 'This Codex is sharing a server' +const OLD_TAB_BODY = + 'This terminal was opened before Orca started giving each Codex its own server.' let paneElement: HTMLDivElement let root: Root -let isCodexOnSharedServer: ReturnType Promise>> +let isCodexOnSharedServer: ReturnType< + typeof vi.fn<(id: string) => Promise> +> let disableCodexSharedServerAutoStart: ReturnType Promise>> let stopCodexSharedServer: ReturnType Promise>> let writeClipboardText: ReturnType Promise>> @@ -42,7 +69,7 @@ function setState(settings: Partial, agent: 'codex' | null = 'co async function renderBanner(): Promise { await act(async () => { - root.render() + root.render() }) } @@ -66,6 +93,8 @@ function button(label: string): HTMLButtonElement { beforeEach(() => { vi.useFakeTimers() + vi.clearAllMocks() + routing.activateAndRevealWorkspace.mockReturnValue({ primaryTabId: null }) vi.stubGlobal('ResizeObserver', ResizeObserverStub) useAppStore.setState(useAppStore.getInitialState(), true) ptyId = `pty-${(nextPtyId += 1)}` @@ -73,7 +102,7 @@ beforeEach(() => { paneElement.className = 'pane' document.body.appendChild(paneElement) root = createRoot(paneElement) - isCodexOnSharedServer = vi.fn(() => Promise.resolve(true)) + isCodexOnSharedServer = vi.fn(() => Promise.resolve(JOINED)) disableCodexSharedServerAutoStart = vi.fn(() => Promise.resolve(true)) stopCodexSharedServer = vi.fn(() => Promise.resolve(true)) writeClipboardText = vi.fn(() => Promise.resolve()) @@ -113,6 +142,19 @@ describe('CodexSharedServerBanner', () => { expect(paneElement.style.getPropertyValue('--orca-pane-top-banner-height')).toMatch(/px$/) }) + it('keeps the default Learn more on the Fix steps, with no old-terminal copy', async () => { + setState({}) + await renderBanner() + await advance(1_000) + expect(paneElement.textContent).not.toContain(OLD_TAB_BODY) + expect(() => button('Open new terminal')).toThrow() + button('Fix') + + await act(async () => button('Learn more').click()) + expect(document.body.textContent).toContain('Give each Codex tab its own server') + expect(document.body.textContent).not.toContain('Turn off sharing everywhere') + }) + it("retires the one-time 'runs Codex without its shared server' toast, which it contradicts", async () => { setState({}) useAppStore.setState({ codexTerminalServerIsolationNoticeSeen: false }) @@ -127,7 +169,7 @@ describe('CodexSharedServerBanner', () => { it('keeps asking while Codex starts, then stops once it has an answer', async () => { setState({}) - isCodexOnSharedServer.mockResolvedValueOnce(false) + isCodexOnSharedServer.mockResolvedValueOnce(NOT_JOINED) await renderBanner() await advance(1_000) expect(paneElement.textContent).toBe('') @@ -183,7 +225,7 @@ describe('CodexSharedServerBanner', () => { const confirm = Array.from(document.querySelectorAll('button')).filter( (candidate) => candidate.textContent?.trim() === 'Stop server' ) - isCodexOnSharedServer.mockResolvedValue(false) + isCodexOnSharedServer.mockResolvedValue(NOT_JOINED) await act(async () => confirm.at(-1)?.click()) expect(stopCodexSharedServer).toHaveBeenCalledWith(ptyId) expect(document.body.textContent).toContain('Stopped') @@ -252,4 +294,279 @@ describe('CodexSharedServerBanner', () => { await act(async () => setState({}, null)) expect(paneElement.textContent).toBe('') }) + + describe('in a terminal opened before the codex wrapper', () => { + function terminalTab(worktreeId: string, groupId: string): Tab { + return { + id: 'unified-1', + entityId: TAB_ID, + groupId, + worktreeId, + contentType: 'terminal', + label: 'Terminal', + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 0 + } + } + + it('says why this terminal shares the server and offers a new one instead of Fix', async () => { + setState({}) + isCodexOnSharedServer.mockResolvedValue(OLD_TAB_JOINED) + await renderBanner() + await advance(1_000) + + expect(paneElement.textContent).toContain(TITLE) + expect(paneElement.textContent).toContain(OLD_TAB_BODY) + expect(paneElement.textContent).not.toContain('agent status may be wrong') + expect(() => button('Fix')).toThrow() + button('Open new terminal') + button("Don't show again") + button('Dismiss') + }) + + async function renderOldTab(worktreeId: string): Promise<() => Promise> { + const openNewTerminalTabInActiveWorkspace = vi.fn(() => Promise.resolve()) + setState({}) + useAppStore.setState({ + activeWorktreeId: 'wt-1', + activeView: 'activity', + unifiedTabsByWorktree: { [worktreeId]: [terminalTab(worktreeId, 'group-2')] }, + openNewTerminalTabInActiveWorkspace + }) + isCodexOnSharedServer.mockResolvedValue(OLD_TAB_JOINED) + await renderBanner() + await advance(1_000) + return openNewTerminalTabInActiveWorkspace + } + + function dialog(): Element | null { + return document.querySelector('[role="dialog"]') + } + + it('explains the old terminal in Learn more, with no global fix', async () => { + await renderOldTab('wt-1') + expect(dialog()).toBeNull() + await act(async () => button('Learn more').click()) + + expect(dialog()?.textContent).toContain('Why this Codex shares a server') + expect(dialog()?.textContent).toContain( + 'Open a new terminal to run Codex on a separate server.' + ) + expect(dialog()?.textContent).not.toContain('Turn off sharing everywhere') + expect(document.body.textContent).not.toContain(CODEX_DISABLE_AUTO_START_COMMAND) + expect(document.body.textContent).not.toContain(CODEX_STOP_SHARED_SERVER_COMMAND) + }) + + it('opens a new terminal from Learn more and closes the dialog', async () => { + const openNewTerminal = await renderOldTab('wt-1') + await act(async () => button('Learn more').click()) + const inDialog = Array.from(dialog()?.querySelectorAll('button') ?? []).find( + (candidate) => candidate.textContent?.trim() === 'Open new terminal' + ) + await act(async () => inDialog?.click()) + + expect(openNewTerminal).toHaveBeenCalledWith('group-2') + expect(dialog()).toBeNull() + expect(paneElement.textContent).toContain(OLD_TAB_BODY) + }) + + async function clickOpenNewTerminal(worktreeId: string): Promise<() => Promise> { + const openNewTerminalTabInActiveWorkspace = await renderOldTab(worktreeId) + await act(async () => button('Open new terminal').click()) + expect(disableCodexSharedServerAutoStart).not.toHaveBeenCalled() + expect(stopCodexSharedServer).not.toHaveBeenCalled() + return openNewTerminalTabInActiveWorkspace + } + + it.each([ + ['an active worktree viewed from Activity', 'wt-1'], + ['another worktree', 'wt-other'], + ['a folder workspace', 'folder:notes'] + ])('activates %s, then opens a terminal in its group', async (_label, worktreeId) => { + const openNewTerminal = await clickOpenNewTerminal(worktreeId) + expect(routing.activateAndRevealWorkspace).toHaveBeenCalledWith(worktreeId) + expect(openNewTerminal).toHaveBeenCalledWith('group-2') + }) + + it('opens nothing when its workspace cannot be activated', async () => { + routing.activateAndRevealWorkspace.mockReturnValue(false) + const openNewTerminal = await clickOpenNewTerminal('folder:unmounted') + expect(openNewTerminal).not.toHaveBeenCalled() + }) + + it('reveals the floating panel and opens a floating terminal', async () => { + const openNewTerminal = await clickOpenNewTerminal(FLOATING_TERMINAL_WORKTREE_ID) + expect(routing.revealFloatingWorkspacePanel).toHaveBeenCalledTimes(1) + expect(routing.createFloatingWorkspaceTerminalTab).toHaveBeenCalledTimes(1) + expect(routing.activateAndRevealWorkspace).not.toHaveBeenCalled() + expect(openNewTerminal).not.toHaveBeenCalled() + }) + }) + + describe('returning focus when a dialog closes', () => { + // Why a real xterm textarea: the dialogs have no trigger, so only the hook can restore focus. + function mountActiveTerminal(): HTMLTextAreaElement { + useAppStore.setState({ + activeWorktreeId: 'wt-1', + activeTabType: 'terminal', + activeTabIdByWorktree: { 'wt-1': TAB_ID }, + terminalLayoutsByTabId: { + [TAB_ID]: { root: null, activeLeafId: LEAF_ID, expandedLeafId: null } + } + }) + const tab = document.createElement('div') + tab.dataset.terminalTabId = TAB_ID + const leaf = document.createElement('div') + leaf.dataset.leafId = LEAF_ID + const xterm = document.createElement('textarea') + xterm.className = 'xterm-helper-textarea' + leaf.append(xterm) + tab.append(leaf) + document.body.append(tab) + xterm.focus() + return xterm + } + + async function pressEscape(): Promise { + await act(async () => { + ;(document.activeElement ?? document.body).dispatchEvent( + new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }) + ) + }) + await advance(100) + } + + afterEach(() => { + document.querySelector('[data-terminal-tab-id]')?.remove() + }) + + it.each([ + ['Fix', JOINED, 'Fix'], + ['old-terminal Learn more', OLD_TAB_JOINED, 'Learn more'] + ])( + 'returns focus to the terminal after Esc closes the %s dialog', + async (_l, status, label) => { + setState({}) + isCodexOnSharedServer.mockResolvedValue(status) + const xterm = mountActiveTerminal() + await renderBanner() + await advance(1_000) + await act(async () => button(label).click()) + expect(document.querySelector('[role="dialog"]')).not.toBeNull() + expect(document.activeElement).not.toBe(xterm) + + await pressEscape() + + expect(document.querySelector('[role="dialog"]')).toBeNull() + expect(document.activeElement).toBe(xterm) + } + ) + + it('leaves focus in the new terminal after Open new terminal', async () => { + setState({}) + const xterm = mountActiveTerminal() + const newTerminal = document.createElement('textarea') + document.body.append(newTerminal) + useAppStore.setState({ + unifiedTabsByWorktree: { + 'wt-1': [ + { + id: 'unified-1', + entityId: TAB_ID, + groupId: 'group-1', + worktreeId: 'wt-1', + contentType: 'terminal', + label: 'Terminal', + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 0 + } + ] + }, + openNewTerminalTabInActiveWorkspace: vi.fn(() => { + newTerminal.focus() + return Promise.resolve() + }) + }) + isCodexOnSharedServer.mockResolvedValue(OLD_TAB_JOINED) + await renderBanner() + await advance(1_000) + await act(async () => button('Learn more').click()) + await clickDialogOpenNewTerminal() + + expect(document.querySelector('[role="dialog"]')).toBeNull() + expect(document.activeElement).toBe(newTerminal) + expect(document.activeElement).not.toBe(xterm) + newTerminal.remove() + }) + + async function clickDialogOpenNewTerminal(): Promise { + const inDialog = Array.from( + document.querySelector('[role="dialog"]')?.querySelectorAll('button') ?? [] + ).find((candidate) => candidate.textContent?.trim() === 'Open new terminal') + await act(async () => inDialog?.click()) + await advance(100) + } + + it('returns focus to this terminal when Open new terminal cannot open one', async () => { + setState({}) + useAppStore.setState({ unifiedTabsByWorktree: {} }) + const xterm = mountActiveTerminal() + isCodexOnSharedServer.mockResolvedValue(OLD_TAB_JOINED) + await renderBanner() + await advance(1_000) + await act(async () => button('Learn more').click()) + + await clickDialogOpenNewTerminal() + + expect(document.querySelector('[role="dialog"]')).toBeNull() + expect(document.activeElement).toBe(xterm) + }) + + it('keeps the Learn more dialog open when this Codex ends, then returns focus', async () => { + setState({}) + const xterm = mountActiveTerminal() + isCodexOnSharedServer.mockResolvedValue(OLD_TAB_JOINED) + await renderBanner() + await advance(1_000) + await act(async () => button('Learn more').click()) + + setState({}, null) + await advance(100) + expect(document.querySelector('[role="dialog"]')).not.toBeNull() + expect(paneElement.textContent).toBe('') + + await pressEscape() + + expect(document.querySelector('[role="dialog"]')).toBeNull() + expect(document.activeElement).toBe(xterm) + }) + + it('returns focus to the pane terminal after Stop server ends its Codex', async () => { + setState({}) + const xterm = mountActiveTerminal() + await renderBanner() + await advance(1_000) + await act(async () => button('Fix').click()) + await act(async () => button('Turn off').click()) + await act(async () => button('Stop server').click()) + isCodexOnSharedServer.mockResolvedValue(NOT_JOINED) + const confirm = Array.from(document.querySelectorAll('button')).filter( + (candidate) => candidate.textContent?.trim() === 'Stop server' + ) + await act(async () => confirm.at(-1)?.click()) + await advance(20_000) + expect(stopCodexSharedServer).toHaveBeenCalledWith(ptyId) + expect(document.body.textContent).toContain('Stopped') + + await pressEscape() + + expect(document.querySelector('[role="dialog"]')).toBeNull() + expect(paneElement.textContent).toBe('') + expect(document.activeElement).toBe(xterm) + }) + }) }) diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 2f29e560803..523e7d7dbe2 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -18657,7 +18657,12 @@ "codexSharedServerBanner": { "title": "This Codex is sharing a server with your other Codex tabs", "body": "Sessions may end unexpectedly, and agent status may be wrong.", + "openedBeforeUpdateBody": "This terminal was opened before Orca started giving each Codex its own server.", + "oldTerminalDialogTitle": "Why this Codex shares a server", + "oldTerminalDialogRisk": "Codex sessions that share a server can end together, and agent status can be wrong.", + "oldTerminalDialogCause": "Older terminals still share a Codex server. Open a new terminal to run Codex on a separate server.", "fix": "Fix", + "openNewTerminal": "Open new terminal", "dontShowAgain": "Don't show again", "dismiss": "Dismiss", "dialogTitle": "Give each Codex tab its own server", diff --git a/src/renderer/src/lib/floating-workspace-panel-reveal.ts b/src/renderer/src/lib/floating-workspace-panel-reveal.ts new file mode 100644 index 00000000000..af3a6e82314 --- /dev/null +++ b/src/renderer/src/lib/floating-workspace-panel-reveal.ts @@ -0,0 +1,24 @@ +import type { AppState } from '@/store/types' +import { TOGGLE_FLOATING_TERMINAL_EVENT } from './floating-terminal' +import { isFloatingWorkspacePanelVisible } from './floating-workspace-terminal-actions' + +function toggleFloatingWorkspacePanelIfHidden(): void { + if (!isFloatingWorkspacePanelVisible()) { + window.dispatchEvent(new Event(TOGGLE_FLOATING_TERMINAL_EVENT)) + } +} + +// Why enable first: floating tabs outlive a feature disable, and the panel ignores the toggle +// while disabled, so a live floating agent row would otherwise be a silent no-op. +export function revealFloatingWorkspacePanel( + state: Pick +): void { + if (state.settings?.floatingTerminalEnabled === true) { + toggleFloatingWorkspacePanelIfHidden() + return + } + void state.updateSettings({ floatingTerminalEnabled: true }).then(() => { + // Why deferred a frame: the panel only honors the toggle once the enabled flag has reached React. + requestAnimationFrame(toggleFloatingWorkspacePanelIfHidden) + }) +} diff --git a/src/renderer/src/web/preload-api/web-terminal-api.ts b/src/renderer/src/web/preload-api/web-terminal-api.ts index c4bf707ea3c..55c0ec364eb 100644 --- a/src/renderer/src/web/preload-api/web-terminal-api.ts +++ b/src/renderer/src/web/preload-api/web-terminal-api.ts @@ -41,7 +41,7 @@ export function createPtyApi(): NonNullable['pty']> { // Why: paired web panes cannot provide a local post-boundary process scan. confirmForegroundProcess: () => Promise.resolve(null), // Why: a paired client's terminals belong to the host, whose Codex settings this client does not own. - isCodexOnSharedServer: () => Promise.resolve(false), + isCodexOnSharedServer: () => Promise.resolve({ joined: false }), disableCodexSharedServerAutoStart: () => Promise.resolve(false), stopCodexSharedServer: () => Promise.resolve(false), getCwd: () => Promise.resolve('~'), diff --git a/src/shared/codex-shared-server-command.ts b/src/shared/codex-shared-server-command.ts index 74c242b4e3b..51343d3d75a 100644 --- a/src/shared/codex-shared-server-command.ts +++ b/src/shared/codex-shared-server-command.ts @@ -120,3 +120,12 @@ export const CODEX_STOP_SHARED_SERVER_ARGS = ['app-server', 'daemon', 'stop'] as /** What the fix dialog shows the user it runs. */ export const CODEX_DISABLE_AUTO_START_COMMAND = `codex ${CODEX_DISABLE_SHARED_SERVER_ARGS.join(' ')}` export const CODEX_STOP_SHARED_SERVER_COMMAND = `codex ${CODEX_STOP_SHARED_SERVER_ARGS.join(' ')}` + +/** Whether a local pane's Codex is a client of Codex's shared server. */ +export type CodexSharedServerStatus = + | { joined: false } + | { + joined: true + /** The pane's shell lacks the codex function a new terminal would give it. */ + openedBeforeWrapper: boolean + }