fix(desktop): improve AppImage icons and remote environment#2538
fix(desktop): improve AppImage icons and remote environment#2538mwolson wants to merge 19 commits intopingdotgg:mainfrom
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
ApprovabilityVerdict: Needs human review This PR introduces significant new Linux desktop environment handling (password store detection, DBus session bus resolution), a new IPC method for removing saved environments, and schema changes affecting date serialization. An unresolved review comment questions whether the Electron pre-ready layer restructuring maintains correct initialization ordering. You can customize Macroscope's approvability policy. Learn more. |
dba46b4 to
bfc4a7c
Compare
|
can you resolve conflcits here? |
…ore-backend # Conflicts: # apps/desktop/src/desktopSettings.test.ts # apps/desktop/src/desktopSettings.ts # apps/desktop/src/main.ts
| registerDesktopSchemePrivilegesSync, | ||
| ).pipe(Effect.withSpan("desktop.electron.protocol.registerSchemePrivileges")); | ||
|
|
||
| export const layerSchemePrivileges = Layer.effectDiscard(registerDesktopSchemePrivileges); |
There was a problem hiding this comment.
the way the layers were setup this ran before electron setup. not sure if you changed some layer structure so that's no longer the case?
There was a problem hiding this comment.
Yep, that ordering is still preserved. main.ts now calls configureElectronBeforeReady() synchronously at module startup, before building the Effect runtime/layers and before NodeRuntime.runMain.
I kept only the pre-ready Electron requirements there: registerSchemesAsPrivileged plus the Linux command-line switches. The file protocol handler still registers later after whenReady.
|
@juliusmarminge fixed the issues you mentioned and re-smoked it on my end. If you don't want the AGENTS.md changes around Effect (or want different content there) let me know. |
|
I can look at it in a bit, but there's nothing that says that just cause it should run before electron it must run synchronously at module scope? The layer graph before was setup so that the protocol was the first thing that executed before the main effect program? Was there an issue with that? We create the electron app inside the effect program ye? |
Good points; I moved this back into the Effect startup graph as an explicit first layer with |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6c34906. Configure here.






Summary
ready, including--password-store, Wayland/X11 app class, and desktop scheme privileges.DBUS_SESSION_BUS_ADDRESS, so GNOME Keyring/libsecret is reachable when launching outside GNOME.Closes #2331.
Fixes #2539.
Diagnosis
The icon issue came from Linux AppImage builds staging only a single large
icon.png. This PR stages a directory of standard icon sizes (16,22,24,32,48,64,128,256, and512) and points electron-builder at that directory. CI installs ImageMagick for Linux release builds so those sizes can be generated reliably.The credential-store failure came from Electron selecting a non-encrypting Linux
safeStoragebackend when running under desktop environments it does not recognize, such as Niri. The app was also relying on shell/session environment values that may not be present when launched from an AppImage or desktop entry.This PR moves the Linux Electron setup into the synchronous process bootstrap path so it happens before Electron emits
ready, which is required for--password-storeand privileged protocol registration to take effect. It also imports enough login/session environment to reach the user's DBus session bus and falls back to/run/user/$UID/buswhen appropriate.While testing the remote flow, two separate persistence issues showed up:
DateTime.Utcvalues.Scope
This is intentionally focused on Linux desktop/AppImage remote environment reliability. It does not change remote server behavior, and SSH process cleanup after removal remains fire-and-forget so the Settings UI does not hang if disconnect stalls.
Test plan
bun fmtbun lintbun typecheckbun run testbun run --filter @t3tools/desktop test -- DesktopEarlyElectronStartup DesktopEnvironment ElectronProtocol DesktopShellEnvironment linuxSecretStoragebun run --filter @t3tools/desktop test -- DesktopSavedEnvironmentsbun run --filter @t3tools/web test -- localApi service.addSavedEnvironment catalogbun run dist:desktop:linux16,22,24,32,48,64,128,256, and512.T3CODE_HOMEisolated.passwordStore: gnome-libsecret,backend: gnome_libsecret, andencryptionAvailable: true.remote-game, and ran a trivial task on it.remote-game, reinstalled/restarted, and confirmed the saved environment did not come back.Note
Medium Risk
Medium risk: changes Electron pre-
readybootstrap behavior on Linux (command-line switches/protocol privileges) and alters saved-environment credential persistence/error handling, which could affect startup and environment management flows.Overview
Improves Linux desktop/AppImage reliability by running required Electron configuration before
app.ready(scheme privileges, WM class, DBus address fallback, and--password-storeselection based on an early-readlinuxPasswordStoresetting).Hardens saved-environment persistence across desktop IPC and the web runtime: adds a dedicated
removeSavedEnvironmentoperation, makes secret persistence failures return descriptive remediation errors (GNOME Keyring/KWallet) instead of a silentfalse, and refactors web-side rollback/removal so environment records and embedded secrets can’t reappear after deletion.Build/release updates: Linux artifact builds now stage standard hicolor icon sizes using ImageMagick (and CI installs it), and auth contract schemas switch
expiresAtdecoding to accept ISO strings (DateTimeUtcFromString) with updated SSH remote API tests.Reviewed by Cursor Bugbot for commit 0f7c19d. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Improve Linux AppImage icons and remote environment configuration for desktop app
icon.pngwith anicons/directory; installs ImageMagick in CI if absent.linuxPasswordStore) to desktop settings, supportingauto,gnome-libsecret,kwallet,kwallet5, andkwallet6; defaults toautoand applies the correct Electron--password-storeswitch before app ready.DBUS_SESSION_BUS_ADDRESS,DISPLAY,XDG_*, andWAYLAND_DISPLAYfrom the login shell on POSIX systems; falls back to the default runtime bus socket path on Linux when the variable is unset.removeSavedEnvironmentIPC channel end-to-end (preload → IPC handler →DesktopSavedEnvironments→ local API → web service), replacing bearer-token removal with persisted-record deletion during environment removal.expiresAtdecoding in auth contract schemas (AuthBearerBootstrapResult,AuthWebSocketTokenResult,AuthSessionState) fromDateTimeUtctoDateTimeUtcFromStringso ISO string inputs decode correctly.setSecretnow fails withDesktopSavedEnvironmentSecretUnavailableErrorinstead of returningfalsewhen encryption is unavailable on Linux, which is a breaking change for callers that checked the boolean return.Macroscope summarized 0f7c19d.