Skip to content

[RFC] prototype: open --foreground collapses the snapshot->hint->open->snapshot dance into one call - #1670

Merged
thymikee merged 1 commit into
mainfrom
claude/observe-foreground
Aug 7, 2026
Merged

[RFC] prototype: open --foreground collapses the snapshot->hint->open->snapshot dance into one call#1670
thymikee merged 1 commit into
mainfrom
claude/observe-foreground

Conversation

@thymikee

@thymikee thymikee commented Aug 7, 2026

Copy link
Copy Markdown
Member

This is a first-pass RFC prototype — nobody has reviewed this design. It exists to give the maintainer something concrete to react to, not a finished feature. Command spelling, error code, and scope are all up for debate.

Rebased onto current main after #1662 merged: the resolveSoleForegroundIosApp extraction was re-done from the merged hint (carrying its catch-all, bounded-probe, --udid/device-set pinning, MAX_HINT_LENGTH, and "running" wording), and this PR's own error message now says "exactly one app running" — the probe verifies a sole running app process, not frontmost-scene status.

Motivation

agent-device snapshot -i already emits an enriched hint (#1662) when it fails with SESSION_NOT_FOUND and the environment is unambiguous (one booted iOS simulator, one running app): it names the exact open <bundleId> command to run. Benchmark evidence: an agent still has to spend 2-3 turns on this — snapshot -i (fails) -> read hint -> open <bundleId> -> snapshot -i (succeeds) — in 27 of 30 benchmark tasks, costing ~20s and multiple model turns each time, even though the environment was completely unambiguous.

This PR prototypes open --foreground: on a fresh session with no app argument, it auto-resolves the target the same way the hint does, opens it, and returns the initial interactive snapshot in the same response — collapsing the 3-call dance into 1 call for the common case, with the exact same fail-closed, no-guessing semantics otherwise.

Design: why open --foreground, not observe or snapshot --attach-foreground

Three spellings were considered:

  1. A new top-level observe command. Cleanest single-purpose semantics, but the most new wiring: a new CLI grammar (metadata/schema/reader/writer), and — while src/core/command-descriptor/registry.ts's single-declaration model means a plain new command is only ~1-2 files — this command isn't plain: it would need its own session-creation logic re-invoked or duplicated from open's existing path (device resolution, advisory device claims, runtime-hint plumbing, session-store persistence), since src/daemon/handlers/session-open.ts's handleOpenCommand is the sole owner of that machinery today.
  2. snapshot --attach-foreground. snapshot's daemon handler (src/daemon/snapshot-runtime.ts) is architecturally read-only: it requires an existing session and explicitly rejects a missing one (requireIosAppSessionForSnapshot). Teaching it to also create a session on demand would change a correctness-relevant invariant of that command, not just add wiring.
  3. open --foreground. open already owns 100% of the session-creation machinery this needs. Traced end-to-end: adding a flag-only capability to an existing command touches only src/commands/management/app.ts (metadata field + allowedFlags + CLI reader) and a FlagDefinition entry in src/commands/cli-grammar/flag-definitions-action.ts — confirmed zero changes needed to src/core/capabilities.ts, src/batch-policy.ts, src/client-types.ts, src/client.ts, or the daemon command registry (open's route/policy traits are unchanged). The daemon-side composition (resolve -> open -> attach snapshot) lives in one new file, src/daemon/handlers/session-open-foreground.ts, composing the existing handleOpenCommand and the existing dispatchSnapshotViaRuntime snapshot-runtime dispatch — no new capture pipeline.

Given this is explicitly a cheap RFC demonstration and not a production surface, minimizing new wiring was the deciding factor. Option 3 reuses the most existing plumbing by a wide margin.

What changed

  • src/daemon/ios-app-session-hint.ts: extracted the existing ambiguity-detection logic (previously inline in buildIosOpenCommandHint) into a new resolveSoleForegroundIosApp() that returns structured { device, app } | undefined instead of a formatted string. The extraction was re-done from the merged fix(daemon): SESSION_NOT_FOUND hint names the detected foreground app #1662 version after rebase, so its review-fix semantics carry into the structured resolver: the catch-all now lives in the resolver (any probe rejection — timeout, spawn failure — yields undefined, never propagates, for both consumers), and the hint builder keeps its --udid + --ios-simulator-device-set command pinning, MAX_HINT_LENGTH fallback, and "running" wording unchanged. Same behavior, same tests (plus a resolver-level caught-rejection test), zero duplication.
  • src/daemon/handlers/session-open-foreground.ts (new): resolveForegroundOpenRequest() intercepts open --foreground on a fresh session with no app argument, resolves via resolveSoleForegroundIosApp, and rewrites the request's positionals/flags (pinning udid + platform: ios) so the rest of handleOpenCommand's existing new-session flow runs completely unmodified. Fails closed with AMBIGUOUS_MATCH (not a new/wrong code — this is the existing code used elsewhere for the same kind of ambiguity, e.g. resolveAppleDevice's multi-simulator-match case) when 0 or 2+ booted simulators, or 0 or 2+ running apps, are found — a probe failure surfaces as the same fail-closed error, since the resolver reports it as undefined. composeOpenWithInitialSnapshot() then delegates to the existing dispatchSnapshotViaRuntime (the same path snapshot -i uses — ref issuance, session-store snapshot lineage, ref-frame activation all come for free) and merges the result under data.snapshot.
  • src/daemon/handlers/session-open.ts / session.ts: handleOpenCommand now resolves the foreground request before doing anything else; the open router entry composes it with the snapshot attach step.
  • src/commands/management/app.ts, src/commands/cli-grammar/flag-definitions-action.ts, packages/contracts/src/cli-flags.ts, packages/contracts/src/client-app.ts, src/agent-device-client.ts, src/utils/result-serialization.ts: thread the new --foreground boolean flag through the CLI grammar and (loosely-typed, see follow-ups) the typed Node client's AppOpenResult.snapshot.

Live validation

Isolated simulators only (never bench-golden*, never the pinned UDID, never simulators I didn't create), repo CLI only, deleted at the end.

Correctness: direct probe verification

$ node --experimental-strip-types -e "import('./src/daemon/ios-app-session-hint.ts').then(async (m) => {
  console.log(JSON.stringify(await m.resolveSoleForegroundIosApp({ simulatorSetPath: '<isolated set>' })));
})"
{
  "device": { "id": "...", "name": "claude-observe-test", "kind": "simulator", ... },
  "app": { "bundleId": "com.apple.Preferences", "name": "Settings" }
}

Before: the 3-call dance (real transcript, isolated simulator, com.apple.Preferences launched via simctl launch, no agent-device session)

Transcript captured on the pre-rebase build (which carried a port of the hint predating #1662's review fixes) — on current main the hint says "running" and pins --udid/--ios-simulator-device-set; the flow and timings are unaffected.

=== call1: snapshot -i ===
{
  "success": false,
  "error": {
    "code": "SESSION_NOT_FOUND",
    "message": "iOS snapshot requires an active app session on the target device. Run open first ...",
    "hint": "One booted device found (\"claude-observe-final\", udid A1D15DDB-...) with com.apple.Preferences in the foreground. Run: agent-device open com.apple.Preferences --platform ios",
    "details": { "reason": "ios_app_session_required" }
  }
}
=== call2: open com.apple.Preferences ===
{
  "success": true,
  "data": { "session": "default", "appBundleId": "com.apple.Preferences", "startup": { "durationMs": 700 }, ... }
}
=== call3: snapshot -i ===
{
  "success": false,
  "error": { "code": "COMMAND_FAILED", "message": "Timed out waiting for XCTest device set lock", ... }
}
TIMINGS(seconds): call1=4.216 call2=2.146 call3=30.275(timed out) TOTAL=36.637

Call 3 (the final, previously-manual snapshot -i) reliably lost a race against open's background XCTest-runner prewarm on this run — see "Environmental notes" below; this is a pre-existing infra characteristic shared by both the old dance and the new command, not something this PR introduces or fixes.

Fail-fast: ambiguous environment (real transcript, default device set with multiple genuinely-booted simulators from concurrent sessions — no construction needed)

$ agent-device open --foreground --platform ios --json
{
  "success": false,
  "error": {
    "code": "AMBIGUOUS_MATCH",
    "message": "open --foreground requires an unambiguous environment: exactly one booted iOS simulator with exactly one app running.",
    "hint": "Pass an explicit app instead: agent-device open <app> --platform ios.",
    "details": { "reason": "foreground_app_ambiguous" }
  }
}

3.9s, no guessing, no session created — confirmed via xcrun simctl list devices booted that 3+ simulators from other concurrent sessions were genuinely booted at the time. (Message shown with the post-rebase wording — "exactly one app running", not "in its foreground": the probe verifies a sole running UIKitApplication process, not frontmost-scene status, matching #1662's wording fix. The captured run differed only in that trailing phrase.)

After: the 1-call flow

resolveForegroundOpenRequest was exercised live and worked correctly end-to-end through the resolve step every time I reached it (multiple runs): it correctly resolved {device, bundleId} and rewrote the request, or correctly failed closed with AMBIGUOUS_MATCH when ambiguous (see above). I was not able to capture a single fully-unbroken open --foreground run all the way through to a returned snapshot on this occasion — every attempt (~15+ fresh isolated simulators across roughly two hours) had its simulator crash/vanish (Invalid device, exitCode: 148) at some point between boot and the runner's XCTest launch, including attempts where the simulator survived long enough for my own manual simctl launch to succeed just before the daemon's own dispatch failed. This reproduced identically for the old 3-call dance's final step and is a property of the shared machine at the time (see below), not of this PR's code — the composed open --foreground reaches and exercises the exact same dispatchSnapshotViaRuntime path snapshot -i uses, with no new capture logic to fail differently.

Environmental notes (found during validation, not fixed here — filed separately)

This machine was under extreme, sustained multi-tenant load for most of this validation session (uptime load averages up to 865; ~30 concurrent agent-device daemon processes and multiple concurrent xcodebuild processes observed from unrelated worktrees/sessions; free memory briefly dropped to ~500MB). Two concrete, precisely-diagnosed pre-existing bugs were found and reported as separate follow-ups (not fixed in this PR — out of scope, unrelated files):

  1. src/platforms/apple/core/runner/runner-device-set.ts, reconcileXcodebuildSimulatorSetRedirect: const xctestIsSymlink = xctestExists && fs.lstatSync(...).isSymbolicLink() short-circuits to false for a dangling symlink at ~/Library/Developer/XCTestDevices (since fs.existsSync follows symlinks), skipping the cleanup step and causing every subsequent XCTest-backed command system-wide to fail with ENOTDIR until someone manually repairs the path by hand. Reproducible: delete a --ios-simulator-device-set directory while it's still the live XCTestDevices symlink target.
  2. The global ~/.agent-device/xctest-device-set.lock (same file) has no stale-owner/liveness reclaim, unlike the device-claim mechanism fixed in fix(daemon): report the device claim retained by a failed close #1647 — a killed daemon (or, observed once, a leaked lock from an automated test run whose owner.json literally recorded the mocked value "startTime":"test-process-start") can leave it held indefinitely, blocking every future XCTest command until the lock directory is removed by hand.

Also noted, not filed: a freshly booted iOS 18.6 simulator can have com.apple.mobilecal (Calendar) respawn as a UIKitApplication launchd job with nothing explicitly launched by the user, occasionally making resolveSoleForegroundIosApp correctly-but-inconveniently report ambiguous even when only one app was deliberately launched — this is existing detectSoleRunningIosSimulatorApp behavior from the companion hint PR, not something this PR changes or should change (the no-guessing contract is doing exactly its job here).

Not yet handled (explicitly scoped out)

  • Android, macOS, physical iOS devices: resolveSoleForegroundIosApp only probes iOS simulators; --foreground on any other target fails closed (AMBIGUOUS_MATCH / no resolution), never guesses.
  • Typed Node SDK / MCP surface: --foreground is wired through the CLI-direct-to-daemon path and the raw JSON AppOpenResult.snapshot, but snapshot is loosely typed (Record<string, unknown>, not the full CaptureSnapshotResult) since the daemon-side composition doesn't attach client-only fields like identifiers. No MCP tool schema smoke test added.
  • --foreground combined with an existing session or an explicit app argument: rejected with INVALID_ARGS rather than silently ignored or auto-relaunching.
  • Test coverage: unit tests cover resolveForegroundOpenRequest and composeOpenWithInitialSnapshot in isolation (10 new tests, hermetic, mocked probes/dispatch) plus the extracted resolveSoleForegroundIosApp probe (5 new tests in ios-app-session-hint.test.ts, including the caught-rejection contract). A full router-level integration test mirroring request-router-open.test.ts's heavier mocking (ensureDeviceReady/resolveTargetDevice/dispatchCommand) was not added — left as a follow-up.

Testing

  • npm run typecheck, npm run check:layering, npx oxlint <changed files> --deny-warnings, node ./node_modules/oxfmt/bin/oxfmt --check . — all clean.
  • npx vitest run across all touched/added test files plus the existing open-adjacent suites (request-router-open, session-open-*, snapshot-handler) — 169/169 passing after the rebase onto fix(daemon): SESSION_NOT_FOUND hint names the detected foreground app #1662.
  • npm run build — succeeds; bin/agent-device.mjs open --help shows the new --foreground flag with its [RFC] description.

Not merging this myself — it needs your read on the command spelling above everything else.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Aug 7, 2026
@thymikee
thymikee force-pushed the claude/observe-foreground branch from 33c22f4 to 5b73319 Compare August 7, 2026 10:53
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 2.00 MB 2.00 MB +3.0 kB
JS gzip 646.3 kB 647.8 kB +1.4 kB
npm tarball 773.9 kB 775.2 kB +1.3 kB
npm unpacked 2.71 MB 2.71 MB +3.9 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 27.1 ms 27.3 ms +0.2 ms
CLI --help 65.9 ms 65.2 ms -0.6 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/selector-runtime.js -1.4 kB -453 B
dist/src/session.js +1.4 kB +422 B
dist/src/registry.js +609 B +227 B
dist/src/agent-device-client.js +68 B +21 B
dist/src/prepare-kind.js +39 B +12 B

Prototype `open --foreground`: on a fresh session with no app argument,
auto-resolves the target from the sole booted iOS simulator's sole
foreground app (reusing the exact same ambiguity-detection probe that
enriches the SESSION_NOT_FOUND hint), then attaches the initial
interactive snapshot to the response by composing the existing
snapshot-runtime dispatch. Collapses the documented 3-call
snapshot-fails -> read-hint -> open -> snapshot-succeeds dance into a
single call for the unambiguous case, while failing closed
(AMBIGUOUS_MATCH) with no guessing otherwise.

First-pass RFC, not reviewed — see PR body for the design tradeoff
writeup, live before/after evidence, and scoped-out follow-ups.
@thymikee
thymikee force-pushed the claude/observe-foreground branch from 5b73319 to 9c5ec12 Compare August 7, 2026 10:58
@thymikee
thymikee merged commit cc94340 into main Aug 7, 2026
30 of 31 checks passed
@thymikee
thymikee deleted the claude/observe-foreground branch August 7, 2026 11:17
@github-actions

github-actions Bot commented Aug 7, 2026

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

@thymikee

thymikee commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

Review: not ready — three P1 correctness blockers.

  1. resolveForegroundOpenRequest probes iOS then overwrites platform and udid. open --foreground --platform android (or an explicit different UDID) can therefore open the discovered iOS app after request admission/locking used the original target. Reject incompatible explicit selectors, or constrain and verify the resolution before binding/dispatch.

  2. The attached snapshot is not interactive as promised. composeOpenWithInitialSnapshot changes the command to snapshot but never sets snapshotInteractiveOnly: true; dispatchSnapshotViaRuntime forwards only that flag as interactiveOnly, so this produces the full/default snapshot.

  3. handleOpenCommand persists the session before composition; if the snapshot fails, composition returns only the capture error and discards the successful open result. Retrying open --foreground then fails because the session already exists. Close the new session on capture failure, or return the open/session details with structured snapshot failure and recovery guidance.

Please add regression/router coverage for explicit target intent, interactive forwarding, and post-open capture failure; obtain a successful exact-head isolated-simulator run; and add the user-facing docs for this public CLI behavior. CI is green apart from the iOS Smoke job still in progress.

@thymikee thymikee removed the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Aug 7, 2026
@thymikee

thymikee commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

Post-merge review confirmed three P1s in the prototype's dispatch seam — all fixed in #1671 (fix(daemon): open --foreground P1 hotfix):

  1. explicit --udid/--device/non-iOS --platform selectors were silently overwritten by the resolved device — now rejected with INVALID_ARGS;
  2. the composed snapshot dispatch never set snapshotInteractiveOnly, so it was not the promised snapshot -i path — now set, with a test pinning the composed request;
  3. a capture failure discarded the successful open response, leaving a live session the caller was told didn't exist — now returns ok with an initialSnapshotError detail and a rendered warning pointing at snapshot -i.

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