Implement Windows system tray features and design specifications#49
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds generated tray icons, a tray controller, and main-process wiring for Windows/Linux hide-to-tray behavior and unread updates. ChangesWindows/Linux system tray
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR adds Windows and Linux system tray support for the desktop app. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (4): Last reviewed commit: "fix: construct tray prefs store lazily i..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/assets/tray-icons.generated.ts (1)
7-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStrip volatile PNG metadata before embedding.
These data URLs still carry PNG text chunks like
date:create/date:modify, so rerunning the icon generator will rewrite this committed module even when the pixels are unchanged. Strip metadata in the generator before base64-encoding to keep the generated output deterministic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/assets/tray-icons.generated.ts` around lines 7 - 12, The generated tray icon data URLs still include volatile PNG metadata, which makes the committed module change on every regeneration. Update the icon generation flow used for tray-icons.generated.ts so it strips non-image metadata such as date:create and date:modify before base64 encoding, and ensure the generator produces deterministic output for both normal and unread assets.docs/superpowers/specs/2026-06-26-windows-system-tray-design.md (1)
112-118: 🩺 Stability & Availability | 🔵 TrivialPlease attach the Windows/Linux verification results before merge.
This change hardcodes close-to-tray behavior on Windows/Linux, and the spec already notes those paths cannot be exercised on the macOS dev machine. I don't see evidence in this PR that the checklist was actually run, so the highest-risk flows are still unverified.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/specs/2026-06-26-windows-system-tray-design.md` around lines 112 - 118, The verification plan is missing the requested Windows/Linux test evidence, so update the spec section and PR notes to include the actual verification results before merging. Use the “Verification plan” block in the document to record the outcomes for the Windows tray/close-to-tray flows and the Linux smoke test, including whether each checklist item passed. If the results are not yet available, keep the plan explicit about blocking merge until the Windows/Linux checks are attached.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/generate-icons.ts`:
- Around line 165-176: generateTrayIcons() should fail closed instead of only
logging when ImageMagick or file generation fails, because
generateTrayIconModule() can otherwise build a partial TRAY_ICONS set from stale
files. Update the tray icon flow in generateTrayIcons(),
generateTrayIconModule(), and the TRAY_ICONS-producing path so any missing or
failed tray asset generation aborts the build and prevents emitting a module
unless both tray.png and tray-unread.png exist for every required variant.
In `@src/main.ts`:
- Around line 335-341: The main window close handler is still treating
update-driven exits as normal closes because `isQuitting` is not set early
enough, so the tray-hide logic in `mainWindow.on('close')` can block
`autoUpdater.quitAndInstall()`. Update the quit flow by setting `isQuitting`
when `autoUpdater.on('before-quit-for-update')` fires, or immediately before
calling `quitAndInstall()`, so the `close` handler in `src/main.ts` lets the app
exit cleanly during updates.
---
Nitpick comments:
In `@docs/superpowers/specs/2026-06-26-windows-system-tray-design.md`:
- Around line 112-118: The verification plan is missing the requested
Windows/Linux test evidence, so update the spec section and PR notes to include
the actual verification results before merging. Use the “Verification plan”
block in the document to record the outcomes for the Windows tray/close-to-tray
flows and the Linux smoke test, including whether each checklist item passed. If
the results are not yet available, keep the plan explicit about blocking merge
until the Windows/Linux checks are attached.
In `@src/assets/tray-icons.generated.ts`:
- Around line 7-12: The generated tray icon data URLs still include volatile PNG
metadata, which makes the committed module change on every regeneration. Update
the icon generation flow used for tray-icons.generated.ts so it strips non-image
metadata such as date:create and date:modify before base64 encoding, and ensure
the generator produces deterministic output for both normal and unread assets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 71f9dd2c-a0ef-45d1-b343-ed38263f4695
⛔ Files ignored due to path filters (4)
src/assets/icons/dev/tray-unread.pngis excluded by!**/*.pngsrc/assets/icons/dev/tray.pngis excluded by!**/*.pngsrc/assets/icons/prod/tray-unread.pngis excluded by!**/*.pngsrc/assets/icons/prod/tray.pngis excluded by!**/*.png
📒 Files selected for processing (9)
.eslintrc.jsonbiome.jsoncdocs/superpowers/plans/2026-06-26-windows-system-tray.mddocs/superpowers/specs/2026-06-26-windows-system-tray-design.mdpackage.jsonscripts/generate-icons.tssrc/assets/tray-icons.generated.tssrc/main.tssrc/tray.ts
- clearInvalidConfig: true so a corrupt app-prefs file self-heals to defaults instead of throwing at startup - wrap maybeShowTrayHint in try/catch so a best-effort hint can never bubble an error out of the window 'close' handler
- generateTrayIcons() now exits non-zero on an ImageMagick/file failure instead of only logging, so a half-written set can't feed the module - generateTrayIconModule() now requires both tray.png and tray-unread.png for every required variant (dev + prod) and aborts (exit 1) without emitting, instead of silently building a partial/stale TRAY_ICONS
new Store() reads+writes the prefs file on construction, which can throw on a filesystem error (unreadable/locked path, or a path that is a directory). At module load that would crash the main process before any window opens, and clearInvalidConfig only heals malformed content, not filesystem errors. Construct lazily inside maybeShowTrayHint's existing try/catch so such failures disable the hint instead of crashing startup.
Summary by CodeRabbit
typecheckscript and updated lint/build configuration for generated tray icon assets.