Skip to content

Migrate snapshot to daemon client with magical reuse#102

Merged
obj-p merged 2 commits intocli-mcp-parityfrom
snapshot-magical-reuse
Apr 15, 2026
Merged

Migrate snapshot to daemon client with magical reuse#102
obj-p merged 2 commits intocli-mcp-parityfrom
snapshot-magical-reuse

Conversation

@obj-p
Copy link
Copy Markdown
Owner

@obj-p obj-p commented Apr 15, 2026

Summary

Third PR in the CLI/MCP parity stack. `snapshot` becomes a thin daemon client that reuses an existing preview session when one is running for the target file, and falls back to an ephemeral session otherwise.

  • New `SessionResolver`: maps CLI flags (`--session` / `--file` / no flag) to a session ID via a new `session_list` MCP tool.
  • New `session_list` MCP tool: one tab-delimited line per active session.
  • `SnapshotCommand` rewrite: `AsyncParsableCommand` using `DaemonClient`. Handles both reuse and ephemeral paths. Format inferred from output file extension.
  • Bug fix: platform auto-detection hung on xcodeproj sources because `findPackageDirectory` walked up to the repo root's Package.swift. Fixed by short-circuiting when the walk crosses an xcodeproj/xcworkspace/WORKSPACE boundary.
  • MCP tool arg-name fixes: `project` → `projectPath`, `device` → `deviceUDID`. RunCommand had the same bug (PR 2); fixed here too.
  • `PreviewsMacOS.PreviewHost.allSessions` exposed so `session_list` enumerates macOS sessions alongside iOS.
  • `DaemonTestLock` wraps every `SnapshotCommandTests` case for cross-suite serialization.
  • `SnapshotCommand` + `RunCommand` no longer live in the "needs NSApplication" branch.

Observed behavior

Live-session reuse is ~80× faster than cold ephemeral:

  • Ephemeral snapshot: ~30 s (compile + render)
  • Reuse of live session: ~0.4 s (capture only)

Test plan

  • 13 existing `SnapshotCommandTests` pass (now through the daemon path)
  • Manual: `run --detach` then `snapshot ` reuses the live window
  • 243/243 non-iOS tests pass in the concurrent full sweep

Related

🤖 Generated with Claude Code

Base automatically changed from run-over-daemon to cli-mcp-parity April 15, 2026 11:51
Third PR in the CLI/MCP parity stack. `snapshot` becomes a thin daemon
client that reuses an existing preview session when one is running for
the target file, and falls back to an ephemeral session otherwise
(create → capture → stop). No more in-process NSApplication + preview
compile from `snapshot` itself — all of that moves into the daemon.

The magical resolution:
- `--session <uuid>` wins if passed.
- `--file <path>` (or the positional file arg): if exactly one active
  session has that source file, reuse it; else error on ambiguity, or
  ephemeral on miss.
- No flag: if exactly one session is running, reuse it.

Changes:

- New SessionResolver: uses the new `session_list` MCP tool to map
  CLI flags to a session ID. Clear error messages on ambiguity or
  missing session.
- New `session_list` MCP tool: returns one tab-delimited line per
  active session (iOS + macOS). Used by SessionResolver; also useful
  for future discovery commands.
- SnapshotCommand rewrite: AsyncParsableCommand using DaemonClient.
  Handles both reuse and ephemeral paths. Traits are applied only on
  ephemeral start; reusing a session that was configured differently
  prints a note and snapshots with the session's current traits.
  Format is inferred from the output file extension (`.png` → PNG,
  `.jpg`/`.jpeg` → JPEG) unless `--quality` is explicit.
- Platform resolution moved client-side. Previously the daemon called
  `inferredPlatformAsync` when no `--platform` was passed, which could
  hang on xcodeproj sources: the walk-up for Package.swift falls
  through to the repo root, then `swift package describe` runs on a
  large package and can sit forever. New `findPackageDirectory`
  short-circuits when it crosses an xcodeproj/xcworkspace/WORKSPACE
  boundary before finding Package.swift, returning nil instead.
  SnapshotCommand resolves platform locally and always passes an
  explicit `platform` to the daemon.
- Fix MCP tool arg names: `project` → `projectPath`, `device` →
  `deviceUDID`. RunCommand had the same bug; fixed there too.
- PreviewsMacOS.PreviewHost exposes `allSessions` so the new
  `session_list` tool can enumerate macOS sessions alongside iOS.
- DaemonTestLock wraps every SnapshotCommandTests case, matching the
  pattern used by RunCommandTests and DaemonLifecycleTests — without
  it, the daemon state races across suites when they run in parallel.
- SnapshotCommand and RunCommand no longer live in the
  "needs NSApplication" branch of PreviewsMCPApp.main. They're pure
  daemon clients.

Test plan:
- 13 existing snapshot tests still pass (with daemon path instead of
  in-process).
- 243/243 non-iOS tests pass in the full concurrent sweep.
- Manual: `previewsmcp run --detach` then `previewsmcp snapshot <same
  file>` captures the live window in ~0.4s vs ~30s for a cold
  ephemeral.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@obj-p obj-p force-pushed the snapshot-magical-reuse branch from a54cb0d to 72d9141 Compare April 15, 2026 11:54
Five small improvements from the review:

1. Surface preview_stop failures on ephemeral teardown. Extract
   stopEphemeralSession helper so a failed stop logs a warning instead
   of being silently swallowed by `try?`. Leaked sessions in a
   long-lived daemon could otherwise confuse future reuse matching.

2. Remove the dead `if lines.isEmpty` branch in handleSessionList —
   `[].joined(separator:)` already produces "".

3. Extract SessionResolver.resolveAgainst as a pure (no MCP client)
   function expressing the resolution policy. Makes the three
   decision branches (file-matches / file-ambiguous / sole-session)
   explicit and testable in principle, even though the current test
   targets are structured such that we exercise them through the
   integration tests rather than import-based unit tests.

4. Regression tests for findPackageDirectory's non-SPM boundary
   detection: one for xcodeproj-sibling, one for Bazel WORKSPACE.
   Guards against reintroducing the hang we chased down in #102.

5. New integration test: "Snapshot reuses an already-running session
   instead of ephemeral". Starts a detached run, then snapshots the
   same file, and asserts the snapshot completes in under 2s —
   reuse is sub-second whereas ephemeral is 5+ seconds. Proves the
   magical resolution actually reuses sessions rather than silently
   falling through to ephemeral.

30/30 daemon-touching tests pass in the targeted sweep.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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