From c0d8568af9d73a2b6b2a6947c59ebaa93ae2b68d Mon Sep 17 00:00:00 2001 From: Guoguo Date: Wed, 29 Apr 2026 02:52:38 -0700 Subject: [PATCH] fix(media): preserve tool-message ordering for multi-tool-call scenarios MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move synthetic user message (carrying base64 tool images) to after the entire contiguous tool-message block instead of immediately after each tool message. This preserves the assistant→tool→tool ordering required by OpenAI-compatible APIs. Also fix load_image to use generic [image: photo] placeholder so injectPathTags can properly replace it with the actual path. Co-Authored-By: Claude Opus 4.6 --- pkg/agent/agent_media.go | 41 +++++++++++++++++++++++---------- pkg/agent/agent_test.go | 46 ++++++++++++++++++++++++++++++++++++++ pkg/tools/fs/load_image.go | 2 +- 3 files changed, 76 insertions(+), 13 deletions(-) diff --git a/pkg/agent/agent_media.go b/pkg/agent/agent_media.go index 1791a8510..3bbeca352 100644 --- a/pkg/agent/agent_media.go +++ b/pkg/agent/agent_media.go @@ -24,7 +24,8 @@ import ( // For user messages: images get path tags only ([image:/path]) so the LLM // can decide whether to view them via load_image or operate on the file. // For tool messages: images are base64-encoded and appended as a synthetic -// user message (many APIs don't support image_url in tool messages). +// user message after the contiguous tool-message block ends, preserving +// the required assistant→tool ordering for LLM APIs. // Non-image files always get path tags regardless of role. // Returns a new slice; original messages are not mutated. func resolveMediaRefs(messages []providers.Message, store media.MediaStore, maxSize int) []providers.Message { @@ -33,17 +34,36 @@ func resolveMediaRefs(messages []providers.Message, store media.MediaStore, maxS } result := make([]providers.Message, 0, len(messages)) + var pendingToolImages []string + + for idx, m := range messages { + // When leaving a tool-message block, flush any accumulated images + // as a synthetic user message. + if m.Role != "tool" && len(pendingToolImages) > 0 { + result = append(result, providers.Message{ + Role: "user", + Content: "[Loaded image from tool result above]", + Media: pendingToolImages, + }) + pendingToolImages = nil + } - for _, m := range messages { if len(m.Media) == 0 { result = append(result, m) + if idx == len(messages)-1 && len(pendingToolImages) > 0 { + result = append(result, providers.Message{ + Role: "user", + Content: "[Loaded image from tool result above]", + Media: pendingToolImages, + }) + pendingToolImages = nil + } continue } msg := m resolved := make([]string, 0, len(m.Media)) var pathTags []string - var toolImageDataURLs []string for _, ref := range m.Media { if !strings.HasPrefix(ref, "media://") { @@ -72,13 +92,10 @@ func resolveMediaRefs(messages []providers.Message, store media.MediaStore, maxS mime := detectMIME(localPath, meta) pathTags = append(pathTags, buildPathTag(mime, localPath)) - // For tool results (e.g. load_image), base64-encode images into a - // separate user message — many LLM APIs don't support image_url in - // tool messages. if m.Role == "tool" && strings.HasPrefix(mime, "image/") { dataURL := encodeImageToDataURL(localPath, mime, info, maxSize) if dataURL != "" { - toolImageDataURLs = append(toolImageDataURLs, dataURL) + pendingToolImages = append(pendingToolImages, dataURL) } } } @@ -89,14 +106,14 @@ func resolveMediaRefs(messages []providers.Message, store media.MediaStore, maxS } result = append(result, msg) - // Append a synthetic user message carrying the image data so the LLM - // can see it (tool messages don't support image_url in most APIs). - if len(toolImageDataURLs) > 0 { + // If this is the last message and we have pending images, flush them. + if idx == len(messages)-1 && len(pendingToolImages) > 0 { result = append(result, providers.Message{ - Role: "user", + Role: "user", Content: "[Loaded image from tool result above]", - Media: toolImageDataURLs, + Media: pendingToolImages, }) + pendingToolImages = nil } } diff --git a/pkg/agent/agent_test.go b/pkg/agent/agent_test.go index e77eb0fac..afc709508 100644 --- a/pkg/agent/agent_test.go +++ b/pkg/agent/agent_test.go @@ -4746,6 +4746,52 @@ func TestResolveMediaRefs_ToolRoleImageAppendedAsUserMessage(t *testing.T) { } } +func TestResolveMediaRefs_MultiToolCallPreservesOrdering(t *testing.T) { + store := media.NewFileMediaStore() + dir := t.TempDir() + + // Create image for tool #1 + pngPath := filepath.Join(dir, "loaded.png") + pngHeader := []byte{ + 0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, // PNG signature + 0x00, 0x00, 0x00, 0x0D, 0x49, 0x48, 0x44, 0x52, // IHDR + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x08, 0x02, + 0x00, 0x00, 0x00, 0x90, 0x77, 0x53, 0xDE, + } + os.WriteFile(pngPath, pngHeader, 0o644) + imgRef, _ := store.Store(pngPath, media.MediaMeta{}, "test") + + // Simulate: assistant called load_image + read_file, two tool results follow + messages := []providers.Message{ + {Role: "assistant", Content: "Let me load the image and read the file."}, + {Role: "tool", Content: "Image loaded [image: photo]", Media: []string{imgRef}}, + {Role: "tool", Content: "file contents here"}, + } + result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize) + + // assistant, tool#1, tool#2 must remain contiguous — no user in between + if result[0].Role != "assistant" { + t.Fatalf("result[0] expected assistant, got %q", result[0].Role) + } + if result[1].Role != "tool" { + t.Fatalf("result[1] expected tool, got %q", result[1].Role) + } + if result[2].Role != "tool" { + t.Fatalf("result[2] expected tool, got %q", result[2].Role) + } + + // Synthetic user message should come AFTER the tool block + if len(result) != 4 { + t.Fatalf("expected 4 messages (assistant + 2 tool + synthetic user), got %d", len(result)) + } + if result[3].Role != "user" { + t.Fatalf("result[3] expected user, got %q", result[3].Role) + } + if len(result[3].Media) != 1 || !strings.HasPrefix(result[3].Media[0], "data:image/png;base64,") { + t.Fatal("expected synthetic user message to contain base64 image") + } +} + func TestResolveMediaRefs_OversizedImageSkipsBase64KeepsPathTag(t *testing.T) { store := media.NewFileMediaStore() dir := t.TempDir() diff --git a/pkg/tools/fs/load_image.go b/pkg/tools/fs/load_image.go index 47022ed13..0a67fa120 100644 --- a/pkg/tools/fs/load_image.go +++ b/pkg/tools/fs/load_image.go @@ -150,7 +150,7 @@ func (t *LoadImageTool) Execute(ctx context.Context, args map[string]any) *ToolR // Build the tool result text. The media:// ref in Media will be picked // up by resolveMediaRefs in agent_media.go and base64-encoded for tool // result messages (role="tool"), so the LLM can see the image content. - msg := fmt.Sprintf("Image loaded: %s\n[image: %s]", filename, ref) + msg := fmt.Sprintf("Image loaded: %s\n[image: photo]", filename) return &ToolResult{ ForLLM: msg,