Fix update restart card release notes#688
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@copilot review but do not make fixes |
📝 WalkthroughWalkthroughChanges update the desktop auto-update service's release notes URL to use a ChangesRelease Notes URL and Installed Dialog
Estimated code review effort: 2 (Simple) | ~12 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/components/app/AutoUpdateControl.tsx (1)
234-236: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winStatic copy assumes release notes are always available.
This text always says "Review what changed in this release," but when
releaseNotesUrlis null, no link/button is rendered (Lines 238, 256), leaving the user with no way to act on that instruction.✏️ Proposed fix
<p className="mt-4 text-[13px] leading-6 text-muted-fg"> - The update is installed and ADE is ready again. Review what changed in this release. + The update is installed and ADE is ready again. + {releaseNotesUrl ? " Review what changed in this release." : ""} </p>🤖 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 `@apps/desktop/src/renderer/components/app/AutoUpdateControl.tsx` around lines 234 - 236, The success copy in AutoUpdateControl assumes release notes are always available, but releaseNotesUrl can be null so the instruction becomes misleading. Update the text rendered in the installed/success state to be conditional on releaseNotesUrl, and in the AutoUpdateControl component align it with the existing conditional rendering for the link/button so users only see “review what changed” when an action is actually available.
🤖 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.
Nitpick comments:
In `@apps/desktop/src/renderer/components/app/AutoUpdateControl.tsx`:
- Around line 234-236: The success copy in AutoUpdateControl assumes release
notes are always available, but releaseNotesUrl can be null so the instruction
becomes misleading. Update the text rendered in the installed/success state to
be conditional on releaseNotesUrl, and in the AutoUpdateControl component align
it with the existing conditional rendering for the link/button so users only see
“review what changed” when an action is actually available.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ba16485b-2d93-456c-bdb0-12199a6d4dc0
📒 Files selected for processing (4)
apps/desktop/src/main/services/updates/autoUpdateService.test.tsapps/desktop/src/main/services/updates/autoUpdateService.tsapps/desktop/src/renderer/components/app/AutoUpdateControl.tsxapps/desktop/src/renderer/index.css
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
Describe the change.
What Changed
Key files and behaviors.
Validation
How you tested.
Risks
Anything to watch.
Summary by CodeRabbit
New Features
Bug Fixes
Greptile Summary
This PR updates the auto-update release notes path and refreshes the installed-update dialog. The main changes are:
Confidence Score: 5/5
Safe to merge with minimal risk.
The changes are narrow and covered by matching test updates. No blocking correctness or security issues were identified.
No files require special attention.
What T-Rex did
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Updater as AutoUpdateService participant State as Global state participant Renderer as AutoUpdateControl participant Browser as External browser Updater->>Updater: buildReleaseNotesUrl(version) Updater->>State: persist pendingInstallUpdate Updater->>State: reconcile after restart Updater-->>Renderer: snapshot.recentlyInstalled Renderer->>Renderer: show installed-update card Renderer->>Browser: openExternal(releaseNotesUrl)%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Updater as AutoUpdateService participant State as Global state participant Renderer as AutoUpdateControl participant Browser as External browser Updater->>Updater: buildReleaseNotesUrl(version) Updater->>State: persist pendingInstallUpdate Updater->>State: reconcile after restart Updater-->>Renderer: snapshot.recentlyInstalled Renderer->>Renderer: show installed-update card Renderer->>Browser: openExternal(releaseNotesUrl)Comments Outside Diff (1)
General comment
AutoUpdateControl.tsximportsArrowsClockwisefrom@phosphor-icons/react, but the installed package does not export that symbol. The browser aborts module evaluation withdoes not provide an export named 'ArrowsClockwise', so the installed-update dialog never appears and none of the claimed refreshed card UI can be verified or delivered.apps/desktop/src/renderer/components/app/AutoUpdateControl.tsximportsArrowsClockwise; the installed@phosphor-icons/reactpackage exposesArrowClockwiseinstead.import { ArrowClockwise, ArrowSquareOut, CheckCircle, X } from '@phosphor-icons/react';and render<ArrowClockwise ... />, or upgrade/pin the icon package to a version that actually exportsArrowsClockwise.Reviews (2): Last reviewed commit: "ship: iteration 1 - address review copy" | Re-trigger Greptile