From 0b8fd31c89d487ab8aa38ea9a33819de4f2c1104 Mon Sep 17 00:00:00 2001 From: Jamie Pine Date: Sat, 4 Jul 2026 21:07:59 -0700 Subject: [PATCH] Address review feedback on cloud login MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - time out status polling after 2 min so an abandoned browser flow doesn't leave the button stuck on "Waiting for browser…" - handle non-JSON / non-object payloads from the exchange and account endpoints instead of 500ing after the state is consumed - make singleton row creation race-safe (IntegrityError -> re-query) - clear device_name on disconnect along with the rest of the metadata - serve the dashboard URL from /cloud/status so the Manage link follows VOICEBOX_CLOUD_URL instead of hardcoding production - keep a "Disconnecting…" label on the disconnect button while pending --- app/src/components/ServerTab/CloudSection.tsx | 26 ++++++++++++++-- app/src/lib/api/types.ts | 1 + backend/models.py | 1 + backend/services/cloud.py | 31 ++++++++++++++++--- 4 files changed, 52 insertions(+), 7 deletions(-) diff --git a/app/src/components/ServerTab/CloudSection.tsx b/app/src/components/ServerTab/CloudSection.tsx index bbede3f4..2821261e 100644 --- a/app/src/components/ServerTab/CloudSection.tsx +++ b/app/src/components/ServerTab/CloudSection.tsx @@ -33,6 +33,22 @@ export function CloudSection() { } }, [connected, polling, status?.device_name, toast]); + // Give up after two minutes so an abandoned browser flow doesn't leave the + // button stuck on "Waiting for browser…". The backend state stays valid for + // ten, so the user can simply start again. + useEffect(() => { + if (!polling) return; + const timeoutId = window.setTimeout(() => { + setPolling(false); + toast({ + title: 'Sign-in timed out', + description: 'The browser sign-in was not completed. Try again.', + variant: 'destructive', + }); + }, 120_000); + return () => window.clearTimeout(timeoutId); + }, [polling, toast]); + const startLogin = useMutation({ mutationFn: () => apiClient.startCloudLogin(), onSuccess: () => { @@ -56,7 +72,8 @@ export function CloudSection() { queryClient.invalidateQueries({ queryKey: ['cloud-status'] }); toast({ title: 'Disconnected', - description: 'This device is no longer linked. The key stays valid until revoked in your account.', + description: + 'This device is no longer linked. The key stays valid until revoked in your account.', }); }, onError: (error: Error) => @@ -88,7 +105,10 @@ export function CloudSection() { variant="outline" > {disconnect.isPending ? ( - + <> + + Disconnecting… + ) : ( 'Disconnect' )} @@ -118,7 +138,7 @@ export function CloudSection() { > diff --git a/app/src/lib/api/types.ts b/app/src/lib/api/types.ts index 9a2c005f..f2c6fae1 100644 --- a/app/src/lib/api/types.ts +++ b/app/src/lib/api/types.ts @@ -534,4 +534,5 @@ export interface CloudStatus { account_user_id: string | null; key_prefix: string | null; connected_at: string | null; + dashboard_url: string; } diff --git a/backend/models.py b/backend/models.py index 4f608f40..2730492f 100644 --- a/backend/models.py +++ b/backend/models.py @@ -813,3 +813,4 @@ class CloudStatusResponse(BaseModel): account_user_id: Optional[str] = None key_prefix: Optional[str] = None connected_at: Optional[datetime] = None + dashboard_url: str diff --git a/backend/services/cloud.py b/backend/services/cloud.py index 1b3983b4..1485b4df 100644 --- a/backend/services/cloud.py +++ b/backend/services/cloud.py @@ -19,6 +19,7 @@ import webbrowser from urllib.parse import urlencode import httpx +from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session from .. import config @@ -41,6 +42,15 @@ def _prune() -> None: _pending.pop(state, None) +def _json_dict(response: httpx.Response) -> dict | None: + """Parsed JSON body, or None when it isn't a JSON object.""" + try: + payload = response.json() + except ValueError: + return None + return payload if isinstance(payload, dict) else None + + def _consume_state(state: str) -> bool: """Validate and single-use-consume a pending state.""" _prune() @@ -84,7 +94,10 @@ async def handle_callback(db: Session, code: str, state: str) -> tuple[bool, str if exchanged.status_code != 200: logger.warning("cloud exchange rejected code: %s", exchanged.status_code) return False, "Could not complete sign-in — the code was rejected." - payload = exchanged.json() + payload = _json_dict(exchanged) + if payload is None: + logger.warning("cloud exchange returned a non-JSON payload") + return False, "Voicebox Cloud returned an unexpected response." api_key = payload.get("key") device_name = payload.get("label") if not api_key: @@ -98,7 +111,9 @@ async def handle_callback(db: Session, code: str, state: str) -> tuple[bool, str if me.status_code != 200: logger.warning("minted key failed verification: %s", me.status_code) return False, "Sign-in succeeded but the key could not be verified." - account_user_id = (me.json().get("data") or {}).get("userId") + # The 200 above proves the key works; the user id is best-effort. + data = (_json_dict(me) or {}).get("data") + account_user_id = data.get("userId") if isinstance(data, dict) else None except httpx.HTTPError: logger.exception("network error during cloud exchange") return False, "Could not reach Voicebox Cloud. Check your connection and try again." @@ -113,8 +128,14 @@ def _get_or_create_row(db: Session) -> DBCloudSettings: if row is None: row = DBCloudSettings(id=SINGLETON_ID) db.add(row) - db.commit() - db.refresh(row) + try: + db.commit() + except IntegrityError: + # Another request created the singleton concurrently. + db.rollback() + row = db.query(DBCloudSettings).filter(DBCloudSettings.id == SINGLETON_ID).one() + else: + db.refresh(row) return row @@ -141,6 +162,7 @@ def get_status(db: Session) -> dict: "account_user_id": row.account_user_id if connected else None, "key_prefix": key_prefix, "connected_at": row.connected_at if connected else None, + "dashboard_url": f"{config.get_cloud_web_url()}/account", } @@ -149,6 +171,7 @@ def disconnect(db: Session) -> None: revoked from the account dashboard — surface that in the UI.""" row = _get_or_create_row(db) row.api_key = None + row.device_name = None row.account_user_id = None row.connected_at = None db.commit()