Skip to content

Revise README, fix setup state loss (#86), auto-detect platform (#83)#91

Merged
obj-p merged 9 commits intomainfrom
readme-and-fixes
Apr 13, 2026
Merged

Revise README, fix setup state loss (#86), auto-detect platform (#83)#91
obj-p merged 9 commits intomainfrom
readme-and-fixes

Conversation

@obj-p
Copy link
Copy Markdown
Owner

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

Summary

  • README rewrite: Revised "Why PreviewsMCP?" section to lead with standalone workflow value (CLI, hot-reload, variants, interaction) and fix inaccuracy about Xcode previews. Xcode previews run in a real app process — but it's Apple's opaque shell, not extensible by the developer. Setup plugin is now positioned as additive, not the core pitch.

  • Fix Setup plugin state lost after hot-reload (static linking gives each dylib its own statics) #86 — Setup plugin state lost after hot-reload: Build the setup module as a dynamic library (libPreviewSetup.dylib) instead of static archives (.a). The dylib is loaded once with RTLD_GLOBAL before any preview dylib, so all preview dylibs share the same statics (e.g., PreviewEnvironment.shared). Previously, static linking gave each dylib its own copy of statics, causing setUp() state to vanish on hot-reload.

  • Fix Auto-detect platform from project instead of defaulting to macOS #83 — Auto-detect platform from SPM metadata: When no explicit platform is specified (CLI flag, MCP parameter, or .previewsmcp.json), detect the platform from swift package describe --type json. If the package declares only iOS (no macOS), the platform defaults to ios instead of macos. Applies to all CLI commands (run, snapshot, variants) and the MCP server.

Test plan

  • Build succeeds (swift build)
  • Existing test suite passes (swift test)
  • Verify setup plugin state persists across hot-reload with the example ToDo app:
    previewsmcp run --preview 1 examples/spm/Sources/ToDo/ToDoView.swift --platform ios \
      --config examples/PreviewSetup/.previewsmcp.json
    Edit ToDoView.swift → purple badge should persist after reload
  • Verify platform auto-detection: run previewsmcp on an iOS-only SPM package without --platform → should default to iOS
  • Verify explicit --platform macos still overrides auto-detection

🤖 Generated with Claude Code

obj-p and others added 4 commits April 13, 2026 13:25
…atform

- Rewrite "Why PreviewsMCP?" to lead with standalone workflow value and
  fix inaccuracy about Xcode previews (they use a real app process, but
  it's opaque and not extensible). Setup plugin is now positioned as
  additive, not the core value prop.

- Fix #86: Build setup module as a dynamic library instead of static so
  all preview dylibs share the same statics. Previously each dylib got
  its own copy of setup statics (e.g. PreviewEnvironment.shared),
  causing setUp() state to be lost on hot-reload. The setup dylib is
  now loaded once with RTLD_GLOBAL before any preview dylib.

- Fix #83: Auto-detect platform from SPM package metadata when no
  explicit platform is provided via CLI flag or .previewsmcp.json.
  If the package declares only iOS (no macOS), the platform defaults
  to iOS instead of macOS. Detection uses `swift package describe`.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…on, tests

Review fixes:
1. Pass -sdk for both macOS and iOS in SetupBuilder.linkDynamicLibrary
   (previously only iOS got an SDK path).
2. Replace -undefined dynamic_lookup with -L/-lPreviewSetup so the
   linker catches genuine missing symbols at link time instead of
   deferring all undefined symbols to runtime.
3. Clean stale .a archives from previous static-linking builds.
4. Store setupDylibPath in PreviewHost so the watchFile hot-reload
   callback passes it through on structural recompile.
5. Add async detectPlatformsAsync() for MCP server to avoid blocking
   the cooperative thread pool. MCP now uses the async variant.
6. Extract findPackageDirectory() and decodePlatforms() as reusable
   helpers, eliminating duplicated directory walk logic.

Tests:
- SetupBuilderTests: Result struct shape, real dylib build from example
  package (verifies dylib exists, flags include -lPreviewSetup, no
  dynamic_lookup), packageNotFound error case.
- BuildSystemTests: findPackageDirectory walk, detectPlatforms from
  real SPM package, detectPlatforms nil for standalone, async variant
  matches sync variant.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
findPackageDirectory now ignores directories named Package.swift,
consistent with SPMBuildSystem.detect(for:). Added a test for this
edge case.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add inferredPlatform(for:) and inferredPlatformAsync(for:) to
  SPMBuildSystem, replacing 4 duplicated string-comparison blocks
  with single-source-of-truth helpers.
- Fix SetupCache to persist and validate dylibPath. The cache entry
  now includes the dylib path, and load() verifies the dylib exists.
- Update validateArtifacts to accept .dylib in addition to .a when
  checking -l library flags.
- Update SetupCacheTests for the new Result shape and dylib artifacts.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@obj-p obj-p force-pushed the readme-and-fixes branch from 5ff96a4 to 9eaa0ea Compare April 13, 2026 17:30
obj-p and others added 3 commits April 13, 2026 13:41
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Previously --platform defaulted to .macos, making it impossible to
distinguish "user passed --platform macos" from "user didn't pass
--platform". With auto-detection, this meant --platform macos couldn't
override an iOS-only package. Now the flag is Optional — nil means
"auto-detect", a value means "use exactly this".

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Without an install_name, dyld may load a second copy of the setup
dylib when a preview dylib references it — defeating the shared-statics
goal. Setting install_name to the absolute path lets dyld recognize the
already-RTLD_GLOBAL-loaded image. The rpath on preview dylib flags
ensures dyld can find it at the same path.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Owner Author

@obj-p obj-p left a comment

Choose a reason for hiding this comment

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

Approve — The dylib approach correctly solves the statics-per-dylib problem (#86), platform auto-detection is cleanly implemented (#83), and the README rewrite is an improvement. Tests are solid and the code is well-commented where it matters. Two minor nits inline — neither blocks merge.

Comment on lines +44 to +51
if let setupIndex = args.firstIndex(of: "--setup-dylib"),
setupIndex + 1 < args.count {
let setupPath = args[setupIndex + 1]
if dlopen(setupPath, RTLD_NOW | RTLD_GLOBAL) == nil {
let err = String(cString: dlerror())
NSLog("PreviewHost: Failed to load setup dylib: \\(err)")
}
}
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Nit: On iOS, if the setup dylib fails to load, this logs and continues — then loadPreview will likely fail when the preview dylib can't resolve setup symbols. On macOS, DylibLoader throws, which propagates through loadPreview with a clear error message. The asymmetry is fine for a dev tool (the NSLog gives enough signal), but worth noting the different failure modes.

progress: progress)

let setupResult = try await buildSetupFromConfig(configResult, platform: .macOS)
setupDylibPath = setupResult?.dylibPath
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Nit: The buildSetupFromConfig call on the line above passes .macOS unconditionally. If the user is running variants for iOS, the setup dylib would be built for the wrong platform. This looks pre-existing (not introduced by this PR), but worth a follow-up to thread resolvedPlatform through here.

@obj-p
Copy link
Copy Markdown
Owner Author

obj-p commented Apr 13, 2026

Re: the two nits —

iOS setup dylib failure mode (#discussion_r3075197072): Intentional. Failing hard here would crash the host app before the TCP connection establishes — the MCP server would get an opaque timeout instead of a useful error. By logging and continuing, the subsequent dlopen of the preview dylib fails with a clear "Library not loaded: libPreviewSetup.dylib" message that propagates back through the TCP protocol. The NSLog gives additional signal in the simulator console.

Hardcoded .macOS in VariantsCommand (#discussion_r3075197267): This is already correct — line 180 is inside runMacOSVariants (dispatched via the switch resolvedPlatform at line 114). The iOS path at line 283 passes .iOS. Each platform branch builds the setup dylib for the matching platform.

Read pipe data before waitUntilExit to prevent deadlock when
swift package describe output exceeds the ~64KB pipe buffer.
The subprocess blocks writing to the full pipe while the parent
blocks waiting for exit — classic pipe deadlock that hung CI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@obj-p obj-p enabled auto-merge (squash) April 13, 2026 20:54
The config check only matched "ios", so "macos" fell through to
inferredPlatform which spawns swift package describe on every CLI
invocation. Also add a 10-second timeout to detectPlatforms as a
safety net against subprocess hangs on CI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@obj-p obj-p merged commit 1a9ca5d into main Apr 13, 2026
4 checks passed
@obj-p obj-p deleted the readme-and-fixes branch April 13, 2026 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant