Skip to content

Show in the desktop app when the daemon has a restart queued - #831

Merged
alexeyzimarev merged 2 commits into
mainfrom
daemon-restart-pending-indicator
Sep 9, 2026
Merged

alexeyzimarev merged 2 commits into
mainfrom
daemon-restart-pending-indicator

Conversation

@alexeyzimarev

Copy link
Copy Markdown
Member

Closes #830 — AI-2604

What & why

A busy daemon keeps running the old binary after a CLI update until it is idle, and the app showed nothing about it. The session rail's daemon indicator and the tray header now carry "update pending" while the daemon's restart-pending marker exists and the app is attached to that daemon; the rail tooltip says the restart happens once no agents are running. Passive only: no button and no forced restart, which would take the running agents down.

Where to look

DaemonRestartPendingWatcher reads the marker file the daemon writes and its successor deletes, the same file kcap daemon status reads, on every attach transition and on a 15-second poll matching the daemon's own binary poll. Both view models gate the indicator on being attached, so a marker left by an unreachable daemon shows nothing.

Verification

Capacitor.App.Tests.Unit: 1616 passed, 0 failed. The eight new tests cover the marker present at start, written later and seen on the poll, cleared on an attach transition, no repeat emissions, and both surfaces showing and hiding the indicator with attach state.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 9, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-09T10:49:31.369460Z c5ff877 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Show queued daemon restarts in the desktop app

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Polls the daemon restart marker on startup, attach transitions, and every 15 seconds.
• Shows update-pending status in the session rail and tray only while attached.
• Adds focused watcher and view-model tests plus change documentation.
Diagram

graph TD
    D["Restart Coordinator"] -->|writes| M[("Pending Marker")] -->|read| W["Marker Watcher"] -->|pending state| MW["Main Window VM"] -->|binds| R["Session Rail"]
    S["Attach Status"] -->|refresh trigger| W
    W -->|pending state| T["Tray VM"] -->|builds header| H["Tray Header"]
    S -->|connection gate| MW
    S -->|connection gate| T
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Expose pending state over the status socket
  • ➕ Avoids periodic file reads
  • ➕ Provides daemon-authoritative state through the existing connection
  • ➖ Requires a daemon protocol and compatibility change
  • ➖ Older daemons would not expose the new field
  • ➖ Broadens scope across daemon, CLI, and desktop components
2. Use filesystem change notifications
  • ➕ Can surface marker changes immediately
  • ➕ Avoids fixed-interval polling during steady state
  • ➖ Adds platform-specific watcher behavior and failure modes
  • ➖ Still requires startup and recovery reads
  • ➖ Offers little benefit over the daemon-aligned 15-second interval

Recommendation: Keep the PR's marker-based polling approach. The marker is already the authoritative cross-version signal, while immediate and attach-triggered reads limit stale state; connection gating prevents unreachable-daemon markers from misleading users. A status-socket field is worth considering only during a future protocol revision.

Files changed (9) +321 / -13

Enhancement (5) +129 / -13
App.axaml.csWire restart-pending state through the application graph +13/-6

Wire restart-pending state through the application graph

• Creates and starts the restart marker watcher, passes its observable to the main-window and tray view models, and includes it in startup-failure and shutdown disposal paths.

src/Capacitor.App/App.axaml.cs

DaemonRestartPendingWatcher.csAdd a daemon restart marker watcher +61/-0

Add a daemon restart marker watcher

• Introduces a replaying watcher that reads the marker immediately, after attach transitions, and every 15 seconds. Serializes subject emissions and suppresses duplicate pending-state notifications.

src/Capacitor.App/Services/DaemonRestartPendingWatcher.cs

MainWindowViewModel.csExpose attached restart-pending state to the session rail +28/-1

Expose attached restart-pending state to the session rail

• Combines restart-pending state with daemon connection status. Exposes indicator visibility and explanatory tooltip text only while attached.

src/Capacitor.App/ViewModels/MainWindowViewModel.cs

TrayViewModel.csAppend update-pending state to the tray header +20/-6

Append update-pending state to the tray header

• Combines the marker signal with attachment status and adds an update-pending suffix to connected tray headers. Existing tray state and behavior remain unchanged.

src/Capacitor.App/ViewModels/TrayViewModel.cs

SessionRailView.axamlRender the session rail update-pending indicator +7/-0

Render the session rail update-pending indicator

• Adds an update-pending label beside daemon identity details and displays the restart timing explanation in the rail tooltip.

src/Capacitor.App/Views/SessionRailView.axaml

Tests (3) +179 / -0
DaemonRestartPendingWatcherTests.csTest restart marker observation and deduplication +98/-0

Test restart marker observation and deduplication

• Covers markers present at startup, markers discovered by polling, clearing on attach transitions, and suppression of unchanged emissions using fake time.

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

MainWindowViewModelTests.csTest session rail pending-state gating +44/-0

Test session rail pending-state gating

• Verifies that indicator state and tooltip text follow marker changes while connected and remain hidden when the daemon is unreachable.

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

TrayViewModelTests.csTest tray header pending-state gating +37/-0

Test tray header pending-state gating

• Verifies that connected tray headers gain and lose the update-pending suffix while unreachable daemon headers remain unchanged.

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

Documentation (1) +13 / -0
CHANGES.mdDocument the desktop restart-pending indicator +13/-0

Document the desktop restart-pending indicator

• Explains why busy daemons defer restarts, where pending state appears, and why the feature remains passive. Documents the marker-based polling and attachment gating behavior.

docs/CHANGES.md

@qodo-code-review

qodo-code-review Bot commented Sep 9, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Remediation recommended

1. Cleared updates can reappear briefly ✓ Resolved 🐞 Bug ≡ Correctness
Description
Refresh reads the marker before acquiring _lock, so concurrent poll and attach callbacks can
publish their filesystem observations in reverse order. When one callback reads the old marker
before its deletion but publishes after another callback reports it absent, both indicators show the
cleared update again until a later refresh.
Code

src/Capacitor.App/Services/DaemonRestartPendingWatcher.cs[R45-46]

+        var pending = DaemonRestartMarker.TryRead(_store, _daemonName) is not null;
+        lock (_lock) _pending.OnNext(pending);
Relevance

●●● Strong

Recent precedent accepts fixes for concurrent observer publication races and unsafe multi-producer
Subjects.

PR-#474

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The attach subscription and polling task both call Refresh, while Refresh performs TryRead
before entering the lock. The daemon independently deletes the marker during successor startup,
making an old read followed by a newer read and reversed publication possible.

src/Capacitor.App/Services/DaemonRestartPendingWatcher.cs[36-53]
src/Capacitor.Cli.Daemon/Services/RestartCoordinator.cs[154-163]
src/Capacitor.App/ViewModels/MainWindowViewModel.cs[353-365]

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

## Issue description
Concurrent `Refresh` calls can publish marker states in a different order from their reads, allowing a cleared restart marker to reappear temporarily.

## Issue Context
The attach stream and polling task invoke `Refresh` independently. The existing lock protects only `OnNext`, leaving the filesystem read outside the serialized operation; move the read inside the same synchronization boundary and add a concurrency regression test.

## Fix Focus Areas
- src/Capacitor.App/Services/DaemonRestartPendingWatcher.cs[42-47]
- test/Capacitor.App.Tests.Unit/DaemonRestartPendingWatcherTests.cs[76-97]

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


2. Startup failures leave polling alive ✓ Resolved 🐞 Bug ☼ Reliability
Description
Dispose removes only the status subscription, while the untracked PollAsync task continues until
the externally owned lifetime token is cancelled. If graph construction throws after the watcher
starts but before _service is assigned, startup cleanup disposes the watcher without cancelling
that token, leaving it polling throughout the error-window lifetime.
Code

src/Capacitor.App/Services/DaemonRestartPendingWatcher.cs[60]

+    public void Dispose() => _subscription?.Dispose();
Relevance

●●● Strong

Recent precedent accepts fixes for abandoned background tasks whose exceptions or lifetime remain
unmanaged.

PR-#446

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The watcher starts PollAsync without retaining its task, and disposal does not cancel the token
that controls its loop. App construction starts the watcher at line 400 but does not assign
_service until line 441, while startup-failure cleanup cancels the shared shutdown token only when
_service is non-null before disposing UI services.

src/Capacitor.App/Services/DaemonRestartPendingWatcher.cs[36-60]
src/Capacitor.App/App.axaml.cs[398-441]
src/Capacitor.App/App.axaml.cs[844-882]

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

## Issue description
Disposing `DaemonRestartPendingWatcher` does not stop its polling task, allowing startup-failure cleanup to leave background polling alive.

## Issue Context
Give the watcher an owned cancellation source linked to the application lifetime, track its polling task, and ensure disposal cancels the poll as well as removing the attach subscription. Add coverage that disposes the watcher without cancelling the supplied lifetime token and verifies that no further polls occur.

## Fix Focus Areas
- src/Capacitor.App/Services/DaemonRestartPendingWatcher.cs[18-60]
- test/Capacitor.App.Tests.Unit/DaemonRestartPendingWatcherTests.cs[26-50]

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


3. Two test headings repeat test names 📘 Rule violation ⚙ Maintainability
Description
MainWindowViewModelTests and TrayViewModelTests add identical section-banner comments that
merely restate the subject of the immediately following restart-pending tests. Because the test
names already describe that behavior, the banners record no non-obvious constraint or rationale and
add decoration a later maintainer must keep aligned.
Code

test/Capacitor.App.Tests.Unit/MainWindowViewModelTests.cs[66]

+    // ---- daemon restart pending (the daemon's own queued restart-after-update) ----
Relevance

●●● Strong

Recent test precedent accepts removing comments that merely restate obvious assertions or test
behavior.

PR-#703

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Compliance rule 2762993 requires comments to document non-obvious, behavior-critical constraints
rather than restating clear code. Both added section banners only label restart-pending tests whose
method names already state that subject.

Rule 2762993: Restrict comments to documenting non-obvious, behavior‑critical constraints
test/Capacitor.App.Tests.Unit/MainWindowViewModelTests.cs[66-66]
test/Capacitor.App.Tests.Unit/TrayViewModelTests.cs[42-42]

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

## Issue description
Remove the decorative restart-pending section comments because the adjacent test names already communicate their subject.

## Issue Context
The comments document no non-obvious constraint, invariant, trade-off, or rationale and are duplicated across two test files.

## Fix Focus Areas
- test/Capacitor.App.Tests.Unit/MainWindowViewModelTests.cs[66-66]
- test/Capacitor.App.Tests.Unit/TrayViewModelTests.cs[42-42]

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


Grey Divider

Context sources
✅ Compliance rules (platform): 61 rules
✅ Cross-repo context — repo relationships
Review mode: ⚖️ Balanced: This introduces runtime watcher, lifecycle/disposal, reactive state, and two UI-surface integrations across several files, creating genuine behavioral and concurrency risks, but not enough independent logic density to warrant extended review.

Grey Divider

Tip of the day
💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

});
}

// ---- daemon restart pending (the daemon's own queued restart-after-update) ----

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. Two test headings repeat test names 📘 Rule violation ⚙ Maintainability

MainWindowViewModelTests and TrayViewModelTests add identical section-banner comments that
merely restate the subject of the immediately following restart-pending tests. Because the test
names already describe that behavior, the banners record no non-obvious constraint or rationale and
add decoration a later maintainer must keep aligned.
Agent Prompt
## Issue description
Remove the decorative restart-pending section comments because the adjacent test names already communicate their subject.

## Issue Context
The comments document no non-obvious constraint, invariant, trade-off, or rationale and are duplicated across two test files.

## Fix Focus Areas
- test/Capacitor.App.Tests.Unit/MainWindowViewModelTests.cs[66-66]
- test/Capacitor.App.Tests.Unit/TrayViewModelTests.cs[42-42]

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed both section banners in 016cf40.

Comment thread src/Capacitor.App/Services/DaemonRestartPendingWatcher.cs Outdated
Comment thread src/Capacitor.App/Services/DaemonRestartPendingWatcher.cs Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c5ff877782

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +355 to +357
var pendingWhileConnected = status
.CombineLatest(restartPending ?? Observable.Return(false), (st, pending) => pending && st.State == AttachState.Connected)
.DistinctUntilChanged();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Marshal restart-pending updates to the UI scheduler

When the marker changes during the 15-second poll, DaemonRestartPendingWatcher.PollAsync publishes from a thread-pool continuation, and CombineLatest consequently updates both bound OAPH properties on that background thread. Unlike the existing status source, restartPending is not passed through ObserveOn(RxSchedulers.MainThreadScheduler), so detecting or clearing an update while the window is active can raise Avalonia-bound property changes off the UI thread. Marshal this stream before creating pendingWhileConnected.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 016cf40: the restart-pending stream is observed on RxSchedulers.MainThreadScheduler before the CombineLatest, the same way status and snapshots are. The tray's pipeline already marshalled its final model, so only the main window needed it.

alexeyzimarev and others added 2 commits September 9, 2026 12:55
The marker file is the only signal: nothing about a queued restart travels over the status socket. No forced restart, it would take the running agents down.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The marker read moves under the lock so two callers cannot publish their observations in the reverse order of their reads.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@alexeyzimarev
alexeyzimarev force-pushed the daemon-restart-pending-indicator branch from c5ff877 to 016cf40 Compare September 9, 2026 10:58
@alexeyzimarev
alexeyzimarev merged commit 55faaa0 into main Sep 9, 2026
7 checks passed
@alexeyzimarev
alexeyzimarev deleted the daemon-restart-pending-indicator branch September 9, 2026 12:07
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.

Show in the desktop app when the daemon has a restart-after-update queued

1 participant