From fe2593c2f42d881c2ae3e7bc477f509e47dd7357 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 12 Aug 2026 22:26:09 -0700 Subject: [PATCH] perf(renderer): keep project catalog identity on SSH readoption SshPane calls recordSshRepoReadoptions on every Manage-pane importConfig(), often with []. The publisher always rebuilt projects and projectHostSetups via projectCompatibilityFromRepos then mergeProjectHostSetupCompatibility and spread the new objects into set(), so WeakMap and Object.is selectors missed even when nothing was re-adopted. Fetch paths already reconcile through mergeFetchedProjectCompatibilityForHost, but that helper is host-scoped and this write is all-repos. Reconcile the merged rows against the previous catalog with the same identity keys, omit unchanged keys from set(), and skip the work entirely when both the incoming and pending readoption lists are empty. Tests reuse the production-shaped nested repo fixture and the structuredClone fetch path. A pending-only readoption stays identity-stable, and removing the two reconcile calls turns that case red. Co-authored-by: Orca --- .../slices/repos-refresh-identity.test.ts | 71 ++++++++++++++++++- src/renderer/src/store/slices/repos.ts | 26 ++++++- 2 files changed, 93 insertions(+), 4 deletions(-) diff --git a/src/renderer/src/store/slices/repos-refresh-identity.test.ts b/src/renderer/src/store/slices/repos-refresh-identity.test.ts index 5ebf8b6602d..cac63ee0024 100644 --- a/src/renderer/src/store/slices/repos-refresh-identity.test.ts +++ b/src/renderer/src/store/slices/repos-refresh-identity.test.ts @@ -283,7 +283,6 @@ describe('repo filter identity across catalog refreshes', () => { expect(store.getState().filterRepoIds).toBe(first) }) }) - describe('setup-script dismissal identity across catalog refreshes', () => { it('keeps the dismissal array when a refetch prunes nothing', async () => { const store = createTestStore() @@ -301,3 +300,73 @@ describe('setup-script dismissal identity across catalog refreshes', () => { expect(store.getState().setupScriptPromptDismissedRepoIds).toBe(first) }) }) + +describe('SSH readoption catalog identity', () => { + it('keeps projects and host setups across a no-op recordSshRepoReadoptions([])', async () => { + const store = createTestStore() + await store.getState().fetchRepos() + const projects = store.getState().projects + const setups = store.getState().projectHostSetups + expect(projects).toHaveLength(1) + expect(setups).toHaveLength(1) + + store.getState().recordSshRepoReadoptions([]) + + expect(store.getState().projects).toBe(projects) + expect(store.getState().projects[0]).toBe(projects[0]) + expect(store.getState().projectHostSetups).toBe(setups) + expect(store.getState().projectHostSetups[0]).toBe(setups[0]) + }) + + it('keeps catalog identity for a pending-only readoption while pending updates', async () => { + const store = createTestStore() + await store.getState().fetchRepos() + const projects = store.getState().projects + const setups = store.getState().projectHostSetups + const readoption = { oldTargetId: 'ssh-old', newTargetId: 'ssh-new', repoIds: [repo.id] } + + store.getState().recordSshRepoReadoptions([readoption]) + + expect(store.getState().pendingSshRepoReadoptions).toEqual([readoption]) + expect(store.getState().projects).toBe(projects) + expect(store.getState().projects[0]).toBe(projects[0]) + expect(store.getState().projectHostSetups).toBe(setups) + expect(store.getState().projectHostSetups[0]).toBe(setups[0]) + }) + + it('replaces the moved setup on a real prune/rehome', async () => { + const oldHostRepo: Repo = { + ...repo, + connectionId: 'ssh-old', + executionHostId: 'ssh:ssh-old' + } + const newHostRepo: Repo = { + ...repo, + connectionId: 'ssh-new', + executionHostId: 'ssh:ssh-new' + } + mockRepos(oldHostRepo, newHostRepo) + const store = createTestStore() + await store.getState().fetchRepos() + const setups = store.getState().projectHostSetups + expect(setups).toHaveLength(2) + const oldSetup = setups.find((setup) => setup.hostId === 'ssh:ssh-old') + const newSetup = setups.find((setup) => setup.hostId === 'ssh:ssh-new') + expect(oldSetup).toBeDefined() + expect(newSetup).toBeDefined() + + store.getState().recordSshRepoReadoptions([ + { oldTargetId: 'ssh-old', newTargetId: 'ssh-new', repoIds: [repo.id] } + ]) + + const next = store.getState().projectHostSetups + expect(next).not.toBe(setups) + expect(next).toHaveLength(1) + expect(next[0]).not.toBe(oldSetup) + expect(next[0]?.hostId).toBe('ssh:ssh-new') + expect(store.getState().repos).toHaveLength(1) + expect(store.getState().repos[0]?.executionHostId).toBe('ssh:ssh-new') + expect(store.getState().pendingSshRepoReadoptions).toEqual([]) + }) +}) + diff --git a/src/renderer/src/store/slices/repos.ts b/src/renderer/src/store/slices/repos.ts index 8bdda9f41e3..d5bd49f5a95 100644 --- a/src/renderer/src/store/slices/repos.ts +++ b/src/renderer/src/store/slices/repos.ts @@ -2004,6 +2004,10 @@ export const createRepoSlice: StateCreator = (set, recordSshRepoReadoptions: (readoptions) => set((s) => { + // Why: SshPane importConfig() often reports [] on every Manage-pane open. + if (readoptions.length === 0 && s.pendingSshRepoReadoptions.length === 0) { + return s + } const pendingSshRepoReadoptions = mergeSshRepoReadoptions( s.pendingSshRepoReadoptions, readoptions @@ -2011,19 +2015,35 @@ export const createRepoSlice: StateCreator = (set, const reconciliation = reconcileReadoptedSshRepoRows(s.repos, pendingSshRepoReadoptions) const repos = reconciliation.repos const worktreeState = reconcileReadoptedSshWorktreeState(s, pendingSshRepoReadoptions) - const projectHostSetups = filterSetupsForPrunedRepoRows(s.projectHostSetups, s.repos, repos) + const remainingSetups = filterSetupsForPrunedRepoRows(s.projectHostSetups, s.repos, repos) const compatibility = mergeProjectHostSetupCompatibility( projectCompatibilityFromRepos(repos), { projects: s.projects, - setups: projectHostSetups + setups: remainingSetups } ) + // Why: mergeProjectHostSetupCompatibility always allocates; a no-op readoption + // must not churn catalog identity. This write is all-repos, not host-scoped, + // so it cannot go through mergeFetchedProjectCompatibilityForHost. + const projects = reconcileCatalogRows( + s.projects, + compatibility.projects, + (project) => project.id + ) + const projectHostSetups = reconcileCatalogRows( + s.projectHostSetups, + compatibility.projectHostSetups, + getProjectHostSetupOwnerKey + ) return { repos, pendingSshRepoReadoptions: reconciliation.pendingReadoptions, ...worktreeState, - ...compatibility + ...(catalogRowsUnchanged(projects, s.projects) ? {} : { projects }), + ...(catalogRowsUnchanged(projectHostSetups, s.projectHostSetups) + ? {} + : { projectHostSetups }) } }),