chore(worktree): composeWorktreeId plus a ratchet on hand-built worktree ids (#30238)

Worktree ids built as `${repo.id}::${path}` by hand are how one worktree got two
spellings (#30150). New code composes them through src/shared/worktree/id.ts; the
ratchet counts the remaining hand-built ids per file (18) and may only go down. Six
byte-identical `${repo.id}::${repo.path}` sites move to getRepoMainWorktreeId.
This commit is contained in:
Neil
2026-10-18 00:51:41 -07:00
committed by GitHub
parent 8f86ccf764
commit a2946f0a65
14 changed files with 207 additions and 14 deletions
+4
View File
@@ -400,6 +400,10 @@ jobs:
if: needs.code_paths.outputs.static_analysis == 'true'
run: node config/scripts/check-path-literal-ratchet.mjs
- name: Enforce worktree-id composer ratchet
if: needs.code_paths.outputs.static_analysis == 'true'
run: node config/scripts/check-worktree-id-composer-ratchet.mjs
- name: Check Node runtime pin
if: needs.code_paths.outputs.static_analysis == 'true'
run: node config/scripts/check-node-runtime-pin.mjs
@@ -0,0 +1,96 @@
import fs from 'node:fs'
import path from 'node:path'
import process from 'node:process'
import { pathToFileURL } from 'node:url'
import {
diffCounts,
parseCountBaseline,
pruneCountBaseline,
readTrackedSources,
totalCount
} from './ratchet-baseline.mjs'
// Ratchet on worktree ids built by hand (`${repo.id}::${path}`) instead of through the composers in
// src/shared/worktree/id.ts. About 22 hand-built ids each pick whichever path spelling is at hand,
// which is how one worktree ends up with two ids (#30150). Counts per file may only go down.
const BASELINE_PATH = 'config/worktree-id-composer-baseline.txt'
const SCAN_ROOTS = ['src', 'mobile/src']
const COMPOSER_FILE = 'src/shared/worktree/id.ts'
const HEADER = [
'# Hand-built worktree ids (`${repoId}::${path}`) per file, outside src/shared/worktree/id.ts.',
'# This is a RATCHET: counts may only go DOWN. Compose ids with composeWorktreeId or',
'# getRepoMainWorktreeId instead. Prune after a fix: pnpm check:worktree-id-composer-ratchet --prune'
]
// A repo-id interpolation, the `::` separator (literal or constant), then a path interpolation.
const HAND_BUILT_WORKTREE_ID =
/\$\{[^{}]*\b(?:repo\.id|repoId|repo_id)\b[^{}]*\}(?:::|\$\{\s*WORKTREE_ID_SEPARATOR\s*\})\$\{[^{}]*[pP]ath\b[^{}]*\}/g
// Block and line comments; `://` inside a string (a URL) is not a line comment.
const COMMENT = /\/\*[\s\S]*?\*\/|(?<![:'"`\\])\/\/.*$/gm
export function isScannedPath(rel) {
return (
SCAN_ROOTS.some((root) => rel.startsWith(`${root}/`)) &&
/\.(ts|tsx)$/.test(rel) &&
!/\.(test|spec)\.tsx?$/.test(rel) &&
!/(?:^|[/.-])(?:test-fixture|test-support|test-harness|fixtures?)(?:[/.-]|$)|__fixtures__/.test(
rel
) &&
rel !== COMPOSER_FILE
)
}
export function countHandBuiltWorktreeIds(sourceText) {
return sourceText.replace(COMMENT, ' ').match(HAND_BUILT_WORKTREE_ID)?.length ?? 0
}
export function collectHandBuiltWorktreeIdCounts(root = process.cwd()) {
const counts = new Map()
for (const [rel, source] of readTrackedSources(root, SCAN_ROOTS, isScannedPath)) {
const count = countHandBuiltWorktreeIds(source)
if (count > 0) {
counts.set(rel, count)
}
}
return counts
}
export function main(root = process.cwd(), arg = undefined) {
const current = collectHandBuiltWorktreeIdCounts(root)
if (arg === '--prune') {
const total = pruneCountBaseline(root, BASELINE_PATH, HEADER, current)
console.log(`worktree-id-composer baseline pruned to ${total}.`)
return 0
}
const baselineFile = path.join(root, BASELINE_PATH)
const baseline = parseCountBaseline(
fs.existsSync(baselineFile) ? fs.readFileSync(baselineFile, 'utf8') : ''
)
const { grown, shrunk } = diffCounts(current, baseline)
for (const { file, now, allowed } of grown) {
console.error(`${file}: ${now} hand-built worktree id(s), baseline allows ${allowed}.`)
}
if (grown.length > 0) {
console.error(
'Compose worktree ids with composeWorktreeId / getRepoMainWorktreeId (src/shared/worktree/id.ts).'
)
}
for (const { file, now, allowed } of shrunk) {
console.error(`${file}: now ${now}, baseline ${allowed}.`)
}
if (shrunk.length > 0) {
console.error(
'The baseline may only shrink. Fix it: pnpm check:worktree-id-composer-ratchet --prune'
)
}
if (grown.length + shrunk.length > 0) {
return 1
}
console.log(`worktree-id-composer ratchet OK — ${totalCount(current)} grandfathered id(s).`)
return 0
}
if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) {
process.exit(main(process.cwd(), process.argv[2]))
}
@@ -0,0 +1,34 @@
import { describe, expect, it } from 'vitest'
import { countHandBuiltWorktreeIds, isScannedPath } from './check-worktree-id-composer-ratchet.mjs'
describe('countHandBuiltWorktreeIds', () => {
it('counts a repo id joined to a path by a literal or constant separator', () => {
expect(countHandBuiltWorktreeIds('const id = `${repo.id}::${worktree.path}`')).toBe(1)
expect(countHandBuiltWorktreeIds('f(`${repoId}::${gitWorktree.path}`)')).toBe(1)
expect(
countHandBuiltWorktreeIds('`${args.repoId}${WORKTREE_ID_SEPARATOR}${newPath}${suffix}`')
).toBe(1)
expect(countHandBuiltWorktreeIds('[`${repo.id}::${a.path}`, `${repoId}::${b.path}`]')).toBe(2)
})
it('ignores comments and non-worktree keys', () => {
expect(
countHandBuiltWorktreeIds('// ids are `${repoId}::${path}`\n/* `${repo.id}::${p.path}` */')
).toBe(0)
expect(countHandBuiltWorktreeIds('`${owner}::${repoName}`')).toBe(0)
expect(countHandBuiltWorktreeIds('composeWorktreeId(repo.id, worktree.path)')).toBe(0)
})
})
describe('isScannedPath', () => {
it('scans production sources outside the composer module', () => {
expect(isScannedPath('src/main/runtime/a.ts')).toBe(true)
expect(isScannedPath('mobile/src/session/a.tsx')).toBe(true)
expect(isScannedPath('src/shared/worktree/id.ts')).toBe(false)
expect(isScannedPath('src/main/a.test.ts')).toBe(false)
expect(isScannedPath('src/main/runtime/__fixtures__/a.ts')).toBe(false)
expect(isScannedPath('src/renderer/src/store/host-twin.test-fixture.ts')).toBe(false)
expect(isScannedPath('config/scripts/a.mjs')).toBe(false)
})
})
@@ -245,6 +245,7 @@ it('pins every foreground and background step to its selected phase', () => {
['Enforce layout-writer ratchets', staticPhase],
['Enforce deprecated runtime-owner importer ratchet', staticPhase],
['Enforce path-literal ratchet', staticPhase],
['Enforce worktree-id composer ratchet', staticPhase],
['Check Node runtime pin', staticPhase],
['Decide whether the orcad terminal smoke runs', staticPhase],
['Verify the generated RPC params catalog', staticPhase],
+19
View File
@@ -0,0 +1,19 @@
# Hand-built worktree ids (`${repoId}::${path}`) per file, outside src/shared/worktree/id.ts.
# This is a RATCHET: counts may only go DOWN. Compose ids with composeWorktreeId or
# getRepoMainWorktreeId instead. Prune after a fix: pnpm check:worktree-id-composer-ratchet --prune
1 src/main/ipc/workspace-cleanup-scan.ts
1 src/main/ipc/worktree-folder-rename-target.ts
1 src/main/ipc/worktree-metadata-merge.ts
1 src/main/ipc/worktree-push-target-reconciliation.ts
2 src/main/ipc/worktree-remote.ts
1 src/main/ipc/worktrees/listing/detected-provider-listing.ts
1 src/main/ipc/worktrees/listing/ssh-worktree-fallback.ts
2 src/main/ipc/worktrees/removal/nested-worktree-removal.ts
1 src/main/memory/hydrate-local-pty-registry.ts
1 src/main/runtime/repo-worktree-row-resolution.ts
1 src/main/runtime/runtime-local-worktree-materialization.ts
1 src/main/runtime/runtime-managed-worktree-queries.ts
1 src/main/runtime/runtime-provisioned-root-adoption.ts
1 src/main/workspace-space-repo-scan.ts
1 src/main/worktree-lineage-pruning.ts
1 src/renderer/src/components/sidebar/DeleteNestedWorktreesDialog.tsx
+1
View File
@@ -40,6 +40,7 @@
"check:ts-nocheck-ratchet": "node config/scripts/check-ts-nocheck-ratchet.mjs",
"check:deprecated-runtime-owner-ratchet": "node config/scripts/check-deprecated-runtime-owner-ratchet.mjs",
"check:path-literal-ratchet": "node config/scripts/check-path-literal-ratchet.mjs",
"check:worktree-id-composer-ratchet": "node config/scripts/check-worktree-id-composer-ratchet.mjs",
"check:runtime-electron-ratchet": "node config/scripts/check-runtime-electron-ratchet.mjs",
"check:focus-read-ratchet": "node config/scripts/check-focus-read-ratchet.mjs",
"check:layout-writer-ratchets": "node config/scripts/check-layout-writer-ratchets.mjs",
+9 -3
View File
@@ -2,7 +2,11 @@ import type { ExecutionHostId } from '../shared/execution-host'
import { isFolderRepo } from '../shared/repo-kind'
import type { Repo } from '../shared/repo-types'
import type { WorktreeMeta } from '../shared/worktree/meta-types'
import { FOLDER_WORKSPACE_INSTANCE_SEPARATOR, getRepoIdFromWorktreeId } from '../shared/worktree/id'
import {
FOLDER_WORKSPACE_INSTANCE_SEPARATOR,
getRepoIdFromWorktreeId,
getRepoMainWorktreeId
} from '../shared/worktree/id'
type FolderWorkspaceMetaReader = {
getRepos: () => Repo[]
@@ -30,7 +34,9 @@ export function isRemovedFolderWorkspaceInstance(
(repo) =>
repo.id === repoId &&
isFolderRepo(repo) &&
worktreeId.startsWith(`${repo.id}::${repo.path}${FOLDER_WORKSPACE_INSTANCE_SEPARATOR}`)
worktreeId.startsWith(
`${getRepoMainWorktreeId(repo)}${FOLDER_WORKSPACE_INSTANCE_SEPARATOR}`
)
)
if (!isFolderInstance) {
return false
@@ -51,6 +57,6 @@ export function findExtraFolderWorkspaceIds(
repo: Pick<Repo, 'id' | 'path'>,
metaById: Record<string, WorktreeMeta>
): string[] {
const prefix = `${repo.id}::${repo.path}${FOLDER_WORKSPACE_INSTANCE_SEPARATOR}`
const prefix = `${getRepoMainWorktreeId(repo)}${FOLDER_WORKSPACE_INSTANCE_SEPARATOR}`
return Object.keys(metaById).filter((worktreeId) => worktreeId.startsWith(prefix))
}
@@ -3,7 +3,10 @@ import { createFolderWorktree } from '../repo-worktrees'
import type { Repo } from '../../shared/repo-types'
import type { Worktree } from '../../shared/worktree/types'
import type { WorktreeMeta } from '../../shared/worktree/meta-types'
import { FOLDER_WORKSPACE_INSTANCE_SEPARATOR } from '../../shared/worktree/id'
import {
FOLDER_WORKSPACE_INSTANCE_SEPARATOR,
getRepoMainWorktreeId
} from '../../shared/worktree/id'
import { mergeWorktree } from './worktree-logic'
import { getRepoOwnedWorktreeMeta } from '../worktree-metadata-ownership'
@@ -12,7 +15,7 @@ export function listWorkspaceCleanupFolderWorkspaces(
repo: Repo,
repoOwnerCount: number
): Worktree[] {
const rootId = `${repo.id}::${repo.path}`
const rootId = getRepoMainWorktreeId(repo)
const instancePrefix = `${rootId}${FOLDER_WORKSPACE_INSTANCE_SEPARATOR}`
const allMeta: Readonly<Record<string, WorktreeMeta>> = store.getAllWorktreeMeta()
const worktreeIds = [
@@ -1,12 +1,15 @@
import { randomUUID } from 'node:crypto'
import type { Repo } from '../../../shared/repo-types'
import { DEFAULT_WORKSPACE_STATUS_ID } from '../../../shared/workspace-statuses'
import { FOLDER_WORKSPACE_INSTANCE_SEPARATOR } from '../../../shared/worktree/id'
import {
FOLDER_WORKSPACE_INSTANCE_SEPARATOR,
getRepoMainWorktreeId
} from '../../../shared/worktree/id'
import type { WorktreeMeta } from '../../../shared/worktree/meta-types'
import type { Worktree } from '../../../shared/worktree/types'
export function getFolderWorkspaceRootId(repo: Repo): string {
return `${repo.id}::${repo.path}`
return getRepoMainWorktreeId(repo)
}
export function getFolderWorkspaceInstanceId(repo: Repo, instanceId: string): string {
+5 -2
View File
@@ -2,11 +2,14 @@ import { DEFAULT_WORKSPACE_STATUS_ID } from '../../shared/workspace-statuses'
import type { Repo } from '../../shared/repo-types'
import type { WorktreeMeta } from '../../shared/worktree/meta-types'
import type { Worktree } from '../../shared/worktree/types'
import { FOLDER_WORKSPACE_INSTANCE_SEPARATOR } from '../../shared/worktree/id'
import {
FOLDER_WORKSPACE_INSTANCE_SEPARATOR,
getRepoMainWorktreeId
} from '../../shared/worktree/id'
import { normalizeWorkspaceCreatorProvenance } from '../../shared/workspace-creator-provenance'
export function getRuntimeFolderWorkspaceRootId(repo: Repo): string {
return `${repo.id}::${repo.path}`
return getRepoMainWorktreeId(repo)
}
export function getRuntimeFolderWorkspaceInstanceId(repo: Repo, instanceId: string): string {
+6 -2
View File
@@ -1,6 +1,10 @@
import { basename } from 'node:path'
import type { Repo } from '../shared/repo-types'
import { splitWorktreeId, splitWorktreeIdForFilesystem } from '../shared/worktree/id'
import {
getRepoMainWorktreeId,
splitWorktreeId,
splitWorktreeIdForFilesystem
} from '../shared/worktree/id'
import { isFolderRepo } from '../shared/repo-kind'
import type { Store } from './persistence'
@@ -35,7 +39,7 @@ export function loadKnownUsageWorktreesByRepo(
for (const repo of localRepos) {
worktreesByRepo.set(repo.id, [
{
worktreeId: `${repo.id}::${repo.path}`,
worktreeId: getRepoMainWorktreeId(repo),
path: repo.path,
displayName: repo.displayName || getDefaultUsageWorktreeLabel(repo.path)
}
@@ -4,7 +4,7 @@ import { toast } from 'sonner'
import { useMountedRef } from '@/hooks/useMountedRef'
import type { Repo } from '../../../../shared/repo-types'
import type { Worktree } from '../../../../shared/worktree/types'
import { getRepoIdFromWorktreeId } from '../../../../shared/worktree/id'
import { composeWorktreeId, getRepoIdFromWorktreeId } from '../../../../shared/worktree/id'
import {
getRepoExecutionHostId,
getRepoSshConnectionId,
@@ -94,7 +94,7 @@ export function McpConfigSection({ repo }: McpConfigSectionProps): React.JSX.Ele
return (
worktreesForRepo.find((worktree) => worktree.isMainWorktree) ??
worktreesForRepo.find((worktree) => worktree.path === repo.path) ??
worktreesForRepo[0] ?? { id: `${repo.id}::${repo.path}`, path: repo.path }
worktreesForRepo[0] ?? { id: composeWorktreeId(repo.id, repo.path), path: repo.path }
)
}, [activeWorktreeId, allWorktreesForRepo, repo.id, repo.path, worktreesForRepo])
const targetWorktreeId = targetWorktree.id
+14
View File
@@ -1,6 +1,7 @@
import { describe, expect, it } from 'vitest'
import {
WORKTREE_ID_SEPARATOR,
composeWorktreeId,
getRepoIdFromWorktreeId,
getRepoMainWorktreeId,
getWorktreePathBasenameFromId,
@@ -23,6 +24,19 @@ describe('getRepoMainWorktreeId', () => {
})
})
describe('composeWorktreeId', () => {
it('keeps the path spelling exactly as given', () => {
for (const path of [
'/srv/Repo/',
'C:\\Users\\me\\repo',
'//wsl$/Ubuntu/home/a',
'/private/tmp/x'
]) {
expect(composeWorktreeId('repo-1', path)).toBe(`repo-1${WORKTREE_ID_SEPARATOR}${path}`)
}
})
})
describe('getRepoIdFromWorktreeId', () => {
it('returns the repo id for a canonical worktree id', () => {
expect(getRepoIdFromWorktreeId('repo-123::/abs/path')).toBe('repo-123')
+6 -1
View File
@@ -19,7 +19,12 @@ const FOLDER_WORKSPACE_INSTANCE_SUFFIX = new RegExp(
* runtimes reject it with `worktree_id_requires_full_path` (#16447).
*/
export function getRepoMainWorktreeId(repo: Pick<Repo, 'id' | 'path'>): string {
return `${repo.id}${WORKTREE_ID_SEPARATOR}${repo.path}`
return composeWorktreeId(repo.id, repo.path)
}
/** Worktree id for a checkout, spelled exactly as given: ids are never reassigned by a fold. */
export function composeWorktreeId(repoId: string, worktreePath: string): string {
return `${repoId}${WORKTREE_ID_SEPARATOR}${worktreePath}`
}
export function getRepoIdFromWorktreeId(worktreeId: string): string {