Skip to content

fix(memory): avoid rejected mid-conversation system injection on Claude when preceding turn isn't a tool result - #11303

Merged
diegosouzapw merged 1 commit into
release/v3.8.50from
fix/11290-opus-memory-injection-position
Aug 24, 2026
Merged

diegosouzapw merged 1 commit into
release/v3.8.50from
fix/11290-opus-memory-injection-position

Conversation

@diegosouzapw

Copy link
Copy Markdown
Owner

Summary

Closes #11290.

Claude Opus 5 rejects OmniRoute's #3890 cache-safe mid-array system-message
splice with HTTP 400 in one specific shape: when the assistant turn
immediately before the splice point is plain text rather than a server-side
tool result (*_tool_result content block). injectMemory()
(src/lib/memory/injection.ts) always spliced the memory-context system
message right before the last user turn whenever cacheSafe was set and the
provider was not in the systemMessageMustBeFirst() strict set — and
anthropic/claude were never in that set, so every Claude cache-safe
request hit the same mid-array shape regardless of what preceded it.

Design decision

Two options were on the table:

  1. Add claude/anthropic to BUILTIN_PROVIDERS_SYSTEM_MUST_BE_FIRST
    (systemMessageMustBeFirst()), forcing every Claude request onto the
    leading-system-message path.
  2. Narrow the condition so the mid-array splice is only skipped for the
    specific requests that trigger the 400.

Option 1 is the simplest change but reverts the #3890 prompt-cache-hit
optimization for every Claude request that uses prompt caching, including
the (presumably majority) requests where the preceding turn already ends in a
tool result and the splice works fine — a real cost regression for no gain
in those cases.

This PR takes option 2: injectMemory() gained a narrower, additive check
that only applies when cacheSafe is on, the provider is in the Claude
family (claude, anthropic, isAnthropicCompatibleProvider() — which also
covers isClaudeCodeCompatibleProvider()'s narrower prefix), and the message
immediately before the splice point is not an assistant turn ending in a
*_tool_result content block. In that case it falls back to the existing
injectSystemFirst() leading-message placement (merging into the request's
system message, exactly like the strict-provider path already does) instead
of splicing. The mid-array splice is preserved everywhere it is safe: for all
non-Claude providers unconditionally, and for Claude providers whose
preceding turn does end in a server tool result.

Changes

  • src/lib/memory/injection.ts: added isClaudeFamilyProvider() and
    endsWithServerToolResult() helpers, and a new gate in injectMemory()
    applied after the existing systemMessageMustBeFirst() check.
  • src/lib/memory/__tests__/injection.test.ts: TDD regression test proving
    the fallback (RED before the fix, GREEN after — verified locally by
    reverting injection.ts and re-running the new test in isolation).
  • tests/unit/memory-cache-safe-injection.test.ts: the 3 pre-existing
    anthropic-provider cache-safe tests encoded exactly the now-fixed buggy
    shape (mid-array splice after a plain-text assistant turn), so they were
    switched to a non-Claude-family provider (openai) to keep testing the
    original #3890 generic mechanism without regressing it. Added a new
    describe block with 4 tests covering: the plain-text fallback, the
    preserved splice when the preceding turn is a server tool result, the
    Claude-Code-compatible passthrough provider id, and confirmation that
    non-Claude providers are unaffected.

Test plan

  • node --import tsx/esm --test tests/unit/memory-cache-safe-injection.test.ts tests/unit/memory-glm-injection.test.ts tests/unit/chatcore-memory-skills-injection.test.ts — all green (39 tests)
  • npx vitest run src/lib/memory/__tests__/injection.test.ts — all green (23 tests)
  • Verified RED→GREEN: reverted injection.ts to the pre-fix version, re-ran the new #11290 vitest test — it failed as expected (mid-array splice still occurred), then restored the fix — it passed.
  • npx eslint on the 3 changed files — clean (no new no-explicit-any beyond the pre-existing, suppressed count for memory-cache-safe-injection.test.ts).
  • tsc --pretty false -p tsconfig.typecheck-core.json — clean, no errors.
  • npm run check:cycles — no cycles introduced by the new src/shared/constants/providers import.

⚠️ base-red inherited: #9985

…de when preceding turn isn't a tool result (#11290)

Claude Opus 5 rejects the #3890 cache-safe mid-array system splice with
HTTP 400 when the assistant turn immediately before the splice point is
plain text rather than a server-side tool result. Adding claude/anthropic
outright to systemMessageMustBeFirst() would revert the #3890 cache-hit
optimization for every Claude request, so instead injectMemory() now only
falls back to leading-system-message placement for the specific Claude-
family requests where that condition holds, preserving the mid-array
splice everywhere it remains safe (non-Claude providers unconditionally,
and Claude providers whose preceding turn does end in a server tool
result).
@diegosouzapw
diegosouzapw merged commit 3daa455 into release/v3.8.50 Aug 24, 2026
17 of 22 checks passed
@diegosouzapw
diegosouzapw deleted the fix/11290-opus-memory-injection-position branch August 25, 2026 02:36
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…de when preceding turn isn't a tool result (diegosouzapw#11290) (diegosouzapw#11303)

Validated on a 4-PR combined board: memory-cache-safe-injection + memory-glm-injection + chatcore-memory-skills-injection 39/39 (node:test), injection.test.ts 23/23 (vitest), typecheck:core clean, check:cycles clean, gates within baseline. Narrows the diegosouzapw#3890 mid-array system-splice skip to the exact shape that 400s on Claude Opus 5 (splice point not preceded by a server tool result) instead of disabling the cache-safe optimization for every Claude request — preserves the fast path everywhere it's actually safe. Closes diegosouzapw#11290.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Opus 5 rejects cache-safe memory system message after assistant text

2 participants