Skip to content

fix(ios): report capture refusals as typed reasons in back-fallback and recording - #2766

Open
thymikee wants to merge 1 commit into
mainfrom
fix/ios-capture-failure-contracts-2728
Open

thymikee wants to merge 1 commit into
mainfrom
fix/ios-capture-failure-contracts-2728

Conversation

@thymikee

@thymikee thymikee commented Sep 22, 2026

Copy link
Copy Markdown
Member

Summary

iOS back fallback and record start now report a capture that did not happen as a typed reason instead of a false verdict, finishing the failure contracts #2741 left papered over (#2728).

  • In-app back coordinate fallback: a nil screenshot sample was read as "no visual change", and when the app resolved no window it sampled SpringBoard, so two identical home-screen captures reported in-app back control is not available after a real leading tap had already run. It now observes only the app's own resolved window and returns the display refusal as an unverified outcome carrying APP_SCREEN_*. An observable no-change is unchanged.
  • record start: every no-frame bootstrap collapsed to an untyped NSError; it now throws the shared APP_SCREEN_* reason, so a runtime with no resolvable window fails closed with a typed code.
  • A zero-pixel capture is refused at the capture type.

macOS/tvOS keep their host-display capture and generic bootstrap error. 6 Swift files, one host doc comment, one ADR paragraph. Builds on #2741; addresses #2728 (parent #2725).

Validation

Commit 7b5b5043, rebased on main (0.21.12).

  • pnpm check:affected --run: all runnable checks passed (Swift runner builds and replay lanes are GitHub/device-lane only).
  • build:xcuitest:ios** TEST BUILD SUCCEEDED **; check:packaged-runner-swift parses + line parity; xctest-selection 276 declared, 0 unreachable; typecheck, lint, format, check:layering, check:fallow clean.
  • New runner unit tests pin the observation split, the refusal-selection, and the typed-vs-generic error mapping. Mutation: revert the iOS typed throw to the old NSError and the mapping test fails.

Outstanding: live iPhone Duo re-verification of these paths (this host has no iPhone Duo / Xcode 27.1). A device handoff runbook is provided for the Duo lane before merge-ready.

@thymikee

Copy link
Copy Markdown
Member Author

Live iPhone Duo verification handoff — #2728 capture failure contracts

Toolchain: Xcode 27.1 beta + booted iPhone Duo (iOS 27.1), plus a single-panel iPhone (17/18 Pro) control. Build the runner at this PR head (cd758bd5bc).

0. Focused runner unit tests (fast, any booted iPhone)

pnpm install --frozen-lockfile && pnpm build && AGENT_DEVICE_XCUITEST_PLATFORM=ios pnpm build:xcuitest:ios** TEST BUILD SUCCEEDED **. Then run (same -only-testing:AgentDeviceRunnerUITests/RunnerTests/<name> style as #2741):

  • testNavigationVisualVerificationSeparatesNoChangeFromNoSample (changed)
  • testNavigationFallbackReportsTheRefusalItHitNotADefaultCode (new)
  • testRecordingBootstrapErrorKeepsTheTypedRefusalAndFallsBackGenerically (new)
  • testRecordingStartSurfacesACaptureRefusalAsATypedCode (new)
  • testScreenCaptureFailureCarriesTheBridgeReasonAsItsOwnCode (regression)

1. back coordinate fallback — three outcomes (Duo, open pose)

Bound to the fixture app (com.callstack.agentdevicelab) on a screen with no real back control so the fallback runs.

  • (a) Observable no-change — REGRESSION: leading tap navigates nowhere → back = in-app back control is not available. runner.log: AGENT_DEVICE_RUNNER_IN_APP_BACK_VISUAL_VERIFICATION beforeBytes=A afterBytes=B changed=no (A==B, both >0). Must be byte-identical to pre-change.
  • (b) Observable change — leading tap navigates → back ok, log ... changed=yes (A != B).
  • (c) Unobserved — NEW: bind the session, then xcrun simctl terminate <duo> <bundle> so the app resolves no window, run back. Expect NO SpringBoard sampling:
    • log AGENT_DEVICE_RUNNER_APP_SCREEN_UNRESOLVED stage=window frame=(0,0 0x0) raised=no resolved=no and ..._VISUAL_VERIFICATION beforeBytes=-1 afterBytes=-1 changed=unknown;
    • the failure carries details.runnerErrorCode: APP_SCREEN_WINDOW_UNRESOLVED with the "unknown outcome, not evidence that a back control is absent" message — not "in-app back control is not available".

2. record start typed refusal — NEW

Same bound-then-terminated session: agent-device record start /tmp/duo-2766.mp4 → failure with details.runnerErrorCode: APP_SCREEN_WINDOW_UNRESOLVED. Relaunch the app (window resolves) and start again → records normally and exports the lit panel (success path intact).

3. Pose matrix (open / half-open / closed)

Re-run 1(c) and 2 at each pose via agent-device fold open|half-open|closed, re-snapshotting after each fold. A refusal stays a typed refusal (never a black capture); any successful capture names the lit panel (LCD id 1 closed; LCD-1 id 3 open/half-open). Read the hinge angle back per pose.

4. Single-panel control

Same no-window back/record → same typed APP_SCREEN_WINDOW_UNRESOLVED; normal back/record start unchanged from main. AGENT_DEVICE_RUNNER_SCREEN_CAPTURE display=1 pixels=WxH pixelsPerPoint=3 unchanged.

5. macOS/tvOS regression (no Duo needed)

AGENT_DEVICE_XCUITEST_PLATFORM=macos pnpm build:xcuitest:macos → SUCCEEDED (already on this head). A macOS record start with no host frame must still fail with the generic untyped record error, never an APP_SCREEN_* code.

Report back

Runner commit SHA built, per-pose hinge angle, the four log lines above, and the details.runnerErrorCode from each failing response. Close the session and stop the daemon afterward.

Mutation sanity: reverting the #if os(iOS) typed throw in RunnerTests+ScreenRecorder.swift back to the plain NSError makes testRecordingStartSurfacesACaptureRefusalAsATypedCode's sibling expectations fail; reverting navigationVisualObservation's guard … else .unobserved to false makes testNavigationVisualVerificationSeparatesNoChangeFromNoSample fail.

@github-actions

github-actions Bot commented Sep 22, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.78 MB 4.78 MB +3.9 kB
Package (unpacked) 4.78 MB 4.78 MB +3.9 kB
Package (download) 1.43 MB 1.43 MB +1.1 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 27.8 ms 26.6 ms -1.2 ms
CLI --help 78.9 ms 77.7 ms -1.3 ms

…nd recording

#2741 routed screenshots through the resolved app display but left two
failure contracts papered over (#2728):

- The in-app `back` coordinate fallback read a nil screenshot sample as "no
  visual change" and sampled SpringBoard when the app resolved no window, so
  a two-home-screen comparison reported "in-app back control is not
  available" after a real leading tap had already run. The check now observes
  only the app's own resolved window and reports the display refusal as an
  unverified outcome; an observable no-change still reads as unavailable.
- `record start` collapsed every no-frame bootstrap into an untyped NSError.
  It now surfaces the shared APP_SCREEN_* reason, so a runtime with no
  resolvable window fails closed with a typed code.

Refuse a zero-pixel capture at the capture type so a required consumer cannot
mistake it for a usable frame. macOS/tvOS keep their host-display capture and
generic bootstrap error. Focused runner unit tests pin the observation split,
the refusal-selection, and the typed-vs-generic error mapping.
@thymikee
thymikee force-pushed the fix/ios-capture-failure-contracts-2728 branch from cd758bd to 7b5b504 Compare September 22, 2026 20:19
@thymikee

Copy link
Copy Markdown
Member Author

At 7b5b504, the typed refusals aren't pinned to the code paths that produce them. RunnerTests+RecordingTests.swift:527 calls the two static helpers directly, while ScreenRecorder.start's bootstrap loop (ScreenRecorder.swift:49-71) runs in no test — reverting line 70 to the old NSError keeps both tests green. The back fix is the same story: Navigation.swift:184 swaps in captureResolvedAppScreen, but restoring captureObservedScreen also leaves every test green, since the suite only covers the pure classifier and navigationFallbackErrorPayload. So both shipped behaviors — record start carrying a typed refusal, and back refusing to sample a second process — can regress under a green suite. Can you inject the capture/sample source the same way selectObservedScreenCapture is injected, call ScreenRecorder.start(capture: { .failure(.unresolvedWindow) }) under AGENT_DEVICE_RUNNER_UNIT_TESTS and assert the thrown error maps through recordingStartErrorPayload, and add a verifyNavigationFallbackOutcome test where an app-side .unresolvedWindow yields .unverified(APP_SCREEN_WINDOW_UNRESOLVED) without ever calling the system-surface closure?

There's no live run at this head. The handoff comment in RunnerTests+Navigation.swift:225 names a stale SHA (cd758bd), and its recipe — terminate the bound app, then run back or record start — can't reach the new paths: back, backInApp and recordStart are non-lifecycle commands, so prepareActiveCommandContext relaunches the bound app first, and even without the relaunch, resolveRunnerWindow falls back to app.frame (0x0), so the tap returns .unavailable before any capture happens; record start still uses captureObservedScreen, which just captures SpringBoard instead of throwing APP_SCREEN_WINDOW_UNRESOLVED. Following that runbook on a device either looks like the PR failed (old "not available" outcome, successful recording) or passes without touching the changed route. What route or fixture reaches the .unobserved branch through production code — for example a top-leading tap that backgrounds the app, or a Duo pose where the window names no display — and what does runner.log show, specifically AGENT_DEVICE_RUNNER_IN_APP_BACK_VISUAL_VERIFICATION ... changed=unknown with details.runnerErrorCode set to an APP_SCREEN_* code? A single-panel control run showing changed=no and changed=yes matching main would confirm nothing else moved. For record start, either show a real state that produces a typed code on-device, or say plainly that its coverage is unit-only.

Is there a smaller shape here? Net production is around 235 lines, well under the usual size line, but it spans the runner and the TS contract doc and adds three new types. NavigationVisualSample duplicates Result<Data, RunnerAppScreenCaptureFailure> — could it carry that Result directly and derive the code and hint from the failure, dropping the struct and the nil-code default branch? The recorder closure only reads image and pixel size, so could it take Result<RunnerImage, RunnerAppScreenCaptureFailure> instead, which would also remove the macOS placeholder CapturedAppScreen (displayID 0, pixelsPerPoint 1) at Lifecycle.swift:84 and the invalid state it represents? Both would be local to the runner, so nothing else needs to change first.

Not blocking: the SpringBoard fallback in the ADR (docs/adr/0025-foldable-apple-panels.md:116) captures the whole display rather than SpringBoard's own window, which was already true at base on single-panel devices and only surfaces on a Duo whose SpringBoard sits on the dark panel, so the fix could either constrain the fallback to when it provably names the app's display or just correct the ADR wording, and separately, when the before-sample resolves but the leading tap backgrounds or terminates the app, head now returns .unverified/APP_SCREEN_WINDOW_UNRESOLVED where base fell back to SpringBoard and returned ok, so it's worth deciding and documenting whether that case counts as a change; both can be taken or left.

I didn't build or run the Swift runner — the claim that the current tests can't catch these mutations comes from reading the test bodies, not from executing one. I also didn't measure what XCUIApplication.frame returns for a terminated app; the preflight relaunch makes that moot on these two routes anyway. And I couldn't measure how often, on a single-panel device, windows.firstMatch misses while resolveRunnerWindow still finds a usable frame.

CI is green across all 18 checks, but the Swift runner's device lanes don't exercise the live back or record-start refusal routes, so that green doesn't cover the behavior this PR changes. Pin both fixes with tests that go through ScreenRecorder.start and an injected back-fallback sample source, then run live on a route that reaches changed=unknown with details.runnerErrorCode set to an APP_SCREEN_* code, plus the single-panel control, before this is ready to merge.

This branch has not been deployed

No deployments
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.

1 participant