Expire the connecting banner when the daemon never catches up

[nortonandreev]

AI-2877 — no GitHub issue

What & why

A live app lane plus a daemon that never reaches connected kept the launcher on "Connecting to the server…" with Sign in hidden: catch-up after auth looks identical to a daemon that 401-retries forever. After 60s (longer than one 30s connect backoff) that line becomes the lost-session notice and Sign in is offered. A daemon that connects inside the bound still clears the banner.

Where to look

HomeViewModel catch-up timer and NoticeAfterCatchUp. The pre-bound catch-up tests must still hide Sign in.

Verification

dotnet run --project test/Capacitor.App.Tests.Unit/Capacitor.App.Tests.Unit.csproj -- --treenode-filter "/*/*/HomeViewModelTests/*"

82 passed, including four new catch-up-bound cases.

[linear-code]

AI-2877

[qodo-code-review]

PR Summary by Qodo

Expire stalled daemon catch-up banners after 60 seconds

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Bound connecting and finishing-sign-in banners to a 60-second catch-up window.
• Convert expired catch-up notices to lost-session state and restore Sign in.
• Cover timeout and successful recovery paths with deterministic fake-time tests.

Diagram

graph TD
  I["Connection state"] --> N["Raw notice"] --> B{"Busy notice?"}
  B -->|Yes| T["60s timer"] --> O["Lost-session state"] --> U["Banner and sign-in"]
  B -->|No, reset| U

Loading

High-Level Assessment

The PR’s injectable TimeProvider/ITimer approach is appropriate because it provides explicit lifecycle control and deterministic tests while preserving the existing reactive notice pipeline. An Rx timer operator was considered, but it would add subscription-switching complexity without improving behavior.

Files changed (2) +159 / -2

Bug fix (1) +50 / -2

HomeViewModel.csBound daemon catch-up notices with a 60-second timeout +50/-2

Bound daemon catch-up notices with a 60-second timeout

• Adds a one-shot catch-up timer and reactive timeout state for connecting and finishing-sign-in notices. When catch-up exceeds 60 seconds, the view model displays the lost-session notice, stops showing a busy banner, and restores Sign in; successful recovery resets the timer and state.

src/Capacitor.App/ViewModels/HomeViewModel.cs

Tests (1) +109 / -0

HomeViewModelTests.csTest catch-up expiration and timely daemon recovery +109/-0

Test catch-up expiration and timely daemon recovery

• Adds FakeTimeProvider coverage for disconnected and connecting daemons, post-auth catch-up expiration, and successful connection before the deadline. Assertions verify the resulting notice, busy state, and Sign in visibility.

test/Capacitor.App.Tests.Unit/HomeViewModelTests.cs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. A test comment repeats its assertions ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The XML comment above ALiveAppLaneStopsTreatingDaemonDisconnectAsConnectingAfterTheCatchUpBound
merely summarizes the timeout, notice, and sign-in behavior asserted immediately below. Because the
method name and assertions already express that behavior, the comment adds no non-obvious constraint
and creates another description that later changes must keep synchronized.

Code

test/Capacitor.App.Tests.Unit/HomeViewModelTests.cs[R718-719]

+    /// A live app lane plus a daemon that never reaches "connected" must leave Connecting
+    /// after CatchUpLimit and offer Sign in.

Relevance

●●● Strong

Recent test-comment cleanup findings were accepted when comments merely repeated method names and
assertions.

PR-#834
PR-#831

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 2762993 permits comments only for non-obvious, behavior-critical constraints, while
the added comment at lines 718-719 duplicates the test's explicit scenario and expected result.

Rule 2762993: Restrict comments to documenting non-obvious, behavior‑critical constraints
test/Capacitor.App.Tests.Unit/HomeViewModelTests.cs[718-719]

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 XML comment above the catch-up timeout test only restates behavior already conveyed by the test name and assertions.

## Fix Focus Areas
- test/Capacitor.App.Tests.Unit/HomeViewModelTests.cs[718-719]

## Recommended Fix
Remove the two-line XML comment while leaving the test and its attributes unchanged.

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

---

2. Renewed catch-up can expire immediately ✓ Resolved 🐞 Bug ☼ Reliability

Description

OnCatchUpElapsed posts an action that validates only the shared _catchUpArmed flag, while
RestartCatchUp reuses that flag and timer without identifying the arm that fired. If an elapsed
callback is queued and sign-in or a notice transition restarts catch-up before it runs, the stale
action passes the check and turns the renewed connecting banner into a lost-session prompt before
its 60-second window.

Code

src/Capacitor.App/ViewModels/HomeViewModel.cs[R731-734]

+        RxSchedulers.MainThreadScheduler.Schedule(() => {
+            if (_disposed || !_catchUpArmed) return;
+            _awaitingServerAfterSignIn.OnNext(false);
+            _catchUpTimedOut.OnNext(true);

Relevance

●●● Strong

Recent HomeViewModel race findings were accepted; stale timer generations are a clear reliability
defect.

PR-#929
PR-#730

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

RestartCatchUp leaves the shared armed flag true and reuses the same timer, while the timer
callback crosses an asynchronous scheduling boundary and later checks only that flag. Consequently,
an old callback cannot be distinguished from the currently armed interval; the immediate scheduler
used by the new tests also removes this production queueing window.

src/Capacitor.App/ViewModels/HomeViewModel.cs[717-727]
src/Capacitor.App/ViewModels/HomeViewModel.cs[730-735]
src/Capacitor.App/ViewModels/HomeViewModel.cs[601-606]
test/Capacitor.App.Tests.Unit/HomeViewModelTests.cs[723-738]

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 elapsed catch-up callback can remain queued while the timer is restarted. Because the queued action checks only `_catchUpArmed`, it can expire the newly armed interval prematurely.

## Fix Focus Areas
- src/Capacitor.App/ViewModels/HomeViewModel.cs[717-735]

## Recommended Fix
Associate each timer arm with a deadline or generation that stale callbacks cannot satisfy. On the main-thread action, verify that the callback still belongs to the current arm and that its deadline has actually elapsed before setting `_catchUpTimedOut`; invalidate that identity whenever catch-up is stopped or restarted.

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

---

Context sources

✅ Compliance rules (platform): 64 rules

✅ Cross-repo context — repo relationships

Review mode: ⚖️ Balanced: This is a localized runtime timeout change affecting reactive state, timers, sign-in visibility, and daemon connection behavior, so it carries meaningful behavioral risk but not enough independent complexity to warrant extended review.

Tip of the day

💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[nortonandreev]

Addressed both Qodo findings:

1. Redundant test comment — Removed the XML summary above ALiveAppLaneStopsTreatingDaemonDisconnectAsConnectingAfterTheCatchUpBound; the name and assertions already pin the behavior.

2. Stale catch-up callback — Agreed. OnCatchUpElapsed schedules onto the main thread, so a stop/restart (e.g. NotifySignInCompleted) could run before a queued expiry and the old handler only checked _catchUpArmed. Catch-up now bumps a generation on stop/restart; the scheduled action applies timeout only when that generation still matches and GetUtcNow() is past the deadline recorded for the current arm.

Capacitor.App.Tests.Unit (2540 tests) passes locally.