Skip to content

fix(computer): preserve modifier keys through VNC - #3097

Draft
quanru wants to merge 2 commits into
mainfrom
fix/computer-vnc-physical-keyboard
Draft

fix(computer): preserve modifier keys through VNC#3097
quanru wants to merge 2 commits into
mainfrom
fix/computer-vnc-physical-keyboard

Conversation

@quanru

@quanru quanru commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • add an opt-in keyboardEventMode: physical mode for macOS AppleScript control of VNC clients
  • decompose uppercase and shifted en-US punctuation into base keys with explicit modifier key-down/key-up transitions while preserving logical as the default
  • expose the option through the public API and computer CLI tools, with VNC-only and keyboard-layout limitations documented
  • isolate AppleScript keyboard generation in a focused module and add regression coverage for defaults, shifted keys, multiple modifiers, special keys, and RDP argument filtering

Validation

  • pnpm run lint
  • pnpm run type-check:tests
  • pnpm exec nx test @midscene/computer --skip-nx-cache (125 passed, 1 skipped)
  • pnpm exec nx build @midscene/computer --skip-nx-cache
  • generated logical, physical-shift, and physical-multi scripts compiled with /usr/bin/osacompile
  • macOS AppKit live smoke passed for the unchanged logical default before the final module extraction; two post-extraction retries stopped before keyboard execution because the isolated fixture could not become the foreground application

Review

A serial standard and thermo-nuclear review was run against a frozen diff. The findings were resolved by extracting the AppleScript backend, documenting the physical-mode constraints, and strengthening boundary and modifier tests. The reconciled verdict is acceptable.

Remaining validation boundary

The explicit modifier sequence is covered by generated-script and integration tests, but this environment does not provide a real VNC server for an end-to-end remote keyboard test.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-04T04:00:13.826939Z 6c230b3 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6c230b3aa4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

shift: 'shift',
alt: 'option',
option: 'option',
meta: 'command',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve the normalized Windows modifier

When physical mode targets a Windows VNC session and KeyboardPress receives a shortcut such as Windows+R, normalizeKeyName() converts the modifier to win, but this map has no win entry; resolveModifierKeys() then silently filters it out and sends only r. Map win to the physical Command/Meta key, or reject it explicitly, so the shortcut is not silently altered.

AGENTS.md reference: AGENTS.md:L8-L8

Useful? React with 👍 / 👎.

@quanru
quanru marked this pull request as draft September 4, 2026 09:02
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