diff --git a/Packages/macOS/CmuxCloud/Sources/CmuxCloud/Machines/MachineSnapshot.swift b/Packages/macOS/CmuxCloud/Sources/CmuxCloud/Machines/MachineSnapshot.swift index bbdfff9cbf39..69eed98db315 100644 --- a/Packages/macOS/CmuxCloud/Sources/CmuxCloud/Machines/MachineSnapshot.swift +++ b/Packages/macOS/CmuxCloud/Sources/CmuxCloud/Machines/MachineSnapshot.swift @@ -11,6 +11,7 @@ public struct MachineSnapshot: Equatable, Identifiable, Sendable { capabilities: VMCapabilities = .all, activity: Activity, createdAt: Date? = nil, + createdBy: VMCreator? = nil, label: String? = nil, slug: String? = nil, freeAccess: FreeAccessState = .unrestricted, @@ -26,6 +27,7 @@ public struct MachineSnapshot: Equatable, Identifiable, Sendable { self.capabilities = capabilities self.activity = activity self.createdAt = createdAt + self.createdBy = createdBy self.label = label self.slug = slug self.freeAccess = freeAccess @@ -66,6 +68,9 @@ public struct MachineSnapshot: Equatable, Identifiable, Sendable { public var capabilities: VMCapabilities = .all public let activity: Activity public let createdAt: Date? + /// Who made this machine; nil for machines the surface catalog discovered + /// on its own and on control planes that do not send an author. + public let createdBy: VMCreator? /// User-chosen label; nil when the machine has no label. public let label: String? /// Server-generated three-word name; nil for machines older than naming. diff --git a/Packages/macOS/CmuxCloud/Sources/CmuxCloud/Machines/MachineSnapshotBuilder.swift b/Packages/macOS/CmuxCloud/Sources/CmuxCloud/Machines/MachineSnapshotBuilder.swift index 3a2aa84dd8b5..d8d74e8f51c9 100644 --- a/Packages/macOS/CmuxCloud/Sources/CmuxCloud/Machines/MachineSnapshotBuilder.swift +++ b/Packages/macOS/CmuxCloud/Sources/CmuxCloud/Machines/MachineSnapshotBuilder.swift @@ -45,6 +45,7 @@ public enum MachineSnapshotBuilder: Sendable { capabilities: summary.capabilities, activity: activity(fromStatus: summary.status), createdAt: createdAt, + createdBy: summary.createdBy, label: summary.displayName, slug: summary.slug, freeAccess: freeAccess, diff --git a/Packages/macOS/CmuxCloud/Sources/CmuxCloud/VMClient/VMClient.swift b/Packages/macOS/CmuxCloud/Sources/CmuxCloud/VMClient/VMClient.swift index 45238cd4c198..a8ce9adfac52 100644 --- a/Packages/macOS/CmuxCloud/Sources/CmuxCloud/VMClient/VMClient.swift +++ b/Packages/macOS/CmuxCloud/Sources/CmuxCloud/VMClient/VMClient.swift @@ -280,7 +280,8 @@ public struct VMSummary: Sendable { freeAccessExpiresAt: Int64? = nil, addressIPv4: String? = nil, addressIPv6: String? = nil, - cmuxTuiContract: String? = nil + cmuxTuiContract: String? = nil, + createdBy: VMCreator? = nil ) { self.id = id self.provider = provider @@ -296,6 +297,7 @@ public struct VMSummary: Sendable { self.addressIPv4 = addressIPv4 self.addressIPv6 = addressIPv6 self.cmuxTuiContract = cmuxTuiContract + self.createdBy = createdBy } public let id: String @@ -310,6 +312,9 @@ public struct VMSummary: Sendable { public var capabilities: VMCapabilities = .all /// User-chosen label; the id stays the machine's address. public var displayName: String? + /// Who made this machine (`GET /api/vm` → `createdBy`). Nil when the + /// control plane does not send one. Display only; see ``VMCreator``. + public var createdBy: VMCreator? /// Server-generated three-word name (`sleepy-teal-otter`), fixed for the /// machine's life and unique among the owner's live machines. Nil on /// machines created before the backend assigned names. @@ -1158,6 +1163,7 @@ public actor VMClient { summary.displayName = label } summary.slug = (dict["slug"] as? String).flatMap { $0.isEmpty ? nil : $0 } + summary.createdBy = VMCreator(vmResponse: dict) summary.freeAccessExpiresAt = Self.epochMilliseconds(dict["freeAccessExpiresAt"]) if let address = dict["address"] as? [String: Any] { summary.addressIPv4 = (address["ipv4"] as? String).flatMap { $0.isEmpty ? nil : $0 } @@ -1587,6 +1593,14 @@ public actor VMClient { summary.capabilities = VMCapabilities(vmResponse: obj) summary.displayName = (obj["displayName"] as? String).flatMap { $0.isEmpty ? nil : $0 } summary.slug = (obj["slug"] as? String).flatMap { $0.isEmpty ? nil : $0 } + // `create` has one caller today, the `vm.create` socket method, so + // this is what `cmux vm new --json` prints. The sidebar does not + // read it: the panel only ever assigns a whole `listPage()` result, + // so a created machine shows its author on the next list refresh + // and not before. Decoded here anyway because the field is in the + // response and a client that did merge this into the row it already + // listed would otherwise blank the author out. + summary.createdBy = VMCreator(vmResponse: obj) // The create receipt names the new machine's private address and // attach contract, so the app can register and dial it without a // fleet re-read or an attach request (see createdMachineAttach). @@ -1668,6 +1682,11 @@ public actor VMClient { summary.displayName = label } summary.slug = (obj["slug"] as? String).flatMap { $0.isEmpty ? nil : $0 } + // Same as the create site: `status(id:)`'s one caller is the + // `vm.status` socket method, so this feeds `cmux vm status --json`. + // The panel's per-machine refresh goes through `SurfaceCatalog`, + // not through here. + summary.createdBy = VMCreator(vmResponse: obj) if let address = obj["address"] as? [String: Any] { summary.addressIPv4 = (address["ipv4"] as? String).flatMap { $0.isEmpty ? nil : $0 } summary.addressIPv6 = (address["ipv6"] as? String).flatMap { $0.isEmpty ? nil : $0 } diff --git a/Packages/macOS/CmuxCloud/Sources/CmuxCloud/VMClient/VMCreator.swift b/Packages/macOS/CmuxCloud/Sources/CmuxCloud/VMClient/VMCreator.swift new file mode 100644 index 000000000000..7542483e4390 --- /dev/null +++ b/Packages/macOS/CmuxCloud/Sources/CmuxCloud/VMClient/VMCreator.swift @@ -0,0 +1,50 @@ +import Foundation + +/// Who made a Cloud machine, for display only. +/// +/// `/api/vm` is scoped by owner team, so on a team every member sees every +/// member's machines. Without this the sidebar shows a pile of generated +/// three-word names with nothing to tell them apart by. +/// +/// `userId` is the stable part and is always present. `displayName` is nil +/// when nothing has recorded a name for that account: the backend writes its +/// identity snapshot when an account resolves and deletes it when that account +/// revokes a lease, so a teammate who just revoked one reads as unnamed until +/// they next sign in. The id is kept in that case so rows by the same person +/// still group together instead of collapsing into one anonymous bucket. +/// +/// Nothing here is an authorization input. The caller is already entitled to +/// every machine it was sent. +public struct VMCreator: Equatable, Hashable, Sendable { + public init(userId: String, displayName: String? = nil) { + self.userId = userId + self.displayName = displayName + } + + public let userId: String + + /// Nil when no name is known. Never the raw account id: an opaque id in + /// place of a name is the same unreadable list this is meant to fix. + public let displayName: String? +} + +extension VMCreator { + /// Reads `createdBy` out of one `GET /api/vm` item. + /// + /// Returns nil for a missing or blank id, which covers both a control + /// plane that predates the field and a malformed entry. An author is + /// decoration on a row, so a bad one is dropped rather than failing the + /// whole list decode the way a missing `id` or `provider` does. + public init?(vmResponse: [String: Any]) { + guard let raw = vmResponse["createdBy"] as? [String: Any] else { return nil } + guard let userId = (raw["userId"] as? String)?.trimmingCharacters(in: .whitespacesAndNewlines), + !userId.isEmpty + else { return nil } + let displayName = (raw["displayName"] as? String)? + .trimmingCharacters(in: .whitespacesAndNewlines) + self.init( + userId: userId, + displayName: displayName.flatMap { $0.isEmpty ? nil : $0 } + ) + } +} diff --git a/Sources/Cloud/VMClientSocketCommands.swift b/Sources/Cloud/VMClientSocketCommands.swift index 36ed7d31634f..29a6e04cd077 100644 --- a/Sources/Cloud/VMClientSocketCommands.swift +++ b/Sources/Cloud/VMClientSocketCommands.swift @@ -780,7 +780,9 @@ extension TerminalController { return .success(kind) } - private nonisolated static func socketWorkerVMSummaryPayload(_ vm: VMSummary) -> [String: Any] { + /// Internal rather than private so `CloudMachineCreatorTests` can check + /// that a relayed client is sent the same machine facts a direct one gets. + nonisolated static func socketWorkerVMSummaryPayload(_ vm: VMSummary) -> [String: Any] { var payload: [String: Any] = [ "id": vm.id, "provider": vm.provider, @@ -808,6 +810,19 @@ extension TerminalController { if let slug = vm.slug, !slug.isEmpty { payload["slug"] = slug } + if let createdBy = vm.createdBy { + // A known account with no recorded name sends an explicit null for + // the name, the same shape the HTTP response uses, so a consumer + // written against `/api/vm` decodes this without a second case. + // No author at all sends no key, where the backend sends + // `"createdBy": null`; both live readers treat absent and null the + // same, so this is narrower than the wire format rather than a + // second meaning. + payload["createdBy"] = [ + "userId": createdBy.userId, + "displayName": createdBy.displayName.map { $0 as Any } ?? NSNull(), + ] + } if let freeAccessExpiresAt = vm.freeAccessExpiresAt { payload["freeAccessExpiresAt"] = freeAccessExpiresAt } diff --git a/cmux.xcodeproj/project.pbxproj b/cmux.xcodeproj/project.pbxproj index c743a5633a58..8b5f2335d874 100644 --- a/cmux.xcodeproj/project.pbxproj +++ b/cmux.xcodeproj/project.pbxproj @@ -738,6 +738,7 @@ C12542000000000000000002 /* CloudInitialWorkspaceNamingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C12542000000000000000001 /* CloudInitialWorkspaceNamingTests.swift */; }; 8E7B97505657BF3E66091D9B /* CloudLinkRetryBackoffTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = FF5E22859B10B563032FECE7 /* CloudLinkRetryBackoffTests.swift */; }; 7A0CE1000000000000000732 /* CloudLoopbackPortForwardTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A0CE1000000000000000731 /* CloudLoopbackPortForwardTests.swift */; }; + A44FBE43FCD42CFEEB6EAF6F /* CloudMachineCreatorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E6CE8A534885E9CE99D3F84 /* CloudMachineCreatorTests.swift */; }; 9E21585251BFCA0BAF06B10B /* CloudMachineDeleteOptimismTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5DEADB23282C5C0D06630294 /* CloudMachineDeleteOptimismTests.swift */; }; A13086000000000000000002 /* CloudMachineDragSourceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A13086000000000000000001 /* CloudMachineDragSourceTests.swift */; }; C6428DFF1BF84994BE7F09B1 /* CloudMachineLoadingReservation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 95D98692372B4596B873883C /* CloudMachineLoadingReservation.swift */; }; @@ -5035,6 +5036,7 @@ C12542000000000000000001 /* CloudInitialWorkspaceNamingTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CloudInitialWorkspaceNamingTests.swift"; sourceTree = ""; }; FF5E22859B10B563032FECE7 /* CloudLinkRetryBackoffTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CloudLinkRetryBackoffTests.swift"; sourceTree = ""; }; 7A0CE1000000000000000731 /* CloudLoopbackPortForwardTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CloudLoopbackPortForwardTests.swift; sourceTree = ""; }; + 9E6CE8A534885E9CE99D3F84 /* CloudMachineCreatorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CloudMachineCreatorTests.swift"; sourceTree = ""; }; 5DEADB23282C5C0D06630294 /* CloudMachineDeleteOptimismTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CloudMachineDeleteOptimismTests.swift"; sourceTree = ""; }; A13086000000000000000001 /* CloudMachineDragSourceTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CloudMachineDragSourceTests.swift; sourceTree = ""; }; 95D98692372B4596B873883C /* CloudMachineLoadingReservation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CloudMachineLoadingReservation.swift; sourceTree = ""; }; @@ -12667,6 +12669,7 @@ 311C986A1E7430418708AAA6 /* MachinesListStatusToolbarRowTests.swift */, 1C38B9AFEEE4FAE6089C250F /* CloudTreeRowToolTipTests.swift */, 6F27EA9972F21B98FB4A7312 /* SurfaceCatalogObservationTests.swift */, + 9E6CE8A534885E9CE99D3F84 /* CloudMachineCreatorTests.swift */, C4C4607FC6F461CC8B35343E /* AgentHibernationBackgroundWorkTests.swift */, A29C9B70624D86CFFD27EFEE /* CmuxEventSequenceStoreTests.swift */, BCAE531397347899AA9F4E4E /* CloudCursorlessSnapshotTests.swift */, @@ -16572,6 +16575,7 @@ C12542000000000000000002 /* CloudInitialWorkspaceNamingTests.swift in Sources */, 8E7B97505657BF3E66091D9B /* CloudLinkRetryBackoffTests.swift in Sources */, 7A0CE1000000000000000732 /* CloudLoopbackPortForwardTests.swift in Sources */, + A44FBE43FCD42CFEEB6EAF6F /* CloudMachineCreatorTests.swift in Sources */, 9E21585251BFCA0BAF06B10B /* CloudMachineDeleteOptimismTests.swift in Sources */, A13086000000000000000002 /* CloudMachineDragSourceTests.swift in Sources */, 62D6740CB59ACBE3EED47A27 /* CloudMachineNotificationDeliveryTests.swift in Sources */, diff --git a/cmuxTests/CloudMachineCreatorTests.swift b/cmuxTests/CloudMachineCreatorTests.swift new file mode 100644 index 000000000000..909a80af8940 --- /dev/null +++ b/cmuxTests/CloudMachineCreatorTests.swift @@ -0,0 +1,152 @@ +import CmuxCloud +import Foundation +import Testing + +#if canImport(cmux_DEV) +@testable import cmux_DEV +#elseif canImport(cmux) +@testable import cmux +#endif + +/// `/api/vm` publishes who made each machine. The sidebar's whole complaint is +/// that a team's fleet reads as a pile of generated three-word names with no +/// way to tell whose is whose, so the author has to survive both hops between +/// the response and the row: the list decode and the snapshot the row renders +/// from. The socket payload is the third place the same facts are published, +/// to the CLI and remote clients rather than to the sidebar. +/// `.serialized` because the network tests here drive the same process-global +/// `CloudRefreshURLProtocol.responses` as `VMClientReadCoalescingTests`, which +/// carries the trait for the same reason: a `reset()` landing between another +/// test's `configure` and its request start serves the wrong body. The trait +/// only orders this suite's own tests; the two suites do not overlap because CI +/// runs with `-parallel-testing-enabled NO`. +@Suite("Cloud machine creator metadata", .serialized) +struct CloudMachineCreatorTests { + @MainActor + @Test("the machine list decodes the author the backend sends") + func listDecodesCreator() async throws { + let fixture = try await CloudRefreshFixture.make() + defer { fixture.session.invalidateAndCancel() } + await CloudRefreshURLProtocol.reset() + await CloudRefreshURLProtocol.configure(.authoredList) + let page = try await fixture.client.listPage() + let byID = Dictionary(uniqueKeysWithValues: page.vms.map { ($0.id, $0) }) + + let named = try #require(byID["fixture-0"]?.createdBy) + #expect(named.userId == "user-a") + #expect(named.displayName == "Ada Lovelace") + + // An account nothing has recorded a name for. The id still arrives, so + // rows by the same person group together even while they read + // "Unknown"; dropping the creator here would lose that. + let anonymous = try #require(byID["fixture-1"]?.createdBy) + #expect(anonymous.userId == "user-b") + #expect(anonymous.displayName == nil) + + // A control plane that predates the field. Absent, not "Unknown". + #expect(byID["fixture-2"]?.createdBy == nil) + } + + /// `create` and `status(id:)` each have exactly one caller, the matching + /// socket method, so what they decode is what `cmux vm new --json` and + /// `cmux vm status --json` print. Neither feeds the sidebar: the panel only + /// ever assigns a whole list result. Pinned so the two endpoints stay + /// consistent with the list rather than each carrying a different subset of + /// the response. + @MainActor + @Test("the single-machine endpoints carry the author too") + func singleMachineReadsCarryCreator() async throws { + let fixture = try await CloudRefreshFixture.make() + defer { fixture.session.invalidateAndCancel() } + await CloudRefreshURLProtocol.reset() + await CloudRefreshURLProtocol.configure(.authoredList) + + let created = try await fixture.client.create(idempotencyKey: UUID().uuidString) + #expect(created.createdBy?.userId == "user-a") + #expect(created.createdBy?.displayName == "Ada Lovelace") + + let read = try await fixture.client.status(id: "fixture-9") + #expect(read.createdBy?.userId == "user-a") + #expect(read.createdBy?.displayName == "Ada Lovelace") + } + + @Test("a blank author id decodes as no author") + func blankCreatorIsNoCreator() { + #expect(VMCreator(vmResponse: ["createdBy": ["userId": " ", "displayName": "Ada"]]) == nil) + #expect(VMCreator(vmResponse: ["createdBy": NSNull()]) == nil) + #expect(VMCreator(vmResponse: [:]) == nil) + // A name of only whitespace is no name, not a row that renders blank. + #expect(VMCreator(vmResponse: ["createdBy": ["userId": "user-a", "displayName": " "]])?.displayName == nil) + } + + @Test("the row's snapshot carries the author through the builder") + func snapshotCarriesCreator() { + let summary = VMSummary( + id: "vm-1", + provider: "fixture", + status: "running", + image: "desktop-vnc", + createdAt: 1_700_000_000_000, + createdBy: VMCreator(userId: "user-a", displayName: "Ada Lovelace") + ) + let snapshot = MachineSnapshotBuilder.snapshot(from: summary) + #expect(snapshot.createdBy?.userId == "user-a") + #expect(snapshot.createdBy?.displayName == "Ada Lovelace") + #expect(snapshot.createdAt == Date(timeIntervalSince1970: 1_700_000_000)) + } + + /// Machines the surface catalog found before the list named them have no + /// author to show, and must not borrow one. + @Test("catalog-discovered machines have no author") + func catalogMachinesHaveNoCreator() { + let summary = VMSummary( + id: "vm-1", provider: "fixture", status: "running", image: "desktop-vnc", createdAt: 0 + ) + #expect(MachineSnapshotBuilder.snapshot(from: summary).createdBy == nil) + } + + /// `vm.list` over the socket is how the CLI and remote clients see the + /// fleet: they never touch `/api/vm` themselves, they get this. Dropping + /// the author here would leave `cmux` on the command line unable to answer + /// a question the sidebar beside it can. + @Test("the socket payload re-publishes the author") + func socketPayloadCarriesCreator() throws { + let payload = TerminalController.socketWorkerVMSummaryPayload( + VMSummary( + id: "vm-1", + provider: "fixture", + status: "running", + image: "desktop-vnc", + createdAt: 0, + createdBy: VMCreator(userId: "user-a", displayName: "Ada Lovelace") + ) + ) + let createdBy = try #require(payload["createdBy"] as? [String: Any]) + #expect(createdBy["userId"] as? String == "user-a") + #expect(createdBy["displayName"] as? String == "Ada Lovelace") + + // No author sends no key. The backend sends an explicit null here, and + // both readers treat absent and null alike, so this is the narrower of + // the two shapes rather than a different meaning. + let anonymous = TerminalController.socketWorkerVMSummaryPayload( + VMSummary(id: "vm-2", provider: "fixture", status: "running", image: "desktop-vnc", createdAt: 0) + ) + #expect(anonymous["createdBy"] == nil) + + // A known account with no recorded name keeps the id and sends an + // explicit null, so the relayed row still groups with its siblings. + let unnamed = TerminalController.socketWorkerVMSummaryPayload( + VMSummary( + id: "vm-3", + provider: "fixture", + status: "running", + image: "desktop-vnc", + createdAt: 0, + createdBy: VMCreator(userId: "user-b", displayName: nil) + ) + ) + let unnamedCreator = try #require(unnamed["createdBy"] as? [String: Any]) + #expect(unnamedCreator["userId"] as? String == "user-b") + #expect(unnamedCreator["displayName"] is NSNull) + } +} diff --git a/cmuxTests/CloudRefreshURLProtocol.swift b/cmuxTests/CloudRefreshURLProtocol.swift index 54187fcab635..0d2bbaf53e57 100644 --- a/cmuxTests/CloudRefreshURLProtocol.swift +++ b/cmuxTests/CloudRefreshURLProtocol.swift @@ -3,7 +3,10 @@ import Foundation /// URLProtocol's synchronous callbacks hand off to one actor; only that actor /// reads the fixture state or calls the client, including after stopLoading. final class CloudRefreshURLProtocol: URLProtocol, @unchecked Sendable { - enum Behavior: Sendable { case normal, statsUnavailable, listUnavailable, throttled } + /// `authoredList` serves the three author shapes `/api/vm` can send: a + /// named account, an account with no recorded name, and (on a control + /// plane that predates the field) no author at all. + enum Behavior: Sendable { case normal, statsUnavailable, listUnavailable, throttled, authoredList } private static let responses = Responses() /// Fixture state is keyed per request, not by object address: URLSession /// frees a finished protocol, and the next request can reuse its address, @@ -66,6 +69,7 @@ final class CloudRefreshURLProtocol: URLProtocol, @unchecked Sendable { let key = source.requestID guard !stoppedRequests.contains(key) else { return } let path = source.request.url!.path + let method = source.request.httpMethod ?? "GET" counts[path, default: 0] += 1 let count = counts.values.reduce(0, +) let ready = startWaiters.filter { $0.0 <= count } @@ -85,6 +89,21 @@ final class CloudRefreshURLProtocol: URLProtocol, @unchecked Sendable { body = #"{"teamId":"fixture-team","kind":"ready","periodDays":30,"machines":[]}"# } else if path.hasSuffix("/stats") { body = #"{"state":"awake","cpus":2}"# + } else if behavior == .authoredList, path != "/api/vm" || method == "POST" { + // A single machine, shaped as the create receipt and the + // status read both are: the machine's fields at the top + // level rather than inside `vms`. + body = #""" + {"id":"fixture-9","provider":"fixture","image":"desktop-vnc","status":"running","createdAt":0,"createdBy":{"userId":"user-a","displayName":"Ada Lovelace"}} + """# + } else if behavior == .authoredList { + body = #""" + {"vms":[ + {"id":"fixture-0","provider":"fixture","image":"desktop-vnc","status":"running","createdAt":0,"capabilities":{"stats":true},"createdBy":{"userId":"user-a","displayName":"Ada Lovelace"}}, + {"id":"fixture-1","provider":"fixture","image":"desktop-vnc","status":"running","createdAt":0,"capabilities":{"stats":true},"createdBy":{"userId":"user-b","displayName":null}}, + {"id":"fixture-2","provider":"fixture","image":"desktop-vnc","status":"running","createdAt":0,"capabilities":{"stats":true}} + ]} + """# } else { body = #"{"vms":[{"id":"fixture-0","provider":"fixture","image":"desktop-vnc","status":"running","createdAt":0,"capabilities":{"stats":true}}]}"# }