From 31abb696dd8f7452ae5ec66e0d55eaf28e59a62c Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Mon, 13 Jul 2026 09:47:07 -0400 Subject: [PATCH] fix(memory): honor explicit store=false on Responses requests (#2017) ## Description This PR addresses the source-backed `store=false` mutation documented inside #1944. Headroom currently injects Responses memory tools by silently flipping explicit `store=false` to `true`, which Codex-backed Responses requests reject. The fix respects explicit `store=false` by skipping only the tool-continuation memory path for those requests. Memory context injection stays unchanged. Refs #1944 ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - Honor explicit `store=false` on `/v1/responses`. - Skip only Responses memory tools that depend on stored-response continuation. - Preserve current behavior when the client does not opt out of storage. - Add focused regression coverage and a changelog note. ## Testing - [x] Unit tests pass - [x] Linting passes - [ ] Type checking passes - [x] New tests added for new functionality when applicable - [ ] Manual testing performed ### Test Output ```text uv run pytest tests/test_openai_responses_context_compaction.py -q 11 passed uv run ruff check headroom/proxy/handlers/openai.py tests/test_openai_responses_context_compaction.py All checks passed uv run ruff format --check headroom/proxy/handlers/openai.py tests/test_openai_responses_context_compaction.py 2 files already formatted ``` ## Real Behavior Proof - Environment: Responses client with explicit `store=false` - Exact command / steps: send a memory-enabled `/v1/responses` request with `store=false` - Observed result: the handler now preserves explicit `store=false` and skips only the Responses memory-tool injection path that depends on stored-response continuation; focused regression coverage proves stored/default requests still allow the path - Not tested: the Desktop disconnect tracked separately in #1944 ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [ ] I have commented my code, particularly in hard-to-understand areas - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] I have updated the CHANGELOG.md if applicable ## Additional Notes The large Codex Desktop mid-stream disconnect remains a separate external-proof problem. This PR is intentionally limited to the explicit `store=false` mutation proven in the same issue thread. Co-authored-by: JerrettDavis Co-authored-by: Tejas Chopra --- CHANGELOG.md | 1 + headroom/proxy/handlers/openai.py | 65 ++++++++----------- ...est_openai_responses_context_compaction.py | 41 +++--------- 3 files changed, 37 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9834eebe..874f799cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -102,6 +102,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Bug Fixes +* **memory:** honor explicit `store=false` on OpenAI `/v1/responses` requests by skipping Headroom memory-tool injection that depends on stored-response continuations. Memory context injection stays available, and requests no longer get rewritten to `store=true` behind the client's back ([#1944](https://github.com/headroomlabs-ai/headroom/issues/1944)). * **proxy/batch:** stop corrupting Google `batchGenerateContent` requests whose contents interleave text turns with text-less entries (functionCall/functionResponse/images). The batch handler restored preserved (non-text) entries with the raw-index loop that #836 replaced everywhere else — indexing the shorter `optimized_contents` (text-less entries produce no message) by the original `contents[]` index, which overwrites the wrong entry and drops any preserved entry whose original index is past the optimized length. A request like `[user text, model functionCall, user functionResponse, model text]` was forwarded to Google as two entries: the model's answer overwritten by the functionCall and the functionResponse dropped. The batch handler now uses the shared `_rebuild_gemini_contents` interleaving helper, so all entries survive in order. * **proxy/gemini:** preserve Gemini code-execution parts (`executableCode` / `codeExecutionResult`) across the compression round-trip. `_has_non_text_parts` only recognized `inlineData`/`fileData`/`functionCall`/`functionResponse`, so a content entry carrying code-execution parts was not marked as preserved. A mixed `text`+`executableCode` entry lost its code payload (only the text survived), and a text-less code-execution entry was treated as a phantom that dropped the entire turn and shifted a neighboring message into the wrong role slot. Both keys are now recognized so those entries are preserved verbatim. * **cache/ccr:** don't evict a live entry when a duplicate hash is re-stored at capacity. `CompressionStore.store` ran `_evict_if_needed()` before checking whether the key already existed, so re-storing an already-present hash while the store was full evicted the oldest *distinct* entry to "make room" and then merely overwrote the existing key in place — no room was ever needed. The store dropped below `max_entries` and a live, never-retrieved entry was destroyed, so its `<>` marker (still in the conversation) resolved to a 404. The CCR mirror bridge re-stores the same `explicit_hash` every turn a marker is re-encountered, so this fired routinely. Eviction now runs only for a genuinely new key. diff --git a/headroom/proxy/handlers/openai.py b/headroom/proxy/handlers/openai.py index 187b3511a..1f40e8e2f 100644 --- a/headroom/proxy/handlers/openai.py +++ b/headroom/proxy/handlers/openai.py @@ -603,27 +603,17 @@ def _compact_openai_responses_tools( return updated, True, before, after -def _ensure_responses_store_for_memory_tools( - payload: dict[str, Any], - *, - memory_tools_injected: bool, -) -> bool: - """Keep Responses API memory-tool continuations addressable. +def _responses_request_allows_memory_tool_continuation(payload: dict[str, Any]) -> bool: + """Return whether Responses memory tools may rely on stored continuations. - Memory tools are transparent to clients: Headroom executes the emitted - function_call, then sends function_call_output in a continuation request - using previous_response_id. OpenAI only allows that continuation when the - previous response was stored. Clients such as pi/Codex can set store=false - to avoid retaining ordinary responses, but that makes memory-tool - continuations fail with previous_response_not_found. - - Return True when this function changes the payload. + Headroom memory tools use ``previous_response_id`` continuations after a + tool call. Those continuations require the originating response to be + stored. When a client explicitly sends ``store=false``, preserve that + contract and skip the Responses memory-tool injection path instead of + mutating the request. """ - if memory_tools_injected and payload.get("store") is False: - payload["store"] = True - return True - return False + return payload.get("store") is not False def _responses_input_item_text_bytes(item: Any) -> int: @@ -4066,28 +4056,27 @@ class OpenAIHandlerMixin: else: memory_tool_defs_responses.append(t) - resp_tools = body.get("tools") or [] - resp_tools, mem_tools_injected = _apply_sticky_mem_tools_resp( - provider="openai", - session_id=_responses_session_id, - request_id=request_id, - existing_tools=resp_tools, - memory_tools_to_inject=memory_tool_defs_responses, - inject_this_turn=bool(self.memory_handler.config.inject_tools), - ) - if mem_tools_injected: - body["tools"] = resp_tools - body_mutation_tracker.mark_mutated("responses_memory_tools") - logger.info(f"[{request_id}] Memory: Injected memory tools (openai/responses)") - - if _ensure_responses_store_for_memory_tools( - body, - memory_tools_injected=True, - ): - body_mutation_tracker.mark_mutated("responses_memory_store") + if _responses_request_allows_memory_tool_continuation(body): + resp_tools = body.get("tools") or [] + resp_tools, mem_tools_injected = _apply_sticky_mem_tools_resp( + provider="openai", + session_id=_responses_session_id, + request_id=request_id, + existing_tools=resp_tools, + memory_tools_to_inject=memory_tool_defs_responses, + inject_this_turn=bool(self.memory_handler.config.inject_tools), + ) + if mem_tools_injected: + body["tools"] = resp_tools + body_mutation_tracker.mark_mutated("responses_memory_tools") logger.info( - f"[{request_id}] Memory: forced store=true for Responses memory tool continuation" + f"[{request_id}] Memory: Injected memory tools (openai/responses)" ) + elif self.memory_handler.config.inject_tools: + logger.info( + "[%s] Memory: skipped Responses memory tools because client set store=false", + request_id, + ) except Exception as e: logger.warning(f"[{request_id}] Memory injection failed (responses): {e}") elif self.memory_handler and memory_user_id and _bypass: diff --git a/tests/test_openai_responses_context_compaction.py b/tests/test_openai_responses_context_compaction.py index 6f5eff3ec..7802caef2 100644 --- a/tests/test_openai_responses_context_compaction.py +++ b/tests/test_openai_responses_context_compaction.py @@ -6,8 +6,8 @@ from typing import Any from headroom.proxy.handlers.openai import ( OpenAIHandlerMixin, _compact_openai_responses_tools, - _ensure_responses_store_for_memory_tools, _openai_responses_context_budget, + _responses_request_allows_memory_tool_continuation, ) from headroom.transforms.content_router import ( CompressionStrategy, @@ -438,48 +438,25 @@ def test_content_router_retries_kompress_when_structured_strategy_noops(monkeypa assert strategy_chain == ["smart_crusher", "kompress"] -def test_responses_memory_tools_store_false_regression() -> None: - """Regression: store=false makes previous_response_id continuations fail.""" +def test_responses_memory_tools_skip_explicit_store_false() -> None: + """Regression: explicit store=false must block Responses memory-tool injection.""" payload = {"model": "gpt-5.5", "input": "remember this", "store": False} - changed = _ensure_responses_store_for_memory_tools( - payload, - memory_tools_injected=True, - ) - - assert changed is True - assert payload["store"] is True + assert _responses_request_allows_memory_tool_continuation(payload) is False + assert payload["store"] is False -def test_responses_memory_tools_do_not_change_unrelated_requests() -> None: +def test_responses_memory_tools_allow_default_and_stored_requests() -> None: no_memory_payload = {"model": "gpt-5.5", "input": "plain", "store": False} already_stored_payload = {"model": "gpt-5.5", "input": "plain", "store": True} default_store_payload = {"model": "gpt-5.5", "input": "plain"} - assert ( - _ensure_responses_store_for_memory_tools( - no_memory_payload, - memory_tools_injected=False, - ) - is False - ) + assert _responses_request_allows_memory_tool_continuation(no_memory_payload) is False assert no_memory_payload["store"] is False - assert ( - _ensure_responses_store_for_memory_tools( - already_stored_payload, - memory_tools_injected=True, - ) - is False - ) + assert _responses_request_allows_memory_tool_continuation(already_stored_payload) is True assert already_stored_payload["store"] is True - assert ( - _ensure_responses_store_for_memory_tools( - default_store_payload, - memory_tools_injected=True, - ) - is False - ) + assert _responses_request_allows_memory_tool_continuation(default_store_payload) is True assert "store" not in default_store_payload