[build-tools] Install ffmpeg for argent screen recording when missing - #4110
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4110 +/- ##
==========================================
+ Coverage 62.25% 62.27% +0.03%
==========================================
Files 994 994
Lines 44820 44847 +27
Branches 9425 9430 +5
==========================================
+ Hits 27897 27924 +27
Misses 15474 15474
Partials 1449 1449 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
sjchmiela
left a comment
There was a problem hiding this comment.
The detection thing is very weird to me
| // Argent encodes screen recordings by piping simulator frames into `ffmpeg`, | ||
| // which it resolves from PATH and then from these prefixes. Keep the list in | ||
| // sync with argent so we never reinstall a binary it can already find. | ||
| const FFMPEG_FALLBACK_PATHS = [ |
There was a problem hiding this comment.
I don't see a reason to do it like this. I think our list should only check our expected places for ffmpeg and our expected list is just $PATH?
Does argent really not adhere to $PATH?
There was a problem hiding this comment.
they also read these paths, but we don't have to
| // large Homebrew dependency tree, so do not block session readiness on it: | ||
| // the session comes up at its usual speed and only a recording started in | ||
| // the first moments misses ffmpeg. Never rejects, so `void` is safe. | ||
| void ensureFfmpegInstalledAsync({ runtimePlatform, env, logger }); |
There was a problem hiding this comment.
Probably good for now but I wish we had easier time:
- logging stuff in phases at will
- run steps in parallel
Then we wouldn't mix so much in this one function and one log group…
Going to wait for hooks and custom fns to land and see if adding parallel: true would be so hard
| // which it resolves from PATH. It inherits this step's env, so PATH here is the | ||
| // PATH argent will search — no need to probe install prefixes ourselves. | ||
| async function isFfmpegAvailableAsync(env: BuildStepEnv): Promise<boolean> { | ||
| return (await asyncResult(spawn('sh', ['-c', 'command -v ffmpeg'], { env }))).ok; |
There was a problem hiding this comment.
honest question -- do we need sh -c?
eas-cli/packages/worker/src/env.ts
Line 72 in c3c3f08
There was a problem hiding this comment.
it should be better now
Argent encodes screen recordings by piping simulator frames into ffmpeg. The macOS worker image does not ship it, so `screen-recording-start` fails with "`ffmpeg` was not found on PATH" on every EAS Simulator argent session. Installing ffmpeg pulls a large Homebrew dependency tree, so this runs in the background rather than delaying session readiness. It is best-effort: screen recording is one optional argent tool, so a failure is logged and the session continues without it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The orchestration test replaces the whole remoteDeviceRunSession module with an explicit factory, so a new export has to be listed there too. Without it the step under test threw "ensureFfmpegInstalledAsync is not a function". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Argent screen recording supports Android emulators and encodes through the same ffmpeg path, so Android sessions hit the same failure as iOS. The Linux workers have passwordless sudo, so install with apt there instead of warning that the platform is unsupported. The package index can be older than the image the worker booted from, so refresh it first. A failed refresh is not fatal — the existing index may still resolve the package. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Argent resolves ffmpeg from PATH first and only probes install prefixes when that lookup fails, which is a workaround for hosts with a sanitized PATH. The tool-server inherits this step's env, so PATH here is the PATH argent will search. Probing the prefixes ourselves added nothing and coupled us to argent's internal fallback list. If ffmpeg is ever installed off PATH, the package manager no-ops on the redundant install. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
16bbcbd to
6c81c10
Compare
`command -v` is a shell builtin, which is the only reason `sh -c` was there. Spawning ffmpeg directly does the PATH lookup in the spawn call itself: it rejects with ENOENT when the binary is absent. Running it also proves the binary actually works, which a path lookup does not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The caller runs ensureFfmpegInstalledAsync in the background with `void`, and there is no unhandledRejection handler in the worker, so any rejection here would crash the process and take the live session down with it. `spawn` is not an async function and can throw synchronously. `asyncResult` only wraps an already-created promise, so it never saw that throw: it escaped through the unguarded availability check and out of the `void`. Wrap the whole body instead of trusting each callee, matching fetchServeSimTurnArgsAsync. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
✅ Thank you for adding the changelog entry! |
Why
Argent screen recording does not work on EAS Simulator sessions. The failure is a missing
ffmpegbinary on the worker VMs.Verified live on session
019fadda-2063-7fbd-a6c6-7176b0de2681(iOS,--type argent):ffmpeg is argent's encoder — it pipes simulator frames into ffmpeg's stdin to produce the mp4. Argent resolves it from
PATH, then/opt/homebrew/bin,/usr/local/bin,/usr/bin. The worker images ship none of them.Nothing else is wrong. Argent's
screen-recording-startchecks, in order:SCREEN_RECORDING_STREAM_UNAVAILABLEstartCapture→resolveFfmpeg()→SCREEN_RECORDING_FFMPEG_NOT_FOUNDWe hit 3, which proves 2 passed: the frame stream already works on the VM. ffmpeg is the only blocker.
Also worth noting: with
--type argentthe tool-server runs on the EAS VM, andlist-devicesreturned a bootediPhone 17with noremote:prefix. So argent's "remote simulators are unsupported" gate does not apply. This is a missing binary, not an architecture problem.Why this lives in eas-cli and not only in the image
There is a companion infra PR that adds ffmpeg to the worker images: https://github.com/expo/turtle-v2/pull/2598. That change applies to images built from there on. It deliberately does not update or backfill existing images.
So this PR is not a stopgap for a short rollout window. It is what makes recording work on every worker running a current image, for as long as those images stay in rotation. Once a worker boots an image that already carries ffmpeg, the check finds it and this does nothing.
How
ensureFfmpegInstalledAsyncinremoteDeviceRunSession.ts, called fromeas/start_argent_remote_session.PATHplus the same three Homebrew prefixes — so we never reinstall a binary argent can already find, and the step is a no-op on images that ship it.android: { emulator: true, device: true, unknown: true }, simulator-server has anandroidsubcommand, and nothing on that path bypassesresolveFfmpeg(). The Linux workers have passwordless sudo (%sudo ALL=(ALL) NOPASSWD: ALL).brew install ffmpegpulls a large dependency tree and takes minutes. Awaiting it would delay session readiness by that much. Fired withvoidinstead, so the session comes up at its usual speed and only a recording started in the first moments misses ffmpeg. This is the main design decision worth pushback if you disagree.voidsafe.Test Plan
Unit tests in
remoteDeviceRunSession.test.tscover six paths: already on a fallback path, already onPATH, installs with Homebrew on darwin, installs with apt on linux, still installs when the apt refresh fails, and warns without throwing when the install fails.yarn typecheck(13 projects),yarn lint(0 errors), andyarn fmt:checkall pass.Not yet verified end-to-end on a live session — this is a draft. To verify, start an argent session on a current image, confirm the build log shows the install, then call
screen-recording-startand expect a started recording instead of the error above.