Skip to content

Commit 24d5538

Browse files
committed
feat: add troubleshooting tab and fix extension management issues
- Add troubleshooting tab to settings with system and network extension controls - Address review feedback related to extension management: - Replace custom notifications with direct method calls for reconfiguration - Restore proper encapsulation for session state management - Improve extension uninstallation with proper state monitoring - Fix deregistration to use existing delegate pattern - Enhance error handling for network extension operations 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> Change-Id: I947bae1bc7680bd1f670245e4541a95619ab41ee Signed-off-by: Thomas Kosiewski <[email protected]>
1 parent b7ccbca commit 24d5538

File tree

9 files changed

+646
-2
lines changed

9 files changed

+646
-2
lines changed

CLAUDE.md

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
# Coder Desktop Development Guide
2+
3+
## Build & Test Commands
4+
- Build Xcode project: `make`
5+
- Format Swift files: `make fmt`
6+
- Lint Swift files: `make lint`
7+
- Run all tests: `make test`
8+
- Run specific test class: `xcodebuild test -project "Coder Desktop/Coder Desktop.xcodeproj" -scheme "Coder Desktop" -only-testing:"Coder DesktopTests/AgentsTests"`
9+
- Run specific test method: `xcodebuild test -project "Coder Desktop/Coder Desktop.xcodeproj" -scheme "Coder Desktop" -only-testing:"Coder DesktopTests/AgentsTests/agentsWhenVPNOff"`
10+
- Generate Swift from proto: `make proto`
11+
- Watch for project changes: `make watch-gen`
12+
13+
## Code Style Guidelines
14+
- Use Swift 6.0 for development
15+
- Follow SwiftFormat and SwiftLint rules
16+
- Use Swift's Testing framework for tests (`@Test`, `#expect` directives)
17+
- Group files logically (Views, Models, Extensions)
18+
- Use environment objects for dependency injection
19+
- Prefer async/await over completion handlers
20+
- Use clear, descriptive naming for functions and variables
21+
- Implement proper error handling with Swift's throwing functions
22+
- Tests should use descriptive names reflecting what they're testing

Coder Desktop/Coder Desktop/Coder_DesktopApp.swift

+6
Original file line numberDiff line numberDiff line change
@@ -88,3 +88,9 @@ extension AppDelegate {
8888
func appActivate() {
8989
NSApp.activate()
9090
}
91+
92+
extension NSApplication {
93+
@objc func showLoginWindow() {
94+
NSApp.sendAction(#selector(NSWindowController.showWindow(_:)), to: nil, from: Windows.login.rawValue)
95+
}
96+
}

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

+70-1
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,15 @@ final class PreviewVPN: Coder_Desktop.VPNService {
2626
UUID(): Agent(id: UUID(), name: "dev", status: .off, hosts: ["asdf.coder"], wsName: "example",
2727
wsID: UUID()),
2828
], workspaces: [:])
29+
@Published var sysExtnState: SystemExtensionState = .installed
30+
@Published var neState: NetworkExtensionState = .enabled
2931
let shouldFail: Bool
3032
let longError = "This is a long error to test the UI with long error messages"
3133

32-
init(shouldFail: Bool = false) {
34+
init(shouldFail: Bool = false, extensionInstalled: Bool = true, networkExtensionEnabled: Bool = true) {
3335
self.shouldFail = shouldFail
36+
sysExtnState = extensionInstalled ? .installed : .uninstalled
37+
neState = networkExtensionEnabled ? .enabled : .disabled
3438
}
3539

3640
var startTask: Task<Void, Never>?
@@ -78,4 +82,69 @@ final class PreviewVPN: Coder_Desktop.VPNService {
7882
func configureTunnelProviderProtocol(proto _: NETunnelProviderProtocol?) {
7983
state = .connecting
8084
}
85+
86+
func uninstall() async -> Bool {
87+
// Simulate uninstallation with a delay
88+
do {
89+
try await Task.sleep(for: .seconds(2))
90+
} catch {
91+
return false
92+
}
93+
94+
if !shouldFail {
95+
sysExtnState = .uninstalled
96+
return true
97+
}
98+
return false
99+
}
100+
101+
func installExtension() async {
102+
// Simulate installation with a delay
103+
do {
104+
try await Task.sleep(for: .seconds(2))
105+
sysExtnState = if !shouldFail {
106+
.installed
107+
} else {
108+
.failed("Failed to install extension")
109+
}
110+
} catch {
111+
sysExtnState = .failed("Installation was interrupted")
112+
}
113+
}
114+
115+
func disableExtension() async -> Bool {
116+
// Simulate disabling with a delay
117+
do {
118+
try await Task.sleep(for: .seconds(1))
119+
} catch {
120+
return false
121+
}
122+
123+
if !shouldFail {
124+
neState = .disabled
125+
state = .disabled
126+
return true
127+
} else {
128+
neState = .failed("Failed to disable network extension")
129+
return false
130+
}
131+
}
132+
133+
func enableExtension() async -> Bool {
134+
// Simulate enabling with a delay
135+
do {
136+
try await Task.sleep(for: .seconds(1))
137+
} catch {
138+
return false
139+
}
140+
141+
if !shouldFail {
142+
neState = .enabled
143+
state = .disabled // Just disabled, not connected yet
144+
return true
145+
} else {
146+
neState = .failed("Failed to enable network extension")
147+
return false
148+
}
149+
}
81150
}

Coder Desktop/Coder Desktop/SystemExtension.swift

+71
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,77 @@ extension CoderVPNService: SystemExtensionAsyncRecorder {
8181
OSSystemExtensionManager.shared.submitRequest(request)
8282
logger.info("submitted SystemExtension request with bundleID: \(bundleID)")
8383
}
84+
85+
func deregisterSystemExtension() async -> Bool {
86+
logger.info("Starting network extension deregistration...")
87+
88+
// Use the existing delegate pattern
89+
let result = await withCheckedContinuation { (continuation: CheckedContinuation<Bool, Never>) in
90+
// Extension bundle identifier - must match what's used in the app
91+
guard let bundleID = extensionBundle.bundleIdentifier else {
92+
logger.error("Bundle has no identifier")
93+
continuation.resume(returning: false)
94+
return
95+
}
96+
97+
// Set a temporary state for deregistration
98+
sysExtnState = .uninstalled
99+
100+
// Create a special delegate that will handle the deregistration and resolve the continuation
101+
let delegate = SystemExtensionDelegate(asyncDelegate: self)
102+
systemExtnDelegate = delegate
103+
104+
// Create the deactivation request
105+
let request = OSSystemExtensionRequest.deactivationRequest(
106+
forExtensionWithIdentifier: bundleID,
107+
queue: .main
108+
)
109+
request.delegate = delegate
110+
111+
// Start a timeout task
112+
Task {
113+
// Allow up to 30 seconds for deregistration
114+
try? await Task.sleep(for: .seconds(30))
115+
116+
// If we're still waiting after timeout, consider it failed
117+
if case .uninstalled = self.sysExtnState {
118+
// Only update if still in uninstalled state (meaning callback never updated it)
119+
self.sysExtnState = .failed("Deregistration timed out")
120+
continuation.resume(returning: false)
121+
}
122+
}
123+
124+
// Submit the request and wait for the delegate to handle completion
125+
OSSystemExtensionManager.shared.submitRequest(request)
126+
logger.info("Submitted system extension deregistration request for \(bundleID)")
127+
128+
// The SystemExtensionDelegate will update our state via recordSystemExtensionState
129+
// We'll monitor this in another task to resolve the continuation
130+
Task {
131+
// Check every 100ms for state changes
132+
for _ in 0 ..< 300 { // 30 seconds max
133+
// If state changed from uninstalled, the delegate has processed the result
134+
if case .installed = self.sysExtnState {
135+
// This should never happen during deregistration
136+
continuation.resume(returning: false)
137+
break
138+
} else if case .failed = self.sysExtnState {
139+
// Failed state was set by delegate
140+
continuation.resume(returning: false)
141+
break
142+
} else if case .uninstalled = self.sysExtnState, self.systemExtnDelegate == nil {
143+
// Uninstalled AND delegate is nil means success (delegate cleared itself)
144+
continuation.resume(returning: true)
145+
break
146+
}
147+
148+
try? await Task.sleep(for: .milliseconds(100))
149+
}
150+
}
151+
}
152+
153+
return result
154+
}
84155
}
85156

86157
/// A delegate for the OSSystemExtensionRequest that maps the callbacks to async calls on the

Coder Desktop/Coder Desktop/VPNService.swift

+179
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,15 @@ import VPNLib
77
protocol VPNService: ObservableObject {
88
var state: VPNServiceState { get }
99
var menuState: VPNMenuState { get }
10+
var sysExtnState: SystemExtensionState { get }
11+
var neState: NetworkExtensionState { get }
1012
func start() async
1113
func stop() async
1214
func configureTunnelProviderProtocol(proto: NETunnelProviderProtocol?)
15+
func uninstall() async -> Bool
16+
func installExtension() async
17+
func disableExtension() async -> Bool
18+
func enableExtension() async -> Bool
1319
}
1420

1521
enum VPNServiceState: Equatable {
@@ -114,6 +120,179 @@ final class CoderVPNService: NSObject, VPNService {
114120
}
115121
}
116122

123+
func uninstall() async -> Bool {
124+
logger.info("Uninstalling VPN system extension...")
125+
126+
// First stop any active VPN tunnels
127+
if tunnelState == .connected || tunnelState == .connecting {
128+
await stop()
129+
130+
// Wait for tunnel state to actually change to disabled
131+
let startTime = Date()
132+
let timeout = TimeInterval(10) // 10 seconds timeout
133+
134+
while tunnelState != .disabled {
135+
// Check for timeout
136+
if Date().timeIntervalSince(startTime) > timeout {
137+
logger.warning("Timeout waiting for VPN to disconnect before uninstall")
138+
break
139+
}
140+
141+
// Wait a bit before checking again
142+
try? await Task.sleep(for: .milliseconds(100))
143+
}
144+
}
145+
146+
// Remove network extension configuration
147+
do {
148+
try await removeNetworkExtension()
149+
neState = .unconfigured
150+
tunnelState = .disabled
151+
} catch {
152+
logger.error("Failed to remove network extension configuration: \(error.localizedDescription)")
153+
// Continue with deregistration even if removing network extension failed
154+
}
155+
156+
// Deregister the system extension
157+
let success = await deregisterSystemExtension()
158+
if success {
159+
logger.info("Successfully uninstalled VPN system extension")
160+
sysExtnState = .uninstalled
161+
} else {
162+
logger.error("Failed to uninstall VPN system extension")
163+
sysExtnState = .failed("Deregistration failed")
164+
}
165+
166+
return success
167+
}
168+
169+
func installExtension() async {
170+
logger.info("Installing VPN system extension...")
171+
172+
// Install the system extension
173+
installSystemExtension()
174+
175+
// We don't need to await here since the installSystemExtension method
176+
// uses a delegate callback system to update the state
177+
}
178+
179+
func disableExtension() async -> Bool {
180+
logger.info("Disabling VPN network extension without uninstalling...")
181+
182+
// First stop any active VPN tunnel
183+
if tunnelState == .connected || tunnelState == .connecting {
184+
await stop()
185+
}
186+
187+
// Remove network extension configuration but keep the system extension
188+
do {
189+
try await removeNetworkExtension()
190+
neState = .unconfigured
191+
tunnelState = .disabled
192+
logger.info("Successfully disabled network extension")
193+
return true
194+
} catch {
195+
logger.error("Failed to disable network extension: \(error.localizedDescription)")
196+
neState = .failed(error.localizedDescription)
197+
return false
198+
}
199+
}
200+
201+
func enableExtension() async -> Bool {
202+
logger.info("Enabling VPN network extension...")
203+
204+
// Ensure system extension is installed
205+
let extensionInstalled = await ensureSystemExtensionInstalled()
206+
if !extensionInstalled {
207+
return false
208+
}
209+
210+
// Get the initial state for comparison
211+
let initialNeState = neState
212+
213+
// Directly inject AppState dependency to call reconfigure
214+
if let appState = (NSApp.delegate as? AppDelegate)?.state, appState.hasSession {
215+
appState.reconfigure()
216+
} else {
217+
// No valid session, the user likely needs to log in again
218+
await MainActor.run {
219+
NSApp.sendAction(#selector(NSApplication.showLoginWindow), to: nil, from: nil)
220+
}
221+
}
222+
223+
// Wait for network extension state to change
224+
let stateChanged = await waitForNetworkExtensionChange(from: initialNeState)
225+
if !stateChanged {
226+
return false
227+
}
228+
229+
logger.info("Network extension was reconfigured successfully")
230+
231+
// Try to connect to VPN if needed
232+
return await tryConnectAfterReconfiguration()
233+
}
234+
235+
private func ensureSystemExtensionInstalled() async -> Bool {
236+
if sysExtnState != .installed {
237+
installSystemExtension()
238+
// Wait for the system extension to be installed
239+
for _ in 0 ..< 30 { // Wait up to 3 seconds
240+
if sysExtnState == .installed {
241+
break
242+
}
243+
try? await Task.sleep(for: .milliseconds(100))
244+
}
245+
246+
if sysExtnState != .installed {
247+
logger.error("Failed to install system extension during enableExtension")
248+
return false
249+
}
250+
}
251+
return true
252+
}
253+
254+
private func waitForNetworkExtensionChange(from initialState: NetworkExtensionState) async -> Bool {
255+
// Wait for network extension state to change from the initial state
256+
for _ in 0 ..< 30 { // Wait up to 3 seconds
257+
// If the state changes at all from the initial state, we consider reconfiguration successful
258+
if neState != initialState || neState == .enabled {
259+
return true
260+
}
261+
try? await Task.sleep(for: .milliseconds(100))
262+
}
263+
264+
logger.error("Network extension configuration didn't change after reconfiguration request")
265+
return false
266+
}
267+
268+
private func tryConnectAfterReconfiguration() async -> Bool {
269+
// If already enabled, we're done
270+
if neState == .enabled {
271+
logger.info("Network extension enabled successfully")
272+
return true
273+
}
274+
275+
// Wait a bit longer for the configuration to be fully applied
276+
try? await Task.sleep(for: .milliseconds(500))
277+
278+
// If the extension is in a state we can work with, try to start the VPN
279+
if case .failed = neState {
280+
logger.error("Network extension in failed state, skipping auto-connection")
281+
} else if neState != .unconfigured {
282+
logger.info("Attempting to automatically connect to VPN after reconfiguration")
283+
await start()
284+
285+
if tunnelState == .connecting || tunnelState == .connected {
286+
logger.info("VPN connection started successfully after reconfiguration")
287+
return true
288+
}
289+
}
290+
291+
// If we get here, the extension was reconfigured but not successfully enabled
292+
// Since configuration was successful, return true so user can manually connect
293+
return true
294+
}
295+
117296
func onExtensionPeerUpdate(_ data: Data) {
118297
logger.info("network extension peer update")
119298
do {

0 commit comments

Comments
 (0)