test(imageinput): cover the darwin unreadable-clipboard decision itself

The test added with the fix asserted around the change rather than on it. It
substituted ReadClipboardImage with a stub returning the error, which proves
the TUI surfaces an error handed to it, but never reaches
readClipboardImageDarwin, where the decision lives. Reverting both production
return sites to nil,nil left every package green, which is the definition of a
test that is not testing the change. Caught by gnanam.

readClipboardImageDarwin shells out twice and had no seam, so the two steps are
now package-level vars, the same shape as the readClipboardImage var this
change already relied on. That makes the case that matters reachable: the info
probe succeeds and reports an image, and the extractor then fails. It cannot be
arranged from outside the function, because from there both steps are one
opaque call.

Five cases, and the negative ones carry as much weight as the positive. An
image with no working extractor, and an extractor that exits 0 with no bytes,
must both be actionable. A clipboard offering no image, and a failing info
probe, must both stay silent, because turning those into a notice would make
every stray Ctrl+V nag the user. The extractor must not run at all in either
silent case.

Repeating gnanam's experiment now fails: reverting both return sites to nil,nil
fails the two actionable cases and leaves the silent ones passing, which is
exactly the discrimination the fix is supposed to add.

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
+// 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
 	}
-	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 "
+	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)
 }

internal/tui/model.go

@@ -1699,6 +1699,27 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) {
 				m.input.SetWidth(maxInt(20, m.chatColumnWidth()-14))
 				return m, nil
 			}
+		case keyCtrl(msg, 'v'), keySuper(msg, 'v'):
+			// Ctrl+V probes the clipboard for an IMAGE only. Text pasting stays
+			// exclusively on the terminal's bracketed-paste path (Bubble's own
+			// Ctrl+V binding is disabled in newModel for exactly that reason), so
+			// this cannot double-insert text. It is needed because a clipboard
+			// holding a screenshot produces no bracketed paste at all: the terminal
+			// has no text to send, so routePaste never runs and its empty-content
+			// image probe never fires. Right-click paste reached that probe only
+			// because it always delivers a clipboardReadMsg, empty or not.
+			// readClipboardImageCmd yields no message when the clipboard holds no
+			// image, so Ctrl+V with text on the clipboard stays a no-op here and is
+			// handled by the bracketed paste exactly as before.
+			//
+			// Cmd+V is matched too, since macOS reports Command as ModSuper rather
+			// than ModCtrl. That only helps on terminals that deliver the key to the
+			// application: one that handles Cmd+V itself pastes the clipboard TEXT
+			// and sends no key event, so an image-only clipboard still produces
+			// nothing for this to react to.
+			if m.noBlockingModal() {
+				return m, readClipboardImageCmd()
+			}
 		case keyCtrl(msg, 'f'):
 			if m.picker != nil && m.picker.kind == pickerModel {
 				if m.modelPickerIsLoading() {