Merge pull request #394 from irisitymichaelgrundberg/fix/ctrl-v-pastes-twice

fix(paste): handle only the first paste event the Ctrl+V trap receives

CLAUDE.md

@@ -253,6 +253,8 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph
 
 **Auto Copy (copy-on-select)** (`autoCopySelection`, per-device, default OFF): a finished terminal selection lands on the clipboard with no keystroke. ⚠️ It fires at the END of a gesture, never in `onSelectionChange` (that callback runs per cell crossed, so copying there is one clipboard write per mouse move); it only ARMS `_autoCopyPending`, and a document-level `mouseup` listener flushes. ⚠️ The flush is SYNCHRONOUS inside the handler because both clipboard paths need user activation (Firefox gates `navigator.clipboard.writeText` on it, and the plain-HTTP `execCommand` fallback must run in the gesture's own task); a timer or a wait for `onSelectionChange` loses it, invisibly in Chrome. ⚠️ Touch needs its OWN calls from `_endTouchSelectionGesture()`/`_selectTouchSelectionLine()`: that path `preventDefault()`s its touchend, so no mouseup ever arrives and the toggle would be dead on phones. ⚠️ Unlike `copyTerminalSelection()` it must NOT clear the selection (the text would vanish under the cursor that highlighted it) and must NOT focus the terminal (that opens the on-screen keyboard over it); focus is RESTORED to whatever held it, which only matters for the `execCommand` fallback. Guards are pure in `decideAutoCopy()` (constants.js): off, blank/whitespace-only, and a 1M-char cap (an autoscrolling drag can sweep the whole 50k-line scrollback), refused rather than truncated with a toast pointing at Ctrl+C. Silent on success except once per page load; failures toast, throttled 10s. Tests: `test/terminal-auto-copy.test.ts`.
 
+**Ctrl+V paste trap** (`image-input.js`): `Ctrl+V` routes through `_handleImagePaste()`, which appends a hidden `contenteditable` trap, focuses it, and reads the clipboard out of the paste event that lands there. Images upload and their saved paths are typed into the session; text goes through `terminal.paste()` so bracketed-paste markers survive. ⚠️ **The trap must consume exactly ONE paste event.** Two routes deliver one for a single keypress and Firefox fires both: `document.execCommand('paste')` dispatches a trusted event and still returns `false`, because the trap cancels it, while Chromium refuses that command and dispatches nothing; separately, the keydown's own default action delivers a paste to the now-focused trap, because returning `false` from the custom key handler never cancels the DOM event (see smart copy above). Measured on a live install: Firefox two events per keypress, Chromium and WebKit one. Handling both wrote the clipboard to the PTY twice, and right-click → Paste stayed correct because it carries no keydown. ⚠️ Removing the `execCommand('paste')` call would also end the doubling, and all three engines still deliver one event without it, but it stays for the mobile engines a desktop measurement cannot reach: where a browser aims the key's default action at the element focused when the keydown began, the command is the only route into the trap, and the trap is the only place image blobs are read. The one-shot flag lives on the trap rather than on a browser check, so any count produces one insert. Tests: `test/image-paste-trap.test.ts`. → [architecture-invariants#terminal-paste-ctrlv](docs/architecture-invariants.md#terminal-paste-ctrlv)
+
 **Terminal scrollback strip + wheel/touch forwarding** (#205): codex/claude/gemini get the FULL strip (alt-screen, `3J`, mouse DECSETs); tmux-backed shell/opencode/antigravity/omp get a NARROW strip (alt-screen toggles only — it removes tmux's own attach-time `smcup`, which otherwise parks xterm in the scrollback-less alt buffer and turns the wheel into arrow keys). ⚠️ Gated on `useMux`: direct-PTY fallback sessions must keep the alt screen for vim/less/htop. Wheel AND touch forward to the CLI transcript for **claude ≥ 2.1.187 ONLY** at ANY scroll position (snap-to-bottom first); Shift+wheel and the `terminalWheelLocalScrollback` setting stay local. ⚠️ Codex was in that list and must never go back without a fresh measurement: codex-cli 0.147.0 ignores SGR wheel reports entirely (`mouse_any_flag=0`, inline viewport, transcript pushed into terminal scrollback), so forwarding produced a dead wheel (#227 follow-up). `_wheelScrollLines()` reads `ev.deltaMode` (Firefox = LINE units). ⚠️ When that gate is FALSE on a claude session whose local buffer is hollow (`baseY === 0`), the gesture becomes coalesced PageUp/PageDown key sends (`_maybePageCliTranscript`) instead of a no-op; ⚠️ and `getClaudeCliVersion()` must never cache a FAILED probe (one timeout used to disable forwarding process-wide until restart). ⚠️ **A click is hand-reported to the CLI only while the CLI actually has mouse tracking on.** The full strip removes the mouse DECSETs, so xterm's `mouseTrackingMode` is permanently `none` there and the browser hand-encodes SGR reports (`_sendSyntheticSgrTap`); without state it did that on EVERY click, so a stripped-mode pane running a plain shell (CLI exited, or a shell started inside a claude-mode session) received reports it never asked for and printed them as literal text (`[<0;88;20M`), garbling the next typed line. `_recordStrippedMouseMode()` (session.ts) records what the strip removes, `toState()` publishes `cliMouseTracking`, and `_shouldReportMouseToCli()` gates all three report sites on it. Only 1000/1001/1002/1003 count (1005/1006 are encodings, 1007 is alt-scroll), and the change broadcasts UNdebounced since a dialog can be clicked inside the 500ms window. `_logScrollRouting()` prints the routing decision and its inputs once per session — read it before diagnosing a scroll report. → [architecture-invariants#terminal-scrollback-strip-flavors-and-wheeltouch-forwarding](docs/architecture-invariants.md#terminal-scrollback-strip-flavors-and-wheeltouch-forwarding)
 **Detached start + service install** (issue #231): `codeman web -d` relaunches the SAME entry script with `detached:true` (setsid), so there is no controlling terminal and no shell job entry. ⚠️ `nohup` is NOT what makes this work: Node re-arms SIGHUP to its default disposition even when it inherits "ignore", and `cli.ts` handles SIGHUP with a graceful shutdown, so a delivered HUP still stops the server. ⚠️ Both `-d` and `service install` must REFUSE when a server is already up on this data dir (pidfile check + `/api/status` probe): a second instance on the shared tmux socket attaches PTYs to the first one's live sessions. ⚠️ Neither may report success it has not observed — the parent polls `/api/status` until the child answers or dies, since `launchctl load` and a clean spawn are both silent about a server that starts and immediately exits. `--stop` verifies the pid still LOOKS like a Codeman server (`ps -o command=`) before signalling, because pids get recycled. Unit/label names live in `config/service-names.ts` so install.sh, `detectSupervisor()` and `service install` cannot drift into supervising two copies; they are instance-scoped, and identical to the historical names for the default instance. `service install` bakes the installing shell's PATH into the unit (launchd gives a job `/usr/bin:/bin:/usr/sbin:/sbin`, which finds neither a Homebrew/nvm `node` nor `tmux`/`claude`) and never writes `CODEMAN_PASSWORD` into it. → [architecture-invariants#detached-start-and-service-install](docs/architecture-invariants.md#detached-start-and-service-install)
 

README.md

@@ -691,6 +691,7 @@ The web UI remains the primary surface; see **[docs/tui.md](docs/tui.md)** for t
 | `Ctrl+Shift+{` / `Ctrl+Shift+}` | Move active tab left / right                                  |
 | `Ctrl/Cmd+C`                    | Copy selection, or interrupt when nothing is selected         |
 | `Ctrl+Shift+C`                  | Copy selection (never interrupts)                             |
+| `Ctrl/Cmd+V`                    | Paste, or upload a clipboard image and paste its path          |
 | `Ctrl/Cmd+L`                    | Clear terminal                                                |
 | `Ctrl+Shift+R`                  | Restore terminal size                                         |
 | `Ctrl+Shift+V`                  | Toggle voice input                                            |

docs/architecture-invariants.md

@@ -302,6 +302,19 @@ Copy goes through `_copyText()` (Clipboard API, then hidden-textarea + `execComm
 
 Feedback is silent on success except ONCE per page load (a feature that works by doing nothing visible cannot otherwise be told from a dead toggle); failures and refusals toast, throttled to 10s so a permanently blocked clipboard cannot paint a toast on every drag. The setting is per-device on both counts required by the settings rule: it is in `displayKeys` AND absent from the `.strict()` `SettingsUpdateSchema` (clipboard access differs by device and by origin, and the plain-HTTP LAN install has no `navigator.clipboard` at all). Tests: `test/terminal-auto-copy.test.ts`.
 
+### Terminal paste (Ctrl+V)
+
+**The Ctrl+V paste trap consumes exactly ONE paste event.** `_handleImagePaste()` (image-input.js) appends a hidden `contenteditable` div, focuses it, and reads the clipboard out of the paste event that lands there. An image becomes an upload whose saved path is typed into the session; text goes through `terminal.paste()`, so the bracketed-paste markers survive. Two independent routes deliver that event for one keypress, and a browser may fire both:
+
+1. **`document.execCommand('paste')`**, which the function calls itself. ⚠️ Its return value proves nothing about whether it fired. Firefox dispatches a trusted `paste` event carrying the real clipboard and still returns `false`, because the trap cancels the event and the command therefore never completes. Chromium refuses the command outright and dispatches nothing.
+2. **The keydown's own default action.** The `Ctrl+V` branch in `attachCustomKeyEventHandler` returns `false`, which stops xterm from evaluating the key into `^V` but does not cancel the DOM event, for the same reason rule 1 of smart copy above spells out. `trap.focus()` has already run by then, so the browser sends its own paste to the trap as well.
+
+Measured against a live install, one `Ctrl+V` each: Firefox delivers two paste events, Chromium and WebKit one. Handling both wrote the clipboard text to the PTY twice, which is why `Ctrl+V` pasted twice while right-click → Paste pasted once. That menu path involves no keydown, so route 2 cannot exist for it.
+
+⚠️ **Route 2 alone is enough in all three engines, so route 1 is redundant where it can be measured.** Strip the `execCommand('paste')` call out and each of the three still delivers exactly one paste event to the trap, Firefox included. The call is kept anyway, because the trap technique was written for the mobile engines that a desktop measurement cannot reach, and a browser that resolves the key's default action against the element focused when the keydown began would send its paste to xterm's textarea instead. Text paste survives that on xterm's own `handlePasteEvent`; image paste does not, since the trap's listener is the only place clipboard image blobs are read. Removing the call is therefore a decision about mobile coverage, not a cleanup.
+
+The guard is a one-shot flag on each trap rather than a browser test or a reading of `execCommand`'s return value, so any count of events produces one insert. Tests: `test/image-paste-trap.test.ts`.
+
 ### Settings surface: App Settings, Session Options, Add Case
 
 **One visual language, three modals.** `#appSettingsModal`, `#sessionOptionsModal` and `#createCaseModal` share the `set-*` surface (left rail, sections of grouped row cards, label + description on the left, control pinned right) through a single `:is(#appSettingsModal, #sessionOptionsModal, #createCaseModal)` scope in `styles.css`. An `:is()` list takes the specificity of its **most specific argument**, and all three arguments are ids, so every rule kept exactly the weight it had when the block was `#appSettingsModal`-only: nothing downstream shifted in the cascade. That property is what let the surface absorb Session Options and then Add Case in two separate commits without a cascade audit each time.

docs/wiki/Input-And-Voice.md

@@ -14,6 +14,7 @@ works, slash commands included.
 | `Shift+Enter` / `Ctrl+Enter` | Newline without sending.                       |
 | `Ctrl+C`                     | Copy if text is selected, otherwise interrupt. |
 | `Ctrl+Shift+C`               | Copy, never interrupts.                        |
+| `Ctrl+V`                     | Paste. A clipboard image uploads instead.      |
 | `Ctrl+L`                     | Clear the terminal.                            |
 
 ### Exactly-once delivery

docs/wiki/Keyboard-Shortcuts.md

@@ -25,6 +25,7 @@ Press `Ctrl+?` in the app for the same list in a floating overlay.
 | `Ctrl+Enter`            | Same.                                                            |
 | `Ctrl+C`                | Copy the selection, or interrupt when nothing is selected.       |
 | `Ctrl+Shift+C`          | Copy the selection. Never interrupts.                            |
+| `Ctrl+V`                | Paste. An image on the clipboard uploads and pastes its file path instead. |
 | `Ctrl+L`                | Clear the terminal.                                              |
 | `Ctrl+Shift+R`          | Restore terminal size.                                           |
 | `Ctrl` `+` / `Ctrl` `-` | Font size.                                                       |

src/web/public/image-input.js

@@ -60,9 +60,22 @@ Object.assign(CodemanApp.prototype, {
     document.body.appendChild(trap);
     trap.focus();
 
+    // One Ctrl+V can deliver TWO paste events to this trap. The
+    // execCommand('paste') below fires one wherever the browser honours that
+    // command, and the key's own default action fires another, because xterm's
+    // custom key handler returns false without cancelling the keydown. Handling
+    // both sends the clipboard text to the PTY twice, which is the "Ctrl+V
+    // pastes twice, right-click Paste does not" report: the context-menu paste
+    // has no keydown, so it only ever produces one event. The trap therefore
+    // accepts the first paste and drops every later one.
+    var pasteConsumed = false;
+
     // Listen for the paste event on our trap
     trap.addEventListener('paste', function(e) {
       e.stopPropagation();
+      e.preventDefault();
+      if (pasteConsumed) return;
+      pasteConsumed = true;
 
       // Check for images in clipboard items
       var imageFiles = [];
@@ -84,7 +97,6 @@ Object.assign(CodemanApp.prototype, {
       }, 0);
 
       if (imageFiles.length > 0) {
-        e.preventDefault();
         self._uploadAndInsertImages(imageFiles);
       } else {
         // No image -- route text through xterm's paste() so bracketed-paste
@@ -94,7 +106,6 @@ Object.assign(CodemanApp.prototype, {
         // indistinguishable from typed input, weakening the CLI's
         // prompt-injection defenses.
         var text = e.clipboardData ? e.clipboardData.getData('text/plain') : '';
-        e.preventDefault();
         if (text && self.terminal) self.terminal.paste(text);
       }
     });

test/image-paste-trap.test.ts

@@ -0,0 +1,176 @@
+/**
+ * @fileoverview Unit tests for the Ctrl+V paste trap in image-input.js.
+ *
+ * `_handleImagePaste()` appends a hidden contenteditable div (the "paste
+ * trap"), focuses it, and reads the clipboard out of the paste event the
+ * browser delivers there. Two things can deliver that event for a single
+ * Ctrl+V: the `document.execCommand('paste')` the function issues itself, and
+ * the keydown's own default action, which still runs because xterm's custom key
+ * handler returns false without cancelling the event. A browser that honours
+ * execCommand('paste') therefore fires the trap's listener twice, and the
+ * clipboard text used to reach the PTY twice with it — while right-click →
+ * Paste, which involves no keydown, stayed correct.
+ *
+ * Loads the browser module into a vm sandbox with a fake document, so the tests
+ * drive the trap's listener directly rather than through a real browser.
+ */
+import { readFileSync } from 'node:fs';
+import { resolve } from 'node:path';
+import vm from 'node:vm';
+import { describe, expect, it } from 'vitest';
+
+interface TrapListener {
+  (e: Record<string, unknown>): void;
+}
+
+interface FakeTrap {
+  contentEditable: string;
+  style: { cssText: string };
+  parentNode: unknown;
+  focus: () => void;
+  addEventListener: (ev: string, fn: TrapListener) => void;
+}
+
+interface Harness {
+  /** Fire a paste event on the trap the last _handleImagePaste() call created. */
+  firePaste: (payload: { text?: string; images?: string[] }) => void;
+  /** Text handed to xterm's terminal.paste(), one entry per call. */
+  pastedText: string[];
+  /** Image batches handed to _uploadAndInsertImages(), one entry per call. */
+  uploadedBatches: Array<Array<{ type: string }>>;
+  /** How many trap divs are still attached to the fake body. */
+  attachedTraps: () => number;
+  runTimers: () => void;
+}
+
+function loadPasteHarness(): Harness {
+  const traps: FakeTrap[] = [];
+  const listeners: TrapListener[] = [];
+  const attached = new Set<FakeTrap>();
+  const timers: Array<() => void> = [];
+
+  const documentObj = {
+    createElement: (): FakeTrap => {
+      const trap: FakeTrap = {
+        contentEditable: '',
+        style: { cssText: '' },
+        parentNode: null,
+        focus: () => {},
+        addEventListener: (ev: string, fn: TrapListener) => {
+          if (ev === 'paste') listeners.push(fn);
+        },
+      };
+      traps.push(trap);
+      return trap;
+    },
+    body: {
+      appendChild: (el: FakeTrap) => {
+        attached.add(el);
+        el.parentNode = documentObj.body;
+      },
+      removeChild: (el: FakeTrap) => {
+        attached.delete(el);
+        el.parentNode = null;
+      },
+    },
+    // A browser that honours the command fires the trap's paste listener from
+    // here as well; the tests model that by firing the listener twice.
+    execCommand: () => true,
+    getElementById: () => null,
+  };
+
+  const context = vm.createContext({
+    window: {},
+    document: documentObj,
+    setTimeout: (fn: () => void) => {
+      timers.push(fn);
+      return timers.length;
+    },
+    clearTimeout: () => {},
+    console,
+  });
+
+  vm.runInContext('class CodemanApp {}', context);
+  const src = readFileSync(resolve(import.meta.dirname, '../src/web/public/image-input.js'), 'utf8');
+  vm.runInContext(src, context, { filename: 'image-input.js' });
+  const CodemanApp = vm.runInContext('CodemanApp', context) as new () => Record<string, unknown>;
+
+  const pastedText: string[] = [];
+  const uploadedBatches: Array<Array<{ type: string }>> = [];
+  const app = new CodemanApp();
+  app.activeSessionId = 'session-1';
+  app.terminal = {
+    paste: (text: string) => pastedText.push(text),
+    focus: () => {},
+  };
+  app._uploadAndInsertImages = (files: Array<{ type: string }>) => {
+    uploadedBatches.push(Array.from(files));
+  };
+  app.showToast = () => {};
+
+  (app._handleImagePaste as () => void).call(app);
+
+  return {
+    firePaste({ text = '', images = [] }) {
+      const items = images.map((type) => ({ type, getAsFile: () => ({ type }) }));
+      const event = {
+        clipboardData: {
+          items,
+          getData: () => text,
+        },
+        preventDefault: () => {},
+        stopPropagation: () => {},
+      };
+      for (const fn of listeners) fn(event);
+    },
+    pastedText,
+    uploadedBatches,
+    attachedTraps: () => attached.size,
+    runTimers: () => {
+      const pending = timers.splice(0, timers.length);
+      for (const fn of pending) fn();
+    },
+  };
+}
+
+describe('Ctrl+V paste trap', () => {
+  it('sends clipboard text to the terminal once for a single paste event', () => {
+    const h = loadPasteHarness();
+
+    h.firePaste({ text: 'hello world' });
+
+    expect(h.pastedText).toEqual(['hello world']);
+  });
+
+  it('ignores a second paste event for the same Ctrl+V', () => {
+    const h = loadPasteHarness();
+
+    // execCommand('paste') and the uncancelled keydown's default action both
+    // land on the same trap in browsers that honour the command.
+    h.firePaste({ text: 'hello world' });
+    h.firePaste({ text: 'hello world' });
+
+    expect(h.pastedText).toEqual(['hello world']);
+  });
+
+  it('uploads a pasted image once when the trap sees two paste events', () => {
+    const h = loadPasteHarness();
+
+    h.firePaste({ images: ['image/png'] });
+    h.firePaste({ images: ['image/png'] });
+
+    expect(h.uploadedBatches).toHaveLength(1);
+    expect(h.uploadedBatches[0]).toEqual([{ type: 'image/png' }]);
+    expect(h.pastedText).toEqual([]);
+  });
+
+  it('removes the trap and hands focus back after the paste it accepted', () => {
+    const h = loadPasteHarness();
+
+    h.firePaste({ text: 'hello world' });
+    expect(h.attachedTraps()).toBe(1);
+
+    h.runTimers();
+    expect(h.attachedTraps()).toBe(0);
+  });
+});