mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
fix(proxy/anthropic): scope session id by top-level system prompt (#2070)
## Description
`SessionTrackerStore.compute_session_id`
(`headroom/cache/prefix_tracker.py`) computes a fallback
session id (when no `x-headroom-session-id` header is present) from
`model` + system-prompt text.
But it harvests system text **only** from `messages` entries with `role
== "system"`:
```python
for msg in messages:
if msg.get("role") == "system":
... # collect system text
system_content = json.dumps(system_parts, ...)
key = f"{model}:{system_content}"
```
Anthropic's `/v1/messages` carries the system prompt as a **top-level**
`body["system"]` field —
it never sends `role:"system"` entries inside `messages`. And
`x-headroom-session-id` is a
Headroom-internal header no client sends. So for every genuine Anthropic
request `system_parts`
is empty and the id collapses to `md5(f"{model}:[]")` — **every
conversation on the same model
shares one session id**, and therefore one `PrefixCacheTracker` and all
session-sticky state.
The colliding state cross-contaminates across conversations
(`anthropic.py:1052`):
- sticky `headroom_retrieve` / memory tools keyed purely on `session_id`
(no content guard) get
injected into another conversation's tool list — busting its tools cache
and adding tools its
client never requested;
- sticky `anthropic-beta` header tokens leak across conversations;
- `frozen_message_count` and the per-session compression cache
cross-contaminate.
(The sibling `StreamingMixin._get_session_key` already reads
`body.get("system")` and its docstring
claims to mirror `compute_session_id` — which it did not.)
Closes: no issue filed — found while auditing the session/prefix
tracker.
## Fix
Add an optional `system` parameter to `compute_session_id` and fold its
text (a plain string or a
list of `{"type":"text"}` blocks) into the hash. The Anthropic handler
passes `body.get("system")`.
OpenAI callers don't pass it (defaults to `None`), so their behavior is
unchanged.
## Type of Change
- [x] Bug fix (non-breaking change that fixes an issue)
## Changes Made
- `headroom/cache/prefix_tracker.py`: `compute_session_id` accepts an
optional `system` and folds it into the id.
- `headroom/proxy/handlers/anthropic.py`: pass
`system=body.get("system")` when computing the session id.
- `tests/test_cache/test_prefix_tracker.py`: add
`test_compute_session_id_distinguishes_top_level_system` (distinct
systems → distinct ids; list-form == string-form; `system=None`
unchanged).
## Testing
- [x] New regression test added
(`tests/test_cache/test_prefix_tracker.py`)
- [x] Linting/formatting clean — run with the CI-pinned `ruff==0.15.17`
- [ ] Full `pytest` deferred to CI (local-OOM reason below).
```text
$ uvx ruff@0.15.17 check headroom/cache/prefix_tracker.py headroom/proxy/handlers/anthropic.py tests/test_cache/test_prefix_tracker.py
All checks passed!
```
## Real Behavior Proof
- Environment: Windows 11, Python 3.10, headroom from this branch.
Importing `headroom` pulls in the torch/transformers stack and a full
`pytest` gets OOM-killed on this box, so I verified the hash logic with
a dependency-free script and left the full pytest to CI.
- Exact command / steps: computed ids for two conversations with the
same model and messages but different top-level `system` prompts,
through the old (never-folds-system) and new logic.
- Observed result: the old logic collapses both to one id (the leak);
the new logic separates them, folds list-form system the same as
string-form, and leaves the `system=None` (OpenAI) path unchanged:
```text
OLD: A=97d8857ba27010bb B=97d8857ba27010bb same=True
NEW: A=1e838c0f6e3980a6 B=18ec49bfa8240852 same=False
SESSION-ID SYSTEM FIX VERIFIED (old collapses Anthropic convos; new separates them)
```
- Not tested: a full two-conversation proxy run asserting no sticky-tool
leakage (needs the heavy stack). The fix is confined to
`compute_session_id` + the one handler call site, and the new test
drives the method directly. Full local `pytest` deferred to CI (OOM, per
above).
## 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
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] 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
- [ ] New and existing unit tests pass locally with my changes — ran
lint + a standalone logic check; full pytest deferred to CI (local OOM,
disclosed above)
- [x] I have updated the CHANGELOG.md if applicable
## Additional Notes
- Backward-compatible: the new `system` parameter defaults to `None`, so
the OpenAI call sites (`openai.py`) need no change and their session ids
are identical.
- @JerrettDavis tagging you — this one lets one Anthropic conversation's
sticky tools/headers leak into another on the same model, so it seemed
worth surfacing. Thanks!
This commit is contained in:
parent
cbb775015e
commit
ec6e60ea3e
4 changed files with 57 additions and 2 deletions
|
|
@ -102,6 +102,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
### Bug Fixes
|
||||
|
||||
* **ccr:** don't crash `parse_tool_call` on a CCR tool call whose arguments aren't an object. For the OpenAI/`openai_responses` shape the arguments are `json.loads`-decoded and only `JSONDecodeError` was caught, so a model that emitted `arguments='[]'`/`'"abc"'`/`'123'` (decoding to a list/str/number) — or a non-dict Anthropic `input` — reached `input_data.get("hash")` and raised `AttributeError`; a null `arguments` raised an uncaught `TypeError` from `json.loads(None)`. Both are now handled: the decode also catches `TypeError`, and a non-dict `input_data` returns `None` (not a valid CCR call) instead of crashing CCR response processing.
|
||||
* **proxy/anthropic:** give each Anthropic conversation its own session id. `SessionTrackerStore.compute_session_id` derived its fallback id from `model` + system text harvested only from `role:"system"` entries inside `messages` — but Anthropic carries the system prompt as a top-level `body["system"]` field, so genuine Anthropic requests (which never carry `x-headroom-session-id`) collapsed to `md5(model:[])` and every conversation on the same model shared one `PrefixCacheTracker`. That let session-sticky state cross-contaminate: conversation A's sticky `headroom_retrieve`/memory tools and `anthropic-beta` headers were injected into conversation B, and frozen-prefix/compression-cache state mixed across conversations. The Anthropic handler now folds the top-level `system` into the session-id inputs (prepending a synthetic `role:"system"` message used only to derive the id), giving distinct conversations distinct ids.
|
||||
* **cache/semantic:** key entries by the full-context hash, not the trailing query text. `SemanticCache.put` stored each response under `sha256(query)[:16]` where `query` is only the last user message, and the exact-match branch of `get` returned the slot without checking the stored entry's `messages_hash`. Two requests that share a trailing message ("continue", "yes", "run the tests") but differ in earlier context therefore collided on one slot — the second overwrote the first, and the first's hash then resolved to the second's cached response (wrong data served). Entries are now keyed by `messages_hash` when present, and `get` verifies `entry.messages_hash` before returning.
|
||||
* **proxy/openai:** stop PRE_SEND from reintroducing `tools: []` after the direct #728 fix. The OpenAI request handler now mirrors the existing `tools or _original_tools is not None` body-write guard during PRE_SEND write-back, so providers that reject empty tool arrays no longer see a tools field when the client omitted it, while explicit client `tools: []` remains preserved ([#1983](https://github.com/headroomlabs-ai/headroom/issues/1983)).
|
||||
* **proxy/openai:** keep the exact Responses function name `terminal` resident during OpenAI tool-search deferral so cache-mode optimization stops forwarding `terminal.terminal` and triggering the reserved-namespace 400 on Codex Responses ([#1946](https://github.com/headroomlabs-ai/headroom/issues/1946)).
|
||||
|
|
|
|||
8
headroom/cache/prefix_tracker.py
vendored
8
headroom/cache/prefix_tracker.py
vendored
|
|
@ -806,6 +806,14 @@ class SessionTrackerStore:
|
|||
Priority:
|
||||
1. x-headroom-session-id header (explicit)
|
||||
2. Hash of (model + system prompt) — stable per conversation
|
||||
|
||||
The system prompt is harvested from ``role:"system"`` entries in
|
||||
``messages``. Anthropic carries the system prompt as a top-level
|
||||
``body["system"]`` field instead, so its handler prepends that as a
|
||||
synthetic ``role:"system"`` message before calling this — otherwise every
|
||||
Anthropic conversation on the same model would collapse to one session id
|
||||
and their session-sticky state (CCR/memory tools, beta headers, frozen
|
||||
prefix) would cross-contaminate.
|
||||
"""
|
||||
# Check for explicit session header
|
||||
if hasattr(request, "headers"):
|
||||
|
|
|
|||
|
|
@ -1048,8 +1048,22 @@ class AnthropicHandlerMixin:
|
|||
optimized_messages = messages
|
||||
optimized_tokens = original_tokens
|
||||
|
||||
# Get prefix cache tracker for this session
|
||||
session_id = self.session_tracker_store.compute_session_id(request, model, messages)
|
||||
# Get prefix cache tracker for this session. Anthropic carries the
|
||||
# system prompt as a top-level field, not a role:"system" message, so
|
||||
# fold it into the session-id inputs as a synthetic system message —
|
||||
# otherwise every conversation on the same model would share one
|
||||
# session id (and its sticky CCR/memory tools, beta headers, and
|
||||
# frozen-prefix state). This synthetic message only derives the id; it
|
||||
# is never forwarded.
|
||||
system_prompt = body.get("system")
|
||||
session_messages = (
|
||||
[{"role": "system", "content": system_prompt}, *messages]
|
||||
if system_prompt is not None
|
||||
else messages
|
||||
)
|
||||
session_id = self.session_tracker_store.compute_session_id(
|
||||
request, model, session_messages
|
||||
)
|
||||
prefix_tracker = self.session_tracker_store.get_or_create(session_id, "anthropic")
|
||||
frozen_message_count = prefix_tracker.get_frozen_message_count()
|
||||
# Idle gap since the previous turn's response, snapshotted at fetch
|
||||
|
|
|
|||
|
|
@ -393,6 +393,38 @@ class TestSessionTrackerStore:
|
|||
|
||||
assert id_a != id_b
|
||||
|
||||
def test_compute_session_id_distinguishes_top_level_system(self, store):
|
||||
"""Anthropic carries the system prompt as a top-level field (not a
|
||||
role:'system' message). The handler folds it in as a synthetic system
|
||||
message so two conversations with the same model and turns but different
|
||||
system prompts get distinct ids — otherwise they share one tracker and
|
||||
their sticky state cross-contaminates. This exercises that mechanism."""
|
||||
|
||||
class MockRequest:
|
||||
headers = {}
|
||||
|
||||
turns = [{"role": "user", "content": "hello"}]
|
||||
|
||||
def with_system(system):
|
||||
# Mirror what handlers/anthropic.py does for the top-level system.
|
||||
return [{"role": "system", "content": system}, *turns]
|
||||
|
||||
id_a = store.compute_session_id(
|
||||
MockRequest(), "claude-3", with_system("You are a Python expert.")
|
||||
)
|
||||
id_b = store.compute_session_id(
|
||||
MockRequest(), "claude-3", with_system("You are a Rust expert.")
|
||||
)
|
||||
assert id_a != id_b
|
||||
|
||||
# A list-of-text-blocks system folds the same text as the string form.
|
||||
id_a_list = store.compute_session_id(
|
||||
MockRequest(),
|
||||
"claude-3",
|
||||
with_system([{"type": "text", "text": "You are a Python expert."}]),
|
||||
)
|
||||
assert id_a_list == id_a
|
||||
|
||||
def test_compute_session_id_is_stable_when_only_non_system_turns_change(self, store):
|
||||
"""Appending non-system turns should keep the same fallback session id."""
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue