From 0d9beb603ea6fa15c1dec6b08d60505c31fc9923 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 16 Oct 2026 21:54:37 -0700 Subject: [PATCH] fix(ssh): reconnecting a stopped managed host keeps the server's project groups (#29962) Relinking projects the server already held still re-imported this desktop's retained copy of their project groups, so a group renamed on the server while it was managed failed the reconnect with orcad_migration_project_group_id_conflict. A relinked move now leaves to the server every group its projects sit in (read from repo.list's projectGroupId) and those groups' ancestors unless a moving row needs one, journals those ids, and keeps them out of the exported rows while still reading them for folder-workspace host ownership. Later delta moves subtract them as they do moved groups. --- .../ssh/orcad-migration-catalog-client.ts | 8 +- src/main/ssh/orcad-migration-delta-plan.ts | 16 ++- .../ssh/orcad-migration-manifest-export.ts | 14 +- .../orcad-migration-repository-link.test.ts | 124 ++++++++++++++++++ .../ssh/orcad-migration-repository-link.ts | 76 +++++++++-- src/shared/orcad-migration-source-cutover.ts | 6 + 6 files changed, 225 insertions(+), 19 deletions(-) diff --git a/src/main/ssh/orcad-migration-catalog-client.ts b/src/main/ssh/orcad-migration-catalog-client.ts index ddc076e7e39..ce4d630b0f3 100644 --- a/src/main/ssh/orcad-migration-catalog-client.ts +++ b/src/main/ssh/orcad-migration-catalog-client.ts @@ -160,7 +160,8 @@ const ServerRepositoriesSchema = z.object({ id: z.string().min(1), path: z.string().min(1), connectionId: z.string().nullish(), - executionHostId: z.string().nullish() + executionHostId: z.string().nullish(), + projectGroupId: z.string().nullish() }) ) }) @@ -179,11 +180,12 @@ export async function listRemoteOrcadRepositories( throw new Error(`orcad_migration_repositories_failed:${response.error.message}`) } return ServerRepositoriesSchema.parse(response.result).repos.map( - ({ id, path, connectionId, executionHostId }) => ({ + ({ id, path, connectionId, executionHostId, projectGroupId }) => ({ id, path, ...(connectionId != null ? { connectionId } : {}), - ...(executionHostId != null ? { executionHostId } : {}) + ...(executionHostId != null ? { executionHostId } : {}), + ...(projectGroupId != null ? { projectGroupId } : {}) }) ) } diff --git a/src/main/ssh/orcad-migration-delta-plan.ts b/src/main/ssh/orcad-migration-delta-plan.ts index 3c9fcf2e704..2063f1ec3d8 100644 --- a/src/main/ssh/orcad-migration-delta-plan.ts +++ b/src/main/ssh/orcad-migration-delta-plan.ts @@ -56,12 +56,19 @@ export function orcadDeltaSourceStore( moved: readonly OrcadMigrationSourceCutover[] ): OrcadDeltaSourceStore { const ids = orcadMigrationCatalogIds(moved.map((cutover) => cutover.manifest.payload)) - // A linked project is the server's already, so a delta never moves it either. + // A linked project or group is the server's already, so a delta never moves it either. const linked = moved.flatMap((cutover) => cutover.linkedRepositoryIds ?? []) + const linkedGroups = moved.flatMap((cutover) => cutover.linkedProjectGroupIds ?? []) + // Why linked groups subtract like moved ones: a group a new row needs still moves, since the + // server may have deleted it; an unused one never reappears as this desktop's addition. return orcadSourceStoreWithout( store, target, - { ...ids, repositoryIds: new Set([...ids.repositoryIds, ...linked]) }, + { + ...ids, + repositoryIds: new Set([...ids.repositoryIds, ...linked]), + projectGroupIds: new Set([...ids.projectGroupIds, ...linkedGroups]) + }, moved.at(-1)?.destinationEnvironmentId ) } @@ -71,7 +78,9 @@ export function orcadSourceStoreWithout( store: Store, target: SshTarget, owned: OrcadMigrationCatalogIds, - destinationEnvironmentId: string | undefined + destinationEnvironmentId: string | undefined, + /** Groups the server's record wins for: never exported, even while a moving row sits in one. */ + serverGroupIds: ReadonlySet = new Set() ): OrcadDeltaSourceStore { const ownedNow = createOrcadMigrationManifest(store, target, { destinationEnvironmentId, @@ -80,6 +89,7 @@ export function orcadSourceStoreWithout( const view = store.createOrcadMigrationDeltaView(ownedNow) return { ...view, + serverOwnedProjectGroupIds: serverGroupIds, getSshTarget: (id) => store.getSshTarget(id), getSshRemotePtyLeases: (id) => store.getSshRemotePtyLeases(id), readOrcadMigrationSourceSnapshotChunk: (...args) => diff --git a/src/main/ssh/orcad-migration-manifest-export.ts b/src/main/ssh/orcad-migration-manifest-export.ts index 513d48312e8..c020e199035 100644 --- a/src/main/ssh/orcad-migration-manifest-export.ts +++ b/src/main/ssh/orcad-migration-manifest-export.ts @@ -29,7 +29,10 @@ export type OrcadMigrationExportStore = Pick< | 'getFolderWorkspaces' | 'getProjectGroups' | 'getRepos' -> +> & { + /** Groups the server's record wins for: still read for row ownership, never exported. */ + serverOwnedProjectGroupIds?: ReadonlySet +} export type OrcadMigrationManifestExportOptions = { migrationId?: string @@ -47,10 +50,17 @@ export function createOrcadMigrationManifest( options: OrcadMigrationManifestExportOptions = {} ): OrcadMigrationManifest { const rewrite = options.rewrite ?? IDENTITY_ORCAD_MIGRATION_PATH_REWRITER - const sourcePayload = restrictCatalog( + const collected = restrictCatalog( collectOrcadMigrationSourceCatalog(store, target), options.onlyCatalog ) + const serverOwned = store.serverOwnedProjectGroupIds + const sourcePayload = serverOwned + ? { + ...collected, + projectGroups: collected.projectGroups.filter((group) => !serverOwned.has(group.id)) + } + : collected const source = { sshTargetId: target.id, sshTargetGeneration: target.generation ?? null, diff --git a/src/main/ssh/orcad-migration-repository-link.test.ts b/src/main/ssh/orcad-migration-repository-link.test.ts index ad500417652..e85469543b6 100644 --- a/src/main/ssh/orcad-migration-repository-link.test.ts +++ b/src/main/ssh/orcad-migration-repository-link.test.ts @@ -42,6 +42,8 @@ vi.mock('./orcad-managed-tunnel', () => ({ })) const { convertSshTargetToManagedOrcad } = await import('./orcad-runtime-conversion') +const { runOrcadDeltaMove } = await import('./orcad-migration-delta-move') +const { runTargetLifecycle } = await import('../ipc/ssh-target-lifecycle-queue') const TARGET: SshTarget = { id: 'ssh-shared', @@ -363,6 +365,128 @@ describe('moving the host again after its server was stopped', () => { ) }) + function groupDesktopRepo(): string { + const group = store.createProjectGroup({ + name: 'Before', + parentPath: '/root', + connectionId: TARGET.id, + createdFrom: 'manual' + }) + store.moveProjectToGroup('desktop-repo', group.id) + return group.id + } + const groupName = (owner: Store, id: string) => + owner.getProjectGroups().find((entry) => entry.id === id)?.name + // The delta move an older build's later edit asks for, against the real server store. + const runDelta = () => { + reconcileManagedOrcadSshTargets(userDataPath, store) + return runOrcadDeltaMove({ + userDataPath, + store, + claims: targetStore.getOrcadRuntimeClaims(), + target: store.getSshTarget(TARGET.id)!, + environment: listEnvironments(userDataPath)[0]!, + destination, + censusHost: async () => ({ verdict: 'exited', count: 0 }), + ensureTunnel: async () => {}, + runTargetLifecycle + }) + } + + // Review repro: the linked project's retained group was imported over the server's renamed one. + it('keeps the server’s project group when it changed while managed', async () => { + const groupId = groupDesktopRepo() + await expect(convert()).resolves.toMatchObject({ outcome: 'converted' }) + server.updateProjectGroup(groupId, { name: 'Renamed on the server' }) + stopServer() + + await expect(convert()).resolves.toMatchObject({ outcome: 'converted' }) + expect(groupName(server, groupId)).toBe('Renamed on the server') + expect(server.getRepo('desktop-repo')?.projectGroupId).toBe(groupId) + // This desktop's retained copy stays as it was, for a downgraded build. + expect(groupName(store, groupId)).toBe('Before') + }) + + it('moves a new project into a group the server holds in changed form', async () => { + const groupId = groupDesktopRepo() + await expect(convert()).resolves.toMatchObject({ outcome: 'converted' }) + server.updateProjectGroup(groupId, { name: 'Renamed on the server' }) + stopServer() + store.addRepo(repo('added-after-stop', '/root/later')) + store.moveProjectToGroup('added-after-stop', groupId) + + await expect(convert()).resolves.toMatchObject({ outcome: 'converted' }) + expect(server.getRepo('added-after-stop')?.projectGroupId).toBe(groupId) + expect(groupName(server, groupId)).toBe('Renamed on the server') + }) + + it('never offers the server’s group again in a later delta move', async () => { + const groupId = groupDesktopRepo() + await convert() + server.updateProjectGroup(groupId, { name: 'Renamed on the server' }) + stopServer() + await convert() + store.addRepo(repo('added-later', '/root/later')) + reconcileManagedOrcadSshTargets(userDataPath, store) + const target = store.getSshTarget(TARGET.id)! + const plan = planOrcadDeltaMove(userDataPath, store, target) + expect(plan.added.map((row) => row.id)).toEqual(['added-later']) + }) + + // Codex repro: a nested group's moved parent was re-exported in a later delta. + it('keeps the server’s nested groups out of the relink and later deltas', async () => { + const parent = store.createProjectGroup({ + name: 'Parent', + parentPath: '/root', + connectionId: TARGET.id, + createdFrom: 'manual' + }) + const child = store.createProjectGroup({ + name: 'Child', + parentPath: '/root', + parentGroupId: parent.id, + connectionId: TARGET.id, + createdFrom: 'manual' + }) + store.moveProjectToGroup('desktop-repo', child.id) + await convert() + server.updateProjectGroup(parent.id, { name: 'Parent renamed on the server' }) + stopServer() + await expect(convert()).resolves.toMatchObject({ outcome: 'converted' }) + expect(groupName(server, parent.id)).toBe('Parent renamed on the server') + + store.addRepo(repo('added-later', '/root/later')) + await expect(runDelta()).resolves.toMatchObject({ outcome: 'moved' }) + expect(server.getRepo('added-later')).toBeDefined() + expect(groupName(server, parent.id)).toBe('Parent renamed on the server') + }) + + // Codex repro: hiding the group from the source also hid the host a folder takes from it. + it('still moves a later folder workspace that takes its host from the server’s group', async () => { + const groupId = groupDesktopRepo() + await convert() + stopServer() + await convert() + const folder = store.createFolderWorkspace({ + projectGroupId: groupId, + folderPath: '/root/notes' + }) + await expect(runDelta()).resolves.toMatchObject({ outcome: 'moved' }) + expect(server.getFolderWorkspace(folder.id)?.projectGroupId).toBe(groupId) + }) + + // Codex repro: a group only a linked project used never reached the server, so it still moves. + it('moves a linked project’s group the server does not hold, so later rows can use it', async () => { + otherDesktopAddsSameCheckout() + const groupId = groupDesktopRepo() + 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) + await expect(runDelta()).resolves.toMatchObject({ outcome: 'moved' }) + expect(server.getRepo('added-later')?.projectGroupId).toBe(groupId) + }) + it('resumes, not relinks, a move the server committed before the reply was lost', async () => { const commit = destination.commit destination.commit = async (manifest) => { diff --git a/src/main/ssh/orcad-migration-repository-link.ts b/src/main/ssh/orcad-migration-repository-link.ts index 8ea39dafbbe..a78583d6d68 100644 --- a/src/main/ssh/orcad-migration-repository-link.ts +++ b/src/main/ssh/orcad-migration-repository-link.ts @@ -31,6 +31,8 @@ import type { SshTargetOrcadClaims } from './ssh-target-orcad-claims' export type OrcadServerRepository = Pick & { connectionId?: string | null executionHostId?: string | null + /** Proves the server holds this group: its record, not this desktop's copy, wins on relink. */ + projectGroupId?: string | null } /** @@ -78,16 +80,56 @@ async function serverHeldRepositoriesToLink( return state.state === 'committed' ? held.otherId : [...held.otherId, ...held.sameId] } -/** The source the cutover was cut from: the real store less any project it linked. */ +/** + * Source groups the server's projects sit in, and their ancestors, renamed there perhaps since: + * re-importing this desktop's retained copy would conflict with the server's. An ancestor the + * server may lack still moves while a moving row needs it. + */ +function serverHeldProjectGroups( + payload: OrcadMigrationSourceCutover['manifest']['payload'], + linkedRepositoryIds: ReadonlySet, + server: readonly OrcadServerRepository[] +): string[] { + const parentById = new Map(payload.projectGroups.map((group) => [group.id, group.parentGroupId])) + const held = new Set(server.flatMap((repo) => (repo.projectGroupId ? [repo.projectGroupId] : []))) + const withAncestors = (ids: (string | null | undefined)[], stopAt: ReadonlySet) => { + const out = new Set() + while (ids.length > 0) { + const id = ids.pop() + if (id && !out.has(id) && !stopAt.has(id) && parentById.has(id)) { + out.add(id) + ids.push(parentById.get(id)) + } + } + return out + } + const candidates = withAncestors([...held], new Set()) + const needed = withAncestors( + [ + ...payload.repositories + .filter((repo) => !linkedRepositoryIds.has(repo.id)) + .map((repo) => repo.projectGroupId), + ...payload.folderWorkspaces.map((workspace) => workspace.projectGroupId), + ...payload.projectGroups + .filter((group) => !candidates.has(group.id)) + .map((group) => group.parentGroupId) + ], + held + ) + return [...candidates].filter((id) => !needed.has(id)) +} + +/** The source the cutover was cut from: the real store less any project or group it linked. */ export function linkedOrcadMigrationSource( store: Store, cutover: Pick< OrcadMigrationSourceCutover, - 'sshTargetId' | 'destinationEnvironmentId' | 'linkedRepositoryIds' + 'sshTargetId' | 'destinationEnvironmentId' | 'linkedRepositoryIds' | 'linkedProjectGroupIds' > ): (() => OrcadDeltaSourceStore) | undefined { const linked = cutover.linkedRepositoryIds ?? [] - if (linked.length === 0) { + const linkedGroups = cutover.linkedProjectGroupIds ?? [] + if (linked.length === 0 && linkedGroups.length === 0) { return undefined } return () => { @@ -103,7 +145,8 @@ export function linkedOrcadMigrationSource( folderWorkspaceIds: new Set(), projectGroupIds: new Set() }, - cutover.destinationEnvironmentId + cutover.destinationEnvironmentId, + new Set(linkedGroups) ) } } @@ -190,11 +233,8 @@ export async function linkServerHeldRepositories(args: { if (cutover.phase !== 'source-fenced' && cutover.phase !== 'destination-staged') { return cutover } - const linked = await serverHeldRepositoriesToLink( - cutover, - args.destination, - await args.serverRepositories() - ) + const serverRepositories = await args.serverRepositories() + const linked = await serverHeldRepositoriesToLink(cutover, args.destination, serverRepositories) if (linked.length === 0) { return cutover } @@ -203,7 +243,21 @@ export async function linkServerHeldRepositories(args: { // The relinked manifest is cut from the source as fenced, so it must still be that source. source.verifyFence(cutover) const linkedRepositoryIds = [...new Set([...(cutover.linkedRepositoryIds ?? []), ...linked])] - const view = linkedOrcadMigrationSource(args.store, { ...cutover, linkedRepositoryIds })?.() + const linkedProjectGroupIds = [ + ...new Set([ + ...(cutover.linkedProjectGroupIds ?? []), + ...serverHeldProjectGroups( + cutover.manifest.payload, + new Set(linkedRepositoryIds), + serverRepositories + ) + ]) + ] + const linkedIds = { + linkedRepositoryIds, + ...(linkedProjectGroupIds.length > 0 ? { linkedProjectGroupIds } : {}) + } + const view = linkedOrcadMigrationSource(args.store, { ...cutover, ...linkedIds })?.() const target = args.store.getSshTarget(cutover.sshTargetId) if (!view || !target) { throw new Error('orcad_migration_source_fence_lost') @@ -219,7 +273,7 @@ export async function linkServerHeldRepositories(args: { updatedAt: (args.now ?? (() => new Date()))().toISOString(), manifestSha256: manifest.manifestSha256, manifest, - linkedRepositoryIds, + ...linkedIds, // The retained rows still hold the linked projects, so the baseline is the whole fenced source. sourceBaselineFingerprint: cutover.sourceBaselineFingerprint ?? orcadCatalogFingerprint(cutover.manifest.payload) diff --git a/src/shared/orcad-migration-source-cutover.ts b/src/shared/orcad-migration-source-cutover.ts index 1676e7bd539..170bc79dff1 100644 --- a/src/shared/orcad-migration-source-cutover.ts +++ b/src/shared/orcad-migration-source-cutover.ts @@ -4,6 +4,7 @@ */ import { z } from 'zod' import { + MAX_ORCAD_MIGRATION_PROJECT_GROUPS, MAX_ORCAD_MIGRATION_REPOSITORIES, parseOrcadMigrationManifest, type OrcadMigrationManifest @@ -53,6 +54,11 @@ const CutoverRecordSchema = z.object({ .array(z.string().min(1).max(1_024)) .max(MAX_ORCAD_MIGRATION_REPOSITORIES) .optional(), + // Source project groups the server already holds, or only linked projects used: its record wins. + linkedProjectGroupIds: z + .array(z.string().min(1).max(1_024)) + .max(MAX_ORCAD_MIGRATION_PROJECT_GROUPS) + .optional(), manifest: z.unknown() })