Repository navigation
fix(ssh): reconnecting a stopped managed host keeps the server's project groups - #29962
Conversation
…ect groups 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.
📝 Walkthrough
Priority: ➖ Normal Merge Risk: 🟡 Moderate · up to
|
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
21125747-242c-4e11-845b-9ea770475ed1
📒 Files selected for processing: 6
src/main/ssh/orcad-migration-catalog-client.tssrc/main/ssh/orcad-migration-delta-plan.tssrc/main/ssh/orcad-migration-manifest-export.tssrc/main/ssh/orcad-migration-repository-link.test.tssrc/main/ssh/orcad-migration-repository-link.tssrc/shared/orcad-migration-source-cutover.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.
| { | ||
| ...ids, | ||
| repositoryIds: new Set([...ids.repositoryIds, ...linked]), | ||
| projectGroupIds: new Set([...ids.projectGroupIds, ...linkedGroups]) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Keep a server-renamed group out of later delta exports.
If a later project is assigned to a linked group renamed on the server, treating that group only as a moved group lets the delta source include its stale desktop row when the project needs it. orcadSourceStoreWithout receives no serverGroupIds here, so the manifest export does not remove the row. The delta can then conflict with the server group and block the project move. Preserve the server record when it still exists, and handle a deleted server group separately.
| 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] : []))) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🔎 Supported by static analysis
🏁 Script executed:
set -eu
printf '%s\n' '--- changed file ---'
nl -ba src/main/ssh/orcad-migration-repository-link.ts | sed -n '1,240p'
printf '%s\n' '--- relevant symbols ---'
rg -n -F --glob '*.ts' -e 'projectGroupId' -e 'repo.list' -e 'projectGroup' src/main/ssh src/main/persistence/migrating-orcad-catalog || test "$?" -eq 1
printf '%s\n' '--- base to head diff ---'
git diff --stat 432c6d0735acb4752eb18aabed0a61455cad9ba8 0a87349964a181266caea786b6fb54f2d951d89e -- src/main/ssh/orcad-migration-repository-link.ts
git diff --unified=25 432c6d0735acb4752eb18aabed0a61455cad9ba 0a87349964a181266caea786b6fb54f2d951d89e -- src/main/ssh/orcad-migration-repository-link.ts
printf '%s\n' '--- catalog files ---'
rg --files src/main/ssh src/main/persistence/migrating-orcad-catalog | sort | sed -n '1,240p'Repository: stablyai/orca
Length of output: 41831
🏁 Script executed:
set -eu
printf '%s\n' '--- catalog client contract ---'
nl -ba src/main/ssh/orcad-migration-catalog-client.ts | sed -n '130,205p'
printf '%s\n' '--- source catalog ---'
nl -ba src/main/persistence/migrating-orcad-catalog/orcad-source-catalog.ts
printf '%s\n' '--- group-related source declarations ---'
rg -n -F --glob '*.ts' --glob '!*.test.ts' -e 'listProjectGroups' -e 'getProjectGroups' -e 'projectGroups' -e 'project-group' -e 'projectGroup' src/main src/shared | sed -n '1,320p'
printf '%s\n' '--- all repo.list references ---'
rg -n -F --glob '*.ts' -e 'repo.list' -e 'repoList' src | sed -n '1,240p'Repository: stablyai/orca
Length of output: 41334
🏁 Script executed:
set -eu
printf '%s\n' '--- project-group RPC and handlers ---'
rg --files src/main/runtime/rpc/methods src/main/ipc/repos | rg 'project.group|project-group|repo\.ts|folder-workspace'
for f in src/main/runtime/rpc/methods/project-group.ts src/main/ipc/repos/project-group-handlers.ts src/main/runtime/rpc/methods/repo.ts; do
if test -f "$f"; then
printf '\n--- %s ---\n' "$f"
nl -ba "$f" | sed -n '1,260p'
fi
done
printf '%s\n' '--- project-group and move persistence symbols ---'
rg -n -F --glob '*.ts' --glob '!*.test.ts' -e 'moveProjectToGroup' -e 'deleteProjectGroup' -e 'deleteProject' -e 'projectGroup.list' -e 'projectGroup.create' -e 'projectGroup.update' src/main src/shared | sed -n '1,280p'
printf '%s\n' '--- project group types ---'
nl -ba src/shared/project-group-types.ts | sed -n '1,180p'Repository: stablyai/orca
Length of output: 22418
🏁 Script executed:
set -eu
printf '%s\n' '--- project-group persistence operations ---'
nl -ba src/main/persistence/tracking-repos/project-group-operations.ts | sed -n '1,190p'
printf '%s\n' '--- runtime project-group controller ---'
nl -ba src/main/runtime/runtime-project-group-controller.ts | sed -n '1,210p'
printf '%s\n' '--- runtime visibility implementation ---'
rg -n -F --glob '*.ts' --glob '!*.test.ts' -e 'listVisibleProjectGroups' -e 'visibleProjectGroups' src/main | sed -n '1,160p'Repository: stablyai/orca
Length of output: 15151
🏁 Script executed:
set -eu
printf '%s\n' '--- retained-source visibility ---'
nl -ba src/main/ssh/orcad-retained-source.ts | sed -n '90,145p'
printf '%s\n' '--- runtime visible catalog ---'
nl -ba src/main/runtime/runtime-visible-catalog.ts | sed -n '1,100p'
printf '%s\n' '--- moveProjectToGroup implementation ---'
nl -ba src/main/persistence/restoring-sessions/folder-workspace-operations.ts | sed -n '235,285p'
printf '%s\n' '--- projectGroup.list client consumers ---'
rg -n -F --glob '*.ts' --glob '*.tsx' -e 'projectGroup.list' src/main src/renderer src/shared | sed -n '1,180p'Repository: stablyai/orca
Length of output: 11602
🏁 Script executed:
set -eu
printf '%s\n' '--- destination group import and duplicate handling ---'
nl -ba src/main/persistence/migrating-orcad-catalog/orcad-catalog-records.ts | sed -n '1,135p'
nl -ba src/main/persistence/migrating-orcad-catalog/orcad-catalog-records.ts | sed -n '145,190p'
printf '%s\n' '--- migration manifest export filtering ---'
nl -ba src/main/ssh/orcad-migration-manifest-export.ts | sed -n '35,78p'Repository: stablyai/orca
Length of output: 9450
Include standalone server groups when relinking.
moveProjectToGroup does not delete an empty group. projectGroup.list can still return it. The reconnect code only builds held from repo.list rows, so it can re-export the desktop's stale group and trigger orcad_migration_project_group_id_conflict:<id> when the server already owns that ID. Fetch projectGroup.list with repo.list and include its group IDs when selecting server-owned groups.
ELI5
You moved a remote host onto a managed server, then renamed one of its project groups on that server. Later you stopped the server and connected again. The reconnect failed, because this desktop tried to send its old copy of the group, and the server already had the same group under the new name. Now the server's group wins and the reconnect goes through.
What Changed
Before: reconnecting a stopped managed host relinks the projects the server already holds (#29586) instead of importing them again. But the desktop's retained copies of those projects' groups were still in the import. A group renamed on the server while it was managed therefore failed the whole reconnect with
orcad_migration_project_group_id_conflict.After:
repo.listreply already carries each project'sprojectGroupId, and that is how the move knows which groups the server holds. An ancestor still moves if a project or folder that is moving needs it, since the server might not have it.linkedProjectGroupIds, an optional field next tolinkedRepositoryIds). The fence check rebuilds the same manifest from them.Why
The review suggested skipping the linked projects' groups, or reusing the server's grouping records. Reading the server's group ids from the
repo.listreply it already sends is the least new surface: no new RPC, and old servers still return whole rows. Codex review rounds steered the details:repository_project_group_missing, so only groups the server is proven to hold (and their unneeded ancestors) are skipped.Left as it was before this change: a later delta that adds a row into a group the server has since renamed still conflicts. That conflict already existed for moved groups and needs the server's live state at delta time to resolve.
Linked Issue
External Codex review 3, finding 3 (no tracked issue). Follows up #29586.
Visual Proof
N/A. This changes migration catalog logic, with no UI change.
Testing
src/main/ssh/orcad-migration-repository-link.test.tsuses realStorefixtures for both the desktop and the server. The server side runs the real import, and delta moves go throughrunOrcadDeltaMove. Cases:origin/mainwithorcad_migration_project_group_id_conflictand now converts. The server keeps the new name, and this desktop's retained copy is unchanged.Earlier relink tests still pass unchanged: #29586, #29220, #28886, #29003 and #29896 (same-path project on another machine).
src/main/sshandsrc/main/persistence/migrating-orcad-catalogpass (3330 tests). Fullpnpm tc, oxlint and its plugin configs, the changed-code quality gate, and the max-lines, layout-writer and focus-read ratchets all pass locally.Verified live versus only in tests: tests only. A live run needs a managed-server deploy, a stop, and a reconnect on a real host. The test fixtures run the real server-side import code.
Review
codex review(gpt-6.1-sol, high) ran 4 rounds. Every finding was fixed (see Why), and the final round reported no findings.Agent skill upstream boundary
Notes
projectGroupIdfromrepo.list, a field the server already sends. Nothing new is sent.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)