fix(application/updater): desktop shortcut icon survives updates on linux - #239
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe change moves desktop shortcut creation into a shared helper, adds a Linux migration to repair existing shortcuts, preserves ChangesDesktop shortcut lifecycle
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ElectronRpcApp
participant addToDesktop
participant FileSystem
participant Updater
ElectronRpcApp->>addToDesktop: create desktop shortcut
addToDesktop->>FileSystem: copy favicon.png and write .desktop file
Updater->>FileSystem: remove update entries
Updater->>FileSystem: preserve favicon.png
FileSystem-->>addToDesktop: icon path remains available
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Greptile SummaryThe PR moves Linux desktop-shortcut icons into the durable OpenGameInstaller data directory, repairs existing shortcuts during migration, and preserves icons created by older versions during updater cleanup.
Confidence Score: 4/5The PR appears safe to merge, with a non-blocking test-coverage gap around preservation of shortcuts created by older versions. The durable icon path and migration are consistent with the application’s Linux data-directory and migration lifecycle, but the updater-specific legacy preservation branch is not directly protected by the added regression test. Files Needing Attention: updater/src/main.ts, application/tests/desktop-shortcut.test.ts
|
| Filename | Overview |
|---|---|
| application/src/electron/handlers/helpers.app/desktop-shortcut.ts | Extracts shortcut creation and relocates the copied icon from the disposable working directory to the Linux OGI data directory. |
| application/src/electron/migrations.ts | Adds a Linux migration that repairs an existing shortcut without recreating one the user removed. |
| updater/src/main.ts | Preserves legacy shortcut icons during update-directory cleanup, but this compatibility behavior lacks direct regression coverage. |
| application/tests/desktop-shortcut.test.ts | Verifies that the newly durable icon survives an update-directory wipe but does not exercise preservation of an older update/favicon.png. |
| application/src/electron/handlers/handler.app.ts | Replaces the inline shortcut implementation with an import from the extracted helper. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Linux application startup] --> B{Desktop shortcut exists?}
B -- Yes --> C[Repair migration]
C --> D[Copy icon to OGI data directory]
D --> E[Rewrite desktop Icon path]
B -- No --> F[Leave shortcut absent]
G[Updater cleanup] --> H[Preserve legacy update/favicon.png]
H --> I[Install updated application]
Prompt To Fix All With AI
### Issue 1
updater/src/main.ts:136-142
**Legacy icon preservation is untested**
The regression test creates the icon in the new durable data directory and simulates cleanup using the old three-entry preservation set, so it never exercises the new `favicon.png` compatibility branch. A regression in `prepareUpdateDestination` could therefore delete icons referenced by shortcuts from older versions without failing this test.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix(updater): preserve favicon.png when ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const PRESERVED_UPDATE_ENTRIES = new Set([ | ||
| 'artifacts', | ||
| 'latest.log', | ||
| 'logs', | ||
| 'favicon.png', | ||
| ]); | ||
| const OGI_REPO_URL = 'https://github.com/Nat3z/OpenGameInstaller'; |
There was a problem hiding this comment.
The regression test creates the icon in the new durable data directory and simulates cleanup using the old three-entry preservation set, so it never exercises the new favicon.png compatibility branch. A regression in prepareUpdateDestination could therefore delete icons referenced by shortcuts from older versions without failing this test.
Prompt To Fix With AI
This is a comment left during a code review.
Path: updater/src/main.ts
Line: 136-142
Comment:
**Legacy icon preservation is untested**
The regression test creates the icon in the new durable data directory and simulates cleanup using the old three-entry preservation set, so it never exercises the new `favicon.png` compatibility branch. A regression in `prepareUpdateDestination` could therefore delete icons referenced by shortcuts from older versions without failing this test.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
1 participant
Description
On Linux,
addToDesktop()copied the shortcut icon to./favicon.png— but the app's cwd is the updater'supdate/directory, whichprepareUpdateDestination()wipes on every update. The.desktopfile'sIcon=path then pointed at a deleted file, blanking the shortcut icon.Fixes:
addToDesktopnow copies the icon into the OGI data dir (~/.local/share/OpenGameInstaller/favicon.png), which updates never touch. Moved it out ofhandler.app.tsintohelpers.app/desktop-shortcut.tsso migrations and the RPC handler import a testable leaf module.repair-desktop-shortcut-iconmigration (linux,2.5.0→4.3.0) that rewrites the shortcut with the durable icon path — only if the user still has the.desktopfile.PRESERVED_UPDATE_ENTRIESnow keepsfavicon.png, so shortcuts written by older app versions survive updates too.Regression test recreates the real layout (cwd =
update/, sandboxed home + data dirs), creates the shortcut, simulates the updater wipe, and asserts the icon path still exists. Red before the fix, green after.Example
.desktopnow points at the durable copy:Next Steps
to: 4.3.0bound if the next release is 4.2.10 instead.Exec=paths.Summary by CodeRabbit
New Features
Bug Fixes
Tests