fix(desktop): isolate preview annotation keyboard input

[Lenoridev]

Typing an annotation in desktop Preview could lose spaces or ignore cursor movement when the inspected page handled those keys first. This moves annotation keyboard handling into the preload's early capture listeners, keeping normal editor defaults while isolating page shortcuts. Only trusted key events trigger keyboard actions, and teardown returns input to the page.

Based on @Lenoridev's original implementation in #20398. The takeover reduces the added tests to four behavioral cases, preserves contributor history, removes a prototype-sensitive shortcut lookup found during review, and integrates current main's viewport-cropping changes.

Fixes #20364.

Validation: 24 focused tests, desktop typecheck, scoped lint and formatting pass. Fresh full Opus and Sol reviews found no remaining source findings at d625e4d.

Native evidence was captured on d3b7e8b in a real Electron 44.4.5 webview running the complete production preload. The baseline produced abcdX; the corrected preload preserved the intended ab Xcd and blocked page key handlers during editing. Native multiline/attach, Ctrl+Enter send gating, synthetic-action rejection, inherited-key protection, tools/focus, teardown and reload were exercised. The later merge changes only viewport crop bounds/comment in this preload; both reviewers accepted bounded reuse for the unchanged keyboard path.

Reused native keyboard evidence: baseline 10b8682, corrected d3b7e8b. These captures are not executions of the final merged head.

Before: page consumes Space/caret input	After: editor retains ab Xcd

Supplemental native focus/teardown continuation at the same corrected proof head (2.67 seconds; not the baseline comparison):

Limits: this was a native preload fixture, not full-app verification. The final merged bundle and new cropping behavior were not executed; OS IME, macOS Cmd+Enter, full PreviewManager permissions/screenshots, composer persistence, real provider sending, network/relay and normal OS sandbox operation were not tested. The fixture used --no-sandbox; its regenerated stylesheet differed by one extra .shadow utility. This change is not a security boundary against deliberately malicious code sharing the page's JavaScript realm.

Current-head CI remains a landing gate.

Implemented with Claude Fable 5.1 in Claude Code; reviewed independently with Claude Opus 5.5 and GPT-6.1-Sol in Codex. Native evidence collected with GPT-6.1-Sol in Codex through T3 Code.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[macroscopeapp]

Approvability

Verdict: Approved at aef0ddf

Macroscope's review found this PR approvable — This is a focused desktop preview bug fix that moves annotation keyboard handling into the preload phase so inspected-page shortcuts cannot consume overlay input. The change is isolated, tested, and does not alter schemas, defaults, deployment behavior, or static-analysis configuration.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

• Last reviewed commit: d625e4d

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: 14b18812-8712-4149-a93e-0ae181723418

📥 Commits

Reviewing files that changed from the base of the PR and between 83044d9 and d625e4d.

📒 Files selected for processing: 3

• apps/desktop/src/preview/AnnotationKeyboard.test.ts
• apps/desktop/src/preview/AnnotationKeyboard.ts
• apps/desktop/src/preview/PickPreload.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

📝 Walkthrough

The change routes preview keyboard events through preload capture listeners and a shared annotation handler. Overlay events are isolated from page listeners. The handler supports comment submission, cancellation, and page tool shortcuts.

Changes: Preview annotation keyboard handling

Layer / File(s)	Summary
Keyboard handling rules and tests apps/desktop/src/preview/AnnotationKeyboard.ts, apps/desktop/src/preview/AnnotationKeyboard.test.ts	handleAnnotationKeyEvent handles overlay event isolation, comment submission, Escape cancellation, and tool shortcuts outside the overlay. Tests cover these cases and event propagation.
Preload event routing apps/desktop/src/preview/PickPreload.ts	Preload capture listeners forward keydown, keyup, and keypress events to the active session. The session uses the shared handler to cancel, submit, or change tools. The former root-level submission handler and session-level keydown listener are removed.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: t3dotgg

Merge Risk: ⚪ Minimal · up to d625e

The desktop annotation keyboard change is mergeable after normal checks; no unresolved behavior issue was established.

Security architecture risk: 🔵 Low · up to d625e

The change is confined to desktop preview annotations and strengthens keyboard-action controls. No expanded submission authority was found. Remaining uncertainty concerns verification of event isolation and editing behavior in the running desktop application.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed authority is bounded to keyboard actions for the active desktop annotation session. The inspected change does not add a service, credential, data-store, or cross-tenant capability; submission continues through the existing annotation IPC path.

Security Findings and Attack Paths

• observed — The new handler rejects untrusted keyboard events before returning submission, cancellation, or tool actions. Thus a page-dispatched Enter on the visible overlay host does not gain annotation submission authority through this handler.

Trust Boundaries and Controls

• inferred — Closed-shadow event retargeting makes the overlay host the outside listener's target, while preload retains direct access to comment focus. Early capture handling therefore isolates overlay keyboard events from later page listeners without cancelling ordinary editing defaults. This is a keyboard-path control, not a general sandbox guarantee against hostile page code.

Resilience and Maintainability Implications

• observed — The keyboard route preserves existing transition guards: pendingCapture blocks repeated submission, send permission is checked before capture and again before delivery, finished suppresses late asynchronous delivery, and cancellation or capture failure uses guarded teardown. These controls contain work from cancelled or replaced sessions.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title is concise, specific, and accurately describes the main change: isolating preview annotation keyboard input in the desktop application.
Description check		The description clearly covers the problem, implementation, issue reference, verification results, screenshots, limitations, and agent attribution. It does not reproduce the template headings verbatim…
Linked Issues check		Issue #20364 requires desktop annotation keys to reach the editor without triggering page shortcuts. PickPreload.ts installs early capture listeners for keydown, keyup, and keypress. `Annotati…
Out of Scope Changes check		The changes are confined to apps/desktop/src/preview/AnnotationKeyboard.ts, apps/desktop/src/preview/PickPreload.ts, and related keyboard tests. The preload also carries the bounded viewport-cropp…

Full details: Description check

Explanation

The description clearly covers the problem, implementation, issue reference, verification results, screenshots, limitations, and agent attribution. It does not reproduce the template headings verbatim, but it provides the required information and is substantially complete.

Full details: Linked Issues check

Explanation

Issue #20364 requires desktop annotation keys to reach the editor without triggering page shortcuts. PickPreload.ts installs early capture listeners for keydown, keyup, and keypress. AnnotationKeyboard.ts stops propagation for overlay events without cancelling ordinary editing keys. It handles trusted Enter, Escape, and tool actions. It leaves spaces, caret keys, multiline input, and composition available to the editor. The focused tests cover isolation, submission, cancellation, untrusted events, keyup handling, and inherited shortcut properties. Teardown restores page handling through the active-session callback.

Full details: Out of Scope Changes check

Explanation

The changes are confined to apps/desktop/src/preview/AnnotationKeyboard.ts, apps/desktop/src/preview/PickPreload.ts, and related keyboard tests. The preload also carries the bounded viewport-cropping integration from current main, as described in the PR. No protocol, provider, web, server-browser, or mobile changes are shown. The implementation and tests support the desktop preview annotation keyboard objective.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[Lenoridev]

Addressed the synthetic-event concern in CodeRabbit's security review in e087d2e.

I reproduced it in the Chromium harness: page JavaScript could dispatch Ctrl+Enter on the visible overlay host while the comment was focused and cause a send. Annotation actions now require event.isTrusted; synthetic Enter, Ctrl/Cmd+Enter, and Escape are ignored. Overlay event isolation still preserves normal typing.

The rebuilt preload ignored all four synthetic events without submitting or closing, then accepted a real Ctrl+Enter. All 26 focused tests, desktop typechecking, targeted lint and formatting pass. The PR description and reproduction archive include the follow-up verification.

[Lenoridev]

The failed Build and Typecheck job reported one error: apps/web/src/components/preview/previewSessionSync.test.ts could not resolve react-test-renderer (TS2307). That test and its dependency declarations were unchanged from the PR's base. The Check job failed because it requires Build and Typecheck to succeed; lint and the test jobs passed.

Merged main in 83044d9 to include the upstream fix from #21274, which replaces the affected test's react-test-renderer use with real-browser testing. The PR still changes only its original three desktop files.

After syncing dependencies, desktop and web typechecks both pass. All 28 focused desktop tests, targeted lint, formatting, and git diff --check pass. CI will run against the updated head.

[juliusmarminge]

Taking over this one. I’ll reduce the tests to the useful coverage, update the implementation against current main, and verify it before publishing the changes.