From 334bcbac93b044d29da88507af4ee8d6415d5c2b Mon Sep 17 00:00:00 2001
From: Teknium <127238744+teknium1@users.noreply.github.com>
Date: Fri, 21 Aug 2026 14:50:29 -0700
Subject: [PATCH] fix(desktop): error card honors the classifier's retry
verdict + failing-session identity (review feedback)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Addresses @helix4u's review on #91493:
- conversation_loop now stamps failure_retryable (the real ClassifiedError
verdict) next to failure_reason; error_surface prefers it and only falls
back to the reason set for older results. Fallback set corrected to match
classify_api_error (auth, format_error, billing_unverified now
non-retryable).
- The descriptor carries the failing session's provider/model captured at
classification time; Copy error details prefers them over the foreground
composer atoms.
- Open logs is labeled 'Open Desktop logs' on remote/cloud connections —
the local folder holds transport logs, not the remote runtime's.
- API-exception module allowlist widened to botocore/boto3/google/grpc/
requests/aiohttp so other adapter SDKs don't misclassify as gateway.
---
agent/conversation_loop.py | 7 +++
agent/error_surface.py | 55 +++++++++++++++----
.../assistant-ui/thread/assistant-message.tsx | 12 +++-
apps/desktop/src/i18n/ar.ts | 1 +
apps/desktop/src/i18n/en.ts | 1 +
apps/desktop/src/i18n/ja.ts | 1 +
apps/desktop/src/i18n/types.ts | 1 +
apps/desktop/src/i18n/zh-hant.ts | 1 +
apps/desktop/src/i18n/zh.ts | 1 +
apps/desktop/src/lib/error-surface.test.ts | 28 ++++++++++
apps/desktop/src/lib/error-surface.ts | 22 ++++++--
tests/agent/test_error_surface.py | 32 ++++++++++-
.../tui_gateway/test_failed_turn_retention.py | 4 ++
website/docs/user-guide/desktop.md | 5 +-
14 files changed, 152 insertions(+), 19 deletions(-)
diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py
index 3d7f07ef39..2d49501518 100644
--- a/agent/conversation_loop.py
+++ b/agent/conversation_loop.py
@@ -755,6 +755,10 @@ def _billing_failure_result(
"failed": True,
"error": summary,
"failure_reason": classified.reason.value,
+ # The classifier's own retry verdict — carried so UI surfaces
+ # (agent/error_surface.py) show Retry only when a re-run can differ,
+ # instead of re-deriving retryability from a second taxonomy.
+ "failure_retryable": bool(classified.retryable),
# The billing verdict may rest on an ambiguous body (#82154) — carry
# that through the structured result, not just the prose.
"billing_unverified": unverified,
@@ -6439,6 +6443,9 @@ def run_conversation(
# different exit code. ``rate_limit`` / ``billing`` here
# mean "quota wall, not a task error".
"failure_reason": classified.reason.value,
+ # The classifier's own retry verdict — UI surfaces use
+ # this instead of re-deriving from the reason string.
+ "failure_retryable": bool(classified.retryable),
# True when the billing verdict rests on an ambiguous
# body (#82154) — may be a content-filter rejection.
"billing_unverified": _billing_unverified,
diff --git a/agent/error_surface.py b/agent/error_surface.py
index 0f20b79900..4016a9fb3c 100644
--- a/agent/error_surface.py
+++ b/agent/error_surface.py
@@ -62,13 +62,19 @@ _TRANSPORT_REASONS = {
}
# Reasons that are deterministic for the request — a bare "Retry" repeats the
-# same failure, so clients shouldn't lead with it.
+# same failure, so clients shouldn't lead with it. Fallback only: results
+# from current backends carry the classifier's own verdict in
+# ``failure_retryable`` and never consult this set. Kept in sync with
+# ``classify_api_error``'s retryable=False verdicts.
_NON_RETRYABLE_REASONS = {
+ "auth",
"auth_permanent",
"billing",
+ "billing_unverified",
"content_policy_blocked",
"provider_policy_blocked",
"model_not_found",
+ "format_error",
"ssl_cert_verification",
}
@@ -99,11 +105,20 @@ _STREAM_DROP_FRAGMENTS = (
# Exception modules that indicate the failure came from an API/transport call
# (vs. a bug in our own dispatcher code, which is a gateway-layer failure).
+# Covers every SDK family our provider adapters raise from: OpenAI-compatible
+# (openai/httpx/httpcore), Anthropic, Bedrock (botocore/boto3), Google
+# (google.*/grpc), plus raw transports (requests/aiohttp/ssl/socket/urllib).
_API_EXC_MODULE_PREFIXES = (
"openai",
"httpx",
"httpcore",
"anthropic",
+ "botocore",
+ "boto3",
+ "google",
+ "grpc",
+ "requests",
+ "aiohttp",
"ssl",
"socket",
"urllib",
@@ -120,8 +135,22 @@ def _looks_like_stream_drop(message: str) -> bool:
return any(fragment in msg for fragment in _STREAM_DROP_FRAGMENTS)
-def _surface(layer: str, code: str, retryable: bool) -> dict:
- return {"layer": layer, "code": code, "retryable": bool(retryable)}
+def _surface(
+ layer: str,
+ code: str,
+ retryable: bool,
+ provider: str = "",
+ model: str = "",
+) -> dict:
+ out = {"layer": layer, "code": code, "retryable": bool(retryable)}
+ # The failing session's identity, captured at classification time so
+ # clients report the model/provider that actually failed — not whatever
+ # the foreground composer points at when a button is clicked later.
+ if provider:
+ out["provider"] = provider
+ if model:
+ out["model"] = model
+ return out
def build_error_surface_from_result(
@@ -147,18 +176,18 @@ def build_error_surface_from_result(
from hermes_state import is_disk_full_error
if error_text and is_disk_full_error(error_text):
- return _surface(LAYER_DISK, "disk_full", False)
+ return _surface(LAYER_DISK, "disk_full", False, provider, model)
except Exception: # pragma: no cover - defensive import guard
pass
if result.get("billing_block") or reason in ("billing", "billing_unverified"):
- return _surface(LAYER_BILLING, reason or "billing", False)
+ return _surface(LAYER_BILLING, reason or "billing", False, provider, model)
if not reason:
# Failed result without a classified reason (legacy paths).
if _looks_like_stream_drop(error_text):
- return _surface(LAYER_STREAMING, "stream_drop", True)
- return _surface(LAYER_PROVIDER, "unknown", True)
+ return _surface(LAYER_STREAMING, "stream_drop", True, provider, model)
+ return _surface(LAYER_PROVIDER, "unknown", True, provider, model)
layer = _REASON_TO_LAYER.get(reason)
if layer is None:
@@ -168,7 +197,13 @@ def build_error_surface_from_result(
layer = LAYER_STREAMING
else:
layer = LAYER_PROVIDER
- return _surface(layer, reason, reason not in _NON_RETRYABLE_REASONS)
+ # Prefer the classifier's own retry verdict when the result carries it
+ # (conversation_loop stamps ``failure_retryable`` next to
+ # ``failure_reason``); the reason-set fallback covers older results.
+ retryable = result.get("failure_retryable")
+ if not isinstance(retryable, bool):
+ retryable = reason not in _NON_RETRYABLE_REASONS
+ return _surface(layer, reason, retryable, provider, model)
except Exception: # pragma: no cover — never break the error path
logger.debug("error_surface: result classification failed", exc_info=True)
return None
@@ -191,7 +226,7 @@ def build_error_surface_from_exception(
from hermes_state import is_disk_full_error
if is_disk_full_error(exc):
- return _surface(LAYER_DISK, "disk_full", False)
+ return _surface(LAYER_DISK, "disk_full", False, provider, model)
except Exception: # pragma: no cover - defensive import guard
pass
@@ -201,7 +236,7 @@ def build_error_surface_from_exception(
)
if not api_like or not isinstance(exc, Exception):
- return _surface(LAYER_GATEWAY, type(exc).__name__, True)
+ return _surface(LAYER_GATEWAY, type(exc).__name__, True, provider, model)
from agent.error_classifier import classify_api_error
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 eaad962d41..e13be5488c 100644
--- a/apps/desktop/src/components/assistant-ui/thread/assistant-message.tsx
+++ b/apps/desktop/src/components/assistant-ui/thread/assistant-message.tsx
@@ -39,7 +39,7 @@ 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 { $currentModel } from '@/store/session'
+import { $connection, $currentModel } from '@/store/session'
import { $voicePlayback } from '@/store/voice-playback'
// Stable empty identity for the settled-parts selector — a fresh [] per render
@@ -490,6 +490,14 @@ const ErrorRecoveryActions: FC = () => {
// child mounts only when one is (see SwitchProviderAction).
const inRouter = useInRouterContext()
const model = useStore($currentModel)
+ const connection = useStore($connection)
+
+ // Open Logs reveals the LOCAL Electron profile's HERMES_HOME/logs. On a
+ // remote/cloud connection the failed turn's gateway+agent logs live on the
+ // remote box — the local folder only holds Desktop-side transport logs, so
+ // the label says "Open Desktop logs" there instead of implying it opens the
+ // runtime's logs.
+ const remoteConnection = connection?.mode === 'remote'
// 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
@@ -543,7 +551,7 @@ const ErrorRecoveryActions: FC = () => {
{showSwitchProvider && inRouter && }
{window.hermesDesktop?.logsRoot && (
)}
diff --git a/apps/desktop/src/i18n/ar.ts b/apps/desktop/src/i18n/ar.ts
index b0aec6714f..c8ebdf5e97 100644
--- a/apps/desktop/src/i18n/ar.ts
+++ b/apps/desktop/src/i18n/ar.ts
@@ -2449,6 +2449,7 @@ export const ar = defineLocale({
errorSwitchProvider: 'تبديل المزوّد',
errorOpenLogs: 'فتح السجلات',
errorOpenLogsFailed: 'تعذّر فتح مجلد السجلات',
+ errorOpenDesktopLogs: 'فتح سجلات سطح المكتب',
errorCopyDiagnostics: 'نسخ تفاصيل الخطأ',
filesChanged: count => `${count} ملفات تم تغييرها`,
reviewChanges: 'مراجعة',
diff --git a/apps/desktop/src/i18n/en.ts b/apps/desktop/src/i18n/en.ts
index 6a10372ed7..64411758e6 100644
--- a/apps/desktop/src/i18n/en.ts
+++ b/apps/desktop/src/i18n/en.ts
@@ -3094,6 +3094,7 @@ export const en: Translations = {
errorSwitchProvider: 'Switch provider',
errorOpenLogs: 'Open logs',
errorOpenLogsFailed: 'Could not open the logs folder',
+ errorOpenDesktopLogs: 'Open Desktop logs',
errorCopyDiagnostics: 'Copy error details',
filesChanged: count => (count === 1 ? '1 file changed' : `${count} files changed`),
reviewChanges: 'Review',
diff --git a/apps/desktop/src/i18n/ja.ts b/apps/desktop/src/i18n/ja.ts
index a65a18bab6..c493562019 100644
--- a/apps/desktop/src/i18n/ja.ts
+++ b/apps/desktop/src/i18n/ja.ts
@@ -2746,6 +2746,7 @@ export const ja = defineLocale({
errorSwitchProvider: 'プロバイダーを切り替え',
errorOpenLogs: 'ログを開く',
errorOpenLogsFailed: 'ログフォルダを開けませんでした',
+ errorOpenDesktopLogs: 'デスクトップのログを開く',
errorCopyDiagnostics: 'エラー詳細をコピー',
filesChanged: count => `${count} 件のファイルを変更`,
reviewChanges: 'レビュー',
diff --git a/apps/desktop/src/i18n/types.ts b/apps/desktop/src/i18n/types.ts
index dc8151982e..1cecefa8a2 100644
--- a/apps/desktop/src/i18n/types.ts
+++ b/apps/desktop/src/i18n/types.ts
@@ -2665,6 +2665,7 @@ export interface Translations {
errorSwitchProvider: string
errorOpenLogs: string
errorOpenLogsFailed: string
+ errorOpenDesktopLogs: string
errorCopyDiagnostics: string
filesChanged: (count: number) => string
reviewChanges: string
diff --git a/apps/desktop/src/i18n/zh-hant.ts b/apps/desktop/src/i18n/zh-hant.ts
index e44d2397f1..ff5145efb8 100644
--- a/apps/desktop/src/i18n/zh-hant.ts
+++ b/apps/desktop/src/i18n/zh-hant.ts
@@ -2656,6 +2656,7 @@ export const zhHant = defineLocale({
errorSwitchProvider: '切換服務商',
errorOpenLogs: '開啟日誌',
errorOpenLogsFailed: '無法開啟日誌資料夾',
+ errorOpenDesktopLogs: '開啟桌面端日誌',
errorCopyDiagnostics: '複製錯誤詳細資訊',
filesChanged: count => `${count} 個檔案已變更`,
reviewChanges: '檢視',
diff --git a/apps/desktop/src/i18n/zh.ts b/apps/desktop/src/i18n/zh.ts
index 7212cf1eec..b1dbcb07ae 100644
--- a/apps/desktop/src/i18n/zh.ts
+++ b/apps/desktop/src/i18n/zh.ts
@@ -3258,6 +3258,7 @@ export const zh: Translations = {
errorSwitchProvider: '切换服务商',
errorOpenLogs: '打开日志',
errorOpenLogsFailed: '无法打开日志文件夹',
+ errorOpenDesktopLogs: '打开桌面端日志',
errorCopyDiagnostics: '复制错误详情',
filesChanged: count => `${count} 个文件已更改`,
reviewChanges: '查看',
diff --git a/apps/desktop/src/lib/error-surface.test.ts b/apps/desktop/src/lib/error-surface.test.ts
index ea7f9bd87d..3ed04ca4b2 100644
--- a/apps/desktop/src/lib/error-surface.test.ts
+++ b/apps/desktop/src/lib/error-surface.test.ts
@@ -32,6 +32,21 @@ describe('parseErrorSurface', () => {
it('honors retryable=false', () => {
expect(parseErrorSurface({ layer: 'auth', code: 'auth_permanent', retryable: false })?.retryable).toBe(false)
})
+
+ it('carries the failing session identity when present', () => {
+ const surface = parseErrorSurface({
+ layer: 'provider',
+ code: 'rate_limit',
+ retryable: true,
+ provider: 'openrouter',
+ model: 'test/m1'
+ })
+
+ expect(surface?.provider).toBe('openrouter')
+ expect(surface?.model).toBe('test/m1')
+ // Absent identity yields no keys, not empty strings.
+ expect(parseErrorSurface({ layer: 'provider', code: 'x', retryable: true })?.provider).toBeUndefined()
+ })
})
describe('formatErrorDiagnostics', () => {
@@ -48,6 +63,19 @@ describe('formatErrorDiagnostics', () => {
expect(text).toContain('error: boom')
})
+ it('prefers the descriptor identity over the caller fallback', () => {
+ const text = formatErrorDiagnostics({
+ errorText: 'boom',
+ // Foreground composer atom — potentially stale by click time.
+ model: 'some/other-model',
+ surface: { layer: 'provider', code: 'rate_limit', retryable: true, provider: 'openrouter', model: 'failed/model' }
+ })
+
+ expect(text).toContain('provider: openrouter')
+ expect(text).toContain('model: failed/model')
+ expect(text).not.toContain('some/other-model')
+ })
+
it('omits absent fields without leaving blank lines', () => {
const text = formatErrorDiagnostics({ errorText: 'boom' })
diff --git a/apps/desktop/src/lib/error-surface.ts b/apps/desktop/src/lib/error-surface.ts
index cf41e3c480..44e4511beb 100644
--- a/apps/desktop/src/lib/error-surface.ts
+++ b/apps/desktop/src/lib/error-surface.ts
@@ -25,6 +25,11 @@ export interface ErrorSurface {
code: string
/** False when retrying unchanged reproduces the same failure. */
retryable: boolean
+ /** The failing session's provider/model, captured at classification time —
+ * preferred over the foreground composer's atoms, which can point at a
+ * different model by the time the user clicks an action. */
+ provider?: string
+ model?: string
}
/** Validate a wire payload into an ErrorSurface, or null when absent/garbled. */
@@ -33,7 +38,7 @@ export function parseErrorSurface(value: unknown): ErrorSurface | null {
return null
}
- const raw = value as { code?: unknown; layer?: unknown; retryable?: unknown }
+ const raw = value as { code?: unknown; layer?: unknown; model?: unknown; provider?: unknown; retryable?: unknown }
const layer = typeof raw.layer === 'string' ? (raw.layer as ErrorSurfaceLayer) : null
if (!layer || !ERROR_SURFACE_LAYERS.includes(layer)) {
@@ -43,7 +48,9 @@ export function parseErrorSurface(value: unknown): ErrorSurface | null {
return {
layer,
code: typeof raw.code === 'string' && raw.code ? raw.code : 'unknown',
- retryable: raw.retryable !== false
+ retryable: raw.retryable !== false,
+ ...(typeof raw.provider === 'string' && raw.provider ? { provider: raw.provider } : {}),
+ ...(typeof raw.model === 'string' && raw.model ? { model: raw.model } : {})
}
}
@@ -55,14 +62,19 @@ export function formatErrorDiagnostics(input: {
provider?: string
surface?: ErrorSurface | null
}): string {
+ // The descriptor's identity (captured when the turn failed) beats the
+ // caller-supplied fallback (typically the foreground composer's atoms).
+ const provider = input.surface?.provider || input.provider
+ const model = input.surface?.model || input.model
+
const lines = [
- '── Hermes error diagnostics ──',
+ '── Hermes error details ──',
`time: ${new Date().toISOString()}`,
input.surface ? `layer: ${input.surface.layer}` : null,
input.surface ? `code: ${input.surface.code}` : null,
input.surface ? `retryable: ${input.surface.retryable}` : null,
- input.provider ? `provider: ${input.provider}` : null,
- input.model ? `model: ${input.model}` : null,
+ provider ? `provider: ${provider}` : null,
+ model ? `model: ${model}` : null,
input.appVersion ? `app: ${input.appVersion}` : null,
`error: ${input.errorText}`
]
diff --git a/tests/agent/test_error_surface.py b/tests/agent/test_error_surface.py
index e17dcfd474..3a9b735d15 100644
--- a/tests/agent/test_error_surface.py
+++ b/tests/agent/test_error_surface.py
@@ -41,8 +41,10 @@ def test_result_none_for_healthy_result():
def test_result_auth_reasons_map_to_auth_layer():
+ # Both auth reasons are non-retryable, matching classify_api_error's own
+ # verdict (a bare retry replays the same rejected credential).
surface = build_error_surface_from_result(_failed_result("auth"))
- assert surface == {"layer": LAYER_AUTH, "code": "auth", "retryable": True}
+ assert surface == {"layer": LAYER_AUTH, "code": "auth", "retryable": False}
surface = build_error_surface_from_result(_failed_result("auth_permanent"))
assert surface["layer"] == LAYER_AUTH
@@ -77,6 +79,8 @@ def test_result_provider_default_for_classified_reasons():
def test_result_non_retryable_reasons():
for reason in (
+ "auth",
+ "format_error",
"content_policy_blocked",
"model_not_found",
"ssl_cert_verification",
@@ -85,6 +89,32 @@ def test_result_non_retryable_reasons():
assert surface["retryable"] is False, reason
+def test_result_prefers_classifier_retry_verdict():
+ """conversation_loop stamps ``failure_retryable`` from the real
+ ClassifiedError — it must win over the fallback reason set."""
+ surface = build_error_surface_from_result(
+ _failed_result("unknown", failure_retryable=False)
+ )
+ assert surface["retryable"] is False
+
+ surface = build_error_surface_from_result(
+ _failed_result("format_error", failure_retryable=True)
+ )
+ assert surface["retryable"] is True
+
+
+def test_result_stamps_failing_session_identity():
+ surface = build_error_surface_from_result(
+ _failed_result("rate_limit"), provider="openrouter", model="test/m1"
+ )
+ assert surface["provider"] == "openrouter"
+ assert surface["model"] == "test/m1"
+
+ # Absent identity omits the keys instead of stamping empty strings.
+ surface = build_error_surface_from_result(_failed_result("rate_limit"))
+ assert "provider" not in surface and "model" not in surface
+
+
def test_result_timeout_on_custom_endpoint_is_endpoint_layer():
surface = build_error_surface_from_result(
_failed_result("timeout"), provider="custom"
diff --git a/tests/tui_gateway/test_failed_turn_retention.py b/tests/tui_gateway/test_failed_turn_retention.py
index 455fcc22d5..30987d5a9e 100644
--- a/tests/tui_gateway/test_failed_turn_retention.py
+++ b/tests/tui_gateway/test_failed_turn_retention.py
@@ -197,6 +197,10 @@ def test_returned_error_result_carries_error_surface(emits, turn_env):
"layer": "provider",
"code": "rate_limit",
"retryable": True,
+ # The failing session's identity rides the descriptor so clients
+ # report the model that actually failed, not the composer's current.
+ "provider": "openrouter",
+ "model": "test/model",
}
snapshot = server._inflight_snapshot(session)
diff --git a/website/docs/user-guide/desktop.md b/website/docs/user-guide/desktop.md
index 80ade53afd..c6d7439a6a 100644
--- a/website/docs/user-guide/desktop.md
+++ b/website/docs/user-guide/desktop.md
@@ -420,7 +420,10 @@ generic error toast. The card offers recovery actions matched to the failure:
deterministically reproduce the failure, e.g. a content-policy rejection).
- **Switch provider** — jumps to Settings → Models for provider, endpoint,
auth, and billing failures.
-- **Open logs** — opens `HERMES_HOME/logs` in your file manager.
+- **Open logs** — opens `HERMES_HOME/logs` in your file manager. On a remote
+ or Cloud connection the button reads **Open Desktop logs**: it opens the
+ local Desktop-side logs (transport evidence), since the failed turn's
+ gateway/agent logs live on the remote machine.
- **Copy error details** — copies a compact plain-text summary (layer, code,
provider/model, error message) you can paste into a bug report or Discord.