fix(settings): tell a failed load apart from a genuinely empty pane

This commit is contained in:
Jinjing
2026-09-14 15:25:15 -07:00
parent 1834adbc1e
commit 7e1598e73f
8 changed files with 387 additions and 40 deletions
@@ -0,0 +1,114 @@
// @vitest-environment happy-dom
import { act } from 'react'
import { createRoot, type Root } from 'react-dom/client'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
const mocks = vi.hoisted(() => ({
claudeList: vi.fn(),
codexList: vi.fn(),
rateLimits: { claude: null, codex: null } as Record<string, unknown>
}))
vi.mock('sonner', () => ({ toast: { error: vi.fn(), success: vi.fn() } }))
vi.mock('@/store', () => ({
useAppStore: (selector: (state: Record<string, unknown>) => unknown) =>
selector({
fetchSettings: async () => {},
fetchRateLimits: async () => {},
rateLimits: mocks.rateLimits
})
}))
import { UsageAccountsCard } from './UsageAccountsCard'
const EMPTY_ACCOUNTS = { accounts: [], activeAccountId: null }
const UNKNOWN_TEXT = 'Account status unknown'
const NOT_SET_UP_TEXT = 'Tracking not set up'
let container: HTMLDivElement
let root: Root
async function renderCard(): Promise<void> {
container = document.createElement('div')
document.body.appendChild(container)
root = createRoot(container)
await act(async () => {
root.render(<UsageAccountsCard />)
})
}
describe('UsageAccountsCard account-list failures', () => {
beforeEach(() => {
vi.clearAllMocks()
mocks.rateLimits = { claude: null, codex: null }
Object.assign(window, {
api: {
claudeAccounts: { list: mocks.claudeList, add: vi.fn() },
codexAccounts: { list: mocks.codexList, add: vi.fn() }
}
})
})
afterEach(() => {
act(() => root.unmount())
container.remove()
})
it('stops asserting "Tracking not set up" for a provider whose list never loaded', async () => {
mocks.claudeList.mockRejectedValue(new Error('offline'))
mocks.codexList.mockResolvedValue(EMPTY_ACCOUNTS)
await renderCard()
const pills = Array.from(container.querySelectorAll('span')).map((node) => node.textContent)
expect(pills).toContain(UNKNOWN_TEXT)
// Why: only the failing provider goes unknown — Codex genuinely answered "none".
expect(pills).toContain(NOT_SET_UP_TEXT)
})
it('keeps the real label when the list resolves empty', async () => {
mocks.claudeList.mockResolvedValue(EMPTY_ACCOUNTS)
mocks.codexList.mockResolvedValue(EMPTY_ACCOUNTS)
await renderCard()
expect(container.textContent).not.toContain(UNKNOWN_TEXT)
expect(container.textContent).toContain(NOT_SET_UP_TEXT)
})
it('prefers the observed connection when rate limits already prove tracking is on', async () => {
mocks.claudeList.mockRejectedValue(new Error('offline'))
mocks.codexList.mockResolvedValue(EMPTY_ACCOUNTS)
mocks.rateLimits = { claude: { status: 'ok', session: null, weekly: null }, codex: null }
await renderCard()
expect(container.textContent).not.toContain(UNKNOWN_TEXT)
expect(container.textContent).toContain('Connected · System default')
})
it('keeps the quiet label while the read is still in flight, and only then goes unknown', async () => {
// Why: pending is not failed. Painting "unknown" before the IPC settles alarms exactly the
// users this card targets — the ones with no tracking configured, whose rateLimits never fill.
let rejectClaude: (reason: Error) => void = () => {}
mocks.claudeList.mockReturnValue(
new Promise((_resolve, reject) => {
rejectClaude = reject
})
)
mocks.codexList.mockResolvedValue(EMPTY_ACCOUNTS)
await renderCard()
expect(container.textContent).not.toContain(UNKNOWN_TEXT)
expect(container.textContent).toContain(NOT_SET_UP_TEXT)
await act(async () => {
rejectClaude(new Error('offline'))
await Promise.resolve()
})
expect(container.textContent).toContain(UNKNOWN_TEXT)
})
})
@@ -16,6 +16,21 @@ import { translate } from '@/i18n/i18n'
type ConnectAction = 'idle' | 'adding'
// Why only 'failed': a read still in flight is not evidence of anything, so it keeps the quiet
// label until it settles. Only a read that actually FAILED may claim the status is unknown.
function usageConnectionLabel(
connection: { connected: boolean; label: string },
accountsRead: 'pending' | 'failed' | 'loaded'
): string {
if (connection.connected || accountsRead !== 'failed') {
return connection.label
}
return translate(
'auto.components.feature.wall.agents.orchestration.UsageAccountsCard.accountStatusUnknown',
'Account status unknown'
)
}
function ConnectionPill(props: { connected: boolean; label: string }): JSX.Element {
const { connected, label } = props
return (
@@ -97,14 +112,14 @@ export function UsageAccountsCard(props: {
const fetchRateLimits = useAppStore((s) => s.fetchRateLimits)
const mountedRef = useMountedRef()
const [claudeAccounts, setClaudeAccounts] = useState<ClaudeRateLimitAccountsState>({
accounts: [],
activeAccountId: null
})
const [codexAccounts, setCodexAccounts] = useState<CodexRateLimitAccountsState>({
accounts: [],
activeAccountId: null
})
const [claudeAccounts, setClaudeAccounts] = useState<ClaudeRateLimitAccountsState>()
const [codexAccounts, setCodexAccounts] = useState<CodexRateLimitAccountsState>()
const [claudeAccountsRead, setClaudeAccountsRead] = useState<'pending' | 'failed' | 'loaded'>(
'pending'
)
const [codexAccountsRead, setCodexAccountsRead] = useState<'pending' | 'failed' | 'loaded'>(
'pending'
)
const [claudeAction, setClaudeAction] = useState<ConnectAction>('idle')
const [codexAction, setCodexAction] = useState<ConnectAction>('idle')
@@ -119,9 +134,13 @@ export function UsageAccountsCard(props: {
const next = await window.api.claudeAccounts.list()
if (!stale) {
setClaudeAccounts(next)
setClaudeAccountsRead('loaded')
}
} catch {
// Silent — empty list is the right fallback for the inline pitch.
// Why: a failed read is distinct from a confirmed empty account list.
if (!stale) {
setClaudeAccountsRead('failed')
}
}
})()
void (async () => {
@@ -129,9 +148,12 @@ export function UsageAccountsCard(props: {
const next = await window.api.codexAccounts.list()
if (!stale) {
setCodexAccounts(next)
setCodexAccountsRead('loaded')
}
} catch {
// Silent — same reason as above.
if (!stale) {
setCodexAccountsRead('failed')
}
}
})()
return () => {
@@ -140,11 +162,11 @@ export function UsageAccountsCard(props: {
}, [fetchRateLimits])
const claudeConnection = getFeatureWallUsageProviderConnection({
managedAccountCount: claudeAccounts.accounts.length,
managedAccountCount: claudeAccounts?.accounts.length ?? 0,
provider: rateLimits.claude
})
const codexConnection = getFeatureWallUsageProviderConnection({
managedAccountCount: codexAccounts.accounts.length,
managedAccountCount: codexAccounts?.accounts.length ?? 0,
provider: rateLimits.codex
})
@@ -240,7 +262,7 @@ export function UsageAccountsCard(props: {
'Track session and weekly usage.'
)}
connected={claudeConnection.connected}
connectionLabel={claudeConnection.label}
connectionLabel={usageConnectionLabel(claudeConnection, claudeAccountsRead)}
isAdding={claudeAction === 'adding'}
onSignIn={() => void handleClaudeSignIn()}
/>
@@ -252,7 +274,7 @@ export function UsageAccountsCard(props: {
'Surface rate limits and swap accounts inline.'
)}
connected={codexConnection.connected}
connectionLabel={codexConnection.label}
connectionLabel={usageConnectionLabel(codexConnection, codexAccountsRead)}
isAdding={codexAction === 'adding'}
onSignIn={() => void handleCodexSignIn()}
/>
@@ -0,0 +1,117 @@
// @vitest-environment happy-dom
import { act } from 'react'
import { createRoot, type Root } from 'react-dom/client'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
const mocks = vi.hoisted(() => ({
refresh: vi.fn(),
status: vi.fn()
}))
vi.mock('./source-control-preflight-card-status', () => ({
usePreflightCardStatuses: () => ({
statuses: { bitbucketStatus: 'not-authenticated', bitbucketAccount: null },
unavailable: false,
refresh: mocks.refresh
})
}))
vi.mock('./bitbucket-credentials-dialog', () => ({
BitbucketCredentialsDialog: () => null
}))
import { BitbucketIntegrationCard } from './bitbucket-integration-card'
const LOAD_FAILED_TEXT = 'Could not check for a saved Bitbucket credential.'
let container: HTMLDivElement
let root: Root
async function renderCard(): Promise<void> {
container = document.createElement('div')
document.body.appendChild(container)
root = createRoot(container)
await act(async () => {
root.render(<BitbucketIntegrationCard />)
})
}
function recheckButton(): HTMLButtonElement {
const button = Array.from(container.querySelectorAll('button')).find(
(candidate) => candidate.textContent === 'Re-check'
)
if (!button) {
throw new Error('Re-check button not rendered')
}
return button
}
describe('BitbucketIntegrationCard credential-read failures', () => {
beforeEach(() => {
vi.clearAllMocks()
Object.assign(window, {
api: {
bitbucket: { status: mocks.status, disconnect: vi.fn(async () => {}) },
shell: { openUrl: vi.fn() }
}
})
})
afterEach(() => {
act(() => root.unmount())
container.remove()
})
it('says the credential could not be read rather than rendering as "nothing stored"', async () => {
mocks.status.mockRejectedValue(new Error('keychain locked'))
await renderCard()
expect(container.textContent).toContain(LOAD_FAILED_TEXT)
})
it('does not claim a read failure when the status resolves', async () => {
mocks.status.mockResolvedValue({ source: 'none', account: null })
await renderCard()
expect(container.textContent).not.toContain(LOAD_FAILED_TEXT)
})
it('retries the failed credential read from Re-check, not just the preflight', async () => {
mocks.status.mockRejectedValueOnce(new Error('keychain locked'))
mocks.status.mockResolvedValueOnce({ source: 'none', account: null })
await renderCard()
expect(container.textContent).toContain(LOAD_FAILED_TEXT)
await act(async () => {
recheckButton().click()
})
expect(mocks.status).toHaveBeenCalledTimes(2)
expect(mocks.refresh).toHaveBeenCalled()
expect(container.textContent).not.toContain(LOAD_FAILED_TEXT)
})
it('does not let an older failed read overwrite a newer successful re-check', async () => {
let rejectInitial!: (error: Error) => void
mocks.status
.mockReturnValueOnce(
new Promise((_resolve, reject) => {
rejectInitial = reject
})
)
.mockResolvedValueOnce({ source: 'none', account: null })
await renderCard()
await act(async () => {
recheckButton().click()
})
await act(async () => {
rejectInitial(new Error('late keychain failure'))
})
expect(container.textContent).not.toContain(LOAD_FAILED_TEXT)
})
})
@@ -1,8 +1,9 @@
import { useCallback, useEffect, useState } from 'react'
import { useCallback, useEffect, useRef, useState } from 'react'
import { ExternalLink, GitPullRequestArrow, LoaderCircle, Unlink } from 'lucide-react'
import type { BitbucketConnectionStatus } from '../../../../shared/bitbucket-credentials'
import { Button } from '@/components/ui/button'
import { useMountedRef } from '@/hooks/useMountedRef'
import { readIpcErrorMessage } from '@/lib/ipc-error'
import { IntegrationCardDetails, IntegrationCardShell } from './integration-card-shell'
import { useIntegrationSubordinateRowClass } from './integration-card-presentation'
import type { BitbucketStatus } from './integrations-pane-status'
@@ -24,17 +25,24 @@ export function BitbucketIntegrationCard(): React.JSX.Element {
const [dialogOpen, setDialogOpen] = useState(false)
const [disconnecting, setDisconnecting] = useState(false)
const [disconnectError, setDisconnectError] = useState<string | null>(null)
const [connectionLoadFailed, setConnectionLoadFailed] = useState(false)
const connectionLoadGenerationRef = useRef(0)
// Reads plaintext metadata only — never the encrypted secret — so mounting the
// pane cannot trigger a keychain prompt.
const loadConnection = useCallback(async () => {
const generation = ++connectionLoadGenerationRef.current
try {
const next = await window.api.bitbucket.status()
if (mountedRef.current) {
if (mountedRef.current && generation === connectionLoadGenerationRef.current) {
setConnection(next)
setConnectionLoadFailed(false)
}
} catch {
// Best-effort: the preflight-driven parts of the card still render.
// Why: without this the card renders exactly like "no credential stored" — say it is unknown.
if (mountedRef.current && generation === connectionLoadGenerationRef.current) {
setConnectionLoadFailed(true)
}
}
}, [mountedRef])
@@ -61,7 +69,9 @@ export function BitbucketIntegrationCard(): React.JSX.Element {
: null
const credentialSummary = [authModeLabel, baseUrlOverride].filter(Boolean).join(' · ')
const handleConnected = (): void => {
// Why: Re-check and a fresh connection both need the preflight AND the credential read, so
// Re-check also retries a status() that failed rather than only re-running the preflight.
const reloadCardState = (): void => {
void loadConnection()
refresh()
}
@@ -76,12 +86,11 @@ export function BitbucketIntegrationCard(): React.JSX.Element {
// Unhandled, the card silently re-renders as still connected.
if (mountedRef.current) {
setDisconnectError(
error instanceof Error
? error.message
: translate(
'auto.components.settings.bitbucket.integration.card.disconnectFailed',
'Could not remove the saved Bitbucket credential.'
)
readIpcErrorMessage(error) ??
translate(
'auto.components.settings.bitbucket.integration.card.disconnectFailed',
'Could not remove the saved Bitbucket credential.'
)
)
}
} finally {
@@ -170,6 +179,14 @@ export function BitbucketIntegrationCard(): React.JSX.Element {
</div>
) : null}
{disconnectError ? <p className="text-xs text-destructive">{disconnectError}</p> : null}
{connectionLoadFailed ? (
<p role="alert" className="text-xs text-destructive">
{translate(
'auto.components.settings.bitbucket.integration.card.statusLoadFailed',
'Could not check for a saved Bitbucket credential.'
)}
</p>
) : null}
<BitbucketCardNote
envManaged={envManaged}
status={status}
@@ -189,7 +206,7 @@ export function BitbucketIntegrationCard(): React.JSX.Element {
)}
</Button>
) : null}
<Button variant="ghost" size="sm" onClick={refresh}>
<Button variant="ghost" size="sm" onClick={reloadCardState}>
{translate(
'auto.components.settings.token.source.control.integration.cards.793a06e899',
'Re-check'
@@ -206,7 +223,7 @@ export function BitbucketIntegrationCard(): React.JSX.Element {
initialEmail={connection?.email}
initialBaseUrl={connection?.baseUrl}
environmentManaged={envManaged}
onConnected={handleConnected}
onConnected={reloadCardState}
/>
</IntegrationCardShell>
)
@@ -451,4 +451,68 @@ describe('SkillsPage', () => {
expect(container?.textContent).toContain('0 selected')
expect(renderedSkillNames()).toEqual(['beta'])
})
it('distinguishes a failed scan from empty skill folders', async () => {
const discover = vi
.fn()
.mockRejectedValue(
new Error(
"Error invoking remote method 'skills:discover': Error: EACCES: permission denied"
)
)
Object.defineProperty(window, 'api', {
configurable: true,
value: { skills: skillsApi(discover), runtimeEnvironments: { call: vi.fn() } }
})
vi.spyOn(console, 'error').mockImplementation(() => {})
await renderPage()
await flushMicrotasks()
expect(container?.textContent).toContain('Could not scan skills')
expect(container?.textContent).toContain('EACCES: permission denied')
// Why: nothing was scanned, so "the scanned skill folders are empty" would be a claim we cannot make.
expect(container?.textContent).not.toContain('No skills found')
})
it('retries the failed scan from the error band and clears it on success', async () => {
const discover = vi
.fn()
.mockRejectedValueOnce(new Error('EACCES: permission denied'))
.mockResolvedValueOnce(discoveryResult(['alpha']))
Object.defineProperty(window, 'api', {
configurable: true,
value: { skills: skillsApi(discover), runtimeEnvironments: { call: vi.fn() } }
})
vi.spyOn(console, 'error').mockImplementation(() => {})
await renderPage()
await flushMicrotasks()
await act(async () => fireEvent.click(buttonNamed('Retry')))
await flushMicrotasks()
expect(container?.textContent).not.toContain('Could not scan skills')
expect(renderedSkillNames()).toEqual(['alpha'])
})
it('keeps a previously confirmed empty result visible when a refresh fails', async () => {
const discover = vi
.fn()
.mockResolvedValueOnce(discoveryResult([]))
.mockRejectedValueOnce(new Error('host unavailable'))
Object.defineProperty(window, 'api', {
configurable: true,
value: { skills: skillsApi(discover), runtimeEnvironments: { call: vi.fn() } }
})
vi.spyOn(console, 'error').mockImplementation(() => {})
await renderPage()
await flushMicrotasks()
expect(container?.textContent).toContain('No skills found')
await act(async () => fireEvent.click(buttonNamed('Refresh')))
await flushMicrotasks()
expect(container?.textContent).toContain('Could not scan skills')
expect(container?.textContent).toContain('No skills found')
})
})
@@ -1,5 +1,6 @@
import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
import { Share2, Trash2 } from 'lucide-react'
import { readIpcErrorDetail } from '@/lib/ipc-error'
import { cn } from '@/lib/utils'
import { useAppStore } from '@/store'
import { discoverSkillsForRuntimeTarget } from '@/runtime/runtime-skills-client'
@@ -68,7 +69,7 @@ export default function SkillsPage(): React.JSX.Element {
const hostLabel = useSkillDiscoveryHostLabel(runtimeTarget)
const [result, setResult] = useState<SkillDiscoveryResult | null>(null)
const [loading, setLoading] = useState(true)
const [scanError, setScanError] = useState<string | null>(null)
const [scanError, setScanError] = useState<{ detail?: string } | null>(null)
const [shareSkills, setShareSkills] = useState<DiscoveredSkill[]>([])
const [selectionMode, setSelectionMode] = useState<'share' | 'delete' | null>(null)
const [selectedSkillIds, setSelectedSkillIds] = useState<Set<string>>(() => new Set())
@@ -121,9 +122,7 @@ export default function SkillsPage(): React.JSX.Element {
if (isCurrentScan()) {
// Why: a failed scan needs to stay on screen with a retry — a toast
// disappears before the user can act on it.
setScanError(
translate('auto.components.skills.SkillsPage.ea72d6185b', 'Could not scan skills')
)
setScanError({ detail: readIpcErrorDetail(error) })
}
} finally {
if (isCurrentScan()) {
@@ -310,7 +309,7 @@ export default function SkillsPage(): React.JSX.Element {
/>
{scanError ? (
<SkillsScanErrorBand
message={scanError}
detail={scanError.detail}
disabled={loading}
onRetry={() => {
deleteFlow.reprobe()
@@ -359,7 +358,7 @@ export default function SkillsPage(): React.JSX.Element {
/>
) : skills.length > 0 ? (
<SkillsNoMatchesState onClearFilters={() => setFilters(NO_FILTERS)} />
) : (
) : result ? (
<SkillsEmptyState
onRefresh={() => {
deleteFlow.reprobe()
@@ -367,7 +366,7 @@ export default function SkillsPage(): React.JSX.Element {
}}
onInstallFromLink={openInstallDialog}
/>
)}
) : null}
</>
)}
</div>
@@ -84,11 +84,12 @@ export function SkillsEmptyState({
}
export function SkillsScanErrorBand({
message,
detail,
disabled,
onRetry
}: {
message: string
/** The underlying scan error, so the user can tell a missing folder from a broken host. */
detail?: string
disabled: boolean
onRetry: () => void
}): React.JSX.Element {
@@ -97,9 +98,20 @@ export function SkillsScanErrorBand({
<div
className={cn(SKILLS_PAGE_COLUMN, 'flex flex-wrap items-center justify-between gap-3 py-2')}
>
<p className="min-w-0 flex-1 text-xs text-destructive" role="alert">
{message}
</p>
{/* Why: role on the wrapper so the reason line is announced too, not just the headline. */}
<div className="min-w-0 flex-1" role="alert">
{/* Why: same two-line shape as the CLI install-failure band — one token, weight separates them. */}
<p className={cn('text-xs text-destructive', detail && 'font-medium')}>
{translate('auto.components.skills.SkillsPage.ea72d6185b', 'Could not scan skills')}
</p>
{detail ? (
// Why whitespace-pre-line: this slot is fed the UNCLAMPED reader precisely so a
// multi-line git/SSH stderr survives; default white-space would collapse it again.
<p className="whitespace-pre-line break-words text-xs leading-snug text-destructive">
{detail}
</p>
) : null}
</div>
<Button type="button" variant="outline" size="xs" disabled={disabled} onClick={onRetry}>
{translate('auto.components.skills.SkillsPage.retry', 'Retry')}
</Button>
+4 -2
View File
@@ -11817,7 +11817,8 @@
"storedAuthFailed": "The saved Bitbucket credential could not authenticate. Edit it, or check that the token still has pull request access.",
"storedCredential": "Saved in Orca on this machine. ORCA_BITBUCKET_* environment variables take precedence when set.",
"notConfigured": "Connect a Bitbucket Cloud account with an Atlassian API token or an access token. ORCA_BITBUCKET_* environment variables work too and take precedence.",
"disconnectFailed": "Could not remove the saved Bitbucket credential."
"disconnectFailed": "Could not remove the saved Bitbucket credential.",
"statusLoadFailed": "Could not check for a saved Bitbucket credential."
}
}
},
@@ -14310,7 +14311,8 @@
"4e71d72912": "Claude sign-in failed.",
"9ddeb558f9": "Claude account added.",
"29d0653961": "Sign in",
"945865332e": "Signing in"
"945865332e": "Signing in",
"accountStatusUnknown": "Account status unknown"
},
"UsagePage": {
"64265cb295": "29% used 5h",