Add tvOS (Apple TV) support for NetBird VPN client #36
Conversation
This commit introduces full tvOS support for the NetBird iOS client,
enabling VPN connectivity on Apple TV devices.
- Added NetBird TV app target with tvOS 16.0+ deployment
- Created tvOS-specific UI using SwiftUI optimized for "10-foot experience"
- Tab-based navigation: Connection, Peers, Networks, Settings
- Large touch targets and text for Siri Remote navigation
- TVMainView: Main tab navigation and connection status
- TVConnectionView: Large connect/disconnect button with status display
- TVPeersView: List of connected peers with connection details
- TVNetworksView: Network routes selection and management
- TVSettingsView: App settings and logout functionality
- TVAuthView: QR code + device code authentication flow
- Implemented OAuth device authorization flow for tvOS
- Displays QR code that user scans with phone to authenticate
- Shows user code as fallback for manual entry
- Polls for authentication completion and auto-dismisses on success
tvOS has stricter sandbox restrictions than iOS:
1. **UserDefaults-based Config Storage**
- tvOS blocks file writes to App Group containers
- Config stored in shared UserDefaults instead of files
- Added Preferences methods: saveConfigToUserDefaults(),
loadConfigFromUserDefaults(), hasConfigInUserDefaults()
2. **Preloaded Config in Go SDK**
- SDK modified to accept config via setConfigFromJSON()
- Avoids file I/O that would fail in tvOS sandbox
- Config passed from UserDefaults to SDK at runtime
3. **Raw Syscall Tunnel FD Discovery**
- tvOS SDK doesn't expose ctl_info, sockaddr_ctl, CTLIOCGINFO
- Implemented findTunnelFileDescriptorTvOS() using raw memory ops
- Manually defines kernel structure layouts at byte level
- Uses getpeername() and ioctl() which ARE available on tvOS
- Added NetBirdTVNetworkExtension target
- Separate PacketTunnelProvider.swift with tvOS-specific handling
- Extensive logging for debugging via Console.app
- Handles "LoginTV" message for device auth flow
- Loads config from UserDefaults into SDK memory
- isLoginRequired() now verifies session with management server
- Previously only checked if config existed (caused post-restart failures)
- Shows QR code re-auth flow when OAuth session expires
- Added Platform.swift for iOS/tvOS conditional compilation
- Shared code uses #if os(tvOS) / #if os(iOS) where needed
- Common ViewModels work across both platforms
📝 WalkthroughWalkthroughAdds a tvOS Apple TV app and packet-tunnel network extension, platform abstractions and tvOS persistence/IPC, SSO/device-code login flows, extensive tvOS SwiftUI views and theming, Xcode project/CI wiring, entitlements, assets, and adapter/extension integration changes. Changes
Sequence Diagram(s)sequenceDiagram
actor User as Apple TV User
participant TVApp as NetBird TV App
participant Pref as ConfigurationProvider / UserDefaults
participant Ext as PacketTunnel Extension
participant SDK as NetBird SDK
participant LoginSvc as Login Service
User->>TVApp: Start device-code login
TVApp->>Pref: loadConfigFromUserDefaults()
Pref-->>TVApp: configJSON
TVApp->>Ext: sendConfigToExtension(configJSON)
Ext->>SDK: initializeConfig / loginTV
SDK->>LoginSvc: Request device code
LoginSvc-->>SDK: DeviceCode + LoginURL
SDK-->>Ext: Return LoginURL + UserCode
Ext-->>TVApp: Config + userCode
TVApp->>User: Display QR and device code
loop Poll every 2s
TVApp->>Ext: checkLoginComplete()
Ext->>SDK: Check auth status
alt Auth complete
SDK-->>Ext: Success + config
Ext-->>TVApp: Login Complete
TVApp->>User: Close Auth UI
else Pending
Ext-->>TVApp: Not ready
end
end
sequenceDiagram
participant TVApp as NetBird TV App
participant Pref as ConfigurationProvider / UserDefaults
participant Ext as PacketTunnel Extension
participant SDK as NetBird SDK
participant VPN as NEPacketTunnelProvider
TVApp->>Pref: loadConfigFromUserDefaults()
Pref-->>TVApp: configJSON
TVApp->>Ext: sendConfigToExtension(configJSON)
Ext->>Ext: save config (UserDefaults / file fallback)
Ext->>SDK: NetBirdSDKNewClient(config)
SDK-->>Ext: client ready
Ext->>VPN: startTunnel()
VPN-->>Ext: Tunnel active
Ext-->>TVApp: Status update
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (5)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Update ServerViewModel to use new SDK callback-based API - Fix focus navigation on Networks and Settings tabs - Add white text on focus for better readability across all cards - Increase filter bar spacing to prevent highlight overlap - Add TVSettingsInfoRow for non-interactive display items
- Remove duplicate TVColors structs from 5 TV view files - Add ClearConfig IPC message to clear extension-local config on logout - Switch MainView from viewModel.isIpad to DeviceType.isPad - Remove unused isTV/isIpad properties from MainViewModel - Add TVColors.swift to Xcode project
- Remove dead shared UserDefaults fallback code in NetBirdAdapter - Document tvOS config storage architecture in Preferences.swift - Fix onChange deprecation warnings in TVSettingsView - Add "no peer auth method" detection in ServerViewModel
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
♻️ Duplicate comments (4)
NetBird/Source/App/NetBirdApp.swift (1)
45-53: Firebase configuration duplication noted.This tvOS initialization mirrors the iOS
AppDelegatesetup at lines 27-30. See the earlier comment for a suggested refactor to eliminate this duplication.NetBird.xcodeproj/xcshareddata/xcschemes/NetbirdNetworkExtension.xcscheme (1)
3-3: Consistent with main scheme update.The
LastUpgradeVersionbump to2610matches the update inNetBird.xcscheme. Same verification note applies regarding the version number.NetBird/Source/App/ViewModels/MainViewModel.swift (2)
264-290: This code appears to be dead/duplicated.Per past review comments, management server URL handling was migrated to
ServerViewModel. If this function is no longer used, it should be removed to avoid confusion.
307-324: This code appears to be dead/duplicated.Per past review comments, setup key handling was migrated to
ServerViewModel. If this function is no longer used, it should be removed.
🧹 Nitpick comments (23)
README.md (2)
101-101: Wrap the repository URL in markdown link syntax for consistency.The other repository entries in this section use markdown link syntax
[text](url), but this line uses a bare URL. Wrap the URL to maintain consistent formatting with the rest of the list.Apply this diff:
- iOS/tvOS Client: https://github.com/netbirdio/ios-client (this repository) + iOS/tvOS Client: [https://github.com/netbirdio/ios-client](https://github.com/netbirdio/ios-client) (this repository)
90-92: Minor: Consider rephrasing for variety.Line 88 uses "needs to be connected" and line 92 uses "needs to be paired"—both opening with "needs to be." Consider varying the phrasing in one of these sentences to improve readability.
NetBird/Source/App/NetBirdApp.swift (1)
19-34: Consider extracting Firebase configuration to reduce duplication.The Firebase setup logic is duplicated between the iOS
AppDelegate(lines 27-30) and the tvOSinit()method (lines 48-51). Consider extracting this into a shared helper method to improve maintainability. Additionally, silent failure when the plist is missing or invalid could complicate debugging—adding error logging would be helpful.Example refactor:
private func configureFirebase() { guard let path = Bundle.main.path(forResource: "GoogleService-Info", ofType: "plist"), let options = FirebaseOptions(contentsOfFile: path) else { print("Warning: GoogleService-Info.plist not found or invalid. Firebase not configured.") return } FirebaseApp.configure(options: options) }Then call
configureFirebase()from both the iOSAppDelegateand tvOSinit().NetbirdKit/Device.swift (1)
27-53: LGTM! Well-designed persistent device name generation.The implementation correctly generates a unique device name once and caches it for consistency across app launches. The force unwrap on
randomElement()is safe sincecharactersis a non-empty literal.Note:
defaults.synchronize()(line 48) is deprecated on modern iOS/tvOS as the system handles persistence automatically, but it's harmless here.NetbirdKit/Preferences.swift (1)
49-59: Redundant conditional—both branches are identical.The
#if os(tvOS)and#elsebranches execute the same code. Either remove the conditional or document why it's kept for future differentiation.static func newPreferences() -> NetBirdSDKPreferences? { - #if os(tvOS) - // On tvOS, creating SDK Preferences may fail if the app doesn't have write access - // to the App Group container. Try anyway - if it fails, settings will be managed - // via the extension instead. - // Note: The SDK now uses DirectWriteOutConfig which may work better on tvOS. - return NetBirdSDKNewPreferences(configFile(), stateFile()) - #else return NetBirdSDKNewPreferences(configFile(), stateFile()) - #endif }NetbirdNetworkExtension/NetBirdAdapter.swift (1)
461-484: Regex-based JSON manipulation is fragile.Using regex patterns like
"ManagementURL"\\s*:\\s*"([^"]*)"to parse/modify JSON can fail on escaped quotes, Unicode escapes, or whitespace variations. Consider usingJSONSerializationorCodable.+ /// Extract the management URL from a config JSON string using proper JSON parsing static func extractManagementURL(from configJSON: String) -> String? { - let pattern = "\"ManagementURL\"\\s*:\\s*\"([^\"]*)\"" - guard let regex = try? NSRegularExpression(pattern: pattern, options: []), - let match = regex.firstMatch(in: configJSON, options: [], range: NSRange(configJSON.startIndex..., in: configJSON)), - let urlRange = Range(match.range(at: 1), in: configJSON) else { + guard let data = configJSON.data(using: .utf8), + let json = try? JSONSerialization.jsonObject(with: data) as? [String: Any], + let url = json["ManagementURL"] as? String, + !url.isEmpty else { return nil } - let url = String(configJSON[urlRange]) - return url.isEmpty ? nil : url + return url }NetbirdKit/NetworkExtensionAdapter.swift (4)
17-27: Duplicate listener class across codebase.
ConfigSSOListenerduplicates the same pattern asSSOCheckListenerinMainViewModel.swiftandSSOListenerImplinServerViewModel.swift. Consider extracting a single shared implementation inNetbirdKitto avoid maintaining multiple copies.
181-216:initializeConfigViaExtension()appears unused.This function is defined but never called. If it's intended for future use, consider marking it with a TODO or removing it to reduce dead code.
#!/bin/bash # Verify that initializeConfigViaExtension is not called anywhere rg -n "initializeConfigViaExtension" --type swift
247-269: SDK client created on everyisLoginRequired()call.On tvOS, a new
NetBirdSDKNewClientis instantiated each timeisLoginRequired()is called. This may be expensive if the SDK performs key generation or other heavy initialization. Consider caching the client or deferring this check.
333-335: Remove redundantVoidreturn type.Per SwiftLint, returning
Voidin a function declaration is redundant.- func stop() -> Void { + func stop() { self.vpnManager?.connection.stopVPNTunnel() }NetBird/Source/App/ViewModels/MainViewModel.swift (1)
18-28: Duplicate SSO listener pattern.
SSOCheckListenerduplicates the same pattern asConfigSSOListenerinNetworkExtensionAdapter.swiftandSSOListenerImplinServerViewModel.swift. Consider consolidating these into a single shared implementation.NetBird/Source/App/ViewModels/ServerViewModel.swift (1)
15-54: Well-designed listener implementations.
SSOListenerImplandErrListenerImpluse typed callbacks instead of optional closures, providing cleaner semantics. However, these duplicate similar listeners inMainViewModel.swiftandNetworkExtensionAdapter.swift.Consider consolidating all SDK listener implementations into a shared location in
NetbirdKitto reduce duplication across the codebase.NetBird/Source/App/Views/MainView.swift (1)
30-30: SwiftLint naming convention note.SwiftLint flags
iOSMainViewfor not starting with an uppercase character. WhileIOSMainViewwould satisfy the linting rule,iOSMainViewfollows Apple's branding convention (iOS, iPhone, iPad) and is more readable. Consider either:
- Keeping
iOSMainViewand suppressing the lint warning with// swiftlint:disable:next type_name- Renaming to
IOSMainViewfor strict conformanceIf you choose to keep the current naming (recommended for readability), add a suppression comment:
+// swiftlint:disable:next type_name struct iOSMainView: View {NetBird/Source/App/Views/PeerTabView.swift (1)
196-225: Platform-specific context menu is well-implemented.The iOS clipboard actions and tvOS static text fallback are correctly guarded. Consider wrapping the debug
#if DEBUGfor consistency with the logging approach used elsewhere in this PR.Button("Copy FQDN") { UIPasteboard.general.string = peer.fqdn + #if DEBUG print("Copied FQDN to clipboard") + #endif viewModel.showFqdnCopiedAlert = true ... } Button("Copy IP") { UIPasteboard.general.string = peer.ip + #if DEBUG print("Copied IP to clipboard") + #endif viewModel.showIpCopiedAlert = true ... }NetBird/Source/App/Views/TV/TVNetworksView.swift (3)
11-12: Remove unusedUIKitimport.The
UIKitimport doesn't appear to be used in this file. All types used are from SwiftUI.import SwiftUI -import UIKit
121-128: Refresh animation duration is not tied to actual data fetch completion.The 2-second delay is arbitrary and doesn't reflect when
getRoutes()actually completes. Consider using a completion handler fromgetRoutes()to stop the animation when data is ready.private func refresh() { isRefreshing = true - viewModel.routeViewModel.getRoutes() - - DispatchQueue.main.asyncAfter(deadline: .now() + 2) { + viewModel.routeViewModel.getRoutes { isRefreshing = false } }This would require adding a completion parameter to
getRoutes()inRoutesViewModel.
190-198: Magic string"invalid Prefix"is fragile.The comparison against
"invalid Prefix"relies on an exact string match from another layer. Consider defining this as a constant or using a more robust check (e.g., an enum or dedicated property on the route model).NetBird/Source/App/Views/TV/TVSettingsView.swift (1)
141-185: Consider makingactionnon-optional or using a different pattern.The
TVSettingsRowaccepts an optionalactionand disables the button whenaction == nil(line 183). However, using a non-focusable info row (TVSettingsInfoRow) for display-only items would be more semantically correct. The current pattern could confuse users who focus on a row that does nothing.If you want to keep this pattern, consider providing clearer visual feedback that the row is disabled:
.disabled(action == nil) +.opacity(action == nil ? 0.5 : 1.0)NetBird/Source/App/Platform/Platform.swift (1)
182-189:tvFocusable()modifier applies.focusable()unconditionally on tvOS.On tvOS,
.focusable()makes a view focusable for the Siri Remote. This is correct, but note that applying this to views that are already naturally focusable (likeButton) is redundant. Consider documenting the intended use case (e.g., for non-interactive views that need focus).NetBird/Source/App/Views/TV/TVPeersView.swift (1)
27-28: String comparison for connection state is fragile.Comparing
viewModel.extensionStateText == "Connected"relies on a magic string. If the state text changes (e.g., localization), this check will break silently. Consider using an enum or constant for state comparison.- if viewModel.extensionStateText == "Connected" && + if viewModel.isConnected && viewModel.peerViewModel.peerInfo.count > 0 {If
isConnecteddoesn't exist on the ViewModel, consider adding it as a computed property that encapsulates this check.NetBirdTVNetworkExtension/PacketTunnelProvider.swift (3)
219-238:UserDefaults.synchronize()is deprecated.
synchronize()is deprecated and generally unnecessary as the system handles persistence automatically. While it might seem safer for IPC scenarios, it can actually cause performance issues.// Save to extension-local UserDefaults (not shared App Group) UserDefaults.standard.set(configJSON, forKey: "netbird_config_json_local") - UserDefaults.standard.synchronize()Also applies to line 252.
613-657: Redundant file clearing before writing ininitializeLogging.Lines 630-639 open the file for writing, clear it with an empty string write, then write the log message. This is overly complex. Simply truncating and writing in one step would be cleaner.
if fileManager.fileExists(atPath: logURLValid.path) { - if let fileHandle = try? FileHandle(forWritingTo: logURLValid) { - do { - try "".write(to: logURLValid, atomically: true, encoding: .utf8) - } catch { - print("Error handling the log file: \(error)") - } - if let data = logMessage.data(using: .utf8) { - fileHandle.write(data) - } - fileHandle.closeFile() - } else { - print("Failed to open the log file for writing.") - } + do { + try logMessage.write(to: logURLValid, atomically: true, encoding: .utf8) + } catch { + print("Failed to overwrite the log file: \(error.localizedDescription)") + } } else {This simplifies the existing-file case to match the new-file case.
619-620: Unused variableserrorandsuccess.The variables
error(line 619) andsuccess(line 620) are declared early but only used much later (lines 652-656). Consider moving their declarations closer to usage for clarity, or removing them if the success/error logging isn't critical.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (12)
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Back.imagestacklayer/Content.imageset/netbird-tvos-icon.pngis excluded by!**/*.pngNetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Front.imagestacklayer/Content.imageset/netbird-tvos-icon.pngis excluded by!**/*.pngNetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Middle.imagestacklayer/Content.imageset/netbird-tvos-icon.pngis excluded by!**/*.pngNetBird TV/Assets.xcassets/icon-netbird-button.imageset/icon-netbird-button.pngis excluded by!**/*.pngNetBird TV/Assets.xcassets/icon-netbird-button.imageset/[email protected]is excluded by!**/*.pngNetBird TV/Assets.xcassets/icon-netbird-button.imageset/[email protected]is excluded by!**/*.pngNetBird TV/Assets.xcassets/netbird-logo-menu.imageset/netbird-logo-menu 1.pngis excluded by!**/*.pngNetBird TV/Assets.xcassets/netbird-logo-menu.imageset/netbird-logo-menu.pngis excluded by!**/*.pngNetBird TV/Assets.xcassets/netbird-logo-menu.imageset/netbird-logo-menu@2x 1.pngis excluded by!**/*.pngNetBird TV/Assets.xcassets/netbird-logo-menu.imageset/[email protected]is excluded by!**/*.pngNetBird TV/Assets.xcassets/netbird-logo-menu.imageset/netbird-logo-menu@3x 1.pngis excluded by!**/*.pngNetBird TV/Assets.xcassets/netbird-logo-menu.imageset/[email protected]is excluded by!**/*.png
📒 Files selected for processing (60)
NetBird TV/Assets.xcassets/AccentColor.colorset/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Back.imagestacklayer/Content.imageset/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Back.imagestacklayer/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Middle.imagestacklayer/Content.imageset/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Middle.imagestacklayer/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Back.imagestacklayer/Content.imageset/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Back.imagestacklayer/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Front.imagestacklayer/Content.imageset/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Front.imagestacklayer/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Middle.imagestacklayer/Content.imageset/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Middle.imagestacklayer/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/Top Shelf Image Wide.imageset/Contents.json(1 hunks)NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/Top Shelf Image.imageset/Contents.json(1 hunks)NetBird TV/Assets.xcassets/Contents.json(1 hunks)NetBird TV/Assets.xcassets/icon-netbird-button.imageset/Contents.json(1 hunks)NetBird TV/Assets.xcassets/netbird-logo-menu.imageset/Contents.json(1 hunks)NetBird TV/ContentView.swift(1 hunks)NetBird TV/NetBird TVDebug.entitlements(1 hunks)NetBird.xcodeproj/project.pbxproj(25 hunks)NetBird.xcodeproj/xcshareddata/xcschemes/NetBird.xcscheme(1 hunks)NetBird.xcodeproj/xcshareddata/xcschemes/NetbirdNetworkExtension.xcscheme(1 hunks)NetBird/Source/App/NetBirdApp.swift(1 hunks)NetBird/Source/App/Platform/Platform.swift(1 hunks)NetBird/Source/App/ViewModels/MainViewModel.swift(9 hunks)NetBird/Source/App/ViewModels/PeerViewModel.swift(1 hunks)NetBird/Source/App/ViewModels/RoutesViewModel.swift(1 hunks)NetBird/Source/App/ViewModels/ServerViewModel.swift(3 hunks)NetBird/Source/App/Views/Components/SafariView.swift(2 hunks)NetBird/Source/App/Views/Components/SideDrawer.swift(2 hunks)NetBird/Source/App/Views/MainView.swift(4 hunks)NetBird/Source/App/Views/PeerTabView.swift(5 hunks)NetBird/Source/App/Views/RouteTabView.swift(3 hunks)NetBird/Source/App/Views/ServerView.swift(1 hunks)NetBird/Source/App/Views/TV/TVAuthView.swift(1 hunks)NetBird/Source/App/Views/TV/TVColors.swift(1 hunks)NetBird/Source/App/Views/TV/TVMainView.swift(1 hunks)NetBird/Source/App/Views/TV/TVNetworksView.swift(1 hunks)NetBird/Source/App/Views/TV/TVPeersView.swift(1 hunks)NetBird/Source/App/Views/TV/TVServerView.swift(1 hunks)NetBird/Source/App/Views/TV/TVSettingsView.swift(1 hunks)NetBirdTV/Info.plist(1 hunks)NetBirdTV/NetBirdTV.entitlements(1 hunks)NetBirdTV/NetBirdTVNetworkExtension.entitlements(1 hunks)NetBirdTVNetworkExtension/Info.plist(1 hunks)NetBirdTVNetworkExtension/NetBirdTVNetworkExtension.entitlements(1 hunks)NetBirdTVNetworkExtension/NetBirdTVNetworkExtensionDebug.entitlements(1 hunks)NetBirdTVNetworkExtension/PacketTunnelProvider.swift(1 hunks)NetbirdKit/ConnectionListener.swift(1 hunks)NetbirdKit/DNSManager.swift(1 hunks)NetbirdKit/Device.swift(1 hunks)NetbirdKit/EnvVarPackager.swift(1 hunks)NetbirdKit/NetworkExtensionAdapter.swift(6 hunks)NetbirdKit/Preferences.swift(1 hunks)NetbirdKit/RoutesSelectionDetails.swift(1 hunks)NetbirdKit/StatusDetails.swift(1 hunks)NetbirdNetworkExtension/NetBirdAdapter.swift(5 hunks)README.md(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
NetBird/Source/App/Views/RouteTabView.swift (1)
NetBird/Source/App/Platform/Platform.swift (1)
size(45-47)
NetBird TV/ContentView.swift (1)
NetBird/Source/App/Views/Components/TransparentGradientButton.swift (1)
body(22-53)
NetBird/Source/App/Views/TV/TVServerView.swift (4)
NetbirdKit/Preferences.swift (1)
configFile(61-66)NetbirdKit/Device.swift (1)
getName(11-17)NetBird/Source/App/Platform/Platform.swift (1)
size(45-47)NetBird/Source/App/ViewModels/ServerViewModel.swift (3)
clearErrorsFor(298-310)loginWithSetupKey(229-296)changeManagementServerAddress(144-206)
NetBird/Source/App/Views/TV/TVNetworksView.swift (4)
NetbirdKit/NetworkExtensionAdapter.swift (1)
getRoutes(478-509)NetBird/Source/App/ViewModels/RoutesViewModel.swift (3)
getRoutes(55-60)deselectRoute(76-82)selectRoute(62-68)NetBird/Source/App/Platform/Platform.swift (1)
size(45-47)NetBirdTVNetworkExtension/PacketTunnelProvider.swift (2)
deselectRoute(586-593)selectRoute(577-584)
NetBird/Source/App/ViewModels/ServerViewModel.swift (3)
NetBirdTVNetworkExtension/PacketTunnelProvider.swift (3)
onSuccess(20-22)onError(24-26)login(209-214)NetbirdKit/NetworkExtensionAdapter.swift (3)
onSuccess(20-22)onError(24-26)login(337-380)NetbirdNetworkExtension/NetBirdAdapter.swift (5)
onSuccess(40-42)onSuccess(49-51)onError(36-38)onError(53-55)login(310-313)
NetbirdNetworkExtension/NetBirdAdapter.swift (3)
NetbirdKit/Device.swift (3)
getName(11-17)getOsVersion(19-21)getOsName(23-25)NetbirdKit/Preferences.swift (3)
configFile(61-66)stateFile(68-73)saveConfigToUserDefaults(86-93)NetbirdKit/EnvVarPackager.swift (1)
getEnvironmentVariables(12-29)
NetBird/Source/App/Views/PeerTabView.swift (1)
NetBird/Source/App/Platform/Platform.swift (1)
size(45-47)
NetbirdKit/NetworkExtensionAdapter.swift (2)
NetBirdTVNetworkExtension/PacketTunnelProvider.swift (4)
onSuccess(20-22)onError(24-26)login(209-214)checkLoginComplete(356-390)NetbirdKit/Preferences.swift (4)
loadConfigFromUserDefaults(96-101)configFile(61-66)stateFile(68-73)hasConfigInUserDefaults(104-109)
NetBird/Source/App/Views/TV/TVSettingsView.swift (1)
NetBird/Source/App/ViewModels/MainViewModel.swift (3)
setRosenpassEnabled(368-379)close(168-178)clearDetails(292-305)
🪛 LanguageTool
README.md
[style] ~92-~92: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...sical device running tvOS 17.0 or later needs to be [paired with Xcode](https://support....
(REP_NEED_TO_VB)
🪛 markdownlint-cli2 (0.18.1)
README.md
101-101: Bare URL used
(MD034, no-bare-urls)
🪛 SwiftLint (0.57.0)
NetBird/Source/App/Views/MainView.swift
[Error] 30-30: Type name 'iOSMainView' should start with an uppercase character
(type_name)
NetbirdKit/NetworkExtensionAdapter.swift
[Warning] 333-333: Returning Void in a function declaration is redundant
(redundant_void_return)
🔇 Additional comments (82)
README.md (1)
31-33: Documentation update comprehensively covers tvOS support.The README expansions effectively communicate the new tvOS capability with clear build instructions, platform requirements, and device setup guidance. The separation of build commands for iOS-only vs. iOS+tvOS scenarios is particularly helpful for developers.
Also applies to: 57-59, 70-82, 86-92
NetBird/Source/App/ViewModels/RoutesViewModel.swift (1)
8-8: Explicit Foundation import is appropriate.Adding the explicit
import Foundationstatement aligns with best practices for cross-platform integration and ensures clarity of dependencies. This is a suitable change for the tvOS support refactoring.NetBird/Source/App/Views/Components/SideDrawer.swift (2)
7-13: LGTM! Clear platform separation.The comments effectively document the iOS-only nature of the side drawer pattern and reference the tvOS alternative (TVSettingsView). The
#if os(iOS)guard properly isolates this iOS-specific navigation pattern.
172-172: LGTM! Properly closes the platform guard.The
#endifcorrectly closes the iOS compilation block, and the inline comment helps readability. The entire SideDrawer and SideMenu implementation (including UIKit-dependent code likeUIDevice.current.userInterfaceIdiomandUIScreen.main.bounds) is appropriately isolated to iOS.NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Back.imagestacklayer/Content.imageset/Contents.json (1)
1-11: Clarify intent for the incomplete tvOS imagestack structure.The imagestack (Back, Middle layers) contains valid but minimal Contents.json files with no image filenames and no actual image asset files. Confirm whether this is:
- An intentional placeholder for future image additions
- An incomplete structure that should include image files now
(Note: Both Back and Middle layers have identical JSON structure with no filename references, and no image files exist anywhere in this imagestack.)
NetBird/Source/App/Views/Components/SafariView.swift (1)
1-11: LGTM! Proper platform gating for iOS-only SafariServices.The conditional compilation block correctly restricts SafariServices usage to iOS builds, which is necessary since this framework is unavailable on tvOS. The header comment clearly documents the iOS-only nature of this component.
NetBird/Source/App/NetBirdApp.swift (2)
14-17: LGTM: Clean conditional compilation for iOS-only Firebase Performance.The conditional import is appropriate since Firebase Performance is not available on tvOS.
59-85: Reconsider unifying lifecycle approaches—behavioral equivalence is not confirmed.iOS uses
NotificationCenterwithUIApplicationlifecycle notifications (lines 60, 65), while tvOS usesscenePhaseobservation (lines 72-84). The main difference is that onReceive hooks into UIKit's notification system, while scenePhase is SwiftUI's built-in, declarative alternative. However, these approaches may not be behaviorally equivalent. ScenePhase has a number of bugs (or at least, unexpected behavior) and is limited to only 3 states: active, inactive, and background. Verify timing alignment and edge cases between the two approaches before unifying them, as the UIApplication notifications and scenePhase changes may not fire at identical moments.NetBird TV/Assets.xcassets/Contents.json (1)
1-6: LGTM! Standard asset catalog metadata.This is a standard Xcode-generated asset catalog root configuration file with correct format and metadata.
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Contents.json (1)
1-14: LGTM! Standard App Store app icon imagestack configuration.The layer structure and metadata follow Apple's tvOS asset catalog conventions correctly.
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/Top Shelf Image Wide.imageset/Contents.json (1)
1-16: LGTM! Correct tvOS Top Shelf Wide image configuration.The asset correctly uses the "tv" idiom and provides appropriate scale variants (1x, 2x) for tvOS Top Shelf imagery.
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Front.imagestacklayer/Contents.json (1)
1-6: LGTM! Standard imagestack layer metadata.This is a correctly formatted layer container metadata file following Xcode asset catalog conventions.
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/Top Shelf Image.imageset/Contents.json (1)
1-16: LGTM! Correct tvOS Top Shelf image configuration.The asset correctly uses the "tv" idiom and provides appropriate scale variants for tvOS Top Shelf imagery.
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Middle.imagestacklayer/Content.imageset/Contents.json (1)
1-11: LGTM! Standard app icon layer image content.The configuration correctly uses the "tv" idiom for the App Store app icon middle layer content.
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Back.imagestacklayer/Contents.json (1)
1-6: LGTM! Standard imagestack layer metadata.This is a correctly formatted layer container metadata file following Xcode asset catalog conventions.
NetBird TV/Assets.xcassets/icon-netbird-button.imageset/Contents.json (1)
1-23: LGTM! Standard cross-platform button icon configuration.The asset correctly uses the "universal" idiom and provides all standard scale variants (1x, 2x, 3x) with proper naming conventions. This allows the icon to be shared across iOS and tvOS targets.
NetBird TV/Assets.xcassets/AccentColor.colorset/Contents.json (1)
1-11: Standard asset catalog structure.This is a correctly formatted Xcode asset catalog definition for AccentColor. Since no specific color values are defined, the app will use the system default accent color, which is a valid design choice.
NetBird TV/Assets.xcassets/netbird-logo-menu.imageset/Contents.json (1)
1-56: Well-structured asset catalog with complete variants.This asset definition properly includes all standard scales (1x, 2x, 3x) with both light and dark appearance variants, ensuring the logo displays correctly across all device resolutions and appearance modes.
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Back.imagestacklayer/Contents.json (1)
1-6: Standard metadata structure for empty layer.This minimal structure is appropriate for the back layer of the App Store icon imagestack, which typically doesn't require visible content.
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Contents.json (1)
1-17: Correctly structured tvOS image stack.The three-layer structure (Front, Middle, Back) follows Apple's tvOS parallax icon conventions and properly coordinates the individual layer assets.
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Middle.imagestacklayer/Contents.json (1)
1-6: Standard layer metadata structure.This metadata file correctly provides Xcode authoring information for the Middle image stack layer, coordinating with the nested Content.imageset.
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon.imagestack/Back.imagestacklayer/Content.imageset/Contents.json (1)
1-17: Asset catalog structure looks correct.The JSON structure is valid for a tvOS Content.imageset. The 1x scale references
netbird-tvos-icon.png, and the 2x scale entry (without a filename) is acceptable when a @2x asset is not provided.Note: The AI summary incorrectly describes this file as being for the "Front.imagestacklayer", but the actual file path indicates it's for the "Back.imagestacklayer".
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/Contents.json (1)
1-32: LGTM! tvOS asset catalog structure is correct.The brandassets Contents.json properly defines all required tvOS assets with correct idioms, roles, and dimensions:
- Primary app icons at both App Store (1280x768) and standard (400x240) sizes
- Top Shelf images in both wide (2320x720) and standard (1920x720) formats
This follows Apple's tvOS asset catalog requirements.
NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App Icon - App Store.imagestack/Middle.imagestacklayer/Contents.json (1)
1-6: File structure is correct—imagestacklayer Contents.json is metadata-only.The layer-level Contents.json file contains only metadata, which is the correct structure. Image definitions belong in the nested
Content.imageset/Contents.jsonfile (which already has theimagesarray with appropriate entries). All comparable imagestacklayer Contents.json files in this project follow the same metadata-only pattern.Likely an incorrect or invalid review comment.
NetbirdKit/ConnectionListener.swift (1)
9-9: LGTM!The
NetBirdSDKimport is required sinceConnectionListenerconforms toNetBirdSDKConnectionListenerProtocol.NetBird.xcodeproj/xcshareddata/xcschemes/NetBird.xcscheme (1)
3-3: Verify the Xcode version in scheme metadata.The
LastUpgradeVersionof2610appears unusually high. Xcode version numbers typically follow the format where 1430 = Xcode 14.3 and 1610 = Xcode 16.1. Version2610would imply Xcode 26.1, which doesn't exist.If this was generated by Xcode automatically, it's harmless. Otherwise, confirm this wasn't a typo (perhaps intended
1610for Xcode 16.1?).NetbirdKit/DNSManager.swift (1)
9-9: LGTM!The
NetBirdSDKimport is required sinceDNSManagerconforms toNetBirdSDKDnsManagerProtocol.NetBird/Source/App/ViewModels/PeerViewModel.swift (1)
8-9: LGTM!The explicit
Foundationimport improves clarity, even thoughCombinere-exports it. The view model logic is clean and well-structured.NetbirdKit/EnvVarPackager.swift (1)
17-25: LGTM! Platform-specific relay defaults are sensible.Defaulting to forced relay on iOS while allowing direct connections on tvOS aligns with the different network environments of these platforms.
NetBirdTV/NetBirdTVNetworkExtension.entitlements (1)
1-19: LGTM! Entitlements are correctly configured.The
packet-tunnel-providercapability andgroup.io.netbird.app.tvapp group are appropriate for the tvOS network extension.NetbirdKit/RoutesSelectionDetails.swift (1)
1-8: LGTM!Standard file header and imports for Combine-backed observable models.
NetbirdKit/StatusDetails.swift (1)
9-9: LGTM!The
Combineimport is required sincePeerInfoconforms toObservableObject.NetbirdNetworkExtension/NetBirdAdapter.swift (2)
133-214: Thorough low-level implementation for tvOS tunnel discovery.The raw memory operations correctly replicate the kernel structure layout for
ctl_infoandsockaddr_ctl. The hardcoded sizes (100 bytes for ctl_info, 32 bytes for sockaddr_ctl) and offsets match the Darwin kernel ABI which is stable.Good use of logging via
adapterLoggerfor debugging tunnel discovery issues on tvOS.
321-451: Well-structured async login flow with proper completion tracking.The
NSLock-based completion guard correctly prevents duplicate callbacks, and the capturedauthRefensures config can be saved after successful authentication. The separation ofhandleSuccess/handleErrorclosures keeps the logic clean.NetbirdKit/NetworkExtensionAdapter.swift (3)
356-369: tvOS response parsing handles missing userCode gracefully.The code correctly falls back to using the whole string if the expected
|separator is not found. Good defensive handling.
385-428: Well-structured diagnostic response parsing.The
checkLoginCompletemethod handles multiple response formats gracefully with progressive fallbacks, which is good for backward compatibility.
596-665: tvOS config IPC methods are well-implemented.
sendConfigToExtensionandclearExtensionConfigproperly handle session availability, encoding errors, and provide completion callbacks. Good error logging throughout.NetBird.xcodeproj/project.pbxproj (1)
1341-1341: Different development team for Release configuration.The iOS Release configuration uses
DEVELOPMENT_TEAM = 94333M4JTAwhile Debug usesTA739QLA7A. Ensure this is intentional (e.g., different teams for development vs distribution).NetBird/Source/App/ViewModels/MainViewModel.swift (4)
104-115: Good optimization with lazy preferences loading.Deferring SDK initialization until first access avoids blocking app startup, especially important on tvOS where key generation is expensive. Well-documented with comments.
149-166: Connect flow is well-structured.The button lock with 3-second timeout, logging, and async Task handling is appropriate for VPN connection management.
240-258: tvOS-specific state text updates are well-implemented.The switch statement correctly maps VPN states to user-friendly text and triggers route fetching when connected. Good use of platform-specific compilation.
297-305: Good cleanup for tvOS config on logout.Clearing both UserDefaults config and extension-local config ensures no stale credentials remain after logout.
NetBird/Source/App/ViewModels/ServerViewModel.swift (3)
164-205: Good use of checked continuation for async bridging.The
withCheckedContinuationpattern correctly bridges the callback-based SDK API to async/await. The tvOS-specific handling of file permission errors by falling back to UserDefaults is a pragmatic workaround.
208-227: tvOS config persistence fallback is well-implemented.The
saveConfigToUserDefaultshelper correctly extracts config JSON from the authenticator and persists it when file writes are blocked. Error logging is appropriate.
259-295: Login with setup key handles tvOS constraints appropriately.Same pattern as
changeManagementServerAddresswith proper continuation bridging and tvOS file permission error handling.NetBird/Source/App/Views/ServerView.swift (1)
272-275: No functional changes.Only trailing whitespace adjustment. LGTM.
NetBird TV/NetBird TVDebug.entitlements (1)
1-14: LGTM! Standard tvOS network extension entitlements.The entitlements correctly declare the packet-tunnel-provider capability and app group for IPC between the tvOS app and its network extension. The configuration follows Apple's standard pattern for VPN-enabled apps.
NetBirdTV/NetBirdTV.entitlements (2)
1-18: LGTM! Entitlements properly configured for tvOS VPN app.The entitlements correctly declare packet-tunnel-provider and app group capabilities, matching the debug entitlements and following Apple's standard pattern.
5-5: Confirm tvOS 17.0 is the correct minimum version requirement.The code consistently specifies tvOS 17.0 as the minimum: NetBirdTV/Info.plist sets MinimumOSVersion to "17.0" with the comment "17.0 required for VPN support" (line 13), and NetBirdTV/NetBirdTV.entitlements confirms this with "requires tvOS 17+" (line 5). If the PR description claims "tvOS 16.0+ deployment", it should be corrected to reflect the actual tvOS 17.0+ requirement.
NetBirdTVNetworkExtension/NetBirdTVNetworkExtensionDebug.entitlements (1)
1-14: LGTM! Network extension debug entitlements properly configured.The entitlements correctly match the main app's app group identifier (group.io.netbird.app.tv), enabling proper IPC between the tvOS app and its network extension.
NetBirdTVNetworkExtension/Info.plist (1)
1-13: LGTM! Standard packet tunnel extension configuration.The Info.plist correctly defines the extension point identifier and principal class using the standard pattern. The $(PRODUCT_MODULE_NAME) variable will be resolved at build time to reference the PacketTunnelProvider implementation.
NetBirdTV/Info.plist (2)
13-15: MinimumOSVersion set to 17.0.This aligns with the VPN/Network Extension requirement noted in the entitlements, but conflicts with the PR summary stating "tvOS 16.0+ deployment". This version requirement was already flagged in the entitlements review.
1-27: LGTM! tvOS app Info.plist properly configured.The configuration is appropriate for a tvOS utilities app with automatic UI style support. The bundle display name, category, and launch screen setup follow Apple's standard patterns.
NetBirdTVNetworkExtension/NetBirdTVNetworkExtension.entitlements (1)
1-14: LGTM! Network extension entitlements consistent across targets.The app group identifier (group.io.netbird.app.tv) is consistent across all tvOS targets (app and extension, debug and release), ensuring proper IPC functionality.
NetBird/Source/App/Views/RouteTabView.swift (2)
86-92: LGTM! Platform-agnostic layout implementation.The refactoring from UIScreen.main.bounds to Screen helper and Layout.fontScale properly abstracts platform-specific sizing, enabling code sharing between iOS and tvOS while maintaining appropriate layout for each platform.
107-110: LGTM! Consistent use of platform abstraction.The padding calculations consistently use Screen helper for cross-platform compatibility.
NetBird/Source/App/Views/MainView.swift (3)
13-27: LGTM! Clean platform-specific routing implementation.The MainView correctly acts as a platform router, delegating to TVMainView for tvOS and iOSMainView for iOS. The conditional compilation ensures each platform only compiles its relevant code, maintaining clean separation.
75-75: LGTM! Platform-agnostic sizing with device type detection.The refactoring correctly uses Screen.height and DeviceType.isPad for adaptive layout that works across iOS device types (iPhone/iPad) while being compatible with the broader platform abstraction supporting tvOS.
Also applies to: 85-85
29-566: LGTM! iOS-specific code properly isolated.The #if os(iOS) conditional compilation correctly isolates iOS-specific UI code, ensuring it doesn't interfere with tvOS builds while maintaining the full iOS implementation.
NetBird/Source/App/Views/PeerTabView.swift (2)
7-15: LGTM!The conditional UIKit import and documentation comments are well-structured. This correctly limits UIKit usage to iOS builds where
UIPasteboardis actually used.
81-89: Good adoption of cross-platform sizing abstraction.The migration from
UIScreen-based sizing toScreen.*metrics aligns well with thePlatform.swiftabstraction for consistent behavior across iOS and tvOS.NetBird/Source/App/Views/TV/TVAuthView.swift (3)
1-19: Well-documented device code flow implementation.Good documentation explaining the authentication pattern. The QR code approach is appropriate for tvOS where Safari isn't available.
214-231: QR code generation looks correct.The use of
CIFilter.qrCodeGenerator()with appropriate scaling for tvOS visibility is well-implemented.
192-198: Good timer lifecycle management.Timer is correctly started on
onAppearand invalidated ononDisappear, preventing memory leaks.NetBird/Source/App/Views/TV/TVMainView.swift (4)
24-55: Clean TabView navigation structure.The tab-based navigation with proper tags and environment object passing is well-implemented for the tvOS 10-foot experience.
62-99: Auth flow integration with TVAuthView is complete.All necessary callbacks (
onCancel,onComplete,onError,checkLoginComplete,checkLoginError) are properly wired to the network extension adapter.
267-282: Connection button logic handles transitional states.The
handleTapfunction correctly handles both "Connected" and "Connecting" states for disconnection, allowing users to cancel an in-progress connection attempt. The logger usage provides good observability.
212-240: Good tvOS focus state handling.The
@FocusStateusage with scale animation provides appropriate visual feedback for Siri Remote navigation. The button is correctly disabled whenbuttonLockis true.NetBird/Source/App/Views/TV/TVServerView.swift (2)
25-28: ServerViewModel initialization looks correct.The view model is properly initialized with the config file path and device name, matching the existing iOS implementation patterns.
305-329: Server change logic is well-structured.The function correctly handles the three cases: empty inputs, URL-only, and URL with setup key. The
Task.yield()allows UI updates before async operations.NetBird/Source/App/Views/TV/TVNetworksView.swift (2)
200-206: Toggle route implementation is clean.The delegation to
routeViewModel.selectRoute/deselectRouteis straightforward and matches the existing RoutesViewModel API.
17-36: TVNetworksView structure is well-organized.The conditional rendering based on connection state and route availability follows the established pattern from other TV views.
NetBird/Source/App/Views/TV/TVColors.swift (2)
18-57: Well-structured color abstraction with safe fallbacks.The
colorOrFallbackhelper pattern ensures graceful degradation when asset catalog colors are missing. This is a clean approach for tvOS theming.
63-165: Layout constants are comprehensive and well-documented.The constants cover all major UI aspects (padding, spacing, typography, focus effects) with appropriate values for TV viewing distances. Consider extracting these to a protocol or making them configurable if you anticipate supporting multiple TV form factors in the future.
NetBird/Source/App/Views/TV/TVSettingsView.swift (2)
17-116: Clean two-pane layout with proper EnvironmentObject usage.The settings view structure is well-organized with sections for Connection, Advanced, and Info. The binding for Rosenpass using a custom getter/setter (lines 57-60) correctly delegates to the ViewModel's
setRosenpassEnabledmethod.
268-359: Focus management in the alert is well-implemented.The
onChangehandler (lines 349-357) that pulls focus back when it escapes is a good pattern for tvOS modal dialogs. The_ = oldValueon line 350 to suppress the unused warning is acceptable, though you could also useonChange(of: focusedButton) { newValue in ... }if targeting older tvOS versions isn't a concern.NetBird/Source/App/Platform/Platform.swift (2)
15-48: tvOS screen dimensions are hardcoded to 1920x1080 points.The comment mentions "3840x2160 for 4K, but points are same" - this is correct for tvOS where the point-based coordinate system uses 1920x1080 regardless of actual resolution. However, note that these are static properties computed at call time, which is fine for tvOS but on iOS, screen dimensions could potentially change (e.g., during Split View on iPad).
91-135: VPN support via Network Extension was added in tvOS 17.0.tvOS 17 added support for third-party VPN apps through Network Extensions. The code correctly checks for tvOS 17.0+ before enabling VPN support. The Network Extension framework allows developers to create custom VPN solutions for tvOS. The capability flags are well-structured and the comments explaining tvOS limitations are helpful.
NetBird/Source/App/Views/TV/TVPeersView.swift (2)
114-184: TVPeerCard is well-implemented with proper focus handling.The focus-driven styling, scale animation, and selection state management are appropriate for tvOS. The status color mapping (lines 177-183) covers the expected states.
244-286: Filter bar with focus-aware styling looks good.The
TVFilterBarandTVFilterButtoncomponents handle focus states well with appropriate visual feedback (scale, color changes).NetBirdTVNetworkExtension/PacketTunnelProvider.swift (2)
17-27: ConfigInitSSOListener handles both success and error paths correctly.The listener properly propagates results via the closure callback. The optional typing for
onResultis appropriate.
395-470: Completion handler may be called multiple times inloginTV.The
completionHandleris called inonURLcallback (line 429), and could potentially be called again inonErrorifurlSentToAppcheck fails due to a race condition (though the lock helps). More critically, the NEPacketTunnelProvider'shandleAppMessagecompletion handler should only be called once. The current logic is safe due to the lock, but consider documenting this invariant clearly.Verify that the
onURLcallback is guaranteed to be called beforeonSuccess/onError, and that only one of success/error is ever called. If bothonURLandonSuccesscan trigger in sequence, the current design is correct. However, if there's any scenario whereonErrorcould be called beforeonURLand thenonURLis called afterward, you'd have a problem.
...f Image.brandassets/App Icon.imagestack/Front.imagestacklayer/Content.imageset/Contents.json
Show resolved
Hide resolved
... Image.brandassets/App Icon.imagestack/Middle.imagestacklayer/Content.imageset/Contents.json
Show resolved
Hide resolved
…ndling - Fix mismatched bundle IDs between Debug/Release configs for tvOS targets - Make Preferences.configFile() and stateFile() return optionals instead of force-unwrapping, with proper error logging when app group unavailable - Make NetBirdAdapter.init failable to handle SDK client creation failures - Fail fast in NetBirdAdapter.start() when tunnel file descriptor is invalid instead of silently passing fd=0 (stdin) to the SDK - Ensure all handleAppMessage switch cases call completionHandler to prevent IPC callers from hanging indefinitely
(functionality migrated to ServerViewModel) - Remove orphaned managementURL property - Simplify supportsKeyboard to just return true (both branches were identical) - Updated README instructions on buiilding tvOS SDK (needs NetBird gomobile fork)
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (2)
NetbirdNetworkExtension/PacketTunnelProvider.swift (1)
106-117: Completion handlers now called consistently.The Select/Deselect and default cases now invoke the completion handler, which resolves the IPC contract issue flagged in previous reviews.
Optional: Consider non-optional Data initializer.
SwiftLint suggests using
Data("true".utf8)instead of"true".data(using: .utf8)for lines 109 and 113. The non-optional initializer avoids unnecessary optionals when the encoding is guaranteed to succeed.NetBirdTVNetworkExtension/PacketTunnelProvider.swift (1)
118-166: LGTM: Comprehensive IPC message handling for tvOS.The handleAppMessage implementation covers all the necessary IPC commands for tvOS, including config initialization, device auth login, and config transfer. The completion handlers are now consistently called for all message types, which addresses the previous review concern.
Optional: Consider non-optional Data initializer for consistency.
Lines 149, 153, 257, 276, 285, 294, 314, 324, 331, 335, 388, and 424 all use
"string".data(using: .utf8)which returns an optional. SwiftLint recommendsData("string".utf8)which is non-optional and more concise. This is a minor style improvement.
🧹 Nitpick comments (4)
README.md (1)
96-96: Reduce repetitive phrasing for improved readability.The phrase "needs to be" is repeated across adjacent sentences. Consider rephrasing for variety.
Consider this revision:
-> **Note:** The app cannot run in the tvOS simulator. To test the app, a physical device running tvOS 17.0 or later must be [paired with Xcode](https://support.apple.com/en-us/101262).NetbirdNetworkExtension/NetBirdAdapter.swift (2)
60-63: Consider making the default management URL configurable.The hardcoded public NetBird server URL may limit users who want to connect to self-hosted instances. While
loginAsync()does attempt to extract a stored URL from config, a more explicit configuration mechanism would improve flexibility.
484-496: Regex-based JSON parsing is fragile.Using regex to extract
ManagementURLfrom JSON (line 488) can fail or produce incorrect results with:
- Malformed JSON
- Escaped quotes within the value
- The field appearing in nested structures or JSON embedded in strings
While this works for simple, well-formed config JSON, proper JSON deserialization would be more robust and maintainable.
Consider using
JSONDecoderorJSONSerializationto parse the config structure instead:static func extractManagementURL(from configJSON: String) -> String? { guard let data = configJSON.data(using: .utf8), let json = try? JSONSerialization.jsonObject(with: data) as? [String: Any], let url = json["ManagementURL"] as? String, !url.isEmpty else { return nil } return url }NetbirdKit/NetworkExtensionAdapter.swift (1)
338-338: Optional: Remove redundant void return.SwiftLint flags the explicit
-> Voidreturn type as redundant. Functions without a return type implicitly returnVoid, so this can be simplified tofunc stop().
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
NetBird.xcodeproj/project.pbxproj(25 hunks)NetBird/Source/App/Platform/Platform.swift(1 hunks)NetBird/Source/App/ViewModels/MainViewModel.swift(9 hunks)NetBird/Source/App/Views/ServerView.swift(2 hunks)NetBird/Source/App/Views/TV/TVServerView.swift(1 hunks)NetBirdTVNetworkExtension/PacketTunnelProvider.swift(1 hunks)NetbirdKit/NetworkExtensionAdapter.swift(6 hunks)NetbirdKit/Preferences.swift(1 hunks)NetbirdNetworkExtension/NetBirdAdapter.swift(4 hunks)NetbirdNetworkExtension/PacketTunnelProvider.swift(10 hunks)README.md(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- NetBird/Source/App/Views/ServerView.swift
- NetBird/Source/App/Platform/Platform.swift
🧰 Additional context used
🧬 Code graph analysis (3)
NetbirdNetworkExtension/PacketTunnelProvider.swift (2)
NetbirdNetworkExtension/NetBirdAdapter.swift (3)
stop(478-480)start(296-323)login(331-334)NetBird/Source/App/ViewModels/RoutesViewModel.swift (1)
deselectRoute(76-82)
NetBird/Source/App/Views/TV/TVServerView.swift (2)
NetbirdKit/Device.swift (1)
getName(11-17)NetBird/Source/App/ViewModels/ServerViewModel.swift (3)
clearErrorsFor(298-310)loginWithSetupKey(229-296)changeManagementServerAddress(144-206)
NetBird/Source/App/ViewModels/MainViewModel.swift (4)
NetBirdTVNetworkExtension/PacketTunnelProvider.swift (2)
onError(24-26)onSuccess(20-22)NetbirdNetworkExtension/NetBirdAdapter.swift (5)
onError(36-38)onError(53-55)onSuccess(40-42)onSuccess(49-51)start(296-323)NetbirdKit/Preferences.swift (2)
newPreferences(49-63)removeConfigFromUserDefaults(120-126)NetBird/Source/App/ViewModels/RoutesViewModel.swift (1)
getRoutes(55-60)
🪛 LanguageTool
README.md
[style] ~96-~96: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...sical device running tvOS 17.0 or later needs to be [paired with Xcode](https://support....
(REP_NEED_TO_VB)
🪛 markdownlint-cli2 (0.18.1)
README.md
63-63: Bare URL used
(MD034, no-bare-urls)
105-105: Bare URL used
(MD034, no-bare-urls)
🪛 SwiftLint (0.57.0)
NetbirdNetworkExtension/PacketTunnelProvider.swift
[Warning] 109-109: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 113-113: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
NetbirdKit/NetworkExtensionAdapter.swift
[Warning] 338-338: Returning Void in a function declaration is redundant
(redundant_void_return)
NetBirdTVNetworkExtension/PacketTunnelProvider.swift
[Warning] 149-149: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 153-153: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 257-257: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 276-276: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 285-285: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 294-294: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 314-314: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 324-324: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 331-331: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 331-331: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 335-335: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 388-388: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
🔇 Additional comments (51)
README.md (1)
57-57: Verify tvOS version requirement consistency.Line 57 specifies tvOS 17.0+, but the PR objectives reference tvOS 16.0+ as the target. Please clarify which version is the actual requirement and ensure consistency across documentation.
NetBird.xcodeproj/project.pbxproj (1)
973-973: Bundle identifier consistency verified.The bundle identifier mismatch flagged in previous reviews has been resolved. Both Debug and Release configurations now use consistent identifiers:
- NetBird TV:
io.netbird.app.tv(lines 973, 1012)- NetBirdTVNetworkExtension:
io.netbird.app.tv.extension(lines 1049, 1086)This ensures proper code signing and provisioning profile compatibility across build configurations.
Also applies to: 1012-1012, 1049-1049, 1086-1086
NetbirdNetworkExtension/NetBirdAdapter.swift (4)
16-56: LGTM! Clean delegation wrapper classes.The login flow helper classes provide a clean bridge between the SDK's Objective-C protocols and Swift closures. The implementation is straightforward and safe.
87-131: LGTM! Platform-specific tunnel FD discovery is well-structured.The conditional compilation cleanly separates iOS and tvOS implementations, with appropriate logging for debugging. The iOS path uses standard SDK APIs while tvOS delegates to a custom implementation.
219-268: LGTM! Failable initializer properly handles platform differences.The initializer now uses
guard letinstead of force unwrapping (addressing the previous review comment), and gracefully handles initialization failures by returningnil. The platform-specific logic correctly handles tvOS's filesystem restrictions by loading config from UserDefaults instead of files.
296-323: LGTM! Proper validation of tunnel file descriptor.The
guardstatement now properly validates the file descriptor, checking for bothniland invalid values (fd <= 0), and returns a descriptive error instead of silently passing invalid data to the SDK. This addresses the previous review concern.NetbirdNetworkExtension/PacketTunnelProvider.swift (7)
23-25: LGTM: Defensive optional adapter initialization.Making the adapter optional allows graceful handling of initialization failures throughout the lifecycle methods. This pairs well with the new guard statements added across the file.
52-60: LGTM: Proper initialization check.The guard ensures the adapter is available before proceeding with the tunnel startup. The error code 1003 and message are clear and actionable.
162-169: LGTM: Safe network change handling.The optional chaining ensures the restart logic handles a nil adapter gracefully without crashing.
171-179: LGTM: Guarded login flow.The guard ensures the adapter is available before attempting login, preventing nil dereference.
181-248: LGTM: Status retrieval with local state caching.The guard protects against nil adapter, and extracting
clientStateto a local variable is a clean refactor that avoids repeated property access.
250-293: LGTM: Guarded route selection.The guard ensures adapter availability before retrieving routes.
295-311: LGTM: Route selection operations guarded.Both functions guard against nil adapter with early returns, maintaining consistency with the rest of the file.
NetBirdTVNetworkExtension/PacketTunnelProvider.swift (11)
15-27: LGTM: Clean SSO listener implementation.The ConfigInitSSOListener follows a standard callback pattern to bridge SDK results to async Swift code.
43-105: LGTM: Comprehensive tvOS startup flow with extensive logging.The startup sequence correctly handles tvOS sandbox restrictions by skipping file-based logging and checking UserDefaults for config. The extensive logging with
privacy: .publicis appropriate for debugging tvOS issues where Console.app is the primary diagnostic tool.Minor question: Why the 2-second delay before returning login error?
Line 86 introduces a 2-second delay before calling the completion handler with a "Login required" error. This seems arbitrary. If there's a specific reason (e.g., waiting for UI to be ready), consider documenting it. Otherwise, the delay could be removed or reduced.
233-259: LGTM: tvOS config transfer via IPC.This function implements the necessary workaround for tvOS where shared UserDefaults between app and extension don't work. Saving to extension-local
UserDefaults.standardand loading directly into the SDK client is the correct approach. The device name update ensures tvOS devices are properly identified.
261-278: LGTM: Simple config storage helpers for tvOS.These functions provide a clean interface for extension-local config storage using
UserDefaults.standard, which is the only reliable storage mechanism available in the tvOS extension sandbox.
280-342: LGTM: Config initialization with SDK file write.This function properly initializes the config file by delegating file writes to the SDK (which has the necessary permissions). Checking UserDefaults for an existing management URL before defaulting is a nice touch that preserves user preferences across initialization.
344-381: LGTM: Platform-specific config initialization.The tvOS path correctly delegates config loading to the adapter, while the iOS path uses a semaphore-based synchronous wait during startup. The 10-second timeout prevents hanging indefinitely.
383-426: LGTM: Detailed login status diagnostics.The diagnostic response format provides comprehensive state information for debugging the tvOS device auth flow. The comment on line 418 correctly notes that
client.isLoginComplete()doesn't work withAuth.Login(), so checkinglastLoginResultis the right approach.
428-512: LGTM: Well-designed tvOS device authentication flow.The loginTV implementation is well thought out:
- Forces device auth (appropriate for tvOS)
- Returns both URL and user code in a pipe-separated format
- Uses a lock to prevent double-completion
- Distinguishes between errors before and after the URL is sent (important for UX)
- Extensive logging aids debugging in the sandbox environment
514-581: LGTM: Status retrieval with proper error handling.The getStatus implementation follows the same pattern as the iOS version, with appropriate guards and a fallback to default status if encoding fails.
583-646: LGTM: Route selection with consistent error handling.The route selection methods follow the established pattern with proper guards and logging. The encoding fallback in getSelectRoutes ensures a response is always returned.
666-710: LGTM: Logging initialization for tvOS extension.The initializeLogging function sets up file-based logging in the App Group container, with appropriate error handling for file operations.
NetBird/Source/App/Views/TV/TVServerView.swift (5)
21-48: LGTM: Well-organized tvOS view structure.The view setup is clean with appropriate use of
@EnvironmentObject,@StateObject, and@FocusStatefor tvOS focus management. The initialization ofServerViewModelwith device name and config path is correct.
49-183: LGTM: Comprehensive form UI with proper focus management.The two-column layout with form fields on the left is well-suited for tvOS. Error messages are displayed per-field, and the optional setup key section is properly gated with clear UI feedback.
184-301: LGTM: Action buttons with proper state handling.The button layout provides clear actions (Cancel, Use NetBird, Change) with disabled states and loading indicators. The lifecycle hooks properly react to SSO errors and successful operations. The two-parameter
onChangesyntax on lines 289 and 294 is correct for the tvOS 17+ target.
305-353: LGTM: Clean async action methods.The action methods properly trim inputs, default to the NetBird URL when needed, clear errors, and call the appropriate async methods on
ServerViewModel. The use ofTaskwithawait Task.yield()is correct for SwiftUI async operations.
355-380: LGTM: Clean helper view and preview.The
InfoRowhelper provides consistent formatting for the info panel, and the preview provider aids development.NetbirdKit/Preferences.swift (6)
11-41: Excellent: Comprehensive architecture documentation.The extensive inline documentation explaining the tvOS config storage architecture is outstanding. It clearly documents the sandbox restrictions, the IPC-based workaround, and contrasts it with iOS behavior. This will be invaluable for future maintainers.
43-63: LGTM: Safe preferences creation with proper nil handling.The optional return type and guard statements address the previous review concern about force unwraps. The function now safely handles cases where the app group container is unavailable, which is important for tvOS.
65-81: LGTM: Safe file path retrieval.The guard statements and optional returns properly handle cases where the app group container is unavailable, addressing the force unwrap concerns from the previous review. The error messages provide clear diagnostic information.
83-101: LGTM: UserDefaults config storage for tvOS.The function provides a clean API for saving config to UserDefaults, which is the tvOS workaround for file write restrictions. The boolean return value allows callers to handle failures appropriately.
103-126: LGTM: Consistent UserDefaults helpers.The load, check, and remove methods follow a consistent pattern with proper nil handling for unavailable UserDefaults.
128-145: LGTM: Config restoration from UserDefaults to file.This function provides the bridge from UserDefaults storage to the file system when needed. The error handling with boolean return is appropriate. Note that this will only work in contexts where file writes are permitted (e.g., the extension).
NetbirdKit/NetworkExtensionAdapter.swift (8)
15-27: LGTM: SSO listener for config initialization.The ConfigSSOListener follows the standard callback pattern for bridging SDK results to Swift code.
29-66: LGTM: Platform-specific extension configuration.The conditional compilation properly sets different extension IDs for tvOS vs iOS. The addition of the
userCodeproperty supports the tvOS device authentication flow.
72-142: LGTM: VPN start sequence with tvOS config initialization.The start sequence properly logs each step for debugging. The tvOS-specific config initialization on lines 120-126 ensures config is available before checking if login is required.
144-219: LGTM: tvOS config initialization with IPC fallback.The
initializeConfigFromAppfunction implements the necessary workaround for tvOS by checking UserDefaults and sending config via IPC. TheinitializeConfigViaExtensionhelper allows requesting default config creation from the extension when it has write permissions the app lacks.
221-299: LGTM: Platform-specific login requirement checks.The tvOS implementation properly checks UserDefaults and then verifies with the management server, which is important for detecting expired tokens. The iOS implementation uses file-based config with SDK verification. The extensive logging aids debugging.
342-385: LGTM: Login flow with tvOS device authentication.The login function properly handles the tvOS device auth flow by sending "LoginTV" and parsing the "url|userCode" response format. Storing the userCode on the main thread is correct for UI binding.
387-481: LGTM: Robust login status checking with user-friendly errors.The
checkLoginCompletefunction robustly parses diagnostic responses in multiple formats for backward compatibility. ThecheckLoginErrorfunction transforms technical error messages into user-friendly guidance, which significantly improves the UX.
601-670: LGTM: IPC-based config transfer for tvOS.The
sendConfigToExtensionandclearExtensionConfigfunctions provide the necessary IPC mechanism for config management on tvOS. The error handling and completion callbacks are appropriate.NetBird/Source/App/ViewModels/MainViewModel.swift (8)
17-42: LGTM: SDK callback listeners.The
SSOCheckListenerandSetupKeyErrListenerclasses provide clean bridges from NetBirdSDK callbacks to Swift closures.
103-114: Excellent: Lazy preferences loading improves startup performance.The lazy loading of preferences avoids expensive SDK initialization (WireGuard/SSH key generation) at app startup. This is particularly important on tvOS where the SDK initialization is costly. The pattern is clean and well-commented.
123-137: LGTM: Deferred Rosenpass settings loading.Deferring the Rosenpass settings load is consistent with the lazy preferences strategy. Since these settings trigger expensive SDK initialization, loading them only when the settings view is accessed is the right trade-off.
148-165: LGTM: Enhanced connect with logging and button lock.The connect method now includes logging for debugging and a 3-second button lock to prevent rapid repeated taps. The async Task block properly awaits the network extension start.
230-261: LGTM: tvOS-specific state text mapping.The tvOS branch properly maps NEVPNStatus to human-readable text since tvOS doesn't use the iOS CustomLottieView. Fetching routes when connected (line 245) ensures the Networks counter on the home screen is accurate.
263-276: LGTM: Comprehensive logout cleanup for tvOS.The clearDetails method properly cleans up UserDefaults config and extension-local config on tvOS, preventing stale credentials from persisting after logout.
278-374: Excellent: Defensive preference methods.All preference-related methods now guard against nil preferences and return safe defaults where appropriate. This prevents crashes when the SDK fails to initialize (which can happen on tvOS) and aligns with the lazy loading strategy.
376-393: LGTM: Platform-appropriate relay connection defaults.The forced relay connection defaults to true on iOS (for battery optimization) and false on tvOS (where battery optimization is not needed). The comment clearly explains the rationale.
README.md
Outdated
|
|
||
| ## gomobile-fork | ||
|
|
||
| Since gomobile doesn't natively support tvOS targets, NetBird has created a fork that does. Please see the repo for more information: https://github.com/netbirdio/gomobile-tvos-fork |
There was a problem hiding this comment.
Wrap bare URLs in markdown link format.
Per markdownlint (MD034), bare URLs should be wrapped in markdown link syntax. Both lines contain unformatted URLs that should be formatted as [URL text](URL).
Apply this diff to fix the bare URL formatting:
-Since gomobile doesn't natively support tvOS targets, NetBird has created a fork that does. Please see the repo for more information: https://github.com/netbirdio/gomobile-tvos-fork
+Since gomobile doesn't natively support tvOS targets, NetBird has created a fork that does. Please see the repo for more information: [https://github.com/netbirdio/gomobile-tvos-fork](https://github.com/netbirdio/gomobile-tvos-fork)Also fix line 105:
-iOS/tvOS Client: https://github.com/netbirdio/ios-client (this repository)
+iOS/tvOS Client: [https://github.com/netbirdio/ios-client](https://github.com/netbirdio/ios-client) (this repository)Also applies to: 105-105
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
63-63: Bare URL used
(MD034, no-bare-urls)
🤖 Prompt for AI Agents
In README.md around lines 63 and 105, both lines contain bare URLs and violate
markdownlint MD034; replace the raw URLs with markdown link syntax by wrapping
each URL in [text](URL) form — for example use descriptive link text like
"gomobile tvOS fork" or the URL itself as text in square brackets — and update
both lines so they no longer contain bare URLs.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.github/workflows/build.yml:
- Around line 19-20: The workflow step named "Select Xcode 26.1.1" is selecting
a non-existent Xcode path; update the xcode-select target to the installed
toolchain by changing the path from
/Applications/Xcode_26.1.1.app/Contents/Developer to
/Applications/Xcode_26.2.app/Contents/Developer (i.e., update the run line in
the step that currently runs `sudo xcode-select -s
/Applications/Xcode_26.1.1.app/Contents/Developer` to use Xcode_26.2).
In @.github/workflows/test.yml:
- Around line 19-20: The workflow step referencing a non-existent Xcode "26.1.1"
should be updated to a valid macOS Xcode version; change the step name "Select
Xcode 26.1.1" and the run command path to a real Xcode install (for example
rename to "Select Xcode 16.2" and use sudo xcode-select -s
/Applications/Xcode_16.2.app/Contents/Developer or the appropriate
/Applications/Xcode_16.app path for your runner).
In @build-go-lib.sh:
- Line 19: The gomobile bind invocation in build-go-lib.sh currently targets
only iOS; update the command that starts with "CGO_ENABLED=0 gomobile bind
-target=..." to use "-target=ios,tvos" (i.e., CGO_ENABLED=0 gomobile bind
-target=ios,tvos -bundleid=io.netbird.framework -ldflags="...") so tvOS
architectures are included, and ensure the project uses the NetBird gomobile
tvOS fork (github.com/netbirdio/gomobile-tvos-fork) as documented in the README.
🧹 Nitpick comments (2)
.github/workflows/build.yml (1)
72-110: Dummy GoogleService-Info.plist approach is reasonable for CI.Creating a placeholder plist with disabled Firebase features (
IS_ANALYTICS_ENABLED,IS_GCM_ENABLED, etc. set tofalse) is a good approach for CI builds that don't require actual Firebase functionality. This allows the build to succeed without exposing real credentials.Consider adding a comment in the workflow explaining why this dummy file is needed, to help future maintainers understand the purpose.
.github/workflows/test.yml (1)
50-88: Duplicate plist content across workflows.The dummy
GoogleService-Info.plistis identical to the one inbuild.yml. This is fine for now, but if it needs to change, you'll have to update both files.Consider extracting this to a shared script or a checked-in dummy file (e.g.,
GoogleService-Info.plist.ci-template) to avoid duplication and reduce maintenance burden.♻️ Optional: Extract to a shared template file
Create a file like
ci/GoogleService-Info.plist.templateand reference it in both workflows:- name: Create dummy GoogleService-Info.plist working-directory: ios-client - run: | - cat > GoogleService-Info.plist << 'EOF' - <?xml version="1.0" encoding="UTF-8"?> - ... - EOF + run: cp ci/GoogleService-Info.plist.template GoogleService-Info.plist
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/build.yml.github/workflows/test.ymlbuild-go-lib.sh
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and Test
- GitHub Check: Build iOS App
🔇 Additional comments (3)
.github/workflows/build.yml (2)
16-16: LGTM on runner update.The macos-15 runner is appropriate for newer Xcode versions and tvOS support.
33-36: Go 1.24 is released and available—no action needed.Go 1.24 was released on February 11, 2025, and binaries are available from the official Go download pages. The specified version is current and appropriate.
.github/workflows/test.yml (1)
16-16: LGTM on runner update.Consistent with the build workflow.
struct - removed unused initializeConfigViaExtension method and converted some print statements to logger.debug for consistency
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@NetBird/Source/App/Views/TV/TVAuthView.swift`:
- Around line 325-337: The initial check calls onCompleteHandler when
checkComplete returns true but does not stop the recurring pollTimer; modify the
DispatchQueue.main.async block in TVAuthView where checkComplete is invoked so
that if isComplete is true you first invalidate and nil out pollTimer (the same
timer used by checkCompletionStatus) and then call onCompleteHandler to prevent
further timer firings and duplicate IPC calls; ensure you reference and update
the pollTimer property and keep these changes inside the main-queue block where
checkComplete, pollTimer, onCompleteHandler, and checkCompletionStatus are
managed.
In `@NetBirdTVNetworkExtension/PacketTunnelProvider.swift`:
- Around line 97-106: stopTunnel currently calls adapter?.stop() without
awaiting its completion and uses a fixed 2s delay before calling
completionHandler; update stopTunnel to call adapter?.stop(completion: ...) and
invoke the provided completionHandler only after the adapter stop completes
(inside the adapter stop completion block), still cancel and nil out pathMonitor
before or inside that block, and optionally keep a short fallback timeout to
guard against a non-returning adapter.stop; reference the stopTunnel method and
the adapter?.stop(...) call and ensure completionHandler() is called only from
the adapter stop completion (or the fallback timeout) to guarantee reliable
cleanup.
♻️ Duplicate comments (4)
NetbirdKit/NetworkExtensionAdapter.swift (2)
11-13: Remove unusedCombineimport.The
Combineframework is imported but not used anywhere in this file.
224-246: Consider caching the SDK client to avoid repeated instantiation.A new
NetBirdSDKNewClientis created on everyisLoginRequired()call. While this is called once per connection attempt, caching the client at the class level would improve efficiency.NetBirdTVNetworkExtension/PacketTunnelProvider.swift (2)
661-698: Unusedsuccessvariable and convoluted file handling.
- The
successvariable (line 661) is assigned but never used for control flow decisions.- Lines 670-680: The logic opens a
FileHandle, then writes an empty string to the file URL (truncating it), then writes via the handle. This is redundant - you could just truncate and write in one operation.♻️ Suggested simplification
- var success = false - let logMessage = "Starting new log file from TV extension" + "\n" guard let logURLValid = logURL else { print("Failed to get the log file URL.") return } - if fileManager.fileExists(atPath: logURLValid.path) { - if let fileHandle = try? FileHandle(forWritingTo: logURLValid) { - do { - try "".write(to: logURLValid, atomically: true, encoding: .utf8) - } catch { - print("Error handling the log file: \(error)") - } - if let data = logMessage.data(using: .utf8) { - fileHandle.write(data) - } - fileHandle.closeFile() - } else { - print("Failed to open the log file for writing.") - } - } else { - do { - try logMessage.write(to: logURLValid, atomically: true, encoding: .utf8) - } catch { - print("Failed to write to the log file: \(error.localizedDescription)") - } + // Truncate or create the log file and write initial message + do { + try logMessage.write(to: logURLValid, atomically: true, encoding: .utf8) + } catch { + print("Failed to write to the log file: \(error.localizedDescription)") } if let logPath = logURL?.path { - success = NetBirdSDKInitializeLog(loglevel, logPath, &error) - } - if !success, let actualError = error { - print("Failed to initialize log: \(actualError.localizedDescription)") + if !NetBirdSDKInitializeLog(loglevel, logPath, &error), let actualError = error { + print("Failed to initialize log: \(actualError.localizedDescription)") + } }
200-210: Potential race condition:start()called beforestop()completes.
restartClient()callsadapter?.stop()and immediatelyadapter?.start()without waiting for the stop to complete. This could cause issues if the adapter hasn't fully cleaned up.🔧 Suggested fix
func restartClient() { logger.info("restartClient: Restarting client due to network change") - adapter?.stop() - adapter?.start { error in - if let error = error { - logger.error("restartClient: Error restarting client: \(error.localizedDescription)") - } else { - logger.info("restartClient: Client restarted successfully") + adapter?.stop { + self.adapter?.start { error in + if let error = error { + logger.error("restartClient: Error restarting client: \(error.localizedDescription)") + } else { + logger.info("restartClient: Client restarted successfully") + } } } }
🧹 Nitpick comments (7)
NetbirdKit/RoutesSelectionDetails.swift (1)
9-17: Well-structured typed diagnostics model.The
LoginDiagnosticsstruct is a clean improvement over pipe-delimited string parsing, providing type safety and clear semantics for login state tracking.Consider moving to its own file. This struct is conceptually unrelated to routes selection details. For better code organization and discoverability, consider placing it in a dedicated
LoginDiagnostics.swiftfile or a more appropriately named shared models file.NetBird/Source/App/Views/TV/TVAuthView.swift (1)
214-231: Consider adding error feedback for QR code generation failure.If
filter.outputImageorcontext.createCGImagereturns nil, the user sees a perpetual loading spinner with no indication that something went wrong. While rare, this could leave users stuck.💡 Suggested improvement
private func generateQRCode() { let context = CIContext() let filter = CIFilter.qrCodeGenerator() filter.message = Data(loginURL.utf8) filter.correctionLevel = "M" - guard let outputImage = filter.outputImage else { return } + guard let outputImage = filter.outputImage else { + errorMessage = "Failed to generate QR code" + return + } // Scale up the QR code for better visibility let scale = 10.0 let transform = CGAffineTransform(scaleX: scale, y: scale) let scaledImage = outputImage.transformed(by: transform) if let cgImage = context.createCGImage(scaledImage, from: scaledImage.extent) { qrCodeImage = UIImage(cgImage: cgImage) + } else { + errorMessage = "Failed to render QR code" } }NetbirdKit/NetworkExtensionAdapter.swift (2)
70-70: Uselogger.errorinstead ofThis is the only place in
init()that usesprint()while the rest of the file uses the structuredLogger.♻️ Suggested fix
- print("Failed to configure manager") + logger.error("Failed to configure manager: \(error.localizedDescription)")
310-312: Remove redundant-> Voidreturn type.The explicit
Voidreturn type is redundant in Swift function declarations.♻️ Suggested fix
- func stop() -> Void { + func stop() { self.vpnManager?.connection.stopVPNTunnel() }NetBirdTVNetworkExtension/PacketTunnelProvider.swift (3)
76-84: Consider documenting the purpose of the 2-second delay.The
DispatchQueue.main.asyncAfter(deadline: .now() + 2)delay before returning the "Login required" error isn't immediately obvious. A brief comment explaining why this delay exists would improve maintainability.
230-231:UserDefaults.synchronize()is deprecated.While
synchronize()still works, it's deprecated since iOS 12. The system automatically synchronizesUserDefaultsat appropriate times. Consider removing unless there's a specific need in the extension context.
262-263:UserDefaults.synchronize()is deprecated.Same as noted in
setConfigFromMainApp- consider removing the deprecatedsynchronize()call.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
NetBird/Source/App/Views/TV/TVAuthView.swiftNetBirdTVNetworkExtension/PacketTunnelProvider.swiftNetbirdKit/NetworkExtensionAdapter.swiftNetbirdKit/RoutesSelectionDetails.swift
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-24T15:54:54.967Z
Learnt from: shuuri-labs
Repo: netbirdio/ios-client PR: 36
File: NetBird/Source/App/Views/TV/TVSettingsView.swift:611-617
Timestamp: 2025-12-24T15:54:54.967Z
Learning: On iOS, the pre-shared key (PSK) is stored in the SDK's config file, not in the Keychain. On tvOS, the config (including PSK) is stored in UserDefaults and transferred to the network extension via IPC as a single JSON blob, making Keychain integration architecturally more complex.
Applied to files:
NetbirdKit/NetworkExtensionAdapter.swift
📚 Learning: 2025-12-24T15:54:45.482Z
Learnt from: shuuri-labs
Repo: netbirdio/ios-client PR: 36
File: NetBird/Source/App/Views/TV/TVSettingsView.swift:611-617
Timestamp: 2025-12-24T15:54:45.482Z
Learning: Platform-specific PSK storage guidance: For iOS builds, store the pre-shared key (PSK) in the SDK config file rather than in the Keychain. For tvOS builds, store the config (including the PSK) in UserDefaults and transfer it to the network extension via inter-process communication (IPC) as a single JSON blob. This pattern avoids fragile cross-process Keychain integrations on tvOS and should be reflected in TV-related Swift files (e.g., NetBird/Source/App/Views/TV/). When reviewing code, verify PSK-related storage follows this separation and that the tvOS flow uses a JSON blob over IPC instead of direct Keychain access.
Applied to files:
NetBird/Source/App/Views/TV/TVAuthView.swift
🧬 Code graph analysis (2)
NetBird/Source/App/Views/TV/TVAuthView.swift (3)
NetBirdTVNetworkExtension/PacketTunnelProvider.swift (2)
onError(24-26)checkLoginComplete(346-414)NetbirdKit/NetworkExtensionAdapter.swift (2)
onError(24-26)checkLoginComplete(362-396)NetBird/Source/App/ViewModels/MainViewModel.swift (2)
onError(22-24)onError(36-38)
NetBirdTVNetworkExtension/PacketTunnelProvider.swift (7)
NetbirdKit/NetworkExtensionAdapter.swift (5)
onSuccess(20-22)onError(24-26)start(79-92)stop(310-312)login(314-357)NetBird/Source/App/ViewModels/MainViewModel.swift (4)
onSuccess(26-28)onSuccess(40-42)onError(22-24)onError(36-38)NetBird/Source/App/ViewModels/ServerViewModel.swift (4)
onSuccess(24-26)onSuccess(45-47)onError(28-32)onError(49-53)NetbirdNetworkExtension/PacketTunnelProvider.swift (9)
login(301-309)getStatus(311-378)getSelectRoutes(380-423)selectRoute(425-432)deselectRoute(434-441)restartClient(209-250)sleep(443-445)setTunnelSettings(450-458)initializeLogging(461-505)NetbirdKit/Preferences.swift (4)
hasConfigInUserDefaults(112-114)configFile(74-76)loadConfigFromUserDefaults(107-109)stateFile(78-80)NetbirdKit/Device.swift (1)
getName(11-17)NetBird/Source/App/Platform/Platform.swift (1)
size(45-47)
🪛 SwiftLint (0.57.0)
NetbirdKit/NetworkExtensionAdapter.swift
[Warning] 310-310: Returning Void in a function declaration is redundant
(redundant_void_return)
NetBirdTVNetworkExtension/PacketTunnelProvider.swift
[Warning] 139-139: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 143-143: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 247-247: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 266-266: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 275-275: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 284-284: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 304-304: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 314-314: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 321-321: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 321-321: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 325-325: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and Test
🔇 Additional comments (15)
NetbirdKit/RoutesSelectionDetails.swift (1)
6-7: LGTM on imports.
Foundationis required forCodable, andCombineis needed by the existingRoutesSelectionInfo: ObservableObjectclass below.NetBird/Source/App/Views/TV/TVAuthView.swift (2)
340-367: LGTM!The
checkCompletionStatushelper properly addresses the mutual exclusivity concern from prior reviews. Timer invalidation before calling the completion handler is correctly implemented.
19-199: Well-structured tvOS authentication view.The UI implementation follows tvOS conventions with appropriate sizing, spacing, and focus handling. The QR code + device code pattern is the correct approach for tvOS where Safari isn't available.
NetbirdKit/NetworkExtensionAdapter.swift (6)
15-27: LGTM!The
ConfigSSOListenerfollows the same callback pattern used in other ViewModels (e.g.,MainViewModel,ServerViewModel) for handling SSO results.
124-149: LGTM!The tvOS-specific flow correctly initializes config from
UserDefaultsand transfers it via IPC before checking login status.
151-191: LGTM!The tvOS config initialization properly handles the sandbox restrictions by checking
UserDefaultsfirst and sending config to the extension via IPC.
333-346: LGTM!The tvOS login flow correctly sends the
LoginTVmessage and parses theurl|userCoderesponse format matching the extension'sloginTVimplementation.
359-443: LGTM!The typed
LoginDiagnosticsstruct provides robust parsing compared to the previous pipe-delimited format. The user-friendly error messages incheckLoginErrorenhance the UX.
597-666: LGTM!The tvOS IPC methods for config transfer properly handle session availability and provide appropriate logging for debugging.
NetBirdTVNetworkExtension/PacketTunnelProvider.swift (6)
15-27: LGTM!The
ConfigInitSSOListenerfollows the same callback pattern used inNetworkExtensionAdapter.ConfigSSOListener.
108-156: LGTM - Completion handler is now called for all message types.All switch branches now properly call the
completionHandler, addressing the previous IPC contract issue.
270-332: LGTM!The config initialization properly handles the SSO check and config file creation with comprehensive error handling for all result cases.
344-414: LGTM!The diagnostic approach with
LoginDiagnosticsprovides comprehensive status information. The comment on lines 391-393 clearly documents whylastLoginResultis used instead ofclient.isLoginComplete().
416-500: LGTM!The device code login flow correctly handles the async callbacks with proper synchronization via
NSLockto ensure the completion handler is called exactly once.
502-569: LGTM!The status retrieval properly maps SDK status to the
StatusDetailsstruct with graceful fallback handling for encoding failures.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| checkComplete { isComplete in | ||
| DispatchQueue.main.async { | ||
| #if DEBUG | ||
| print("TVAuthView: Initial check - login complete = \(isComplete)") | ||
| #endif | ||
| if isComplete { | ||
| #if DEBUG | ||
| print("TVAuthView: Login already complete, dismissing auth view") | ||
| #endif | ||
| onCompleteHandler?() | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Timer not invalidated when initial check finds login already complete.
If checkComplete returns true on the initial check (lines 330-334), onCompleteHandler is called but pollTimer is not invalidated. The timer continues firing every 2 seconds, causing unnecessary IPC calls and potential duplicate onComplete invocations from checkCompletionStatus.
🐛 Proposed fix
checkComplete { isComplete in
DispatchQueue.main.async {
`#if` DEBUG
print("TVAuthView: Initial check - login complete = \(isComplete)")
`#endif`
if isComplete {
`#if` DEBUG
print("TVAuthView: Login already complete, dismissing auth view")
`#endif`
+ self.pollTimer?.invalidate()
onCompleteHandler?()
}
}
}🤖 Prompt for AI Agents
In `@NetBird/Source/App/Views/TV/TVAuthView.swift` around lines 325 - 337, The
initial check calls onCompleteHandler when checkComplete returns true but does
not stop the recurring pollTimer; modify the DispatchQueue.main.async block in
TVAuthView where checkComplete is invoked so that if isComplete is true you
first invalidate and nil out pollTimer (the same timer used by
checkCompletionStatus) and then call onCompleteHandler to prevent further timer
firings and duplicate IPC calls; ensure you reference and update the pollTimer
property and keep these changes inside the main-queue block where checkComplete,
pollTimer, onCompleteHandler, and checkCompletionStatus are managed.
| override func stopTunnel(with reason: NEProviderStopReason, completionHandler: @escaping () -> Void) { | ||
| adapter?.stop() | ||
| if let pathMonitor = self.pathMonitor { | ||
| pathMonitor.cancel() | ||
| self.pathMonitor = nil | ||
| } | ||
| DispatchQueue.main.asyncAfter(deadline: .now() + 2) { | ||
| completionHandler() | ||
| } | ||
| } |
There was a problem hiding this comment.
adapter.stop() completion is not awaited before calling completionHandler.
The adapter?.stop() method accepts an optional completion handler (as seen in the iOS extension), but here it's called without one. The 2-second delay is a workaround, but the adapter may not have fully stopped. Consider using the completion handler for reliable cleanup.
🔧 Suggested improvement
override func stopTunnel(with reason: NEProviderStopReason, completionHandler: `@escaping` () -> Void) {
- adapter?.stop()
if let pathMonitor = self.pathMonitor {
pathMonitor.cancel()
self.pathMonitor = nil
}
- DispatchQueue.main.asyncAfter(deadline: .now() + 2) {
- completionHandler()
+ if let adapter = adapter {
+ adapter.stop {
+ completionHandler()
+ }
+ } else {
+ completionHandler()
}
}🤖 Prompt for AI Agents
In `@NetBirdTVNetworkExtension/PacketTunnelProvider.swift` around lines 97 - 106,
stopTunnel currently calls adapter?.stop() without awaiting its completion and
uses a fixed 2s delay before calling completionHandler; update stopTunnel to
call adapter?.stop(completion: ...) and invoke the provided completionHandler
only after the adapter stop completes (inside the adapter stop completion
block), still cancel and nil out pathMonitor before or inside that block, and
optionally keep a short fallback timeout to guard against a non-returning
adapter.stop; reference the stopTunnel method and the adapter?.stop(...) call
and ensure completionHandler() is called only from the adapter stop completion
(or the fallback timeout) to guarantee reliable cleanup.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
NetbirdKit/RoutesSelectionDetails.swift (1)
30-48:ObservableObjectwithout@Publishedproperties won't trigger SwiftUI updates.
RoutesSelectionInfoconforms toObservableObject, but none of its properties are marked with@Published. SwiftUI views observing this object will not be notified when properties change.If reactive updates are needed, mark mutable properties with
@Published:Proposed fix
class RoutesSelectionInfo: ObservableObject, Codable, Identifiable { - var id: UUID - var name: String - var network: String? - var domains: [DomainDetails]? - var selected: Bool + `@Published` var id: UUID + `@Published` var name: String + `@Published` var network: String? + `@Published` var domains: [DomainDetails]? + `@Published` var selected: BoolNote: If
@Publishedis added, you will need to implement custominit(from:)andencode(to:)methods since@Publishedproperties don't get automatic Codable synthesis.Alternatively, if this class is only used for data transfer and not for reactive UI binding, consider converting it to a
structand removing theObservableObjectconformance.
🤖 Fix all issues with AI agents
In `@NetBirdTVNetworkExtension/PacketTunnelProvider.swift`:
- Around line 200-210: restartClient() currently calls adapter?.stop() and
immediately adapter?.start(), causing a race because stop() is asynchronous;
modify restartClient() to only call adapter?.start(...) after the stop operation
completes — either by using a completion callback on adapter.stop(completion:),
by observing adapter state (e.g., adapter.state == .stopped) and starting when
stopped, or by chaining start inside the existing stop completion; update the
adapter.stop API if necessary and move the adapter?.start { error in ... } call
into the stop completion handler so start runs strictly after stop finishes.
♻️ Duplicate comments (3)
NetBirdTVNetworkExtension/PacketTunnelProvider.swift (1)
97-106:adapter.stop()completion is not awaited before callingcompletionHandler.This was flagged in a previous review and remains unaddressed. The adapter's
stop()method likely accepts a completion handler (as seen in the iOS extension). The 2-second delay is a workaround that may not reliably wait for the adapter to finish stopping, potentially causing race conditions on restart.Suggested fix
override func stopTunnel(with reason: NEProviderStopReason, completionHandler: `@escaping` () -> Void) { - adapter?.stop() if let pathMonitor = self.pathMonitor { pathMonitor.cancel() self.pathMonitor = nil } - DispatchQueue.main.asyncAfter(deadline: .now() + 2) { - completionHandler() + if let adapter = adapter { + adapter.stop { + completionHandler() + } + } else { + completionHandler() } }NetbirdKit/NetworkExtensionAdapter.swift (2)
11-13: Remove unusedCombineimport.The
Combineframework is imported but not referenced anywhere in this file. OnlyNetBirdSDKandos(Logger) are actively used.
224-246: Consider caching the SDK client to avoid repeated instantiation.Both tvOS (line 224) and non-tvOS (line 262) branches create a new
NetBirdSDKNewClienton everyisLoginRequired()call. While this is called once per connection attempt rather than in a loop, caching the client at the class level would reduce unnecessary initialization overhead.
🧹 Nitpick comments (8)
NetbirdKit/RoutesSelectionDetails.swift (2)
60-70: RedundantEquatableextension forDomainDetails.
DomainDetailsalready conforms toHashable, which requiresEquatable. Since all stored properties (String,String?) areEquatable, Swift automatically synthesizes the==implementation. The explicitEquatableextension (lines 65-70) is unnecessary.Proposed simplification
struct DomainDetails: Codable, Hashable { let domain: String let resolvedips: String? } - -extension DomainDetails: Equatable { - static func == (lhs: DomainDetails, rhs: DomainDetails) -> Bool { - return lhs.domain == rhs.domain && - lhs.resolvedips == rhs.resolvedips - } -}
62-62: Consider Swift naming conventions forresolvedips.The property name
resolvedipsdoesn't follow Swift's camelCase convention. Consider renaming toresolvedIPswith aCodingKeysenum if the JSON key must remain lowercase.Proposed fix
struct DomainDetails: Codable, Hashable { let domain: String - let resolvedips: String? + let resolvedIPs: String? + + enum CodingKeys: String, CodingKey { + case domain + case resolvedIPs = "resolvedips" + } }NetBirdTVNetworkExtension/PacketTunnelProvider.swift (5)
136-143: Consider using non-optionalDatainitializer.Per SwiftLint, prefer
Data("true".utf8)over"true".data(using: .utf8)for guaranteed non-optional results when the string is a compile-time constant.Suggested fix
case let s where s.hasPrefix("Select-"): let id = String(s.dropFirst("Select-".count)) selectRoute(id: id) - completionHandler("true".data(using: .utf8)) + completionHandler(Data("true".utf8)) case let s where s.hasPrefix("Deselect-"): let id = String(s.dropFirst("Deselect-".count)) deselectRoute(id: id) - completionHandler("true".data(using: .utf8)) + completionHandler(Data("true".utf8))
230-231:synchronize()is deprecated.
UserDefaults.synchronize()is deprecated since iOS 12/tvOS 12. The system automatically synchronizes UserDefaults at appropriate times. Remove these calls.Suggested fix
UserDefaults.standard.set(configJSON, forKey: "netbird_config_json_local") - UserDefaults.standard.synchronize()
262-263: Remove deprecatedsynchronize()call.Same as above -
UserDefaults.synchronize()is deprecated and unnecessary.UserDefaults.standard.removeObject(forKey: "netbird_config_json_local") - UserDefaults.standard.synchronize()
674-687: Usedeferfor reliable file handle cleanup.While the current code does call
closeFile(), usingdeferensures the file handle is closed even if an exception occurs elsewhere in the block. This is a defensive coding practice.Suggested fix
if fileManager.fileExists(atPath: logURLValid.path) { if let fileHandle = try? FileHandle(forWritingTo: logURLValid) { + defer { fileHandle.closeFile() } do { try "".write(to: logURLValid, atomically: true, encoding: .utf8) } catch { print("Error handling the log file: \(error)") } if let data = logMessage.data(using: .utf8) { fileHandle.write(data) } - fileHandle.closeFile() } else {
676-680: Redundant file truncation logic.The code opens a
FileHandlefor writing, then usesString.write(to:atomically:encoding:)to truncate the file, then writes via the file handle. TheString.write()call creates a new file internally, making theFileHandlepotentially invalid. Consider usingFileHandle.truncateFile(atOffset:)instead, or simplifying by just usingString.write()for the entire operation.Simplified approach
if fileManager.fileExists(atPath: logURLValid.path) { - if let fileHandle = try? FileHandle(forWritingTo: logURLValid) { - do { - try "".write(to: logURLValid, atomically: true, encoding: .utf8) - } catch { - print("Error handling the log file: \(error)") - } - if let data = logMessage.data(using: .utf8) { - fileHandle.write(data) - } - fileHandle.closeFile() - } else { - print("Failed to open the log file for writing.") - } - } else { do { try logMessage.write(to: logURLValid, atomically: true, encoding: .utf8) } catch { - print("Failed to write to the log file: \(error.localizedDescription)") + print("Error writing to log file: \(error.localizedDescription)") } + } else { + do { + try logMessage.write(to: logURLValid, atomically: true, encoding: .utf8) + } catch { + print("Failed to create log file: \(error.localizedDescription)") + } }NetbirdKit/NetworkExtensionAdapter.swift (1)
310-312: Remove redundant-> Voidreturn type.Per SwiftLint, the explicit
-> Voidreturn type is redundant for functions that don't return a value.🧹 Suggested fix
- func stop() -> Void { + func stop() { self.vpnManager?.connection.stopVPNTunnel() }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
NetBirdTVNetworkExtension/PacketTunnelProvider.swiftNetbirdKit/NetworkExtensionAdapter.swiftNetbirdKit/RoutesSelectionDetails.swift
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-24T15:54:54.967Z
Learnt from: shuuri-labs
Repo: netbirdio/ios-client PR: 36
File: NetBird/Source/App/Views/TV/TVSettingsView.swift:611-617
Timestamp: 2025-12-24T15:54:54.967Z
Learning: On iOS, the pre-shared key (PSK) is stored in the SDK's config file, not in the Keychain. On tvOS, the config (including PSK) is stored in UserDefaults and transferred to the network extension via IPC as a single JSON blob, making Keychain integration architecturally more complex.
Applied to files:
NetbirdKit/NetworkExtensionAdapter.swift
🧬 Code graph analysis (2)
NetbirdKit/NetworkExtensionAdapter.swift (4)
NetBird/Source/App/ViewModels/MainViewModel.swift (4)
onSuccess(26-28)onSuccess(40-42)onError(22-24)onError(36-38)NetBird/Source/App/ViewModels/ServerViewModel.swift (4)
onSuccess(24-26)onSuccess(45-47)onError(28-32)onError(49-53)NetbirdKit/Preferences.swift (4)
loadConfigFromUserDefaults(107-109)configFile(74-76)stateFile(78-80)hasConfigInUserDefaults(112-114)NetBird/Source/App/Platform/Platform.swift (1)
size(45-47)
NetBirdTVNetworkExtension/PacketTunnelProvider.swift (4)
NetbirdKit/NetworkExtensionAdapter.swift (4)
onSuccess(20-22)onError(24-26)start(79-92)login(314-361)NetBird/Source/App/ViewModels/MainViewModel.swift (4)
onSuccess(26-28)onSuccess(40-42)onError(22-24)onError(36-38)NetBird/Source/App/ViewModels/ServerViewModel.swift (4)
onSuccess(24-26)onSuccess(45-47)onError(28-32)onError(49-53)NetBird/Source/App/Platform/Platform.swift (1)
size(45-47)
🪛 SwiftLint (0.57.0)
NetbirdKit/NetworkExtensionAdapter.swift
[Warning] 310-310: Returning Void in a function declaration is redundant
(redundant_void_return)
NetBirdTVNetworkExtension/PacketTunnelProvider.swift
[Warning] 139-139: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 143-143: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 247-247: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 266-266: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 275-275: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 284-284: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 304-304: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 314-314: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 321-321: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 321-321: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 325-325: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and Test
- GitHub Check: Build iOS App
🔇 Additional comments (12)
NetbirdKit/RoutesSelectionDetails.swift (1)
9-22: LGTM!The
LoginDiagnosticsandDeviceAuthResponsestructs are straightforward Codable models. The structure is clean and appropriate for JSON serialization.NetBirdTVNetworkExtension/PacketTunnelProvider.swift (3)
15-27: LGTM!The
ConfigInitSSOListenerimplementation follows the established callback pattern used elsewhere in the codebase (e.g.,MainViewModel.swift).
75-85: Clarify purpose of 2-second delay before returning login error.The
asyncAfter(deadline: .now() + 2)delay before callingcompletionHandlerwith the login-required error appears intentional but undocumented. This delays the VPN framework's error handling. If this is a workaround for a timing issue, please add a comment explaining why it's needed; otherwise, consider calling the completion handler immediately.if needsLogin { - DispatchQueue.main.asyncAfter(deadline: .now() + 2) { - let error = NSError( - domain: "io.netbird.NetBirdTVNetworkExtension", - code: 1001, - userInfo: [NSLocalizedDescriptionKey: "Login required."] - ) - completionHandler(error) - } + let error = NSError( + domain: "io.netbird.NetBirdTVNetworkExtension", + code: 1001, + userInfo: [NSLocalizedDescriptionKey: "Login required."] + ) + completionHandler(error) return }
416-504: LGTM!The device auth flow implementation handles the complex async scenario correctly:
- Uses
NSLockappropriately for thread-safe access tourlSentToApp- Ensures
completionHandleris called exactly once (either inonURLor inonErrorif URL was never sent)- Properly logs detailed diagnostic information for debugging
NetbirdKit/NetworkExtensionAdapter.swift (8)
15-27: LGTM!The
ConfigSSOListenercleanly encapsulates the SSO protocol callbacks with a single result closure, following the established pattern fromServerViewModelandMainViewModel.
79-92: LGTM!The
@MainActorannotation is appropriate since this method triggers UI state changes, and the added logging will aid tvOS debugging.
124-191: LGTM!The tvOS-specific initialization flow correctly addresses sandbox restrictions by checking UserDefaults first, then transferring config via IPC. The async wrapper properly uses
withCheckedContinuation.
289-307: LGTM!Good addition of the nil-session guard with early return and descriptive error logging.
314-361: LGTM!The tvOS-specific
LoginTVmessage andDeviceAuthResponsestruct decoding with fallback handling is well-implemented. TheuserCodeupdate correctly dispatches to the main thread since it's a@Publishedproperty.
363-447: LGTM!The
checkLoginComplete()andcheckLoginError()methods provide robust IPC-based login status checking with proper error handling. The user-friendly error message mapping incheckLoginError()improves the UX for common authentication failures.
637-670: LGTM!The
clearExtensionConfig()method provides proper cleanup for logout scenarios on tvOS, ensuring stale credentials don't persist in the extension.
33-35: ThedefaultManagementURLconstant is properly used in the tvOS codebase. It is referenced inNetBirdTVNetworkExtension/PacketTunnelProvider.swift(lines 290 and 427) and serves as a shared default value accessible to tvOS-specific components. No action required.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| #if os(tvOS) | ||
| /// Send config JSON to the Network Extension via IPC | ||
| /// On tvOS, shared UserDefaults doesn't work between app and extension, | ||
| /// so we transfer config directly via IPC | ||
| func sendConfigToExtension(_ configJSON: String, completion: ((Bool) -> Void)? = nil) { | ||
| guard let session = self.session else { | ||
| logger.warning("sendConfigToExtension: No session available") | ||
| completion?(false) | ||
| return | ||
| } | ||
|
|
||
| let messageString = "SetConfig:\(configJSON)" | ||
| guard let messageData = messageString.data(using: .utf8) else { | ||
| logger.error("sendConfigToExtension: Failed to convert message to Data") | ||
| completion?(false) | ||
| return | ||
| } | ||
|
|
||
| do { | ||
| try session.sendProviderMessage(messageData) { response in | ||
| if let response = response, | ||
| let responseString = String(data: response, encoding: .utf8), | ||
| responseString == "true" { | ||
| self.logger.info("sendConfigToExtension: Config sent successfully") | ||
| completion?(true) | ||
| } else { | ||
| self.logger.warning("sendConfigToExtension: Extension did not confirm receipt") | ||
| completion?(false) | ||
| } | ||
| } | ||
| } catch { | ||
| logger.error("sendConfigToExtension: Failed to send message: \(error.localizedDescription)") | ||
| completion?(false) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
NETunnelProviderSession sendProviderMessage size limit documentation
💡 Result:
Short answer: Apple does not document a fixed size limit for NETunnelProviderSession.sendProviderMessage; the API accepts an NSData and returns a response via a handler, but no maximum byte-size is specified in the documentation. (learn.microsoft.com)
Notes/implications:
- Practical limits come from the underlying IPC (Mach/XPC) and performance: Mach messages can be large (and support out-of-line memory), but large inline messages are slower and may hit system/resource constraints. Test and prefer smaller messages or out-of-band file transfer if you need to move large payloads. (gnu.org)
Sources:
[1] NETunnelProviderSession.SendProviderMessage (API reference). (learn.microsoft.com)
[2] Mach message / IPC behavior and performance notes. (gnu.org)
If you want, I can: (a) search Apple Developer forums or radar/technical notes for any unofficial limits; or (b) suggest an implementation pattern for sending large data (e.g., write to shared file / URL and send small message with reference). Which do you prefer?
Citations:
- 1: https://learn.microsoft.com/en-us/dotnet/api/networkextension.netunnelprovidersession.sendprovidermessage?utm_source=openai
- 2: https://www.gnu.org/software/hurd/gnumach-doc/Message-Format.html?utm_source=openai
- 3: https://learn.microsoft.com/en-us/dotnet/api/networkextension.netunnelprovidersession.sendprovidermessage?utm_source=openai
- 4: https://www.gnu.org/software/hurd/gnumach-doc/Message-Format.html?utm_source=openai
🌐 Web query:
Apple NetworkExtension tvOS IPC message size limits documentation
💡 Result:
Short answer: Apple does not publish a specific IPC/message-size limit for the NetworkExtension framework on tvOS. The NetworkExtension docs/headers don’t state a hard per-message cap, and Apple’s XPC/send-message documentation likewise does not list a single documented maximum for message payloads — so you should assume no documented fixed limit and test empirically for your use case (or switch to streaming/file-backed transfers for large payloads). [1][2]
Sources
- NetworkExtension framework headers/notes (tvOS SDK diffs showing NE Provider/API presence). [1]
- Apple XPC send(message:) reference (no explicit payload-size limit documented). [2]
If you want, I can: run/describe a small empirical test you can run on tvOS to measure practical limits, or search for community-reported limits.
Consider potential IPC message size constraints for large configurations.
The config JSON is sent as a single IPC message via sendProviderMessage. While Apple doesn't document a specific message size limit, the underlying IPC mechanism has practical constraints. For safety, consider adding validation to ensure configs don't grow unexpectedly large, or add logging when sending large payloads to help diagnose potential IPC issues.
| func restartClient() { | ||
| logger.info("restartClient: Restarting client due to network change") | ||
| adapter?.stop() | ||
| adapter?.start { error in | ||
| if let error = error { | ||
| logger.error("restartClient: Error restarting client: \(error.localizedDescription)") | ||
| } else { | ||
| logger.info("restartClient: Client restarted successfully") | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Race condition: start() called immediately after async stop().
The restartClient() method calls adapter?.stop() followed immediately by adapter?.start(). Since stop() is asynchronous, this creates a race condition where start() may execute before the adapter has fully stopped, potentially causing undefined behavior.
Suggested fix
func restartClient() {
logger.info("restartClient: Restarting client due to network change")
- adapter?.stop()
- adapter?.start { error in
- if let error = error {
- logger.error("restartClient: Error restarting client: \(error.localizedDescription)")
- } else {
- logger.info("restartClient: Client restarted successfully")
+ adapter?.stop {
+ self.adapter?.start { error in
+ if let error = error {
+ logger.error("restartClient: Error restarting client: \(error.localizedDescription)")
+ } else {
+ logger.info("restartClient: Client restarted successfully")
+ }
}
}
}🤖 Prompt for AI Agents
In `@NetBirdTVNetworkExtension/PacketTunnelProvider.swift` around lines 200 - 210,
restartClient() currently calls adapter?.stop() and immediately
adapter?.start(), causing a race because stop() is asynchronous; modify
restartClient() to only call adapter?.start(...) after the stop operation
completes — either by using a completion callback on adapter.stop(completion:),
by observing adapter state (e.g., adapter.state == .stopped) and starting when
stopped, or by chaining start inside the existing stop completion; update the
adapter.stop API if necessary and move the adapter?.start { error in ... } call
into the stop completion handler so start runs strictly after stop finishes.
…k, sdk version bump
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@build-go-lib.sh`:
- Around line 19-20: The CI is installing the upstream gomobile but the build
script uses gomobile-netbird; update the "Install gomobile" step in both
.github/workflows/test.yml and .github/workflows/build.yml to install
gomobile-netbird instead: change the existing go install
golang.org/x/mobile/cmd/gomobile... line to run go install
github.com/netbirdio/gomobile-tvos-fork/cmd/gomobile-netbird@latest and rename
the step to "Install gomobile-netbird" so the gomobile-netbird binary used by
build-go-lib.sh (which invokes gomobile-netbird bind) is available in the
runners.
In `@NetBirdTVNetworkExtension/PacketTunnelProvider.swift`:
- Around line 465-503: onSuccess currently logs success but never calls
completionHandler if onURL was never invoked, potentially leaving the caller
hanging; inside the onSuccess closure (function onSuccess), acquire urlSentLock,
check the urlSentToApp flag, and if it is false call completionHandler(nil) (and
optionally set a local flag to avoid double-calling), then unlock the lock —
ensure you use the same urlSentLock and urlSentToApp symbols so the behavior
matches the onError logic and that completionHandler is only invoked once.
- Around line 302-331: The locally created NetBirdSDKNewAuth instance (auth) is
deallocated before the async callback runs; assign it to a persistent instance
property (e.g., self.authRef) before creating the ConfigInitSSOListener and
calling auth.saveConfigIfSSOSupported(listener) so the object remains retained
for the duration of the async operation, and optionally clear self.authRef
inside the listener’s completion branch once work is done.
♻️ Duplicate comments (5)
README.md (2)
70-70: Wrap bare URL in markdown link format.Per markdownlint (MD034), bare URLs should use markdown link syntax:
-This project requires `gomobile-netbird`, NetBird's fork of gomobile that adds tvOS support. See: https://github.com/netbirdio/gomobile-tvos-fork +This project requires `gomobile-netbird`, NetBird's fork of gomobile that adds tvOS support. See: [gomobile-tvos-fork](https://github.com/netbirdio/gomobile-tvos-fork)
123-123: Wrap bare URL in markdown link format.-- iOS/tvOS Client: https://github.com/netbirdio/ios-client (this repository) +- iOS/tvOS Client: [ios-client](https://github.com/netbirdio/ios-client) (this repository)NetBirdTVNetworkExtension/PacketTunnelProvider.swift (3)
664-701: Unused initial assignment and file handle cleanup pattern.
success = falseon line 665 is never read before being overwritten on line 697.- The file handle should use
deferfor reliable cleanup.Suggested fix
var error: NSError? -var success = false let logMessage = "Starting new log file from TV extension" + "\n"For the file handle:
if fileManager.fileExists(atPath: logURLValid.path) { if let fileHandle = try? FileHandle(forWritingTo: logURLValid) { + defer { fileHandle.closeFile() } do { try "".write(to: logURLValid, atomically: true, encoding: .utf8) } catch { print("Error handling the log file: \(error)") } if let data = logMessage.data(using: .utf8) { fileHandle.write(data) } - fileHandle.closeFile() }
97-106:adapter.stop()completion is still not awaited before callingcompletionHandler.The adapter may not have fully stopped before
completionHandler()is invoked. The 2-second delay is an unreliable workaround. Use the adapter's completion handler for deterministic cleanup.Suggested improvement
override func stopTunnel(with reason: NEProviderStopReason, completionHandler: `@escaping` () -> Void) { - adapter?.stop() if let pathMonitor = self.pathMonitor { pathMonitor.cancel() self.pathMonitor = nil } - DispatchQueue.main.asyncAfter(deadline: .now() + 2) { - completionHandler() + if let adapter = adapter { + adapter.stop { + completionHandler() + } + } else { + completionHandler() } }
200-210: Race condition:start()still called immediately after asyncstop().The
adapter?.stop()call is asynchronous, butadapter?.start()is invoked immediately after, creating a race where start may execute before stop completes.Suggested fix
func restartClient() { logger.info("restartClient: Restarting client due to network change") - adapter?.stop() - adapter?.start { error in - if let error = error { - logger.error("restartClient: Error restarting client: \(error.localizedDescription)") - } else { - logger.info("restartClient: Client restarted successfully") + adapter?.stop { + self.adapter?.start { error in + if let error = error { + logger.error("restartClient: Error restarting client: \(error.localizedDescription)") + } else { + logger.info("restartClient: Client restarted successfully") + } } } }
🧹 Nitpick comments (3)
NetBirdTVNetworkExtension/PacketTunnelProvider.swift (3)
76-84: Unnecessary 2-second delay before returning login-required error.The
asyncAfterdelay appears arbitrary and delays error feedback to the caller without clear benefit. If there's a specific reason for this delay (e.g., allowing some initialization to complete), it should be documented; otherwise, consider returning the error immediately.if needsLogin { - DispatchQueue.main.asyncAfter(deadline: .now() + 2) { - let error = NSError( - domain: "io.netbird.NetBirdTVNetworkExtension", - code: 1001, - userInfo: [NSLocalizedDescriptionKey: "Login required."] - ) - completionHandler(error) - } + let error = NSError( + domain: "io.netbird.NetBirdTVNetworkExtension", + code: 1001, + userInfo: [NSLocalizedDescriptionKey: "Login required."] + ) + completionHandler(error) return }
136-154: Consider using non-optionalDatainitializer for string conversion.SwiftLint suggests using
Data("true".utf8)instead of"true".data(using: .utf8)for guaranteed non-optional conversion. This pattern appears throughout the file.Example fix for this section
case let s where s.hasPrefix("Select-"): let id = String(s.dropFirst("Select-".count)) selectRoute(id: id) - completionHandler("true".data(using: .utf8)) + completionHandler(Data("true".utf8)) case let s where s.hasPrefix("Deselect-"): let id = String(s.dropFirst("Deselect-".count)) deselectRoute(id: id) - completionHandler("true".data(using: .utf8)) + completionHandler(Data("true".utf8))This pattern should be applied consistently throughout the file (lines 247, 266, 275, 284, 304, 314, 321, 325).
230-231:UserDefaults.synchronize()is deprecated.Since iOS 12,
synchronize()is unnecessary and effectively a no-op. UserDefaults automatically persists changes. The same applies to line 263.UserDefaults.standard.set(configJSON, forKey: "netbird_config_json_local") -UserDefaults.standard.synchronize()
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
NetBirdTVNetworkExtension/PacketTunnelProvider.swiftREADME.mdbuild-go-lib.sh
🧰 Additional context used
🪛 GitHub Actions: Test
build-go-lib.sh
[error] 19-19: gomobile-netbird: command not found while executing './build-go-lib.sh ../netbird'.
🪛 markdownlint-cli2 (0.18.1)
README.md
70-70: Bare URL used
(MD034, no-bare-urls)
123-123: Bare URL used
(MD034, no-bare-urls)
🪛 SwiftLint (0.57.0)
NetBirdTVNetworkExtension/PacketTunnelProvider.swift
[Warning] 139-139: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 143-143: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 247-247: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 266-266: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 275-275: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 284-284: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 304-304: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 314-314: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 321-321: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 321-321: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 325-325: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and Test
🔇 Additional comments (9)
README.md (2)
37-110: Documentation updates look good.The README has been well-updated to reflect tvOS support with clear sections for requirements, installation of gomobile-netbird, and platform-specific running notes. The structure is logical and informative.
63-63: No action needed. The project is consistent with tvOS 17.0+ across all configurations. The README states tvOS 17.0+, the Xcode project deployment target is set to 17.0, and all documentation references mention tvOS 17.0+ consistently. There is no evidence of any tvOS 16.0+ requirement in the codebase or configuration.Likely an incorrect or invalid review comment.
NetBirdTVNetworkExtension/PacketTunnelProvider.swift (7)
17-27: LGTM!The
ConfigInitSSOListenerclass is a clean implementation of the SSO listener protocol with appropriate callback handling.
29-41: LGTM!Properties are appropriately initialized with lazy loading for the adapter and tunnel manager. The serial dispatch queue for network monitoring is correct.
158-198: LGTM!Network monitoring is correctly implemented with proper weak self capture to avoid retain cycles, and the logic correctly prevents unnecessary restarts on initial network detection.
344-414: LGTM!The
checkLoginCompletemethod provides comprehensive diagnostic information with proper error handling for encoding failures. The comments clearly document the difference between SDK login completion methods.
506-573: LGTM!The
getStatusmethod properly handles the SDK's indexed collection pattern and includes defensive fallback encoding if the primary encoding fails.
575-638: LGTM!Route selection methods are cleanly implemented with proper error handling and logging.
640-655: LGTM!The lifecycle methods are appropriately minimal for tvOS where connections are maintained during sleep mode.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| guard let auth = NetBirdSDKNewAuth(configPath, managementURL, nil) else { | ||
| logger.error("initializeConfig: Failed to create Auth object") | ||
| let data = "false".data(using: .utf8) | ||
| completionHandler(data) | ||
| return | ||
| } | ||
|
|
||
| // Use an SSO listener to save the config | ||
| let listener = ConfigInitSSOListener() | ||
| listener.onResult = { ssoSupported, error in | ||
| if let error = error { | ||
| logger.error("initializeConfig: Error checking SSO - \(error.localizedDescription)") | ||
| let data = "false".data(using: .utf8) | ||
| completionHandler(data) | ||
| } else if let supported = ssoSupported { | ||
| logger.info("initializeConfig: SSO supported = \(supported), config should be saved") | ||
| // Verify config was written | ||
| let configExists = fileManager.fileExists(atPath: configPath) | ||
| logger.info("initializeConfig: Config exists after save = \(configExists)") | ||
| let data = configExists ? "true".data(using: .utf8) : "false".data(using: .utf8) | ||
| completionHandler(data) | ||
| } else { | ||
| logger.warning("initializeConfig: Unknown result") | ||
| let data = "false".data(using: .utf8) | ||
| completionHandler(data) | ||
| } | ||
| } | ||
|
|
||
| // This will save the config if SSO is supported | ||
| auth.saveConfigIfSSOSupported(listener) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for all usages of NetBirdSDKNewAuth to understand the pattern
rg "NetBirdSDKNewAuth" -B 2 -A 5Repository: netbirdio/ios-client
Length of output: 2326
🏁 Script executed:
# Search for uses of saveConfigIfSSOSupported to see the pattern
rg "saveConfigIfSSOSupported" -B 3 -A 3Repository: netbirdio/ios-client
Length of output: 1679
🏁 Script executed:
# Check the full initializeConfig method to see if auth is stored anywhere
rg "func initializeConfig" -A 100 | head -150Repository: netbirdio/ios-client
Length of output: 15200
🏁 Script executed:
# Look for instance properties that store auth objects
rg "var auth" -B 2 -A 2Repository: netbirdio/ios-client
Length of output: 465
🏁 Script executed:
# Search for ConfigInitSSOListener implementation
rg "class ConfigInitSSOListener|struct ConfigInitSSOListener" -A 10Repository: netbirdio/ios-client
Length of output: 924
🏁 Script executed:
# Check if listener.onResult closure captures auth
rg "listener.onResult" -B 2 -A 2Repository: netbirdio/ios-client
Length of output: 614
🏁 Script executed:
# Look for the saveConfigIfSSOSupported method signature in SDK bindings
rg "saveConfigIfSSOSupported" -B 5 -A 10 | head -80Repository: netbirdio/ios-client
Length of output: 4239
Store auth object as instance property to prevent premature deallocation.
The auth object is created locally and passed to the async saveConfigIfSSOSupported() method. The SDK does not retain auth internally (as evidenced by the explicit retention pattern in NetBirdAdapter.swift), so it will be deallocated before the callback fires, causing undefined behavior or crashes.
Store auth as an instance property (e.g., self.authRef) as done elsewhere in the codebase, or ensure it remains in scope until the callback completes.
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 304-304: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 314-314: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 321-321: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 321-321: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
[Warning] 325-325: Prefer non-optional Data(_:) initializer when converting String to Data
(non_optional_string_data_conversion)
🤖 Prompt for AI Agents
In `@NetBirdTVNetworkExtension/PacketTunnelProvider.swift` around lines 302 - 331,
The locally created NetBirdSDKNewAuth instance (auth) is deallocated before the
async callback runs; assign it to a persistent instance property (e.g.,
self.authRef) before creating the ConfigInitSSOListener and calling
auth.saveConfigIfSSOSupported(listener) so the object remains retained for the
duration of the async operation, and optionally clear self.authRef inside the
listener’s completion branch once work is done.
| onSuccess: { | ||
| // Login completed - the app will detect this via polling | ||
| // and start the VPN tunnel via startVPNConnection() | ||
| logger.info("loginTV: Login completed successfully!") | ||
| logger.info("loginTV: Config should now be saved to App Group container") | ||
|
|
||
| // Debug: Verify config file was written | ||
| let configPath = Preferences.configFile() ?? "" | ||
| let statePath = Preferences.stateFile() ?? "" | ||
| let fileManager = FileManager.default | ||
| logger.info("loginTV: configFile exists = \(!configPath.isEmpty && fileManager.fileExists(atPath: configPath))") | ||
| logger.info("loginTV: stateFile exists = \(!statePath.isEmpty && fileManager.fileExists(atPath: statePath))") | ||
| }, | ||
| onError: { error in | ||
| // Log with privacy: .public to avoid iOS privacy redaction | ||
| if let nsError = error as NSError? { | ||
| logger.error("loginTV: Login failed - domain: \(nsError.domain, privacy: .public), code: \(nsError.code, privacy: .public), description: \(nsError.localizedDescription, privacy: .public)") | ||
| if let underlyingError = nsError.userInfo[NSUnderlyingErrorKey] as? Error { | ||
| logger.error("loginTV: Underlying error: \(String(describing: underlyingError), privacy: .public)") | ||
| } | ||
| } else { | ||
| logger.error("loginTV: Login failed: \(error?.localizedDescription ?? "unknown error", privacy: .public)") | ||
| } | ||
|
|
||
| // Only call completion with nil if we never sent the URL | ||
| // If URL was sent, the error just means the user didn't complete auth yet | ||
| // (e.g., device code expired) - but we already returned control to the app | ||
| urlSentLock.lock() | ||
| let alreadySentUrl = urlSentToApp | ||
| urlSentLock.unlock() | ||
|
|
||
| if !alreadySentUrl { | ||
| logger.error("loginTV: Error before URL was sent, returning nil to app") | ||
| completionHandler(nil) | ||
| } else { | ||
| logger.warning("loginTV: Error after URL was sent (device code may have expired), app is still polling") | ||
| } | ||
| } | ||
| ) |
There was a problem hiding this comment.
Edge case: completionHandler may never be called if onSuccess fires without onURL.
If the device auth flow somehow succeeds without triggering onURL (unlikely but possible depending on SDK behavior), completionHandler would never be called, leaving the caller hanging.
Consider adding a safeguard in onSuccess:
onSuccess: {
logger.info("loginTV: Login completed successfully!")
logger.info("loginTV: Config should now be saved to App Group container")
+
+ // Safety: ensure completionHandler was called
+ urlSentLock.lock()
+ let alreadySentUrl = urlSentToApp
+ urlSentLock.unlock()
+ if !alreadySentUrl {
+ logger.warning("loginTV: onSuccess without onURL - sending nil to unblock caller")
+ completionHandler(nil)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onSuccess: { | |
| // Login completed - the app will detect this via polling | |
| // and start the VPN tunnel via startVPNConnection() | |
| logger.info("loginTV: Login completed successfully!") | |
| logger.info("loginTV: Config should now be saved to App Group container") | |
| // Debug: Verify config file was written | |
| let configPath = Preferences.configFile() ?? "" | |
| let statePath = Preferences.stateFile() ?? "" | |
| let fileManager = FileManager.default | |
| logger.info("loginTV: configFile exists = \(!configPath.isEmpty && fileManager.fileExists(atPath: configPath))") | |
| logger.info("loginTV: stateFile exists = \(!statePath.isEmpty && fileManager.fileExists(atPath: statePath))") | |
| }, | |
| onError: { error in | |
| // Log with privacy: .public to avoid iOS privacy redaction | |
| if let nsError = error as NSError? { | |
| logger.error("loginTV: Login failed - domain: \(nsError.domain, privacy: .public), code: \(nsError.code, privacy: .public), description: \(nsError.localizedDescription, privacy: .public)") | |
| if let underlyingError = nsError.userInfo[NSUnderlyingErrorKey] as? Error { | |
| logger.error("loginTV: Underlying error: \(String(describing: underlyingError), privacy: .public)") | |
| } | |
| } else { | |
| logger.error("loginTV: Login failed: \(error?.localizedDescription ?? "unknown error", privacy: .public)") | |
| } | |
| // Only call completion with nil if we never sent the URL | |
| // If URL was sent, the error just means the user didn't complete auth yet | |
| // (e.g., device code expired) - but we already returned control to the app | |
| urlSentLock.lock() | |
| let alreadySentUrl = urlSentToApp | |
| urlSentLock.unlock() | |
| if !alreadySentUrl { | |
| logger.error("loginTV: Error before URL was sent, returning nil to app") | |
| completionHandler(nil) | |
| } else { | |
| logger.warning("loginTV: Error after URL was sent (device code may have expired), app is still polling") | |
| } | |
| } | |
| ) | |
| onSuccess: { | |
| // Login completed - the app will detect this via polling | |
| // and start the VPN tunnel via startVPNConnection() | |
| logger.info("loginTV: Login completed successfully!") | |
| logger.info("loginTV: Config should now be saved to App Group container") | |
| // Safety: ensure completionHandler was called | |
| urlSentLock.lock() | |
| let alreadySentUrl = urlSentToApp | |
| urlSentLock.unlock() | |
| if !alreadySentUrl { | |
| logger.warning("loginTV: onSuccess without onURL - sending nil to unblock caller") | |
| completionHandler(nil) | |
| } | |
| // Debug: Verify config file was written | |
| let configPath = Preferences.configFile() ?? "" | |
| let statePath = Preferences.stateFile() ?? "" | |
| let fileManager = FileManager.default | |
| logger.info("loginTV: configFile exists = \(!configPath.isEmpty && fileManager.fileExists(atPath: configPath))") | |
| logger.info("loginTV: stateFile exists = \(!statePath.isEmpty && fileManager.fileExists(atPath: statePath))") | |
| }, |
🤖 Prompt for AI Agents
In `@NetBirdTVNetworkExtension/PacketTunnelProvider.swift` around lines 465 - 503,
onSuccess currently logs success but never calls completionHandler if onURL was
never invoked, potentially leaving the caller hanging; inside the onSuccess
closure (function onSuccess), acquire urlSentLock, check the urlSentToApp flag,
and if it is false call completionHandler(nil) (and optionally set a local flag
to avoid double-calling), then unlock the lock — ensure you use the same
urlSentLock and urlSentToApp symbols so the behavior matches the onError logic
and that completionHandler is only invoked once.
…n after completing sso flow
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@NetBird/Source/App/Views/TV/TVMainView.swift`:
- Around line 79-94: The VPN is started immediately after calling
viewModel.networkExtensionAdapter.sendConfigToExtension(...) causing a race;
move the call to viewModel.networkExtensionAdapter.startVPNConnection() into the
sendConfigToExtension completion handler so it runs only after the transfer
finishes, and add handling for failure (when success == false) to avoid blindly
starting the VPN—use the existing Preferences.loadConfigFromUserDefaults() check
to decide whether to attempt start on failure or present/log an error instead.
🧹 Nitpick comments (1)
NetBird/Source/App/Views/TV/TVMainView.swift (1)
15-15: Consider removing unusedUIKitimport.
UIKitis imported but no UIKit-specific types appear to be used in this file. SwiftUI views don't require an explicit UIKit import.Proposed fix
import SwiftUI -import UIKit import NetworkExtension
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
NetBird/Source/App/Views/TV/TVMainView.swift
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-24T15:54:45.482Z
Learnt from: shuuri-labs
Repo: netbirdio/ios-client PR: 36
File: NetBird/Source/App/Views/TV/TVSettingsView.swift:611-617
Timestamp: 2025-12-24T15:54:45.482Z
Learning: Platform-specific PSK storage guidance: For iOS builds, store the pre-shared key (PSK) in the SDK config file rather than in the Keychain. For tvOS builds, store the config (including the PSK) in UserDefaults and transfer it to the network extension via inter-process communication (IPC) as a single JSON blob. This pattern avoids fragile cross-process Keychain integrations on tvOS and should be reflected in TV-related Swift files (e.g., NetBird/Source/App/Views/TV/). When reviewing code, verify PSK-related storage follows this separation and that the tvOS flow uses a JSON blob over IPC instead of direct Keychain access.
Applied to files:
NetBird/Source/App/Views/TV/TVMainView.swift
🧬 Code graph analysis (1)
NetBird/Source/App/Views/TV/TVMainView.swift (2)
NetbirdKit/Preferences.swift (1)
loadConfigFromUserDefaults(107-109)NetbirdKit/NetworkExtensionAdapter.swift (1)
sendConfigToExtension(605-635)
🔇 Additional comments (4)
NetBird/Source/App/Views/TV/TVMainView.swift (4)
121-228: Well-structured 10-foot UI layout.The
TVConnectionViewis well-designed for tvOS with appropriate spacing, sizing, and defensive computed properties that guard against accessing data when not connected. The status color mapping and stats display follow good patterns.
303-341: Clean presentational component.
TVStatCardis a well-structured, reusable presentation component with clean handling of the optionaltotalparameter.
343-348: LGTM!Standard preview setup for SwiftUI development.
292-299: The "Disconnecting" state edge case is already handled by the buttonLock guard.When
extensionStateTextis"Disconnecting", the condition at lines 292-293 is false, so the else branch would callconnect(). However, whenclose()is invoked,buttonLockis set to true (MainViewModel line 166) and remains locked for 3 seconds, covering the entire disconnection transition period. The guard at line 287 prevents any taps during this window, so no unintended connect attempt will occur. This defense-in-depth approach is effective given that the disconnection timeout andbuttonLockduration are aligned.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@NetBird/Source/App/NetBirdApp.swift`:
- Around line 22-37: Move Firebase initialization out of the background
DispatchQueue and call FirebaseApp.configure(...) on the main thread inside
AppDelegate's application(_:didFinishLaunchingWithOptions:) method: check
FirebaseApp.app() == nil first to avoid double-initialization, then load the
GoogleService-Info plist via Bundle.main.path(forResource:ofType:) and create
FirebaseOptions(contentsOfFile:) before calling FirebaseApp.configure(options:).
Ensure the call to FirebaseApp.configure is executed on the main thread (not
inside DispatchQueue.global).
In `@NetbirdKit/NetworkExtensionAdapter.swift`:
- Around line 155-161: The code silently ignores failures when sending the
stored config via IPC; modify sendConfigToExtensionAsync to return a Bool (true
on success, false on failure) by wrapping sendConfigToExtension callback in a
CheckedContinuation that resumes with the success flag, then update the call
site in initializeConfigFromApp (and its caller loginIfRequired) to await the
Bool result, log a warning/error via logger if the send failed (e.g.,
logger.warning("IPC config send failed: ...")) and let loginIfRequired handle
the failure path instead of assuming config was applied.
♻️ Duplicate comments (3)
NetbirdKit/NetworkExtensionAdapter.swift (3)
11-11: Remove unusedCombineimport.The
Combineframework is imported but not used anywhere in this file.
189-267: Client instantiation on every call remains unaddressed.As noted in a prior review, both the tvOS path (line 220) and non-tvOS path (line 258) create a new
NetBirdSDKNewClienton each invocation. While this is called infrequently (once per connection attempt), caching the client at the class level would avoid redundant initialization.
596-630: IPC message size consideration remains relevant.As noted previously, sending the full config JSON as a single IPC message may encounter practical size limits with large configurations. The current implementation is functional, but consider adding size logging or validation for debugging potential IPC issues with large payloads.
🧹 Nitpick comments (5)
NetBird/Source/App/NetBirdApp.swift (1)
42-43: Misleading comment about background thread.The comment states the ViewModel is created on a "background thread," but
ViewModelLoadercreates the ViewModel inside aTask {@mainactorin ... }within a@MainActorclass, which runs on the main thread asynchronously—not a background thread. This is still beneficial as it allows the splash screen to render before blocking initialization.- // Create ViewModel on background thread to avoid blocking app launch with Go runtime init + // Create ViewModel asynchronously to avoid blocking initial UI render with Go runtime initNetbirdKit/NetworkExtensionAdapter.swift (4)
264-264: Inconsistent logging: useloggerinstead ofprint().Line 264 uses
print()while the rest of the method useslogger. For consistency and better log filtering, uselogger.info()orlogger.debug().♻️ Suggested fix
let result = client.isLoginRequired() - print("isLoginRequired: SDK returned \(result)") + logger.info("isLoginRequired: SDK returned \(result)") return result
336-336: Useloggerinstead ofprint()for consistency.♻️ Suggested fix
} catch { - print("login: Failed to decode DeviceAuthResponse - \(error)") + logger.warning("login: Failed to decode DeviceAuthResponse - \(error)") // Fallback to plain string for backwards compatibility
358-395: Inconsistent logging: multipleprint()calls should uselogger.Lines 370, 380, 383, 387, and 392 use
print()while the method starts withlogger.error(). For consistency and structured logging, replaceprint()with appropriateloggercalls.
397-442: Consider extracting shared IPC diagnostic logic.Both
checkLoginComplete()andcheckLoginError()send the same"IsLoginComplete"message and decodeLoginDiagnostics. If both are called in sequence (e.g., during polling), this results in duplicate IPC calls. Consider a single method that returns the fullLoginDiagnosticsstruct, with callers extracting what they need.♻️ Sketch of consolidated approach
/// Fetch login diagnostics from the extension (single IPC call) func fetchLoginDiagnostics(completion: `@escaping` (LoginDiagnostics?) -> Void) { guard let session = self.session else { completion(nil) return } let messageString = "IsLoginComplete" guard let messageData = messageString.data(using: .utf8) else { completion(nil) return } do { try session.sendProviderMessage(messageData) { response in guard let response = response else { completion(nil) return } let diagnostic = try? self.decoder.decode(LoginDiagnostics.self, from: response) completion(diagnostic) } } catch { completion(nil) } } // Then callers can do: // fetchLoginDiagnostics { diag in // let isComplete = diag?.isComplete ?? false // let error = (diag?.lastResult == "error") ? diag?.lastError : nil // }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
NetBird/Source/App/NetBirdApp.swiftNetbirdKit/NetworkExtensionAdapter.swift
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-24T15:54:54.967Z
Learnt from: shuuri-labs
Repo: netbirdio/ios-client PR: 36
File: NetBird/Source/App/Views/TV/TVSettingsView.swift:611-617
Timestamp: 2025-12-24T15:54:54.967Z
Learning: On iOS, the pre-shared key (PSK) is stored in the SDK's config file, not in the Keychain. On tvOS, the config (including PSK) is stored in UserDefaults and transferred to the network extension via IPC as a single JSON blob, making Keychain integration architecturally more complex.
Applied to files:
NetbirdKit/NetworkExtensionAdapter.swift
🧬 Code graph analysis (1)
NetBird/Source/App/NetBirdApp.swift (2)
NetBird/Source/App/Views/Components/TransparentGradientButton.swift (1)
body(22-53)NetBird/Source/App/ViewModels/MainViewModel.swift (4)
checkExtensionState(225-256)checkLoginRequiredFlag(456-500)startPollingDetails(174-219)stopPollingDetails(221-223)
🔇 Additional comments (8)
NetBird/Source/App/NetBirdApp.swift (3)
11-18: LGTM!Clean conditional compilation for platform-specific Firebase imports.
65-106: LGTM!Clean conditional rendering with the splash screen pattern. The intentional omission of
checkLoginRequiredFlag()on tvOS aligns with the IPC-based login error detection documented inMainViewModel.swift.
111-125: LGTM!Good pattern for deferring heavy initialization while showing a loading state. The unstructured
Taskis acceptable here sinceViewModelLoaderis a@StateObjecton theAppand persists for the application's lifetime.NetbirdKit/NetworkExtensionAdapter.swift (5)
63-69: Good performance optimization with lazy manager configuration.Deferring
configureManager()frominit()to when it's actually needed avoids blocking app startup with slow system calls. The comment clearly documents this design decision.
75-88: LGTM - Comprehensive logging for VPN start sequence.The logging at entry, exit, and key checkpoints will help diagnose connection issues. The
@MainActorannotation ensures UI-safe updates.
632-664: LGTM - Clean implementation for clearing extension config.The
clearExtensionConfigmethod properly handles the session check, error cases, and provides feedback via the completion handler. The logging is appropriate.
33-35: Consider removing the duplicatedefaultManagementURLconstant or verify it's intentionally exposed as a tvOS API.This constant is defined at lines 33-35 for tvOS but is never referenced within this file. A duplicate definition exists in
NetBirdAdapter.swift(line 62), and external references inPacketTunnelProvider.swiftuse theNetBirdAdapterversion instead. If this is meant as a public API export for tvOS, clarify through documentation or naming; otherwise, consider removing the duplicate.
15-27: RemoveConfigSSOListener— it is unused dead code.This class is never instantiated anywhere in the codebase. Other modules define similar SSO listener classes (
ConfigInitSSOListener,LoginConfigSaveListener,SSOListenerImpl) which are actively used, butConfigSSOListenerremains unused.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| if let configJSON = Preferences.loadConfigFromUserDefaults(), !configJSON.isEmpty { | ||
| logger.info("initializeConfigFromApp: Config exists in UserDefaults, sending to extension via IPC") | ||
|
|
||
| // Send config to extension via IPC (settings are already in the JSON) | ||
| await sendConfigToExtensionAsync(configJSON) | ||
| return | ||
| } |
There was a problem hiding this comment.
Silent failure when IPC config transfer fails.
sendConfigToExtensionAsync is fire-and-forget here. If the IPC fails, the function returns without indication, and the caller (loginIfRequired) proceeds assuming config is ready. Consider logging a warning or returning a status to allow the caller to handle failures appropriately.
💡 Suggested improvement
if let configJSON = Preferences.loadConfigFromUserDefaults(), !configJSON.isEmpty {
logger.info("initializeConfigFromApp: Config exists in UserDefaults, sending to extension via IPC")
- // Send config to extension via IPC (settings are already in the JSON)
- await sendConfigToExtensionAsync(configJSON)
+ // Send config to extension via IPC (settings are already in the JSON)
+ let success = await sendConfigToExtensionAsync(configJSON)
+ if !success {
+ logger.warning("initializeConfigFromApp: Failed to send config to extension via IPC")
+ }
return
}This requires modifying sendConfigToExtensionAsync to return Bool:
private func sendConfigToExtensionAsync(_ configJSON: String) async -> Bool {
await withCheckedContinuation { (continuation: CheckedContinuation<Bool, Never>) in
sendConfigToExtension(configJSON) { success in
continuation.resume(returning: success)
}
}
}🤖 Prompt for AI Agents
In `@NetbirdKit/NetworkExtensionAdapter.swift` around lines 155 - 161, The code
silently ignores failures when sending the stored config via IPC; modify
sendConfigToExtensionAsync to return a Bool (true on success, false on failure)
by wrapping sendConfigToExtension callback in a CheckedContinuation that resumes
with the success flag, then update the call site in initializeConfigFromApp (and
its caller loginIfRequired) to await the Bool result, log a warning/error via
logger if the send failed (e.g., logger.warning("IPC config send failed: ..."))
and let loginIfRequired handle the failure path instead of assuming config was
applied.
… thread - Move startVPNConnection() inside sendConfigToExtension completion handler to ensure config transfer completes before VPN starts - Revert Firebase initialization to main thread as required by Firebase documentation - Keep ViewModelLoader with loading screen to improve perceived startup time - Keep lazy NetworkExtensionAdapter.configureManager() call removed from init
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/test.yml:
- Around line 39-40: The workflow step named "Install gomobile-netbird"
currently has an invalid run command containing only the module path; update
that step so the run command invokes the Go installer (e.g., replace the run
value "github.com/netbirdio/gomobile-tvos-fork/cmd/gomobile-netbird@latest" with
a proper "go install
github.com/netbirdio/gomobile-tvos-fork/cmd/gomobile-netbird@latest") so the
module is installed correctly during the CI job.
In `@NetBird.xcodeproj/project.pbxproj`:
- Line 59: The project file contains invalid non-hex UUIDs for the TVColors
entries (e.g., identifiers "44TVCOLORS12345678901234" and
"44TVCOLORSF12345678901234") which will break Xcode parsing; fix by replacing
those identifiers with valid 24-character hexadecimal UUIDs (0-9, A-F) for every
occurrence referencing TVColors.swift (search for "TVColors.swift",
"44TVCOLORS12345678901234", and "44TVCOLORSF12345678901234") or alternatively
remove and re-add TVColors.swift in Xcode so it regenerates correct UUIDs,
ensuring all references in PBXBuildFile and fileRef entries are updated
consistently.
♻️ Duplicate comments (8)
.github/workflows/build.yml (2)
119-145: Add build steps for tvOS targets.The PR introduces
NetBird TVandNetBirdTVNetworkExtensiontargets, but the workflow only builds iOS targets. Without tvOS build steps, regressions in tvOS code won't be caught by CI.🔧 Proposed fix — add tvOS build steps
- name: Build Network Extension working-directory: ios-client run: | set -o pipefail xcodebuild build \ -project NetBird.xcodeproj \ -scheme NetbirdNetworkExtension \ -destination 'generic/platform=iOS' \ -configuration Debug \ CODE_SIGNING_ALLOWED=NO \ CODE_SIGNING_REQUIRED=NO \ CODE_SIGN_IDENTITY="" \ | xcpretty --color + + - name: Build tvOS App + working-directory: ios-client + run: | + set -o pipefail + xcodebuild build \ + -project NetBird.xcodeproj \ + -scheme "NetBird TV" \ + -destination 'generic/platform=tvOS' \ + -configuration Debug \ + CODE_SIGNING_ALLOWED=NO \ + CODE_SIGNING_REQUIRED=NO \ + CODE_SIGN_IDENTITY="" \ + | xcpretty --color + + - name: Build tvOS Network Extension + working-directory: ios-client + run: | + set -o pipefail + xcodebuild build \ + -project NetBird.xcodeproj \ + -scheme "NetBirdTVNetworkExtension" \ + -destination 'generic/platform=tvOS' \ + -configuration Debug \ + CODE_SIGNING_ALLOWED=NO \ + CODE_SIGNING_REQUIRED=NO \ + CODE_SIGN_IDENTITY="" \ + | xcpretty --color#!/bin/bash # Verify the exact scheme names for tvOS targets in the Xcode project grep -r "NetBird.*TV" NetBird.xcodeproj/xcshareddata/xcschemes/ 2>/dev/null || \ fd -e xcscheme . NetBird.xcodeproj --exec grep -l -i "tv" {} \; 2>/dev/null || \ echo "Checking project.pbxproj for tvOS target names..." grep -E "name\s*=\s*\".*TV.*\"" NetBird.xcodeproj/project.pbxproj | head -10
16-20: Xcode 26.1.1 is not available on macos-15 runners — use Xcode 26.2.The macos-15 runner (as of January 2026) has Xcode 26.2 installed, not 26.1.1. This step will fail because
/Applications/Xcode_26.1.1.appdoes not exist.🔧 Proposed fix
- - name: Select Xcode 26.1.1 - run: sudo xcode-select -s /Applications/Xcode_26.1.1.app/Contents/Developer + - name: Select Xcode 26.2 + run: sudo xcode-select -s /Applications/Xcode_26.2.app/Contents/DeveloperNetBird/Source/App/Views/TV/TVMainView.swift (1)
83-96: Missing error handling when config transfer fails.The race condition from the previous review was addressed (VPN now starts inside the completion handler). However, the VPN still starts even when
success == false, which will likely cause a connection failure without user feedback.Proposed fix: Only start VPN on successful config transfer
viewModel.networkExtensionAdapter.sendConfigToExtension(configJSON) { success in - `#if` DEBUG - print("Config transfer \(success ? "succeeded" : "failed"), starting VPN connection...") - `#endif` - // Start VPN only after config transfer completes - viewModel.networkExtensionAdapter.startVPNConnection() + if success { + `#if` DEBUG + print("Config transfer succeeded, starting VPN connection...") + `#endif` + viewModel.networkExtensionAdapter.startVPNConnection() + } else { + `#if` DEBUG + print("Config transfer failed, cannot start VPN") + `#endif` + // Consider showing an alert to the user or triggering an error state + } }.github/workflows/test.yml (1)
19-20: Invalid Xcode version 26.1.1.This Xcode version does not exist. Current versions are in the 15.x-16.x range. This will cause the workflow to fail.
NetBird.xcodeproj/project.pbxproj (4)
957-957: UI component PeerCard.swift should not be compiled into network extension.
PeerCard.swiftis a SwiftUI view that shouldn't be included in theNetbirdNetworkExtensiontarget. Network extensions run in the background without UI capability. This unnecessarily increases binary size and violates separation of concerns.Remove this entry from the iOS network extension's Sources phase.
783-796: TV targets missing required resources will cause runtime failures.Both TV Resources phases are empty, but the TV app depends on Firebase (lines 629-630) and Lottie (line 631). Without
GoogleService-Info.plist, Firebase initialization will fail. Without animation JSON files, Lottie views will crash.Add to the TV app Resources phase:
GoogleService-Info.plist(file reference:506331F72AF1676B00BC8F0E)- Animation files:
button-disconnecting.json,logo_NetBird.json,button-connected.json,button-start-connecting.json,button-full.json,button-connecting-loop.json,button-full2.json,button-full2-dark.json,loading.json
612-618: Missing Crashlytics dSYM upload script for TV target.The NetBird TV target includes
FirebaseCrashlytics(line 630) but lacks the shell script build phase for dSYM upload that exists in the iOS target (lines 841-862). This results in unsymbolicated crash reports in Firebase console.Add a
PBXShellScriptBuildPhaseto the TV target'sbuildPhasesarray, mirroring the iOS configuration.
1107-1107: tvOS deployment target 17.0 conflicts with PR objectives stating 16.0+ support.All four tvOS build configurations set
TVOS_DEPLOYMENT_TARGET = 17.0, but the PR description explicitly states "tvOS 16.0+" support. This prevents the app from running on tvOS 16.x devices.Update all occurrences to
TVOS_DEPLOYMENT_TARGET = 16.0if 16.0+ support is intended, or update the PR description to reflect the actual minimum requirement.
🧹 Nitpick comments (1)
NetBird/Source/App/NetBirdApp.swift (1)
105-119: Clarify the async pattern's actual behavior.The current implementation shows a loading screen before the ViewModel initializes, but the
Task {@mainactorin }pattern doesn't move work off the main thread—it only defers execution to the next run loop iteration. If the Go runtime truly takes 10+ seconds, the main thread will still be blocked duringViewModel()initialization, potentially causing watchdog termination on tvOS.If the goal is only to display the loading screen first (before blocking), the current approach works. However, if you need to avoid blocking the main thread entirely, consider:
- Moving synchronous Go initialization to a background thread within
ViewModel.init()itself- Using
Task.detachedwith subsequent main-actor hop (though this has its own complexity with@StateObject)If the current behavior is intentional and acceptable, consider updating the comment to clarify:
- // Create ViewModel asynchronously on main thread - // The ViewModel itself must be created on MainActor since it's an ObservableObject + // Defer ViewModel creation to allow the loading screen to render first. + // Note: ViewModel initialization still runs on main thread and may block briefly.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/build.yml.github/workflows/test.ymlNetBird.xcodeproj/project.pbxprojNetBird/Source/App/NetBirdApp.swiftNetBird/Source/App/Views/TV/TVMainView.swift
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-24T15:54:45.482Z
Learnt from: shuuri-labs
Repo: netbirdio/ios-client PR: 36
File: NetBird/Source/App/Views/TV/TVSettingsView.swift:611-617
Timestamp: 2025-12-24T15:54:45.482Z
Learning: Platform-specific PSK storage guidance: For iOS builds, store the pre-shared key (PSK) in the SDK config file rather than in the Keychain. For tvOS builds, store the config (including the PSK) in UserDefaults and transfer it to the network extension via inter-process communication (IPC) as a single JSON blob. This pattern avoids fragile cross-process Keychain integrations on tvOS and should be reflected in TV-related Swift files (e.g., NetBird/Source/App/Views/TV/). When reviewing code, verify PSK-related storage follows this separation and that the tvOS flow uses a JSON blob over IPC instead of direct Keychain access.
Applied to files:
NetBird/Source/App/Views/TV/TVMainView.swift
🧬 Code graph analysis (2)
NetBird/Source/App/NetBirdApp.swift (2)
NetBird/Source/App/Views/Components/TransparentGradientButton.swift (1)
body(22-53)NetBird/Source/App/ViewModels/MainViewModel.swift (4)
checkExtensionState(225-256)checkLoginRequiredFlag(456-500)startPollingDetails(174-219)stopPollingDetails(221-223)
NetBird/Source/App/Views/TV/TVMainView.swift (4)
NetBird/Source/App/Views/Components/TransparentGradientButton.swift (1)
body(22-53)NetbirdKit/Preferences.swift (1)
loadConfigFromUserDefaults(107-109)NetbirdKit/NetworkExtensionAdapter.swift (5)
sendConfigToExtension(600-630)startVPNConnection(284-302)onError(24-26)checkLoginComplete(361-395)checkLoginError(399-442)NetBirdTVNetworkExtension/PacketTunnelProvider.swift (2)
onError(24-26)checkLoginComplete(346-414)
🔇 Additional comments (16)
.github/workflows/build.yml (2)
39-40: LGTM — gomobile-netbird fork enables tvOS bindings.Using the forked
gomobile-netbirdpackage is correct for building xcframeworks that include tvOS slices, aligning with thebuild-go-lib.shchanges.
72-111: Dummy plist enables Firebase SDK compilation without real credentials.This approach is appropriate for CI builds where actual Firebase configuration isn't needed. The plist contains all required keys with placeholder values, allowing the build to succeed.
NetBird/Source/App/Views/TV/TVMainView.swift (6)
14-23: LGTM!Imports and tvOS compilation guard are appropriate. The module-level logger scoped to "TVConnectionButton" is a reasonable pattern for centralized button action logging.
62-119: Authentication flow integration looks correct.The
TVAuthViewintegration properly handles:
- Login URL and user code propagation
- Polling via
checkLoginCompleteand error checking viacheckLoginError- Proper closure delegation to the network extension adapter
123-230: LGTM!The
TVConnectionViewlayout is well-structured for tvOS 10-foot UI with appropriate spacing and sizing. The computed properties correctly handle the disconnected state by returning "0" before filtering collections, avoiding unnecessary work when not connected.
232-303: LGTM!The
TVConnectionButtonproperly implements tvOS-specific focus handling with@FocusStateand scale animation. ThehandleTaplogic correctly differentiates between connect and disconnect actions, with appropriate logging and button lock checks.
305-343: LGTM!Clean, reusable component with appropriate conditional rendering for the optional total value.
345-352: LGTM!Standard preview provider setup.
.github/workflows/test.yml (1)
50-88: LGTM!Creating a dummy GoogleService-Info.plist for CI is a sensible approach to satisfy build requirements without exposing real Firebase credentials. Analytics and other features are appropriately disabled.
NetBird/Source/App/NetBirdApp.swift (4)
11-18: LGTM!The conditional compilation for platform-specific imports is appropriate. FirebasePerformance is correctly guarded for iOS-only, and the Combine import supports the notification publishers used for lifecycle handling.
20-35: LGTM! Firebase initialization correctly moved to main thread.The Firebase configuration is now properly called on the main thread within the AppDelegate's
application(_:didFinishLaunchingWithOptions:)method, which addresses the previous review concern. The conditional compilation correctly scopes this to iOS only.
37-56: LGTM!The platform-specific initialization is well-structured. The tvOS Firebase configuration in
init()executes on the main thread (App'sinit()runs on main), satisfying Firebase's thread requirements. TheViewModelLoaderpattern appropriately defers the potentially slow Go runtime initialization.
57-103: Platform-specific lifecycle handling is correctly implemented.The conditional compilation appropriately separates iOS notification-based lifecycle handling from tvOS's
scenePhaseapproach. The omission ofcheckLoginRequiredFlag()on tvOS is intentional—tvOS handles login errors via IPC rather than the flag-based approach used on iOS.The
onChange(of:scenePhase)signature with two parameters{ _, newPhase in }is correct: the project's deployment target is tvOS 17.0, which fully supports this API.NetBird.xcodeproj/project.pbxproj (3)
1069-1148: LGTM: TV target configurations are well-structured.The NetBird TV build configurations correctly specify:
- Consistent bundle identifiers (
io.netbird.app.tv)- Matching development team across Debug/Release
- Proper entitlements files for both configurations
- Correct tvOS-specific settings (
TARGETED_DEVICE_FAMILY = 3,SDKROOT = appletvos)Previous review concerns about bundle ID and entitlements mismatches have been addressed.
1149-1222: LGTM: TV network extension configurations are correct.The extension configurations properly specify
SKIP_INSTALL = YES, consistent bundle identifiers, and matching entitlements for Debug/Release. The extension correctly embeds into the main TV app via the dependency chain.
888-918: The review comment is based on an incorrect assumption. CustomLottieView.swift is intentionally not included in the TV app sources. The codebase contains an explicit comment in MainViewModel.swift stating "On tvOS, update extensionStateText directly since we don't have CustomLottieView," confirming that TV views use an alternative implementation path that does not require this component. Search results show no Lottie usage in TV views.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/test.yml:
- Around line 57-96: The workflow step "Create dummy GoogleService-Info.plist"
currently uses working-directory: ios-client which doesn't exist; update that
step (the job named "Create dummy GoogleService-Info.plist" or the block
containing working-directory: ios-client) to remove the working-directory
directive or change it to working-directory: . so the cat heredoc writes
GoogleService-Info.plist into the repository root where NetBird.xcodeproj and
the iOS sources live.
♻️ Duplicate comments (1)
.github/workflows/test.yml (1)
19-21: Xcode version 26.1.1 does not exist.This was flagged in a previous review but appears unresolved. Xcode versions are currently in the 15.x-16.x range. This will cause the workflow to fail.
🐛 Proposed fix
- - name: Select Xcode 26.1.1 - run: sudo xcode-select -s /Applications/Xcode_26.1.1.app/Contents/Developer + - name: Select Xcode 16.2 + run: sudo xcode-select -s /Applications/Xcode_16.2.app/Contents/Developer#!/bin/bash # Verify available Xcode versions on macos-15 runners curl -s https://raw.githubusercontent.com/actions/runner-images/main/images/macos/macos-15-Readme.md | grep -A 20 "Xcode"
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test.yml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: shuuri-labs
Repo: netbirdio/ios-client PR: 36
File: NetBird/Source/App/Views/TV/TVSettingsView.swift:611-617
Timestamp: 2025-12-24T15:54:54.967Z
Learning: On iOS, the pre-shared key (PSK) is stored in the SDK's config file, not in the Keychain. On tvOS, the config (including PSK) is stored in UserDefaults and transferred to the network extension via IPC as a single JSON blob, making Keychain integration architecturally more complex.
🔇 Additional comments (3)
.github/workflows/test.yml (3)
16-16: LGTM!The runner update to
macos-15is appropriate for the tvOS support additions in this PR.
39-45: LGTM!The gomobile-netbird installation and initialization steps are correctly structured. The PATH export ensures the
gomobile-netbirdbinary is discoverable.
47-51: LGTM!The PATH export before running
build-go-lib.shensures the Go binaries installed in the previous step are available.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@build-go-lib.sh`:
- Around line 19-20: The script uses the wrong gomobile binary and omits tvOS
targets: replace the calls to gomobile (the lines invoking "gomobile init" and
"gomobile bind" in build-go-lib.sh) with the forked tool "gomobile-netbird" and
update the bind target flag on the NetBirdSDK bind command from
"-target=ios,iossimulator" to "-target=ios,iossimulator,tvos,tvossimulator".
Also update CI install steps in .github/workflows/build.yml and
.github/workflows/test.yml by replacing the current "go install
golang.org/x/mobile/cmd/gomobile@latest" with "go install
github.com/netbirdio/gomobile-tvos-fork/cmd/gomobile-netbird@latest" so the
workflows install the correct forked binary.
♻️ Duplicate comments (2)
.github/workflows/build.yml (2)
119-145: Add build steps for tvOS targets.This PR adds tvOS support with
NetBird TVandNetBirdTVNetworkExtensiontargets, but the CI workflow only builds the iOS app and extension. Add build steps for the tvOS targets to ensure they compile successfully.Proposed additions after the iOS build steps
- name: Build tvOS App working-directory: ios-client run: | set -o pipefail xcodebuild build \ -project NetBird.xcodeproj \ -scheme "NetBird TV" \ -destination 'generic/platform=tvOS' \ -configuration Debug \ CODE_SIGNING_ALLOWED=NO \ CODE_SIGNING_REQUIRED=NO \ CODE_SIGN_IDENTITY="" \ | xcpretty --color - name: Build tvOS Network Extension working-directory: ios-client run: | set -o pipefail xcodebuild build \ -project NetBird.xcodeproj \ -scheme NetBirdTVNetworkExtension \ -destination 'generic/platform=tvOS' \ -configuration Debug \ CODE_SIGNING_ALLOWED=NO \ CODE_SIGNING_REQUIRED=NO \ CODE_SIGN_IDENTITY="" \ | xcpretty --colorRun the following to verify the tvOS scheme names:
#!/bin/bash # Find available schemes in the Xcode project xcodebuild -project ios-client/NetBird.xcodeproj -list 2>/dev/null | grep -A 50 "Schemes:" || \ rg -i "NetBird.*TV|tvos" ios-client/NetBird.xcodeproj/project.pbxproj | head -20
19-20: Xcode 26.1.1 is not available on the macos-15 runner.The macos-15 GitHub Actions runner (as of January 2026) includes Xcode 26.2, not 26.1.1. This step will fail because
/Applications/Xcode_26.1.1.appdoes not exist. Update to use the installed version.Proposed fix
- - name: Select Xcode 26.1.1 - run: sudo xcode-select -s /Applications/Xcode_26.1.1.app/Contents/Developer + - name: Select Xcode 26.2 + run: sudo xcode-select -s /Applications/Xcode_26.2.app/Contents/Developer
🧹 Nitpick comments (1)
.github/workflows/build.yml (1)
40-40: Consider pinning gomobile to a specific version for reproducible builds.Using
@latestcan lead to non-reproducible builds if gomobile introduces breaking changes. Pin to a specific version or commit hash to ensure consistent CI behavior.Proposed fix
- run: go install golang.org/x/mobile/cmd/gomobile@latest + run: go install golang.org/x/mobile/cmd/[email protected]Replace the commit hash with the latest known-working version.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/build.yml.github/workflows/test.ymlbuild-go-lib.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/test.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and Test
- GitHub Check: Build iOS App
🔇 Additional comments (1)
.github/workflows/build.yml (1)
72-110: LGTM!Creating a dummy
GoogleService-Info.plistfor CI builds is a sensible approach to allow compilation without real Firebase credentials. The plist structure is correct and all Firebase features are appropriately disabled.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| gomobile init | ||
| CGO_ENABLED=0 gomobile bind -target=ios -bundleid=io.netbird.framework -ldflags="-X github.com/netbirdio/netbird/version.version=$version" -o $rn_app_path/NetBird/NetBirdSDK.xcframework $netbirdPath/client/ios/NetBirdSDK | ||
| CGO_ENABLED=0 gomobile bind -target=ios,iossimulator -bundleid=io.netbird.framework -ldflags="-X github.com/netbirdio/netbird/version.version=$version" -o $rn_app_path/NetBirdSDK.xcframework $netbirdPath/client/ios/NetBirdSDK |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Checking build-go-lib.sh ==="
cat build-go-lib.sh 2>/dev/null || echo "File not found at root"
# Search for build-go-lib.sh
echo -e "\n=== Finding build-go-lib.sh ==="
find . -name "build-go-lib.sh" -type f 2>/dev/null | head -5Repository: netbirdio/ios-client
Length of output: 614
🏁 Script executed:
#!/bin/bash
echo "=== Checking CI workflows for gomobile references ==="
echo "--- .github/workflows/test.yml ---"
grep -n -i "gomobile" .github/workflows/test.yml 2>/dev/null || echo "No gomobile reference found"
echo -e "\n--- .github/workflows/build.yml ---"
grep -n -i "gomobile" .github/workflows/build.yml 2>/dev/null || echo "No gomobile reference found"
echo -e "\n--- All workflow files ---"
find .github/workflows -name "*.yml" -o -name "*.yaml" 2>/dev/null | xargs grep -l "gomobile" 2>/dev/null || echo "No gomobile found in any workflows"Repository: netbirdio/ios-client
Length of output: 474
🏁 Script executed:
#!/bin/bash
echo "=== Searching for gomobile fork references ==="
grep -r "gomobile-netbird" . --include="*.md" --include="*.sh" --include="*.yml" --include="*.yaml" 2>/dev/null | head -20
echo -e "\n=== Searching for tvOS references ==="
grep -r -i "tvos" . --include="*.md" --include="*.sh" --include="*.go" --include="*.yml" 2>/dev/null | head -20
echo -e "\n=== Searching for gomobile fork setup ==="
grep -r "gomobile" . --include="*.sh" --include="*.md" --include="*.yml" 2>/dev/null | grep -v "Binary file" | head -30Repository: netbirdio/ios-client
Length of output: 3124
🏁 Script executed:
#!/bin/bash
echo "=== Checking README for gomobile documentation ==="
grep -A 5 -B 5 -i "gomobile" README.md 2>/dev/null | head -50
echo -e "\n=== Checking if there's a setup or installation doc ==="
find . -name "README*" -o -name "SETUP*" -o -name "INSTALL*" | head -10Repository: netbirdio/ios-client
Length of output: 1187
🏁 Script executed:
#!/bin/bash
echo "=== Listing .github/workflows directory ==="
ls -la .github/workflows/ 2>/dev/null || echo "Workflows directory not found"
echo -e "\n=== Checking top-level files ==="
ls -la | grep -E "\.(md|sh|yml|yaml)$"Repository: netbirdio/ios-client
Length of output: 568
Fix script to use gomobile-netbird and add tvOS targets; fix CI workflows.
The script uses standard gomobile instead of gomobile-netbird, which doesn't support tvOS. Additionally, the targets are missing tvOS and tvossimulator, and the CI workflows are installing the wrong gomobile version.
Required fixes:
- Replace
gomobilewithgomobile-netbirdon lines 19-20 - Change
-target=ios,iossimulatorto-target=ios,iossimulator,tvos,tvossimulator - Update
.github/workflows/build.ymland.github/workflows/test.ymlto install the correct fork:- Replace:
go install golang.org/x/mobile/cmd/gomobile@latest - With:
go install github.com/netbirdio/gomobile-tvos-fork/cmd/gomobile-netbird@latest
- Replace:
The README correctly documents these requirements but the implementation does not follow them.
Proposed fix for build-go-lib.sh
cd $netbirdPath
-gomobile init
-CGO_ENABLED=0 gomobile bind -target=ios,iossimulator -bundleid=io.netbird.framework -ldflags="-X github.com/netbirdio/netbird/version.version=$version" -o $rn_app_path/NetBirdSDK.xcframework $netbirdPath/client/ios/NetBirdSDK
+gomobile-netbird init
+CGO_ENABLED=0 gomobile-netbird bind -target=ios,iossimulator,tvos,tvossimulator -bundleid=io.netbird.framework -ldflags="-X github.com/netbirdio/netbird/version.version=$version" -o $rn_app_path/NetBirdSDK.xcframework $netbirdPath/client/ios/NetBirdSDK🤖 Prompt for AI Agents
In `@build-go-lib.sh` around lines 19 - 20, The script uses the wrong gomobile
binary and omits tvOS targets: replace the calls to gomobile (the lines invoking
"gomobile init" and "gomobile bind" in build-go-lib.sh) with the forked tool
"gomobile-netbird" and update the bind target flag on the NetBirdSDK bind
command from "-target=ios,iossimulator" to
"-target=ios,iossimulator,tvos,tvossimulator". Also update CI install steps in
.github/workflows/build.yml and .github/workflows/test.yml by replacing the
current "go install golang.org/x/mobile/cmd/gomobile@latest" with "go install
github.com/netbirdio/gomobile-tvos-fork/cmd/gomobile-netbird@latest" so the
workflows install the correct forked binary.
Capture full xcodebuild output and system crash logs when tests fail. This will help debug the test crash that occurs during app bootstrapping in CI but not locally. - Tee test output to file before xcpretty filtering - Search for and print crash reports from DiagnosticReports - Upload test logs and crash reports as artifacts on failure
|
|
||
| gomobile init | ||
| CGO_ENABLED=0 gomobile bind -target=ios -bundleid=io.netbird.framework -ldflags="-X github.com/netbirdio/netbird/version.version=$version" -o $rn_app_path/NetBird/NetBirdSDK.xcframework $netbirdPath/client/ios/NetBirdSDK | ||
| CGO_ENABLED=0 gomobile bind -target=ios,iossimulator -bundleid=io.netbird.framework -ldflags="-X github.com/netbirdio/netbird/version.version=$version" -o $rn_app_path/NetBirdSDK.xcframework $netbirdPath/client/ios/NetBirdSDK |
There was a problem hiding this comment.
Shouldn't this be changed to gomobile-netbird?
This commit introduces full tvOS support for the NetBird iOS client, enabling VPN connectivity on Apple TV devices.
Added NetBird TV app target with tvOS 16.0+ deployment
Created tvOS-specific UI using SwiftUI optimized for "10-foot experience"
Tab-based navigation: Connection, Peers, Networks, Settings
Large touch targets and text for Siri Remote navigation
TVMainView: Main tab navigation and connection status
TVConnectionView: Large connect/disconnect button with status display
TVPeersView: List of connected peers with connection details
TVNetworksView: Network routes selection and management
TVSettingsView: App settings and logout functionality
TVAuthView: QR code + device code authentication flow
Implemented OAuth device authorization flow for tvOS
Displays QR code that user scans with phone to authenticate
Shows user code as fallback for manual entry
Polls for authentication completion and auto-dismisses on success
tvOS has stricter sandbox restrictions than iOS:
UserDefaults-based Config Storage
Preloaded Config in Go SDK
Raw Syscall Tunnel FD Discovery
Added NetBirdTVNetworkExtension target
Separate PacketTunnelProvider.swift with tvOS-specific handling
Extensive logging for debugging via Console.app
Handles "LoginTV" message for device auth flow
Loads config from UserDefaults into SDK memory
isLoginRequired() now verifies session with management server
Previously only checked if config existed (caused post-restart failures)
Shows QR code re-auth flow when OAuth session expires
Added Platform.swift for iOS/tvOS conditional compilation
Shared code uses #if os(tvOS) / #if os(iOS) where needed
Common ViewModels work across both platforms
Summary by CodeRabbit
New Features
Improvements
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.