fix(worktrees): resolve id: worktree selectors by path equivalence (#16243) (#16494)

* test(worktrees): cover id: selector path-spelling parity with path: (#16243)

The renderer can only address a workspace by id (toRuntimeWorktreeSelector always
emits id:<repoId>::<path>), and the runtime matches that id byte for byte while a
path: selector has always compared through normalizeRuntimePathForComparison. A
stored id that spells its path differently from `git worktree list` therefore
resolves for the CLI and answers selector_not_found for the UI, which reads that
as a stale local mirror, calls forgetLocal, reports success, and lets the row
return on the next catalog refresh: a silent delete.

These tests fail on both resolution sites -- the fleet `id:` branch of
resolveWorktreeSelector and the scoped resolveScopedWorktreeIdRow a
host-qualified removal takes -- and pin what must stay closed: an exact repo id
(STA-4343), host qualification, dot segments neither selector canonicalizes, and
a refusal rather than a guess when two rows spell one path.

13 failing, 35 passing.

* fix(worktrees): resolve id: worktree selectors by path equivalence (#16243)

worktreeIdComparisonKey names one repo, one filesystem location, and one
folder-workspace instance, folding exactly the path spellings
normalizeRuntimePathForComparison already folds for a path: selector -- and
nothing more, so dot segments stay unresolved for both shapes. Both id:
resolution sites consult it only after an exact match finds nothing: the fleet
branch of resolveWorktreeSelector and resolveScopedWorktreeIdRow, which a
host-qualified removal takes. runtimeWorktreeIdsEqual now derives from the same
key so the runtime has one normalizer rather than a parallel one.

Not a pure refactor at that last site: runtimeWorktreeIdsEqual used to
normalize-compare ids that parse but carry an empty repoId or an empty path
('::/p', or 'repo::' against 'repo::/'), and worktreeIdComparisonKey returns
null for those, so across its call sites (PTY identity, refresh, mutation
queue) such ids now compare byte-exact instead. That narrows matching rather
than widening it, no real worktree carries such an id, and it is the behavior
#15616 guarantees for malformed ids -- but it is a behavior delta, not just a
tidy-up.

Perf (#14399): the exact match is still tried first and still wins outright, so
a resolvable id costs exactly what it did before. Neither site adds a scan --
the fleet branch re-filters the array it had already listed, the scoped lookup
re-filters the single owning repo's projected rows -- so an explicit id still
never scans every repo.

Fail-closed behavior is unchanged: the repo id compares exactly (STA-4343), host
qualification is untouched, the folder-workspace instance suffix stays part of
the path, and a scoped lookup with two equivalent rows refuses instead of
guessing. The bare unprefixed selector branch keeps byte-exact id matching,
since only the id: shape reaches a renderer caller.

Shares src/shared/worktree/id.ts with the open #15616, which introduces
worktreeIdComparisonKey for the same divergence in lineage pruning and
authoritative-scan purging; this adopts that helper rather than adding a second
one. Complementary to the open #16295, which makes the miss visible; this
removes the miss.

* chore(worktrees): satisfy oxfmt and oxlint on #16243 tests

oxfmt --check flagged both new test files and oxlint's
unicorn/no-useless-fallback-in-spread flagged the store mock; the full
lint and format gates now match the pre-change baseline.

* test(worktrees): pin Windows spellings and malformed-id exactness (#16243)

Review found two axes the first pass left unproven at the two id: resolution
sites. Both are the invariants the open #15616 guarantees for the shared
worktreeIdComparisonKey it introduces for #15598, so violating either here would
break a contract a sibling PR depends on.

Windows: #15598's whole defect is that one checkout is recorded under both
`D:\Agentic\game2` and `D:/Agentic/game2`. The fleet branch, the scoped removal
lookup, and the key itself now each resolve the backslash spelling against the
forward-slash spelling git reports, and fold drive-letter case -- while a
backslash inside a POSIX path stays a filename character and a POSIX root stays
case-sensitive, exactly as normalizeRuntimePathForComparison already decides for
a path: selector.

Malformed ids keep exact matching at both sites: an id with no repo boundary or
an empty path still refuses, and the scoped lookup still refuses it without
scanning.

Four of these fail without the production change (three fleet/removal Windows
cases and the scoped one); the malformed-id and POSIX-backslash cases are
invariant guards that hold either way.

Verified: 58 passed in the three files; 18 fail with the production hunks
reverted; orca-runtime.test.ts and worktree-teardown-unstopped-pty.test.ts green
(1270 passed | 1 skipped); pnpm tc:node clean.

* test(worktrees): pin Windows id: spelling folds and fleet ambiguity refusal (#16243)

The Windows backslash spelling now rides the ID_SPELLINGS rows, so it is driven
through both id: sites -- resolveWorktreeSelector and the scoped removal target --
and compared against what the same workspace's path: selector resolves, rather
than only through worktreeIdComparisonKey. That is the spelling #15598/#15616
found in the wild and the one the owner's Windows client produces.

The fleet path's ambiguity refusal had no test: two same-repo rows spelling one
directory, an id: matching neither exactly, must reject selector_ambiguous. It is
the fail-closed guard on a delete-capable resolver, and the property a later
refactor is most likely to turn into a silent pick.

Also records two limits at the source instead of leaving them to be rediscovered:
a UNC or WSL root never folds into a drive-letter location (while Windows' two
WSL UNC aliases do name one location), and a folder-workspace id keeps a trailing
slash placed before the ::workspace:<uuid> suffix, so that spelling stays
exact-match-only. Neither behavior changes here.

The file docblock overclaimed parity. path: collapses duplicate same-host
registrations to the first row while a folded id: refuses them; the contract this
file pins is path-spelling parity, not dedup parity, and the divergence is
deliberate because this resolver also serves delete.

Non-vacuity, verified by temporarily reverting the production hunks: neutralizing
both id: fallbacks turns 10 of these tests red, including both new Windows rows
and the ambiguity refusal (it degrades to selector_not_found). Making the fleet
fallback pick the first folded match instead of collecting all of them turns the
ambiguity test red on its own. The remaining cases -- malformed ids, dot
segments, the POSIX backslash, the folder-workspace slash -- pass against the
pre-fix code too: they guard against future widening rather than proving this
fix.

Drops the two Windows cases the ID_SPELLINGS row subsumes.

The drive-letter case test asserted only the Windows half its name promised; it
now also pins that a POSIX root does NOT fold case, since an unconditional
lowercase would merge /data/Foo with /data/foo on the platform CI runs on.

Fixture paths use the upstream-attested /srv/projects prefix (and a neutral
plugin-host leaf) instead of a local install root; the spelling variations the
tests exist to pin -- doubled separator, dot segment, trailing slash, uppercase
POSIX, cafe NFC/NFD, and the Windows D: rows -- are unchanged in form.

* docs(worktrees): trim the id: selector test header and document the comparison key (#16243)
This commit is contained in:
Lesley Murfin
2026-08-26 23:35:18 -07:00
committed by GitHub
parent 8d0156ae0b
commit de6fe8b7ea
6 changed files with 473 additions and 10 deletions
+15 -7
View File
@@ -534,7 +534,8 @@ import {
WORKTREE_ID_SEPARATOR,
getRepoIdFromWorktreeId,
splitWorktreeId,
splitWorktreeIdForFilesystem
splitWorktreeIdForFilesystem,
worktreeIdComparisonKey
} from '../../shared/worktree/id'
import {
getProjectIdForProviderIdentity,
@@ -32294,6 +32295,7 @@ export class OrcaRuntimeService {
return worktreeId
}
/** Resolves one workspace or throws `selector_not_found` / `selector_ambiguous` — never picks a winner. */
private async resolveWorktreeSelector(selector: string): Promise<ResolvedWorktree> {
const explicitWorktreeId = this.getValidatedExplicitWorktreeIdSelector(selector)
// Why only `id:`: every other selector kind is matched across the whole fleet, and their
@@ -32315,6 +32317,16 @@ export class OrcaRuntimeService {
if (selector.startsWith('id:')) {
const worktreeId = explicitWorktreeId ?? selector.slice(3)
candidates = worktrees.filter((worktree) => worktree.id === worktreeId)
if (candidates.length === 0) {
// Why (#16243): `id:` is the only shape the renderer can send, and a stored id can spell
// its path differently from the scan — the divergence `path:` has always absorbed.
// The bare unprefixed branch below stays byte-exact on purpose: only `id:` reaches a
// renderer caller, so `id:repo::p/` folds here while bare `repo::p/` still misses.
const comparisonKey = worktreeIdComparisonKey(worktreeId)
candidates = comparisonKey
? worktrees.filter((worktree) => worktreeIdComparisonKey(worktree.id) === comparisonKey)
: candidates
}
if (candidates.length === 0) {
const parsed = splitWorktreeIdForFilesystem(worktreeId)
const repo = parsed ? this.store?.getRepo(parsed.repoId) : null
@@ -41428,12 +41440,8 @@ function runtimePathsEqual(left: string, right: string): boolean {
* Windows/WSL/SSH ids still match themselves across hosts.
*/
function runtimeWorktreeIdsEqual(left: string, right: string): boolean {
const parsedLeft = splitWorktreeId(left)
const parsedRight = splitWorktreeId(right)
return parsedLeft && parsedRight
? parsedLeft.repoId === parsedRight.repoId &&
runtimePathsEqual(parsedLeft.worktreePath, parsedRight.worktreePath)
: left === right
const leftKey = worktreeIdComparisonKey(left)
return leftKey === null ? left === right : leftKey === worktreeIdComparisonKey(right)
}
function runtimeWorktreeIdentityKey(worktreeId: string): string {
@@ -200,3 +200,88 @@ describe('host-qualified scoped worktree resolution', () => {
expect(getRepos).not.toHaveBeenCalled()
})
})
/**
* #16243: the renderer can only address a workspace by `id:<repoId>::<path>`, and this scoped
* lookup is what a host-qualified removal resolves through. It matched the id byte for byte while a
* `path:` selector has always compared through `normalizeRuntimePathForComparison`, so a stored id
* spelling its path differently from `git worktree list` resolved for the CLI and not for the UI.
*/
describe('scoped worktree id resolution across path spellings (#16243)', () => {
it.each([
['a trailing slash', '/same/worktree', 'shared::/same/worktree/'],
['a doubled separator', '/same/worktree', 'shared::/same//worktree'],
['an NFD name', '/same/café', `shared::${'/same/café'.normalize('NFD')}`]
])('resolves the scanned row when the id carries %s', async (_label, scannedPath, worktreeId) => {
const owner = repo('shared', '/local/repo', { executionHostId: 'local' })
const deps = createDeps([owner])
deps.scanRepo.mockImplementation(async () => ({
ok: true,
worktrees: [gitWorktree(scannedPath)]
}))
await expect(resolveScopedWorktreeIdRow(deps, worktreeId, 'local')).resolves.toMatchObject({
id: `shared::${scannedPath}`,
path: scannedPath
})
})
it('still refuses the same path under a different repo id', async () => {
const deps = createDeps([
repo('shared', '/local/repo', { executionHostId: 'local' }),
repo('unrelated', '/unrelated/repo', { executionHostId: 'local' })
])
await expect(
resolveScopedWorktreeIdRow(deps, 'unrelated::/same/worktree/', 'local')
).resolves.toBeNull()
})
it('refuses rather than guessing when two rows spell one path', async () => {
const owner = repo('shared', '/local/repo', { executionHostId: 'local' })
const deps = createDeps([owner])
deps.scanRepo.mockImplementation(async () => ({
ok: true,
worktrees: [gitWorktree('/same/worktree'), gitWorktree('/same//worktree')]
}))
await expect(
resolveScopedWorktreeIdRow(deps, 'shared::/same/worktree/', 'local')
).resolves.toBeNull()
})
it('prefers the exactly matching row over an equivalent spelling', async () => {
const owner = repo('shared', '/local/repo', { executionHostId: 'local' })
const deps = createDeps([owner])
deps.scanRepo.mockImplementation(async () => ({
ok: true,
worktrees: [gitWorktree('/same//worktree'), gitWorktree('/same/worktree')]
}))
await expect(
resolveScopedWorktreeIdRow(deps, 'shared::/same//worktree', 'local')
).resolves.toMatchObject({ id: 'shared::/same//worktree' })
})
// #15598/#15616: the backslash spelling is what a pre-restart Windows registration recorded.
it('resolves a Windows backslash id against the forward-slash spelling git reports', async () => {
const path = 'D:/Agentic/game2/battle-core'
const owner = repo('shared', 'D:/Agentic/game2', { executionHostId: 'runtime:windows' })
const deps = createDeps([owner])
deps.scanRepo.mockImplementation(async () => ({ ok: true, worktrees: [gitWorktree(path)] }))
await expect(
resolveScopedWorktreeIdRow(deps, 'shared::D:\\Agentic\\game2\\battle-core', 'runtime:windows')
).resolves.toMatchObject({ id: `shared::${path}`, path })
})
it.each([
['no repo boundary', 'not-an-id'],
['an empty path', 'shared::']
])('keeps exact matching for a malformed id with %s', async (_label, worktreeId) => {
const deps = createDeps([repo('shared', '/local/repo', { executionHostId: 'local' })])
await expect(resolveScopedWorktreeIdRow(deps, worktreeId, 'local')).resolves.toBeNull()
expect(deps.scanRepo).not.toHaveBeenCalled()
})
})
@@ -1,4 +1,8 @@
import { splitWorktreeId, splitWorktreeIdForFilesystem } from '../../shared/worktree/id'
import {
splitWorktreeId,
splitWorktreeIdForFilesystem,
worktreeIdComparisonKey
} from '../../shared/worktree/id'
import { getRepoExecutionHostId, type ExecutionHostId } from '../../shared/execution-host'
import { isFolderRepo } from '../../shared/repo-kind'
import { projectResolvedWorktreeLineage } from '../../shared/resolved-worktree-lineage'
@@ -195,5 +199,18 @@ export async function resolveScopedWorktreeIdRow(
resolveLocalProjectRuntimesForRepos(store, [repo])
)
const projected = projectResolvedWorktreeLineage(rows, store.getAllWorktreeLineage?.() ?? {})
return projected.find((worktree) => worktree.id === worktreeId) ?? null
const exact = projected.find((worktree) => worktree.id === worktreeId)
if (exact) {
return exact
}
// 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)
if (comparisonKey === null) {
return null
}
const equivalent = projected.filter(
(worktree) => worktreeIdComparisonKey(worktree.id) === comparisonKey
)
return equivalent.length === 1 ? equivalent[0] : null
}
@@ -0,0 +1,273 @@
/**
* Pins path-SPELLING parity between an `id:` selector and `path:`, deliberately not dedupe parity:
* where two same-host rows spell one directory, `path:` collapses them to the first while `id:`
* refuses as ambiguous, because this resolver also serves delete (#16243).
*/
import { beforeEach, describe, expect, it, vi } from 'vitest'
const electronMocks = vi.hoisted(() => {
const ipcMain = {
on: vi.fn(() => ipcMain),
removeListener: vi.fn(() => ipcMain),
emit: vi.fn(() => true)
}
return {
BrowserWindow: { fromId: vi.fn((): unknown => null) },
webContents: { fromId: vi.fn((): unknown => null) },
ipcMain,
app: { getPath: vi.fn(() => '/tmp'), isPackaged: false }
}
})
vi.mock('electron', () => electronMocks)
const getSshGitProviderMock = vi.hoisted(() => vi.fn())
vi.mock('../providers/ssh-git-dispatch', () => ({
getSshGitProvider: getSshGitProviderMock,
getSshGitProviderGeneration: vi.fn(() => 0),
SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE: 'unavailable',
requireSshGitProvider: (connectionId: string) => getSshGitProviderMock(connectionId)
}))
const listWorktreesStrictMock = vi.hoisted(() => vi.fn())
vi.mock('../git/worktree', async (importOriginal) => ({
...(await importOriginal<Record<string, unknown>>()),
listWorktreesStrict: listWorktreesStrictMock
}))
import { LOCAL_EXECUTION_HOST_ID } from '../../shared/execution-host'
import { OrcaRuntimeService } from './orca-runtime'
const REPO_ID = 'repo-local'
const REPO_PATH = '/srv/projects/app'
/** The spelling `git worktree list` reports. */
const WORKTREE_PATH = '/srv/projects/workspaces/plugin-host'
const CANONICAL_ID = `${REPO_ID}::${WORKTREE_PATH}`
/** One directory, other spellings a stored id can legitimately carry. */
const ID_SPELLINGS: [label: string, worktreePath: string, repoPath?: string][] = [
['a trailing slash', `${WORKTREE_PATH}/`],
['a doubled separator', '/srv/projects//workspaces/plugin-host'],
['an NFD workspace name', '/srv/projects/workspaces/café-plugin'.normalize('NFD')],
// #15598/#15616: a Windows registration records backslashes; git reports forward slashes.
['a backslash Windows spelling', 'D:\\Agentic\\game2\\battle-core', 'D:/Agentic/game2']
]
/** What a scan reports for a stored id: the same directory, canonically spelled. */
function scannedSpellingOf(storedPath: string): string {
const slashed = /^[A-Za-z]:[\\/]/.test(storedPath) ? storedPath.replace(/\\/g, '/') : storedPath
return slashed.normalize('NFC').replace(/\/+/g, '/').replace(/\/$/, '')
}
/** One registered repo whose worktree meta is writable, so a delete's `forgetLocal` is observable. */
function makeStore(repoPath: string = REPO_PATH) {
const metaById: Record<string, Record<string, unknown>> = {}
const store = {
getRepo: (id: string) => store.getRepos().find((repo) => repo.id === id),
getRepos: () => [
{ id: REPO_ID, path: repoPath, displayName: 'app', badgeColor: 'blue', addedAt: 1 }
],
getAllWorktreeMeta: vi.fn(() => metaById),
getWorktreeMeta: (id: string) => metaById[id],
setWorktreeMeta: (id: string, meta: Record<string, unknown>) => {
metaById[id] = { ...metaById[id], ...meta }
return metaById[id]
},
removeWorktreeMeta: () => {},
getAllWorktreeLineage: () => ({}),
getAllWorkspaceLineage: () => ({}),
removeWorktreeLineage: vi.fn(),
removeWorkspaceLineage: vi.fn(),
getGitHubCache: () => undefined as never,
getSettings: () => ({
workspaceDir: '/tmp/workspaces',
nestWorkspaces: false,
refreshLocalBaseRefOnWorktreeCreate: false,
branchPrefix: 'none',
branchPrefixCustom: ''
}),
getProjects: () => []
}
return store
}
/** `git worktree list` output: the main checkout plus one workspace at `worktreePath`. */
function scanReports(worktreePath: string, repoPath: string = REPO_PATH): void {
listWorktreesStrictMock.mockResolvedValue([
{ path: repoPath, head: 'abc', branch: 'main', isBare: false, isMainWorktree: true },
{ path: worktreePath, head: 'def', branch: 'feature', isBare: false, isMainWorktree: false }
])
}
type RemovalInternals = {
resolveWorktreeRemovalTarget: (
worktreeSelector: string,
requiredHostId?: string
) => Promise<{ id: string; repoId: string; path: string }>
}
beforeEach(() => {
getSshGitProviderMock.mockReset()
listWorktreesStrictMock.mockReset()
})
describe('worktree id selectors vs. the path spelling git reports (#16243)', () => {
it.each(ID_SPELLINGS)(
'resolves through the fleet path what `path:` resolves when the id carries %s',
async (_label, storedPath, repoPath) => {
scanReports(scannedSpellingOf(storedPath), repoPath)
const runtime = new OrcaRuntimeService(makeStore(repoPath) as never)
// The CLI's shape, and the live data point: `path:` already resolves it.
const byPath = await runtime.showManagedWorktree(`path:${storedPath}`)
// The only shape the renderer can send must resolve the SAME workspace.
await expect(
runtime.showManagedWorktree(`id:${REPO_ID}::${storedPath}`)
).resolves.toMatchObject({ id: byPath.id, path: byPath.path })
}
)
it.each(ID_SPELLINGS)(
'resolves a host-qualified removal target when the id carries %s',
async (_label, storedPath, repoPath) => {
scanReports(scannedSpellingOf(storedPath), repoPath)
const runtime = new OrcaRuntimeService(makeStore(repoPath) as never)
const internals = runtime as unknown as RemovalInternals
// The scoped path the UI's delete takes must agree with the fleet path above.
await expect(
internals.resolveWorktreeRemovalTarget(
`id:${REPO_ID}::${storedPath}`,
LOCAL_EXECUTION_HOST_ID
)
).resolves.toMatchObject({ repoId: REPO_ID, path: scannedSpellingOf(storedPath) })
}
)
it('still refuses an id whose path names a different workspace', async () => {
scanReports(WORKTREE_PATH)
const runtime = new OrcaRuntimeService(makeStore() as never)
await expect(
runtime.showManagedWorktree(`id:${REPO_ID}::/srv/projects/workspaces/other-plugin`)
).rejects.toThrow('selector_not_found')
})
// STA-4343: matching across repo ids would delete a workspace the caller never confirmed.
it('still refuses the same path under a different repo id', async () => {
scanReports(WORKTREE_PATH)
const runtime = new OrcaRuntimeService(makeStore() as never)
await expect(runtime.showManagedWorktree(`id:other-repo::${WORKTREE_PATH}/`)).rejects.toThrow(
'selector_not_found'
)
})
it('still refuses a removal qualified to a host that does not own the repo id', async () => {
scanReports(WORKTREE_PATH)
const runtime = new OrcaRuntimeService(makeStore() as never)
const internals = runtime as unknown as RemovalInternals
await expect(
internals.resolveWorktreeRemovalTarget(`id:${CANONICAL_ID}/`, 'runtime:env-b')
).rejects.toThrow('selector_not_found')
})
it('keeps `id:` and `path:` agreeing on a dot segment neither canonicalizes', async () => {
scanReports(WORKTREE_PATH)
const runtime = new OrcaRuntimeService(makeStore() as never)
const dotted = '/srv/projects/./workspaces/plugin-host'
await expect(runtime.showManagedWorktree(`path:${dotted}`)).rejects.toThrow(
'selector_not_found'
)
await expect(runtime.showManagedWorktree(`id:${REPO_ID}::${dotted}`)).rejects.toThrow(
'selector_not_found'
)
})
// #15598/#15616: on Windows one checkout is recorded under both spellings, and git reports the
// forward-slash one. The backslash id itself rides the ID_SPELLINGS rows above; these pin the
// folding limits around it.
describe('Windows drive-letter spellings', () => {
const WINDOWS_REPO = 'D:/Agentic/game2'
const WINDOWS_WORKTREE = 'D:/Agentic/game2/battle-core'
it('folds drive-letter case only for Windows paths, never for a POSIX path', async () => {
scanReports(WINDOWS_WORKTREE, WINDOWS_REPO)
const windowsRuntime = new OrcaRuntimeService(makeStore(WINDOWS_REPO) as never)
// A Windows root is case-insensitive, as `path:` already treats it.
await expect(
windowsRuntime.showManagedWorktree(`id:${REPO_ID}::d:/agentic/game2/battle-core`)
).resolves.toMatchObject({ path: WINDOWS_WORKTREE })
// A POSIX root is not: folding case there would merge distinct directories for a delete.
scanReports(WORKTREE_PATH)
const posixRuntime = new OrcaRuntimeService(makeStore() as never)
await expect(
posixRuntime.showManagedWorktree(`id:${REPO_ID}::/SRV/projects/workspaces/plugin-host`)
).rejects.toThrow('selector_not_found')
})
it('does not fold a backslash inside a POSIX path, where it is a valid filename character', async () => {
scanReports(WORKTREE_PATH)
const runtime = new OrcaRuntimeService(makeStore() as never)
await expect(
runtime.showManagedWorktree(`id:${REPO_ID}::/srv/projects\\workspaces\\plugin-host`)
).rejects.toThrow('selector_not_found')
})
})
// The fail-closed guard on a delete-capable resolver: two rows spelling one directory must not
// let a folded id pick one. `path:` collapses same-host duplicates; `id:` deliberately refuses.
it('refuses a folded id when two same-repo rows spell one directory', async () => {
listWorktreesStrictMock.mockResolvedValue([
{ path: REPO_PATH, head: 'abc', branch: 'main', isBare: false, isMainWorktree: true },
{ path: WORKTREE_PATH, head: 'def', branch: 'feature', isBare: false, isMainWorktree: false },
{
path: '/srv/projects//workspaces/plugin-host',
head: 'ghi',
branch: 'feature-2',
isBare: false,
isMainWorktree: false
}
])
const runtime = new OrcaRuntimeService(makeStore() as never)
// Matches neither row exactly, folds to both.
await expect(runtime.showManagedWorktree(`id:${CANONICAL_ID}/`)).rejects.toThrow(
'selector_ambiguous'
)
})
// Live-proof limit, pinned so nobody "fixes" the trimming: a folder-workspace id only trims a
// trailing slash at end of string, so one placed before the instance suffix stays exact-only.
it('keeps a folder-workspace id exact when a slash precedes the instance suffix', async () => {
const instanceSuffix = '::workspace:123e4567-e89b-12d3-a456-426614174000'
scanReports(WORKTREE_PATH)
const runtime = new OrcaRuntimeService(makeStore() as never)
await expect(
runtime.showManagedWorktree(`id:${REPO_ID}::${WORKTREE_PATH}/${instanceSuffix}`)
).rejects.toThrow('selector_not_found')
})
// #15616 guarantees malformed ids keep exact-match behavior; both sites must honour that too.
it.each([
['no repo boundary', 'not-an-id'],
['an empty path', `${REPO_ID}::`]
])('keeps exact matching for a malformed id with %s', async (_label, malformedId) => {
scanReports(WORKTREE_PATH)
const runtime = new OrcaRuntimeService(makeStore() as never)
const internals = runtime as unknown as RemovalInternals
await expect(runtime.showManagedWorktree(`id:${malformedId}`)).rejects.toThrow(
'selector_not_found'
)
await expect(
internals.resolveWorktreeRemovalTarget(`id:${malformedId}`, LOCAL_EXECUTION_HOST_ID)
).rejects.toThrow('selector_not_found')
})
})
+64 -1
View File
@@ -4,7 +4,8 @@ import {
getRepoIdFromWorktreeId,
getWorktreePathBasenameFromId,
splitWorktreeId,
splitWorktreeIdForFilesystem
splitWorktreeIdForFilesystem,
worktreeIdComparisonKey
} from './id'
describe('WORKTREE_ID_SEPARATOR', () => {
@@ -119,3 +120,65 @@ describe('getWorktreePathBasenameFromId', () => {
expect(getWorktreePathBasenameFromId('repo-123::')).toBeNull()
})
})
/**
* #16243: the renderer can only address a workspace by `id:<repoId>::<path>`, 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)', () => {
const canonical = 'repo-123::/srv/workspaces/plugin'
const key = (worktreeId: string): string | null => worktreeIdComparisonKey(worktreeId)
it('folds the path spellings a `path:` selector already accepts', () => {
expect(key('repo-123::/srv/workspaces/plugin/')).toBe(key(canonical))
expect(key('repo-123::/srv//workspaces/plugin')).toBe(key(canonical))
expect(key('repo-123::/srv/workspaces/Café'.normalize('NFD'))).toBe(
key('repo-123::/srv/workspaces/Café'.normalize('NFC'))
)
})
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))
// 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))
})
// #15598/#15616: the same Windows checkout is recorded with both separators.
it('folds Windows separator and drive-letter case, as `path:` already does', () => {
const windows = 'repo-123::D:/Agentic/game2'
expect(key('repo-123::D:\\Agentic\\game2')).toBe(key(windows))
expect(key('repo-123::d:/agentic/game2')).toBe(key(windows))
// A backslash is a valid POSIX filename character, so a POSIX path never folds it.
expect(key('repo-123::/srv\\workspaces')).not.toBe(key('repo-123::/srv/workspaces'))
// POSIX roots stay case-sensitive.
expect(key('repo-123::/srv/Workspaces')).not.toBe(key('repo-123::/srv/workspaces'))
})
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(
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')
)
})
it('never merges different repos, workspaces, or folder sessions', () => {
// STA-4343: the repo id stays exact, or a removal lands on a repo the caller never confirmed.
expect(key('repo-999::/srv/workspaces/plugin')).not.toBe(key(canonical))
expect(key('repo-123::/srv/workspaces/other')).not.toBe(key(canonical))
expect(key('repo-a::/srv/folder::workspace:123e4567-e89b-12d3-a456-426614174000')).not.toBe(
key('repo-a::/srv/folder::workspace:123e4567-e89b-12d3-a456-426614174001')
)
})
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()
})
})
+17
View File
@@ -1,3 +1,4 @@
import { normalizeRuntimePathForComparison } from '../cross-platform-path'
import { WORKTREE_ID_SEPARATOR } from '../pty-session-id-format'
export { WORKTREE_ID_SEPARATOR } from '../pty-session-id-format'
@@ -17,6 +18,22 @@ export function getRepoIdFromWorktreeId(worktreeId: string): string {
return separatorIdx === -1 ? worktreeId : worktreeId.slice(0, separatorIdx)
}
/**
* Canonical comparison form 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.
*/
export function worktreeIdComparisonKey(worktreeId: string): string | null {
const parsed = splitWorktreeId(worktreeId)
if (!parsed || !parsed.repoId || !parsed.worktreePath) {
return null
}
return `${parsed.repoId}${WORKTREE_ID_SEPARATOR}${normalizeRuntimePathForComparison(
parsed.worktreePath
)}`
}
export function splitWorktreeId(worktreeId: string): ParsedWorktreeId | null {
const separatorIdx = worktreeId.indexOf(WORKTREE_ID_SEPARATOR)
if (separatorIdx === -1) {