mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-22 16:25:58 +00:00
fix(codex): harden final cache-key boundaries
Fold #62349's broader provider-boundary handling into the header fix: bound top-level and xAI override keys again at preflight after middleware, preserve unrelated headers, and cover boundaries and collisions. Co-authored-by: Nick Taylor <nicktaylor@TheWorldofNick-Lappy.local>
This commit is contained in:
Teknium
parent
81496a8925
commit
61bbc39330
4 changed files with 142 additions and 22 deletions
|
|
@ -13,6 +13,20 @@ from agent.transports.base import ProviderTransport
|
|||
from agent.transports.types import NormalizedResponse, ToolCall
|
||||
|
||||
|
||||
def _bounded_prompt_cache_key(value: Any) -> Optional[str]:
|
||||
"""Return a provider-safe cache key without changing session identity."""
|
||||
if value is None:
|
||||
return None
|
||||
key = str(value).strip()
|
||||
if not key:
|
||||
return None
|
||||
if len(key) <= 64:
|
||||
return key
|
||||
# Match _content_cache_key's compact, collision-resistant routing-key shape.
|
||||
digest = hashlib.sha256(key.encode("utf-8", errors="replace")).hexdigest()[:24]
|
||||
return f"pck_{digest}"
|
||||
|
||||
|
||||
def _content_cache_key(instructions: str, tools: Optional[List[Dict[str, Any]]]) -> Optional[str]:
|
||||
"""Content-address the prompt cache key from the static request prefix.
|
||||
|
||||
|
|
@ -304,6 +318,13 @@ class ResponsesApiTransport(ProviderTransport):
|
|||
if request_overrides:
|
||||
kwargs.update(request_overrides)
|
||||
|
||||
if "prompt_cache_key" in kwargs:
|
||||
bounded_cache_key = _bounded_prompt_cache_key(kwargs["prompt_cache_key"])
|
||||
if bounded_cache_key:
|
||||
kwargs["prompt_cache_key"] = bounded_cache_key
|
||||
else:
|
||||
kwargs.pop("prompt_cache_key", None)
|
||||
|
||||
# xAI Responses API rejects ``service_tier`` (HTTP 400 "Argument not
|
||||
# supported: service_tier") — hit when ``/fast`` priority-processing
|
||||
# mode lingers from a prior model in the same session, or when a
|
||||
|
|
@ -337,15 +358,7 @@ class ResponsesApiTransport(ProviderTransport):
|
|||
# remain high. Send session_id / x-client-request-id as HTTP
|
||||
# headers while keeping ``prompt_cache_key`` in the body for
|
||||
# standard OpenAI routing as a belt-and-braces fallback.
|
||||
cache_scope_id = str(session_id or "").strip()
|
||||
# OpenAI/Codex backend enforces 64-char limit on the session_id
|
||||
# and x-client-request-id headers, mapping them internally to
|
||||
# prompt_cache_key. Hash long IDs to stay within the limit;
|
||||
# the hash is a cache-routing hint only, never a correctness bound.
|
||||
if len(cache_scope_id) > 64:
|
||||
cache_scope_id = "pck_" + hashlib.sha256(
|
||||
cache_scope_id.encode("utf-8", "replace")
|
||||
).hexdigest()[:24]
|
||||
cache_scope_id = _bounded_prompt_cache_key(session_id)
|
||||
if cache_scope_id:
|
||||
existing_extra_headers = kwargs.get("extra_headers")
|
||||
merged_extra_headers: Dict[str, str] = {}
|
||||
|
|
@ -390,6 +403,14 @@ class ResponsesApiTransport(ProviderTransport):
|
|||
merged_extra_body.setdefault("prompt_cache_key", cache_key)
|
||||
kwargs["extra_body"] = merged_extra_body
|
||||
|
||||
extra_body = kwargs.get("extra_body")
|
||||
if isinstance(extra_body, dict) and "prompt_cache_key" in extra_body:
|
||||
bounded_cache_key = _bounded_prompt_cache_key(extra_body["prompt_cache_key"])
|
||||
if bounded_cache_key:
|
||||
extra_body["prompt_cache_key"] = bounded_cache_key
|
||||
else:
|
||||
extra_body.pop("prompt_cache_key", None)
|
||||
|
||||
return kwargs
|
||||
|
||||
def normalize_response(self, response: Any, **kwargs) -> NormalizedResponse:
|
||||
|
|
@ -478,11 +499,26 @@ class ResponsesApiTransport(ProviderTransport):
|
|||
Normalizes input items, strips unsupported fields, validates structure.
|
||||
"""
|
||||
from agent.codex_responses_adapter import _preflight_codex_api_kwargs
|
||||
return _preflight_codex_api_kwargs(
|
||||
|
||||
normalized = _preflight_codex_api_kwargs(
|
||||
api_kwargs,
|
||||
allow_stream=allow_stream,
|
||||
is_github_responses=is_github_responses,
|
||||
)
|
||||
if "prompt_cache_key" in normalized:
|
||||
bounded = _bounded_prompt_cache_key(normalized["prompt_cache_key"])
|
||||
if bounded:
|
||||
normalized["prompt_cache_key"] = bounded
|
||||
else:
|
||||
normalized.pop("prompt_cache_key", None)
|
||||
extra_body = normalized.get("extra_body")
|
||||
if isinstance(extra_body, dict) and "prompt_cache_key" in extra_body:
|
||||
bounded = _bounded_prompt_cache_key(extra_body["prompt_cache_key"])
|
||||
if bounded:
|
||||
extra_body["prompt_cache_key"] = bounded
|
||||
else:
|
||||
extra_body.pop("prompt_cache_key", None)
|
||||
return normalized
|
||||
|
||||
def map_finish_reason(self, raw_reason: str) -> str:
|
||||
"""Map Codex response.status to OpenAI finish_reason.
|
||||
|
|
|
|||
|
|
@ -373,6 +373,7 @@ AUTHOR_MAP = {
|
|||
"dirtyren@users.noreply.github.com": "dirtyren",
|
||||
"krowd3v@users.noreply.github.com": "krowd3v",
|
||||
"dfein38347g@users.noreply.github.com": "dfein38347g",
|
||||
"nicktaylor@TheWorldofNick-Lappy.local": "thegoodguysla",
|
||||
"s96919@gmail.com": "s96919",
|
||||
"rasitakyol@hotmail.com": "rasitakyol",
|
||||
"thatgfsj@gmail.com": "Thatgfsj",
|
||||
|
|
|
|||
|
|
@ -341,20 +341,74 @@ class TestCodexBuildKwargs:
|
|||
assert kw["prompt_cache_key"].startswith("pck_")
|
||||
assert len(kw["prompt_cache_key"]) <= 64
|
||||
|
||||
def test_codex_backend_overlength_cache_scope_is_stable(self, transport):
|
||||
session_id = "paperclip:company:" + "a" * 80
|
||||
kwargs = {
|
||||
"model": "gpt-5.4",
|
||||
"messages": [{"role": "user", "content": "Hi"}],
|
||||
"tools": [],
|
||||
"session_id": session_id,
|
||||
"is_codex_backend": True,
|
||||
}
|
||||
@pytest.mark.parametrize("length", [64, 65])
|
||||
def test_codex_cache_scope_boundary(self, transport, length):
|
||||
session_id = "s" * length
|
||||
scope = transport.build_kwargs(
|
||||
model="gpt-5.4",
|
||||
messages=[{"role": "user", "content": "Hi"}],
|
||||
tools=[],
|
||||
session_id=session_id,
|
||||
is_codex_backend=True,
|
||||
request_overrides={"extra_headers": {"x-test": "1"}},
|
||||
)["extra_headers"]
|
||||
|
||||
first = transport.build_kwargs(**kwargs)["extra_headers"]["session_id"]
|
||||
second = transport.build_kwargs(**kwargs)["extra_headers"]["session_id"]
|
||||
assert scope["x-test"] == "1"
|
||||
assert len(scope["session_id"]) <= 64
|
||||
assert scope["x-client-request-id"] == scope["session_id"]
|
||||
if length == 64:
|
||||
assert scope["session_id"] == session_id
|
||||
else:
|
||||
assert scope["session_id"].startswith("pck_")
|
||||
assert scope["session_id"] != session_id
|
||||
|
||||
assert first == second
|
||||
def test_codex_backend_overlength_cache_scope_is_stable_and_collision_resistant(self, transport):
|
||||
common = "paperclip:company:" + "a" * 80
|
||||
|
||||
def cache_scope(session_id):
|
||||
return transport.build_kwargs(
|
||||
model="gpt-5.4",
|
||||
messages=[{"role": "user", "content": "Hi"}],
|
||||
tools=[],
|
||||
session_id=session_id,
|
||||
is_codex_backend=True,
|
||||
)["extra_headers"]["session_id"]
|
||||
|
||||
assert cache_scope(common + "1") == cache_scope(common + "1")
|
||||
assert cache_scope(common + "1") != cache_scope(common + "2")
|
||||
|
||||
def test_long_override_keys_are_bounded_at_build_and_preflight(self, transport):
|
||||
long_key = "paperclip:" + "x" * 130
|
||||
kwargs = transport.build_kwargs(
|
||||
model="gpt-5.4",
|
||||
messages=[{"role": "user", "content": "Hi"}],
|
||||
tools=[],
|
||||
request_overrides={"prompt_cache_key": long_key},
|
||||
)
|
||||
assert len(kwargs["prompt_cache_key"]) <= 64
|
||||
|
||||
middleware_payload = dict(kwargs)
|
||||
middleware_payload["prompt_cache_key"] = long_key
|
||||
preflight = transport.preflight_kwargs(middleware_payload)
|
||||
assert preflight["prompt_cache_key"].startswith("pck_")
|
||||
assert len(preflight["prompt_cache_key"]) <= 64
|
||||
|
||||
def test_xai_long_override_key_is_bounded_at_build_and_preflight(self, transport):
|
||||
long_key = "paperclip:" + "x" * 130
|
||||
kwargs = transport.build_kwargs(
|
||||
model="grok-4.3",
|
||||
messages=[{"role": "user", "content": "Hi"}],
|
||||
tools=[],
|
||||
is_xai_responses=True,
|
||||
request_overrides={"extra_body": {"prompt_cache_key": long_key}},
|
||||
)
|
||||
assert len(kwargs["extra_body"]["prompt_cache_key"]) <= 64
|
||||
|
||||
middleware_payload = dict(kwargs)
|
||||
middleware_payload["extra_body"] = {"prompt_cache_key": long_key}
|
||||
preflight = transport.preflight_kwargs(middleware_payload)
|
||||
assert preflight["extra_body"]["prompt_cache_key"].startswith("pck_")
|
||||
assert len(preflight["extra_body"]["prompt_cache_key"]) <= 64
|
||||
|
||||
def test_codex_backend_no_headers_without_session_id(self, transport):
|
||||
messages = [{"role": "user", "content": "Hi"}]
|
||||
|
|
|
|||
|
|
@ -1163,6 +1163,35 @@ def test_copilot_final_preflight_sanitizes_both_middleware_layers(monkeypatch):
|
|||
]
|
||||
|
||||
|
||||
def test_codex_final_preflight_bounds_middleware_cache_key(monkeypatch):
|
||||
"""Execution middleware cannot reintroduce an over-length provider key."""
|
||||
agent = _build_agent(monkeypatch)
|
||||
setattr(agent, "_disable_streaming", True)
|
||||
captured = {}
|
||||
long_key = "paperclip:" + "x" * 130
|
||||
|
||||
def _execution_middleware(request, next_call, **_context):
|
||||
replacement = dict(request)
|
||||
replacement["prompt_cache_key"] = long_key
|
||||
return next_call(replacement)
|
||||
|
||||
def _capture_api_call(api_kwargs):
|
||||
captured.update(api_kwargs)
|
||||
return _codex_message_response("OK")
|
||||
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.middleware.run_llm_execution_middleware",
|
||||
_execution_middleware,
|
||||
)
|
||||
monkeypatch.setattr(agent, "_interruptible_api_call", _capture_api_call)
|
||||
|
||||
result = agent.run_conversation("Say OK")
|
||||
|
||||
assert result["completed"] is True
|
||||
assert captured["prompt_cache_key"].startswith("pck_")
|
||||
assert len(captured["prompt_cache_key"]) <= 64
|
||||
|
||||
|
||||
def test_run_conversation_codex_empty_output_with_output_text(monkeypatch):
|
||||
"""Regression: empty response.output + valid output_text should succeed,
|
||||
not trigger retry/fallback. The validation stage must defer to
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue