mirror of
https://github.com/stablyai/orca.git
synced 2026-10-19 16:03:26 +00:00
fix(terminal): close remaining Windows ownership gaps
This commit is contained in:
@@ -712,7 +712,7 @@
|
||||
"ssh",
|
||||
"runtime"
|
||||
],
|
||||
"coverageNotes": "Live macOS Electron tests prove exact local PTY disappearance after parked-tab close and detached-pgid descendant death after agent close. Windows 2022 Actions red/green runs prove production store.removeWorktree(id, true) retires an agent PTY and its reparented native lock-holder before deleting the worktree, plain terminal close preserves an intentionally detached child, and natural agent-root exit publishes PTY exit only after its detached Job child is absent. Deterministic tests cover native Windows agent-only Job selection, fail-closed Job teardown and bridge errors, safe failure when Job ownership is unavailable, adopted-v24 destructive-shutdown refusal, provider-scoped recovered routing, fallback/daemon duplicate refusal, duplicate-exit suppression, sleeping legacy ownership, daemon WSL exclusion, daemon-wide shutdown, daemon and SSH routing, local/daemon pending-snapshot ownership across natural exit, stale-root descendant-signal suppression, graceful-to-immediate kill upgrades, duplicate-kill completion sharing, locale-stable bounded/fresh/coalesced process-table reads, deadline-safe successor scans, cycle-safe linear descendant traversal, target-only escalation indexing, 32-wide bulk teardown, source-scan timestamp identity, same-second PID ambiguity, ordinary runtime close ownership, unified-only hydration, split ownership, pane detach transfer, restart alias hydration, and late-hook suppression; live Linux, WSL, SSH, and remote-runtime process evidence remains pending.",
|
||||
"coverageNotes": "Live macOS Electron tests prove exact local PTY disappearance after parked-tab close and detached-pgid descendant death after agent close. Windows 2022 Actions red/green runs prove production store.removeWorktree(id, true) retires an agent PTY and its reparented native lock-holder before deleting the worktree, plain terminal close preserves an intentionally detached child, and natural agent-root exit publishes PTY exit only after its detached Job child is absent. Deterministic tests cover explicit and spawn-recognized native Windows agent Job selection, daemon session identity propagation, fail-closed Job teardown and bridge errors, safe failure when Job ownership is unavailable, adopted-v24 destructive-shutdown refusal, provider-scoped recovered routing, cached fallback/daemon duplicate refusal, duplicate-exit suppression, sleeping legacy ownership, daemon WSL exclusion, daemon-wide shutdown, daemon and SSH routing, local/daemon pending-snapshot ownership across natural exit, stale-root descendant-signal suppression, graceful-to-immediate kill upgrades, duplicate-kill completion sharing, locale-stable bounded/fresh/coalesced process-table reads, deadline-safe successor scans, cycle-safe linear descendant traversal, target-only escalation indexing, 32-wide bulk teardown, source-scan timestamp identity, same-second PID ambiguity, ordinary runtime close ownership, unified-only hydration, split ownership, pane detach transfer, restart alias hydration, and late-hook suppression; live Linux, WSL, SSH, and remote-runtime process evidence remains pending.",
|
||||
"motivatingLinks": [
|
||||
"https://github.com/stablyai/orca/pull/8628",
|
||||
"https://github.com/stablyai/orca/pull/8706",
|
||||
@@ -825,14 +825,14 @@
|
||||
{
|
||||
"file": "src/main/daemon/degraded-daemon-pty-provider.test.ts",
|
||||
"assertions": [
|
||||
"startup and direct discovery reject fallback/daemon duplicates, including a fallback spawn concurrent with daemon inventory",
|
||||
"startup and destructive revalidation reject fallback/daemon duplicates, including cached fallback routes and a fallback spawn concurrent with daemon inventory",
|
||||
"physical and synthetic daemon exits remain hidden while a fallback duplicate survives"
|
||||
]
|
||||
},
|
||||
{
|
||||
"file": "src/main/daemon/pty-subprocess.test.ts",
|
||||
"assertions": [
|
||||
"trusted native Windows agent ConPTY spawn requests and exposes Job teardown while plain and daemon WSL agent ConPTYs do neither"
|
||||
"explicit and spawn-recognized native Windows agent ConPTY spawns request and expose Job teardown while plain and daemon WSL agent ConPTYs do neither"
|
||||
]
|
||||
},
|
||||
{
|
||||
|
||||
@@ -299,6 +299,21 @@ describe('DegradedDaemonPtyProvider', () => {
|
||||
expect(fallback.shutdown).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('revalidates a cached fallback route before destructive shutdown', async () => {
|
||||
const daemonSessions: string[] = []
|
||||
const current = createDaemonAdapter('daemon', daemonSessions)
|
||||
const fallback = createProvider('fallback')
|
||||
const provider = new DegradedDaemonPtyProvider({ current, legacy: [], fallback })
|
||||
await provider.spawn({ sessionId: 'duplicate-session', cols: 80, rows: 24 })
|
||||
daemonSessions.push('duplicate-session')
|
||||
|
||||
await expect(provider.shutdown('duplicate-session', { immediate: true })).rejects.toThrow(
|
||||
'Ambiguous PTY session ownership'
|
||||
)
|
||||
expect(current.shutdown).not.toHaveBeenCalled()
|
||||
expect(fallback.shutdown).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('keeps a daemon inventory result when fallback spawns during the listing', async () => {
|
||||
const current = createDaemonAdapter('daemon')
|
||||
const fallback = createProvider('fallback')
|
||||
|
||||
@@ -119,7 +119,7 @@ export class DegradedDaemonPtyProvider implements IPtyProvider {
|
||||
if (this.sessionRouting.isAmbiguous(id)) {
|
||||
throw this.ambiguousOwnershipError([id])
|
||||
}
|
||||
const target = this.providerFor(id)
|
||||
const target = this.sessionRouting.providerForShutdown(id, this.current)
|
||||
if (this.sessionRouting.isAmbiguous(id)) {
|
||||
throw this.ambiguousOwnershipError([id])
|
||||
}
|
||||
|
||||
@@ -126,6 +126,16 @@ export class DegradedDaemonSessionRouting {
|
||||
return this.isAmbiguous(sessionId) ? ambiguousProvider : (provider ?? this.fallback)
|
||||
}
|
||||
|
||||
providerForShutdown(
|
||||
sessionId: string,
|
||||
ambiguousProvider: DegradedManagedPtyProvider
|
||||
): DegradedManagedPtyProvider {
|
||||
// Why: a cached fallback route can predate recovery of a daemon with the
|
||||
// same id; destructive shutdown must re-probe every physical owner.
|
||||
const provider = this.discoverExistingProvider(sessionId)
|
||||
return this.isAmbiguous(sessionId) ? ambiguousProvider : (provider ?? this.fallback)
|
||||
}
|
||||
|
||||
providerMapFor(provider: DegradedManagedPtyProvider): Map<string, DegradedManagedPtyProvider> {
|
||||
return new Map(this.routing.idsFor(provider).map((id) => [id, provider]))
|
||||
}
|
||||
|
||||
@@ -226,6 +226,32 @@ describe('createPtySubprocess', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('requests a Windows Job for a spawn-recognized daemon agent command', () => {
|
||||
spawnMock.mockReturnValue(mockPtyProcess())
|
||||
const platform = Object.getOwnPropertyDescriptor(process, 'platform')
|
||||
Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
|
||||
|
||||
try {
|
||||
createPtySubprocess({
|
||||
sessionId: 'windows-recognized-agent-job',
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
command: 'claude',
|
||||
cwd: process.cwd(),
|
||||
env: {}
|
||||
})
|
||||
expect(spawnMock).toHaveBeenCalledWith(
|
||||
expect.any(String),
|
||||
expect.any(Array),
|
||||
expect.objectContaining({ windowsAgentJob: true })
|
||||
)
|
||||
} finally {
|
||||
if (platform) {
|
||||
Object.defineProperty(process, 'platform', platform)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
it('does not request a Windows Job for a plain ConPTY', () => {
|
||||
spawnMock.mockReturnValue(mockPtyProcess())
|
||||
const platform = Object.getOwnPropertyDescriptor(process, 'platform')
|
||||
|
||||
@@ -558,6 +558,8 @@ function spawnDaemonPtyWithWindowsFallback(args: {
|
||||
*/
|
||||
export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandle {
|
||||
const size = normalizePtySize(opts.cols, opts.rows)
|
||||
const startupAgentRecognition = recognizeAgentProcessFromCommandLine(opts.command)
|
||||
const effectiveLaunchAgent = opts.launchAgent ?? startupAgentRecognition?.agent
|
||||
const env: Record<string, string> = {
|
||||
...mergeGitConfigEnvProtocol(process.env, opts.env),
|
||||
TERM: 'xterm-256color',
|
||||
@@ -576,7 +578,7 @@ export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandl
|
||||
// restores clickable refs like `owner/repo#123` / `PR#123`.
|
||||
FORCE_HYPERLINK: '1'
|
||||
} as Record<string, string>
|
||||
composeGuardedDaemonGitConfigEnv(env, opts.env, opts.launchAgent)
|
||||
composeGuardedDaemonGitConfigEnv(env, opts.env, effectiveLaunchAgent)
|
||||
for (const key of opts.envToDelete ?? []) {
|
||||
delete env[key]
|
||||
}
|
||||
@@ -605,7 +607,6 @@ export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandl
|
||||
let shellArgs: string[]
|
||||
let startupCommandDeliveredInShellArgs = false
|
||||
let windowsFallbackAttempts: WindowsShellSpawnAttempt[] = []
|
||||
const startupAgentRecognition = recognizeAgentProcessFromCommandLine(opts.command)
|
||||
const isCodexStartupCommand = startupAgentRecognition?.agent === 'codex'
|
||||
if (opts.command && startupAgentRecognition) {
|
||||
assertSafeAgentStartupCwd(opts.cwd, opts.command)
|
||||
@@ -824,12 +825,12 @@ export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandl
|
||||
cols: size.cols,
|
||||
rows: size.rows,
|
||||
windowsFallbackAttempts,
|
||||
// Why: WSL owns a Linux process tree, while only explicit native agent
|
||||
// sessions should change Windows process-launch semantics.
|
||||
// Why: WSL owns a Linux process tree, while only explicit or spawn-recognized
|
||||
// native agent sessions should change Windows process-launch semantics.
|
||||
windowsAgentJob:
|
||||
process.platform === 'win32' &&
|
||||
pathWin32.basename(shellPath).toLowerCase() !== 'wsl.exe' &&
|
||||
Boolean(opts.launchAgent)
|
||||
Boolean(effectiveLaunchAgent)
|
||||
})
|
||||
proc = spawned.process
|
||||
// Why: a Windows fallback (e.g. cmd.exe) carries its own argv-embedded
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { StartupCommandDelivery } from '../../shared/codex-startup-delivery'
|
||||
import type { TuiAgent } from '../../shared/types'
|
||||
import type { SubprocessHandle } from './session'
|
||||
import type { TakePendingOutputResult, TerminalSnapshot } from './types'
|
||||
|
||||
@@ -12,6 +13,7 @@ export type TerminalHostOptions = {
|
||||
envToDelete?: string[]
|
||||
command?: string
|
||||
startupCommandDelivery?: StartupCommandDelivery
|
||||
launchAgent?: TuiAgent
|
||||
shellOverride?: string
|
||||
terminalWindowsWslDistro?: string | null
|
||||
terminalWindowsPowerShellImplementation?: 'auto' | 'powershell.exe' | 'pwsh.exe'
|
||||
|
||||
@@ -137,14 +137,14 @@ describe('TerminalHost', () => {
|
||||
expect(result.snapshot?.cols).toBe(80)
|
||||
})
|
||||
|
||||
it('passes cwd, env, and trusted agent identity to spawn', async () => {
|
||||
await host.createOrAttach({
|
||||
it('passes command-recognized agent identity, cwd, and env to spawn', async () => {
|
||||
const result = await host.createOrAttach({
|
||||
sessionId: 'session-1',
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
cwd: '/home/user',
|
||||
env: { FOO: 'bar' },
|
||||
launchAgent: 'claude',
|
||||
command: 'claude',
|
||||
streamClient: { onData: vi.fn(), onExit: vi.fn() }
|
||||
})
|
||||
|
||||
@@ -153,9 +153,11 @@ describe('TerminalHost', () => {
|
||||
sessionId: 'session-1',
|
||||
cwd: '/home/user',
|
||||
env: { FOO: 'bar' },
|
||||
command: 'claude',
|
||||
launchAgent: 'claude'
|
||||
})
|
||||
)
|
||||
expect(result.launchAgent).toBe('claude')
|
||||
})
|
||||
|
||||
it('queues startup commands through the session shell-ready barrier', async () => {
|
||||
|
||||
@@ -15,6 +15,7 @@ import { shutdownTerminalHostSessions } from './terminal-host-session-shutdown'
|
||||
import { TerminalSessionTeardown } from './terminal-session-teardown'
|
||||
import { resolveWslSessionContext } from './wsl-session-context'
|
||||
import { getDaemonSessionResultMetadata } from './daemon-create-or-attach-result'
|
||||
import { recognizeAgentProcessFromCommandLine } from '../../shared/agent-process-recognition'
|
||||
|
||||
export type { CreateOrAttachOptions, CreateOrAttachResult } from './terminal-host-create-contract'
|
||||
export type { TerminalHostOptions } from './terminal-host-options'
|
||||
@@ -86,6 +87,10 @@ export class TerminalHost {
|
||||
this.killedTombstones.delete(opts.sessionId)
|
||||
const size = normalizePtySize(opts.cols, opts.rows)
|
||||
const wslDistro = resolveWslSessionContext(opts)?.distro
|
||||
// Why: configured default-tab commands do not carry launchAgent metadata;
|
||||
// preserve the recognized identity for spawn-time ownership and teardown.
|
||||
const launchAgent =
|
||||
opts.launchAgent ?? recognizeAgentProcessFromCommandLine(opts.command)?.agent
|
||||
|
||||
const subprocess = this.spawnSubprocess({
|
||||
sessionId: opts.sessionId,
|
||||
@@ -96,7 +101,7 @@ export class TerminalHost {
|
||||
envToDelete: opts.envToDelete,
|
||||
command: opts.command,
|
||||
startupCommandDelivery: opts.startupCommandDelivery,
|
||||
...(opts.launchAgent ? { launchAgent: opts.launchAgent } : {}),
|
||||
...(launchAgent ? { launchAgent } : {}),
|
||||
shellOverride: opts.shellOverride,
|
||||
terminalWindowsWslDistro: opts.terminalWindowsWslDistro,
|
||||
terminalWindowsPowerShellImplementation: opts.terminalWindowsPowerShellImplementation
|
||||
@@ -117,7 +122,7 @@ export class TerminalHost {
|
||||
cols: size.cols,
|
||||
rows: size.rows,
|
||||
terminalHandle: opts.env?.ORCA_TERMINAL_HANDLE,
|
||||
launchAgent: opts.launchAgent,
|
||||
launchAgent,
|
||||
subprocess,
|
||||
shellReadySupported,
|
||||
historySeed: opts.historySeed,
|
||||
|
||||
Reference in New Issue
Block a user