diff --git a/.changeset/f51b4ef6.md b/.changeset/f51b4ef6.md index 405bb7b1..c8431577 100644 --- a/.changeset/f51b4ef6.md +++ b/.changeset/f51b4ef6.md @@ -19,3 +19,7 @@ Codex was measured the same way and gets the same two columns. It renders nothin The strip sits behind `copyStripMargin` in App Settings under Selection & clipboard, per-device and default ON. It is a display key, deliberately absent from the `.strict()` `SettingsUpdateSchema`, and read as `!== false` because the desktop branch of `getDefaultSettings()` returns `{}`. The toggle is checked before the map is consulted. The mid-row flag is back and governs one line rather than the whole block. It reads an ordered range, so neither end of a drag can move the result, and `range.start.x > 0` excludes only the first line — the one the mousedown genuinely cut the margin off. + +Both panes of a split strip the width their own CLI declares. `_cliGutterColumns()` and `_normalisedSelectionRange()` take the session and the terminal to read, defaulting to the primary pane's, so Pane B looks its own run mode up instead of keeping a margin Pane A drops on the same keystroke. + +A detached session window (`/session/:id`) receives the gutter map too. It runs a terminal and so copies through the same path, and the injection needs no availability probe, so it sits outside the block that skips the run menu's payloads for a solo window. diff --git a/CLAUDE.md b/CLAUDE.md index 2ffabbeb..84188112 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -322,7 +322,7 @@ Frontend JS modules have `@fileoverview` with `@dependency`/`@loadorder` tags. L **Welcome "Resume Conversation" list** (terminal-ui.js): `loadHistorySessions()` fetches once and caches the corpus on `_historyAll`/`_historyCases`; every subsequent view (filter box, sort select, expand, the periodic refresh in panels-ui.js) goes through `_renderHistoryList()`, so never append rows to `#historyList` directly or re-fetch to re-sort. ⚠️ The box height is **class-driven**: expanding the list without `.history-list.expanded` leaves the collapsed `max-height` in place and just deepens a scroll well, which is the bug #260 reported (35 sessions in a ~4-row box). ⚠️ The A–Z sort keys off `_historyRowLabel()`, the SAME string the row renders (`name || firstPrompt || path`), most rows are transcript-backed and have no session name, so sorting on `name` alone silently does nothing. ⚠️ A filter implies expansion, and `_renderSearch()` hides `#historyHeader` (title + controls) as one unit while a search is active. Tests: `test/history-list-controls.test.ts`. -**Command palette + shortcut registry**: `Ctrl/Cmd/Alt+K` opens the session palette; shortcuts live in a rebindable registry (`DEFAULT_SHORTCUTS`/`getShortcutRegistry()`/`matchesShortcutEvent()` in app.js, overrides in `settings.shortcutOverrides`). ⚠️ Palette-chord keys must ALSO be swallowed in `attachCustomKeyEventHandler` (terminal-ui.js) or xterm writes the control byte (0x0B) into the PTY. ⚠️ `saveAppSettings()` rebuilds settings from the DOM, so keys edited elsewhere (`shortcutOverrides`, `showTokenCount`, `showCost`) need explicit `_prev` carry-over. ⚠️ **Smart copy (`Ctrl+C`)** lives in that same handler: with a selection it copies, with none it must `return true` **without** `preventDefault()` or the interrupt is lost. `copyTerminalSelection` is deliberately absent from `SHORTCUT_ACTIONS` because the generic capture loop preventDefaults every match it dispatches. ⚠️ **The gate tests the CLEANED selection, not the raw one** (`CodemanCopySelection.clean` in constants.js, pure; `cleanedTerminalSelection()` reads the live terminal): xterm returns whole screen ROWS and trims only never-written cells, so the spaces a full-screen TUI paints across the rest of a row are content and reach the clipboard (138 of them per line, measured in a 282-column pane). The clean drops each line's TRAILING run and nothing else. ⚠️ **A LEADING margin is stripped only when the CLI DECLARES one** (`capabilities.transcriptGutter`, a bounded integer; claude and codex each declare 2, measured on live panes, and nothing else declares any). The server publishes the map as `window.__codemanTranscriptGutter` off the capability, never as an id list, and `_activeCliGutterColumns()` looks the session's mode up in it. ⚠️ **Deriving the width from the pane is what fails, and it failed twice.** The selection's own shared indent fired on 73% of ordinary indented text (401,445 windows), because a three-row window of nested YAML shares an indent for the same reason a margin does. Painted trailing padding — a TUI writes real spaces across the unused part of a row, a shell leaves them never-written — has no false positives but is a function of pane WIDTH, since that padding exists only while a rendered line stops short of the CLI's own layout width and Claude's prose wraps to fill it: measured on one live transcript the padded-row share ran 44%, 6%, 6%, 7% and 87% at 123, 160, 198, 235 and 298 columns, so the strip silently did nothing at every ordinary window size. Taking the narrowest indent on the surrounding rows fires at every width and over-strips ~1%, because a file listing inside the transcript can be the narrowest thing on screen. ⚠️ The declared width is a **CEILING**: `clean()` strips the lesser of it and the run every selected line shares, so a block only ever shifts as a unit and a selection reaching column 0 loses nothing. Over 1,392,281 selections across six pane widths of real Claude screens it over-strips none and leaves every relative indent intact. Behind the per-device `copyStripMargin` (default **ON**, in `displayKeys`, deliberately NOT in the `.strict()` `SettingsUpdateSchema`), read as `!== false` because the desktop branch of `getDefaultSettings()` returns `{}`. Nothing reads the terminal buffer on this path. ⚠️ Alt+drag COLUMN selections are returned untouched (`_activeSelectionMode === 3`), and the padding-only clear is FEEDBACK rather than interrupt protection, since a padding-only selection now cleans to `''` and falls through to the PTY on its own. → [architecture-invariants#command-palette-and-shortcut-registry](docs/architecture-invariants.md#command-palette-and-shortcut-registry) +**Command palette + shortcut registry**: `Ctrl/Cmd/Alt+K` opens the session palette; shortcuts live in a rebindable registry (`DEFAULT_SHORTCUTS`/`getShortcutRegistry()`/`matchesShortcutEvent()` in app.js, overrides in `settings.shortcutOverrides`). ⚠️ Palette-chord keys must ALSO be swallowed in `attachCustomKeyEventHandler` (terminal-ui.js) or xterm writes the control byte (0x0B) into the PTY. ⚠️ `saveAppSettings()` rebuilds settings from the DOM, so keys edited elsewhere (`shortcutOverrides`, `showTokenCount`, `showCost`) need explicit `_prev` carry-over. ⚠️ **Smart copy (`Ctrl+C`)** lives in that same handler: with a selection it copies, with none it must `return true` **without** `preventDefault()` or the interrupt is lost. `copyTerminalSelection` is deliberately absent from `SHORTCUT_ACTIONS` because the generic capture loop preventDefaults every match it dispatches. ⚠️ **The gate tests the CLEANED selection, not the raw one** (`CodemanCopySelection.clean` in constants.js, pure; `cleanedTerminalSelection()` reads the live terminal): xterm returns whole screen ROWS and trims only never-written cells, so the spaces a full-screen TUI paints across the rest of a row are content and reach the clipboard (138 of them per line, measured in a 282-column pane). The clean drops each line's trailing run of spaces and tabs, and it takes a LEADING run only under the rule below. ⚠️ **A LEADING margin is stripped only when the CLI DECLARES one** (`capabilities.transcriptGutter`, a bounded integer; claude and codex each declare 2, measured on live panes, and nothing else declares any). The server publishes the map as `window.__codemanTranscriptGutter` off the capability, never as an id list, and `_activeCliGutterColumns()` looks the session's mode up in it. ⚠️ **Deriving the width from the pane is what fails, and it failed twice.** The selection's own shared indent fired on 73% of ordinary indented text (401,445 windows), because a three-row window of nested YAML shares an indent for the same reason a margin does. Painted trailing padding — a TUI writes real spaces across the unused part of a row, a shell leaves them never-written — has no false positives but is a function of pane WIDTH, since that padding exists only while a rendered line stops short of the CLI's own layout width and Claude's prose wraps to fill it: measured on one live transcript the padded-row share ran 44%, 6%, 6%, 7% and 87% at 123, 160, 198, 235 and 298 columns, so the strip silently did nothing at every ordinary window size. Taking the narrowest indent on the surrounding rows fires at every width and over-strips ~1%, because a file listing inside the transcript can be the narrowest thing on screen. ⚠️ The declared width is a **CEILING**: `clean()` strips the lesser of it and the run every selected line shares, so a block only ever shifts as a unit and a selection reaching column 0 loses nothing. Over 1,392,281 selections across six pane widths of real Claude screens it over-strips none and leaves every relative indent intact. Behind the per-device `copyStripMargin` (default **ON**, in `displayKeys`, deliberately NOT in the `.strict()` `SettingsUpdateSchema`), read as `!== false` because the desktop branch of `getDefaultSettings()` returns `{}`. Nothing reads the terminal buffer on this path. ⚠️ **The margin strip is NOT idempotent, so no caller may clean twice**: it takes the lesser of the declared width and the shared run, so a second pass takes up to `margin` columns more. The trailing trim alone is a fixed point, and `copyTerminalSelection()` leaned on that by re-cleaning whatever it was handed — so the Ctrl+C branch cleans to decide whether to copy and then passes the RAW selection on, and every other copy path already hands over the raw text or reads it live. Both panes of a split resolve their own width, since `_cliGutterColumns()` and `_normalisedSelectionRange()` take the session and the terminal to read. ⚠️ Alt+drag COLUMN selections are returned untouched (`_activeSelectionMode === 3`), and the padding-only clear is FEEDBACK rather than interrupt protection, since a padding-only selection now cleans to `''` and falls through to the PTY on its own. → [architecture-invariants#command-palette-and-shortcut-registry](docs/architecture-invariants.md#command-palette-and-shortcut-registry) **Per-device vs synced settings**: the `displayKeys` set in settings-ui.js is a **client-side merge policy**, not a wire filter. A display key seeds from the server only when localStorage has no value for it, which is what prevents one device overwriting another; `showPlanUsageLimits` is additionally `delete`d from the incoming payload outright. Separately, `SettingsUpdateSchema` is `.strict()` and simply **does not declare** `skin`, `showFileViewerButton`, `showCronButton`, `webglRendererEnabled`, `localEchoEnabled`, `cjkInputEnabled`, or `extendedKeyboardBar`, so sending one of those is a validation error. The rest (`showResponseViewer`, `showPlanUsageLimits`, `language`, and most `show*` keys) ARE in the schema and do persist server-side; they are per-device by client policy only. ⚠️ Adding a new per-device setting means deciding **both** questions: membership in `displayKeys`, and presence in the schema. diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 218a9909..d05e32fd 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -310,7 +310,7 @@ Copy goes through `_copyText()` (Clipboard API, then hidden-textarea + `execComm **The main terminal's four copy paths clean the selection first** (`CodemanCopySelection.clean` in constants.js, pure; `cleanedTerminalSelection()` in terminal-ui.js is the half that reads the live terminal). Those four are the `Ctrl+C` chord, right-click, the phone selection button and Auto Copy. ⚠️ Three routes still copy the RAW padded rows, all of them predating the clean: the browser's own Edit → Copy, which xterm's own `copy` listener on the terminal element serves with `selectionText` directly; a `copy-selection` shortcut the user disabled in App Settings, where nothing calls `preventDefault()` and that native listener runs; and the subagent/teammate windows, which build their own `Terminal` in panels-ui.js with no copy wiring at all. xterm hands back whole screen ROWS and its own trim drops only cells that were never written to, so the real spaces a full-screen TUI paints across the unused part of a row count as content and reach the clipboard. Measured against Claude Code in a 282-column pane, single lines arrived carrying 138 trailing spaces on top of the two-space transcript indent. The clean drops each line's trailing run, and strips a LEADING margin when the pane behind the selection turns out to have one. Four rules keep it honest: -1. **A leading strip uses the width the CLI DECLARES, never one derived from the pane.** `capabilities.transcriptGutter` (CLI registry, a bounded integer) is the whole source: claude and codex each declare 2, no other stock entry declares any, and an entry that declares nothing never has a margin taken off a copy. The server publishes the map as `window.__codemanTranscriptGutter`, built by filtering `enabledClis()` on that capability rather than by listing ids, and `_activeCliGutterColumns()` (terminal-ui.js) looks the active session's mode up in it. ⚠️ **Three ways of deriving the width from the text were built and measured, and two of them shipped looking correct.** The selection's own shared indent fired on **73%** of 401,445 three-row windows across 1,010 tracked files, because a three-row window of nested YAML shares an indent for the same reason a margin does. **Painted trailing padding** — a full-screen TUI writes real spaces across the part of a row it is not using, while a shell leaves those cells never-written for xterm to trim — has no false positives and never over-stripped, and is nonetheless a function of pane WIDTH: the padding exists only while a rendered line stops short of the CLI's own layout width, and Claude Code's prose wraps to fill it. Dragging the same two prose rows of one live transcript at five window sizes, the share of rows carrying padding measured 44%, 6%, 6%, 7% and 87% at 123, 160, 198, 235 and 298 columns, so at every ordinary size the strip did nothing at all while every test and every 282-column measurement said it worked. The **narrowest indent on the surrounding rows** fires at every width and over-strips about 1% of selections, because a file listing inside the transcript can be the narrowest thing on screen. ⚠️ The declared width is a **CEILING, not the answer**: `clean()` strips the lesser of it and the run every selected line shares, so a block can only ever shift as a unit, the relative structure inside a selection survives by construction, and a selection reaching column 0 loses nothing. That is what keeps a `git log` body at its own four spaces inside an agent's two-column gutter. Measured over 1,392,281 selections — every 1, 2, 3, 5, 10 and 20-row window of real Claude screens replayed from live PTY streams at 100, 120, 160, 198, 235 and 282 columns — it over-strips none, breaks no relative indent and alters no text, at 100% of the selections whose own indent covers the gutter. ⚠️ Nothing reads the terminal buffer on this path; the old version scanned up to ~240 rows per `Ctrl+C` to measure a width the CLI can simply state. +1. **A leading strip uses the width the CLI DECLARES, never one derived from the pane.** `capabilities.transcriptGutter` (CLI registry, a bounded integer) is the whole source: claude and codex each declare 2, no other stock entry declares any, and an entry that declares nothing never has a margin taken off a copy. The server publishes the map as `window.__codemanTranscriptGutter`, built by filtering `enabledClis()` on that capability rather than by listing ids, and `_cliGutterColumns()` (terminal-ui.js) looks that session's mode up in it, defaulting to the active session and taking an explicit id for Pane B of a split. ⚠️ **Three ways of deriving the width from the text were built and measured, and two of them shipped looking correct.** The selection's own shared indent fired on **73%** of 401,445 three-row windows across 1,010 tracked files, because a three-row window of nested YAML shares an indent for the same reason a margin does. **Painted trailing padding** — a full-screen TUI writes real spaces across the part of a row it is not using, while a shell leaves those cells never-written for xterm to trim — has no false positives and never over-stripped, and is nonetheless a function of pane WIDTH: the padding exists only while a rendered line stops short of the CLI's own layout width, and Claude Code's prose wraps to fill it. Dragging the same two prose rows of one live transcript at five window sizes, the share of rows carrying padding measured 44%, 6%, 6%, 7% and 87% at 123, 160, 198, 235 and 298 columns, so at every ordinary size the strip did nothing at all while every test and every 282-column measurement said it worked. The **narrowest indent on the surrounding rows** fires at every width and over-strips about 1% of selections, because a file listing inside the transcript can be the narrowest thing on screen. ⚠️ The declared width is a **CEILING, not the answer**: `clean()` strips the lesser of it and the run every selected line shares, so a block can only ever shift as a unit, the relative structure inside a selection survives by construction, and a selection reaching column 0 loses nothing. That is what keeps a `git log` body at its own four spaces inside an agent's two-column gutter. Measured over 1,392,281 selections — every 1, 2, 3, 5, 10 and 20-row window of real Claude screens replayed from live PTY streams at 100, 120, 160, 198, 235 and 282 columns — it over-strips none, breaks no relative indent and alters no text, at 100% of the selections whose own indent covers the gutter. ⚠️ Nothing reads the terminal buffer on this path; the old version scanned up to ~240 rows per `Ctrl+C` to measure a width the CLI can simply state. ⚠️ **The margin strip is not idempotent, so no caller may clean twice.** It takes the lesser of the declared width and the shared run, so a second pass takes up to `margin` columns more off whatever the first left. The trailing trim alone is a fixed point, and `copyTerminalSelection()` relied on that by re-cleaning whatever argument it was handed, which dedented every claude and codex copy twice on the `Ctrl+C` path — the most-used of the four, while right-click, the phone selection button and Auto Copy stayed correct because each hands over the raw selection or reads it live. The gate still tests the CLEANED string, so a padding-only selection still falls through to the PTY, and the RAW string is what travels on. A source-level test pins that branch, because it lives inside `initTerminal`'s `attachCustomKeyEventHandler` closure over a real xterm that the vm harness cannot build. 2. **A COLUMN selection is returned untouched.** Alt+drag makes one (xterm's `shouldColumnSelect` keys on `altKey` alone, and neither `Terminal` Codeman builds passes the one option, `macOptionClickForcesSelection`, that would disable it), and a rectangle's rows lining up is the whole point of the gesture. xterm exposes the mode nowhere public, so the check reads `terminal._core._selectionService._activeSelectionMode` (`SelectionMode.COLUMN` is 3) and cleans normally if a future xterm renames it. 3. **The mid-row flag reads an ORDERED range, and it governs one line.** ⚠️ `getSelectionPosition()` is reversal-aware on the pinned xterm and the review note that called it the raw mousedown anchor is out of date: `CoreBrowserTerminal.ts` reads `_selectionService.selectionStart`, whose getter returns `SelectionModel.finalSelectionStart`, and that swaps the pair when `areSelectionValuesReversed()` says so. Driving a real upward mouse drag through chromium against xterm 6.0 reports the same range as the downward drag of the same rows. `_normalisedSelectionRange()` orders the pair anyway, because the private model one layer down exposes unnormalised fields under the same two names, and a reversed pair would put the mid-row flag on the wrong end of the drag. `range.start.x > 0` then means the first selected line began mid-row and never carried the margin, so that line alone is left out of both the shared-indent ceiling and the strip. ⚠️ This is the ONE thing the mousedown column still decides and it decides it for that line only. In the dropped version it decided whether the first row joined the measurement, which changed what **every** row lost, so the same three rows produced three different clipboard results depending on where the click landed. 4. **The emptiness gate is `trim()`, not truthiness, and it still clears the selection.** A multi-row drag across padding cleans to line breaks alone, which are truthy, and a bare newline pasted into a chat composer submits it. ⚠️ The clear is FEEDBACK, not protection for the interrupt, and the comments that said otherwise were describing the pre-clean code: the `Ctrl+C` gate above now tests the CLEANED selection, so a padding-only selection left set cleans to `''` on every later press and falls through to the PTY as `0x03` anyway. What the clear buys is that a highlight which copied nothing does not linger unexplained, which is also what the toast is for. diff --git a/docs/wiki/Settings-Reference.md b/docs/wiki/Settings-Reference.md index da04800f..0c93b7a4 100644 --- a/docs/wiki/Settings-Reference.md +++ b/docs/wiki/Settings-Reference.md @@ -46,6 +46,7 @@ supervised by systemd or launchd; npm installs report as non-updatable. See | Extended Keyboard Bar | Per device | Which accessory bar phones get. Shell sessions override it while they are active. | | Wheel Scrolls Local History | Off | Keeps the wheel on the local buffer instead of forwarding it to the CLI. | | Auto Copy Selection | Off | Copies highlighted terminal text to the clipboard the moment you finish selecting it. Ctrl+C still copies on demand. | +| Trim The Pane Margin On Copy | On | Takes the left margin a full-screen agent CLI paints down its own edge off a copy, so the text pastes flush. Each CLI declares its own width, and the strip never exceeds the indent every selected line shares, so nesting is kept. Claude Code and Codex declare a margin; a shell does not. | | Normal / Bold font weight | xterm defaults | Per device, each slot from 100 to 900. The bundled JetBrains Mono renders every step, so a lighter normal weight makes Claude's bold headings stand out. Applies live to the terminal, both echo overlays and open team panes. | | WebGL Renderer | On | With a GPU-stall watchdog that falls back to DOM rendering. | | Gesture Control | Off | Camera hand tracking. Also needs `CODEMAN_GESTURE=1` on the server. | diff --git a/src/web/public/constants.js b/src/web/public/constants.js index 218d08f6..94821b3a 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -774,8 +774,6 @@ function resolveTerminalFontWeights(settings) { */ const AUTO_COPY_MAX_CHARS = 1_000_000; - - /** * What an auto-copy attempt should do at the end of a selection gesture. * diff --git a/src/web/public/index.html b/src/web/public/index.html index b602051c..eff75353 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -1794,7 +1794,7 @@
Trim the pane margin on copy - Drop the left margin a full-screen agent CLI paints down its own edge, so copied text pastes flush instead of indented. Measured off rows you did not select, and never wider than the indent every selected line shares, so nesting inside the selection is kept. Panes that paint no margin, such as a shell or Codex, are left alone. + Drop the left margin a full-screen agent CLI paints down its own edge, so copied text pastes flush instead of indented. Each CLI declares its own width, and the strip is never wider than the indent every selected line shares, so nesting inside the selection is kept. Claude Code and Codex declare a margin today; a shell, and any CLI that declares none, is left alone.
diff --git a/src/web/public/terminal-split.js b/src/web/public/terminal-split.js index e42a8b53..3c18dd81 100644 --- a/src/web/public/terminal-split.js +++ b/src/web/public/terminal-split.js @@ -188,7 +188,18 @@ if (ev.type === 'keydown' && global.app?.shouldCopyTerminalSelectionFromShortcut?.(ev)) { const raw = this.terminal?.getSelection?.() || ''; const isColumnSelection = this.terminal?._core?._selectionService?._activeSelectionMode === 3; - const selection = isColumnSelection ? raw : (global.CodemanCopySelection?.clean?.(raw) ?? raw); + // Both clean options are read for THIS pane, never the primary one: + // the gutter width comes from this.sessionId's own run mode, and the + // partial-first-line flag from this terminal's own selection range. + // Passing neither left Pane B keeping a margin Pane A dropped, on the + // same split and the same keystroke. + const range = global.app?._normalisedSelectionRange?.(this.terminal); + const selection = isColumnSelection + ? raw + : (global.CodemanCopySelection?.clean?.(raw, { + margin: global.app?._cliGutterColumns?.(this.sessionId) ?? 0, + firstLinePartial: !!range && range.start.x > 0, + }) ?? raw); if (selection.trim()) { ev.preventDefault(); void global.app._copyText?.(selection).then((ok) => { diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index 1cb695e8..5d2b398d 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -368,10 +368,18 @@ Object.assign(CodemanApp.prototype, { // part of a row selects real padding spaces, which are truthy, so testing // the raw text would spend this press on a copy of nothing and make the // user press again to interrupt. - const selection = this.cleanedTerminalSelection(); + // + // ⚠️ The gate cleans, and the copy is handed the RAW selection, because + // copyTerminalSelection cleans again on its own. The margin strip is not + // idempotent: a second pass takes up to `margin` more columns off what + // the first pass left, so passing the cleaned string through dedented a + // Claude or Codex copy twice. Every other copy path already hands over + // the raw selection or reads it live. + const raw = this.terminal?.hasSelection?.() ? this.terminal.getSelection() : ''; + const selection = this.cleanedTerminalSelection(raw); if (selection.trim()) { ev.preventDefault(); - void this.copyTerminalSelection(selection); + void this.copyTerminalSelection(raw); return false; } // Nothing worth copying. The clear is for feedback, not for the @@ -4170,11 +4178,16 @@ Object.assign(CodemanApp.prototype, { * * `text` is for the callers that already read the selection to decide whether * to copy at all (the Ctrl+C gate and the right-click handler), so the read is - * not repeated. The transform is idempotent on xterm output, so an - * already-cleaned string is an acceptable argument: a CR is consumed by the - * parser as a cursor move and never stored in a cell, so the only \r the - * selection can carry is the Windows line join, and that is what makes the - * trailing scan a fixed point. Fuzzed over 300 000 realistic selections. + * not repeated. + * + * ⚠️ **Pass the RAW selection, never an already-cleaned one.** The trailing + * trim alone is a fixed point, because a CR is consumed by the parser as a + * cursor move and never stored in a cell, so the only \r the selection can + * carry is the Windows line join. The MARGIN strip is not: it takes the + * narrower of the declared width and the run every line shares, so a second + * pass over an already-stripped block takes up to `margin` columns more. A + * caller that cleans to decide whether to copy must still hand the raw text + * to copyTerminalSelection, which cleans once on its own. * * A COLUMN selection comes back untouched. Alt+drag makes one (xterm's * shouldColumnSelect keys on altKey alone, and Codeman sets neither of the @@ -4193,7 +4206,7 @@ Object.assign(CodemanApp.prototype, { if (!clean) return raw; const range = this._normalisedSelectionRange(); return clean(raw, { - margin: this._activeCliGutterColumns(), + margin: this._cliGutterColumns(), firstLinePartial: !!range && range.start.x > 0, }); }, @@ -4210,9 +4223,13 @@ Object.assign(CodemanApp.prototype, { * UNNORMALISED fields under the same two names, and a reversed pair would * make the row window below run backwards and collapse, which would report * no margin at all for every upward drag in a deep buffer. + * + * `terminal` names which xterm to read, defaulting to the primary pane's. + * Pane B of a split owns a second terminal and passes it, because this file's + * `this` is always the primary pane. */ - _normalisedSelectionRange() { - const range = this.terminal?.getSelectionPosition?.(); + _normalisedSelectionRange(terminal) { + const range = (terminal ?? this.terminal)?.getSelectionPosition?.(); if (!range?.start || !range?.end) return null; const { start, end } = range; const reversed = end.y < start.y || (end.y === start.y && end.x < start.x); @@ -4220,8 +4237,10 @@ Object.assign(CodemanApp.prototype, { }, /** - * How many columns to take off a copy from the active session's pane: the - * transcript gutter its CLI declares, or 0 when it declares none. + * How many columns to take off a copy from one session's pane: the transcript + * gutter its CLI declares, or 0 when it declares none. `sessionId` defaults to + * the active session, and Pane B of a split passes its own, so both panes of a + * split strip the width their own CLI declares rather than Pane A's. * * ⚠️ Read from `window.__codemanTranscriptGutter`, the map the server derives * from the `transcriptGutter` CAPABILITY at render time — never an id literal @@ -4245,11 +4264,11 @@ Object.assign(CodemanApp.prototype, { * A missing map means no session gets a strip, the same direction an * unmeasured CLI takes by declaring nothing. */ - _activeCliGutterColumns() { + _cliGutterColumns(sessionId) { if (!this._copyStripMarginEnabled()) return 0; const byMode = window.__codemanTranscriptGutter; if (!byMode || typeof byMode !== 'object') return 0; - const mode = this.sessions?.get(this.activeSessionId)?.mode; + const mode = this.sessions?.get(sessionId ?? this.activeSessionId)?.mode; const columns = mode ? byMode[mode] : 0; return Number.isInteger(columns) && columns > 0 ? columns : 0; }, diff --git a/src/web/server.ts b/src/web/server.ts index 28843479..851b9670 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -1682,21 +1682,28 @@ export class WebServer extends EventEmitter { '', () => `\n` ); - // How many columns each run mode indents its transcript by, so a copy can drop - // that much. Read off `capabilities` like the payload above and never as an id - // list here, so a CLI that declares a gutter later needs no frontend change. - // Ids and small integers only, no user-settable strings, so JSON.stringify - // alone is enough (same reasoning as __codemanCliAvailable's booleans). - const gutterClis: Record = {}; - for (const entry of enabledClis()) { - const columns = entry.capabilities.transcriptGutter; - if (typeof columns === 'number') gutterClis[entry.id] = columns; - } - html = html.replace( - '', - () => `\n` - ); } + // How many columns each run mode indents its transcript by, so a copy can drop + // that much. Read off `capabilities` like the payload above and never as an id + // list here, so a CLI that declares a gutter later needs no frontend change. + // Ids and small integers only, no user-settable strings, so JSON.stringify + // alone is enough (same reasoning as __codemanCliAvailable's booleans). + // + // ⚠️ Outside the `if (!soloSessionId)` block above, unlike every other payload + // here: a detached session window (`/session/:id`) runs a terminal, so Ctrl+C + // copies there, and an absent map reads as "no session gets a strip". The + // toggle used to work in the main window and do nothing in the popup on the + // same device. This needs no availability probe, so it costs a solo window + // nothing that the run menu's own payloads would have cost it. + const gutterClis: Record = {}; + for (const entry of enabledClis()) { + const columns = entry.capabilities.transcriptGutter; + if (typeof columns === 'number') gutterClis[entry.id] = columns; + } + html = html.replace( + '', + () => `\n` + ); if (!soloSessionId && process.env.CODEMAN_GESTURE === '1') { html = html.replace('', () => `\n`); if (settings.gestureControlEnabled === true) { diff --git a/test/keyboard-shortcuts.test.ts b/test/keyboard-shortcuts.test.ts index b9a0e989..00a8a1ac 100644 --- a/test/keyboard-shortcuts.test.ts +++ b/test/keyboard-shortcuts.test.ts @@ -67,9 +67,14 @@ describe('keyboard shortcuts', () => { // selects real padding spaces, so the raw text is truthy and testing it would // spend the press on a copy of nothing — the same lost interrupt this test // guards, reached by a different door. - expect(terminalUiSource).toMatch(/const selection = this\.cleanedTerminalSelection\(\);/); + // + // The RAW selection is what travels on, because copyTerminalSelection cleans + // again on its own and the margin strip is not idempotent. Passing the + // cleaned string dedented every claude and codex copy twice; see + // test/terminal-copy-clean.test.ts for the branch's own pin. + expect(terminalUiSource).toMatch(/const selection = this\.cleanedTerminalSelection\(raw\);/); expect(terminalUiSource).toMatch(/if \(selection\.trim\(\)\) \{/); - expect(terminalUiSource).toContain('void this.copyTerminalSelection(selection);'); + expect(terminalUiSource).toContain('void this.copyTerminalSelection(raw);'); expect(appSource).toContain("id: 'copy-selection'"); }); diff --git a/test/terminal-copy-clean.test.ts b/test/terminal-copy-clean.test.ts index f4a244c3..7f5e4549 100644 --- a/test/terminal-copy-clean.test.ts +++ b/test/terminal-copy-clean.test.ts @@ -407,7 +407,7 @@ describe('copyStripMargin — the per-device toggle', () => { // toggle is checked first. There is no buffer to read: the width is declared. const h = loadHarness({ copyStripMargin: false }); select(h); - expect(h.app._activeCliGutterColumns()).toBe(0); + expect(h.app._cliGutterColumns()).toBe(0); expect(h.app.cleanedTerminalSelection()).toBe(body); }); @@ -455,7 +455,7 @@ describe('the gutter is DECLARED by the CLI, never measured off the pane', () => it('takes the declared width off a mode that declares one', () => { const { app, setSelection } = loadHarness(); setSelection(body, { mode: 'claude', from: 1, to: 2 }); - expect(app._activeCliGutterColumns()).toBe(2); + expect(app._cliGutterColumns()).toBe(2); expect(app.cleanedTerminalSelection()).toBe(flush); }); @@ -466,14 +466,14 @@ describe('the gutter is DECLARED by the CLI, never measured off the pane', () => // indents were 0, 2, 4, 6 and 8 at every one, never 1. const { app, setSelection } = loadHarness(); setSelection(' terminal:\n pane:\n gutter:\n width: 1', { mode: 'codex', from: 1, to: 4 }); - expect(app._activeCliGutterColumns()).toBe(2); + expect(app._cliGutterColumns()).toBe(2); expect(app.cleanedTerminalSelection()).toBe('terminal:\n pane:\n gutter:\n width: 1'); }); it('leaves a mode nobody has measured alone, because it declares none', () => { const { app, setSelection } = loadHarness(); setSelection(' build:\n steps:', { mode: 'opencode', from: 1, to: 2 }); - expect(app._activeCliGutterColumns()).toBe(0); + expect(app._cliGutterColumns()).toBe(0); expect(app.cleanedTerminalSelection()).toBe(' build:\n steps:'); }); @@ -488,7 +488,7 @@ describe('the gutter is DECLARED by the CLI, never measured off the pane', () => // the injection: no session gets a strip rather than every session guessing. const { app, setSelection } = loadHarness(undefined, null); setSelection(body, { mode: 'claude', from: 1, to: 2 }); - expect(app._activeCliGutterColumns()).toBe(0); + expect(app._cliGutterColumns()).toBe(0); expect(app.cleanedTerminalSelection()).toBe(body); }); @@ -496,7 +496,7 @@ describe('the gutter is DECLARED by the CLI, never measured off the pane', () => for (const bad of [0, -2, 2.5, '2', null] as unknown[]) { const { app, setSelection } = loadHarness(undefined, { claude: bad } as Record); setSelection(body, { mode: 'claude', from: 1, to: 2 }); - expect(app._activeCliGutterColumns()).toBe(0); + expect(app._cliGutterColumns()).toBe(0); } }); @@ -534,7 +534,7 @@ describe('the transcriptGutter capability, as the registry and server carry it', // The frontend looks the mode up in that map; the helper holds no id itself. const terminalUi = read('terminal-ui.js'); const helper = terminalUi.slice( - terminalUi.indexOf('_activeCliGutterColumns() {'), + terminalUi.indexOf('_cliGutterColumns(sessionId) {'), terminalUi.indexOf('async copyTerminalSelection') ); expect(helper).toContain('window.__codemanTranscriptGutter'); @@ -606,6 +606,56 @@ describe('copyTerminalSelection — what reaches the clipboard', () => { expect(terminal.clearSelection).toHaveBeenCalledTimes(1); }); }); + + // Every case above runs on the harness default mode, which declares no gutter, + // so none of them can see a margin stripped twice. These two run on a mode that + // declares one. + it('takes the declared width off a claude pane exactly once', () => { + const { app, setSelection } = loadHarness(); + setSelection(' fix(terminal): trim it', { mode: 'claude', from: 1, to: 2 }); + return app.copyTerminalSelection().then(() => { + // 2 gutter columns off a body that carries 4 of its own. + expect(app._copyText).toHaveBeenCalledWith(' fix(terminal): trim it'); + }); + }); + + it("keeps a nested block's own indentation on a claude pane", () => { + const { app, setSelection } = loadHarness(); + setSelection(' build:\n steps:\n - run: npm ci', { mode: 'claude', from: 1, to: 4 }); + return app.copyTerminalSelection().then(() => { + expect(app._copyText).toHaveBeenCalledWith(' build:\n steps:\n - run: npm ci'); + }); + }); +}); + +describe('the margin strip is not idempotent, so no caller may clean twice', () => { + // The trailing trim is a fixed point, and copyTerminalSelection leaned on that + // by re-cleaning whatever it was handed. The margin strip broke it: it takes + // the narrower of the declared width and the run every line shares, so a + // second pass takes up to `margin` columns more. Ctrl+C cleaned to decide + // whether to copy and then passed the CLEANED string on, which dedented every + // claude and codex copy twice on the most-used copy path of the four. + it('takes more off a block that has already been stripped', () => { + const h = loadHarness(); + const clean = h.windowRef.CodemanCopySelection.clean; + const once = clean(' fix(terminal): trim it', { margin: 2 }); + expect(once).toBe(' fix(terminal): trim it'); + expect(clean(once, { margin: 2 })).toBe(' fix(terminal): trim it'); + }); + + it('is pinned in the Ctrl+C branch, which gates on the clean and copies the raw', () => { + // The branch lives inside initTerminal's attachCustomKeyEventHandler closure, + // over a real xterm this harness cannot build, so the rule is pinned at the + // source rather than driven by a keystroke. + const terminalUi = read('terminal-ui.js'); + const branch = terminalUi.slice( + terminalUi.indexOf('if (this.shouldCopyTerminalSelectionFromShortcut?.(ev)) {'), + terminalUi.indexOf('// Session-sidebar toggle chord') + ); + expect(branch).toContain('const selection = this.cleanedTerminalSelection(raw);'); + expect(branch).toContain('void this.copyTerminalSelection(raw);'); + expect(branch).not.toContain('this.copyTerminalSelection(selection)'); + }); }); describe('_flushAutoCopySelection — cleaned text is what Auto Copy handles', () => {