From 0a72e71dae582b348e9663a83363599469f20652 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Thu, 27 Aug 2026 15:03:30 -0700 Subject: [PATCH 1/3] Fix STA-5661: prevent rolldown const-folding of bridged exports (#16869) Rolldown miscompiles `export let fn = noop` by const-folding initializers and dropping setters. Refactor to use null-initialized impl vars behind wrapper functions instead, and add test to prevent regression. --- src/main/ipc/pty/delivery/debug.ts | 40 +++- src/main/ipc/pty/delivery/visibility-state.ts | 17 +- .../ipc/pty/provider/listener-lifecycle.ts | 9 +- ...xport-let-function-initializer-ban.test.ts | 176 ++++++++++++++++++ src/shared/source-scan/source-tree-scan.ts | 6 +- 5 files changed, 231 insertions(+), 17 deletions(-) create mode 100644 src/shared/export-let-function-initializer-ban.test.ts 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/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/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__/') ) } From a724aa7b0888f5ce7b9915a9f416ac36a56bc30b Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 27 Aug 2026 15:08:14 -0700 Subject: [PATCH 2/3] fix(mobile): report composing state from accessory backspace (#16757) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(mobile): report composing state from accessory backspace The accessory path edits the field itself and then mirrors it, but called applyLiveInputMirror with two arguments where the signature takes three. The local option type declared it 2-ary, so the type checker never saw the drop. An omitted composing flag is not "not composing": it selects the Android-only heuristic that holds the trailing non-ASCII run. A pinyin preedit is plain ASCII, so the heuristic reads it as committed text and sends it. Typing `ni hao`, tapping accessory Backspace, then picking a candidate put `ni ha` on the PTY before the commit, giving `ni ha你好`. Korean survived this by accident - the non-ASCII heuristic re-derives the correct hold for Hangul - which is why it went unnoticed. The held range is the fact the mirror needs, and it is already in scope. Refs #13345 * fix(mobile): preserve accessory IME report provenance --- ...rminal-live-accessory-input-commit.test.ts | 34 ++++++++++++++++--- ...se-terminal-live-accessory-input-commit.ts | 8 +++-- .../use-terminal-live-input-commit.test.ts | 29 ++++++++++++++++ .../use-terminal-live-input-commit.ts | 2 ++ .../use-terminal-live-pending-input-flush.ts | 6 ++++ 5 files changed, 72 insertions(+), 7 deletions(-) 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 From b241a68ae4467253beaac3ed1bb6a663fd9c5a01 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 27 Aug 2026 15:08:40 -0700 Subject: [PATCH 3/3] Fix worktree identity collisions across hosts (#16691) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(workspaces): add collision-safe worktree identity * fix(workspaces): read worktree metadata per host and repair ambiguous identities The canonical identity store landed write-only: getWorktreeMetaForHost had no production callers while setWorktreeMetaForHost kept the legacy projection only for the first known owner, so a second host's edits persisted and were never read back. Wire the listing paths through host-qualified reads. An ambiguous alias was also unrecoverable — reads returned undefined and writes threw forever, and the throw escaped the detected-worktree loop, emptying the whole repo's sidebar. Fail open onto the most recently active instance instead. - collapse ambiguous aliases deterministically and persist the repair - reclaim identity rows in the metadata GC so they cannot outlive their locator or resurrect onto a worktree recreated at the same path - drop every host's rows when a locator is removed outright, not just the owner's - honour an explicit instanceId so the stale-lineage rotation guard still works - scope a rename to the moving host; other hosts keep their own locator - prefer the project host setup matching the repo's own execution host, so a repoId registered on two hosts no longer stamps the wrong one durably - reject an unencoded `|` in a host id, the invariant the alias delimiter needs - drop the never-populated hostGeneration from the canonical key * fix(workspaces): close remaining identity review gaps * fix(workspaces): close remaining review gaps * fix(workspaces): address review and CI regressions * test(workspaces): update host-qualified metadata expectations * fix(workspaces): preserve ambiguous identity records * fix(workspaces): snapshot metadata during listing * test(workspaces): mirror listing metadata snapshot in windows fixture * fix(workspaces): preserve identity routing for metadata writes * fix(workspaces): scope stale metadata cleanup by host * fix(workspaces): rekey identities on SSH readoption * fix(workspaces): fail closed for ambiguous board ids * perf(workspaces): snapshot metadata across catalog listing * fix(workspaces): retain neighboring manual order updates * test(workspaces): cover ambiguous board id index * fix(persistence): harden host-qualified worktree metadata * refactor(shared): split project host setup lookup * refactor(workspaces): simplify host-qualified metadata --- src/cli/help.ts | 10 +- src/cli/index.test.ts | 3 + src/cli/selectors.ts | 2 +- .../ipc/repos/project-host-setup-handlers.ts | 6 +- .../ipc/workspace-cleanup-broad-scan.test.ts | 32 +- .../ipc/workspace-cleanup-disconnected-ssh.ts | 13 +- src/main/ipc/workspace-cleanup-scan.ts | 7 +- src/main/ipc/worktree-metadata-merge.test.ts | 47 +++ src/main/ipc/worktree-metadata-merge.ts | 13 +- src/main/ipc/worktree-remote.ts | 2 +- ...ktrees-create-metadata-persistence.test.ts | 30 ++ .../worktrees-listing-fallback-rows.test.ts | 64 ++++ src/main/ipc/worktrees-test-harness.ts | 12 + src/main/ipc/worktrees-test-ipc-surface.ts | 4 + src/main/ipc/worktrees-windows.test.ts | 11 + .../create/folder-workspace-creation.ts | 2 +- .../listing/detected-provider-listing.ts | 9 +- .../listing/register-host-catalog-handlers.ts | 8 +- .../register-worktree-catalog-handlers.ts | 55 ++- .../listing/ssh-worktree-fallback.ts | 31 +- .../listing/worktree-discovery-metadata.ts | 28 +- .../register-worktree-metadata-handlers.ts | 23 +- src/main/persistence-repo-lifecycle.test.ts | 6 + .../host-qualified-worktree-meta.ts | 65 ++++ .../canonical-worktree-host-reassignment.ts | 128 +++++++ .../ssh-target-reassignment.ts | 21 +- .../metadata-lineage-operations.ts | 142 ++++--- .../worktree-identity-metadata.ts | 236 ++++++++++++ .../worktree-meta-write-normalization.ts | 55 +++ .../tracking-repos/repo-worktree-pruning.ts | 10 + .../worktree-metadata-normalization.test.ts | 132 ++++++- .../worktree-metadata-normalization.ts | 105 +++++- src/main/provisioned-root-ssh-adoption.ts | 2 +- src/main/runtime/orca-runtime.ts | 30 +- .../repo-worktree-row-resolution.test.ts | 65 ++++ .../runtime/repo-worktree-row-resolution.ts | 18 +- ...rktree-scan-admin-fingerprint-gate.test.ts | 28 +- src/main/workspace-space-repo-scan.ts | 12 +- .../worktree-identity-persistence.test.ts | 351 ++++++++++++++++++ src/main/worktree-metadata-ownership.test.ts | 35 ++ src/preload/api/worktree-api.ts | 6 +- ...-checks-panel-check-and-review-actions.tsx | 16 +- .../WorkspaceKanbanDrawer.search.test.tsx | 11 +- ...treeList.status-lane-lineage-drop.test.tsx | 9 +- .../sidebar/WorktreeMetaDialog.test.tsx | 23 ++ .../components/sidebar/WorktreeMetaDialog.tsx | 10 +- .../use-workspace-kanban-board-projection.ts | 3 +- .../use-workspace-kanban-status-actions.ts | 6 +- .../use-workspace-kanban-worktree-actions.ts | 66 +++- .../use-worktree-card-activation-actions.ts | 18 +- .../sidebar/use-worktree-card-foundation.ts | 2 + .../use-worktree-card-secondary-details.ts | 13 +- .../use-worktree-context-menu-commands.ts | 18 +- .../workspace-kanban-sidebar-drop.test.ts | 22 +- .../sidebar/workspace-kanban-sidebar-drop.ts | 27 +- .../drag/use-status-mutations.ts | 54 ++- .../worktree-unambiguous-id-index.test.ts | 43 +++ .../sidebar/worktree-unambiguous-id-index.ts | 17 + ...r-workspace-owner-routed-mutations.test.ts | 11 +- .../src/store/slices/worktree-helpers.ts | 81 +--- .../worktree-meta-update-application.ts | 69 ++++ .../worktrees-activity-persistence.test.ts | 2 +- .../worktrees-fetch-listing-merge.test.ts | 10 +- ...orktrees-linked-review-push-target.test.ts | 64 +++- .../worktrees-metadata-persistence.test.ts | 54 ++- .../worktrees-terminal-pr-url-linking.test.ts | 3 + .../listing/detected-worktree-meta.ts | 7 +- .../listing/worktree-meta-host-match.ts | 19 + .../metadata/update-worktree-meta.ts | 40 +- .../metadata/update-worktrees-meta.ts | 51 ++- .../metadata/worktree-meta-persist.ts | 14 +- .../worktrees/session/worktree-pin-reveal.ts | 20 +- src/shared/execution-host.test.ts | 13 + src/shared/execution-host.ts | 8 + src/shared/persisted-state-types.ts | 4 + src/shared/project-host-setup-lookup.ts | 30 ++ .../project-host-setup-projection.test.ts | 22 +- src/shared/project-host-setup-projection.ts | 23 -- src/shared/worktree/identity.test.ts | 28 ++ src/shared/worktree/identity.ts | 29 ++ src/shared/worktree/types.ts | 3 + 81 files changed, 2457 insertions(+), 365 deletions(-) create mode 100644 src/main/ipc/worktree-metadata-merge.test.ts create mode 100644 src/main/persistence/host-qualified-worktree-meta.ts create mode 100644 src/main/persistence/leasing-ssh-ptys/canonical-worktree-host-reassignment.ts create mode 100644 src/main/persistence/loading-store/worktree-identity-metadata.ts create mode 100644 src/main/persistence/loading-store/worktree-meta-write-normalization.ts create mode 100644 src/main/worktree-identity-persistence.test.ts create mode 100644 src/main/worktree-metadata-ownership.test.ts create mode 100644 src/renderer/src/components/sidebar/worktree-unambiguous-id-index.test.ts create mode 100644 src/renderer/src/components/sidebar/worktree-unambiguous-id-index.ts create mode 100644 src/renderer/src/store/slices/worktree-meta-update-application.ts create mode 100644 src/renderer/src/store/slices/worktrees/listing/worktree-meta-host-match.ts create mode 100644 src/shared/project-host-setup-lookup.ts create mode 100644 src/shared/worktree/identity.test.ts create mode 100644 src/shared/worktree/identity.ts 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/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 f83e5e777e4..495045821da 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -538,11 +538,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' @@ -918,6 +918,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, @@ -1354,6 +1355,7 @@ type RuntimeStore = { getAllWorktreeMeta: Store['getAllWorktreeMeta'] getWorktreeMeta: Store['getWorktreeMeta'] setWorktreeMeta: Store['setWorktreeMeta'] + setWorktreeMetaForHost?: Store['setWorktreeMetaForHost'] removeWorktreeMeta: Store['removeWorktreeMeta'] getWorktreeLineage?: Store['getWorktreeLineage'] getAllWorktreeLineage?: Store['getAllWorktreeLineage'] @@ -20835,7 +20837,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, @@ -24261,7 +24263,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 @@ -27129,12 +27134,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 } { @@ -32322,7 +32335,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/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/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