Skip to content

feat(tui): copy a settled selection on every platform, and stop the clipboard path lying - #360

Merged
LivXue merged 8 commits into
EverMind-AI:mainfrom
LivXue:feat/tui_copy_on_select_all_platforms
Aug 24, 2026
Merged

feat(tui): copy a settled selection on every platform, and stop the clipboard path lying#360
LivXue merged 8 commits into
EverMind-AI:mainfrom
LivXue:feat/tui_copy_on_select_all_platforms

Conversation

@LivXue

@LivXue LivXue commented Aug 23, 2026

Copy link
Copy Markdown
Member

Summary

A TUI that enables mouse tracking owns the drag, so the terminal never builds a
native selection and its own copy shortcut has nothing to copy. Copy-on-select is
what makes a transcript selection copyable at all -- but the subscription bailed
out on !isMac, so on Linux and Windows a drag highlighted text and copied
nothing. This lifts that gate, and fixes what was found while making the result
observable.

Copy-on-select on every platform. The subscription moves out of useMainApp
into subscribeCopyOnSelect(). Inlined in a hook that needs a live gateway to
start, none of its four guards could be tested; each now has a case, and each was
checked by removing only that guard and watching the suite go red. The read of the
bus state stops being an unchecked cast: the ambient useSelection() declaration
types it as unknown, so the module narrows it instead.

The copy is reported to the transcript. Nothing on screen changes when a drag
ends, so a line is written once a clipboard path has actually taken the text. The
callback fires on a non-empty result only, since copySelectionNoClear() resolves
to an empty text when nothing reached the clipboard. The first report of a session
carries the path caveat, later ones stay terse: OSC 52 is the one path a terminal
can still refuse, and the first copy is when a user is looking for the reason a
paste came up empty.

The reported path comes from the write, not from the environment. A path
predicted from env state cannot be right in general, because the environment does
not record what happened: inside tmux a load-buffer that fails falls through to
raw OSC 52 and leaves TMUX set exactly as the case that worked. With a stale
tmux socket the bytes went out as OSC 52 while both callers said "copied to the
tmux buffer", pointing the user at a set-clipboard setting that was never
involved. setClipboard() already computed the three facts that decide this --
whether native was attempted, whether the buffer loaded, whether a sequence was
emitted -- so it now reports which one took the text, and null when none did. The
value travels with the copied text out of copySelectionNoClear() and
copySelection() to the two call sites that display it. The predictor is deleted
rather than left exported from the package: after the rewiring it had no callers,
and a helper that can name a path the data did not take is a defect waiting for
the next caller. Its env-matrix cases were assertions about the predictor itself,
so what replaces them drives the real setClipboard() with tmux stubbed,
including the failed-load fallback.

Reporting an observed path also has to wait for the observation. On Linux the
native tool is discovered by a probe, and copyNative() answered the first call
before that probe settled -- a display server means a tool could exist, not that
one does. Naming the path from that optimism claimed a native write on a machine
with DISPLAY and no wl-copy, xclip or xsel, and the same call reported
failure once the probe finished. The first call now answers with the probe's own
result. It is still started before the tmux await, so the probe runs alongside
load-buffer rather than ahead of the report, and later calls still answer
synchronously from the cache. The copy never ran ahead of that probe either --
the tool is spawned inside it -- so only the report did.

The path caveat is scoped to a session, not to the process. The first copy of
a session carries the caveat and later ones stay terse, but the flag holding that
lived in a hook that outlives the session: newSession() and resumeById()
replace ui.sid without remounting useMainApp, so every session after the first
opened with the terse line and the user never learned an OSC 52 paste can come up
empty. The tally moves into a reporter keyed on a session identifier, so the state
and the thing it is scoped to live together and the boundary is unit-testable.
The hook reads the sid through getUiState(), which keeps a session change from
tearing down the bus subscription.

The report must not overclaim. OSC 52 hands bytes to the terminal and the
terminal decides whether to keep them, and nothing in the write path caps or
chunks the payload. Measured on the real code: one 200x50 viewport of CJK is a
40 KB escape sequence, and a selection dragged through 2000 rows is 536 KB in a
single sequence. Terminals drop an oversized sequence without a word, and
setClipboard() reports success for all of it because bytes were written to
stdout. So that path reports what it sent; native and tmux, which really did
write a clipboard, still say copied. The count was UTF-16 code units, which
reads three emoji as six characters and a combining accent as two, so it is
counted by grapheme now. CJK was already right, being one code unit per
character.

The documented env knobs were dead. The OSC 52 override and the clipboard
debug switch were readable only under the upstream HERMES_TUI_ names, while
every knob this project documents uses RAVEN_TUI_ and /copy's own failure hint
names RAVEN_TUI_FORCE_OSC52 and RAVEN_TUI_DEBUG_CLIPBOARD. Following that hint
changed nothing. Both spellings are now read, RAVEN_TUI_ first, with the
HERMES_TUI_ names kept as aliases so an environment that worked before still
works. The debug switch moves behind clipboardDebugEnabled() so its four call
sites cannot drift apart.

Type

  • Fix
  • Feature
  • Docs
  • CI / tooling
  • Refactor
  • Other

Two boxes: the platform gate is the feature, the clipboard-path and env-name
defects are fixes found while making it observable.

Verification

Run from ui-tui/, on Linux, node 22, at branch head after the rebase.

npx vitest run --no-file-parallelism
  Test Files  91 passed (91)
  Tests  1066 passed | 13 skipped (1079)

npx tsc --noEmit -p tsconfig.json
  clean, exit 0

npm run lint
  22 problems (0 errors, 22 warnings)

python3 -m scripts.check_commit_messages github/main..HEAD
  exit 0

The suite is run with --no-file-parallelism deliberately: the ink render tests
fail under default worker parallelism at this suite size, and no CI job runs them.

The 22 lint warnings are the baseline, not new: 22 before the branch and 22 after,
none of them in a file this branch touches.

Every file this branch touches passes prettier --check. Two of them did not at
first, and that was this branch's doing rather than the base's: dropping a name
from the @hermes/ink import and adding a wide function signature left both
inside the print width while still wrapped. They are formatted in their own
commit. The files already unformatted on github/main, chatStream.ts among
them, are left alone rather than reformatted into this diff.

Suites that exercise the changed code directly:
src/__tests__/copyOnSelect.test.ts (12 cases),
src/__tests__/clipboard.test.ts (30 cases),
packages/hermes-ink/src/ink/termio/osc.test.ts (39 cases).

An environment gap that made the first pass of this verification worthless, since
it would silently affect anyone reviewing from a worktree: ui-tui/node_modules
here was a symlink to another checkout's, so node_modules/@hermes/ink -> ../../packages/hermes-ink resolved against the symlink target and landed on that
other checkout's copy of the package. tsc and the dist build therefore read an
unmodified hermes-ink while the app code under test was the edited one. Fixed by
giving the worktree a real node_modules whose @hermes/ink points at its own
package, confirmed with tsc --listFiles naming the edited ink.tsx, then
rebuilt and re-ran everything. Every number above is from after that.

New tests were proved load-bearing rather than assumed:

  • Each of the four guards in subscribeCopyOnSelect() was removed on its own and
    the suite watched go red (1, 1, 2 and 2 failures), then restored.
  • The three cases that assert a copy did not happen are each paired with a
    positive assertion on the same subscription, because a bus that was never wired
    up satisfies the negative half by itself.
  • The path-reporting cases were checked by reverting the derivation to the deleted
    predictor's rule and watching the failed-load case report tmux-buffer again.
  • The Linux first-probe case was checked by reverting copyNative() to its
    optimistic return true and watching it fail on success: true, path: 'native'
    where it now asserts success: false, path: null. Establishing that this one
    was introduced here rather than inherited took running the same scenario
    against github/main, which reported path: 'osc52' for it: success: true
    with nothing written predates the branch, the native claim does not.
  • Of the three session-scoping cases, one is load-bearing: reverting the reporter
    to a process-wide flag fails "spends the caveat once per session". The other two
    -- a session returned to, and two reporters not sharing a tally -- pass with the
    flag as well, and are there to pin the property against a later refactor. Said
    plainly rather than implying all three prove the fix.

One pre-existing assertion is flipped rather than kept. It pinned the terse
copy-on-select line to "copied" on the OSC 52 path, which is the overclaim this
branch removes, so the assertion encoded the defect instead of the behaviour
worth holding.

Rebased onto 22b71765. The base gained #361, #362 and #355; the only file this
branch and those commits both touch is ui-tui/src/app/useMainApp.ts, where #361
reworked the cover and prompt block while this branch replaced the copy-on-select
effect in the same hook. The replay was clean and the whole verification above was
re-run at the new head rather than carried forward. git merge-tree --write-tree
needs git 2.38 and this box has 2.34.1, so the check was the file-overlap table
plus the re-run, not a dry-run merge.

Manual check of the rendered artifact, since raven tui runs the prebuilt
ui-tui/dist/entry.js and nothing rebuilds it automatically: npm run build,
then confirmed the new strings are in dist/entry.js.

  • Relevant tests pass locally
  • Relevant lint / type checks pass locally
  • User-facing docs or screenshots are updated when needed

No doc change. CONTEXT.md has no clipboard or selection entry, and
copy-on-select was already the term in use in the vendored fork, so nothing new is
coined. The /help hotkey table is deliberately untouched: select-then-copy is
ordinary terminal behaviour and does not need advertising.

Risk

Behaviour changes for users who are not on macOS: a drag now writes the clipboard
and adds a transcript line. On macOS nothing changes except the wording of that
line. /copy keeps its shape and gains the path the write actually took.

One API change inside the vendored fork: Ink.copySelection() and
copySelectionNoClear() return { text, path } instead of the copied string, and
getClipboardPath() is gone from the package's exports. Internally copyNative()
may now answer with a promise on Linux's first copy, which setClipboard() awaits
after starting the tmux write; no caller outside that function sees it. Both are consumed only
within this repo, and the ambient declaration, the app interface and the two call
sites move with them.

Rollback is per concern. Reverting the feature commit restores the platform gate
and leaves the fixes, which stand on their own. Reverting all of them returns the
files to their base state; no data, config or on-disk format is involved, and no
migration exists to undo.

Checked and deliberately not fixed:

  • The OSC 52 write has no size cap and no chunking, so a large selection is
    handed to the terminal whole and silently dropped past whatever that terminal
    tolerates. This is why the OSC 52 path reports what it sent rather than
    claiming a copy. Capping or chunking it means changing the vendored write path
    and picking a threshold per terminal, neither of which belongs in this branch.
  • Ink.copySelectionNoClear() calls getSelectedText() outside its own try, so
    a throw there rejects the promise. The base already left that rejection
    unhandled at a bare void call, and adding a .then() does not change it.
    Fixing it means restructuring a vendored method this branch has no other reason
    to touch.
  • The effect's dependency array gains sys. It is provably stable (useCallback
    over appendMessage, itself useCallback with []), so the subscription is
    not re-created in practice. If it ever were, the version de-dupe would reset and
    one extra line could be written; the path caveat would not repeat, because that
    tally now lives in a ref that survives the effect.
  • Running /copy before the first drag of a session shows the path caveat twice,
    once from each lane. Sharing that state between an explicit command and an
    automatic one to save a duplicated sentence is not worth the coupling.
  • Two narrative comments in copyOnSelect.test.ts were reduced rather than
    removed: each kept the constraint it was wrapped around (that the platform case
    can only mean something on a non-macOS runner, and that an empty result is the
    whole signal that nothing reached the clipboard) and lost the "defect this
    closes" framing. Reading section 1.1 as rejecting the task-context wrapper
    rather than the constraint underneath is a judgement call, flagged as one.

The selection bus is faked in the new tests. The real one returns no-ops outside a
fullscreen Ink instance bound to a TTY, so there is no way to drive it from
vitest; the fake mirrors the ambient useSelection() contract, including
getState(): unknown, which is why the narrowing is exercised rather than
assumed. Flagging it because a hand-rolled stand-in for a real type is exactly the
blind spot that hides a defect.

Security: no new surface. Selected transcript text already reached the terminal
through this path on macOS and through /copy everywhere; the same bytes now
travel on the other platforms, on a user-initiated drag, which is what a native
terminal selection would have done anyway.

  • Security impact considered
  • Backward compatibility considered
  • Rollback path is clear for risky changes

Related Issues

N/A

@LivXue
LivXue requested a review from 0xKT August 23, 2026 09:00
@LivXue LivXue self-assigned this Aug 23, 2026
@LivXue
LivXue requested a review from gloryfromca August 24, 2026 06:48

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: the clipboard path can still be reported incorrectly, and two additional revision issues need correction; see the inline notes.

Covered: the live head and every GitHub discussion channel; git diff github/main...HEAD; AGENTS.md, the context map, and TUI terms; changed callers from mouse release through setClipboard() to transcript output; session lifecycle; the three-commit history; backward compatibility of the environment aliases; test changes and whether coverage was weakened; security impact; and merge/diff hygiene. I found no weakened base assertion, no dependency change, and no new security boundary. git diff --check and git merge-tree --write-tree HEAD github/main were clean.

Verification: the three focused changed-code files passed 81/81 tests, TypeScript type-check passed, and after building the required @hermes/ink artifact the full TUI suite passed 88 files / 1,030 tests. The first raw npx vitest full run was not a code result because the generated dist/entry-exports.js was absent; the rebuilt retry is the valid result. ESLint completed with 0 errors / 22 warnings. Prettier still reports the untouched src/app/chatStream.ts. Commit-message validation passed through python3 -m scripts.check_commit_messages github/main..HEAD, and all current GitHub checks are green.

The blocking path probe used a stale TMUX socket plus SSH and forced OSC 52: setClipboard() returned success: true with an ESC ] 52 ; sequence, while getClipboardPath() returned tmux-buffer.

}

if (process.env['TMUX']) {
if (env.TMUX) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getClipboardPath predicts tmux-buffer from $TMUX even when tmuxLoadBuffer failed and setClipboard fell back to raw OSC 52, so both callers report a buffer write that never happened. I reproduced this with a stale TMUX socket, SSH_CONNECTION set, and RAVEN_TUI_FORCE_OSC52=1: setClipboard('probe') returned success: true and a raw OSC 52 sequence, while this helper returned tmux-buffer. /copy and copy-on-select will therefore say copied ... to the tmux buffer although tmux wrote nothing. Please carry the actual path/outcome out of setClipboard() (or its awaited caller) instead of recomputing it from environment state, and cover the failed-load fallback.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed. setClipboard() already held the answer: nativeAttempted, tmuxBufferLoaded and whether a sequence was emitted are exactly the three inputs to its own success, and it discarded which of them it was -- leaving the callers to re-derive a path from an environment that cannot tell a stale TMUX from a working one.

Fixed in a7245ca: ClipboardResult carries path, read off those three observations in the precedence the doc already described, and null when none took the text. It travels with the copied text out of copySelectionNoClear() / copySelection() to the two call sites that show it.

Deviation: you asked for the outcome to be carried out of setClipboard() and did not ask for the predictor to go. I deleted getClipboardPath() and its suite. After the rewiring it had no callers, and leaving a helper exported from the package that can name a path the data did not take is leaving the same defect for whoever picks it up next. Say the word if you would rather it stayed as an explicitly-named prediction.

Its env-matrix cases were assertions about the predictor, so what replaces them drives the real setClipboard() with tmux stubbed, including the fallback that was wrong:

load-buffer exits 0                          -> path 'tmux-buffer'
load-buffer exits 1, sequence emitted        -> path 'osc52', success true   (your repro)
load-buffer exits 1, sequence suppressed     -> path null,   success false

Relocated rather than dropped: the deleted doc block carried the note on why the native gate reads SSH_CONNECTION and not SSH_TTY (a tmux pane inherits SSH_TTY across a local reattach). That now sits on shouldUseNativeClipboard(), which is the function that makes the choice.

One environment note, since it bears on the verification below: this worktree resolved @hermes/ink through a node_modules symlink into another checkout, so my first type-check and dist build read an unmodified copy of the package. I gave the worktree its own node_modules with @hermes/ink pointed at its own packages/hermes-ink, confirmed tsc --listFiles now reads the edited ink.tsx, rebuilt, and re-ran everything. The numbers below are from after that.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified. The stale-TMUX success, fallback-to-OSC52, and no-path cases now agree with the observations inside setClipboard(), and removing the unused predictor is the safer boundary. I also traced the new SelectionCopy result through both reporting callers. I found a distinct first-probe Linux outcome in the new path reporting and am raising that separately on the current diff; the tmux defect in this thread is fixed.

Comment thread ui-tui/src/app/useMainApp.ts Outdated
// only confirmation the clipboard was written. Lives below `sys` because the
// dependency array is evaluated during render, while `sys` is still in its
// temporal dead zone further up.
const copiedOnSelectRef = useRef(false)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copiedOnSelectRef is never reset when ui.sid changes, so after copying in one Raven session and switching sessions, the new session's first copy omits the clipboard-path caveat. newSession() and resumeById() reset transcript state and replace ui.sid, but App/useMainApp stays mounted and this ref remains true. The first drag in the new transcript therefore gets only the terse notice, contrary to the stated per-session behavior. Please reset or key this state on the current session id and add a switch-session case.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed. The parameter is named firstOfSession and the docstring on copyOnSelectNotice says "except the first one of a session", so the contract was explicit and only the scope of the state was wrong.

Fixed in d314729. Deviation: you offered "reset or key this state on the current session id" and I keyed it, but moved the state out of the hook to do so. createCopyOnSelectReporter() in lib/clipboard.ts owns the tally and takes a session key; the hook keeps one in a ref and passes getUiState().sid ?? 'draft'. Two reasons for that over a reset inside the effect: the sid never joins the dependency array, so a session change no longer tears down and rebuilds the bus subscription; and the boundary becomes unit-testable, which a ref inside useMainApp was not.

Reading the sid through getUiState() inside the callback follows the existing use at useMainApp.ts:542, and ?? 'draft' mirrors the heightCacheKey idiom on line 255. That default also matters for correctness: a null initial value compared against a draft session's null sid would have made the very first copy count as already-told, which is worse than the bug being fixed.

Tests: three in clipboard.test.ts. I reverted the reporter to a process-wide flag and watched "spends the caveat once per session" fail. The other two -- a session returned to, and two reporters not sharing a tally -- pass with the flag as well; they pin properties against a future refactor rather than proving this fix, and I would rather say that than imply all three are load-bearing.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified. Keying the reporter by the live session ID fixes the scope without resubscribing the selection bus, and the three reporter cases pin the intended lifetime clearly. The draft key also handles the pre-session state correctly. This finding is resolved.

@@ -0,0 +1,222 @@
import { describe, expect, it } from 'vitest'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copyOnSelect.test.ts has no required file-level purpose documentation and repeatedly stores transient defect/review narrative that AGENTS.md section 1.1 forbids. Every new code file must start with an English purpose docstring or equivalent, while comments such as The defect this closes and this box, and CI are specifically the transient task context that the same hard rule rejects. Please add a concise file-purpose header and reduce the inline commentary to hidden constraints that the test names and assertions cannot express.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in f031e1a: a file-level purpose block, and the narrative comments cut back to the constraint each was wrapped around.

Two of them I reduced rather than removed, which is a judgement call worth putting in front of you:

  • The platform case keeps one sentence -- that the case is the host runner's own, so reaching the copy at all is what says no platform gate remains, and the assertion therefore only means something on a non-macOS runner. That is a property of the test its name cannot carry. The "defect this closes" framing and "this box, and CI" are gone.
  • The silent-path case keeps "copySelectionNoClear() resolves to '' when nothing reached the clipboard" and drops the "exact lie this change set out to remove" clause.

Tell me if you would rather both went entirely; I read section 1.1 as rejecting the task-context framing rather than the constraint underneath it, but that is my reading of where the line falls.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified, and the two retained comments are appropriate. They now explain durable constraints that the assertions alone do not establish: the host-platform limitation and the empty-result contract. The transient defect/review narrative is gone, and the file-purpose block satisfies the new-file rule. This finding is resolved.

LivXue and others added 6 commits August 24, 2026 14:03
getClipboardPath() returned 'native' only on darwin, while setClipboard()
gates the native tool on shouldUseNativeClipboard() plus copyNative()'s own
per-platform check. A local Linux desktop with a display server therefore had
xclip or wl-copy write the clipboard successfully and was still told the copy
had only left as an escape sequence; Windows, where clip.exe always exists,
read the same way. The function now asks the same two questions setClipboard()
asks, in the same order, and takes env, platform and terminal as arguments so
every branch is covered by a case rather than only the one CI runs on.

It had no callers before, so nothing depended on the old answer.

The OSC 52 override and the clipboard debug switch were readable only under
the upstream HERMES_TUI_ names, while every knob this project documents uses
RAVEN_TUI_ and /copy's own failure hint names RAVEN_TUI_FORCE_OSC52 and
RAVEN_TUI_DEBUG_CLIPBOARD. Following that hint changed nothing. Both spellings
are now read, RAVEN_TUI_ first, with the HERMES_TUI_ names kept as aliases so
an environment that worked before still works. The debug switch moves behind
clipboardDebugEnabled() so its four call sites cannot drift apart.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
A TUI that enables mouse tracking owns the drag, so the terminal never builds
a native selection and its own copy shortcut has nothing to copy. Copy-on-
select is what makes a transcript selection copyable at all, but the
subscription bailed out on !isMac, so on Linux and Windows a drag highlighted
text and copied nothing.

The subscription moves out of useMainApp into subscribeCopyOnSelect(). Inlined
in a hook that needs a live gateway to start, none of its four guards could be
tested; each now has a case, and each was checked by removing only that guard
and watching the suite go red. The read of the bus state stops being an
unchecked cast: the ambient useSelection() declaration types it as unknown, so
the module narrows it instead.

Nothing on screen changes when a drag ends, so the copy is reported to the
transcript once a clipboard path has actually taken the text. The callback
fires on a non-empty result only, since copySelectionNoClear() resolves to ''
when nothing reached the clipboard, and announcing a copy there would be the
same false success this branch removes from /copy. The first report of a
session carries the resolved path, later ones stay terse: OSC 52 is the one
path a terminal can still refuse, and the first copy is when a user is looking
for the reason a paste came up empty.

The effect sits below sys() because a dependency array is built during render,
where sys is still in its temporal dead zone.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Measured on the real code: one 200x50 viewport of CJK is a 40 KB OSC 52
sequence, and a selection dragged through 2000 rows is 536 KB in a single
escape sequence. Nothing in the write path caps or chunks it, and terminals
drop an oversized sequence without a word, yet setClipboard() reports success
for all three because bytes were written to stdout. So the osc52 path now
reports what it sent rather than claiming a copy; native and tmux, which really
did write a clipboard, keep saying copied.

The count was UTF-16 code units, which reads three emoji as six characters and
a combining accent as two. graphemeCount() counts what is on screen, via
Intl.Segmenter where it exists and a code-point spread otherwise. CJK was
already correct, being one code unit per character.

An existing assertion pinned the old verb for the terse form and is flipped
here, since it encoded the overclaim rather than the behaviour worth keeping.

getClipboardPath() gains the one thing its callers rely on and it did not say:
the answer is only meaningful after a copy succeeded. Headless with no tmux and
OSC 52 suppressed takes no path at all, and the type has no word for that; both
callers ask only after a non-empty result, so they never see it.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
AGENTS.md section 1.1 requires a file-level purpose on every new code
file, and this one opened straight into its imports. Three comments also
recorded why the change was made rather than what the code must hold to,
which the same rule rejects as transient task context.

The narrative goes; the constraints behind it stay. The platform case
keeps the note that it can only mean something on a non-macOS runner,
since there is no platform to inject.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
The first copy of a session carries the clipboard-path caveat and later
ones stay terse, but the flag holding that lived in a hook that outlives
the session. `newSession()` and `resumeById()` replace `ui.sid` without
remounting `useMainApp`, so every session after the first opened with the
terse line and the user never learned an OSC 52 paste can come up empty.

Move the tally into a reporter that keys on a session identifier, so the
state and the thing it is scoped to live together and the boundary can be
asserted. The hook reads the sid through `getUiState()`, which keeps a
session change from tearing down the bus subscription.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
…rom env

getClipboardPath() re-derived the path from environment state after the
fact, and inside tmux the environment cannot tell a load-buffer that
worked from one that did not: both have TMUX set. With a stale socket the
data went out as raw OSC 52 while both callers said "copied to the tmux
buffer", sending the user to a tmux set-clipboard setting that had nothing
to do with it.

setClipboard() already knew. It computes nativeAttempted, tmuxBufferLoaded
and the emitted sequence to decide success, so it now reports which of
them took the text and returns null when none did. The path travels with
the copied text out of copySelectionNoClear()/copySelection() to the two
callers that show it, and the predictor is deleted rather than left in the
package for the next caller to trust.

The env-matrix cases the predictor's suite covered were assertions about
the predictor itself; what replaces them drives the real setClipboard()
with tmux stubbed, including the failed-load fallback that was wrong.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
@LivXue
LivXue force-pushed the feat/tui_copy_on_select_all_platforms branch from 89db89c to a7245ca Compare August 24, 2026 14:05

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: all three previous findings are fixed, but the new result still reports a native Linux path before the asynchronous tool probe establishes that any native path exists; see the inline note.

I replied under the three prior threads after verifying them: the stale-tmux fallback now carries the observed path, the first-copy caveat is scoped by session ID, and the test file now satisfies the repository comment rules.

Covered on a7245ca86d51: live head and every discussion channel; range-diff of the rebase (the original three commits are patch-equivalent, followed by the three fix commits); the complete github/main...HEAD diff; AGENTS.md and the TUI context; all SelectionCopy callers through transcript output; session lifecycle; commit history; backward compatibility (the changed ink package is private and every in-repo caller was checked); test deletion/replacement; and security impact. git diff --check and git merge-tree --write-tree HEAD github/main are clean.

Verification: rebuilt @hermes/ink; focused changed-code tests passed 81/81; TypeScript type-check passed; full TUI suite passed 91 files / 1,078 tests; commit validation passed; ESLint completed with 0 errors / 22 warnings; all current GitHub checks are green. Prettier remains red on seven files: five are already unformatted on github/main, while this branch also leaves useMainApp.ts and clipboard.ts unformatted.

The new blocker was reproduced by forcing process.platform=linux, setting DISPLAY, disabling OSC 52, and providing no wl-copy/xclip/xsel: the first call returned {sequence:'', success:true, path:'native'}; after the background probe established that no tool exists, the same call returned {sequence:'', success:false, path:null}.

// than off the environment: native outranks tmux because it is the write
// that is not contingent on a terminal or multiplexer setting, and a failed
// load-buffer has to fall through to osc52 here exactly as the data did.
const path: ClipboardPath | null = nativeAttempted

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setClipboard reports path native before the first Linux clipboard probe has found a tool, so a machine with DISPLAY but no wl-copy/xclip/xsel gets a successful native-copy notice even though no path took the text. copyNative() returns true immediately while its background probe is still unresolved, and this new branch promotes that attempt to an actual native path. I reproduced the no-fallback case with Linux + DISPLAY + RAVEN_TUI_FORCE_OSC52=0: the first call returned {sequence:'', success:true, path:'native'}; 300 ms later, after all three tools had failed probing, the same call returned {sequence:'', success:false, path:null}. The first /copy or copy-on-select therefore says copied when nothing was written. Please make completion of the first Linux probe part of the result (it can still be started before the tmux await), or otherwise avoid returning native success until a tool is known, and cover the all-tools-missing first call.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed. I checked whether it was mine or inherited before answering, because the base mis-reports the same machine too and I did not want to hand you "pre-existing" for something this branch changed. It is mine.

Reproduced with the probe mocked to fail all three tools, Linux + DISPLAY + OSC 52 suppressed, comparing the two trees on the same scenario:

this branch, before the fix:  {sequence:'', success:true, path:'native'}
github/main, same scenario:   {sequence:'', success:true, path:'osc52'}

So success: true with nothing written predates the branch, but the native claim does not. The old predictor never returned native on Linux at all -- that was one of the defects this branch set out to fix -- so promoting nativeAttempted to an observed path is what created this particular false report. It belongs here.

Fixed in 4550a44: copyNative() answers the first Linux call with the probe's own result rather than treating a display server as proof of a tool. Still invoked before the tmux await, per your parenthetical, so the probe runs alongside load-buffer instead of ahead of the report; every later call answers synchronously from the cache as before. Worth naming the thing that does not change: the copy itself never ran ahead of this probe either, since the tool is spawned inside it -- only the report did.

Test: Linux + DISPLAY + all three probes failing now asserts {success: false, path: null} on the first call. Reverted to the optimistic return true and watched it fail with expected { sequence: '', success: true, ... } to match { success: false, path: null }, then restored. The case forces process.platform, so the suite now restores it in afterEach -- without that it would leak to every later case in the file, and only a non-Linux runner would notice.

Separately, from your summary: useMainApp.ts and clipboard.ts were left unformatted by me, not by the base. Dropping a name from the @hermes/ink import and adding a wide signature left both inside the print width while still wrapped. Formatted in f025c32, kept out of the fix commit, and every file this branch touches now passes prettier --check. The five you found already red on github/main are untouched here.

Re-verified at f025c32: type-check clean, full TUI suite 91 files / 1066 passed / 13 skipped, ESLint 0 errors / 22 warnings, commit validation exit 0.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified. On the first Linux call with DISPLAY, OSC 52 disabled, and no wl-copy/xclip/xsel, setClipboard('probe') now returns {sequence:'', success:false, path:null}. The source starts the probe before tmuxLoadBuffer(), then awaits its result before choosing success/path; when a winner exists, true is returned only after that tool is selected and its copy command is launched. The new case also restores process.platform, and Prettier now passes every TypeScript file this branch touches. This finding is resolved.

@LivXue

LivXue commented Aug 24, 2026

Copy link
Copy Markdown
Member Author

All three findings are fixed, one commit each, with the reasoning in their
threads.

f031e1af  give the copy-on-select suite a purpose header
d314729a  scope the copy-on-select caveat to the session it was shown in
a7245ca8  report the clipboard path the write took, not one guessed from env

Two of the fixes deviate from the shape you suggested and say so in their
threads: the path fix deletes getClipboardPath() rather than leaving a
predictor in the package, and the session fix keys the tally in a reporter
outside the hook rather than resetting a ref inside it. The comment fix reduced
two narrative comments instead of removing them, which is a judgement call about
where section 1.1 draws its line, and it is the one I would most like a second
opinion on.

One thing worth carrying beyond this MR, because it would silently affect anyone
reviewing from a worktree: ui-tui/node_modules here was a symlink to another
checkout's, so node_modules/@hermes/ink -> ../../packages/hermes-ink resolved
against the symlink target and landed on that other checkout's copy. My first
type-check and dist build therefore read an unmodified hermes-ink while the
app code under test was the edited one -- green, and meaningless. Fixed by
giving the worktree a real node_modules whose @hermes/ink points at its own
package, confirmed via tsc --listFiles, then rebuilt and re-ran everything.

Rebased onto 22b7176, so the SHAs you cited are stale. The description is
rewritten at the new head: 91 files / 1065 passed, and the paragraph that
justified the predictor across fourteen env combinations is gone, since the
predictor is.

LivXue and others added 2 commits August 24, 2026 14:22
copyNative() answers the first Linux call before its probe has found a
tool, because a display server means one could exist, not that one does.
Reporting the observed path promoted that optimism into a claimed native
write: a machine with DISPLAY and no wl-copy, xclip or xsel was told its
first copy landed while nothing had been written, and the same call
reported failure once the probe settled a moment later.

The first call now answers with the probe's own result. It is still
invoked before the tmux await, so the probe runs alongside load-buffer
rather than ahead of the report, and every later call still answers
synchronously from the cache. The copy itself never ran ahead of this
probe -- the tool is spawned inside it -- only the report did.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Dropping a name from the @hermes/ink import and adding a wide function
signature left both inside Prettier's print width while still wrapped, so
this branch was the reason those two files reported unformatted. Every
file it touches now passes prettier --check.

Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
@LivXue

LivXue commented Aug 24, 2026

Copy link
Copy Markdown
Member Author

Round two: the new blocker is fixed, and so is the Prettier point from your summary.

4550a442  wait for the linux clipboard probe before naming a native path
f025c32b  format the two lines this branch left wrapped

The reasoning is in the thread. Short version: it was this branch's doing rather
than inherited, and I ran the same scenario against github/main to establish
which -- the base reported path: 'osc52' for that machine, so success: true
with nothing written predates the branch while the native claim does not.

The two Prettier files were mine as well, not the base's, so they are fixed here
rather than explained away. Every file this branch touches now passes
prettier --check; the five you found already red on github/main are untouched.

Description updated at the new head.

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

The first-probe Linux false native path is fixed: when DISPLAY exists but wl-copy/xclip/xsel do not and OSC 52 is disabled, the first call now returns {success:false, path:null}. I replied on that thread after reproducing the case and tracing the awaited result through setClipboard(). The separate formatting commit is mechanical and all touched TypeScript files pass Prettier.

For this moved revision I checked the live head and discussions, the a7245ca...f025c32 delta and final github/main...HEAD diff, repository rules and TUI terminology, affected callers and async lifecycle, relevant history and backward compatibility, and that tests were extended rather than weakened. git diff --check and the merge-tree check are clean. Verification passed: focused clipboard/copy-on-select tests (82), full TUI suite (91 files, 1079 tests), TypeScript type-check, ESLint (0 errors, 22 warnings), touched-file Prettier, commit-message validation, and the direct missing-tool reproduction. All completed GitHub checks pass; the Python unit job remains pending with no failure reported.

@LivXue
LivXue merged commit 3de832b into EverMind-AI:main Aug 24, 2026
18 checks passed
@LivXue
LivXue deleted the feat/tui_copy_on_select_all_platforms branch August 24, 2026 14:58
LivXue added a commit that referenced this pull request Aug 25, 2026
…366)

## Summary

PR #360 reports every copy-on-select write as a permanent transcript
line, so a
reading session that drags through the transcript piles up "copied N
characters" rows under the conversation. The confirmation splits by what
the
line is for: the unbounded terse repeats become a transient notice
rendered
above the input box -- one line that replaces itself on every new copy
and
clears after 3 seconds -- while the first copy of a session keeps its
path
caveat in the transcript. The caveat is the answer a user comes looking
for
after a paste comes up empty minutes later, and it cannot pile up
because it
is once per session by construction.

The reporter still owns the per-session caveat tally and now returns
`{ text, firstOfSession }` so the caller can route the caveat to the
transcript and the repeats to the notice. A new `copyNoticeStore` holds
the
single notice; `showCopyNotice` keeps one dismiss timer, clearing any
pending
one before arming the next, so an earlier deadline can never clear a
newer
notice. `/copy` is deliberately untouched: an explicit command's answer
still
belongs in the transcript, and it cannot stack on its own.

## Type

- [x] Fix

## Verification

Run from `ui-tui/` at branch head, in a fresh worktree of `github/main`.
Dependencies came from `npm ci` in `ui-tui/` and in the hermes package;
the
hermes package's own `node_modules` holds only esbuild, so the ink
render
tests run against one react copy (two copies break every hook call).

```
npx vitest run --no-file-parallelism
  Test Files  93 passed (93)
  Tests  1084 passed | 13 skipped (1097)

npm run type-check
  clean, exit 0

npm run lint
  22 problems (0 errors, 22 warnings)

npx prettier --check src/app/copyNoticeStore.ts src/__tests__/copyNotice.test.tsx
  src/__tests__/clipboard.test.ts src/app/useMainApp.ts
  src/components/appLayout.tsx src/lib/clipboard.ts
  All matched files use Prettier code style!

python3 -m scripts.check_commit_messages github/main..HEAD
  exit 0
```

The 22 lint warnings are the baseline, not new: 22 before the branch and
22
after, none in a file this branch touches. The suite is run with
`--no-file-parallelism` deliberately: the ink render tests fail under
default
worker parallelism at this suite size, and no CI job runs them.

Suites that exercise the changed code directly:
`src/__tests__/copyNotice.test.tsx`
(7 cases), `src/__tests__/clipboard.test.ts` (30 cases, the 3 reporter
cases
updated to the new return shape).

New tests were proved load-bearing rather than assumed:

- Removing the notice render line in `appLayout.tsx` makes the render
test
  fail (content and placement above the input box).
- Removing the pending-timer clear in `showCopyNotice` makes the
re-shown-notice test fail: the earlier 3s deadline then clears the newer
  notice.
- The dismissed-notice case now shows a notice first, and removing
  `dismissCopyNotice`'s atom clear turns it red (review fix).

- [x] Relevant tests pass locally
- [x] Relevant lint / type checks pass locally
- [ ] User-facing docs or screenshots are updated when needed

No doc change. `ui-tui/CONTEXT.md` has no clipboard or selection entry,
and
"copy notice" continues the existing "notice" vocabulary of
`copyResultNotice`
/ `copyOnSelectNotice`, so nothing new is coined.

## Risk

Behaviour change: the per-drag copy confirmation is now transient and
above
the input box instead of a permanent transcript line, and the first copy
of a
session lands one transcript line, exactly as on `main`. Reverting the
branch
restores the #360 behaviour; no data or on-disk format is involved.

- [x] Security impact considered
- [x] Backward compatibility considered
- [x] Rollback path is clear for risky changes

## Related Issues

N/A

---------

Co-authored-by: Claude (deepseek-v4-pro) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants