diff --git a/mobile/src/terminal/use-terminal-live-accessory-input-commit.test.ts b/mobile/src/terminal/use-terminal-live-accessory-input-commit.test.ts index 2542b315a8f..4474f043ebe 100644 --- a/mobile/src/terminal/use-terminal-live-accessory-input-commit.test.ts +++ b/mobile/src/terminal/use-terminal-live-accessory-input-commit.test.ts @@ -26,6 +26,7 @@ function createDeferredBoolean(): DeferredBoolean { } type AccessoryInputCommitHarnessOptions = { + readonly composing?: boolean readonly heldText?: string readonly sentText?: string readonly pendingHandle?: string | null @@ -46,6 +47,7 @@ type AccessoryInputCommitHarness = { } function createAccessoryInputCommitHarness({ + composing, heldText = '', sentText = '', pendingHandle = null, @@ -55,6 +57,7 @@ function createAccessoryInputCommitHarness({ }: AccessoryInputCommitHarnessOptions = {}): AccessoryInputCommitHarness { const activeHandle = 'terminal-a' const heldLiveInputTextRef: RefObject = { current: heldText } + const liveInputComposingRef: RefObject = { current: composing } const sentLiveInputTextRef: RefObject = { current: sentText } const pendingLiveInputHandleRef: RefObject = { current: pendingHandle } const liveInputRef: RefObject = { current: null } @@ -66,7 +69,9 @@ function createAccessoryInputCommitHarness({ return sendResult } } - const applyLiveInputMirror = vi.fn((_handle: string, _fieldText: string) => {}) + const applyLiveInputMirror = vi.fn( + (_handle: string, _fieldText: string, _composing?: boolean) => {} + ) const clearPendingLiveInputCommit = vi.fn(() => {}) const flushPendingLiveInputText = vi.fn(async (_expectedHandle: string | null) => flushResult) const waitForPendingLiveInputFlush = vi.fn(async () => waitResult) @@ -82,6 +87,7 @@ function createAccessoryInputCommitHarness({ clearPendingLiveInputCommit, flushPendingLiveInputText, heldLiveInputTextRef, + liveInputComposingRef, liveInputRef, liveInputTerminalHandles, pendingLiveInputHandleRef, @@ -178,10 +184,11 @@ describe('terminal live accessory input commit hook', () => { expect(harness.sent).toEqual([]) }) - it('Given accessory backspace with a held syllable When committed Then mirrors the emptied field without terminal bytes', async () => { + it('Given accessory backspace with reported composition When committed Then keeps the edited preedit held', async () => { // Given const harness = createAccessoryInputCommitHarness({ - heldText: '한', + composing: true, + heldText: 'ni hao', sentText: '', pendingHandle: 'terminal-a' }) @@ -190,14 +197,31 @@ describe('terminal live accessory input commit hook', () => { const result = await harness.commit({ bytes: '\x7f', localEdit: 'backspace' }) // Then - expect(harness.applyLiveInputMirror).toHaveBeenCalledWith('terminal-a', '') + expect(harness.applyLiveInputMirror).toHaveBeenCalledWith('terminal-a', 'ni ha', true) expect(result).toEqual({ kind: 'handled' }) expect(harness.sent).toEqual([]) }) + it('Given accessory backspace with an unreported hold When committed Then preserves the Android fallback', async () => { + // Given + const harness = createAccessoryInputCommitHarness({ + heldText: '한글', + sentText: '', + pendingHandle: 'terminal-a' + }) + + // When + const result = await harness.commit({ bytes: '\x7f', localEdit: 'backspace' }) + + // Then + expect(harness.applyLiveInputMirror).toHaveBeenCalledWith('terminal-a', '한', undefined) + expect(result).toEqual({ kind: 'handled' }) + }) + it('Given accessory backspace with mirrored sent text When committed Then mirrors the shortened field so the diff emits DEL', async () => { // Given const harness = createAccessoryInputCommitHarness({ + composing: false, heldText: '', sentText: 'ab', pendingHandle: 'terminal-a' @@ -207,7 +231,7 @@ describe('terminal live accessory input commit hook', () => { const result = await harness.commit({ bytes: '\x7f', localEdit: 'backspace' }) // Then - expect(harness.applyLiveInputMirror).toHaveBeenCalledWith('terminal-a', 'a') + expect(harness.applyLiveInputMirror).toHaveBeenCalledWith('terminal-a', 'a', false) expect(result).toEqual({ kind: 'handled' }) }) }) diff --git a/mobile/src/terminal/use-terminal-live-accessory-input-commit.ts b/mobile/src/terminal/use-terminal-live-accessory-input-commit.ts index 0893ae16098..ef6a2339822 100644 --- a/mobile/src/terminal/use-terminal-live-accessory-input-commit.ts +++ b/mobile/src/terminal/use-terminal-live-accessory-input-commit.ts @@ -21,10 +21,11 @@ export async function getTerminalLiveAccessoryInactiveInputCommitResult( type TerminalLiveAccessoryInputCommitOptions = { readonly activeHandle: string | null - readonly applyLiveInputMirror: (handle: string, fieldText: string) => void + readonly applyLiveInputMirror: (handle: string, fieldText: string, composing?: boolean) => void readonly clearPendingLiveInputCommit: () => void readonly flushPendingLiveInputText: (expectedHandle: string | null) => Promise readonly heldLiveInputTextRef: RefObject + readonly liveInputComposingRef: RefObject readonly liveInputRef: RefObject readonly liveInputTerminalHandles: ReadonlySet readonly pendingLiveInputHandleRef: RefObject @@ -40,6 +41,7 @@ export function useTerminalLiveAccessoryInputCommit({ clearPendingLiveInputCommit, flushPendingLiveInputText, heldLiveInputTextRef, + liveInputComposingRef, liveInputRef, liveInputTerminalHandles, pendingLiveInputHandleRef, @@ -81,7 +83,8 @@ export function useTerminalLiveAccessoryInputCommit({ // field is edited here and the mirror diff syncs the PTY echo. setLiveInputCapture(editedText) liveInputRef.current?.setNativeProps({ text: editedText }) - applyLiveInputMirror(activeHandle, editedText) + // Preserve undefined so Android's heuristic hold still settles on its timer. + applyLiveInputMirror(activeHandle, editedText, liveInputComposingRef.current) return { kind: 'handled' } } case 'commit-held-then-send': @@ -101,6 +104,7 @@ export function useTerminalLiveAccessoryInputCommit({ clearPendingLiveInputCommit, flushPendingLiveInputText, heldLiveInputTextRef, + liveInputComposingRef, liveInputRef, liveInputTerminalHandles, pendingLiveInputHandleRef, diff --git a/mobile/src/terminal/use-terminal-live-input-commit.test.ts b/mobile/src/terminal/use-terminal-live-input-commit.test.ts index d4a39d2d417..b24636114a9 100644 --- a/mobile/src/terminal/use-terminal-live-input-commit.test.ts +++ b/mobile/src/terminal/use-terminal-live-input-commit.test.ts @@ -161,6 +161,35 @@ describe('terminal live input commit hook', () => { expect(sent).toEqual([]) }) + it('Given an iOS pinyin preedit When accessory Backspace edits it Then only the candidate reaches the terminal', async () => { + // Given + vi.useFakeTimers() + const { handlers, sent } = createTerminalLiveInputCommitHarness() + changeLiveInput(handlers, 'ni hao', true) + + // When + await handlers.handleLiveInputAccessoryBytes({ bytes: '\x7f', localEdit: 'backspace' }) + await vi.advanceTimersByTimeAsync(TERMINAL_LIVE_HELD_PREEDIT_COMMIT_DELAY_MS * 2) + changeLiveInput(handlers, '你好', false) + + // Then + await vi.waitFor(() => expect(sent).toEqual(['你好'])) + }) + + it('Given an Android heuristic hold When accessory Backspace edits it Then the remaining text still settles', async () => { + // Given + vi.useFakeTimers() + const { handlers, sent } = createTerminalLiveInputCommitHarness() + changeLiveInput(handlers, '한글') + + // When + await handlers.handleLiveInputAccessoryBytes({ bytes: '\x7f', localEdit: 'backspace' }) + await vi.advanceTimersByTimeAsync(TERMINAL_LIVE_HELD_PREEDIT_COMMIT_DELAY_MS) + + // Then + await vi.waitFor(() => expect(sent).toEqual(['한'])) + }) + it('Given a held syllable When the settle timer elapses Then commits it to the terminal', async () => { // Given vi.useFakeTimers() diff --git a/mobile/src/terminal/use-terminal-live-input-commit.ts b/mobile/src/terminal/use-terminal-live-input-commit.ts index b05241f6421..25bbf1ae002 100644 --- a/mobile/src/terminal/use-terminal-live-input-commit.ts +++ b/mobile/src/terminal/use-terminal-live-input-commit.ts @@ -68,6 +68,7 @@ export function useTerminalLiveInputCommit({ clearPendingLiveInputCommit, flushPendingLiveInputText, heldLiveInputTextRef, + liveInputComposingRef, pendingLiveInputHandleRef, sentLiveInputTextRef, waitForPendingLiveInputFlush @@ -196,6 +197,7 @@ export function useTerminalLiveInputCommit({ clearPendingLiveInputCommit, flushPendingLiveInputText, heldLiveInputTextRef, + liveInputComposingRef, liveInputRef, liveInputTerminalHandles, pendingLiveInputHandleRef, diff --git a/mobile/src/terminal/use-terminal-live-pending-input-flush.ts b/mobile/src/terminal/use-terminal-live-pending-input-flush.ts index 75dd3e49f61..c01aa5af213 100644 --- a/mobile/src/terminal/use-terminal-live-pending-input-flush.ts +++ b/mobile/src/terminal/use-terminal-live-pending-input-flush.ts @@ -34,6 +34,7 @@ type TerminalLivePendingInputFlush = { readonly clearPendingLiveInputCommit: () => void readonly flushPendingLiveInputText: (expectedHandle: string | null) => Promise readonly heldLiveInputTextRef: RefObject + readonly liveInputComposingRef: RefObject readonly pendingLiveInputHandleRef: RefObject readonly sentLiveInputTextRef: RefObject readonly waitForPendingLiveInputFlush: () => Promise @@ -50,6 +51,7 @@ export function useTerminalLivePendingInputFlush({ const heldCommitTimerRef = useRef | null>(null) const pendingLiveInputFlushRef = useRef(createTerminalLivePendingFlushState()) const heldLiveInputTextRef = useRef('') + const liveInputComposingRef = useRef(undefined) const sentLiveInputTextRef = useRef('') const pendingLiveInputHandleRef = useRef(null) const runMirrorStepRef = useRef(async () => false) @@ -65,6 +67,7 @@ export function useTerminalLivePendingInputFlush({ clearHeldCommitTimer() cancelTerminalLivePendingFlush(pendingLiveInputFlushRef.current) heldLiveInputTextRef.current = '' + liveInputComposingRef.current = undefined sentLiveInputTextRef.current = '' pendingLiveInputHandleRef.current = null }, [clearHeldCommitTimer]) @@ -106,6 +109,7 @@ export function useTerminalLivePendingInputFlush({ }) sentLiveInputTextRef.current = step.nextSentText heldLiveInputTextRef.current = step.heldText + liveInputComposingRef.current = composing pendingLiveInputHandleRef.current = step.heldText.length > 0 || step.nextSentText.length > 0 ? handle : null @@ -187,6 +191,7 @@ export function useTerminalLivePendingInputFlush({ heldCommitTimerRef.current = null } heldLiveInputTextRef.current = '' + liveInputComposingRef.current = undefined sentLiveInputTextRef.current = '' pendingLiveInputHandleRef.current = null cancelTerminalLivePendingFlush(pendingLiveInputFlushRef.current) @@ -198,6 +203,7 @@ export function useTerminalLivePendingInputFlush({ clearPendingLiveInputCommit, flushPendingLiveInputText, heldLiveInputTextRef, + liveInputComposingRef, pendingLiveInputHandleRef, sentLiveInputTextRef, waitForPendingLiveInputFlush diff --git a/src/cli/help.ts b/src/cli/help.ts index d43c8294963..22272a2c28d 100644 --- a/src/cli/help.ts +++ b/src/cli/help.ts @@ -268,9 +268,9 @@ Common Commands: Selectors: --repo Registered repo selector such as id:, name:, or path: - --worktree Worktree selector such as id:::, name:, branch:, issue:, path:, or active/current + --worktree Worktree selector such as identity:, id:::, name:, branch:, issue:, path:, or active/current --terminal Runtime-issued terminal handle returned by \`orca terminal list --json\` - --parent-worktree Parent worktree selector such as id:::, branch:, issue:, path:, or active/current + --parent-worktree Parent worktree selector such as identity:, id:::, branch:, issue:, path:, or active/current --no-parent Force no parent lineage for unrelated worktree creation/update Terminal Send Options: @@ -509,7 +509,7 @@ function formatCommandFlagHelp(flag: string, commandPath: string[]): string { return '--parent-current Use the current linked issue as parent' } if (command === 'worktree create' && flag === 'parent-worktree') { - return '--parent-worktree Parent selector such as active/current, id:::, branch:, issue:, path:, folder:, or worktree:' + return '--parent-worktree Parent selector such as identity:, active/current, id:::, branch:, issue:, path:, folder:, or worktree:' } if (command === 'orchestration task-create' && flag === 'task-title') { return '--task-title Concise title for the orchestration task' @@ -577,7 +577,7 @@ export function formatFlagHelp(flag: string): string { 'no-screenshot': '--no-screenshot Skip screenshot capture after the operation', pages: '--pages Number of scroll pages', 'parent-worktree': - '--parent-worktree Parent worktree selector such as id:::, branch:, issue:, path:, or active/current', + '--parent-worktree Parent worktree selector such as identity:, id:::, branch:, issue:, path:, or active/current', path: '--path Path argument for the command', prompt: '--prompt Prompt text for agent-backed commands', query: '--query Search text for matching refs', @@ -601,7 +601,7 @@ export function formatFlagHelp(flag: string): string { 'to-x': '--to-x Destination window-local x coordinate', 'to-y': '--to-y Destination window-local y coordinate', worktree: - '--worktree Worktree selector such as id:::, name:, branch:, issue:, path:, or active/current', + '--worktree Worktree selector such as identity:, id:::, name:, branch:, issue:, path:, or active/current', workspace: '--workspace Existing worktree selector for automation runs', 'workspace-status': '--workspace-status Board status id (defaults: todo, in-progress, in-review, completed)', diff --git a/src/cli/index.test.ts b/src/cli/index.test.ts index 8daf4db772b..1b190d9dbde 100644 --- a/src/cli/index.test.ts +++ b/src/cli/index.test.ts @@ -41,6 +41,7 @@ vi.mock('child_process', async () => { }) import { COMMAND_SPECS, main } from './index' +import { formatFlagHelp } from './help' import { GLOBAL_FLAGS, specPaths } from './args' import { okFixture, queueFixtures } from './test-fixtures' @@ -480,6 +481,7 @@ describe('orca root help', () => { const rootHelp = String(logSpy.mock.calls[0][0]) expect(rootHelp).not.toContain('--parent-workspace') expect(rootHelp).toContain('[--parent-worktree ] [--no-parent]') + expect(rootHelp).toContain('identity:') logSpy.mockClear() await main(['worktree', 'create', '--help'], '/tmp/repo') @@ -504,6 +506,7 @@ describe('orca root help', () => { const setHelp = String(logSpy.mock.calls[0][0]) expect(setHelp).not.toContain('--parent-workspace') expect(setHelp).not.toContain('folder:') + expect(formatFlagHelp('parent-worktree')).toContain('identity:') expect(setHelp).not.toContain('worktree:') expect(callMock).not.toHaveBeenCalled() }) diff --git a/src/cli/selectors.ts b/src/cli/selectors.ts index aef583ad0ac..cfc2dd68749 100644 --- a/src/cli/selectors.ts +++ b/src/cli/selectors.ts @@ -39,7 +39,7 @@ function assertLocalCwdWorktreeSelector(selector: string, client: RuntimeClient) // server, so cwd-derived worktree selectors are only valid locally. throw new RuntimeClientError( 'invalid_argument', - `${selector} is a local cwd shortcut and cannot be resolved against a remote runtime. Pass an explicit server-side worktree selector such as id:::, name:, branch:, issue:, or path:.` + `${selector} is a local cwd shortcut and cannot be resolved against a remote runtime. Pass an explicit server-side worktree selector such as identity:, id:::, name:, branch:, issue:, or path:.` ) } diff --git a/src/main/ipc/pty/delivery/debug.ts b/src/main/ipc/pty/delivery/debug.ts index 99dae69488d..f9c11c19d97 100644 --- a/src/main/ipc/pty/delivery/debug.ts +++ b/src/main/ipc/pty/delivery/debug.ts @@ -88,31 +88,51 @@ export const EMPTY_PTY_RENDERER_DELIVERY_DEBUG_SNAPSHOT: PtyRendererDeliveryDebu rendererDispatcherReadyForcedCount: 0 } -export let readPtyRendererDeliveryDebugSnapshot = (): PtyRendererDeliveryDebugSnapshot => ({ - ...EMPTY_PTY_RENDERER_DELIVERY_DEBUG_SNAPSHOT -}) -export let resetPtyRendererDeliveryDebugSnapshot = (): void => {} +// Why null-init + wrapper fns (not `export let fn = noop`): rolldown const-folds the noop into +// call sites and drops the setter's reassignment, so bridged closures never run in the built app (STA-5661). +let readPtyRendererDeliveryDebugSnapshotImpl: (() => PtyRendererDeliveryDebugSnapshot) | null = null +let resetPtyRendererDeliveryDebugSnapshotImpl: (() => void) | null = null // Bridged into the registerPtyHandlers closure so the module-scope lifecycle-reset handler can zero closure-owned delivery accounting on renderer reload/crash. -export let resetRendererDeliveryAccountingForLifecycleReset = (): void => {} +let resetRendererDeliveryAccountingForLifecycleResetImpl: (() => void) | null = null // Bridged so a re-registration can cancel the prior closure's dispatcher-ready watchdog before wiring its own. -export let clearRendererDispatcherReadyWatchdog = (): void => {} +let clearRendererDispatcherReadyWatchdogImpl: (() => void) | null = null + +export function readPtyRendererDeliveryDebugSnapshot(): PtyRendererDeliveryDebugSnapshot { + return ( + readPtyRendererDeliveryDebugSnapshotImpl?.() ?? { + ...EMPTY_PTY_RENDERER_DELIVERY_DEBUG_SNAPSHOT + } + ) +} + +export function resetPtyRendererDeliveryDebugSnapshot(): void { + resetPtyRendererDeliveryDebugSnapshotImpl?.() +} + +export function resetRendererDeliveryAccountingForLifecycleReset(): void { + resetRendererDeliveryAccountingForLifecycleResetImpl?.() +} + +export function clearRendererDispatcherReadyWatchdog(): void { + clearRendererDispatcherReadyWatchdogImpl?.() +} export function setReadPtyRendererDeliveryDebugSnapshot( fn: () => PtyRendererDeliveryDebugSnapshot ): void { - readPtyRendererDeliveryDebugSnapshot = fn + readPtyRendererDeliveryDebugSnapshotImpl = fn } export function setResetPtyRendererDeliveryDebugSnapshot(fn: () => void): void { - resetPtyRendererDeliveryDebugSnapshot = fn + resetPtyRendererDeliveryDebugSnapshotImpl = fn } export function setResetRendererDeliveryAccountingForLifecycleReset(fn: () => void): void { - resetRendererDeliveryAccountingForLifecycleReset = fn + resetRendererDeliveryAccountingForLifecycleResetImpl = fn } export function setClearRendererDispatcherReadyWatchdog(fn: () => void): void { - clearRendererDispatcherReadyWatchdog = fn + clearRendererDispatcherReadyWatchdogImpl = fn } export function getPtyRendererDeliveryDebugSnapshot(): PtyRendererDeliveryDebugSnapshot { diff --git a/src/main/ipc/pty/delivery/visibility-state.ts b/src/main/ipc/pty/delivery/visibility-state.ts index dbe9526ae2e..21c18508055 100644 --- a/src/main/ipc/pty/delivery/visibility-state.ts +++ b/src/main/ipc/pty/delivery/visibility-state.ts @@ -9,8 +9,17 @@ export const interactiveOutputCharsByPty = new Map() export const activeRendererPtys = new Set() export const visibleRendererPtys = new Set() export const rendererVisibilityKnownPtys = new Set() -export let invalidatePendingPtyDrainPriority = (_id?: string, _schedule?: boolean): void => {} -export let invalidatePendingPtyDrainPolicy = (_id?: string, _schedule?: boolean): void => {} +// Why null-init + wrapper fns: see debug.ts — rolldown const-folds `export let fn = noop` bridges (STA-5661). +let invalidatePendingPtyDrainPriorityImpl: ((id?: string, schedule?: boolean) => void) | null = null +let invalidatePendingPtyDrainPolicyImpl: ((id?: string, schedule?: boolean) => void) | null = null + +export function invalidatePendingPtyDrainPriority(id?: string, schedule?: boolean): void { + invalidatePendingPtyDrainPriorityImpl?.(id, schedule) +} + +export function invalidatePendingPtyDrainPolicy(id?: string, schedule?: boolean): void { + invalidatePendingPtyDrainPolicyImpl?.(id, schedule) +} export const pendingHiddenRendererResizeOutputPtys = new Set() export const deliveredHiddenRendererResizeOutputPtys = new Set() export const KEEP_HISTORY_STOP_SETTLE_MS = 1_000 @@ -21,11 +30,11 @@ export const providerSnapshotRequiredPtys = new Set() export function setInvalidatePendingPtyDrainPriority( fn: (id?: string, schedule?: boolean) => void ): void { - invalidatePendingPtyDrainPriority = fn + invalidatePendingPtyDrainPriorityImpl = fn } export function setInvalidatePendingPtyDrainPolicy( fn: (id?: string, schedule?: boolean) => void ): void { - invalidatePendingPtyDrainPolicy = fn + invalidatePendingPtyDrainPolicyImpl = fn } diff --git a/src/main/ipc/pty/provider/listener-lifecycle.ts b/src/main/ipc/pty/provider/listener-lifecycle.ts index d6cb383c9a3..2b31257234e 100644 --- a/src/main/ipc/pty/provider/listener-lifecycle.ts +++ b/src/main/ipc/pty/provider/listener-lifecycle.ts @@ -17,7 +17,12 @@ export let rendererGateResetLoadHandler: (() => void) | null = null export let rendererGateResetGoneHandler: (() => void) | null = null export let rendererGateResetWebContents: WebContents | null = null // Why: the backgrounded-delivery dedupe map lives in the registerPtyHandlers closure but teardown funnels through module-scope clearProviderPtyState. -export let clearBackgroundedDeliverySyncForPty: (id: string) => void = () => {} +// Why null-init + wrapper fn: see delivery/debug.ts — rolldown const-folds `export let fn = noop` bridges (STA-5661). +let clearBackgroundedDeliverySyncForPtyImpl: ((id: string) => void) | null = null + +export function clearBackgroundedDeliverySyncForPty(id: string): void { + clearBackgroundedDeliverySyncForPtyImpl?.(id) +} export type RendererNavigationDetails = { isMainFrame: boolean @@ -42,7 +47,7 @@ export function setRebindProviderListeners(fn: (() => void) | null): void { } export function setClearBackgroundedDeliverySyncForPty(fn: (id: string) => void): void { - clearBackgroundedDeliverySyncForPty = fn + clearBackgroundedDeliverySyncForPtyImpl = fn } export function setSshOutputIntakeCleanup(fn: (() => void) | null): void { diff --git a/src/main/ipc/repos/project-host-setup-handlers.ts b/src/main/ipc/repos/project-host-setup-handlers.ts index 610968614a2..3feb101324c 100644 --- a/src/main/ipc/repos/project-host-setup-handlers.ts +++ b/src/main/ipc/repos/project-host-setup-handlers.ts @@ -12,10 +12,8 @@ import type { ProjectHostSetupUpdateArgs, ProjectHostSetupUpdateResult } from '../../../shared/project-types' -import { - getProjectHostSetupForRepo, - getProjectIdForProviderIdentity -} from '../../../shared/project-host-setup-projection' +import { getProjectIdForProviderIdentity } from '../../../shared/project-host-setup-projection' +import { getProjectHostSetupForRepo } from '../../../shared/project-host-setup-lookup' import { parseExecutionHostId } from '../../../shared/execution-host' import { prepareLocalWorktreeRootForRepo } from '../../worktree-root-preparation' import { invalidateAuthorizedRootsCache } from '../registered-worktree-roots-cache' diff --git a/src/main/ipc/workspace-cleanup-broad-scan.test.ts b/src/main/ipc/workspace-cleanup-broad-scan.test.ts index 070ea461906..0bfa3e3ccc9 100644 --- a/src/main/ipc/workspace-cleanup-broad-scan.test.ts +++ b/src/main/ipc/workspace-cleanup-broad-scan.test.ts @@ -179,7 +179,7 @@ describe('workspace cleanup broad scan opt-in', () => { ...makeStore(), getWorktreeMeta: (worktreeId: string) => worktreeId === 'repo-1::/repo-old' ? runtimeMeta : META_BY_WORKTREE_ID[worktreeId] - } as Store + } as unknown as Store const result = await scanWorkspaceCleanup(store, { includeAllWorkspaces: true @@ -561,6 +561,36 @@ describe('workspace cleanup broad scan opt-in', () => { }) }) + it('keeps a canonical-only SSH cleanup row blocked while local metadata collides', async () => { + const worktreeId = 'repo-1::/shared/workspace' + const localRepo = { ...REPO, path: '/local/repo' } + const sshRepo = { ...REPO, path: '/remote/repo', connectionId: 'ssh-1' } + const localMeta = makeWorktreeMeta({ + displayName: 'Local workspace', + hostId: 'local' + }) + const remoteMeta = makeWorktreeMeta({ + displayName: 'Canonical remote workspace', + hostId: 'ssh:ssh-1' + }) + const store = { + ...makeStore([localRepo, sshRepo], { [worktreeId]: localMeta }), + getAllWorktreeMetaForHost: (hostId: string) => + hostId === 'ssh:ssh-1' ? { [worktreeId]: remoteMeta } : { [worktreeId]: localMeta } + } as unknown as Store + + const result = await scanWorkspaceCleanup(store, { includeAllWorkspaces: true }) + + expect(result.candidates).toContainEqual( + expect.objectContaining({ + worktreeId, + displayName: 'Canonical remote workspace', + executionHostId: 'ssh:ssh-1', + blockers: ['ssh-disconnected'] + }) + ) + }) + it('does not synthesize a disconnected SSH row from same-id local metadata', async () => { const sharedPath = '/shared/workspace' const worktreeId = `${REPO.id}::${sharedPath}` diff --git a/src/main/ipc/workspace-cleanup-disconnected-ssh.ts b/src/main/ipc/workspace-cleanup-disconnected-ssh.ts index c0e136b7b77..b616bec22bb 100644 --- a/src/main/ipc/workspace-cleanup-disconnected-ssh.ts +++ b/src/main/ipc/workspace-cleanup-disconnected-ssh.ts @@ -15,6 +15,10 @@ import { isWorkspaceInactiveForCleanup } from './workspace-cleanup-candidate' import { getRepoOwnedWorktreeMeta, isWorktreeMetaOwnedByRepo } from '../worktree-metadata-ownership' +import { + readAllWorktreeMetaForHost, + readWorktreeMetaForHost +} from '../persistence/host-qualified-worktree-meta' export function synthesizeDisconnectedSshCleanupCandidates( store: Store, @@ -25,6 +29,7 @@ export function synthesizeDisconnectedSshCleanupCandidates( includeAllWorkspaces = false ): WorkspaceCleanupCandidate[] { const repoWorktreePrefix = `${repo.id}::` + const executionHostId = getRepoExecutionHostId(repo) if (targetWorktreeIds) { const candidates: WorkspaceCleanupCandidate[] = [] // Why: targeted refreshes name their workspaces already; walking all @@ -33,7 +38,11 @@ export function synthesizeDisconnectedSshCleanupCandidates( if (!worktreeId.startsWith(repoWorktreePrefix)) { continue } - const meta = store.getWorktreeMeta(worktreeId) + const meta = + readWorktreeMetaForHost(store, worktreeId, executionHostId) ?? + (typeof store.getWorktreeMetaForHost === 'function' + ? undefined + : store.getWorktreeMeta(worktreeId)) if (isWorktreeMetaOwnedByRepo(repo, meta, repoOwnerCount)) { candidates.push(createDisconnectedSshCandidate(repo, scannedAt, worktreeId, meta)) } @@ -42,7 +51,7 @@ export function synthesizeDisconnectedSshCleanupCandidates( } const candidates: WorkspaceCleanupCandidate[] = [] - const allMeta = store.getAllWorktreeMeta() + const allMeta = readAllWorktreeMetaForHost(store, executionHostId) for (const worktreeId in allMeta) { if (!Object.hasOwn(allMeta, worktreeId) || !worktreeId.startsWith(repoWorktreePrefix)) { continue diff --git a/src/main/ipc/workspace-cleanup-scan.ts b/src/main/ipc/workspace-cleanup-scan.ts index b87667f0a63..58b093dbd72 100644 --- a/src/main/ipc/workspace-cleanup-scan.ts +++ b/src/main/ipc/workspace-cleanup-scan.ts @@ -1,6 +1,8 @@ import type { Store } from '../persistence' import type { IGitProvider } from '../providers/types' import { isFolderRepo } from '../../shared/repo-kind' +import { getRepoExecutionHostId } from '../../shared/execution-host' +import { readWorktreeMetaForHost } from '../persistence/host-qualified-worktree-meta' import type { Repo } from '../../shared/repo-types' import type { GitWorktreeInfo, Worktree } from '../../shared/worktree/types' import { mergeWorktree } from './worktree-logic' @@ -187,8 +189,11 @@ async function scanRepoWorkspaces( ? listWorkspaceCleanupFolderWorkspaces(store, repo, repoOwnerCount) : gitWorktrees.map((gitWorktree) => { const worktreeId = `${repo.id}::${gitWorktree.path}` + // Host-qualified first: the same repoId::path is a different checkout on each host. + const hostMeta = readWorktreeMetaForHost(store, worktreeId, getRepoExecutionHostId(repo)) const meta = store.getWorktreeMeta(worktreeId) - const ownedMeta = isWorktreeMetaOwnedByRepo(repo, meta, repoOwnerCount) ? meta : undefined + const ownedMeta = + hostMeta ?? (isWorktreeMetaOwnedByRepo(repo, meta, repoOwnerCount) ? meta : undefined) return mergeWorktree(repo.id, gitWorktree, ownedMeta, repo.displayName) }) // Why: with includeAllWorkspaces the browser shows every workspace and lets diff --git a/src/main/ipc/worktree-metadata-merge.test.ts b/src/main/ipc/worktree-metadata-merge.test.ts new file mode 100644 index 00000000000..561897beb5c --- /dev/null +++ b/src/main/ipc/worktree-metadata-merge.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, it } from 'vitest' +import { canonicalWorktreeIdentity } from '../../shared/worktree/identity' +import type { GitWorktreeInfo } from '../../shared/worktree/types' +import { mergeWorktree } from './worktree-metadata-merge' + +const git: GitWorktreeInfo = { + path: '/workspace/feature', + head: 'abc123', + branch: 'refs/heads/feature', + isBare: false, + isMainWorktree: false +} + +describe('mergeWorktree identity projection', () => { + it('publishes canonical identity when host and instance metadata are known', () => { + const worktree = mergeWorktree('repo-1', git, { + instanceId: '11111111-1111-4111-8111-111111111111', + hostId: 'ssh:build-box', + displayName: 'Feature', + comment: '', + linkedIssue: null, + linkedPR: null, + linkedLinearIssue: null, + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 0, + lastActivityAt: 0 + }) + + expect(worktree.identity).toEqual({ + key: canonicalWorktreeIdentity({ + worktreeId: worktree.id, + executionHostId: 'ssh:build-box', + instanceId: '11111111-1111-4111-8111-111111111111' + }), + executionHostId: 'ssh:build-box', + instanceId: '11111111-1111-4111-8111-111111111111' + }) + }) + + it('omits canonical identity for legacy metadata without a proven host', () => { + const worktree = mergeWorktree('repo-1', git, undefined) + + expect(worktree.identity).toBeUndefined() + }) +}) diff --git a/src/main/ipc/worktree-metadata-merge.ts b/src/main/ipc/worktree-metadata-merge.ts index 4fdfe07c72a..742467c63a2 100644 --- a/src/main/ipc/worktree-metadata-merge.ts +++ b/src/main/ipc/worktree-metadata-merge.ts @@ -4,6 +4,7 @@ import type { GitWorktreeInfo, Worktree } from '../../shared/worktree/types' import { DEFAULT_WORKSPACE_STATUS_ID } from '../../shared/workspace-statuses' import { getLinkedWorkItemMetadata } from './worktree-linked-work-item-metadata' import { normalizeWorkspaceCreatorProvenance } from '../../shared/workspace-creator-provenance' +import { createWorktreeIdentity } from '../../shared/worktree/identity' /** * Merge raw git worktree info with persisted user metadata into a full Worktree. @@ -16,8 +17,18 @@ export function mergeWorktree( ): Worktree { const branchShort = git.branch.replace(/^refs\/heads\//, '') const creatorProvenance = normalizeWorkspaceCreatorProvenance(meta?.creatorProvenance) + const worktreeId = `${repoId}::${git.path}` return { - id: `${repoId}::${git.path}`, + id: worktreeId, + ...(meta?.instanceId && meta.hostId + ? { + identity: createWorktreeIdentity({ + worktreeId, + executionHostId: meta.hostId, + instanceId: meta.instanceId + }) + } + : {}), ...(meta?.instanceId !== undefined ? { instanceId: meta.instanceId } : {}), repoId, ...(meta?.projectId !== undefined ? { projectId: meta.projectId } : {}), diff --git a/src/main/ipc/worktree-remote.ts b/src/main/ipc/worktree-remote.ts index e71c16c7ffe..c20899ea9cf 100644 --- a/src/main/ipc/worktree-remote.ts +++ b/src/main/ipc/worktree-remote.ts @@ -48,7 +48,7 @@ import type { RemoteFetchResult, RemoteTrackingBase } from '../runtime/orca-runtime' -import { getProjectHostSetupWorktreeMeta } from '../../shared/project-host-setup-projection' +import { getProjectHostSetupWorktreeMeta } from '../../shared/project-host-setup-lookup' import { getEffectiveHooks, loadHooks, parseOrcaYaml } from '../hooks' import { buildPosixRunnerScript, buildWindowsRunnerScript } from '../setup-runner-script-text' import { createSetupRunnerScript, resolveSetupRunnerShell } from '../worktree-runner-script' diff --git a/src/main/ipc/worktrees-create-metadata-persistence.test.ts b/src/main/ipc/worktrees-create-metadata-persistence.test.ts index 7e2ebac49f5..1aeff9627b4 100644 --- a/src/main/ipc/worktrees-create-metadata-persistence.test.ts +++ b/src/main/ipc/worktrees-create-metadata-persistence.test.ts @@ -205,6 +205,36 @@ describe('registerWorktreeHandlers', () => { }) expect(result).toMatchObject({ comment: 'keep me', isPinned: true }) }) + it('routes metadata updates through the explicitly selected host', () => { + store.setWorktreeMetaForHost.mockImplementation((_worktreeId, _executionHostId, meta) => meta) + + const result = handlers['worktrees:updateMeta'](null, { + worktreeId: 'repo-1::/workspace/feature-wt', + executionHostId: 'ssh:build-box', + updates: { comment: 'remote note' } + }) + + expect(store.setWorktreeMetaForHost).toHaveBeenCalledWith( + 'repo-1::/workspace/feature-wt', + 'ssh:build-box', + { comment: 'remote note' } + ) + expect(store.setWorktreeMeta).not.toHaveBeenCalled() + expect(result).toMatchObject({ comment: 'remote note' }) + }) + + it('rejects malformed execution-host identities at the IPC boundary', () => { + expect(() => + handlers['worktrees:updateMeta'](null, { + worktreeId: 'repo-1::/workspace/feature-wt', + executionHostId: 'container:untrusted', + updates: { comment: 'must not write' } + }) + ).toThrow('Invalid execution host identity.') + + expect(store.setWorktreeMetaForHost).not.toHaveBeenCalled() + expect(store.setWorktreeMeta).not.toHaveBeenCalled() + }) it('pushes a remote-client invalidation for renames but not read-state updates', () => { store.setWorktreeMeta.mockImplementation((_worktreeId, meta) => meta) diff --git a/src/main/ipc/worktrees-listing-fallback-rows.test.ts b/src/main/ipc/worktrees-listing-fallback-rows.test.ts index 45bea853e67..c33f9e14ef8 100644 --- a/src/main/ipc/worktrees-listing-fallback-rows.test.ts +++ b/src/main/ipc/worktrees-listing-fallback-rows.test.ts @@ -2,6 +2,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { listWorktreesMock, getSshGitProviderMock } from './worktrees-test-module-mocks' import { handlers, setupWorktreeHandlers, store } from './worktrees-test-harness' import { makeWorktreeMeta } from './worktrees-test-fixtures' +import { buildDetectedGitWorktrees } from './worktrees/listing/ssh-worktree-fallback' +import type { Store } from '../persistence/loading-store/store' vi.mock('electron', async () => (await import('./worktrees-test-module-mocks')).electronModuleMock() @@ -224,6 +226,41 @@ describe('registerWorktreeHandlers', () => { }) }) + it('lists a canonical-only SSH row without leaking the same-id local row', async () => { + const repo = { + id: 'repo-ssh', + path: '/remote/repo', + displayName: 'SSH Repo', + badgeColor: '#000', + addedAt: 0, + connectionId: 'conn-1' + } + const worktreeId = 'repo-ssh::/shared/feature-wt' + const canonicalRemote = makeWorktreeMeta({ + displayName: 'Canonical remote', + hostId: 'ssh:conn-1' + }) + store.getRepo.mockReturnValue(repo) + store.getAllWorktreeMeta.mockReturnValue({ + [worktreeId]: makeWorktreeMeta({ displayName: 'Legacy local', hostId: 'local' }) + }) + ;( + store as typeof store & { + getAllWorktreeMetaForHost: ReturnType + } + ).getAllWorktreeMetaForHost = vi.fn(() => ({ [worktreeId]: canonicalRemote })) + + const listed = await handlers['worktrees:list'](null, { repoId: repo.id }) + + expect(listed).toEqual([ + expect.objectContaining({ + id: worktreeId, + displayName: 'Canonical remote', + hostId: 'ssh:conn-1' + }) + ]) + }) + it('falls back to reconstructed SSH rows when provider listing throws', async () => { const repo = { id: 'repo-ssh', @@ -424,4 +461,31 @@ describe('registerWorktreeHandlers', () => { expect.objectContaining({ id: 'repo-ssh-b::/remote/b/two' }) ]) }) + + it('loads repo ownership once while building multiple detected rows', () => { + const owner = { + id: 'repo-1', + path: '/workspace/repo', + displayName: 'repo', + badgeColor: '#000', + addedAt: 0 + } + store.getRepos.mockReturnValue([owner]) + + const detected = buildDetectedGitWorktrees( + store as unknown as Store, + owner, + ['/workspace/one', '/workspace/two', '/workspace/three'].map((path) => ({ + path, + head: 'abc123', + branch: 'refs/heads/feature', + isBare: false, + isMainWorktree: false + })), + {} + ) + + expect(detected).toHaveLength(3) + expect(store.getRepos).toHaveBeenCalledTimes(1) + }) }) diff --git a/src/main/ipc/worktrees-test-harness.ts b/src/main/ipc/worktrees-test-harness.ts index 4e1c524fa67..0eacf52e472 100644 --- a/src/main/ipc/worktrees-test-harness.ts +++ b/src/main/ipc/worktrees-test-harness.ts @@ -73,6 +73,8 @@ export { /** Registers worktree IPC handlers against freshly reset shared mocks and returns the runtime stub. */ export function setupWorktreeHandlers(): WorktreeRuntimeStub { + delete (store as typeof store & { getAllWorktreeMetaForHost?: (...args: unknown[]) => unknown }) + .getAllWorktreeMetaForHost setPlatform(ORIGINAL_PLATFORM) clearConfiguredWorktreeSharedDirectoriesCacheForTests() __resetSshWorktreeCreateFetchCacheForTests() @@ -125,8 +127,10 @@ export function setupWorktreeHandlers(): WorktreeRuntimeStub { store.getSparsePresets, store.getSettings, store.getWorktreeMeta, + store.getWorktreeMetaForHost, store.getAllWorktreeMeta, store.setWorktreeMeta, + store.setWorktreeMetaForHost, store.getProjectHostSetups, store.removeWorktreeMeta, store.removeWorkspaceSessionStateForWorktree, @@ -190,10 +194,18 @@ export function setupWorktreeHandlers(): WorktreeRuntimeStub { workspaceDir: '/workspace' }) store.getWorktreeMeta.mockReturnValue(undefined) + // Host-qualified accessors delegate by default so payload assertions stay on one spy; + // host routing itself is covered by worktree-identity-persistence.test.ts. + store.getWorktreeMetaForHost.mockImplementation((...args: unknown[]) => + store.getWorktreeMeta(args[0] as string) + ) store.getAllWorktreeMeta.mockReturnValue({}) store.getRetiredWorktreeNameRegistry.mockReturnValue({ exhaustedTiers: 0, names: [] }) resetRetirementCollisionKeyCacheForTests() store.setWorktreeMeta.mockReturnValue({}) + store.setWorktreeMetaForHost.mockImplementation((...args: unknown[]) => + store.setWorktreeMeta(args[0] as string, args[2] as object) + ) store.getProjectHostSetups.mockReturnValue([ { id: 'repo-1', diff --git a/src/main/ipc/worktrees-test-ipc-surface.ts b/src/main/ipc/worktrees-test-ipc-surface.ts index f8d70bfbbbc..cc4b497b1bc 100644 --- a/src/main/ipc/worktrees-test-ipc-surface.ts +++ b/src/main/ipc/worktrees-test-ipc-surface.ts @@ -22,8 +22,10 @@ export type TestStore = { getSparsePresets: StoreMock getSettings: StoreMock getWorktreeMeta: KeyedStoreMock + getWorktreeMetaForHost: StoreMock getAllWorktreeMeta: StoreMock setWorktreeMeta: KeyedStoreWriteMock + setWorktreeMetaForHost: StoreMock getProjectHostSetups: StoreMock removeWorktreeMeta: KeyedStoreMock removeWorkspaceSessionStateForWorktree: KeyedStoreMock @@ -54,8 +56,10 @@ export const store: TestStore = { getSparsePresets: vi.fn(), getSettings: vi.fn(), getWorktreeMeta: vi.fn(), + getWorktreeMetaForHost: vi.fn(), getAllWorktreeMeta: vi.fn(), setWorktreeMeta: vi.fn(), + setWorktreeMetaForHost: vi.fn(), getProjectHostSetups: vi.fn(), removeWorktreeMeta: vi.fn(), removeWorkspaceSessionStateForWorktree: vi.fn(), diff --git a/src/main/ipc/worktrees-windows.test.ts b/src/main/ipc/worktrees-windows.test.ts index d4b3dde71dc..299c3690a1a 100644 --- a/src/main/ipc/worktrees-windows.test.ts +++ b/src/main/ipc/worktrees-windows.test.ts @@ -165,6 +165,7 @@ describe('registerWorktreeHandlers – Windows path handling', () => { getProjectHostSetups: vi.fn(), getSettings: vi.fn(), getWorktreeMeta: vi.fn(), + getAllWorktreeMeta: vi.fn(), setWorktreeMeta: vi.fn(), removeWorktreeMeta: vi.fn(), addRetiredWorktreeName: vi.fn(), @@ -209,6 +210,7 @@ describe('registerWorktreeHandlers – Windows path handling', () => { store.getProjectHostSetups.mockReset() store.getSettings.mockReset() store.getWorktreeMeta.mockReset() + store.getAllWorktreeMeta.mockReset() store.setWorktreeMeta.mockReset() store.removeWorktreeMeta.mockReset() store.addRetiredWorktreeName.mockReset() @@ -251,6 +253,7 @@ describe('registerWorktreeHandlers – Windows path handling', () => { }) resolveSetupRunnerShellMock.mockReturnValue(undefined) store.getWorktreeMeta.mockReturnValue(undefined) + store.getAllWorktreeMeta.mockReturnValue({}) store.getRetiredWorktreeNameRegistry.mockReturnValue({ exhaustedTiers: 0, names: [] }) store.setWorktreeMeta.mockReturnValue({}) resolveLocalGitUsernameMock.mockResolvedValue('') @@ -501,6 +504,14 @@ describe('registerWorktreeHandlers – Windows path handling', () => { } : undefined ) + store.getAllWorktreeMeta.mockReturnValue({ + 'repo-1::C:/workspaces/improve-dashboard': { + lastActivityAt: 123, + displayName: 'Improve Dashboard', + linkedIssue: 123, + linkedPR: 456 + } + }) await handlers['worktrees:create'](null, { repoId: 'repo-1', diff --git a/src/main/ipc/worktrees/create/folder-workspace-creation.ts b/src/main/ipc/worktrees/create/folder-workspace-creation.ts index 9a1bc65fc02..0d50cb2e565 100644 --- a/src/main/ipc/worktrees/create/folder-workspace-creation.ts +++ b/src/main/ipc/worktrees/create/folder-workspace-creation.ts @@ -1,6 +1,6 @@ import { randomUUID } from 'node:crypto' import type { Repo } from '../../../../shared/repo-types' -import { getProjectHostSetupWorktreeMeta } from '../../../../shared/project-host-setup-projection' +import { getProjectHostSetupWorktreeMeta } from '../../../../shared/project-host-setup-lookup' import type { CreateWorktreeResult } from '../../../../shared/worktree/create-types' import type { Store } from '../../../persistence/loading-store/store' import type { CreateWorktreeArgsWithSystemProvenance } from '../ipc-context-schemas' diff --git a/src/main/ipc/worktrees/listing/detected-provider-listing.ts b/src/main/ipc/worktrees/listing/detected-provider-listing.ts index c3692d5c926..d855e582161 100644 --- a/src/main/ipc/worktrees/listing/detected-provider-listing.ts +++ b/src/main/ipc/worktrees/listing/detected-provider-listing.ts @@ -24,6 +24,8 @@ import { rememberLocalWorktreeRoots } from './detected-worktree-scan-cache' import { loggedWorktreeListFailures, warnOnce } from './worktree-listing-diagnostics' +import { readAllWorktreeMetaForHost } from '../../../persistence/host-qualified-worktree-meta' +import { getRepoExecutionHostId } from '../../../../shared/execution-host' export async function listDetectedWorktreesForCapturedRepo( store: Store, @@ -36,8 +38,11 @@ export async function listDetectedWorktreesForCapturedRepo( providerAbort?.signal.aborted ? ({ providerAbortStatus: providerAbort.status() } as const) : undefined + const allMeta = isFolderRepo(repo) + ? undefined + : readAllWorktreeMetaForHost(store, getRepoExecutionHostId(repo)) const sshWorktreeMetaIndex = repo.connectionId - ? createSshWorktreeMetaIndex(Object.entries(store.getAllWorktreeMeta())) + ? createSshWorktreeMetaIndex(Object.entries(allMeta ?? {})) : new Map() try { @@ -118,7 +123,7 @@ export async function listDetectedWorktreesForCapturedRepo( repoId: repo.id, authoritative: true, source: 'git', - worktrees: buildDetectedGitWorktrees(store, repo, gitWorktrees) + worktrees: buildDetectedGitWorktrees(store, repo, gitWorktrees, allMeta) } } catch (err) { const aborted = abortedResult() diff --git a/src/main/ipc/worktrees/listing/register-host-catalog-handlers.ts b/src/main/ipc/worktrees/listing/register-host-catalog-handlers.ts index 50ce6082a42..0d47f3638c2 100644 --- a/src/main/ipc/worktrees/listing/register-host-catalog-handlers.ts +++ b/src/main/ipc/worktrees/listing/register-host-catalog-handlers.ts @@ -23,6 +23,7 @@ import { listDisconnectedSshWorktrees } from './ssh-worktree-fallback' import type { WorktreeIpcContext } from '../worktree-ipc-context' +import { readAllWorktreeMetaForHost } from '../../../persistence/host-qualified-worktree-meta' export function registerHostCatalogHandlers(context: WorktreeIpcContext): void { const { store } = context @@ -76,7 +77,10 @@ export function registerHostCatalogHandlers(context: WorktreeIpcContext): void { ) ) } - const metaIndex = createSshWorktreeMetaIndexForRepo(store.getAllWorktreeMeta(), repo.id) + const metaIndex = createSshWorktreeMetaIndexForRepo( + readAllWorktreeMetaForHost(store, requestedExecutionHostId), + repo.id + ) return complete( buildDisconnectedDetectedWorktrees( store, @@ -111,7 +115,7 @@ export function registerHostCatalogHandlers(context: WorktreeIpcContext): void { if (isFolderRepo(repo)) { return nothingForgotten } - const allMeta = store.getAllWorktreeMeta() + const allMeta = readAllWorktreeMetaForHost(store, requestedExecutionHostId) const forgottenWorktreeIds: string[] = [] for (const worktreeId of worktreeIds) { const meta = typeof worktreeId === 'string' ? allMeta[worktreeId] : undefined diff --git a/src/main/ipc/worktrees/listing/register-worktree-catalog-handlers.ts b/src/main/ipc/worktrees/listing/register-worktree-catalog-handlers.ts index 64c28c08495..f2388b3ce43 100644 --- a/src/main/ipc/worktrees/listing/register-worktree-catalog-handlers.ts +++ b/src/main/ipc/worktrees/listing/register-worktree-catalog-handlers.ts @@ -1,5 +1,6 @@ import { ipcMain } from 'electron' import { isFolderRepo } from '../../../../shared/repo-kind' +import { getRepoExecutionHostId, type ExecutionHostId } from '../../../../shared/execution-host' import { getSshGitProvider } from '../../../providers/ssh-git-dispatch' import { pruneLineageForMissingRepoWorktrees } from '../../../worktree-lineage-pruning' import { EMPTY_RETIRED_NAME_REGISTRY } from '../../../../shared/worktree/retired-name-registry' @@ -21,6 +22,8 @@ import { warnOnce } from './worktree-listing-diagnostics' import type { WorktreeIpcContext } from '../worktree-ipc-context' +import { readAllWorktreeMetaForHost } from '../../../persistence/host-qualified-worktree-meta' +import type { WorktreeMeta } from '../../../../shared/worktree/meta-types' const WORKTREE_LIST_ALL_CONCURRENCY = 8 @@ -49,9 +52,36 @@ export function registerWorktreeCatalogHandlers(context: WorktreeIpcContext): vo ipcMain.handle('worktrees:listAll', async () => { const repos = store.getRepos() - const sshWorktreeMetaIndex = repos.some((repo) => repo.connectionId) - ? createSshWorktreeMetaIndex(Object.entries(store.getAllWorktreeMeta())) - : new Map() + const legacyMetadata = + typeof store.getAllWorktreeMetaForHost === 'function' ? undefined : store.getAllWorktreeMeta() + const metadataByHost = new Map>() + const metadataForRepo = (repo: (typeof repos)[number]): Record => { + const hostId = getRepoExecutionHostId(repo) + const cached = metadataByHost.get(hostId) + if (cached) { + return cached + } + const metadata = + typeof store.getAllWorktreeMetaForHost === 'function' + ? store.getAllWorktreeMetaForHost(hostId) + : readAllWorktreeMetaForHost({ getAllWorktreeMeta: () => legacyMetadata ?? {} }, hostId) + metadataByHost.set(hostId, metadata) + return metadata + } + const sshMetaIndexByHost = new Map< + ExecutionHostId, + ReturnType + >() + const sshMetaIndexForRepo = (repo: (typeof repos)[number]) => { + const hostId = getRepoExecutionHostId(repo) + const cached = sshMetaIndexByHost.get(hostId) + if (cached) { + return cached + } + const index = createSshWorktreeMetaIndex(Object.entries(metadataForRepo(repo))) + sshMetaIndexByHost.set(hostId, index) + return index + } // Why: each local repo listing can spawn `git worktree list`; cap fan-out so large fleets don't start unbounded subprocesses. const results = await mapWithConcurrency(repos, WORKTREE_LIST_ALL_CONCURRENCY, async (repo) => { @@ -68,7 +98,7 @@ export function registerWorktreeCatalogHandlers(context: WorktreeIpcContext): vo `${repo.connectionId}:${repo.id}`, `[worktrees] SSH git provider unavailable; skipping worktree list for repo "${repo.displayName}" (${repo.id}) at ${repo.path} on connection ${repo.connectionId}` ) - return listDisconnectedSshWorktrees(store, repo, sshWorktreeMetaIndex) + return listDisconnectedSshWorktrees(store, repo, sshMetaIndexForRepo(repo)) } loggedUnavailableSshGitProviders.delete(`${repo.connectionId}:${repo.id}`) try { @@ -80,7 +110,7 @@ export function registerWorktreeCatalogHandlers(context: WorktreeIpcContext): vo `[worktrees] failed to list worktrees for repo "${repo.displayName}" (${repo.id}) at ${repo.path}`, err ) - return listDisconnectedSshWorktrees(store, repo, sshWorktreeMetaIndex) + return listDisconnectedSshWorktrees(store, repo, sshMetaIndexForRepo(repo)) } } else { const scan = await listDetectedGitWorktrees(store, repo) @@ -92,9 +122,10 @@ export function registerWorktreeCatalogHandlers(context: WorktreeIpcContext): vo pruneLineageForMissingRepoWorktrees(store, repo, gitWorktrees) } loggedWorktreeListFailures.delete(`${repo.id}:${repo.path}`) - return buildDetectedGitWorktrees(store, repo, gitWorktrees) + const metadata = metadataForRepo(repo) + return buildDetectedGitWorktrees(store, repo, gitWorktrees, metadata) .filter((worktree) => worktree.visible) - .map((worktree) => stampAndMergeVisibleDetectedWorktree(store, repo, worktree)) + .map((worktree) => stampAndMergeVisibleDetectedWorktree(store, repo, worktree, metadata)) } catch (err) { warnOnce( loggedWorktreeListFailures, @@ -123,8 +154,11 @@ export function registerWorktreeCatalogHandlers(context: WorktreeIpcContext): vo if (!repo) { return [] } + const allMeta = repo.connectionId + ? readAllWorktreeMetaForHost(store, getRepoExecutionHostId(repo)) + : undefined const sshWorktreeMetaIndex = repo.connectionId - ? createSshWorktreeMetaIndex(Object.entries(store.getAllWorktreeMeta())) + ? createSshWorktreeMetaIndex(Object.entries(allMeta ?? {})) : new Map() try { @@ -164,9 +198,10 @@ export function registerWorktreeCatalogHandlers(context: WorktreeIpcContext): vo pruneLineageForMissingRepoWorktrees(store, repo, gitWorktrees) } loggedWorktreeListFailures.delete(`${repo.id}:${repo.path}`) - return buildDetectedGitWorktrees(store, repo, gitWorktrees) + const metadata = allMeta ?? readAllWorktreeMetaForHost(store, getRepoExecutionHostId(repo)) + return buildDetectedGitWorktrees(store, repo, gitWorktrees, metadata) .filter((worktree) => worktree.visible) - .map((worktree) => stampAndMergeVisibleDetectedWorktree(store, repo, worktree)) + .map((worktree) => stampAndMergeVisibleDetectedWorktree(store, repo, worktree, metadata)) } catch (err) { warnOnce( loggedWorktreeListFailures, diff --git a/src/main/ipc/worktrees/listing/ssh-worktree-fallback.ts b/src/main/ipc/worktrees/listing/ssh-worktree-fallback.ts index 26fb14f2341..7ec2cac1fc9 100644 --- a/src/main/ipc/worktrees/listing/ssh-worktree-fallback.ts +++ b/src/main/ipc/worktrees/listing/ssh-worktree-fallback.ts @@ -8,6 +8,11 @@ import type { Repo } from '../../../../shared/repo-types' import type { GitWorktreeInfo, DetectedWorktree, Worktree } from '../../../../shared/worktree/types' import type { Store } from '../../../persistence/loading-store/store' import { getRepoExecutionHostId } from '../../../../shared/execution-host' +import { + readWorktreeMetaForHost, + writeWorktreeMetaForHost +} from '../../../persistence/host-qualified-worktree-meta' +import { getRepoOwnedWorktreeMeta } from '../../../worktree-metadata-ownership' import { buildKnownOrcaWorkspaceLayouts, isLegacyRepoForExternalWorktreeVisibility, @@ -110,7 +115,7 @@ export function listDisconnectedSshWorktrees( ? { ...candidate.meta, ...ownershipUpdates } : candidate.meta if (Object.keys(ownershipUpdates).length > 0) { - store.setWorktreeMeta(candidate.id, ownershipUpdates) + writeWorktreeMetaForHost(store, candidate.id, expectedHostId, ownershipUpdates) } // Why: synthesized rows carry no branch, so the title would fall through to the DESKTOP's basename() // applied to a REMOTE path — a Windows remote then renders its whole C:\... path as the name. Rows must @@ -131,7 +136,8 @@ export function listDisconnectedSshWorktrees( export function buildDetectedGitWorktrees( store: Store, repo: Repo, - gitWorktrees: GitWorktreeInfo[] + gitWorktrees: GitWorktreeInfo[], + allMetaOverride?: Record ): DetectedWorktree[] { const settings = store.getSettings() const knownOrcaLayouts = buildKnownOrcaWorkspaceLayouts(settings, repo) @@ -145,9 +151,15 @@ export function buildDetectedGitWorktrees( resolveCustomWorktreeVisibilitySources(repo, settings.worktreeVisibilityDefaults), resolveConfiguredWorktreeBasePaths(repo) ) + const allMeta = allMetaOverride ?? store.getAllWorktreeMeta?.() + const repoOwnerCount = store.getRepos().filter((candidate) => candidate.id === repo.id).length const detected = liveWorktrees.map((gitWorktree) => { const worktreeId = `${repo.id}::${gitWorktree.path}` - let meta = store.getWorktreeMeta(worktreeId) + const legacyMeta = store.getWorktreeMeta?.(worktreeId) + const metaById = allMeta ?? (legacyMeta ? { [worktreeId]: legacyMeta } : {}) + let meta = + readWorktreeMetaForHost(store, worktreeId, getRepoExecutionHostId(repo)) ?? + getRepoOwnedWorktreeMeta(repo, worktreeId, metaById, repoOwnerCount) const worktree = mergeWorktree(repo.id, gitWorktree, meta, repo.displayName) const detected = toDetectedWorktree({ repo, @@ -162,7 +174,13 @@ export function buildDetectedGitWorktrees( return detected } - meta = resolveWorktreeMetaWithDiscoveryBackfill(store, repo, worktreeId) + meta = resolveWorktreeMetaWithDiscoveryBackfill( + store, + repo, + worktreeId, + allMeta, + repoOwnerCount + ) return toDetectedWorktree({ repo, worktree: mergeWorktree(repo.id, gitWorktree, meta, repo.displayName), @@ -179,8 +197,9 @@ export function buildDetectedGitWorktrees( export function stampAndMergeVisibleDetectedWorktree( store: Store, repo: Repo, - detected: DetectedWorktree + detected: DetectedWorktree, + allMetaOverride?: Record ) { - const meta = resolveWorktreeMetaWithDiscoveryBackfill(store, repo, detected.id) + const meta = resolveWorktreeMetaWithDiscoveryBackfill(store, repo, detected.id, allMetaOverride) return mergeWorktree(repo.id, detected, meta, repo.displayName) } diff --git a/src/main/ipc/worktrees/listing/worktree-discovery-metadata.ts b/src/main/ipc/worktrees/listing/worktree-discovery-metadata.ts index c96e8dc2978..6af4d45c3cd 100644 --- a/src/main/ipc/worktrees/listing/worktree-discovery-metadata.ts +++ b/src/main/ipc/worktrees/listing/worktree-discovery-metadata.ts @@ -1,7 +1,13 @@ import type { Store } from '../../../persistence/loading-store/store' import type { Repo } from '../../../../shared/repo-types' import type { WorktreeMeta } from '../../../../shared/worktree/meta-types' -import { getProjectHostSetupWorktreeMeta } from '../../../../shared/project-host-setup-projection' +import { getProjectHostSetupWorktreeMeta } from '../../../../shared/project-host-setup-lookup' +import { getRepoExecutionHostId } from '../../../../shared/execution-host' +import { + readWorktreeMetaForHost, + writeWorktreeMetaForHost +} from '../../../persistence/host-qualified-worktree-meta' +import { getRepoOwnedWorktreeMeta } from '../../../worktree-metadata-ownership' import { randomUUID } from 'node:crypto' export function getProjectHostSetupMetaUpdates( @@ -29,9 +35,21 @@ export function getProjectHostSetupMetaUpdates( export function resolveWorktreeMetaWithDiscoveryBackfill( store: Store, repo: Repo, - worktreeId: string + worktreeId: string, + allMetaOverride?: Record, + repoOwnerCount = store.getRepos().filter((candidate) => candidate.id === repo.id).length ): WorktreeMeta { - const existing = store.getWorktreeMeta(worktreeId) + const executionHostId = getRepoExecutionHostId(repo) + const legacyMeta = store.getWorktreeMeta?.(worktreeId) + const allMeta = allMetaOverride ?? store.getAllWorktreeMeta?.() + const existing = + readWorktreeMetaForHost(store, worktreeId, executionHostId) ?? + getRepoOwnedWorktreeMeta( + repo, + worktreeId, + allMeta ?? (legacyMeta ? { [worktreeId]: legacyMeta } : {}), + repoOwnerCount + ) const ownershipUpdates = getProjectHostSetupMetaUpdates(store, repo, existing) if (existing) { const updates = { @@ -40,11 +58,11 @@ export function resolveWorktreeMetaWithDiscoveryBackfill( } if (Object.keys(updates).length > 0) { // Why: pre-lineage profiles already have WorktreeMeta rows; backfill on discovery so upgraded workspaces get lineage and host routing. - return store.setWorktreeMeta(worktreeId, updates) + return writeWorktreeMetaForHost(store, worktreeId, executionHostId, updates) } return existing } - return store.setWorktreeMeta(worktreeId, { + return writeWorktreeMetaForHost(store, worktreeId, executionHostId, { lastActivityAt: Date.now(), ...ownershipUpdates }) diff --git a/src/main/ipc/worktrees/metadata/register-worktree-metadata-handlers.ts b/src/main/ipc/worktrees/metadata/register-worktree-metadata-handlers.ts index 97d175a70df..359bd87f67d 100644 --- a/src/main/ipc/worktrees/metadata/register-worktree-metadata-handlers.ts +++ b/src/main/ipc/worktrees/metadata/register-worktree-metadata-handlers.ts @@ -1,5 +1,6 @@ import { ipcMain } from 'electron' import type { WorktreeMeta } from '../../../../shared/worktree/meta-types' +import { parseExecutionHostId } from '../../../../shared/execution-host' import { stripOrcaProvenanceMetaUpdates } from '../../../worktree-removal-safety' import { getRepoIdFromWorktreeId } from '../../../../shared/worktree/id' import type { @@ -16,10 +17,23 @@ import type { WorktreeIpcContext } from '../worktree-ipc-context' export function registerWorktreeMetadataHandlers(context: WorktreeIpcContext): void { const { mainWindow, store, runtime } = context - ipcMain.handle( 'worktrees:updateMeta', - (_event, args: { worktreeId: string; updates: Partial }) => { + ( + _event, + args: { + worktreeId: string + executionHostId?: string + updates: Partial + } + ) => { + const executionHostId = + args.executionHostId === undefined + ? undefined + : parseExecutionHostId(args.executionHostId)?.id + if (args.executionHostId !== undefined && !executionHostId) { + throw new Error('Invalid execution host identity.') + } const validatedUpdates = normalizeLinkedWorkItemFields(args.updates) const updates = validatedUpdates.displayName !== undefined @@ -29,7 +43,10 @@ export function registerWorktreeMetadataHandlers(context: WorktreeIpcContext): v firstAgentMessageRenameError: null } : validatedUpdates - const meta = store.setWorktreeMeta(args.worktreeId, stripOrcaProvenanceMetaUpdates(updates)) + const sanitizedUpdates = stripOrcaProvenanceMetaUpdates(updates) + const meta = executionHostId + ? store.setWorktreeMetaForHost(args.worktreeId, executionHostId, sanitizedUpdates) + : store.setWorktreeMeta(args.worktreeId, sanitizedUpdates) // Do NOT notify here: renderer already applied this optimistically; a notification would re-sort the sidebar (bug PR #209). if (args.updates.displayName !== undefined) { // Why: remote clients have no optimistic rename and stopped polling titles, so push a remote-only invalidation; gate on displayName so per-click isUnread updates stay event-free. diff --git a/src/main/persistence-repo-lifecycle.test.ts b/src/main/persistence-repo-lifecycle.test.ts index e27287c147e..53b759a7d01 100644 --- a/src/main/persistence-repo-lifecycle.test.ts +++ b/src/main/persistence-repo-lifecycle.test.ts @@ -395,6 +395,12 @@ describe('Store', () => { // Local worktree meta survives; the SSH host's meta is pruned. expect(store.getWorktreeMeta('shared::/local/repo/wt')).toBeDefined() expect(store.getWorktreeMeta('shared::/remote/repo/wt')).toBeUndefined() + store.flush() + const persisted = readDataFile() as PersistedState + const identityMetadata = Object.values(persisted.worktreeMetaByIdentity ?? {}) + expect(identityMetadata).toEqual([ + expect.objectContaining({ hostId: 'local', displayName: 'local-wt' }) + ]) }) it('removeProjectForHost keeps the surviving host session for a shared repo id + path', async () => { diff --git a/src/main/persistence/host-qualified-worktree-meta.ts b/src/main/persistence/host-qualified-worktree-meta.ts new file mode 100644 index 00000000000..a9d1c0e8fb1 --- /dev/null +++ b/src/main/persistence/host-qualified-worktree-meta.ts @@ -0,0 +1,65 @@ +import type { ExecutionHostId } from '../../shared/execution-host' +import type { WorktreeMeta } from '../../shared/worktree/meta-types' + +/** + * The listing paths take partial store shapes (`Pick`), so the + * host-qualified accessors are optional here and fall back to the locator-keyed + * ones. A real Store always has them. + */ +export type HostQualifiedWorktreeMetaStore = { + getAllWorktreeMeta: () => Record + getWorktreeMetaForHost?: ( + worktreeId: string, + executionHostId: ExecutionHostId + ) => WorktreeMeta | undefined + getAllWorktreeMetaForHost?: (executionHostId: ExecutionHostId) => Record + setWorktreeMeta: (worktreeId: string, meta: Partial) => WorktreeMeta + setWorktreeMetaForHost?: ( + worktreeId: string, + executionHostId: ExecutionHostId, + meta: Partial + ) => WorktreeMeta +} + +/** Canonical host snapshot, with a filtered legacy fallback for partial test/compatibility stores. */ +export function readAllWorktreeMetaForHost( + store: Pick, + executionHostId: ExecutionHostId +): Record { + const qualified = store.getAllWorktreeMetaForHost?.(executionHostId) + if (qualified) { + return qualified + } + const projected: Record = {} + for (const [worktreeId, meta] of Object.entries(store.getAllWorktreeMeta())) { + if (!meta.hostId || meta.hostId === executionHostId) { + projected[worktreeId] = meta + } + } + return projected +} + +/** + * The row this host owns at `worktreeId`, or undefined. Deliberately does NOT fall back to the + * locator-keyed read: that would hand one host's metadata to another's row, which is the collision + * every caller here is trying to avoid. Callers keep their own same-id ownership guard as fallback. + */ +export function readWorktreeMetaForHost( + store: Pick, + worktreeId: string, + executionHostId: ExecutionHostId +): WorktreeMeta | undefined { + return store.getWorktreeMetaForHost?.(worktreeId, executionHostId) +} + +export function writeWorktreeMetaForHost( + store: Pick, + worktreeId: string, + executionHostId: ExecutionHostId, + meta: Partial +): WorktreeMeta { + return ( + store.setWorktreeMetaForHost?.(worktreeId, executionHostId, meta) ?? + store.setWorktreeMeta(worktreeId, { ...meta, hostId: executionHostId }) + ) +} diff --git a/src/main/persistence/leasing-ssh-ptys/canonical-worktree-host-reassignment.ts b/src/main/persistence/leasing-ssh-ptys/canonical-worktree-host-reassignment.ts new file mode 100644 index 00000000000..f9d3228d924 --- /dev/null +++ b/src/main/persistence/leasing-ssh-ptys/canonical-worktree-host-reassignment.ts @@ -0,0 +1,128 @@ +import { randomUUID } from 'node:crypto' +import { isDeepStrictEqual } from 'node:util' +import type { ExecutionHostId } from '../../../shared/execution-host' +import type { PersistedState } from '../../../shared/persisted-state-types' +import type { WorktreeMeta } from '../../../shared/worktree/meta-types' +import { canonicalWorktreeIdentity } from '../../../shared/worktree/identity' +import { + composeWorktreeHostIdentity, + getExecutionHostIdFromWorktreeHostIdentity, + getWorktreeIdFromHostIdentity +} from '../../../shared/worktree/host-qualified-identity' + +type IdentityMove = { + sourceKey: string + nextKey: string + instanceId: string + sourceMeta: WorktreeMeta + nextMeta: WorktreeMeta +} + +type AliasMove = { + oldAlias: string + nextAlias: string + identities: IdentityMove[] +} + +/** Move canonical aliases only after every destination occupant is proven compatible. */ +export function reassignCanonicalWorktreeMetadataHost( + state: PersistedState, + oldHostId: ExecutionHostId, + newHostId: ExecutionHostId +): { changed: boolean; preservedWorktreeIds: ReadonlySet } { + const aliases = state.worktreeIdentityAliases ?? {} + const identityMeta = state.worktreeMetaByIdentity ?? {} + const repairedInstanceIds = new Map() + const plans: AliasMove[] = [] + + for (const [oldAlias, rawKeys] of Object.entries(aliases)) { + if (getExecutionHostIdFromWorktreeHostIdentity(oldAlias) !== oldHostId) { + continue + } + const worktreeId = getWorktreeIdFromHostIdentity(oldAlias) + if (!worktreeId || rawKeys.length === 0) { + continue + } + const identities: IdentityMove[] = [] + let safe = true + for (const sourceKey of new Set(rawKeys)) { + const sourceMeta = identityMeta[sourceKey] + if (!sourceMeta) { + safe = false + break + } + let instanceId = sourceMeta.instanceId ?? repairedInstanceIds.get(sourceKey) + if (instanceId === undefined) { + instanceId = randomUUID() + repairedInstanceIds.set(sourceKey, instanceId) + } + const nextKey = canonicalWorktreeIdentity({ + worktreeId, + executionHostId: newHostId, + instanceId + }) + const nextMeta = { ...sourceMeta, instanceId, hostId: newHostId } + const duplicate = identities.find((identity) => identity.nextKey === nextKey) + if (duplicate && !isDeepStrictEqual(duplicate.nextMeta, nextMeta)) { + safe = false + break + } + if (!duplicate) { + identities.push({ sourceKey, nextKey, instanceId, sourceMeta, nextMeta }) + } + } + if (!safe) { + continue + } + + const nextAlias = composeWorktreeHostIdentity(newHostId, worktreeId) + const candidateKeys = new Set(identities.map((identity) => identity.nextKey)) + if ((aliases[nextAlias] ?? []).some((key) => !candidateKeys.has(key))) { + continue + } + if ( + identities.some((identity) => { + const existing = identityMeta[identity.nextKey] + return existing !== undefined && !isDeepStrictEqual(existing, identity.nextMeta) + }) + ) { + continue + } + plans.push({ oldAlias, nextAlias, identities }) + } + + if (plans.length > 0) { + state.worktreeIdentityAliases ??= {} + state.worktreeMetaByIdentity ??= {} + for (const plan of plans) { + for (const identity of plan.identities) { + identity.sourceMeta.instanceId ??= identity.instanceId + state.worktreeMetaByIdentity[identity.nextKey] ??= identity.nextMeta + } + state.worktreeIdentityAliases[plan.nextAlias] = [ + ...new Set([ + ...(state.worktreeIdentityAliases[plan.nextAlias] ?? []), + ...plan.identities.map((identity) => identity.nextKey) + ]) + ] + delete state.worktreeIdentityAliases[plan.oldAlias] + } + + const referenced = new Set(Object.values(state.worktreeIdentityAliases).flat()) + for (const sourceKey of new Set( + plans.flatMap((plan) => plan.identities.map((identity) => identity.sourceKey)) + )) { + if (!referenced.has(sourceKey)) { + delete state.worktreeMetaByIdentity[sourceKey] + } + } + } + + const preservedWorktreeIds = new Set() + for (const alias of Object.keys(state.worktreeIdentityAliases ?? {})) { + if (getExecutionHostIdFromWorktreeHostIdentity(alias) === oldHostId) { + preservedWorktreeIds.add(getWorktreeIdFromHostIdentity(alias)) + } + } + return { changed: plans.length > 0, preservedWorktreeIds } +} diff --git a/src/main/persistence/leasing-ssh-ptys/ssh-target-reassignment.ts b/src/main/persistence/leasing-ssh-ptys/ssh-target-reassignment.ts index 919e4996486..c996dc445f2 100644 --- a/src/main/persistence/leasing-ssh-ptys/ssh-target-reassignment.ts +++ b/src/main/persistence/leasing-ssh-ptys/ssh-target-reassignment.ts @@ -17,6 +17,7 @@ import { migrateAutomationsForSshReadoption } from '../../automations/automation-ssh-readoption-migration' import { automationIdsPinnedToSshTarget } from '../scheduling-automations/automation-owner-projection' +import { reassignCanonicalWorktreeMetadataHost } from './canonical-worktree-host-reassignment' export type SshTargetReassignmentOperations = { state: PersistedState @@ -80,10 +81,16 @@ export function reassignSshTargetId( } repoIds.add(repo.id) } - // Re-point worktree metas whose hostId pointed at the old SSH host. + // Legacy locator rows cannot recover canonical-only metadata after target readoption. + const identityResult = reassignCanonicalWorktreeMetadataHost( + operations.state, + oldHostId, + newHostId + ) + // Re-point legacy rows unless a conflicting destination kept their canonical source in place. let metaChanged = false - for (const meta of Object.values(operations.state.worktreeMeta)) { - if (meta.hostId === oldHostId) { + for (const [worktreeId, meta] of Object.entries(operations.state.worktreeMeta)) { + if (meta.hostId === oldHostId && !identityResult.preservedWorktreeIds.has(worktreeId)) { meta.hostId = newHostId metaChanged = true } @@ -175,7 +182,13 @@ export function reassignSshTargetId( if (repoIds.size > 0 || setupsChanged) { operations.syncProjectHostSetupCompatibilityState() } - if (repoIds.size > 0 || metaChanged || carrierChanged || setupsChanged) { + if ( + repoIds.size > 0 || + metaChanged || + carrierChanged || + setupsChanged || + identityResult.changed + ) { // The rewrites above patch rows in place; the list projection caches on array // identity, so a same-identity array would keep serving pre-readoption owners. operations.state.repos = [...operations.state.repos] diff --git a/src/main/persistence/loading-store/metadata-lineage-operations.ts b/src/main/persistence/loading-store/metadata-lineage-operations.ts index 056991dd1f4..f12af5d1583 100644 --- a/src/main/persistence/loading-store/metadata-lineage-operations.ts +++ b/src/main/persistence/loading-store/metadata-lineage-operations.ts @@ -1,45 +1,14 @@ -import { randomUUID } from 'node:crypto' import type { WorkspaceKey } from '../../../shared/folder-workspace-types' import type { WorkspaceLineage, WorktreeLineage } from '../../../shared/worktree/lineage-types' import type { WorktreeMeta } from '../../../shared/worktree/meta-types' -import { normalizeStoredTaskSourceContext } from '../../../shared/task-source-context' -import { normalizeWorkspaceLinkedItem } from '../../../shared/workspace-linked-item' -import { isWorkspaceLinkedItemSourceContextMatch } from '../../../shared/workspace-linked-item-source-context' -import type { ExecutionHostId } from '../../../shared/execution-host' +import { LOCAL_EXECUTION_HOST_ID, type ExecutionHostId } from '../../../shared/execution-host' import { getRepoIdFromWorktreeId } from '../../../shared/worktree/id' import { hasWorktreeRemovalRepoOwnerOnOtherHost } from '../../worktree-removal-repo-owner' -import { DEFAULT_WORKSPACE_STATUS_ID } from '../../../shared/workspace-statuses' import { folderWorkspaceKey, worktreeWorkspaceKey } from '../../../shared/workspace-scope' import { workspaceSessionOwnerPartitionForHost, workspaceSessionPartitionIdsForHost } from '../restoring-sessions/session-owner-removal' -import { migrateWorktreeIdentity as migrateWorktreeIdentityOperation } from '../tracking-repos/worktree-identity-migration' - -function getDefaultWorktreeMeta(): WorktreeMeta { - return { - instanceId: randomUUID(), - displayName: '', - comment: '', - linkedIssue: null, - linkedPR: null, - linkedLinearIssue: null, - linkedGitLabMR: null, - linkedGitLabIssue: null, - linkedBitbucketPR: null, - linkedAzureDevOpsPR: null, - linkedGiteaPR: null, - linkedWorkItem: null, - linkedTaskSourceContext: null, - isArchived: false, - isUnread: false, - isPinned: false, - sortOrder: Date.now(), - lastActivityAt: 0, - workspaceStatus: DEFAULT_WORKSPACE_STATUS_ID - } -} - import type { StoreRuntimeState } from './store-runtime-state' import type { WriteSchedulingOperations } from './write-scheduling' import type { SessionHostPartitionOperations } from './session-host-partitions' @@ -50,6 +19,15 @@ import { partitionHasOtherRepoWorktreeTabs, removeWorkspaceSessionOwnerInPartition } from './session-host-partitions' +import { migrateWorktreeIdentity as migrateWorktreeIdentityOperation } from '../tracking-repos/worktree-identity-migration' +import { + getAllWorktreeMetaForHost as getAllWorktreeMetaForHostOperation, + getWorktreeMetaForHost as getWorktreeMetaForHostOperation, + migrateWorktreeMetadataLocator, + removeWorktreeMetadataForHost, + setWorktreeMetaForHost as setWorktreeMetaForHostOperation +} from './worktree-identity-metadata' +import { mergeWorktreeMetaForWrite } from './worktree-meta-write-normalization' type MetadataLineageOperationsRuntime = Pick @@ -70,34 +48,62 @@ export class MetadataLineageOperations { ) { this[metadataLineageOperationsContext] = { runtime, scheduling, sessions } } - getWorktreeMeta(worktreeId: string): WorktreeMeta | undefined { return this[metadataLineageOperationsContext].runtime.state.worktreeMeta[worktreeId] } + getWorktreeMetaForHost( + worktreeId: string, + executionHostId: ExecutionHostId + ): WorktreeMeta | undefined { + return getWorktreeMetaForHostOperation( + this[metadataLineageOperationsContext].runtime, + this[metadataLineageOperationsContext].scheduling, + worktreeId, + executionHostId + ) + } + + setWorktreeMetaForHost( + worktreeId: string, + executionHostId: ExecutionHostId, + meta: Partial + ): WorktreeMeta { + return setWorktreeMetaForHostOperation( + this[metadataLineageOperationsContext].runtime, + this[metadataLineageOperationsContext].scheduling, + worktreeId, + executionHostId, + meta + ) + } + getAllWorktreeMeta(): Record { return this[metadataLineageOperationsContext].runtime.state.worktreeMeta } + getAllWorktreeMetaForHost(executionHostId: ExecutionHostId): Record { + return getAllWorktreeMetaForHostOperation( + this[metadataLineageOperationsContext].runtime, + executionHostId + ) + } + setWorktreeMeta(worktreeId: string, meta: Partial): WorktreeMeta { - const existing = - this[metadataLineageOperationsContext].runtime.state.worktreeMeta[worktreeId] || - getDefaultWorktreeMeta() - const updated = { ...existing, ...meta } - updated.linkedWorkItem = normalizeWorkspaceLinkedItem(updated.linkedWorkItem) - const linkedTaskSourceContext = normalizeStoredTaskSourceContext( - updated.linkedTaskSourceContext - ) - updated.linkedTaskSourceContext = isWorkspaceLinkedItemSourceContextMatch( - updated.linkedWorkItem, - linkedTaskSourceContext - ) - ? linkedTaskSourceContext - : null - if (!updated.instanceId) { - updated.instanceId = randomUUID() + const state = this[metadataLineageOperationsContext].runtime.state + const stored = state.worktreeMeta[worktreeId] + const executionHostId = meta.hostId ?? stored?.hostId + if (executionHostId) { + return setWorktreeMetaForHostOperation( + this[metadataLineageOperationsContext].runtime, + this[metadataLineageOperationsContext].scheduling, + worktreeId, + executionHostId, + meta + ) } - this[metadataLineageOperationsContext].runtime.state.worktreeMeta[worktreeId] = updated + const updated = mergeWorktreeMetaForWrite(stored, meta) + state.worktreeMeta[worktreeId] = updated scheduleSave(this[metadataLineageOperationsContext].scheduling) return updated } @@ -120,6 +126,13 @@ export class MetadataLineageOperations { ownerPartition )) ) + // Mirror the legacy delete's scope below: a full removal takes every host's identity rows, + // otherwise the surviving owner keeps its own. + removeWorktreeMetadataForHost( + this[metadataLineageOperationsContext].runtime.state, + worktreeId, + hostId === undefined ? undefined : (hostId ?? undefined) + ) // Skip partitions main never wrote: materializing one fences every sibling worktree of the repo. const partitions = new Set( workspaceSessionPartitionIdsForHost(owner).filter( @@ -190,14 +203,27 @@ export class MetadataLineageOperations { scheduleSave(this[metadataLineageOperationsContext].scheduling) } - migrateWorktreeIdentity(oldWorktreeId: string, newWorktreeId: string): void { - if ( - migrateWorktreeIdentityOperation( - this[metadataLineageOperationsContext].runtime.state, - oldWorktreeId, - newWorktreeId - ) - ) { + migrateWorktreeIdentity( + oldWorktreeId: string, + newWorktreeId: string, + executionHostId?: ExecutionHostId + ): void { + const state = this[metadataLineageOperationsContext].runtime.state + const persistedOwner = state.worktreeMeta[oldWorktreeId]?.hostId + const mover = executionHostId ?? persistedOwner ?? LOCAL_EXECUTION_HOST_ID + const legacyChanged = + executionHostId !== undefined && + persistedOwner !== undefined && + persistedOwner !== executionHostId + ? false + : migrateWorktreeIdentityOperation(state, oldWorktreeId, newWorktreeId) + const canonicalChanged = migrateWorktreeMetadataLocator( + state, + oldWorktreeId, + newWorktreeId, + mover + ) + if (legacyChanged || canonicalChanged) { scheduleSave(this[metadataLineageOperationsContext].scheduling) } } diff --git a/src/main/persistence/loading-store/worktree-identity-metadata.ts b/src/main/persistence/loading-store/worktree-identity-metadata.ts new file mode 100644 index 00000000000..396c2ffae85 --- /dev/null +++ b/src/main/persistence/loading-store/worktree-identity-metadata.ts @@ -0,0 +1,236 @@ +import { randomUUID } from 'node:crypto' +import type { WorktreeMeta } from '../../../shared/worktree/meta-types' +import type { ExecutionHostId } from '../../../shared/execution-host' +import { canonicalWorktreeIdentity } from '../../../shared/worktree/identity' +import { + composeWorktreeHostIdentity, + getExecutionHostIdFromWorktreeHostIdentity, + getWorktreeIdFromHostIdentity +} from '../../../shared/worktree/host-qualified-identity' +import type { PersistedState } from '../../../shared/persisted-state-types' +import type { StoreRuntimeState } from './store-runtime-state' +import type { WriteSchedulingOperations } from './write-scheduling' +import { scheduleSave } from './write-scheduling' +import { mergeWorktreeMetaForWrite } from './worktree-meta-write-normalization' + +type MetadataRuntime = Pick + +/** Select one readable row without discarding competing alias candidates. */ +function resolveAliasIdentityKey(state: PersistedState, alias: string): string | undefined { + const identityKeys = state.worktreeIdentityAliases?.[alias] ?? [] + if (identityKeys.length === 0) { + return undefined + } + const resolvable = identityKeys.filter((key) => state.worktreeMetaByIdentity?.[key]) + const candidates = resolvable.length > 0 ? resolvable : identityKeys + // Newest activity wins, then the greater key, so every host agrees on the survivor. + const winner = candidates.reduce((best, key) => { + const bestTouch = state.worktreeMetaByIdentity?.[best]?.lastActivityAt ?? 0 + const keyTouch = state.worktreeMetaByIdentity?.[key]?.lastActivityAt ?? 0 + if (keyTouch !== bestTouch) { + return keyTouch > bestTouch ? key : best + } + return key > best ? key : best + }) + return winner +} + +/** Drop identity metadata no alias points at any more. */ +export function pruneUnreferencedWorktreeIdentityMeta(state: PersistedState): boolean { + const referenced = new Set(Object.values(state.worktreeIdentityAliases ?? {}).flat()) + let changed = false + for (const identityKey of Object.keys(state.worktreeMetaByIdentity ?? {})) { + if (!referenced.has(identityKey)) { + delete state.worktreeMetaByIdentity?.[identityKey] + changed = true + } + } + return changed +} + +function migrateLegacyWorktreeMetadata( + state: PersistedState, + worktreeId: string, + executionHostId: ExecutionHostId +): boolean { + const meta = state.worktreeMeta[worktreeId] + if (!meta || (meta.hostId !== undefined && meta.hostId !== executionHostId)) { + return false + } + state.worktreeMetaByIdentity ??= {} + state.worktreeIdentityAliases ??= {} + let changed = false + const instanceId = meta.instanceId ?? randomUUID() + if (!meta.instanceId) { + meta.instanceId = instanceId + changed = true + } + if (!meta.hostId) { + meta.hostId = executionHostId + changed = true + } + const identityKey = canonicalWorktreeIdentity({ + worktreeId, + executionHostId, + instanceId + }) + if (!state.worktreeMetaByIdentity[identityKey]) { + state.worktreeMetaByIdentity[identityKey] = { + ...meta, + instanceId, + hostId: executionHostId + } + changed = true + } + const alias = composeWorktreeHostIdentity(executionHostId, worktreeId) + const aliases = state.worktreeIdentityAliases[alias] ?? [] + if (aliases.length === 0) { + state.worktreeIdentityAliases[alias] = [identityKey] + changed = true + } + return changed +} + +/** Drop the identity rows a locator owns, for one host or for every host. */ +export function removeWorktreeMetadataForHost( + state: PersistedState, + worktreeId: string, + executionHostId: ExecutionHostId | undefined +): boolean { + let changed = false + for (const alias of Object.keys(state.worktreeIdentityAliases ?? {})) { + if (getWorktreeIdFromHostIdentity(alias) !== worktreeId) { + continue + } + if ( + executionHostId !== undefined && + getExecutionHostIdFromWorktreeHostIdentity(alias) !== executionHostId + ) { + continue + } + delete state.worktreeIdentityAliases?.[alias] + changed = true + } + return pruneUnreferencedWorktreeIdentityMeta(state) || changed +} + +/** + * Re-point one host's alias at a renamed locator. Host-scoped on purpose: a local + * folder move must not drag a remote host's alias to a path that host does not have. + */ +export function migrateWorktreeMetadataLocator( + state: PersistedState, + oldWorktreeId: string, + newWorktreeId: string, + executionHostId: ExecutionHostId +): boolean { + if (oldWorktreeId === newWorktreeId) { + return false + } + const oldAlias = composeWorktreeHostIdentity(executionHostId, oldWorktreeId) + const identityKeys = state.worktreeIdentityAliases?.[oldAlias] + if (!identityKeys || identityKeys.length === 0) { + return false + } + const newAlias = composeWorktreeHostIdentity(executionHostId, newWorktreeId) + state.worktreeIdentityAliases ??= {} + // A taken destination keeps its own occupant; stranding the mover at the old locator loses less + // than merging (which makes both unreadable) or dropping its row outright. + if ((state.worktreeIdentityAliases[newAlias] ?? []).length > 0) { + return false + } + state.worktreeIdentityAliases[newAlias] = [...identityKeys] + delete state.worktreeIdentityAliases[oldAlias] + return true +} +export function getWorktreeMetaForHost( + runtime: MetadataRuntime, + scheduling: WriteSchedulingOperations, + worktreeId: string, + executionHostId: ExecutionHostId +): WorktreeMeta | undefined { + const state = runtime.state + const changed = migrateLegacyWorktreeMetadata(state, worktreeId, executionHostId) + const alias = composeWorktreeHostIdentity(executionHostId, worktreeId) + const identityKey = resolveAliasIdentityKey(state, alias) + if (changed) { + scheduleSave(scheduling) + } + if (identityKey) { + return state.worktreeMetaByIdentity?.[identityKey] + } + const legacy = state.worktreeMeta[worktreeId] + return !legacy?.hostId || legacy.hostId === executionHostId ? legacy : undefined +} + +/** Read-only host projection used by listings that must include canonical-only rows. */ +export function getAllWorktreeMetaForHost( + runtime: MetadataRuntime, + executionHostId: ExecutionHostId +): Record { + const state = runtime.state + const projected: Record = {} + for (const [worktreeId, meta] of Object.entries(state.worktreeMeta)) { + if (!meta.hostId || meta.hostId === executionHostId) { + projected[worktreeId] = meta + } + } + for (const alias of Object.keys(state.worktreeIdentityAliases ?? {})) { + if (getExecutionHostIdFromWorktreeHostIdentity(alias) !== executionHostId) { + continue + } + const worktreeId = getWorktreeIdFromHostIdentity(alias) + const identityKey = resolveAliasIdentityKey(state, alias) + const meta = identityKey ? state.worktreeMetaByIdentity?.[identityKey] : undefined + if (!worktreeId || !meta || (meta.hostId && meta.hostId !== executionHostId)) { + continue + } + projected[worktreeId] = + meta.hostId === executionHostId ? meta : { ...meta, hostId: executionHostId } + } + return projected +} + +export function setWorktreeMetaForHost( + runtime: MetadataRuntime, + scheduling: WriteSchedulingOperations, + worktreeId: string, + executionHostId: ExecutionHostId, + meta: Partial +): WorktreeMeta { + const state = runtime.state + migrateLegacyWorktreeMetadata(state, worktreeId, executionHostId) + const alias = composeWorktreeHostIdentity(executionHostId, worktreeId) + const identityKeys = state.worktreeIdentityAliases?.[alias] ?? [] + if (identityKeys.length > 1 && meta.instanceId === undefined) { + throw new Error('Worktree identity is ambiguous for this host and locator.') + } + const existingIdentityKey = resolveAliasIdentityKey(state, alias) + const existingIdentityMeta = existingIdentityKey + ? state.worktreeMetaByIdentity?.[existingIdentityKey] + : undefined + const legacy = state.worktreeMeta[worktreeId] + const existing = + existingIdentityMeta ?? + (!legacy?.hostId || legacy.hostId === executionHostId ? legacy : undefined) + // An explicit instanceId is a deliberate rotation (see worktree-lineage-pruning), so it wins. + const instanceId = meta.instanceId ?? existing?.instanceId ?? randomUUID() + const identityKey = canonicalWorktreeIdentity({ worktreeId, executionHostId, instanceId }) + const updated = mergeWorktreeMetaForWrite(existing, meta, { + instanceId, + hostId: executionHostId + }) + state.worktreeMetaByIdentity ??= {} + state.worktreeIdentityAliases ??= {} + if (existingIdentityKey && existingIdentityKey !== identityKey) { + delete state.worktreeMetaByIdentity[existingIdentityKey] + } + state.worktreeMetaByIdentity[identityKey] = updated + state.worktreeIdentityAliases[alias] = [identityKey] + // Keep the legacy projection only for the first known owner. + if (!legacy || legacy.hostId === executionHostId) { + state.worktreeMeta[worktreeId] = updated + } + scheduleSave(scheduling) + return updated +} diff --git a/src/main/persistence/loading-store/worktree-meta-write-normalization.ts b/src/main/persistence/loading-store/worktree-meta-write-normalization.ts new file mode 100644 index 00000000000..e2b8f338bdd --- /dev/null +++ b/src/main/persistence/loading-store/worktree-meta-write-normalization.ts @@ -0,0 +1,55 @@ +import { randomUUID } from 'node:crypto' +import type { ExecutionHostId } from '../../../shared/execution-host' +import { normalizeStoredTaskSourceContext } from '../../../shared/task-source-context' +import { normalizeWorkspaceLinkedItem } from '../../../shared/workspace-linked-item' +import { isWorkspaceLinkedItemSourceContextMatch } from '../../../shared/workspace-linked-item-source-context' +import { DEFAULT_WORKSPACE_STATUS_ID } from '../../../shared/workspace-statuses' +import type { WorktreeMeta } from '../../../shared/worktree/meta-types' + +type WorktreeMetaIdentity = { + instanceId: string + hostId: ExecutionHostId +} + +function createDefaultWorktreeMeta(): WorktreeMeta { + return { + instanceId: randomUUID(), + displayName: '', + comment: '', + linkedIssue: null, + linkedPR: null, + linkedLinearIssue: null, + linkedGitLabMR: null, + linkedGitLabIssue: null, + linkedBitbucketPR: null, + linkedAzureDevOpsPR: null, + linkedGiteaPR: null, + linkedWorkItem: null, + linkedTaskSourceContext: null, + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: Date.now(), + lastActivityAt: 0, + workspaceStatus: DEFAULT_WORKSPACE_STATUS_ID + } +} + +/** Merge and normalize the metadata shape shared by legacy and identity-keyed writes. */ +export function mergeWorktreeMetaForWrite( + existing: WorktreeMeta | undefined, + updates: Partial, + identity?: WorktreeMetaIdentity +): WorktreeMeta { + const updated = { ...(existing ?? createDefaultWorktreeMeta()), ...updates, ...identity } + updated.linkedWorkItem = normalizeWorkspaceLinkedItem(updated.linkedWorkItem) + const sourceContext = normalizeStoredTaskSourceContext(updated.linkedTaskSourceContext) + updated.linkedTaskSourceContext = isWorkspaceLinkedItemSourceContextMatch( + updated.linkedWorkItem, + sourceContext + ) + ? sourceContext + : null + updated.instanceId ||= randomUUID() + return updated +} diff --git a/src/main/persistence/tracking-repos/repo-worktree-pruning.ts b/src/main/persistence/tracking-repos/repo-worktree-pruning.ts index 7032ef2ed60..f6d17b24aa2 100644 --- a/src/main/persistence/tracking-repos/repo-worktree-pruning.ts +++ b/src/main/persistence/tracking-repos/repo-worktree-pruning.ts @@ -4,9 +4,11 @@ import { parseWorkspaceKey } from '../../../shared/workspace-scope' import type { PersistedState } from '../../../shared/persisted-state-types' import { removeWorkspaceSessionOwners } from '../restoring-sessions/session-owner-removal' import { + getExecutionHostIdFromWorktreeHostIdentity, getWorktreeIdFromHostIdentity, isWorktreeHostIdentity } from '../../../shared/worktree/host-qualified-identity' +import { pruneUnreferencedWorktreeIdentityMeta } from '../loading-store/worktree-identity-metadata' export function pruneWorktreeStateForRepo( state: PersistedState, @@ -67,6 +69,14 @@ export function pruneWorktreeStateForRepo( delete state.worktreeMeta[key] } } + for (const alias of Object.keys(state.worktreeIdentityAliases ?? {})) { + const rawId = getWorktreeIdFromHostIdentity(alias) + const aliasHost = getExecutionHostIdFromWorktreeHostIdentity(alias) + if (rawId.startsWith(prefix) && (hostId === null || aliasHost === hostId)) { + delete state.worktreeIdentityAliases?.[alias] + } + } + pruneUnreferencedWorktreeIdentityMeta(state) // Why: a host-scoped prune must touch only that host's session (legacy blob = local, one partition // per remote host); pruning every partition would wipe a surviving host's tabs and sleeping agents // for a shared repo id/path. A full removal (hostId === null) still clears every host. diff --git a/src/main/persistence/tracking-repos/worktree-metadata-normalization.test.ts b/src/main/persistence/tracking-repos/worktree-metadata-normalization.test.ts index 5886ebb7196..2bb4fe2eec9 100644 --- a/src/main/persistence/tracking-repos/worktree-metadata-normalization.test.ts +++ b/src/main/persistence/tracking-repos/worktree-metadata-normalization.test.ts @@ -1,7 +1,11 @@ import { describe, expect, it } from 'vitest' import type { WorktreeMeta } from '../../../shared/worktree/meta-types' import type { PersistedState } from '../../../shared/persisted-state-types' -import { normalizeWorktreeLinkedItemMetadata } from './worktree-metadata-normalization' +import { + gcStaleWorktreeMeta, + normalizeWorktreeLinkedItemMetadata, + WORKTREE_META_GC_GRACE_MS +} from './worktree-metadata-normalization' // Only the presence of an entry matters here; the normalizer never reads its linked-item fields. function makeMeta(): WorktreeMeta { @@ -38,6 +42,99 @@ describe('normalizeWorktreeLinkedItemMetadata', () => { expect(normalizeWorktreeLinkedItemMetadata(state)).toBe(true) expect(state.workspaceLineageByChildKey).toEqual({}) }) + it('repairs malformed canonical metadata and alias maps', () => { + const state = makeState({ + worktreeMetaByIdentity: null as unknown as PersistedState['worktreeMetaByIdentity'], + worktreeIdentityAliases: { + 'local|r1::/tmp/wt': ['wt2:local:one', 'wt2:local:one', '', 42] + } as unknown as PersistedState['worktreeIdentityAliases'] + }) + + expect(normalizeWorktreeLinkedItemMetadata(state)).toBe(true) + expect(state.worktreeMetaByIdentity).toEqual({}) + // A key with no backing row is what turns the next write into an ambiguous alias, so it goes. + expect(state.worktreeIdentityAliases).toEqual({}) + }) + + it('keeps alias keys that still resolve to a metadata row', () => { + const state = makeState({ + worktreeMetaByIdentity: { 'wt2:local:one': makeMeta() }, + worktreeIdentityAliases: { 'local|r1::/tmp/wt': ['wt2:local:one'] } + }) + + expect(normalizeWorktreeLinkedItemMetadata(state)).toBe(false) + expect(state.worktreeIdentityAliases).toEqual({ 'local|r1::/tmp/wt': ['wt2:local:one'] }) + }) + + it('drops malformed canonical metadata entries', () => { + const state = makeState({ + worktreeMetaByIdentity: { + valid: makeMeta(), + malformed: null + } as unknown as PersistedState['worktreeMetaByIdentity'], + worktreeIdentityAliases: { 'local|r1::/tmp/wt': ['valid', 'malformed'] } + }) + + expect(normalizeWorktreeLinkedItemMetadata(state)).toBe(true) + expect(state.worktreeMetaByIdentity).toEqual({ valid: makeMeta() }) + expect(state.worktreeIdentityAliases).toEqual({ 'local|r1::/tmp/wt': ['valid'] }) + }) + + it('preserves every host canonical row when one legacy locator row is malformed', () => { + const localKey = 'wt2:local:local-instance' + const remoteKey = 'wt2:ssh%3Abuilder:remote-instance' + const worktreeId = 'r1::/tmp/wt' + const state = makeState({ + worktreeMeta: { [worktreeId]: null } as unknown as PersistedState['worktreeMeta'], + worktreeMetaByIdentity: { [localKey]: makeMeta(), [remoteKey]: makeMeta() }, + worktreeIdentityAliases: { + [`local|${worktreeId}`]: [localKey], + [`ssh:builder|${worktreeId}`]: [remoteKey] + } + }) + + expect(normalizeWorktreeLinkedItemMetadata(state)).toBe(true) + expect(state.worktreeMeta).toEqual({}) + expect(state.worktreeMetaByIdentity).toEqual({ + [localKey]: makeMeta(), + [remoteKey]: makeMeta() + }) + expect(state.worktreeIdentityAliases).toEqual({ + [`local|${worktreeId}`]: [localKey], + [`ssh:builder|${worktreeId}`]: [remoteKey] + }) + }) + + it('preserves valid canonical state when the whole legacy projection is malformed', () => { + const identityKey = 'wt2:ssh%3Abuilder:remote-instance' + const alias = 'ssh:builder|r1::/tmp/wt' + const state = makeState({ + worktreeMeta: null as unknown as PersistedState['worktreeMeta'], + worktreeMetaByIdentity: { [identityKey]: makeMeta() }, + worktreeIdentityAliases: { [alias]: [identityKey] } + }) + + expect(normalizeWorktreeLinkedItemMetadata(state)).toBe(true) + expect(state.worktreeMeta).toEqual({}) + expect(state.worktreeMetaByIdentity).toEqual({ [identityKey]: makeMeta() }) + expect(state.worktreeIdentityAliases).toEqual({ [alias]: [identityKey] }) + }) + + it('prunes canonical metadata left unreachable by dangling-alias cleanup', () => { + const liveKey = 'wt2:local:live' + const orphanKey = 'wt2:local:orphan' + const state = makeState({ + worktreeMetaByIdentity: { [liveKey]: makeMeta(), [orphanKey]: makeMeta() }, + worktreeIdentityAliases: { + 'local|r1::/tmp/live': [liveKey], + 'local|r1::/tmp/dangling': ['missing-key'] + } + }) + + expect(normalizeWorktreeLinkedItemMetadata(state)).toBe(true) + expect(state.worktreeMetaByIdentity).toEqual({ [liveKey]: makeMeta() }) + expect(state.worktreeIdentityAliases).toEqual({ 'local|r1::/tmp/live': [liveKey] }) + }) it('leaves already-normalized state clean', () => { const state = makeState({ @@ -47,3 +144,36 @@ describe('normalizeWorktreeLinkedItemMetadata', () => { expect(normalizeWorktreeLinkedItemMetadata(state)).toBe(false) }) }) + +describe('gcStaleWorktreeMeta', () => { + it('reclaims the identity rows of a collected worktree', () => { + const worktreeId = 'r1::/definitely/missing/orca/path' + const identityKey = 'wt2:local:dead' + const remoteIdentityKey = 'wt2:ssh:live' + const state = makeState({ + repos: [], + projects: [], + worktreeMeta: { + [worktreeId]: { + hostId: 'local', + instanceId: 'dead', + displayName: 'dead', + lastActivityAt: Date.now() - WORKTREE_META_GC_GRACE_MS - 1 + } as unknown as WorktreeMeta + }, + worktreeMetaByIdentity: { [identityKey]: makeMeta(), [remoteIdentityKey]: makeMeta() }, + worktreeIdentityAliases: { + [`local|${worktreeId}`]: [identityKey], + [`ssh:conn-1|${worktreeId}`]: [remoteIdentityKey] + } + }) + + expect(gcStaleWorktreeMeta(state)).toBe(1) + // A surviving alias would re-attach this dead metadata to a worktree later + // created at the same repoId::path, and nothing else ever reclaims it. + expect(state.worktreeMetaByIdentity).toEqual({ [remoteIdentityKey]: expect.anything() }) + expect(state.worktreeIdentityAliases).toEqual({ + [`ssh:conn-1|${worktreeId}`]: [remoteIdentityKey] + }) + }) +}) diff --git a/src/main/persistence/tracking-repos/worktree-metadata-normalization.ts b/src/main/persistence/tracking-repos/worktree-metadata-normalization.ts index 81c6e94843e..2d5d5a647ed 100644 --- a/src/main/persistence/tracking-repos/worktree-metadata-normalization.ts +++ b/src/main/persistence/tracking-repos/worktree-metadata-normalization.ts @@ -15,6 +15,11 @@ import { } from '../../../shared/workspace-linked-item' import { isWorkspaceLinkedItemSourceContextMatch } from '../../../shared/workspace-linked-item-source-context' import type { PersistedState } from '../../../shared/persisted-state-types' +import { + pruneUnreferencedWorktreeIdentityMeta, + removeWorktreeMetadataForHost +} from '../loading-store/worktree-identity-metadata' +import type { WorktreeMeta } from '../../../shared/worktree/meta-types' // Why: worktrees deleted outside Orca orphan their worktreeMeta, so the map grew monotonically (63% dead on a heavy install). // GC stays narrow: local-host entries only (a local existsSync would falsely condemn SSH/WSL remote paths) and only after a 30-day idle grace. @@ -75,11 +80,35 @@ export function gcStaleWorktreeMeta(state: PersistedState): number { delete state.worktreeMeta[key] delete state.worktreeLineageById[key] delete state.workspaceLineageByChildKey[worktreeWorkspaceKey(key)] + // Identity rows are companions too: a surviving alias would re-attach this dead metadata to a + // worktree later created at the same repoId::path, and nothing else ever reclaims them. + removeWorktreeMetadataForHost(state, key, LOCAL_EXECUTION_HOST_ID) removed++ } return removed } +function normalizeLinkedMetadata(meta: WorktreeMeta): boolean { + const linkedWorkItem = normalizeWorkspaceLinkedItem(meta.linkedWorkItem) + const sourceContext = normalizeStoredTaskSourceContext(meta.linkedTaskSourceContext) + const linkedTaskSourceContext = isWorkspaceLinkedItemSourceContextMatch( + linkedWorkItem, + sourceContext + ) + ? sourceContext + : null + let changed = false + if (!areWorkspaceLinkedItemsEqual(meta.linkedWorkItem, linkedWorkItem)) { + meta.linkedWorkItem = linkedWorkItem + changed = true + } + if (!areTaskSourceContextsEqual(meta.linkedTaskSourceContext, linkedTaskSourceContext)) { + meta.linkedTaskSourceContext = linkedTaskSourceContext + changed = true + } + return changed +} + export function normalizeWorktreeLinkedItemMetadata(state: PersistedState): boolean { // Why: a hand-corrupted null lineage map would throw on the companion deletes below. The repair // itself is dirty state — without it the map stays null on disk and is repaired again every load. @@ -98,8 +127,7 @@ export function normalizeWorktreeLinkedItemMetadata(state: PersistedState): bool Object.keys(state.worktreeLineageById).length > 0 || Object.keys(state.workspaceLineageByChildKey).length > 0 state.worktreeMeta = {} - // Companions go with the discarded map; a stranded lineage row would otherwise re-attach to a - // worktree recreated at the same repoId::path. + // Legacy lineage belongs to the discarded locator projection; canonical host state does not. state.worktreeLineageById = {} state.workspaceLineageByChildKey = {} changed ||= rawWorktreeMeta !== undefined || hadLineage @@ -109,29 +137,74 @@ export function normalizeWorktreeLinkedItemMetadata(state: PersistedState): bool // keeps timestamp-less keys forever and every downstream consumer trusts the Record type. if (typeof meta !== 'object' || meta === null || Array.isArray(meta)) { delete state.worktreeMeta[key] - // Companions go with it, matching gcStaleWorktreeMeta/removeWorktreeMeta; a stranded lineage row would - // otherwise re-attach to a worktree recreated at the same repoId::path. + // Legacy lineage belongs to this corrupt locator row; canonical host state is independent. delete state.worktreeLineageById[key] delete state.workspaceLineageByChildKey[worktreeWorkspaceKey(key)] changed = true continue } - const linkedWorkItem = normalizeWorkspaceLinkedItem(meta.linkedWorkItem) - const sourceContext = normalizeStoredTaskSourceContext(meta.linkedTaskSourceContext) - const linkedTaskSourceContext = isWorkspaceLinkedItemSourceContextMatch( - linkedWorkItem, - sourceContext - ) - ? sourceContext - : null - if (!areWorkspaceLinkedItemsEqual(meta.linkedWorkItem, linkedWorkItem)) { - meta.linkedWorkItem = linkedWorkItem + changed = normalizeLinkedMetadata(meta) || changed + } + const rawIdentityMetadata = state.worktreeMetaByIdentity as unknown + if ( + rawIdentityMetadata !== undefined && + (typeof rawIdentityMetadata !== 'object' || + rawIdentityMetadata === null || + Array.isArray(rawIdentityMetadata)) + ) { + state.worktreeMetaByIdentity = {} + changed = true + } + for (const [identityKey, meta] of Object.entries(state.worktreeMetaByIdentity ?? {})) { + if (typeof meta !== 'object' || meta === null || Array.isArray(meta)) { + delete state.worktreeMetaByIdentity?.[identityKey] changed = true + continue } - if (!areTaskSourceContextsEqual(meta.linkedTaskSourceContext, linkedTaskSourceContext)) { - meta.linkedTaskSourceContext = linkedTaskSourceContext + changed = normalizeLinkedMetadata(meta) || changed + } + + const rawAliases = state.worktreeIdentityAliases as unknown + const aliasesAreObject = + typeof rawAliases === 'object' && rawAliases !== null && !Array.isArray(rawAliases) + const aliasesAreMalformed = rawAliases !== undefined && !aliasesAreObject + const aliasesCanProveReachability = aliasesAreObject && Object.keys(rawAliases).length > 0 + if (aliasesAreMalformed) { + state.worktreeIdentityAliases = {} + changed = true + } + for (const [alias, rawIdentityKeys] of Object.entries(state.worktreeIdentityAliases ?? {})) { + if (!Array.isArray(rawIdentityKeys)) { + delete state.worktreeIdentityAliases?.[alias] + changed = true + continue + } + // Drop keys with no backing row: a dangling key is what turns the next write into an + // ambiguous alias, and nothing downstream can resolve it anyway. + const identityKeys = [ + ...new Set( + rawIdentityKeys.filter( + (key): key is string => + typeof key === 'string' && + key.length > 0 && + Boolean(state.worktreeMetaByIdentity?.[key]) + ) + ) + ] + if (identityKeys.length === 0) { + delete state.worktreeIdentityAliases?.[alias] + changed = true + } else if ( + identityKeys.length !== rawIdentityKeys.length || + identityKeys.some((key, index) => key !== rawIdentityKeys[index]) + ) { + state.worktreeIdentityAliases ??= {} + state.worktreeIdentityAliases[alias] = identityKeys changed = true } } + if (aliasesCanProveReachability) { + changed = pruneUnreferencedWorktreeIdentityMeta(state) || changed + } return changed } diff --git a/src/main/provisioned-root-ssh-adoption.ts b/src/main/provisioned-root-ssh-adoption.ts index dd74f6acbed..6a12c63b4a2 100644 --- a/src/main/provisioned-root-ssh-adoption.ts +++ b/src/main/provisioned-root-ssh-adoption.ts @@ -15,7 +15,7 @@ import { } from '../shared/ephemeral-vm-recipes' import { listEphemeralVmRuntimes } from '../shared/ephemeral-vm-runtime-store' import type { EphemeralVmRuntimeRecord } from '../shared/ephemeral-vm-runtimes' -import { getProjectHostSetupWorktreeMeta } from '../shared/project-host-setup-projection' +import { getProjectHostSetupWorktreeMeta } from '../shared/project-host-setup-lookup' import { isTuiAgent } from '../shared/tui-agent-config' import { getSshGitProvider } from './providers/ssh-git-dispatch' import { diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index d19e3ec9c3b..d471363ab1b 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -539,11 +539,11 @@ import { splitWorktreeIdForFilesystem, worktreeIdComparisonKey } from '../../shared/worktree/id' +import { getProjectIdForProviderIdentity } from '../../shared/project-host-setup-projection' import { - getProjectIdForProviderIdentity, getProjectHostSetupForRepo, getProjectHostSetupWorktreeMeta -} from '../../shared/project-host-setup-projection' +} from '../../shared/project-host-setup-lookup' import { parsePtySessionId } from '../../shared/pty-session-id-format' import { clampLinearIssueListLimit } from '../../shared/linear/issue-read-limits' import { isFolderRepo } from '../../shared/repo-kind' @@ -919,6 +919,7 @@ import { type RepoWorktreeRowDeps } from './repo-worktree-row-resolution' import { getRepoOwnedWorktreeMeta } from '../worktree-metadata-ownership' +import { readWorktreeMetaForHost } from '../persistence/host-qualified-worktree-meta' import { withTimeout } from '../../shared/promise-timeout-fallback' import { getLocalWorktreePathAccess, @@ -1355,6 +1356,7 @@ type RuntimeStore = { getAllWorktreeMeta: Store['getAllWorktreeMeta'] getWorktreeMeta: Store['getWorktreeMeta'] setWorktreeMeta: Store['setWorktreeMeta'] + setWorktreeMetaForHost?: Store['setWorktreeMetaForHost'] removeWorktreeMeta: Store['removeWorktreeMeta'] getWorktreeLineage?: Store['getWorktreeLineage'] getAllWorktreeLineage?: Store['getAllWorktreeLineage'] @@ -20853,7 +20855,7 @@ export class OrcaRuntimeService { workspaceKind: 'git', worktreeId: worktree.id, repoId: worktree.repoId, - ...((worktree.hostId ?? meta?.hostId) ? { hostId: worktree.hostId ?? meta?.hostId } : {}), + ...((meta?.hostId ?? worktree.hostId) ? { hostId: meta?.hostId ?? worktree.hostId } : {}), terminalPlatform, repo: repo?.displayName ?? worktree.repoId, path: worktree.path, @@ -24279,7 +24281,10 @@ export class OrcaRuntimeService { const metaById = store.getAllWorktreeMeta() const detected = scan.worktrees.map((gitWorktree) => { const worktreeId = `${repo.id}::${gitWorktree.path}` - const meta = getRepoOwnedWorktreeMeta(repo, worktreeId, metaById, repoOwnerCount) + // A host-qualified row is exact; the locator-keyed one is only trustworthy when this repo owns it. + const meta = + readWorktreeMetaForHost(store, worktreeId, expectedHostId) ?? + getRepoOwnedWorktreeMeta(repo, worktreeId, metaById, repoOwnerCount) const worktree = { ...mergeWorktree(repo.id, gitWorktree, meta, repo.displayName), hostId: repoOwnerCount === 1 ? (meta?.hostId ?? expectedHostId) : expectedHostId @@ -27147,12 +27152,20 @@ export class OrcaRuntimeService { createdAt }) } - this.store.setWorktreeMeta(worktree.id, stripOrcaProvenanceMetaUpdates(persistedMetaUpdates)) + const metadataUpdates = stripOrcaProvenanceMetaUpdates(persistedMetaUpdates) + const executionHostId = worktree.identity?.executionHostId ?? worktree.hostId + if (executionHostId && this.store.setWorktreeMetaForHost) { + this.store.setWorktreeMetaForHost(worktree.id, executionHostId, metadataUpdates) + } else { + this.store.setWorktreeMeta(worktree.id, metadataUpdates) + } // Why: unlike renderer-initiated optimistic updates, CLI callers need an // explicit push so the editor refreshes metadata changed outside the UI. this.invalidateResolvedWorktreeCache() this.notifyWorktreesChanged(worktree.repoId) - return await this.showManagedWorktree(`id:${worktree.id}`) + return await this.showManagedWorktree( + worktree.identity?.key ? `identity:${worktree.identity.key}` : `id:${worktree.id}` + ) } persistManagedWorktreeSortOrder(orderedIds: string[]): { updated: number } { @@ -32340,7 +32353,10 @@ export class OrcaRuntimeService { throw new Error('selector_not_found') } - if (selector.startsWith('id:')) { + if (selector.startsWith('identity:')) { + const identityKey = selector.slice('identity:'.length) + candidates = worktrees.filter((worktree) => worktree.identity?.key === identityKey) + } else if (selector.startsWith('id:')) { const worktreeId = explicitWorktreeId ?? selector.slice(3) candidates = worktrees.filter((worktree) => worktree.id === worktreeId) if (candidates.length === 0) { diff --git a/src/main/runtime/repo-worktree-row-resolution.test.ts b/src/main/runtime/repo-worktree-row-resolution.test.ts index d4bff4a9442..32111ef2a53 100644 --- a/src/main/runtime/repo-worktree-row-resolution.test.ts +++ b/src/main/runtime/repo-worktree-row-resolution.test.ts @@ -5,6 +5,7 @@ import type { WorktreeMeta } from '../../shared/worktree/meta-types' import type { GitWorktreeInfo, Worktree } from '../../shared/worktree/types' import type { Store } from '../persistence' import { + listStoredWorktreeRowsForRepo, resolveRepoWorktreeRows, resolveScopedWorktreeIdRow, type RepoWorktreeRowDeps @@ -111,6 +112,70 @@ describe('host-qualified scoped worktree resolution', () => { displayName: 'feature' }) }) + it('uses the host-qualified metadata owner when a legacy row has another host', async () => { + const deps = createDeps([ + repo('shared', '/remote/repo', { + connectionId: 'builder', + executionHostId: 'ssh:builder' + }) + ]) + const worktreeId = 'shared::/same/worktree' + const remoteMeta = { + displayName: 'remote workspace', + hostId: 'ssh:builder', + instanceId: 'remote-instance' + } as unknown as WorktreeMeta + deps.metaById[worktreeId] = { + displayName: 'stale local workspace', + hostId: 'local', + instanceId: 'local-instance' + } as unknown as WorktreeMeta + ;( + deps.store as Store & { + getWorktreeMetaForHost: () => WorktreeMeta + } + ).getWorktreeMetaForHost = () => remoteMeta + + const rows = await resolveRepoWorktreeRows( + deps, + deps.store.getRepos()[0]!, + deps.metaById, + new Map() + ) + + expect(rows[0]).toMatchObject({ + displayName: 'remote workspace', + hostId: 'ssh:builder', + instanceId: 'remote-instance' + }) + }) + + it('restores canonical-only SSH rows without leaking colliding local metadata', () => { + const local = repo('shared', '/local/repo', { executionHostId: 'local' }) + const remote = repo('shared', '/remote/repo', { + connectionId: 'builder', + executionHostId: 'ssh:builder' + }) + const deps = createDeps([local, remote]) + deps.metaById['shared::/local/worktree'] = { + displayName: 'local workspace', + hostId: 'local' + } as WorktreeMeta + ;( + deps.store as Store & { + getAllWorktreeMetaForHost: () => Record + } + ).getAllWorktreeMetaForHost = () => ({ + 'shared::/remote/worktree': { + displayName: 'remote workspace', + hostId: 'ssh:builder' + } as unknown as WorktreeMeta + }) + + expect(listStoredWorktreeRowsForRepo(deps.store, remote, 2)).toEqual([ + expect.objectContaining({ path: '/remote/worktree' }) + ]) + }) it.each([ ['runtime:windows', String.raw`C:\Users\dev\orca worktree`], diff --git a/src/main/runtime/repo-worktree-row-resolution.ts b/src/main/runtime/repo-worktree-row-resolution.ts index a9ef618ced7..e5e88607629 100644 --- a/src/main/runtime/repo-worktree-row-resolution.ts +++ b/src/main/runtime/repo-worktree-row-resolution.ts @@ -4,6 +4,11 @@ import { worktreeIdComparisonKey } from '../../shared/worktree/id' import { getRepoExecutionHostId, type ExecutionHostId } from '../../shared/execution-host' +import { + readAllWorktreeMetaForHost, + readWorktreeMetaForHost, + writeWorktreeMetaForHost +} from '../persistence/host-qualified-worktree-meta' import { isFolderRepo } from '../../shared/repo-kind' import { projectResolvedWorktreeLineage } from '../../shared/resolved-worktree-lineage' import { withTimeout } from '../../shared/promise-timeout-fallback' @@ -62,7 +67,9 @@ export function listStoredWorktreeRowsForRepo( ): GitWorktreeInfo[] { const expectedHostId = getRepoExecutionHostId(repo) const byWorktreeId = new Map() - for (const [worktreeId, meta] of Object.entries(store.getAllWorktreeMeta())) { + for (const [worktreeId, meta] of Object.entries( + readAllWorktreeMetaForHost(store, expectedHostId) + )) { const parsed = splitWorktreeId(worktreeId) if (!parsed || parsed.repoId !== repo.id) { continue @@ -134,15 +141,18 @@ export async function resolveRepoWorktreeRows( const worktreeId = `${repo.id}::${gitWorktree.path}` // Why: lineage validation needs a durable instance ID even when the runtime sees a workspace before renderer discovery-stamp. const existingMeta = metaById[worktreeId] - const ownedExistingMeta = getRepoOwnedWorktreeMeta(repo, worktreeId, metaById, repoOwnerCount) + // A host-qualified row is exact; the locator-keyed one is only trustworthy when this repo owns it. + const ownedExistingMeta = + readWorktreeMetaForHost(store, worktreeId, expectedHostId) ?? + getRepoOwnedWorktreeMeta(repo, worktreeId, metaById, repoOwnerCount) const meta = ownedExistingMeta?.instanceId ? ownedExistingMeta : ownedExistingMeta || (!existingMeta && repoOwnerCount === 1) - ? store.setWorktreeMeta(worktreeId, {}) + ? writeWorktreeMetaForHost(store, worktreeId, expectedHostId, {}) : undefined const merged = { ...mergeWorktree(repo.id, gitWorktree, meta, repo.displayName), - hostId: repoOwnerCount === 1 ? (existingMeta?.hostId ?? expectedHostId) : expectedHostId + hostId: meta?.hostId ?? expectedHostId } return { ...merged, diff --git a/src/main/runtime/worktree-scan-admin-fingerprint-gate.test.ts b/src/main/runtime/worktree-scan-admin-fingerprint-gate.test.ts index 1aa49b76d8e..3750884f84f 100644 --- a/src/main/runtime/worktree-scan-admin-fingerprint-gate.test.ts +++ b/src/main/runtime/worktree-scan-admin-fingerprint-gate.test.ts @@ -43,6 +43,7 @@ import { WORKTREE_SCAN_ADMIN_RECONCILE_INTERVAL_MS } from './orca-runtime' import { RESOLVED_WORKTREE_REPO_TIMEOUT_MS } from './repo-worktree-row-resolution' +import { canonicalWorktreeIdentity } from '../../shared/worktree/identity' const REPO_ID = 'repo-local' const REPO_PATH = '/Users/me/dev/app' @@ -75,8 +76,15 @@ function makeMeta(overrides: Record = {}) { function makeStore(options: { connectionId?: string; repoCount?: number; repoPath?: string } = {}) { const metaById: Record> = { - [WORKTREE_ID]: makeMeta(), - [MAIN_WORKTREE_ID]: makeMeta({ displayName: 'main' }) + [WORKTREE_ID]: makeMeta({ + hostId: 'local', + instanceId: '11111111-1111-4111-8111-111111111111' + }), + [MAIN_WORKTREE_ID]: makeMeta({ + displayName: 'main', + hostId: 'local', + instanceId: '22222222-2222-4222-8222-222222222222' + }) } const basePath = options.repoPath ?? REPO_PATH const repos = Array.from({ length: options.repoCount ?? 1 }, (_unused, index) => ({ @@ -586,6 +594,22 @@ describe('scoped explicit worktree-id resolution', () => { expect(resolved.id).toBe(MAIN_WORKTREE_ID) expect(scannedRepoPaths()).toEqual([REPO_PATH]) }) + it('resolves an exact canonical identity without relying on the mutable locator', async () => { + const runtime = new OrcaRuntimeService(makeStore({ repoCount: 10 }) as never) + const resolve = (selector: string): Promise<{ id: string }> => + ( + runtime as unknown as { resolveWorktreeSelector: (s: string) => Promise<{ id: string }> } + ).resolveWorktreeSelector(selector) + const identityKey = canonicalWorktreeIdentity({ + worktreeId: MAIN_WORKTREE_ID, + executionHostId: 'local', + instanceId: '22222222-2222-4222-8222-222222222222' + }) + + const resolved = await resolve(`identity:${identityKey}`) + + expect(resolved.id).toBe(MAIN_WORKTREE_ID) + }) it('still finds worktrees in other repos through the fleet path', async () => { const runtime = new OrcaRuntimeService(makeStore({ repoCount: 10 }) as never) diff --git a/src/main/workspace-space-repo-scan.ts b/src/main/workspace-space-repo-scan.ts index 8b97a89c041..ba1b2e4db4c 100644 --- a/src/main/workspace-space-repo-scan.ts +++ b/src/main/workspace-space-repo-scan.ts @@ -10,6 +10,8 @@ import type { } from '../shared/workspace-space-types' import { mapWithConcurrency } from '../shared/map-with-concurrency' import { getRepoExecutionHostId } from '../shared/execution-host' +import { readWorktreeMetaForHost } from './persistence/host-qualified-worktree-meta' +import { getRepoOwnedWorktreeMeta } from './worktree-metadata-ownership' import { getSshFilesystemProvider } from './providers/ssh-filesystem-dispatch' import { getSshGitProvider } from './providers/ssh-git-dispatch' import { createFolderWorktree, listRepoWorktrees } from './repo-worktrees' @@ -104,7 +106,15 @@ function reportProgress( function mergeForSpaceScan(repo: Repo, gitWorktree: GitWorktreeInfo, store: Store): Worktree { const worktreeId = `${repo.id}::${gitWorktree.path}` - return mergeWorktree(repo.id, gitWorktree, store.getWorktreeMeta(worktreeId), repo.displayName) + const executionHostId = getRepoExecutionHostId(repo) + const repoOwnerCount = store.getRepos().filter((candidate) => candidate.id === repo.id).length + const allMeta = store.getAllWorktreeMeta?.() + const legacyMeta = store.getWorktreeMeta?.(worktreeId) + const metaById = allMeta ?? (legacyMeta ? { [worktreeId]: legacyMeta } : {}) + const meta = + readWorktreeMetaForHost(store, worktreeId, executionHostId) ?? + getRepoOwnedWorktreeMeta(repo, worktreeId, metaById, repoOwnerCount) + return mergeWorktree(repo.id, gitWorktree, meta, repo.displayName) } export async function scanWorkspaceSpaceRepo(args: { diff --git a/src/main/worktree-identity-persistence.test.ts b/src/main/worktree-identity-persistence.test.ts new file mode 100644 index 00000000000..c66a2d72ce8 --- /dev/null +++ b/src/main/worktree-identity-persistence.test.ts @@ -0,0 +1,351 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { rmSync, mkdtempSync } from 'node:fs' +import { join } from 'node:path' +import { tmpdir } from 'node:os' +import type { PersistedState } from '../shared/persisted-state-types' +import { canonicalWorktreeIdentity } from '../shared/worktree/identity' +import { composeWorktreeHostIdentity } from '../shared/worktree/host-qualified-identity' +import { createStore, readDataFile, testState, writeDataFile } from './persistence-test-harness' + +describe('host-qualified worktree metadata', () => { + const worktreeId = 'repo-1::/workspace/feature' + const ROTATED_INSTANCE_ID = '44444444-4444-4444-8444-444444444444' + + beforeEach(() => { + testState.dir = mkdtempSync(join(tmpdir(), 'orca-worktree-identity-')) + }) + + afterEach(() => { + rmSync(testState.dir, { recursive: true, force: true }) + }) + + it('stores independent metadata for colliding locators on two hosts', () => { + const store = createStore() + + store.setWorktreeMetaForHost(worktreeId, 'local', { displayName: 'Local feature' }) + store.setWorktreeMetaForHost(worktreeId, 'ssh:build-box', { displayName: 'Remote feature' }) + + expect(store.getWorktreeMetaForHost(worktreeId, 'local')?.displayName).toBe('Local feature') + expect(store.getWorktreeMetaForHost(worktreeId, 'ssh:build-box')?.displayName).toBe( + 'Remote feature' + ) + store.flush() + const persisted = readDataFile() as PersistedState + expect(Object.keys(persisted.worktreeMetaByIdentity ?? {})).toHaveLength(2) + expect(store.getWorktreeMeta(worktreeId)?.displayName).toBe('Local feature') + }) + it('projects canonical-only metadata for exactly one host', () => { + const store = createStore() + store.setWorktreeMetaForHost(worktreeId, 'local', { displayName: 'Local feature' }) + store.setWorktreeMetaForHost(worktreeId, 'ssh:build-box', { + displayName: 'Remote feature' + }) + + expect(store.getAllWorktreeMetaForHost('local')).toEqual({ + [worktreeId]: expect.objectContaining({ displayName: 'Local feature', hostId: 'local' }) + }) + expect(store.getAllWorktreeMetaForHost('ssh:build-box')).toEqual({ + [worktreeId]: expect.objectContaining({ + displayName: 'Remote feature', + hostId: 'ssh:build-box' + }) + }) + }) + it('reloads host-specific metadata without collapsing it to the legacy locator', () => { + const store = createStore() + store.setWorktreeMetaForHost(worktreeId, 'local', { displayName: 'Local feature' }) + store.setWorktreeMetaForHost(worktreeId, 'ssh:build-box', { displayName: 'Remote feature' }) + store.flush() + + const reloaded = createStore() + expect(reloaded.getWorktreeMetaForHost(worktreeId, 'local')?.displayName).toBe('Local feature') + expect(reloaded.getWorktreeMetaForHost(worktreeId, 'ssh:build-box')?.displayName).toBe( + 'Remote feature' + ) + }) + it('keeps canonical metadata current for legacy writers on a known owner', () => { + const store = createStore() + store.setWorktreeMetaForHost(worktreeId, 'local', { comment: 'before' }) + + store.setWorktreeMeta(worktreeId, { comment: 'after' }) + + expect(store.getWorktreeMetaForHost(worktreeId, 'local')?.comment).toBe('after') + }) + it('backfills one stable instance for legacy metadata that omitted it', () => { + const seed = createStore() + seed.setWorktreeMeta(worktreeId, { displayName: 'Legacy feature' }) + seed.flush() + const legacy = readDataFile() as PersistedState + delete legacy.worktreeMeta[worktreeId]?.instanceId + delete legacy.worktreeMetaByIdentity + delete legacy.worktreeIdentityAliases + writeDataFile(legacy) + + const store = createStore() + const first = store.getWorktreeMetaForHost(worktreeId, 'local') + const second = store.getWorktreeMetaForHost(worktreeId, 'local') + store.setWorktreeMeta(worktreeId, { comment: 'after migration' }) + store.flush() + + expect(first?.instanceId).toBeTruthy() + expect(second?.instanceId).toBe(first?.instanceId) + expect(store.getWorktreeMetaForHost(worktreeId, 'local')?.comment).toBe('after migration') + const migrated = readDataFile() as PersistedState + expect(migrated.worktreeMeta[worktreeId]?.hostId).toBe('local') + expect(Object.keys(migrated.worktreeMetaByIdentity ?? {})).toHaveLength(1) + }) + // Fails open on purpose: an ambiguous alias used to brick reads and throw out of the worktree + // listing loop, taking every workspace in the repo down with it and never self-healing. + it('collapses an ambiguous locator onto its most recently active instance', () => { + const seed = createStore() + const first = seed.setWorktreeMetaForHost(worktreeId, 'local', { displayName: 'First' }) + seed.flush() + const persisted = readDataFile() as PersistedState + const alias = composeWorktreeHostIdentity('local', worktreeId) + const secondKey = canonicalWorktreeIdentity({ + worktreeId, + executionHostId: 'local', + instanceId: '33333333-3333-4333-8333-333333333333' + }) + persisted.worktreeMetaByIdentity ??= {} + persisted.worktreeIdentityAliases ??= {} + persisted.worktreeMetaByIdentity[secondKey] = { + ...first, + instanceId: '33333333-3333-4333-8333-333333333333', + displayName: 'Second', + lastActivityAt: 1 + } + persisted.worktreeIdentityAliases[alias] = [ + ...(persisted.worktreeIdentityAliases[alias] ?? []), + secondKey + ] + writeDataFile(persisted) + + const store = createStore() + expect(() => + store.setWorktreeMetaForHost(worktreeId, 'local', { comment: 'ambiguous write' }) + ).toThrow('Worktree identity is ambiguous for this host and locator.') + + const repaired = createStore() + expect(repaired.getWorktreeMetaForHost(worktreeId, 'local')?.displayName).toBe('Second') + repaired.flush() + const afterRead = readDataFile() as PersistedState + expect(afterRead.worktreeIdentityAliases?.[alias]).toEqual([ + persisted.worktreeIdentityAliases?.[alias]?.[0], + secondKey + ]) + expect(Object.keys(afterRead.worktreeMetaByIdentity ?? {})).toHaveLength(2) + }) + + it('removes only the selected legacy-owner metadata when locators collide', () => { + const store = createStore() + store.setWorktreeMetaForHost(worktreeId, 'local', { displayName: 'Local feature' }) + store.setWorktreeMetaForHost(worktreeId, 'ssh:build-box', { displayName: 'Remote feature' }) + + store.removeWorktreeMeta(worktreeId, 'local') + + expect(store.getWorktreeMetaForHost(worktreeId, 'local')).toBeUndefined() + expect(store.getWorktreeMetaForHost(worktreeId, 'ssh:build-box')?.displayName).toBe( + 'Remote feature' + ) + }) + + it('removes only the selected host metadata when locators collide', () => { + const store = createStore() + store.setWorktreeMetaForHost(worktreeId, 'local', { displayName: 'Local feature' }) + store.setWorktreeMetaForHost(worktreeId, 'ssh:build-box', { displayName: 'Remote feature' }) + + store.removeWorktreeMeta(worktreeId, 'ssh:build-box') + + expect(store.getWorktreeMetaForHost(worktreeId, 'local')?.displayName).toBe('Local feature') + expect(store.getWorktreeMetaForHost(worktreeId, 'ssh:build-box')).toBeUndefined() + store.flush() + expect( + Object.keys((readDataFile() as PersistedState).worktreeMetaByIdentity ?? {}) + ).toHaveLength(1) + }) + + it('moves the locator alias without changing canonical identity', () => { + const store = createStore() + const meta = store.setWorktreeMetaForHost(worktreeId, 'local', { displayName: 'Feature' }) + const identityKey = canonicalWorktreeIdentity({ + worktreeId, + executionHostId: 'local', + instanceId: meta.instanceId! + }) + const renamedId = 'repo-1::/workspace/renamed-feature' + + store.migrateWorktreeIdentity(worktreeId, renamedId) + + expect(store.getWorktreeMetaForHost(worktreeId, 'local')).toBeUndefined() + expect(store.getWorktreeMetaForHost(renamedId, 'local')?.instanceId).toBe(meta.instanceId) + store.flush() + const persisted = readDataFile() as PersistedState + expect(persisted.worktreeMetaByIdentity?.[identityKey]?.displayName).toBe('Feature') + expect( + persisted.worktreeIdentityAliases?.[composeWorktreeHostIdentity('local', renamedId)] + ).toEqual([identityKey]) + }) + + it('keeps the canonical identity stable when the locator changes', () => { + const store = createStore() + const meta = store.setWorktreeMetaForHost(worktreeId, 'local', { + displayName: 'Feature' + }) + + const before = canonicalWorktreeIdentity({ + worktreeId, + executionHostId: 'local', + instanceId: meta.instanceId! + }) + const after = canonicalWorktreeIdentity({ + worktreeId: 'repo-1::/workspace/renamed-feature', + executionHostId: 'local', + instanceId: meta.instanceId! + }) + + expect(after).toBe(before) + expect(composeWorktreeHostIdentity('local', worktreeId)).toBe( + 'local|repo-1::/workspace/feature' + ) + }) + it('honours a deliberate instanceId rotation instead of pinning the stored one', () => { + const store = createStore() + store.setWorktreeMetaForHost(worktreeId, 'local', { displayName: 'Feature' }) + + // worktree-lineage-pruning rotates a proven-missing parent so path reuse cannot + // validate old lineage; pinning the existing id silently disarmed that guard. + const rotated = store.setWorktreeMeta(worktreeId, { instanceId: ROTATED_INSTANCE_ID }) + + expect(rotated.instanceId).toBe(ROTATED_INSTANCE_ID) + expect(store.getWorktreeMetaForHost(worktreeId, 'local')?.instanceId).toBe(ROTATED_INSTANCE_ID) + store.flush() + const persisted = readDataFile() as PersistedState + // The old identity row goes with the rotation rather than lingering unreachable. + expect(Object.keys(persisted.worktreeMetaByIdentity ?? {})).toEqual([ + canonicalWorktreeIdentity({ + worktreeId, + executionHostId: 'local', + instanceId: ROTATED_INSTANCE_ID + }) + ]) + }) + + it('leaves other hosts at the old locator when one host renames its folder', () => { + const store = createStore() + store.setWorktreeMetaForHost(worktreeId, 'local', { displayName: 'Local feature' }) + store.setWorktreeMetaForHost(worktreeId, 'ssh:build-box', { displayName: 'Remote feature' }) + const renamedId = 'repo-1::/workspace/renamed-feature' + + store.migrateWorktreeIdentity(worktreeId, renamedId, 'local') + + expect(store.getWorktreeMetaForHost(renamedId, 'local')?.displayName).toBe('Local feature') + // The SSH host never moved, so its row must stay reachable at the path it still has. + expect(store.getWorktreeMetaForHost(worktreeId, 'ssh:build-box')?.displayName).toBe( + 'Remote feature' + ) + }) + + it('drops every host identity row when a locator is removed outright', () => { + const store = createStore() + store.setWorktreeMetaForHost(worktreeId, 'local', { displayName: 'Local feature' }) + store.setWorktreeMetaForHost(worktreeId, 'ssh:build-box', { displayName: 'Remote feature' }) + + store.removeWorktreeMeta(worktreeId) + + store.flush() + const persisted = readDataFile() as PersistedState + expect(persisted.worktreeMetaByIdentity ?? {}).toEqual({}) + expect(persisted.worktreeIdentityAliases ?? {}).toEqual({}) + }) + + it('repairs a missing canonical instance id while re-adopting an SSH target', () => { + const oldHostId = 'ssh:old-target' as const + const newHostId = 'ssh:new-target' as const + const seed = createStore() + seed.setWorktreeMetaForHost(worktreeId, oldHostId, { displayName: 'Remote feature' }) + seed.flush() + const persisted = readDataFile() as PersistedState + const oldAlias = composeWorktreeHostIdentity(oldHostId, worktreeId) + const oldKey = persisted.worktreeIdentityAliases?.[oldAlias]?.[0] + expect(oldKey).toBeTruthy() + delete persisted.worktreeMetaByIdentity?.[oldKey!]?.instanceId + writeDataFile(persisted) + + const store = createStore() + store.reassignSshTargetId('old-target', 'new-target') + store.flush() + + const repaired = readDataFile() as PersistedState + const newAlias = composeWorktreeHostIdentity(newHostId, worktreeId) + const newKey = repaired.worktreeIdentityAliases?.[newAlias]?.[0] + expect(repaired.worktreeIdentityAliases?.[oldAlias]).toBeUndefined() + expect(newKey).toBeTruthy() + expect(repaired.worktreeMetaByIdentity?.[newKey!]).toMatchObject({ + displayName: 'Remote feature', + hostId: newHostId, + instanceId: expect.any(String) + }) + expect(repaired.worktreeMetaByIdentity?.[oldKey!]).toBeUndefined() + }) + + it('preserves source metadata when the re-adoption destination is divergent', () => { + const oldHostId = 'ssh:old-target' as const + const newHostId = 'ssh:new-target' as const + const store = createStore() + store.setWorktreeMetaForHost(worktreeId, oldHostId, { displayName: 'Source feature' }) + store.setWorktreeMetaForHost(worktreeId, newHostId, { + displayName: 'Destination feature' + }) + + store.reassignSshTargetId('old-target', 'new-target') + + expect(store.getWorktreeMetaForHost(worktreeId, oldHostId)?.displayName).toBe('Source feature') + expect(store.getWorktreeMetaForHost(worktreeId, newHostId)?.displayName).toBe( + 'Destination feature' + ) + expect(store.getWorktreeMeta(worktreeId)?.hostId).toBe(oldHostId) + store.flush() + const persisted = readDataFile() as PersistedState + expect( + persisted.worktreeIdentityAliases?.[composeWorktreeHostIdentity(oldHostId, worktreeId)] + ).toHaveLength(1) + expect( + persisted.worktreeIdentityAliases?.[composeWorktreeHostIdentity(newHostId, worktreeId)] + ).toHaveLength(1) + }) + + it('deduplicates an equivalent destination during SSH target re-adoption', () => { + const oldHostId = 'ssh:old-target' as const + const newHostId = 'ssh:new-target' as const + const seed = createStore() + seed.setWorktreeMetaForHost(worktreeId, oldHostId, { displayName: 'Remote feature' }) + seed.flush() + const persisted = readDataFile() as PersistedState + const oldAlias = composeWorktreeHostIdentity(oldHostId, worktreeId) + const oldKey = persisted.worktreeIdentityAliases?.[oldAlias]?.[0] + const source = oldKey ? persisted.worktreeMetaByIdentity?.[oldKey] : undefined + expect(source?.instanceId).toBeTruthy() + const newAlias = composeWorktreeHostIdentity(newHostId, worktreeId) + const newKey = canonicalWorktreeIdentity({ + worktreeId, + executionHostId: newHostId, + instanceId: source!.instanceId! + }) + persisted.worktreeMetaByIdentity ??= {} + persisted.worktreeIdentityAliases ??= {} + persisted.worktreeMetaByIdentity[newKey] = { ...source!, hostId: newHostId } + persisted.worktreeIdentityAliases[newAlias] = [newKey] + writeDataFile(persisted) + + const store = createStore() + store.reassignSshTargetId('old-target', 'new-target') + store.flush() + + const deduped = readDataFile() as PersistedState + expect(deduped.worktreeIdentityAliases?.[oldAlias]).toBeUndefined() + expect(deduped.worktreeIdentityAliases?.[newAlias]).toEqual([newKey]) + expect(deduped.worktreeMetaByIdentity?.[oldKey!]).toBeUndefined() + expect(deduped.worktreeMetaByIdentity?.[newKey]).toEqual({ ...source!, hostId: newHostId }) + }) +}) diff --git a/src/main/worktree-metadata-ownership.test.ts b/src/main/worktree-metadata-ownership.test.ts new file mode 100644 index 00000000000..5582decec01 --- /dev/null +++ b/src/main/worktree-metadata-ownership.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it } from 'vitest' +import { getRepoOwnedWorktreeMeta } from './worktree-metadata-ownership' +import type { Repo } from '../shared/repo-types' +import type { WorktreeMeta } from '../shared/worktree/meta-types' + +const worktreeId = 'repo-1::/workspace/feature' +const localRepo: Repo = { + id: 'repo-1', + path: '/workspace/repo', + displayName: 'Local repo', + badgeColor: '#000', + addedAt: 1, + executionHostId: 'local' +} +const sshRepo: Repo = { + ...localRepo, + path: '/workspace/repo', + connectionId: 'build-box', + executionHostId: 'ssh:build-box' +} +const localMeta = { hostId: 'local', displayName: 'local metadata' } as WorktreeMeta + +function resolve(repo: Repo, ownerCount: number): WorktreeMeta | undefined { + return getRepoOwnedWorktreeMeta(repo, worktreeId, { [worktreeId]: localMeta }, ownerCount) +} + +describe('getRepoOwnedWorktreeMeta', () => { + it('rejects another host metadata when repo IDs and paths collide', () => { + expect(resolve(sshRepo, 2)).toBeUndefined() + }) + + it('keeps legacy metadata for a single-owner repo', () => { + expect(resolve(localRepo, 1)).toBe(localMeta) + }) +}) diff --git a/src/preload/api/worktree-api.ts b/src/preload/api/worktree-api.ts index 7a85bd51a32..d665d278580 100644 --- a/src/preload/api/worktree-api.ts +++ b/src/preload/api/worktree-api.ts @@ -112,7 +112,11 @@ export type WorktreeApi = { expectedHead: string hostId?: ExecutionHostId }) => Promise - updateMeta: (args: { worktreeId: string; updates: Partial }) => Promise + updateMeta: (args: { + worktreeId: string + executionHostId?: ExecutionHostId + updates: Partial + }) => Promise listLineage: () => Promise<{ lineage: Record workspaceLineage?: Record diff --git a/src/renderer/src/components/right-sidebar/checks-panel/use-checks-panel-check-and-review-actions.tsx b/src/renderer/src/components/right-sidebar/checks-panel/use-checks-panel-check-and-review-actions.tsx index d4e2b96c1d8..e423c3e8797 100644 --- a/src/renderer/src/components/right-sidebar/checks-panel/use-checks-panel-check-and-review-actions.tsx +++ b/src/renderer/src/components/right-sidebar/checks-panel/use-checks-panel-check-and-review-actions.tsx @@ -349,11 +349,20 @@ export function useChecksPanelCheckAndReviewActions(model: ChecksPanelCheckAndRe ) const handleUnlinkPullRequest = useCallback(() => { - if (!activeWorktreeId || activeReview?.provider !== 'github' || linkedPR === null) { + if ( + !activeWorktreeId || + !activeWorktree || + activeReview?.provider !== 'github' || + linkedPR === null + ) { return } - void updateWorktreeMeta(activeWorktreeId, { linkedPR: null }) - }, [activeReview?.provider, activeWorktreeId, linkedPR, updateWorktreeMeta]) + void updateWorktreeMeta( + activeWorktreeId, + { linkedPR: null }, + { executionHostId: activeWorktree.hostId } + ) + }, [activeReview?.provider, activeWorktree, activeWorktreeId, linkedPR, updateWorktreeMeta]) const handleLinkAnotherPullRequest = useCallback(() => { if (!activeWorktreeId || !activeWorktree || activeReview?.provider !== 'github') { @@ -364,6 +373,7 @@ export function useChecksPanelCheckAndReviewActions(model: ChecksPanelCheckAndRe // Why: the same workspace ID can exist under two hosts. Naming the owner // keeps the dialog on this workspace instead of the ambiguous lookup. repoId: activeWorktree.repoId, + executionHostId: activeWorktree.hostId, currentDisplayName: activeWorktree.displayName, currentIssue: activeWorktree.linkedIssue, currentPR: activeWorktree.linkedPR ?? activeReview.number, diff --git a/src/renderer/src/components/sidebar/WorkspaceKanbanDrawer.search.test.tsx b/src/renderer/src/components/sidebar/WorkspaceKanbanDrawer.search.test.tsx index 7cb3081474e..5cb49c71cce 100644 --- a/src/renderer/src/components/sidebar/WorkspaceKanbanDrawer.search.test.tsx +++ b/src/renderer/src/components/sidebar/WorkspaceKanbanDrawer.search.test.tsx @@ -8,6 +8,7 @@ import { useAppStore } from '@/store' import type { Repo } from '../../../../shared/repo-types' import type { WorktreeMeta } from '../../../../shared/worktree/meta-types' import type { Worktree } from '../../../../shared/worktree/types' +import type { WorktreeMetaBatchUpdate } from '../../store/slices/worktree-helpers' import { getWorktreeHostIdentity } from '../../../../shared/worktree/host-qualified-identity' import WorkspaceKanbanDrawer from './WorkspaceKanbanDrawer' import type { WorkspaceKanbanLaneView } from './workspace-kanban-search' @@ -164,7 +165,7 @@ vi.mock('./workspace-board-task-status-sync', async (importOriginal) => ({ })) type UpdateWorktreesMeta = ( - updatesByWorktreeId: ReadonlyMap> + updates: readonly WorktreeMetaBatchUpdate[] | ReadonlyMap> ) => Promise const statuses = [ @@ -410,7 +411,13 @@ describe('WorkspaceKanbanDrawer search', () => { }) }) - const dropped = updateWorktreesMeta.mock.calls.at(-1)?.[0].get(omega.id) + const payload = updateWorktreesMeta.mock.calls.at(-1)?.[0] + let dropped: Partial | undefined + if (Array.isArray(payload)) { + dropped = payload.find((entry) => entry.worktreeId === omega.id)?.updates + } else if (payload && 'get' in payload) { + dropped = payload.get(omega.id) + } expect(dropped?.workspaceStatus).toBe('todo') expect(dropped?.manualOrder).toBeGreaterThan(gamma.manualOrder ?? 0) expect(dropped?.manualOrder).toBeLessThan(delta.manualOrder ?? 0) diff --git a/src/renderer/src/components/sidebar/WorktreeList.status-lane-lineage-drop.test.tsx b/src/renderer/src/components/sidebar/WorktreeList.status-lane-lineage-drop.test.tsx index 47cc038799d..4e726dca60c 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.status-lane-lineage-drop.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.status-lane-lineage-drop.test.tsx @@ -8,6 +8,7 @@ import type { WorktreeCardProperty } from '../../../../shared/ui-chrome-types' import type { WorktreeLineage } from '../../../../shared/worktree/lineage-types' import type { WorktreeMeta } from '../../../../shared/worktree/meta-types' import type { WorkspaceStatus, Worktree } from '../../../../shared/worktree/types' +import type { WorktreeMetaBatchUpdate } from '../../store/slices/worktree-helpers' import { DEFAULT_WORKSPACE_STATUSES } from '../../../../shared/workspace-status-defaults' import { WORKSPACE_STATUS_DRAG_IDS_TYPE, @@ -369,7 +370,13 @@ async function dropWorktreesOnStatusLane( function committedStatusUpdates(): Map> { expect(mockStore.updateWorktreesMeta).toHaveBeenCalledTimes(1) - return mockStore.updateWorktreesMeta.mock.calls[0]![0] as Map> + const input = mockStore.updateWorktreesMeta.mock.calls[0]![0] as + | readonly WorktreeMetaBatchUpdate[] + | ReadonlyMap> + if ('get' in input) { + return new Map(input) + } + return new Map(input.map((entry) => [entry.worktreeId, entry.updates])) } describe('WorktreeList status-lane drop carries visible lineage children (#9083)', () => { diff --git a/src/renderer/src/components/sidebar/WorktreeMetaDialog.test.tsx b/src/renderer/src/components/sidebar/WorktreeMetaDialog.test.tsx index 13b4bbac1b8..338c61190c5 100644 --- a/src/renderer/src/components/sidebar/WorktreeMetaDialog.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeMetaDialog.test.tsx @@ -133,6 +133,7 @@ function openDialog( /** Extra owners of the same workspace ID, which the index reads as ambiguous. */ otherRepos?: { repoId: string; worktree?: Partial }[] modalRepoId?: string + modalExecutionHostId?: string linearViewerOrganizationUrlKey?: string } = {} ): void { @@ -169,6 +170,7 @@ function openDialog( modalData: { worktreeId: options.worktreeId ?? worktree.id, ...(options.modalRepoId ? { repoId: options.modalRepoId } : {}), + ...(options.modalExecutionHostId ? { executionHostId: options.modalExecutionHostId } : {}), currentDisplayName: worktree.displayName, currentComment: worktree.comment, focus: 'comment' @@ -328,6 +330,27 @@ describe('WorktreeMetaDialog issue link row', () => { expect(updates.linkedIssue).toBe(99) expect(updates).not.toHaveProperty('linkedLinearIssue') }) + it('qualifies a save with the host selected by the opening row', async () => { + openDialog({ + worktree: { hostId: 'ssh:build-box' }, + modalExecutionHostId: 'ssh:build-box' + }) + + fireEvent.change(screen.getByPlaceholderText('Notes about this worktree...'), { + target: { value: 'remote note' } + }) + await act(async () => { + fireEvent.click(saveButton()) + }) + + await waitFor(() => + expect(updateWorktreeMeta).toHaveBeenCalledWith( + WORKTREE_ID, + expect.objectContaining({ comment: 'remote note' }), + { executionHostId: 'ssh:build-box' } + ) + ) + }) // updateWorktreeMeta stamps lastActivityAt on any comment write, which would // reorder the workspace under the time-decay sidebar sort. diff --git a/src/renderer/src/components/sidebar/WorktreeMetaDialog.tsx b/src/renderer/src/components/sidebar/WorktreeMetaDialog.tsx index 0482a644221..e714deb0453 100644 --- a/src/renderer/src/components/sidebar/WorktreeMetaDialog.tsx +++ b/src/renderer/src/components/sidebar/WorktreeMetaDialog.tsx @@ -30,6 +30,7 @@ import { parseIssueLinkInput, type IssueLinkProvider } from '../../../../shared/issue-link-input' +import { parseExecutionHostId } from '../../../../shared/execution-host' import { WorktreeDisplayNameField } from './WorktreeDisplayNameField' function resizeCommentTextarea(textarea: HTMLTextAreaElement): void { @@ -56,6 +57,10 @@ const WorktreeMetaDialog = React.memo(function WorktreeMetaDialog() { const isOpen = isEditMeta const worktreeId = typeof modalData.worktreeId === 'string' ? modalData.worktreeId : '' + const executionHostId = + typeof modalData.executionHostId === 'string' + ? (parseExecutionHostId(modalData.executionHostId)?.id ?? undefined) + : undefined const currentDisplayName = typeof modalData.currentDisplayName === 'string' ? modalData.currentDisplayName : '' const currentComment = @@ -224,7 +229,9 @@ const WorktreeMetaDialog = React.memo(function WorktreeMetaDialog() { try { const updates = buildWorktreeMetaUpdates(draft, snapshot, liveLinks) - const result = await updateWorktreeMeta(worktreeId, updates) + const result = executionHostId + ? await updateWorktreeMeta(worktreeId, updates, { executionHostId }) + : await updateWorktreeMeta(worktreeId, updates) // Why: a failed save refetches and reverts the optimistic write. Closing // here would report success for an edit that silently undid itself, and // would discard the name, comment and PR changes in the same payload. @@ -249,6 +256,7 @@ const WorktreeMetaDialog = React.memo(function WorktreeMetaDialog() { } }, [ worktreeId, + executionHostId, canSave, draft, snapshot, diff --git a/src/renderer/src/components/sidebar/use-workspace-kanban-board-projection.ts b/src/renderer/src/components/sidebar/use-workspace-kanban-board-projection.ts index 455fd81849c..74d3a9ec47a 100644 --- a/src/renderer/src/components/sidebar/use-workspace-kanban-board-projection.ts +++ b/src/renderer/src/components/sidebar/use-workspace-kanban-board-projection.ts @@ -5,6 +5,7 @@ import { groupWorkspaceKanbanWorktrees } from './workspace-kanban-worktree-group import { buildWorkspaceKanbanLaneViews } from './workspace-kanban-search' import { useWorkspaceKanbanSearch } from './use-workspace-kanban-search' import { registerWorkspaceKanbanSidebarDropGroups } from './workspace-kanban-sidebar-drop' +import { buildUnambiguousWorktreeIdIndex } from './worktree-unambiguous-id-index' import { composeWorktreeHostIdentity, getWorktreeHostIdentity @@ -38,7 +39,7 @@ export function useWorkspaceKanbanBoardProjection(args: { [args.allWorktrees, args.sortBy, args.workspaceStatuses, visibleWorktreeIds] ) const worktreeById = useMemo( - () => new Map(args.allWorktrees.map((worktree) => [worktree.id, worktree])), + () => buildUnambiguousWorktreeIdIndex(args.allWorktrees), [args.allWorktrees] ) const boardWorktrees = useMemo( diff --git a/src/renderer/src/components/sidebar/use-workspace-kanban-status-actions.ts b/src/renderer/src/components/sidebar/use-workspace-kanban-status-actions.ts index 3945c648a6d..5e5f9729ddc 100644 --- a/src/renderer/src/components/sidebar/use-workspace-kanban-status-actions.ts +++ b/src/renderer/src/components/sidebar/use-workspace-kanban-status-actions.ts @@ -90,7 +90,11 @@ export function useWorkspaceKanbanStatusActions(args: { recordInteraction() for (const worktree of args.allWorktrees) { if (getWorkspaceStatus(worktree, args.workspaceStatuses) === statusId) { - void args.updateWorktreeMeta(worktree.id, { workspaceStatus: fallbackStatus }) + void args.updateWorktreeMeta( + worktree.id, + { workspaceStatus: fallbackStatus }, + { executionHostId: worktree.hostId ?? 'local' } + ) } } }, diff --git a/src/renderer/src/components/sidebar/use-workspace-kanban-worktree-actions.ts b/src/renderer/src/components/sidebar/use-workspace-kanban-worktree-actions.ts index 687328e30c6..96647d65763 100644 --- a/src/renderer/src/components/sidebar/use-workspace-kanban-worktree-actions.ts +++ b/src/renderer/src/components/sidebar/use-workspace-kanban-worktree-actions.ts @@ -9,6 +9,7 @@ import { } from './worktree-manual-order' import type { WorktreeMeta } from '../../../../shared/worktree/meta-types' import type { WorkspaceStatus, Worktree } from '../../../../shared/worktree/types' +import type { WorktreeMetaBatchUpdate } from '../../store/slices/worktree-helpers' import type { WorktreeManualOrderCatalog } from './worktree-manual-order-catalog' type LaneView = { items: readonly Worktree[] } @@ -54,14 +55,18 @@ export function useWorkspaceKanbanWorktreeActions(args: { return } recordInteraction() - void args.updateWorktreeMeta(worktreeId, { workspaceStatus: status }) + void args.updateWorktreeMeta( + worktreeId, + { workspaceStatus: status }, + { executionHostId: current.hostId ?? 'local' } + ) args.maybeSyncTaskStatuses([worktreeId], status) }, [args] ) const moveWorktreesToStatus = useCallback( (worktreeIds: readonly string[], status: WorkspaceStatus) => { - const updates = new Map>() + const updates: WorktreeMetaBatchUpdate[] = [] const changedIds: string[] = [] for (const worktreeId of worktreeIds) { const current = args.worktreeById.get(worktreeId) @@ -69,7 +74,11 @@ export function useWorkspaceKanbanWorktreeActions(args: { continue } changedIds.push(worktreeId) - updates.set(worktreeId, { workspaceStatus: status }) + updates.push({ + worktreeId, + updates: { workspaceStatus: status }, + executionHostId: current.hostId ?? 'local' + }) } if (changedIds.length === 0) { return @@ -87,7 +96,7 @@ export function useWorkspaceKanbanWorktreeActions(args: { dropIndex: number writeManualOrder?: boolean }) => { - const updates = new Map>() + const updates: WorktreeMetaBatchUpdate[] = [] const writeManualOrder = drop.writeManualOrder ?? shouldWriteDropManualOrder(drop.worktreeIds, drop.status) const order = writeManualOrder @@ -106,23 +115,40 @@ export function useWorkspaceKanbanWorktreeActions(args: { if (!current) { continue } + const next: Partial = {} if (getWorkspaceStatus(current, args.workspaceStatuses) !== drop.status) { - updates.set(worktreeId, { workspaceStatus: drop.status }) + next.workspaceStatus = drop.status + } + updates.push({ + worktreeId, + updates: next, + executionHostId: current.hostId ?? 'local' + }) + } + for (const [worktreeId, manualOrder] of order.updates) { + const entry = updates.find((candidate) => candidate.worktreeId === worktreeId) + if (entry) { + entry.updates = { ...entry.updates, ...manualOrder } + continue + } + const current = args.worktreeById.get(worktreeId) + if (current) { + updates.push({ + worktreeId, + updates: manualOrder, + executionHostId: current.hostId ?? 'local' + }) } } - if (writeManualOrder) { - for (const [worktreeId, manualOrder] of order.updates) { - updates.set(worktreeId, { ...updates.get(worktreeId), ...manualOrder }) - } - } - if (updates.size === 0) { + const changed = updates.filter((entry) => Object.keys(entry.updates).length > 0) + if (changed.length === 0) { return } if (writeManualOrder && order.changed) { args.setSortBy('manual') } recordInteraction() - void args.updateWorktreesMeta(updates) + void args.updateWorktreesMeta(changed) args.maybeSyncTaskStatuses(drop.worktreeIds, drop.status) }, [args, shouldWriteDropManualOrder] @@ -157,20 +183,28 @@ export function useWorkspaceKanbanWorktreeActions(args: { if (!current || current.isPinned) { return } - void args.updateWorktreeMeta(worktreeId, { isPinned: true }) + void args.updateWorktreeMeta( + worktreeId, + { isPinned: true }, + { executionHostId: current.hostId ?? 'local' } + ) }, [args] ) const pinWorktrees = useCallback( (worktreeIds: readonly string[]) => { - const updates = new Map() + const updates: WorktreeMetaBatchUpdate[] = [] for (const worktreeId of worktreeIds) { const current = args.worktreeById.get(worktreeId) if (current && !current.isPinned) { - updates.set(worktreeId, { isPinned: true }) + updates.push({ + worktreeId, + updates: { isPinned: true }, + executionHostId: current.hostId ?? 'local' + }) } } - if (updates.size > 0) { + if (updates.length > 0) { recordInteraction() void args.updateWorktreesMeta(updates) } diff --git a/src/renderer/src/components/sidebar/use-worktree-card-activation-actions.ts b/src/renderer/src/components/sidebar/use-worktree-card-activation-actions.ts index 84874e4fa54..cc8f6002e59 100644 --- a/src/renderer/src/components/sidebar/use-worktree-card-activation-actions.ts +++ b/src/renderer/src/components/sidebar/use-worktree-card-activation-actions.ts @@ -107,9 +107,13 @@ export function useWorktreeCardActivationActions({ // Inline rename has no surface for the failure; the store already logs and // refetches, which reverts the optimistic title in place. async (displayName: string): Promise => { - await updateWorktreeMeta(worktree.id, { displayName }) + await updateWorktreeMeta( + worktree.id, + { displayName }, + { executionHostId: worktree.hostId ?? 'local' } + ) }, - [updateWorktreeMeta, worktree.id] + [updateWorktreeMeta, worktree.hostId, worktree.id] ) const handleDoubleClick = useCallback( @@ -123,6 +127,7 @@ export function useWorktreeCardActivationActions({ openModal('edit-meta', { worktreeId: worktree.id, repoId: worktree.repoId, + executionHostId: worktree.hostId, currentDisplayName: worktree.displayName, currentIssue: worktree.linkedIssue, currentPR: worktree.linkedPR, @@ -134,6 +139,7 @@ export function useWorktreeCardActivationActions({ affiliateListMode, worktree.comment, worktree.displayName, + worktree.hostId, worktree.id, worktree.linkedIssue, worktree.linkedPR, @@ -145,9 +151,13 @@ export function useWorktreeCardActivationActions({ (event: React.MouseEvent) => { event.preventDefault() event.stopPropagation() - updateWorktreeMeta(worktree.id, { isUnread: !worktree.isUnread }) + updateWorktreeMeta( + worktree.id, + { isUnread: !worktree.isUnread }, + { executionHostId: worktree.hostId ?? 'local' } + ) }, - [worktree.id, worktree.isUnread, updateWorktreeMeta] + [worktree.hostId, worktree.id, worktree.isUnread, updateWorktreeMeta] ) return { handleClick, handleRenameTitle, handleDoubleClick, handleToggleUnreadQuick } diff --git a/src/renderer/src/components/sidebar/use-worktree-card-foundation.ts b/src/renderer/src/components/sidebar/use-worktree-card-foundation.ts index ec59f8172e9..fc339bbe33f 100644 --- a/src/renderer/src/components/sidebar/use-worktree-card-foundation.ts +++ b/src/renderer/src/components/sidebar/use-worktree-card-foundation.ts @@ -50,6 +50,7 @@ export function useWorktreeCardFoundation({ // Why: the same workspace ID can exist under two hosts. Naming the owner // keeps the dialog on the clicked row instead of the ambiguous lookup. repoId: worktree.repoId, + executionHostId: worktree.hostId, currentDisplayName: worktree.displayName, currentIssue: worktree.linkedIssue, currentPR: worktree.linkedPR, @@ -66,6 +67,7 @@ export function useWorktreeCardFoundation({ openModal('edit-meta', { worktreeId: worktree.id, repoId: worktree.repoId, + executionHostId: worktree.hostId, currentDisplayName: worktree.displayName, currentIssue: worktree.linkedIssue, currentPR: worktree.linkedPR, diff --git a/src/renderer/src/components/sidebar/use-worktree-card-secondary-details.ts b/src/renderer/src/components/sidebar/use-worktree-card-secondary-details.ts index 64e9da831a6..e5ffe899c8d 100644 --- a/src/renderer/src/components/sidebar/use-worktree-card-secondary-details.ts +++ b/src/renderer/src/components/sidebar/use-worktree-card-secondary-details.ts @@ -149,27 +149,28 @@ export function useWorktreeCardSecondaryDetails({ (hoverReviewProvider === 'azure-devops' && linkedAzureDevOpsPR !== null) || (hoverReviewProvider === 'gitea' && linkedGiteaPR !== null) const handleUnlinkReview = useCallback(() => { + const options = { executionHostId: worktree.hostId ?? 'local' } switch (hoverReviewProvider) { case 'github': - void updateWorktreeMeta(worktree.id, { linkedPR: null }) + void updateWorktreeMeta(worktree.id, { linkedPR: null }, options) return case 'gitlab': - void updateWorktreeMeta(worktree.id, { linkedGitLabMR: null }) + void updateWorktreeMeta(worktree.id, { linkedGitLabMR: null }, options) return case 'bitbucket': - void updateWorktreeMeta(worktree.id, { linkedBitbucketPR: null }) + void updateWorktreeMeta(worktree.id, { linkedBitbucketPR: null }, options) return case 'azure-devops': - void updateWorktreeMeta(worktree.id, { linkedAzureDevOpsPR: null }) + void updateWorktreeMeta(worktree.id, { linkedAzureDevOpsPR: null }, options) return case 'gitea': - void updateWorktreeMeta(worktree.id, { linkedGiteaPR: null }) + void updateWorktreeMeta(worktree.id, { linkedGiteaPR: null }, options) break case 'unsupported': case undefined: break } - }, [hoverReviewProvider, updateWorktreeMeta, worktree.id]) + }, [hoverReviewProvider, updateWorktreeMeta, worktree.hostId, worktree.id]) const handleOpenLinearIssueInOrca = useCallback( (e: React.MouseEvent) => { e.stopPropagation() diff --git a/src/renderer/src/components/sidebar/use-worktree-context-menu-commands.ts b/src/renderer/src/components/sidebar/use-worktree-context-menu-commands.ts index 41320c81c02..7fb0790267b 100644 --- a/src/renderer/src/components/sidebar/use-worktree-context-menu-commands.ts +++ b/src/renderer/src/components/sidebar/use-worktree-context-menu-commands.ts @@ -42,7 +42,11 @@ export function useWorktreeContextMenuCommands(args: { window.api.ui.writeClipboardText(args.worktree.path) }, [args]) const handleToggleRead = useCallback(() => { - args.updateWorktreeMeta(args.worktree.id, { isUnread: !args.worktree.isUnread }) + args.updateWorktreeMeta( + args.worktree.id, + { isUnread: !args.worktree.isUnread }, + { executionHostId: args.worktree.hostId ?? 'local' } + ) }, [args]) const handleTogglePin = useCallback(() => { args.setWorktreesPinnedAndReveal([args.worktree.id], !args.worktree.isPinned) @@ -100,8 +104,17 @@ export function useWorktreeContextMenuCommands(args: { args.onAssignWorkspaceStatus?.(plan.worktreeIds, status) return } + const localWriteIds = new Set(plan.localWriteIds) void Promise.all( - plan.localWriteIds.map((id) => args.updateWorktreeMeta(id, { workspaceStatus: status })) + args.activeContextWorktrees + .filter((worktree) => localWriteIds.has(worktree.id)) + .map((worktree) => + args.updateWorktreeMeta( + worktree.id, + { workspaceStatus: status }, + { executionHostId: worktree.hostId ?? 'local' } + ) + ) ) }, [args] @@ -110,6 +123,7 @@ export function useWorktreeContextMenuCommands(args: { args.openModal('edit-meta', { worktreeId: args.worktree.id, repoId: args.worktree.repoId, + executionHostId: args.worktree.hostId, currentDisplayName: args.worktree.displayName, currentIssue: args.worktree.linkedIssue, currentPR: args.worktree.linkedPR, diff --git a/src/renderer/src/components/sidebar/workspace-kanban-sidebar-drop.test.ts b/src/renderer/src/components/sidebar/workspace-kanban-sidebar-drop.test.ts index 0a333ca9180..4edd77c6357 100644 --- a/src/renderer/src/components/sidebar/workspace-kanban-sidebar-drop.test.ts +++ b/src/renderer/src/components/sidebar/workspace-kanban-sidebar-drop.test.ts @@ -574,7 +574,9 @@ describe('workspace kanban sidebar drop updates', () => { }) expect(result.shouldSwitchToManual).toBe(false) - expect(Array.from(result.updates)).toEqual([['todo-a', { workspaceStatus: 'doing' }]]) + expect(result.updates).toEqual([ + { worktreeId: 'todo-a', updates: { workspaceStatus: 'doing' }, executionHostId: 'local' } + ]) }) it('keeps the dropped board position when Manual sort is active', () => { @@ -601,12 +603,16 @@ describe('workspace kanban sidebar drop updates', () => { }) expect(result.shouldSwitchToManual).toBe(true) - expect(result.updates.get('todo-a')).toEqual({ + expect(result.updates.find((entry) => entry.worktreeId === 'todo-a')?.updates).toEqual({ workspaceStatus: 'doing', manualOrder: 9000 }) - expect(result.updates.get('doing-a')).toEqual({ manualOrder: 10_000 }) - expect(result.updates.get('doing-b')).toEqual({ manualOrder: 8000 }) + expect(result.updates.find((entry) => entry.worktreeId === 'doing-a')?.updates).toEqual({ + manualOrder: 10_000 + }) + expect(result.updates.find((entry) => entry.worktreeId === 'doing-b')?.updates).toEqual({ + manualOrder: 8000 + }) }) it('keeps board drops sparse when filtered rows already have durable ranks', () => { @@ -662,8 +668,12 @@ describe('workspace kanban sidebar drop updates', () => { now: 10_000 }) - expect([...result.updates]).toEqual([ - ['todo-a', { workspaceStatus: 'doing', manualOrder: 1500 }] + expect(result.updates).toEqual([ + { + worktreeId: 'todo-a', + updates: { workspaceStatus: 'doing', manualOrder: 1500 }, + executionHostId: 'local' + } ]) }) }) diff --git a/src/renderer/src/components/sidebar/workspace-kanban-sidebar-drop.ts b/src/renderer/src/components/sidebar/workspace-kanban-sidebar-drop.ts index 8672bbc5582..d0db89c9faa 100644 --- a/src/renderer/src/components/sidebar/workspace-kanban-sidebar-drop.ts +++ b/src/renderer/src/components/sidebar/workspace-kanban-sidebar-drop.ts @@ -14,6 +14,7 @@ import { shouldWriteManualOrderForGroupDrop, type WorktreeDragGroup } from './worktree-manual-order' +import type { WorktreeMetaBatchUpdate } from '../../store/slices/worktree-helpers' import { CARD_SELECTOR, getCardDropTarget, @@ -221,7 +222,7 @@ export function buildWorkspaceKanbanSidebarDropUpdates(args: { sortBy: string now: number }): { - updates: Map> + updates: WorktreeMetaBatchUpdate[] shouldSwitchToManual: boolean } { const sourceGroupKeys = args.worktreeIds.flatMap((worktreeId) => { @@ -245,25 +246,41 @@ export function buildWorkspaceKanbanSidebarDropUpdates(args: { }) : { changed: false, updates: new Map() } - const updates = new Map>() + const updates: WorktreeMetaBatchUpdate[] = [] for (const worktreeId of args.worktreeIds) { const current = args.worktreeById.get(worktreeId) if (!current) { continue } + const next: Partial = {} if (getWorkspaceStatus(current, args.workspaceStatuses) !== args.status) { - updates.set(worktreeId, { workspaceStatus: args.status }) + next.workspaceStatus = args.status } + updates.push({ + worktreeId, + updates: next, + executionHostId: current.hostId ?? 'local' + }) } if (writeManualOrder) { for (const [worktreeId, manualOrder] of order.updates) { - updates.set(worktreeId, { ...updates.get(worktreeId), ...manualOrder }) + const current = args.worktreeById.get(worktreeId) + const entry = updates.find((candidate) => candidate.worktreeId === worktreeId) + if (entry) { + entry.updates = { ...entry.updates, ...manualOrder } + } else if (current) { + updates.push({ + worktreeId, + updates: manualOrder, + executionHostId: current.hostId ?? 'local' + }) + } } } return { - updates, + updates: updates.filter((entry) => Object.keys(entry.updates).length > 0), shouldSwitchToManual: writeManualOrder && order.changed } } diff --git a/src/renderer/src/components/sidebar/worktree-list/drag/use-status-mutations.ts b/src/renderer/src/components/sidebar/worktree-list/drag/use-status-mutations.ts index 435d8e34cf2..6243f34cf02 100644 --- a/src/renderer/src/components/sidebar/worktree-list/drag/use-status-mutations.ts +++ b/src/renderer/src/components/sidebar/worktree-list/drag/use-status-mutations.ts @@ -6,6 +6,7 @@ import type { Worktree } from '../../../../../../shared/worktree/types' import type { WorktreeMeta } from '../../../../../../shared/worktree/meta-types' +import type { WorktreeMetaBatchUpdate } from '../../../../store/slices/worktree-helpers' import { getWorkspaceStatus, getWorkspaceStatusGroupKey } from '../../workspace-status' import { buildManualOrderUpdatesForGroupDrop, @@ -37,22 +38,30 @@ export function useWorktreeStatusMutations(args: { if (!current || getWorkspaceStatus(current, workspaceStatuses) === status) { return } - void updateWorktreeMeta(worktreeId, { workspaceStatus: status }) + void updateWorktreeMeta( + worktreeId, + { workspaceStatus: status }, + { executionHostId: current.hostId ?? 'local' } + ) }, [updateWorktreeMeta, worktreeMap, workspaceStatuses] ) const moveWorktreesToStatus = useCallback( (worktreeIds: readonly string[], status: WorkspaceStatus) => { - const updates = new Map() + const updates: WorktreeMetaBatchUpdate[] = [] for (const worktreeId of worktreeIds) { const current = worktreeMap.get(worktreeId) if (!current || getWorkspaceStatus(current, workspaceStatuses) === status) { continue } - updates.set(worktreeId, { workspaceStatus: status }) + updates.push({ + worktreeId, + updates: { workspaceStatus: status }, + executionHostId: current.hostId ?? 'local' + }) } - if (updates.size > 0) { + if (updates.length > 0) { void updateWorktreesMeta(updates) } }, @@ -70,7 +79,7 @@ export function useWorktreeStatusMutations(args: { rankByWorktreeId: manualOrderCatalog.rankByWorktreeId, allWorktreeIds: manualOrderCatalog.orderedIds }) - const updates = new Map>() + const updates = new Map() for (const worktreeId of dropArgs.worktreeIds) { const current = worktreeMap.get(worktreeId) if (!current) { @@ -80,13 +89,20 @@ export function useWorktreeStatusMutations(args: { if (getWorkspaceStatus(current, workspaceStatuses) !== dropArgs.status) { next.workspaceStatus = dropArgs.status } - updates.set(worktreeId, next) + updates.set(worktreeId, { + worktreeId, + updates: next, + executionHostId: current.hostId ?? 'local' + }) } for (const [worktreeId, manualOrder] of order.updates) { - updates.set(worktreeId, { ...updates.get(worktreeId), ...manualOrder }) + const entry = updates.get(worktreeId) + if (entry) { + entry.updates = { ...entry.updates, ...manualOrder } + } } - for (const [worktreeId, update] of Array.from(updates)) { - if (Object.keys(update).length === 0) { + for (const [worktreeId, entry] of updates) { + if (Object.keys(entry.updates).length === 0) { updates.delete(worktreeId) } } @@ -97,7 +113,7 @@ export function useWorktreeStatusMutations(args: { if (order.changed) { setSortBy('manual') } - void updateWorktreesMeta(updates) + void updateWorktreesMeta([...updates.values()]) }, [manualOrderCatalog, setSortBy, updateWorktreesMeta, worktreeMap, workspaceStatuses] ) @@ -129,14 +145,18 @@ export function useWorktreeStatusMutations(args: { rankByWorktreeId: manualOrderCatalog.rankByWorktreeId, allWorktreeIds: manualOrderCatalog.orderedIds }) - if (!result.changed) { - return + if (result.changed) { + setSortBy('manual') } - // Why: only switch to Manual after a real move so accidental click-drags don't change the sort. - setSortBy('manual') - void updateWorktreesMeta(result.updates) + void updateWorktreesMeta( + [...result.updates].map(([worktreeId, updates]) => ({ + worktreeId, + updates, + executionHostId: worktreeMap.get(worktreeId)?.hostId ?? 'local' + })) + ) }, - [manualOrderCatalog, setSortBy, updateWorktreesMeta] + [manualOrderCatalog, setSortBy, updateWorktreesMeta, worktreeMap] ) const shouldShowWorkspaceBoardDropIndicator = useCallback( @@ -165,7 +185,7 @@ export function useWorktreeStatusMutations(args: { allWorktreeIds: manualOrderCatalog.orderedIds, rankByWorktreeId: manualOrderCatalog.rankByWorktreeId }) - if (result.updates.size === 0) { + if (result.updates.length === 0) { return } // Why: switch to Manual when the drop changes order so the placement stays visible. diff --git a/src/renderer/src/components/sidebar/worktree-unambiguous-id-index.test.ts b/src/renderer/src/components/sidebar/worktree-unambiguous-id-index.test.ts new file mode 100644 index 00000000000..5c6afdd3199 --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-unambiguous-id-index.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from 'vitest' +import { buildUnambiguousWorktreeIdIndex } from './worktree-unambiguous-id-index' +import type { Worktree } from '../../../../shared/worktree/types' + +function worktree(id: string, hostId: Worktree['hostId']): Worktree { + return { + id, + repoId: 'repo', + path: `/repo/${id}`, + branch: id, + isMainWorktree: false, + hostId + } as Worktree +} + +describe('buildUnambiguousWorktreeIdIndex', () => { + it('keeps unique bare ids', () => { + const local = worktree('repo::local', 'local') + const remote = worktree('repo::remote', 'ssh:host') + + expect(buildUnambiguousWorktreeIdIndex([local, remote])).toEqual( + new Map([ + ['repo::local', local], + ['repo::remote', remote] + ]) + ) + }) + + it('drops bare ids claimed by multiple hosts', () => { + const local = worktree('repo::feature', 'local') + const remote = worktree('repo::feature', 'ssh:host') + + expect(buildUnambiguousWorktreeIdIndex([local, remote]).has('repo::feature')).toBe(false) + }) + + it('keeps later unique ids after an ambiguity is found', () => { + const local = worktree('repo::feature', 'local') + const remote = worktree('repo::feature', 'ssh:host') + const other = worktree('repo::other', 'local') + + expect(buildUnambiguousWorktreeIdIndex([local, remote, other]).get('repo::other')).toBe(other) + }) +}) diff --git a/src/renderer/src/components/sidebar/worktree-unambiguous-id-index.ts b/src/renderer/src/components/sidebar/worktree-unambiguous-id-index.ts new file mode 100644 index 00000000000..ce3f2fcf91b --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-unambiguous-id-index.ts @@ -0,0 +1,17 @@ +import type { Worktree } from '../../../../shared/worktree/types' + +export function buildUnambiguousWorktreeIdIndex( + worktrees: readonly Worktree[] +): Map { + const index = new Map() + const ambiguous = new Set() + for (const worktree of worktrees) { + if (index.has(worktree.id)) { + index.delete(worktree.id) + ambiguous.add(worktree.id) + } else if (!ambiguous.has(worktree.id)) { + index.set(worktree.id, worktree) + } + } + return index +} diff --git a/src/renderer/src/store/slices/folder-workspace-owner-routed-mutations.test.ts b/src/renderer/src/store/slices/folder-workspace-owner-routed-mutations.test.ts index cf15e35e885..91c78d42a82 100644 --- a/src/renderer/src/store/slices/folder-workspace-owner-routed-mutations.test.ts +++ b/src/renderer/src/store/slices/folder-workspace-owner-routed-mutations.test.ts @@ -79,11 +79,12 @@ describe('folder workspace owner-routed mutations', () => { folderWorkspaces: [folderWorkspace] }) - await store - .getState() - .updateWorktreesMeta( - new Map([[folderWorkspaceKey(folderWorkspace.id), { manualOrder: 9000 }]]) - ) + await store.getState().updateWorktreesMeta([ + { + worktreeId: folderWorkspaceKey(folderWorkspace.id), + updates: { manualOrder: 9000 } + } + ]) expect(folderWorkspacesUpdate).toHaveBeenCalledWith({ folderWorkspaceId: folderWorkspace.id, diff --git a/src/renderer/src/store/slices/worktree-helpers.ts b/src/renderer/src/store/slices/worktree-helpers.ts index fb368bec070..5d4fafd1e22 100644 --- a/src/renderer/src/store/slices/worktree-helpers.ts +++ b/src/renderer/src/store/slices/worktree-helpers.ts @@ -37,7 +37,6 @@ import type { PendingWorktreeCreation, WorktreeCreationPhase } from '@/lib/pending-worktree-creation' -import { getRepoIdFromWorktreeId } from '../../../../shared/worktree/id' import type { AppState } from '../types' import type { WorktreeRefreshAllOptions } from './worktree-refresh-options' export type { WorktreePurgeTarget, WorktreePurgeTargets } from './worktree-purge-target' @@ -46,6 +45,10 @@ export type { WorktreeDeleteState, WorktreeDeleteStateTarget } from './worktree- import type { WorktreeDeleteState, WorktreeDeleteStateTarget } from './worktree-delete-state-types' export { getRepoIdFromWorktreeId } from '../../../../shared/worktree/id' +export { + applyWorktreeUpdates, + withoutErasedRequiredWorktreeFields +} from './worktree-meta-update-application' import type { RendererRemoveWorktreeResult } from './renderer-remove-worktree-result' export type WorktreeFetchOptions = { @@ -64,10 +67,17 @@ export type DirectSshWorktreeFetchOptions = WorktreeFetchOptions & { export type WorktreeMetaUpdateGuard = (worktree: Worktree | DetectedWorktree | undefined) => boolean export type WorktreeMetaUpdateOptions = { + /** Required to mutate one row when the legacy locator exists on multiple hosts. */ + executionHostId?: ExecutionHostId shouldApply?: WorktreeMetaUpdateGuard /** Skip the automatic review refetch when the caller owns an equivalent refresh. */ suppressHostedReviewRefresh?: boolean } +export type WorktreeMetaBatchUpdate = { + worktreeId: string + updates: Partial + executionHostId?: ExecutionHostId +} export type WorktreeRenameRequest = { worktreeId: string @@ -264,9 +274,7 @@ export type WorktreeSlice = { options?: WorktreeMetaUpdateOptions ) => Promise<{ ok: true } | { ok: false; error: string }> ensureHostedReviewPushTarget: (worktreeId: string) => Promise - updateWorktreesMeta: ( - updatesByWorktreeId: ReadonlyMap> - ) => Promise + updateWorktreesMeta: (updatesByWorktreeId: readonly WorktreeMetaBatchUpdate[]) => Promise /** * Pin/unpin worktrees, then reveal the first changed one. The reveal keeps * the shortcut action visible even though pinned worktrees also remain in @@ -369,68 +377,3 @@ export function findWorktreeById( return undefined } - -type RequiredKey = { [K in keyof T]-?: undefined extends T[K] ? never : K }[keyof T] - -// Why: a present-but-undefined key in a spread ERASES the field. That is the -// intended wire signal for clearing optional metadata (pushTarget), but on a -// field Worktree declares required it produced a live `displayName: undefined` -// that crashed the worktree palette (crash a1f81ea1). Typed off Worktree so a -// newly-required field is protected automatically. -const ERASURE_PROTECTED_KEYS: Record, keyof WorktreeMeta>, true> = { - displayName: true, - comment: true, - linkedIssue: true, - linkedPR: true, - linkedLinearIssue: true, - isArchived: true, - isUnread: true, - isPinned: true, - sortOrder: true, - lastActivityAt: true -} - -export function withoutErasedRequiredWorktreeFields( - updates: Partial -): Partial { - const erased = Object.keys(ERASURE_PROTECTED_KEYS).filter( - (key) => updates[key as keyof WorktreeMeta] === undefined && Object.hasOwn(updates, key) - ) - if (erased.length === 0) { - return updates - } - - const next = { ...updates } - for (const key of erased) { - delete next[key as keyof WorktreeMeta] - } - return next -} - -export function applyWorktreeUpdates( - worktreesByRepo: Record, - worktreeId: string, - rawUpdates: Partial -): Record { - const updates = withoutErasedRequiredWorktreeFields(rawUpdates) - const repoId = getRepoIdFromWorktreeId(worktreeId) - const worktrees = worktreesByRepo[repoId] - if (!worktrees) { - return worktreesByRepo - } - - let changed = false - const nextWorktrees = worktrees.map((worktree) => { - if (worktree.id !== worktreeId) { - return worktree - } - - changed = true - return { ...worktree, ...updates } - }) - if (!changed) { - return worktreesByRepo - } - - return { ...worktreesByRepo, [repoId]: nextWorktrees } -} diff --git a/src/renderer/src/store/slices/worktree-meta-update-application.ts b/src/renderer/src/store/slices/worktree-meta-update-application.ts new file mode 100644 index 00000000000..a65309f9adf --- /dev/null +++ b/src/renderer/src/store/slices/worktree-meta-update-application.ts @@ -0,0 +1,69 @@ +import type { WorktreeMeta } from '../../../../shared/worktree/meta-types' +import { getRepoIdFromWorktreeId } from '../../../../shared/worktree/id' +import type { Worktree } from '../../../../shared/worktree/types' +import type { ExecutionHostId } from '../../../../shared/execution-host' +import { worktreeRowMatchesMetaHost } from './worktrees/listing/worktree-meta-host-match' + +type RequiredKey = { [K in keyof T]-?: undefined extends T[K] ? never : K }[keyof T] + +// Why: a present-but-undefined key in a spread ERASES the field. That is the +// intended wire signal for clearing optional metadata, but this protects required +// Worktree fields from disappearing in optimistic projections. +const ERASURE_PROTECTED_KEYS: Record, keyof WorktreeMeta>, true> = { + displayName: true, + comment: true, + linkedIssue: true, + linkedPR: true, + linkedLinearIssue: true, + isArchived: true, + isUnread: true, + isPinned: true, + sortOrder: true, + lastActivityAt: true +} + +export function withoutErasedRequiredWorktreeFields( + updates: Partial +): Partial { + const erased = Object.keys(ERASURE_PROTECTED_KEYS).filter( + (key) => updates[key as keyof WorktreeMeta] === undefined && Object.hasOwn(updates, key) + ) + if (erased.length === 0) { + return updates + } + + const next = { ...updates } + for (const key of erased) { + delete next[key as keyof WorktreeMeta] + } + return next +} + +export function applyWorktreeUpdates( + worktreesByRepo: Record, + worktreeId: string, + rawUpdates: Partial, + executionHostId?: ExecutionHostId +): Record { + const updates = withoutErasedRequiredWorktreeFields(rawUpdates) + const repoId = getRepoIdFromWorktreeId(worktreeId) + const worktrees = worktreesByRepo[repoId] + if (!worktrees) { + return worktreesByRepo + } + + let changed = false + const nextWorktrees = worktrees.map((worktree) => { + if (worktree.id !== worktreeId || !worktreeRowMatchesMetaHost(worktree, executionHostId)) { + return worktree + } + + changed = true + return { ...worktree, ...updates } + }) + if (!changed) { + return worktreesByRepo + } + + return { ...worktreesByRepo, [repoId]: nextWorktrees } +} diff --git a/src/renderer/src/store/slices/worktrees-activity-persistence.test.ts b/src/renderer/src/store/slices/worktrees-activity-persistence.test.ts index c2f9e18b4fd..f1fdbfc2a5e 100644 --- a/src/renderer/src/store/slices/worktrees-activity-persistence.test.ts +++ b/src/renderer/src/store/slices/worktrees-activity-persistence.test.ts @@ -102,7 +102,7 @@ describe('worktree remote runtime mutations', () => { [ 'updateWorktreesMeta', (store: ReturnType, worktreeId: string) => - store.getState().updateWorktreesMeta(new Map([[worktreeId, { isUnread: true }]])) + store.getState().updateWorktreesMeta([{ worktreeId, updates: { isUnread: true } }]) ], [ 'markWorktreeUnread', diff --git a/src/renderer/src/store/slices/worktrees-fetch-listing-merge.test.ts b/src/renderer/src/store/slices/worktrees-fetch-listing-merge.test.ts index 3e295c7c73b..0e3ead18d88 100644 --- a/src/renderer/src/store/slices/worktrees-fetch-listing-merge.test.ts +++ b/src/renderer/src/store/slices/worktrees-fetch-listing-merge.test.ts @@ -282,12 +282,10 @@ describe('fetchWorktrees', () => { const refresh = store.getState().fetchWorktrees('repo1') await vi.waitFor(() => expect(worktreeListMock).toHaveBeenCalledTimes(1)) - await store.getState().updateWorktreesMeta( - new Map([ - [daily.id, { manualOrder: 100 }], - [relay.id, { manualOrder: 200 }] - ]) - ) + await store.getState().updateWorktreesMeta([ + { worktreeId: daily.id, updates: { manualOrder: 100 } }, + { worktreeId: relay.id, updates: { manualOrder: 200 } } + ]) resolveListing([refreshedDaily, relay]) await refresh diff --git a/src/renderer/src/store/slices/worktrees-linked-review-push-target.test.ts b/src/renderer/src/store/slices/worktrees-linked-review-push-target.test.ts index df136f1ee3a..09f191ac732 100644 --- a/src/renderer/src/store/slices/worktrees-linked-review-push-target.test.ts +++ b/src/renderer/src/store/slices/worktrees-linked-review-push-target.test.ts @@ -65,6 +65,7 @@ describe('worktree remote runtime mutations', () => { }) expect(mockApi.worktrees.updateMeta).toHaveBeenCalledWith({ worktreeId: wt.id, + executionHostId: wt.hostId ?? 'local', updates: { linkedPR: 2548, pushTarget } }) expect(store.getState().worktreesByRepo.repo1[0]?.pushTarget).toEqual(pushTarget) @@ -88,9 +89,9 @@ describe('worktree remote runtime mutations', () => { } as Partial) await store.getState().updateWorktreeMeta(wt.id, { linkedPR: null }) - expect(mockApi.worktrees.updateMeta).toHaveBeenCalledWith({ worktreeId: wt.id, + executionHostId: wt.hostId ?? 'local', updates: { linkedPR: null, pushTarget: undefined } }) expect(store.getState().worktreesByRepo.repo1[0]?.pushTarget).toBeUndefined() @@ -119,9 +120,9 @@ describe('worktree remote runtime mutations', () => { .getState() .updateWorktreeMeta(wt.id, { linkedPR: null }, { suppressHostedReviewRefresh: true }) - expect(fetchHostedReviewForBranch).not.toHaveBeenCalled() expect(mockApi.worktrees.updateMeta).toHaveBeenCalledWith({ worktreeId: wt.id, + executionHostId: wt.hostId ?? 'local', updates: { linkedPR: null, pushTarget: undefined } }) expect(store.getState().worktreesByRepo.repo1[0]).toMatchObject({ @@ -152,9 +153,9 @@ describe('worktree remote runtime mutations', () => { } as Partial) await store.getState().updateWorktreeMeta(wt.id, { linkedGitLabMR: 42 }) - expect(mockApi.worktrees.updateMeta).toHaveBeenCalledWith({ worktreeId: wt.id, + executionHostId: wt.hostId ?? 'local', updates: { linkedGitLabMR: 42, linkedPR: null, pushTarget: undefined } }) expect(store.getState().worktreesByRepo.repo1[0]).toMatchObject({ @@ -186,6 +187,7 @@ describe('worktree remote runtime mutations', () => { }) expect(mockApi.worktrees.updateMeta).toHaveBeenCalledWith({ worktreeId: wt.id, + executionHostId: wt.hostId ?? 'local', updates: { pushTarget: newPushTarget } }) expect(store.getState().worktreesByRepo.repo1[0]?.pushTarget).toEqual(newPushTarget) @@ -300,9 +302,9 @@ describe('worktree remote runtime mutations', () => { await store.getState().updateWorktreeMeta(wt.id, { linkedPR: 0 }) - expect(mockApi.worktrees.resolvePrBase).not.toHaveBeenCalled() expect(mockApi.worktrees.updateMeta).toHaveBeenCalledWith({ worktreeId: wt.id, + executionHostId: wt.hostId ?? 'local', updates: { linkedPR: 0 } }) }) @@ -326,9 +328,9 @@ describe('worktree remote runtime mutations', () => { await store.getState().updateWorktreeMeta(wt.id, { linkedPR: 2548 }) - expect(mockApi.worktrees.resolvePrBase).not.toHaveBeenCalled() expect(mockApi.worktrees.updateMeta).toHaveBeenCalledWith({ worktreeId: wt.id, + executionHostId: wt.hostId ?? 'local', updates: { linkedPR: 2548 } }) }) @@ -361,6 +363,7 @@ describe('worktree remote runtime mutations', () => { }) expect(mockApi.worktrees.updateMeta).toHaveBeenCalledWith({ worktreeId: wt.id, + executionHostId: wt.hostId ?? 'local', updates: { linkedPR: 2548, pushTarget } }) expect(store.getState().worktreesByRepo.repo1[0]?.pushTarget).toEqual(pushTarget) @@ -397,6 +400,7 @@ describe('worktree remote runtime mutations', () => { }) expect(mockApi.worktrees.updateMeta).toHaveBeenCalledWith({ worktreeId: wt.id, + executionHostId: wt.hostId ?? 'local', updates: { pushTarget } }) expect(store.getState().worktreesByRepo.repo1[0]?.pushTarget).toEqual(pushTarget) @@ -520,7 +524,8 @@ describe('worktree remote runtime mutations', () => { id: 'repo-ssh::/home/orca/runtime-wt', repoId: 'repo-ssh', path: '/home/orca/runtime-wt', - linkedPR: 5571 + linkedPR: 5571, + hostId: 'ssh:ssh-1' }) mockApi.worktrees.resolvePrBase.mockResolvedValueOnce({ baseBranch: 'fork-head', @@ -549,6 +554,7 @@ describe('worktree remote runtime mutations', () => { }) expect(mockApi.worktrees.updateMeta).toHaveBeenCalledWith({ worktreeId: wt.id, + executionHostId: wt.hostId ?? 'local', updates: { pushTarget } }) expect(runtimeEnvironmentCall).not.toHaveBeenCalled() @@ -607,6 +613,51 @@ describe('worktree remote runtime mutations', () => { expect(mockApi.worktrees.resolvePrBase).not.toHaveBeenCalled() expect(mockApi.worktrees.updateMeta).not.toHaveBeenCalled() }) + it('updates only the explicitly selected owner when locators collide', async () => { + const store = createTestStore() + const worktreeId = 'repo-shared::/same/path' + const sshA = makeWorktree({ + id: worktreeId, + repoId: 'repo-shared', + hostId: 'ssh:ssh-a', + runtimeOwnerEnvironmentId: 'hub-a', + comment: 'A' + }) + const sshB = makeWorktree({ + id: worktreeId, + repoId: 'repo-shared', + hostId: 'ssh:ssh-b', + runtimeOwnerEnvironmentId: 'hub-b', + comment: 'B' + }) + runtimeEnvironmentCall.mockResolvedValue({ + id: 'rpc-set-selected-owner', + ok: true, + result: { worktree: sshB }, + _meta: { runtimeId: 'hub-b' } + }) + store.setState({ + settings: { activeRuntimeEnvironmentId: 'hub-a' } as never, + worktreesByRepo: { 'repo-shared': [sshA, sshB] } + } as Partial) + + const result = await store + .getState() + .updateWorktreeMeta(worktreeId, { comment: 'selected B' }, { executionHostId: 'ssh:ssh-b' }) + + expect(result).toEqual({ ok: true }) + expect(store.getState().worktreesByRepo['repo-shared']).toEqual([ + expect.objectContaining({ hostId: 'ssh:ssh-a', comment: 'A' }), + expect.objectContaining({ hostId: 'ssh:ssh-b', comment: 'selected B' }) + ]) + expect(runtimeEnvironmentCall).toHaveBeenCalledWith( + expect.objectContaining({ + selector: 'hub-b', + method: 'worktree.set', + params: expect.objectContaining({ comment: 'selected B' }) + }) + ) + }) it('cleans up the in-flight lookup when restore is skipped for a genuinely ambiguous owner', async () => { const store = createTestStore() @@ -703,6 +754,7 @@ describe('worktree remote runtime mutations', () => { }) expect(mockApi.worktrees.updateMeta).toHaveBeenCalledWith({ worktreeId: wt.id, + executionHostId: wt.hostId ?? 'local', updates: { pushTarget } }) expect(store.getState().worktreesByRepo.repo1[0]?.pushTarget).toEqual(pushTarget) diff --git a/src/renderer/src/store/slices/worktrees-metadata-persistence.test.ts b/src/renderer/src/store/slices/worktrees-metadata-persistence.test.ts index 52cbef1b691..fb7b28c01e8 100644 --- a/src/renderer/src/store/slices/worktrees-metadata-persistence.test.ts +++ b/src/renderer/src/store/slices/worktrees-metadata-persistence.test.ts @@ -155,7 +155,8 @@ describe('worktree remote runtime mutations', () => { const wt = makeWorktree({ id: 'repo-ssh::/home/orca/wt1', repoId: 'repo-ssh', - path: '/home/orca/wt1' + path: '/home/orca/wt1', + hostId: 'ssh:ssh-1' }) store.setState({ settings: { activeRuntimeEnvironmentId: 'env-1' } as never, @@ -176,6 +177,7 @@ describe('worktree remote runtime mutations', () => { expect(mockApi.worktrees.updateMeta).toHaveBeenCalledWith({ worktreeId: wt.id, + executionHostId: 'ssh:ssh-1', updates: expect.objectContaining({ comment: 'ssh note' }) }) expect(runtimeEnvironmentCall).not.toHaveBeenCalled() @@ -199,6 +201,7 @@ describe('worktree remote runtime mutations', () => { expect(mockApi.worktrees.updateMeta).toHaveBeenCalledWith({ worktreeId: wt.id, + executionHostId: 'local', updates: { displayName: 'Fix auth', pendingFirstAgentMessageRename: false, @@ -378,12 +381,10 @@ describe('worktree remote runtime mutations', () => { } as Partial) const unsubscribe = store.subscribe(subscriber) - await store.getState().updateWorktreesMeta( - new Map([ - [first.id, { workspaceStatus: 'in-review' }], - [second.id, { workspaceStatus: 'completed' }] - ]) - ) + await store.getState().updateWorktreesMeta([ + { worktreeId: first.id, updates: { workspaceStatus: 'in-review' } }, + { worktreeId: second.id, updates: { workspaceStatus: 'completed' } } + ]) unsubscribe() expect(store.getState().worktreesByRepo.repo1.map((w) => w.workspaceStatus)).toEqual([ @@ -412,13 +413,14 @@ describe('worktree remote runtime mutations', () => { }) store.setState({ worktreesByRepo: { repo1: [local, remote] } } as Partial) - await store.getState().updateWorktreesMeta(new Map([[worktreeId, { manualOrder: 9000 }]])) + await store.getState().updateWorktreesMeta([{ worktreeId, updates: { manualOrder: 9000 } }]) expect(store.getState().worktreesByRepo.repo1.map((row) => row.manualOrder)).toEqual([ 9000, 9000 ]) expect(mockApi.worktrees.updateMeta).toHaveBeenCalledWith({ worktreeId, + executionHostId: 'local', updates: { manualOrder: 9000 } }) expect(runtimeEnvironmentCall).toHaveBeenCalledWith({ @@ -428,4 +430,40 @@ describe('worktree remote runtime mutations', () => { timeoutMs: 15_000 }) }) + it('persists an explicit bulk host update to only the selected owner', async () => { + const store = createTestStore() + const worktreeId = 'repo1::/same/path' + const local = makeWorktree({ id: worktreeId, repoId: 'repo1', hostId: 'local' }) + const remote = makeWorktree({ + id: worktreeId, + repoId: 'repo1', + hostId: 'runtime:env-1' + }) + runtimeEnvironmentCall.mockResolvedValue({ + id: 'rpc-set-selected-owner', + ok: true, + result: { worktree: { ...remote, comment: 'selected' } }, + _meta: { runtimeId: 'runtime-remote' } + }) + store.setState({ worktreesByRepo: { repo1: [local, remote] } } as Partial) + + await store + .getState() + .updateWorktreesMeta([ + { worktreeId, updates: { comment: 'selected' }, executionHostId: 'runtime:env-1' } + ]) + + expect(store.getState().worktreesByRepo.repo1).toEqual([ + expect.objectContaining({ hostId: 'local', comment: '' }), + expect.objectContaining({ hostId: 'runtime:env-1', comment: 'selected' }) + ]) + expect(mockApi.worktrees.updateMeta).not.toHaveBeenCalled() + expect(runtimeEnvironmentCall).toHaveBeenCalledWith( + expect.objectContaining({ + selector: 'env-1', + method: 'worktree.set', + params: expect.objectContaining({ comment: 'selected' }) + }) + ) + }) }) diff --git a/src/renderer/src/store/slices/worktrees-terminal-pr-url-linking.test.ts b/src/renderer/src/store/slices/worktrees-terminal-pr-url-linking.test.ts index 2dfb3acad50..b0456581292 100644 --- a/src/renderer/src/store/slices/worktrees-terminal-pr-url-linking.test.ts +++ b/src/renderer/src/store/slices/worktrees-terminal-pr-url-linking.test.ts @@ -77,6 +77,7 @@ describe('worktree remote runtime mutations', () => { expect(mockApi.worktrees.updateMeta).toHaveBeenCalledWith({ worktreeId: wt.id, + executionHostId: 'local', updates: { linkedPR: 42 } }) }) @@ -123,6 +124,7 @@ describe('worktree remote runtime mutations', () => { expect(mockApi.worktrees.updateMeta).toHaveBeenCalledWith({ worktreeId: wt.id, + executionHostId: 'local', updates: { linkedPR: 42 } }) }) @@ -314,6 +316,7 @@ describe('worktree remote runtime mutations', () => { expect(mockApi.worktrees.updateMeta).toHaveBeenCalledWith({ worktreeId: wt.id, + executionHostId: 'local', updates: { linkedPR: 42 } }) }) diff --git a/src/renderer/src/store/slices/worktrees/listing/detected-worktree-meta.ts b/src/renderer/src/store/slices/worktrees/listing/detected-worktree-meta.ts index d8d32bb78e2..762ad7e882a 100644 --- a/src/renderer/src/store/slices/worktrees/listing/detected-worktree-meta.ts +++ b/src/renderer/src/store/slices/worktrees/listing/detected-worktree-meta.ts @@ -16,10 +16,13 @@ import { worktreeMatchesHost } from './worktree-host-ownership' const folderWorkspaceWorktreeCache = new WeakMap() +import { worktreeRowMatchesMetaHost } from './worktree-meta-host-match' + export function applyDetectedWorktreeUpdates( detectedWorktreesByRepo: AppState['detectedWorktreesByRepo'], worktreeId: string, - rawUpdates: Partial + rawUpdates: Partial, + executionHostId?: ExecutionHostId ): AppState['detectedWorktreesByRepo'] { // Why: mirrors applyWorktreeUpdates — detected rows feed the same palette. const updates = withoutErasedRequiredWorktreeFields(rawUpdates) @@ -29,7 +32,7 @@ export function applyDetectedWorktreeUpdates( for (const [repoId, result] of Object.entries(detectedWorktreesByRepo)) { let repoChanged = false const nextWorktrees = result.worktrees.map((worktree) => { - if (worktree.id !== worktreeId) { + if (worktree.id !== worktreeId || !worktreeRowMatchesMetaHost(worktree, executionHostId)) { return worktree } repoChanged = true diff --git a/src/renderer/src/store/slices/worktrees/listing/worktree-meta-host-match.ts b/src/renderer/src/store/slices/worktrees/listing/worktree-meta-host-match.ts new file mode 100644 index 00000000000..dfb05ab4415 --- /dev/null +++ b/src/renderer/src/store/slices/worktrees/listing/worktree-meta-host-match.ts @@ -0,0 +1,19 @@ +import { + LOCAL_EXECUTION_HOST_ID, + type ExecutionHostId +} from '../../../../../../shared/execution-host' + +/** + * Mirror findKnownWorktreeById: an unstamped row belongs to the local host. A stricter test + * silently no-ops the optimistic write, so a rename appears to do nothing until the next refetch. + */ +export function worktreeRowMatchesMetaHost( + worktree: { hostId?: ExecutionHostId }, + executionHostId: ExecutionHostId | undefined +): boolean { + return ( + executionHostId === undefined || + worktree.hostId === executionHostId || + (worktree.hostId === undefined && executionHostId === LOCAL_EXECUTION_HOST_ID) + ) +} diff --git a/src/renderer/src/store/slices/worktrees/metadata/update-worktree-meta.ts b/src/renderer/src/store/slices/worktrees/metadata/update-worktree-meta.ts index c8f0bae7971..3107d89ab15 100644 --- a/src/renderer/src/store/slices/worktrees/metadata/update-worktree-meta.ts +++ b/src/renderer/src/store/slices/worktrees/metadata/update-worktree-meta.ts @@ -28,13 +28,19 @@ import { trySettingsForWorktreeOwner } from '../listing/worktree-owner-settings' +import { findRepoForHost } from '../../repo-host-identity' export function createUpdateWorktreeMeta( set: WorktreeSliceSet, get: WorktreeSliceGet ): WorktreeSlice['updateWorktreeMeta'] { return async (worktreeId, updates, options) => { const shouldApplyUpdate = options?.shouldApply - const existingWorktree = get().getKnownWorktreeById(worktreeId) + const requestedHostId = options?.executionHostId + const existingWorktree = findKnownWorktreeById(get(), worktreeId, requestedHostId) + const executionHostId = + requestedHostId ?? + existingWorktree?.hostId ?? + (get().settings?.activeRuntimeEnvironmentId ? undefined : 'local') if (shouldApplyUpdate && !shouldApplyUpdate(existingWorktree)) { return { ok: true } } @@ -78,7 +84,7 @@ export function createUpdateWorktreeMeta( normalizedUpdates.pushTarget === undefined && existingWorktree && !existingWorktree.pushTarget - ? trySettingsForWorktreeOwner(get(), worktreeId) + ? trySettingsForWorktreeOwner(get(), worktreeId, executionHostId) : null const resolvedPushTarget = pushTargetOwnerSettings && existingWorktree && linkedPrForPushTarget !== null @@ -101,7 +107,7 @@ export function createUpdateWorktreeMeta( resolvedPushTarget === undefined && existingHostedReviewPushTargetLookup !== null && existingHostedReviewPushTargetLookup.key !== nextHostedReviewPushTargetLookup?.key - const worktreeForUpdate = get().getKnownWorktreeById(worktreeId) + const worktreeForUpdate = get().getKnownWorktreeById(worktreeId, executionHostId) if (shouldApplyUpdate && !shouldApplyUpdate(worktreeForUpdate)) { return { ok: true } } @@ -116,7 +122,10 @@ export function createUpdateWorktreeMeta( (normalizedUpdates.linkedGiteaPR === null && (worktreeForUpdate?.linkedGiteaPR ?? null) !== null) const reviewRepo = shouldRefreshHostedReview - ? get().repos.find((repo) => repo.id === worktreeForUpdate?.repoId) + ? (findRepoForHost(get().repos, worktreeForUpdate?.repoId ?? '', { + hostId: executionHostId, + settings: get().settings + }) ?? undefined) : undefined const reviewBranch = worktreeForUpdate?.branch.replace(/^refs\/heads\//, '') @@ -139,15 +148,24 @@ export function createUpdateWorktreeMeta( let didApply = false set((s) => { - if (shouldApplyUpdate && !shouldApplyUpdate(findKnownWorktreeById(s, worktreeId))) { + if ( + shouldApplyUpdate && + !shouldApplyUpdate(findKnownWorktreeById(s, worktreeId, executionHostId)) + ) { return {} } didApply = true - const nextWorktrees = applyWorktreeUpdates(s.worktreesByRepo, worktreeId, enriched) + const nextWorktrees = applyWorktreeUpdates( + s.worktreesByRepo, + worktreeId, + enriched, + executionHostId + ) const nextDetectedWorktrees = applyDetectedWorktreeUpdates( s.detectedWorktreesByRepo, worktreeId, - enriched + enriched, + executionHostId ) const cacheKey = reviewRepo && reviewBranch @@ -231,7 +249,13 @@ export function createUpdateWorktreeMeta( } try { - await persistWorktreeMeta(settingsForWorktreeOwner(get(), worktreeId), worktreeId, enriched) + await persistWorktreeMeta( + settingsForWorktreeOwner(get(), worktreeId, executionHostId), + worktreeId, + enriched, + executionHostId ?? existingWorktree?.hostId, + worktreeForUpdate?.identity?.key + ) if ( !options?.suppressHostedReviewRefresh && reviewRepo && diff --git a/src/renderer/src/store/slices/worktrees/metadata/update-worktrees-meta.ts b/src/renderer/src/store/slices/worktrees/metadata/update-worktrees-meta.ts index 8ec3d887ae2..78254535106 100644 --- a/src/renderer/src/store/slices/worktrees/metadata/update-worktrees-meta.ts +++ b/src/renderer/src/store/slices/worktrees/metadata/update-worktrees-meta.ts @@ -1,4 +1,4 @@ -import type { WorktreeSlice } from '../../worktree-helpers' +import type { WorktreeMetaBatchUpdate, WorktreeSlice } from '../../worktree-helpers' import type { WorktreeSliceGet, WorktreeSliceSet } from '../listing/worktree-slice-types' import { applyWorktreeUpdates, getRepoIdFromWorktreeId } from '../../worktree-helpers' import { @@ -11,7 +11,6 @@ import { settingsForWorktreeOwner } from '../listing/worktree-owner-settings' import { parseWorkspaceKey } from '../../../../../../shared/workspace-scope' import { parseExecutionHostId, type ExecutionHostId } from '../../../../../../shared/execution-host' import { getIndexedWorktreesById } from '../../../worktree-repo-index' -import type { WorktreeMeta } from '../../../../../../shared/worktree/meta-types' function getKnownOwnerHostIds( state: ReturnType, @@ -26,25 +25,24 @@ function getKnownOwnerHostIds( } return [...hostIds] } - export function createUpdateWorktreesMeta( set: WorktreeSliceSet, get: WorktreeSliceGet ): WorktreeSlice['updateWorktreesMeta'] { - return async (updatesByWorktreeId) => { - if (updatesByWorktreeId.size === 0) { + return async (updates) => { + if (updates.length === 0) { return } - const gitWorktreeUpdates = new Map>() + const gitWorktreeUpdates: WorktreeMetaBatchUpdate[] = [] const folderWorkspaceUpdates: { folderWorkspaceId: string updates: ReturnType }[] = [] - for (const [worktreeId, updates] of updatesByWorktreeId) { - const scope = parseWorkspaceKey(worktreeId) + for (const entry of updates) { + const scope = parseWorkspaceKey(entry.worktreeId) if (scope?.type === 'folder') { - const folderUpdates = getFolderWorkspaceMetaUpdates(updates) + const folderUpdates = getFolderWorkspaceMetaUpdates(entry.updates) if (Object.keys(folderUpdates).length > 0) { folderWorkspaceUpdates.push({ folderWorkspaceId: scope.folderWorkspaceId, @@ -52,19 +50,25 @@ export function createUpdateWorktreesMeta( }) } } else { - gitWorktreeUpdates.set(worktreeId, updates) + gitWorktreeUpdates.push(entry) } } set((s) => { let nextWorktrees = s.worktreesByRepo let nextDetectedWorktrees = s.detectedWorktreesByRepo - for (const [worktreeId, updates] of gitWorktreeUpdates) { - nextWorktrees = applyWorktreeUpdates(nextWorktrees, worktreeId, updates) + for (const entry of gitWorktreeUpdates) { + nextWorktrees = applyWorktreeUpdates( + nextWorktrees, + entry.worktreeId, + entry.updates, + entry.executionHostId + ) nextDetectedWorktrees = applyDetectedWorktreeUpdates( nextDetectedWorktrees, - worktreeId, - updates + entry.worktreeId, + entry.updates, + entry.executionHostId ) } return nextWorktrees === s.worktreesByRepo && @@ -84,20 +88,27 @@ export function createUpdateWorktreesMeta( ...folderWorkspaceUpdates.map(({ folderWorkspaceId, updates }) => get().updateFolderWorkspace(folderWorkspaceId, updates) ), - ...Array.from(gitWorktreeUpdates, async ([worktreeId, updates]) => { + ...gitWorktreeUpdates.map(async ({ worktreeId, updates, executionHostId }) => { try { const state = get() - const ownerHostIds = getKnownOwnerHostIds(state, worktreeId) + const ownerHostIds = executionHostId + ? [executionHostId] + : getKnownOwnerHostIds(state, worktreeId) await (ownerHostIds.length === 0 ? persistWorktreeMeta(settingsForWorktreeOwner(state, worktreeId), worktreeId, updates) : Promise.all( - ownerHostIds.map((hostId) => - persistWorktreeMeta( + ownerHostIds.map((hostId) => { + const worktree = getIndexedWorktreesById(state.worktreesByRepo, worktreeId).find( + (candidate) => candidate.hostId === hostId + ) + return persistWorktreeMeta( settingsForWorktreeOwner(state, worktreeId, hostId), worktreeId, - updates + updates, + hostId, + worktree?.identity?.key ) - ) + }) )) } catch (err) { if (isRuntimeSelectorNotFoundError(err)) { diff --git a/src/renderer/src/store/slices/worktrees/metadata/worktree-meta-persist.ts b/src/renderer/src/store/slices/worktrees/metadata/worktree-meta-persist.ts index 7ebdbcc2ccd..99e4ac18ce1 100644 --- a/src/renderer/src/store/slices/worktrees/metadata/worktree-meta-persist.ts +++ b/src/renderer/src/store/slices/worktrees/metadata/worktree-meta-persist.ts @@ -11,16 +11,22 @@ import { toRuntimeWorktreeSelector } from '../../../../runtime/runtime-worktree- import { translate } from '@/i18n/i18n' import type { AppState } from '../../../types' import type { WorktreeMeta } from '../../../../../../shared/worktree/meta-types' +import type { ExecutionHostId } from '../../../../../../shared/execution-host' import { encodePushTargetClearForRuntimeRpc } from './hosted-review-link-mutation' - export async function persistWorktreeMeta( settings: AppState['settings'], worktreeId: string, - updates: Partial + updates: Partial, + executionHostId?: ExecutionHostId, + identityKey?: string ): Promise { const target = getActiveRuntimeTarget(settings) if (target.kind === 'local') { - await window.api.worktrees.updateMeta({ worktreeId, updates }) + await window.api.worktrees.updateMeta({ + worktreeId, + ...(executionHostId ? { executionHostId } : {}), + updates + }) return } // Why: `worktree.set` parses in strip mode, so an older runtime drops the key @@ -55,7 +61,7 @@ export async function persistWorktreeMeta( target, 'worktree.set', { - worktree: toRuntimeWorktreeSelector(worktreeId), + worktree: identityKey ? `identity:${identityKey}` : toRuntimeWorktreeSelector(worktreeId), ...encodePushTargetClearForRuntimeRpc(updates) }, { timeoutMs: 15_000 } diff --git a/src/renderer/src/store/slices/worktrees/session/worktree-pin-reveal.ts b/src/renderer/src/store/slices/worktrees/session/worktree-pin-reveal.ts index e8975c76587..7c104f06ca2 100644 --- a/src/renderer/src/store/slices/worktrees/session/worktree-pin-reveal.ts +++ b/src/renderer/src/store/slices/worktrees/session/worktree-pin-reveal.ts @@ -1,4 +1,4 @@ -import type { WorktreeSlice } from '../../worktree-helpers' +import type { WorktreeMetaBatchUpdate, WorktreeSlice } from '../../worktree-helpers' import type { WorktreeSliceGet, WorktreeSliceSet } from '../listing/worktree-slice-types' import { getActiveSidebarWorkspaceId, @@ -9,11 +9,9 @@ import { isValidResolvedWorktreeLineageEdge } from '../../../../../../shared/resolved-worktree-lineage' import type { WorktreeLineage } from '../../../../../../shared/worktree/lineage-types' -import type { WorktreeMeta } from '../../../../../../shared/worktree/meta-types' import type { Worktree } from '../../../../../../shared/worktree/types' type WorktreeWithEmbeddedLineage = Worktree & { lineage?: WorktreeLineage | null } - function getProjectedLineage(get: WorktreeSliceGet, worktree: Worktree): WorktreeLineage | null { if (Object.hasOwn(get().worktreeLineageById, worktree.id)) { return get().worktreeLineageById[worktree.id] ?? null @@ -66,7 +64,7 @@ export function createSetWorktreesPinnedAndReveal( get().activeWorktreeId ) // Skip worktrees already in the target state so a no-op toggle doesn't scroll the viewport away. - const updates = new Map>() + const updates: WorktreeMetaBatchUpdate[] = [] const changedWorktreeIds = new Set() let didChange = false let revealWorktreeId: string | null = null @@ -79,9 +77,17 @@ export function createSetWorktreesPinnedAndReveal( changedWorktreeIds.add(worktreeId) const workspaceScope = parseWorkspaceKey(worktreeId) if (workspaceScope?.type === 'folder') { - void get().updateWorktreeMeta(worktreeId, { isPinned }) + void get().updateWorktreeMeta( + worktreeId, + { isPinned }, + { executionHostId: current.hostId ?? 'local' } + ) } else { - updates.set(worktreeId, { isPinned }) + updates.push({ + worktreeId, + updates: { isPinned }, + executionHostId: current.hostId ?? 'local' + }) } if (revealWorktreeId === null && worktreeId === activeSidebarWorktreeId) { revealWorktreeId = worktreeId @@ -99,7 +105,7 @@ export function createSetWorktreesPinnedAndReveal( revealWorktreeId = activeSidebarWorktreeId } // updateWorktreesMeta applies the store update synchronously, so the reveal below sees the row already rendered. - if (updates.size > 0) { + if (updates.length > 0) { void get().updateWorktreesMeta(updates) } if (revealWorktreeId !== null) { diff --git a/src/shared/execution-host.test.ts b/src/shared/execution-host.test.ts index 75ed6210ff8..de895978f3b 100644 --- a/src/shared/execution-host.test.ts +++ b/src/shared/execution-host.test.ts @@ -120,3 +120,16 @@ describe('execution host identity', () => { ) }) }) + +describe('execution host id delimiter invariant', () => { + it('rejects an unencoded pipe so a crafted id cannot rebind a worktree identity alias', () => { + // composeWorktreeHostIdentity splits at the first `|`, so `ssh:a|b` would resolve as `ssh:a`. + expect(parseExecutionHostId('ssh:a|b')).toBeNull() + expect(parseExecutionHostId('runtime:a|b')).toBeNull() + expect(parseExecutionHostId(toSshExecutionHostId('a|b'))).toEqual({ + kind: 'ssh', + id: 'ssh:a%7Cb', + targetId: 'a|b' + }) + }) +}) diff --git a/src/shared/execution-host.ts b/src/shared/execution-host.ts index 18d584674bf..aacbc695cdc 100644 --- a/src/shared/execution-host.ts +++ b/src/shared/execution-host.ts @@ -81,6 +81,11 @@ export function parseExecutionHostId(value: string | null | undefined): ParsedEx if (!encoded) { return null } + // `|` must stay out of a host id: composeWorktreeHostIdentity uses it as its delimiter and + // splits at the first one, so an unencoded pipe would rebind an alias to a different host. + if (encoded.includes('|')) { + return null + } try { const targetId = decodeURIComponent(encoded) return targetId ? { kind: 'ssh', id: `ssh:${encoded}`, targetId } : null @@ -93,6 +98,9 @@ export function parseExecutionHostId(value: string | null | undefined): ParsedEx if (!encoded) { return null } + if (encoded.includes('|')) { + return null + } try { const environmentId = decodeURIComponent(encoded) return environmentId ? { kind: 'runtime', id: `runtime:${encoded}`, environmentId } : null diff --git a/src/shared/export-let-function-initializer-ban.test.ts b/src/shared/export-let-function-initializer-ban.test.ts new file mode 100644 index 00000000000..a152ed61cf1 --- /dev/null +++ b/src/shared/export-let-function-initializer-ban.test.ts @@ -0,0 +1,176 @@ +import { join, resolve } from 'node:path' +import { beforeAll, describe, expect, it } from 'vitest' +import type { parseAst as ParseAstFn } from 'vite' with { 'resolution-mode': 'import' } +import { scanSourceTree } from './source-scan/source-tree-scan' + +// Dynamic import: vite is ESM-only and this file typechecks under tsconfig.tc.cli.json's node16/CJS. +let parseAst: typeof ParseAstFn + +beforeAll(async () => { + ;({ parseAst } = await import('vite')) +}) + +/** + * Ban `export let fn = ` tree-wide: rolldown const-folds the + * initializer into call sites and drops the setter's reassignment, so the + * bridged closure silently never runs in the built app (dev is fine). Use a + * module-local null-initialized impl var behind exported wrapper functions + * instead (see src/main/ipc/pty/delivery/debug.ts); null-initialized + * `export let` slots compile correctly and stay allowed. Detection uses + * vite/rolldown's own oxc parser so this test sees exactly what the bundler sees. + */ + +type AstNode = { + type: string + start?: number + expression?: AstNode + expressions?: AstNode[] + declaration?: AstNode + kind?: string + declarations?: { + id?: { name?: string } + init?: AstNode | null + start?: number + }[] + specifiers?: { local?: { name?: string } }[] +} + +/** Casts, parens, `!`, and comma sequences around the function don't change what rolldown folds. */ +function unwrapInitializer(node: AstNode): AstNode { + let current = node + for (;;) { + if ( + current.expression && + [ + 'ParenthesizedExpression', + 'TSAsExpression', + 'TSSatisfiesExpression', + 'TSTypeAssertion', + 'TSNonNullExpression' + ].includes(current.type) + ) { + current = current.expression + continue + } + // A sequence evaluates to its last expression — that's what gets folded. + const sequenceTail = + current.type === 'SequenceExpression' ? current.expressions?.at(-1) : undefined + if (sequenceTail) { + current = sequenceTail + continue + } + return current + } +} + +export type FunctionInitializedExportLet = { name: string; line: number } + +export function findFunctionInitializedExportLets( + fileName: string, + sourceText: string +): FunctionInitializedExportLet[] { + // Cheap prefilter: full parses only for files that could contain the pattern. + if (!/\bexport\b/.test(sourceText) || !/\blet\b/.test(sourceText)) { + return [] + } + const lineOf = (offset: number): number => sourceText.slice(0, offset).split('\n').length + const program = parseAst(sourceText, { + lang: fileName.endsWith('.tsx') ? 'tsx' : 'ts' + }) as unknown as { body: AstNode[] } + const functionLets: (FunctionInitializedExportLet & { exported: boolean })[] = [] + const exportedNames = new Set() + const collectLets = (declaration: AstNode | undefined, exported: boolean): void => { + if (declaration?.type !== 'VariableDeclaration' || declaration.kind !== 'let') { + return + } + for (const declarator of declaration.declarations ?? []) { + const initializer = declarator.init && unwrapInitializer(declarator.init) + if ( + initializer && + ['ArrowFunctionExpression', 'FunctionExpression'].includes(initializer.type) + ) { + functionLets.push({ + name: declarator.id?.name ?? '', + line: lineOf(declarator.start ?? 0), + exported + }) + } + } + } + for (const statement of program.body) { + if (statement.type === 'ExportNamedDeclaration') { + collectLets(statement.declaration, true) + // `let fn = () => {}; export { fn }` is the same live binding — same hazard. + for (const specifier of statement.specifiers ?? []) { + if (specifier.local?.name) { + exportedNames.add(specifier.local.name) + } + } + continue + } + collectLets(statement, false) + } + return functionLets + .filter((candidate) => candidate.exported || exportedNames.has(candidate.name)) + .map(({ name, line }) => ({ name, line })) +} + +describe('function-initialized export let ban', () => { + it('flags the shapes rolldown miscompiles', () => { + const flagged = [ + 'export let f = () => {}', + 'export let f = function () {}', + 'export let f = async () => {}', + 'export let f = (() => {}) as () => void', + 'export let f = (() => {})!', + 'export let f = (0, () => {})', + 'export let f: (id: string) => void = (_id) => {}', + 'let f = () => {}\nexport { f }', + 'let f = () => {}\nexport { f as g }' + ] + for (const source of flagged) { + expect(findFunctionInitializedExportLets('a.ts', source), source).toHaveLength(1) + } + }) + + it('reports the declaration line', () => { + const source = 'const a = 1\n\nexport let f = () => {}' + expect(findFunctionInitializedExportLets('a.ts', source)).toEqual([{ name: 'f', line: 3 }]) + }) + + it('allows the safe shapes', () => { + const allowed = [ + 'export let f: (() => void) | null = null', + 'export const f = () => {}', + 'let f = () => {}', + 'let f: (() => void) | null = null\nexport { f }', + 'export let n = 0', + 'export let f' + ] + for (const source of allowed) { + expect(findFunctionInitializedExportLets('a.ts', source), source).toEqual([]) + } + }) + + const repoRoot = resolve(__dirname, '..', '..') + // Tests are not rolldown-bundled (scanSourceTree excludes them by default); + // several drive the banned shape on purpose. + const files = scanSourceTree(join(repoRoot, 'src')) + + it('scans a plausible number of files', () => { + // A broken root or extension list would make the guard silently vacuous. + expect(files.length).toBeGreaterThan(500) + }) + + it('has no function-initialized export let in shipped code', () => { + const offenders = files.flatMap(({ relativePath, source }) => + findFunctionInitializedExportLets(relativePath, source).map( + (offender) => `src/${relativePath}:${offender.line} (${offender.name})` + ) + ) + expect( + offenders, + 'Rolldown miscompiles `export let fn = ` bridges: the initializer is folded into call sites and the setter reassignment is dropped in the built app. Use a module-local null-initialized impl var behind an exported wrapper function instead — see src/main/ipc/pty/delivery/debug.ts.' + ).toEqual([]) + }) +}) diff --git a/src/shared/persisted-state-types.ts b/src/shared/persisted-state-types.ts index 6266f8bc08f..a081eee76ab 100644 --- a/src/shared/persisted-state-types.ts +++ b/src/shared/persisted-state-types.ts @@ -77,6 +77,10 @@ export type PersistedState = { /** Per paired device last tab selection by worktree; keeps mobile navigation across host restarts. */ mobileClientTabSelectionsByDeviceId?: PersistedMobileClientTabSelections worktreeMeta: Record + /** Canonical host/instance metadata; optional for legacy profiles. */ + worktreeMetaByIdentity?: Record + /** Host-qualified locator aliases to canonical identity keys. */ + worktreeIdentityAliases?: Record worktreeLineageById: Record workspaceLineageByChildKey: Record settings: GlobalSettings diff --git a/src/shared/project-host-setup-lookup.ts b/src/shared/project-host-setup-lookup.ts new file mode 100644 index 00000000000..d7d59f4cef9 --- /dev/null +++ b/src/shared/project-host-setup-lookup.ts @@ -0,0 +1,30 @@ +import { getRepoExecutionHostId } from './execution-host' +import type { ProjectHostSetup } from './project-types' +import type { Repo } from './repo-types' +import type { WorktreeMeta } from './worktree/meta-types' +import { projectHostSetupProjectionFromRepos } from './project-host-setup-projection' + +export function getProjectHostSetupForRepo( + setups: readonly ProjectHostSetup[], + repo: Repo +): ProjectHostSetup { + // A repo id can exist on multiple hosts; the host-qualified setup is authoritative. + const executionHostId = getRepoExecutionHostId(repo) + return ( + setups.find((setup) => setup.repoId === repo.id && setup.hostId === executionHostId) ?? + setups.find((setup) => setup.repoId === repo.id) ?? + projectHostSetupProjectionFromRepos([repo]).setups[0] + ) +} + +export function getProjectHostSetupWorktreeMeta( + setups: readonly ProjectHostSetup[], + repo: Repo +): Pick { + const setup = getProjectHostSetupForRepo(setups, repo) + return { + projectId: setup.projectId, + hostId: setup.hostId, + projectHostSetupId: setup.id + } +} diff --git a/src/shared/project-host-setup-projection.test.ts b/src/shared/project-host-setup-projection.test.ts index 97ea8cc66b0..67fca1ad6b5 100644 --- a/src/shared/project-host-setup-projection.test.ts +++ b/src/shared/project-host-setup-projection.test.ts @@ -2,11 +2,11 @@ import { describe, expect, it } from 'vitest' import { projectHostSetupProjectionFromRepos, getProjectHostSetupsForProject, - getProjectHostSetupWorktreeMeta, isGitHubBackedRepo, getProjectIdForProviderIdentity, isProjectRemoteIdentityPending } from './project-host-setup-projection' +import { getProjectHostSetupWorktreeMeta } from './project-host-setup-lookup' import type { Repo } from './repo-types' function repo(overrides: Partial & Pick): Repo { @@ -664,3 +664,23 @@ describe('derived project identity stability', () => { expect(projection.projects.map((project) => project.id)).toEqual(['github:acme/app']) }) }) + +describe('getProjectHostSetupWorktreeMeta host selection', () => { + it("picks the setup for the repo's own execution host when a repoId spans two hosts", () => { + const sshRepo = repo({ + id: 'repo-shared', + path: '/remote/orca', + displayName: 'orca', + connectionId: 'build-box' + }) + // Local first in the array: a repoId-only match would stamp the wrong host durably. + const setups = [ + ...projectHostSetupProjectionFromRepos([ + repo({ id: 'repo-shared', path: '/local/orca', displayName: 'orca' }) + ]).setups, + ...projectHostSetupProjectionFromRepos([sshRepo]).setups + ] + + expect(getProjectHostSetupWorktreeMeta(setups, sshRepo).hostId).toBe('ssh:build-box') + }) +}) diff --git a/src/shared/project-host-setup-projection.ts b/src/shared/project-host-setup-projection.ts index 090f421ccdd..954208ee27d 100644 --- a/src/shared/project-host-setup-projection.ts +++ b/src/shared/project-host-setup-projection.ts @@ -7,7 +7,6 @@ import { normalizeGitHubRemoteHost } from './git-remote-host-alias' import { githubRepoIdentityKey, isDefaultGitHubHost } from './github/repository-identity-key' import type { Project, ProjectHostSetup, ProjectProviderIdentity } from './project-types' import type { Repo } from './repo-types' -import type { WorktreeMeta } from './worktree/meta-types' type ProjectAccumulator = { project: Project @@ -337,25 +336,3 @@ export function getProjectHostSetupsForProject( ): readonly ProjectHostSetup[] { return setups.filter((setup) => setup.projectId === projectId) } - -export function getProjectHostSetupForRepo( - setups: readonly ProjectHostSetup[], - repo: Repo -): ProjectHostSetup { - return ( - setups.find((setup) => setup.repoId === repo.id) ?? - projectHostSetupProjectionFromRepos([repo]).setups[0] - ) -} - -export function getProjectHostSetupWorktreeMeta( - setups: readonly ProjectHostSetup[], - repo: Repo -): Pick { - const setup = getProjectHostSetupForRepo(setups, repo) - return { - projectId: setup.projectId, - hostId: setup.hostId, - projectHostSetupId: setup.id - } -} diff --git a/src/shared/source-scan/source-tree-scan.ts b/src/shared/source-scan/source-tree-scan.ts index 780354434c0..900d969b2ea 100644 --- a/src/shared/source-scan/source-tree-scan.ts +++ b/src/shared/source-scan/source-tree-scan.ts @@ -18,7 +18,11 @@ const IGNORED_DIRECTORIES = new Set(['node_modules', 'dist', 'out', 'build', '.g export function isTestFile(relativePath: string): boolean { return ( /\.(?:test|spec)\.tsx?$/.test(relativePath) || - /(?:test-harness|test-utils|test-setup|test-fixture|repro)/.test(relativePath) || + // `repro` must be a whole token: a bare substring exempted the shipped + // windows-terminal-capability-reprobe.ts from every guard using this walk. + /(?:test-harness|test-utils|test-setup|test-fixture|\brepro\b|reproduction)/.test( + relativePath + ) || relativePath.includes('/__tests__/') ) } diff --git a/src/shared/worktree/identity.test.ts b/src/shared/worktree/identity.test.ts new file mode 100644 index 00000000000..c1e109cb7ad --- /dev/null +++ b/src/shared/worktree/identity.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, it } from 'vitest' +import { canonicalWorktreeIdentity, type WorktreeIdentityRef } from './identity' + +describe('canonical worktree identity', () => { + const local: WorktreeIdentityRef = { + worktreeId: 'repo-1::/workspace/feature', + executionHostId: 'local', + instanceId: '11111111-1111-4111-8111-111111111111' + } + const remote: WorktreeIdentityRef = { + ...local, + executionHostId: 'ssh:build-box' + } + + it('separates same locator and instance across execution hosts', () => { + expect(canonicalWorktreeIdentity(local)).not.toBe(canonicalWorktreeIdentity(remote)) + }) + + it('is stable when only the path locator changes', () => { + expect( + canonicalWorktreeIdentity({ ...local, worktreeId: 'repo-1::/workspace/renamed-feature' }) + ).toBe(canonicalWorktreeIdentity(local)) + }) + + it('does not use the display name as identity input', () => { + expect(canonicalWorktreeIdentity(local)).toBe(canonicalWorktreeIdentity({ ...local })) + }) +}) diff --git a/src/shared/worktree/identity.ts b/src/shared/worktree/identity.ts new file mode 100644 index 00000000000..382d4329792 --- /dev/null +++ b/src/shared/worktree/identity.ts @@ -0,0 +1,29 @@ +import type { ExecutionHostId } from '../execution-host' + +/** Stable fields needed to identify one checkout occupant. */ +export type WorktreeIdentityRef = { + /** Existing repo/path locator; mutable when the folder moves. */ + worktreeId: string + /** Host that owns execution for this checkout. */ + executionHostId: ExecutionHostId + /** Durable identity for this checkout occupant. */ + instanceId: string +} +export type WorktreeIdentity = Omit & { + key: string +} + +/** + * Canonical exact identity for one worktree occupant. + * The locator is deliberately excluded so a folder rename preserves identity. + */ +export function canonicalWorktreeIdentity(ref: WorktreeIdentityRef): string { + return `wt2:${encodeURIComponent(ref.executionHostId)}:${encodeURIComponent(ref.instanceId)}` +} +export function createWorktreeIdentity(ref: WorktreeIdentityRef): WorktreeIdentity { + return { + key: canonicalWorktreeIdentity(ref), + executionHostId: ref.executionHostId, + instanceId: ref.instanceId + } +} diff --git a/src/shared/worktree/types.ts b/src/shared/worktree/types.ts index af6252f5c3a..97c11abf3eb 100644 --- a/src/shared/worktree/types.ts +++ b/src/shared/worktree/types.ts @@ -5,6 +5,7 @@ import type { TuiAgent } from '../tui-agent' import type { DiffComment, MobileDiffReviewState } from '../diff-comment-types' import type { EphemeralVmCheckoutMode } from '../orca-yaml-hook-types' import type { BuiltInWorktreeVisibilitySourceId } from '../repo-types' +import type { WorktreeIdentity } from './identity' export type WorkspaceLinkedItem = { provider: 'github' | 'gitlab' | 'linear' | 'jira' @@ -60,6 +61,8 @@ export type WorkspaceStatusDefinition = { export type Worktree = { id: string // `${repoId}::${path}` instanceId?: string + /** Immutable host/instance identity. Optional while legacy rows migrate. */ + identity?: WorktreeIdentity repoId: string /** Durable project identity. Optional while legacy repo-only workspaces migrate. */ projectId?: string