Skip to content

Commit 972f269

Browse files
ethanndicksonethanndickson
committed
review
1 parent ce1883e commit 972f269

File tree

13 files changed

+301
-56
lines changed

13 files changed

+301
-56
lines changed

‎Coder Desktop/Coder Desktop/About.swift‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import SwiftUI
22

33
enum About {
4+
public static let repo: String = "https://github.com/coder/coder-desktop-macos"
45
private static var credits: NSAttributedString {
56
let coder = NSMutableAttributedString(
67
string: "Coder.com",
@@ -21,7 +22,7 @@ enum About {
2122
string: "GitHub",
2223
attributes: [
2324
.foregroundColor: NSColor.labelColor,
24-
.link: NSURL(string: "https://github.com/coder/coder-desktop-macos")!,
25+
.link: NSURL(string: About.repo)!,
2526
.font: NSFont.systemFont(ofSize: NSFont.systemFontSize),
2627
]
2728
)

‎Coder Desktop/Coder Desktop/Preview Content/PreviewVPN.swift‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,27 +3,27 @@ import SwiftUI
33

44
@MainActor
55
final class PreviewVPN: Coder_Desktop.VPNService {
6-
@Published var state: Coder_Desktop.VPNServiceState = .disabled
6+
@Published var state: Coder_Desktop.VPNServiceState = .connected
77
@Published var menuState: VPNMenuState = .init(agents: [
8-
UUID(): Agent(id: UUID(), name: "dev", status: .error, copyableDNS: "asdf.coder", wsName: "dogfood2",
8+
UUID(): Agent(id: UUID(), name: "dev", status: .error, hosts: ["asdf.coder"], wsName: "dogfood2",
99
wsID: UUID()),
10-
UUID(): Agent(id: UUID(), name: "dev", status: .okay, copyableDNS: "asdf.coder",
10+
UUID(): Agent(id: UUID(), name: "dev", status: .okay, hosts: ["asdf.coder"],
1111
wsName: "testing-a-very-long-name", wsID: UUID()),
12-
UUID(): Agent(id: UUID(), name: "dev", status: .warn, copyableDNS: "asdf.coder", wsName: "opensrc",
12+
UUID(): Agent(id: UUID(), name: "dev", status: .warn, hosts: ["asdf.coder"], wsName: "opensrc",
1313
wsID: UUID()),
14-
UUID(): Agent(id: UUID(), name: "dev", status: .off, copyableDNS: "asdf.coder", wsName: "gvisor",
14+
UUID(): Agent(id: UUID(), name: "dev", status: .off, hosts: ["asdf.coder"], wsName: "gvisor",
1515
wsID: UUID()),
16-
UUID(): Agent(id: UUID(), name: "dev", status: .off, copyableDNS: "asdf.coder", wsName: "example",
16+
UUID(): Agent(id: UUID(), name: "dev", status: .off, hosts: ["asdf.coder"], wsName: "example",
1717
wsID: UUID()),
18-
UUID(): Agent(id: UUID(), name: "dev", status: .error, copyableDNS: "asdf.coder", wsName: "dogfood2",
18+
UUID(): Agent(id: UUID(), name: "dev", status: .error, hosts: ["asdf.coder"], wsName: "dogfood2",
1919
wsID: UUID()),
20-
UUID(): Agent(id: UUID(), name: "dev", status: .okay, copyableDNS: "asdf.coder",
20+
UUID(): Agent(id: UUID(), name: "dev", status: .okay, hosts: ["asdf.coder"],
2121
wsName: "testing-a-very-long-name", wsID: UUID()),
22-
UUID(): Agent(id: UUID(), name: "dev", status: .warn, copyableDNS: "asdf.coder", wsName: "opensrc",
22+
UUID(): Agent(id: UUID(), name: "dev", status: .warn, hosts: ["asdf.coder"], wsName: "opensrc",
2323
wsID: UUID()),
24-
UUID(): Agent(id: UUID(), name: "dev", status: .off, copyableDNS: "asdf.coder", wsName: "gvisor",
24+
UUID(): Agent(id: UUID(), name: "dev", status: .off, hosts: ["asdf.coder"], wsName: "gvisor",
2525
wsID: UUID()),
26-
UUID(): Agent(id: UUID(), name: "dev", status: .off, copyableDNS: "asdf.coder", wsName: "example",
26+
UUID(): Agent(id: UUID(), name: "dev", status: .off, hosts: ["asdf.coder"], wsName: "example",
2727
wsID: UUID()),
2828
], workspaces: [:])
2929
let shouldFail: Bool

‎Coder Desktop/Coder Desktop/VPNMenuState.swift‎

Lines changed: 41 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ struct Agent: Identifiable, Equatable, Comparable {
66
let id: UUID
77
let name: String
88
let status: AgentStatus
9-
let copyableDNS: String
9+
let hosts: [String]
1010
let wsName: String
1111
let wsID: UUID
1212

@@ -17,6 +17,9 @@ struct Agent: Identifiable, Equatable, Comparable {
1717
}
1818
return lhs.wsName.localizedCompare(rhs.wsName) == .orderedAscending
1919
}
20+
21+
// Hosts arrive sorted by length, the shortest looks best in the UI.
22+
var primaryHost: String? { hosts.first }
2023
}
2124

2225
enum AgentStatus: Int, Equatable, Comparable {
@@ -42,7 +45,7 @@ enum AgentStatus: Int, Equatable, Comparable {
4245
struct Workspace: Identifiable, Equatable, Comparable {
4346
let id: UUID
4447
let name: String
45-
var agents: [UUID]
48+
var agents: Set<UUID>
4649

4750
static func < (lhs: Workspace, rhs: Workspace) -> Bool {
4851
lhs.name.localizedCompare(rhs.name) == .orderedAscending
@@ -52,42 +55,63 @@ struct Workspace: Identifiable, Equatable, Comparable {
5255
struct VPNMenuState {
5356
var agents: [UUID: Agent] = [:]
5457
var workspaces: [UUID: Workspace] = [:]
58+
// Upserted agents that don't belong to any known workspace, have no FQDNs,
59+
// or have any invalid UUIDs.
60+
var invalidAgents: [Vpn_Agent] = []
5561

5662
mutating func upsertAgent(_ agent: Vpn_Agent) {
57-
guard let id = UUID(uuidData: agent.id) else { return }
58-
guard let wsID = UUID(uuidData: agent.workspaceID) else { return }
63+
guard
64+
let id = UUID(uuidData: agent.id),
65+
let wsID = UUID(uuidData: agent.workspaceID),
66+
var workspace = workspaces[wsID],
67+
!agent.fqdn.isEmpty
68+
else {
69+
invalidAgents.append(agent)
70+
return
71+
}
5972
// An existing agent with the same name, belonging to the same workspace
6073
// is from a previous workspace build, and should be removed.
6174
agents.filter { $0.value.name == agent.name && $0.value.wsID == wsID }
6275
.forEach { agents[$0.key] = nil }
63-
workspaces[wsID]?.agents.append(id)
64-
let wsName = workspaces[wsID]?.name ?? "Unknown Workspace"
76+
workspace.agents.insert(id)
77+
workspaces[wsID] = workspace
78+
6579
agents[id] = Agent(
6680
id: id,
6781
name: agent.name,
6882
// If last handshake was not within last five minutes, the agent is unhealthy
6983
status: agent.lastHandshake.date > Date.now.addingTimeInterval(-300) ? .okay : .warn,
70-
// Choose the shortest hostname, and remove trailing dot if present
71-
copyableDNS: agent.fqdn.min(by: { $0.count < $1.count })
72-
.map { $0.hasSuffix(".") ? String($0.dropLast()) : $0 } ?? "UNKNOWN",
73-
wsName: wsName,
84+
// Remove trailing dot if present
85+
hosts: agent.fqdn.map { $0.hasSuffix(".") ? String($0.dropLast()) : $0 },
86+
wsName: workspace.name,
7487
wsID: wsID
7588
)
7689
}
7790

7891
mutating func deleteAgent(withId id: Data) {
79-
guard let id = UUID(uuidData: id) else { return }
92+
guard let agentUUID = UUID(uuidData: id) else { return }
8093
// Update Workspaces
81-
if let agent = agents[id], var ws = workspaces[agent.wsID] {
82-
ws.agents.removeAll { $0 == id }
94+
if let agent = agents[agentUUID], var ws = workspaces[agent.wsID] {
95+
ws.agents.remove(agentUUID)
8396
workspaces[agent.wsID] = ws
8497
}
85-
agents[id] = nil
98+
agents[agentUUID] = nil
99+
// Remove from invalid agents if present
100+
invalidAgents.removeAll { invalidAgent in
101+
invalidAgent.id == id
102+
}
86103
}
87104

88105
mutating func upsertWorkspace(_ workspace: Vpn_Workspace) {
89-
guard let id = UUID(uuidData: workspace.id) else { return }
90-
workspaces[id] = Workspace(id: id, name: workspace.name, agents: [])
106+
guard let wsID = UUID(uuidData: workspace.id) else { return }
107+
workspaces[wsID] = Workspace(id: wsID, name: workspace.name, agents: [])
108+
// Check if we can associate any invalid agents with this workspace
109+
invalidAgents.filter { agent in
110+
agent.workspaceID == workspace.id
111+
}.forEach { agent in
112+
invalidAgents.removeAll { $0 == agent }
113+
upsertAgent(agent)
114+
}
91115
}
92116

93117
mutating func deleteWorkspace(withId id: Data) {
@@ -100,7 +124,7 @@ struct VPNMenuState {
100124
workspaces[wsID] = nil
101125
}
102126

103-
func sorted() -> [VPNMenuItem] {
127+
var sorted: [VPNMenuItem] {
104128
var items = agents.values.map { VPNMenuItem.agent($0) }
105129
// Workspaces with no agents are shown as offline
106130
items += workspaces.filter { _, value in

‎Coder Desktop/Coder Desktop/Views/Agents.swift‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,24 @@ struct Agents<VPN: VPNService, S: Session>: View {
1212
Group {
1313
// Agents List
1414
if vpn.state == .connected {
15-
let items = vpn.menuState.sorted()
16-
let visibleItems = viewAll ? items[...] : items.prefix(defaultVisibleRows)
15+
let items = vpn.menuState.sorted
16+
let visibleOnlineItems = items.prefix(defaultVisibleRows) {
17+
$0.status != .off
18+
}
19+
let visibleItems = viewAll ? items[...] : visibleOnlineItems
1720
ForEach(visibleItems, id: \.id) { agent in
1821
MenuItemView(item: agent, baseAccessURL: session.baseAccessURL!)
1922
.padding(.horizontal, Theme.Size.trayMargin)
2023
}
21-
if items.count > defaultVisibleRows {
24+
if visibleItems.count == 0 {
25+
Text("No \(items.count > 0 ? "running " : "")workspaces!")
26+
.font(.body)
27+
.foregroundColor(.gray)
28+
.padding(.horizontal, Theme.Size.trayInset)
29+
.padding(.top, 2)
30+
}
31+
// Only show the toggle if there are more items to show
32+
if visibleOnlineItems.count < items.count {
2233
Toggle(isOn: $viewAll) {
2334
Text(viewAll ? "Show less" : "Show all")
2435
.font(.headline)

‎Coder Desktop/Coder Desktop/Views/ButtonRow.swift‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
import SwiftUI
22

33
struct ButtonRowView<Label: View>: View {
4+
init(highlightColor: Color = .accentColor, isSelected: Bool = false, label: @escaping () -> Label) {
5+
self.highlightColor = highlightColor
6+
self.isSelected = isSelected
7+
self.label = label
8+
}
9+
10+
let highlightColor: Color
411
@State private var isSelected: Bool = false
512
@ViewBuilder var label: () -> Label
613

@@ -12,8 +19,8 @@ struct ButtonRowView<Label: View>: View {
1219
.padding(.horizontal, Theme.Size.trayPadding)
1320
.frame(minHeight: 22)
1421
.frame(maxWidth: .infinity, alignment: .leading)
15-
.foregroundStyle(isSelected ? Color.white : .primary)
16-
.background(isSelected ? Color.accentColor.opacity(0.8) : .clear)
22+
.foregroundStyle(isSelected ? .white : .primary)
23+
.background(isSelected ? highlightColor.opacity(0.8) : .clear)
1724
.clipShape(.rect(cornerRadius: Theme.Size.rectCornerRadius))
1825
.onHover { hovering in isSelected = hovering }
1926
}

‎Coder Desktop/Coder Desktop/Views/InvalidAgents.swift‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import SwiftUI
2+
import VPNLib
3+
4+
struct InvalidAgentsButton<VPN: VPNService>: View {
5+
@Environment(\.dismiss) var dismiss
6+
@EnvironmentObject var vpn: VPN
7+
var msg: String {
8+
"\(vpn.menuState.invalidAgents.count) invalid \(vpn.menuState.invalidAgents.count > 1 ? "agents" : "agent").."
9+
}
10+
11+
var body: some View {
12+
Button {
13+
showAlert()
14+
} label: {
15+
ButtonRowView(highlightColor: .red) { Text(msg) }
16+
}.buttonStyle(.plain)
17+
}
18+
19+
// `.alert` from SwiftUI doesn't play nice when the calling view is in the
20+
// menu bar.
21+
private func showAlert() {
22+
let formattedAgents = vpn.menuState.invalidAgents.map { agent in
23+
let agent_id = if let agent_id = UUID(uuidData: agent.id) {
24+
agent_id.uuidString
25+
} else {
26+
"Invalid ID: \(agent.id.base64EncodedString())"
27+
}
28+
let wsID = if let wsID = UUID(uuidData: agent.workspaceID) {
29+
wsID.uuidString
30+
} else {
31+
"Invalid ID: \(agent.workspaceID.base64EncodedString())"
32+
}
33+
let lastHandshake = agent.hasLastHandshake ? "\(agent.lastHandshake)" : "Never"
34+
return """
35+
Agent Name: \(agent.name)
36+
ID: \(agent_id)
37+
Workspace ID: \(wsID)
38+
Last Handshake: \(lastHandshake)
39+
FQDNs: \(agent.fqdn)
40+
Addresses: \(agent.ipAddrs)
41+
"""
42+
}.joined(separator: "\n\n")
43+
44+
let alert = NSAlert()
45+
alert.messageText = "Invalid Agents"
46+
alert.informativeText = """
47+
Coder Desktop received invalid agents from the VPN. This should
48+
never happen. Please open an issue on \(About.repo).
49+
50+
\(formattedAgents)
51+
"""
52+
alert.alertStyle = .warning
53+
dismiss()
54+
alert.runModal()
55+
}
56+
}

‎Coder Desktop/Coder Desktop/Views/Util.swift‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,11 @@ extension UUID {
3131
self.init(uuid: uuid)
3232
}
3333
}
34+
35+
extension Array {
36+
func prefix(_ maxCount: Int, while predicate: (Element) -> Bool) -> ArraySlice<Element> {
37+
let failureIndex = enumerated().first(where: { !predicate($0.element) })?.offset ?? count
38+
let endIndex = Swift.min(failureIndex, maxCount)
39+
return self[..<endIndex]
40+
}
41+
}

‎Coder Desktop/Coder Desktop/Views/VPNMenu.swift‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,13 @@ struct VPNMenu<VPN: VPNService, S: Session>: View {
3737
// Trailing stack
3838
VStack(alignment: .leading, spacing: 3) {
3939
TrayDivider()
40+
if vpn.state == .connected, !vpn.menuState.invalidAgents.isEmpty {
41+
InvalidAgentsButton<VPN>()
42+
}
4043
if session.hasSession {
4144
Link(destination: session.baseAccessURL!.appending(path: "templates")) {
4245
ButtonRowView {
4346
Text("Create workspace")
44-
EmptyView()
4547
}
4648
}.buttonStyle(.plain)
4749
TrayDivider()

‎Coder Desktop/Coder Desktop/Views/VPNMenuItem.swift‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,17 @@ struct MenuItemView: View {
4747
@State private var nameIsSelected: Bool = false
4848
@State private var copyIsSelected: Bool = false
4949

50-
private var fmtWsName: AttributedString {
51-
var formattedName = AttributedString(item.wsName)
50+
private var itemName: AttributedString {
51+
let name = switch item {
52+
case let .agent(agent): agent.primaryHost ?? "\(item.wsName).coder"
53+
case .offlineWorkspace: "\(item.wsName).coder"
54+
}
55+
56+
var formattedName = AttributedString(name)
5257
formattedName.foregroundColor = .primary
53-
var coderPart = AttributedString(".coder")
54-
coderPart.foregroundColor = .gray
55-
formattedName.append(coderPart)
58+
if let range = formattedName.range(of: ".coder") {
59+
formattedName[range].foregroundColor = .gray
60+
}
5661
return formattedName
5762
}
5863

@@ -73,26 +78,26 @@ struct MenuItemView: View {
7378
.fill(item.status.color.opacity(1.0))
7479
.frame(width: 7, height: 7)
7580
}
76-
Text(fmtWsName).lineLimit(1).truncationMode(.tail)
81+
Text(itemName).lineLimit(1).truncationMode(.tail)
7782
Spacer()
7883
}.padding(.horizontal, Theme.Size.trayPadding)
7984
.frame(minHeight: 22)
8085
.frame(maxWidth: .infinity, alignment: .leading)
81-
.foregroundStyle(nameIsSelected ? Color.white : .primary)
86+
.foregroundStyle(nameIsSelected ? .white : .primary)
8287
.background(nameIsSelected ? Color.accentColor.opacity(0.8) : .clear)
8388
.clipShape(.rect(cornerRadius: Theme.Size.rectCornerRadius))
8489
.onHover { hovering in nameIsSelected = hovering }
8590
Spacer()
8691
}.buttonStyle(.plain)
87-
if case let .agent(agent) = item {
92+
if case let .agent(agent) = item, let copyableDNS = agent.primaryHost {
8893
Button {
8994
NSPasteboard.general.clearContents()
90-
NSPasteboard.general.setString(agent.copyableDNS, forType: .string)
95+
NSPasteboard.general.setString(copyableDNS, forType: .string)
9196
} label: {
9297
Image(systemName: "doc.on.doc")
9398
.symbolVariant(.fill)
9499
.padding(3)
95-
}.foregroundStyle(copyIsSelected ? Color.white : .primary)
100+
}.foregroundStyle(copyIsSelected ? .white : .primary)
96101
.imageScale(.small)
97102
.background(copyIsSelected ? Color.accentColor.opacity(0.8) : .clear)
98103
.clipShape(.rect(cornerRadius: Theme.Size.rectCornerRadius))

‎Coder Desktop/Coder Desktop/Views/VPNState.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ struct VPNState<VPN: VPNService, S: Session>: View {
1818
.font(.body)
1919
.foregroundColor(.gray)
2020
case (.disabled, _):
21-
Text("Enable CoderVPN to see agents")
21+
Text("Enable CoderVPN to see workspaces")
2222
.font(.body)
2323
.foregroundStyle(.gray)
2424
case (.connecting, _), (.disconnecting, _):

0 commit comments

Comments
 (0)