Skip to content

fix(llm-drivers): Copilot streaming empty tool calls + Claude assistant strip#3084

Merged
houko merged 5 commits into
mainfrom
fix/copilot-msg-handling
Apr 25, 2026
Merged

fix(llm-drivers): Copilot streaming empty tool calls + Claude assistant strip#3084
houko merged 5 commits into
mainfrom
fix/copilot-msg-handling

Conversation

@houko

@houko houko commented Apr 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Two fixes to the OpenAI driver (used by github-copilot proxy):

  1. Skip streaming tool-calls with empty id or name — Copilot streaming occasionally delivers a function-call delta with an empty id or name, which then poisons the next API request. We log a warn and drop those entries.
  2. Model-aware trailing-assistant strip — Claude (via Copilot proxy) does not support assistant prefill; if the message history ends with an assistant turn that has no tool_calls, we strip it. For non-Claude models we only strip empty content (preserving the previous behaviour).

The strip is wired through the single build_request entry-point in LibreFang (one call site instead of upstream's two).

Ported from openfang abeaaf5 and 64b1ec5.

Test plan

  • 5 new unit tests in drivers/openai.rs covering claude/non-claude × empty/non-empty/with-tool_calls
  • grep'd LibreFang main — neither fix exists upstream; no overlap
  • Manual: run a Telegram/Matrix channel agent against provider = "github-copilot" + a Claude model, send a message that triggers a tool call, verify the conversation continues without the upstream "assistant prefill not supported" error

houko added 2 commits April 25, 2026 14:22
…g response

The Copilot API proxy can sometimes deliver streaming tool call chunks
without a function name, resulting in empty-name tool calls stored in
conversation history. When replayed to the API, these cause
"tool call must have a tool call ID and function name" errors.

Skip malformed tool calls (empty ID or name) during streaming response
finalization and log a warning.

Ported from openfang abeaaf5.
…via Copilot

The Copilot proxy for Claude enforces Anthropic's rule that conversations
must end with a user message. For Claude models, strip any trailing
assistant message without tool_calls (including non-empty ones). For
other models, only strip truly empty assistant messages.

This fixes the 'assistant message prefill not supported' error seen in
Telegram and other channel adapters when using Claude via Copilot,
without causing infinite agent loops for other models.

Tool-call-bearing assistant messages are always kept (protocol-essential).

Includes unit tests covering both Claude and non-Claude code paths.

Ported from openfang 64b1ec5.
@github-actions github-actions Bot added the ready-for-review PR is ready for maintainer review label Apr 25, 2026
@github-actions github-actions Bot added the size/M 50-249 lines changed label Apr 25, 2026

@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: 0bfa2eebea

ℹ️ 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".

/// are part of the protocol and must stay). Checks the model name to only
/// apply for Claude models which enforce this restriction.
fn strip_trailing_empty_assistant(messages: &mut Vec<OaiMessage>, model: &str) {
let is_claude = model.contains("claude");

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 Match Claude model names case-insensitively

strip_trailing_empty_assistant decides Claude behavior with model.contains("claude"), which is case-sensitive. Elsewhere we normalize by lowercasing only for matching but preserve the original model casing, so values like anthropic/Claude-3-5-sonnet can reach this path and skip the Claude-specific strip logic. In those cases the original Copilot prefill error can still occur depending on how the model name was cased in config, making behavior inconsistent for the same model.

Useful? React with 👍 / 👎.

houko and others added 3 commits April 25, 2026 15:09
…are filtered

When the Copilot proxy drops function-name chunks, every entry in
tool_accum can be skipped by the empty-id/name guard. The upstream
finish_reason is still 'tool_calls', which previously left
stop_reason=ToolUse with an empty tool_calls vector — staging a
tool-use turn the agent loop cannot execute. Downgrade to EndTurn
in that case so the turn ends cleanly.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@houko
houko merged commit c16b428 into main Apr 25, 2026
20 checks passed
@houko
houko deleted the fix/copilot-msg-handling branch April 25, 2026 08:03
@github-actions github-actions Bot removed the ready-for-review PR is ready for maintainer review label Apr 25, 2026
@houko houko mentioned this pull request Apr 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M 50-249 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant