diff --git a/pkg/agent/agent_media.go b/pkg/agent/agent_media.go index 84f386f2d..1791a8510 100644 --- a/pkg/agent/agent_media.go +++ b/pkg/agent/agent_media.go @@ -21,25 +21,29 @@ import ( ) // resolveMediaRefs resolves media:// refs in messages. -// Images are base64-encoded into the Media array for multimodal LLMs. -// Non-image files (documents, audio, video) have their local path injected -// into Content so the agent can access them via file tools like read_file. +// 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). +// 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 { if store == nil { return messages } - result := make([]providers.Message, len(messages)) - copy(result, messages) + result := make([]providers.Message, 0, len(messages)) - for i, m := range result { + for _, m := range messages { if len(m.Media) == 0 { + result = append(result, m) 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://") { @@ -68,24 +72,78 @@ func resolveMediaRefs(messages []providers.Message, store media.MediaStore, maxS mime := detectMIME(localPath, meta) pathTags = append(pathTags, buildPathTag(mime, localPath)) - if strings.HasPrefix(mime, "image/") { + // 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 != "" { - resolved = append(resolved, dataURL) + toolImageDataURLs = append(toolImageDataURLs, dataURL) } - continue } } - result[i].Media = resolved + msg.Media = resolved if len(pathTags) > 0 { - result[i].Content = injectPathTags(result[i].Content, pathTags) + msg.Content = injectPathTags(msg.Content, pathTags) + } + 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 { + result = append(result, providers.Message{ + Role: "user", + Content: "[Loaded image from tool result above]", + Media: toolImageDataURLs, + }) } } return result } +// encodeImageToDataURL base64-encodes an image file into a data URL. +// Returns empty string if the file exceeds maxSize or encoding fails. +func encodeImageToDataURL(localPath, mime string, info os.FileInfo, maxSize int) string { + if info.Size() > int64(maxSize) { + logger.WarnCF("agent", "Media file too large, skipping", map[string]any{ + "path": localPath, + "size": info.Size(), + "max_size": maxSize, + }) + return "" + } + + f, err := os.Open(localPath) + if err != nil { + logger.WarnCF("agent", "Failed to open media file", map[string]any{ + "path": localPath, + "error": err.Error(), + }) + return "" + } + defer f.Close() + + prefix := "data:" + mime + ";base64," + encodedLen := base64.StdEncoding.EncodedLen(int(info.Size())) + var buf bytes.Buffer + buf.Grow(len(prefix) + encodedLen) + buf.WriteString(prefix) + + encoder := base64.NewEncoder(base64.StdEncoding, &buf) + if _, err := io.Copy(encoder, f); err != nil { + logger.WarnCF("agent", "Failed to encode media file", map[string]any{ + "path": localPath, + "error": err.Error(), + }) + return "" + } + encoder.Close() + + return buf.String() +} + func buildArtifactTags(store media.MediaStore, refs []string) []string { if store == nil || len(refs) == 0 { return nil @@ -136,49 +194,8 @@ func detectMIME(localPath string, meta media.MediaMeta) string { return kind.MIME.Value } -// encodeImageToDataURL base64-encodes an image file into a data URL. -// Returns empty string if the file exceeds maxSize or encoding fails. -func encodeImageToDataURL(localPath, mime string, info os.FileInfo, maxSize int) string { - if info.Size() > int64(maxSize) { - logger.WarnCF("agent", "Media file too large, skipping", map[string]any{ - "path": localPath, - "size": info.Size(), - "max_size": maxSize, - }) - return "" - } - - f, err := os.Open(localPath) - if err != nil { - logger.WarnCF("agent", "Failed to open media file", map[string]any{ - "path": localPath, - "error": err.Error(), - }) - return "" - } - defer f.Close() - - prefix := "data:" + mime + ";base64," - encodedLen := base64.StdEncoding.EncodedLen(int(info.Size())) - var buf bytes.Buffer - buf.Grow(len(prefix) + encodedLen) - buf.WriteString(prefix) - - encoder := base64.NewEncoder(base64.StdEncoding, &buf) - if _, err := io.Copy(encoder, f); err != nil { - logger.WarnCF("agent", "Failed to encode media file", map[string]any{ - "path": localPath, - "error": err.Error(), - }) - return "" - } - encoder.Close() - - return buf.String() -} - // buildPathTag creates a structured tag exposing the local file path. -// Tag type is derived from MIME: [audio:/path], [video:/path], or [file:/path]. +// Tag type is derived from MIME: [image:/path], [audio:/path], [video:/path], or [file:/path]. func buildPathTag(mime, localPath string) string { switch { case strings.HasPrefix(mime, "image/"): diff --git a/pkg/agent/agent_test.go b/pkg/agent/agent_test.go index 14136aa38..e77eb0fac 100644 --- a/pkg/agent/agent_test.go +++ b/pkg/agent/agent_test.go @@ -4661,7 +4661,7 @@ func TestRun_PicoToolFeedbackSuppressesDuplicateInterimAssistantContent(t *testi } } -func TestResolveMediaRefs_ImageBase64AndPathTag(t *testing.T) { +func TestResolveMediaRefs_ImageInjectsPathTag(t *testing.T) { store := media.NewFileMediaStore() dir := t.TempDir() @@ -4689,11 +4689,8 @@ func TestResolveMediaRefs_ImageBase64AndPathTag(t *testing.T) { } result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize) - if len(result[0].Media) != 1 { - t.Fatalf("expected 1 resolved media, got %d", len(result[0].Media)) - } - if !strings.HasPrefix(result[0].Media[0], "data:image/png;base64,") { - t.Fatalf("expected data:image/png;base64, prefix, got %q", result[0].Media[0][:40]) + if len(result[0].Media) != 0 { + t.Fatalf("expected 0 media (images use path tags), got %d", len(result[0].Media)) } localPath, _, _ := store.ResolveWithMeta(ref) expectedContent := "describe this [image:" + localPath + "]" @@ -4702,6 +4699,53 @@ func TestResolveMediaRefs_ImageBase64AndPathTag(t *testing.T) { } } +func TestResolveMediaRefs_ToolRoleImageAppendedAsUserMessage(t *testing.T) { + store := media.NewFileMediaStore() + dir := t.TempDir() + + pngPath := filepath.Join(dir, "tool-result.png") + pngHeader := []byte{ + 0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, // PNG signature + 0x00, 0x00, 0x00, 0x0D, // IHDR length + 0x49, 0x48, 0x44, 0x52, // "IHDR" + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x08, 0x02, // 1x1 RGB + 0x00, 0x00, 0x00, // no interlace + 0x90, 0x77, 0x53, 0xDE, // CRC + } + if err := os.WriteFile(pngPath, pngHeader, 0o644); err != nil { + t.Fatal(err) + } + ref, _ := store.Store(pngPath, media.MediaMeta{}, "test") + + messages := []providers.Message{ + {Role: "tool", Content: "Image loaded", Media: []string{ref}}, + } + result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize) + + // Tool message should have path tag but no base64 + if len(result[0].Media) != 0 { + t.Fatalf("expected 0 media in tool message, got %d", len(result[0].Media)) + } + localPath, _, _ := store.ResolveWithMeta(ref) + if !strings.Contains(result[0].Content, "[image:"+localPath+"]") { + t.Fatalf("expected image path tag in tool content, got %q", result[0].Content) + } + + // A synthetic user message with base64 should follow + if len(result) != 2 { + t.Fatalf("expected 2 messages (tool + synthetic user), got %d", len(result)) + } + if result[1].Role != "user" { + t.Fatalf("expected synthetic message role=user, got %q", result[1].Role) + } + if len(result[1].Media) != 1 { + t.Fatalf("expected 1 base64 media in synthetic user message, got %d", len(result[1].Media)) + } + if !strings.HasPrefix(result[1].Media[0], "data:image/png;base64,") { + t.Fatalf("expected data:image/png;base64, prefix, got %q", result[1].Media[0][:40]) + } +} + func TestResolveMediaRefs_OversizedImageSkipsBase64KeepsPathTag(t *testing.T) { store := media.NewFileMediaStore() dir := t.TempDir() @@ -4806,11 +4850,8 @@ func TestResolveMediaRefs_UsesMetaContentType(t *testing.T) { } result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize) - if len(result[0].Media) != 1 { - t.Fatalf("expected 1 media, got %d", len(result[0].Media)) - } - if !strings.HasPrefix(result[0].Media[0], "data:image/jpeg;base64,") { - t.Fatalf("expected jpeg prefix, got %q", result[0].Media[0][:30]) + if len(result[0].Media) != 0 { + t.Fatalf("expected 0 media (images use path tags), got %d", len(result[0].Media)) } localPath, _, _ := store.ResolveWithMeta(ref) expectedContent := "hi [image:" + localPath + "]" @@ -4948,11 +4989,8 @@ func TestResolveMediaRefs_MixedImageAndFile(t *testing.T) { } result := resolveMediaRefs(messages, store, config.DefaultMaxMediaSize) - if len(result[0].Media) != 1 { - t.Fatalf("expected 1 media (image base64 only), got %d", len(result[0].Media)) - } - if !strings.HasPrefix(result[0].Media[0], "data:image/png;base64,") { - t.Fatal("expected image to be base64 encoded") + if len(result[0].Media) != 0 { + t.Fatalf("expected 0 media (all types use path tags), got %d", len(result[0].Media)) } imgLocalPath, _, _ := store.ResolveWithMeta(imgRef) pdfLocalPath, _, _ := store.ResolveWithMeta(fileRef) diff --git a/pkg/agent/steering_test.go b/pkg/agent/steering_test.go index 9330ec238..1ff699976 100644 --- a/pkg/agent/steering_test.go +++ b/pkg/agent/steering_test.go @@ -1051,18 +1051,16 @@ func TestAgentLoop_Continue_PreservesSteeringMedia(t *testing.T) { foundResolvedMedia := false for _, msg := range msgs { - if msg.Role != "user" { + if msg.Role != "user" || !strings.Contains(msg.Content, "describe this image") { continue } - hasBase64 := len(msg.Media) > 0 && strings.HasPrefix(msg.Media[0], "data:image/png;base64,") - hasPathTag := strings.Contains(msg.Content, "[image:") - if hasBase64 && hasPathTag { + if strings.Contains(msg.Content, "[image:") { foundResolvedMedia = true break } } if !foundResolvedMedia { - t.Fatal("expected continue path to inject both base64 media and image path tag") + t.Fatal("expected continue path to inject image path tag into the provider request") } defaultAgent := al.registry.GetDefaultAgent() diff --git a/pkg/tools/fs/load_image.go b/pkg/tools/fs/load_image.go index 6f612faea..47022ed13 100644 --- a/pkg/tools/fs/load_image.go +++ b/pkg/tools/fs/load_image.go @@ -147,9 +147,9 @@ func (t *LoadImageTool) Execute(ctx context.Context, args map[string]any) *ToolR return ErrorResult(fmt.Sprintf("failed to register image in media store: %v", err)) } - // Build the tool result text. The media:// ref will be picked up by - // resolveMediaRefs in loop_media.go and converted to a base64 data URL - // before the next LLM call, exactly like channel-received images. + // 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) return &ToolResult{