fix(tui): paste clipboard images with Ctrl+V (#534)

internal/imageinput/clipboard.go

@@ -2,6 +2,7 @@ package imageinput
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"net/http"
 	"os"
@@ -16,6 +17,13 @@ import (
 // clipboard, or (nil, "", nil) when the clipboard has no image. Called when
 // text clipboard is empty (the user pasted a screenshot). The media type is
 // sniffed from the bytes, not trusted from the clipboard.
+// ErrClipboardImageUnreadable reports that the clipboard holds an image the host
+// has no way to extract. It is deliberately distinct from "no image on the
+// clipboard", which is an ordinary no-op: this one is actionable, and silence is
+// the wrong response to it.
+var ErrClipboardImageUnreadable = errors.New(
+	"the clipboard holds an image but no helper on this host can read it; on macOS install pngpaste (brew install pngpaste) or a Python with pyobjc")
+
 func ReadClipboardImage() ([]byte, string, error) {
 	data, err := readClipboardImageBytes()
 	if err != nil {
@@ -95,35 +103,71 @@ func readClipboardImageWindows() ([]byte, error) {
 
 // readClipboardImageDarwin uses osascript to check for and read a clipboard
 // image. Returns (nil, nil) when no image is present.
-func readClipboardImageDarwin() ([]byte, error) {
-	// Check clipboard info for image classes.
-	check := `osascript -e 'clipboard info'`
-	out, err := exec.Command("sh", "-c", check).Output()
-	if err != nil {
-		return nil, nil
-	}
-	info := string(out)
-	if !strings.Contains(info, "PNG") && !strings.Contains(info, "JPEG") && !strings.Contains(info, "TIFF") && !strings.Contains(info, "GIF") {
-		return nil, nil
-	}
-	// Write clipboard image to a temp file via AppleScript, then read it.
-	// Using pngpaste if available, falling back to a Python one-liner.
-	cmd := exec.Command("sh", "-c", `pngpaste - 2>/dev/null || python3 -c "
+// darwinClipboardInfo reports what the pasteboard is currently offering, and
+// darwinClipboardExtract pulls the PNG bytes out of it.
+//
+// Both are vars so the two-step logic below can be driven in a test. The step
+// that matters, "the clipboard holds an image and nothing here can extract it",
+// only happens when the first succeeds and the second fails, which cannot be
+// arranged by stubbing the caller: a test that substitutes ReadClipboardImage
+// asserts the plumbing around this function rather than the decision inside it,
+// and stays green with that decision reverted.
+var (
+	darwinClipboardInfo = func() (string, error) {
+		out, err := exec.Command("sh", "-c", `osascript -e 'clipboard info'`).Output()
+		return string(out), err
+	}
+	darwinClipboardExtract = func() ([]byte, error) {
+		// pngpaste if available, falling back to a Python one-liner. Neither
+		// ships with macOS: pngpaste is Homebrew-only and Apple dropped the
+		// bundled PyObjC, so a stock Mac fails here every time.
+		cmd := exec.Command("sh", "-c", `pngpaste - 2>/dev/null || python3 -c "
 import AppKit, sys
 pb = AppKit.NSPasteboard.generalPasteboard()
 data = pb.dataForType_(AppKit.NSPasteboardTypePNG)
 if data:
     sys.stdout.buffer.write(data.bytes())
 "`)
-	var stdout bytes.Buffer
-	cmd.Stdout = &stdout
-	if err := cmd.Run(); err != nil {
+		var stdout bytes.Buffer
+		cmd.Stdout = &stdout
+		if err := cmd.Run(); err != nil {
+			return nil, err
+		}
+		return stdout.Bytes(), nil
+	}
+)
+
+// darwinClipboardOffersImage reports whether the pasteboard info line names an
+// image class.
+func darwinClipboardOffersImage(info string) bool {
+	for _, class := range []string{"PNG", "JPEG", "TIFF", "GIF"} {
+		if strings.Contains(info, class) {
+			return true
+		}
+	}
+	return false
+}
+
+func readClipboardImageDarwin() ([]byte, error) {
+	info, err := darwinClipboardInfo()
+	if err != nil {
 		return nil, nil
 	}
-	if stdout.Len() == 0 {
+	if !darwinClipboardOffersImage(info) {
 		return nil, nil
 	}
-	return stdout.Bytes(), nil
+	// Past this point the clipboard is KNOWN to hold an image, because the probe
+	// above said so. A failure here is therefore not "no image", it is "there is
+	// an image and nothing on this host can extract it". Reporting that as
+	// absence is what left users staring at a paste that silently did nothing.
+	data, err := darwinClipboardExtract()
+	if err != nil {
+		return nil, ErrClipboardImageUnreadable
+	}
+	if len(data) == 0 {
+		return nil, ErrClipboardImageUnreadable
+	}
+	return data, nil
 }
 
 // readClipboardImageLinux tries wl-paste (Wayland) then xclip (X11) to read

internal/imageinput/clipboard_darwin_logic_test.go

@@ -0,0 +1,84 @@
+package imageinput
+
+import (
+	"errors"
+	"testing"
+)
+
+// Drives readClipboardImageDarwin's own decision rather than the plumbing around
+// it. Both shell steps are substituted, so this runs on any platform and, more
+// importantly, actually reaches the branch under test: a test that stubs
+// ReadClipboardImage instead stays green with the production change reverted,
+// which is how the first version of this coverage slipped through.
+func TestReadClipboardImageDarwinDistinguishesUnreadableFromAbsent(t *testing.T) {
+	restore := func() func() {
+		info, extract := darwinClipboardInfo, darwinClipboardExtract
+		return func() { darwinClipboardInfo, darwinClipboardExtract = info, extract }
+	}()
+	t.Cleanup(restore)
+
+	const pngInfo = `«class PNGf», «class 8BPS»`
+
+	t.Run("image present but no extractor is actionable", func(t *testing.T) {
+		darwinClipboardInfo = func() (string, error) { return pngInfo, nil }
+		darwinClipboardExtract = func() ([]byte, error) { return nil, errors.New("exit status 127") }
+
+		data, err := readClipboardImageDarwin()
+		if !errors.Is(err, ErrClipboardImageUnreadable) {
+			t.Fatalf("err = %v, want ErrClipboardImageUnreadable; a stock Mac gets silence instead of a remedy", err)
+		}
+		if data != nil {
+			t.Fatalf("data = %q, want none", data)
+		}
+	})
+
+	t.Run("extractor producing nothing is also actionable", func(t *testing.T) {
+		darwinClipboardInfo = func() (string, error) { return pngInfo, nil }
+		darwinClipboardExtract = func() ([]byte, error) { return nil, nil }
+
+		if _, err := readClipboardImageDarwin(); !errors.Is(err, ErrClipboardImageUnreadable) {
+			t.Fatalf("err = %v, want ErrClipboardImageUnreadable for an extractor that exits 0 with no bytes", err)
+		}
+	})
+
+	// The other half of the distinction, and the reason this cannot simply always
+	// report an error: an empty clipboard is an ordinary no-op, and turning that
+	// into a notice would make every stray Ctrl+V nag the user.
+	t.Run("no image on the clipboard stays silent", func(t *testing.T) {
+		darwinClipboardInfo = func() (string, error) { return `«class utf8»`, nil }
+		darwinClipboardExtract = func() ([]byte, error) {
+			t.Fatal("extractor ran despite the clipboard offering no image")
+			return nil, nil
+		}
+
+		data, err := readClipboardImageDarwin()
+		if err != nil || data != nil {
+			t.Fatalf("data=%q err=%v, want a silent no-op", data, err)
+		}
+	})
+
+	t.Run("probe failure stays silent", func(t *testing.T) {
+		darwinClipboardInfo = func() (string, error) { return "", errors.New("osascript missing") }
+		darwinClipboardExtract = func() ([]byte, error) {
+			t.Fatal("extractor ran despite the info probe failing")
+			return nil, nil
+		}
+
+		if data, err := readClipboardImageDarwin(); err != nil || data != nil {
+			t.Fatalf("data=%q err=%v, want a silent no-op", data, err)
+		}
+	})
+
+	t.Run("working extractor returns the bytes", func(t *testing.T) {
+		darwinClipboardInfo = func() (string, error) { return pngInfo, nil }
+		darwinClipboardExtract = func() ([]byte, error) { return []byte("png-bytes"), nil }
+
+		data, err := readClipboardImageDarwin()
+		if err != nil {
+			t.Fatalf("err = %v, want none", err)
+		}
+		if string(data) != "png-bytes" {
+			t.Fatalf("data = %q, want the extractor's bytes", data)
+		}
+	})
+}

internal/tui/clipboard.go

@@ -23,12 +23,16 @@ type clipboardImageMsg struct {
 	err       error
 }
 
+// readClipboardImage is the OS clipboard image reader. A var so tests can
+// substitute a stub instead of depending on the developer's real clipboard.
+var readClipboardImage = imageinput.ReadClipboardImage
+
 // readClipboardImageCmd reads the OS clipboard for image content off the
 // Update goroutine. Returns a clipboardImageMsg with the bytes, or nil (no
 // command) if there is no image — the caller treats nil as a silent no-op.
 func readClipboardImageCmd() tea.Cmd {
 	return func() tea.Msg {
-		data, mediaType, err := imageinput.ReadClipboardImage()
+		data, mediaType, err := readClipboardImage()
 		if err != nil {
 			return clipboardImageMsg{err: err}
 		}

internal/tui/composer_test.go

@@ -6,6 +6,8 @@ import (
 	"testing"
 
 	tea "charm.land/bubbletea/v2"
+
+	"github.com/Gitlawb/zero/internal/imageinput"
 )
 
 func TestComposerInsertNewlineAtCursor(t *testing.T) {
@@ -129,22 +131,77 @@ func TestSanitizeComposerPastePreservesNewlines(t *testing.T) {
 	}
 }
 
-func TestCtrlVDoesNotPasteIntoComposer(t *testing.T) {
+// Ctrl+V must never insert TEXT: bracketed paste is the only text path, so
+// letting Bubble's own Ctrl+V binding also read the clipboard would double-paste.
+// It may return a command, but that command is the image probe below, never a
+// text paste, so the composer contents are unchanged either way.
+func TestCtrlVDoesNotPasteTextIntoComposer(t *testing.T) {
 	m := newModel(context.Background(), Options{})
 	m.input.SetValue("hello")
 	m.input.CursorEnd()
 
-	updated, cmd := m.Update(testKeyCtrl('v'))
+	updated, _ := m.Update(testKeyCtrl('v'))
 	next := updated.(model)
 
-	if cmd != nil {
-		t.Fatal("ctrl+v should not run the textinput clipboard paste command")
-	}
 	if got := next.composerValue(); got != "hello" {
 		t.Fatalf("composer value after ctrl+v = %q, want unchanged", got)
 	}
 }
 
+// Ctrl+V probes the clipboard for an image (#534). A clipboard holding a
+// screenshot produces no bracketed paste at all, so without this the empty-paste
+// image probe in routePaste never runs and Ctrl+V does nothing, which is exactly
+// what users reported: right-click paste attached the image, Ctrl+V did not.
+func TestCtrlVProbesClipboardForImage(t *testing.T) {
+	// Stubbed BEFORE Update, so the command Update returns is the one that runs.
+	// Asserting only that some command came back, then running a freshly built
+	// probe, would pass even if the key never reached the image route at all.
+	original := readClipboardImage
+	t.Cleanup(func() { readClipboardImage = original })
+	readClipboardImage = func() ([]byte, string, error) {
+		return []byte("png-bytes"), "image/png", nil
+	}
+
+	// Both modifiers: macOS reports Command as ModSuper, so a handler matching
+	// only ModCtrl leaves Cmd+V doing nothing on the platform where screenshots
+	// are most often on the clipboard.
+	for name, key := range map[string]tea.KeyPressMsg{
+		"ctrl+v": testKeyCtrl('v'),
+		"cmd+v":  testKeyPressMod('v', tea.ModSuper),
+	} {
+		t.Run(name, func(t *testing.T) {
+			m := newModel(context.Background(), Options{})
+			_, cmd := m.Update(key)
+			if cmd == nil {
+				t.Fatal("no command issued; the clipboard image probe never ran")
+			}
+			image, ok := execCmd(cmd).(clipboardImageMsg)
+			if !ok {
+				t.Fatal("the command issued was not the clipboard image probe")
+			}
+			if string(image.data) != "png-bytes" || image.mediaType != "image/png" {
+				t.Fatalf("unexpected image message: %+v", image)
+			}
+		})
+	}
+}
+
+// With no image on the clipboard the probe stays silent: Ctrl+V while copying
+// text must not emit a notice or disturb the composer. Driven through Update
+// rather than by calling the probe directly, so this covers the real key route
+// and not just the command in isolation.
+func TestClipboardImageProbeSilentWithoutImage(t *testing.T) {
+	original := readClipboardImage
+	t.Cleanup(func() { readClipboardImage = original })
+	readClipboardImage = func() ([]byte, string, error) { return nil, "", nil }
+
+	m := newModel(context.Background(), Options{})
+	_, cmd := m.Update(testKeyCtrl('v'))
+	if msg := execCmd(cmd); msg != nil {
+		t.Fatalf("probe emitted %T with no image on the clipboard, want no message", msg)
+	}
+}
+
 func TestPastedMultilineComposerContentRendersAsPreview(t *testing.T) {
 	paste := strings.Join([]string{
 		"Create a book library dashboard page with the Bootstrap theme.",
@@ -465,3 +522,53 @@ func TestComposerTerminalWordKeybindings(t *testing.T) {
 		})
 	}
 }
+
+// A clipboard that holds an image the host cannot extract must say so. This is
+// the macOS case: neither pngpaste nor PyObjC ships with the OS, so the reader
+// fails on a stock Mac, and reporting that as "no image" meant Ctrl+V did
+// nothing at all with no explanation. Silence is the wrong answer to a
+// condition the user can act on.
+func TestClipboardImageUnreadableSurfacesToUser(t *testing.T) {
+	original := readClipboardImage
+	t.Cleanup(func() { readClipboardImage = original })
+	readClipboardImage = func() ([]byte, string, error) {
+		return nil, "", imageinput.ErrClipboardImageUnreadable
+	}
+
+	m := newModel(context.Background(), Options{})
+	_, cmd := m.Update(testKeyCtrl('v'))
+	if cmd == nil {
+		t.Fatal("no command issued; the clipboard image probe never ran")
+	}
+	msg := execCmd(cmd)
+	image, ok := msg.(clipboardImageMsg)
+	if !ok {
+		t.Fatalf("probe returned %T, want clipboardImageMsg carrying the failure", msg)
+	}
+	if image.err == nil {
+		t.Fatal("the unreadable-clipboard failure was swallowed; the user is told nothing")
+	}
+	// The message has to name the remedy, since the whole point is that the user
+	// can fix this by installing something.
+	if !strings.Contains(image.err.Error(), "pngpaste") {
+		t.Fatalf("error = %q, want it to name what to install", image.err.Error())
+	}
+
+	// And it must reach the transcript as an error row rather than stopping at
+	// the message, which is where the old silent no-op ended.
+	updated, _ := m.Update(image)
+	next, ok := updated.(model)
+	if !ok {
+		t.Fatalf("Update returned %T, want model", updated)
+	}
+	found := false
+	for _, row := range next.transcript {
+		if row.kind == rowError && strings.Contains(row.text, "Clipboard image read failed") {
+			found = true
+			break
+		}
+	}
+	if !found {
+		t.Fatal("the failure never reached the transcript; the user still sees nothing")
+	}
+}

internal/tui/input_compat.go

@@ -34,6 +34,14 @@ func keyCtrl(msg tea.KeyMsg, code rune) bool {
 	return keyCode(msg) == code && keyHasMod(msg, tea.ModCtrl)
 }
 
+// keySuper matches the Command key on macOS, which the rest of the TUI models as
+// ModSuper (see keybindings.go). Kept separate from keyCtrl because a binding
+// that means Ctrl on Linux and Windows usually means Command on macOS, and the
+// two arrive as different modifiers.
+func keySuper(msg tea.KeyMsg, code rune) bool {
+	return keyCode(msg) == code && keyHasMod(msg, tea.ModSuper)
+}
+
 func keyCtrlArrow(msg tea.KeyMsg, code rune) bool {
 	return keyIs(msg, code) && keyHasMod(msg, tea.ModCtrl)
 }