From 97ca90f184c5c7c1a9ab8d9052f99be440e7fbd7 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 9 Sep 2026 16:19:06 -0700 Subject: [PATCH] fix(desktop): expired OAuth grant shows a one-click 'Sign in again' instead of a retryable Provider error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A rejected OAuth token (HTTP 401 'User not found' from Nous Portal, Codex, xAI…) reached the desktop error card as 'Provider error' with Retry as the first action, which just replays the same dead credential. Backend: nonretryable_client_error_result dropped failure_reason / failure_retryable, so error_surface classified every non-retryable 4xx as a retryable provider failure. It now stamps the classifier verdict like the max-retries path, and auth-layer descriptors carry auth_kind (oauth|api_key, derived from the provider catalog tab) + provider_label. Desktop: an auth/oauth surface renders 'Authentication error', explains that the sign-in expired/was revoked, and offers 'Sign in to again' which launches that provider's existing onboarding OAuth flow scoped to the failed session's gateway profile. Retry stays as the follow-up click. Re-login to the provider already in use keeps the current model instead of swapping in the recommended default. --- agent/error_surface.py | 30 +++++++++++- agent/turn_recovery.py | 10 +++- .../thread/assistant-message.test.tsx | 47 +++++++++++++++++++ .../assistant-ui/thread/assistant-message.tsx | 45 ++++++++++++++++-- apps/desktop/src/i18n/ar.ts | 2 + apps/desktop/src/i18n/en.ts | 3 ++ apps/desktop/src/i18n/ja.ts | 2 + apps/desktop/src/i18n/types.ts | 6 +++ apps/desktop/src/i18n/zh-hant.ts | 2 + apps/desktop/src/i18n/zh.ts | 2 + apps/desktop/src/lib/error-surface.ts | 30 +++++++++++- apps/desktop/src/store/onboarding.ts | 13 +++++ tests/agent/test_error_surface.py | 15 ++++++ ...est_nonretryable_result_carries_verdict.py | 43 +++++++++++++++++ 14 files changed, 241 insertions(+), 9 deletions(-) create mode 100644 tests/agent/test_nonretryable_result_carries_verdict.py diff --git a/agent/error_surface.py b/agent/error_surface.py index 77e7cd6393..95fbdadd03 100644 --- a/agent/error_surface.py +++ b/agent/error_surface.py @@ -77,7 +77,35 @@ def _surface(layer: str, code: str, retryable: bool, provider: str = "", model: # Identity captured at classification time, so clients report the session # that actually failed — not whatever the composer points at later. identity = {k: v for k, v in (("provider", provider), ("model", model)) if v} - return {"layer": layer, "code": code, "retryable": bool(retryable), **identity} + surface = {"layer": layer, "code": code, "retryable": bool(retryable), **identity} + if layer == LAYER_AUTH and provider: + # OAuth providers are fixed by signing in again; API-key providers by + # replacing the key. The client's one-click recovery needs to know which + # and how to name the account it re-opens. + surface["auth_kind"] = _auth_kind(provider) + surface["provider_label"] = _provider_label(provider) + return surface + + +def _provider_label(provider: str) -> str: + try: + from hermes_cli.models import provider_label + + return provider_label(provider) + except Exception: # pragma: no cover — advisory only + return provider + + +def _auth_kind(provider: Optional[str]) -> str: + """``"oauth"`` for providers whose credential is an OAuth/subscription grant + (desktop Accounts tab), ``"api_key"`` for everything else.""" + try: + from hermes_cli.provider_catalog import provider_catalog_by_slug + + descriptor = provider_catalog_by_slug().get((provider or "").strip().lower()) + return "oauth" if descriptor is not None and descriptor.tab == "accounts" else "api_key" + except Exception: # pragma: no cover — advisory only + return "api_key" def _disk_full(candidate: Any) -> bool: diff --git a/agent/turn_recovery.py b/agent/turn_recovery.py index 1d54332758..38019505fe 100644 --- a/agent/turn_recovery.py +++ b/agent/turn_recovery.py @@ -738,7 +738,15 @@ def nonretryable_client_error_result( classified=classified, summary=_nonretryable_summary, messages=messages, api_call_count=api_call_count, provider=provider, base_url=base_url, model=model, ) - return _failed_turn_result(_nonretryable_summary, messages, api_call_count, _nonretryable_summary) + result = _failed_turn_result(_nonretryable_summary, messages, api_call_count, _nonretryable_summary) + # Same verdict fields as the max-retries path: without them the UI descriptor + # (agent/error_surface.py) reads a rejected OAuth token as a retryable + # "Provider error" and offers Retry instead of a re-login. + result.update({ + "failure_reason": classified.reason.value, + "failure_retryable": bool(classified.retryable), + }) + return result _STREAM_DROP_MARKERS = ( diff --git a/apps/desktop/src/components/assistant-ui/thread/assistant-message.test.tsx b/apps/desktop/src/components/assistant-ui/thread/assistant-message.test.tsx index 4b76e5eb57..0f75238f05 100644 --- a/apps/desktop/src/components/assistant-ui/thread/assistant-message.test.tsx +++ b/apps/desktop/src/components/assistant-ui/thread/assistant-message.test.tsx @@ -17,12 +17,18 @@ import { formatTimelineRange, formatTimelineTimestamp } from './timestamp' import { Thread } from '.' const requestFreshSession = vi.hoisted(() => vi.fn()) +const startManualProviderOAuth = vi.hoisted(() => vi.fn()) vi.mock('@/store/profile', async importOriginal => ({ ...(await importOriginal>()), requestFreshSession: () => requestFreshSession() })) +vi.mock('@/store/onboarding', async importOriginal => ({ + ...(await importOriginal>()), + startManualProviderOAuth: (...args: unknown[]) => startManualProviderOAuth(...args) +})) + // Timeline timestamps render only when `display.timestamps` is enabled. $displayTimestamps.set(true) @@ -33,6 +39,7 @@ stubThreadEnvironment() afterEach(() => { cleanup() requestFreshSession.mockClear() + startManualProviderOAuth.mockClear() }) function userMessage(): ThreadMessage { @@ -100,6 +107,33 @@ function ownershipRefusalMessage(): ThreadMessage { } as unknown as ThreadMessage } +function oauthExpiredMessage(): ThreadMessage { + return { + id: 'assistant-error-2', + role: 'assistant', + content: [], + status: { type: 'incomplete', reason: 'error', error: 'HTTP 401: User not found.' }, + createdAt, + metadata: { + unstable_state: null, + unstable_annotations: [], + unstable_data: [], + steps: [], + // What agent/error_surface.py stamps on a rejected OAuth grant. + custom: { + errorSurface: { + authKind: 'oauth', + code: 'auth', + layer: 'auth', + provider: 'nous', + providerLabel: 'Nous Portal', + retryable: false + } + } + } + } as unknown as ThreadMessage +} + function Harness({ assistant = assistantMessage(), onBranchInNewChat @@ -151,6 +185,19 @@ describe('ownership refusal recovery (#106217)', () => { }) }) +describe('expired OAuth grant recovery', () => { + it('explains the expiry and re-runs that provider sign-in in one click', async () => { + render() + + expect(await screen.findByText(/Nous Portal sign-in has expired/)).toBeTruthy() + // Signing in changes the outcome, so Retry stays as the follow-up click. + expect(screen.getByRole('button', { name: 'Retry' })).toBeTruthy() + + screen.getByRole('button', { name: 'Sign in to Nous Portal again' }).click() + expect(startManualProviderOAuth).toHaveBeenCalledWith('nous', undefined) + }) +}) + describe('message timeline timestamps', () => { it('always renders precise user and assistant lifecycle times', async () => { const { container } = render() diff --git a/apps/desktop/src/components/assistant-ui/thread/assistant-message.tsx b/apps/desktop/src/components/assistant-ui/thread/assistant-message.tsx index aa019a63ef..76fa933395 100644 --- a/apps/desktop/src/components/assistant-ui/thread/assistant-message.tsx +++ b/apps/desktop/src/components/assistant-ui/thread/assistant-message.tsx @@ -30,11 +30,12 @@ import { PreviewAttachment } from '@/components/chat/preview-attachment' import { Codicon } from '@/components/ui/codicon' import { CopyButton } from '@/components/ui/copy-button' import { useI18n } from '@/i18n' -import { type ErrorSurface, formatErrorDiagnostics } from '@/lib/error-surface' +import { type ErrorSurface, formatErrorDiagnostics, isOAuthReauthSurface } from '@/lib/error-surface' import { triggerHaptic } from '@/lib/haptics' import { AudioLines, GitForkIcon, + KeyRound, Loader2Icon, RefreshCwIcon, SmilePlusIcon, @@ -48,7 +49,8 @@ import { useEnterAnimation } from '@/lib/use-enter-animation' import { cn } from '@/lib/utils' import { playSpeechText, stopVoicePlayback } from '@/lib/voice-playback' import { notifyError } from '@/store/notifications' -import { requestFreshSession } from '@/store/profile' +import { startManualProviderOAuth } from '@/store/onboarding' +import { $activeGatewayProfile, normalizeProfileKey, requestFreshSession } from '@/store/profile' import { requestSendDiagnostics } from '@/store/send-diagnostics' import { $connection, $currentModel } from '@/store/session' import { $voicePlayback } from '@/store/voice-playback' @@ -470,7 +472,14 @@ const ErrorLayerLabel: FC = () => { const labels = t.assistant.thread.errorLayers const label = (surface && labels[surface.layer]) || labels.generic - return
{label}
+ return ( + <> +
{label}
+ {isOAuthReauthSurface(surface) && ( +
{t.assistant.thread.errorOauthExpired(surface.providerLabel || surface.provider)}
+ )} + + ) } // Isolated because useNavigate() THROWS outside a (bare test @@ -510,10 +519,30 @@ const ErrorRecoveryActions: FC = () => { // runtime's logs. const remoteConnection = connection?.mode === 'remote' + // An expired/revoked OAuth grant (HTTP 401 on nous / openai-codex / ...): + // the one-click fix is re-running that provider's sign-in, which the + // onboarding overlay already owns end to end (device code → poll → + // reload.env → model confirm). Scoped to the gateway profile the failed + // session runs on, so a Bot profile's grant is renewed, not the primary's. + const oauthReauth = isOAuthReauthSurface(surface) + const gatewayProfile = useStore($activeGatewayProfile) + + const signInAgain = useCallback(() => { + if (!oauthReauth) { + return + } + + triggerHaptic('submit') + const key = normalizeProfileKey(gatewayProfile) + startManualProviderOAuth(surface.provider, key === 'default' ? undefined : key) + }, [gatewayProfile, oauthReauth, surface]) + // Retry = assistant-ui reload (same wiring as the footer's refresh action): // re-runs the failed turn's prompt in place. Suppressed when the classifier - // says the failure is deterministic (retrying reproduces it). - const retryable = !surface || surface.retryable + // says the failure is deterministic (retrying reproduces it) — except for an + // OAuth rejection, where signing in again changes the outcome and Retry is + // the natural second click. + const retryable = !surface || surface.retryable || oauthReauth // Another surface holds this session's lease (#106217): Retry would hit the // same refusal, so the way out is a fresh session on this surface. @@ -565,6 +594,12 @@ const ErrorRecoveryActions: FC = () => { {copy.errorStartNewSession} )} + {oauthReauth && ( + + )} {retryable && (