Hold the finishing banner until the daemon reconnects

[nortonandreev]

Closes #928 — AI-2759

What & why

App-lane Connected still lags the daemon's connection word after OAuth, so Sign in would restart a flow that already succeeded. The launcher holds finishing copy, hides Sign in, and shows a busy bar until the daemon reports connected, the local daemon is down, or the lane parks SignedOut. A live app lane with the daemon still catching up shows Connecting, not Sign in again.

Where to look

NoticeFor and the lane subscription that settles awaiting only on SignedOut — app-lane Connected is not "caught up".

Verification

dotnet run --project test/Capacitor.App.Tests.Unit/Capacitor.App.Tests.Unit.csproj

1767 passed, 0 failed (14s).

[qodo-code-review]

Code Review by Qodo

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

1. Another server's loss stays busy ✓ Resolved 🐞 Bug ≡ Correctness

Description

NoticeFor treats a live app lane as proof that the selected local daemon is catching up without
comparing their server URLs. When the app lane is connected to one server and the local daemon
reports disconnection from another, the launcher indefinitely shows Connecting and suppresses Sign
in for the unrelated local failure.

Code

src/Capacitor.App/ViewModels/HomeViewModel.cs[R563-564]

+        if (!remoteSelected && selectedAvailability == LaunchAvailability.ServerDisconnected && LaneIsCatchingUp(lane))
+            return ConnectingNotice;

Relevance

●● Moderate

Potential cross-server state conflation is credible, but no close precedent confirms this
topology-specific concern.

PR-#766

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Local availability is computed solely from the local daemon, but the added branch overrides its
disconnected result using the independent app lane. The daemon snapshot exposes its own ServerUrl,
the app lane is constructed from a separately resolved URL, and an existing test explicitly
documents that the local daemon and app-owned remote topology can use different servers.

src/Capacitor.App/ViewModels/HomeViewModel.cs[373-375]
src/Capacitor.App/ViewModels/HomeViewModel.cs[452-455]
src/Capacitor.App/ViewModels/HomeViewModel.cs[563-564]
src/Capacitor.Cli.Core/LocalIpc/StatusIpc.cs[21-27]
src/Capacitor.App/Services/ServerConnectionService.cs[48-58]
test/Capacitor.App.Tests.Unit/HomeViewModelTests.cs[876-879]

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 catch-up branch applies the app lane's state to every local daemon connection, although the app lane and local daemon can target different servers. A connected lane for one server therefore masks a persistent local disconnection from another server.

## Fix Focus Areas
- src/Capacitor.App/ViewModels/HomeViewModel.cs[410-430]
- src/Capacitor.App/ViewModels/HomeViewModel.cs[452-455]
- src/Capacitor.App/ViewModels/HomeViewModel.cs[549-564]
- src/Capacitor.Cli.Core/LocalIpc/StatusIpc.cs[21-27]

## Recommended Fix
Carry the daemon snapshot's `ServerUrl` and the app lane's configured server identity into the catch-up decision, and apply `LaneIsCatchingUp` only when both connections target the same normalized server. Preserve the normal disconnected notice and sign-in affordance for different-server selections, with a regression test covering that topology.

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

---

2. Failed daemon starts still show progress ✓ Resolved 🐞 Bug ≡ Correctness ⭐ New

Description

_bannerBusy derives from notices, even though BannerMessageFor can replace that notice with a
lifecycle failure message. When a connecting or finishing notice is hidden by “Daemon start did not
finish. Press Retry.”, the banner still renders an indeterminate progress bar beside a settled
failure that needs user action.

Code

src/Capacitor.App/ViewModels/HomeViewModel.cs[R478-480]

+        _bannerBusy = notices
+            .Select(BusyNotice)
+            .ToProperty(this, x => x.BannerBusy, initialValue: true)

Relevance

●●● Strong

Recent precedent accepts clearing stale progress indicators when lifecycle failure text replaces an
in-flight status.

PR-#766

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The visible banner text combines the connection notice with lifecycle status and gives any non-empty
lifecycle message precedence, but the new busy property observes only the unmodified connection
notice. The lifecycle controller emits an explicit settled failure asking the user to press Retry,
while BusyNotice marks the hidden connecting or finishing notice as busy.

src/Capacitor.App/ViewModels/HomeViewModel.cs[468-480]
src/Capacitor.App/ViewModels/HomeViewModel.cs[551-553]
src/Capacitor.App/ViewModels/HomeViewModel.cs[592-617]
src/Capacitor.App/Services/DaemonLifecycleController.cs[555-563]

Agent prompt

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

## Issue description
`BannerBusy` is calculated from the underlying connection notice even when a lifecycle message overrides the text displayed in the banner, allowing failure messages to retain an indeterminate progress bar.

## Fix Focus Areas
- src/Capacitor.App/ViewModels/HomeViewModel.cs[468-481]
- test/Capacitor.App.Tests.Unit/HomeViewModelTests.cs[619-697]

## Recommended Fix
Calculate the busy state from the effective banner presentation, including lifecycle-message precedence and machine selection. Ensure an actionable lifecycle failure suppresses the progress bar, and add a regression test covering a busy connection notice overridden by a failure message.

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

---

3. Smoke failures leak the launcher window ✓ Resolved 🐞 Bug ☼ Reliability ⭐ New

Description

After_sign_in_the_banner_shows_a_loader_until_the_daemon_reconnects closes the window and disposes
the view model only on the successful path instead of in a finally. If view lookup, binding
evaluation, or result capture throws, the process-global Avalonia session retains the shown window
and subscriptions and can contaminate later tests.

Code

test/Capacitor.App.Tests.Unit/HomeViewSmokeTests.cs[R156-158]

+            window.Close();
+            Dispatcher.UIThread.RunJobs();
+            vm.Dispose();

Relevance

●●● Strong

Recent exact precedent accepted finally-based cleanup for Avalonia smoke-test windows and
subscriptions.

PR-#858

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The test shows an Avalonia window before performing view lookups and binding reads, while cleanup
appears afterward without a finally. Past PR #858 established the same failure pattern for
headless smoke tests using the process-global Avalonia session.

test/Capacitor.App.Tests.Unit/HomeViewSmokeTests.cs[143-159]
PR-#858

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 new Avalonia smoke test cleans up its shown window and view model only after all interactions succeed, so exceptions before those statements leak process-global UI resources.

## Fix Focus Areas
- test/Capacitor.App.Tests.Unit/HomeViewSmokeTests.cs[132-159]

## Recommended Fix
Wrap the shown window and view-model lifecycle in `try`/`finally`, closing the window, draining dispatcher jobs, and disposing the view model from the `finally` block so every exit path performs cleanup.

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

---

4. Three tests bypass temp injection ✓ Resolved 📘 Rule violation ▣ Testability ⭐ New

Description

LaneSignedOutAfterSignInShowsSignInAgain, ALiveAppLaneTreatsDaemonDisconnectAsConnecting, and
After_sign_in_the_banner_shows_a_loader_until_the_daemon_reconnects create locally managed
directories with TempDir.WithPathTo instead of using an injected [TempDir] property. Each
execution therefore manages fixture state and disposal outside the test framework’s
injected-directory lifecycle across both test classes.

Code

test/Capacitor.App.Tests.Unit/HomeViewModelTests.cs[656]

+            using var tmp = TempDir.WithPathTo("app-state.json", out var path);

Evidence

Compliance rule 2808173 requires test classes that need temporary directories to use one public
required [TempDir] property rather than manually managed instances. The cited additions introduce
three local TempDir.WithPathTo scopes in two test classes that have no injected
temporary-directory property.

Rule 2808173: Use injected [TempDir] public required property in test classes instead of manual fields
test/Capacitor.App.Tests.Unit/HomeViewModelTests.cs[656-656]
test/Capacitor.App.Tests.Unit/HomeViewModelTests.cs[681-681]
test/Capacitor.App.Tests.Unit/HomeViewSmokeTests.cs[134-134]

Agent prompt

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

## Issue description
Three newly added tests manually create and dispose `TempDir` instances instead of obtaining temporary state through the test framework’s `[TempDir]` injection mechanism.

## Fix Focus Areas
- test/Capacitor.App.Tests.Unit/HomeViewModelTests.cs[656-681]
- test/Capacitor.App.Tests.Unit/HomeViewSmokeTests.cs[130-164]

## Recommended Fix
Add a single `[TempDir] public required TempDir Tmp { get; init; }` property to each affected test class, replace the local `TempDir.WithPathTo` calls with paths created from the injected `Tmp`, and remove manual disposal of those directories.

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

---

View medium (2)

5. Remote retry asks for hidden sign-in ✓ Resolved 🐞 Bug ≡ Correctness

Description

SignInVisible suppresses the button for Retrying, while RemoteAvailabilityFor still maps that
state to ServerDisconnected and NoticeFor displays the sign-in-again message. When the server
lane enters its normal retry path with a remote machine selected, the banner requests an action the
user cannot perform until another lane transition occurs.

Code

src/Capacitor.App/ViewModels/HomeViewModel.cs[R453-454]

+            .Select(t => !t.Awaiting && (t.Expired || (
+                t.Availability == LaunchAvailability.ServerDisconnected && !LaneIsCatchingUp(t.Lane))))

Relevance

●●● Strong

Accepted precedents flag contradictory user-facing status and action states during reconnect flows.

PR-#766
PR-#790

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new visibility condition classifies Retrying as catch-up and hides Sign in, but the existing
remote availability and notice mappings classify the same state as disconnected and request Sign in.
The server connection service publishes Retrying during ordinary reconnects and failures, so this
contradictory state is reachable.

src/Capacitor.App/ViewModels/HomeViewModel.cs[452-455]
src/Capacitor.App/ViewModels/HomeViewModel.cs[566-570]
src/Capacitor.App/ViewModels/HomeViewModel.cs[581-582]
src/Capacitor.App/ViewModels/HomeViewModel.cs[795-798]
src/Capacitor.App/Services/ServerConnectionService.cs[169-181]

Agent prompt

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

## Issue description
A remote selection in `ServerLaneState.Retrying` shows `ServerLostNotice`, which asks the user to sign in, while the new sign-in visibility condition hides the button.

## Fix Focus Areas
- src/Capacitor.App/ViewModels/HomeViewModel.cs[452-455]
- src/Capacitor.App/ViewModels/HomeViewModel.cs[566-570]
- src/Capacitor.App/ViewModels/HomeViewModel.cs[795-798]

## Recommended Fix
Treat a remote lane in `Retrying` as pending/reconnecting, consistently with `Connecting`, so the banner shows `ConnectingNotice` and its busy indicator rather than requesting a hidden sign-in. Add a regression test covering a remote selection while the lane transitions to `Retrying`.

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

---

6. Merged commit loses issue linkage ✗ Dismissed 📘 Rule violation ⚙ Maintainability

Description

Hold the finishing banner until the daemon reconnects does not end with the required (#<digits>)
GitHub issue reference. When this commit is merged or squashed using that subject, the resulting
history lacks the mandated explicit issue linkage despite the PR description naming #928.

Code

src/Capacitor.App/ViewModels/HomeViewModel.cs[165]

+    readonly ObservableAsPropertyHelper<bool> _bannerBusy;

Relevance

●●● Strong

Explicit repository rule mandates issue suffixes; commit subject clearly violates it.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 2897961 requires every commit subject to end with a parenthesized GitHub issue
reference, while the supplied commit subject has no such suffix.

Rule 2897961: Enforce single-clause imperative commit subject with GitHub issue reference and 80-char limit

---

Context sources

✅ Compliance rules (platform): 64 rules

✅ Cross-repo context — repo relationships

Review mode: ⚖️ Balanced: This changes reactive connection-state logic and user-facing sign-in behavior across view model, UI, and tests, including a known pending server-mismatch concern; it warrants a complete single-pass review.

Tip of the day

💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit 419e9d8

Results up to commit a7dd0e5 ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Another server's loss stays busy ✓ Resolved 🐞 Bug ≡ Correctness

Description

NoticeFor treats a live app lane as proof that the selected local daemon is catching up without
comparing their server URLs. When the app lane is connected to one server and the local daemon
reports disconnection from another, the launcher indefinitely shows Connecting and suppresses Sign
in for the unrelated local failure.

Code

src/Capacitor.App/ViewModels/HomeViewModel.cs[R563-564]

+        if (!remoteSelected && selectedAvailability == LaunchAvailability.ServerDisconnected && LaneIsCatchingUp(lane))
+            return ConnectingNotice;

Relevance

●● Moderate

Potential cross-server state conflation is credible, but no close precedent confirms this
topology-specific concern.

PR-#766

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Local availability is computed solely from the local daemon, but the added branch overrides its
disconnected result using the independent app lane. The daemon snapshot exposes its own ServerUrl,
the app lane is constructed from a separately resolved URL, and an existing test explicitly
documents that the local daemon and app-owned remote topology can use different servers.

src/Capacitor.App/ViewModels/HomeViewModel.cs[373-375]
src/Capacitor.App/ViewModels/HomeViewModel.cs[452-455]
src/Capacitor.App/ViewModels/HomeViewModel.cs[563-564]
src/Capacitor.Cli.Core/LocalIpc/StatusIpc.cs[21-27]
src/Capacitor.App/Services/ServerConnectionService.cs[48-58]
test/Capacitor.App.Tests.Unit/HomeViewModelTests.cs[876-879]

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 catch-up branch applies the app lane's state to every local daemon connection, although the app lane and local daemon can target different servers. A connected lane for one server therefore masks a persistent local disconnection from another server.

## Fix Focus Areas
- src/Capacitor.App/ViewModels/HomeViewModel.cs[410-430]
- src/Capacitor.App/ViewModels/HomeViewModel.cs[452-455]
- src/Capacitor.App/ViewModels/HomeViewModel.cs[549-564]
- src/Capacitor.Cli.Core/LocalIpc/StatusIpc.cs[21-27]

## Recommended Fix
Carry the daemon snapshot's `ServerUrl` and the app lane's configured server identity into the catch-up decision, and apply `LaneIsCatchingUp` only when both connections target the same normalized server. Preserve the normal disconnected notice and sign-in affordance for different-server selections, with a regression test covering that topology.

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

---

2. Remote retry asks for hidden sign-in ✓ Resolved 🐞 Bug ≡ Correctness

Description

SignInVisible suppresses the button for Retrying, while RemoteAvailabilityFor still maps that
state to ServerDisconnected and NoticeFor displays the sign-in-again message. When the server
lane enters its normal retry path with a remote machine selected, the banner requests an action the
user cannot perform until another lane transition occurs.

Code

src/Capacitor.App/ViewModels/HomeViewModel.cs[R453-454]

+            .Select(t => !t.Awaiting && (t.Expired || (
+                t.Availability == LaunchAvailability.ServerDisconnected && !LaneIsCatchingUp(t.Lane))))

Relevance

●●● Strong

Accepted precedents flag contradictory user-facing status and action states during reconnect flows.

PR-#766
PR-#790

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new visibility condition classifies Retrying as catch-up and hides Sign in, but the existing
remote availability and notice mappings classify the same state as disconnected and request Sign in.
The server connection service publishes Retrying during ordinary reconnects and failures, so this
contradictory state is reachable.

src/Capacitor.App/ViewModels/HomeViewModel.cs[452-455]
src/Capacitor.App/ViewModels/HomeViewModel.cs[566-570]
src/Capacitor.App/ViewModels/HomeViewModel.cs[581-582]
src/Capacitor.App/ViewModels/HomeViewModel.cs[795-798]
src/Capacitor.App/Services/ServerConnectionService.cs[169-181]

Agent prompt

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

## Issue description
A remote selection in `ServerLaneState.Retrying` shows `ServerLostNotice`, which asks the user to sign in, while the new sign-in visibility condition hides the button.

## Fix Focus Areas
- src/Capacitor.App/ViewModels/HomeViewModel.cs[452-455]
- src/Capacitor.App/ViewModels/HomeViewModel.cs[566-570]
- src/Capacitor.App/ViewModels/HomeViewModel.cs[795-798]

## Recommended Fix
Treat a remote lane in `Retrying` as pending/reconnecting, consistently with `Connecting`, so the banner shows `ConnectingNotice` and its busy indicator rather than requesting a hidden sign-in. Add a regression test covering a remote selection while the lane transitions to `Retrying`.

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

---

3. Merged commit loses issue linkage ✗ Dismissed 📘 Rule violation ⚙ Maintainability

Description

Hold the finishing banner until the daemon reconnects does not end with the required (#<digits>)
GitHub issue reference. When this commit is merged or squashed using that subject, the resulting
history lacks the mandated explicit issue linkage despite the PR description naming #928.

Code

src/Capacitor.App/ViewModels/HomeViewModel.cs[165]

+    readonly ObservableAsPropertyHelper<bool> _bannerBusy;

Relevance

●●● Strong

Explicit repository rule mandates issue suffixes; commit subject clearly violates it.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 2897961 requires every commit subject to end with a parenthesized GitHub issue
reference, while the supplied commit subject has no such suffix.

Rule 2897961: Enforce single-clause imperative commit subject with GitHub issue reference and 80-char limit

---

[linear-code]

AI-2759

[qodo-code-review]

PR Summary by Qodo

Hold sign-in banner until the daemon reconnects

🐞 Bug fix 🧪 Tests ✨ Enhancement 🕐 20-40 Minutes

AI Description

• Keeps post-OAuth finishing state active until daemon availability confirms reconnection.
• Treats live lane retries as connecting and suppresses redundant Sign in actions.
• Adds banner progress feedback and coverage for catch-up, failure, and remote states.

Diagram

graph TD
  A["OAuth Success"] --> B["Awaiting Daemon"] --> E{"Notice State"} -->|busy notice| F["Launcher Banner"]
  C["Lane Status"] --> E
  D["Daemon Status"] --> E
  E -->|auth failed| G["Sign-in Action"]

Loading

High-Level Assessment

The reactive derivation from authoritative lane and daemon streams is the appropriate approach. Clearing on lane Connected would preserve the original race, while a timeout would be nondeterministic; waiting for daemon readiness or an explicit SignedOut outcome accurately models completion and failure.

Files changed (4) +148 / -24

Enhancement (1) +3 / -0

LauncherPaneView.axamlShow progress while the connection banner is busy +3/-0

Show progress while the connection banner is busy

• Adds an indeterminate progress bar to the launcher connection banner. Its visibility follows the new BannerBusy view-model property.

src/Capacitor.App/Views/LauncherPaneView.axaml

Bug fix (1) +36 / -13

HomeViewModel.csModel daemon catch-up as an in-progress sign-in state +36/-13

Model daemon catch-up as an in-progress sign-in state

• Includes server-lane state when deriving notices and Sign in visibility. Connected and Retrying lanes remain catch-up states until daemon availability recovers, while SignedOut explicitly ends the wait and restores authentication controls. It also exposes whether the current notice should display busy feedback.

src/Capacitor.App/ViewModels/HomeViewModel.cs

Tests (2) +109 / -11

HomeViewModelTests.csCover lane and daemon catch-up state transitions +67/-8

Cover lane and daemon catch-up state transitions

• Verifies that Connected and Retrying lanes retain connecting or finishing feedback while the daemon catches up, without exposing Sign in. Adds coverage for SignedOut recovery, busy-state behavior, and remote retry availability.

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

HomeViewSmokeTests.csVerify launcher banner loader and Sign in visibility +42/-3

Verify launcher banner loader and Sign in visibility

• Extends launcher smoke coverage to inspect the progress bar. Confirms ordinary disconnects remain actionable while post-sign-in daemon catch-up displays finishing text and busy feedback without Sign in.

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

[qodo-code-review]

Code review by qodo was updated up to the latest commit ab69bb2