Repository navigation
feat: add ios-device screenshot backed by devicectl - #118
Merged
Merged
Conversation
Physical iOS devices could observe (ui) and act (tap) but not capture the screen — issue #115 even records screenshot as unsupportable on this channel. That is true for the accessibility audit channel, but CoreDevice offers it: `xcrun devicectl device capture screenshot` captures any screen, with no development-signing requirement. - `sim-use ios-device screenshot`: resolves the device like the other ios-device verbs (same DeviceSession discovery and errors, --device optional with exactly one attached), then shells out to devicectl for the capture. Output mirrors the simulator verb: path on stdout, confirmation on stderr. Non-.png output paths are rejected up front (devicectl writes PNG only). - --output path resolution (file / directory / default naming) is hoisted into a shared SimUseCore OutputFilePath, replacing the two existing per-target copies in IOSSimScreenshotCommand and VideoOutputFile; both keep their public API as thin delegates. - DeviceSession.resolveDevice exposes discovery + selection without opening the audit service, for verbs that hand the work to another channel. Screen recording exists on the same channel (capture screen-record) but CoreDevice reports the capability unsupported on the available test device, so it is not exposed yet. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: onevcat <onevcat@gmail.com>
Review findings on the initial revision: - OutputFilePath.resolve() removed an existing file at the target before ios-device screenshot's .png validation ran, so a rejected `--output important.jpg` destroyed the user's important.jpg and then errored. Resolution and preparation are now separate steps: resolve() is read-only, and the new prepare() carries the destructive work (mkdir parent, replace existing file). The verb validates between the two; the simulator/video wrappers recombine them, so their replace-on- success behaviour (which predates this PR) is unchanged. Regression tests pin both: a rejected path leaves the existing file intact, an accepted path still replaces. - README example showed `./…` for the saved-path output; the command prints an absolute path (stdout: the path, stderr: confirmation). Example updated to match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: onevcat <onevcat@gmail.com>
Round-2 review finding: for a valid .png target the previous flow was validate -> prepare (removes existing file) -> devicectl capture, so a capture failure (device unplugged, devicectl timeout, CoreDevice error) destroyed the old file with nothing written. The prior regression test even pinned that early deletion as expected. ios-device screenshot now resolves and validates without touching the target (parent directories are still created up front), captures into a temporary sibling that keeps the .png suffix devicectl requires, and moves it over the target only on success; on failure the temporary is cleaned up and the existing file is untouched. Regression tests cover the injected-failure branch (existing content intact, no temporary left behind), atomic replacement, and fresh-file creation. OutputFilePath gains a non-destructive createParentDirectory(for:); prepare() (parent dirs + remove existing) remains for the simulator and video wrappers, whose replace-before-write behaviour predates this PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: onevcat <onevcat@gmail.com>
…tract
Two round-3 review findings on ios-device screenshot:
- The temporary capture name embedded the full target basename plus a
~50-byte suffix, so a valid NAME_MAX-length (255-byte) *.png target
failed with ENAMETOOLONG when devicectl created the temporary. The
temporary basename is now fixed and short
(.sim-use-screenshot-partial-<UUID>.png) — same directory and the
UUID already guarantee uniqueness; nothing needed the target name.
- The default filename interpolated the user-editable device name
verbatim, so a name like "My iPhone/Work" turned the default output
into a directory hierarchy (and ".." segments could walk out of the
current directory), breaking the documented "single file in the
current directory" default. Device names now pass through
OutputFilePath.safeFilenameComponent ("/" -> "-"); with separators
gone, a ".." can never form a standalone path component, which
closes the traversal case too.
Regression tests: 255-byte target captures (also verified live on
device), slash names stay one component, and a traversal-shaped device
name resolves into the current directory.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: onevcat <onevcat@gmail.com>
This was referenced Aug 27, 2026
angelmic
pushed a commit
to angelmic/sim-use
that referenced
this pull request
Sep 1, 2026
Follow-up to lycorp-jp#118, which fixed this for ios-device screenshot: the simulator default filename interpolated the FBSimulator name verbatim, and simctl accepts any free text as a name — "My iPhone/Work" turned the default output into a directory hierarchy, and ".." segments could walk out of the current directory. The name now passes through OutputFilePath.safeFilenameComponent, same as the physical-device verb. The Android default filename embeds the adb serial, whose accepted router charset already excludes separators; it is sanitised too as defence in depth. Video default filenames (sim-use-video-<ISO8601>) embed no user-controlled text and need no change. Regression tests pin the slash and traversal cases; verified live with a simulator actually named "Slash/Name Probe". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: onevcat <onevcat@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
sim-use ios-device screenshotcaptures a PNG of a connected iPhone or iPad display. Contributes to #115: the issue's constraints section records screenshot as unsupportable on the physical-device channel — true for the accessibility audit channel, but CoreDevice (xcrun devicectl device capture screenshot) provides it, and with fewer restrictions thanui/tap: whatever is on screen is captured, SpringBoard and system apps included, with no development-signing requirement. This removes the largest entry from the physical-device exception list ahead of the capability-model work.Design
DeviceSessiondiscovery asui/tap(newDeviceSession.resolveDevice— discovery + selection without opening the audit service), so everyios-deviceverb sees the same device set and errors, and--deviceremains optional with exactly one attached. Only the capture itself runs over CoreDevice.xcrun devicectl device capture screenshot --quiet --timeout 30. Pipe drain and error mapping follow theSimctlDeviceLister.runSimctl/Adb.runpattern; a non-zero exit surfaces devicectl's stderr. Non-.pngoutput paths are rejected up front (devicectl writes PNG only).--outputpath resolution is now shared. The identical file/directory/default-naming logic previously duplicated inIOSSimScreenshotCommandandVideoOutputFileis hoisted into oneSimUseCore.OutputFilePath; both keep their public API as thin delegates, and the new verb reuses it. Default name mirrors the simulator convention:Device Screenshot - <device name> - <timestamp>.png.ios-device tap'svalidate()(if let alias, id != nil).Not included (deliberately)
screen-record: exists on the same channel, but CoreDevice reportsThe capability "Screen Recording" is not supported by this deviceon the available test hardware (iPhone 15 Pro Max, iOS 26.6), so it cannot be verified and is not exposed. Documented in the README as capability-gated follow-up material.--json/ daemon routing forios-deviceverbs — separate steps of the CLI shape: make physical iOS devices a first-class target (listing, device-kind field, top-level + daemon routing) #115 plan.Verification
make build— passed, 0 warnings.make test— 1343 passed, 0 warnings; new offline coverage pins the devicectl argument vector, exit-code/stderr error mapping, drain success, and the default-filename convention.--device <UDID>,--output file.png, and--output <dir>all produce a valid 1290×2796 PNG in ~2.8 s end-to-end; a non-.pngpath fails with exit 1 and a clear message; the home screen (SpringBoard) captures fine, confirming the no-signing-requirement claim.ui/tapre-checked after theDeviceSessionrefactor.sim-use screenshotdefault and explicit paths unchanged after theOutputFilePathrefactor.devicectl device captureverified present on both installed Xcodes (26.6.0 and 27.0.0 Beta 5 — both ship devicectl 642.9.1).