fix: harden Windows clipboard persistence (#27 part 2)#30
Merged
ShunmeiCho merged 1 commit intomainfrom Apr 22, 2026
Merged
Conversation
Clipboard data vanished between windowsSetClipboardText and windowsSendCtrlShiftV: Set-Clipboard / Clipboard.SetText give clipboard ownership to an internal window inside the short-lived PowerShell process; when the process exits, Windows destroys the window and empties the clipboard before the paste keystroke fires. Adopt explicit persistence semantics rather than asserting a single root cause: 1. Use Clipboard.SetDataObject(data, $true) — WinForms' documented "leave the data on the clipboard after this app exits" contract. 2. Call OleFlushClipboard() as a belt-and-braces commit, because the exact persistence path for delayed-render formats (notably Bitmap) depends on Windows version and format. Using both keeps the fix robust without relying on any single API. The same treatment is applied to windowsSetClipboardImage (used for the optional --restore-clipboard flow). Needs Windows smoke verification by the reporter before this can be marked ready for review. The deterministic hotkey-conflict portion of #27 already landed separately via #29. Refs: #27
This was referenced Apr 21, 2026
ShunmeiCho
added a commit
that referenced
this pull request
Apr 21, 2026
* docs: slim README structure before i18n Extracts long-form sections to docs/ to reduce README from 784 to ~400 lines before introducing trilingual translation. Also fixes three positioning issues flagged in the v0.6.0 post-release review. Extractions (content preserved in docs/, replaced in README with intent + link): - SSH Notifications setup/manual config/nonce registration/ troubleshooting -> docs/notifications.md (new, 136 lines). README section shrinks from ~120 lines to ~25. - Full command reference (all flags, Windows workflow, environment variables) -> docs/commands.md (new, 69 lines). README section shrinks to the 10 most-used commands. - Four troubleshooting entries already duplicated in docs/troubleshooting.md (SSH ControlMaster, stale sshd, token expired, launchd pngpaste) -> deleted from README. - "Setup fails: killed during re-deployment" -> appended to docs/troubleshooting.md (was README-only). Content fixes (no extraction, just correctness): - Headline: clarified that notifications apply to Claude Code and Codex CLI, not opencode (which is listed but notifications are⚠️ in the Supported Remote CLIs table — avoid headline overclaim). - Solution data flow: added opencode row (previously only Claude Code and Codex CLI were shown, but opencode is a supported remote CLI since v0.6.0). - Windows Quick Start: added experimental callout with pointer to open #30 (clipboard persistence hardening) so users don't assume v0.6.0 fixed both parts of #27. - Configuration table: collapsed duplicate "top + details" into a single table with a link to docs/commands.md. Out of scope (future PRs): - Any translation work. - i18n scaffold (README.zh-CN.md, README.ja.md, docs/i18n/). - Removing more low-frequency troubleshooting entries — pending evidence from post-release issue frequency. Ref: i18n prep sequence (step 1 of 5). * docs: clarify Codex setup path in README Promotes the Codex workflow from a collapsed `<details>` block to a first-class Quick Start step so users running Codex CLI don't miss the `--codex` flag on their first install. Targets the "not very clear" complaint raised during v0.6.0 review. Scope: 1. "Which setup command do I run?" decision table at Step 2: maps remote CLI (Claude only / Claude + Codex / opencode / Windows local) to the exact command, with a rule-of-thumb callout so people don't add `--codex` unnecessarily. 2. New Step 3 "what `--codex` adds" — explains the three things `--codex` actually does on the remote (Xvfb, x11-bridge, DISPLAY injection) so users have a model for later troubleshooting. 3. Codex-specific verify block at the Verify section: four commands that check DISPLAY, Xvfb, x11-bridge, and end-to-end xclip negotiation on the remote, with expected outputs. Uses only existing CLI flags — does NOT invent a `cc-clip doctor --codex` flag. 4. "`setup` vs `connect`" table: explains when to use first-time setup vs the repair/redeploy path (`connect --force`, with `--codex` preserved for the Codex variant), plus `--token-only`. Deliberately out of scope: - No changes to How It Works, SSH Notifications, or Related. - No release-notes inlining (badge already shows latest version). - No contributor/agent process regs (those belong in CLAUDE.md). - No invented flags. The Codex verify uses the four commands already documented in the Troubleshooting section. Change: +58/-16 in README.md, no other files touched. TOC is unaffected because new sub-sections are H4/H3 under the existing Quick Start H2. * docs: fix two review findings on canonical English before i18n MEDIUM (README:228) — `setup` vs `connect` table used `cc-clip setup myserver [--codex]` as if the brackets were literal shell syntax. First-time users copy-pasting it would hit `zsh: no matches found: [--codex]`. Split the column into two explicit commands (Claude-only / Claude + Codex), each a ready-to-paste string. Reverted the bracket-annotation anti-pattern at the same time. MEDIUM (README:247) — the Mermaid diagram and numbered path list under "How It Works" omitted opencode, even though the Solution data-flow block at README:59 and the Supported Remote CLIs table both call it out. This is exactly the canonical-source inconsistency that would get multiplied into the ZH and JA translations, so fix it before the i18n scaffold: - Added an `M["opencode"]` node to the Mermaid graph routed through the same `xclip / wl-paste shim` edge as Claude Code (shim is now renamed in the diagram to reflect both binaries it intercepts). - Inserted a new numbered path 2 describing the opencode flow explicitly as "same shim as the Claude Code path" so readers don't have to infer it from the diagram. - Expanded the Codex path 4 annotation with the `arboard` reason for why that path needs its own Xvfb/x11-bridge instead of the shim. No LOW items fixed in this commit: - LOW on `--from-codex-stdin` not appearing in `cc-clip --help` is a CLI gap, not a doc gap (the flag does work). Tracking separately. - LOW on "slim to 400" framing applied to chat context, not the PR body (which already reads "-20% from 784 to 631"). * docs: surface sudo prerequisite for --codex at the decision point Review flagged that the "two sudo-requiring contents" (Xvfb install on Debian/Ubuntu via apt, on RHEL/Fedora via dnf) were mentioned only in Step 3's parenthetical and in the Requirements section — after users had already decided to add --codex. Users on servers without passwordless sudo would walk through the entire setup flow before discovering they need to open a ticket with IT. Fixes: 1. Decision table now has a fourth column "Remote sudo needed?" with check/cross per workflow. Only the Claude + Codex row needs sudo; Claude-only, opencode, and Windows workflows do not. 2. A new callout directly under the table explains the sudo prerequisite, the two package-manager commands, the abort-and-retry flow, and the fallback (skip --codex if neither passwordless sudo nor manual install is possible). 3. Step 3 bullet 1 (Xvfb) promotes the sudo requirement from a parenthetical to a bolded "Requires sudo" statement so readers skimming the "what --codex adds" section also see it. Semantics-preserving: no new prerequisites invented, just made existing ones visible at the decision point rather than after commitment.
|
"1 ✅, 2 ✅, 3:✅ although with 3, I had to change the command to this to get it to work right: Cheers! |
Owner
Author
|
Follow-up on the cc-clip send --paste C:\test.png smoke-test note: I traced the current send parser and that trailing path was left in fs.Args() rather than being consumed as --file, so the command path still fell back to the clipboard upload path. I opened #41 to make this syntax explicit and tested: as a positional-file equivalent to: Keeping it separate from this PR because #30 is the Windows clipboard persistence fix; #41 is CLI argument UX. |
ShunmeiCho
added a commit
that referenced
this pull request
Apr 22, 2026
Windows smoke feedback showed that users naturally try cc-clip send --paste C:\test.png when they already have a saved image file. The old parser left that trailing path in fs.Args() and silently uploaded from the clipboard instead. Send now treats one positional image path as a --file equivalent, preserves host/default-host forms, and rejects ambiguous parser edges. Constraint: Go flag parsing stops at the first positional argument, so send normalizes leading and trailing positionals before upload Rejected: Error on all positional image paths | would keep an intuitive user command from working and preserve the confusing silent-drop behavior Confidence: high Scope-risk: narrow Directive: Keep parser tests covering accepted and rejected command forms before changing send CLI grammar Tested: go test ./cmd/cc-clip -run TestParseSendArgs -count=1; go test ./... -count=1; go vet ./...; go run ./cmd/cc-clip --help; env GOOS=windows GOARCH=amd64 go test -c ./cmd/cc-clip -o /tmp/cc-clip-windows.test.exe; combined #30 + #41 worktree go test/go vet/windows cross-compile Not-tested: Real Windows clipboard paste against a remote host for #41-specific parser behavior
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part 2 of 2 for #27. Part 1 (hotkey conflict validation) landed as #29.
This PR supersedes #28, which combined both portions and is being closed to keep the diff and description focused.
Summary
Clipboard data vanished between
windowsSetClipboardTextandwindowsSendCtrlShiftV:Set-Clipboard/Clipboard.SetTextultimately give clipboard ownership to an internal window inside the short-lived PowerShell process.This PR adopts explicit persistence semantics rather than asserting a single root cause:
Clipboard.SetDataObject(data, $true)— WinForms' documented "leave the data on the clipboard after this app exits" contract.OleFlushClipboard()(via a small P/Invoke shim) as a belt-and-braces commit, because the exact persistence path for delayed-render formats (notably Bitmap) depends on Windows version and format. Using both keeps the fix robust without relying on any single API.The same treatment is applied to
windowsSetClipboardImage(used for the optional--restore-clipboardflow).Why this PR is draft
This is a PowerShell/WinForms behavioral change on Windows. The project has no Windows CI, so Go-level green tests cannot prove runtime correctness. Held as draft until the reporter or a Windows host confirms the smoke checks below.
Verification performed locally
make test— full suite passes on darwinmake vet— cleanGOOS=windows GOARCH=amd64 go build ./cmd/cc-clip— passesGOOS=windows GOARCH=arm64 go build ./cmd/cc-clip— passesGOOS=windows GOARCH=amd64 go test -c ./cmd/cc-clip— Windows-tagged tests compile cleanlyVerification still needed (Windows host)
Requested smoke checks — full instructions posted in the issue comment thread (#27):
cc-clip send --paste <image>on Windows 11: remote path actually pastes into the focused terminalcc-clip hotkey --run-loopwith defaultalt+shift+v: copy image → hotkey → path pasted--restore-clipboard, the original image is back on the clipboard after pasteGet-Clipboardafter a failed paste (if any) to distinguish "data lost" from "keystroke not delivered"Files touched
cmd/cc-clip/send_windows.go(+30/-4) —SetDataObject(..., $true)+OleFlushClipboardwrapper, applied to both text and image settersRefs #27. Supersedes #28.