fix(github-copilot): Properly handle streaming errors and filter images from non-user messages#6212
Conversation
albina-popova
force-pushed
the
fix/github-copilot-error-handling-5845
branch
from
99ee20a to
669f6e9
Compare
…es from non-user messages Fixes block#5845 - Goose randomly becomes unresponsive when using GitHub Copilot provider. This commit addresses two related issues: 1. Streaming error handling in GitHub Copilot provider: - Added proper HTTP status code checking before processing streaming responses - Added error response parsing to surface meaningful error messages - Added empty response detection 2. Image filtering for non-user messages: - Replace MessageContent::Image in non-user messages with placeholder text - Only detect and load embedded images from text paths in user messages - Prevents "image urls not allowed in non-user messages" errors Added tests: - test_format_messages_filters_images_in_non_user_messages - test_format_messages_assistant_text_with_image_path_no_image_loading 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: albina-popova <97168987+albina-popova@users.noreply.github.com>
albina-popova
force-pushed
the
fix/github-copilot-error-handling-5845
branch
from
669f6e9 to
70d964e
Compare
DOsinga
left a comment
DOsinga
left a comment
There was a problem hiding this comment.
sorry for the delay in responding. can you check on the error handling and whether we can use the generalized handler there?
| }) | ||
| } | ||
|
|
||
| #[allow(clippy::too_many_lines)] |
| "GitHub Copilot API returned status {}: {}", | ||
| status, error_msg | ||
| ))); | ||
| } |
There was a problem hiding this comment.
can we use handle_status_openai_compat here instead?
| content_array.push(json!({"type": "text", "text": text.text})); | ||
| content_array.push(convert_image(&image, image_format)); | ||
| // Only detect and load embedded images from text in user messages | ||
| // Many providers don't support images in assistant or system messages |
There was a problem hiding this comment.
I think this is the right thing to do, but not for this reason (only) - the whole extracting images from user messages is an outdated hack that should just never be randomly applied to assistant messages. can you drop the comment?
|
@albina-popova mind merging in main and taking a look at the comments and we'll get it merged in! |
|
thanks for doing this. in order to move this along, I've copied over the changes and applied comments to: https://github.com/block/goose/pull/6771/changes which I will get merged soon. closing this one! |
|
@DOsinga apologies for a delay and thank you! |
|
no worries - it just struck me that in days of agents if I have a trivial change request for your PR and you have a merge conflict, I can just tell goose what I want changed and do the merge etc |
3 participants
Summary
Fixes #5845 - Goose randomly becomes unresponsive when using GitHub Copilot provider.
This PR addresses two related issues:
Streaming error handling in GitHub Copilot provider: Added proper HTTP status code checking before processing streaming responses, error response parsing, and empty response detection. Previously, errors during streaming were silently ignored, causing Goose to appear unresponsive.
Image filtering for non-user messages: Many providers (including GitHub Copilot) don't support images in assistant or system messages. Added filtering to:
MessageContent::Imagein non-user messages with placeholder textChanges
crates/goose/src/providers/githubcopilot.rs- Added streaming error handlingcrates/goose/src/providers/formats/openai.rs- Added image filtering for non-user messagesTest plan
test_format_messages_filters_images_in_non_user_messagestest_format_messages_assistant_text_with_image_path_no_image_loading🤖 Generated with Claude Code