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
WalkthroughAdds full tvOS support: a new NetBird tvOS app target, a tvOS Network Extension, platform abstraction utilities, tvOS-specific UI (views, colors, layout), config/storage adaptations, IPC for app↔extension, and project/entitlement updates for tvOS targets. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant TVApp as tvOS App
participant TVExt as Network Extension
participant SDK as NetBird SDK
participant Mgmt as Management Server
User->>TVApp: Launch App
TVApp->>TVExt: Initialize / Request Config
alt No Config (First-time)
TVApp->>TVExt: LoginTV (device-code)
TVExt->>SDK: Start device-code flow
SDK-->>TVExt: Auth URL + device code
TVExt-->>TVApp: Return URL|userCode
TVApp->>User: Show QR + device code
User->>Mgmt: Approve device via URL
TVApp->>TVExt: Poll CheckLoginComplete
TVExt->>SDK: Query login state
SDK-->>TVExt: Config on success
TVExt->>UserDefaults: saveConfigToUserDefaults()
TVExt-->>TVApp: Login success, config available
else Config Exists
TVApp->>TVExt: Load cached config
TVExt->>UserDefaults: Return cached config
end
TVApp->>TVExt: Start Tunnel
TVExt->>SDK: Initialize client with config
SDK->>Mgmt: Register peer / fetch routes
Mgmt-->>SDK: Routes, registration ack
SDK-->>TVExt: VPN active
TVExt-->>TVApp: Status connected
TVApp->>User: Show connected UI
sequenceDiagram
participant TVApp as tvOS App
participant TVExt as Network Extension
participant UserDefaults as UserDefaults
participant SDK as NetBird SDK
TVApp->>TVExt: Request available routes
TVExt->>SDK: getRoutes()
SDK-->>TVExt: routes list
TVExt-->>TVApp: routes delivered
User->>TVApp: Toggle route
TVApp->>TVExt: SelectRoute / DeselectRoute
TVExt->>SDK: update route selection
SDK-->>TVExt: ack
TVExt-->>TVApp: route update result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–75 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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.
| { | ||
| "idiom" : "tv", | ||
| "scale" : "2x" | ||
| } |
There was a problem hiding this comment.
Consider providing 2x scale icon asset.
Similar to the Middle layer, the 2x scale entry has no filename. Providing dedicated 2x versions for all icon layers would ensure optimal quality on higher-resolution displays.
🤖 Prompt for AI Agents
In NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App
Icon.imagestack/Front.imagestacklayer/Content.imageset around lines 8-11, the 2x
scale entry exists but has no "filename" specified; add a filename key pointing
to the 2x PNG asset (e.g., "filename": "[email protected]") so Xcode can find the
high-resolution icon. Ensure the referenced 2x image file is added to the asset
catalog, matches the correct dimensions for tv 2x, and update the Contents.json
entry accordingly.
| { | ||
| "idiom" : "tv", | ||
| "scale" : "2x" | ||
| } |
There was a problem hiding this comment.
Consider providing 2x scale icon asset.
The 2x scale entry has no filename specified. While the app will function using the 1x asset scaled up, providing a dedicated 2x version would ensure optimal icon quality on higher-resolution tvOS devices.
🤖 Prompt for AI Agents
In NetBird TV/Assets.xcassets/App Icon & Top Shelf Image.brandassets/App
Icon.imagestack/Middle.imagestacklayer/Content.imageset around lines 8-11, the
2x scale entry lacks a "filename" field; add a "filename" property pointing to a
dedicated 2x image file (and add that image to the .imageset) so the asset
catalog uses a proper high-resolution icon for tvOS devices; ensure the provided
image matches the expected 2x dimensions and the filename exactly matches the
file added to the asset catalog.
…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.
|
|
||
| ## 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.
- Use empty configPath on tvOS in loginAsync() since config uses UserDefaults - Add JSON escaping in updateDeviceNameInConfig() for special characters - Standardize TVOS_DEPLOYMENT_TARGET to 17.0 across all configurations - Add missing CODE_SIGN_ENTITLEMENTS to NetBird TV Release config
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
NetbirdNetworkExtension/NetBirdAdapter.swift (3)
296-323: Consider validatinginterfaceNamesimilarly totunnelFileDescriptor.The file descriptor validation (line 299) is good. However,
interfaceNamefalls back to"unknown"on line 308 rather than failing. If the SDK requires a valid interface name to function correctly, consider returning an error for this case as well.- let ifName = self.interfaceName ?? "unknown" + guard let ifName = self.interfaceName else { + adapterLogger.warning("start: Could not determine interface name, using 'unknown'") + // If interface name is required, uncomment the following: + // completionHandler(NSError(domain: "io.netbird.NetbirdNetworkExtension", code: 1005, userInfo: [NSLocalizedDescriptionKey: "Failed to get interface name"])) + // return + } + let ifName = self.interfaceName ?? "unknown"
366-382: Config save result at line 376 is silently ignored.
Preferences.saveConfigToUserDefaults(configJSON)returns aBoolindicating success, but the result is discarded. On tvOS, this is the primary persistence mechanism—a failure should at least be logged.- _ = Preferences.saveConfigToUserDefaults(configJSON) + if !Preferences.saveConfigToUserDefaults(configJSON) { + adapterLogger.warning("loginAsync: Failed to save config to UserDefaults") + }
490-523: Consider usingJSONSerializationfor more robust parsing and serialization.The regex-based approach works for simple cases but can fail on edge cases (e.g., nested quotes in strings, unicode escapes). For
extractManagementURL, usingJSONSerializationwould be more reliable: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 }The
updateDeviceNameInConfigescaping is now correct for common cases. The regex approach is acceptable here since the structure is simple and controlled.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
NetBird TV/NetBird TV.entitlements(1 hunks)NetBird.xcodeproj/project.pbxproj(25 hunks)NetbirdNetworkExtension/NetBirdAdapter.swift(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- NetBird TV/NetBird TV.entitlements
🔇 Additional comments (13)
NetBird.xcodeproj/project.pbxproj (4)
947-1026: LGTM! Previous issues have been resolved.The NetBird TV build configurations now have:
- Consistent bundle identifiers across Debug and Release (
io.netbird.app.tv)- CODE_SIGN_ENTITLEMENTS properly set in both configurations
- Matching deployment targets (tvOS 17.0)
- Consistent development team settings
Great work addressing the previous review feedback!
1027-1100: LGTM! Network extension configuration is correct.The NetBirdTVNetworkExtension build settings are properly configured with:
- Consistent bundle identifier hierarchy (
io.netbird.app.tv.extension)- Matching tvOS deployment target (17.0) with the main app
- Proper entitlements for both Debug and Release
- Consistent development team settings
560-710: LGTM! tvOS target integration is well-structured.The new tvOS targets (NetBird TV and NetBirdTVNetworkExtension) are properly integrated:
- Targets are correctly defined with appropriate build phases
- Dependencies between app and extension are properly configured
- Framework and resource linking is complete
- File references are organized logically
The project structure follows Xcode best practices for adding a new platform target.
670-678: Verify Xcode version numbers are correct for your environment.The Xcode version
26.1and build number2610are set for the new tvOS targets. While these may be valid for your development environment in December 2025, please confirm these match your actual Xcode installation to avoid compatibility issues.#!/bin/bash # Verify the Xcode version installed xcodebuild -versionNetbirdNetworkExtension/NetBirdAdapter.swift (9)
13-15: LGTM!Good use of structured logging with
os.Loggerfor debugging the adapter.
16-29: LGTM!Clean implementation of the URL opener protocol with appropriate nil handling for the URL parameter.
31-43: LGTM!Standard error listener implementation with optional callback pattern.
45-57: LGTM!Unified result callback pattern handles both success and error cases cleanly.
58-84: LGTM!Good design: strong references to
loginURLOpenerandloginErrListenerprevent premature deallocation during async login flows. The state tracking properties aid debugging.
87-131: LGTM!iOS-specific utun file descriptor discovery logic is correct. The added logging helps with debugging tunnel setup issues.
133-215: This segment was flagged in a previous review regarding the fragility of hardcoded kernel structure offsets. The implementation appears correct for current tvOS versions and includes proper memory management withdeferblocks.
217-268: LGTM!The failable initializer properly guards against SDK client creation failures. The tvOS branch correctly uses extension-local
UserDefaults.standardwith clear documentation about sandbox restrictions.
440-482: LGTM!The platform-specific auth flow is well-structured. The tvOS branch correctly uses device auth with deviceName, while iOS uses the standard auth flow. Error handling for
NetBirdSDKNewAuthfailure is appropriate.
|
|
||
| /// Legacy synchronous login - returns URL string directly | ||
| /// Used by iOS which opens Safari | ||
| public func login() -> String { | ||
| self.isExecutingLogin = true | ||
| return self.client.loginForMobile() | ||
| } |
There was a problem hiding this comment.
isExecutingLogin is set but never cleared in the synchronous login() path.
The loginAsync() method correctly clears isExecutingLogin in its completion handlers (lines 386, 404). However, the legacy login() method sets the flag at line 332 but never resets it. This could leave the adapter in a stale "executing login" state.
+ /// Legacy synchronous login - returns URL string directly
+ /// Used by iOS which opens Safari
+ /// Note: Caller is responsible for clearing isExecutingLogin after handling the URL
public func login() -> String {
self.isExecutingLogin = true
return self.client.loginForMobile()
}Alternatively, ensure the caller clears this flag, or remove it from the synchronous path if it's not needed there.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In NetbirdNetworkExtension/NetBirdAdapter.swift around lines 328–334, the
synchronous login() method sets isExecutingLogin = true but never clears it;
update the method so the flag is cleared before returning (e.g., call
client.loginForMobile(), assign the result to a local variable, set
isExecutingLogin = false, then return the result) or remove the flag
manipulation entirely from this synchronous path if the flag isn’t needed here;
ensure the flag state matches the async path behavior.
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
Chores
✏️ Tip: You can customize this high-level summary in your review settings.