Repository navigation
CmuxIPCService: extract AppDelegate multi-window CLI routing (MultiWindowRouter behind MultiWindowRouting) #5920
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
00da345
CmuxControlSocket stage 3c-1: ControlCommandCoordinator + window domain
azooz2003-bit 2383068
stage 3c: multi-protocol seam umbrella + shared param/ref helper port
azooz2003-bit b02c28d
stage 3c: extract App Focus + Feed + Notification domains
azooz2003-bit 9bdae00
stage 3c: extract Mobile Host + Workspace Groups + Pane domains
azooz2003-bit 4b8118f
stage 3c: fix int/double param helpers to match legacy NSNumber coercion
azooz2003-bit 1905b38
stage 3c: integrate Workspace + Surface domains (full lifts)
azooz2003-bit 8f7ac89
stage 3c: organize coordinator files into per-domain subfolders
azooz2003-bit 487baaf
stage 3c: dedupe workspace.create onto the shared v2WorkspaceCreate body
azooz2003-bit 6f2cba5
stage 3c: fix 8 divergences found by Workspace+Surface adversarial re…
azooz2003-bit fe419f0
Budget: entries for the two coordinator files grown by the divergence…
azooz2003-bit a94589d
Merge remote-tracking branch 'origin/main' into feat-ctl-coordinator-…
azooz2003-bit f731c27
Fix duplicate parameter name warning (sessionID sessionID) in workspa…
azooz2003-bit 7d234dc
stage 3c: package side of System/Project/Debug/Sidebar/Browser domain…
azooz2003-bit 39d111b
stage 3c: cut over System/Project/Debug/Sidebar/Browser domains to th…
azooz2003-bit ecd5148
stage 3c: dedupe file.open onto the shared v2FileOpen body
azooz2003-bit 3178b68
stage 3c: fix browser.focus_mode.set/zoom.set error precedence; finis…
azooz2003-bit 42f9332
Budget tsv: strip accidental leading whitespace
azooz2003-bit a9ce013
Fix duplicate 'sessionID' parameter label warning (Xcode 26.5 local g…
azooz2003-bit 718e4b1
CmuxIPCService: faithful lift of AppDelegate multi-window CLI routing
azooz2003-bit 3c4273e
CmuxIPCService: modernize route to async throws
azooz2003-bit d47e003
Merge pull request #5909 from manaflow-ai/feat-ctl-3c-final-domains
azooz2003-bit 29d8342
stage 3c: address review threads (handles observation, window.close t…
azooz2003-bit 9cbcddd
CmuxIPCService: mark file-scoped logger nonisolated (review)
azooz2003-bit 2be538e
Merge origin/main: adopt PR 5778's socket-worker browser JS lane; rev…
azooz2003-bit 8110c97
Merge origin/feat-ctl-coordinator-3c-1 into feat-ipc-service
azooz2003-bit 04a9f21
Merge origin/main into feat-ipc-service
azooz2003-bit e9e22c1
Merge remote-tracking branch 'origin/main' into feat-ipc-service
azooz2003-bit 18958a9
Merge origin/main into feat-ipc-service: re-sync onto main post-5894/…
azooz2003-bit File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| // swift-tools-version: 6.0 | ||
|
|
||
| import PackageDescription | ||
|
|
||
| let package = Package( | ||
| name: "CmuxIPCService", | ||
| platforms: [ | ||
| .macOS(.v14), | ||
| ], | ||
| products: [ | ||
| .library( | ||
| name: "CmuxIPCService", | ||
| targets: ["CmuxIPCService"] | ||
| ), | ||
| ], | ||
| targets: [ | ||
| .target( | ||
| name: "CmuxIPCService", | ||
| swiftSettings: [ | ||
| .swiftLanguageMode(.v6), | ||
| .enableUpcomingFeature("ExistentialAny"), | ||
| .enableUpcomingFeature("InternalImportsByDefault"), | ||
| ] | ||
| ), | ||
| .testTarget( | ||
| name: "CmuxIPCServiceTests", | ||
| dependencies: ["CmuxIPCService"], | ||
| swiftSettings: [ | ||
| .swiftLanguageMode(.v6), | ||
| .enableUpcomingFeature("ExistentialAny"), | ||
| .enableUpcomingFeature("InternalImportsByDefault"), | ||
| ] | ||
| ), | ||
| ] | ||
| ) |
22 changes: 22 additions & 0 deletions
22
Packages/CmuxIPCService/Sources/CmuxIPCService/MultiWindowRouteLaunchError.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| /// Thrown by ``MultiWindowRouting/route(arguments:)`` when the cmux CLI | ||
| /// process could not be launched at all (the executable is missing, not | ||
| /// executable, or `posix_spawn` failed). | ||
| /// | ||
| /// This is the only error the route throws; once the CLI launches, every | ||
| /// outcome (including non-zero exit) is data in ``MultiWindowRouteResult``. | ||
| /// `description` preserves the underlying Foundation error's | ||
| /// `String(describing:)` text verbatim, so callers that re-encode the legacy | ||
| /// `"-1"` capture (`String(describing: error)` into the UI-test data file) | ||
| /// produce byte-identical output to the pre-extraction code. | ||
| public struct MultiWindowRouteLaunchError: Error, Sendable, Equatable, CustomStringConvertible { | ||
| /// The underlying launch error rendered with `String(describing:)`, | ||
| /// preserved verbatim for the legacy capture encoding. | ||
| public let description: String | ||
|
|
||
| /// Creates a launch error. | ||
| /// - Parameter description: The underlying error's `String(describing:)` | ||
| /// text. | ||
| public init(description: String) { | ||
| self.description = description | ||
| } | ||
| } |
31 changes: 31 additions & 0 deletions
31
Packages/CmuxIPCService/Sources/CmuxIPCService/MultiWindowRouteResult.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| /// The captured outcome of one multi-window route call through the bundled | ||
| /// cmux CLI. | ||
| /// | ||
| /// Produced by ``MultiWindowRouting/route(arguments:)`` once the CLI process | ||
| /// launched and exited (a process that never launched throws | ||
| /// ``MultiWindowRouteLaunchError`` instead). The streams are UTF-8 decoded | ||
| /// with non-decodable output collapsing to the empty string, matching the | ||
| /// legacy AppDelegate capture exactly; consumers (the multi-window UI-test | ||
| /// scaffolding) write `String(terminationStatus)` and the streams verbatim | ||
| /// into the shared test-data file. | ||
| public struct MultiWindowRouteResult: Sendable, Equatable { | ||
| /// The CLI process termination status. | ||
| public let terminationStatus: Int32 | ||
| /// The captured standard output, UTF-8 decoded; empty when absent or not | ||
| /// valid UTF-8. | ||
| public let stdout: String | ||
| /// The captured standard error, UTF-8 decoded; empty when absent or not | ||
| /// valid UTF-8. | ||
| public let stderr: String | ||
|
|
||
| /// Creates a route result. | ||
| /// - Parameters: | ||
| /// - terminationStatus: The CLI process termination status. | ||
| /// - stdout: The captured standard output. | ||
| /// - stderr: The captured standard error. | ||
| public init(terminationStatus: Int32, stdout: String, stderr: String) { | ||
| self.terminationStatus = terminationStatus | ||
| self.stdout = stdout | ||
| self.stderr = stderr | ||
| } | ||
| } |
146 changes: 146 additions & 0 deletions
146
Packages/CmuxIPCService/Sources/CmuxIPCService/MultiWindowRouter.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,146 @@ | ||
| public import Foundation | ||
| import Darwin | ||
| import os | ||
|
|
||
| /// Diagnostics for partial pipe reads; mirrors the app-side ProcessPipeReader | ||
| /// warning the lifted code emitted (file-scoped `os.Logger` per house style). | ||
| nonisolated private let logger = Logger(subsystem: "com.cmuxterm.app", category: "MultiWindowRouter") | ||
|
|
||
| /// Runs the bundled cmux CLI against the app's control socket to route a | ||
| /// request to a specific window, capturing its output. | ||
| /// | ||
| /// This is the production ``MultiWindowRouting``, extracted from AppDelegate's | ||
| /// `runMultiWindowRouteCLI`: it spawns the CLI with an implicit | ||
| /// `--socket <path>` argument pair and an explicit child environment (the | ||
| /// child inherits nothing beyond what is injected), then captures termination | ||
| /// status and both streams. A launch failure throws | ||
| /// ``MultiWindowRouteLaunchError``; a launched CLI always returns a result. | ||
| /// | ||
| /// Isolation design: the router holds only immutable `Sendable` configuration | ||
| /// (CLI URL, socket path, environment), so there is no state to protect and an | ||
| /// actor would serialize unrelated route calls for no benefit (the same ruling | ||
| /// that made `CmuxProcess.CommandRunner` a stateless struct). The legacy | ||
| /// synchronous `waitUntilExit` is replaced by a `terminationHandler` | ||
| /// continuation, so `route` never blocks its calling thread; the two stream | ||
| /// readers run on detached tasks (the `CommandRunner` pattern) so a stream | ||
| /// larger than the pipe buffer cannot deadlock the child against an unread | ||
| /// pipe. | ||
| public struct MultiWindowRouter: MultiWindowRouting, Sendable { | ||
| private let cliURL: URL | ||
| private let socketPath: String | ||
| // Environment is value-like once copied; stored immutable so the struct | ||
| // stays Sendable. | ||
| private let environment: [String: String] | ||
|
|
||
| /// Creates a router for one CLI binary, socket, and child environment. | ||
| /// - Parameters: | ||
| /// - cliURL: The bundled cmux CLI executable. | ||
| /// - socketPath: The control socket path passed to every call as | ||
| /// `--socket <path>`. | ||
| /// - environment: The complete child process environment (replaces, not | ||
| /// merges with, the app's environment). | ||
| public init(cliURL: URL, socketPath: String, environment: [String: String]) { | ||
| self.cliURL = cliURL | ||
| self.socketPath = socketPath | ||
| self.environment = environment | ||
| } | ||
|
|
||
| /// Runs the CLI with `arguments` and captures its outcome. | ||
| /// | ||
| /// Implements ``MultiWindowRouting/route(arguments:)``; see the protocol | ||
| /// for the full contract and the type docs for the isolation rationale. | ||
| public func route(arguments: [String]) async throws -> MultiWindowRouteResult { | ||
| let process = Process() | ||
| process.executableURL = cliURL | ||
| process.arguments = ["--socket", socketPath] + arguments | ||
| process.environment = environment | ||
|
|
||
| let stdoutPipe = Pipe() | ||
| let stderrPipe = Pipe() | ||
| process.standardOutput = stdoutPipe | ||
| process.standardError = stderrPipe | ||
|
|
||
| // Drain both streams on detached tasks, keyed by raw fd so no | ||
| // non-Sendable FileHandle crosses the task boundary. Started before the | ||
| // spawn so a child that fills a pipe buffer can never deadlock against | ||
| // an unread pipe; the reads block until data or EOF arrives. | ||
| let stdoutDescriptor = stdoutPipe.fileHandleForReading.fileDescriptor | ||
| let stderrDescriptor = stderrPipe.fileHandleForReading.fileDescriptor | ||
| let stdoutReader = Task.detached { | ||
| self.readDataToEndOfFileOrEmpty(fromFileDescriptor: stdoutDescriptor) | ||
| } | ||
| let stderrReader = Task.detached { | ||
| self.readDataToEndOfFileOrEmpty(fromFileDescriptor: stderrDescriptor) | ||
| } | ||
|
azooz2003-bit marked this conversation as resolved.
|
||
|
|
||
| let terminationStatus: Int32 | ||
| do { | ||
| terminationStatus = try await withCheckedThrowingContinuation { continuation in | ||
| process.terminationHandler = { finished in | ||
| continuation.resume(returning: finished.terminationStatus) | ||
| } | ||
| do { | ||
| try process.run() | ||
| // Close the parent's write ends so the readers see EOF once | ||
| // the child closes its copies. | ||
| try? stdoutPipe.fileHandleForWriting.close() | ||
| try? stderrPipe.fileHandleForWriting.close() | ||
| } catch { | ||
| process.terminationHandler = nil | ||
| try? stdoutPipe.fileHandleForWriting.close() | ||
| try? stderrPipe.fileHandleForWriting.close() | ||
| continuation.resume( | ||
| throwing: MultiWindowRouteLaunchError(description: String(describing: error)) | ||
| ) | ||
| } | ||
| } | ||
| } catch { | ||
| // The write ends are closed, so the readers finish on EOF; await | ||
| // them before rethrowing so no detached work outlives the call. | ||
| _ = await stdoutReader.value | ||
| _ = await stderrReader.value | ||
| throw error | ||
| } | ||
|
|
||
| let stdoutData = await stdoutReader.value | ||
| let stderrData = await stderrReader.value | ||
| return MultiWindowRouteResult( | ||
| terminationStatus: terminationStatus, | ||
| stdout: String(data: stdoutData, encoding: .utf8) ?? "", | ||
| stderr: String(data: stderrData, encoding: .utf8) ?? "" | ||
| ) | ||
| } | ||
|
|
||
| /// Reads `fileDescriptor` to end-of-file, retrying `EINTR`, returning | ||
| /// partial data (with a logged warning) on a read error. Carried over from | ||
| /// the app-side `ProcessPipeReader.readDataToEndOfFileOrEmpty`, which stays | ||
| /// app-target for its other callers. Operates on the raw descriptor so the | ||
| /// detached reader tasks capture only `Sendable` values (`self` is a | ||
| /// Sendable value type). | ||
| private func readDataToEndOfFileOrEmpty(fromFileDescriptor fileDescriptor: Int32) -> Data { | ||
| let chunkSize = 64 * 1024 | ||
| var data = Data() | ||
| var buffer = [UInt8](repeating: 0, count: chunkSize) | ||
| while true { | ||
| let bytesRead = buffer.withUnsafeMutableBytes { pointer -> Int in | ||
| guard let baseAddress = pointer.baseAddress else { return 0 } | ||
| return Darwin.read(fileDescriptor, baseAddress, chunkSize) | ||
| } | ||
| if bytesRead > 0 { | ||
| data.append(contentsOf: buffer[0..<bytesRead]) | ||
| continue | ||
| } | ||
| if bytesRead == 0 { | ||
| return data | ||
| } | ||
| let code = errno | ||
| if code == EINTR { | ||
| continue | ||
| } | ||
| logger.warning( | ||
| "multiWindowRouter.readFailed errno=\(Int(code), privacy: .public) fd=\(fileDescriptor, privacy: .public) partialBytes=\(data.count, privacy: .public)" | ||
| ) | ||
| return data | ||
| } | ||
| } | ||
| } | ||
52 changes: 52 additions & 0 deletions
52
Packages/CmuxIPCService/Sources/CmuxIPCService/MultiWindowRouting.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| /// Routes a cmux CLI request to a specific window over the app's control | ||
| /// socket and returns the captured outcome. | ||
| /// | ||
| /// This is the seam for the multi-window CLI-over-socket capability extracted | ||
| /// from AppDelegate: production code uses ``MultiWindowRouter``, and tests | ||
| /// inject a fake conforming type so they never spawn the real CLI. The window | ||
| /// targeting itself is expressed in `arguments` (for example | ||
| /// `["list-workspaces", "--window", id]`); the conforming type supplies the | ||
| /// CLI binary, socket path, and child environment. | ||
| public protocol MultiWindowRouting: Sendable { | ||
| /// Runs the bundled cmux CLI against the configured socket with `arguments` | ||
| /// and captures its termination status and output. | ||
| /// | ||
| /// - Parameter arguments: The CLI arguments after the implicit | ||
| /// `--socket <path>` pair (subcommand, window targeting flags, output | ||
| /// format flags). | ||
| /// - Returns: The ``MultiWindowRouteResult`` describing how the launched | ||
| /// CLI call finished. | ||
| /// - Throws: ``MultiWindowRouteLaunchError`` when the CLI process could | ||
| /// not be launched; a launched CLI never throws, its outcome (including | ||
| /// non-zero exit) is the returned result. | ||
| func route(arguments: [String]) async throws -> MultiWindowRouteResult | ||
| } | ||
|
|
||
| extension MultiWindowRouting { | ||
| /// Routes one CLI call, encoding a launch failure into the result instead | ||
| /// of throwing. | ||
| /// | ||
| /// This is the legacy capture encoding the pre-extraction AppDelegate | ||
| /// helper produced and the multi-window UI-test data file still expects: | ||
| /// a CLI that never launched yields termination status `-1` with the | ||
| /// launch error's description in `stderr` (byte-identical to the old | ||
| /// `String(describing:)` text via ``MultiWindowRouteLaunchError``'s | ||
| /// `CustomStringConvertible`). Use it when every call in a batch must run | ||
| /// regardless of earlier launch failures. | ||
| /// | ||
| /// - Parameter arguments: The CLI arguments after the implicit | ||
| /// `--socket <path>` pair. | ||
| /// - Returns: The route result, with launch failure folded in as | ||
| /// termination status `-1`. | ||
| public func routeCapturingLaunchFailure(arguments: [String]) async -> MultiWindowRouteResult { | ||
| do { | ||
| return try await route(arguments: arguments) | ||
| } catch { | ||
| return MultiWindowRouteResult( | ||
| terminationStatus: -1, | ||
| stdout: "", | ||
| stderr: String(describing: error) | ||
| ) | ||
| } | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.