diff --git a/src/main/memory/hydrate-local-pty-registry.test.ts b/src/main/memory/hydrate-local-pty-registry.test.ts index 0854fa249..714f18b3c 100644 --- a/src/main/memory/hydrate-local-pty-registry.test.ts +++ b/src/main/memory/hydrate-local-pty-registry.test.ts @@ -32,9 +32,9 @@ vi.mock('../daemon/daemon-init', () => ({ getDaemonProvider: () => getDaemonProviderMock() })) -// Why: the hydrator builds its worktreeId → connectionId map by calling -// listRepoWorktrees(repo) for every repo in the store. The git I/O is -// out of scope for this unit; mock returns whatever the test wants. +// Why: the hydrator builds its worktreeId → connectionId map through +// listRepoWorktrees(repo). The git I/O is out of scope for this unit; the mock +// also lets count tests prove repos without live sessions launch no Git work. const listRepoWorktreesMock = vi.fn() vi.mock('../repo-worktrees', () => ({ listRepoWorktrees: (repo: unknown) => listRepoWorktreesMock(repo) @@ -79,6 +79,17 @@ function makeLocalSessions(repoId: string, worktreePath: string, count: number): return sessions } +function createDeferred(): { + promise: Promise + resolve: (value: T) => void +} { + let resolve!: (value: T) => void + const promise = new Promise((resolver) => { + resolve = resolver + }) + return { promise, resolve } +} + // Why: the module under test memoizes `hasHydrated` at module scope so it // only runs the git/RPC pass once per process. The pty-registry module // also stashes state in a module-level Map, so we have to load BOTH @@ -127,6 +138,7 @@ describe('hydrateLocalPtyRegistryAtBoot', () => { await hydrate(makeStore([{ id: 'repo-a' }])) expect(provider.listSessions).toHaveBeenCalledTimes(1) + expect(listRepoWorktreesMock).not.toHaveBeenCalled() }) it('catches provider.listSessions rejection and does not throw', async () => { @@ -145,6 +157,7 @@ describe('hydrateLocalPtyRegistryAtBoot', () => { warnSpy.mockRestore() expect(listRegisteredPtys()).toHaveLength(0) + expect(listRepoWorktreesMock).not.toHaveBeenCalled() }) it('does not clobber a pre-existing registry pid with a null pid from listSessions', async () => { @@ -180,6 +193,69 @@ describe('hydrateLocalPtyRegistryAtBoot', () => { expect(entry).toBeDefined() expect(entry!.pid).toBe(12345) expect(entry!.paneKey).toBe('tab-1:1') + expect(listRepoWorktreesMock).not.toHaveBeenCalled() + }) + + it('does not clobber a pty:spawn registration that arrives during worktree enumeration', async () => { + const { hydrate, listRegisteredPtys, registerPty } = await loadFresh() + const ptyId = 'repo-a::/local/Triton@@deadbeef' + const provider = makeProvider([ + { sessionId: ptyId, pid: null, cwd: '/local/Triton' } as unknown as SessionInfo + ]) + getDaemonProviderMock.mockReturnValue(provider) + const worktrees = + createDeferred< + { path: string; head: string; branch: string; isBare: boolean; isMainWorktree: boolean }[] + >() + listRepoWorktreesMock.mockReturnValue(worktrees.promise) + + const hydration = hydrate(makeStore([{ id: 'repo-a', connectionId: null }])) + await vi.waitFor(() => expect(listRepoWorktreesMock).toHaveBeenCalledTimes(1)) + registerPty({ + ptyId, + worktreeId: 'repo-a::/local/Triton', + sessionId: ptyId, + paneKey: 'tab-1:1', + pid: 12345 + }) + worktrees.resolve([ + { path: '/local/Triton', head: '', branch: '', isBare: false, isMainWorktree: true } + ]) + await hydration + + expect(listRegisteredPtys()).toEqual([ + expect.objectContaining({ ptyId, pid: 12345, paneKey: 'tab-1:1' }) + ]) + }) + + it('does not resurrect a daemon session that exits during worktree enumeration', async () => { + const { hydrate, listRegisteredPtys, unregisterPty } = await loadFresh() + const ptyId = 'repo-a::/local/Triton@@deadbeef' + const provider = { + listSessions: vi + .fn() + .mockResolvedValueOnce([ + { sessionId: ptyId, pid: 4242, cwd: '/local/Triton' } as unknown as SessionInfo + ]) + .mockResolvedValueOnce([]) + } + getDaemonProviderMock.mockReturnValue(provider) + const worktrees = + createDeferred< + { path: string; head: string; branch: string; isBare: boolean; isMainWorktree: boolean }[] + >() + listRepoWorktreesMock.mockReturnValue(worktrees.promise) + + const hydration = hydrate(makeStore([{ id: 'repo-a', connectionId: null }])) + await vi.waitFor(() => expect(listRepoWorktreesMock).toHaveBeenCalledTimes(1)) + unregisterPty(ptyId) + worktrees.resolve([ + { path: '/local/Triton', head: '', branch: '', isBare: false, isMainWorktree: true } + ]) + await hydration + + expect(provider.listSessions).toHaveBeenCalledTimes(2) + expect(listRegisteredPtys()).toHaveLength(0) }) it('skips SSH repos before enumerating worktrees', async () => { @@ -223,6 +299,99 @@ describe('hydrateLocalPtyRegistryAtBoot', () => { expect(entry!.worktreeId).toBe('repo-a::/local/Triton') }) + it('does not register a daemon session whose worktree was removed', async () => { + const { hydrate, listRegisteredPtys } = await loadFresh() + const provider = makeProvider([ + { + sessionId: 'repo-a::/local/removed@@deadbeef', + pid: 4242, + cwd: '/local/removed' + } as unknown as SessionInfo + ]) + getDaemonProviderMock.mockReturnValue(provider) + listRepoWorktreesMock.mockResolvedValue([ + { path: '/local/current', head: '', branch: '', isBare: false, isMainWorktree: true } + ]) + + await hydrate(makeStore([{ id: 'repo-a', connectionId: null }])) + + expect(listRepoWorktreesMock).toHaveBeenCalledTimes(1) + expect(listRegisteredPtys()).toHaveLength(0) + }) + + it('enumerates worktrees only for repos referenced by live daemon sessions', async () => { + const { hydrate, listRegisteredPtys } = await loadFresh() + const repos = Array.from({ length: 100 }, (_, index) => ({ id: `repo-${index}` })) + const activeRepoIds = ['repo-17', 'repo-83'] + const provider = makeProvider( + activeRepoIds.map( + (repoId, index) => + ({ + sessionId: `${repoId}::/local/${repoId}@@0000000${index}`, + pid: 4000 + index, + cwd: `/local/${repoId}` + }) as unknown as SessionInfo + ) + ) + getDaemonProviderMock.mockReturnValue(provider) + listRepoWorktreesMock.mockImplementation(async (repo: Repo) => [ + { + path: `/local/${repo.id}`, + head: '', + branch: '', + isBare: false, + isMainWorktree: true + } + ]) + + await hydrate(makeStore(repos)) + + expect(listRepoWorktreesMock).toHaveBeenCalledTimes(activeRepoIds.length) + expect(listRepoWorktreesMock.mock.calls.map(([repo]) => (repo as Repo).id)).toEqual( + activeRepoIds + ) + expect(listRegisteredPtys()).toHaveLength(activeRepoIds.length) + }) + + it('scans a repo that only becomes visible on the post-enumeration re-read', async () => { + const { hydrate, listRegisteredPtys } = await loadFresh() + + const sessionA = { + sessionId: 'repo-a::/local/repo-a@@00000001', + pid: 4001, + cwd: '/local/repo-a' + } as unknown as SessionInfo + const sessionB = { + sessionId: 'repo-b::/local/repo-b@@00000002', + pid: 4002, + cwd: '/local/repo-b' + } as unknown as SessionInfo + // Why: a briefly unreachable adapter can omit a session from the first + // listing; once it reappears on the re-read its repo must still be + // scanned and the session registered instead of silently dropped. + const listSessions = vi + .fn() + .mockResolvedValueOnce([sessionA]) + .mockResolvedValue([sessionA, sessionB]) + getDaemonProviderMock.mockReturnValue({ listSessions }) + listRepoWorktreesMock.mockImplementation(async (repo: Repo) => [ + { path: `/local/${repo.id}`, head: '', branch: '', isBare: false, isMainWorktree: true } + ]) + + await hydrate(makeStore([{ id: 'repo-a' }, { id: 'repo-b' }])) + + expect(listRepoWorktreesMock.mock.calls.map(([repo]) => (repo as Repo).id)).toEqual([ + 'repo-a', + 'repo-b' + ]) + expect(listSessions).toHaveBeenCalledTimes(3) + const registered = listRegisteredPtys() + expect(registered.map((p) => p.ptyId).sort()).toEqual([ + 'repo-a::/local/repo-a@@00000001', + 'repo-b::/local/repo-b@@00000002' + ]) + }) + it('hydrates large daemon session lists', async () => { const { hydrate, listRegisteredPtys } = await loadFresh() diff --git a/src/main/memory/hydrate-local-pty-registry.ts b/src/main/memory/hydrate-local-pty-registry.ts index 4bb82763e..f65630493 100644 --- a/src/main/memory/hydrate-local-pty-registry.ts +++ b/src/main/memory/hydrate-local-pty-registry.ts @@ -25,6 +25,7 @@ import type { SessionInfo } from '../daemon/types' import { listRegisteredPtys, registerPty } from './pty-registry' import { listRepoWorktrees } from '../repo-worktrees' import { parsePtySessionId } from '../../shared/pty-session-id-format' +import { splitWorktreeId } from '../../shared/worktree-id' import type { Store } from '../persistence' // Why: `attachMainWindowServices` runs on every macOS dock re-activation @@ -70,34 +71,62 @@ export async function hydrateLocalPtyRegistryAtBoot(store: Pick() + // Why: ask the daemon which repos matter before launching Git worktree + // enumeration. Most configured repos have no preserved session at boot, + // so scanning all of them creates pure background subprocess churn. + const reposById = new Map(store.getRepos().map((repo) => [repo.id, repo])) + // Why: live git enumeration verifies that a referenced local worktree + // still exists instead of resurrecting removed worktrees. + const liveLocalWorktreeIds = new Set() + const resolvedRepoIds = new Set() - for (const repo of repos) { - const connectionId = repo.connectionId ?? null - if (connectionId) { - // Why: SSH PTYs are never registered for local process sampling, so - // avoid startup SSH/git enumeration for repos we will skip anyway. - continue + let sessionInfos = await collectSessionInfos(provider) + let alreadyRegistered = new Set(listRegisteredPtys().map((p) => p.ptyId)) + + // Why: repo selection and registration must come from the same daemon + // snapshot. Git enumeration can take seconds, so after each scan pass we + // re-read daemon and registry state; sessions that exited or were + // authoritatively registered meanwhile are not resurrected or overwritten, + // and a session that only became visible during a slow scan (e.g. a + // briefly unreachable legacy adapter) gets its repo scanned on the next + // pass instead of being silently dropped. Terminates because every pass + // permanently resolves at least one new repo id. + for (;;) { + const newlyReferencedRepos = new Map[number]>() + for (const info of sessionInfos) { + if (alreadyRegistered.has(info.sessionId)) { + continue + } + const { worktreeId } = parsePtySessionId(info.sessionId) + const parsedWorktreeId = worktreeId ? splitWorktreeId(worktreeId) : null + if (!parsedWorktreeId || resolvedRepoIds.has(parsedWorktreeId.repoId)) { + continue + } + const repo = reposById.get(parsedWorktreeId.repoId) + if (!repo || (repo.connectionId ?? null)) { + // Why: unknown repos can't be proven local, and SSH PTYs are never + // registered for local process sampling — resolve without git + // enumeration so neither can extend the loop. + resolvedRepoIds.add(parsedWorktreeId.repoId) + continue + } + newlyReferencedRepos.set(repo.id, repo) } - const worktrees = await listRepoWorktrees(repo) - for (const wt of worktrees) { - const worktreeId = `${repo.id}::${wt.path}` - repoConnectionIdByWorktreeId.set(worktreeId, connectionId) + if (newlyReferencedRepos.size === 0) { + break } + + for (const repo of newlyReferencedRepos.values()) { + resolvedRepoIds.add(repo.id) + const worktrees = await listRepoWorktrees(repo) + for (const wt of worktrees) { + liveLocalWorktreeIds.add(`${repo.id}::${wt.path}`) + } + } + + sessionInfos = await collectSessionInfos(provider) + alreadyRegistered = new Set(listRegisteredPtys().map((p) => p.ptyId)) } - - // Why: SessionInfo is read through the adapter's listSessions() so we - // get the pid alongside each id. Routing through every adapter - // (current + legacy) keeps protocol coverage symmetric with the - // orphan-cleanup path. - const sessionInfos = await collectSessionInfos(provider) - - const alreadyRegistered = new Set(listRegisteredPtys().map((p) => p.ptyId)) - for (const info of sessionInfos) { // Why: pid-write ordering — `pty:spawn` is the authoritative // writer for in-session sessions; if that fired before this loop @@ -115,10 +144,7 @@ export async function hydrateLocalPtyRegistryAtBoot(store: Pick