From baf123eda8406fc55755951448fe772f9df80fbc Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 19 Oct 2026 10:14:07 -0700 Subject: [PATCH] fix(persistence): repo-row store writes take the row, not the id; ratchet the rest (R4-2a) (#30756) * fix(persistence): repo-row store writes take the row, not the id; ratchet the rest (R4-2a) * fix(persistence): host-filter the runtime nested import; widen the id-only write ratchet (review) * fix(runtime): nested import keeps matching runtime-stamped native rows on a paired host * fix(persistence): class-scope the ratchet param exemptions; update a new main test to the target shape * fix(ipc): fence a project-group move by the resolved twin host, so an SSH twin is not WSL-fenced * test(ci): pin the id-only store write ratchet step to the static phase --- .github/workflows/pr.yml | 4 + config/id-only-store-write-baseline.txt | 17 +++ .../check-id-only-store-write-ratchet.mjs | 143 ++++++++++++++++++ ...check-id-only-store-write-ratchet.test.mjs | 84 ++++++++++ config/scripts/pr-preflight-gates.test.mjs | 1 + package.json | 3 +- .../ipc/repos-execution-host-catalog.test.ts | 89 ++++++++++- .../repos-local-add-and-project-setup.test.ts | 8 +- .../ipc/repos-remote-client-events.test.ts | 5 +- .../ipc/repos/nested-repo-import-handler.ts | 2 +- src/main/ipc/repos/project-group-handlers.ts | 18 ++- .../ipc/repos/project-host-setup-handlers.ts | 2 +- src/main/ipc/repos/repo-catalog-handlers.ts | 2 +- src/main/ipc/repos/repo-ipc-arg-schemas.ts | 14 +- .../repo-remove-for-host-teardown.test.ts | 5 +- ...tence-duplicate-repo-id-host-scope.test.ts | 39 ++++- ...ersistence-orcad-migration-catalog.test.ts | 2 +- ...d-migration-terminal-restore-hints.test.ts | 2 +- ...persistence-remote-session-startup.test.ts | 4 +- src/main/persistence-repo-lifecycle.test.ts | 32 ++-- ...tence-worktree-lineage-and-backups.test.ts | 2 +- ...rsistence-worktree-name-retirement.test.ts | 24 +-- .../project-collection-operations.ts | 9 +- .../repo-lifecycle-operations.ts | 4 +- .../folder-workspace-operations.ts | 13 +- .../tracking-repos/project-host-operations.ts | 9 +- ...oject-setup-folder-remote-identity.spec.ts | 2 +- .../repository-project-operations.spec.ts | 4 +- ...roject-removal-structured-sessions.test.ts | 8 +- .../project-removal-terminal-stop.test.ts | 2 +- ...ntime-nested-repo-import-host-twin.test.ts | 76 ++++++++++ .../runtime/runtime-nested-repo-import.ts | 11 +- .../runtime-project-group-controller.ts | 2 +- .../runtime-project-host-setup-controller.ts | 4 +- .../runtime-remove-project-host-scope.spec.ts | 36 ++++- .../runtime-repository-settings-controller.ts | 4 +- .../ssh/orcad-migration-delta-move.test.ts | 8 +- .../orcad-migration-repository-link.test.ts | 14 +- .../worktree-removal-close-records.test.ts | 2 +- src/preload/api/repository-api.ts | 1 + .../project-header-drag-commit.test.ts | 4 +- .../sidebar/project-header-drag-commit.ts | 16 +- .../sidebar/project-header-drag-contract.ts | 8 +- .../worktree-list/drag/use-header-drag.ts | 4 +- .../project-groups/project-group-mutations.ts | 8 +- ...epos-project-group-cross-host-move.test.ts | 19 +++ .../store/slices/repos-project-groups.test.ts | 3 +- 47 files changed, 669 insertions(+), 104 deletions(-) create mode 100644 config/id-only-store-write-baseline.txt create mode 100644 config/scripts/check-id-only-store-write-ratchet.mjs create mode 100644 config/scripts/check-id-only-store-write-ratchet.test.mjs create mode 100644 src/main/runtime/runtime-nested-repo-import-host-twin.test.ts diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 65f32e88080..b9ce1c282f8 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -417,6 +417,10 @@ jobs: if: needs.code_paths.outputs.static_analysis == 'true' run: node config/scripts/check-unproven-pty-retirement-ratchet.mjs + - name: Enforce id-only store write ratchet + if: needs.code_paths.outputs.static_analysis == 'true' + run: node config/scripts/check-id-only-store-write-ratchet.mjs + - name: Check Node runtime pin if: needs.code_paths.outputs.static_analysis == 'true' run: node config/scripts/check-node-runtime-pin.mjs diff --git a/config/id-only-store-write-baseline.txt b/config/id-only-store-write-baseline.txt new file mode 100644 index 00000000000..edb23958c19 --- /dev/null +++ b/config/id-only-store-write-baseline.txt @@ -0,0 +1,17 @@ +# Store writes whose first parameter is a bare repo or worktree id (R4-2), as Class.method. +# This is a RATCHET: entries may only be REMOVED. Take RepoHostTarget or HostWorktreeTarget. +# Prune after a fix: pnpm check:id-only-store-write-ratchet --prune +AutomationPersistence.snapshotAutomationRunWorkspaceDisplayName +MetadataLineageOperations.migrateWorktreeIdentity +MetadataLineageOperations.removeWorkspaceLineage +MetadataLineageOperations.removeWorktreeLineage +MetadataLineageOperations.removeWorktreeMeta +MetadataLineageOperations.setWorktreeLineage +MetadataLineageOperations.setWorktreeMeta +MetadataLineageOperations.setWorktreeMetaForHost +OrcadMigrationBlobInstallPersistence.installDeferredOrcadMigrationShellHistory +ProjectCollectionOperations.deleteProjectGroup +ProjectCollectionOperations.removeFolderWorkspace +ProjectCollectionOperations.updateFolderWorkspace +ProjectCollectionOperations.updateProjectGroup +SessionHostPartitionOperations.removeWorkspaceSessionStateForWorktree diff --git a/config/scripts/check-id-only-store-write-ratchet.mjs b/config/scripts/check-id-only-store-write-ratchet.mjs new file mode 100644 index 00000000000..23aa711cf59 --- /dev/null +++ b/config/scripts/check-id-only-store-write-ratchet.mjs @@ -0,0 +1,143 @@ +import fs from 'node:fs' +import path from 'node:path' +import process from 'node:process' +import { pathToFileURL } from 'node:url' +import { runEntryRatchet } from './ratchet-baseline.mjs' + +// Ratchet on Store writes addressed by a bare repo or worktree id (R4-2). A local row and an SSH +// row can share an id, so an id-only write lands on whichever row matches first. Writes take the +// row's identity instead: `RepoHostTarget` (src/shared/repo-host-identity.ts) or +// `HostWorktreeTarget` (src/shared/worktree/host-qualified-identity.ts). Entries may only go away. +// Matched by the first parameter's NAME, whatever its type (`string`, `Repo['id']`, a brand), so a +// retyped id still counts. Known limit: an id inside an object parameter (`{ projectId }`) is not seen. + +const BASELINE_PATH = 'config/id-only-store-write-baseline.txt' +const COMPOSITION = 'src/main/persistence/loading-store/store-domain-composition.ts' +// The only type names that make an id-named parameter host-carrying. +const HOST_TARGET_TYPE = /^(?:RepoHostTarget|HostWorktreeTarget)\b/ +const ID_PARAM = /^(?:id|ids|\w*Ids?|(?:\w*(?:Worktree|Workspace)|worktree|workspace)Keys?)$/ +const READ_METHOD = /^(?:get|list|has|peek|read|is|select|find|automation)[A-Z]/ +// A public method or arrow property with its first parameter's name and type. `private`, +// `protected` and `#` members don't match: they aren't on the Store's surface. +const MEMBER = + /^ {2}(?:(?:public|override|static|async|readonly)\s+)*(\w+)\s*(?:<[^>\n]*>)?(?:\s*=\s*(?:async\s*)?(?:<[^>\n]*>)?)?\(\s*(\w+)\??\s*:\s*([^,)=]+)/gm + +/** `Class.param` first parameters whose id has no host twin, with the reason. */ +export const NOT_HOST_TWINNED_PARAMS = new Map([ + ['AutomationPersistence.automationId', 'automation ids are minted UUIDs'], + ['OrcadSourceExportPersistence.targetId', 'the id names the host itself'], + ['RemoteLayoutSnapshotPersistence.environmentId', 'the id names the host itself'], + ['SessionHostPartitionOperations.hostId', 'the id names the host itself'], + ['SshLeaseRecoveryOperations.targetId', 'the id names the host itself'], + ['SshProfileOperations.oldTargetId', 'the id names the host itself'] +]) + +/** Matches exempt by method, with the reason. Keep each reason true. */ +export const NOT_HOST_TWINNED = new Map([ + ['AutomationPersistence.advanceAutomationNextRun', 'automation ids are minted UUIDs'], + ['AutomationPersistence.deleteAutomation', 'automation ids are minted UUIDs'], + ['AutomationPersistence.updateAutomation', 'automation ids are minted UUIDs'], + ['ProfilePreferences.recordFeatureInteraction', 'a feature id, not a row'], + ['ProjectCollectionOperations.updateProject', 'one Project row spans every host'], + [ + 'RepoLifecycleOperations.reorderRepos', + 'a whole-catalog permutation: a duplicated id fails its check, so it refuses instead of guessing' + ], + ['RepoLifecycleOperations.reorderReposForHost', 'its second parameter scopes it to one host'], + [ + 'RetiredWorktreeNamePersistence.addRetiredWorktreeName', + 'twins share one id-keyed bucket by design; re-keying it is a non-goal (D7)' + ], + [ + 'RetiredWorktreeNamePersistence.mergeRetiredWorktreeNames', + 'twins share one id-keyed bucket by design; re-keying it is a non-goal (D7)' + ], + [ + 'SparsePresetPersistence.removeSparsePreset', + 'twins share one id-keyed bucket by design; re-keying it is a non-goal (D7)' + ], + ['SshProfileOperations.removeSshTarget', 'the id names the host itself'], + ['SshProfileOperations.updateSshTarget', 'the id names the host itself'] +]) + +/** `[className, file]` for every operations class the Store mixes in, in composition order. */ +export function listStoreOperationClasses(root) { + const text = fs.readFileSync(path.join(root, COMPOSITION), 'utf8') + const imports = new Map( + [...text.matchAll(/^import \{ (\w+) \} from '([^']+)'/gm)].map((m) => [m[1], m[2]]) + ) + const list = /STORE_DOMAIN_OPERATION_CLASSES = \[([\s\S]*?)\]/.exec(text) + if (!list) { + throw new Error(`${COMPOSITION}: STORE_DOMAIN_OPERATION_CLASSES not found`) + } + return list[1] + .split(',') + .map((name) => name.trim()) + .filter(Boolean) + .map((name) => { + const specifier = imports.get(name) + if (!specifier) { + throw new Error(`${COMPOSITION}: no import for ${name}`) + } + return [name, path.posix.join(path.posix.dirname(COMPOSITION), `${specifier}.ts`)] + }) +} + +/** Public mutating methods of `className` in `source` whose first parameter is a bare id string. */ +export function findIdOnlyWrites(className, source) { + const start = source.indexOf(`export class ${className}`) + if (start === -1) { + return [] + } + const rest = source.slice(start) + const end = rest.search(/^\}/m) + const body = end < 0 ? rest : rest.slice(0, end) + const found = [] + for (const [, method, param, type] of body.matchAll(MEMBER)) { + if ( + method !== 'constructor' && + !READ_METHOD.test(method) && + ID_PARAM.test(param) && + !NOT_HOST_TWINNED_PARAMS.has(`${className}.${param}`) && + !HOST_TARGET_TYPE.test(type.trim()) + ) { + found.push(`${className}.${method}`) + } + } + return found +} + +export function collectIdOnlyStoreWrites(root = process.cwd()) { + const entries = [] + for (const [className, rel] of listStoreOperationClasses(root)) { + const source = fs.readFileSync(path.join(root, rel), 'utf8') + entries.push(...findIdOnlyWrites(className, source).filter((e) => !NOT_HOST_TWINNED.has(e))) + } + return [...new Set(entries)].sort() +} + +export function main(root = process.cwd(), arg = undefined) { + return runEntryRatchet( + { + name: 'id-only-store-write', + baselinePath: BASELINE_PATH, + header: [ + '# Store writes whose first parameter is a bare repo or worktree id (R4-2), as Class.method.', + '# This is a RATCHET: entries may only be REMOVED. Take RepoHostTarget or HostWorktreeTarget.', + '# Prune after a fix: pnpm check:id-only-store-write-ratchet --prune' + ], + collect: collectIdOnlyStoreWrites, + fix: [ + 'Take the row, not its id: RepoHostTarget (src/shared/repo-host-identity.ts) for repo rows,', + 'HostWorktreeTarget (src/shared/worktree/host-qualified-identity.ts) for worktree rows. A caller', + 'with only an id resolves the row where its host is known (RPC selector, IPC hostId).' + ] + }, + root, + arg + ) +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + process.exit(main(process.cwd(), process.argv[2])) +} diff --git a/config/scripts/check-id-only-store-write-ratchet.test.mjs b/config/scripts/check-id-only-store-write-ratchet.test.mjs new file mode 100644 index 00000000000..680962591b1 --- /dev/null +++ b/config/scripts/check-id-only-store-write-ratchet.test.mjs @@ -0,0 +1,84 @@ +import { describe, expect, it } from 'vitest' + +import { + collectIdOnlyStoreWrites, + findIdOnlyWrites, + listStoreOperationClasses, + NOT_HOST_TWINNED +} from './check-id-only-store-write-ratchet.mjs' + +const SOURCE = `export class RowOps { + constructor(runtime: string) {} + + moveProjectToGroup(repoId: string, groupId: string | null): void {} + + updateFolderWorkspace( + id: string, + updates: object + ): void {} + + removeWorktreeMeta(worktreeId?: string): void {} + + getWorktreeMeta(worktreeId: string): void {} + + updateRepo(target: RepoHostTarget): void {} + + reorderRepos(orderedIds: string[]): void {} + + setRemoteLayoutSnapshot(environmentId: string): void {} + + private writeRow(repoId: string): void {} + + public renameRepo(repoId: Repo['id']): void {} + + override setLineage(worktreeKey: WorktreeKey, value: T): void {} + + removeWorkspaceLineage(childWorkspaceKey: WorkspaceKey): void {} + + dropRow = (id: BrandedRepoId): void => {} + + moveRow(target: RepoHostTarget): void {} + + dropWorktree(worktreeId: HostWorktreeTarget): void {} + + removeSessionHost(hostId: ExecutionHostId): void {} +} + +export class Other { + setWorktreeLineage(worktreeId: string): void {} +} +` + +describe('findIdOnlyWrites', () => { + it('lists public writes whose first parameter is an id by name, whatever its type', () => { + expect(findIdOnlyWrites('RowOps', SOURCE)).toEqual([ + 'RowOps.moveProjectToGroup', + 'RowOps.updateFolderWorkspace', + 'RowOps.removeWorktreeMeta', + 'RowOps.reorderRepos', + 'RowOps.setRemoteLayoutSnapshot', + 'RowOps.renameRepo', + 'RowOps.setLineage', + 'RowOps.removeWorkspaceLineage', + 'RowOps.dropRow', + // Param exemptions are per class, so these host-named params count on RowOps. + 'RowOps.removeSessionHost' + ]) + }) + + it('stays inside the named class', () => { + expect(findIdOnlyWrites('Other', SOURCE)).toEqual(['Other.setWorktreeLineage']) + expect(findIdOnlyWrites('Missing', SOURCE)).toEqual([]) + }) +}) + +describe('collectIdOnlyStoreWrites', () => { + it('scans every Store operations class and skips the documented exemptions', () => { + expect(listStoreOperationClasses(process.cwd()).length).toBeGreaterThan(10) + const entries = collectIdOnlyStoreWrites(process.cwd()) + expect(entries).not.toContain('RepoLifecycleOperations.updateRepo') + for (const exempt of NOT_HOST_TWINNED.keys()) { + expect(entries).not.toContain(exempt) + } + }) +}) diff --git a/config/scripts/pr-preflight-gates.test.mjs b/config/scripts/pr-preflight-gates.test.mjs index 30196f8c0f2..34f8847d782 100644 --- a/config/scripts/pr-preflight-gates.test.mjs +++ b/config/scripts/pr-preflight-gates.test.mjs @@ -248,6 +248,7 @@ it('pins every foreground and background step to its selected phase', () => { ['Enforce e2e typecheck ratchet', staticPhase], ['Enforce legacy relay fence ratchet', staticPhase], ['Enforce unproven PTY retirement ratchet', staticPhase], + ['Enforce id-only store write ratchet', staticPhase], ['Check Node runtime pin', staticPhase], ['Decide whether the orcad terminal smoke runs', staticPhase], ['Verify the generated RPC params catalog', staticPhase], diff --git a/package.json b/package.json index 9790bf028f5..fcf0e68d7d5 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,7 @@ "audit:perf": "oxlint --config config/oxlint-performance-audit.json --format json src", "test:perf:contracts": "vitest run --config config/vitest.performance.config.ts", "format": "oxfmt --write .", - "lint": "oxlint && pnpm run audit:anti-slop && pnpm run audit:code-quality:native && pnpm run audit:code-quality:type-aware && pnpm run check:reliability-gates && pnpm run check:dead-classes && pnpm run check:max-lines-ratchet && pnpm run check:ts-nocheck-ratchet && pnpm run check:runtime-electron-ratchet && pnpm run check:focus-read-ratchet && pnpm run check:layout-writer-ratchets && pnpm run check:path-literal-ratchet && pnpm run check:legacy-relay-fence-ratchet && pnpm run check:unproven-pty-retirement-ratchet && pnpm run check:readme-local-links && pnpm run check:node-runtime-pin && pnpm run verify:rpc-params-catalog && pnpm run verify:acp-protocol && pnpm run verify:bundled-skill-guides && pnpm run verify:skill-bundle-manifest && pnpm run verify:localization-catalogs && pnpm run verify:localization-extraction && pnpm run verify:localization-coverage && pnpm run check:process-host-imports && pnpm run check:main-sync-spawn-ratchet && pnpm run check:compat-boundary-imports", + "lint": "oxlint && pnpm run audit:anti-slop && pnpm run audit:code-quality:native && pnpm run audit:code-quality:type-aware && pnpm run check:reliability-gates && pnpm run check:dead-classes && pnpm run check:max-lines-ratchet && pnpm run check:ts-nocheck-ratchet && pnpm run check:runtime-electron-ratchet && pnpm run check:focus-read-ratchet && pnpm run check:layout-writer-ratchets && pnpm run check:path-literal-ratchet && pnpm run check:legacy-relay-fence-ratchet && pnpm run check:unproven-pty-retirement-ratchet && pnpm run check:id-only-store-write-ratchet && pnpm run check:readme-local-links && pnpm run check:node-runtime-pin && pnpm run verify:rpc-params-catalog && pnpm run verify:acp-protocol && pnpm run verify:bundled-skill-guides && pnpm run verify:skill-bundle-manifest && pnpm run verify:localization-catalogs && pnpm run verify:localization-extraction && pnpm run verify:localization-coverage && pnpm run check:process-host-imports && pnpm run check:main-sync-spawn-ratchet && pnpm run check:compat-boundary-imports", "audit:code-quality": "pnpm run audit:code-quality:native && pnpm run audit:code-quality:type-aware && pnpm run audit:react-doctor", "audit:code-quality:native": "oxlint --config config/oxlint-code-quality-native-plugins.json src config tests mobile --deny-warnings", "audit:code-quality:type-aware": "oxlint --type-aware --config config/oxlint-code-quality-type-aware.json src config tests --deny-warnings", @@ -41,6 +41,7 @@ "check:path-literal-ratchet": "node config/scripts/check-path-literal-ratchet.mjs", "check:legacy-relay-fence-ratchet": "node config/scripts/check-legacy-relay-fence-ratchet.mjs", "check:unproven-pty-retirement-ratchet": "node config/scripts/check-unproven-pty-retirement-ratchet.mjs", + "check:id-only-store-write-ratchet": "node config/scripts/check-id-only-store-write-ratchet.mjs", "check:worktree-id-composer-ratchet": "node config/scripts/check-worktree-id-composer-ratchet.mjs", "check:e2e-typecheck-ratchet": "node config/scripts/check-e2e-typecheck-ratchet.mjs", "check:runtime-electron-ratchet": "node config/scripts/check-runtime-electron-ratchet.mjs", diff --git a/src/main/ipc/repos-execution-host-catalog.test.ts b/src/main/ipc/repos-execution-host-catalog.test.ts index 4ffced7d679..82b4e221db5 100644 --- a/src/main/ipc/repos-execution-host-catalog.test.ts +++ b/src/main/ipc/repos-execution-host-catalog.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type * as GitRunner from '../git/runner' import type * as RepoModule from '../git/repo' @@ -27,6 +27,16 @@ import { clearGitCapabilityStateForTests } from '../git/git-capability-state' import { resetSshProviderAuthorities } from '../ssh/ssh-provider-authority' import { getGitRepoRoot, isGitRepo } from '../git/repo' import { createRepoHandlerHarness, resetProjectGroupMocks } from './repos-remote-test-harness' +import { mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { wslMoveCutoverFixture as cutover } from '../persistence/__fixtures__/wsl-mover/wsl-move-cutover-fixture' +import { seedWslMoveCutover } from '../persistence/__fixtures__/wsl-mover/wsl-move-journal-seeding' +import { + WslMoveProjectFencedError, + loadWslMoveFence, + resetWslMoveFenceForTests +} from '../persistence/moving-wsl-projects/wsl-move-scope-guard' const { handleMock, mockStore } = reposMocks @@ -64,4 +74,81 @@ describe('projectGroups IPC validation', () => { expect(mockStore.updateProjectGroup).not.toHaveBeenCalled() }) + + it('moves the twin on the host the renderer named, not the first row with the id', () => { + const localRow = { id: 'dup', path: '/laptop/dup', executionHostId: 'local' } + mockStore.getRepo.mockReturnValue(localRow) + mockStore.moveProjectToGroup.mockReturnValue({ id: 'dup', connectionId: 'ssh-1' }) + + handlers.get('projectGroups:moveProject')!(null, { + projectId: 'dup', + groupId: 'group-1', + hostId: 'ssh:ssh-1' + }) + + expect(mockStore.moveProjectToGroup).toHaveBeenCalledWith( + { id: 'dup', executionHostId: 'ssh:ssh-1' }, + 'group-1', + undefined + ) + }) + + it('keeps moving the first row with the id for a caller that names no host', () => { + const localRow = { id: 'dup', path: '/laptop/dup', executionHostId: 'local' } + mockStore.getRepo.mockReturnValue(localRow) + + handlers.get('projectGroups:moveProject')!(null, { projectId: 'dup', groupId: null }) + + expect(mockStore.moveProjectToGroup).toHaveBeenCalledWith(localRow, null, undefined) + }) + + it('rejects an invalid host id before persistence', () => { + expect(() => + handlers.get('projectGroups:moveProject')!(null, { + projectId: 'dup', + groupId: null, + hostId: 'not a host' + }) + ).toThrow('invalid_project_group_move_repo_args') + + expect(mockStore.moveProjectToGroup).not.toHaveBeenCalled() + }) + + describe('while a WSL move fences the local repo-1', () => { + let userDataPath: string + beforeEach(() => { + userDataPath = mkdtempSync(join(tmpdir(), 'wsl-move-project-group-ipc-')) + loadWslMoveFence(userDataPath) + seedWslMoveCutover(userDataPath, cutover('move-1', 'source-fenced')) + }) + afterEach(() => { + resetWslMoveFenceForTests() + rmSync(userDataPath, { recursive: true, force: true }) + }) + + it('still moves the SSH twin, which the move does not take', () => { + handlers.get('projectGroups:moveProject')!(null, { + projectId: 'repo-1', + groupId: 'group-1', + hostId: 'ssh:ssh-1' + }) + + expect(mockStore.moveProjectToGroup).toHaveBeenCalledWith( + { id: 'repo-1', executionHostId: 'ssh:ssh-1' }, + 'group-1', + undefined + ) + }) + + it('refuses the local twin', () => { + expect(() => + handlers.get('projectGroups:moveProject')!(null, { + projectId: 'repo-1', + groupId: 'group-1', + hostId: 'local' + }) + ).toThrow(WslMoveProjectFencedError) + expect(mockStore.moveProjectToGroup).not.toHaveBeenCalled() + }) + }) }) diff --git a/src/main/ipc/repos-local-add-and-project-setup.test.ts b/src/main/ipc/repos-local-add-and-project-setup.test.ts index 7a367e8c2e2..5406e4bd33e 100644 --- a/src/main/ipc/repos-local-add-and-project-setup.test.ts +++ b/src/main/ipc/repos-local-add-and-project-setup.test.ts @@ -338,7 +338,9 @@ describe('repos:add + repos:clone', () => { }) ).rejects.toThrow('Imported folder does not match the selected project identity.') - expect(mockStore.removeProjectForHost).toHaveBeenCalledWith(added[0]?.id, 'local') + expect(mockStore.removeProjectForHost).toHaveBeenCalledWith( + expect.objectContaining({ id: added[0]?.id }) + ) expect(invalidateAuthorizedRootsCacheMock).toHaveBeenCalled() }) @@ -370,7 +372,9 @@ describe('repos:add + repos:clone', () => { } expect(mockStore.updateRepo).not.toHaveBeenCalled() - expect(mockStore.removeProjectForHost).toHaveBeenCalledWith(added[0]?.id, 'local') + expect(mockStore.removeProjectForHost).toHaveBeenCalledWith( + expect.objectContaining({ id: added[0]?.id }) + ) }) it('re-reads an already-registered folder whose stored remote is still pending (#30663)', async () => { diff --git a/src/main/ipc/repos-remote-client-events.test.ts b/src/main/ipc/repos-remote-client-events.test.ts index 0427c612c2e..174527bb5c8 100644 --- a/src/main/ipc/repos-remote-client-events.test.ts +++ b/src/main/ipc/repos-remote-client-events.test.ts @@ -77,7 +77,10 @@ describe('repo IPC mutations notify paired clients', () => { await handlers.get('repos:removeForHost')!(null, { repoId: 'repo-1', hostId: 'ssh:host-1' }) - expect(mockStore.removeProjectForHost).toHaveBeenCalledWith('repo-1', 'ssh:host-1') + expect(mockStore.removeProjectForHost).toHaveBeenCalledWith({ + id: 'repo-1', + executionHostId: 'ssh:host-1' + }) expect(notify).toHaveBeenCalledTimes(1) expect(mainWindow.webContents.send).toHaveBeenCalledWith('repos:changed') }) diff --git a/src/main/ipc/repos/nested-repo-import-handler.ts b/src/main/ipc/repos/nested-repo-import-handler.ts index 69fa84a4723..6b74d80090d 100644 --- a/src/main/ipc/repos/nested-repo-import-handler.ts +++ b/src/main/ipc/repos/nested-repo-import-handler.ts @@ -102,7 +102,7 @@ export function registerNestedRepoImportHandler(mainWindow: BrowserWindow, store const group = groupResolver.getGroupForRepo(repoPath) if (existing) { if (group) { - store.moveProjectToGroup(existing.id, group.id, projectGroupOrder) + store.moveProjectToGroup(existing, group.id, projectGroupOrder) } importedProjectIdsByRepoPath.set(normalizedImportRepoPath, existing.id) results.push({ path: repoPath, projectId: existing.id, status: 'already-known' }) diff --git a/src/main/ipc/repos/project-group-handlers.ts b/src/main/ipc/repos/project-group-handlers.ts index 89839bfef11..8a0c18b8a96 100644 --- a/src/main/ipc/repos/project-group-handlers.ts +++ b/src/main/ipc/repos/project-group-handlers.ts @@ -2,6 +2,7 @@ import type { BrowserWindow } from 'electron' import { ipcMain } from 'electron' import type { Store } from '../../persistence' import type { Repo } from '../../../shared/repo-types' +import type { RepoHostTarget } from '../../../shared/repo-host-identity' import type { NestedRepoScanResult, ProjectGroup } from '../../../shared/project-group-types' import { notifyReposChanged } from './repos-changed-notification' import { @@ -13,7 +14,10 @@ import { ProjectGroupUpdateArgs, parseProjectGroupIpcArgs } from './repo-ipc-arg-schemas' -import { assertWslMoveProjectWritable } from '../../persistence/moving-wsl-projects/wsl-move-scope-guard' +import { + assertWslMoveProjectWritable, + assertWslMoveRepoWritable +} from '../../persistence/moving-wsl-projects/wsl-move-scope-guard' import { activeNestedRepoScans, runNestedRepoScanForIpc } from './nested-repo-scan-ipc' import { visibleProjectGroups } from '../../ssh/orcad-retained-source' @@ -69,8 +73,16 @@ export function registerProjectGroupHandlers(mainWindow: BrowserWindow, store: S rawArgs, 'invalid_project_group_move_repo_args' ) - assertWslMoveProjectWritable(store, { repoId: args.projectId }) - const moved = store.moveProjectToGroup(args.projectId, args.groupId, args.order) + const target: RepoHostTarget | undefined = args.hostId + ? { id: args.projectId, executionHostId: args.hostId } + : store.getRepo(args.projectId) + if (!target) { + assertWslMoveProjectWritable(store, { repoId: args.projectId }) + return null + } + // Why the target's host: a WSL move fences the local twin only, never its SSH sibling. + assertWslMoveRepoWritable(store, target) + const moved = store.moveProjectToGroup(target, args.groupId, args.order) if (moved) { notifyReposChanged(mainWindow) } diff --git a/src/main/ipc/repos/project-host-setup-handlers.ts b/src/main/ipc/repos/project-host-setup-handlers.ts index 7c42ce75d3a..a99931cedc3 100644 --- a/src/main/ipc/repos/project-host-setup-handlers.ts +++ b/src/main/ipc/repos/project-host-setup-handlers.ts @@ -201,7 +201,7 @@ export function registerProjectHostSetupHandlers(mainWindow: BrowserWindow, stor } catch (err) { // Why: an import that cannot be linked must not leave a new repo registration or authorization root behind. if (!result.alreadyExisted) { - store.removeProjectForHost(result.repo.id, getRepoExecutionHostId(result.repo)) + store.removeProjectForHost(result.repo) invalidateAuthorizedRootsCache() } throw err diff --git a/src/main/ipc/repos/repo-catalog-handlers.ts b/src/main/ipc/repos/repo-catalog-handlers.ts index 0db9c5b89fd..0c59cf3b6b8 100644 --- a/src/main/ipc/repos/repo-catalog-handlers.ts +++ b/src/main/ipc/repos/repo-catalog-handlers.ts @@ -114,7 +114,7 @@ export function registerRepoCatalogHandlers( scheduleCurrentWorktreeBaseDirectoryWatcherSync() return } - store.removeProjectForHost(args.repoId, hostId) + store.removeProjectForHost({ id: args.repoId, executionHostId: hostId }) invalidateAuthorizedRootsCache() notifyReposChanged(mainWindow) } diff --git a/src/main/ipc/repos/repo-ipc-arg-schemas.ts b/src/main/ipc/repos/repo-ipc-arg-schemas.ts index 9c0ff14dca2..11f132e26b6 100644 --- a/src/main/ipc/repos/repo-ipc-arg-schemas.ts +++ b/src/main/ipc/repos/repo-ipc-arg-schemas.ts @@ -29,12 +29,6 @@ export const ProjectGroupSelectorArgs = z.object({ groupId: z.string().min(1) }) -export const ProjectGroupMoveProjectArgs = z.object({ - projectId: z.string().min(1), - groupId: z.string().nullable(), - order: z.number().finite().optional() -}) - export const ProjectHostSetupExistingFolderIpcArgs = z.object({ projectId: z.string().min(1), projectProviderIdentity: z @@ -77,6 +71,14 @@ const IpcExecutionHostId = z return hostId }) +// Why optional: the row's host; without it the first row with the id moves, as before. +export const ProjectGroupMoveProjectArgs = z.object({ + projectId: z.string().min(1), + groupId: z.string().nullable(), + order: z.number().finite().optional(), + hostId: IpcExecutionHostId.optional() +}) + export const ProjectHostSetupCreateIpcArgs = z.object({ projectId: z.string().min(1), hostId: IpcExecutionHostId, diff --git a/src/main/ipc/repos/repo-remove-for-host-teardown.test.ts b/src/main/ipc/repos/repo-remove-for-host-teardown.test.ts index 55f05f054b2..0fd0e3652ee 100644 --- a/src/main/ipc/repos/repo-remove-for-host-teardown.test.ts +++ b/src/main/ipc/repos/repo-remove-for-host-teardown.test.ts @@ -53,6 +53,9 @@ describe('repos:removeForHost', () => { const { store, runtime, remove } = register([paired]) await remove(null, { repoId: 'repo-1', hostId: 'runtime:env-1' }) expect(runtime.removeProject).not.toHaveBeenCalled() - expect(store.removeProjectForHost).toHaveBeenCalledWith('repo-1', 'runtime:env-1') + expect(store.removeProjectForHost).toHaveBeenCalledWith({ + id: 'repo-1', + executionHostId: 'runtime:env-1' + }) }) }) diff --git a/src/main/persistence-duplicate-repo-id-host-scope.test.ts b/src/main/persistence-duplicate-repo-id-host-scope.test.ts index ab1e5c6af01..fc7cc463bbd 100644 --- a/src/main/persistence-duplicate-repo-id-host-scope.test.ts +++ b/src/main/persistence-duplicate-repo-id-host-scope.test.ts @@ -132,7 +132,7 @@ describe('deleting one host copy of a repo id shared by two hosts', () => { }) store.setWorktreeMetaForHost('dup::/remote/dup', 'ssh:ssh-1', { displayName: 'SSH wt' }) - store.removeProjectForHost('dup', 'local') + store.removeProjectForHost({ id: 'dup', executionHostId: 'local' }) expect(store.getRepos().map((repo) => repo.path)).toEqual(['/remote/dup']) expect(Object.values(store.getAllWorktreeMeta()).map((meta) => meta.displayName)).toContain( 'SSH wt' @@ -143,11 +143,46 @@ describe('deleting one host copy of a repo id shared by two hosts', () => { it('removeProjectForHost drops only the addressed host row', async () => { const store = await createStoreWithDuplicateRepoId() - store.removeProjectForHost('dup', 'ssh:ssh-1') + store.removeProjectForHost({ id: 'dup', executionHostId: 'ssh:ssh-1' }) expect(store.getRepos().map((repo) => repo.path)).toEqual(['/laptop/dup']) }) + it('removeProjectForHost given the SSH row itself leaves the local twin', async () => { + const store = await createStoreWithDuplicateRepoId() + const sshRow = store.getRepos().find((repo) => repo.connectionId === 'ssh-1')! + + store.removeProjectForHost(sshRow) + + expect(store.getRepos().map((repo) => repo.path)).toEqual(['/laptop/dup']) + }) + + it('moveProjectToGroup moves the addressed twin, not the first row with the id', async () => { + const store = await createStoreWithDuplicateRepoId() + const group = store.createProjectGroup({ + name: 'Remote', + connectionId: 'ssh-1', + createdFrom: 'manual' + }) + const sshRow = store.getRepos().find((repo) => repo.connectionId === 'ssh-1')! + + const moved = store.moveProjectToGroup(sshRow, group.id, 3) + + expect(moved?.path).toBe('/remote/dup') + const groupByPath = Object.fromEntries( + store.getRepos().map((repo) => [repo.path, repo.projectGroupId ?? null]) + ) + expect(groupByPath).toEqual({ '/laptop/dup': null, '/remote/dup': group.id }) + }) + + it('moveProjectToGroup on a host holding no twin moves nothing', async () => { + const store = await createStoreWithDuplicateRepoId() + const group = store.createProjectGroup({ name: 'G', createdFrom: 'manual' }) + + expect(store.moveProjectToGroup({ id: 'dup', connectionId: 'ssh-2' }, group.id)).toBeNull() + expect(store.getRepos().every((repo) => !repo.projectGroupId)).toBe(true) + }) + it('deleteProjectHostSetup drops only the row it reports, never the sibling host', async () => { // Repo-derived setups reuse the repo id, so a duplicated id yields two setups with the // same setup id and the lookup can only resolve one. Whichever it resolves, the other diff --git a/src/main/persistence-orcad-migration-catalog.test.ts b/src/main/persistence-orcad-migration-catalog.test.ts index b6e99250c34..6d8621798e7 100644 --- a/src/main/persistence-orcad-migration-catalog.test.ts +++ b/src/main/persistence-orcad-migration-catalog.test.ts @@ -487,7 +487,7 @@ describe('orcad migration catalog persistence', () => { const store = createStore() const input = manifest() commitCatalog(store, input) - store.removeProjectForHost(REPOSITORY.id, 'local') + store.removeProjectForHost({ id: REPOSITORY.id, executionHostId: 'local' }) expect(store.getOrcadMigrationCatalogState(input).state).toBe('committed') expect(store.getRepos()).toEqual([]) diff --git a/src/main/persistence-orcad-migration-terminal-restore-hints.test.ts b/src/main/persistence-orcad-migration-terminal-restore-hints.test.ts index 4b285ee3632..d3ed332d40e 100644 --- a/src/main/persistence-orcad-migration-terminal-restore-hints.test.ts +++ b/src/main/persistence-orcad-migration-terminal-restore-hints.test.ts @@ -234,7 +234,7 @@ describe('orcad migration terminal restore hints through the store', () => { stageBlob(store, input, encodedHints()) expect(await commitDurably(store, input)).toBe('committed') - store.removeProjectForHost(REPOSITORY.id, 'local') + store.removeProjectForHost({ id: REPOSITORY.id, executionHostId: 'local' }) expect(peek(store, PANE, DORMANT_WORKTREE_ID)).toBeNull() expect(peek(store, OTHER_PANE, FOLDER_KEY)).toBe('/srv/app') diff --git a/src/main/persistence-remote-session-startup.test.ts b/src/main/persistence-remote-session-startup.test.ts index 91328732cd3..89f082d665c 100644 --- a/src/main/persistence-remote-session-startup.test.ts +++ b/src/main/persistence-remote-session-startup.test.ts @@ -103,7 +103,7 @@ describe('remote session startup ownership', () => { const reloaded = createStore() // Assert the row survived load first, or an empty partition below would prove nothing. expect(reloaded.getWorkspaceSession(HOST).browserPagesByWorkspace).not.toEqual({}) - reloaded.removeProjectForHost(REPO, HOST) + reloaded.removeProjectForHost({ id: REPO, executionHostId: HOST }) reloaded.flush() expect(createStore().getWorkspaceSession(HOST).browserPagesByWorkspace).toEqual({}) }) @@ -118,7 +118,7 @@ describe('remote session startup ownership', () => { seed.flush() const reloaded = createStore() - reloaded.removeProjectForHost(REPO, HOST) + reloaded.removeProjectForHost({ id: REPO, executionHostId: HOST }) reloaded.flush() const after = createStore() diff --git a/src/main/persistence-repo-lifecycle.test.ts b/src/main/persistence-repo-lifecycle.test.ts index e9e58d05343..43574be2e0b 100644 --- a/src/main/persistence-repo-lifecycle.test.ts +++ b/src/main/persistence-repo-lifecycle.test.ts @@ -108,7 +108,7 @@ describe('Store', () => { expect(isLocalWorktreeScanGenerationCurrent(repoId, beforeAdd)).toBe(false) const beforeRemove = getLocalWorktreeScanGeneration(repoId) - store.removeProjectForHost(repoId, 'local') + store.removeProjectForHost({ id: repoId, executionHostId: 'local' }) expect(isLocalWorktreeScanGenerationCurrent(repoId, beforeRemove)).toBe(false) const beforeReAdd = getLocalWorktreeScanGeneration(repoId) @@ -122,7 +122,7 @@ describe('Store', () => { for (let index = 0; index < 200; index += 1) { const repoId = `scan-churn-${index}` store.addRepo(makeRepo({ id: repoId })) - store.removeProjectForHost(repoId, 'local') + store.removeProjectForHost({ id: repoId, executionHostId: 'local' }) } expect(_getLocalWorktreeScanGenerationCacheSize()).toBe(initialCacheSize) @@ -293,7 +293,7 @@ describe('Store', () => { store.setWorktreeMeta('r1::/path/wt2', { displayName: 'wt2' }) store.setWorktreeMeta('r2::/other', { displayName: 'other' }) - store.removeProjectForHost('r1', 'local') + store.removeProjectForHost({ id: 'r1', executionHostId: 'local' }) expect(store.getRepo('r1')).toBeUndefined() expect(store.getWorktreeMeta('r1::/path/wt1')).toBeUndefined() @@ -312,7 +312,7 @@ describe('Store', () => { ...store.getWorkspaceSession(), terminalTopologyRevisionByRepoId: { [repoId]: 1 } }) - store.removeProjectForHost(repoId, 'local') + store.removeProjectForHost({ id: repoId, executionHostId: 'local' }) } expect(store.getWorkspaceSession().terminalTopologyRevisionByRepoId).toEqual({}) @@ -331,7 +331,7 @@ describe('Store', () => { lastVisitedAtByWorktreeId: { 'r1::/path/wt1': 111, 'r2::/other': 222 } }) - store.removeProjectForHost('r1', 'local') + store.removeProjectForHost({ id: 'r1', executionHostId: 'local' }) const session = store.getWorkspaceSession() expect(session.lastVisitedAtByWorktreeId?.['r1::/path/wt1']).toBeUndefined() @@ -353,7 +353,7 @@ describe('Store', () => { hostId ) - store.removeProjectForHost('r1', 'local') + store.removeProjectForHost({ id: 'r1', executionHostId: 'local' }) const hostSession = store.getWorkspaceSession(hostId) expect(hostSession.lastVisitedAtByWorktreeId?.['r1::/path/wt1']).toBeUndefined() @@ -379,7 +379,7 @@ describe('Store', () => { store.addRepo(makeRepo({ id: 'r1' })) store.addRepo(makeRepo({ id: 'r2', path: '/repo2' })) - store.removeProjectForHost('r1', 'local') + store.removeProjectForHost({ id: 'r1', executionHostId: 'local' }) expect(store.getProjects().map((project) => project.id)).toEqual(['repo:r2']) expect(store.getProjectHostSetups().map((setup) => setup.id)).toEqual(['r2']) @@ -412,7 +412,7 @@ describe('Store', () => { }) ) - store.removeProjectForHost('r1', 'local') + store.removeProjectForHost({ id: 'r1', executionHostId: 'local' }) expect(store.getWorktreeLineage('r1::/path/child')).toBeUndefined() expect(store.getWorktreeLineage('r2::/other-child')).toBeUndefined() @@ -434,7 +434,7 @@ describe('Store', () => { ) const beforeRemove = getLocalWorktreeScanGeneration('shared') - store.removeProjectForHost('shared', 'ssh:ssh-old') + store.removeProjectForHost({ id: 'shared', executionHostId: 'ssh:ssh-old' }) expect(isLocalWorktreeScanGenerationCurrent('shared', beforeRemove)).toBe(false) }) @@ -457,7 +457,7 @@ describe('Store', () => { hostId: 'ssh:ssh-old' }) - store.removeProjectForHost('shared', 'ssh:ssh-old') + store.removeProjectForHost({ id: 'shared', executionHostId: 'ssh:ssh-old' }) const remaining = store.getRepos().filter((r) => r.id === 'shared') expect(remaining).toHaveLength(1) @@ -501,7 +501,7 @@ describe('Store', () => { 'ssh:ssh-a' ) - store.removeProjectForHost('shared', 'ssh:ssh-a') + store.removeProjectForHost({ id: 'shared', executionHostId: 'ssh:ssh-a' }) // The removed SSH host's session is pruned; the surviving local session stays. expect( @@ -535,7 +535,7 @@ describe('Store', () => { 'ssh:ssh-a' ) - store.removeProjectForHost('shared', 'local') + store.removeProjectForHost({ id: 'shared', executionHostId: 'local' }) expect(store.getWorkspaceSession().lastVisitedAtByWorktreeId?.['shared::/repo']).toBeUndefined() expect( @@ -562,7 +562,7 @@ describe('Store', () => { lastVisitedAtByWorktreeId: { '|shared::/repo/wt-orphan': 111 } }) - store.removeProjectForHost('shared', 'local') + store.removeProjectForHost({ id: 'shared', executionHostId: 'local' }) expect( store.getWorkspaceSession().lastVisitedAtByWorktreeId?.['|shared::/repo/wt-orphan'] @@ -613,7 +613,7 @@ describe('Store', () => { 'ssh:ssh-b' ) - store.removeProjectForHost('shared', 'ssh:ssh-a') + store.removeProjectForHost({ id: 'shared', executionHostId: 'ssh:ssh-a' }) // Only the removed host's partition is pruned; local and the other SSH host survive. expect( @@ -700,7 +700,7 @@ describe('Store', () => { // A hostId-less meta is treated as local and left behind (never delete the wrong host's meta). store.setWorktreeMeta('shared::/local/repo/wt', { displayName: 'local-wt' }) - store.removeProjectForHost('shared', 'ssh:ssh-old') + store.removeProjectForHost({ id: 'shared', executionHostId: 'ssh:ssh-old' }) expect(store.getWorktreeMeta('shared::/remote/repo/wt')).toBeUndefined() expect(store.getWorktreeMeta('shared::/local/repo/wt')).toBeDefined() @@ -711,7 +711,7 @@ describe('Store', () => { store.addRepo(makeRepo({ id: 'only', connectionId: 'ssh-x', executionHostId: 'ssh:ssh-x' })) store.setWorktreeMeta('only::/repo/wt', { displayName: 'wt', hostId: 'ssh:ssh-x' }) - store.removeProjectForHost('only', 'ssh:ssh-x') + store.removeProjectForHost({ id: 'only', executionHostId: 'ssh:ssh-x' }) expect(store.getRepo('only')).toBeUndefined() expect(store.getWorktreeMeta('only::/repo/wt')).toBeUndefined() diff --git a/src/main/persistence-worktree-lineage-and-backups.test.ts b/src/main/persistence-worktree-lineage-and-backups.test.ts index 270c700d449..f90c6779378 100644 --- a/src/main/persistence-worktree-lineage-and-backups.test.ts +++ b/src/main/persistence-worktree-lineage-and-backups.test.ts @@ -213,7 +213,7 @@ describe('Store', () => { } }) - store.removeProjectForHost('r1', 'local') + store.removeProjectForHost({ id: 'r1', executionHostId: 'local' }) store.flush() expect(store.getMobileClientTabSelections()['device-a']).toEqual({ diff --git a/src/main/persistence-worktree-name-retirement.test.ts b/src/main/persistence-worktree-name-retirement.test.ts index 1602b540f5b..eb112e918b0 100644 --- a/src/main/persistence-worktree-name-retirement.test.ts +++ b/src/main/persistence-worktree-name-retirement.test.ts @@ -177,7 +177,7 @@ describe('worktree name retirement registry', () => { store.addRepo({ id: REPO, path: '/repos/a', displayName: 'a', badgeColor: '', addedAt: 0 }) store.addRetiredWorktreeName(REPO, 'nautilus') - store.removeProjectForHost(REPO, 'local') + store.removeProjectForHost({ id: REPO, executionHostId: 'local' }) expect(store.getRetiredWorktreeNameRegistry(REPO).names).toEqual([]) }) @@ -196,7 +196,7 @@ describe('worktree name retirement registry', () => { const { getRetiredNameRegistryForRepo, retireGeneratedWorktreeName } = await import('./worktree-name-retirement') await retireGeneratedWorktreeName(store, oldRepo, store.getSettings(), 'nautilus') - store.removeProjectForHost(REPO, getRepoExecutionHostId(oldRepo)) + store.removeProjectForHost({ id: REPO, executionHostId: getRepoExecutionHostId(oldRepo) }) store.flush() store = await reloadStore() @@ -225,7 +225,7 @@ describe('worktree name retirement registry', () => { await retireGeneratedWorktreeName(store, oldRepo, store.getSettings(), 'nautilus') // The deleted workspace has no Claude bucket; Codex rollout files are not backfilled. - store.removeProjectForHost(REPO, 'local') + store.removeProjectForHost({ id: REPO, executionHostId: 'local' }) const newRepo = { ...oldRepo, id: OTHER_REPO } store.addRepo(newRepo) @@ -244,7 +244,7 @@ describe('worktree name retirement registry', () => { await retireGeneratedWorktreeName(store, oldRepo, store.getSettings(), 'nautilus') // Removing and re-adding the host mints a fresh row id for the same machine and account. - store.removeProjectForHost(REPO, getRepoExecutionHostId(oldRepo)) + store.removeProjectForHost({ id: REPO, executionHostId: getRepoExecutionHostId(oldRepo) }) store.removeSshTarget('ssh-old') store.addSshTarget(sshTarget('ssh-new')) const newRepo = { ...oldRepo, id: OTHER_REPO, connectionId: 'ssh-new' } @@ -265,7 +265,7 @@ describe('worktree name retirement registry', () => { await import('./worktree-name-retirement') await retireGeneratedWorktreeName(store, oldRepo, store.getSettings(), 'nautilus') - store.removeProjectForHost(REPO, getRepoExecutionHostId(oldRepo)) + store.removeProjectForHost({ id: REPO, executionHostId: getRepoExecutionHostId(oldRepo) }) const otherRepo = { ...oldRepo, id: OTHER_REPO, connectionId: 'ssh-other' } store.addRepo(otherRepo) @@ -308,7 +308,7 @@ describe('worktree name retirement registry', () => { await retireGeneratedWorktreeName(store, repo, store.getSettings(), 'nautilus') // Drop the repo row, so the namespace copy is the only thing left holding the tombstone. - store.removeProjectForHost(REPO, getRepoExecutionHostId(repo)) + store.removeProjectForHost({ id: REPO, executionHostId: getRepoExecutionHostId(repo) }) // The ssh-config alias is unchanged, so re-adoption matches even though the host moved. store.removeSshTarget('ssh-old') store.addRemovedSshTargetTombstone({ @@ -345,7 +345,7 @@ describe('worktree name retirement registry', () => { store.updateSshTarget('ssh-1', { host: 'new.example.com' }) // Drop the repo row the way a project remove/re-add does, leaving the mirror as the only source. - store.removeProjectForHost(REPO, getRepoExecutionHostId(repo)) + store.removeProjectForHost({ id: REPO, executionHostId: getRepoExecutionHostId(repo) }) const readded = { ...REMOTE_REPO, id: OTHER_REPO, connectionId: 'ssh-1' } store.addRepo(readded) await expect( @@ -393,7 +393,7 @@ describe('worktree name retirement registry', () => { store.updateSshTarget(runtimeId, { host: 'vm-new.example.com' }) - store.removeProjectForHost(REPO, getRepoExecutionHostId(repo)) + store.removeProjectForHost({ id: REPO, executionHostId: getRepoExecutionHostId(repo) }) const readded = { ...REMOTE_REPO, id: OTHER_REPO, connectionId: runtimeId } store.addRepo(readded) await expect( @@ -430,7 +430,7 @@ describe('worktree name retirement registry', () => { // `ssh-y` never moved. Drop its repo row the way a project remove/re-add does, so the namespace // copy is the only thing left holding the tombstone. - store.removeProjectForHost(REPO, getRepoExecutionHostId(sibling)) + store.removeProjectForHost({ id: REPO, executionHostId: getRepoExecutionHostId(sibling) }) const readded = { ...REMOTE_REPO, id: OTHER_REPO, connectionId: 'ssh-y' } store.addRepo(readded) await expect( @@ -448,7 +448,7 @@ describe('worktree name retirement registry', () => { await import('./worktree-name-retirement') await retireGeneratedWorktreeName(store, oldRepo, store.getSettings(), 'nautilus') - store.removeProjectForHost(REPO, getRepoExecutionHostId(oldRepo)) + store.removeProjectForHost({ id: REPO, executionHostId: getRepoExecutionHostId(oldRepo) }) const newRepo = { ...oldRepo, id: OTHER_REPO, connectionId: 'ssh-new' } store.addRepo(newRepo) @@ -489,11 +489,11 @@ describe('worktree name retirement registry', () => { } as never) store.addRetiredWorktreeName(REPO, 'nautilus') - store.removeProjectForHost(REPO, 'runtime:env-1') + store.removeProjectForHost({ id: REPO, executionHostId: 'runtime:env-1' }) expect(store.getRetiredWorktreeNameRegistry(REPO).names).toEqual(['nautilus']) - store.removeProjectForHost(REPO, 'local') + store.removeProjectForHost({ id: REPO, executionHostId: 'local' }) expect(store.getRetiredWorktreeNameRegistry(REPO).names).toEqual([]) }) diff --git a/src/main/persistence/loading-store/project-collection-operations.ts b/src/main/persistence/loading-store/project-collection-operations.ts index 1ae4ec2d15e..e52a8c8d110 100644 --- a/src/main/persistence/loading-store/project-collection-operations.ts +++ b/src/main/persistence/loading-store/project-collection-operations.ts @@ -12,6 +12,7 @@ import type { ProjectUpdateArgs } from '../../../shared/project-types' import type { Repo } from '../../../shared/repo-types' +import type { RepoHostTarget } from '../../../shared/repo-host-identity' import { ProjectGroupPersistenceOperations } from '../tracking-repos/project-group-operations' import { FolderWorkspacePersistenceOperations } from '../restoring-sessions/folder-workspace-operations' import { ProjectHostPersistenceOperations } from '../tracking-repos/project-host-operations' @@ -164,8 +165,8 @@ export class ProjectCollectionOperations { return removed } - moveProjectToGroup(repoId: string, groupId: string | null, order?: number): Repo | null { - return getFolderWorkspaceOperations(this).moveProjectToGroup(repoId, groupId, order) + moveProjectToGroup(target: RepoHostTarget, groupId: string | null, order?: number): Repo | null { + return getFolderWorkspaceOperations(this).moveProjectToGroup(target, groupId, order) } } @@ -191,8 +192,8 @@ export function getProjectHostOperations( setup, updates ), - removeProjectForHost: (id, hostId) => - owner[projectCollectionOperationsContext].repos.removeProjectForHost(id, hostId), + removeProjectForHost: (target) => + owner[projectCollectionOperationsContext].repos.removeProjectForHost(target), scheduleSave: () => scheduleSave(owner[projectCollectionOperationsContext].scheduling) }) return owner[projectCollectionOperationsContext].runtime.projectHostOperations diff --git a/src/main/persistence/loading-store/repo-lifecycle-operations.ts b/src/main/persistence/loading-store/repo-lifecycle-operations.ts index 29a697cda59..084dec0b5cc 100644 --- a/src/main/persistence/loading-store/repo-lifecycle-operations.ts +++ b/src/main/persistence/loading-store/repo-lifecycle-operations.ts @@ -78,7 +78,9 @@ export class RepoLifecycleOperations { return getRepoOrderOperations(this).reorderReposForHost(orderedIds, hostId) } - removeProjectForHost(id: string, hostId: ExecutionHostId): void { + removeProjectForHost(target: RepoHostTarget): void { + const id = target.id + const hostId = getRepoExecutionHostId(target) const repoRemoved = this[repoLifecycleOperationsContext].runtime.state.repos.some( (repo) => repo.id === id && getRepoExecutionHostId(repo) === hostId ) diff --git a/src/main/persistence/restoring-sessions/folder-workspace-operations.ts b/src/main/persistence/restoring-sessions/folder-workspace-operations.ts index 76eac91c48e..839117973a3 100644 --- a/src/main/persistence/restoring-sessions/folder-workspace-operations.ts +++ b/src/main/persistence/restoring-sessions/folder-workspace-operations.ts @@ -2,6 +2,8 @@ import { randomUUID } from 'node:crypto' import type { FolderWorkspace } from '../../../shared/folder-workspace-types' import type { PersistedState } from '../../../shared/persisted-state-types' import type { Repo } from '../../../shared/repo-types' +import type { RepoHostTarget } from '../../../shared/repo-host-identity' +import { getRepoExecutionHostId } from '../../../shared/execution-host' import { normalizeFolderWorkspaceName } from '../../../shared/folder-workspaces' import { getNextProjectGroupOrder } from '../../../shared/project-groups' import { normalizeStoredTaskSourceContext } from '../../../shared/task-source-context' @@ -9,6 +11,7 @@ import { normalizeWorkspaceLinkedItem } from '../../../shared/workspace-linked-i import { isWorkspaceLinkedItemSourceContextMatch } from '../../../shared/workspace-linked-item-source-context' import { folderWorkspaceKey } from '../../../shared/workspace-scope' import { removeWorkspaceSessionOwnerEverywhere } from './session-owner-removal' +import { findRepoRowForHostScopedWrite } from '../tracking-repos/repo-update-operations' import type { WorkspaceAttachmentMutation } from '../../../shared/workspace-attachment-mutation' import { normalizeWorkspaceAttachmentUpdate } from '../../../shared/workspace-attachments' import { ORCA_SIDEBAR_INPUTS_IS_MAIN } from '../../../shared/sidebar-inputs-switch' @@ -273,8 +276,12 @@ export class FolderWorkspacePersistenceOperations { return true } - moveProjectToGroup(repoId: string, groupId: string | null, order?: number): Repo | null { - const repo = this.state.repos.find((entry) => entry.id === repoId) + moveProjectToGroup(target: RepoHostTarget, groupId: string | null, order?: number): Repo | null { + const repo = findRepoRowForHostScopedWrite( + this.state.repos, + target.id, + getRepoExecutionHostId(target) + ) if (!repo) { return null } @@ -282,7 +289,7 @@ export class FolderWorkspacePersistenceOperations { groupId && (this.state.projectGroups ?? []).some((group) => group.id === groupId) ? groupId : null - const siblingRepos = this.state.repos.filter((entry) => entry.id !== repoId) + const siblingRepos = this.state.repos.filter((entry) => entry !== repo) repo.projectGroupId = normalizedGroupId repo.projectGroupOrder = typeof order === 'number' && Number.isFinite(order) diff --git a/src/main/persistence/tracking-repos/project-host-operations.ts b/src/main/persistence/tracking-repos/project-host-operations.ts index 106e956bcbe..c93bace6da6 100644 --- a/src/main/persistence/tracking-repos/project-host-operations.ts +++ b/src/main/persistence/tracking-repos/project-host-operations.ts @@ -11,6 +11,7 @@ import type { } from '../../../shared/project-types' import type { PersistedState } from '../../../shared/persisted-state-types' import type { Repo } from '../../../shared/repo-types' +import type { RepoHostTarget } from '../../../shared/repo-host-identity' import { getRepoExecutionHostId, normalizeExecutionHostId } from '../../../shared/execution-host' import { normalizeProjectRuntimePreference } from '../../../shared/project-execution-runtime' import { findProjectHostSetup } from '../../../shared/project-host-setup-target' @@ -31,7 +32,7 @@ export type ProjectHostMutationOperations = { setup: ProjectHostSetup, updates: ProjectHostSetupUpdateArgs['updates'] ) => ProjectHostSetup - removeProjectForHost: (id: string, hostId: ProjectHostSetup['hostId']) => void + removeProjectForHost: (target: RepoHostTarget) => void scheduleSave: () => void } @@ -65,8 +66,8 @@ export class ProjectHostPersistenceOperations { return this.operations.updateIndependentProjectHostSetup(setup, updates) } - private removeProjectForHost(id: string, hostId: ProjectHostSetup['hostId']): void { - this.operations.removeProjectForHost(id, hostId) + private removeProjectForHost(target: RepoHostTarget): void { + this.operations.removeProjectForHost(target) } private scheduleSave(): void { @@ -195,7 +196,7 @@ export class ProjectHostPersistenceOperations { // With no exact row the setup is stale, and the path below drops just the setup. const repo = this.findSetupRepo(setup) if (repo) { - this.removeProjectForHost(repo.id, setup.hostId) + this.removeProjectForHost(repo) return { project, setup, repo: this.hydrateRepo(repo) } } this.state.projectHostSetups = this.state.projectHostSetups.filter((entry) => entry !== setup) diff --git a/src/main/runtime/orca-runtime-tests/project-setup-folder-remote-identity.spec.ts b/src/main/runtime/orca-runtime-tests/project-setup-folder-remote-identity.spec.ts index a506aa12154..dcaeafeb530 100644 --- a/src/main/runtime/orca-runtime-tests/project-setup-folder-remote-identity.spec.ts +++ b/src/main/runtime/orca-runtime-tests/project-setup-folder-remote-identity.spec.ts @@ -30,7 +30,7 @@ function createRuntime(repos: Repo[]): OrcaRuntimeService { Object.assign(repo, updates) return { ...repo } }, - removeProjectForHost: (id: string) => { + removeProjectForHost: ({ id }: { id: string }) => { const index = repos.findIndex((repo) => repo.id === id) if (index !== -1) { repos.splice(index, 1) diff --git a/src/main/runtime/orca-runtime-tests/repository-project-operations.spec.ts b/src/main/runtime/orca-runtime-tests/repository-project-operations.spec.ts index c0e313c2e93..7480f7b89d7 100644 --- a/src/main/runtime/orca-runtime-tests/repository-project-operations.spec.ts +++ b/src/main/runtime/orca-runtime-tests/repository-project-operations.spec.ts @@ -267,7 +267,7 @@ describe('OrcaRuntimeService', () => { Object.assign(repo, updates) return { ...repo } as never }, - removeProjectForHost: (id: string) => { + removeProjectForHost: ({ id }: { id: string }) => { const index = repos.findIndex((repo) => repo.id === id) if (index !== -1) { repos.splice(index, 1) @@ -309,7 +309,7 @@ describe('OrcaRuntimeService', () => { const runtimeStore = { ...store, getRepos: () => [...repos] as never, - removeProjectForHost: (id: string) => { + removeProjectForHost: ({ id }: { id: string }) => { const index = repos.findIndex((repo) => repo.id === id) if (index !== -1) { repos.splice(index, 1) diff --git a/src/main/runtime/project-removal-structured-sessions.test.ts b/src/main/runtime/project-removal-structured-sessions.test.ts index be2f6219e58..a9c44e31c47 100644 --- a/src/main/runtime/project-removal-structured-sessions.test.ts +++ b/src/main/runtime/project-removal-structured-sessions.test.ts @@ -4,6 +4,7 @@ */ import { beforeEach, describe, expect, it, vi } from 'vitest' import type { Repo } from '../../shared/repo-types' +import type { RepoHostTarget } from '../../shared/repo-host-identity' import { getRepoExecutionHostId } from '../../shared/execution-host' const hostRef: { current: unknown } = { current: null } @@ -74,8 +75,11 @@ function createRuntime(rows: Repo[]) { ...store, getRepo: (id: string) => current.find((row) => row.id === id), getRepos: () => current, - removeProjectForHost: (id: string, hostId: string) => { - current = current.filter((row) => row.id !== id || getRepoExecutionHostId(row) !== hostId) + removeProjectForHost: (target: RepoHostTarget) => { + const hostId = getRepoExecutionHostId(target) + current = current.filter( + (row) => row.id !== target.id || getRepoExecutionHostId(row) !== hostId + ) } }) runtime.setPtyController({ diff --git a/src/main/runtime/project-removal-terminal-stop.test.ts b/src/main/runtime/project-removal-terminal-stop.test.ts index f637761fbc2..f384707b4f4 100644 --- a/src/main/runtime/project-removal-terminal-stop.test.ts +++ b/src/main/runtime/project-removal-terminal-stop.test.ts @@ -57,7 +57,7 @@ describe('project removal on the execution host', () => { await expect(runtime.removeProject(TEST_REPO_ID)).resolves.toEqual({ removed: true }) expect(stoppedBeforeRemoval.sort()).toEqual(['pty-hidden', 'pty-visible']) - expect(removeProjectForHost).toHaveBeenCalledWith(TEST_REPO_ID, 'local') + expect(removeProjectForHost).toHaveBeenCalledWith(expect.objectContaining({ id: TEST_REPO_ID })) }) it('stops native shells of a project a paired client registered under a runtime stamp', async () => { diff --git a/src/main/runtime/runtime-nested-repo-import-host-twin.test.ts b/src/main/runtime/runtime-nested-repo-import-host-twin.test.ts new file mode 100644 index 00000000000..819fd5390b4 --- /dev/null +++ b/src/main/runtime/runtime-nested-repo-import-host-twin.test.ts @@ -0,0 +1,76 @@ +import { describe, expect, it, vi } from 'vitest' +import type { Repo } from '../../shared/repo-types' +import type { RuntimeStore } from './runtime-store-contract' + +const PARENT = '/work/platform' +const CHILD = '/work/platform/api' + +vi.mock('../git/runner', () => ({ awaitWindowsHostGitEnvironmentReady: vi.fn(async () => {}) })) +vi.mock('../git/repo', () => ({ + isGitRepoAsync: vi.fn(async () => true), + getRepoName: (path: string) => path.split('/').pop() +})) +vi.mock('../project-groups/nested-repo-discovery', () => ({ + scanNestedRepos: vi.fn(async () => ({ repos: [{ path: CHILD, displayName: 'api', depth: 1 }] })) +})) +vi.mock('../project-groups/nested-repo-import-target', () => ({ + createNestedRepoImportTargetResolver: () => ({ resolveLocal: async (path: string) => path }) +})) + +const { RuntimeNestedRepoImport } = await import('./runtime-nested-repo-import') + +function row(id: string, host: Pick): Repo { + return { id, path: CHILD, displayName: 'api', badgeColor: '#000', addedAt: 1, ...host } +} + +function createImporter(repos: Repo[]) { + const moveProjectToGroup = vi.fn() + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the stub covers every store call the import makes. + const store = { + getRepos: () => repos, + addRepo: (repo: Repo) => repos.push(repo), + createProjectGroup: (input: { name: string }) => ({ id: 'group-1', ...input }), + moveProjectToGroup + } as unknown as RuntimeStore + const importer = new RuntimeNestedRepoImport({ + getStore: () => store, + invalidateResolvedWorktrees: vi.fn(), + invalidateWorktreeScan: vi.fn(), + notifyReposChanged: vi.fn() + }) + const run = () => + importer.import({ + parentPath: PARENT, + groupName: 'Platform', + projectPaths: [CHILD], + mode: 'group' + }) + return { run, moveProjectToGroup } +} + +describe('runtime nested repo import with an SSH row at the same path', () => { + it('imports the local repo and leaves the SSH row out of the new local group', async () => { + const repos = [row('ssh-api', { connectionId: 'ssh-1' })] + const { run, moveProjectToGroup } = createImporter(repos) + + const result = await run() + + expect(moveProjectToGroup).not.toHaveBeenCalled() + expect(result.projects).toEqual([expect.objectContaining({ path: CHILD, status: 'imported' })]) + expect(repos.map((repo) => repo.connectionId ?? 'local')).toEqual(['ssh-1', 'local']) + }) + + it('groups a paired host runtime-stamped native row instead of importing it twice', async () => { + const native = row('native-api', { executionHostId: 'runtime:env-1' }) + const repos = [native] + const { run, moveProjectToGroup } = createImporter(repos) + + const result = await run() + + expect(moveProjectToGroup).toHaveBeenCalledWith(native, 'group-1', 0) + expect(result.projects).toEqual([ + expect.objectContaining({ projectId: 'native-api', status: 'already-known' }) + ]) + expect(repos).toHaveLength(1) + }) +}) diff --git a/src/main/runtime/runtime-nested-repo-import.ts b/src/main/runtime/runtime-nested-repo-import.ts index 81ab6d99f00..51e97d970fd 100644 --- a/src/main/runtime/runtime-nested-repo-import.ts +++ b/src/main/runtime/runtime-nested-repo-import.ts @@ -8,6 +8,7 @@ import type { ProjectGroupImportResult } from '../../shared/project-group-types' import type { Repo } from '../../shared/repo-types' +import { getRepoExecutionHostId, getSshTargetIdForExecutionHost } from '../../shared/execution-host' import { awaitWindowsHostGitEnvironmentReady } from '../git/runner' import { getRepoName, isGitRepoAsync } from '../git/repo' import { scanNestedRepos } from '../project-groups/nested-repo-discovery' @@ -92,13 +93,19 @@ export class RuntimeNestedRepoImport { }) continue } + // Why not SSH: an SSH row can sit at the same path, and this import adds native rows. A + // paired host stamps its native rows `runtime:`, so those still match. const existing = store .getRepos() - .find((repo) => normalizeRuntimePathForComparison(repo.path) === normalizedImportRepoPath) + .find( + (repo) => + getSshTargetIdForExecutionHost(getRepoExecutionHostId(repo)) === null && + normalizeRuntimePathForComparison(repo.path) === normalizedImportRepoPath + ) const group = groupResolver.getGroupForRepo(repoPath) if (existing) { if (group) { - store.moveProjectToGroup(existing.id, group.id, projectGroupOrder) + store.moveProjectToGroup(existing, group.id, projectGroupOrder) } importedProjectIdsByRepoPath.set(normalizedImportRepoPath, existing.id) results.push({ path: repoPath, projectId: existing.id, status: 'already-known' }) diff --git a/src/main/runtime/runtime-project-group-controller.ts b/src/main/runtime/runtime-project-group-controller.ts index 491318deb43..9480bec0e8f 100644 --- a/src/main/runtime/runtime-project-group-controller.ts +++ b/src/main/runtime/runtime-project-group-controller.ts @@ -135,7 +135,7 @@ export class RuntimeProjectGroupController { } const repo = await this.deps.resolveRepo(repoSelector) assertWslMoveRepoWritable(store, repo) - const moved = store.moveProjectToGroup(repo.id, groupId, order) + const moved = store.moveProjectToGroup(repo, groupId, order) if (!moved) { throw new Error('repo_not_found') } diff --git a/src/main/runtime/runtime-project-host-setup-controller.ts b/src/main/runtime/runtime-project-host-setup-controller.ts index ecb78c138da..f644b7849d2 100644 --- a/src/main/runtime/runtime-project-host-setup-controller.ts +++ b/src/main/runtime/runtime-project-host-setup-controller.ts @@ -264,9 +264,7 @@ export class RuntimeProjectHostSetupController { } catch (error) { if (repoWasCreated) { // Why: host-scoped so a rollback never takes a same-id row on another host. - this.deps - .getStore() - ?.removeProjectForHost?.(initialRepo.id, getRepoExecutionHostId(initialRepo)) + this.deps.getStore()?.removeProjectForHost?.(initialRepo) this.deps.invalidateResolvedWorktrees() this.deps.invalidateWorktreeScan(initialRepo.id) invalidateAuthorizedRootsCache() diff --git a/src/main/runtime/runtime-remove-project-host-scope.spec.ts b/src/main/runtime/runtime-remove-project-host-scope.spec.ts index 51891522070..1f853da6510 100644 --- a/src/main/runtime/runtime-remove-project-host-scope.spec.ts +++ b/src/main/runtime/runtime-remove-project-host-scope.spec.ts @@ -36,7 +36,9 @@ export function registerRuntimeRemoveProjectHostScopeTests(): void { function createRuntime() { const repos = makeRepos() - const removeProjectForHost = vi.fn((id: string, hostId: string) => { + const removeProjectForHost = vi.fn((target: RepoHostTarget) => { + const id = target.id + const hostId = getRepoExecutionHostId(target) for (let index = repos.length - 1; index >= 0; index -= 1) { const repo = repos[index] const repoHostId = @@ -60,6 +62,10 @@ export function registerRuntimeRemoveProjectHostScopeTests(): void { const row = findRepoRowForHostScopedWrite(repos, target.id, getRepoExecutionHostId(target)) return row ? Object.assign(row, updates) : null }, + moveProjectToGroup: (target: RepoHostTarget, groupId: string | null) => { + const row = findRepoRowForHostScopedWrite(repos, target.id, getRepoExecutionHostId(target)) + return row ? Object.assign(row, { projectGroupId: groupId }) : null + }, removeProjectForHost } as never) return { runtime, repos, removeProjectForHost } @@ -91,6 +97,34 @@ export function registerRuntimeRemoveProjectHostScopeTests(): void { }) }) + describe('projectGroup.moveProject with the same repo id on two execution hosts', () => { + it('moves the SSH row a path selector picked, not its local sibling', async () => { + const { runtime, repos } = createRuntime() + + await runtime.moveProjectToGroup('path:/remote/dup', 'group-1') + + expect(repos.map((repo) => repo.projectGroupId)).toEqual([undefined, 'group-1']) + }) + + it('moves the local row a name selector picked, not its SSH sibling', async () => { + const { runtime, repos } = createRuntime() + + const moved = await runtime.moveProjectToGroup('name:Dup Local', 'group-1') + + expect(moved).toMatchObject({ path: '/laptop/dup', projectGroupId: 'group-1' }) + expect(repos.map((repo) => repo.projectGroupId)).toEqual(['group-1', undefined]) + }) + + it('refuses a bare duplicated id rather than guessing a host', async () => { + const { runtime, repos } = createRuntime() + + await expect(runtime.moveProjectToGroup('dup', 'group-1')).rejects.toThrow( + 'selector_ambiguous' + ) + expect(repos.map((repo) => repo.projectGroupId)).toEqual([undefined, undefined]) + }) + }) + describe('repo.update with the same repo id on two execution hosts', () => { it('writes the SSH row a path selector picked, not its local sibling', async () => { const { runtime, repos } = createRuntime() diff --git a/src/main/runtime/runtime-repository-settings-controller.ts b/src/main/runtime/runtime-repository-settings-controller.ts index 014f9482dc7..559200ffba1 100644 --- a/src/main/runtime/runtime-repository-settings-controller.ts +++ b/src/main/runtime/runtime-repository-settings-controller.ts @@ -1,4 +1,3 @@ -import { getRepoExecutionHostId } from '../../shared/execution-host' import { isFolderRepo } from '../../shared/repo-kind' import { commitProvenRepoKindChange } from '../folder-repo-git-conversion' import type { Repo } from '../../shared/repo-types' @@ -130,7 +129,6 @@ export class RuntimeRepositorySettingsController { throw new Error('runtime_unavailable') } const repo = typeof target === 'string' ? await this.deps.resolveRepo(target) : target - const hostId = getRepoExecutionHostId(repo) assertWslMoveRepoWritable(store, repo) // Why here: the host owns these shells; a client only stops the workspaces it has loaded, // and older clients stop none. Bounded so repo.rm still answers inside the client's timeout. @@ -145,7 +143,7 @@ export class RuntimeRepositorySettingsController { await stopProjectTerminals(false) // Why again: a WSL move may have fenced the project while the stop was awaited. assertWslMoveRepoWritable(store, repo) - store.removeProjectForHost(repo.id, hostId) + store.removeProjectForHost(repo) // Why again: a terminal spawned while the first stop ran is past that sweep's snapshot, and // nothing can spawn into the project once it is gone. Chats only now: retiring their tabs is // irreversible, and the WSL fence above can still refuse the removal. diff --git a/src/main/ssh/orcad-migration-delta-move.test.ts b/src/main/ssh/orcad-migration-delta-move.test.ts index 346b9b0b4d3..41e330904c2 100644 --- a/src/main/ssh/orcad-migration-delta-move.test.ts +++ b/src/main/ssh/orcad-migration-delta-move.test.ts @@ -426,9 +426,9 @@ describe('moving what an older build added to a converted host', () => { connectionId: TARGET.id, createdFrom: 'manual' }) - store.moveProjectToGroup('repo-1', group.id) + store.moveProjectToGroup(store.getRepo('repo-1')!, group.id) await convertedThenChangedOnOlderBuild() - store.moveProjectToGroup('repo-2', group.id) + store.moveProjectToGroup(store.getRepo('repo-2')!, group.id) const reconnect = loseContactAtDeltaCommit() await expect(deltaMove()).rejects.toThrow('socket closed') const target = store.getSshTarget(TARGET.id)! @@ -608,7 +608,7 @@ describe('a retained source is never deleted', () => { [ 'a folder workspace on a folder-only host', () => { - store.removeProjectForHost('repo-1', `ssh:${TARGET.id}`) + store.removeProjectForHost({ id: 'repo-1', executionHostId: `ssh:${TARGET.id}` }) const group = store.createProjectGroup({ name: 'folders', parentPath: '/srv/folders', @@ -725,7 +725,7 @@ describe('a delta move against a rollback of an update taken before it', () => { it('protects a folder-only delta the same way', async () => { await convertedThenChangedOnOlderBuild() - store.removeProjectForHost('repo-2', `ssh:${TARGET.id}`) + store.removeProjectForHost({ id: 'repo-2', executionHostId: `ssh:${TARGET.id}` }) const group = store.createProjectGroup({ name: 'folders', parentPath: '/srv/folders', diff --git a/src/main/ssh/orcad-migration-repository-link.test.ts b/src/main/ssh/orcad-migration-repository-link.test.ts index 66e9b60f834..2aaf2cd7fe9 100644 --- a/src/main/ssh/orcad-migration-repository-link.test.ts +++ b/src/main/ssh/orcad-migration-repository-link.test.ts @@ -402,7 +402,7 @@ describe('moving the host again after its server was stopped', () => { connectionId: TARGET.id, createdFrom: 'manual' }) - store.moveProjectToGroup('desktop-repo', group.id) + store.moveProjectToGroup(store.getRepo('desktop-repo')!, group.id) return group.id } const groupName = (owner: Store, id: string) => @@ -443,7 +443,7 @@ describe('moving the host again after its server was stopped', () => { server.updateProjectGroup(groupId, { name: 'Renamed on the server' }) stopServer() store.addRepo(repo('added-after-stop', '/root/later')) - store.moveProjectToGroup('added-after-stop', groupId) + store.moveProjectToGroup(store.getRepo('added-after-stop')!, groupId) await expect(convert()).resolves.toMatchObject({ outcome: 'converted' }) expect(server.getRepo('added-after-stop')?.projectGroupId).toBe(groupId) @@ -478,7 +478,7 @@ describe('moving the host again after its server was stopped', () => { connectionId: TARGET.id, createdFrom: 'manual' }) - store.moveProjectToGroup('desktop-repo', child.id) + store.moveProjectToGroup(store.getRepo('desktop-repo')!, child.id) await convert() server.updateProjectGroup(parent.id, { name: 'Parent renamed on the server' }) stopServer() @@ -512,7 +512,7 @@ describe('moving the host again after its server was stopped', () => { await expect(convert()).resolves.toMatchObject({ outcome: 'converted' }) expect(groupName(server, groupId)).toBe('Before') store.addRepo(repo('added-later', '/root/later')) - store.moveProjectToGroup('added-later', groupId) + store.moveProjectToGroup(store.getRepo('added-later')!, groupId) await expect(runDelta()).resolves.toMatchObject({ outcome: 'moved' }) expect(server.getRepo('added-later')?.projectGroupId).toBe(groupId) }) @@ -605,7 +605,7 @@ describe('moving the host again after its server was stopped', () => { await convert() server.updateProjectGroup(groupId, { name: 'Renamed on the server', isCollapsed: true }) store.addRepo(repo('added-later', '/root/later')) - store.moveProjectToGroup('added-later', groupId) + store.moveProjectToGroup(store.getRepo('added-later')!, groupId) await expect(runDelta()).resolves.toMatchObject({ outcome: 'moved' }) expect(server.getRepo('added-later')?.projectGroupId).toBe(groupId) expect(groupName(server, groupId)).toBe('Renamed on the server') @@ -624,10 +624,10 @@ describe('moving the host again after its server was stopped', () => { stopServer() await convert() } - server.moveProjectToGroup('desktop-repo', null) + server.moveProjectToGroup(server.getRepo('desktop-repo')!, null) server.deleteProjectGroup(groupId) store.addRepo(repo('added-later', '/root/later')) - store.moveProjectToGroup('added-later', groupId) + store.moveProjectToGroup(store.getRepo('added-later')!, groupId) await expect(runDelta()).resolves.toMatchObject({ outcome: 'refused', code: 'orcad_delta_refused_by_server' diff --git a/src/main/worktree-removal-close-records.test.ts b/src/main/worktree-removal-close-records.test.ts index 7762fb79b9f..a805273c127 100644 --- a/src/main/worktree-removal-close-records.test.ts +++ b/src/main/worktree-removal-close-records.test.ts @@ -198,7 +198,7 @@ describe('close records on workspace removal', () => { ]) }) - store.removeProjectForHost('repo-a', 'local') + store.removeProjectForHost({ id: 'repo-a', executionHostId: 'local' }) expect( Object.keys(store.getWorkspaceSession().closedTerminalTabTombstonesByTabId ?? {}) diff --git a/src/preload/api/repository-api.ts b/src/preload/api/repository-api.ts index 3d8eb6540b4..5a4003e12e5 100644 --- a/src/preload/api/repository-api.ts +++ b/src/preload/api/repository-api.ts @@ -131,6 +131,7 @@ export type ProjectGroupsApi = { projectId: string groupId: string | null order?: number + hostId?: string }) => Promise scanNested: (args: { path: string diff --git a/src/renderer/src/components/sidebar/project-header-drag-commit.test.ts b/src/renderer/src/components/sidebar/project-header-drag-commit.test.ts index 32fc47acef3..aed02a9876f 100644 --- a/src/renderer/src/components/sidebar/project-header-drag-commit.test.ts +++ b/src/renderer/src/components/sidebar/project-header-drag-commit.test.ts @@ -94,7 +94,7 @@ describe('commitProjectHeaderDragDrop', () => { const repos = [ makeRepo('a', { projectGroupId: 'group-1' }), makeRepo('b', { projectGroupId: 'group-1' }), - makeRepo('c', { projectGroupId: 'group-1' }) + makeRepo('c', { projectGroupId: 'group-1', connectionId: 'ssh-1' }) ] const repoById = new Map(repos.map((repo) => [repo.id, repo])) @@ -108,6 +108,6 @@ describe('commitProjectHeaderDragDrop', () => { onCommitProjectGroupOrder }) - expect(onCommitProjectGroupOrder).toHaveBeenCalledWith('c', 'group-1', -1) + expect(onCommitProjectGroupOrder).toHaveBeenCalledWith('c', 'group-1', -1, 'ssh:ssh-1') }) }) diff --git a/src/renderer/src/components/sidebar/project-header-drag-commit.ts b/src/renderer/src/components/sidebar/project-header-drag-commit.ts index 5786391c253..2bca5c764f8 100644 --- a/src/renderer/src/components/sidebar/project-header-drag-commit.ts +++ b/src/renderer/src/components/sidebar/project-header-drag-commit.ts @@ -7,6 +7,7 @@ import { } from './project-header-drop' import type { ProjectHeaderDragSession } from './project-header-drag-contract' import type { Repo } from '../../../../shared/repo-types' +import { getRepoExecutionHostId, type ExecutionHostId } from '../../../../shared/execution-host' export function commitProjectHeaderDragDrop(args: { session: ProjectHeaderDragSession @@ -15,7 +16,12 @@ export function commitProjectHeaderDragDrop(args: { repoById: ReadonlyMap usesProjectGroupOrdering: boolean onCommitRepoOrder: (orderedIds: string[]) => void - onCommitProjectGroupOrder: (repoId: string, projectGroupId: string | null, order: number) => void + onCommitProjectGroupOrder: ( + repoId: string, + projectGroupId: string | null, + order: number, + hostId: ExecutionHostId + ) => void }): void { const draggedRepo = args.repoById.get(args.session.repoId) if (!draggedRepo) { @@ -59,7 +65,13 @@ export function commitProjectHeaderDragDrop(args: { dropIndex: siblingDropIndex, repoOrderRankById }) - args.onCommitProjectGroupOrder(args.session.repoId, draggedRepo.projectGroupId ?? null, order) + // Why the dragged row's host: its group and order came from that row, so main writes that twin. + args.onCommitProjectGroupOrder( + args.session.repoId, + draggedRepo.projectGroupId ?? null, + order, + getRepoExecutionHostId(draggedRepo) + ) return } diff --git a/src/renderer/src/components/sidebar/project-header-drag-contract.ts b/src/renderer/src/components/sidebar/project-header-drag-contract.ts index f28f34eaffb..476f13fea12 100644 --- a/src/renderer/src/components/sidebar/project-header-drag-contract.ts +++ b/src/renderer/src/components/sidebar/project-header-drag-contract.ts @@ -2,6 +2,7 @@ import type { PointerEvent } from 'react' import type { ProjectHeaderDragBucketKey, ProjectHeaderDragRect } from './project-header-drop' import type { Repo } from '../../../../shared/repo-types' +import type { ExecutionHostId } from '../../../../shared/execution-host' export type RepoDragState = { draggingRepoId: string | null @@ -21,7 +22,12 @@ export type UseRepoHeaderDragArgs = { repoById: ReadonlyMap usesProjectGroupOrdering: boolean onCommitRepoOrder: (orderedIds: string[]) => void - onCommitProjectGroupOrder: (repoId: string, projectGroupId: string | null, order: number) => void + onCommitProjectGroupOrder: ( + repoId: string, + projectGroupId: string | null, + order: number, + hostId: ExecutionHostId + ) => void getScrollContainer: () => HTMLElement | null } diff --git a/src/renderer/src/components/sidebar/worktree-list/drag/use-header-drag.ts b/src/renderer/src/components/sidebar/worktree-list/drag/use-header-drag.ts index dbe79fdd820..d37069945fb 100644 --- a/src/renderer/src/components/sidebar/worktree-list/drag/use-header-drag.ts +++ b/src/renderer/src/components/sidebar/worktree-list/drag/use-header-drag.ts @@ -160,8 +160,8 @@ export function useWorktreeSidebarHeaderDrag(args: { [sidebarProjectGroupHeaderIdsByBucket] ) const commitProjectGroupOrder = useCallback( - (repoId: string, projectGroupId: string | null, order: number) => { - void moveProjectToGroup(repoId, projectGroupId, order) + (repoId: string, projectGroupId: string | null, order: number, hostId: ExecutionHostId) => { + void moveProjectToGroup(repoId, projectGroupId, order, hostId) }, [moveProjectToGroup] ) diff --git a/src/renderer/src/store/project-groups/project-group-mutations.ts b/src/renderer/src/store/project-groups/project-group-mutations.ts index 68c870e856b..2ca879db89f 100644 --- a/src/renderer/src/store/project-groups/project-group-mutations.ts +++ b/src/renderer/src/store/project-groups/project-group-mutations.ts @@ -252,8 +252,9 @@ export function createProjectGroupMutationActions( moveProjectToGroup: async (projectId, groupId, order, hostId) => { try { const repo = findRepoForHost(get().repos, projectId, { settings: get().settings, hostId }) - const target = repo ? ipcOrRuntimeTargetForHostId(getRepoExecutionHostId(repo)) : null - if (!target) { + const repoHostId = repo ? getRepoExecutionHostId(repo) : null + const target = repoHostId ? ipcOrRuntimeTargetForHostId(repoHostId) : null + if (!repoHostId || !target) { return false } if (groupId && !isProjectGroupJoinableFromCatalog(get().projectGroups, groupId, target)) { @@ -265,7 +266,8 @@ export function createProjectGroupMutationActions( ? await window.api.projectGroups.moveProject({ projectId, groupId, - order + order, + hostId: repoHostId }) : ( await callRuntimeRpc<{ repo: Repo | null }>( diff --git a/src/renderer/src/store/slices/repos-project-group-cross-host-move.test.ts b/src/renderer/src/store/slices/repos-project-group-cross-host-move.test.ts index 5a525e026e1..562d37e5200 100644 --- a/src/renderer/src/store/slices/repos-project-group-cross-host-move.test.ts +++ b/src/renderer/src/store/slices/repos-project-group-cross-host-move.test.ts @@ -135,3 +135,22 @@ describe('moving a project into a group on another host', () => { expect(toast.error).not.toHaveBeenCalled() }) }) + +describe('moving one twin of a repo id shared by a local and an SSH row', () => { + it('names the SSH twin host to the local IPC, so main moves that row', async () => { + const localTwin: Repo = { ...remoteRepo, executionHostId: 'local', path: '/laptop/dup' } + const sshTwin: Repo = { ...remoteRepo, executionHostId: undefined, connectionId: 'ssh-1' } + moveProject.mockResolvedValue({ ...sshTwin, projectGroupId: 'ssh-group' }) + const store = createTestStore() + store.setState({ + repos: [localTwin, sshTwin], + projectGroups: [group('ssh-group', 'ssh:ssh-1')] + }) + + await store.getState().moveProjectToGroup(remoteRepo.id, 'ssh-group', undefined, 'ssh:ssh-1') + + expect(moveProject).toHaveBeenCalledWith( + expect.objectContaining({ projectId: remoteRepo.id, hostId: 'ssh:ssh-1' }) + ) + }) +}) diff --git a/src/renderer/src/store/slices/repos-project-groups.test.ts b/src/renderer/src/store/slices/repos-project-groups.test.ts index f21623ab4f2..fe5d6b7af52 100644 --- a/src/renderer/src/store/slices/repos-project-groups.test.ts +++ b/src/renderer/src/store/slices/repos-project-groups.test.ts @@ -856,7 +856,8 @@ describe('project group store routing', () => { expect(projectGroupsMoveProject).toHaveBeenCalledWith({ projectId: remoteRepo.id, groupId: projectGroup.id, - order: 3 + order: 3, + hostId: 'local' }) // Why: the repos slice stamps updated repos with their owning execution // host so multi-host routing never has to guess (multi-host design).