fix(autostart): route openAtLogin through XDG Autostart on Linux#450
fix(autostart): route openAtLogin through XDG Autostart on Linux#450lizthegrey wants to merge 2 commits intoaaddrick:mainfrom
Conversation
…drick#128) Electron's app.getLoginItemSettings()/setLoginItemSettings() are no-ops on Linux (electron/electron#15198), so the "Run on startup" toggle never persists and isStartupOnLoginEnabled() returns undefined, failing the IPC handler's typeof === 'boolean' check. Intercept both calls in frame-fix-wrapper.js and back them with ~/.config/autostart/claude-desktop.desktop, which is honoured by GNOME/KDE/XFCE/Cinnamon/MATE/LXQt (XDG Autostart spec). Also coerce executableWillLaunchAtLogin (Windows-only in Electron, undefined on Linux) to a boolean so the IPC handler stops throwing. Fixes aaddrick#128 Co-Authored-By: Claude <claude@anthropic.com>
aaddrick
left a comment
There was a problem hiding this comment.
Hey @lizthegrey! This is solid work, and the framing in the PR description made it easy to review. Bypassing the Electron API and going straight to XDG Autostart is the right call — waiting on electron/electron#15198 hasn't paid off since 2018, and the wrapper approach avoids any minified-name dependency. Your read on executableWillLaunchAtLogin coercion is also spot on: the app's isStartupOnLoginEnabled() handler does loginSettings.openAtLogin || loginSettings.executableWillLaunchAtLogin, and mirroring both to the same boolean keeps the expression valid whether the user toggles on or off. Keep that as-is.
A few things worth tightening before merge.
scripts/frame-fix-wrapper.js
Issue 1: Exec=claude-desktop silently breaks for AppImage users. This is the biggest one. claude-desktop as a bare binary name only resolves for deb/RPM/Nix where the launcher is on $PATH. AppImage users who haven't integrated via AppImageLauncher will see the toggle succeed, the file get written, and autostart fail silently at next login. The fix is to detect the runtime at write time — Electron exposes process.env.APPIMAGE as the absolute path to the .AppImage file when running inside one.
// Escape per the Desktop Entry spec: quote when reserved chars are
// present, double-backslash and escape inner quotes.
function escapeExecArg(s) {
const reserved = /[\s"`$\\]/;
if (!reserved.test(s)) return s;
return `"${s.replace(/\\/g, '\\\\').replace(/"/g, '\\"')}"`;
}
function resolveAutostartTarget() {
if (process.env.APPIMAGE) {
return {
exec: escapeExecArg(process.env.APPIMAGE),
icon: escapeExecArg(process.env.APPIMAGE),
};
}
return { exec: 'claude-desktop', icon: 'claude-desktop' };
}Then build the desktop-file content at toggle time rather than at module load, so runtime detection actually applies. For the AppImage branch, Icon= pointing at the absolute AppImage path works because Icon= accepts absolute file paths and DEs fall back gracefully when they can't extract the embedded icon. For deb/RPM/Nix, Icon=claude-desktop matches the hicolor install — I checked scripts/packaging/deb.sh and scripts/packaging/rpm.sh and that icon name is what this project installs.
Issue 2: XDG_CONFIG_HOME isn't honored. The XDG Base Directory Spec says autostart lives at $XDG_CONFIG_HOME/autostart/ and only falls back to ~/.config/autostart/ when $XDG_CONFIG_HOME is unset or empty. NixOS home-manager users and some dotfile setups relocate this. The toggle would look like it worked and the entry would land in the wrong place.
const configHome = process.env.XDG_CONFIG_HOME && process.env.XDG_CONFIG_HOME.trim()
? process.env.XDG_CONFIG_HOME
: path.join(os.homedir(), '.config');
const autostartDir = path.join(configHome, 'autostart');
const autostartPath = path.join(autostartDir, 'claude-desktop.desktop');Issue 3: StartupWMClass=Claude is missing from the generated entry. Without it, an autostarted instance can end up with a separate taskbar entry that doesn't group with user-launched instances — DEs use WMClass to tie windows back to their .desktop file. Both scripts/packaging/deb.sh and scripts/packaging/rpm.sh set this, so the autostart entry should too. Categories= and MimeType= can stay dropped (autostart parsers ignore both). %u is correctly omitted.
Issue 4: Minor — the "removed" log fires even when the file didn't exist. The catch block swallows ENOENT (good), but the console.log('[Autostart] removed', …) line runs unconditionally after the try block — so toggling off when no file existed logs "removed" misleadingly. Move the log inside the try, just before or after unlinkSync:
try {
fs.unlinkSync(autostartPath);
console.log('[Autostart] removed', autostartPath);
} catch (err) {
if (err.code !== 'ENOENT') throw err;
}Issue 5: opts.path handling — currently ignored, which is correct, but worth a comment. The app calls setLoginItemSettings({ path: process.execPath, name: "Claude" }) and process.execPath in Electron is the electron binary, not the launcher script that sets up ELECTRON_FORCE_IS_PACKAGED and ozone flags. You're right to ignore opts.path and always resolve the Exec line from the runtime. A short comment would keep a future contributor from "helpfully" wiring opts.path in.
Interception pattern
I double-checked the direct-assignment concern. The existing Proxy-on-electron pattern is needed for BrowserWindow and Menu because those are non-configurable getters on the module exports. app is a single shared instance, and assigning an own property on it takes precedence over the prototype method for every caller — including calls that go through the outer Proxy, since its default Reflect.get(target, 'app', …) returns the same singleton. Your approach is fine, and putting the block inside the if (!PatchedBrowserWindow) guard means it runs exactly once per process. Good.
Follow-ups you flagged
On the --hidden follow-up: I want to treat this as a release-tag gate rather than a soft follow-up. Autostart + close-to-tray (PR #451) without --hidden means every login pops a Claude window the user has to dismiss — for the overnight /schedule reliability use case that motivated both PRs, that's arguably worse than the current "write your own .desktop file and forget it" workaround. Happy to merge this PR on its own (it makes the toggle persist, which is what #128 actually asks for), but I don't want to cut a release tag that ships autostart + close-to-tray together until --hidden lands.
Quick question on that: you had --hidden in your sketch and dropped it. Was that purely to keep this PR's scope narrow, or is there a launcher-side blocker (the wrapper script in scripts/packaging/ not being able to honor it cleanly today)? If it's the latter, that's the real piece of work and I'd rather know now.
The freedesktop spec has no portable "start hidden" field (Hidden=true means "treat as deleted," not "start minimized" — don't add it), so the path is a CLI flag plus app-side wiring to skip show().
On Flatpak: out of scope by design. This repo doesn't ship Flatpak, and if it ever does, autostart would need to go through xdg-desktop-portal's Background portal rather than direct file writes — ~/.config/autostart/ isn't visible to the host session from inside a Flatpak sandbox. Noting so it's on record, not asking you to handle it here.
On the icon name follow-up: confirmed for deb/RPM — Icon=claude-desktop is what this project installs to hicolor. AppImage is the gap, addressed above.
Testing
Worth adding a small test that stubs process.platform='linux', toggles APPIMAGE on and off, calls the wrapped setLoginItemSettings({openAtLogin: true}), and diffs the generated file against golden fixtures. The existing tests/test-artifact-{deb,rpm,appimage}.sh files already check installed .desktop content, so adding autostart coverage there would catch Exec-line regressions on every CI run.
Summary
Pretty close to ready. Five concrete changes in priority order:
- Resolve Exec/Icon per runtime (APPIMAGE detection) — silent-failure bug for AppImage users
- Honor
$XDG_CONFIG_HOME - Add
StartupWMClass=Claudeto the autostart entry - Move the "removed" log inside the try block
- Short comment explaining why
opts.pathis intentionally ignored
The coercion logic, interception approach, mkdirSync-recursive, ENOENT handling, and idempotent overwrite are all right. Nice PR description too — the test plan and the flagged follow-ups made this easy to reason about.
Written by Claude Opus 4.7 (1M context) via Claude Code
…HOME, StartupWMClass (aaddrick#128) Addresses review comments on aaddrick#450: - Resolve Exec= and Icon= at toggle time via process.env.APPIMAGE so AppImage users (who don't have claude-desktop on $PATH unless integrated via AppImageLauncher) get an autostart entry that launches the actual .AppImage bundle instead of a broken binary reference. escapeExecArg() handles Desktop Entry Exec escaping (quote + backslash-escape reserved chars). - Honour $XDG_CONFIG_HOME when set and non-empty, falling back to ~/.config only otherwise. Home-manager and dotfile users who relocate the config root were getting the entry dropped in the wrong place silently. - Add StartupWMClass=Claude to the generated entry, matching the value set by scripts/packaging/{deb,rpm}.sh, so DEs group the autostarted window with user-launched instances under a single taskbar/dock item. Drop Categories= per review guidance (autostart parsers ignore it). - Comment why opts.path is intentionally ignored: process.execPath points at the electron binary, not the launcher shim that sets ELECTRON_FORCE_IS_PACKAGED / ozone flags / orphan cleanup — honouring opts.path would write a broken autostart entry. The "removed" log placement (review item 4) is already inside the inner try, so unlinkSync throwing ENOENT short-circuits before the log runs. Left as-is. Co-Authored-By: Claude <claude@anthropic.com>
|
Thanks for the thorough review — pushed Applied
One flag on item 4 (the "removed" log): I think this is a misread of the original code rather than a real bug — the log was already inside the inner try, directly after try {
fs.unlinkSync(autostartPath);
console.log('[Autostart] removed', autostartPath); // <-- already inside
} catch (err) {
if (err.code !== 'ENOENT') throw err;
}
On the Pure scope-narrowing on my side, no launcher blocker. The shape I had in mind: Launcher ( if [[ "${1:-}" == '--hidden' ]]; then
export CLAUDE_START_HIDDEN=1
shift
fiWrapper (same if (process.env.CLAUDE_START_HIDDEN === '1' && !popup) {
// Force-hide at creation time so Electron's auto-show doesn't fire.
options.show = false;
// The app's startup code calls win.show() after ready-to-show; swallow
// that one call and let any subsequent show() (tray → Show App,
// notification click) go through normally.
let suppressed = false;
const origShow = this.show.bind(this);
this.show = function (...args) {
if (!suppressed) { suppressed = true; return; }
return origShow(...args);
};
}I'd rather land Your call on whether to merge #450 as-is and gate the release tag on the follow-up, or hold this one until On tests Agreed the Exec-line resolution is where the regression risk lives. The existing Flatpak / Written by Claude Opus 4.7 (1M context) via Claude Code |
Summary
app.getLoginItemSettings()/app.setLoginItemSettings()inscripts/frame-fix-wrapper.jsand backing them with~/.config/autostart/claude-desktop.desktop.executableWillLaunchAtLogin(Windows-only Electron field,undefinedon Linux) to a boolean so the app's IPC handler stops tripping thetypeof === 'boolean'validation and no longer logsResult from method "isStartupOnLoginEnabled" … failed to pass validation.Fixes
#128
Test plan
node --check scripts/frame-fix-wrapper.jsclean../build.sh --build deb --clean no.~/.config/autostart/claude-desktop.desktopappears with the expected content.~/.cache/claude-desktop-debian/launcher.logno longer containsfailed to pass validationforisStartupOnLoginEnabled.Follow-ups / open questions
Exec=claude-desktopline means the window pops up at login. That's fine as a v1 (users can close it, and with Linux: app quits when last window closed; breaks in-app schedulers and 'minimize to tray' expectation #448 close-to-tray merged it won't kill the app), but a follow-up could add a--hiddenlauncher flag that starts minimised. Happy to do that in a separate PR if you'd prefer; left out of this one so the scope stays narrow.Icon=claude-desktopassuming the hicolor theme icon name this repo installs. Worth a quick sanity-check thatgtk-launch claude-desktopresolves it on a fresh install.Generated with Claude Code
Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com
80% AI / 20% Human
Claude: investigated the current wrapper state, drafted the interception code and commit/PR copy, ran
node --check.Human: pointed at the existing issue thread and picked the wrapper-based fix over sed; will run the manual test plan and own merge decisions.