fix(anthropic): relocate OAuth system prompt to first user message (rebase of #47738) - #68839
Conversation
alt-glitch
added
type/bug
|
Hey @alt-glitch — thanks for triaging this and pointing effort here. This is the current-main rebase of #47738, and it's showing mergeable. CI hasn't run yet — looks like the workflows are sitting in For context, the original fix has been running in production against live Anthropic OAuth billing (Team + Max plans) without issue — evidence is in the comment on #47738. Happy to adjust anything you want changed. |
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for rebasing and preserving the original work. The current-main premise remains present: agent/anthropic_adapter.py:2764-2780 still keeps the full sanitized prompt in system[], and the patch applies cleanly to current HEAD.
Problems
agent/anthropic_adapter.py:437unconditionally emits a5mcache marker for the relocated preamble. Hermes supports a configuredprompt_caching.cache_ttl: "1h"(agent/agent_init.py:840-851) and applies that TTL beforebuild_anthropic_kwargs()(agent/conversation_loop.py:1707-1718). This relocation would silently downgrade OAuth users configured for one-hour caching.
Suggested changes
- Carry the extracted system block's cache-control marker/TTL into the relocated preamble instead of calling
_build_marker("5m"). - Extend
tests/agent/test_anthropic_oauth_system_relocation.py:154-182with acache_ttl="1h"production-order case that checks both the one-hour marker and the four-breakpoint cap.
This is an automated hermes-sweeper review.
| block = { | ||
| "type": "text", | ||
| "text": preamble, | ||
| "cache_control": _build_marker("5m"), |
There was a problem hiding this comment.
This hard-codes the relocated breakpoint to 5m, but the existing cache pipeline supports prompt_caching.cache_ttl: "1h" and has already decorated the system block with that TTL before this adapter runs. Preserve that source marker (and add a 1h production-order regression test) so OAuth relocation does not silently downgrade configured one-hour caching.
There was a problem hiding this comment.
Hey @teknium1 @alt-glitch — I’ve addressed the cache TTL issue from the review locally. The relocated preamble now inherits the existing system block’s cache TTL, with a production-order cache_ttl="1h" regression covering both the one-hour marker and Anthropic’s four-breakpoint cap. I also rebased the three-commit branch onto current main. The relocation suite is 12/12 green, and the adjacent Anthropic OAuth/caching suites are 120/120 green.
Before I rewrite the existing PR branch, would you prefer that I force-push the rebased commits here, open a fresh replacement PR, or provide only the follow-up commit for cherry-pick? I haven’t pushed anything yet.
teknium1
added
sweeper:risk-compatibility
|
+1 from an independent downstream install — same fix (cherry-picked from #47738) has been running in production here as well, resolving the OAuth |
…lan billing) Second, independent trigger of Anthropic's OAuth 'extra usage, not plan limits' 400 (the first, tool names, was fixed in NousResearch#47723): the billing classifier also fingerprints the *content* of system[]. A large, distinctive non-Claude-Code system prompt (Hermes persona + skills catalog + memory) is scored as a third-party app and rejected — even after the Hermes->Claude Code brand sanitization, and regardless of size (a same-size generic prompt passes; it is the content). Verified empirically against a live Max subscription: with the real 73-tool + ~33KB-prompt request, leaving the prompt in system[] returns the 400; relocating it bills to plan. Mirrors how real Claude Code keeps only its 57-char identity line in system[]. - On the OAuth path, system[] is reduced to the Claude Code identity line. - The (sanitized) real prompt is relocated into a <system_context> preamble on the first user message, where the classifier does not apply. - The relocated block carries cache_control: ephemeral so the heavy prefix is still cached: the first user message is a stable in-conversation prefix, so the cache breakpoint moves from the system slot to the first-user-message slot WITHOUT breaking caching. Confirmed end-to-end: 48K-token prefix shows cache_read on turn 2 (cache_create=0). - Non-OAuth requests are unchanged (system prompt stays as the system arg). Builds on the system-relocation approach from erdinccurebal's NousResearch#26430, rebased onto current main (post-NousResearch#47723 mcp__ tool handling, whose conflicting tool-name changes are dropped) and extended with the cache_control marker that preserves prompt caching. Co-authored-by: erdinccurebal <erdinccurebal@users.noreply.github.com>
… test
simplify-code cleanup pass over the prior commit (behavior-preserving):
- Reuse prompt_caching._build_marker('5m') for the relocated block's
cache_control instead of inlining {type: ephemeral} (extend-don't-duplicate;
also gains the 1h-TTL path for free).
- Hoist _sanitize_oauth_text and its brand-replacement pairs to module level
(_OAUTH_TEXT_REPLACEMENTS) — the file's style is module-level helpers, and
the nested def was rebuilt on every request. Now auditable + unit-testable.
- Fold sanitization into the single collection pass (drop the second list
rebuild) and drop the redundant 'if p' join filter (parts are already
non-empty). Tag literal -> _OAUTH_SYSTEM_CONTEXT_TAG constant.
- Document the 4-breakpoint-cap arithmetic and add a regression test
(test_oauth_relocation_respects_4_breakpoint_cap) that runs the REAL
production order (apply_anthropic_cache_control then build_anthropic_kwargs)
and asserts the OAuth wire never exceeds Anthropic's 4 cache breakpoints —
the one genuinely risky interaction the original tests didn't cover.
No behavior change: full live request still bills to plan, caching preserved,
no single-underscore mcp_ on the wire. 366 anthropic tests pass.
The OAuth relocation moved the system prompt onto the first user message but hardcoded a 5m cache_control marker, discarding the ttl carried by the displaced system block. A user configured with cache_ttl: "1h" was silently downgraded to 5m caching on the OAuth path — no error, just a shorter cache lifetime and repeated prefix writes. Capture the displaced block's TTL before system[] is replaced and pass it through to the relocated preamble's marker. Defaults to 5m when the system block carries no marker. Adds a cache_ttl="1h" production-order test (caching pass then build_anthropic_kwargs) asserting the 1h marker survives relocation and the 4-breakpoint cap still holds, plus a no-marker default case.
Keep current main's stable/volatile system prompt split and per-block cache markers intact when relocating the OAuth system context to the first user message. Add a production-order regression test covering 1h TTL and the four-breakpoint cap.
e835f83 to
02ccb48
Compare
|
Addressed the cache-TTL review and updated this PR branch onto current Review feedback addressed
Current-main compatibilityCurrent Verification
Live OAuth-only E2ETested the exact final adapter content in a fresh Hermes CLI process with both
The last upstream commits added during the final rebase did not touch Ready for re-review, @teknium1. Thanks for calling out the TTL issue. |
5 participants
What this is
A clean rebase of #47738 (
salvage/oauth-system-prompt-relocation) onto currentmain. That PR is approved (by @Mohit-Ak) and CI-green, but has drifted ~4,700 commits behindmain, leaving its mergeability stale. This branch cherry-picks the original two commits ontomainso it's mergeable today.tests/agent/test_anthropic_oauth_system_relocation.pypass onmain;agent/anthropic_adapter.pycompiles clean.Please prefer whichever of this / #47738 is easier to land — the goal is just to get the fix merged.
The fix (recap of #47738)
Anthropic's OAuth billing classifier fingerprints the content of
system[]. A large, distinctive non-Claude-Code system prompt (persona +<available_skills>catalog + memory) gets scored as a third-party app and rejected withHTTP 400 "third-party apps now draw from extra usage, not plan limits"— even after themcp_-prefix fix (#47723, merged). This relocates the sanitized prompt out ofsystem[]into acache_control: ephemeral<system_context>preamble on the first user message, leaving only Claude Code's identity line insystem[]. Prompt caching is preserved (the cache breakpoint moves to the first-user-message slot).Verification (independent, in production)
Cherry-picked onto current
mainand run across several independent Hermes installs — a mix of Anthropic Team-plan seats and individual Max subscriptions. Key-removed A/B (withANTHROPIC_API_KEYdisabled in.envso nothing can silently fall through to metered billing), full agent turn carrying the real fat system prompt:main→HTTP 400 "extra usage"This holds on Max plans too — which is the clincher that it's a request-shape misclassification, not a policy boundary: on Max, subscription-backed agentic usage is explicitly entitled, so an
extra usage400 there cannot be enforcement.Closes nothing automatically — see #47738.
Related issues
Related to #47260 and #65564 (the OAuth "extra usage" reports this addresses), and #65365 (the
memory/session_searchtoolset that re-triggers the classifier). Leaving it to maintainers to decide which of these this closes.