Skip to content

Conversation

@AustinChangLinksys
Copy link
Collaborator

@AustinChangLinksys AustinChangLinksys commented Feb 5, 2026

Summary

This PR migrates PrivacyGUI 1.2.8 features to use the UI Kit library and addresses code review feedback.

Changes

PWA Installation Feature

  • Implement PWA install prompt banner with platform-specific flows (iOS, Mac Safari, Android/Chrome)
  • Add showAppBottomSheet from UI Kit v2.12.3 for instruction sheets
  • Use AppSurface with SurfaceVariant.highlight for banner styling
  • Use AppIconButton.icon for transparent dismiss button

Speed Test Server Selection

  • Add HealthCheckServer model with safe JSON parsing
  • Implement server selection dialog using AppRadioList from UI Kit
  • Add HealthCheckManager2 JNAP service support check
  • Integrate server list fetching in polling service

Session State Management

  • Convert SessionProvider from Notifier<void> to Notifier<SessionState>
  • Consolidate modelNumber into SessionState (removed global_model_number_provider.dart)
  • Add clear() method called on logout to prevent stale device identity

Code Quality Improvements (Qodo Review Fixes)

  • Add empty server ID validation to avoid sending empty strings to router
  • Use safe type conversions (?.toString(), (as num?)?.toInt()) for external API data
  • Mask sensitive data (serial number) in session logs for security
  • Translate migration documentation to English

CI/CD Enhancement

  • Add workflow_dispatch manual trigger for main_snapshot_baseline job

Test Coverage

  • Add PWA banner golden tests with multiple modes (native, iOS, Mac)
  • Add standalone instruction sheet tests
  • Update health check provider mocks for server selection
  • Update session notifier mocks for SessionState return type

Testing

  • All existing tests pass (3107 functional tests)
  • flutter analyze reports no issues
  • Golden screenshot tests updated

Files Changed

  • PWA: lib/core/pwa/, lib/page/components/pwa/
  • Health Check: lib/page/health_check/ (models, providers, views)
  • Session: lib/core/data/providers/session_provider.dart
  • Auth: lib/providers/auth/auth_provider.dart
  • CI: .github/workflows/ci.yml
  • Docs: doc/migration/ (English translation)

AustinChangLinksys and others added 7 commits February 4, 2026 15:47
Phase 1 Migration (Clean additions):

1. PWA Install Banner (DU models only)
   - Add PWA install service and platform-specific logic
   - Add device features system for model-based feature detection
   - Add install prompt banner with iOS/Mac Safari instruction sheets
   - Add PWA localization strings
   - Update web resources (manifest.json, service_worker.js, index.html)

2. Brand Asset Provider
   - Add GlobalModelNumberProvider for persistent model state
   - Add BrandAssetProvider with asset path resolution
   - Add brand assets for TB variant (logo, support image)
   - Add BrandUtils class with manifest-based asset resolution

3. SI Unit Formatting
   - Update formatBits/formatBitsWithUnit to use base 1000 (SI units)
   - Update corresponding unit tests

4. SDK Version Update
   - Bump Dart SDK requirement to >=3.3.0 for extension types support

Also includes migration plan documentation in doc/migration/

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add HealthCheckServer model for server data
- Add getCloseHealthCheckServers JNAP action
- Update HealthCheckState with servers and selectedServer fields
- Update HealthCheckProvider with loadServers() and setSelectedServer() methods
- Update SpeedTestService with getHealthCheckServers() and targetServerId support
- Add server selection dialog component (SpeedTestServerSelectionList)
- Add server selection button to SpeedTestView
- Add selectServer localization string
- Update mocks and tests for new service method signatures

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add healthCheckManager2 to JNAPService enum and switch cases
- Add isSupportHealthCheckManager2() method to ServiceHelper
- Update HealthCheckProvider.loadData() to check support before loading servers
- Update tests with mockDependencyRegister() and regenerated mocks

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Change mock class from `extends Mock implements Provider` to
  `extends HealthCheckProvider with Mock` to inherit Riverpod's
  internal methods (_setElement) while preserving Mock functionality
- Add isSupportHealthCheckManager2 mock to test_helper.dart
- Update speed_test_view_test.dart to use anyNamed('serverId')
- Update migration tasks document with Phase 1 & 2 completion status

This fixes the localization tests that were failing with:
"Class 'MockHealthCheckProvider' has no instance method '_setElement'"

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Phase 3 Changes:
- Add getCloseHealthCheckServers to polling_service.dart for server caching
- Add 5 new error state golden tests (STV-ERROR-02 ~ STV-ERROR-06)
  - license, execution, aborted, dbError, timeout errors
- Group existing error test (STV-ERROR-01) with new tests
- Update migration tasks document with Phase 3 completion status

All 3107 functional tests pass.
All 80 SpeedTest localization tests pass (55 original + 25 new error tests).

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Replace deprecated RadioListTile with AppRadioList component
- Maintain ValueNotifier pattern for temporary dialog state
- No functional changes, only UI component update

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Update UI Kit dependency to v2.12.3 for showAppBottomSheet support
- Refactor iOS and Mac install instruction sheets to use static show() method
- Update InstallPromptBanner to use AppSurface with SurfaceVariant.highlight
- Change dismiss button to AppIconButton.icon for transparent background
- Consolidate modelNumber into SessionState (remove global_model_number_provider)
- Add PwaInstallService dependency on SessionProvider.modelNumber
- Rewrite install_prompt_banner_test.dart following screenshot test guidelines
- Add standalone tests for iOS and Mac instruction sheets
- Add manual trigger (workflow_dispatch) to CI for main_snapshot_baseline job
- Update test mocks for SessionNotifier with SessionState return type

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@qodo-code-review
Copy link

Review Summary by Qodo

PrivacyGUI 1.2.8 Migration: PWA Installation, Server Selection, and Device Features

✨ Enhancement 🧪 Tests

Grey Divider

Walkthroughs

Description
• **PWA Installation Feature**: Implemented comprehensive PWA install prompt system with
  platform-specific flows for iOS, Mac Safari, and native Android/Chrome, including dismissal cooldown
  and reactive state management
• **Speed Test Server Selection**: Added server selection UI and backend support with new
  HealthCheckManager2 service, including server list fetching and selection persistence
• **Device Feature Detection System**: Created DeviceFeature enum and isFeatureSupported()
  function with model number parsing for region and provider-based feature rules
• **Reactive Session State Management**: Converted SessionProvider from void-based to reactive
  SessionState management for better device info tracking and model number access
• **Network Speed Formatting**: Updated speed formatting from binary (base 1024) to SI units (base
  1000) with corresponding test updates
• **Brand Utilities**: Added BrandUtils class with manifest-based asset discovery supporting webp
  and png formats with priority ordering
• **Comprehensive Test Coverage**: Added 30+ new test cases covering PWA banner, speed test errors,
  device features, and server selection with golden screenshot tests
• **Web PWA Logic**: Implemented web-specific PWA logic using JS interop for browser APIs with early
  event capture in index.html
• **PWA Manifest Updates**: Updated manifest with new branding, icon assets, and accessibility
  improvements
• **Migration Documentation**: Added detailed migration task tracking document for dev-1.2.8 to
  dev-2.0.0 transition
• **CI/CD Enhancement**: Enabled manual workflow dispatch for CI pipeline with configurable job
  selection
Diagram
flowchart LR
  A["Session Provider<br/>Reactive State"] -->|"Device Info"| B["Device Features<br/>Detection System"]
  B -->|"Feature Support"| C["PWA Install Service<br/>Platform Detection"]
  C -->|"Install Prompt"| D["Install Prompt Banner<br/>iOS/Mac/Native"]
  E["Health Check Service<br/>Server Support"] -->|"Server List"| F["Server Selection UI<br/>Radio List"]
  F -->|"Selected Server"| G["Health Check Provider<br/>Server Tracking"]
  H["Brand Utils<br/>Asset Discovery"] -->|"Brand Assets"| I["Brand Asset Provider<br/>Dynamic Resolution"]
  J["Polling Service<br/>HealthCheckManager2"] -->|"Server Updates"| G
  D -->|"Integrated"| K["Dashboard Shell<br/>PWA Banner"]
Loading

Grey Divider

File Changes

1. test/page/health_check/views/localizations/speed_test_view_test.dart 🧪 Tests +174/-31

Expand speed test error state test coverage

• Added 5 new error state test cases (STV-ERROR-02 through STV-ERROR-06) covering license,
 execution, aborted, database, and timeout errors
• Grouped error tests into a dedicated test group for better organization
• Updated mock setup to include serverId parameter in runHealthCheck calls
• Fixed golden filename for STV-ERROR-01 from generic 'error_state' to specific
 'configuration_error'
• Refactored test state setup for STV-ACTION-01 to improve code clarity

test/page/health_check/views/localizations/speed_test_view_test.dart


2. test/page/components/pwa/install_prompt_banner_test.dart 🧪 Tests +280/-0

Add PWA install banner widget tests

• Created comprehensive test suite for PWA install prompt banner with 5 test groups covering native,
 iOS, and Mac modes
• Implemented FakePwaInstallService for testing PWA mode states without actual browser APIs
• Added tests for iOS and Mac Safari instruction sheets with interaction verification
• Includes golden screenshot tests for multiple device modes and locales

test/page/components/pwa/install_prompt_banner_test.dart


3. test/mocks/health_check_provider_mocks.dart 🧪 Tests +50/-102

Refactor health check provider mock implementation

• Refactored from auto-generated Mockito code to custom mock extending real HealthCheckProvider
• Updated runHealthCheck method signature to include optional serverId named parameter
• Added mock implementations for loadServers() and setSelectedServer() methods
• Simplified mock structure by extending real provider instead of using fake objects

test/mocks/health_check_provider_mocks.dart


View more (41)
4. lib/core/data/providers/session_provider.dart ✨ Enhancement +52/-12

Convert session provider to reactive state management

• Created new SessionState class extending Equatable to manage device info reactively
• Changed SessionNotifier from Notifier<void> to Notifier<SessionState> for reactive state
 management
• Added modelNumber getter to SessionState for convenient access to device model
• Updated all device info methods to update session state and log model number changes
• Added clear() method for session cleanup during logout

lib/core/data/providers/session_provider.dart


5. lib/core/pwa/pwa_install_service.dart ✨ Enhancement +176/-0

Implement PWA install service with platform detection

• Implemented PwaInstallService notifier managing PWA installation prompts across platforms
• Added platform detection for iOS, Mac Safari, and native Android/Chrome prompts
• Integrated feature flag checking via device_features.dart with model number support
• Implemented dismissal cooldown (30 days) using SharedPreferences
• Added reactive listening to session state changes for device switching scenarios

lib/core/pwa/pwa_install_service.dart


6. test/utils_test.dart 🧪 Tests +12/-12

Update network speed formatting tests to SI units

• Updated network speed formatting tests to use SI units (base 1000) instead of binary (base 1024)
• Corrected expected values in formatBits tests for kilobytes, megabytes, gigabytes, and petabytes
• Updated test comments to reflect SI unit calculations
• Fixed test data to use 1000-based multipliers instead of 1024-based

test/utils_test.dart


7. lib/page/health_check/providers/health_check_state.dart ✨ Enhancement +29/-0

Add server selection fields to health check state

• Added servers list field to store available speed test servers
• Added selectedServer field to track currently selected speed test server
• Updated copyWith() method to support server-related fields
• Updated toMap() and fromJson() methods for serialization/deserialization
• Updated props list for proper equality comparison

lib/page/health_check/providers/health_check_state.dart


8. test/core/utils/device_features_test.dart 🧪 Tests +128/-0

Add device features system tests

• Created comprehensive test suite for device feature detection system
• Tests parsing logic for model numbers, regions, and provider suffixes
• Verifies PWA feature support rules for 'DU' provider models
• Tests case insensitivity and custom rule overrides
• Includes pattern matching and exact model matching test cases

test/core/utils/device_features_test.dart


9. lib/utils.dart ✨ Enhancement +78/-2

Update speed formatting to SI units and add brand utilities

• Changed network speed formatting from binary (base 1024) to SI units (base 1000)
• Updated formatBits() and formatBitsWithUnit() to use 1000 divisor
• Added documentation comments explaining SI unit formatting
• Added BrandUtils class with asset path resolution and model suffix mapping
• Implemented manifest-based asset discovery with webp and png format priority

lib/utils.dart


10. test/mocks/session_notifier_mocks.dart 🧪 Tests +70/-12

Update session notifier mock for reactive state

• Updated MockSessionNotifier to extend Notifier<SessionState> instead of Notifier<void>
• Added _FakeSessionState_2 fake class for mock return values
• Updated build() method to return SessionState instead of void
• Added mock implementations for forceFetchDeviceInfo() and clear() methods
• Updated listener method signatures to use SessionState parameters

test/mocks/session_notifier_mocks.dart


11. test/mocks/jnap_service_supported_mocks.dart 🧪 Tests +34/-21

Update service helper mocks with new methods

• Updated Mockito version comment from 5.4.4 to 5.4.6
• Added mock implementations for isSupportVPN() and isSupportSetup() methods
• Added mock implementation for isSupportHealthCheckManager2() method
• Added ignore comments for internal member usage

test/mocks/jnap_service_supported_mocks.dart


12. lib/core/utils/device_features.dart ✨ Enhancement +126/-0

Implement device feature detection system

• Created device feature detection system with DeviceFeature enum
• Implemented isFeatureSupported() function with model number parsing
• Added parsing logic for region and provider suffixes from model numbers
• Implemented rule matching system supporting pattern, region, provider, and model criteria
• Includes comprehensive documentation and support for test rule overrides

lib/core/utils/device_features.dart


13. lib/page/health_check/services/health_check_service.dart ✨ Enhancement +35/-4

Add speed test server selection support

• Added optional targetServerId parameter to runHealthCheck() method
• Updated _initiateHealthCheck() to pass server ID to router API
• Implemented getHealthCheckServers() method to fetch available speed test servers
• Added proper error handling and logging for server fetch operations

lib/page/health_check/services/health_check_service.dart


14. test/common/test_helper.dart 🧪 Tests +25/-1

Add PWA service mock and update test helper

• Added _FakePwaInstallService implementation for testing PWA features
• Registered PWA install service provider override to hide banner in tests
• Updated mockServiceHelper to mock isSupportHealthCheckManager2() method
• Updated mockSessionNotifier.build() to return SessionState instead of void

test/common/test_helper.dart


15. lib/page/health_check/providers/health_check_provider.dart ✨ Enhancement +37/-3

Add server selection to health check provider

• Added loadServers() method to fetch available speed test servers
• Added setSelectedServer() method to update selected server in state
• Updated runHealthCheck() to accept optional serverId parameter
• Implemented logic to use provided server ID or fall back to selected server
• Added conditional server loading based on HealthCheckManager2 support

lib/page/health_check/providers/health_check_provider.dart


16. lib/page/health_check/views/speed_test_view.dart ✨ Enhancement +51/-0

Add server selection UI to speed test view

• Added server selection UI with DNS icon button in page actions
• Implemented _showServerSelectionDialog() method for server selection
• Integrated SpeedTestServerSelectionList component for radio list selection
• Updated state to track available servers and selected server
• Added conditional rendering of server selection button based on server availability

lib/page/health_check/views/speed_test_view.dart


17. test/mocks/health_check_service_mocks.dart 🧪 Tests +22/-1

Update health check service mocks

• Updated runHealthCheck() mock signature to include optional targetServerId parameter
• Added mock implementation for getHealthCheckServers() method
• Added import for HealthCheckServer model
• Added ignore comment for internal member usage

test/mocks/health_check_service_mocks.dart


18. lib/core/pwa/pwa_logic_web.dart ✨ Enhancement +86/-0

Implement web-specific PWA logic with JS interop

• Implemented web-specific PWA logic using JS interop for browser APIs
• Added BeforeInstallPromptEvent and UserChoice extension types for JS interop
• Implemented initListeners() to capture beforeinstallprompt and appinstalled events
• Implemented platform detection for iOS, Mac Safari, and standalone mode
• Added prompt() method to show native install prompt

lib/core/pwa/pwa_logic_web.dart


19. test/page/health_check/providers/health_check_provider_test.dart 🧪 Tests +17/-1

Update health check provider tests for server support

• Added dependency registration via mockDependencyRegister() in setUp
• Updated runHealthCheck() mock to include targetServerId named parameter
• Added mock setup for getHealthCheckServers() returning empty list
• Added async wait for loadData() completion before running health check
• Added mock setup for server-related methods in stream controller tests

test/page/health_check/providers/health_check_provider_test.dart


20. lib/page/components/pwa/install_prompt_banner.dart ✨ Enhancement +82/-0

Implement PWA install prompt banner widget

• Created InstallPromptBanner widget displaying PWA installation prompt
• Implemented platform-specific installation flows for iOS, Mac, and native
• Added dismiss button with cooldown persistence
• Integrated with pwaInstallServiceProvider for reactive mode updates
• Uses UI Kit components for consistent styling

lib/page/components/pwa/install_prompt_banner.dart


21. lib/page/components/pwa/ios_install_instruction_sheet.dart ✨ Enhancement +83/-0

Implement iOS PWA installation instruction sheet

• Created iOS-specific installation instruction sheet component
• Implemented step-by-step visual guide with numbered steps and icons
• Added show() static method for convenient bottom sheet display
• Uses UI Kit components for consistent styling and spacing

lib/page/components/pwa/ios_install_instruction_sheet.dart


22. lib/page/components/pwa/mac_safari_install_instruction_sheet.dart ✨ Enhancement +83/-0

Implement Mac Safari PWA installation instruction sheet

• Created Mac Safari-specific installation instruction sheet component
• Implemented step-by-step visual guide with numbered steps and icons
• Added show() static method for convenient bottom sheet display
• Uses UI Kit components for consistent styling and spacing

lib/page/components/pwa/mac_safari_install_instruction_sheet.dart


23. lib/core/jnap/actions/better_action.dart ✨ Enhancement +4/-0

Register health check manager 2 service and action

• Added case for JNAPService.healthCheckManager2 in service switch statement
• Added mapping for JNAPAction.getCloseHealthCheckServers in action map

lib/core/jnap/actions/better_action.dart


24. lib/page/health_check/views/components/speed_test_server_selection_dialog.dart ✨ Enhancement +39/-0

Implement speed test server selection dialog

• Created SpeedTestServerSelectionList widget for server selection UI
• Implemented radio list using AppRadioList from UI Kit
• Added ValueListenableBuilder for reactive selection updates
• Displays server name and location in list items

lib/page/health_check/views/components/speed_test_server_selection_dialog.dart


25. lib/core/jnap/actions/jnap_service.dart ✨ Enhancement +2/-0

Add health check manager 2 service enum

• Added healthCheckManager2 enum value for new health check service

lib/core/jnap/actions/jnap_service.dart


26. lib/page/dashboard/views/dashboard_shell.dart ✨ Enhancement +7/-1

Integrate PWA banner into dashboard shell

• Integrated InstallPromptBanner into dashboard layout
• Wrapped child widget in Column with banner positioned at bottom
• Banner displays above bottom navigation for consistent placement

lib/page/dashboard/views/dashboard_shell.dart


27. lib/core/jnap/actions/jnap_action_value.dart ✨ Enhancement +2/-0

Add get close health check servers action

• Added getCloseHealthCheckServers action value for fetching available servers

lib/core/jnap/actions/jnap_action_value.dart


28. lib/core/pwa/pwa_logic_stub.dart ✨ Enhancement +22/-0

Implement PWA logic stub for non-web platforms

• Created stub implementation of PWA logic for non-web platforms
• Provides no-op implementations of all PWA methods
• Includes BeforeInstallPromptEvent stub class

lib/core/pwa/pwa_logic_stub.dart


29. lib/core/jnap/actions/jnap_service_supported.dart ✨ Enhancement +3/-0

Add health check manager 2 support check

• Added isSupportHealthCheckManager2() method to check for new health check service support

lib/core/jnap/actions/jnap_service_supported.dart


30. lib/core/data/services/polling_service.dart ✨ Enhancement +4/-0

Add server polling to polling service

• Added conditional polling for getCloseHealthCheckServers when HealthCheckManager2 is supported

lib/core/data/services/polling_service.dart


31. lib/core/jnap/actions/jnap_action.dart ✨ Enhancement +1/-0

Add get close health check servers action enum

• Added getCloseHealthCheckServers action to enum

lib/core/jnap/actions/jnap_action.dart


32. lib/providers/brand_asset_provider.dart ✨ Enhancement +8/-0

Implement brand asset provider

• Created brandAssetProvider FutureProvider for dynamic brand asset path resolution
• Supports model number and asset type parameters
• Integrates with BrandUtils for asset discovery

lib/providers/brand_asset_provider.dart


33. lib/core/pwa/pwa_logic.dart ✨ Enhancement +1/-0

Create PWA logic conditional export

• Created conditional export file selecting web or stub implementation based on platform

lib/core/pwa/pwa_logic.dart


34. web/index.html ✨ Enhancement +10/-1

Add PWA prompt early capture to web index

• Added early capture script for beforeinstallprompt event to prevent missing it during Flutter
 load
• Stores deferred prompt in window.deferredBeforeInstallPromptEvent for later use
• Includes console logging for PWA event tracking

web/index.html


35. doc/migration/dev-1.2.8-migration-tasks.md 📝 Documentation +609/-0

Migration task tracking and progress documentation

• Created comprehensive migration task list document tracking progress from dev-1.2.8 to dev-2.0.0
• Documents 4 phases of migration: Clean Additions, Moderate Integration, Architecture Adaptation,
 and Final Verification
• Includes detailed task checklists with git commands, decision log, and issue tracking for PWA
 features, server selection, error handling, and polling service integration
• Captures architectural differences and resolutions between dev-1.2.8 and dev-2.0.0 implementations

doc/migration/dev-1.2.8-migration-tasks.md


36. .github/workflows/ci.yml ⚙️ Configuration changes +13/-2

Enable manual workflow dispatch for CI pipeline

• Added workflow_dispatch trigger to allow manual job execution with input selection
• Added job input parameter with main_snapshot_baseline as default and available option
• Updated main_snapshot_baseline job condition to support both push to main branch and manual
 workflow dispatch

.github/workflows/ci.yml


37. web/manifest.json ⚙️ Configuration changes +11/-5

Update PWA manifest with branding and icon assets

• Changed background_color from #0175C2 to #FDFBFF (light background)
• Updated description to "Setup your Linksys router easily from your mobile browser."
• Changed orientation from portrait-primary to portrait
• Updated icon purpose from maskable to any maskable for 192x192 icon
• Added new 512x512 logo icon with any maskable purpose

web/manifest.json


38. assets/brand/meta.json ⚙️ Configuration changes +26/-0

Add brand asset metadata configuration

• Created new brand metadata configuration file with brand logo and support image settings
• Defines default and dark theme variants for brand_logo and brand_img_sup assets
• Includes image dimensions (144x144) and alt text for accessibility

assets/brand/meta.json


39. doc/migration/dev-1.2.8-to-dev-2.0.0-migration-plan.md Additional files +560/-0

...

doc/migration/dev-1.2.8-to-dev-2.0.0-migration-plan.md


40. lib/l10n/app_en.arb Additional files +13/-1

...

lib/l10n/app_en.arb


41. lib/page/health_check/models/health_check_server.dart Additional files +67/-0

...

lib/page/health_check/models/health_check_server.dart


42. pubspec.yaml Additional files +4/-4

...

pubspec.yaml


43. web/flutter_bootstrap.js Additional files +2/-1

...

web/flutter_bootstrap.js


44. web/service_worker.js Additional files +16/-0

...

web/service_worker.js


Grey Divider

Qodo Logo

@qodo-code-review
Copy link

qodo-code-review bot commented Feb 5, 2026

Code Review by Qodo

🐞 Bugs (6) 📘 Rule violations (5) 📎 Requirement gaps (0)

Grey Divider


Action required

1. saveSelectedNetwork logs serialNumber 📘 Rule violation ⛨ Security
Suggestion Impact:Updated saveSelectedNetwork logging to avoid exposing raw identifiers by masking the serial number, logging only whether networkId is non-empty, and removing the second log that printed both values.

code diff:

-    logger.i('[Session]: saveSelectedNetwork - $networkId, $serialNumber');
+    // Log with masked sensitive data for security
+    final maskedSN = serialNumber.length > 4
+        ? '****${serialNumber.substring(serialNumber.length - 4)}'
+        : '****';
+    logger.d('[Session]: saveSelectedNetwork - networkId: ${networkId.isNotEmpty}, SN: $maskedSN');
     final pref = await SharedPreferences.getInstance();
-    logger.d('[Session]: save selected network - $serialNumber, $networkId');
     await pref.setString(pCurrentSN, serialNumber);
     await pref.setString(pSelectedNetworkId, networkId);

Description
saveSelectedNetwork writes both serialNumber and networkId into application logs.
• Serial numbers and network identifiers are device/session identifiers and can be considered
  sensitive; logging them increases exposure risk (especially on web where logs are cached).
• This violates the requirement that logs must not contain sensitive data at any log level.
Code

↗ lib/core/data/providers/session_provider.dart

  /// - After configuration changes
Evidence
PR Compliance ID 5 requires that no sensitive data is present in logs. The updated session provider
logs serialNumber and networkId values directly via logger.i and logger.d.

Rule 5: Generic: Secure Logging Practices
lib/core/data/providers/session_provider.dart[107-114]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`saveSelectedNetwork` currently logs `serialNumber` and `networkId`, which are sensitive device/session identifiers.

## Issue Context
Compliance requires that logs contain no sensitive data at any log level.

## Fix Focus Areas
- lib/core/data/providers/session_provider.dart[107-114]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. debugPrint prints stacktrace 📘 Rule violation ⛨ Security
Description
_checkPlatformAndPersistence prints the exception and full stack trace via debugPrint.
• On Flutter Web this is visible in the browser console and may expose internal implementation
  details to end users.
• Secure error handling requires that detailed stack traces are restricted to secure internal logs,
  not user-accessible output.
Code

lib/core/pwa/pwa_install_service.dart[R72-74]

+    } catch (e, stack) {
+      debugPrint('PWA: Error validating platform/persistence: $e\n$stack');
+    }
Evidence
PR Compliance ID 4 forbids exposing internal details like stack traces to end users. The code prints
$stack as part of the error output.

Rule 4: Generic: Secure Error Handling
lib/core/pwa/pwa_install_service.dart[72-74]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PWA install service prints exception + stack trace via `debugPrint`, which can leak internal details (especially on Web).

## Issue Context
Compliance requires generic user-facing errors and restricts detailed stack traces to secure internal logs.

## Fix Focus Areas
- lib/core/pwa/pwa_install_service.dart[72-74]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Native PWA prompt never shown 🐞 Bug ✓ Correctness
Description
web/index.html calls preventDefault() on the beforeinstallprompt event, which suppresses the
  browser’s native install UI and requires the app to present its own install prompt.
• PwaInstallService captures the deferred event but never sets state = PwaMode.native, and
  _checkPlatformAndPersistence() only sets ios/mac modes—so Chrome/Android users can end up with
  no install UI at all.
• This effectively breaks the PWA install experience for eligible devices (and may block
  installation entirely if no other install entry point is available).
Code

lib/core/pwa/pwa_install_service.dart[R65-132]

+      // 4. Platform checks for non-native-prompt platforms (iOS/Mac)
+      if (isIOS) {
+        state = PwaMode.ios;
+      } else if (isMacSafari) {
+        state = PwaMode.mac;
+      }
+      // Android/Desktop Chrome will be handled by 'beforeinstallprompt' event
+    } catch (e, stack) {
+      debugPrint('PWA: Error validating platform/persistence: $e\n$stack');
+    }
+  }
+
+  // Method to be called by UI to ignore the prompt for X days
+  Future<void> dismiss() async {
+    state = PwaMode.none; // Hide immediately
+    _deferredPrompt = null; // Discard prompt for this session
+    try {
+      final prefs = await SharedPreferences.getInstance();
+      await prefs.setInt(_kDismissedKey, DateTime.now().millisecondsSinceEpoch);
+    } catch (e) {
+      debugPrint('PWA: Error saving dismissal state: $e');
+    }
+  }
+
+  // Check if we are within the "cooldown" period (e.g. 30 days)
+  Future<bool> isDismissedRecently() async {
+    try {
+      final prefs = await SharedPreferences.getInstance();
+      final timestamp = prefs.getInt(_kDismissedKey);
+      if (timestamp == null) return false;
+
+      final dismissedAt = DateTime.fromMillisecondsSinceEpoch(timestamp);
+      final difference = DateTime.now().difference(dismissedAt);
+
+      // 30 Days cooldown
+      return difference.inDays < 30;
+    } catch (e) {
+      debugPrint('PWA: Error checking dismissal state: $e');
+      return false; // Fail safe: Assume not dismissed so user can see it if eligible
+    }
+  }
+
+  void _initListeners() {
+    if (!kIsWeb) return;
+
+    // Listen to session state changes to handle device switching or delayed load
+    ref.listen(sessionProvider, (prev, next) {
+      if (prev?.modelNumber == next.modelNumber) return;
+      _checkPlatformAndPersistence(next.modelNumber);
+    });
+
+    try {
+      // Use the shared Logic Loader
+      _pwaLogic.initListeners(
+        onBeforeInstallPrompt: (event) {
+          _deferredPrompt = event;
+          // Check persistence
+          isDismissedRecently().then((recentlyDismissed) {
+            // ...
+            // Re-check using current model number
+            final modelNumber = ref.read(sessionProvider).modelNumber;
+            if (!isFeatureSupported(DeviceFeature.pwa, modelNumber)) {
+              // ...
+              return;
+            }
+            // ...
+          });
+        },
Evidence
The web layer explicitly suppresses the default install prompt and stores it for later, but the
notifier never transitions into PwaMode.native when the deferred prompt arrives; and the
platform/persistence check only assigns ios or mac, leaving non-iOS/mac platforms with none
forever.

web/index.html[26-34]
lib/core/pwa/pwa_install_service.dart[41-75]
lib/core/pwa/pwa_install_service.dart[116-137]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The browser native install prompt is suppressed in `web/index.html` via `preventDefault()`. The app is expected to show its own UI based on `PwaInstallService` state. However, the service never sets `state = PwaMode.native` when the deferred prompt event arrives, and `_checkPlatformAndPersistence()` only sets iOS/Mac modes.

### Issue Context
This can leave Chrome/Android users without any install prompt/UI.

### Fix Focus Areas
- lib/core/pwa/pwa_install_service.dart[41-75]
- lib/core/pwa/pwa_install_service.dart[116-137]
- web/index.html[26-34]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (1)
4. Session state not cleared 🐞 Bug ⛯ Reliability
Suggestion Impact:The logout flow in auth_provider.dart was updated to call ref.read(sessionProvider.notifier).clear(), clearing in-memory session state during logout. The additional session_provider.dart logging/masking changes are unrelated to the suggestion.

code diff:

# File: lib/providers/auth/auth_provider.dart
@@ -340,6 +340,7 @@
       // Reset provider states
       ref.read(deviceManagerProvider.notifier).init();
       ref.read(pollingProvider.notifier).init();
+      ref.read(sessionProvider.notifier).clear();
       return AuthState.empty();
     });

Description
sessionProvider is now stateful and retains deviceInfo/modelNumber, which can drive feature
  gating (e.g., the PWA banner watches sessionProvider.modelNumber).
• The logout flow resets other providers but does not call sessionProvider.clear(), leaving stale
  device identity in-memory after logout.
• This can cause incorrect UI/feature gating after logout/re-login (especially when switching
  routers/models).
Code

lib/core/data/providers/session_provider.dart[R171-177]

+  /// Clears the session state.
+  ///
+  /// This should be called during logout to reset the session.
+  void clear() {
+    state = const SessionState();
+    logger.d('[Session]: Session state cleared');
+  }
Evidence
A clear() API exists on SessionNotifier specifically to reset session state, but the logout
implementation does not invoke it, so the new reactive session state can persist across logout.

lib/core/data/providers/session_provider.dart[171-177]
lib/providers/auth/auth_provider.dart[320-347]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`sessionProvider` now maintains reactive session state (deviceInfo/modelNumber) but logout does not clear it. This can leave stale device identity across logout and affect feature gating/UI.

### Issue Context
`SessionNotifier.clear()` explicitly documents it should be called during logout, but `AuthProvider.logout()` does not.

### Fix Focus Areas
- lib/providers/auth/auth_provider.dart[320-347]
- lib/core/data/providers/session_provider.dart[171-177]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

5. getHealthCheckServers swallows exceptions 📘 Rule violation ⛯ Reliability
Description
getHealthCheckServers() catches all exceptions, logs a short message, and returns an empty list.
• This makes failures indistinguishable from a legitimate "no servers" response, reducing
  debuggability and potentially causing incorrect UI behavior.
• Robust error handling should preserve actionable context (and/or surface an error state) instead
  of silently degrading.
Code

lib/page/health_check/services/health_check_service.dart[R269-288]

+  /// Fetches the list of available speed test servers from the router.
+  Future<List<HealthCheckServer>> getHealthCheckServers() async {
+    try {
+      final result = await _repo.send(
+        JNAPAction.getCloseHealthCheckServers,
+        auth: true,
+        fetchRemote: false,
+        cacheLevel: CacheLevel.localCached,
+      );
+
+      final List<dynamic> serverList =
+          result.output['healthCheckServers'] ?? [];
+      return serverList
+          .map((e) => HealthCheckServer.fromJson(e as Map<String, dynamic>))
+          .toList();
+    } catch (e) {
+      logger.d('Failed to fetch health check servers: $e');
+    }
+    return [];
+  }
Evidence
PR Compliance ID 3 requires handling failure points with meaningful context and avoiding swallowed
exceptions. The function catches exceptions broadly and returns [] with only a minimal debug log
line.

Rule 3: Generic: Robust Error Handling and Edge Case Management
lib/page/health_check/services/health_check_service.dart[269-288]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`getHealthCheckServers()` currently swallows exceptions and returns an empty list, which can hide real failures.

## Issue Context
Compliance requires robust error handling with actionable context and avoidance of silent failures.

## Fix Focus Areas
- lib/page/health_check/services/health_check_service.dart[269-288]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. HealthCheckServer.fromJson unsafe cast 📘 Rule violation ⛨ Security
Suggestion Impact:Replaced direct `as String?` casts with `?.toString()` and changed `serverPort` parsing from `as int?` to a safer `(as num?)?.toInt()` conversion with defaults, reducing crash risk from unexpected JSON types.

code diff:

   factory HealthCheckServer.fromJson(Map<String, dynamic> json) {
+    // Use safe type conversions for external API data
     return HealthCheckServer(
-      serverID: json['serverID'] as String? ?? '',
-      serverName: json['serverName'] as String? ?? '',
-      serverLocation: json['serverLocation'] as String? ?? '',
-      serverCountry: json['serverCountry'] as String? ?? '',
-      serverHostname: json['serverHostname'] as String? ?? '',
-      serverPort: json['serverPort'] as int? ?? 0,
+      serverID: json['serverID']?.toString() ?? '',
+      serverName: json['serverName']?.toString() ?? '',
+      serverLocation: json['serverLocation']?.toString() ?? '',
+      serverCountry: json['serverCountry']?.toString() ?? '',
+      serverHostname: json['serverHostname']?.toString() ?? '',
+      serverPort: (json['serverPort'] as num?)?.toInt() ?? 0,
     );

Description
HealthCheckServer.fromJson uses direct casts like json['serverPort'] as int?, which will throw
  if the API returns unexpected types (e.g., num or String).
• This parses external/router-provided data and should be validated/sanitized to avoid runtime
  crashes.
• Missing defensive parsing violates the security-first input validation requirement.
Code

lib/page/health_check/models/health_check_server.dart[R20-28]

+  factory HealthCheckServer.fromJson(Map<String, dynamic> json) {
+    return HealthCheckServer(
+      serverID: json['serverID'] as String? ?? '',
+      serverName: json['serverName'] as String? ?? '',
+      serverLocation: json['serverLocation'] as String? ?? '',
+      serverCountry: json['serverCountry'] as String? ?? '',
+      serverHostname: json['serverHostname'] as String? ?? '',
+      serverPort: json['serverPort'] as int? ?? 0,
+    );
Evidence
PR Compliance ID 6 requires external inputs be validated and handled securely. The model parsing
uses direct casts that can fail at runtime when input types differ from expectations.

Rule 6: Generic: Security-First Input Validation and Data Handling
lib/page/health_check/models/health_check_server.dart[20-28]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`HealthCheckServer.fromJson` uses direct casts for API-provided fields, which can crash if types differ.

## Issue Context
Router/JNAP outputs are external inputs and must be validated/sanitized per compliance.

## Fix Focus Areas
- lib/page/health_check/models/health_check_server.dart[20-28]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


7. Empty server ID sent 🐞 Bug ✓ Correctness
Suggestion Impact:Updated HealthCheckProvider.runHealthCheck() to only use provided/selected server IDs when they are non-empty, otherwise pass null so the service layer omits targetServerID. Also adjusted HealthCheckServer.fromJson() to use safe toString()/num conversions for external API data.

code diff:

# File: lib/page/health_check/models/health_check_server.dart
@@ -18,13 +18,14 @@
   });
 
   factory HealthCheckServer.fromJson(Map<String, dynamic> json) {
+    // Use safe type conversions for external API data
     return HealthCheckServer(
-      serverID: json['serverID'] as String? ?? '',
-      serverName: json['serverName'] as String? ?? '',
-      serverLocation: json['serverLocation'] as String? ?? '',
-      serverCountry: json['serverCountry'] as String? ?? '',
-      serverHostname: json['serverHostname'] as String? ?? '',
-      serverPort: json['serverPort'] as int? ?? 0,
+      serverID: json['serverID']?.toString() ?? '',
+      serverName: json['serverName']?.toString() ?? '',
+      serverLocation: json['serverLocation']?.toString() ?? '',
+      serverCountry: json['serverCountry']?.toString() ?? '',
+      serverHostname: json['serverHostname']?.toString() ?? '',
+      serverPort: (json['serverPort'] as num?)?.toInt() ?? 0,
     );
   }

# File: lib/page/health_check/providers/health_check_provider.dart
@@ -103,10 +103,12 @@
     final service = ref.read(speedTestServiceProvider);
 
     // Use the provided serverId, or fall back to the selected server's ID
-    final targetServerId = serverId?.toString() ??
-        (state.selectedServer?.serverID != null
-            ? state.selectedServer!.serverID
-            : null);
+    // Only use non-empty server IDs to avoid sending empty strings to the router
+    final providedId = serverId?.toString();
+    final selectedId = state.selectedServer?.serverID;
+    final targetServerId = (providedId?.isNotEmpty == true)
+        ? providedId
+        : (selectedId?.isNotEmpty == true ? selectedId : null);
 
     _streamSubscription = service
         .runHealthCheck(module, targetServerId: targetServerId)

Description
HealthCheckServer.fromJson() defaults a missing serverID to an empty string, so
  selectedServer.serverID can be "".
• HealthCheckProvider.runHealthCheck() treats any selected server as valid and will pass that
  empty string through as targetServerID.
• Because _initiateHealthCheck() only strips null values (not empty strings), the request can
  include an invalid server ID and cause router-side errors or unpredictable behavior.
Code

lib/page/health_check/providers/health_check_provider.dart[R105-113]

+    // Use the provided serverId, or fall back to the selected server's ID
+    final targetServerId = serverId?.toString() ??
+        (state.selectedServer?.serverID != null
+            ? state.selectedServer!.serverID
+            : null);
+
+    _streamSubscription = service
+        .runHealthCheck(module, targetServerId: targetServerId)
+        .listen((event) {
Evidence
The model parser can produce serverID == '', and the provider forwards it without checking
isNotEmpty. The service layer then includes targetServerID unless it’s null, meaning '' will
be sent.

lib/page/health_check/models/health_check_server.dart[20-28]
lib/page/health_check/providers/health_check_provider.dart[105-113]
lib/page/health_check/services/health_check_service.dart[109-115]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
An empty `serverID` (&quot;&quot;&quot;) can be produced by JSON parsing and then sent to the router as `targetServerID`, since only `null` is removed from the payload.

### Issue Context
This can cause API errors or unintended default behavior.

### Fix Focus Areas
- lib/page/health_check/models/health_check_server.dart[20-28]
- lib/page/health_check/providers/health_check_provider.dart[105-113]
- lib/page/health_check/services/health_check_service.dart[109-115]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (3)
8. Asset manifest log spam 🐞 Bug ✧ Quality
Description
BrandUtils._loadManifest() logs the entire list of AssetManifest.json keys.
• The manifest can be large; logging all keys can bloat logs and slow the first call to brand-asset
  resolution.
• This is especially noisy in debug builds and makes relevant logs harder to find.
Code

lib/utils.dart[R684-688]

+      final manifestContent = await rootBundle.loadString('AssetManifest.json');
+      final Map<String, dynamic> manifestMap = json.decode(manifestContent);
+      logger.d('Loaded AssetManifest.json: ${manifestMap.keys}');
+      _manifestAssets = manifestMap.keys.toSet();
+    } catch (e) {
Evidence
The implementation prints the full manifestMap.keys collection rather than a summary, which can be
very large.

lib/utils.dart[681-688]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Logging the full list of `AssetManifest.json` keys is noisy and can be expensive.

### Issue Context
This runs on first use of `BrandUtils.resolveAsset()`.

### Fix Focus Areas
- lib/utils.dart[681-688]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


9. SW version mismatch 🐞 Bug ➹ Performance
Description
flutter_bootstrap.js sets serviceWorkerUrl: "service_worker.js" without embedding the
  serviceWorkerVersion in the URL.
• Flutter’s loader logic checks registration.active.scriptURL.endsWith(serviceWorkerVersion); with
  a custom URL that does not end with the version, it will always treat the SW as out-of-date and
  attempt update().
• This can increase load time and cause repeated SW update cycles on every page load.
Code

web/flutter_bootstrap.js[R18-20]

+    serviceWorkerVersion: "3346174710",
+    serviceWorkerUrl: "service_worker.js"
  }
Evidence
The bootstrap config overrides the SW URL to a stable filename, while the loader code uses the
version string as a suffix match against the active script URL; the condition can never be true when
the URL doesn’t contain the version.

web/flutter_bootstrap.js[12-21]
web/flutter_bootstrap.js[1-3]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Flutter loader determines whether the active service worker matches the expected version by checking if the active script URL ends with `serviceWorkerVersion`. With a custom `serviceWorkerUrl` that doesn’t include the version, the loader can end up updating every time.

### Issue Context
This affects web/PWA load performance and SW lifecycle behavior.

### Fix Focus Areas
- web/flutter_bootstrap.js[12-21]
- web/flutter_bootstrap.js[1-3]
- web/service_worker.js[1-16]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


10. SW forces immediate activation 🐞 Bug ⛯ Reliability
Description
• The custom service_worker.js unconditionally calls skipWaiting() on install and
  clients.claim() on activate.
• This can cause the new service worker to take control immediately, potentially while the app still
  has older cached assets loaded, increasing the risk of transient inconsistencies after an update.
• If kept, consider coordinating updates with an app-side “new version available, refresh” flow.
Code

web/service_worker.js[R8-16]

+// Force immediate activation when a new SW is installed
+self.addEventListener('install', (event) => {
+    self.skipWaiting();
+});
+
+self.addEventListener('activate', (event) => {
+    // Claim any clients immediately, so the new SW controls the page ASAP
+    event.waitUntil(clients.claim());
+});
Evidence
The wrapper SW forces immediate activation/claim without any guardrails or coordination with app
asset versioning.

web/service_worker.js[8-16]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Forcing SW activation/control immediately can create mixed-version states during updates.

### Issue Context
This wrapper SW imports Flutter’s generated SW but overrides lifecycle behavior.

### Fix Focus Areas
- web/service_worker.js[8-16]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments

11. notifier field too generic 📘 Rule violation ✓ Correctness
Description
SpeedTestServerSelectionList exposes a ValueNotifier field named notifier, which is not
  self-documenting.
• This makes call sites harder to read (it’s unclear what the notifier represents without inspecting
  implementation).
• Renaming it to something intent-revealing (e.g., selectedServerNotifier) improves clarity and
  meets naming requirements.
Code

lib/page/health_check/views/components/speed_test_server_selection_dialog.dart[R5-13]

+class SpeedTestServerSelectionList extends StatelessWidget {
+  final List<HealthCheckServer> servers;
+  final ValueNotifier<HealthCheckServer?> notifier;
+
+  const SpeedTestServerSelectionList({
+    super.key,
+    required this.servers,
+    required this.notifier,
+  });
Evidence
PR Compliance ID 2 requires identifiers to clearly express purpose. The name notifier is generic
and does not describe the state being notified (selected server).

Rule 2: Generic: Meaningful Naming and Self-Documenting Code
lib/page/health_check/views/components/speed_test_server_selection_dialog.dart[5-13]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The identifier `notifier` is generic and not self-documenting for server selection state.

## Issue Context
Compliance requires meaningful naming so code is readable without extra comments.

## Fix Focus Areas
- lib/page/health_check/views/components/speed_test_server_selection_dialog.dart[5-13]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

AustinChangLinksys and others added 2 commits February 5, 2026 14:44
- Add session state clear on logout to prevent stale device identity
- Add empty server ID validation to avoid sending empty strings to router
- Use safe type conversions in HealthCheckServer.fromJson for external API data
- Mask sensitive data (serial number) in session logs
- Translate migration documentation to English

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@PeterJhongLinksys PeterJhongLinksys merged commit 9730458 into dev-2.0.0 Feb 5, 2026
2 checks passed
@PeterJhongLinksys PeterJhongLinksys deleted the austin/migration-1.2.8 branch February 5, 2026 07:46
@AustinChangLinksys AustinChangLinksys linked an issue Feb 5, 2026 that may be closed by this pull request
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.

[2.0.0] Merge 1.2.8 to 2.0.0

2 participants