From 18348e1491a119ac15071d91ab7a0fbc89dd6d77 Mon Sep 17 00:00:00 2001 From: Zoltan Papp Date: Sun, 31 May 2026 04:26:15 +0200 Subject: [PATCH] client+ui: remove SSO handoff flicker and clean up abandoned login via context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups to the "hold NeedsLogin during the SSO browser wait" change. Both target the visible state churn the tray showed during the auto-login handoff (Connect / profile-switch lands on NeedsLogin -> the UI's startLogin kicks off the SSO flow) and the broken recovery after the user dismisses the browser-login popup with the window's X. Background ---------- When a connect attempt lands on NeedsLogin, the UI's startLogin() drives the SSO flow: Connection.Login() -> (NeedsSSOLogin) open the browser-login popup -> Connection.WaitSSOLogin() blocks until the browser leg completes. The tray and the React status page both paint the raw daemon status, so any transient state the daemon publishes during this handoff is visible as a flicker. Previously the handoff churned the daemon status through NeedsLogin -> Idle -> Connecting -> NeedsLogin which read as a flicker on the tray icon and the status dot. Two distinct sources produced the two intermediate states: * Idle came from the UI's defensive cli.Down() at the top of Connection.Login (services/connection.go): it tore the engine down before every login to dislodge a possibly-parked WaitSSOLogin, emitting a StatusIdle on the way. * Connecting came from server.go Login() unconditionally setting StatusConnecting before deciding whether the request is an SSO flow (which immediately returns NeedsLogin) or a setup-key flow (which actually dials Management). Changes ------- 1. server.go Login(): only set StatusConnecting on the setup-key path, where we are about to dial Management with the key and the Connecting paint is meaningful. The SSO path returns NeedsLogin and parks on the browser leg, so it no longer flashes Connecting first. Removes the Connecting blip. 2. services/connection.go Login(): drop the pre-Login cli.Down(). The daemon already dislodges a pending WaitSSOLogin at Login entry (actCancel), and an abandoned browser leg is now torn down by cancelling the WaitSSOLogin RPC (see 3/4). Removing the Down removes the Idle blip on every login. 3. MainConnectionStatusSwitch.tsx startLogin(): on cancel (the browser-login popup's Cancel button or its window X, both routed through EventBrowserLoginCancel), cancel the in-flight WaitSSOLogin gRPC call via waitPromise.cancel() instead of issuing a heavy Connection.Down(). The daemon ties the wait to this call's context, so cancelling the call ends the wait cleanly with no engine teardown and no Idle paint. 4. server.go WaitSSOLogin(): when the wait unblocks with context.Canceled and the cancellation came from our caller (callerCtx.Err() != nil — the client cancelled the RPC or went away), clear the cached oauthAuthFlow so a fresh Login starts a new device code instead of reusing the abandoned one. The entry NeedsLogin stays in place, so a reattaching client still shows the login affordance. An internal abort (actCancel fired by a newer Login/WaitSSOLogin while our callerCtx is still live) is left untouched so the new owner's flow is not clobbered. Effect ------ The auto-login handoff now goes Connected -> Connecting -> NeedsLogin and holds, with no Idle/Connecting flicker in between. Dismissing the browser-login popup with X now recovers the same way as the Cancel button: the WaitSSOLogin RPC is cancelled, the stale OAuth flow is cleared, and the next connect opens a fresh browser-login window instead of getting stuck. --- client/server/server.go | 24 ++++++++++++++----- .../main/MainConnectionStatusSwitch.tsx | 15 ++++++------ client/ui/services/connection.go | 17 ++++++------- 3 files changed, 33 insertions(+), 23 deletions(-) diff --git a/client/server/server.go b/client/server/server.go index 82f683cde..22165fbcf 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -539,8 +539,6 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro return &proto.LoginResponse{}, nil } - state.Set(internal.StatusConnecting) - if msg.SetupKey == "" { hint := "" if msg.Hint != nil { @@ -592,6 +590,11 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro }, nil } + // Setup-key path: we are about to dial Management with the key, so the + // Connecting paint is meaningful here — unlike the SSO branch above, + // which returns NeedsLogin and parks on the browser leg. + state.Set(internal.StatusConnecting) + if loginStatus, err := s.loginAttempt(ctx, msg.SetupKey, ""); err != nil { state.Set(loginStatus) return nil, err @@ -717,10 +720,19 @@ func (s *Server) WaitSSOLogin(callerCtx context.Context, msg *proto.WaitSSOLogin s.mutex.Unlock() switch { case errors.Is(err, context.Canceled): - // External abort (profile switch, app quit, another - // WaitSSOLogin started). Not a login failure — let the - // top-level defer fall through to StatusIdle so the next - // flow starts from a clean state. + // External abort. If our caller cancelled (the client closed + // the browser-login popup, or the UI went away — callerCtx is + // done), clear the abandoned OAuth flow so a fresh Login starts + // a new device code instead of reusing this one. The entry + // NeedsLogin stays in place, so a reattaching client shows the + // login affordance. An internal abort (actCancel from a new + // Login/WaitSSOLogin, callerCtx still live) leaves the flow for + // the new owner — don't clobber it. + if callerCtx.Err() != nil { + s.mutex.Lock() + s.oauthAuthFlow = oauthAuthFlow{} + s.mutex.Unlock() + } case errors.Is(err, context.DeadlineExceeded): // OAuth device-code window expired with no user action. // Retryable — leave the daemon in NeedsLogin so the UI diff --git a/client/ui/frontend/src/modules/main/MainConnectionStatusSwitch.tsx b/client/ui/frontend/src/modules/main/MainConnectionStatusSwitch.tsx index b1f4fd393..18f36f45f 100644 --- a/client/ui/frontend/src/modules/main/MainConnectionStatusSwitch.tsx +++ b/client/ui/frontend/src/modules/main/MainConnectionStatusSwitch.tsx @@ -89,13 +89,14 @@ async function startLogin(): Promise { } if (cancelled) { - // Tell the daemon to drop the in-flight WaitSSOLogin so a - // future Login starts fresh; see services/connection.go:74. - try { - await Connection.Down(); - } catch (e) { - console.error(e); - } + // Cancel the in-flight WaitSSOLogin gRPC instead of a heavy + // Down. The daemon ties the wait to this call's context + // (server.go WaitSSOLogin), so cancelling ends the wait and + // clears the abandoned OAuth flow — a fresh Login then starts + // a new device code, with no Idle blink on the tray. Swallow + // the cancellation rejection on the abandoned promise. + waitPromise.cancel?.(); + void waitPromise.catch(() => {}); return; } } diff --git a/client/ui/services/connection.go b/client/ui/services/connection.go index 4499d3c42..95d98aa74 100644 --- a/client/ui/services/connection.go +++ b/client/ui/services/connection.go @@ -199,16 +199,13 @@ func (s *Connection) Login(ctx context.Context, p LoginParams) (LoginResult, err return LoginResult{}, err } - // Reset the daemon's connection loop before kicking off a new login. - // If a previous Login left a WaitSSOLogin pending (user closed the - // browser without completing the flow), the daemon stays parked on the - // old UserCode and replies with "invalid setup-key or no sso information - // provided" to a fresh Login. Calling Down first dislodges that state; - // we ignore the error since Down on an already-idle daemon is a no-op. - if _, derr := cli.Down(ctx, &proto.DownRequest{}); derr != nil { - // Down failed — likely because the daemon is already idle. Continue. - _ = derr - } + // No pre-Login Down: the daemon's Login dislodges a pending WaitSSOLogin + // itself (server.go cancels the in-flight wait via actCancel), and an + // abandoned browser leg is torn down by startLogin cancelling the + // WaitSSOLogin RPC, which the daemon reacts to by clearing the stale + // OAuth flow. A defensive Down here would only add a visible Idle blink + // to the tray during the SSO handoff (Connect/profile-switch → + // NeedsLogin → auto-login) for no gain. // Mirror the Fyne client's defaulting: when the frontend doesn't supply // profile / username, fall back to the daemon's active profile and the