Skip to content

chore: Require explicit CLI compatibility decisions - #1202

Closed
hatayama wants to merge 1 commit into
v3-betafrom
feature/hatayama/add-cli-minimum-version-guard
Closed

chore: Require explicit CLI compatibility decisions#1202
hatayama wants to merge 1 commit into
v3-betafrom
feature/hatayama/add-cli-minimum-version-guard

Conversation

@hatayama

Copy link
Copy Markdown
Owner

Summary

  • PRs now fail fast when the Unity package minimum CLI version is newer than the bundled CLI contract.
  • Compatibility-sensitive CLI or Editor/CLI changes now require either a minimum-version bump or a documented keep decision.

User Impact

  • Maintainers can keep the Unity package minimum below the latest bundled CLI when older CLIs remain compatible.
  • Reviewers get an explicit reason when a change keeps accepting older CLIs, instead of relying on an implicit assumption.

Changes

  • Add a Go-based CI guard for CLI minimum-version compatibility decisions.
  • Run the guard in build-and-test with full checkout history so PR diffs can be inspected against the base branch.
  • Cover version ordering, sensitive-change detection, keep-decision validation, and minimum-version updates with focused tests.

Verification

  • go test ./cmd/cli-minimum-version-guard
  • scripts/check-cli-minimum-version-compatibility.sh
  • scripts/check-go-cli.sh
  • git diff --check

Closes #1200

Ensure CI distinguishes the bundled CLI contract version from the Unity package minimum while still forcing compatibility-sensitive changes to document whether older CLIs remain supported.

- Add a Go guard that compares the package minimum with the latest CLI contract and checks PR diffs for sensitive paths.
- Require either a minimum version update or a keep-decision file with a reason when sensitive files change.
- Run the guard in build-and-test after Go setup with full checkout history.
@coderabbitai

coderabbitai Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds a CLI minimum-version compatibility guard that runs in CI to detect compatibility-sensitive CLI changes and enforce explicit documentation when the minimum required version is not incremented. The guard reads version constraints, detects changed files, classifies breaking changes, validates optional decision JSON, and fails the build when sensitive changes lack proper justification.

Changes

Guard implementation and testing

Layer / File(s) Summary
Guard program setup and version reading
Packages/src/Cli~/cmd/cli-minimum-version-guard/main.go
Program initialization, file path constants, regex for extracting MINIMUM_REQUIRED_CLI_VERSION from C# source, and functions to read the minimum version from source and the latest contract version from JSON.
Change detection: base ref and sensitive paths
Packages/src/Cli~/cmd/cli-minimum-version-guard/main.go
Resolves git base reference from environment or fallback logic, lists changed files, and classifies each path as compatibility-sensitive (contract, editor setup, certain scripts, CLI sources, or decision file).
Decision validation and enforcement
Packages/src/Cli~/cmd/cli-minimum-version-guard/main.go
Validates optional keep-decision JSON file, ensuring it matches the current minimum version, has decision set to "keep", and includes a non-empty reason.
Guard orchestration and git integration
Packages/src/Cli~/cmd/cli-minimum-version-guard/main.go
Main check function orchestrating version reading, change detection, and decision validation; enforces that either the minimum version incremented or a keep decision was added when compatibility-sensitive changes are detected. Git helper wraps all command execution with error context.
Comprehensive test suite
Packages/src/Cli~/cmd/cli-minimum-version-guard/main_test.go
Test creates temporary repositories with configurable versions and mutations, simulating sensitive and non-sensitive changes, validating version bounds, decision file requirements, reason validation, and state transitions.

CI integration

Layer / File(s) Summary
Workflow and shell integration
.github/workflows/build-and-test.yml, scripts/check-cli-minimum-version-compatibility.sh
GitHub Actions workflow updated to fetch full git history and add a build step that runs the shell script entrypoint, which sources the Go toolchain setup and invokes the Go-based minimum-version guard from the CLI directory.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • hatayama/unity-cli-loop#1101: Updates MINIMUM_REQUIRED_CLI_VERSION constant and related setup tests/docs that this guard reads and enforces during CI checks.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.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: adding explicit CLI compatibility decision requirements to the CI/build process.
Description check ✅ Passed The description clearly relates to the changeset, explaining the purpose, user impact, and implementation of the CLI compatibility guard feature.
Linked Issues check ✅ Passed The PR implements all acceptance criteria from issue #1200: version comparison check, sensitive-change detection, keep-decision validation with reasons, and comprehensive tests.
Out of Scope Changes check ✅ Passed All changes are directly scoped to implementing the CLI minimum-version guard: workflow integration, the guard program, tests, and entry script.

✏️ 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/add-cli-minimum-version-guard

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
.github/workflows/build-and-test.yml (1)

18-21: ⚡ Quick win

Consider setting persist-credentials: false for security hardening.

As a security best practice, explicitly disable credential persistence in the checkout step to minimize the risk of token exposure in subsequent steps or artifacts. While the current risk is low (the workflow has only contents: read permission and doesn't publish artifacts), this change aligns with security hardening guidelines.

🔒 Proposed security hardening
       - name: Checkout repository
         uses: actions/checkout@v6
         with:
           fetch-depth: 0
+          persist-credentials: false
🤖 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/build-and-test.yml around lines 18 - 21, Update the
"Checkout repository" step (the step with name "Checkout repository" and uses:
actions/checkout@v6) to explicitly disable credential persistence by adding
persist-credentials: false under its with block so the runner does not persist
GITHUB_TOKEN credentials to subsequent steps or side-effects.
🤖 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 @.github/workflows/build-and-test.yml:
- Around line 18-21: Update the "Checkout repository" step (the step with name
"Checkout repository" and uses: actions/checkout@v6) to explicitly disable
credential persistence by adding persist-credentials: false under its with block
so the runner does not persist GITHUB_TOKEN credentials to subsequent steps or
side-effects.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 08568f1e-4b48-4279-8a03-3dd5044530cd

📥 Commits

Reviewing files that changed from the base of the PR and between b8d89c3 and c3154ee.

📒 Files selected for processing (4)
  • .github/workflows/build-and-test.yml
  • Packages/src/Cli~/cmd/cli-minimum-version-guard/main.go
  • Packages/src/Cli~/cmd/cli-minimum-version-guard/main_test.go
  • scripts/check-cli-minimum-version-compatibility.sh

@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 4 files

Re-trigger cubic

@hatayama hatayama closed this May 25, 2026
@hatayama
hatayama deleted the feature/hatayama/add-cli-minimum-version-guard branch May 25, 2026 01:37
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