-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Cloud terminals: manual-IO data path (attach --pipe-io relay + reconnecting pump) #11062
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
Changes from all commits
07ca9a1
7c80dac
2bd73f4
f542a65
01528d0
f402019
3e6eae2
5e27336
00efa77
8281ce2
c0f2b65
999b0d0
97e74f2
0cfc8f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,104 @@ | ||
| import CmuxSettings | ||
| import Foundation | ||
| #if DEBUG | ||
| import CMUXDebugLog | ||
| #endif | ||
|
|
||
| /// Everything a workspace needs to run one `--pipe-io` relay for a cloud | ||
| /// machine's cmux-tui terminal: the bundled client, the machine link's local | ||
| /// mux socket, and the daemon terminal id. | ||
| struct CloudTuiManualIOAttach: Equatable, Sendable { | ||
| let clientPath: String | ||
| let socketPath: String | ||
| let terminalID: String | ||
|
|
||
| @MainActor | ||
| func makePump() -> TuiManualIOPump { | ||
| TuiManualIOPump( | ||
| binaryPath: clientPath, | ||
| target: .socket(socketPath), | ||
| terminalID: terminalID, | ||
| // The relay only dials a local unix socket; the ambient | ||
| // environment (PATH, HOME, TMPDIR) is all it needs, same as the | ||
| // link client itself. | ||
| environment: ProcessInfo.processInfo.environment | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| /// Gate for the cloud manual-IO data path. On (the default) a cloud | ||
| /// terminal pane renders through a manual-mirror Ghostty surface fed by | ||
| /// `attach --pipe-io`; off (or when the bundled client predates the flag) | ||
| /// it falls back to the exec attach pane running the full TUI renderer. | ||
| enum CloudTuiManualIO { | ||
| nonisolated static var isEnabled: Bool { | ||
| let key = SettingCatalog().betaFeatures.cloudTerminalManualIO | ||
| return Bool.decodeFromUserDefaults(UserDefaults.standard.object(forKey: key.userDefaultsKey)) | ||
| ?? key.defaultValue | ||
| } | ||
|
|
||
| /// True when `client` understands `attach --pipe-io`. The bundled | ||
| /// client comes from a rolling artifacts manifest, so a freshly built | ||
| /// app can carry a client older than this feature; probing keeps that | ||
| /// skew a silent fallback to the exec pane instead of a relay crash | ||
| /// loop. One probe per client path+mtime, cached for the process. | ||
| static func clientSupportsPipeIO(clientURL: URL) async -> Bool { | ||
| await CloudTuiPipeIOProbe.shared.supportsPipeIO(clientURL: clientURL) | ||
| } | ||
| } | ||
|
|
||
| /// Serializes `--help` probes and caches their results; an actor so the | ||
| /// child `Process` and its deadline task stay on one isolation domain (the | ||
| /// same shape as ``CloudMachineLink/run(arguments:timeout:)``). | ||
| actor CloudTuiPipeIOProbe { | ||
| static let shared = CloudTuiPipeIOProbe() | ||
| private var results: [String: Bool] = [:] | ||
|
|
||
| func supportsPipeIO(clientURL: URL) async -> Bool { | ||
| let path = clientURL.path | ||
| let mtime = ((try? FileManager.default.attributesOfItem(atPath: path))?[.modificationDate] as? Date)? | ||
| .timeIntervalSince1970 ?? 0 | ||
| let cacheKey = "\(path)#\(mtime)" | ||
| if let cached = results[cacheKey] { | ||
| return cached | ||
| } | ||
| let supported = await probeHelp(clientURL: clientURL) | ||
| results[cacheKey] = supported | ||
| #if DEBUG | ||
| cmuxDebugLog("cloudTuiManualIO.probe client=\(path) supportsPipeIO=\(supported)") | ||
| #endif | ||
| return supported | ||
| } | ||
|
|
||
| private func probeHelp(clientURL: URL) async -> Bool { | ||
| let process = Process() | ||
| process.executableURL = clientURL | ||
| // The top-level help is the short index; `--pipe-io` is documented | ||
| // only in the attach/start options help. | ||
| process.arguments = ["attach", "--help"] | ||
| process.standardInput = FileHandle.nullDevice | ||
| process.standardError = FileHandle.nullDevice | ||
| let stdout = Pipe() | ||
| process.standardOutput = stdout | ||
| let exit = CloudLinkFirstValue<Int32>() | ||
| process.terminationHandler = { exit.resolve($0.terminationStatus) } | ||
| do { | ||
| try process.run() | ||
| } catch { | ||
| return false | ||
| } | ||
| async let output = CloudLinkPipe.readToEnd(stdout.fileHandleForReading) | ||
| let deadline = Task { | ||
| do { | ||
| try await Task.sleep(for: .seconds(10)) | ||
| } catch { | ||
| return | ||
| } | ||
| process.terminate() | ||
| } | ||
| _ = await exit.result | ||
| deadline.cancel() | ||
| let text = String(decoding: await output, as: UTF8.self) | ||
| return text.contains("--pipe-io") | ||
| } | ||
|
Comment on lines
+73
to
+103
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
# Description: Check whether the cmux-tui client already exposes a machine-readable capability surface the probe could use.
set -euo pipefail
# Named capabilities advertised over the control protocol.
rg -nP --type=rust -C3 '_CAPABILITY:\s*&str\s*=' cmux-tui
# Any existing client-side command that prints capabilities or a structured version.
rg -nP --type=rust -C4 '"capabilities"' cmux-tui/crates/cmux-tui/src | head -60
# Confirm the exact help string the probe depends on today.
rg -nP --type=rust -C3 -- '--pipe-io' cmux-tui/crates/cmux-tui/src/main.rsRepository: manaflow-ai/cmux Length of output: 15106 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- repository rules and learnings ---'
head -5 /tmp/coderabbit-repo-knowledge/manaflow-ai-cmux-b0f68d40/*/*.md 2>/dev/null || true
printf '%s\n' '--- reviewed implementation ---'
cat -n Sources/Cloud/CloudTuiManualIO.swift | sed -n '1,135p'
printf '%s\n' '--- RemoteSession capability flow ---'
rg -n -C5 'identify|parse_identity_capabilities|capabilities|SURFACE_SUBSCRIBE_FILTER_CAPABILITY' \
cmux-tui/crates/cmux-tui/src/session/remote.rs \
cmux-tui/crates/cmux-tui/src/session/mod.rs \
cmux-tui/crates/cmux-tui-machine-protocol/src/lib.rs
printf '%s\n' '--- pipe-io ownership and client argument flow ---'
rg -n -C5 -- '--pipe-io|pipe_io|probeHelp|CloudTuiManualIO' Sources cmux-tui/crates/cmux-tui/src/main.rsRepository: manaflow-ai/cmux Length of output: 50373 Use a machine-readable client capability check for
🧰 Tools🪛 SwiftLint (0.65.0)[Warning] 101-101: Prefer failable (optional_data_string_conversion) 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Replace the static namespace and the process-wide probe singleton with an injectable owner.
CloudTuiManualIOis a namespace-only enum with static members, andCloudTuiPipeIOProbe.sharedis a new runtime singleton holding a mutableresultscache. The coding guidelines for**/Sources/**/*.swiftstate: "do not add public or internal top-level free functions, top-level mutable variables, stub types holding global flags or counters, static-only namespaces, or new singletons for runtime state. Put state and behavior on a constructable, injectable owning type."The cache also cannot be reset between tests, so one probe result leaks across the whole process.
Make
CloudTuiPipeIOProbean injected dependency of the surface provider that calls it, and moveisEnabledonto that owner.As per coding guidelines: "Avoid new ambient global runtime state: top-level API functions, mutable globals, namespace-only types, and runtime singletons. Prefer constructable injectable owners and private/fileprivate helpers."
Also applies to: 53-56
🤖 Prompt for AI Agents
Source: Coding guidelines