From a2b43fa1eb2e578a549bf585b790ad46378af3ec Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Tue, 8 Sep 2026 12:48:26 -0700 Subject: [PATCH 1/2] feat: emit osc 52 for pane copies Manager pane copies now write the OSC 52 clipboard sequence to the pane terminal in addition to the native clipboard writer, so Herdr 0.9.0 forwards the copy to the viewing client when the plugin runs on a remote server. When only the terminal copy lands, the manager stays open and reports it instead of hiding the native failure. The global copy-context and copy-archive actions run with piped stdout and no terminal, so they are unchanged. The parity harness now retains the raw PTY stream and compares the emitted sequences and their payloads in both runtimes. refs #40 --- README.md | 4 + docs/rust-lite-parity-proof.md | 87 +++++++++++------- docs/rust-lite-parity.md | 7 +- rust/src/lib.rs | 1 + rust/src/manager.rs | 10 ++- rust/src/pane_clipboard.rs | 158 +++++++++++++++++++++++++++++++++ scripts/parity-lite.py | 69 +++++++++++++- src/manager.ts | 6 +- src/pane-clipboard.ts | 44 +++++++++ test/pane-clipboard.test.ts | 67 ++++++++++++++ 10 files changed, 413 insertions(+), 40 deletions(-) create mode 100644 rust/src/pane_clipboard.rs create mode 100644 src/pane-clipboard.ts create mode 100644 test/pane-clipboard.test.ts diff --git a/README.md b/README.md index fb58a70..e567705 100644 --- a/README.md +++ b/README.md @@ -145,6 +145,10 @@ herdr server reload-config | `Ctrl+B Ctrl+A` | copy all annotations as Markdown, then archive them | | `Ctrl+B M` | manage · `y` copy one · `c` copy all · `Shift+C` copy and archive · `Tab` archives (`y` copy · `u` restore · `d d` delete) | +Copies made inside the manager pane also emit OSC 52, so on Herdr 0.9.0 they reach the clipboard of +the machine you are viewing from even when the plugin runs on a remote server; `Ctrl+B Shift+A` and +`Ctrl+B Ctrl+A` do not, because those actions run outside a pane and have no terminal to write to. + ### Review documents and agent replies Full install. Works with Claude Code, Codex, pi, Copilot CLI, Droid, Oh My Pi, Hermes CLI and OpenCode (1 and 2). diff --git a/docs/rust-lite-parity-proof.md b/docs/rust-lite-parity-proof.md index e7d8e48..b323df4 100644 --- a/docs/rust-lite-parity-proof.md +++ b/docs/rust-lite-parity-proof.md @@ -5,7 +5,7 @@ TypeScript Lite surface to the Rust call path at function granularity and names case that compares the result. The current local result is: ```text -Parity Lite: 459 observables compared, 92 screens diffed, zero divergences / 1 deliberate +Parity Lite: 496 observables compared, 95 screens diffed, zero divergences / 1 deliberate ``` Run it from the repository root with `bash scripts/parity-lite.sh`. The shell wrapper stages a fresh @@ -21,14 +21,18 @@ diffs and prints their path. A green run removes its temporary workspace. manifest and compare the declared entrypoints field by field, so a new action cannot reach one runtime only. - Screen cases use real PTYs at 86×22 (editor) and 98×28 (manager). The ANSI parser at - `scripts/parity-lite.py:156` ignores style escapes but retains the terminal cell grid, including + `scripts/parity-lite.py:158` ignores style escapes but retains the terminal cell grid, including wide-character continuation cells. It snapshots the initial frame and the frame after every input. +- Screen cases also retain the raw PTY byte stream and compare every OSC 52 clipboard sequence a + pane emitted, in order (`scripts/parity-lite.py:388`). Where a case compares clipboard bytes, the + payload of the last sequence is additionally required to equal the bytes the native writer received, + so a runtime cannot emit a different copy to the terminal than it wrote to the clipboard. - Store cases byte-compare JSONL, modes, and leftover lock/temp files after scripted editor and manager mutations. `store.cross-read` makes Bun export the Rust editor's store and Rust export the Bun editor's store, then compares the Markdown and subprocess traces. - The only normalized values are each case's deliberately different temporary root, generated UUIDs, generated ISO timestamps, and pid/time components in pending and temporary filenames - (`scripts/parity-lite.py:459`). Seed timestamps are allow-listed and remain literal. Product files + (`scripts/parity-lite.py:481`). Seed timestamps are allow-listed and remain literal. Product files are never rewritten or filtered. Screen cells and clipboard bytes are never normalized. - The harness runs on the host's real adapter branch. macOS therefore proves `pbpaste`/`pbcopy` and Ubuntu proves the Wayland → xclip → xsel chain. Windows is intentionally outside this PTY harness @@ -73,7 +77,7 @@ contract: success and blank input exit 0; a defined failure produces one stderr | Observable decision or effect | TypeScript call path | Rust call path | Mechanical evidence | |---|---|---|---| | Manifest and state | `lite/herdr-plugin.toml:31` → top-level `src/export-archive.ts:76` → `main` at `:45`; state is required at `:47`. | `lite-rs/herdr-plugin.toml:41` → `rust/src/main.rs:3` → `cli::run` at `rust/src/cli.rs:33` → `copy_archive` at `:183`; state is required at `:184`. | Every `process.copy-archive.*` case. | -| Shared workflow | `copyAndArchiveAnnotations` at `src/archive-workflow.ts:29` receives the same dependencies `src/manager.ts:226` injects, wired at `src/export-archive.ts:55`. | `copy_and_archive_annotations` at `rust/src/archive_workflow.rs:27` receives the dependencies `rust/src/manager.rs:555` injects, wired at `rust/src/cli.rs:191`. | `test/archive-workflow.test.ts` and `archive_workflow::tests` cover the workflow; `store.manager.copy-archive` covers the manager key; the process cases cover the action. | +| Shared workflow | `copyAndArchiveAnnotations` at `src/archive-workflow.ts:29` receives the same dependencies `src/manager.ts:228` injects, wired at `src/export-archive.ts:55`. | `copy_and_archive_annotations` at `rust/src/archive_workflow.rs:27` receives the dependencies `rust/src/manager.rs:556` injects, wired at `rust/src/cli.rs:191`. | `test/archive-workflow.test.ts` and `archive_workflow::tests` cover the workflow; `store.manager.copy-archive` covers the manager key; the process cases cover the action. | | Operation order | Load, format newest first, clipboard write, `appendArchivedSet`, then `removeAnnotationsById` — `src/archive-workflow.ts:32`-`:52`. | The same order at `rust/src/archive_workflow.rs:38`-`:65`. | `process.copy-archive.populated` byte-compares both JSONL stores, modes, and leftover lock/temp files afterwards, plus the unnormalized clipboard bytes. | | Empty store | The injected loader records the empty read at `src/export-archive.ts:58`; `copyArchiveReport` at `:39` notifies `No annotations` / `There is nothing to copy yet.` and exits 0, matching `export.ts`. | `Cell` flag at `rust/src/cli.rs:194`; `copy_archive_report` at `:170` sends the same notification and returns success. | `process.copy-archive.empty` compares the notification, exit 0, and the untouched state tree. | | Success reporting | `copyArchiveReport` at `src/export-archive.ts:25` sends singular/plural `Annotations copied and archived`. | `copy_archive_report` at `rust/src/cli.rs:157` builds the same title and body; `copy_archive` notifies at `:207`. | `process.copy-archive.populated`; `test/export-archive.test.ts` and `cli::tests::copy_archive_maps_every_outcome_to_its_notification_and_exit_status` pin both grammars. | @@ -99,11 +103,11 @@ contract: success and blank input exit 0; a defined failure produces one stderr | Observable decision or effect | TypeScript call path | Rust call path | Mechanical evidence | |---|---|---|---| -| Manifest, state, initial load | `lite/herdr-plugin.toml:54` → `requireStateDir` at `src/manager.ts:29`; `reloadActive`/`reloadArchives` at `:53`/`:64`. | `lite-rs/herdr-plugin.toml:64` → dispatcher → `manager::run` at `rust/src/manager.rs:718`; `ManagerApp::load` at `:62`; reload methods at `:79`/`:94`. | `process.manager.missing-state`; initial screens in all manager PTY cases. | -| Active screen | `render` at `src/manager.ts:196` → `renderActive` at `:93`, with `clipped` at `:75`, formatting helpers, newest-first state, source and timestamp metadata. | `ManagerApp::draw` at `rust/src/manager.rs:109` → `draw_active` at `:161`, with `clipped` at `:680` and `format_timestamp` at `:690`. | Every active-view snapshot, including the detail-width regression fixture in `screen.manager.all-views`. | -| Archive screen | `render` → `renderArchives` at `src/manager.ts:136`; archive annotations are previewed newest first. | `ManagerApp::draw` → `draw_archives` at `rust/src/manager.rs:299`; same preview ordering and clipping. | Every archive-view snapshot and scripted archive mutation. | -| Input and mutations | Top-level key dispatch is `src/manager.ts:402`; active/archive handlers are `:318`/`:344`; action functions are `:216`–`:316`. | `ManagerApp::handle_key` is `rust/src/manager.rs:433`; view handlers are `:465`/`:502`; action methods are `:546`–`:650`. | `screen.manager.all-views`, empty-actions, success-copy sessions, and the exit/signal sessions. Resulting JSONL and clipboard bytes are compared. | -| Cleanup and signals | `cleanup`/`exit` at `src/manager.ts:376`/`:383`; signal handlers at `:390`. | `Termination` at `rust/src/termination.rs:17`; polling/restore at `rust/src/manager.rs:718`. | `screen.manager.sighup` compares exit 0 and restored cells. | +| Manifest, state, initial load | `lite/herdr-plugin.toml:54` → `requireStateDir` at `src/manager.ts:30`; `reloadActive`/`reloadArchives` at `:55`/`:66`. | `lite-rs/herdr-plugin.toml:64` → dispatcher → `manager::run` at `rust/src/manager.rs:724`; `ManagerApp::load` at `:63`; reload methods at `:80`/`:95`. | `process.manager.missing-state`; initial screens in all manager PTY cases. | +| Active screen | `render` at `src/manager.ts:198` → `renderActive` at `:95`, with `clipped` at `:77`, formatting helpers, newest-first state, source and timestamp metadata. | `ManagerApp::draw` at `rust/src/manager.rs:110` → `draw_active` at `:162`, with `clipped` at `:681` and `format_timestamp` at `:691`. | Every active-view snapshot, including the detail-width regression fixture in `screen.manager.all-views`. | +| Archive screen | `render` → `renderArchives` at `src/manager.ts:138`; archive annotations are previewed newest first. | `ManagerApp::draw` → `draw_archives` at `rust/src/manager.rs:300`; same preview ordering and clipping. | Every archive-view snapshot and scripted archive mutation. | +| Input and mutations | Top-level key dispatch is `src/manager.ts:404`; active/archive handlers are `:320`/`:346`; action functions are `:218`–`:318`. | `ManagerApp::handle_key` is `rust/src/manager.rs:434`; view handlers are `:466`/`:503`; action methods are `:547`–`:651`. | `screen.manager.all-views`, empty-actions, success-copy sessions, and the exit/signal sessions. Resulting JSONL and clipboard bytes are compared. | +| Cleanup and signals | `cleanup`/`exit` at `src/manager.ts:378`/`:385`; signal handlers at `:392`. | `Termination` at `rust/src/termination.rs:17`; polling/restore at `rust/src/manager.rs:724`. | `screen.manager.sighup` compares exit 0 and restored cells. | ## Every editor key @@ -126,23 +130,23 @@ transition. ## Every manager key in both views -The common TypeScript dispatcher is `src/manager.ts:402`; Rust's is `rust/src/manager.rs:433`. +The common TypeScript dispatcher is `src/manager.ts:404`; Rust's is `rust/src/manager.rs:434`. Each non-exit transition re-renders the full grid. Arrow Up/Down are aliases of `k`/`j` and are also fed by `screen.manager.all-views`. | Key | Active view: TypeScript → Rust and effect | Archives view: TypeScript → Rust and effect | Evidence | |---|---|---|---| -| `j` / Down | `handleActiveKey` `src/manager.ts:328` → `handle_active_key` `rust/src/manager.rs:477`; increment/clamp active selection, changing list highlight and detail. | `handleArchiveKey` `src/manager.ts:364` → `handle_archive_key` `rust/src/manager.rs:526`; increment/clamp archive selection and detail. | `active-j`, `active-arrow-down`, `archives-j`, `archives-arrow-down`. | -| `k` / Up | `src/manager.ts:326` → `rust/src/manager.rs:474`; decrement/saturate. | `src/manager.ts:362` → `rust/src/manager.rs:523`; decrement/saturate. | Corresponding `k` and arrow-up steps. | -| `y` | `copy` at `src/manager.ts:216` receives the selected annotation → `ManagerApp::copy` at `rust/src/manager.rs:546`; exact one-item Markdown is copied; success exits 0, failure/empty displays status. | Same helpers receive the selected archive's annotations in newest-first order (`src/manager.ts:366`, `rust/src/manager.rs:528`). | Failure frames in all-views, success sessions `store.manager.active-y-success` / `archives-y-success`, and empty-actions. | -| `c` | `src/manager.ts:335` copies the displayed newest-first active list → `rust/src/manager.rs:491`; success exits, failure remains. | Not handled by `src/manager.ts:344` or `rust/src/manager.rs:502`; clears any prior confirmation/status through normal dispatch, otherwise store/view unchanged. | Active failure/success and `archives-c-ignored`. | -| `C` | `copyAndArchive` at `src/manager.ts:225` → `copyAndArchiveAnnotations` at `src/archive-workflow.ts:29`; Rust `rust/src/manager.rs:553` → `copy_and_archive_annotations` at `rust/src/archive_workflow.rs:27`. Order is load → copy → append archive → remove captured active IDs. Success exits; partial failure is reported without data loss. | Not handled; same no-op/confirmation-clear behavior as archive `c`. | Failure in all-views; success and byte-diff in `store.manager.copy-archive`; `archives-C-ignored`; workflow failure ordering has paired TS/Rust unit specs. | -| `d` | `deleteSelectedAnnotation` at `src/manager.ts:243` → `rust/src/manager.rs:574`; remove selected ID through locked atomic rewrite, reload, status `Annotation deleted.` | First press records the selected archive id and renders `Press d again…`; second matching press calls `deleteSelectedArchive` (`src/manager.ts:295`, `rust/src/manager.rs:638`) and atomically removes it. No selection displays `No archive selected.` Esc or another ordinary key cancels confirmation. | Active delete, archive confirm/cancel/double-confirm in all-views; empty-actions. | -| `D` | `src/manager.ts:319` / `rust/src/manager.rs:466`: first press renders `Press Shift+D again…`; second calls `clearActive` (`src/manager.ts:255`, `rust/src/manager.rs:588`), rewrites active JSONL empty, reloads, and displays `All active annotations cleared.` | Uppercase `D` is not an archive action; it cancels a pending archive confirmation like any non-`d` archive key, otherwise no store effect. | Active confirm/cancel/double-confirm and `archives-D-ignored` in all-views. | -| `r` | `reloadActive` (`src/manager.ts:53`, `rust/src/manager.rs:79`); success status `Reloaded.`, failure status is the store error. | `reloadArchives` (`src/manager.ts:64`, `rust/src/manager.rs:94`) with the same status rule. | Both reload steps in all-views; invalid/busy store process cases cover propagated store errors. | -| `u` | Not handled in active view; clears transient status/confirmation, leaves selection and stores unchanged. | `restoreSelectedArchive` at `src/manager.ts:269` → `restoreArchivedSet` at `src/archive-workflow.ts:71`; Rust `rust/src/manager.rs:605` → `restore_archived_set` at `rust/src/archive_workflow.rs:96`. Order is merge missing annotation IDs, then remove archive; partial removal failure keeps the archive and reports it. | `active-u-ignored`; archive restore in all-views; no-selection in empty-actions; paired workflow unit specs cover partial failures and concurrent active records. | -| Tab | `switchView` at `src/manager.ts:306` → `rust/src/manager.rs:650`; clear confirmation/status, switch view, reload destination store. | Same in reverse. | Both Tab directions in all-views and all archive exit sessions. | -| Esc | `src/manager.ts:404`: if confirming, clear confirmation/status and stay; otherwise cleanly exit. Rust `rust/src/manager.rs:439` is identical. | Same. | Active confirmation cancel and active exit; archive confirmation cancel and archive exit. | +| `j` / Down | `handleActiveKey` `src/manager.ts:330` → `handle_active_key` `rust/src/manager.rs:478`; increment/clamp active selection, changing list highlight and detail. | `handleArchiveKey` `src/manager.ts:366` → `handle_archive_key` `rust/src/manager.rs:527`; increment/clamp archive selection and detail. | `active-j`, `active-arrow-down`, `archives-j`, `archives-arrow-down`. | +| `k` / Up | `src/manager.ts:328` → `rust/src/manager.rs:475`; decrement/saturate. | `src/manager.ts:364` → `rust/src/manager.rs:524`; decrement/saturate. | Corresponding `k` and arrow-up steps. | +| `y` | `copy` at `src/manager.ts:218` receives the selected annotation → `ManagerApp::copy` at `rust/src/manager.rs:547`; exact one-item Markdown is copied; success exits 0, failure/empty displays status. | Same helpers receive the selected archive's annotations in newest-first order (`src/manager.ts:368`, `rust/src/manager.rs:529`). | Failure frames in all-views, success sessions `store.manager.active-y-success` / `archives-y-success`, and empty-actions. | +| `c` | `src/manager.ts:337` copies the displayed newest-first active list → `rust/src/manager.rs:492`; success exits, failure remains. | Not handled by `src/manager.ts:346` or `rust/src/manager.rs:503`; clears any prior confirmation/status through normal dispatch, otherwise store/view unchanged. | Active failure/success and `archives-c-ignored`. | +| `C` | `copyAndArchive` at `src/manager.ts:227` → `copyAndArchiveAnnotations` at `src/archive-workflow.ts:29`; Rust `rust/src/manager.rs:554` → `copy_and_archive_annotations` at `rust/src/archive_workflow.rs:27`. Order is load → copy → append archive → remove captured active IDs. Success exits; partial failure is reported without data loss. | Not handled; same no-op/confirmation-clear behavior as archive `c`. | Failure in all-views; success and byte-diff in `store.manager.copy-archive`; `archives-C-ignored`; workflow failure ordering has paired TS/Rust unit specs. | +| `d` | `deleteSelectedAnnotation` at `src/manager.ts:245` → `rust/src/manager.rs:575`; remove selected ID through locked atomic rewrite, reload, status `Annotation deleted.` | First press records the selected archive id and renders `Press d again…`; second matching press calls `deleteSelectedArchive` (`src/manager.ts:297`, `rust/src/manager.rs:639`) and atomically removes it. No selection displays `No archive selected.` Esc or another ordinary key cancels confirmation. | Active delete, archive confirm/cancel/double-confirm in all-views; empty-actions. | +| `D` | `src/manager.ts:321` / `rust/src/manager.rs:467`: first press renders `Press Shift+D again…`; second calls `clearActive` (`src/manager.ts:257`, `rust/src/manager.rs:589`), rewrites active JSONL empty, reloads, and displays `All active annotations cleared.` | Uppercase `D` is not an archive action; it cancels a pending archive confirmation like any non-`d` archive key, otherwise no store effect. | Active confirm/cancel/double-confirm and `archives-D-ignored` in all-views. | +| `r` | `reloadActive` (`src/manager.ts:55`, `rust/src/manager.rs:80`); success status `Reloaded.`, failure status is the store error. | `reloadArchives` (`src/manager.ts:66`, `rust/src/manager.rs:95`) with the same status rule. | Both reload steps in all-views; invalid/busy store process cases cover propagated store errors. | +| `u` | Not handled in active view; clears transient status/confirmation, leaves selection and stores unchanged. | `restoreSelectedArchive` at `src/manager.ts:271` → `restoreArchivedSet` at `src/archive-workflow.ts:71`; Rust `rust/src/manager.rs:606` → `restore_archived_set` at `rust/src/archive_workflow.rs:96`. Order is merge missing annotation IDs, then remove archive; partial removal failure keeps the archive and reports it. | `active-u-ignored`; archive restore in all-views; no-selection in empty-actions; paired workflow unit specs cover partial failures and concurrent active records. | +| Tab | `switchView` at `src/manager.ts:308` → `rust/src/manager.rs:651`; clear confirmation/status, switch view, reload destination store. | Same in reverse. | Both Tab directions in all-views and all archive exit sessions. | +| Esc | `src/manager.ts:406`: if confirming, clear confirmation/status and stay; otherwise cleanly exit. Rust `rust/src/manager.rs:440` is identical. | Same. | Active confirmation cancel and active exit; archive confirmation cancel and archive exit. | | `q` | Common dispatcher exits 0 through cleanup. | Same. | Active `q` in all-views; `screen.manager.q-archives`. | | Ctrl+C | Common dispatcher exits 0 through cleanup. | Same. | `screen.manager.control-c-active` and `control-c-archives`. | @@ -152,9 +156,9 @@ also fed by `screen.manager.all-views`. |---|---|---|---| | Markdown export | `formatAnnotations` at `src/format.ts:44`; `fenceFor` at `:38`. | `format_annotations` at `rust/src/format.rs:67`; `fence_for` at `:52`. | Both emit `# Annotated context`, then newest-first `## Annotation N` sections. Optional source is `workspace_label / tab_label`; selected text and comment retain line breaks; selected text uses a backtick fence one longer than its longest run (minimum three); duplicate blank lines are collapsed; the document ends in exactly one `\n`. Populated/single copy and cross-read compare raw clipboard bytes using backticks, multiline text, and wide characters. | | Terminal-safe text | `sanitizeTerminalText`/`wrapText` at `src/format.ts:5`/`:12`. | `sanitize_terminal_text`/`wrap_text` at `rust/src/format.rs:7`/`:26`. | Control characters are removed except newline/tab, tabs become four spaces, CRLF becomes LF, explicit newlines are preserved, and wrapping uses terminal cells. Editor and both manager views compare resulting cells. | -| Width and clipping | `charWidth`, `stringWidth`, `truncateToWidth` at `src/width.ts:37`/`:48`/`:55`; manager `clipped` at `src/manager.ts:75`. | `char_width`, `string_width`, `truncate_to_width` at `rust/src/width.rs:35`/`:47`/`:52`; manager `clipped` at `rust/src/manager.rs:680`. | Same zero-width controls/combining ranges and same wide ranges; truncation never splits a wide glyph and adds exactly one ellipsis cell. Wide input occurs in editor, list, detail, metadata, and archive snapshots. | +| Width and clipping | `charWidth`, `stringWidth`, `truncateToWidth` at `src/width.ts:37`/`:48`/`:55`; manager `clipped` at `src/manager.ts:77`. | `char_width`, `string_width`, `truncate_to_width` at `rust/src/width.rs:35`/`:47`/`:52`; manager `clipped` at `rust/src/manager.rs:681`. | Same zero-width controls/combining ranges and same wide ranges; truncation never splits a wide glyph and adds exactly one ellipsis cell. Wide input occurs in editor, list, detail, metadata, and archive snapshots. | | Editor geometry | `render` at `src/editor.ts:77` and `layoutComment` at `src/layout.ts:9`. | `EditorApp::draw` at `rust/src/editor.rs:67` and `layout_comment` at `rust/src/layout.rs:14`. | At 86×22, identical selected-text cap/overflow marker, comment viewport, cursor cell, footer/status placement, and full clears. Every editor input has a post-step grid diff. | -| Manager geometry and labels | `render`/active/archive/footer at `src/manager.ts:196`/`:93`/`:136`/`:182`. | `ManagerApp::draw`/active/archive/footer at `rust/src/manager.rs:109`/`:161`/`:299`/`:408`. | At 98×28, identical 36%-clamped list, divider, selected marker/reverse cell region, detail width, newest-first labels, counts, metadata line, preview overflow, empty-state text, confirmation footer, help footer, and transient status. The archive clipping boundary that exposed the earlier one-cell defect is in every seeded archive screen. | +| Manager geometry and labels | `render`/active/archive/footer at `src/manager.ts:198`/`:95`/`:138`/`:184`. | `ManagerApp::draw`/active/archive/footer at `rust/src/manager.rs:110`/`:162`/`:300`/`:409`. | At 98×28, identical 36%-clamped list, divider, selected marker/reverse cell region, detail width, newest-first labels, counts, metadata line, preview overflow, empty-state text, confirmation footer, help footer, and transient status. The archive clipping boundary that exposed the earlier one-cell defect is in every seeded archive screen. | ## Filesystem effects @@ -215,6 +219,24 @@ Clipboard candidates and arguments are defined at `src/clipboard.ts:13`/`:30` an | Windows | `powershell.exe -NoProfile -NonInteractive -Command "Get-Clipboard -Raw"` | `powershell.exe -NoProfile -NonInteractive -Command "$input | Set-Clipboard"` | | Linux/other Unix | `wl-paste --no-newline`; `xclip -selection clipboard -out`; `xsel --clipboard --output` | `wl-copy`; `xclip -selection clipboard -in`; `xsel --clipboard --input` | +### Pane copies and OSC 52 + +Herdr 0.9.0 forwards OSC 52 sequences emitted by pane output to the viewing client, so a copy made +inside a pane can reach the clipboard of the machine the person is sitting at even when the plugin +runs on a different server. Only the manager pane can use this path: the global `copy-context` and +`copy-archive` actions run with piped stdout and no terminal, so their behavior is unchanged. + +| Behavior | TypeScript | Rust | Proof | +|---|---|---|---| +| Sequence encoding | `osc52ClipboardSequence` at `src/pane-clipboard.ts:18` builds `ESC ] 52 ; c ; BEL`. | `osc52_clipboard_sequence` at `rust/src/pane_clipboard.rs:42` over the same bytes, with a hand-rolled base64 at `:17`. | `test/pane-clipboard.test.ts` and `pane_clipboard::tests` assert the exact bytes for `hi` (`\x1b]52;c;aGk=\x07`), an empty string, and multi-byte UTF-8; `store.manager.*` compares the sequences observed on real PTYs. | +| Emission point | `paneClipboardWriter` at `src/pane-clipboard.ts:34` wraps `writeClipboard`; `src/manager.ts:41` injects it into both manager copy call sites. | `write_pane_clipboard` at `rust/src/pane_clipboard.rs:64` wrapped by `pane_clipboard_write` at `rust/src/manager.rs:703` and injected at `:548`/`:558`. | Both runtimes emit exactly once per writer call, after the native attempt and before the next frame, so the emitted sequence lists match step for step. | +| Native failure | The native error is kept and prefixed, so the manager stays open and reports the copy that did land. | Identical string through `map_err`. | `store.manager.osc52-remote-copy` compares the status frame, the emitted sequence, and the resulting store. | +| Oversized payloads | `exceedsCommonOsc52Limit` at `src/pane-clipboard.ts:26`. | `exceeds_common_osc52_limit` at `rust/src/pane_clipboard.rs:47`. | Terminals commonly refuse a base64 payload over 74994 bytes. Both runtimes emit the full sequence anyway and never truncate a copy; the predicate exists so the limit is stated rather than silently applied. | + +Replacing one runtime's emitter with a no-op and rerunning the harness fails six cases +(`screen.manager.all-views`, the three `*-success` sessions, `store.manager.copy-archive`, and +`store.manager.osc52-remote-copy`), so these comparisons are not vacuously green. + Readers return the first exit-0 stdout, decoded with UTF-8 replacement. Writers pipe the exact UTF-8 Markdown to stdin and accept the first exit-0 adapter. Child stdout/stderr is suppressed. The fake adapters log every attempt and capture writer stdin; macOS and Ubuntu CI together prove both Unix @@ -239,7 +261,7 @@ the prefix exactly as shown. | Exact string or template | TypeScript emitter | Rust emitter | Proof | |---|---|---|---| -| `HERDR_PLUGIN_STATE_DIR is not set` | `src/capture.ts:22`, `src/export.ts:9`, `src/export-archive.ts:48`, `src/manager.ts:32` | `rust/src/cli.rs:69`/`:125`/`:185`, `rust/src/manager.rs:719` | Missing-state process cases. | +| `HERDR_PLUGIN_STATE_DIR is not set` | `src/capture.ts:22`, `src/export.ts:9`, `src/export-archive.ts:48`, `src/manager.ts:33` | `rust/src/cli.rs:69`/`:125`/`:185`, `rust/src/manager.rs:725` | Missing-state process cases. | | `HERDR_PLUGIN_ROOT is not set` | `src/capture.ts:24`, `src/open-manager.ts:7` | `rust/src/cli.rs:70`/`:215` | Missing-root cases. | | `No supported clipboard reader is available` | `src/clipboard.ts:59` | `rust/src/clipboard.rs:106` | `process.capture.no-clipboard`. | | `Missing pending annotation` | `src/editor.ts:32` | `rust/src/editor.rs:326` | `process.editor.missing-pending`. | @@ -254,10 +276,11 @@ the prefix exactly as shown. | `Annotations are busy; try again.` / `Archives are busy; try again.` | `src/store.ts:224`/`:235` | `rust/src/store.rs:314`/`:324` | Active busy-lock differential plus paired per-store tests. | | `Unable to update annotations[ ()]` / `Unable to update archives[ ()]` | prefixes supplied at `src/store.ts:135`/`:147` | `rust/src/store.rs:202`/`:223` | Static catalog and rewrite failure unit paths. | | `Nothing to copy.` | `src/manager-copy.ts:18` | `rust/src/manager_copy.rs:20` | `screen.manager.empty-actions`. | +| `Copied to this terminal. Server clipboard: ` | `src/pane-clipboard.ts:42` | `rust/src/pane_clipboard.rs:71` | `store.manager.osc52-remote-copy` and `screen.manager.all-views` failure frames, plus paired unit specs. | | `Nothing to copy and archive.` | `src/archive-workflow.ts:35` | `rust/src/archive_workflow.rs:44` | `screen.manager.empty-actions`. | -| `No archive selected.` | `src/manager.ts:272`/`:349` | `rust/src/manager.rs:504`/`:608` | `screen.manager.empty-actions`. | -| `Copied and archived, but active annotations remain: ` | `src/manager.ts:237`, `src/export-archive.ts:35` | `rust/src/manager.rs:569`, `rust/src/cli.rs:167` | Paired workflow partial-failure tests plus static catalog. | -| `Annotations restored, but the archive remains: ` | `src/manager.ts:287` | `rust/src/manager.rs:627` | Paired workflow partial-failure tests plus static catalog. | +| `No archive selected.` | `src/manager.ts:274`/`:351` | `rust/src/manager.rs:505`/`:609` | `screen.manager.empty-actions`. | +| `Copied and archived, but active annotations remain: ` | `src/manager.ts:239`, `src/export-archive.ts:35` | `rust/src/manager.rs:570`, `rust/src/cli.rs:167` | Paired workflow partial-failure tests plus static catalog. | +| `Annotations restored, but the archive remains: ` | `src/manager.ts:289` | `rust/src/manager.rs:628` | Paired workflow partial-failure tests plus static catalog. | | Child `herdr` stderr, or `herdr failed` | `src/herdr.ts:20` | `rust/src/herdr.rs:32`–`:39` | Manage child-stderr and empty-stderr cases; capture pane failure. | `Unable to save annotation.` (with a period) at `src/editor.ts:146` is specifically a Bun dynamic @@ -289,8 +312,10 @@ Notable differential groups: - `screen.editor.*`: both pending sources, every requested edit key, save branches, cancel keys, and SIGTERM. - `screen.manager.*`: both views, every view-valid key, ignored cross-view keys by handler mapping, both confirmation flows, empty actions, copy failure, exit keys, and SIGHUP. -- `store.manager.*`: successful clipboard-only and copy/archive products; `store.cross-read` proves - each implementation parses and exports the other's editor-written record. +- `store.manager.*`: successful clipboard-only and copy/archive products; + `store.manager.osc52-remote-copy` copies with the native writer failing, which is the remote-server + shape of issue #40; `store.cross-read` proves each implementation parses and exports the other's + editor-written record. The harness itself asserts the requested key-coverage set before it can print green. @@ -310,8 +335,8 @@ No TypeScript source changed. ## Deliberate divergences There is one. TypeScript delegates manager timestamps to `Date.prototype.toLocaleString()` and the -host's locale database (`src/manager.ts:132`, `:158`, `:169`). Rust parses into local time and emits -the en-US shape explicitly (`rust/src/manager.rs:690`). The persisted ISO timestamp, ordering, export, +host's locale database (`src/manager.ts:134`, `:160`, `:171`). Rust parses into local time and emits +the en-US shape explicitly (`rust/src/manager.rs:691`). The persisted ISO timestamp, ordering, export, and en-US display are identical. A non-en-US host can display localized punctuation/order in TypeScript while Rust stays en-US. The harness pins UTC and en-US and compares the literal seeded timestamp cells; it does not normalize them. diff --git a/docs/rust-lite-parity.md b/docs/rust-lite-parity.md index ad0ffdb..693949c 100644 --- a/docs/rust-lite-parity.md +++ b/docs/rust-lite-parity.md @@ -21,7 +21,8 @@ separately so test coverage is not confused with behavior observed inside Herdr. save delay, Esc/Ctrl+C cancellation, raw terminal restoration. - [x] `manager.ts` → `herdr-annotate manager`: active/archive views, newest-first lists, detail panes, navigation, copy one/all, copy-and-archive, delete, double-confirm clear, restore, double-confirm - permanent archive deletion, reload, status messages, Esc/Tab/q/Ctrl+C behavior. + permanent archive deletion, reload, status messages, Esc/Tab/q/Ctrl+C behavior. Every pane copy also + emits OSC 52 so Herdr 0.9.0 forwards it to the viewing client's terminal. - [x] `lite-rs/herdr-plugin.toml` preserves plugin id `annotate`, action ids, pane ids, placements, dimensions, contexts, and supported platform declarations. All commands are the one native binary. `scripts/parity-lite.py` compares those declarations against `lite/herdr-plugin.toml` and the root @@ -43,6 +44,9 @@ separately so test coverage is not confused with behavior observed inside Herdr. splitting truncation/wrapping, terminal-cell cursor coordinates. - [x] `handoff.ts`: per-user runtime/temp path, 15-second freshness, read-once removal, blank rejection. - [x] `clipboard.ts`: `pbpaste`/`pbcopy`; PowerShell raw get/set; Linux Wayland then xclip then xsel. +- [x] `pane-clipboard.ts`: identical `ESC ] 52 ; c ; BEL` bytes over the raw UTF-8 text, + emitted after the native write at the same two manager call sites, the same composed status when + only the terminal copy lands, and the same advisory 74994-byte payload limit with no truncation. - [x] `herdr.ts`: `HERDR_BIN_PATH` override, stderr projection, best-effort notifications. - [x] `archive-workflow.ts` and `manager-copy.ts`: operation order and partial-failure states, including preserving a concurrently saved annotation. @@ -64,6 +68,7 @@ document-anchor/API wire shape, not Lite's existing terminal-selection JSONL sha | `test/archive-workflow.test.ts` | `archive_workflow::tests` (8 tests) | | `test/export-archive.test.ts` | `cli::tests::copy_archive_maps_every_outcome_to_its_notification_and_exit_status` | | `test/manager-copy.test.ts` | `manager_copy::tests` (3 tests) | +| `test/pane-clipboard.test.ts` | `pane_clipboard::tests` (7 tests, including base64 chunk padding) | Additional Rust-only coverage: diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 296b188..a8d38ae 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -9,6 +9,7 @@ pub mod herdr; pub mod layout; pub mod manager; pub mod manager_copy; +pub mod pane_clipboard; pub mod paths; pub mod store; pub mod types; diff --git a/rust/src/manager.rs b/rust/src/manager.rs index ea5ae26..a8eb5d8 100644 --- a/rust/src/manager.rs +++ b/rust/src/manager.rs @@ -20,6 +20,7 @@ use crate::clipboard::write_clipboard; use crate::editor::now_iso_for_manager; use crate::format::{sanitize_terminal_text, wrap_text}; use crate::manager_copy::{ManagerCopyOutcome, copy_annotations}; +use crate::pane_clipboard::{emit_to_terminal, write_pane_clipboard}; use crate::paths::state_dir; use crate::store::{ append_archived_set, load_annotations, load_archived_sets, merge_annotations, @@ -544,7 +545,7 @@ impl ManagerApp { } fn copy(&mut self, items: &[Annotation]) { - match copy_annotations(items, write_clipboard) { + match copy_annotations(items, pane_clipboard_write) { ManagerCopyOutcome::Close => self.quit = true, ManagerCopyOutcome::StayOpen { message } => self.status = message, } @@ -554,7 +555,7 @@ impl ManagerApp { let dir = self.dir.clone(); let outcome = copy_and_archive_annotations(CopyAndArchiveDependencies { load_active: || load_annotations(&dir), - write_clipboard: |text: String| write_clipboard(&text), + write_clipboard: |text: String| pane_clipboard_write(&text), save_archive: |archive: ArchivedAnnotationSet| append_archived_set(&dir, &archive), remove_active: |ids: Vec| remove_annotations_by_id(&dir, &ids), create_archive_id: || Uuid::new_v4().to_string(), @@ -698,6 +699,11 @@ fn format_timestamp(value: &str) -> String { ) } +/// Write a pane copy to the native clipboard and to the viewing client through OSC 52. +fn pane_clipboard_write(text: &str) -> Result<(), String> { + write_pane_clipboard(text, write_clipboard, emit_to_terminal) +} + fn render_line(frame: &mut Frame<'_>, x: usize, y: usize, text: &str, width: usize, style: Style) { let (Ok(x), Ok(y), Ok(width)) = (u16::try_from(x), u16::try_from(y), u16::try_from(width)) else { diff --git a/rust/src/pane_clipboard.rs b/rust/src/pane_clipboard.rs new file mode 100644 index 0000000..072b3d1 --- /dev/null +++ b/rust/src/pane_clipboard.rs @@ -0,0 +1,158 @@ +//! Clipboard writes performed from inside a Herdr pane. +//! +//! Herdr 0.9.0 forwards OSC 52 sequences emitted by pane output to the viewing client, so a copy +//! made inside a pane can reach the clipboard of the machine the person is sitting at rather than +//! only the machine the plugin runs on. Panes emit the sequence in addition to the native clipboard +//! write; the global actions run with piped stdout and no terminal, so they cannot use this path. + +use std::io::Write; + +/// Base64 payload size that terminals commonly refuse beyond. Herdr forwards whatever the terminal +/// accepts, so this is advisory only: oversized text is still emitted in full, never truncated. +pub const OSC52_COMMON_PAYLOAD_LIMIT_BYTES: usize = 74_994; + +const BASE64_ALPHABET: &[u8; 64] = + b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; + +fn base64(bytes: &[u8]) -> String { + let mut encoded = String::with_capacity(bytes.len().div_ceil(3) * 4); + for chunk in bytes.chunks(3) { + let first = u32::from(chunk.first().copied().unwrap_or(0)); + let second = u32::from(chunk.get(1).copied().unwrap_or(0)); + let third = u32::from(chunk.get(2).copied().unwrap_or(0)); + let value = (first << 16) | (second << 8) | third; + let indexes = [ + (value >> 18) & 0x3F, + (value >> 12) & 0x3F, + (value >> 6) & 0x3F, + value & 0x3F, + ]; + for (offset, index) in indexes.into_iter().enumerate() { + if offset > chunk.len() { + encoded.push('='); + } else if let Some(symbol) = BASE64_ALPHABET.get(index as usize) { + encoded.push(char::from(*symbol)); + } + } + } + encoded +} + +/// Build the OSC 52 sequence that sets the terminal clipboard selection to `text`. +pub fn osc52_clipboard_sequence(text: &str) -> String { + format!("\x1b]52;c;{}\x07", base64(text.as_bytes())) +} + +/// Whether the encoded payload is larger than terminals commonly accept. +pub fn exceeds_common_osc52_limit(text: &str) -> bool { + base64(text.as_bytes()).len() > OSC52_COMMON_PAYLOAD_LIMIT_BYTES +} + +/// Write the OSC 52 sequence to the pane's terminal. +/// +/// Emission is best effort: a terminal that drops the sequence must not fail the copy. +pub fn emit_to_terminal(sequence: &str) { + let mut stdout = std::io::stdout(); + let _ = stdout.write_all(sequence.as_bytes()); + let _ = stdout.flush(); +} + +/// Perform a pane clipboard write: the native write first, then the OSC 52 sequence. +/// +/// When only the sequence lands the result reports the copy that succeeded and keeps the native +/// failure visible instead of hiding it behind a bare success. +pub fn write_pane_clipboard( + text: &str, + write_clipboard: impl FnOnce(&str) -> Result<(), String>, + emit: impl FnOnce(&str), +) -> Result<(), String> { + let native = write_clipboard(text); + emit(&osc52_clipboard_sequence(text)); + native.map_err(|message| format!("Copied to this terminal. Server clipboard: {message}")) +} + +#[cfg(test)] +mod tests { + use std::cell::RefCell; + + use super::*; + + #[test] + fn plain_text_encodes_to_the_exact_osc52_bytes() { + let sequence = osc52_clipboard_sequence("hi"); + assert_eq!(sequence, "\x1b]52;c;aGk=\x07"); + assert_eq!( + sequence.as_bytes(), + &[ + 0x1b, 0x5d, 0x35, 0x32, 0x3b, 0x63, 0x3b, 0x61, 0x47, 0x6b, 0x3d, 0x07 + ] + ); + } + + #[test] + fn an_empty_string_encodes_to_an_empty_payload() { + assert_eq!(osc52_clipboard_sequence(""), "\x1b]52;c;\x07"); + } + + #[test] + fn multi_byte_text_encodes_its_raw_utf8_bytes() { + assert_eq!( + osc52_clipboard_sequence("한글 · é"), + "\x1b]52;c;7ZWc6riAIMK3IMOp\x07" + ); + } + + #[test] + fn base64_pads_every_chunk_length() { + assert_eq!(base64(b""), ""); + assert_eq!(base64(b"a"), "YQ=="); + assert_eq!(base64(b"ab"), "YWI="); + assert_eq!(base64(b"abc"), "YWJj"); + assert_eq!(base64(b"abcd"), "YWJjZA=="); + assert_eq!(base64(&[0xFF, 0xFE, 0xFD]), "//79"); + } + + #[test] + fn oversized_payloads_are_reported_without_truncation() { + let text = "a".repeat(OSC52_COMMON_PAYLOAD_LIMIT_BYTES); + assert!(!exceeds_common_osc52_limit("hi")); + assert!(exceeds_common_osc52_limit(&text)); + assert!(osc52_clipboard_sequence(&text).contains(&base64(text.as_bytes()))); + } + + #[test] + fn a_successful_native_write_emits_the_sequence_afterwards() { + let order = RefCell::new(Vec::new()); + let result = write_pane_clipboard( + "hi", + |text| { + order.borrow_mut().push(format!("native:{text}")); + Ok(()) + }, + |sequence| order.borrow_mut().push(format!("emit:{sequence}")), + ); + assert_eq!(result, Ok(())); + assert_eq!( + *order.borrow(), + vec!["native:hi".to_owned(), "emit:\x1b]52;c;aGk=\x07".to_owned()] + ); + } + + #[test] + fn a_failed_native_write_still_emits_and_reports_the_terminal_copy() { + let emitted = RefCell::new(Vec::new()); + let result = write_pane_clipboard( + "hi", + |_| Err("No supported clipboard writer is available".to_owned()), + |sequence| emitted.borrow_mut().push(sequence.to_owned()), + ); + assert_eq!( + result, + Err( + "Copied to this terminal. Server clipboard: No supported clipboard writer is available" + .to_owned() + ) + ); + assert_eq!(*emitted.borrow(), vec!["\x1b]52;c;aGk=\x07".to_owned()]); + } +} diff --git a/scripts/parity-lite.py b/scripts/parity-lite.py index 4927455..dd07a5d 100755 --- a/scripts/parity-lite.py +++ b/scripts/parity-lite.py @@ -4,6 +4,7 @@ from __future__ import annotations import argparse +import base64 import codecs import copy import difflib @@ -108,6 +109,7 @@ class Step: class PtyResult: screens: list[tuple[str, tuple[tuple[str, ...], ...]]] exit_code: int + osc52: list[str] class Proof: @@ -325,6 +327,7 @@ def __init__(self, command: Sequence[str], env: Mapping[str, str], cwd: Path, ro os.set_blocking(master, False) self.master = master self.grid = TerminalGrid(rows, cols) + self.raw = bytearray() def drain(self, *, quiet: float = 0.08, maximum: float = 2.0) -> None: deadline = time.monotonic() + maximum @@ -344,6 +347,7 @@ def drain(self, *, quiet: float = 0.08, maximum: float = 2.0) -> None: if self.process.poll() is not None: return continue + self.raw.extend(data) self.grid.feed(data) quiet_deadline = time.monotonic() + quiet @@ -378,6 +382,27 @@ def finish(self, timeout: float = 3.0) -> int: return code +OSC52_PATTERN = re.compile(rb"\x1b\]52;([^;]*);([A-Za-z0-9+/=]*)(?:\x07|\x1b\\)") + + +def osc52_sequences(raw: bytes) -> list[str]: + """Every OSC 52 clipboard sequence a pane wrote to its terminal, in emission order. + + Sequences are pure ASCII, so they are kept as text and stay readable in a divergence diff. + """ + return [match.group(0).decode("ascii", "backslashreplace") for match in OSC52_PATTERN.finditer(raw)] + + +def osc52_payload(sequences: Sequence[str]) -> bytes: + """The text carried by the last OSC 52 sequence, which is the copy the client keeps.""" + if not sequences: + return b"" + match = OSC52_PATTERN.fullmatch(sequences[-1].encode("ascii", "backslashreplace")) + if match is None: + return b"" + return base64.b64decode(match.group(2)) + + def safe_name(value: str) -> str: return re.sub(r"[^A-Za-z0-9_.-]+", "-", value).strip("-") @@ -684,7 +709,13 @@ def pty_pair( session.send_signal(step.process_signal) screens.append((step.label, session.grid.snapshot())) code = session.finish() - results[implementation] = (PtyResult(screens, code), state, runtime, log, clipboard_output) + results[implementation] = ( + PtyResult(screens, code, osc52_sequences(bytes(session.raw))), + state, + runtime, + log, + clipboard_output, + ) ts, rs = results["typescript"], results["rust"] roots = [ts[1], ts[2], rs[1], rs[2], self.workspace] self.proof.compare(f"{name}.exit", ts[0].exit_code, rs[0].exit_code) @@ -693,11 +724,23 @@ def pty_pair( self.proof.compare(f"{name}.screen.{ts_label}.label", ts_label, rs_label) self.proof.compare(f"{name}.screen.{ts_label}", ts_screen, rs_screen, screen=True) self.proof.compare(f"{name}.processes", read_process_log(ts[3], roots), read_process_log(rs[3], roots)) + self.proof.compare(f"{name}.osc52", ts[0].osc52, rs[0].osc52) if compare_clipboard: + ts_clipboard = normalize_bytes(ts[4].read_bytes() if ts[4].exists() else b"", roots) + rs_clipboard = normalize_bytes(rs[4].read_bytes() if rs[4].exists() else b"", roots) + self.proof.compare(f"{name}.clipboard", ts_clipboard, rs_clipboard) + # Every pane copy reaches the viewing client too: the payload the terminal received is + # byte-for-byte the text the native writer was handed. + def readable(value: bytes) -> str: + return value.decode("utf-8", "backslashreplace") + self.proof.compare( - f"{name}.clipboard", - normalize_bytes(ts[4].read_bytes() if ts[4].exists() else b"", roots), - normalize_bytes(rs[4].read_bytes() if rs[4].exists() else b"", roots), + f"{name}.osc52-payload", + [readable(ts_clipboard), readable(rs_clipboard)], + [ + readable(normalize_bytes(osc52_payload(ts[0].osc52), roots)), + readable(normalize_bytes(osc52_payload(rs[0].osc52), roots)), + ], ) if compare_state: self.proof.compare(f"{name}.state", state_snapshot(ts[1], roots), state_snapshot(rs[1], roots)) @@ -1379,6 +1422,24 @@ def run_screen_and_store_layer(harness: Harness) -> tuple[Path, Path]: compare_clipboard=True, ) + # A pane copy on a machine with no working clipboard writer: the native write fails, the OSC 52 + # sequence still reaches the viewing client, and the manager stays open saying so. + harness.pty_pair( + "store.manager.osc52-remote-copy", + "manager", + [ + Step("copy-one", b"y", ("manager:active:y",)), + Step("quit", b"q", ("manager:active:q",)), + ], + "Annotations (", + 28, + 98, + manager_seed, + {"PARITY_CLIPBOARD_FAIL": "write"}, + compare_state=True, + compare_clipboard=True, + ) + harness.proof.require_coverage(EDITOR_REQUIRED | MANAGER_REQUIRED) return editor_ts, editor_rs diff --git a/src/manager.ts b/src/manager.ts index bac4c6c..1761728 100644 --- a/src/manager.ts +++ b/src/manager.ts @@ -6,6 +6,7 @@ import { writeClipboard } from "./clipboard"; import { sanitizeTerminalText, wrapText } from "./format"; import { stringWidth, truncateToWidth } from "./width"; import { copyAnnotations } from "./manager-copy"; +import { paneClipboardWriter } from "./pane-clipboard"; import { stateDir } from "./paths"; import { appendArchivedSet, @@ -37,6 +38,7 @@ function requireStateDir(): string { const dir = requireStateDir(); const out = (value: string) => process.stdout.write(value); +const writePaneClipboard = paneClipboardWriter(writeClipboard, out); let annotations: Annotation[] = []; let archives: ArchivedAnnotationSet[] = []; let activeSelected = 0; @@ -214,7 +216,7 @@ function render(): void { } function copy(items: readonly Annotation[]): void { - const outcome = copyAnnotations(items, writeClipboard); + const outcome = copyAnnotations(items, writePaneClipboard); if (outcome._tag === "stay_open") { status = outcome.message; return; @@ -225,7 +227,7 @@ function copy(items: readonly Annotation[]): void { function copyAndArchive(): void { const outcome = copyAndArchiveAnnotations({ loadActive: () => loadAnnotations(dir), - writeClipboard, + writeClipboard: writePaneClipboard, saveArchive: (archive) => appendArchivedSet(dir, archive), removeActive: (annotationIds) => removeAnnotationsById(dir, annotationIds), createArchiveId: () => crypto.randomUUID(), diff --git a/src/pane-clipboard.ts b/src/pane-clipboard.ts new file mode 100644 index 0000000..e2bbc4d --- /dev/null +++ b/src/pane-clipboard.ts @@ -0,0 +1,44 @@ +/** + * Clipboard writes performed from inside a Herdr pane. + * + * Herdr 0.9.0 forwards OSC 52 sequences emitted by pane output to the viewing client, so a copy made + * inside a pane can reach the clipboard of the machine the person is sitting at rather than only the + * machine the plugin runs on. Panes emit the sequence in addition to the native clipboard write; the + * global actions run with piped stdout and no terminal, so they cannot use this path. + */ +import type { ClipboardResult } from "./clipboard"; + +/** + * Base64 payload size that terminals commonly refuse beyond. Herdr forwards whatever the terminal + * accepts, so this is advisory only: oversized text is still emitted in full, never truncated. + */ +export const OSC52_COMMON_PAYLOAD_LIMIT_BYTES = 74994; + +/** Build the OSC 52 sequence that sets the terminal clipboard selection to `text`. */ +export function osc52ClipboardSequence(text: string): string { + return `\x1b]52;c;${Buffer.from(text, "utf8").toString("base64")}\x07`; +} + +/** Whether the encoded payload is larger than terminals commonly accept. */ +export function exceedsCommonOsc52Limit(text: string): boolean { + return Buffer.from(text, "utf8").toString("base64").length > OSC52_COMMON_PAYLOAD_LIMIT_BYTES; +} + +/** + * Wrap a native clipboard writer so every pane copy also reaches the viewing client's terminal. + * + * The native write is attempted first and the sequence is emitted afterwards, so a server without a + * clipboard writer still delivers the copy. When only the sequence lands the result reports the copy + * that succeeded and keeps the native failure visible instead of hiding it behind a bare success. + */ +export function paneClipboardWriter( + writeClipboard: (text: string) => ClipboardResult, + emit: (sequence: string) => void, +): (text: string) => ClipboardResult { + return (text: string) => { + const native = writeClipboard(text); + emit(osc52ClipboardSequence(text)); + if (native.ok) return native; + return { ok: false, message: `Copied to this terminal. Server clipboard: ${native.message}` }; + }; +} diff --git a/test/pane-clipboard.test.ts b/test/pane-clipboard.test.ts new file mode 100644 index 0000000..442312e --- /dev/null +++ b/test/pane-clipboard.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, test } from "bun:test"; +import { + OSC52_COMMON_PAYLOAD_LIMIT_BYTES, + exceedsCommonOsc52Limit, + osc52ClipboardSequence, + paneClipboardWriter, +} from "../src/pane-clipboard"; + +describe("OSC 52 encoding", () => { + test("emits the exact bytes for plain text", () => { + const sequence = osc52ClipboardSequence("hi"); + + expect(sequence).toBe("\x1b]52;c;aGk=\x07"); + expect([...Buffer.from(sequence, "utf8")]).toEqual([ + 0x1b, 0x5d, 0x35, 0x32, 0x3b, 0x63, 0x3b, 0x61, 0x47, 0x6b, 0x3d, 0x07, + ]); + }); + + test("emits an empty payload for an empty string", () => { + expect(osc52ClipboardSequence("")).toBe("\x1b]52;c;\x07"); + }); + + test("encodes the raw UTF-8 bytes of multi-byte text", () => { + expect(osc52ClipboardSequence("한글 · é")).toBe("\x1b]52;c;7ZWc6riAIMK3IMOp\x07"); + expect(osc52ClipboardSequence("한글 · é")).toBe( + `\x1b]52;c;${Buffer.from("한글 · é", "utf8").toString("base64")}\x07`, + ); + }); + + test("reports oversized payloads without truncating them", () => { + const text = "a".repeat(OSC52_COMMON_PAYLOAD_LIMIT_BYTES); + + expect(exceedsCommonOsc52Limit("hi")).toBe(false); + expect(exceedsCommonOsc52Limit(text)).toBe(true); + expect(osc52ClipboardSequence(text)).toContain(Buffer.from(text, "utf8").toString("base64")); + }); +}); + +describe("pane clipboard writer", () => { + test("emits the sequence after a successful native write", () => { + const order: string[] = []; + const write = paneClipboardWriter( + (text) => { + order.push(`native:${text}`); + return { ok: true, value: undefined }; + }, + (sequence) => order.push(`emit:${sequence}`), + ); + + expect(write("hi")).toEqual({ ok: true, value: undefined }); + expect(order).toEqual(["native:hi", "emit:\x1b]52;c;aGk=\x07"]); + }); + + test("still emits and reports the terminal copy when the native write fails", () => { + const emitted: string[] = []; + const write = paneClipboardWriter( + () => ({ ok: false, message: "No supported clipboard writer is available" }), + (sequence) => emitted.push(sequence), + ); + + expect(write("hi")).toEqual({ + ok: false, + message: "Copied to this terminal. Server clipboard: No supported clipboard writer is available", + }); + expect(emitted).toEqual(["\x1b]52;c;aGk=\x07"]); + }); +}); From 3ac3d42a9e01d8855b0dfbc57f7bbf53dfc269b3 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Tue, 8 Sep 2026 12:58:24 -0700 Subject: [PATCH 2/2] fix: treat a pane copy the terminal received as a success A remote server commonly has no clipboard tool at all, so the native write fails on every copy there. Reporting that as a failure defeated the case the change is for: the OSC 52 copy had already reached the person's clipboard, and copy-and-archive returned before its archive step, so Shift+C never archived. Either destination landing is now a successful copy, in both runtimes. Only a copy that reached neither carries the native error, which a pane with a live terminal cannot produce. Manager copy failure is therefore unreachable, so the copy-failure steps are gone from the all-views parity case and three osc52-remote-* cases replace them, asserting that Shift+C still writes its archive and clears the active list when only the terminal copy landed. refs #40 --- README.md | 5 +-- docs/rust-lite-parity-proof.md | 36 ++++++++++++--------- docs/rust-lite-parity.md | 7 ++-- rust/src/pane_clipboard.rs | 56 +++++++++++++++++++++----------- scripts/parity-lite.py | 59 +++++++++++++++++++++------------- src/manager.ts | 4 +-- src/pane-clipboard.ts | 29 +++++++++++++---- test/pane-clipboard.test.ts | 25 +++++++++++--- 8 files changed, 146 insertions(+), 75 deletions(-) diff --git a/README.md b/README.md index e567705..ef10a3b 100644 --- a/README.md +++ b/README.md @@ -146,8 +146,9 @@ herdr server reload-config | `Ctrl+B M` | manage · `y` copy one · `c` copy all · `Shift+C` copy and archive · `Tab` archives (`y` copy · `u` restore · `d d` delete) | Copies made inside the manager pane also emit OSC 52, so on Herdr 0.9.0 they reach the clipboard of -the machine you are viewing from even when the plugin runs on a remote server; `Ctrl+B Shift+A` and -`Ctrl+B Ctrl+A` do not, because those actions run outside a pane and have no terminal to write to. +the machine you are viewing from even when the plugin runs on a remote server with no clipboard tool +installed; `Ctrl+B Shift+A` and `Ctrl+B Ctrl+A` do not, because those actions run outside a pane and +have no terminal to write to. ### Review documents and agent replies diff --git a/docs/rust-lite-parity-proof.md b/docs/rust-lite-parity-proof.md index b323df4..ac3d874 100644 --- a/docs/rust-lite-parity-proof.md +++ b/docs/rust-lite-parity-proof.md @@ -5,7 +5,7 @@ TypeScript Lite surface to the Rust call path at function granularity and names case that compares the result. The current local result is: ```text -Parity Lite: 496 observables compared, 95 screens diffed, zero divergences / 1 deliberate +Parity Lite: 510 observables compared, 94 screens diffed, zero divergences / 1 deliberate ``` Run it from the repository root with `bash scripts/parity-lite.sh`. The shell wrapper stages a fresh @@ -138,9 +138,9 @@ also fed by `screen.manager.all-views`. |---|---|---|---| | `j` / Down | `handleActiveKey` `src/manager.ts:330` → `handle_active_key` `rust/src/manager.rs:478`; increment/clamp active selection, changing list highlight and detail. | `handleArchiveKey` `src/manager.ts:366` → `handle_archive_key` `rust/src/manager.rs:527`; increment/clamp archive selection and detail. | `active-j`, `active-arrow-down`, `archives-j`, `archives-arrow-down`. | | `k` / Up | `src/manager.ts:328` → `rust/src/manager.rs:475`; decrement/saturate. | `src/manager.ts:364` → `rust/src/manager.rs:524`; decrement/saturate. | Corresponding `k` and arrow-up steps. | -| `y` | `copy` at `src/manager.ts:218` receives the selected annotation → `ManagerApp::copy` at `rust/src/manager.rs:547`; exact one-item Markdown is copied; success exits 0, failure/empty displays status. | Same helpers receive the selected archive's annotations in newest-first order (`src/manager.ts:368`, `rust/src/manager.rs:529`). | Failure frames in all-views, success sessions `store.manager.active-y-success` / `archives-y-success`, and empty-actions. | -| `c` | `src/manager.ts:337` copies the displayed newest-first active list → `rust/src/manager.rs:492`; success exits, failure remains. | Not handled by `src/manager.ts:346` or `rust/src/manager.rs:503`; clears any prior confirmation/status through normal dispatch, otherwise store/view unchanged. | Active failure/success and `archives-c-ignored`. | -| `C` | `copyAndArchive` at `src/manager.ts:227` → `copyAndArchiveAnnotations` at `src/archive-workflow.ts:29`; Rust `rust/src/manager.rs:554` → `copy_and_archive_annotations` at `rust/src/archive_workflow.rs:27`. Order is load → copy → append archive → remove captured active IDs. Success exits; partial failure is reported without data loss. | Not handled; same no-op/confirmation-clear behavior as archive `c`. | Failure in all-views; success and byte-diff in `store.manager.copy-archive`; `archives-C-ignored`; workflow failure ordering has paired TS/Rust unit specs. | +| `y` | `copy` at `src/manager.ts:218` receives the selected annotation → `ManagerApp::copy` at `rust/src/manager.rs:547`; exact one-item Markdown is copied; success exits 0, an empty selection displays status. | Same helpers receive the selected archive's annotations in newest-first order (`src/manager.ts:368`, `rust/src/manager.rs:529`). | Success sessions `store.manager.active-y-success` / `archives-y-success` / `osc52-remote-copy`, and empty-actions. | +| `c` | `src/manager.ts:337` copies the displayed newest-first active list → `rust/src/manager.rs:492`; success exits. | Not handled by `src/manager.ts:346` or `rust/src/manager.rs:503`; clears any prior confirmation/status through normal dispatch, otherwise store/view unchanged. | `store.manager.active-c-success`, `osc52-remote-copy-all`, and `archives-c-ignored`. | +| `C` | `copyAndArchive` at `src/manager.ts:227` → `copyAndArchiveAnnotations` at `src/archive-workflow.ts:29`; Rust `rust/src/manager.rs:554` → `copy_and_archive_annotations` at `rust/src/archive_workflow.rs:27`. Order is load → copy → append archive → remove captured active IDs. Success exits; partial failure is reported without data loss. | Not handled; same no-op/confirmation-clear behavior as archive `c`. | Success and byte-diff in `store.manager.copy-archive`; `osc52-remote-copy-archive` proves the archive still runs when only the terminal copy landed; `archives-C-ignored`; workflow failure ordering has paired TS/Rust unit specs. | | `d` | `deleteSelectedAnnotation` at `src/manager.ts:245` → `rust/src/manager.rs:575`; remove selected ID through locked atomic rewrite, reload, status `Annotation deleted.` | First press records the selected archive id and renders `Press d again…`; second matching press calls `deleteSelectedArchive` (`src/manager.ts:297`, `rust/src/manager.rs:639`) and atomically removes it. No selection displays `No archive selected.` Esc or another ordinary key cancels confirmation. | Active delete, archive confirm/cancel/double-confirm in all-views; empty-actions. | | `D` | `src/manager.ts:321` / `rust/src/manager.rs:467`: first press renders `Press Shift+D again…`; second calls `clearActive` (`src/manager.ts:257`, `rust/src/manager.rs:589`), rewrites active JSONL empty, reloads, and displays `All active annotations cleared.` | Uppercase `D` is not an archive action; it cancels a pending archive confirmation like any non-`d` archive key, otherwise no store effect. | Active confirm/cancel/double-confirm and `archives-D-ignored` in all-views. | | `r` | `reloadActive` (`src/manager.ts:55`, `rust/src/manager.rs:80`); success status `Reloaded.`, failure status is the store error. | `reloadArchives` (`src/manager.ts:66`, `rust/src/manager.rs:95`) with the same status rule. | Both reload steps in all-views; invalid/busy store process cases cover propagated store errors. | @@ -229,13 +229,18 @@ runs on a different server. Only the manager pane can use this path: the global | Behavior | TypeScript | Rust | Proof | |---|---|---|---| | Sequence encoding | `osc52ClipboardSequence` at `src/pane-clipboard.ts:18` builds `ESC ] 52 ; c ; BEL`. | `osc52_clipboard_sequence` at `rust/src/pane_clipboard.rs:42` over the same bytes, with a hand-rolled base64 at `:17`. | `test/pane-clipboard.test.ts` and `pane_clipboard::tests` assert the exact bytes for `hi` (`\x1b]52;c;aGk=\x07`), an empty string, and multi-byte UTF-8; `store.manager.*` compares the sequences observed on real PTYs. | -| Emission point | `paneClipboardWriter` at `src/pane-clipboard.ts:34` wraps `writeClipboard`; `src/manager.ts:41` injects it into both manager copy call sites. | `write_pane_clipboard` at `rust/src/pane_clipboard.rs:64` wrapped by `pane_clipboard_write` at `rust/src/manager.rs:703` and injected at `:548`/`:558`. | Both runtimes emit exactly once per writer call, after the native attempt and before the next frame, so the emitted sequence lists match step for step. | -| Native failure | The native error is kept and prefixed, so the manager stays open and reports the copy that did land. | Identical string through `map_err`. | `store.manager.osc52-remote-copy` compares the status frame, the emitted sequence, and the resulting store. | +| Emission point | `paneClipboardWriter` at `src/pane-clipboard.ts:49` wraps `writeClipboard`; `src/manager.ts:41` injects it into both manager copy call sites. | `write_pane_clipboard` at `rust/src/pane_clipboard.rs:68` wrapped by `pane_clipboard_write` at `rust/src/manager.rs:703` and injected at `:548`/`:558`. | Both runtimes emit exactly once per writer call, after the native attempt and before the next frame, so the emitted sequence lists match step for step. | +| Native failure | Either destination landing is a successful copy (`src/pane-clipboard.ts:56`). A remote server commonly has no clipboard tool at all, and the sequence is the copy that actually reached the person, so it is not reported as a failure and does not stop a copy-and-archive before its archive step. | Identical rule at `rust/src/pane_clipboard.rs:75`. | The three `store.manager.osc52-remote-*` cases run with the native writer failing: the copy exits 0, `C` still writes its archive and clears the active list, and both runtimes emit the same sequence. | +| Neither destination | Only a copy that reached neither the clipboard nor the terminal fails, carrying the unchanged native error. | Same. | Paired unit specs; unreachable from a pane whose stdout is a live terminal. | | Oversized payloads | `exceedsCommonOsc52Limit` at `src/pane-clipboard.ts:26`. | `exceeds_common_osc52_limit` at `rust/src/pane_clipboard.rs:47`. | Terminals commonly refuse a base64 payload over 74994 bytes. Both runtimes emit the full sequence anyway and never truncate a copy; the predicate exists so the limit is stated rather than silently applied. | -Replacing one runtime's emitter with a no-op and rerunning the harness fails six cases -(`screen.manager.all-views`, the three `*-success` sessions, `store.manager.copy-archive`, and -`store.manager.osc52-remote-copy`), so these comparisons are not vacuously green. +Replacing one runtime's emitter with a no-op and rerunning the harness fails every manager copy +case, so these comparisons are not vacuously green. + +Because a pane's stdout is a live terminal, a manager copy no longer fails there. `No supported +clipboard writer is available` therefore remains reachable from the global actions, which have no +terminal, but not from the manager; the manager's copy-failure frames were removed from +`screen.manager.all-views` for that reason. Readers return the first exit-0 stdout, decoded with UTF-8 replacement. Writers pipe the exact UTF-8 Markdown to stdin and accept the first exit-0 adapter. Child stdout/stderr is suppressed. The fake @@ -251,7 +256,7 @@ branches. Windows arguments are source-mapped and build-checked, not run by this | Editor/manager initialization error | 1 with stderr, no action-level notification | | Editor Esc/Ctrl+C, manager Esc/q/Ctrl+C, editor SIGTERM, manager SIGHUP | 0 after terminal restoration | | Successful editor save | 0 after final `Saved.` frame and 250 ms delay | -| Manager copy/copy+archive success | 0 after clipboard/store completion; failure remains in the TUI until a later exit key | +| Manager copy/copy+archive success | 0 after clipboard/store completion; a pane copy succeeds whenever the clipboard write or the OSC 52 emission lands | ## Error and failure strings @@ -276,7 +281,6 @@ the prefix exactly as shown. | `Annotations are busy; try again.` / `Archives are busy; try again.` | `src/store.ts:224`/`:235` | `rust/src/store.rs:314`/`:324` | Active busy-lock differential plus paired per-store tests. | | `Unable to update annotations[ ()]` / `Unable to update archives[ ()]` | prefixes supplied at `src/store.ts:135`/`:147` | `rust/src/store.rs:202`/`:223` | Static catalog and rewrite failure unit paths. | | `Nothing to copy.` | `src/manager-copy.ts:18` | `rust/src/manager_copy.rs:20` | `screen.manager.empty-actions`. | -| `Copied to this terminal. Server clipboard: ` | `src/pane-clipboard.ts:42` | `rust/src/pane_clipboard.rs:71` | `store.manager.osc52-remote-copy` and `screen.manager.all-views` failure frames, plus paired unit specs. | | `Nothing to copy and archive.` | `src/archive-workflow.ts:35` | `rust/src/archive_workflow.rs:44` | `screen.manager.empty-actions`. | | `No archive selected.` | `src/manager.ts:274`/`:351` | `rust/src/manager.rs:505`/`:609` | `screen.manager.empty-actions`. | | `Copied and archived, but active annotations remain: ` | `src/manager.ts:239`, `src/export-archive.ts:35` | `rust/src/manager.rs:570`, `rust/src/cli.rs:167` | Paired workflow partial-failure tests plus static catalog. | @@ -311,11 +315,11 @@ Notable differential groups: - `process.manage.*`, `process.editor.*`, `process.manager.*`: argv/error fallback and initialization. - `screen.editor.*`: both pending sources, every requested edit key, save branches, cancel keys, and SIGTERM. - `screen.manager.*`: both views, every view-valid key, ignored cross-view keys by handler mapping, - both confirmation flows, empty actions, copy failure, exit keys, and SIGHUP. -- `store.manager.*`: successful clipboard-only and copy/archive products; - `store.manager.osc52-remote-copy` copies with the native writer failing, which is the remote-server - shape of issue #40; `store.cross-read` proves each implementation parses and exports the other's - editor-written record. + both confirmation flows, empty actions, exit keys, and SIGHUP. +- `store.manager.*`: successful clipboard-only and copy/archive products; the three + `store.manager.osc52-remote-*` cases copy with the native writer failing, which is the + remote-server shape of issue #40; `store.cross-read` proves each implementation parses and exports + the other's editor-written record. The harness itself asserts the requested key-coverage set before it can print green. diff --git a/docs/rust-lite-parity.md b/docs/rust-lite-parity.md index 693949c..c029a19 100644 --- a/docs/rust-lite-parity.md +++ b/docs/rust-lite-parity.md @@ -45,8 +45,9 @@ separately so test coverage is not confused with behavior observed inside Herdr. - [x] `handoff.ts`: per-user runtime/temp path, 15-second freshness, read-once removal, blank rejection. - [x] `clipboard.ts`: `pbpaste`/`pbcopy`; PowerShell raw get/set; Linux Wayland then xclip then xsel. - [x] `pane-clipboard.ts`: identical `ESC ] 52 ; c ; BEL` bytes over the raw UTF-8 text, - emitted after the native write at the same two manager call sites, the same composed status when - only the terminal copy lands, and the same advisory 74994-byte payload limit with no truncation. + emitted after the native write at the same two manager call sites, the same rule that either + destination landing is a successful copy, and the same advisory 74994-byte payload limit with no + truncation. - [x] `herdr.ts`: `HERDR_BIN_PATH` override, stderr projection, best-effort notifications. - [x] `archive-workflow.ts` and `manager-copy.ts`: operation order and partial-failure states, including preserving a concurrently saved annotation. @@ -68,7 +69,7 @@ document-anchor/API wire shape, not Lite's existing terminal-selection JSONL sha | `test/archive-workflow.test.ts` | `archive_workflow::tests` (8 tests) | | `test/export-archive.test.ts` | `cli::tests::copy_archive_maps_every_outcome_to_its_notification_and_exit_status` | | `test/manager-copy.test.ts` | `manager_copy::tests` (3 tests) | -| `test/pane-clipboard.test.ts` | `pane_clipboard::tests` (7 tests, including base64 chunk padding) | +| `test/pane-clipboard.test.ts` | `pane_clipboard::tests` (8 tests, including base64 chunk padding) | Additional Rust-only coverage: diff --git a/rust/src/pane_clipboard.rs b/rust/src/pane_clipboard.rs index 072b3d1..2abdd71 100644 --- a/rust/src/pane_clipboard.rs +++ b/rust/src/pane_clipboard.rs @@ -48,27 +48,34 @@ pub fn exceeds_common_osc52_limit(text: &str) -> bool { base64(text.as_bytes()).len() > OSC52_COMMON_PAYLOAD_LIMIT_BYTES } -/// Write the OSC 52 sequence to the pane's terminal. +/// Write the OSC 52 sequence to the pane's terminal, reporting whether it was written. /// -/// Emission is best effort: a terminal that drops the sequence must not fail the copy. -pub fn emit_to_terminal(sequence: &str) { +/// Best effort: a closed or broken stdout must not fail a copy with an I/O error. +pub fn emit_to_terminal(sequence: &str) -> bool { let mut stdout = std::io::stdout(); - let _ = stdout.write_all(sequence.as_bytes()); - let _ = stdout.flush(); + stdout + .write_all(sequence.as_bytes()) + .and_then(|()| stdout.flush()) + .is_ok() } /// Perform a pane clipboard write: the native write first, then the OSC 52 sequence. /// -/// When only the sequence lands the result reports the copy that succeeded and keeps the native -/// failure visible instead of hiding it behind a bare success. +/// Either one landing is a successful copy: a remote server commonly has no clipboard tool at all, +/// and the sequence is the copy that actually reached the person, so it must not be reported as a +/// failure or block the rest of a copy-and-archive. Only a copy that reached neither destination +/// fails, with the native error. pub fn write_pane_clipboard( text: &str, write_clipboard: impl FnOnce(&str) -> Result<(), String>, - emit: impl FnOnce(&str), + emit: impl FnOnce(&str) -> bool, ) -> Result<(), String> { let native = write_clipboard(text); - emit(&osc52_clipboard_sequence(text)); - native.map_err(|message| format!("Copied to this terminal. Server clipboard: {message}")) + let emitted = emit(&osc52_clipboard_sequence(text)); + if native.is_ok() || emitted { + return Ok(()); + } + native } #[cfg(test)] @@ -129,7 +136,10 @@ mod tests { order.borrow_mut().push(format!("native:{text}")); Ok(()) }, - |sequence| order.borrow_mut().push(format!("emit:{sequence}")), + |sequence| { + order.borrow_mut().push(format!("emit:{sequence}")); + true + }, ); assert_eq!(result, Ok(())); assert_eq!( @@ -139,20 +149,30 @@ mod tests { } #[test] - fn a_failed_native_write_still_emits_and_reports_the_terminal_copy() { + fn a_copy_the_terminal_received_succeeds_even_when_the_native_write_failed() { let emitted = RefCell::new(Vec::new()); let result = write_pane_clipboard( "hi", |_| Err("No supported clipboard writer is available".to_owned()), - |sequence| emitted.borrow_mut().push(sequence.to_owned()), + |sequence| { + emitted.borrow_mut().push(sequence.to_owned()); + true + }, + ); + assert_eq!(result, Ok(())); + assert_eq!(*emitted.borrow(), vec!["\x1b]52;c;aGk=\x07".to_owned()]); + } + + #[test] + fn a_copy_that_reached_neither_destination_fails_with_the_native_error() { + let result = write_pane_clipboard( + "hi", + |_| Err("No supported clipboard writer is available".to_owned()), + |_| false, ); assert_eq!( result, - Err( - "Copied to this terminal. Server clipboard: No supported clipboard writer is available" - .to_owned() - ) + Err("No supported clipboard writer is available".to_owned()) ); - assert_eq!(*emitted.borrow(), vec!["\x1b]52;c;aGk=\x07".to_owned()]); } } diff --git a/scripts/parity-lite.py b/scripts/parity-lite.py index dd07a5d..abd3ae1 100755 --- a/scripts/parity-lite.py +++ b/scripts/parity-lite.py @@ -1263,9 +1263,6 @@ def run_screen_and_store_layer(harness: Harness) -> tuple[Path, Path]: Step("active-k", b"k", ("manager:active:k",)), Step("active-arrow-down", b"\x1b[B"), Step("active-arrow-up", b"\x1b[A"), - Step("active-y-failure", b"y", ("manager:active:y",)), - Step("active-c-failure", b"c", ("manager:active:c",)), - Step("active-C-failure", b"C", ("manager:active:C",)), Step("active-delete", b"d", ("manager:active:d",)), Step("active-clear-confirm", b"D", ("manager:active:D",)), Step("active-clear-cancel", b"\x1b", ("manager:active:Esc",)), @@ -1278,7 +1275,6 @@ def run_screen_and_store_layer(harness: Harness) -> tuple[Path, Path]: Step("archives-k", b"k", ("manager:archives:k",)), Step("archives-arrow-down", b"\x1b[B"), Step("archives-arrow-up", b"\x1b[A"), - Step("archives-y-failure", b"y", ("manager:archives:y",)), Step("archives-reload", b"r", ("manager:archives:r",)), Step("archives-c-ignored", b"c", ("manager:archives:c",)), Step("archives-C-ignored", b"C", ("manager:archives:C",)), @@ -1299,7 +1295,6 @@ def run_screen_and_store_layer(harness: Harness) -> tuple[Path, Path]: 28, 98, manager_seed, - {"PARITY_CLIPBOARD_FAIL": "write"}, compare_state=True, compare_clipboard=True, ) @@ -1422,23 +1417,43 @@ def run_screen_and_store_layer(harness: Harness) -> tuple[Path, Path]: compare_clipboard=True, ) - # A pane copy on a machine with no working clipboard writer: the native write fails, the OSC 52 - # sequence still reaches the viewing client, and the manager stays open saying so. - harness.pty_pair( - "store.manager.osc52-remote-copy", - "manager", - [ - Step("copy-one", b"y", ("manager:active:y",)), - Step("quit", b"q", ("manager:active:q",)), - ], - "Annotations (", - 28, - 98, - manager_seed, - {"PARITY_CLIPBOARD_FAIL": "write"}, - compare_state=True, - compare_clipboard=True, - ) + # A pane copy on a machine with no working clipboard writer, which is the remote-server shape of + # issue #40: the native write fails, the OSC 52 sequence still reaches the viewing client, and the + # copy is a success. `C` must therefore still archive and clear the active list. + for key, steps in ( + ("osc52-remote-copy", [Step("copy-one", b"y", ("manager:active:y",))]), + ("osc52-remote-copy-all", [Step("copy-all", b"c", ("manager:active:c",))]), + ("osc52-remote-copy-archive", [Step("copy-archive", b"C", ("manager:active:C",))]), + ): + states = harness.pty_pair( + f"store.manager.{key}", + "manager", + steps, + "Annotations (", + 28, + 98, + manager_seed, + {"PARITY_CLIPBOARD_FAIL": "write"}, + compare_state=True, + compare_clipboard=True, + ) + if key != "osc52-remote-copy-archive": + continue + # The copy succeeded on the OSC 52 path alone, so copy-and-archive must have gone on to + # write its archive (a third, beside the two seeded) and clear the active list, rather than + # stopping at the copy. + for implementation, state in zip(("typescript", "rust"), states): + def records(name: str, state: Path = state) -> int: + path = state / name + if not path.exists(): + return 0 + return len([line for line in path.read_text(encoding="utf-8").splitlines() if line.strip()]) + + harness.proof.compare( + f"store.manager.{key}.archived.{implementation}", + {"archives": 3, "active": 0}, + {"archives": records("archives.jsonl"), "active": records("annotations.jsonl")}, + ) harness.proof.require_coverage(EDITOR_REQUIRED | MANAGER_REQUIRED) return editor_ts, editor_rs diff --git a/src/manager.ts b/src/manager.ts index 1761728..24e4b97 100644 --- a/src/manager.ts +++ b/src/manager.ts @@ -6,7 +6,7 @@ import { writeClipboard } from "./clipboard"; import { sanitizeTerminalText, wrapText } from "./format"; import { stringWidth, truncateToWidth } from "./width"; import { copyAnnotations } from "./manager-copy"; -import { paneClipboardWriter } from "./pane-clipboard"; +import { emitToTerminal, paneClipboardWriter } from "./pane-clipboard"; import { stateDir } from "./paths"; import { appendArchivedSet, @@ -38,7 +38,7 @@ function requireStateDir(): string { const dir = requireStateDir(); const out = (value: string) => process.stdout.write(value); -const writePaneClipboard = paneClipboardWriter(writeClipboard, out); +const writePaneClipboard = paneClipboardWriter(writeClipboard, emitToTerminal); let annotations: Annotation[] = []; let archives: ArchivedAnnotationSet[] = []; let activeSelected = 0; diff --git a/src/pane-clipboard.ts b/src/pane-clipboard.ts index e2bbc4d..c438cab 100644 --- a/src/pane-clipboard.ts +++ b/src/pane-clipboard.ts @@ -24,21 +24,36 @@ export function exceedsCommonOsc52Limit(text: string): boolean { return Buffer.from(text, "utf8").toString("base64").length > OSC52_COMMON_PAYLOAD_LIMIT_BYTES; } +/** + * Write the sequence to the pane's terminal, reporting whether it was written. + * + * Best effort: a closed or broken stdout must not throw out of a copy. + */ +export function emitToTerminal(sequence: string): boolean { + try { + process.stdout.write(sequence); + return true; + } catch { + return false; + } +} + /** * Wrap a native clipboard writer so every pane copy also reaches the viewing client's terminal. * - * The native write is attempted first and the sequence is emitted afterwards, so a server without a - * clipboard writer still delivers the copy. When only the sequence lands the result reports the copy - * that succeeded and keeps the native failure visible instead of hiding it behind a bare success. + * The native write is attempted first and the sequence is emitted afterwards. Either one landing is + * a successful copy: a remote server commonly has no clipboard tool at all, and the sequence is the + * copy that actually reached the person, so it must not be reported as a failure or block the rest + * of a copy-and-archive. Only a copy that reached neither destination fails, with the native error. */ export function paneClipboardWriter( writeClipboard: (text: string) => ClipboardResult, - emit: (sequence: string) => void, + emit: (sequence: string) => boolean, ): (text: string) => ClipboardResult { return (text: string) => { const native = writeClipboard(text); - emit(osc52ClipboardSequence(text)); - if (native.ok) return native; - return { ok: false, message: `Copied to this terminal. Server clipboard: ${native.message}` }; + const emitted = emit(osc52ClipboardSequence(text)); + if (native.ok || emitted) return { ok: true, value: undefined }; + return native; }; } diff --git a/test/pane-clipboard.test.ts b/test/pane-clipboard.test.ts index 442312e..3b9160b 100644 --- a/test/pane-clipboard.test.ts +++ b/test/pane-clipboard.test.ts @@ -44,24 +44,39 @@ describe("pane clipboard writer", () => { order.push(`native:${text}`); return { ok: true, value: undefined }; }, - (sequence) => order.push(`emit:${sequence}`), + (sequence) => { + order.push(`emit:${sequence}`); + return true; + }, ); expect(write("hi")).toEqual({ ok: true, value: undefined }); expect(order).toEqual(["native:hi", "emit:\x1b]52;c;aGk=\x07"]); }); - test("still emits and reports the terminal copy when the native write fails", () => { + test("a copy the terminal received succeeds even when the native write failed", () => { const emitted: string[] = []; const write = paneClipboardWriter( () => ({ ok: false, message: "No supported clipboard writer is available" }), - (sequence) => emitted.push(sequence), + (sequence) => { + emitted.push(sequence); + return true; + }, + ); + + expect(write("hi")).toEqual({ ok: true, value: undefined }); + expect(emitted).toEqual(["\x1b]52;c;aGk=\x07"]); + }); + + test("a copy that reached neither destination fails with the native error", () => { + const write = paneClipboardWriter( + () => ({ ok: false, message: "No supported clipboard writer is available" }), + () => false, ); expect(write("hi")).toEqual({ ok: false, - message: "Copied to this terminal. Server clipboard: No supported clipboard writer is available", + message: "No supported clipboard writer is available", }); - expect(emitted).toEqual(["\x1b]52;c;aGk=\x07"]); }); });