Repository navigation
Add release candidate app channel - #4512
Closed
lawrencecchen wants to merge 18 commits into
Closed
lawrencecchen wants to merge 18 commits into
lawrencecchen wants to merge 18 commits into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds RC channel support across CI/workflows, app icon tooling, entitlements, channel metadata verification and tests, socket-variant/runtime changes, auth deep-link scheme, and promotion automation to publish RC artifacts as stable releases. ChangesRC Build and Release Infrastructure
Sequence DiagramsequenceDiagram
participant User
participant RCWf as "RC Build Workflow"
participant GH as "GitHub API/Actions"
participant Xcode as "Xcode Build"
participant Sign as "Signing/Notary"
participant R2 as "Cloudflare R2"
participant PromWf as "Promotion Workflow"
User->>RCWf: Trigger with version, rc_number, commit
RCWf->>RCWf: validate inputs, resolve tags
RCWf->>GH: fetch RC tag, guard existing RC assets
RCWf->>Xcode: build universal app, verify architectures
RCWf->>Sign: import certs, embed profiles, codesign, notarize
RCWf->>GH: publish RC prerelease with artifacts
RCWf->>R2: upload RC appcast.xml (when latest)
User->>PromWf: promote RC tag to stable
PromWf->>GH: guard existing stable release assets
PromWf->>GH: download stable-candidate assets from RC, rename and upload
PromWf->>R2: upload stable appcast.xml (when applicable)
PromWf->>GH: trigger Homebrew update
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 16 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (16 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 @.github/workflows/promote-release-candidate.yml:
- Around line 172-175: The "Upload stable appcast to R2" job step currently sets
continue-on-error: true which allows the workflow to proceed even if the stable
appcast upload fails; remove the continue-on-error: true setting (or set it to
false) in the step named "Upload stable appcast to R2" so that any failure in
the upload will fail the workflow and prevent a stable release from publishing
without a successful stable feed upload; optionally add a retry mechanism or
explicit error handling around the upload command if transient failures are a
concern.
- Around line 28-33: The run step is unsafe because it expands ${{ inputs.rc_tag
}} directly into the shell and the appcast upload step is allowed to fail; to
fix, move the input into an environment variable (declare env: RC_TAG: ${{
inputs.rc_tag }} on the job or step and then use RC_TAG inside the script) so
the raw input is not interpolated into the script before validation, update the
script to validate the RC_TAG variable (the existing RC_TAG regex guard) and
ensure any assignment uses the shell variable (RC_TAG) rather than expanding the
workflow expression inline, and remove or set continue-on-error: false on the
"Upload stable appcast to R2" step so appcast upload failures stop the release
promotion instead of proceeding.
In @.github/workflows/release-candidate.yml:
- Around line 50-58: The workflow step named "Validate release candidate inputs"
currently interpolates inputs inside the bash run script (VERSION="${{
inputs.version }}", RC_NUMBER="${{ inputs.rc_number }}" and echoing
candidate_ref into GITHUB_STEP_SUMMARY), which risks shell expansion of
malicious input; instead map inputs to environment variables in the step's env
block (e.g., VERSION, RC_NUMBER, CANDIDATE_REF) and reference them in the run
script with plain shell variables ($VERSION, $RC_NUMBER, $CANDIDATE_REF), then
use the env variable when writing to GITHUB_STEP_SUMMARY so no ${ { inputs.* } }
interpolation occurs inside the shell.
In `@scripts/generate_rc_icon.py`:
- Around line 104-116: The loop currently skips missing source files (src_path)
and prints a misleading summary using len(SIZES); change this to fail fast and
report the real generated count: when the code detects not
os.path.exists(src_path) for a given filename, print an error with the filename
and exit non‑zero (e.g., sys.exit(1)) instead of continuing; introduce and
increment a generated_count variable where recolor_banner(img).save(...) is
called and use that generated_count in the final print that references DST_DIR
so the summary reflects actual outputs rather than len(SIZES).
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 004d8dbe-ba43-4917-bfd1-b9a84b6ce666
⛔ Files ignored due to path filters (10)
Assets.xcassets/AppIcon-RC.appiconset/128.pngis excluded by!**/*.pngAssets.xcassets/AppIcon-RC.appiconset/128@2x.pngis excluded by!**/*.pngAssets.xcassets/AppIcon-RC.appiconset/16.pngis excluded by!**/*.pngAssets.xcassets/AppIcon-RC.appiconset/16@2x.pngis excluded by!**/*.pngAssets.xcassets/AppIcon-RC.appiconset/256.pngis excluded by!**/*.pngAssets.xcassets/AppIcon-RC.appiconset/256@2x.pngis excluded by!**/*.pngAssets.xcassets/AppIcon-RC.appiconset/32.pngis excluded by!**/*.pngAssets.xcassets/AppIcon-RC.appiconset/32@2x.pngis excluded by!**/*.pngAssets.xcassets/AppIcon-RC.appiconset/512.pngis excluded by!**/*.pngAssets.xcassets/AppIcon-RC.appiconset/512@2x.pngis excluded by!**/*.png
📒 Files selected for processing (9)
.github/workflows/ci.yml.github/workflows/promote-release-candidate.yml.github/workflows/release-candidate.ymlAssets.xcassets/AppIcon-RC.appiconset/Contents.jsoncmux.rc.entitlementsscripts/compile-app-icon-icns.shscripts/generate_rc_icon.pyscripts/verify-app-bundle-channel-metadata.shtests/test_verify_app_bundle_channel_metadata.sh
Contributor
Greptile SummaryAdds a macOS release-candidate distribution channel: a new
Confidence Score: 5/5Safe to merge. The RC channel is fully additive — new bundle ID, URL scheme, socket paths, and workflows leave the existing stable and nightly channels untouched. All changed Swift paths correctly thread the RC variant through actor-safe, nonisolated enum dispatches that mirror the established nightly/staging patterns. The workflow changes use env: indirection for dispatch inputs, enforce immutable-asset guards, and the attestation signerWorkflow embedded in stable-candidate manifests correctly points to release-candidate.yml (the workflow that created the attestations). The socket-resolver ordering change is well-tested with both Swift unit tests and the new Python integration tests. No files require special attention. Important Files Changed
Reviews (13): Last reviewed commit: "fix: keep rc tags out of stable release ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 19 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/promote-release-candidate.yml (1)
162-179:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't turn release-list lookup failures into
--latest.Line 162 ignores
gh release listerrors, so a transient GitHub API/auth failure leavesEXISTING_STABLE_TAGSempty and makes Lines 164-179 publish this release with--latest. That can incorrectly move the stable "Latest" badge to an older backfill release.Suggested patch
- EXISTING_STABLE_TAGS="$(gh release list --repo "$GITHUB_REPOSITORY" --exclude-drafts --exclude-pre-releases \ - --json tagName -q '.[].tagName' || true)" + EXISTING_STABLE_TAGS="$(gh release list --repo "$GITHUB_REPOSITORY" --exclude-drafts --exclude-pre-releases \ + --json tagName -q '.[].tagName')"🤖 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 @.github/workflows/promote-release-candidate.yml around lines 162 - 179, gh release list is being silenced with "|| true" so failures produce an empty EXISTING_STABLE_TAGS and can mistakenly make LATEST_FLAG=(--latest) for the new release; update the logic around the gh release list call (EXISTING_STABLE_TAGS) to detect non-zero exit status from gh release list (or capture stderr), and on failure either abort (exit non-zero with a clear error) or explicitly keep LATEST_FLAG=(--latest=false) instead of allowing it to flip to --latest; ensure the subsequent LATEST_STABLE_TAG / LATEST_FLAG decision uses that failure detection so transient API/auth errors do not mark a backfill as latest.
🤖 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 @.github/workflows/release-candidate.yml:
- Around line 698-706: The script currently treats a failed `gh release list` as
an empty LATEST_RC and continues with the R2 upload, risking overwriting the
appcast; change the LATEST_RC assignment and surrounding block so that if `gh
release list --repo "$GITHUB_REPOSITORY" ...` fails (non-zero exit) the script
exits non-zero instead of proceeding. Concretely, run the gh command and check
its exit status (or use `|| { echo "Failed to list releases"; exit 1; }`) when
populating LATEST_RC, and only proceed to the RC upload when the command
succeeded and LATEST_RC is set and equals RC_TAG; update the block referencing
LATEST_RC and RC_TAG accordingly.
---
Outside diff comments:
In @.github/workflows/promote-release-candidate.yml:
- Around line 162-179: gh release list is being silenced with "|| true" so
failures produce an empty EXISTING_STABLE_TAGS and can mistakenly make
LATEST_FLAG=(--latest) for the new release; update the logic around the gh
release list call (EXISTING_STABLE_TAGS) to detect non-zero exit status from gh
release list (or capture stderr), and on failure either abort (exit non-zero
with a clear error) or explicitly keep LATEST_FLAG=(--latest=false) instead of
allowing it to flip to --latest; ensure the subsequent LATEST_STABLE_TAG /
LATEST_FLAG decision uses that failure detection so transient API/auth errors do
not mark a backfill as latest.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4e714cd1-6c1f-43a3-ab3c-01d99f101441
📒 Files selected for processing (2)
.github/workflows/promote-release-candidate.yml.github/workflows/release-candidate.yml
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 `@tests/test_cli_socket_autodiscovery.py`:
- Line 264: The test instantiates a PingServer with accept_timeout=1.0 which is
too short for slower CI runs; update the PingServer constructor calls (e.g., the
creation of default_server using PingServer(default_socket_path,
response=b"WRONG\n", accept_timeout=1.0)) to use a larger accept_timeout (for
example 5.0) at both occurrences (the one around default_server and the second
occurrence referenced around line 524) so the server stays alive long enough for
the subprocess to connect and the socket-selection assertions become
deterministic.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: dd5c9b1a-f520-405a-8f81-7ec7a84c6f6a
📒 Files selected for processing (13)
.github/workflows/promote-release-candidate.yml.github/workflows/release-candidate.ymlCLI/CLISocketPathResolver.swiftPackages/CMUXSocketPathDomain/Sources/CMUXSocketPathDomain/SocketPathMarkerFiles.swiftPackages/CMUXSocketPathDomain/Tests/CMUXSocketPathDomainTests/SocketPathMarkerFilesTests.swiftSources/Auth/AuthCallbackRouter.swiftSources/Auth/AuthEnvironment.swiftSources/CmuxApplicationSupportDirectories.swiftSources/CmuxSSHURLRequest.swiftcmuxTests/CmuxSSHURLRequestTests.swifttests/cmux.pytests/test_cli_socket_autodiscovery.pyweb/app/handler/after-sign-in/page.tsx
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1edaf4b. Configure here.
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
cmux RC.appand stable candidate assets from the same unsigned app payload.Release setup needed
Add
APPLE_RC_PROVISIONING_PROFILE_BASE64before running the RC workflow. It must be a Developer ID all-devices profile for7WLXT3NR37.com.cmuxterm.app.rcwith the WebAuthn browser entitlement.Testing
tests/test_verify_app_bundle_channel_metadata.shscripts/compile-app-icon-icns.sh Assets.xcassets/AppIcon-RC.appiconset /tmp/cmux-rc-icon-test.icnsshellcheck scripts/compile-app-icon-icns.sh tests/test_verify_app_bundle_channel_metadata.shnode scripts/release_asset_guard.test.jsplutil -lint cmux.rc.entitlementspython3 -m json.tool Assets.xcassets/AppIcon-RC.appiconset/Contents.json >/dev/nullactionlinton the new workflows and CI workflowASSETCATALOG_COMPILER_APPICON_NAME=AppIcon-RCsucceededNote
Medium Risk
Adds new release automation and a new
cmux-rcapp channel (URL scheme/bundle ID/socket paths) which affects packaging, update feeds, and deep-link routing; mistakes could break release publishing or RC/stable coexistence.Overview
Introduces a release-candidate (RC) distribution channel for the macOS app, including new
release-candidate.yml(build/sign/notarizecmux RC.app, generate RC + stable appcasts, publish RC prerelease assets, upload RC appcast) andpromote-release-candidate.yml(promote prebuiltstable-candidate-*assets to the stable tag without rebuilding, with guards against overwriting immutable assets).Extends the app/CLI ecosystem to recognize
cmux-rcalongside stable/nightly/dev: adds RC bundle ID, URL scheme allowlists (auth + SSH + web after-sign-in), RC socket variant/marker files and theme-reload targeting, plus tests covering RC socket autodiscovery and bundle metadata verification.Remote daemon release manifests now optionally embed
signerWorkflow(plumbed throughbuild_remote_daemon_release_assets.shand workflows) and the CLI uses it (with tag-based fallback) when printinggh attestation verifyinstructions; CI additionally runs a newtest_verify_app_bundle_channel_metadata.shcheck.Reviewed by Cursor Bugbot for commit b1cc8c8. Bugbot is set up for automated code reviews on this repo. Configure here.
Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds a macOS release-candidate channel that builds/signs/notarizes a side-by-side
cmux RC.app, publishes an RC appcast to R2 only when latest, and promotes the same built bits to stable without rebuilding. Also records the remote-daemon attestation signer workflow in manifests and verifies attestations against it.New Features
release-candidate.yml: builds/signs/notarizes stable +cmux RC.app, generates Sparkle appcasts, uploads dSYMs, attests remote-daemon assets, supportspublish=falsedry runs, verifies/creates the RC tag at the candidate SHA, guards immutable assets (skip when complete; fail on partial), reuses the release appcast on retries, and uploads the RC appcast to R2 only when the RC is latest.promote-release-candidate.yml: promotesstable-candidate-*assets to the stable tag without rebuilding; verifies the tag points to the candidate SHA, enforces immutable-asset guards (skip when complete; fail on partial/mismatch), validates the appcast references the stable tag, uploads the stable appcast to R2 only when latest, optionally triggers Homebrew, and reuses the release appcast for idempotent retries.signerWorkflow;release.yml/nightly.ymlpass--signer-workflow; CLI prefersmanifest.signerWorkflowforgh attestation verifyand falls back to a tag-based workflow when absent.cmux-rcURL scheme,com.cmuxterm.app.rcbundle ID, default socket/tmp/cmux-rc.sock(+ slug variants/marker files); CLI prefers the channel default socket over stale markers and routes theme reloads to the RC bundle; Python client recognizes RC bundle IDs/markers/sockets; auth and SSH parsing acceptcmux-rc://; web after-sign-in supportscmux-rc://.cmux.rc.entitlements,Assets.xcassets/AppIcon-RC.appiconset, icon helpers (compile-app-icon-icns.sh,generate_rc_icon.py); CI validates bundle-channel metadata for stable/nightly/rc and strengthens socket autodiscovery tests to cover RC and default-over-stale precedence;release.ymlignoresv*-rc.*tags and fails fast if invoked on an RC tag.Migration
APPLE_RC_PROVISIONING_PROFILE_BASE64(Developer ID all-devices for7WLXT3NR37.com.cmuxterm.app.rcwith the WebAuthn browser entitlement) before runningrelease-candidate.yml.Written for commit b1cc8c8. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Chores
Tests