From 570cb71cbba79f02e48f0881f4f4a4d9359e73f7 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 28 Aug 2026 05:58:23 -0700 Subject: [PATCH] fix(cleanup): stop reading an unreachable terminal as an idle one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Workspace cleanup asked main two questions per terminal — `pty:hasChildProcesses` and `pty:getForegroundProcess` — and both coerce a degraded read to `false`/`null` before it crosses the IPC boundary. A disconnected SSH provider short-circuits to those values, and the daemon adapter catches its own dead-socket request and answers `null`. Nothing throws, so `probeTerminalLiveness`'s `unknown` arm never ran for the case it exists for, and a worktree whose agent was still working presented as an idle, unblocked cleanup row. Read `pty:inspectProcess` instead. It is the one inspection that carries `unavailable` across the boundary, and it is a single round trip instead of two. An `unavailable` answer, or a rejection, now yields `unverifiable` and the `terminal-liveness-unknown` blocker — unknown falls toward "do not clean up", which is the conservative arm here. The inspection read moves to a store-free module so the cleanup store can call it without a store->runtime import cycle; the type declaration stays put. --- .../runtime/runtime-terminal-inspection.ts | 60 +----- .../runtime-terminal-process-inspection.ts | 56 ++++++ .../workspace-cleanup-candidate-enrichment.ts | 2 +- ...ace-cleanup-enrichment-performance.test.ts | 54 ++---- .../workspace-cleanup-local-evidence.ts | 26 ++- ...orkspace-cleanup-removal-preflight.test.ts | 12 +- .../workspace-cleanup-scan-progress.test.ts | 74 +++----- ...cleanup-terminal-liveness-coercion.test.ts | 179 ++++++++++++++++++ 8 files changed, 318 insertions(+), 145 deletions(-) create mode 100644 src/renderer/src/runtime/runtime-terminal-process-inspection.ts create mode 100644 src/renderer/src/store/slices/workspace-cleanup-terminal-liveness-coercion.test.ts diff --git a/src/renderer/src/runtime/runtime-terminal-inspection.ts b/src/renderer/src/runtime/runtime-terminal-inspection.ts index de9a464f60c..1bc45c8ad98 100644 --- a/src/renderer/src/runtime/runtime-terminal-inspection.ts +++ b/src/renderer/src/runtime/runtime-terminal-inspection.ts @@ -4,11 +4,20 @@ import type { RuntimeTerminalSend } from '../../../shared/runtime-types' import { makePaneKey } from '../../../shared/stable-pane-id' import { isTerminalInputTooLargeWithDeferredMeasurement } from '../../../shared/terminal-input' import { useAppStore } from '../store' -import { RuntimeRpcCallError, callRuntimeRpc, getActiveRuntimeTarget } from './runtime-rpc-client' +import { callRuntimeRpc, getActiveRuntimeTarget } from './runtime-rpc-client' import { getRemoteRuntimePtyEnvironmentId, getRemoteRuntimeTerminalHandle } from './runtime-terminal-stream' +import { + inspectRuntimeTerminalProcess, + isTerminalGoneError +} from './runtime-terminal-process-inspection' + +// Why: the inspection read moved to a store-free module so the cleanup store can +// call it without a store->runtime import cycle. Re-exported so callers keep one +// import site. +export { inspectRuntimeTerminalProcess } export type RuntimeTerminalProcessInspection = { foregroundProcess: string | null @@ -31,26 +40,6 @@ export function isRemoteRuntimePtyId(ptyId: string): boolean { return ptyId.startsWith(REMOTE_PTY_ID_PREFIX) } -function isTerminalGoneError(error: unknown): boolean { - const message = error instanceof Error ? error.message : String(error) - const code = - error instanceof RuntimeRpcCallError - ? error.code - : error && typeof error === 'object' && 'code' in error - ? String((error as { code?: unknown }).code) - : '' - return ( - code === 'no_connected_pty' || - code === 'terminal_handle_stale' || - code === 'terminal_exited' || - code === 'terminal_gone' || - message.includes('terminal_handle_stale') || - message.includes('terminal_exited') || - message.includes('terminal_gone') || - message.includes('no_connected_pty') - ) -} - export function recordRuntimeTerminalInputForPtyId(ptyId: string, timestamp = Date.now()): void { const state = useAppStore.getState() for (const [tabId, layout] of Object.entries(state.terminalLayoutsByTabId)) { @@ -71,35 +60,6 @@ export function recordRuntimeTerminalInputForPtyId(ptyId: string, timestamp = Da } } -export async function inspectRuntimeTerminalProcess( - settings: Pick | null | undefined, - ptyId: string -): Promise { - const ownerEnvironmentId = getRemoteRuntimePtyEnvironmentId(ptyId) - const target = ownerEnvironmentId - ? ({ kind: 'environment', environmentId: ownerEnvironmentId } as const) - : getActiveRuntimeTarget(settings) - const terminal = getRemoteRuntimeTerminalHandle(ptyId) - if (target.kind !== 'environment' || !terminal) { - return window.api.pty.inspectProcess(ptyId) - } - - try { - const result = await callRuntimeRpc<{ process: RuntimeTerminalProcessInspection }>( - target, - 'terminal.inspectProcess', - { terminal }, - { timeoutMs: 15_000 } - ) - return result.process - } catch (error) { - if (isTerminalGoneError(error)) { - return { foregroundProcess: null, hasChildProcesses: false, unavailable: true } - } - throw error - } -} - /** * Forces a fresh, uncached foreground scan for a pane whose cached inspection * is suspect (issue #11064: the cached read can flap to the shell for a live diff --git a/src/renderer/src/runtime/runtime-terminal-process-inspection.ts b/src/renderer/src/runtime/runtime-terminal-process-inspection.ts new file mode 100644 index 00000000000..a7777747ec9 --- /dev/null +++ b/src/renderer/src/runtime/runtime-terminal-process-inspection.ts @@ -0,0 +1,56 @@ +import { RuntimeRpcCallError, callRuntimeRpc, getActiveRuntimeTarget } from './runtime-rpc-client' +import type { GlobalSettings } from '../../../shared/global-settings-types' +import type { RuntimeTerminalProcessInspection } from './runtime-terminal-inspection' +import { + getRemoteRuntimePtyEnvironmentId, + getRemoteRuntimeTerminalHandle +} from './runtime-terminal-stream' + +export function isTerminalGoneError(error: unknown): boolean { + const message = error instanceof Error ? error.message : String(error) + const code = + error instanceof RuntimeRpcCallError + ? error.code + : error && typeof error === 'object' && 'code' in error + ? String((error as { code?: unknown }).code) + : '' + return ( + code === 'no_connected_pty' || + code === 'terminal_handle_stale' || + code === 'terminal_exited' || + code === 'terminal_gone' || + message.includes('terminal_handle_stale') || + message.includes('terminal_exited') || + message.includes('terminal_gone') || + message.includes('no_connected_pty') + ) +} + +export async function inspectRuntimeTerminalProcess( + settings: Pick | null | undefined, + ptyId: string +): Promise { + const ownerEnvironmentId = getRemoteRuntimePtyEnvironmentId(ptyId) + const target = ownerEnvironmentId + ? ({ kind: 'environment', environmentId: ownerEnvironmentId } as const) + : getActiveRuntimeTarget(settings) + const terminal = getRemoteRuntimeTerminalHandle(ptyId) + if (target.kind !== 'environment' || !terminal) { + return window.api.pty.inspectProcess(ptyId) + } + + try { + const result = await callRuntimeRpc<{ process: RuntimeTerminalProcessInspection }>( + target, + 'terminal.inspectProcess', + { terminal }, + { timeoutMs: 15_000 } + ) + return result.process + } catch (error) { + if (isTerminalGoneError(error)) { + return { foregroundProcess: null, hasChildProcesses: false, unavailable: true } + } + throw error + } +} diff --git a/src/renderer/src/store/slices/workspace-cleanup-candidate-enrichment.ts b/src/renderer/src/store/slices/workspace-cleanup-candidate-enrichment.ts index d23a137ae78..f79d2641e20 100644 --- a/src/renderer/src/store/slices/workspace-cleanup-candidate-enrichment.ts +++ b/src/renderer/src/store/slices/workspace-cleanup-candidate-enrichment.ts @@ -241,7 +241,7 @@ async function enrichWorkspaceCleanupCandidate( const terminalProbe = await probeTerminalLiveness(state, tabs) if (terminalProbe === 'running') { blockers.push('running-terminal') - } else if (terminalProbe === 'unknown') { + } else if (terminalProbe === 'unverifiable') { blockers.push('terminal-liveness-unknown') } diff --git a/src/renderer/src/store/slices/workspace-cleanup-enrichment-performance.test.ts b/src/renderer/src/store/slices/workspace-cleanup-enrichment-performance.test.ts index ed710d1af7c..1ca44b9ab94 100644 --- a/src/renderer/src/store/slices/workspace-cleanup-enrichment-performance.test.ts +++ b/src/renderer/src/store/slices/workspace-cleanup-enrichment-performance.test.ts @@ -301,28 +301,20 @@ describe('workspace cleanup enrichment performance', () => { let probeGate = deferred() const active: Record = { stream: 0, final: 0 } const peak: Record = { stream: 0, final: 0 } - const hasChildProcesses = vi.fn(async () => { + const inspectProcess = vi.fn(async () => { const callPhase = phase const callGate = probeGate active[callPhase] += 1 peak[callPhase] = Math.max(peak[callPhase], active[callPhase]) await callGate.promise active[callPhase] -= 1 - return false + return { foregroundProcess: 'zsh', hasChildProcesses: false } }) ;( globalThis.window as unknown as { - api: { - pty: { - hasChildProcesses: typeof hasChildProcesses - getForegroundProcess: ReturnType - } - } + api: { pty: { inspectProcess: typeof inspectProcess } } } - ).api.pty = { - hasChildProcesses, - getForegroundProcess: vi.fn().mockResolvedValue('zsh') - } + ).api.pty = { inspectProcess } const store = createCleanupTestStore() store.setState({ tabsByWorktree, ptyIdsByTabId } as Partial) @@ -356,7 +348,7 @@ describe('workspace cleanup enrichment performance', () => { const finalScan = await scanPromise expect(peak.final).toBe(WORKSPACE_CLEANUP_ENRICHMENT_CONCURRENCY) - expect(hasChildProcesses).toHaveBeenCalledTimes(candidateCount * 2) + expect(inspectProcess).toHaveBeenCalledTimes(candidateCount * 2) expect(finalScan.candidates.map((candidate) => candidate.worktreeId)).toEqual( finalCandidates.map((candidate) => candidate.worktreeId) ) @@ -393,26 +385,18 @@ describe('workspace cleanup enrichment performance', () => { const probeGate = deferred() let active = 0 let peak = 0 - const hasChildProcesses = vi.fn(async () => { + const inspectProcess = vi.fn(async () => { active += 1 peak = Math.max(peak, active) await probeGate.promise active -= 1 - return false + return { foregroundProcess: 'zsh', hasChildProcesses: false } }) ;( globalThis.window as unknown as { - api: { - pty: { - hasChildProcesses: typeof hasChildProcesses - getForegroundProcess: ReturnType - } - } + api: { pty: { inspectProcess: typeof inspectProcess } } } - ).api.pty = { - hasChildProcesses, - getForegroundProcess: vi.fn().mockResolvedValue('zsh') - } + ).api.pty = { inspectProcess } const store = createCleanupTestStore() store.setState({ tabsByWorktree, ptyIdsByTabId } as Partial) @@ -438,7 +422,7 @@ describe('workspace cleanup enrichment performance', () => { probeGate.resolve() await scanPromise - expect(hasChildProcesses).toHaveBeenCalledTimes(candidateCount) + expect(inspectProcess).toHaveBeenCalledTimes(candidateCount) }) it('rebuilds focused removal enrichment from state changed after the broad scan', async () => { @@ -450,20 +434,14 @@ describe('workspace cleanup enrichment performance', () => { : Promise.resolve({ scannedAt: NOW, candidates: [candidate], errors: [] }) ) installWorkspaceCleanupApi(scan) - const hasChildProcesses = vi.fn().mockResolvedValue(false) + const inspectProcess = vi + .fn() + .mockResolvedValue({ foregroundProcess: 'zsh', hasChildProcesses: false }) ;( globalThis.window as unknown as { - api: { - pty: { - hasChildProcesses: typeof hasChildProcesses - getForegroundProcess: ReturnType - } - } + api: { pty: { inspectProcess: typeof inspectProcess } } } - ).api.pty = { - hasChildProcesses, - getForegroundProcess: vi.fn().mockResolvedValue('zsh') - } + ).api.pty = { inspectProcess } const removeWorktree = vi.fn().mockResolvedValue({ ok: true }) const store = createCleanupTestStore(removeWorktree) @@ -480,6 +458,6 @@ describe('workspace cleanup enrichment performance', () => { focusedScan.resolve({ scannedAt: NOW, candidates: [candidate], errors: [] }) await expect(removal).resolves.toMatchObject({ removedIds: [candidate.worktreeId] }) - expect(hasChildProcesses).toHaveBeenCalledWith('post-scan-pty') + expect(inspectProcess).toHaveBeenCalledWith('post-scan-pty') }) }) diff --git a/src/renderer/src/store/slices/workspace-cleanup-local-evidence.ts b/src/renderer/src/store/slices/workspace-cleanup-local-evidence.ts index 7508f59a4bf..bea296d04b5 100644 --- a/src/renderer/src/store/slices/workspace-cleanup-local-evidence.ts +++ b/src/renderer/src/store/slices/workspace-cleanup-local-evidence.ts @@ -6,6 +6,7 @@ import { import { classifyTitleActivity, isExplicitAgentStatusFresh } from '@/lib/pane-agent-evidence' import type { WorkspaceCleanupCandidate } from '../../../../shared/workspace-cleanup' import { getWorktreeVisitTimestamp } from '@/lib/worktree-visit-recency' +import { inspectRuntimeTerminalProcess } from '@/runtime/runtime-terminal-process-inspection' const RECENT_VISIBLE_CONTEXT_MS = 24 * 60 * 60 * 1000 const VIEWED_FROM_CLEANUP_MS = 2 * 60 * 60 * 1000 @@ -158,7 +159,7 @@ export function hasWorkingTitleAgent( export async function probeTerminalLiveness( state: AppState, tabs: { id: string; title: string }[] -): Promise<'idle' | 'running' | 'unknown'> { +): Promise<'idle' | 'running' | 'unverifiable'> { const ptyChecks = tabs.flatMap((tab) => (state.ptyIdsByTabId[tab.id] ?? []).map((ptyId) => ({ tab, ptyId })) ) @@ -166,15 +167,20 @@ export async function probeTerminalLiveness( return 'idle' } - let unknown = false + let unverifiable = false for (const { tab, ptyId } of ptyChecks) { try { - const [hasChildProcesses, foregroundProcess] = await Promise.all([ - window.api.pty.hasChildProcesses(ptyId), - window.api.pty.getForegroundProcess(ptyId) - ]) - const processName = normalizeProcessName(foregroundProcess) - if (!hasChildProcesses && (!processName || SHELL_PROCESS_NAMES.has(processName))) { + // Why: the standalone hasChildProcesses/getForegroundProcess handlers coerce a + // degraded read to false/null below IPC, so an unreachable host arrived here as + // an idle shell and cleanup dropped its blocker. inspectProcess is the only read + // that carries `unavailable` across the boundary; loss of contact is never idle. + const inspection = await inspectRuntimeTerminalProcess(state.settings, ptyId) + if (inspection.unavailable === true) { + unverifiable = true + continue + } + const processName = normalizeProcessName(inspection.foregroundProcess) + if (!inspection.hasChildProcesses && (!processName || SHELL_PROCESS_NAMES.has(processName))) { continue } if ( @@ -186,11 +192,11 @@ export async function probeTerminalLiveness( } return 'running' } catch { - unknown = true + unverifiable = true } } - return unknown ? 'unknown' : 'idle' + return unverifiable ? 'unverifiable' : 'idle' } function hasIdleAgentTitleForPty( diff --git a/src/renderer/src/store/slices/workspace-cleanup-removal-preflight.test.ts b/src/renderer/src/store/slices/workspace-cleanup-removal-preflight.test.ts index d9e656c3242..3cb4d8c9195 100644 --- a/src/renderer/src/store/slices/workspace-cleanup-removal-preflight.test.ts +++ b/src/renderer/src/store/slices/workspace-cleanup-removal-preflight.test.ts @@ -528,7 +528,11 @@ describe('workspace cleanup removal and protection', () => { api: { pty: { hasChildProcesses: vi.fn().mockResolvedValue(true), - getForegroundProcess: vi.fn().mockResolvedValue('codex') + getForegroundProcess: vi.fn().mockResolvedValue('codex'), + inspectProcess: vi.fn().mockResolvedValue({ + foregroundProcess: 'codex', + hasChildProcesses: true + }) } } } @@ -556,7 +560,11 @@ describe('workspace cleanup removal and protection', () => { hasChildProcesses: vi.fn(async (ptyId: string) => ptyId === 'pty-running'), getForegroundProcess: vi.fn(async (ptyId: string) => ptyId === 'pty-running' ? 'codex' : 'zsh' - ) + ), + inspectProcess: vi.fn(async (ptyId: string) => ({ + foregroundProcess: ptyId === 'pty-running' ? 'codex' : 'zsh', + hasChildProcesses: ptyId === 'pty-running' + })) } } } diff --git a/src/renderer/src/store/slices/workspace-cleanup-scan-progress.test.ts b/src/renderer/src/store/slices/workspace-cleanup-scan-progress.test.ts index 7d426a98374..ead24972342 100644 --- a/src/renderer/src/store/slices/workspace-cleanup-scan-progress.test.ts +++ b/src/renderer/src/store/slices/workspace-cleanup-scan-progress.test.ts @@ -15,6 +15,11 @@ import { } from './workspace-cleanup-slice-test-harness' import { WorkspaceCleanupScanSupersededError } from './workspace-cleanup-broad-scan-registry' +type PtyProcessProbe = { foregroundProcess: string | null; hasChildProcesses: boolean } + +/** What main answers for a pty sitting at an idle shell prompt. */ +const IDLE_SHELL_PROBE: PtyProcessProbe = { foregroundProcess: 'zsh', hasChildProcesses: false } + describe('workspace cleanup scan progress', () => { it('joins duplicate broad cleanup scans', async () => { const pending = deferred() @@ -172,18 +177,14 @@ describe('workspace cleanup scan progress', () => { return pending.promise }) installWorkspaceCleanupApi(scan) - const hasChildProcesses = vi.fn().mockResolvedValue(false) - const getForegroundProcess = vi.fn().mockResolvedValue('zsh') + const inspectProcess = vi.fn().mockResolvedValue(IDLE_SHELL_PROBE) ;( globalThis.window as unknown as { api: { - pty?: { - hasChildProcesses: typeof hasChildProcesses - getForegroundProcess: typeof getForegroundProcess - } + pty?: { inspectProcess: typeof inspectProcess } } } - ).api.pty = { hasChildProcesses, getForegroundProcess } + ).api.pty = { inspectProcess } const store = createCleanupTestStore() store.setState({ tabsByWorktree: { @@ -208,8 +209,7 @@ describe('workspace cleanup scan progress', () => { await vi.waitFor(() => { expect(store.getState().workspaceCleanupProgress?.scannedWorktreeCount).toBe(1) }) - expect(hasChildProcesses).toHaveBeenCalledTimes(1) - expect(getForegroundProcess).toHaveBeenCalledTimes(1) + expect(inspectProcess).toHaveBeenCalledTimes(1) onProgress?.({ scanId: 'scan-1', @@ -225,8 +225,7 @@ describe('workspace cleanup scan progress', () => { expect(store.getState().workspaceCleanupProgress?.scannedWorktreeCount).toBe(2) }) expect(store.getState().workspaceCleanupScan?.candidates).toHaveLength(2) - expect(hasChildProcesses).toHaveBeenCalledTimes(1) - expect(getForegroundProcess).toHaveBeenCalledTimes(1) + expect(inspectProcess).toHaveBeenCalledTimes(1) pending.resolve({ scannedAt: NOW, @@ -235,8 +234,7 @@ describe('workspace cleanup scan progress', () => { }) await scanPromise - expect(hasChildProcesses).toHaveBeenCalledTimes(1) - expect(getForegroundProcess).toHaveBeenCalledTimes(1) + expect(inspectProcess).toHaveBeenCalledTimes(1) }) it('updates count-only append progress without replacing existing candidate rows', async () => { @@ -291,7 +289,7 @@ describe('workspace cleanup scan progress', () => { it('keeps append progress rows when terminal enrichment resolves out of order', async () => { const pending = deferred() - const terminalProbe = deferred() + const terminalProbe = deferred() let onProgress: ((progress: WorkspaceCleanupScanProgress) => void) | undefined const terminalCandidate = makeCandidate({ worktreeId: 'repo1::/tmp/terminal' }) const laterCandidate = makeCandidate({ worktreeId: 'repo1::/tmp/later' }) @@ -300,18 +298,14 @@ describe('workspace cleanup scan progress', () => { return pending.promise }) installWorkspaceCleanupApi(scan) - const hasChildProcesses = vi.fn().mockReturnValue(terminalProbe.promise) - const getForegroundProcess = vi.fn().mockResolvedValue('zsh') + const inspectProcess = vi.fn().mockReturnValue(terminalProbe.promise) ;( globalThis.window as unknown as { api: { - pty?: { - hasChildProcesses: typeof hasChildProcesses - getForegroundProcess: typeof getForegroundProcess - } + pty?: { inspectProcess: typeof inspectProcess } } } - ).api.pty = { hasChildProcesses, getForegroundProcess } + ).api.pty = { inspectProcess } const store = createCleanupTestStore() store.setState({ tabsByWorktree: { @@ -333,7 +327,7 @@ describe('workspace cleanup scan progress', () => { candidateMode: 'append' }) await vi.waitFor(() => { - expect(hasChildProcesses).toHaveBeenCalledTimes(1) + expect(inspectProcess).toHaveBeenCalledTimes(1) }) onProgress?.({ @@ -348,7 +342,7 @@ describe('workspace cleanup scan progress', () => { await Promise.resolve() expect(store.getState().workspaceCleanupProgress).toBeNull() - terminalProbe.resolve(false) + terminalProbe.resolve(IDLE_SHELL_PROBE) await vi.waitFor(() => { expect(store.getState().workspaceCleanupProgress?.scannedWorktreeCount).toBe(2) }) @@ -366,7 +360,7 @@ describe('workspace cleanup scan progress', () => { it('drains queued progress before publishing the final scan result', async () => { const pending = deferred() - const terminalProbe = deferred() + const terminalProbe = deferred() let scanSettled = false let onProgress: ((progress: WorkspaceCleanupScanProgress) => void) | undefined const terminalCandidate = makeCandidate({ worktreeId: 'repo1::/tmp/terminal' }) @@ -376,18 +370,14 @@ describe('workspace cleanup scan progress', () => { return pending.promise }) installWorkspaceCleanupApi(scan) - const hasChildProcesses = vi.fn().mockReturnValue(terminalProbe.promise) - const getForegroundProcess = vi.fn().mockResolvedValue('zsh') + const inspectProcess = vi.fn().mockReturnValue(terminalProbe.promise) ;( globalThis.window as unknown as { api: { - pty?: { - hasChildProcesses: typeof hasChildProcesses - getForegroundProcess: typeof getForegroundProcess - } + pty?: { inspectProcess: typeof inspectProcess } } } - ).api.pty = { hasChildProcesses, getForegroundProcess } + ).api.pty = { inspectProcess } const store = createCleanupTestStore() store.setState({ tabsByWorktree: { @@ -414,7 +404,7 @@ describe('workspace cleanup scan progress', () => { candidateMode: 'append' }) await vi.waitFor(() => { - expect(hasChildProcesses).toHaveBeenCalledTimes(1) + expect(inspectProcess).toHaveBeenCalledTimes(1) }) pending.resolve({ @@ -425,14 +415,14 @@ describe('workspace cleanup scan progress', () => { await Promise.resolve() expect(scanSettled).toBe(false) - terminalProbe.resolve(false) + terminalProbe.resolve(IDLE_SHELL_PROBE) await scanPromise expect(scanSettled).toBe(true) expect( store.getState().workspaceCleanupScan?.candidates.map((candidate) => candidate.worktreeId) ).toEqual(['repo1::/tmp/final']) - expect(hasChildProcesses).toHaveBeenCalledTimes(1) + expect(inspectProcess).toHaveBeenCalledTimes(1) expect(store.getState().workspaceCleanupProgress).toMatchObject({ scannedWorktreeCount: 1, totalWorktreeCount: 1 @@ -442,7 +432,7 @@ describe('workspace cleanup scan progress', () => { it('ignores stale progress without replacing the active scan progress queue', async () => { const firstPending = deferred() const secondPending = deferred() - const terminalProbe = deferred() + const terminalProbe = deferred() const progressCallbacks: ((progress: WorkspaceCleanupScanProgress) => void)[] = [] let secondScanSettled = false const firstCandidate = makeCandidate({ worktreeId: 'repo1::/tmp/first' }) @@ -455,18 +445,14 @@ describe('workspace cleanup scan progress', () => { : firstPending.promise }) installWorkspaceCleanupApi(scan) - const hasChildProcesses = vi.fn().mockReturnValue(terminalProbe.promise) - const getForegroundProcess = vi.fn().mockResolvedValue('zsh') + const inspectProcess = vi.fn().mockReturnValue(terminalProbe.promise) ;( globalThis.window as unknown as { api: { - pty?: { - hasChildProcesses: typeof hasChildProcesses - getForegroundProcess: typeof getForegroundProcess - } + pty?: { inspectProcess: typeof inspectProcess } } } - ).api.pty = { hasChildProcesses, getForegroundProcess } + ).api.pty = { inspectProcess } const store = createCleanupTestStore() store.setState({ tabsByWorktree: { @@ -495,7 +481,7 @@ describe('workspace cleanup scan progress', () => { candidateMode: 'append' }) await vi.waitFor(() => { - expect(hasChildProcesses).toHaveBeenCalledTimes(1) + expect(inspectProcess).toHaveBeenCalledTimes(1) }) progressCallbacks[0]?.({ scanId: 'scan-1', @@ -515,7 +501,7 @@ describe('workspace cleanup scan progress', () => { await Promise.resolve() expect(secondScanSettled).toBe(false) - terminalProbe.resolve(false) + terminalProbe.resolve(IDLE_SHELL_PROBE) await secondScan firstPending.resolve({ scannedAt: NOW - 1, candidates: [firstCandidate], errors: [] }) // Why: the first fleet scan was superseded when the second started. diff --git a/src/renderer/src/store/slices/workspace-cleanup-terminal-liveness-coercion.test.ts b/src/renderer/src/store/slices/workspace-cleanup-terminal-liveness-coercion.test.ts new file mode 100644 index 00000000000..052a8cd9e43 --- /dev/null +++ b/src/renderer/src/store/slices/workspace-cleanup-terminal-liveness-coercion.test.ts @@ -0,0 +1,179 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { AppState } from '../types' +import { enrichWorkspaceCleanupCandidates } from './workspace-cleanup-candidate-enrichment' +import { probeTerminalLiveness } from './workspace-cleanup-local-evidence' +import { WORKTREE_ID, makeCandidate, makeState } from './workspace-cleanup-slice-test-harness' + +const TAB_ID = 'tab-1' +const PTY_ID = 'pty-1' + +type PtyInspection = { + foregroundProcess: string | null + hasChildProcesses: boolean + unavailable?: true +} + +/** + * Models the three answers main really gives for ONE pty read. The standalone + * `hasChildProcesses`/`getForegroundProcess` handlers coerce a degraded read to + * `false`/`null` before it crosses IPC; only `inspectProcess` carries evidence. + */ +function installPtyApi(inspection: { + hasChildProcesses: boolean + foregroundProcess: string | null + inspectProcess: () => Promise +}) { + const calls = { hasChildProcesses: 0, getForegroundProcess: 0, inspectProcess: 0 } + ;(globalThis as { window: unknown }).window = { + api: { + pty: { + hasChildProcesses: vi.fn(async () => { + calls.hasChildProcesses += 1 + return inspection.hasChildProcesses + }), + getForegroundProcess: vi.fn(async () => { + calls.getForegroundProcess += 1 + return inspection.foregroundProcess + }), + inspectProcess: vi.fn(async () => { + calls.inspectProcess += 1 + return inspection.inspectProcess() + }), + confirmForegroundProcess: vi.fn(async () => null) + } + } + } + return calls +} + +/** A degraded read: main answered, but the answer carries no process evidence. */ +function installDegradedPtyApi(inspection: () => Promise) { + // Why: the standalone reads are SWALLOWED below IPC (a disconnected SSH + // provider short-circuits to false/null; the daemon adapter catches its own + // dead-socket request and returns null). Nothing throws at the caller. + return installPtyApi({ + hasChildProcesses: false, + foregroundProcess: null, + inspectProcess: inspection + }) +} + +function stateWithOnePty(overrides: Partial = {}): AppState { + return makeState({ + tabsByWorktree: { [WORKTREE_ID]: [{ id: TAB_ID, title: 'zsh' }] } as never, + ptyIdsByTabId: { [TAB_ID]: [PTY_ID] }, + ...overrides + }) +} + +async function enrichOne(state: AppState) { + const [candidate] = await enrichWorkspaceCleanupCandidates([makeCandidate()], state, { + applyDismissals: false + }) + return candidate! +} + +afterEach(() => { + delete (globalThis as { window?: unknown }).window + vi.restoreAllMocks() +}) + +describe('a degraded terminal read is unverifiable, never idle', () => { + it('reports unverifiable when the host answers without process evidence', async () => { + installDegradedPtyApi(async () => ({ + foregroundProcess: null, + hasChildProcesses: false, + unavailable: true + })) + + await expect( + probeTerminalLiveness(stateWithOnePty(), [{ id: TAB_ID, title: 'zsh' }]) + ).resolves.toBe('unverifiable') + }) + + it('blocks cleanup on an unavailable read instead of letting the row look clean', async () => { + installDegradedPtyApi(async () => ({ + foregroundProcess: null, + hasChildProcesses: false, + unavailable: true + })) + + const candidate = await enrichOne(stateWithOnePty()) + + expect(candidate.blockers).toContain('terminal-liveness-unknown') + expect(candidate.blockers).not.toContain('running-terminal') + expect(candidate.tier).toBe('protected') + expect(candidate.selectedByDefault).toBe(false) + }) + + it('reports unverifiable when contact with the host is lost mid-read', async () => { + // Why: a dead daemon socket rejects `inspectProcess` while the standalone + // reads still resolve false/null — a swallowing adapter, not a thrower. + installDegradedPtyApi(async () => { + throw new Error('terminal_gone') + }) + + const candidate = await enrichOne(stateWithOnePty()) + + expect(candidate.blockers).toContain('terminal-liveness-unknown') + expect(candidate.tier).toBe('protected') + }) + + it('reads the evidence-carrying handler, not the coerced ones', async () => { + const calls = installDegradedPtyApi(async () => ({ + foregroundProcess: null, + hasChildProcesses: false, + unavailable: true + })) + + await probeTerminalLiveness(stateWithOnePty(), [{ id: TAB_ID, title: 'zsh' }]) + + expect(calls.inspectProcess).toBe(1) + expect(calls.hasChildProcesses).toBe(0) + expect(calls.getForegroundProcess).toBe(0) + }) +}) + +describe('verified reads keep their existing verdicts', () => { + it('still permits cleanup for a genuinely idle shell', async () => { + installPtyApi({ + hasChildProcesses: false, + foregroundProcess: 'zsh', + inspectProcess: async () => ({ foregroundProcess: 'zsh', hasChildProcesses: false }) + }) + + const candidate = await enrichOne(stateWithOnePty()) + + expect(candidate.blockers).toEqual([]) + expect(candidate.tier).toBe('ready') + expect(candidate.selectedByDefault).toBe(true) + }) + + it('still blocks cleanup for a live agent', async () => { + installPtyApi({ + hasChildProcesses: true, + foregroundProcess: 'claude', + inspectProcess: async () => ({ foregroundProcess: 'claude', hasChildProcesses: true }) + }) + + const candidate = await enrichOne(stateWithOnePty()) + + expect(candidate.blockers).toContain('running-terminal') + expect(candidate.tier).toBe('protected') + }) + + it('still permits cleanup for a worktree with no terminals at all', async () => { + installPtyApi({ + hasChildProcesses: false, + foregroundProcess: null, + inspectProcess: async () => { + throw new Error('should not be asked about a worktree with no ptys') + } + }) + + const candidate = await enrichOne(makeState()) + + expect(candidate.blockers).toEqual([]) + expect(candidate.tier).toBe('ready') + }) +})