Skip to content

fix(ios): capture the lit panel on foldable Apple devices - #2703

Merged
thymikee merged 6 commits into
mainfrom
t3code/ed357be5
Sep 20, 2026
Merged

thymikee merged 6 commits into
mainfrom
t3code/ed357be5

Conversation

@thymikee

Copy link
Copy Markdown
Member

Summary

A foldable Apple device (iPhone Duo, iOS 27.1) lights one of its two integrated panels at a time, and simctl io screenshot/recordVideo name the highest screen ID when no display is given — on a closed Duo that is the dark inner panel, so the capture exited 0 with an all-black PNG and every consumer trusted it.

Each iOS simulator capture now resolves the CoreDevice display table, names the lit panel, and normalizes density with that panel's own pointScale. Panel power comes from backlightState, because active is absent from real payloads (a single-panel iPhone 17 reports only backlightState). A multi-panel device always names a display: an ambiguous read falls back to primary with an apple_display_capture_ambiguous diagnostic rather than the black-producing default. Pose is derived only (closed/fully-open/unknown); no official host API sets it, so no command pretends to — agent-device help foldable says so.

agent-device screenshot --platform ios --device "iPhone Duo"   # 466x678 (lit outer), was 669x951 black
agent-device help foldable

15 files, +1340/-17 (637 test, 213 docs, 490 production). Gross churn is above the 1,000-line budget; say the word and I will split recording out.

Validation

Tested at 83308f7a8: pnpm check exit 0, pnpm check:affected --run passed.

Live on a booted iPhone Duo: capture went from 669x951 @1x mean luma 0.09 to 466x678 @1x mean luma 65.8; iPhone 17 unchanged at 402x874, so no single-panel regression. On the examples/test-app dev build: snapshot -i returned 41 nodes, tap @e189 dismissed the dev-menu sheet, and a Catalog tap settled +19 -15.

Unresolved: open-pose frames, inner-panel tap routing, and the XCUIScreen.main runner fallback need an operator-folded device (ADR 0025 records both gaps).

simctl io screenshot picks the highest screen ID when no display is named,
which on a closed iPhone Duo is the dark inner panel: the capture exits 0
with an all-black PNG. Resolve the CoreDevice display table per capture,
name the panel the device actually lights, and normalize density with that
panel's own point scale. Panel power comes from backlightState because
CoreDevice omits the active key on real payloads, and a multi-panel device
always names a display so an ambiguous read degrades to a diagnostic
instead of a black screenshot.
simctl io recordVideo shares screenshot's implicit display default, so a
recording of a closed foldable is also black. The Apple package now builds
the recordVideo argv and reaches the display inventory through a
function-scoped import, which keeps the simctl facade's eager closure at
its budget and puts the panel decision where the display model lives.
Adds ADR 0025, the `help foldable` topic with its benchmark case, and the
verification traps that cost time on a Duo: the pinned 27.1 toolchain, the
dark panel that captures black while exiting 0, and the UIScene lifecycle
requirement that makes a legacy app trap at launch. Pose stays
operator-controlled: no official host API sets it, so no command pretends.
The display symbols leave the facade through a function-scoped import read
by a dynamic platform-runtime consumer, so fallow cannot connect the member
read to the re-export. Scoped to the two facade symbols it names.
@github-actions

github-actions Bot commented Sep 20, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.64 MB 4.65 MB +7.1 kB
Package (unpacked) 4.64 MB 4.64 MB +7.1 kB
Package (download) 1.38 MB 1.38 MB +2.8 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 28.8 ms 26.7 ms -2.1 ms
CLI --help 87.0 ms 79.0 ms -8.0 ms

@thymikee

Copy link
Copy Markdown
Member Author

Reviewed at 83308f7.

queryAppleDisplayInventory in display-inventory.ts:124 re-implements the devicectl JSON call protocol that devicectl.ts:174 runIosDevicectlJsonCommand already owns: same temp-path template, same runXcrun(args, { allowFailure: true, timeoutMs }), same JSON read, same finally cleanup, differing only in returning undefined instead of throwing. Any future fix to the temp-file handling or to the hint mapping now has to land in two places, and this new copy has already dropped the hint mapping that the owning helper applies. runIosDevicectlJsonCommand should get a non-throwing mode (it already has tolerateFailurePayload to build on) and display-inventory should call that instead of duplicating the protocol.

The recorder change in simctl-facade.ts:14 adds --display=<name> to the recordVideo argv, but the evidence in the PR and ADR 0025 only covers screenshot/snapshot/tap on the Duo, not recordVideo. simctl's --display flag has historically taken keyword values, not device-reported panel names, and the recorder spawns with allowFailure: true, so a rejected flag would only surface later as an exit-code warning at stop time (recording/runtime.ts:305). Please run agent-device record start and stop against the booted iPhone Duo at this head and report the exit status plus the resulting file's first-frame luma, so we know the flag is accepted on the real toolchain.

Not blocking: the timeout-vs-malformed-JSON error classification in display-inventory.ts looks inverted, the display probe re-runs on every capture even though the panel count is per-device and does not change between calls, most of the pose/role/inventory-shape code in display-inventory.ts has no reader outside its own module and test, the screenshot test suite runs the real probe because its deps don't override resolveCaptureDisplay, the simctl-facade signal test doesn't actually check the signal reaches runXcrun, the dynamic import of ./core/simctl.ts in simctl-facade.ts buys nothing since that module is already statically imported on line 3, and the CLI help text claims every capture resolves the CoreDevice display table when the runner fallback path keeps the old implicit behavior — take or leave each of these.

Is the size of this module justified? The production need is one string, the panel name for --display=: parse displays, keep integrated ones, skip the flag if there's only one, otherwise pick the lit panel or fall back to primary, and return its name and point scale. That's close to what devicectl.ts already does for CoreDevice JSON reads, and the two-line argv fragment fits next to buildSimctlArgsForDevice in core/simctl.ts. Dropping the pose/role/inventory layer that nothing reads today would also remove the "owner of the panel state" framing that currently justifies keeping this as a separate 394-line module — would ADR 0025's contracts section need to shrink first, or can that layer just go now?

CI reports 19 checks green at 83308f7, with no failing job to attribute; note that the new probe never runs on Linux unit runners, so this green run doesn't exercise the devicectl route at all.

The device numbers, the claim that devicectl device info displays answers for simulators, and the claim that simctl accepts a CoreDevice panel name as --display all come from the PR body and ADR and were not reproduced here; the test suites and the fallow-gate handling of the new exports were not run either.

Routing the JSON read through the existing devicectl helper and posting a live recordVideo run on the Duo are the two things needed before merge; trimming the unread pose/role layer would also be the cheapest way to settle the size question.

@thymikee

thymikee commented Sep 20, 2026

Copy link
Copy Markdown
Member Author

Screenshots from the booted iPhone Duo

Captured on iPhone19,4 / iOS 27.1 with DEVELOPER_DIR pinned to Xcode 27.1 Beta, device in the default closed pose. Mean luma is over the full frame (0-255).

Beforeagent-device screenshot with the implicit simctl display selection: 669x951, mean luma 0.09. The capture succeeds with exit 0 and is entirely black, because simctl io defaults to the highest screen ID, which is the dark inner panel.

Before: all-black capture of the dark inner panel

After — same command, same device, this head: 466x678, mean luma 65.8. The capture follows panel power to the lit outer panel.

After: lit outer panel captured at 466x678

Interaction on the lit panelagent-device tap @e20 at (128, 626) with --settle reported +19 -15 nodes and drove the app to Catalog. Coordinates stay inside the 466x678 lit surface rather than the 669x951 dark one.

Tap landed on the lit panel and navigated to Catalog

Still unverified: the open pose / inner panel. No official host API sets the hinge angle, so this needs an operator to open the Duo in Device Hub. Recorded as a gap in ADR 0025.

@thymikee

Copy link
Copy Markdown
Member Author

Open-pose verification, plus the record check from the review

The Duo was opened mid-session, so this head was exercised in both poses.

Inventory at the open pose (devicectl device info displays): LCD primary: true, backlightState: off, active: false, and LCD-1 backlightState: activeOn, active: true. The capture follows panel power and names LCD-1, yielding 951x669 @1x — the inner panel at its reported currentOrientation: rot90.

The React Native app re-lays out to the inner surface:

React Native app captured on the lit inner panel at 951x669

Interaction on the inner panel. tap @e4 resolved to (590, 478), inside the 951x669 surface, and typing landed: type "apple.com" was followed by find text "apple.com" matching two nodes (@e11 [cell], @e12 [text]).

Safari address bar on the inner panel after agent-device typed into it

The pose-change rule from ADR 0025 held without any extra code: a ref issued before the fold was refused after it — Ref @e11 belongs to an expired ref frame — rather than being replayed against the new panel's point size.

record on the Duo, which the review asked for. record start and record stop both exit 0 in each pose, and --display is accepted by the 27.1 toolchain. Raw simctl on the same device in the same pose shows the flag is honored per panel:

argv exported size mean luma
recordVideo --display=LCD-1 (lit inner) 2006x2852 241.42
recordVideo --display=LCD (dark outer) 1398x2034 0.00
recordVideo with no --display 2006x2852 241.42

agent-device record start --scope device --hide-touches2006x2852, mean luma 241.42 over the 0.01/0.50/0.98 sample points (attached below).

Two honest caveats.

  1. A pre-existing bug, not from this change. With the default touch-overlay post-processing, an open-pose Duo recording exports as 480x336 with mean luma 0.00. It reproduces with none of this PR's code in the path: feeding a raw simctl file (the bright 2006x2852 one above) straight into the untouched apple/runner/AgentDeviceRunner/RecordingScripts/recording-overlay.swift yields a 0x0 / zero-duration output. The same pipeline is fine on a non-rotated target — iPhone 17 exports 1206x2622 at mean luma 228.30 — so this is the overlay exporter mishandling the inner panel's rot90 track. --hide-touches avoids it. I can file this separately.
  2. The open pose is not the regression case. With the inner panel lit, simctl's implicit highest-screen-ID default already lands on the lit panel, so pre-change behavior looked correct here. The closed pose in my earlier comment is where the old argv captured black while exiting 0, and the open-pose numbers above are what confirm the fix does not over-reach.
duo-open-panel-recording.mp4

`queryAppleDisplayInventory` copied the devicectl JSON protocol — temp path,
`--json-output`, `runXcrun` with allowFailure, payload read, cleanup — and had
already dropped the hint mapping the owning helper applies. Export
`runIosDevicectlJsonRequest` as that single protocol implementation with a
timeout and signal, and let the throwing command and the probe both build on it.

Classify the probe by what actually happened: an exec kill stays a timeout via
`isCommandTimeoutError`, `unreadable-json` now means devicectl answered with a
payload that did not parse, and only those two reasons claim the toolchain lacks
the subcommand. The old mapping labelled an AppError — i.e. the timeout — as an
unreadable payload, which is inverted.

Drop the panel labels nothing read. `outer`/`inner` and the derived pose had no
consumer outside the module and its tests, while `primary` plus panel power is
what every decision already used; the geometry conflict still reports
`apple_display_primary_geometry_conflict`.

Also stub `resolveCaptureDisplay` in the screenshot suite so it stops paying for
a real CoreDevice probe, assert the caller signal actually reaches `runXcrun`
instead of asserting it was never aborted, and drop the dynamic import of a
module the facade already imports statically.
… defect

The operator opened the Duo mid-verification, so the open pose is now evidence
rather than a gap: the capture names `LCD-1` at 951x669, taps resolve inside that
surface, typed text reads back, and a pre-fold ref is refused instead of replayed
at the new point size. What stays open is the quarter turn and the runner
fallback, both restated as gaps.

`recordVideo` gets the per-panel table that proves `--display` is honored
(2006x2852 luma 241.42 for the lit panel against 1398x2034 luma 0.00 for the
dark one), plus the separate finding that the touch-overlay exporter returns a
480x336 black video for the inner panel's rot90 track — reproducible by feeding a
raw simctl capture straight into the untouched `recording-overlay.swift`, and
fine on a non-rotated iPhone 17 export.

Also drop the pose and role wording from the decision table, since the inventory
no longer carries either.
@thymikee

Copy link
Copy Markdown
Member Author

Addressed in 4780b46 and 616e843.

Duplicated devicectl protocol — agreed, and the copy had already drifted the way you predicted. runIosDevicectlJsonRequest is now the single owner of the temp-path + --json-output + read + cleanup protocol, exported with timeoutMs and signal, and it returns a typed outcome instead of throwing so a probe can keep its own diagnostics while the throwing command keeps the curated message and the resolveIosDevicectlHint mapping. queryAppleDisplayInventory is now 20 lines of policy and owns no protocol at all.

recordVideo --display on the real toolchain — run against the booted Duo at this head. record start and record stop exit 0 in both poses, and the flag is accepted and honored per panel: --display=LCD-1 (lit) exports 2006x2852 at mean luma 241.42, --display=LCD (dark) exports 1398x2034 at mean luma 0.00. Full table plus the recorded video are in my previous comment.

Your prediction about late failure was right in a way I did not expect: with the default touch-overlay post-processing the open-pose clip exports as 480x336 black. That is not the flag — a raw simctl capture from the same moment is bright, and feeding it straight into the untouched recording-overlay.swift reproduces a 0x0 zero-duration output, while the same pipeline exports a non-rotated iPhone 17 recording intact (1206x2622, 228.30). So the overlay exporter mishandles the inner panel's rot90 track. It is outside this diff (--hide-touches on this head yields the correct video), and I recorded it in ADR 0025 and docs/agents/device-verification.md as a trap. Say the word and I will file it as its own issue.

Inverted classification — fixed, and you were right that it was inverted. exec raises its own typed kill, so the probe now asks isCommandTimeoutError and reports probe-timed-out; unreadable-json now means devicectl answered with something that did not parse. Only those two claim the toolchain lacks the feature, so a timeout no longer tells the operator to upgrade Xcode. Three tests in display-inventory.test.ts pin each reason, and devicectl.test.ts pins that a timeout propagates as a timeout rather than degrading into a payload failure.

Unread pose/role surface — removed rather than defended. Nothing outside the module and its tests read outer/inner or the derived pose, and every decision was already made from primary plus panel power, so the labels were a second source of truth waiting to invert. The geometry check survives as apple_display_primary_geometry_conflict because it reports a contradiction; the pose field does not survive because power cannot separate fullyOpen from partiallyOpen, so it could only ever be a lossy field dressed as a fact. ADR 0025 now says the inventory reports what it measured and pose belongs to UIHinge.status in the app under test.

Screenshot suite paying for a real probe — all five deps sites now stub resolveCaptureDisplay. The signal test asserts the caller's signal object reaches runXcrun by identity, which is what was missing. And the dynamic import('./core/simctl.ts') in the facade is gone, since that module was already a static import.

Probe per capture — deliberately left uncached, and I would like to keep it that way. The panel count is static, but the thing the resolver returns is the lit panel, and folding the device is exactly what changes it; a cache keyed on the device would resume capturing the dark panel, which is the failure this PR exists to fix. The cost is measured and documented at 0.16–0.26s against a 4.93s screenshot under a 5s budget. If you want the single-panel case cheap, the honest place for that is a device-type capability check, not a cache.

pnpm check and pnpm check:affected --run both pass on 616e8439.

@thymikee

Copy link
Copy Markdown
Member Author

Correction on the recording I attached earlier: that clip only exercised scroll on a screen that had nothing to scroll, and it was recorded with --hide-touches, so it showed a static Safari window and proved capture but not interaction. Re-recorded with an interaction that is visible on screen.

What happens in this clip, on the open Duo with only the inner panel lit (LCD off, LCD-1 activeOn, capture at 2006x2852):

  1. tap @e4 on the address field — focus and keyboard appear
  2. type " macbook" — the appended characters are visible in the field and in the suggestion row
  3. keyboard enter submits the highlighted suggestion
  4. the page loads: address bar reads google.com, search results render, then scroll down moves the document

Snapshot node count goes from 9 visible (address overlay) to 19 visible (loaded page with browser chrome), and the recorded mean luma falls from 241.95 to 194.06 across the clip, so the frames are the interaction rather than one frozen screen.

Two honest notes about what this clip does not show:

  • No touch indicators. The overlay exporter cannot burn touches into this track: recording-overlay.swift emits 480x336 black, or 0x0, for a rot90 inner-panel track, while the same overlay path works on non-rotated panels. That is the defect I flagged in the PR body and it is independent of this change — the raw simctl capture honors --display correctly, which is what this PR fixes, and the video above is --hide-touches. The interaction is legible here only because the effects are on screen.
  • fill on Safari's address field fails with no text input found at the provided coordinates to clear, and tap on a suggestion cell behind the keyboard is refused with tap_keyboard_occludes_target. Both behaved as designed (the second is the occlusion guard doing its job, and keyboard enter is the documented way to submit) — noting them because I worked around both to get this clip.
duo-open-navigation.mp4

nav-80

@thymikee

Copy link
Copy Markdown
Member Author

Reviewed at 616e843. The lit-panel fix looks correct, and the delta answers both blocking items from the earlier round: runIosDevicectlJsonRequest is now the only place that builds the --json-output argv, reads the file and cleans it up, and the recording numbers cover --display on the real toolchain. No blocking issues this time.

Two things the delta itself introduced. In devicectl.ts:210 runXcrun is awaited before the try, so a probe timeout or an abort skips the finally that unlinks the temp file. The old copy in display-inventory.ts wrapped the call, so the one path this PR adds to every capture is exactly the path that lost its cleanup. And in display-inventory.ts:105 the detail picks result.stderr whenever result exists, but an unreadable-json outcome also carries a result with empty stderr, so the parse error in cause is dropped and the least diagnosable reason ships with no detail at all. Both are small and neither blocks, but they are worth one more commit.

Take or leave the rest: five deps sites in screenshot.test.ts still pass deps: { normalizeDensity } without stubbing resolveCaptureDisplay, so those tests can still reach the live probe — mocking the module once would state that invariant where the sites are enumerated instead of per call; the renamed apple_display_primary_geometry_conflict check has no test asserting its phase, so a later rename or an inverted comparison could not go red; and cli-help.ts:744 gives the open inner panel as 669x951 points while the ADR records the live capture as 951x669 at 1x — is the help line transposed?

Smoke Tests is still running at this head with no failure logged. It does exercise the changed route, since every simulator screenshot now runs the panel probe, so it is worth watching rather than assuming; the previous head was green on that same route. I did not wait for the run to finish and did not reproduce the Duo numbers, which are taken as reported.

Once Smoke Tests reports green, this is ready to land.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 20, 2026
@thymikee
thymikee merged commit 057dfb7 into main Sep 20, 2026
19 of 20 checks passed
@thymikee
thymikee deleted the t3code/ed357be5 branch September 20, 2026 11:11
@github-actions

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-20 11:11 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant