Skip to content

feat(ios): add observe-only snapshots without foreground repair - #3108

Merged
thymikee merged 2 commits into
callstack:mainfrom
AdzeB:upstream/ios-observe-only
Oct 8, 2026
Merged

thymikee merged 2 commits into
callstack:mainfrom
AdzeB:upstream/ios-observe-only

Conversation

@AdzeB

@AdzeB AdzeB commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

What changed

Adds snapshot --observe-only for an existing local iOS app session. Unlike ordinary snapshots, this mode refuses rather than activating the app or repairing foreground state. Default snapshot repair and targetActivation disclosure are unchanged.

The runner must already be ready. A non-activating capability probe pins the subsequent capture to that exact runner session; missing support, a replaced session or missing/contradictory observation metadata fails closed. There is no runner startup/recycle, activating fallback or system-surface substitution in this mode. Other platforms, provider-owned sessions and diff are rejected.

Successful data.observation is exactly:

{
  "mode": "observe-only",
  "activationPerformed": false,
  "appState": "runningForeground",
  "appStateSource": "xcuiapplication-state"
}

This is sourced XCTest state, not verified foreground or screen ownership. The native path requires the foreground state report before/after capture and an unchanged process identity. Observe-only never emits targetActivation. Native unavailability is OBSERVATION_UNAVAILABLE; host/capability refusals use COMMAND_FAILED with details.reason: "observation-unavailable".

Review notes

This is the opt-in prevent complement to #2682's disclose behavior. It does not fix #2694's ordinary interaction disclosure gap or infer ownership from AX PID ordering. #2696 remains the reason not to treat XCTest state as a screen-ownership guarantee. Stronger ownership, observe-only settle, early response history, inline screenshot reuse, private Android comparison and prerelease version codes remain separate follow-ups.

Validation

Original feature validation on 40d0b191c01c8b458f41817552cc9a22d2ebd5c2:

  • corepack pnpm check passed: 1,457 Vitest files / 11,973 tests passed, one existing skip, plus the tooling/fallow checks.
  • corepack pnpm check:affected --run passed its local selected set. Selection included Apple/XCTest source checks; GitHub-authoritative and parked device lanes were explicitly skipped by the selector, not counted as local passes.
  • corepack pnpm build:xcuitest:ios and corepack pnpm build:xcuitest:macos passed, using isolated derived-data paths. The iOS unit-test-enabled build also passed.
  • Four selected native iOS tests passed: background refusal without activation/disclosure, sourced foreground metadata without target binding/activation, unchanged ordinary repair/disclosure, and capability reporting. Recorded in an .xcresult bundle.
  • Capability/identity regressions failed before the continuity fix and passed afterward: absent capability identity is refused before snapshot dispatch, replacement runners cannot replace the pinned identity, and unavailable ready registrations cannot create a runner session.
  • Live iPhone 17 Pro / iOS 26.5 simulator, with freshly built CLI and runner: Settings observe-only capture → Home → typed observe-only refusal → ordinary raw snapshot's repair/disclosure → successful observe-only capture. A brief foreground state report after Home was retained as sourced state, not promoted to ownership evidence. The repeatable CLI check and compact JSON artifact passed; session, daemon and disposable simulator were cleaned up.

Protocol fixture follow-up and final CI

The initial macOS host gate found that Command.observeOnly had no production request in the protocol fixture. Added real interactor producer sites for both the non-activating uptime probe and observe-only snapshot. Fixture rows come from the producer helper's captured-request diff; there is no separate generator. The drive also asserts the host-only expectedRunnerSessionId, which is not sent as a Swift request field. The other new Swift fields are responses/context, not missing request vocabulary.

Fix commit: f14ab3589. Final pushed head: d8484932c75a460a5c5a2798b980b86aacb71e15; the latter is a CI-only empty retry with the identical source tree.

  • Focused producer tests: 5 passed across 3 files.
  • Exact macOS workflow XCTest invocation (platform=macOS,arch=arm64, skipping only RunnerTests/testCommand): 279 passed locally; the source-derived execution-count reporter also passed. Local ad-hoc signing resolved the original unsigned-bootstrap limitation.
  • corepack pnpm check: 11,973 passed, one existing skip.
  • corepack pnpm check:affected --run: local selected gates passed on the final pushed head.
  • All PR-attached checks finished: 13 passed, 3 intentionally skipped, none pending or failing. This includes all four platform smoke jobs and Coverage.

An untouched iOS synthesized-text pacing assertion failed once. The same implementation/test passed on recent main run 36887001093; the identical-source retry run 36897563973 passed that test and all 79 targeted iOS XCTests. No typing behavior/test changes were made. GitHub required repository admin rights for a job rerun, hence the CI-only empty commit.

Limitation: no physical-device run.

Refs #3106

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 28 files

Tip: instead of fixing issues one by one fix them all with cubic

Re-trigger cubic

Comment thread src/daemon/snapshot-runtime-binding.ts
Comment thread src/daemon/snapshot-runtime-binding.ts Outdated
Comment thread packages/platform-apple/src/runner/runner-lifecycle.ts Outdated
Comment thread packages/platform-apple/src/interactor.ts Outdated
Comment thread website/docs/docs/snapshots.md
Comment thread packages/platform-apple/src/__tests__/interactor-target-activation.test.ts Outdated
Comment thread packages/platform-apple/src/interactor.ts Outdated
@thymikee

thymikee commented Oct 1, 2026

Copy link
Copy Markdown
Member

At 40d0b19, the Smoke Tests failure is caused by this PR, and the live simulator evidence is still missing. The new Command.observeOnly field has no production-request row, so RunnerTests.testEveryRunnerRequestFieldHasAProductionRequest fails with ["observeOnly"]. The other Smoke jobs and Coverage were still running, so I do not attribute them.

The PR body describes a live simulator sequence on the exact route, but it attaches no output (https://github.com/callstack/agent-device/blob/40d0b19/website/docs/docs/snapshots.md#L47). Without it, nobody can check that the refusal left the app in the background, or that the runner session id stayed the same across probe, capture and refusal. Please attach the run's JSON or daemon-log output. It should show: (1) an observe-only success with data.observation; (2) after Home, an observe-only refusal with error.code OBSERVATION_UNAVAILABLE and details.runnerErrorCode; (3) a non-activating read right after the refusal (screenshot or appstate) showing the app still in the background; (4) no ios_runner_session_startup or restart diagnostics, and one runnerSessionId across all observe-only calls; (5) the not-ready-runner refusal with reason observation-unavailable and no runner started.

To fix CI, please capture both observe-only requests (the uptime probe and the snapshot) in the interactor producer test through assertProducedRunnerRequests, and add the produced rows to https://github.com/callstack/agent-device/blob/40d0b19/contracts/fixtures/runner-requests.json. Once Smoke Tests are green and the live artifact is attached, I will look again.

Not blocking, and you can take or leave these: (a) the success rule should be that the observation proves a non-nil pid at preparation and the same pid after capture, since RunnerTests.processIdentifier(of:) returns nil for pid <= 0 and nil == nil passes a relaunched process as unchanged (RunnerTests+SnapshotExecution.swift#L121), so refuse with OBSERVATION_UNAVAILABLE when the pid is nil; (b) enforce "observeOnly implies result.observation present and targetActivation absent" once in captureRuntimeSnapshot, and add a daemon-route test with an eligible bridge route asserting data.observation, because the bypass in runtime-snapshot.ts#L31, the daemon guard in snapshot-runtime-binding.ts#L139, the diff refusal and the observation response-views key have no test today; (c) a ready runner with a stale artifact is stopped by resolveReusableRunnerSession during the uptime probe, so the refusal carries runner_session_ownership_changed instead of the documented observation-unavailable, so map it to the documented reason (keeping the original as a detail) or document both; (d) the ready-only and pin policy on the uptime probe is a host decision, so it fits better as a host option in AppleRunnerCommandOptions than as a RunnerCommand wire field the runner ignores (runner-lifecycle.ts#L277).

Could admission become one declared snapshot requirement (for example captureSnapshotWithoutActivation) in snapshotRuntimeOperationFacts, picked by resolveSnapshotRuntimePlan? Platform admission is now re-derived in the daemon binding, in contracts assertSnapshotObservationOwner, in interactor.ts#L215 and in the Swift #if os(iOS) branch, and one declared fact would delete those guards and the runtime-snapshot bypass. The reuse check already stops stale-artifact runners, so is the capability probe reachable outside externally supplied or adopted runners? If not, refusing on an externally supplied runner artifact could replace it and drop the extra round trip per snapshot. No ADR is needed. Extending the snapshot runtime-fact owner in packages/contracts/src/snapshot-runtime.ts and each platform's facts first would be the order to do it in.

I did not run the Swift or Vitest suites, and I did not trace adopted runners from other versions.

@thymikee

thymikee commented Oct 1, 2026

Copy link
Copy Markdown
Member

The earlier gaps are mostly closed at d848493, but one blocker remains. The missing observeOnly rows are now in contracts/fixtures/runner-requests.json, which is the fixture the Swift test testEveryRunnerRequestFieldHasAProductionRequest reads, so that check should pass now. I have not confirmed this, because Smoke Tests were still running when I looked.

The remaining problem is that the observe-only path has no live evidence. This path adds a Swift runner command field and a TS runner-lifecycle gate. It runs an uptime probe, then a snapshot pinned to the probed runnerSessionId, and it promises not to activate the app. Every test uses a fake runner, so nothing shows the real XCTest runner keeping that promise, advertising supportsObserveOnlySnapshot, or returning observation without targetActivation. If the runner drifts from the TS side, the command either fails with OBSERVATION_UNAVAILABLE on every call or activates the app despite observeOnly, and the fake-runner tests catch neither. Could you attach live iOS simulator output from d848493, run with the repo CLI? It should show three things. First, with a warm ready runner session and the app backgrounded, snapshot --observe-only succeeds with observation.mode observe-only, activationPerformed false and no targetActivation, and the app stays backgrounded. The --debug ndjson for that run should show exactly two runner requests, uptime then snapshot, with the same runnerSessionId. Second, with no ready runner session, the command should fail with reason observation-unavailable and dispatched no, and it should create no session or app launch. Third, a normal snapshot of the same app should still activate it and carry targetActivation.

I did not run the Swift RunnerTests or the TS runner-requests tests. The fixture conclusion comes from reading the code. After you attach the output, please also wait for Smoke Tests to finish green.

@AdzeB

AdzeB commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

Fixed the macOS request-vocabulary failure in f14ab3589: producer sites now drive the real observe-only snapshot path, capturing both its uptime capability probe and snapshot. The golden rows are copied from the existing helper's captured-request diff (there is no generator). expectedRunnerSessionId is host-only and now asserted on the snapshot call options; no extra Swift wire field is introduced.

Local before/after proof: the exact macOS workflow XCTest invocation went from 279 tests / 1 missing-observeOnly failure to 279 / 0 failures, with the source-derived execution-count report passing. Five producer tests and corepack pnpm check passed (11,973 tests, one existing skip). Local ad-hoc signing resolved the prior bootstrap problem. Exact-head affected checks passed.

All checks on final head d8484932c have finished: 13 passed, 3 intentionally skipped, no failures/pending checks, including macOS/iOS/Android/Linux Smoke Tests and Coverage.

One unchanged synthesized-text pacing assertion failed in iOS run 36894206456. The same implementation/test passed on recent main run 36887001093, and the content-identical retry run 36897563973 passed that assertion and all 79 targeted iOS XCTests. No typing implementation or test was changed. The failed-job rerun API required admin rights; d8484932c is a CI-only empty commit with the same tree as f14ab3589.

@thymikee

thymikee commented Oct 4, 2026

Copy link
Copy Markdown
Member

At d848493 this branch now conflicts with main after today's merges. Please rebase onto main. The earlier review findings still apply; I will review the rebased head.

@AdzeB

AdzeB commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

@thymikee no problem

@AdzeB
AdzeB force-pushed the upstream/ios-observe-only branch from d848493 to 5a3ea91 Compare October 5, 2026 08:26
@thymikee

thymikee commented Oct 5, 2026

Copy link
Copy Markdown
Member

The runner-side gap from the earlier review at d848493 is fixed at 5a3ea91. The observe-only context now returns after the SpringBoard and system-surface routing, so that thread no longer applies. One finding remains, and it is about missing live evidence.

The live iOS simulator output asked for in the earlier review is still not attached. The TS tests use a fake runnerProvider that supplies runnerSessionId and the observation itself (interactor-target-activation.test.ts:58-85). So nothing shows that the real XCTest runner advertises supportsObserveOnlySnapshot, keeps a backgrounded app in the background, omits targetActivation, and holds the host pin across the uptime probe and the snapshot (interactor.ts:288). If the TS and Swift sides drift apart, every call fails with OBSERVATION_UNAVAILABLE, or the app activates despite --observe-only, and the fake-runner tests catch neither. Please run the repo CLI at 5a3ea91 on an iOS simulator with --debug and attach the JSON and ndjson. The output must show four things. (1) With a warm ready runner and the app in the foreground, snapshot --observe-only --json succeeds with data.observation {mode: observe-only, activationPerformed: false} and no targetActivation. The ndjson must have exactly two runner requests, uptime then snapshot, sharing one runnerSessionId, with no ios_runner_session_startup or restart lines. (2) After Home, the command fails with OBSERVATION_UNAVAILABLE and details.runnerErrorCode, and a following appstate still reports background. (3) With no ready runner, the command fails with reason observation-unavailable and dispatched no, and it starts no runner and launches no app. (4) A normal snapshot of the same app still activates it and carries targetActivation.

Of the open bot threads, the P1 on a pid that is nil on both sides (r4157930686) and the P2 on iPad refusal (r4157930727) still apply, as do the P2 on the progress-model label (r4157930719) and two P3 threads. Seven threads do not apply, so please resolve them. r4157930638 and r4157930705 cover admission guards for unsupported cases, which are a different class from unavailable observation and are documented as unsupported. r4157930673 fails closed after one probe. r4157930695 belongs to the existing session pin, and any runner that receives the request still gets observeOnly:true. r4157930733 is fine because the bundle id comes from the session. r4157930755 is fine because the snapshot never reached the runner. r4157930930 is the P1 on the system-surface routing that the new head fixes.

I read the code at 5a3ea91 and did not run the Swift RunnerTests or Vitest. I did not trace which provider runtimes bind through bindAppleSnapshotRuntime with an injected transport. The provider fail-closed conclusion rests on runnerSessionId being injected only by the local executeRunnerCommand.

All 16 checks pass at 5a3ea91, including the iOS smoke and macOS XCTest lanes that exercise the changed runner requests. Before merge, please attach the simulator output described above.

@thymikee

thymikee commented Oct 6, 2026

Copy link
Copy Markdown
Member

Live simulator evidence for snapshot --observe-only

  • Head: 5a3ea91fcbed52fc26157f14b233c8ec3e36a576. I built the CLI and the runner from this commit.
  • Device: new iPhone 17 Pro simulator, iOS 26.2 (23C54), Xcode 26.2.
  • App: Settings (com.apple.Preferences). The Settings process kept pid 60300 during all steps.
  • All commands used --platform ios --udid <sim> --debug --json and a private state dir.

Order: open → snapshot -i (warms the runner) → (1) → home → (2) → (4) → stop the runner → (3).

(1) Warm runner, app in foreground: pass, with one note

snapshot --observe-only succeeds. targetActivation is absent.

{"success": true, "data": {
  "observation": {"mode": "observe-only", "activationPerformed": false,
                  "appState": "runningForeground", "appStateSource": "xcuiapplication-state"},
  "targetActivation": "absent", "nodeCount": 73}}

Request ndjson (trimmed):

ios_runner_session_reuse          sessionId 9BF0A50C-…:56118:1791292598128  ready true
ios_runner_command_send           command uptime    (23 ms)
ios_runner_session_reuse          sessionId 9BF0A50C-…:56118:1791292598128  ready true
ios_runner_readiness_preflight    command snapshot  reason conservative_command  (5 ms)
ios_runner_command_send           command snapshot  (309 ms)
snapshot_capture                  backend xctest
request_success
  • There are exactly two ios_runner_command_send lines: uptime, then snapshot.
  • Both use the same runner session id.
  • The request has no ios_runner_session_startup, restart, or invalidation line.
  • Note: before the snapshot, the existing readiness preflight sends one more plain uptime to the runner. Thus the runner gets three HTTP requests: observe-only uptime, preflight uptime, and snapshot. The preflight uptime does not activate the app. This PR does not add the preflight. But the result is not "exactly two runner requests" if you count HTTP requests. Please decide if the observe-only path must skip the preflight.

(2) After Home: pass

snapshot --observe-only fails. The runner refuses with no activation.

{"success": false, "error": {"code": "OBSERVATION_UNAVAILABLE",
  "message": "The session app cannot be observed without activation.",
  "details": {"runnerErrorCode": "OBSERVATION_UNAVAILABLE", "dispatched": "no"}}}

The next appstate reads the runner. The app is still in the background:

{"success": true, "data": {"appBundleId": "com.apple.Preferences", "source": "runner", "state": "runningBackground"}}

The ndjson has uptime, the readiness preflight, and snapshot. All use the same runner session. It has no startup or restart line.

(3) No ready runner: pass

Setup: the session stays open with Settings in the foreground. I stopped only this simulator's runner xcodebuild process with kill -TERM. The runner app also exited on the simulator.

{"success": false, "error": {"code": "COMMAND_FAILED",
  "message": "observe-only snapshot requires an already-ready runner.",
  "details": {"reason": "observation-unavailable", "dispatched": "no"}}}
  • The request ndjson contains only request_start, two retry lines, and two request_failed lines, all within 3 ms. It has no runner startup, no xcodebuild exec, no simctl launch, and no ios_runner_command_send.
  • After the command, ps shows no xcodebuild process for this simulator. launchctl list on the simulator shows no runner app.
  • Settings has pid 60300 before and after the command. So the command did not launch the app again.
  • appstate before and after reports source: "session". With no live runner, appstate reads only the session record. Thus the pid gives the live proof, and appstate does not.

(4) Normal snapshot on the backgrounded app: pass

A normal snapshot after Home (and after the refusal in step 2) activates Settings and reports it:

{"targetActivation": {"reason": "stale_target", "priorState": "runningBackground", "otherActiveApplicationPid": 7621},
 "observation": "absent", "nodeCount": 72}

The next appstate (source runner) reports runningForeground.

Other observations (not caused by this PR)

  • Each runner-reported error, including the step (2) refusal, has details.xcodebuild: {"exitCode": 1, "stdout": "", "stderr": ""}. This is a placeholder from buildRunnerResponseError. It can make the refusal look like an xcodebuild failure.
  • The host was under heavy load (load average about 400). Because of this, the first open timed out at 90 s during the runner build, and I ran it again with --timeout.

Full evidence (JSON for each step, request ndjson, and the command log) is available on request.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 10 files (changes from recent commits).

Tip: Review your code locally with the cubic CLI to iterate faster.

Turn on auto-fix | Re-trigger cubic

Comment thread packages/platform-apple/src/interactor.ts Outdated
@thymikee

thymikee commented Oct 6, 2026

Copy link
Copy Markdown
Member

I pushed four commits on top of ad853d0 to finish this PR:

  • 01be1a3 merges main. The branch was 36 commits behind, and the Fallow audit then counted main's changed files as this PR's.
  • f008951 makes snapshotObservationPolicy return the pinned runner options. This keeps captureAppleRunnerSnapshot under the complexity threshold after the merge.
  • f9acee9 names iPadOS in the four observe-only refusal messages.
  • 1e75ce4 settles the readiness-preflight question from my live evidence. An observe-only snapshot is now preflight-exempt through its command traits. The observe-only uptime probe has already proved the pinned session ready, so the second uptime added a request but no evidence. On the healthy path the runner now gets exactly two requests: the probe, then the snapshot. A normal snapshot still preflights. The preflight was safe before this change, because a pinned session rethrows before any restart or recovery (runner-lifecycle.ts). Transport retries can still add requests on a failing path.

All review threads are resolved, and CI passes at 1e75ce4.

@thymikee

thymikee commented Oct 6, 2026

Copy link
Copy Markdown
Member

The code at 1e75ce4 fixes the earlier gaps, but the live evidence for the new pid check is still missing. All 16 checks pass at 1e75ce4, including the iOS smoke and macOS XCTest lanes that build the changed runner. Thanks for the quick follow-ups. The iPadOS wording in all four refusal messages, the nil-pid guard, the shared handheld-device check, the progress-model exclusion, the docs row and the named-command test assertions are now in place.

The success rule in RunnerTests+SnapshotExecution.swift changed in ad853d0. An observe-only capture now succeeds only if processIdentifier(of:) returns a non-nil pid for the app that resolveAppWithoutActivation built. The live run at 5a3ea91 used the old check, which also passed when both pids were nil, so it does not show that the real runner reads a pid on this path. 1e75ce4 also changes the request sequence, and only a fake HTTP server has exercised that. If the pid reads as nil for an app the runner did not launch, every observe-only snapshot of a foreground app fails with OBSERVATION_UNAVAILABLE, and no test in the repo would catch it. Please run the repo CLI built at 1e75ce4 on an iOS simulator with --debug and repeat only steps (1) and (2). (1) With a warm runner and the app in the foreground, snapshot --observe-only --json should succeed with data.observation showing mode observe-only and activationPerformed false, and no targetActivation. The request ndjson should show exactly two ios_runner_command_send lines, uptime then snapshot, on one runnerSessionId. The snapshot should have an ios_runner_readiness_preflight_skipped line with reason preflight_exempt_command, and there should be no ios_runner_readiness_preflight or startup line. (2) After Home, the command should still fail with OBSERVATION_UNAVAILABLE, and appstate should report runningBackground.

The six cubic-dev-ai threads are fixed at 1e75ce4, so the author can resolve them: iPadOS refusal messages, nil pid guard, handheld device check, progress model exclusion, observe-only docs row and named command assertions.

Not blocking, and fine to take or leave: the new iPadOS admission at snapshot-runtime-binding.ts and in packages/contracts/src/snapshot-runtime.ts has no test through those routes, so reverting either gate to appleOs === 'ios' would still pass the suite. One daemon-route test that snapshots with --observe-only on an IPADOS_SIMULATOR session and reaches the interactor would cover it. Also, isHandheldAppleSimulator could be written as kind === 'simulator' && isHandheldAppleDevice(device) instead of repeating the same os check.

I did not run Vitest or the Swift RunnerTests. The claim that the runner-exchange test fails on old code comes from reading resolveRunnerReadinessPreflightDecision. I also could not tell whether XCUIApplication.processID is non-zero for an app the runner resolved but did not launch. Nobody has run iPadOS live, so admission there rests on the #if os(iOS) runner branch and the unchanged state check. I reviewed only the four logical commits ad853d0, f008951, f9acee9 and 1e75ce4. The range also contains a main merge, and the earlier three commits are unchanged.

Before merge, the live simulator run of steps (1) and (2) above must show the new pid guard accepts a real foreground app.

@thymikee

thymikee commented Oct 7, 2026

Copy link
Copy Markdown
Member

I pushed e3a9b87c4cfca8b9b334b869cac250bad3c59178 on top of 1e75ce4 to slim the host side of this PR down to the runner primitive.

What changed. The capability probe (uptime with observeOnly), supportsObserveOnlySnapshot, the ready-runner-only rule, runner session pinning and the readiness-preflight exemption are gone. An observe-only snapshot now sends observeOnly: true on the snapshot request alone; the runner enforces the no-activation contract and returns observation, and the host asserts that shape on the response. That scaffolding existed to survive a CLI and runner that disagree about the field, which the repo ships together and already guards through testEveryRunnerRequestFieldHasAProductionRequest. The Swift side, the observation payload, the OBSERVATION_UNAVAILABLE refusal and the daemon/contract admission are unchanged. Net PR size drops from 36 files / +632 to 31 files / +402. Docs updated accordingly.

Validation at e3a9b87c4cfca8b9b334b869cac250bad3c59178.

  • pnpm check:affected --run: all runnable checks passed (1200 test files, 10751 tests). Fallow clean after extracting the runner snapshot request builder.
  • iOS runner built with unit tests on Xcode 27.1; on an iPhone 17 Pro simulator: testObserveOnlySnapshotRefusesBackgroundWithoutActivationOrDisclosure, testObserveOnlySnapshotReportsStateWithoutBindingOrActivatingTarget, testRegularSnapshotStillRepairsBackgroundAndDisclosesPriorState, testEveryRunnerRequestFieldHasAProductionRequest, testEveryRunnerCommandTypeHasAProductionRequest: 5 passed, 0 failures.
  • Live CLI on a disposable iPhone 17 Pro / iOS 27.0 simulator with Settings:
    1. Warm runner, app foreground: snapshot --observe-only --json succeeded with observation: {mode: observe-only, activationPerformed: false, appState: runningForeground, appStateSource: xcuiapplication-state} and no targetActivation. Request log: one ios_runner_session_reuse, the ordinary readiness preflight, one ios_runner_command_send snapshot, no startup or restart line.
    2. After home: three observe-only calls (immediate, +3 s, +debug) all failed with OBSERVATION_UNAVAILABLE, details.runnerErrorCode: OBSERVATION_UNAVAILABLE, dispatched: no; appstate afterwards reported runningBackground. In a separate run the call issued within a second of home succeeded once with a Settings tree while XCTest still reported foreground, which is the state lag iOS CI simulator: XCUIApplication.state reports runningForeground for the runner target while a foreign app is foreground #2696 and the docs already describe.
    3. Regular snapshot -i of the backgrounded app still activated it and carried targetActivation: {reason: stale_target, priorState: runningBackground, otherActiveApplicationPid}.
      Session, daemons and the disposable simulator were cleaned up.

Not run: physical device.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 14 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic

Comment thread website/docs/docs/snapshots.md Outdated
@thymikee

thymikee commented Oct 7, 2026

Copy link
Copy Markdown
Member

Thanks for the update. The two earlier concerns from 1e75ce4 are addressed in e3a9b87, and CI is green (16 of 16 checks). One live-validation gap remains.

The change at runner-lifecycle.ts:284 removes the guard that refused observe-only when the runner was not 'ready'. An observe-only snapshot now goes through executeRunnerCommandAttempt like any other command. With liveness 'gone' or 'stopped' it boots xcodebuild and the XCTest runner. With a warm runner whose uptime preflight fails, it takes the restart-and-replay path. Both routes run before the runner's no-activation check. The new docs say the screen is left exactly as it was, so this must hold on every route. If starting or recovering the runner changes the foreground app, the snapshot disturbs the screen, which is the #2682/#3106 behaviour this PR prevents. The runner would then refuse with OBSERVATION_UNAVAILABLE, so the disturbance would look like a normal refusal. I could not tell from the code whether the launch moves the foreground app, so this is an evidence gap and not a confirmed defect.

The rule to prove is that an observe-only request never changes the foreground app on any route that reaches the runner snapshot: reused runner, cold boot, and preflight-driven restart. The live run at e3a9b87 covered only the warm, reused runner. Could you run the CLI built at e3a9b87 on an iOS simulator with --debug, with the session app in the foreground and no runner process (stop the runner, or start a fresh daemon after open)? The output should show an ios_runner_session_startup line in the request ndjson, then one ios_runner_command_send snapshot. The call should succeed with data.observation of {mode: observe-only, activationPerformed: false, appState: runningForeground} and no targetActivation. A later appstate or screenshot should show the session app still in the foreground. If the boot moves the app, please restore the ready-runner refusal (or an equivalent no-boot rule) inside executeRunnerCommand, and say in snapshots.md that observe-only never starts a runner.

The cubic-dev-ai docs thread still applies: interactor.ts:293-296 still throws COMMAND_FAILED with reason 'observation-unavailable', and snapshots.md lines 55-57 do not name that refusal. See #3108 (comment).

I reviewed only the changes from 1e75ce4 to e3a9b87. I did not run Vitest, the Swift RunnerTests, or a simulator, so the live and Swift results come from the maintainer's comment. I also did not check whether a runner built by an older CLI can be reused after an upgrade. If it can, that runner ignores observeOnly and activates the app before the host refusal at interactor.ts:293. Before merge, we need the cold-boot simulator evidence above and the docs sentence from the cubic thread.

@thymikee

thymikee commented Oct 8, 2026

Copy link
Copy Markdown
Member

Pushed 16e58a0c896e28a10f4d0dd5ee4acff4b5bd8ce2 (docs only: the cubic thread's host-side refusal sentence). pnpm check:affected --run passed on it.

Cold-boot and restart evidence at e3a9b87 (same runner and host code as 16e58a0c896e28a10f4d0dd5ee4acff4b5bd8ce2), disposable iPhone 17 Pro / iOS 27.0 simulator, Settings open and in the foreground, repo CLI with --debug. Sessions live in the daemon, so daemon stop cannot leave a session without a runner; instead I terminated the runner host app on the simulator with simctl terminate <udid> com.callstack.agentdevice.runner.uitests.xctrunner.

Route B, preflight-driven restart (terminate, then snapshot --observe-only immediately). Request ndjson, in order:

ios_runner_session_reuse ready=true
ios_runner_readiness_preflight snapshot reason=conservative_command
ios_runner_session_invalidated reason=runner_connect_failed_before_command_send
ios_runner_session_startup
ios_runner_startup_launch_xcodebuild
ios_runner_readiness_preflight_skipped snapshot reason=read_only_startup_command
ios_runner_command_send snapshot
snapshot_capture
request_success

Response: success: true, data.observation: {mode: observe-only, activationPerformed: false, appState: runningForeground, appStateSource: xcuiapplication-state}, no targetActivation. The following appstate read runningForeground from the runner.

Route A (terminate, wait 8 s, then observe-only): the daemon had already recovered the session, so the request shows ios_runner_session_reuse, preflight_skipped reason=read_only_startup_command, one ios_runner_command_send snapshot, request_success, with the same observation payload and appstate runningForeground afterwards.

So a runner boot does not move the foreground app, and the ready-runner refusal stays out. The docs sentence "the screen is left exactly as it was" holds on the reused, restarted and freshly started routes.

On the older-runner question: resolveReusableRunnerSession compares the existing artifact's cache key with the expected one, and verifyLeaseArtifactFingerprint does the same for lease adoption. The cache key includes runnerSourceFingerprint, so a runner built from older sources is rebuilt rather than reused and cannot ignore observeOnly.

Simulator, daemons and state dirs were cleaned up.

@thymikee

thymikee commented Oct 8, 2026

Copy link
Copy Markdown
Member

This PR is ready at 16e58a0. The earlier findings from e3a9b87 are fixed: the docs at https://github.com/callstack/agent-device/blob/16e58a0/website/docs/docs/snapshots.md#L57-L59 now name the host COMMAND_FAILED refusal with details.reason 'observation-unavailable', thrown at https://github.com/callstack/agent-device/blob/16e58a0/packages/platform-apple/src/interactor.ts#L295-L297. The Swift code has not changed since e3a9b87.

Not blocking: isHandheldAppleDevice at https://github.com/callstack/agent-device/blob/16e58a0/packages/kernel/src/device.ts#L136 repeats the iOS/iPadOS check that isHandheldAppleSimulator and hasAppleActionButton already hold, so you could keep it as the single leaf predicate and have the other two delegate to it, or leave it as is.

The cubic-dev-ai thread on the snapshots.md docs (#3108 (comment)) no longer applies, because 16e58a0 now names the refusal. Please resolve it.

The iOS Smoke Tests job failed in testObserveOnlySnapshotRefusesBackgroundWithoutActivationOrDisclosure, a test this PR adds. The failure looks like setup flakiness, not a break in the refusal contract. The app.wait(for: .runningBackground, timeout: 10) at RunnerTests+CommandDispatchTests.swift:73 timed out after the Home press. The test then went on with the app still in the foreground, so the refusal path never ran. The Swift code is unchanged since e3a9b87, where all 16 checks passed, and the regular-snapshot test passed in the same job with the same Home press. This matches the Home-press state lag in #2696. I did not run Vitest, the Swift tests or a simulator myself. The cold-boot and restart evidence comes from the maintainer's comment on e3a9b87, which has the same code as this head. Physical devices and iPadOS have not been run live.

The second Smoke Tests job is still queued, and there are no conflicts. Before merge, the iOS Smoke Tests job must pass. Please rerun it, and guard the wait at line 73 (return XCTFail with a setup message, or retry the Home press) so a setup failure does not read as a refusal failure.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 8, 2026
@thymikee

thymikee commented Oct 8, 2026

Copy link
Copy Markdown
Member

This PR is ready at 16e58a0. The two issues from the earlier review (#3108 (comment)) are fixed: the snapshots doc now names COMMAND_FAILED with reason observation-unavailable, and the unsupported-target refusal now says "supported on iOS and iPadOS only".

Not blocking: the setup check at https://github.com/callstack/agent-device/blob/16e58a0/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+CommandDispatchTests.swift#L73 uses XCTAssertTrue(app.wait(for: .runningBackground, timeout: 10)), which does not stop the test. If Home does not land, the test goes on and fails at line 80 with a refusal-contract message when the real cause was setup. A guard ... else { return XCTFail("setup: Home did not background the target") } would fix it, and the same guard fits every Home-based test in that file, including testRegularSnapshotStillRepairsBackgroundAndDisclosesPriorState. You can take or leave this.

The two cubic-dev-ai threads no longer apply, so please resolve them: the snapshots.md note (#3108 (comment)) and the unsupported-refusal wording (#3108 (comment)).

The iOS Smoke Tests failure is likely tied to this PR. The failing test, testObserveOnlySnapshotRefusesBackgroundWithoutActivationOrDisclosure, is new here. It failed in its setup, not in the refusal path: the Home wait at line 73 timed out with the app still reported as not in the background. The same Swift code passed all checks at e3a9b87, and the sibling Home-press test passed in the same job. This points to simulator state, not the new behavior. No conflicts are known.

I did not run Vitest, the Swift RunnerTests, or a simulator. The cold-boot and restart evidence comes from the maintainer's run at e3a9b87, which has the same code as this head. I did not trace every lease-adoption path through verifyLeaseArtifactFingerprint for runners built by an older CLI. Physical devices and iPadOS have not been run live.

Before merge, guard the setup wait at line 73 and get a green iOS Smoke Tests run on the new head.

AdzeB and others added 2 commits October 8, 2026 15:22
Squashed from the PR callstack#3108 history through 1e75ce4 and rebased onto main.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
An observe-only snapshot now sends the `observeOnly` field on the snapshot
request alone. The runner enforces the no-activation contract and returns
`observation`; the host asserts that shape on the response. The separate
`uptime` capability probe, `supportsObserveOnlySnapshot`, the ready-runner-only
rule, runner session pinning and the readiness-preflight exemption existed to
survive a CLI and runner that disagree about the field, which the repo ships
together and already guards through the protocol fixture.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@thymikee

thymikee commented Oct 8, 2026

Copy link
Copy Markdown
Member

Rebased onto main at bf22464 (the branch was 34 commits behind and carried a merge commit). Head is now 10bc4f7e48286ffce64982a7ed9e27dbca12c2f6, two commits:

  • 55213fd38 feat(ios): add non-activating observe-only snapshots, the PR history through 1e75ce4 squashed under the original author.
  • 10bc4f7e48286ffce64982a7ed9e27dbca12c2f6 refactor(ios): drop the observe-only capability probe and runner pinning, including the docs sentence from the cubic thread.

The only conflict was website/docs/docs/snapshots.md against #3295's wording; resolved by taking main's text and re-inserting the flag row and the observe-only section. Diff against main: 31 files, +421/-23.

Validation at 10bc4f7e48286ffce64982a7ed9e27dbca12c2f6: pnpm check:affected --run passed (1218 test files, 10899 tests, fallow clean); iOS runner built with unit tests on Xcode 27.1 and the five runner tests (three observe-only dispatch tests plus both protocol fixture tests) passed on a disposable iPhone 17 Pro simulator, 0 failures. Runner and host code are unchanged from e3a9b87/16e58a0c apart from the rebase, so the live simulator evidence posted earlier still describes this head.

@thymikee
thymikee force-pushed the upstream/ios-observe-only branch from 16e58a0 to 10bc4f7 Compare October 8, 2026 13:30
@thymikee

thymikee commented Oct 8, 2026

Copy link
Copy Markdown
Member

The earlier conflict is gone, and the cold-boot, restart and warm evidence from e3a9b87 covers the runner-readiness change. One defect remains at 10bc4f7, and the checks look unrelated to this diff.

Observe-only snapshots still reach the SpringBoard system-modal probe in snapshotFast and snapshotRaw (line 289), both called from executeSnapshotPrepared. Suppose a permission prompt is over the session app. boundedBlockingSystemAlertSnapshot then returns SpringBoard's alert nodes instead of the app tree, with no systemSurface provenance. The post-capture check at RunnerTests+SnapshotExecution.swift:114-127 only re-reads app state and pid, which have not changed, so it stamps the payload as an observation of the session app. This contradicts the snapshots.md line saying observe-only does not substitute SpringBoard or a presented system surface. The earlier P1 thread on this path was resolved because routing returns early, but the substitution happens in the capture tier. The rule should be: a successful observe-only response contains only nodes captured from the observed session app, and every capture tier that can return a non-app payload must refuse with OBSERVATION_UNAVAILABLE when observation is set. Today that is the probe both entry points run, so please enforce it once in executeSnapshotPrepared, or pass the observation into both. Please add a Swift unit test that sets systemModalProbeOverrideForTesting to return a payload with the app in the foreground and expects OBSERVATION_UNAVAILABLE. It should fail on the current code. If you would rather return the alert, please drop the docs and PR-body sentence and stamp systemSurface provenance on that payload.

Not blocking, and you can take or leave these: the cliDetail text in src/commands/capture/snapshot.ts:79 still says "an already-ready compatible runner", which e3a9b87 removed, and the inputDescription at flag-definitions-workflow.ts:283 says "iOS only" although iPadOS is admitted.

On the open threads, the SpringBoard probe thread still applies. The snapshots.md refusal thread (r4211182132) is fixed at this head, because the section now names the COMMAND_FAILED observation-unavailable refusal; please resolve it. The interactor guard thread (r4199054861) is fixed too, because the guard is gone and the remaining refusals name iPadOS; please resolve it.

Coverage fails in packages/platform-apple/src/snapshot-source/lifecycle.test.ts, where a bridge-request-deadline timer fired. This PR touches no snapshot-source file, and observe-only bypasses the bridge route at runtime-snapshot.ts:43, so it looks like a timing flake and a re-run should clear it. There are no conflicts.

I did not run Vitest, the Swift tests or a simulator for this review. The live evidence is from the run at e3a9b87, and I checked that no upstream commit since then touches the snapshot route. I could not confirm live whether XCUIApplication.state stays runningForeground while a SpringBoard alert is up. If it reports another state, the post-capture check already refuses and this path is safe. Before merge, the capture tier needs to refuse the SpringBoard payload, or the docs need to change, with the Swift regression test in place.

@thymikee
thymikee merged commit a6efd55 into callstack:main Oct 8, 2026
21 of 22 checks passed
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.

iOS: decode the targetActivation stamp for commands that consume no capture (press <x> <y>, live @ref)

2 participants