ceos/unsloth
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

studio/chat: OpenAI container picker delete reliability (#5466)

Browse source 
* 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<id, expiry> to Set<id>: 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.
This commit is contained in:
Roland Tannous 2026-05-16 01:53:13 +04:00 • committed by GitHub
commit a70bf02bb8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 159 additions and 23 deletions
Download patch file Download diff file Expand all files Collapse all files

43
studio/backend/core/inference/external_provider.py
View file

@ -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:

6
studio/backend/models/inference.py
View file

@ -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."
),
)

30
studio/backend/routes/inference.py
View file

@ -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}",

38
studio/backend/tests/test_openai_container_crud.py
View file

@ -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

5
studio/frontend/src/features/chat/api/openai-containers.ts
View file

@ -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));
}
}

58
studio/frontend/src/features/chat/components/openai-code-exec-section.tsx
View file

@ -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<number>(
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<Set<string>>(() => 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)
</label>
<Input
id="openai-container-ttl"
@ -360,11 +394,11 @@ export function OpenAICodeExecSection({
<span className="text-[11px] uppercase tracking-wider text-muted-foreground">
All containers
</span>
{isLoading && containers.length === 0 ? (
{isLoading && visibleContainers.length === 0 ? (
<Skeleton className="h-16 w-full" />
) : containers.length > 0 ? (
) : visibleContainers.length > 0 ? (
<ul className="flex flex-col gap-1 max-h-44 overflow-auto">
{containers.map((c) => {
{visibleContainers.map((c) => {
const isActive = c.id === activeContainerId;
return (
<li

2
studio/frontend/src/features/chat/external-providers.ts
View file

@ -237,7 +237,7 @@ function normalizeProvider(raw: ExternalProviderConfig): ExternalProviderConfig
providerType === "openai" &&
typeof raw.openaiContainerTtlMinutes === "number" &&
raw.openaiContainerTtlMinutes >= 1
? Math.min(raw.openaiContainerTtlMinutes, 10080)
? Math.min(raw.openaiContainerTtlMinutes, 20)
: undefined,
};
}