fix(desktop): expired OAuth grant shows a one-click 'Sign in again' instead of a retryable Provider error
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 <provider> sign-in expired/was revoked, and offers 'Sign in to <provider> 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.
This commit is contained in:
+29
-1
@@ -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:
|
||||
|
||||
@@ -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 = (
|
||||
|
||||
@@ -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<Record<string, unknown>>()),
|
||||
requestFreshSession: () => requestFreshSession()
|
||||
}))
|
||||
|
||||
vi.mock('@/store/onboarding', async importOriginal => ({
|
||||
...(await importOriginal<Record<string, unknown>>()),
|
||||
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(<Harness assistant={oauthExpiredMessage()} />)
|
||||
|
||||
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(<Harness />)
|
||||
|
||||
@@ -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 <div className="font-medium">{label}</div>
|
||||
return (
|
||||
<>
|
||||
<div className="font-medium">{label}</div>
|
||||
{isOAuthReauthSurface(surface) && (
|
||||
<div>{t.assistant.thread.errorOauthExpired(surface.providerLabel || surface.provider)}</div>
|
||||
)}
|
||||
</>
|
||||
)
|
||||
}
|
||||
|
||||
// Isolated because useNavigate() THROWS outside a <Router> (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}
|
||||
</button>
|
||||
)}
|
||||
{oauthReauth && (
|
||||
<button className="aui-error-action" onClick={signInAgain} type="button">
|
||||
<KeyRound className="size-3" />
|
||||
{copy.errorSignInAgain(surface.providerLabel || surface.provider)}
|
||||
</button>
|
||||
)}
|
||||
{retryable && (
|
||||
<ActionBarPrimitive.Reload asChild>
|
||||
<button className="aui-error-action" onClick={() => triggerHaptic('submit')} type="button">
|
||||
|
||||
@@ -2700,6 +2700,8 @@ export const ar = defineLocale({
|
||||
errorRetry: 'إعادة المحاولة',
|
||||
errorStartNewSession: 'بدء جلسة جديدة',
|
||||
errorSwitchProvider: 'تبديل المزوّد',
|
||||
errorSignInAgain: provider => `تسجيل الدخول إلى ${provider} مجدداً`,
|
||||
errorOauthExpired: provider => `انتهت صلاحية تسجيل دخولك إلى ${provider} أو تم إلغاؤه. سجّل الدخول مجدداً لمتابعة المحادثة.`,
|
||||
errorOpenLogs: 'فتح السجلات',
|
||||
errorOpenLogsFailed: 'تعذّر فتح مجلد السجلات',
|
||||
errorOpenDesktopLogs: 'فتح سجلات سطح المكتب',
|
||||
|
||||
@@ -3578,6 +3578,9 @@ export const en: Translations = {
|
||||
errorRetry: 'Retry',
|
||||
errorStartNewSession: 'Start new session',
|
||||
errorSwitchProvider: 'Switch provider',
|
||||
errorSignInAgain: provider => `Sign in to ${provider} again`,
|
||||
errorOauthExpired: provider =>
|
||||
`Your ${provider} sign-in has expired or was revoked. Sign in again to keep chatting.`,
|
||||
errorOpenLogs: 'Open logs',
|
||||
errorOpenLogsFailed: 'Could not open the logs folder',
|
||||
errorOpenDesktopLogs: 'Open Desktop logs',
|
||||
|
||||
@@ -3150,6 +3150,8 @@ export const ja = defineLocale({
|
||||
errorRetry: '再試行',
|
||||
errorStartNewSession: '新しいセッションを開始',
|
||||
errorSwitchProvider: 'プロバイダーを切り替え',
|
||||
errorSignInAgain: provider => `${provider} に再度サインイン`,
|
||||
errorOauthExpired: provider => `${provider} のサインインが期限切れか取り消されました。続けるには再度サインインしてください。`,
|
||||
errorOpenLogs: 'ログを開く',
|
||||
errorOpenLogsFailed: 'ログフォルダを開けませんでした',
|
||||
errorOpenDesktopLogs: 'デスクトップのログを開く',
|
||||
|
||||
@@ -3112,6 +3112,12 @@ export interface Translations {
|
||||
/** Escape hatch when Retry would only reproduce SESSION_NOT_OWNED (#106217). */
|
||||
errorStartNewSession: string
|
||||
errorSwitchProvider: string
|
||||
/** One-click recovery for an expired/revoked OAuth grant: re-runs that
|
||||
* provider's sign-in flow (auth layer, authKind 'oauth'). */
|
||||
errorSignInAgain: (provider: string) => string
|
||||
/** Explains WHY the turn failed for an OAuth 401 — the raw body
|
||||
* ("HTTP 401: User not found.") doesn't say "sign in again". */
|
||||
errorOauthExpired: (provider: string) => string
|
||||
errorOpenLogs: string
|
||||
errorOpenLogsFailed: string
|
||||
errorOpenDesktopLogs: string
|
||||
|
||||
@@ -3042,6 +3042,8 @@ export const zhHant = defineLocale({
|
||||
errorRetry: '重試',
|
||||
errorStartNewSession: '開始新工作階段',
|
||||
errorSwitchProvider: '切換服務商',
|
||||
errorSignInAgain: provider => `重新登入 ${provider}`,
|
||||
errorOauthExpired: provider => `您的 ${provider} 登入已過期或被撤銷。請重新登入以繼續對話。`,
|
||||
errorOpenLogs: '開啟日誌',
|
||||
errorOpenLogsFailed: '無法開啟日誌資料夾',
|
||||
errorOpenDesktopLogs: '開啟桌面端日誌',
|
||||
|
||||
@@ -3718,6 +3718,8 @@ export const zh: Translations = {
|
||||
errorRetry: '重试',
|
||||
errorStartNewSession: '开始新会话',
|
||||
errorSwitchProvider: '切换服务商',
|
||||
errorSignInAgain: provider => `重新登录 ${provider}`,
|
||||
errorOauthExpired: provider => `您的 ${provider} 登录已过期或被撤销。请重新登录以继续对话。`,
|
||||
errorOpenLogs: '打开日志',
|
||||
errorOpenLogsFailed: '无法打开日志文件夹',
|
||||
errorOpenDesktopLogs: '打开桌面端日志',
|
||||
|
||||
@@ -30,6 +30,12 @@ export interface ErrorSurface {
|
||||
* different model by the time the user clicks an action. */
|
||||
provider?: string
|
||||
model?: string
|
||||
/** Auth layer only: how the failing provider is credentialed. `oauth` means
|
||||
* the fix is signing in again (expired/revoked grant); `api_key` means a
|
||||
* key needs replacing. Absent from older backends. */
|
||||
authKind?: 'api_key' | 'oauth'
|
||||
/** Auth layer only: display name of the failing provider ("Nous Portal"). */
|
||||
providerLabel?: string
|
||||
}
|
||||
|
||||
/** Validate a wire payload into an ErrorSurface, or null when absent/garbled. */
|
||||
@@ -38,7 +44,16 @@ export function parseErrorSurface(value: unknown): ErrorSurface | null {
|
||||
return null
|
||||
}
|
||||
|
||||
const raw = value as { code?: unknown; layer?: unknown; model?: unknown; provider?: unknown; retryable?: unknown }
|
||||
const raw = value as {
|
||||
auth_kind?: unknown
|
||||
code?: unknown
|
||||
layer?: unknown
|
||||
model?: unknown
|
||||
provider?: unknown
|
||||
provider_label?: unknown
|
||||
retryable?: unknown
|
||||
}
|
||||
|
||||
const layer = typeof raw.layer === 'string' ? (raw.layer as ErrorSurfaceLayer) : null
|
||||
|
||||
if (!layer || !ERROR_SURFACE_LAYERS.includes(layer)) {
|
||||
@@ -50,10 +65,21 @@ export function parseErrorSurface(value: unknown): ErrorSurface | null {
|
||||
code: typeof raw.code === 'string' && raw.code ? raw.code : 'unknown',
|
||||
retryable: raw.retryable !== false,
|
||||
...(typeof raw.provider === 'string' && raw.provider ? { provider: raw.provider } : {}),
|
||||
...(typeof raw.model === 'string' && raw.model ? { model: raw.model } : {})
|
||||
...(typeof raw.model === 'string' && raw.model ? { model: raw.model } : {}),
|
||||
...(raw.auth_kind === 'oauth' || raw.auth_kind === 'api_key' ? { authKind: raw.auth_kind } : {}),
|
||||
...(typeof raw.provider_label === 'string' && raw.provider_label ? { providerLabel: raw.provider_label } : {})
|
||||
}
|
||||
}
|
||||
|
||||
/** True when the failed turn's provider rejected an OAuth grant — the
|
||||
* one-click recovery is re-running that provider's sign-in, not editing keys. */
|
||||
export function isOAuthReauthSurface(surface: ErrorSurface | null | undefined): surface is ErrorSurface & {
|
||||
authKind: 'oauth'
|
||||
provider: string
|
||||
} {
|
||||
return surface?.layer === 'auth' && surface.authKind === 'oauth' && Boolean(surface.provider)
|
||||
}
|
||||
|
||||
/** Plain-text error-details blob for the error card's "Copy error details". */
|
||||
export function formatErrorDiagnostics(input: {
|
||||
appVersion?: string
|
||||
|
||||
@@ -294,6 +294,19 @@ async function fetchProviderDefaultModel(
|
||||
return null
|
||||
}
|
||||
|
||||
// Re-login to the provider already in use (expired OAuth grant): keep the
|
||||
// model the user was on. Swapping in the provider's recommended default
|
||||
// would silently change what they're chatting with.
|
||||
const currentModel = String(options?.model ?? '')
|
||||
|
||||
if (
|
||||
currentModel &&
|
||||
String(options?.provider ?? '').toLowerCase() === String(matched.slug).toLowerCase() &&
|
||||
models.map(String).includes(currentModel)
|
||||
) {
|
||||
return { providerSlug: String(matched.slug), defaultModel: currentModel }
|
||||
}
|
||||
|
||||
// Prefer the backend's recommended default — it mirrors the curation
|
||||
// `hermes model` does (for Nous it honors the user's free/paid tier, so a
|
||||
// free user gets a free model rather than a paid default like opus). Fall
|
||||
|
||||
@@ -51,6 +51,21 @@ def test_result_auth_reasons_map_to_auth_layer():
|
||||
assert surface["retryable"] is False
|
||||
|
||||
|
||||
def test_auth_surface_names_oauth_vs_api_key_recovery():
|
||||
"""The desktop's one-click fix differs by credential kind: an OAuth provider
|
||||
(Accounts tab) needs a re-login, an API-key provider a new key. The descriptor
|
||||
carries the kind + display label so the client never guesses from the slug."""
|
||||
oauth = build_error_surface_from_result(_failed_result("auth"), provider="nous")
|
||||
assert oauth["auth_kind"] == "oauth"
|
||||
assert oauth["provider_label"] == "Nous Portal"
|
||||
|
||||
key = build_error_surface_from_result(_failed_result("auth"), provider="openrouter")
|
||||
assert key["auth_kind"] == "api_key"
|
||||
|
||||
# Non-auth layers never carry the field (clients gate the button on it).
|
||||
assert "auth_kind" not in build_error_surface_from_result(_failed_result("rate_limit"), provider="nous")
|
||||
|
||||
|
||||
def test_result_billing_block_wins():
|
||||
surface = build_error_surface_from_result(
|
||||
_failed_result("rate_limit", billing_block={"provider": "nous"})
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
"""A non-retryable 4xx (rejected OAuth token, bad key) must reach UI clients with the
|
||||
classifier's verdict: without ``failure_reason`` the desktop error card read a 401 as a
|
||||
retryable "Provider error" and offered Retry instead of a re-login."""
|
||||
from __future__ import annotations
|
||||
|
||||
from agent.error_classifier import classify_api_error
|
||||
from agent.error_surface import LAYER_AUTH, build_error_surface_from_result
|
||||
from agent.turn_recovery import nonretryable_client_error_result
|
||||
|
||||
|
||||
class _Rejected(Exception):
|
||||
status_code = 401
|
||||
|
||||
def __init__(self) -> None:
|
||||
super().__init__("HTTP 401: User not found.")
|
||||
|
||||
|
||||
class _Agent:
|
||||
log_prefix = ""
|
||||
verbose = False
|
||||
|
||||
def _summarize_api_error(self, error):
|
||||
return str(error)
|
||||
|
||||
def __getattr__(self, name): # status/persist/print helpers the terminal path calls
|
||||
return lambda *args, **kwargs: None
|
||||
|
||||
|
||||
def test_nonretryable_401_result_classifies_as_auth_for_the_ui():
|
||||
error = _Rejected()
|
||||
classified = classify_api_error(error, provider="nous", model="m")
|
||||
result = nonretryable_client_error_result(
|
||||
_Agent(), error, classified, status_code=401, api_kwargs=None, api_messages=[], messages=[],
|
||||
conversation_history=None, api_call_count=1, approx_tokens=10, provider="nous",
|
||||
base_url="https://inference-api.nousresearch.com/v1", model="m",
|
||||
)
|
||||
assert result["failure_reason"] == classified.reason.value
|
||||
assert result["failure_retryable"] is classified.retryable is False
|
||||
|
||||
surface = build_error_surface_from_result(result, provider="nous", model="m")
|
||||
assert surface["layer"] == LAYER_AUTH
|
||||
assert surface["retryable"] is False
|
||||
assert surface["auth_kind"] == "oauth"
|
||||
Reference in New Issue
Block a user