mirror of
https://github.com/stablyai/orca.git
synced 2026-10-19 16:03:26 +00:00
fix(codex): drain queued restarts on real pane binds and keep the launch account across reattach
The pending-restart effect keyed off ptyIdsByTabId, which a restored pane already lists before it ever mounts, so an unmounted pane's queued restart never drained. Key off the pane->PTY layout bindings, which bindActivePanePty and the reattach path both rewrite when a PTY actually binds. pty:spawn re-records a pane's launch account on every daemon reattach, but a reattached shell keeps the CODEX_HOME baked in at its original spawn. Recording the current selection there erased the only evidence the pane was stale, so the startup sweep found nothing to prompt about. Record after the spawn resolves and skip reattaches.
This commit is contained in:
@@ -58,7 +58,9 @@ const {
|
||||
setMigrationUnsupportedPtyMock,
|
||||
clearMigrationUnsupportedPtyMock,
|
||||
clearMigrationUnsupportedPtysForPaneKeyMock,
|
||||
clearPaneKeyAliasesForPtyMock
|
||||
clearPaneKeyAliasesForPtyMock,
|
||||
recordCodexPaneAccountMock,
|
||||
forgetCodexPaneAccountMock
|
||||
} = vi.hoisted(() => ({
|
||||
handleMock: vi.fn(),
|
||||
onMock: vi.fn(),
|
||||
@@ -90,7 +92,9 @@ const {
|
||||
setMigrationUnsupportedPtyMock: vi.fn(),
|
||||
clearMigrationUnsupportedPtyMock: vi.fn(),
|
||||
clearMigrationUnsupportedPtysForPaneKeyMock: vi.fn(),
|
||||
clearPaneKeyAliasesForPtyMock: vi.fn()
|
||||
clearPaneKeyAliasesForPtyMock: vi.fn(),
|
||||
recordCodexPaneAccountMock: vi.fn(),
|
||||
forgetCodexPaneAccountMock: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('electron', () => ({
|
||||
@@ -193,6 +197,11 @@ vi.mock('../agent-hooks/migration-unsupported-pty-state', () => ({
|
||||
clearMigrationUnsupportedPty: clearMigrationUnsupportedPtyMock,
|
||||
clearMigrationUnsupportedPtysForPaneKey: clearMigrationUnsupportedPtysForPaneKeyMock
|
||||
}))
|
||||
|
||||
vi.mock('../codex/codex-pane-account-registry', () => ({
|
||||
recordCodexPaneAccount: recordCodexPaneAccountMock,
|
||||
forgetCodexPaneAccount: forgetCodexPaneAccountMock
|
||||
}))
|
||||
import { LocalPtyProvider } from '../providers/local-pty-provider'
|
||||
import { makePaneKey } from '../../shared/stable-pane-id'
|
||||
import { SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV } from '../../shared/setup-agent-sequencing'
|
||||
@@ -348,6 +357,8 @@ describe('registerPtyHandlers', () => {
|
||||
clearMigrationUnsupportedPtyMock.mockReset()
|
||||
clearMigrationUnsupportedPtysForPaneKeyMock.mockReset()
|
||||
clearPaneKeyAliasesForPtyMock.mockReset()
|
||||
recordCodexPaneAccountMock.mockReset()
|
||||
forgetCodexPaneAccountMock.mockReset()
|
||||
mainWindow.webContents.on.mockReset()
|
||||
mainWindow.webContents.send.mockReset()
|
||||
mainWindow.webContents.removeListener.mockReset()
|
||||
@@ -13504,6 +13515,36 @@ describe('registerPtyHandlers', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('records the launch Codex account for a fresh spawn but not for a reattach', async () => {
|
||||
const spawn = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce({ id: 'pty-fresh' })
|
||||
.mockResolvedValueOnce({ id: 'pty-reattached', isReattach: true })
|
||||
setLocalPtyProvider({
|
||||
spawn,
|
||||
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)
|
||||
const getSettings = vi.fn().mockReturnValue({ activeCodexManagedAccountId: 'account-a' })
|
||||
registerPtyHandlers(mainWindow as never, undefined, undefined, getSettings as never)
|
||||
|
||||
await handlers.get('pty:spawn')!(null, { cols: 80, rows: 24 })
|
||||
await handlers.get('pty:spawn')!(null, { cols: 80, rows: 24, sessionId: 'pty-reattached' })
|
||||
|
||||
// Why: a reattached shell keeps the CODEX_HOME baked in at its original
|
||||
// spawn, so re-recording it under the current selection would erase the only
|
||||
// evidence that the pane is stale.
|
||||
expect(recordCodexPaneAccountMock.mock.calls).toEqual([
|
||||
['pty-fresh', { selectionKey: 'host', accountId: 'account-a' }]
|
||||
])
|
||||
})
|
||||
|
||||
it('seeds cold restore at recovered dimensions with a legacy dimensionless fallback', async () => {
|
||||
const oscLinks = [{ row: 0, startCol: 0, endCol: 8, uri: 'https://example.com/restored' }]
|
||||
const coldRestore = {
|
||||
|
||||
+22
-17
@@ -830,16 +830,19 @@ function getCompatibleSelectedCodexHomePath(
|
||||
// Why: CODEX_HOME is fixed in a shell's environment at spawn and the daemon
|
||||
// keeps that shell alive across app restarts, so the launch account is the only
|
||||
// way to tell later that a pane still runs Codex as the previously selected
|
||||
// account. A resume-pinned pane is deliberately on its session's own account,
|
||||
// so it is forgotten rather than recorded as stale.
|
||||
// account. A reattach inherits that baked environment rather than choosing one,
|
||||
// so re-recording it under the current selection would erase the very evidence
|
||||
// that the pane is stale. A resume-pinned pane is deliberately on its session's
|
||||
// own account, so it is forgotten rather than recorded as stale.
|
||||
function recordCodexPaneAccountForSpawn(args: {
|
||||
ptyId: string | undefined
|
||||
isDaemonHostSpawn: boolean
|
||||
isReattach: boolean
|
||||
pinnedByResume: boolean
|
||||
target: CodexAccountSelectionTarget
|
||||
settings: GlobalSettings | undefined
|
||||
}): void {
|
||||
if (!args.ptyId || !args.isDaemonHostSpawn) {
|
||||
if (!args.ptyId || !args.isDaemonHostSpawn || args.isReattach) {
|
||||
return
|
||||
}
|
||||
if (args.pinnedByResume || !args.settings) {
|
||||
@@ -3275,13 +3278,6 @@ export function registerPtyHandlers(
|
||||
}) ?? null)
|
||||
)
|
||||
: null
|
||||
recordCodexPaneAccountForSpawn({
|
||||
ptyId: effectiveSessionAppId ?? sessionId,
|
||||
isDaemonHostSpawn,
|
||||
pinnedByResume: Boolean(codexResumeHome),
|
||||
target: codexSelectionTarget,
|
||||
settings: getSettings?.()
|
||||
})
|
||||
const skipCodexHomeEnv =
|
||||
isDaemonHostSpawn &&
|
||||
shouldSkipCodexHomeEnvForWindowsShell(daemonShellOverride, cwd) &&
|
||||
@@ -3698,6 +3694,14 @@ export function registerPtyHandlers(
|
||||
if (effectiveSessionAppId !== undefined && effectiveSessionAppId !== result.id) {
|
||||
ptySizes.delete(effectiveSessionAppId)
|
||||
}
|
||||
recordCodexPaneAccountForSpawn({
|
||||
ptyId: result.id,
|
||||
isDaemonHostSpawn,
|
||||
isReattach: result.isReattach === true,
|
||||
pinnedByResume: Boolean(codexResumeHome),
|
||||
target: codexSelectionTarget,
|
||||
settings: getSettings?.()
|
||||
})
|
||||
if (hostSessionBinding) {
|
||||
try {
|
||||
const binding = {
|
||||
@@ -4431,13 +4435,6 @@ export function registerPtyHandlers(
|
||||
}) ?? null)
|
||||
)
|
||||
: null
|
||||
recordCodexPaneAccountForSpawn({
|
||||
ptyId: effectiveSessionAppId ?? effectiveSessionId,
|
||||
isDaemonHostSpawn,
|
||||
pinnedByResume: Boolean(codexResumeHome),
|
||||
target: codexSelectionTarget,
|
||||
settings: getSettings?.()
|
||||
})
|
||||
const skipCodexHomeEnv =
|
||||
isDaemonHostSpawn &&
|
||||
shouldSkipCodexHomeEnvForWindowsShell(effectiveShellOverride, cwd) &&
|
||||
@@ -4739,6 +4736,14 @@ export function registerPtyHandlers(
|
||||
daemon: isDaemonHostSpawn,
|
||||
reattach: result.isReattach ?? false
|
||||
})
|
||||
recordCodexPaneAccountForSpawn({
|
||||
ptyId: result.id,
|
||||
isDaemonHostSpawn,
|
||||
isReattach: result.isReattach === true,
|
||||
pinnedByResume: Boolean(codexResumeHome),
|
||||
target: codexSelectionTarget,
|
||||
settings: getSettings?.()
|
||||
})
|
||||
ptyOwnership.set(result.id, args.connectionId ?? null)
|
||||
if (result.incarnationId) {
|
||||
ptyIncarnationById.set(result.id, result.incarnationId)
|
||||
|
||||
@@ -574,10 +574,6 @@ export default function TerminalPane({
|
||||
const setTabCanExpandPane = useAppStore((store) => store.setTabCanExpandPane)
|
||||
const suppressPtyExit = useAppStore((store) => store.suppressPtyExit)
|
||||
const pendingCodexPaneRestartIds = useAppStore((store) => store.pendingCodexPaneRestartIds)
|
||||
// Why: a parked or deferred tab mounts with no transport yet, so the queued
|
||||
// restart below has no ptyId to match on the mount pass. This re-runs it once
|
||||
// the reconnected PTY is bound, which is the only moment the pane can act.
|
||||
const attachedPtyIds = useAppStore((store) => store.ptyIdsByTabId[tabId])
|
||||
const consumePendingCodexPaneRestart = useAppStore(
|
||||
(store) => store.consumePendingCodexPaneRestart
|
||||
)
|
||||
@@ -1675,6 +1671,11 @@ export default function TerminalPane({
|
||||
]
|
||||
)
|
||||
|
||||
// Why leaf bindings are a dep: a parked or deferred tab mounts with no
|
||||
// transport, so a queued restart has no ptyId to match on the mount pass. The
|
||||
// reconnected PTY rewrites this map when it binds — `ptyIdsByTabId` does not,
|
||||
// because a restored id is already listed there before the pane ever mounts.
|
||||
const panePtyLayoutBindings = savedLayout.ptyIdsByLeafId
|
||||
useEffect(() => {
|
||||
const manager = managerRef.current
|
||||
if (!manager) {
|
||||
@@ -1692,9 +1693,9 @@ export default function TerminalPane({
|
||||
}
|
||||
}
|
||||
}, [
|
||||
attachedPtyIds,
|
||||
consumePendingCodexPaneRestart,
|
||||
handleRestartCodexPane,
|
||||
panePtyLayoutBindings,
|
||||
pendingCodexPaneRestartIds
|
||||
])
|
||||
|
||||
|
||||
Reference in New Issue
Block a user