mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
perf(renderer): keep project catalog identity on SSH readoption
* 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 <help@stably.ai> * type(store): mark pending SSH readoptions readonly Widen pendingSshRepoReadoptions, recordSshRepoReadoptions, and the merge/reconcile helpers to readonly so a reused pending queue cannot be mutated in place after a no-op catalog reconcile. Co-authored-by: Orca <help@stably.ai> * perf(renderer): drop redundant catalogRowsUnchanged after reconcile reconcileCatalogRows already hands back the previous array when nothing moved, so the extra element-wise compare could never disagree. Pin the empty-readoption early return with a whole-state identity assertion, which is the only guard the previous test could not distinguish. Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -43,7 +43,7 @@ export async function saveNewSshHostFromForm({
|
||||
}: {
|
||||
form: EditingTarget
|
||||
ssh: SshApi
|
||||
recordSshRepoReadoptions: (readoptions: SshRepoReadoption[]) => void
|
||||
recordSshRepoReadoptions: (readoptions: readonly SshRepoReadoption[]) => void
|
||||
setSshTargetsMetadata: (targets: SshTarget[]) => void
|
||||
recordFeatureInteraction: (feature: 'ssh') => void
|
||||
}): Promise<'saved' | 'validation-failed' | 'failed'> {
|
||||
@@ -170,7 +170,7 @@ export async function addAllSshConfigHostsToOrca({
|
||||
recordFeatureInteraction
|
||||
}: {
|
||||
ssh: SshApi
|
||||
recordSshRepoReadoptions: (readoptions: SshRepoReadoption[]) => void
|
||||
recordSshRepoReadoptions: (readoptions: readonly SshRepoReadoption[]) => void
|
||||
setSshTargetsMetadata: (targets: SshTarget[]) => void
|
||||
recordFeatureInteraction: (feature: 'ssh') => void
|
||||
}): Promise<{ kind: 'added'; count: number } | { kind: 'already-synced' } | { kind: 'failed' }> {
|
||||
|
||||
@@ -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,78 @@ 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 before = store.getState()
|
||||
const projects = before.projects
|
||||
const setups = before.projectHostSetups
|
||||
expect(projects).toHaveLength(1)
|
||||
expect(setups).toHaveLength(1)
|
||||
|
||||
store.getState().recordSshRepoReadoptions([])
|
||||
|
||||
// Why: the empty-in/empty-pending call must hand the state object back untouched, or the
|
||||
// freshly allocated pendingSshRepoReadoptions alone would wake every store subscriber.
|
||||
expect(store.getState()).toBe(before)
|
||||
expect(store.getState().pendingSshRepoReadoptions).toBe(before.pendingSshRepoReadoptions)
|
||||
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([])
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -1781,8 +1781,8 @@ export type RepoSlice = {
|
||||
activeRepoId: string | null
|
||||
// Monotonic sequence so overlapping catalog fetches can drop stale same-host results (#7020).
|
||||
reposFetchGeneration: number
|
||||
pendingSshRepoReadoptions: SshRepoReadoption[]
|
||||
recordSshRepoReadoptions: (readoptions: SshRepoReadoption[]) => void
|
||||
pendingSshRepoReadoptions: readonly SshRepoReadoption[]
|
||||
recordSshRepoReadoptions: (readoptions: readonly SshRepoReadoption[]) => void
|
||||
fetchRepos: (options?: RuntimeCatalogFetchOptions) => Promise<void>
|
||||
fetchReposForAllHosts: (options?: AllHostCatalogFetchOptions) => Promise<void>
|
||||
awaitLocalRepoCatalogSettlement: () => Promise<void>
|
||||
@@ -2004,6 +2004,10 @@ export const createRepoSlice: StateCreator<AppState, [], [], RepoSlice> = (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<AppState, [], [], RepoSlice> = (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. Reconcile
|
||||
// hands the store arrays straight back when nothing moved, so writing them
|
||||
// unconditionally still leaves identity-keyed selectors untouched.
|
||||
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
|
||||
projects,
|
||||
projectHostSetups
|
||||
}
|
||||
}),
|
||||
|
||||
|
||||
@@ -4,7 +4,7 @@ import { getRepoExecutionHostId, toSshExecutionHostId } from '../../../../shared
|
||||
|
||||
export type SshRepoReconciliation = {
|
||||
repos: readonly Repo[]
|
||||
pendingReadoptions: SshRepoReadoption[]
|
||||
pendingReadoptions: readonly SshRepoReadoption[]
|
||||
}
|
||||
|
||||
function repoBelongsToTarget(repo: Repo, targetId: string): boolean {
|
||||
@@ -69,7 +69,7 @@ export function reconcileReadoptedSshRepoRows(
|
||||
export function mergeSshRepoReadoptions(
|
||||
pending: readonly SshRepoReadoption[],
|
||||
incoming: readonly SshRepoReadoption[]
|
||||
): SshRepoReadoption[] {
|
||||
): readonly SshRepoReadoption[] {
|
||||
const repoIdsByMigration = new Map<string, Set<string>>()
|
||||
for (const readoption of [...pending, ...incoming]) {
|
||||
const key = `${readoption.oldTargetId}\0${readoption.newTargetId}`
|
||||
|
||||
Reference in New Issue
Block a user