From b4eea187d5650dee46de155c67f21d1b7b9150ef Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Mon, 25 May 2026 23:24:34 -0700 Subject: [PATCH] fix(xai-oauth): gate slash-enum strip on model name + add regression tests (#28490) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three additions on top of @Nami4D's salvage: 1. Gate the preflight slash-enum strip on the model name pattern (grok-* / x-ai/grok-*). The original PR stripped slash-containing enum values from every codex_responses request, but native Codex (OpenAI) and GitHub Models DO accept slash enums — stripping them there would silently degrade tool-schema constraints. xAI is the only Responses-API surface that rejects the shape. 2. Resolve the merge conflict in agent/transports/codex.py by preserving both the timeout-forwarding block that landed on main between the PR's branch point and now AND the new service_tier strip. Behavioural intent of both is preserved. 3. Six new tests in tests/agent/transports/test_codex_transport.py covering: - TestCodexTransportXaiServiceTierStrip (3 tests): xAI strips service_tier from request_overrides; non-xAI codex_responses and GitHub Models both KEEP service_tier (regression guards so the strip stays xAI-only). - TestPreflightSlashEnumStrip (3 tests): Grok and aggregator- prefixed Grok model names both trigger the safety-net strip; non-Grok models preserve slash enums as a regression guard against the strip becoming too broad. 51/51 in tests/agent/transports/test_codex_transport.py. Co-authored-by: Nami4D --- scripts/release.py | 1 + .../agent/transports/test_codex_transport.py | 142 ++++++++++++++++++ 2 files changed, 143 insertions(+) diff --git a/scripts/release.py b/scripts/release.py index 7d55b52c7f5..9eb5f25fec2 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -251,6 +251,7 @@ AUTHOR_MAP = { "harryykyle1@gmail.com": "hharry11", "wysie@users.noreply.github.com": "wysie", "ronhi@buildabear1.localdomain": "RonHillDev", # PR #29523 salvage (machine-local commit email) + "hello@nami4d.tech": "Nami4D", # PR #28490 salvage "jkausel@gmail.com": "jkausel-ai", "e.silacandmr@gmail.com": "Es1la", "51599529+stephen0110@users.noreply.github.com": "stephen0110", diff --git a/tests/agent/transports/test_codex_transport.py b/tests/agent/transports/test_codex_transport.py index 96a80827204..1309c979218 100644 --- a/tests/agent/transports/test_codex_transport.py +++ b/tests/agent/transports/test_codex_transport.py @@ -513,3 +513,145 @@ class TestCodexTransportTimeout: request_overrides={"timeout": 450.0}, ) assert kw.get("timeout") == 450.0 + + +class TestCodexTransportXaiServiceTierStrip: + """xAI Responses API rejects ``service_tier`` (#28490). + + ``resolve_fast_mode_overrides`` only returns ``service_tier`` for + OpenAI fast-eligible models, so on paper the field should never + reach a Grok request. But ``self.service_tier`` lingers across + model switches and can also be set directly via ``agent.service_tier`` + in config.yaml — both leak paths plumb through ``request_overrides`` + and would 400 against xAI's ``/v1/responses``. + Strip defensively when targeting xAI. + """ + + @pytest.fixture + def transport(self): + from agent.transports.codex import ResponsesApiTransport + return ResponsesApiTransport() + + def test_xai_strips_service_tier_from_request_overrides(self, transport): + """Headline #28490 case: service_tier=priority leaks through + request_overrides, must not reach the xAI request body.""" + kw = transport.build_kwargs( + model="grok-4.3", + messages=[{"role": "user", "content": "hi"}], + tools=[], + is_xai_responses=True, + request_overrides={"service_tier": "priority"}, + ) + assert "service_tier" not in kw, ( + f"service_tier must be stripped on xAI requests, " + f"got {kw.get('service_tier')!r}" + ) + + def test_non_xai_codex_preserves_service_tier(self, transport): + """The strip is xAI-only — native Codex DOES accept + service_tier=priority (OpenAI Priority Processing). Stripping + it elsewhere would silently disable the user's fast-mode opt-in. + """ + kw = transport.build_kwargs( + model="gpt-5.5", + messages=[{"role": "user", "content": "hi"}], + tools=[], + is_xai_responses=False, + is_codex_backend=True, + request_overrides={"service_tier": "priority"}, + ) + assert kw.get("service_tier") == "priority", ( + "non-xAI codex_responses providers must keep service_tier" + ) + + def test_github_responses_preserves_service_tier(self, transport): + """GitHub Models (Copilot) is another codex_responses surface + that should not be affected by the xAI strip.""" + kw = transport.build_kwargs( + model="gpt-5.5", + messages=[{"role": "user", "content": "hi"}], + tools=[], + is_github_responses=True, + request_overrides={"service_tier": "priority"}, + ) + assert kw.get("service_tier") == "priority" + + +class TestPreflightSlashEnumStrip: + """xAI Responses safety-net: strip slash-containing enum values + when the model name indicates a Grok target (#28490). + + Native Codex accepts ``/``-containing enums; xAI rejects them with + HTTP 400 "Invalid arguments passed to the model". The main agent + loop and the auxiliary client already sanitize at request-build + time; this preflight catches any future code path that bypasses + those — gated on model name so we don't unnecessarily strip on + non-xAI providers. + """ + + def _make_kwargs(self, model: str, enum_values: list[str]) -> dict: + return { + "model": model, + "instructions": "test", + "input": [{"role": "user", "content": "hi"}], + "tools": [ + { + "type": "function", + "name": "pick_model", + "description": "pick a model", + "parameters": { + "type": "object", + "properties": { + "model_id": { + "type": "string", + "enum": enum_values, + }, + }, + }, + }, + ], + } + + def test_grok_model_strips_slash_enum_values(self): + """When the model name is Grok-family, slash-containing enum + values are stripped so xAI doesn't 400 on the tool schema.""" + from agent.codex_responses_adapter import _preflight_codex_api_kwargs + kwargs = self._make_kwargs( + "grok-4.3", + ["Qwen/Qwen3.5-0.8B", "openai/gpt-oss-20b", "plain-id"], + ) + result = _preflight_codex_api_kwargs(kwargs) + # The enum keyword itself is stripped (per strip_slash_enum's + # semantics — it removes the constraint entirely when any value + # contains /). + params = result["tools"][0]["parameters"] + assert "enum" not in params["properties"]["model_id"], ( + "slash-containing enum must be stripped on Grok" + ) + + def test_aggregator_prefixed_grok_also_strips(self): + """Aggregator-prefixed (x-ai/grok-*) names hit the same path.""" + from agent.codex_responses_adapter import _preflight_codex_api_kwargs + kwargs = self._make_kwargs( + "x-ai/grok-4.3", + ["Qwen/Qwen3.5-0.8B"], + ) + result = _preflight_codex_api_kwargs(kwargs) + assert "enum" not in result["tools"][0]["parameters"]["properties"]["model_id"] + + def test_non_grok_model_preserves_slash_enum_values(self): + """Native Codex / GitHub Models DO accept slash-containing + enums. The safety-net must NOT strip there or we silently + degrade tool-schema constraints on every codex_responses + provider that isn't xAI.""" + from agent.codex_responses_adapter import _preflight_codex_api_kwargs + kwargs = self._make_kwargs( + "gpt-5.5", + ["Qwen/Qwen3.5-0.8B", "plain-id"], + ) + result = _preflight_codex_api_kwargs(kwargs) + params = result["tools"][0]["parameters"] + # The enum must survive on non-xAI providers. + assert params["properties"]["model_id"].get("enum") == [ + "Qwen/Qwen3.5-0.8B", "plain-id" + ]