diff --git a/.changeset/f51b4ef6.md b/.changeset/f51b4ef6.md new file mode 100644 index 00000000..c8431577 --- /dev/null +++ b/.changeset/f51b4ef6.md @@ -0,0 +1,25 @@ +--- +'aicodeman': patch +--- + +Terminal copy: take the transcript gutter off the clipboard, using the width the CLI declares. + +Copying a paragraph out of a Claude Code pane put the pane's own two-column transcript gutter on the clipboard, so every pasted line arrived indented. #451 shipped the trailing half of the copy clean and deliberately left the leading half out, because deriving the width from the selection fires on 73% of ordinary indented text and cannot tell a margin from content. + +The width is now declared rather than derived. `capabilities.transcriptGutter` on the CLI registry is a bounded integer; claude and codex each declare 2, measured on live panes, and no other stock entry declares any, so a CLI whose transcript layout nobody has measured is never touched. The server publishes the map as `window.__codemanTranscriptGutter`, built by filtering `enabledClis()` on the capability rather than by listing ids, and the copy path looks the active session's mode up in it. It reads no terminal buffer at all. + +The declared width is a ceiling, not the answer: `clean()` strips the lesser of it and the run every selected line shares. A block can therefore only shift as a unit, the 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-space indent inside an agent's two-column gutter. + +Two derived versions were built and measured first, and both are worth recording because both looked correct. Painted trailing padding — a full-screen TUI writes real spaces across the unused part of a row, a shell leaves them never-written — has no false positives and never over-stripped, and is a function of pane WIDTH: 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. Dragging the same two prose rows of one live transcript at five window sizes, the share of padded rows ran 44%, 6%, 6%, 7% and 87% at 123, 160, 198, 235 and 298 columns, so the strip silently did nothing at every ordinary size. Taking the narrowest indent on the surrounding rows fires at every width and over-strips about 1%, because a file listing inside the transcript can be the narrowest thing on screen. + +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 — the declared width over-strips none, breaks no relative indent and alters no text, and serves 100% of the selections whose own indent covers the gutter. Verified end to end in a browser with a real mouse drag and a real Ctrl+C at 123, 160, 198, 235 and 298 columns: a Claude pane pastes flush at every one, a shell pane is untouched at every one. + +Codex was measured the same way and gets the same two columns. It renders nothing like Claude — it draws boxes narrower than the pane and pushes its transcript into ordinary scrollback — so its layout was checked on its own live answer: the `•`/`›`/`⚠` markers sit in the gutter, prose continuations sit at 2, and a nested YAML block the model wrote rendered at 2/4/6/8 for its own 0/2/4/6. Replayed at 100, 120, 160, 198, 235 and 282 columns its indents were 0, 2, 4, 6 and 8 at every one and never 1. Copying that YAML out of a live Codex pane now yields 0/2/4/6: the gutter gone, the block's own nesting intact and paste-ready. + +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 1c2577e0..522490e2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -324,7 +324,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 shared LEADING indent is deliberately NOT stripped**, and that is a decision, not a gap: it was built, measured and dropped before #451 merged, because it fired on 73% of real non-TUI text (401,445 windows sampled) and no width threshold separates a TUI margin from content (a claude pane's own margins are 2 and 5 columns; the commonest non-TUI shared run is 4). Do not re-add it without reading the rule in architecture-invariants. ⚠️ 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 a3344e26..f35c624c 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -312,11 +312,12 @@ Invariants: Copy goes through `_copyText()` (Clipboard API, then hidden-textarea + `execCommand`), not raw `navigator.clipboard`, because `install.sh`'s LAN option serves plain HTTP where `navigator.clipboard` is undefined; the fallback steals focus, so the terminal is refocused afterwards. Related: xterm registers its own `copy` listener on the terminal element gated on `hasSelection()`, which is why right-click → Copy has always worked. Selection itself is unavailable on touch devices by design (`user-select: none` on the terminal subtree), and in `shell`/`opencode`/`antigravity` tabs the TUI owns the mouse, so selecting there needs Shift+drag. Tests: `test/terminal-copy-selection.test.ts` (gate + wiring invariants), `test/terminal-copy-shortcut.test.ts` (browser, real key presses). -**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. Three rules keep it honest: +**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. **Trailing padding only. A shared LEADING indent is deliberately NOT stripped**, and that is a decision rather than an omission: it was built, measured and dropped before #451 merged. It looks like the mirror image of the trailing trim and is not, because no native terminal does it and the transform cannot tell a TUI's margin from content that is genuinely indented. Measured over 401,445 three-row windows across 1,010 tracked files in this repo it fired on **73%** of them (92% inside a YAML workflow, 76% over `git log` output, 48% in a TypeScript source), and no width threshold separates the two because they are the same widths: a live Claude Code pane's own margins measure 2 and 5 columns while the most common non-TUI shared run is 4, sitting between them. The failure modes are what settle it. A wrong trailing trim costs nothing; a wrong dedent silently deletes information that was on screen, with no signal and nothing in the clipboard to hint at it, and it is wrong on `git log` bodies, on indented code read out of `cat` (semantic in Python), on `git diff` context rows where the leading space is the marker, and on stack traces. ⚠️ It also could not be made self-consistent cheaply: whether the first row joined the measurement depended on the mousedown COLUMN, which the user never sees, so one block of three rows produced three different clipboard results, and the flag read `getSelectionPosition().start`, which is the mousedown anchor xterm never normalises, so dragging UP through a block read it off the bottom row (the PR's test stub hardcoded a downward drag, so its suite could not express the case). If it is ever revisited, the one qualification that measured clean is **painted trailing padding** (a full-screen TUI writes real spaces across every row, while a shell pane leaves those cells never-written for xterm to trim): zero false positives over all 401,445 windows, no new plumbing. It still mangles a `git log` body sitting inside an agent's own gutter, which is why it was not taken. +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 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. +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. Tests: `test/terminal-copy-clean.test.ts`. 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/config/cli-registry/schema.ts b/src/config/cli-registry/schema.ts index d1249384..68dee0d8 100644 --- a/src/config/cli-registry/schema.ts +++ b/src/config/cli-registry/schema.ts @@ -294,6 +294,23 @@ const capabilitiesSchema = z effort: z.boolean(), agentSkillInjection: z.boolean(), statusLineTelemetry: z.boolean(), + // How many columns this CLI indents its transcript body by, so a copy can take + // that much off the clipboard. Bounded, because it is the whole strip: a copy + // never removes more than this, nor more than every selected line shares. + // + // ⚠ DECLARED, not measured off the pane, and two measured attempts are why. + // Asking whether the pane painted spaces across the unused part of each row + // separates a TUI from a shell perfectly where it fires and never + // over-stripped, but it is a function of pane WIDTH: that 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 — the share of padded rows on one + // live transcript ran 44%, 6%, 6%, 7% and 87% at 123, 160, 198, 235 and 298 + // columns, so the strip did nothing at any ordinary size. Taking the + // narrowest indent on screen instead fires everywhere and over-strips, since + // a file listing inside the transcript can be the narrowest thing on it. + // A declared width cannot do either. Absent means no strip, so a CLI whose + // transcript layout nobody has measured is never touched. + transcriptGutter: z.number().int().min(1).max(8).optional(), workDetect: z .object({ promptGlyph: z.string().min(1).max(8), diff --git a/src/config/cli-registry/stock.ts b/src/config/cli-registry/stock.ts index 9ca275cf..c0b7b58d 100644 --- a/src/config/cli-registry/stock.ts +++ b/src/config/cli-registry/stock.ts @@ -210,6 +210,10 @@ const CLAUDE: CliEntry = { }, capabilities: { external: false, + // Claude indents its transcript body two columns and puts its own ●/✻/❯ markers + // in them, so a copy can drop two and paste flush. The only entry that declares + // this, because it is the only one whose gutter has been measured. + transcriptGutter: 2, // The historical hard-coded pair, now stated as data. `workingLine` matches both the // `✻ Actualizing… (39s · ↓ 2.0k tokens)` status line and the bare `esc to interrupt` // footer, because tmux repaints partially and only one of the two may land in a chunk. @@ -529,6 +533,12 @@ const CODEX: CliEntry = { // braille spinner, and it never prints `esc to interrupt` at rest, so that phrase // alone separates a running turn from an idle one. workDetect: { promptGlyph: '›', workingLine: '[Ee]sc to interrupt' }, + // Two columns, like claude's, measured on a live 0.154.0 answer: the `•`/`›`/`⚠` + // markers sit in the gutter, prose continuations sit at 2, and a nested YAML block + // the model wrote rendered at 2/4/6/8 for its own 0/2/4/6. Replayed at 100, 120, + // 160, 198, 235 and 282 columns the indents were 0, 2, 4, 6 and 8 at every one, + // never 1, so the width is not a function of the pane. + transcriptGutter: 2, transcript: 'codex-rollout', altScreen: 'strip-full', echo: { policy: 'predict', anchor: { kind: 'cursor' }, predictProfile: 'codex' }, diff --git a/src/config/cli-registry/types.ts b/src/config/cli-registry/types.ts index 57be0c79..abe77677 100644 --- a/src/config/cli-registry/types.ts +++ b/src/config/cli-registry/types.ts @@ -345,6 +345,29 @@ export interface CliCapabilities { /** Source of a regex matching the status line this CLI draws while a turn runs. */ workingLine: string; }; + /** + * How many columns this CLI indents its transcript body by, so a copy taken from its + * pane can drop that much and paste flush. Claude Code indents two and puts its own + * markers in those columns. + * + * ⚠ DECLARED rather than measured off the pane, and two measured attempts are why. + * Asking whether the pane painted real spaces across the unused part of each row + * separates a TUI from a shell perfectly where it fires and never over-stripped; it + * is also a function of pane WIDTH, because that 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. On one live transcript the share of padded rows ran 44%, 6%, 6%, + * 7% and 87% at 123, 160, 198, 235 and 298 columns, so at any ordinary window size + * the strip silently did nothing. Taking the narrowest indent on the surrounding + * rows instead fires at every width and over-strips on roughly 1% of selections, + * because a file listing inside the transcript can be the narrowest thing on screen. + * + * A declared width can do neither. The strip is the lesser of this and what every + * selected line shares, so a block can only ever shift as a unit, and it can never + * shift further than the CLI itself says its gutter is. + * + * Absent means no strip at all, the same fail-safe direction `workDetect` takes. + */ + transcriptGutter?: number; /** No direct-PTY fallback: the CLI must run inside tmux (secrets ride tmux setenv). */ requiresMux: boolean; /** diff --git a/src/web/public/constants.js b/src/web/public/constants.js index 17b4b6a7..94821b3a 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -819,36 +819,32 @@ function decideAutoCopy({ enabled, text, lastCopied, pending } = {}) { // _selectTouchSelectionLine already treats those cells as padding. This is that // same rule for the mouse and keyboard paths, which never had it. // -// ⚠ Trailing padding ONLY. A shared LEADING indent is deliberately left alone, -// and this note is here so the idea is not re-derived: it was built, measured -// and dropped before merge. Removing the longest leading run every selected row -// shares looks like the mirror image of the trailing trim and is not, because -// no native terminal does it and the transform cannot tell a TUI's margin from -// content that is genuinely indented. Measured over 401 445 three-row windows -// across 1 010 tracked files in this repo, it fired on 73% of them: 92% inside -// a YAML workflow, 76% over `git log` output, 48% in a TypeScript source file. -// No width threshold separates the two, because they are the same widths: a -// live Claude Code pane's own margins measure 2 and 5 columns while the most -// common non-TUI shared run is 4, sitting between them. +// A LEADING margin is stripped too, but only the one the CLI in the pane +// DECLARES as its transcript gutter, passed in as `options.margin`. Called with +// no options this trims trailing padding and nothing else, which is what keeps +// every caller that has no declared gutter on the old behaviour. // -// The asymmetry that settles it is in the failure modes. A wrong trailing trim +// ⚠ The failure modes are not symmetrical, and that asymmetry sets how much +// evidence a leading strip has to show before it fires. A wrong trailing trim // costs nothing. A wrong dedent silently deletes information that was on the // screen, with no signal to the user and nothing in the clipboard to hint at // it, and it is wrong on `git log` bodies, on indented code read out of `cat` // (semantic in Python), on `git diff` context rows where the leading space is // the marker, and on stack traces. // -// ⚠ It also cannot be made consistent cheaply. Whether the first row joins the -// measurement depended on the mousedown COLUMN, which the user never sees, so -// one block of three rows produced three different clipboard results; and the -// flag read `getSelectionPosition().start`, which is the mousedown anchor that -// xterm never normalises, so dragging UP through a block read it off the bottom -// row. If it is ever revisited, the one qualification that measured clean is -// painted trailing padding (a full-screen TUI writes real spaces across every -// row; a shell pane leaves those cells never-written, so xterm trims them): -// zero false positives over all 401 445 windows. It still mangles a `git log` -// body sitting inside an agent's own gutter, which is why it was not taken now. -function cleanCopiedSelection(text) { +// ⚠ The declared gutter is a CEILING, not the answer. The strip is 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 at all. +// +// ⚠ Deriving the width from the text instead is what fails, twice over. The +// selection's own shared indent cannot tell a margin from content, because a +// three-row window of nested YAML shares an indent for the same reason a margin +// does — it fired on 73% of ordinary indented text. Taking the narrowest indent +// on the surrounding rows fails more quietly: a file listing inside the +// transcript can be the narrowest thing on screen, which over-stripped about 1% +// of selections across six pane widths. +function cleanCopiedSelection(text, options) { if (typeof text !== 'string' || !text) return ''; // Split on \n and leave any \r in place: xterm joins rows with \r\n on // Windows, and the clipboard should keep the endings xterm chose. @@ -868,7 +864,36 @@ function cleanCopiedSelection(text) { while (cut > 0 && (line[cut - 1] === ' ' || line[cut - 1] === '\t')) cut--; return cut === end ? line : line.slice(0, cut) + line.slice(end); }; - return text.split('\n').map(trimEnd).join('\n'); + const lines = text.split('\n'); + for (let i = 0; i < lines.length; i++) lines[i] = trimEnd(lines[i]); + + const margin = Math.max(0, Math.trunc(Number(options?.margin) || 0)); + if (!margin) return lines.join('\n'); + + // The first line of a selection that began mid-row carries no margin — the + // mousedown cut it off — so it neither votes on the shared indent nor gets + // stripped. This is the ONE thing the mousedown column still decides, and it + // decides it for that line alone. Whether the rest of the block is dedented + // no longer depends on where the click landed, which is what made the same + // three rows produce three different clipboard results before. + const from = options?.firstLinePartial === true ? 1 : 0; + + // The pane's margin is a ceiling, not the answer. Strip the narrower of it + // and what every selected line shares, so the block shifts as a unit and no + // line can lose indentation another line keeps. + let shared = margin; + for (let i = from; i < lines.length && shared > 0; i++) { + const line = lines[i]; + if (!line || line === '\r') continue; // a padding-only row, already trimmed away + let run = 0; + while (run < line.length && line[run] === ' ') run++; + if (run < shared) shared = run; + } + if (!shared) return lines.join('\n'); + for (let i = from; i < lines.length; i++) { + if (lines[i] && lines[i] !== '\r') lines[i] = lines[i].slice(shared); + } + return lines.join('\n'); } if (typeof window !== 'undefined') { diff --git a/src/web/public/index.html b/src/web/public/index.html index dd972edc..eff75353 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -1791,6 +1791,13 @@ +
+
+ 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. 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/settings-ui.js b/src/web/public/settings-ui.js index 975efa77..2dfbcaf9 100644 --- a/src/web/public/settings-ui.js +++ b/src/web/public/settings-ui.js @@ -446,6 +446,8 @@ Object.assign(CodemanApp.prototype, { // overwrites the system clipboard on a gesture the user may have meant only as // a way to read, so it is opt-in rather than a default anyone has to discover. document.getElementById('appSettingsAutoCopySelection').checked = settings.autoCopySelection === true; + // Default ON, so an absent key reads as enabled rather than as off. + document.getElementById('appSettingsCopyStripMargin').checked = settings.copyStripMargin !== false; document.getElementById('appSettingsTerminalFont').value = settings.terminalFontFamily || ''; this.populateTerminalFontWeight(document.getElementById('appSettingsTerminalFontWeight'), settings.terminalFontWeight); this.populateTerminalFontWeight( @@ -2137,6 +2139,7 @@ Object.assign(CodemanApp.prototype, { tunnelEnabled: document.getElementById('appSettingsTunnelEnabled').checked, localEchoEnabled: document.getElementById('appSettingsLocalEcho').checked, autoCopySelection: document.getElementById('appSettingsAutoCopySelection').checked, + copyStripMargin: document.getElementById('appSettingsCopyStripMargin').checked, terminalFontFamily: document.getElementById('appSettingsTerminalFont').value.trim(), terminalFontWeight: this.readTerminalFontWeight(document.getElementById('appSettingsTerminalFontWeight')), terminalFontWeightBold: this.readTerminalFontWeight( @@ -2363,6 +2366,10 @@ Object.assign(CodemanApp.prototype, { // and absent from SettingsUpdateSchema (.strict()), so sending it would // 400 the whole settings PUT. autoCopySelection: _acs, + // What the clipboard gets is a property of what this device is looking + // at, and the key is absent from SettingsUpdateSchema (.strict()), so + // sending it would 400 the whole settings PUT. + copyStripMargin: _csm, // Per-device by nature (the font must exist on the device) and absent // from SettingsUpdateSchema (.strict()) — sending it would 400 the PUT. terminalFontFamily: _tff, @@ -3372,7 +3379,7 @@ Object.assign(CodemanApp.prototype, { 'terminalFontFamily', 'terminalFontWeight', 'terminalFontWeightBold', 'language', 'terminalWheelLocalScrollback', - 'autoCopySelection', + 'autoCopySelection', 'copyStripMargin', 'showSessionButton', 'showAwayDigestButton', 'showCronButton', 'showTabDetachButton', 'mobileOverviewEnabled', 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 9538b5ce..f0089101 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 @@ -4168,11 +4176,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 @@ -4189,7 +4202,73 @@ Object.assign(CodemanApp.prototype, { if (this.terminal?._core?._selectionService?._activeSelectionMode === 3) return raw; const clean = window.CodemanCopySelection?.clean; if (!clean) return raw; - return clean(raw); + const range = this._normalisedSelectionRange(); + return clean(raw, { + margin: this._cliGutterColumns(), + firstLinePartial: !!range && range.start.x > 0, + }); + }, + + /** + * xterm's selection range with its two ends in reading order. + * + * `getSelectionPosition()` reports `start` and `end` as the two ends of the + * drag, and on xterm 6.0 it already hands back the earlier one first: it + * reads `_selectionService.selectionStart`, whose getter returns the model's + * `finalSelectionStart`, and that swaps the pair for a reversed selection. + * A real upward mouse drag through chromium confirms it. The ordering here + * is a guard rather than a fix. One layer down the same model exposes the + * 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(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); + return reversed ? { start: end, end: start } : { start, end }; + }, + + /** + * 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 + * here, which is the registry's standing rule and is also what lets a CLI that + * declares a gutter later work with no change to this file. + * + * ⚠️ DECLARED rather than measured off the buffer, and two measured versions + * are why. Asking whether the pane painted spaces across the unused part of + * each row separates a TUI from a shell perfectly where it fires and never + * over-stripped, but it 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 Code's prose wraps to fill it: the share of padded rows on one live + * transcript ran 44%, 6%, 6%, 7% and 87% at 123, 160, 198, 235 and 298 + * columns, so the strip did nothing at any ordinary window size. Taking the + * narrowest indent on the rows around the selection instead fires at every + * width and over-strips on about 1% of them, because a file listing inside the + * transcript can be the narrowest thing on screen. A declared width does + * neither, and it reads no buffer rows at all on a path that runs on every + * Ctrl+C. + * + * A missing map means no session gets a strip, the same direction an + * unmeasured CLI takes by declaring nothing. + */ + _cliGutterColumns(sessionId) { + if (!this._copyStripMarginEnabled()) return 0; + const byMode = window.__codemanTranscriptGutter; + if (!byMode || typeof byMode !== 'object') return 0; + const mode = this.sessions?.get(sessionId ?? this.activeSessionId)?.mode; + const columns = mode ? byMode[mode] : 0; + return Number.isInteger(columns) && columns > 0 ? columns : 0; }, // Copy the current terminal selection. Goes through _copyText (Clipboard API, @@ -4224,6 +4303,26 @@ Object.assign(CodemanApp.prototype, { return ok; }, + /** + * Whether this device wants the pane's left margin off the clipboard + * (`copyStripMargin`, per-device, default ON). + * + * Read here rather than mirrored into a field, for the same reason + * `_autoCopySelectionEnabled` is: there is then no apply-path a future + * settings save can forget to call, and the toggle takes effect on the next + * selection instead of the next reload. ⚠️ The test is `!== false`, not + * `=== true`: this one defaults ON, and the desktop branch of + * getDefaultSettings returns {} and leans on the read sites for defaults, so + * a device that has never opened App Settings has no stored value at all. + */ + _copyStripMarginEnabled() { + try { + return this.loadAppSettingsFromStorage?.()?.copyStripMargin !== false; + } catch { + return true; + } + }, + /** * Auto Copy's ON/OFF, read at flush time from the CACHED settings object * (loadAppSettingsFromStorage memoizes, so this is not a localStorage hit). diff --git a/src/web/server.ts b/src/web/server.ts index 3514b649..093b8215 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -1688,6 +1688,27 @@ 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). + // + // ⚠️ 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/server-index-title.test.ts b/test/server-index-title.test.ts index e2c7b979..b8bddc82 100644 --- a/test/server-index-title.test.ts +++ b/test/server-index-title.test.ts @@ -96,9 +96,9 @@ describe('WebServer index.html templating (#82)', () => { it('only substitutes the <title> tag — the rest of the template is identical (modulo asset cache-busting)', async () => { // renderIndexHtml also appends ?v=<mtime> cache-bust params to same-origin - // .js/.css refs, and injects the CLI-availability flags plus the custom-model - // Run-menu picker's CLI list before </head>; strip all so the title remains - // the only other change. + // .js/.css refs, and injects the CLI-availability flags, the custom-model + // Run-menu picker's CLI list and the transcript-gutter widths before </head>; + // strip all so the title remains the only other change. // // The flag strips are what keep this test environment-independent. The // CLI-availability one used to pass here by luck: that script was injected @@ -109,7 +109,10 @@ describe('WebServer index.html <title> templating (#82)', () => { const html = (await render('laptop')) .replace(/(\.(?:js|css))\?v=[^"]*/g, '$1') .replace(/<script>window\.__codemanCliAvailable=\{.*?\};<\/script>\n/, '') - .replace(/<script>window\.__codemanCustomModelClis=\[.*?\];<\/script>\n/, ''); + .replace(/<script>window\.__codemanCustomModelClis=\[.*?\];<\/script>\n/, '') + // Injected unconditionally as an object keyed by run mode, empty when no + // enabled CLI declares a gutter, so it needs stripping on every machine. + .replace(/<script>window\.__codemanTranscriptGutter=\{.*?\};<\/script>\n/, ''); const beforeTitle = rawTemplate.split('<title>Codeman')[0]; const afterTitle = rawTemplate.split('Codeman')[1]; expect(html.startsWith(beforeTitle)).toBe(true); diff --git a/test/terminal-copy-clean.test.ts b/test/terminal-copy-clean.test.ts index 63ac18b4..7f5e4549 100644 --- a/test/terminal-copy-clean.test.ts +++ b/test/terminal-copy-clean.test.ts @@ -9,9 +9,11 @@ * the string handling: a padding-only selection must not silently keep the * user's Ctrl+C, and must not put a bare newline on the clipboard. * - * A shared LEADING indent is deliberately left alone. The block below pins that - * as a contract rather than an accident, because stripping it was built and - * dropped before merge: see the rule in docs/architecture-invariants.md. + * A shared LEADING indent is stripped only when a caller has measured a MARGIN + * off the pane and passed it in. The transform called with text alone still + * touches trailing padding and nothing else, and the block below pins that, + * because measuring the indent off the SELECTION was built and dropped before + * #451 merged: see the rule in docs/architecture-invariants.md. * * Strategy: constants.js and terminal-ui.js in one vm with a stub CodemanApp, * the harness shape test/terminal-auto-copy.test.ts uses. No DOM, no xterm. @@ -25,9 +27,19 @@ import { describe, expect, it, vi } from 'vitest'; const publicDir = resolve(import.meta.dirname, '../src/web/public'); const read = (name: string) => readFileSync(resolve(publicDir, name), 'utf8'); -function loadHarness() { +/** + * The map the server derives from the `transcriptGutter` CAPABILITY and injects + * at render, keyed by run mode. Claude is the only stock entry that declares one. + */ +const STOCK_GUTTERS = { claude: 2, codex: 2 }; + +function loadHarness( + settingsOverride?: Record, + gutters: Record | null = STOCK_GUTTERS +) { const CodemanApp = function CodemanApp(this: unknown) {}; const windowRef: Record = {}; + if (gutters) windowRef.__codemanTranscriptGutter = gutters; const context = vm.createContext({ window: windowRef, document: { @@ -59,13 +71,23 @@ function loadHarness() { const toasts: { message: string; type: string }[] = []; app.showToast = (message: string, type: string) => toasts.push({ message, type }); app._copyText = vi.fn(async () => true); - app.loadAppSettingsFromStorage = () => ({ autoCopySelection: true }); + app.loadAppSettingsFromStorage = () => ({ autoCopySelection: true, ...(settingsOverride ?? {}) }); - const setSelection = (selection: string, { startX = 0, columnMode = false } = {}) => { + // `mode` is what decides the strip: the session's run mode is looked up in the + // injected gutter map. No buffer is involved, because the width is declared + // rather than measured off the pane. The default is a mode that declares NO + // gutter, so a test about the trailing trim keeps its exact meaning, and only a + // test asking for `mode: 'claude'` gets a leading strip at all. + const setSelection = ( + selection: string, + { startX = 0, columnMode = false, mode = 'shell', from = 0, to = 1 } = {} + ) => { + app.activeSessionId = 'S1'; + app.sessions = new Map([['S1', { id: 'S1', mode }]]); app.terminal = { hasSelection: () => !!selection, getSelection: vi.fn(() => selection), - getSelectionPosition: () => ({ start: { x: startX, y: 0 }, end: { x: 0, y: 1 } }), + getSelectionPosition: vi.fn(() => ({ start: { x: startX, y: from }, end: { x: 0, y: to } })), clearSelection: vi.fn(), focus: vi.fn(), _core: { _selectionService: { _activeSelectionMode: columnMode ? 3 : 0 } }, @@ -100,11 +122,12 @@ describe('CodemanCopySelection.clean — trailing padding', () => { }); }); -describe('CodemanCopySelection.clean — a shared leading indent is kept', () => { +describe('CodemanCopySelection.clean — a shared leading indent is kept without a margin', () => { // Measured over 401,445 three-row windows across 1,010 tracked files, stripping // the run every row shares fired on 73% of them, and the transform cannot tell - // a TUI margin from content. These are the cases that settled it: each one is - // real output a user copies, and each one loses information if this changes. + // a TUI margin from content by looking at the selection. These are the cases + // that settled it, and each one still loses information if the transform ever + // strips an indent nobody measured off the pane. it('keeps the indent every selected row shares', () => { expect(clean(' first line\n second line')).toBe(' first line\n second line'); }); @@ -176,15 +199,10 @@ describe('cleanedTerminalSelection — wiring', () => { expect(app.cleanedTerminalSelection()).toBe(' first\n second'); }); - it('does not read the selection position at all', () => { - // The mid-row flag is gone. It read getSelectionPosition().start, which is - // xterm's mousedown ANCHOR and is never normalised, so an upward drag read - // it off the bottom row of the selection. + it('strips nothing for a run mode that declares no gutter', () => { const { app, setSelection } = loadHarness(); - const terminal = setSelection(' first\n second'); - terminal.getSelectionPosition = vi.fn(() => ({ start: { x: 6, y: 0 }, end: { x: 0, y: 1 } })); + setSelection(' first\n second'); expect(app.cleanedTerminalSelection()).toBe(' first\n second'); - expect(terminal.getSelectionPosition).not.toHaveBeenCalled(); }); it('uses the text it is given without reading the selection again', () => { @@ -212,6 +230,318 @@ describe('cleanedTerminalSelection — wiring', () => { }); }); +describe('CodemanCopySelection.clean — the margin is a ceiling, never the answer', () => { + const clean2 = (text: string, options: Record) => + loadHarness().windowRef.CodemanCopySelection.clean(text, options); + + it('strips a measured margin', () => { + expect(clean2(' first\n second', { margin: 2 })).toBe('first\nsecond'); + }); + + it('strips the margin and no more from a block indented past it', () => { + // Four of these six columns are the git log body's own, and they stay. + expect(clean2(' fix(terminal): trim it\n xterm hands back rows', { margin: 2 })).toBe( + ' fix(terminal): trim it\n xterm hands back rows' + ); + }); + + it('strips nothing when any selected line sits at column 0', () => { + // Selecting a marker row along with the prose under it means the block's + // own shared indent is zero, and the block shifts as a unit or not at all. + expect(clean2('● Creating a job\n Intent. A daily check.', { margin: 2 })).toBe( + '● Creating a job\n Intent. A daily check.' + ); + }); + + it("keeps every line's indentation relative to every other", () => { + const before = ' name: CI\n on:\n push:\n branches: [master]'; + expect(clean2(before, { margin: 2 })).toBe('name: CI\non:\n push:\n branches: [master]'); + }); + + it('leaves the first line alone when the selection began mid-row', () => { + // That line never carried the margin: the mousedown cut it off. + expect(clean2('rst line\n second\n third', { margin: 2, firstLinePartial: true })).toBe( + 'rst line\nsecond\nthird' + ); + }); + + it('still trims trailing padding while it dedents', () => { + expect(clean2(' first \n \n second ', { margin: 2 })).toBe('first\n\nsecond'); + }); + + it('keeps the Windows line join intact on both blank and content rows', () => { + expect(clean2(' first\r\n\r\n second\r\n', { margin: 2 })).toBe('first\r\n\r\nsecond\r\n'); + }); + + it('treats a margin it cannot use as no margin at all', () => { + expect(clean2(' first\n second', { margin: 0 })).toBe(' first\n second'); + expect(clean2(' first\n second', { margin: -4 })).toBe(' first\n second'); + expect(clean2(' first\n second', { margin: 'two' as never })).toBe(' first\n second'); + }); +}); + +describe('cleanedTerminalSelection — the two bugs that kept this out of #451', () => { + // Both were real, and both came from the mid-row flag reading the selection's + // own geometry to decide how much every row lost. The width is declared by the + // CLI now, so neither end of a drag can move it, and the flag governs one line. + const ROWS = [ + ' Intent. A daily check tells you when it publishes.', + ' Scope. One recurring routine and nothing else.', + ' Risks. Three are worth naming here.', + ]; + const body = + ' Intent. A daily check tells you when it publishes.\n Scope. One recurring routine and nothing else.\n Risks. Three are worth naming here.'; + const dedented = + 'Intent. A daily check tells you when it publishes.\nScope. One recurring routine and nothing else.\nRisks. Three are worth naming here.'; + + it('survives a reversed range, whatever end xterm reports first', () => { + // xterm 6.0 orders the pair itself, verified by driving a real upward drag + // through chromium, so this pins the guard rather than a live bug: an + // unordered pair would put the mid-row flag on the wrong end of the drag. + const from = 401; + const down = loadHarness(); + down.setSelection(body, { mode: 'claude', from, to: from + 2 }); + const up = loadHarness(); + up.setSelection(body, { mode: 'claude', from, to: from + 2 }); + up.app.terminal.getSelectionPosition = () => ({ start: { x: 0, y: from + 2 }, end: { x: 0, y: from } }); + expect(down.app.cleanedTerminalSelection()).toBe(dedented); + expect(up.app.cleanedTerminalSelection()).toBe(dedented); + }); + + it('reads the mid-row flag off the earlier end of the range, not the later one', () => { + // A drag between column 9 on the first row and column 0 on the last leaves + // the FIRST line partial. Read off the wrong end the flag says the block is + // flush, and that partial line loses two characters of its own content. + const { app, setSelection } = loadHarness(); + setSelection(`nt. A daily check tells you when it publishes.\n${ROWS[1]}`, { mode: 'claude', from: 1, to: 2 }); + app.terminal.getSelectionPosition = () => ({ start: { x: 0, y: 2 }, end: { x: 9, y: 1 } }); + expect(app.cleanedTerminalSelection()).toBe( + 'nt. A daily check tells you when it publishes.\nScope. One recurring routine and nothing else.' + ); + }); + + it('gives the rows below the first one the same result whatever column the mousedown hit', () => { + // Three rows used to produce three different clipboards depending on where + // the click landed, which the user never sees. Only the partial first line + // may differ now, and it differs because it is different text. + const tails = [0, 1, 2, 7].map((startX) => { + const h = loadHarness(); + h.setSelection(`${ROWS[0].slice(startX)}\n${ROWS[1]}\n${ROWS[2]}`, { mode: 'claude', from: 1, to: 3, startX }); + return h.app.cleanedTerminalSelection().split('\n').slice(1).join('\n'); + }); + expect(new Set(tails).size).toBe(1); + expect(tails[0]).toBe('Scope. One recurring routine and nothing else.\nRisks. Three are worth naming here.'); + }); +}); + +describe('cleanedTerminalSelection — the cases the margin has to get right', () => { + it('takes the gutter off Claude Code prose, which is what people copy', () => { + const { app, setSelection } = loadHarness(); + setSelection( + ' Intent. A daily check tells you when it publishes.\n Scope. One recurring routine on your account.', + { mode: 'claude', from: 1, to: 2 } + ); + expect(app.cleanedTerminalSelection()).toBe( + 'Intent. A daily check tells you when it publishes.\nScope. One recurring routine on your account.' + ); + }); + + it('keeps a git log body at its four-space indent inside an agent gutter', () => { + // This is the case that kept the painted-padding gate out on its own: the + // pane is a TUI, so that gate says yes, and the body shares six columns. + const { app, setSelection } = loadHarness(); + setSelection( + ' fix(terminal): trim the padding a TUI paints\n xterm hands back whole rows and trims only the\n cells nothing ever wrote to.', + { mode: 'claude', from: 4, to: 6 } + ); + expect(app.cleanedTerminalSelection()).toBe( + ' fix(terminal): trim the padding a TUI paints\n xterm hands back whole rows and trims only the\n cells nothing ever wrote to.' + ); + }); + + it('keeps YAML nesting inside an agent gutter', () => { + const { app, setSelection } = loadHarness(); + setSelection(' build:\n steps:\n - run: npm ci', { mode: 'claude', from: 2, to: 4 }); + expect(app.cleanedTerminalSelection()).toBe(' build:\n steps:\n - run: npm ci'); + }); + + it('leaves indented Python alone in a shell pane, where the indent is semantic', () => { + const { app, setSelection } = loadHarness(); + setSelection(' if event.ready:\n run(event)', { from: 2, to: 3 }); + expect(app.cleanedTerminalSelection()).toBe(' if event.ready:\n run(event)'); + }); + + it('leaves git diff context rows alone, where the leading space is the marker', () => { + const { app, setSelection } = loadHarness(); + setSelection(' const x = 1;\n const y = 2;\n }', { from: 2, to: 4 }); + expect(app.cleanedTerminalSelection()).toBe(' const x = 1;\n const y = 2;\n }'); + }); +}); + +describe('copyStripMargin — the per-device toggle', () => { + const body = ' Intent. A daily check tells you when it publishes.\n Scope. One recurring routine and nothing else.'; + const select = (h: ReturnType) => h.setSelection(body, { mode: 'claude', from: 1, to: 2 }); + + it('strips the margin when the device has never stored a value, because it defaults ON', () => { + const h = loadHarness({}); + select(h); + expect(h.app.cleanedTerminalSelection()).toBe( + 'Intent. A daily check tells you when it publishes.\nScope. One recurring routine and nothing else.' + ); + }); + + it('leaves the margin alone when the device turned it off', () => { + const h = loadHarness({ copyStripMargin: false }); + select(h); + expect(h.app.cleanedTerminalSelection()).toBe(body); + }); + + it('still trims trailing padding while the strip is off', () => { + const h = loadHarness({ copyStripMargin: false }); + h.setSelection(' first \n second ', { mode: 'claude', from: 1, to: 2 }); + expect(h.app.cleanedTerminalSelection()).toBe(' first\n second'); + }); + + it('never consults the gutter map while it is off', () => { + // The lookup is the whole cost now, and it runs on every Ctrl+C, so the + // toggle is checked first. There is no buffer to read: the width is declared. + const h = loadHarness({ copyStripMargin: false }); + select(h); + expect(h.app._cliGutterColumns()).toBe(0); + expect(h.app.cleanedTerminalSelection()).toBe(body); + }); + + it('treats an unreadable settings store as ON, matching the default', () => { + const h = loadHarness(); + h.app.loadAppSettingsFromStorage = () => { + throw new Error('localStorage unavailable'); + }; + select(h); + expect(h.app.cleanedTerminalSelection()).toBe( + 'Intent. A daily check tells you when it publishes.\nScope. One recurring routine and nothing else.' + ); + }); + + it('keeps the toggle per-device: display key, stripped from the PUT, absent from the schema', () => { + const settingsUi = read('settings-ui.js'); + const schemas = readFileSync(resolve(import.meta.dirname, '../src/web/schemas.ts'), 'utf8'); + const displayKeys = settingsUi.slice( + settingsUi.indexOf('const displayKeys = new Set(['), + settingsUi.indexOf('])', settingsUi.indexOf('const displayKeys = new Set([')) + ); + expect(displayKeys).toContain("'copyStripMargin'"); + // SettingsUpdateSchema is .strict(), so a key it does not declare 400s the + // whole settings PUT if the client sends it. + expect(settingsUi).toContain('copyStripMargin: _csm,'); + expect(schemas).not.toContain('copyStripMargin'); + }); + + it('keeps the control loadable and savable by id', () => { + const settingsUi = read('settings-ui.js'); + expect(read('index.html')).toContain('id="appSettingsCopyStripMargin"'); + // `!== false`, because this one defaults ON and the desktop branch of + // getDefaultSettings returns {}. + expect(settingsUi).toContain( + "document.getElementById('appSettingsCopyStripMargin').checked = settings.copyStripMargin !== false;" + ); + expect(settingsUi).toContain("copyStripMargin: document.getElementById('appSettingsCopyStripMargin').checked,"); + }); +}); + +describe('the gutter is DECLARED by the CLI, never measured off the pane', () => { + const body = ' Intent. A daily check tells you when it publishes.\n Scope. One recurring routine.'; + const flush = 'Intent. A daily check tells you when it publishes.\nScope. One recurring routine.'; + + 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._cliGutterColumns()).toBe(2); + expect(app.cleanedTerminalSelection()).toBe(flush); + }); + + it('takes the two columns off Codex as well, and keeps the nesting under them', () => { + // Measured on a live codex-cli 0.154.0 answer: its •/›/⚠ markers sit in the + // gutter, prose continuations sit at 2, and a nested YAML block the model + // wrote rendered at 2/4/6/8 for its own 0/2/4/6. Replayed at six widths the + // 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._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._cliGutterColumns()).toBe(0); + expect(app.cleanedTerminalSelection()).toBe(' build:\n steps:'); + }); + + it('leaves a shell alone for the same reason', () => { + const { app, setSelection } = loadHarness(); + setSelection(' if event.ready:\n run(event)', { mode: 'shell', from: 1, to: 2 }); + expect(app.cleanedTerminalSelection()).toBe(' if event.ready:\n run(event)'); + }); + + it('strips nothing at all when the server injected no map', () => { + // A page served before the capability existed, or a render path that skips + // 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._cliGutterColumns()).toBe(0); + expect(app.cleanedTerminalSelection()).toBe(body); + }); + + it('ignores a width that is not a positive whole number', () => { + 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._cliGutterColumns()).toBe(0); + } + }); + + it('reads no terminal buffer on the copy path at all', () => { + // The old version scanned up to ~240 rows per Ctrl+C to measure a width that + // the CLI can simply state. A buffer here would be a regression to that. + const { app, setSelection } = loadHarness(); + const terminal = setSelection(body, { mode: 'claude', from: 1, to: 2 }); + const getLine = vi.fn(() => undefined); + terminal.buffer = { active: { length: 0, getLine } }; + expect(app.cleanedTerminalSelection()).toBe(flush); + expect(getLine).not.toHaveBeenCalled(); + }); +}); + +describe('the transcriptGutter capability, as the registry and server carry it', () => { + const registryDir = resolve(import.meta.dirname, '../src/config/cli-registry'); + const readSrc = (n: string) => readFileSync(resolve(registryDir, n), 'utf8'); + + it('is a bounded integer in the schema, so a clis.json cannot declare a huge one', () => { + expect(readSrc('schema.ts')).toContain('transcriptGutter: z.number().int().min(1).max(8).optional()'); + }); + + it('is declared by claude and codex, and by nothing else in the stock registry', () => { + const stock = readSrc('stock.ts'); + expect(stock.match(/transcriptGutter: 2,/g)).toHaveLength(2); + // Exactly the two whose transcript layout has been measured on a live pane. + expect(stock.match(/transcriptGutter:/g)).toHaveLength(2); + }); + + it('reaches the page off the capability rather than as an id list', () => { + const server = readFileSync(resolve(import.meta.dirname, '../src/web/server.ts'), 'utf8'); + expect(server).toContain('entry.capabilities.transcriptGutter'); + expect(server).toContain('window.__codemanTranscriptGutter='); + // 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('_cliGutterColumns(sessionId) {'), + terminalUi.indexOf('async copyTerminalSelection') + ); + expect(helper).toContain('window.__codemanTranscriptGutter'); + expect(helper).not.toMatch(/'claude'/); + }); +}); + describe('the xterm internals the column check depends on', () => { // The column check reads a private field and compares it to a literal, because // xterm publishes the selection mode nowhere. A rename or a renumber would make @@ -276,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', () => {