Skip to content

fix(claude): keep mid-conversation system messages in the timeline - #4161

Merged
lidge-jun merged 1 commit into
devfrom
lane-a/2-4148
Sep 9, 2026
Merged

fix(claude): keep mid-conversation system messages in the timeline#4161
lidge-jun merged 1 commit into
devfrom
lane-a/2-4148

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 9, 2026

Copy link
Copy Markdown
Owner

Summary

Claude Code sends role: "system" entries inside messages, and every one of them was folded into instructions. parseRequest pushes data.instructions onto the system prompt before anything else, so each injected reminder rewrote the head of the prompt — which is exactly the span an upstream KV cache matches on. Every mid-conversation reminder therefore invalidated the cache prefix. When the client sends no metadata.user_id, the Desktop prompt_cache_key fallback hashes the same system parts, so the cache key rotated along with the prefix and the request could not even land on the same routing cohort.

Each non-empty in-messages system message now becomes a chronological input item:

{ "type": "message", "role": "developer", "content": [{ "type": "input_text", "text": "..." }] }

Top-level Anthropic system keeps flowing through systemToInstructions into body.instructions unchanged. instructions now has exactly one source and stops moving mid-conversation.

Why developer and not system. The schema admits a system role in input, but ChatGPT Codex rejects it live, parseRequest re-hoists it onto the system prompt, and the canonical Responses forward folds text-only system items back into instructions — so it would land back where it started. developer is first-class in the schema, survives parseRequest as an ordinary chronological message, and preserves timeline order.

The original fold was deliberate, not an oversight: cee918ce3 introduced it so the native ChatGPT backend would not 400 on a system input item. That constraint is real and is still respected here — it rules out system, not developer. What turned out to be stale is the accompanying 2026-07-11 note that folding is "the only shape that works on every route".

A scope decision a reviewer may want to object to

All in-messages system messages become developer items, not only the ones after the first user turn.

The narrower alternative — hoist a leading system message, keep only mid-conversation ones in the timeline — would have left the old contract test untouched, but it does not close the issue. A client that injects a fresh leading system message on every turn still mutates instructions every turn, and that is the reported failure. Taking the narrow option would have meant shipping something that looks like a fix and still rotates the prefix for the reporter.

The cost is that privileged system text now arrives as developer text. On Anthropic and Google outbound, developer items are presented as chronological user messages, so there is real semantic drift from "this is a system instruction" to "this is part of the conversation". That is prefix-stable, which is the property this change is buying.

systemParts deliberately keeps its array shape inside the cache-key hash. Joining it to a string would rotate every existing cohort key for a reason unrelated to this bug.

Deliberately out of scope

src/adapters/openai-chat.ts re-hoists all text developer messages into a leading system chat message for non-api.openai.com Chat Completions, locked by tests/adapters/openai/openai-chat-system-order.test.ts. That is what keeps the reporter's DeepSeek/SenseNova path broken, and reversing it is a separate compatibility tradeoff with its own regression surface. OpenCode Go's Muse is openai-responses, so this inbound fix does reach it. src/chat/inbound.ts has the same anti-pattern for Chat inbound; also not this issue.

Verification

Remote CI at this PR's exact head SHA is the gate for this change.

Local checks: NOT RUN. bun test, bun run test:changed, bun run typecheck, bun install, bun run build:gui, bun run lint:gui, and bun run privacy:scan were all skipped by explicit maintainer instruction for this delivery round, which overrides the PR-ready gate in AGENTS.md. Nothing here claims a local check passed.

Independent review that was done: a read-only reviewer confirmed that developer is admitted by src/responses/schema.ts and kept chronological by src/responses/parser.ts while a system item would be re-hoisted; that the prompt_cache_key fallback hashes an array whose shape is unchanged; that the canonical Responses forward folds only role: "system", so developer items survive it; and — the part that mattered most — searched tests/ exhaustively for any other assertion encoding the old fold and found none.

Regression coverage in tests/claude-integration/claude-inbound.test.ts:

  • the old fold-contract case is rewritten, not deleted. It is the test that encoded the previous behavior, so it now asserts instructions === "top-level" and input roles ["developer", "developer", "user"], and it still asserts that no input item carries role: "system".
  • a new two-turn case: system: "S" with a reminder injected on each turn. instructions is "S" on both turns, turn 2's roles are ["user", "developer", "assistant", "user", "developer"] with texts u1, r1, a1, u2, r2, turn 1's items are a byte-equal prefix of turn 2's, and with no metadata.user_id the two turns produce the same prompt_cache_key. Both bodies pass responsesRequestSchema.parse and parseRequest.

The cases using top-level system and the prompt-cache-key provenance suite are untouched and must stay green.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed. No documented behavior changes: the Anthropic surface contract is unchanged, and the wire shape this produces was already valid.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. No credential, auth, or logging path is touched; message text that was already being forwarded is forwarded in a different position.

Closes #4148.

Summary by CodeRabbit

  • Bug Fixes
    • Anthropic system messages included within a conversation are now preserved in chronological order as developer messages.
    • Top-level system instructions remain separate and are used exclusively for the conversation’s instructions.
    • Mid-conversation system reminders no longer alter prompt-cache prefixes or cache keys.

Claude Code sends role:"system" entries inside `messages`, and every one of
them was folded into `instructions`. The parser pushes `data.instructions`
onto the system prompt before anything else, so each injected reminder rewrote
the head of the prompt and invalidated the upstream KV cache prefix. Where no
metadata.user_id is present, the Desktop prompt_cache_key fallback hashes the
same system parts, so the cache key rotated along with it.

Emit each non-empty in-messages system message as a chronological
{ type: "message", role: "developer", content: [{ type: "input_text", text }] }
input item instead. Top-level Anthropic `system` keeps flowing through
systemToInstructions into body.instructions unchanged, so `instructions` now
has exactly one source and stops moving mid-conversation.

Not role:"system" in `input`. The schema allows it, but ChatGPT Codex rejects
it, parseRequest re-hoists it onto the system prompt, and the canonical
Responses forward folds text-only system items back into instructions - so it
would land back where it started. `developer` is first-class in the schema and
keeps timeline order.

The fold was deliberate in origin (cee918c) so native ChatGPT would not 400
on a system input item. That constraint is real and still respected; the
2026-07-11 note that folding is "the only shape that works on every route" is
what turned out to be stale.

systemParts keeps its array shape in the cache-key hash on purpose. Joining it
to a string would rotate every existing cohort key for a reason unrelated to
this bug.

Closes #4148.
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 9, 2026 23:01
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-09T23:07:17.410373Z 799330b PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Sep 9, 2026
@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 7c87c03d-cb5c-44de-8467-eed20a4a07c9

📥 Commits

Reviewing files that changed from the base of the PR and between 4498fb9 and 799330b.

📒 Files selected for processing (2)
  • src/claude/inbound.ts
  • tests/claude-integration/claude-inbound.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.


📝 Walkthrough

Walkthrough

Claude inbound translation now keeps top-level system content in instructions and emits message-level system entries as chronological Responses developer items. Integration tests validate ordering, schema parsing, input-prefix preservation, and stable prompt cache keys.

Changes

Claude system translation

Layer / File(s) Summary
Translate message-level system entries
src/claude/inbound.ts
Message-level system entries are appended to input as developer messages. Only top-level system content populates instructions.
Validate ordering and cache stability
tests/claude-integration/claude-inbound.test.ts
Tests validate chronological developer items, schema and parser compatibility, unchanged prompt prefixes, and stable Desktop prompt_cache_key values.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 79933

Message-level Claude system messages now remain chronological developer inputs while top-level instructions stay stable, preserving prompt-cache reuse without leaving a concrete current-head merge risk.

Sequence Diagram(s)

sequenceDiagram
  participant ClaudeMessages
  participant InboundTranslator
  participant ResponsesRequest
  participant PromptCache
  ClaudeMessages->>InboundTranslator: Send top-level and message-level system content
  InboundTranslator->>ResponsesRequest: Set top-level content as instructions
  InboundTranslator->>ResponsesRequest: Append message-level content as developer input items
  ResponsesRequest->>PromptCache: Preserve the existing input prefix
Loading

Suggested reviewers: invalid-email-address

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the Claude inbound fix and the preservation of mid-conversation system messages in the timeline. It is concise and directly related to the primary change.
Linked Issues check ✅ Passed The changes satisfy issue #4148. src/claude/inbound.ts converts embedded Anthropic role:"system" messages into chronological developer input items, keeps top-level system content as body.instructions,…
Out of Scope Changes check ✅ Passed The changes remain within the linked issue scope. The source change fixes Claude inbound system-message translation, and the test changes add regression coverage for timeline ordering and prompt-cache…
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch lane-a/2-4148

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 74 / 80

이 PR은 이슈 #4148을 닫는 Lane A 다음 단위다. 바로 직전 tip #4157(콤보 shadow failover, #4129)이 dev에 들어간 뒤, 로드맵이 가리키던 Claude 인바운드 시스템 메시지 접기 문제가 이 브랜치다. 베이스는 dev, 헤드는 lane-a/2-4148이다. 지금 dev HEAD(4498fb910, 패키지 2.50.0)의 src/claude/inbound.tsmessages 안의 role:"system"을 전부 systemParts에 모아 body.instructions로 합친다. Claude Code가 대화 중간에 리마인더·날짜·서브에이전트 안내를 넣으면, 그 텍스트가 매 턴 프롬프트 맨 앞(instructions)을 다시 쓰게 된다. 업스트림 KV 캐시가 맞추는 접두가 깨지고, metadata.user_id가 없을 때 Desktop prompt_cache_key 폴백도 같은 systemParts를 해시해서 캐시 키까지 같이 돈다. 보고된 실패는 “중간에 시스템 문구가 붙는다”가 아니라 “붙을 때마다 캐시 접두와 코호트가 리셋된다”다.

고치는 방법은 단순하다. 비어 있지 않은 in-messages system은 시간 순서대로 Responses input{ "type": "message", "role": "developer", "content": [{ "type": "input_text", "text": "..." }] }로 넣는다. 최상위 Anthropic system만 기존처럼 systemToInstructionsbody.instructions로 간다. 그래서 instructions 출처가 하나이고, 중간 턴에 흔들리지 않는다. role:"system"input에 넣지 않는 이유는 예전과 같다. 네이티브 ChatGPT는 system input을 거부하고, parseRequest가 다시 system prompt로 끌어올리고, canonical Responses forward도 text-only system을 instructions로 접는다. developer는 스키마(src/responses/schema.ts)에서 1급이고 parser.ts에서 시간순 메시지로 남는다. 원래 접기(cee918ce3)는 ChatGPT 400을 피하려는 의도였고, 그 제약 자체는 여전히 맞다. 틀린 건 “접는 것만이 모든 경로에서 된다”는 2026-07-11 메모였다.

범위 결정도 문서에 분명히 적혀 있다. 맨 앞 system만 접고 중간만 timeline에 남기는 좁은 안은 계약 테스트를 덜 건드렸겠지만, 매 턴 맨 앞에 새 system을 넣는 클라이언트는 여전히 instructions를 돌린다. 그래서 이번엔 in-messages system을 전부 developer로 보낸다. 대가는 Anthropic/Google 아웃바운드에서 developer가 시간순 user로 보이는 의미 드리프트다. 캐시 접두 안정성을 사는 선택이다. systemParts 해시가 배열 모양을 유지하는 것도 의도다. 문자열로 합치면 이 버그와 무관하게 기존 코호트 키가 전부 돌아간다. 고의로 뺀 것: src/adapters/openai-chat.ts가 비-api.openai.com Chat Completions에서 text developer를 다시 leading system으로 끌어올리는 경로(DeepSeek/SenseNova 쪽 잔존), src/chat/inbound.ts의 같은 패턴. OpenCode Go Muse는 openai-responses라 이번 인바운드 수정이 닿는다.

회귀는 tests/claude-integration/claude-inbound.test.ts에 있다. 옛 fold 계약 테스트는 지우지 않고 다시 썼다. instructions === "top-level", input roles ["developer","developer","user"], input에 role:"system" 없음을 본다. 새 두 턴 케이스는 system:"S"에 턴마다 리마인더를 넣고, 두 턴 모두 instructions"S", turn2 roles/texts가 u1,r1,a1,u2,r2, turn1 items가 turn2의 접두, metadata.user_id 없을 때 prompt_cache_key가 같음을 본다. 둘 다 responsesRequestSchema.parseparseRequest를 통과한다. 로컬 bun test 등은 이 배달 라운드 지시로 스킵했고, exact-head 원격 CI가 게이트다.

src/claude/inbound.ts (system 분기) - 옛 systemParts.push(text)를 없애고 developer input item만 push한다. top-level만 systemParts에 남는지, Desktop 캐시 키 폴백이 여전히 배열 systemParts만 해시하는지 머지 전에 한 번 더 눈으로 확인하면 좋다.
tests/claude-integration/claude-inbound.test.ts (rewritten fold + two-turn) - 옛 “접어라” 계약을 “developer로 남겨라”로 바꾼 점이 핵심이다. 다른 테스트가 옛 fold를 아직 기대한다면 CI에서 드러난다. PR 본문 기준으로는 tests/ 전수 검색에서 추가 단언이 없었다고 한다.
경로 src/adapters/openai-chat.ts · src/chat/inbound.ts - 이번 diff 밖이지만, Chat Completions 재호이스트와 Chat inbound 접기는 같은 계열 잔존이다. #4148을 닫아도 DeepSeek/SenseNova·Chat inbound 보고는 남을 수 있다. 별 이슈로 남겨 둘지 로드맵에 적을지 정하면 된다.
경로 CI · 로컬 스위트 - 제품 스위트 미실행은 라운드 정책과 맞다. exact-head CI가 초록이 될 때까지는 머지 재료가 아직 절반이다.

메인테이너의 판단이 필요한 지점

  • in-messages system을 전부 developer로 보내는 넓은 범위를 확정할지, 아니면 “중간만 timeline”으로 좁혀 의미 드리프트를 줄일지(PR 본문은 넓은 쪽이 실제 실패를 닫는다고 주장한다).
  • openai-chat 재호이스트·chat inbound 접기를 follow-up 이슈로 바로 팔지, Muse/Responses 경로만 이번 닫기로 끝낼지.
  • Lane A 다음 #4141(launchctl bootout)과의 순서: 이 PR CI만 보고 바로 머지할지, 서비스 격리 out-of-scope(fix(service): stop the test suite from mutating a live service manager #4152)와 겹치지 않는지만 한 번 더 볼지.

너의 추천
exact-head CI가 초록이면 dev에 머지하고 #4148을 닫아라. Lane A 로드맵상 #4129 다음 자리와 맞고, types/config 분할에 걸려 무효화될 형태도 아니며 중복 닫기 대상도 아니다. 머지 후 원 이슈에 Landed via #4161 at <commit>landed-via-maintainer를 붙이고, openai-chat/chat-inbound 잔존은 별 티켓으로만 남기면 된다.

이 댓글은 grok-bot이 작성했습니다

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 799330bcfc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/claude/inbound.ts
// schema and survives parseRequest as a chronological message, where `system`
// would be re-hoisted back onto the system prompt and defeat the point.
if (text.length > 0) {
input.push({ type: "message", role: "developer", content: [{ type: "input_text", text }] });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Avoid splitting tool calls from their results

When a role:"system" entry occurs between an assistant tool_use and its matching user tool_result, inserting this developer item breaks their adjacency. On the Ollama-native route, buildNativeMessages treats the developer item as a hard boundary and calls flushPending(), which throws because the following result has not been processed yet; the Anthropic and Google adapters similarly synthesize a missing result and later downgrade the real result to an orphan. This request shape worked before because the system entry was folded out of the message timeline, so defer such reminders until after the result batch or make each affected adapter preserve the pending pair across this barrier.

AGENTS.md reference: src/AGENTS.md:L19-L19

Useful? React with 👍 / 👎.

Comment thread src/claude/inbound.ts
// schema and survives parseRequest as a chronological message, where `system`
// would be re-hoisted back onto the system prompt and defeat the point.
if (text.length > 0) {
input.push({ type: "message", role: "developer", content: [{ type: "input_text", text }] });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Retain cache affinity without a top-level system field

For an accepted request that has in-message system entries but neither top-level system nor metadata.user_id, moving the text here leaves systemParts empty, so the fallback at the later else if (systemParts.length > 0) no longer emits any prompt_cache_key. The same request previously received a stable system-derived key, meaning clients that express all system context through messages now lose cache routing entirely. Track that this request contained system reminders and derive a stable model/tool cohort key that excludes their growing timeline text.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant