From 69a907e7144b9c8c0e43ba29269c1af01eec67e2 Mon Sep 17 00:00:00 2001 From: SardorBek Sattarov <95975648+sardorb3k@users.noreply.github.com> Date: Fri, 15 May 2026 18:58:12 +0500 Subject: [PATCH] perf(team): cut CreateTeamDialog re-render storm during CLI verify Stabilize cliStatus identity across cloned IPC snapshots, memoize the roster editor boundary, and defer/cancel provider prepare diagnostics idle work. --- .../team/dialogs/CreateTeamDialog.tsx | 536 ++++++++++-------- .../team/members/TeamRosterEditorSection.tsx | 5 +- .../__tests__/cliInstallerSlice.test.ts | 164 ++++++ .../store/slices/cliInstallerSlice.ts | 165 +++++- 4 files changed, 636 insertions(+), 234 deletions(-) create mode 100644 src/renderer/store/slices/__tests__/cliInstallerSlice.test.ts diff --git a/src/renderer/components/team/dialogs/CreateTeamDialog.tsx b/src/renderer/components/team/dialogs/CreateTeamDialog.tsx index 4a35d310..b72b6ac3 100644 --- a/src/renderer/components/team/dialogs/CreateTeamDialog.tsx +++ b/src/renderer/components/team/dialogs/CreateTeamDialog.tsx @@ -24,7 +24,6 @@ import { createMemberDraft, normalizeLeadProviderForMode, normalizeMemberDraftForProviderMode, - normalizeProviderForMode, validateMemberNameInline, } from '@renderer/components/team/members/MembersEditorSection'; import { TeamRosterEditorSection } from '@renderer/components/team/members/TeamRosterEditorSection'; @@ -85,6 +84,7 @@ import { DEFAULT_PROVIDER_MODEL_SELECTION } from '@shared/utils/providerModelSel import { resolveTeamLeadColorName } from '@shared/utils/teamMemberColors'; import { isTeamProviderId, normalizeOptionalTeamProviderId } from '@shared/utils/teamProvider'; import { AlertTriangle, CheckCircle2, Info, Loader2, X } from 'lucide-react'; +import { useShallow } from 'zustand/react/shallow'; import { AdvancedCliSection } from './AdvancedCliSection'; import { AnthropicFastModeSelector } from './AnthropicFastModeSelector'; @@ -331,6 +331,36 @@ function validateRequest( return { valid: true }; } +type IdleWindow = Window & { + requestIdleCallback?: (callback: () => void, options?: { timeout: number }) => number; + cancelIdleCallback?: (id: number) => void; +}; + +interface ScheduledIdleHandle { + kind: 'idle' | 'timeout'; + id: number; +} + +function scheduleIdle(cb: () => void): ScheduledIdleHandle { + const idleWindow = window as IdleWindow; + if (typeof idleWindow.requestIdleCallback === 'function') { + return { kind: 'idle', id: idleWindow.requestIdleCallback(cb, { timeout: 2000 }) }; + } + return { kind: 'timeout', id: window.setTimeout(cb, 0) }; +} + +function cancelScheduledIdle(handle: ScheduledIdleHandle | null): void { + if (!handle) return; + if (handle.kind === 'idle') { + const idleWindow = window as IdleWindow; + if (typeof idleWindow.cancelIdleCallback === 'function') { + idleWindow.cancelIdleCallback(handle.id); + } + return; + } + window.clearTimeout(handle.id); +} + export const CreateTeamDialog = ({ open, canCreate, @@ -350,8 +380,9 @@ export const CreateTeamDialog = ({ const anthropicProviderFastModeDefault = useStore( (s) => s.appConfig?.providerConnections?.anthropic.fastModeDefault ?? false ); - const cliStatus = useStore((s) => s.cliStatus); - const cliStatusLoading = useStore((s) => s.cliStatusLoading); + const { cliStatus, cliStatusLoading } = useStore( + useShallow((s) => ({ cliStatus: s.cliStatus, cliStatusLoading: s.cliStatusLoading })) + ); const bootstrapCliStatus = useStore((s) => s.bootstrapCliStatus); const fetchCliStatus = useStore((s) => s.fetchCliStatus); const openDashboard = useStore((s) => s.openDashboard); @@ -413,6 +444,7 @@ export const CreateTeamDialog = ({ const [prepareWarnings, setPrepareWarnings] = useState([]); const [prepareChecks, setPrepareChecks] = useState([]); const prepareRequestSeqRef = useRef(0); + const prepareIdleHandleRef = useRef(null); const appliedDefaultProjectPathRef = useRef(null); const lastAutoDescriptionRef = useRef(null); const [fieldErrors, setFieldErrors] = useState<{ @@ -435,7 +467,7 @@ export const CreateTeamDialog = ({ const [anthropicRuntimeNotice, setAnthropicRuntimeNotice] = useState(null); // Advanced CLI section state (use teamName-derived key for localStorage) - const advancedKey = sanitizeTeamName(teamName.trim()) || '_new_'; + const advancedKey = useMemo(() => sanitizeTeamName(teamName.trim()) || '_new_', [teamName]); const [worktreeEnabled, setWorktreeEnabledRaw] = useState(false); const [worktreeName, setWorktreeNameRaw] = useState(''); const [customArgs, setCustomArgsRaw] = useState(''); @@ -454,38 +486,44 @@ export const CreateTeamDialog = ({ setCustomArgsRaw(localStorage.getItem(`team:lastCustomArgs:${advancedKey}`) ?? ''); }, [advancedKey]); - const setSelectedModel = (value: string): void => { - const normalizedValue = normalizeExplicitTeamModelForUi(selectedProviderId, value); - setSelectedModelRaw(normalizedValue); - setStoredCreateTeamModel(selectedProviderId, normalizedValue); - }; + const setSelectedModel = useCallback( + (value: string): void => { + const normalizedValue = normalizeExplicitTeamModelForUi(selectedProviderId, value); + setSelectedModelRaw(normalizedValue); + setStoredCreateTeamModel(selectedProviderId, normalizedValue); + }, + [selectedProviderId] + ); - const setSelectedProviderId = (value: TeamProviderId): void => { - const normalizedValue = normalizeLeadProviderForMode(value, multimodelEnabled); - setSelectedProviderIdRaw(normalizedValue); - setStoredCreateTeamProvider(normalizedValue); - setSelectedModelRaw(getStoredTeamModel(normalizedValue)); - }; + const setSelectedProviderId = useCallback( + (value: TeamProviderId): void => { + const normalizedValue = normalizeLeadProviderForMode(value, multimodelEnabled); + setSelectedProviderIdRaw(normalizedValue); + setStoredCreateTeamProvider(normalizedValue); + setSelectedModelRaw(getStoredTeamModel(normalizedValue)); + }, + [multimodelEnabled] + ); - const setLimitContext = (value: boolean): void => { + const setLimitContext = useCallback((value: boolean): void => { setLimitContextRaw(value); setStoredCreateTeamLimitContext(value); - }; + }, []); - const setSkipPermissions = (value: boolean): void => { + const setSkipPermissions = useCallback((value: boolean): void => { setSkipPermissionsRaw(value); setStoredCreateTeamSkipPermissions(value); - }; + }, []); - const setSelectedEffort = (value: string): void => { + const setSelectedEffort = useCallback((value: string): void => { setSelectedEffortRaw(value); setStoredCreateTeamEffort(value); - }; + }, []); - const setSelectedFastMode = (value: TeamFastMode): void => { + const setSelectedFastMode = useCallback((value: TeamFastMode): void => { setSelectedFastModeRaw(value); setStoredCreateTeamFastMode(value); - }; + }, []); const setWorktreeEnabled = (value: boolean): void => { setWorktreeEnabledRaw(value); @@ -541,7 +579,10 @@ export const CreateTeamDialog = ({ () => [...new Set([...existingTeamNames, ...provisioningTeamNames])], [existingTeamNames, provisioningTeamNames] ); - const suggestedTeamName = getNextSuggestedTeamName(allTakenTeamNames); + const suggestedTeamName = useMemo( + () => getNextSuggestedTeamName(allTakenTeamNames), + [allTakenTeamNames] + ); // Clear stale provisioning error when dialog opens useEffect(() => { @@ -717,7 +758,6 @@ export const CreateTeamDialog = ({ }, [ effectiveAnthropicRuntimeLimitContext, effectiveCwd, - prepareChecks, prepareRuntimeStatusSignature, runtimeBackendSummaryByProvider, selectedMemberProviders, @@ -802,176 +842,197 @@ export const CreateTeamDialog = ({ setPrepareWarnings([]); setPrepareChecks(initialChecks); - void (async () => { - await Promise.resolve(); - let checks = initialChecks; - const providerPlans = selectedMemberProviders.map((providerId) => { - const selectedModelChecks = (() => { - const next = new Set(); - let hasDefaultSelection = false; - const supportsProviderDefaultCheck = - providerId === 'codex' || - providerId === 'gemini' || - (providerId === 'anthropic' && selectedProviderId === 'anthropic'); - const leadModel = computeEffectiveTeamModel( - selectedModel, - effectiveAnthropicRuntimeLimitContext, - selectedProviderId - ); - if (selectedProviderId === providerId && selectedModel.trim()) { - if (leadModel?.trim()) { - next.add(leadModel.trim()); - } - } else if (selectedProviderId === providerId && supportsProviderDefaultCheck) { - hasDefaultSelection = true; - } - for (const member of effectiveMemberDrafts) { - if (member.removedAt) { - continue; - } - const scopedModel = resolveProviderScopedMemberModel({ - memberProviderId: member.providerId, - memberModel: member.model, - selectedProviderId, - runtimeProviderStatusById, - }); - if (scopedModel.providerId !== providerId) { - continue; - } - if (scopedModel.model) { - next.add(scopedModel.model); - } else if (supportsProviderDefaultCheck) { + // Defer the heavy IPC orchestration until the renderer is idle so the + // synchronous state updates above (setPrepareState etc.) can paint first. + // Cancel any pending idle work from a superseded run so a stale callback + // can't start expensive diagnostics for an obsolete request. + cancelScheduledIdle(prepareIdleHandleRef.current); + prepareIdleHandleRef.current = null; + + prepareIdleHandleRef.current = scheduleIdle(() => { + prepareIdleHandleRef.current = null; + if (prepareRequestSeqRef.current !== requestSeq) return; + void (async () => { + let checks = initialChecks; + const providerPlans = selectedMemberProviders.map((providerId) => { + const selectedModelChecks = (() => { + const next = new Set(); + let hasDefaultSelection = false; + const supportsProviderDefaultCheck = + providerId === 'codex' || + providerId === 'gemini' || + (providerId === 'anthropic' && selectedProviderId === 'anthropic'); + const leadModel = computeEffectiveTeamModel( + selectedModel, + effectiveAnthropicRuntimeLimitContext, + selectedProviderId + ); + if (selectedProviderId === providerId && selectedModel.trim()) { + if (leadModel?.trim()) { + next.add(leadModel.trim()); + } + } else if (selectedProviderId === providerId && supportsProviderDefaultCheck) { hasDefaultSelection = true; } - } - if (supportsProviderDefaultCheck && hasDefaultSelection) { - next.add(DEFAULT_PROVIDER_MODEL_SELECTION); - } - return Array.from(next); - })(); - const backendSummary = runtimeBackendSummaryByProviderRef.current.get(providerId) ?? null; - const cacheKey = buildProviderPrepareModelCacheKey({ - cwd: effectiveCwd, - providerId, - backendSummary, - limitContext: effectiveAnthropicRuntimeLimitContext, - runtimeStatusSignature: prepareRuntimeStatusSignature, - }); - const cachedModelResultsById = { - ...getShortLivedProviderPrepareModelResults({ + for (const member of effectiveMemberDrafts) { + if (member.removedAt) { + continue; + } + const scopedModel = resolveProviderScopedMemberModel({ + memberProviderId: member.providerId, + memberModel: member.model, + selectedProviderId, + runtimeProviderStatusById, + }); + if (scopedModel.providerId !== providerId) { + continue; + } + if (scopedModel.model) { + next.add(scopedModel.model); + } else if (supportsProviderDefaultCheck) { + hasDefaultSelection = true; + } + } + if (supportsProviderDefaultCheck && hasDefaultSelection) { + next.add(DEFAULT_PROVIDER_MODEL_SELECTION); + } + return Array.from(next); + })(); + const backendSummary = runtimeBackendSummaryByProviderRef.current.get(providerId) ?? null; + const cacheKey = buildProviderPrepareModelCacheKey({ + cwd: effectiveCwd, providerId, - cacheKey, - }), - ...(prepareModelResultsCacheRef.current.get(cacheKey) ?? {}), - }; - const cachedSnapshot = getProviderPrepareCachedSnapshot({ - providerId, - selectedModelIds: selectedModelChecks, - cachedModelResultsById, - }); - return { - providerId, - selectedModelChecks, - backendSummary, - cacheKey, - cachedModelResultsById, - cachedSnapshot, - }; - }); - - try { - for (const plan of providerPlans) { - checks = updateProviderCheck(checks, plan.providerId, { - status: plan.selectedModelChecks.length > 0 ? plan.cachedSnapshot.status : 'checking', - backendSummary: plan.backendSummary, - details: plan.cachedSnapshot.details, + backendSummary, + limitContext: effectiveAnthropicRuntimeLimitContext, + runtimeStatusSignature: prepareRuntimeStatusSignature, }); - } - if (prepareRequestSeqRef.current === requestSeq) { - setPrepareChecks(checks); - } - const providerResults = await Promise.all( - providerPlans.map(async (plan) => { - const prepResult = await runProviderPrepareDiagnostics({ - cwd: effectiveCwd, - providerId: plan.providerId, - selectedModelIds: plan.selectedModelChecks, - prepareProvisioning: api.teams.prepareProvisioning, - limitContext: effectiveAnthropicRuntimeLimitContext, - cachedModelResultsById: plan.cachedModelResultsById, - onModelProgress: ({ status, details }) => { - checks = updateProviderCheck(checks, plan.providerId, { - status, - backendSummary: plan.backendSummary, - details, - }); - if (prepareRequestSeqRef.current === requestSeq) { - setPrepareChecks(checks); - } - }, + const cachedModelResultsById = { + ...getShortLivedProviderPrepareModelResults({ + providerId, + cacheKey, + }), + ...(prepareModelResultsCacheRef.current.get(cacheKey) ?? {}), + }; + const cachedSnapshot = getProviderPrepareCachedSnapshot({ + providerId, + selectedModelIds: selectedModelChecks, + cachedModelResultsById, + }); + return { + providerId, + selectedModelChecks, + backendSummary, + cacheKey, + cachedModelResultsById, + cachedSnapshot, + }; + }); + + try { + for (const plan of providerPlans) { + checks = updateProviderCheck(checks, plan.providerId, { + status: plan.selectedModelChecks.length > 0 ? plan.cachedSnapshot.status : 'checking', + backendSummary: plan.backendSummary, + details: plan.cachedSnapshot.details, }); - return { ...plan, prepResult }; - }) - ); - let anyFailure = false; - let anyNotes = false; - const collectedWarnings: string[] = []; - for (const plan of providerResults) { - if (plan.prepResult.warnings.length > 0) { - anyNotes = true; - collectedWarnings.push( - ...plan.prepResult.warnings.map( - (warning) => `${getProviderLabel(plan.providerId)}: ${warning}` - ) - ); - } - if (plan.prepResult.status === 'failed') { - anyFailure = true; - } else if (plan.prepResult.status === 'notes') { - anyNotes = true; } if (prepareRequestSeqRef.current === requestSeq) { - const reusableModelResults = buildReusableProviderPrepareModelResults( - plan.prepResult.modelResultsById - ); - prepareModelResultsCacheRef.current.set(plan.cacheKey, reusableModelResults); - storeShortLivedProviderPrepareModelResults({ - providerId: plan.providerId, - cacheKey: plan.cacheKey, - modelResultsById: plan.prepResult.modelResultsById, + setPrepareChecks(checks); + } + const providerResults = await Promise.all( + providerPlans.map(async (plan) => { + const prepResult = await runProviderPrepareDiagnostics({ + cwd: effectiveCwd, + providerId: plan.providerId, + selectedModelIds: plan.selectedModelChecks, + prepareProvisioning: api.teams.prepareProvisioning, + limitContext: effectiveAnthropicRuntimeLimitContext, + cachedModelResultsById: plan.cachedModelResultsById, + onModelProgress: ({ status, details }) => { + checks = updateProviderCheck(checks, plan.providerId, { + status, + backendSummary: plan.backendSummary, + details, + }); + if (prepareRequestSeqRef.current === requestSeq) { + setPrepareChecks(checks); + } + }, + }); + return { ...plan, prepResult }; + }) + ); + let anyFailure = false; + let anyNotes = false; + const collectedWarnings: string[] = []; + for (const plan of providerResults) { + if (plan.prepResult.warnings.length > 0) { + anyNotes = true; + collectedWarnings.push( + ...plan.prepResult.warnings.map( + (warning) => `${getProviderLabel(plan.providerId)}: ${warning}` + ) + ); + } + if (plan.prepResult.status === 'failed') { + anyFailure = true; + } else if (plan.prepResult.status === 'notes') { + anyNotes = true; + } + if (prepareRequestSeqRef.current === requestSeq) { + const reusableModelResults = buildReusableProviderPrepareModelResults( + plan.prepResult.modelResultsById + ); + prepareModelResultsCacheRef.current.set(plan.cacheKey, reusableModelResults); + storeShortLivedProviderPrepareModelResults({ + providerId: plan.providerId, + cacheKey: plan.cacheKey, + modelResultsById: plan.prepResult.modelResultsById, + }); + } + checks = updateProviderCheck(checks, plan.providerId, { + status: plan.prepResult.status, + backendSummary: plan.backendSummary, + details: plan.prepResult.details, }); } - checks = updateProviderCheck(checks, plan.providerId, { - status: plan.prepResult.status, - backendSummary: plan.backendSummary, - details: plan.prepResult.details, - }); + if (prepareRequestSeqRef.current === requestSeq) { + setPrepareChecks(checks); + } + if (prepareRequestSeqRef.current !== requestSeq) return; + const failureMessage = + getPrimaryProvisioningFailureDetail(checks) ?? + 'Some selected providers need attention.'; + setPrepareState(anyFailure ? 'failed' : 'ready'); + setPrepareMessage( + anyFailure + ? failureMessage + : anyNotes + ? 'Selected providers are ready with notes.' + : 'Selected providers are ready.' + ); + setPrepareWarnings(collectedWarnings); + } catch (error) { + if (prepareRequestSeqRef.current !== requestSeq) return; + const failureMessage = + error instanceof Error ? error.message : 'Failed to warm up Claude CLI environment'; + setPrepareState('failed'); + setPrepareWarnings([]); + setPrepareChecks(failIncompleteProviderChecks(checks, failureMessage)); + setPrepareMessage(failureMessage); } - if (prepareRequestSeqRef.current === requestSeq) { - setPrepareChecks(checks); - } - if (prepareRequestSeqRef.current !== requestSeq) return; - const failureMessage = - getPrimaryProvisioningFailureDetail(checks) ?? 'Some selected providers need attention.'; - setPrepareState(anyFailure ? 'failed' : 'ready'); - setPrepareMessage( - anyFailure - ? failureMessage - : anyNotes - ? 'Selected providers are ready with notes.' - : 'Selected providers are ready.' - ); - setPrepareWarnings(collectedWarnings); - } catch (error) { - if (prepareRequestSeqRef.current !== requestSeq) return; - const failureMessage = - error instanceof Error ? error.message : 'Failed to warm up Claude CLI environment'; - setPrepareState('failed'); - setPrepareWarnings([]); - setPrepareChecks(failIncompleteProviderChecks(checks, failureMessage)); - setPrepareMessage(failureMessage); + })(); + }); + + return () => { + cancelScheduledIdle(prepareIdleHandleRef.current); + prepareIdleHandleRef.current = null; + // Bump the request sequence so any callback that already woke up but + // hasn't checked yet treats itself as superseded. + if (prepareRequestSeqRef.current === requestSeq) { + prepareRequestSeqRef.current += 1; } - })(); + }; }, [ open, canCreate, @@ -1747,6 +1808,66 @@ export const CreateTeamDialog = ({ }); }; + const rosterHeaderTop = useMemo( + () => ( +
+ setSoloTeam(checked === true)} + /> + +
+ ), + [setSoloTeam, soloTeam] + ); + + const rosterHeaderBottom = useMemo( + () => + teammateRuntimeCompatibility.visible || + soloTeam || + (canCreate && hasSelectedWorktreeIsolation) ? ( +
+ { + onClose(); + openDashboard(); + }} + /> + {soloTeam ? ( +
+ +

+ Only the team lead (main process) will be started — no teammates will be + spawned. Works like a regular agent session in your chosen runtime (Claude Code, + Codex, OpenCode, Gemini) but with access to the task board for planning. Saves + tokens by avoiding teammate coordination overhead. You can add members later from + the team settings. +

+
+ ) : null} + {canCreate && hasSelectedWorktreeIsolation ? ( + + ) : null} +
+ ) : null, + [ + canCreate, + hasSelectedWorktreeIsolation, + onClose, + openDashboard, + soloTeam, + teammateRuntimeCompatibility, + worktreeGitReadiness, + ] + ); + return ( - setSoloTeam(checked === true)} - /> - - - } - headerBottom={ - teammateRuntimeCompatibility.visible || - soloTeam || - (canCreate && hasSelectedWorktreeIsolation) ? ( -
- { - onClose(); - openDashboard(); - }} - /> - {soloTeam ? ( -
- -

- Only the team lead (main process) will be started — no teammates - will be spawned. Works like a regular agent session in your chosen runtime - (Claude Code, Codex, OpenCode, Gemini) but with access to the task board - for planning. Saves tokens by avoiding teammate coordination overhead. You - can add members later from the team settings. -

-
- ) : null} - {canCreate && hasSelectedWorktreeIsolation ? ( - - ) : null} -
- ) : null - } + headerTop={rosterHeaderTop} + headerBottom={rosterHeaderBottom} /> diff --git a/src/renderer/components/team/members/TeamRosterEditorSection.tsx b/src/renderer/components/team/members/TeamRosterEditorSection.tsx index 5a146dec..e59c9409 100644 --- a/src/renderer/components/team/members/TeamRosterEditorSection.tsx +++ b/src/renderer/components/team/members/TeamRosterEditorSection.tsx @@ -61,7 +61,7 @@ interface TeamRosterEditorSectionProps { onTeammateWorktreeDefaultChange?: (enabled: boolean) => void; } -export const TeamRosterEditorSection = ({ +const TeamRosterEditorSectionImpl = ({ members, onMembersChange, fieldError, @@ -197,3 +197,6 @@ export const TeamRosterEditorSection = ({ /> ); }; + +export const TeamRosterEditorSection = React.memo(TeamRosterEditorSectionImpl); +TeamRosterEditorSection.displayName = 'TeamRosterEditorSection'; diff --git a/src/renderer/store/slices/__tests__/cliInstallerSlice.test.ts b/src/renderer/store/slices/__tests__/cliInstallerSlice.test.ts new file mode 100644 index 00000000..078d34b3 --- /dev/null +++ b/src/renderer/store/slices/__tests__/cliInstallerSlice.test.ts @@ -0,0 +1,164 @@ +import { + createLoadingMultimodelCliStatus, + mergeCliStatusPreservingHydratedProviders, +} from '@renderer/store/slices/cliInstallerSlice'; +import { describe, expect, it } from 'vitest'; + +import type { CliProviderReasoningEffort } from '@shared/types/cliInstaller'; + +describe('mergeCliStatusPreservingHydratedProviders', () => { + it('returns the previous status reference when a structurally identical clone arrives', () => { + // This mirrors the real IPC path: `CliInstallerService.cloneCliInstallationStatus()` + // (called from `publishStatusSnapshot()`) hands the renderer a fresh + // `CliInstallationStatus` whose `providers` are also freshly-cloned + // objects, even when nothing has actually changed. The merge function + // must compare provider content (not just reference) so that no-op + // progress ticks do not produce a new `cliStatus` identity and trigger + // a re-render storm across every consumer. + const current = createLoadingMultimodelCliStatus(); + const incoming = structuredClone(current); + + const merged = mergeCliStatusPreservingHydratedProviders(current, incoming); + + expect(merged).toBe(current); + }); + + it('returns the previous status reference when an authenticated clone arrives', () => { + const base = createLoadingMultimodelCliStatus(); + const current = { + ...base, + authLoggedIn: true, + authStatusChecking: false, + authMethod: 'oauth' as const, + providers: base.providers.map((provider, index) => + index === 0 + ? { + ...provider, + authenticated: true, + authMethod: 'oauth' as const, + supported: true, + verificationState: 'verified' as const, + statusMessage: null, + models: ['model-a', 'model-b'], + } + : provider + ), + }; + const incoming = structuredClone(current); + + const merged = mergeCliStatusPreservingHydratedProviders(current, incoming); + + expect(merged).toBe(current); + }); + + it('returns a new status when an incoming provider field actually differs', () => { + const current = createLoadingMultimodelCliStatus(); + const incoming = structuredClone(current); + incoming.providers[0] = { + ...incoming.providers[0], + statusMessage: 'Verifying credentials...', + }; + + const merged = mergeCliStatusPreservingHydratedProviders(current, incoming); + + expect(merged).not.toBe(current); + expect(merged.providers[0].statusMessage).toBe('Verifying credentials...'); + }); + + it('returns current when a structurally identical populated provider clone arrives', () => { + // Mirrors the real IPC flow with a fully-populated provider: ChatGPT-Codex + // authenticated, with a model catalog, model availability records, + // runtime capabilities, available backends, and a selected backend. + // None of these fields are reference-stable across IPC clones, so the + // equality guard must compare them by content, not reference. + const base = createLoadingMultimodelCliStatus(); + const populatedProvider = { + ...base.providers[1], + authenticated: true, + authMethod: 'codex_chatgpt' as const, + supported: true, + verificationState: 'verified' as const, + statusMessage: null, + models: ['gpt-5.2'], + modelAvailability: [ + { + modelId: 'gpt-5.2', + status: 'available' as const, + checkedAt: '2026-05-14T00:00:00.000Z', + }, + ], + runtimeCapabilities: { + reasoningEffort: { + supported: true, + values: ['low', 'medium', 'high'] as CliProviderReasoningEffort[], + }, + }, + availableBackends: [ + { + id: 'codex-native', + label: 'Codex native', + description: 'App-managed Codex runtime', + selectable: true, + recommended: true, + available: true, + }, + ], + backend: { kind: 'codex-cli' as const, label: 'Codex CLI' }, + }; + const current = { + ...base, + authLoggedIn: true, + authStatusChecking: false, + authMethod: 'codex_chatgpt' as const, + providers: base.providers.map((provider, index) => + index === 1 ? populatedProvider : provider + ), + }; + const incoming = structuredClone(current); + + const merged = mergeCliStatusPreservingHydratedProviders(current, incoming); + + expect(merged).toBe(current); + expect(merged.providers[1]).toBe(current.providers[1]); + }); + + it('produces a new status when a cloned populated field actually changed', () => { + // Negative companion to the populated-clone test: confirms that when a + // cloned DTO field really differs, the merge does NOT preserve the + // previous reference (i.e. we never let stale data through). + const base = createLoadingMultimodelCliStatus(); + const populatedProvider = { + ...base.providers[1], + authenticated: true, + authMethod: 'codex_chatgpt' as const, + supported: true, + verificationState: 'verified' as const, + models: ['gpt-5.2'], + availableBackends: [ + { + id: 'codex-native', + label: 'Codex native', + description: 'App-managed Codex runtime', + selectable: true, + recommended: true, + available: true, + }, + ], + }; + const current = { + ...base, + providers: base.providers.map((provider, index) => + index === 1 ? populatedProvider : provider + ), + }; + const incoming = structuredClone(current); + // Flip a nested DTO field on the cloned snapshot. + incoming.providers[1].availableBackends![0].available = false; + + const merged = mergeCliStatusPreservingHydratedProviders(current, incoming); + + expect(merged).not.toBe(current); + expect(merged.providers[1]).not.toBe(current.providers[1]); + expect(merged.providers[1].availableBackends?.[0].available).toBe(false); + }); +}); diff --git a/src/renderer/store/slices/cliInstallerSlice.ts b/src/renderer/store/slices/cliInstallerSlice.ts index 364d92c5..d8213c10 100644 --- a/src/renderer/store/slices/cliInstallerSlice.ts +++ b/src/renderer/store/slices/cliInstallerSlice.ts @@ -214,6 +214,142 @@ export function reconcileMultimodelProviderLoading( ); } +function areArraysEqual( + a: readonly T[], + b: readonly T[], + isEqual: (left: T, right: T) => boolean +): boolean { + if (a === b) return true; + if (a.length !== b.length) return false; + for (let i = 0; i < a.length; i += 1) { + if (!isEqual(a[i], b[i])) return false; + } + return true; +} + +/** + * Content-level equality for cloned IPC DTO values. The provider snapshot is + * serialised by `CliInstallerService.cloneCliInstallationStatus()` and + * `publishStatusSnapshot()` before reaching the renderer, so every nested + * array/object arrives as a fresh reference even when nothing changed. These + * values are plain JSON-serialisable DTOs, so a stringify-based comparator is + * acceptable: false negatives are fine (we just produce a new merged status + * unnecessarily), but false positives are not (we must never preserve stale + * data). + */ +function areDtoValuesEqual(a: T | null | undefined, b: T | null | undefined): boolean { + if (a === b) return true; + if (a == null || b == null) return a == null && b == null; + return JSON.stringify(a) === JSON.stringify(b); +} + +function areExtensionCapabilitiesEqual( + a: CliProviderStatus['capabilities']['extensions']['plugins'], + b: CliProviderStatus['capabilities']['extensions']['plugins'] +): boolean { + if (a === b) return true; + return ( + a.status === b.status && + a.ownership === b.ownership && + (a.reason ?? null) === (b.reason ?? null) + ); +} + +function areProviderCapabilitiesEqual( + a: CliProviderStatus['capabilities'], + b: CliProviderStatus['capabilities'] +): boolean { + if (a === b) return true; + return ( + a.teamLaunch === b.teamLaunch && + a.oneShot === b.oneShot && + areExtensionCapabilitiesEqual(a.extensions.plugins, b.extensions.plugins) && + areExtensionCapabilitiesEqual(a.extensions.mcp, b.extensions.mcp) && + areExtensionCapabilitiesEqual(a.extensions.skills, b.extensions.skills) && + areExtensionCapabilitiesEqual(a.extensions.apiKeys, b.extensions.apiKeys) + ); +} + +function areProviderBackendsEqual( + a: CliProviderStatus['backend'], + b: CliProviderStatus['backend'] +): boolean { + if (a === b) return true; + if (a == null || b == null) return false; + return ( + a.kind === b.kind && + a.label === b.label && + (a.endpointLabel ?? null) === (b.endpointLabel ?? null) && + (a.projectId ?? null) === (b.projectId ?? null) && + (a.authMethodDetail ?? null) === (b.authMethodDetail ?? null) + ); +} + +/** + * Content-level equality check for a single CliProviderStatus. + * + * Compares all scalar fields explicitly, the well-typed nested structures + * (capabilities, backend) via dedicated comparators, and the cloned DTO + * fields (modelCatalog, modelAvailability, runtimeCapabilities, + * subscriptionRateLimits, connection, availableBackends, + * externalRuntimeDiagnostics) by content. This is necessary because the + * IPC path (`CliInstallerService.cloneCliInstallationStatus()` then + * `publishStatusSnapshot()`) hands the renderer freshly-deserialised + * provider objects on every tick — reference equality on those nested + * fields would never hold even when the snapshot is structurally + * identical. + */ +function areProviderStatusContentEqual(a: CliProviderStatus, b: CliProviderStatus): boolean { + if (a === b) return true; + return ( + a.providerId === b.providerId && + a.displayName === b.displayName && + a.supported === b.supported && + a.authenticated === b.authenticated && + a.authMethod === b.authMethod && + a.verificationState === b.verificationState && + (a.modelVerificationState ?? null) === (b.modelVerificationState ?? null) && + (a.statusMessage ?? null) === (b.statusMessage ?? null) && + (a.detailMessage ?? null) === (b.detailMessage ?? null) && + a.canLoginFromUi === b.canLoginFromUi && + (a.selectedBackendId ?? null) === (b.selectedBackendId ?? null) && + (a.resolvedBackendId ?? null) === (b.resolvedBackendId ?? null) && + areArraysEqual(a.models, b.models, (left, right) => left === right) && + areProviderCapabilitiesEqual(a.capabilities, b.capabilities) && + areProviderBackendsEqual(a.backend ?? null, b.backend ?? null) && + areDtoValuesEqual(a.modelCatalog ?? null, b.modelCatalog ?? null) && + areDtoValuesEqual(a.modelAvailability ?? [], b.modelAvailability ?? []) && + areDtoValuesEqual(a.runtimeCapabilities ?? null, b.runtimeCapabilities ?? null) && + areDtoValuesEqual(a.subscriptionRateLimits ?? null, b.subscriptionRateLimits ?? null) && + areDtoValuesEqual(a.connection ?? null, b.connection ?? null) && + areDtoValuesEqual(a.availableBackends ?? [], b.availableBackends ?? []) && + areDtoValuesEqual(a.externalRuntimeDiagnostics ?? [], b.externalRuntimeDiagnostics ?? []) + ); +} + +function isCliInstallationStatusContentEqual( + a: CliInstallationStatus, + b: CliInstallationStatus +): boolean { + return ( + a.flavor === b.flavor && + a.displayName === b.displayName && + a.supportsSelfUpdate === b.supportsSelfUpdate && + a.showVersionDetails === b.showVersionDetails && + a.showBinaryPath === b.showBinaryPath && + a.installed === b.installed && + a.installedVersion === b.installedVersion && + a.binaryPath === b.binaryPath && + (a.launchError ?? null) === (b.launchError ?? null) && + a.latestVersion === b.latestVersion && + a.updateAvailable === b.updateAvailable && + a.authLoggedIn === b.authLoggedIn && + a.authStatusChecking === b.authStatusChecking && + a.authMethod === b.authMethod && + areArraysEqual(a.providers, b.providers, Object.is) + ); +} + export function mergeCliStatusPreservingHydratedProviders( current: CliInstallationStatus | null, incoming: CliInstallationStatus @@ -222,6 +358,9 @@ export function mergeCliStatusPreservingHydratedProviders( current?.flavor !== 'agent_teams_orchestrator' || incoming.flavor !== 'agent_teams_orchestrator' ) { + if (current && isCliInstallationStatusContentEqual(current, incoming)) { + return current; + } return incoming; } @@ -231,7 +370,15 @@ export function mergeCliStatusPreservingHydratedProviders( const incomingProviderIds = new Set(incoming.providers.map((provider) => provider.providerId)); const providers = incoming.providers.map((incomingProvider) => { const currentProvider = currentProvidersById.get(incomingProvider.providerId); - if (currentProvider && shouldPreserveCurrentProviderStatus(currentProvider, incomingProvider)) { + if (!currentProvider) { + return incomingProvider; + } + if (shouldPreserveCurrentProviderStatus(currentProvider, incomingProvider)) { + return currentProvider; + } + // Preserve the current reference when content is identical so the + // providers array stays reference-stable across steady-state IPC polls. + if (areProviderStatusContentEqual(currentProvider, incomingProvider)) { return currentProvider; } return incomingProvider; @@ -248,12 +395,22 @@ export function mergeCliStatusPreservingHydratedProviders( const authenticatedProvider = providers.find((provider) => provider.authenticated) ?? null; - return { + const mergedProviders = areArraysEqual(providers, current.providers, Object.is) + ? current.providers + : providers; + + const merged: CliInstallationStatus = { ...incoming, - providers, - authLoggedIn: providers.some((provider) => provider.authenticated), + providers: mergedProviders, + authLoggedIn: mergedProviders.some((provider) => provider.authenticated), authMethod: authenticatedProvider?.authMethod ?? null, }; + + if (isCliInstallationStatusContentEqual(current, merged)) { + return current; + } + + return merged; } export async function refreshOpenCodeProviderStatusAfterRuntimeInstall(