From c621241a2e308b6910b556696da40f1820ca9a1d Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Sat, 8 Aug 2026 20:59:37 -0400 Subject: [PATCH] test(proxy): harden system message ordering Cover supported late-system and tool-result ordering invariants. Refs #1687 --- src/proxy/transformers/request-transformer.ts | 11 +-- .../request-transformer-regressions.test.ts | 72 ++++++++++++++++++- 2 files changed, 77 insertions(+), 6 deletions(-) diff --git a/src/proxy/transformers/request-transformer.ts b/src/proxy/transformers/request-transformer.ts index a87c4dff..bde13890 100644 --- a/src/proxy/transformers/request-transformer.ts +++ b/src/proxy/transformers/request-transformer.ts @@ -672,7 +672,7 @@ function transformMessages(messagesValue: unknown): OpenAIMessage[] { } /** - * Hoist every `role: "system"` message to a single leading system message. + * Hoist every accepted `role: "system"` message to one leading system message. * * Claude Code sends the system prompt as the top-level `system` field *and*, * separately, sends skill/plugin listings as `role: "system"` entries inside @@ -687,9 +687,12 @@ function transformMessages(messagesValue: unknown): OpenAIMessage[] { * where a `system` message is not alone at index 0: * `400 A 'system' message can only appear at index 0 of the messages array.` * - * This pass extracts all `system` messages in encounter order, joins their - * content with a blank line, and reinserts the result as the sole leading - * message — content-preserving, order-preserving for everything else. + * Inline system messages may appear between complete turns, including after + * tool results. They may not interrupt a pending assistant tool-call/result + * sequence; `transformMessages` rejects that ambiguous placement before this + * pass. Accepted system messages are extracted in encounter order, joined with + * a blank line, and reinserted as the sole leading message. Everything else + * keeps its relative order before normal same-role coalescing. */ function hoistSystemMessages(messages: OpenAIMessage[]): OpenAIMessage[] { const systemParts: string[] = []; diff --git a/tests/unit/proxy/transformers/request-transformer-regressions.test.ts b/tests/unit/proxy/transformers/request-transformer-regressions.test.ts index bbeffec3..2497b2f4 100644 --- a/tests/unit/proxy/transformers/request-transformer-regressions.test.ts +++ b/tests/unit/proxy/transformers/request-transformer-regressions.test.ts @@ -332,11 +332,12 @@ describe('ProxyRequestTransformer regressions', () => { const result = new ProxyRequestTransformer().transform({ system: [{ type: 'text', text: 'You are Claude Code, a CLI tool.' }], messages: [ + { role: 'user', content: 'ping' }, { role: 'system', content: 'The following skills are available for use with the Skill tool:\n- foo', }, - { role: 'user', content: 'ping' }, + { role: 'user', content: 'pong' }, ], }); @@ -347,6 +348,73 @@ describe('ProxyRequestTransformer regressions', () => { content: 'You are Claude Code, a CLI tool.\n\nThe following skills are available for use with the Skill tool:\n- foo', }); - expect(result.messages[1]).toEqual({ role: 'user', content: 'ping' }); + expect(result.messages[1]).toEqual({ role: 'user', content: 'ping\npong' }); + }); + + it('hoists a late system message after complete parallel tool results without disturbing tool order', () => { + const result = new ProxyRequestTransformer().transform({ + system: 'base instructions', + messages: [ + { role: 'user', content: 'inspect both files' }, + { + role: 'assistant', + content: [ + { type: 'tool_use', id: 'toolu_1', name: 'read', input: { path: 'a.ts' } }, + { type: 'tool_use', id: 'toolu_2', name: 'read', input: { path: 'b.ts' } }, + ], + }, + { + role: 'user', + content: [ + { type: 'tool_result', tool_use_id: 'toolu_1', content: 'a contents' }, + { type: 'tool_result', tool_use_id: 'toolu_2', content: 'b contents' }, + ], + }, + { role: 'system', content: 'late instructions' }, + { role: 'user', content: 'compare them' }, + ], + }); + + expect(result.messages).toEqual([ + { role: 'system', content: 'base instructions\n\nlate instructions' }, + { role: 'user', content: 'inspect both files' }, + { + role: 'assistant', + content: '', + tool_calls: [ + { + id: 'toolu_1', + type: 'function', + function: { name: 'read', arguments: '{"path":"a.ts"}' }, + }, + { + id: 'toolu_2', + type: 'function', + function: { name: 'read', arguments: '{"path":"b.ts"}' }, + }, + ], + }, + { role: 'tool', tool_call_id: 'toolu_1', content: 'a contents' }, + { role: 'tool', tool_call_id: 'toolu_2', content: 'b contents' }, + { role: 'user', content: 'compare them' }, + ]); + }); + + it('rejects a system message inserted before pending tool results', () => { + expect(() => + new ProxyRequestTransformer().transform({ + messages: [ + { + role: 'assistant', + content: [{ type: 'tool_use', id: 'toolu_1', name: 'read', input: { path: 'a.ts' } }], + }, + { role: 'system', content: 'interrupting instructions' }, + { + role: 'user', + content: [{ type: 'tool_result', tool_use_id: 'toolu_1', content: 'a contents' }], + }, + ], + }) + ).toThrow('role must be "user" with tool_result blocks after assistant tool_use'); }); });