From 6d9193ccec5f04075892b905a970a5e5ccaa8304 Mon Sep 17 00:00:00 2001 From: tauraamui Date: Fri, 10 Oct 2025 09:30:50 +0100 Subject: [PATCH] test: ensure file picker onPaste works correctly --- internal/tui/components/chat/editor/editor.go | 49 +------ .../tui/components/chat/editor/editor_test.go | 131 ------------------ .../dialogs/filepicker/filepicker.go | 52 ++++--- .../dialogs/filepicker/filepicker_test.go | 32 +++++ 4 files changed, 65 insertions(+), 199 deletions(-) create mode 100644 internal/tui/components/dialogs/filepicker/filepicker_test.go diff --git a/internal/tui/components/chat/editor/editor.go b/internal/tui/components/chat/editor/editor.go index 8f5f7122f353e0e8fdb62129697d1eccdc558485..a355dad512a9772ef8bcd0d954a44e0f6d5aefcd 100644 --- a/internal/tui/components/chat/editor/editor.go +++ b/internal/tui/components/chat/editor/editor.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "io/fs" + "log/slog" "math/rand" "net/http" "os" @@ -185,7 +186,8 @@ func onCompletionItemSelect(fsys fs.FS, activeModelHasImageSupport func() (bool, // TODO(tauraamui): consolidate this kind of standard image attachment related warning return m, util.ReportWarn("File attachments are not supported by the current model: " + modelName) } - tooBig, _ := filepicker.IsFileTooBigWithFS(fsys, path, filepicker.MaxAttachmentSize) + slog.Debug("checking if image is too big", path, 1) + tooBig, _ := filepicker.IsFileTooBigWithFS(os.DirFS(filepath.Dir(path)), path, filepicker.MaxAttachmentSize) if tooBig { return m, nil } @@ -238,49 +240,6 @@ func isExtOfAllowedImageType(path string) bool { type ResolveAbs func(path string) (string, error) -func onPaste(fsysAbs ResolveAbs, activeModelHasImageSupport func() (bool, string), m *editorCmp, msg tea.PasteMsg) (tea.Model, tea.Cmd) { - var cmd tea.Cmd - path := strings.ReplaceAll(string(msg), "\\ ", " ") - // try to get an image, in this case specifically because the file - // path cannot have been limited to just the PWD as the root, since the - // path is coming from the contents of a clipboard - path, err := fsysAbs(strings.TrimSpace(path)) - if err != nil { - m.textarea, cmd = m.textarea.Update(msg) - return m, cmd - } - // TODO(tauraamui) [17/09/2025]: this needs to be combined with the actual data inference/checking - // of the contents that happens when we resolve the "mime" type - if !isExtOfAllowedImageType(path) { - m.textarea, cmd = m.textarea.Update(msg) - return m, cmd - } - if imagesSupported, modelName := activeModelHasImageSupport(); !imagesSupported { - // TODO(tauraamui): consolidate this kind of standard image attachment related warning - return m, util.ReportWarn("File attachments are not supported by the current model: " + modelName) - } - tooBig, _ := filepicker.IsFileTooBig(path, filepicker.MaxAttachmentSize) - if tooBig { - m.textarea, cmd = m.textarea.Update(msg) - return m, cmd - } - - // FIX(tauraamui) [19/09/2025]: this is incorrectly attempting to read a file from its abs path, - // whereas the FS we're accessing only starts from our relative dir/PWD - content, err := os.ReadFile(path) - if err != nil { - m.textarea, cmd = m.textarea.Update(msg) - return m, cmd - } - mimeBufferSize := min(512, len(content)) - mimeType := http.DetectContentType(content[:mimeBufferSize]) - fileName := filepath.Base(path) - attachment := message.Attachment{FilePath: path, FileName: fileName, MimeType: mimeType, Content: content} - return m, util.CmdHandler(filepicker.FilePickedMsg{ - Attachment: attachment, - }) -} - func activeModelHasImageSupport() (bool, string) { agentCfg := config.Get().Agents["coder"] model := config.Get().GetModelByType(agentCfg.Model) @@ -320,8 +279,6 @@ func (m *editorCmp) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case OpenEditorMsg: m.textarea.SetValue(msg.Text) m.textarea.MoveToEnd() - case tea.PasteMsg: - return onPaste(filepath.Abs, activeModelHasImageSupport, m, msg) // inject fsys accessible from PWD case commands.ToggleYoloModeMsg: m.setEditorPrompt() return m, nil diff --git a/internal/tui/components/chat/editor/editor_test.go b/internal/tui/components/chat/editor/editor_test.go index 82cd2045794d4a0dbc73c69bb4acdbd4ab210ccb..60164cfb35efceea373ecf38b346a7046c48bf37 100644 --- a/internal/tui/components/chat/editor/editor_test.go +++ b/internal/tui/components/chat/editor/editor_test.go @@ -1,8 +1,6 @@ package editor import ( - "os" - "path/filepath" "testing" "testing/fstest" @@ -307,135 +305,6 @@ func TestEditor_OnCompletionPathToNonImageEmitsAttachFileMessage(t *testing.T) { assert.Nil(t, cmd) } -func TestEditor_OnPastePathToImageEmitsAttachFileMessage(t *testing.T) { - entriesForAutoComplete := mockDirLister([]string{"image.png", "random.txt"}) - - // Create a temporary directory and files for testing - tempDir := t.TempDir() - - // Create test image file - imagePath := filepath.Join(tempDir, "image.png") - err := os.WriteFile(imagePath, pngMagicNumberData, 0o644) - require.NoError(t, err) - - // Create test text file - textPath := filepath.Join(tempDir, "random.txt") - err = os.WriteFile(textPath, []byte("Some content"), 0o644) - require.NoError(t, err) - - testEditor := newEditor(&app.App{}, entriesForAutoComplete) - - // Change to temp directory so paths resolve correctly - originalWd, err := os.Getwd() - require.NoError(t, err) - defer os.Chdir(originalWd) - err = os.Chdir(tempDir) - require.NoError(t, err) - - modelHasImageSupport := func() (bool, string) { - return true, "TestModel" - } - absRef := filepath.Abs - _, cmd := onPaste(absRef, modelHasImageSupport, testEditor, tea.PasteMsg("image.png")) - - require.NotNil(t, cmd) - msg := cmd() - assert.NotNil(t, msg) - - var attachmentMsg message.Attachment - if fpickedMsg, ok := msg.(filepicker.FilePickedMsg); ok { - attachmentMsg = fpickedMsg.Attachment - } - - assert.NoError(t, err) - - // Create a copy of the attachment for comparison, but use the actual FilePath from the message - // This handles the case on macOS where the path might have a "/private" prefix - expectedAttachment := message.Attachment{ - FilePath: attachmentMsg.FilePath, // Use the actual path from the message - FileName: "image.png", - MimeType: "image/png", - Content: pngMagicNumberData, - } - - assert.Equal(t, expectedAttachment, attachmentMsg) -} - -func TestEditor_OnPastePathToNonImageEmitsAttachFileMessage(t *testing.T) { - entriesForAutoComplete := mockDirLister([]string{"image.png", "random.txt"}) - - // Create a temporary directory and files for testing - tempDir := t.TempDir() - - // Create test image file - imagePath := filepath.Join(tempDir, "image.png") - err := os.WriteFile(imagePath, pngMagicNumberData, 0o644) - require.NoError(t, err) - - // Create test text file - textPath := filepath.Join(tempDir, "random.txt") - err = os.WriteFile(textPath, []byte("Some content"), 0o644) - require.NoError(t, err) - - testEditor := newEditor(&app.App{}, entriesForAutoComplete) - - // Change to temp directory so paths resolve correctly - originalWd, err := os.Getwd() - require.NoError(t, err) - defer os.Chdir(originalWd) - err = os.Chdir(tempDir) - require.NoError(t, err) - - modelHasImageSupport := func() (bool, string) { - return true, "TestModel" - } - _, cmd := onPaste(filepath.Abs, modelHasImageSupport, testEditor, tea.PasteMsg("random.txt")) - - assert.Nil(t, cmd) -} - -func TestEditor_OnPastePathToNonImageEmitsWanrningMessageWhenModelDoesNotSupportImages(t *testing.T) { - entriesForAutoComplete := mockDirLister([]string{"image.png", "random.txt"}) - - // Create a temporary directory and files for testing - tempDir := t.TempDir() - - // Create test image file - imagePath := filepath.Join(tempDir, "image.png") - err := os.WriteFile(imagePath, pngMagicNumberData, 0o644) - require.NoError(t, err) - - // Create test text file - textPath := filepath.Join(tempDir, "random.txt") - err = os.WriteFile(textPath, []byte("Some content"), 0o644) - require.NoError(t, err) - - testEditor := newEditor(&app.App{}, entriesForAutoComplete) - - // Change to temp directory so paths resolve correctly - originalWd, err := os.Getwd() - require.NoError(t, err) - defer os.Chdir(originalWd) - err = os.Chdir(tempDir) - require.NoError(t, err) - - modelDoesNotHaveImageSupport := func() (bool, string) { - return false, "ImagesUnsupportedTestModel" - } - _, cmd := onPaste(filepath.Abs, modelDoesNotHaveImageSupport, testEditor, tea.PasteMsg("image.png")) - - require.NotNil(t, cmd) - msg := cmd() - require.NotNil(t, msg) - - warningMsg, ok := msg.(util.InfoMsg) - require.True(t, ok) - assert.Equal(t, util.InfoMsg{ - Type: util.InfoTypeWarn, - Msg: "File attachments are not supported by the current model: ImagesUnsupportedTestModel", - }, warningMsg) -} - // TestHelperFunctions demonstrates how to use the batch message helpers func TestHelperFunctions(t *testing.T) { testEditor := newEditor(&app.App{}, mockDirLister([]string{"file1.txt", "file2.txt"})) diff --git a/internal/tui/components/dialogs/filepicker/filepicker.go b/internal/tui/components/dialogs/filepicker/filepicker.go index 62f7bad59d4d0db7139146b08dc75daad1dc8438..83ee989018ee877171d50c7c148a5ee19b689b64 100644 --- a/internal/tui/components/dialogs/filepicker/filepicker.go +++ b/internal/tui/components/dialogs/filepicker/filepicker.go @@ -130,28 +130,7 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { // Get the path of the selected file. return m, tea.Sequence( util.CmdHandler(dialogs.CloseDialogMsg{}), - func() tea.Msg { - isFileLarge, err := IsFileTooBig(path, MaxAttachmentSize) - if err != nil { - return util.ReportError(fmt.Errorf("unable to read the image: %w", err)) - } - if isFileLarge { - return util.ReportError(fmt.Errorf("file too large, max 5MB")) - } - - content, err := os.ReadFile(path) - if err != nil { - return util.ReportError(fmt.Errorf("unable to read the image: %w", err)) - } - - mimeBufferSize := min(512, len(content)) - mimeType := http.DetectContentType(content[:mimeBufferSize]) - fileName := filepath.Base(path) - attachment := message.Attachment{FilePath: path, FileName: fileName, MimeType: mimeType, Content: content} - return FilePickedMsg{ - Attachment: attachment, - } - }, + onPaste(resolveFS(filepath.Dir(path)), path), ) } m.image, cmd = m.image.Update(msg) @@ -159,6 +138,35 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, tea.Batch(cmds...) } +func resolveFS(baseDirPath string) fs.FS { + return os.DirFS(baseDirPath) +} + +func onPaste(fsys fs.FS, path string) func() tea.Msg { + name := filepath.Base(path) + return func() tea.Msg { + isFileLarge, err := IsFileTooBigWithFS(fsys, name, MaxAttachmentSize) + if err != nil { + return util.ReportError(fmt.Errorf("unable to read the image: %w, %s", err, path)) + } + if isFileLarge { + return util.ReportError(fmt.Errorf("file too large, max 5MB")) + } + + content, err := fs.ReadFile(fsys, name) + if err != nil { + return util.ReportError(fmt.Errorf("unable to read the image: %w", err)) + } + + mimeBufferSize := min(512, len(content)) + mimeType := http.DetectContentType(content[:mimeBufferSize]) + attachment := message.Attachment{FilePath: path, FileName: name, MimeType: mimeType, Content: content} + return FilePickedMsg{ + Attachment: attachment, + } + } +} + func (m *model) View() string { t := styles.CurrentTheme() diff --git a/internal/tui/components/dialogs/filepicker/filepicker_test.go b/internal/tui/components/dialogs/filepicker/filepicker_test.go new file mode 100644 index 0000000000000000000000000000000000000000..dca652c854754f8625f558e5e38a234519afd2b3 --- /dev/null +++ b/internal/tui/components/dialogs/filepicker/filepicker_test.go @@ -0,0 +1,32 @@ +package filepicker + +import ( + "testing" + "testing/fstest" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +var pngMagicNumberData = []byte("\x89PNG\x0D\x0A\x1A\x0A") + +func TestOnPasteMockFSWithValidPath(t *testing.T) { + mockFS := fstest.MapFS{ + "image1.png": &fstest.MapFile{ + Data: pngMagicNumberData, + }, + "image2.png": &fstest.MapFile{ + Data: []byte("fake png content"), + }, + } + + // Test with the first file + cmd := onPaste(mockFS, "image1.png") + msg := cmd() + + filePickedMsg, ok := msg.(FilePickedMsg) + require.True(t, ok) + require.NotNil(t, filePickedMsg) + assert.Equal(t, "image1.png", filePickedMsg.Attachment.FileName) + assert.Equal(t, "image/png", filePickedMsg.Attachment.MimeType) +}