From bb8a6b5ba2e06dfc244f7f469aaed5bea7d83b76 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 13 Nov 2026 08:03:59 -0800 Subject: [PATCH] fix(cli): agent show labels a place from its own host's catalog row or keyed records A shown place carries the host its status row's connection names, and its workspace and repo names come from the published catalog row for that host and id (indexed once per publication) or, cold, from the repo registered on that host (a store-owned index rebuilt only when the repo collection changes) and that host's metadata. A same-id workspace on another host no longer lends its name, and no show scans every workspace or repo. Adds the refused-clear and rewind read fixtures, and words the WSL saved-location refusal by what it proves. --- .../agent-read-journal-continuation.test.ts | 77 ++++- .../project-collection-operations.ts | 69 ++-- .../tracking-repos/project-host-operations.ts | 31 +- .../agent-conversation-list.ts | 15 +- .../agent-conversation-place-labels.test.ts | 318 ++++++++++++++++++ .../agent-conversation-read.test.ts | 8 +- .../agent-conversation-read.ts | 5 +- .../construction/runtime-store-contract.ts | 1 + .../catalog/resolved-worktree-row.ts | 60 ++++ .../runtime-resolved-worktree-cache.ts | 38 ++- .../session-hosts/get-runtime-id.ts | 40 +-- .../workspace-display-name.test.ts | 47 --- .../runtime/workspaces/workspace-selectors.ts | 41 +-- 13 files changed, 615 insertions(+), 135 deletions(-) create mode 100644 src/main/runtime/agent-conversations/agent-conversation-place-labels.test.ts create mode 100644 src/main/runtime/workspaces/catalog/resolved-worktree-row.ts delete mode 100644 src/main/runtime/workspaces/session-hosts/workspace-display-name.test.ts diff --git a/src/main/native-chat/agent-session-wire/publication/history/testing/agent-read-journal-continuation.test.ts b/src/main/native-chat/agent-session-wire/publication/history/testing/agent-read-journal-continuation.test.ts index 78e79f67da4..434db5ee02f 100644 --- a/src/main/native-chat/agent-session-wire/publication/history/testing/agent-read-journal-continuation.test.ts +++ b/src/main/native-chat/agent-session-wire/publication/history/testing/agent-read-journal-continuation.test.ts @@ -3,6 +3,8 @@ import { afterEach, expect, it, vi } from 'vitest' import { AGENT_JOURNAL_THREAD_SCOPE } from '../../../../../../shared/agent-session/journal/agent-session-journal-types' import { agentJournalItemKey } from '../../../../../../shared/agent-session/journal/agent-session-journal-item-key' import type { AgentSessionJournal } from '../../../../agent-session-journal/journal-store' +import { rewindRefusalBehindClear } from '../../../rewind/structured-rewind-refusal' +import { agentSessionJournalProviderHandle } from '../../../../../../shared/agent-session/lifecycle/agent-session-provider-handle-encoding' import { isMobileE2EETextPayloadWithinLimit } from '../../../../../runtime/rpc/mobile/e2ee/mobile-e2ee-outbound-admission' import { readAgentJournalPage, @@ -390,11 +392,11 @@ it('resumes a row read in part after the journal reopens, and refuses a cursor c expect(resume(first.next)).toEqual({ ok: false }) }) -const clear = (journal: AgentSessionJournal, operationId: string) => - journal.context.clear( - { operationId, afterFence: 1, clearedAt: 1_000 }, - { write: () => {}, committed: () => {} } - ) +const clear = ( + journal: AgentSessionJournal, + operationId: string, + receipt = { write: () => {}, committed: () => {} } +) => journal.context.clear({ operationId, afterFence: 1, clearedAt: 1_000 }, receipt) const CLEARED = { kind: 'message', role: 'system', text: 'Context cleared' } as const /** APPLE and BANANA, then `/clear`, then CHERRY, with cursors cut before the clear. */ @@ -587,3 +589,68 @@ it.fails.each([ limited: false }) }) + +it('leaves the read untouched when a clear commits no divider', async () => { + const { journal } = await clearedChat() + await write(journal, 'cherry', [['cherry', 'CHERRY']]) + const before = tail(journal, 100) + const head = journal.cursor() + + // The receipt's write failing rolls the clear back: no divider is committed. + await expect( + clear(journal, 'refused', { + write: () => { + throw new Error('refused-clear') + }, + committed: () => {} + }) + ).rejects.toThrow('refused-clear') + + expect(journal.cursor()).toEqual(head) + expect(tail(journal, 100)).toEqual(before) + expect(readAgentJournalPage(journal, { limit: 5, after: before.next }, waiting)).toMatchObject({ + ok: true, + entries: [] + }) +}) + +it.each(['plain', 'forked'] as const)( + 'rewinds within the current context (%s), keeping its divider, and refuses a target before it', + async (kind) => { + const { journal } = await clearedChat() + await write(journal, 'cherry', [['cherry', 'CHERRY']]) + const before = tail(journal, 100) + const floor = before.next.floor ?? 0 + const work = resetWork(journal) + + // A rewind to an item before the divider is refused before any agent runs. + const epoch = journal.cursor().epoch + expect( + rewindRefusalBehindClear(journal, { itemId: 'banana', expectedEpoch: epoch }) + ).not.toBeNull() + expect(rewindRefusalBehindClear(journal, { itemId: 'cherry', expectedEpoch: epoch })).toBeNull() + expect(tail(journal, 100)).toEqual(before) + + await journal.context.rewind( + { epoch, sequence: floor }, + 1, + [], + () => ({ write: () => {}, committed: () => {} }), + // A provider that forked its history starts a new epoch, as structured rewind recovery does. + kind === 'forked' + ? { reason: 'handle_forked', providerHandle: agentSessionJournalProviderHandle(identity) } + : undefined + ) + + expect(tail(journal, 100)).toMatchObject({ entries: [CLEARED], limited: false }) + const old = readAgentJournalPage(journal, { limit: 5, after: before.next }, waiting) + if (kind === 'forked') { + // A new epoch: pre-rewind cursors are refused and the owner rebuilds from what survived. + expect(old).toEqual({ ok: false }) + expect(work.builds).toBe(1) + } else { + // The same epoch: an old cursor reads the removal as nothing new, at the head. + expect(old).toMatchObject({ ok: true, entries: [], limited: false, next: { floor } }) + } + } +) diff --git a/src/main/persistence/loading-store/project-catalog/project-collection-operations.ts b/src/main/persistence/loading-store/project-catalog/project-collection-operations.ts index 5a811252866..b16eb246aac 100644 --- a/src/main/persistence/loading-store/project-catalog/project-collection-operations.ts +++ b/src/main/persistence/loading-store/project-catalog/project-collection-operations.ts @@ -11,6 +11,7 @@ import type { ProjectHostSetupUpdateResult, ProjectUpdateArgs } from '../../../../shared/project-types' +import type { ExecutionHostId } from '../../../../shared/execution-host' import type { Repo } from '../../../../shared/repo-types' import type { RepoHostTarget } from '../../../../shared/repo-host-identity' import { ProjectGroupPersistenceOperations } from '../../tracking-repos/project-group-operations' @@ -46,6 +47,7 @@ type ProjectCollectionOperationsRuntime = Pick< | 'projectGroupOperations' | 'projectHostOperations' | 'state' + | 'stateWriteListeners' | 'orcadTerminalRestoreHints' > @@ -110,6 +112,10 @@ export class ProjectCollectionOperations { return getProjectHostOperations(this).getRepo(id) } + getRepoForHost(id: string, hostId: ExecutionHostId): Repo | undefined { + return getProjectHostOperations(this).getRepoForHost(id, hostId) + } + setResolvedRepoGitUsername( target: Pick, username: string @@ -195,31 +201,46 @@ export class ProjectCollectionOperations { export function getProjectHostOperations( owner: ProjectCollectionOperations ): ProjectHostPersistenceOperations { - owner[projectCollectionOperationsContext].runtime.projectHostOperations ??= - new ProjectHostPersistenceOperations({ - state: owner[projectCollectionOperationsContext].runtime.state, - gitUsernameCache: owner[projectCollectionOperationsContext].runtime.gitUsernameCache, - bumpLocalWorktreeScanGeneration, - hydrateRepo: (repo) => hydrateRepo(owner[projectCollectionOperationsContext].repos, repo), - updateRepoBackedProjectHostSetup: (setup, repo, updates) => - updateRepoBackedProjectHostSetup( - owner[projectCollectionOperationsContext].repos, - setup, - repo, - updates - ), - updateIndependentProjectHostSetup: (setup, updates) => - updateIndependentProjectHostSetup( - owner[projectCollectionOperationsContext].repos, - setup, - updates - ), - removeProjectForHost: (target) => - owner[projectCollectionOperationsContext].repos.removeProjectForHost(target), - scheduleSave: (domains) => - scheduleSave(owner[projectCollectionOperationsContext].scheduling, domains) + const { runtime } = owner[projectCollectionOperationsContext] + if (!runtime.projectHostOperations) { + const operations = createProjectHostOperations(owner) + // Repo writes save the `repos` domain; load and migration save everything. + runtime.stateWriteListeners.add((domains) => { + if (!domains || domains.includes('repos')) { + operations.invalidateRepoIndex() + } }) - return owner[projectCollectionOperationsContext].runtime.projectHostOperations + runtime.projectHostOperations = operations + } + return runtime.projectHostOperations +} + +function createProjectHostOperations( + owner: ProjectCollectionOperations +): ProjectHostPersistenceOperations { + return new ProjectHostPersistenceOperations({ + state: owner[projectCollectionOperationsContext].runtime.state, + gitUsernameCache: owner[projectCollectionOperationsContext].runtime.gitUsernameCache, + bumpLocalWorktreeScanGeneration, + hydrateRepo: (repo) => hydrateRepo(owner[projectCollectionOperationsContext].repos, repo), + updateRepoBackedProjectHostSetup: (setup, repo, updates) => + updateRepoBackedProjectHostSetup( + owner[projectCollectionOperationsContext].repos, + setup, + repo, + updates + ), + updateIndependentProjectHostSetup: (setup, updates) => + updateIndependentProjectHostSetup( + owner[projectCollectionOperationsContext].repos, + setup, + updates + ), + removeProjectForHost: (target) => + owner[projectCollectionOperationsContext].repos.removeProjectForHost(target), + scheduleSave: (domains) => + scheduleSave(owner[projectCollectionOperationsContext].scheduling, domains) + }) } export function getProjectGroupOperations( diff --git a/src/main/persistence/tracking-repos/project-host-operations.ts b/src/main/persistence/tracking-repos/project-host-operations.ts index b0577b7867f..10ec1729b75 100644 --- a/src/main/persistence/tracking-repos/project-host-operations.ts +++ b/src/main/persistence/tracking-repos/project-host-operations.ts @@ -13,7 +13,15 @@ import type { import type { PersistedState } from '../../../shared/persisted-state-types' import type { Repo } from '../../../shared/repo-types' import type { RepoHostTarget } from '../../../shared/repo-host-identity' -import { getRepoExecutionHostId, normalizeExecutionHostId } from '../../../shared/execution-host' +import { + getRepoExecutionHostId, + normalizeExecutionHostId, + type ExecutionHostId +} from '../../../shared/execution-host' +import { + createRepoRowExecutionHostLookup, + type RepoRowExecutionHostLookup +} from '../../../shared/execution-host/worktree-execution-host-resolution' import { normalizeProjectRuntimePreference } from '../../../shared/execution-host/project-runtime-preference' import { findProjectHostSetup } from '../../../shared/project-host-setup-target' import { makeProjectHostSetupId } from './project-host-compatibility' @@ -38,8 +46,29 @@ export type ProjectHostMutationOperations = { } export class ProjectHostPersistenceOperations { + /** Repo rows by id, for keyed reads; dropped by every `repos` write and rebuilt on next use. */ + private repoIndex: { source: Repo[]; lookup: RepoRowExecutionHostLookup } | null = null + constructor(private readonly operations: ProjectHostMutationOperations) {} + /** A write changed the repo collection (membership, order or a row's host). */ + invalidateRepoIndex(): void { + this.repoIndex = null + } + + /** The repo registered under `id` on `hostId`, by key: never a pass over every registration. */ + getRepoForHost(id: string, hostId: ExecutionHostId): Repo | undefined { + // A replaced collection is a new array, even with no write notice. + if (this.repoIndex?.source !== this.state.repos) { + this.repoIndex = { + source: this.state.repos, + lookup: createRepoRowExecutionHostLookup(this.state.repos) + } + } + const repo = this.repoIndex.lookup.byHost(id, hostId) + return repo ? this.hydrateRepo(repo) : undefined + } + private get state(): PersistedState { return this.operations.state } diff --git a/src/main/runtime/agent-conversations/agent-conversation-list.ts b/src/main/runtime/agent-conversations/agent-conversation-list.ts index 4c20c66d675..3d42654042e 100644 --- a/src/main/runtime/agent-conversations/agent-conversation-list.ts +++ b/src/main/runtime/agent-conversations/agent-conversation-list.ts @@ -12,6 +12,8 @@ import { import { resolveLineageRunningSession } from '../orchestration/addressing/session-lineage/structured-session-lineage' import { agentHookServer } from '../../agent-hooks/server/agent-hook-server' import type { ResolvedAgentConversation } from './agent-conversation-address' +import type { ExecutionHostId } from '../../../shared/execution-host' +import { connectionEvidenceHostId } from '../workspaces/process-listing/runtime-worktree-summary-paths' /** `worktree.ps` agent rows as one flat list of chats and terminal agents, oldest state first. */ export function projectAgentConversationList( @@ -56,7 +58,12 @@ export function projectAgentConversationList( /** Where one resolved conversation is open now, from its own keyed status rows: never a listing * of every workspace. A closed chat or a terminal agent with no live pane has none. */ export async function agentConversationPlaces( - runtime: { getWorkspaceLabel(worktreeId: string): { workspace: string; repo?: string } | null }, + runtime: { + getWorkspaceLabel( + worktreeId: string, + hostId: ExecutionHostId + ): { workspace: string; repo?: string } | null + }, resolved: ResolvedAgentConversation ): Promise { const { orcaSessionId } = resolved @@ -85,6 +92,9 @@ export async function agentConversationPlaces( resolved.kind === 'chat' ? status.structuredHost !== undefined : status.orcaSessionId === orcaSessionId + // The place's own host, from the row's connection evidence, so a same-id workspace on + // another host never lends its name. + const hostId = connectionEvidenceHostId(status.connectionId) return worktreeId && ours ? [ { @@ -93,7 +103,8 @@ export async function agentConversationPlaces( agent: status.agentType ?? null, state: status.state, worktreeId, - ...(runtime.getWorkspaceLabel(worktreeId) ?? { workspace: worktreeId }), + ...(runtime.getWorkspaceLabel(worktreeId, hostId) ?? { workspace: worktreeId }), + hostId, paneKey: status.paneKey, title: null, lastAssistantMessage: status.lastAssistantMessage ?? null, diff --git a/src/main/runtime/agent-conversations/agent-conversation-place-labels.test.ts b/src/main/runtime/agent-conversations/agent-conversation-place-labels.test.ts new file mode 100644 index 00000000000..6e7c6749517 --- /dev/null +++ b/src/main/runtime/agent-conversations/agent-conversation-place-labels.test.ts @@ -0,0 +1,318 @@ +import { mkdtempSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' +import type * as HostResolution from '../../../shared/execution-host/worktree-execution-host-resolution' +import type { Repo } from '../../../shared/repo-types' +import { isOrcaSessionId } from '../../../shared/orca-session-address' +import { hostWorktreeTarget } from '../../../shared/worktree/host-qualified-identity' +import type { ResolvedWorktree } from '../workspaces/catalog/runtime-worktree-path-identity' +import { RuntimeResolvedWorktreeCache } from '../workspaces/catalog/runtime-resolved-worktree-cache' +import type { RuntimeWorkspaceSelectors } from '../workspaces/workspace-selectors' +import { agentConversationPlaces } from './agent-conversation-list' + +/** Repo-index builds, wherever the store or a caller groups repos by id. */ +const builds = vi.hoisted(() => ({ count: 0 })) +vi.mock('../../../shared/execution-host/worktree-execution-host-resolution', async (load) => { + const actual = await load() + const counted: typeof actual.createRepoRowExecutionHostLookup = (repos) => { + builds.count++ + return actual.createRepoRowExecutionHostLookup(repos) + } + return { + ...actual, + createRepoRowExecutionHostLookup: counted + } +}) +const hooks = vi.hoisted(() => ({ + statuses: new Map() +})) +vi.mock('../../agent-hooks/server/agent-hook-server', () => { + const places = { + terminalAgentPlaces: async () => [...hooks.statuses.keys()].map((paneKey) => ({ paneKey })), + getPublicStatusSnapshotForPane: (paneKey: string) => { + const status = hooks.statuses.get(paneKey) + return status + ? [ + { + ...status, + paneKey, + worktreeId: ID, + orcaSessionId: AGENT, + state: 'done', + receivedAt: 1 + } + ] + : [] + } + } + // The store's own hook-server calls are no-ops here. + const storeCalls = Object.fromEntries( + [ + 'clearPaneKeyAliasesForPty', + 'getStatusSnapshot', + 'ingestTerminalStatus', + 'registerPaneKeyAlias', + 'setPaneKeyAliasPersistenceListener', + 'stop', + 'transferPaneAuthority' + ].map((name) => [name, () => undefined]) + ) + return { agentHookServer: { ...storeCalls, ...places } } +}) +vi.mock('electron', () => ({ + app: { getPath: () => harness.testState.dir }, + safeStorage: { isEncryptionAvailable: () => false } +})) +vi.mock('../../telemetry/client', () => ({ track: vi.fn() })) +vi.mock('../../telemetry/cohort-classifier', () => ({ getCohortAtEmit: vi.fn() })) + +const harness = await import('../../persistence/persistence-test-harness') +const { RuntimeWorkspaceSelectors: Selectors } = await import('../workspaces/workspace-selectors') + +const ID = 'repo-a::/srv/app' +const AGENT = 'claude_place_0001' +const SSH = 'ssh:server' + +beforeEach(() => { + harness.testState.dir = mkdtempSync(join(tmpdir(), 'orca-place-labels-')) + hooks.statuses.clear() + builds.count = 0 +}) +afterEach(async () => { + await harness.closeTestStores() +}) + +const local = (displayName: string): Repo => + harness.makeRepo({ id: 'repo-a', path: '/srv/app', displayName, executionHostId: 'local' }) +const remote = (displayName: string): Repo => + harness.makeRepo({ id: 'repo-a', path: '/srv/app', displayName, connectionId: 'server' }) + +/** A real store holding `others` unrelated repos plus the same-id pair, target first or last. */ +function storeWith(others: number, order: 'local first' | 'ssh first', at: 'first' | 'last') { + const store = harness.createStore() + const pair = + order === 'local first' + ? [local('Local repo'), remote('Remote repo')] + : [remote('Remote repo'), local('Local repo')] + const fillers = Array.from({ length: others }, (_, index) => + harness.makeRepo({ id: `other-${index}`, path: `/srv/other-${index}` }) + ) + for (const repo of at === 'first' ? [...pair, ...fillers] : [...fillers, ...pair]) { + store.addRepo(repo) + } + store.setWorktreeMeta(hostWorktreeTarget({ id: ID, hostId: 'local' }), { + displayName: 'Local workspace', + displayNameIsPinned: true + }) + store.setWorktreeMeta(hostWorktreeTarget({ id: ID, hostId: SSH }), { + displayName: 'Remote workspace', + displayNameIsPinned: true + }) + return store +} + +function row(id: string, hostId: string, displayName: string): ResolvedWorktree { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the label lookup reads only a row's id, host, repo and name. + return { id, repoId: id.split('::')[0], hostId, displayName } as unknown as ResolvedWorktree +} + +/** A published catalog of `count` unrelated rows, with the pair or not, counting reads of each + * row's id: indexing a publication reads every one; a show reads none. */ +function catalog(count: number, withPair: boolean) { + const reads = { count: 0 } + const rows = Array.from({ length: count }, (_, index) => + row(`other-${index % 120}::/srv/w-${index}`, 'local', `w-${index}`) + ) + if (withPair) { + rows.push(row(ID, SSH, 'Remote workspace'), row(ID, 'local', 'Local workspace')) + } + for (const entry of rows) { + const id = entry.id + Object.defineProperty(entry, 'id', { + get: () => { + reads.count++ + return id + } + }) + } + return { worktrees: rows, reads } +} + +function runtimeOver( + store: ReturnType, + cache: RuntimeResolvedWorktreeCache +) { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: a real runtime prototype; the label and place code read only the store and catalog cache assigned here. + const runtime = Object.create(Selectors.prototype) as RuntimeWorkspaceSelectors + return Object.assign(runtime, { store, resolvedWorktrees: cache }) +} + +async function show(runtime: RuntimeWorkspaceSelectors, connectionId: string | null) { + hooks.statuses.clear() + hooks.statuses.set('pane-1', { connectionId }) + if (!isOrcaSessionId(AGENT)) { + throw new Error('expected an Orca session id') + } + const places = await agentConversationPlaces(runtime, { kind: 'terminal', orcaSessionId: AGENT }) + return places.map(({ workspace, repo, hostId }) => ({ workspace, repo, hostId })) +} + +const LOCAL_PLACE = { workspace: 'Local workspace', repo: 'Local repo', hostId: 'local' } +const REMOTE_PLACE = { workspace: 'Remote workspace', repo: 'Remote repo', hostId: SSH } + +it.each([ + [12, 750, 'first'], + [120, 7_500, 'last'] +] as const)( + 'labels a place from keyed records with %i repos and %i workspaces (target %s), warm or cold', + async (repos, worktrees, at) => { + const store = storeWith(repos, 'local first', at) + const getRepos = vi.spyOn(store, 'getRepos') + const getRepo = vi.spyOn(store, 'getRepo') + const getAllMeta = vi.spyOn(store, 'getAllWorktreeMeta') + const cache = new RuntimeResolvedWorktreeCache() + const runtime = runtimeOver(store, cache) + // The owner's repo index is built once, by its first keyed read. + await show(runtime, null) + builds.count = 0 + + const missing = catalog(worktrees, false) + const warm = catalog(worktrees, true) + for (const published of [null, missing, warm]) { + if (published) { + await cache.getSnapshot( + async () => ({ worktrees: published.worktrees, platformByRepoId: new Map() }), + 60_000, + 0 + ) + cache.invalidateResolved() + await cache.getSnapshot( + async () => ({ worktrees: published.worktrees, platformByRepoId: new Map() }), + 60_000, + 0 + ) + published.reads.count = 0 + } + for (let repeat = 0; repeat < 3; repeat++) { + expect(await show(runtime, null)).toEqual([LOCAL_PLACE]) + expect(await show(runtime, 'server')).toEqual([REMOTE_PLACE]) + } + expect(published?.reads.count ?? 0).toBe(0) + } + + expect({ + builds: builds.count, + getRepos: getRepos.mock.calls.length, + getRepo: getRepo.mock.calls.length, + getAllMeta: getAllMeta.mock.calls.length + }).toEqual({ + builds: 0, + getRepos: 0, + getRepo: 0, + getAllMeta: 0 + }) + } +) + +it('indexes each equivalent catalog once at publication, never again per show', async () => { + const store = storeWith(12, 'local first', 'first') + const cache = new RuntimeResolvedWorktreeCache() + const runtime = runtimeOver(store, cache) + await show(runtime, null) + builds.count = 0 + + for (let publication = 0; publication < 3; publication++) { + const published = catalog(750, true) + cache.invalidateResolved() + await cache.getSnapshot( + async () => ({ worktrees: published.worktrees, platformByRepoId: new Map() }), + 60_000, + 0 + ) + const atPublication = published.reads.count + for (let repeat = 0; repeat < 3; repeat++) { + expect(await show(runtime, 'server')).toEqual([REMOTE_PLACE]) + } + + expect(atPublication).toBe(752) + expect(published.reads.count).toBe(atPublication) + } + expect(builds.count).toBe(0) +}) + +it.each(['local first', 'ssh first'] as const)( + 'keeps each host its own label for one id on two hosts, %s', + async (order) => { + const store = storeWith(0, order, 'first') + const runtime = runtimeOver(store, new RuntimeResolvedWorktreeCache()) + + expect(await show(runtime, null)).toEqual([LOCAL_PLACE]) + expect(await show(runtime, 'wsl:Ubuntu')).toEqual([LOCAL_PLACE]) + expect(await show(runtime, 'server')).toEqual([REMOTE_PLACE]) + // A host with no registration of this repo borrows no sibling's name. + expect(await show(runtime, 'elsewhere')).toEqual([ + { workspace: ID, repo: undefined, hostId: 'ssh:elsewhere' } + ]) + } +) + +it('reads current labels after renames, removal, a host move and invalidation, and only then', async () => { + const store = storeWith(3, 'local first', 'first') + const cache = new RuntimeResolvedWorktreeCache() + const runtime = runtimeOver(store, cache) + await show(runtime, null) + builds.count = 0 + + // Writes elsewhere in the profile leave the repo index alone. + store.setWorktreeMeta(hostWorktreeTarget({ id: 'other-0::/srv/other-0', hostId: 'local' }), { + comment: 'unrelated' + }) + await show(runtime, null) + expect(builds.count).toBe(0) + + store.updateRepo({ id: 'repo-a', executionHostId: 'local' }, { displayName: 'Renamed repo' }) + store.setWorktreeMeta(hostWorktreeTarget({ id: ID, hostId: 'local' }), { + displayName: 'Renamed workspace', + displayNameIsPinned: true + }) + expect(await show(runtime, null)).toEqual([ + { workspace: 'Renamed workspace', repo: 'Renamed repo', hostId: 'local' } + ]) + + store.removeProjectForHost({ id: 'repo-a', connectionId: 'server' }) + expect(await show(runtime, 'server')).toEqual([{ workspace: ID, repo: undefined, hostId: SSH }]) + store.addRepo(remote('Back again')) + // Removal dropped that host's saved name, so the workspace is named after its repo again. + const readded = { workspace: 'Back again', repo: 'Back again', hostId: SSH } + expect(await show(runtime, 'server')).toEqual([readded]) + + // The same length, with the local registration moved to another host. + store.removeProjectForHost({ id: 'repo-a', executionHostId: 'local' }) + store.addRepo(harness.makeRepo({ id: 'repo-a', path: '/srv/app', connectionId: 'other-host' })) + expect((await show(runtime, null))[0]?.repo).toBeUndefined() + + const published = catalog(10, true) + await cache.getSnapshot( + async () => ({ worktrees: published.worktrees, platformByRepoId: new Map() }), + 60_000, + 0 + ) + // The published row names the workspace; its repo is read from the store's current record. + expect(await show(runtime, 'server')).toEqual([{ ...readded, workspace: 'Remote workspace' }]) + cache.invalidateResolved() + expect(await show(runtime, 'server')).toEqual([readded]) +}) + +it('labels a folder workspace by its own record', () => { + const store = harness.createStore() + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the label reads only the folder's name. + const docs = { name: 'Docs' } as ReturnType + vi.spyOn(store, 'getFolderWorkspace').mockImplementation((id) => + id === 'folder-1' ? docs : undefined + ) + const runtime = runtimeOver(store, new RuntimeResolvedWorktreeCache()) + + expect(runtime.getWorkspaceLabel('folder:folder-1', 'local')).toEqual({ workspace: 'Docs' }) + expect(runtime.getWorkspaceLabel('folder:folder-2', 'local')).toBeNull() +}) diff --git a/src/main/runtime/agent-conversations/agent-conversation-read.test.ts b/src/main/runtime/agent-conversations/agent-conversation-read.test.ts index 86051ce1f9c..338725d0c30 100644 --- a/src/main/runtime/agent-conversations/agent-conversation-read.test.ts +++ b/src/main/runtime/agent-conversations/agent-conversation-read.test.ts @@ -164,8 +164,12 @@ it('takes a cursor ahead of the journal to the reader, not as an empty read', as }) it.each([ - ['heard without its WSL relay', '', 'without its WSL relay'], - ['heard through its WSL relay', 'Ubuntu', null] + [ + 'saved with no distro', + '', + "Orca can't read Claude agent claude_dd9b33e3's transcript because its saved location doesn't identify which WSL distro holds it. Run `orca agent show claude_dd9b33e3` to find any open terminal for this agent and view it there." + ], + ['saved with its distro', 'Ubuntu', null] ])( 'reads a WSL terminal agent %s only when a distro was attested', async (_, wslDistro, refusal) => { diff --git a/src/main/runtime/agent-conversations/agent-conversation-read.ts b/src/main/runtime/agent-conversations/agent-conversation-read.ts index a68f8fa0536..52097b85b88 100644 --- a/src/main/runtime/agent-conversations/agent-conversation-read.ts +++ b/src/main/runtime/agent-conversations/agent-conversation-read.ts @@ -20,6 +20,7 @@ import { createWorkerOutputSourceIdentity } from '../orchestration/workers/trans import { agentHookServer } from '../../agent-hooks/server/agent-hook-server' import type { TerminalAgentTranscriptSource } from '../../agent-hooks/server/server-agent-identities' import { sameProviderKey } from '../../agent-hooks/identity/agent-provider-key' +import { tuiAgentDisplayName } from '../../../shared/agent-launch/catalog/tui-agent-display-names' import { needsWslHostTranslation } from '../../native-chat/transcript-discovery/host-readable-transcript-path' import { isReverseLookupUnavailable, @@ -175,10 +176,10 @@ async function readTerminalTranscript( providerSession.transcriptPath && needsWslHostTranslation(providerSession.transcriptPath) ) { - // Hooks that skipped the WSL relay never said which distro holds this guest path. + // A saved guest path with no distro names no file this host can open. throw new AgentConversationRefusal( CODES.noTranscript, - `Agent ${orcaSessionId}'s transcript is inside WSL, but Orca heard from it without its WSL relay, so it can't tell which distro to read it from.` + `Orca can't read ${tuiAgentDisplayName(key.agent) ?? key.agent} agent ${orcaSessionId}'s transcript because its saved location doesn't identify which WSL distro holds it. Run \`orca agent show ${orcaSessionId}\` to find any open terminal for this agent and view it there.` ) } const transcript = await readWorkerTranscript({ diff --git a/src/main/runtime/construction/runtime-store-contract.ts b/src/main/runtime/construction/runtime-store-contract.ts index 3d976eede42..84bc7fbec64 100644 --- a/src/main/runtime/construction/runtime-store-contract.ts +++ b/src/main/runtime/construction/runtime-store-contract.ts @@ -6,6 +6,7 @@ export type RuntimeStore = { getRepos: Store['getRepos'] getSshTargets?: Store['getSshTargets'] getRepo: Store['getRepo'] + getRepoForHost?: Store['getRepoForHost'] addRepo: Store['addRepo'] updateRepo: Store['updateRepo'] addRetiredWorktreeName?: Store['addRetiredWorktreeName'] diff --git a/src/main/runtime/workspaces/catalog/resolved-worktree-row.ts b/src/main/runtime/workspaces/catalog/resolved-worktree-row.ts new file mode 100644 index 00000000000..6ba60de0701 --- /dev/null +++ b/src/main/runtime/workspaces/catalog/resolved-worktree-row.ts @@ -0,0 +1,60 @@ +import type { ExecutionHostId } from '../../../../shared/execution-host' +import type { Repo } from '../../../../shared/repo-types' +import type { WorktreeMeta } from '../../../../shared/worktree/meta-types' +import { splitWorktreeIdForFilesystem } from '../../../../shared/worktree/id' +import { areWorktreePathsEqual } from '../../../git/worktrees/catalog/worktree-path-comparison' +import { mergeWorktree } from '../../../ipc/worktree-metadata-merge' +import { readWorktreeMetaForHost } from '../../../persistence/host-qualified-worktree-meta' +import type { RuntimeStore } from '../../construction/runtime-store-contract' +import type { ResolvedWorktree } from './runtime-worktree-path-identity' + +/** One place's row from its own host's records, by key: the repo registered on that host and + * that host's metadata. Null when no repo is registered there. */ +export function resolvedWorktreeForHost( + store: RuntimeStore, + worktreeId: string, + hostId: ExecutionHostId +): ResolvedWorktree | null { + const parsed = splitWorktreeIdForFilesystem(worktreeId) + const repo = parsed?.repoId ? store.getRepoForHost?.(parsed.repoId, hostId) : undefined + return repo + ? resolvedWorktreeRow( + worktreeId, + hostId, + repo, + readWorktreeMetaForHost(store, worktreeId, hostId) + ) + : null +} + +/** A worktree row from its id, owner host, repo and metadata, with no git listing: names follow + * `mergeWorktree`, so pinned and automatic labels read as they do in a catalog row. */ +export function resolvedWorktreeRow( + worktreeId: string, + hostId: ExecutionHostId, + repo: Repo | null | undefined, + meta: WorktreeMeta | undefined +): ResolvedWorktree | null { + const parsed = splitWorktreeIdForFilesystem(worktreeId) + if (!parsed?.repoId || !parsed.worktreePath) { + return null + } + const git = { + path: parsed.worktreePath, + head: '', + branch: '', + isBare: false, + isMainWorktree: repo ? areWorktreePathsEqual(parsed.worktreePath, repo.path) : false + } + const merged = { ...mergeWorktree(parsed.repoId, git, meta, repo?.displayName), hostId } + return { + ...merged, + id: worktreeId, + parentWorktreeId: null, + childWorktreeIds: [], + lineage: null, + git, + displayName: merged.displayName, + comment: merged.comment + } +} diff --git a/src/main/runtime/workspaces/catalog/runtime-resolved-worktree-cache.ts b/src/main/runtime/workspaces/catalog/runtime-resolved-worktree-cache.ts index ee4dfe3d8ae..2db032f4fba 100644 --- a/src/main/runtime/workspaces/catalog/runtime-resolved-worktree-cache.ts +++ b/src/main/runtime/workspaces/catalog/runtime-resolved-worktree-cache.ts @@ -1,11 +1,21 @@ import type { ResolvedWorktree } from './runtime-worktree-path-identity' +import type { ExecutionHostId } from '../../../../shared/execution-host' +import { + composeWorktreeHostIdentity, + type HostScopedWorktreeKey +} from '../../../../shared/worktree/host-qualified-identity' export type ResolvedWorktreeSnapshot = { worktrees: ResolvedWorktree[] platformByRepoId: ReadonlyMap } -type ResolvedCache = ResolvedWorktreeSnapshot & { expiresAt: number; inventoryRevision: number } +type ResolvedCache = ResolvedWorktreeSnapshot & { + expiresAt: number + inventoryRevision: number + /** The published rows by host-qualified id, built with the snapshot it belongs to. */ + byHostKey: ReadonlyMap +} type ResolvedInFlight = { generation: number inventoryRevision: number @@ -24,6 +34,11 @@ export class RuntimeResolvedWorktreeCache { return this.resolved } + /** One published row by its host and id, or undefined when none is published. */ + lookup(hostId: ExecutionHostId, worktreeId: string): ResolvedWorktree | undefined { + return this.resolved?.byHostKey.get(composeWorktreeHostIdentity(hostId, worktreeId)) + } + /** Revision invalidates catalogs missing newly registered repos; TTL alone could wrongly report remote worktrees absent. */ isFresh(inventoryRevision: number, now = Date.now()): boolean { return Boolean( @@ -54,7 +69,12 @@ export class RuntimeResolvedWorktreeCache { const result = await promise if (generation === this.resolvedGeneration) { // Start TTL on completion or slow computation publishes already-expired entries. - this.resolved = { ...result, inventoryRevision, expiresAt: Date.now() + ttlMs } + this.resolved = { + ...result, + inventoryRevision, + expiresAt: Date.now() + ttlMs, + byHostKey: indexByHostKey(result.worktrees) + } } return result } finally { @@ -69,3 +89,17 @@ export class RuntimeResolvedWorktreeCache { this.resolved = null } } + +/** Part of publishing a snapshot: one pass over rows the publication already holds. */ +function indexByHostKey( + worktrees: readonly ResolvedWorktree[] +): Map { + const byHostKey = new Map() + for (const worktree of worktrees) { + const hostId = worktree.identity?.executionHostId ?? worktree.hostId + if (hostId) { + byHostKey.set(composeWorktreeHostIdentity(hostId, worktree.id), worktree) + } + } + return byHostKey +} diff --git a/src/main/runtime/workspaces/session-hosts/get-runtime-id.ts b/src/main/runtime/workspaces/session-hosts/get-runtime-id.ts index c8c47cf42a1..9bfa22b5ed6 100644 --- a/src/main/runtime/workspaces/session-hosts/get-runtime-id.ts +++ b/src/main/runtime/workspaces/session-hosts/get-runtime-id.ts @@ -5,26 +5,13 @@ import type { WorkspaceSessionState } from '../../../../shared/workspace-session import type { SleepingAgentSessionRecord } from '../../../../shared/agent-session/lifecycle/agent-session-resume' import { parseWorkspaceKey } from '../../../../shared/workspace-scope' import { TerminalListHostScope } from '../../terminal/query/terminal-list-host-scope' -import type { ResolvedWorktree } from '../catalog/runtime-worktree-path-identity' - -const worktreesById = new WeakMap>() - -function resolvedWorktreeById( - worktrees: readonly ResolvedWorktree[] | undefined, - worktreeId: string -): ResolvedWorktree | undefined { - if (!worktrees) { - return undefined - } - let byId = worktreesById.get(worktrees) - if (!byId) { - byId = new Map(worktrees.map((worktree) => [worktree.id, worktree])) - worktreesById.set(worktrees, byId) - } - return byId.get(worktreeId) -} +import type { RuntimeWorkspaceSelectors } from '../workspace-selectors' export abstract class OrcaRuntimeWithGetRuntimeId extends TerminalListHostScope { + protected abstract buildResolvedWorktreeForHost( + ...args: Parameters + ): ReturnType + getRuntimeId(): string { return this.runtimeId } @@ -78,22 +65,25 @@ export abstract class OrcaRuntimeWithGetRuntimeId extends TerminalListHostScope ) } - /** One workspace's labels as `agent list` shows them, by key: its row in the cached resolved - * worktree list (indexed once per list), else its saved record; never a scan of every - * workspace. */ - getWorkspaceLabel(worktreeId: string): { workspace: string; repo?: string } | null { + /** One place's labels as `agent list` shows them, from that host's records by key: its row in + * the published catalog, else its own repo and metadata. Never another host's same-id + * workspace, and never a scan of every workspace or repo. */ + getWorkspaceLabel( + worktreeId: string, + hostId: ExecutionHostId + ): { workspace: string; repo?: string } | null { const folder = parseWorkspaceKey(worktreeId) if (folder?.type === 'folder') { const name = this.store?.getFolderWorkspace?.(folder.folderWorkspaceId)?.name return name ? { workspace: name } : null } const worktree = - resolvedWorktreeById(this.resolvedWorktrees.peek()?.worktrees, worktreeId) ?? - this.buildResolvedWorktreeFromId(worktreeId) + this.resolvedWorktrees.lookup(hostId, worktreeId) ?? + this.buildResolvedWorktreeForHost(worktreeId, hostId) if (!worktree?.displayName) { return null } - const repo = this.store?.getRepo(worktree.repoId) + const repo = this.store?.getRepoForHost?.(worktree.repoId, hostId) return { workspace: worktree.displayName, repo: repo?.displayName ?? worktree.repoId } } diff --git a/src/main/runtime/workspaces/session-hosts/workspace-display-name.test.ts b/src/main/runtime/workspaces/session-hosts/workspace-display-name.test.ts deleted file mode 100644 index da22b0a82d4..00000000000 --- a/src/main/runtime/workspaces/session-hosts/workspace-display-name.test.ts +++ /dev/null @@ -1,47 +0,0 @@ -import { expect, it, vi } from 'vitest' -import { OrcaRuntimeWithGetRuntimeId } from './get-runtime-id' - -const WSL_REPO = String.raw`52390e5d-872a-4a89-a008-7c6cd8e1c3b3::\\wsl.localhost\Ubuntu\home\orcatest\repo` - -function runtimeWith( - folders: number, - worktrees: { id: string; repoId: string; displayName: string }[] | null -) { - const store = { - getWorktreeMeta: vi.fn(() => undefined), - getRepo: vi.fn((id: string) => - id === '52390e5d-872a-4a89-a008-7c6cd8e1c3b3' ? { displayName: 'repo' } : undefined - ), - getFolderWorkspace: vi.fn((id: string) => (id === 'folder-1' ? { name: 'Docs' } : undefined)), - getFolderWorkspaces: vi.fn(() => - Array.from({ length: folders }, (_, index) => ({ id: `folder-${index}`, name: 'f' })) - ) - } - const runtime = Object.assign(Object.create(OrcaRuntimeWithGetRuntimeId.prototype), { - store, - resolvedWorktrees: { peek: () => (worktrees ? { worktrees } : null) }, - buildResolvedWorktreeFromId: vi.fn(() => null) - }) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the method reads only the store, the resolved-worktree cache and the keyed resolver set above. - return { runtime: runtime as OrcaRuntimeWithGetRuntimeId, store } -} - -it.each([750, 7_500])( - 'labels one workspace from its own records, whatever the other %i folder workspaces', - (folders) => { - const { runtime, store } = runtimeWith(folders, [ - { id: WSL_REPO, repoId: '52390e5d-872a-4a89-a008-7c6cd8e1c3b3', displayName: 'repo' } - ]) - - // A WSL worktree with no saved name reads as `agent list` shows it, not as its raw id. - expect(runtime.getWorkspaceLabel(WSL_REPO)).toEqual({ workspace: 'repo', repo: 'repo' }) - expect(runtime.getWorkspaceLabel('folder:folder-1')).toEqual({ workspace: 'Docs' }) - expect(store.getFolderWorkspaces).not.toHaveBeenCalled() - } -) - -it('labels nothing it has no record of, so the caller shows the id', () => { - const { runtime } = runtimeWith(0, null) - - expect(runtime.getWorkspaceLabel(WSL_REPO)).toBeNull() -}) diff --git a/src/main/runtime/workspaces/workspace-selectors.ts b/src/main/runtime/workspaces/workspace-selectors.ts index 23498c26bba..bc5995cda03 100644 --- a/src/main/runtime/workspaces/workspace-selectors.ts +++ b/src/main/runtime/workspaces/workspace-selectors.ts @@ -18,14 +18,13 @@ import type { Repo } from '../../../shared/repo-types' import { isFolderRepo } from '../../../shared/repo-kind' import type { RuntimeStore } from '../construction/runtime-store-contract' import type { RuntimeWorkspaceCatalog } from './catalog/workspace-catalog' -import { areWorktreePathsEqual } from '../../git/worktrees/catalog/worktree-path-comparison' import { resolveWorktreeMetaOwner } from '../../persistence/host-qualified-worktree-meta' +import { resolvedWorktreeForHost, resolvedWorktreeRow } from './catalog/resolved-worktree-row' import { resolveWorktreeHostRouting } from './catalog/worktree-launch-host-repo' import { createRepoRowExecutionHostLookup, type RepoRowExecutionHostLookup } from '../../../shared/execution-host/worktree-execution-host-resolution' -import { mergeWorktree } from '../../ipc/worktree-metadata-merge' import { RuntimeProjectLaunchContext } from './startup/runtime-project-launch-context' import { folderIdOfSelector } from '../../../shared/workspace-scope' import { @@ -287,28 +286,20 @@ export abstract class RuntimeWorkspaceSelectors extends RuntimeProjectLaunchCont if (!owner) { return null } - const repo = repos.byHost(parsed.repoId, owner.hostId) - const git = { - path: parsed.worktreePath, - head: '', - branch: '', - isBare: false, - isMainWorktree: repo ? areWorktreePathsEqual(parsed.worktreePath, repo.path) : false - } - const meta = this.store?.getWorktreeMeta(worktreeId) - const merged = { - ...mergeWorktree(parsed.repoId, git, meta, repo?.displayName), - hostId: owner.hostId - } - return { - ...merged, - id: worktreeId, - parentWorktreeId: null, - childWorktreeIds: [], - lineage: null, - git, - displayName: merged.displayName, - comment: merged.comment - } + return resolvedWorktreeRow( + worktreeId, + owner.hostId, + repos.byHost(parsed.repoId, owner.hostId), + this.store?.getWorktreeMeta(worktreeId) + ) + } + + /** One known place's row from its own host's records: the repo registered there (keyed) and + * that host's metadata. Null when no repo is registered on that host. */ + protected buildResolvedWorktreeForHost( + worktreeId: string, + hostId: ExecutionHostId + ): ResolvedWorktree | null { + return this.store ? resolvedWorktreeForHost(this.store, worktreeId, hostId) : null } }