fix(polling): start timer when dashboardDomainReadyProvider already resolved

[AustinChangLinksys]

Summary

• Fix polling providers (traffic analysis, system monitor) not starting their timers when dashboardDomainReadyProvider has already completed
• ref.listen() only fires on state changes — if the provider was first read after domain ready resolved, the listener never fires
• Add immediate state check in build() to start timer if conditions are already met

Test plan

• All 2869 functional tests pass
• Affected provider tests pass (16 tests)
• Verified Dashboard traffic card displays data after login
• Verified Statistics page displays traffic data when navigating from Dashboard

🤖 Generated with Claude Code

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[AustinChangLinksys]

🤖 Automated Review — Round 1 · 34d7e43..954a39a (full)
Verdict: 💬 Self-review (comment only) — GitHub prohibits self-approval; this review is provided as a reference to catch blind spots.

	Where	Issue (one-liner)
🔴	usp_system_monitor_notifier.dart:47–56 · usp_traffic_analysis_notifier.dart:47–56	Identical 10-line block copy-pasted into both notifiers; violates DRY
⚠️	usp_system_monitor_notifier.dart:54 · usp_traffic_analysis_notifier.dart:54	[both reviewers] Future.microtask lacks ref.mounted guard; dispose race can throw StateError
⚠️	usp_system_monitor_notifier.dart:41–45 · usp_traffic_analysis_notifier.dart:42–46	[both reviewers] Auth-guard asymmetry: new microtask path checks isAuthenticated, existing ref.listen path does not
⚠️	test/	No unit tests for the new "already-resolved" startup path
💡	usp_system_monitor_notifier.dart:40 · usp_traffic_analysis_notifier.dart:41	defaultInterval is function-local; elevate to static const for test reuse
💡	usp_traffic_analysis_notifier.dart diff context	Context line shows refreshInterval: refreshInterval but local file has defaultInterval; verify base-commit correctness
💡	Both files	Add comment noting that rare double-call (microtask + listen both fire) is safe because setRefreshInterval cancels-then-restarts

Items marked [both reviewers] were independently flagged by two agents → higher confidence.

🔴 Critical — Duplicate Logic Details

UspSystemMonitorNotifier.build() lines 47–56 and UspTrafficAnalysisNotifier.build() lines 47–56 contain a byte-for-byte identical 10-line block (3-line comment + 7-line guard). Any future change to the guard condition (e.g. add logging, extend predicate) must be applied in both files in sync — a maintenance trap.

Fix: Extract a private helper in each notifier (can also combine the ref.mounted fix):

void _startTimerIfDomainAlreadyReady(Duration interval) {
  final domainReady = ref.read(dashboardDomainReadyProvider);
  final isAuthenticated = ref.read(appConnectionStateProvider) ==
      AppConnectionState.authenticated;
  if (domainReady is AsyncData && isAuthenticated) {
    Future.microtask(() {
      if (ref.mounted) setRefreshInterval(interval);
    });
  }
}

Then in build(): _startTimerIfDomainAlreadyReady(defaultInterval);

⚠️ Warning Details

ref.mounted guard missing (both files, new line ~54)

Future.microtask callbacks are not lifecycle-managed by Riverpod. If the container is disposed before the microtask executes (test teardown, rapid navigation), setRefreshInterval will call state = … on a disposed notifier → StateError: Cannot use "state" after the notifier was disposed. Riverpod 2.x provides ref.mounted; a search of the codebase found 0 current usages of it.

Fix:

Future.microtask(() {
  if (ref.mounted) setRefreshInterval(defaultInterval);
});

---

Auth-guard asymmetry between ref.listen and new ref.read path

The new microtask fallback checks appConnectionStateProvider == authenticated before starting the timer. The pre-existing ref.listen(dashboardDomainReadyProvider, …) callback in both notifiers does not perform the same check — it calls setRefreshInterval(defaultInterval) unconditionally whenever the provider resolves.

Result: if the user logs out and then dashboardDomainReadyProvider fires (e.g. from a cached resolution), the timer will be started in the unauthenticated state via the ref.listen path. The downstream _fetchAndAppend() auth guard prevents actual USP requests, but the timer runs needlessly.

Fix: Unify both paths — either add the auth check to the ref.listen callback, or remove it from the microtask path with a comment explaining that the downstream guard is sufficient.

---

No unit tests for new startup path

No test files exist for UspSystemMonitorNotifier or UspTrafficAnalysisNotifier in test/. The project has well-established notifier test patterns (see usp_dmz_notifier_test.dart, usp_wifi_settings_notifier_test.dart) that handle Future.microtask via await Future.delayed(Duration.zero).

Minimum coverage needed:

1. Provider already-resolved + authenticated → timer starts after microtask pump
2. Provider starts AsyncLoading → transitions to AsyncData via ref.listen → timer starts
3. Provider already-resolved + unauthenticated → timer does NOT start

✅ What looks good

• Core fix is correct and necessary. ref.listen does not replay the current value on subscription — the ref.read fallback is the right approach and matches the pattern used elsewhere in the codebase.
• Future.microtask deferral is appropriate. Calling setRefreshInterval synchronously inside build() would mutate state during the build phase; the microtask correctly defers to after build completes.
• All PrivacyGUI architecture rules satisfied: layer dependency direction ✅, L1 non-autoDispose ✅, ref.read (not watch) for cross-provider reads ✅, mutation lock not applicable, no codegen model misuse.
• No security concerns: no credentials, no injection surface, no auth bypass. USP/JNAP call paths are entirely unaffected by this PR.
• Symmetry across the two notifiers is maintained (both get the same fix, same pattern).

Cross-reviewed by two independent agents (security+correctness / architecture+maintainability). Automated — please sanity-check before merge.

[HankYuLinksys]

Two follow-ups before merge. The root-cause analysis is correct and the fix is sound, but (1) the new code path has no test coverage, and (2) the two start paths now use inconsistent guard conditions. Details inline.

[HankYuLinksys]

Fix looks correct. Two follow-ups inline (both apply to the traffic notifier too).

[HankYuLinksys]

This new path is never hit by the tests — neither test overrides dashboardDomainReadyProvider, so it stays AsyncLoading at first read and this if never runs. Please add a test that resolves it before first read and asserts the timer started. (Same for the traffic notifier.)

[HankYuLinksys]

Guard inconsistency: this path checks isAuthenticated, but the ref.listen(dashboardDomainReadyProvider) path above doesn't. Not a bug, but worth aligning. (Same for the traffic notifier.)

[HankYuLinksys]

Both points from my earlier review are addressed: _startTimerIfAuthenticated() unifies the auth guard across the listen/read paths, and the new tests cover the already-resolved startup path (plus the logged-out case). Ran both notifier test files locally — all 20 pass. LGTM 👍

[HankYuLinksys]

Looks good!