diff --git a/coderd/exp_chats.go b/coderd/exp_chats.go index 3433402ef9..acc815e54d 100644 --- a/coderd/exp_chats.go +++ b/coderd/exp_chats.go @@ -28,7 +28,6 @@ import ( "cdr.dev/slog/v3" "github.com/coder/coder/v2/agent/agentssh" "github.com/coder/coder/v2/coderd/audit" - "github.com/coder/coder/v2/coderd/chatfiles" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/database/dbauthz" @@ -50,6 +49,7 @@ import ( "github.com/coder/coder/v2/coderd/wsbuilder" "github.com/coder/coder/v2/coderd/x/chatd" "github.com/coder/coder/v2/coderd/x/chatd/chatprovider" + "github.com/coder/coder/v2/coderd/x/chatfiles" "github.com/coder/coder/v2/coderd/x/gitsync" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk/wsjson" diff --git a/coderd/x/chatd/attachments.go b/coderd/x/chatd/attachments.go index 363479af5d..a7a9885fab 100644 --- a/coderd/x/chatd/attachments.go +++ b/coderd/x/chatd/attachments.go @@ -23,7 +23,7 @@ func buildAssistantPartsForPersist( ) []codersdk.ChatMessagePart { parts := make([]codersdk.ChatMessagePart, 0, len(assistantBlocks)+len(toolResults)) for _, block := range assistantBlocks { - part := chatprompt.PartFromContent(block) + part := chatprompt.PartFromContentWithLogger(ctx, logger, block) if part.ToolName != "" { if configID, ok := toolNameToConfigID[part.ToolName]; ok { part.MCPServerConfigID = uuid.NullUUID{UUID: configID, Valid: true} diff --git a/coderd/x/chatd/chatd.go b/coderd/x/chatd/chatd.go index 9b6ff51d28..927c6f28a4 100644 --- a/coderd/x/chatd/chatd.go +++ b/coderd/x/chatd/chatd.go @@ -6015,7 +6015,7 @@ func (p *Server) runChat( // FOR UPDATE lock is held only for the INSERT statements. // Marshaling is pure CPU work with no database dependency. assistantParts := buildAssistantPartsForPersist( - ctx, + persistCtx, p.logger, assistantBlocks, toolResults, @@ -6035,7 +6035,7 @@ func (p *Server) runChat( toolResultContents := make([]pqtype.NullRawMessage, len(toolResults)) for i, tr := range toolResults { - trPart := chatprompt.PartFromContent(tr) + trPart := chatprompt.PartFromContentWithLogger(ctx, logger, tr) if trPart.ToolName != "" { if configID, ok := toolNameToConfigID[trPart.ToolName]; ok { trPart.MCPServerConfigID = uuid.NullUUID{UUID: configID, Valid: true} @@ -6496,6 +6496,7 @@ func (p *Server) runChat( } p.publishMessagePart(chat.ID, role, part) }, + Logger: logger, Compaction: compactionOptions, ReloadMessages: func(reloadCtx context.Context) ([]fantasy.Message, error) { reloadedMsgs, err := p.db.GetChatMessagesForPromptByChatID(reloadCtx, chat.ID) diff --git a/coderd/x/chatd/chatloop/chatloop.go b/coderd/x/chatd/chatloop/chatloop.go index b59c73197c..43ed4bbc0c 100644 --- a/coderd/x/chatd/chatloop/chatloop.go +++ b/coderd/x/chatd/chatloop/chatloop.go @@ -19,11 +19,13 @@ import ( "charm.land/fantasy/schema" "golang.org/x/xerrors" + "cdr.dev/slog/v3" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/x/chatd/chatdebug" "github.com/coder/coder/v2/coderd/x/chatd/chaterror" "github.com/coder/coder/v2/coderd/x/chatd/chatprompt" "github.com/coder/coder/v2/coderd/x/chatd/chatretry" + "github.com/coder/coder/v2/coderd/x/chatd/chattool" "github.com/coder/coder/v2/codersdk" "github.com/coder/quartz" ) @@ -146,6 +148,7 @@ type RunOptions struct { role codersdk.ChatMessageRole, part codersdk.ChatMessagePart, ) + Logger slog.Logger Compaction *CompactionOptions ReloadMessages func(context.Context) ([]fantasy.Message, error) DisableChainMode func() @@ -492,7 +495,8 @@ func Run(ctx context.Context, opts RunOptions) error { // Execute only built-in tools. toolResults = executeTools(ctx, opts.Tools, opts.ActiveTools, opts.ProviderTools, builtinCalls, opts.Metrics, provider, modelName, opts.BuiltinToolNames, func(tr fantasy.ToolResultContent, completedAt time.Time) { recordToolResultTimestamp(&result, tr.ToolCallID, completedAt) - ssePart := chatprompt.PartFromContent(tr) + publishToolAttachments(ctx, opts.Logger, tr, completedAt, publishMessagePart) + ssePart := chatprompt.PartFromContentWithLogger(ctx, opts.Logger, tr) ssePart.CreatedAt = &completedAt publishMessagePart(codersdk.ChatMessageRoleTool, ssePart) }) @@ -1545,6 +1549,33 @@ func recordToolResultTimestamp(result *stepResult, toolCallID string, ts time.Ti result.toolResultCreatedAt[toolCallID] = ts } +func publishToolAttachments( + ctx context.Context, + logger slog.Logger, + tr fantasy.ToolResultContent, + createdAt time.Time, + publishMessagePart func(codersdk.ChatMessageRole, codersdk.ChatMessagePart), +) { + attachments, err := chattool.AttachmentsFromMetadata(tr.ClientMetadata) + if err != nil { + logger.Warn(ctx, "skipping malformed tool attachment metadata", + slog.F("tool_name", tr.ToolName), + slog.F("tool_call_id", tr.ToolCallID), + slog.Error(err), + ) + return + } + for _, attachment := range attachments { + filePart := codersdk.ChatMessageFile( + attachment.FileID, + attachment.MediaType, + attachment.Name, + ) + filePart.CreatedAt = &createdAt + publishMessagePart(codersdk.ChatMessageRoleAssistant, filePart) + } +} + func extractContextLimit(metadata fantasy.ProviderMetadata) sql.NullInt64 { if len(metadata) == 0 { return sql.NullInt64{} diff --git a/coderd/x/chatd/chatprompt/chatprompt.go b/coderd/x/chatd/chatprompt/chatprompt.go index 43613c3693..452fd77144 100644 --- a/coderd/x/chatd/chatprompt/chatprompt.go +++ b/coderd/x/chatd/chatprompt/chatprompt.go @@ -16,6 +16,7 @@ import ( "cdr.dev/slog/v3" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/x/chatd/chattool" "github.com/coder/coder/v2/codersdk" ) @@ -702,6 +703,29 @@ func MarshalToolResult(toolCallID, toolName string, result json.RawMessage, isEr // PartFromContent converts fantasy content into a SDK chat message // part, preserving ProviderMetadata and ProviderExecuted fields. func PartFromContent(block fantasy.Content) codersdk.ChatMessagePart { + return sdkPartFromContent(block, nil) +} + +// PartFromContentWithLogger is for call sites that can surface malformed +// attachment metadata immediately instead of dropping it silently. +func PartFromContentWithLogger( + ctx context.Context, + logger slog.Logger, + block fantasy.Content, +) codersdk.ChatMessagePart { + return sdkPartFromContent(block, func(content fantasy.ToolResultContent, err error) { + logger.Warn(ctx, "skipping malformed tool attachment metadata", + slog.F("tool_name", content.ToolName), + slog.F("tool_call_id", content.ToolCallID), + slog.Error(err), + ) + }) +} + +func sdkPartFromContent( + block fantasy.Content, + logMalformedAttachmentMetadata func(fantasy.ToolResultContent, error), +) codersdk.ChatMessagePart { switch value := block.(type) { case fantasy.TextContent: return codersdk.ChatMessagePart{ @@ -776,9 +800,9 @@ func PartFromContent(block fantasy.Content) codersdk.ChatMessagePart { ProviderMetadata: marshalProviderMetadata(value.ProviderMetadata), } case fantasy.ToolResultContent: - return toolResultContentToPart(value) + return toolResultContentToPart(value, logMalformedAttachmentMetadata) case *fantasy.ToolResultContent: - return toolResultContentToPart(*value) + return toolResultContentToPart(*value, logMalformedAttachmentMetadata) default: return codersdk.ChatMessagePart{} } @@ -794,7 +818,10 @@ func ToolResultToPart(toolCallID, toolName string, result json.RawMessage, isErr // toolResultContentToPart converts a fantasy ToolResultContent into a // ChatMessagePart. -func toolResultContentToPart(content fantasy.ToolResultContent) codersdk.ChatMessagePart { +func toolResultContentToPart( + content fantasy.ToolResultContent, + logMalformedAttachmentMetadata func(fantasy.ToolResultContent, error), +) codersdk.ChatMessagePart { var result json.RawMessage var isError bool var isMedia bool @@ -820,11 +847,24 @@ func toolResultContentToPart(content fantasy.ToolResultContent) codersdk.ChatMes } case fantasy.ToolResultOutputContentMedia: isMedia = true - result, _ = json.Marshal(persistedMediaResult{ + persisted := persistedMediaResult{ Data: output.Data, MimeType: output.MediaType, Text: output.Text, - }) + } + // Tool renderers only receive the persisted result JSON, while + // ClientMetadata is consumed later to append sibling file parts. + // Mirror attachment identity here so promoted media can be + // recognized as the same durable attachment downstream. + if attachment, ok := matchingAttachmentForMedia( + content, + output.MediaType, + logMalformedAttachmentMetadata, + ); ok { + persisted.AttachmentFileID = attachment.FileID.String() + persisted.AttachmentName = attachment.Name + } + result, _ = json.Marshal(persisted) default: result = []byte(`{}`) } @@ -835,6 +875,26 @@ func toolResultContentToPart(content fantasy.ToolResultContent) codersdk.ChatMes return part } +func matchingAttachmentForMedia( + content fantasy.ToolResultContent, + mediaType string, + logMalformedAttachmentMetadata func(fantasy.ToolResultContent, error), +) (chattool.AttachmentMetadata, bool) { + attachments, err := chattool.AttachmentsFromMetadata(content.ClientMetadata) + if err != nil { + if logMalformedAttachmentMetadata != nil { + logMalformedAttachmentMetadata(content, err) + } + return chattool.AttachmentMetadata{}, false + } + for _, attachment := range attachments { + if attachment.MediaType == mediaType { + return attachment, true + } + } + return chattool.AttachmentMetadata{}, false +} + // Keep in sync with coderd/x/chatd/subagent.go. func isSubagentLifecycleToolName(name string) bool { switch name { @@ -1267,10 +1327,11 @@ func toolResultPartToMessagePart(logger slog.Logger, part codersdk.ChatMessagePa // IsError takes precedence and is handled above. // Detect media content flagged by toolResultContentToPart. // Screenshots from the computer use tool are stored as - // {"data":"","mime_type":"image/png","text":"..."}. - // Without this detection, the entire base64 payload is sent - // as text tokens, which quickly exceeds the context limit - // on follow-up messages. + // {"data":"","mime_type":"image/png","text":"..."} + // with optional attachment identity fields when the same image + // was also promoted into a durable file part. Without this + // detection, the entire base64 payload is sent as text tokens, + // which quickly exceeds the context limit on follow-up messages. if part.IsMedia { var media persistedMediaResult unmarshalErr := json.Unmarshal(part.Result, &media) @@ -1319,12 +1380,17 @@ func toolResultPartToMessagePart(logger slog.Logger, part codersdk.ChatMessagePa // cannot drift. // // The "mime_type" key intentionally diverges from the fantasy -// struct tag (json:"media_type"). Do not change it without -// updating both paths. +// struct tag (json:"media_type"). Optional attachment identity +// fields are UI hints only. They let the frontend recognize when the +// same media was also promoted into a durable file part, but the prompt +// reconstruction path must continue to ignore them. Keep additions +// backwards-compatible because existing rows may omit these fields. type persistedMediaResult struct { - Data string `json:"data"` - MimeType string `json:"mime_type"` - Text string `json:"text"` + Data string `json:"data"` + MimeType string `json:"mime_type"` + Text string `json:"text"` + AttachmentFileID string `json:"attachment_file_id,omitempty"` + AttachmentName string `json:"attachment_name,omitempty"` } type missingFilePolicy uint8 diff --git a/coderd/x/chatd/chatprompt/chatprompt_test.go b/coderd/x/chatd/chatprompt/chatprompt_test.go index 6750d489ed..06c707d7b8 100644 --- a/coderd/x/chatd/chatprompt/chatprompt_test.go +++ b/coderd/x/chatd/chatprompt/chatprompt_test.go @@ -21,6 +21,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/x/chatd/chatprompt" + "github.com/coder/coder/v2/coderd/x/chatd/chattool" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/testutil" ) @@ -2384,6 +2385,103 @@ func TestMediaToolResultRoundTrip(t *testing.T) { require.Equal(t, mimeType, mediaOutput.MediaType) }) + t.Run("MediaResultCarriesPromotedAttachmentMetadata", func(t *testing.T) { + t.Parallel() + + const callID = "call-screenshot-promoted" + const toolName = "computer" + const mimeType = "image/png" + const attachmentName = "screenshot-2026-04-21T00-00-00Z.png" + + attachmentID := uuid.MustParse("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee") + response := chattool.WithAttachments( + fantasy.NewImageResponse([]byte(imageData), mimeType), + chattool.AttachmentMetadata{ + FileID: attachmentID, + MediaType: mimeType, + Name: attachmentName, + }, + ) + + sdkPart := chatprompt.PartFromContent(fantasy.ToolResultContent{ + ToolCallID: callID, + ToolName: toolName, + ClientMetadata: response.Metadata, + Result: fantasy.ToolResultOutputContentMedia{ + Data: imageData, + MediaType: mimeType, + }, + }) + + var persisted struct { + Data string `json:"data"` + MimeType string `json:"mime_type"` + Text string `json:"text"` + AttachmentFileID string `json:"attachment_file_id"` + AttachmentName string `json:"attachment_name"` + } + require.NoError(t, json.Unmarshal(sdkPart.Result, &persisted)) + require.Equal(t, imageData, persisted.Data) + require.Equal(t, mimeType, persisted.MimeType) + require.Equal(t, attachmentID.String(), persisted.AttachmentFileID) + require.Equal(t, attachmentName, persisted.AttachmentName) + + chat := insertPair(t, callID, toolName, []codersdk.ChatMessagePart{sdkPart}) + + prompt := loadPrompt(t, chat) + require.Len(t, prompt, 2) + + resultPart, ok := fantasy.AsMessagePart[fantasy.ToolResultPart](prompt[1].Content[0]) + require.True(t, ok, "expected ToolResultPart") + + mediaOutput, ok := fantasy.AsToolResultOutputType[fantasy.ToolResultOutputContentMedia](resultPart.Output) + require.True(t, ok, "expected ToolResultOutputContentMedia, got %T", resultPart.Output) + require.Equal(t, imageData, mediaOutput.Data) + require.Equal(t, mimeType, mediaOutput.MediaType) + }) + t.Run("MediaResultUsesMatchingAttachmentMetadata", func(t *testing.T) { + t.Parallel() + + const callID = "call-screenshot-matching-attachment" + const toolName = "computer" + const mimeType = "image/png" + const attachmentName = "screenshot-2026-04-21T00-00-01Z.png" + + mismatchedAttachmentID := uuid.MustParse("11111111-2222-3333-4444-555555555555") + matchingAttachmentID := uuid.MustParse("aaaaaaaa-bbbb-cccc-dddd-ffffffffffff") + response := chattool.WithAttachments( + fantasy.NewImageResponse([]byte(imageData), mimeType), + chattool.AttachmentMetadata{ + FileID: mismatchedAttachmentID, + MediaType: "application/pdf", + Name: "report.pdf", + }, + chattool.AttachmentMetadata{ + FileID: matchingAttachmentID, + MediaType: mimeType, + Name: attachmentName, + }, + ) + + sdkPart := chatprompt.PartFromContent(fantasy.ToolResultContent{ + ToolCallID: callID, + ToolName: toolName, + ClientMetadata: response.Metadata, + Result: fantasy.ToolResultOutputContentMedia{ + Data: imageData, + MediaType: mimeType, + }, + }) + + var persisted struct { + AttachmentFileID string `json:"attachment_file_id"` + AttachmentName string `json:"attachment_name"` + } + require.NoError(t, json.Unmarshal(sdkPart.Result, &persisted)) + require.Equal(t, matchingAttachmentID.String(), persisted.AttachmentFileID) + require.Equal(t, attachmentName, persisted.AttachmentName) + }) + t.Run("MediaResultWithText", func(t *testing.T) { t.Parallel() diff --git a/coderd/x/chatd/recording.go b/coderd/x/chatd/recording.go index c9bf58922b..ea912df84d 100644 --- a/coderd/x/chatd/recording.go +++ b/coderd/x/chatd/recording.go @@ -11,10 +11,10 @@ import ( "github.com/google/uuid" "cdr.dev/slog/v3" - "github.com/coder/coder/v2/coderd/chatfiles" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/x/chatd/chattool" + "github.com/coder/coder/v2/coderd/x/chatfiles" "github.com/coder/coder/v2/codersdk/workspacesdk" ) diff --git a/coderd/x/chatd/store_chat_attachment.go b/coderd/x/chatd/store_chat_attachment.go index 7537a81f68..cb286639f7 100644 --- a/coderd/x/chatd/store_chat_attachment.go +++ b/coderd/x/chatd/store_chat_attachment.go @@ -6,9 +6,9 @@ import ( "github.com/google/uuid" "golang.org/x/xerrors" - "github.com/coder/coder/v2/coderd/chatfiles" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/x/chatd/chattool" + "github.com/coder/coder/v2/coderd/x/chatfiles" "github.com/coder/coder/v2/codersdk" ) diff --git a/coderd/x/chatd/store_chat_attachment_test.go b/coderd/x/chatd/store_chat_attachment_test.go index e8fef3a627..0bb4eb29bb 100644 --- a/coderd/x/chatd/store_chat_attachment_test.go +++ b/coderd/x/chatd/store_chat_attachment_test.go @@ -8,10 +8,10 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" - "github.com/coder/coder/v2/coderd/chatfiles" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbmock" "github.com/coder/coder/v2/coderd/x/chatd/chattool" + "github.com/coder/coder/v2/coderd/x/chatfiles" "github.com/coder/coder/v2/codersdk" ) diff --git a/coderd/chatfiles/mime.go b/coderd/x/chatfiles/mime.go similarity index 100% rename from coderd/chatfiles/mime.go rename to coderd/x/chatfiles/mime.go diff --git a/coderd/chatfiles/mime_test.go b/coderd/x/chatfiles/mime_test.go similarity index 99% rename from coderd/chatfiles/mime_test.go rename to coderd/x/chatfiles/mime_test.go index bff79e3e13..4e42a5da6d 100644 --- a/coderd/chatfiles/mime_test.go +++ b/coderd/x/chatfiles/mime_test.go @@ -6,7 +6,7 @@ import ( "github.com/stretchr/testify/require" - "github.com/coder/coder/v2/coderd/chatfiles" + "github.com/coder/coder/v2/coderd/x/chatfiles" ) func TestDetectMediaType_WebP(t *testing.T) { diff --git a/site/src/pages/AgentsPage/components/AttachmentPreview.stories.tsx b/site/src/pages/AgentsPage/components/AttachmentPreview.stories.tsx index 251d63fcf2..94a772ed8a 100644 --- a/site/src/pages/AgentsPage/components/AttachmentPreview.stories.tsx +++ b/site/src/pages/AgentsPage/components/AttachmentPreview.stories.tsx @@ -164,7 +164,7 @@ export const TextAttachment: Story = { play: async ({ args, canvasElement }) => { const canvas = within(canvasElement); const textCard = await canvas.findByRole("button", { - name: "View text attachment", + name: "View clipboard.txt", }); expect(textCard).toHaveTextContent(/This is the pasted text content\./i); await userEvent.click(textCard); @@ -207,7 +207,7 @@ export const ThreeTextAttachments: Story = { play: async ({ canvasElement }) => { const canvas = within(canvasElement); expect( - await canvas.findAllByRole("button", { name: "View text attachment" }), + await canvas.findAllByRole("button", { name: /View paste-[1-3]\.txt/ }), ).toHaveLength(3); expect( canvas.getByText( @@ -269,7 +269,7 @@ export const MixedImageAndText: Story = { await canvas.findByRole("img", { name: "photo.png" }), ).toBeInTheDocument(); expect( - canvas.getByRole("button", { name: "View text attachment" }), + canvas.getByRole("button", { name: "View clipboard.txt" }), ).toBeInTheDocument(); }, }; diff --git a/site/src/pages/AgentsPage/components/AttachmentPreview.tsx b/site/src/pages/AgentsPage/components/AttachmentPreview.tsx index 150f2d0dd9..fd56fbd69b 100644 --- a/site/src/pages/AgentsPage/components/AttachmentPreview.tsx +++ b/site/src/pages/AgentsPage/components/AttachmentPreview.tsx @@ -1,5 +1,6 @@ import { AlertTriangleIcon, ClipboardPasteIcon, XIcon } from "lucide-react"; import type { FC, ReactEventHandler } from "react"; +import { toast } from "sonner"; import { Spinner } from "#/components/Spinner/Spinner"; import { Tooltip, @@ -12,6 +13,7 @@ import { isAbortError } from "../utils/chatAttachments"; import { fetchTextAttachmentContent, formatTextAttachmentPreview, + getTextAttachmentErrorMessage, } from "../utils/fetchTextAttachment"; export type UploadState = { @@ -82,7 +84,10 @@ export const AttachmentPreview: FC<{ if (result.kind === "loaded") { return result.content; } - console.warn("Failed to load text attachment:", result); + const resultMessage = getTextAttachmentErrorMessage(result); + if (resultMessage !== null) { + toast.error(resultMessage); + } return undefined; } catch (err) { if (!textAttachmentRequest.clear(controller)) { @@ -91,7 +96,12 @@ export const AttachmentPreview: FC<{ if (isAbortError(err)) { return undefined; } - console.warn("Failed to load text attachment:", err); + const errorMessage = getTextAttachmentErrorMessage(err); + if (errorMessage === null) { + return undefined; + } + console.error("Failed to load text attachment:", err); + toast.error(errorMessage); return undefined; } }; @@ -125,7 +135,7 @@ export const AttachmentPreview: FC<{ ) : hasTextAttachment ? ( )} {uploadState?.status === "uploading" && ( @@ -191,7 +204,7 @@ export const AttachmentPreview: FC<{ className="absolute -right-2 -top-2 flex h-6 w-6 cursor-pointer items-center justify-center rounded-full border-0 bg-surface-primary text-content-secondary shadow-sm opacity-0 transition-opacity hover:bg-surface-secondary hover:text-content-primary group-hover:opacity-100 group-focus-within:opacity-100 focus:opacity-100" aria-label={`Remove ${file.name}`} > - +