Skip to content

Pre-release code review: safety, formatting, view extraction, and concurrency fixes#11

Open
darox wants to merge 1 commit intomainfrom
claude/review-project-improvements-eUgOu
Open

Pre-release code review: safety, formatting, view extraction, and concurrency fixes#11
darox wants to merge 1 commit intomainfrom
claude/review-project-improvements-eUgOu

Conversation

@darox
Copy link
Copy Markdown
Owner

@darox darox commented Apr 19, 2026

User description

Summary

  • Safety: Replace all force-unwraps in StatusBarController with safe Optional/Task patterns — eliminates potential crashes when optional data is nil
  • Formatting: Switch byte/bitrate units from 1024-based (binary) to 1000-based (decimal) to match SpeedTestResult — consistent Mbps/Gbps across the UI
  • View extraction: Break up large computed some View properties into dedicated structs (DiagnosticsSection, NotConfiguredView, MenuErrorView, MenuFooterView, SectionToggleRow) — follows SwiftUI best practices for stable identity and testability
  • Concurrency: Cache DateFormatter in DiagnosticsLog as a stored property instead of creating inline on every call
  • Gateway detection: Expand DeviceDTO.isGateway to match USG/router models and devices with wan feature flag
  • DDNS IDs: Use meaningful defaults ("unknown", "ddns") instead of empty strings in DDNSStatusDTO.id
  • Preferences: Guard launchAtLogin onChange against initial-load trigger loop; remove hardcoded window height so contentSize drives layout naturally
  • Tests: Add unit tests for WiFiStatus formatting, PreferencesManager, UpdateChecker, UniFiError, and DecodeFlexibleArray

Test plan

  • swift build passes with zero warnings (Swift 6 strict concurrency)
  • swift test passes (42/42)
  • App launches and Preferences window shows all sections without scrolling
  • Verify menu bar displays WiFi metrics, VPN, DDNS, and network sections correctly
  • Verify error states (controller unreachable, invalid API key, cert changed) render properly
  • Verify diagnostics "Copy Report" includes all expected sections

PR Type

Enhancement, Bug fix, Tests


Description

  • Security: Keychain cert pinning, cert change detection, HTTP header injection guard, HTTPS-only, response caps

    • Moved cert pins from UserDefaults to Keychain (kSecAttrAccessibleWhenUnlockedThisDeviceOnly)
    • Cert mismatch now rejects connection and surfaces error instead of silently re-pinning
    • API key stripped of control characters; URLs validated for HTTPS, no query/fragment
    • Client count capped at 5,000; devices at 500; sessions at 1,000
  • Monitoring: Added alarms, IDS/IPS, DDNS, port forwards, and nearby APs sections

    • New fetchAlarms, fetchIPSEvents, fetchDDNSStatus, fetchPortForwards, fetchRogueAPs endpoints
    • Section visibility controlled by preferences toggles
  • Error handling: Exponential backoff, auth failure stops polling, detailed error states

    • ErrorState now carries reason strings and HTTP codes
    • certChanged error state with reset button in UI
    • Debounced manual refresh, overlapping refresh guard
  • Diagnostics: Event log, copy report, section preferences, update checker, unit tests


Diagram Walkthrough

flowchart LR
  A["PinnedCertDelegate"] -->|"Keychain storage & cert change detection"| B["UniFiClient"]
  B -->|"New monitoring endpoints"| C["StatusBarController"]
  C -->|"Error states & backoff"| D["WiFiStatus"]
  C -->|"Section-aware fetch"| E["MenuContentView"]
  E -->|"New sections"| F["Monitoring Views"]
  C -->|"Diagnostics log"| G["PreferencesView"]
Loading

File Walkthrough

Relevant files
Enhancement
16 files
UniFiClient.swift
Security hardening, monitoring endpoints, flexible decode
+314/-38
StatusBarController.swift
Error handling, exponential backoff, monitoring fetch loop
+270/-31
PreferencesView.swift
Redesigned UI with sections, diagnostics, and cert reset 
+303/-80
MenuContentView.swift
Section visibility, monitoring sections, extracted views 
+244/-62
WiFiStatus.swift
Error state refactoring, monitoring fields, decimal units
+167/-20
MonitoringDTO.swift
New monitoring DTOs with safe display formatting                 
+295/-0 
ConnectionSection.swift
Parameterized view signatures replacing WiFiStatus dependencies
+55/-34 
DiagnosticsLog.swift
Event logging with categories and text export                       
+173/-0 
SetupView.swift
HTTPS-only validation and improved error messages               
+61/-50 
UpdateChecker.swift
Periodic update checking utility                                                 
+110/-0 
CollapsibleSection.swift
Collapsible section UI component                                                 
+130/-0 
SecuritySection.swift
IDS/IPS events display section                                                     
+66/-0   
NearbyAPsSection.swift
Nearby/rogue APs display section                                                 
+40/-0   
DDNSSection.swift
Dynamic DNS status display section                                             
+34/-0   
PortForwardsSection.swift
Port forwarding rules display section                                       
+37/-0   
AlertsSection.swift
Controller alarms display section                                               
+35/-0   
Tests
4 files
WiFiStatusTests.swift
Unit tests for WiFiStatus formatting and state                     
+176/-0 
PreferencesManagerTests.swift
Unit tests for PreferencesManager                                               
+99/-0   
DecodeFlexibleArrayTests.swift
Unit tests for flexible JSON array decoder                             
+98/-0   
UpdateCheckerTests.swift
Unit tests for UpdateChecker                                                         
+64/-0   
Additional files
20 files
pr-agent.yml +1/-0     
.pr_agent.toml +4/-0     
Package.swift +5/-0     
compile_and_run.sh +33/-20 
package_app.sh +33/-20 
probe_endpoints.sh +56/-0   
AppDelegate.swift +4/-0     
UniFiBarApp.swift +1/-0     
DeviceDTO.swift +50/-6   
PreferencesManager.swift +113/-2 
DeviceDetector.swift +31/-19 
Formatters.swift +18/-0   
MetricRow.swift +2/-0     
ProgressBarView.swift +1/-0     
InternetSection.swift +69/-14 
SessionTimeSection.swift +10/-1   
VPNSection.swift +2/-2     
WiFiExperienceSection.swift +14/-10 
StatusBarLabel.swift +5/-0     
UniFiErrorTests.swift +34/-0   

@darox darox force-pushed the claude/review-project-improvements-eUgOu branch from 9e7ae1b to b815fa2 Compare April 19, 2026 08:18
@github-actions
Copy link
Copy Markdown

Failed to generate code suggestions for PR

@darox darox force-pushed the claude/review-project-improvements-eUgOu branch from 0ff122f to f660ea6 Compare April 19, 2026 12:01
…d PR Agent config

- Replace force-unwraps in StatusBarController with safe Optional/Task patterns
- Expand DeviceDTO.isGateway to match router/USG models and wan feature flag
- Switch WiFiStatus byte formatting to 1000-based units (matching SpeedTestResult)
- Fix DDNSStatusDTO.id to use meaningful defaults instead of empty strings
- Cache DateFormatter in DiagnosticsLog as stored property
- Extract dedicated view structs: DiagnosticsSection, NotConfiguredView,
  MenuErrorView, MenuFooterView, SectionToggleRow
- Guard PreferencesView launchAtLogin onChange against initial-load trigger
- Add Formatters utility for shared RelativeDateTimeFormatter
- Add unit tests for WiFiStatus, PreferencesManager, UpdateChecker,
  UniFiError, and DecodeFlexibleArray
- Remove hardcoded Preferences window height (let contentSize drive it)
- Remove Alerts and Security (IPS) sections — legacy endpoints with no real data
- Fix wired Ethernet: iterate all en* IPs against UniFi client list, fall back
  to minimal wired view when Mac IP isn't in UniFi (e.g. non-UniFi router)
- Add site ID cache reset on discovery errors so stale IDs get cleared
- Add NSLocalNetworkUsageDescription to Info.plist (required for App Store)
- Use activate(ignoringOtherApps: true) for reliable window foregrounding
- Configure PR Agent to run on synchronize events via GITHUB_ACTION_CONFIG
- Increase PR Agent timeout to 30 minutes
- Add .env to .gitignore

AI assisted to create this change
@darox darox force-pushed the claude/review-project-improvements-eUgOu branch from f660ea6 to 681da97 Compare April 19, 2026 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant