diff --git a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/CmuxInternalHostnames.swift b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/CmuxInternalHostnames.swift index b340a3e2dbf9..760ab1b09c68 100644 --- a/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/CmuxInternalHostnames.swift +++ b/Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/CmuxInternalHostnames.swift @@ -4,10 +4,12 @@ import Foundation /// address, and the `/etc/hosts` management that makes the name resolve. /// /// The name only works from a Mac whose WireGuard tunnel is up (the address -/// it resolves to is unroutable otherwise), so this is deliberately a courtesy -/// convenience layered on the private IP, never a substitute for it: every -/// caller that offers a `.internal` link falls back to the bare address when -/// the entry cannot be written or has not been synced yet. +/// it resolves to is unroutable otherwise) AND whose `/etc/hosts` has been +/// synced (`cmux vpn hosts`), so nothing in the app builds a clickable link +/// out of it — `directPortURL` uses the raw address instead, unconditionally. +/// The `.internal` name is a manual-use convenience (typed into a terminal, +/// an SSH config) that this module still builds and publishes to +/// `/etc/hosts`; it is just never the thing a click depends on. /// /// Shared between the app (which builds the link a person clicks) and the CLI /// (`cmux vpn hosts`, which owns writing `/etc/hosts`) so the slug algorithm @@ -65,6 +67,19 @@ public enum CmuxInternalHostnames { slug(raw) ?? raw } + /// The URL a click on a machine's port row actually navigates to: straight + /// to its private-network address, over the WireGuard tunnel — never + /// through a provider port-forwarding proxy, which Freestyle's public + /// platform does not offer for arbitrary ports. Deliberately the raw + /// address, not the `.internal` name `cmux vpn hosts` publishes: that name + /// only resolves once `/etc/hosts` has been synced, and a link that only + /// sometimes works is worse than one that always does. An IPv6 literal is + /// bracketed the way every URL scheme requires. + public static func directPortURL(privateAddress: String, port: Int) -> String { + let bracketed = privateAddress.contains(":") ? "[\(privateAddress)]" : privateAddress + return "http://\(bracketed):\(port)" + } + /// Render entries as the managed block's body (no markers): one `ip host` /// line per hostname, sorted for a stable, diffable file. public static func renderBlockBody(_ entries: [Entry]) -> String { diff --git a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/CmuxInternalHostnamesTests.swift b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/CmuxInternalHostnamesTests.swift index fbd4014e9997..6eced43a7178 100644 --- a/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/CmuxInternalHostnamesTests.swift +++ b/Packages/macOS/CmuxFoundation/Tests/CmuxFoundationTests/CmuxInternalHostnamesTests.swift @@ -81,4 +81,20 @@ struct CmuxInternalHostnamesTests { let current = "127.0.0.1 localhost\n" #expect(CmuxInternalHostnames.mergedHostsFile(current: current, body: "") == current) } + + @Test("A port link is always the raw private address, never the .internal name") + func directPortURLUsesTheRawAddress() { + // Deliberately not the `.internal` name: it only resolves once + // `cmux vpn hosts` has synced `/etc/hosts`, and a link that only + // sometimes works is worse than one that always does. + #expect(CmuxInternalHostnames.directPortURL(privateAddress: "10.0.0.2", port: 8000) == "http://10.0.0.2:8000") + } + + @Test("Brackets an IPv6 address the way every URL scheme requires") + func directPortURLBracketsIPv6() { + #expect( + CmuxInternalHostnames.directPortURL(privateAddress: "fd60:1e5e:6720::3", port: 22) + == "http://[fd60:1e5e:6720::3]:22" + ) + } } diff --git a/Sources/Cloud/CloudTreeNode.swift b/Sources/Cloud/CloudTreeNode.swift index 7f0d4b850f40..f8cf2e215f2a 100644 --- a/Sources/Cloud/CloudTreeNode.swift +++ b/Sources/Cloud/CloudTreeNode.swift @@ -45,13 +45,13 @@ final class CloudTreeNode: NSObject { case browser(CloudTreeBrowserRow) /// "Ports" group under a cloud machine. case portsGroup(machine: SurfaceMachineID) - /// The resource plus the URL a person would paste to reach it — - /// `http://.internal:` when the machine has a - /// private address (resolved by the app's own DNS override, so the - /// name only works from a Mac with the tunnel active), else - /// `http://:`, else nil when the machine has no - /// private address to build one from (public-only machines, or one - /// this Mac hasn't attached to yet). + /// The resource plus the URL a click actually opens — + /// `http://:`, reachable over the WireGuard tunnel — + /// or nil when the machine has no private address yet (public-only + /// machines, or one this Mac hasn't attached to yet). Deliberately the + /// raw address, never the `.internal` name: that name only resolves + /// once `cmux vpn hosts` has synced `/etc/hosts`, so a link built from + /// it would work only sometimes. case port(SurfaceResource, url: String?) /// A single explanatory line (asleep, connecting, link error, empty). case placeholder(machine: SurfaceMachineID, CloudTreePlaceholder) @@ -505,12 +505,8 @@ enum CloudTreeNodeBuilder { guard let port, let address = info?.privateAddress else { return nil } // Cloud machines only: the local Mac has no private-network address to // begin with, so it never reaches here with one. - if case .cloud(let id) = machine { - let hostname = CmuxInternalHostnames.hostname(id: id, label: info?.name) - return "http://\(hostname):\(port)" - } - let bracketed = address.contains(":") ? "[\(address)]" : address - return "http://\(bracketed):\(port)" + guard machine.isLocal == false else { return nil } + return CmuxInternalHostnames.directPortURL(privateAddress: address, port: port) } private static func cloudChildren(machine: SurfaceMachineID, info: SurfaceMachineInfo?, snapshot: SurfaceCatalogSnapshot) -> [CloudTreeNode] { diff --git a/Sources/Cloud/CloudTreeRowContentView.swift b/Sources/Cloud/CloudTreeRowContentView.swift index 3d6d15f5da5d..6ab13f5ab46e 100644 --- a/Sources/Cloud/CloudTreeRowContentView.swift +++ b/Sources/Cloud/CloudTreeRowContentView.swift @@ -321,8 +321,11 @@ struct CloudTreeLeafRow: View { } private var titleColor: AnyShapeStyle { - if titleIsLink { return AnyShapeStyle(Color.accentColor) } - return titleDimmed ? AnyShapeStyle(.secondary) : AnyShapeStyle(.primary) + // Underlined-but-primary, not accent-tinted: a port link sits among + // plain-text rows in the same tree, and the accent color read as an + // unrelated highlight rather than "this text is a link" the way the + // underline alone already says. + titleDimmed ? AnyShapeStyle(.secondary) : AnyShapeStyle(.primary) } private func detailText(_ text: String) -> some View { diff --git a/Sources/Cloud/NewMachineSheet.swift b/Sources/Cloud/NewMachineSheet.swift index d169d2d70a70..58a6d564dbaf 100644 --- a/Sources/Cloud/NewMachineSheet.swift +++ b/Sources/Cloud/NewMachineSheet.swift @@ -74,28 +74,6 @@ struct NewMachineSheet: View { .fixedSize(horizontal: false, vertical: true) } } - if model.supportsSize { - GridRow { - label(String(localized: "machines.new.size.label", defaultValue: "Size")) - VStack(alignment: .leading, spacing: 4) { - Picker("", selection: $model.memoryMb) { - ForEach(model.memoryOptions, id: \.self) { mb in - Text(NewMachineModel.memoryLabel(mb: mb)).tag(mb) - } - } - .pickerStyle(.menu) - .labelsHidden() - .frame(maxWidth: 140, alignment: .leading) - .accessibilityIdentifier("NewMachineSheet.size") - Text(String( - localized: "machines.new.size.summary", - defaultValue: "Memory. CPU scales with it." - )) - .cmuxFont(size: 11) - .foregroundStyle(.secondary) - } - } - } if let image = model.selectedImage { GridRow { label(String(localized: "machines.new.image.label", defaultValue: "Image")) diff --git a/Sources/Surfaces/CmuxTuiSnapshotParser.swift b/Sources/Surfaces/CmuxTuiSnapshotParser.swift index c1f7c93a4a32..d4aa9ff0c60f 100644 --- a/Sources/Surfaces/CmuxTuiSnapshotParser.swift +++ b/Sources/Surfaces/CmuxTuiSnapshotParser.swift @@ -384,8 +384,13 @@ struct CmuxTuiSnapshotParser: Sendable { return pool.filter { !($0.kind == .display && pointed.contains($0.id)) } + parsed } - /// A forwarded port, shown as a browser resource. - static func portBrowser(machine: SurfaceMachineID, port: Int) -> SurfaceResource { + /// A forwarded port, shown as a browser resource. `directURL`, when + /// given, is where opening it actually navigates — the machine's private + /// address over the WireGuard tunnel, never a provider port-forwarding + /// proxy (Freestyle's public platform has none for arbitrary ports). nil + /// only for a machine with no private-network address yet, which falls + /// back to the legacy provider-minted-endpoint path. + static func portBrowser(machine: SurfaceMachineID, port: Int, directURL: String? = nil) -> SurfaceResource { SurfaceResource( id: SurfaceResourceID(machine: machine, kind: .browser, key: "port:\(port)"), title: ":\(port)", @@ -394,7 +399,7 @@ struct CmuxTuiSnapshotParser: Sendable { agent: nil, remoteWorkspace: nil, port: port, - url: nil + url: directURL ) } diff --git a/Sources/Surfaces/CmuxTuiSurfaceProviders.swift b/Sources/Surfaces/CmuxTuiSurfaceProviders.swift index 3be9dbf2c6a6..4ac5d04c3b98 100644 --- a/Sources/Surfaces/CmuxTuiSurfaceProviders.swift +++ b/Sources/Surfaces/CmuxTuiSurfaceProviders.swift @@ -1,3 +1,4 @@ +import CmuxFoundation import Foundation /// Owns one ``CmuxTuiSurfaceProvider`` per cloud machine and keeps the catalog's machine @@ -304,8 +305,10 @@ final class CmuxTuiSurfaceProvider: SurfaceProvider { where reconnectableSessionIDs.contains(ObjectIdentifier(session)) { session.reconnect(socketPath: connected.socketPath) } + let privateAddress = summary.preferredPrivateAddress for port in await ports(client: client, force: force) { - resources.append(CmuxTuiSnapshotParser.portBrowser(machine: machine, port: port)) + let directURL = privateAddress.map { CmuxInternalHostnames.directPortURL(privateAddress: $0, port: port) } + resources.append(CmuxTuiSnapshotParser.portBrowser(machine: machine, port: port, directURL: directURL)) } } catch { let status = await links.status(machineID: machineID) @@ -452,7 +455,20 @@ final class CmuxTuiSurfaceProvider: SurfaceProvider { guard let port = resource.port ?? (desktop ? CmuxTuiSnapshotParser.desktopPort : nil) else { throw SurfaceCatalogError.unsupported("browser \(resource.id) has no port") } - if let url = endpointURL(port: port, desktop: desktop) { + // A forwarded-port row (`portBrowser`, id key "port:") already + // carries the URL to open — its own private address over the + // WireGuard tunnel — and must navigate there directly, never + // through the endpoint()/openPort proxy below: Freestyle's public + // platform has no port-forwarding proxy for arbitrary ports, so + // that call fails outright for exactly the machines this exists + // for. A regular daemon browser's `url` is a different thing (the + // remote tab's own address, not a locally-openable link) and must + // still go through the proxy/CDP path, so this only ever fires + // for the id shape `portBrowser` mints. + if !desktop, resource.id.key.hasPrefix("port:"), + let directURLString = resource.url, let directURL = URL(string: directURLString) { + created = try SurfacePaneFactory.makeBrowserPane(url: directURL, at: destination, focus: focus) + } else if let url = endpointURL(port: port, desktop: desktop) { created = try SurfacePaneFactory.makeBrowserPane(url: url, at: destination, focus: focus) } else { // Optimistic: the pane exists before its endpoint does. Minting the preview diff --git a/web/services/vms/drivers/freestyle.ts b/web/services/vms/drivers/freestyle.ts index 7ab92f003c67..315482b66072 100644 --- a/web/services/vms/drivers/freestyle.ts +++ b/web/services/vms/drivers/freestyle.ts @@ -196,20 +196,31 @@ type FreestyleNetworkAddress = { * Private wins unconditionally, and deliberately never falls back: a machine on * a VPC has no public inbound rule, so a public route for it would not be a * degraded path but a guaranteed timeout with a misleading address in the - * error. IPv6 is preferred within the network only because the daemon binds - * `[::]` — the IPv4 address is the honest second choice on a network whose v6 - * allocation was declined, not a fallback for an unreachable v6. + * error. + * + * Within the network, IPv4 is preferred, because only the v4 path is reliable + * over the WireGuard tunnel. The tunnel routes the VPC's v4 prefix as a subnet, + * so it reaches any member the moment that member exists; its v6 path does not + * pick up members created after the tunnel came up. A VM created into an + * established tunnel therefore answers on its private v4 and blackholes on its + * private v6 from the same Mac, while both work VM-to-VM inside the VPC. With + * v6 first, every freshly created machine spent the full 60s connect timeout + * and surfaced as "Command timed out"; only machines predating the tunnel + * connected. Preferring v4 also matches the app's own `preferredPrivateAddress` + * (v4 then v6), so the address a person copies from the sidebar is the address + * the daemon is dialed on. The public fallback below stays v6 — Freestyle + * allocates no public v4 at all. */ export function freestyleCmuxRemoteRoute(addresses: FreestyleRouteAddresses, vmId: string): string { const networks = addresses.vpcs ?? addresses.networks ?? []; - for (const network of networks) { - const ipv6 = network.ipv6?.trim(); - if (ipv6) return `ws://[${ipv6}]:${CMUX_TUI_PORT}/v1/link`; - } for (const network of networks) { const ipv4 = network.ipv4?.trim(); if (ipv4) return `ws://${ipv4}:${CMUX_TUI_PORT}/v1/link`; } + for (const network of networks) { + const ipv6 = network.ipv6?.trim(); + if (ipv6) return `ws://[${ipv6}]:${CMUX_TUI_PORT}/v1/link`; + } if (networks.length > 0) { throw new ProviderError( "freestyle", diff --git a/web/tests/vm-freestyle-provider.test.ts b/web/tests/vm-freestyle-provider.test.ts index 3cf464ae9840..0d56c9d6847b 100644 --- a/web/tests/vm-freestyle-provider.test.ts +++ b/web/tests/vm-freestyle-provider.test.ts @@ -87,8 +87,12 @@ describe("Freestyle platform contract", () => { }); test("cmux-remote route prefers the private VPC address and never falls back from it", () => { - // On a VPC: the private IPv6 wins even when a public address exists, - // because a VPC machine has no public inbound rule. + // On a VPC: the private address wins even when a public address exists, + // because a VPC machine has no public inbound rule. v4 is preferred within + // the network — the tunnel routes the VPC's v4 prefix as a subnet and so + // reaches new members immediately, while its v6 path does not pick up VMs + // created after the tunnel came up, stalling their connect for the full + // timeout. expect( freestyleCmuxRemoteRoute( { @@ -97,14 +101,14 @@ describe("Freestyle platform contract", () => { }, VM_ID, ), - ).toBe("ws://[fd7a:115c:a1e0::a]:1337/v1/link"); - // v4-only membership is the honest second choice, not a public fallback. + ).toBe("ws://10.40.0.10:1337/v1/link"); + // v6-only membership is the honest second choice, not a public fallback. expect( freestyleCmuxRemoteRoute( - { publicIpv6: "2602:f75c:0:1::2a", vpcs: [{ ipv4: "10.40.0.10", ipv6: null }] }, + { publicIpv6: "2602:f75c:0:1::2a", vpcs: [{ ipv4: null, ipv6: "fd7a:115c:a1e0::a" }] }, VM_ID, ), - ).toBe("ws://10.40.0.10:1337/v1/link"); + ).toBe("ws://[fd7a:115c:a1e0::a]:1337/v1/link"); // A membership with no address is unreachable and must say so, not // silently dial a public address the firewall will drop. expect(() =>