chore(www,ci): drop unsupported mac builds from downloads, template release notes#732
Conversation
…elease notes Two related cleanups now that macOS builds are arm64-only (PR #731): **1. Downloads page — remove no-longer-built macOS variants.** - `editor/app/(www)/(downloads)/downloads/downloads.ts` - Drop `mac_dmg_x64` and `mac_dmg_universal` from the DownloadLinks interface and from `getLinks` / `getLinks_v001`. - Drop `"universal"` from the Arch union; tighten the mac distro config to arm64 only. - Switch the per-platform default for `"mac"` from `mac_dmg_universal` → `mac_dmg_arm64`. - `editor/app/(www)/(downloads)/downloads/page.tsx` - Remove the "macOS (Universal)" and "macOS (Intel-based Macs)" buttons. Keep "macOS (Apple Silicon)". The v0.0.1 release still has the legacy universal/x64 dmg assets so existing direct links keep working; the page just doesn't surface them. New releases won't include those artifacts. **2. Release notes templating.** GitHub Releases have no built-in body template, and forge's publisher-github doesn't expose `releaseNotes`. New releases land with an empty body unless we templatize it ourselves. - New file `.github/release-notes/desktop.md` — the canonical body for desktop releases, with a `{{version}}` placeholder. Lives in `.github/` because it's release-process content, not desktop app source. - New step `Update release notes from template` in the macOS row of the release workflow. After publish creates the tag, the step interpolates the template and applies it via `gh release edit`. Runs once per release (gated to the macOS matrix row).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
6 Skipped Deployments
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAligns desktop downloads to macOS arm64-only: adds release notes and a workflow step, updates the download types and retrieval logic (including memoization), adds a cached page helper, prunes legacy static links, and updates the UI to show only Apple Silicon downloads. ChangesmacOS arm64-only downloads alignment
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: deac3c2788
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| - name: Update release notes from template | ||
| # Runs once (only on the macOS row) after the release tag has been | ||
| # created by the publisher. Template at .github/release-notes/desktop.md. | ||
| if: ${{ matrix.os.name == 'macos-apple-silicon' && env.HAS_SIGNING == 'true' && env.HAS_NOTARIZE == 'true' }} |
There was a problem hiding this comment.
The new release-notes step is conditioned on env.HAS_SIGNING == 'true' && env.HAS_NOTARIZE == 'true', so if Apple signing/notarization secrets are missing or temporarily unavailable, the Windows/Linux publish steps can still run and publish a release while this templating step is skipped. In that case the workflow reverts to empty/manual release bodies, which defeats the automation introduced here; this step should be gated only to a single matrix row (or another always-running path), not to mac signing availability.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — applied in e207677. Re-gated the step to matrix.os.name == 'linux' (no signing-secret dependency, always runs). The release tag exists by the time linux's publish step finishes, so the body edit is safe.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: deac3c2788
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| - name: Update release notes from template | ||
| # Runs once (only on the macOS row) after the release tag has been | ||
| # created by the publisher. Template at .github/release-notes/desktop.md. | ||
| if: ${{ matrix.os.name == 'macos-apple-silicon' && env.HAS_SIGNING == 'true' && env.HAS_NOTARIZE == 'true' }} |
There was a problem hiding this comment.
The new release-notes step is conditioned on env.HAS_SIGNING == 'true' && env.HAS_NOTARIZE == 'true', so if Apple signing/notarization secrets are missing or temporarily unavailable, the Windows/Linux publish steps can still run and publish a release while this templating step is skipped. In that case the workflow reverts to empty/manual release bodies, which defeats the automation introduced here; this step should be gated only to a single matrix row (or another always-running path), not to mac signing availability.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — applied in e207677. Re-gated the step to matrix.os.name == 'linux' (no signing-secret dependency, always runs). The release tag exists by the time linux's publish step finishes, so the body edit is safe. (duplicate thread — same fix applies)
The downloads page was using `getLinks_v001(os)`, which hardcoded every asset URL to v0.0.1. Every new release would have left the page stuck on the old version. - Add `getLinksForPage(os)` that wraps `getLinks()` in Next.js `unstable_cache` with a 1h `revalidate`. Hits GitHub's unauthenticated /releases/latest endpoint at most once per hour per region, well under the 60 req/hr/IP limit. `unstable_cache` only memoizes successful resolutions; transient API blips self-heal on the next render. - Add `pickDefault(os, links)` helper that derives the per-OS default button target from the dynamic links — mac→arm64, win→x64, linux→deb x64. - Fall back to `getLinks_v001(os)` when the API is unreachable, so a GitHub outage doesn't break the page (stale URLs are better than no URLs). - Mark `getLinks_v001` deprecated comment more clearly: static fallback only, prefer `getLinksForPage`. - Switch `page.tsx` to call the new helper.
Two review fixes: 1. Address Codex P2 (gh#732 comments 3275230142 / 3275230335). The release-notes update step was gated on `env.HAS_SIGNING && env.HAS_NOTARIZE`, which meant that if Apple secrets were ever unavailable, linux/windows would still publish a release while the notes step silently skipped — defeating the automation. Re-gate to the linux row (which has no signing dependency and always runs). The release tag exists by the time linux's publish step finishes, so we can safely edit its body. 2. Apply oxfmt to the release-notes template — fixes a CI check failure on a trailing-space / `*not*` → `_not_` style nit.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
editor/app/(www)/(downloads)/downloads/downloads.ts (1)
176-177: ⚡ Quick winAdd telemetry on fallback path.
The empty catch makes persistent API failures invisible while serving static v0.0.1 links. Add at least a warning/error log before fallback.
🤖 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 `@editor/app/`(www)/(downloads)/downloads/downloads.ts around lines 176 - 177, The empty catch swallowing errors around the dynamic links fetch should be replaced with telemetry and logging before returning the fallback from getLinks_v001(os); modify the catch in downloads.ts (the block that currently does "catch { return getLinks_v001(os); }") to log a warning/error with the caught error details (e.g., processLogger.warn/error or similar) and emit a telemetry event/metric indicating the fallback was used (include context like function name, os, and error.message) and then return getLinks_v001(os).
🤖 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 `@editor/app/`(www)/(downloads)/downloads/downloads.ts:
- Around line 158-160: getCachedLinks wraps getLinks but a cache miss still
triggers multiple GitHub API calls because getLinks calls
getLatestRelease/getAsset per asset; fix by ensuring getLinks only calls
getLatestRelease once and reuses that release data when resolving assets (or
memoize getAsset calls inside getLinks) so a single cache miss results in one
getLatestRelease and then local processing (use Promise.all on assets derived
from the single release object rather than calling getLatestRelease/getAsset
repeatedly). Update code references: unstable_cache(getCachedLinks), function
getLinks, helper getLatestRelease and getAsset to implement single-release fetch
and asset-level reuse/memoization.
---
Nitpick comments:
In `@editor/app/`(www)/(downloads)/downloads/downloads.ts:
- Around line 176-177: The empty catch swallowing errors around the dynamic
links fetch should be replaced with telemetry and logging before returning the
fallback from getLinks_v001(os); modify the catch in downloads.ts (the block
that currently does "catch { return getLinks_v001(os); }") to log a
warning/error with the caught error details (e.g., processLogger.warn/error or
similar) and emit a telemetry event/metric indicating the fallback was used
(include context like function name, os, and error.message) and then return
getLinks_v001(os).
🪄 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: 23f51cb8-2861-422f-b24f-ba4855008add
📒 Files selected for processing (2)
editor/app/(www)/(downloads)/downloads/downloads.tseditor/app/(www)/(downloads)/downloads/page.tsx
Address CodeRabbit on PR #732: 1. Fetcher.fetch() was unconditional — each `getAsset` call re-hit the GitHub /releases/latest endpoint. With 6 assets to resolve per cache miss in getCachedLinks, that was 6 API calls against the 60/hr/IP unauthenticated limit, not 1. Guard `fetch()` to return the cached release if already loaded. Within-render now caps at one API call; `unstable_cache` already caps cross-render. 2. The silent `catch { fallback }` masked persistent API failures (rate-limit, GitHub outage). Add `console.warn` with the caught error so the failure mode is visible in server logs.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@editor/app/`(www)/(downloads)/downloads/downloads.ts:
- Around line 290-294: The memoization check using this.m_assets still allows
repeated GitHub retries when fetchRelease() fails because this.m_assets stays
null; update getLinks()/the fetchRelease() error path to set this.m_assets to a
stable sentinel (e.g., an empty array or a dedicated failure value) immediately
on failure so subsequent calls short-circuit and do not re-call
getAsset()/GitHub; ensure you only overwrite that sentinel with real assets on a
successful fetch so that getLinks(), getAsset(), and getCachedLinks() find the
memoized value even after errors.
🪄 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: 995a0eea-e11c-4712-86a8-9ca3a78133bd
📒 Files selected for processing (3)
.github/release-notes/desktop.md.github/workflows/realease-desktop-app.ymleditor/app/(www)/(downloads)/downloads/downloads.ts
✅ Files skipped from review due to trivial changes (1)
- .github/release-notes/desktop.md
…er cache miss CodeRabbit follow-up: previous commit only memoized successful fetches. On API failure, m_assets stayed null, so each of the 6 sequential getAsset calls in getLinks() would retry the API — up to 6 hits per failed cache miss. Cache the failure as an empty array sentinel. The empty-array path flows through getAssetsByPlatform's filter → returns [] → getAsset's assert(length === 1) fails → caught → returns null. Subsequent getAsset calls see the cached [] and bypass the API entirely. Net: at most 1 API call per cache miss, regardless of whether it succeeded. A fresh Fetcher is created per render (per getLinks call), so the error cache is per-render and transient failures self-heal on the next request.
1 participant
Two related cleanups now that macOS builds are arm64-only (#731).
Summary
1. Downloads page
The
/downloadspage advertised macOS Universal and macOS Intel builds that we no longer produce. Removed those references end-to-end:downloads.ts— dropmac_dmg_x64/mac_dmg_universalfrom theDownloadLinksinterface and from bothgetLinksandgetLinks_v001. Drop"universal"from theArchunion. Switch the per-platform default for"mac"frommac_dmg_universal→mac_dmg_arm64.page.tsx— remove the "macOS (Universal)" and "macOS (Intel-based Macs)" buttons. Keep "macOS (Apple Silicon)".The legacy
v0.0.1release still has the universal/x64 dmg artifacts so any direct download links out there keep working; the page just doesn't surface them anymore. New releases won't include those artifacts.2. Release notes templating
GitHub Releases have no built-in body template, and forge's
publisher-githubdoesn't exposereleaseNotes. New tags land with empty bodies unless templatized externally — which means the neat Markdown table currently onv0.0.1would have to be re-authored by hand each release..github/release-notes/desktop.md— canonical body for desktop releases, with a{{version}}placeholder. Lives in.github/because it's release-process content, not desktop app source.Update release notes from templatein.github/workflows/realease-desktop-app.yml. Runs once per release (gated to the macOS matrix row, which is the slowest job and so effectively the "end" of the release). Interpolates{{version}}fromdesktop/package.jsonand applies the body viagh release edit.Test plan
mac_dmg_x64ormac_dmg_universalper grep).Summary by CodeRabbit
New Features
Changes