diff --git a/mobile/src/host-screen/host-sidebar-feed-sections.test.ts b/mobile/src/host-screen/host-sidebar-feed-sections.test.ts index 46ab448ded0..a70c89a170c 100644 --- a/mobile/src/host-screen/host-sidebar-feed-sections.test.ts +++ b/mobile/src/host-screen/host-sidebar-feed-sections.test.ts @@ -13,6 +13,7 @@ import { FEED_ROWS, SERVED_SECTIONS, feedDomains, + feedKey, feedRow, liveRow, phoneView @@ -137,7 +138,7 @@ describe('the phone list from the sidebar feed', () => { const domains = feedDomains({ workspaces: new Map([ ...base.workspaces, - ['|orca::/w/solo', feedRow({ id: 'orca::/w/solo', manualOrder: 4 })] + [feedKey('|orca::/w/solo'), feedRow({ id: 'orca::/w/solo', manualOrder: 4 })] ]) }) const built = buildSidebarFeedRows(domains, grouped, phoneFeedView(phoneView())) @@ -209,7 +210,7 @@ describe('the phone list from the sidebar feed', () => { it('highlights nothing once the row is deleted', () => { const workspaces = new Map(feedDomains().workspaces) - workspaces.delete('|orca::/w/child') + workspaces.delete(feedKey('|orca::/w/child')) const domains = feedDomains({ workspaces }) const sections = sectionsFromDomains(domains, phoneView(), '', { worktreeId: 'orca::/w/child' @@ -324,8 +325,8 @@ describe('the phone list from the sidebar feed', () => { const built = buildSidebarFeedRows( feedDomains({ workspaces: new Map([ - ['|orca::/w/parent', FEED_ROWS.parent], - ['|orca::/w/child', feedChild] + [feedKey('|orca::/w/parent'), FEED_ROWS.parent], + [feedKey('|orca::/w/child'), feedChild] ]) }), catalog, diff --git a/mobile/src/host-screen/host-sidebar-feed.test-fixture.ts b/mobile/src/host-screen/host-sidebar-feed.test-fixture.ts index cab760cdf69..57edd6883c0 100644 --- a/mobile/src/host-screen/host-sidebar-feed.test-fixture.ts +++ b/mobile/src/host-screen/host-sidebar-feed.test-fixture.ts @@ -1,5 +1,9 @@ import type { SidebarFeedDomains } from '../../../src/shared/sidebar-list/sidebar-feed-domains' import type { WorkspaceRow } from '../../../src/shared/sidebar-list/workspace-rows' +import { + parseWorktreeHostIdentity, + type HostScopedWorktreeKey +} from '../../../src/shared/worktree/host-qualified-identity' import type { ServedProjectSections } from '../worktree/project-sections-operations' import { INITIAL_MOBILE_VIEW, type MobileViewState } from '../worktree/workspace-view-settings' import type { Worktree } from '../worktree/workspace-list-types' @@ -51,12 +55,21 @@ export const FEED_ROWS = { remote: feedRow({ id: 'lab::/w/gpu', hostId: 'ssh:gpu', repoId: 'lab', manualOrder: 3 }) } +/** A row identity written as its text, as the feed carries it. */ +export function feedKey(identity: string): HostScopedWorktreeKey { + const key = parseWorktreeHostIdentity(identity) + if (!key) { + throw new Error(`not a row identity: ${identity}`) + } + return key +} + export function feedDomains(overrides: Partial = {}): SidebarFeedDomains { return { workspaces: new Map([ - ['|orca::/w/parent', FEED_ROWS.parent], - ['|orca::/w/child', FEED_ROWS.child], - ['ssh:gpu|lab::/w/gpu', FEED_ROWS.remote] + [feedKey('|orca::/w/parent'), FEED_ROWS.parent], + [feedKey('|orca::/w/child'), FEED_ROWS.child], + [feedKey('ssh:gpu|lab::/w/gpu'), FEED_ROWS.remote] ]), activity: new Map(), review: new Map(), diff --git a/src/main/runtime/sidebar-inputs/orca-runtime-sidebar-temporal.ts b/src/main/runtime/sidebar-inputs/orca-runtime-sidebar-temporal.ts index e1e941cefab..396c50cf16e 100644 --- a/src/main/runtime/sidebar-inputs/orca-runtime-sidebar-temporal.ts +++ b/src/main/runtime/sidebar-inputs/orca-runtime-sidebar-temporal.ts @@ -242,7 +242,6 @@ export abstract class OrcaRuntimeWithSidebarTemporal extends OrcaRuntimeWithDete private readSidebarRetentionCandidates(): SidebarRetentionCandidates { const { facts } = this.workspaceActivityFacts.snapshot() return { - ids: Object.keys(facts), isActive: (worktreeId) => worktreeId in facts, isUnverifiableGap: (worktreeId) => !anySleepKeyNames(this.terminalSleepByHostWorktree.keys(), worktreeId) && diff --git a/src/main/runtime/sidebar-inputs/orca-runtime-workspace-review-facts.test.ts b/src/main/runtime/sidebar-inputs/orca-runtime-workspace-review-facts.test.ts index e6c3893122c..d0b0b6e156d 100644 --- a/src/main/runtime/sidebar-inputs/orca-runtime-workspace-review-facts.test.ts +++ b/src/main/runtime/sidebar-inputs/orca-runtime-workspace-review-facts.test.ts @@ -47,7 +47,7 @@ function worktree(id: string, branch: string): ResolvedWorktree { } describe('getWorkspaceReviewFacts', () => { - it('keys the persisted PR cache by workspace', async () => { + it('keys the persisted PR cache by workspace row', async () => { const merged: PRInfo = { number: 3, title: 'Ship it', @@ -73,7 +73,7 @@ describe('getWorkspaceReviewFacts', () => { // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the read touches only repos and the GitHub cache. const runtime = withSidebarFacts(new Probe(store as RuntimeStore)) await expect(runtime.getWorkspaceReviewFacts()).resolves.toEqual({ - facts: { 'repo-1::/r/a': { provider: 'github', reviewState: 'merged' } } + facts: { '|repo-1::/r/a': { provider: 'github', reviewState: 'merged' } } }) }) diff --git a/src/main/runtime/sidebar-inputs/sidebar-inputs-feed-announcements.test.ts b/src/main/runtime/sidebar-inputs/sidebar-inputs-feed-announcements.test.ts index 9c6a85195a2..31f1728f606 100644 --- a/src/main/runtime/sidebar-inputs/sidebar-inputs-feed-announcements.test.ts +++ b/src/main/runtime/sidebar-inputs/sidebar-inputs-feed-announcements.test.ts @@ -10,7 +10,10 @@ import type { WslHostService } from '../../wsl/wsl-host-service' import { setWslHostService } from '../../wsl/wsl-host-service-instance' import { getGitHubPRCacheKeyCandidates } from '../../../shared/github/github-cache-key' import { toRuntimeExecutionHostId } from '../../../shared/execution-host' -import { hostWorktreeTarget } from '../../../shared/worktree/host-qualified-identity' +import { + getWorktreeHostIdentity, + hostWorktreeTarget +} from '../../../shared/worktree/host-qualified-identity' import { prGroupOfReviewFact, WorkspaceReviewFactsSnapshotSchema, @@ -245,7 +248,7 @@ describe('sidebar inputs feed announcements', () => { } phone.notice(notice) await phone.settle() - const fact = phone.view().review?.[worktree.id] + const fact = phone.view().review?.[getWorktreeHostIdentity(worktree)] expect(fact).toEqual({ provider: 'github', reviewState: 'merged' }) expect(prGroupOfReviewFact(feedReviewFact(fact))).toBe('done') phone.disconnect() diff --git a/src/main/runtime/sidebar-inputs/sidebar-inputs-feed-domains.test.ts b/src/main/runtime/sidebar-inputs/sidebar-inputs-feed-domains.test.ts new file mode 100644 index 00000000000..e828611864f --- /dev/null +++ b/src/main/runtime/sidebar-inputs/sidebar-inputs-feed-domains.test.ts @@ -0,0 +1,135 @@ +import { describe, expect, it, vi } from 'vitest' +import { normalizePairedServerRow } from '../../../shared/sidebar-list/paired-server-rows' +import { + IDLE_WORKSPACE_ACTIVITY, + type WorkspaceActivityFacts +} from '../../../shared/sidebar-list/workspace-activity-facts' +import { buildWorkspaceRows } from '../../../shared/sidebar-list/workspace-rows' +import type { Repo } from '../../../shared/repo-types' +import type { Worktree } from '../../../shared/worktree/types' +import { ProducerRevision } from './producer-revision' +import { + sidebarInputsFeedDomains, + type SidebarInputsFeedSources +} from './sidebar-inputs-feed-domains' + +vi.mock('../../ssh/targets/ssh-target-registry', () => ({ + getRegisteredSshState: () => null, + listRegisteredSshTargets: () => [] +})) +vi.mock('../../wsl/wsl-host-service-instance', () => ({ getWslHostService: () => null })) + +const ID = 'r::/w/same' + +function worktree(hostId: Worktree['hostId']): Worktree { + return { + id: ID, + hostId, + repoId: 'r', + path: '/w/same', + head: '', + branch: 'refs/heads/same', + isBare: false, + isMainWorktree: false, + displayName: 'same', + comment: '', + linkedIssue: null, + linkedPR: null, + linkedLinearIssue: null, + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 0, + lastActivityAt: 0 + } +} + +function repo(overrides: Partial = {}): Repo { + return { id: 'r', path: '/r', displayName: 'r', badgeColor: '', addedAt: 0, ...overrides } +} + +/** This host lists `r::/w/same` locally and over SSH; a paired server lists the same id. */ +function harness(hostFacts: Record) { + const listing = { scanned: true } + const repos = [repo(), repo({ connectionId: 'box' })] + const serverRow = normalizePairedServerRow('runtime:env', { + worktreeId: ID, + repoId: 'r', + liveTerminalCount: 0, + status: 'working' + }) + const sources = { + facts: { + getWorkspaceActivityFacts: () => ({ revision: 0, facts: hostFacts }), + workspaceActivityFacts: { revision: new ProducerRevision() } + }, + pairedServerRows: { + revision: new ProducerRevision(), + snapshot: () => ({ + revision: 0, + servers: [ + { + hostId: 'runtime:env', + freshness: 'live' as const, + host: null, + rows: serverRow ? [serverRow] : [] + } + ] + }) + }, + workspaceRows: { + revision: new ProducerRevision(), + ready: async () => {}, + snapshot: () => + listing.scanned + ? { + revision: 0, + rows: Object.fromEntries( + buildWorkspaceRows({ + worktrees: [worktree('local'), worktree('ssh:box')], + folderWorkspaces: [], + repos, + lineageById: {} + }) + ) + } + : null + }, + store: () => null, + onClientEvent: () => () => {} + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the activity and workspaces domains read only the members built above. + const domains = sidebarInputsFeedDomains(sources as unknown as SidebarInputsFeedSources) + const read = (name: string) => { + const domain = domains.find((candidate) => candidate.name === name) + if (!domain) { + throw new Error(`no ${name} domain`) + } + return domain.read() + } + return { read, listing } +} + +describe('the activity domain', () => { + it('keys this host’s facts by its rows and a paired server’s by its own host', async () => { + const busy = { ...IDLE_WORKSPACE_ACTIVITY, browser: true } + const { read } = harness({ [ID]: busy }) + // The desktop's session names only the id, so both of its own copies carry it. + expect(await read('activity')).toEqual({ + [`local|${ID}`]: busy, + [`ssh:box|${ID}`]: busy, + [`runtime:env|${ID}`]: { ...IDLE_WORKSPACE_ACTIVITY, agent: 'working' } + }) + }) + + it('never lends a paired server’s activity to this host’s same-id rows', async () => { + const { read } = harness({}) + expect(Object.keys((await read('activity')) ?? {})).toEqual([`runtime:env|${ID}`]) + }) + + it('publishes nothing until this host’s rows are first scanned', async () => { + const { read, listing } = harness({}) + listing.scanned = false + expect(await read('activity')).toBeNull() + }) +}) diff --git a/src/main/runtime/sidebar-inputs/sidebar-inputs-feed-domains.ts b/src/main/runtime/sidebar-inputs/sidebar-inputs-feed-domains.ts index a3ae1633d21..92edad60af4 100644 --- a/src/main/runtime/sidebar-inputs/sidebar-inputs-feed-domains.ts +++ b/src/main/runtime/sidebar-inputs/sidebar-inputs-feed-domains.ts @@ -22,6 +22,7 @@ import { } from '../../../shared/sidebar-list/workspace-review-facts' import { buildPairedServerActivityFacts } from '../../../shared/sidebar-list/paired-server-activity-facts' import { parseExecutionHostId } from '../../../shared/execution-host' +import { keyWorkspaceActivityByRow } from '../../../shared/sidebar-list/workspace-activity-facts' /** This host's store and connection registries, read the way sidebarInputs.desktopFacts reads them. */ export function readRuntimeDesktopSidebarFacts(store: RuntimeStore): DesktopSidebarFacts { @@ -102,17 +103,25 @@ export function sidebarInputsFeedDomains( return [ { name: 'activity', - read: () => ({ - // A paired server's rows too; this host's own rows win an id both list. - ...buildPairedServerActivityFacts( - pairedServerRows.snapshot().servers, - (hostId) => sources.store()?.getWorkspaceSession?.(hostId) ?? null - ), - ...facts.getWorkspaceActivityFacts().facts - }), + read: () => { + const rows = workspaceRows.snapshot()?.rows + if (!rows) { + return null + } + return { + // This host's session tabs and agents are by bare id, so a same-id pair it lists share them. + ...keyWorkspaceActivityByRow(facts.getWorkspaceActivityFacts().facts, rows), + ...buildPairedServerActivityFacts( + pairedServerRows.snapshot().servers, + (hostId) => sources.store()?.getWorkspaceSession?.(hostId) ?? null + ) + } + }, + ready: () => workspaceRows.ready(), watch: (changed) => { const detaches = [ facts.workspaceActivityFacts.revision.onChange(changed), + workspaceRows.revision.onChange(changed), pairedServerRows.revision.onChange(changed), // This desktop's saved session for a paired server holds its browser and chat tabs there. sources.store()?.onWorkspaceSessionWritten?.((hostId) => { @@ -248,9 +257,9 @@ export function sidebarInputsFeedDomains( const store = requireStore() const prCache = store.getGitHubCache().pr return { - // A paired server's rows by the window's cache too; this host's own rows win an id both list. - ...buildPairedServerReviewFacts(pairedServerRows.snapshot().servers, prCache), - ...buildWorkspaceReviewFacts(worktrees, store.getRepos(), prCache) + ...buildWorkspaceReviewFacts(worktrees, store.getRepos(), prCache), + // A paired server's rows by the window's cache too. + ...buildPairedServerReviewFacts(pairedServerRows.snapshot().servers, prCache) } }, watch: (changed) => { diff --git a/src/main/runtime/sidebar-inputs/sidebar-retention-owner.test.ts b/src/main/runtime/sidebar-inputs/sidebar-retention-owner.test.ts index a0e413aa588..d6d51872289 100644 --- a/src/main/runtime/sidebar-inputs/sidebar-retention-owner.test.ts +++ b/src/main/runtime/sidebar-inputs/sidebar-retention-owner.test.ts @@ -56,7 +56,6 @@ function harness(worktrees: Worktree[]) { return rows.map(getWorktreeHostIdentity) }, retentionCandidates: () => ({ - ids: world.active, isActive: (id) => world.active.has(id), isUnverifiableGap: (id) => world.tabsOpen.has(id) }) @@ -141,7 +140,7 @@ describe('SidebarRetentionOwner', () => { expect(owner.snapshot().retention?.retainedIds).toEqual([getWorktreeHostIdentity(remote)]) }) - it('retains every host’s row of a same-id pair, as activity is by bare id', async () => { + it('retains every host’s row of a same-id pair, as this host’s session is by bare id', async () => { const local = worktree('a', { hostId: 'local' }) const remote = worktree('a', { hostId: 'ssh:box' }) const { world, owner } = harness([local, remote]) @@ -154,4 +153,21 @@ describe('SidebarRetentionOwner', () => { owner.noteActivityChange() expect(owner.snapshot().retention?.retainedIds).toEqual(['local|a', 'ssh:box|a']) }) + + it('gives each host’s row of a same-id pair its own grace window', async () => { + const local = worktree('a', { hostId: 'local' }) + const { world, owner } = harness([local]) + owner.beginSession('s1') + await flush() + world.active = new Set(['a']) + world.tabsOpen = new Set(['a']) + owner.noteActivityChange() + world.active = new Set() + owner.noteActivityChange() + // A twin listed only after the id went idle was never seen live, so it earns no grace. + world.worktrees = [local, worktree('a', { hostId: 'ssh:box' })] + owner.noteCatalogChange() + await flush() + expect(owner.snapshot().retention?.retainedIds).toEqual(['local|a']) + }) }) diff --git a/src/main/runtime/sidebar-inputs/sidebar-retention-owner.ts b/src/main/runtime/sidebar-inputs/sidebar-retention-owner.ts index 6fc1e30ade3..ea4c84aca9f 100644 --- a/src/main/runtime/sidebar-inputs/sidebar-retention-owner.ts +++ b/src/main/runtime/sidebar-inputs/sidebar-retention-owner.ts @@ -1,5 +1,8 @@ import { ProducerRevision } from './producer-revision' -import { getWorktreeIdFromHostIdentity } from '../../../shared/worktree/host-qualified-identity' +import { + getWorktreeIdFromHostIdentity, + type HostScopedWorktreeKey +} from '../../../shared/worktree/host-qualified-identity' import { createSleepingSweepRetentionState, updateSleepingSweepRetention @@ -10,10 +13,8 @@ import type { } from '../../../shared/sidebar-list/sidebar-temporal-snapshot' import type { SidebarClock } from './sidebar-clock' -/** The sweep's view of one workspace, by bare id. */ +/** This host's activity by bare id, read for each listed row: its session names no host. */ export type SidebarRetentionCandidates = { - /** Active ids; the last listing's rows are added to them. */ - ids: Iterable isActive: (worktreeId: string) => boolean /** False when the inactivity is definitive: a sleep this host started, or no terminal tab left. */ isUnverifiableGap: (worktreeId: string) => boolean @@ -22,7 +23,7 @@ export type SidebarRetentionCandidates = { export type SidebarRetentionInputs = { showSleepingWorkspaces: () => boolean /** The keys the workspaces domain lists this host's rows under; readers match retention on them. */ - listRowKeys: () => Promise + listRowKeys: () => Promise retentionCandidates: () => SidebarRetentionCandidates } @@ -38,8 +39,8 @@ export class SidebarRetentionOwner { private disposed = false // A listing started before a reset or a later one must not land. private listingSeq = 0 - // Both of a same-id pair on two hosts: the sweep and activity are by bare id. - private keysById = new Map() + // The last listing's rows; the sweep keeps a grace window per row, not per bare id. + private listedKeys: readonly HostScopedWorktreeKey[] = [] private retentionState = createSleepingSweepRetentionState() private retention: SidebarRetention | null = null // Until a listing lands no retained id can be host-qualified, so nothing is published. @@ -109,16 +110,12 @@ export class SidebarRetentionOwner { ) } - private applyListing(keys: readonly string[]): void { + private applyListing(keys: readonly HostScopedWorktreeKey[]): void { if (!this.listed) { this.listed = true this.revision.bump() } - this.keysById = new Map() - for (const key of keys) { - const id = getWorktreeIdFromHostIdentity(key) - this.keysById.set(id, [...(this.keysById.get(id) ?? []), key]) - } + this.listedKeys = keys this.retentionPass() } @@ -137,12 +134,12 @@ export class SidebarRetentionOwner { } const candidates = this.inputs.retentionCandidates() const now = Date.now() + // Every listed row is a candidate, so one that just went idle is not pruned out of its grace. const { retainedIds, nextExpiryInMs } = updateSleepingSweepRetention({ state: this.retentionState, - // The last listing's rows too, so an id that just went idle is not pruned out of its grace. - candidateWorktreeIds: [...new Set([...candidates.ids, ...this.keysById.keys()])], - isActive: candidates.isActive, - isUnverifiableGap: candidates.isUnverifiableGap, + candidateWorktreeIds: this.listedKeys, + isActive: (key) => candidates.isActive(getWorktreeIdFromHostIdentity(key)), + isUnverifiableGap: (key) => candidates.isUnverifiableGap(getWorktreeIdFromHostIdentity(key)), nowMs: now }) if (nextExpiryInMs === null) { @@ -150,8 +147,7 @@ export class SidebarRetentionOwner { } else { this.clock.schedule(RETENTION_DEADLINE, now + nextExpiryInMs, () => this.retentionPass()) } - // An id no listing has carried yet has no host to qualify it with, so it waits for one. - const ids = [...retainedIds].flatMap((id) => this.keysById.get(id) ?? []).sort() + const ids = [...retainedIds].sort() const previous = this.retention?.retainedIds if (!previous || previous.length !== ids.length || previous.some((id, i) => id !== ids[i])) { this.retention = { retainedIds: ids, retentionDecidedAt: now } diff --git a/src/renderer/src/components/sidebar/sidebar-list-parity.test-fixture.ts b/src/renderer/src/components/sidebar/sidebar-list-parity.test-fixture.ts index d0585cf8e98..15814d7d3cb 100644 --- a/src/renderer/src/components/sidebar/sidebar-list-parity.test-fixture.ts +++ b/src/renderer/src/components/sidebar/sidebar-list-parity.test-fixture.ts @@ -21,6 +21,7 @@ import type { } from '../../../../shared/sidebar-list/sidebar-feed-rows' import type { SmartStampFields } from '../../../../shared/sidebar-list/sort-stamps' import { buildPairedServerActivityFacts } from '../../../../shared/sidebar-list/paired-server-activity-facts' +import { keyWorkspaceActivityByRow } from '../../../../shared/sidebar-list/workspace-activity-facts' import { GOLDEN_NOW, folder, lineage } from './sidebar-list-golden-factories.test-fixture' import { GROUPED_REPOS, @@ -213,17 +214,16 @@ export function feedDomainsFromWindowState( read, watch: () => () => {} }) + const workspaceRows = Object.fromEntries( + buildWorkspaceRows({ + worktrees, + folderWorkspaces: state.folderWorkspaces, + repos: state.repos, + lineageById: state.worktreeLineageById + }) + ) return [ - domain('workspaces', () => - Object.fromEntries( - buildWorkspaceRows({ - worktrees, - folderWorkspaces: state.folderWorkspaces, - repos: state.repos, - lineageById: state.worktreeLineageById - }) - ) - ), + domain('workspaces', () => workspaceRows), domain('activity', () => ({ // The window's browser and chat tabs, as its saved session for the server holds them. ...buildPairedServerActivityFacts( @@ -237,7 +237,10 @@ export function feedDomainsFromWindowState( unifiedTabs: state.unifiedTabsByWorktree }) ), - ...Object.fromEntries(toLocallyOwnedRows(state, windowActivity)) + ...keyWorkspaceActivityByRow( + Object.fromEntries(toLocallyOwnedRows(state, windowActivity)), + workspaceRows + ) })), domain('review', () => ({ ...buildPairedServerReviewFacts( @@ -247,12 +250,7 @@ export function feedDomainsFromWindowState( })), state.prCache ), - ...Object.fromEntries( - toLocallyOwnedRows( - state, - new Map(Object.entries(buildWorkspaceReviewFacts(worktrees, state.repos, state.prCache))) - ) - ) + ...buildWorkspaceReviewFacts(worktrees, state.repos, state.prCache) })), domain('creations', () => Object.fromEntries( diff --git a/src/renderer/src/components/sidebar/worktree-list/grouping/host-labels.test.ts b/src/renderer/src/components/sidebar/worktree-list/grouping/host-labels.test.ts index f07e287e7cf..f7930497a30 100644 --- a/src/renderer/src/components/sidebar/worktree-list/grouping/host-labels.test.ts +++ b/src/renderer/src/components/sidebar/worktree-list/grouping/host-labels.test.ts @@ -25,19 +25,20 @@ function makeWorktrees(rows: { id: string; repoId: string }[]): Worktree[] { describe('host worktree counts and ids', () => { it('counts a repeated host identity once', () => { const repoMap = makeRepos({ 'repo-local': undefined }) + const repoOf = (worktree: Worktree) => repoMap.get(worktree.repoId) const worktrees = makeWorktrees([ { id: 'a', repoId: 'repo-local' }, { id: 'a', repoId: 'repo-local' }, { id: 'b', repoId: 'repo-local' } ]) - expect(getHostWorktreeCounts(worktrees, repoMap, LOCAL)?.get(LOCAL)).toBe(2) - expect(getHostWorktreeIds(worktrees, repoMap, LOCAL)?.get(LOCAL)).toEqual(['a', 'b']) + expect(getHostWorktreeCounts(worktrees, repoOf, LOCAL)?.get(LOCAL)).toBe(2) + expect(getHostWorktreeIds(worktrees, repoOf, LOCAL)?.get(LOCAL)).toEqual(['a', 'b']) }) it('returns undefined for an empty lane', () => { - const repoMap = makeRepos({}) - expect(getHostWorktreeCounts([], repoMap, LOCAL)).toBeUndefined() - expect(getHostWorktreeIds([], repoMap, LOCAL)).toBeUndefined() + const repoOf = () => undefined + expect(getHostWorktreeCounts([], repoOf, LOCAL)).toBeUndefined() + expect(getHostWorktreeIds([], repoOf, LOCAL)).toBeUndefined() }) }) diff --git a/src/renderer/src/components/sidebar/worktree-list/grouping/recent-project-rank-scan-budget.test.ts b/src/renderer/src/components/sidebar/worktree-list/grouping/recent-project-rank-scan-budget.test.ts index a80e45c52b6..bb794d5aaf8 100644 --- a/src/renderer/src/components/sidebar/worktree-list/grouping/recent-project-rank-scan-budget.test.ts +++ b/src/renderer/src/components/sidebar/worktree-list/grouping/recent-project-rank-scan-budget.test.ts @@ -71,6 +71,7 @@ describe('recent project rank scan budget', () => { collapsedGroups: new Set(grouped.map(([key]) => key)), workspaceStatuses: [], repoMap: new Map(), + repoOf: () => undefined, defaultHostId: 'local', hostLabelById: undefined, projectIndex: null, diff --git a/src/renderer/src/components/sidebar/worktree-list/listing/use-order-hold.test.ts b/src/renderer/src/components/sidebar/worktree-list/listing/use-order-hold.test.ts index 68022de7e7a..04480984983 100644 --- a/src/renderer/src/components/sidebar/worktree-list/listing/use-order-hold.test.ts +++ b/src/renderer/src/components/sidebar/worktree-list/listing/use-order-hold.test.ts @@ -108,7 +108,8 @@ describe('useProjectRecencyHeldWhileEngaged', () => { workspaces: [], worktrees: [makeWorktree('b', 'b', { lastActivityAt: recentAt })], folderWorkspaces: [], - repoMap: new Map() + repoMap: new Map(), + repoOf: () => undefined }) const recentOf = (index: SidebarFeedIndex): number[] => index.worktrees.map((worktree: Worktree) => worktree.lastActivityAt) diff --git a/src/renderer/src/lib/desktop-sidebar-feed.test.ts b/src/renderer/src/lib/desktop-sidebar-feed.test.ts index 789077b5214..dbdc367cd7a 100644 --- a/src/renderer/src/lib/desktop-sidebar-feed.test.ts +++ b/src/renderer/src/lib/desktop-sidebar-feed.test.ts @@ -22,7 +22,7 @@ function workspaceRow(id: string) { function connectMain() { const workspaces = testDomain('workspaces', workspaceRow('repo-1::/w/a')) const review = testDomain('review', { - 'repo-1::/w/a': { provider: 'github', reviewState: 'open' } + 'local|repo-1::/w/a': { provider: 'github', reviewState: 'open' } }) const notices = new Set<(event: SidebarInputsChangedEvent) => void>() const feed = new SidebarInputsFeed( diff --git a/src/renderer/src/lib/sidebar-pending-layer.test.ts b/src/renderer/src/lib/sidebar-pending-layer.test.ts index 6931dae1135..2048b5ca9f5 100644 --- a/src/renderer/src/lib/sidebar-pending-layer.test.ts +++ b/src/renderer/src/lib/sidebar-pending-layer.test.ts @@ -2,6 +2,8 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { SidebarFeedDomains } from '../../../shared/sidebar-list/sidebar-feed-domains' import { buildWorkspaceRows, type WorkspaceRow } from '../../../shared/sidebar-list/workspace-rows' import type { PairedServerWorkspaceRow } from '../../../shared/sidebar-list/paired-server-rows' +import { parseExecutionHostId } from '../../../shared/execution-host' +import { composeWorktreeHostIdentity } from '../../../shared/worktree/host-qualified-identity' import { makeFolderWorkspace, makeRepo, @@ -31,7 +33,12 @@ function domains( serverRows: PairedServerWorkspaceRow[] = [] ): SidebarFeedDomains { return { - workspaces: new Map(rows.map((row) => [`${row.hostId}|${row.id}`, row])), + workspaces: new Map( + rows.map((row) => [ + composeWorktreeHostIdentity(parseExecutionHostId(row.hostId)?.id, row.id), + row + ]) + ), activity: new Map(), review: new Map(), creations: new Map(), diff --git a/src/renderer/src/lib/sidebar-pending-layer.ts b/src/renderer/src/lib/sidebar-pending-layer.ts index b93093b4c80..a4c26b6bea2 100644 --- a/src/renderer/src/lib/sidebar-pending-layer.ts +++ b/src/renderer/src/lib/sidebar-pending-layer.ts @@ -277,10 +277,10 @@ function patch>>( return patched } -function withMissing( - rows: ReadonlyMap, - added: ReadonlyMap -): Map { +function withMissing( + rows: ReadonlyMap, + added: ReadonlyMap +): Map { const merged = new Map(rows) for (const [key, row] of added) { if (!merged.has(key)) { diff --git a/src/renderer/src/store/slices/repo-host-identity.ts b/src/renderer/src/store/slices/repo-host-identity.ts index 026fe0ccce2..24e6d321a16 100644 --- a/src/renderer/src/store/slices/repo-host-identity.ts +++ b/src/renderer/src/store/slices/repo-host-identity.ts @@ -7,18 +7,15 @@ import { type ExecutionHostId } from '../../../../shared/execution-host' import { + getRepoHostIdentity, getRepoHostIdentityForParts, type HostScopedRepoKey } from '../../../../shared/repo-host-identity' -export { getRepoHostIdentityForParts, type HostScopedRepoKey } +export { getRepoHostIdentity, getRepoHostIdentityForParts, type HostScopedRepoKey } type RepoIdentityParts = Pick -export function getRepoHostIdentity(repo: RepoIdentityParts): HostScopedRepoKey { - return getRepoHostIdentityForParts(repo.id, getRepoExecutionHostId(repo)) -} - /** One exact row: a paired server's twins share its `runtime:` host and differ by nested SSH target. */ export function isSameRepoRow(left: RepoIdentityParts, right: RepoIdentityParts): boolean { return ( diff --git a/src/shared/repo-host-identity.ts b/src/shared/repo-host-identity.ts index f835687a691..0f8dfad7097 100644 --- a/src/shared/repo-host-identity.ts +++ b/src/shared/repo-host-identity.ts @@ -16,6 +16,11 @@ export function getRepoHostIdentityForParts(repoId: string, hostId: string): Hos return `${hostId}\0${repoId}` as HostScopedRepoKey } +/** One host's registration of a repo; the same repo id may be registered on several hosts. */ +export function getRepoHostIdentity(repo: RepoHostTarget): HostScopedRepoKey { + return getRepoHostIdentityForParts(repo.id, getRepoExecutionHostId(repo)) +} + /** The hosts registering `repoId`; two or more means same-id twins on different hosts. */ export function getRepoRegistrationHostIds( repos: readonly RepoHostTarget[], diff --git a/src/shared/sidebar-list/group-sections.ts b/src/shared/sidebar-list/group-sections.ts index bb034a579fc..fcacda28c75 100644 --- a/src/shared/sidebar-list/group-sections.ts +++ b/src/shared/sidebar-list/group-sections.ts @@ -23,6 +23,7 @@ import type { ImportedWorktreesCardCandidate, NewExternalWorktreesInboxCandidate, PendingCreationRef, + RepoOfWorktree, Row, WorktreeGroupBy } from './row-types' @@ -34,6 +35,7 @@ export type SectionAppendContext = { collapsedGroups: Set workspaceStatuses: readonly WorkspaceStatusDefinition[] repoMap: Map + repoOf: RepoOfWorktree defaultHostId: ExecutionHostId hostLabelById: ReadonlyMap | undefined projectIndex: ProjectGroupingIndex | null @@ -56,6 +58,7 @@ export function appendOrderedGroups( collapsedGroups, workspaceStatuses, repoMap, + repoOf, defaultHostId, hostLabelById, projectIndex, @@ -96,13 +99,13 @@ export function appendOrderedGroups( hostWorktreeCounts: getLaneHostWorktreeCounts( group.items, folderPairs, - repoMap, + repoOf, defaultHostId ), hostWorktreeIds: getLaneHostWorktreeIds( group.items, folderPairs, - repoMap, + repoOf, defaultHostId ), worktreeIds: group.items.map((worktree) => worktree.id) @@ -118,13 +121,13 @@ export function appendOrderedGroups( hostWorktreeCounts: getLaneHostWorktreeCounts( group.items, folderPairs, - repoMap, + repoOf, defaultHostId ), hostWorktreeIds: getLaneHostWorktreeIds( group.items, folderPairs, - repoMap, + repoOf, defaultHostId ), worktreeIds: group.items.map((worktree) => worktree.id) @@ -178,7 +181,7 @@ export function appendOrderedGroups( groupBy === 'repo' ? getMixedHostContextLabels(group, repoMap, projectIndex, hostLabelById) : undefined - appendWorktreeRows(result, repoMap, occurrencePlan, { + appendWorktreeRows(result, repoOf, occurrencePlan, { collapsedGroups, groupDepth: projectGroupDepth, sectionKey: key, diff --git a/src/shared/sidebar-list/host-labels.ts b/src/shared/sidebar-list/host-labels.ts index 345c3d6fd4f..7a0896f5633 100644 --- a/src/shared/sidebar-list/host-labels.ts +++ b/src/shared/sidebar-list/host-labels.ts @@ -11,6 +11,7 @@ import { getProjectGroupingForRepo, type ProjectGroupingIndex } from '../project import type { WorktreeGroupEntry } from './worktree-group-entry' import { getFolderWorkspaceHostId } from './folder-workspace-host-id' import type { RenderableFolderWorkspace } from './folder-workspace-lanes' +import type { RepoOfWorktree } from './row-types' function getRepoHostId(repoId: string, repoMap: Map): ExecutionHostId | null { const repo = repoMap.get(repoId) @@ -127,13 +128,12 @@ export function getNoticeHostContextLabels( /** Keyed by host-qualified identity: two hosts sharing an id need two labels. */ export function getMixedWorktreeHostContextLabels( worktrees: readonly Worktree[], - repoMap: Map, + repoOf: RepoOfWorktree, hostLabelById: ReadonlyMap | undefined, defaultHostId: ExecutionHostId ): Map | undefined { return getSharedMixedHostContextLabels(worktrees, { - getHostId: (worktree) => - getWorktreeExecutionHostId(worktree, repoMap.get(worktree.repoId), defaultHostId), + getHostId: (worktree) => getWorktreeExecutionHostId(worktree, repoOf(worktree), defaultHostId), getIdentity: getWorktreeHostIdentity, sources: { hostLabelById } }) @@ -141,7 +141,7 @@ export function getMixedWorktreeHostContextLabels( export function getHostWorktreeCounts( worktrees: readonly Worktree[], - repoMap: Map, + repoOf: RepoOfWorktree, defaultHostId: ExecutionHostId ): Map | undefined { if (worktrees.length === 0) { @@ -151,7 +151,7 @@ export function getHostWorktreeCounts( // and a host's count is exactly the length of its id list by construction. // Dedup by host, not by bare id: the same id on two hosts is two workspaces // and has to be counted under each of them (STA-4343). - const idsByHost = getHostWorktreeIds(worktrees, repoMap, defaultHostId) + const idsByHost = getHostWorktreeIds(worktrees, repoOf, defaultHostId) if (!idsByHost) { return undefined } @@ -164,7 +164,7 @@ export function getHostWorktreeCounts( export function getHostWorktreeIds( worktrees: readonly Worktree[], - repoMap: Map, + repoOf: RepoOfWorktree, defaultHostId: ExecutionHostId ): Map | undefined { if (worktrees.length === 0) { @@ -179,7 +179,7 @@ export function getHostWorktreeIds( continue } seenIdentities.add(identity) - const hostId = getWorktreeExecutionHostId(worktree, repoMap.get(worktree.repoId), defaultHostId) + const hostId = getWorktreeExecutionHostId(worktree, repoOf(worktree), defaultHostId) const ids = idsByHost.get(hostId) ?? [] ids.push(worktree.id) idsByHost.set(hostId, ids) @@ -198,13 +198,13 @@ export function getHostWorktreeIds( export function getLaneHostWorktreeCounts( worktrees: readonly Worktree[], folderWorkspaces: readonly RenderableFolderWorkspace[], - repoMap: Map, + repoOf: RepoOfWorktree, defaultHostId: ExecutionHostId ): Map | undefined { if (worktrees.length === 0 && folderWorkspaces.length === 0) { return undefined } - const counts = getHostWorktreeCounts(worktrees, repoMap, defaultHostId) ?? new Map() + const counts = getHostWorktreeCounts(worktrees, repoOf, defaultHostId) ?? new Map() for (const { folderWorkspace, projectGroup } of folderWorkspaces) { const hostId = getFolderWorkspaceHostId(folderWorkspace, projectGroup, defaultHostId) counts.set(hostId, (counts.get(hostId) ?? 0) + 1) @@ -223,13 +223,13 @@ export function getLaneHostWorktreeCounts( export function getLaneHostWorktreeIds( worktrees: readonly Worktree[], folderWorkspaces: readonly RenderableFolderWorkspace[], - repoMap: Map, + repoOf: RepoOfWorktree, defaultHostId: ExecutionHostId ): Map | undefined { if (worktrees.length === 0 && folderWorkspaces.length === 0) { return undefined } - const idsByHost = getHostWorktreeIds(worktrees, repoMap, defaultHostId) ?? new Map() + const idsByHost = getHostWorktreeIds(worktrees, repoOf, defaultHostId) ?? new Map() for (const { folderWorkspace, projectGroup } of folderWorkspaces) { const hostId = getFolderWorkspaceHostId(folderWorkspace, projectGroup, defaultHostId) if (!idsByHost.has(hostId)) { diff --git a/src/shared/sidebar-list/paired-server-activity-facts.test.ts b/src/shared/sidebar-list/paired-server-activity-facts.test.ts index 34ef0287006..e20069e275e 100644 --- a/src/shared/sidebar-list/paired-server-activity-facts.test.ts +++ b/src/shared/sidebar-list/paired-server-activity-facts.test.ts @@ -3,6 +3,7 @@ import type { ExecutionHostId } from '../execution-host' import type { WorkspaceSessionState } from '../workspace-session-state-types' import { buildPairedServerActivityFacts } from './paired-server-activity-facts' import { normalizePairedServerRow, type PairedServerWorkspaceRow } from './paired-server-rows' +import { composeWorktreeHostIdentity } from '../worktree/host-qualified-identity' const HOST = 'runtime:env-1' @@ -50,10 +51,25 @@ describe('paired server activity facts', () => { serverRow('idle', { status: 'done' }) ]) expect(facts).toEqual({ - live: { terminal: 'live', browser: false, structuredChat: false, agent: null }, - lost: { terminal: 'unverifiable', browser: false, structuredChat: false, agent: null }, - agent: { terminal: null, browser: false, structuredChat: false, agent: 'monitoring' }, - asks: { terminal: 'live', browser: false, structuredChat: false, agent: 'permission' } + [`${HOST}|live`]: { terminal: 'live', browser: false, structuredChat: false, agent: null }, + [`${HOST}|lost`]: { + terminal: 'unverifiable', + browser: false, + structuredChat: false, + agent: null + }, + [`${HOST}|agent`]: { + terminal: null, + browser: false, + structuredChat: false, + agent: 'monitoring' + }, + [`${HOST}|asks`]: { + terminal: 'live', + browser: false, + structuredChat: false, + agent: 'permission' + } }) }) @@ -61,7 +77,7 @@ describe('paired server activity facts', () => { const { facts } = build([serverRow('live', { liveTerminalCount: 1, status: 'active' })], { freshness: 'unverifiable' }) - expect(facts.live?.terminal).toBe('unverifiable') + expect(facts[composeWorktreeHostIdentity(HOST, 'live')]?.terminal).toBe('unverifiable') }) it("adds the browser tabs this desktop's session holds in a server workspace", () => { @@ -87,7 +103,7 @@ describe('paired server activity facts', () => { } }) expect(asked).toEqual([HOST]) - expect(facts.browsing).toEqual({ + expect(facts[composeWorktreeHostIdentity(HOST, 'browsing')]).toEqual({ terminal: null, browser: true, structuredChat: false, @@ -95,6 +111,23 @@ describe('paired server activity facts', () => { }) }) + it('keeps each server’s copy of a same-id workspace under its own key', () => { + const other = normalizePairedServerRow('runtime:env-2', { + worktreeId: 'same', + repoId: 'repo', + liveTerminalCount: 0, + status: 'inactive' + }) + const facts = buildPairedServerActivityFacts( + [ + { hostId: HOST, freshness: 'live', rows: [serverRow('same', { liveTerminalCount: 1 })] }, + { hostId: 'runtime:env-2', freshness: 'live', rows: other ? [other] : [] } + ], + () => null + ) + expect(Object.keys(facts)).toEqual([`${HOST}|same`]) + }) + it('an older server that sends no counts reports nothing', () => { const row = normalizePairedServerRow(HOST, { worktreeId: 'old', repoId: 'repo' }) expect(row?.activity).toEqual({ terminal: null, agent: null }) diff --git a/src/shared/sidebar-list/paired-server-activity-facts.ts b/src/shared/sidebar-list/paired-server-activity-facts.ts index 251297ca043..ffd924eace6 100644 --- a/src/shared/sidebar-list/paired-server-activity-facts.ts +++ b/src/shared/sidebar-list/paired-server-activity-facts.ts @@ -5,12 +5,16 @@ import { } from '../execution-host' import type { WorkspaceSessionState } from '../workspace-session-state-types' import { getWorkspaceSessionTabActivity } from '../workspace-session-tab-activity' +import { + composeWorktreeHostIdentity, + type HostScopedWorktreeKey +} from '../worktree/host-qualified-identity' import { worktreeIdentityKey } from '../worktree/id' import type { PairedServerRowsSnapshot } from './paired-server-rows' import { isIdleWorkspaceActivity, type WorkspaceActivityFacts } from './workspace-activity-facts' /** - * A paired server's rows under the server's bare ids, like the review facts: its terminals and + * A paired server's rows under their host-qualified keys, like the review facts: its terminals and * agents as the server reports them, and the browser and chat tabs this desktop's session holds * there, which only the desktop knows. Rows from a server out of contact keep their terminals * unverifiable, never gone (docs/reference/ssh-execution-boundary.md). @@ -23,16 +27,15 @@ export function buildPairedServerActivityFacts( sessionOf: ( hostId: ExecutionHostId ) => Pick | null -): Record { - const facts: Record = {} +): Record { + const facts: Record = {} for (const server of servers) { const parsed = parseExecutionHostId(server.hostId) if (parsed?.kind !== 'runtime') { continue } - const tabs = getWorkspaceSessionTabActivity( - sessionOf(toRuntimeExecutionHostId(parsed.environmentId)) - ) + const hostId = toRuntimeExecutionHostId(parsed.environmentId) + const tabs = getWorkspaceSessionTabActivity(sessionOf(hostId)) const browser = new Set([...tabs.browser].map(worktreeIdentityKey)) const structuredChat = new Set([...tabs.structuredChat].map(worktreeIdentityKey)) for (const row of server.rows) { @@ -48,7 +51,7 @@ export function buildPairedServerActivityFacts( agent: reported?.agent ?? null } if (!isIdleWorkspaceActivity(fact)) { - facts[row.worktreeId] = fact + facts[composeWorktreeHostIdentity(hostId, row.worktreeId)] = fact } } } diff --git a/src/shared/sidebar-list/pinned-group-rows.ts b/src/shared/sidebar-list/pinned-group-rows.ts index b0fdc646c62..38c0af66198 100644 --- a/src/shared/sidebar-list/pinned-group-rows.ts +++ b/src/shared/sidebar-list/pinned-group-rows.ts @@ -1,4 +1,3 @@ -import type { Repo } from '../repo-types' import type { WorktreeOccurrencePlan } from '../worktree-occurrence-plan' import type { Worktree } from '../worktree/types' import { getWorktreeExecutionHostId } from '../execution-host' @@ -6,7 +5,7 @@ import type { ExecutionHostId } from '../execution-host' import { PINNED_GROUP_KEY } from './group-keys' import { appendWorktreeRows, buildImportedWorktreesCardRow } from './row-builders' import type { NoticeHostContext } from './host-labels' -import type { ImportedWorktreesCardCandidate, Row } from './row-types' +import type { ImportedWorktreesCardCandidate, RepoOfWorktree, Row } from './row-types' /** * The Pinned section's rows. @@ -16,7 +15,7 @@ import type { ImportedWorktreesCardCandidate, Row } from './row-types' */ export function emitPinnedGroup( pinnedSectionWorktrees: Worktree[], - repoMap: Map, + repoOf: RepoOfWorktree, defaultHostId: ExecutionHostId, collapsedGroups: Set, renderedNaturalAnchorRepoIds: ReadonlySet, @@ -35,7 +34,7 @@ export function emitPinnedGroup( const pinnedRepoOrder: string[] = [] const seenPinnedRepoIds = new Set() for (const worktree of pinnedSectionWorktrees) { - const hostId = getWorktreeExecutionHostId(worktree, repoMap.get(worktree.repoId), defaultHostId) + const hostId = getWorktreeExecutionHostId(worktree, repoOf(worktree), defaultHostId) hostWorktreeCounts.set(hostId, (hostWorktreeCounts.get(hostId) ?? 0) + 1) const hostIds = hostWorktreeIds.get(hostId) ?? [] hostIds.push(worktree.id) @@ -72,7 +71,7 @@ export function emitPinnedGroup( } const firstItemIndex = result.length - appendWorktreeRows(result, repoMap, occurrencePlan, { + appendWorktreeRows(result, repoOf, occurrencePlan, { collapsedGroups, groupDepth: 0, sectionKey: PINNED_GROUP_KEY, diff --git a/src/shared/sidebar-list/row-builders.ts b/src/shared/sidebar-list/row-builders.ts index a32bfc67843..f92a12af757 100644 --- a/src/shared/sidebar-list/row-builders.ts +++ b/src/shared/sidebar-list/row-builders.ts @@ -17,6 +17,7 @@ import type { NewExternalWorktreesInboxRow, PendingCreationRef, PendingCreationRow, + RepoOfWorktree, Row } from './row-types' @@ -66,7 +67,7 @@ export function buildNewExternalWorktreesInboxRow( export function appendWorktreeRows( result: Row[], - repoMap: Map, + repoOf: RepoOfWorktree, plan: WorktreeOccurrencePlan, options: { collapsedGroups: Set @@ -88,7 +89,7 @@ export function appendWorktreeRows( rowKey: occurrence.key, sectionKey: occurrence.sectionKey, worktree, - repo: repoMap.get(worktree.repoId), + repo: repoOf(worktree), depth: occurrence.depth, groupDepth: options.groupDepth, isLastLineageChild: occurrence.isLastChild, diff --git a/src/shared/sidebar-list/row-types.ts b/src/shared/sidebar-list/row-types.ts index 0c205f97bf3..9c58f04be2f 100644 --- a/src/shared/sidebar-list/row-types.ts +++ b/src/shared/sidebar-list/row-types.ts @@ -7,6 +7,8 @@ import type { ExecutionHostId } from '../execution-host' import type { PRGroupKey } from '../workspace-view/workspace-pr-groups' export type WorktreeGroupBy = 'none' | 'workspace-status' | 'repo' | 'pr-status' +/** A row's own registration of its repo; the same repo id can be registered on several hosts. */ +export type RepoOfWorktree = (worktree: Worktree) => Repo | undefined export type PinnedWorktreeDisplayPolicy = 'single-location' | 'duplicate-in-groups' export function getPinnedWorktreeDisplayPolicy( diff --git a/src/shared/sidebar-list/sidebar-feed-domains.ts b/src/shared/sidebar-list/sidebar-feed-domains.ts index a0f4eed5ca3..dee0010ab77 100644 --- a/src/shared/sidebar-list/sidebar-feed-domains.ts +++ b/src/shared/sidebar-list/sidebar-feed-domains.ts @@ -17,6 +17,10 @@ import { type WorkspaceReviewFact } from './workspace-review-facts' import { WorkspaceRowsSnapshotSchema, type WorkspaceRow } from './workspace-rows' +import { + parseWorktreeHostIdentity, + type HostScopedWorktreeKey +} from '../worktree/host-qualified-identity' import { ProjectSectionCatalogSchema, type ServedProjectSections @@ -24,9 +28,10 @@ import { /** One installed feed, typed per domain; an entity this build cannot parse is left out. */ export type SidebarFeedDomains = { - workspaces: ReadonlyMap - activity: ReadonlyMap - review: ReadonlyMap + /** Workspace facts are keyed by row identity: the same id on two hosts is two workspaces. */ + workspaces: ReadonlyMap + activity: ReadonlyMap + review: ReadonlyMap creations: ReadonlyMap pairedServerHosts: ReadonlyMap< string, @@ -39,7 +44,21 @@ export type SidebarFeedDomains = { desktop: DesktopSidebarFacts | null /** Every host's repos, project groups and projects, in header order; null from a host that predates it. */ projectCatalog: ServedProjectSections | null - retention: SidebarRetention | null + /** Retained rows by identity; an entry that is not one (a bare id) keeps nothing. */ + retention: SidebarFeedRetention | null +} + +export type SidebarFeedRetention = Omit & { + retainedIds: readonly HostScopedWorktreeKey[] +} + +function readRetention(retention: SidebarRetention | null): SidebarFeedRetention | null { + return retention + ? { + ...retention, + retainedIds: retention.retainedIds.flatMap((id) => parseWorktreeHostIdentity(id) ?? []) + } + : null } const entitySchemas = { @@ -55,21 +74,37 @@ const entitySchemas = { repoIcons: z.string() } -function readDomain( +function readKeyedDomain( installed: InstalledSidebarInputs, name: string, - schema: Schema -): Map> { - const entities = new Map>() - for (const [key, value] of installed.domains.get(name) ?? []) { + schema: Schema, + keyOf: (key: string) => Key | null +): Map> { + const entities = new Map>() + for (const [rawKey, value] of installed.domains.get(name) ?? []) { + const key = keyOf(rawKey) const parsed = schema.safeParse(value) - if (parsed.success) { + if (key !== null && parsed.success) { entities.set(key, parsed.data) } } return entities } +const readDomain = ( + installed: InstalledSidebarInputs, + name: string, + schema: Schema +): Map> => readKeyedDomain(installed, name, schema, (key) => key) + +// A key that is not a row identity (a bare id) is not one of this build's workspace entities. +const readWorkspaceDomain = ( + installed: InstalledSidebarInputs, + name: string, + schema: Schema +): Map> => + readKeyedDomain(installed, name, schema, parseWorktreeHostIdentity) + /** An install and what was read off it, for the next read to reuse. */ export type SidebarFeedRead = { installed: InstalledSidebarInputs; domains: SidebarFeedDomains } @@ -99,10 +134,12 @@ export function readSidebarFeedDomains( return { workspaces: unchanged('workspaces')?.workspaces ?? - readDomain(installed, 'workspaces', entitySchemas.workspaces), + readWorkspaceDomain(installed, 'workspaces', entitySchemas.workspaces), activity: - unchanged('activity')?.activity ?? readDomain(installed, 'activity', entitySchemas.activity), - review: unchanged('review')?.review ?? readDomain(installed, 'review', entitySchemas.review), + unchanged('activity')?.activity ?? + readWorkspaceDomain(installed, 'activity', entitySchemas.activity), + review: + unchanged('review')?.review ?? readWorkspaceDomain(installed, 'review', entitySchemas.review), creations: unchanged('creations')?.creations ?? readDomain(installed, 'creations', entitySchemas.creations), @@ -117,6 +154,6 @@ export function readSidebarFeedDomains( readDomain(installed, 'repoIcons', entitySchemas.repoIcons), desktop: desktop.success ? desktop.data : null, projectCatalog, - retention: temporal.success ? temporal.data.retention : null + retention: temporal.success ? readRetention(temporal.data.retention) : null } } diff --git a/src/shared/sidebar-list/sidebar-feed-index.ts b/src/shared/sidebar-list/sidebar-feed-index.ts index 3f51aa1a21a..53917b1ab1c 100644 --- a/src/shared/sidebar-list/sidebar-feed-index.ts +++ b/src/shared/sidebar-list/sidebar-feed-index.ts @@ -1,10 +1,16 @@ import { getSshTargetIdForExecutionHost, + LOCAL_EXECUTION_HOST_ID, parseExecutionHostId, type ExecutionHostId } from '../execution-host' import { buildExecutionHostRegistry } from '../execution-host-registry' import type { FolderWorkspace } from '../folder-workspace-types' +import { + getRepoHostIdentity, + getRepoHostIdentityForParts, + type HostScopedRepoKey +} from '../repo-host-identity' import type { Repo } from '../repo-types' import { getWorktreeHostIdentity, @@ -73,7 +79,27 @@ export type SidebarFeedIndex = { /** Every git workspace, archived ones included; folder rows are listed apart. */ worktrees: Worktree[] folderWorkspaces: FolderWorkspace[] + /** By repo id, for what is one per project (sections, labels); one host's registration wins. */ repoMap: Map + /** The registration a workspace belongs to; the same repo id may be registered on several hosts. */ + repoOf: (worktree: Pick) => Repo | undefined +} + +/** + * Its own host's registration when the catalog has one. A host that lists a same-id copy without + * its own registration still reads as that repo's, as the window reads it. + */ +function repoOfWorktree( + repoMap: ReadonlyMap, + reposByKey: ReadonlyMap, + worktree: Pick +): Repo | undefined { + // An unqualified row is a local one whose metadata names no host (workspace-rows.ts). + const hostId = worktree.hostId ?? LOCAL_EXECUTION_HOST_ID + return ( + reposByKey.get(getRepoHostIdentityForParts(worktree.repoId, hostId)) ?? + repoMap.get(worktree.repoId) + ) } export function indexSidebarFeed( @@ -81,11 +107,14 @@ export function indexSidebarFeed( repos: readonly Repo[] ): SidebarFeedIndex { const workspaces = readFeedWorkspaces(domains) + const repoMap = new Map(repos.map((repo) => [repo.id, repo])) + const reposByKey = new Map(repos.map((repo) => [getRepoHostIdentity(repo), repo])) return { workspaces, worktrees: workspaces.flatMap((entry) => (entry.folderWorkspace ? [] : [entry.worktree])), folderWorkspaces: workspaces.flatMap((entry) => entry.folderWorkspace ?? []), - repoMap: new Map(repos.map((repo) => [repo.id, repo])) + repoMap, + repoOf: (worktree) => repoOfWorktree(repoMap, reposByKey, worktree) } } diff --git a/src/shared/sidebar-list/sidebar-feed-rows.test.ts b/src/shared/sidebar-list/sidebar-feed-rows.test.ts index a26620b4ebb..304d9478d56 100644 --- a/src/shared/sidebar-list/sidebar-feed-rows.test.ts +++ b/src/shared/sidebar-list/sidebar-feed-rows.test.ts @@ -22,6 +22,7 @@ import { type SidebarFeedView } from './sidebar-feed-rows' import { IDLE_WORKSPACE_ACTIVITY } from './workspace-activity-facts' +import { composeWorktreeHostIdentity } from '../worktree/host-qualified-identity' import { buildWorkspaceRows } from './workspace-rows' function worktree(id: string, overrides: Partial = {}): Worktree { @@ -130,7 +131,12 @@ describe('buildSidebarFeedRows keeps same-id workspaces on two hosts apart', () const built = buildSidebarFeedRows( domains({ workspaces, - activity: new Map([['r::/w/other', { ...IDLE_WORKSPACE_ACTIVITY, browser: true }]]), + activity: new Map([ + [ + composeWorktreeHostIdentity('local', 'r::/w/other'), + { ...IDLE_WORKSPACE_ACTIVITY, browser: true } + ] + ]), retention: { retainedIds: [...workspaces.keys()].filter((key) => key.startsWith('ssh:')), retentionDecidedAt: 0 @@ -141,6 +147,78 @@ describe('buildSidebarFeedRows keeps same-id workspaces on two hosts apart', () ) expect(outline(built.sectionRows)).toEqual(['ssh:box|r::/w/same@0', 'local|r::/w/other@0']) }) + + const hideSleeping = view({ + filters: { ...NO_WORKSPACE_ROW_FILTERS, hideSleepingWorkspaces: true } + }) + const busy = { ...IDLE_WORKSPACE_ACTIVITY, browser: true } + + it('keeps one host’s activity off its same-id twin', () => { + const activity = new Map([[composeWorktreeHostIdentity('ssh:box', 'r::/w/same'), busy]]) + const built = buildSidebarFeedRows( + domains({ workspaces, activity }), + catalog(repos), + hideSleeping + ) + expect(outline(built.sectionRows)).toEqual(['ssh:box|r::/w/same@0']) + }) + + it('keeps one host’s retention off its same-id twin', () => { + const built = buildSidebarFeedRows( + domains({ + workspaces, + retention: { + retainedIds: [composeWorktreeHostIdentity('local', 'r::/w/same')], + retentionDecidedAt: 0 + } + }), + catalog(repos), + hideSleeping + ) + expect(outline(built.sectionRows)).toEqual(['local|r::/w/same@0']) + }) + + it('lanes each twin by its own PR', () => { + const review = new Map([ + [ + composeWorktreeHostIdentity('ssh:box', 'r::/w/same'), + { provider: 'github' as const, reviewState: 'merged' } + ] + ]) + const built = buildSidebarFeedRows( + domains({ workspaces, review }), + catalog(repos), + view({ groupBy: 'pr-status' }) + ) + const laneOf = (identity: string) => { + let lane = '' + for (const row of built.sectionRows) { + if (row.type === 'header') { + lane = row.key + } else if ( + row.type === 'item' && + `${row.worktree.hostId ?? ''}|${row.worktree.id}` === identity + ) { + return lane + } + } + return null + } + expect(laneOf('ssh:box|r::/w/same')).not.toEqual(laneOf('local|r::/w/same')) + expect(laneOf('local|r::/w/same')).toEqual(laneOf('local|r::/w/other')) + }) + + it('gives each twin’s row its own host’s registration of the repo', () => { + const built = buildSidebarFeedRows(domains({ workspaces }), catalog(repos), view()) + const repoOf = (hostId: string) => + built.sectionRows.find( + (row) => + row.type === 'item' && row.worktree.id === 'r::/w/same' && row.worktree.hostId === hostId + ) + expect(repoOf('ssh:box')).toMatchObject({ repo: { connectionId: 'box' } }) + expect(repoOf('local')).toMatchObject({ repo: { id: 'r' } }) + expect(repoOf('local')).not.toMatchObject({ repo: { connectionId: 'box' } }) + }) }) describe('buildSidebarFeedRows nests each paired server’s own lineage', () => { @@ -181,20 +259,20 @@ describe('buildSidebarFeedRows nests each paired server’s own lineage', () => ]) }) - it('brings back each server’s filtered-out parent above its own child', () => { - // Activity is per bare id, so both servers' children are awake and both parents asleep. - const awake = new Map([['r::/r/c', { ...IDLE_WORKSPACE_ACTIVITY, browser: true }]]) + it('brings back a server’s filtered-out parent above its own child, not its twin’s', () => { + // Only server A's child is awake: B's same-id rows stay asleep. + const awake = new Map([ + [ + composeWorktreeHostIdentity('runtime:a', 'r::/r/c'), + { ...IDLE_WORKSPACE_ACTIVITY, browser: true } + ] + ]) const built = buildSidebarFeedRows( feed(false, awake), catalog(repos), view({ filters: { ...NO_WORKSPACE_ROW_FILTERS, hideSleepingWorkspaces: true } }) ) - expect(outline(built.sectionRows)).toEqual([ - 'runtime:b|r::/r/p@0', - 'runtime:b|r::/r/c@1', - 'runtime:a|r::/r/p@0', - 'runtime:a|r::/r/c@1' - ]) + expect(outline(built.sectionRows)).toEqual(['runtime:a|r::/r/p@0', 'runtime:a|r::/r/c@1']) }) }) @@ -284,3 +362,24 @@ describe('the desktop’s hover hold over the shared builder', () => { ]) }) }) + +describe('buildSidebarFeedRows reads a row’s own registration of a repo two hosts register', () => { + it('files and counts an unqualified local row under this computer, not the server', () => { + // The server's registration of `r` comes last, so a lookup by repo id alone finds it. + const repos = [repo('r'), repo('r', { executionHostId: 'runtime:env' })] + const workspaces = buildWorkspaceRows({ + worktrees: [worktree('r::/w/a')], + folderWorkspaces: [], + repos: [repos[0]], + lineageById: {} + }) + const built = buildSidebarFeedRows( + domains({ workspaces }), + catalog(repos), + view({ visibleWorkspaceHostIds: ['local'] }) + ) + expect(outline(built.sectionRows)).toEqual(['|r::/w/a@0']) + const header = built.sectionRows.find((row) => row.type === 'header') + expect(header).toMatchObject({ hostWorktreeIds: new Map([['local', ['r::/w/a']]]) }) + }) +}) diff --git a/src/shared/sidebar-list/sidebar-feed-rows.ts b/src/shared/sidebar-list/sidebar-feed-rows.ts index a530d07548d..852bdf6d5da 100644 --- a/src/shared/sidebar-list/sidebar-feed-rows.ts +++ b/src/shared/sidebar-list/sidebar-feed-rows.ts @@ -143,7 +143,7 @@ export function planSidebarFeedRows( } // Each row carries its own edge (sidebar-feed-worktrees.ts); an id-keyed map would keep one host's. const lineageById = NO_LINEAGE_MAP - const retained = new Set(domains.retention?.retainedIds ?? []) + const retained = new Set(domains.retention?.retainedIds) const rankByIdentity = new Map(rankedIdentities.map((identity, rank) => [identity, rank])) const factsOf = (worktree: Worktree) => byIdentity.get(getWorktreeHostIdentity(worktree))?.facts const pairedDeviceIdsByEnvironment = new Map( @@ -165,7 +165,7 @@ export function planSidebarFeedRows( isFolderWorkspace: () => false, repoId: (worktree) => worktree.repoId, hostId: (worktree) => { - const repo = repoMap.get(worktree.repoId) + const repo = index.repoOf(worktree) return repo ? getWorktreeExecutionHostId(worktree, repo, defaultHostId) : null }, isOtherClient: (worktree) => @@ -176,11 +176,9 @@ export function planSidebarFeedRows( isDetachedHead: (worktree) => factsOf(worktree)?.detachedHead === true, isMainWorktree: (worktree) => worktree.isMainWorktree, isSleeping: (worktree) => { - const activity = domains.activity.get(worktree.id) - return ( - !retained.has(getWorktreeHostIdentity(worktree)) && - (!activity || isIdleWorkspaceActivity(activity)) - ) + const identity = getWorktreeHostIdentity(worktree) + const activity = domains.activity.get(identity) + return !retained.has(identity) && (!activity || isIdleWorkspaceActivity(activity)) } }, worktreeLineageById: lineageById, @@ -231,7 +229,9 @@ export function planSidebarFeedRows( groupBy, worktrees: visibleWorktrees, repoMap, - prGroupOf: (worktree) => prGroupOfReviewFact(domains.review.get(worktree.id)), + repoOfWorktree: index.repoOf, + prGroupOf: (worktree) => + prGroupOfReviewFact(domains.review.get(getWorktreeHostIdentity(worktree))), repoOrder, workspaceStatuses: view.workspaceStatuses, projectOrderBy: view.projectOrderBy, diff --git a/src/shared/sidebar-list/sidebar-inputs-feed-contract.ts b/src/shared/sidebar-list/sidebar-inputs-feed-contract.ts index 86ea9b0ae06..e3704f058b2 100644 --- a/src/shared/sidebar-list/sidebar-inputs-feed-contract.ts +++ b/src/shared/sidebar-list/sidebar-inputs-feed-contract.ts @@ -6,8 +6,9 @@ export const SIDEBAR_INPUTS_SCHEMA_VERSION = 1 /** * What the inputs mean: bumps whenever the same inputs would build a different list. * 2: desktop facts carry the focused host, host labels and pinned placement (#30462). + * 3: activity and review are keyed by row identity (host + id), not by bare id. */ -export const SIDEBAR_INPUTS_SEMANTICS_VERSION = 2 +export const SIDEBAR_INPUTS_SEMANTICS_VERSION = 3 /** A page's encoded ceiling. A phone's bridge frame (640 KiB) holds it with the RPC envelope. */ export const SIDEBAR_INPUTS_PAGE_MAX_BYTES = 256 * 1024 diff --git a/src/shared/sidebar-list/sidebar-row-plan.ts b/src/shared/sidebar-list/sidebar-row-plan.ts index 537161811a2..a6e48828d70 100644 --- a/src/shared/sidebar-list/sidebar-row-plan.ts +++ b/src/shared/sidebar-list/sidebar-row-plan.ts @@ -42,6 +42,7 @@ import type { NewExternalWorktreesInboxCandidate, PendingCreationRef, PinnedWorktreeDisplayPolicy, + RepoOfWorktree, Row, SidebarPRGroupReader, WorktreeGroupBy @@ -58,6 +59,8 @@ export type SidebarRowPlanInputs = { /** Visible worktrees in the frozen sort order. */ worktrees: Worktree[] repoMap: Map + /** Defaults to repoMap's registration for the row's repo id. */ + repoOfWorktree?: RepoOfWorktree prGroupOf: SidebarPRGroupReader repoOrder?: Map workspaceStatuses: readonly WorkspaceStatusDefinition[] @@ -84,6 +87,7 @@ export function planSidebarRows(inputs: SidebarRowPlanInputs) { groupBy, worktrees, repoMap, + repoOfWorktree, prGroupOf, repoOrder, workspaceStatuses, @@ -104,6 +108,7 @@ export function planSidebarRows(inputs: SidebarRowPlanInputs) { filterRepoIds } = inputs const projectIndex = buildProjectGroupingIndex(projectGrouping) + const repoOf = repoOfWorktree ?? ((worktree: Worktree) => repoMap.get(worktree.repoId)) // Membership is decided once, above the groupBy switch: every mode renders the // same set of folder workspaces and only chooses where they land (#15362). const renderableFolderWorkspaces = getRenderableFolderWorkspaces(folderWorkspaces, projectGroups) @@ -185,7 +190,7 @@ export function planSidebarRows(inputs: SidebarRowPlanInputs) { // mixed at all. Scoping to naturalWorktrees left pinned remotes unlabelled. const mixedWorktreeHostContextLabels = getMixedWorktreeHostContextLabels( worktrees, - repoMap, + repoOf, hostLabelById, defaultHostId ) @@ -211,7 +216,7 @@ export function planSidebarRows(inputs: SidebarRowPlanInputs) { }) emitPinnedGroup( pinnedSectionWorktrees, - repoMap, + repoOf, defaultHostId, collapsedGroups, renderedNaturalAnchorRepoIds, @@ -234,19 +239,19 @@ export function planSidebarRows(inputs: SidebarRowPlanInputs) { hostWorktreeCounts: getLaneHostWorktreeCounts( naturalWorktrees, renderableFolderWorkspaces, - repoMap, + repoOf, defaultHostId ), hostWorktreeIds: getLaneHostWorktreeIds( naturalWorktrees, renderableFolderWorkspaces, - repoMap, + repoOf, defaultHostId ), worktreeIds: naturalWorktrees.map((worktree) => worktree.id) }) if (!collapsedGroups.has(ALL_GROUP_KEY)) { - appendWorktreeRows(result, repoMap, occurrencePlan, { + appendWorktreeRows(result, repoOf, occurrencePlan, { collapsedGroups, groupDepth: 0, sectionKey: ALL_GROUP_KEY, @@ -268,6 +273,7 @@ export function planSidebarRows(inputs: SidebarRowPlanInputs) { collapsedGroups, workspaceStatuses, repoMap, + repoOf, defaultHostId, hostLabelById, projectIndex, diff --git a/src/shared/sidebar-list/workspace-activity-facts.ts b/src/shared/sidebar-list/workspace-activity-facts.ts index 0d47ccf5208..23c11d2c2a8 100644 --- a/src/shared/sidebar-list/workspace-activity-facts.ts +++ b/src/shared/sidebar-list/workspace-activity-facts.ts @@ -1,5 +1,9 @@ import { z } from 'zod' import type { AgentStatusEntry } from '../agent-status-types' +import { + parseWorktreeHostIdentity, + type HostScopedWorktreeKey +} from '../worktree/host-qualified-identity' const LiveAgentWorktreeStatusSchema = z.enum(['working', 'monitoring', 'permission']) @@ -38,6 +42,22 @@ export const IDLE_WORKSPACE_ACTIVITY: WorkspaceActivityFacts = { agent: null } +/** A host's bare-id activity under each key its rows list that id at; an unlisted id has no row. */ +export function keyWorkspaceActivityByRow( + facts: Readonly>, + rows: Readonly> +): Record { + const keyed: Record = {} + for (const [identity, row] of Object.entries(rows)) { + const key = parseWorktreeHostIdentity(identity) + const fact = facts[row.id] + if (key && fact) { + keyed[key] = fact + } + } + return keyed +} + export function isIdleWorkspaceActivity(facts: WorkspaceActivityFacts): boolean { return facts.terminal === null && !facts.browser && !facts.structuredChat && facts.agent === null } diff --git a/src/shared/sidebar-list/workspace-review-facts.test.ts b/src/shared/sidebar-list/workspace-review-facts.test.ts index 60c130753fc..da120b1b484 100644 --- a/src/shared/sidebar-list/workspace-review-facts.test.ts +++ b/src/shared/sidebar-list/workspace-review-facts.test.ts @@ -52,9 +52,20 @@ describe('prGroupOfReviewFact', () => { }) }) -it('lists only rows with a PR', () => { +it('lists only rows with a PR, under the key the workspaces domain lists them at', () => { const other = { ...row, id: 'repo-1::/r/b', branch: 'b' } expect(buildWorkspaceReviewFacts([row, other], [repo], { 'repo-1::a': pr('open') })).toEqual({ - [row.id]: { provider: 'github', reviewState: 'open' } + [`|${row.id}`]: { provider: 'github', reviewState: 'open' } + }) +}) + +it('reads each host’s copy of a same-id workspace with its own repo, under its own key', () => { + const sshRepo = { ...repo, connectionId: 'box' } + const local = { ...row, hostId: 'local' as const } + const remote = { ...row, hostId: 'ssh:box' as const } + const cache = { 'repo-1::a': pr('merged'), 'ssh:box::repo-1::a': pr('open') } + expect(buildWorkspaceReviewFacts([local, remote], [repo, sshRepo], cache)).toEqual({ + [`local|${row.id}`]: { provider: 'github', reviewState: 'merged' }, + [`ssh:box|${row.id}`]: { provider: 'github', reviewState: 'open' } }) }) diff --git a/src/shared/sidebar-list/workspace-review-facts.ts b/src/shared/sidebar-list/workspace-review-facts.ts index 09c6e2a9b75..34257687ad4 100644 --- a/src/shared/sidebar-list/workspace-review-facts.ts +++ b/src/shared/sidebar-list/workspace-review-facts.ts @@ -6,6 +6,11 @@ import { isGitHubPRSuppressed } from '../worktree/github-pr-suppression' import { getPRGroupKeyForState, type PRGroupKey } from '../workspace-view/workspace-pr-groups' import type { PairedServerWorkspaceRow } from './paired-server-rows' import { parseExecutionHostId } from '../execution-host' +import { + composeWorktreeHostIdentity, + type HostScopedWorktreeKey +} from '../worktree/host-qualified-identity' +import { listGitWorkspaceRows } from './workspace-rows' /** Grouping by PR reads GitHub PRs only, so only a row with a GitHub PR carries a fact. */ const WorkspaceReviewFactSchema = z.object({ @@ -13,7 +18,7 @@ const WorkspaceReviewFactSchema = z.object({ reviewState: z.string() }) -/** Read on demand, so it carries no revision. An absent id has no PR. */ +/** Read on demand, so it carries no revision. Keyed by row identity; an absent key has no PR. */ export const WorkspaceReviewFactsSnapshotSchema = z.object({ facts: z.record(z.string(), WorkspaceReviewFactSchema) }) @@ -58,20 +63,20 @@ export function workspaceReviewFactsEqual( return a?.provider === b?.provider && a?.reviewState === b?.reviewState } -/** Facts for every worktree with a PR; the window and main run this over their own copies. */ +/** Facts for every worktree this host lists with a PR, under its row key and read with its own repo. */ export function buildWorkspaceReviewFacts( - worktrees: Iterable< - Pick - >, - repos: Iterable>, + worktrees: readonly Pick< + Worktree, + 'id' | 'repoId' | 'hostId' | 'branch' | 'linkedPR' | 'suppressedGitHubPR' + >[], + repos: readonly Pick[], prCache: PRCacheView -): Record { - const repoById = new Map([...repos].map((repo) => [repo.id, repo])) - const facts: Record = {} - for (const worktree of worktrees) { - const fact = readWorkspaceReviewFact(worktree, repoById.get(worktree.repoId), prCache) +): Record { + const facts: Record = {} + for (const { identity, worktree, repo } of listGitWorkspaceRows(worktrees, repos)) { + const fact = readWorkspaceReviewFact(worktree, repo, prCache) if (fact) { - facts[worktree.id] = fact + facts[identity] = fact } } return facts @@ -81,8 +86,8 @@ export function buildWorkspaceReviewFacts( export function buildPairedServerReviewFacts( servers: readonly { hostId: string; rows: readonly PairedServerWorkspaceRow[] }[], prCache: PRCacheView -): Record { - const facts: Record = {} +): Record { + const facts: Record = {} for (const server of servers) { const hostId = parseExecutionHostId(server.hostId)?.id for (const row of hostId ? server.rows : []) { @@ -95,7 +100,7 @@ export function buildPairedServerReviewFacts( ) : null if (fact) { - facts[row.worktreeId] = fact + facts[composeWorktreeHostIdentity(hostId, row.worktreeId)] = fact } } } diff --git a/src/shared/sidebar-list/workspace-rows.test.ts b/src/shared/sidebar-list/workspace-rows.test.ts index 339d00d1e44..0265eb01d4d 100644 --- a/src/shared/sidebar-list/workspace-rows.test.ts +++ b/src/shared/sidebar-list/workspace-rows.test.ts @@ -3,11 +3,17 @@ import type { FolderWorkspace } from '../folder-workspace-types' import type { Repo } from '../repo-types' import type { Worktree } from '../worktree/types' import { - buildWorkspaceRows, + buildWorkspaceRows as buildKeyedWorkspaceRows, listGitWorkspaceRowKeys, - WorkspaceRowsSnapshotSchema + WorkspaceRowsSnapshotSchema, + type WorkspaceRow } from './workspace-rows' +/** The rows under their keys' text, so a test can name a key as a literal. */ +const buildWorkspaceRows = ( + catalog: Parameters[0] +): Map => new Map(buildKeyedWorkspaceRows(catalog)) + const localRepo: Repo = { id: 'repo-1', path: '/r', diff --git a/src/shared/sidebar-list/workspace-rows.ts b/src/shared/sidebar-list/workspace-rows.ts index 44ed3b24e6e..f17f313c585 100644 --- a/src/shared/sidebar-list/workspace-rows.ts +++ b/src/shared/sidebar-list/workspace-rows.ts @@ -11,7 +11,10 @@ import type { Repo } from '../repo-types' import { WorkspaceAttachmentsSchema } from '../workspace-attachment-schema' import { getWorkspaceAttachments } from '../workspace-attachments' import { DEFAULT_WORKSPACE_STATUS_ID } from '../workspace-statuses' -import { composeWorktreeHostIdentity } from '../worktree/host-qualified-identity' +import { + composeWorktreeHostIdentity, + type HostScopedWorktreeKey +} from '../worktree/host-qualified-identity' import type { WorktreeLineage } from '../worktree/lineage-types' import type { Worktree } from '../worktree/types' import { getWorkspaceRowKindFacts } from '../worktree/workspace-kinds' @@ -133,10 +136,12 @@ function toWorkspaceRow( } } -type ListedRow = { identity: string; hostId: ExecutionHostId } +type ListedRow = { identity: HostScopedWorktreeKey; hostId: ExecutionHostId } +type ListedWorktree = Pick +type ListedRepo = Pick /** Where this host lists a workspace, or null for one it does not (a paired server's). */ -function listedRow(worktree: Worktree, repo: Repo | undefined): ListedRow | null { +function listedRow(worktree: ListedWorktree, repo: ListedRepo | undefined): ListedRow | null { const hostId = worktree.hostId ?? (repo ? getRepoExecutionHostId(repo) : undefined) if (!hostId || parseExecutionHostId(hostId)?.kind === 'runtime') { return null @@ -148,11 +153,11 @@ function listedRow(worktree: Worktree, repo: Repo | undefined): ListedRow | null } /** Each git worktree with the repo it belongs to; the same repo id can be registered on two hosts. */ -function withOwnerRepos( - worktrees: readonly Worktree[], - repos: readonly Repo[] -): { worktree: Worktree; repo: Repo | undefined }[] { - const reposById = new Map() +function withOwnerRepos( + worktrees: readonly W[], + repos: readonly R[] +): { worktree: W; repo: R | undefined }[] { + const reposById = new Map() for (const repo of repos) { reposById.set(repo.id, [...(reposById.get(repo.id) ?? []), repo]) } @@ -165,14 +170,23 @@ function withOwnerRepos( }) } +/** Each git worktree this host lists, under the key `buildWorkspaceRows` lists it and with its own repo. */ +export function listGitWorkspaceRows( + worktrees: readonly W[], + repos: readonly R[] +): { identity: HostScopedWorktreeKey; worktree: W; repo: R | undefined }[] { + return withOwnerRepos(worktrees, repos).flatMap(({ worktree, repo }) => { + const listed = listedRow(worktree, repo) + return listed ? [{ identity: listed.identity, worktree, repo }] : [] + }) +} + /** The keys `buildWorkspaceRows` lists these git worktrees under. */ export function listGitWorkspaceRowKeys( worktrees: readonly Worktree[], repos: readonly Repo[] -): string[] { - return withOwnerRepos(worktrees, repos).flatMap( - ({ worktree, repo }) => listedRow(worktree, repo)?.identity ?? [] - ) +): HostScopedWorktreeKey[] { + return listGitWorkspaceRows(worktrees, repos).map(({ identity }) => identity) } /** @@ -184,8 +198,8 @@ export function buildWorkspaceRows(catalog: { folderWorkspaces: readonly FolderWorkspace[] repos: readonly Repo[] lineageById: Readonly> -}): Map { - const rows = new Map() +}): Map { + const rows = new Map() const add = ( worktree: Worktree, kind: WorkspaceRow['kind'], diff --git a/src/shared/worktree/host-qualified-identity.ts b/src/shared/worktree/host-qualified-identity.ts index c8393c8ce83..de1fc63dcc2 100644 --- a/src/shared/worktree/host-qualified-identity.ts +++ b/src/shared/worktree/host-qualified-identity.ts @@ -110,6 +110,16 @@ export function getWorktreeIdFromHostIdentity(identity: string): string { return identity.slice(identity.indexOf(HOST_SEPARATOR) + 1) } +/** A wire key back as a typed identity, or null when it is not one (a bare id). */ +export function parseWorktreeHostIdentity(identity: string): HostScopedWorktreeKey | null { + return isWorktreeHostIdentity(identity) + ? composeWorktreeHostIdentity( + getExecutionHostIdFromWorktreeHostIdentity(identity), + getWorktreeIdFromHostIdentity(identity) + ) + : null +} + /** True only for the canonical host-qualified form, not a legacy id containing `|`. */ export function isWorktreeHostIdentity(identity: string): boolean { const separator = identity.indexOf(HOST_SEPARATOR) diff --git a/tests/e2e/sidebar-review-facts.spec.ts b/tests/e2e/sidebar-review-facts.spec.ts index 2910e291095..4d9daa0f652 100644 --- a/tests/e2e/sidebar-review-facts.spec.ts +++ b/tests/e2e/sidebar-review-facts.spec.ts @@ -3,6 +3,7 @@ import { test, expect } from './helpers/orca-app' import { SIDEBAR_INPUTS_BUILD } from './helpers/sidebar-inputs-build' import { waitForActiveWorktree, waitForSessionReady } from './helpers/store' import { WorkspaceReviewFactsSnapshotSchema } from '../../src/shared/sidebar-list/workspace-review-facts' +import { getWorktreeIdFromHostIdentity } from '../../src/shared/worktree/host-qualified-identity' test.skip( !SIDEBAR_INPUTS_BUILD, @@ -45,7 +46,9 @@ async function readMainReviewFact(page: Page, worktreeId: string): Promise getWorktreeIdFromHostIdentity(key) === worktreeId)?.[1] ?? null } test('main follows the window PR cache for each workspace', async ({ orcaPage }) => {