Add Per os download section#3784
Conversation
|
Warning Review limit reached
Next review available in: 33 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR centralizes platform detection, adds a latest-release GitHub API route, and refactors the desktop download page into per-OS cards with a Linux install command. It also updates login, sidebar, and social auth desktop CTAs to use the shared hook and generic “Get Desktop app” wording. ChangesPlatform detection and desktop download flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant DownloadHero
participant LatestReleaseRoute
participant GitHubAPI
DownloadHero->>LatestReleaseRoute: GET /api/latest-release
LatestReleaseRoute->>GitHubAPI: fetch releases (optional GITHUB_TOKEN)
GitHubAPI-->>LatestReleaseRoute: releases + assets
LatestReleaseRoute->>LatestReleaseRoute: find desktop release, match asset URLs
LatestReleaseRoute-->>DownloadHero: JSON download URLs or error (404/502/500)
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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 |
|
New azure vertex changes available for preview here |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@src/vertex/app/api/latest-release/route.ts`:
- Around line 62-64: The catch block in the latest-release route is swallowing
the underlying failure, so update the handler to bind the thrown error and log
it before returning the 500 response. Use the route handler in route.ts and the
surrounding try/catch around the GitHub release fetch/parse logic to emit a
meaningful error message with the actual exception details, then keep the
existing NextResponse.json fallback.
In `@src/vertex/app/changelog.md`:
- Around line 59-70: The release notes heading is out of sync with the actual
list length in the changelog summary. Update the “Files Modified & Added” count
in the changelog section so it matches the nine listed entries, keeping the
heading and the `src/vertex/app/changelog.md` summary consistent.
In `@src/vertex/components/features/download/DownloadHero.tsx`:
- Around line 31-35: `handleCopy` in `CopyButton` should handle
`navigator.clipboard.writeText` failures instead of letting the promise reject
unhandled. Wrap the clipboard call in a try/catch, keep `setCopied(true)` and
the reset timeout only on success, and on failure leave the copied state false
and surface a user-facing fallback (for example via an existing toast/error
message path if available). Ensure the fix is applied in the `handleCopy` logic
used by `DownloadHero`.
- Around line 48-59: The macOS CTA in getPrimaryRelease always targets
release.mac.arm64Dmg, so Intel Macs are routed to the ARM build. Update the
DownloadHero flow so macOS users are handled with architecture awareness by
using getMacArchitecture() before choosing the primary href, or fall back to a
`#releases` CTA for macOS in getPrimaryRelease while keeping useDetectedPlatform()
as the platform entry point.
🪄 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
Run ID: cd5d0b84-0a42-499a-9df1-5c07d1ff8071
📒 Files selected for processing (11)
src/vertex/app/api/latest-release/route.tssrc/vertex/app/changelog.mdsrc/vertex/app/download/page.tsxsrc/vertex/app/login/page.tsxsrc/vertex/components/features/auth/social-auth-section.tsxsrc/vertex/components/features/download/DownloadHero.tsxsrc/vertex/components/features/download/PlatformInfo.tsxsrc/vertex/components/layout/secondary-sidebar.tsxsrc/vertex/core/constants/app-downloads.tssrc/vertex/core/hooks/useDetectedPlatform.tssrc/vertex/core/utils/platform.ts
| } catch { | ||
| return NextResponse.json({ error: 'Internal error' }, { status: 500 }); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Log errors in the catch block before returning 500.
The catch block silently swallows the error with no logging. In production, if this route starts returning 500s, there's no way to diagnose the root cause (e.g., GitHub API outage, parse failure, network timeout). Bind the error and log it.
🔧 Proposed fix
- } catch {
+ } catch (error) {
+ console.error('[latest-release] Failed to fetch or parse GitHub releases:', error);
return NextResponse.json({ error: 'Internal error' }, { status: 500 });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch { | |
| return NextResponse.json({ error: 'Internal error' }, { status: 500 }); | |
| } | |
| } catch (error) { | |
| console.error('[latest-release] Failed to fetch or parse GitHub releases:', error); | |
| return NextResponse.json({ error: 'Internal error' }, { status: 500 }); | |
| } |
🤖 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/vertex/app/api/latest-release/route.ts` around lines 62 - 64, The catch
block in the latest-release route is swallowing the underlying failure, so
update the handler to bind the thrown error and log it before returning the 500
response. Use the route handler in route.ts and the surrounding try/catch around
the GitHub release fetch/parse logic to emit a meaningful error message with the
actual exception details, then keep the existing NextResponse.json fallback.
| <summary><strong>Files Modified & Added (8)</strong></summary> | ||
|
|
||
| - `src/vertex/app/api/latest-release/route.ts` [NEW] | ||
| - `src/vertex/core/hooks/useDetectedPlatform.ts` [NEW] | ||
| - `src/vertex/core/utils/platform.ts` [MODIFIED] | ||
| - `src/vertex/core/constants/app-downloads.ts` [MODIFIED] | ||
| - `src/vertex/components/features/download/DownloadHero.tsx` [MODIFIED] | ||
| - `src/vertex/components/features/download/PlatformInfo.tsx` [MODIFIED] | ||
| - `src/vertex/app/download/page.tsx` [MODIFIED] | ||
| - `src/vertex/components/layout/secondary-sidebar.tsx` [MODIFIED] | ||
| - `src/vertex/app/login/page.tsx` [MODIFIED] | ||
| - `src/vertex/components/features/auth/social-auth-section.tsx` [MODIFIED] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the file-count heading.
“Files Modified & Added (8)” is no longer accurate: the list below contains 9 entries. Please update the count so the release notes stay consistent.
🤖 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/vertex/app/changelog.md` around lines 59 - 70, The release notes heading
is out of sync with the actual list length in the changelog summary. Update the
“Files Modified & Added” count in the changelog section so it matches the nine
listed entries, keeping the heading and the `src/vertex/app/changelog.md`
summary consistent.
There was a problem hiding this comment.
Pull request overview
This PR enhances the Vertex Desktop download experience by adding OS-aware download UX on /download, dynamically sourcing installer assets from the latest GitHub Release via a new Next.js route handler, and centralizing OS/Electron detection to remove duplicated user-agent checks across the app.
Changes:
- Added
/api/latest-releaseroute handler (ISR-cached) to fetch and parse latest desktop release assets from GitHub. - Introduced centralized platform detection (
getDetectedPlatform/getIsElectron) plus a newuseDetectedPlatformhook and replaced duplicated UA logic in sidebar/login/social auth. - Redesigned
/downloadhero + added per-OS release cards (including Linux terminal install snippet + copy button) and fixed scrolling via a local scroll container.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vertex/app/api/latest-release/route.ts | New ISR-cached route handler that fetches/parses latest desktop release assets from GitHub. |
| src/vertex/core/utils/platform.ts | Adds centralized platform + Electron detection utilities. |
| src/vertex/core/hooks/useDetectedPlatform.ts | New client hook wrapping platform/Electron detection to avoid SSR mismatches. |
| src/vertex/core/constants/app-downloads.ts | Replaces single Windows URL with structured cross-platform fallback URLs + Linux install command constant. |
| src/vertex/components/features/download/DownloadHero.tsx | Implements OS-aware primary CTA, fetches latest release, renders per-platform cards and Linux terminal copy UX. |
| src/vertex/app/download/page.tsx | Updates metadata + makes page independently scrollable (h-screen overflow-y-auto). |
| src/vertex/components/features/download/PlatformInfo.tsx | Aligns section background with theme tokens (bg-background). |
| src/vertex/components/layout/secondary-sidebar.tsx | Replaces inline UA detection; shows “Get Desktop app” CTA for non-Electron users. |
| src/vertex/app/login/page.tsx | Replaces inline UA detection; shows “Get Desktop app” CTA for non-Electron users. |
| src/vertex/components/features/auth/social-auth-section.tsx | Replaces inline Electron UA check with useDetectedPlatform. |
| src/vertex/app/changelog.md | Documents the new download UX + API route handler and related changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export function getDetectedPlatform(): DetectedPlatform { | ||
| if (typeof window === 'undefined') return 'other' | ||
| const ua = window.navigator.userAgent.toLowerCase() | ||
| if (ua.includes('win')) return 'win' | ||
| if (ua.includes('mac')) return 'mac' | ||
| if (ua.includes('linux')) return 'linux' | ||
| return 'other' | ||
| } |
| const handleCopy = async () => { | ||
| await navigator.clipboard.writeText(text); | ||
| setCopied(true); | ||
| setTimeout(() => setCopied(false), 2000); | ||
| }; |
| case 'mac': | ||
| return { label: 'Download for macOS', href: release.mac.arm64Dmg, Icon: AppleIcon }; | ||
| case 'linux': |
| </details> | ||
|
|
||
| <details> | ||
| <summary><strong>Files Modified & Added (8)</strong></summary> |
…acOS arch for primary download
|
New azure vertex changes available for preview here |
Codecov Report❌ Patch coverage is Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. @@ Coverage Diff @@
## staging #3784 +/- ##
===========================================
- Coverage 53.30% 53.16% -0.15%
===========================================
Files 112 112
Lines 4523 4535 +12
Branches 1476 1481 +5
===========================================
Hits 2411 2411
- Misses 2112 2124 +12 🚀 New features to boost your workflow:
|
Codebmk
left a comment
There was a problem hiding this comment.
Very nice! One thing, please resolve the lighthouse issues (check failed lighthouse workflow at bottom of PR) @BwanikaRobert
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/vertex/components/features/download/DownloadHero.tsx (2)
224-237: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
target="_blank"andrel="noopener noreferrer"to platform card download links for consistency with the primary CTA.The primary button conditionally opens http hrefs in a new tab (line 131-132), but the per-platform card links don't. While GitHub release assets typically set
Content-Disposition: attachment(so downloads work regardless), adding these attributes is safer and consistent.♻️ Proposed fix
<a key={linkLabel + suffix} href={href} + target="_blank" + rel="noopener noreferrer" className="flex items-center justify-between rounded-md border border-border px-3 py-2 text-sm hover:bg-muted/50 hover:border-primary/30 transition-colors group" download >🤖 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/vertex/components/features/download/DownloadHero.tsx` around lines 224 - 237, The per-platform download links in DownloadHero’s card list should match the primary CTA behavior by opening http(s) links in a new tab. Update the anchor used for each platform card so it includes target="_blank" and rel="noopener noreferrer" alongside the existing download attribute, keeping the behavior consistent and safe for external release asset URLs.
52-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueType
platformasDetectedPlatforminstead ofstring.
useDetectedPlatformalready returns a typedDetectedPlatform, butgetPrimaryReleasewidens it tostring, losing compile-time safety on the switch cases. Import and use the narrower type.♻️ Proposed fix
-function getPrimaryRelease(platform: string, release: DesktopReleaseUrls) { +function getPrimaryRelease(platform: DetectedPlatform, release: DesktopReleaseUrls) {🤖 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/vertex/components/features/download/DownloadHero.tsx` at line 52, The getPrimaryRelease helper currently widens the platform argument to string, which drops the type safety already provided by useDetectedPlatform. Update the getPrimaryRelease signature in DownloadHero to accept DetectedPlatform instead, and add the corresponding import so the switch cases remain exhaustively checked against the narrower platform type.
🤖 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 `@src/vertex/components/features/download/DownloadHero.tsx`:
- Around line 224-237: The per-platform download links in DownloadHero’s card
list should match the primary CTA behavior by opening http(s) links in a new
tab. Update the anchor used for each platform card so it includes
target="_blank" and rel="noopener noreferrer" alongside the existing download
attribute, keeping the behavior consistent and safe for external release asset
URLs.
- Line 52: The getPrimaryRelease helper currently widens the platform argument
to string, which drops the type safety already provided by useDetectedPlatform.
Update the getPrimaryRelease signature in DownloadHero to accept
DetectedPlatform instead, and add the corresponding import so the switch cases
remain exhaustively checked against the narrower platform type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 67e52a14-3c8f-4999-b6b1-c2056d9f4ea0
📒 Files selected for processing (1)
src/vertex/components/features/download/DownloadHero.tsx
|
New azure vertex changes available for preview here |
cd1599b to
82d7d0b
Compare
|
New azure vertex changes available for preview here |
| "path": ".next/static/chunks/app/**/*.js", | ||
| "limit": "200 kB" | ||
| "limit": "205 kB" | ||
| } |
There was a problem hiding this comment.
Kindly justify reason for increasing limit instead of optimising file size @BwanikaRobert
There was a problem hiding this comment.
Kindly justify reason for increasing limit instead of optimising file size @BwanikaRobert
Seen in changelog, awesome move
Summary of Changes (What does this PR do?)
/downloadthat detects the visitor's OS and links to the correct release asset (Windows.exe, macOS Apple Silicon.dmg, Linux.AppImage)./api/latest-release) that fetches the latest Vertex Desktop release from GitHub automatically, cached via ISR (1 hr) — no more hardcoded version URLs.core/utils/platform.ts+ a newuseDetectedPlatformhook, replacing inlineuserAgentchecks across sidebar, login page, and social auth./downloadpage not being scrollable (caused byglobals.csssettinghtml { overflow: hidden }globally).Status of maturity (all need to be checked before merging):
How should this be manually tested?
npm run devfromsrc/vertex./download— verify the page scrolls and the primary button label matches your OS./api/latest-releaseshould return live GitHub data).platforminuseDetectedPlatformto'win','mac','linux'./download— confirm fallback URLs still render (v0.1.11).What are the relevant tickets?
Screenshots (optional)
Summary by CodeRabbit