[Misty-Star]

	Files	Added	Deleted	Net
Test	3	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​74	$\color{#cf222e}{\Huge{\mathbf{−}}}$​2	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​72
Prod	2	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​35	$\color{#cf222e}{\Huge{\mathbf{−}}}$​22	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​13

ELI5

When Orca attaches to an iOS simulator, it checks that the helper process it just started is really running. It did that by reading the whole list of processes on the Mac. On machines where that list is bigger than 1 MiB, the read failed, Orca took the failure to mean "no helper", killed the healthy helper, and reported an unreachable stream. Now Orca asks about the one helper it knows by PID, and a failed process read is never mistaken for a missing helper.

What Changed

• Before: on a busy Mac (ps -axo pid=,command= over 1 MiB) every orca emulator attach for iOS failed with serve-sim started but its stream endpoint is not reachable., even though the helper was serving. After: attach keeps the reachable helper.
• listServeSimHelperProcessesForDevice checks a known helper PID with ps -p <pid>, so attach and session reuse no longer read the whole process table. ps -p exiting 1 with no output means the PID is gone. Any other failure now rejects instead of being turned into an empty list.
• The orphan scan used by cleanup (includeOrphaned) still reads the full table, now with the shared 32 MiB PS_MAX_BUFFER_BYTES ceiling used by Orca's other ps readers.
• Both paths require the helper's command to carry the device UDID, so a recycled PID that belongs to another process doesn't count.
• hasHelperForSession only runs after the stream endpoint has answered, so a failed lookup now counts as present instead of recycling the helper.

Why

The wrong fact ("no helper exists") was decided in listServeSimHelperProcessesForDevice, which ran every lookup through a 1 MiB execFile buffer and turned any error into []. hasHelperForSession swallowed errors the same way. Raising the buffer alone would still let a failed or slow ps read as absence. A targeted PID query keeps the attach path off a multi-second, machine-wide read, and propagating errors keeps "couldn't look" separate from "not there".

Linked Issue

Fixes #34787

Visual Proof

N/A: no UI changes. The visible effect is that attach succeeds instead of erroring; the before/after evidence is the real-process check under Testing.

Testing

• Real-process check on macOS: inflated the live process table to 1.56 MB with 10 long-argv processes, then started a fake serve-sim.js FAKE-UDID --exit-on-simulator-shutdown helper.

  • main: listServeSimHelperProcessesForDevice returned [] for both the known-PID and the orphan-scan lookups while the helper was running.
  • This branch: both found the helper, and both returned [] once it exited (the ps -p exit-1 path).

• pnpm test src/main/emulator/serve-sim-helper-processes.test.ts src/main/emulator/backends/ios-emulator-backend.test.ts src/main/emulator/emulator-bridge.test.ts: 42 passed. The new backend test (a reachable helper whose lookup fails is not killed) fails on main.

• Platforms: macOS only. The lookup returns [] on other platforms and the iOS backend is macOS-only.

• I manually tested these changes locally

• Automated tests added/updated, or explained why not below

AI Disclosure

Review

Maintainer follow-up on top of @Misty-Star's commits: replaced the hand-rolled streaming ps reader with the existing execFile reader at the shared 32 MiB ceiling, and collapsed the lookup to a single ps call. The known-PID merge was redundant because the device-filtered full scan already includes the known helper. Codex and Claude reviews came back clean apart from one P3: a dead recorded PID now hides a different live helper for the same device. That case costs one helper recycle, not a failure, and the narrower check is what keeps attach off the full table.

Agent skill upstream boundary

• Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Ensure no issues in: Security, Cross-platform support (Linux, Windows, Mac), Remote SSH, Mobile, general backwards compatibility, performance

• Cross-platform: the lookup returns [] on non-darwin, unchanged. The iOS backend only runs on macOS.
• Remote/SSH and mobile: emulator helpers run in the desktop main process. Mobile and remote clients reach them through the same attach RPC, so they get the fix without any wire change.
• Performance: attach and reuse now read one process instead of the whole table.

Checklist

• This PR is small and focused
• I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
• Before/after screenshots or videos attached for UI changes, or N/A with reason
• Self-reviewed for correctness, security, and performance
• Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
• pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

[coderabbitai]

Walkthrough

Device-scoped helper-process listing now queries a supplied helper PID or, when enabled, the full process table. It filters results by device and propagates process-list errors other than ps exit code 1 with empty output. The iOS backend treats a failed helper-process lookup as evidence that a helper is present. A test covers starting a reachable detached session without requesting a helper-process kill when lookup fails.

Severity of issue fixed: Medium

Merge Risk: Minimal · up to 98fc9

Helper discovery preserves the intended cleanup and missing-PID behavior. No material merge-blocking risk remains.

Pre-merge checks: 3 · 2

Failed checks: 2 warnings

Check name	Status	Explanation	Resolution
Linked Issues check		The changes satisfy the normal attach path in #34787. listServeSimHelperProcessesForDevice uses ps -p for a known PID, validates the device UDID, propagates lookup errors, and the backend preserve…	Implement bounded orphan discovery without relying on a complete process-table capture, such as streaming and filtering ps output. Add regression tests that exercise successful oversized output, verify the matching orphan list, and verify…
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 3 passed

Check name	Status	Explanation
Title check		The title clearly identifies the primary change: preserving iOS helpers when macOS ps output exceeds the buffer limit.
Description check		The description includes all required sections and clearly explains the user impact, implementation, rationale, linked issue, testing, platform scope, and checklist status. Visual proof is correctly m…
Out of Scope Changes check		The production changes and tests stay within #34787. They change iOS helper discovery, orphan cleanup behavior, process-read error handling, and reachable-helper preservation. The bridge expectation u…

Full details: Description check

Explanation

The description includes all required sections and clearly explains the user impact, implementation, rationale, linked issue, testing, platform scope, and checklist status. Visual proof is correctly marked N/A because there are no UI changes.

Full details: Out of Scope Changes check

Explanation

The production changes and tests stay within #34787. They change iOS helper discovery, orphan cleanup behavior, process-read error handling, and reachable-helper preservation. The bridge expectation update supports the changed helper lookup path.

Full details: Linked Issues check

Explanation

The changes satisfy the normal attach path in #34787. listServeSimHelperProcessesForDevice uses ps -p for a known PID, validates the device UDID, propagates lookup errors, and the backend preserves a reachable helper when lookup fails. The orphan path still reads the complete ps -axo output with a 32 MiB maxBuffer; it does not stream or filter incrementally. This only postpones the failure described by #34787. The tests cover known-PID lookup, missing PIDs, lookup rejection, and helper preservation, but they do not pass oversized successful output through orphan discovery or verify the related diagnostic and returned helper list.

Resolution

Implement bounded orphan discovery without relying on a complete process-table capture, such as streaming and filtering ps output. Add regression tests that exercise successful oversized output, verify the matching orphan list, and verify the process-inspection diagnostic and helper lifecycle.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. 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 1 functions across 5 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

• 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

• X
• Mastodon
• Reddit
• LinkedIn

[coderabbitai]

Actionable comments posted: 2

---

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: e3ab6d3b-8083-462c-aed7-1e50be674baa

Commits

Reviewing files that changed from the base of the PR and between 14ab7f0 and 8ac097e.

Files selected for processing: 4

• src/main/emulator/backends/ios-emulator-backend.test.ts
• src/main/emulator/backends/ios-emulator-backend.ts
• src/main/emulator/serve-sim-helper-processes.test.ts
• src/main/emulator/serve-sim-helper-processes.ts

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

[Misty-Star]

Addressed the CodeRabbit and reviewer findings in commit e180a5b262e:\n\n- orphan-inclusive cleanup now streams the full process table and merges matching orphan helpers with the known PID, avoiding the aggregate maxBuffer limit;\n- normal session validation remains a bounded PID query;\n- macOS ps -p exit code 1 with no output is treated as an absent PID, while stderr/other failures still propagate;\n- added regression coverage for orphan merging, missing PIDs, real query failures, and updated the bridge expectation.\n\nValidation: pnpm check:changed --maxWorkers=2 and 52 affected emulator tests pass.

[AmoabaKelvin]

Thanks @Misty-Star, the fix was at the right place. I merged main and pushed one commit on top: the orphan scan now uses the existing execFile reader at the shared 32 MiB ceiling instead of a hand-rolled streaming reader, and the lookup makes a single ps call, since the device-filtered scan already includes the known PID.

[coderabbitai]

Nitpick comments: 1

src/main/emulator/serve-sim-helper-processes.test.ts: 1

77-123: Trivial · Functional Correctness · Quick win

Add a combined-options orphan-merging regression test.

The real listing suite does not exercise helperPid with includeOrphaned: true and two helpers for the same device. The backend test cannot detect this regression because it mocks the listing function. Add a fixture that asserts both helpers are returned and that the full-table ps query is used.

Suggested fix

   it('treats a missing known PID as an absent helper', async () => {
     execFileMock.mockImplementationOnce((_command, _args, _options, callback) => {
       callback(Object.assign(new Error('ps exited with code 1'), { code: 1 }), Buffer.from(''), '')
     })
 
     await expect(
       listServeSimHelperProcessesForDevice('UDID-1', { helperPid: 123 })
     ).resolves.toEqual([])
   })
 
+  it('includes the known and orphaned helpers for the same device', async () => {
+    execFileMock.mockImplementationOnce((_command, args, _options, callback) => {
+      expect(args).toEqual(['-axo', 'pid=,command='])
+      callback(
+        null,
+        Buffer.from(
+          ' 123 /Applications/serve-sim/bin/serve-sim-bin UDID-1 --port 3100\n' +
+            ' 456 /Applications/serve-sim/bin/serve-sim-bin UDID-1 --port 3101\n' +
+            ' 789 /Applications/serve-sim/bin/serve-sim-bin UDID-2 --port 3102\n'
+        ),
+        Buffer.from('')
+      )
+    })
+
+    await expect(
+      listServeSimHelperProcessesForDevice('UDID-1', {
+        helperPid: 123,
+        includeOrphaned: true
+      })
+    ).resolves.toEqual([
+      {
+        pid: 123,
+        command: '/Applications/serve-sim/bin/serve-sim-bin UDID-1 --port 3100'
+      },
+      {
+        pid: 456,
+        command: '/Applications/serve-sim/bin/serve-sim-bin UDID-1 --port 3101'
+      }
+    ])
+  })
+
   it('propagates process enumeration failures instead of treating them as no helper', async () => {

---

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: b6a2f3e6-84e0-4f55-ab7a-c855038c1eca

Commits

Reviewing files that changed from the base of the PR and between e180a5b and 98fc9a7.

Files selected for processing: 4

• src/main/emulator/backends/ios-emulator-backend.ts
• src/main/emulator/emulator-bridge.test.ts
• src/main/emulator/serve-sim-helper-processes.test.ts
• src/main/emulator/serve-sim-helper-processes.ts

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