From 3d301c7115df32865ae9e6f3bc4a3b3686c6a6db Mon Sep 17 00:00:00 2001 From: LEFBE Date: Wed, 2 Sep 2026 09:17:21 +0200 Subject: [PATCH] server: accept inline images on tool/function role messages (#933) parse_messages rejected any message carrying an inline image whose role wasn't exactly "user", with a generic "invalid JSON request" 400. That breaks the normal OpenAI Chat Completions shape for coding agents that attach a tool-result screenshot as an image_url content part on a role=tool (or role=function) message -- the same data URI on role=user was accepted. As diagnosed in #933: the user-only restriction was meant to keep image tokens landing on a user-shaped turn, but DS4's own chat template already renders tool results inside a user turn (<|User|>...), so the role tag at the API layer doesn't need to match that internal shape. Allow user/tool/function; keep rejecting assistant/system roles, file paths, and https URLs exactly as before. Added test_openai_tool_role_image_content (mirrors the existing test_openai_inline_image_content / test_http_image_paths_and_urls_are_rejected pair): asserts user/tool/function all parse and capture the image, and that assistant/system still fail. Verified as a real regression test, not just a happy-path check: reverted the fix locally, confirmed the new test fails at the exact assertions added, then restored it and confirmed green. Verified end-to-end, not just at the parser: built ds4-server with the fix, sent the exact three-message shape from the issue (user text, assistant tool_calls, tool message with a real screenshot as image_url) over HTTP -- got 200, and the model's answer correctly described the screenshot's actual content. Confirmed the two negative cases still return 400: an image on role=assistant, and an https URL on role=tool. Full make test suite green throughout. Not included: the issue's "prefer a specific error string" suggestion is explicitly marked optional there and left out to keep this change to the one behavioral fix it's meant to be. --- ds4_server.c | 44 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/ds4_server.c b/ds4_server.c index 50efbe6bc..3f75ce5ef 100644 --- a/ds4_server.c +++ b/ds4_server.c @@ -2028,7 +2028,8 @@ static bool parse_messages(const char **p, chat_msgs *msgs) { (*p)++; if (!msg.role) msg.role = xstrdup("user"); if (!msg.content) msg.content = xstrdup(""); - if (msg.images.len && strcmp(msg.role, "user")) goto fail; + if (msg.images.len && strcmp(msg.role, "user") && + strcmp(msg.role, "tool") && strcmp(msg.role, "function")) goto fail; chat_msgs_push(msgs, msg); memset(&msg, 0, sizeof(msg)); json_ws(p); @@ -19351,6 +19352,46 @@ static void test_openai_inline_image_content(void) { buf_free(&json); } +static void test_openai_tool_role_image_content(void) { + /* #933: a tool-result message carrying an inline image (the normal shape + * for coding agents attaching a screenshot as a tool result) must parse, + * matching the OpenAI wire format DeepSeek's own template already wraps + * in a user turn. Assistant/system images stay rejected. */ + const char *accepted_roles[] = {"tool", "function", "user"}; + for (size_t i = 0; i < sizeof(accepted_roles) / sizeof(accepted_roles[0]); i++) { + buf json = {0}; + buf_puts(&json, "[{\"role\":\""); + buf_puts(&json, accepted_roles[i]); + buf_puts(&json, "\",\"content\":[{\"type\":\"text\",\"text\":\"result: \"}," + "{\"type\":\"image_url\",\"image_url\":{\"url\":\"data:image/png;base64,"); + buf_puts(&json, test_inline_png_base64); + buf_puts(&json, "\"}}]}]"); + const char *p = json.ptr; + chat_msgs msgs = {0}; + TEST_ASSERT(parse_messages(&p, &msgs)); + TEST_ASSERT(msgs.len == 1); + TEST_ASSERT(msgs.v[0].images.len == 1); + chat_msgs_free(&msgs); + buf_free(&json); + } + + const char *rejected_roles[] = {"assistant", "system"}; + for (size_t i = 0; i < sizeof(rejected_roles) / sizeof(rejected_roles[0]); i++) { + buf json = {0}; + buf_puts(&json, "[{\"role\":\""); + buf_puts(&json, rejected_roles[i]); + buf_puts(&json, "\",\"content\":[{\"type\":\"image_url\"," + "\"image_url\":{\"url\":\"data:image/png;base64,"); + buf_puts(&json, test_inline_png_base64); + buf_puts(&json, "\"}}]}]"); + const char *p = json.ptr; + chat_msgs msgs = {0}; + TEST_ASSERT(!parse_messages(&p, &msgs)); + chat_msgs_free(&msgs); + buf_free(&json); + } +} + static void test_http_image_paths_and_urls_are_rejected(void) { const char *cases[] = { "[{\"role\":\"user\",\"content\":[{\"type\":\"image_url\"," @@ -19488,6 +19529,7 @@ static void ds4_server_unit_tests_run(void) { test_thinking_canonical_with_tools_preserves_reasoning(); test_thinking_canonical_non_thinking_mode_noop(); test_openai_inline_image_content(); + test_openai_tool_role_image_content(); test_http_image_paths_and_urls_are_rejected(); test_anthropic_inline_image_content(); test_responses_inline_image_content();