Skip to content
Merged
11 changes: 11 additions & 0 deletions config/scripts/check-layout-writer-ratchets.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,17 @@ export function countLayoutWrites(sourceText, ratchet, fileName = 'file.tsx') {
!(ts.isPropertyAssignment(node.parent) && node.parent.name === node) &&
!ts.isComputedPropertyName(node.parent) &&
!(ts.isElementAccessExpression(node.parent) && node.parent.argumentExpression === node) &&
!(
ts.isCallExpression(node.parent) &&
!node.parent.questionDotToken &&
node.parent.arguments.length === 2 &&
node.parent.arguments[1] === node &&
ts.isPropertyAccessExpression(node.parent.expression) &&
!node.parent.expression.questionDotToken &&
ts.isIdentifier(node.parent.expression.expression) &&
node.parent.expression.expression.text === 'Object' &&
node.parent.expression.name.text === 'hasOwn'
) &&
!ts.isLiteralTypeNode(node.parent)
) {
count += 1
Expand Down
1 change: 1 addition & 0 deletions config/scripts/check-layout-writer-ratchets.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ describe('renderer layout writes', () => {
'const useTabs = () => ({ activeTabId, tabsByWorktree })',
"type Field = Pick<AppState, 'tabsByWorktree'>",
"const read = state['unifiedTabsByWorktree']",
"Object.hasOwn(requested, 'terminalLayoutsByTabId')",
'const { tabsByWorktree } = state'
].join('\n')
expect(countLayoutWrites(src, rendererRatchet, 'a.tsx')).toBe(5)
Expand Down
1 change: 1 addition & 0 deletions src/main/daemon/daemon-pty-adapter-wsl-sleep-cwd.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ describe('a sleeping bridge WSL pane keeps its shell folder', () => {
const harness = await startDaemonAdapterHarness((opts) => {
spawnCwds.push(opts.cwd)
if (opts.cwd && deletedCwds.has(opts.cwd)) {
opts.onCreationDisposition?.({ kind: 'no-fork' })
throw new Error(`Working directory "${opts.cwd}" does not exist. It may have been deleted.`)
}
lastSubprocess = createMockSubprocess()
Expand Down
24 changes: 24 additions & 0 deletions src/main/daemon/daemon-server-async-spawn-cancellation.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { registerDaemonCreationFailureTests } from './daemon-server-creation-failure.test-fixture'
import './mock-descendant-sweep'
import { mkdtempSync, rmSync } from 'node:fs'
import { tmpdir } from 'node:os'
Expand All @@ -15,6 +16,7 @@ type DaemonServerInternals = {
}

describe('daemon async spawn cancellation', () => {
let spawned = 0
let directory: string
let server: DaemonServer
let client: DaemonClient
Expand All @@ -23,6 +25,7 @@ describe('daemon async spawn cancellation', () => {
let subprocess: ReturnType<typeof createMockSubprocess>

beforeEach(async () => {
spawned = 0
directory = mkdtempSync(join(tmpdir(), 'daemon-async-spawn-cancel-'))
let markSpawnStarted: () => void = () => {}
spawnStarted = new Promise<void>((resolve) => {
Expand All @@ -35,6 +38,7 @@ describe('daemon async spawn cancellation', () => {
socketPath: getDaemonSocketPath(directory),
tokenPath: join(directory, 'daemon.token'),
spawnSubprocess: async () => {
spawned++
subprocess = createMockSubprocess()
markSpawnStarted()
await spawnGate
Expand All @@ -53,9 +57,29 @@ describe('daemon async spawn cancellation', () => {
releaseSpawn?.()
client?.disconnect()
await server?.shutdown()
vi.restoreAllMocks()
rmSync(directory, { recursive: true, force: true })
})

registerDaemonCreationFailureTests({
get server() {
return server
},
get client() {
return client
},
get subprocess() {
return subprocess
},
get spawnStarted() {
return spawnStarted
},
get spawned() {
return spawned
},
releaseSpawn: () => releaseSpawn()
})

it.each(['cancelCreateOrAttach', 'kill'] as const)(
'reaps a subprocess canceled by %s during async spawn',
async (requestType) => {
Expand Down
92 changes: 92 additions & 0 deletions src/main/daemon/daemon-server-creation-failure.test-fixture.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import './mock-descendant-sweep'
import { expect, it, vi } from 'vitest'
import type { DaemonClient } from './client'
import type { createMockSubprocess } from './daemon-pty-adapter-test-harness'
import type { DaemonServer } from './daemon-server'
export type DaemonCreationFailureFixture = {
server: DaemonServer
client: DaemonClient
subprocess: ReturnType<typeof createMockSubprocess>
releaseSpawn: () => void
spawnStarted: Promise<void>
spawned: number
}
export function registerDaemonCreationFailureTests(fixture: DaemonCreationFailureFixture) {
it.each([
['observer', 'retry'],
['constructor', 'retry'],
['publication', 'disposal']
] as const)(
'ordinary authenticated create drives failed unpublished cleanup: %s/%s',
async (failure, wake) => {
const options = {
sessionId: 'ordinary-failed',
cols: 80,
rows: 24,
agentSessionEnsure: {
claim: {
digestVersion: 1 as const,
keyId: 'key',
identityDigest: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa',
worktreeScopeDigest: 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb',
agent: 'codex' as const
},
surface: {
worktreeId: 'worktree',
tabId: 'ordinary-tab',
leafId: '11111111-1111-4111-8111-111111111111',
terminalHandle: 'term_ordinary'
}
}
}
const creating = fixture.client.request('createOrAttach', options)
const observed = expect(creating).rejects.toThrow('fixture subscription failed')
await fixture.spawnStarted
const failed = fixture.subprocess
if (failure === 'observer') {
failed.onExit = vi.fn(failed.onExit).mockImplementationOnce(() => {
throw new Error('fixture subscription failed')
})
} else if (failure === 'constructor') {
failed.onData = () => {
throw new Error('fixture subscription failed')
}
} else {
Object.assign(failed, {
onInputClosed: () => {
throw new Error('fixture subscription failed')
}
})
}
vi.mocked(failed.forceKill).mockImplementation(() => {
throw new Error('fixture close unavailable')
})
fixture.releaseSpawn()
await observed
await vi.waitFor(() => expect(failed.forceKill).toHaveBeenCalled())
expect(fixture.server.host.listSessions()).toEqual([])
expect(fixture.server.automaticWork.gate.snapshot().terminals).toBe(1)
const attempts = vi.mocked(failed.forceKill).mock.calls.length
// The ordinary next authenticated request drives the retained resource, not a test callback.
vi.mocked(failed.forceKill).mockImplementation(() => {
setTimeout(() => failed._simulateExit(137), 5)
})
if (wake === 'disposal') {
await fixture.server.host.dispose()
expect(failed.dispose).toHaveBeenCalledOnce()
expect(fixture.spawned).toBe(1)
expect(fixture.server.automaticWork.gate.snapshot().terminals).toBe(0)
return
}
expect(await fixture.client.request('createOrAttach', options)).toMatchObject({ isNew: true })
expect(vi.mocked(failed.forceKill).mock.calls.length).toBeGreaterThan(attempts)
expect(failed.dispose).toHaveBeenCalledOnce()
expect(fixture.spawned).toBe(2)
expect(fixture.server.automaticWork.gate.snapshot().terminals).toBe(1)
await fixture.client.request('write', { sessionId: options.sessionId, data: 'new input' })
expect(fixture.subprocess.write).toHaveBeenCalledWith('new input')
await fixture.server.host.dispose()
expect(fixture.server.automaticWork.gate.snapshot().terminals).toBe(0)
}
)
}
13 changes: 2 additions & 11 deletions src/main/daemon/daemon-server-options.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { DaemonFileLog } from './daemon-file-log'
import type { SubprocessHandle } from './session-subprocess-handle'
import type { TerminalHostOptions } from './terminal-host-options'
import type { TerminalSpawnAdmissionMode } from './terminal-spawn-gate'
import type { HostTerminalPolicy } from './host-terminal-policy'

Expand Down Expand Up @@ -37,14 +37,5 @@ export type DaemonServerOptions = {
/** Shared terminal counts for the host-wide cap. */
hostTerminalLedgerDir?: string
terminalLimitPolicy?: HostTerminalPolicy
spawnSubprocess: (opts: {
sessionId: string
cols: number
rows: number
cwd?: string
env?: Record<string, string>
command?: string
shellOverride?: string
isCanceled?: () => boolean
}) => SubprocessHandle | Promise<SubprocessHandle>
spawnSubprocess: TerminalHostOptions['spawnSubprocess']
}
7 changes: 6 additions & 1 deletion src/main/daemon/pty-subprocess-handle-lifecycle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ vi.mock('../providers/windows-pty-job-membership', () => ({
import { createPtySubprocess } from './pty-subprocess'
import { mockPtyProcess, useDaemonPtySubprocessEnv } from './pty-subprocess-test-harness'
import { __setConptyJobNativeForTests } from '../windows/windows-pty-job'
import * as ptyJob from '../windows/windows-pty-job'

describe('createPtySubprocess', () => {
beforeEach(() => {
Expand Down Expand Up @@ -453,16 +454,20 @@ describe('createPtySubprocess', () => {
const killSpy = vi.spyOn(process, 'kill').mockImplementation(() => {
throw new Error('already gone')
})
const terminateJob = vi.spyOn(ptyJob, 'terminatePtyJob').mockReturnValue('unavailable')
const origPlatform = Object.getOwnPropertyDescriptor(process, 'platform')
Object.defineProperty(process, 'platform', { value: 'win32' })
try {
const handle = await createPtySubprocess({ sessionId: 'test', cols: 80, rows: 24 })
handle.forceKill()
handle.dispose()
expect(killSpy).toHaveBeenCalledWith(123456, 'SIGKILL')
// An unverified PID can be reused; the per-PTY job/native handle owns this process tree.
expect(killSpy).not.toHaveBeenCalled()
expect(terminateJob).toHaveBeenCalledExactlyOnceWith(proc)
expect(proc.kill).toHaveBeenCalledOnce()
expect(proc.destroy).not.toHaveBeenCalled()
} finally {
terminateJob.mockRestore()
killSpy.mockRestore()
restorePlatform(origPlatform)
}
Expand Down
34 changes: 29 additions & 5 deletions src/main/daemon/pty-subprocess-io-failure-cleanup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ describe.each(['darwin', 'linux', 'win32'] as const)('%s native-handle contract'
it('keeps graceful and forced termination available until physical exit', () => {
const fixture = createFixture()
const originalKill = fixture.proc.kill
const terminateJob = vi.spyOn(ptyJob, 'terminatePtyJob').mockReturnValue('unavailable')
const signal = vi.spyOn(process, 'kill').mockReturnValue(true)
failIo(fixture, operation)

Expand All @@ -154,7 +155,14 @@ describe.each(['darwin', 'linux', 'win32'] as const)('%s native-handle contract'
const forced = createFixture()
failIo(forced, operation)
forced.handle.forceKill()
expect(signal).toHaveBeenCalledWith(4242, 'SIGKILL')
if (platform === 'win32') {
// An unverified PID can be reused; the per-PTY job/native handle owns this process tree.
expect(signal).not.toHaveBeenCalled()
expect(terminateJob).toHaveBeenCalledExactlyOnceWith(forced.proc)
expect(forced.proc.kill).toHaveBeenCalledOnce()
} else {
expect(signal).toHaveBeenCalledWith(4242, 'SIGKILL')
}

forced.proc._simulateExit(137)
signal.mockClear()
Expand All @@ -168,6 +176,7 @@ describe.each(['darwin', 'linux', 'win32'] as const)('%s native-handle contract'

it('reaps every session and native handle across 32 failed-I/O create/close cycles', async () => {
const emulatorDispose = vi.spyOn(HeadlessEmulator.prototype, 'dispose')
const terminateJob = vi.spyOn(ptyJob, 'terminatePtyJob').mockReturnValue('unavailable')
const signal = vi.spyOn(process, 'kill').mockReturnValue(true)
let fixture = createFixture()
const host = new TerminalHost({ spawnSubprocess: () => fixture.handle })
Expand Down Expand Up @@ -195,17 +204,32 @@ describe.each(['darwin', 'linux', 'win32'] as const)('%s native-handle contract'
expect(host.listSessions()).toHaveLength(1)
expect(fixture.proc.destroy).not.toHaveBeenCalled()
expect(onExit).not.toHaveBeenCalled()
await vi.waitFor(() => expect(signal).toHaveBeenCalledWith(4242, 'SIGKILL'), {
interval: 1
})
if (platform === 'win32') {
await vi.waitFor(() => expect(fixture.proc.kill).toHaveBeenCalledOnce(), {
interval: 1
})
expect(terminateJob).toHaveBeenCalledWith(fixture.proc)
// An unverified PID can be reused; the per-PTY job/native handle owns this process tree.
expect(signal).not.toHaveBeenCalled()
} else {
await vi.waitFor(() => expect(signal).toHaveBeenCalledWith(4242, 'SIGKILL'), {
interval: 1
})
}
} finally {
fixture.proc._simulateExit(137)
killFailure = await settled
}
expect(killFailure).toBeNull()
expect(host.listSessions()).toHaveLength(0)
expect(onExit).toHaveBeenCalledOnce()
expect(fixture.proc.destroy).toHaveBeenCalledOnce()
if (platform === 'win32') {
// An unverified PID can be reused; job/native close owns the tree and must not be repeated by destroy.
expect(fixture.proc.destroy).not.toHaveBeenCalled()
expect(fixture.proc.kill).toHaveBeenCalledOnce()
} else {
expect(fixture.proc.destroy).toHaveBeenCalledOnce()
}
expect(emulatorDispose).toHaveBeenCalledTimes(index + 1)
}
} finally {
Expand Down
Loading
Loading