Skip to content

fix: track recording process by UDID for targeted stop#64

Open
saen-ai wants to merge 8 commits into
joshuayoes:mainfrom
saen-ai:fix/recording-pid-tracking
Open

fix: track recording process by UDID for targeted stop#64
saen-ai wants to merge 8 commits into
joshuayoes:mainfrom
saen-ai:fix/recording-pid-tracking

Conversation

@saen-ai
Copy link
Copy Markdown
Contributor

@saen-ai saen-ai commented Apr 21, 2026

Summary

stop_recording previously used pkill -f simctl.*recordVideo which killed every recording process on the machine — a problem when multiple simulators are recording simultaneously or when other idb/simctl commands are running concurrently.

This PR fixes that by tracking each recording's ChildProcess in a Map<udid, ChildProcess>. stop_recording now sends SIGINT to that specific PID only.

Fixes #20

Changes

  • Added activeRecordings: Map<string, ChildProcess> at module scope
  • record_video stores the spawned process after it starts, removes it on exit
  • stop_recording accepts an optional udid param, looks up the tracked process, kills by PID
  • Falls back to pkill if no tracked process found (e.g. server restarted mid-recording) — never worse than before

Test plan

  • Start a recording, then call stop_recording → video file saved correctly
  • Start two recordings on two different simulators → stop_recording with one UDID stops only that recording, the other continues
  • Restart MCP server mid-recording, call stop_recording → fallback pkill still works

saen-ai and others added 8 commits April 21, 2026 10:41
…dep CVEs

- record_video: was hardcoded to "booted", ignoring the udid param — now
  uses getBootedDeviceId() consistently with all other tools; also adds
  udid to the tool schema so callers can target a specific simulator

- ui_view: JSON.parse on idb output had no error handling — server would
  crash on malformed output; wrapped in try/catch with a clear error
  message; also validates frame dimensions are positive numbers before use

- ui_view: temp PNG/JPEG files now deleted immediately after reading
  instead of accumulating until server exit; file names include a random
  suffix to prevent collisions on rapid successive calls

- record_video: improved start detection — now rejects properly if the
  process exits early, increased timeout from 3s to 5s, tracks resolved
  state to avoid double-settling the promise

- deps: updated @modelcontextprotocol/sdk to latest (fixes CVE ReDoS,
  cross-client data leak, DNS rebinding — all high severity); ran
  npm audit fix for 6 additional moderate/low vulns in ajv, body-parser,
  minimatch, path-to-regexp, qs, diff

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
idb ui text only supports ASCII keycodes and throws 'No keycode found'
for any emoji or non-ASCII character. This adds a new ui_paste tool that
works around the limitation using the macOS pasteboard:

1. Copies text to the Mac clipboard via pbcopy
2. Syncs it to the simulator pasteboard via xcrun simctl pbsync
3. Long-presses at the given coordinates to trigger the paste menu
4. Finds the Paste button in the accessibility tree and taps it

This enables typing emoji, Arabic, Chinese, and any Unicode text into
simulator inputs — essential for testing apps with international users
or emoji-heavy content.

ui_type is unchanged and remains the right tool for ASCII text.
1.5s was triggering iOS system gestures (app switcher / home screen),
dismissing the app before the paste menu appeared. 0.8s is long enough
to trigger the contextual paste menu without conflicting with system gestures.
idb ui tap requires integer x/y values — passing floats like 55.166...
causes 'invalid int value' error. Round the calculated center coordinates.
terminate_app: kills a running app by bundle ID without having to
relaunch it — useful for testing cold-start flows and crash recovery

open_url: opens any URL or deep link in the simulator — essential for
testing universal links, custom URL schemes, and OAuth redirect flows

list_apps: lists all installed apps with their bundle IDs and display
names, sorted alphabetically — removes the need to manually look up
bundle IDs before calling launch_app or terminate_app
- max_size: resizes screenshot proportionally using sips when the image
  exceeds the given pixel dimension (width or height). Solves the Claude
  2000px API limit issue (joshuayoes#42).
- force: prevents silent overwrites by erroring when the output file
  already exists. Defaults to false (joshuayoes#19).

Fixes joshuayoes#42, Fixes joshuayoes#19
Instead of pkill-ing all simctl recordVideo processes, we now store
each recording's ChildProcess in a Map keyed by UDID. stop_recording
sends SIGINT to that specific PID, leaving any other simulators or
concurrent idb operations untouched.

Falls back to pkill if no tracked process is found (e.g. server
restarted mid-recording) so behaviour is never worse than before.

Also adds an optional udid param to stop_recording for multi-simulator
setups.

Fixes joshuayoes#20
@joshuayoes
Copy link
Copy Markdown
Owner

Thanks — PID tracking via a module-level Map<udid, ChildProcess> with an exit handler for cleanup is the right approach, and fixes #20 cleanly. A few things before I can land it:

  1. Same stacking issue as feat: add max_size and force params to screenshot tool #63. This branch sits on top of feat: add ui_paste tool for emoji and Unicode text input #60, feat: add terminate_app, open_url, and list_apps tools #61, and feat: add max_size and force params to screenshot tool #63, plus the merged fix: record_video ignores udid, ui_view JSON crash, temp file leaks, dep CVEs #59. Net-real-change for this PR is just commit 611e44e5. Please rebase onto the current joshuayoes/main:

    git fetch upstream
    git checkout -B fix/recording-pid-tracking upstream/main
    git cherry-pick 611e44e5
    git push --force-with-lease origin fix/recording-pid-tracking
    
  2. Silent pkill fallback doesn't match the existing error pattern. The current code uses .catch(() => {}) which hides two different outcomes: (a) pkill found nothing to kill (legitimate "no recording in progress" — user should see that message, not a success claim) and (b) pkill failed for an actual reason (should surface via errorWithTroubleshooting like every other tool). Nothing else in this file uses .catch(() => {}) — every tool body just wraps in try/catch and lets errors bubble. Suggest: when the Map lookup misses, either (i) just return "no recording in progress for udid X" without touching pkill, or (ii) run pkill and distinguish its exit codes (1 = nothing matched → friendly message; other nonzero → error through the normal path).

  3. Demo recording. Short ## Demo showing two scenarios would help: (a) two concurrent recordings on two UDIDs, stop one, confirm the other keeps going; (b) stop without a tracked process (post server restart) falls back cleanly. See Releases and feat: add ui_find_element tool #51 for format.

  4. Non-blocking question. If the MCP server process is killed mid-recording (hard crash, not SIGINT), does the recording subprocess get orphaned and keep writing, or does OS process-tree-termination handle it? Not something to solve in this PR, but worth a test if you haven't already.

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.

Targeted Recording Stop: Track Child PID and Kill Only That Process

2 participants