Skip to content

Fix #15: distinguish too-large clipboard images from empty - #23

Merged
aakashrajput merged 3 commits into
KlaatAI:mainfrom
nandanadileep:fix-issue-15
Jul 19, 2026
Merged

Fix #15: distinguish too-large clipboard images from empty#23
aakashrajput merged 3 commits into
KlaatAI:mainfrom
nandanadileep:fix-issue-15

Conversation

@nandanadileep

Copy link
Copy Markdown
Contributor

Fixes #15

What changed

readClipboardImage() returned null for both "no image on clipboard" and "image found but > 8MB", so pressing ctrl+v on a real Retina full-screen screenshot silently showed "No image on the clipboard" even though an image was there.

src/utils/clipboard-image.ts

Return type changed to a discriminated union so callers can tell the two apart:

export type ClipboardImageResult =
  | { ok: true;  image: ClipboardImage }
  | { ok: false; reason: "empty" }
  | { ok: false; reason: "too_large"; sizeBytes: number };
  • added a wrapPng(buf) helper that picks the right outcome based on size
  • MAX_IMAGE_BYTES is now exported (callers want to render the cap in messages)
  • each platform path (fromMac / fromLinux / fromWindows) returns null only on "no image at all", and readClipboardImage collapses that to { ok: false, reason: "empty" }

src/screens/repl.ts (ctrl+v handler, line ~4318)

On reason: "too_large", render:

Clipboard image is 13.9MB, over the 8MB limit — try a smaller crop or window screenshot.

The empty case keeps the existing "No image on the clipboard..." message.

src/utils/clipboard-image.test.ts (new)

Covers the three observable states by mocking node:child_process.spawnSync to return a 13.9MB, 1KB, and failing osascript invocation against the darwin path:

  • oversized → { ok:false, reason:"too_large", sizeBytes }
  • fits → { ok:true, image:{ mime:"image/png", b64 }}
  • empty → { ok:false, reason:"empty" }
  • sanity: MAX_IMAGE_BYTES === 8 * 1024 * 1024

Verification

  • tsc --noEmit clean
  • tsc --noEmit -p tests-tsconfig.json clean (only the standard `bun:test` type-resolution noise that already affects every test file)
  • Mocked-spawn test harness drives the discriminated result directly

Follow-ups (not in this PR)

Issue suggests an optional follow-up to auto-downscale / re-encode an oversized clipboard image to JPEG before giving up. Left out intentionally to keep this fix minimal and reviewable.

- readClipboardImage now returns a tagged result: {ok:true, image} |
  {ok:false, reason:'empty'} | {ok:false, reason:'too_large', sizeBytes}
- exported MAX_IMAGE_BYTES so the repl can render the cap in the message
- ctrl+v handler in repl.ts shows the actual size + limit (was: silently
  reported 'No image on the clipboard' for any image over 8MB, e.g. a
  full-screen Retina screenshot)
- added src/utils/clipboard-image.test.ts covering too_large / success /
  empty via a mocked spawnSync
mock.module('node:child_process', ...) replaces the module globally and
broke other tests that import spawn (headless-agent.test.ts). Exposed
wrapPng as a pure exported function and test that directly instead.
@github-actions

Copy link
Copy Markdown
Contributor

🤖 KlaatAI Review Bot (powered by Klaatu, advisory only — a maintainer makes the real call)

Issue match
Fully addresses #15. The return type is updated to a discriminated union, repl.ts surfaces the specific too-large message, and the optional auto-downscale follow-up is correctly deferred.

Test coverage
clipboard-image.test.ts thoroughly covers the new wrapPng pure function (oversized, under cap, exact boundary, empty). However, it lacks integration tests for readClipboardImage() (which would require mocking spawnSync) and does not verify the formatted string output in repl.ts.

Correctness concerns
In src/utils/clipboard-image.ts, fromLinux() correctly short-circuits and returns a too_large result if wl-paste succeeds but exceeds the cap, rather than falling through to xclip. In src/screens/repl.ts, (MAX_IMAGE_BYTES / (1024 * 1024)).toFixed() correctly evaluates to "8". No unhandled edge cases or risky interactions stand out.

Verdict
Needs human judgment call on whether the missing spawnSync mocks for readClipboardImage and UI string assertions in repl.ts are acceptable for this minimal fix.

This is an automated review to help triage faster, not a gate. Nothing here blocks merging.

@aakashrajput
aakashrajput merged commit d138697 into KlaatAI:main Jul 19, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ctrl+v clipboard image paste silently fails on real (large) screenshots

2 participants