Address host identity review feedback

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinwoo-H
2026-06-23 23:49:29 -07:00
co-authored by Orca
parent 8b37d052bd
commit 3a039f78bf
12 changed files with 173 additions and 22 deletions
+2 -1
View File
@@ -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,
+2 -1
View File
@@ -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,
+3 -1
View File
@@ -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)
@@ -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<Store, 'getRepos'> {
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()
@@ -74,6 +74,7 @@ export async function hydrateLocalPtyRegistryAtBoot(store: Pick<Store, 'getRepos
// sessions (`repoId::path`) hydrate into the new host-qualified bucket.
const repos = store.getRepos()
const canonicalWorktreeIdByAlias = new Map<string, string>()
const ambiguousAliases = new Set<string>()
for (const repo of repos) {
const connectionId = repo.connectionId ?? null
@@ -86,6 +87,15 @@ export async function hydrateLocalPtyRegistryAtBoot(store: Pick<Store, 'getRepos
for (const wt of worktrees) {
const canonicalWorktreeId = makeRepoWorktreeKey(repo, wt.path)
for (const alias of getRepoWorktreeIdAliases(repo, wt.path)) {
const existing = canonicalWorktreeIdByAlias.get(alias)
if (existing && existing !== canonicalWorktreeId) {
ambiguousAliases.add(alias)
canonicalWorktreeIdByAlias.delete(alias)
continue
}
if (ambiguousAliases.has(alias)) {
continue
}
canonicalWorktreeIdByAlias.set(alias, canonicalWorktreeId)
}
}
+2 -1
View File
@@ -5,6 +5,7 @@ import { homedir } from 'os'
import { isAbsolute, join, posix, win32 } from 'path'
import type { Repo } from '../../shared/types'
import { areWorktreePathsEqual } from '../ipc/worktree-logic'
import { getUsageRepoKey } from '../usage-worktree-metadata'
import Database from '../sqlite/sync-database'
import { columnExists, tableExists } from './schema-helpers'
import { canonicalizeUsageWorktreePaths } from '../usage-worktree-canonicalizer'
@@ -909,7 +910,7 @@ export function createWorktreeRefs(
): OpenCodeUsageWorktreeRef[] {
const refs: OpenCodeUsageWorktreeRef[] = []
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,
+77 -2
View File
@@ -1,6 +1,6 @@
import { describe, expect, it, vi } from 'vitest'
import { makeWorktreeKey } from '../shared/worktree-id'
import { loadKnownUsageWorktreesByRepo } from './usage-worktree-metadata'
import { getUsageRepoKey, loadKnownUsageWorktreesByRepo } from './usage-worktree-metadata'
describe('loadKnownUsageWorktreesByRepo', () => {
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'
}
])
})
})
+24 -7
View File
@@ -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<Repo, 'id' | 'connectionId' | 'executionHostId'>
): string {
return `${getRepoExecutionHostId(repo)}\0${repo.id}`
}
export function loadKnownUsageWorktreesByRepo(
store: Pick<Store, 'getAllWorktreeMeta'>,
repos: Repo[]
): Map<string, UsageWorktreeRef[]> {
const localRepos = repos.filter((repo) => !repo.connectionId)
const repoIds = new Set(localRepos.map((repo) => repo.id))
const worktreesByRepo = new Map<string, UsageWorktreeRef[]>()
const seenPathsByRepo = new Map<string, Set<string>>()
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)
@@ -25,6 +25,7 @@ type RepositoryHostSetupsSectionProps = {
}
function getProjectHostSetupKey(setup: Pick<ProjectHostSetup, 'hostId' | 'id'>): 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')}
+2 -1
View File
@@ -816,6 +816,7 @@ export function useIpcEvents(): void {
const handleWorktreesChanged = async (
repoId: string,
ownerHostId?: ExecutionHostId,
renamed?: { oldWorktreeId: string; newWorktreeId: string }
): Promise<void> => {
// 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,
@@ -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<AppState>)
store.setState({
worktreesByRepo: { repo1: [existing] },
detectedWorktreesByRepo: { repo1: previousDetected },
sortEpoch: 7
} as Partial<AppState>)
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)
})
+7 -3
View File
@@ -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<AppState, [], [], WorktreeSlice>
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) {