diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index 0486aef7532..4d9fb5c4a15 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -1,6 +1,6 @@ { "schemaVersion": 1, - "updatedAt": "2026-07-01", + "updatedAt": "2026-07-02", "policy": { "maturityLevels": ["experimental", "soak", "blocking", "accepted-gap", "deprecated"], "blockingPromotion": { @@ -158,6 +158,72 @@ "knownGaps": ["No manifest command yet."], "demotionRule": "Cannot promote without deterministic oracle and runtime history." }, + { + "id": "terminal-session.targeted-liveness", + "title": "Hot terminal paths use targeted PTY liveness instead of broad session inventory", + "maturity": "experimental", + "owner": "terminal-runtime", + "layer": "renderer-main-contract", + "surfaces": [ + "typing", + "focus", + "light tab switch", + "workspace switch", + "visibility resume", + "resize", + "render", + "per-pane liveness", + "resource manager session inventory" + ], + "platforms": ["macos", "linux", "windows"], + "providers": ["local", "daemon", "ssh", "wsl", "remote-runtime"], + "motivatingLinks": [ + "https://github.com/stablyai/orca/issues/6773", + "https://github.com/stablyai/orca/pull/6514" + ], + "invariant": "Typing, focus, terminal switch, workspace switch, visibility resume, resize, render, and per-pane liveness paths must not call global pty:listSessions; destructive liveness may act only on authoritative exact-id local/daemon false, while unknown provider state is non-destructive.", + "oracle": "The executable slice asserts visibility resume and first input use targeted pty.hasPty without pty:listSessions, light/fresh input paths avoid liveness IPC, SSH/remote panes skip destructive local liveness, resize resume uses targeted pty:getSize without listSessions, Resource Manager closed badge code has no session inventory interval, and main pty:hasPty uses cached ownership without provider listing.", + "commands": [ + "pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/components/terminal-pane/terminal-dead-session-reconcile.test.ts src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts src/renderer/src/components/terminal-pane/pty-connection.test.ts src/renderer/src/components/status-bar/resource-manager-session-polling.test.ts src/main/ipc/pty.test.ts src/main/daemon/daemon-pty-adapter.test.ts src/main/daemon/daemon-pty-router.test.ts src/main/daemon/degraded-daemon-pty-provider.test.ts" + ], + "testFiles": [ + "src/renderer/src/components/terminal-pane/terminal-dead-session-reconcile.test.ts", + "src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts", + "src/renderer/src/components/terminal-pane/pty-connection.test.ts", + "src/renderer/src/components/status-bar/resource-manager-session-polling.test.ts", + "src/main/ipc/pty.test.ts", + "src/main/daemon/daemon-pty-adapter.test.ts", + "src/main/daemon/daemon-pty-router.test.ts", + "src/main/daemon/degraded-daemon-pty-provider.test.ts" + ], + "runtimeBudget": { + "p95Seconds": 90, + "scope": "focused renderer/main/daemon unit tests" + }, + "flakeHistory": { + "status": "unknown", + "evidence": "New experimental count/contract gate; no CI soak history yet." + }, + "redGreenEvidence": { + "status": "partial", + "evidence": "Tests fail if visibility/input liveness falls back to listSessions, if pty:hasPty scans provider inventory, or if unknown liveness closes. Needs saved intentional-break evidence and live high-session Electron counters before promotion." + }, + "performanceBudget": { + "required": true, + "evidence": "Focused count tests assert zero pty:listSessions for covered hot-path slices and zero provider listing from pty:hasPty. Broader raw focus/workspace-switch/render/high-session Electron instrumentation remains a known gap." + }, + "promotionCriteria": [ + "Run in soak for at least 100 consecutive passes or 14 days across required CI platforms.", + "Attach red/green evidence for broad-list fallback regressions.", + "Add live Electron high-session counter coverage for raw focus, workspace switch, render ticks, SSH, WSL, and Windows ConPTY before promoting beyond experimental." + ], + "knownGaps": [ + "Current command is deterministic macOS unit/contract coverage, not live local/daemon/SSH/WSL/remote-runtime provider execution.", + "Current command does not yet instrument every raw focus, workspace-switch, render, or high-session Electron path listed in the invariant.", + "SSH and remote-runtime liveness remain intentionally unknown/non-destructive rather than authoritative." + ], + "demotionRule": "Demote or quarantine if the count tests flake without an owner bug, or if any hot-path regression reintroduces pty:listSessions/provider-list fanout." + }, { "id": "xterm-addon.boundary-containment", "title": "xterm addon failures stay pane-scoped and input survives", @@ -240,6 +306,50 @@ ], "knownGaps": ["No fixture corpus or command yet."], "demotionRule": "Cannot promote without old production fixture provenance." + }, + { + "id": "startup-upgrade.local-pty-hydration-observability", + "title": "Boot PTY registry hydration is bounded and observable", + "maturity": "experimental", + "owner": "startup-persistence", + "layer": "main-unit", + "surfaces": ["startup", "daemon warm reattach", "resource manager memory attribution"], + "platforms": ["macos", "linux", "windows"], + "providers": ["local", "daemon"], + "motivatingLinks": ["https://github.com/stablyai/orca/pull/6514"], + "invariant": "Boot-time local PTY registry hydration may enumerate daemon sessions only in the boot hydrator, must not block first usable UI, and must expose bounded counters for provider-unavailable, success, partial adapter failure, skipped remote/unknown records, already-registered records, and elapsed time.", + "oracle": "The main-unit hydrator tests assert provider-unavailable does not flip the once-only guard, success records bounded repo/worktree/adapter/session/registration counters, existing spawn-written rows are not clobbered, SSH repos skip worktree enumeration, adapter partial failure remains non-throwing, and large session lists are processed without spread limits.", + "commands": [ + "pnpm exec vitest run --config config/vitest.config.ts src/main/memory/hydrate-local-pty-registry.test.ts" + ], + "testFiles": ["src/main/memory/hydrate-local-pty-registry.test.ts"], + "runtimeBudget": { + "p95Seconds": 30, + "scope": "focused main unit test" + }, + "flakeHistory": { + "status": "unknown", + "evidence": "New experimental diagnostic gate; no CI soak history yet." + }, + "redGreenEvidence": { + "status": "partial", + "evidence": "Tests encode bounded counter shape and partial-failure behavior. Needs saved intentional-break evidence and startup timing artifact before promotion." + }, + "performanceBudget": { + "required": true, + "evidence": "The command asserts boot-scoped counters and large-list bounded iteration. It does not measure full Electron first-paint timing." + }, + "promotionCriteria": [ + "Attach a startup timing artifact proving hydration remains off the first usable UI critical path.", + "Run in soak for at least 100 consecutive passes or 14 days across required CI platforms.", + "Add upgrade fixture coverage that exercises a real daemon warm reattach profile." + ], + "knownGaps": [ + "Current command is a main-unit hydrator test, not an Electron startup timing benchmark.", + "Current command does not exercise real daemon sockets across macOS, Linux, or Windows.", + "Current command does not validate SSH/remote provider live behavior because those records are intentionally skipped for local process sampling." + ], + "demotionRule": "Demote or quarantine if counters become unbounded, raw terminal output is added to diagnostics, or hydration moves onto the first-UI critical path." } ] } diff --git a/src/main/daemon/daemon-pty-adapter.test.ts b/src/main/daemon/daemon-pty-adapter.test.ts index 539f66b5582..1ead6cde5d9 100644 --- a/src/main/daemon/daemon-pty-adapter.test.ts +++ b/src/main/daemon/daemon-pty-adapter.test.ts @@ -399,6 +399,8 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { expect(procs[0]).toHaveProperty('id') expect(procs[0]).toHaveProperty('cwd') expect(procs[0]).toHaveProperty('title') + expect(adapter.hasPty(procs[0]!.id)).toBe(true) + expect(adapter.hasPty(procs[1]!.id)).toBe(true) }) }) diff --git a/src/main/daemon/daemon-pty-adapter.ts b/src/main/daemon/daemon-pty-adapter.ts index b3252aae307..330cee556d0 100644 --- a/src/main/daemon/daemon-pty-adapter.ts +++ b/src/main/daemon/daemon-pty-adapter.ts @@ -472,11 +472,16 @@ export class DaemonPtyAdapter implements IPtyProvider { const result = await this.client.request('listSessions', undefined) return result.sessions .filter((s) => s.isAlive) - .map((s) => ({ - id: s.sessionId, - cwd: s.cwd ?? '', - title: 'shell' - })) + .map((s) => { + // Why: discovery can be the first production observation of a daemon + // session after restart; targeted hasPty must agree with that route. + this.activeSessionIds.add(s.sessionId) + return { + id: s.sessionId, + cwd: s.cwd ?? '', + title: 'shell' + } + }) } // Why: the Manage Sessions panel needs the full SessionInfo (pid, state, diff --git a/src/main/daemon/daemon-pty-router.test.ts b/src/main/daemon/daemon-pty-router.test.ts index 47d44aa807c..776b32f6918 100644 --- a/src/main/daemon/daemon-pty-router.test.ts +++ b/src/main/daemon/daemon-pty-router.test.ts @@ -145,6 +145,30 @@ describe('DaemonPtyRouter', () => { expect(current.hasPty).not.toHaveBeenCalledWith('legacy-session') }) + it('does not fan out targeted hasPty to legacy adapters for unknown ids', () => { + const current = createAdapter('current') + const legacy = createAdapter('legacy', ['legacy-session']) + const router = new DaemonPtyRouter({ current, legacy: [legacy] }) + + expect(router.hasPty('legacy-session')).toBe(false) + expect(current.hasPty).toHaveBeenCalledWith('legacy-session') + expect(legacy.hasPty).not.toHaveBeenCalledWith('legacy-session') + }) + + it('uses explicit listProcesses inventory to seed later targeted liveness routes', async () => { + const current = createAdapter('current') + const legacy = createAdapter('legacy', ['legacy-session']) + const router = new DaemonPtyRouter({ current, legacy: [legacy] }) + + await router.listProcesses() + + vi.mocked(current.hasPty).mockClear() + vi.mocked(legacy.hasPty).mockClear() + expect(router.hasPty('legacy-session')).toBe(true) + expect(current.hasPty).not.toHaveBeenCalledWith('legacy-session') + expect(legacy.hasPty).toHaveBeenCalledWith('legacy-session') + }) + it('fails listProcesses closed when any routed adapter cannot list sessions', async () => { const current = createAdapter('current', ['current-session']) const legacy = createAdapter('legacy', ['legacy-session']) diff --git a/src/main/daemon/daemon-pty-router.ts b/src/main/daemon/daemon-pty-router.ts index 2212635d1fa..4c445b477f8 100644 --- a/src/main/daemon/daemon-pty-router.ts +++ b/src/main/daemon/daemon-pty-router.ts @@ -35,7 +35,7 @@ export class DaemonPtyRouter implements IPtyProvider { try { const sessions = await adapter.listProcesses() for (const session of sessions) { - this.sessionAdapters.set(session.id, adapter) + this.rememberSessionAdapter(session.id, adapter) } } catch (error) { console.warn('[daemon] Failed to discover legacy daemon sessions', error) @@ -47,7 +47,7 @@ export class DaemonPtyRouter implements IPtyProvider { const adapter = opts.sessionId ? this.sessionAdapters.get(opts.sessionId) : undefined const target = adapter ?? this.current const result = await target.spawn(opts) - this.sessionAdapters.set(result.id, target) + this.rememberSessionAdapter(result.id, target) return result } @@ -60,7 +60,11 @@ export class DaemonPtyRouter implements IPtyProvider { if (routed) { return routed.hasPty(id) } - return this.current.hasPty(id) || this.legacy.some((adapter) => adapter.hasPty(id)) + const currentHasPty = this.current.hasPty(id) + if (currentHasPty) { + this.rememberSessionAdapter(id, this.current) + } + return currentHasPty } write(id: string, data: string): void { @@ -127,8 +131,20 @@ export class DaemonPtyRouter implements IPtyProvider { async listProcesses(): Promise<{ id: string; cwd: string; title: string }[]> { // Why: runtime exact-stop/liveness flows must fail closed if any adapter // cannot provide a trustworthy process list. - const results = await Promise.all(this.allAdapters().map((adapter) => adapter.listProcesses())) - return results.flat() + const results = await Promise.all( + this.allAdapters().map(async (adapter) => ({ + adapter, + sessions: await adapter.listProcesses() + })) + ) + const out: { id: string; cwd: string; title: string }[] = [] + for (const { adapter, sessions } of results) { + for (const session of sessions) { + this.rememberSessionAdapter(session.id, adapter) + out.push(session) + } + } + return out } async getDefaultShell(): Promise { @@ -248,6 +264,10 @@ export class DaemonPtyRouter implements IPtyProvider { return this.sessionAdapters.get(sessionId) ?? this.current } + private rememberSessionAdapter(sessionId: string, adapter: DaemonPtyAdapter): void { + this.sessionAdapters.set(sessionId, adapter) + } + private allAdapters(): DaemonPtyAdapter[] { return [this.current, ...this.legacy] } diff --git a/src/main/daemon/degraded-daemon-pty-provider.test.ts b/src/main/daemon/degraded-daemon-pty-provider.test.ts index 7bd656fc6fb..b7c160ed88c 100644 --- a/src/main/daemon/degraded-daemon-pty-provider.test.ts +++ b/src/main/daemon/degraded-daemon-pty-provider.test.ts @@ -151,6 +151,22 @@ describe('DegradedDaemonPtyProvider', () => { expect(fallback.write).not.toHaveBeenCalled() }) + it('keeps targeted liveness nullable until an explicit route is cached', async () => { + const current = createDaemonAdapter('daemon', ['daemon-session']) + const fallback = createProvider('fallback') + const provider = new DegradedDaemonPtyProvider({ current, legacy: [], fallback }) + + expect(provider.targetedHasPty('daemon-session')).toBeNull() + expect(current.hasPty).not.toHaveBeenCalled() + expect(fallback.hasPty).not.toHaveBeenCalled() + + await provider.listProcesses() + + expect(provider.targetedHasPty('daemon-session')).toBe(true) + expect(current.hasPty).toHaveBeenCalledWith('daemon-session') + expect(fallback.hasPty).not.toHaveBeenCalledWith('daemon-session') + }) + it('forwards replay output from fallback and daemon providers', () => { const current = createDaemonAdapter('daemon') const fallback = createProvider('fallback') diff --git a/src/main/daemon/degraded-daemon-pty-provider.ts b/src/main/daemon/degraded-daemon-pty-provider.ts index acf95ffcdfe..c22a86bfc22 100644 --- a/src/main/daemon/degraded-daemon-pty-provider.ts +++ b/src/main/daemon/degraded-daemon-pty-provider.ts @@ -80,6 +80,16 @@ export class DegradedDaemonPtyProvider implements IPtyProvider { return this.findProviderForExistingSession(id) !== null } + targetedHasPty(id: string): boolean | null { + const mapped = this.sessionProviders.get(id) + if (!mapped) { + // Why: hot-path liveness must not fan out across fallback/current/legacy + // providers. Explicit inventory paths seed this map before targeted use. + return null + } + return mapped.hasPty?.(id) ?? true + } + write(id: string, data: string): void { this.providerFor(id).write(id, data) } @@ -137,9 +147,19 @@ export class DegradedDaemonPtyProvider implements IPtyProvider { async listProcesses(): Promise<{ id: string; cwd: string; title: string }[]> { const results = await Promise.all( - this.allProviders().map((provider) => provider.listProcesses()) + this.allProviders().map(async (provider) => ({ + provider, + sessions: await provider.listProcesses() + })) ) - return results.flat() + const out: { id: string; cwd: string; title: string }[] = [] + for (const { provider, sessions } of results) { + for (const session of sessions) { + this.sessionProviders.set(session.id, provider) + out.push(session) + } + } + return out } async getDefaultShell(): Promise { diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index e8274e64192..2a5a1e6a9dd 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -3181,6 +3181,126 @@ describe('registerPtyHandlers', () => { }) }) + it('checks targeted PTY liveness through cached ownership without listing providers', async () => { + const hasPty = vi.fn((id: string) => id === 'pty-live') + const listProcesses = vi.fn(async () => [{ id: 'pty-live', cwd: '/repo', title: 'shell' }]) + setLocalPtyProvider({ + spawn: vi.fn(), + attach: vi.fn(), + hasPty, + write: vi.fn(), + resize: vi.fn(), + shutdown: vi.fn(), + sendSignal: vi.fn(), + getCwd: vi.fn(), + getInitialCwd: vi.fn(), + clearBuffer: vi.fn(), + acknowledgeDataEvent: vi.fn(), + hasChildProcesses: vi.fn(), + getForegroundProcess: vi.fn(), + serialize: vi.fn(), + revive: vi.fn(), + listProcesses, + getDefaultShell: vi.fn(), + getProfiles: vi.fn(), + onData: vi.fn(() => () => {}), + onReplay: vi.fn(() => () => {}), + onExit: vi.fn(() => () => {}) + } as never) + setPtyOwnership('pty-live', null) + setPtyOwnership('pty-dead', null) + registerPtyHandlers(mainWindow as never) + + await expect(handlers.get('pty:hasPty')!(null, { id: 'pty-live' })).resolves.toBe(true) + await expect(handlers.get('pty:hasPty')!(null, { id: 'pty-dead' })).resolves.toBe(false) + + expect(hasPty).toHaveBeenCalledTimes(2) + expect(listProcesses).not.toHaveBeenCalled() + deletePtyOwnership('pty-live') + deletePtyOwnership('pty-dead') + }) + + it('treats missing PTY ownership and provider liveness failures as unknown', async () => { + const hasPty = vi.fn(() => { + throw new Error('provider unavailable') + }) + const listProcesses = vi.fn(async () => []) + setLocalPtyProvider({ + spawn: vi.fn(), + attach: vi.fn(), + hasPty, + write: vi.fn(), + resize: vi.fn(), + shutdown: vi.fn(), + sendSignal: vi.fn(), + getCwd: vi.fn(), + getInitialCwd: vi.fn(), + clearBuffer: vi.fn(), + acknowledgeDataEvent: vi.fn(), + hasChildProcesses: vi.fn(), + getForegroundProcess: vi.fn(), + serialize: vi.fn(), + revive: vi.fn(), + listProcesses, + getDefaultShell: vi.fn(), + getProfiles: vi.fn(), + onData: vi.fn(() => () => {}), + onReplay: vi.fn(() => () => {}), + onExit: vi.fn(() => () => {}) + } as never) + setPtyOwnership('pty-owned', null) + registerPtyHandlers(mainWindow as never) + + await expect(handlers.get('pty:hasPty')!(null, { id: 'pty-unknown' })).resolves.toBeNull() + await expect(handlers.get('pty:hasPty')!(null, { id: 'pty-owned' })).resolves.toBeNull() + + expect(hasPty).toHaveBeenCalledTimes(1) + expect(hasPty).toHaveBeenCalledWith('pty-owned') + expect(listProcesses).not.toHaveBeenCalled() + deletePtyOwnership('pty-owned') + }) + + it('preserves provider receiver when calling targeted liveness', async () => { + const listProcesses = vi.fn(async () => []) + const provider = { + liveIds: new Set(['pty-live']), + spawn: vi.fn(), + attach: vi.fn(), + targetedHasPty(id: string) { + return this.liveIds.has(id) + }, + write: vi.fn(), + resize: vi.fn(), + shutdown: vi.fn(), + sendSignal: vi.fn(), + getCwd: vi.fn(), + getInitialCwd: vi.fn(), + clearBuffer: vi.fn(), + acknowledgeDataEvent: vi.fn(), + hasChildProcesses: vi.fn(), + getForegroundProcess: vi.fn(), + serialize: vi.fn(), + revive: vi.fn(), + listProcesses, + getDefaultShell: vi.fn(), + getProfiles: vi.fn(), + onData: vi.fn(() => () => {}), + onReplay: vi.fn(() => () => {}), + onExit: vi.fn(() => () => {}) + } + setLocalPtyProvider(provider as never) + setPtyOwnership('pty-live', null) + setPtyOwnership('pty-dead', null) + registerPtyHandlers(mainWindow as never) + + await expect(handlers.get('pty:hasPty')!(null, { id: 'pty-live' })).resolves.toBe(true) + await expect(handlers.get('pty:hasPty')!(null, { id: 'pty-dead' })).resolves.toBe(false) + + expect(listProcesses).not.toHaveBeenCalled() + deletePtyOwnership('pty-live') + deletePtyOwnership('pty-dead') + }) + it('kills app-scoped SSH PTY ids through the parsed provider when ownership is not rebuilt', async () => { const localShutdown = vi.fn() setLocalPtyProvider({ diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index 1ed6716db22..d1aa972224b 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -110,6 +110,9 @@ let localProvider: IPtyProvider = new LocalPtyProvider() type FreshLocalFallbackProvider = IPtyProvider & { routesFreshSpawnsToLocalProvider?: true } +type TargetedPtyLivenessProvider = IPtyProvider & { + targetedHasPty?: (id: string) => boolean | null +} const sshProviders = new Map() // Why: PTY IDs are assigned at spawn time with a connectionId, but subsequent // write/resize/kill calls only carry the PTY ID. This map lets us route @@ -953,6 +956,20 @@ function routesFreshSpawnsToLocalProvider( return (provider as FreshLocalFallbackProvider).routesFreshSpawnsToLocalProvider === true } +function getTargetedPtyLiveness( + provider: IPtyProvider, + ptyId: string +): boolean | Promise | null { + const targetedHasPty = (provider as TargetedPtyLivenessProvider).targetedHasPty + if (targetedHasPty) { + return targetedHasPty.call(provider, ptyId) + } + if (!provider.hasPty) { + return null + } + return provider.hasPty(ptyId) +} + /** Register an SSH PTY provider for a connection. */ export function registerSshPtyProvider(connectionId: string, provider: IPtyProvider): void { sshProviders.set(connectionId, provider) @@ -1215,6 +1232,7 @@ export function registerPtyHandlers( ipcMain.removeHandler('pty:spawn') ipcMain.removeHandler('pty:kill') ipcMain.removeHandler('pty:listSessions') + ipcMain.removeHandler('pty:hasPty') ipcMain.removeHandler('pty:hasChildProcesses') ipcMain.removeHandler('pty:getForegroundProcess') ipcMain.removeHandler('pty:getCwd') @@ -3288,6 +3306,26 @@ export function registerPtyHandlers( } ) + ipcMain.handle('pty:hasPty', async (_event, args: { id: string }): Promise => { + const ownedConnectionId = ptyOwnership.get(args.id) + const parsedSshId = ownedConnectionId === undefined ? parseAppSshPtyId(args.id) : null + if (ownedConnectionId === undefined && !parsedSshId) { + return null + } + const provider = parsedSshId + ? sshProviders.get(parsedSshId.connectionId) + : tryGetProviderForPty(args.id) + if (!provider) { + return null + } + try { + return await getTargetedPtyLiveness(provider, args.id) + } catch { + // Why: liveness may close a pane only on authoritative false. + return null + } + }) + ipcMain.handle( 'pty:hasChildProcesses', async (_event, args: { id: string }): Promise => { diff --git a/src/main/memory/hydrate-local-pty-registry.test.ts b/src/main/memory/hydrate-local-pty-registry.test.ts index 0854fa2493b..5ed0ecc309f 100644 --- a/src/main/memory/hydrate-local-pty-registry.test.ts +++ b/src/main/memory/hydrate-local-pty-registry.test.ts @@ -11,10 +11,12 @@ */ import { beforeEach, describe, expect, it, vi } from 'vitest' -import type { Repo } from '../../shared/types' +import type { Repo, WorktreeMeta } from '../../shared/types' import type { SessionInfo } from '../daemon/types' import type { DaemonPtyAdapter } from '../daemon/daemon-pty-adapter' +import { DaemonPtyRouter } from '../daemon/daemon-pty-router' import type { Store } from '../persistence' +import type { getLocalPtyRegistryHydrationDebugSnapshot as GetHydrationDebugSnapshotFn } from './hydrate-local-pty-registry' import type { hydrateLocalPtyRegistryAtBoot as HydrateFn } from './hydrate-local-pty-registry' import type { listRegisteredPtys as ListFn, @@ -46,9 +48,14 @@ vi.mock('../repo-worktrees', () => ({ // present at the type level. Filling them with placeholder values keeps the // test schema-compliant without coupling to anything the hydrator doesn't // touch. +type HydrationTestStore = Pick & { + getWorktreeMeta: (worktreeId: string) => WorktreeMeta | undefined +} + function makeStore( - repos: { id: string; connectionId?: string | null }[] = [] -): Pick { + repos: { id: string; connectionId?: string | null }[] = [], + worktreeMetaById: Record> = {} +): HydrationTestStore { const built: Repo[] = repos.map((r) => ({ id: r.id, path: `/tmp/${r.id}`, @@ -57,7 +64,11 @@ function makeStore( addedAt: 0, connectionId: r.connectionId ?? null })) - return { getRepos: () => built } + return { + getRepos: () => built, + getWorktreeMeta: (worktreeId: string) => + worktreeMetaById[worktreeId] as WorktreeMeta | undefined + } } function makeProvider(sessions: SessionInfo[]): Pick { @@ -66,6 +77,22 @@ function makeProvider(sessions: SessionInfo[]): Pick () => {}), + onExit: vi.fn(() => () => {}) + } as unknown as DaemonPtyAdapter +} + +function makeRejectingAdapter(error: Error): DaemonPtyAdapter { + return { + listSessions: vi.fn().mockRejectedValue(error), + onData: vi.fn(() => () => {}), + onExit: vi.fn(() => () => {}) + } as unknown as DaemonPtyAdapter +} + function makeLocalSessions(repoId: string, worktreePath: string, count: number): SessionInfo[] { const sessions: SessionInfo[] = [] for (let index = 0; index < count; index += 1) { @@ -87,6 +114,7 @@ function makeLocalSessions(repoId: string, worktreePath: string, count: number): // a coherent pair. async function loadFresh(): Promise<{ hydrate: typeof HydrateFn + getHydrationDebugSnapshot: typeof GetHydrationDebugSnapshotFn listRegisteredPtys: typeof ListFn registerPty: typeof RegisterFn unregisterPty: typeof UnregisterFn @@ -96,6 +124,7 @@ async function loadFresh(): Promise<{ const registryMod = await import('./pty-registry') return { hydrate: hydrateMod.hydrateLocalPtyRegistryAtBoot, + getHydrationDebugSnapshot: hydrateMod.getLocalPtyRegistryHydrationDebugSnapshot, listRegisteredPtys: registryMod.listRegisteredPtys, registerPty: registryMod.registerPty, unregisterPty: registryMod.unregisterPty @@ -109,13 +138,19 @@ describe('hydrateLocalPtyRegistryAtBoot', () => { }) it('no-op when daemon provider is null at first call (retries on later activation)', async () => { - const { hydrate, listRegisteredPtys } = await loadFresh() + const { hydrate, getHydrationDebugSnapshot, listRegisteredPtys } = await loadFresh() getDaemonProviderMock.mockReturnValue(null) await hydrate(makeStore([{ id: 'repo-a' }])) expect(listRegisteredPtys()).toHaveLength(0) expect(listRepoWorktreesMock).not.toHaveBeenCalled() + expect(getHydrationDebugSnapshot()).toMatchObject({ + phase: 'provider-unavailable', + hasHydrated: false, + repoCount: 0, + sessionsScannedCount: 0 + }) // Why: the design says the hasHydrated guard must stay false until a // provider is obtained, so a later macOS dock re-activation can retry. @@ -127,10 +162,16 @@ describe('hydrateLocalPtyRegistryAtBoot', () => { await hydrate(makeStore([{ id: 'repo-a' }])) expect(provider.listSessions).toHaveBeenCalledTimes(1) + expect(getHydrationDebugSnapshot()).toMatchObject({ + phase: 'complete', + hasHydrated: true, + repoCount: 1, + adapterCount: 1 + }) }) it('catches provider.listSessions rejection and does not throw', async () => { - const { hydrate, listRegisteredPtys } = await loadFresh() + const { hydrate, getHydrationDebugSnapshot, listRegisteredPtys } = await loadFresh() const provider = { listSessions: vi.fn().mockRejectedValue(new Error('socket EPIPE')) } @@ -145,10 +186,16 @@ describe('hydrateLocalPtyRegistryAtBoot', () => { warnSpy.mockRestore() expect(listRegisteredPtys()).toHaveLength(0) + expect(getHydrationDebugSnapshot()).toMatchObject({ + phase: 'partial', + failedAdapterCount: 1, + registeredCount: 0 + }) }) it('does not clobber a pre-existing registry pid with a null pid from listSessions', async () => { - const { hydrate, listRegisteredPtys, registerPty } = await loadFresh() + const { hydrate, getHydrationDebugSnapshot, listRegisteredPtys, registerPty } = + await loadFresh() const ptyId = 'repo-a::/local/Triton@@deadbeef' // Why: simulate the spawn-time path having already written the row @@ -180,10 +227,15 @@ describe('hydrateLocalPtyRegistryAtBoot', () => { expect(entry).toBeDefined() expect(entry!.pid).toBe(12345) expect(entry!.paneKey).toBe('tab-1:1') + expect(getHydrationDebugSnapshot()).toMatchObject({ + alreadyRegisteredCount: 1, + registeredCount: 0, + sessionsScannedCount: 1 + }) }) it('skips SSH repos before enumerating worktrees', async () => { - const { hydrate, listRegisteredPtys } = await loadFresh() + const { hydrate, getHydrationDebugSnapshot, listRegisteredPtys } = await loadFresh() const ptyId = 'repo-ssh::/remote/Stingray@@feedface' const provider = makeProvider([ @@ -201,10 +253,15 @@ describe('hydrateLocalPtyRegistryAtBoot', () => { // around `registerPty` in `pty.ts`'s `pty:spawn` handler. expect(listRegisteredPtys()).toHaveLength(0) expect(listRepoWorktreesMock).not.toHaveBeenCalled() + expect(getHydrationDebugSnapshot()).toMatchObject({ + skippedRemoteRepoCount: 1, + worktreeEnumerationCount: 0, + skippedUnknownWorktreeCount: 1 + }) }) it('registers a local session whose worktree is in the store with the daemon-published pid', async () => { - const { hydrate, listRegisteredPtys } = await loadFresh() + const { hydrate, getHydrationDebugSnapshot, listRegisteredPtys } = await loadFresh() const ptyId = 'repo-a::/local/Triton@@cafebabe' const provider = makeProvider([ @@ -221,6 +278,106 @@ describe('hydrateLocalPtyRegistryAtBoot', () => { expect(entry).toBeDefined() expect(entry!.pid).toBe(4242) expect(entry!.worktreeId).toBe('repo-a::/local/Triton') + expect(getHydrationDebugSnapshot()).toMatchObject({ + phase: 'complete', + repoCount: 1, + localRepoCount: 1, + worktreeEnumerationCount: 1, + worktreeCount: 1, + adapterCount: 1, + sessionsScannedCount: 1, + registeredCount: 1, + failedAdapterCount: 0 + }) + expect(getHydrationDebugSnapshot().elapsedMs).not.toBeNull() + }) + + it('registers a daemon session minted under a prior worktree id to the current worktree', async () => { + const { hydrate, getHydrationDebugSnapshot, listRegisteredPtys } = await loadFresh() + + const currentWorktreeId = 'repo-a::/local/NewTriton' + const priorWorktreeId = 'repo-a::/local/OldTriton' + const ptyId = `${priorWorktreeId}@@cafebabe` + const provider = makeProvider([ + { sessionId: ptyId, pid: 4242, cwd: '/local/OldTriton' } as unknown as SessionInfo + ]) + getDaemonProviderMock.mockReturnValue(provider) + listRepoWorktreesMock.mockResolvedValue([ + { path: '/local/NewTriton', head: '', branch: '', isBare: false, isMainWorktree: true } + ]) + + await hydrate( + makeStore([{ id: 'repo-a', connectionId: null }], { + [currentWorktreeId]: { priorWorktreeIds: [priorWorktreeId] } + }) + ) + + const entry = listRegisteredPtys().find((p) => p.ptyId === ptyId) + expect(entry).toBeDefined() + expect(entry!.pid).toBe(4242) + expect(entry!.sessionId).toBe(ptyId) + expect(entry!.worktreeId).toBe(currentWorktreeId) + expect(getHydrationDebugSnapshot()).toMatchObject({ + phase: 'complete', + sessionsScannedCount: 1, + registeredCount: 1, + skippedUnknownWorktreeCount: 0 + }) + }) + + it('keeps successful adapter hydration observable when one adapter fails', async () => { + const { hydrate, getHydrationDebugSnapshot, listRegisteredPtys } = await loadFresh() + const ptyId = 'repo-a::/local/Triton@@cafebabe' + const provider = { + getAllAdapters: () => [ + makeAdapter([ + { sessionId: ptyId, pid: 4242, cwd: '/local/Triton' } as unknown as SessionInfo + ]), + makeRejectingAdapter(new Error('legacy unavailable')) + ] + } + getDaemonProviderMock.mockReturnValue(provider) + listRepoWorktreesMock.mockResolvedValue([ + { path: '/local/Triton', head: '', branch: '', isBare: false, isMainWorktree: true } + ]) + + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + await hydrate(makeStore([{ id: 'repo-a', connectionId: null }])) + warnSpy.mockRestore() + + expect(listRegisteredPtys()).toHaveLength(1) + expect(getHydrationDebugSnapshot()).toMatchObject({ + phase: 'partial', + adapterCount: 2, + sessionsScannedCount: 1, + registeredCount: 1, + failedAdapterCount: 1 + }) + }) + + it('hydrates through a real daemon router adapter inventory', async () => { + const { hydrate, getHydrationDebugSnapshot, listRegisteredPtys } = await loadFresh() + const ptyId = 'repo-a::/local/Triton@@cafebabe' + const provider = new DaemonPtyRouter({ + current: makeAdapter([ + { sessionId: ptyId, pid: 4242, cwd: '/local/Triton' } as unknown as SessionInfo + ]), + legacy: [] + }) + getDaemonProviderMock.mockReturnValue(provider) + listRepoWorktreesMock.mockResolvedValue([ + { path: '/local/Triton', head: '', branch: '', isBare: false, isMainWorktree: true } + ]) + + await hydrate(makeStore([{ id: 'repo-a', connectionId: null }])) + + expect(listRegisteredPtys()).toHaveLength(1) + expect(getHydrationDebugSnapshot()).toMatchObject({ + phase: 'complete', + adapterCount: 1, + sessionsScannedCount: 1, + registeredCount: 1 + }) }) it('hydrates large daemon session lists', async () => { diff --git a/src/main/memory/hydrate-local-pty-registry.ts b/src/main/memory/hydrate-local-pty-registry.ts index 4bb82763ec7..53bd894b55e 100644 --- a/src/main/memory/hydrate-local-pty-registry.ts +++ b/src/main/memory/hydrate-local-pty-registry.ts @@ -26,6 +26,7 @@ import { listRegisteredPtys, registerPty } from './pty-registry' import { listRepoWorktrees } from '../repo-worktrees' import { parsePtySessionId } from '../../shared/pty-session-id-format' import type { Store } from '../persistence' +import type { WorktreeMeta } from '../../shared/types' // Why: `attachMainWindowServices` runs on every macOS dock re-activation // (see `app.on('activate', ...)` in src/main/index.ts), so this module @@ -35,6 +36,71 @@ import type { Store } from '../persistence' // re-activations. let hasHydrated = false +export type LocalPtyRegistryHydrationDebugSnapshot = { + phase: 'not-started' | 'provider-unavailable' | 'running' | 'complete' | 'partial' | 'failed' + hasHydrated: boolean + startedAtMs: number | null + finishedAtMs: number | null + elapsedMs: number | null + repoCount: number + localRepoCount: number + skippedRemoteRepoCount: number + worktreeEnumerationCount: number + worktreeCount: number + adapterCount: number + sessionsScannedCount: number + registeredCount: number + alreadyRegisteredCount: number + skippedUnparseableCount: number + skippedUnknownWorktreeCount: number + skippedRemoteWorktreeCount: number + failedAdapterCount: number +} + +function emptyHydrationDebugSnapshot(): LocalPtyRegistryHydrationDebugSnapshot { + return { + phase: 'not-started', + hasHydrated: false, + startedAtMs: null, + finishedAtMs: null, + elapsedMs: null, + repoCount: 0, + localRepoCount: 0, + skippedRemoteRepoCount: 0, + worktreeEnumerationCount: 0, + worktreeCount: 0, + adapterCount: 0, + sessionsScannedCount: 0, + registeredCount: 0, + alreadyRegisteredCount: 0, + skippedUnparseableCount: 0, + skippedUnknownWorktreeCount: 0, + skippedRemoteWorktreeCount: 0, + failedAdapterCount: 0 + } +} + +let lastHydrationDebugSnapshot = emptyHydrationDebugSnapshot() + +function finishHydrationDebugSnapshot( + phase: LocalPtyRegistryHydrationDebugSnapshot['phase'] +): void { + const finishedAtMs = Date.now() + lastHydrationDebugSnapshot = { + ...lastHydrationDebugSnapshot, + phase, + finishedAtMs, + elapsedMs: + lastHydrationDebugSnapshot.startedAtMs === null + ? null + : Math.max(0, finishedAtMs - lastHydrationDebugSnapshot.startedAtMs) + } +} + +export function getLocalPtyRegistryHydrationDebugSnapshot(): LocalPtyRegistryHydrationDebugSnapshot { + return { ...lastHydrationDebugSnapshot } +} + /** * Read the live daemon session list and register every local session * the registry doesn't already know about. @@ -53,13 +119,21 @@ let hasHydrated = false * union still covers that case until the daemon comes back. Any failure * here is a coverage degradation, not a correctness regression. */ -export async function hydrateLocalPtyRegistryAtBoot(store: Pick): Promise { +type HydrationStore = Pick & { + getWorktreeMeta?: (worktreeId: string) => WorktreeMeta | undefined +} + +export async function hydrateLocalPtyRegistryAtBoot(store: HydrationStore): Promise { try { if (hasHydrated) { return } const provider = getDaemonProvider() if (!provider) { + lastHydrationDebugSnapshot = { + ...emptyHydrationDebugSnapshot(), + phase: 'provider-unavailable' + } // Why: leave hasHydrated false so a later activation (after the // daemon comes up) can retry. return @@ -69,24 +143,48 @@ export async function hydrateLocalPtyRegistryAtBoot(store: Pick() + lastHydrationDebugSnapshot.repoCount = repos.length + const repoConnectionIdByWorktreeId = new Map< + string, + { connectionId: string | null; currentWorktreeId: string } + >() for (const repo of repos) { const connectionId = repo.connectionId ?? null if (connectionId) { + lastHydrationDebugSnapshot.skippedRemoteRepoCount += 1 // Why: SSH PTYs are never registered for local process sampling, so // avoid startup SSH/git enumeration for repos we will skip anyway. continue } + lastHydrationDebugSnapshot.localRepoCount += 1 + lastHydrationDebugSnapshot.worktreeEnumerationCount += 1 const worktrees = await listRepoWorktrees(repo) + lastHydrationDebugSnapshot.worktreeCount += worktrees.length for (const wt of worktrees) { const worktreeId = `${repo.id}::${wt.path}` - repoConnectionIdByWorktreeId.set(worktreeId, connectionId) + repoConnectionIdByWorktreeId.set(worktreeId, { + connectionId, + currentWorktreeId: worktreeId + }) + const priorWorktreeIds = store.getWorktreeMeta?.(worktreeId)?.priorWorktreeIds ?? [] + for (const priorWorktreeId of priorWorktreeIds) { + repoConnectionIdByWorktreeId.set(priorWorktreeId, { + connectionId, + currentWorktreeId: worktreeId + }) + } } } @@ -94,7 +192,7 @@ export async function hydrateLocalPtyRegistryAtBoot(store: Pick p.ptyId)) @@ -104,10 +202,12 @@ export async function hydrateLocalPtyRegistryAtBoot(store: Pick 0 ? 'partial' : 'complete' + ) } catch (err) { + finishHydrationDebugSnapshot('failed') console.warn( '[memory] Boot-time pty-registry hydration failed:', err instanceof Error ? err.message : String(err) @@ -141,25 +249,34 @@ export async function hydrateLocalPtyRegistryAtBoot(store: Pick { // Why: the router fans `listSessions` out across current + legacy adapters // so we get every protocol-version daemon's sessions; the bare-adapter // fallback is only the in-process restart edge case. - const adapters: readonly DaemonPtyAdapter[] = - provider instanceof DaemonPtyRouter || provider instanceof DegradedDaemonPtyProvider - ? provider.getAllAdapters() - : [provider] + const providerWithAdapters = provider as { getAllAdapters?: () => readonly DaemonPtyAdapter[] } + let adapters: readonly DaemonPtyAdapter[] + if (typeof providerWithAdapters.getAllAdapters === 'function') { + adapters = providerWithAdapters.getAllAdapters() + } else if (provider instanceof DaemonPtyRouter || provider instanceof DegradedDaemonPtyProvider) { + adapters = provider.getAllAdapters() + } else { + adapters = [provider] + } + debugSnapshot.adapterCount = adapters.length const out: SessionInfo[] = [] for (const adapter of adapters) { try { const sessions = await adapter.listSessions() + debugSnapshot.sessionsScannedCount += sessions.length // Why: warm reattach can discover many daemon sessions at once; spreading // listSessions() into push can exceed JavaScript's argument limit. for (const session of sessions) { out.push(session) } } catch (err) { + debugSnapshot.failedAdapterCount += 1 // Why: a single adapter failing should not abort hydration of the // others — the current adapter and any legacy daemons each have // their own socket and one being unreachable is normal. diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 19a84e49864..87f749c46f9 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -1156,6 +1156,7 @@ export type PreloadApi = { getCwd: (id: string) => Promise getSize: (id: string) => Promise<{ cols: number; rows: number } | null> listSessions: () => Promise<{ id: string; cwd: string; title: string }[]> + hasPty: (id: string) => Promise getMainBufferSnapshot: ( id: string, opts?: { scrollbackRows?: number } diff --git a/src/preload/index.ts b/src/preload/index.ts index d415e0efdfa..7835bb622f4 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -877,6 +877,9 @@ const api = { * re-assert, instead of trusting the size it last fired blind. */ getSize: (id: string): Promise<{ cols: number; rows: number } | null> => ipcRenderer.invoke('pty:getSize', { id }), + /** Targeted liveness for one PTY id. Null means provider state is unknown + * and must never be treated as evidence that a pane is dead. */ + hasPty: (id: string): Promise => ipcRenderer.invoke('pty:hasPty', { id }), onData: ( callback: (data: { id: string; data: string; seq?: number; rawLength?: number }) => void diff --git a/src/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsx b/src/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsx index 67095fee7c8..be4f4ddb3a5 100644 --- a/src/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsx +++ b/src/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsx @@ -31,7 +31,6 @@ import { cn } from '@/lib/utils' import { useMountedRef } from '@/hooks/useMountedRef' import { activateAndRevealWorktree } from '@/lib/worktree-activation' import { activateTabAndFocusPane } from '@/lib/activate-tab-and-focus-pane' -import { installWindowVisibilityInterval } from '@/lib/window-visibility-interval' import { useAppStore } from '../../store' import { useWorktreeMap } from '../../store/selectors' import { runWorktreeDelete } from '../sidebar/delete-worktree-flow' @@ -877,24 +876,22 @@ export function ResourceUsageStatusSegment({ } const spaceScanReady = nextSpaceScanSnapshot.ready - // Poll memory + sessions when popover is open. Sessions also poll in the - // background at a slower rate so the badge count stays reasonably fresh - // without keeping the Memory IPC hot. + // Poll memory and broad session inventory only while the management popover is open. useEffect(() => { if (!open || runtimeEnvironmentActive) { return } void fetchSnapshot() void refreshSessions() - // Why: sessions already have an always-on poll in the effect below; only - // the memory snapshot is gated on the popover being open. Stacking a - // second sessions interval here doubled IPC traffic while the popover - // was open. const memTimer = window.setInterval(() => { void fetchSnapshot() }, POLL_MS) + const sessionTimer = window.setInterval(() => { + void refreshSessions() + }, SESSIONS_POLL_MS) return () => { window.clearInterval(memTimer) + window.clearInterval(sessionTimer) } }, [open, runtimeEnvironmentActive, fetchSnapshot, refreshSessions]) @@ -902,16 +899,8 @@ export function ResourceUsageStatusSegment({ if (runtimeEnvironmentActive) { setSessions([]) setSessionsError(false) - return } - // Why: the closed-popover badge is informational. Polling daemon sessions - // while the whole window is hidden keeps IPC and daemon list calls hot for - // no visible UI; visibility refreshes catch the badge up immediately. - return installWindowVisibilityInterval({ - run: () => void refreshSessions(), - intervalMs: SESSIONS_POLL_MS - }) - }, [runtimeEnvironmentActive, refreshSessions]) + }, [runtimeEnvironmentActive]) const repoDisplayNameById = useMemo(() => { const map = new Map() @@ -1132,8 +1121,8 @@ export function ResourceUsageStatusSegment({ if (orphans.length === 0) { return } - // Why: optimistic removal so the rows disappear immediately rather than - // lingering up to SESSIONS_POLL_MS while the daemon-side list reconciles. + // Why: optimistic removal keeps the management row from reappearing while + // the explicit post-kill daemon inventory refresh settles. const orphanIds = new Set(orphans.map((s) => s.id)) setSessions((prev) => prev.filter((s) => !orphanIds.has(s.id))) await Promise.allSettled(orphans.map((s) => window.api.pty.kill(s.id))) @@ -1147,8 +1136,8 @@ export function ResourceUsageStatusSegment({ const target = killConfirm setKilling(true) // Why: optimistic removal — the kill X was on the row that's about to be - // unmounted, so updating local state immediately avoids a flash where the - // dialog closes but the killed row stays for up to 10s. + // unmounted, so updating local state avoids a flash before the explicit + // post-kill daemon inventory refresh settles. setSessions((prev) => prev.filter((s) => s.id !== target.sessionId)) try { await window.api.pty.kill(target.sessionId) diff --git a/src/renderer/src/components/status-bar/resource-manager-session-polling.test.ts b/src/renderer/src/components/status-bar/resource-manager-session-polling.test.ts new file mode 100644 index 00000000000..1e3ef6ab60c --- /dev/null +++ b/src/renderer/src/components/status-bar/resource-manager-session-polling.test.ts @@ -0,0 +1,29 @@ +import { readFileSync } from 'node:fs' +import { fileURLToPath } from 'node:url' +import { describe, expect, it } from 'vitest' + +function readResourceUsageStatusSegmentSource(): string { + return readFileSync( + fileURLToPath(new URL('./ResourceUsageStatusSegment.tsx', import.meta.url)), + 'utf8' + ) +} + +describe('resource manager session polling', () => { + it('keeps broad PTY session inventory scoped to the open popover', () => { + const source = readResourceUsageStatusSegmentSource() + const pollingSectionStart = source.indexOf( + '// Poll memory and broad session inventory only while the management popover is open.' + ) + const pollingSectionEnd = source.indexOf('const repoDisplayNameById') + expect(pollingSectionStart).toBeGreaterThan(-1) + expect(pollingSectionEnd).toBeGreaterThan(pollingSectionStart) + + const pollingSection = source.slice(pollingSectionStart, pollingSectionEnd) + expect(pollingSection).toContain('if (!open || runtimeEnvironmentActive) {') + expect(pollingSection).not.toContain('installWindowVisibilityInterval({') + expect(pollingSection).toContain('window.setInterval(() => {\n void refreshSessions()') + expect(pollingSection).toContain('}, SESSIONS_POLL_MS)') + expect(source).toContain('const SESSIONS_POLL_MS') + }) +}) diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index 5daf3a42484..16811238504 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -610,6 +610,7 @@ describe('connectPanePty', () => { pty: { signal: vi.fn(), listSessions: vi.fn().mockResolvedValue([]), + hasPty: vi.fn().mockResolvedValue(true), getSize: vi.fn().mockResolvedValue(null), getMainBufferSnapshot: vi.fn().mockResolvedValue(null), getForegroundProcess: vi.fn().mockResolvedValue(null), @@ -11087,10 +11088,9 @@ describe('connectPanePty', () => { }) describe('input liveness re-check IPC gating (perf)', () => { - // Why (perf regression guard): listSessions() is a renderer→main→daemon - // round-trip. The input-driven liveness re-check must fire at most once per - // resume window, never once per keystroke, or every healthy local pane puts - // a process-enumeration round-trip on the typing hot path. + // Why (perf regression guard): broad listSessions() is a renderer→main→daemon + // inventory round-trip. The input-driven liveness re-check must use targeted + // hasPty at most once per resume window, never once per keystroke. async function connectActivePaneWithInput(): Promise<{ binding: { noteVisibilityResume: () => void } typeKeystroke: (data?: string) => void @@ -11114,9 +11114,11 @@ describe('connectPanePty', () => { } } - it('does not fire listSessions for first input on a fresh mount', async () => { + it('does not fire liveness IPC for first input on a fresh mount', async () => { const listSessions = vi.mocked(window.api.pty.listSessions) + const hasPty = vi.mocked(window.api.pty.hasPty) listSessions.mockClear() + hasPty.mockClear() const { typeKeystroke } = await connectActivePaneWithInput() for (let i = 0; i < 25; i++) { @@ -11124,39 +11126,66 @@ describe('connectPanePty', () => { } expect(listSessions).not.toHaveBeenCalled() + expect(hasPty).not.toHaveBeenCalled() }) - it('fires listSessions once for the first input after a visibility resume', async () => { + it('uses targeted hasPty once for the first input after a visibility resume', async () => { const listSessions = vi.mocked(window.api.pty.listSessions) + const hasPty = vi.mocked(window.api.pty.hasPty) listSessions.mockClear() + hasPty.mockClear() const { binding, typeKeystroke } = await connectActivePaneWithInput() binding.noteVisibilityResume() typeKeystroke('a') typeKeystroke('b') - expect(listSessions).toHaveBeenCalledTimes(1) + expect(hasPty).toHaveBeenCalledTimes(1) + expect(hasPty).toHaveBeenCalledWith('pty-pane-2') + expect(listSessions).not.toHaveBeenCalled() }) it('re-arms one re-check after a second visibility resume', async () => { const listSessions = vi.mocked(window.api.pty.listSessions) + const hasPty = vi.mocked(window.api.pty.hasPty) listSessions.mockClear() + hasPty.mockClear() const { binding, typeKeystroke } = await connectActivePaneWithInput() binding.noteVisibilityResume() typeKeystroke('a') typeKeystroke('b') - expect(listSessions).toHaveBeenCalledTimes(1) + expect(hasPty).toHaveBeenCalledTimes(1) + expect(listSessions).not.toHaveBeenCalled() binding.noteVisibilityResume() typeKeystroke('c') typeKeystroke('d') - expect(listSessions).toHaveBeenCalledTimes(2) + expect(hasPty).toHaveBeenCalledTimes(2) + expect(listSessions).not.toHaveBeenCalled() + }) + + it('does not close on unknown or rejected targeted liveness', async () => { + const hasPty = vi.mocked(window.api.pty.hasPty) + hasPty.mockResolvedValueOnce(null).mockRejectedValueOnce(new Error('provider unavailable')) + const { binding, typeKeystroke } = await connectActivePaneWithInput() + + binding.noteVisibilityResume() + typeKeystroke('a') + await flushAsyncTicks() + binding.noteVisibilityResume() + typeKeystroke('b') + await flushAsyncTicks() + + expect(hasPty).toHaveBeenCalledTimes(2) + expect(window.api.pty.listSessions).not.toHaveBeenCalled() }) it('never fires listSessions for a remote: web-runtime pane (liveness owned by host snapshot)', async () => { const listSessions = vi.mocked(window.api.pty.listSessions) + const hasPty = vi.mocked(window.api.pty.hasPty) listSessions.mockClear() + hasPty.mockClear() const { connectPanePty } = await import('./pty-connection') // Remote panes report a null connectionId but a remote:-prefixed ptyId, so // the SSH/connectionId guard alone would not exclude them — the remote @@ -11179,11 +11208,14 @@ describe('connectPanePty', () => { sendTerminalInputThroughPane(pane, 'y') expect(listSessions).not.toHaveBeenCalled() + expect(hasPty).not.toHaveBeenCalled() }) it('never fires listSessions for an SSH pane after resume', async () => { const listSessions = vi.mocked(window.api.pty.listSessions) + const hasPty = vi.mocked(window.api.pty.hasPty) listSessions.mockClear() + hasPty.mockClear() const { connectPanePty } = await import('./pty-connection') const transport = createMockTransport('ssh-pty-2') transport.getConnectionId.mockReturnValue('ssh-connection-1') @@ -11203,6 +11235,7 @@ describe('connectPanePty', () => { sendTerminalInputThroughPane(pane, 'y') expect(listSessions).not.toHaveBeenCalled() + expect(hasPty).not.toHaveBeenCalled() }) }) diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index d6db319ac99..8e00388c397 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -22,7 +22,11 @@ import { hasCachedWindowsTerminalCapabilities } from '@/lib/windows-terminal-capabilities' import { shouldSeedCacheTimerOnInitialTitle } from './cache-timer-seeding' -import { shouldReconcileDeadSession } from './terminal-dead-session-reconcile' +import { + shouldReconcileDeadSession, + shouldReconcileMissingSession, + type HasPty +} from './terminal-dead-session-reconcile' import type { PtyConnectionDeps } from './pty-connection-types' import { safeFit } from '@/lib/pane-manager/pane-tree-ops' import { getFitOverrideForPty, bindPanePtyId } from '@/lib/pane-manager/mobile-fit-overrides' @@ -521,6 +525,7 @@ type PanePtyBinding = IDisposable & { syncProcessTracking: () => void noteVisibilityResume: () => void reconcileIfSessionDead: (liveSessionIds: Set, snapshotRequestedAt?: number) => void + reconcileIfSessionMissing: (hasPty: HasPty, livenessRequestedAt?: number) => void } function isAgentTaskCompleteNotificationEnabled(): boolean { @@ -4609,9 +4614,9 @@ export function connectPanePty( // Why: on visibility resume a pane may still be bound to a daemon session // reaped while hidden (the missed-exit defect). Route it through the SAME // teardown a real onExit runs. Re-validate identity at apply time so a - // reattach racing the listSessions snapshot is never clobbered, and respect - // the remote/SSH guards. Suppression semantics come for free via onExit - // (which consults consumeSuppressedPtyExit) plus the per-ptyId guard above. + // reattach racing a liveness response is never clobbered, and respect the + // remote/SSH guards. Suppression semantics come for free via onExit (which + // consults consumeSuppressedPtyExit) plus the per-ptyId guard above. const reconcileIfSessionDead = ( liveSessionIds: Set, snapshotRequestedAt?: number @@ -4638,9 +4643,55 @@ export function connectPanePty( onExit(currentPtyId) } - // Why (perf + startup correctness): listSessions() is authoritative only - // after a real visibility resume. Fresh PTY startup can briefly lag the daemon - // listing, so newborn terminals start disarmed and noteVisibilityResume grants + const reconcileIfSessionMissing = ( + hasPty: HasPty, + livenessRequestedAt = performance.now() + ): void => { + const requestedPtyId = transport.getPtyId() + if ( + !requestedPtyId || + requestedPtyId === handledExitPtyId || + isRemoteRuntimePtyId(requestedPtyId) || + transport.getConnectionId?.() != null + ) { + return + } + + let livenessPromise: Promise + try { + livenessPromise = Promise.resolve(hasPty(requestedPtyId)) + } catch { + return + } + + void livenessPromise + .then((isLive) => { + if (disposed) { + return + } + const currentPtyId = transport.getPtyId() + if ( + !currentPtyId || + currentPtyId !== requestedPtyId || + handledExitPtyId === currentPtyId || + !shouldReconcileMissingSession({ + ptyId: currentPtyId, + connectionId: transport.getConnectionId?.(), + isLive, + ptyBoundAt: activePanePtyBindingBoundAt, + livenessRequestedAt + }) + ) { + return + } + onExit(currentPtyId) + }) + .catch(() => {}) + } + + // Why (perf + startup correctness): targeted hasPty liveness is only useful + // after a real visibility resume. Fresh PTY startup can briefly lag provider + // state, so newborn terminals start disarmed and noteVisibilityResume grants // exactly one first-input liveness probe for the next resume window. let livenessRecheckArmedForResume = false @@ -4664,24 +4715,17 @@ export function connectPanePty( !currentPtyId || // Why: this ptyId's exit was already handled — nothing left to reconcile. handledExitPtyId === currentPtyId || - // Why: `remote:` web-runtime liveness is owned by the host snapshot, not - // listSessions; skip here so a remote pane's keystrokes never put a local - // daemon round-trip on the typing hot path (reconcile would no-op anyway). + // Why: `remote:` web-runtime liveness is owned by the host snapshot; skip + // here so a remote pane's keystrokes never put a local daemon round-trip + // on the typing hot path (reconcile would no-op anyway). isRemoteRuntimePtyId(currentPtyId) || (currentConnectionId !== null && currentConnectionId !== undefined) ) { return } - // Why: capture request time before the round-trip so a pane that bound after - // this request is not torn down by its (pre-bind) stale snapshot. - const requestedAt = performance.now() - void window.api.pty - .listSessions() - .then((sessions) => { - reconcileIfSessionDead(new Set(sessions.map((session) => session.id)), requestedAt) - }) - // Why: a rejected listing is "unknown" — never close a pane on it. - .catch(() => {}) + if (typeof window.api.pty.hasPty === 'function') { + reconcileIfSessionMissing(window.api.pty.hasPty) + } } return { @@ -4690,7 +4734,7 @@ export function connectPanePty( }, // Why: re-arm the once-per-resume input re-check when the pane becomes // visible again. Called from the lifecycle visibility effect; the gate - // keeps the typing hot path off the listSessions IPC between resumes. + // keeps repeated typing off liveness IPC between resumes. noteVisibilityResume() { livenessRecheckArmedForResume = true // Why: re-assert the PTY size on resume so a resize that was dropped while @@ -4699,6 +4743,7 @@ export function connectPanePty( reassertPtySizeOnResume() }, reconcileIfSessionDead, + reconcileIfSessionMissing, dispose() { disposed = true // Why: the post-spawn reconcile polls across frames; cancel its pending diff --git a/src/renderer/src/components/terminal-pane/terminal-dead-session-reconcile.test.ts b/src/renderer/src/components/terminal-pane/terminal-dead-session-reconcile.test.ts index 9e5c5d2bc1f..9b508d4aed5 100644 --- a/src/renderer/src/components/terminal-pane/terminal-dead-session-reconcile.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-dead-session-reconcile.test.ts @@ -1,7 +1,9 @@ import { describe, expect, it, vi } from 'vitest' import { reconcileDeadSessions, - shouldReconcileDeadSession + reconcileMissingSessions, + shouldReconcileDeadSession, + shouldReconcileMissingSession } from './terminal-dead-session-reconcile' describe('shouldReconcileDeadSession', () => { @@ -128,6 +130,58 @@ describe('shouldReconcileDeadSession', () => { }) }) +describe('shouldReconcileMissingSession', () => { + it('reconciles only on an authoritative false liveness result', () => { + expect( + shouldReconcileMissingSession({ + ptyId: 'wt@@dead', + connectionId: null, + isLive: false + }) + ).toBe(true) + expect( + shouldReconcileMissingSession({ + ptyId: 'wt@@maybe', + connectionId: null, + isLive: null + }) + ).toBe(false) + expect( + shouldReconcileMissingSession({ + ptyId: 'wt@@live', + connectionId: null, + isLive: true + }) + ).toBe(false) + }) + + it('keeps SSH, remote, and newborn bindings safe for missing-session checks', () => { + expect( + shouldReconcileMissingSession({ + ptyId: 'wt@@ssh', + connectionId: 'ssh-1', + isLive: false + }) + ).toBe(false) + expect( + shouldReconcileMissingSession({ + ptyId: 'remote:env-1:abc', + connectionId: null, + isLive: false + }) + ).toBe(false) + expect( + shouldReconcileMissingSession({ + ptyId: 'wt@@newborn', + connectionId: null, + isLive: false, + ptyBoundAt: 100, + livenessRequestedAt: 100 + }) + ).toBe(false) + }) +}) + describe('reconcileDeadSessions', () => { function createBinding() { return { @@ -189,3 +243,26 @@ describe('reconcileDeadSessions', () => { expect(requestedAt).toBeLessThanOrEqual(after) }) }) + +describe('reconcileMissingSessions', () => { + it('invokes each binding with targeted liveness and a shared request timestamp', () => { + const bindingA = { + reconcileIfSessionMissing: + vi.fn<(hasPty: (ptyId: string) => Promise, requestedAt?: number) => void>() + } + const bindingB = { + reconcileIfSessionMissing: + vi.fn<(hasPty: (ptyId: string) => Promise, requestedAt?: number) => void>() + } + const hasPty = vi.fn(async () => true) + + reconcileMissingSessions({ bindings: [bindingA, bindingB], hasPty }) + + expect(bindingA.reconcileIfSessionMissing).toHaveBeenCalledWith(hasPty, expect.any(Number)) + expect(bindingB.reconcileIfSessionMissing).toHaveBeenCalledWith(hasPty, expect.any(Number)) + expect(bindingA.reconcileIfSessionMissing.mock.calls[0]?.[1]).toBe( + bindingB.reconcileIfSessionMissing.mock.calls[0]?.[1] + ) + expect(hasPty).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-dead-session-reconcile.ts b/src/renderer/src/components/terminal-pane/terminal-dead-session-reconcile.ts index 8aa0f22685f..f77f78d7005 100644 --- a/src/renderer/src/components/terminal-pane/terminal-dead-session-reconcile.ts +++ b/src/renderer/src/components/terminal-pane/terminal-dead-session-reconcile.ts @@ -13,8 +13,11 @@ const REMOTE_PTY_ID_PREFIX = 'remote:' */ export type ReconcilableBinding = { reconcileIfSessionDead?: (liveSessionIds: Set, snapshotRequestedAt?: number) => void + reconcileIfSessionMissing?: (hasPty: HasPty, livenessRequestedAt?: number) => void } +export type HasPty = (ptyId: string) => Promise + /** * PURE decision: should the pane bound to `ptyId` be reconciled (torn down) * given the resolved set of live session ids? @@ -60,6 +63,37 @@ export function shouldReconcileDeadSession(args: { return !liveSessionIds.has(ptyId) } +export function shouldReconcileMissingSession(args: { + ptyId: string | null | undefined + connectionId: string | null | undefined + isLive: boolean | null | undefined + ptyBoundAt?: number | null + livenessRequestedAt?: number | null +}): boolean { + if (args.isLive !== false) { + return false + } + return shouldReconcileDeadSession({ + ptyId: args.ptyId, + connectionId: args.connectionId, + liveSessionIds: new Set(), + ptyBoundAt: args.ptyBoundAt, + snapshotRequestedAt: args.livenessRequestedAt + }) +} + +export function reconcileMissingSessions(args: { + bindings: Iterable + hasPty: HasPty +}): void { + // Why: request time predates every async response, so stale liveness cannot + // close a PTY that bound after the request started. + const requestedAt = performance.now() + for (const binding of args.bindings) { + binding.reconcileIfSessionMissing?.(args.hasPty, requestedAt) + } +} + /** * Thin orchestration: fetch the live session listing once and invoke each * binding's `reconcileIfSessionDead` with the resolved set. diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts index 8806cde6030..2eeea2fcdd0 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts @@ -325,7 +325,30 @@ describe('scheduleVisibilityReconcilePass', () => { expect(isTerminalPaneVisibilityResume({ previousIsVisible: false, isVisible: true })).toBe(true) }) - it('schedules a reconcile pass over the bindings when becoming visible', async () => { + it('schedules targeted liveness over the bindings when becoming visible', () => { + const reconcileIfSessionDead = vi.fn() + const reconcileIfSessionMissing = vi.fn() + const hasPty = vi.fn(async () => true) + const listSessions = vi + .fn<() => Promise<{ id: string; cwd: string; title: string }[]>>() + .mockResolvedValue([{ id: 'live-1', cwd: '/a', title: 'a' }]) + + const scheduled = scheduleVisibilityReconcilePass({ + previousIsVisible: false, + isVisible: true, + bindings: [{ reconcileIfSessionDead, reconcileIfSessionMissing }], + hasPty, + listSessions + }) + + expect(scheduled).toBe(true) + expect(reconcileIfSessionMissing).toHaveBeenCalledWith(hasPty, expect.any(Number)) + expect(reconcileIfSessionDead).not.toHaveBeenCalled() + expect(listSessions).not.toHaveBeenCalled() + expect(hasPty).not.toHaveBeenCalled() + }) + + it('keeps the broad listing fallback only when targeted liveness is unavailable', async () => { const reconcileIfSessionDead = vi.fn() const listSessions = vi .fn<() => Promise<{ id: string; cwd: string; title: string }[]>>() @@ -339,7 +362,6 @@ describe('scheduleVisibilityReconcilePass', () => { }) expect(scheduled).toBe(true) - // Fire-and-forget: let the async listSessions resolve before asserting. await Promise.resolve() await Promise.resolve() expect(listSessions).toHaveBeenCalledTimes(1) diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts index 9fce06be2aa..59760538741 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts @@ -76,7 +76,12 @@ import { installMouseHideWhileTyping } from './mouse-hide-while-typing' import type { EffectiveMacOptionAsAlt } from '@/lib/keyboard-layout/detect-option-as-alt' import { resolveEffectiveTerminalAppearance } from '@/lib/terminal-theme' import { connectPanePty } from './pty-connection' -import { reconcileDeadSessions, type ReconcilableBinding } from './terminal-dead-session-reconcile' +import { + reconcileDeadSessions, + reconcileMissingSessions, + type HasPty, + type ReconcilableBinding +} from './terminal-dead-session-reconcile' import type { PtyTransport } from './pty-transport' import { getRemoteRuntimePtyEnvironmentId } from '@/runtime/runtime-terminal-stream' import { getConnectionId } from '@/lib/connection-context' @@ -467,14 +472,21 @@ export function scheduleVisibilityReconcilePass(args: { previousIsVisible: boolean | null isVisible: boolean bindings: Iterable - listSessions: () => Promise<{ id: string; cwd: string; title: string }[]> + hasPty?: HasPty + listSessions?: () => Promise<{ id: string; cwd: string; title: string }[]> }): boolean { if (!isTerminalPaneVisibilityResume(args)) { return false } - // Why: fire-and-forget so the async listSessions IPC never blocks the - // synchronous WebGL/fit resume work the user sees first. - void reconcileDeadSessions({ bindings: args.bindings, listSessions: args.listSessions }) + if (args.hasPty) { + reconcileMissingSessions({ bindings: args.bindings, hasPty: args.hasPty }) + return true + } + if (args.listSessions) { + // Why: retained for older preload/test shims; production should use + // targeted hasPty so visibility resume never fans out provider listings. + void reconcileDeadSessions({ bindings: args.bindings, listSessions: args.listSessions }) + } return true } @@ -1695,7 +1707,7 @@ export function useTerminalPaneLifecycle({ previousIsVisible, isVisible, bindings: panePtyBindingsRef.current.values() as Iterable, - listSessions: () => window.api.pty.listSessions() + hasPty: window.api.pty.hasPty }) // eslint-disable-next-line react-hooks/exhaustive-deps -- Why: visibility and terminal identity changes must refresh existing PTY process tracking even though the ref object identity is stable. }, [cwd, isVisible, isVisibleRef, panePtyBindingsRef, tabId]) diff --git a/src/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index 664b37895dd..fa20a7344c6 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -2545,6 +2545,7 @@ function createPtyApi(): NonNullable['pty']> { getCwd: () => Promise.resolve('~'), getSize: () => Promise.resolve(null), listSessions: () => Promise.resolve([]), + hasPty: () => Promise.resolve(null), getMainBufferSnapshot: () => Promise.resolve(null), getRendererDeliveryDebugSnapshot: () => Promise.resolve({