From 75dfd5259dcc8c720b8055099b7f78e81236c651 Mon Sep 17 00:00:00 2001 From: hatayama Date: Sat, 13 Jun 2026 10:26:14 +0900 Subject: [PATCH 1/9] feat: Declare an IPC protocol version in the CLI contract The semver-based compatibility floor forced PR authors to predict the next release number, which diverged from release-please numbering and broke a release. Compatibility is really about the C# IPC contract, so the contract now carries an integer protocolVersion that only moves when the Unity package and the CLI stop interoperating. The CLI sends it in the uloop request metadata so the Unity side can gate on it. --- cli/contract.go | 10 ++++++++-- cli/contract.json | 1 + cli/contract_test.go | 7 +++++++ cli/internal/unityipc/client.go | 4 ++++ cli/internal/unityipc/client_test.go | 5 +++++ 5 files changed, 25 insertions(+), 2 deletions(-) diff --git a/cli/contract.go b/cli/contract.go index 2be6d44f16..90456f40a2 100644 --- a/cli/contract.go +++ b/cli/contract.go @@ -17,8 +17,11 @@ var contractFiles embed.FS var Current = mustLoad() type Contract struct { - SchemaVersion int `json:"schemaVersion"` - CliVersion string `json:"cliVersion"` + SchemaVersion int `json:"schemaVersion"` + // ProtocolVersion is the C# IPC contract generation this binary speaks. It moves only + // when the Unity package and the CLI can no longer interoperate, never per release. + ProtocolVersion int `json:"protocolVersion"` + CliVersion string `json:"cliVersion"` } func mustLoad() Contract { @@ -35,6 +38,9 @@ func mustLoad() Contract { panic(fmt.Sprintf("CLI contract schema version mismatch: %d", contract.SchemaVersion)) } requireString(contract.CliVersion, "cliVersion") + if contract.ProtocolVersion < 1 { + panic(fmt.Sprintf("CLI contract protocolVersion must be at least 1, got %d", contract.ProtocolVersion)) + } return contract } diff --git a/cli/contract.json b/cli/contract.json index 9516c87585..b108c8dcc2 100644 --- a/cli/contract.json +++ b/cli/contract.json @@ -1,4 +1,5 @@ { "schemaVersion": 1, + "protocolVersion": 1, "cliVersion": "3.0.0-beta.31" } diff --git a/cli/contract_test.go b/cli/contract_test.go index e2c423898c..b1c8638187 100644 --- a/cli/contract_test.go +++ b/cli/contract_test.go @@ -11,6 +11,13 @@ func TestCliContractProvidesRuntimeVersion(t *testing.T) { requireValidContractVersion(t, "cliVersion", Current.CliVersion) } +func TestCliContractProvidesProtocolVersion(t *testing.T) { + // Verifies that the contract declares which C#-side IPC protocol the binary speaks. + if Current.ProtocolVersion < 1 { + t.Fatalf("protocolVersion must be at least 1, got %d", Current.ProtocolVersion) + } +} + func requireValidContractVersion(t *testing.T, label string, value string) { t.Helper() diff --git a/cli/internal/unityipc/client.go b/cli/internal/unityipc/client.go index 610e5e20ad..81b45a1c12 100644 --- a/cli/internal/unityipc/client.go +++ b/cli/internal/unityipc/client.go @@ -9,6 +9,8 @@ import ( "net" "os" "time" + + clicontract "github.com/hatayama/unity-cli-loop/cli" ) const ( @@ -64,6 +66,7 @@ type rpcRequest struct { type rpcClientMetadata struct { CLIVersion string `json:"cliVersion"` + ProtocolVersion int `json:"protocolVersion"` AcceptsDispatchAck bool `json:"acceptsDispatchAck"` AcceptsHeartbeat bool `json:"acceptsHeartbeat"` } @@ -181,6 +184,7 @@ func (client *Client) SendWithProgressOutcomeAcceptContext( Params: params, ULoop: rpcClientMetadata{ CLIVersion: client.clientVersion, + ProtocolVersion: clicontract.Current.ProtocolVersion, AcceptsDispatchAck: true, AcceptsHeartbeat: true, }, diff --git a/cli/internal/unityipc/client_test.go b/cli/internal/unityipc/client_test.go index 424a11bf62..9e694d3f27 100644 --- a/cli/internal/unityipc/client_test.go +++ b/cli/internal/unityipc/client_test.go @@ -10,6 +10,8 @@ import ( "strings" "testing" "time" + + clicontract "github.com/hatayama/unity-cli-loop/cli" ) func TestFormatConnectionAttemptErrorExplainsDialFailureWithoutDisconnectClaim(t *testing.T) { @@ -110,6 +112,9 @@ func TestSendIncludesCliVersionWithoutProjectIdentityMetadata(t *testing.T) { if metadata["cliVersion"] != "3.0.0-beta.6" { t.Fatalf("cli version metadata mismatch: %#v", metadata) } + if metadata["protocolVersion"] != float64(clicontract.Current.ProtocolVersion) { + t.Fatalf("protocol version metadata mismatch: %#v", metadata) + } if metadata["acceptsDispatchAck"] != true { t.Fatalf("dispatch ack metadata mismatch: %#v", metadata) } From 5c13341f28c36275af1e782a8076cf6df0af4a7e Mon Sep 17 00:00:00 2001 From: hatayama Date: Sat, 13 Jun 2026 11:11:21 +0900 Subject: [PATCH 2/9] feat: Gate CLI compatibility on IPC protocol version, not semver The Unity package rejected old CLIs by comparing release numbers against MINIMUM_REQUIRED_CLI_VERSION, which forced PR authors to predict the next release number and broke when CLI changes accumulated across releases. The gate now reads uloop.protocolVersion from the request metadata and rejects clients below REQUIRED_CLI_PROTOCOL_VERSION, an integer that moves only on a breaking IPC contract change. A missing or non-integer protocolVersion fails the gate, so CLIs released before the handshake are treated as outdated. The cli_update_required error now reports current/required protocol versions instead of a target release tag. MINIMUM_REQUIRED_CLI_VERSION stays as the installer pin for setup/update flows. --- .../JsonRpcProcessorCliVersionGateTests.cs | 81 +++++++++++++------ ...opFirstPartyServerLifecycleBindingTests.cs | 4 +- ...CliLoopFirstPartyServerLifecycleBinding.cs | 2 +- Packages/src/Editor/Domain/CliConstants.cs | 6 ++ .../Infrastructure/Api/JsonRpcProcessor.cs | 63 ++++++++++----- .../Infrastructure/Api/JsonRpcRequest.cs | 6 ++ 6 files changed, 113 insertions(+), 49 deletions(-) diff --git a/Assets/Tests/Editor/JsonRpcProcessorCliVersionGateTests.cs b/Assets/Tests/Editor/JsonRpcProcessorCliVersionGateTests.cs index 0ea907e602..3a9aeba32c 100644 --- a/Assets/Tests/Editor/JsonRpcProcessorCliVersionGateTests.cs +++ b/Assets/Tests/Editor/JsonRpcProcessorCliVersionGateTests.cs @@ -22,11 +22,11 @@ namespace io.github.hatayama.UnityCliLoop.Tests.Editor public class JsonRpcProcessorCliVersionGateTests { [Test] - public async Task ProcessRequest_WhenCliVersionSatisfiesMinimum_AllowsRequest() + public async Task ProcessRequest_WhenProtocolVersionMatches_AllowsRequest() { - // Verifies compatible CLI clients can execute bridge commands. + // Verifies CLI clients that speak the required IPC protocol can execute bridge commands. string response = await JsonRpcProcessor.ProcessRequest( - BuildGetVersionRequest(CliConstants.MINIMUM_REQUIRED_CLI_VERSION), + BuildGetVersionRequest(CliConstants.REQUIRED_CLI_PROTOCOL_VERSION), CancellationToken.None); JObject parsed = JObject.Parse(response); @@ -35,23 +35,35 @@ public async Task ProcessRequest_WhenCliVersionSatisfiesMinimum_AllowsRequest() } [Test] - public async Task ProcessRequest_WhenCliVersionIsTooOld_ReturnsCliUpdateRequiredError() + public async Task ProcessRequest_WhenProtocolVersionIsNewer_AllowsRequest() { - // Verifies old CLI clients receive an exact update command before any tool runs. + // Verifies a newer CLI keeps working against this package version. string response = await JsonRpcProcessor.ProcessRequest( - BuildGetVersionRequest("3.0.0-beta.5"), + BuildGetVersionRequest(CliConstants.REQUIRED_CLI_PROTOCOL_VERSION + 1), + CancellationToken.None); + JObject parsed = JObject.Parse(response); + + Assert.That(parsed["error"], Is.Null); + Assert.That(parsed["result"], Is.Not.Null); + } + + [Test] + public async Task ProcessRequest_WhenProtocolVersionIsTooOld_ReturnsCliUpdateRequiredError() + { + // Verifies CLIs on an older IPC protocol receive update instructions before any tool runs. + string response = await JsonRpcProcessor.ProcessRequest( + BuildGetVersionRequest(CliConstants.REQUIRED_CLI_PROTOCOL_VERSION - 1), CancellationToken.None); JObject data = ParseErrorData(response); Assert.That(data["type"]?.ToString(), Is.EqualTo("cli_update_required")); - Assert.That(data["currentCliVersion"]?.ToString(), Is.EqualTo("3.0.0-beta.5")); - Assert.That(data["requiredCliVersion"]?.ToString(), Is.EqualTo(CliConstants.MINIMUM_REQUIRED_CLI_VERSION)); Assert.That( - data["updateCommand"]?.ToString(), - Is.EqualTo("uloop update")); + data["currentProtocolVersion"]?.ToObject(), + Is.EqualTo(CliConstants.REQUIRED_CLI_PROTOCOL_VERSION - 1)); Assert.That( - data["targetUpdateCommand"]?.ToString(), - Is.EqualTo($"uloop update --to-version {CliConstants.MINIMUM_REQUIRED_CLI_VERSION}")); + data["requiredProtocolVersion"]?.ToObject(), + Is.EqualTo(CliConstants.REQUIRED_CLI_PROTOCOL_VERSION)); + Assert.That(data["updateCommand"]?.ToString(), Is.EqualTo("uloop update")); Assert.That(data["retryableAfterUpdate"]?.ToObject(), Is.True); } @@ -65,20 +77,37 @@ public async Task ProcessRequest_WhenCliMetadataIsMissing_ReturnsCliUpdateRequir JObject data = ParseErrorData(response); Assert.That(data["type"]?.ToString(), Is.EqualTo("cli_update_required")); + Assert.That(data["currentProtocolVersion"]?.Type, Is.EqualTo(JTokenType.Null)); Assert.That(data["currentCliVersion"]?.Type, Is.EqualTo(JTokenType.Null)); } [Test] - public async Task ProcessRequest_WhenCliVersionIsInvalid_ReturnsCliUpdateRequiredError() + public async Task ProcessRequest_WhenClientSendsOnlySemverVersion_ReturnsCliUpdateRequiredError() + { + // Verifies CLIs released before the protocol handshake are treated as outdated. + string response = await JsonRpcProcessor.ProcessRequest( + "{\"jsonrpc\":\"2.0\",\"method\":\"get-version\",\"params\":{},\"id\":1," + + "\"uloop\":{\"cliVersion\":\"3.0.0-beta.24\"}}", + CancellationToken.None); + JObject data = ParseErrorData(response); + + Assert.That(data["type"]?.ToString(), Is.EqualTo("cli_update_required")); + Assert.That(data["currentProtocolVersion"]?.Type, Is.EqualTo(JTokenType.Null)); + Assert.That(data["currentCliVersion"]?.ToString(), Is.EqualTo("3.0.0-beta.24")); + } + + [Test] + public async Task ProcessRequest_WhenProtocolVersionIsNotAnInteger_ReturnsCliUpdateRequiredError() { - // Verifies malformed CLI versions cannot bypass the compatibility gate. + // Verifies malformed protocol values cannot bypass the compatibility gate. string response = await JsonRpcProcessor.ProcessRequest( - BuildGetVersionRequest("not-a-version"), + "{\"jsonrpc\":\"2.0\",\"method\":\"get-version\",\"params\":{},\"id\":1," + + "\"uloop\":{\"protocolVersion\":\"not-a-number\"}}", CancellationToken.None); JObject data = ParseErrorData(response); Assert.That(data["type"]?.ToString(), Is.EqualTo("cli_update_required")); - Assert.That(data["currentCliVersion"]?.ToString(), Is.EqualTo("not-a-version")); + Assert.That(data["currentProtocolVersion"]?.Type, Is.EqualTo(JTokenType.Null)); } [Test] @@ -509,12 +538,12 @@ public async Task ProcessRequest_WhenMainThreadSwitchIsCanceled_ReleasesExecutio } } - private static string BuildGetVersionRequest(string cliVersion) + private static string BuildGetVersionRequest(int protocolVersion) { return - "{\"jsonrpc\":\"2.0\",\"method\":\"get-version\",\"params\":{},\"id\":1,\"uloop\":{\"cliVersion\":\"" + - cliVersion + - "\"}}"; + "{\"jsonrpc\":\"2.0\",\"method\":\"get-version\",\"params\":{},\"id\":1,\"uloop\":{\"protocolVersion\":" + + protocolVersion + + "}}"; } private static string BuildToolRequest(string toolName, int id) @@ -524,9 +553,9 @@ private static string BuildToolRequest(string toolName, int id) toolName + "\",\"params\":{},\"id\":" + id + - ",\"uloop\":{\"cliVersion\":\"" + - CliConstants.MINIMUM_REQUIRED_CLI_VERSION + - "\",\"acceptsDispatchAck\":true}}"; + ",\"uloop\":{\"protocolVersion\":" + + CliConstants.REQUIRED_CLI_PROTOCOL_VERSION + + ",\"acceptsDispatchAck\":true}}"; } private static string BuildToolRequestWithParams(string toolName, string paramsJson, int id) @@ -538,9 +567,9 @@ private static string BuildToolRequestWithParams(string toolName, string paramsJ paramsJson + ",\"id\":" + id + - ",\"uloop\":{\"cliVersion\":\"" + - CliConstants.MINIMUM_REQUIRED_CLI_VERSION + - "\",\"acceptsDispatchAck\":true}}"; + ",\"uloop\":{\"protocolVersion\":" + + CliConstants.REQUIRED_CLI_PROTOCOL_VERSION + + ",\"acceptsDispatchAck\":true}}"; } private static JObject ParseErrorData(string response) diff --git a/Assets/Tests/Editor/UnityCliLoopFirstPartyServerLifecycleBindingTests.cs b/Assets/Tests/Editor/UnityCliLoopFirstPartyServerLifecycleBindingTests.cs index 5410e35499..f639a944fa 100644 --- a/Assets/Tests/Editor/UnityCliLoopFirstPartyServerLifecycleBindingTests.cs +++ b/Assets/Tests/Editor/UnityCliLoopFirstPartyServerLifecycleBindingTests.cs @@ -18,7 +18,9 @@ public void CreateGetVersionReadinessRequestJson_UsesInternalHealthCheckWithCliM JObject request = JObject.Parse(requestJson); Assert.That(request["method"]?.ToString(), Is.EqualTo("get-version")); - Assert.That(request["uloop"]?["cliVersion"]?.ToString(), Is.EqualTo(CliConstants.MINIMUM_REQUIRED_CLI_VERSION)); + Assert.That( + request["uloop"]?["protocolVersion"]?.ToObject(), + Is.EqualTo(CliConstants.REQUIRED_CLI_PROTOCOL_VERSION)); } } } diff --git a/Packages/src/Editor/CompositionRoot/UnityCliLoopFirstPartyServerLifecycleBinding.cs b/Packages/src/Editor/CompositionRoot/UnityCliLoopFirstPartyServerLifecycleBinding.cs index 28a71868ce..80815827d3 100644 --- a/Packages/src/Editor/CompositionRoot/UnityCliLoopFirstPartyServerLifecycleBinding.cs +++ b/Packages/src/Editor/CompositionRoot/UnityCliLoopFirstPartyServerLifecycleBinding.cs @@ -61,7 +61,7 @@ internal static string CreateGetVersionReadinessRequestJson() ["id"] = 1, ["uloop"] = new JObject { - ["cliVersion"] = CliConstants.MINIMUM_REQUIRED_CLI_VERSION + ["protocolVersion"] = CliConstants.REQUIRED_CLI_PROTOCOL_VERSION } }; return request.ToString(Formatting.None); diff --git a/Packages/src/Editor/Domain/CliConstants.cs b/Packages/src/Editor/Domain/CliConstants.cs index f96383076b..c25af9e129 100644 --- a/Packages/src/Editor/Domain/CliConstants.cs +++ b/Packages/src/Editor/Domain/CliConstants.cs @@ -6,6 +6,12 @@ namespace io.github.hatayama.UnityCliLoop.Domain public static class CliConstants { public const string EXECUTABLE_NAME = "uloop"; + // Why: the runtime IPC gate compares this contract generation, not release numbers. + // Bump it together with cli/contract.json protocolVersion only when this package can + // no longer interoperate with older CLI binaries. + public const int REQUIRED_CLI_PROTOCOL_VERSION = 1; + // Why: setup installs this pinned release; it must always point at a published CLI + // release, so it advances after releases, never inside feature PRs. public const string MINIMUM_REQUIRED_CLI_VERSION = "3.0.0-beta.31"; public const string MINIMUM_REQUIRED_CLI_RELEASE_TAG = CLI_RELEASE_TAG_PREFIX + MINIMUM_REQUIRED_CLI_VERSION; public const string VERSION_FLAG = "--version"; diff --git a/Packages/src/Editor/Infrastructure/Api/JsonRpcProcessor.cs b/Packages/src/Editor/Infrastructure/Api/JsonRpcProcessor.cs index 8f92e095c7..c42c0315b6 100644 --- a/Packages/src/Editor/Infrastructure/Api/JsonRpcProcessor.cs +++ b/Packages/src/Editor/Infrastructure/Api/JsonRpcProcessor.cs @@ -93,6 +93,7 @@ private static JsonRpcRequest ParseRequest(string jsonRequest) Method = request["method"]?.ToString(), Params = request["params"], ClientCliVersion = ReadClientCliVersion(request), + ClientProtocolVersion = ReadClientProtocolVersion(request), AcceptsDispatchAck = ReadAcceptsDispatchAck(request), AcceptsHeartbeat = ReadAcceptsHeartbeat(request), Id = request["id"]?.ToObject() @@ -111,6 +112,23 @@ private static string ReadClientCliVersion(JObject request) return string.IsNullOrWhiteSpace(cliVersion) ? null : cliVersion; } + private static int? ReadClientProtocolVersion(JObject request) + { + JObject metadata = request["uloop"] as JObject; + if (metadata == null) + { + return null; + } + + JToken protocolVersionToken = metadata["protocolVersion"]; + if (protocolVersionToken == null || protocolVersionToken.Type != JTokenType.Integer) + { + return null; + } + + return protocolVersionToken.Value(); + } + private static bool ReadAcceptsDispatchAck(JObject request) { JObject metadata = request["uloop"] as JObject; @@ -180,9 +198,12 @@ private static async Task ProcessRpcRequest( try { ct.ThrowIfCancellationRequested(); - if (IsCliUpdateRequired(request.ClientCliVersion)) + if (IsCliUpdateRequired(request.ClientProtocolVersion)) { - return CreateCliUpdateRequiredResponse(request.Id, request.ClientCliVersion); + return CreateCliUpdateRequiredResponse( + request.Id, + request.ClientCliVersion, + request.ClientProtocolVersion); } if (request.AcceptsDispatchAck && earlyResponseWriter != null) @@ -278,21 +299,21 @@ private static bool CompileRequestWaitsForDomainReload(JToken paramsToken) return waitForDomainReloadToken.Value(); } - private static bool IsCliUpdateRequired(string currentCliVersion) + private static bool IsCliUpdateRequired(int? currentProtocolVersion) { - if (string.IsNullOrWhiteSpace(currentCliVersion)) + if (currentProtocolVersion == null) { return true; } - return !CliVersionComparer.IsVersionGreaterThanOrEqual( - currentCliVersion, - CliConstants.MINIMUM_REQUIRED_CLI_VERSION); + return currentProtocolVersion.Value < CliConstants.REQUIRED_CLI_PROTOCOL_VERSION; } - private static string CreateCliUpdateRequiredResponse(object id, string currentCliVersion) + private static string CreateCliUpdateRequiredResponse( + object id, + string currentCliVersion, + int? currentProtocolVersion) { - string requiredCliVersion = CliConstants.MINIMUM_REQUIRED_CLI_VERSION; JsonRpcErrorResponse errorResponse = new( UnityCliLoopServerConfig.JSONRPC_VERSION, id, @@ -301,9 +322,9 @@ private static string CreateCliUpdateRequiredResponse(object id, string currentC "The installed uloop CLI is too old for this Unity package.", new CliUpdateRequiredErrorData( currentCliVersion, - requiredCliVersion, - $"{CliConstants.EXECUTABLE_NAME} update", - $"{CliConstants.EXECUTABLE_NAME} update --to-version {requiredCliVersion}"))); + currentProtocolVersion, + CliConstants.REQUIRED_CLI_PROTOCOL_VERSION, + $"{CliConstants.EXECUTABLE_NAME} update"))); return JsonConvert.SerializeObject(errorResponse, Formatting.None, ResponseSerializerSettings); } @@ -563,7 +584,7 @@ public ServerBusyErrorData( } /// - /// Carries exact CLI update instructions so clients do not infer release tags. + /// Carries CLI update instructions plus the protocol generations that failed the gate. /// public class CliUpdateRequiredErrorData : JsonRpcErrorData { @@ -571,24 +592,24 @@ public class CliUpdateRequiredErrorData : JsonRpcErrorData public string currentCliVersion { get; } - public string requiredCliVersion { get; } + public int? currentProtocolVersion { get; } - public string updateCommand { get; } + public int requiredProtocolVersion { get; } - public string targetUpdateCommand { get; } + public string updateCommand { get; } public bool retryableAfterUpdate { get; } public CliUpdateRequiredErrorData( string currentCliVersion, - string requiredCliVersion, - string updateCommand, - string targetUpdateCommand) : base("Update the uloop CLI and retry the original command.") + int? currentProtocolVersion, + int requiredProtocolVersion, + string updateCommand) : base("Update the uloop CLI and retry the original command.") { this.currentCliVersion = string.IsNullOrWhiteSpace(currentCliVersion) ? null : currentCliVersion; - this.requiredCliVersion = requiredCliVersion; + this.currentProtocolVersion = currentProtocolVersion; + this.requiredProtocolVersion = requiredProtocolVersion; this.updateCommand = updateCommand; - this.targetUpdateCommand = targetUpdateCommand; retryableAfterUpdate = true; } } diff --git a/Packages/src/Editor/Infrastructure/Api/JsonRpcRequest.cs b/Packages/src/Editor/Infrastructure/Api/JsonRpcRequest.cs index 3c30b1f6bf..63d33204e2 100644 --- a/Packages/src/Editor/Infrastructure/Api/JsonRpcRequest.cs +++ b/Packages/src/Editor/Infrastructure/Api/JsonRpcRequest.cs @@ -18,6 +18,12 @@ internal class JsonRpcRequest public string ClientCliVersion { get; set; } + /// + /// IPC protocol generation the client speaks. Null when the client predates the + /// protocol handshake or sent a malformed value; both must fail the compatibility gate. + /// + public int? ClientProtocolVersion { get; set; } + public bool AcceptsDispatchAck { get; set; } public bool AcceptsHeartbeat { get; set; } From 3860bf55ac8bca7aba2ce1356989d519b6af7bd2 Mon Sep 17 00:00:00 2001 From: hatayama Date: Sat, 13 Jun 2026 11:35:05 +0900 Subject: [PATCH 3/9] chore: Replace CLI version-warning CI with a protocol consistency check The minimum-version warning fired whenever Go CLI files changed without bumping MINIMUM_REQUIRED_CLI_VERSION. That constant is now only the installer pin, so the warning enforced the wrong thing, depended on a git diff, and tangled with release-please numbering. Replace the whole apparatus (comment binary, scripts, pull_request_target workflow, and the fail-on-warning build step) with one deterministic Go test: cli/contract.json protocolVersion must equal the Unity package's REQUIRED_CLI_PROTOCOL_VERSION. It runs in the existing test suite, needs no diff, PR number, or GitHub API, and has no false positives. Whether a given change is breaking enough to bump the protocol is a code-review and docs concern, not a path heuristic. --- .github/workflows/build-and-test.yml | 11 - .../workflows/cli-minimum-version-warning.yml | 38 -- .../main.go | 12 - ...omment_cli_minimum_version_warning_test.go | 394 ------------------ .../automation/minimum_version_warning.go | 380 ----------------- .../minimum_version_warning_test.go | 205 --------- cli/protocol_version_consistency_test.go | 45 ++ scripts/check-cli-minimum-version-warning.sh | 9 - .../comment-cli-minimum-version-warning.sh | 8 - ...st-cli-minimum-version-warning-workflow.sh | 41 -- ...est-comment-cli-minimum-version-warning.sh | 7 - 11 files changed, 45 insertions(+), 1105 deletions(-) delete mode 100644 .github/workflows/cli-minimum-version-warning.yml delete mode 100644 cli/cmd/comment-cli-minimum-version-warning/main.go delete mode 100644 cli/internal/architecture/comment_cli_minimum_version_warning_test.go delete mode 100644 cli/internal/automation/minimum_version_warning.go delete mode 100644 cli/internal/automation/minimum_version_warning_test.go create mode 100644 cli/protocol_version_consistency_test.go delete mode 100755 scripts/check-cli-minimum-version-warning.sh delete mode 100755 scripts/comment-cli-minimum-version-warning.sh delete mode 100755 scripts/test-cli-minimum-version-warning-workflow.sh delete mode 100755 scripts/test-comment-cli-minimum-version-warning.sh diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index c89827ab91..cf78404d3b 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -30,19 +30,8 @@ jobs: go-version-file: cli/.go-version cache: false - - name: Check CLI minimum version bump - if: github.event_name == 'pull_request' - env: - PR_NUMBER: ${{ github.event.pull_request.number }} - GITHUB_REPOSITORY: ${{ github.repository }} - GITHUB_BASE_REF: ${{ github.base_ref }} - CLI_MINIMUM_VERSION_FAIL_ON_WARNING: true - run: scripts/check-cli-minimum-version-warning.sh - - name: Test release automation shell helpers run: | - scripts/test-comment-cli-minimum-version-warning.sh - scripts/test-cli-minimum-version-warning-workflow.sh scripts/test-go-cli-toolchain.sh scripts/test-resolve-native-cli-release-target.sh scripts/test-is-release-please-release-commit.sh diff --git a/.github/workflows/cli-minimum-version-warning.yml b/.github/workflows/cli-minimum-version-warning.yml deleted file mode 100644 index c11becc02e..0000000000 --- a/.github/workflows/cli-minimum-version-warning.yml +++ /dev/null @@ -1,38 +0,0 @@ -name: CLI Minimum Version Warning - -on: - pull_request_target: - branches: [main, v3-beta] - -permissions: - contents: read - issues: write - pull-requests: read - -jobs: - comment: - runs-on: ubuntu-latest - - steps: - - name: Checkout base repository - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd - with: - fetch-depth: 0 - - - name: Setup Go - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c - with: - go-version-file: cli/.go-version - cache: false - - - name: Fetch pull request head - run: git fetch --no-tags origin "pull/${{ github.event.pull_request.number }}/head:cli-minimum-version-pr-head" - - - name: Comment on CLI minimum version check - env: - GH_TOKEN: ${{ github.token }} - PR_NUMBER: ${{ github.event.pull_request.number }} - GITHUB_REPOSITORY: ${{ github.repository }} - CLI_MINIMUM_VERSION_BASE_REF: origin/${{ github.base_ref }} - CLI_MINIMUM_VERSION_HEAD_REF: cli-minimum-version-pr-head - run: scripts/comment-cli-minimum-version-warning.sh diff --git a/cli/cmd/comment-cli-minimum-version-warning/main.go b/cli/cmd/comment-cli-minimum-version-warning/main.go deleted file mode 100644 index 6028e8c100..0000000000 --- a/cli/cmd/comment-cli-minimum-version-warning/main.go +++ /dev/null @@ -1,12 +0,0 @@ -package main - -import ( - "context" - "os" - - "github.com/hatayama/unity-cli-loop/cli/internal/automation" -) - -func main() { - os.Exit(automation.RunMinimumVersionWarning(context.Background(), os.Stdout, os.Stderr)) -} diff --git a/cli/internal/architecture/comment_cli_minimum_version_warning_test.go b/cli/internal/architecture/comment_cli_minimum_version_warning_test.go deleted file mode 100644 index 2c9ff0399e..0000000000 --- a/cli/internal/architecture/comment_cli_minimum_version_warning_test.go +++ /dev/null @@ -1,394 +0,0 @@ -package architecture - -import ( - "os" - "os/exec" - "path/filepath" - "runtime" - "strings" - "testing" -) - -const ( - commentScriptPath = "scripts/comment-cli-minimum-version-warning.sh" - minimumVersionPath = "Packages/src/Editor/Domain/CliConstants.cs" - goCliPath = "cli/internal/cli/run.go" -) - -type commentScriptResult struct { - GitHubLog string - BodyLog string - Output string -} - -type commentScriptOptions struct { - ExistingCommentID string - Mutation string - BaseRef string - HeadRef string - HeadBranch string - IncludePRNumber bool - FailOnWarning bool - ExpectFailure bool -} - -// Verifies that Go CLI changes without a minimum-version bump create or update the reminder comment. -func TestCommentCliMinimumVersionWarningWarnsForGoCliChange(t *testing.T) { - result := runCommentScriptCase(t, commentScriptOptions{ - Mutation: "go-cli", - BaseRef: "HEAD^", - IncludePRNumber: true, - }) - - assertContains(t, result.GitHubLog, "POST repos/hatayama/unity-cli-loop/issues/123/comments") - assertContains(t, result.BodyLog, "Go CLI files changed") - assertContains(t, result.Output, "Posted CLI minimum version comment.") - - result = runCommentScriptCase(t, commentScriptOptions{ - ExistingCommentID: "456", - Mutation: "go-cli", - BaseRef: "HEAD^", - IncludePRNumber: true, - }) - - assertContains(t, result.GitHubLog, "PATCH repos/hatayama/unity-cli-loop/issues/comments/456") - assertContains(t, result.BodyLog, "Go CLI files changed") - assertNotContains(t, result.GitHubLog, "POST") - assertContains(t, result.Output, "Updated CLI minimum version comment.") -} - -// Verifies that CI check mode fails instead of only writing a reminder comment. -func TestCommentCliMinimumVersionWarningFailsCheckModeForGoCliChange(t *testing.T) { - result := runCommentScriptCase(t, commentScriptOptions{ - Mutation: "go-cli", - BaseRef: "HEAD^", - IncludePRNumber: true, - FailOnWarning: true, - ExpectFailure: true, - }) - - assertContains(t, result.Output, "MINIMUM_REQUIRED_CLI_VERSION") - assertNotContains(t, result.GitHubLog, "POST") - assertNotContains(t, result.GitHubLog, "PATCH") -} - -// Verifies that touching the constants file without changing the required CLI still fails check mode. -func TestCommentCliMinimumVersionWarningFailsCheckModeForUnchangedMinimumVersion(t *testing.T) { - result := runCommentScriptCase(t, commentScriptOptions{ - Mutation: "go-cli-and-unchanged-minimum", - BaseRef: "HEAD^", - IncludePRNumber: true, - FailOnWarning: true, - ExpectFailure: true, - }) - - assertContains(t, result.Output, "MINIMUM_REQUIRED_CLI_VERSION") - assertNotContains(t, result.GitHubLog, "POST") - assertNotContains(t, result.GitHubLog, "PATCH") -} - -// Verifies that pull_request_target can diff a fetched PR head without checking it out. -func TestCommentCliMinimumVersionWarningUsesExplicitHeadRef(t *testing.T) { - workDir := t.TempDir() - repositoryPath := createRepository(t, workDir) - baseBranch := strings.TrimSpace(runCommand(t, repositoryPath, "git", "branch", "--show-current")) - - runCommand(t, repositoryPath, "git", "switch", "-q", "-c", "pr-head") - writeFile(t, filepath.Join(repositoryPath, goCliPath), "package cli // changed") - runCommand(t, repositoryPath, "git", "add", ".") - runCommand(t, repositoryPath, "git", "commit", "-q", "-m", "go-cli") - runCommand(t, repositoryPath, "git", "switch", "-q", baseBranch) - - result := runCommentScript(t, workDir, repositoryPath, commentScriptOptions{ - BaseRef: "HEAD", - HeadRef: "pr-head", - IncludePRNumber: true, - }) - - assertContains(t, result.GitHubLog, "POST repos/hatayama/unity-cli-loop/issues/123/comments") - assertContains(t, result.BodyLog, "Go CLI files changed") -} - -// Verifies that a matching minimum-version update resolves an existing reminder. -func TestCommentCliMinimumVersionWarningResolvesExistingReminder(t *testing.T) { - result := runCommentScriptCase(t, commentScriptOptions{ - ExistingCommentID: "456", - Mutation: "go-cli-and-minimum", - BaseRef: "HEAD^", - IncludePRNumber: true, - }) - - assertContains(t, result.GitHubLog, "PATCH repos/hatayama/unity-cli-loop/issues/comments/456") - assertContains(t, result.BodyLog, "Resolved:") - assertContains(t, result.Output, "Resolved CLI minimum version comment.") -} - -// Verifies that unrelated changes do not create a reminder comment. -func TestCommentCliMinimumVersionWarningIgnoresUnrelatedChanges(t *testing.T) { - result := runCommentScriptCase(t, commentScriptOptions{ - Mutation: "docs", - BaseRef: "HEAD^", - IncludePRNumber: true, - }) - - assertNotContains(t, result.GitHubLog, "POST") - assertNotContains(t, result.GitHubLog, "PATCH") -} - -// Verifies that release-please release PRs skip check mode even with Go CLI changes. -func TestCommentCliMinimumVersionWarningSkipsReleasePleaseBranch(t *testing.T) { - result := runCommentScriptCase(t, commentScriptOptions{ - Mutation: "go-cli", - BaseRef: "HEAD^", - HeadBranch: "release-please--branches--v3-beta", - IncludePRNumber: true, - FailOnWarning: true, - }) - - assertContains(t, result.Output, "release-please release PR") - assertNotContains(t, result.GitHubLog, "POST") - assertNotContains(t, result.GitHubLog, "PATCH") -} - -// Verifies that non-PR runs stop before calling GitHub issue APIs. -func TestCommentCliMinimumVersionWarningSkipsNonPullRequestRuns(t *testing.T) { - result := runCommentScriptCase(t, commentScriptOptions{ - BaseRef: "HEAD^", - }) - - assertContains(t, result.Output, "no PR number") - assertNotContains(t, result.GitHubLog, "repos/") -} - -func runCommentScriptCase(t *testing.T, options commentScriptOptions) commentScriptResult { - t.Helper() - - workDir := t.TempDir() - repositoryPath := createRepository(t, workDir) - - switch options.Mutation { - case "go-cli": - writeFile(t, filepath.Join(repositoryPath, goCliPath), "package cli // changed") - case "go-cli-and-minimum": - writeFile(t, filepath.Join(repositoryPath, goCliPath), "package cli // changed") - writeFile(t, filepath.Join(repositoryPath, minimumVersionPath), `public const string MINIMUM_REQUIRED_CLI_VERSION = "3.0.0-beta.15";`) - case "go-cli-and-unchanged-minimum": - writeFile(t, filepath.Join(repositoryPath, goCliPath), "package cli // changed") - writeFile(t, filepath.Join(repositoryPath, minimumVersionPath), `public const string MINIMUM_REQUIRED_CLI_VERSION = "3.0.0-beta.14"; // touched`) - case "docs": - writeFile(t, filepath.Join(repositoryPath, "README.md"), "documentation") - case "": - default: - t.Fatalf("unknown mutation: %s", options.Mutation) - } - - if options.Mutation != "" { - runCommand(t, repositoryPath, "git", "add", ".") - runCommand(t, repositoryPath, "git", "commit", "-q", "-m", options.Mutation) - } - - return runCommentScript(t, workDir, repositoryPath, options) -} - -func runCommentScript( - t *testing.T, - workDir string, - repositoryPath string, - options commentScriptOptions, -) commentScriptResult { - t.Helper() - - mockBinPath := writeMockGitHubCli(t, workDir) - gitHubLogPath := filepath.Join(workDir, "gh.log") - bodyLogPath := filepath.Join(workDir, "body.log") - writeFile(t, gitHubLogPath, "") - writeFile(t, bodyLogPath, "") - - command := exec.Command(filepath.Join(repositoryRoot(t), commentScriptPath)) - command.Dir = repositoryPath - // GITHUB_HEAD_REF is always set explicitly because CI inherits the real PR head - // branch into the test process, which would trigger the release-please skip path. - command.Env = append(os.Environ(), - "PATH="+mockBinPath+string(os.PathListSeparator)+os.Getenv("PATH"), - "GH_LOG="+gitHubLogPath, - "GH_BODY_LOG="+bodyLogPath, - "GH_EXISTING_COMMENT_ID="+options.ExistingCommentID, - "ULOOP_REPOSITORY_ROOT="+repositoryPath, - "GITHUB_REPOSITORY=hatayama/unity-cli-loop", - "GITHUB_HEAD_REF="+options.HeadBranch, - ) - if options.IncludePRNumber { - command.Env = append(command.Env, "PR_NUMBER=123") - } - if options.BaseRef != "" { - command.Env = append(command.Env, "CLI_MINIMUM_VERSION_BASE_REF="+options.BaseRef) - } - if options.HeadRef != "" { - command.Env = append(command.Env, "CLI_MINIMUM_VERSION_HEAD_REF="+options.HeadRef) - } - if options.FailOnWarning { - command.Env = append(command.Env, "CLI_MINIMUM_VERSION_FAIL_ON_WARNING=true") - } - - output, err := command.CombinedOutput() - if options.ExpectFailure { - if err == nil { - t.Fatalf("expected comment script to fail, got success\n%s", string(output)) - } - } else if err != nil { - t.Fatalf("comment script failed: %v\n%s", err, string(output)) - } - - return commentScriptResult{ - GitHubLog: readFile(t, gitHubLogPath), - BodyLog: readFile(t, bodyLogPath), - Output: string(output), - } -} - -func createRepository(t *testing.T, workDir string) string { - t.Helper() - - repositoryPath := filepath.Join(workDir, "repo") - runCommand(t, workDir, "git", "init", "-q", repositoryPath) - runCommand(t, repositoryPath, "git", "config", "user.email", "test@example.invalid") - runCommand(t, repositoryPath, "git", "config", "user.name", "Test User") - writeFile(t, filepath.Join(repositoryPath, goCliPath), "package cli") - writeFile(t, filepath.Join(repositoryPath, minimumVersionPath), `public const string MINIMUM_REQUIRED_CLI_VERSION = "3.0.0-beta.14";`) - runCommand(t, repositoryPath, "git", "add", ".") - runCommand(t, repositoryPath, "git", "commit", "-q", "-m", "base") - return repositoryPath -} - -func writeMockGitHubCli(t *testing.T, workDir string) string { - t.Helper() - - mockBinPath := filepath.Join(workDir, "bin") - if err := os.MkdirAll(mockBinPath, 0o755); err != nil { - t.Fatalf("failed to create mock bin: %v", err) - } - - mockGitHubCliPath := filepath.Join(mockBinPath, "gh") - writeFile(t, mockGitHubCliPath, `#!/bin/sh -set -eu - -method=GET -path= -input= - -while [ "$#" -gt 0 ]; do - case "$1" in - api) - ;; - --method) - shift - method=$1 - ;; - --input) - shift - input=$1 - ;; - --jq) - shift - ;; - --*) - ;; - *) - if [ -z "$path" ]; then - path=$1 - fi - ;; - esac - shift -done - -printf '%s %s\n' "$method" "$path" >> "$GH_LOG" - -if [ "$method" = "GET" ]; then - if [ -n "${GH_EXISTING_COMMENT_ID:-}" ]; then - printf '%s\n' "$GH_EXISTING_COMMENT_ID" - fi - exit 0 -fi - -if [ -n "$input" ]; then - cat "$input" >> "$GH_BODY_LOG" -fi -`) - if err := os.Chmod(mockGitHubCliPath, 0o755); err != nil { - t.Fatalf("failed to chmod mock gh: %v", err) - } - - return mockBinPath -} - -func repositoryRoot(t *testing.T) string { - t.Helper() - - _, filePath, _, ok := runtime.Caller(0) - if !ok { - t.Fatal("failed to resolve test file path") - } - - directory := filepath.Dir(filePath) - for { - if _, err := os.Stat(filepath.Join(directory, commentScriptPath)); err == nil { - return directory - } - - parent := filepath.Dir(directory) - if parent == directory { - t.Fatalf("failed to find repository root from %s", filePath) - } - directory = parent - } -} - -func runCommand(t *testing.T, workDir string, name string, args ...string) string { - t.Helper() - - command := exec.Command(name, args...) - command.Dir = workDir - output, err := command.CombinedOutput() - if err != nil { - t.Fatalf("%s %s failed: %v\n%s", name, strings.Join(args, " "), err, string(output)) - } - return string(output) -} - -func writeFile(t *testing.T, path string, content string) { - t.Helper() - - if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { - t.Fatalf("failed to create directory for %s: %v", path, err) - } - if err := os.WriteFile(path, []byte(content+"\n"), 0o644); err != nil { - t.Fatalf("failed to write %s: %v", path, err) - } -} - -func readFile(t *testing.T, path string) string { - t.Helper() - - content, err := os.ReadFile(path) - if err != nil { - t.Fatalf("failed to read %s: %v", path, err) - } - return string(content) -} - -func assertContains(t *testing.T, actual string, expected string) { - t.Helper() - - if !strings.Contains(actual, expected) { - t.Fatalf("expected text to contain %q\nactual:\n%s", expected, actual) - } -} - -func assertNotContains(t *testing.T, actual string, unexpected string) { - t.Helper() - - if strings.Contains(actual, unexpected) { - t.Fatalf("expected text not to contain %q\nactual:\n%s", unexpected, actual) - } -} diff --git a/cli/internal/automation/minimum_version_warning.go b/cli/internal/automation/minimum_version_warning.go deleted file mode 100644 index 459bd353b2..0000000000 --- a/cli/internal/automation/minimum_version_warning.go +++ /dev/null @@ -1,380 +0,0 @@ -package automation - -import ( - "bytes" - "context" - "encoding/json" - "fmt" - "io" - "os" - "os/exec" - "path/filepath" - "regexp" - "strings" -) - -const ( - minimumVersionWarningFile = "Packages/src/Editor/Domain/CliConstants.cs" - minimumVersionWarningMarker = "" - minimumVersionWarningBody = minimumVersionWarningMarker + ` -Warning: Go CLI files changed, but ` + "`MINIMUM_REQUIRED_CLI_VERSION`" + ` was not updated. - -Please confirm whether the Unity package can still accept older CLI versions. If the package now depends on new CLI behavior, update ` + "`Packages/src/Editor/Domain/CliConstants.cs`" + `. -` - resolvedMinimumVersionWarningBody = minimumVersionWarningMarker + ` -Resolved: this PR no longer has Go CLI changes without a ` + "`MINIMUM_REQUIRED_CLI_VERSION`" + ` update. -` - goCliPackageRoot = "cli/" - // release-please names its release branches "release-please--branches--". - releasePleaseBranchPrefix = "release-please--" -) - -var minimumVersionWarningValuePattern = regexp.MustCompile(`MINIMUM_REQUIRED_CLI_VERSION\s*=\s*"([^"]+)"`) - -type minimumVersionWarningConfig struct { - repositoryRoot string - pullRequest string - repository string - baseRef string - headRef string - headBranch string - failOnWarning bool -} - -func RunMinimumVersionWarning(ctx context.Context, stdout io.Writer, stderr io.Writer) int { - config, err := minimumVersionWarningConfigFromEnvironment() - if err != nil { - writeMinimumVersionWarningLine(stderr, err) - return 1 - } - if config.pullRequest == "" && !config.failOnWarning { - writeMinimumVersionWarningLine(stdout, "Skipping CLI minimum version comment because no PR number was provided.") - return 0 - } - - // Release PRs only contain release-please version stamps (cli/contract.json and - // internal/tools/default-tools.json); the reminder targets human-authored CLI changes. - if strings.HasPrefix(config.headBranch, releasePleaseBranchPrefix) { - writeMinimumVersionWarningLine(stdout, "Skipping CLI minimum version check because this is a release-please release PR.") - return 0 - } - - if config.baseRef == "" { - writeMinimumVersionWarningLine(stdout, "Skipping CLI minimum version comment because no base ref was provided.") - return 0 - } - - changedFiles, err := minimumVersionWarningChangedFiles(ctx, config) - if err != nil { - writeMinimumVersionWarningLine(stderr, err) - return 1 - } - - requiresComment, err := minimumVersionWarningRequiresCommentForDiff(ctx, config, changedFiles) - if err != nil { - writeMinimumVersionWarningLine(stderr, err) - return 1 - } - if config.failOnWarning { - if requiresComment { - writeMinimumVersionWarningLine(stderr, strings.TrimSpace(minimumVersionWarningBody)) - return 1 - } - return 0 - } - - repository, err := resolveMinimumVersionWarningRepository(ctx, config) - if err != nil { - writeMinimumVersionWarningLine(stderr, err) - return 1 - } - config.repository = repository - - if requiresComment { - message, err := upsertMinimumVersionWarningComment(ctx, config, minimumVersionWarningBody) - if err != nil { - writeMinimumVersionWarningLine(stderr, err) - return 1 - } - writeMinimumVersionWarningLine(stdout, message) - return 0 - } - - resolved, err := resolveMinimumVersionWarningComment(ctx, config) - if err != nil { - writeMinimumVersionWarningLine(stderr, err) - return 1 - } - if resolved { - writeMinimumVersionWarningLine(stdout, "Resolved CLI minimum version comment.") - } - return 0 -} - -func minimumVersionWarningConfigFromEnvironment() (minimumVersionWarningConfig, error) { - repositoryRoot := os.Getenv("ULOOP_REPOSITORY_ROOT") - if repositoryRoot == "" { - workingDirectory, err := os.Getwd() - if err != nil { - return minimumVersionWarningConfig{}, fmt.Errorf("failed to resolve repository root: %w", err) - } - repositoryRoot = workingDirectory - } - - baseRef := os.Getenv("CLI_MINIMUM_VERSION_BASE_REF") - if baseRef == "" && os.Getenv("GITHUB_BASE_REF") != "" { - baseRef = "origin/" + os.Getenv("GITHUB_BASE_REF") - } - - headRef := os.Getenv("CLI_MINIMUM_VERSION_HEAD_REF") - if headRef == "" { - headRef = "HEAD" - } - - return minimumVersionWarningConfig{ - repositoryRoot: repositoryRoot, - pullRequest: os.Getenv("PR_NUMBER"), - repository: os.Getenv("GITHUB_REPOSITORY"), - baseRef: baseRef, - headRef: headRef, - headBranch: os.Getenv("GITHUB_HEAD_REF"), - failOnWarning: os.Getenv("CLI_MINIMUM_VERSION_FAIL_ON_WARNING") == "true", - }, nil -} - -func resolveMinimumVersionWarningRepository(ctx context.Context, config minimumVersionWarningConfig) (string, error) { - if config.repository != "" { - return config.repository, nil - } - output, err := runMinimumVersionWarningOutput(ctx, config.repositoryRoot, "gh", "repo", "view", "--json", "nameWithOwner", "--jq", ".nameWithOwner") - if err != nil { - return "", err - } - return strings.TrimSpace(output), nil -} - -func minimumVersionWarningChangedFiles(ctx context.Context, config minimumVersionWarningConfig) ([]string, error) { - output, err := runMinimumVersionWarningOutput(ctx, config.repositoryRoot, "git", "-C", config.repositoryRoot, "diff", "--name-only", config.baseRef+"..."+config.headRef, "--") - if err != nil { - return nil, err - } - - changedFiles := []string{} - for _, line := range strings.Split(output, "\n") { - changedFile := strings.TrimSpace(line) - if changedFile != "" { - changedFiles = append(changedFiles, changedFile) - } - } - return changedFiles, nil -} - -func minimumVersionWarningRequiresComment(changedFiles []string) bool { - return minimumVersionWarningHasGoCliChange(changedFiles) && - !minimumVersionWarningChangesMinimumVersionFile(changedFiles) -} - -func minimumVersionWarningRequiresCommentForDiff( - ctx context.Context, - config minimumVersionWarningConfig, - changedFiles []string, -) (bool, error) { - if !minimumVersionWarningHasGoCliChange(changedFiles) { - return false, nil - } - - if !minimumVersionWarningChangesMinimumVersionFile(changedFiles) { - return true, nil - } - - changed, err := minimumVersionWarningMinimumVersionChanged(ctx, config) - if err != nil { - return false, err - } - return !changed, nil -} - -func minimumVersionWarningHasGoCliChange(changedFiles []string) bool { - for _, changedFile := range changedFiles { - if minimumVersionWarningIsGoCliFile(changedFile) { - return true - } - } - return false -} - -func minimumVersionWarningChangesMinimumVersionFile(changedFiles []string) bool { - for _, changedFile := range changedFiles { - if changedFile == minimumVersionWarningFile { - return true - } - } - return false -} - -func minimumVersionWarningMinimumVersionChanged( - ctx context.Context, - config minimumVersionWarningConfig, -) (bool, error) { - baseVersion, err := minimumVersionWarningVersionAtRef(ctx, config, config.baseRef) - if err != nil { - return false, err - } - headVersion, err := minimumVersionWarningVersionAtRef(ctx, config, config.headRef) - if err != nil { - return false, err - } - return baseVersion != headVersion, nil -} - -func minimumVersionWarningVersionAtRef( - ctx context.Context, - config minimumVersionWarningConfig, - ref string, -) (string, error) { - output, err := runMinimumVersionWarningOutput( - ctx, - config.repositoryRoot, - "git", - "-C", - config.repositoryRoot, - "show", - ref+":"+minimumVersionWarningFile, - ) - if err != nil { - return "", err - } - - matches := minimumVersionWarningValuePattern.FindStringSubmatch(output) - if len(matches) != 2 { - return "", fmt.Errorf("%s does not define MINIMUM_REQUIRED_CLI_VERSION at %s", minimumVersionWarningFile, ref) - } - return matches[1], nil -} - -func upsertMinimumVersionWarningComment(ctx context.Context, config minimumVersionWarningConfig, body string) (string, error) { - commentID, err := existingMinimumVersionWarningComment(ctx, config) - if err != nil { - return "", err - } - bodyFile, cleanup, err := writeMinimumVersionWarningBodyFile(body) - if err != nil { - return "", err - } - defer cleanup() - - if commentID != "" { - _, err := runMinimumVersionWarningOutput(ctx, config.repositoryRoot, "gh", "api", "--method", "PATCH", "repos/"+config.repository+"/issues/comments/"+commentID, "--input", bodyFile) - return "Updated CLI minimum version comment.", err - } - _, err = runMinimumVersionWarningOutput(ctx, config.repositoryRoot, "gh", "api", "--method", "POST", "repos/"+config.repository+"/issues/"+config.pullRequest+"/comments", "--input", bodyFile) - return "Posted CLI minimum version comment.", err -} - -func resolveMinimumVersionWarningComment(ctx context.Context, config minimumVersionWarningConfig) (bool, error) { - commentID, err := existingMinimumVersionWarningComment(ctx, config) - if err != nil { - return false, err - } - if commentID == "" { - return false, nil - } - - bodyFile, cleanup, err := writeMinimumVersionWarningBodyFile(resolvedMinimumVersionWarningBody) - if err != nil { - return false, err - } - defer cleanup() - - _, err = runMinimumVersionWarningOutput(ctx, config.repositoryRoot, "gh", "api", "--method", "PATCH", "repos/"+config.repository+"/issues/comments/"+commentID, "--input", bodyFile) - return err == nil, err -} - -func existingMinimumVersionWarningComment(ctx context.Context, config minimumVersionWarningConfig) (string, error) { - output, err := runMinimumVersionWarningOutput( - ctx, - config.repositoryRoot, - "gh", - "api", - "--paginate", - "repos/"+config.repository+"/issues/"+config.pullRequest+"/comments", - "--jq", - ".[] | select(.body | contains(\""+minimumVersionWarningMarker+"\")) | .id", - ) - if err != nil { - return "", err - } - - lines := strings.Split(strings.TrimSpace(output), "\n") - for index := len(lines) - 1; index >= 0; index-- { - line := strings.TrimSpace(lines[index]) - if line != "" { - return line, nil - } - } - return "", nil -} - -func runMinimumVersionWarningOutput(ctx context.Context, workDir string, name string, args ...string) (string, error) { - command := exec.CommandContext(ctx, name, args...) - command.Dir = filepath.Clean(workDir) - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} - command.Stdout = &stdout - command.Stderr = &stderr - err := command.Run() - if err != nil { - return "", fmt.Errorf("%s %s failed: %w\n%s%s", name, strings.Join(args, " "), err, stderr.String(), stdout.String()) - } - return stdout.String(), nil -} - -func minimumVersionWarningIsGoCliFile(changedFile string) bool { - if changedFile == goCliPackageRoot+"CHANGELOG.md" || - strings.HasPrefix(changedFile, goCliPackageRoot+"dist/") || - strings.HasPrefix(changedFile, goCliPackageRoot+"cmd/comment-cli-minimum-version-warning/") || - strings.HasPrefix(changedFile, goCliPackageRoot+"cmd/dispatch-release-please-pr-checks/") || - strings.HasPrefix(changedFile, goCliPackageRoot+"internal/automation/") || - strings.HasSuffix(changedFile, "_test.go") { - return false - } - if strings.HasPrefix(changedFile, goCliPackageRoot+"cmd/") || strings.HasPrefix(changedFile, goCliPackageRoot+"internal/") { - return true - } - switch changedFile { - case goCliPackageRoot + "go.mod", - goCliPackageRoot + "go.sum", - goCliPackageRoot + "contract.json", - goCliPackageRoot + "layout-contract.json": - return true - default: - relativePath := strings.TrimPrefix(changedFile, goCliPackageRoot) - return relativePath != changedFile && strings.HasSuffix(relativePath, ".go") && !strings.Contains(relativePath, "/") - } -} - -func writeMinimumVersionWarningBodyFile(body string) (string, func(), error) { - bodyFile, err := os.CreateTemp("", "uloop-minimum-version-warning-*.json") - if err != nil { - return "", func() {}, fmt.Errorf("failed to create comment body file: %w", err) - } - cleanup := func() { _ = os.Remove(bodyFile.Name()) } - - err = json.NewEncoder(bodyFile).Encode(struct { - Body string `json:"body"` - }{Body: body}) - closeErr := bodyFile.Close() - if err != nil || closeErr != nil { - cleanup() - if err != nil { - return "", func() {}, fmt.Errorf("failed to encode comment body: %w", err) - } - return "", func() {}, fmt.Errorf("failed to close comment body file: %w", closeErr) - } - return bodyFile.Name(), cleanup, nil -} - -func writeMinimumVersionWarningLine(writer io.Writer, values ...any) { - // CI status output failures cannot be recovered after the command outcome is known. - _, _ = fmt.Fprintln(writer, values...) -} diff --git a/cli/internal/automation/minimum_version_warning_test.go b/cli/internal/automation/minimum_version_warning_test.go deleted file mode 100644 index aac3006b57..0000000000 --- a/cli/internal/automation/minimum_version_warning_test.go +++ /dev/null @@ -1,205 +0,0 @@ -package automation - -import ( - "bytes" - "context" - "os" - "path/filepath" - "testing" -) - -// Verifies that GitHub Actions environment values map to the diff configuration. -func TestMinimumVersionWarningConfigUsesGitHubEnvironmentFallbacks(t *testing.T) { - t.Setenv("ULOOP_REPOSITORY_ROOT", "/tmp/repository") - t.Setenv("PR_NUMBER", "123") - t.Setenv("GITHUB_REPOSITORY", "owner/repository") - t.Setenv("GITHUB_BASE_REF", "main") - t.Setenv("CLI_MINIMUM_VERSION_BASE_REF", "") - t.Setenv("CLI_MINIMUM_VERSION_HEAD_REF", "") - - config, err := minimumVersionWarningConfigFromEnvironment() - if err != nil { - t.Fatalf("expected environment config, got error: %v", err) - } - - if config.repositoryRoot != "/tmp/repository" { - t.Fatalf("expected repository root from env, got %q", config.repositoryRoot) - } - if config.pullRequest != "123" { - t.Fatalf("expected pull request from env, got %q", config.pullRequest) - } - if config.repository != "owner/repository" { - t.Fatalf("expected repository from env, got %q", config.repository) - } - if config.baseRef != "origin/main" { - t.Fatalf("expected base ref fallback, got %q", config.baseRef) - } - if config.headRef != "HEAD" { - t.Fatalf("expected default head ref, got %q", config.headRef) - } -} - -// Verifies that missing base refs skip before repository resolution needs gh. -func TestMinimumVersionWarningSkipsMissingBaseRefBeforeRepositoryResolution(t *testing.T) { - t.Setenv("ULOOP_REPOSITORY_ROOT", t.TempDir()) - t.Setenv("PR_NUMBER", "123") - t.Setenv("GITHUB_REPOSITORY", "") - t.Setenv("GITHUB_BASE_REF", "") - // CI sets the real PR head branch for every step; clear it so the - // release-please skip cannot fire before the base-ref skip under test. - t.Setenv("GITHUB_HEAD_REF", "") - t.Setenv("CLI_MINIMUM_VERSION_BASE_REF", "") - t.Setenv("CLI_MINIMUM_VERSION_HEAD_REF", "") - t.Setenv("PATH", t.TempDir()) - - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} - exitCode := RunMinimumVersionWarning(context.Background(), &stdout, &stderr) - - if exitCode != 0 { - t.Fatalf("expected missing base ref to skip with exit code 0, got %d", exitCode) - } - if stdout.String() != "Skipping CLI minimum version comment because no base ref was provided.\n" { - t.Fatalf("expected base-ref skip message, got %q", stdout.String()) - } - if stderr.String() != "" { - t.Fatalf("expected no stderr on base-ref skip, got %q", stderr.String()) - } -} - -// Verifies that release-please release PRs skip the check before any git diff runs. -func TestMinimumVersionWarningSkipsReleasePleaseHeadBranch(t *testing.T) { - t.Setenv("ULOOP_REPOSITORY_ROOT", t.TempDir()) - t.Setenv("PR_NUMBER", "123") - t.Setenv("GITHUB_REPOSITORY", "owner/repository") - t.Setenv("GITHUB_BASE_REF", "v3-beta") - t.Setenv("GITHUB_HEAD_REF", "release-please--branches--v3-beta") - t.Setenv("CLI_MINIMUM_VERSION_BASE_REF", "") - t.Setenv("CLI_MINIMUM_VERSION_HEAD_REF", "") - t.Setenv("CLI_MINIMUM_VERSION_FAIL_ON_WARNING", "true") - t.Setenv("PATH", t.TempDir()) - - stdout := bytes.Buffer{} - stderr := bytes.Buffer{} - exitCode := RunMinimumVersionWarning(context.Background(), &stdout, &stderr) - - if exitCode != 0 { - t.Fatalf("expected release-please head branch to skip with exit code 0, got %d\n%s", exitCode, stderr.String()) - } - if stdout.String() != "Skipping CLI minimum version check because this is a release-please release PR.\n" { - t.Fatalf("expected release-please skip message, got %q", stdout.String()) - } - if stderr.String() != "" { - t.Fatalf("expected no stderr on release-please skip, got %q", stderr.String()) - } -} - -// Verifies that Go CLI source changes require a minimum-version warning. -func TestMinimumVersionWarningRequiresCommentForGoCliSourceChanges(t *testing.T) { - testCases := []struct { - name string - changedFile string - shouldWarn bool - }{ - { - name: "command source", - changedFile: goCliPackageRoot + "cmd/uloop/main.go", - shouldWarn: true, - }, - { - name: "internal source", - changedFile: goCliPackageRoot + "internal/cli/run.go", - shouldWarn: true, - }, - { - name: "internal test source", - changedFile: goCliPackageRoot + "internal/architecture/comment_cli_minimum_version_warning_test.go", - shouldWarn: false, - }, - { - name: "warning command source", - changedFile: goCliPackageRoot + "cmd/comment-cli-minimum-version-warning/main.go", - shouldWarn: false, - }, - { - name: "release check command source", - changedFile: goCliPackageRoot + "cmd/dispatch-release-please-pr-checks/main.go", - shouldWarn: false, - }, - { - name: "warning automation source", - changedFile: goCliPackageRoot + "internal/automation/minimum_version_warning.go", - shouldWarn: false, - }, - { - name: "root source", - changedFile: goCliPackageRoot + "main.go", - shouldWarn: true, - }, - { - name: "module file", - changedFile: goCliPackageRoot + "go.mod", - shouldWarn: true, - }, - { - name: "contract file", - changedFile: goCliPackageRoot + "contract.json", - shouldWarn: true, - }, - { - name: "generated binary", - changedFile: goCliPackageRoot + "dist/darwin-arm64/uloop", - shouldWarn: false, - }, - { - name: "release notes", - changedFile: goCliPackageRoot + "CHANGELOG.md", - shouldWarn: false, - }, - { - name: "unrelated documentation", - changedFile: "docs/usage.md", - shouldWarn: false, - }, - } - - for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - requiresComment := minimumVersionWarningRequiresComment([]string{testCase.changedFile}) - if requiresComment != testCase.shouldWarn { - t.Fatalf("expected warning=%t for %s, got %t", testCase.shouldWarn, testCase.changedFile, requiresComment) - } - }) - } -} - -// Verifies that a minimum-version bump suppresses the warning even when Go CLI files changed. -func TestMinimumVersionWarningSkipsCommentWhenMinimumVersionChanges(t *testing.T) { - requiresComment := minimumVersionWarningRequiresComment([]string{ - goCliPackageRoot + "internal/cli/run.go", - minimumVersionWarningFile, - }) - - if requiresComment { - t.Fatal("expected minimum-version changes to suppress the warning") - } -} - -// Verifies that successful command stderr does not pollute parsed stdout. -func TestMinimumVersionWarningOutputKeepsStderrOutOfStdout(t *testing.T) { - workDir := t.TempDir() - commandPath := filepath.Join(workDir, "emit-output") - err := os.WriteFile(commandPath, []byte("#!/bin/sh\nprintf 'parsed-output\\n'\nprintf 'stderr notice\\n' >&2\n"), 0o755) - if err != nil { - t.Fatalf("failed to write test command: %v", err) - } - - output, err := runMinimumVersionWarningOutput(context.Background(), workDir, commandPath) - if err != nil { - t.Fatalf("expected command success, got error: %v", err) - } - - if output != "parsed-output\n" { - t.Fatalf("expected stdout only, got %q", output) - } -} diff --git a/cli/protocol_version_consistency_test.go b/cli/protocol_version_consistency_test.go new file mode 100644 index 0000000000..2ada83f9ff --- /dev/null +++ b/cli/protocol_version_consistency_test.go @@ -0,0 +1,45 @@ +package clicontract + +import ( + "os" + "path/filepath" + "regexp" + "strconv" + "testing" +) + +// unityProtocolConstantPath is the C# source that declares the protocol generation the +// Unity package accepts. It is relative to this package directory (the cli/ module root). +const unityProtocolConstantPath = "../Packages/src/Editor/Domain/CliConstants.cs" + +var unityRequiredProtocolVersionPattern = regexp.MustCompile(`REQUIRED_CLI_PROTOCOL_VERSION\s*=\s*(\d+)`) + +// TestProtocolVersionMatchesUnityPackage is the keystone compatibility invariant: the +// protocol generation the CLI advertises (cli/contract.json) and the generation the Unity +// package requires (CliConstants.REQUIRED_CLI_PROTOCOL_VERSION) must be identical on every +// commit. They are released together, so a breaking IPC change bumps both in one PR; if the +// two ever diverge, a CLI built from this commit would be wrongly rejected or accepted by +// its own package. This check is deterministic and needs no git diff or release numbering. +func TestProtocolVersionMatchesUnityPackage(t *testing.T) { + content, err := os.ReadFile(filepath.Clean(unityProtocolConstantPath)) + if err != nil { + t.Fatalf("failed to read Unity protocol constant from %s: %v", unityProtocolConstantPath, err) + } + + matches := unityRequiredProtocolVersionPattern.FindStringSubmatch(string(content)) + if len(matches) != 2 { + t.Fatalf("%s does not define REQUIRED_CLI_PROTOCOL_VERSION", unityProtocolConstantPath) + } + + unityProtocolVersion, err := strconv.Atoi(matches[1]) + if err != nil { + t.Fatalf("REQUIRED_CLI_PROTOCOL_VERSION is not an integer: %v", err) + } + + if Current.ProtocolVersion != unityProtocolVersion { + t.Fatalf( + "protocol version mismatch: cli/contract.json declares %d but %s requires %d; "+ + "bump both together on a breaking IPC change", + Current.ProtocolVersion, unityProtocolConstantPath, unityProtocolVersion) + } +} diff --git a/scripts/check-cli-minimum-version-warning.sh b/scripts/check-cli-minimum-version-warning.sh deleted file mode 100755 index e49f46641a..0000000000 --- a/scripts/check-cli-minimum-version-warning.sh +++ /dev/null @@ -1,9 +0,0 @@ -#!/bin/sh -set -eu - -ROOT_DIR=${ULOOP_REPOSITORY_ROOT:-$(CDPATH= cd "$(dirname "$0")/.." && pwd)} - -CLI_MINIMUM_VERSION_FAIL_ON_WARNING=true -export CLI_MINIMUM_VERSION_FAIL_ON_WARNING - -ULOOP_REPOSITORY_ROOT="$ROOT_DIR" "$ROOT_DIR/scripts/comment-cli-minimum-version-warning.sh" diff --git a/scripts/comment-cli-minimum-version-warning.sh b/scripts/comment-cli-minimum-version-warning.sh deleted file mode 100755 index 3ff9be946b..0000000000 --- a/scripts/comment-cli-minimum-version-warning.sh +++ /dev/null @@ -1,8 +0,0 @@ -#!/bin/sh -set -eu - -ROOT_DIR=${ULOOP_REPOSITORY_ROOT:-$(CDPATH= cd "$(dirname "$0")/.." && pwd)} -CLI_DIR=$(CDPATH= cd "$(dirname "$0")/../cli" && pwd) - -cd "$CLI_DIR" -ULOOP_REPOSITORY_ROOT="$ROOT_DIR" go run ./cmd/comment-cli-minimum-version-warning diff --git a/scripts/test-cli-minimum-version-warning-workflow.sh b/scripts/test-cli-minimum-version-warning-workflow.sh deleted file mode 100755 index ad79584bcb..0000000000 --- a/scripts/test-cli-minimum-version-warning-workflow.sh +++ /dev/null @@ -1,41 +0,0 @@ -#!/bin/sh -set -eu - -ROOT_DIR=$(CDPATH= cd "$(dirname "$0")/.." && pwd) -WARNING_WORKFLOW="$ROOT_DIR/.github/workflows/cli-minimum-version-warning.yml" -BUILD_WORKFLOW="$ROOT_DIR/.github/workflows/build-and-test.yml" - -assert_contains() { - file=$1 - expected=$2 - - if ! grep -F -- "$expected" "$file" >/dev/null 2>&1; then - echo "Expected $file to contain: $expected" >&2 - exit 1 - fi -} - -assert_not_contains() { - file=$1 - unexpected=$2 - - if grep -F -- "$unexpected" "$file" >/dev/null 2>&1; then - echo "Expected $file not to contain: $unexpected" >&2 - exit 1 - fi -} - -assert_contains "$WARNING_WORKFLOW" " pull_request_target:" -assert_contains "$WARNING_WORKFLOW" " issues: write" -assert_contains "$WARNING_WORKFLOW" " - name: Setup Go" -assert_contains "$WARNING_WORKFLOW" " uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c" -assert_contains "$WARNING_WORKFLOW" " go-version-file: cli/.go-version" -assert_contains "$WARNING_WORKFLOW" " cache: false" -assert_contains "$WARNING_WORKFLOW" " - name: Fetch pull request head" -assert_contains "$WARNING_WORKFLOW" " CLI_MINIMUM_VERSION_HEAD_REF: cli-minimum-version-pr-head" -assert_contains "$WARNING_WORKFLOW" " run: scripts/comment-cli-minimum-version-warning.sh" - -assert_contains "$BUILD_WORKFLOW" " - name: Check CLI minimum version bump" -assert_contains "$BUILD_WORKFLOW" " PR_NUMBER: \${{ github.event.pull_request.number }}" -assert_contains "$BUILD_WORKFLOW" " CLI_MINIMUM_VERSION_FAIL_ON_WARNING: true" -assert_contains "$BUILD_WORKFLOW" " run: scripts/check-cli-minimum-version-warning.sh" diff --git a/scripts/test-comment-cli-minimum-version-warning.sh b/scripts/test-comment-cli-minimum-version-warning.sh deleted file mode 100755 index c8f923e3a9..0000000000 --- a/scripts/test-comment-cli-minimum-version-warning.sh +++ /dev/null @@ -1,7 +0,0 @@ -#!/bin/sh -set -eu - -ROOT_DIR=$(CDPATH= cd "$(dirname "$0")/.." && pwd) - -cd "$ROOT_DIR/cli" -go test ./internal/architecture -run TestCommentCliMinimumVersionWarning -count=1 From 33b33ed60decf6d1c9b8ba9453c6686cb454111b Mon Sep 17 00:00:00 2001 From: hatayama Date: Sat, 13 Jun 2026 11:35:46 +0900 Subject: [PATCH 4/9] docs: Document the protocol-version compatibility workflow Replace the commit-time MINIMUM_REQUIRED_CLI_VERSION rule, which no longer reflects how the gate works, with guidance for the protocol version: keep cli/contract.json protocolVersion and CliConstants.REQUIRED_CLI_PROTOCOL_VERSION equal and bump both only on a breaking IPC change. Clarify that release-please owns the version stamps and that MINIMUM_REQUIRED_CLI_VERSION is the installer pin, neither of which belongs in a feature PR. --- AGENTS.md | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2edab9f431..f7d3b54b2d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -10,11 +10,27 @@ Comments in the code, commit messages, PR titles, and PR descriptions must all b Every test method must have a short comment that states what behavior the test verifies. -## Commit-Time Version Checks +## CLI / Unity Package Compatibility -Before committing a C# package version bump, check whether `CliConstants.MINIMUM_REQUIRED_CLI_VERSION` must also change. -If the package release depends on behavior from a newer native CLI contract, update the minimum required CLI version and add or update focused tests. -If the minimum CLI version stays unchanged, make sure that decision is intentional before committing. +Runtime compatibility between the Unity package and the native CLI is gated on an integer +protocol version, not on release numbers. Two declarations must always stay equal: + +- Go side: `protocolVersion` in `cli/contract.json` (the generation the CLI advertises over IPC). +- C# side: `CliConstants.REQUIRED_CLI_PROTOCOL_VERSION` (the minimum generation the package accepts). + +`TestProtocolVersionMatchesUnityPackage` fails the build if they diverge, so never bump one alone. + +Bump both, together, in the same PR only when the IPC contract changes in a way that makes an +older CLI and this package unable to interoperate — for example renaming or removing a request +field, changing the readiness/dispatch handshake, or altering a response shape the other side +parses. Ordinary CLI features and bug fixes that keep the wire format compatible must not bump it. + +Do not touch the protocol version to "keep up with releases": + +- `cli/contract.json` `cliVersion` and `cli/internal/tools/default-tools.json` `version` are + stamped by release-please only. Never edit them by hand in a feature PR. +- `CliConstants.MINIMUM_REQUIRED_CLI_VERSION` is the release that setup installs. It must always + point at a published CLI release, so it advances after a release, never inside a feature PR. ## Generated Skill Files From b99cc8762c26cd540326dcad64c96ea8ed772ed7 Mon Sep 17 00:00:00 2001 From: hatayama Date: Sat, 13 Jun 2026 12:12:46 +0900 Subject: [PATCH 5/9] Gate CLI compatibility by protocol version Use the IPC protocol generation, rather than release semver, for runtime compatibility and setup UI decisions. - Require exact protocol matches at IPC handshake time with distinct guidance for older and newer CLI protocols. - Expose `uloop --version --json` so the Unity package can detect CLI protocol metadata. - Make setup update and replacement actions protocol-aware while keeping release tags limited to installation. --- AGENTS.md | 14 ++- .../Editor/CliInstallationDetectorTests.cs | 52 ++++++++- Assets/Tests/Editor/CliPathSetupFlowTests.cs | 1 + .../Editor/CliSetupApplicationServiceTests.cs | 1 + .../Tests/Editor/CliVersionComparerTests.cs | 28 +++++ .../JsonRpcProcessorCliVersionGateTests.cs | 30 +++-- .../Editor/ProjectIpcWarmupClientTests.cs | 4 +- Assets/Tests/Editor/SetupWizardWindowTests.cs | 33 +++--- ...nityCliLoopSettingsWindowCliActionTests.cs | 80 ++++++++----- .../Application/CliSetupApplicationService.cs | 31 +++++ Packages/src/Editor/Domain/CliConstants.cs | 3 +- .../src/Editor/Domain/CliVersionComparer.cs | 10 ++ .../Infrastructure/Api/JsonRpcProcessor.cs | 39 +++++-- .../CLI/CliInstallationDetector.cs | 108 ++++++++++++++++-- .../Presentation/Setup/SetupWizardWindow.cs | 73 +++++++++--- .../UnityCliLoopSettingsWindow.cs | 54 ++++++--- cli/internal/cli/error_envelope.go | 21 +++- cli/internal/cli/error_envelope_test.go | 22 +++- cli/internal/cli/help_test.go | 23 ++++ cli/internal/cli/run.go | 15 +++ cli/internal/cli/run_help.go | 4 + cli/internal/cli/tools.go | 5 +- 22 files changed, 536 insertions(+), 115 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index f7d3b54b2d..e911f92103 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,14 +16,17 @@ Runtime compatibility between the Unity package and the native CLI is gated on a protocol version, not on release numbers. Two declarations must always stay equal: - Go side: `protocolVersion` in `cli/contract.json` (the generation the CLI advertises over IPC). -- C# side: `CliConstants.REQUIRED_CLI_PROTOCOL_VERSION` (the minimum generation the package accepts). +- C# side: `CliConstants.REQUIRED_CLI_PROTOCOL_VERSION` (the exact generation the package accepts). `TestProtocolVersionMatchesUnityPackage` fails the build if they diverge, so never bump one alone. +The runtime gate expects equality because the protocol version is a contract generation, not a +minimum-compatible range. Bump both, together, in the same PR only when the IPC contract changes in a way that makes an -older CLI and this package unable to interoperate — for example renaming or removing a request -field, changing the readiness/dispatch handshake, or altering a response shape the other side -parses. Ordinary CLI features and bug fixes that keep the wire format compatible must not bump it. +CLI and package from different protocol generations unable to interoperate — for example renaming +or removing a request field, changing the readiness/dispatch handshake, or altering a response +shape the other side parses. Ordinary CLI features and bug fixes that keep the wire format +compatible must not bump it. Do not touch the protocol version to "keep up with releases": @@ -31,6 +34,9 @@ Do not touch the protocol version to "keep up with releases": stamped by release-please only. Never edit them by hand in a feature PR. - `CliConstants.MINIMUM_REQUIRED_CLI_VERSION` is the release that setup installs. It must always point at a published CLI release, so it advances after a release, never inside a feature PR. +- Runtime protocol mismatch guidance must not pin updates to `MINIMUM_REQUIRED_CLI_VERSION`; that + value may be older than the protocol the package now requires. Use the unpinned CLI update path + for older clients and tell newer clients to align the package and CLI releases. ## Generated Skill Files diff --git a/Assets/Tests/Editor/CliInstallationDetectorTests.cs b/Assets/Tests/Editor/CliInstallationDetectorTests.cs index e566d9fb6f..d29ca0d352 100644 --- a/Assets/Tests/Editor/CliInstallationDetectorTests.cs +++ b/Assets/Tests/Editor/CliInstallationDetectorTests.cs @@ -17,9 +17,11 @@ public void SelectPreferredDetection_WhenShellCommandShadowsPackageOwnedCliUsesS // Verifies that the settings UI reports the same CLI command the user's terminal runs. CliInstallationDetection packageOwnedDetection = new( "3.0.0-beta.3", + 1, "/Users/ExampleUser/.local/bin/uloop"); CliInstallationDetection shellDetection = new( "2.1.0", + null, "/Users/ExampleUser/.npm-global/bin/uloop"); CliInstallationDetection result = CliInstallationDetector.SelectPreferredDetection( @@ -36,8 +38,10 @@ public void SelectPreferredDetection_WhenShellCommandMissingUsesPackageOwnedCli( // Verifies that package-owned installs still count when the shell cannot resolve uloop. CliInstallationDetection packageOwnedDetection = new( "3.0.0-beta.3", + 1, "/Users/ExampleUser/.local/bin/uloop"); CliInstallationDetection shellDetection = new( + null, null, null); @@ -55,8 +59,10 @@ public void SelectPreferredDetection_WhenShellCommandExistsButVersionFailsUsesSh // Verifies that a broken PATH command is surfaced instead of hidden by the package-owned binary. CliInstallationDetection packageOwnedDetection = new( "3.0.0-beta.3", + 1, "/Users/ExampleUser/.local/bin/uloop"); CliInstallationDetection shellDetection = new( + null, null, "/Users/ExampleUser/.npm-global/bin/uloop"); @@ -73,10 +79,12 @@ public void SelectPreferredDetection_WhenPackageOwnedCliMissingUsesShellPath() { // Verifies that legacy CLI installs still surface as update candidates. CliInstallationDetection packageOwnedDetection = new( + null, null, "/Users/ExampleUser/.local/bin/uloop"); CliInstallationDetection shellDetection = new( "2.1.0", + null, "/Users/ExampleUser/.npm-global/bin/uloop"); CliInstallationDetection result = CliInstallationDetector.SelectPreferredDetection( @@ -93,9 +101,11 @@ public void SelectPreferredDetection_WhenShellVersionExistsWithoutPathUsesShellV // Verifies that installed state does not depend on command path availability. CliInstallationDetection packageOwnedDetection = new( "3.0.0-beta.3", + 1, "/Users/ExampleUser/.local/bin/uloop"); CliInstallationDetection shellDetection = new( "2.1.0", + null, null); CliInstallationDetection result = CliInstallationDetector.SelectPreferredDetection( @@ -113,10 +123,12 @@ public void BuildShellCliDetectionCommand_UsesShortVersionFlag() string command = CliInstallationDetector.BuildShellCliDetectionCommand("uloop"); Assert.That(command, Does.Contain("command -v uloop")); + Assert.That(command, Does.Contain("uloop --version --json")); + Assert.That(command, Does.Contain("uloop_contract_status=$?")); Assert.That(command, Does.Contain("uloop -v")); Assert.That(command, Does.Contain("uloop_version_status=$?")); + Assert.That(command, Does.Contain("__ULOOP_CONTRACT_STATUS_START__")); Assert.That(command, Does.Contain("__ULOOP_VERSION_STATUS_START__")); - Assert.That(command, Does.Not.Contain("uloop --version")); } [Test] @@ -128,6 +140,7 @@ public void BuildShellCliDetectionCommandForShell_WhenRuntimeShellIsFish_UsesFis "/opt/homebrew/bin/fish"); Assert.That(command, Does.Contain("set uloop_version_status $status")); + Assert.That(command, Does.Contain("set uloop_contract_status $status")); Assert.That(command, Does.Not.Contain("uloop_version_status=$?")); } @@ -139,6 +152,12 @@ public void ParseShellCliInstallationOutput_WhenPathAndVersionExist_ReturnsDetec + "__ULOOP_PATH_START__\n" + "/Users/ExampleUser/.npm-global/bin/uloop\n" + "__ULOOP_PATH_END__\n" + + "__ULOOP_CONTRACT_START__\n" + + "unknown option: --json\n" + + "__ULOOP_CONTRACT_END__\n" + + "__ULOOP_CONTRACT_STATUS_START__\n" + + "1\n" + + "__ULOOP_CONTRACT_STATUS_END__\n" + "__ULOOP_VERSION_START__\n" + "2.1.1\n" + "__ULOOP_VERSION_END__\n" @@ -150,6 +169,35 @@ public void ParseShellCliInstallationOutput_WhenPathAndVersionExist_ReturnsDetec CliInstallationDetector.ParseShellCliInstallationOutput(output); Assert.That(detection.Version, Is.EqualTo("2.1.1")); + Assert.That(detection.ProtocolVersion, Is.Null); + Assert.That(detection.ExecutablePath, Is.EqualTo("/Users/ExampleUser/.npm-global/bin/uloop")); + } + + [Test] + public void ParseShellCliInstallationOutput_WhenContractJsonExists_ReturnsProtocolDetection() + { + // Verifies that new CLIs expose protocol metadata for setup compatibility checks. + string output = "__ULOOP_PATH_START__\n" + + "/Users/ExampleUser/.npm-global/bin/uloop\n" + + "__ULOOP_PATH_END__\n" + + "__ULOOP_CONTRACT_START__\n" + + "{\"cliVersion\":\"3.0.0-beta.31\",\"protocolVersion\":1}\n" + + "__ULOOP_CONTRACT_END__\n" + + "__ULOOP_CONTRACT_STATUS_START__\n" + + "0\n" + + "__ULOOP_CONTRACT_STATUS_END__\n" + + "__ULOOP_VERSION_START__\n" + + "2.1.1\n" + + "__ULOOP_VERSION_END__\n" + + "__ULOOP_VERSION_STATUS_START__\n" + + "0\n" + + "__ULOOP_VERSION_STATUS_END__\n"; + + CliInstallationDetection detection = + CliInstallationDetector.ParseShellCliInstallationOutput(output); + + Assert.That(detection.Version, Is.EqualTo("3.0.0-beta.31")); + Assert.That(detection.ProtocolVersion, Is.EqualTo(1)); Assert.That(detection.ExecutablePath, Is.EqualTo("/Users/ExampleUser/.npm-global/bin/uloop")); } @@ -170,6 +218,7 @@ public void ParseShellCliInstallationOutput_WhenOnlyVersionExists_ReturnsInstall CliInstallationDetector.ParseShellCliInstallationOutput(output); Assert.That(detection.Version, Is.EqualTo("2.1.1")); + Assert.That(detection.ProtocolVersion, Is.Null); Assert.That(detection.ExecutablePath, Is.Null); } @@ -191,6 +240,7 @@ public void ParseShellCliInstallationOutput_WhenVersionCommandFails_ReturnsPathW CliInstallationDetector.ParseShellCliInstallationOutput(output); Assert.That(detection.Version, Is.Null); + Assert.That(detection.ProtocolVersion, Is.Null); Assert.That(detection.ExecutablePath, Is.EqualTo("/Users/ExampleUser/.npm-global/bin/uloop")); } diff --git a/Assets/Tests/Editor/CliPathSetupFlowTests.cs b/Assets/Tests/Editor/CliPathSetupFlowTests.cs index 580b489d1a..b7dc02a107 100644 --- a/Assets/Tests/Editor/CliPathSetupFlowTests.cs +++ b/Assets/Tests/Editor/CliPathSetupFlowTests.cs @@ -134,6 +134,7 @@ public FakeCliInstallationDetector(params bool[] visibilityResults) public bool IsCliInstalled() => true; public string GetCachedCliVersion() => "3.0.0-beta.14"; + public int? GetCachedCliProtocolVersion() => 1; public string GetCachedCliExecutablePath() => "/Users/ExampleUser/.local/bin/uloop"; public bool IsCheckCompleted() => true; public Task RefreshCliVersionAsync(CancellationToken ct) => Task.CompletedTask; diff --git a/Assets/Tests/Editor/CliSetupApplicationServiceTests.cs b/Assets/Tests/Editor/CliSetupApplicationServiceTests.cs index fbfe10131e..e3883ba8a0 100644 --- a/Assets/Tests/Editor/CliSetupApplicationServiceTests.cs +++ b/Assets/Tests/Editor/CliSetupApplicationServiceTests.cs @@ -84,6 +84,7 @@ public FakeCliInstallationDetector(string[] versions) } public bool IsCliInstalled() => GetCachedCliVersion() != null; public string GetCachedCliVersion() => _versions[_versionIndex]; + public int? GetCachedCliProtocolVersion() => null; public string GetCachedCliExecutablePath() => ""; public bool IsCheckCompleted() => true; public Task RefreshCliVersionAsync(CancellationToken ct) => Task.CompletedTask; diff --git a/Assets/Tests/Editor/CliVersionComparerTests.cs b/Assets/Tests/Editor/CliVersionComparerTests.cs index 738f0931aa..08865a9aaf 100644 --- a/Assets/Tests/Editor/CliVersionComparerTests.cs +++ b/Assets/Tests/Editor/CliVersionComparerTests.cs @@ -40,6 +40,34 @@ public void IsVersionLessThan_ReturnsExpectedResult( Assert.That(result, Is.EqualTo(expected)); } + [TestCase("3.0.0-beta.2", "3.0.0-beta.1", true)] + [TestCase("3.0.0-beta.1", "3.0.0-beta.1", false)] + [TestCase("3.0.0-beta.0", "3.0.0-beta.1", false)] + public void IsVersionGreaterThan_ReturnsExpectedResult( + string leftVersion, + string rightVersion, + bool expected) + { + // Verifies strict greater-than comparison for CLI setup downgrade detection. + bool result = CliVersionComparer.IsVersionGreaterThan(leftVersion, rightVersion); + + Assert.That(result, Is.EqualTo(expected)); + } + + [TestCase("3.0.0-beta.1", "3.0.0-beta.1", true)] + [TestCase("v3.0.0-beta.1", "3.0.0-beta.1", true)] + [TestCase("3.0.0-beta.2", "3.0.0-beta.1", false)] + public void IsVersionEqual_ReturnsExpectedResult( + string leftVersion, + string rightVersion, + bool expected) + { + // Verifies semantic equality comparison for CLI setup exact-match detection. + bool result = CliVersionComparer.IsVersionEqual(leftVersion, rightVersion); + + Assert.That(result, Is.EqualTo(expected)); + } + [Test] public void IsVersionGreaterThanOrEqual_WhenVersionIsInvalid_ReturnsFalse() { diff --git a/Assets/Tests/Editor/JsonRpcProcessorCliVersionGateTests.cs b/Assets/Tests/Editor/JsonRpcProcessorCliVersionGateTests.cs index 3a9aeba32c..fa507991e3 100644 --- a/Assets/Tests/Editor/JsonRpcProcessorCliVersionGateTests.cs +++ b/Assets/Tests/Editor/JsonRpcProcessorCliVersionGateTests.cs @@ -35,22 +35,30 @@ public async Task ProcessRequest_WhenProtocolVersionMatches_AllowsRequest() } [Test] - public async Task ProcessRequest_WhenProtocolVersionIsNewer_AllowsRequest() + public async Task ProcessRequest_WhenProtocolVersionIsNewer_ReturnsCliProtocolMismatchError() { - // Verifies a newer CLI keeps working against this package version. + // Verifies future protocol clients are rejected because IPC compatibility is exact-match. string response = await JsonRpcProcessor.ProcessRequest( BuildGetVersionRequest(CliConstants.REQUIRED_CLI_PROTOCOL_VERSION + 1), CancellationToken.None); - JObject parsed = JObject.Parse(response); + JObject error = ParseError(response); + JObject data = ParseErrorData(response); - Assert.That(parsed["error"], Is.Null); - Assert.That(parsed["result"], Is.Not.Null); + Assert.That(error["message"]?.ToString(), Does.Contain("does not match")); + Assert.That(data["type"]?.ToString(), Is.EqualTo("cli_update_required")); + Assert.That( + data["currentProtocolVersion"]?.ToObject(), + Is.EqualTo(CliConstants.REQUIRED_CLI_PROTOCOL_VERSION + 1)); + Assert.That( + data["requiredProtocolVersion"]?.ToObject(), + Is.EqualTo(CliConstants.REQUIRED_CLI_PROTOCOL_VERSION)); + Assert.That(data["updateCommand"], Is.Null); } [Test] public async Task ProcessRequest_WhenProtocolVersionIsTooOld_ReturnsCliUpdateRequiredError() { - // Verifies CLIs on an older IPC protocol receive update instructions before any tool runs. + // Verifies CLIs on an older IPC protocol receive CLI update instructions before any tool runs. string response = await JsonRpcProcessor.ProcessRequest( BuildGetVersionRequest(CliConstants.REQUIRED_CLI_PROTOCOL_VERSION - 1), CancellationToken.None); @@ -63,7 +71,7 @@ public async Task ProcessRequest_WhenProtocolVersionIsTooOld_ReturnsCliUpdateReq Assert.That( data["requiredProtocolVersion"]?.ToObject(), Is.EqualTo(CliConstants.REQUIRED_CLI_PROTOCOL_VERSION)); - Assert.That(data["updateCommand"]?.ToString(), Is.EqualTo("uloop update")); + Assert.That(data["updateCommand"]?.ToString(), Is.EqualTo(ExpectedCliUpdateCommand())); Assert.That(data["retryableAfterUpdate"]?.ToObject(), Is.True); } @@ -79,6 +87,7 @@ public async Task ProcessRequest_WhenCliMetadataIsMissing_ReturnsCliUpdateRequir Assert.That(data["type"]?.ToString(), Is.EqualTo("cli_update_required")); Assert.That(data["currentProtocolVersion"]?.Type, Is.EqualTo(JTokenType.Null)); Assert.That(data["currentCliVersion"]?.Type, Is.EqualTo(JTokenType.Null)); + Assert.That(data["updateCommand"]?.ToString(), Is.EqualTo(ExpectedCliUpdateCommand())); } [Test] @@ -94,6 +103,7 @@ public async Task ProcessRequest_WhenClientSendsOnlySemverVersion_ReturnsCliUpda Assert.That(data["type"]?.ToString(), Is.EqualTo("cli_update_required")); Assert.That(data["currentProtocolVersion"]?.Type, Is.EqualTo(JTokenType.Null)); Assert.That(data["currentCliVersion"]?.ToString(), Is.EqualTo("3.0.0-beta.24")); + Assert.That(data["updateCommand"]?.ToString(), Is.EqualTo(ExpectedCliUpdateCommand())); } [Test] @@ -108,6 +118,7 @@ public async Task ProcessRequest_WhenProtocolVersionIsNotAnInteger_ReturnsCliUpd Assert.That(data["type"]?.ToString(), Is.EqualTo("cli_update_required")); Assert.That(data["currentProtocolVersion"]?.Type, Is.EqualTo(JTokenType.Null)); + Assert.That(data["updateCommand"]?.ToString(), Is.EqualTo(ExpectedCliUpdateCommand())); } [Test] @@ -546,6 +557,11 @@ private static string BuildGetVersionRequest(int protocolVersion) "}}"; } + private static string ExpectedCliUpdateCommand() + { + return "uloop update"; + } + private static string BuildToolRequest(string toolName, int id) { return diff --git a/Assets/Tests/Editor/ProjectIpcWarmupClientTests.cs b/Assets/Tests/Editor/ProjectIpcWarmupClientTests.cs index bb5915ba4e..bb93203ac5 100644 --- a/Assets/Tests/Editor/ProjectIpcWarmupClientTests.cs +++ b/Assets/Tests/Editor/ProjectIpcWarmupClientTests.cs @@ -43,9 +43,9 @@ public void ValidateJsonRpcSuccessResponse_WhenResponseContainsError_Throws() InvalidOperationException exception = Assert.Throws( () => client.ValidateJsonRpcSuccessResponse( - "{\"jsonrpc\":\"2.0\",\"id\":1,\"error\":{\"code\":-32603,\"message\":\"The installed uloop CLI is too old for this Unity package.\"}}")); + "{\"jsonrpc\":\"2.0\",\"id\":1,\"error\":{\"code\":-32603,\"message\":\"The installed uloop CLI uses an IPC protocol that does not match this Unity package.\"}}")); - Assert.That(exception.Message, Does.Contain("too old")); + Assert.That(exception.Message, Does.Contain("does not match")); } [Test] diff --git a/Assets/Tests/Editor/SetupWizardWindowTests.cs b/Assets/Tests/Editor/SetupWizardWindowTests.cs index b6c7014e85..4a27e505cb 100644 --- a/Assets/Tests/Editor/SetupWizardWindowTests.cs +++ b/Assets/Tests/Editor/SetupWizardWindowTests.cs @@ -474,20 +474,23 @@ public void ShouldShowSkillsTargetList_WhenCliInstalledAndNotFirstInstall_Return Assert.That(shouldShow, Is.True); } - [TestCase(false, false, false, false, false, null, "3.0.0", "Install CLI")] - [TestCase(false, false, false, false, true, null, "3.0.0", "Fix PATH")] - [TestCase(true, false, false, false, false, "3.0.0", "3.0.0", "Installed")] - [TestCase(true, false, false, false, true, "3.0.0", "3.0.0", "Fix PATH")] - [TestCase(true, false, false, true, false, "2.9.0", "3.0.0", "Update CLI (v2.9.0 \u2192 v3.0.0)")] - [TestCase(true, false, false, true, true, "2.9.0", "3.0.0", "Update CLI (v2.9.0 \u2192 v3.0.0)")] - [TestCase(true, true, false, false, false, "3.0.0", "3.0.0", "Installing...")] - [TestCase(true, true, false, false, true, "3.0.0", "3.0.0", "Fixing PATH...")] - [TestCase(false, false, true, false, false, null, "3.0.0", "Checking...")] + [TestCase(false, false, false, false, false, false, null, "3.0.0", "Install CLI")] + [TestCase(false, false, false, false, false, true, null, "3.0.0", "Fix PATH")] + [TestCase(true, false, false, false, false, false, "3.0.0", "3.0.0", "Installed")] + [TestCase(true, false, false, false, false, true, "3.0.0", "3.0.0", "Fix PATH")] + [TestCase(true, false, false, true, false, false, "2.9.0", "3.0.0", "Update CLI (v2.9.0 \u2192 v3.0.0)")] + [TestCase(true, false, false, true, false, true, "2.9.0", "3.0.0", "Update CLI (v2.9.0 \u2192 v3.0.0)")] + [TestCase(true, false, false, false, true, false, "3.1.0", "3.0.0", "Downgrade CLI (v3.1.0 \u2192 v3.0.0)")] + [TestCase(true, true, false, false, false, false, "3.0.0", "3.0.0", "Installing...")] + [TestCase(true, true, false, false, false, true, "3.0.0", "3.0.0", "Fixing PATH...")] + [TestCase(true, true, false, false, true, true, "3.1.0", "3.0.0", "Installing...")] + [TestCase(false, false, true, false, false, false, null, "3.0.0", "Checking...")] public void GetCliButtonTextForSetupWizard_ReturnsExpectedLabel( bool cliInstalled, bool isInstallingCli, bool isChecking, bool needsUpdate, + bool needsDowngrade, bool needsCliPathSetup, string cliVersion, string requiredCliVersion, @@ -498,6 +501,7 @@ public void GetCliButtonTextForSetupWizard_ReturnsExpectedLabel( isInstallingCli, isChecking, needsUpdate, + needsDowngrade, needsCliPathSetup, cliVersion, requiredCliVersion); @@ -529,18 +533,21 @@ public void IsCliButtonEnabledForSetupWizard_ReturnsExpectedValue( Assert.That(enabled, Is.EqualTo(expectedEnabled)); } - [TestCase(false, false, false)] - [TestCase(true, false, true)] - [TestCase(true, true, false)] + [TestCase(false, false, false, false)] + [TestCase(true, false, false, true)] + [TestCase(true, true, false, false)] + [TestCase(true, false, true, false)] public void ShouldRepairCliPathFromPrimaryButton_ReturnsExpectedAction( bool needsCliPathSetup, bool needsUpdate, + bool needsDowngrade, bool expected) { // Verifies that setup wizard chooses PATH repair only after the CLI version is already usable. bool result = SetupWizardWindow.ShouldRepairCliPathFromPrimaryButton( needsCliPathSetup, - needsUpdate); + needsUpdate, + needsDowngrade); Assert.That(result, Is.EqualTo(expected)); } diff --git a/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs b/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs index 5a174dba3c..aac1aeb275 100644 --- a/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs +++ b/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs @@ -10,60 +10,68 @@ namespace io.github.hatayama.UnityCliLoop.Tests.Editor /// public class UnityCliLoopSettingsWindowCliActionTests { - [TestCase(null, "3.0.0", true, false)] - [TestCase("2.9.0", "3.0.0", true, false)] - [TestCase("3.1.0", "3.0.0", true, true)] - [TestCase("3.0.0", "3.0.0", true, true)] - [TestCase("3.0.0", "3.0.0", false, false)] + [TestCase(null, null, true, false)] + [TestCase("2.9.0", null, true, false)] + [TestCase("3.1.0", 1, true, true)] + [TestCase("3.1.0", 2, true, false)] + [TestCase("3.0.0", 1, true, true)] + [TestCase("3.0.0", 1, false, false)] public void ShouldUninstallCliFromPrimaryButton_ReturnsExpectedAction( string cliVersion, - string requiredCliVersion, + int? cliProtocolVersion, bool canUninstallCli, bool expected) { - // Verifies that package-owned installs route to uninstall when the CLI satisfies the package minimum. + // Verifies that package-owned installs route to uninstall only when the CLI protocol matches the package. bool result = UnityCliLoopSettingsWindow.ShouldUninstallCliFromPrimaryButton( cliVersion, - requiredCliVersion, + cliProtocolVersion, canUninstallCli); Assert.That(result, Is.EqualTo(expected)); } - [TestCase(false, false, false)] - [TestCase(true, false, true)] - [TestCase(true, true, false)] + [TestCase(false, false, false, false)] + [TestCase(true, false, false, true)] + [TestCase(true, true, false, false)] + [TestCase(true, false, true, false)] public void ShouldRepairCliPathFromPrimaryButton_ReturnsExpectedAction( bool needsCliPathSetup, bool needsUpdate, + bool needsDowngrade, bool expected) { - // Verifies that stale terminal PATH state routes to repair only when install/update is not needed. + // Verifies that stale terminal PATH state routes to repair only when CLI replacement is not needed. bool result = UnityCliLoopSettingsWindow.ShouldRepairCliPathFromPrimaryButton( needsCliPathSetup, - needsUpdate); + needsUpdate, + needsDowngrade); Assert.That(result, Is.EqualTo(expected)); } - [TestCase(true, "3.0.0", "3.0.0", true, "RepairPath")] - [TestCase(true, "2.9.0", "3.0.0", true, "InstallOrUpdate")] - [TestCase(false, "3.0.0", "3.0.0", true, "Uninstall")] - [TestCase(false, "2.9.0", "3.0.0", true, "InstallOrUpdate")] - [TestCase(false, null, "3.0.0", true, "InstallOrUpdate")] + [TestCase(true, "3.0.0", 1, true, "RepairPath")] + [TestCase(true, "2.9.0", null, true, "InstallOrUpdate")] + [TestCase(true, "3.1.0", 1, true, "RepairPath")] + [TestCase(true, "3.1.0", 2, true, "InstallOrUpdate")] + [TestCase(false, "3.0.0", 1, true, "Uninstall")] + [TestCase(false, "2.9.0", null, true, "InstallOrUpdate")] + [TestCase(false, "3.1.0", 1, true, "Uninstall")] + [TestCase(false, "3.1.0", 2, true, "InstallOrUpdate")] + [TestCase(false, null, null, true, "InstallOrUpdate")] public void ResolveCliPrimaryButtonAction_ReturnsClickedPrimaryAction( bool needsCliPathSetup, string cliVersion, - string requiredCliVersion, + int? cliProtocolVersion, bool canUninstallCli, string expected) { - // Verifies that the Settings window preserves the primary action chosen before refresh. + // Verifies that the Settings window chooses repair only when protocol replacement is unnecessary. UnityCliLoopSettingsWindow.CliPrimaryButtonAction result = UnityCliLoopSettingsWindow.ResolveCliPrimaryButtonAction( needsCliPathSetup, cliVersion, - requiredCliVersion, + cliProtocolVersion, canUninstallCli); Assert.That(result.ToString(), Is.EqualTo(expected)); @@ -107,16 +115,32 @@ public void ShouldCheckCliPathSetupForPlatform_RequiresPackageOwnedCli( Assert.That(result, Is.EqualTo(expected)); } - [TestCase("3.0.0-beta.0", "3.0.0-beta.1", true)] - [TestCase("3.0.0-beta.1", "3.0.0-beta.1", false)] - [TestCase("3.0.0", "3.0.0-beta.1", false)] - public void IsCliUpdateNeeded_UsesMinimumRequiredCliVersion( + [TestCase(null, null, false)] + [TestCase("3.0.0-beta.32", 1, false)] + [TestCase("3.0.0-beta.0", null, true)] + [TestCase("3.0.0-beta.0", 0, true)] + [TestCase("3.0.0-beta.0", 2, false)] + public void IsCliUpdateNeeded_UsesProtocolVersion( string cliVersion, - string requiredCliVersion, + int? cliProtocolVersion, bool expected) { - // Verifies that the settings UI ignores package version drift and only updates old CLIs. - bool result = UnityCliLoopSettingsWindow.IsCliUpdateNeeded(cliVersion, requiredCliVersion); + // Verifies that the settings UI updates missing or older CLI protocol generations. + bool result = UnityCliLoopSettingsWindow.IsCliUpdateNeeded(cliVersion, cliProtocolVersion); + + Assert.That(result, Is.EqualTo(expected)); + } + + [TestCase(null, false)] + [TestCase(0, false)] + [TestCase(1, false)] + [TestCase(2, true)] + public void IsCliDowngradeNeeded_UsesProtocolVersion( + int? cliProtocolVersion, + bool expected) + { + // Verifies that the settings UI replaces CLIs newer than the package protocol generation. + bool result = UnityCliLoopSettingsWindow.IsCliDowngradeNeeded(cliProtocolVersion); Assert.That(result, Is.EqualTo(expected)); } diff --git a/Packages/src/Editor/Application/CliSetupApplicationService.cs b/Packages/src/Editor/Application/CliSetupApplicationService.cs index d141eea0e4..19bdae15bb 100644 --- a/Packages/src/Editor/Application/CliSetupApplicationService.cs +++ b/Packages/src/Editor/Application/CliSetupApplicationService.cs @@ -33,6 +33,7 @@ public interface ICliInstallationDetector { bool IsCliInstalled(); string GetCachedCliVersion(); + int? GetCachedCliProtocolVersion(); string GetCachedCliExecutablePath(); bool IsCheckCompleted(); Task RefreshCliVersionAsync(CancellationToken ct); @@ -92,6 +93,11 @@ public string GetCachedCliVersion() return _cliInstallationDetector.GetCachedCliVersion(); } + public int? GetCachedCliProtocolVersion() + { + return _cliInstallationDetector.GetCachedCliProtocolVersion(); + } + public string GetCachedCliExecutablePath() { return _cliInstallationDetector.GetCachedCliExecutablePath(); @@ -144,11 +150,21 @@ public bool IsCliVersionLessThan(string leftVersion, string rightVersion) return CliVersionComparer.IsVersionLessThan(leftVersion, rightVersion); } + public bool IsCliVersionGreaterThan(string leftVersion, string rightVersion) + { + return CliVersionComparer.IsVersionGreaterThan(leftVersion, rightVersion); + } + public bool IsCliVersionGreaterThanOrEqual(string leftVersion, string rightVersion) { return CliVersionComparer.IsVersionGreaterThanOrEqual(leftVersion, rightVersion); } + public bool IsCliVersionEqual(string leftVersion, string rightVersion) + { + return CliVersionComparer.IsVersionEqual(leftVersion, rightVersion); + } + public async Task InstallGlobalCliAsync(RuntimePlatform platform, CancellationToken ct) { ct.ThrowIfCancellationRequested(); @@ -269,6 +285,11 @@ public static string GetCachedCliVersion() return GetService().GetCachedCliVersion(); } + public static int? GetCachedCliProtocolVersion() + { + return GetService().GetCachedCliProtocolVersion(); + } + public static string GetCachedCliExecutablePath() { return GetService().GetCachedCliExecutablePath(); @@ -321,11 +342,21 @@ public static bool IsCliVersionLessThan(string leftVersion, string rightVersion) return GetService().IsCliVersionLessThan(leftVersion, rightVersion); } + public static bool IsCliVersionGreaterThan(string leftVersion, string rightVersion) + { + return GetService().IsCliVersionGreaterThan(leftVersion, rightVersion); + } + public static bool IsCliVersionGreaterThanOrEqual(string leftVersion, string rightVersion) { return GetService().IsCliVersionGreaterThanOrEqual(leftVersion, rightVersion); } + public static bool IsCliVersionEqual(string leftVersion, string rightVersion) + { + return GetService().IsCliVersionEqual(leftVersion, rightVersion); + } + public static Task InstallGlobalCliAsync(RuntimePlatform platform, CancellationToken ct) { return GetService().InstallGlobalCliAsync(platform, ct); diff --git a/Packages/src/Editor/Domain/CliConstants.cs b/Packages/src/Editor/Domain/CliConstants.cs index c25af9e129..3cbe2dea65 100644 --- a/Packages/src/Editor/Domain/CliConstants.cs +++ b/Packages/src/Editor/Domain/CliConstants.cs @@ -8,7 +8,7 @@ public static class CliConstants public const string EXECUTABLE_NAME = "uloop"; // Why: the runtime IPC gate compares this contract generation, not release numbers. // Bump it together with cli/contract.json protocolVersion only when this package can - // no longer interoperate with older CLI binaries. + // no longer interoperate with a different CLI protocol generation. public const int REQUIRED_CLI_PROTOCOL_VERSION = 1; // Why: setup installs this pinned release; it must always point at a published CLI // release, so it advances after releases, never inside feature PRs. @@ -16,6 +16,7 @@ public static class CliConstants public const string MINIMUM_REQUIRED_CLI_RELEASE_TAG = CLI_RELEASE_TAG_PREFIX + MINIMUM_REQUIRED_CLI_VERSION; public const string VERSION_FLAG = "--version"; public const string SHORT_VERSION_FLAG = "-v"; + public const string JSON_FLAG = "--json"; public const string RAW_CONTENT_BASE_URL = "https://raw.githubusercontent.com/hatayama/unity-cli-loop"; public const string SCRIPTS_DIR_NAME = "scripts"; public const string POSIX_INSTALL_SCRIPT_NAME = "install.sh"; diff --git a/Packages/src/Editor/Domain/CliVersionComparer.cs b/Packages/src/Editor/Domain/CliVersionComparer.cs index 1931e17bce..a67c5301b1 100644 --- a/Packages/src/Editor/Domain/CliVersionComparer.cs +++ b/Packages/src/Editor/Domain/CliVersionComparer.cs @@ -13,11 +13,21 @@ public static bool IsVersionLessThan(string leftVersion, string rightVersion) return TryCompareCliVersions(leftVersion, rightVersion, out int comparison) && comparison < 0; } + public static bool IsVersionGreaterThan(string leftVersion, string rightVersion) + { + return TryCompareCliVersions(leftVersion, rightVersion, out int comparison) && comparison > 0; + } + public static bool IsVersionGreaterThanOrEqual(string leftVersion, string rightVersion) { return TryCompareCliVersions(leftVersion, rightVersion, out int comparison) && comparison >= 0; } + public static bool IsVersionEqual(string leftVersion, string rightVersion) + { + return TryCompareCliVersions(leftVersion, rightVersion, out int comparison) && comparison == 0; + } + internal static bool TryCompareCliVersions( string leftVersion, string rightVersion, diff --git a/Packages/src/Editor/Infrastructure/Api/JsonRpcProcessor.cs b/Packages/src/Editor/Infrastructure/Api/JsonRpcProcessor.cs index c42c0315b6..3bdbfa8ec8 100644 --- a/Packages/src/Editor/Infrastructure/Api/JsonRpcProcessor.cs +++ b/Packages/src/Editor/Infrastructure/Api/JsonRpcProcessor.cs @@ -198,9 +198,9 @@ private static async Task ProcessRpcRequest( try { ct.ThrowIfCancellationRequested(); - if (IsCliUpdateRequired(request.ClientProtocolVersion)) + if (IsCliProtocolMismatch(request.ClientProtocolVersion)) { - return CreateCliUpdateRequiredResponse( + return CreateCliProtocolMismatchResponse( request.Id, request.ClientCliVersion, request.ClientProtocolVersion); @@ -299,17 +299,17 @@ private static bool CompileRequestWaitsForDomainReload(JToken paramsToken) return waitForDomainReloadToken.Value(); } - private static bool IsCliUpdateRequired(int? currentProtocolVersion) + private static bool IsCliProtocolMismatch(int? currentProtocolVersion) { if (currentProtocolVersion == null) { return true; } - return currentProtocolVersion.Value < CliConstants.REQUIRED_CLI_PROTOCOL_VERSION; + return currentProtocolVersion.Value != CliConstants.REQUIRED_CLI_PROTOCOL_VERSION; } - private static string CreateCliUpdateRequiredResponse( + private static string CreateCliProtocolMismatchResponse( object id, string currentCliVersion, int? currentProtocolVersion) @@ -319,16 +319,36 @@ private static string CreateCliUpdateRequiredResponse( id, new JsonRpcError( UnityCliLoopServerConfig.INTERNAL_ERROR_CODE, - "The installed uloop CLI is too old for this Unity package.", + "The installed uloop CLI uses an IPC protocol that does not match this Unity package.", new CliUpdateRequiredErrorData( currentCliVersion, currentProtocolVersion, CliConstants.REQUIRED_CLI_PROTOCOL_VERSION, - $"{CliConstants.EXECUTABLE_NAME} update"))); + GetCliUpdateCommandForProtocolMismatch(currentProtocolVersion)))); return JsonConvert.SerializeObject(errorResponse, Formatting.None, ResponseSerializerSettings); } + private static string GetCliUpdateCommandForProtocolMismatch(int? currentProtocolVersion) + { + if (currentProtocolVersion == null) + { + return CreateCliUpdateCommand(); + } + + if (currentProtocolVersion.Value < CliConstants.REQUIRED_CLI_PROTOCOL_VERSION) + { + return CreateCliUpdateCommand(); + } + + return null; + } + + private static string CreateCliUpdateCommand() + { + return $"{CliConstants.EXECUTABLE_NAME} update"; + } + internal static string CreateDispatchAcceptedResponse(object id, int heartbeatIntervalSeconds) { // heartbeatIntervalSeconds is only advertised when the client negotiated @@ -584,7 +604,7 @@ public ServerBusyErrorData( } /// - /// Carries CLI update instructions plus the protocol generations that failed the gate. + /// Carries protocol mismatch details plus optional CLI update instructions. /// public class CliUpdateRequiredErrorData : JsonRpcErrorData { @@ -596,6 +616,7 @@ public class CliUpdateRequiredErrorData : JsonRpcErrorData public int requiredProtocolVersion { get; } + [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] public string updateCommand { get; } public bool retryableAfterUpdate { get; } @@ -604,7 +625,7 @@ public CliUpdateRequiredErrorData( string currentCliVersion, int? currentProtocolVersion, int requiredProtocolVersion, - string updateCommand) : base("Update the uloop CLI and retry the original command.") + string updateCommand) : base("Install matching uloop CLI and Unity package versions, then retry the original command.") { this.currentCliVersion = string.IsNullOrWhiteSpace(currentCliVersion) ? null : currentCliVersion; this.currentProtocolVersion = currentProtocolVersion; diff --git a/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs b/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs index bfe6cc9b44..e149abbc04 100644 --- a/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs +++ b/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs @@ -4,6 +4,8 @@ using System.Text; using System.Threading; using System.Threading.Tasks; +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; using UnityEngine; using io.github.hatayama.UnityCliLoop.Application; @@ -14,13 +16,15 @@ namespace io.github.hatayama.UnityCliLoop.Infrastructure { internal readonly struct CliInstallationDetection { - public CliInstallationDetection(string version, string executablePath) + public CliInstallationDetection(string version, int? protocolVersion, string executablePath) { Version = version; + ProtocolVersion = protocolVersion; ExecutablePath = executablePath; } public string Version { get; } + public int? ProtocolVersion { get; } public string ExecutablePath { get; } } @@ -36,9 +40,14 @@ public sealed class CliInstallationDetector : ICliInstallationDetector private const string SHELL_VERSION_END_MARKER = "__ULOOP_VERSION_END__"; private const string SHELL_VERSION_STATUS_START_MARKER = "__ULOOP_VERSION_STATUS_START__"; private const string SHELL_VERSION_STATUS_END_MARKER = "__ULOOP_VERSION_STATUS_END__"; + private const string SHELL_CONTRACT_START_MARKER = "__ULOOP_CONTRACT_START__"; + private const string SHELL_CONTRACT_END_MARKER = "__ULOOP_CONTRACT_END__"; + private const string SHELL_CONTRACT_STATUS_START_MARKER = "__ULOOP_CONTRACT_STATUS_START__"; + private const string SHELL_CONTRACT_STATUS_END_MARKER = "__ULOOP_CONTRACT_STATUS_END__"; private const string SHELL_SUCCESS_EXIT_CODE = "0"; private string _cachedCliVersion; + private int? _cachedCliProtocolVersion; private string _cachedCliExecutablePath; private bool _cacheInitialized; private bool _isRefreshing; @@ -53,6 +62,11 @@ public string GetCachedCliVersion() return _cacheInitialized ? _cachedCliVersion : null; } + public int? GetCachedCliProtocolVersion() + { + return _cacheInitialized ? _cachedCliProtocolVersion : null; + } + public string GetCachedCliExecutablePath() { return _cacheInitialized ? _cachedCliExecutablePath : null; @@ -75,6 +89,7 @@ public async Task RefreshCliVersionAsync(CancellationToken ct) { CliInstallationDetection detection = await DetectCliInstallationAsync(ct); _cachedCliVersion = detection.Version; + _cachedCliProtocolVersion = detection.ProtocolVersion; _cachedCliExecutablePath = detection.ExecutablePath; _cacheInitialized = true; } @@ -88,6 +103,7 @@ public async Task ForceRefreshCliVersionAsync(CancellationToken ct) { CliInstallationDetection detection = await DetectCliInstallationAsync(ct); _cachedCliVersion = detection.Version; + _cachedCliProtocolVersion = detection.ProtocolVersion; _cachedCliExecutablePath = detection.ExecutablePath; _cacheInitialized = true; } @@ -114,6 +130,7 @@ public Task IsCliVisibleFromShellAsync(RuntimePlatform platform, Cancellat public void InvalidateCache() { _cachedCliVersion = null; + _cachedCliProtocolVersion = null; _cachedCliExecutablePath = null; _cacheInitialized = false; _isRefreshing = false; @@ -154,7 +171,7 @@ private static CliInstallationDetection DetectPackageOwnedCliInstallationBlockin string executablePath = NativeCliInstaller.GetCurrentUserGlobalCliInstallPath(platform); if (string.IsNullOrEmpty(executablePath) || !File.Exists(executablePath)) { - return new CliInstallationDetection(null, executablePath); + return new CliInstallationDetection(null, null, executablePath); } return DetectCliInstallationAtExecutablePath(executablePath, ct); @@ -257,6 +274,13 @@ internal static string BuildShellCliDetectionCommand(string executableName) return "echo " + SHELL_PATH_START_MARKER + "\n" + "command -v " + executableName + "\n" + "echo " + SHELL_PATH_END_MARKER + "\n" + + "echo " + SHELL_CONTRACT_START_MARKER + "\n" + + executableName + " " + CliConstants.VERSION_FLAG + " " + CliConstants.JSON_FLAG + "\n" + + "uloop_contract_status=$?\n" + + "echo " + SHELL_CONTRACT_END_MARKER + "\n" + + "echo " + SHELL_CONTRACT_STATUS_START_MARKER + "\n" + + "echo $uloop_contract_status\n" + + "echo " + SHELL_CONTRACT_STATUS_END_MARKER + "\n" + "echo " + SHELL_VERSION_START_MARKER + "\n" + executableName + " " + CliConstants.SHORT_VERSION_FLAG + "\n" + "uloop_version_status=$?\n" @@ -273,6 +297,13 @@ private static string BuildFishShellCliDetectionCommand(string executableName) return "echo " + SHELL_PATH_START_MARKER + "\n" + "command -v " + executableName + "\n" + "echo " + SHELL_PATH_END_MARKER + "\n" + + "echo " + SHELL_CONTRACT_START_MARKER + "\n" + + executableName + " " + CliConstants.VERSION_FLAG + " " + CliConstants.JSON_FLAG + "\n" + + "set uloop_contract_status $status\n" + + "echo " + SHELL_CONTRACT_END_MARKER + "\n" + + "echo " + SHELL_CONTRACT_STATUS_START_MARKER + "\n" + + "echo $uloop_contract_status\n" + + "echo " + SHELL_CONTRACT_STATUS_END_MARKER + "\n" + "echo " + SHELL_VERSION_START_MARKER + "\n" + executableName + " " + CliConstants.SHORT_VERSION_FLAG + "\n" + "set uloop_version_status $status\n" @@ -296,11 +327,52 @@ internal static CliInstallationDetection ParseShellCliInstallationOutput(string output, SHELL_VERSION_STATUS_START_MARKER, SHELL_VERSION_STATUS_END_MARKER); + string contractBlock = NodeEnvironmentResolver.ExtractBetweenMarkers( + output, + SHELL_CONTRACT_START_MARKER, + SHELL_CONTRACT_END_MARKER); + string contractStatusBlock = NodeEnvironmentResolver.ExtractBetweenMarkers( + output, + SHELL_CONTRACT_STATUS_START_MARKER, + SHELL_CONTRACT_STATUS_END_MARKER); string executablePath = NodeEnvironmentResolver.ExtractAbsolutePathLine(pathBlock); + if (IsSuccessfulShellStatus(contractStatusBlock)) + { + CliInstallationDetection contractDetection = ParseCliContractOutput(contractBlock, executablePath); + if (!string.IsNullOrEmpty(contractDetection.Version)) + { + return contractDetection; + } + } + string version = IsSuccessfulShellStatus(versionStatusBlock) ? ExtractFirstNonEmptyLine(versionBlock) : null; - return new CliInstallationDetection(version, executablePath); + return new CliInstallationDetection(version, null, executablePath); + } + + private static CliInstallationDetection ParseCliContractOutput(string output, string executablePath) + { + string jsonLine = ExtractFirstNonEmptyLine(output); + if (string.IsNullOrEmpty(jsonLine)) + { + return new CliInstallationDetection(null, null, executablePath); + } + + try + { + JObject parsed = JObject.Parse(jsonLine); + string version = parsed["cliVersion"]?.ToString(); + JToken protocolVersionToken = parsed["protocolVersion"]; + int? protocolVersion = protocolVersionToken != null && protocolVersionToken.Type == JTokenType.Integer + ? protocolVersionToken.Value() + : null; + return new CliInstallationDetection(version, protocolVersion, executablePath); + } + catch (JsonException) + { + return new CliInstallationDetection(null, null, executablePath); + } } private static string ExecuteAndGetOutput(ProcessStartInfo startInfo, CancellationToken ct) @@ -395,10 +467,27 @@ private static CliInstallationDetection DetectCliInstallationAtExecutablePath( CancellationToken ct) { string fileName = executablePath ?? CliConstants.EXECUTABLE_NAME; + string contractOutput = ExecuteCliVersionCommand(fileName, CliConstants.VERSION_FLAG + " " + CliConstants.JSON_FLAG, ct); + CliInstallationDetection contractDetection = ParseCliContractOutput(contractOutput, executablePath); + if (!string.IsNullOrEmpty(contractDetection.Version)) + { + return contractDetection; + } + + string versionOutput = ExecuteCliVersionCommand(fileName, CliConstants.VERSION_FLAG, ct); + string version = string.IsNullOrEmpty(versionOutput) ? null : versionOutput; + return new CliInstallationDetection(version, null, executablePath); + } - ProcessStartInfo startInfo = new() { + private static string ExecuteCliVersionCommand( + string fileName, + string arguments, + CancellationToken ct) + { + ProcessStartInfo startInfo = new() + { FileName = fileName, - Arguments = CliConstants.VERSION_FLAG, + Arguments = arguments, UseShellExecute = false, RedirectStandardOutput = true, RedirectStandardError = true, @@ -408,7 +497,7 @@ private static CliInstallationDetection DetectCliInstallationAtExecutablePath( Process process = ProcessStartHelper.TryStart(startInfo); if (process == null) { - return new CliInstallationDetection(null, executablePath); + return null; } StringBuilder outputBuilder = new(); @@ -438,7 +527,7 @@ private static CliInstallationDetection DetectCliInstallationAtExecutablePath( { KillProcessIfRunning(process); process.Dispose(); - return new CliInstallationDetection(null, executablePath); + return null; } // Parameterless WaitForExit flushes async output buffers @@ -448,8 +537,7 @@ private static CliInstallationDetection DetectCliInstallationAtExecutablePath( bool failed = process.ExitCode != 0 || string.IsNullOrEmpty(output); process.Dispose(); - string version = failed ? null : output; - return new CliInstallationDetection(version, executablePath); + return failed ? null : output; } catch (Exception ex) { @@ -458,7 +546,7 @@ private static CliInstallationDetection DetectCliInstallationAtExecutablePath( { UnityEngine.Debug.LogWarning($"[UnityCliLoop] Failed to detect CLI version: {ex.Message}"); } - return new CliInstallationDetection(null, executablePath); + return null; } } } diff --git a/Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs b/Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs index 533c91adb7..477f26876c 100644 --- a/Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs +++ b/Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs @@ -195,13 +195,13 @@ private static async Task NeedsCliUpdateForSetupWizardAsync(CancellationTo { await CliSetupApplicationFacade.ForceRefreshCliVersionAsync(ct); string cliVersion = CliSetupApplicationFacade.GetCachedCliVersion(); + int? cliProtocolVersion = CliSetupApplicationFacade.GetCachedCliProtocolVersion(); if (string.IsNullOrEmpty(cliVersion)) { return false; } - string requiredCliVersion = GetMinimumRequiredCliVersion(); - return !IsCliVersionSatisfied(cliVersion, requiredCliVersion); + return !IsCliCompatibleForSetupWizard(cliVersion, cliProtocolVersion); } private static async Task HasSkillUpdateForSetupWizardAsync(CancellationToken ct) @@ -636,13 +636,14 @@ private async void RefreshUI(bool refreshSkillsSection = true) await CliSetupApplicationFacade.ForceRefreshCliVersionAsync(CancellationToken.None); string cliVersion = CliSetupApplicationFacade.GetCachedCliVersion(); + int? cliProtocolVersion = CliSetupApplicationFacade.GetCachedCliProtocolVersion(); string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); string requiredCliVersion = GetMinimumRequiredCliVersion(); bool cliInstalled = IsCliInstalled(cliVersion); - bool cliVersionMatched = IsCliVersionSatisfied(cliVersion, requiredCliVersion) && cliInstalled; + bool cliVersionMatched = IsCliCompatibleForSetupWizard(cliVersion, cliProtocolVersion) && cliInstalled; _needsCliPathSetup = await ShouldRepairCliPathSetupAsync(CancellationToken.None); - UpdateCliStep(cliInstalled, cliVersion, requiredCliVersion, cliVersionMatched); + UpdateCliStep(cliInstalled, cliVersion, cliProtocolVersion, requiredCliVersion, cliVersionMatched); if (!refreshSkillsSection) { @@ -800,15 +801,18 @@ internal static List GetFirstInstallableSkillTargets( private void UpdateCliStep( bool cliInstalled, string cliVersion, + int? cliProtocolVersion, string requiredCliVersion, bool cliVersionMatched) { - bool needsUpdate = cliInstalled && !cliVersionMatched; + bool needsUpdate = IsCliUpdateNeededForSetupWizard(cliVersion, cliProtocolVersion); + bool needsDowngrade = IsCliDowngradeNeededForSetupWizard(cliProtocolVersion); string buttonText = GetCliButtonTextForSetupWizard( cliInstalled, _isInstallingCli, false, needsUpdate, + needsDowngrade, _needsCliPathSetup, cliVersion, requiredCliVersion); @@ -849,6 +853,7 @@ internal static string GetCliButtonTextForSetupWizard( bool isInstallingCli, bool isChecking, bool needsUpdate, + bool needsDowngrade, bool needsCliPathSetup, string cliVersion, string requiredCliVersion) @@ -860,7 +865,7 @@ internal static string GetCliButtonTextForSetupWizard( if (isInstallingCli) { - if (needsCliPathSetup && !needsUpdate) + if (needsCliPathSetup && !needsUpdate && !needsDowngrade) { return "Fixing PATH..."; } @@ -873,6 +878,11 @@ internal static string GetCliButtonTextForSetupWizard( return $"Update CLI (v{cliVersion} \u2192 v{requiredCliVersion})"; } + if (needsDowngrade) + { + return $"Downgrade CLI (v{cliVersion} \u2192 v{requiredCliVersion})"; + } + if (needsCliPathSetup) { return "Fix PATH"; @@ -921,14 +931,40 @@ internal static bool ShouldCheckCliPathSetupForSetupWizard( && hasPackageOwnedCurrentUserInstall; } - private static bool IsCliVersionSatisfied(string cliVersion, string requiredCliVersion) + private static bool IsCliCompatibleForSetupWizard(string cliVersion, int? cliProtocolVersion) { if (string.IsNullOrEmpty(cliVersion)) { return false; } - return CliSetupApplicationFacade.IsCliVersionGreaterThanOrEqual(cliVersion, requiredCliVersion); + if (cliProtocolVersion == null) + { + return false; + } + + return cliProtocolVersion.Value == CliConstants.REQUIRED_CLI_PROTOCOL_VERSION; + } + + private static bool IsCliUpdateNeededForSetupWizard(string cliVersion, int? cliProtocolVersion) + { + if (string.IsNullOrEmpty(cliVersion)) + { + return false; + } + + if (cliProtocolVersion == null) + { + return true; + } + + return cliProtocolVersion.Value < CliConstants.REQUIRED_CLI_PROTOCOL_VERSION; + } + + private static bool IsCliDowngradeNeededForSetupWizard(int? cliProtocolVersion) + { + return cliProtocolVersion != null + && cliProtocolVersion.Value > CliConstants.REQUIRED_CLI_PROTOCOL_VERSION; } private static string GetMinimumRequiredCliVersion() @@ -1136,11 +1172,13 @@ private async void HandleInstallCli() await RefreshCliPrimaryActionStateAsync(CancellationToken.None); string cliVersion = CliSetupApplicationFacade.GetCachedCliVersion(); + int? cliProtocolVersion = CliSetupApplicationFacade.GetCachedCliProtocolVersion(); string requiredCliVersion = GetMinimumRequiredCliVersion(); bool cliInstalled = IsCliInstalled(cliVersion); - bool cliVersionMatched = IsCliVersionSatisfied(cliVersion, requiredCliVersion) && cliInstalled; - bool needsUpdate = cliInstalled && !cliVersionMatched; - if (ShouldRepairCliPathFromPrimaryButton(_needsCliPathSetup, needsUpdate)) + bool cliVersionMatched = IsCliCompatibleForSetupWizard(cliVersion, cliProtocolVersion) && cliInstalled; + bool needsUpdate = IsCliUpdateNeededForSetupWizard(cliVersion, cliProtocolVersion); + bool needsDowngrade = IsCliDowngradeNeededForSetupWizard(cliProtocolVersion); + if (ShouldRepairCliPathFromPrimaryButton(_needsCliPathSetup, needsUpdate, needsDowngrade)) { await HandleRepairCliPathSetup(); return; @@ -1149,7 +1187,7 @@ private async void HandleInstallCli() bool wasCliInstalledBeforeInstall = CliSetupApplicationFacade.IsCliInstalled(); _needsCliPathSetup = false; _isInstallingCli = true; - UpdateCliStep(false, null, GetMinimumRequiredCliVersion(), false); + UpdateCliStep(false, null, null, GetMinimumRequiredCliVersion(), false); try { @@ -1202,17 +1240,19 @@ private async Task RefreshCliPrimaryActionStateAsync(CancellationToken ct) private void RefreshCliStepFromCachedState() { string cliVersion = CliSetupApplicationFacade.GetCachedCliVersion(); + int? cliProtocolVersion = CliSetupApplicationFacade.GetCachedCliProtocolVersion(); string requiredCliVersion = GetMinimumRequiredCliVersion(); bool cliInstalled = IsCliInstalled(cliVersion); - bool cliVersionMatched = IsCliVersionSatisfied(cliVersion, requiredCliVersion) && cliInstalled; - UpdateCliStep(cliInstalled, cliVersion, requiredCliVersion, cliVersionMatched); + bool cliVersionMatched = IsCliCompatibleForSetupWizard(cliVersion, cliProtocolVersion) && cliInstalled; + UpdateCliStep(cliInstalled, cliVersion, cliProtocolVersion, requiredCliVersion, cliVersionMatched); } internal static bool ShouldRepairCliPathFromPrimaryButton( bool needsCliPathSetup, - bool needsUpdate) + bool needsUpdate, + bool needsDowngrade) { - return needsCliPathSetup && !needsUpdate; + return needsCliPathSetup && !needsUpdate && !needsDowngrade; } private async Task HandleRepairCliPathSetup() @@ -1221,6 +1261,7 @@ private async Task HandleRepairCliPathSetup() UpdateCliStep( cliInstalled: true, cliVersion: CliSetupApplicationFacade.GetCachedCliVersion(), + cliProtocolVersion: CliSetupApplicationFacade.GetCachedCliProtocolVersion(), requiredCliVersion: GetMinimumRequiredCliVersion(), cliVersionMatched: true); diff --git a/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs b/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs index 67d97d8a7e..d7a1f1ce9c 100644 --- a/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs +++ b/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs @@ -617,6 +617,7 @@ private void RefreshCliSetupSection(bool includeSkillDirectoryChecks = true) private CliSetupData CreateCliSetupData(bool includeSkillDirectoryChecks = true) { string cliVersion = CliSetupApplicationFacade.GetCachedCliVersion(); + int? cliProtocolVersion = CliSetupApplicationFacade.GetCachedCliProtocolVersion(); string cliExecutablePath = CliSetupApplicationFacade.GetCachedCliExecutablePath(); string requiredCliVersion = GetMinimumRequiredCliVersion(); @@ -628,8 +629,8 @@ private CliSetupData CreateCliSetupData(bool includeSkillDirectoryChecks = true) || _isRefreshingVersion || _isRefreshingCliPathSetup || !includeSkillDirectoryChecks; - bool needsUpdate = IsCliUpdateNeeded(cliVersion, requiredCliVersion); - bool needsDowngrade = false; + bool needsUpdate = IsCliUpdateNeeded(cliVersion, cliProtocolVersion); + bool needsDowngrade = IsCliDowngradeNeeded(cliProtocolVersion); bool groupSkillsUnderUnityCliLoop = !_installSkillsFlat; SkillInstallState selectedTargetInstallState = includeSkillDirectoryChecks ? _selectedTargetInstallState @@ -832,6 +833,7 @@ await CliPathSetupPrompt.EnsureVisibleAndShowResultAsync( private CliPrimaryButtonAction GetCurrentCliPrimaryButtonAction() { string cliVersion = CliSetupApplicationFacade.GetCachedCliVersion(); + int? cliProtocolVersion = CliSetupApplicationFacade.GetCachedCliProtocolVersion(); string cliExecutablePath = CliSetupApplicationFacade.GetCachedCliExecutablePath(); bool canUninstallCli = CliSetupApplicationFacade.IsPackageOwnedCurrentUserInstallPath( cliExecutablePath, @@ -839,48 +841,52 @@ private CliPrimaryButtonAction GetCurrentCliPrimaryButtonAction() return ResolveCliPrimaryButtonAction( _needsCliPathSetup, cliVersion, - GetMinimumRequiredCliVersion(), + cliProtocolVersion, canUninstallCli); } private bool ShouldUninstallCliFromPrimaryButton() { string cliVersion = CliSetupApplicationFacade.GetCachedCliVersion(); + int? cliProtocolVersion = CliSetupApplicationFacade.GetCachedCliProtocolVersion(); string cliExecutablePath = CliSetupApplicationFacade.GetCachedCliExecutablePath(); bool canUninstallCli = CliSetupApplicationFacade.IsPackageOwnedCurrentUserInstallPath( cliExecutablePath, UnityEngine.Application.platform); return ShouldUninstallCliFromPrimaryButton( cliVersion, - GetMinimumRequiredCliVersion(), + cliProtocolVersion, canUninstallCli); } internal static bool ShouldUninstallCliFromPrimaryButton( string cliVersion, - string requiredCliVersion, + int? cliProtocolVersion, bool canUninstallCli) { bool isCliInstalled = cliVersion != null; - bool needsUpdate = IsCliUpdateNeeded(cliVersion, requiredCliVersion); - return CliSetupSection.IsUninstallCliAction(isCliInstalled, needsUpdate, needsDowngrade: false, canUninstallCli); + bool needsUpdate = IsCliUpdateNeeded(cliVersion, cliProtocolVersion); + bool needsDowngrade = IsCliDowngradeNeeded(cliProtocolVersion); + return CliSetupSection.IsUninstallCliAction(isCliInstalled, needsUpdate, needsDowngrade, canUninstallCli); } internal static CliPrimaryButtonAction ResolveCliPrimaryButtonAction( bool needsCliPathSetup, string cliVersion, - string requiredCliVersion, + int? cliProtocolVersion, bool canUninstallCli) { - bool needsUpdate = IsCliUpdateNeeded(cliVersion, requiredCliVersion); + bool needsUpdate = IsCliUpdateNeeded(cliVersion, cliProtocolVersion); + bool needsDowngrade = IsCliDowngradeNeeded(cliProtocolVersion); if (ShouldRepairCliPathFromPrimaryButton( needsCliPathSetup, - needsUpdate)) + needsUpdate, + needsDowngrade)) { return CliPrimaryButtonAction.RepairPath; } - if (ShouldUninstallCliFromPrimaryButton(cliVersion, requiredCliVersion, canUninstallCli)) + if (ShouldUninstallCliFromPrimaryButton(cliVersion, cliProtocolVersion, canUninstallCli)) { return CliPrimaryButtonAction.Uninstall; } @@ -908,9 +914,10 @@ internal static CliPrimaryButtonAction ResolveExecutableCliPrimaryButtonAction( internal static bool ShouldRepairCliPathFromPrimaryButton( bool needsCliPathSetup, - bool needsUpdate) + bool needsUpdate, + bool needsDowngrade) { - return needsCliPathSetup && !needsUpdate; + return needsCliPathSetup && !needsUpdate && !needsDowngrade; } private async Task RefreshCliPrimaryActionStateAsync(CancellationToken ct) @@ -963,10 +970,25 @@ await CliPathSetupPrompt.EnsureVisibleAndShowResultAsync( } } - internal static bool IsCliUpdateNeeded(string cliVersion, string requiredCliVersion) + internal static bool IsCliUpdateNeeded(string cliVersion, int? cliProtocolVersion) { - return cliVersion != null - && CliSetupApplicationFacade.IsCliVersionLessThan(cliVersion, requiredCliVersion); + if (string.IsNullOrEmpty(cliVersion)) + { + return false; + } + + if (cliProtocolVersion == null) + { + return true; + } + + return cliProtocolVersion.Value < CliConstants.REQUIRED_CLI_PROTOCOL_VERSION; + } + + internal static bool IsCliDowngradeNeeded(int? cliProtocolVersion) + { + return cliProtocolVersion != null + && cliProtocolVersion.Value > CliConstants.REQUIRED_CLI_PROTOCOL_VERSION; } private async Task HandleUninstallCli() diff --git a/cli/internal/cli/error_envelope.go b/cli/internal/cli/error_envelope.go index a41c8e4647..ebfc75acee 100644 --- a/cli/internal/cli/error_envelope.go +++ b/cli/internal/cli/error_envelope.go @@ -354,18 +354,29 @@ func cliUpdateRequiredError(rpcErr *unityipc.RPCError, details map[string]any, d func cliUpdateRequiredNextActions(data map[string]any) []string { updateCommand, _ := data["updateCommand"].(string) - targetCommand, _ := data["targetUpdateCommand"].(string) actions := []string{} if updateCommand != "" { actions = append(actions, "Run `"+updateCommand+"`.") + } else if cliProtocolMismatchIsNewer(data) { + actions = append(actions, "Update the Unity package to a version that supports this CLI protocol, or install the CLI from the same release as the package.") + } else { + actions = append(actions, "Install matching uloop CLI and Unity package versions.") } - if targetCommand != "" && targetCommand != updateCommand { - actions = append(actions, "If your CLI supports exact updates, run `"+targetCommand+"` instead.") - } - actions = append(actions, "Retry the original command after the update completes.") + actions = append(actions, "Retry the original command after the versions match.") return actions } +func cliProtocolMismatchIsNewer(data map[string]any) bool { + currentProtocolVersion, currentOk := protocolVersionFromRPCData(data, "currentProtocolVersion") + requiredProtocolVersion, requiredOk := protocolVersionFromRPCData(data, "requiredProtocolVersion") + return currentOk && requiredOk && currentProtocolVersion > requiredProtocolVersion +} + +func protocolVersionFromRPCData(data map[string]any, key string) (float64, bool) { + value, ok := data[key].(float64) + return value, ok +} + func disconnectedAfterAcceptError(err error, context errorContext) cliError { return cliError{ ErrorCode: errorCodeUnityDisconnectedAfterAccept, diff --git a/cli/internal/cli/error_envelope_test.go b/cli/internal/cli/error_envelope_test.go index 1a4580c11a..e40b238653 100644 --- a/cli/internal/cli/error_envelope_test.go +++ b/cli/internal/cli/error_envelope_test.go @@ -195,9 +195,9 @@ func TestClassifyCliUpdateRequiredRPCError(t *testing.T) { // Verifies Unity compatibility errors become self-repair guidance for AI clients. err := &unityipc.RPCError{ Code: -32603, - Message: "The installed uloop CLI is too old for this Unity package.", + Message: "The installed uloop CLI uses an IPC protocol that does not match this Unity package.", Data: json.RawMessage( - `{"type":"cli_update_required","currentCliVersion":"3.0.0-beta.5","requiredCliVersion":"3.0.0-beta.6","updateCommand":"uloop update","targetUpdateCommand":"uloop update --to-version 3.0.0-beta.6","retryableAfterUpdate":true}`), + `{"type":"cli_update_required","currentCliVersion":"3.0.0-beta.5","currentProtocolVersion":1,"requiredProtocolVersion":2,"updateCommand":"uloop update","retryableAfterUpdate":true}`), } cliErr := classifyError(err, errorContext{projectRoot: "/tmp/MyProject", command: "compile"}) @@ -212,6 +212,24 @@ func TestClassifyCliUpdateRequiredRPCError(t *testing.T) { } } +func TestClassifyCliUpdateRequiredRPCErrorForNewerProtocol(t *testing.T) { + // Verifies a future CLI protocol mismatch does not tell users to update the CLI again. + err := &unityipc.RPCError{ + Code: -32603, + Message: "The installed uloop CLI uses an IPC protocol that does not match this Unity package.", + Data: json.RawMessage( + `{"type":"cli_update_required","currentCliVersion":"3.0.0-beta.99","currentProtocolVersion":3,"requiredProtocolVersion":2,"retryableAfterUpdate":true}`), + } + + cliErr := classifyError(err, errorContext{projectRoot: "/tmp/MyProject", command: "compile"}) + if cliErr.ErrorCode != errorCodeCLIUpdateRequired { + t.Fatalf("error code mismatch: %#v", cliErr) + } + if len(cliErr.NextActions) == 0 || cliErr.NextActions[0] != "Update the Unity package to a version that supports this CLI protocol, or install the CLI from the same release as the package." { + t.Fatalf("next actions mismatch: %#v", cliErr.NextActions) + } +} + func TestClassifyServerBusyRPCError(t *testing.T) { // Verifies server_busy RPC failures keep retryable classification while using the lightweight message. err := &unityipc.RPCError{ diff --git a/cli/internal/cli/help_test.go b/cli/internal/cli/help_test.go index d4c6ec922d..f6f6d0bab0 100644 --- a/cli/internal/cli/help_test.go +++ b/cli/internal/cli/help_test.go @@ -3,6 +3,7 @@ package cli import ( "bytes" "context" + "encoding/json" "strings" "testing" ) @@ -46,6 +47,28 @@ func TestPrintLauncherHelpListsNativeCommandsAndLiveToolGuidance(t *testing.T) { } } +func TestRunProjectLocalVersionJSONIncludesProtocolVersion(t *testing.T) { + // Verifies Unity setup can inspect protocol compatibility without parsing human help text. + var stdout bytes.Buffer + var stderr bytes.Buffer + + code := RunProjectLocal(context.Background(), []string{"--version", "--json"}, &stdout, &stderr) + + if code != 0 { + t.Fatalf("version json command failed with code %d: %s", code, stderr.String()) + } + var payload map[string]any + if err := json.Unmarshal(stdout.Bytes(), &payload); err != nil { + t.Fatalf("version json output is not JSON: %v\n%s", err, stdout.String()) + } + if payload["cliVersion"] != version { + t.Fatalf("cliVersion mismatch: %#v", payload) + } + if payload["protocolVersion"] != float64(protocolVersion) { + t.Fatalf("protocolVersion mismatch: %#v", payload) + } +} + // Tests that project-local help lists native commands and points users to live tool discovery. func TestPrintProjectLocalHelpListsNativeCommandsAndLiveToolGuidance(t *testing.T) { var stdout bytes.Buffer diff --git a/cli/internal/cli/run.go b/cli/internal/cli/run.go index f1477dab99..3f4785d3b6 100644 --- a/cli/internal/cli/run.go +++ b/cli/internal/cli/run.go @@ -25,6 +25,10 @@ func RunProjectLocal(ctx context.Context, args []string, stdout io.Writer, stder printHelpForResolvedProject(stdout, projectPath) return 0 } + if isVersionJSONRequest(remainingArgs) { + writeVersionJSON(stdout) + return 0 + } if isVersionRequest(remainingArgs) { writeLine(stdout, version) return 0 @@ -147,6 +151,17 @@ func RunProjectLocal(ctx context.Context, args []string, stdout io.Writer, stder } } +func writeVersionJSON(stdout io.Writer) { + content, err := json.Marshal(map[string]any{ + "cliVersion": version, + "protocolVersion": protocolVersion, + }) + if err != nil { + panic(err) + } + writeLine(stdout, string(content)) +} + func isUnknownLeadingOption(command string) bool { return strings.HasPrefix(command, "-") } diff --git a/cli/internal/cli/run_help.go b/cli/internal/cli/run_help.go index 5e104cf76b..fc5c060b04 100644 --- a/cli/internal/cli/run_help.go +++ b/cli/internal/cli/run_help.go @@ -17,6 +17,10 @@ func isVersionRequest(args []string) bool { return len(args) == 1 && (args[0] == "--version" || args[0] == "-v") } +func isVersionJSONRequest(args []string) bool { + return len(args) == 2 && (args[0] == "--version" || args[0] == "-v") && args[1] == "--json" +} + func isHelpRequest(args []string) bool { return len(args) == 1 && (args[0] == "--help" || args[0] == "-h") } diff --git a/cli/internal/cli/tools.go b/cli/internal/cli/tools.go index 024ec03d5c..1703aa9dc2 100644 --- a/cli/internal/cli/tools.go +++ b/cli/internal/cli/tools.go @@ -10,7 +10,10 @@ import ( "github.com/hatayama/unity-cli-loop/cli/internal/tools" ) -var version = clicontract.Current.CliVersion +var ( + version = clicontract.Current.CliVersion + protocolVersion = clicontract.Current.ProtocolVersion +) const ( cacheDirectoryName = tools.CacheDirectoryName From f8c8e9d0a9f5b2b13062ff4788a875a7ed90f20e Mon Sep 17 00:00:00 2001 From: hatayama Date: Sat, 13 Jun 2026 13:38:15 +0900 Subject: [PATCH 6/9] Add protocol review guidance to CodeRabbit Ask CodeRabbit to flag IPC-facing changes for protocol-version review without treating ordinary CLI or release changes as automatic bump triggers. --- .coderabbit.yaml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.coderabbit.yaml b/.coderabbit.yaml index 1a579e3109..d5c3a59f01 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -32,5 +32,13 @@ reviews: - "**/*.tsx" - "**/*.js" - "**/*.jsx" + path_instructions: + - path: "**/*" + instructions: | + For changes that touch the Unity package/native CLI IPC contract, explicitly check whether the integer protocol generation must be bumped. Relevant IPC surfaces include JSON-RPC method names, request/response shapes, request metadata under `uloop`, readiness/dispatch/heartbeat handshakes, tool catalog schema, `cli/internal/unityipc/**`, `Packages/src/Editor/Infrastructure/Api/**`, `cli/contract.json`, and `CliConstants.REQUIRED_CLI_PROTOCOL_VERSION`. + + The compatibility gate is an exact protocol match, not a release-semver range. If a change makes a CLI/package from the previous protocol generation unable to interoperate, require both `cli/contract.json` `protocolVersion` and `CliConstants.REQUIRED_CLI_PROTOCOL_VERSION` to be incremented in the same PR. + + Do not request a protocol bump for ordinary CLI features, bug fixes, UI changes, documentation, or additive wire-format changes that older counterparts can safely ignore. `cliVersion`, `default-tools.json` `version`, and `CliConstants.MINIMUM_REQUIRED_CLI_VERSION` are release/install concerns and must not be hand-edited to keep up with feature PRs. chat: auto_reply: true From a434187131576f1e580f7ab7c0c6cd091b11eb9c Mon Sep 17 00:00:00 2001 From: hatayama Date: Sat, 13 Jun 2026 13:40:43 +0900 Subject: [PATCH 7/9] Clarify protocol compatibility guidance Tighten the AGENTS wording for protocol-version bump guidance so the contract-generation rule reads cleanly. --- AGENTS.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e911f92103..b9ca6b4a29 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -22,8 +22,8 @@ protocol version, not on release numbers. Two declarations must always stay equa The runtime gate expects equality because the protocol version is a contract generation, not a minimum-compatible range. -Bump both, together, in the same PR only when the IPC contract changes in a way that makes an -CLI and package from different protocol generations unable to interoperate — for example renaming +Bump both, together, in the same PR only when the IPC contract changes in a way that makes +CLI and package builds from different protocol generations unable to interoperate — for example renaming or removing a request field, changing the readiness/dispatch handshake, or altering a response shape the other side parses. Ordinary CLI features and bug fixes that keep the wire format compatible must not bump it. From ede0d8d2fbeaea0e63fa2fb906e689c9726a5343 Mon Sep 17 00:00:00 2001 From: hatayama Date: Sun, 14 Jun 2026 15:06:20 +0900 Subject: [PATCH 8/9] Add IPC protocol reminder checks Add contract-shape tests for protocol and heartbeat metadata, plus a non-blocking PR reminder that highlights IPC-facing diffs when protocol declarations were not changed. --- .github/workflows/build-and-test.yml | 5 + AGENTS.md | 3 + .../JsonRpcProcessorCliVersionGateTests.cs | 41 ++++ cli/cmd/check-ipc-protocol-reminder/main.go | 25 +++ .../automation/ipc_protocol_reminder.go | 208 ++++++++++++++++++ .../automation/ipc_protocol_reminder_test.go | 80 +++++++ cli/internal/unityipc/client_test.go | 3 + 7 files changed, 365 insertions(+) create mode 100644 cli/cmd/check-ipc-protocol-reminder/main.go create mode 100644 cli/internal/automation/ipc_protocol_reminder.go create mode 100644 cli/internal/automation/ipc_protocol_reminder_test.go diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index cf78404d3b..f555fc23ed 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -43,6 +43,11 @@ jobs: scripts/test-native-cli-publish-workflow.sh scripts/test-install-release-filter.sh + - name: Check IPC protocol reminder + if: github.event_name == 'pull_request' + working-directory: cli + run: go run ./cmd/check-ipc-protocol-reminder --base "origin/${{ github.base_ref }}" --head HEAD + - name: Install golangci-lint run: | go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.0 diff --git a/AGENTS.md b/AGENTS.md index b9ca6b4a29..8fda8569f7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -21,6 +21,9 @@ protocol version, not on release numbers. Two declarations must always stay equa `TestProtocolVersionMatchesUnityPackage` fails the build if they diverge, so never bump one alone. The runtime gate expects equality because the protocol version is a contract generation, not a minimum-compatible range. +Pull request CI also runs a non-blocking IPC protocol reminder when IPC-facing files changed +without protocol declaration changes; treat it as a review prompt, not as proof that a bump is +required. Bump both, together, in the same PR only when the IPC contract changes in a way that makes CLI and package builds from different protocol generations unable to interoperate — for example renaming diff --git a/Assets/Tests/Editor/JsonRpcProcessorCliVersionGateTests.cs b/Assets/Tests/Editor/JsonRpcProcessorCliVersionGateTests.cs index fa507991e3..596f176f39 100644 --- a/Assets/Tests/Editor/JsonRpcProcessorCliVersionGateTests.cs +++ b/Assets/Tests/Editor/JsonRpcProcessorCliVersionGateTests.cs @@ -75,6 +75,35 @@ public async Task ProcessRequest_WhenProtocolVersionIsTooOld_ReturnsCliUpdateReq Assert.That(data["retryableAfterUpdate"]?.ToObject(), Is.True); } + [Test] + public async Task ProcessRequest_WhenClientNegotiatesHeartbeat_WritesAcceptedHandshakeShape() + { + // Verifies the request metadata drives the accepted response and heartbeat frame contract. + string acceptedResponse = null; + Func createHeartbeatJson = null; + + string response = await JsonRpcProcessor.ProcessRequestWithEarlyResponseAsync( + BuildHeartbeatNegotiatedToolRequest(UnityCliLoopConstants.COMMAND_NAME_GET_VERSION, 1), + CancellationToken.None, + (earlyResponse, _, heartbeatFactory) => + { + acceptedResponse = earlyResponse; + createHeartbeatJson = heartbeatFactory; + return Task.CompletedTask; + }); + JObject parsed = JObject.Parse(response); + JObject accepted = JObject.Parse(acceptedResponse); + JObject heartbeat = JObject.Parse(createHeartbeatJson()); + + Assert.That(parsed["error"], Is.Null); + Assert.That(accepted["uloop"]?["phase"]?.ToString(), Is.EqualTo(JsonRpcResponsePhases.Accepted)); + Assert.That( + accepted["uloop"]?["heartbeatIntervalSeconds"]?.ToObject(), + Is.EqualTo(UnityCliLoopServerConfig.HEARTBEAT_INTERVAL_SECONDS)); + Assert.That(heartbeat["uloop"]?["phase"]?.ToString(), Is.EqualTo(JsonRpcResponsePhases.Heartbeat)); + Assert.That(heartbeat["uloop"]?["mainThreadStallSeconds"]?.Type, Is.EqualTo(JTokenType.Float)); + } + [Test] public async Task ProcessRequest_WhenCliMetadataIsMissing_ReturnsCliUpdateRequiredError() { @@ -588,6 +617,18 @@ private static string BuildToolRequestWithParams(string toolName, string paramsJ ",\"acceptsDispatchAck\":true}}"; } + private static string BuildHeartbeatNegotiatedToolRequest(string toolName, int id) + { + return + "{\"jsonrpc\":\"2.0\",\"method\":\"" + + toolName + + "\",\"params\":{},\"id\":" + + id + + ",\"uloop\":{\"protocolVersion\":" + + CliConstants.REQUIRED_CLI_PROTOCOL_VERSION + + ",\"acceptsDispatchAck\":true,\"acceptsHeartbeat\":true}}"; + } + private static JObject ParseErrorData(string response) { JObject error = ParseError(response); diff --git a/cli/cmd/check-ipc-protocol-reminder/main.go b/cli/cmd/check-ipc-protocol-reminder/main.go new file mode 100644 index 0000000000..15a6d95880 --- /dev/null +++ b/cli/cmd/check-ipc-protocol-reminder/main.go @@ -0,0 +1,25 @@ +package main + +import ( + "context" + "flag" + "os" + + "github.com/hatayama/unity-cli-loop/cli/internal/automation" +) + +func main() { + baseRef := flag.String("base", "", "base git ref to compare from") + headRef := flag.String("head", "HEAD", "head git ref to compare to") + flag.Parse() + + os.Exit(automation.RunIPCProtocolReminder( + context.Background(), + os.Stdout, + os.Stderr, + automation.IPCProtocolReminderConfig{ + BaseRef: *baseRef, + HeadRef: *headRef, + StepSummaryPath: os.Getenv("GITHUB_STEP_SUMMARY"), + })) +} diff --git a/cli/internal/automation/ipc_protocol_reminder.go b/cli/internal/automation/ipc_protocol_reminder.go new file mode 100644 index 0000000000..5c8de2997a --- /dev/null +++ b/cli/internal/automation/ipc_protocol_reminder.go @@ -0,0 +1,208 @@ +package automation + +import ( + "context" + "fmt" + "io" + "os" + "os/exec" + "sort" + "strings" +) + +var ipcProtocolReminderPatterns = []string{ + "cli/contract.json", + "cli/contract.go", + "cli/layout-contract.json", + "cli/internal/unityipc/**", + "cli/internal/tools/**", + "Packages/src/Editor/CompositionRoot/UnityCliLoopFirstPartyServerLifecycleBinding.cs", + "Packages/src/Editor/Domain/CliConstants.cs", + "Packages/src/Editor/Infrastructure/Api/**", + "Packages/src/Editor/ToolContracts/UnityCliLoopConstants.cs", + "Packages/src/Editor/ToolContracts/UnityCliLoopToolSchema.cs", + "Packages/src/Editor/ToolContracts/UnityCliLoopToolParameterSchemaGenerator.cs", +} + +var ipcProtocolDeclarationPaths = []string{ + "cli/contract.json", + "Packages/src/Editor/Domain/CliConstants.cs", +} + +type IPCProtocolReminderConfig struct { + BaseRef string + HeadRef string + StepSummaryPath string +} + +type IPCProtocolReminderResult struct { + ChangedIPCFiles []string + ChangedProtocolFiles []string + NeedsProtocolBumpReview bool + HasProtocolFileChange bool + HasIPCContractFileChange bool +} + +func RunIPCProtocolReminder(ctx context.Context, stdout io.Writer, stderr io.Writer, config IPCProtocolReminderConfig) int { + if config.BaseRef == "" { + writeIPCProtocolReminderLine(stderr, "--base is required") + return 1 + } + if config.HeadRef == "" { + config.HeadRef = "HEAD" + } + + repoRoot, err := gitRepoRoot(ctx) + if err != nil { + writeIPCProtocolReminderLine(stderr, fmt.Sprintf("failed to resolve git repository root: %v", err)) + return 1 + } + + changedFiles, err := gitChangedFiles(ctx, repoRoot, config.BaseRef, config.HeadRef) + if err != nil { + writeIPCProtocolReminderLine(stderr, fmt.Sprintf("failed to inspect changed files: %v", err)) + return 1 + } + + result := AnalyzeIPCProtocolReminder(changedFiles) + message := FormatIPCProtocolReminder(result) + writeIPCProtocolReminderLine(stdout, message) + + if result.NeedsProtocolBumpReview { + writeIPCProtocolReminderLine(stdout, "::notice title=Review IPC protocol version::IPC-facing files changed without protocol declaration changes. Review whether protocolVersion and REQUIRED_CLI_PROTOCOL_VERSION must be bumped.") + } + + if config.StepSummaryPath != "" { + err = AppendIPCProtocolReminderSummary(config.StepSummaryPath, result) + if err != nil { + writeIPCProtocolReminderLine(stderr, fmt.Sprintf("failed to append GitHub step summary: %v", err)) + return 1 + } + } + + return 0 +} + +func writeIPCProtocolReminderLine(writer io.Writer, value string) { + _, _ = fmt.Fprintln(writer, value) +} + +func AnalyzeIPCProtocolReminder(changedFiles []string) IPCProtocolReminderResult { + changedIPCFiles := []string{} + changedProtocolFiles := []string{} + for _, file := range changedFiles { + normalizedFile := strings.TrimPrefix(strings.TrimSpace(file), "./") + if normalizedFile == "" { + continue + } + if isProtocolDeclarationPath(normalizedFile) { + changedProtocolFiles = append(changedProtocolFiles, normalizedFile) + } + if matchesAnyIPCProtocolReminderPattern(normalizedFile) { + changedIPCFiles = append(changedIPCFiles, normalizedFile) + } + } + + sort.Strings(changedIPCFiles) + sort.Strings(changedProtocolFiles) + hasIPCContractFileChange := len(changedIPCFiles) > 0 + hasProtocolFileChange := len(changedProtocolFiles) > 0 + return IPCProtocolReminderResult{ + ChangedIPCFiles: changedIPCFiles, + ChangedProtocolFiles: changedProtocolFiles, + NeedsProtocolBumpReview: hasIPCContractFileChange && !hasProtocolFileChange, + HasProtocolFileChange: hasProtocolFileChange, + HasIPCContractFileChange: hasIPCContractFileChange, + } +} + +func FormatIPCProtocolReminder(result IPCProtocolReminderResult) string { + if !result.HasIPCContractFileChange { + return "No IPC contract surfaces changed." + } + if result.NeedsProtocolBumpReview { + return "IPC contract surfaces changed without protocol declaration changes; review whether protocolVersion must be bumped." + } + return "IPC contract surfaces changed and protocol declarations changed; verify the bump is intentional and both sides stay aligned." +} + +func AppendIPCProtocolReminderSummary(summaryPath string, result IPCProtocolReminderResult) error { + content := strings.Builder{} + content.WriteString("## IPC protocol version reminder\n\n") + content.WriteString(FormatIPCProtocolReminder(result)) + content.WriteString("\n\n") + if len(result.ChangedIPCFiles) > 0 { + content.WriteString("Changed IPC-facing files:\n") + for _, file := range result.ChangedIPCFiles { + content.WriteString("- `") + content.WriteString(file) + content.WriteString("`\n") + } + content.WriteString("\n") + } + if result.NeedsProtocolBumpReview { + content.WriteString("If this PR breaks interoperability with the previous protocol generation, bump both `cli/contract.json` `protocolVersion` and `CliConstants.REQUIRED_CLI_PROTOCOL_VERSION` in the same PR.\n\n") + } + + file, err := os.OpenFile(summaryPath, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o644) + if err != nil { + return err + } + defer func() { + _ = file.Close() + }() + _, err = file.WriteString(content.String()) + return err +} + +func matchesAnyIPCProtocolReminderPattern(file string) bool { + for _, pattern := range ipcProtocolReminderPatterns { + if matchesIPCProtocolReminderPattern(file, pattern) { + return true + } + } + return false +} + +func matchesIPCProtocolReminderPattern(file string, pattern string) bool { + if strings.HasSuffix(pattern, "/**") { + prefix := strings.TrimSuffix(pattern, "**") + return strings.HasPrefix(file, prefix) + } + return file == pattern +} + +func isProtocolDeclarationPath(file string) bool { + for _, path := range ipcProtocolDeclarationPaths { + if file == path { + return true + } + } + return false +} + +func gitRepoRoot(ctx context.Context) (string, error) { + output, err := exec.CommandContext(ctx, "git", "rev-parse", "--show-toplevel").Output() + if err != nil { + return "", err + } + return strings.TrimSpace(string(output)), nil +} + +func gitChangedFiles(ctx context.Context, repoRoot string, baseRef string, headRef string) ([]string, error) { + rangeSpec := baseRef + "..." + headRef + command := exec.CommandContext(ctx, "git", "-C", repoRoot, "diff", "--name-only", rangeSpec) + output, err := command.Output() + if err != nil { + return nil, err + } + lines := strings.Split(strings.TrimSpace(string(output)), "\n") + files := []string{} + for _, line := range lines { + file := strings.TrimSpace(line) + if file != "" { + files = append(files, file) + } + } + return files, nil +} diff --git a/cli/internal/automation/ipc_protocol_reminder_test.go b/cli/internal/automation/ipc_protocol_reminder_test.go new file mode 100644 index 0000000000..bcfe471834 --- /dev/null +++ b/cli/internal/automation/ipc_protocol_reminder_test.go @@ -0,0 +1,80 @@ +package automation + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestAnalyzeIPCProtocolReminder_WhenNoIPCFilesChanged_DoesNotRemind(t *testing.T) { + // Verifies ordinary changes do not create protocol bump noise. + result := AnalyzeIPCProtocolReminder([]string{ + "README.md", + "Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs", + }) + + if result.NeedsProtocolBumpReview { + t.Fatalf("unexpected reminder: %#v", result) + } + if result.HasIPCContractFileChange { + t.Fatalf("unexpected IPC file detection: %#v", result) + } +} + +func TestAnalyzeIPCProtocolReminder_WhenIPCFilesChangedWithoutProtocolDeclarations_Reminds(t *testing.T) { + // Verifies IPC-facing changes surface a non-blocking protocol bump review reminder. + result := AnalyzeIPCProtocolReminder([]string{ + "cli/internal/unityipc/client.go", + "Packages/src/Editor/Infrastructure/Api/JsonRpcProcessor.cs", + }) + + if !result.NeedsProtocolBumpReview { + t.Fatalf("expected reminder: %#v", result) + } + if len(result.ChangedIPCFiles) != 2 { + t.Fatalf("changed IPC files mismatch: %#v", result.ChangedIPCFiles) + } +} + +func TestAnalyzeIPCProtocolReminder_WhenProtocolDeclarationsChanged_DoesNotRemind(t *testing.T) { + // Verifies explicit protocol declaration edits satisfy the reminder check. + result := AnalyzeIPCProtocolReminder([]string{ + "cli/internal/unityipc/client.go", + "cli/contract.json", + "Packages/src/Editor/Domain/CliConstants.cs", + }) + + if result.NeedsProtocolBumpReview { + t.Fatalf("unexpected reminder: %#v", result) + } + if !result.HasProtocolFileChange { + t.Fatalf("expected protocol file change: %#v", result) + } +} + +func TestAppendIPCProtocolReminderSummary_WritesChangedFilesAndGuidance(t *testing.T) { + // Verifies the GitHub step summary contains enough context for reviewers. + summaryPath := filepath.Join(t.TempDir(), "summary.md") + result := AnalyzeIPCProtocolReminder([]string{ + "Packages/src/Editor/Infrastructure/Api/JsonRpcProcessor.cs", + }) + + if err := AppendIPCProtocolReminderSummary(summaryPath, result); err != nil { + t.Fatalf("failed to append summary: %v", err) + } + content, err := os.ReadFile(summaryPath) + if err != nil { + t.Fatalf("failed to read summary: %v", err) + } + text := string(content) + if !strings.Contains(text, "IPC protocol version reminder") { + t.Fatalf("summary misses heading:\n%s", text) + } + if !strings.Contains(text, "JsonRpcProcessor.cs") { + t.Fatalf("summary misses changed file:\n%s", text) + } + if !strings.Contains(text, "REQUIRED_CLI_PROTOCOL_VERSION") { + t.Fatalf("summary misses protocol guidance:\n%s", text) + } +} diff --git a/cli/internal/unityipc/client_test.go b/cli/internal/unityipc/client_test.go index 9e694d3f27..fba0ed234e 100644 --- a/cli/internal/unityipc/client_test.go +++ b/cli/internal/unityipc/client_test.go @@ -118,6 +118,9 @@ func TestSendIncludesCliVersionWithoutProjectIdentityMetadata(t *testing.T) { if metadata["acceptsDispatchAck"] != true { t.Fatalf("dispatch ack metadata mismatch: %#v", metadata) } + if metadata["acceptsHeartbeat"] != true { + t.Fatalf("heartbeat metadata mismatch: %#v", metadata) + } } } From 6f860ec78f92a92942fad421b41bfd461c3d0415 Mon Sep 17 00:00:00 2001 From: hatayama Date: Sun, 14 Jun 2026 15:25:11 +0900 Subject: [PATCH 9/9] Handle oversized protocol metadata Treat protocolVersion integers outside the Int32 range as missing metadata so compatibility checks return deterministic update guidance instead of parser overflow errors. --- .../Editor/CliInstallationDetectorTests.cs | 28 +++++++++++++++++ .../JsonRpcProcessorCliVersionGateTests.cs | 15 +++++++++ .../Infrastructure/Api/JsonRpcProcessor.cs | 19 +++++++++++- .../CLI/CliInstallationDetector.cs | 31 +++++++++++++++++-- 4 files changed, 89 insertions(+), 4 deletions(-) diff --git a/Assets/Tests/Editor/CliInstallationDetectorTests.cs b/Assets/Tests/Editor/CliInstallationDetectorTests.cs index d29ca0d352..3800c6d038 100644 --- a/Assets/Tests/Editor/CliInstallationDetectorTests.cs +++ b/Assets/Tests/Editor/CliInstallationDetectorTests.cs @@ -201,6 +201,34 @@ public void ParseShellCliInstallationOutput_WhenContractJsonExists_ReturnsProtoc Assert.That(detection.ExecutablePath, Is.EqualTo("/Users/ExampleUser/.npm-global/bin/uloop")); } + [Test] + public void ParseShellCliInstallationOutput_WhenProtocolVersionIsOutsideIntRange_ReturnsVersionWithoutProtocol() + { + // Verifies oversized protocol metadata cannot break setup compatibility detection. + string output = "__ULOOP_PATH_START__\n" + + "/tmp/uloop\n" + + "__ULOOP_PATH_END__\n" + + "__ULOOP_CONTRACT_START__\n" + + "{\"cliVersion\":\"3.0.0-beta.31\",\"protocolVersion\":2147483648}\n" + + "__ULOOP_CONTRACT_END__\n" + + "__ULOOP_CONTRACT_STATUS_START__\n" + + "0\n" + + "__ULOOP_CONTRACT_STATUS_END__\n" + + "__ULOOP_VERSION_START__\n" + + "2.1.1\n" + + "__ULOOP_VERSION_END__\n" + + "__ULOOP_VERSION_STATUS_START__\n" + + "0\n" + + "__ULOOP_VERSION_STATUS_END__\n"; + + CliInstallationDetection detection = + CliInstallationDetector.ParseShellCliInstallationOutput(output); + + Assert.That(detection.Version, Is.EqualTo("3.0.0-beta.31")); + Assert.That(detection.ProtocolVersion, Is.Null); + Assert.That(detection.ExecutablePath, Is.EqualTo("/tmp/uloop")); + } + [Test] public void ParseShellCliInstallationOutput_WhenOnlyVersionExists_ReturnsInstalledDetection() { diff --git a/Assets/Tests/Editor/JsonRpcProcessorCliVersionGateTests.cs b/Assets/Tests/Editor/JsonRpcProcessorCliVersionGateTests.cs index 596f176f39..aa1f8c99aa 100644 --- a/Assets/Tests/Editor/JsonRpcProcessorCliVersionGateTests.cs +++ b/Assets/Tests/Editor/JsonRpcProcessorCliVersionGateTests.cs @@ -150,6 +150,21 @@ public async Task ProcessRequest_WhenProtocolVersionIsNotAnInteger_ReturnsCliUpd Assert.That(data["updateCommand"]?.ToString(), Is.EqualTo(ExpectedCliUpdateCommand())); } + [Test] + public async Task ProcessRequest_WhenProtocolVersionIsOutsideIntRange_ReturnsCliUpdateRequiredError() + { + // Verifies oversized protocol values are treated as missing metadata instead of parser failures. + string response = await JsonRpcProcessor.ProcessRequest( + "{\"jsonrpc\":\"2.0\",\"method\":\"get-version\",\"params\":{},\"id\":1," + + "\"uloop\":{\"protocolVersion\":2147483648}}", + CancellationToken.None); + JObject data = ParseErrorData(response); + + Assert.That(data["type"]?.ToString(), Is.EqualTo("cli_update_required")); + Assert.That(data["currentProtocolVersion"]?.Type, Is.EqualTo(JTokenType.Null)); + Assert.That(data["updateCommand"]?.ToString(), Is.EqualTo(ExpectedCliUpdateCommand())); + } + [Test] public async Task ProcessRequest_WhenFirstToolWaitsForMainThread_ReturnsServerBusyForSecondTool() { diff --git a/Packages/src/Editor/Infrastructure/Api/JsonRpcProcessor.cs b/Packages/src/Editor/Infrastructure/Api/JsonRpcProcessor.cs index 3bdbfa8ec8..7999aa7aae 100644 --- a/Packages/src/Editor/Infrastructure/Api/JsonRpcProcessor.cs +++ b/Packages/src/Editor/Infrastructure/Api/JsonRpcProcessor.cs @@ -126,7 +126,24 @@ private static string ReadClientCliVersion(JObject request) return null; } - return protocolVersionToken.Value(); + JValue protocolVersionValue = protocolVersionToken as JValue; + object rawProtocolVersion = protocolVersionValue?.Value; + if (rawProtocolVersion is int protocolVersion) + { + return protocolVersion; + } + + if (!(rawProtocolVersion is long longProtocolVersion)) + { + return null; + } + + if (longProtocolVersion < int.MinValue || longProtocolVersion > int.MaxValue) + { + return null; + } + + return (int)longProtocolVersion; } private static bool ReadAcceptsDispatchAck(JObject request) diff --git a/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs b/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs index e149abbc04..a867c85b31 100644 --- a/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs +++ b/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs @@ -364,9 +364,7 @@ private static CliInstallationDetection ParseCliContractOutput(string output, st JObject parsed = JObject.Parse(jsonLine); string version = parsed["cliVersion"]?.ToString(); JToken protocolVersionToken = parsed["protocolVersion"]; - int? protocolVersion = protocolVersionToken != null && protocolVersionToken.Type == JTokenType.Integer - ? protocolVersionToken.Value() - : null; + int? protocolVersion = ReadProtocolVersion(protocolVersionToken); return new CliInstallationDetection(version, protocolVersion, executablePath); } catch (JsonException) @@ -375,6 +373,33 @@ private static CliInstallationDetection ParseCliContractOutput(string output, st } } + private static int? ReadProtocolVersion(JToken protocolVersionToken) + { + if (protocolVersionToken == null || protocolVersionToken.Type != JTokenType.Integer) + { + return null; + } + + JValue protocolVersionValue = protocolVersionToken as JValue; + object rawProtocolVersion = protocolVersionValue?.Value; + if (rawProtocolVersion is int protocolVersion) + { + return protocolVersion; + } + + if (!(rawProtocolVersion is long longProtocolVersion)) + { + return null; + } + + if (longProtocolVersion < int.MinValue || longProtocolVersion > int.MaxValue) + { + return null; + } + + return (int)longProtocolVersion; + } + private static string ExecuteAndGetOutput(ProcessStartInfo startInfo, CancellationToken ct) { UnityEngine.Debug.Assert(startInfo != null, "startInfo must not be null");