fix(computer): type each character once when the key goes up

[innocarpe]

Description

orca computer type-text posted each character on both the key-down and key-up events. Electron contentEditable composers insert both payloads, so the text appeared twice with the placeholder between the copies.

Focused fix

In:

• The synthetic Unicode payload is attached only to the key-down event.
• The key-up event is still posted, with no character.

Out:

• Verifying the composer value after typing.
• Re-focusing the target window.
• Linux or Windows typing.

Preserves

Apps that already inserted on key-down still receive one character. The key-up event remains, so key state does not stay logically down. Synthetic typing stays unverified; this does not claim a read-back.

Evidence

swift test --filter KeyboardInputSafetyTests in native/computer-use-macos.

KeyboardInputSafetyTests passed, including testSyntheticUnicodeIsAttachedOnlyToKeyDown. swift build compiled main.swift.

A live Electron composer was not rechecked.

User-regression-tradeoffs

Receivers that inserted only from the key-up Unicode payload will stop receiving text. The reported composer inserted both, and key-down is the event that carries typed text.

Fixes #17942

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: edfdd6ed-eccb-4883-8c64-148cd4cb0834

📥 Commits

Reviewing files that changed from the base of the PR and between 537593a and c4a6785.

📒 Files selected for processing (1)

• native/computer-use-macos/Sources/OrcaComputerUseMacOSCore/KeyboardInputSafety.swift

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

Walkthrough

Input.typeText now uses KeyboardInputSafety.attachCharacter to attach each UTF-16 unit to the key-down event. The helper leaves the key-up event without a new Unicode unit. Tests check the unit counts and verify that attaching a character does not replace the key-up event’s existing Unicode units.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to c4a67

The change limits Unicode payloads to key-down events to address duplicate text. The live Electron composer was not rechecked, but synthetic input remains explicitly unverified and no concrete merge blocker is established.

Security Architecture Review

Security architecture risk: 🔵 Low · up to c4a67

The change preserves existing focus checks, typing authority, and key-release behavior. No new security exposure was demonstrated, but compatibility with external applications and callers of the new helper remains incompletely verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• observed — Synthetic text is posted through the global HID event tap, not through a PID-bound posting operation. This desktop-input scope predates the change; the new helper does not acquire posting authority or broaden that scope.

Trust Boundaries and Controls

• inferred — Within the inspected production path, caller-supplied text reaches the same focus-gated synthetic-input sink as before. Exporting the non-posting helper does not itself create a new external endpoint or bypass that gate; broader transport authentication and external consumers remain outside the established coverage.

Resilience and Maintainability Implications

• inferred — The change preserves normal key release and introduces no demonstrated new recovery failure. Typing remains non-atomic and best-effort: interruption between posts, repetition, concurrent focus changes, and partial delivery are not resolved by the helper and were not established as newly worsened.

🚥 Pre-merge checks | ✅ 1 | ❌ 4

❌ Failed checks (4 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title relates to the duplicate-typing fix, but it incorrectly states that characters are typed when the key goes up. The change attaches Unicode data only to key-down events.	Use a title such as "fix(computer): attach Unicode text only to key-down events" or "fix(computer): prevent duplicate text during type-text".
Description check	⚠️ Warning	The description explains the problem, implementation, scope, trade-offs, linked issue, and test results. It does not follow the required template and omits the required Visual Proof section, checklist…	Rewrite the description using the repository template. Add the required headings, mark Visual Proof as N/A with a reason or provide evidence, complete the Testing checklist, address cross-platform and compatibility considerations, and compl…
Linked Issues check	⚠️ Warning	Issue #17942 requires the macOS Electron contentEditable path to insert the supplied text once, keep the exact composer value, update React state, show Send, and return verified only after an exact …	Add pre-post confirmation for the target PID, window, and focused receiver. Read the exact value from the same receiver after typing. Return verified only when the value exactly matches the supplied text. Add automated coverage for verifi…
Docstring Coverage	⚠️ Warning	Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Out of Scope Changes check	✅ Passed	The production change is limited to macOS synthetic keyboard Unicode attachment. The tests cover the key-down and key-up event contract. These changes directly support the duplicate-text defect in iss…

Full details: Description check

Explanation

The description explains the problem, implementation, scope, trade-offs, linked issue, and test results. It does not follow the required template and omits the required Visual Proof section, checklist status, cross-platform and compatibility notes, review section, and Agent skill upstream boundary section.

Resolution

Rewrite the description using the repository template. Add the required headings, mark Visual Proof as N/A with a reason or provide evidence, complete the Testing checklist, address cross-platform and compatibility considerations, and complete the remaining checklist sections.

Full details: Linked Issues check

Explanation

Issue #17942 requires the macOS Electron contentEditable path to insert the supplied text once, keep the exact composer value, update React state, show Send, and return verified only after an exact same-receiver read-back. This PR attaches Unicode only to key-down and tests the event contract. It does not confirm the target PID, window, or focused receiver before posting. It does not read back the composer value or implement exact-match verification. The PR provides no evidence for React state or Send-button behavior.

Resolution

Add pre-post confirmation for the target PID, window, and focused receiver. Read the exact value from the same receiver after typing. Return verified only when the value exactly matches the supplied text. Add automated coverage for verification and reviewable integration coverage for the Electron contentEditable path.

• Fix all pre-merge checks with AI

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

Autopilot is currently an internal CodeRabbit preview.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[greptile-apps]

Confidence Score: 5/5

[Medium risk] Changes how keyboard input is synthesized on macOS.

The PR appears safe to merge; no outstanding findings or new actionable issues were identified.

Summary

The PR attaches synthetic Unicode text to key-down events while retaining character-free key-up events. It also adds a test that inspects the constructed events.

_{Reviews (3) · Last reviewed commit: "fix(computer): keep key-event helpers in..."}

[pullfrog]

Important

The change is safe and matches the reporter's suggested fix, but it is unverified against the receiver it targets, and the new comment's stated mechanism is contradicted by current Chromium source. Since the PR is written as Fixes #17942 (which auto-closes), please confirm the live repro and correct the rationale before merging.

Reviewed changes

• Key-up no longer carries a character — typeText attaches the Unicode payload only to the key-down CGEvent; the key-up is still posted but empty (main.swift:2666).
• Policy extracted to Core — KeyboardInputSafety.unicodeUnitCount(forKeyDown:) returns 1 for key-down and 0 for key-up (KeyboardInputSafety.swift:16), with a unit test pinning the two values (KeyboardInputSafetyTests.swift:24).

⚠️ The fix is not verified against the reported receiver

The only evidence is a unit test of a one-line helper plus a successful compile; the PR body states the live Electron composer was not rechecked. The macOS type-text e2e (tests/e2e/computer-mac.e2e.ts) is opt-in (ORCA_COMPUTER_E2E=1), is not run on PR (.github/workflows/computer-e2e.yml only runs the native-smoke jobs on PRs), and asserts toContain(marker), which would pass even if the text were duplicated. Because Fixes #17942 auto-closes the issue, verify the exact repro on an Electron contentEditable composer first, or scope the claim down.

Technical details

# Verify against the real receiver before claiming the fix

## Affected sites
- `native/computer-use-macos/Sources/OrcaComputerUseMacOS/main.swift:2658-2677` — the changed `typeText` path.
- `tests/e2e/computer-mac.e2e.ts:50-76` — `toContain(marker)` only; no assertion of a single occurrence.
- `.github/workflows/computer-e2e.yml` — PR triggers do not run `computer-mac.e2e.ts`; `native-smoke` runs on ubuntu/windows, `mac-native-owner-smoke` runs `pnpm verify:computer-native` (Swift tests).

## Required outcome
- Evidence the reported duplication is gone on an Electron/React `contentEditable` composer (or a clear statement narrowing what the PR fixes).
- If verification is not possible in this PR, avoid an auto-closing `Fixes #17942` until it is.

ℹ️ Nitpicks

• main.swift:2671-2673: if upUnits > 0 { up.keyboardSetUnicodeString(...) } is now unreachable — unicodeUnitCount(forKeyDown: false) is always 0. Dropping the guard (or the helper entirely) would make the key-up omission explicit.
• Pre-existing, but directly in this loop: iterating text.utf16 one code unit at a time splits non-BMP characters (emoji) into lone surrogate events. Other macOS type-string tools coalesce surrogate pairs into a single 2-unit event; worth a follow-up if emoji input matters.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[innocarpe]

Sync update (537593a5e3)

The typed character is attached only to the key-down event. The comment now matches Chromium, which forwards key-up without inserting text, and the test reads that Unicode string off the constructed events.

• Commit: 537593a5e35cfb421f7ad73ad6e076e0d4fff6da
• Upstream: fix(computer): type each character once when the key goes up #24382
• Fork portfolio: [upstream #24382] fix(computer): type each character once when the key goes up innocarpe/orca#296

[pullfrog]

ℹ️ No critical issues — prior feedback addressed cleanly; one minor API-surface suggestion inline.

Reviewed changes — re-reviewed the two commits pushed since the prior pullfrog review (bf10533).

• Corrected the key-up rationale — the comment now explains Chromium's key-down insertion path (key-up only dispatches a DOM keyup) instead of the inaccurate "Electron inserts the payload of both events" claim.
• Extracted the per-character event mutation — typeText now calls KeyboardInputSafety.attachCharacter(_:keyDown:keyUp:), removing the unreachable key-up guard the prior review flagged.
• Added event-level coverage — a test constructs a real key-down/key-up CGEvent pair and asserts the key-up carries no character, so it fails if a character is re-attached to key-up.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[innocarpe]

Sync update (c4a6785ad7)

The key-up Unicode reader and the unit count stay inside the Core module. The executable still calls attachCharacter, and the tests reach the helpers through testable import.

• Commit: c4a6785ad7035c85702f441241d143f06109a105
• Upstream: fix(computer): type each character once when the key goes up #24382
• Fork portfolio: [upstream #24382] fix(computer): type each character once when the key goes up innocarpe/orca#296

[pullfrog]

✅ No new issues found.

Reviewed changes — re-reviewed the commit pushed since the prior pullfrog review (537593a), a one-line-range visibility change that is a pure API-surface tightening.

• Tightened helper visibility — KeyboardInputSafety.unicodeUnitCount(forKeyDown:) and unicodeUnits(of:) are now internal instead of public. The only readers are the @testable test target, and attachCharacter (the sole symbol the executable calls) stays public, so the Core module no longer exposes test-only helpers.

  ^{｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}