From 9810a5cc92e909e9231b824610ba7cbcb2e7633b Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 17 Oct 2026 23:43:55 -0700 Subject: [PATCH] refactor(worktree): one comparison-only worktreeIdentityKey replaces three key wrappers (#30217) worktreeIdComparisonKey (nullable '::' key), runtimeWorktreeIdentityKey ('\0' key) and runtimeWorktreeLookupKey (JSON tuple) all folded the same path spelling. They become one total key in shared/worktree/id.ts; a malformed id keys as itself. Also fixes id.test.ts literals that were missing a colon, which made the WSL/UNC parity assertions compare null to null. --- .../worktree-push-target-reconciliation.ts | 13 ++++---- src/main/memory/hydrate-local-pty-registry.ts | 19 +++++------- .../mobile-relay-host-catalog.ts | 7 ++--- .../runtime/repo-worktree-row-resolution.ts | 11 ++++--- src/main/runtime/rpc/methods/session-tabs.ts | 6 ++-- .../runtime/runtime-worktree-path-identity.ts | 23 ++------------ .../orca-runtime-workspace-activity-facts.ts | 7 ++--- .../terminal/query/pty-terminal-summary.ts | 8 ++--- .../terminal/stop/worktree-terminal-sleep.ts | 10 +++---- .../terminal/stop/worktree-terminal-stop.ts | 12 +++----- .../runtime/workspaces/workspace-selectors.ts | 10 +++---- src/shared/worktree/id.test.ts | 30 +++++++++++-------- src/shared/worktree/id.ts | 16 +++++----- 13 files changed, 72 insertions(+), 100 deletions(-) diff --git a/src/main/ipc/worktree-push-target-reconciliation.ts b/src/main/ipc/worktree-push-target-reconciliation.ts index 561e9cc12f0..09c8b492579 100644 --- a/src/main/ipc/worktree-push-target-reconciliation.ts +++ b/src/main/ipc/worktree-push-target-reconciliation.ts @@ -11,7 +11,7 @@ import { gitExecFileAsync } from '../git/runner' import { listWorktrees } from '../git/worktree' import type { GitPushTarget } from '../../shared/worktree/types' -import { WORKTREE_ID_SEPARATOR, worktreeIdComparisonKey } from '../../shared/worktree/id' +import { WORKTREE_ID_SEPARATOR, worktreeIdentityKey } from '../../shared/worktree/id' import { parseGitRemoteFetchUrls } from '../../shared/git-remote-url-index' import { findWorktreeMetaReferencingRemote, @@ -66,10 +66,9 @@ async function shouldReclaimPrRemote( if (!referencingEntries.some(({ meta }) => meta.pushTarget?.remoteCreated === true)) { return false } - const stillClaimedByLiveWorktree = referencingEntries.some(({ worktreeId }) => { - const key = worktreeIdComparisonKey(worktreeId) - return key !== null && liveWorktreeKeys.has(key) - }) + const stillClaimedByLiveWorktree = referencingEntries.some(({ worktreeId }) => + liveWorktreeKeys.has(worktreeIdentityKey(worktreeId)) + ) if (stillClaimedByLiveWorktree) { return false } @@ -93,9 +92,7 @@ export async function reconcileOrphanedPrRemotesWithExec( liveWorktreePaths: readonly string[] ): Promise { const liveWorktreeKeys = new Set( - liveWorktreePaths - .map((path) => worktreeIdComparisonKey(`${repoId}${WORKTREE_ID_SEPARATOR}${path}`)) - .filter((key): key is string => key !== null) + liveWorktreePaths.map((path) => worktreeIdentityKey(`${repoId}${WORKTREE_ID_SEPARATOR}${path}`)) ) const reclaimed: string[] = [] for (const remote of await listPrRemoteCandidates(execGit, repoPath)) { diff --git a/src/main/memory/hydrate-local-pty-registry.ts b/src/main/memory/hydrate-local-pty-registry.ts index 9e4e97bceb2..cf3e4bdf795 100644 --- a/src/main/memory/hydrate-local-pty-registry.ts +++ b/src/main/memory/hydrate-local-pty-registry.ts @@ -4,7 +4,7 @@ import { mapSettledWithConcurrency } from '../../shared/map-with-concurrency' import { parsePtySessionId } from '../../shared/pty-session-id-format' import { isFolderRepo } from '../../shared/repo-kind' import type { Repo } from '../../shared/repo-types' -import { splitWorktreeId, worktreeIdComparisonKey } from '../../shared/worktree/id' +import { splitWorktreeId, worktreeIdentityKey } from '../../shared/worktree/id' import { getDaemonProvider } from '../daemon/daemon-provider-state' import { getAllDaemonAdapters, type DaemonProvider } from '../daemon/daemon-provider-routing' import type { SessionInfo } from '../daemon/types' @@ -144,14 +144,12 @@ async function hydrateLocalPtyRegistry( const { repo, worktrees } = result.value for (const worktree of worktrees) { const worktreeId = `${repo.id}::${worktree.path}` - const key = worktreeIdComparisonKey(worktreeId) - if (key) { - const existing = liveGitWorktreeIdsByKey.get(key) - liveGitWorktreeIdsByKey.set( - key, - existing === undefined || existing === worktreeId ? worktreeId : null - ) - } + const key = worktreeIdentityKey(worktreeId) + const existing = liveGitWorktreeIdsByKey.get(key) + liveGitWorktreeIdsByKey.set( + key, + existing === undefined || existing === worktreeId ? worktreeId : null + ) } } @@ -190,8 +188,7 @@ async function hydrateLocalPtyRegistry( if (verifiedLocalFolderKeys.has(worktreeId) || verifiedFolderWorktreeIds.has(worktreeId)) { return true } - const key = worktreeIdComparisonKey(worktreeId) - return key !== null && typeof liveGitWorktreeIdsByKey.get(key) === 'string' + return typeof liveGitWorktreeIdsByKey.get(worktreeIdentityKey(worktreeId)) === 'string' } } diff --git a/src/main/runtime/mobile-desktop-relay/mobile-relay-host-catalog.ts b/src/main/runtime/mobile-desktop-relay/mobile-relay-host-catalog.ts index 5220dc0ca01..f8f94fc02c8 100644 --- a/src/main/runtime/mobile-desktop-relay/mobile-relay-host-catalog.ts +++ b/src/main/runtime/mobile-desktop-relay/mobile-relay-host-catalog.ts @@ -14,7 +14,7 @@ import { } from '../../../shared/workspace-creator-visibility' import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' import { getWorkspaceSessionTabActivityIds } from '../../../shared/workspace-session-tab-activity' -import { runtimeWorktreeIdentityKey } from '../runtime-worktree-path-identity' +import { worktreeIdentityKey } from '../../../shared/worktree/id' import { SERVER_FETCH_TIMEOUT_MS } from './server-worktree-fetch' import type { MobileDesktopRelayHosts } from './mobile-desktop-relay-hosts' import { describeRelayHosts, relayHostFence } from './mobile-relay-host-descriptions' @@ -86,7 +86,7 @@ export class MobileRelayHostCatalog { ...getWorkspaceSessionTabActivityIds( this.options.getWorkspaceSession(toRuntimeExecutionHostId(host.environmentId)) ) - ].map(runtimeWorktreeIdentityKey) + ].map(worktreeIdentityKey) ) // Why: the desktop asks with background-removal support; a phone asking itself never sees these rows. return { @@ -127,8 +127,7 @@ function stampDesktopSidebarView( desktopDeviceIds ) const active = - typeof row.worktreeId === 'string' && - sessionActivity.has(runtimeWorktreeIdentityKey(row.worktreeId)) + typeof row.worktreeId === 'string' && sessionActivity.has(worktreeIdentityKey(row.worktreeId)) if (!otherClient && !active) { return row } diff --git a/src/main/runtime/repo-worktree-row-resolution.ts b/src/main/runtime/repo-worktree-row-resolution.ts index 93a6f02e3d8..e0cce0d2ac3 100644 --- a/src/main/runtime/repo-worktree-row-resolution.ts +++ b/src/main/runtime/repo-worktree-row-resolution.ts @@ -2,7 +2,7 @@ import { preserveFolderUpgradeWorktreePath } from '../folder-upgrade-worktree-pa import { splitWorktreeId, splitWorktreeIdForFilesystem, - worktreeIdComparisonKey + worktreeIdentityKey } from '../../shared/worktree/id' import { getRepoExecutionHostId, type ExecutionHostId } from '../../shared/execution-host' import { @@ -230,12 +230,11 @@ export async function resolveScopedWorktreeIdRow( ) // Why (#16243): the scan can spell this id's path differently — the divergence `path:` absorbs. // One equivalent row may stand in; two is an ambiguity a scoped lookup must refuse, not guess. - const comparisonKey = worktreeIdComparisonKey(worktreeId) + const comparisonKey = worktreeIdentityKey(worktreeId) const exact = ownerRows.find((worktree) => worktree.id === worktreeId) - const equivalent = - comparisonKey === null - ? [] - : ownerRows.filter((worktree) => worktreeIdComparisonKey(worktree.id) === comparisonKey) + const equivalent = ownerRows.filter( + (worktree) => worktreeIdentityKey(worktree.id) === comparisonKey + ) const target = exact ?? (equivalent.length === 1 ? equivalent[0] : null) if (!target) { return null diff --git a/src/main/runtime/rpc/methods/session-tabs.ts b/src/main/runtime/rpc/methods/session-tabs.ts index 37b34992b52..f27cca352ad 100644 --- a/src/main/runtime/rpc/methods/session-tabs.ts +++ b/src/main/runtime/rpc/methods/session-tabs.ts @@ -1,7 +1,7 @@ import { terminalImageViewerSupported } from '../../terminal-image-viewer-env' import { resolveRuntimeNavigationTarget } from '../../../../shared/runtime-navigation' import { getExplicitWorktreeIdSelector } from '../../runtime-worktree-selection' -import { worktreeIdComparisonKey, worktreeIdsEqual } from '../../../../shared/worktree/id' +import { worktreeIdentityKey, worktreeIdsEqual } from '../../../../shared/worktree/id' import { defineMethod, defineStreamingMethod } from '../core' import { CreateTerminalTab, @@ -121,7 +121,7 @@ export const SESSION_TAB_METHODS = [ } const register = (worktreeId: string): void => { // Why: clients may unsubscribe using the resolved spelling learned from the snapshot. - const cleanupPrefix = `session.tabs:${connectionId ?? 'local'}:${worktreeIdComparisonKey(worktreeId) ?? worktreeId}` + const cleanupPrefix = `session.tabs:${connectionId ?? 'local'}:${worktreeIdentityKey(worktreeId)}` // Why: shared-control can carry multiple subscribers for one worktree on // one socket; include the RPC id so one subscriber cannot evict another. subscriptionId = requestId ? `${cleanupPrefix}:${requestId}` : cleanupPrefix @@ -233,7 +233,7 @@ export const SESSION_TAB_METHODS = [ getExplicitWorktreeIdSelector(params.worktree) ?? (await runtime.listMobileSessionTabs(params.worktree, pairedDeviceId)).worktree const connection = connectionId ?? 'local' - const cleanupPrefix = `session.tabs:${connection}:${worktreeIdComparisonKey(worktreeId) ?? worktreeId}` + const cleanupPrefix = `session.tabs:${connection}:${worktreeIdentityKey(worktreeId)}` if (params.subscriptionId) { runtime.cleanupSubscription(`${cleanupPrefix}:${params.subscriptionId}`) return { unsubscribed: true } diff --git a/src/main/runtime/runtime-worktree-path-identity.ts b/src/main/runtime/runtime-worktree-path-identity.ts index b07a8e93810..3989209086d 100644 --- a/src/main/runtime/runtime-worktree-path-identity.ts +++ b/src/main/runtime/runtime-worktree-path-identity.ts @@ -6,7 +6,7 @@ import { normalizeRuntimePathForComparison } from '../../shared/cross-platform-path' import { parsePtySessionId } from '../../shared/pty-session-id-format' -import { splitWorktreeId, worktreeIdsEqual } from '../../shared/worktree/id' +import { splitWorktreeId, worktreeIdentityKey, worktreeIdsEqual } from '../../shared/worktree/id' import { normalizeLocalBranchName } from './runtime-worktree-selection' export type ResolvedWorktree = Worktree & { @@ -28,30 +28,13 @@ export function runtimePathsEqual(left: string, right: string): boolean { /** Runtime-side name for the shared workspace-identity comparison; see `worktreeIdsEqual`. */ export const runtimeWorktreeIdsEqual = worktreeIdsEqual -export function runtimeWorktreeIdentityKey(worktreeId: string): string { - // Same suffix rule: this keys PTY refresh, sleep, and mutation-queue state per session. - const parsed = splitWorktreeId(worktreeId) - return parsed - ? `${parsed.repoId}\0${normalizeRuntimePathForComparison(parsed.worktreePath)}` - : worktreeId -} - -export function runtimeWorktreeLookupKey(worktreeId: string): string { - const parsed = splitWorktreeId(worktreeId) - return JSON.stringify( - parsed - ? ['parsed', parsed.repoId, normalizeRuntimePathForComparison(parsed.worktreePath)] - : ['raw', worktreeId] - ) -} - export function createIncrementalResolvedWorktreeLookup( resolvedWorktrees: ResolvedWorktree[] ): (worktreeId: string) => ResolvedWorktree | undefined { const worktreeByIdentity = new Map() let indexedCount = 0 return (worktreeId) => { - const lookupKey = runtimeWorktreeLookupKey(worktreeId) + const lookupKey = worktreeIdentityKey(worktreeId) const indexed = worktreeByIdentity.get(lookupKey) if (indexed) { return indexed @@ -59,7 +42,7 @@ export function createIncrementalResolvedWorktreeLookup( while (indexedCount < resolvedWorktrees.length) { const worktree = resolvedWorktrees[indexedCount] indexedCount += 1 - const key = runtimeWorktreeLookupKey(worktree.id) + const key = worktreeIdentityKey(worktree.id) // Why: preserve Array.find's first match when normalized identities collide. if (!worktreeByIdentity.has(key)) { worktreeByIdentity.set(key, worktree) diff --git a/src/main/runtime/sidebar-inputs/orca-runtime-workspace-activity-facts.ts b/src/main/runtime/sidebar-inputs/orca-runtime-workspace-activity-facts.ts index 0cba12733f0..6d6c40b970c 100644 --- a/src/main/runtime/sidebar-inputs/orca-runtime-workspace-activity-facts.ts +++ b/src/main/runtime/sidebar-inputs/orca-runtime-workspace-activity-facts.ts @@ -4,7 +4,7 @@ import type { WorkspaceActivityFactsSnapshot, WorkspaceTerminalLiveness } from '../../../shared/sidebar-list/workspace-activity-facts' -import { worktreeIdComparisonKey } from '../../../shared/worktree/id' +import { worktreeIdentityKey } from '../../../shared/worktree/id' import { SidebarClock } from './sidebar-clock' import { WorkspaceActivityFactsProducer, @@ -126,9 +126,8 @@ export abstract class OrcaRuntimeWithWorkspaceActivityFacts extends OrcaRuntimeW // getLivePtyIdsForWorktree's identity rule in one pass. A host that only lost contact keeps its // terminals unverifiable, never gone, even while the record still reads connected. private readWorkspaceTerminalLiveness(worktreeId: string): WorkspaceTerminalLiveness | null { - const key = worktreeIdComparisonKey(worktreeId) - const owns = (id: string): boolean => - id === worktreeId || (key !== null && worktreeIdComparisonKey(id) === key) + const key = worktreeIdentityKey(worktreeId) + const owns = (id: string): boolean => id === worktreeId || worktreeIdentityKey(id) === key let unverifiable = false for (const surfaces of [this.leaves.values(), this.ptysById.values()]) { for (const surface of surfaces) { diff --git a/src/main/runtime/terminal/query/pty-terminal-summary.ts b/src/main/runtime/terminal/query/pty-terminal-summary.ts index 582773a0202..ceeae78c61f 100644 --- a/src/main/runtime/terminal/query/pty-terminal-summary.ts +++ b/src/main/runtime/terminal/query/pty-terminal-summary.ts @@ -3,10 +3,8 @@ import type { RuntimeLeafRecord, RuntimePtyWorktreeRecord } from '../../runtime-terminal-state-records' -import { - runtimeWorktreeIdentityKey, - type ResolvedWorktree -} from '../../runtime-worktree-path-identity' +import type { ResolvedWorktree } from '../../runtime-worktree-path-identity' +import { worktreeIdentityKey } from '../../../../shared/worktree/id' import type { RuntimeTerminalRead, RuntimeTerminalSummary } from '../../../../shared/runtime-types' import { getLatestPtyTitle } from '../../runtime-worktree-status-projection' import { parsePaneKey } from '../../../../shared/stable-pane-id' @@ -53,7 +51,7 @@ export abstract class OrcaRuntimeWithBuildPtyTerminalSummary extends OrcaRuntime if ( orphaned && (this.pendingPtyRegistrationIncarnations.has(pty.ptyId) || - this.terminalMutationLock.hasActiveSpawns(runtimeWorktreeIdentityKey(pty.worktreeId))) + this.terminalMutationLock.hasActiveSpawns(worktreeIdentityKey(pty.worktreeId))) ) { throw new Error('terminal_surface_ownership_unavailable') } diff --git a/src/main/runtime/terminal/stop/worktree-terminal-sleep.ts b/src/main/runtime/terminal/stop/worktree-terminal-sleep.ts index a177be418b9..c9b9ef08c6e 100644 --- a/src/main/runtime/terminal/stop/worktree-terminal-sleep.ts +++ b/src/main/runtime/terminal/stop/worktree-terminal-sleep.ts @@ -1,11 +1,11 @@ import { OrcaRuntimeWithWorktreeSleepResumeRecording } from './worktree-sleep-resume-recording' -import type { ResolvedWorktree } from '../../runtime-worktree-path-identity' -import type { RuntimeWorktreeTerminalSleepResult } from '../../../../shared/runtime-types' -import { WORKTREE_TERMINAL_SLEEP_TIMEOUT_MS } from '../../orca-runtime-postlude' import { includeTargetResolvedWorktree, - runtimeWorktreeIdentityKey + type ResolvedWorktree } from '../../runtime-worktree-path-identity' +import { worktreeIdentityKey } from '../../../../shared/worktree/id' +import type { RuntimeWorktreeTerminalSleepResult } from '../../../../shared/runtime-types' +import { WORKTREE_TERMINAL_SLEEP_TIMEOUT_MS } from '../../orca-runtime-postlude' import { teardownRpcDeadline } from '../../worktree-teardown' export abstract class OrcaRuntimeWithSleepResolvedWorktreeTerminals extends OrcaRuntimeWithWorktreeSleepResumeRecording { @@ -18,7 +18,7 @@ export abstract class OrcaRuntimeWithSleepResolvedWorktreeTerminals extends Orca 'exclusive', sleepDeadline ) - const key = runtimeWorktreeIdentityKey(worktree.id) + const key = worktreeIdentityKey(worktree.id) const existingSleepState = this.terminalSleepStateByWorktreeId.get(key) if (existingSleepState?.phase === 'sleeping') { try { diff --git a/src/main/runtime/terminal/stop/worktree-terminal-stop.ts b/src/main/runtime/terminal/stop/worktree-terminal-stop.ts index 6b18ee2da8a..7dc3851ef30 100644 --- a/src/main/runtime/terminal/stop/worktree-terminal-stop.ts +++ b/src/main/runtime/terminal/stop/worktree-terminal-stop.ts @@ -1,9 +1,9 @@ import { OrcaRuntimeWithResolveTerminalSplitSourceAuthority } from '../launch/terminal-split-source-authority' import { type ResolvedWorktree, - runtimeWorktreeIdentityKey, runtimeWorktreeIdsEqual } from '../../runtime-worktree-path-identity' +import { worktreeIdentityKey } from '../../../../shared/worktree/id' import { teardownRpcDeadline } from '../../worktree-teardown' import { WORKTREE_TERMINALS_SLEEPING_ERROR } from '../../worktree-terminals-sleeping-error' import type { @@ -261,7 +261,7 @@ export abstract class OrcaRuntimeWithStopTerminalsForWorktree extends OrcaRuntim return () => {} } const release = await this.acquireWorktreeTerminalMutation(worktreeId, 'shared') - const key = runtimeWorktreeIdentityKey(worktreeId) + const key = worktreeIdentityKey(worktreeId) const sleepState = this.terminalSleepStateByWorktreeId.get(key) // Why: re-checked under the lock, since a sleep can commit while an automatic spawn waits for it. if (sleepState && opts.refuseSleptWorktree) { @@ -284,7 +284,7 @@ export abstract class OrcaRuntimeWithStopTerminalsForWorktree extends OrcaRuntim /** Whether the host itself has put (or is putting) this worktree's terminals to sleep. */ protected isWorktreeTerminalSleepHeld(worktreeId: string): boolean { - return this.terminalSleepStateByWorktreeId.has(runtimeWorktreeIdentityKey(worktreeId)) + return this.terminalSleepStateByWorktreeId.has(worktreeIdentityKey(worktreeId)) } protected async runWorktreeTerminalMutation( @@ -306,10 +306,6 @@ export abstract class OrcaRuntimeWithStopTerminalsForWorktree extends OrcaRuntim kind: WorktreeTerminalMutationKind, deadline?: number ): Promise<() => void> { - return await this.terminalMutationLock.acquire( - runtimeWorktreeIdentityKey(worktreeId), - kind, - deadline - ) + return await this.terminalMutationLock.acquire(worktreeIdentityKey(worktreeId), kind, deadline) } } diff --git a/src/main/runtime/workspaces/workspace-selectors.ts b/src/main/runtime/workspaces/workspace-selectors.ts index 0468d7115a9..67def6fad2b 100644 --- a/src/main/runtime/workspaces/workspace-selectors.ts +++ b/src/main/runtime/workspaces/workspace-selectors.ts @@ -1,6 +1,6 @@ import { OrcaRuntimeWithResolveBrowserNetworkExecutionHostForWorktree } from '../terminal/launch/terminal-workspace-scope' import type { ResolvedWorktree } from '../runtime-worktree-path-identity' -import { splitWorktreeIdForFilesystem, worktreeIdComparisonKey } from '../../../shared/worktree/id' +import { splitWorktreeIdForFilesystem, worktreeIdentityKey } from '../../../shared/worktree/id' import { branchSelectorMatches, runtimePathsEqual } from '../runtime-worktree-path-identity' import { getRepoExecutionHostId, getWorktreeExecutionHostId } from '../../../shared/execution-host' import type { @@ -54,10 +54,10 @@ export abstract class RuntimeWorkspaceSelectors extends OrcaRuntimeWithResolveBr const worktreeId = explicitWorktreeId ?? selector.slice(3) candidates = worktrees.filter((worktree) => worktree.id === worktreeId) if (candidates.length === 0) { - const comparisonKey = worktreeIdComparisonKey(worktreeId) - candidates = comparisonKey - ? worktrees.filter((worktree) => worktreeIdComparisonKey(worktree.id) === comparisonKey) - : candidates + const comparisonKey = worktreeIdentityKey(worktreeId) + candidates = worktrees.filter( + (worktree) => worktreeIdentityKey(worktree.id) === comparisonKey + ) } if (candidates.length === 0) { const parsed = splitWorktreeIdForFilesystem(worktreeId) diff --git a/src/shared/worktree/id.test.ts b/src/shared/worktree/id.test.ts index 5cb9ffb479e..c357586def5 100644 --- a/src/shared/worktree/id.test.ts +++ b/src/shared/worktree/id.test.ts @@ -6,7 +6,8 @@ import { getWorktreePathBasenameFromId, splitWorktreeId, splitWorktreeIdForFilesystem, - worktreeIdComparisonKey + worktreeIdentityKey, + worktreeIdsEqual } from './id' describe('getRepoMainWorktreeId', () => { @@ -133,9 +134,9 @@ describe('getWorktreePathBasenameFromId', () => { * #16243: the renderer can only address a workspace by `id:::`, so the key must fold * exactly the path spellings a `path:` selector already folds — and nothing more. */ -describe('worktreeIdComparisonKey path-spelling parity for id: selectors (#16243)', () => { +describe('worktreeIdentityKey path-spelling parity for id: selectors (#16243)', () => { const canonical = 'repo-123::/srv/workspaces/plugin' - const key = (worktreeId: string): string | null => worktreeIdComparisonKey(worktreeId) + const key = (worktreeId: string): string => worktreeIdentityKey(worktreeId) it('folds the path spellings a `path:` selector already accepts', () => { expect(key('repo-123::/srv/workspaces/plugin/')).toBe(key(canonical)) @@ -147,7 +148,7 @@ describe('worktreeIdComparisonKey path-spelling parity for id: selectors (#16243 it('folds no more loosely than `path:` does', () => { // A leading `//` is a UNC root, not a doubled separator. - expect(key('repo-123://srv/workspaces/plugin')).not.toBe(key(canonical)) + expect(key('repo-123:://srv/workspaces/plugin')).not.toBe(key(canonical)) // Dot segments are not canonicalized, so `id:` and `path:` still agree on refusing them. expect(key('repo-123::/srv/./workspaces/plugin')).not.toBe(key(canonical)) }) @@ -165,13 +166,13 @@ describe('worktreeIdComparisonKey path-spelling parity for id: selectors (#16243 it('keeps a UNC or WSL root distinct from a drive-letter location', () => { // Different roots naming different filesystems must never collapse into one key. - expect(key('repo-123://server/share/game2')).not.toBe(key('repo-123::D:/server/share/game2')) - expect(key('repo-123://wsl.localhost/Ubuntu/home/dev/game2')).not.toBe( + expect(key('repo-123:://server/share/game2')).not.toBe(key('repo-123::D:/server/share/game2')) + expect(key('repo-123:://wsl.localhost/Ubuntu/home/dev/game2')).not.toBe( key('repo-123::D:/home/dev/game2') ) // The two UNC aliases Windows exposes for one WSL distro are the same location. - expect(key('repo-123://wsl.localhost/Ubuntu/home/dev/game2')).toBe( - key('repo-123://wsl$/Ubuntu/home/dev/game2') + expect(key('repo-123:://wsl.localhost/Ubuntu/home/dev/game2')).toBe( + key('repo-123:://wsl$/Ubuntu/home/dev/game2') ) }) @@ -184,9 +185,14 @@ describe('worktreeIdComparisonKey path-spelling parity for id: selectors (#16243 ) }) - it('returns null for ids with no repo boundary so callers keep exact matching', () => { - expect(key('repo-a')).toBeNull() - expect(key('repo-a::')).toBeNull() - expect(key('/srv/workspaces/plugin')).toBeNull() + it('keys a malformed id as itself, so it only ever matches itself', () => { + for (const malformed of ['repo-a', 'repo-a::', '::/srv/a', '/srv/workspaces/plugin']) { + expect(key(malformed)).toBe(malformed) + expect(worktreeIdsEqual(malformed, malformed)).toBe(true) + } + // A malformed spelling never collides with a well-formed id's key. + expect(key('repo-a::')).not.toBe(key('repo-a::/')) + expect(worktreeIdsEqual('repo-a::', 'repo-a::/')).toBe(false) + expect(worktreeIdsEqual('::/srv/A/', '::/srv/A')).toBe(false) }) }) diff --git a/src/shared/worktree/id.ts b/src/shared/worktree/id.ts index 6f7df555199..f8ef4f81d4f 100644 --- a/src/shared/worktree/id.ts +++ b/src/shared/worktree/id.ts @@ -28,15 +28,15 @@ export function getRepoIdFromWorktreeId(worktreeId: string): string { } /** - * Canonical comparison form of a worktree id: the repoId is compared EXACT and only the path folds, + * Comparison-only identity of a worktree id: the repoId is compared EXACT and only the path folds, * through the same `normalizeRuntimePathForComparison` a `path:` selector has always applied and - * byte-exact id matching denied the renderer (#16243). Null for a malformed id, so callers keep - * exact matching for it. Comparison only — never persist or return this key. + * byte-exact id matching denied the renderer (#16243). A malformed id keys as itself, so it only + * ever matches itself. Never persist or return this key. */ -export function worktreeIdComparisonKey(worktreeId: string): string | null { +export function worktreeIdentityKey(worktreeId: string): string { const parsed = splitWorktreeId(worktreeId) if (!parsed || !parsed.repoId || !parsed.worktreePath) { - return null + return worktreeId } return `${parsed.repoId}${WORKTREE_ID_SEPARATOR}${normalizeRuntimePathForComparison( parsed.worktreePath @@ -47,12 +47,10 @@ export function worktreeIdComparisonKey(worktreeId: string): string | null { * Why: workspace identity is per *workspace*, not per checkout dir. Folder projects back several * independent workspaces with one directory, separated only by the `::workspace:` suffix that * filesystem callers must strip; stripping it here instead lets one session steal a sibling's PTYs. - * Normalize only path spelling, so Windows/WSL/SSH ids still match themselves across hosts, and - * fall back to exact equality for a malformed id. + * Normalize only path spelling, so Windows/WSL/SSH ids still match themselves across hosts. */ export function worktreeIdsEqual(left: string, right: string): boolean { - const leftKey = worktreeIdComparisonKey(left) - return leftKey === null ? left === right : leftKey === worktreeIdComparisonKey(right) + return worktreeIdentityKey(left) === worktreeIdentityKey(right) } export function splitWorktreeId(worktreeId: string): ParsedWorktreeId | null {