Skip to content

refactor(computer): merge protocol-level RDP runtime into @midscene/computer#2380

Merged
quanru merged 2 commits intomainfrom
feat/rdp-cross-platform-helper
Apr 24, 2026
Merged

refactor(computer): merge protocol-level RDP runtime into @midscene/computer#2380
quanru merged 2 commits intomainfrom
feat/rdp-cross-platform-helper

Conversation

@quanru
Copy link
Copy Markdown
Collaborator

@quanru quanru commented Apr 20, 2026

Summary

This PR folds the protocol-level RDP runtime into @midscene/computer and removes the standalone @midscene/rdp package.

After this change, RDP is maintained as a remote Windows runtime under the computer package instead of as a separate package with its own agent surface.

Why

The previous split introduced an unnecessary package boundary:

  • @midscene/computer already owns the desktop automation surface
  • RDPAgent added no real behavior beyond wrapping PageAgent<RDPDevice>
  • maintaining a standalone @midscene/rdp package made the API and long-term ownership more fragmented than necessary

The intended model is:

  • one public desktop package: @midscene/computer
  • one agent helper: agentFromComputer()
  • multiple runtime implementations behind it, including local desktop control and protocol-level RDP control

What changed

Package and API consolidation

  • Moved the RDP runtime under packages/computer/src/rdp/**
  • Removed the standalone packages/rdp package
  • Removed the standalone RDPAgent / agentFromRdp() surface
  • Extended agentFromComputer() to support remote RDP targets via remote: { type: 'rdp', ... }
  • Re-exported RDP device/runtime types from @midscene/computer

Runtime and native helper ownership

  • Moved the RDP helper transport and protocol code into @midscene/computer
  • Moved the portable native helper sources into packages/computer/native/rdp/**
  • Updated the native build script so @midscene/computer now builds both:
    • bin/darwin/phased-scroll
    • bin/<platform>/rdp-helper

Tests and docs

  • Moved RDP unit tests under packages/computer/tests/unit-test/rdp/**
  • Moved RDP AI tests under packages/computer/tests/ai/rdp/**
  • Added computer-rdp AI test selection in the computer Vitest config
  • Updated the computer README to document RDP usage from @midscene/computer
  • Added a research note describing the consolidation rationale and maintenance boundary

Impact

  • Desktop automation now has a single public package boundary
  • RDP remains a separate runtime implementation, but not a separate package
  • Local desktop control and protocol-level RDP control still use separate device implementations, which keeps maintenance cleaner than merging both paths into one class

Validation

  • pnpm --filter @midscene/computer run build:native
  • NX_TUI=false npx nx build @midscene/computer --skip-nx-cache
  • NX_TUI=false npx nx test @midscene/computer --skip-nx-cache
  • AI_TEST_TYPE=computer-rdp pnpm --filter @midscene/computer exec vitest --run tests/ai/rdp/login-form.test.ts
  • pnpm install --lockfile-only
  • pnpm run lint

Notes

The fixture-based RDP AI flow passed after the consolidation.

The real-host RDP AI test is still environment-dependent. During the latest verification, the target host 10.75.166.249:3389 rejected FreeRDP connections with ERRCONNECT_CONNECT_FAILED, and the same failure reproduced outside the Midscene helper path with xfreerdp /auth-only, so that specific failure did not point to a code regression in this PR.

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Apr 20, 2026

Deploying midscene with  Cloudflare Pages  Cloudflare Pages

Latest commit: 4187da0
Status: ✅  Deploy successful!
Preview URL: https://4d75fa41.midscene.pages.dev
Branch Preview URL: https://codex-evaluate-naming-differ.midscene.pages.dev

View logs

@quanru quanru force-pushed the feat/rdp-cross-platform-helper branch from 1457484 to 6879bd0 Compare April 20, 2026 11:57
@quanru quanru marked this pull request as ready for review April 22, 2026 09:12
Copy link
Copy Markdown

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

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: 289cec48f5

ℹ️ 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".

Comment on lines +124 to +130
async connect(config: RDPConnectionConfig): Promise<RDPConnectionInfo> {
this.fatalHelperError = undefined;
await this.ensureHelperStarted();
const response = await this.send({
type: 'connect',
config,
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Terminate helper process after failed connect

When connect() receives a helper error (for example bad RDP credentials), it throws immediately after send() but never shuts down the spawned helper process. In retry-heavy flows like repeated agentFromComputer({ remote: ... }) attempts, each failed attempt can leave an rdp-helper child running, which wastes resources and can keep the parent process alive. Add cleanup in the failure path so unsuccessful connects do not leak a live helper.

Useful? React with 👍 / 👎.

Comment thread .gitignore Outdated
.rslib
**/.rslib
**/.native-build/
docs/
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 Restrict docs ignore rule to intended path

The docs/ pattern is unanchored, so it matches any docs directory in the repo, including apps/site/docs. That means new documentation files under the site docs tree will be hidden from git status unless force-added, making it easy to accidentally omit required EN/ZH doc updates. This should be scoped to the actual generated-output path (for example /docs/ if root-only) instead of globally ignoring all docs folders.

Useful? React with 👍 / 👎.

@quanru quanru force-pushed the feat/rdp-cross-platform-helper branch from 961b6ee to 4230180 Compare April 23, 2026 08:59
@quanru quanru merged commit e7d7c07 into main Apr 24, 2026
15 checks passed
@quanru quanru deleted the feat/rdp-cross-platform-helper branch April 24, 2026 03:16
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