Skip to content

fix: Setup now requires the CLI needed for reliable compile waits - #1246

Merged
hatayama merged 2 commits into
v3-betafrom
feature/hatayama/fix-cli-minimum-version-warning
May 30, 2026
Merged

fix: Setup now requires the CLI needed for reliable compile waits#1246
hatayama merged 2 commits into
v3-betafrom
feature/hatayama/fix-cli-minimum-version-warning

Conversation

@hatayama

Copy link
Copy Markdown
Owner

Summary

  • Setup and Settings now require the CLI release that includes accepted compile result polling.
  • Pull request builds now fail when Go CLI behavior changes without a matching minimum CLI version update.

User Impact

  • Before this change, package releases could accept older CLI installs even when newer package behavior depended on a newer CLI release.
  • After this change, users are prompted toward the CLI version needed for reliable compile waits, and future pull requests catch missing minimum-version bumps during CI.

Changes

  • Raise the minimum required CLI version to 3.0.0-beta.20.
  • Add a CI check mode for the existing minimum-version warning automation.
  • Wire the check into the active pull request build while keeping the existing reminder comment flow.

Verification

  • scripts/test-cli-minimum-version-warning-workflow.sh
  • scripts/test-comment-cli-minimum-version-warning.sh
  • go -C Packages/src/Cli~ test ./internal/automation ./internal/architecture -count=1
  • Packages/src/Cli~/dist/darwin-arm64/uloop compile --project-path /Users/a12115/ghq/hatayama/unity-cli-loop
  • Packages/src/Cli~/dist/darwin-arm64/uloop run-tests --project-path /Users/a12115/ghq/hatayama/unity-cli-loop --test-mode EditMode --filter-type regex --filter-value "CliSetupApplicationServiceTests|JsonRpcProcessorCliVersionGateTests|UnityCliLoopFirstPartyServerLifecycleBindingTests"
  • scripts/check-go-cli.sh

Fail the active pull request build when Go CLI changes omit MINIMUM_REQUIRED_CLI_VERSION, while keeping the existing reminder comment flow for pull_request_target.

Raise the package minimum CLI requirement to cli-v3.0.0-beta.20 because compile recovery now depends on the accepted compile result polling behavior from that CLI release.
@coderabbitai

coderabbitai Bot commented May 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 15283b62-1f2a-4b3d-b5c3-830c711ee78e

📥 Commits

Reviewing files that changed from the base of the PR and between a21a86e and 1f93fb5.

📒 Files selected for processing (2)
  • Packages/src/Cli~/internal/architecture/comment_cli_minimum_version_warning_test.go
  • Packages/src/Cli~/internal/automation/minimum_version_warning.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • Packages/src/Cli~/internal/architecture/comment_cli_minimum_version_warning_test.go
  • Packages/src/Cli~/internal/automation/minimum_version_warning.go

📝 Walkthrough

Walkthrough

This PR adds a fail-on-warning mode for CLI minimum-version checks, bumps the minimum CLI version to 3.0.0-beta.20, updates tests, refactors the Go automation to support check-mode exit behavior, and wires a PR-only workflow step plus a wrapper script to run the check in CI.

Changes

CLI Minimum Version Fail-on-Warning Mode

Layer / File(s) Summary
Minimum version constant and test updates
Packages/src/Editor/Domain/CliConstants.cs, Assets/Tests/Editor/CliSetupApplicationServiceTests.cs
Updated MINIMUM_REQUIRED_CLI_VERSION from 3.0.0-beta.18 to 3.0.0-beta.20 and replaced/updated C# tests to assert the new required release stream and GitHub release tag.
Fail-on-warning mode implementation
Packages/src/Cli~/internal/automation/minimum_version_warning.go
Add regex parsing and failOnWarning config; refactor RunMinimumVersionWarning to compute changed files and diff-based requirement early and, when CLI_MINIMUM_VERSION_FAIL_ON_WARNING=true, print the warning to stderr and exit 1/0 without calling GitHub APIs.
Go test coverage for fail-on-warning
Packages/src/Cli~/internal/architecture/comment_cli_minimum_version_warning_test.go
Extend test options with FailOnWarning and ExpectFailure; add tests that run the comment script in check mode for Go CLI changes and for a touched-but-unchanged minimum file, asserting the MINIMUM_REQUIRED_CLI_VERSION marker and that no GitHub POST/PATCH calls occur.
Workflow step and script wiring
scripts/check-cli-minimum-version-warning.sh, .github/workflows/build-and-test.yml, scripts/test-cli-minimum-version-warning-workflow.sh
Add a POSIX wrapper that exports CLI_MINIMUM_VERSION_FAIL_ON_WARNING=true and delegates to the comment script; add a pull-request-only workflow step that runs this wrapper and passes PR/repo/base-ref context; update workflow test to assert the step is present and configured.

Sequence Diagram(s)

sequenceDiagram
  participant GitHubActions
  participant WrapperScript as check-cli-minimum-version-warning.sh
  participant CommentCLI as comment-cli-minimum-version-warning.sh
  participant Automation as minimum_version_warning.go
  participant GitHubAPI

  GitHubActions->>WrapperScript: run (PR_NUMBER, REPOSITORY, BASE_REF)
  WrapperScript->>CommentCLI: invoke with repo root (CLI_MINIMUM_VERSION_FAIL_ON_WARNING=true)
  CommentCLI->>Automation: RunMinimumVersionWarning(diff, baseRef, headRef)
  Automation->>Automation: compute changedFiles, requiresComment (regex + git show)
  Automation-->>GitHubActions: write warning to stderr & exit 1 (when fail-on-warning and requiresComment)
  Automation->>GitHubAPI: (skipped when fail-on-warning)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: updating the CLI minimum version requirement for reliable compile waits, which is the primary purpose of this PR.
Description check ✅ Passed The description is directly related to the changeset, providing clear context about why the minimum CLI version was raised and how the CI check was implemented.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/hatayama/fix-cli-minimum-version-warning

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 7 files

Re-trigger cubic

Compare the required CLI version value across the pull request diff instead of treating any CliConstants.cs edit as sufficient.

This keeps the CI check from passing when Go CLI behavior changes and the minimum-version file is touched without actually bumping MINIMUM_REQUIRED_CLI_VERSION.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant