On packaged Linux AppImage installs, a Settings restart for network access or Tailscale HTTPS launches Electron from the old temporary mount. That mount can disappear before the replacement loads its runtime files.
Restart through the durable, exact APPIMAGE launcher with the original arguments. A detached Bash helper closes inherited AppImage/Chromium descriptors, waits for parent-owned stdin EOF, then executes the launcher. Select this path only when the running executable is inside APPDIR, so leaked AppImage variables cannot relaunch an unrelated application. Other launch modes retain native Electron relaunch.
The helper suppresses its own Bash startup files while preserving the replacement’s original BASH_ENV and ENV. Once backend shutdown has completed, a scheduling failure is logged and the app exits; there is no native AppImage fallback or recovery dialog.
Based on @sergeytiron’s original #15681, preserving the contributor’s commits with current main merged. The rebuild uses current HostProcess references, removes unrelated configuration changes, and replaces Bash 4.1-only descriptor syntax with validated numeric redirections. AppRun already invokes Bash through PATH; only Bash 5.2 was executed here.
Verification:
-p, its pre-exec set +p, or the APPDIR component boundary each makes focused tests fail. Omitting descriptor closure also failed the process fixture. Sources were restored exactly after probes.git diff --check pass.This is a nonvisual lifecycle change; proof comes from behavior tests and real helper-process execution. The helper test simulates parent exit by closing stdin under managed test ownership. It does not establish native AppImage mount teardown, lock ordering, or detached survival after Electron exits.
Native limits: Electron cannot launch on this VM because libcups.so.2 is missing. Historical packaged Fedora evidence predates the maintainer rebuild and is not current-head proof. Fresh packaged network access and Tailscale enable/disable restarts, exported Bash shell options, symlinked TMPDIR, flag-only extract-and-run, older Bash, NixOS, other Linux distributions/architectures, macOS and Windows remain unverified. Web/mobile/provider behavior does not own this restart path. Human approval of the detached helper and log-only post-shutdown failure policy remains required; unmerged.
Implemented with gpt-6.1-sol through Codex in T3 Code; tests by gpt-6-astra; independent adversarial reviews by Claude claude-opus-5-5 and Codex gpt-6-sol.
Verdict: Not approved
Macroscope's review found this PR not approvable — The PR fixes a narrowly scoped Linux AppImage restart failure, but it introduces nontrivial detached-process, descriptor-cleanup, and EOF-synchronization logic into an existing production restart path. Human review is warranted to validate those lifecycle assumptions across supported Linux environments.
You can add or adjust custom eligibility rules. Learn more.
Warning
Only developers with an assigned seat can use this organization's usage-based review budget, and seats here are assigned manually. Ask an admin to assign a seat, or change the review continuation mode in Billing.
Next included review available in 16 minutes.
Limit details: You’ve used all 10 included reviews currently available.
Review configuration:
529c56c0-9c51-447d-b1fc-b95f479e2fd4Reviewing files that changed from the base of the PR and between 6e405b34917388eb52076dd951614acb5ba2535f and 59eb93faabc6d89864d5663935ee0d6bf7ada00f.
apps/desktop/src/app/DesktopLifecycle.test.tsapps/desktop/src/app/DesktopLifecycle.tsapps/desktop/src/backend/DesktopBackendPool.test.tsapps/desktop/src/electron/ElectronApp.test.tsapps/desktop/src/electron/ElectronApp.tsapps/desktop/src/ipc/methods/notificationBadge.test.tsapps/desktop/src/main.tsNo actionable comments were generated in the recent review. 🎉
7572e861-946b-421d-8320-3a22dbae973eReviewing files that changed from the base of the PR and between 4ee6bfd50ef4a089440d5c3662db2298da9cc50e and 6e405b34917388eb52076dd951614acb5ba2535f.
apps/desktop/src/app/DesktopConfig.tsapps/desktop/src/app/DesktopEnvironment.test.tsapps/desktop/src/app/DesktopLifecycle.test.tsapps/desktop/src/app/DesktopLifecycle.tsapps/desktop/src/electron/ElectronApp.test.tsapps/desktop/src/electron/ElectronApp.tsapps/desktop/src/ipc/methods/notificationBadge.test.tsapps/desktop/src/main.tsIncluded review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
The desktop app now preserves nonblank APPIMAGE values and uses a detached Bash waiter for packaged Linux AppImage relaunches. Lifecycle shutdown logs relaunch failures and continues to exit with code 0 after a scheduling failure.
Linux AppImage Relaunch
| Layer / File(s) | Summary |
|---|---|
APPIMAGE configuration apps/desktop/src/app/DesktopConfig.ts, apps/desktop/src/app/DesktopEnvironment.test.ts | Blank APPIMAGE values are omitted. Nonblank values retain their original whitespace. |
Electron relaunch and service wiring apps/desktop/src/electron/ElectronApp.ts, apps/desktop/src/electron/ElectronApp.test.ts, apps/desktop/src/main.ts, apps/desktop/src/ipc/methods/notificationBadge.test.ts | On packaged Linux with a nonblank APPIMAGE, ElectronApp spawns detached Bash to wait for pipe EOF, then execute the requested path or APPIMAGE with the supplied arguments or current process arguments. Other configurations use Electron's native relaunch. Application and test layers provide Node services. |
Lifecycle relaunch failure handling apps/desktop/src/app/DesktopLifecycle.ts, apps/desktop/src/app/DesktopLifecycle.test.ts | Relaunch failures are logged. A scheduling failure is caught locally, and shutdown proceeds to exit with code 0. |
Priority: ➖ Normal
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Bug fix
| 1 | sequenceDiagram |
| 2 | participant DesktopLifecycle |
| 3 | participant ElectronApp |
| 4 | participant ChildProcessSpawner |
| 5 | participant DetachedBash |
| 6 | participant APPIMAGE |
| 7 | DesktopLifecycle->>ElectronApp: request relaunch with arguments |
| 8 | ElectronApp->>ChildProcessSpawner: spawn detached Bash waiter |
| 9 | ChildProcessSpawner-->>ElectronApp: return spawned child |
| 10 | DetachedBash->>DetachedBash: wait for pipe EOF |
| 11 | DetachedBash->>APPIMAGE: exec launcher with path and arguments |
Suggested reviewers: juliusmarminge
6e405The AppImage restart path preserves the launcher name and waits for the old process to exit. No actionable issue remains beyond the stated limitation that a failed launch after shutdown has no recovery UI.
Security architecture risk: 🔵 Low · up to 6e405
The change is limited to packaged Linux AppImages and includes controls for argument handling, shell startup files, and inherited descriptors. No material security regression was established. Some uncertainty remains around real process-exit timing, concurrent restart requests, and interrupted handoffs.
Retained concerns No architecture-level concerns identified.
Security Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Failed Failed checks: 2 warnings
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Warning | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 8 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
| Description check | Warning | The description clearly covers the problem, implementation, verification, limitations, and test results. However, it does not provide the required explicit maintainer approval or approval comment for … | Add the triaged bug issue or discussion link with the explicit maintainer approval comment. If this is an obvious small fix that does not require prior approval, explain why it qualifies for the exemption. |
| Check name | Status | Explanation |
|---|---|---|
| Linked Issues check | Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | Passed | Check skipped because no linked issues were found for this pull request. |
| Title check | Passed | The title clearly identifies the main change: fixing Linux AppImage settings restarts so they launch a fresh instance. |
Explanation
The description clearly covers the problem, implementation, verification, limitations, and test results. However, it does not provide the required explicit maintainer approval or approval comment for scope, and it states that human approval remains outstanding.
Resolution
Add the triaged bug issue or discussion link with the explicit maintainer approval comment. If this is an obvious small fix that does not require prior approval, explain why it qualifies for the exemption.
Comment @coderabbitai help to get the list of available commands.
Note
This comment is posted by Julius' dot
The AppImage restart failure is well established in #16629, and the packaged restart checks here are useful. One scope question remains under prior approval: the old restart path calls Electron directly, while this path requires Bash 4.1+ on PATH and exits the already shut-down app if spawning it fails.
Can you establish that this Bash requirement is already guaranteed for supported AppImage installs, or does it add a new runtime prerequisite? If it is new, maintainers need to decide whether that requirement and the log-only failure behavior are acceptable for settings restarts. Please link that decision or adjust the approach accordingly. Leaving this open pending that clarification; this is not a finding that the packaged reproduction is inadequate.
I investigated the Bash dependency more closely. Our AppImages already launch through AppRun with #!/usr/bin/env bash; this is present in the installed nightly and in electron-builder 26.15.6’s generated launcher. Bash on PATH is therefore an existing requirement.
The 4.1 minimum is new, and I haven’t established an explicit T3 guarantee for it. It seems a conservative desktop baseline: even RHEL 6 shipped Bash 4.1. Modern Bash 3.2 examples exist in embedded Linux, but I found no evidence of a supported T3 desktop environment requiring it. NixOS remains untested.
The failure behavior is a separate tradeoff: after shutdown, a failed spawn is logged and the app exits without restarting. Successful spawning also does not establish that the helper initialized correctly.
Would you accept Bash 4.1+ and that failure behavior for this focused fix, or prefer an adjustment that verifies helper readiness before shutdown and reports preparation failures visibly?
Added 1 commit
Taking over this one as maintainer for desktop-app-10. I will preserve the original commits, integrate current main, minimize the tests, and reassess the durable AppImage launcher/helper against current desktop lifecycle conventions. The Bash requirement and failure behavior will remain explicit for human review; this does not include merging.
Added 5 commits
Maintainer rebuild pushed, preserving @sergeytiron’s commits. Two full Opus/Sol review rounds are complete, with both round-one defects fixed; 55 focused tests and scoped desktop typecheck/lint pass.
For the earlier prerequisite question: the helper no longer uses Bash 4.1-only descriptor syntax. AppRun already requires Bash through PATH; only Bash 5.2 was executed here. After completed backend shutdown, a scheduling failure still logs and exits, and a later exec failure has no recovery UI. That policy remains for human approval. Native Electron is blocked here by missing libcups.so.2; the prior Fedora proof is stale for this head, so fresh packaged network/Tailscale enable/disable proof remains required. The body now records the current behavior and limits.
Supplemental packaged-runtime verification for #16629, run on Ubuntu 24.04 x86_64 under Xvfb with isolated empty profiles, FUSE-mounted AppImages, --no-sandbox --disable-gpu.
local-only trials invoked desktopBridge.setServerExposureMode('network-accessible'). The old mount disappeared; replacement main died with SIGBUS and children with SIGTRAP/SIGBUS. No usable replacement appeared during the observation period. The historical ICU text/stack was not reproduced.network-accessible, stable at the five-second follow-up.Commands, environment, SHAs and full limitations · Runtime transcripts and scratch harness
| Before: packaged current main | After: packaged PR head |
|---|---|
| Restart fails recording | Replacement returns recording |
After: PR replacement window at the stable check
Independent proof audit inspected recording frames, transcripts and harness and accepted these limited observations. This is supplemental runtime proof, not a merge-readiness verdict. Main bundles Electron 44.9.3, this older exact PR head 44.4.5, so this does not isolate the patch or prove a rebased result. Stock PR packaging failed on an unused node-pty patch; the tested artifact used an external hook pinning staged node-pty to the PR lockfile version, with no repository edits. Clips are excerpts; the full observation and stable follow-up are in the transcripts. Archived harness revisions limit exact replayability.
Manual confirmation dialog, Fedora/KDE/Wayland, remote reachability, reverse/Tailscale restarts and longer-term recovery remain unverified. Both successful controls reported null advertised endpoints. No implementation or tests changed. Test processes, scratch and assigned worktree were removed. Existing human-review concerns remain outstanding; no merge or outcome label was applied.
Verified with gpt-6.1-sol through Codex in T3 Code.