From 0a215aa99a9be3b0a4548ad373c47ee22be017b5 Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Tue, 14 Jul 2026 11:18:29 +0900 Subject: [PATCH 1/7] chore: add get-compile-status response contract fixture (#1773) Co-authored-by: Cursor --- .../GetCompileStatusResponseContractTests.cs | 123 ++++++++++++++++++ ...CompileStatusResponseContractTests.cs.meta | 11 ++ .../compile_status_response_contract_test.go | 72 ++++++++++ .../compile_status_response_contract.json | 18 +++ 4 files changed, 224 insertions(+) create mode 100644 Assets/Tests/Editor/GetCompileStatusResponseContractTests.cs create mode 100644 Assets/Tests/Editor/GetCompileStatusResponseContractTests.cs.meta create mode 100644 cli/project-runner/internal/projectrunner/compile_status_response_contract_test.go create mode 100644 tests/contracts/compile_status_response_contract.json diff --git a/Assets/Tests/Editor/GetCompileStatusResponseContractTests.cs b/Assets/Tests/Editor/GetCompileStatusResponseContractTests.cs new file mode 100644 index 0000000000..12ad062480 --- /dev/null +++ b/Assets/Tests/Editor/GetCompileStatusResponseContractTests.cs @@ -0,0 +1,123 @@ +using System; +using System.IO; +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; +using io.github.hatayama.UnityCliLoop.Infrastructure; +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Test fixture that verifies Unity get-compile-status responses stay aligned with the shared Go CLI contract. + /// + public sealed class GetCompileStatusResponseContractTests + { + private const string SharedContractPath = "tests/contracts/compile_status_response_contract.json"; + + [Test] + public void GetCompileStatusResponse_WhenSerialized_MatchesSharedContractFieldShape() + { + // Verifies C# does not add, remove, or rename get-compile-status fields without updating the + // shared CLI contract. Result must come from a real CompileResponse serialization because that + // is what CompileStatusBridgeCommand restores from ResultJson for Go compileResultStatus.Success. + JObject expected = ReadSharedContractFieldShape(); + JObject compileResultJson = SerializeCompileResponseResult(); + GetCompileStatusResponse response = new() + { + Ready = true, + HasResult = true, + IsCompiling = true, + IsUpdating = true, + IsDomainReloadInProgress = true, + Result = compileResultJson, + Message = "Compile result is available." + }; + string json = JsonConvert.SerializeObject( + response, + Formatting.None, + UnityCliLoopJsonResponseSerializerSettings.Settings); + JObject actual = NormalizeFieldShape(JObject.Parse(json)); + + Assert.That(JToken.DeepEquals(actual, expected), Is.True, $"Expected {expected} but got {actual}"); + } + + [Test] + public void CompileResponse_WhenSerializedForCompileStatusResult_IncludesSuccessProperty() + { + // Verifies the wire Result payload still exposes Success under the name Go unmarshals into + // compileResultStatus — a rename on CompileResponse must fail this test. + JObject compileResultJson = SerializeCompileResponseResult(); + Assert.That(compileResultJson.Property("Success"), Is.Not.Null); + Assert.That(compileResultJson["Success"]!.Type, Is.EqualTo(JTokenType.Boolean)); + } + + private static JObject SerializeCompileResponseResult() + { + CompileResponse compileResult = new( + success: true, + errorCount: 0, + warningCount: 0, + errors: Array.Empty(), + warnings: Array.Empty(), + message: "Compile result is available."); + string resultJson = JsonConvert.SerializeObject( + compileResult, + Formatting.None, + UnityCliLoopJsonResponseSerializerSettings.Settings); + return JObject.Parse(resultJson); + } + + private static JObject ReadSharedContractFieldShape() + { + string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); + string json = File.ReadAllText(Path.Combine(projectRoot, SharedContractPath)); + return NormalizeFieldShape(JObject.Parse(json)); + } + + private static JObject NormalizeFieldShape(JObject value) + { + JObject shape = new(); + foreach (JProperty property in value.Properties()) + { + shape[property.Name] = NormalizeTokenFieldShape(property.Value); + } + return shape; + } + + private static JToken NormalizeTokenFieldShape(JToken value) + { + if (value is JObject objectValue) + { + return NormalizeFieldShape(objectValue); + } + + if (value is JArray arrayValue) + { + JArray arrayShape = new(); + if (arrayValue.Count > 0) + { + arrayShape.Add(NormalizeTokenFieldShape(arrayValue[0])); + } + return arrayShape; + } + + return new JValue(NormalizeScalarType(value.Type)); + } + + private static string NormalizeScalarType(JTokenType type) + { + return type switch + { + JTokenType.Boolean => "boolean", + JTokenType.Float => "number", + JTokenType.Integer => "number", + JTokenType.Null => "null", + JTokenType.String => "string", + _ => "unknown" + }; + } + } +} diff --git a/Assets/Tests/Editor/GetCompileStatusResponseContractTests.cs.meta b/Assets/Tests/Editor/GetCompileStatusResponseContractTests.cs.meta new file mode 100644 index 0000000000..c10d6078a4 --- /dev/null +++ b/Assets/Tests/Editor/GetCompileStatusResponseContractTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 8f2a4c6e1d3b5a790c8e4f6a2b0d1e3f +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/cli/project-runner/internal/projectrunner/compile_status_response_contract_test.go b/cli/project-runner/internal/projectrunner/compile_status_response_contract_test.go new file mode 100644 index 0000000000..71d3d906a7 --- /dev/null +++ b/cli/project-runner/internal/projectrunner/compile_status_response_contract_test.go @@ -0,0 +1,72 @@ +package projectrunner + +import ( + "encoding/json" + "os" + "reflect" + "testing" +) + +const compileStatusResponseContractPath = "tests/contracts/compile_status_response_contract.json" + +// Verifies the Go compileStatusResponse DTO preserves every field in the shared Unity +// get-compile-status response contract, including nested Result.Success used by compileResultStatus. +func TestCompileStatusResponseMatchesSharedContract(t *testing.T) { + fixturePath := findRepoRelativeFile(t, compileStatusResponseContractPath) + fixture, err := os.ReadFile(fixturePath) + if err != nil { + t.Fatalf("failed to read shared compile status response contract: %v", err) + } + + var response compileStatusResponse + if err := json.Unmarshal(fixture, &response); err != nil { + t.Fatalf("failed to unmarshal shared compile status response contract: %v", err) + } + + assertCompileStatusResponseFieldsPopulated(t, response) + + var resultStatus compileResultStatus + if err := json.Unmarshal(response.Result, &resultStatus); err != nil { + t.Fatalf("failed to unmarshal Result into compileResultStatus: %v", err) + } + if resultStatus.Success == nil { + t.Fatal("compileResultStatus.Success must be non-nil after unmarshaling the shared contract") + } + + roundTripped, err := json.Marshal(response) + if err != nil { + t.Fatalf("failed to marshal Go compile status response: %v", err) + } + + expected := readJSONFieldShape(t, fixture) + actual := readJSONFieldShape(t, roundTripped) + if !reflect.DeepEqual(actual, expected) { + t.Fatalf("compile status response field shape drifted\nexpected: %#v\nactual: %#v", expected, actual) + } +} + +func assertCompileStatusResponseFieldsPopulated(t *testing.T, response compileStatusResponse) { + t.Helper() + + if !response.Ready { + t.Fatal("Ready must be true in the shared contract fixture") + } + if !response.HasResult { + t.Fatal("HasResult must be true in the shared contract fixture") + } + if !response.IsCompiling { + t.Fatal("IsCompiling must be true in the shared contract fixture") + } + if !response.IsUpdating { + t.Fatal("IsUpdating must be true in the shared contract fixture") + } + if !response.IsDomainReloadInProgress { + t.Fatal("IsDomainReloadInProgress must be true in the shared contract fixture") + } + if len(response.Result) == 0 { + t.Fatal("Result must be non-empty in the shared contract fixture") + } + if response.Message == "" { + t.Fatal("Message must be non-empty in the shared contract fixture") + } +} diff --git a/tests/contracts/compile_status_response_contract.json b/tests/contracts/compile_status_response_contract.json new file mode 100644 index 0000000000..243ff58398 --- /dev/null +++ b/tests/contracts/compile_status_response_contract.json @@ -0,0 +1,18 @@ +{ + "Ready": true, + "HasResult": true, + "IsCompiling": true, + "IsUpdating": true, + "IsDomainReloadInProgress": true, + "Result": { + "Success": true, + "ErrorCount": 0, + "WarningCount": 0, + "Errors": [], + "Warnings": [], + "Message": "Compile result is available.", + "NextActions": null, + "ProjectRoot": null + }, + "Message": "Compile result is available." +} From 56c1c011061a05067f0f56ad240012737e571d63 Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Tue, 14 Jul 2026 11:24:11 +0900 Subject: [PATCH 2/7] chore: add endpoint and cli_update_required contract fixtures (#1774) Co-authored-by: Cursor --- .../Editor/BridgeTransportEndpointTests.cs | 72 +++++++++++++ .../CliUpdateRequiredErrorContractTests.cs | 90 ++++++++++++++++ ...liUpdateRequiredErrorContractTests.cs.meta | 11 ++ ...cli_update_required_error_contract_test.go | 76 +++++++++++++ cli/common/project/endpoint_contract_test.go | 100 ++++++++++++++++++ .../cli_update_required_error_contract.json | 12 +++ tests/contracts/endpoint_contract.json | 30 ++++++ 7 files changed, 391 insertions(+) create mode 100644 Assets/Tests/Editor/CliUpdateRequiredErrorContractTests.cs create mode 100644 Assets/Tests/Editor/CliUpdateRequiredErrorContractTests.cs.meta create mode 100644 cli/common/errors/cli_update_required_error_contract_test.go create mode 100644 cli/common/project/endpoint_contract_test.go create mode 100644 tests/contracts/cli_update_required_error_contract.json create mode 100644 tests/contracts/endpoint_contract.json diff --git a/Assets/Tests/Editor/BridgeTransportEndpointTests.cs b/Assets/Tests/Editor/BridgeTransportEndpointTests.cs index a1d90518af..bb81625bcf 100644 --- a/Assets/Tests/Editor/BridgeTransportEndpointTests.cs +++ b/Assets/Tests/Editor/BridgeTransportEndpointTests.cs @@ -1,7 +1,12 @@ +using System; +using System.Collections.Generic; using System.IO; +using System.Runtime.InteropServices; +using Newtonsoft.Json.Linq; using NUnit.Framework; using io.github.hatayama.UnityCliLoop.Infrastructure; +using io.github.hatayama.UnityCliLoop.ToolContracts; namespace io.github.hatayama.UnityCliLoop.Tests.Editor { @@ -10,6 +15,8 @@ namespace io.github.hatayama.UnityCliLoop.Tests.Editor /// public class BridgeTransportEndpointTests { + private const string SharedEndpointContractPath = "tests/contracts/endpoint_contract.json"; + [Test] public void CanonicalizeProjectRoot_WhenPathIsFilesystemRoot_ShouldPreserveRoot() { @@ -20,5 +27,70 @@ public void CanonicalizeProjectRoot_WhenPathIsFilesystemRoot_ShouldPreserveRoot( Assert.That(canonicalProjectRoot, Is.EqualTo(filesystemRoot)); } + + [Test] + public void CreateProjectIpc_WhenCanonicalRootsFromSharedContract_MatchExpectedEndpointPaths() + { + // Verifies Go and C# derive the same IPC endpoint names from already-canonicalized roots + // listed in the shared endpoint contract (symlink resolution is out of scope). + foreach (JObject contractCase in ReadEndpointContractCases()) + { + string caseId = contractCase.Value("id"); + string canonicalProjectRoot = contractCase.Value("canonicalProjectRoot"); + if (!CanExerciseProjectRootOnCurrentPlatform(canonicalProjectRoot)) + { + continue; + } + + string expectedPath = ExpectedEndpointPathForCurrentPlatform(contractCase); + BridgeTransportEndpoint endpoint = BridgeTransportEndpoint.CreateProjectIpc(canonicalProjectRoot); + Assert.That(endpoint.Path, Is.EqualTo(expectedPath), $"case {caseId} canonical root"); + + JArray trimOnlyEquivalentRoots = contractCase.Value("trimOnlyEquivalentRoots") ?? new JArray(); + foreach (JToken equivalentRootToken in trimOnlyEquivalentRoots) + { + string equivalentRoot = equivalentRootToken.Value(); + BridgeTransportEndpoint trimmedEndpoint = BridgeTransportEndpoint.CreateProjectIpc(equivalentRoot); + Assert.That( + trimmedEndpoint.Path, + Is.EqualTo(expectedPath), + $"case {caseId} trim-only equivalent root {equivalentRoot}"); + } + } + } + + private static IEnumerable ReadEndpointContractCases() + { + string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); + string json = File.ReadAllText(Path.Combine(projectRoot, SharedEndpointContractPath)); + JObject contract = JObject.Parse(json); + JArray cases = contract.Value("cases"); + Assert.That(cases, Is.Not.Null.And.Not.Empty); + foreach (JToken caseToken in cases) + { + yield return (JObject)caseToken; + } + } + + private static bool CanExerciseProjectRootOnCurrentPlatform(string projectRoot) + { + bool looksLikeWindowsPath = projectRoot.Length >= 2 && projectRoot[1] == ':'; + if (looksLikeWindowsPath) + { + return RuntimeInformation.IsOSPlatform(OSPlatform.Windows); + } + + return !RuntimeInformation.IsOSPlatform(OSPlatform.Windows); + } + + private static string ExpectedEndpointPathForCurrentPlatform(JObject contractCase) + { + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + return contractCase.Value("windowsPipePath"); + } + + return contractCase.Value("unixSocketPath"); + } } } diff --git a/Assets/Tests/Editor/CliUpdateRequiredErrorContractTests.cs b/Assets/Tests/Editor/CliUpdateRequiredErrorContractTests.cs new file mode 100644 index 0000000000..606e84afa8 --- /dev/null +++ b/Assets/Tests/Editor/CliUpdateRequiredErrorContractTests.cs @@ -0,0 +1,90 @@ +using System.IO; +using Newtonsoft.Json.Linq; +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.Domain; +using io.github.hatayama.UnityCliLoop.Infrastructure; +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Freezes the cli_update_required JSON-RPC error.data frame that old CLIs must keep parsing forever. + /// + public sealed class CliUpdateRequiredErrorContractTests + { + private const string SharedContractPath = "tests/contracts/cli_update_required_error_contract.json"; + + [Test] + public void CreateCliProtocolMismatchResponse_WhenOlderProtocol_MatchesSharedErrorDataFieldShape() + { + // This frame must remain backward compatible forever — old CLIs parse it to learn they must update. + // Verifies JsonRpcResponseFactory still emits the frozen error.data field shape for older CLIs. + JObject expected = ReadSharedErrorDataFieldShape(); + string responseJson = JsonRpcResponseFactory.CreateCliProtocolMismatchResponse( + id: "contract-test", + currentCliVersion: "3.0.0-beta.5", + currentProtocolVersion: 1); + JObject actualData = (JObject)JObject.Parse(responseJson)["error"]!["data"]!; + JObject actual = NormalizeFieldShape(actualData); + + Assert.That(JToken.DeepEquals(actual, expected), Is.True, $"Expected {expected} but got {actual}"); + Assert.That(actualData.Value("type"), Is.EqualTo("cli_update_required")); + Assert.That( + actualData.Value("requiredProtocolVersion"), + Is.EqualTo(CliConstants.REQUIRED_CLI_PROTOCOL_VERSION)); + Assert.That(actualData.Value("updateCommand"), Is.EqualTo("uloop update")); + } + + private static JObject ReadSharedErrorDataFieldShape() + { + string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); + string json = File.ReadAllText(Path.Combine(projectRoot, SharedContractPath)); + JObject contract = JObject.Parse(json); + return NormalizeFieldShape((JObject)contract["errorData"]!); + } + + private static JObject NormalizeFieldShape(JObject value) + { + JObject shape = new(); + foreach (JProperty property in value.Properties()) + { + shape[property.Name] = NormalizeTokenFieldShape(property.Value); + } + return shape; + } + + private static JToken NormalizeTokenFieldShape(JToken value) + { + if (value is JObject objectValue) + { + return NormalizeFieldShape(objectValue); + } + + if (value is JArray arrayValue) + { + JArray arrayShape = new(); + if (arrayValue.Count > 0) + { + arrayShape.Add(NormalizeTokenFieldShape(arrayValue[0])); + } + return arrayShape; + } + + return new JValue(NormalizeScalarType(value.Type)); + } + + private static string NormalizeScalarType(JTokenType type) + { + return type switch + { + JTokenType.Boolean => "boolean", + JTokenType.Float => "number", + JTokenType.Integer => "number", + JTokenType.Null => "null", + JTokenType.String => "string", + _ => "unknown" + }; + } + } +} diff --git a/Assets/Tests/Editor/CliUpdateRequiredErrorContractTests.cs.meta b/Assets/Tests/Editor/CliUpdateRequiredErrorContractTests.cs.meta new file mode 100644 index 0000000000..d3ba7cad64 --- /dev/null +++ b/Assets/Tests/Editor/CliUpdateRequiredErrorContractTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 3d9e7b1a5c4f48e2a6b0d8f1c2e4a7b9 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/cli/common/errors/cli_update_required_error_contract_test.go b/cli/common/errors/cli_update_required_error_contract_test.go new file mode 100644 index 0000000000..412f583cde --- /dev/null +++ b/cli/common/errors/cli_update_required_error_contract_test.go @@ -0,0 +1,76 @@ +package clierrors + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/hatayama/unity-cli-loop/common/unityipc" +) + +const cliUpdateRequiredErrorContractPath = "tests/contracts/cli_update_required_error_contract.json" + +type cliUpdateRequiredErrorContractFile struct { + // This frame must remain backward compatible forever — old CLIs parse it to learn they must update. + Comment string `json:"comment"` + ErrorData json.RawMessage `json:"errorData"` +} + +// Verifies the frozen cli_update_required error.data frame still classifies as CLI_UPDATE_REQUIRED. +// This frame must remain backward compatible forever — old CLIs parse it to learn they must update. +func TestClassifyError_WhenCliUpdateRequiredContractFixture_ClassifiesAsCLIUpdateRequired(t *testing.T) { + contract := readCliUpdateRequiredErrorContract(t) + + err := &unityipc.RPCError{ + Code: -32603, + Message: "The installed uloop CLI uses an IPC protocol that does not match this Unity package.", + Data: contract.ErrorData, + } + + cliErr := ClassifyError(err, ErrorContext{ProjectRoot: "/tmp/MyProject", Command: "compile"}) + if cliErr.ErrorCode != ErrorCodeCLIUpdateRequired { + t.Fatalf("expected %s, got %#v", ErrorCodeCLIUpdateRequired, cliErr) + } +} + +func readCliUpdateRequiredErrorContract(t *testing.T) cliUpdateRequiredErrorContractFile { + t.Helper() + + path := findRepoRelativeFile(t, cliUpdateRequiredErrorContractPath) + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("failed to read cli_update_required error contract: %v", err) + } + + var contract cliUpdateRequiredErrorContractFile + if err := json.Unmarshal(data, &contract); err != nil { + t.Fatalf("failed to unmarshal cli_update_required error contract: %v", err) + } + if len(contract.ErrorData) == 0 { + t.Fatal("cli_update_required error contract must include errorData") + } + return contract +} + +func findRepoRelativeFile(t *testing.T, relativePath string) string { + t.Helper() + + directory, err := os.Getwd() + if err != nil { + t.Fatalf("failed to resolve current directory: %v", err) + } + + for { + candidate := filepath.Join(directory, relativePath) + if _, err := os.Stat(candidate); err == nil { + return candidate + } + + parent := filepath.Dir(directory) + if parent == directory { + t.Fatalf("failed to find %s from %s", relativePath, directory) + } + directory = parent + } +} diff --git a/cli/common/project/endpoint_contract_test.go b/cli/common/project/endpoint_contract_test.go new file mode 100644 index 0000000000..e5ecf28131 --- /dev/null +++ b/cli/common/project/endpoint_contract_test.go @@ -0,0 +1,100 @@ +package project + +import ( + "encoding/json" + "os" + "path/filepath" + "runtime" + "testing" +) + +const endpointContractPath = "tests/contracts/endpoint_contract.json" + +type endpointContractFile struct { + Comment string `json:"comment"` + Cases []endpointContractCase `json:"cases"` +} + +type endpointContractCase struct { + ID string `json:"id"` + CanonicalProjectRoot string `json:"canonicalProjectRoot"` + TrimOnlyEquivalentRoots []string `json:"trimOnlyEquivalentRoots"` + UnixSocketPath string `json:"unixSocketPath"` + WindowsPipePath string `json:"windowsPipePath"` +} + +// Verifies Go CreateEndpoint matches the shared endpoint contract for already-canonicalized roots. +func TestCreateEndpointMatchesSharedContract(t *testing.T) { + contract := readEndpointContract(t) + for _, contractCase := range contract.Cases { + contractCase := contractCase + t.Run(contractCase.ID, func(t *testing.T) { + assertEndpointMatchesContract(t, contractCase.CanonicalProjectRoot, contractCase) + for _, equivalentRoot := range contractCase.TrimOnlyEquivalentRoots { + trimmed := trimTrailingSeparators(equivalentRoot) + if trimmed != contractCase.CanonicalProjectRoot { + t.Fatalf( + "trimOnlyEquivalentRoot %q should trim to canonical %q, got %q", + equivalentRoot, + contractCase.CanonicalProjectRoot, + trimmed) + } + assertEndpointMatchesContract(t, trimmed, contractCase) + } + }) + } +} + +func assertEndpointMatchesContract(t *testing.T, projectRoot string, contractCase endpointContractCase) { + t.Helper() + + endpoint := CreateEndpoint(projectRoot) + expected := contractCase.UnixSocketPath + if runtime.GOOS == "windows" { + expected = contractCase.WindowsPipePath + } + if endpoint.Address != expected { + t.Fatalf("endpoint mismatch for %q\nexpected: %s\nactual: %s", projectRoot, expected, endpoint.Address) + } +} + +func readEndpointContract(t *testing.T) endpointContractFile { + t.Helper() + + path := findRepoRelativeFile(t, endpointContractPath) + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("failed to read shared endpoint contract: %v", err) + } + + var contract endpointContractFile + if err := json.Unmarshal(data, &contract); err != nil { + t.Fatalf("failed to unmarshal shared endpoint contract: %v", err) + } + if len(contract.Cases) == 0 { + t.Fatal("endpoint contract must contain at least one case") + } + return contract +} + +func findRepoRelativeFile(t *testing.T, relativePath string) string { + t.Helper() + + directory, err := os.Getwd() + if err != nil { + t.Fatalf("failed to resolve current directory: %v", err) + } + + for { + candidate := filepath.Join(directory, relativePath) + if _, err := os.Stat(candidate); err == nil { + return candidate + } + + parent := filepath.Dir(directory) + if parent == directory { + t.Fatalf("failed to find %s from %s", relativePath, directory) + } + directory = parent + } +} diff --git a/tests/contracts/cli_update_required_error_contract.json b/tests/contracts/cli_update_required_error_contract.json new file mode 100644 index 0000000000..23028c8e9d --- /dev/null +++ b/tests/contracts/cli_update_required_error_contract.json @@ -0,0 +1,12 @@ +{ + "comment": "This frame must remain backward compatible forever — old CLIs parse it to learn they must update.", + "errorData": { + "type": "cli_update_required", + "message": "Install matching uloop CLI and Unity package versions, then retry the original command.", + "currentCliVersion": "3.0.0-beta.5", + "currentProtocolVersion": 1, + "requiredProtocolVersion": 3, + "updateCommand": "uloop update", + "retryableAfterUpdate": true + } +} diff --git a/tests/contracts/endpoint_contract.json b/tests/contracts/endpoint_contract.json new file mode 100644 index 0000000000..ef91b80ac7 --- /dev/null +++ b/tests/contracts/endpoint_contract.json @@ -0,0 +1,30 @@ +{ + "comment": "Maps already-canonicalized project roots to IPC endpoint names. Symlink/realpath resolution (Go filepath.EvalSymlinks / C# realpath) is intentionally out of scope because it depends on the live filesystem. Each canonicalProjectRoot is the post-resolution string that both sides hash. trimOnlyEquivalentRoots are trailing-separator variants that must hash identically after separator trimming only.", + "cases": [ + { + "id": "unix-stable", + "canonicalProjectRoot": "/Users/example/MyProject", + "trimOnlyEquivalentRoots": [ + "/Users/example/MyProject/" + ], + "unixSocketPath": "/tmp/uloop/UnityCliLoop-bebe6bce5923715b.sock", + "windowsPipePath": "\\\\.\\pipe\\uloop-UnityCliLoop-bebe6bce5923715b" + }, + { + "id": "unix-case-sensitive", + "canonicalProjectRoot": "/Users/example/myproject", + "trimOnlyEquivalentRoots": [], + "unixSocketPath": "/tmp/uloop/UnityCliLoop-6b4ae0bfeab42b19.sock", + "windowsPipePath": "\\\\.\\pipe\\uloop-UnityCliLoop-6b4ae0bfeab42b19" + }, + { + "id": "windows-stable", + "canonicalProjectRoot": "C:\\Users\\example\\MyProject", + "trimOnlyEquivalentRoots": [ + "C:\\Users\\example\\MyProject\\" + ], + "unixSocketPath": "/tmp/uloop/UnityCliLoop-4abce6d9d9e51244.sock", + "windowsPipePath": "\\\\.\\pipe\\uloop-UnityCliLoop-4abce6d9d9e51244" + } + ] +} From 1c2f5efe3b161920a1043697711d2490bac15594 Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Tue, 14 Jul 2026 11:27:55 +0900 Subject: [PATCH 3/7] fix: classify empty Unity RPC results via typed NoResponseError (#1775) Co-authored-by: Cursor --- cli/common/errors/transport_errors.go | 10 ++++++++-- cli/common/errors/transport_errors_test.go | 12 +++++++++++- cli/common/unityipc/client.go | 10 +++++++++- cli/dispatcher/shared-inputs-stamp.json | 2 +- cli/project-runner/shared-inputs-stamp.json | 2 +- 5 files changed, 30 insertions(+), 6 deletions(-) diff --git a/cli/common/errors/transport_errors.go b/cli/common/errors/transport_errors.go index 099b1c3ea1..ac9955a3cc 100644 --- a/cli/common/errors/transport_errors.go +++ b/cli/common/errors/transport_errors.go @@ -7,6 +7,8 @@ import ( "os" "strings" "syscall" + + "github.com/hatayama/unity-cli-loop/common/unityipc" ) // Reports whether the error means the transport dropped mid-request. Domain reload @@ -27,10 +29,14 @@ func IsTransportDisconnectError(err error) bool { return true } + var noResponseErr *unityipc.NoResponseError + if errors.As(err, &noResponseErr) { + return true + } + // Fallback for errors that expose no typed cause. message := err.Error() - return message == "UNITY_NO_RESPONSE" || - strings.Contains(message, "EOF") || + return strings.Contains(message, "EOF") || strings.Contains(message, "connection reset") || strings.Contains(message, "broken pipe") || strings.Contains(message, "use of closed network connection") diff --git a/cli/common/errors/transport_errors_test.go b/cli/common/errors/transport_errors_test.go index 35815c95eb..00025b082c 100644 --- a/cli/common/errors/transport_errors_test.go +++ b/cli/common/errors/transport_errors_test.go @@ -7,6 +7,8 @@ import ( "os" "syscall" "testing" + + "github.com/hatayama/unity-cli-loop/common/unityipc" ) // Test support type that exposes a typed cause behind an opaque message, simulating @@ -73,7 +75,6 @@ func TestIsTransportDisconnectErrorRejectsUnrelatedErrors(t *testing.T) { // Verifies the legacy string fallback still classifies errors without a typed cause. func TestIsTransportDisconnectErrorKeepsStringFallback(t *testing.T) { fallbackErrors := []error{ - fmt.Errorf("UNITY_NO_RESPONSE"), fmt.Errorf("read tcp 127.0.0.1:1: connection reset by peer"), } @@ -84,6 +85,15 @@ func TestIsTransportDisconnectErrorKeepsStringFallback(t *testing.T) { } } +// Verifies a wrapped NoResponseError is classified as a disconnect even when the +// outer error message no longer matches the legacy UNITY_NO_RESPONSE sentinel. +func TestIsTransportDisconnectErrorMatchesWrappedNoResponseError(t *testing.T) { + wrapped := fmt.Errorf("rpc failed: %w", &unityipc.NoResponseError{}) + if !IsTransportDisconnectError(wrapped) { + t.Fatalf("wrapped NoResponseError was not classified as disconnect: %v", wrapped) + } +} + // Verifies that final-response timeout classification matches typed deadline errors // and Timeout()-reporting errors instead of relying on the "i/o timeout" message. func TestIsFinalResponseTimeoutErrorMatchesTypedCauses(t *testing.T) { diff --git a/cli/common/unityipc/client.go b/cli/common/unityipc/client.go index 69785e3bc6..e3f86ca3da 100644 --- a/cli/common/unityipc/client.go +++ b/cli/common/unityipc/client.go @@ -118,6 +118,14 @@ func (err *RPCError) Error() string { return fmt.Sprintf("unity error: %s", err.Message) } +// Signals that Unity accepted the request but returned an empty JSON-RPC result. +// Domain-reload recovery treats this as a transport disconnect so status polling can continue. +type NoResponseError struct{} + +func (err *NoResponseError) Error() string { + return "unity returned no RPC result" +} + func NewClient(connection Connection, clientVersion string, options ...ClientOption) *Client { clientOptions := ClientOptions{} for _, option := range options { @@ -375,7 +383,7 @@ func finishRPCResponse( }) } if len(response.Result) == 0 { - return finishOutcomeWithError(outcome, timing, startedAt, fmt.Errorf("UNITY_NO_RESPONSE")) + return finishOutcomeWithError(outcome, timing, startedAt, &NoResponseError{}) } outcome.Result = response.Result diff --git a/cli/dispatcher/shared-inputs-stamp.json b/cli/dispatcher/shared-inputs-stamp.json index 3345afda4f..2a069e1d14 100644 --- a/cli/dispatcher/shared-inputs-stamp.json +++ b/cli/dispatcher/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "fbbc3c34d301331c513155ea2bab2a33a4b5de97" + "sharedInputsHash": "677ac7b5437d18efa8a09b575d0ec2619de4a1fa" } diff --git a/cli/project-runner/shared-inputs-stamp.json b/cli/project-runner/shared-inputs-stamp.json index 7e7d299f1b..38681b53cd 100644 --- a/cli/project-runner/shared-inputs-stamp.json +++ b/cli/project-runner/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "581697442cd42c4467c8a80ab3bce98f403890c6" + "sharedInputsHash": "c25f44bebdff747f69e1e6c653a4d8d9a5fc7c4f" } From 920da27a1deaeea11ff5cc36d6029d10f7c3029f Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Tue, 14 Jul 2026 11:47:53 +0900 Subject: [PATCH 4/7] fix: JSON-RPC serialization errors, ConfigureAwait guard, dead doc ref (#1776) Co-authored-by: Cursor --- ...sponseFactorySerializationFallbackTests.cs | 35 +++ ...eFactorySerializationFallbackTests.cs.meta | 11 + .../RunTestsTestFrameworkResultTests.cs | 2 +- .../Editor/StaticFacadeStateGuardTests.cs | 2 +- .../Compile/CompilationExecutionService.cs | 2 +- .../Compile/CompileController.cs | 4 +- .../Compile/CompileLifecycleWatchdog.cs | 2 +- .../FirstPartyTools/Compile/CompileUseCase.cs | 6 +- .../ExecuteDynamicCode/DynamicCodeServices.cs | 4 +- .../DynamicCompilation/AutoUsingResolver.cs | 4 +- .../CompiledAssemblyBuilder.cs | 10 +- .../DynamicCompilation/DynamicCodeCompiler.cs | 2 +- .../Execution/CommandRunner.cs | 11 +- .../Execution/DynamicCodeExecutionFacade.cs | 6 +- .../DynamicCodeExecutionScheduler.cs | 24 +- .../DynamicCodeExecutionSchedulerHooks.cs | 4 +- .../Execution/DynamicCodeExecutor.cs | 6 +- .../DynamicCodeForegroundWarmupRunner.cs | 4 +- .../ReplayInput/ReplayInputUseCase.cs | 2 +- .../RunTests/RunTestsCancelStopRestore.cs | 8 +- .../RunTests/RunTestsUseCase.cs | 8 +- .../TestFramework/PlayModeTestExecuter.cs | 10 +- .../SimulateKeyboardUseCase.cs | 6 +- .../SimulateMouseInputUseCase.cs | 10 +- .../FirstPartyTools/Watch/WatchTools.cs | 2 +- .../Api/JsonRpcRequestProcessor.cs | 2 - .../Api/JsonRpcResponseFactory.cs | 23 +- .../ConfigureAwaitGuardTests.cs | 132 +++++++++++ .../ConfigureAwaitGuard.cs | 220 ++++++++++++++++++ 29 files changed, 482 insertions(+), 80 deletions(-) create mode 100644 Assets/Tests/Editor/JsonRpcResponseFactorySerializationFallbackTests.cs create mode 100644 Assets/Tests/Editor/JsonRpcResponseFactorySerializationFallbackTests.cs.meta create mode 100644 tests/UnityCliLoop.CodeComplexity.Tests/ConfigureAwaitGuardTests.cs create mode 100644 tools/UnityCliLoop.CodeComplexity/ConfigureAwaitGuard.cs diff --git a/Assets/Tests/Editor/JsonRpcResponseFactorySerializationFallbackTests.cs b/Assets/Tests/Editor/JsonRpcResponseFactorySerializationFallbackTests.cs new file mode 100644 index 0000000000..070792bbe8 --- /dev/null +++ b/Assets/Tests/Editor/JsonRpcResponseFactorySerializationFallbackTests.cs @@ -0,0 +1,35 @@ +using Newtonsoft.Json.Linq; +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.Infrastructure; +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Verifies JSON-RPC success serialization failures become error frames instead of fake success payloads. + /// + public sealed class JsonRpcResponseFactorySerializationFallbackTests + { + [Test] + public void CreateSuccessResponse_WhenResultCannotBeSerialized_ReturnsJsonRpcError() + { + // Verifies serialization failure returns INTERNAL_ERROR with internal_error data instead of a + // success frame that embeds an error-shaped object in result. + string responseJson = JsonRpcResponseFactory.CreateSuccessResponse( + id: 1, + result: new UnserializableToolResponse()); + JObject response = JObject.Parse(responseJson); + + Assert.That(response["result"], Is.Null); + Assert.That(response["error"], Is.Not.Null); + Assert.That(response["error"]!["code"]!.Value(), Is.EqualTo(UnityCliLoopServerConfig.INTERNAL_ERROR_CODE)); + Assert.That(response["error"]!["data"]!["type"]!.Value(), Is.EqualTo(JsonRpcErrorTypes.InternalError)); + } + + private sealed class UnserializableToolResponse : UnityCliLoopToolResponse + { + public string Boom => throw new System.InvalidOperationException("intentional serialization failure"); + } + } +} diff --git a/Assets/Tests/Editor/JsonRpcResponseFactorySerializationFallbackTests.cs.meta b/Assets/Tests/Editor/JsonRpcResponseFactorySerializationFallbackTests.cs.meta new file mode 100644 index 0000000000..b09134a27c --- /dev/null +++ b/Assets/Tests/Editor/JsonRpcResponseFactorySerializationFallbackTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 5e8c1a2b4d6f7093a1c5e7b9d0f2a4c6 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/RunTestsTestFrameworkResultTests.cs b/Assets/Tests/Editor/RunTestsTestFrameworkResultTests.cs index 9efe1345e9..db79e4478a 100644 --- a/Assets/Tests/Editor/RunTestsTestFrameworkResultTests.cs +++ b/Assets/Tests/Editor/RunTestsTestFrameworkResultTests.cs @@ -192,7 +192,7 @@ public void TrySaveFailureXml_WhenExporterThrows_ReturnsNull() TestResultStatus.Failed, 0.1, new List()); - Regex warningPattern = new Regex("^Failed to save NUnit XML result file: "); + Regex warningPattern = new Regex("^Failed to save failure XML result file: "); LogAssert.Expect(LogType.Warning, warningPattern); diff --git a/Assets/Tests/Editor/StaticFacadeStateGuardTests.cs b/Assets/Tests/Editor/StaticFacadeStateGuardTests.cs index f86a5b346e..0ccadc5016 100644 --- a/Assets/Tests/Editor/StaticFacadeStateGuardTests.cs +++ b/Assets/Tests/Editor/StaticFacadeStateGuardTests.cs @@ -334,7 +334,7 @@ public void ExecuteDynamicCodeUseCase_WhenAwaitingWorkflow_DoesNotCaptureEditorS Assert.That(source, Does.Contain("await DynamicCodeMissingReturnRetryPolicy.RetryMissingReturnIfNeeded(\n executionResult,")); Assert.That(source, Does.Contain("cancellationToken).ConfigureAwait(false);")); Assert.That(source, Does.Contain("await _runtime.ExecuteAsync(request, cancellationToken).ConfigureAwait(false);")); - Assert.That(source, Does.Contain("await _runtime.TryExecuteIfIdleAsync(\n request,\n cancellationToken).ConfigureAwait(false);")); + Assert.That(source, Does.Contain("await _runtime.TryExecuteIfIdleAsync(\n request,\n cancellationToken).ConfigureAwait(false);")); Assert.That( retryPolicySource, Does.Contain("await executeRetryAsync(codeWithReturn, ct)\n .ConfigureAwait(false);")); diff --git a/Packages/src/Editor/FirstPartyTools/Compile/CompilationExecutionService.cs b/Packages/src/Editor/FirstPartyTools/Compile/CompilationExecutionService.cs index 1523aff763..d89e208d62 100644 --- a/Packages/src/Editor/FirstPartyTools/Compile/CompilationExecutionService.cs +++ b/Packages/src/Editor/FirstPartyTools/Compile/CompilationExecutionService.cs @@ -47,7 +47,7 @@ public async Task ExecuteCompilationAsync(CompileSchema request, _pendingCompileSessionRepository); compileController.SetResultRecordingContext(CompileResultRecordingContext.Create(request)); compileController.SetExternalSceneChangePolicy(request.ReloadExternalSceneChanges); - return await compileController.TryCompileAsync(request.ForceRecompile, ct); + return await compileController.TryCompileAsync(request.ForceRecompile, ct).ConfigureAwait(false); } } } diff --git a/Packages/src/Editor/FirstPartyTools/Compile/CompileController.cs b/Packages/src/Editor/FirstPartyTools/Compile/CompileController.cs index 219755be51..d9a9f3421f 100644 --- a/Packages/src/Editor/FirstPartyTools/Compile/CompileController.cs +++ b/Packages/src/Editor/FirstPartyTools/Compile/CompileController.cs @@ -105,7 +105,7 @@ public async Task TryCompileAsync(bool forceRecompile, Cancellati { ["requested_force_recompile"] = forceRecompile })); - return await _currentCompileTask.Task; + return await _currentCompileTask.Task.ConfigureAwait(false); } throw new InvalidOperationException("Compilation is in progress, but the task could not be found."); } @@ -180,7 +180,7 @@ public async Task TryCompileAsync(bool forceRecompile, Cancellati StartCompileLifecycleWatchdog(compileTask, ct); compileTaskTransferred = true; - return await compileTask.Task; + return await compileTask.Task.ConfigureAwait(false); } finally { diff --git a/Packages/src/Editor/FirstPartyTools/Compile/CompileLifecycleWatchdog.cs b/Packages/src/Editor/FirstPartyTools/Compile/CompileLifecycleWatchdog.cs index b3fc9c3af5..57efa25237 100644 --- a/Packages/src/Editor/FirstPartyTools/Compile/CompileLifecycleWatchdog.cs +++ b/Packages/src/Editor/FirstPartyTools/Compile/CompileLifecycleWatchdog.cs @@ -102,7 +102,7 @@ internal async Task WatchAsync(CancellationToken ct) } // The delay is not cancelled directly because the watchdog must convert cancellation into cleanup. - await _waitForPollAsync(); + await _waitForPollAsync().ConfigureAwait(false); if (!observedStart) { waitedForStartMs += UnityCliLoopConstants.COMPILE_START_POLL_INTERVAL_MS; diff --git a/Packages/src/Editor/FirstPartyTools/Compile/CompileUseCase.cs b/Packages/src/Editor/FirstPartyTools/Compile/CompileUseCase.cs index fadd26070b..8f88cc03da 100644 --- a/Packages/src/Editor/FirstPartyTools/Compile/CompileUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/Compile/CompileUseCase.cs @@ -103,7 +103,7 @@ public async Task CompileAsync(CompileSchema request, Cancellat BuildCompileLogContext(request), correlationId); preparationService.StopPlayMode(); - bool exited = await WaitForPlayModeExitAsync(ct); + bool exited = await WaitForPlayModeExitAsync(ct).ConfigureAwait(false); VibeLogger.LogInfo( "compile_playmode_exit_observed", exited ? "Play Mode exited before compile." : "Play Mode did not exit before compile.", @@ -151,7 +151,7 @@ public async Task CompileAsync(CompileSchema request, Cancellat // 3. Compilation execution ct.ThrowIfCancellationRequested(); - CompileResult result = await _executeCompilationAsync(request, ct); + CompileResult result = await _executeCompilationAsync(request, ct).ConfigureAwait(false); // 4. Result formatting CompileResponse successResponse = @@ -171,7 +171,7 @@ private async Task WaitForPlayModeExitAsync(CancellationToken ct) while (EditorApplication.isPlaying && waitedMs < MAX_WAIT_MS) { ct.ThrowIfCancellationRequested(); - await TimerDelay.Wait(POLL_INTERVAL_MS, ct); + await TimerDelay.Wait(POLL_INTERVAL_MS, ct).ConfigureAwait(false); waitedMs += POLL_INTERVAL_MS; } diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeServices.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeServices.cs index d30334c922..d008753e1d 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeServices.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeServices.cs @@ -149,8 +149,8 @@ private static Task ChainDrainTask(Task currentDrainTask, Task nextDrainTask) private static async Task ContinueAfterDrainAsync(Task currentDrainTask, Task nextDrainTask) { - await currentDrainTask; - await nextDrainTask; + await currentDrainTask.ConfigureAwait(false); + await nextDrainTask.ConfigureAwait(false); } private static Task ObserveDrainTask(Task drainTask) diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/AutoUsingResolver.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/AutoUsingResolver.cs index 70626a75b5..b1384f73dd 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/AutoUsingResolver.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/AutoUsingResolver.cs @@ -40,7 +40,7 @@ public async Task ResolveAsync( { File.WriteAllText(sourcePath, currentSource); // AssemblyBuilder retries must stay on Unity's main thread because the compiler API is not thread-safe. - CompilerMessage[] messages = await buildFunc(sourcePath, dllPath, mutableReferences, ct); + CompilerMessage[] messages = await buildFunc(sourcePath, dllPath, mutableReferences, ct).ConfigureAwait(false); List unresolvedTypes = ExtractUnresolvedTypes(messages); if (unresolvedTypes.Count == 0) @@ -103,7 +103,7 @@ public async Task ResolveAsync( // Final compilation with all added usings File.WriteAllText(sourcePath, currentSource); - CompilerMessage[] finalMessages = await buildFunc(sourcePath, dllPath, mutableReferences, ct); + CompilerMessage[] finalMessages = await buildFunc(sourcePath, dllPath, mutableReferences, ct).ConfigureAwait(false); return new AutoUsingResult( currentSource, finalMessages, diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/CompiledAssemblyBuilder.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/CompiledAssemblyBuilder.cs index 22320f2524..c60309d8be 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/CompiledAssemblyBuilder.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/CompiledAssemblyBuilder.cs @@ -101,14 +101,14 @@ async Task BuildFunc( cancellationToken, () => canDeleteTempFiles = false, () => canDeleteTempFiles = true, - () => buildCount++); + () => buildCount++).ConfigureAwait(false); compilationBackendKind = backendResult.BackendKind; buildStopwatch.Stop(); buildMilliseconds += buildStopwatch.Elapsed.TotalMilliseconds; return backendResult.CompilerMessages; } - BuildAttemptResult attemptResult = await BuildPreparedCodeAsync(plan.PreparedCode, ct); + BuildAttemptResult attemptResult = await BuildPreparedCodeAsync(plan.PreparedCode, ct).ConfigureAwait(false); bool shouldCacheResult = true; if (ShouldRetryWithoutLiteralHoisting(plan.PreparedCode, attemptResult.Diagnostics)) @@ -117,7 +117,7 @@ async Task BuildFunc( plan.OriginalRequest.Code, plan.NamespaceName, plan.ClassName); - attemptResult = await BuildPreparedCodeAsync(fallbackPreparedCode, ct); + attemptResult = await BuildPreparedCodeAsync(fallbackPreparedCode, ct).ConfigureAwait(false); shouldCacheResult = false; } @@ -160,7 +160,7 @@ async Task BuildPreparedCodeAsync( wrappedCode, initialReferences, BuildFunc, - cancellationToken); + cancellationToken).ConfigureAwait(false); referenceResolutionMilliseconds += autoResult.ReferenceResolutionMilliseconds; wrappedCode = autoResult.UpdatedSource; @@ -183,7 +183,7 @@ async Task BuildPreparedCodeAsync( originalWrappedCode, rollbackReferences, BuildFunc, - cancellationToken); + cancellationToken).ConfigureAwait(false); referenceResolutionMilliseconds += rollbackResult.ReferenceResolutionMilliseconds; CompilerDiagnostics rollbackDiagnostics = CompilerDiagnostics.FromMessages(rollbackResult.Messages); diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeCompiler.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeCompiler.cs index ad9e0cd8ee..b6cdcbb2bc 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeCompiler.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeCompiler.cs @@ -95,7 +95,7 @@ public async Task CompileAsync(CompilationRequest request, Ca } Stopwatch builderTotalStopwatch = Stopwatch.StartNew(); - CompiledAssemblyBuildResult buildResult = await _assemblyBuilder.BuildAsync(plan, ct); + CompiledAssemblyBuildResult buildResult = await _assemblyBuilder.BuildAsync(plan, ct).ConfigureAwait(false); builderTotalStopwatch.Stop(); LastBuildCount = buildResult.BuildCount; diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/CommandRunner.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/CommandRunner.cs index 67267c403d..2af461ed72 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/CommandRunner.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/CommandRunner.cs @@ -66,6 +66,9 @@ public void Cancel() public async Task ExecuteAsync(ExecutionContext context) { string correlationId = UnityCliLoopConstants.GenerateCorrelationId(); + // Why switch first: callers often resume off-thread via ConfigureAwait(false), but + // BeginUndoGroup / EndExecution use Unity Undo APIs that require the main thread. + await MainThreadSwitcher.SwitchToMainThread(context.CancellationToken); if (!TryBeginExecution(out int undoGroup, out CancellationTokenSource executionCancellationTokenSource)) { return CreateErrorResult(UnityCliLoopConstants.ERROR_MESSAGE_EXECUTION_IN_PROGRESS); @@ -76,15 +79,19 @@ public async Task ExecuteAsync(ExecutionContext context) using CancellationTokenSource combinedCts = CreateCombinedCancellationTokenSource( context, executionCancellationTokenSource); - return await ExecuteInternalAsync(context, combinedCts.Token); + ExecutionResult result = await ExecuteInternalAsync(context, combinedCts.Token).ConfigureAwait(false); + await MainThreadSwitcher.SwitchToMainThread(); + return result; } catch (OperationCanceledException) { + await MainThreadSwitcher.SwitchToMainThread(); return CreateCancelledResult(); } catch (Exception ex) { LogExecutionError(ex, correlationId); + await MainThreadSwitcher.SwitchToMainThread(); return CreateErrorResult( ex.Message, @@ -296,7 +303,7 @@ private async Task ExecuteInternalAsync(ExecutionContext contex cancellationToken); object invoked = executeAsyncMethod.Invoke(instance, callArgs); - object awaitedResult = await AwaitableHelper.AwaitIfNeeded(invoked, cancellationToken); + object awaitedResult = await AwaitableHelper.AwaitIfNeeded(invoked, cancellationToken).ConfigureAwait(false); string resultString = awaitedResult?.ToString() ?? ""; return CreateSuccessResult(resultString); diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionFacade.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionFacade.cs index bfe9f1d381..cbd9fc2133 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionFacade.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionFacade.cs @@ -39,7 +39,7 @@ public async Task ExecuteAsync( ExecutionResult result = await _executionScheduler.RunForegroundAsync( innerCancellationToken => ExecuteCoreAsync(request, innerCancellationToken), CreateExecutionInProgressResult, - cancellationToken); + cancellationToken).ConfigureAwait(false); schedulerStopwatch.Stop(); AppendTiming(result, $"[Perf] SchedulerTotal: {schedulerStopwatch.Elapsed.TotalMilliseconds:F1}ms"); return result; @@ -56,7 +56,7 @@ public async Task ExecuteAsync( (bool entered, ExecutionResult result) = await _executionScheduler.TryRunIfIdleAsync( request.YieldToForegroundRequests, innerCancellationToken => ExecuteCoreAsync(request, innerCancellationToken), - cancellationToken); + cancellationToken).ConfigureAwait(false); schedulerStopwatch.Stop(); AppendTiming(result, $"[Perf] SchedulerTotal: {schedulerStopwatch.Elapsed.TotalMilliseconds:F1}ms"); return (entered, result); @@ -76,7 +76,7 @@ private async Task ExecuteCoreAsync( request.ClassName, request.Parameters, cancellationToken, - request.CompileOnly); + request.CompileOnly).ConfigureAwait(false); executorTotalStopwatch.Stop(); if (result.Timings == null) diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionScheduler.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionScheduler.cs index d76b589bbf..eb8d48b3b3 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionScheduler.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionScheduler.cs @@ -67,13 +67,13 @@ public async Task RunForegroundAsync( ThrowIfDisposed(); cancellationToken.ThrowIfCancellationRequested(); - bool entered = await _executionSemaphore.WaitAsync(0, cancellationToken); + bool entered = await _executionSemaphore.WaitAsync(0, cancellationToken).ConfigureAwait(false); if (!entered) { - await _hooks.InvokeAfterBusySemaphoreProbeFailedAsync(); + await _hooks.InvokeAfterBusySemaphoreProbeFailedAsync().ConfigureAwait(false); if (!TryCancelBackgroundPrewarm()) { - entered = await TryAcquireAfterBusyHandoffAsync(cancellationToken); + entered = await TryAcquireAfterBusyHandoffAsync(cancellationToken).ConfigureAwait(false); if (!entered) { return createBusyResult(); @@ -83,7 +83,7 @@ public async Task RunForegroundAsync( { entered = await TryAcquireAfterBusyHandoffAsync( cancellationToken, - _cancelledPrewarmHandoffWindowMilliseconds); + _cancelledPrewarmHandoffWindowMilliseconds).ConfigureAwait(false); if (!entered) { return createBusyResult(); @@ -99,7 +99,7 @@ public async Task RunForegroundAsync( executionCancellationTokenSource = CreateExecutionCancellationTokenSource(cancellationToken); SetExecutionState(false, executionCancellationTokenSource); - return await action(executionCancellationTokenSource.Token); + return await action(executionCancellationTokenSource.Token).ConfigureAwait(false); } finally { @@ -117,7 +117,7 @@ public async Task RunForegroundAsync( if (!yieldToForegroundRequests) { - return await TryRunWithoutForegroundYieldAsync(action, cancellationToken); + return await TryRunWithoutForegroundYieldAsync(action, cancellationToken).ConfigureAwait(false); } bool entered; @@ -149,9 +149,9 @@ public async Task RunForegroundAsync( try { - await _hooks.InvokeAfterBackgroundExecutionStatePublishedAsync(); + await _hooks.InvokeAfterBackgroundExecutionStatePublishedAsync().ConfigureAwait(false); _hooks.AfterSemaphoreEntered?.Invoke(); - T result = await action(executionCancellationTokenSource.Token); + T result = await action(executionCancellationTokenSource.Token).ConfigureAwait(false); return (true, result); } finally @@ -209,7 +209,7 @@ public async Task ShutdownAsync() TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default); - Task completedTask = await Task.WhenAny(_shutdownCompletionSource.Task, delayTask); + Task completedTask = await Task.WhenAny(_shutdownCompletionSource.Task, delayTask).ConfigureAwait(false); if (completedTask == _shutdownCompletionSource.Task) { timeoutCancellationTokenSource.Cancel(); @@ -227,7 +227,7 @@ public async Task ShutdownAsync() Func> action, CancellationToken cancellationToken) { - bool entered = await _executionSemaphore.WaitAsync(0, cancellationToken); + bool entered = await _executionSemaphore.WaitAsync(0, cancellationToken).ConfigureAwait(false); if (!entered) { return (false, default); @@ -241,7 +241,7 @@ public async Task ShutdownAsync() executionCancellationTokenSource = CreateExecutionCancellationTokenSource(cancellationToken); SetExecutionState(false, executionCancellationTokenSource); - T result = await action(executionCancellationTokenSource.Token); + T result = await action(executionCancellationTokenSource.Token).ConfigureAwait(false); return (true, result); } finally @@ -302,7 +302,7 @@ private async Task TryAcquireAfterBusyHandoffAsync( try { - await _executionSemaphore.WaitAsync(handoffCancellationTokenSource.Token); + await _executionSemaphore.WaitAsync(handoffCancellationTokenSource.Token).ConfigureAwait(false); return true; } catch (OperationCanceledException) diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionSchedulerHooks.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionSchedulerHooks.cs index 4c5becd9e1..19bb0c8253 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionSchedulerHooks.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutionSchedulerHooks.cs @@ -38,7 +38,7 @@ public async Task InvokeAfterBackgroundExecutionStatePublishedAsync() return; } - await AfterBackgroundExecutionStatePublishedAsync(); + await AfterBackgroundExecutionStatePublishedAsync().ConfigureAwait(false); } public async Task InvokeAfterBusySemaphoreProbeFailedAsync() @@ -48,7 +48,7 @@ public async Task InvokeAfterBusySemaphoreProbeFailedAsync() return; } - await AfterBusySemaphoreProbeFailedAsync(); + await AfterBusySemaphoreProbeFailedAsync().ConfigureAwait(false); } public void InvokeLogWarning(string message) diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutor.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutor.cs index 33ca3185a2..a6de8cbf92 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutor.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeExecutor.cs @@ -56,7 +56,7 @@ public async Task ExecuteCodeAsync( className); sourcePreparationStopwatch.Stop(); Stopwatch compileTotalStopwatch = Stopwatch.StartNew(); - CompilationResult compilationResult = await CompileCodeAsync(code, className, cancellationToken); + CompilationResult compilationResult = await CompileCodeAsync(code, className, cancellationToken).ConfigureAwait(false); compileTotalStopwatch.Stop(); ExecutionResult compilationFailureResult = TryCreateCompilationFailureResult( @@ -88,7 +88,7 @@ public async Task ExecuteCodeAsync( }; Stopwatch executionStopwatch = Stopwatch.StartNew(); - ExecutionResult executionResult = await _invoker.ExecuteAsync(context); + ExecutionResult executionResult = await _invoker.ExecuteAsync(context).ConfigureAwait(false); executionStopwatch.Stop(); executionResult.ExecutionTime = totalStopwatch.Elapsed; @@ -155,7 +155,7 @@ private async Task CompileCodeAsync( Namespace = DynamicCodeConstants.DEFAULT_NAMESPACE }; - CompilationResult result = await _compiler.CompileAsync(request, ct); + CompilationResult result = await _compiler.CompileAsync(request, ct).ConfigureAwait(false); if (!result.Success) { lock (_statsLock) diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeForegroundWarmupRunner.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeForegroundWarmupRunner.cs index 68f74ead0a..6ae35352a4 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeForegroundWarmupRunner.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/DynamicCodeForegroundWarmupRunner.cs @@ -22,7 +22,7 @@ internal static async Task RunForegroundSequenceAsync( DynamicCodeExecutionRequest request = CreateRequest( warmupCode, yieldToForegroundRequests); - ExecutionResult result = await runtime.ExecuteAsync(request, ct); + ExecutionResult result = await runtime.ExecuteAsync(request, ct).ConfigureAwait(false); if (!result.Success) { return false; @@ -46,7 +46,7 @@ internal static async Task TryRunBackgroundSequenceAsync( DynamicCodeExecutionRequest request = CreateRequest( warmupCode, yieldToForegroundRequests); - (bool entered, ExecutionResult result) = await runtime.TryExecuteIfIdleAsync(request, ct); + (bool entered, ExecutionResult result) = await runtime.TryExecuteIfIdleAsync(request, ct).ConfigureAwait(false); if (!entered || !result.Success) { return false; diff --git a/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputUseCase.cs b/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputUseCase.cs index 829c485450..9e3ce27179 100644 --- a/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/ReplayInput/ReplayInputUseCase.cs @@ -86,7 +86,7 @@ public async Task ReplayInputAsync( correlationId: correlationId ); - await Task.CompletedTask; + await Task.CompletedTask.ConfigureAwait(false); return response; #endif } diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsCancelStopRestore.cs b/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsCancelStopRestore.cs index 0df8214281..dc33115457 100644 --- a/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsCancelStopRestore.cs +++ b/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsCancelStopRestore.cs @@ -108,7 +108,7 @@ public static async Task StopAndRestoreAsync( runGuid, hooks, stopWaitTimeoutMilliseconds, - pollIntervalMilliseconds); + pollIntervalMilliseconds).ConfigureAwait(false); } catch (Exception exception) { @@ -172,7 +172,7 @@ private static async Task StopAndRestoreCoreAsy () => !hooks.IsPlaying(), hooks.DelayAsync, stopWaitTimeoutMilliseconds, - pollIntervalMilliseconds); + pollIntervalMilliseconds).ConfigureAwait(false); playModeExitConfirmed = confirmed; stopWaitTimedOut = timedOut; } @@ -182,7 +182,7 @@ private static async Task StopAndRestoreCoreAsy () => !hooks.IsRunActive(), hooks.DelayAsync, stopWaitTimeoutMilliseconds, - pollIntervalMilliseconds); + pollIntervalMilliseconds).ConfigureAwait(false); stopWaitTimedOut = timedOut; if (!confirmed) { @@ -221,7 +221,7 @@ private static async Task StopAndRestoreCoreAsy return (true, false); } - await delayAsync(pollIntervalMilliseconds, CancellationToken.None); + await delayAsync(pollIntervalMilliseconds, CancellationToken.None).ConfigureAwait(false); elapsedMilliseconds += pollIntervalMilliseconds; } diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsUseCase.cs b/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsUseCase.cs index 983b0380b1..2a04be8360 100644 --- a/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsUseCase.cs @@ -114,17 +114,17 @@ public async Task ExecuteAsync(RunTestsSchema parameters, Canc { if (parameters.TestMode == UnityCliLoopTestMode.PlayMode) { - result = await _executionService.ExecutePlayModeTestAsync(filter, executionCt); + result = await _executionService.ExecutePlayModeTestAsync(filter, executionCt).ConfigureAwait(false); } else { - result = await _executionService.ExecuteEditModeTestAsync(filter, executionCt); + result = await _executionService.ExecuteEditModeTestAsync(filter, executionCt).ConfigureAwait(false); } // Why parent ct (not executionCt): CancelAfter only guards the RunFinished wait. // Using the linked token here would mis-report a successful run as timed out when // the fixed cleanup delay straddles the deadline after RunFinished already arrived. - await _waitForTestRunnerCleanupAsync(ct); + await _waitForTestRunnerCleanupAsync(ct).ConfigureAwait(false); } catch (RunTestsExecutionCanceledException canceledException) when (RunTestsExecutionTimeout.IsTimeoutCancellation(ct)) @@ -207,7 +207,7 @@ private static async Task WaitForTestRunnerCleanupAsync(CancellationToken ct) { // Why: Unity Test Framework exposes the real active-run signal only through internal API, // while the public RunFinished callback fires before cleanup tasks such as RestoreSceneSetupTask. - await TimerDelay.Wait(TestRunnerCleanupFallbackDelayMilliseconds, ct); + await TimerDelay.Wait(TestRunnerCleanupFallbackDelayMilliseconds, ct).ConfigureAwait(false); } } } diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/TestFramework/PlayModeTestExecuter.cs b/Packages/src/Editor/FirstPartyTools/RunTests/TestFramework/PlayModeTestExecuter.cs index c6e43c43b1..1119e0c1cb 100644 --- a/Packages/src/Editor/FirstPartyTools/RunTests/TestFramework/PlayModeTestExecuter.cs +++ b/Packages/src/Editor/FirstPartyTools/RunTests/TestFramework/PlayModeTestExecuter.cs @@ -52,14 +52,14 @@ public static async Task ExecutePlayModeTest( TestMode.PlayMode, filter, ct, - startedRunGuid => runGuid = startedRunGuid); + startedRunGuid => runGuid = startedRunGuid).ConfigureAwait(false); } catch (OperationCanceledException originalException) { RunTestsCancelStopRestoreResult stopResult = await RunTestsCancelStopRestore.StopAndRestoreAsync( isPlayMode: true, runGuid: runGuid, - RunTestsCancelStopRestoreUnityHooks.Resolve()); + RunTestsCancelStopRestoreUnityHooks.Resolve()).ConfigureAwait(false); throw new RunTestsExecutionCanceledException(originalException.CancellationToken, stopResult); } finally @@ -80,14 +80,14 @@ public static async Task ExecuteEditModeTest( TestMode.EditMode, filter, ct, - startedRunGuid => runGuid = startedRunGuid); + startedRunGuid => runGuid = startedRunGuid).ConfigureAwait(false); } catch (OperationCanceledException originalException) { RunTestsCancelStopRestoreResult stopResult = await RunTestsCancelStopRestore.StopAndRestoreAsync( isPlayMode: false, runGuid: runGuid, - RunTestsCancelStopRestoreUnityHooks.Resolve()); + RunTestsCancelStopRestoreUnityHooks.Resolve()).ConfigureAwait(false); throw new RunTestsExecutionCanceledException(originalException.CancellationToken, stopResult); } } @@ -123,7 +123,7 @@ private static async Task ExecuteTestWithEventNotificati // keeping the TestRunnerApi callback subscription alive forever. using CancellationTokenRegistration cancellationRegistration = ct.Register(() => taskCompletionSource.TrySetCanceled(ct)); - SerializableTestResult result = await taskCompletionSource.Task; + SerializableTestResult result = await taskCompletionSource.Task.ConfigureAwait(false); ct.ThrowIfCancellationRequested(); return result; } diff --git a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs index 94531d9db9..3c539fe5ad 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateKeyboard/SimulateKeyboardUseCase.cs @@ -117,15 +117,15 @@ public async Task ExecuteAsync( keyboard, key, parameters.Duration, - ct); + ct).ConfigureAwait(false); break; case UnityCliLoopKeyboardAction.KeyDown: - response = await KeyboardInputActionExecutor.ExecuteKeyDown(keyboard, key, ct); + response = await KeyboardInputActionExecutor.ExecuteKeyDown(keyboard, key, ct).ConfigureAwait(false); break; case UnityCliLoopKeyboardAction.KeyUp: - response = await KeyboardInputActionExecutor.ExecuteKeyUp(keyboard, key, ct); + response = await KeyboardInputActionExecutor.ExecuteKeyUp(keyboard, key, ct).ConfigureAwait(false); break; default: diff --git a/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/SimulateMouseInputUseCase.cs b/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/SimulateMouseInputUseCase.cs index ae245509a5..ef3de4fe4a 100644 --- a/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/SimulateMouseInputUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/SimulateMouseInput/SimulateMouseInputUseCase.cs @@ -106,23 +106,23 @@ public async Task ExecuteAsync( switch (parameters.Action) { case UnityCliLoopMouseInputAction.Click: - response = await MouseInputPressActionExecutor.ExecuteClick(mouse, parameters, ct); + response = await MouseInputPressActionExecutor.ExecuteClick(mouse, parameters, ct).ConfigureAwait(false); break; case UnityCliLoopMouseInputAction.LongPress: - response = await MouseInputPressActionExecutor.ExecuteLongPress(mouse, parameters, ct); + response = await MouseInputPressActionExecutor.ExecuteLongPress(mouse, parameters, ct).ConfigureAwait(false); break; case UnityCliLoopMouseInputAction.MoveDelta: - response = await MouseInputMotionActionExecutor.ExecuteMoveDelta(mouse, parameters, ct); + response = await MouseInputMotionActionExecutor.ExecuteMoveDelta(mouse, parameters, ct).ConfigureAwait(false); break; case UnityCliLoopMouseInputAction.Scroll: - response = await MouseInputMotionActionExecutor.ExecuteScroll(mouse, parameters, ct); + response = await MouseInputMotionActionExecutor.ExecuteScroll(mouse, parameters, ct).ConfigureAwait(false); break; case UnityCliLoopMouseInputAction.SmoothDelta: - response = await MouseInputMotionActionExecutor.ExecuteSmoothDelta(mouse, parameters, ct); + response = await MouseInputMotionActionExecutor.ExecuteSmoothDelta(mouse, parameters, ct).ConfigureAwait(false); break; default: diff --git a/Packages/src/Editor/FirstPartyTools/Watch/WatchTools.cs b/Packages/src/Editor/FirstPartyTools/Watch/WatchTools.cs index fefd567996..fba4ee0a58 100644 --- a/Packages/src/Editor/FirstPartyTools/Watch/WatchTools.cs +++ b/Packages/src/Editor/FirstPartyTools/Watch/WatchTools.cs @@ -189,7 +189,7 @@ public static async Task EnableAsync(EnableWatchSchema parameters } WatchCompilationResult compilationResult = await WatchExpressionServices.Compiler - .CompileAsync(parameters.Expression, ct); + .CompileAsync(parameters.Expression, ct).ConfigureAwait(false); if (!compilationResult.Success) { return CreateCompilationFailure(compilationResult); diff --git a/Packages/src/Editor/Infrastructure/Api/JsonRpcRequestProcessor.cs b/Packages/src/Editor/Infrastructure/Api/JsonRpcRequestProcessor.cs index f165f2788a..fb50105b3e 100644 --- a/Packages/src/Editor/Infrastructure/Api/JsonRpcRequestProcessor.cs +++ b/Packages/src/Editor/Infrastructure/Api/JsonRpcRequestProcessor.cs @@ -14,8 +14,6 @@ namespace io.github.hatayama.UnityCliLoop.Infrastructure /// /// Class specialized in handling JSON-RPC 2.0 processing /// - /// Design document reference: Packages/src/Editor/ARCHITECTURE.md - /// /// Related classes: /// - UnityCliLoopExecutionRouter: Executes Unity requests based on JSON-RPC requests /// - Project IPC server: Receives JSON-RPC messages from CLI clients diff --git a/Packages/src/Editor/Infrastructure/Api/JsonRpcResponseFactory.cs b/Packages/src/Editor/Infrastructure/Api/JsonRpcResponseFactory.cs index d7133bb9ea..4cfdcae6b9 100644 --- a/Packages/src/Editor/Infrastructure/Api/JsonRpcResponseFactory.cs +++ b/Packages/src/Editor/Infrastructure/Api/JsonRpcResponseFactory.cs @@ -34,21 +34,20 @@ internal static string CreateSuccessResponse(object id, UnityCliLoopToolResponse ); return JsonConvert.SerializeObject(response, Formatting.None, ResponseSerializerSettings); } - catch (Exception) + catch (Exception ex) { - // Return safe fallback response for any serialization errors - object fallbackResult = new - { - error = "Serialization failed - returning safe fallback", - commandType = result != null ? result.GetType().Name : "unknown" - }; - - JsonRpcSuccessResponse fallbackResponse = new( + // Why error frame instead of success+error object: clients treat jsonrpc success as + // completed tool results; a fake success would hide serialization failure from agents. + string commandType = result != null ? result.GetType().Name : "unknown"; + JsonRpcErrorResponse errorResponse = new( UnityCliLoopServerConfig.JSONRPC_VERSION, id, - fallbackResult - ); - return JsonConvert.SerializeObject(fallbackResponse, Formatting.None, ResponseSerializerSettings); + new JsonRpcError( + UnityCliLoopServerConfig.INTERNAL_ERROR_CODE, + "Failed to serialize the tool response.", + new InternalErrorData( + $"Serialization failed for response type '{commandType}': {ex.Message}"))); + return JsonConvert.SerializeObject(errorResponse, Formatting.None, ResponseSerializerSettings); } } diff --git a/tests/UnityCliLoop.CodeComplexity.Tests/ConfigureAwaitGuardTests.cs b/tests/UnityCliLoop.CodeComplexity.Tests/ConfigureAwaitGuardTests.cs new file mode 100644 index 0000000000..e6ed58f24a --- /dev/null +++ b/tests/UnityCliLoop.CodeComplexity.Tests/ConfigureAwaitGuardTests.cs @@ -0,0 +1,132 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Text; +using NUnit.Framework; +using UnityCliLoop.CodeComplexity; + +namespace UnityCliLoop.CodeComplexity.Tests +{ + [TestFixture] + public sealed class ConfigureAwaitGuardTests + { + // Verifies Task-like awaits without ConfigureAwait(false) are reported while custom + // awaitables (intentional main-thread hops) are excluded by return type. + [Test] + public void FindMissingConfigureAwait_WhenSampleHasTaskAwaitWithoutConfigureAwait_ReportsViolation() + { + string rootPath = Path.Combine( + TestContext.CurrentContext.WorkDirectory, + $"configure-await-{Guid.NewGuid():N}"); + Directory.CreateDirectory(rootPath); + try + { + string toolsDir = Path.Combine(rootPath, "Packages", "src", "Editor", "FirstPartyTools", "Sample"); + string contractsDir = Path.Combine(rootPath, "Packages", "src", "Editor", "ToolContracts"); + Directory.CreateDirectory(toolsDir); + Directory.CreateDirectory(contractsDir); + File.WriteAllText( + Path.Combine(contractsDir, "MainThreadSwitcher.cs"), + """ + using System; + using System.Runtime.CompilerServices; + using System.Threading; + namespace io.github.hatayama.UnityCliLoop.ToolContracts + { + public static class MainThreadSwitcher + { + public static SwitchToMainThreadAwaitable SwitchToMainThread(CancellationToken ct = default) + => new SwitchToMainThreadAwaitable(ct); + } + public struct SwitchToMainThreadAwaitable + { + public SwitchToMainThreadAwaitable(CancellationToken cancellationToken) { } + public Awaiter GetAwaiter() => new Awaiter(); + public struct Awaiter : INotifyCompletion + { + public bool IsCompleted => true; + public void GetResult() { } + public void OnCompleted(Action continuation) => continuation(); + } + } + } + """); + File.WriteAllText( + Path.Combine(toolsDir, "SampleTool.cs"), + """ + using System.Threading.Tasks; + using io.github.hatayama.UnityCliLoop.ToolContracts; + namespace Sample + { + public static class SampleTool + { + public static async Task RunAsync() + { + await Task.Delay(1); + await MainThreadSwitcher.SwitchToMainThread(); + await Task.CompletedTask; + } + } + } + """); + + ConfigureAwaitGuard guard = new(); + IReadOnlyList violations = guard.FindMissingConfigureAwait(rootPath); + + Assert.That(violations.Count, Is.EqualTo(2)); + Assert.That(violations.All(violation => + violation.AwaitExpression.Contains("Task.Delay", StringComparison.Ordinal) + || violation.AwaitExpression.Contains("Task.CompletedTask", StringComparison.Ordinal)), Is.True); + Assert.That(violations.Any(violation => + violation.AwaitExpression.Contains("SwitchToMainThread", StringComparison.Ordinal)), Is.False); + } + finally + { + Directory.Delete(rootPath, recursive: true); + } + } + + // Verifies FirstPartyTools Task-like awaits all use ConfigureAwait(false) so paused + // Play Mode cannot hang tool continuations captured on Unity's SynchronizationContext. + [Test] + public void FindMissingConfigureAwait_ForRepositoryFirstPartyTools_HasNoViolations() + { + string repositoryRoot = FindRepositoryRoot(); + ConfigureAwaitGuard guard = new(); + IReadOnlyList violations = guard.FindMissingConfigureAwait(repositoryRoot); + + if (violations.Count == 0) + { + return; + } + + StringBuilder message = new(); + message.AppendLine($"Found {violations.Count} Task-like await(s) missing ConfigureAwait(false):"); + foreach (ConfigureAwaitViolation violation in violations.Take(40)) + { + message.AppendLine($"{violation.FilePath}:{violation.Line}: {violation.AwaitExpression}"); + } + + Assert.Fail(message.ToString()); + } + + private static string FindRepositoryRoot() + { + string? directory = TestContext.CurrentContext.TestDirectory; + while (!string.IsNullOrEmpty(directory)) + { + if (File.Exists(Path.Combine(directory, "Packages", "src", "package.json")) + || File.Exists(Path.Combine(directory, "Packages", "manifest.json"))) + { + return directory; + } + + directory = Directory.GetParent(directory)?.FullName; + } + + Assert.Fail("Failed to locate repository root from test directory."); + return string.Empty; + } + } +} diff --git a/tools/UnityCliLoop.CodeComplexity/ConfigureAwaitGuard.cs b/tools/UnityCliLoop.CodeComplexity/ConfigureAwaitGuard.cs new file mode 100644 index 0000000000..c4e0f58a29 --- /dev/null +++ b/tools/UnityCliLoop.CodeComplexity/ConfigureAwaitGuard.cs @@ -0,0 +1,220 @@ +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Diagnostics; +using System.IO; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Text; + +namespace UnityCliLoop.CodeComplexity +{ + /// + /// Finds Task-like awaits in tool assemblies that omit ConfigureAwait(false). + /// + public sealed class ConfigureAwaitGuard + { + private static readonly string[] DefaultPreprocessorSymbols = + { + "UNITY_EDITOR", + "UNITY_EDITOR_OSX", + "UNITY_2022_3_OR_NEWER", + "UNITY_6000_0_OR_NEWER", + "UNITY_6000_3_OR_NEWER", + "UNITY_6000_4_OR_NEWER", + "ULOOP_HAS_INPUT_SYSTEM", + "ULOOP_DEBUG", + "ULOOP_HAS_TEST_FRAMEWORK" + }; + + /// + /// Scans FirstPartyTools for await expressions of Task/Task<T>/ValueTask/ValueTask<T> + /// that are missing ConfigureAwait(false). + /// Custom awaitables (e.g. SwitchToMainThreadAwaitable) are excluded by return type — + /// intentional main-thread hops are the mechanism, not the hazard. + /// + public IReadOnlyList FindMissingConfigureAwait(string repositoryRoot) + { + Debug.Assert(!string.IsNullOrWhiteSpace(repositoryRoot), "repositoryRoot must not be empty"); + + string[] sourceFiles = CollectToolAssemblySourceFiles(repositoryRoot); + if (sourceFiles.Length == 0) + { + return Array.Empty(); + } + + CSharpCompilation compilation = CreateCompilation(sourceFiles); + List violations = new(); + foreach (SyntaxTree syntaxTree in compilation.SyntaxTrees) + { + if (!IsFirstPartyToolsPath(syntaxTree.FilePath, repositoryRoot)) + { + continue; + } + + SemanticModel semanticModel = compilation.GetSemanticModel(syntaxTree); + CompilationUnitSyntax root = syntaxTree.GetCompilationUnitRoot(); + foreach (AwaitExpressionSyntax awaitExpression in root.DescendantNodes().OfType()) + { + if (HasConfigureAwaitFalse(awaitExpression.Expression)) + { + continue; + } + + ITypeSymbol? awaitedType = semanticModel.GetTypeInfo(awaitExpression.Expression).Type; + if (awaitedType == null || !IsTaskLike(awaitedType)) + { + // Why skip non-Task awaitables: SwitchToMainThreadAwaitable and similar + // intentional main-thread hops are the mechanism, not the hazard. + continue; + } + + FileLinePositionSpan lineSpan = awaitExpression.GetLocation().GetLineSpan(); + violations.Add(new ConfigureAwaitViolation( + CreateReportPath(repositoryRoot, syntaxTree.FilePath), + lineSpan.StartLinePosition.Line + 1, + lineSpan.StartLinePosition.Character + 1, + awaitExpression.ToString())); + } + } + + return violations + .OrderBy(violation => violation.FilePath, StringComparer.Ordinal) + .ThenBy(violation => violation.Line) + .ThenBy(violation => violation.Column) + .ToArray(); + } + + private static string[] CollectToolAssemblySourceFiles(string repositoryRoot) + { + string firstPartyToolsPath = Path.Combine(repositoryRoot, "Packages", "src", "Editor", "FirstPartyTools"); + string toolContractsPath = Path.Combine(repositoryRoot, "Packages", "src", "Editor", "ToolContracts"); + return CollectCsFiles(firstPartyToolsPath) + .Concat(CollectCsFiles(toolContractsPath)) + .Distinct(StringComparer.Ordinal) + .OrderBy(path => path, StringComparer.Ordinal) + .ToArray(); + } + + private static IEnumerable CollectCsFiles(string directoryPath) + { + if (!Directory.Exists(directoryPath)) + { + return Array.Empty(); + } + + return Directory.GetFiles(directoryPath, "*.cs", SearchOption.AllDirectories); + } + + private static bool IsFirstPartyToolsPath(string filePath, string repositoryRoot) + { + string relativePath = CreateReportPath(repositoryRoot, filePath); + return relativePath.StartsWith("Packages/src/Editor/FirstPartyTools/", StringComparison.Ordinal); + } + + private static bool HasConfigureAwaitFalse(ExpressionSyntax expression) + { + if (expression is not InvocationExpressionSyntax invocation) + { + return false; + } + + if (invocation.Expression is not MemberAccessExpressionSyntax memberAccess) + { + return false; + } + + if (!string.Equals(memberAccess.Name.Identifier.ValueText, "ConfigureAwait", StringComparison.Ordinal)) + { + return false; + } + + if (invocation.ArgumentList.Arguments.Count != 1) + { + return false; + } + + ExpressionSyntax argumentExpression = invocation.ArgumentList.Arguments[0].Expression; + return argumentExpression is LiteralExpressionSyntax literal + && literal.IsKind(SyntaxKind.FalseLiteralExpression); + } + + private static bool IsTaskLike(ITypeSymbol type) + { + ITypeSymbol unbound = type.OriginalDefinition; + string? fullName = unbound.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat); + return fullName is + "global::System.Threading.Tasks.Task" or + "global::System.Threading.Tasks.Task" or + "global::System.Threading.Tasks.ValueTask" or + "global::System.Threading.Tasks.ValueTask"; + } + + private static CSharpCompilation CreateCompilation(IReadOnlyList sourceFiles) + { + List syntaxTrees = new(); + foreach (string sourceFile in sourceFiles) + { + SourceText sourceText = SourceText.From(File.ReadAllText(sourceFile)); + syntaxTrees.Add(CSharpSyntaxTree.ParseText( + sourceText, + CSharpParseOptions.Default + .WithLanguageVersion(LanguageVersion.Latest) + .WithPreprocessorSymbols(DefaultPreprocessorSymbols), + path: sourceFile)); + } + + return CSharpCompilation.Create( + "UnityCliLoop.ConfigureAwaitGuard.Analysis", + syntaxTrees, + CreateMetadataReferences(), + new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); + } + + private static ImmutableArray CreateMetadataReferences() + { + string? trustedPlatformAssemblies = AppContext.GetData("TRUSTED_PLATFORM_ASSEMBLIES") as string; + if (string.IsNullOrEmpty(trustedPlatformAssemblies)) + { + return ImmutableArray.Empty; + } + + return trustedPlatformAssemblies + .Split(Path.PathSeparator) + .Where(File.Exists) + .Distinct(StringComparer.Ordinal) + .Select(path => (MetadataReference)MetadataReference.CreateFromFile(path)) + .ToImmutableArray(); + } + + private static string CreateReportPath(string rootPath, string filePath) + { + string fullRootPath = Path.GetFullPath(rootPath); + string fullFilePath = Path.GetFullPath(filePath); + return Path.GetRelativePath(fullRootPath, fullFilePath) + .Replace(Path.DirectorySeparatorChar, '/') + .Replace(Path.AltDirectorySeparatorChar, '/'); + } + } + + /// + /// One Task-like await that omitted ConfigureAwait(false). + /// + public sealed class ConfigureAwaitViolation + { + public string FilePath { get; } + public int Line { get; } + public int Column { get; } + public string AwaitExpression { get; } + + public ConfigureAwaitViolation(string filePath, int line, int column, string awaitExpression) + { + FilePath = filePath; + Line = line; + Column = column; + AwaitExpression = awaitExpression; + } + } +} From a9e279800cf0253aca89b7e36c548de90e6254fb Mon Sep 17 00:00:00 2001 From: Masamichi Hatayama Date: Tue, 14 Jul 2026 16:05:51 +0900 Subject: [PATCH 5/7] chore: Harden Settings/Setup async UI and shallow presenter split (#1777) Co-authored-by: Cursor --- ...SettingsPresentationDecisionPolicyTests.cs | 58 ++++ ...ngsPresentationDecisionPolicyTests.cs.meta | 11 + ...nityCliLoopSettingsWindowCliActionTests.cs | 14 +- .../Editor/Domain/CliPathSetupCheckPolicy.cs | 18 ++ .../Domain/CliPathSetupCheckPolicy.cs.meta | 11 + .../Editor/Domain/SkillInstallDialogPolicy.cs | 30 ++ .../Domain/SkillInstallDialogPolicy.cs.meta | 11 + .../Setup/SetupWizardStartupFlow.cs | 4 +- .../Presentation/Setup/SetupWizardWindow.cs | 13 +- .../ThirdPartyToolMigrationWizardWindow.cs | 20 +- .../UnityCliLoopSettingsCliSetupPresenter.cs | 85 ++++- ...ityCliLoopSettingsToolSettingsPresenter.cs | 139 +++++++- .../UnityCliLoopSettingsWindow.cs | 298 +++--------------- 13 files changed, 419 insertions(+), 293 deletions(-) create mode 100644 Assets/Tests/Editor/SettingsPresentationDecisionPolicyTests.cs create mode 100644 Assets/Tests/Editor/SettingsPresentationDecisionPolicyTests.cs.meta create mode 100644 Packages/src/Editor/Domain/CliPathSetupCheckPolicy.cs create mode 100644 Packages/src/Editor/Domain/CliPathSetupCheckPolicy.cs.meta create mode 100644 Packages/src/Editor/Domain/SkillInstallDialogPolicy.cs create mode 100644 Packages/src/Editor/Domain/SkillInstallDialogPolicy.cs.meta diff --git a/Assets/Tests/Editor/SettingsPresentationDecisionPolicyTests.cs b/Assets/Tests/Editor/SettingsPresentationDecisionPolicyTests.cs new file mode 100644 index 0000000000..ba7a41fc5a --- /dev/null +++ b/Assets/Tests/Editor/SettingsPresentationDecisionPolicyTests.cs @@ -0,0 +1,58 @@ +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.Domain; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Verifies UnityEditor-independent Settings decision policies extracted for Presentation Move Method. + /// + public class SettingsPresentationDecisionPolicyTests + { + [TestCase(false, true, true)] + [TestCase(false, false, false)] + [TestCase(true, true, false)] + [TestCase(true, false, false)] + public void CliPathSetupCheckPolicy_ShouldCheck_UsesWindowsAndOwnership( + bool isWindowsEditor, + bool hasPackageOwnedCurrentUserInstall, + bool expected) + { + // Verifies PATH checks stay off for Windows and non package-owned installs. + bool result = CliPathSetupCheckPolicy.ShouldCheck( + isWindowsEditor, + hasPackageOwnedCurrentUserInstall); + + Assert.That(result, Is.EqualTo(expected)); + } + + [Test] + public void SkillInstallDialogPolicy_ShouldShowForInstallableTargets_RequiresAllEligible() + { + // Verifies wizard dialog policy requires every target to be first-install eligible. + SkillSetupTargetInfo[] targets = + { + CreateSkillTarget(SkillInstallState.Missing, false), + CreateSkillTarget(SkillInstallState.Outdated, false) + }; + + bool result = SkillInstallDialogPolicy.ShouldShowForInstallableTargets(targets); + + Assert.That(result, Is.False); + } + + private static SkillSetupTargetInfo CreateSkillTarget( + SkillInstallState installState, + bool hasDifferentLayoutSkills) + { + return new SkillSetupTargetInfo( + "Claude Code", + ".claude", + "--claude", + hasSkillsDirectory: true, + hasExistingSkills: true, + hasDifferentLayoutSkills, + installState); + } + } +} diff --git a/Assets/Tests/Editor/SettingsPresentationDecisionPolicyTests.cs.meta b/Assets/Tests/Editor/SettingsPresentationDecisionPolicyTests.cs.meta new file mode 100644 index 0000000000..6266ff9fd2 --- /dev/null +++ b/Assets/Tests/Editor/SettingsPresentationDecisionPolicyTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: fac85b7f95dfb4a3683922dfaea778f5 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs b/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs index d8b9b093c4..4f8cf14bc7 100644 --- a/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs +++ b/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs @@ -27,7 +27,7 @@ public void ShouldUninstallCliFromPrimaryButton_ReturnsExpectedAction( bool expected) { // Verifies that package-owned installs route to uninstall only when the dispatcher minimum is satisfied. - bool result = UnityCliLoopSettingsWindow.ShouldUninstallCliFromPrimaryButton( + bool result = UnityCliLoopSettingsCliSetupPresenter.ShouldUninstallCliFromPrimaryButton( cliVersion, cliIsDispatcher, canUninstallCli, @@ -45,7 +45,7 @@ public void ShouldRepairCliPathFromPrimaryButton_ReturnsExpectedAction( bool expected) { // Verifies that stale terminal PATH state routes to repair only when CLI replacement is not needed. - bool result = UnityCliLoopSettingsWindow.ShouldRepairCliPathFromPrimaryButton( + bool result = UnityCliLoopSettingsCliSetupPresenter.ShouldRepairCliPathFromPrimaryButton( needsCliPathSetup, needsUpdate); @@ -70,7 +70,7 @@ public void ResolveCliPrimaryButtonAction_ReturnsClickedPrimaryAction( { // Verifies that the Settings window chooses repair only when dispatcher replacement is unnecessary. CliSetupPrimaryAction result = - UnityCliLoopSettingsWindow.ResolveCliPrimaryButtonAction( + UnityCliLoopSettingsCliSetupPresenter.ResolveCliPrimaryButtonAction( needsCliPathSetup, cliVersion, cliIsDispatcher, @@ -94,7 +94,7 @@ public void ResolveExecutableCliPrimaryButtonAction_IgnoresUnsafeStaleActions( { // Verifies that a refreshed Settings state cannot turn a stale click into a destructive action. CliSetupPrimaryAction result = - UnityCliLoopSettingsWindow.ResolveExecutableCliPrimaryButtonAction( + UnityCliLoopSettingsCliSetupPresenter.ResolveExecutableCliPrimaryButtonAction( ParseAction(clickedAction), ParseAction(refreshedAction)); @@ -111,7 +111,7 @@ public void ShouldCheckCliPathSetupForPlatform_RequiresPackageOwnedCli( bool expected) { // Verifies that PATH repair only runs for POSIX package-owned current-user installs. - bool result = UnityCliLoopSettingsWindow.ShouldCheckCliPathSetupForPlatform( + bool result = UnityCliLoopSettingsCliSetupPresenter.ShouldCheckCliPathSetupForPlatform( platform, hasPackageOwnedCurrentUserInstall); @@ -131,7 +131,7 @@ public void IsCliUpdateNeeded_UsesDispatcherMinimumVersion( bool expected) { // Verifies that the settings UI updates non-dispatcher or older dispatcher installs. - bool result = UnityCliLoopSettingsWindow.IsCliUpdateNeeded( + bool result = UnityCliLoopSettingsCliSetupPresenter.IsCliUpdateNeeded( cliVersion, cliIsDispatcher, TestMinimumDispatcherVersion); @@ -150,7 +150,7 @@ public void ShouldShowSkillsInstalledDialog_ReturnsExpectedValue( // Verifies that Settings keeps the success dialog for first install only. SkillSetupTargetInfo targetInfo = CreateSkillTarget(installState, hasDifferentLayoutSkills); - bool result = UnityCliLoopSettingsWindow.ShouldShowSkillsInstalledDialog(targetInfo); + bool result = SkillInstallDialogPolicy.ShouldShowForSelectedTarget(targetInfo); Assert.That(result, Is.EqualTo(expected)); } diff --git a/Packages/src/Editor/Domain/CliPathSetupCheckPolicy.cs b/Packages/src/Editor/Domain/CliPathSetupCheckPolicy.cs new file mode 100644 index 0000000000..0de9a24b31 --- /dev/null +++ b/Packages/src/Editor/Domain/CliPathSetupCheckPolicy.cs @@ -0,0 +1,18 @@ +namespace io.github.hatayama.UnityCliLoop.Domain +{ + /// + /// Decides when Settings should run CLI PATH visibility checks. + /// + public static class CliPathSetupCheckPolicy + { + /// + /// PATH repair checks run only for non-Windows package-owned current-user installs. + /// + public static bool ShouldCheck( + bool isWindowsEditor, + bool hasPackageOwnedCurrentUserInstall) + { + return !isWindowsEditor && hasPackageOwnedCurrentUserInstall; + } + } +} diff --git a/Packages/src/Editor/Domain/CliPathSetupCheckPolicy.cs.meta b/Packages/src/Editor/Domain/CliPathSetupCheckPolicy.cs.meta new file mode 100644 index 0000000000..c421e6e9a1 --- /dev/null +++ b/Packages/src/Editor/Domain/CliPathSetupCheckPolicy.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: dbe22adc4dab44338bd089a25d15c8fa +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Domain/SkillInstallDialogPolicy.cs b/Packages/src/Editor/Domain/SkillInstallDialogPolicy.cs new file mode 100644 index 0000000000..2ad5853342 --- /dev/null +++ b/Packages/src/Editor/Domain/SkillInstallDialogPolicy.cs @@ -0,0 +1,30 @@ +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; + +namespace io.github.hatayama.UnityCliLoop.Domain +{ + /// + /// Decides when the skills-installed success dialog should appear after an install attempt. + /// + public static class SkillInstallDialogPolicy + { + /// + /// Returns true when Settings should show the success dialog for a single selected target. + /// + public static bool ShouldShowForSelectedTarget(SkillSetupTargetInfo targetInfo) + { + return targetInfo.InstallState != SkillInstallState.Outdated + && !targetInfo.HasDifferentLayoutSkills; + } + + /// + /// Returns true when Setup Wizard should show the success dialog for all installable targets. + /// + public static bool ShouldShowForInstallableTargets(IEnumerable targets) + { + Debug.Assert(targets != null, "targets must not be null"); + return targets.All(ShouldShowForSelectedTarget); + } + } +} diff --git a/Packages/src/Editor/Domain/SkillInstallDialogPolicy.cs.meta b/Packages/src/Editor/Domain/SkillInstallDialogPolicy.cs.meta new file mode 100644 index 0000000000..1b99a4478b --- /dev/null +++ b/Packages/src/Editor/Domain/SkillInstallDialogPolicy.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: cb88c5b20f2e34bab93527cb64cbcda7 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Presentation/Setup/SetupWizardStartupFlow.cs b/Packages/src/Editor/Presentation/Setup/SetupWizardStartupFlow.cs index d0cf8aaeaa..68c372e24b 100644 --- a/Packages/src/Editor/Presentation/Setup/SetupWizardStartupFlow.cs +++ b/Packages/src/Editor/Presentation/Setup/SetupWizardStartupFlow.cs @@ -154,7 +154,7 @@ internal static bool HasSkillUpdateForSetupWizard(IEnumerable targets) { - Debug.Assert(targets != null, "targets must not be null"); - return targets.All(target => - target.InstallState != SkillInstallState.Outdated - && !target.HasDifferentLayoutSkills); + return SkillInstallDialogPolicy.ShouldShowForInstallableTargets(targets); } internal static bool ShouldUseFirstInstallSkillsUi(string lastSeenSetupWizardVersion) @@ -579,8 +576,9 @@ internal static bool ShouldCheckCliPathSetupForSetupWizard( RuntimePlatform platform, bool hasPackageOwnedCurrentUserInstall) { - return platform != RuntimePlatform.WindowsEditor - && hasPackageOwnedCurrentUserInstall; + return CliPathSetupCheckPolicy.ShouldCheck( + isWindowsEditor: platform == RuntimePlatform.WindowsEditor, + hasPackageOwnedCurrentUserInstall); } private static CliSetupCompatibilityState EvaluateCliSetupCompatibilityForSetupWizard( @@ -762,7 +760,8 @@ private async Task HandleInstallSkillsAsync(CancellationToken ct) : SetupWizardSkillsStepPresenter.FilterInstallableSkillTargets(targets); if (installableTargets.Count == 0) return; - bool shouldShowSkillsInstalledDialog = ShouldShowSkillsInstalledDialog(installableTargets); + bool shouldShowSkillsInstalledDialog = + SkillInstallDialogPolicy.ShouldShowForInstallableTargets(installableTargets); _isInstallingSkills = true; UpdateSkillsStep(true, targets); diff --git a/Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardWindow.cs b/Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardWindow.cs index dc9a8142f9..fbd20b0aca 100644 --- a/Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardWindow.cs +++ b/Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardWindow.cs @@ -379,10 +379,10 @@ private void CreateGUI() InitializeApplicationServices(); _view = ThirdPartyToolMigrationWizardView.Create( rootVisualElement, - RefreshUI, - HandleMigrateThirdPartyTools, + () => RefreshUI().Forget(), + () => HandleMigrateThirdPartyTools().Forget(), HandleMigrationSkillTargetChanged, - HandleToggleMigrationSkill, + () => HandleToggleMigrationSkill().Forget(), Close); _resizer = new ThirdPartyToolMigrationWizardWindowResizer(this, _view.MainScrollView); _resizer.BindSizeUpdates(); @@ -425,10 +425,10 @@ private void ScheduleInitialRefresh(bool shouldStartInitialRefresh) return; } - rootVisualElement.schedule.Execute(RefreshUI).StartingIn(0); + rootVisualElement.schedule.Execute(() => RefreshUI().Forget()).StartingIn(0); } - private async void RefreshUI() + private async Task RefreshUI() { CancellationToken ct = BeginMigrationOperation(); ShowCheckingState(new ThirdPartyToolMigrationProgress(0, 0)); @@ -477,7 +477,7 @@ private async void RefreshUI() ShowMigrationTargetsState(preview.FilePaths); } - private async void HandleMigrateThirdPartyTools() + private async Task HandleMigrateThirdPartyTools() { if (!ConfirmMigrationApply( _pendingMigrationFilePaths.Length, @@ -546,7 +546,7 @@ private async void HandleMigrateThirdPartyTools() CompleteMigrationOperation(ct); if (shouldRefreshAfterInterruptedMigration) { - RefreshUI(); + await RefreshUI(); } } @@ -557,7 +557,7 @@ private async void HandleMigrateThirdPartyTools() if (ShouldRefreshAfterMigration(result)) { - RefreshUI(); + await RefreshUI(); return; } @@ -626,7 +626,7 @@ private void HandleMigrationSkillTargetChanged(SkillsTarget target) RefreshMigrationSkillState(); } - private async void HandleToggleMigrationSkill() + private async Task HandleToggleMigrationSkill() { CancellationToken ct = BeginMigrationSkillOperation(); string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); @@ -767,7 +767,7 @@ private void TryStartInitialRefresh() } ShowCheckingState(new ThirdPartyToolMigrationProgress(0, 0)); - rootVisualElement.schedule.Execute(RefreshUI).StartingIn(0); + rootVisualElement.schedule.Execute(() => RefreshUI().Forget()).StartingIn(0); } private void CompleteMigrationOperation(CancellationToken ct) diff --git a/Packages/src/Editor/Presentation/UnityCliLoopSettingsCliSetupPresenter.cs b/Packages/src/Editor/Presentation/UnityCliLoopSettingsCliSetupPresenter.cs index ae666dc9da..f1a0057cfe 100644 --- a/Packages/src/Editor/Presentation/UnityCliLoopSettingsCliSetupPresenter.cs +++ b/Packages/src/Editor/Presentation/UnityCliLoopSettingsCliSetupPresenter.cs @@ -7,7 +7,7 @@ namespace io.github.hatayama.UnityCliLoop.Presentation { /// - /// Presents the CLI setup section in the Unity CLI Loop settings window. + /// Presents the CLI setup section and owns Settings primary-action mediation decisions. /// internal sealed class UnityCliLoopSettingsCliSetupPresenter { @@ -50,6 +50,89 @@ internal void Update( _view.UpdateCliSetup(cliData); } + internal CliSetupPrimaryAction ResolveCurrentPrimaryButtonAction(bool needsCliPathSetup) + { + string cliVersion = _cliSetupApplicationService.GetCachedCliVersion(); + bool cliIsDispatcher = _cliSetupApplicationService.GetCachedCliIsDispatcher(); + string cliExecutablePath = _cliSetupApplicationService.GetCachedCliExecutablePath(); + bool canUninstallCli = _cliSetupApplicationService.IsPackageOwnedCurrentUserInstallPath( + cliExecutablePath, + UnityEngine.Application.platform); + string requiredCliVersion = _cliSetupApplicationService.GetMinimumRequiredCliVersion(); + return ResolveCliPrimaryButtonAction( + needsCliPathSetup, + cliVersion, + cliIsDispatcher, + canUninstallCli, + requiredCliVersion); + } + + internal static bool ShouldUninstallCliFromPrimaryButton( + string cliVersion, + bool cliIsDispatcher, + bool canUninstallCli, + string requiredCliVersion) + { + bool isCliInstalled = cliVersion != null; + bool needsUpdate = IsCliUpdateNeeded(cliVersion, cliIsDispatcher, requiredCliVersion); + return CliSetupPrimaryActionPolicy.ShouldUninstallCli( + isCliInstalled, + needsUpdate, + canUninstallCli); + } + + internal static CliSetupPrimaryAction ResolveCliPrimaryButtonAction( + bool needsCliPathSetup, + string cliVersion, + bool cliIsDispatcher, + bool canUninstallCli, + string requiredCliVersion) + { + bool needsUpdate = IsCliUpdateNeeded(cliVersion, cliIsDispatcher, requiredCliVersion); + bool isCliInstalled = cliVersion != null; + return CliSetupPrimaryActionPolicy.ResolveSettingsPrimaryAction( + needsCliPathSetup, + needsUpdate, + isCliInstalled, + canUninstallCli); + } + + internal static CliSetupPrimaryAction ResolveExecutableCliPrimaryButtonAction( + CliSetupPrimaryAction clickedAction, + CliSetupPrimaryAction refreshedAction) + { + return CliSetupPrimaryActionPolicy.ResolveExecutableSettingsAction( + clickedAction, + refreshedAction); + } + + internal static bool ShouldRepairCliPathFromPrimaryButton( + bool needsCliPathSetup, + bool needsUpdate) + { + return CliSetupPrimaryActionPolicy.ShouldRepairCliPath(needsCliPathSetup, needsUpdate); + } + + internal static bool ShouldCheckCliPathSetupForPlatform( + RuntimePlatform platform, + bool hasPackageOwnedCurrentUserInstall) + { + return CliPathSetupCheckPolicy.ShouldCheck( + isWindowsEditor: platform == RuntimePlatform.WindowsEditor, + hasPackageOwnedCurrentUserInstall); + } + + internal static bool IsCliUpdateNeeded( + string cliVersion, + bool cliIsDispatcher, + string requiredCliVersion) + { + return CliSetupCompatibility.Evaluate( + cliVersion, + cliIsDispatcher, + requiredCliVersion).NeedsUpdate; + } + private CliSetupData CreateCliSetupData( bool needsCliPathSetup, bool isInstallingCli, diff --git a/Packages/src/Editor/Presentation/UnityCliLoopSettingsToolSettingsPresenter.cs b/Packages/src/Editor/Presentation/UnityCliLoopSettingsToolSettingsPresenter.cs index 477f89d5a3..929b81640b 100644 --- a/Packages/src/Editor/Presentation/UnityCliLoopSettingsToolSettingsPresenter.cs +++ b/Packages/src/Editor/Presentation/UnityCliLoopSettingsToolSettingsPresenter.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using UnityEditor; using UnityEngine; using io.github.hatayama.UnityCliLoop.Application; @@ -7,13 +8,24 @@ namespace io.github.hatayama.UnityCliLoop.Presentation { /// - /// Presents the Tool Settings section in the Unity CLI Loop settings window. + /// Presents the Tool Settings section and owns catalog cache / registry warmup. /// internal sealed class UnityCliLoopSettingsToolSettingsPresenter { + private const double RegistryWarmupInitialDelaySeconds = 0.05; + private const double RegistryWarmupMaxDelaySeconds = 0.8; + private const int RegistryWarmupMaxAttempts = 5; + private readonly UnityCliLoopSettingsWindowUI _view; private readonly ToolSettingsUseCase _toolSettingsUseCase; + private bool _isCatalogDirty = true; + private bool _isRegistryWarmupScheduled; + private double _registryWarmupDueTime; + private int _registryWarmupAttemptCount; + private bool _showToolSettings = true; + private bool _isViewReady; + internal UnityCliLoopSettingsToolSettingsPresenter( UnityCliLoopSettingsWindowUI view, ToolSettingsUseCase toolSettingsUseCase) @@ -26,25 +38,138 @@ internal UnityCliLoopSettingsToolSettingsPresenter( ?? throw new ArgumentNullException(nameof(toolSettingsUseCase)); } + internal void SetViewReady(bool isViewReady) + { + _isViewReady = isViewReady; + if (!isViewReady) + { + CancelRegistryWarmup(); + } + } + + internal void InvalidateCatalog() + { + _isCatalogDirty = true; + } + internal void UpdateHeader(bool showToolSettings) { + _showToolSettings = showToolSettings; ToolSettingsSectionData toolSettingsData = CreateToolSettingsHeaderData(showToolSettings); _view.UpdateToolSettings(toolSettingsData); } - internal ToolSettingsSectionData UpdateCatalog(bool showToolSettings) + internal void RefreshCatalogIfNeeded(bool showToolSettings) + { + _showToolSettings = showToolSettings; + if (!showToolSettings || !_isCatalogDirty || !_isViewReady) + { + return; + } + + RefreshCatalog(showToolSettings); + } + + internal void HandleShowToolSettingsChanged(bool show) + { + UpdateHeader(show); + if (!show) + { + _isCatalogDirty = true; + CancelRegistryWarmup(); + ResetRegistryWarmupAttemptCount(); + return; + } + + RefreshCatalogIfNeeded(show); + } + + internal void CancelRegistryWarmup() + { + if (!_isRegistryWarmupScheduled) + { + return; + } + + EditorApplication.update -= RunRegistryWarmupWhenDue; + _isRegistryWarmupScheduled = false; + } + + internal void ResetRegistryWarmupAttemptCount() + { + _registryWarmupAttemptCount = 0; + } + + private void RefreshCatalog(bool showToolSettings) { ToolSettingsSectionData toolSettingsData = CreateToolSettingsData(showToolSettings); _view.UpdateToolSettings(toolSettingsData); - return toolSettingsData; + + if (UnityCliLoopSettingsWindowRefreshPolicy.ShouldKeepToolSettingsCatalogDirty(toolSettingsData)) + { + if (ScheduleRegistryWarmup()) + { + _isCatalogDirty = true; + return; + } + + _isCatalogDirty = false; + return; + } + + CancelRegistryWarmup(); + ResetRegistryWarmupAttemptCount(); + _isCatalogDirty = false; + } + + private bool ScheduleRegistryWarmup() + { + if (UnityCliLoopSettingsWindowRefreshPolicy.ShouldStartToolSettingsRegistryWarmup( + _isRegistryWarmupScheduled, + _registryWarmupAttemptCount, + RegistryWarmupMaxAttempts)) + { + double delaySeconds = UnityCliLoopSettingsWindowRefreshPolicy.CalculateToolSettingsRegistryWarmupDelaySeconds( + RegistryWarmupInitialDelaySeconds, + RegistryWarmupMaxDelaySeconds, + _registryWarmupAttemptCount); + + _isRegistryWarmupScheduled = true; + _registryWarmupDueTime = EditorApplication.timeSinceStartup + delaySeconds; + _registryWarmupAttemptCount++; + EditorApplication.update += RunRegistryWarmupWhenDue; + return true; + } + + return _isRegistryWarmupScheduled; + } + + private void RunRegistryWarmupWhenDue() + { + if (EditorApplication.timeSinceStartup < _registryWarmupDueTime) + { + return; + } + + CancelRegistryWarmup(); + + if (!_isViewReady || !_showToolSettings) + { + ResetRegistryWarmupAttemptCount(); + return; + } + + _toolSettingsUseCase.WarmupRegistry(); + InvalidateCatalog(); + RefreshCatalogIfNeeded(_showToolSettings); } private static ToolSettingsSectionData CreateToolSettingsHeaderData(bool showToolSettings) { return new ToolSettingsSectionData( showToolSettings, - System.Array.Empty(), - System.Array.Empty(), + Array.Empty(), + Array.Empty(), true, false); } @@ -58,8 +183,8 @@ private ToolSettingsSectionData CreateToolSettingsData(bool showToolSettings) { return new ToolSettingsSectionData( showToolSettings, - System.Array.Empty(), - System.Array.Empty(), + Array.Empty(), + Array.Empty(), false, true); } diff --git a/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs b/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs index 0b4069b1c1..0fec5b81c6 100644 --- a/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs +++ b/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs @@ -19,9 +19,6 @@ public class UnityCliLoopSettingsWindow : EditorWindow { private const bool ForceFlatSkillInstall = true; private const double DeferredInitialRefreshDelaySeconds = 0.05; - private const double ToolSettingsRegistryWarmupInitialDelaySeconds = 0.05; - private const double ToolSettingsRegistryWarmupMaxDelaySeconds = 0.8; - private const int ToolSettingsRegistryWarmupMaxAttempts = 5; private static IUnityCliLoopEditorSettingsPort RegisteredEditorSettingsPort; private static ISessionFlagsRepository RegisteredSessionFlagsRepository; @@ -49,13 +46,9 @@ public class UnityCliLoopSettingsWindow : EditorWindow private bool _isRefreshingVersion; private bool _isRefreshingCliPathSetup; private bool _needsCliPathSetup; - private bool _isToolSettingsCatalogDirty = true; private bool _isDeferredInitialRefreshScheduled; private bool _hasCompletedDeferredInitialRefresh; private double _deferredInitialRefreshDueTime; - private bool _isToolSettingsRegistryWarmupScheduled; - private double _toolSettingsRegistryWarmupDueTime; - private int _toolSettingsRegistryWarmupAttemptCount; private SkillInstallState _selectedTargetInstallState = SkillInstallState.Missing; private CancellationTokenSource _skillInstallStateRefreshCts; @@ -107,9 +100,10 @@ private void OnEnable() private void OnDestroy() { CancelDeferredInitialRefresh(); - CancelToolSettingsRegistryWarmup(); - ResetToolSettingsRegistryWarmupAttemptCount(); + _toolSettingsPresenter?.CancelRegistryWarmup(); + _toolSettingsPresenter?.ResetRegistryWarmupAttemptCount(); CancelSkillInstallStateRefresh(); + _toolSettingsPresenter?.SetViewReady(false); _view?.Dispose(); _view = null; _cliSetupPresenter = null; @@ -129,8 +123,8 @@ private void InitializeAll() InitializeModel(); InitializeEventHandler(); LoadSavedSettings(); - RestoreSessionState(); - HandlePostCompileMode(); + _model.LoadFromSessionState(); + HandlePostCompileMode().Forget(); } private void InitializeModel() @@ -159,14 +153,15 @@ private void InitializeView() _toolSettingsPresenter = new UnityCliLoopSettingsToolSettingsPresenter( _view, _toolSettingsUseCase); + _toolSettingsPresenter.SetViewReady(true); SetupViewCallbacks(); } private void SetupViewCallbacks() { - _view.OnRefreshCliVersion += HandleRefreshCliVersion; - _view.OnInstallCli += HandleInstallCli; - _view.OnInstallSkills += HandleInstallSkills; + _view.OnRefreshCliVersion += () => HandleRefreshCliVersion().Forget(); + _view.OnInstallCli += () => HandleInstallCli().Forget(); + _view.OnInstallSkills += () => HandleInstallSkills().Forget(); _view.OnRefreshSkillsState += HandleRefreshSkillsState; _view.OnSkillsTargetChanged += value => { @@ -196,12 +191,7 @@ private void LoadSavedSettings() _installSkillsFlat = ForceFlatSkillInstall; } - private void RestoreSessionState() - { - _model.LoadFromSessionState(); - } - - private async void HandlePostCompileMode() + private async Task HandlePostCompileMode() { _model.EnablePostCompileMode(); _sessionFlagsRepository.SetShowReconnectingUI(false); @@ -226,11 +216,12 @@ private async void HandlePostCompileMode() private void OnDisable() { CancelDeferredInitialRefresh(); - CancelToolSettingsRegistryWarmup(); - ResetToolSettingsRegistryWarmupAttemptCount(); + _toolSettingsPresenter?.CancelRegistryWarmup(); + _toolSettingsPresenter?.ResetRegistryWarmupAttemptCount(); CancelSkillInstallStateRefresh(); CleanupEventHandler(); - SaveSessionState(); + _model?.SaveToSessionState(); + _toolSettingsPresenter?.SetViewReady(false); _view?.Dispose(); _view = null; _cliSetupPresenter = null; @@ -242,11 +233,6 @@ private void CleanupEventHandler() _eventHandler?.Cleanup(); } - private void SaveSessionState() - { - _model.SaveToSessionState(); - } - private void ScheduleDeferredInitialRefresh() { if (!UnityCliLoopSettingsWindowRefreshPolicy.ShouldScheduleDeferredInitialRefresh( @@ -324,8 +310,8 @@ internal void RefreshAllSections( if (runExpensiveChecks) { - RefreshCliVersionInBackground(); - RefreshCliPathSetupInBackground(); + RefreshCliVersionInBackground().Forget(); + RefreshCliPathSetupInBackground().Forget(); if (refreshSkillInstallState) { RefreshSelectedTargetInstallStateInBackground(); @@ -336,11 +322,11 @@ internal void RefreshAllSections( RefreshToolSettingsHeader(); if (runExpensiveChecks) { - RefreshToolSettingsCatalogIfNeeded(); + _toolSettingsPresenter?.RefreshCatalogIfNeeded(_model.UI.ShowToolSettings); } } - private async void RefreshCliVersionInBackground() + private async Task RefreshCliVersionInBackground() { if (_cliSetupApplicationService.IsCliCheckCompleted()) { @@ -348,12 +334,12 @@ private async void RefreshCliVersionInBackground() } await _cliSetupApplicationService.RefreshCliVersionAsync(CancellationToken.None); - RefreshCliPathSetupInBackground(); + RefreshCliPathSetupInBackground().Forget(); RefreshCliSetupSection(); RefreshSelectedTargetInstallStateInBackground(); } - private async void RefreshCliPathSetupInBackground() + private async Task RefreshCliPathSetupInBackground() { if (_isRefreshingCliPathSetup) { @@ -383,7 +369,7 @@ private async void RefreshCliPathSetupInBackground() } } - private async void HandleRefreshCliVersion() + private async Task HandleRefreshCliVersion() { if (_isRefreshingVersion) { @@ -398,7 +384,7 @@ private async void HandleRefreshCliVersion() Task forceRefresh = _cliSetupApplicationService.ForceRefreshCliVersionAsync(CancellationToken.None); Task minimumDelay = Task.Delay(500); await Task.WhenAll(forceRefresh, minimumDelay); - RefreshCliPathSetupInBackground(); + RefreshCliPathSetupInBackground().Forget(); } finally { @@ -409,7 +395,7 @@ private async void HandleRefreshCliVersion() public void InvalidateToolSettingsCatalog() { - _isToolSettingsCatalogDirty = true; + _toolSettingsPresenter?.InvalidateCatalog(); } private void RefreshToolSettingsHeader() @@ -422,120 +408,10 @@ private void RefreshToolSettingsHeader() _toolSettingsPresenter.UpdateHeader(_model.UI.ShowToolSettings); } - private void RefreshToolSettingsCatalog() - { - if (_toolSettingsPresenter == null) - { - return; - } - - ToolSettingsSectionData toolSettingsData = - _toolSettingsPresenter.UpdateCatalog(_model.UI.ShowToolSettings); - - if (UnityCliLoopSettingsWindowRefreshPolicy.ShouldKeepToolSettingsCatalogDirty(toolSettingsData)) - { - if (ScheduleToolSettingsRegistryWarmup()) - { - _isToolSettingsCatalogDirty = true; - return; - } - - _isToolSettingsCatalogDirty = false; - return; - } - - CancelToolSettingsRegistryWarmup(); - ResetToolSettingsRegistryWarmupAttemptCount(); - _isToolSettingsCatalogDirty = false; - } - - private void RefreshToolSettingsCatalogIfNeeded() - { - if (!_model.UI.ShowToolSettings || !_isToolSettingsCatalogDirty) - { - return; - } - - if (_view == null) - { - return; - } - - RefreshToolSettingsCatalog(); - } - private void UpdateShowToolSettings(bool show) { _model.UpdateShowToolSettings(show); - RefreshToolSettingsHeader(); - - if (!show) - { - _isToolSettingsCatalogDirty = true; - CancelToolSettingsRegistryWarmup(); - ResetToolSettingsRegistryWarmupAttemptCount(); - return; - } - - RefreshToolSettingsCatalogIfNeeded(); - } - - private bool ScheduleToolSettingsRegistryWarmup() - { - if (UnityCliLoopSettingsWindowRefreshPolicy.ShouldStartToolSettingsRegistryWarmup( - _isToolSettingsRegistryWarmupScheduled, - _toolSettingsRegistryWarmupAttemptCount, - ToolSettingsRegistryWarmupMaxAttempts)) - { - double delaySeconds = UnityCliLoopSettingsWindowRefreshPolicy.CalculateToolSettingsRegistryWarmupDelaySeconds( - ToolSettingsRegistryWarmupInitialDelaySeconds, - ToolSettingsRegistryWarmupMaxDelaySeconds, - _toolSettingsRegistryWarmupAttemptCount); - - _isToolSettingsRegistryWarmupScheduled = true; - _toolSettingsRegistryWarmupDueTime = EditorApplication.timeSinceStartup + delaySeconds; - _toolSettingsRegistryWarmupAttemptCount++; - EditorApplication.update += RunToolSettingsRegistryWarmupWhenDue; - return true; - } - - return _isToolSettingsRegistryWarmupScheduled; - } - - private void RunToolSettingsRegistryWarmupWhenDue() - { - if (EditorApplication.timeSinceStartup < _toolSettingsRegistryWarmupDueTime) - { - return; - } - - CancelToolSettingsRegistryWarmup(); - - if (_view == null || !_model.UI.ShowToolSettings) - { - ResetToolSettingsRegistryWarmupAttemptCount(); - return; - } - - _toolSettingsUseCase.WarmupRegistry(); - InvalidateToolSettingsCatalog(); - RefreshToolSettingsCatalogIfNeeded(); - } - - private void CancelToolSettingsRegistryWarmup() - { - if (!_isToolSettingsRegistryWarmupScheduled) - { - return; - } - - EditorApplication.update -= RunToolSettingsRegistryWarmupWhenDue; - _isToolSettingsRegistryWarmupScheduled = false; - } - - private void ResetToolSettingsRegistryWarmupAttemptCount() - { - _toolSettingsRegistryWarmupAttemptCount = 0; + _toolSettingsPresenter?.HandleShowToolSettingsChanged(show); } private void HandleToolToggled(string toolName, bool enabled) @@ -544,10 +420,10 @@ private void HandleToolToggled(string toolName, bool enabled) _view?.UpdateSingleToolToggle(toolName, enabled); // Skill synchronization can touch many files, so defer it to keep UI input responsive. - EditorApplication.delayCall += () => ApplyToolToggleSideEffects(toolName, enabled); + EditorApplication.delayCall += () => ApplyToolToggleSideEffects(toolName, enabled).Forget(); } - private async void ApplyToolToggleSideEffects(string toolName, bool enabled) + private async Task ApplyToolToggleSideEffects(string toolName, bool enabled) { if (!enabled) { @@ -597,18 +473,11 @@ private void RefreshCliSetupSection(bool includeSkillDirectoryChecks = true) private bool ShouldCheckCliPathSetup() { - return ShouldCheckCliPathSetupForPlatform( + return UnityCliLoopSettingsCliSetupPresenter.ShouldCheckCliPathSetupForPlatform( UnityEngine.Application.platform, _cliSetupApplicationService.HasPackageOwnedCurrentUserInstall(UnityEngine.Application.platform)); } - internal static bool ShouldCheckCliPathSetupForPlatform( - RuntimePlatform platform, - bool hasPackageOwnedCurrentUserInstall) - { - return platform != RuntimePlatform.WindowsEditor && hasPackageOwnedCurrentUserInstall; - } - private void RefreshSelectedTargetInstallStateFast() { if (!_cliSetupApplicationService.IsCliInstalled()) @@ -646,10 +515,10 @@ private void RefreshSelectedTargetInstallStateInBackground(bool allowDuringCliRe CancellationTokenSource cts = new(); _skillInstallStateRefreshCts = cts; - RefreshSelectedTargetInstallStateAsync(cts.Token); + RefreshSelectedTargetInstallStateAsync(cts.Token).Forget(); } - private async void RefreshSelectedTargetInstallStateAsync(CancellationToken ct) + private async Task RefreshSelectedTargetInstallStateAsync(CancellationToken ct) { string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); SkillInstallState installState = await Task.Run( @@ -707,15 +576,16 @@ private void CancelSkillInstallStateRefresh() _skillInstallStateRefreshCts = null; } - private async void HandleInstallCli() + private async Task HandleInstallCli() { - CliSetupPrimaryAction clickedAction = GetCurrentCliPrimaryButtonAction(); + CliSetupPrimaryAction clickedAction = _cliSetupPresenter.ResolveCurrentPrimaryButtonAction(_needsCliPathSetup); await RefreshCliPrimaryActionStateAsync(CancellationToken.None); - CliSetupPrimaryAction refreshedAction = GetCurrentCliPrimaryButtonAction(); - CliSetupPrimaryAction executableAction = ResolveExecutableCliPrimaryButtonAction( - clickedAction, - refreshedAction); + CliSetupPrimaryAction refreshedAction = _cliSetupPresenter.ResolveCurrentPrimaryButtonAction(_needsCliPathSetup); + CliSetupPrimaryAction executableAction = + UnityCliLoopSettingsCliSetupPresenter.ResolveExecutableCliPrimaryButtonAction( + clickedAction, + refreshedAction); if (executableAction == CliSetupPrimaryAction.None) { return; @@ -771,69 +641,6 @@ await CliPathSetupPrompt.EnsureVisibleAndShowResultAsync( } } - private CliSetupPrimaryAction GetCurrentCliPrimaryButtonAction() - { - string cliVersion = _cliSetupApplicationService.GetCachedCliVersion(); - bool cliIsDispatcher = _cliSetupApplicationService.GetCachedCliIsDispatcher(); - string cliExecutablePath = _cliSetupApplicationService.GetCachedCliExecutablePath(); - bool canUninstallCli = _cliSetupApplicationService.IsPackageOwnedCurrentUserInstallPath( - cliExecutablePath, - UnityEngine.Application.platform); - string requiredCliVersion = _cliSetupApplicationService.GetMinimumRequiredCliVersion(); - return ResolveCliPrimaryButtonAction( - _needsCliPathSetup, - cliVersion, - cliIsDispatcher, - canUninstallCli, - requiredCliVersion); - } - - internal static bool ShouldUninstallCliFromPrimaryButton( - string cliVersion, - bool cliIsDispatcher, - bool canUninstallCli, - string requiredCliVersion) - { - bool isCliInstalled = cliVersion != null; - bool needsUpdate = IsCliUpdateNeeded(cliVersion, cliIsDispatcher, requiredCliVersion); - return CliSetupPrimaryActionPolicy.ShouldUninstallCli( - isCliInstalled, - needsUpdate, - canUninstallCli); - } - - internal static CliSetupPrimaryAction ResolveCliPrimaryButtonAction( - bool needsCliPathSetup, - string cliVersion, - bool cliIsDispatcher, - bool canUninstallCli, - string requiredCliVersion) - { - bool needsUpdate = IsCliUpdateNeeded(cliVersion, cliIsDispatcher, requiredCliVersion); - bool isCliInstalled = cliVersion != null; - return CliSetupPrimaryActionPolicy.ResolveSettingsPrimaryAction( - needsCliPathSetup, - needsUpdate, - isCliInstalled, - canUninstallCli); - } - - internal static CliSetupPrimaryAction ResolveExecutableCliPrimaryButtonAction( - CliSetupPrimaryAction clickedAction, - CliSetupPrimaryAction refreshedAction) - { - return CliSetupPrimaryActionPolicy.ResolveExecutableSettingsAction( - clickedAction, - refreshedAction); - } - - internal static bool ShouldRepairCliPathFromPrimaryButton( - bool needsCliPathSetup, - bool needsUpdate) - { - return CliSetupPrimaryActionPolicy.ShouldRepairCliPath(needsCliPathSetup, needsUpdate); - } - private async Task RefreshCliPrimaryActionStateAsync(CancellationToken ct) { _isRefreshingVersion = true; @@ -885,34 +692,6 @@ await CliPathSetupPrompt.EnsureVisibleAndShowResultAsync( } } - internal static bool IsCliUpdateNeeded( - string cliVersion, - bool cliIsDispatcher, - string requiredCliVersion) - { - return EvaluateCliSetupCompatibility( - cliVersion, - cliIsDispatcher, - requiredCliVersion).NeedsUpdate; - } - - internal static bool ShouldShowSkillsInstalledDialog(SkillSetupTargetInfo targetInfo) - { - return targetInfo.InstallState != SkillInstallState.Outdated - && !targetInfo.HasDifferentLayoutSkills; - } - - private static CliSetupCompatibilityState EvaluateCliSetupCompatibility( - string cliVersion, - bool cliIsDispatcher, - string requiredCliVersion) - { - return CliSetupCompatibility.Evaluate( - cliVersion, - cliIsDispatcher, - requiredCliVersion); - } - private async Task HandleUninstallCli() { if (!CliUninstallPrompt.ConfirmUninstall()) @@ -944,7 +723,7 @@ private async Task HandleUninstallCli() } } - private async void HandleInstallSkills() + private async Task HandleInstallSkills() { if (!_cliSetupApplicationService.IsCliInstalled()) { @@ -958,7 +737,8 @@ private async void HandleInstallSkills() string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); SkillSetupTargetInfo selectedTargetInfo = GetSelectedTargetInfo(projectRoot, includeFreshnessCheck: true); - bool shouldShowSkillsInstalledDialog = ShouldShowSkillsInstalledDialog(selectedTargetInfo); + bool shouldShowSkillsInstalledDialog = + SkillInstallDialogPolicy.ShouldShowForSelectedTarget(selectedTargetInfo); CancelSkillInstallStateRefresh(); _isInstallingSkills = true; RefreshCliSetupSection(); From c729617c4919379791143923b691631e1464ffe2 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 14 Jul 2026 16:38:01 +0900 Subject: [PATCH 6/7] fix: restore main-thread resumes after ConfigureAwait(false) PR-4's bulk ConfigureAwait(false) left Unity API continuations on worker threads. Switch back before Editor callbacks (watch, compile watchdog, run-tests), keep AssemblyBuilder/play-mode polls on the Editor context, map pre-cancelled CommandRunner switches to CreateCancelledResult, and treat empty cached CLI versions as not installed. Co-authored-by: Cursor --- .../UnityCliLoopSettingsWindowCliActionTests.cs | 2 ++ .../Compile/CompileLifecycleWatchdog.cs | 3 +++ .../Editor/FirstPartyTools/Compile/CompileUseCase.cs | 4 ++-- .../DynamicCompilation/AutoUsingResolver.cs | 6 ++++-- .../ExecuteDynamicCode/Execution/CommandRunner.cs | 12 +++++++++++- .../RunTests/RunTestsCancelStopRestore.cs | 4 ++++ .../FirstPartyTools/RunTests/RunTestsUseCase.cs | 10 +++++----- .../RunTests/TestFramework/PlayModeTestExecuter.cs | 10 ++++++++++ .../src/Editor/FirstPartyTools/Watch/WatchTools.cs | 3 +++ .../UnityCliLoopSettingsCliSetupPresenter.cs | 6 +++--- 10 files changed, 47 insertions(+), 13 deletions(-) diff --git a/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs b/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs index 4f8cf14bc7..5c067ad6c4 100644 --- a/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs +++ b/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs @@ -14,6 +14,7 @@ public class UnityCliLoopSettingsWindowCliActionTests private const string TestMinimumDispatcherVersion = "3.0.0"; [TestCase(null, false, true, false)] + [TestCase("", false, true, false)] [TestCase("2.9.0", false, true, false)] [TestCase("3.0.0", false, true, false)] [TestCase("2.9.0", true, true, false)] @@ -61,6 +62,7 @@ public void ShouldRepairCliPathFromPrimaryButton_ReturnsExpectedAction( [TestCase(false, "3.0.0", false, true, "InstallOrUpdate")] [TestCase(false, "3.0.0", true, false, "InstallOrUpdate")] [TestCase(false, null, false, true, "InstallOrUpdate")] + [TestCase(false, "", false, true, "InstallOrUpdate")] public void ResolveCliPrimaryButtonAction_ReturnsClickedPrimaryAction( bool needsCliPathSetup, string cliVersion, diff --git a/Packages/src/Editor/FirstPartyTools/Compile/CompileLifecycleWatchdog.cs b/Packages/src/Editor/FirstPartyTools/Compile/CompileLifecycleWatchdog.cs index 57efa25237..482ac98952 100644 --- a/Packages/src/Editor/FirstPartyTools/Compile/CompileLifecycleWatchdog.cs +++ b/Packages/src/Editor/FirstPartyTools/Compile/CompileLifecycleWatchdog.cs @@ -103,6 +103,9 @@ internal async Task WatchAsync(CancellationToken ct) // The delay is not cancelled directly because the watchdog must convert cancellation into cleanup. await _waitForPollAsync().ConfigureAwait(false); + // Why switch back: the poll delay resumes off-thread, but _isEditorCompiling and + // completion callbacks use Unity Editor APIs that require the main thread. + await MainThreadSwitcher.SwitchToMainThread(ct); if (!observedStart) { waitedForStartMs += UnityCliLoopConstants.COMPILE_START_POLL_INTERVAL_MS; diff --git a/Packages/src/Editor/FirstPartyTools/Compile/CompileUseCase.cs b/Packages/src/Editor/FirstPartyTools/Compile/CompileUseCase.cs index 8f88cc03da..38ed3ba1af 100644 --- a/Packages/src/Editor/FirstPartyTools/Compile/CompileUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/Compile/CompileUseCase.cs @@ -103,7 +103,7 @@ public async Task CompileAsync(CompileSchema request, Cancellat BuildCompileLogContext(request), correlationId); preparationService.StopPlayMode(); - bool exited = await WaitForPlayModeExitAsync(ct).ConfigureAwait(false); + bool exited = await WaitForPlayModeExitAsync(ct); VibeLogger.LogInfo( "compile_playmode_exit_observed", exited ? "Play Mode exited before compile." : "Play Mode did not exit before compile.", @@ -171,7 +171,7 @@ private async Task WaitForPlayModeExitAsync(CancellationToken ct) while (EditorApplication.isPlaying && waitedMs < MAX_WAIT_MS) { ct.ThrowIfCancellationRequested(); - await TimerDelay.Wait(POLL_INTERVAL_MS, ct).ConfigureAwait(false); + await TimerDelay.Wait(POLL_INTERVAL_MS, ct); waitedMs += POLL_INTERVAL_MS; } diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/AutoUsingResolver.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/AutoUsingResolver.cs index b1384f73dd..9c161e05d0 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/AutoUsingResolver.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/AutoUsingResolver.cs @@ -40,7 +40,8 @@ public async Task ResolveAsync( { File.WriteAllText(sourcePath, currentSource); // AssemblyBuilder retries must stay on Unity's main thread because the compiler API is not thread-safe. - CompilerMessage[] messages = await buildFunc(sourcePath, dllPath, mutableReferences, ct).ConfigureAwait(false); + // Why no ConfigureAwait(false): the next File.WriteAllText / buildFunc iteration must remain on the Editor thread. + CompilerMessage[] messages = await buildFunc(sourcePath, dllPath, mutableReferences, ct); List unresolvedTypes = ExtractUnresolvedTypes(messages); if (unresolvedTypes.Count == 0) @@ -103,7 +104,8 @@ public async Task ResolveAsync( // Final compilation with all added usings File.WriteAllText(sourcePath, currentSource); - CompilerMessage[] finalMessages = await buildFunc(sourcePath, dllPath, mutableReferences, ct).ConfigureAwait(false); + // Why no ConfigureAwait(false): AssemblyBuilder must continue on the Unity main thread. + CompilerMessage[] finalMessages = await buildFunc(sourcePath, dllPath, mutableReferences, ct); return new AutoUsingResult( currentSource, finalMessages, diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/CommandRunner.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/CommandRunner.cs index 2af461ed72..f990998b67 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/CommandRunner.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Execution/CommandRunner.cs @@ -68,7 +68,17 @@ public async Task ExecuteAsync(ExecutionContext context) string correlationId = UnityCliLoopConstants.GenerateCorrelationId(); // Why switch first: callers often resume off-thread via ConfigureAwait(false), but // BeginUndoGroup / EndExecution use Unity Undo APIs that require the main thread. - await MainThreadSwitcher.SwitchToMainThread(context.CancellationToken); + try + { + await MainThreadSwitcher.SwitchToMainThread(context.CancellationToken); + } + catch (OperationCanceledException) + { + // Why catch here: a pre-cancelled token throws before TryBeginExecution, + // and callers expect a cancelled ExecutionResult rather than a leaked OCE. + return CreateCancelledResult(); + } + if (!TryBeginExecution(out int undoGroup, out CancellationTokenSource executionCancellationTokenSource)) { return CreateErrorResult(UnityCliLoopConstants.ERROR_MESSAGE_EXECUTION_IN_PROGRESS); diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsCancelStopRestore.cs b/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsCancelStopRestore.cs index dc33115457..d7bf29f59d 100644 --- a/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsCancelStopRestore.cs +++ b/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsCancelStopRestore.cs @@ -4,6 +4,8 @@ using System.Threading; using System.Threading.Tasks; +using io.github.hatayama.UnityCliLoop.ToolContracts; + namespace io.github.hatayama.UnityCliLoop.FirstPartyTools { /// @@ -216,6 +218,7 @@ private static async Task StopAndRestoreCoreAsy int elapsedMilliseconds = 0; while (elapsedMilliseconds < timeoutMilliseconds) { + await MainThreadSwitcher.SwitchToMainThread(CancellationToken.None); if (isDone()) { return (true, false); @@ -225,6 +228,7 @@ private static async Task StopAndRestoreCoreAsy elapsedMilliseconds += pollIntervalMilliseconds; } + await MainThreadSwitcher.SwitchToMainThread(CancellationToken.None); return (isDone(), true); } diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsUseCase.cs b/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsUseCase.cs index 2a04be8360..038f9c67dd 100644 --- a/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/RunTests/RunTestsUseCase.cs @@ -100,11 +100,10 @@ public async Task ExecuteAsync(RunTestsSchema parameters, Canc string[] clearedPausePointIds = _clearActivePausePoints(); // 2. Test execution - // Why these awaits do not use ConfigureAwait(false): the entry clears all active - // pause points and the server is single-flight (BUSY rejects concurrent commands), - // so no CLI-originated pause can fire during test execution. The only remaining - // pause source is a human manually pausing via the Editor UI, which is native - // Unity behavior and out of scope. + // Why ConfigureAwait(false) is paired with SwitchToMainThread inside execution helpers: + // the server is single-flight (BUSY rejects concurrent commands), so no CLI-originated + // pause can fire during test execution. Off-thread resumes must switch back before any + // Unity API work (stop/restore, cleanup delay, callback dispose). ct.ThrowIfCancellationRequested(); using CancellationTokenSource timeoutCancellationTokenSource = RunTestsExecutionTimeout.CreateLinkedTimeoutSource(ct, parameters.TimeoutSeconds); @@ -124,6 +123,7 @@ public async Task ExecuteAsync(RunTestsSchema parameters, Canc // Why parent ct (not executionCt): CancelAfter only guards the RunFinished wait. // Using the linked token here would mis-report a successful run as timed out when // the fixed cleanup delay straddles the deadline after RunFinished already arrived. + await MainThreadSwitcher.SwitchToMainThread(ct); await _waitForTestRunnerCleanupAsync(ct).ConfigureAwait(false); } catch (RunTestsExecutionCanceledException canceledException) diff --git a/Packages/src/Editor/FirstPartyTools/RunTests/TestFramework/PlayModeTestExecuter.cs b/Packages/src/Editor/FirstPartyTools/RunTests/TestFramework/PlayModeTestExecuter.cs index 1119e0c1cb..862493c2e4 100644 --- a/Packages/src/Editor/FirstPartyTools/RunTests/TestFramework/PlayModeTestExecuter.cs +++ b/Packages/src/Editor/FirstPartyTools/RunTests/TestFramework/PlayModeTestExecuter.cs @@ -6,6 +6,8 @@ using UnityEditor.TestTools.TestRunner.Api; using UnityEngine; +using io.github.hatayama.UnityCliLoop.ToolContracts; + [assembly: InternalsVisibleTo("UnityCLILoop.FirstPartyTools.Editor")] namespace io.github.hatayama.UnityCliLoop.FirstPartyTools @@ -56,6 +58,9 @@ public static async Task ExecutePlayModeTest( } catch (OperationCanceledException originalException) { + // Why switch first: ExecuteTestWithEventNotification may resume off-thread via + // ConfigureAwait(false), but stop/restore hooks call Unity Play Mode APIs. + await MainThreadSwitcher.SwitchToMainThread(CancellationToken.None); RunTestsCancelStopRestoreResult stopResult = await RunTestsCancelStopRestore.StopAndRestoreAsync( isPlayMode: true, runGuid: runGuid, @@ -64,6 +69,7 @@ public static async Task ExecutePlayModeTest( } finally { + await MainThreadSwitcher.SwitchToMainThread(CancellationToken.None); scope.Dispose(); } } @@ -84,6 +90,7 @@ public static async Task ExecuteEditModeTest( } catch (OperationCanceledException originalException) { + await MainThreadSwitcher.SwitchToMainThread(CancellationToken.None); RunTestsCancelStopRestoreResult stopResult = await RunTestsCancelStopRestore.StopAndRestoreAsync( isPlayMode: false, runGuid: runGuid, @@ -124,6 +131,9 @@ private static async Task ExecuteTestWithEventNotificati using CancellationTokenRegistration cancellationRegistration = ct.Register(() => taskCompletionSource.TrySetCanceled(ct)); SerializableTestResult result = await taskCompletionSource.Task.ConfigureAwait(false); + // Why switch back: UnifiedTestCallback.Dispose unregisters TestRunnerApi callbacks + // and must run on the Unity main thread. + await MainThreadSwitcher.SwitchToMainThread(ct); ct.ThrowIfCancellationRequested(); return result; } diff --git a/Packages/src/Editor/FirstPartyTools/Watch/WatchTools.cs b/Packages/src/Editor/FirstPartyTools/Watch/WatchTools.cs index fba4ee0a58..188b861a82 100644 --- a/Packages/src/Editor/FirstPartyTools/Watch/WatchTools.cs +++ b/Packages/src/Editor/FirstPartyTools/Watch/WatchTools.cs @@ -190,6 +190,9 @@ public static async Task EnableAsync(EnableWatchSchema parameters WatchCompilationResult compilationResult = await WatchExpressionServices.Compiler .CompileAsync(parameters.Expression, ct).ConfigureAwait(false); + // Why switch back: CompileAsync resumes off-thread, but Registry/EnsureMonitorStarted + // touch EditorApplication.update and must run on the Unity main thread. + await MainThreadSwitcher.SwitchToMainThread(ct); if (!compilationResult.Success) { return CreateCompilationFailure(compilationResult); diff --git a/Packages/src/Editor/Presentation/UnityCliLoopSettingsCliSetupPresenter.cs b/Packages/src/Editor/Presentation/UnityCliLoopSettingsCliSetupPresenter.cs index f1a0057cfe..e480328ab0 100644 --- a/Packages/src/Editor/Presentation/UnityCliLoopSettingsCliSetupPresenter.cs +++ b/Packages/src/Editor/Presentation/UnityCliLoopSettingsCliSetupPresenter.cs @@ -73,7 +73,7 @@ internal static bool ShouldUninstallCliFromPrimaryButton( bool canUninstallCli, string requiredCliVersion) { - bool isCliInstalled = cliVersion != null; + bool isCliInstalled = !string.IsNullOrEmpty(cliVersion); bool needsUpdate = IsCliUpdateNeeded(cliVersion, cliIsDispatcher, requiredCliVersion); return CliSetupPrimaryActionPolicy.ShouldUninstallCli( isCliInstalled, @@ -89,7 +89,7 @@ internal static CliSetupPrimaryAction ResolveCliPrimaryButtonAction( string requiredCliVersion) { bool needsUpdate = IsCliUpdateNeeded(cliVersion, cliIsDispatcher, requiredCliVersion); - bool isCliInstalled = cliVersion != null; + bool isCliInstalled = !string.IsNullOrEmpty(cliVersion); return CliSetupPrimaryActionPolicy.ResolveSettingsPrimaryAction( needsCliPathSetup, needsUpdate, @@ -149,7 +149,7 @@ private CliSetupData CreateCliSetupData( string cliExecutablePath = _cliSetupApplicationService.GetCachedCliExecutablePath(); string requiredCliVersion = _cliSetupApplicationService.GetMinimumRequiredCliVersion(); - bool isCliInstalled = cliVersion != null || needsCliPathSetup; + bool isCliInstalled = !string.IsNullOrEmpty(cliVersion) || needsCliPathSetup; bool canUninstallCli = _cliSetupApplicationService.IsPackageOwnedCurrentUserInstallPath( cliExecutablePath, UnityEngine.Application.platform); From 181756296b245e8528ee4c5eda4c57acaa4b35d2 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 14 Jul 2026 16:44:09 +0900 Subject: [PATCH 7/7] fix: keep ConfigureAwait guard compliance and watchdog cancel cleanup Restore ConfigureAwait(false) with SwitchToMainThread for CompileUseCase and AutoUsingResolver so the FirstPartyTools guard stays green, and use CancellationToken.None on the watchdog main-thread switch so cancelled polls still reach the loop-head AbortCompile path. Co-authored-by: Cursor --- .../Compile/CompileLifecycleWatchdog.cs | 6 +++--- .../FirstPartyTools/Compile/CompileUseCase.cs | 18 ++++++++++++------ .../DynamicCompilation/AutoUsingResolver.cs | 14 ++++++++++---- 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/Packages/src/Editor/FirstPartyTools/Compile/CompileLifecycleWatchdog.cs b/Packages/src/Editor/FirstPartyTools/Compile/CompileLifecycleWatchdog.cs index 482ac98952..ac7143dac9 100644 --- a/Packages/src/Editor/FirstPartyTools/Compile/CompileLifecycleWatchdog.cs +++ b/Packages/src/Editor/FirstPartyTools/Compile/CompileLifecycleWatchdog.cs @@ -103,9 +103,9 @@ internal async Task WatchAsync(CancellationToken ct) // The delay is not cancelled directly because the watchdog must convert cancellation into cleanup. await _waitForPollAsync().ConfigureAwait(false); - // Why switch back: the poll delay resumes off-thread, but _isEditorCompiling and - // completion callbacks use Unity Editor APIs that require the main thread. - await MainThreadSwitcher.SwitchToMainThread(ct); + // Why CancellationToken.None: a cancelled ct would throw OCE here and skip the + // loop-head _onCancelled conversion; cleanup must still run on the main thread. + await MainThreadSwitcher.SwitchToMainThread(CancellationToken.None); if (!observedStart) { waitedForStartMs += UnityCliLoopConstants.COMPILE_START_POLL_INTERVAL_MS; diff --git a/Packages/src/Editor/FirstPartyTools/Compile/CompileUseCase.cs b/Packages/src/Editor/FirstPartyTools/Compile/CompileUseCase.cs index 38ed3ba1af..a864833350 100644 --- a/Packages/src/Editor/FirstPartyTools/Compile/CompileUseCase.cs +++ b/Packages/src/Editor/FirstPartyTools/Compile/CompileUseCase.cs @@ -103,7 +103,10 @@ public async Task CompileAsync(CompileSchema request, Cancellat BuildCompileLogContext(request), correlationId); preparationService.StopPlayMode(); - bool exited = await WaitForPlayModeExitAsync(ct); + bool exited = await WaitForPlayModeExitAsync(ct).ConfigureAwait(false); + // Why switch back: the poll loop uses ConfigureAwait(false), but subsequent + // validation and compile orchestration call Unity Editor APIs. + await MainThreadSwitcher.SwitchToMainThread(ct); VibeLogger.LogInfo( "compile_playmode_exit_observed", exited ? "Play Mode exited before compile." : "Play Mode did not exit before compile.", @@ -160,10 +163,11 @@ public async Task CompileAsync(CompileSchema request, Cancellat return successResponse; } - // Why these awaits do not use ConfigureAwait(false): StopPlayMode() clears isPaused - // before setting isPlaying=false, so by the time the poll loop's TimerDelay continuation - // is posted to UnitySynchronizationContext the pause is already lifted and the queue - // drains normally. Measured 2026-07-11: compile during pause self-resolved in ~5s. + // Why ConfigureAwait(false) is paired with SwitchToMainThread at the call site: + // StopPlayMode() clears isPaused before setting isPlaying=false, so by the time the + // poll loop's TimerDelay continuation is posted the pause is already lifted. Measured + // 2026-07-11: compile during pause self-resolved in ~5s. Off-thread resumes must still + // switch back before Unity API work after the wait returns. private async Task WaitForPlayModeExitAsync(CancellationToken ct) { int waitedMs = 0; @@ -171,7 +175,9 @@ private async Task WaitForPlayModeExitAsync(CancellationToken ct) while (EditorApplication.isPlaying && waitedMs < MAX_WAIT_MS) { ct.ThrowIfCancellationRequested(); - await TimerDelay.Wait(POLL_INTERVAL_MS, ct); + await TimerDelay.Wait(POLL_INTERVAL_MS, ct).ConfigureAwait(false); + // Why switch each poll: EditorApplication.isPlaying must be read on the main thread. + await MainThreadSwitcher.SwitchToMainThread(ct); waitedMs += POLL_INTERVAL_MS; } diff --git a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/AutoUsingResolver.cs b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/AutoUsingResolver.cs index 9c161e05d0..e68b95ba08 100644 --- a/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/AutoUsingResolver.cs +++ b/Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/AutoUsingResolver.cs @@ -7,6 +7,8 @@ using System.Threading.Tasks; using UnityEditor.Compilation; +using io.github.hatayama.UnityCliLoop.ToolContracts; + namespace io.github.hatayama.UnityCliLoop.FirstPartyTools { /// @@ -40,8 +42,10 @@ public async Task ResolveAsync( { File.WriteAllText(sourcePath, currentSource); // AssemblyBuilder retries must stay on Unity's main thread because the compiler API is not thread-safe. - // Why no ConfigureAwait(false): the next File.WriteAllText / buildFunc iteration must remain on the Editor thread. - CompilerMessage[] messages = await buildFunc(sourcePath, dllPath, mutableReferences, ct); + CompilerMessage[] messages = await buildFunc(sourcePath, dllPath, mutableReferences, ct).ConfigureAwait(false); + // Why switch back: ConfigureAwait(false) resumes off-thread, but the next + // File.WriteAllText / buildFunc iteration must remain on the Editor thread. + await MainThreadSwitcher.SwitchToMainThread(ct); List unresolvedTypes = ExtractUnresolvedTypes(messages); if (unresolvedTypes.Count == 0) @@ -104,8 +108,10 @@ public async Task ResolveAsync( // Final compilation with all added usings File.WriteAllText(sourcePath, currentSource); - // Why no ConfigureAwait(false): AssemblyBuilder must continue on the Unity main thread. - CompilerMessage[] finalMessages = await buildFunc(sourcePath, dllPath, mutableReferences, ct); + CompilerMessage[] finalMessages = await buildFunc(sourcePath, dllPath, mutableReferences, ct).ConfigureAwait(false); + // Why switch back: AssemblyBuilder continuations must return to the Unity main thread + // before the resolver returns to callers that may touch Editor state. + await MainThreadSwitcher.SwitchToMainThread(ct); return new AutoUsingResult( currentSource, finalMessages,