mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix: preserve native PTY context on reattach
This commit is contained in:
@@ -8,17 +8,24 @@ export type DaemonCreateOrAttachResult = {
|
||||
shellState: ShellReadyState
|
||||
historySeeded?: boolean
|
||||
launchAgent?: TuiAgent
|
||||
wslDistro?: string
|
||||
/** Undefined only when talking to a daemon predating WSL session context. */
|
||||
wslDistro?: string | null
|
||||
}
|
||||
|
||||
export function getDaemonSessionResultMetadata(session: {
|
||||
launchAgent: TuiAgent | null
|
||||
historySeeded: boolean | undefined
|
||||
wslDistro: string | null
|
||||
}): Pick<DaemonCreateOrAttachResult, 'launchAgent' | 'historySeeded' | 'wslDistro'> {
|
||||
}): {
|
||||
launchAgent?: TuiAgent
|
||||
historySeeded?: boolean
|
||||
wslDistro: string | null
|
||||
} {
|
||||
return {
|
||||
...(session.launchAgent ? { launchAgent: session.launchAgent } : {}),
|
||||
...(session.historySeeded !== undefined ? { historySeeded: session.historySeeded } : {}),
|
||||
...(session.wslDistro ? { wslDistro: session.wslDistro } : {})
|
||||
// Why: null authoritatively identifies a native session; omission is
|
||||
// reserved for older daemons that predate this wire field.
|
||||
wslDistro: session.wslDistro
|
||||
}
|
||||
}
|
||||
|
||||
@@ -149,6 +149,38 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => {
|
||||
expect(result.id).toContain('wt-1')
|
||||
})
|
||||
|
||||
it('keeps a reattached native UNC session native despite a conflicting WSL preference', async () => {
|
||||
const platform = Object.getOwnPropertyDescriptor(process, 'platform')
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||||
try {
|
||||
const sessionId = 'native-conflicting-wsl-attach'
|
||||
const created = await adapter.spawn({
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
sessionId,
|
||||
cwd: '\\\\server\\share\\repo',
|
||||
shellOverride: 'powershell.exe'
|
||||
})
|
||||
const attached = await adapter.spawn({
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
sessionId,
|
||||
cwd: 'C:\\repo',
|
||||
shellOverride: 'wsl.exe',
|
||||
terminalWindowsWslDistro: 'Ubuntu'
|
||||
})
|
||||
|
||||
expect(created.wslDistro).toBeNull()
|
||||
expect(attached.wslDistro).toBeNull()
|
||||
expect(attached.isReattach).toBe(true)
|
||||
expect(lastSpawnOpts?.cwd).toBe('\\\\server\\share\\repo')
|
||||
} finally {
|
||||
if (platform) {
|
||||
Object.defineProperty(process, 'platform', platform)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
itOnPosix('keeps plain Codex startup on the short daemon shell-ready timeout', async () => {
|
||||
await adapter.spawn({
|
||||
cols: 80,
|
||||
|
||||
@@ -301,10 +301,13 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
|
||||
let scrollback = restoreInfo ? getRecoveredHistorySeed(restoreInfo) : null
|
||||
let result = await createOrAttach(scrollback)
|
||||
wslDistro = result.wslDistro ?? wslDistro
|
||||
let providerWslDistro = result.wslDistro === undefined ? wslDistro : result.wslDistro
|
||||
// Why: explicit null from a current daemon overrides the caller's WSL
|
||||
// preference; undefined preserves compatibility with older daemons.
|
||||
wslDistro = providerWslDistro ?? undefined
|
||||
if (wslDistro) {
|
||||
this.wslDistrosBySessionId.set(sessionId, wslDistro)
|
||||
} else if (result.isNew) {
|
||||
} else if (providerWslDistro === null || result.isNew) {
|
||||
this.wslDistrosBySessionId.delete(sessionId)
|
||||
}
|
||||
const launchIdentity = (): { launchAgent?: NonNullable<typeof result.launchAgent> } =>
|
||||
@@ -337,7 +340,7 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
pid,
|
||||
...launchIdentity(),
|
||||
coldRestore: cachedRestore,
|
||||
...(wslDistro ? { wslDistro } : {}),
|
||||
...(providerWslDistro !== undefined ? { wslDistro: providerWslDistro } : {}),
|
||||
...(!result.isNew ? { isReattach: true } : {})
|
||||
}
|
||||
}
|
||||
@@ -361,10 +364,11 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
effectiveCols = restoreInfo.cols
|
||||
effectiveRows = restoreInfo.rows
|
||||
result = await createOrAttach(scrollback)
|
||||
wslDistro = result.wslDistro ?? wslDistro
|
||||
providerWslDistro = result.wslDistro === undefined ? wslDistro : result.wslDistro
|
||||
wslDistro = providerWslDistro ?? undefined
|
||||
if (wslDistro) {
|
||||
this.wslDistrosBySessionId.set(sessionId, wslDistro)
|
||||
} else if (result.isNew) {
|
||||
} else if (providerWslDistro === null || result.isNew) {
|
||||
this.wslDistrosBySessionId.delete(sessionId)
|
||||
}
|
||||
pid = typeof result.pid === 'number' && result.pid > 0 ? result.pid : null
|
||||
@@ -409,7 +413,7 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
pid,
|
||||
...launchIdentity(),
|
||||
coldRestore,
|
||||
...(wslDistro ? { wslDistro } : {}),
|
||||
...(providerWslDistro !== undefined ? { wslDistro: providerWslDistro } : {}),
|
||||
...(providerSequence ? { providerSequence } : {}),
|
||||
...(!result.isNew ? { isReattach: true } : {})
|
||||
}
|
||||
@@ -418,7 +422,7 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
id: sessionId,
|
||||
pid,
|
||||
...launchIdentity(),
|
||||
...(wslDistro ? { wslDistro } : {}),
|
||||
...(providerWslDistro !== undefined ? { wslDistro: providerWslDistro } : {}),
|
||||
...(providerSequence ? { providerSequence } : {})
|
||||
}
|
||||
}
|
||||
@@ -458,7 +462,7 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
id: sessionId,
|
||||
pid,
|
||||
...launchIdentity(),
|
||||
...(wslDistro ? { wslDistro } : {}),
|
||||
...(providerWslDistro !== undefined ? { wslDistro: providerWslDistro } : {}),
|
||||
...(providerSequence ? { providerSequence } : {}),
|
||||
...(isReattach ? { isReattach: true } : {})
|
||||
}
|
||||
@@ -478,7 +482,7 @@ export class DaemonPtyAdapter implements IPtyProvider {
|
||||
id: sessionId,
|
||||
pid,
|
||||
...launchIdentity(),
|
||||
...(wslDistro ? { wslDistro } : {}),
|
||||
...(providerWslDistro !== undefined ? { wslDistro: providerWslDistro } : {}),
|
||||
snapshot: snapshotPayload,
|
||||
snapshotCols: result.snapshot.cols,
|
||||
snapshotRows: result.snapshot.rows,
|
||||
|
||||
@@ -467,7 +467,7 @@ export class DaemonServer {
|
||||
pid: result.pid,
|
||||
shellState: result.shellState,
|
||||
...(result.launchAgent ? { launchAgent: result.launchAgent } : {}),
|
||||
...(result.wslDistro ? { wslDistro: result.wslDistro } : {}),
|
||||
wslDistro: result.wslDistro,
|
||||
...(result.historySeeded !== undefined ? { historySeeded: result.historySeeded } : {})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -29,6 +29,6 @@ export type CreateOrAttachResult = {
|
||||
shellState: ShellReadyState
|
||||
historySeeded?: boolean
|
||||
launchAgent?: TuiAgent
|
||||
wslDistro?: string
|
||||
wslDistro: string | null
|
||||
attachToken: symbol
|
||||
}
|
||||
|
||||
@@ -60,6 +60,39 @@ describe('TerminalHost WSL context', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('returns authoritative null when a native session is attached with a WSL preference', async () => {
|
||||
const spawnSubprocess = vi.fn(() => createSubprocess())
|
||||
host = new TerminalHost({ spawnSubprocess })
|
||||
const platform = Object.getOwnPropertyDescriptor(process, 'platform')
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||||
try {
|
||||
const created = await host.createOrAttach({
|
||||
sessionId: 'session-native',
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
cwd: '\\\\server\\share\\repo',
|
||||
shellOverride: 'powershell.exe',
|
||||
streamClient: { onData: vi.fn(), onExit: vi.fn() }
|
||||
})
|
||||
const attached = await host.createOrAttach({
|
||||
sessionId: 'session-native',
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
shellOverride: 'wsl.exe',
|
||||
terminalWindowsWslDistro: 'Ubuntu',
|
||||
streamClient: { onData: vi.fn(), onExit: vi.fn() }
|
||||
})
|
||||
|
||||
expect(created.wslDistro).toBeNull()
|
||||
expect(attached.wslDistro).toBeNull()
|
||||
expect(spawnSubprocess).toHaveBeenCalledOnce()
|
||||
} finally {
|
||||
if (platform) {
|
||||
Object.defineProperty(process, 'platform', platform)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
it('uses a remembered distro only when the selected shell is WSL', () => {
|
||||
const platform = Object.getOwnPropertyDescriptor(process, 'platform')
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||||
|
||||
@@ -1414,14 +1414,18 @@ describe('registerPtyHandlers', () => {
|
||||
// OpenCode plugin dir, Pi managed extension env, Codex home, and dev-mode CLI
|
||||
// overrides were silently missing for daemon users (the common case).
|
||||
|
||||
function setupDaemonAdapter(supportsGitCredentialGuardHost = true) {
|
||||
function setupDaemonAdapter(
|
||||
supportsGitCredentialGuardHost = true,
|
||||
reportedWslDistro?: string | null
|
||||
) {
|
||||
const daemonSpawn = vi.fn(
|
||||
async (options: {
|
||||
env: Record<string, string>
|
||||
sessionId?: string
|
||||
isNewSession?: boolean
|
||||
}) => ({
|
||||
id: options.sessionId ?? 'daemon-pty'
|
||||
id: options.sessionId ?? 'daemon-pty',
|
||||
...(reportedWslDistro !== undefined ? { wslDistro: reportedWslDistro } : {})
|
||||
})
|
||||
)
|
||||
setLocalPtyProvider({
|
||||
@@ -1934,6 +1938,67 @@ describe('registerPtyHandlers', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('distinguishes an attached native context from an older daemon fallback', async () => {
|
||||
await withWin32Platform(async () => {
|
||||
_setWslCachesForTests({ available: true, distros: ['Ubuntu'] })
|
||||
const settings = {
|
||||
localWindowsRuntimeDefault: { kind: 'wsl', distro: 'Ubuntu' },
|
||||
terminalWindowsShell: 'wsl.exe',
|
||||
terminalWindowsWslDistro: 'Ubuntu',
|
||||
terminalWindowsPowerShellImplementation: 'auto'
|
||||
}
|
||||
const cases: {
|
||||
reportedWslDistro: string | null | undefined
|
||||
expectedWslDistro: string | null
|
||||
sessionId: string
|
||||
}[] = [
|
||||
{
|
||||
reportedWslDistro: null,
|
||||
expectedWslDistro: null,
|
||||
sessionId: 'native-session'
|
||||
},
|
||||
{
|
||||
reportedWslDistro: undefined,
|
||||
expectedWslDistro: 'Ubuntu',
|
||||
sessionId: 'older-daemon-session'
|
||||
}
|
||||
]
|
||||
|
||||
for (const testCase of cases) {
|
||||
setupDaemonAdapter(true, testCase.reportedWslDistro)
|
||||
const runtime = {
|
||||
setPtyController: vi.fn(),
|
||||
createPreAllocatedTerminalHandle: vi.fn(() => null),
|
||||
preAllocateHandleForPty: vi.fn(),
|
||||
registerPty: vi.fn(),
|
||||
onPtySpawned: vi.fn(),
|
||||
onPtyExit: vi.fn(),
|
||||
onPtyData: vi.fn(),
|
||||
preparePtyExecutionContext: vi.fn().mockReturnValue(true)
|
||||
}
|
||||
handlers.clear()
|
||||
registerPtyHandlers(
|
||||
mainWindow as never,
|
||||
runtime as never,
|
||||
undefined,
|
||||
(() => settings) as never
|
||||
)
|
||||
|
||||
await handlers.get('pty:spawn')!(null, {
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
sessionId: testCase.sessionId,
|
||||
cwd: '\\\\server\\share\\repo'
|
||||
})
|
||||
|
||||
expect(runtime.preparePtyExecutionContext).toHaveBeenLastCalledWith(
|
||||
testCase.sessionId,
|
||||
testCase.expectedWslDistro
|
||||
)
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
it('blocks runtime-created daemon PTYs when project WSL runtime requires repair', async () => {
|
||||
await withWin32Platform(async () => {
|
||||
_setWslCachesForTests({ available: true, distros: ['Debian'] })
|
||||
|
||||
+10
-2
@@ -3235,7 +3235,11 @@ export function registerPtyHandlers(
|
||||
}
|
||||
runtime?.preparePtyExecutionContext?.(
|
||||
result.id,
|
||||
args.connectionId ? null : (result.wslDistro ?? expectedWslDistro)
|
||||
args.connectionId
|
||||
? null
|
||||
: result.wslDistro === undefined
|
||||
? expectedWslDistro
|
||||
: result.wslDistro
|
||||
)
|
||||
} catch (err) {
|
||||
if ((isMintedSessionId || preparedProvisionalExecutionContext) && effectiveSessionAppId) {
|
||||
@@ -4228,7 +4232,11 @@ export function registerPtyHandlers(
|
||||
}
|
||||
runtime?.preparePtyExecutionContext?.(
|
||||
result.id,
|
||||
args.connectionId ? null : (result.wslDistro ?? expectedWslDistro)
|
||||
args.connectionId
|
||||
? null
|
||||
: result.wslDistro === undefined
|
||||
? expectedWslDistro
|
||||
: result.wslDistro
|
||||
)
|
||||
spawnTiming.mark('provider_spawn')
|
||||
} catch (err) {
|
||||
|
||||
@@ -228,11 +228,63 @@ describe('LocalPtyProvider', () => {
|
||||
|
||||
const second = await provider.spawn({ cols: 120, rows: 40, sessionId: first.id })
|
||||
|
||||
expect(second).toEqual({ id: 'serve-session-1', pid: 12345, isReattach: true })
|
||||
expect(second).toEqual({
|
||||
id: 'serve-session-1',
|
||||
pid: 12345,
|
||||
wslDistro: null,
|
||||
isReattach: true
|
||||
})
|
||||
expect(mockProc.resize).toHaveBeenCalledWith(120, 40)
|
||||
expect(spawnMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('keeps a native UNC session native on a conflicting WSL reattach', async () => {
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||||
const first = await provider.spawn({
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
sessionId: 'native-session',
|
||||
cwd: '\\\\server\\share\\repo',
|
||||
shellOverride: 'powershell.exe'
|
||||
})
|
||||
spawnMock.mockClear()
|
||||
|
||||
const second = await provider.spawn({
|
||||
cols: 120,
|
||||
rows: 40,
|
||||
sessionId: first.id,
|
||||
shellOverride: 'wsl.exe',
|
||||
terminalWindowsWslDistro: 'Ubuntu'
|
||||
})
|
||||
|
||||
expect(first.wslDistro).toBeNull()
|
||||
expect(second.wslDistro).toBeNull()
|
||||
expect(spawnMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('keeps the first WSL distro on a conflicting distro reattach', async () => {
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||||
const first = await provider.spawn({
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
sessionId: 'wsl-session',
|
||||
cwd: '\\\\wsl.localhost\\Ubuntu\\home\\jin\\repo'
|
||||
})
|
||||
spawnMock.mockClear()
|
||||
|
||||
const second = await provider.spawn({
|
||||
cols: 120,
|
||||
rows: 40,
|
||||
sessionId: first.id,
|
||||
shellOverride: 'wsl.exe',
|
||||
terminalWindowsWslDistro: 'Debian'
|
||||
})
|
||||
|
||||
expect(first.wslDistro).toBe('Ubuntu')
|
||||
expect(second.wslDistro).toBe('Ubuntu')
|
||||
expect(spawnMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('does not reattach numeric caller session ids that can collide after restart', async () => {
|
||||
const first = await provider.spawn({ cols: 80, rows: 24 })
|
||||
spawnMock.mockClear()
|
||||
|
||||
@@ -103,6 +103,9 @@ const ptyAgentForegroundContextPaths = new Map<string, string[]>()
|
||||
const ptyLastRecognizedForeground = new Map<string, string>()
|
||||
const ptyTerminalHandle = new Map<string, string>()
|
||||
const ptyInitialCwd = new Map<string, string>()
|
||||
// Why: reattach requests carry current settings, not the live process's launch
|
||||
// context. Keep the first creator's WSL/native identity for the PTY incarnation.
|
||||
const ptyWslDistroById = new Map<string, string | null>()
|
||||
// Why: node-pty callbacks must be disposed before environment teardown, but
|
||||
// onExit separately owns physical process-exit proof during termination.
|
||||
const ptyDisposables = new Map<string, { dispose: () => void }[]>()
|
||||
@@ -239,6 +242,7 @@ function clearPtyState(id: string): void {
|
||||
ptyLastRecognizedForeground.delete(id)
|
||||
ptyTerminalHandle.delete(id)
|
||||
ptyInitialCwd.delete(id)
|
||||
ptyWslDistroById.delete(id)
|
||||
ptyLoadGeneration.delete(id)
|
||||
ptyTerminationMode.delete(id)
|
||||
ptyPhysicalExits.delete(id)
|
||||
@@ -518,12 +522,18 @@ export class LocalPtyProvider implements IPtyProvider {
|
||||
}
|
||||
const existing = ptyProcesses.get(reattachId)
|
||||
if (existing) {
|
||||
const existingWslDistro = ptyWslDistroById.get(reattachId)
|
||||
try {
|
||||
existing.resize(args.cols, args.rows)
|
||||
} catch {
|
||||
/* Existing PTY may reject resize during teardown; still return the live handle. */
|
||||
}
|
||||
return { id: reattachId, pid: existing.pid, isReattach: true }
|
||||
return {
|
||||
id: reattachId,
|
||||
pid: existing.pid,
|
||||
...(ptyWslDistroById.has(reattachId) ? { wslDistro: existingWslDistro ?? null } : {}),
|
||||
isReattach: true
|
||||
}
|
||||
}
|
||||
}
|
||||
const id = allocatePtyId(reattachId ?? undefined)
|
||||
@@ -871,9 +881,15 @@ export class LocalPtyProvider implements IPtyProvider {
|
||||
}
|
||||
|
||||
const proc = spawnResult.process
|
||||
const spawnedShellIsWsl =
|
||||
process.platform === 'win32' && pathWin32.basename(shellPath).toLowerCase() === 'wsl.exe'
|
||||
const spawnedWslDistro = spawnedShellIsWsl ? (launchWslDistro ?? undefined) : null
|
||||
createPtyPhysicalExit(id)
|
||||
ptyProcesses.set(id, proc)
|
||||
ptyInitialCwd.set(id, cwd)
|
||||
if (spawnedWslDistro !== undefined) {
|
||||
ptyWslDistroById.set(id, spawnedWslDistro)
|
||||
}
|
||||
// Why both signals: launchAgent is the caller's explicit intent and
|
||||
// survives command rewriting (e.g. auth env prefixes); recognition covers
|
||||
// callers that pass a bare agent command line without the flag.
|
||||
@@ -1031,7 +1047,11 @@ export class LocalPtyProvider implements IPtyProvider {
|
||||
// briefly 0/undefined if node-pty hasn't observed the forked child yet.
|
||||
const rawPid = proc.pid
|
||||
const pid = typeof rawPid === 'number' && Number.isFinite(rawPid) && rawPid > 0 ? rawPid : null
|
||||
return { id, pid }
|
||||
return {
|
||||
id,
|
||||
pid,
|
||||
...(spawnedWslDistro !== undefined ? { wslDistro: spawnedWslDistro } : {})
|
||||
}
|
||||
}
|
||||
|
||||
// Local PTYs are always attached -- no-op. Remote providers use this to resubscribe.
|
||||
|
||||
@@ -111,8 +111,10 @@ export type PtySpawnResult = {
|
||||
pid?: number | null
|
||||
/** Minimal allowlisted launch ownership returned by daemon reattach. */
|
||||
launchAgent?: TuiAgent
|
||||
/** Immutable local WSL execution context returned by a daemon session. */
|
||||
wslDistro?: string
|
||||
/** Immutable local WSL execution context returned by a daemon session.
|
||||
* Null authoritatively identifies a native session; undefined means the
|
||||
* provider cannot report the context (including older daemons). */
|
||||
wslDistro?: string | null
|
||||
/** ANSI snapshot of the terminal screen, present when reattaching to an
|
||||
* existing daemon session. Write this to xterm.js to restore visual state. */
|
||||
snapshot?: string
|
||||
|
||||
Reference in New Issue
Block a user