mirror of
https://github.com/stablyai/orca.git
synced 2026-11-04 11:07:42 +00:00
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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: m4air <m4air@Mac.localdomain> Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -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<typeof vi.fn>
|
||||
rosters: Map<string, unknown>
|
||||
} = { pendingSelect: null, write: vi.fn(), rosters: new Map() }
|
||||
return state
|
||||
})
|
||||
vi.mock('@/i18n/i18n', () => ({
|
||||
i18n: { language: 'en' },
|
||||
translate: (_key: string, fallback: string, values?: Record<string, string | number>) =>
|
||||
Object.entries(values ?? {}).reduce(
|
||||
(text, [key, value]) => text.replaceAll(`{{${key}}}`, String(value)),
|
||||
fallback
|
||||
)
|
||||
}))
|
||||
vi.mock('@/store', () => ({
|
||||
useAppStore: (selector: (state: Record<string, unknown>) => 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(<AccountsPane settings={settingsFor('server-a')} updateSettings={vi.fn()} />)
|
||||
await act(async () => {})
|
||||
fireEvent.click(screen.getByText('alpha@server-a.test'))
|
||||
expect(fake.pendingSelect).not.toBeNull()
|
||||
|
||||
await act(async () =>
|
||||
view.rerender(<AccountsPane settings={settingsFor('server-b')} updateSettings={vi.fn()} />)
|
||||
)
|
||||
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(<AccountsPane settings={settingsFor('server-a')} updateSettings={vi.fn()} />)
|
||||
await act(async () => {})
|
||||
fireEvent.click(screen.getByText('alpha@server-a.test'))
|
||||
for (const environmentId of ['server-b', 'server-a']) {
|
||||
await act(async () =>
|
||||
view.rerender(<AccountsPane settings={settingsFor(environmentId)} updateSettings={vi.fn()} />)
|
||||
)
|
||||
}
|
||||
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()
|
||||
})
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -40,6 +40,8 @@ type CodexActionContext = {
|
||||
setCodexAction: Dispatch<SetStateAction<CodexAccountAction>>
|
||||
fetchSettings: () => Promise<void>
|
||||
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<void> => {
|
||||
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<SetStateAction<ClaudeAccountAction>>
|
||||
fetchSettings: () => Promise<void>
|
||||
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<void> => {
|
||||
setClaudeAccounts(next)
|
||||
if (isCurrentAccountOwner()) {
|
||||
setClaudeAccounts(next)
|
||||
}
|
||||
if (!isRemoteAccountScope) {
|
||||
await fetchSettings()
|
||||
}
|
||||
|
||||
@@ -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<string, Promise<ProviderAccountsSnapshot>>()
|
||||
|
||||
function getProviderAccountsOwnerKey(
|
||||
export function getProviderAccountsOwnerKey(
|
||||
settings: Pick<GlobalSettings, 'activeRuntimeEnvironmentId'> | null | undefined
|
||||
): string {
|
||||
const target = getActiveRuntimeTarget(settings)
|
||||
|
||||
Reference in New Issue
Block a user