(feat) restart UI process after automatic update#2155
Conversation
Separate variable declaration from assignment in the SHA256 validation logic to prevent variable shadowing and ensure proper error handling scope.
…on relaunch failure This fixes Linux-related issue when UI process do not start automatically after upgrade. - replace direct current_exe relaunch usage with verified launch program resolution - consider both current_exe and argv0, but only accept verified launchable file paths - fail relaunch with explicit error when no safe executable path is available - in reconnect flow, exit current UI only if relaunch spawn succeeds - if relaunch request fails, keep current UI process running and continue normal startup https://github.com/safing/portmaster-shadow/issues/40
|
Alexandr Stelnykovych seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
📝 WalkthroughWalkthroughThis pull request implements a UI relaunch mechanism triggered during service upgrades. When an upgrade requires a restart, the service sends a Changes
Sequence DiagramsequenceDiagram
participant UpdateService as Upgrade Service
participant CoreAPI as Core API
participant EventSys as Event System
participant TrayUI as UI (Tray Handler)
participant PortmasterIF as PortmasterInterface
participant WSHandler as WebSocket Handler
participant Relaunch as Relaunch Process
UpdateService->>CoreAPI: POST /core/restart?source=upgrade
CoreAPI->>EventSys: pushModuleEvent("core", "restart-ui-process")
EventSys->>TrayUI: emit restart-ui-process event
TrayUI->>PortmasterIF: mark_restart_ui_proc_requested()
PortmasterIF->>PortmasterIF: pending_restart = true
Note over TrayUI,PortmasterIF: UI continues, eventually reconnects
WSHandler->>PortmasterIF: consume_restart_ui_proc_requested()
PortmasterIF-->>WSHandler: true (clears flag)
WSHandler->>Relaunch: request_ui_relaunch()
Relaunch->>Relaunch: Spawn helper with PORTMASTER_UI_RELAUNCH_HELPER=1
Relaunch-->>WSHandler: Ok
WSHandler->>WSHandler: exit(0)
Relaunch->>Relaunch: Retry helper with current exe + filtered args
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
service/updates/index.go (1)
385-387:⚠️ Potential issue | 🟡 MinorIncorrect error wrapping:
erris nil here.At line 386,
erris guaranteed to be nil because the check at line 382 would have returned early otherwise. Wrapping a nil error with%wis semantically incorrect—the size validation failure is unrelated to the hex decoding result.Note: This same bug exists at lines 332 and 362 in
CheckSHA256SumFileandCheckSHA256Sum.🐛 Proposed fix
if len(expectedDigest) != sha256.Size { - return fmt.Errorf("invalid size for expected hash %s: %w", sha256sum, err) + return fmt.Errorf("invalid size for expected hash %s (got %d bytes, expected %d)", sha256sum, len(expectedDigest), sha256.Size) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/updates/index.go` around lines 385 - 387, The error formatting wraps a nil err (using %w) when validating expectedDigest length in functions CheckSHA256SumFile and CheckSHA256Sum (code around the if checking len(expectedDigest) != sha256.Size), which is incorrect; change the fmt.Errorf call to not wrap err and instead return a clear error that states the expected and actual sizes (e.g., use fmt.Errorf("invalid size for expected hash %s: expected %d, got %d", sha256sum, sha256.Size, len(expectedDigest))) so the message is accurate and does not reference a nil error.service/updates/module.go (1)
436-438:⚠️ Potential issue | 🟠 MajorAuto-restart still bypasses the new UI relaunch flow.
This branch calls
u.instance.Restart()directly, so it never reaches thesource=upgradehandling inservice/core/api.gothat emitsrestart-ui-process. Manual “Restart Now” works, but automatic/no-notify upgrades will still restart the core without telling the Tauri UI to relaunch. Route this branch through the same upgrade-aware restart helper, or emit the UI-restart event before callingRestart().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/updates/module.go` around lines 436 - 438, This branch directly calls u.instance.Restart(), bypassing the upgrade-aware flow that emits the restart-ui-process event in service/core/api.go; change this to use the same upgrade-aware restart helper (the function used by the manual “Restart Now” path) or explicitly emit the restart-ui-process event before calling u.instance.Restart() so the Tauri UI receives the source=upgrade relaunch signal; locate references to u.instance.Restart() in module.go and replace the direct call with the helper invocation (or add the event emission) so the automatic/no-notify upgrade path follows the same upgrade-aware behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@desktop/tauri/src-tauri/src/main.rs`:
- Around line 67-78: The restart-request flag is being cleared by
consume_restart_ui_proc_requested() before calling
relaunch::request_ui_relaunch(), so on Err the request is lost; fix by either
re-marking the flag in the Err branch via the portmaster API (i.e. call the
inverse setter on self.handle.portmaster() to re-set the restart-ui-process
request) or change the flow to check the flag without consuming it and only call
consume_restart_ui_proc_requested() after relaunch::request_ui_relaunch()
returns Ok; keep existing logging and exit behavior (self.handle.exit(0)) on
success.
---
Outside diff comments:
In `@service/updates/index.go`:
- Around line 385-387: The error formatting wraps a nil err (using %w) when
validating expectedDigest length in functions CheckSHA256SumFile and
CheckSHA256Sum (code around the if checking len(expectedDigest) != sha256.Size),
which is incorrect; change the fmt.Errorf call to not wrap err and instead
return a clear error that states the expected and actual sizes (e.g., use
fmt.Errorf("invalid size for expected hash %s: expected %d, got %d", sha256sum,
sha256.Size, len(expectedDigest))) so the message is accurate and does not
reference a nil error.
In `@service/updates/module.go`:
- Around line 436-438: This branch directly calls u.instance.Restart(),
bypassing the upgrade-aware flow that emits the restart-ui-process event in
service/core/api.go; change this to use the same upgrade-aware restart helper
(the function used by the manual “Restart Now” path) or explicitly emit the
restart-ui-process event before calling u.instance.Restart() so the Tauri UI
receives the source=upgrade relaunch signal; locate references to
u.instance.Restart() in module.go and replace the direct call with the helper
invocation (or add the event emission) so the automatic/no-notify upgrade path
follows the same upgrade-aware behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 42dd42ec-2b5a-4b72-a09b-eddd31092038
📒 Files selected for processing (7)
desktop/tauri/src-tauri/src/main.rsdesktop/tauri/src-tauri/src/portmaster/mod.rsdesktop/tauri/src-tauri/src/relaunch.rsdesktop/tauri/src-tauri/src/traymenu.rsservice/core/api.goservice/updates/index.goservice/updates/module.go
Overview
This PR implements automatic UI process (Tauri) restart after successful system updates, improving user experience by ensuring the latest UI version is running without requiring manual restart.
Changes
Main Features
Bug Fixes
copyAndCheckSHA256Sumfunction in update validation logicFiles Changed
desktop/tauri/src-tauri/src/main.rs- Integration of relaunch trigger on update completiondesktop/tauri/src-tauri/src/relaunch.rs- Core relaunch logic with hardened path resolutiondesktop/tauri/src-tauri/src/portmaster/mod.rs- Update event handlingdesktop/tauri/src-tauri/src/traymenu.rs- Tray menu integrationservice/core/api.go- Backend API changes for update completion signalingservice/updates/module.go- Update module integrationservice/updates/index.go- SHA256 validation fixBreaking Changes
None
Commits Included
Summary by CodeRabbit