From 6a9fa8b35056e12dab2dfce164f2dac0d8732b27 Mon Sep 17 00:00:00 2001 From: Roland Tannous Date: Fri, 15 May 2026 22:40:21 +0400 Subject: [PATCH] studio/chat: send OpenAI-Beta header for /v1/containers CRUD Without OpenAI-Beta: containers=v1, OpenAI returns 200 {"deleted": true} for DELETE /v1/containers/{id} but does not actually remove the container. The list call then keeps returning it, making it look like Studio's "Delete container" button is broken. Verified 2026-05-15 against api.openai.com: DELETE with the beta header returns 200 and removes the container; the same DELETE without the header returns the same 200 deleted:true body but the container stays alive. - Add _container_headers() that merges OpenAI-Beta on top of the shared auth headers; route list / create / delete through it. - Verify the DELETE response body reports {"deleted": true}; raise httpx.HTTPError otherwise so the route surfaces a 5xx instead of silently reporting success on a silent no-op. - Add tests covering header propagation and the deleted-flag guard (true, false, missing key, non-JSON body, 4xx passthrough). --- .../core/inference/external_provider.py | 35 +++- .../tests/test_openai_container_crud.py | 151 ++++++++++++++++++ 2 files changed, 182 insertions(+), 4 deletions(-) create mode 100644 studio/backend/tests/test_openai_container_crud.py diff --git a/studio/backend/core/inference/external_provider.py b/studio/backend/core/inference/external_provider.py index be80b9a97d..eb57e99090 100644 --- a/studio/backend/core/inference/external_provider.py +++ b/studio/backend/core/inference/external_provider.py @@ -2836,6 +2836,19 @@ class ExternalProviderClient: ) raise + def _container_headers(self) -> dict[str, str]: + """Auth headers plus the OpenAI-Beta opt-in for /v1/containers. + + OpenAI's containers API requires ``OpenAI-Beta: containers=v1``. + Without it, DELETE silently no-ops: the API returns 200 with a + ``{"deleted": true}`` body but does not actually remove the + container (verified 2026-05-15). The header is required for + list / create / delete to behave consistently. + """ + headers = self._auth_headers() + headers["OpenAI-Beta"] = "containers=v1" + return headers + async def list_openai_containers(self) -> list[dict[str, Any]]: """ GET /v1/containers on the user's OpenAI account. @@ -2850,7 +2863,7 @@ class ExternalProviderClient: """ response = await _http_client.get( f"{self.base_url}/containers", - headers = self._auth_headers(), + headers = self._container_headers(), timeout = self._timeout, ) response.raise_for_status() @@ -2878,20 +2891,34 @@ class ExternalProviderClient: response = await _http_client.post( f"{self.base_url}/containers", json = body, - headers = self._auth_headers(), + headers = self._container_headers(), timeout = self._timeout, ) response.raise_for_status() return response.json() async def delete_openai_container(self, container_id: str) -> None: - """DELETE /v1/containers/{id}. 404s are surfaced as HTTPError.""" + """DELETE /v1/containers/{id}. 404s are surfaced as HTTPError. + + 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._auth_headers(), + headers = self._container_headers(), timeout = self._timeout, ) response.raise_for_status() + try: + payload = response.json() + except ValueError: + payload = None + if not (isinstance(payload, dict) and payload.get("deleted") is True): + raise httpx.HTTPError( + f"OpenAI did not confirm container deletion: {response.text[:200]}" + ) async def close(self) -> None: """No-op — the underlying client is shared across requests.""" diff --git a/studio/backend/tests/test_openai_container_crud.py b/studio/backend/tests/test_openai_container_crud.py new file mode 100644 index 0000000000..2965ec6649 --- /dev/null +++ b/studio/backend/tests/test_openai_container_crud.py @@ -0,0 +1,151 @@ +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 + +"""Unit tests for the /v1/containers CRUD client methods. + +Covers: +- All three calls (list / create / delete) send + ``OpenAI-Beta: containers=v1``. Without it, OpenAI silently no-ops + the DELETE while still returning 200 ``{"deleted": true}``. +- ``delete_openai_container`` raises when the response body does not + report ``{"deleted": true}``, even on a 2xx response. +""" + +from __future__ import annotations + +import asyncio +import json + +import httpx +import pytest + +from core.inference import external_provider as ep_mod +from core.inference.external_provider import ExternalProviderClient + + +def _drive(coro): + return asyncio.new_event_loop().run_until_complete(coro) + + +def _mock_http_client(monkeypatch, handler): + transport = httpx.MockTransport(handler) + monkeypatch.setattr(ep_mod, "_http_client", httpx.AsyncClient(transport = transport)) + + +def _make_client() -> ExternalProviderClient: + return ExternalProviderClient( + provider_type = "openai", + base_url = "https://api.openai.com/v1", + api_key = "sk-test", + ) + + +def test_list_sends_openai_beta_header(monkeypatch): + seen: dict = {} + + def handler(request: httpx.Request) -> httpx.Response: + seen["headers"] = dict(request.headers) + seen["url"] = str(request.url) + return httpx.Response( + 200, + json = {"data": [{"id": "cntr_x", "name": "auto"}]}, + ) + + _mock_http_client(monkeypatch, handler) + result = _drive(_make_client().list_openai_containers()) + + assert result == [{"id": "cntr_x", "name": "auto"}] + assert seen["headers"].get("openai-beta") == "containers=v1" + assert seen["url"] == "https://api.openai.com/v1/containers" + + +def test_create_sends_openai_beta_header(monkeypatch): + seen: dict = {} + + def handler(request: httpx.Request) -> httpx.Response: + seen["headers"] = dict(request.headers) + seen["body"] = json.loads(request.content.decode("utf-8")) + return httpx.Response(200, json = {"id": "cntr_new", "name": "analysis"}) + + _mock_http_client(monkeypatch, handler) + result = _drive( + _make_client().create_openai_container(name = "analysis", ttl_minutes = 30) + ) + + assert result == {"id": "cntr_new", "name": "analysis"} + assert seen["headers"].get("openai-beta") == "containers=v1" + assert seen["body"]["name"] == "analysis" + assert seen["body"]["expires_after"] == { + "anchor": "last_active_at", + "minutes": 30, + } + + +def test_delete_sends_openai_beta_header_and_accepts_confirmation(monkeypatch): + seen: dict = {} + + def handler(request: httpx.Request) -> httpx.Response: + seen["headers"] = dict(request.headers) + seen["url"] = str(request.url) + seen["method"] = request.method + return httpx.Response( + 200, + json = {"id": "cntr_x", "object": "container.deleted", "deleted": True}, + ) + + _mock_http_client(monkeypatch, handler) + _drive(_make_client().delete_openai_container("cntr_x")) + + assert seen["method"] == "DELETE" + assert seen["url"] == "https://api.openai.com/v1/containers/cntr_x" + assert seen["headers"].get("openai-beta") == "containers=v1" + + +def test_delete_raises_when_response_lacks_deleted_true(monkeypatch): + """OpenAI returns 200 ``{"deleted": true}`` even when the request is + silently rejected (e.g. before we started sending OpenAI-Beta). + Defensive guard: when the body omits ``deleted: true``, surface it + as an error so the UI can report the failure instead of falsely + reporting success.""" + + def handler(request: httpx.Request) -> httpx.Response: + # 200 but no deleted flag — simulate an unexpected payload shape. + return httpx.Response(200, json = {"id": "cntr_x", "object": "container"}) + + _mock_http_client(monkeypatch, handler) + + with pytest.raises(httpx.HTTPError, match = "did not confirm container deletion"): + _drive(_make_client().delete_openai_container("cntr_x")) + + +def test_delete_raises_when_deleted_is_false(monkeypatch): + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, + json = {"id": "cntr_x", "object": "container.deleted", "deleted": False}, + ) + + _mock_http_client(monkeypatch, handler) + + with pytest.raises(httpx.HTTPError, match = "did not confirm container deletion"): + _drive(_make_client().delete_openai_container("cntr_x")) + + +def test_delete_raises_when_body_is_not_json(monkeypatch): + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, content = b"OK") + + _mock_http_client(monkeypatch, handler) + + with pytest.raises(httpx.HTTPError, match = "did not confirm container deletion"): + _drive(_make_client().delete_openai_container("cntr_x")) + + +def test_delete_propagates_openai_4xx(monkeypatch): + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(404, json = {"error": {"message": "not found"}}) + + _mock_http_client(monkeypatch, handler) + + with pytest.raises(httpx.HTTPStatusError): + _drive(_make_client().delete_openai_container("cntr_missing"))