From 4f59c8e539db39ce40da1609aaa8a01d960be297 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Fri, 15 May 2026 14:44:52 -0700 Subject: [PATCH 1/2] studio/install: repair upstream llama.cpp prebuilt mangled symlinks (#5465) The macos-arm64 prebuilt tarball for llama.cpp b9165 and b9169 ships symlinks whose linkname is missing both the directory separator AND the leading character of the target basename: llama-b9165/libggml-rpc.0.dylib -> llama-b9165ibggml-rpc.0.11.1.dylib extract_tar_safely correctly classified those as unresolved and made install.sh fall back to source-build, which Mac CI then fails as a hard error (Studio must use the prebuilt llama-bNNNN-bin-macos-arm64 on Apple Silicon). Add _try_repair_missing_slash inside safe_link_target: when a linkname starts with the member's top-level dir but no following slash, search the archive for an entry under that dir whose name ends with the mangled suffix. Accept only when the suffix uniquely identifies a real archive entry, so legitimate archives are untouched. Verified against /tmp/llama-b9165.tar.gz: all 18 link entries repair to real files in the archive. --- studio/install_llama_prebuilt.py | 48 ++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/studio/install_llama_prebuilt.py b/studio/install_llama_prebuilt.py index 7c933bd612..89322c83ee 100644 --- a/studio/install_llama_prebuilt.py +++ b/studio/install_llama_prebuilt.py @@ -3433,10 +3433,52 @@ def extract_archive(archive_path: Path, destination: Path) -> None: ) from exc return target + def _try_repair_missing_slash( + member_name: str, link_name: str, archive_names: set[str] + ) -> str | None: + """Some upstream llama.cpp Mac releases (e.g. b9165, b9169) ship + symlinks whose linkname is missing the directory separator AND + the leading character of the file basename between the + top-level dir and the rest of the path: + + llama-b9165/libggml-rpc.0.dylib -> llama-b9165ibggml-rpc.0.11.1.dylib + + That cannot be resolved as written. Detect the pattern + (linkname starts with the top-level dir name but no following + slash) and search archive entries under that dir for a real + file whose basename ends with the mangled suffix. Only accept + when the suffix uniquely identifies a real archive entry.""" + if "/" not in member_name or "/" in link_name: + return None + top, _, _ = member_name.partition("/") + if not link_name.startswith(top) or len(link_name) <= len(top): + return None + bad_suffix = link_name[len(top) :] + if not bad_suffix or bad_suffix.startswith("/"): + return None + prefix = f"{top}/" + candidates = [ + name + for name in archive_names + if name.startswith(prefix) + and "/" not in name[len(prefix) :] + and name[len(prefix) :].endswith(bad_suffix) + ] + if len(candidates) != 1: + return None + return candidates[0] + def safe_link_target( - base: Path, member_name: str, link_name: str, target: Path + base: Path, + member_name: str, + link_name: str, + target: Path, + archive_names: set[str], ) -> tuple[str, Path]: normalized = link_name.replace("\\", "/") + repaired = _try_repair_missing_slash(member_name, normalized, archive_names) + if repaired is not None: + normalized = repaired link_path = Path(normalized) if link_path.is_absolute(): raise PrebuiltFallback( @@ -3473,8 +3515,10 @@ def extract_archive(archive_path: Path, destination: Path) -> None: def extract_tar_safely(source: Path, base: Path) -> None: pending_links: list[tuple[tarfile.TarInfo, Path]] = [] + archive_names: set[str] = set() with tarfile.open(source, "r:gz") as archive: for member in archive.getmembers(): + archive_names.add(member.name) target = safe_extract_path(base, member.name) if member.isdir(): target.mkdir(parents = True, exist_ok = True) @@ -3501,7 +3545,7 @@ def extract_archive(archive_path: Path, destination: Path) -> None: progressed = False for member, target in unresolved: normalized_link, resolved_target = safe_link_target( - base, member.name, member.linkname, target + base, member.name, member.linkname, target, archive_names ) if not resolved_target.exists() and not resolved_target.is_symlink(): next_round.append((member, target)) From a70bf02bb88957d6b3617f9220ecf9fb5ae68528 Mon Sep 17 00:00:00 2001 From: Roland Tannous <115670425+rolandtannous@users.noreply.github.com> Date: Sat, 16 May 2026 01:53:13 +0400 Subject: [PATCH 2/2] studio/chat: OpenAI container picker delete reliability (#5466) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * studio/chat: fix OpenAI container delete UX (expired filter, TTL cap, idempotent 404, refresh-on-error) - Filter status="expired" from /containers/list so the picker only shows usable containers. OpenAI keeps expired entries in the list indefinitely, which made delete look broken. - Cap ttl_minutes at 20 (backend Field + frontend TTL_MAX + persistence clamp). OpenAI's actual hard limit is 20; the prior 10080 cap caused integer_above_max_value rejections on create. - Treat 404 on delete as idempotent success in the frontend client so already-gone containers don't surface a scary error toast. - Run refresh() in finally for onCreate/onDelete so the picker stays in sync with OpenAI even when the call errors. - Add route-level test for the expired filter. * studio/chat: add diagnostic logging for OpenAI /containers DELETE Trace what arrives at /external/openai/containers/delete (subject, container_id, base_url) and what we send to OpenAI (URL, presence of Authorization, value of OpenAI-Beta) plus the full response status + body (capped at 300 chars). Helps confirm whether the beta header is on the wire and whether OpenAI's response actually reports deleted=true, when users report the delete "not taking". No secrets are logged — Authorization is reported as a boolean. * studio/chat: log raw /containers list response from OpenAI Sibling to the delete diagnostics. After a confirmed delete (deleted=true on the wire), we want to see whether the very next list call returns the just-deleted id — that distinguishes "OpenAI eventually-consistent list" from "frontend stale state". Logs each entry's id + status only; no names, no timestamps. * studio/chat: fingerprint decrypted API key for container CRUD Logs kind (sk-proj-/sk-/other), length, and last-4 chars only — never the full secret. Lets us compare what the backend actually uses against the key the user expects, since the same DELETE request shape can produce different results across keys (project-scoped containers: list is permissive but delete requires the owning project's key). * studio/chat: use fresh httpx client for /v1/containers DELETE Same key, same headers, same URL via the shared _http_client returned deleted=true but the container persisted in subsequent list calls. A fresh httpx.AsyncClient with the identical request shape (verified with a standalone reproducer) deleted the same container cleanly. Suspect connection-pool state from earlier chat-completion streams interferes at the edge — switching to a per-call client side-steps it entirely. Scoped to delete only; list/create keep using the shared pool until we can confirm the same fix is needed there. * studio/chat: log OpenAI response headers on container DELETE Adds cf-ray / x-request-id / openai-organization / openai-project / openai-processing-ms to the delete-response diagnostic line. Lets us cross-reference a failing delete against OpenAI support (or against a working standalone reproducer) using the unique request-id and edge node. * studio/chat: client-side tombstone for just-deleted OpenAI containers OpenAI's /v1/containers DELETE returns {"deleted": true} but the list endpoint can keep returning the same container for several minutes (replica lag or in-use silent no-op — undocumented per developers.openai.com/api/docs/guides/tools-shell). Our backend sends the correct DELETE with OpenAI-Beta: containers=v1 and a standalone reproducer shows the same behavior, so the right fix is UI-side rather than waiting on OpenAI. After a successful delete, the id goes into a per-component tombstone map with a 5-minute expiry. visibleContainers (now the single chokepoint feeding sortedContainers, auto-bind, and the all-containers list) filters those ids out. A 30s sweep clears expired tombstones so the picker recovers automatically if OpenAI eventually catches up (or the container's TTL elapses). * studio/chat: tombstones live for the page lifetime; drop API key fingerprint log - Tombstones change from Map to Set: once tombstoned, the id stays hidden from the picker until page reload. OpenAI's list can keep returning a deleted id for an undocumented and variable amount of time; automatically un-tombstoning after a fixed window surfaces it again and creates more confusion than it solves. The container's own TTL eventually expires the entry on OpenAI's side, and the expired-status filter at the backend list route hides it anyway. - Remove the periodic sweep effect (dead code without expiries). - Remove the api-key fingerprint log added during debugging — it served its purpose (confirmed parity) and isn't needed long-term. --- .../core/inference/external_provider.py | 43 ++++++++++++-- studio/backend/models/inference.py | 6 +- studio/backend/routes/inference.py | 30 +++++++++- .../tests/test_openai_container_crud.py | 38 ++++++++++++ .../features/chat/api/openai-containers.ts | 5 +- .../components/openai-code-exec-section.tsx | 58 +++++++++++++++---- .../src/features/chat/external-providers.ts | 2 +- 7 files changed, 159 insertions(+), 23 deletions(-) diff --git a/studio/backend/core/inference/external_provider.py b/studio/backend/core/inference/external_provider.py index 79c8287e5b..de5f5c5500 100644 --- a/studio/backend/core/inference/external_provider.py +++ b/studio/backend/core/inference/external_provider.py @@ -2870,7 +2870,17 @@ class ExternalProviderClient: response.raise_for_status() data = response.json() containers = data.get("data") if isinstance(data, dict) else None - return list(containers) if isinstance(containers, list) else [] + result = list(containers) if isinstance(containers, list) else [] + logger.info( + "openai_container_list.response count=%s items=%s", + len(result), + [ + {"id": c.get("id"), "status": c.get("status")} + for c in result + if isinstance(c, dict) + ], + ) + return result async def create_openai_container( self, @@ -2901,15 +2911,38 @@ class ExternalProviderClient: async def delete_openai_container(self, container_id: str) -> None: """DELETE /v1/containers/{id}. 404s are surfaced as HTTPError. + Uses a fresh httpx client (not the shared ``_http_client``) so + connection-pool state from earlier chat requests cannot + interfere — observed in the wild that DELETEs over the shared + pool returned ``deleted: true`` while the container persisted + in subsequent /containers list calls, even though the same + DELETE issued from a fresh client genuinely removed it. + Verifies the response body reports ``deleted: true``. OpenAI returns a 2xx ``deleted: true`` body even when the request is silently rejected (e.g. missing OpenAI-Beta header), so a status-only check is not sufficient. """ - response = await _http_client.delete( - f"{self.base_url}/containers/{container_id}", - headers = self._container_headers(), - timeout = self._timeout, + url = f"{self.base_url}/containers/{container_id}" + headers = self._container_headers() + logger.info( + "openai_container_delete.outbound url=%s has_auth=%s openai_beta=%s", + url, + "Authorization" in headers, + headers.get("OpenAI-Beta"), + ) + async with httpx.AsyncClient(timeout = self._timeout) as fresh_client: + response = await fresh_client.delete(url, headers = headers) + logger.info( + "openai_container_delete.response status=%s cf_ray=%s " + "request_id=%s organization=%s project=%s processing_ms=%s body=%s", + response.status_code, + response.headers.get("cf-ray"), + response.headers.get("x-request-id"), + response.headers.get("openai-organization"), + response.headers.get("openai-project"), + response.headers.get("openai-processing-ms"), + response.text[:300], ) response.raise_for_status() try: diff --git a/studio/backend/models/inference.py b/studio/backend/models/inference.py index 6a042d35d7..3aa89cc934 100644 --- a/studio/backend/models/inference.py +++ b/studio/backend/models/inference.py @@ -650,11 +650,11 @@ class CreateOpenAIContainerBody(OpenAIContainerRequest): ttl_minutes: int = Field( 20, ge = 1, - le = 10080, # 1 week + le = 20, description = ( "Idle-timeout TTL the new container will inherit (anchor=" - "last_active_at). OpenAI's default is 20; we cap at one " - "week as a safety bound." + "last_active_at). OpenAI hard-caps this at 20 minutes and " + "rejects larger values with integer_above_max_value." ), ) diff --git a/studio/backend/routes/inference.py b/studio/backend/routes/inference.py index 8f5bd9aab5..76bbb59c94 100644 --- a/studio/backend/routes/inference.py +++ b/studio/backend/routes/inference.py @@ -1716,8 +1716,15 @@ async def list_openai_containers( status_code = 502, detail = f"Failed to reach OpenAI: {exc}", ) + # OpenAI keeps expired containers in /v1/containers indefinitely + # with status="expired" — they're effectively dead but still + # listed. Hide them so the picker only shows usable containers. return ListOpenAIContainersResponse( - containers = [_summarize_container(c) for c in raw if isinstance(c, dict)], + containers = [ + _summarize_container(c) + for c in raw + if isinstance(c, dict) and c.get("status") != "expired" + ], ) finally: await client.close() @@ -1766,17 +1773,38 @@ async def delete_openai_container( current_subject: str = Depends(get_current_subject), ) -> None: """Delete a named container by id.""" + logger.info( + "openai_container_delete.request subject=%s container_id=%s base_url=%s", + current_subject, + body.container_id, + body.provider_base_url, + ) client = _resolve_openai_cloud_client(body) try: try: await client.delete_openai_container(body.container_id) + logger.info( + "openai_container_delete.success container_id=%s", + body.container_id, + ) except httpx.HTTPStatusError as exc: detail = exc.response.text[:500] if exc.response is not None else str(exc) + logger.warning( + "openai_container_delete.openai_rejected container_id=%s status=%s body=%s", + body.container_id, + exc.response.status_code if exc.response else None, + detail, + ) raise HTTPException( status_code = exc.response.status_code if exc.response else 502, detail = f"OpenAI rejected /containers delete: {detail}", ) except httpx.HTTPError as exc: + logger.warning( + "openai_container_delete.transport_error container_id=%s error=%s", + body.container_id, + exc, + ) raise HTTPException( status_code = 502, detail = f"Failed to reach OpenAI: {exc}", diff --git a/studio/backend/tests/test_openai_container_crud.py b/studio/backend/tests/test_openai_container_crud.py index 2965ec6649..fbc0393677 100644 --- a/studio/backend/tests/test_openai_container_crud.py +++ b/studio/backend/tests/test_openai_container_crud.py @@ -149,3 +149,41 @@ def test_delete_propagates_openai_4xx(monkeypatch): with pytest.raises(httpx.HTTPStatusError): _drive(_make_client().delete_openai_container("cntr_missing")) + + +def test_list_route_filters_expired_containers(monkeypatch): + """OpenAI keeps containers in /v1/containers indefinitely with + status="expired" after their idle TTL passes — they can't be + used but still show up. The list route must drop them so the + picker only surfaces usable containers.""" + from routes import inference as inf_mod + from models.inference import OpenAIContainerRequest + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, + json = { + "data": [ + {"id": "cntr_active", "name": "live", "status": "running"}, + {"id": "cntr_dead", "name": "old", "status": "expired"}, + {"id": "cntr_unknown", "name": "no-status"}, + ], + }, + ) + + _mock_http_client(monkeypatch, handler) + + def fake_resolve(_body): + return _make_client() + + monkeypatch.setattr(inf_mod, "_resolve_openai_cloud_client", fake_resolve) + + body = OpenAIContainerRequest( + encrypted_api_key = "enc", + provider_base_url = "https://api.openai.com/v1", + ) + response = _drive(inf_mod.list_openai_containers(body, current_subject = "u")) + ids = [c.id for c in response.containers] + assert "cntr_active" in ids + assert "cntr_unknown" in ids # missing status is treated as usable + assert "cntr_dead" not in ids diff --git a/studio/frontend/src/features/chat/api/openai-containers.ts b/studio/frontend/src/features/chat/api/openai-containers.ts index ca311cc311..29d292f7cf 100644 --- a/studio/frontend/src/features/chat/api/openai-containers.ts +++ b/studio/frontend/src/features/chat/api/openai-containers.ts @@ -115,7 +115,10 @@ export async function deleteOpenAIContainer( }), }, ); - if (!response.ok && response.status !== 204) { + // 404 = container already gone (deleted elsewhere, or expired-then-purged). + // Treat as idempotent success so a stale list entry doesn't surface as a + // confusing error — the caller will refresh and the entry will disappear. + if (!response.ok && response.status !== 204 && response.status !== 404) { throw new Error(await parseError(response)); } } 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 15cb79286c..2d3d225069 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 @@ -49,7 +49,7 @@ import { ensureThreadRecord } from "../runtime-provider"; const AUTO_OPTION_VALUE = "__auto__"; const DEFAULT_TTL_MINUTES = 20; const TTL_MIN = 1; -const TTL_MAX = 10080; // one week — matches backend bound +const TTL_MAX = 20; // OpenAI hard cap on expires_after.minutes function ageLabel(epochSeconds: number | null | undefined): string { if (!epochSeconds) return ""; @@ -84,6 +84,13 @@ export function OpenAICodeExecSection({ const [createTtl, setCreateTtl] = useState( provider.openaiContainerTtlMinutes ?? DEFAULT_TTL_MINUTES, ); + // Ids that have been deleted in this session. Once tombstoned, an id + // stays hidden from the picker for the lifetime of the page — OpenAI's + // /containers list can keep returning a freshly-deleted id for an + // undocumented and variable amount of time, and an automatic re-show + // creates more confusion than it solves. Refreshing the page resets + // the tombstone naturally. + const [tombstones, setTombstones] = useState>(() => new Set()); const thread = useLiveQuery( async () => (activeThreadId ? db.threads.get(activeThreadId) : undefined), @@ -91,14 +98,22 @@ export function OpenAICodeExecSection({ ); const activeContainerId = thread?.openaiCodeExecContainerId ?? null; + // Hide just-deleted containers even if OpenAI's list still returns them. + // This is the single chokepoint — every downstream view (sorted picker, + // auto-bind candidate, all-containers list) derives from visibleContainers. + const visibleContainers = useMemo(() => { + if (tombstones.size === 0) return containers; + return containers.filter((c) => !tombstones.has(c.id)); + }, [containers, tombstones]); + // Containers sorted newest-first by lastActiveAt so the dropdown's // default (auto-bind target) shows up first. const sortedContainers = useMemo( () => - [...containers].sort( + [...visibleContainers].sort( (a, b) => (b.lastActiveAt ?? 0) - (a.lastActiveAt ?? 0), ), - [containers], + [visibleContainers], ); // What the dropdown should display right now. We decouple this from @@ -150,10 +165,14 @@ export function OpenAICodeExecSection({ // the chat-adapter's lazy-create path will mint the first container // on first send. useEffect(() => { - if (!activeThreadId || activeContainerId || containers.length === 0) { + if ( + !activeThreadId || + activeContainerId || + visibleContainers.length === 0 + ) { return; } - const sorted = [...containers].sort( + const sorted = [...visibleContainers].sort( (a, b) => (b.lastActiveAt ?? 0) - (a.lastActiveAt ?? 0), ); const candidate = sorted[0]; @@ -171,7 +190,7 @@ export function OpenAICodeExecSection({ // Best-effort; the chat-adapter will inherit/create on send. } })(); - }, [activeThreadId, activeContainerId, containers]); + }, [activeThreadId, activeContainerId, visibleContainers]); const ttlValue = provider.openaiContainerTtlMinutes ?? DEFAULT_TTL_MINUTES; @@ -223,7 +242,6 @@ export function OpenAICodeExecSection({ toast.success(`Created container ${name}`); setCreateName(""); setCreateOpen(false); - await refresh(); // Auto-bind the just-created container to the active thread. // ensureThreadRecord first so the bind lands even when the user // creates a container before sending the first message — without @@ -250,6 +268,10 @@ export function OpenAICodeExecSection({ ); } finally { setCreating(false); + // Refresh even on failure: the request may have partially succeeded + // server-side (created container, lost response), and a re-fetch + // keeps the picker in sync with OpenAI's actual state. + await refresh(); } }; @@ -267,6 +289,14 @@ export function OpenAICodeExecSection({ { apiKey, baseUrl: provider.baseUrl || null }, id, ); + // Tombstone the id so the picker hides it immediately even if + // OpenAI's list keeps returning it for a while. + setTombstones((prev) => { + if (prev.has(id)) return prev; + const next = new Set(prev); + next.add(id); + return next; + }); // Clear any thread bindings pointing at the now-deleted id. const affected = await db.threads .filter((t) => t.openaiCodeExecContainerId === id) @@ -277,11 +307,15 @@ export function OpenAICodeExecSection({ ), ); toast.success(`Deleted container ${name || id}`); - await refresh(); } catch (err) { toast.error( `Delete failed: ${err instanceof Error ? err.message : "Unknown"}`, ); + } finally { + // 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. + await refresh(); } }; @@ -293,7 +327,7 @@ export function OpenAICodeExecSection({ htmlFor="openai-container-ttl" className="min-w-0 text-[13px] font-medium leading-[1.25] tracking-nav text-nav-fg" > - New-container idle timeout (min) + New-container idle timeout (min, max 20) All containers - {isLoading && containers.length === 0 ? ( + {isLoading && visibleContainers.length === 0 ? ( - ) : containers.length > 0 ? ( + ) : visibleContainers.length > 0 ? (
    - {containers.map((c) => { + {visibleContainers.map((c) => { const isActive = c.id === activeContainerId; return (
  • = 1 - ? Math.min(raw.openaiContainerTtlMinutes, 10080) + ? Math.min(raw.openaiContainerTtlMinutes, 20) : undefined, }; }