close
Skip to content

fix: attach clipboard images despite text/html and synthesize filenames (#2489) - #2520

Merged
kyleseaman merged 1 commit into
mainfrom
fix/chat-clipboard-image-paste-2489
Aug 10, 2026
Merged

fix: attach clipboard images despite text/html and synthesize filenames (#2489)#2520
kyleseaman merged 1 commit into
mainfrom
fix/chat-clipboard-image-paste-2489

Conversation

@bolichen97

Copy link
Copy Markdown
Collaborator

Summary

The chat composer's paste handler (from #2340) deferred to a text paste whenever the clipboard carried text/plain or text/html. A <textarea> can only ever insert the text/plain representation — so for clipboards with text/html but no text/plain (a browser's right-click Copy Image, an Office chart copy) the whole gesture was a silent no-op: no attachment, no text, nothing. Separately, clipboard images arrive unnamed (rejected by the server's extension allowlist) or as the browser's fixed image.png placeholder (identical chip labels for every pasted screenshot).

Changes

  • Defer to text only when text/plain is present. Image-bearing clipboards without insertable text now attach through the SAME onUploadFiles path the file picker and drag-and-drop use — pasted and picked images are indistinguishable downstream, so the 50 MB cap, 20-file cap, MIME allowlist, and the upload error banner all apply identically.
  • Synthesize pasted-image-<timestamp>[-<n>].<ext> for clipboard images arriving unnamed or as image.png (same class as useScreenSnip's snip-<ts>.png precedent). Real filenames are kept; the batch suffix counts only renamed files, so a real-named sibling never produces an orphan -2.
  • The macOS Office guard is deliberately preserved: clipboards with real text/plain still paste as text. The issue's "attach AND insert on mixed payloads" variant was deliberately rejected — Office ships a junk image rendering alongside every copied text selection, and attaching it on every Office paste would be a regression of feat: opt-in Slack setup and multi-channel repositioning #2340's fix.

Tests

  • ChatInput.paste.test.tsx: Copy-Image clipboard attaches; Office text+image clipboard still does NOT; Files-only paste unchanged; filename synthesis (placeholder, unnamed, real-name kept, batch disambiguation, renamed-only counting, non-image never renamed); text-paste suite untouched.
  • ChatPagePasteImage.test.tsx (new): page-level funnel — a pasted image reaches api.uploadFiles; an OVERSIZE pasted image surfaces the same file_too_large banner a picked file gets and never reaches the server.
  • Local gates: npx tsc -b clean; vitest 11 802 passed (877 files); backend isort/flake8/mypy clean (no backend files changed).

Screenshots

Captured by the new asserting harness website/scripts/capture-clipboard-image-paste.mjs (real built SPA, gateway-free):

Pasted image attaches with synthesized name Oversize pasted image → picker-path error
paste attaches oversize error

Review

Pre-push fleet: Opus 5 — CLEAN, 0/2 blocking (both optional LOWs addressed: batch counter applied + test-pinned; harness cleanup matches sibling convention). GPT reviewer spawn timed out then was governance-barred; a contract-driven self-review against the codex charter covered guard-narrowing clipboard shapes, new File support, SVG extension surface (pre-existing posture, unchanged trust boundary), and cross-browser clipboardData — no Critical/High.

Closes #2489

…es (#2489)

The chat composer's paste handler (added in #2340) deferred to a text
paste whenever the clipboard carried text/plain OR text/html. A textarea
can only ever insert the text/plain representation, so for clipboards
with text/html but no text/plain — a browser's right-click Copy Image,
an Office chart copy — the whole gesture was a silent no-op: no
attachment, no text, nothing.

Changes:
- Defer to text only when text/plain is actually present. Image-bearing
  clipboards without insertable text now attach through the SAME
  onUploadFiles path the file picker and drag-and-drop use, so pasted
  and picked images are indistinguishable downstream (size cap, type
  allowlist, max-file cap, and the upload error banner all apply).
- Synthesize pasted-image-<timestamp>.<ext> filenames for clipboard
  images that arrive unnamed (rejected by the server's extension
  allowlist) or with the browser's fixed 'image.png' placeholder
  (identical chip labels for every pasted screenshot). Files carrying a
  real name keep it. A batch index disambiguates multiple images in one
  paste.
- Keep the macOS Office guard: clipboards with real text/plain still
  paste as text, never attaching the junk image rendering Office ships
  alongside copied text.

Tests: paste attaches (ChatInput unit + ChatPage funnel); text paste
untouched; oversize pasted image rejected with the same file_too_large
banner a picked file gets, without reaching the server; filename
synthesis (placeholder, unnamed, real-name, batch, non-image).

Evidence: temp-screenshots/chat-clipboard-image-paste/ captured by
scripts/capture-clipboard-image-paste.mjs (asserting harness).

Closes #2489
@bolichen97
bolichen97 requested a review from a team August 10, 2026 10:29
@bolichen97
bolichen97 requested a review from a team as a code owner August 10, 2026 10:29
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Aug 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — ✅ PASS

Advisory design-level review of 55e018b1f7db05d7d59a3fc719a4c670caf65086 — updated in place on each push; does not block merge.

Design-Verdict: PASS

Defer condition now matches what a textarea can actually insert (text/plain), fixing the root cause; naming and funnel reuse follow established precedents.

[DESIGN-REVIEWED] 55e018b

@github-actions

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed 55e018b1f7db05d7d59a3fc719a4c670caf65086 — this comment is updated in place on each push.

Review details

I've reviewed the diff, the surrounding paste handler, and the base AUTOSDE rules.

The core change is sound:

  • hasText narrowed to text/plain only — correct, since a <textarea> can only insert the plain-text representation; the Office-text guard still holds because Office text copies include text/plain.
  • nameClipboardImage renames only generic/unnamed image files, keeps real names, and the renamedCount (incremented only on actual rename via named !== f identity check) correctly produces an unsuffixed first name and -2 for the second, with no orphan suffix when a real-named sibling is present.
  • Non-image files fall through unrenamed; files still funnel through the shared onUploadFiles path, so the size/count/MIME caps and error banner apply identically.

The batch/identity logic, extension mapping, and new File([f], …) reconstruction all check out on the normal path. No completed consequence chain to user- or system-visible harm on any changed line.

No findings.

[OPUS-REVIEWED] 55e018b

Verdict parsed from the review's SHA-scoped output markers for commit 55e018b1f7db05d7d59a3fc719a4c670caf65086.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 55e018b1f7db05d7d59a3fc719a4c670caf65086: <one-sentence reason>

@github-actions

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 55e018b1f7db05d7d59a3fc719a4c670caf65086 and found no blocking issues.

This comment is updated in place on each push.

Review details

FINDING -- website/src/components/ChatInput.tsx:87 -- a real pasted file named image.jpg matches "image.${ext}", contradicting the promise to preserve real filenames -> Fix: only treat an empty name or "image.png" as generic.
[GPT-REVIEWED] 55e018b

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 55e018b1f7db05d7d59a3fc719a4c670caf65086: <one-sentence reason>

@github-actions

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — 🟡 CONCERNS

Advisory UX-level review of 55e018b1f7db05d7d59a3fc719a4c670caf65086 — updated in place on each push; does not block merge.

UX-Verdict: CONCERNS

Both evidence screenshots show the pasted-image chip as a broken thumbnail with a raw server path overflowing the composer — the feature's one visual proof looks broken.

Watch

  • In paste-attaches.png (and repeated in paste-oversize-error.png), the attachment chip renders the broken-image glyph plus the full alt text /home/user/.kiro/crew/uploads/abc123_pasted-image-…png, unclipped and bleeding past the chip and the composer border — contradicting the harness's own claim that the stubbed /api/file-raw makes "the chip thumbnail render as an image instead of a broken-image placeholder." If real (not a stub artifact), every pasted screenshot lands as an overflowing path string, making the marquee interaction look failed the moment it succeeds (every paste × task-doubt impact × persistent). Fix: make the harness actually serve the thumbnail and recapture, and confirm a real gateway renders the chip image.

Suggestions

  • In ChatInput.tsx's preview strip, the <img alt={path}> fallback exposes the whole uploads path; use the basename (as the adjacent open_preview_of aria-label already does) so a failed thumbnail degrades to pasted-image-….png, truncated.

[UX-REVIEWED] 55e018b

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention labels Aug 10, 2026
@kyleseaman
kyleseaman enabled auto-merge (squash) August 10, 2026 12:16
@kyleseaman
kyleseaman merged commit 8282a76 into main Aug 10, 2026
77 of 79 checks passed
@kyleseaman
kyleseaman deleted the fix/chat-clipboard-image-paste-2489 branch August 10, 2026 12:16
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Aug 10, 2026
encomjp pushed a commit to encomjp/kirocrew-customapi that referenced this pull request Aug 22, 2026
…es (kirodotdev#2489) (kirodotdev#2520)

The chat composer's paste handler (added in kirodotdev#2340) deferred to a text
paste whenever the clipboard carried text/plain OR text/html. A textarea
can only ever insert the text/plain representation, so for clipboards
with text/html but no text/plain — a browser's right-click Copy Image,
an Office chart copy — the whole gesture was a silent no-op: no
attachment, no text, nothing.

Changes:
- Defer to text only when text/plain is actually present. Image-bearing
  clipboards without insertable text now attach through the SAME
  onUploadFiles path the file picker and drag-and-drop use, so pasted
  and picked images are indistinguishable downstream (size cap, type
  allowlist, max-file cap, and the upload error banner all apply).
- Synthesize pasted-image-<timestamp>.<ext> filenames for clipboard
  images that arrive unnamed (rejected by the server's extension
  allowlist) or with the browser's fixed 'image.png' placeholder
  (identical chip labels for every pasted screenshot). Files carrying a
  real name keep it. A batch index disambiguates multiple images in one
  paste.
- Keep the macOS Office guard: clipboards with real text/plain still
  paste as text, never attaching the junk image rendering Office ships
  alongside copied text.

Tests: paste attaches (ChatInput unit + ChatPage funnel); text paste
untouched; oversize pasted image rejected with the same file_too_large
banner a picked file gets, without reaching the server; filename
synthesis (placeholder, unnamed, real-name, batch, non-image).

Evidence: temp-screenshots/chat-clipboard-image-paste/ captured by
scripts/capture-clipboard-image-paste.mjs (asserting harness).

Closes kirodotdev#2489
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.

Support pasting images directly into the chat input (clipboard paste)

2 participants