Fix Linux autostart reboot bug and release-publish race, add PR CI - #2
Merged
Merged
Conversation
process.execPath inside a running AppImage resolves to a path under a per-launch /tmp/.mount_* FUSE mount (confirmed directly against a running instance), which stops existing once that instance exits, so the autostart entry written with it pointed nowhere after the next sign-in. The AppImage runtime sets APPIMAGE to the original file's stable path before exec'ing the wrapped binary (also confirmed directly); resolveLinuxExecutablePath() now prefers that when set. Separately, autostartDirectory() always wrote to ~/.config/autostart regardless of $XDG_CONFIG_HOME, which the XDG Base Directory Specification says should take precedence when set.
Each platform job independently ran gh release view/create/upload, so if one platform's job failed after the other had already published, the release would end up with only one platform's asset; concurrent create calls from both jobs were also a race. Both jobs now only build and upload a workflow artifact; a new publish job that needs both creates or updates the GitHub Release in one place, with all four files, only once both builds have actually succeeded.
pnpm test previously only ran inside the release workflow, which only triggers on a version tag -- meaning a broken test could only be discovered by cutting a release. Adds a CI workflow that runs the same test suite on both Windows and Linux for every push to main and every pull request. electron-runtime-smoke.js and visual-smoke.js still require a display and are not wired in here; they stay manual/local for now rather than adding Xvfb/headless-GUI plumbing in the same change.
README still said the Linux AppImage hadn't been released, but v0.4.0 published both platform assets. Also documents the corrected executable-path resolution and XDG_CONFIG_HOME handling in architecture.md, including the two facts confirmed directly against a running AppImage instance that motivated the fix.
A literal "%" in the Exec value (e.g. a path like .../100%free/...) would have been read as the start of a field code (%f, %u, etc.) by a conforming launcher, since NEEDS_QUOTING never covered "%" and quoting doesn't affect field-code substitution anyway. Now always escaped to "%%" regardless of whether the value also needs quoting. Separately, the XDG Base Directory Specification requires XDG_*_HOME values to be absolute and says a relative value should be treated as invalid; autostartDirectory() now falls back to ~/.config in that case instead of joining a relative path onto it.
Three changes to release.yml, all from external review: - gh release upload --clobber only replaces files with matching names, so a release that already had unrelated or stale assets under its tag (e.g. from a past mistake, or a manually re-run tag) would keep them alongside the new upload. The publish job now fails outright if a release already exists for the tag rather than silently reusing it -- replacing a published release is now a deliberate action (delete it first), never an implicit side effect of re-running this workflow. - contents: write was set at the workflow level, so both build jobs carried it despite never touching repo contents or the Releases API themselves (they only upload a build artifact). Scoped to contents: read by default, with contents: write added only on the publish job that actually needs it. - Added a check-version job that both build jobs now depend on, comparing the pushed tag against package.json's version and failing fast if they don't match, instead of building and possibly publishing artifacts under a version string nobody intended.
electron-runtime-smoke.js exercises the real safeStorage encryption round trip and Tray creation through actual Electron APIs rather than the mocks the unit tests use, so it catches things those can't (e.g. a genuinely broken DPAPI call). It needs no BrowserWindow/ display, so it isn't blocked by the same headless-GUI concerns as visual-smoke.js. Only wired into the windows-latest matrix leg, though, since safeStorage's Linux backends depend on a gnome-keyring/D-Bus session that GitHub's ubuntu-latest runners don't provide -- running it there would likely just be flaky rather than meaningful.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses a code review of the v0.4.0 release. All four original findings were independently verified before fixing; one review claim about how the AppImage-path fix would work was checked empirically and confirmed correct. A follow-up review of this same PR then found three more issues, also independently verified and fixed below.
Round 1
Round 2 (this PR, before merge)
Not in scope here
Test plan