From b55506a56adf974e0d1ab093ecc70c9c84a3c167 Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 15 May 2026 16:36:48 +0900 Subject: [PATCH 1/6] Recover login shell when detecting CLI installs Use the macOS directory service shell when GUI-launched Unity does not inherit SHELL, so npm-installed legacy uloop commands remain visible as update candidates instead of looking absent. Add focused coverage for shell selection and directory service output parsing. --- .../Editor/NodeEnvironmentResolverTests.cs | 47 ++++++++ .../Utils/NodeEnvironmentResolver.cs | 109 ++++++++++++++++-- 2 files changed, 147 insertions(+), 9 deletions(-) diff --git a/Assets/Tests/Editor/NodeEnvironmentResolverTests.cs b/Assets/Tests/Editor/NodeEnvironmentResolverTests.cs index fa78274a2..84ce10afb 100644 --- a/Assets/Tests/Editor/NodeEnvironmentResolverTests.cs +++ b/Assets/Tests/Editor/NodeEnvironmentResolverTests.cs @@ -232,5 +232,52 @@ public void ExtractAbsolutePathLine_BannerThenAliasTextThenPath_ReturnsPath() Assert.AreEqual("/usr/local/bin/node", result); } + + [Test] + public void ExtractDirectoryServiceUserShell_WhenUserShellLineExists_ReturnsShellPath() + { + // Verifies that macOS directory service output can recover the user's login shell. + string output = "UserShell: /bin/zsh\n"; + + string result = NodeEnvironmentResolver.ExtractDirectoryServiceUserShell(output); + + Assert.AreEqual("/bin/zsh", result); + } + + [Test] + public void SelectUserShell_WhenEnvironmentShellIsMissing_UsesDirectoryServiceShell() + { + // Verifies that GUI-launched Unity can still resolve terminal PATH through the user's login shell. + string result = NodeEnvironmentResolver.SelectUserShell( + null, + "/bin/zsh", + path => path == "/bin/zsh"); + + Assert.AreEqual("/bin/zsh", result); + } + + [Test] + public void SelectUserShell_WhenEnvironmentShellExists_PrefersEnvironmentShell() + { + // Verifies that an inherited SHELL remains authoritative when it points to a real shell. + string result = NodeEnvironmentResolver.SelectUserShell( + "/bin/bash", + "/bin/zsh", + path => path == "/bin/bash" || path == "/bin/zsh"); + + Assert.AreEqual("/bin/bash", result); + } + + [Test] + public void SelectUserShell_WhenNoCandidateExists_ReturnsPosixFallbackShell() + { + // Verifies that shell resolution keeps a deterministic fallback when no user shell is available. + string result = NodeEnvironmentResolver.SelectUserShell( + null, + null, + path => false); + + Assert.AreEqual("/bin/sh", result); + } } } diff --git a/Packages/src/Editor/Infrastructure/Utils/NodeEnvironmentResolver.cs b/Packages/src/Editor/Infrastructure/Utils/NodeEnvironmentResolver.cs index 41d55fcf2..5fc1ab4d3 100644 --- a/Packages/src/Editor/Infrastructure/Utils/NodeEnvironmentResolver.cs +++ b/Packages/src/Editor/Infrastructure/Utils/NodeEnvironmentResolver.cs @@ -181,6 +181,11 @@ private static string ExecuteAndGetOutput(ProcessStartInfo startInfo) private const string PATH_END_MARKER = "__PATH_END__"; private const string WHICH_START_MARKER = "__WHICH_START__"; private const string WHICH_END_MARKER = "__WHICH_END__"; + private const string POSIX_FALLBACK_SHELL_PATH = "/bin/sh"; + private const string DIRECTORY_SERVICE_EXECUTABLE_PATH = "/usr/bin/dscl"; + private const string DIRECTORY_SERVICE_USERS_PATH_PREFIX = "/Users/"; + private const string DIRECTORY_SERVICE_USER_SHELL_ATTRIBUTE = "UserShell"; + private const string DIRECTORY_SERVICE_USER_SHELL_PREFIX = DIRECTORY_SERVICE_USER_SHELL_ATTRIBUTE + ":"; // Uses markers to extract PATH value, ignoring any banner/echo output from shell startup files internal static string GetLoginShellPathAtPlatform(RuntimePlatform platform) @@ -247,20 +252,106 @@ internal static string ExtractAbsolutePathLine(string block) return null; } - // Falls back to /bin/sh when $SHELL is unset or invalid. /bin/sh won't load - // .zshrc/.bashrc, so version-manager paths may be missed — but $SHELL being unset - // is extremely rare on macOS/Linux and there is no reliable way to detect the user's - // preferred shell without it. internal static string GetUserShell() { - string shell = System.Environment.GetEnvironmentVariable("SHELL"); - // $SHELL is external input — validate the path exists to avoid Process.Start exceptions - if (!string.IsNullOrEmpty(shell) && File.Exists(shell)) + string environmentShell = System.Environment.GetEnvironmentVariable("SHELL"); + if (IsExistingShell(environmentShell, File.Exists)) { - return shell; + return environmentShell; } - return "/bin/sh"; + string directoryServiceShell = TryReadDirectoryServiceUserShell(System.Environment.UserName); + return SelectUserShell(null, directoryServiceShell, File.Exists); + } + + internal static string SelectUserShell( + string environmentShell, + string directoryServiceShell, + System.Func fileExists) + { + UnityEngine.Debug.Assert(fileExists != null, "fileExists must not be null"); + + if (IsExistingShell(environmentShell, fileExists)) + { + return environmentShell; + } + + if (IsExistingShell(directoryServiceShell, fileExists)) + { + return directoryServiceShell; + } + + return POSIX_FALLBACK_SHELL_PATH; + } + + internal static string ExtractDirectoryServiceUserShell(string output) + { + if (string.IsNullOrEmpty(output)) + { + return null; + } + + string[] lines = output.Split('\n'); + foreach (string rawLine in lines) + { + string line = rawLine.Trim(); + if (!line.StartsWith(DIRECTORY_SERVICE_USER_SHELL_PREFIX, System.StringComparison.Ordinal)) + { + continue; + } + + string shell = line.Substring(DIRECTORY_SERVICE_USER_SHELL_PREFIX.Length).Trim(); + return string.IsNullOrEmpty(shell) ? null : shell; + } + + return null; + } + + private static bool IsExistingShell( + string shell, + System.Func fileExists) + { + return !string.IsNullOrEmpty(shell) && fileExists(shell); + } + + private static string TryReadDirectoryServiceUserShell(string userName) + { + if (!File.Exists(DIRECTORY_SERVICE_EXECUTABLE_PATH) || !IsSafeDirectoryServiceUserName(userName)) + { + return null; + } + + ProcessStartInfo startInfo = new() { + FileName = DIRECTORY_SERVICE_EXECUTABLE_PATH, + Arguments = ". -read " + DIRECTORY_SERVICE_USERS_PATH_PREFIX + userName + + " " + DIRECTORY_SERVICE_USER_SHELL_ATTRIBUTE, + UseShellExecute = false, + RedirectStandardOutput = true, + RedirectStandardError = true, + CreateNoWindow = true + }; + + return ExtractDirectoryServiceUserShell(ExecuteAndGetOutput(startInfo)); + } + + private static bool IsSafeDirectoryServiceUserName(string userName) + { + if (string.IsNullOrEmpty(userName)) + { + return false; + } + + foreach (char character in userName) + { + if (char.IsLetterOrDigit(character) || character == '_' || character == '-' || character == '.') + { + continue; + } + + return false; + } + + return true; } private static bool IsWindowsEditor(RuntimePlatform platform) From 432ea3aef2e8a60fee73dd68fa0a30605cdb1984 Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 15 May 2026 16:57:35 +0900 Subject: [PATCH 2/6] Run shell CLI version detection in login shell Execute command discovery and `uloop -v` in the same login shell so npm shims resolve node exactly as they do from the user's terminal. Keep path data as optional UI context, but treat version output as the installed-state signal. --- .../Editor/CliInstallationDetectorTests.cs | 66 ++++++++++ Packages/src/Editor/Domain/CliConstants.cs | 1 + .../CLI/CliInstallationDetector.cs | 123 +++++++++++++++++- 3 files changed, 189 insertions(+), 1 deletion(-) diff --git a/Assets/Tests/Editor/CliInstallationDetectorTests.cs b/Assets/Tests/Editor/CliInstallationDetectorTests.cs index 056eb1592..da66607b8 100644 --- a/Assets/Tests/Editor/CliInstallationDetectorTests.cs +++ b/Assets/Tests/Editor/CliInstallationDetectorTests.cs @@ -84,5 +84,71 @@ public void SelectPreferredDetection_WhenPackageOwnedCliMissingUsesShellPath() Assert.That(result.Version, Is.EqualTo("2.1.0")); Assert.That(result.ExecutablePath, Is.EqualTo("/Users/masamichi/.npm-global/bin/uloop")); } + + [Test] + public void SelectPreferredDetection_WhenShellVersionExistsWithoutPathUsesShellVersion() + { + // Verifies that installed state does not depend on command path availability. + CliInstallationDetection packageOwnedDetection = new( + "3.0.0-beta.3", + "/Users/masamichi/.local/bin/uloop"); + CliInstallationDetection shellDetection = new( + "2.1.0", + null); + + CliInstallationDetection result = CliInstallationDetector.SelectPreferredDetection( + packageOwnedDetection, + shellDetection); + + Assert.That(result.Version, Is.EqualTo("2.1.0")); + Assert.That(result.ExecutablePath, Is.Null); + } + + [Test] + public void BuildShellCliDetectionCommand_UsesShortVersionFlag() + { + // Verifies that shell detection asks the command itself for its terminal-visible version. + string command = CliInstallationDetector.BuildShellCliDetectionCommand("uloop"); + + Assert.That(command, Does.Contain("command -v uloop")); + Assert.That(command, Does.Contain("uloop -v")); + Assert.That(command, Does.Not.Contain("uloop --version")); + } + + [Test] + public void ParseShellCliInstallationOutput_WhenPathAndVersionExist_ReturnsDetection() + { + // Verifies that shell detection keeps terminal-visible path data as auxiliary UI context. + string output = "banner\n" + + "__ULOOP_PATH_START__\n" + + "/Users/masamichi/.npm-global/bin/uloop\n" + + "__ULOOP_PATH_END__\n" + + "__ULOOP_VERSION_START__\n" + + "2.1.1\n" + + "__ULOOP_VERSION_END__\n"; + + CliInstallationDetection detection = + CliInstallationDetector.ParseShellCliInstallationOutput(output); + + Assert.That(detection.Version, Is.EqualTo("2.1.1")); + Assert.That(detection.ExecutablePath, Is.EqualTo("/Users/masamichi/.npm-global/bin/uloop")); + } + + [Test] + public void ParseShellCliInstallationOutput_WhenOnlyVersionExists_ReturnsInstalledDetection() + { + // Verifies that installation state depends on version output, not path availability. + string output = "__ULOOP_PATH_START__\n" + + "__ULOOP_PATH_END__\n" + + "__ULOOP_VERSION_START__\n" + + "2.1.1\n" + + "__ULOOP_VERSION_END__\n"; + + CliInstallationDetection detection = + CliInstallationDetector.ParseShellCliInstallationOutput(output); + + Assert.That(detection.Version, Is.EqualTo("2.1.1")); + Assert.That(detection.ExecutablePath, Is.Null); + } } } diff --git a/Packages/src/Editor/Domain/CliConstants.cs b/Packages/src/Editor/Domain/CliConstants.cs index 0bff7036f..71afaabc0 100644 --- a/Packages/src/Editor/Domain/CliConstants.cs +++ b/Packages/src/Editor/Domain/CliConstants.cs @@ -8,6 +8,7 @@ public static class CliConstants public const string EXECUTABLE_NAME = "uloop"; public const string MINIMUM_REQUIRED_CLI_VERSION = "3.0.0-beta.7"; public const string VERSION_FLAG = "--version"; + public const string SHORT_VERSION_FLAG = "-v"; public const string RAW_CONTENT_BASE_URL = "https://raw.githubusercontent.com/hatayama/unity-cli-loop"; public const string STABLE_INSTALLER_SOURCE_REF = "main"; public const string BETA_INSTALLER_SOURCE_REF = "v3-beta"; diff --git a/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs b/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs index ed0855fdb..a822d7aff 100644 --- a/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs +++ b/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs @@ -30,6 +30,10 @@ public CliInstallationDetection(string version, string executablePath) public sealed class CliInstallationDetector : ICliInstallationDetector { private const int PROCESS_TIMEOUT_MS = 5000; + private const string SHELL_PATH_START_MARKER = "__ULOOP_PATH_START__"; + private const string SHELL_PATH_END_MARKER = "__ULOOP_PATH_END__"; + private const string SHELL_VERSION_START_MARKER = "__ULOOP_VERSION_START__"; + private const string SHELL_VERSION_END_MARKER = "__ULOOP_VERSION_END__"; private string _cachedCliVersion; private string _cachedCliExecutablePath; @@ -115,7 +119,8 @@ internal static CliInstallationDetection SelectPreferredDetection( CliInstallationDetection packageOwnedDetection, CliInstallationDetection shellDetection) { - return !string.IsNullOrEmpty(shellDetection.ExecutablePath) + return !string.IsNullOrEmpty(shellDetection.Version) + || !string.IsNullOrEmpty(shellDetection.ExecutablePath) ? shellDetection : packageOwnedDetection; } @@ -135,12 +140,128 @@ private static CliInstallationDetection DetectPackageOwnedCliInstallationBlockin private static CliInstallationDetection DetectShellCliInstallationBlocking(RuntimePlatform platform, CancellationToken ct) { + if (platform != RuntimePlatform.WindowsEditor) + { + return DetectShellCliInstallationFromLoginShell(ct); + } + string executablePath = NodeEnvironmentResolver.FindExecutablePathAtPlatform( CliConstants.EXECUTABLE_NAME, platform); return DetectCliInstallationAtExecutablePath(executablePath, ct); } + private static CliInstallationDetection DetectShellCliInstallationFromLoginShell(CancellationToken ct) + { + string shell = NodeEnvironmentResolver.GetUserShell(); + ProcessStartInfo startInfo = new() { + FileName = shell, + Arguments = "-l -i -c " + QuoteProcessArgument(BuildShellCliDetectionCommand(CliConstants.EXECUTABLE_NAME)), + UseShellExecute = false, + RedirectStandardOutput = true, + RedirectStandardError = true, + CreateNoWindow = true + }; + + string output = ExecuteAndGetOutput(startInfo, ct); + return ParseShellCliInstallationOutput(output); + } + + internal static string BuildShellCliDetectionCommand(string executableName) + { + UnityEngine.Debug.Assert(!string.IsNullOrEmpty(executableName), "executableName must not be null or empty"); + + return "echo " + SHELL_PATH_START_MARKER + "\n" + + "command -v " + executableName + "\n" + + "echo " + SHELL_PATH_END_MARKER + "\n" + + "echo " + SHELL_VERSION_START_MARKER + "\n" + + executableName + " " + CliConstants.SHORT_VERSION_FLAG + "\n" + + "echo " + SHELL_VERSION_END_MARKER; + } + + internal static CliInstallationDetection ParseShellCliInstallationOutput(string output) + { + string pathBlock = NodeEnvironmentResolver.ExtractBetweenMarkers( + output, + SHELL_PATH_START_MARKER, + SHELL_PATH_END_MARKER); + string versionBlock = NodeEnvironmentResolver.ExtractBetweenMarkers( + output, + SHELL_VERSION_START_MARKER, + SHELL_VERSION_END_MARKER); + string executablePath = NodeEnvironmentResolver.ExtractAbsolutePathLine(pathBlock); + string version = ExtractFirstNonEmptyLine(versionBlock); + return new CliInstallationDetection(version, executablePath); + } + + private static string ExecuteAndGetOutput(ProcessStartInfo startInfo, CancellationToken ct) + { + UnityEngine.Debug.Assert(startInfo != null, "startInfo must not be null"); + UnityEngine.Debug.Assert(startInfo.RedirectStandardOutput, "RedirectStandardOutput must be true"); + UnityEngine.Debug.Assert(startInfo.RedirectStandardError, "RedirectStandardError must be true"); + + Process process = ProcessStartHelper.TryStart(startInfo); + if (process == null) + { + return null; + } + + using (process) + { + StringBuilder outputBuilder = new(); + + process.OutputDataReceived += (sender, e) => + { + if (e.Data != null) + { + outputBuilder.AppendLine(e.Data); + } + }; + process.ErrorDataReceived += (sender, e) => { }; + + process.BeginOutputReadLine(); + process.BeginErrorReadLine(); + + using CancellationTokenRegistration registration = ct.Register(() => process.Kill()); + bool exited = process.WaitForExit(PROCESS_TIMEOUT_MS); + if (!exited) + { + process.Kill(); + return null; + } + + process.WaitForExit(); + return outputBuilder.ToString().Trim(); + } + } + + private static string ExtractFirstNonEmptyLine(string block) + { + if (string.IsNullOrEmpty(block)) + { + return null; + } + + string[] lines = block.Split('\n'); + foreach (string rawLine in lines) + { + string line = rawLine.Trim(); + if (!string.IsNullOrEmpty(line)) + { + return line; + } + } + + return null; + } + + private static string QuoteProcessArgument(string value) + { + UnityEngine.Debug.Assert(value != null, "value must not be null"); + + return "\"" + value.Replace("\\", "\\\\").Replace("\"", "\\\"") + "\""; + } + private static CliInstallationDetection DetectCliInstallationAtExecutablePath( string executablePath, CancellationToken ct) From d7ac71555aedb8874086154ec2124f0ddda65b33 Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 15 May 2026 18:46:38 +0900 Subject: [PATCH 3/6] Keep first install skill target visible The setup wizard should let first-time users choose a skill target before the CLI is installed. Split the target-row visibility from CLI skill management so the dropdown remains visible while status rows still wait for CLI inspection. --- Assets/Tests/Editor/SetupWizardWindowTests.cs | 42 +++++++++++++++++++ .../Presentation/Setup/SetupWizardWindow.cs | 25 ++++++++--- 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/Assets/Tests/Editor/SetupWizardWindowTests.cs b/Assets/Tests/Editor/SetupWizardWindowTests.cs index b47a3b80a..232aafa50 100644 --- a/Assets/Tests/Editor/SetupWizardWindowTests.cs +++ b/Assets/Tests/Editor/SetupWizardWindowTests.cs @@ -315,6 +315,48 @@ public void CanManageSkills_WhenCliIsInstalled_ReturnsTrue() Assert.That(canManageSkills, Is.True); } + [Test] + public void ShouldShowSkillsTargetRow_WhenFirstInstallAndCliMissing_ReturnsTrue() + { + // Verifies that first-time setup can choose a skill target before CLI installation. + bool shouldShow = SetupWizardWindow.ShouldShowSkillsTargetRowForSetupWizard( + shouldUseFirstInstallSkillsUi: true); + + Assert.That(shouldShow, Is.True); + } + + [Test] + public void ShouldShowSkillsTargetRow_WhenNotFirstInstall_ReturnsFalse() + { + // Verifies that returning setup keeps the compact target row hidden. + bool shouldShow = SetupWizardWindow.ShouldShowSkillsTargetRowForSetupWizard( + shouldUseFirstInstallSkillsUi: false); + + Assert.That(shouldShow, Is.False); + } + + [Test] + public void ShouldShowSkillsTargetList_WhenCliMissing_ReturnsFalse() + { + // Verifies that multi-target status rows stay hidden until the CLI can inspect skill state. + bool shouldShow = SetupWizardWindow.ShouldShowSkillsTargetListForSetupWizard( + canManageSkills: false, + shouldUseFirstInstallSkillsUi: false); + + Assert.That(shouldShow, Is.False); + } + + [Test] + public void ShouldShowSkillsTargetList_WhenCliInstalledAndNotFirstInstall_ReturnsTrue() + { + // Verifies that returning users keep the multi-target skill status view. + bool shouldShow = SetupWizardWindow.ShouldShowSkillsTargetListForSetupWizard( + canManageSkills: true, + shouldUseFirstInstallSkillsUi: false); + + Assert.That(shouldShow, Is.True); + } + [TestCase(false, false, false, false, null, "3.0.0", "Install CLI")] [TestCase(true, false, false, false, "3.0.0", "3.0.0", "Installed")] [TestCase(true, false, false, true, "2.9.0", "3.0.0", "Update CLI (v2.9.0 \u2192 v3.0.0)")] diff --git a/Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs b/Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs index a240fdcfd..4df65d1eb 100644 --- a/Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs +++ b/Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs @@ -577,6 +577,18 @@ internal static bool CanManageSkills(bool cliInstalled) return cliInstalled; } + internal static bool ShouldShowSkillsTargetRowForSetupWizard(bool shouldUseFirstInstallSkillsUi) + { + return shouldUseFirstInstallSkillsUi; + } + + internal static bool ShouldShowSkillsTargetListForSetupWizard( + bool canManageSkills, + bool shouldUseFirstInstallSkillsUi) + { + return canManageSkills && !shouldUseFirstInstallSkillsUi; + } + internal static SkillSetupTargetInfo CreateFirstInstallSkillTarget( SkillsTarget target, bool groupSkillsUnderUnityCliLoop) @@ -736,6 +748,13 @@ private void UpdateSkillsStep( List targets) { _skillsTargetList.Clear(); + bool useFirstInstallSkillsUi = _shouldUseFirstInstallSkillsUi; + ViewDataBinder.SetVisible( + _skillsTargetRow, + ShouldShowSkillsTargetRowForSetupWizard(useFirstInstallSkillsUi)); + ViewDataBinder.SetVisible( + _skillsTargetList, + ShouldShowSkillsTargetListForSetupWizard(canManageSkills, useFirstInstallSkillsUi)); if (!canManageSkills) { @@ -746,17 +765,11 @@ private void UpdateSkillsStep( _isInstallingSkills, hasOutdatedSkills: false); _groupSkillsToggle.SetEnabled(false); - ViewDataBinder.SetVisible(_skillsTargetRow, false); - ViewDataBinder.SetVisible(_skillsTargetList, false); return; } _groupSkillsToggle.SetEnabled(!_isInstallingSkills); - bool useFirstInstallSkillsUi = _shouldUseFirstInstallSkillsUi; - ViewDataBinder.SetVisible(_skillsTargetRow, useFirstInstallSkillsUi); - ViewDataBinder.SetVisible(_skillsTargetList, !useFirstInstallSkillsUi); - if (useFirstInstallSkillsUi) { SkillSetupTargetInfo selectedTargetInfo = GetSelectedSkillTargetInfo( From acb9a11c1dc00090710784ab3426ce7aab27fa11 Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 15 May 2026 19:34:15 +0900 Subject: [PATCH 4/6] Reset setup wizard state on package removal When users remove and reinstall the Unity package, the persisted setup wizard version could prevent first-install setup from appearing again. Subscribe to package removal events and clear only wizard auto-show state while keeping other editor preferences intact. --- ...LoopPackageRemovalSettingsResetterTests.cs | 165 ++++++++++++++++++ ...ackageRemovalSettingsResetterTests.cs.meta | 11 ++ .../InfrastructureEditorStartup.cs | 2 + ...tyCliLoopPackageRemovalSettingsResetter.cs | 101 +++++++++++ ...LoopPackageRemovalSettingsResetter.cs.meta | 11 ++ .../ToolContracts/UnityCliLoopConstants.cs | 1 + 6 files changed, 291 insertions(+) create mode 100644 Assets/Tests/Editor/UnityCliLoopPackageRemovalSettingsResetterTests.cs create mode 100644 Assets/Tests/Editor/UnityCliLoopPackageRemovalSettingsResetterTests.cs.meta create mode 100644 Packages/src/Editor/Infrastructure/Settings/UnityCliLoopPackageRemovalSettingsResetter.cs create mode 100644 Packages/src/Editor/Infrastructure/Settings/UnityCliLoopPackageRemovalSettingsResetter.cs.meta diff --git a/Assets/Tests/Editor/UnityCliLoopPackageRemovalSettingsResetterTests.cs b/Assets/Tests/Editor/UnityCliLoopPackageRemovalSettingsResetterTests.cs new file mode 100644 index 000000000..52a2a97ad --- /dev/null +++ b/Assets/Tests/Editor/UnityCliLoopPackageRemovalSettingsResetterTests.cs @@ -0,0 +1,165 @@ +using System.Collections.Generic; +using System.IO; + +using NUnit.Framework; +using Newtonsoft.Json.Linq; + +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 +{ + public sealed class UnityCliLoopPackageRemovalSettingsResetterTests + { + private static readonly string SettingsFilePath = + Path.Combine(UnityCliLoopConstants.USER_SETTINGS_FOLDER, UnityCliLoopConstants.SETTINGS_FILE_NAME); + + private bool _settingsFileExisted; + private string _settingsFileContent; + private UnityCliLoopEditorSettingsService _editorSettingsService; + private UnityCliLoopEditorSettingsRepository _editorSettingsRepository; + + [SetUp] + public void SetUp() + { + _settingsFileExisted = File.Exists(SettingsFilePath); + _settingsFileContent = _settingsFileExisted ? File.ReadAllText(SettingsFilePath) : null; + + if (!Directory.Exists(UnityCliLoopConstants.USER_SETTINGS_FOLDER)) + { + Directory.CreateDirectory(UnityCliLoopConstants.USER_SETTINGS_FOLDER); + } + + DeleteIfExists(SettingsFilePath); + _editorSettingsService = + UnityCliLoopEditorSettingsTestFactory.CreateServiceWithRepository(out _editorSettingsRepository); + _editorSettingsRepository.InvalidateCache(); + } + + [TearDown] + public void TearDown() + { + RestoreFile(SettingsFilePath, _settingsFileExisted, _settingsFileContent); + _editorSettingsRepository.InvalidateCache(); + } + + [Test] + public void PackageNameConstant_WhenComparedWithPackageManifest_Matches() + { + // Verifies that uninstall detection uses the package manifest identity. + string packageJson = File.ReadAllText("Packages/src/package.json"); + JObject packageManifest = JObject.Parse(packageJson); + + Assert.That(UnityCliLoopConstants.PACKAGE_NAME, Is.EqualTo(packageManifest["name"]?.Value())); + } + + [Test] + public void ShouldResetSetupWizardState_WhenOwnPackageRemoved_ReturnsTrue() + { + // Verifies that uninstall detection matches this package name. + List removedPackageNames = new() + { + UnityCliLoopConstants.PACKAGE_NAME + }; + + bool shouldReset = UnityCliLoopPackageRemovalSettingsResetter.ShouldResetSetupWizardState( + removedPackageNames, + UnityCliLoopConstants.PACKAGE_NAME); + + Assert.That(shouldReset, Is.True); + } + + [Test] + public void ShouldResetSetupWizardState_WhenOtherPackageRemoved_ReturnsFalse() + { + // Verifies that unrelated package removals do not reset setup state. + List removedPackageNames = new() + { + "com.unity.textmeshpro" + }; + + bool shouldReset = UnityCliLoopPackageRemovalSettingsResetter.ShouldResetSetupWizardState( + removedPackageNames, + UnityCliLoopConstants.PACKAGE_NAME); + + Assert.That(shouldReset, Is.False); + } + + [Test] + public void ResetSetupWizardState_WhenWizardStateExists_ClearsOnlyWizardFields() + { + // Verifies that package uninstall resets auto-show state without discarding user settings. + UnityCliLoopEditorSettingsData settings = CreateSettingsWithNonWizardPreferences(); + + UnityCliLoopEditorSettingsData resetSettings = + UnityCliLoopPackageRemovalSettingsResetter.ResetSetupWizardState(settings); + + Assert.That(resetSettings.lastSeenSetupWizardVersion, Is.Empty); + Assert.That(resetSettings.suppressSetupWizardAutoShow, Is.False); + Assert.That(resetSettings.showDeveloperTools, Is.EqualTo(settings.showDeveloperTools)); + Assert.That(resetSettings.showToolSettings, Is.EqualTo(settings.showToolSettings)); + Assert.That(resetSettings.installSkillsFlat, Is.EqualTo(settings.installSkillsFlat)); + Assert.That(resetSettings.isServerRunning, Is.EqualTo(settings.isServerRunning)); + } + + [Test] + public void ResetSetupWizardStateIfPackageRemoved_WhenOwnPackageRemoved_ClearsStoredWizardFields() + { + // Verifies that the removal handler persists the setup wizard reset. + UnityCliLoopEditorSettingsData settings = CreateSettingsWithNonWizardPreferences(); + _editorSettingsService.SaveSettings(settings); + + UnityCliLoopPackageRemovalSettingsResetter.ResetSetupWizardStateIfPackageRemoved( + _editorSettingsService, + new List { UnityCliLoopConstants.PACKAGE_NAME }, + UnityCliLoopConstants.PACKAGE_NAME); + + UnityCliLoopEditorSettingsData updatedSettings = _editorSettingsService.GetSettings(); + Assert.That(updatedSettings.lastSeenSetupWizardVersion, Is.Empty); + Assert.That(updatedSettings.suppressSetupWizardAutoShow, Is.False); + Assert.That(updatedSettings.showDeveloperTools, Is.EqualTo(settings.showDeveloperTools)); + Assert.That(updatedSettings.showToolSettings, Is.EqualTo(settings.showToolSettings)); + Assert.That(updatedSettings.installSkillsFlat, Is.EqualTo(settings.installSkillsFlat)); + Assert.That(updatedSettings.isServerRunning, Is.EqualTo(settings.isServerRunning)); + } + + private static UnityCliLoopEditorSettingsData CreateSettingsWithNonWizardPreferences() + { + return new UnityCliLoopEditorSettingsData + { + showDeveloperTools = true, + lastSeenSetupWizardVersion = "3.0.0-beta.7", + suppressSetupWizardAutoShow = true, + showUnityCliLoopSecuritySetting = false, + showToolSettings = false, + installSkillsFlat = false, + isServerRunning = false, + isAfterCompile = true, + isDomainReloadInProgress = true, + isReconnecting = true, + showReconnectingUI = true, + showPostCompileReconnectingUI = true + }; + } + + private static void DeleteIfExists(string path) + { + if (File.Exists(path)) + { + File.Delete(path); + } + } + + private static void RestoreFile(string path, bool existed, string content) + { + if (existed) + { + File.WriteAllText(path, content); + return; + } + + DeleteIfExists(path); + } + } +} diff --git a/Assets/Tests/Editor/UnityCliLoopPackageRemovalSettingsResetterTests.cs.meta b/Assets/Tests/Editor/UnityCliLoopPackageRemovalSettingsResetterTests.cs.meta new file mode 100644 index 000000000..69b8b3114 --- /dev/null +++ b/Assets/Tests/Editor/UnityCliLoopPackageRemovalSettingsResetterTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 3fdb5491fd48e4aabb43ad491cf88d29 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Infrastructure/InfrastructureEditorStartup.cs b/Packages/src/Editor/Infrastructure/InfrastructureEditorStartup.cs index ceced2526..bb4e0acdb 100644 --- a/Packages/src/Editor/Infrastructure/InfrastructureEditorStartup.cs +++ b/Packages/src/Editor/Infrastructure/InfrastructureEditorStartup.cs @@ -11,6 +11,8 @@ internal static class InfrastructureEditorStartup { internal static void Initialize(UnityCliLoopEditorSettingsService editorSettingsService) { + UnityCliLoopPackageRemovalSettingsResetter packageRemovalSettingsResetter = new(editorSettingsService); + packageRemovalSettingsResetter.RegisterForEditorStartup(); UnityCliLoopEditorSettingsRecoveryScheduler.ScheduleForEditorStartup(editorSettingsService); CompilationLockService.RegisterForEditorStartup(); } diff --git a/Packages/src/Editor/Infrastructure/Settings/UnityCliLoopPackageRemovalSettingsResetter.cs b/Packages/src/Editor/Infrastructure/Settings/UnityCliLoopPackageRemovalSettingsResetter.cs new file mode 100644 index 000000000..fb14098f6 --- /dev/null +++ b/Packages/src/Editor/Infrastructure/Settings/UnityCliLoopPackageRemovalSettingsResetter.cs @@ -0,0 +1,101 @@ +using System; +using System.Collections.Generic; +using System.Linq; + +using UnityEditor; +using UnityEditor.PackageManager; + +using io.github.hatayama.UnityCliLoop.Domain; +using io.github.hatayama.UnityCliLoop.ToolContracts; +using Debug = System.Diagnostics.Debug; + +namespace io.github.hatayama.UnityCliLoop.Infrastructure +{ + /// + /// Resets Setup Wizard state when Unity removes this package from the project. + /// + internal sealed class UnityCliLoopPackageRemovalSettingsResetter + { + private readonly UnityCliLoopEditorSettingsService _editorSettingsService; + + internal UnityCliLoopPackageRemovalSettingsResetter(UnityCliLoopEditorSettingsService editorSettingsService) + { + Debug.Assert(editorSettingsService != null, "editorSettingsService must not be null"); + + _editorSettingsService = editorSettingsService + ?? throw new ArgumentNullException(nameof(editorSettingsService)); + } + + internal void RegisterForEditorStartup() + { + if (AssetDatabase.IsAssetImportWorkerProcess()) + { + return; + } + + if (UnityEngine.Application.isBatchMode) + { + return; + } + + Events.registeringPackages += HandleRegisteringPackages; + } + + internal static bool ShouldResetSetupWizardState( + IEnumerable removedPackageNames, + string packageName) + { + Debug.Assert(removedPackageNames != null, "removedPackageNames must not be null"); + Debug.Assert(!string.IsNullOrEmpty(packageName), "packageName must not be null or empty"); + + return removedPackageNames.Any( + removedPackageName => string.Equals(removedPackageName, packageName, StringComparison.Ordinal)); + } + + internal static UnityCliLoopEditorSettingsData ResetSetupWizardState(UnityCliLoopEditorSettingsData settings) + { + Debug.Assert(settings != null, "settings must not be null"); + + return settings with + { + lastSeenSetupWizardVersion = string.Empty, + suppressSetupWizardAutoShow = false + }; + } + + internal static void ResetSetupWizardStateIfPackageRemoved( + UnityCliLoopEditorSettingsService editorSettingsService, + IEnumerable removedPackageNames, + string packageName) + { + Debug.Assert(editorSettingsService != null, "editorSettingsService must not be null"); + Debug.Assert(removedPackageNames != null, "removedPackageNames must not be null"); + Debug.Assert(!string.IsNullOrEmpty(packageName), "packageName must not be null or empty"); + + if (!ShouldResetSetupWizardState(removedPackageNames, packageName)) + { + return; + } + + editorSettingsService.UpdateSettings(ResetSetupWizardState); + } + + private void HandleRegisteringPackages(PackageRegistrationEventArgs args) + { + Debug.Assert(args != null, "args must not be null"); + + IEnumerable removedPackageNames = GetRemovedPackageNames(args); + ResetSetupWizardStateIfPackageRemoved( + _editorSettingsService, + removedPackageNames, + UnityCliLoopConstants.PACKAGE_NAME); + } + + private static IEnumerable GetRemovedPackageNames(PackageRegistrationEventArgs args) + { + Debug.Assert(args != null, "args must not be null"); + + return args.removed.Select(packageInfo => packageInfo.name); + } + } +} diff --git a/Packages/src/Editor/Infrastructure/Settings/UnityCliLoopPackageRemovalSettingsResetter.cs.meta b/Packages/src/Editor/Infrastructure/Settings/UnityCliLoopPackageRemovalSettingsResetter.cs.meta new file mode 100644 index 000000000..a6ca62f50 --- /dev/null +++ b/Packages/src/Editor/Infrastructure/Settings/UnityCliLoopPackageRemovalSettingsResetter.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: b9432fa43f890411cb65c2cdd0cc92b1 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/ToolContracts/UnityCliLoopConstants.cs b/Packages/src/Editor/ToolContracts/UnityCliLoopConstants.cs index 3491e311e..d4cdb3abb 100644 --- a/Packages/src/Editor/ToolContracts/UnityCliLoopConstants.cs +++ b/Packages/src/Editor/ToolContracts/UnityCliLoopConstants.cs @@ -32,6 +32,7 @@ public static UnityEditor.PackageManager.PackageInfo PackageInfo public static string PackageResolvedPath => PackageInfo.resolvedPath; public const string PROJECT_NAME = "UnityCliLoop"; + public const string PACKAGE_NAME = "io.github.hatayama.uloopmcp"; // Editor settings public const string SETTINGS_FILE_NAME = "UnityCliLoopSettings.json"; From cb1257d6ba6383d2037d3b8aad2d92294a468177 Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 15 May 2026 20:29:50 +0900 Subject: [PATCH 5/6] Guard CLI detection process kill races The shell CLI detection path could kill a process after it had already exited during cancellation or timeout handling. Reuse a guarded process kill helper so only the already-exited race is ignored while preserving the detection flow. --- .../Editor/CliInstallationDetectorTests.cs | 22 +++++++++++++++++++ .../CLI/CliInstallationDetector.cs | 21 ++++++++++++++---- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/Assets/Tests/Editor/CliInstallationDetectorTests.cs b/Assets/Tests/Editor/CliInstallationDetectorTests.cs index da66607b8..d519c42e7 100644 --- a/Assets/Tests/Editor/CliInstallationDetectorTests.cs +++ b/Assets/Tests/Editor/CliInstallationDetectorTests.cs @@ -1,3 +1,5 @@ +using System.Diagnostics; + using NUnit.Framework; using io.github.hatayama.UnityCliLoop.Infrastructure; @@ -150,5 +152,25 @@ public void ParseShellCliInstallationOutput_WhenOnlyVersionExists_ReturnsInstall Assert.That(detection.Version, Is.EqualTo("2.1.1")); Assert.That(detection.ExecutablePath, Is.Null); } + + [Test] + public void KillProcessIfRunning_WhenProcessAlreadyExited_DoesNotThrow() + { + // Verifies that process cleanup tolerates the race where the child exits before Kill. + ProcessStartInfo startInfo = new() + { + FileName = "/bin/sh", + Arguments = "-c \"exit 0\"", + UseShellExecute = false, + RedirectStandardOutput = true, + RedirectStandardError = true, + CreateNoWindow = true + }; + + using Process process = Process.Start(startInfo); + process.WaitForExit(); + + Assert.DoesNotThrow(() => CliInstallationDetector.KillProcessIfRunning(process)); + } } } diff --git a/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs b/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs index a822d7aff..f5be8723a 100644 --- a/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs +++ b/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs @@ -222,11 +222,11 @@ private static string ExecuteAndGetOutput(ProcessStartInfo startInfo, Cancellati process.BeginOutputReadLine(); process.BeginErrorReadLine(); - using CancellationTokenRegistration registration = ct.Register(() => process.Kill()); + using CancellationTokenRegistration registration = ct.Register(() => KillProcessIfRunning(process)); bool exited = process.WaitForExit(PROCESS_TIMEOUT_MS); if (!exited) { - process.Kill(); + KillProcessIfRunning(process); return null; } @@ -235,6 +235,19 @@ private static string ExecuteAndGetOutput(ProcessStartInfo startInfo, Cancellati } } + internal static void KillProcessIfRunning(Process process) + { + UnityEngine.Debug.Assert(process != null, "process must not be null"); + + try + { + process.Kill(); + } + catch (System.InvalidOperationException) + { + } + } + private static string ExtractFirstNonEmptyLine(string block) { if (string.IsNullOrEmpty(block)) @@ -299,7 +312,7 @@ private static CliInstallationDetection DetectCliInstallationAtExecutablePath( using CancellationTokenRegistration registration = ct.Register(() => { - try { process.Kill(); } catch (System.InvalidOperationException) { } + KillProcessIfRunning(process); }); try @@ -308,7 +321,7 @@ private static CliInstallationDetection DetectCliInstallationAtExecutablePath( if (!exited) { - try { process.Kill(); } catch (System.InvalidOperationException) { } + KillProcessIfRunning(process); process.Dispose(); return new CliInstallationDetection(null, executablePath); } From e5dc8736647ac07ca9caf37589d30dd10f08789c Mon Sep 17 00:00:00 2001 From: hatayama Date: Fri, 15 May 2026 21:03:34 +0900 Subject: [PATCH 6/6] Reject failed shell CLI version probes Record the shell-resolved uloop -v exit status before parsing stdout so a broken PATH command cannot be treated as an installed version. Also make the process cleanup race test choose a platform-specific immediate-exit command for Windows Editor runs. --- .../Editor/CliInstallationDetectorTests.cs | 63 ++++++++++++++++--- .../CLI/CliInstallationDetector.cs | 26 +++++++- 2 files changed, 78 insertions(+), 11 deletions(-) diff --git a/Assets/Tests/Editor/CliInstallationDetectorTests.cs b/Assets/Tests/Editor/CliInstallationDetectorTests.cs index d519c42e7..c0c6fcc70 100644 --- a/Assets/Tests/Editor/CliInstallationDetectorTests.cs +++ b/Assets/Tests/Editor/CliInstallationDetectorTests.cs @@ -114,6 +114,8 @@ public void BuildShellCliDetectionCommand_UsesShortVersionFlag() Assert.That(command, Does.Contain("command -v uloop")); Assert.That(command, Does.Contain("uloop -v")); + Assert.That(command, Does.Contain("uloop_version_status=$?")); + Assert.That(command, Does.Contain("__ULOOP_VERSION_STATUS_START__")); Assert.That(command, Does.Not.Contain("uloop --version")); } @@ -127,7 +129,10 @@ public void ParseShellCliInstallationOutput_WhenPathAndVersionExist_ReturnsDetec + "__ULOOP_PATH_END__\n" + "__ULOOP_VERSION_START__\n" + "2.1.1\n" - + "__ULOOP_VERSION_END__\n"; + + "__ULOOP_VERSION_END__\n" + + "__ULOOP_VERSION_STATUS_START__\n" + + "0\n" + + "__ULOOP_VERSION_STATUS_END__\n"; CliInstallationDetection detection = CliInstallationDetector.ParseShellCliInstallationOutput(output); @@ -144,7 +149,10 @@ public void ParseShellCliInstallationOutput_WhenOnlyVersionExists_ReturnsInstall + "__ULOOP_PATH_END__\n" + "__ULOOP_VERSION_START__\n" + "2.1.1\n" - + "__ULOOP_VERSION_END__\n"; + + "__ULOOP_VERSION_END__\n" + + "__ULOOP_VERSION_STATUS_START__\n" + + "0\n" + + "__ULOOP_VERSION_STATUS_END__\n"; CliInstallationDetection detection = CliInstallationDetector.ParseShellCliInstallationOutput(output); @@ -153,11 +161,55 @@ public void ParseShellCliInstallationOutput_WhenOnlyVersionExists_ReturnsInstall Assert.That(detection.ExecutablePath, Is.Null); } + [Test] + public void ParseShellCliInstallationOutput_WhenVersionCommandFails_ReturnsPathWithoutVersion() + { + // Verifies that failed shell probes do not treat stdout usage text as a CLI version. + string output = "__ULOOP_PATH_START__\n" + + "/Users/masamichi/.npm-global/bin/uloop\n" + + "__ULOOP_PATH_END__\n" + + "__ULOOP_VERSION_START__\n" + + "usage: broken uloop\n" + + "__ULOOP_VERSION_END__\n" + + "__ULOOP_VERSION_STATUS_START__\n" + + "1\n" + + "__ULOOP_VERSION_STATUS_END__\n"; + + CliInstallationDetection detection = + CliInstallationDetector.ParseShellCliInstallationOutput(output); + + Assert.That(detection.Version, Is.Null); + Assert.That(detection.ExecutablePath, Is.EqualTo("/Users/masamichi/.npm-global/bin/uloop")); + } + [Test] public void KillProcessIfRunning_WhenProcessAlreadyExited_DoesNotThrow() { // Verifies that process cleanup tolerates the race where the child exits before Kill. - ProcessStartInfo startInfo = new() + ProcessStartInfo startInfo = BuildImmediateExitProcessStartInfo(); + + using Process process = Process.Start(startInfo); + process.WaitForExit(); + + Assert.DoesNotThrow(() => CliInstallationDetector.KillProcessIfRunning(process)); + } + + private static ProcessStartInfo BuildImmediateExitProcessStartInfo() + { + if (UnityEngine.Application.platform == UnityEngine.RuntimePlatform.WindowsEditor) + { + return new ProcessStartInfo + { + FileName = "cmd.exe", + Arguments = "/c exit 0", + UseShellExecute = false, + RedirectStandardOutput = true, + RedirectStandardError = true, + CreateNoWindow = true + }; + } + + return new ProcessStartInfo { FileName = "/bin/sh", Arguments = "-c \"exit 0\"", @@ -166,11 +218,6 @@ public void KillProcessIfRunning_WhenProcessAlreadyExited_DoesNotThrow() RedirectStandardError = true, CreateNoWindow = true }; - - using Process process = Process.Start(startInfo); - process.WaitForExit(); - - Assert.DoesNotThrow(() => CliInstallationDetector.KillProcessIfRunning(process)); } } } diff --git a/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs b/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs index f5be8723a..41272e3d0 100644 --- a/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs +++ b/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs @@ -34,6 +34,9 @@ public sealed class CliInstallationDetector : ICliInstallationDetector private const string SHELL_PATH_END_MARKER = "__ULOOP_PATH_END__"; private const string SHELL_VERSION_START_MARKER = "__ULOOP_VERSION_START__"; private const string SHELL_VERSION_END_MARKER = "__ULOOP_VERSION_END__"; + private const string SHELL_VERSION_STATUS_START_MARKER = "__ULOOP_VERSION_STATUS_START__"; + private const string SHELL_VERSION_STATUS_END_MARKER = "__ULOOP_VERSION_STATUS_END__"; + private const string SHELL_SUCCESS_EXIT_CODE = "0"; private string _cachedCliVersion; private string _cachedCliExecutablePath; @@ -154,7 +157,8 @@ private static CliInstallationDetection DetectShellCliInstallationBlocking(Runti private static CliInstallationDetection DetectShellCliInstallationFromLoginShell(CancellationToken ct) { string shell = NodeEnvironmentResolver.GetUserShell(); - ProcessStartInfo startInfo = new() { + ProcessStartInfo startInfo = new() + { FileName = shell, Arguments = "-l -i -c " + QuoteProcessArgument(BuildShellCliDetectionCommand(CliConstants.EXECUTABLE_NAME)), UseShellExecute = false, @@ -176,7 +180,11 @@ internal static string BuildShellCliDetectionCommand(string executableName) + "echo " + SHELL_PATH_END_MARKER + "\n" + "echo " + SHELL_VERSION_START_MARKER + "\n" + executableName + " " + CliConstants.SHORT_VERSION_FLAG + "\n" - + "echo " + SHELL_VERSION_END_MARKER; + + "uloop_version_status=$?\n" + + "echo " + SHELL_VERSION_END_MARKER + "\n" + + "echo " + SHELL_VERSION_STATUS_START_MARKER + "\n" + + "echo $uloop_version_status\n" + + "echo " + SHELL_VERSION_STATUS_END_MARKER; } internal static CliInstallationDetection ParseShellCliInstallationOutput(string output) @@ -189,8 +197,14 @@ internal static CliInstallationDetection ParseShellCliInstallationOutput(string output, SHELL_VERSION_START_MARKER, SHELL_VERSION_END_MARKER); + string versionStatusBlock = NodeEnvironmentResolver.ExtractBetweenMarkers( + output, + SHELL_VERSION_STATUS_START_MARKER, + SHELL_VERSION_STATUS_END_MARKER); string executablePath = NodeEnvironmentResolver.ExtractAbsolutePathLine(pathBlock); - string version = ExtractFirstNonEmptyLine(versionBlock); + string version = IsSuccessfulShellStatus(versionStatusBlock) + ? ExtractFirstNonEmptyLine(versionBlock) + : null; return new CliInstallationDetection(version, executablePath); } @@ -248,6 +262,12 @@ internal static void KillProcessIfRunning(Process process) } } + private static bool IsSuccessfulShellStatus(string statusBlock) + { + string status = ExtractFirstNonEmptyLine(statusBlock); + return string.Equals(status, SHELL_SUCCESS_EXIT_CODE, StringComparison.Ordinal); + } + private static string ExtractFirstNonEmptyLine(string block) { if (string.IsNullOrEmpty(block))