From ec1a01a4bf13df9b28eb99b9e9818a9fc2881dce Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Sat, 10 Oct 2026 08:07:51 -0700 Subject: [PATCH] fix(accounts): a server's late account result stays with that server (#27262) * fix(accounts): a server's late account result stays with that server The Accounts pane kept one roster across default-runtime changes, so a select or remove still in flight on the previous server overwrote the new owner's roster when it returned. Key the pane by its account owner so rosters, account ids and in-flight results are scoped to one owner. Co-Authored-By: Claude * fix(accounts): drop a stale owner's result instead of remounting the pane Remounting lost an in-progress local login's spinner and Cancel button and hid an account added after returning to the same owner. Guard each action's result by the owner it was sent to instead. Co-Authored-By: Claude * fix(accounts): keep the stale-owner check on the roster write only The toast, settings refresh and restart prompt describe what happened on the owner the action went to; only the roster belongs to the shown owner. Co-Authored-By: Claude --------- Co-authored-by: m4air Co-authored-by: Claude --- .../AccountsPane.owner-scope.test.tsx | 156 ++++++++++++++++++ .../AccountsPane.section-lifetime.test.tsx | 2 + .../src/components/settings/AccountsPane.tsx | 11 ++ .../settings/accounts-pane-account-actions.ts | 17 +- .../runtime-provider-accounts-client.ts | 2 +- 5 files changed, 184 insertions(+), 4 deletions(-) create mode 100644 src/renderer/src/components/settings/AccountsPane.owner-scope.test.tsx diff --git a/src/renderer/src/components/settings/AccountsPane.owner-scope.test.tsx b/src/renderer/src/components/settings/AccountsPane.owner-scope.test.tsx new file mode 100644 index 00000000000..14ed3e79009 --- /dev/null +++ b/src/renderer/src/components/settings/AccountsPane.owner-scope.test.tsx @@ -0,0 +1,156 @@ +// @vitest-environment happy-dom +import { act, cleanup, fireEvent, render, screen, within } from '@testing-library/react' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' +import { getDefaultSettings } from '../../../../shared/constants' +import type { + ClaudeManagedAccountSummary, + ClaudeRateLimitAccountsState +} from '../../../../shared/managed-account-types' +import { AccountsPane } from './AccountsPane' + +const fake = vi.hoisted(() => { + const state: { + pendingSelect: ((roster: unknown) => void) | null + write: ReturnType + rosters: Map + } = { pendingSelect: null, write: vi.fn(), rosters: new Map() } + return state +}) +vi.mock('@/i18n/i18n', () => ({ + i18n: { language: 'en' }, + translate: (_key: string, fallback: string, values?: Record) => + Object.entries(values ?? {}).reduce( + (text, [key, value]) => text.replaceAll(`{{${key}}}`, String(value)), + fallback + ) +})) +vi.mock('@/store', () => ({ + useAppStore: (selector: (state: Record) => unknown) => + selector({ + settingsSearchQuery: 'claude', + rateLimits: { + codex: null, + codexTarget: { runtime: 'host', wslDistro: null }, + minimax: null, + cursor: { updatedAt: 0 }, + grok: { updatedAt: 0 } + }, + runtimeEnvironments: [], + recordFeatureInteraction: fake.write, + fetchSettings: fake.write + }) +})) +vi.mock('@/runtime/runtime-provider-accounts-client', () => { + const empty = () => ({ + accounts: [], + activeAccountId: null, + activeAccountIdsByRuntime: { host: null, wsl: {} } + }) + return { + emptyClaudeAccountsState: empty, + emptyCodexAccountsState: empty, + getProviderAccountsOwnerKey: (settings: { activeRuntimeEnvironmentId?: string | null }) => + settings.activeRuntimeEnvironmentId ?? 'local', + hasRemoteProviderAccountOwner: (settings: { activeRuntimeEnvironmentId?: string | null }) => + Boolean(settings.activeRuntimeEnvironmentId), + watchProviderAccounts: ( + settings: { activeRuntimeEnvironmentId?: string | null }, + handlers: { onSnapshot: (snapshot: unknown) => void } + ) => { + handlers.onSnapshot({ + claude: fake.rosters.get(settings.activeRuntimeEnvironmentId ?? 'local'), + codex: empty(), + rateLimits: null + }) + return { close: vi.fn() } + }, + selectClaudeProviderAccount: () => + new Promise((resolve) => { + fake.pendingSelect = resolve + }), + selectCodexProviderAccount: fake.write, + removeClaudeProviderAccount: fake.write, + removeCodexProviderAccount: fake.write + } +}) + +function claudeRoster(id: string, email: string, active: boolean): ClaudeRateLimitAccountsState { + const account: ClaudeManagedAccountSummary = { + id, + email, + authMethod: 'subscription-oauth', + createdAt: 1, + updatedAt: 1, + lastAuthenticatedAt: 1 + } + const activeId = active ? id : null + return { + accounts: [account], + activeAccountId: activeId, + activeAccountIdsByRuntime: { host: activeId, wsl: {} } + } +} + +beforeEach(() => { + fake.pendingSelect = null + fake.rosters.set('server-a', claudeRoster('acct-a', 'alpha@server-a.test', false)) + fake.rosters.set('server-b', claudeRoster('acct-b', 'beta@server-b.test', false)) + Object.assign(window, { + api: { + minimaxCredentials: { + getStatus: vi.fn(async () => ({ cookieConfigured: false, apiKeyConfigured: false })) + } + } + }) +}) +afterEach(() => { + cleanup() + Reflect.deleteProperty(window, 'api') +}) + +it("drops a server's late account result after the default runtime moves to another server", async () => { + const settingsFor = (environmentId: string) => ({ + ...getDefaultSettings('/synthetic'), + activeRuntimeEnvironmentId: environmentId + }) + const view = render() + await act(async () => {}) + fireEvent.click(screen.getByText('alpha@server-a.test')) + expect(fake.pendingSelect).not.toBeNull() + + await act(async () => + view.rerender() + ) + expect(screen.getByText('beta@server-b.test')).toBeTruthy() + + await act(async () => fake.pendingSelect?.(claudeRoster('acct-a', 'alpha@server-a.test', true))) + + // Server A's roster must not replace server B's in the pane now scoped to B. + expect(screen.queryByText('alpha@server-a.test')).toBeNull() + expect(screen.getByText('beta@server-b.test')).toBeTruthy() +}) + +it('keeps a late account result once the pane is back on the server it was sent to', async () => { + const settingsFor = (environmentId: string) => ({ + ...getDefaultSettings('/synthetic'), + activeRuntimeEnvironmentId: environmentId + }) + const view = render() + await act(async () => {}) + fireEvent.click(screen.getByText('alpha@server-a.test')) + for (const environmentId of ['server-b', 'server-a']) { + await act(async () => + view.rerender() + ) + } + const alphaRow = (): HTMLElement => { + const row = screen.getByText('alpha@server-a.test').closest('button') + expect(row).not.toBeNull() + return row ?? document.body + } + expect(within(alphaRow()).queryByText('Active')).toBeNull() + + await act(async () => fake.pendingSelect?.(claudeRoster('acct-a', 'alpha@server-a.test', true))) + + expect(within(alphaRow()).getByText('Active')).toBeTruthy() +}) diff --git a/src/renderer/src/components/settings/AccountsPane.section-lifetime.test.tsx b/src/renderer/src/components/settings/AccountsPane.section-lifetime.test.tsx index 4f225e89ba9..66f1ab23f13 100644 --- a/src/renderer/src/components/settings/AccountsPane.section-lifetime.test.tsx +++ b/src/renderer/src/components/settings/AccountsPane.section-lifetime.test.tsx @@ -68,6 +68,8 @@ vi.mock('@/runtime/runtime-provider-accounts-client', () => ({ activeAccountId: null, activeAccountIdsByRuntime: { host: null, wsl: {} } }), + getProviderAccountsOwnerKey: (settings: { activeRuntimeEnvironmentId?: string | null }) => + settings.activeRuntimeEnvironmentId ?? 'local', hasRemoteProviderAccountOwner: (settings: { activeRuntimeEnvironmentId?: string }) => Boolean(settings.activeRuntimeEnvironmentId), watchProviderAccounts: fake.watcher, diff --git a/src/renderer/src/components/settings/AccountsPane.tsx b/src/renderer/src/components/settings/AccountsPane.tsx index 5bbf1b69d98..786324a708e 100644 --- a/src/renderer/src/components/settings/AccountsPane.tsx +++ b/src/renderer/src/components/settings/AccountsPane.tsx @@ -12,6 +12,7 @@ import { isWebClientLocation } from '@/lib/web-client-location' import { emptyClaudeAccountsState, emptyCodexAccountsState, + getProviderAccountsOwnerKey, hasRemoteProviderAccountOwner, watchProviderAccounts } from '@/runtime/runtime-provider-accounts-client' @@ -110,6 +111,14 @@ export function AccountsPane({ // (see #7973); every list/select/remove below must scope to it, not host/WSL. const isRemoteAccountScope = hasRemoteProviderAccountOwner(settings) const activeRuntimeEnvironmentId = settings.activeRuntimeEnvironmentId?.trim() || null + // Why: a select/remove can outlive a default-runtime change; its result + // belongs to the owner it was sent to, not the one now shown. + const accountOwnerKey = getProviderAccountsOwnerKey(settings) + const currentAccountOwnerKeyRef = useRef(accountOwnerKey) + useEffect(() => { + currentAccountOwnerKeyRef.current = accountOwnerKey + }, [accountOwnerKey]) + const isCurrentAccountOwner = (): boolean => currentAccountOwnerKeyRef.current === accountOwnerKey // Why: keep the real name separate from the prose fallback below; the scope // label must not interpolate the fallback. const remoteServerName = isRemoteAccountScope @@ -301,6 +310,7 @@ export function AccountsPane({ const runCodexAccountAction = createCodexAccountActionRunner({ settings, + isCurrentAccountOwner, accountRuntime, isRemoteAccountScope, codexAccounts, @@ -312,6 +322,7 @@ export function AccountsPane({ }) const runClaudeAccountAction = createClaudeAccountActionRunner({ settings, + isCurrentAccountOwner, accountRuntime, isRemoteAccountScope, claudeAccounts, diff --git a/src/renderer/src/components/settings/accounts-pane-account-actions.ts b/src/renderer/src/components/settings/accounts-pane-account-actions.ts index 1f0099876d2..9ceff1fb36c 100644 --- a/src/renderer/src/components/settings/accounts-pane-account-actions.ts +++ b/src/renderer/src/components/settings/accounts-pane-account-actions.ts @@ -40,6 +40,8 @@ type CodexActionContext = { setCodexAction: Dispatch> fetchSettings: () => Promise recordFeatureInteraction: (featureId: FeatureInteractionId) => void + /** False once the pane shows a different account owner than this action targeted. */ + isCurrentAccountOwner: () => boolean } export function createCodexAccountActionRunner( @@ -49,6 +51,7 @@ export function createCodexAccountActionRunner( accountRuntime, codexAccounts, fetchSettings, + isCurrentAccountOwner, isRemoteAccountScope, recordFeatureInteraction, setCodexAccounts, @@ -56,8 +59,11 @@ export function createCodexAccountActionRunner( setCodexAction } = context const syncCodexAccounts = async (next: CodexRateLimitAccountsState): Promise => { - setCodexAccounts(next) - setCodexAccountsLoaded(true) + // Why: only the roster is the shown owner's; the toast and follow-up still describe this action. + if (isCurrentAccountOwner()) { + setCodexAccounts(next) + setCodexAccountsLoaded(true) + } // Why: remote mutations never change local GlobalSettings account fields. if (!isRemoteAccountScope) { await fetchSettings() @@ -147,6 +153,8 @@ type ClaudeActionContext = { setClaudeAction: Dispatch> fetchSettings: () => Promise recordFeatureInteraction: (featureId: FeatureInteractionId) => void + /** False once the pane shows a different account owner than this action targeted. */ + isCurrentAccountOwner: () => boolean } export function createClaudeAccountActionRunner( @@ -156,13 +164,16 @@ export function createClaudeAccountActionRunner( accountRuntime, claudeAccounts, fetchSettings, + isCurrentAccountOwner, isRemoteAccountScope, recordFeatureInteraction, setClaudeAccounts, setClaudeAction } = context const syncClaudeAccounts = async (next: ClaudeRateLimitAccountsState): Promise => { - setClaudeAccounts(next) + if (isCurrentAccountOwner()) { + setClaudeAccounts(next) + } if (!isRemoteAccountScope) { await fetchSettings() } diff --git a/src/renderer/src/runtime/runtime-provider-accounts-client.ts b/src/renderer/src/runtime/runtime-provider-accounts-client.ts index 2620a55cf2f..30beafcf794 100644 --- a/src/renderer/src/runtime/runtime-provider-accounts-client.ts +++ b/src/renderer/src/runtime/runtime-provider-accounts-client.ts @@ -35,7 +35,7 @@ const REMOTE_ACCOUNTS_FIRST_SNAPSHOT_TIMEOUT_MS = 15_000 const REMOTE_ACCOUNT_MUTATION_TIMEOUT_MS = 30_000 const pendingProviderAccountsSnapshots = new Map>() -function getProviderAccountsOwnerKey( +export function getProviderAccountsOwnerKey( settings: Pick | null | undefined ): string { const target = getActiveRuntimeTarget(settings)