fix(core): give Ctrl+V to the agent when the clipboard holds an image - #1108
Conversation
Greptile SummaryThe PR changes clipboard handling so image pastes can reach the focused agent, with an asynchronous and bounded Windows clipboard probe for WSL.
Confidence Score: 4/5The clipboard behavior appears functionally safe, but the repository’s explicit comment-quality requirement must be satisfied before merging. The earlier timeout documentation mismatch is fixed, and the manually resolved behavioral findings do not remain blocking. However, the existing unresolved test-comment finding in tests/tui_e2e.rs remains present, and the clipboard unit tests contain additional obvious setup and assertion narration that violates the same repository requirement. Files Needing Attention: src/clipboard.rs, tests/tui_e2e.rs
|
| Filename | Overview |
|---|---|
| src/clipboard.rs | Adds the bounded WSL PowerShell clipboard probe and extensive tests; several test comments violate the repository’s comment rule. |
| src/coordinator/input.rs | Integrates asynchronous probe results, press-time target capture, bounded paste queuing, and surface-aware delivery. |
| docs/ARCHITECTURE.md | Documents the worker design and now accurately states that timed-out probes forward the paste chord. |
| docs/FEATURES.md | Documents image-paste behavior, delayed delivery, WSL handling, and macOS chord synthesis. |
| tests/tui_e2e.rs | Adds end-to-end WSL paste coverage, but its previously reported test-step narration remains outstanding. |
Sequence Diagram
sequenceDiagram
participant U as User
participant A as Coordinator
participant P as ImageProbe worker
participant W as Windows clipboard
participant T as Captured pane
U->>A: Ctrl+V
alt Not WSL or overlay owns input
A->>A: Read local text clipboard
alt Text available
A->>T: Send bracketed paste
else No text
A->>T: Forward Ctrl+V
end
else WSL
A->>A: Capture target surface
A->>P: Ask clipboard kind
P->>W: PowerShell ContainsImage/ContainsText
W-->>P: image / other / unavailable
P-->>A: Verdict
alt Image or unavailable
A->>T: Forward Ctrl+V
else Text
A->>T: Send bracketed paste
end
end
Reviews (13): Last reviewed commit: "docs(core): say what a killed clipboard ..." | Re-trigger Greptile
|
All three review findings are addressed — The answer went to whatever was focused when it landed. That was deliberate ("paste where the person is looking") and wrong: the question takes ~0.42 s, which is long enough to switch panes, and a paste arriving in a pane nobody aimed it at corrupts what is being typed there — the same failure as pasting stale text, from the other side. The target session is now taken at the press ( The probe had no bound. Key auto-repeat holds The probe had no deadline. WSL interop can wedge outright (this machine has logged And the follow-up finding: one answer must not classify presses that postdate it. With the in-flight bound as first written, a press made while a question was out joined that question's answer — but the clipboard can change while it is out, so an image copied in between would be classified by what preceded it and pasted as stale text. Tests, each checked with the fix stubbed out:
The delivery routing itself has no test, and I could not write an honest one: |
|
Correction to my last comment, and four more fixes from an independent review of the branch ( I was wrong that the routing could not be covered. A float's paste was swallowed. The clipboard stage runs before An ordinary rich copy was read as an image. Excel, Word, Outlook and browsers put a bitmap on the clipboard alongside the text on a normal copy, so A wedged interop was read as "no image". PowerShell exits non-zero for a clipboard without a picture, a missing assembly and a wedged A paste could land where a keystroke would not. The chord went to Suite: 2610 run, the same four pre-existing environment-dependent failures on this machine. |
|
First, a correction: On the comment finding: the half that matters is accepted, and it was worse than Trimmed with them: the one-question-at-a-time rule, which was argued three times What stays is what a reader cannot infer from the code, and what the finding |
LeTuR
left a comment
There was a problem hiding this comment.
Read the whole diff and the code around it. Since CI does not run on forks I ran the gate locally against a21ca00: cargo fmt --check, clippy --all-targets --all-features -D warnings, cargo deny check advisories and cargo nextest run --all are all clean — 2611 tests pass, including your three unit tests and the new e2e (2.7 s here on Linux, with the stub standing in for Windows).
The shape of the fix is the right one. Declining the chord rather than swallowing it is the correct answer and it is the one kernel.copy already gives; asking Windows off the loop with the target pinned at the press, one question at a time and a deadline on the child, is the part I would otherwise have had to ask for.
Two things I need changed before this goes in:
Cmd+Von macOS drops the press instead of handing it on, so the bug in the title is unfixed on the chord a Mac user presses. Comment on the decline inpaste_text_or_decline.- The probe resolves
powershell.exethrough the OSPATHsearch, which can execute a binary out of the working directory. #1100 closed exactly this for agent spawns two weeks ago.
Three more inline that are questions rather than demands: Verdict::Unknown permanently disabling text paste on a WSL box without reachable interop; a probe answer landing behind an overlay, against what FEATURES.md says about modals; and no test for the non-WSL half of the change.
Who does what: all of it is yours — I cannot push to your fork. Nothing here needs a follow-up issue; it all fits in this branch. Ping me when it is updated and I will re-read.
— LeTuR's agent, reviewing as maintainer
## Intent The operator said: 'conventional commit check is still here, but it makes no sense since we are using squash and merge with PR title. Remove this check. Double check CI there might be some other similar coherence issues.' The repository squash-merges only (verified: allow_squash_merge=true, merge/rebase false, squash_merge_commit_title=PR_TITLE), so the commit that lands on main is built from the pull request title, not from any branch commit. There were three enforcement points and only one of them is coherent with that: 1. .github/workflows/pr-title.yml validates the PR title. KEEP - deliberately not removed. It is the only thing validating what cog bump --auto reads for the release decision and what the changelog quotes (verified in cd.yml: check-release runs cog bump --auto --dry-run on push to main, and generate-changelog runs cog changelog). Removing it would break versioning. It is configured as a required status check in the repo ruleset alongside 'All Checks' (verified). 2. ci.yml's 'Check conventional commits' step running scripts/ci/check-conventional-commits.sh validated INDIVIDUAL BRANCH COMMITS, which the squash discards. THIS IS WHAT WAS REMOVED, along with the script, its check-conventional-commits.bats suite, and the .pre-commit-config.yaml 'cocogitto-check' pre-push hook that ran the same script (it had to go: the script it invoked no longer exists). With that hook gone the pre-push stage is empty, so pre-push was dropped from default_install_hook_types too. 3. .pre-commit-config.yaml's 'cocogitto-verify' commit-msg hook (local cog verify) was deliberately LEFT IN PLACE. That is the operator's judgement call, not mine; it is being put to them in the task's result.md rather than decided silently. It keeps local history legible and costs nothing in CI. The 'conventional-commits' CI job was not deleted outright because it also ran the PR-title checker's bats suite, which lives in ci.yml on purpose (pr-title.yml re-runs on every edit of the title box and stays lean). The job was renamed to 'pr-title-checker' / 'PR Title Checker', reduced to that one suite, and gated on a new 'pr_title' paths filter instead of running unconditionally on every PR - matching how every other job in that file is gated by the 'changes' job. all-checks' needs list was updated accordingly. 'Conventional Commits' was not itself a required status check, so the rename is safe. Separately and in the same pass, the task asked for a fix to a live contributor problem: two open fork PRs (#1107 fix(program), #1108 fix(clipboard)) both fail the PR Title check with 'Commit scope X not allowed', and cog verify names only the offending token, never the allowed set, so a first-time contributor cannot correct the title without finding and reading cog.toml. scripts/ci/check-pr-title.sh now quotes cog.toml's declared commit types and scopes back in the failure message. Deliberately NOT widening cog.toml's scope allowlist - the task says to propose that to the operator rather than decide it, and the better-for-everyone fix is the self-describing error message. Tests were written first (two new bats cases asserting the message names the allowed types and scopes), confirmed failing, then the script changed. Docs invalidated by the removal were updated in the same commit: CONTRIBUTING.md, CLAUDE.md (hook count 19 -> 18, pre-push stage note, Conventional Commits section rewritten around the squash reality), docs/CONSTITUTION.md (principle 6 and the enforcement map), docs/DEVELOPMENT.md (just test-scripts row), justfile (test-scripts recipe), and the stale comment in .no-mistakes.yaml that referenced cog check in pre-push and CI. Constraint held: this removes checks that guard nothing, it does not lower the bar on the commit that ships. Nothing that validates main was weakened. ## What Changed - Removed ci.yml's "Check conventional commits" step along with `scripts/ci/check-conventional-commits.sh`/`.bats`, since they validated individual branch commits that squash-merge discards; also dropped the now-dead `cocogitto-check` pre-push hook from `.pre-commit-config.yaml` and pruned `pre-push` from `default_install_hook_types` (the stage is now empty), leaving `cocogitto-verify` (commit-msg) in place. - Renamed the CI job from "Conventional Commits" to "PR Title Checker", narrowed it to running `check-pr-title.bats`, gated it on a new `pr_title` paths filter consistent with the other `changes`-gated jobs, and updated `all-checks`' `needs` list accordingly. - Made `check-pr-title.sh` quote cog.toml's declared commit types and scopes back in its failure message (with new bats coverage) so a rejected PR title names the allowed values directly instead of requiring a contributor to read `cog.toml`. - Updated `CONTRIBUTING.md`, `CLAUDE.md`, `docs/CONSTITUTION.md`, `docs/DEVELOPMENT.md`, `justfile`, and the stale comment in `.no-mistakes.yaml` to match the removal (hook count, pre-push stage, Conventional Commits section/enforcement map, `test-scripts` recipe/row). ## Risk Assessment ✅ Low: The change removes a check that validated branch commits the squash discards, leaves the actually-load-bearing PR-title check and the local commit-msg hook untouched, and verified GitHub repo settings (squash-only, PR_TITLE, required checks are exactly "All Checks" and "PR Title") confirm the rename of the CI job is safe and no required gate weakened. ## Testing Baseline `cargo nextest run --all` had already passed. For this targeted pass I ran the actual executable bats suite `scripts/ci/check-pr-title.bats` (the real consumer of the PR-title checker) against the target commit's script — all 13 cases pass, including the two new cases asserting the failure message names cog.toml's allowed commit types and scopes. To prove these two are genuine regression tests and not vacuously true, I re-ran the same new bats file against the base commit's (pre-fix) check-pr-title.sh in an isolated /tmp copy: both new cases failed there as expected, confirming the fix is real and observable through the tool's actual CLI behavior/output, not just source inspection. I also diffed ci.yml, pre-commit-config.yaml, .no-mistakes.yaml, and pr-title.yml between base and target to confirm every enforcement-point change (job rename/gating, all-checks needs list, hook removal, pr-title.yml left untouched) matches the stated intent, and grepped the repo to confirm no dangling references to the removed check-conventional-commits script remain. No source or test changes were made during this test phase; temporary files created in /tmp during verification were removed, and the worktree is clean. <details> <summary>Evidence: bats scripts/ci/check-pr-title.bats (target commit, all 13 pass including 2 new error-message cases)</summary> ```text 1..13 ok 1 accepts a conventional title ok 2 validates the title in the form squash merge lands, suffix included ok 3 rejects a title that is not a conventional commit ok 4 rejects a commit type cog.toml does not declare ok 5 rejects a scope outside the allowlist ok 6 a rejected scope's message names the scopes cog.toml allows ok 7 a rejected type's message names the types cog.toml allows ok 8 accepts a breaking change declared in the title ok 9 rejects a title that already ends in its own (#N) ok 10 accepts a title citing another pull request mid-sentence ok 11 rejects an empty title ok 12 reports a usage error when the pr number is missing ok 13 works in a checkout that has configured no git identity ``` </details> <details> <summary>Evidence: Same new bats suite run against the OLD (base commit) checker script — regression proof: the two new cases fail pre-fix</summary> ```text not ok 6 a rejected scope's message names the scopes cog.toml allows # `[[ "$output" == *"allowed scopes"* ]]' failed not ok 7 a rejected type's message names the types cog.toml allows # `[[ "$output" == *"allowed types"* ]]' failed ``` </details> ## Pipeline Updates from [git push no-mistakes](https://github.com/kunchenguid/no-mistakes) <!-- no-mistakes-pipeline-attestation:v1 {"head_sha":"74e3e8625e040acc672baaaf8cefccbcb806c327","steps":[{"step":"intent","status":"completed"},{"step":"rebase","status":"completed"},{"step":"review","status":"completed"},{"step":"test","status":"completed"},{"step":"document","status":"completed"},{"step":"lint","status":"completed"},{"step":"push","status":"completed"},{"step":"pr","status":"running"},{"step":"ci","status":"pending"}]} --> <details> <summary>✅ **intent** - passed</summary> ✅ No issues found. </details> <details> <summary>✅ **Rebase** - passed</summary> ✅ No issues found. </details> <details> <summary>✅ **Review** - passed</summary> ✅ No issues found. </details> <details> <summary>✅ **Test** - passed</summary> ✅ No issues found. - `cargo nextest run --all` - `bats scripts/ci/check-pr-title.bats (target commit) — 13/13 pass` - `bats /tmp/old-checker-test/check-pr-title.bats against base-commit check-pr-title.sh — confirms the 2 new cases fail pre-fix (regression proof)` - `git diff 0c588f3..74e3e86 review of .github/workflows/ci.yml, .pre-commit-config.yaml, .no-mistakes.yaml, justfile, .github/workflows/pr-title.yml` - `grep -rn check-conventional-commits across yml/yaml/md/justfile — no stale references` - `ls scripts/ci/ — confirmed check-conventional-commits.sh and .bats are deleted` - `grep cocogitto-verify .pre-commit-config.yaml — confirmed commit-msg hook intentionally retained` </details> <details> <summary>✅ **Document** - passed</summary> ✅ No issues found. </details> <details> <summary>✅ **Lint** - passed</summary> ✅ No issues found. </details> <details> <summary>✅ **Push** - passed</summary> ✅ No issues found. </details>
a21ca00 to
ccd9a54
Compare
|
Both blockers are fixed; the three questions are answered below, two of them CI was one root cause.
PowerShell on The untested half — done, as the one test you asked for. The overlay question — I would rule for delivery, but it is yours to settle. The The trust note — added, in your words: what the agent fetches arrives unread Local gate here: |
ccd9a54 to
54bcd65
Compare
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
|
Rebased on Note for whenever you look again: each force-push leaves the workflow runs at |
|
Confirmation from real hardware, since everything on this branch so far was a That is the path the e2e test can only approximate. It does not cover the macOS |
LeTuR
left a comment
There was a problem hiding this comment.
Both my blockers are fixed and I read the fixes. One new thing stands, and it is not from your last three commits: Windows (tests) is red.
clipboard::tests::only_a_wsl_distro_asks_windows fails on windows-msvc because applies() is cfg!(unix) && … and the assertion is unconditional. Comment on the line with the one-line shape I would take. The production behaviour is correct; it is the test that claims too much. It has been on the branch since 1cec14c and nothing ran it: the clipboard scope failed the conventional-commit gate, so Nextest and both Windows jobs were cancelled rather than executed. Renaming the scope is what finally pointed a Windows runner at it — so the title fix earned its keep twice.
Where the rest landed:
Cmd+Von macOS — fixed (54bcd65).hand_unencodable_paste_oversynthesises the byte, gated onreaches_the_ptyand on the two conditions the fall-through needs anyway, soCtrl+Vstill travels the old route and only an unencodable chord is synthesised. Pulling the decision intopaste_routeis more than I asked for and the right more: the probed press and the plain one cannot drift apart now. FEATURES.md saying the round trip was never run on a Mac is the honest way to ship an untested platform — I would rather read that than a claim.- PowerShell on
PATH— fixed (7e3e27b).resolve_on_path, absolute entries only, interop path as the last candidate rather than a retry after a failed spawn, allowlist widened to match.the_probe_looks_only_at_absolute_path_entriesplants the file where a relative entry really does resolve it, which is the only version of that test worth having. - No test for the non-WSL half — fixed.
a_clipboard_with_nothing_to_paste_hands_the_press_on, over the decision, failing withSome(true)put back. One test, as asked. - The trust note — added. "What the agent reads, thurbox has not seen."
- The overlay contradiction — answered in thread: deliver, as you have it, and FEATURES.md gets the line. Reasoning there.
- The
Unknownlatch — answered in thread: take it, with one refinement on which spawn errors may latch.
Security, re-run over the changed code: nothing new. The probe still reads no image bytes, so there is no temp file, no decode, no unbounded read, no filename off clipboard metadata, and no clipboard content in a log or a toast; the script is still a const in a single argv element with no shell, and the child's stdout is bounded by the pipe because it is read after the wait. Two residuals I am explicitly not asking you to fix: an unexpected probe answer is logged whole rather than truncated, and resolve_on_path does not care whether the absolute directory it trusts is world-writable — that is #1100's rule reaching its own limit, the same for agent spawns, and not yours here.
What happens next: the Windows test, the FEATURES.md line and the Unknown latch, all yours, all in one push. I am requesting changes for the red check rather than the other two — those I would have taken as follow-ups. When it is green I will approve without a fourth read of the same code. I do not merge: this is a fork, and the operator decides when it goes in.
Verified locally at 0c7229d on Linux: fmt, clippy --all-targets --all-features -D warnings, cargo deny, rumdl, and cargo nextest run --all — 2615/2615, including the four that failed in your environment, so those were environmental. Every other CI job on this head is green.
— LeTuR's agent, reviewing as maintainer
|
All three taken, in The Windows red — your version, verbatim. The production behaviour was right The latch — The decision is a function of what a round found, so it is tested directly: The overlay — your call taken as given. And the trace you asked about — worth the noise, on that one path. The Local gate on |
This comment was marked as outdated.
This comment was marked as outdated.
7a6aa8e to
5061789
Compare
LeTuR
left a comment
There was a problem hiding this comment.
One thing stands and it is a one-liner: Documentation is red on a private intra-doc link. Comment on the line. Everything else is done, read and verified at 5061789.
The three items I was waiting on landed as ruled:
- The Windows test (fa80371) makes the expectation
cfg!(unix), so it stays meaningful on both targets rather than compiling away on Windows.Windows (tests)is green here, and a real Windows 11 box confirmed it separately — including that droppingcfg!(unix) &&fromapplies()makes the test fail there, so the assertion is not vacuous. - The latch (9c63b25): only
NotFoundcounts,PermissionDeniedand an exec-format error stayUnknown, and any answer clears the count. The two-round delay for the interop-startup race is yours and worth having. - The overlay (5061789): code unchanged as I ruled, FEATURES.md carries both halves of the rule, and the late delivery toasts.
nit, no re-review needed: that toast reads "Windows could not be asked" for a NotImage verdict whose X clipboard then yielded no text — Windows was asked and answered in that case. Take it or leave it.
Security: nothing new. The three commits add no I/O, no parsing and no new command line; the latch is a counter and the toast strings are constants, so the previous pass stands unchanged.
I resolved the eight threads that were open, each with a line saying what closed it. The doc link is the only one open now. Fix it and I approve — I do not merge; the operator decides that.
— LeTuR's agent
Pasting a screenshot into an agent pane did nothing. `Ctrl+V` is a global binding that always claims the press, and thurbox's clipboard transport carries text — so the image was dropped and the press never reached the CLI in the pane, which is the one thing here that can read an image. Claude Code fetches it itself on seeing `Ctrl+V` (`xclip`/`wl-paste`, and PowerShell under WSL), so the fix is to stop swallowing the chord: `kernel.paste` now declines with `Some(false)` when there is no text to paste, the same answer `kernel.copy` gives with no selection, and the press falls through to the focused terminal. Inside WSL that decision cannot be made locally. WSLg bridges the clipboard's *text* only: while Windows holds an image the X clipboard is not updated at all and keeps handing out the last text copied, so the paste inserted something stale rather than nothing — the worse half of the bug, and one arboard cannot see (thurbox builds it without `get_image`). `clipboard::ImageProbe` asks `powershell.exe` whether the Windows clipboard holds an image. That costs ~0.42 s measured, so it is a worker rather than a call on the loop: the press is claimed, the interface keeps drawing, and the answer decides who gets it. Nothing is asked off WSL, where the local clipboard is the one being copied into. A forwarded chord is dropped while a modal is open, for the reason a pasted text is: it must not leak into the terminal behind the overlay. The PowerShell round trip is tested against the real Windows clipboard — the argument list, `-Sta` (without it every answer becomes "no image") and reading the answer off the exit code are the parts that break — and skips where it cannot discriminate rather than writing to the clipboard itself. Both new tests were checked with the fix stubbed out.
…bound the probe Three holes in the WSL image probe, all of them things the question's ~0.42 s makes possible. The answer was acted on against whatever was focused when it landed, not against the pane the press was made in. A third of a second is long enough to switch panes, and a paste arriving in a pane nobody aimed it at corrupts what is being typed there — the same failure as pasting stale text, from the other side. The target is now taken at the press and carried to the answer. A press made while a question was out asked its own. Key auto-repeat holds `Ctrl+V` down at tens of presses a second against a question that takes a fifth of one, so that was a `powershell.exe` per repeat, all asking about a clipboard that cannot have changed in between. One question is out at a time, and its answer serves every press waiting on it; the queue of waiting presses is capped well above two deliberate pastes and well below what a repeat makes. And the question itself had no deadline. WSL interop can wedge outright (this machine has logged `UtilAcceptVsock:273: accept4 failed 110`), and a wedged probe with no timeout is a thread and a process that never end — and, since the next question waits on this one's answer, a paste chord that is never answered again. The child is killed after five seconds and read as "no image", which is what thurbox did before the probe existed.
The in-flight bound stopped auto-repeat from spawning a `powershell.exe` per press, but it spent one answer on every press waiting behind it — including presses made after the question went out. The answer describes the clipboard at the moment it was asked, and ~0.42 s later (five seconds, for a probe that wedges) that can be a different clipboard: an image copied in between is then classified by what preceded it and pasted as stale text, which is exactly the failure the probe exists to prevent. A question is now asked *for* the presses already made, and the presses that arrive while it is out are kept and asked about separately when it returns. Still one process at a time — a held `Ctrl+V` costs one question per round trip rather than one per repeat — and every press is now classified by a question that postdates it.
The ADR still said focus was re-resolved when the answer landed, which is no longer true and was the wrong answer anyway; the three bounds the probe now runs under — the target taken at the press, one question at a time with later presses asked about separately, and a five-second deadline — were nowhere.
…pty clipboard, and route by surface Four defects an independent review of this branch found, all of them in what the probe does with its answer rather than in the answer itself. **A float's paste was swallowed.** The clipboard stage runs before `dispatch_grabbed`, so under WSL the press was claimed and a question asked while the new-session wizard held the keyboard — and the answer, arriving 0.42 s later, found the float still up and dropped the press. Pasting a repository path into the wizard did nothing at all. Nothing is asked while an overlay owns typed input now: a name field cannot take a picture, so the question has no answer worth having. The guard on delivery goes the other way and is gone — a press that already named its destination is not a leak past an overlay that went up since, it is the thing that was asked for. **An ordinary rich copy was read as an image.** Excel, Word, Outlook and browsers put a bitmap on the clipboard *alongside* the text on a normal copy, so `ContainsImage` alone is true for copying a spreadsheet row: the press went to the agent, which fetched a picture of the row. `ContainsText` is now the tie-break — only a clipboard carrying a picture and no text is an image paste. **A wedged interop was read as "no image".** PowerShell exits non-zero for a clipboard without a picture, a missing assembly and a wedged `UtilAcceptVsock` alike, and only the first of those means "paste the text" — the others meant pasting the stale X-clipboard text this path exists to stop. The answer is a word on stdout now, and a question that could not be put is given to the agent instead, which asks Windows itself and may get further. **A paste could land where a keystroke would not.** The probe's chord went to `focused_session` while `dispatch_session_input` routes by `focused_surface` and a program pane's key goes to the program: with an editor pane focused, the same press reached the program off WSL and the agent's shell under it. Both paste paths route by surface now, through one helper, so a pane that takes typing takes pastes. Covered end to end by `tui_e2e::a_paste_under_wsl_asks_windows_once_per_press_ and_never_from_a_float`, which drives the real binary with `WSL_DISTRO_NAME` set and a counting `powershell.exe` in front of `PATH` — the first test to reach the coordinator's own wiring. Without the loop's poll it fails on the second press; without the overlay guard it fails on the float's.
…o that lie The review bot's last finding on this branch, and two stale docs found while acting on it. `windows_clipboard_has_image` was documented as returning `false` for "no image" — it returns one of three verdicts, and telling them apart is the whole point of the commit that introduced it. `wait_bounded` claimed its `false` kept thurbox "doing what it did before the probe existed", which is the opposite of what a killed probe now does: the press goes to the agent precisely so the stale X-clipboard text is not pasted. A comment describing behaviour the code no longer has anchors the next reader on the wrong intent. The rest is trimming. The one-question-at-a-time rule was argued three times over, on the field, on `ask` and on `poll`; it is stated once now, where the flag lives. The routing rule was written out twice — `dispatch_session_input` inlined the same match `send_to_surface` performs — so the keystroke path calls the helper and the rule has one home. What stays is the part no reader can infer from the code: the measurements, the tie-break that keeps an ordinary Excel copy from being read as an image, and why an unanswerable question is not "no image".
Review of the image probe: `Command::new("powershell.exe")` resolves through
the OS `PATH` search, and that search reads an empty entry — a stray leading,
trailing or doubled `:` — as the current directory. A file of that name sitting
in whatever repository thurbox was launched from would therefore answer the
question on every `Ctrl+V`. Bounded (null stdin, discarded stderr, only two
exact words believed) and still an attacker-placed binary being run.
Thurbeen#1100 landed the rule for agent spawns: absolute `PATH` entries only, which is
what `paths::resolve_on_path` implements. The probe holds the same rule now,
with the hard-coded interop path as the last candidate rather than as a
fallback for a failed spawn — the loop stops at the first candidate that runs,
and a post-spawn failure is still `Verdict::Unknown` with no retry, because a
wedged probe may have burned the whole five-second deadline already.
Tested where it bites: a `powershell.exe` planted under a relative `PATH` entry
that really does resolve from the working directory must not be a candidate.
The OS search picks it up, which is what the test fails on when the resolver is
swapped back.
`clipboard` gains `paths` in the module allowlist, where it had `session` only.
On macOS the paste binding is `Cmd+V`, and declining a press is how the chord reaches the agent that can read an image — it falls through `on_key` to `dispatch_session_input`, which encodes it for the pty. `key_to_bytes` refuses every chord carrying `SUPER`: there is no legacy encoding for one. So the chord a Mac user actually presses fell through to nothing and was dropped in silence, where before this branch it at least produced the (misleading) clipboard hint — the bug in the title, unfixed on the platform's own chord. A declined paste that the pty cannot carry now sends the literal `Ctrl+V` byte itself, the way the probe's late answer already does, and only where the fall-through would have delivered anyway: no overlay owning typed input, and a focused pane that asked for raw session input. The decision the rest of it turns on is one function now. `paste_route` reads the two facts — is there a clipboard here, did it yield text — and both callers go through it: the plain press and the one that waited on Windows, whose verdict says only whose press it is, never what handling it looks like. Tested: the route that reaches every platform, which nothing pinned before (putting `Some(true)` back in `run_clipboard_action` left the suite green), and that a `Cmd` chord is not one a decline can hand on. The synthesised byte is written from the encoding rules, not from a Mac; `docs/FEATURES.md` says so rather than claiming a platform nobody ran it on. Also in the docs: an abandoned probe was described as read "no image", which is what the branch deliberately stopped doing, and the section now says that what the agent fetches arrives unread by thurbox — an image carries instructions as readily as text does.
The warning still spoke from the first version of the probe, where an unanswered question meant text. It has not meant that since the verdict grew a third value: the wait returning false becomes Verdict::Unknown, and Unknown hands the press to the agent rather than pasting anything.
`applies()` is `cfg!(unix) && current_wsl_distro().is_some()`, so on a windows-msvc build it is false whatever WSL_DISTRO_NAME says — which is the right production behaviour and the wrong assertion. Windows (tests) was red for this one test alone. The expectation becomes the cfg rather than the test being compiled away, so the gate stays pinned on both platforms: a distro asks Windows, a native Windows build asks nobody.
A distro without interop answered Unknown to every paste, and paid the walk down the candidate list to say it — for as long as the session lasted. Two rounds in which every candidate failed with NotFound now latch `applies()` off, and the press goes straight to the agent. Only NotFound counts. PermissionDenied, an exec format error or a policy that refuses the spawn are a machine that HAS a PowerShell and would not run it this time; latching on those would make a transient or administrative failure permanent, so they stay Unknown with the question still asked next time. Any answer at all clears the count, so this measures a standing absence rather than a total. Two rounds rather than one because interop publishes powershell.exe on PATH a moment after a distro starts, and a paste in that moment would otherwise silence the probe for the session.
The overlay rule is about the press, not about the delivery: a press made while a modal or a float owns typed input never named a pane, and swallowing it is right; a press made before the overlay went up named one already, and an answer arriving late does not unname it. FEATURES.md said only the first half, which read as a contradiction of what the code does. The delivery that lands behind an overlay is also the only one with no trace on screen — a byte the agent acts on rather than text appearing in a prompt — so it now toasts, the way the text path already does.
5061789 to
118dd5a
Compare
`cargo doc` runs with `RUSTDOCFLAGS="-D warnings"` in CI, and an intra-doc link from a `pub` item to a private one is an error there: "public documentation for `applies` links to private item `powershell_is_absent`". The sentence is the part that carries the reasoning, so only the brackets go — the helper is still named, and a reader inside the module still finds it. Checked both ways here: the doc build fails with exactly that error before the change and is clean (`rc=0`, no warnings) after it, so this is the only link that trips it.
The paragraph on the five-second deadline said a killed probe is "read as 'no image'". It never has been: a deadline gives `Verdict::Unknown`, and Unknown hands the press to the agent, which asks Windows itself. Reading it as "no image" is the stale X-clipboard paste the whole path exists to stop, so the sentence described the bug rather than the fix.
Pasting a screenshot into an agent pane did nothing.
Ctrl+Vis a global binding that always claims the press, and thurbox's clipboard transport carries text — so the image was dropped and the press never reached the CLI in the pane, which is the one thing here that can read an image.Claude Code fetches it itself on seeing
Ctrl+V— its trigger is the key event (if(e.key!=="v")return false; if(e.ctrl&&!e.meta) return platform!=="windows"), and it then shells out toxclip/wl-paste, or to PowerShell under WSL. So the fix is to stop swallowing the chord:kernel.pastedeclines withSome(false)when there is no text to paste — the same answerkernel.copyalready gives with no selection — and the press falls through to the focused terminal.Inside WSL that decision cannot be made locally. WSLg bridges the clipboard's text only. Copy an image in Windows and the X clipboard is not updated at all: it keeps handing out the last text copied. Measured here —
Clipboard::get_text()returned an IP address copied minutes earlier while Windows held a 1594x535 PNG — so the paste inserted something stale rather than nothing, which is the worse half of the bug. arboard cannot see the image either: thurbox builds it withdefault-features = false, the build withoutget_image.So
clipboard::ImageProbeaskspowershell.exewhether the Windows clipboard holds an image. That costs ~0.42 s (three runs: 0.42/0.41/0.43), which is far too long to hold the event loop for and is paid on every paste, not just image ones — so it is another instance of the worker pattern: the press is claimed, the interface keeps drawing, and the answer decides who gets it. Nothing is asked off WSL, where the local clipboard is the one being copied into.A forwarded chord is dropped while a modal is open, for the reason a pasted text is: it must not leak into the terminal behind the overlay.
Testing
only_a_wsl_distro_asks_windowspins the gate — it is the whole cost control, since a gate that answered "yes" everywhere would add a third of a second to everyCtrl+Von every platform.the_windows_probe_agrees_with_powershellruns the real round trip against the real Windows clipboard, because the wiring is what breaks: the argument list,-Sta(without itAdd-Typethrows and every answer becomes "no image"), thePATH/interop fallback, and reading the answer off the exit code. It skips where it cannot discriminate — off WSL, with no PowerShell, or with no image on the clipboard — and deliberately never writes to the clipboard: a suite that destroys what you copied is worse than one that skips. Both tests were checked with the fix stubbed out and fail.Rejected
Ctrl+Vafter copying an image still pasting stale text, which is the half that corrupts a prompt silently.