Skip to content

refactor: Reorganize shared widgets and fix wired pairing bug#618

Merged
PeterJhongLinksys merged 2 commits intodev-2.0.0from
austin/faq-agent-refactor
Feb 6, 2026
Merged

refactor: Reorganize shared widgets and fix wired pairing bug#618
PeterJhongLinksys merged 2 commits intodev-2.0.0from
austin/faq-agent-refactor

Conversation

@AustinChangLinksys
Copy link
Collaborator

Summary

  • Reorganize reusable widgets into shared_widgets/ folders for better code organization
  • Refactor FAQ Agent into modular structure with separated constants, services, generators, and components
  • Fix critical bug where startAutoOnboarding was called repeatedly (40+ times/second) in wired pairing dialog

Changes

Shared Widgets Reorganization

  • FAQ Agent: support/widgets/support/shared_widgets/faq_agent/
  • Speed Test: health_check/widgets/health_check/shared_widgets/
  • Topology Card: instant_topology/views/widgets/instant_topology/shared_widgets/

Bug Fixes

  • Wired Pairing Dialog: Fixed addPostFrameCallback inside Consumer builder causing repeated startAutoOnboarding calls on every rebuild
  • instant_verify: Fixed _doInstantPairWired to show proper wired pairing dialog instead of navigating to addNodes page

New Files

  • lib/page/nodes/widgets/wired_pairing_dialog_content.dart - ConsumerStatefulWidget ensuring single initialization

Test plan

  • Verify FAQ Agent FAB works correctly on Support page
  • Verify Speed Test widget displays on Dashboard and InstantVerify
  • Verify Topology Card displays correctly in InstantVerify
  • Test wired node pairing from Topology menu - confirm startAutoOnboarding is called only once
  • Test wired node pairing from InstantVerify menu

🤖 Generated with Claude Code

- Move reusable widgets to `shared_widgets/` folders for clarity:
  - FAQ Agent: support/widgets/ → support/shared_widgets/faq_agent/
  - Speed Test: health_check/widgets/ → health_check/shared_widgets/
  - Topology Card: instant_topology/views/widgets/ → instant_topology/shared_widgets/

- Refactor FAQ Agent into modular structure:
  - Extract constants, services, generators, and components
  - Improve code organization and maintainability

- Fix wired pairing dialog bug where startAutoOnboarding was called
  repeatedly (40+ times/second) due to addPostFrameCallback in Consumer builder
  - Create WiredPairingDialogContent as ConsumerStatefulWidget
  - Move startAutoOnboarding call to initState for single execution

- Fix instant_verify _doInstantPairWired to show proper pairing dialog
  instead of navigating to addNodes page

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

Review Summary by Qodo

Refactor shared widgets, fix wired pairing bug, and add JNAP-TR181 protocol mapper

✨ Enhancement 🐞 Bug fix 🧪 Tests

Grey Divider

Walkthroughs

Description
• **Major refactoring**: Reorganized shared widgets into dedicated shared_widgets/ folders across
  multiple features (FAQ Agent, Speed Test, Topology Card) for improved code organization and
  reusability
• **Critical bug fix**: Fixed startAutoOnboarding being called repeatedly (40+ times/second) in
  wired pairing dialog by moving addPostFrameCallback outside Consumer builder into
  ConsumerStatefulWidget
• **Protocol enhancement**: Added comprehensive JNAP to TR-181 protocol mapper
  (jnap_tr181_mapper.dart) with 1100+ lines supporting 15+ JNAP actions for USP protocol
  compatibility
• **Internet settings refactoring**: Decomposed massive 1300+ line monolithic view into modular
  components (Ipv4ConnectionView, Ipv6ConnectionView, ReleaseAndRenewView) with simplified state
  management
• **WAN form factory**: New factory pattern for dynamic WAN form creation supporting 6 connection
  types (DHCP, PPPoE, Static IP, PPTP, L2TP, Bridge)
• **Test improvements**: Refactored multiple test files to use new TestHelper utility class,
  reducing boilerplate and improving maintainability (dashboard tests reduced from 1225 to 447 lines)
• **Device support**: Added M60, M61, M62 router models to device mapping with cognitive mesh
  support
• **Dependency cleanup**: Removed RouterRadio dependency from WiFi settings and consolidated
  imports with null-safety improvements
• **Route configuration**: Added PnP add nodes route with centered column layout
Diagram
flowchart LR
  A["Monolithic Views<br/>Internet Settings<br/>Dashboard<br/>Apps & Gaming"] -->|"Refactor into<br/>Modular Components"| B["Component Architecture<br/>Ipv4ConnectionView<br/>Ipv6ConnectionView<br/>ReleaseAndRenewView"]
  C["Shared Widgets<br/>FAQ Agent<br/>Speed Test<br/>Topology Card"] -->|"Reorganize into<br/>shared_widgets/"| D["Reusable Widget<br/>Modules"]
  E["Wired Pairing<br/>Dialog Bug"] -->|"Move callback<br/>to ConsumerStatefulWidget"| F["Single Initialization<br/>startAutoOnboarding<br/>called once"]
  G["JNAP Actions"] -->|"Map to TR-181<br/>Paths"| H["USP Protocol<br/>Compatibility"]
  I["WAN Form<br/>Creation"] -->|"Factory Pattern"| J["Dynamic Form<br/>Support<br/>6 Types"]
Loading

Grey Divider

File Changes

1. test/page/advanced_settings/apps_and_gaming/views/localizations/apps_and_gaming_view_test.dart 🧪 Tests +727/-1301

Refactor apps and gaming tests with TestHelper utility

• Refactored test file to use new TestHelper utility class instead of manual mock setup
• Simplified imports by consolidating related imports into barrel exports
• Added comprehensive test documentation with test IDs and descriptions for all test cases
• Replaced manual widget tree construction with testHelper.pumpView() helper method
• Updated test assertions to use localization context and added golden file references
• Improved tab switching logic with new switchToTab() helper function
• Enhanced test coverage with better error state validation and form interaction patterns

test/page/advanced_settings/apps_and_gaming/views/localizations/apps_and_gaming_view_test.dart


2. lib/page/wifi_settings/providers/wifi_item.dart Refactoring +36/-179

Remove RouterRadio dependency and add null-safety

• Removed fromRadio() factory constructor that depended on RouterRadio model
• Updated imports to use local wifi_enums.dart instead of core/jnap/models/radio_info.dart
• Added null-safety checks in fromMap() factory to handle missing fields with sensible defaults
• Added barrel export for wifi_enums.dart to simplify imports

lib/page/wifi_settings/providers/wifi_item.dart


3. lib/page/instant_device/providers/device_list_state.dart Refactoring +2/-171

Update device list state imports and exports

• Removed unused imports (collection package and device_manager_state)
• Changed import from core/jnap/providers/device_manager_state.dart to
 core/models/device_list_item.dart
• Added barrel export for DeviceListItem model
• Removed file-level ignore comment for linting rules

lib/page/instant_device/providers/device_list_state.dart


View more (109)
4. lib/route/route_pnp.dart ✨ Enhancement +12/-35

Add PnP add nodes route configuration

• Removed extra blank line after part of statement
• Added new nested route pnpAddNodes under pnpRoute with AddNodesView builder
• Configured route with centered column layout (6 columns) and no navigation rail
• Route accepts extra arguments passed as Map<String, dynamic>

lib/route/route_pnp.dart


5. lib/core/jnap/models/back_haul_info.dart Formatting +5/-2

Format code for readability in back haul info

• Improved code formatting in copyWith() method for isMultiLinkOperation field
• Improved code formatting in fromJson() factory method with proper line breaks

lib/core/jnap/models/back_haul_info.dart


6. lib/page/advanced_settings/internet_settings/views/internet_settings_view.dart Refactoring +128/-1788

Refactor internet settings into modular component architecture

• Refactored massive monolithic view into modular components (Ipv4ConnectionView,
 Ipv6ConnectionView, ReleaseAndRenewView)
• Removed 1300+ lines of inline form building logic and replaced with extracted widget components
• Simplified state management by removing individual TextEditingControllers and replacing with form
 provider pattern
• Updated to use new UiKitPageView.withSliver and UiKitBottomBarConfig from ui_kit_library
• Removed complex editing state tracking (isIpv4Editing, isIpv6Editing) in favor of single
 isEditing boolean

lib/page/advanced_settings/internet_settings/views/internet_settings_view.dart


7. lib/core/usp/jnap_tr181_mapper.dart ✨ Enhancement +1187/-0

Add comprehensive JNAP to TR-181 protocol mapper

• New comprehensive mapper class converting JNAP Actions to TR-181 paths and vice versa
• Implements 1100+ lines of mapping logic for 15+ JNAP actions (GetDeviceInfo, GetRadioInfo,
 GetDevices, GetWANStatus, etc.)
• Provides getTr181Paths() to get TR-181 paths for JNAP actions and toJnapResponse() to convert
 USP responses to JNAP format
• Includes detailed conversion methods for device info, radio settings, network connections,
 backhaul info, and system stats
• Supports version suffix stripping (e.g., GetRadioInfo3GetRadioInfo) for flexible action
 matching

lib/core/usp/jnap_tr181_mapper.dart


8. lib/page/advanced_settings/internet_settings/widgets/wan_forms/wan_form_factory.dart ✨ Enhancement +43/-0

Add WAN form factory for dynamic form creation

• New factory class for creating WAN form widgets based on connection type
• Supports 6 WAN types: DHCP, PPPoE, Static IP, PPTP, L2TP, and Bridge
• Centralizes form widget instantiation logic with consistent isEditing parameter
• Enables easy addition of new WAN form types in future

lib/page/advanced_settings/internet_settings/widgets/wan_forms/wan_form_factory.dart


9. test/page/dashboard/localizations/dashboard_home_view_test.dart 🧪 Tests +407/-1185

Refactor dashboard home view tests with helper utilities

• Refactored test file from 1225 to 447 lines, removing verbose mock setup and replacing with
 TestHelper utility class
• Reorganized tests into 15 focused test cases with descriptive IDs (DHOME-NOLAN_BASE,
 DHOME-VERT_BASE, etc.)
• Simplified test structure by extracting common setup logic into helper methods (pumpDashboard,
 scrollToQuickPanel, toggleInstantPrivacy)
• Updated imports to use new component paths (fixed_layout/ subdirectory) and new provider types

test/page/dashboard/localizations/dashboard_home_view_test.dart


10. test/test_data/instant_verify_test_state.dart 🧪 Tests +72/-1677

Simplify instant verify test data radio configurations

• Removed verbose radio configuration details (physicalRadioID, bssid, supportedModes,
 supportedChannelsForChannelWidths, supportedSecurityTypes, maxRADIUSSharedKeyLength,
 wpaPersonalSettings)
• Simplified radio objects to contain only essential fields: radioID, band, isEnabled, ssid,
 channel, channelWidth, security
• Removed unnecessary guest network fields (guestWPAPassphrase, canEnableRadio) from all test
 state variants
• Applied simplification consistently across four test state objects: instantVerifyTestState,
 instantVerifyMultiDNSTestState, instantVerifyInternetOffTestState,
 instantVerifyAllWiFiOffTestState

test/test_data/instant_verify_test_state.dart


11. lib/core/utils/nodes.dart ✨ Enhancement +24/-1

Add M60, M61, M62 router models to device mapping

• Added three new router models to the _velopModelMap: M60, M61, and M62
• Each new model configured as non-mesh router (isMeshRouter: false) with cognitive mesh support
 (isCognitiveMesh: true)
• Added corresponding regex patterns for model matching: ^m60, ^m61, ^m62

lib/core/utils/nodes.dart


12. .agent/workflows/manual-testing.md Additional files +169/-0

...

.agent/workflows/manual-testing.md


13. .agent/workflows/review-screenshot-tests.md Additional files +142/-0

...

.agent/workflows/review-screenshot-tests.md


14. .agent/workflows/service-decoupling-audit.md Additional files +193/-0

...

.agent/workflows/service-decoupling-audit.md


15. .claude/commands/speckit.analyze.md Additional files +184/-0

...

.claude/commands/speckit.analyze.md


16. .claude/commands/speckit.checklist.md Additional files +294/-0

...

.claude/commands/speckit.checklist.md


17. .claude/commands/speckit.clarify.md Additional files +181/-0

...

.claude/commands/speckit.clarify.md


18. .claude/commands/speckit.constitution.md Additional files +82/-0

...

.claude/commands/speckit.constitution.md


19. .claude/commands/speckit.implement.md Additional files +135/-0

...

.claude/commands/speckit.implement.md


20. .claude/commands/speckit.plan.md Additional files +89/-0

...

.claude/commands/speckit.plan.md


21. .claude/commands/speckit.specify.md Additional files +258/-0

...

.claude/commands/speckit.specify.md


22. .claude/commands/speckit.tasks.md Additional files +137/-0

...

.claude/commands/speckit.tasks.md


23. .claude/commands/speckit.taskstoissues.md Additional files +30/-0

...

.claude/commands/speckit.taskstoissues.md


24. .claude/settings.local.json Additional files +24/-0

...

.claude/settings.local.json


25. .fvmrc Additional files +3/-0

...

.fvmrc


26. .github/workflows/ci.yml Additional files +167/-0

...

.github/workflows/ci.yml


27. .github/workflows/deploy-demo.yml Additional files +90/-0

...

.github/workflows/deploy-demo.yml


28. .gitmodules Additional files +0/-3

...

.gitmodules


29. .metadata Additional files +30/-0

...

.metadata


30. .specify/memory/constitution.md Additional files +1093/-0

...

.specify/memory/constitution.md


31. .specify/scripts/bash/check-prerequisites.sh Additional files +166/-0

...

.specify/scripts/bash/check-prerequisites.sh


32. .specify/scripts/bash/common.sh Additional files +156/-0

...

.specify/scripts/bash/common.sh


33. .specify/scripts/bash/create-new-feature.sh Additional files +297/-0

...

.specify/scripts/bash/create-new-feature.sh


34. .specify/scripts/bash/setup-plan.sh Additional files +61/-0

...

.specify/scripts/bash/setup-plan.sh


35. .specify/scripts/bash/update-agent-context.sh Additional files +799/-0

...

.specify/scripts/bash/update-agent-context.sh


36. .specify/templates/agent-file-template.md Additional files +28/-0

...

.specify/templates/agent-file-template.md


37. .specify/templates/checklist-template.md Additional files +40/-0

...

.specify/templates/checklist-template.md


38. .specify/templates/plan-template.md Additional files +104/-0

...

.specify/templates/plan-template.md


39. .specify/templates/spec-template.md Additional files +115/-0

...

.specify/templates/spec-template.md


40. .specify/templates/tasks-template.md Additional files +251/-0

...

.specify/templates/tasks-template.md


41. .vscode/launch.json Additional files +32/-84

...

.vscode/launch.json


42. .windsurfrules Additional files +385/-0

...

.windsurfrules


43. AGENTS.md Additional files +29/-0

...

AGENTS.md


44. APPGAP_MAPPING.md Additional files +148/-0

...

APPGAP_MAPPING.md


45. CLAUDE.md Additional files +169/-0

...

CLAUDE.md


46. README.md Additional files +121/-10

...

README.md


47. SCREENSHOT_TESTING.md Additional files +146/-0

...

SCREENSHOT_TESTING.md


48. TEST_CASES.md Additional files +381/-0

...

TEST_CASES.md


49. THEME.md Additional files +201/-0

...

THEME.md


50. android/app/google-services.json Additional files +0/-46

...

android/app/google-services.json


51. android/app/src/debug/AndroidManifest.xml Additional files +0/-7

...

android/app/src/debug/AndroidManifest.xml


52. android/app/src/main/AndroidManifest.xml Additional files +0/-73

...

android/app/src/main/AndroidManifest.xml


53. android/app/src/main/kotlin/com/linksys/moab/app/MainActivity.kt Additional files +0/-7

...

android/app/src/main/kotlin/com/linksys/moab/app/MainActivity.kt


54. android/app/src/main/res/drawable-night-v21/launch_background.xml Additional files +0/-9

...

android/app/src/main/res/drawable-night-v21/launch_background.xml


55. android/app/src/main/res/drawable-night/launch_background.xml Additional files +0/-9

...

android/app/src/main/res/drawable-night/launch_background.xml


56. android/app/src/main/res/drawable-v21/launch_background.xml Additional files +0/-9

...

android/app/src/main/res/drawable-v21/launch_background.xml


57. android/app/src/main/res/drawable/launch_background.xml Additional files +0/-9

...

android/app/src/main/res/drawable/launch_background.xml


58. android/app/src/main/res/mipmap-anydpi-v26/ic_launcher.xml Additional files +0/-5

...

android/app/src/main/res/mipmap-anydpi-v26/ic_launcher.xml


59. android/app/src/main/res/mipmap-anydpi-v26/ic_launcher_round.xml Additional files +0/-5

...

android/app/src/main/res/mipmap-anydpi-v26/ic_launcher_round.xml


60. android/app/src/main/res/values-night-v31/styles.xml Additional files +0/-21

...

android/app/src/main/res/values-night-v31/styles.xml


61. android/app/src/main/res/values-night/styles.xml Additional files +0/-21

...

android/app/src/main/res/values-night/styles.xml


62. android/app/src/main/res/values-v31/styles.xml Additional files +0/-21

...

android/app/src/main/res/values-v31/styles.xml


63. android/app/src/main/res/values/styles.xml Additional files +0/-21

...

android/app/src/main/res/values/styles.xml


64. android/app/src/main/res/xml/network_security_config.xml Additional files +0/-7

...

android/app/src/main/res/xml/network_security_config.xml


65. android/app/src/profile/AndroidManifest.xml Additional files +0/-7

...

android/app/src/profile/AndroidManifest.xml


66. android/gradle.properties Additional files +0/-3

...

android/gradle.properties


67. android/gradle/wrapper/gradle-wrapper.properties Additional files +0/-6

...

android/gradle/wrapper/gradle-wrapper.properties


68. android/project.properties Additional files +0/-4

...

android/project.properties


69. assets/a2ui/widgets/device_count.json Additional files +73/-0

...

assets/a2ui/widgets/device_count.json


70. assets/a2ui/widgets/guest_network.json Additional files +144/-0

...

assets/a2ui/widgets/guest_network.json


71. assets/a2ui/widgets/network_traffic.json Additional files +188/-0

...

assets/a2ui/widgets/network_traffic.json


72. assets/a2ui/widgets/node_count.json Additional files +74/-0

...

assets/a2ui/widgets/node_count.json


73. assets/a2ui/widgets/quick_actions.json Additional files +206/-0

...

assets/a2ui/widgets/quick_actions.json


74. assets/a2ui/widgets/router_control.json Additional files +203/-0

...

assets/a2ui/widgets/router_control.json


75. assets/a2ui/widgets/system_health.json Additional files +266/-0

...

assets/a2ui/widgets/system_health.json


76. assets/a2ui/widgets/wan_status.json Additional files +81/-0

...

assets/a2ui/widgets/wan_status.json


77. assets/agents/env.template Additional files +5/-0

...

assets/agents/env.template


78. assets/brand/meta.json Additional files +26/-0

...

assets/brand/meta.json


79. assets/resources/demo_cache_data.json Additional files +1580/-0

...

assets/resources/demo_cache_data.json


80. build_android.sh Additional files +0/-61

...

build_android.sh


81. build_ios.sh Additional files +0/-53

...

build_ios.sh


82. build_web.sh Additional files +9/-2

...

build_web.sh


83. clear_goldens.sh Additional files +30/-0

...

clear_goldens.sh


84. constitution.md Additional files +1085/-0

...

constitution.md


85. coverage/lcov.info Additional files +59460/-0

...

coverage/lcov.info


86. dart_test.yaml Additional files +2/-1

...

dart_test.yaml


87. devtools_options.yaml Additional files +3/-0

...

devtools_options.yaml


88. doc/accessibility/README.md Additional files +29/-0

...

doc/accessibility/README.md


89. doc/accessibility/analysis_engine_demo.md Additional files +250/-0

...

doc/accessibility/analysis_engine_demo.md


90. doc/accessibility/history/2026-01_compliance_report.md Additional files +91/-0

...

doc/accessibility/history/2026-01_compliance_report.md


91. doc/accessibility/implementation_analysis.md Additional files +55/-0

...

doc/accessibility/implementation_analysis.md


92. doc/accessibility/integration_guide.md Additional files +677/-0

...

doc/accessibility/integration_guide.md


93. doc/accessibility/testing_guide.md Additional files +497/-0

...

doc/accessibility/testing_guide.md


94. doc/ai_assistant/FAQ_AGENT.md Additional files +279/-0

...

doc/ai_assistant/FAQ_AGENT.md


95. doc/ai_assistant/router_ai_assistant_architecture.md Additional files +277/-0

...

doc/ai_assistant/router_ai_assistant_architecture.md


96. doc/architecture_analysis_2026-01-16.md Additional files +609/-0

...

doc/architecture_analysis_2026-01-16.md


97. doc/archive/architecture_analysis_2026-01-05.md Additional files +727/-0

...

doc/archive/architecture_analysis_2026-01-05.md


98. doc/audit/architecture-analysis.md Additional files +277/-0

...

doc/audit/architecture-analysis.md


99. doc/audit/architecture-violations-detail.md Additional files +330/-0

...

doc/audit/architecture-violations-detail.md


100. doc/audit/platform-conditional-exports-audit.md Additional files +287/-0

...

doc/audit/platform-conditional-exports-audit.md


101. doc/audit/service-decoupling-audit.md Additional files +105/-0

...

doc/audit/service-decoupling-audit.md


102. doc/dashboard/dashboard_custom_layout_comprehensive_report_en.md Additional files +799/-0

...

doc/dashboard/dashboard_custom_layout_comprehensive_report_en.md


103. doc/dirty_guard/dirty_guard_framework_guide.md Additional files +226/-0

...

doc/dirty_guard/dirty_guard_framework_guide.md


104. doc/dirty_guard/dirty_guard_framework_requirements.md Additional files +266/-0

...

doc/dirty_guard/dirty_guard_framework_requirements.md


105. doc/migration/dev-1.2.8-migration-tasks.md Additional files +609/-0

...

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


106. 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


107. doc/pnp/pnp-flow.md Additional files +144/-0

...

doc/pnp/pnp-flow.md


108. doc/pnp/pnp.md Additional files +58/-0

...

doc/pnp/pnp.md


109. doc/responsive_page_view.md Additional files +574/-0

...

doc/responsive_page_view.md


110. doc/routing.md Additional files +51/-0

...

doc/routing.md


111. doc/screenshot_test/SCREENSHOT_TEST_ANALYSIS_REPORT.md Additional files +205/-0

...

doc/screenshot_test/SCREENSHOT_TEST_ANALYSIS_REPORT.md


112. Additional files not shown Additional files +0/-0

...

Additional files not shown


Grey Divider

Qodo Logo

@qodo-code-review
Copy link

Code Review by Qodo

🐞 Bugs (4) 📘 Rule violations (2) 📎 Requirement gaps (0)

Grey Divider


Action required

1. No cleanup on onboarding error 📘 Rule violation ⛯ Reliability
Description
startAutoOnboarding() performs multiple awaited operations after pausing the idle checker and
  enabling router auto-onboarding, but it has no try/catch/finally to guarantee cleanup.
• If any awaited call throws, the router setting may remain enabled and idleCheckerPauseProvider
  may remain true, leaving the app in a degraded state.
• This violates the requirement to handle failure points and edge cases with graceful recovery and
  actionable context.
Code

lib/page/nodes/providers/add_wired_nodes_provider.dart[R47-86]

+  Future startAutoOnboarding(BuildContext context) async {
+    logger.d('[AddWiredNode]: start auto onboarding');
+    final log = BenchMarkLogger(name: 'Add Wired Node Process')..start();
+    state = state.copyWith(
+      isLoading: true,
+      forceStop: false,
+      loadingMessage: loc(context).addNodesSearchingNodes,
+    );
+    // Pause the idle checker
+    ref.read(idleCheckerPauseProvider.notifier).state = true;
+    // Set router auto onboarding to true
+    await setAutoOnboardingSettings(true);
+    if (!context.mounted) return;
+
+    // Get backhaul info from deviceManager and convert to UI model
+    final backhaulData = ref.read(deviceManagerProvider).backhaulInfoData;
+    final backhaulSnapshot = backhaulData
+        .map((data) => BackhaulInfoUIModel(
+              deviceUUID: data.deviceUUID,
+              connectionType: data.connectionType,
+              timestamp: data.timestamp,
+            ))
+        .toList();
+    state = state.copyWith(backhaulSnapshot: backhaulSnapshot);
+
+    logger.d('[AddWiredNode]: check backhaul changes');
+    await _checkBackhaulChanges(context, backhaulSnapshot);
+    // Set router auto onboarding to false
+    await stopAutoOnboarding();
+    // Fetch latest status
+    logger.d('[AddWiredNode]: fetch nodes');
+    final nodes = await _service.fetchNodes();
+    if (!context.mounted) return;
+    state = state.copyWith(nodes: nodes);
+    stopCheckingBackhaul(context);
+    await ref.read(pollingProvider.notifier).forcePolling();
+    // Resume the idle checker
+    ref.read(idleCheckerPauseProvider.notifier).state = false;
+    final delta = log.end();
+    logger.d('[AddWiredNode]: end auto onboarding, cost time: ${delta}ms');
Evidence
PR Compliance ID 3 requires robust handling of failure points and edge cases. In
startAutoOnboarding(), the idle checker is paused and auto-onboarding is enabled before multiple
awaited calls; however, cleanup (disabling auto-onboarding and resuming the idle checker) only
occurs on the success path and is not protected by finally, so exceptions can leave the system in
an inconsistent state.

Rule 3: Generic: Robust Error Handling and Edge Case Management
lib/page/nodes/providers/add_wired_nodes_provider.dart[47-86]

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

## Issue description
`AddWiredNodesNotifier.startAutoOnboarding()` pauses the idle checker and enables wired auto-onboarding on the router, then performs multiple awaited operations without a `try/catch/finally`. Any exception can prevent cleanup (disabling auto-onboarding and resuming the idle checker), leaving the app/router in an inconsistent state.

## Issue Context
This flow includes router JNAP calls and polling; these are failure-prone (network/auth/timeouts). Cleanup must be guaranteed even when exceptions occur.

## Fix Focus Areas
- lib/page/nodes/providers/add_wired_nodes_provider.dart[47-86]

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


2. VPN test blocked remote 🐞 Bug ✓ Correctness
Description
• RouterRepository now blocks any JNAP action not allowlisted as read-only when the app is in remote
  read-only mode.
• The allowlist does not include the VPN connection test action, but the new VPN settings UI
  triggers a VPN test; this will throw and can surface as an unhandled UI error.
• Result: VPN “Test Again” is unusable in remote mode and may degrade UX via thrown exceptions.
Code

lib/core/jnap/router_repository.dart[R81-106]

+  bool _isReadOnlyOperation(JNAPAction action) {
+    final actionValue = action.actionValue;
+    // Extract the last segment of the URL path (after the last '/')
+    final lastSegment = actionValue.split('/').last.toLowerCase();
+
+    // Allowlist of safe read-only operation prefixes
+    const safePrefixes = [
+      'get', // Get operations (getDeviceInfo, getWANSettings, etc.)
+      'is', // Status checks (isAdminPasswordDefault, etc.)
+      'check', // Validation operations (checkAdminPassword, etc.)
+    ];
+
+    // Allowlist of specific safe operations that don't modify configuration
+    const safeOperations = [
+      'startblinkingnodeled', // LED blinking operations
+      'stopblinkingnodeled',
+      'runhealthcheck', // Speed test operations (read-only, no config changes)
+      'stophealthcheck',
+      'startping', // Network diagnostic operations (read-only)
+      'stopping',
+      'starttraceroute',
+      'stoptraceroute',
+    ];
+
+    return safePrefixes.any((prefix) => lastSegment.startsWith(prefix)) ||
+        safeOperations.contains(lastSegment);
Evidence
The read-only allowlist only permits actions whose last URL segment starts with get/is/check, plus a
small list of explicit operations. TestVPNConnection does not match these rules, so
RouterRepository.send() will throw in remote read-only mode. The VPN page calls testVPNConnection
via doSomethingWithSpinner; doSomethingWithSpinner rethrows errors, and the VPN page call site
doesn’t attach an error handler for this action.

lib/core/jnap/router_repository.dart[75-136]
lib/page/vpn/service/vpn_service.dart[52-56]
lib/page/vpn/views/vpn_settings_page.dart[257-273]
lib/page/components/shortcuts/dialogs.dart[45-56]

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

## Issue description
Remote read-only mode blocks non-allowlisted JNAP actions. `TestVPNConnection` is not allowlisted, but the VPN UI triggers it, causing `RouterRepository.send()` to throw. The current UI call path doesn’t attach an error handler, so users can hit an unhandled error.

## Issue Context
Remote read-only mode is intended to prevent configuration changes. The VPN test appears to be a diagnostic operation, but it currently fails the read-only predicate.

## Fix Focus Areas
- lib/core/jnap/router_repository.dart[75-136]
- lib/page/vpn/views/vpn_settings_page.dart[257-273]
- lib/page/vpn/service/vpn_service.dart[52-56]
- lib/page/components/shortcuts/dialogs.dart[45-56]

## Suggested fix
1) Update `_isReadOnlyOperation` to allow VPN test if it is safe (e.g., add `&#x27;testvpnconnection&#x27;` to `safeOperations`, or broaden read-only prefixes with a clear comment).
2) In `VPNSettingsPage`, attach `.onError(...)` to show an error snackbar, and/or disable the test button when `remoteAccessProvider.isRemoteReadOnly` is true.

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


3. PnP pauses polling 🐞 Bug ⛯ Reliability
Description
• PnPService pauses the global polling loop (pollingProvider.notifier.paused = true) as part of
  device info fetch.
• There is no corresponding resume call on the normal (successful) PnP path, so background polling
  can remain stopped.
• Result: app-wide state can become stale (polling timer cancelled; startPolling is a no-op while
  paused).
Code

lib/page/instant_setup/services/pnp_service.dart[R64-73]

+    // Handle SharedPreferences
+    final prefs = await SharedPreferences.getInstance();
+    if (clearCurrentSN) {
+      await prefs.remove(pCurrentSN);
+    }
+    await prefs.setString(pPnpConfiguredSN, _rawDeviceInfo!.serialNumber);
+
+    // Pause polling
+    _ref.read(pollingProvider.notifier).paused = true;
+
Evidence
PnPService explicitly sets PollingNotifier.paused=true, which cancels the polling timer.
PollingNotifier.startPolling() immediately returns when paused, so polling will not restart unless
something explicitly unpauses (or forces polling). PnPNotifier only forces polling on a specific
interrupt/exit exception path, not on the normal completion path.

lib/page/instant_setup/services/pnp_service.dart[58-76]
lib/core/data/providers/polling_provider.dart[58-65]
lib/core/data/providers/polling_provider.dart[177-180]
lib/page/instant_setup/providers/pnp_provider.dart[233-237]

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

## Issue description
PnP flow pauses global polling (`paused=true`) but doesn’t clearly resume it on successful completion. This can leave the app without periodic polling updates.

## Issue Context
`PollingNotifier.paused` cancels the timer, and `startPolling()` is a no-op while paused. So a missing resume can lead to stale state.

## Fix Focus Areas
- lib/page/instant_setup/services/pnp_service.dart[58-76]
- lib/core/data/providers/polling_provider.dart[58-65]
- lib/core/data/providers/polling_provider.dart[177-180]
- lib/page/instant_setup/providers/pnp_provider.dart[309-358]

## Suggested fix
1) Add an explicit resume when leaving PnP (e.g., after `completeFwCheck()` sets `wizardWifiReady`, or when navigating away):
  - `ref.read(pollingProvider.notifier).paused = false;` OR
  - `ref.read(pollingProvider.notifier).checkAndStartPolling(true);`
2) Also resume on error states where the user exits PnP.
3) Consider scoping the pause to only the PnP experience if possible, so other app areas remain up-to-date.

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



Remediation recommended

4. fetchNodes() swallows exceptions 📘 Rule violation ⛯ Reliability
Description
AddWiredNodesService.fetchNodes() catches all exceptions, logs a generic message, and returns an
  empty list.
• This can mask underlying router/JNAP failures as "no nodes found", reducing debuggability and
  potentially leading to incorrect UI behavior.
• Robust error handling typically requires propagating a typed error (or returning a typed result)
  so callers can react appropriately.
Code

lib/page/nodes/services/add_wired_nodes_service.dart[R231-248]

+  Future<List<LinksysDevice>> fetchNodes() async {
+    try {
+      final response = await _routerRepository.send(
+        JNAPAction.getDevices,
+        fetchRemote: true,
+        auth: true,
+      );
+
+      final nodeList = List.from(response.output['devices'])
+          .map((e) => LinksysDevice.fromMap(e))
+          .where((device) => device.nodeType != null)
+          .toList();
+
+      return nodeList;
+    } catch (error) {
+      logger.i('[AddWiredNodesService]: fetch node failed! $error');
+      return [];
+    }
Evidence
PR Compliance ID 3 discourages silent failures and requires meaningful context and graceful
degradation. Here the method converts any exception into an empty list return value, which can be
interpreted as a successful "no nodes" response rather than an operational failure.

Rule 3: Generic: Robust Error Handling and Edge Case Management
lib/page/nodes/services/add_wired_nodes_service.dart[231-248]

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

## Issue description
`fetchNodes()` currently catches any exception and returns an empty list, which can hide operational failures and make the UI behave as if there are simply no nodes.

## Issue Context
This service already maps `JNAPError` to service-layer errors in other methods; `fetchNodes()` should align with that approach or otherwise return a result that distinguishes error vs empty-success.

## Fix Focus Areas
- lib/page/nodes/services/add_wired_nodes_service.dart[231-248]

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


5. PnP auth returns false 🐞 Bug ✓ Correctness
Description
PnpService.checkAdminPassword is declared Future<bool> but always returns false after
  awaiting a successful login.
• This violates the method contract and is misleading for future callers (or refactors) that may
  rely on the boolean to branch logic.
• It also makes debugging harder since success cannot be observed via the return value.
Code

lib/page/instant_setup/services/pnp_service.dart[R130-150]

+  /// Validates the admin password against the router.
+  Future<bool> checkAdminPassword(String? password) async {
+    logger.i('[PnP]: Service - Checking admin password.');
+    if (password == null) {
+      logger.w('[PnP]: Service - Admin password is null.');
+      throw ExceptionInvalidAdminPassword();
+    }
+    await _ref
+        .read(authProvider.notifier)
+        .localLogin(password, pnp: true, guardError: false)
+        .then((value) {
+      logger.i('[PnP]: Service - Admin password accepted.');
+      return true;
+    }).catchError((error) {
+      logger.e('[PnP]: Service - Admin password check failed.', error: error);
+      throw ExceptionInvalidAdminPassword();
+    },
+            test: (error) =>
+                error is JNAPError && error.result == errorJNAPUnauthorized);
+    return false; // Should not reach here if successful or error is thrown
+  }
Evidence
The method awaits the login Future, but then unconditionally returns false. The `.then((...) =>
true)` value is discarded because it is not returned/assigned; the function’s final return is always
false unless an exception is thrown.

lib/page/instant_setup/services/pnp_service.dart[130-150]

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

## Issue description
`PnpService.checkAdminPassword()` is typed as `Future&lt;bool&gt;` but currently returns `false` even on successful login.

## Issue Context
This is a contract violation and can easily cause future bugs if any caller uses the boolean return.

## Fix Focus Areas
- lib/page/instant_setup/services/pnp_service.dart[130-150]

## Suggested fix
Option A (keep boolean):
```dart
Future&lt;bool&gt; checkAdminPassword(String? password) async {
 if (password == null) throw ExceptionInvalidAdminPassword();
 try {
   await _ref.read(authProvider.notifier)
     .localLogin(password, pnp: true, guardError: false);
   return true;
 } on JNAPError catch (e) {
   if (e.result == errorJNAPUnauthorized) throw ExceptionInvalidAdminPassword();
   rethrow;
 }
}
```
Option B (if unused): change signature to `Future&lt;void&gt;` and remove the return value entirely.

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


6. Side-effect progress miscomputed 🐞 Bug ✧ Quality
Description
_updateProgress computes progress as (maxRetry - currentRetry) / maxRetry, which makes
  progress run backwards (starts near 100% and decreases).
• If maxRetry == -1 and maxPollTime == -1, this formula yields progress values >100% (e.g.,
  retry=1 → 200%), which can break/overflow progress UI.
• Result: misleading or invalid progress reporting during side-effect polling.
Code

lib/core/data/providers/side_effect_provider.dart[R345-358]

+  _updateProgress(
+    int currentRetry,
+    int maxRetry,
+    int startTime,
+    int maxPollTime,
+  ) {
+    if (maxPollTime != -1) {
+      final diff = DateTime.now().millisecondsSinceEpoch - startTime;
+      state = state.copyWith(
+          progress: ((diff / (maxPollTime * 1000)) * 100).toInt());
+    } else {
+      final diff = maxRetry - currentRetry;
+      state = state.copyWith(progress: ((diff / maxRetry) * 100).toInt());
+    }
Evidence
The else-branch uses remaining retries rather than completed retries, so progress decreases over
time. With maxRetry=-1, the computed ratio grows as currentRetry increases because both numerator
and denominator are negative.

lib/core/data/providers/side_effect_provider.dart[345-358]

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

## Issue description
Side-effect polling progress is computed backwards and can exceed 100% when `maxRetry` is -1.

## Issue Context
This affects any UI/logic that uses `SideEffectState.progress` while polling for side effects.

## Fix Focus Areas
- lib/core/data/providers/side_effect_provider.dart[345-358]

## Suggested fix
1) For retry-based progress:
```dart
if (maxRetry &gt; 0) {
 final pct = (currentRetry / maxRetry) * 100;
 state = state.copyWith(progress: pct.clamp(0, 100).toInt());
} else {
 // indeterminate
 state = state.copyWith(progress: 0);
}
```
2) For time-based progress: clamp to [0,100] as well.
3) Consider renaming `diff` to `elapsed`/`completed` to avoid confusion.

ⓘ 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

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@AustinChangLinksys AustinChangLinksys changed the base branch from dev-1.2.0 to dev-2.0.0 February 6, 2026 07:03
@PeterJhongLinksys PeterJhongLinksys merged commit b1a1880 into dev-2.0.0 Feb 6, 2026
2 checks passed
@PeterJhongLinksys PeterJhongLinksys deleted the austin/faq-agent-refactor branch February 6, 2026 08:10
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 participants