General refinements: dependency pinning, bug fixes, and improvements#36
General refinements: dependency pinning, bug fixes, and improvements#36
Conversation
…ment - Embed instance ID in Bonjour TXT record to identify each app process - Filter out self-discovered endpoints in RuntimeEngineManager - Start local Bonjour server to advertise this Mac on the network - Extract makeService/instanceID helpers to reduce duplication - Exclude instanceID from Hashable/Equatable (metadata, not identity)
Switch multiple SPM dependencies from branch-based to version-based resolution for more stable and reproducible builds. Remove unused AppKitUI dependency and clean up commented-out code. Signed-off-by: Mx-Iris <mxiris@126.com>
… unused products Pin CocoaCoordinator, RxSwiftPlus, fuzzy-search, and swift-navigation to version tags instead of branch refs. Switch fuzzy-search to fork with tagged releases. Remove unused AppKitNavigation and UIKitNavigation product dependencies from RuntimeViewerArchitectures. Signed-off-by: Mx-Iris <mxiris@126.com>
Pin Semaphore and SwiftyXPC to versioned releases in RuntimeViewerCore. Revert CocoaCoordinator to branch ref (no suitable release tag). Bump RunningApplicationKit from 0.1.1 to 0.2.0. Signed-off-by: Mx-Iris <mxiris@126.com>
Move LaunchServicesPrivate dependency from app/server targets into RuntimeViewerCore/RuntimeViewerUtilities, consolidating sandbox detection (LSBundleProxy.isSandbox, NSRunningApplication.isSandbox) into a shared location. Update AttachToProcess to support RunningPickerTabViewController with both application and process attachment. Pin CocoaCoordinator to versioned release, add UXKitCoordinator package, bump filter-ui to 0.1.2, switch SwiftMCP to upstream Cocoanetics repo, and move per-tool @mcptool naming to server-level toolNaming on @MCPSERVER. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Mx-Iris <mxiris@126.com>
When scheduleRestart(enabled: false) called stop(), the observeToken was cancelled along with the server. This meant re-enabling MCP from Settings had no effect because no observer was watching for the change. Add observe() after stop() in the disabled path to keep listening. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Mx-Iris <mxiris@126.com>
- Enable local MachOObjCSection, pin MachOSwiftSection and swift-mobile-gestalt to versioned releases - Rename SwiftMobileGestalt module alias to SMobileGestalt - Disable local dependency paths in RuntimeViewerPackages, pin XCoordinator to versioned release Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Mx-Iris <mxiris@126.com>
…tting from release workflow Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Mx-Iris <mxiris@126.com>
- Use process identifier (PID) instead of bundle identifier for RuntimeViewerServer identification, fixing issues with processes that lack a bundle ID (e.g. mediaremoted) - Guard LSBundleProxy.forCurrentProcess() which may return nil for non-bundled processes - Pin ide-icons dependency to exact version 0.1.1 - Switch schemes to Debug-arm64e configuration for PAC support - Add ARCHS and ONLY_ACTIVE_ARCH settings for universal builds Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Mx-Iris <mxiris@126.com>
Use RuntimeViewer-Debug as the MCP server name in Debug builds to avoid conflicts with Release builds. Also fix command argument format for claude and codex CLI tools. Signed-off-by: Mx-Iris <mxiris@126.com>
…finements # Conflicts: # RuntimeViewerCore/Sources/RuntimeViewerCommunication/RuntimeNetwork.swift
Design for mirroring RuntimeEngines across macOS hosts via Bonjour, including management/data connection architecture, generic proxy server, transitive sharing with cycle detection, and UI grouping.
…scriptor RuntimeSource is already Codable — no need for a separate EngineKind abstraction.
- Bump MachOObjCSection to 0.6.101, MachOSwiftSection to 0.8.1 - UIFoundation to 0.4.0 with AppleInternal trait, remove separate re-export - Support traits in local package dependency resolution - Remove moduleAliases for SMobileGestalt - Rename debug product to RuntimeViewer-Debug Signed-off-by: Mx-Iris <mxiris@126.com>
Using bundle identifier could match wrong processes when multiple instances share the same bundle ID. PID is unique per process. Signed-off-by: Mx-Iris <mxiris@126.com>
Signed-off-by: Mx-Iris <mxiris@126.com>
- Start Bonjour server before browser to ensure TXT record is registered - Use bonjourWithTXTRecord for browser to receive instance IDs - Log instance IDs in discovery for better debugging - Use explicit RuntimeSource.bonjour initializer Signed-off-by: Mx-Iris <mxiris@126.com>
Add re-entrancy guard and preserve existing attachment characters
when tokenizing new ${...} patterns. Restore cursor position after
text replacements.
Signed-off-by: Mx-Iris <mxiris@126.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing the stability, functionality, and maintainability of the project through a series of general refinements. Key areas of improvement include a significant overhaul of dependency management to use versioned releases, resolving several critical bugs across different modules, and introducing new features like advanced Swift interface generation options and improved Bonjour network discovery. Additionally, the PR includes a detailed design document for future "Remote Engine Mirroring" capabilities and various build system optimizations. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Pull request overview
This PR focuses on stabilizing dependency management, improving runtime discovery/connection behavior (Bonjour + MCP), and polishing build/configuration ergonomics across the macOS app and server.
Changes:
- Pinned many SwiftPM dependencies from branch-based references to versioned releases and consolidated LaunchServices-related helpers.
- Improved Bonjour discovery (TXT-record instance filtering) and added local Bonjour server advertisement; refined MCP behavior/config output.
- Added a new Swift interface generation option (
printVTableOffset) and updated related UI + tests; adjusted arm64e build/scheme settings.
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| RuntimeViewerUsingAppKit/Utils/RuntimeEngineManager.swift | Starts a local Bonjour server early and filters self-discovery via TXT record instance IDs |
| RuntimeViewerUsingAppKit/MCP/MCPStatusPopoverViewModel.swift | Produces Debug/Release-specific MCP configuration strings |
| RuntimeViewerUsingAppKit/Attach Process/* | Migrates attach flow to a unified RunningItem picker and PID-based identifiers |
| RuntimeViewerCore/Sources/RuntimeViewerCommunication/RuntimeNetwork.swift | Adds Bonjour TXT record instance ID support and adjusts endpoint identity semantics |
| RuntimeViewerCore/Sources/RuntimeViewerCore/Core/RuntimeSwiftSection.swift + related UI/tests | Adds printVTableOffset option end-to-end |
| RuntimeViewerCore/Sources/RuntimeViewerUtilities/LaunchServices+.swift | Centralizes sandbox detection via LaunchServicesPrivate |
| RuntimeViewerPackages/Package.swift + other Package.swift files | Dependency pinning and dependency graph adjustments |
| *.xcscheme / project.pbxproj | Switches some scheme actions to Debug-arm64e and adjusts build settings/products |
Comments suppressed due to low confidence (1)
RuntimeViewerPackages/Sources/RuntimeViewerSettingsUI/Components/TransformerSettingsView.swift:1
- Cursor restoration always collapses the selection to length 0 when an adjustment is applied. If the user had an active selection (sel.length > 0), this will unexpectedly clear it. Consider preserving the selection length (and adjusting both location/length as needed), or only forcing
length: 0when the prior selection length was already 0.
#if os(macOS)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| switch runningItem { | ||
| case let runningApplication as RunningApplication: | ||
| try await runtimeEngineManager.launchAttachedRuntimeEngine(name: name, identifier: runningApplication.processIdentifier.description, isSandbox: runningApplication.isSandboxed) | ||
| case let runningProcess as RunningProcess: | ||
| try await runtimeEngineManager.launchAttachedRuntimeEngine(name: name, identifier: runningProcess.processIdentifier.description, isSandbox: runningProcess.isSandboxed) | ||
| default: | ||
| return | ||
| } |
There was a problem hiding this comment.
The identifier used to launch the attached runtime engine is PID-based (processIdentifier.description), but the identifier used to terminate on error is bundleID/name-based. This mismatch can prevent termination of the engine created earlier (leaking state / leaving an engine running). Use a single, consistent identifier for both launch and terminate (e.g., compute identifier = runningItem.processIdentifier.description once and reuse it in both the success and error paths).
| switch runningItem { | ||
| case let runningApplication as RunningApplication: | ||
| runtimeEngineManager.terminateAttachedRuntimeEngine(name: name, identifier: runningApplication.bundleIdentifier ?? name, isSandbox: runningApplication.isSandboxed) | ||
| case let runningProcess as RunningProcess: | ||
| runtimeEngineManager.terminateAttachedRuntimeEngine(name: name, identifier: name, isSandbox: runningProcess.isSandboxed) | ||
| default: | ||
| return | ||
| } |
There was a problem hiding this comment.
The identifier used to launch the attached runtime engine is PID-based (processIdentifier.description), but the identifier used to terminate on error is bundleID/name-based. This mismatch can prevent termination of the engine created earlier (leaking state / leaving an engine running). Use a single, consistent identifier for both launch and terminate (e.g., compute identifier = runningItem.processIdentifier.description once and reuse it in both the success and error paths).
| switch self { | ||
| case .claudeCode: | ||
| return "claude mcp add RuntimeViewer --transport http --url \(url)" | ||
| return "claude mcp add RuntimeViewer-Debug --transport http \(url)" |
There was a problem hiding this comment.
These updated claude mcp add strings dropped the --url flag that existed previously (--transport http --url <url>). If the CLI still requires --url, the generated command will fail for users. Consider restoring --url (and only varying the server name between Debug/Release).
| #else | ||
| switch self { | ||
| case .claudeCode: | ||
| return "claude mcp add RuntimeViewer --transport http \(url)" |
There was a problem hiding this comment.
These updated claude mcp add strings dropped the --url flag that existed previously (--transport http --url <url>). If the CLI still requires --url, the generated command will fail for users. Consider restoring --url (and only varying the server name between Debug/Release).
| .package( | ||
| url: "https://github.com/OpenUXKit/UXKitCoordinator", | ||
| branch: "main" | ||
| ), |
There was a problem hiding this comment.
The PR description says all SPM dependencies were pinned from branch refs to versioned releases, but this new dependency is still branch-pinned (branch: \"main\"). Either pin this dependency to a tagged release (preferred for reproducibility) or update the PR description to reflect that at least one dependency remains branch-based.
| extension LSBundleProxy { | ||
| public var isSandbox: Bool { | ||
| guard let entitlements = entitlements else { return false } | ||
| guard let isSandbox = entitlements["com.apple.security.app-sandbox"] as? Bool else { return false } | ||
| return isSandbox | ||
| } | ||
| } | ||
|
|
||
| extension NSRunningApplication { | ||
| public var applicationProxy: LSApplicationProxy? { | ||
| guard let bundleIdentifier else { return nil } | ||
| return LSApplicationProxy(forIdentifier: bundleIdentifier) | ||
| } | ||
|
|
||
| public var isSandbox: Bool { | ||
| applicationProxy?.isSandbox ?? false | ||
| } | ||
| } |
There was a problem hiding this comment.
This introduces public APIs that mention and extend types from LaunchServicesPrivate (e.g., LSBundleProxy, LSApplicationProxy). If RuntimeViewerUtilities is intended to be a reusable/public module, this risks leaking private-framework types into its public surface area. Prefer making these extensions internal (or gating behind SPI) and expose any needed behavior through your own public wrapper types/protocols instead.
| .font: NSFont.monospacedSystemFont(ofSize: NSFont.systemFontSize, weight: .regular), | ||
| .foregroundColor: NSColor.textColor, | ||
| ] | ||
| let sel = selectedRange() |
There was a problem hiding this comment.
Cursor restoration always collapses the selection to length 0 when an adjustment is applied. If the user had an active selection (sel.length > 0), this will unexpectedly clear it. Consider preserving the selection length (and adjusting both location/length as needed), or only forcing length: 0 when the prior selection length was already 0.
| let newPos = max(0, min(sel.location + cursorAdjustment, textStorage.length)) | ||
| setSelectedRange(NSRange(location: newPos, length: 0)) |
There was a problem hiding this comment.
Cursor restoration always collapses the selection to length 0 when an adjustment is applied. If the user had an active selection (sel.length > 0), this will unexpectedly clear it. Consider preserving the selection length (and adjusting both location/length as needed), or only forcing length: 0 when the prior selection length was already 0.
| let newPos = max(0, min(sel.location + cursorAdjustment, textStorage.length)) | |
| setSelectedRange(NSRange(location: newPos, length: 0)) | |
| let newLocation = max(0, min(sel.location + cursorAdjustment, textStorage.length)) | |
| // Preserve prior selection length when possible | |
| let newLength: Int | |
| if sel.length <= 0 { | |
| newLength = 0 | |
| } else { | |
| // Clamp selection end to the current text length | |
| let originalEnd = sel.location + sel.length | |
| let adjustedEnd = max(newLocation, min(originalEnd + cursorAdjustment, textStorage.length)) | |
| newLength = max(0, adjustedEnd - newLocation) | |
| } | |
| setSelectedRange(NSRange(location: newLocation, length: newLength)) |
| </BuildAction> | ||
| <TestAction | ||
| buildConfiguration = "Debug" | ||
| buildConfiguration = "Debug-arm64e" |
There was a problem hiding this comment.
Switching shared scheme actions (Test/Profile/Analyze) to Debug-arm64e can reduce portability for contributors/CI environments that don’t use that configuration (or aren’t targeting arm64e). If this is intended only for local development, consider keeping shared schemes on Debug and introducing a separate arm64e-specific scheme, or ensure the repository’s documented build/test instructions and CI match this scheme configuration.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive remote engine mirroring design, enabling macOS clients to mirror RuntimeEngines from remote hosts discovered via Bonjour. Key features include transitive mirroring, cycle prevention, and a new proxy server for remote engines. The UI is updated to group engines by host. Additionally, the PR refactors process attachment logic to support both applications and generic processes, updates several package dependencies, and adds a 'Print VTable Offset' option to generation settings. Review comments point out a potential bug in engine termination logic, a typo in an error message, and an opportunity to improve error handling for server-push relays.
| switch runningItem { | ||
| case let runningApplication as RunningApplication: | ||
| runtimeEngineManager.terminateAttachedRuntimeEngine(name: name, identifier: runningApplication.bundleIdentifier ?? name, isSandbox: runningApplication.isSandboxed) | ||
| case let runningProcess as RunningProcess: | ||
| runtimeEngineManager.terminateAttachedRuntimeEngine(name: name, identifier: name, isSandbox: runningProcess.isSandboxed) | ||
| default: | ||
| return | ||
| } |
There was a problem hiding this comment.
在处理注入失败的错误逻辑中，用于终止 RuntimeEngine 的标识符与启动它时使用的标识符不一致。启动引擎时，您使用了 processIdentifier.description 作为唯一标识符。但在错误处理的 catch 块中，您使用了 bundleIdentifier 或 name 来终止引擎。这可能导致在注入失败后，无法正确清理已启动的引擎实例。
建议在终止引擎时也使用 processIdentifier.description 以确保一致性。
| switch runningItem { | |
| case let runningApplication as RunningApplication: | |
| runtimeEngineManager.terminateAttachedRuntimeEngine(name: name, identifier: runningApplication.bundleIdentifier ?? name, isSandbox: runningApplication.isSandboxed) | |
| case let runningProcess as RunningProcess: | |
| runtimeEngineManager.terminateAttachedRuntimeEngine(name: name, identifier: name, isSandbox: runningProcess.isSandboxed) | |
| default: | |
| return | |
| } | |
| switch runningItem { | |
| case let runningApplication as RunningApplication: | |
| runtimeEngineManager.terminateAttachedRuntimeEngine(name: name, identifier: runningApplication.processIdentifier.description, isSandbox: runningApplication.isSandboxed) | |
| case let runningProcess as RunningProcess: | |
| runtimeEngineManager.terminateAttachedRuntimeEngine(name: name, identifier: runningProcess.processIdentifier.description, isSandbox: runningProcess.isSandboxed) | |
| default: | |
| return | |
| } |
| ) { | ||
| guard let connection else { return } | ||
| connection.setMessageHandler(name: name.commandName) { [weak self] () -> Response in | ||
| guard let self else { throw RequestError.senderConnectionIsLose } |
There was a problem hiding this comment.
在 RequestError 的枚举成员名中似乎有一个拼写错误。senderConnectionIsLose 应该是 senderConnectionIsLost，表示连接已丢失。
| guard let self else { throw RequestError.senderConnectionIsLose } | |
| guard let self else { throw RequestError.senderConnectionIsLost } |
| engine.$imageNodes.sink { [connection] imageNodes in | ||
| Task { try? await connection.sendMessage(name: .imageNodes, request: imageNodes) } | ||
| }.store(in: &subscriptions) | ||
|
|
||
| engine.reloadDataPublisher.sink { [weak engine, connection] in | ||
| Task { | ||
| guard let engine else { return } | ||
| let imageList = await engine.imageList | ||
| try? await connection.sendMessage(name: .imageList, request: imageList) | ||
| try? await connection.sendMessage(name: .reloadData) | ||
| } | ||
| }.store(in: &subscriptions) |
There was a problem hiding this comment.
在服务器推送中继的示例代码中，使用了 try? 来忽略 sendMessage 可能抛出的错误。在实际实现中，建议考虑更完善的错误处理机制，例如记录错误日志，以便于调试和监控。忽略错误可能会导致静默失败，使得问题难以被发现。
Summary
printVTableOffsetoption for Swift interface generation, Bonjour self-discovery filtering, and local server broadcasting.Commits (18)
372ce3dfeat: add Bonjour self-discovery filtering and local server advertisement1d6d70arefactor: pin dependencies to versioned releases instead of branch refs0b6db56refactor: pin remaining dependencies to versioned releases and remove unused productsd947453refactor: pin Core dependencies and update RunningApplicationKit version46bc416refactor: consolidate LaunchServices extensions and update dependenciesda792e2fix: re-establish settings observation after MCP server disabled575487drefactor: update dependency configurations and module aliases5ee48ddchore: remove unnecessary ASSETCATALOG_COMPILER_APPICON_NAME build settingd4180abfix: use PID as identifier and improve arm64e build configurationa460d88fix: differentiate MCP config names between Debug and Release builds4a1b040docs: add remote engine mirroring design spec84baaa1docs: revert EngineKind, use RuntimeSource directly in RemoteEngineDescriptor983df86refactor: update dependency versions and module configurationad439c5fix: use PID instead of bundle identifier for caller tracking7889188feat: add printVTableOffset option for Swift interface generation4c67687fix: improve Bonjour discovery reliability1820db0fix: prevent tokenizer from destroying existing token attachments