feat(coderd/x/chatd): agent-created file attachments in chat (#24280)

Agents can already see workspace files and take screenshots, but users could not download those artifacts from chat. This PR adds durable chat attachments to chatd. `attach_file`, explicit `computer` screenshot actions (not the automatic post-action screenshots), and `propose_plan` now fetch bytes over the agent connection, store them in `chat_files`, link them to the chat, and carry attachment metadata in tool responses so `buildAssistantPartsForPersist` can materialize ordinary `type:"file"` assistant parts that the chat file APIs serve.

The same storage helpers are reused for other artifact-producing paths. `wait_agent` recordings and thumbnails are stored as chat files and linked back to the parent chat, with best-effort relinking so parent chats retain those artifacts without leaving orphaned rows when chat-file caps reject links. `storeChatAttachment` wraps insert + link in one transaction, files are capped at 10 MB each and 20 per chat, and serving defaults to `Content-Disposition: attachment` with an explicit inline-safe allowlist.

This PR also consolidates chat-file media policy in `coderd/chatfiles`. Uploads and tool-generated attachments share byte-based MIME detection, SVG blocking, inline-safety rules, and compatible `text/plain` refinement for JSON, CSV, and Markdown. Prompt construction still only inlines synthetic pasted text for model consumption; assistant-created attachments are persisted for the user and intentionally not replayed into later LLM turns.

UI follow-up lives in #24281.

Relates to CODAGT-91
This commit is contained in:
Ethan
2026-04-20 18:04:35 +10:00
committed by GitHub
parent 596e55b136
commit ef6969dd70
26 changed files with 3081 additions and 857 deletions
+250
View File
@@ -0,0 +1,250 @@
package chatfiles
import (
"bytes"
"encoding/json"
"encoding/xml"
"maps"
"mime"
"path/filepath"
"slices"
"strings"
"unicode"
"github.com/gabriel-vasile/mimetype"
"golang.org/x/xerrors"
)
const MaxStoredFileNameBytes = 255
var (
// ErrStoredFileNameRequired indicates that a durable file name is empty
// after normalization.
ErrStoredFileNameRequired = xerrors.New("stored file name is required")
// ErrUnsupportedStoredFileType indicates that classified file bytes do not
// map to an allowed durable file type.
ErrUnsupportedStoredFileType = xerrors.New("unsupported attachment type")
utf8BOM = []byte{0xEF, 0xBB, 0xBF}
allowedStoredMediaTypes = map[string]struct{}{
"image/png": {},
"image/jpeg": {},
"image/gif": {},
"image/webp": {},
"text/plain": {},
"text/markdown": {},
"text/csv": {},
"application/json": {},
"application/pdf": {},
}
recordingArtifactMediaTypes = map[string]struct{}{
"video/mp4": {},
"image/jpeg": {},
}
)
// DetectMediaType detects the base media type of the given file contents.
func DetectMediaType(data []byte) string {
return BaseMediaType(mimetype.Detect(data).String())
}
// BaseMediaType strips parameters from a media type.
func BaseMediaType(mediaType string) string {
if parsed, _, err := mime.ParseMediaType(mediaType); err == nil {
return parsed
}
return mediaType
}
// AllowedStoredMediaTypesString returns the supported durable chat file media
// types as a comma-separated list.
func AllowedStoredMediaTypesString() string {
return strings.Join(slices.Sorted(maps.Keys(allowedStoredMediaTypes)), ", ")
}
// IsAllowedStoredMediaType reports whether the media type is supported for
// durable chat file storage.
func IsAllowedStoredMediaType(mediaType string) bool {
_, ok := allowedStoredMediaTypes[BaseMediaType(mediaType)]
return ok
}
// IsInlineRenderableStoredMediaType reports whether a stored chat file may be
// served with Content-Disposition: inline. PDFs remain storable but
// download-only because browser PDF viewers have a broader active-content
// attack surface than the other media types we allow inline.
func IsInlineRenderableStoredMediaType(mediaType string) bool {
mediaType = BaseMediaType(mediaType)
if !IsAllowedStoredMediaType(mediaType) {
return false
}
return mediaType != "application/pdf"
}
// NormalizeStoredFileName trims surrounding whitespace, strips control
// characters, and truncates the name to the durable storage byte limit
// without splitting UTF-8 runes.
func NormalizeStoredFileName(name string) string {
name = strings.Map(func(r rune) rune {
if unicode.IsControl(r) {
return -1
}
return r
}, name)
name = strings.TrimSpace(name)
return truncateUTF8Bytes(name, MaxStoredFileNameBytes)
}
// PrepareStoredFile normalizes the display name, rejects empty normalized
// names, and classifies the file bytes using detectName when provided, so
// callers can preserve subtype detection even when the user-facing filename is
// overridden.
func PrepareStoredFile(name, detectName string, data []byte) (storedName, mediaType string, err error) {
storedName = NormalizeStoredFileName(name)
if storedName == "" {
return "", "", ErrStoredFileNameRequired
}
if strings.TrimSpace(detectName) == "" {
detectName = storedName
}
mediaType = ClassifyStoredMediaType(detectName, data)
if !IsAllowedStoredMediaType(mediaType) {
return "", "", xerrors.Errorf("%w %q", ErrUnsupportedStoredFileType, mediaType)
}
return storedName, mediaType, nil
}
// PrepareRecordingArtifact normalizes the recording artifact name, rejects
// empty normalized names, and verifies that the bytes match the expected
// recording media type.
func PrepareRecordingArtifact(name, expectedMediaType string, data []byte) (storedName, mediaType string, err error) {
expectedMediaType = BaseMediaType(expectedMediaType)
if _, ok := recordingArtifactMediaTypes[expectedMediaType]; !ok {
return "", "", xerrors.Errorf("unsupported recording artifact type %q", expectedMediaType)
}
storedName = NormalizeStoredFileName(name)
if storedName == "" {
return "", "", ErrStoredFileNameRequired
}
mediaType = DetectMediaType(data)
if mediaType != expectedMediaType {
return "", "", xerrors.Errorf("recording artifact type mismatch: expected %q, detected %q", expectedMediaType, mediaType)
}
return storedName, mediaType, nil
}
// IsCompatibleUploadMediaType reports whether an upload request that declared
// declaredMediaType may be stored as storedMediaType after byte
// classification. Exact matches are always compatible; the compatibility
// table only covers explicit refinements like text/plain uploads that safely
// store as richer text subtypes.
func IsCompatibleUploadMediaType(declaredMediaType, storedMediaType string) bool {
declaredMediaType = BaseMediaType(declaredMediaType)
storedMediaType = BaseMediaType(storedMediaType)
if declaredMediaType == storedMediaType {
return true
}
if declaredMediaType != "text/plain" {
return false
}
switch storedMediaType {
case "text/markdown", "text/csv", "application/json":
return true
default:
return false
}
}
// HasSVGRootElement reports whether the provided file bytes decode to an SVG
// root element. This catches SVG content even when generic sniffers classify it
// as text or XML.
func HasSVGRootElement(data []byte) bool {
data = bytes.TrimPrefix(data, utf8BOM)
if len(data) == 0 {
return false
}
decoder := xml.NewDecoder(bytes.NewReader(data))
for {
token, err := decoder.Token()
if err != nil {
return false
}
switch token := token.(type) {
case xml.ProcInst, xml.Directive, xml.Comment:
continue
case xml.CharData:
if len(bytes.TrimSpace(token)) == 0 {
continue
}
return false
case xml.StartElement:
return strings.EqualFold(token.Name.Local, "svg")
default:
return false
}
}
}
// ClassifyStoredMediaType returns the media type that durable chat storage
// would use for the given filename and bytes. Unsupported or blocked content is
// returned as its detected media type so callers can report the specific type.
func ClassifyStoredMediaType(name string, data []byte) string {
if HasSVGRootElement(data) {
return "image/svg+xml"
}
mediaType := DetectMediaType(data)
switch mediaType {
case "image/png", "image/jpeg", "image/gif", "image/webp",
"text/markdown", "text/csv", "application/json",
"application/pdf", "application/xml", "text/xml":
return mediaType
case "text/plain":
return refineTextMediaType(name, data)
default:
if strings.HasPrefix(mediaType, "text/") {
return "text/plain"
}
return mediaType
}
}
func refineTextMediaType(name string, data []byte) string {
switch strings.ToLower(filepath.Ext(name)) {
case ".json":
if json.Valid(data) {
return "application/json"
}
case ".md", ".markdown":
return "text/markdown"
case ".csv":
return "text/csv"
}
return "text/plain"
}
func truncateUTF8Bytes(value string, maxBytes int) string {
if maxBytes <= 0 || value == "" {
return ""
}
if len(value) <= maxBytes {
return value
}
cut := 0
for idx := range value {
if idx > maxBytes {
break
}
cut = idx
}
return value[:cut]
}
+345
View File
@@ -0,0 +1,345 @@
package chatfiles_test
import (
"strings"
"testing"
"github.com/stretchr/testify/require"
"github.com/coder/coder/v2/coderd/chatfiles"
)
func TestDetectMediaType_WebP(t *testing.T) {
t.Parallel()
data := append([]byte("RIFF"), []byte{0x24, 0x00, 0x00, 0x00}...)
data = append(data, []byte("WEBPVP8 ")...)
require.Equal(t, "image/webp", chatfiles.DetectMediaType(data))
}
func TestClassifyStoredMediaType(t *testing.T) {
t.Parallel()
tests := []struct {
name string
fileName string
data []byte
want string
}{
{
name: "PlainText",
fileName: "build.log",
data: []byte("build succeeded\n"),
want: "text/plain",
},
{
name: "MarkdownFromExtension",
fileName: "notes.md",
data: []byte("# Release notes\n"),
want: "text/markdown",
},
{
name: "CSVFromDetector",
fileName: "report.txt",
data: []byte("name,count\nwidgets,3\n"),
want: "text/csv",
},
{
name: "JSONFromDetector",
fileName: "payload.txt",
data: []byte(`{"ok":true}`),
want: "application/json",
},
{
name: "UppercaseJSONExtension",
fileName: "data.JSON",
data: []byte(`{"ok":true}`),
want: "application/json",
},
{
name: "InvalidJSONExtensionFallsBackToPlainText",
fileName: "broken.json",
data: []byte("not json"),
want: "text/plain",
},
{
name: "UppercaseMDExtension",
fileName: "NOTES.MD",
data: []byte("# Notes\n"),
want: "text/markdown",
},
{
name: "PDF",
fileName: "report.pdf",
data: []byte("%PDF-1.7\n"),
want: "application/pdf",
},
{
name: "BinaryOctetStream",
fileName: "data.bin",
data: []byte{0x00, 0x01, 0x02, 0x03, 0x04, 0x05},
want: "application/octet-stream",
},
{
name: "HTMLFallsBackToTextPlain",
fileName: "snippet.txt",
data: []byte("<!DOCTYPE html><html><body>hello</body></html>"),
want: "text/plain",
},
{
name: "XMLStaysBlocked",
fileName: "note.xml",
data: []byte(`<?xml version="1.0"?><note><to>Tove</to></note>`),
want: "text/xml",
},
{
name: "SVGBlockedEvenWhenNamedText",
fileName: "notes.txt",
data: []byte(`<svg xmlns="http://www.w3.org/2000/svg"><text>Hello</text></svg>`),
want: "image/svg+xml",
},
{
name: "MarkdownMentioningSVGStaysMarkdown",
fileName: "notes.md",
data: []byte("# SVG Example\n<svg width=\"100\">...</svg>"),
want: "text/markdown",
},
{
name: "CSVMentioningSVGStaysCSV",
fileName: "report.csv",
data: []byte("name,icon\nlogo,<svg><rect/></svg>\n"),
want: "text/csv",
},
{
name: "TextMentioningSVGStaysPlainText",
fileName: "main.go",
data: []byte("package main\n// renders <svg> tags\n"),
want: "text/plain",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
require.Equal(t, tt.want, chatfiles.ClassifyStoredMediaType(tt.fileName, tt.data))
})
}
}
func TestPrepareStoredFile(t *testing.T) {
t.Parallel()
t.Run("UsesDetectNameForSubtypeRefinement", func(t *testing.T) {
t.Parallel()
name, mediaType, err := chatfiles.PrepareStoredFile(
"payload.txt",
"report.json",
[]byte(`{"ok":true}`),
)
require.NoError(t, err)
require.Equal(t, "payload.txt", name)
require.Equal(t, "application/json", mediaType)
})
t.Run("StripsControlCharactersAndTrimsExposedWhitespace", func(t *testing.T) {
t.Parallel()
name, mediaType, err := chatfiles.PrepareStoredFile(
"\x00 release\t notes.txt \x00",
"release-notes.txt",
[]byte("hello"),
)
require.NoError(t, err)
require.Equal(t, "release notes.txt", name)
require.Equal(t, "text/plain", mediaType)
})
t.Run("RejectsEmptyNormalizedName", func(t *testing.T) {
t.Parallel()
_, _, err := chatfiles.PrepareStoredFile(
" \r\n\t ",
"notes.txt",
[]byte("hello"),
)
require.ErrorIs(t, err, chatfiles.ErrStoredFileNameRequired)
})
t.Run("RejectsUnsupportedStoredFileType", func(t *testing.T) {
t.Parallel()
_, _, err := chatfiles.PrepareStoredFile(
"evil.svg",
"evil.svg",
[]byte(`<svg xmlns="http://www.w3.org/2000/svg"><rect/></svg>`),
)
require.ErrorIs(t, err, chatfiles.ErrUnsupportedStoredFileType)
require.ErrorContains(t, err, "image/svg+xml")
})
t.Run("TruncatesNamesAtRuneBoundaries", func(t *testing.T) {
t.Parallel()
name, _, err := chatfiles.PrepareStoredFile(
strings.Repeat("界", 100),
"notes.txt",
[]byte("hello"),
)
require.NoError(t, err)
require.Equal(t, strings.Repeat("界", 85), name)
require.Equal(t, 255, len(name))
})
}
func TestPrepareRecordingArtifact(t *testing.T) {
t.Parallel()
t.Run("MP4", func(t *testing.T) {
t.Parallel()
name, mediaType, err := chatfiles.PrepareRecordingArtifact(
"recording.mp4",
"video/mp4",
[]byte{0x00, 0x00, 0x00, 0x18, 'f', 't', 'y', 'p', 'm', 'p', '4', '2', 0x00, 0x00, 0x00, 0x00, 'm', 'p', '4', '1', 'i', 's', 'o', 'm'},
)
require.NoError(t, err)
require.Equal(t, "recording.mp4", name)
require.Equal(t, "video/mp4", mediaType)
})
t.Run("JPEG", func(t *testing.T) {
t.Parallel()
name, mediaType, err := chatfiles.PrepareRecordingArtifact(
"thumbnail.jpg",
"image/jpeg",
[]byte{0xFF, 0xD8, 0xFF, 0xE0, 0x00, 0x10, 'J', 'F', 'I', 'F', 0x00, 0x01, 0x01, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00},
)
require.NoError(t, err)
require.Equal(t, "thumbnail.jpg", name)
require.Equal(t, "image/jpeg", mediaType)
})
t.Run("TypeMismatch", func(t *testing.T) {
t.Parallel()
_, _, err := chatfiles.PrepareRecordingArtifact(
"recording.mp4",
"video/mp4",
[]byte{0xFF, 0xD8, 0xFF, 0xE0, 0x00, 0x10, 'J', 'F', 'I', 'F', 0x00, 0x01, 0x01, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00},
)
require.ErrorContains(t, err, "recording artifact type mismatch")
})
t.Run("RejectsEmptyNormalizedName", func(t *testing.T) {
t.Parallel()
_, _, err := chatfiles.PrepareRecordingArtifact(
" \r\n\t ",
"video/mp4",
[]byte{0x00, 0x00, 0x00, 0x18, 'f', 't', 'y', 'p', 'm', 'p', '4', '2', 0x00, 0x00, 0x00, 0x00, 'm', 'p', '4', '1', 'i', 's', 'o', 'm'},
)
require.ErrorIs(t, err, chatfiles.ErrStoredFileNameRequired)
})
t.Run("UnsupportedExpectedType", func(t *testing.T) {
t.Parallel()
_, _, err := chatfiles.PrepareRecordingArtifact(
"recording.webm",
"video/webm",
[]byte("webm"),
)
require.ErrorContains(t, err, "unsupported recording artifact type")
})
}
func TestIsCompatibleUploadMediaType(t *testing.T) {
t.Parallel()
tests := []struct {
name string
declared string
stored string
want bool
}{
{
name: "ExactMatch",
declared: "text/plain",
stored: "text/plain",
want: true,
},
{
name: "TextPlainRefinesToMarkdown",
declared: "text/plain",
stored: "text/markdown",
want: true,
},
{
name: "TextPlainRefinesToCSV",
declared: "text/plain",
stored: "text/csv",
want: true,
},
{
name: "TextPlainRefinesToJSON",
declared: "text/plain",
stored: "application/json",
want: true,
},
{
name: "TextPlainDoesNotRefineToPNG",
declared: "text/plain",
stored: "image/png",
want: false,
},
{
name: "JSONDoesNotRefineToPlainText",
declared: "application/json",
stored: "text/plain",
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
require.Equal(t, tt.want, chatfiles.IsCompatibleUploadMediaType(tt.declared, tt.stored))
})
}
}
func TestIsAllowedStoredMediaType(t *testing.T) {
t.Parallel()
require.True(t, chatfiles.IsAllowedStoredMediaType("text/plain; charset=utf-8"))
require.True(t, chatfiles.IsAllowedStoredMediaType("text/markdown"))
require.True(t, chatfiles.IsAllowedStoredMediaType("text/csv"))
require.True(t, chatfiles.IsAllowedStoredMediaType("application/json"))
require.True(t, chatfiles.IsAllowedStoredMediaType("application/pdf"))
require.True(t, chatfiles.IsAllowedStoredMediaType("image/png"))
require.False(t, chatfiles.IsAllowedStoredMediaType("image/svg+xml"))
require.False(t, chatfiles.IsAllowedStoredMediaType("image/avif"))
require.False(t, chatfiles.IsAllowedStoredMediaType("application/zip"))
}
func TestIsInlineRenderableStoredMediaType(t *testing.T) {
t.Parallel()
require.True(t, chatfiles.IsInlineRenderableStoredMediaType("text/plain; charset=utf-8"))
require.True(t, chatfiles.IsInlineRenderableStoredMediaType("text/markdown"))
require.True(t, chatfiles.IsInlineRenderableStoredMediaType("image/png"))
require.False(t, chatfiles.IsInlineRenderableStoredMediaType("application/pdf"))
require.False(t, chatfiles.IsInlineRenderableStoredMediaType("image/svg+xml"))
}
func TestHasSVGRootElement(t *testing.T) {
t.Parallel()
require.True(t, chatfiles.HasSVGRootElement([]byte(`<?xml version="1.0"?><svg xmlns="http://www.w3.org/2000/svg"></svg>`)))
require.True(t, chatfiles.HasSVGRootElement([]byte("\xef\xbb\xbf<svg></svg>")))
require.False(t, chatfiles.HasSVGRootElement([]byte("<html><body>not svg</body></html>")))
require.False(t, chatfiles.HasSVGRootElement([]byte("# SVG Example\n<svg width=\"100\">...</svg>")))
require.False(t, chatfiles.HasSVGRootElement([]byte("name,icon\nlogo,<svg><rect/></svg>\n")))
}
+54 -114
View File
@@ -1,8 +1,6 @@
package coderd
import (
"bufio"
"bytes"
"context"
"database/sql"
"encoding/json"
@@ -14,7 +12,6 @@ import (
"net/http"
"net/http/httptest"
"net/url"
"slices"
"strconv"
"strings"
"sync"
@@ -30,6 +27,7 @@ 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"
@@ -3292,49 +3290,8 @@ func parseCompactionThresholdKey(key string) (uuid.UUID, error) {
const (
// maxChatFileSize is the maximum size of a chat file upload (10 MB).
maxChatFileSize = 10 << 20
// maxChatFileName is the maximum length of an uploaded file name.
maxChatFileName = 255
)
// allowedChatFileMIMETypes lists the content types accepted for chat
// file uploads. SVG is explicitly excluded because it can contain scripts.
var allowedChatFileMIMETypes = map[string]bool{
"image/png": true,
"image/jpeg": true,
"image/gif": true,
"image/webp": true,
"text/plain": true,
"image/svg+xml": false, // SVG can contain scripts.
}
func allowedChatFileMIMETypesStr() string {
var types []string
for t, allowed := range allowedChatFileMIMETypes {
if allowed {
types = append(types, t)
}
}
slices.Sort(types)
return strings.Join(types, ", ")
}
var (
webpMagicRIFF = []byte("RIFF")
webpMagicWEBP = []byte("WEBP")
)
// detectChatFileType detects the MIME type of the given data.
// It extends http.DetectContentType with support for WebP, which
// Go's standard sniffer does not recognize.
func detectChatFileType(data []byte) string {
if len(data) >= 12 &&
bytes.Equal(data[0:4], webpMagicRIFF) &&
bytes.Equal(data[8:12], webpMagicWEBP) {
return "image/webp"
}
return http.DetectContentType(data)
}
//nolint:revive // get-return: revive assumes get* must be a getter, but this is an HTTP handler.
func (api *API) getChatSystemPrompt(rw http.ResponseWriter, r *http.Request) {
ctx := r.Context()
@@ -4114,61 +4071,24 @@ func (api *API) postChatFile(rw http.ResponseWriter, r *http.Request) {
if mediaType, _, err := mime.ParseMediaType(contentType); err == nil {
contentType = mediaType
}
if allowed, ok := allowedChatFileMIMETypes[contentType]; !ok || !allowed {
if !chatfiles.IsAllowedStoredMediaType(contentType) {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Unsupported file type.",
Detail: fmt.Sprintf("Allowed types: %s.", allowedChatFileMIMETypesStr()),
Detail: fmt.Sprintf("Allowed types: %s.", chatfiles.AllowedStoredMediaTypesString()),
})
return
}
// Extract filename from Content-Disposition header if provided.
var filename string
if cd := r.Header.Get("Content-Disposition"); cd != "" {
if _, params, err := mime.ParseMediaType(cd); err == nil {
filename = params["filename"]
}
}
r.Body = http.MaxBytesReader(rw, r.Body, maxChatFileSize)
br := bufio.NewReader(r.Body)
// Peek at the leading bytes to sniff the real content type
// before reading the entire body.
peek, peekErr := br.Peek(512)
if peekErr != nil && !errors.Is(peekErr, io.EOF) && !errors.Is(peekErr, bufio.ErrBufferFull) {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Failed to read file from request.",
Detail: peekErr.Error(),
})
return
}
// Verify the actual content matches an allowed file type so that
// a client cannot spoof Content-Type to serve active content.
detected := detectChatFileType(peek)
if mediaType, _, err := mime.ParseMediaType(detected); err == nil {
detected = mediaType
}
if contentType == "text/plain" && strings.HasPrefix(detected, "text/") {
detected = "text/plain"
}
if allowed, ok := allowedChatFileMIMETypes[detected]; !ok || !allowed {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Unsupported file type.",
Detail: fmt.Sprintf("Allowed types: %s.", allowedChatFileMIMETypesStr()),
})
return
}
// The mismatch check below is security-critical: it prevents a text
// body from being uploaded under an image Content-Type (or vice
// versa) now that both text/plain and image types are in the
// allowlist. Combined with the X-Content-Type-Options: nosniff
// header applied globally, this ensures browsers respect the
// stored MIME type.
if detected != contentType {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "File content type does not match Content-Type header.",
Detail: fmt.Sprintf("Header declared %q but file content was detected as %q.", contentType, detected),
})
return
}
// Read the full body now that we know the type is valid.
data, err := io.ReadAll(br)
data, err := io.ReadAll(r.Body)
if err != nil {
var maxBytesErr *http.MaxBytesError
if errors.As(err, &maxBytesErr) {
@@ -4185,27 +4105,43 @@ func (api *API) postChatFile(rw http.ResponseWriter, r *http.Request) {
return
}
// Extract filename from Content-Disposition header if provided.
var filename string
if cd := r.Header.Get("Content-Disposition"); cd != "" {
if _, params, err := mime.ParseMediaType(cd); err == nil {
filename = params["filename"]
if len(filename) > maxChatFileName {
// Truncate at rune boundary to avoid splitting
// multi-byte UTF-8 characters.
var truncated []byte
for _, r := range filename {
encoded := []byte(string(r))
if len(truncated)+len(encoded) > maxChatFileName {
break
}
truncated = append(truncated, encoded...)
}
filename = string(truncated)
}
// Verify the actual content matches an allowed file type so that
// a client cannot spoof Content-Type to serve active content.
filename, detected, err := chatfiles.PrepareStoredFile(filename, filename, data)
if err != nil {
switch {
case errors.Is(err, chatfiles.ErrStoredFileNameRequired):
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Filename is required.",
Detail: "Provide a filename in the Content-Disposition header.",
})
case errors.Is(err, chatfiles.ErrUnsupportedStoredFileType):
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Unsupported file type.",
Detail: fmt.Sprintf("Allowed types: %s.", chatfiles.AllowedStoredMediaTypesString()),
})
default:
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Invalid file.",
Detail: err.Error(),
})
}
return
}
// The compatibility check below is security-critical: it keeps exact
// media-type matching by default while allowing safe text/plain
// refinements such as JSON, CSV, and Markdown now that upload
// classification can return richer stored media types. Combined with
// the X-Content-Type-Options: nosniff header applied globally, this
// still prevents clients from smuggling binary or active content under
// a safer declared Content-Type.
if !chatfiles.IsCompatibleUploadMediaType(contentType, detected) {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "File content type does not match Content-Type header.",
Detail: fmt.Sprintf("Header declared %q but file content was detected as %q.", contentType, detected),
})
return
}
chatFile, err := api.Database.InsertChatFile(ctx, database.InsertChatFileParams{
OwnerID: apiKey.UserID,
OrganizationID: orgID,
@@ -4252,10 +4188,14 @@ func (api *API) chatFileByID(rw http.ResponseWriter, r *http.Request) {
}
rw.Header().Set("Content-Type", chatFile.Mimetype)
disposition := "attachment"
if chatfiles.IsInlineRenderableStoredMediaType(chatFile.Mimetype) {
disposition = "inline"
}
if chatFile.Name != "" {
rw.Header().Set("Content-Disposition", mime.FormatMediaType("inline", map[string]string{"filename": chatFile.Name}))
rw.Header().Set("Content-Disposition", mime.FormatMediaType(disposition, map[string]string{"filename": chatFile.Name}))
} else {
rw.Header().Set("Content-Disposition", "inline")
rw.Header().Set("Content-Disposition", disposition)
}
rw.Header().Set("Cache-Control", "private, max-age=31536000, immutable")
rw.Header().Set("Content-Length", strconv.Itoa(len(chatFile.Data)))
@@ -4325,7 +4265,7 @@ func createChatInputFromParts(
Detail: fmt.Sprintf("Failed to retrieve file for %s[%d].", fieldName, i),
}
}
content = append(content, codersdk.ChatMessageFile(part.FileID, chatFile.Mimetype))
content = append(content, codersdk.ChatMessageFile(part.FileID, chatFile.Mimetype, chatFile.Name))
fileIDs = append(fileIDs, part.FileID)
// file-reference parts carry inline code snippets, not uploaded
// files. They have no FileID and are excluded from file tracking.
+74 -53
View File
@@ -6958,31 +6958,17 @@ func TestPostChatFile(t *testing.T) {
require.NotEqual(t, uuid.Nil, resp.ID)
})
t.Run("Success/JPEG", func(t *testing.T) {
t.Run("MissingFilename", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client := newChatClient(t)
firstUser := coderdtest.CreateFirstUser(t, client.Client)
data := append([]byte{0xFF, 0xD8, 0xFF, 0xE0}, make([]byte, 64)...)
resp, err := client.UploadChatFile(ctx, firstUser.OrganizationID, "image/jpeg", "test.jpg", bytes.NewReader(data))
require.NoError(t, err)
require.NotEqual(t, uuid.Nil, resp.ID)
})
t.Run("Success/WebP", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client := newChatClient(t)
firstUser := coderdtest.CreateFirstUser(t, client.Client)
// WebP: RIFF + 4-byte size + WEBP + padding.
data := append([]byte("RIFF"), make([]byte, 4)...)
data = append(data, []byte("WEBP")...)
data = append(data, make([]byte, 64)...)
resp, err := client.UploadChatFile(ctx, firstUser.OrganizationID, "image/webp", "test.webp", bytes.NewReader(data))
require.NoError(t, err)
require.NotEqual(t, uuid.Nil, resp.ID)
data := append([]byte{0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A}, make([]byte, 64)...)
_, err := client.UploadChatFile(ctx, firstUser.OrganizationID, "image/png", "", bytes.NewReader(data))
sdkErr := requireSDKError(t, err, http.StatusBadRequest)
require.Contains(t, sdkErr.Message, "Filename is required")
require.Contains(t, sdkErr.Detail, "Content-Disposition")
})
t.Run("Success/TextPlain", func(t *testing.T) {
@@ -6991,19 +6977,45 @@ func TestPostChatFile(t *testing.T) {
client := newChatClient(t)
firstUser := coderdtest.CreateFirstUser(t, client.Client)
data := []byte("This is a test paste.\nWith multiple lines.\n")
data := []byte(`This is a test paste.
With multiple lines.
`)
resp, err := client.UploadChatFile(ctx, firstUser.OrganizationID, "text/plain", "test.txt", bytes.NewReader(data))
require.NoError(t, err)
require.NotEqual(t, uuid.Nil, resp.ID)
})
t.Run("Success/TextPlainRefinesToJSON", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client := newChatClient(t)
firstUser := coderdtest.CreateFirstUser(t, client.Client)
resp, err := client.UploadChatFile(ctx, firstUser.OrganizationID, "text/plain", "pasted-text.txt", bytes.NewReader([]byte(`{"ok":true}`)))
require.NoError(t, err)
require.NotEqual(t, uuid.Nil, resp.ID)
})
t.Run("Success/TextPlainRefinesToCSV", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client := newChatClient(t)
firstUser := coderdtest.CreateFirstUser(t, client.Client)
resp, err := client.UploadChatFile(ctx, firstUser.OrganizationID, "text/plain", "pasted-text.txt", bytes.NewReader([]byte(`name,count
widgets,3
`)))
require.NoError(t, err)
require.NotEqual(t, uuid.Nil, resp.ID)
})
t.Run("UnsupportedContentType", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client := newChatClient(t)
firstUser := coderdtest.CreateFirstUser(t, client.Client)
_, err := client.UploadChatFile(ctx, firstUser.OrganizationID, "application/pdf", "test.pdf", bytes.NewReader([]byte("%PDF-1.7")))
_, err := client.UploadChatFile(ctx, firstUser.OrganizationID, "application/zip", "test.zip", bytes.NewReader([]byte("PK")))
requireSDKError(t, err, http.StatusBadRequest)
})
@@ -7017,18 +7029,6 @@ func TestPostChatFile(t *testing.T) {
requireSDKError(t, err, http.StatusBadRequest)
})
t.Run("ContentSniffingRejects", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client := newChatClient(t)
firstUser := coderdtest.CreateFirstUser(t, client.Client)
// Header says PNG but body is plain text.
_, err := client.UploadChatFile(ctx, firstUser.OrganizationID, "image/png", "test.png", bytes.NewReader([]byte("hello world")))
sdkErr := requireSDKError(t, err, http.StatusBadRequest)
require.Contains(t, sdkErr.Message, "does not match")
})
t.Run("ContentSniffingRejectsPNGAsText", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
@@ -7051,13 +7051,25 @@ func TestPostChatFile(t *testing.T) {
sdkErr := requireSDKError(t, err, http.StatusBadRequest)
require.Contains(t, sdkErr.Message, "does not match")
})
t.Run("ContentSniffingRejectsPlainTextAsJSON", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client := newChatClient(t)
firstUser := coderdtest.CreateFirstUser(t, client.Client)
_, err := client.UploadChatFile(ctx, firstUser.OrganizationID, "application/json", "payload.json", bytes.NewReader([]byte("not actually json")))
sdkErr := requireSDKError(t, err, http.StatusBadRequest)
require.Contains(t, sdkErr.Message, "does not match")
})
t.Run("TooLarge", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client := newChatClient(t)
firstUser := coderdtest.CreateFirstUser(t, client.Client)
// 10 MB + 1 byte, with valid PNG header to pass MIME check.
// 10 MB + 1 byte, with valid PNG header to pass media type check.
data := make([]byte, 10<<20+1)
copy(data, []byte{0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A})
_, err := client.UploadChatFile(ctx, firstUser.OrganizationID, "image/png", "test.png", bytes.NewReader(data))
@@ -7070,7 +7082,9 @@ func TestPostChatFile(t *testing.T) {
client := newChatClient(t)
firstUser := coderdtest.CreateFirstUser(t, client.Client)
data := []byte("<!DOCTYPE html>\n<html><body><p>Paste me as plain text.</p></body></html>\n")
data := []byte(`<!DOCTYPE html>
<html><body><p>Paste me as plain text.</p></body></html>
`)
resp, err := client.UploadChatFile(ctx, firstUser.OrganizationID, "text/plain", "snippet.txt", bytes.NewReader(data))
require.NoError(t, err)
require.NotEqual(t, uuid.Nil, resp.ID)
@@ -7160,22 +7174,6 @@ func TestGetChatFile(t *testing.T) {
require.Equal(t, data, got)
})
t.Run("Success/TextPlain", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client := newChatClient(t)
firstUser := coderdtest.CreateFirstUser(t, client.Client)
data := []byte("This is a test paste.\nWith multiple lines.\n")
uploaded, err := client.UploadChatFile(ctx, firstUser.OrganizationID, "text/plain", "test.txt", bytes.NewReader(data))
require.NoError(t, err)
got, contentType, err := client.GetChatFile(ctx, uploaded.ID)
require.NoError(t, err)
require.Equal(t, "text/plain", contentType)
require.Equal(t, data, got)
})
t.Run("CacheHeaders", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
@@ -7197,6 +7195,29 @@ func TestGetChatFile(t *testing.T) {
require.Contains(t, res.Header.Get("Content-Disposition"), "test.png")
})
t.Run("PDFServedAsAttachment", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client := newChatClient(t)
firstUser := coderdtest.CreateFirstUser(t, client.Client)
uploaded, err := client.UploadChatFile(ctx, firstUser.OrganizationID, "application/pdf", "report.pdf", bytes.NewReader([]byte("%PDF-1.7\n")))
require.NoError(t, err)
res, err := client.Request(ctx, http.MethodGet,
fmt.Sprintf("/api/experimental/chats/files/%s", uploaded.ID), nil)
require.NoError(t, err)
defer res.Body.Close()
require.Equal(t, http.StatusOK, res.StatusCode)
require.Equal(t, "application/pdf", res.Header.Get("Content-Type"))
require.Equal(t, "nosniff", res.Header.Get("X-Content-Type-Options"))
disposition, params, err := mime.ParseMediaType(res.Header.Get("Content-Disposition"))
require.NoError(t, err)
require.Equal(t, "attachment", disposition)
require.Equal(t, "report.pdf", params["filename"])
})
t.Run("LongFilename", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
@@ -7213,7 +7234,7 @@ func TestGetChatFile(t *testing.T) {
require.NoError(t, err)
defer res.Body.Close()
require.Equal(t, http.StatusOK, res.StatusCode)
// Filename should be truncated to maxChatFileName (255) bytes.
// Filename should be truncated to chatfiles.MaxStoredFileNameBytes (255) bytes.
cd := res.Header.Get("Content-Disposition")
require.Contains(t, cd, "inline")
require.Contains(t, cd, strings.Repeat("a", 255))
+63
View File
@@ -0,0 +1,63 @@
package chatd
import (
"context"
"charm.land/fantasy"
"github.com/google/uuid"
"cdr.dev/slog/v3"
"github.com/coder/coder/v2/coderd/x/chatd/chatloop"
"github.com/coder/coder/v2/coderd/x/chatd/chatprompt"
"github.com/coder/coder/v2/coderd/x/chatd/chattool"
"github.com/coder/coder/v2/codersdk"
)
func buildAssistantPartsForPersist(
ctx context.Context,
logger slog.Logger,
assistantBlocks []fantasy.Content,
toolResults []fantasy.ToolResultContent,
step chatloop.PersistedStep,
toolNameToConfigID map[string]uuid.UUID,
) []codersdk.ChatMessagePart {
parts := make([]codersdk.ChatMessagePart, 0, len(assistantBlocks)+len(toolResults))
for _, block := range assistantBlocks {
part := chatprompt.PartFromContent(block)
if part.ToolName != "" {
if configID, ok := toolNameToConfigID[part.ToolName]; ok {
part.MCPServerConfigID = uuid.NullUUID{UUID: configID, Valid: true}
}
}
if part.Type == codersdk.ChatMessagePartTypeToolCall && part.ToolCallID != "" && step.ToolCallCreatedAt != nil {
if ts, ok := step.ToolCallCreatedAt[part.ToolCallID]; ok {
part.CreatedAt = &ts
}
}
if part.Type == codersdk.ChatMessagePartTypeToolResult && part.ToolCallID != "" && step.ToolResultCreatedAt != nil {
if ts, ok := step.ToolResultCreatedAt[part.ToolCallID]; ok {
part.CreatedAt = &ts
}
}
parts = append(parts, part)
}
for _, tr := range toolResults {
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),
)
continue
}
for _, attachment := range attachments {
parts = append(parts, codersdk.ChatMessageFile(
attachment.FileID,
attachment.MediaType,
attachment.Name,
))
}
}
return parts
}
+136
View File
@@ -0,0 +1,136 @@
package chatd //nolint:testpackage
import (
"context"
"testing"
"time"
"charm.land/fantasy"
"github.com/google/uuid"
"github.com/stretchr/testify/require"
"github.com/coder/coder/v2/coderd/x/chatd/chatloop"
"github.com/coder/coder/v2/coderd/x/chatd/chattool"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/testutil"
)
func TestBuildAssistantPartsForPersist_PromotesToolAttachments(t *testing.T) {
t.Parallel()
fileID := uuid.MustParse("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee")
response := chattool.WithAttachments(
fantasy.NewTextResponse(`{"ok":true}`),
chattool.AttachmentMetadata{
FileID: fileID,
MediaType: "image/png",
Name: "screenshot.png",
},
)
toolCallAt := time.Date(2026, time.April, 10, 0, 0, 0, 0, time.UTC)
parts := buildAssistantPartsForPersist(
context.Background(),
testutil.Logger(t),
[]fantasy.Content{fantasy.TextContent{Text: "Here is the screenshot."}},
[]fantasy.ToolResultContent{{
ToolCallID: "call-1",
ToolName: "computer",
ClientMetadata: response.Metadata,
ProviderExecuted: false,
}},
chatloop.PersistedStep{
ToolCallCreatedAt: map[string]time.Time{
"call-1": toolCallAt,
},
},
nil,
)
require.Len(t, parts, 2)
require.Equal(t, codersdk.ChatMessagePartTypeText, parts[0].Type)
require.Equal(t, "Here is the screenshot.", parts[0].Text)
require.Equal(t, codersdk.ChatMessagePartTypeFile, parts[1].Type)
require.True(t, parts[1].FileID.Valid)
require.Equal(t, fileID, parts[1].FileID.UUID)
require.Equal(t, "image/png", parts[1].MediaType)
require.Equal(t, "screenshot.png", parts[1].Name)
}
func TestBuildAssistantPartsForPersist_PromotesProposePlanAttachment(t *testing.T) {
t.Parallel()
fileID := uuid.MustParse("bbbbbbbb-cccc-dddd-eeee-ffffffffffff")
response := chattool.WithAttachments(
fantasy.NewTextResponse(`{"ok":true,"kind":"plan"}`),
chattool.AttachmentMetadata{
FileID: fileID,
MediaType: "text/markdown",
Name: "PLAN.md",
},
)
parts := buildAssistantPartsForPersist(
context.Background(),
testutil.Logger(t),
[]fantasy.Content{fantasy.TextContent{Text: "Here is the proposed plan."}},
[]fantasy.ToolResultContent{{
ToolCallID: "call-plan",
ToolName: "propose_plan",
ClientMetadata: response.Metadata,
}},
chatloop.PersistedStep{},
nil,
)
require.Len(t, parts, 2)
require.Equal(t, codersdk.ChatMessagePartTypeText, parts[0].Type)
require.Equal(t, "Here is the proposed plan.", parts[0].Text)
require.Equal(t, codersdk.ChatMessagePartTypeFile, parts[1].Type)
require.True(t, parts[1].FileID.Valid)
require.Equal(t, fileID, parts[1].FileID.UUID)
require.Equal(t, "text/markdown", parts[1].MediaType)
require.Equal(t, "PLAN.md", parts[1].Name)
}
func TestBuildAssistantPartsForPersist_InvalidAttachmentMetadataSkipsOnlyBrokenResult(t *testing.T) {
t.Parallel()
goodFileID := uuid.MustParse("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee")
goodResponse := chattool.WithAttachments(
fantasy.NewTextResponse(`{"ok":true}`),
chattool.AttachmentMetadata{
FileID: goodFileID,
MediaType: "image/png",
Name: "good.png",
},
)
parts := buildAssistantPartsForPersist(
context.Background(),
testutil.Logger(t),
[]fantasy.Content{fantasy.TextContent{Text: "Here are the results."}},
[]fantasy.ToolResultContent{
{
ToolCallID: "call-good",
ToolName: "computer",
ClientMetadata: goodResponse.Metadata,
},
{
ToolCallID: "call-bad",
ToolName: "attach_file",
ClientMetadata: `{"attachments":[{"file_id":"aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"}]}`,
},
},
chatloop.PersistedStep{},
nil,
)
require.Len(t, parts, 2)
require.Equal(t, codersdk.ChatMessagePartTypeText, parts[0].Type)
require.Equal(t, codersdk.ChatMessagePartTypeFile, parts[1].Type)
require.True(t, parts[1].FileID.Valid)
require.Equal(t, goodFileID, parts[1].FileID.UUID)
require.Equal(t, "image/png", parts[1].MediaType)
require.Equal(t, "good.png", parts[1].Name)
}
+24 -87
View File
@@ -4713,6 +4713,7 @@ type rootChatToolsOptions struct {
instruction *string
skills *[]chattool.SkillMeta
resolvePlanPath func(context.Context) (string, string, error)
storeFile chattool.StoreFileFunc
isPlanModeTurn bool
}
@@ -4815,9 +4816,7 @@ func (p *Server) appendRootChatTools(
GetWorkspaceConn: opts.workspaceCtx.getWorkspaceConn,
ResolvePlanPath: opts.resolvePlanPath,
IsPlanTurn: opts.isPlanModeTurn,
StoreFile: func(ctx context.Context, name string, mediaType string, data []byte) (uuid.UUID, error) {
return p.storePlanSnapshotFile(ctx, opts.workspaceCtx, name, mediaType, data)
},
StoreFile: opts.storeFile,
}))
}
@@ -4826,59 +4825,6 @@ func (p *Server) appendRootChatTools(
}, opts.modelConfigID)...)
}
func (p *Server) storePlanSnapshotFile(
ctx context.Context,
workspaceCtx *turnWorkspaceContext,
name string,
mediaType string,
data []byte,
) (uuid.UUID, error) {
chatSnapshot := workspaceCtx.currentChatSnapshot()
if !chatSnapshot.WorkspaceID.Valid {
return uuid.Nil, xerrors.New("no workspace is associated with this chat. Use the create_workspace tool to create one")
}
ws, err := p.db.GetWorkspaceByID(ctx, chatSnapshot.WorkspaceID.UUID)
if err != nil {
return uuid.Nil, xerrors.Errorf("resolve workspace: %w", err)
}
row, err := p.db.InsertChatFile(ctx, database.InsertChatFileParams{
OwnerID: chatSnapshot.OwnerID,
OrganizationID: ws.OrganizationID,
Name: name,
Mimetype: mediaType,
Data: data,
})
if err != nil {
return uuid.Nil, xerrors.Errorf("insert chat file: %w", err)
}
// Cap enforcement and dedup are handled atomically in SQL.
// rejected > 0 means the cap was exceeded.
rejected, err := p.db.LinkChatFiles(ctx, database.LinkChatFilesParams{
ChatID: chatSnapshot.ID,
MaxFileLinks: int32(codersdk.MaxChatFileIDs),
FileIds: []uuid.UUID{row.ID},
})
switch {
case err != nil:
p.logger.Error(ctx, "failed to link file to chat",
slog.F("chat_id", chatSnapshot.ID),
slog.F("file_id", row.ID),
slog.Error(err),
)
case rejected > 0:
p.logger.Warn(ctx, "file cap reached, file not linked to chat",
slog.F("chat_id", chatSnapshot.ID),
slog.F("file_id", row.ID),
slog.F("max_file_links", codersdk.MaxChatFileIDs),
)
}
return row.ID, nil
}
func appendDynamicTools(
ctx context.Context,
logger slog.Logger,
@@ -5381,36 +5327,20 @@ func (p *Server) runChat(
// Pre-marshal all content outside the transaction so the
// FOR UPDATE lock is held only for the INSERT statements.
// Marshaling is pure CPU work with no database dependency.
assistantParts := buildAssistantPartsForPersist(
ctx,
p.logger,
assistantBlocks,
toolResults,
step,
toolNameToConfigID,
)
var assistantContent pqtype.NullRawMessage
if len(assistantBlocks) > 0 {
sdkParts := make([]codersdk.ChatMessagePart, 0, len(assistantBlocks))
for _, block := range assistantBlocks {
part := chatprompt.PartFromContent(block)
if part.ToolName != "" {
if configID, ok := toolNameToConfigID[part.ToolName]; ok {
part.MCPServerConfigID = uuid.NullUUID{UUID: configID, Valid: true}
}
}
// Apply recorded timestamps so persisted
// tool-call parts carry accurate CreatedAt.
if part.Type == codersdk.ChatMessagePartTypeToolCall && part.ToolCallID != "" && step.ToolCallCreatedAt != nil {
if ts, ok := step.ToolCallCreatedAt[part.ToolCallID]; ok {
part.CreatedAt = &ts
}
}
// Provider-executed tool results appear in
// assistantBlocks rather than toolResults,
// so apply their timestamps here as well.
if part.Type == codersdk.ChatMessagePartTypeToolResult && part.ToolCallID != "" && step.ToolResultCreatedAt != nil {
if ts, ok := step.ToolResultCreatedAt[part.ToolCallID]; ok {
part.CreatedAt = &ts
}
}
sdkParts = append(sdkParts, part)
}
finalAssistantText = strings.TrimSpace(contentBlocksToText(sdkParts))
if len(assistantParts) > 0 {
finalAssistantText = strings.TrimSpace(contentBlocksToText(assistantParts))
var marshalErr error
assistantContent, marshalErr = chatprompt.MarshalParts(sdkParts)
assistantContent, marshalErr = chatprompt.MarshalParts(assistantParts)
if marshalErr != nil {
return xerrors.Errorf("marshal assistant content: %w", marshalErr)
}
@@ -5443,7 +5373,7 @@ func (p *Server) runChat(
totalCostMicros := chatcost.CalculateTotalCostMicros(usageForCost, callConfig.Cost)
var insertedMessages []database.ChatMessage
err := p.db.InTx(func(tx database.Store) error {
if err := p.db.InTx(func(tx database.Store) error {
// Verify this worker still owns the chat before
// inserting messages. This closes the race where
// EditMessage soft-deletes history and clears worker_id
@@ -5536,8 +5466,7 @@ func (p *Server) runChat(
}
return nil
}, nil)
if err != nil {
}, nil); err != nil {
return xerrors.Errorf("persist step transaction: %w", err)
}
@@ -5633,6 +5562,7 @@ func (p *Server) runChat(
)
allowAskUserQuestion := isPlanModeTurn && isRootChat
storeChatAttachment := p.newStoreChatAttachmentFunc(&workspaceCtx)
tools := []fantasy.AgentTool{
chattool.ReadFile(chattool.ReadFileOptions{
GetWorkspaceConn: workspaceCtx.getWorkspaceConn,
@@ -5647,6 +5577,10 @@ func (p *Server) runChat(
ResolvePlanPath: resolvePlanPathForTools,
IsPlanTurn: isPlanModeTurn,
}),
chattool.AttachFile(chattool.AttachFileOptions{
GetWorkspaceConn: workspaceCtx.getWorkspaceConn,
StoreFile: storeChatAttachment,
}),
chattool.Execute(chattool.ExecuteOptions{
GetWorkspaceConn: workspaceCtx.getWorkspaceConn,
}),
@@ -5676,6 +5610,7 @@ func (p *Server) runChat(
instruction: &instruction,
skills: &skills,
resolvePlanPath: resolvePlanPathForTools,
storeFile: storeChatAttachment,
isPlanModeTurn: isPlanModeTurn,
})
}
@@ -5744,7 +5679,9 @@ func (p *Server) runChat(
desktopGeometry.DeclaredWidth,
desktopGeometry.DeclaredHeight,
workspaceCtx.getWorkspaceConn,
storeChatAttachment,
quartz.NewReal(),
p.logger.Named("computer_use"),
),
})
}
+14 -15
View File
@@ -64,19 +64,10 @@ func ExtractFileID(raw json.RawMessage) (uuid.UUID, error) {
return uuid.Parse(envelope.Data.FileID)
}
// ConvertMessages converts persisted chat messages into LLM prompt
// messages without resolving file references from storage. Inline
// file data is preserved when present (backward compat).
func ConvertMessages(
messages []database.ChatMessage,
) ([]fantasy.Message, error) {
return ConvertMessagesWithFiles(context.Background(), messages, nil, slog.Logger{})
}
// ConvertMessagesWithFiles converts persisted chat messages into LLM
// prompt messages, resolving file references via the provided
// resolver. When resolver is nil, file blocks without inline data
// are passed through as-is (same behavior as ConvertMessages).
// prompt messages, resolving user file references via the provided
// resolver. Persisted file references without bytes are omitted from
// the prompt instead of being replayed back to the model.
func ConvertMessagesWithFiles(
ctx context.Context,
messages []database.ChatMessage,
@@ -85,7 +76,8 @@ func ConvertMessagesWithFiles(
) ([]fantasy.Message, error) {
// Phase 1: Parse all messages via ParseContent (→ SDK parts)
// and collect file_id references from user messages for batch
// resolution.
// resolution. Assistant-side file attachments remain persisted chat
// metadata and are intentionally not replayed to the model.
type parsedMessage struct {
role codersdk.ChatMessageRole
parts []codersdk.ChatMessagePart
@@ -162,7 +154,7 @@ func ConvertMessagesWithFiles(
})
case codersdk.ChatMessageRoleAssistant:
fantasyParts := normalizeAssistantToolCallInputs(
partsToMessageParts(logger, pm.parts, resolved),
partsToMessageParts(logger, pm.parts, nil),
)
for _, toolCall := range ExtractToolCalls(fantasyParts) {
if toolCall.ToolCallID == "" || strings.TrimSpace(toolCall.ToolName) == "" {
@@ -186,7 +178,7 @@ func ConvertMessagesWithFiles(
}
}
}
toolParts := partsToMessageParts(logger, pm.parts, resolved)
toolParts := partsToMessageParts(logger, pm.parts, nil)
if len(toolParts) == 0 {
continue
}
@@ -1338,6 +1330,13 @@ func partsToMessageParts(
}
}
}
if len(data) == 0 {
// File parts without bytes are persistence metadata, not
// prompt content. User uploads should have been resolved
// above; assistant tool attachments intentionally are not
// replayed into later model turns.
continue
}
// Providers only accept a small set of MIME types in file
// content blocks, typically images and PDFs. A synthetic
// paste sent as a text/plain FilePart is dropped or rejected,
+93 -13
View File
@@ -44,7 +44,20 @@ func testMsgV1(role codersdk.ChatMessageRole, raw pqtype.NullRawMessage) databas
}
}
func TestConvertMessages_NormalizesAssistantToolCallInput(t *testing.T) {
func convertMessagesWithoutFiles(t *testing.T, messages []database.ChatMessage) []fantasy.Message {
t.Helper()
prompt, err := chatprompt.ConvertMessagesWithFiles(
context.Background(),
messages,
nil,
slogtest.Make(t, nil),
)
require.NoError(t, err)
return prompt
}
func TestConvertMessagesWithFiles_NormalizesAssistantToolCallInput(t *testing.T) {
t.Parallel()
testCases := []struct {
@@ -98,7 +111,7 @@ func TestConvertMessages_NormalizesAssistantToolCallInput(t *testing.T) {
)
require.NoError(t, err)
prompt, err := chatprompt.ConvertMessages([]database.ChatMessage{
prompt := convertMessagesWithoutFiles(t, []database.ChatMessage{
{
Role: database.ChatMessageRoleAssistant,
Visibility: database.ChatMessageVisibilityBoth,
@@ -110,7 +123,6 @@ func TestConvertMessages_NormalizesAssistantToolCallInput(t *testing.T) {
Content: toolContent,
},
})
require.NoError(t, err)
require.Len(t, prompt, 2)
require.Equal(t, fantasy.MessageRoleAssistant, prompt[0].Role)
@@ -302,7 +314,7 @@ func TestInjectMissingToolResults_SkipsProviderExecuted(t *testing.T) {
false, false, false,
)
prompt, err := chatprompt.ConvertMessages([]database.ChatMessage{
prompt := convertMessagesWithoutFiles(t, []database.ChatMessage{
{
Role: database.ChatMessageRoleAssistant,
Visibility: database.ChatMessageVisibilityBoth,
@@ -314,7 +326,6 @@ func TestInjectMissingToolResults_SkipsProviderExecuted(t *testing.T) {
Content: localResult,
},
})
require.NoError(t, err)
// Expected: assistant + tool(local result). No synthetic error
// for the provider-executed tool call.
@@ -404,7 +415,7 @@ func TestInjectMissingToolUses_DropsProviderExecutedOrphans(t *testing.T) {
false, false, false,
)
prompt, err := chatprompt.ConvertMessages([]database.ChatMessage{
prompt := convertMessagesWithoutFiles(t, []database.ChatMessage{
// Step 1
{Role: database.ChatMessageRoleAssistant, Visibility: database.ChatMessageVisibilityBoth, Content: step1Assistant},
{Role: database.ChatMessageRoleTool, Visibility: database.ChatMessageVisibilityBoth, Content: resultA},
@@ -419,7 +430,6 @@ func TestInjectMissingToolUses_DropsProviderExecutedOrphans(t *testing.T) {
fantasy.TextContent{Text: "?"},
})},
})
require.NoError(t, err)
// Expected message sequence:
// [0] assistant [tool_use A, B, C(PE)]
@@ -492,13 +502,12 @@ func TestInjectMissingToolUses_DropsOnlyProviderExecutedMessage(t *testing.T) {
false, false, true,
)
prompt, err := chatprompt.ConvertMessages([]database.ChatMessage{
prompt := convertMessagesWithoutFiles(t, []database.ChatMessage{
{Role: database.ChatMessageRoleAssistant, Visibility: database.ChatMessageVisibilityBoth, Content: assistantContent},
{Role: database.ChatMessageRoleTool, Visibility: database.ChatMessageVisibilityBoth, Content: localResult},
{Role: database.ChatMessageRoleAssistant, Visibility: database.ChatMessageVisibilityBoth, Content: assistant2Content},
{Role: database.ChatMessageRoleTool, Visibility: database.ChatMessageVisibilityBoth, Content: peResult},
})
require.NoError(t, err)
// The PE-only tool message should be dropped entirely.
// Expected: assistant, tool(local), assistant(text)
@@ -537,13 +546,12 @@ func TestProviderExecutedResultInAssistantContent(t *testing.T) {
fantasy.TextContent{Text: "Here is what I found."},
})
prompt, err := chatprompt.ConvertMessages([]database.ChatMessage{
prompt := convertMessagesWithoutFiles(t, []database.ChatMessage{
{Role: database.ChatMessageRoleAssistant, Visibility: database.ChatMessageVisibilityBoth, Content: assistantContent},
{Role: database.ChatMessageRoleUser, Visibility: database.ChatMessageVisibilityBoth, Content: mustMarshalContent(t, []fantasy.Content{
fantasy.TextContent{Text: "Thanks!"},
})},
})
require.NoError(t, err)
// Should be 2 messages: assistant + user.
require.Len(t, prompt, 2)
@@ -610,7 +618,7 @@ func TestProviderExecutedResult_LegacyToolRow(t *testing.T) {
false, false, false,
)
prompt, err := chatprompt.ConvertMessages([]database.ChatMessage{
prompt := convertMessagesWithoutFiles(t, []database.ChatMessage{
{Role: database.ChatMessageRoleAssistant, Visibility: database.ChatMessageVisibilityBoth, Content: assistantContent},
{Role: database.ChatMessageRoleTool, Visibility: database.ChatMessageVisibilityBoth, Content: peResult},
{Role: database.ChatMessageRoleTool, Visibility: database.ChatMessageVisibilityBoth, Content: execResult},
@@ -618,7 +626,6 @@ func TestProviderExecutedResult_LegacyToolRow(t *testing.T) {
fantasy.TextContent{Text: "next"},
})},
})
require.NoError(t, err)
// The PE tool result should be dropped by injectMissingToolUses,
// leaving: assistant, tool(exec), user.
@@ -1902,6 +1909,79 @@ func TestConvertMessagesWithFiles_IsSyntheticPaste(t *testing.T) {
}
}
func TestConvertMessagesWithFiles_AssistantAttachmentIsNotReplayed(t *testing.T) {
t.Parallel()
userFileID := uuid.New()
assistantFileID := uuid.New()
userContent, err := chatprompt.MarshalParts([]codersdk.ChatMessagePart{
codersdk.ChatMessageFile(userFileID, "image/png", "user.png"),
})
require.NoError(t, err)
assistantContent, err := chatprompt.MarshalParts([]codersdk.ChatMessagePart{
codersdk.ChatMessageText("I attached logs above."),
codersdk.ChatMessageFile(assistantFileID, "text/plain", "agent.log"),
})
require.NoError(t, err)
var resolverCalls [][]uuid.UUID
resolver := func(_ context.Context, ids []uuid.UUID) (map[uuid.UUID]chatprompt.FileData, error) {
resolverCalls = append(resolverCalls, append([]uuid.UUID(nil), ids...))
result := make(map[uuid.UUID]chatprompt.FileData, len(ids))
for _, id := range ids {
switch id {
case userFileID:
result[id] = chatprompt.FileData{
Name: "user.png",
Data: []byte("png-bytes"),
MediaType: "image/png",
}
case assistantFileID:
t.Fatalf("assistant attachment should not be resolved for prompt replay")
}
}
return result, nil
}
prompt, err := chatprompt.ConvertMessagesWithFiles(
context.Background(),
[]database.ChatMessage{
{
Role: database.ChatMessageRoleUser,
Visibility: database.ChatMessageVisibilityBoth,
Content: userContent,
},
{
Role: database.ChatMessageRoleAssistant,
Visibility: database.ChatMessageVisibilityBoth,
Content: assistantContent,
},
},
resolver,
slogtest.Make(t, nil),
)
require.NoError(t, err)
require.Len(t, resolverCalls, 1)
require.Equal(t, []uuid.UUID{userFileID}, resolverCalls[0])
require.Len(t, prompt, 2)
userFilePart, ok := fantasy.AsMessagePart[fantasy.FilePart](prompt[0].Content[0])
require.True(t, ok, "expected resolved user file to stay in the prompt")
require.Equal(t, []byte("png-bytes"), userFilePart.Data)
require.Equal(t, "image/png", userFilePart.MediaType)
require.Equal(t, fantasy.MessageRoleAssistant, prompt[1].Role)
require.Len(t, prompt[1].Content, 1)
assistantText, ok := fantasy.AsMessagePart[fantasy.TextPart](prompt[1].Content[0])
require.True(t, ok, "expected assistant text to remain after attachment omission")
require.Equal(t, "I attached logs above.", assistantText.Text)
_, hasAssistantFilePart := fantasy.AsMessagePart[fantasy.FilePart](prompt[1].Content[0])
require.False(t, hasAssistantFilePart, "assistant attachments should not be replayed into the prompt")
}
func convertSingleResolvedFileMessage(t *testing.T, fileID uuid.UUID, fileData chatprompt.FileData) []fantasy.Message {
t.Helper()
+78
View File
@@ -0,0 +1,78 @@
package chattool
import (
"context"
"strings"
"charm.land/fantasy"
"github.com/coder/coder/v2/codersdk/workspacesdk"
)
// AttachFileOptions configures the attach_file tool.
type AttachFileOptions struct {
GetWorkspaceConn func(context.Context) (workspacesdk.AgentConn, error)
StoreFile StoreFileFunc
}
// AttachFileArgs are the arguments for the attach_file tool.
type AttachFileArgs struct {
Path string `json:"path"`
Name string `json:"name,omitempty"`
}
// AttachFile returns a tool that stores a workspace file as a durable chat
// attachment so the user can download it directly from the conversation.
func AttachFile(options AttachFileOptions) fantasy.AgentTool {
return fantasy.NewAgentTool(
"attach_file",
"Attach a workspace file to the current chat so the user can download it directly from the conversation. "+
"Use this when the user should receive an artifact such as a screenshot, log, patch, or document. "+
"Pass an absolute file path. The file must already exist in the workspace.",
func(ctx context.Context, args AttachFileArgs, _ fantasy.ToolCall) (fantasy.ToolResponse, error) {
if options.GetWorkspaceConn == nil {
return fantasy.NewTextErrorResponse("workspace connection resolver is not configured"), nil
}
if options.StoreFile == nil {
return fantasy.NewTextErrorResponse("file storage is not configured"), nil
}
conn, err := options.GetWorkspaceConn(ctx)
if err != nil {
return fantasy.NewTextErrorResponse(err.Error()), nil
}
return executeAttachFileTool(ctx, conn, args, options.StoreFile)
},
)
}
func executeAttachFileTool(
ctx context.Context,
conn workspacesdk.AgentConn,
args AttachFileArgs,
storeFile StoreFileFunc,
) (fantasy.ToolResponse, error) {
path := strings.TrimSpace(args.Path)
if path == "" {
return fantasy.NewTextErrorResponse("path is required (use an absolute path, e.g. /home/coder/build.log)"), nil
}
attachment, size, err := storeWorkspaceAttachment(
ctx,
conn,
path,
strings.TrimSpace(args.Name),
storeFile,
)
if err != nil {
return fantasy.NewTextErrorResponse(err.Error()), nil
}
return WithAttachments(toolResponse(map[string]any{
"ok": true,
"path": path,
"file_id": attachment.FileID.String(),
"name": attachment.Name,
"media_type": attachment.MediaType,
"size": size,
}), attachment), nil
}
+290
View File
@@ -0,0 +1,290 @@
package chattool_test
import (
"context"
"encoding/json"
"io"
"strings"
"testing"
"charm.land/fantasy"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/mock/gomock"
"golang.org/x/xerrors"
"github.com/coder/coder/v2/coderd/x/chatd/chattool"
"github.com/coder/coder/v2/codersdk/workspacesdk"
"github.com/coder/coder/v2/codersdk/workspacesdk/agentconnmock"
)
type attachFileResponse struct {
OK bool `json:"ok"`
Path string `json:"path"`
FileID string `json:"file_id"`
Name string `json:"name"`
MediaType string `json:"media_type"`
Size int `json:"size"`
}
func TestAttachFile(t *testing.T) {
t.Parallel()
t.Run("EmptyPathReturnsError", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
tool := newAttachFileTool(t, mockConn, func(_ context.Context, _ string, _ string, _ []byte) (chattool.AttachmentMetadata, error) {
return chattool.AttachmentMetadata{}, nil
})
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1", Name: "attach_file", Input: `{"path":""}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.Contains(t, resp.Content, "path is required")
})
t.Run("RelativePathErrorComesFromAgent", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
mockConn.EXPECT().
ReadFile(gomock.Any(), "notes.txt", int64(0), int64(10<<20+1)).
Return(nil, "", xerrors.New(`file path must be absolute: "notes.txt"`))
tool := newAttachFileTool(t, mockConn, func(_ context.Context, _ string, _ string, _ []byte) (chattool.AttachmentMetadata, error) {
return chattool.AttachmentMetadata{}, nil
})
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1", Name: "attach_file", Input: `{"path":"notes.txt"}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.Contains(t, resp.Content, `file path must be absolute: "notes.txt"`)
})
t.Run("ValidTextFileStoresAttachment", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
content := "build succeeded\n"
mockConn.EXPECT().
ReadFile(gomock.Any(), "/home/coder/build.log", int64(0), int64(10<<20+1)).
Return(io.NopCloser(strings.NewReader(content)), "text/plain", nil)
var storedName string
var storedType string
var storedData []byte
tool := newAttachFileTool(t, mockConn, func(_ context.Context, name string, detectName string, data []byte) (chattool.AttachmentMetadata, error) {
storedName = name
require.Equal(t, "/home/coder/build.log", detectName)
storedType = "text/plain"
storedData = append([]byte(nil), data...)
return chattool.AttachmentMetadata{
FileID: uuid.MustParse("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"),
MediaType: storedType,
Name: name,
}, nil
})
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1", Name: "attach_file", Input: `{"path":"/home/coder/build.log"}`,
})
require.NoError(t, err)
assert.False(t, resp.IsError)
assert.Equal(t, "build.log", storedName)
assert.Equal(t, "text/plain", storedType)
assert.Equal(t, []byte(content), storedData)
decoded := decodeAttachFileResponse(t, resp)
assert.True(t, decoded.OK)
assert.Equal(t, "/home/coder/build.log", decoded.Path)
assert.Equal(t, "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", decoded.FileID)
assert.Equal(t, "build.log", decoded.Name)
assert.Equal(t, "text/plain", decoded.MediaType)
assert.Equal(t, len(content), decoded.Size)
attachments, err := chattool.AttachmentsFromMetadata(resp.Metadata)
require.NoError(t, err)
require.Len(t, attachments, 1)
assert.Equal(t, uuid.MustParse(decoded.FileID), attachments[0].FileID)
assert.Equal(t, decoded.MediaType, attachments[0].MediaType)
assert.Equal(t, decoded.Name, attachments[0].Name)
})
t.Run("WindowsAbsolutePathUsesBaseName", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
content := "build succeeded\n"
path := `C:\Users\coder\build.log`
mockConn.EXPECT().
ReadFile(gomock.Any(), path, int64(0), int64(10<<20+1)).
Return(io.NopCloser(strings.NewReader(content)), "text/plain", nil)
var storedName string
tool := newAttachFileTool(t, mockConn, func(_ context.Context, name string, detectName string, data []byte) (chattool.AttachmentMetadata, error) {
storedName = name
require.Equal(t, path, detectName)
assert.Equal(t, []byte(content), data)
return chattool.AttachmentMetadata{
FileID: uuid.MustParse("dddddddd-eeee-ffff-0000-111111111111"),
MediaType: "text/plain",
Name: name,
}, nil
})
input, err := json.Marshal(chattool.AttachFileArgs{Path: path})
require.NoError(t, err)
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-windows",
Name: "attach_file",
Input: string(input),
})
require.NoError(t, err)
assert.False(t, resp.IsError)
assert.Equal(t, "build.log", storedName)
decoded := decodeAttachFileResponse(t, resp)
assert.Equal(t, path, decoded.Path)
assert.Equal(t, "build.log", decoded.Name)
assert.Equal(t, len(content), decoded.Size)
})
t.Run("CustomNameOverridePreservesJSONSubtype", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
content := `{"ok":true}`
mockConn.EXPECT().
ReadFile(gomock.Any(), "/home/coder/report.json", int64(0), int64(10<<20+1)).
Return(io.NopCloser(strings.NewReader(content)), "text/plain", nil)
var storedName string
var storedType string
tool := newAttachFileTool(t, mockConn, func(_ context.Context, name string, detectName string, data []byte) (chattool.AttachmentMetadata, error) {
storedName = name
require.Equal(t, "/home/coder/report.json", detectName)
storedType = "application/json"
assert.Equal(t, []byte(content), data)
return chattool.AttachmentMetadata{
FileID: uuid.MustParse("bbbbbbbb-cccc-dddd-eeee-ffffffffffff"),
MediaType: storedType,
Name: name,
}, nil
})
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-json", Name: "attach_file", Input: `{"path":"/home/coder/report.json","name":"payload.txt"}`,
})
require.NoError(t, err)
assert.False(t, resp.IsError)
assert.Equal(t, "payload.txt", storedName)
assert.Equal(t, "application/json", storedType)
decoded := decodeAttachFileResponse(t, resp)
assert.Equal(t, "payload.txt", decoded.Name)
assert.Equal(t, "application/json", decoded.MediaType)
assert.Equal(t, len(content), decoded.Size)
})
t.Run("EmptyFileRejected", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
mockConn.EXPECT().
ReadFile(gomock.Any(), "/home/coder/empty.txt", int64(0), int64(10<<20+1)).
Return(io.NopCloser(strings.NewReader("")), "text/plain", nil)
tool := newAttachFileTool(t, mockConn, func(_ context.Context, _ string, _ string, _ []byte) (chattool.AttachmentMetadata, error) {
t.Fatal("storeFile should not be called for empty attachments")
return chattool.AttachmentMetadata{}, nil
})
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-empty", Name: "attach_file", Input: `{"path":"/home/coder/empty.txt"}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.Contains(t, resp.Content, "attachment is empty")
attachments, err := chattool.AttachmentsFromMetadata(resp.Metadata)
require.NoError(t, err)
assert.Empty(t, attachments)
})
t.Run("OversizedFileRejected", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
largeContent := strings.Repeat("x", 10<<20+1)
mockConn.EXPECT().
ReadFile(gomock.Any(), "/home/coder/build.log", int64(0), int64(10<<20+1)).
Return(io.NopCloser(strings.NewReader(largeContent)), "text/plain", nil)
tool := newAttachFileTool(t, mockConn, func(_ context.Context, _ string, _ string, _ []byte) (chattool.AttachmentMetadata, error) {
return chattool.AttachmentMetadata{}, xerrors.New("should not be called")
})
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1", Name: "attach_file", Input: `{"path":"/home/coder/build.log"}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.Contains(t, resp.Content, "attachment exceeds 10 MiB size limit")
})
t.Run("ReadFileError", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
mockConn.EXPECT().
ReadFile(gomock.Any(), "/home/coder/build.log", int64(0), int64(10<<20+1)).
Return(nil, "", xerrors.New("file not found"))
tool := newAttachFileTool(t, mockConn, func(_ context.Context, _ string, _ string, _ []byte) (chattool.AttachmentMetadata, error) {
return chattool.AttachmentMetadata{}, nil
})
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1", Name: "attach_file", Input: `{"path":"/home/coder/build.log"}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.Contains(t, resp.Content, "file not found")
})
t.Run("StoreFileErrorSurfaces", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
mockConn.EXPECT().
ReadFile(gomock.Any(), "/home/coder/build.log", int64(0), int64(10<<20+1)).
Return(io.NopCloser(strings.NewReader("build succeeded\n")), "text/plain", nil)
tool := newAttachFileTool(t, mockConn, func(_ context.Context, _ string, _ string, _ []byte) (chattool.AttachmentMetadata, error) {
return chattool.AttachmentMetadata{}, xerrors.New("chat already has the maximum of 20 linked files")
})
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-cap", Name: "attach_file", Input: `{"path":"/home/coder/build.log"}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.Contains(t, resp.Content, "chat already has the maximum of 20 linked files")
})
}
func newAttachFileTool(
t *testing.T,
mockConn *agentconnmock.MockAgentConn,
storeFile chattool.StoreFileFunc,
) fantasy.AgentTool {
t.Helper()
return chattool.AttachFile(chattool.AttachFileOptions{
GetWorkspaceConn: func(_ context.Context) (workspacesdk.AgentConn, error) {
return mockConn, nil
},
StoreFile: storeFile,
})
}
func decodeAttachFileResponse(t *testing.T, resp fantasy.ToolResponse) attachFileResponse {
t.Helper()
var result attachFileResponse
require.NoError(t, json.Unmarshal([]byte(resp.Content), &result))
return result
}
+171
View File
@@ -0,0 +1,171 @@
package chattool
import (
"context"
"encoding/base64"
"encoding/json"
"io"
"strings"
"charm.land/fantasy"
"github.com/google/uuid"
"golang.org/x/xerrors"
"github.com/coder/coder/v2/codersdk/workspacesdk"
)
const maxAttachmentSize = 10 << 20 // 10 MiB
// StoreFileFunc persists a chat attachment after classifying it for durable
// storage and returns the stored attachment metadata.
type StoreFileFunc func(ctx context.Context, name string, detectName string, data []byte) (AttachmentMetadata, error)
// AttachmentMetadata identifies a durable chat attachment that should be
// promoted into a standard file message part for the user.
type AttachmentMetadata struct {
FileID uuid.UUID `json:"file_id"`
MediaType string `json:"media_type"`
Name string `json:"name,omitempty"`
}
type attachmentResponseMetadata struct {
Attachments []AttachmentMetadata `json:"attachments,omitempty"`
}
func storeAttachmentData(
ctx context.Context,
storeFile StoreFileFunc,
name string,
detectName string,
data []byte,
) (AttachmentMetadata, error) {
if storeFile == nil {
return AttachmentMetadata{}, xerrors.New("file storage is not configured")
}
if len(data) == 0 {
return AttachmentMetadata{}, xerrors.New("attachment is empty")
}
if len(data) > maxAttachmentSize {
return AttachmentMetadata{}, xerrors.Errorf("attachment exceeds %d MiB size limit", maxAttachmentSize>>20)
}
name = strings.TrimSpace(name)
if name == "" {
return AttachmentMetadata{}, xerrors.New("attachment name is required")
}
if strings.TrimSpace(detectName) == "" {
detectName = name
}
attachment, err := storeFile(ctx, name, detectName, data)
if err != nil {
return AttachmentMetadata{}, err
}
if attachment.FileID == uuid.Nil {
return AttachmentMetadata{}, xerrors.New("stored attachment is missing file ID")
}
if attachment.MediaType == "" {
return AttachmentMetadata{}, xerrors.New("stored attachment is missing media type")
}
if attachment.Name == "" {
attachment.Name = name
}
return attachment, nil
}
func storeWorkspaceAttachment(
ctx context.Context,
conn workspacesdk.AgentConn,
path string,
name string,
storeFile StoreFileFunc,
) (AttachmentMetadata, int, error) {
if conn == nil {
return AttachmentMetadata{}, 0, xerrors.New("workspace connection is not configured")
}
if strings.TrimSpace(path) == "" {
return AttachmentMetadata{}, 0, xerrors.New("path is required")
}
reader, _, err := conn.ReadFile(ctx, path, 0, maxAttachmentSize+1)
if err != nil {
return AttachmentMetadata{}, 0, err
}
defer reader.Close()
data, err := io.ReadAll(io.LimitReader(reader, maxAttachmentSize+1))
if err != nil {
return AttachmentMetadata{}, 0, err
}
if strings.TrimSpace(name) == "" {
path = strings.TrimRight(path, "/\\")
if idx := strings.LastIndexAny(path, "/\\"); idx >= 0 {
name = path[idx+1:]
} else {
name = path
}
}
attachment, err := storeAttachmentData(ctx, storeFile, name, path, data)
if err != nil {
return AttachmentMetadata{}, 0, err
}
return attachment, len(data), nil
}
func storeScreenshotAttachment(
ctx context.Context,
storeFile StoreFileFunc,
name string,
encodedPNG string,
) (AttachmentMetadata, error) {
if strings.TrimSpace(encodedPNG) == "" {
return AttachmentMetadata{}, xerrors.New("screenshot data is empty")
}
decoder := base64.NewDecoder(base64.StdEncoding, strings.NewReader(encodedPNG))
data, err := io.ReadAll(io.LimitReader(decoder, maxAttachmentSize+1))
if err != nil {
return AttachmentMetadata{}, xerrors.Errorf("decode screenshot: %w", err)
}
if strings.TrimSpace(name) == "" {
name = "screenshot.png"
}
return storeAttachmentData(ctx, storeFile, name, name, data)
}
// WithAttachments stores durable attachment metadata on a tool response so the
// persistence layer can promote the files into assistant chat attachments.
func WithAttachments(
response fantasy.ToolResponse,
attachments ...AttachmentMetadata,
) fantasy.ToolResponse {
if len(attachments) == 0 {
return response
}
return fantasy.WithResponseMetadata(response, attachmentResponseMetadata{
Attachments: attachments,
})
}
// AttachmentsFromMetadata decodes durable attachment metadata from a tool
// response so the persistence layer can promote them into assistant file parts.
func AttachmentsFromMetadata(metadata string) ([]AttachmentMetadata, error) {
if strings.TrimSpace(metadata) == "" {
return nil, nil
}
var decoded attachmentResponseMetadata
if err := json.Unmarshal([]byte(metadata), &decoded); err != nil {
return nil, xerrors.Errorf("unmarshal attachment metadata: %w", err)
}
attachments := make([]AttachmentMetadata, 0, len(decoded.Attachments))
for i, attachment := range decoded.Attachments {
if attachment.FileID == uuid.Nil {
return nil, xerrors.Errorf("attachment %d is missing file_id", i)
}
if attachment.MediaType == "" {
return nil, xerrors.Errorf("attachment %d is missing media_type", i)
}
attachments = append(attachments, attachment)
}
return attachments, nil
}
+76 -49
View File
@@ -8,6 +8,7 @@ import (
"charm.land/fantasy"
fantasyanthropic "charm.land/fantasy/providers/anthropic"
"cdr.dev/slog/v3"
"github.com/coder/coder/v2/codersdk/workspacesdk"
"github.com/coder/quartz"
)
@@ -27,8 +28,10 @@ type computerUseTool struct {
declaredWidth int
declaredHeight int
getWorkspaceConn func(ctx context.Context) (workspacesdk.AgentConn, error)
storeFile StoreFileFunc
providerOptions fantasy.ProviderOptions
clock quartz.Clock
logger slog.Logger
}
// NewComputerUseTool creates a computer use AgentTool that delegates to the
@@ -38,22 +41,28 @@ type computerUseTool struct {
func NewComputerUseTool(
declaredWidth, declaredHeight int,
getWorkspaceConn func(ctx context.Context) (workspacesdk.AgentConn, error),
storeFile StoreFileFunc,
clock quartz.Clock,
logger slog.Logger,
) fantasy.AgentTool {
return &computerUseTool{
declaredWidth: declaredWidth,
declaredHeight: declaredHeight,
getWorkspaceConn: getWorkspaceConn,
storeFile: storeFile,
clock: clock,
logger: logger,
}
}
func (*computerUseTool) Info() fantasy.ToolInfo {
return fantasy.ToolInfo{
Name: "computer",
Description: "Control the desktop: take screenshots, move the mouse, click, type, and scroll.",
Parameters: map[string]any{},
Required: []string{},
Name: "computer",
Description: "Control the desktop: take screenshots, move the mouse, click, type, and scroll. " +
"Use an explicit screenshot action when you want to share a screenshot with the user; " +
"those screenshots are also attached to the chat.",
Parameters: map[string]any{},
Required: []string{},
}
}
@@ -110,38 +119,12 @@ func (t *computerUseTool) Run(ctx context.Context, call fantasy.ToolCall) (fanta
case <-ctx.Done():
case <-timer.C:
}
screenshotAction := workspacesdk.DesktopAction{
Action: "screenshot",
ScaledWidth: &declaredWidth,
ScaledHeight: &declaredHeight,
}
screenResp, sErr := conn.ExecuteDesktopAction(ctx, screenshotAction)
if sErr != nil {
return fantasy.NewTextErrorResponse(
fmt.Sprintf("screenshot failed: %v", sErr),
), nil
}
return fantasy.NewImageResponse(
[]byte(screenResp.ScreenshotData), "image/png",
), nil
return t.captureScreenshot(ctx, conn, declaredWidth, declaredHeight)
}
// For screenshot action, use ExecuteDesktopAction.
if input.Action == fantasyanthropic.ActionScreenshot {
screenshotAction := workspacesdk.DesktopAction{
Action: "screenshot",
ScaledWidth: &declaredWidth,
ScaledHeight: &declaredHeight,
}
screenResp, sErr := conn.ExecuteDesktopAction(ctx, screenshotAction)
if sErr != nil {
return fantasy.NewTextErrorResponse(
fmt.Sprintf("screenshot failed: %v", sErr),
), nil
}
return fantasy.NewImageResponse(
[]byte(screenResp.ScreenshotData), "image/png",
), nil
return t.captureSharedScreenshot(ctx, conn, declaredWidth, declaredHeight)
}
// Build the action request.
@@ -182,21 +165,72 @@ func (t *computerUseTool) Run(ctx context.Context, call fantasy.ToolCall) (fanta
}
// Take a screenshot after every action (Anthropic pattern).
return t.captureScreenshot(ctx, conn, declaredWidth, declaredHeight)
}
func (*computerUseTool) captureScreenshot(
ctx context.Context,
conn workspacesdk.AgentConn,
declaredWidth, declaredHeight int,
) (fantasy.ToolResponse, error) {
screenResp, err := executeScreenshotAction(ctx, conn, declaredWidth, declaredHeight)
if err != nil {
return fantasy.NewTextErrorResponse(
fmt.Sprintf("screenshot failed: %v", err),
), nil
}
return fantasy.NewImageResponse([]byte(screenResp.ScreenshotData), "image/png"), nil
}
func (t *computerUseTool) captureSharedScreenshot(
ctx context.Context,
conn workspacesdk.AgentConn,
declaredWidth, declaredHeight int,
) (fantasy.ToolResponse, error) {
screenResp, err := executeScreenshotAction(ctx, conn, declaredWidth, declaredHeight)
if err != nil {
return fantasy.NewTextErrorResponse(
fmt.Sprintf("screenshot failed: %v", err),
), nil
}
attachmentName := fmt.Sprintf(
"screenshot-%s.png",
t.clock.Now().UTC().Format("2006-01-02T15-04-05Z"),
)
if t.storeFile == nil {
t.logger.Warn(ctx, "screenshot attachment storage is not configured")
return fantasy.NewImageResponse([]byte(screenResp.ScreenshotData), "image/png"), nil
}
attachment, err := storeScreenshotAttachment(
ctx,
t.storeFile,
attachmentName,
screenResp.ScreenshotData,
)
response := fantasy.NewImageResponse([]byte(screenResp.ScreenshotData), "image/png")
if err != nil {
t.logger.Warn(ctx, "failed to persist screenshot attachment",
slog.F("attachment_name", attachmentName),
slog.Error(err),
)
return response, nil
}
return WithAttachments(response, attachment), nil
}
func executeScreenshotAction(
ctx context.Context,
conn workspacesdk.AgentConn,
declaredWidth, declaredHeight int,
) (workspacesdk.DesktopActionResponse, error) {
screenshotAction := workspacesdk.DesktopAction{
Action: "screenshot",
ScaledWidth: &declaredWidth,
ScaledHeight: &declaredHeight,
}
screenResp, sErr := conn.ExecuteDesktopAction(ctx, screenshotAction)
if sErr != nil {
return fantasy.NewTextErrorResponse(
fmt.Sprintf("screenshot failed: %v", sErr),
), nil
}
return fantasy.NewImageResponse(
[]byte(screenResp.ScreenshotData), "image/png",
), nil
return conn.ExecuteDesktopAction(ctx, screenshotAction)
}
func (t *computerUseTool) declaredActionDimensions() (declaredWidth, declaredHeight int) {
@@ -206,10 +240,3 @@ func (t *computerUseTool) declaredActionDimensions() (declaredWidth, declaredHei
}
return t.declaredWidth, t.declaredHeight
}
// computeScaledScreenshotSize preserves the historical helper name while using
// the shared declared-geometry selection logic.
func computeScaledScreenshotSize(width, height int) (scaledWidth int, scaledHeight int) {
geometry := workspacesdk.NewDesktopGeometry(width, height)
return geometry.DeclaredWidth, geometry.DeclaredHeight
}
@@ -1,85 +0,0 @@
package chattool
import (
"testing"
"github.com/stretchr/testify/assert"
)
func TestComputeScaledScreenshotSize(t *testing.T) {
t.Parallel()
tests := []struct {
name string
width, height int
wantW, wantH int
}{
{
name: "1920x1080_prefers_standard_1280x720",
width: 1920,
height: 1080,
wantW: 1280,
wantH: 720,
},
{
name: "1280x800_no_scaling",
width: 1280,
height: 800,
wantW: 1280,
wantH: 800,
},
{
name: "3840x2160_prefers_standard_1280x720",
width: 3840,
height: 2160,
wantW: 1280,
wantH: 720,
},
{
name: "1568x1000_prefers_standard_1280x816",
width: 1568,
height: 1000,
wantW: 1280,
wantH: 816,
},
{
name: "100x100_small_display",
width: 100,
height: 100,
wantW: 100,
wantH: 100,
},
{
name: "4000x3000_prefers_standard_1024x768",
width: 4000,
height: 3000,
wantW: 1024,
wantH: 768,
},
{
name: "1920x1200_prefers_standard_1280x800",
width: 1920,
height: 1200,
wantW: 1280,
wantH: 800,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
gotW, gotH := computeScaledScreenshotSize(tt.width, tt.height)
assert.Equal(t, tt.wantW, gotW)
assert.Equal(t, tt.wantH, gotH)
// Invariant: results must respect Anthropic constraints.
const maxLongEdge = 1568
const maxTotalPixels = 1_150_000
longEdge := max(gotW, gotH)
assert.LessOrEqual(t, longEdge, maxLongEdge,
"long edge %d exceeds max %d", longEdge, maxLongEdge)
assert.LessOrEqual(t, gotW*gotH, maxTotalPixels,
"total pixels %d exceeds max %d", gotW*gotH, maxTotalPixels)
})
}
}
+161 -32
View File
@@ -1,31 +1,25 @@
package chattool_test
import (
"bytes"
"context"
"encoding/base64"
"testing"
"charm.land/fantasy"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/mock/gomock"
"golang.org/x/xerrors"
"cdr.dev/slog/v3/sloggers/slogtest"
"github.com/coder/coder/v2/coderd/x/chatd/chattool"
"github.com/coder/coder/v2/codersdk/workspacesdk"
"github.com/coder/coder/v2/codersdk/workspacesdk/agentconnmock"
"github.com/coder/quartz"
)
func TestComputerUseTool_Info(t *testing.T) {
t.Parallel()
geometry := workspacesdk.DefaultDesktopGeometry()
tool := chattool.NewComputerUseTool(geometry.DeclaredWidth, geometry.DeclaredHeight, nil, quartz.NewReal())
info := tool.Info()
assert.Equal(t, "computer", info.Name)
assert.NotEmpty(t, info.Description)
}
func TestComputerUseProviderTool(t *testing.T) {
t.Parallel()
@@ -39,17 +33,6 @@ func TestComputerUseProviderTool(t *testing.T) {
assert.Equal(t, int64(geometry.DeclaredHeight), pdt.Args["display_height_px"])
}
func TestComputerUseProviderTool_PrefersDeclaredGeometry(t *testing.T) {
t.Parallel()
geometry := workspacesdk.NewDesktopGeometry(1920, 1080)
def := chattool.ComputerUseProviderTool(geometry.DeclaredWidth, geometry.DeclaredHeight)
pdt, ok := def.(fantasy.ProviderDefinedTool)
require.True(t, ok, "ComputerUseProviderTool should return a ProviderDefinedTool")
assert.Equal(t, int64(1280), pdt.Args["display_width_px"])
assert.Equal(t, int64(720), pdt.Args["display_height_px"])
}
func TestComputerUseTool_Run_Screenshot(t *testing.T) {
t.Parallel()
@@ -67,7 +50,7 @@ func TestComputerUseTool_Run_Screenshot(t *testing.T) {
assert.Equal(t, geometry.DeclaredHeight, *action.ScaledHeight)
return workspacesdk.DesktopActionResponse{
Output: "screenshot",
ScreenshotData: "base64png",
ScreenshotData: "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4n539HwAHFwLVF8kc1wAAAABJRU5ErkJggg==",
ScreenshotWidth: geometry.DeclaredWidth,
ScreenshotHeight: geometry.DeclaredHeight,
}, nil
@@ -75,7 +58,7 @@ func TestComputerUseTool_Run_Screenshot(t *testing.T) {
tool := chattool.NewComputerUseTool(geometry.DeclaredWidth, geometry.DeclaredHeight, func(_ context.Context) (workspacesdk.AgentConn, error) {
return mockConn, nil
}, quartz.NewReal())
}, nil, quartz.NewReal(), slogtest.Make(t, nil))
call := fantasy.ToolCall{
ID: "test-1",
@@ -87,16 +70,149 @@ func TestComputerUseTool_Run_Screenshot(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, "image", resp.Type)
assert.Equal(t, "image/png", resp.MediaType)
assert.Equal(t, []byte("base64png"), resp.Data)
assert.Equal(t, []byte("iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4n539HwAHFwLVF8kc1wAAAABJRU5ErkJggg=="), resp.Data)
assert.False(t, resp.IsError)
}
func TestComputerUseTool_Run_Screenshot_PersistsAttachment(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
geometry := workspacesdk.DefaultDesktopGeometry()
const screenshotPNG = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4n539HwAHFwLVF8kc1wAAAABJRU5ErkJggg=="
mockConn.EXPECT().ExecuteDesktopAction(
gomock.Any(),
gomock.AssignableToTypeOf(workspacesdk.DesktopAction{}),
).DoAndReturn(func(_ context.Context, action workspacesdk.DesktopAction) (workspacesdk.DesktopActionResponse, error) {
require.Equal(t, "screenshot", action.Action)
return workspacesdk.DesktopActionResponse{
Output: "screenshot",
ScreenshotData: screenshotPNG,
ScreenshotWidth: geometry.DeclaredWidth,
ScreenshotHeight: geometry.DeclaredHeight,
}, nil
})
var storedName string
var storedType string
var storedData []byte
tool := chattool.NewComputerUseTool(geometry.DeclaredWidth, geometry.DeclaredHeight, func(_ context.Context) (workspacesdk.AgentConn, error) {
return mockConn, nil
}, func(_ context.Context, name string, detectName string, data []byte) (chattool.AttachmentMetadata, error) {
storedName = name
require.Equal(t, name, detectName)
storedType = "image/png"
storedData = append([]byte(nil), data...)
return chattool.AttachmentMetadata{
FileID: uuid.MustParse("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"),
MediaType: storedType,
Name: name,
}, nil
}, quartz.NewReal(), slogtest.Make(t, nil))
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "test-screenshot-persist", Name: "computer", Input: `{"action":"screenshot"}`,
})
require.NoError(t, err)
assert.Equal(t, "image", resp.Type)
assert.Equal(t, "image/png", resp.MediaType)
assert.Equal(t, []byte(screenshotPNG), resp.Data)
assert.Contains(t, storedName, "screenshot-")
assert.Equal(t, "image/png", storedType)
expectedPNG, decodeErr := base64.StdEncoding.DecodeString(screenshotPNG)
require.NoError(t, decodeErr)
require.Equal(t, expectedPNG, storedData)
attachments, err := chattool.AttachmentsFromMetadata(resp.Metadata)
require.NoError(t, err)
require.Len(t, attachments, 1)
assert.Equal(t, uuid.MustParse("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"), attachments[0].FileID)
assert.Equal(t, "image/png", attachments[0].MediaType)
}
func TestComputerUseTool_Run_Screenshot_StoreErrorFallsBackToImage(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
geometry := workspacesdk.DefaultDesktopGeometry()
const screenshotPNG = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4n539HwAHFwLVF8kc1wAAAABJRU5ErkJggg=="
mockConn.EXPECT().ExecuteDesktopAction(
gomock.Any(),
gomock.AssignableToTypeOf(workspacesdk.DesktopAction{}),
).Return(workspacesdk.DesktopActionResponse{
Output: "screenshot",
ScreenshotData: screenshotPNG,
ScreenshotWidth: geometry.DeclaredWidth,
ScreenshotHeight: geometry.DeclaredHeight,
}, nil)
tool := chattool.NewComputerUseTool(geometry.DeclaredWidth, geometry.DeclaredHeight, func(_ context.Context) (workspacesdk.AgentConn, error) {
return mockConn, nil
}, func(_ context.Context, _ string, _ string, _ []byte) (chattool.AttachmentMetadata, error) {
return chattool.AttachmentMetadata{}, xerrors.New("chat already has the maximum of 20 linked files")
}, quartz.NewReal(), slogtest.Make(t, nil))
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "test-screenshot-store-error", Name: "computer", Input: `{"action":"screenshot"}`,
})
require.NoError(t, err)
assert.Equal(t, "image", resp.Type)
assert.Equal(t, "image/png", resp.MediaType)
assert.False(t, resp.IsError)
attachments, err := chattool.AttachmentsFromMetadata(resp.Metadata)
require.NoError(t, err)
assert.Empty(t, attachments)
}
func TestComputerUseTool_Run_Screenshot_OversizedAttachmentFallsBackToImage(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
geometry := workspacesdk.DefaultDesktopGeometry()
oversizedScreenshot := base64.StdEncoding.EncodeToString(bytes.Repeat([]byte{0xAB}, 10<<20+1))
mockConn.EXPECT().ExecuteDesktopAction(
gomock.Any(),
gomock.AssignableToTypeOf(workspacesdk.DesktopAction{}),
).Return(workspacesdk.DesktopActionResponse{
Output: "screenshot",
ScreenshotData: oversizedScreenshot,
ScreenshotWidth: geometry.DeclaredWidth,
ScreenshotHeight: geometry.DeclaredHeight,
}, nil)
tool := chattool.NewComputerUseTool(geometry.DeclaredWidth, geometry.DeclaredHeight, func(_ context.Context) (workspacesdk.AgentConn, error) {
return mockConn, nil
}, func(_ context.Context, _ string, _ string, _ []byte) (chattool.AttachmentMetadata, error) {
t.Fatal("storeFile should not be called for oversized screenshots")
return chattool.AttachmentMetadata{}, nil
}, quartz.NewReal(), slogtest.Make(t, nil))
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "test-screenshot-oversized", Name: "computer", Input: `{"action":"screenshot"}`,
})
require.NoError(t, err)
assert.Equal(t, "image", resp.Type)
assert.Equal(t, "image/png", resp.MediaType)
assert.False(t, resp.IsError)
require.Len(t, resp.Data, len(oversizedScreenshot))
attachments, err := chattool.AttachmentsFromMetadata(resp.Metadata)
require.NoError(t, err)
assert.Empty(t, attachments)
}
func TestComputerUseTool_Run_LeftClick(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
geometry := workspacesdk.DefaultDesktopGeometry()
followUpScreenshot := base64.StdEncoding.EncodeToString([]byte("after-click"))
mockConn.EXPECT().ExecuteDesktopAction(
gomock.Any(),
@@ -122,7 +238,7 @@ func TestComputerUseTool_Run_LeftClick(t *testing.T) {
assert.Equal(t, geometry.DeclaredHeight, *action.ScaledHeight)
return workspacesdk.DesktopActionResponse{
Output: "screenshot",
ScreenshotData: "after-click",
ScreenshotData: followUpScreenshot,
ScreenshotWidth: geometry.DeclaredWidth,
ScreenshotHeight: geometry.DeclaredHeight,
}, nil
@@ -130,7 +246,10 @@ func TestComputerUseTool_Run_LeftClick(t *testing.T) {
tool := chattool.NewComputerUseTool(geometry.DeclaredWidth, geometry.DeclaredHeight, func(_ context.Context) (workspacesdk.AgentConn, error) {
return mockConn, nil
}, quartz.NewReal())
}, func(_ context.Context, _ string, _ string, _ []byte) (chattool.AttachmentMetadata, error) {
t.Fatal("storeFile should not be called for left_click follow-up screenshots")
return chattool.AttachmentMetadata{}, nil
}, quartz.NewReal(), slogtest.Make(t, nil))
call := fantasy.ToolCall{
ID: "test-2",
@@ -141,7 +260,10 @@ func TestComputerUseTool_Run_LeftClick(t *testing.T) {
resp, err := tool.Run(context.Background(), call)
require.NoError(t, err)
assert.Equal(t, "image", resp.Type)
assert.Equal(t, []byte("after-click"), resp.Data)
assert.Equal(t, []byte(followUpScreenshot), resp.Data)
attachments, err := chattool.AttachmentsFromMetadata(resp.Metadata)
require.NoError(t, err)
assert.Empty(t, attachments)
}
func TestComputerUseTool_Run_Wait(t *testing.T) {
@@ -150,6 +272,7 @@ func TestComputerUseTool_Run_Wait(t *testing.T) {
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
geometry := workspacesdk.DefaultDesktopGeometry()
followUpScreenshot := base64.StdEncoding.EncodeToString([]byte("after-wait"))
mockConn.EXPECT().ExecuteDesktopAction(
gomock.Any(),
@@ -161,7 +284,7 @@ func TestComputerUseTool_Run_Wait(t *testing.T) {
assert.Equal(t, geometry.DeclaredHeight, *action.ScaledHeight)
return workspacesdk.DesktopActionResponse{
Output: "screenshot",
ScreenshotData: "after-wait",
ScreenshotData: followUpScreenshot,
ScreenshotWidth: geometry.DeclaredWidth,
ScreenshotHeight: geometry.DeclaredHeight,
}, nil
@@ -169,7 +292,10 @@ func TestComputerUseTool_Run_Wait(t *testing.T) {
tool := chattool.NewComputerUseTool(geometry.DeclaredWidth, geometry.DeclaredHeight, func(_ context.Context) (workspacesdk.AgentConn, error) {
return mockConn, nil
}, quartz.NewReal())
}, func(_ context.Context, _ string, _ string, _ []byte) (chattool.AttachmentMetadata, error) {
t.Fatal("storeFile should not be called for wait screenshots")
return chattool.AttachmentMetadata{}, nil
}, quartz.NewReal(), slogtest.Make(t, nil))
call := fantasy.ToolCall{
ID: "test-3",
@@ -181,8 +307,11 @@ func TestComputerUseTool_Run_Wait(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, "image", resp.Type)
assert.Equal(t, "image/png", resp.MediaType)
assert.Equal(t, []byte("after-wait"), resp.Data)
assert.Equal(t, []byte(followUpScreenshot), resp.Data)
assert.False(t, resp.IsError)
attachments, err := chattool.AttachmentsFromMetadata(resp.Metadata)
require.NoError(t, err)
assert.Empty(t, attachments)
}
func TestComputerUseTool_Run_ConnError(t *testing.T) {
@@ -191,7 +320,7 @@ func TestComputerUseTool_Run_ConnError(t *testing.T) {
geometry := workspacesdk.DefaultDesktopGeometry()
tool := chattool.NewComputerUseTool(geometry.DeclaredWidth, geometry.DeclaredHeight, func(_ context.Context) (workspacesdk.AgentConn, error) {
return nil, xerrors.New("workspace not available")
}, quartz.NewReal())
}, nil, quartz.NewReal(), slogtest.Make(t, nil))
call := fantasy.ToolCall{
ID: "test-4",
@@ -211,7 +340,7 @@ func TestComputerUseTool_Run_InvalidInput(t *testing.T) {
geometry := workspacesdk.DefaultDesktopGeometry()
tool := chattool.NewComputerUseTool(geometry.DeclaredWidth, geometry.DeclaredHeight, func(_ context.Context) (workspacesdk.AgentConn, error) {
return nil, xerrors.New("should not be called")
}, quartz.NewReal())
}, nil, quartz.NewReal(), slogtest.Make(t, nil))
call := fantasy.ToolCall{
ID: "test-5",
+7 -8
View File
@@ -7,7 +7,6 @@ import (
"strings"
"charm.land/fantasy"
"github.com/google/uuid"
"github.com/coder/coder/v2/codersdk/workspacesdk"
)
@@ -18,7 +17,7 @@ const maxProposePlanSize = 32 * 1024 // 32 KiB
type ProposePlanOptions struct {
GetWorkspaceConn func(context.Context) (workspacesdk.AgentConn, error)
ResolvePlanPath func(context.Context) (chatPath string, home string, err error)
StoreFile func(ctx context.Context, name string, mediaType string, data []byte) (uuid.UUID, error)
StoreFile StoreFileFunc
IsPlanTurn bool
}
@@ -72,7 +71,7 @@ func executeProposePlanTool(
conn workspacesdk.AgentConn,
args ProposePlanArgs,
resolvePlanPath func(context.Context) (chatPath string, home string, err error),
storeFile func(ctx context.Context, name string, mediaType string, data []byte) (uuid.UUID, error),
storeFile StoreFileFunc,
) (fantasy.ToolResponse, error) {
requestedPath := strings.TrimSpace(args.Path)
if requestedPath == "" {
@@ -113,16 +112,16 @@ func executeProposePlanTool(
return fantasy.NewTextErrorResponse("plan file exceeds 32 KiB size limit"), nil
}
fileID, err := storeFile(ctx, filepath.Base(requestedPath), "text/markdown", data)
attachment, err := storeFile(ctx, filepath.Base(requestedPath), requestedPath, data)
if err != nil {
return fantasy.NewTextErrorResponse("failed to store plan file: " + err.Error()), nil
}
return toolResponse(map[string]any{
return WithAttachments(toolResponse(map[string]any{
"ok": true,
"path": requestedPath,
"kind": "plan",
"file_id": fileID.String(),
"media_type": "text/markdown",
}), nil
"file_id": attachment.FileID.String(),
"media_type": attachment.MediaType,
}), attachment), nil
}
+444 -78
View File
@@ -4,8 +4,10 @@ import (
"context"
"encoding/json"
"io"
"path/filepath"
"strings"
"testing"
"testing/iotest"
"charm.land/fantasy"
"github.com/google/uuid"
@@ -30,13 +32,13 @@ type proposePlanResponse struct {
func TestProposePlan(t *testing.T) {
t.Parallel()
t.Run("RejectsEmptyPath", func(t *testing.T) {
t.Run("EmptyPathReturnsError", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
storeFile, _ := fakeStoreFile(t)
tool := newProposePlanToolWithPlanPath(t, mockConn, storeFile, nil, false)
tool := newProposePlanTool(t, mockConn, storeFile)
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "propose_plan",
@@ -47,23 +49,399 @@ func TestProposePlan(t *testing.T) {
assert.Equal(t, "path is required (use the chat-specific absolute plan path)", resp.Content)
})
t.Run("RejectsNonMarkdownPath", func(t *testing.T) {
t.Run("WhitespaceOnlyPathReturnsError", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
storeFile, _ := fakeStoreFile(t)
tool := newProposePlanToolWithPlanPath(t, mockConn, storeFile, nil, false)
tool := newProposePlanTool(t, mockConn, storeFile)
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "propose_plan",
Input: `{"path":"/home/coder/.coder/plans/PLAN-chat.txt"}`,
Input: `{"path":" "}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.Equal(t, "path is required (use the chat-specific absolute plan path)", resp.Content)
})
t.Run("NonMdPathReturnsError", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
storeFile, _ := fakeStoreFile(t)
tool := newProposePlanTool(t, mockConn, storeFile)
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "propose_plan",
Input: `{"path":"/home/coder/plan.txt"}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.Equal(t, "path must end with .md", resp.Content)
})
t.Run("RelativePlanPathReturnsError", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
storeFile, _ := fakeStoreFile(t)
resolvePlanPathCalled := false
tool := newProposePlanToolWithPlanPath(
t,
mockConn,
storeFile,
func(context.Context) (string, string, error) {
resolvePlanPathCalled = true
return "/home/coder/.coder/plans/PLAN-chat.md", "/home/coder", nil
},
)
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "propose_plan",
Input: `{"path":"plan.md"}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.False(t, resolvePlanPathCalled)
assert.Equal(t, relativePlanPathMessage(), resp.Content)
})
t.Run("OversizedFileRejected", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
largeContent := strings.Repeat("x", 32*1024+1)
mockConn.EXPECT().
ReadFile(gomock.Any(), "/home/coder/PLAN.md", int64(0), int64(32*1024+1)).
Return(io.NopCloser(strings.NewReader(largeContent)), "text/markdown", nil)
storeFile, _ := fakeStoreFile(t)
tool := newProposePlanTool(t, mockConn, storeFile)
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "propose_plan",
Input: `{"path":"/home/coder/PLAN.md"}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.Equal(t, "plan file exceeds 32 KiB size limit", resp.Content)
})
t.Run("ExactBoundaryFileSucceeds", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
content := strings.Repeat("x", 32*1024)
mockConn.EXPECT().
ReadFile(gomock.Any(), "/home/coder/PLAN.md", int64(0), int64(32*1024+1)).
Return(io.NopCloser(strings.NewReader(content)), "text/markdown", nil)
storeFile, _ := fakeStoreFile(t)
tool := newProposePlanTool(t, mockConn, storeFile)
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "propose_plan",
Input: `{"path":"/home/coder/PLAN.md"}`,
})
require.NoError(t, err)
assert.False(t, resp.IsError)
})
t.Run("ValidPlanReadsFile", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
mockConn.EXPECT().
ReadFile(gomock.Any(), "/home/coder/docs/PLAN.md", int64(0), int64(32*1024+1)).
Return(io.NopCloser(strings.NewReader("# Plan\n\nContent")), "text/markdown", nil)
storeFile, stored := fakeStoreFile(t)
planPathCalled := false
tool := newProposePlanToolWithPlanPath(
t,
mockConn,
storeFile,
func(context.Context) (string, string, error) {
planPathCalled = true
return "/home/coder/.coder/plans/PLAN-xxx.md", "/home/coder", nil
},
)
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "propose_plan",
Input: `{"path":"/home/coder/docs/PLAN.md"}`,
})
require.NoError(t, err)
assert.False(t, resp.IsError)
assert.True(t, planPathCalled)
result := decodeProposePlanResponse(t, resp)
assert.True(t, result.OK)
assert.Equal(t, "/home/coder/docs/PLAN.md", result.Path)
assert.Equal(t, "plan", result.Kind)
assert.Equal(t, "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", result.FileID)
assert.Equal(t, "text/markdown", result.MediaType)
assert.Equal(t, []byte("# Plan\n\nContent"), *stored)
assert.NotContains(t, resp.Content, "content")
attachments, err := chattool.AttachmentsFromMetadata(resp.Metadata)
require.NoError(t, err)
require.Len(t, attachments, 1)
assert.Equal(t, uuid.MustParse(result.FileID), attachments[0].FileID)
assert.Equal(t, result.MediaType, attachments[0].MediaType)
assert.Equal(t, filepath.Base(result.Path), attachments[0].Name)
})
t.Run("NestedPlanPathUnderHomeIsAllowed", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
mockConn.EXPECT().
ReadFile(gomock.Any(), "/home/coder/myproject/plan.md", int64(0), int64(32*1024+1)).
Return(io.NopCloser(strings.NewReader("# Nested Plan")), "text/markdown", nil)
storeFile, stored := fakeStoreFile(t)
planPathCalled := false
tool := newProposePlanToolWithPlanPath(
t,
mockConn,
storeFile,
func(context.Context) (string, string, error) {
planPathCalled = true
return "/home/coder/.coder/plans/PLAN-chat.md", "/home/coder", nil
},
)
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "propose_plan",
Input: `{"path":"/home/coder/myproject/plan.md"}`,
})
require.NoError(t, err)
assert.False(t, resp.IsError)
assert.True(t, planPathCalled)
result := decodeProposePlanResponse(t, resp)
assert.True(t, result.OK)
assert.Equal(t, "/home/coder/myproject/plan.md", result.Path)
assert.Equal(t, []byte("# Nested Plan"), *stored)
})
t.Run("FileNotFound", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
mockConn.EXPECT().
ReadFile(gomock.Any(), "/home/coder/PLAN.md", int64(0), int64(32*1024+1)).
Return(nil, "", xerrors.New("file not found"))
storeFile, _ := fakeStoreFile(t)
tool := newProposePlanTool(t, mockConn, storeFile)
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "propose_plan",
Input: `{"path":"/home/coder/PLAN.md"}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.Contains(t, resp.Content, "file not found")
})
t.Run("ReadFileError", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
mockConn.EXPECT().
ReadFile(gomock.Any(), "/home/coder/PLAN.md", int64(0), int64(32*1024+1)).
Return(nil, "", xerrors.New("read failed"))
storeFile, _ := fakeStoreFile(t)
tool := newProposePlanTool(t, mockConn, storeFile)
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "propose_plan",
Input: `{"path":"/home/coder/PLAN.md"}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.Equal(t, "read failed", resp.Content)
})
t.Run("ReadAllError", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
mockConn.EXPECT().
ReadFile(gomock.Any(), "/home/coder/PLAN.md", int64(0), int64(32*1024+1)).
Return(io.NopCloser(iotest.ErrReader(xerrors.New("connection reset"))), "text/markdown", nil)
storeFile, _ := fakeStoreFile(t)
tool := newProposePlanTool(t, mockConn, storeFile)
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "propose_plan",
Input: `{"path":"/home/coder/PLAN.md"}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.Contains(t, resp.Content, "connection reset")
})
t.Run("StoreFileError", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
mockConn.EXPECT().
ReadFile(gomock.Any(), "/home/coder/PLAN.md", int64(0), int64(32*1024+1)).
Return(io.NopCloser(strings.NewReader("# Plan")), "text/markdown", nil)
tool := newProposePlanTool(t, mockConn, func(_ context.Context, _ string, _ string, _ []byte) (chattool.AttachmentMetadata, error) {
return chattool.AttachmentMetadata{}, xerrors.New("storage unavailable")
})
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "propose_plan",
Input: `{"path":"/home/coder/PLAN.md"}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.Equal(t, "failed to store plan file: storage unavailable", resp.Content)
})
t.Run("RejectsSharedPlanPathWithResolvedPath", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
storeFile, _ := fakeStoreFile(t)
tool := newProposePlanToolWithPlanPath(
t,
mockConn,
storeFile,
func(context.Context) (string, string, error) {
return "/home/coder/.coder/plans/PLAN-chat.md", "/home/coder", nil
},
)
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "propose_plan",
Input: `{"path":"` + chattool.LegacySharedPlanPath + `"}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.Equal(
t,
sharedPlanPathResolvedMessage(chattool.LegacySharedPlanPath, "/home/coder/.coder/plans/PLAN-chat.md"),
resp.Content,
)
})
t.Run("RejectsSharedPlanPathWhenResolverFails", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
storeFile, _ := fakeStoreFile(t)
tool := newProposePlanToolWithPlanPath(
t,
mockConn,
storeFile,
func(context.Context) (string, string, error) {
return "", "", xerrors.New("workspace unavailable")
},
)
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "propose_plan",
Input: `{"path":"` + chattool.LegacySharedPlanPath + `"}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.Equal(t, planPathVerificationMessage(chattool.LegacySharedPlanPath), resp.Content)
})
t.Run("PerChatPlanPathIsAllowed", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
chatPlanPath := "/home/coder/.coder/plans/PLAN-123e4567-e89b-12d3-a456-426614174000.md"
mockConn.EXPECT().
ReadFile(gomock.Any(), chatPlanPath, int64(0), int64(32*1024+1)).
Return(io.NopCloser(strings.NewReader("# Per-Chat Plan")), "text/markdown", nil)
storeFile, stored := fakeStoreFile(t)
resolvePlanPathCalled := false
tool := newProposePlanToolWithPlanPath(
t,
mockConn,
storeFile,
func(context.Context) (string, string, error) {
resolvePlanPathCalled = true
return chatPlanPath, "/home/coder", nil
},
)
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "propose_plan",
Input: `{"path":"` + chatPlanPath + `"}`,
})
require.NoError(t, err)
assert.False(t, resp.IsError)
assert.False(t, resolvePlanPathCalled)
result := decodeProposePlanResponse(t, resp)
assert.True(t, result.OK)
assert.Equal(t, chatPlanPath, result.Path)
assert.Equal(t, []byte("# Per-Chat Plan"), *stored)
})
t.Run("NestedPlanPathAllowedWhenResolverFails", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
mockConn.EXPECT().
ReadFile(gomock.Any(), "/home/coder/myproject/plan.md", int64(0), int64(32*1024+1)).
Return(io.NopCloser(strings.NewReader("# Nested Plan")), "text/markdown", nil)
storeFile, stored := fakeStoreFile(t)
tool := newProposePlanToolWithPlanPath(
t,
mockConn,
storeFile,
func(context.Context) (string, string, error) {
return "", "", xerrors.New("workspace unavailable")
},
)
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "propose_plan",
Input: `{"path":"/home/coder/myproject/plan.md"}`,
})
require.NoError(t, err)
assert.False(t, resp.IsError)
result := decodeProposePlanResponse(t, resp)
assert.True(t, result.OK)
assert.Equal(t, "/home/coder/myproject/plan.md", result.Path)
assert.Equal(t, []byte("# Nested Plan"), *stored)
})
t.Run("PlanTurnDefaultsEmptyPathToResolvedPath", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
@@ -126,27 +504,6 @@ func TestProposePlan(t *testing.T) {
assert.True(t, resp.IsError)
assert.Equal(t, "during plan turns, propose_plan path must be "+chatPlanPath, resp.Content)
})
t.Run("RejectsReadFileErrors", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
chatPlanPath := "/home/coder/.coder/plans/PLAN-chat.md"
mockConn.EXPECT().
ReadFile(gomock.Any(), chatPlanPath, int64(0), int64(32*1024+1)).
Return(nil, "", xerrors.New("read failed"))
storeFile, _ := fakeStoreFile(t)
tool := newProposePlanToolWithPlanPath(t, mockConn, storeFile, nil, false)
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "propose_plan",
Input: `{"path":"` + chatPlanPath + `"}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.Equal(t, "read failed", resp.Content)
})
t.Run("PlanTurnRejectsEmptyPlan", func(t *testing.T) {
t.Parallel()
@@ -163,9 +520,9 @@ func TestProposePlan(t *testing.T) {
tool := newProposePlanToolWithPlanPath(
t,
mockConn,
func(ctx context.Context, name string, mediaType string, data []byte) (uuid.UUID, error) {
func(ctx context.Context, name string, detectName string, data []byte) (chattool.AttachmentMetadata, error) {
storeCalled = true
return storeFile(ctx, name, mediaType, data)
return storeFile(ctx, name, detectName, data)
},
func(context.Context) (string, string, error) {
return chatPlanPath, "/home/coder", nil
@@ -185,97 +542,106 @@ func TestProposePlan(t *testing.T) {
assert.Nil(t, *stored)
})
t.Run("RejectsOversizedPlan", func(t *testing.T) {
t.Run("WorkspaceConnectionError", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
chatPlanPath := "/home/coder/.coder/plans/PLAN-chat.md"
mockConn.EXPECT().
ReadFile(gomock.Any(), chatPlanPath, int64(0), int64(32*1024+1)).
Return(io.NopCloser(strings.NewReader(strings.Repeat("x", 32*1024+1))), "text/markdown", nil)
storeFile, stored := fakeStoreFile(t)
storeCalled := false
tool := newProposePlanToolWithPlanPath(
t,
mockConn,
func(ctx context.Context, name string, mediaType string, data []byte) (uuid.UUID, error) {
storeCalled = true
return storeFile(ctx, name, mediaType, data)
storeFile, _ := fakeStoreFile(t)
tool := chattool.ProposePlan(chattool.ProposePlanOptions{
GetWorkspaceConn: func(context.Context) (workspacesdk.AgentConn, error) {
return nil, xerrors.New("connection failed")
},
nil,
false,
)
StoreFile: storeFile,
})
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "propose_plan",
Input: `{"path":"` + chatPlanPath + `"}`,
Input: `{"path":"/home/coder/PLAN.md"}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.Equal(t, "plan file exceeds 32 KiB size limit", resp.Content)
assert.False(t, storeCalled)
assert.Nil(t, *stored)
assert.Contains(t, resp.Content, "connection failed")
})
t.Run("PropagatesStoreFileErrors", func(t *testing.T) {
t.Run("NilWorkspaceResolver", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
chatPlanPath := "/home/coder/.coder/plans/PLAN-chat.md"
tool := chattool.ProposePlan(chattool.ProposePlanOptions{})
mockConn.EXPECT().
ReadFile(gomock.Any(), chatPlanPath, int64(0), int64(32*1024+1)).
Return(io.NopCloser(strings.NewReader("# Plan")), "text/markdown", nil)
tool := newProposePlanToolWithPlanPath(
t,
mockConn,
func(context.Context, string, string, []byte) (uuid.UUID, error) {
return uuid.Nil, xerrors.New("store failed")
},
nil,
false,
)
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "propose_plan",
Input: `{"path":"` + chatPlanPath + `"}`,
Input: `{"path":"/home/coder/PLAN.md"}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.Equal(t, "failed to store plan file: store failed", resp.Content)
assert.Contains(t, resp.Content, "workspace connection resolver is not configured")
})
t.Run("NilStoreFile", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
tool := chattool.ProposePlan(chattool.ProposePlanOptions{
GetWorkspaceConn: func(_ context.Context) (workspacesdk.AgentConn, error) {
return mockConn, nil
},
})
resp, err := tool.Run(context.Background(), fantasy.ToolCall{
ID: "call-1",
Name: "propose_plan",
Input: `{"path":"/home/coder/PLAN.md"}`,
})
require.NoError(t, err)
assert.True(t, resp.IsError)
assert.Contains(t, resp.Content, "file storage is not configured")
})
}
func newProposePlanTool(
t *testing.T,
mockConn *agentconnmock.MockAgentConn,
storeFile chattool.StoreFileFunc,
) fantasy.AgentTool {
t.Helper()
return newProposePlanToolWithPlanPath(t, mockConn, storeFile, nil)
}
func newProposePlanToolWithPlanPath(
t *testing.T,
mockConn *agentconnmock.MockAgentConn,
storeFile func(ctx context.Context, name string, mediaType string, data []byte) (uuid.UUID, error),
storeFile chattool.StoreFileFunc,
resolvePlanPath func(context.Context) (string, string, error),
isPlanTurn bool,
isPlanTurn ...bool,
) fantasy.AgentTool {
t.Helper()
enabled := false
if len(isPlanTurn) > 0 {
enabled = isPlanTurn[0]
}
return chattool.ProposePlan(chattool.ProposePlanOptions{
GetWorkspaceConn: func(_ context.Context) (workspacesdk.AgentConn, error) {
return mockConn, nil
},
ResolvePlanPath: resolvePlanPath,
StoreFile: storeFile,
IsPlanTurn: isPlanTurn,
IsPlanTurn: enabled,
})
}
func fakeStoreFile(t *testing.T) (func(ctx context.Context, name string, mediaType string, data []byte) (uuid.UUID, error), *[]byte) {
func fakeStoreFile(t *testing.T) (chattool.StoreFileFunc, *[]byte) {
t.Helper()
var stored []byte
return func(_ context.Context, name string, mediaType string, data []byte) (uuid.UUID, error) {
return func(_ context.Context, name string, detectName string, data []byte) (chattool.AttachmentMetadata, error) {
assert.NotEmpty(t, name)
assert.Equal(t, "text/markdown", mediaType)
assert.NotEmpty(t, detectName)
stored = append([]byte(nil), data...)
return uuid.MustParse("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"), nil
return chattool.AttachmentMetadata{
FileID: uuid.MustParse("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"),
MediaType: "text/markdown",
Name: name,
}, nil
}, &stored
}
+102 -87
View File
@@ -9,12 +9,12 @@ import (
"mime/multipart"
"github.com/google/uuid"
"golang.org/x/xerrors"
"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/codersdk"
"github.com/coder/coder/v2/coderd/x/chatd/chattool"
"github.com/coder/coder/v2/codersdk/workspacesdk"
)
@@ -33,24 +33,40 @@ func (p *Server) stopAndStoreRecording(
ctx context.Context,
conn workspacesdk.AgentConn,
recordingID string,
parentChatID uuid.UUID,
ownerID uuid.UUID,
workspaceID uuid.NullUUID,
chatID uuid.UUID,
) recordingResult {
var result recordingResult
workspaceIDValue := ""
if workspaceID.Valid {
workspaceIDValue = workspaceID.UUID.String()
}
recordingWarnFields := []slog.Field{
slog.F("recording_id", recordingID),
slog.F("parent_chat_id", parentChatID.String()),
slog.F("workspace_id", workspaceIDValue),
}
warn := func(msg string, fields ...slog.Field) {
allFields := make([]slog.Field, 0, len(recordingWarnFields)+len(fields))
allFields = append(allFields, recordingWarnFields...)
allFields = append(allFields, fields...)
p.logger.Warn(ctx, msg, allFields...)
}
select {
case p.recordingSem <- struct{}{}:
defer func() { <-p.recordingSem }()
case <-ctx.Done():
p.logger.Warn(ctx, "context canceled waiting for recording semaphore", slog.Error(ctx.Err()))
warn("context canceled waiting for recording semaphore", slog.Error(ctx.Err()))
return result
}
resp, err := conn.StopDesktopRecording(ctx,
workspacesdk.StopDesktopRecordingRequest{RecordingID: recordingID})
if err != nil {
p.logger.Warn(ctx, "failed to stop desktop recording",
warn("failed to stop desktop recording",
slog.Error(err))
return result
}
@@ -58,29 +74,30 @@ func (p *Server) stopAndStoreRecording(
_, params, err := mime.ParseMediaType(resp.ContentType)
if err != nil {
p.logger.Warn(ctx, "failed to parse content type from recording response",
warn("failed to parse content type from recording response",
slog.F("content_type", resp.ContentType),
slog.Error(err))
return result
}
boundary := params["boundary"]
if boundary == "" {
p.logger.Warn(ctx, "missing boundary in recording response content type",
warn("missing boundary in recording response content type",
slog.F("content_type", resp.ContentType))
return result
}
if !workspaceID.Valid {
p.logger.Warn(ctx, "chat has no workspace, cannot store recording")
warn("chat has no workspace, cannot store recording")
return result
}
// The chatd actor is used here because the recording is stored on
// behalf of the chat system, not a specific user request.
//nolint:gocritic // AsChatd is required to read the workspace for org lookup.
ws, err := p.db.GetWorkspaceByID(dbauthz.AsChatd(ctx), workspaceID.UUID)
chatdCtx := dbauthz.AsChatd(ctx)
ws, err := p.db.GetWorkspaceByID(chatdCtx, workspaceID.UUID)
if err != nil {
p.logger.Warn(ctx, "failed to resolve workspace for recording",
warn("failed to resolve workspace for recording",
slog.Error(err))
return result
}
@@ -99,7 +116,7 @@ func (p *Server) stopAndStoreRecording(
var videoData, thumbnailData []byte
for range maxParts {
if ctx.Err() != nil {
p.logger.Warn(ctx, "context canceled while reading recording parts", slog.Error(ctx.Err()))
warn("context canceled while reading recording parts", slog.Error(ctx.Err()))
break
}
@@ -108,7 +125,7 @@ func (p *Server) stopAndStoreRecording(
break
}
if err != nil {
p.logger.Warn(ctx, "error reading next multipart part", slog.Error(err))
warn("error reading next multipart part", slog.Error(err))
break
}
@@ -129,20 +146,20 @@ func (p *Server) stopAndStoreRecording(
data, err := io.ReadAll(io.LimitReader(part, maxSize+1))
if err != nil {
p.logger.Warn(ctx, "failed to read recording part data",
warn("failed to read recording part data",
slog.F("content_type", contentType),
slog.Error(err))
continue
}
if int64(len(data)) > maxSize {
p.logger.Warn(ctx, "recording part exceeds maximum size, skipping",
warn("recording part exceeds maximum size, skipping",
slog.F("content_type", contentType),
slog.F("size", len(data)),
slog.F("max_size", maxSize))
continue
}
if len(data) == 0 {
p.logger.Warn(ctx, "recording part is empty, skipping",
warn("recording part is empty, skipping",
slog.F("content_type", contentType))
continue
}
@@ -150,13 +167,13 @@ func (p *Server) stopAndStoreRecording(
switch contentType {
case "video/mp4":
if videoData != nil {
p.logger.Warn(ctx, "duplicate video/mp4 part in recording response, skipping")
warn("duplicate video/mp4 part in recording response, skipping")
continue
}
videoData = data
case "image/jpeg":
if thumbnailData != nil {
p.logger.Warn(ctx, "duplicate image/jpeg part in recording response, skipping")
warn("duplicate image/jpeg part in recording response, skipping")
continue
}
thumbnailData = data
@@ -166,78 +183,76 @@ func (p *Server) stopAndStoreRecording(
}
}
// Second pass: store the collected data in the database and
// link it to the parent chat atomically. Insert + link must
// happen in the same transaction so that we never end up with
// chat_files rows that lack chat_file_links entries (which the
// purge job would treat as orphans and which users cannot view).
// Errors inside the transaction cause both inserts to be rolled
// back, so recording remains best-effort end-to-end.
txResult := struct {
recordingFileID string
thumbnailFileID string
}{}
//nolint:gocritic // AsChatd is required to insert and link chat files from the recording pipeline.
txErr := p.db.InTx(func(tx database.Store) error {
var fileIDs []uuid.UUID
if videoData != nil {
row, err := tx.InsertChatFile(dbauthz.AsChatd(ctx), database.InsertChatFileParams{
OwnerID: ownerID,
OrganizationID: ws.OrganizationID,
Name: fmt.Sprintf("recording-%s.mp4", p.clock.Now().UTC().Format("2006-01-02T15-04-05Z")),
Mimetype: "video/mp4",
Data: videoData,
})
if err != nil {
return xerrors.Errorf("insert recording: %w", err)
}
txResult.recordingFileID = row.ID.String()
fileIDs = append(fileIDs, row.ID)
}
if thumbnailData != nil && txResult.recordingFileID != "" {
row, err := tx.InsertChatFile(dbauthz.AsChatd(ctx), database.InsertChatFileParams{
OwnerID: ownerID,
OrganizationID: ws.OrganizationID,
Name: fmt.Sprintf("thumbnail-%s.jpg", p.clock.Now().UTC().Format("2006-01-02T15-04-05Z")),
Mimetype: "image/jpeg",
Data: thumbnailData,
})
if err != nil {
return xerrors.Errorf("insert thumbnail: %w", err)
}
txResult.thumbnailFileID = row.ID.String()
fileIDs = append(fileIDs, row.ID)
}
if len(fileIDs) == 0 {
return nil
}
rejected, err := tx.LinkChatFiles(dbauthz.AsChatd(ctx), database.LinkChatFilesParams{
ChatID: chatID,
MaxFileLinks: int32(codersdk.MaxChatFileIDs),
FileIds: fileIDs,
})
// Second pass: store the collected data in the database.
if videoData != nil {
attachment, err := p.storeRecordingArtifact(
chatdCtx,
parentChatID,
ownerID,
ws.OrganizationID,
fmt.Sprintf("recording-%s.mp4", p.clock.Now().UTC().Format("2006-01-02T15-04-05Z")),
"video/mp4",
videoData,
)
if err != nil {
return xerrors.Errorf("link recording files: %w", err)
warn("failed to store recording in database",
slog.Error(err))
} else {
result.recordingFileID = attachment.FileID.String()
}
if rejected > 0 {
// The cap would be exceeded. Rolling back ensures the
// files are not persisted as orphans. MaxChatFileIDs is
// 20 today; hitting the cap with a 1-2 file batch means
// the chat is already saturated, and silently dropping
// the recording is preferable to leaving an unreachable
// blob.
return xerrors.Errorf("chat file link cap exceeded: %d file(s) rejected", rejected)
}
return nil
}, nil)
if txErr != nil {
p.logger.Warn(ctx, "failed to store and link recording",
slog.F("chat_id", chatID),
slog.Error(txErr))
return result
}
result.recordingFileID = txResult.recordingFileID
result.thumbnailFileID = txResult.thumbnailFileID
if thumbnailData != nil && result.recordingFileID != "" {
attachment, err := p.storeRecordingArtifact(
chatdCtx,
parentChatID,
ownerID,
ws.OrganizationID,
fmt.Sprintf("thumbnail-%s.jpg", p.clock.Now().UTC().Format("2006-01-02T15-04-05Z")),
"image/jpeg",
thumbnailData,
)
if err != nil {
warn("failed to store thumbnail in database",
slog.Error(err))
} else {
result.thumbnailFileID = attachment.FileID.String()
}
}
return result
}
func (p *Server) storeRecordingArtifact(
ctx context.Context,
chatID uuid.UUID,
ownerID uuid.UUID,
organizationID uuid.UUID,
name string,
mediaType string,
data []byte,
) (chattool.AttachmentMetadata, error) {
storedName, verifiedMediaType, err := chatfiles.PrepareRecordingArtifact(name, mediaType, data)
if err != nil {
return chattool.AttachmentMetadata{}, err
}
var attachment chattool.AttachmentMetadata
err = p.db.InTx(func(tx database.Store) error {
var err error
attachment, err = storeLinkedChatFileTx(
ctx,
tx,
chatID,
ownerID,
organizationID,
storedName,
verifiedMediaType,
data,
)
return err
}, database.DefaultTXOptions().WithID("store_recording_artifact"))
if err != nil {
return chattool.AttachmentMetadata{}, err
}
return attachment, nil
}
+160 -223
View File
@@ -62,6 +62,22 @@ func buildMultipartResponse(parts ...partSpec) workspacesdk.StopDesktopRecording
}
}
func validRecordingMP4(extra int, fill byte) []byte {
data := []byte{0x00, 0x00, 0x00, 0x18, 'f', 't', 'y', 'p', 'm', 'p', '4', '2', 0x00, 0x00, 0x00, 0x00, 'm', 'p', '4', '1', 'i', 's', 'o', 'm'}
if extra <= 0 {
return data
}
return append(data, bytes.Repeat([]byte{fill}, extra)...)
}
func validRecordingJPEG(extra int, fill byte) []byte {
data := []byte{0xFF, 0xD8, 0xFF, 0xE0, 0x00, 0x10, 'J', 'F', 'I', 'F', 0x00, 0x01, 0x01, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00}
if extra <= 0 {
return data
}
return append(data, bytes.Repeat([]byte{fill}, extra)...)
}
// createComputerUseParentChild creates a parent chat and a
// computer_use child chat bound to the given workspace/agent.
// Both chats are inserted directly via DB to avoid triggering
@@ -191,7 +207,7 @@ func TestWaitAgentComputerUseRecording(t *testing.T) {
setChatStatus(ctx, t, db, child.ID, database.ChatStatusWaiting, "")
// Set up mock expectations for start and stop.
fakeMp4 := []byte("fake-mp4-data-for-recording-test")
fakeMp4 := validRecordingMP4(32, 0xA1)
mockConn.EXPECT().
StartDesktopRecording(gomock.Any(), gomock.Any()).
@@ -229,10 +245,14 @@ func TestWaitAgentComputerUseRecording(t *testing.T) {
assert.Equal(t, user.ID, chatFile.OwnerID)
assert.Equal(t, fakeMp4, chatFile.Data)
// Verify the file is linked to the parent chat.
linkedFiles, err := db.GetChatFileMetadataByChatID(ctx, parent.ID)
parentFiles, err := db.GetChatFileMetadataByChatID(ctx, parent.ID)
require.NoError(t, err)
assert.Len(t, linkedFiles, 1)
require.Len(t, parentFiles, 1)
assert.Equal(t, fileUUID, parentFiles[0].ID)
childFiles, err := db.GetChatFileMetadataByChatID(ctx, child.ID)
require.NoError(t, err)
assert.Empty(t, childFiles)
}
// TestWaitAgentComputerUseRecordingWithThumbnail verifies the
@@ -268,8 +288,8 @@ func TestWaitAgentComputerUseRecordingWithThumbnail(t *testing.T) {
setChatStatus(ctx, t, db, child.ID, database.ChatStatusWaiting, "")
fakeMp4 := []byte("fake-mp4-data-with-thumbnail-test")
fakeThumb := []byte("fake-jpeg-thumbnail-data")
fakeMp4 := validRecordingMP4(48, 0xA2)
fakeThumb := validRecordingJPEG(32, 0xB1)
mockConn.EXPECT().
StartDesktopRecording(gomock.Any(), gomock.Any()).
@@ -315,10 +335,15 @@ func TestWaitAgentComputerUseRecordingWithThumbnail(t *testing.T) {
assert.Equal(t, "image/jpeg", thumbFile.Mimetype)
assert.Equal(t, fakeThumb, thumbFile.Data)
// Verify both files are linked to the parent chat.
linkedFiles, err := db.GetChatFileMetadataByChatID(ctx, parent.ID)
parentFiles, err := db.GetChatFileMetadataByChatID(ctx, parent.ID)
require.NoError(t, err)
assert.Len(t, linkedFiles, 2)
require.Len(t, parentFiles, 2)
assert.Equal(t, fileUUID, parentFiles[0].ID)
assert.Equal(t, thumbUUID, parentFiles[1].ID)
childFiles, err := db.GetChatFileMetadataByChatID(ctx, child.ID)
require.NoError(t, err)
assert.Empty(t, childFiles)
}
// TestWaitAgentNonComputerUseNoRecording verifies that when the
@@ -581,10 +606,11 @@ func TestStopAndStoreRecording_Oversized(t *testing.T) {
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
user, _, _ := seedInternalChatDeps(ctx, t, db)
user, org, model := seedInternalChatDeps(ctx, t, db)
workspace, _, _ := seedWorkspaceBinding(t, db, user.ID)
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{})
parent, _ := createParentChildChats(ctx, t, server, user, org, model)
// Build a streaming multipart response with a video/mp4 part
// that exceeds MaxRecordingSize without allocating the full
@@ -611,9 +637,8 @@ func TestStopAndStoreRecording_Oversized(t *testing.T) {
recordingID := uuid.New().String()
result := server.stopAndStoreRecording(
ctx, mockConn, recordingID, user.ID,
ctx, mockConn, recordingID, parent.ID, user.ID,
uuid.NullUUID{UUID: workspace.ID, Valid: true},
uuid.Nil,
)
assert.Empty(t, result.recordingFileID, "oversized recording should not be stored")
}
@@ -631,23 +656,12 @@ func TestStopAndStoreRecording_OversizedThumbnail(t *testing.T) {
mockConn := agentconnmock.NewMockAgentConn(ctrl)
user, org, model := seedInternalChatDeps(ctx, t, db)
workspace, _, agent := seedWorkspaceBinding(t, db, user.ID)
workspace, _, _ := seedWorkspaceBinding(t, db, user.ID)
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{})
parent, _ := createParentChildChats(ctx, t, server, user, org, model)
chat, err := db.InsertChat(ctx, database.InsertChatParams{
OrganizationID: org.ID,
OwnerID: user.ID,
WorkspaceID: uuid.NullUUID{UUID: workspace.ID, Valid: true},
AgentID: uuid.NullUUID{UUID: agent.ID, Valid: true},
LastModelConfigID: model.ID,
Title: "test-recording",
Status: database.ChatStatusPending,
ClientType: database.ChatClientTypeUi,
})
require.NoError(t, err)
videoData := bytes.Repeat([]byte{0xAA}, 1024)
videoData := validRecordingMP4(1024, 0xAA)
// Build a streaming multipart response with a normal video part
// and an oversized thumbnail part.
@@ -677,9 +691,8 @@ func TestStopAndStoreRecording_OversizedThumbnail(t *testing.T) {
recordingID := uuid.New().String()
result := server.stopAndStoreRecording(
ctx, mockConn, recordingID, user.ID,
ctx, mockConn, recordingID, parent.ID, user.ID,
uuid.NullUUID{UUID: workspace.ID, Valid: true},
chat.ID,
)
// Video should be stored.
@@ -692,11 +705,6 @@ func TestStopAndStoreRecording_OversizedThumbnail(t *testing.T) {
// Thumbnail should be skipped (oversized).
assert.Empty(t, result.thumbnailFileID, "oversized thumbnail should not be stored")
// Verify that the stored files are linked to the chat.
linkedFiles, err := db.GetChatFileMetadataByChatID(ctx, chat.ID)
require.NoError(t, err)
assert.Len(t, linkedFiles, 1)
}
// TestStopAndStoreRecording_DuplicatePartsIgnored verifies that when
@@ -712,24 +720,13 @@ func TestStopAndStoreRecording_DuplicatePartsIgnored(t *testing.T) {
mockConn := agentconnmock.NewMockAgentConn(ctrl)
user, org, model := seedInternalChatDeps(ctx, t, db)
workspace, _, agent := seedWorkspaceBinding(t, db, user.ID)
workspace, _, _ := seedWorkspaceBinding(t, db, user.ID)
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{})
parent, _ := createParentChildChats(ctx, t, server, user, org, model)
chat, err := db.InsertChat(ctx, database.InsertChatParams{
OrganizationID: org.ID,
OwnerID: user.ID,
WorkspaceID: uuid.NullUUID{UUID: workspace.ID, Valid: true},
AgentID: uuid.NullUUID{UUID: agent.ID, Valid: true},
LastModelConfigID: model.ID,
Title: "test-recording",
Status: database.ChatStatusPending,
ClientType: database.ChatClientTypeUi,
})
require.NoError(t, err)
firstVideo := bytes.Repeat([]byte{0x01}, 512)
secondVideo := bytes.Repeat([]byte{0x02}, 512)
firstVideo := validRecordingMP4(512, 0x01)
secondVideo := validRecordingMP4(512, 0x02)
mockConn.EXPECT().
StopDesktopRecording(gomock.Any(), gomock.Any()).
@@ -741,9 +738,8 @@ func TestStopAndStoreRecording_DuplicatePartsIgnored(t *testing.T) {
recordingID := uuid.New().String()
result := server.stopAndStoreRecording(
ctx, mockConn, recordingID, user.ID,
ctx, mockConn, recordingID, parent.ID, user.ID,
uuid.NullUUID{UUID: workspace.ID, Valid: true},
chat.ID,
)
// Only the first video part should be stored.
@@ -752,11 +748,6 @@ func TestStopAndStoreRecording_DuplicatePartsIgnored(t *testing.T) {
recFile, err := db.GetChatFileByID(ctx, recUUID)
require.NoError(t, err)
assert.Equal(t, firstVideo, recFile.Data, "first video part should be stored, not the duplicate")
// Verify that the stored files are linked to the chat.
linkedFiles, err := db.GetChatFileMetadataByChatID(ctx, chat.ID)
require.NoError(t, err)
assert.Len(t, linkedFiles, 1)
}
// TestStopAndStoreRecording_Empty verifies that when the recording
@@ -771,10 +762,11 @@ func TestStopAndStoreRecording_Empty(t *testing.T) {
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
user, _, _ := seedInternalChatDeps(ctx, t, db)
user, org, model := seedInternalChatDeps(ctx, t, db)
workspace, _, _ := seedWorkspaceBinding(t, db, user.ID)
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{})
parent, _ := createParentChildChats(ctx, t, server, user, org, model)
// Build a multipart response with an empty video/mp4 part.
mockConn.EXPECT().
@@ -783,13 +775,66 @@ func TestStopAndStoreRecording_Empty(t *testing.T) {
recordingID := uuid.New().String()
result := server.stopAndStoreRecording(
ctx, mockConn, recordingID, user.ID,
ctx, mockConn, recordingID, parent.ID, user.ID,
uuid.NullUUID{UUID: workspace.ID, Valid: true},
uuid.Nil,
)
assert.Empty(t, result.recordingFileID, "empty recording should not be stored")
}
// TestStopAndStoreRecording_LinkFailureRollsBackInsert verifies that a
// chat-file cap rejection does not leave behind an unlinked recording row.
func TestStopAndStoreRecording_LinkFailureRollsBackInsert(t *testing.T) {
t.Parallel()
db, ps, sqlDB := dbtestutil.NewDBWithSQLDB(t)
ctx := chatdTestContext(t)
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
user, org, model := seedInternalChatDeps(ctx, t, db)
workspace, _, _ := seedWorkspaceBinding(t, db, user.ID)
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{})
parent, _ := createParentChildChats(ctx, t, server, user, org, model)
for i := range codersdk.MaxChatFileIDs {
insertLinkedChatFile(
ctx,
t,
db,
parent.ID,
user.ID,
workspace.OrganizationID,
fmt.Sprintf("existing-%02d.txt", i),
"text/plain",
[]byte("existing"),
)
}
var beforeCount int
require.NoError(t, sqlDB.QueryRowContext(ctx, "SELECT COUNT(*) FROM chat_files").Scan(&beforeCount))
videoData := validRecordingMP4(1000, 0xDE)
mockConn.EXPECT().
StopDesktopRecording(gomock.Any(), gomock.Any()).
Return(buildMultipartResponse(partSpec{"video/mp4", videoData}), nil).
Times(1)
recordingID := uuid.New().String()
result := server.stopAndStoreRecording(
ctx, mockConn, recordingID, parent.ID, user.ID,
uuid.NullUUID{UUID: workspace.ID, Valid: true},
)
assert.Empty(t, result.recordingFileID)
assert.Empty(t, result.thumbnailFileID)
var afterCount int
require.NoError(t, sqlDB.QueryRowContext(ctx, "SELECT COUNT(*) FROM chat_files").Scan(&afterCount))
assert.Equal(t, beforeCount, afterCount)
}
// TestStopAndStoreRecording_WithThumbnail verifies that a multipart
// response containing both a video/mp4 part and an image/jpeg part
// results in both files being stored with correct mimetypes.
@@ -803,24 +848,13 @@ func TestStopAndStoreRecording_WithThumbnail(t *testing.T) {
mockConn := agentconnmock.NewMockAgentConn(ctrl)
user, org, model := seedInternalChatDeps(ctx, t, db)
workspace, _, agent := seedWorkspaceBinding(t, db, user.ID)
workspace, _, _ := seedWorkspaceBinding(t, db, user.ID)
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{})
parent, _ := createParentChildChats(ctx, t, server, user, org, model)
chat, err := db.InsertChat(ctx, database.InsertChatParams{
OrganizationID: org.ID,
OwnerID: user.ID,
WorkspaceID: uuid.NullUUID{UUID: workspace.ID, Valid: true},
AgentID: uuid.NullUUID{UUID: agent.ID, Valid: true},
LastModelConfigID: model.ID,
Title: "test-recording",
Status: database.ChatStatusPending,
ClientType: database.ChatClientTypeUi,
})
require.NoError(t, err)
videoData := bytes.Repeat([]byte{0xDE, 0xAD}, 512) // 1024 bytes
thumbData := bytes.Repeat([]byte{0xFF, 0xD8}, 256) // 512 bytes
videoData := validRecordingMP4(1000, 0xDE)
thumbData := validRecordingJPEG(492, 0xD8)
mockConn.EXPECT().
StopDesktopRecording(gomock.Any(), gomock.Any()).
@@ -832,9 +866,8 @@ func TestStopAndStoreRecording_WithThumbnail(t *testing.T) {
recordingID := uuid.New().String()
result := server.stopAndStoreRecording(
ctx, mockConn, recordingID, user.ID,
ctx, mockConn, recordingID, parent.ID, user.ID,
uuid.NullUUID{UUID: workspace.ID, Valid: true},
chat.ID,
)
// Both file IDs should be valid UUIDs.
@@ -854,104 +887,6 @@ func TestStopAndStoreRecording_WithThumbnail(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, "image/jpeg", thumbFile.Mimetype)
assert.Equal(t, thumbData, thumbFile.Data)
// Verify that the stored files are linked to the chat.
linkedFiles, err := db.GetChatFileMetadataByChatID(ctx, chat.ID)
require.NoError(t, err)
assert.Len(t, linkedFiles, 2)
}
// TestStopAndStoreRecording_CapExceededRollback verifies that when
// the chat's file-link cap would be exceeded by the new recording,
// stopAndStoreRecording rolls back the transaction so that the
// chat_files inserts are reverted. This prevents orphaned files
// that would be invisible to users and bypass the purge retention
// logic.
func TestStopAndStoreRecording_CapExceededRollback(t *testing.T) {
t.Parallel()
db, ps := dbtestutil.NewDB(t)
ctx := chatdTestContext(t)
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
user, org, model := seedInternalChatDeps(ctx, t, db)
workspace, _, agent := seedWorkspaceBinding(t, db, user.ID)
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{})
chat, err := db.InsertChat(ctx, database.InsertChatParams{
OrganizationID: org.ID,
OwnerID: user.ID,
WorkspaceID: uuid.NullUUID{UUID: workspace.ID, Valid: true},
AgentID: uuid.NullUUID{UUID: agent.ID, Valid: true},
LastModelConfigID: model.ID,
Title: "test-recording",
Status: database.ChatStatusPending,
ClientType: database.ChatClientTypeUi,
})
require.NoError(t, err)
// Pre-fill the chat with MaxChatFileIDs links so any new
// recording file will be rejected by LinkChatFiles.
preFileIDs := make([]uuid.UUID, codersdk.MaxChatFileIDs)
for i := range preFileIDs {
row, insertErr := db.InsertChatFile(ctx, database.InsertChatFileParams{
OwnerID: user.ID,
OrganizationID: org.ID,
Name: fmt.Sprintf("prefill-%d.bin", i),
Mimetype: "application/octet-stream",
Data: []byte{byte(i)},
})
require.NoError(t, insertErr)
preFileIDs[i] = row.ID
}
rejected, err := db.LinkChatFiles(ctx, database.LinkChatFilesParams{
ChatID: chat.ID,
MaxFileLinks: int32(codersdk.MaxChatFileIDs),
FileIds: preFileIDs,
})
require.NoError(t, err)
require.Zero(t, rejected)
videoData := bytes.Repeat([]byte{0xAB}, 1024)
thumbData := bytes.Repeat([]byte{0xCD}, 512)
mockConn.EXPECT().
StopDesktopRecording(gomock.Any(), gomock.Any()).
Return(buildMultipartResponse(
partSpec{"video/mp4", videoData},
partSpec{"image/jpeg", thumbData},
), nil).
Times(1)
recordingID := uuid.New().String()
result := server.stopAndStoreRecording(
ctx, mockConn, recordingID, user.ID,
uuid.NullUUID{UUID: workspace.ID, Valid: true},
chat.ID,
)
// Both IDs must be empty because the transaction was rolled
// back, so there is no file to reference.
assert.Empty(t, result.recordingFileID,
"recordingFileID must be empty when linking fails")
assert.Empty(t, result.thumbnailFileID,
"thumbnailFileID must be empty when linking fails")
// Only the pre-fill files should exist; the recording and
// thumbnail rows must have been rolled back.
for _, id := range preFileIDs {
_, err := db.GetChatFileByID(ctx, id)
require.NoError(t, err, "prefill file must still exist")
}
// Chat should still have exactly MaxChatFileIDs links; no
// new orphan or link was added.
linked, err := db.GetChatFileMetadataByChatID(ctx, chat.ID)
require.NoError(t, err)
assert.Len(t, linked, codersdk.MaxChatFileIDs)
}
// TestStopAndStoreRecording_VideoOnly verifies that a multipart
@@ -967,23 +902,12 @@ func TestStopAndStoreRecording_VideoOnly(t *testing.T) {
mockConn := agentconnmock.NewMockAgentConn(ctrl)
user, org, model := seedInternalChatDeps(ctx, t, db)
workspace, _, agent := seedWorkspaceBinding(t, db, user.ID)
workspace, _, _ := seedWorkspaceBinding(t, db, user.ID)
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{})
parent, _ := createParentChildChats(ctx, t, server, user, org, model)
chat, err := db.InsertChat(ctx, database.InsertChatParams{
OrganizationID: org.ID,
OwnerID: user.ID,
WorkspaceID: uuid.NullUUID{UUID: workspace.ID, Valid: true},
AgentID: uuid.NullUUID{UUID: agent.ID, Valid: true},
LastModelConfigID: model.ID,
Title: "test-recording",
Status: database.ChatStatusPending,
ClientType: database.ChatClientTypeUi,
})
require.NoError(t, err)
videoData := make([]byte, 1024)
videoData := validRecordingMP4(1000, 0xCC)
mockConn.EXPECT().
StopDesktopRecording(gomock.Any(), gomock.Any()).
@@ -991,9 +915,8 @@ func TestStopAndStoreRecording_VideoOnly(t *testing.T) {
recordingID := uuid.New().String()
result := server.stopAndStoreRecording(
ctx, mockConn, recordingID, user.ID,
ctx, mockConn, recordingID, parent.ID, user.ID,
uuid.NullUUID{UUID: workspace.ID, Valid: true},
chat.ID,
)
// Recording should be stored.
@@ -1007,11 +930,42 @@ func TestStopAndStoreRecording_VideoOnly(t *testing.T) {
// No thumbnail.
assert.Empty(t, result.thumbnailFileID, "ThumbnailFileID should be empty when no thumbnail part is present")
}
// Verify that the stored files are linked to the chat.
linkedFiles, err := db.GetChatFileMetadataByChatID(ctx, chat.ID)
// TestStopAndStoreRecording_MismatchedVideoBytesSkipped verifies that a
// part labeled video/mp4 is skipped when its bytes do not sniff as MP4.
func TestStopAndStoreRecording_MismatchedVideoBytesSkipped(t *testing.T) {
t.Parallel()
db, ps := dbtestutil.NewDB(t)
ctx := chatdTestContext(t)
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
user, org, model := seedInternalChatDeps(ctx, t, db)
workspace, _, _ := seedWorkspaceBinding(t, db, user.ID)
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{})
parent, _ := createParentChildChats(ctx, t, server, user, org, model)
mockConn.EXPECT().
StopDesktopRecording(gomock.Any(), gomock.Any()).
Return(buildMultipartResponse(partSpec{"video/mp4", validRecordingJPEG(32, 0x44)}), nil).
Times(1)
recordingID := uuid.New().String()
result := server.stopAndStoreRecording(
ctx, mockConn, recordingID, parent.ID, user.ID,
uuid.NullUUID{UUID: workspace.ID, Valid: true},
)
assert.Empty(t, result.recordingFileID)
assert.Empty(t, result.thumbnailFileID)
parentFiles, err := db.GetChatFileMetadataByChatID(ctx, parent.ID)
require.NoError(t, err)
assert.Len(t, linkedFiles, 1)
assert.Empty(t, parentFiles)
}
// TestStopAndStoreRecording_DownloadFailure verifies that when
@@ -1026,10 +980,11 @@ func TestStopAndStoreRecording_DownloadFailure(t *testing.T) {
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
user, _, _ := seedInternalChatDeps(ctx, t, db)
user, org, model := seedInternalChatDeps(ctx, t, db)
workspace, _, _ := seedWorkspaceBinding(t, db, user.ID)
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{})
parent, _ := createParentChildChats(ctx, t, server, user, org, model)
mockConn.EXPECT().
StopDesktopRecording(gomock.Any(), gomock.Any()).
@@ -1038,9 +993,8 @@ func TestStopAndStoreRecording_DownloadFailure(t *testing.T) {
recordingID := uuid.New().String()
result := server.stopAndStoreRecording(
ctx, mockConn, recordingID, user.ID,
ctx, mockConn, recordingID, parent.ID, user.ID,
uuid.NullUUID{UUID: workspace.ID, Valid: true},
uuid.Nil,
)
assert.Empty(t, result.recordingFileID, "RecordingFileID should be empty on download failure")
@@ -1060,24 +1014,13 @@ func TestStopAndStoreRecording_UnknownPartIgnored(t *testing.T) {
mockConn := agentconnmock.NewMockAgentConn(ctrl)
user, org, model := seedInternalChatDeps(ctx, t, db)
workspace, _, agent := seedWorkspaceBinding(t, db, user.ID)
workspace, _, _ := seedWorkspaceBinding(t, db, user.ID)
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{})
parent, _ := createParentChildChats(ctx, t, server, user, org, model)
chat, err := db.InsertChat(ctx, database.InsertChatParams{
OrganizationID: org.ID,
OwnerID: user.ID,
WorkspaceID: uuid.NullUUID{UUID: workspace.ID, Valid: true},
AgentID: uuid.NullUUID{UUID: agent.ID, Valid: true},
LastModelConfigID: model.ID,
Title: "test-recording",
Status: database.ChatStatusPending,
ClientType: database.ChatClientTypeUi,
})
require.NoError(t, err)
videoData := make([]byte, 1024)
thumbData := make([]byte, 512)
videoData := validRecordingMP4(1000, 0x11)
thumbData := validRecordingJPEG(492, 0x22)
unknownData := make([]byte, 256)
mockConn.EXPECT().
@@ -1090,9 +1033,8 @@ func TestStopAndStoreRecording_UnknownPartIgnored(t *testing.T) {
recordingID := uuid.New().String()
result := server.stopAndStoreRecording(
ctx, mockConn, recordingID, user.ID,
ctx, mockConn, recordingID, parent.ID, user.ID,
uuid.NullUUID{UUID: workspace.ID, Valid: true},
chat.ID,
)
// Both known parts should be stored.
@@ -1112,11 +1054,6 @@ func TestStopAndStoreRecording_UnknownPartIgnored(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, "image/jpeg", thumbFile.Mimetype)
assert.Equal(t, thumbData, thumbFile.Data)
// Verify that the stored files are linked to the chat.
linkedFiles, err := db.GetChatFileMetadataByChatID(ctx, chat.ID)
require.NoError(t, err)
assert.Len(t, linkedFiles, 2)
}
// TestStopAndStoreRecording_MalformedContentType verifies that a
@@ -1130,10 +1067,11 @@ func TestStopAndStoreRecording_MalformedContentType(t *testing.T) {
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
user, _, _ := seedInternalChatDeps(ctx, t, db)
user, org, model := seedInternalChatDeps(ctx, t, db)
workspace, _, _ := seedWorkspaceBinding(t, db, user.ID)
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{})
parent, _ := createParentChildChats(ctx, t, server, user, org, model)
mockConn.EXPECT().
StopDesktopRecording(gomock.Any(), gomock.Any()).
@@ -1145,9 +1083,8 @@ func TestStopAndStoreRecording_MalformedContentType(t *testing.T) {
recordingID := uuid.New().String()
result := server.stopAndStoreRecording(
ctx, mockConn, recordingID, user.ID,
ctx, mockConn, recordingID, parent.ID, user.ID,
uuid.NullUUID{UUID: workspace.ID, Valid: true},
uuid.Nil,
)
assert.Empty(t, result.recordingFileID, "RecordingFileID should be empty for malformed content type")
@@ -1166,10 +1103,11 @@ func TestStopAndStoreRecording_MissingBoundary(t *testing.T) {
ctrl := gomock.NewController(t)
mockConn := agentconnmock.NewMockAgentConn(ctrl)
user, _, _ := seedInternalChatDeps(ctx, t, db)
user, org, model := seedInternalChatDeps(ctx, t, db)
workspace, _, _ := seedWorkspaceBinding(t, db, user.ID)
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{})
parent, _ := createParentChildChats(ctx, t, server, user, org, model)
mockConn.EXPECT().
StopDesktopRecording(gomock.Any(), gomock.Any()).
@@ -1181,9 +1119,8 @@ func TestStopAndStoreRecording_MissingBoundary(t *testing.T) {
recordingID := uuid.New().String()
result := server.stopAndStoreRecording(
ctx, mockConn, recordingID, user.ID,
ctx, mockConn, recordingID, parent.ID, user.ID,
uuid.NullUUID{UUID: workspace.ID, Valid: true},
uuid.Nil,
)
assert.Empty(t, result.recordingFileID, "RecordingFileID should be empty when boundary is missing")
+111
View File
@@ -0,0 +1,111 @@
package chatd
import (
"context"
"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/codersdk"
)
func (p *Server) newStoreChatAttachmentFunc(workspaceCtx *turnWorkspaceContext) chattool.StoreFileFunc {
return func(
ctx context.Context,
name string,
detectName string,
data []byte,
) (chattool.AttachmentMetadata, error) {
workspaceCtx.chatStateMu.Lock()
chatSnapshot := *workspaceCtx.currentChat
workspaceCtx.chatStateMu.Unlock()
return p.storeChatAttachment(ctx, chatSnapshot, name, detectName, data)
}
}
func (p *Server) storeChatAttachment(
ctx context.Context,
chatSnapshot database.Chat,
name string,
detectName string,
data []byte,
) (chattool.AttachmentMetadata, error) {
if !chatSnapshot.WorkspaceID.Valid {
return chattool.AttachmentMetadata{}, xerrors.New("no workspace is associated with this chat. Use the create_workspace tool to create one")
}
storedName, mediaType, err := chatfiles.PrepareStoredFile(name, detectName, data)
if err != nil {
return chattool.AttachmentMetadata{}, err
}
// Insert and link in one transaction so a cap rejection or linking
// failure does not leave behind an unlinked chat file row.
var attachment chattool.AttachmentMetadata
err = p.db.InTx(func(tx database.Store) error {
ws, err := tx.GetWorkspaceByID(ctx, chatSnapshot.WorkspaceID.UUID)
if err != nil {
return xerrors.Errorf("resolve workspace: %w", err)
}
attachment, err = storeLinkedChatFileTx(
ctx,
tx,
chatSnapshot.ID,
chatSnapshot.OwnerID,
ws.OrganizationID,
storedName,
mediaType,
data,
)
return err
}, database.DefaultTXOptions().WithID("store_chat_attachment"))
if err != nil {
return chattool.AttachmentMetadata{}, err
}
return attachment, nil
}
func storeLinkedChatFileTx(
ctx context.Context,
tx database.Store,
chatID uuid.UUID,
ownerID uuid.UUID,
organizationID uuid.UUID,
name string,
mediaType string,
data []byte,
) (chattool.AttachmentMetadata, error) {
row, err := tx.InsertChatFile(ctx, database.InsertChatFileParams{
OwnerID: ownerID,
OrganizationID: organizationID,
Name: name,
Mimetype: mediaType,
Data: data,
})
if err != nil {
return chattool.AttachmentMetadata{}, xerrors.Errorf("insert chat file: %w", err)
}
rejected, err := tx.LinkChatFiles(ctx, database.LinkChatFilesParams{
ChatID: chatID,
MaxFileLinks: int32(codersdk.MaxChatFileIDs),
FileIds: []uuid.UUID{row.ID},
})
if err != nil {
return chattool.AttachmentMetadata{}, xerrors.Errorf("link chat file: %w", err)
}
if rejected > 0 {
return chattool.AttachmentMetadata{}, xerrors.Errorf("chat already has the maximum of %d linked files", codersdk.MaxChatFileIDs)
}
return chattool.AttachmentMetadata{
FileID: row.ID,
MediaType: mediaType,
Name: name,
}, nil
}
@@ -0,0 +1,267 @@
package chatd //nolint:testpackage
import (
"context"
"testing"
"github.com/google/uuid"
"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/codersdk"
)
func TestStoreChatAttachment_Success(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
db := dbmock.NewMockStore(ctrl)
tx := dbmock.NewMockStore(ctrl)
server := &Server{db: db}
chatID := uuid.New()
ownerID := uuid.New()
workspaceID := uuid.New()
orgID := uuid.New()
fileID := uuid.New()
chatSnapshot := database.Chat{
ID: chatID,
OwnerID: ownerID,
WorkspaceID: uuid.NullUUID{UUID: workspaceID, Valid: true},
}
expectStoreChatAttachmentTx(t, db, tx)
tx.EXPECT().GetWorkspaceByID(gomock.Any(), workspaceID).Return(database.Workspace{ID: workspaceID, OrganizationID: orgID}, nil)
tx.EXPECT().InsertChatFile(gomock.Any(), gomock.AssignableToTypeOf(database.InsertChatFileParams{})).DoAndReturn(
func(_ context.Context, arg database.InsertChatFileParams) (database.InsertChatFileRow, error) {
require.Equal(t, ownerID, arg.OwnerID)
require.Equal(t, orgID, arg.OrganizationID)
require.Equal(t, "build.log", arg.Name)
require.Equal(t, "text/plain", arg.Mimetype)
require.Equal(t, []byte("build output"), arg.Data)
return database.InsertChatFileRow{ID: fileID}, nil
},
)
tx.EXPECT().LinkChatFiles(gomock.Any(), database.LinkChatFilesParams{
ChatID: chatID,
MaxFileLinks: int32(codersdk.MaxChatFileIDs),
FileIds: []uuid.UUID{fileID},
}).Return(int32(0), nil)
attachment, err := server.storeChatAttachment(context.Background(), chatSnapshot, "build.log", "build.log", []byte("build output"))
require.NoError(t, err)
require.Equal(t, chattool.AttachmentMetadata{
FileID: fileID,
MediaType: "text/plain",
Name: "build.log",
}, attachment)
}
func TestStoreChatAttachment_UsesDetectNameForClassification(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
db := dbmock.NewMockStore(ctrl)
tx := dbmock.NewMockStore(ctrl)
server := &Server{db: db}
chatID := uuid.New()
ownerID := uuid.New()
workspaceID := uuid.New()
orgID := uuid.New()
fileID := uuid.New()
chatSnapshot := database.Chat{
ID: chatID,
OwnerID: ownerID,
WorkspaceID: uuid.NullUUID{UUID: workspaceID, Valid: true},
}
expectStoreChatAttachmentTx(t, db, tx)
tx.EXPECT().GetWorkspaceByID(gomock.Any(), workspaceID).Return(database.Workspace{ID: workspaceID, OrganizationID: orgID}, nil)
tx.EXPECT().InsertChatFile(gomock.Any(), gomock.AssignableToTypeOf(database.InsertChatFileParams{})).DoAndReturn(
func(_ context.Context, arg database.InsertChatFileParams) (database.InsertChatFileRow, error) {
require.Equal(t, "payload.txt", arg.Name)
require.Equal(t, "application/json", arg.Mimetype)
return database.InsertChatFileRow{ID: fileID}, nil
},
)
tx.EXPECT().LinkChatFiles(gomock.Any(), database.LinkChatFilesParams{
ChatID: chatID,
MaxFileLinks: int32(codersdk.MaxChatFileIDs),
FileIds: []uuid.UUID{fileID},
}).Return(int32(0), nil)
attachment, err := server.storeChatAttachment(context.Background(), chatSnapshot, "payload.txt", "report.json", []byte(`{"ok":true}`))
require.NoError(t, err)
require.Equal(t, "payload.txt", attachment.Name)
require.Equal(t, "application/json", attachment.MediaType)
}
func TestStoreChatAttachment_RejectsUnsupportedStoredFileTypeBeforeDBWork(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
db := dbmock.NewMockStore(ctrl)
server := &Server{db: db}
chatSnapshot := database.Chat{
ID: uuid.New(),
OwnerID: uuid.New(),
WorkspaceID: uuid.NullUUID{UUID: uuid.New(), Valid: true},
}
attachment, err := server.storeChatAttachment(
context.Background(),
chatSnapshot,
"evil.svg",
"evil.svg",
[]byte(`<svg xmlns="http://www.w3.org/2000/svg"><rect/></svg>`),
)
require.ErrorIs(t, err, chatfiles.ErrUnsupportedStoredFileType)
require.ErrorContains(t, err, "image/svg+xml")
require.Equal(t, chattool.AttachmentMetadata{}, attachment)
}
func TestStoreChatAttachment_NoWorkspace(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
db := dbmock.NewMockStore(ctrl)
server := &Server{db: db}
attachment, err := server.storeChatAttachment(context.Background(), database.Chat{}, "build.log", "build.log", []byte("build output"))
require.ErrorContains(t, err, "no workspace is associated")
require.Equal(t, chattool.AttachmentMetadata{}, attachment)
}
func TestStoreChatAttachment_WorkspaceLookupError(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
db := dbmock.NewMockStore(ctrl)
tx := dbmock.NewMockStore(ctrl)
server := &Server{db: db}
workspaceID := uuid.New()
chatSnapshot := database.Chat{
ID: uuid.New(),
OwnerID: uuid.New(),
WorkspaceID: uuid.NullUUID{UUID: workspaceID, Valid: true},
}
expectStoreChatAttachmentTx(t, db, tx)
tx.EXPECT().GetWorkspaceByID(gomock.Any(), workspaceID).Return(database.Workspace{}, context.DeadlineExceeded)
attachment, err := server.storeChatAttachment(context.Background(), chatSnapshot, "build.log", "build.log", []byte("build output"))
require.ErrorContains(t, err, "resolve workspace")
require.ErrorIs(t, err, context.DeadlineExceeded)
require.Equal(t, chattool.AttachmentMetadata{}, attachment)
}
func TestStoreChatAttachment_InsertError(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
db := dbmock.NewMockStore(ctrl)
tx := dbmock.NewMockStore(ctrl)
server := &Server{db: db}
workspaceID := uuid.New()
chatSnapshot := database.Chat{
ID: uuid.New(),
OwnerID: uuid.New(),
WorkspaceID: uuid.NullUUID{UUID: workspaceID, Valid: true},
}
expectStoreChatAttachmentTx(t, db, tx)
tx.EXPECT().GetWorkspaceByID(gomock.Any(), workspaceID).Return(database.Workspace{ID: workspaceID, OrganizationID: uuid.New()}, nil)
tx.EXPECT().InsertChatFile(gomock.Any(), gomock.Any()).Return(database.InsertChatFileRow{}, context.DeadlineExceeded)
attachment, err := server.storeChatAttachment(context.Background(), chatSnapshot, "build.log", "build.log", []byte("build output"))
require.ErrorContains(t, err, "insert chat file")
require.ErrorIs(t, err, context.DeadlineExceeded)
require.Equal(t, chattool.AttachmentMetadata{}, attachment)
}
func TestStoreChatAttachment_StrictCapError(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
db := dbmock.NewMockStore(ctrl)
tx := dbmock.NewMockStore(ctrl)
server := &Server{db: db}
chatID := uuid.New()
ownerID := uuid.New()
workspaceID := uuid.New()
orgID := uuid.New()
fileID := uuid.New()
chatSnapshot := database.Chat{
ID: chatID,
OwnerID: ownerID,
WorkspaceID: uuid.NullUUID{UUID: workspaceID, Valid: true},
}
expectStoreChatAttachmentTx(t, db, tx)
tx.EXPECT().GetWorkspaceByID(gomock.Any(), workspaceID).Return(database.Workspace{ID: workspaceID, OrganizationID: orgID}, nil)
tx.EXPECT().InsertChatFile(gomock.Any(), gomock.AssignableToTypeOf(database.InsertChatFileParams{})).Return(database.InsertChatFileRow{ID: fileID}, nil)
tx.EXPECT().LinkChatFiles(gomock.Any(), database.LinkChatFilesParams{
ChatID: chatID,
MaxFileLinks: int32(codersdk.MaxChatFileIDs),
FileIds: []uuid.UUID{fileID},
}).Return(int32(1), nil)
attachment, err := server.storeChatAttachment(context.Background(), chatSnapshot, "build.log", "build.log", []byte("build output"))
require.ErrorContains(t, err, "chat already has the maximum of 20 linked files")
require.Equal(t, chattool.AttachmentMetadata{}, attachment)
}
func TestStoreChatAttachment_LinkError(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
db := dbmock.NewMockStore(ctrl)
tx := dbmock.NewMockStore(ctrl)
server := &Server{db: db}
chatID := uuid.New()
ownerID := uuid.New()
workspaceID := uuid.New()
orgID := uuid.New()
fileID := uuid.New()
chatSnapshot := database.Chat{
ID: chatID,
OwnerID: ownerID,
WorkspaceID: uuid.NullUUID{UUID: workspaceID, Valid: true},
}
expectStoreChatAttachmentTx(t, db, tx)
tx.EXPECT().GetWorkspaceByID(gomock.Any(), workspaceID).Return(database.Workspace{ID: workspaceID, OrganizationID: orgID}, nil)
tx.EXPECT().InsertChatFile(gomock.Any(), gomock.Any()).Return(database.InsertChatFileRow{ID: fileID}, nil)
tx.EXPECT().LinkChatFiles(gomock.Any(), database.LinkChatFilesParams{
ChatID: chatID,
MaxFileLinks: int32(codersdk.MaxChatFileIDs),
FileIds: []uuid.UUID{fileID},
}).Return(int32(0), context.DeadlineExceeded)
attachment, err := server.storeChatAttachment(context.Background(), chatSnapshot, "build.log", "build.log", []byte("build output"))
require.ErrorContains(t, err, "link chat file")
require.ErrorIs(t, err, context.DeadlineExceeded)
require.Equal(t, chattool.AttachmentMetadata{}, attachment)
}
func expectStoreChatAttachmentTx(t *testing.T, db, tx *dbmock.MockStore) {
t.Helper()
db.EXPECT().InTx(gomock.Any(), gomock.AssignableToTypeOf(&database.TxOptions{})).DoAndReturn(
func(fn func(database.Store) error, opts *database.TxOptions) error {
require.NotNil(t, opts)
require.Equal(t, "store_chat_attachment", opts.TxIdentifier)
return fn(tx)
},
)
}
+3 -1
View File
@@ -41,6 +41,8 @@ Your primary tool is the "computer" tool which lets you interact with the deskto
Guidelines:
- Always start by taking a screenshot to see the current state of the desktop.
- Use wait or ordinary actions when you only need a screenshot for your own reasoning.
- Use an explicit screenshot action when you want to share a durable screenshot with the user; those screenshots are attached to the chat automatically.
- Be precise with coordinates when clicking or typing.
- Wait for UI elements to load before interacting with them.
- If an action doesn't produce the expected result, try alternative approaches.
@@ -373,7 +375,7 @@ func (p *Server) subagentTools(
stopCtx, stopCancel := context.WithTimeout(context.WithoutCancel(ctx), 90*time.Second)
defer stopCancel()
recResult = p.stopAndStoreRecording(stopCtx, agentConn,
recordingID, parent.OwnerID, parent.WorkspaceID, parent.ID)
recordingID, parent.ID, parent.OwnerID, parent.WorkspaceID)
}
resp := map[string]any{
"chat_id": targetChatID.String(),
+153 -10
View File
@@ -19,6 +19,7 @@ import (
"github.com/coder/coder/v2/coderd/database/dbtestutil"
"github.com/coder/coder/v2/coderd/database/pubsub"
coderdpubsub "github.com/coder/coder/v2/coderd/pubsub"
"github.com/coder/coder/v2/coderd/x/chatd/chatloop"
"github.com/coder/coder/v2/coderd/x/chatd/chatprompt"
"github.com/coder/coder/v2/coderd/x/chatd/chatprovider"
"github.com/coder/coder/v2/coderd/x/chatd/chattool"
@@ -27,16 +28,6 @@ import (
"github.com/coder/quartz"
)
func TestComputerUseSubagentSystemPrompt(t *testing.T) {
t.Parallel()
// Verify the system prompt constant is non-empty and contains
// key instructions for the computer use agent.
assert.NotEmpty(t, computerUseSubagentSystemPrompt)
assert.Contains(t, computerUseSubagentSystemPrompt, "computer")
assert.Contains(t, computerUseSubagentSystemPrompt, "screenshot")
}
func TestSubagentFallbackChatTitle(t *testing.T) {
t.Parallel()
@@ -1291,6 +1282,158 @@ func insertAssistantMessage(
require.NoError(t, err)
}
func insertLinkedChatFile(
ctx context.Context,
t *testing.T,
db database.Store,
chatID uuid.UUID,
ownerID uuid.UUID,
organizationID uuid.UUID,
name string,
mediaType string,
data []byte,
) uuid.UUID {
t.Helper()
file, err := db.InsertChatFile(ctx, database.InsertChatFileParams{
OwnerID: ownerID,
OrganizationID: organizationID,
Name: name,
Mimetype: mediaType,
Data: data,
})
require.NoError(t, err)
rejected, err := db.LinkChatFiles(ctx, database.LinkChatFilesParams{
ChatID: chatID,
MaxFileLinks: int32(codersdk.MaxChatFileIDs),
FileIds: []uuid.UUID{file.ID},
})
require.NoError(t, err)
require.Zero(t, rejected)
return file.ID
}
func TestWaitAgentDoesNotRelayComputerUseSubagentAttachments(t *testing.T) {
t.Parallel()
db, ps := dbtestutil.NewDB(t)
ctx := chatdTestContext(t)
user, org, model := seedInternalChatDeps(ctx, t, db)
workspace, _, agent := seedWorkspaceBinding(t, db, user.ID)
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{})
parent, child := createComputerUseParentChild(
ctx, t, server, user, org, model, workspace, agent,
"parent-relay", "child-relay",
)
insertedFile := insertLinkedChatFile(
ctx,
t,
db,
child.ID,
user.ID,
workspace.OrganizationID,
"screenshot.png",
"image/png",
[]byte("fake-png"),
)
insertAssistantMessage(ctx, t, db, child.ID, model.ID, "Shared the screenshot.")
setChatStatus(ctx, t, db, child.ID, database.ChatStatusWaiting, "")
resp, err := invokeWaitAgentTool(ctx, t, server, db, parent.ID, child.ID, 5)
require.NoError(t, err)
require.False(t, resp.IsError, "expected successful response, got: %s", resp.Content)
var result map[string]any
require.NoError(t, json.Unmarshal([]byte(resp.Content), &result))
require.Equal(t, "Shared the screenshot.", result["report"])
require.Equal(t, string(database.ChatStatusWaiting), result["status"])
assert.NotContains(t, result, "attachment_count")
assert.NotContains(t, result, "attachment_warning")
attachments, err := chattool.AttachmentsFromMetadata(resp.Metadata)
require.NoError(t, err)
assert.Empty(t, attachments)
parts := buildAssistantPartsForPersist(
context.Background(),
testutil.Logger(t),
nil,
[]fantasy.ToolResultContent{{
ToolCallID: "call-1",
ToolName: "wait_agent",
ClientMetadata: resp.Metadata,
}},
chatloop.PersistedStep{},
nil,
)
assert.Empty(t, parts)
parentFiles, err := db.GetChatFileMetadataByChatID(ctx, parent.ID)
require.NoError(t, err)
assert.Empty(t, parentFiles)
childFiles, err := db.GetChatFileMetadataByChatID(ctx, child.ID)
require.NoError(t, err)
require.Len(t, childFiles, 1)
assert.Equal(t, insertedFile, childFiles[0].ID)
assert.Equal(t, "screenshot.png", childFiles[0].Name)
assert.Equal(t, "image/png", childFiles[0].Mimetype)
}
func TestWaitAgentDoesNotRelayRegularSubagentAttachments(t *testing.T) {
t.Parallel()
db, ps := dbtestutil.NewDB(t)
ctx := chatdTestContext(t)
user, org, model := seedInternalChatDeps(ctx, t, db)
workspace, _, _ := seedWorkspaceBinding(t, db, user.ID)
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{})
parent, child := createParentChildChats(ctx, t, server, user, org, model)
server.drainInflight()
insertedFile := insertLinkedChatFile(
ctx,
t,
db,
child.ID,
user.ID,
workspace.OrganizationID,
"notes.txt",
"text/plain",
[]byte("release notes"),
)
insertAssistantMessage(ctx, t, db, child.ID, model.ID, "Shared the release notes.")
setChatStatus(ctx, t, db, child.ID, database.ChatStatusWaiting, "")
resp, err := invokeWaitAgentTool(ctx, t, server, db, parent.ID, child.ID, 5)
require.NoError(t, err)
require.False(t, resp.IsError, "expected successful response, got: %s", resp.Content)
var result map[string]any
require.NoError(t, json.Unmarshal([]byte(resp.Content), &result))
require.Equal(t, "Shared the release notes.", result["report"])
assert.NotContains(t, result, "attachment_count")
assert.NotContains(t, result, "attachment_warning")
attachments, err := chattool.AttachmentsFromMetadata(resp.Metadata)
require.NoError(t, err)
assert.Empty(t, attachments)
parentFiles, err := db.GetChatFileMetadataByChatID(ctx, parent.ID)
require.NoError(t, err)
assert.Empty(t, parentFiles)
childFiles, err := db.GetChatFileMetadataByChatID(ctx, child.ID)
require.NoError(t, err)
require.Len(t, childFiles, 1)
assert.Equal(t, insertedFile, childFiles[0].ID)
assert.Equal(t, "notes.txt", childFiles[0].Name)
assert.Equal(t, "text/plain", childFiles[0].Mimetype)
}
func TestAwaitSubagentCompletion(t *testing.T) {
t.Parallel()
+3 -1
View File
@@ -212,6 +212,7 @@ type ChatMessagePart struct {
URL string `json:"url" variants:"source"`
Title string `json:"title,omitempty" variants:"source?"`
MediaType string `json:"media_type" variants:"file"`
Name string `json:"name,omitempty" variants:"file?"`
Data []byte `json:"data,omitempty" variants:"file?"`
FileID uuid.NullUUID `json:"file_id,omitempty" format:"uuid" variants:"file?"`
FileName string `json:"file_name" variants:"file-reference"`
@@ -327,11 +328,12 @@ func ChatMessageToolResult(toolCallID, toolName string, result json.RawMessage,
}
// ChatMessageFile builds a file chat message part.
func ChatMessageFile(fileID uuid.UUID, mediaType string) ChatMessagePart {
func ChatMessageFile(fileID uuid.UUID, mediaType string, name string) ChatMessagePart {
return ChatMessagePart{
Type: ChatMessagePartTypeFile,
FileID: uuid.NullUUID{UUID: fileID, Valid: true},
MediaType: mediaType,
Name: name,
}
}
+1 -1
View File
@@ -324,7 +324,7 @@ require (
github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f // indirect
github.com/felixge/httpsnoop v1.0.4 // indirect
github.com/fxamacker/cbor/v2 v2.9.0 // indirect
github.com/gabriel-vasile/mimetype v1.4.12 // indirect
github.com/gabriel-vasile/mimetype v1.4.12
github.com/go-chi/hostrouter v0.3.0 // indirect
github.com/go-ini/ini v1.67.0 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
+1
View File
@@ -1614,6 +1614,7 @@ export interface ChatFileMetadata {
export interface ChatFilePart {
readonly type: "file";
readonly media_type: string;
readonly name?: string;
readonly data?: string;
readonly file_id?: string;
}