-
Notifications
You must be signed in to change notification settings - Fork 3.6k
fix(desktop): make AppImage restarts come back on Linux #4918
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,172 @@ | ||
| // @effect-diagnostics nodeBuiltinImport:off - Skip predicate runs at collection time, outside any Effect runtime. | ||
| import * as NodeFS from "node:fs"; | ||
|
|
||
| import { assert, describe, it } from "@effect/vitest"; | ||
| import * as Effect from "effect/Effect"; | ||
|
|
||
| import { | ||
| buildAppImageRelaunchShellCommand, | ||
| posixShellSingleQuote, | ||
| resolveDesktopRelaunchPlan, | ||
| } from "./resolveDesktopRelaunchOptions.ts"; | ||
| import { scheduleAppImageRelaunch } from "./scheduleAppImageRelaunch.ts"; | ||
|
|
||
| describe("resolveDesktopRelaunchPlan", () => { | ||
| it.effect("schedules a delayed AppImage re-exec with flag argv only", () => | ||
| Effect.sync(() => { | ||
| const plan = resolveDesktopRelaunchPlan({ | ||
| appImagePath: "/home/user/T3-Code-0.0.28-x86_64.AppImage", | ||
| execPath: "/tmp/.mount_t3_codXXXX/t3code", | ||
| argv: [ | ||
| "/tmp/.mount_t3_codXXXX/t3code", | ||
| "--no-sandbox", | ||
| "/tmp/.mount_t3_codXXXX/resources/app.asar", | ||
| ], | ||
| }); | ||
|
|
||
| assert.deepEqual(plan, { | ||
| kind: "appimage-delayed", | ||
| appImagePath: "/home/user/T3-Code-0.0.28-x86_64.AppImage", | ||
| args: ["--no-sandbox"], | ||
| delayMs: 1_000, | ||
| }); | ||
| }), | ||
| ); | ||
|
|
||
| it.effect("trims APPIMAGE and falls back to electron relaunch when empty", () => | ||
| Effect.sync(() => { | ||
| assert.deepEqual( | ||
| resolveDesktopRelaunchPlan({ | ||
| appImagePath: " /opt/T3-Code.AppImage ", | ||
| execPath: "/tmp/.mount/app", | ||
| argv: ["/tmp/.mount/app"], | ||
| }), | ||
| { | ||
| kind: "appimage-delayed", | ||
| appImagePath: "/opt/T3-Code.AppImage", | ||
| args: [], | ||
| delayMs: 1_000, | ||
| }, | ||
| ); | ||
|
|
||
| assert.deepEqual( | ||
| resolveDesktopRelaunchPlan({ | ||
| appImagePath: " ", | ||
| execPath: "/usr/bin/t3-code", | ||
| argv: ["/usr/bin/t3-code", "--flag"], | ||
| }), | ||
| { | ||
| kind: "electron", | ||
| execPath: "/usr/bin/t3-code", | ||
| args: ["--flag"], | ||
| }, | ||
| ); | ||
| }), | ||
| ); | ||
|
|
||
| it.effect("preserves packaged non-AppImage exec path and argv", () => | ||
| Effect.sync(() => { | ||
| const plan = resolveDesktopRelaunchPlan({ | ||
| appImagePath: null, | ||
| execPath: "/Applications/T3 Code.app/Contents/MacOS/T3 Code", | ||
| argv: ["/Applications/T3 Code.app/Contents/MacOS/T3 Code", "--inspect"], | ||
| }); | ||
|
|
||
| assert.deepEqual(plan, { | ||
| kind: "electron", | ||
| execPath: "/Applications/T3 Code.app/Contents/MacOS/T3 Code", | ||
| args: ["--inspect"], | ||
| }); | ||
| }), | ||
| ); | ||
| }); | ||
|
|
||
| describe("buildAppImageRelaunchShellCommand", () => { | ||
| it.effect("quotes the AppImage path and delays before exec", () => | ||
| Effect.sync(() => { | ||
| const command = buildAppImageRelaunchShellCommand({ | ||
| appImagePath: "/home/user/T3 Code.AppImage", | ||
| args: ["--no-sandbox"], | ||
| delayMs: 1_000, | ||
| }); | ||
|
|
||
| assert.equal( | ||
| command, | ||
| `sleep 1 && exec ${posixShellSingleQuote("/home/user/T3 Code.AppImage")} ${posixShellSingleQuote("--no-sandbox")}`, | ||
| ); | ||
|
|
||
| assert.equal(posixShellSingleQuote("it's"), `'it'\\''s'`); | ||
| }), | ||
| ); | ||
|
|
||
| it.effect("omits the fd cleanup unless the shell is known to handle it", () => | ||
| Effect.sync(() => { | ||
| const base = { | ||
| appImagePath: "/opt/T3.AppImage", | ||
| args: [], | ||
| delayMs: 1_000, | ||
| } as const; | ||
|
|
||
| // dash reads `exec 10>&-` as a command named "10" and a failed exec kills | ||
| // a non-interactive shell, so emitting this for /bin/sh would abort the | ||
| // helper before the re-exec and lose the app entirely. | ||
| assert.isFalse(buildAppImageRelaunchShellCommand(base).includes("/proc/$$/fd")); | ||
|
|
||
| const withCleanup = buildAppImageRelaunchShellCommand({ | ||
| ...base, | ||
| closeInheritedFds: true, | ||
| }); | ||
| // Must run before the sleep, so the outgoing mount can be released during it. | ||
| assert.isTrue(withCleanup.startsWith("for fd in /proc/$$/fd/*;"), withCleanup); | ||
| assert.isTrue( | ||
| withCleanup.indexOf("exec $n>&-") < withCleanup.indexOf("sleep 1"), | ||
| withCleanup, | ||
| ); | ||
| }), | ||
| ); | ||
|
|
||
| it.effect("records a relaunch failure without letting the log block the exec", () => | ||
| Effect.sync(() => { | ||
| const command = buildAppImageRelaunchShellCommand({ | ||
| appImagePath: "/opt/T3.AppImage", | ||
| args: [], | ||
| delayMs: 1_000, | ||
| logPath: "/home/user/.t3/logs/relaunch.log", | ||
| }); | ||
|
|
||
| // Wrapping the group in `2>>log` would abort the whole helper when the | ||
| // log directory is missing, and would also follow a *successful* exec | ||
| // into the relaunched app. Guard with a test and append afterwards. | ||
| assert.notInclude(command, "} 2>>"); | ||
| assert.include(command, `[ -x ${posixShellSingleQuote("/opt/T3.AppImage")} ] && exec `); | ||
| // A bad log path must not matter once we are past the exec. | ||
| assert.isTrue( | ||
| command.endsWith( | ||
| `>>${posixShellSingleQuote("/home/user/.t3/logs/relaunch.log")} 2>/dev/null`, | ||
| ), | ||
| command, | ||
| ); | ||
| }), | ||
| ); | ||
| }); | ||
|
|
||
| describe("scheduleAppImageRelaunch", () => { | ||
| // Really spawns the helper. The code path is POSIX-only by construction, so | ||
| // gate on the shell it needs rather than on a platform name — on a Windows | ||
| // dev machine (a supported setup) this would otherwise fail with ENOENT. | ||
| it.effect.skipIf(!NodeFS.existsSync("/bin/sh"))( | ||
| "resolves only once the detached helper has actually spawned", | ||
| () => | ||
| // Resolves on the `spawn` event, well before the helper's sleep elapses. | ||
| // The caller depends on this to know it is safe to release the | ||
| // single-instance lock and exit rather than vanishing on a failed spawn. | ||
| Effect.promise(() => | ||
| scheduleAppImageRelaunch({ | ||
| kind: "appimage-delayed", | ||
| appImagePath: "/bin/true", | ||
| args: [], | ||
| delayMs: 200, | ||
| }), | ||
| ), | ||
| ); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,120 @@ | ||
| export interface DesktopElectronRelaunchPlan { | ||
| readonly kind: "electron"; | ||
| readonly execPath: string; | ||
| readonly args: string[]; | ||
| } | ||
|
|
||
| export interface DesktopAppImageRelaunchPlan { | ||
| readonly kind: "appimage-delayed"; | ||
| readonly appImagePath: string; | ||
| readonly args: string[]; | ||
| /** Delay before re-exec so the current AppImage can unmount its FUSE image. */ | ||
| readonly delayMs: number; | ||
| } | ||
|
|
||
| export type DesktopRelaunchPlan = DesktopElectronRelaunchPlan | DesktopAppImageRelaunchPlan; | ||
|
|
||
| export const DEFAULT_APPIMAGE_RELAUNCH_DELAY_MS = 1_000; | ||
|
|
||
| /** | ||
| * Resolve how the desktop process should restart for the current packaging mode. | ||
| * | ||
| * AppImage mounts the payload under a temporary directory and sets `APPIMAGE` | ||
| * to the outer `.AppImage` path. `process.execPath` / `process.argv` point at | ||
| * the mount, which is unmounted when the process exits — so relaunching with | ||
| * those paths (or racing a second FUSE mount while the first unmounts) fails | ||
| * with "Cannot mount AppImage, please check your FUSE setup." | ||
| * | ||
| * For AppImage we schedule a delayed re-exec of `$APPIMAGE` after the current | ||
| * process exits. Keep only flag-style argv entries (e.g. `--no-sandbox` from | ||
| * Gear Lever / Flatpak launches). | ||
| */ | ||
| export function resolveDesktopRelaunchPlan(input: { | ||
| readonly appImagePath?: string | null | undefined; | ||
| readonly execPath: string; | ||
| readonly argv: readonly string[]; | ||
| readonly appImageDelayMs?: number; | ||
| }): DesktopRelaunchPlan { | ||
| const appImagePath = input.appImagePath?.trim(); | ||
| if (appImagePath) { | ||
|
Comment on lines
+38
to
+39
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 High
- const appImagePath = input.appImagePath?.trim();
- if (appImagePath) {
+ const trimmedAppImagePath = input.appImagePath?.trim();
+ if (trimmedAppImagePath) {
+ const appImagePath = input.appImagePath!;
🤖 Copy this AI Prompt to have your agent fix this: |
||
| return { | ||
| kind: "appimage-delayed", | ||
| appImagePath, | ||
| args: input.argv.slice(1).filter((arg) => arg.startsWith("--")), | ||
| delayMs: input.appImageDelayMs ?? DEFAULT_APPIMAGE_RELAUNCH_DELAY_MS, | ||
| }; | ||
| } | ||
|
|
||
| return { | ||
| kind: "electron", | ||
| execPath: input.execPath, | ||
| args: input.argv.slice(1), | ||
| }; | ||
| } | ||
|
|
||
| /** POSIX-safe single-quoting for `/bin/sh -c` command construction. */ | ||
| export function posixShellSingleQuote(value: string): string { | ||
| return `'${value.replaceAll("'", `'\\''`)}'`; | ||
| } | ||
|
|
||
| /** | ||
| * Drop file descriptors inherited from the exiting app before re-exec. | ||
| * | ||
| * Chromium keeps fds open without CLOEXEC (`app.asar`, `icudtl.dat`, the `.pak` | ||
| * files, the mount directory itself), and `spawn` cannot close them for us. | ||
| * Inherited through this helper's `exec`, they keep the *outgoing* AppImage's | ||
| * FUSE mount busy forever: its runtime process parks in `fuse_dev_do_read` and | ||
| * never unmounts, so every restart strands a mount and a process. | ||
| * | ||
| * Only emitted for bash. Chromium's fds are well above 9, and dash — `/bin/sh` | ||
| * on Debian and Ubuntu — does not accept multi-digit fd numbers in | ||
| * redirections: it reads `exec 10>&-` as running a command named `10`, and a | ||
| * failed `exec` terminates a non-interactive shell on the spot. That kills the | ||
| * helper before it reaches the re-exec, leaving the app gone for good. `eval` | ||
| * does not help, because this is a failed command rather than a parse error, | ||
| * so `|| true` never runs. | ||
| */ | ||
| const CLOSE_INHERITED_FDS_SNIPPET = | ||
| 'for fd in /proc/$$/fd/*; do n=${fd##*/}; case "$n" in 0|1|2) continue;; esac; ' + | ||
| 'eval "exec $n>&-" 2>/dev/null || true; done'; | ||
|
|
||
| export function buildAppImageRelaunchShellCommand(input: { | ||
| readonly appImagePath: string; | ||
| readonly args: readonly string[]; | ||
| readonly delayMs: number; | ||
| /** Only safe under bash; see {@link CLOSE_INHERITED_FDS_SNIPPET}. */ | ||
| readonly closeInheritedFds?: boolean; | ||
| /** Captures the helper's stderr, so a failed delayed `exec` is diagnosable. */ | ||
| readonly logPath?: string | undefined; | ||
| }): string { | ||
| // Whole seconds only: POSIX specifies `sleep` as taking an integer, and a | ||
| // /bin/sh whose sleep rejects "1.0" would short-circuit the `&&` and never | ||
| // exec — an app that quits and never returns, which is the failure this | ||
| // helper exists to avoid. The spawn succeeds either way, so nothing would | ||
| // report it. | ||
| const sleepSeconds = Math.max(1, Math.ceil(input.delayMs / 1_000)); | ||
| const quotedPath = posixShellSingleQuote(input.appImagePath); | ||
| const quotedArgs = input.args.map(posixShellSingleQuote).join(" "); | ||
| const execTarget = quotedArgs.length > 0 ? `${quotedPath} ${quotedArgs}` : quotedPath; | ||
| // The exec happens after we have exited, so a missing or non-executable | ||
| // AppImage can only be reported by leaving a trace behind. Test for it and | ||
| // append a line *after* the exec instead of redirecting the whole group: | ||
| // | ||
| // - `{ ...; } 2>>log` fails the redirection outright when the log's | ||
| // directory is missing or unwritable, and the re-exec then never runs at | ||
| // all — trading a diagnosable failure for a guaranteed one. | ||
| // - that redirection also survives a *successful* exec, so the relaunched | ||
| // app would spend its whole life appending Chromium stderr to an | ||
| // unrotated file. | ||
| // | ||
| // `2>/dev/null` on the append keeps a bad log path from mattering, and the | ||
| // exec'd process keeps the stdio it was always meant to have. | ||
| const relaunch = input.logPath | ||
| ? `sleep ${sleepSeconds}; [ -x ${quotedPath} ] && exec ${execTarget}; ` + | ||
| `echo ${posixShellSingleQuote(`T3 Code relaunch failed: ${input.appImagePath} is missing or not executable`)} ` + | ||
| `>>${posixShellSingleQuote(input.logPath)} 2>/dev/null` | ||
| : `sleep ${sleepSeconds} && exec ${execTarget}`; | ||
| // Close first, then sleep: releasing the fds up front lets the outgoing | ||
| // mount unmount *during* the delay rather than after it. | ||
| return input.closeInheritedFds ? `${CLOSE_INHERITED_FDS_SNIPPET}; ${relaunch}` : relaunch; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This re-export has no consumers (
DesktopLifecycle.tsitself andDesktopLifecycle.relaunch.test.tsboth import from./resolveDesktopRelaunchOptions.ts), so it only adds a second import path for the helper and interrupts the canonical module order (imports, errors,Context.Servicetag,make,layer). Consider dropping it and letting consumers import the helper from its own module.Posted via Macroscope — Effect Service Conventions