From b8abad2b4f21bb9f19594fd1ec83d4acda080228 Mon Sep 17 00:00:00 2001 From: amosblomqvist Date: Sat, 24 Oct 2026 14:02:54 +0300 Subject: [PATCH] Refine ChatGPT setup and model provider settings --- components/model-picker.tsx | 1 - components/settings/agent-section.tsx | 2 + components/settings/chatgpt-connection.tsx | 266 ++++++++++++++++----- components/settings/keys-section.tsx | 2 +- components/settings/settings-dialog.tsx | 13 +- components/setup-dialog.tsx | 6 +- electron/chatgpt.cjs | 6 +- tests/chatgpt-connection-ui.test.tsx | 172 +++++++++++++ tests/chatgpt-ui.test.tsx | 4 +- tests/render-settings.test.tsx | 7 +- tests/zero-keys.test.tsx | 3 +- 11 files changed, 402 insertions(+), 80 deletions(-) create mode 100644 tests/chatgpt-connection-ui.test.tsx diff --git a/components/model-picker.tsx b/components/model-picker.tsx index 3c7a142..a442d69 100644 --- a/components/model-picker.tsx +++ b/components/model-picker.tsx @@ -311,7 +311,6 @@ export function ModelControl({ chatId }: { chatId?: string }) { return (
- {effectiveProvider === "chatgpt" && Using ChatGPT plan} + {slot !== "strong" && } {}, + manage: (_id: string) => {}, + select: (_id: string) => {}, + dismissWarning: () => {}, +}); -/** Status/control surface only; OAuth URLs, codes, and tokens never reach React. */ -export function ChatGPTConnection() { +/** UI lifetime spans setup's keys → models transition. Only existing safe controls + * are used; account IDs remain control values, never displayed account labels. */ +export function ChatGPTUIProvider({ children }: { children: React.ReactNode }) { const [status, setStatus] = useState(empty); - const [selected, setSelected] = useState(""); + const [dialog, setDialog] = useState(null); const [busy, setBusy] = useState(false); const [error, setError] = useState(null); + const [dismissedError, setDismissedError] = useState(null); + const [background, setBackground] = useState(false); + const [registration, setRegistration] = useState(""); + const operation = useRef<"login" | "change" | "logout" | null>(null); + const generation = useRef(0); + const statusSequence = useRef(0); + const mounted = useRef(false); + function publishStatus(next: ChatGPTStatus) { statusSequence.current++; setStatus(next); } useEffect(() => { + mounted.current = true; const bridge = window.lh?.chatgpt; - if (!bridge) return; - let mounted = true; const refresh = async () => { + const request = ++statusSequence.current; try { - const next = await bridge.status(); - if (!mounted) return; - setStatus(next); - setSelected((old) => old || next.activeId || ""); - } catch { if (mounted) setError("The desktop connection is unavailable. Retry when the app is ready."); } + const next = await bridge?.status(); + if (next && mounted.current && request === statusSequence.current) setStatus(next); + } catch { if (mounted.current) setError("The desktop connection is unavailable. Retry when the app is ready."); } }; void refresh(); - const off = bridge.onChange(() => void refresh()); - return () => { mounted = false; off(); }; + const off = bridge?.onChange(() => void refresh()); + return () => { mounted.current = false; generation.current++; off?.(); }; }, []); - const account = status.accounts.find((a) => a.id === selected); - async function control(action: () => Promise) { - if (busy) return; - setBusy(true); + + const managed = dialog?.kind === "manage" ? status.accounts.find((a) => a.id === dialog.accountId && a.connected) : undefined; + const visibleError = error || (status.error !== dismissedError ? status.error : null) || status.catalogError || null; + const warning = visibleError === REVOCATION_WARNING ? visibleError : null; + function openConnect() { + setBackground(false); + setRegistration(""); setError(null); + setDismissedError(status.error); + setDialog({ kind: "connect" }); + } + function close() { + if (operation.current === "logout" || operation.current === "change") return; + generation.current++; + setDialog(null); + setError(null); + setDismissedError(status.error); + if (operation.current === "login") { + // Closing an in-flight sign-in cancels it and never applies the draft consent. + void window.lh?.chatgpt?.cancel().then((next) => { + if (mounted.current) { publishStatus(next); setDismissedError(next.error); } + }).catch(() => {}); + } + } + async function control(kind: "change" | "logout", action: () => Promise) { + if (operation.current) return; + operation.current = kind; + setBusy(true); setError(null); setDismissedError(null); try { const next = await action(); - if (typeof next.available === "boolean") setStatus(next); - setError(next.error ?? null); - } catch { setError("The ChatGPT connection could not be changed. Try again."); } - finally { setBusy(false); } + if (!mounted.current) return; + publishStatus(next); + if (kind === "logout" && !next.accounts.find((a) => a.id === status.activeId && a.connected)) { + setDialog(next.error === REVOCATION_WARNING ? { kind: "warning" } : null); + } else setError(next.error); + } catch { if (mounted.current) setError("The ChatGPT connection could not be changed. Try again."); } + finally { operation.current = null; if (mounted.current) setBusy(false); } + } + async function login() { + const bridge = window.lh?.chatgpt; + if (!bridge || operation.current || !status.available) return; + const attempt = ++generation.current; + const allowBackground = background; + operation.current = "login"; + setBusy(true); setError(null); setDismissedError(null); + try { + let next = await bridge.login(registration || undefined); + if (!mounted.current || attempt !== generation.current) return; + publishStatus(next); + const account = next.accounts.find((a) => a.id === next.activeId && a.connected); + if (account && !next.error) { + // Explicit initial choice is applied only after verified successful login. + // Choosing OFF also revokes any prior consent on a returning registration. + if (account.background !== allowBackground) { + operation.current = "change"; + next = await bridge.consent(account.id, allowBackground); + } + if (!mounted.current || attempt !== generation.current) return; + publishStatus(next); + if (!next.error) setDialog(null); + } + setError(next.error); + } catch { if (mounted.current && attempt === generation.current) setError("The ChatGPT connection could not be changed. Try again."); } + finally { operation.current = null; if (mounted.current) setBusy(false); } } const bridge = typeof window !== "undefined" ? window.lh?.chatgpt : undefined; - return ( -
-
- ChatGPT - {status.pending ? "Waiting for browser sign-in…" : status.accounts.some((a) => a.id === status.activeId && a.connected) ? "ChatGPT plan connection" : "Not connected"} -
-

{CHATGPT_NOTICE}

- - {status.accounts.length > 0 && ( - - )} -
- {/* Official black/white format and unmodified OpenAI-provided mark. */} - - {status.pending && } - {account?.connected && selected !== status.activeId && } - {account && } -
- {!status.available &&

ChatGPT sign-in is available only in the desktop app with secure OS credential storage. API-key connections remain available here.

} - {account && ( - - )} - {(error || status.error || status.catalogError) &&

{error || status.error || status.catalogError}

} + } + {managed &&

{managed.email || "ChatGPT account"}

} +

{CHATGPT_NOTICE}

+ + {(dialog.kind === "connect" || managed) &&
+ { + if (dialog.kind === "connect") setBackground(enabled); + else if (managed && bridge) void control("change", () => bridge.consent(managed.id, enabled)); + }} /> + Allow background use{CHATGPT_BACKGROUND_HINT} +
} + {!status.available &&

ChatGPT sign-in is available only in the desktop app with secure OS credential storage. API-key connections remain available here.

} + {status.pending &&

Waiting for browser sign-in…

} + {context.error &&

{context.error}

} +
+ + {dialog.kind === "connect" && void login()} disabled={!status.available || busy || status.pending} testid="chatgpt-proceed" />} + {managed && } +
+ } + } + ; +} + +function ContinueButton({ onClick, disabled, testid = "chatgpt-login" }: { onClick: () => void; disabled: boolean; testid?: string }) { + return ; +} + +function AccountDialog({ children, onClose, busy }: { children: React.ReactNode; onClose: () => void; busy: boolean }) { + const ref = useRef(null); + const closeRef = useRef(onClose); + closeRef.current = onClose; + const backdropDismiss = useBackdropDismiss(onClose); + useEffect(() => { + const previous = document.activeElement as HTMLElement | null; + const focusable = () => [...(ref.current?.querySelectorAll('button:not([disabled]), a[href], input:not([disabled]), select:not([disabled])') ?? [])]; + focusable()[0]?.focus(); + const key = (event: KeyboardEvent) => { + if (event.key === "Escape") { + event.preventDefault(); event.stopImmediatePropagation(); closeRef.current(); + } else if (event.key === "Tab") { + const items = focusable(), first = items[0], last = items.at(-1); + if (!first) { event.preventDefault(); return; } + if (event.shiftKey && (document.activeElement === first || !ref.current?.contains(document.activeElement))) { event.preventDefault(); last?.focus(); } + else if (!event.shiftKey && (document.activeElement === last || !ref.current?.contains(document.activeElement))) { event.preventDefault(); first.focus(); } + } + }; + // Capture claims the nested dialog's Escape before outer Settings/setup. + document.addEventListener("keydown", key, true); + return () => { document.removeEventListener("keydown", key, true); if (previous?.isConnected) previous.focus(); }; + }, []); + return createPortal(
+
event.stopPropagation()} className="settings-dialog-pop mx-4 max-h-[85vh] w-full max-w-lg overflow-y-auto rounded-md border border-border bg-card p-5 shadow-xl">{children}
+
, document.body); +} + +/** Compact settings row, or setup's secondary action only. */ +export function ChatGPTConnection({ setup = false }: { setup?: boolean }) { + const ui = useContext(Context); + const active = ui.status.accounts.find((a) => a.id === ui.status.activeId && a.connected); + const accounts = ui.status.accounts.filter((a) => a.connected); + return
+
+ {!setup && ChatGPT} + {active && !setup ? <> + + + : }
- ); + {ui.warning &&
{ui.warning}
} +
; +} + +/** Contextual consent discovery belongs beside background model choices only. */ +export function ChatGPTBackgroundAccess({ provider }: { provider: string | undefined }) { + const ui = useContext(Context); + const account: Account | undefined = ui.status.accounts.find((a) => a.id === ui.status.activeId && a.connected); + if (provider !== "chatgpt" || !account || account.background) return null; + return ; } diff --git a/components/settings/keys-section.tsx b/components/settings/keys-section.tsx index 21388eb..8e5e864 100644 --- a/components/settings/keys-section.tsx +++ b/components/settings/keys-section.tsx @@ -413,7 +413,6 @@ export function KeysSection({ return ( <> - {providerRows.map((status) => ( void saveNewProvider(adding, v)} onCancel={() => setAdding(null)} error={addError} />
)} +
diff --git a/components/settings/settings-dialog.tsx b/components/settings/settings-dialog.tsx index 5026908..f550252 100644 --- a/components/settings/settings-dialog.tsx +++ b/components/settings/settings-dialog.tsx @@ -49,13 +49,14 @@ import { KeysSection } from "./keys-section.tsx"; import { Hint } from "@/components/ui/hint"; import { AppearanceSection, GraphSection } from "./appearance-section.tsx"; import { KeyboardSection } from "./keyboard-section.tsx"; +import { ChatGPTUIProvider } from "./chatgpt-connection.tsx"; /** Owner (2026-09-20): hints earn their place — mechanics-explaining * prose is out; only the keyboard note stays (storage location is a * real question users ask). */ const SECTIONS = [ + { id: "keys", label: "Model providers" }, { id: "general", label: "General" }, - { id: "keys", label: "Connections" }, { id: "agent", label: "Agent" }, { id: "appearance", label: "Appearance" }, { id: "graph", label: "Graph" }, @@ -83,7 +84,7 @@ export function SettingsDialogProvider({ children: React.ReactNode; }) { const [show, setShow] = useState(false); - const [landOn, setLandOn] = useState("general"); + const [landOn, setLandOn] = useState("keys"); const [landOnRow, setLandOnRow] = useState(null); const open = useCallback((section?: string, row?: string) => { if (section) setLandOn(section as SectionId); @@ -91,12 +92,12 @@ export function SettingsDialogProvider({ setShow(true); }, []); return ( - + {children} {show && ( setShow(false)} initialSection={landOn} initialRow={landOnRow} /> )} - + ); } @@ -133,7 +134,7 @@ function SettingsDialog({ * (portals render nothing under renderToStaticMarkup). */ export function SettingsDialogBody({ onClose, - initialSection = "general", + initialSection = "keys", initialRow = null, }: { onClose: () => void; @@ -376,7 +377,7 @@ export function SettingsDialogBody({ )} {active === "keys" && ( - + )} diff --git a/components/setup-dialog.tsx b/components/setup-dialog.tsx index d5e2bb7..2372d01 100644 --- a/components/setup-dialog.tsx +++ b/components/setup-dialog.tsx @@ -44,7 +44,7 @@ import { notifySettingsSaved, useSettingsSaved } from "@/components/settings-sig import { Hint } from "@/components/ui/hint"; import { type ProviderKeyInfo } from "@/components/settings/keys-section"; import { ProviderMenuList } from "@/components/settings/model-menu"; -import { ChatGPTConnection } from "@/components/settings/chatgpt-connection"; +import { ChatGPTBackgroundAccess, ChatGPTConnection } from "@/components/settings/chatgpt-connection"; import { defaultOrderLevel, populateLevel, THINKING_LABEL } from "@/components/thinking-default"; import { ThinkingMenuList } from "@/components/settings/model-menu"; import type { ThinkingLevel } from "@earendil-works/pi-agent-core"; @@ -148,8 +148,6 @@ export function SetupKeysBody({ }) { return (
- -

Or add an API key.

Choose your AI provider and paste its API key. You’ll pick your models next.

@@ -213,6 +211,7 @@ export function SetupKeysBody({

)}
+
); } @@ -478,6 +477,7 @@ export function SetupDialog({ )} {SLOT_META[slot].roster} + {slot !== "strong" && }