fix(daemon): open --foreground P1 hotfix — selector rejection, interactive snapshot, capture-failure masking

[thymikee]

P1 hotfix for the open --foreground RFC prototype merged in #1670 — three defects confirmed by post-merge review, all at the src/daemon/handlers/session-open-foreground.ts dispatch seam.

1. Explicit device selectors were silently overwritten

resolveForegroundOpenRequest rewrote the request with { ...req.flags, udid: resolved.device.id, platform: 'ios' }, clobbering an explicit --udid/--device and overriding --platform: open --foreground --udid B with simulator A sole-booted silently opened A. Now fails fast — matching the existing app-positional rejection:

• --udid or --device present → INVALID_ARGS ("open --foreground resolves the device itself; drop --udid/--device or open explicitly")
• --platform present and not ios → INVALID_ARGS
• an explicit --platform ios passes through (it states nothing the rewrite contradicts)

2. The promised interactive snapshot was never requested

composeOpenWithInitialSnapshot dispatched { ...req, command: 'snapshot', positionals: [] } with the open request's flags untouched — no snapshotInteractiveOnly, so the composed capture was not the snapshot -i path the doc comment promised. Verified the mapping before hardcoding: the CLI maps -i → snapshotInteractiveOnly (src/commands/cli-grammar/flag-definitions-workflow.ts), and src/daemon/snapshot-runtime.ts reads exactly req.flags?.snapshotInteractiveOnly as interactiveOnly. The composed request now sets snapshotInteractiveOnly: true, and a test asserts the composed dispatch carries it.

3. A capture failure masked the successful open

if (!snapshotResponse.ok) return snapshotResponse; discarded openResponse — but the session exists at that point, so the caller saw only a capture error, and retrying open --foreground then failed with "close the current session first". Now, open-success + snapshot-failure returns ok: true with:

• openResponse.data intact (session name, state dir, app identity)
• an explicit initialSnapshotError: { code, message } detail
• a rendered warning (appended to warnings, which openCliOutput prints): the session IS open — run agent-device snapshot -i

A capture error never masks a created session.

Testing

• 5 new/updated regression tests in src/daemon/handlers/__tests__/session-open-foreground.test.ts: explicit-selector rejection (--udid, --device, both, non-iOS --platform, and the --platform ios pass-through), the composed dispatch carrying snapshotInteractiveOnly: true, and the snapshot-failure path returning ok + warning + initialSnapshotError + usable session (15/15 in the file).
• Touched suites re-run: session-open-foreground, ios-app-session-hint, request-router-open, session-open-existing, snapshot-handler — 102/102 passing.
• npm run typecheck, npm run check:layering, npx oxlint <touched> --deny-warnings, npx oxfmt --check . — all clean.
• Live simulator validation skipped, deliberately: host load was 58 (1-min avg) with ~30 concurrent agent-device daemons at hotfix time — the same contention that destabilized simulators during [RFC] prototype: open --foreground collapses the snapshot->hint->open->snapshot dance into one call #1670's validation. All three fixes sit at the daemon dispatch seam and are fully exercised by the hermetic unit tests above; nothing here changes platform/probe behavior that would need a live device to observe.

[github-actions]

Size Report

Metric	Base	Current	Diff
JS raw	1.99 MB	1.99 MB	+1.2 kB
JS gzip	642.6 kB	643.0 kB	+400 B
npm tarball	770.6 kB	771.2 kB	+593 B
npm unpacked	2.70 MB	2.70 MB	+1.6 kB

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	26.7 ms	27.0 ms	+0.3 ms
CLI --help	66.2 ms	67.2 ms	+1.0 ms

Top changed chunks:

Chunk	Raw diff	Gzip diff
dist/src/session.js	+832 B	+240 B
dist/src/registry.js	+202 B	+77 B
dist/src/agent-device-client.js	+115 B	+62 B
dist/src/prepare-kind.js	+75 B	+21 B

[thymikee]

Reviewed exact head 2b878f8a: the three helper-level fixes are sound, but two public-boundary findings still block readiness.

• P1: default agent-device open --foreground still renders only the ordinary open text; openCliOutput never renders the nested snapshot. Node/--json preserve it, but the default one-call benchmark workflow still needs a second snapshot -i call. Add packaged production-route coverage proving the default CLI prints the interactive tree.
• P2: initialSnapshotError is daemon-only: it is absent from AppOpenResult, Node normalization, and serializeOpenResult, so public Node/CLI JSON drops the structured failure. The wrapper also truncates the normalized snapshot error to code/message, losing hint, details, diagnosticId, and logPath. Define/project the public field and preserve recovery/diagnostic metadata.

Because this is device-facing composition, attach an exact-head simulator run proving default text output, JSON output with usable refs, and capture-failure behavior with the session remaining usable. The helper mocks do not replace that production evidence. Completed checks are green; iOS Smoke is still pending.

[thymikee]

CI follow-up: the completed red iOS Smoke lane is failing RunnerTests.testBareTypeUsesTappedInputWhenSoftwareKeyboardIsHidden. That test exercises an unchanged Swift-runner path and has also failed intermittently on unrelated PRs, so I classify this as an unrelated lane flake requiring an owner rerun—not evidence against this hotfix. The P1/P2 public-boundary findings in the review above remain the merge blockers.

[thymikee]

Both public-boundary findings are fixed in 8e239d1 (worker-authored; committed by the coordinating session after the worker stalled twice mid-push):

• P1: openCliOutput now renders the composed interactive tree through the same snapshotCliOutput path snapshot -i uses, after the open confirmation — the one-call promise holds on default stdout. Production-route coverage added in output.test.ts (renders tree text + refs from an open result carrying snapshot data).
• P2: initialSnapshotError is now a public field on AppOpenResult, carried through Node normalization (normalizeInitialSnapshotError) and serializeOpenResult, preserving the FULL daemon error shape — hint, details, diagnosticId, logPath, plus the additive retriable/supportedOn signals. The daemon handler passes the whole error instead of a code+message truncation.

Live evidence, honestly scoped — this shared host currently has two booted simulators owned by other active sessions, which cannot be shut down, so the exact-head happy-path stdout transcript is not capturable here without violating the sole-booted-device precondition:

• Fail-closed on multi-boot (exact head, live): open --foreground → AMBIGUOUS_MATCH: … exactly one booted iOS simulator with exactly one app running with the explicit-open hint. ✔
• Set-scoped resolution (exact head, live): with --ios-simulator-device-set <own set> containing exactly one booted sim running Preferences, resolution succeeded and proceeded to runner launch — which then failed with Invalid device, a pre-existing XCTest-runner limitation with custom device sets, downstream of every line this PR changes.
• The default-stdout tree rendering is pinned by the production-route unit test; a quiet host (or CI e2e) will produce the visual transcript trivially — happy to re-run it here once the sibling sims are gone.

Touched suites pass (26 tests), typecheck + oxfmt clean. Evidence simulator and custom set deleted after use.

[thymikee]

Re-reviewed exact head 8e239d1f. The two original implementation findings are fixed: default open --foreground stdout now renders the composed tree through snapshotCliOutput, and the full structured capture error survives the handler, serializer, and Node normalizer. Two P2s remain. First, no createAgentDeviceClient().apps.open regression exercises initialSnapshotError; current tests stop at the handler/formatter, so deleting the new public-boundary normalization would leave them green. Add a client-route test that proves hint/details/diagnosticId/logPath survive. Second, normalizeInitialSnapshotError duplicates the existing daemon-error normalization (normalizeTargetShutdownError) and pushes the touched module over its quality threshold; consolidate the shared shape rather than shipping a second clone. Practical exact-head evidence should also cover successful default stdout and capture-failure-with-session-still-usable before readiness.

[thymikee]

Both P2s addressed in a517639 (branch also rebased onto current main):

• Consolidation: normalizeInitialSnapshotError and the target-shutdown error normalizer are now ONE internal normalizeDaemonError (table-driven, full shape including the additive retriable/supportedOn signals), used by both result paths and projected via normalizeOpenForegroundComposition. The fallow gate that failed on the previous head passes clean locally (0 findings in 36 changed files) — the duplicate-shape complexity was indeed the CI failure.
• Client-route regression: createAgentDeviceClient().apps.open now has two boundary tests — the full error shape (hint/details/diagnosticId/logPath/retriable) survives normalization verbatim, and a malformed error (missing message) is dropped rather than projected. Deleting the public-boundary normalization fails these tests.

Live exact-head evidence for default stdout + capture-failure remains blocked by the two sibling-session simulators still booted on this host (same constraint as the prior comment); the production formatter route is covered by output.test.ts in the meantime, and I'll post the transcripts as soon as the host allows a sole-booted-sim run.

[thymikee]

P1 — the capture-failure fix still misses the ordinary failure path. composeOpenWithInitialSnapshot only embeds a returned { ok: false } response, but dispatchSnapshotViaRuntime rethrows normal capture/runner exceptions after its Android-specific timeout handling. That rejection escapes through session.ts into the outer request router, so open --foreground still returns a failure after creating the session; retrying then wedges on the existing session — the exact condition this change is meant to remove.

Please catch and normalize that rejected capture into the same successful-open initialSnapshotError + warning response, and add a rejecting-mock regression alongside the returned-failure case.

Separately, this is iOS-facing dispatch/output behavior and has no exact-head live simulator evidence. After the fix, validate the one-call open, rendered interactive snapshot, and capture-failure recovery on the current head.

[thymikee]

Thrown-path P1 fixed in 95fd33e: the dispatch call is now wrapped, and a rejection is normalized via the kernel's normalizeError (the same conversion the request router applies) into the shared openWithInitialSnapshotFailure path — ok response, full-shape initialSnapshotError, session-usable warning. A rejecting-mock regression (AppError with diagnosticId) sits alongside the returned-failure case; both prove the open is never masked. Typecheck/fallow/oxfmt clean, 16/16 handler tests.

Live exact-head validation of the one-call open + rendered tree + capture-failure recovery remains queued behind the two booted sibling-session simulators on this host; it will be posted the moment a sole-booted-simulator window exists.

[thymikee]

Re-reviewed exact head : code review is clean. The thrown snapshot-runtime failure now follows the same successful-open contract as a returned capture error, preserving the full normalized error and the usable session; the rejecting regression is revert-sensitive.\n\nExact-head live evidence on iPhone 17 Pro (iOS 26.2): resolved Settings and rendered the 18-visible-node interactive snapshot in default stdout; the immediate follow-up on the same session succeeded in 0.68s. Session was then closed and the simulator shut down.\n\nReady for human review.

[thymikee]

Correction (shell formatting stripped identifiers from the previous comment): Re-reviewed exact head 95fd33e: code review is clean. The thrown snapshot-runtime failure now follows the same successful-open contract as a returned capture error, preserving the full normalized error and the usable session; the rejecting regression is revert-sensitive.\n\nExact-head live evidence on iPhone 17 Pro (iOS 26.2): open --foreground resolved Settings and rendered the 18-visible-node interactive snapshot in default stdout; the immediate follow-up snapshot -i on the same session succeeded in 0.68s. Session was then closed and the simulator shut down.\n\nReady for human review.