Skip to content

fix(terminal): allow Windows paste shortcuts - #5485

Open
Glucksberg wants to merge 2 commits into
block:mainfrom
Glucksberg:agent/windows-terminal-paste
Open

Glucksberg wants to merge 2 commits into
block:mainfrom
Glucksberg:agent/windows-terminal-paste

Conversation

@Glucksberg

@Glucksberg Glucksberg commented Aug 10, 2026 •

Copy link
Copy Markdown

Problem

Buzz Term encoded both Ctrl+V and Ctrl+Shift+V as the ^V control byte on Windows, then called preventDefault(). That prevented WebView2 from delivering the clipboard payload to the terminal's existing paste handler.

Implementation

  • Detect paste chords before terminal key encoding.
  • Let WebView handle Ctrl+V and Ctrl+Shift+V on Windows.
  • Preserve the existing platform conventions: Cmd+V on macOS and Ctrl+Shift+V on Linux, while Linux Ctrl+V remains Readline quoted-insert.
  • Match the logical event.key so alternate keyboard layouts work, and reject Alt/mixed-modifier chords.
  • Keep clipboard text on the existing synchronous onPaste path, including bracketed-paste wrapping.

This is a focused follow-up discovered while field-testing #5425 / #4930. It is not a duplicate: #5425 fixes native PTY spawn, teardown, and PATH behavior; this PR fixes frontend keyboard event routing. I searched open PRs and issues for the Windows paste shortcuts and found no existing report or patch.

Tests

  • Added platform-matrix unit tests for Windows, Linux, and macOS paste chords.
  • Added a mounted Windows regression proving both shortcuts are not prevented, never send ^V, and paste exactly once through bracketed paste.
  • Added a mounted headless Linux regression proving Ctrl+V still sends ^V, while Ctrl+Shift+V waits for the paste event and forwards the clipboard exactly once through bracketed paste.
  • Targeted terminal suite: 35 passed, 0 failed.
  • Full just ci component set passed. The aggregate invocation was interrupted during Tauri compilation only because the isolated worktree filled the host disk; after removing its regenerable root target/, just desktop-tauri-test and just mobile-test both passed. Notable totals: 2,383 Tauri app tests passed (14 environment-dependent tests ignored), terminal crate suites passed, and 1,261 Flutter tests passed.
  • pnpm check, desktop typecheck, Rust fmt/clippy/check, web check/build, and git diff --check passed. The frontend check reports one warning and two informational findings already present on current main, outside these files.

Windows field proof

The exact patch was built and tested on Windows 11 before being cleanly rebased onto current main (the canary commit and the production-code commit in this PR have the same stable patch ID).

  • Successful Windows canary workflow
  • Canary commit: b00790a6ceb4cb0967fe07711d7174338cd92f8d
  • Installer: Buzz_0.5.9-fork.8_x64-setup.exe
  • Installer SHA-256: 049a0507694002af0f7232c3158f5915f0536fd9e260f506075404023695f4ef
  • Manual result: both Ctrl+V and Ctrl+Shift+V paste once without emitting ^V. In shells with bracketed paste enabled, the temporary active-region emphasis clears on the next keypress as expected.

Manual test

  1. Copy text in Windows and focus Buzz Term.
  2. Press Ctrl+V; confirm it appears once and no ^V is inserted.
  3. Repeat with Ctrl+Shift+V.
  4. With shell bracketed paste enabled, confirm the temporary active-region styling clears on the next keypress.

Visual evidence

No screenshot is included because this changes keyboard event routing and has no persistent visual state. The Windows canary run and manual interaction results above are the relevant behavioral evidence.

Deferred work

None for this fix.

@Glucksberg
Glucksberg marked this pull request as ready for review August 10, 2026 18:56
@Glucksberg
Glucksberg requested a review from a team as a code owner August 10, 2026 18:56
matt2e added a commit that referenced this pull request Nov 14, 2026
… closes (#9440)

## Summary
`desktop/src/features/terminal/TerminalSubstrate.test.mjs` can pass
every test and then run the Desktop JS job out of memory several minutes
later. This moves the React Testing Library unmount from `beforeEach` to
`afterEach`, so no tree is still mounted when the jsdom window closes.
Only the test file changes (6 insertions, 2 deletions).

Commit (cherry-picked unchanged from a local flake-fix series):
- `test(desktop): unmount every TerminalSubstrate tree before the window
closes`

### CI sighting

One sighting was found in the CI runs from 7 to 12 November 2026 that
were searched:

- 9 November 2026, "Desktop Domain / Desktop JS", run 42706004493
attempt 1 (on #9227; the rerun passed):
-
https://github.com/block/buzz/actions/runs/42706004493/job/129463488242

In that job the suite reported 6,950 of 6,951 passing. The log shows `An
update to TerminalSubstrate inside a test was not wrapped in act(...)`
at 21:54:33Z, then `FATAL ERROR: Ineffective mark-compacts near heap
limit Allocation failed - JavaScript heap out of memory` at 22:02:17Z,
and the file is reported as failed at file level (`test at
src/features/terminal/TerminalSubstrate.test.mjs:1:1`) after 504.6 s.

### Root cause

The file called `cleanup()` only in `beforeEach`, so the last test's
tree was still mounted when `after(() => dom.window.close())` ran. If
ThemeProvider's asynchronous palette update landed after that close, it
re-rendered the substrate, the splash effect's palette guard passed, and
`requestAnimationFrame` on the closed window started a fresh jsdom
interval that nothing cleared. That 60 Hz loop repainted the banner into
the test's recording `getContext` stub indefinitely, and `paintLog` grew
until the heap limit. No individual test hangs, so a per-test timeout
would not catch it.

### Fix

`afterEach(() => cleanup?.())` unmounts every tree, including the last,
before `after()` closes the window. The `beforeEach` call became
redundant and is removed. No assertions change, and no timeouts, retries
or sleeps are added.

### Related issue
N/A (flake fix). No existing issue or PR for this failure was found.
Three open PRs touch the same test file (#5057, #5485, #7777); none of
them changes where `cleanup` runs.

### Testing
Human testing is still required. This PR does not carry
`buzz-review-completed`; the author has not yet run this test themselves
on this branch.

#### Verification of the fix (before this branch was split out)
- The test file, 5 consecutive runs: 25 of 25 tests passed each time in
about 6.8 s, with no `act` warning.
- The CI out-of-memory failure was not reproduced with the real test
file. Its mechanism was reproduced in a scratch harness with the same
stubs: without cleanup before the window close, 3 of 3 runs left the
palette unresolved at close, added 11,556 `paintLog` entries per 100 ms,
kept one live timer, and never exited. With cleanup before the close, 3
of 3 runs added no entries, left no timers, and exited on their own.

#### Local checks on this branch
The push ran the repository's full pre-push hook set from a checkout of
this branch's head (`7f4117340`), with no lanes excluded and without
`--no-verify`. The branch is one commit behind `main`; that commit
changes only mobile and docs files, none of them touched here.

Ran and passed:
- `branch-skew`, `push-head-scope` and `file-size-check`
- `desktop-check` (`pnpm check`)
- `desktop-typecheck` (`pnpm typecheck`)
- `desktop-test` (`pnpm test`, the full desktop unit suite, which
includes `TerminalSubstrate.test.mjs`)

Did not run:
- `rust-tests`, `desktop-tauri-checks` and `mobile-checks`. The hook
selects lanes by changed path, and this branch changes no Rust, Tauri or
mobile file.
- Desktop builds and the Playwright E2E suites, which are CI-only.
- `just ci`.
- The 5-run repetition and the scratch harness above were not repeated
on this branch.

A green run does not prove the flake is gone: the failure needs the
palette update to land after the window closes.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: mtoohey <[email protected]>
Signed-off-by: Matt Toohey <[email protected]>
Co-authored-by: Claude Fable 5.1 <[email protected]>

This branch has not been deployed

No deployments
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.

1 participant