Skip to content

fix(desktop): only treat real 401/403 as OAuth session expiry - #59238

Closed
33hodl wants to merge 1 commit into
NousResearch:mainfrom
33hodl:fix/oauth-ws-ticket-misclassification
Closed

fix(desktop): only treat real 401/403 as OAuth session expiry#59238
33hodl wants to merge 1 commit into
NousResearch:mainfrom
33hodl:fix/oauth-ws-ticket-misclassification

Conversation

@33hodl

@33hodl 33hodl commented Jul 5, 2026

Copy link
Copy Markdown

Summary

Previously, any failure during POST /api/auth/ws-ticket (including timeouts, 5xx, and transient network errors) was wrapped as a GatewayReauthRequiredError and surfaced to the user as:

"Your remote gateway session has expired. Open Settings → Gateway and click 'Sign in' again."

This caused false-positive "session expired" messages on healthy but occasionally slow remote OAuth gateways.

Root cause

resolveGatewayWsUrl, resolveTestWsUrl, and buildRemoteConnection treated every ticket-mint error as an auth failure, without distinguishing real auth failures (401/403) from transport/runtime failures (timeouts, 5xx, connection resets).

Fix

Introduced two small helpers:

  • isGatewayAuthFailure (shared)
  • isOauthSessionAuthFailure (electron)

These only match actual auth failures (statusCode 401/403 or equivalent message patterns). Non-auth failures now propagate unchanged.

Verification

  • node --check apps/desktop/electron/main.cjs
  • node --check apps/desktop/electron/connection-config.cjs
  • node --test apps/desktop/electron/connection-config.test.cjs → 52 passed
  • cd apps/desktop && npx vitest run src/lib/gateway-ws-url.test.ts → 13 passed

Impact

  • Real expired/revoked sessions still correctly prompt re-login.
  • Timeouts and gateway health issues now surface as connectivity problems instead of misleading reauth prompts.

Previously, any error during ws-ticket minting (including timeouts and
5xx responses) was turned into a 'session expired, sign in again' error.
This caused false-positive 'remote gateway session has expired' messages
when the gateway was reachable but slow or temporarily unhealthy.

- Add isGatewayAuthFailure / isOauthSessionAuthFailure helpers that only
  match statusCode 401/403 or equivalent message patterns.
- Non-auth failures now propagate unchanged instead of being wrapped as
  GatewayReauthRequiredError.
- Updated tests to cover both the auth-failure path and the transport-
  failure path.

This directly addresses the misclassification seen in Desktop boot logs
where 'Cached remote Hermes backend failed liveness probe' was followed
by a spurious reauth prompt.

Fixes the root cause of many false 'sign in again' prompts on remote
OAuth gateways.
@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have labels Jul 5, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for isolating the false-positive reauthentication path and adding focused tests.

Problems

  • Current Desktop guidance intentionally defines an OAuth WebSocket-ticket mint failure as reauthentication-required (apps/desktop/AGENTS.md:127-130). The current shared reconnect resolver preserves that behavior at apps/shared/src/websocket-url.ts:45-51; Electron boot and the connection test path do the same at apps/desktop/electron/main.ts:6014-6023 and apps/desktop/electron/connection-config.ts:128-138.
  • The patch targets CJS Electron files that no longer exist on current main; the corresponding implementation is now TypeScript (39d09453f, feat(desktop): ts-ify everything).

Suggested changes

  • Please obtain maintainer direction on changing the documented mint-failure contract before porting this. If approved, rework it against the current TypeScript paths while retaining the no-stale-ticket fallback invariant.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
@33hodl

33hodl commented Jul 17, 2026

Copy link
Copy Markdown
Author

Closing this superseded OAuth-classification PR in favor of the consolidated, rebased implementation in #64673: #64673. The consolidated PR includes the 401/403-only auth classification, bounded transport retry, fresh ticket resolution, session recovery, and the updated Desktop guidance.

@33hodl 33hodl closed this Jul 17, 2026
@teknium1 teknium1 added the area/sessions Session lifecycle, resume, persistence, history label Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants