From ba0cae1aff53ba3c0348887918c8e2136822454a Mon Sep 17 00:00:00 2001 From: Lee Jackson <130007945+Imagineer99@users.noreply.github.com> Date: Fri, 15 May 2026 23:17:03 +0100 Subject: [PATCH] Stop: drop Ollama API key, clean up code execution UI (#5464) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chat: drop Ollama API key, clean up code execution UI * studio/chat: fix undefined candidateId + keyboard a11y on container list - Auto-bind effect referenced `candidateId`, which is not declared in this scope (only `candidate` is) — would fail the TS/Next build. Use `candidate.id` to match the variable that's actually defined. - Container list items get `role="button"` when `canActivate` is true but had no keyboard activation. Add `onKeyDown` for Enter/Space and `tabIndex={0}` so the row is focusable and activatable from the keyboard, matching the existing onClick behavior. * studio/chat: restore declarations dropped by the main merge The 75646444d auto-merge with main (#5466) silently dropped the declarations a4f19171c added in regions #5466 also rewrote, while leaving the usages further down in the file. No textual conflict markers, but the result referenced undeclared names: - REFRESH_POLL_MS constant (drives the 30s list refresh interval). - pendingDelete / setPendingDelete / deleting / setDeleting state (drives the in-sheet AlertDialog delete confirm — replaces the window.confirm() that landed via #5466). - Per-row locals inside the container list .map callback: running, isActive (recomputed with running), ttlMinutes, canActivate, statusLabel (drive click-to-activate, expired/active badges, and the muted styling for expired containers). Also wire setDeleting(false) + setPendingDelete(null) into the confirmDelete finally so the AlertDialog closes after the delete call resolves; previously the busy state never cleared. The all-containers list now iterates sortedContainers (matches the picker above and the "newest-active first" UX) instead of the unsorted visibleContainers. --------- Co-authored-by: Roland Tannous <115670425+rolandtannous@users.noreply.github.com> Co-authored-by: Roland Tannous --- .../features/chat/chat-providers-dialog.tsx | 82 +++--- .../components/openai-code-exec-section.tsx | 242 ++++++++++++++---- 2 files changed, 241 insertions(+), 83 deletions(-) diff --git a/studio/frontend/src/features/chat/chat-providers-dialog.tsx b/studio/frontend/src/features/chat/chat-providers-dialog.tsx index 0127369eb4..ea99ada0e0 100644 --- a/studio/frontend/src/features/chat/chat-providers-dialog.tsx +++ b/studio/frontend/src/features/chat/chat-providers-dialog.tsx @@ -188,6 +188,11 @@ export function ChatProvidersSettings({ const [isReasoningModel, setIsReasoningModel] = useState(false); const reduceMotion = useReducedMotion(); const isCustomProvider = isCustomProviderType(providerType); + // Ollama runs locally and does not require an API key. Hide the input + // entirely rather than just marking it optional so users aren't prompted + // for a credential the provider never uses. + const isOllamaProvider = providerType === "ollama"; + const showApiKeyField = !isOllamaProvider; const showReasoningToggle = supportsProviderReasoningToggle(providerType); const registryByType = useMemo( @@ -734,7 +739,10 @@ export function ChatProvidersSettings({ async function testProvider(provider: ExternalProviderConfig) { const savedKey = getExternalProviderApiKey(provider.id).trim(); - if (!savedKey) { + // Ollama runs locally and never requires a key — fall through to the + // real connection check instead of prompting for credentials the form + // no longer exposes. + if (!savedKey && provider.providerType !== "ollama") { if (isCustomProviderType(provider.providerType)) { await editProvider(provider); toast.info(CUSTOM_PROVIDER_MISSING_KEY_MESSAGE); @@ -884,42 +892,44 @@ export function ChatProvidersSettings({ -
-
- -

- Stored locally. -

+ {showApiKeyField ? ( +
+
+ +

+ Stored locally. +

+
+
+ setApiKey(event.target.value)} + placeholder="Enter API key" + className="h-9 pr-9 text-sm" + /> + +
-
- setApiKey(event.target.value)} - placeholder="Enter API key" - className="h-9 pr-9 text-sm" - /> - -
-
+ ) : null} {isCustomProvider ? (
diff --git a/studio/frontend/src/features/chat/components/openai-code-exec-section.tsx b/studio/frontend/src/features/chat/components/openai-code-exec-section.tsx index 2d3d225069..d76b4a282f 100644 --- a/studio/frontend/src/features/chat/components/openai-code-exec-section.tsx +++ b/studio/frontend/src/features/chat/components/openai-code-exec-section.tsx @@ -31,6 +31,16 @@ import { useCallback, useEffect, useMemo, useState } from "react"; import { toast } from "sonner"; +import { + AlertDialog, + AlertDialogAction, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from "@/components/ui/alert-dialog"; import { Button } from "@/components/ui/button"; import { Input } from "@/components/ui/input"; import { Skeleton } from "@/components/ui/skeleton"; @@ -50,6 +60,11 @@ const AUTO_OPTION_VALUE = "__auto__"; const DEFAULT_TTL_MINUTES = 20; const TTL_MIN = 1; const TTL_MAX = 20; // OpenAI hard cap on expires_after.minutes +// Cadence for re-fetching the container list while the section is +// mounted. OpenAI's container TTL flips at minute granularity, so 30s +// is fast enough that an expired container loses its ACTIVE pill within +// half a minute without hammering /v1/containers. +const REFRESH_POLL_MS = 30_000; function ageLabel(epochSeconds: number | null | undefined): string { if (!epochSeconds) return ""; @@ -63,6 +78,21 @@ function ageLabel(epochSeconds: number | null | undefined): string { return `${ageDay}d ago`; } +function shortContainerId(id: string): string { + // Mid-truncate keeps the "cntr_" prefix readable and still surfaces the + // tail digits users sometimes copy off OpenAI's dashboard. + if (id.length <= 18) return id; + return `${id.slice(0, 12)}…${id.slice(-4)}`; +} + +function isContainerRunning(c: OpenAIContainerSummary): boolean { + // OpenAI's containers API reports `status: "running"` while idle TTL is + // valid and `status: "expired"` once the idle window has passed. Treat + // a missing status as running so we don't false-positive on any older + // payloads that didn't include the field. + return c.status == null || c.status === "running"; +} + interface OpenAICodeExecSectionProps { provider: ExternalProviderConfig; apiKey: string | null; @@ -91,6 +121,12 @@ export function OpenAICodeExecSection({ // creates more confusion than it solves. Refreshing the page resets // the tombstone naturally. const [tombstones, setTombstones] = useState>(() => new Set()); + // Target row for the destructive confirmation dialog. Held in state + // (rather than blocking with window.confirm) so the dialog sits inside + // the settings sheet instead of a native browser alert. + const [pendingDelete, setPendingDelete] = + useState(null); + const [deleting, setDeleting] = useState(false); const thread = useLiveQuery( async () => (activeThreadId ? db.threads.get(activeThreadId) : undefined), @@ -116,15 +152,27 @@ export function OpenAICodeExecSection({ [visibleContainers], ); - // What the dropdown should display right now. We decouple this from - // `activeContainerId` (which is whatever is in Dexie) so the user - // immediately sees the most-recent container by name when there is - // no thread binding yet, rather than a "Selecting most recent…" - // placeholder while the auto-bind effect's async write propagates - // back through useLiveQuery. The auto-bind effect still writes the - // bind to Dexie so the chat adapter sees it on send. + // First running container by lastActiveAt — the auto-bind target and + // also what we surface visually before Dexie catches up. + const firstRunningContainer = useMemo( + () => sortedContainers.find(isContainerRunning) ?? null, + [sortedContainers], + ); + + // What the picker should treat as "active" right now. We decouple + // this from `activeContainerId` (Dexie state) so the user immediately + // sees the most-recent running container while the auto-bind effect's + // async write propagates. If the Dexie-bound container has since + // expired, fall back to the first running candidate — the stale-bind + // sweeper below will clear Dexie shortly after. + const boundContainer = useMemo( + () => sortedContainers.find((c) => c.id === activeContainerId) ?? null, + [sortedContainers, activeContainerId], + ); const displayedContainerId = - activeContainerId ?? sortedContainers[0]?.id ?? null; + (boundContainer && isContainerRunning(boundContainer) + ? boundContainer.id + : firstRunningContainer?.id) ?? null; const refresh = useCallback(async () => { if (!apiKey) return; @@ -144,9 +192,28 @@ export function OpenAICodeExecSection({ } }, [apiKey, provider.baseUrl]); - // Fetch once when the section mounts (or provider changes). + // Fetch once when the section mounts (or provider changes), then + // poll on a low cadence so an expired container's ACTIVE pill clears + // without the user clicking the refresh button. Also re-fetch when + // the tab regains visibility — covers the common case of leaving the + // sheet open across a long idle period. useEffect(() => { void refresh(); + const interval = window.setInterval(() => { + if (document.visibilityState === "visible") { + void refresh(); + } + }, REFRESH_POLL_MS); + const onVisibility = () => { + if (document.visibilityState === "visible") { + void refresh(); + } + }; + document.addEventListener("visibilitychange", onVisibility); + return () => { + window.clearInterval(interval); + document.removeEventListener("visibilitychange", onVisibility); + }; }, [refresh]); // Auto-bind the active thread to the most-recently-active container @@ -275,15 +342,10 @@ export function OpenAICodeExecSection({ } }; - const onDelete = async (id: string, name: string | null | undefined) => { - if (!apiKey) return; - if ( - !window.confirm( - `Delete container ${name || id}? Threads using it will fall back to auto-create on their next turn.`, - ) - ) { - return; - } + const confirmDelete = async () => { + if (!apiKey || !pendingDelete) return; + const { id, name } = pendingDelete; + setDeleting(true); try { await deleteOpenAIContainer( { apiKey, baseUrl: provider.baseUrl || null }, @@ -312,6 +374,8 @@ export function OpenAICodeExecSection({ `Delete failed: ${err instanceof Error ? err.message : "Unknown"}`, ); } finally { + setDeleting(false); + setPendingDelete(null); // Always refresh so a stale list entry (e.g. container deleted // elsewhere, or already expired) is purged from the UI even when // the delete call itself errored. @@ -319,6 +383,8 @@ export function OpenAICodeExecSection({ } }; + const displayActiveId = displayedContainerId; + return (
{/* TTL */} @@ -336,23 +402,23 @@ export function OpenAICodeExecSection({ max={TTL_MAX} value={ttlValue} onChange={(e) => onTtlChange(e.target.value)} - className="h-8 w-24 text-sm" + className="h-8 w-14 px-2 text-center text-sm tabular-nums" />
- {/* Active container picker — visually emphasized so it reads as - the primary control vs. the static list below. Accent - background + ring outline distinguish it from the plain - bordered list items beneath. */} -
+ {/* Single container list. The previously-separate "Active for + this thread" picker collapses into this list: clicking a row + binds it to the active thread, and the ACTIVE pill marks + which one. Avoids duplicating state across two controls. */} +
- - Active for this thread + + Containers
@@ -506,6 +617,43 @@ export function OpenAICodeExecSection({ New container )} + + { + if (!nextOpen && deleting) return; + if (!nextOpen) setPendingDelete(null); + }} + > + + + + Delete{" "} + + {pendingDelete?.name ?? "container"} + + ? + + + Threads using this container will fall back to auto-create on + their next turn. This cannot be undone. + + + + Cancel + { + e.preventDefault(); + void confirmDelete(); + }} + > + {deleting ? "Deleting…" : "Delete"} + + + +
); }