diff --git a/internal/runtime/executor/github_copilot_executor.go b/internal/runtime/executor/github_copilot_executor.go index d1391480..2c640f93 100644 --- a/internal/runtime/executor/github_copilot_executor.go +++ b/internal/runtime/executor/github_copilot_executor.go @@ -385,7 +385,19 @@ func (e *GitHubCopilotExecutor) ExecuteStream(ctx context.Context, auth *cliprox if useResponses && from.String() == "claude" { chunks = translateGitHubCopilotResponsesStreamToClaude(bytes.Clone(line), ¶m) } else { - normalizedLine := normalizeGitHubCopilotReasoningField(bytes.Clone(line)) + // Strip SSE "data: " prefix before reasoning field normalization, + // since normalizeGitHubCopilotReasoningField expects pure JSON. + // Re-wrap with the prefix afterward for the translator. + normalizedLine := bytes.Clone(line) + if bytes.HasPrefix(line, dataTag) { + sseData := bytes.TrimSpace(line[len(dataTag):]) + if !bytes.Equal(sseData, []byte("[DONE]")) && gjson.ValidBytes(sseData) { + normalized := normalizeGitHubCopilotReasoningField(bytes.Clone(sseData)) + if !bytes.Equal(normalized, sseData) { + normalizedLine = append(append([]byte(nil), dataTag...), normalized...) + } + } + } chunks = sdktranslator.TranslateStream(ctx, to, from, req.Model, bytes.Clone(opts.OriginalRequest), body, normalizedLine, ¶m) } for i := range chunks { @@ -601,14 +613,6 @@ func isAgentInitiated(body []byte) bool { } } } - // Fallback: if ANY message has role "tool", this conversation - // involves tool use — mark as agent to cover compaction and - // multi-turn agent continuations. - for _, msg := range arr { - if msg.Get("role").String() == "tool" { - return true - } - } } return false @@ -740,19 +744,28 @@ func isCopilotUnsupportedBeta(beta string) bool { // (choices[].delta.reasoning_text) and non-streaming messages // (choices[].message.reasoning_text). The field is only renamed when // 'reasoning_content' is absent or null, preserving standard responses. +// All choices are processed to support n>1 requests. func normalizeGitHubCopilotReasoningField(data []byte) []byte { - // Non-streaming: choices[].message.reasoning_text - if rt := gjson.GetBytes(data, "choices.0.message.reasoning_text"); rt.Exists() && rt.String() != "" { - rc := gjson.GetBytes(data, "choices.0.message.reasoning_content") - if !rc.Exists() || rc.Type == gjson.Null || rc.String() == "" { - data, _ = sjson.SetBytes(data, "choices.0.message.reasoning_content", rt.String()) - } + choices := gjson.GetBytes(data, "choices") + if !choices.Exists() || !choices.IsArray() { + return data } - // Streaming: choices[].delta.reasoning_text - if rt := gjson.GetBytes(data, "choices.0.delta.reasoning_text"); rt.Exists() && rt.String() != "" { - rc := gjson.GetBytes(data, "choices.0.delta.reasoning_content") - if !rc.Exists() || rc.Type == gjson.Null || rc.String() == "" { - data, _ = sjson.SetBytes(data, "choices.0.delta.reasoning_content", rt.String()) + for i := range choices.Array() { + // Non-streaming: choices[i].message.reasoning_text + msgRT := fmt.Sprintf("choices.%d.message.reasoning_text", i) + msgRC := fmt.Sprintf("choices.%d.message.reasoning_content", i) + if rt := gjson.GetBytes(data, msgRT); rt.Exists() && rt.String() != "" { + if rc := gjson.GetBytes(data, msgRC); !rc.Exists() || rc.Type == gjson.Null || rc.String() == "" { + data, _ = sjson.SetBytes(data, msgRC, rt.String()) + } + } + // Streaming: choices[i].delta.reasoning_text + deltaRT := fmt.Sprintf("choices.%d.delta.reasoning_text", i) + deltaRC := fmt.Sprintf("choices.%d.delta.reasoning_content", i) + if rt := gjson.GetBytes(data, deltaRT); rt.Exists() && rt.String() != "" { + if rc := gjson.GetBytes(data, deltaRC); !rc.Exists() || rc.Type == gjson.Null || rc.String() == "" { + data, _ = sjson.SetBytes(data, deltaRC, rt.String()) + } } } return data diff --git a/internal/runtime/executor/github_copilot_executor_test.go b/internal/runtime/executor/github_copilot_executor_test.go index 909e4d32..90a5b26f 100644 --- a/internal/runtime/executor/github_copilot_executor_test.go +++ b/internal/runtime/executor/github_copilot_executor_test.go @@ -324,13 +324,13 @@ func TestApplyHeaders_XInitiator_AgentWhenLastUserButHistoryHasAssistant(t *test t.Parallel() e := &GitHubCopilotExecutor{} req, _ := http.NewRequest(http.MethodPost, "https://example.com", nil) - // When the last role is "user" but the conversation contains tool messages, - // the request is a continuation (e.g. tool result with attached text - // translated to a synthetic user message). Should be "agent". - body := []byte(`{"messages":[{"role":"user","content":"hello"},{"role":"assistant","content":"I will read the file"},{"role":"tool","content":"file contents..."},{"role":"user","content":"tool result here"}]}`) + // When the last role is "user" and the message contains tool_result content, + // the request is a continuation (e.g. Claude tool result translated to a + // synthetic user message). Should be "agent". + body := []byte(`{"messages":[{"role":"user","content":"hello"},{"role":"assistant","content":"I will read the file"},{"role":"user","content":[{"type":"tool_result","tool_use_id":"tu1","content":"file contents..."}]}]}`) e.applyHeaders(req, "token", body) if got := req.Header.Get("X-Initiator"); got != "agent" { - t.Fatalf("X-Initiator = %q, want agent (history has tool role)", got) + t.Fatalf("X-Initiator = %q, want agent (last user contains tool_result)", got) } } @@ -338,10 +338,11 @@ func TestApplyHeaders_XInitiator_AgentWithToolRole(t *testing.T) { t.Parallel() e := &GitHubCopilotExecutor{} req, _ := http.NewRequest(http.MethodPost, "https://example.com", nil) + // When the last message has role "tool", it's clearly agent-initiated. body := []byte(`{"messages":[{"role":"user","content":"hello"},{"role":"tool","content":"result"}]}`) e.applyHeaders(req, "token", body) if got := req.Header.Get("X-Initiator"); got != "agent" { - t.Fatalf("X-Initiator = %q, want agent (tool role exists)", got) + t.Fatalf("X-Initiator = %q, want agent (last role is tool)", got) } } @@ -392,17 +393,17 @@ func TestApplyHeaders_XInitiator_UserInMultiTurnNoTools(t *testing.T) { } } -func TestApplyHeaders_XInitiator_AgentCompactionWithToolHistory(t *testing.T) { +func TestApplyHeaders_XInitiator_UserFollowUpAfterToolHistory(t *testing.T) { t.Parallel() e := &GitHubCopilotExecutor{} req, _ := http.NewRequest(http.MethodPost, "https://example.com", nil) - // Compaction scenario: user prompt after a conversation with tool use history. - // The last message is a plain "user" message (compaction summary request), - // but the conversation contains tool messages → should be "agent". + // User follow-up after a completed tool-use conversation. + // The last message is a genuine user question — should be "user", not "agent". + // This aligns with opencode's behavior: only active tool loops are agent-initiated. body := []byte(`{"messages":[{"role":"user","content":"hello"},{"role":"assistant","content":[{"type":"tool_use","id":"tu1","name":"Read","input":{}}]},{"role":"tool","tool_call_id":"tu1","content":"file data"},{"role":"assistant","content":"I read the file."},{"role":"user","content":"What did we do so far?"}]}`) e.applyHeaders(req, "token", body) - if got := req.Header.Get("X-Initiator"); got != "agent" { - t.Fatalf("X-Initiator = %q, want agent (compaction with tool history)", got) + if got := req.Header.Get("X-Initiator"); got != "user" { + t.Fatalf("X-Initiator = %q, want user (genuine follow-up after tool history)", got) } } @@ -502,6 +503,61 @@ func TestApplyGitHubCopilotResponsesDefaults_NoReasoningEffort(t *testing.T) { } } +// --- Tests for normalizeGitHubCopilotReasoningField --- + +func TestNormalizeReasoningField_NonStreaming(t *testing.T) { + t.Parallel() + data := []byte(`{"choices":[{"message":{"content":"hello","reasoning_text":"I think..."}}]}`) + got := normalizeGitHubCopilotReasoningField(data) + rc := gjson.GetBytes(got, "choices.0.message.reasoning_content").String() + if rc != "I think..." { + t.Fatalf("reasoning_content = %q, want %q", rc, "I think...") + } +} + +func TestNormalizeReasoningField_Streaming(t *testing.T) { + t.Parallel() + data := []byte(`{"choices":[{"delta":{"reasoning_text":"thinking delta"}}]}`) + got := normalizeGitHubCopilotReasoningField(data) + rc := gjson.GetBytes(got, "choices.0.delta.reasoning_content").String() + if rc != "thinking delta" { + t.Fatalf("reasoning_content = %q, want %q", rc, "thinking delta") + } +} + +func TestNormalizeReasoningField_PreservesExistingReasoningContent(t *testing.T) { + t.Parallel() + data := []byte(`{"choices":[{"message":{"reasoning_text":"old","reasoning_content":"existing"}}]}`) + got := normalizeGitHubCopilotReasoningField(data) + rc := gjson.GetBytes(got, "choices.0.message.reasoning_content").String() + if rc != "existing" { + t.Fatalf("reasoning_content = %q, want %q (should not overwrite)", rc, "existing") + } +} + +func TestNormalizeReasoningField_MultiChoice(t *testing.T) { + t.Parallel() + data := []byte(`{"choices":[{"message":{"reasoning_text":"thought-0"}},{"message":{"reasoning_text":"thought-1"}}]}`) + got := normalizeGitHubCopilotReasoningField(data) + rc0 := gjson.GetBytes(got, "choices.0.message.reasoning_content").String() + rc1 := gjson.GetBytes(got, "choices.1.message.reasoning_content").String() + if rc0 != "thought-0" { + t.Fatalf("choices[0].reasoning_content = %q, want %q", rc0, "thought-0") + } + if rc1 != "thought-1" { + t.Fatalf("choices[1].reasoning_content = %q, want %q", rc1, "thought-1") + } +} + +func TestNormalizeReasoningField_NoChoices(t *testing.T) { + t.Parallel() + data := []byte(`{"id":"chatcmpl-123"}`) + got := normalizeGitHubCopilotReasoningField(data) + if string(got) != string(data) { + t.Fatalf("expected no change, got %s", string(got)) + } +} + func TestApplyHeaders_OpenAIIntentValue(t *testing.T) { t.Parallel() e := &GitHubCopilotExecutor{}