From 3a039f78bf00cae83f4f897ef398bf8397069d78 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Tue, 23 Jun 2026 18:13:50 -0700 Subject: [PATCH] Address host identity review feedback Co-authored-by: Orca --- src/main/claude-usage/scanner.ts | 3 +- src/main/codex-usage/scanner.ts | 3 +- src/main/ipc/worktrees.ts | 4 +- .../memory/hydrate-local-pty-registry.test.ts | 31 +++++++- src/main/memory/hydrate-local-pty-registry.ts | 10 +++ src/main/opencode-usage/scanner.ts | 3 +- src/main/usage-worktree-metadata.test.ts | 79 ++++++++++++++++++- src/main/usage-worktree-metadata.ts | 31 ++++++-- .../settings/RepositoryHostSetupsSection.tsx | 8 +- src/renderer/src/hooks/useIpcEvents.ts | 3 +- .../src/store/slices/worktrees.test.ts | 10 ++- src/renderer/src/store/slices/worktrees.ts | 10 ++- 12 files changed, 173 insertions(+), 22 deletions(-) diff --git a/src/main/claude-usage/scanner.ts b/src/main/claude-usage/scanner.ts index 9bfe007a49f..baedb8142b0 100644 --- a/src/main/claude-usage/scanner.ts +++ b/src/main/claude-usage/scanner.ts @@ -5,6 +5,7 @@ import { realpath, readdir, stat } from 'fs/promises' import { createReadStream } from 'fs' import { createInterface } from 'readline' import type { Repo } from '../../shared/types' +import { getUsageRepoKey } from '../usage-worktree-metadata' import type { ClaudeUsageAttributedTurn, ClaudeUsageDailyAggregate, @@ -671,7 +672,7 @@ export function createWorktreeRefs( ): ClaudeUsageWorktreeRef[] { const refs: ClaudeUsageWorktreeRef[] = [] for (const repo of repos) { - for (const worktree of worktreesByRepo.get(repo.id) ?? []) { + for (const worktree of worktreesByRepo.get(getUsageRepoKey(repo)) ?? []) { refs.push({ repoId: repo.id, worktreeId: worktree.worktreeId, diff --git a/src/main/codex-usage/scanner.ts b/src/main/codex-usage/scanner.ts index f23aafd6c14..06046d88c11 100644 --- a/src/main/codex-usage/scanner.ts +++ b/src/main/codex-usage/scanner.ts @@ -5,6 +5,7 @@ import { realpath, readdir, stat } from 'fs/promises' import { createInterface } from 'readline' import type { Repo } from '../../shared/types' import { areWorktreePathsEqual } from '../ipc/worktree-logic' +import { getUsageRepoKey } from '../usage-worktree-metadata' import { getOrcaManagedCodexHomePath, getSystemCodexHomePath } from '../codex/codex-home-paths' import { getLegacyCopiedCodexSessionBridgeScanPreference } from '../codex/codex-session-bridge' import { canonicalizeUsageWorktreePaths } from '../usage-worktree-canonicalizer' @@ -1070,7 +1071,7 @@ export function createWorktreeRefs( ): CodexUsageWorktreeRef[] { const refs: CodexUsageWorktreeRef[] = [] for (const repo of repos) { - for (const worktree of worktreesByRepo.get(repo.id) ?? []) { + for (const worktree of worktreesByRepo.get(getUsageRepoKey(repo)) ?? []) { refs.push({ repoId: repo.id, worktreeId: worktree.worktreeId, diff --git a/src/main/ipc/worktrees.ts b/src/main/ipc/worktrees.ts index 92a2f084df0..4285e4c11fa 100644 --- a/src/main/ipc/worktrees.ts +++ b/src/main/ipc/worktrees.ts @@ -657,7 +657,9 @@ function listDisconnectedSshWorktrees( const worktree = mergeWorktree( repo.id, synthesizeSshGitWorktree(repo, candidate.path, meta), - meta + meta, + repo.displayName, + { hostId: getRepoExecutionHostId(repo) } ) byWorktreeId.delete(worktree.id) byWorktreeId.set(worktree.id, worktree) diff --git a/src/main/memory/hydrate-local-pty-registry.test.ts b/src/main/memory/hydrate-local-pty-registry.test.ts index c751bb68efe..ab7dc83444f 100644 --- a/src/main/memory/hydrate-local-pty-registry.test.ts +++ b/src/main/memory/hydrate-local-pty-registry.test.ts @@ -48,7 +48,11 @@ vi.mock('../repo-worktrees', () => ({ // test schema-compliant without coupling to anything the hydrator doesn't // touch. function makeStore( - repos: { id: string; connectionId?: string | null }[] = [] + repos: { + id: string + connectionId?: string | null + executionHostId?: Repo['executionHostId'] + }[] = [] ): Pick { const built: Repo[] = repos.map((r) => ({ id: r.id, @@ -56,7 +60,8 @@ function makeStore( displayName: r.id, badgeColor: '#000000', addedAt: 0, - connectionId: r.connectionId ?? null + connectionId: r.connectionId ?? null, + executionHostId: r.executionHostId ?? null })) return { getRepos: () => built } } @@ -233,6 +238,28 @@ describe('hydrateLocalPtyRegistryAtBoot', () => { ) }) + it('skips legacy session ids that map to multiple host-qualified worktrees', async () => { + const { hydrate, listRegisteredPtys } = await loadFresh() + + const ptyId = 'repo-a::/shared/Triton@@cafebabe' + const provider = makeProvider([ + { sessionId: ptyId, pid: 4242, cwd: '/shared/Triton' } as unknown as SessionInfo + ]) + getDaemonProviderMock.mockReturnValue(provider) + listRepoWorktreesMock.mockResolvedValue([ + { path: '/shared/Triton', head: '', branch: '', isBare: false, isMainWorktree: true } + ]) + + await hydrate( + makeStore([ + { id: 'repo-a', connectionId: null }, + { id: 'repo-a', connectionId: null, executionHostId: 'runtime:server-1' } + ]) + ) + + expect(listRegisteredPtys()).toHaveLength(0) + }) + 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 d8a51a2689d..8c4f01c0d66 100644 --- a/src/main/memory/hydrate-local-pty-registry.ts +++ b/src/main/memory/hydrate-local-pty-registry.ts @@ -74,6 +74,7 @@ export async function hydrateLocalPtyRegistryAtBoot(store: Pick() + const ambiguousAliases = new Set() for (const repo of repos) { const connectionId = repo.connectionId ?? null @@ -86,6 +87,15 @@ export async function hydrateLocalPtyRegistryAtBoot(store: Pick { it('builds usage worktree refs from repo roots and persisted metadata', () => { @@ -34,7 +34,7 @@ describe('loadKnownUsageWorktreesByRepo', () => { expect(loadKnownUsageWorktreesByRepo(store as never, repos as never)).toEqual( new Map([ [ - 'repo-1', + getUsageRepoKey({ id: 'repo-1', connectionId: null, executionHostId: null }), [ { worktreeId: makeWorktreeKey({ @@ -56,4 +56,79 @@ describe('loadKnownUsageWorktreesByRepo', () => { ) expect(store.getAllWorktreeMeta).toHaveBeenCalledTimes(1) }) + + it('keeps same-id local and runtime repos in separate usage buckets', () => { + const store = { + getAllWorktreeMeta: vi.fn(() => ({ + [makeWorktreeKey({ + hostId: 'local', + repoId: 'repo-1', + path: '/workspace/local-feature' + })]: { + displayName: 'Local feature' + }, + [makeWorktreeKey({ + hostId: 'runtime:server-1', + repoId: 'repo-1', + path: '/srv/runtime-feature' + })]: { + displayName: 'Runtime feature' + } + })) + } + const localRepo = { + id: 'repo-1', + path: '/workspace/repo-a', + displayName: 'Local Repo' + } + const runtimeRepo = { + id: 'repo-1', + path: '/srv/repo-a', + displayName: 'Runtime Repo', + executionHostId: 'runtime:server-1' as const + } + + const result = loadKnownUsageWorktreesByRepo(store as never, [localRepo, runtimeRepo] as never) + + expect(result.get(getUsageRepoKey(localRepo))).toEqual([ + { + worktreeId: makeWorktreeKey({ + hostId: 'local', + repoId: 'repo-1', + path: '/workspace/repo-a' + }), + path: '/workspace/repo-a', + displayName: 'Local Repo' + }, + { + worktreeId: makeWorktreeKey({ + hostId: 'local', + repoId: 'repo-1', + path: '/workspace/local-feature' + }), + path: '/workspace/local-feature', + displayName: 'Local feature' + } + ]) + expect(result.get(getUsageRepoKey(runtimeRepo))).toEqual([ + { + worktreeId: makeWorktreeKey({ + hostId: 'runtime:server-1', + repoId: 'repo-1', + path: '/srv/repo-a' + }), + path: '/srv/repo-a', + displayName: 'Runtime Repo' + }, + { + worktreeId: makeWorktreeKey({ + hostId: 'runtime:server-1', + repoId: 'repo-1', + path: '/srv/runtime-feature' + }), + path: '/srv/runtime-feature', + displayName: 'Runtime feature' + } + ]) + }) }) diff --git a/src/main/usage-worktree-metadata.ts b/src/main/usage-worktree-metadata.ts index 9ad763725cb..d1806e224c4 100644 --- a/src/main/usage-worktree-metadata.ts +++ b/src/main/usage-worktree-metadata.ts @@ -7,6 +7,7 @@ import { } from '../shared/worktree-id' import { isFolderRepo } from '../shared/repo-kind' import type { Store } from './persistence' +import { getRepoExecutionHostId } from '../shared/execution-host' export type UsageWorktreeRef = { worktreeId: string @@ -18,44 +19,60 @@ function getDefaultUsageWorktreeLabel(pathValue: string): string { return basename(pathValue) } +export function getUsageRepoKey( + repo: Pick +): string { + return `${getRepoExecutionHostId(repo)}\0${repo.id}` +} + export function loadKnownUsageWorktreesByRepo( store: Pick, repos: Repo[] ): Map { const localRepos = repos.filter((repo) => !repo.connectionId) - const repoIds = new Set(localRepos.map((repo) => repo.id)) const worktreesByRepo = new Map() const seenPathsByRepo = new Map>() for (const repo of localRepos) { - worktreesByRepo.set(repo.id, [ + const repoKey = getUsageRepoKey(repo) + worktreesByRepo.set(repoKey, [ { worktreeId: makeRepoWorktreeKey(repo, repo.path), path: repo.path, displayName: repo.displayName || getDefaultUsageWorktreeLabel(repo.path) } ]) - seenPathsByRepo.set(repo.id, new Set([repo.path])) + seenPathsByRepo.set(repoKey, new Set([repo.path])) } // Why: usage scans are background/opt-in analytics. Do not spawn // `git worktree list` here; it can re-touch macOS protected folders. for (const [worktreeId, meta] of Object.entries(store.getAllWorktreeMeta())) { const parsed = splitWorktreeId(worktreeId) - if (!parsed || !repoIds.has(parsed.repoId)) { + if (!parsed) { continue } - const repo = localRepos.find((item) => item.id === parsed.repoId) + const matchingRepos = localRepos.filter((item) => { + if (item.id !== parsed.repoId) { + return false + } + return parsed.hostId === undefined || getRepoExecutionHostId(item) === parsed.hostId + }) + if (matchingRepos.length !== 1) { + continue + } + const [repo] = matchingRepos + const repoKey = getUsageRepoKey(repo) const worktreePath = repo && isFolderRepo(repo) ? (splitWorktreeIdForFilesystem(worktreeId)?.worktreePath ?? parsed.worktreePath) : parsed.worktreePath - const seenPaths = seenPathsByRepo.get(parsed.repoId) + const seenPaths = seenPathsByRepo.get(repoKey) if (seenPaths?.has(worktreePath)) { continue } seenPaths?.add(worktreePath) - worktreesByRepo.get(parsed.repoId)?.push({ + worktreesByRepo.get(repoKey)?.push({ worktreeId, path: worktreePath, displayName: meta.displayName || getDefaultUsageWorktreeLabel(worktreePath) diff --git a/src/renderer/src/components/settings/RepositoryHostSetupsSection.tsx b/src/renderer/src/components/settings/RepositoryHostSetupsSection.tsx index 9ccd66f56f5..d8393b4c432 100644 --- a/src/renderer/src/components/settings/RepositoryHostSetupsSection.tsx +++ b/src/renderer/src/components/settings/RepositoryHostSetupsSection.tsx @@ -25,6 +25,7 @@ type RepositoryHostSetupsSectionProps = { } function getProjectHostSetupKey(setup: Pick): string { + // Why: setup IDs can repeat across hosts after compatibility merges. return `${setup.hostId}\0${setup.id}` } @@ -219,8 +220,11 @@ export function RepositoryHostSetupsSection({ size="sm" onClick={async () => { setDeletingSetupKey(setupKey) - await deleteProjectHostSetup({ setupId: setup.id, hostId: setup.hostId }) - setDeletingSetupKey(null) + try { + await deleteProjectHostSetup({ setupId: setup.id, hostId: setup.hostId }) + } finally { + setDeletingSetupKey(null) + } }} > {translate('auto.components.settings.RepositoryPane.removeSetup', 'Remove')} diff --git a/src/renderer/src/hooks/useIpcEvents.ts b/src/renderer/src/hooks/useIpcEvents.ts index d19cea07d4e..beb32cf4c30 100644 --- a/src/renderer/src/hooks/useIpcEvents.ts +++ b/src/renderer/src/hooks/useIpcEvents.ts @@ -816,6 +816,7 @@ export function useIpcEvents(): void { const handleWorktreesChanged = async ( repoId: string, + ownerHostId?: ExecutionHostId, renamed?: { oldWorktreeId: string; newWorktreeId: string } ): Promise => { // Why: a folder rename changes the worktree's path-derived id. Re-key every @@ -842,7 +843,7 @@ export function useIpcEvents(): void { const before = getAuthoritativeDetectedWorktreeIds(state, repoId) ?? getVisibleWorktreeIdsForRepo(state, repoId) - await state.fetchWorktrees(repoId) + await state.fetchWorktrees(repoId, { ownerHostId }) await useAppStore.getState().fetchWorktreeLineage() // Why: changing the worktree's id unmounts the active pane without // re-rendering it under the new id. Now that the list has refreshed, diff --git a/src/renderer/src/store/slices/worktrees.test.ts b/src/renderer/src/store/slices/worktrees.test.ts index 89b8bb10850..ab4ff7f4d22 100644 --- a/src/renderer/src/store/slices/worktrees.test.ts +++ b/src/renderer/src/store/slices/worktrees.test.ts @@ -425,6 +425,7 @@ describe('fetchWorktrees', () => { it('keeps the last known worktree list when a refresh transiently returns empty', async () => { const store = createTestStore() const existing = makeWorktree({ id: 'repo1::/path/wt1', repoId: 'repo1', path: '/path/wt1' }) + const previousDetected = makeDetectedResult('repo1', [existing]) mockApi.worktrees.listDetected.mockResolvedValueOnce( makeDetectedResult('repo1', [], { @@ -432,11 +433,18 @@ describe('fetchWorktrees', () => { source: 'metadata-fallback' }) ) - store.setState({ worktreesByRepo: { repo1: [existing] }, sortEpoch: 7 } as Partial) + store.setState({ + worktreesByRepo: { repo1: [existing] }, + detectedWorktreesByRepo: { repo1: previousDetected }, + sortEpoch: 7 + } as Partial) const result = await store.getState().fetchWorktrees('repo1') expect(store.getState().worktreesByRepo.repo1).toEqual([existing]) + expect(store.getState().detectedWorktreesByRepo.repo1.worktrees).toEqual( + previousDetected.worktrees + ) expect(store.getState().sortEpoch).toBe(7) expect(result).toBe(false) }) diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index 0f6bdfd1bd4..c884a1fef25 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -16,8 +16,7 @@ import type { WorktreeLineage, WorkspaceLineage, WorkspaceKey, - WorktreeMeta, - Repo + WorktreeMeta } from '../../../../shared/types' import type { RuntimeWorktreeListResult } from '../../../../shared/runtime-types' import { @@ -450,6 +449,11 @@ function mergeDetectedWorktreesForHost( hostId: ExecutionHostId, options?: WorktreeHostMatchOptions ): DetectedWorktreeListResult { + if (!refreshed.authoritative && refreshed.worktrees.length === 0 && current) { + // Why: a non-authoritative empty scan means the backend could not prove the + // host is empty. Preserve cached rows instead of deleting display metadata. + return { ...refreshed, worktrees: current.worktrees } + } const refreshedForHost = refreshed.worktrees.map((worktree) => withRepoHostId(worktree, hostId)) return { ...refreshed, @@ -1770,7 +1774,7 @@ export const createWorktreeSlice: StateCreator fetchWorktrees: async (repoId, options) => { try { const ownerState = get() - const hostId = repoHostId(ownerState, repoId) + const hostId = repoHostId(ownerState, repoId, options?.ownerHostId) const settings = settingsForRepoOwner(ownerState, repoId, hostId) const detected = await listDetectedWorktreesForRepo(settings, repoId) if (options?.requireAuthoritative && !detected.authoritative) {