fix: Setup now installs CLI releases that match the required protocol#1340
Merged
Conversation
Ensure Unity setup installs the published CLI release that advertises the required IPC protocol, and add PR/release automation so future protocol bumps cannot leave the installer target stale.
Update the repository guidance and CodeRabbit instructions to match the new protocol bump policy: PRs that change the required protocol must also advance the minimum CLI version to a published CLI release that advertises that protocol.
Ensure protocol bump PRs only pass after the selected minimum CLI release tag advertises the required protocol. Keep the PR warning comment open for mismatched release tags instead of clearing it when the version string merely changes.
Refresh the selected CLI release tag after the release readiness wait, so package release sync can verify protocol metadata even when the tag was created after the initial fetch.
Run the minimum CLI release protocol check whenever the installer target changes, even if the required protocol constant is unchanged. Use the PR merge ref for warning comments so stale branches are compared as GitHub will test them.
Validate protocol minimum targets against published GitHub releases with complete native CLI assets, and verify package-release backfills against the resolved release commit instead of the current checkout.
Contributor
📝 WalkthroughWalkthroughAdds a Go-based protocol minimum version guard system with two CLI entrypoints, a PR comment automation tool, new CI workflows ( ChangesProtocol Minimum Version Guard & Automation
Sequence Diagram(s)sequenceDiagram
participant PR as Pull Request
participant BuildAndTest as build-and-test
participant WarningWorkflow as protocol-minimum-version-warning
participant CheckCmd as check-protocol-minimum-version
participant CommentCmd as comment-protocol-minimum-version
participant GhAPI as GitHub API / gh CLI
rect rgba(173, 216, 230, 0.5)
note over PR, GhAPI: PR guard (fails if MINIMUM_REQUIRED_CLI_VERSION not updated)
PR->>BuildAndTest: pull_request event
BuildAndTest->>CheckCmd: go run (GH_TOKEN, base/head refs)
CheckCmd->>CheckCmd: AnalyzeProtocolMinimumVersionGuardForRefs
CheckCmd->>GhAPI: gh release view (verify published + assets)
CheckCmd-->>BuildAndTest: exit 0 (pass) or exit 1 (fail)
end
rect rgba(255, 228, 181, 0.5)
note over PR, GhAPI: PR comment automation (pull_request_target, trusted)
PR->>WarningWorkflow: pull_request_target event
WarningWorkflow->>WarningWorkflow: checkout + fetch PR merge ref
WarningWorkflow->>CommentCmd: go run (GITHUB_TOKEN, base/head refs)
CommentCmd->>GhAPI: list PR comments (paginated)
CommentCmd->>GhAPI: PATCH/POST/DELETE comment
CommentCmd-->>WarningWorkflow: done
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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)
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 |
Contributor
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 @.github/workflows/protocol-minimum-version-warning.yml:
- Around line 17-20: The checkout action in the "Checkout base repository" step
does not disable credential persistence, which unnecessarily exposes the
authentication token to later workflow steps in this privileged
pull_request_target job. Add the `persist-credentials: false` parameter to the
`with` section of the actions/checkout action to prevent the token from being
stored in the local git config.
🪄 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: CHILL
Plan: Pro
Run ID: 0ac6d330-6978-443e-b035-af94ae7d2563
📒 Files selected for processing (17)
.coderabbit.yaml.github/workflows/build-and-test.yml.github/workflows/native-cli-publish.yml.github/workflows/protocol-minimum-version-warning.yml.github/workflows/release-please.ymlAGENTS.mdPackages/src/Editor/Domain/CliConstants.cscli/cmd/check-protocol-minimum-version/main.gocli/cmd/comment-protocol-minimum-version/main.gocli/internal/automation/protocol_minimum_version_comment.gocli/internal/automation/protocol_minimum_version_guard.gocli/internal/automation/protocol_minimum_version_guard_test.goscripts/sync-release-please-package-releases.shscripts/test-native-cli-publish-workflow.shscripts/test-protocol-minimum-version-workflow.shscripts/test-release-please-config.shscripts/test-sync-release-please-package-releases.sh
There was a problem hiding this comment.
4 issues found across 17 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Ensure privileged PR warning workflows do not persist checkout credentials, require draft package releases to be protocol-verified before publishing, and scope automation comment updates to the workflow bot author.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/test-sync-release-please-package-releases.sh (1)
244-244: 💤 Low valueConsider parameterizing the CLI release tag in the origin push.
The tag ref is hardcoded to
cli-v3.0.0-beta.6. If a future test needs to callprepare_origin_branchwith a differentCLI_RELEASE_TAG, the hardcoded tag would be incorrect. Current tests don't exercise this path, so it's not a bug today.♻️ Optional refactor to use CLI_RELEASE_TAG
- git push -q origin "$commit_sha:refs/heads/$branch_name" refs/tags/cli-v3.0.0-beta.6 + git push -q origin "$commit_sha:refs/heads/$branch_name" "refs/tags/${CLI_RELEASE_TAG:-cli-v3.0.0-beta.6}"🤖 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 `@scripts/test-sync-release-please-package-releases.sh` at line 244, The git push command in the script hardcodes the tag reference as `cli-v3.0.0-beta.6` instead of using the parameterized `CLI_RELEASE_TAG` variable. Replace the hardcoded tag `cli-v3.0.0-beta.6` with `$CLI_RELEASE_TAG` in the refs/tags portion of the git push command to make the tag configurable for future tests that may use different CLI release tags.
🤖 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 `@scripts/test-sync-release-please-package-releases.sh`:
- Line 244: The git push command in the script hardcodes the tag reference as
`cli-v3.0.0-beta.6` instead of using the parameterized `CLI_RELEASE_TAG`
variable. Replace the hardcoded tag `cli-v3.0.0-beta.6` with `$CLI_RELEASE_TAG`
in the refs/tags portion of the git push command to make the tag configurable
for future tests that may use different CLI release tags.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 25e456c4-50b8-4204-a608-0cccaa008c89
📒 Files selected for processing (7)
.github/workflows/protocol-minimum-version-warning.ymlcli/internal/automation/protocol_minimum_version_comment.gocli/internal/automation/protocol_minimum_version_guard.gocli/internal/automation/protocol_minimum_version_guard_test.goscripts/sync-release-please-package-releases.shscripts/test-protocol-minimum-version-workflow.shscripts/test-sync-release-please-package-releases.sh
🚧 Files skipped from review as they are similar to previous changes (5)
- scripts/test-protocol-minimum-version-workflow.sh
- scripts/sync-release-please-package-releases.sh
- .github/workflows/protocol-minimum-version-warning.yml
- cli/internal/automation/protocol_minimum_version_guard.go
- cli/internal/automation/protocol_minimum_version_guard_test.go
Merged
This was referenced Jun 26, 2026
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
1 participant
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
User Impact
Changes
3.0.0-beta.33.Verification
scripts/check-go-cli.shcd cli && go test ./internal/automation ./cmd/check-protocol-minimum-version ./cmd/comment-protocol-minimum-versionsh scripts/test-protocol-minimum-version-workflow.sh && sh scripts/test-sync-release-please-package-releases.sh && sh scripts/test-native-cli-publish-workflow.sh && sh scripts/test-release-please-config.shcd cli && go run ./cmd/check-protocol-minimum-version --base origin/v3-beta --head HEADcd cli && go run ./cmd/check-protocol-minimum-version --verify-release$codex-review v3-beta --parallel-tests "scripts/check-go-cli.sh"