From 9463f00883a51d8be967cfb819ca2deeae1f7c1d Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 6 Jul 2026 02:08:51 +0900 Subject: [PATCH 1/2] Port CLI pin reading behind an Application-owned interface The Application layer performed File.ReadAllText and JObject.Parse directly through static CliPinReader calls, hiding a file-system dependency inside setup and installation-detection flows. - Declare ICliPinReader in Application; the static class keeps only the pure BuildDispatcherReleaseTag helper - Move the IO, parsing, and validation into Infrastructure's CliPinReaderService with identical semantics and error messages - Inject the reader into CliSetupApplicationService and CliInstallationDetector, wired once in the composition root; the pin is still resolved on the caller thread before Task.Run - Add CliPinReaderServiceTests covering the valid, missing-file, empty-file, and missing-key branches --- Assets/Tests/Editor/CliPathSetupFlowTests.cs | 12 +- .../Tests/Editor/CliPinReaderServiceTests.cs | 136 ++++++++++++++++++ .../Editor/CliPinReaderServiceTests.cs.meta | 11 ++ .../Editor/CliSetupApplicationServiceTests.cs | 15 +- .../src/Editor/Application/CliPinReader.cs | 84 ++--------- .../Application/CliSetupApplicationService.cs | 14 +- .../UnityCliLoopApplicationRegistration.cs | 6 +- .../CLI/CliInstallationDetector.cs | 11 +- .../Infrastructure/CLI/CliPinReaderService.cs | 82 +++++++++++ .../CLI/CliPinReaderService.cs.meta | 11 ++ 10 files changed, 292 insertions(+), 90 deletions(-) create mode 100644 Assets/Tests/Editor/CliPinReaderServiceTests.cs create mode 100644 Assets/Tests/Editor/CliPinReaderServiceTests.cs.meta create mode 100644 Packages/src/Editor/Infrastructure/CLI/CliPinReaderService.cs create mode 100644 Packages/src/Editor/Infrastructure/CLI/CliPinReaderService.cs.meta diff --git a/Assets/Tests/Editor/CliPathSetupFlowTests.cs b/Assets/Tests/Editor/CliPathSetupFlowTests.cs index fd33f6ba0b..f167dced8d 100644 --- a/Assets/Tests/Editor/CliPathSetupFlowTests.cs +++ b/Assets/Tests/Editor/CliPathSetupFlowTests.cs @@ -5,6 +5,7 @@ using UnityEngine; using io.github.hatayama.UnityCliLoop.Application; +using io.github.hatayama.UnityCliLoop.Infrastructure; namespace io.github.hatayama.UnityCliLoop.Tests.Editor { @@ -21,7 +22,7 @@ public async Task EnsureCliVisibleFromShellAsync_WhenAlreadyVisibleDoesNotApplyP FakeNativeCliInstaller installer = new FakeNativeCliInstaller( CreateSupportedPlan(), CreateAppliedResult()); - CliSetupApplicationService service = new(detector, installer); + CliSetupApplicationService service = new(detector, installer, new CliPinReaderService()); CliPathSetupFlowResult result = await service.EnsureCliVisibleFromShellAsync( RuntimePlatform.OSXEditor, @@ -39,7 +40,7 @@ public async Task EnsureCliVisibleFromShellAsync_WhenApplySucceedsRechecksShell( FakeNativeCliInstaller installer = new FakeNativeCliInstaller( CreateSupportedPlan(), CreateAppliedResult()); - CliSetupApplicationService service = new(detector, installer); + CliSetupApplicationService service = new(detector, installer, new CliPinReaderService()); CliPathSetupFlowResult result = await service.EnsureCliVisibleFromShellAsync( RuntimePlatform.OSXEditor, @@ -58,7 +59,7 @@ public async Task EnsureCliVisibleFromShellAsync_WhenUnsupportedShellReturnsManu FakeNativeCliInstaller installer = new FakeNativeCliInstaller( CreateUnsupportedPlan(), CreateAppliedResult()); - CliSetupApplicationService service = new(detector, installer); + CliSetupApplicationService service = new(detector, installer, new CliPinReaderService()); CliPathSetupFlowResult result = await service.EnsureCliVisibleFromShellAsync( RuntimePlatform.OSXEditor, @@ -78,7 +79,10 @@ public async Task EnsureCliVisibleFromShellAsync_WhenProfileApplyFailsReturnsFai CliPathSetupApplyStatus.Failed, "profile is read-only"); FakeNativeCliInstaller installer = new FakeNativeCliInstaller(CreateSupportedPlan(), applyResult); - CliSetupApplicationService service = new CliSetupApplicationService(detector, installer); + CliSetupApplicationService service = new CliSetupApplicationService( + detector, + installer, + new CliPinReaderService()); CliPathSetupFlowResult result = await service.EnsureCliVisibleFromShellAsync( RuntimePlatform.OSXEditor, diff --git a/Assets/Tests/Editor/CliPinReaderServiceTests.cs b/Assets/Tests/Editor/CliPinReaderServiceTests.cs new file mode 100644 index 0000000000..efb95b2129 --- /dev/null +++ b/Assets/Tests/Editor/CliPinReaderServiceTests.cs @@ -0,0 +1,136 @@ +using System; +using System.IO; + +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.Application; +using io.github.hatayama.UnityCliLoop.Infrastructure; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Test fixture that verifies CLI pin reader service file IO behavior. + /// + public sealed class CliPinReaderServiceTests + { + [Test] + public void LoadPinFromPath_WhenPinFileIsValid_ReturnsBothVersions() + { + // Tests that a well-formed pin file yields the project runner and dispatcher versions. + string root = CreateTestRoot(); + string pinPath = Path.Combine(root, "project-runner-pin.json"); + + try + { + Directory.CreateDirectory(root); + File.WriteAllText( + pinPath, + "{\"projectRunnerVersion\":\"3.0.0\",\"minimumDispatcherVersion\":\"3.0.1\"}"); + + CliPinLoadResult result = CliPinReaderService.LoadPinFromPath(pinPath); + + Assert.That(result.Success, Is.True); + Assert.That(result.Pin.ProjectRunnerVersion, Is.EqualTo("3.0.0")); + Assert.That(result.Pin.MinimumDispatcherVersion, Is.EqualTo("3.0.1")); + } + finally + { + Directory.Delete(root, recursive: true); + } + } + + [Test] + public void LoadPinFromPath_WhenFileIsMissing_ReturnsFailureWithNotFoundMessage() + { + // Tests that a missing pin file fails with the exact "not found" message. + string root = CreateTestRoot(); + string pinPath = Path.Combine(root, "project-runner-pin.json"); + + CliPinLoadResult result = CliPinReaderService.LoadPinFromPath(pinPath); + + Assert.That(result.Success, Is.False); + Assert.That(result.ErrorMessage, Is.EqualTo($"Unity CLI Loop pin file not found at {pinPath}.")); + } + + [Test] + public void LoadPinFromPath_WhenFileIsEmpty_ReturnsFailureWithEmptyMessage() + { + // Tests that an empty pin file fails with the exact "is empty" message. + string root = CreateTestRoot(); + string pinPath = Path.Combine(root, "project-runner-pin.json"); + + try + { + Directory.CreateDirectory(root); + File.WriteAllText(pinPath, ""); + + CliPinLoadResult result = CliPinReaderService.LoadPinFromPath(pinPath); + + Assert.That(result.Success, Is.False); + Assert.That(result.ErrorMessage, Is.EqualTo($"Unity CLI Loop pin file at {pinPath} is empty.")); + } + finally + { + Directory.Delete(root, recursive: true); + } + } + + [Test] + public void LoadPinFromPath_WhenProjectRunnerVersionKeyIsMissing_ReturnsFailureWithMissingKeyMessage() + { + // Tests that a pin file without projectRunnerVersion fails with the exact missing-key message. + string root = CreateTestRoot(); + string pinPath = Path.Combine(root, "project-runner-pin.json"); + + try + { + Directory.CreateDirectory(root); + File.WriteAllText(pinPath, "{\"minimumDispatcherVersion\":\"3.0.1\"}"); + + CliPinLoadResult result = CliPinReaderService.LoadPinFromPath(pinPath); + + Assert.That(result.Success, Is.False); + Assert.That( + result.ErrorMessage, + Is.EqualTo($"Unity CLI Loop pin file at {pinPath} is missing projectRunnerVersion.")); + } + finally + { + Directory.Delete(root, recursive: true); + } + } + + [Test] + public void LoadPinFromPath_WhenMinimumDispatcherVersionKeyIsMissing_ReturnsFailureWithMissingKeyMessage() + { + // Tests that a pin file without minimumDispatcherVersion fails with the exact missing-key message. + string root = CreateTestRoot(); + string pinPath = Path.Combine(root, "project-runner-pin.json"); + + try + { + Directory.CreateDirectory(root); + File.WriteAllText(pinPath, "{\"projectRunnerVersion\":\"3.0.0\"}"); + + CliPinLoadResult result = CliPinReaderService.LoadPinFromPath(pinPath); + + Assert.That(result.Success, Is.False); + Assert.That( + result.ErrorMessage, + Is.EqualTo($"Unity CLI Loop pin file at {pinPath} is missing minimumDispatcherVersion.")); + } + finally + { + Directory.Delete(root, recursive: true); + } + } + + private static string CreateTestRoot() + { + return Path.Combine( + Path.GetTempPath(), + "unity-cli-loop-tests", + Guid.NewGuid().ToString("N")); + } + } +} diff --git a/Assets/Tests/Editor/CliPinReaderServiceTests.cs.meta b/Assets/Tests/Editor/CliPinReaderServiceTests.cs.meta new file mode 100644 index 0000000000..65ae99cfa9 --- /dev/null +++ b/Assets/Tests/Editor/CliPinReaderServiceTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 52ccf95db8d754def9878110a5abbdef +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/CliSetupApplicationServiceTests.cs b/Assets/Tests/Editor/CliSetupApplicationServiceTests.cs index 73ba3ee678..1fed0ee1a9 100644 --- a/Assets/Tests/Editor/CliSetupApplicationServiceTests.cs +++ b/Assets/Tests/Editor/CliSetupApplicationServiceTests.cs @@ -5,6 +5,7 @@ using UnityEngine; using io.github.hatayama.UnityCliLoop.Application; +using io.github.hatayama.UnityCliLoop.Infrastructure; namespace io.github.hatayama.UnityCliLoop.Tests.Editor { @@ -20,7 +21,8 @@ public async Task InstallGlobalCliAsync_UsesMinimumRequiredDispatcherReleaseTag( FakeNativeCliInstaller nativeCliInstaller = new(); CliSetupApplicationService service = new( new FakeCliInstallationDetector(new string[] { null }), - nativeCliInstaller); + nativeCliInstaller, + new CliPinReaderService()); await service.InstallGlobalCliAsync(RuntimePlatform.OSXEditor, CancellationToken.None); @@ -35,7 +37,8 @@ public void GetMinimumRequiredCliVersion_UsesDispatcherVersion() // Verifies setup reads the minimum dispatcher version from the package pin JSON. CliSetupApplicationService service = new( new FakeCliInstallationDetector(new string[] { null }), - new FakeNativeCliInstaller()); + new FakeNativeCliInstaller(), + new CliPinReaderService()); Assert.That(service.GetMinimumRequiredCliVersion(), Is.EqualTo(ExpectedMinimumDispatcherVersion())); } @@ -46,7 +49,8 @@ public void GetMinimumRequiredCliReleaseTag_UsesDispatcherReleaseTag() // Verifies setup derives the prefixed release tag from the package pin instead of a duplicated constant. CliSetupApplicationService service = new( new FakeCliInstallationDetector(new string[] { null }), - new FakeNativeCliInstaller()); + new FakeNativeCliInstaller(), + new CliPinReaderService()); Assert.That(service.GetMinimumRequiredCliReleaseTag(), Is.EqualTo(ExpectedDispatcherReleaseTag())); } @@ -58,7 +62,8 @@ public void GetGlobalCliInstallCommand_UsesMinimumRequiredDispatcherReleaseTag() FakeNativeCliInstaller nativeCliInstaller = new(); CliSetupApplicationService service = new( new FakeCliInstallationDetector(new string[] { null }), - nativeCliInstaller); + nativeCliInstaller, + new CliPinReaderService()); NativeCliInstallCommand command = service.GetGlobalCliInstallCommand( RuntimePlatform.OSXEditor, @@ -71,7 +76,7 @@ public void GetGlobalCliInstallCommand_UsesMinimumRequiredDispatcherReleaseTag() private static string ExpectedMinimumDispatcherVersion() { - CliPinLoadResult result = CliPinReader.LoadPackagePin(); + CliPinLoadResult result = new CliPinReaderService().LoadPackagePin(); Assert.That(result.Success, Is.True, result.ErrorMessage); return result.Pin.MinimumDispatcherVersion; } diff --git a/Packages/src/Editor/Application/CliPinReader.cs b/Packages/src/Editor/Application/CliPinReader.cs index 00344b3cbf..5f260a9312 100644 --- a/Packages/src/Editor/Application/CliPinReader.cs +++ b/Packages/src/Editor/Application/CliPinReader.cs @@ -1,10 +1,4 @@ -using System; -using System.IO; - -using Newtonsoft.Json.Linq; - using io.github.hatayama.UnityCliLoop.Domain; -using io.github.hatayama.UnityCliLoop.ToolContracts; namespace io.github.hatayama.UnityCliLoop.Application { @@ -51,80 +45,24 @@ public static CliPinLoadResult FromFailure(string errorMessage) } /// - /// Reads the CLI pin JSON that ships with the Unity package so callers can - /// consult a single source of truth for minimum-version requirements. + /// Defines how the CLI pin JSON shipped with the Unity package is read, so Application code can + /// consult a single source of truth for minimum-version requirements without depending on file IO. /// - public static class CliPinReader + public interface ICliPinReader { - // Why: keep pin JSON keys named in one place so callers do not spread string literals. - private const string PIN_JSON_PROJECT_RUNNER_VERSION_KEY = "projectRunnerVersion"; - private const string PIN_JSON_MINIMUM_DISPATCHER_VERSION_KEY = "minimumDispatcherVersion"; - - public static CliPinLoadResult LoadPackagePin() - { - string pinPath = Path.Combine( - UnityCliLoopConstants.PackageResolvedPath, - UnityCliLoopConstants.ULOOP_PROJECT_RUNNER_PIN_FILE_NAME); - return LoadPinFromPath(pinPath); - } - - // Why: exposed so tests and edge-case callers can point at an alternate pin file layout. - public static CliPinLoadResult LoadPinFromPath(string pinPath) - { - if (string.IsNullOrWhiteSpace(pinPath)) - { - return CliPinLoadResult.FromFailure("Unity CLI Loop pin file path is empty."); - } - - if (!File.Exists(pinPath)) - { - return CliPinLoadResult.FromFailure( - $"Unity CLI Loop pin file not found at {pinPath}."); - } - - string content = File.ReadAllText(pinPath); - if (string.IsNullOrWhiteSpace(content)) - { - return CliPinLoadResult.FromFailure( - $"Unity CLI Loop pin file at {pinPath} is empty."); - } - - JObject parsed = JObject.Parse(content); - string projectRunnerVersion = parsed[PIN_JSON_PROJECT_RUNNER_VERSION_KEY]?.ToString(); - string minimumDispatcherVersion = parsed[PIN_JSON_MINIMUM_DISPATCHER_VERSION_KEY]?.ToString(); - if (string.IsNullOrWhiteSpace(projectRunnerVersion)) - { - return CliPinLoadResult.FromFailure( - $"Unity CLI Loop pin file at {pinPath} is missing {PIN_JSON_PROJECT_RUNNER_VERSION_KEY}."); - } - if (string.IsNullOrWhiteSpace(minimumDispatcherVersion)) - { - return CliPinLoadResult.FromFailure( - $"Unity CLI Loop pin file at {pinPath} is missing {PIN_JSON_MINIMUM_DISPATCHER_VERSION_KEY}."); - } - - return CliPinLoadResult.FromSuccess( - new CliPin(projectRunnerVersion, minimumDispatcherVersion)); - } + CliPinLoadResult LoadPackagePin(); + string LoadMinimumDispatcherVersionOrThrow(); + } + /// + /// Builds derived values from the CLI pin JSON that ships with the Unity package. + /// + public static class CliPinReader + { // Why: composes the dispatcher release tag from the pin so callers do not have to know the prefix. public static string BuildDispatcherReleaseTag(string minimumDispatcherVersion) { return CliConstants.DISPATCHER_RELEASE_TAG_PREFIX + minimumDispatcherVersion; } - - // Why: setup/detection paths must fail closed when the pin is unreadable rather than defaulting - // to "compatible", so both call sites share one Fail-Fast helper instead of duplicating it. - public static string LoadMinimumDispatcherVersionOrThrow() - { - CliPinLoadResult pinResult = LoadPackagePin(); - if (!pinResult.Success) - { - throw new InvalidOperationException( - "Unity CLI Loop cannot resolve minimum dispatcher version from the package pin: " - + pinResult.ErrorMessage); - } - return pinResult.Pin.MinimumDispatcherVersion; - } } } diff --git a/Packages/src/Editor/Application/CliSetupApplicationService.cs b/Packages/src/Editor/Application/CliSetupApplicationService.cs index 1214d2de96..f791007fb9 100644 --- a/Packages/src/Editor/Application/CliSetupApplicationService.cs +++ b/Packages/src/Editor/Application/CliSetupApplicationService.cs @@ -66,16 +66,20 @@ public sealed class CliSetupApplicationService { private readonly ICliInstallationDetector _cliInstallationDetector; private readonly INativeCliInstaller _nativeCliInstaller; + private readonly ICliPinReader _cliPinReader; public CliSetupApplicationService( ICliInstallationDetector cliInstallationDetector, - INativeCliInstaller nativeCliInstaller) + INativeCliInstaller nativeCliInstaller, + ICliPinReader cliPinReader) { Debug.Assert(cliInstallationDetector != null, "cliInstallationDetector must not be null"); Debug.Assert(nativeCliInstaller != null, "nativeCliInstaller must not be null"); + Debug.Assert(cliPinReader != null, "cliPinReader must not be null"); - _cliInstallationDetector = cliInstallationDetector; - _nativeCliInstaller = nativeCliInstaller; + _cliInstallationDetector = cliInstallationDetector ?? throw new ArgumentNullException(nameof(cliInstallationDetector)); + _nativeCliInstaller = nativeCliInstaller ?? throw new ArgumentNullException(nameof(nativeCliInstaller)); + _cliPinReader = cliPinReader ?? throw new ArgumentNullException(nameof(cliPinReader)); } public bool IsCliCheckCompleted() @@ -127,14 +131,14 @@ public string GetMinimumRequiredCliVersion() { // Why: v3 setup installs the global dispatcher and reads the minimum from the package pin JSON // so that the single source of truth stays consistent with the dispatcher installation flow. - return CliPinReader.LoadMinimumDispatcherVersionOrThrow(); + return _cliPinReader.LoadMinimumDispatcherVersionOrThrow(); } public string GetMinimumRequiredCliReleaseTag() { // Why: v3 setup installs the global dispatcher, and the release tag is derived from the pin so it // cannot drift from the version reported by GetMinimumRequiredCliVersion. - return CliPinReader.BuildDispatcherReleaseTag(CliPinReader.LoadMinimumDispatcherVersionOrThrow()); + return CliPinReader.BuildDispatcherReleaseTag(_cliPinReader.LoadMinimumDispatcherVersionOrThrow()); } public bool IsPackageOwnedCurrentUserInstallPath( diff --git a/Packages/src/Editor/CompositionRoot/UnityCliLoopApplicationRegistration.cs b/Packages/src/Editor/CompositionRoot/UnityCliLoopApplicationRegistration.cs index 2c60e8a250..750d7a7111 100644 --- a/Packages/src/Editor/CompositionRoot/UnityCliLoopApplicationRegistration.cs +++ b/Packages/src/Editor/CompositionRoot/UnityCliLoopApplicationRegistration.cs @@ -41,9 +41,11 @@ internal UnityCliLoopApplicationServices Register() SkillSetupUseCaseRegistry.Register(skillSetupUseCase); ThirdPartyToolMigrationUseCaseRegistry.Register( new ThirdPartyToolMigrationUseCase(new ThirdPartyToolMigrationFileService())); + CliPinReaderService cliPinReaderService = new(); CliSetupApplicationFacade.RegisterService(new CliSetupApplicationService( - new CliInstallationDetector(), - new NativeCliInstallerService())); + new CliInstallationDetector(cliPinReaderService), + new NativeCliInstallerService(), + cliPinReaderService)); UnityCliLoopBridgeServerInstanceFactory serverFactory = new(domainReloadDetectionService); UnityCliLoopServerLifecycleRegistryService lifecycleRegistry = new(); lifecycleRegistry.RegisterSource(serverFactory); diff --git a/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs b/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs index e812882e2e..ead180ea42 100644 --- a/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs +++ b/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs @@ -49,12 +49,21 @@ public sealed class CliInstallationDetector : ICliInstallationDetector private const string VERSION_JSON_LEGACY_CLI_VERSION_PROPERTY = "CliVersion"; private const string VERSION_JSON_DISPATCHER_VERSION_PROPERTY = "DispatcherVersion"; + private readonly ICliPinReader _cliPinReader; + private string _cachedCliVersion; private bool _cachedCliIsDispatcher; private string _cachedCliExecutablePath; private bool _cacheInitialized; private bool _isRefreshing; + public CliInstallationDetector(ICliPinReader cliPinReader) + { + UnityEngine.Debug.Assert(cliPinReader != null, "cliPinReader must not be null"); + + _cliPinReader = cliPinReader ?? throw new ArgumentNullException(nameof(cliPinReader)); + } + public bool IsCliInstalled() { return GetCachedCliVersion() != null; @@ -120,7 +129,7 @@ public Task IsCliVisibleFromShellAsync(RuntimePlatform platform, Cancellat // Why: resolve the minimum dispatcher version once on the caller thread so the background // detection task does not perform Unity package IO from a worker thread. - string minimumDispatcherVersion = CliPinReader.LoadMinimumDispatcherVersionOrThrow(); + string minimumDispatcherVersion = _cliPinReader.LoadMinimumDispatcherVersionOrThrow(); return Task.Run( () => diff --git a/Packages/src/Editor/Infrastructure/CLI/CliPinReaderService.cs b/Packages/src/Editor/Infrastructure/CLI/CliPinReaderService.cs new file mode 100644 index 0000000000..0acce5a1e8 --- /dev/null +++ b/Packages/src/Editor/Infrastructure/CLI/CliPinReaderService.cs @@ -0,0 +1,82 @@ +using System; +using System.IO; + +using Newtonsoft.Json.Linq; + +using io.github.hatayama.UnityCliLoop.Application; +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.Infrastructure +{ + /// + /// Reads the CLI pin JSON that ships with the Unity package from disk. + /// + public sealed class CliPinReaderService : ICliPinReader + { + // Why: keep pin JSON keys named in one place so callers do not spread string literals. + private const string PIN_JSON_PROJECT_RUNNER_VERSION_KEY = "projectRunnerVersion"; + private const string PIN_JSON_MINIMUM_DISPATCHER_VERSION_KEY = "minimumDispatcherVersion"; + + public CliPinLoadResult LoadPackagePin() + { + string pinPath = Path.Combine( + UnityCliLoopConstants.PackageResolvedPath, + UnityCliLoopConstants.ULOOP_PROJECT_RUNNER_PIN_FILE_NAME); + return LoadPinFromPath(pinPath); + } + + // Why: setup/detection paths must fail closed when the pin is unreadable rather than defaulting + // to "compatible", so both call sites share one Fail-Fast helper instead of duplicating it. + public string LoadMinimumDispatcherVersionOrThrow() + { + CliPinLoadResult pinResult = LoadPackagePin(); + if (!pinResult.Success) + { + throw new InvalidOperationException( + "Unity CLI Loop cannot resolve minimum dispatcher version from the package pin: " + + pinResult.ErrorMessage); + } + return pinResult.Pin.MinimumDispatcherVersion; + } + + // Why: internal so tests can point at an alternate pin file layout without exposing path-based + // loading on the public ICliPinReader surface. + internal static CliPinLoadResult LoadPinFromPath(string pinPath) + { + if (string.IsNullOrWhiteSpace(pinPath)) + { + return CliPinLoadResult.FromFailure("Unity CLI Loop pin file path is empty."); + } + + if (!File.Exists(pinPath)) + { + return CliPinLoadResult.FromFailure( + $"Unity CLI Loop pin file not found at {pinPath}."); + } + + string content = File.ReadAllText(pinPath); + if (string.IsNullOrWhiteSpace(content)) + { + return CliPinLoadResult.FromFailure( + $"Unity CLI Loop pin file at {pinPath} is empty."); + } + + JObject parsed = JObject.Parse(content); + string projectRunnerVersion = parsed[PIN_JSON_PROJECT_RUNNER_VERSION_KEY]?.ToString(); + string minimumDispatcherVersion = parsed[PIN_JSON_MINIMUM_DISPATCHER_VERSION_KEY]?.ToString(); + if (string.IsNullOrWhiteSpace(projectRunnerVersion)) + { + return CliPinLoadResult.FromFailure( + $"Unity CLI Loop pin file at {pinPath} is missing {PIN_JSON_PROJECT_RUNNER_VERSION_KEY}."); + } + if (string.IsNullOrWhiteSpace(minimumDispatcherVersion)) + { + return CliPinLoadResult.FromFailure( + $"Unity CLI Loop pin file at {pinPath} is missing {PIN_JSON_MINIMUM_DISPATCHER_VERSION_KEY}."); + } + + return CliPinLoadResult.FromSuccess( + new CliPin(projectRunnerVersion, minimumDispatcherVersion)); + } + } +} diff --git a/Packages/src/Editor/Infrastructure/CLI/CliPinReaderService.cs.meta b/Packages/src/Editor/Infrastructure/CLI/CliPinReaderService.cs.meta new file mode 100644 index 0000000000..4aafba1050 --- /dev/null +++ b/Packages/src/Editor/Infrastructure/CLI/CliPinReaderService.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 5a6bdf61389344c71a0a07f54be3f735 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: From 70495e8985edae7791de459f89ca7f9eab283073 Mon Sep 17 00:00:00 2001 From: hatayama Date: Mon, 6 Jul 2026 02:23:19 +0900 Subject: [PATCH 2/2] Return structured failure for corrupt pin JSON in review follow-up Address cubic review findings on the pin reader port: a corrupt pin file now surfaces as a CliPinLoadResult failure instead of an unhandled JsonReaderException, matching the JsonRpcProcessor pattern, and test cleanup guards Directory.Delete so setup failures are not masked by DirectoryNotFoundException. Adds a corrupt-JSON test. --- .../Tests/Editor/CliPinReaderServiceTests.cs | 59 +++++++++++++++++-- .../Infrastructure/CLI/CliPinReaderService.cs | 16 ++++- 2 files changed, 70 insertions(+), 5 deletions(-) diff --git a/Assets/Tests/Editor/CliPinReaderServiceTests.cs b/Assets/Tests/Editor/CliPinReaderServiceTests.cs index efb95b2129..7616c43b10 100644 --- a/Assets/Tests/Editor/CliPinReaderServiceTests.cs +++ b/Assets/Tests/Editor/CliPinReaderServiceTests.cs @@ -35,7 +35,12 @@ public void LoadPinFromPath_WhenPinFileIsValid_ReturnsBothVersions() } finally { - Directory.Delete(root, recursive: true); + // Why: guard so a failure before directory creation does not mask the original + // exception with a DirectoryNotFoundException from cleanup. + if (Directory.Exists(root)) + { + Directory.Delete(root, recursive: true); + } } } @@ -71,7 +76,12 @@ public void LoadPinFromPath_WhenFileIsEmpty_ReturnsFailureWithEmptyMessage() } finally { - Directory.Delete(root, recursive: true); + // Why: guard so a failure before directory creation does not mask the original + // exception with a DirectoryNotFoundException from cleanup. + if (Directory.Exists(root)) + { + Directory.Delete(root, recursive: true); + } } } @@ -96,7 +106,12 @@ public void LoadPinFromPath_WhenProjectRunnerVersionKeyIsMissing_ReturnsFailureW } finally { - Directory.Delete(root, recursive: true); + // Why: guard so a failure before directory creation does not mask the original + // exception with a DirectoryNotFoundException from cleanup. + if (Directory.Exists(root)) + { + Directory.Delete(root, recursive: true); + } } } @@ -121,7 +136,43 @@ public void LoadPinFromPath_WhenMinimumDispatcherVersionKeyIsMissing_ReturnsFail } finally { - Directory.Delete(root, recursive: true); + // Why: guard so a failure before directory creation does not mask the original + // exception with a DirectoryNotFoundException from cleanup. + if (Directory.Exists(root)) + { + Directory.Delete(root, recursive: true); + } + } + } + + [Test] + public void LoadPinFromPath_WhenFileIsInvalidJson_ReturnsFailureWithInvalidJsonMessage() + { + // Tests that a pin file with malformed JSON fails with a message naming the pin path + // instead of letting the JsonReaderException escape. + string root = CreateTestRoot(); + string pinPath = Path.Combine(root, "project-runner-pin.json"); + + try + { + Directory.CreateDirectory(root); + File.WriteAllText(pinPath, "{\"projectRunnerVersion\":"); + + CliPinLoadResult result = CliPinReaderService.LoadPinFromPath(pinPath); + + Assert.That(result.Success, Is.False); + Assert.That( + result.ErrorMessage, + Does.StartWith($"Unity CLI Loop pin file at {pinPath} contains invalid JSON:")); + } + finally + { + // Why: guard so a failure before directory creation does not mask the original + // exception with a DirectoryNotFoundException from cleanup. + if (Directory.Exists(root)) + { + Directory.Delete(root, recursive: true); + } } } diff --git a/Packages/src/Editor/Infrastructure/CLI/CliPinReaderService.cs b/Packages/src/Editor/Infrastructure/CLI/CliPinReaderService.cs index 0acce5a1e8..821f92091e 100644 --- a/Packages/src/Editor/Infrastructure/CLI/CliPinReaderService.cs +++ b/Packages/src/Editor/Infrastructure/CLI/CliPinReaderService.cs @@ -1,6 +1,7 @@ using System; using System.IO; +using Newtonsoft.Json; using Newtonsoft.Json.Linq; using io.github.hatayama.UnityCliLoop.Application; @@ -61,7 +62,20 @@ internal static CliPinLoadResult LoadPinFromPath(string pinPath) $"Unity CLI Loop pin file at {pinPath} is empty."); } - JObject parsed = JObject.Parse(content); + // Why: a corrupt pin must surface as a structured failure like every other unreadable-pin + // case, not as a raw parse exception; mirrors the JsonReaderException handling in + // JsonRpcProcessor and keeps LoadMinimumDispatcherVersionOrThrow's fail-closed message useful. + JObject parsed; + try + { + parsed = JObject.Parse(content); + } + catch (JsonReaderException ex) + { + return CliPinLoadResult.FromFailure( + $"Unity CLI Loop pin file at {pinPath} contains invalid JSON: {ex.Message}"); + } + string projectRunnerVersion = parsed[PIN_JSON_PROJECT_RUNNER_VERSION_KEY]?.ToString(); string minimumDispatcherVersion = parsed[PIN_JSON_MINIMUM_DISPATCHER_VERSION_KEY]?.ToString(); if (string.IsNullOrWhiteSpace(projectRunnerVersion))