Skip to content

perf(emulator): reuse registered device backend routing - #35616

Merged
OrcaWin merged 1 commit into
mainfrom
audit/emulator-registered-backend-routing
Oct 29, 2026
Merged

OrcaWin merged 1 commit into
mainfrom
audit/emulator-registered-backend-routing

Conversation

@OrcaWin

@OrcaWin OrcaWin commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​27 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​27
Prod 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​2 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1

Explicit emulator commands rediscover the owning backend even when its canonical device ID is already registered. Android discovery launches ADB/emulator inventory and queries every running AVD name before validating the target again.

Select the recorded backend from the existing session registry. Keep discovery for unknown IDs/names and after session removal, and preserve normal target validation. Production change:+2/-1; no new cache or wire fields. Workspace-derived renderer controls already use registered routing.

Validation:62 tests/four suites, exact parent one regression fails/27 controls pass, production focused TypeScript and all five normal precommit tasks. Real Node/Linux SDK-process fixture:10 devices/10 explicit taps141→21 launches, identical input argv; unknown/name/workspace/after-kill/offline controls preserved. Full matrix316→132 launches, all448 owned PIDs/profiles released. Exact committed62-module bundle rebuilt byte-identically. No actual device/ADB/ElectronUI/macOS/Windows/CPU/RSS/latency claim.

Evidence:/home/neil/.local/state/orca-electron-audit/emulator-registered-backend-routing/report.md

Required pnpm check:changed --maxWorkers=2 queued after #35607. Keep draft until it passes; rebase/revalidate current main and preserve adjacent emulator fixes before merge.

@OrcaWin
OrcaWin marked this pull request as ready for review October 29, 2026 20:38
@greptile-apps

greptile-apps Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The routing change appears safe to merge.

Summary

Explicit device commands now reuse the backend recorded in the session registry.

  • Explicit emulator commands use the saved backend for registered devices.

Reviews (4) · Last reviewed commit: "perf(emulator): reuse registered device ..." · Reviewed by Greptile

@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 09c4f16a-30d0-4fdb-98e5-3aefc2eec5e7
Commits

Reviewing files that changed from the base of the PR and between 4eab2b6 and 5e7e8a2.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
Walkthrough

EmulatorBridge now selects the backend recorded for a registered device session. Without a registered session, it continues to check backend ownership. A test verifies that tapping a registered device skips simulator inventory lookup, and that tapping after kill clears the active session uses simulator inventory.

Priority: Low

Merge Risk: Minimal · previous review 4eab2

Registered devices reuse their recorded backend. No merge-blocking defect was established; complete the queued checks before merging.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description explains the implementation, rationale, testing, and known limitations, but it does not follow the required template. It omits the required section headings and information for ELI5, W… Rewrite the description using all required template headings. Add a plain-language ELI5 summary, explicit before/after behavior and mechanism, rationale, an actual linked issue using the required Fixes reference, Visual Proof or N/A with a …
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly identifies the main change: reusing registered emulator backend routing. It is concise and specific.
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files.

Full details: Description check

Explanation

The description explains the implementation, rationale, testing, and known limitations, but it does not follow the required template. It omits the required section headings and information for ELI5, What Changed, Why, Linked Issue with a Fixes reference, Visual Proof or explicit N/A, AI Disclosure, Review, Notes, and Checklist.

Resolution

Rewrite the description using all required template headings. Add a plain-language ELI5 summary, explicit before/after behavior and mechanism, rationale, an actual linked issue using the required Fixes reference, Visual Proof or N/A with a reason, testing steps and platform coverage, AI Disclosure when applicable, review notes, Agent skill boundary, Notes, and completed Checklist items. Keep the existing test evidence and pending check status.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Follow-up Comments

Nitpick Add a registered Android routing case.

95-121: Trivial · Functional Correctness · Quick win

Add a registered Android routing case.

This test records device-1 as iOS because session() omits backend. It cannot detect a regression that routes a registered Android device to iOS. Add an Android registration and assert the selected backend without starting adb.

Suggested fix
+  it('routes a registered explicit Android device to its recorded backend', async () => {
+    listSimulatorDevicesMock.mockResolvedValue([
+      { udid: 'device-android', name: 'Android-shaped', state: 'Booted', runtime: 'iOS' }
+    ])
+    const bridge = new EmulatorBridge()
+    bridge.registerActiveEmulator('wt-android', session('device-android'), {
+      managed: true,
+      backend: 'android'
+    })
+
+    await expect(
+      bridge.runCapability('install', { device: 'device-android' }, async (backend) => backend.kind)
+    ).resolves.toBe('android')
+  })

Open the source location


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c1181ed4-2fb2-46e0-8c14-5f184f18b337
Commits

Reviewing files that changed from the base of the PR and between 6037d91 and 4eab2b6.

Files selected for processing: 2
  • src/main/emulator/emulator-bridge.test.ts
  • src/main/emulator/emulator-bridge.ts

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

@OrcaWin
OrcaWin force-pushed the audit/emulator-registered-backend-routing branch 2 times, most recently from e67f665 to 589ad11 Compare October 29, 2026 21:18
@OrcaWin
OrcaWin force-pushed the audit/emulator-registered-backend-routing branch from 589ad11 to 5e7e8a2 Compare October 29, 2026 21:43
@OrcaWin
OrcaWin merged commit a444742 into main Oct 29, 2026
41 checks passed
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