Skip to content

fix: harden Windows clipboard persistence (#27 part 2)#30

Merged
ShunmeiCho merged 1 commit intomainfrom
fix/issue-27-clipboard-persistence
Apr 22, 2026
Merged

fix: harden Windows clipboard persistence (#27 part 2)#30
ShunmeiCho merged 1 commit intomainfrom
fix/issue-27-clipboard-persistence

Conversation

@ShunmeiCho
Copy link
Copy Markdown
Owner

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 windowsSetClipboardText and windowsSendCtrlShiftV:

  • Set-Clipboard / Clipboard.SetText ultimately 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 can fire.
  • Result: paste delivers an empty clipboard to the terminal.

This PR adopts 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() (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-clipboard flow).

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 darwin
  • make vet — clean
  • GOOS=windows GOARCH=amd64 go build ./cmd/cc-clip — passes
  • GOOS=windows GOARCH=arm64 go build ./cmd/cc-clip — passes
  • GOOS=windows GOARCH=amd64 go test -c ./cmd/cc-clip — Windows-tagged tests compile cleanly

Verification 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 terminal
  • cc-clip hotkey --run-loop with default alt+shift+v: copy image → hotkey → path pasted
  • With --restore-clipboard, the original image is back on the clipboard after paste
  • Get-Clipboard after 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) + OleFlushClipboard wrapper, applied to both text and image setters

Refs #27. Supersedes #28.

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
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.
@nucleoid
Copy link
Copy Markdown

"1 ✅, 2 ✅, 3:✅ although with 3, I had to change the command to this to get it to work right:
cc-clip send --paste C:\test.png

Cheers!

@ShunmeiCho
Copy link
Copy Markdown
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:

cc-clip send --paste C:\test.png

as a positional-file equivalent to:

cc-clip send --paste --file C:\test.png

Keeping it separate from this PR because #30 is the Windows clipboard persistence fix; #41 is CLI argument UX.

@ShunmeiCho ShunmeiCho marked this pull request as ready for review April 22, 2026 11:36
@ShunmeiCho ShunmeiCho merged commit f5c20f6 into main Apr 22, 2026
@ShunmeiCho ShunmeiCho deleted the fix/issue-27-clipboard-persistence branch April 22, 2026 11:36
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
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