[mikepitre]

Why

CodeRabbit reviewed the Expo copy of the shared e2e harness in clerk/javascript#10236 and found five defects that also exist here, since the files are shared byte for byte. The shared part matches the clerk-ios change.

What changed

• run sent SIGTERM once on timeout. A child that ignores SIGTERM kept the promise pending, so timeoutMs set no bound. It now sends SIGKILL 2 s later.
• isRunning treated any ps failure as "exited". It now returns false only when ps exits 1 (no such pid).
• doctor --live ended the run inside finally without a catch. A failed run read there threw away every check and left the session token on disk. The failure is now a failed live-stop check.
• killEmulator called process.kill(-pid, 'SIGTERM') directly. If the emulator's group was gone by then, ESRCH skipped freeSlot and left the slot claimed. It now uses terminateGroup, which already ignores ESRCH.
• printed lowercased the recorder output but not the words it waited for.
• readTarget now refuses CLERK_E2E_DEV_SERVER with a borrowed Android emulator, whose dev-client app would start through adb on this machine.

Scope

The shared files are byte-identical in clerk-ios and the Expo stack, and MANIFEST is regenerated. The Android change is in the platform file this repo shares with Expo.

Verification

• Each new test failed on main and passes here.
• Unit tests: 910 of 910 pass, and tsc is clean.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved handling of timed-out processes, including forced shutdown when they do not exit promptly.
  • Made process-status checks more resilient when system tools are unavailable.
  • Prevented borrowed Android devices from being used with a development server.
  • Improved remote session cleanup and made output matching insensitive to letter case.
  • Improved emulator shutdown handling.
• Tests
  • Added coverage for Android device claims, process timeouts, remote session cleanup, and case-insensitive output matching.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: clerk/clerk-android/.coderabbit.yaml
• Review profile: CHILL
• Plan: Team
• Run ID: f5ac3f4d-4452-4c1b-8824-34a0171006f2

📥 Commits

Reviewing files that changed from the base of the PR and between f690ffd and b80b439.

📒 Files selected for processing: 11

• e2e-tests/specs/support/inputs.ts
• e2e-tests/src/core/MANIFEST
• e2e-tests/src/core/exec.ts
• e2e-tests/src/core/remote/preflight.ts
• e2e-tests/src/core/remote/session-agent.ts
• e2e-tests/src/platform/android/local.ts
• e2e-tests/test/android.test.ts
• e2e-tests/test/inputs.test.ts
• e2e-tests/test/processes.test.ts
• e2e-tests/test/remote.test.ts
• e2e-tests/test/session-agent.test.ts

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

📝 Walkthrough

The E2E suite now validates borrowed Android targets, escalates timed-out child processes, adjusts process-status checks, and records remote live-stop errors. It also uses process-group termination for owned emulators and matches recorder output without case sensitivity.

Changes: Android target validation

Layer / File(s)	Summary
Validate borrowed Android target configuration e2e-tests/specs/support/inputs.ts, e2e-tests/test/inputs.test.ts, e2e-tests/src/core/MANIFEST	readTarget rejects a borrowed Android device when a dev-server URL is set. The test covers the accepted device-daemon configuration and the rejected dev-server configuration. The manifest checksum is updated.

Process handling

Layer / File(s)	Summary
Handle process timeouts and status checks e2e-tests/src/core/exec.ts, e2e-tests/test/processes.test.ts, e2e-tests/src/core/MANIFEST	run sends SIGKILL and destroys output streams if a timed-out child remains alive after two seconds. isRunning returns false for a caught ps error only when its status is 1. Tests cover timeout handling and a missing ps executable.
Terminate emulator process groups e2e-tests/src/platform/android/local.ts, e2e-tests/test/android.test.ts	killEmulator uses terminateGroup for an owned emulator process that is not marked as the lane on the device. A test checks that release clears the claim after the process group exits.

Remote checks and session output

Layer / File(s)	Summary
Report live-stop errors e2e-tests/src/core/remote/preflight.ts, e2e-tests/test/remote.test.ts, e2e-tests/src/core/MANIFEST	liveChecks records session-shutdown or job-status errors as a failed live-stop check. The test checks reported failures and session-directory contents.
Match recorder output without case sensitivity e2e-tests/src/core/remote/session-agent.ts, e2e-tests/test/session-agent.test.ts	printed lowercases the expected phrase before matching child output. The test checks a match between “Recording started” and “Recording Started.”

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to b80b4

This change bounds timed-out child processes, improves doctor live-stop reporting, and tightens Android target validation. It adds tests for each behavior and shows no concrete merge-blocking risk.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 10 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly summarizes the primary changes: bounded process timeouts and preservation of doctor checks when a probe fails. It is concise and specific.
Description check		The description explains why the changes are needed, lists the main fixes, defines the scope, and reports verification results. It does not use the template headings exactly and omits screenshots, but…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description explains why the changes are needed, lists the main fixes, defines the scope, and reports verification results. It does not use the template headings exactly and omits screenshots, but it provides the required review context.

Full details: Docstring Coverage

Explanation

Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 10 files. (1 skipped: 1 unsupported.)

Resolution

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

• Fix all pre-merge checks with AI

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

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

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