From 13852f24f942eae2b028ead16a250633f40a7d7c Mon Sep 17 00:00:00 2001 From: hatayama Date: Wed, 20 May 2026 16:57:03 +0900 Subject: [PATCH 1/5] Add CLI PATH setup rebuild plan Document the rebuild goal, scope, and verification gates before reimplementing the installer and Unity UI flow from a clean v3-beta branch. --- .../cli-path-setup-rebuild-plan.md | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 docs/architecture/cli-path-setup-rebuild-plan.md diff --git a/docs/architecture/cli-path-setup-rebuild-plan.md b/docs/architecture/cli-path-setup-rebuild-plan.md new file mode 100644 index 000000000..dda7903c8 --- /dev/null +++ b/docs/architecture/cli-path-setup-rebuild-plan.md @@ -0,0 +1,113 @@ +# CLI PATH Setup Rebuild Plan + +## Goal + +Unity UI から CLI を入れる初心者向けフローでは、インストール後に fresh login interactive shell で `uloop` が解決できるかを実測し、見えない場合はユーザー操作を追加で求めずに shell 設定ファイルへ最小の PATH 行を追記する。 + +コマンドライン installer から入れる上級者向けフローでは、shell 設定ファイルを変更せず、`~/.local/bin` が `PATH` に無い場合だけ、ユーザーが自分で追記できる短い案内を表示する。 + +## Current Branch Reference + +参考実装は `/Users/masamichi/work/unity-cli-loop2` の `codex/fix-v3-cli-path-setup-ui` ブランチに置かれている。 + +そこから再利用する考え方は次の通り。 + +- Unity の起動時に持っていた stale `PATH` を信用しない。 +- fresh login interactive shell で `command -v uloop` と `uloop -v` を実行して判断する。 +- CLI installer は shell profile を勝手に変更しない。 +- UI からのインストールでは、shell profile 変更を初心者向け自動セットアップの一部として扱う。 +- zsh / bash / fish 以外は自動追記しない。 +- 追記前には既存設定を確認し、ファイル末尾に改行が無い場合でも行を連結しない。 + +捨てる複雑さは次の通り。 + +- shell profile 全体を静的解析して PATH の実効値を推測しない。 +- zsh / bash / fish のあらゆる PATH 記法を解釈しようとしない。 +- `ZDOTDIR` や login hook の推測を installer script 側へ過剰に持ち込まない。 +- UI と Setup Wizard の PATH repair 手順を別々に増やさない。 + +## Design + +### 1. Terminal visibility is authoritative + +CLI が terminal から使えるかどうかは、常に shell 実行結果で判断する。 + +POSIX ではユーザー shell を `-l -i -c` で起動し、`command -v uloop` と `uloop -v` を marker 付きで取得する。Unity プロセスが保持している `PATH` から package install directory を一時的に除外し、Unity 起動時点の stale `PATH` で誤判定しないようにする。 + +fish の `$status` だけは POSIX shell と異なるため、shell kind ごとの小さい command builder を用意する。 + +### 2. Profile writes are intentionally boring + +自動追記する行は canonical line だけにする。 + +- zsh / bash: `export PATH="$HOME/.local/bin:$PATH"` +- fish: `fish_add_path "$HOME/.local/bin"` + +既存設定判定は次の順で十分とする。 + +1. 自分たちが書く canonical line がコメントでない行として存在する。 +2. コメントでない行に install directory の literal path または `$HOME` 形式が PATH 設定として含まれる。 + +この判定は「重複追記を避けるための軽い確認」であり、正しさの最終判断には使わない。最終判断は追記後の fresh shell visibility check で行う。 + +### 3. Profile target selection is simple + +自動追記先は shell kind ごとに限定する。 + +- zsh: `$ZDOTDIR/.zshrc` が使えるならそれを使う。`ZDOTDIR` が無ければ `$HOME/.zshrc`。 +- bash: 既存の `$HOME/.bash_profile`、`$HOME/.bash_login`、`$HOME/.profile` の順で選び、無ければ `$HOME/.bash_profile`。 +- fish: `${XDG_CONFIG_HOME:-$HOME/.config}/fish/config.fish`。 + +`ZDOTDIR` を login shell から探りに行く処理は採用しない。必要なら将来、実際の不具合が出た時に小さく追加する。 + +### 4. One application flow owns PATH setup + +UI と Setup Wizard は、それぞれが shell profile の詳細を知らない。 + +共通の application service が次を行う。 + +1. CLI install または repair ボタン押下を受ける。 +2. fresh shell visibility check を実行する。 +3. 見えなければ shell kind に応じた profile plan を作る。 +4. 対応 shell なら profile に追記する。 +5. もう一度 fresh shell visibility check を実行する。 +6. 成功、未対応 shell、書き込み失敗、追記済みだが未解決、の結果を UI に返す。 + +### 5. Command installer remains read-only for profiles + +`scripts/install.sh` は `~/.local/bin` へ CLI を置く。`PATH` に含まれていない場合は案内だけ出す。 + +案内は zsh / bash / fish ごとに最低限の追記コマンドを表示する。設定ファイルの探索や推測は UI 側ほど行わない。 + +## ToDo + +- [ ] 参照 checkout の差分から必要な挙動だけを抽出する。 +- [ ] 方針ファイルを先に commit する。 +- [ ] `CliPathSetupPlan` 系 DTO を小さく作る。 +- [ ] `CliPathSetupProfileResolver` を作り、shell kind と設定ファイルだけを決める。 +- [ ] `CliPathSetupWriter` を作り、canonical line の重複防止と newline-safe append を行う。 +- [ ] `CliTerminalVisibilityChecker` を作り、fresh shell 実測を担当させる。 +- [ ] `CliSetupApplicationService` に install / repair 後の PATH setup flow を集約する。 +- [ ] Settings Window と Setup Wizard は共通 flow の結果表示だけに寄せる。 +- [ ] `scripts/install.sh` を profile 非変更、案内のみの実装へ更新する。 +- [ ] C# unit tests を Red-Green-Refactor で追加する。 +- [ ] shell script tests を追加し、profile を作成・変更しないことを確認する。 +- [ ] `sh -n scripts/install.sh` を通す。 +- [ ] `scripts/test-install-release-filter.sh` を通す。 +- [ ] focused EditMode tests を通す。 +- [ ] `Packages/src/Cli~/dist/darwin-arm64/uloop compile --project-path /Users/masamichi/work/unity-cli-loop` を通す。 +- [ ] 関心ごとごとに commit する。 +- [ ] commit 後に difit を表示する。 + +## Completion Criteria + +- 新ブランチが `origin/v3-beta` 由来である。 +- この方針ファイルが実装前の判断材料として残っている。 +- UI install / repair は stale Unity `PATH` を信用せず、fresh shell で `uloop` が見えない時だけ PATH setup を実行する。 +- UI 自動追記は zsh / bash / fish のみで行う。 +- 未対応 shell は自動追記せず、manual command を表示できる。 +- command installer は shell profile を作成・変更しない。 +- profile 追記は末尾改行なしファイルでも安全に行う。 +- 重複判定は canonical line または明確な install directory PATH 設定だけを対象にする。 +- 参照 checkout にあった巨大な shell profile 静的解析を持ち込まない。 +- 検証コマンドが通り、commit が作成されている。 From 845c95b9c0fa8c7eeb590e62c21afc6ee1004041 Mon Sep 17 00:00:00 2001 From: hatayama Date: Wed, 20 May 2026 17:29:53 +0900 Subject: [PATCH 2/5] Add terminal PATH repair flow for native CLI Keep the POSIX shell installer informational while moving profile writes behind the Unity UI action. Use fresh login-shell command detection as the authority so a running Unity session does not trust stale PATH state. - Add shell-specific PATH setup plans and newline-safe profile writes for zsh, bash, and fish - Refresh the settings and setup wizard primary action so missing terminal visibility shows Fix PATH - Cover installer guidance, profile detection, duplicate prevention, and repair flow with focused tests --- Assets/Tests/Editor/CliPathSetupFlowTests.cs | 173 ++++++++++++ .../Editor/CliPathSetupFlowTests.cs.meta | 11 + .../CliPathSetupProfileResolverTests.cs | 88 ++++++ .../CliPathSetupProfileResolverTests.cs.meta | 11 + .../Tests/Editor/CliPathSetupWriterTests.cs | 102 +++++++ .../Editor/CliPathSetupWriterTests.cs.meta | 11 + .../Editor/CliSetupApplicationServiceTests.cs | 25 ++ Assets/Tests/Editor/CliSetupSectionTests.cs | 21 +- Assets/Tests/Editor/SetupWizardWindowTests.cs | 55 +++- .../SkillsTargetSelectionResolverTests.cs | 1 + ...nityCliLoopSettingsWindowCliActionTests.cs | 30 +++ .../Editor/Application/CliPathSetupPlan.cs | 114 ++++++++ .../Application/CliPathSetupPlan.cs.meta | 11 + .../Application/CliSetupApplicationService.cs | 80 ++++++ .../CLI/CliInstallationDetector.cs | 102 ++++++- .../CLI/CliPathSetupProfileResolver.cs | 251 ++++++++++++++++++ .../CLI/CliPathSetupProfileResolver.cs.meta | 11 + .../Infrastructure/CLI/CliPathSetupWriter.cs | 159 +++++++++++ .../CLI/CliPathSetupWriter.cs.meta | 11 + .../Infrastructure/CLI/NativeCliInstaller.cs | 27 ++ .../Editor/Presentation/CliPathSetupPrompt.cs | 98 +++++++ .../Presentation/CliPathSetupPrompt.cs.meta | 11 + .../Presentation/Setup/SetupWizardWindow.cs | 94 ++++++- .../UIToolkit/Components/CliSetupSection.cs | 7 + .../UnityCliLoopSettingsWindow.cs | 122 ++++++++- .../UnityCliLoopSettingsWindowViewData.cs | 3 + scripts/install.sh | 114 +++++++- scripts/test-install-release-filter.sh | 152 ++++++++++- 28 files changed, 1866 insertions(+), 29 deletions(-) create mode 100644 Assets/Tests/Editor/CliPathSetupFlowTests.cs create mode 100644 Assets/Tests/Editor/CliPathSetupFlowTests.cs.meta create mode 100644 Assets/Tests/Editor/CliPathSetupProfileResolverTests.cs create mode 100644 Assets/Tests/Editor/CliPathSetupProfileResolverTests.cs.meta create mode 100644 Assets/Tests/Editor/CliPathSetupWriterTests.cs create mode 100644 Assets/Tests/Editor/CliPathSetupWriterTests.cs.meta create mode 100644 Packages/src/Editor/Application/CliPathSetupPlan.cs create mode 100644 Packages/src/Editor/Application/CliPathSetupPlan.cs.meta create mode 100644 Packages/src/Editor/Infrastructure/CLI/CliPathSetupProfileResolver.cs create mode 100644 Packages/src/Editor/Infrastructure/CLI/CliPathSetupProfileResolver.cs.meta create mode 100644 Packages/src/Editor/Infrastructure/CLI/CliPathSetupWriter.cs create mode 100644 Packages/src/Editor/Infrastructure/CLI/CliPathSetupWriter.cs.meta create mode 100644 Packages/src/Editor/Presentation/CliPathSetupPrompt.cs create mode 100644 Packages/src/Editor/Presentation/CliPathSetupPrompt.cs.meta diff --git a/Assets/Tests/Editor/CliPathSetupFlowTests.cs b/Assets/Tests/Editor/CliPathSetupFlowTests.cs new file mode 100644 index 000000000..ddb4b80a7 --- /dev/null +++ b/Assets/Tests/Editor/CliPathSetupFlowTests.cs @@ -0,0 +1,173 @@ +using System.Threading; +using System.Threading.Tasks; + +using NUnit.Framework; +using UnityEngine; + +using io.github.hatayama.UnityCliLoop.Application; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Test fixture that verifies the application-owned CLI PATH setup flow. + /// + public class CliPathSetupFlowTests + { + [Test] + public async Task EnsureCliVisibleFromShellAsync_WhenAlreadyVisibleDoesNotApplyProfile() + { + // Verifies that fresh shell visibility is the authority and no profile write happens when uloop is visible. + FakeCliInstallationDetector detector = new(true); + FakeNativeCliInstaller installer = new(CreateSupportedPlan()); + CliSetupApplicationService service = new(detector, installer); + + CliPathSetupFlowResult result = await service.EnsureCliVisibleFromShellAsync( + RuntimePlatform.OSXEditor, + CancellationToken.None); + + Assert.That(result.Status, Is.EqualTo(CliPathSetupFlowStatus.AlreadyVisible)); + Assert.That(installer.ApplyCount, Is.EqualTo(0)); + } + + [Test] + public async Task EnsureCliVisibleFromShellAsync_WhenApplySucceedsRechecksShell() + { + // Verifies that PATH setup is only considered complete after a second fresh shell check passes. + FakeCliInstallationDetector detector = new(false, true); + FakeNativeCliInstaller installer = new(CreateSupportedPlan()); + CliSetupApplicationService service = new(detector, installer); + + CliPathSetupFlowResult result = await service.EnsureCliVisibleFromShellAsync( + RuntimePlatform.OSXEditor, + CancellationToken.None); + + Assert.That(result.Status, Is.EqualTo(CliPathSetupFlowStatus.AppliedAndVisible)); + Assert.That(installer.ApplyCount, Is.EqualTo(1)); + Assert.That(detector.VisibilityCheckCount, Is.EqualTo(2)); + } + + [Test] + public async Task EnsureCliVisibleFromShellAsync_WhenUnsupportedShellReturnsManualSetup() + { + // Verifies that unsupported shells never receive automatic profile edits. + FakeCliInstallationDetector detector = new(false); + FakeNativeCliInstaller installer = new(CreateUnsupportedPlan()); + CliSetupApplicationService service = new(detector, installer); + + CliPathSetupFlowResult result = await service.EnsureCliVisibleFromShellAsync( + RuntimePlatform.OSXEditor, + CancellationToken.None); + + Assert.That(result.Status, Is.EqualTo(CliPathSetupFlowStatus.ManualSetupRequired)); + Assert.That(installer.ApplyCount, Is.EqualTo(0)); + } + + private static CliPathSetupPlan CreateUnsupportedPlan() + { + return new CliPathSetupPlan( + CliPathSetupShellKind.Unsupported, + "tcsh", + false, + "/Users/ExampleUser/.local/bin", + "/Users/ExampleUser/.local/bin", + "", + "", + "export PATH='/Users/ExampleUser/.local/bin':\"$PATH\""); + } + + private static CliPathSetupPlan CreateSupportedPlan() + { + return new CliPathSetupPlan( + CliPathSetupShellKind.Zsh, + "zsh", + true, + "/Users/ExampleUser/.local/bin", + "$HOME/.local/bin", + "/Users/ExampleUser/.zshrc", + "export PATH=\"$HOME/.local/bin:$PATH\"", + "printf '\\n%s\\n' 'export PATH=\"$HOME/.local/bin:$PATH\"' >> '/Users/ExampleUser/.zshrc'"); + } + + private sealed class FakeCliInstallationDetector : ICliInstallationDetector + { + private readonly bool[] _visibilityResults; + + public FakeCliInstallationDetector(params bool[] visibilityResults) + { + _visibilityResults = visibilityResults; + } + + public int VisibilityCheckCount { get; private set; } + + public bool IsCliInstalled() => true; + public string GetCachedCliVersion() => "3.0.0-beta.9"; + public string GetCachedCliExecutablePath() => "/Users/ExampleUser/.local/bin/uloop"; + public bool IsCheckCompleted() => true; + public Task RefreshCliVersionAsync(CancellationToken ct) => Task.CompletedTask; + public Task ForceRefreshCliVersionAsync(CancellationToken ct) => Task.CompletedTask; + public void InvalidateCache() { } + + public Task IsCliVisibleFromShellAsync(RuntimePlatform platform, CancellationToken ct) + { + bool result = _visibilityResults[ + System.Math.Min(VisibilityCheckCount, _visibilityResults.Length - 1)]; + VisibilityCheckCount++; + return Task.FromResult(result); + } + } + + private sealed class FakeNativeCliInstaller : INativeCliInstaller + { + private readonly CliPathSetupPlan _plan; + + public FakeNativeCliInstaller(CliPathSetupPlan plan) + { + _plan = plan; + } + + public int ApplyCount { get; private set; } + + public bool IsPackageOwnedCurrentUserInstallPath(string cliExecutablePath, RuntimePlatform platform) + { + return true; + } + + public bool HasPackageOwnedCurrentUserInstall(RuntimePlatform platform) + { + return true; + } + + public Task InstallGlobalCliAsync( + RuntimePlatform platform, + string cliReleaseTag, + CancellationToken ct) + { + return Task.FromResult(new CliInstallResult(true, "")); + } + + public Task UninstallGlobalCliAsync(RuntimePlatform platform, CancellationToken ct) + { + return Task.FromResult(new CliInstallResult(true, "")); + } + + public Task GetGlobalCliPathSetupPlanAsync(RuntimePlatform platform, CancellationToken ct) + { + return Task.FromResult(_plan); + } + + public CliPathSetupApplyResult ApplyGlobalCliPathSetup(CliPathSetupPlan plan) + { + ApplyCount++; + return new CliPathSetupApplyResult(true, CliPathSetupApplyStatus.Applied, ""); + } + + public NativeCliInstallCommand GetGlobalCliInstallCommand( + RuntimePlatform platform, + string cliReleaseTag, + bool removeLegacyLaunchers) + { + return new NativeCliInstallCommand("sh", "-c true", "install"); + } + } + } +} diff --git a/Assets/Tests/Editor/CliPathSetupFlowTests.cs.meta b/Assets/Tests/Editor/CliPathSetupFlowTests.cs.meta new file mode 100644 index 000000000..1a948c48a --- /dev/null +++ b/Assets/Tests/Editor/CliPathSetupFlowTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: a152a6fcd43614295b67a87b4f56a7fa +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/CliPathSetupProfileResolverTests.cs b/Assets/Tests/Editor/CliPathSetupProfileResolverTests.cs new file mode 100644 index 000000000..ba49472fb --- /dev/null +++ b/Assets/Tests/Editor/CliPathSetupProfileResolverTests.cs @@ -0,0 +1,88 @@ +using NUnit.Framework; +using UnityEngine; + +using io.github.hatayama.UnityCliLoop.Application; +using io.github.hatayama.UnityCliLoop.Infrastructure; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Test fixture that verifies shell profile selection for CLI PATH setup. + /// + public class CliPathSetupProfileResolverTests + { + [Test] + public void ResolvePlan_WhenZshUsesZshrcInZdotdir() + { + // Verifies that zsh setup honors the explicit ZDOTDIR environment root without probing login hooks. + CliPathSetupPlan plan = CliPathSetupProfileResolver.ResolvePlan( + RuntimePlatform.OSXEditor, + "/bin/zsh", + "/Users/ExampleUser", + "/Users/ExampleUser/.config/zsh", + null, + "/Users/ExampleUser/.local/bin", + path => false); + + Assert.That(plan.ShellKind, Is.EqualTo(CliPathSetupShellKind.Zsh)); + Assert.That(plan.ConfigurationFilePath, Is.EqualTo("/Users/ExampleUser/.config/zsh/.zshrc")); + Assert.That(plan.ConfigurationLine, Is.EqualTo("export PATH=\"$HOME/.local/bin:$PATH\"")); + } + + [Test] + public void ResolvePlan_WhenBashProfileExistsUsesExistingProfile() + { + // Verifies that bash setup avoids creating .bash_profile when a login profile already exists. + CliPathSetupPlan plan = CliPathSetupProfileResolver.ResolvePlan( + RuntimePlatform.OSXEditor, + "/bin/bash", + "/Users/ExampleUser", + null, + null, + "/Users/ExampleUser/.local/bin", + path => path == "/Users/ExampleUser/.profile"); + + Assert.That(plan.ShellKind, Is.EqualTo(CliPathSetupShellKind.Bash)); + Assert.That(plan.ConfigurationFilePath, Is.EqualTo("/Users/ExampleUser/.profile")); + Assert.That(plan.ConfigurationLine, Is.EqualTo("export PATH=\"$HOME/.local/bin:$PATH\"")); + } + + [Test] + public void ResolvePlan_WhenFishUsesXdgConfigHome() + { + // Verifies that fish setup writes config.fish under XDG_CONFIG_HOME. + CliPathSetupPlan plan = CliPathSetupProfileResolver.ResolvePlan( + RuntimePlatform.OSXEditor, + "/opt/homebrew/bin/fish", + "/Users/ExampleUser", + null, + "/Users/ExampleUser/Library/Application Support", + "/Users/ExampleUser/.local/bin", + path => false); + + Assert.That(plan.ShellKind, Is.EqualTo(CliPathSetupShellKind.Fish)); + Assert.That( + plan.ConfigurationFilePath, + Is.EqualTo("/Users/ExampleUser/Library/Application Support/fish/config.fish")); + Assert.That(plan.ConfigurationLine, Is.EqualTo("fish_add_path \"$HOME/.local/bin\"")); + } + + [Test] + public void ResolvePlan_WhenUnsupportedShellDisablesAutomaticApply() + { + // Verifies that unknown shells fall back to a manual command instead of guessing a profile file. + CliPathSetupPlan plan = CliPathSetupProfileResolver.ResolvePlan( + RuntimePlatform.OSXEditor, + "/bin/tcsh", + "/Users/ExampleUser", + null, + null, + "/Users/ExampleUser/.local/bin", + path => false); + + Assert.That(plan.ShellKind, Is.EqualTo(CliPathSetupShellKind.Unsupported)); + Assert.That(plan.CanApplyAutomatically, Is.False); + Assert.That(plan.ManualCommand, Does.Contain("/Users/ExampleUser/.local/bin")); + } + } +} diff --git a/Assets/Tests/Editor/CliPathSetupProfileResolverTests.cs.meta b/Assets/Tests/Editor/CliPathSetupProfileResolverTests.cs.meta new file mode 100644 index 000000000..da098c85e --- /dev/null +++ b/Assets/Tests/Editor/CliPathSetupProfileResolverTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: eec738dc55ab340f785efc9b849b8ca1 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/CliPathSetupWriterTests.cs b/Assets/Tests/Editor/CliPathSetupWriterTests.cs new file mode 100644 index 000000000..0e4aa3ddf --- /dev/null +++ b/Assets/Tests/Editor/CliPathSetupWriterTests.cs @@ -0,0 +1,102 @@ +using System.Collections.Generic; +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 profile writes for CLI PATH setup. + /// + public class CliPathSetupWriterTests + { + [Test] + public void Apply_WhenProfileHasNoTrailingNewlinePrependsNewlineBeforeAppending() + { + // Verifies that appending the PATH line never joins it to the previous shell command. + CliPathSetupPlan plan = CreateZshPlan(); + List appendedContent = new List(); + + CliPathSetupApplyResult result = CliPathSetupWriter.Apply( + plan, + path => true, + path => "# existing", + path => new DirectoryInfo(path), + (path, content) => appendedContent.Add(content)); + + Assert.That(result.Status, Is.EqualTo(CliPathSetupApplyStatus.Applied)); + Assert.That(appendedContent, Has.Count.EqualTo(1)); + Assert.That(appendedContent[0], Is.EqualTo("\nexport PATH=\"$HOME/.local/bin:$PATH\"\n")); + } + + [Test] + public void Apply_WhenCanonicalLineExistsDoesNotAppend() + { + // Verifies that the writer does not duplicate the exact line it owns. + CliPathSetupPlan plan = CreateZshPlan(); + int appendCount = 0; + + CliPathSetupApplyResult result = CliPathSetupWriter.Apply( + plan, + path => true, + path => "export PATH=\"$HOME/.local/bin:$PATH\"\n", + path => new DirectoryInfo(path), + (path, content) => appendCount++); + + Assert.That(result.Status, Is.EqualTo(CliPathSetupApplyStatus.AlreadyConfigured)); + Assert.That(appendCount, Is.EqualTo(0)); + } + + [Test] + public void Apply_WhenLineIsCommentedAppends() + { + // Verifies that disabled PATH lines are not treated as configured. + CliPathSetupPlan plan = CreateZshPlan(); + int appendCount = 0; + + CliPathSetupApplyResult result = CliPathSetupWriter.Apply( + plan, + path => true, + path => "# export PATH=\"$HOME/.local/bin:$PATH\"\n", + path => new DirectoryInfo(path), + (path, content) => appendCount++); + + Assert.That(result.Status, Is.EqualTo(CliPathSetupApplyStatus.Applied)); + Assert.That(appendCount, Is.EqualTo(1)); + } + + [Test] + public void Apply_WhenInstallDirectoryAppearsInPathAssignmentDoesNotAppend() + { + // Verifies that an existing PATH assignment with the install directory is enough to avoid duplication. + CliPathSetupPlan plan = CreateZshPlan(); + int appendCount = 0; + + CliPathSetupApplyResult result = CliPathSetupWriter.Apply( + plan, + path => true, + path => "PATH=\"$PATH:$HOME/.local/bin\"\n", + path => new DirectoryInfo(path), + (path, content) => appendCount++); + + Assert.That(result.Status, Is.EqualTo(CliPathSetupApplyStatus.AlreadyConfigured)); + Assert.That(appendCount, Is.EqualTo(0)); + } + + private static CliPathSetupPlan CreateZshPlan() + { + return new CliPathSetupPlan( + CliPathSetupShellKind.Zsh, + "zsh", + true, + "/Users/ExampleUser/.local/bin", + "$HOME/.local/bin", + "/Users/ExampleUser/.zshrc", + "export PATH=\"$HOME/.local/bin:$PATH\"", + "printf '\\n%s\\n' 'export PATH=\"$HOME/.local/bin:$PATH\"' >> '/Users/ExampleUser/.zshrc'"); + } + } +} diff --git a/Assets/Tests/Editor/CliPathSetupWriterTests.cs.meta b/Assets/Tests/Editor/CliPathSetupWriterTests.cs.meta new file mode 100644 index 000000000..f4fb57ca6 --- /dev/null +++ b/Assets/Tests/Editor/CliPathSetupWriterTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 262347b7e9227449fa4a2f9892b162a6 +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 fb8618a19..6a59a125e 100644 --- a/Assets/Tests/Editor/CliSetupApplicationServiceTests.cs +++ b/Assets/Tests/Editor/CliSetupApplicationServiceTests.cs @@ -87,6 +87,8 @@ public FakeCliInstallationDetector(string[] versions) public string GetCachedCliExecutablePath() => ""; public bool IsCheckCompleted() => true; public Task RefreshCliVersionAsync(CancellationToken ct) => Task.CompletedTask; + public Task IsCliVisibleFromShellAsync(RuntimePlatform platform, CancellationToken ct) + => Task.FromResult(true); public Task ForceRefreshCliVersionAsync(CancellationToken ct) { @@ -110,6 +112,11 @@ public bool IsPackageOwnedCurrentUserInstallPath(string cliExecutablePath, Runti return false; } + public bool HasPackageOwnedCurrentUserInstall(RuntimePlatform platform) + { + return false; + } + public Task InstallGlobalCliAsync( RuntimePlatform platform, string cliReleaseTag, @@ -124,6 +131,24 @@ public Task UninstallGlobalCliAsync(RuntimePlatform platform, return Task.FromResult(new CliInstallResult(true, "")); } + public Task GetGlobalCliPathSetupPlanAsync(RuntimePlatform platform, CancellationToken ct) + { + return Task.FromResult(new CliPathSetupPlan( + CliPathSetupShellKind.Zsh, + "zsh", + true, + "/Users/ExampleUser/.local/bin", + "$HOME/.local/bin", + "/Users/ExampleUser/.zshrc", + "export PATH=\"$HOME/.local/bin:$PATH\"", + "printf '\\n%s\\n' 'export PATH=\"$HOME/.local/bin:$PATH\"' >> '/Users/ExampleUser/.zshrc'")); + } + + public CliPathSetupApplyResult ApplyGlobalCliPathSetup(CliPathSetupPlan plan) + { + return new CliPathSetupApplyResult(true, CliPathSetupApplyStatus.Applied, ""); + } + public NativeCliInstallCommand GetGlobalCliInstallCommand( RuntimePlatform platform, string cliReleaseTag, diff --git a/Assets/Tests/Editor/CliSetupSectionTests.cs b/Assets/Tests/Editor/CliSetupSectionTests.cs index 99cac0092..a06895c23 100644 --- a/Assets/Tests/Editor/CliSetupSectionTests.cs +++ b/Assets/Tests/Editor/CliSetupSectionTests.cs @@ -13,14 +13,16 @@ namespace io.github.hatayama.UnityCliLoop.Tests.Editor /// public class CliSetupSectionTests { - [TestCase(false, false, false, false, false, false, null, "3.0.0", "Install CLI")] - [TestCase(true, false, false, false, false, true, "3.0.0", "3.0.0", "Uninstall CLI")] - [TestCase(true, false, false, false, false, false, "3.0.0", "3.0.0", "Install CLI")] - [TestCase(true, false, false, true, false, true, "2.9.0", "3.0.0", "Update CLI (v2.9.0 \u2192 v3.0.0)")] - [TestCase(true, false, false, false, true, true, "3.1.0", "3.0.0", "Downgrade CLI (v3.1.0 \u2192 v3.0.0)")] - [TestCase(true, true, false, false, false, true, "3.0.0", "3.0.0", "Uninstalling...")] - [TestCase(false, true, false, false, false, false, null, "3.0.0", "Installing...")] - [TestCase(false, false, true, false, false, false, null, "3.0.0", "Checking...")] + [TestCase(false, false, false, false, false, false, false, null, "3.0.0", "Install CLI")] + [TestCase(false, false, false, false, false, false, true, null, "3.0.0", "Fix PATH")] + [TestCase(true, false, false, false, false, true, false, "3.0.0", "3.0.0", "Uninstall CLI")] + [TestCase(true, false, false, false, false, false, false, "3.0.0", "3.0.0", "Install CLI")] + [TestCase(true, false, false, false, false, true, true, "3.0.0", "3.0.0", "Fix PATH")] + [TestCase(true, false, false, true, false, true, false, "2.9.0", "3.0.0", "Update CLI (v2.9.0 \u2192 v3.0.0)")] + [TestCase(true, false, false, false, true, true, false, "3.1.0", "3.0.0", "Downgrade CLI (v3.1.0 \u2192 v3.0.0)")] + [TestCase(true, true, false, false, false, true, false, "3.0.0", "3.0.0", "Uninstalling...")] + [TestCase(false, true, false, false, false, false, false, null, "3.0.0", "Installing...")] + [TestCase(false, false, true, false, false, false, false, null, "3.0.0", "Checking...")] public void GetInstallCliButtonText_ReturnsExpectedText( bool isCliInstalled, bool isInstallingCli, @@ -28,6 +30,7 @@ public void GetInstallCliButtonText_ReturnsExpectedText( bool needsUpdate, bool needsDowngrade, bool canUninstallCli, + bool needsCliPathSetup, string cliVersion, string requiredCliVersion, string expectedText) @@ -39,6 +42,7 @@ public void GetInstallCliButtonText_ReturnsExpectedText( needsUpdate, needsDowngrade, canUninstallCli, + needsCliPathSetup, cliVersion, requiredCliVersion); @@ -191,6 +195,7 @@ private static CliSetupData CreateData( needsUpdate: false, needsDowngrade: false, canUninstallCli: true, + needsCliPathSetup: false, isInstallingCli: false, isChecking, isClaudeSkillsInstalled: false, diff --git a/Assets/Tests/Editor/SetupWizardWindowTests.cs b/Assets/Tests/Editor/SetupWizardWindowTests.cs index 232aafa50..633bc970b 100644 --- a/Assets/Tests/Editor/SetupWizardWindowTests.cs +++ b/Assets/Tests/Editor/SetupWizardWindowTests.cs @@ -357,16 +357,19 @@ public void ShouldShowSkillsTargetList_WhenCliInstalledAndNotFirstInstall_Return 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)")] - [TestCase(true, true, false, false, "3.0.0", "3.0.0", "Installing...")] - [TestCase(false, false, true, false, null, "3.0.0", "Checking...")] + [TestCase(false, false, false, false, false, null, "3.0.0", "Install CLI")] + [TestCase(false, false, false, false, true, null, "3.0.0", "Fix PATH")] + [TestCase(true, false, false, false, false, "3.0.0", "3.0.0", "Installed")] + [TestCase(true, false, false, false, true, "3.0.0", "3.0.0", "Fix PATH")] + [TestCase(true, false, false, true, false, "2.9.0", "3.0.0", "Update CLI (v2.9.0 \u2192 v3.0.0)")] + [TestCase(true, true, false, false, false, "3.0.0", "3.0.0", "Installing...")] + [TestCase(false, false, true, false, false, null, "3.0.0", "Checking...")] public void GetCliButtonTextForSetupWizard_ReturnsExpectedLabel( bool cliInstalled, bool isInstallingCli, bool isChecking, bool needsUpdate, + bool needsCliPathSetup, string cliVersion, string requiredCliVersion, string expectedLabel) @@ -376,20 +379,23 @@ public void GetCliButtonTextForSetupWizard_ReturnsExpectedLabel( isInstallingCli, isChecking, needsUpdate, + needsCliPathSetup, cliVersion, requiredCliVersion); Assert.That(label, Is.EqualTo(expectedLabel)); } - [TestCase(false, false, false, false, true)] - [TestCase(true, false, false, false, true)] - [TestCase(true, true, false, false, false)] - [TestCase(false, false, true, false, false)] - [TestCase(false, false, false, true, false)] + [TestCase(false, false, false, false, false, true)] + [TestCase(true, false, true, false, false, true)] + [TestCase(true, false, false, false, false, true)] + [TestCase(true, true, false, false, false, false)] + [TestCase(false, false, false, true, false, false)] + [TestCase(false, false, false, false, true, false)] public void IsCliButtonEnabledForSetupWizard_ReturnsExpectedValue( bool cliInstalled, bool cliVersionMatched, + bool needsCliPathSetup, bool isInstallingCli, bool isChecking, bool expectedEnabled) @@ -397,12 +403,41 @@ public void IsCliButtonEnabledForSetupWizard_ReturnsExpectedValue( bool enabled = SetupWizardWindow.IsCliButtonEnabledForSetupWizard( cliInstalled, cliVersionMatched, + needsCliPathSetup, isInstallingCli, isChecking); Assert.That(enabled, Is.EqualTo(expectedEnabled)); } + [TestCase(false, false)] + [TestCase(true, true)] + public void ShouldRepairCliPathFromPrimaryButton_ReturnsExpectedAction( + bool needsCliPathSetup, + bool expected) + { + // Verifies that setup wizard chooses PATH repair for installed terminal-invisible CLIs. + bool result = SetupWizardWindow.ShouldRepairCliPathFromPrimaryButton(needsCliPathSetup); + + Assert.That(result, Is.EqualTo(expected)); + } + + [TestCase(RuntimePlatform.OSXEditor, true, true)] + [TestCase(RuntimePlatform.OSXEditor, false, false)] + [TestCase(RuntimePlatform.WindowsEditor, true, false)] + public void ShouldCheckCliPathSetupForSetupWizard_RequiresPackageOwnedCli( + RuntimePlatform platform, + bool hasPackageOwnedCurrentUserInstall, + bool expected) + { + // Verifies that setup wizard only repairs PATH for package-owned POSIX CLIs. + bool result = SetupWizardWindow.ShouldCheckCliPathSetupForSetupWizard( + platform, + hasPackageOwnedCurrentUserInstall); + + Assert.That(result, Is.EqualTo(expected)); + } + [Test] public void CreateFirstInstallSkillTarget_WhenClaudeSelected_ReturnsClaudeProjectTarget() { diff --git a/Assets/Tests/Editor/SkillsTargetSelectionResolverTests.cs b/Assets/Tests/Editor/SkillsTargetSelectionResolverTests.cs index 7f3bfa41f..9f940635c 100644 --- a/Assets/Tests/Editor/SkillsTargetSelectionResolverTests.cs +++ b/Assets/Tests/Editor/SkillsTargetSelectionResolverTests.cs @@ -50,6 +50,7 @@ public void IsInstalled_ReturnsExpectedStateForTarget( needsUpdate: false, needsDowngrade: false, canUninstallCli: true, + needsCliPathSetup: false, isInstallingCli: false, isChecking: false, isClaudeSkillsInstalled: true, diff --git a/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs b/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs index 8d90016da..244cac40a 100644 --- a/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs +++ b/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs @@ -1,4 +1,5 @@ using NUnit.Framework; +using UnityEngine; using io.github.hatayama.UnityCliLoop.Presentation; @@ -29,6 +30,35 @@ public void ShouldUninstallCliFromPrimaryButton_ReturnsExpectedAction( Assert.That(result, Is.EqualTo(expected)); } + [TestCase(false, false)] + [TestCase(true, true)] + public void ShouldRepairCliPathFromPrimaryButton_ReturnsExpectedAction( + bool needsCliPathSetup, + bool expected) + { + // Verifies that stale terminal PATH state routes the primary button to repair before uninstall. + bool result = UnityCliLoopSettingsWindow.ShouldRepairCliPathFromPrimaryButton(needsCliPathSetup); + + Assert.That(result, Is.EqualTo(expected)); + } + + [TestCase(RuntimePlatform.OSXEditor, true, true)] + [TestCase(RuntimePlatform.OSXEditor, false, false)] + [TestCase(RuntimePlatform.LinuxEditor, true, true)] + [TestCase(RuntimePlatform.WindowsEditor, true, false)] + public void ShouldCheckCliPathSetupForPlatform_RequiresPackageOwnedCli( + RuntimePlatform platform, + bool hasPackageOwnedCurrentUserInstall, + bool expected) + { + // Verifies that PATH repair only runs for POSIX package-owned current-user installs. + bool result = UnityCliLoopSettingsWindow.ShouldCheckCliPathSetupForPlatform( + platform, + hasPackageOwnedCurrentUserInstall); + + Assert.That(result, Is.EqualTo(expected)); + } + [TestCase("3.0.0-beta.0", "3.0.0-beta.1", true)] [TestCase("3.0.0-beta.1", "3.0.0-beta.1", false)] [TestCase("3.0.0", "3.0.0-beta.1", false)] diff --git a/Packages/src/Editor/Application/CliPathSetupPlan.cs b/Packages/src/Editor/Application/CliPathSetupPlan.cs new file mode 100644 index 000000000..e62ff66b4 --- /dev/null +++ b/Packages/src/Editor/Application/CliPathSetupPlan.cs @@ -0,0 +1,114 @@ +using UnityEngine; + +namespace io.github.hatayama.UnityCliLoop.Application +{ + /// + /// Carries the shell-specific target needed to add the native CLI install directory to PATH. + /// + public readonly struct CliPathSetupPlan + { + public CliPathSetupPlan( + CliPathSetupShellKind shellKind, + string shellName, + bool canApplyAutomatically, + string installDirectory, + string profileInstallDirectory, + string configurationFilePath, + string configurationLine, + string manualCommand) + { + Debug.Assert(!string.IsNullOrWhiteSpace(installDirectory), "installDirectory must not be null or empty"); + Debug.Assert(!string.IsNullOrWhiteSpace(profileInstallDirectory), "profileInstallDirectory must not be null or empty"); + Debug.Assert(!string.IsNullOrWhiteSpace(manualCommand), "manualCommand must not be null or empty"); + if (canApplyAutomatically) + { + Debug.Assert(!string.IsNullOrWhiteSpace(configurationFilePath), "configurationFilePath must not be null or empty"); + Debug.Assert(!string.IsNullOrWhiteSpace(configurationLine), "configurationLine must not be null or empty"); + } + + ShellKind = shellKind; + ShellName = shellName ?? string.Empty; + CanApplyAutomatically = canApplyAutomatically; + InstallDirectory = installDirectory; + ProfileInstallDirectory = profileInstallDirectory; + ConfigurationFilePath = configurationFilePath ?? string.Empty; + ConfigurationLine = configurationLine ?? string.Empty; + ManualCommand = manualCommand; + } + + public CliPathSetupShellKind ShellKind { get; } + public string ShellName { get; } + public bool CanApplyAutomatically { get; } + public string InstallDirectory { get; } + public string ProfileInstallDirectory { get; } + public string ConfigurationFilePath { get; } + public string ConfigurationLine { get; } + public string ManualCommand { get; } + } + + public enum CliPathSetupShellKind + { + Unsupported, + Zsh, + Bash, + Fish + } + + public enum CliPathSetupApplyStatus + { + Applied, + AlreadyConfigured, + Unsupported, + Failed + } + + /// + /// Reports the outcome of the profile append step, before terminal visibility is checked again. + /// + public readonly struct CliPathSetupApplyResult + { + public CliPathSetupApplyResult( + bool success, + CliPathSetupApplyStatus status, + string errorOutput) + { + Success = success; + Status = status; + ErrorOutput = errorOutput ?? string.Empty; + } + + public bool Success { get; } + public CliPathSetupApplyStatus Status { get; } + public string ErrorOutput { get; } + } + + public enum CliPathSetupFlowStatus + { + AlreadyVisible, + AppliedAndVisible, + AlreadyConfiguredAndVisible, + ManualSetupRequired, + AppliedButStillMissing, + Failed + } + + /// + /// Carries the user-facing result of making the CLI visible from a terminal shell. + /// + public readonly struct CliPathSetupFlowResult + { + public CliPathSetupFlowResult( + CliPathSetupFlowStatus status, + CliPathSetupPlan plan, + string errorOutput) + { + Status = status; + Plan = plan; + ErrorOutput = errorOutput ?? string.Empty; + } + + public CliPathSetupFlowStatus Status { get; } + public CliPathSetupPlan Plan { get; } + public string ErrorOutput { get; } + } +} diff --git a/Packages/src/Editor/Application/CliPathSetupPlan.cs.meta b/Packages/src/Editor/Application/CliPathSetupPlan.cs.meta new file mode 100644 index 000000000..e7f771389 --- /dev/null +++ b/Packages/src/Editor/Application/CliPathSetupPlan.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 43c69a1e3b3fb4d95b1228673e939504 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Application/CliSetupApplicationService.cs b/Packages/src/Editor/Application/CliSetupApplicationService.cs index 3b9ec4e2a..d141eea0e 100644 --- a/Packages/src/Editor/Application/CliSetupApplicationService.cs +++ b/Packages/src/Editor/Application/CliSetupApplicationService.cs @@ -37,6 +37,7 @@ public interface ICliInstallationDetector bool IsCheckCompleted(); Task RefreshCliVersionAsync(CancellationToken ct); Task ForceRefreshCliVersionAsync(CancellationToken ct); + Task IsCliVisibleFromShellAsync(RuntimePlatform platform, CancellationToken ct); void InvalidateCache(); } @@ -46,8 +47,11 @@ public interface ICliInstallationDetector public interface INativeCliInstaller { bool IsPackageOwnedCurrentUserInstallPath(string cliExecutablePath, RuntimePlatform platform); + bool HasPackageOwnedCurrentUserInstall(RuntimePlatform platform); Task InstallGlobalCliAsync(RuntimePlatform platform, string cliReleaseTag, CancellationToken ct); Task UninstallGlobalCliAsync(RuntimePlatform platform, CancellationToken ct); + Task GetGlobalCliPathSetupPlanAsync(RuntimePlatform platform, CancellationToken ct); + CliPathSetupApplyResult ApplyGlobalCliPathSetup(CliPathSetupPlan plan); NativeCliInstallCommand GetGlobalCliInstallCommand( RuntimePlatform platform, string cliReleaseTag, @@ -103,6 +107,11 @@ public Task ForceRefreshCliVersionAsync(CancellationToken ct) return _cliInstallationDetector.ForceRefreshCliVersionAsync(ct); } + public Task IsCliVisibleFromShellAsync(RuntimePlatform platform, CancellationToken ct) + { + return _cliInstallationDetector.IsCliVisibleFromShellAsync(platform, ct); + } + public void InvalidateCliCache() { _cliInstallationDetector.InvalidateCache(); @@ -125,6 +134,11 @@ public bool IsPackageOwnedCurrentUserInstallPath( return _nativeCliInstaller.IsPackageOwnedCurrentUserInstallPath(cliExecutablePath, platform); } + public bool HasPackageOwnedCurrentUserInstall(RuntimePlatform platform) + { + return _nativeCliInstaller.HasPackageOwnedCurrentUserInstall(platform); + } + public bool IsCliVersionLessThan(string leftVersion, string rightVersion) { return CliVersionComparer.IsVersionLessThan(leftVersion, rightVersion); @@ -147,6 +161,55 @@ public async Task InstallGlobalCliAsync(RuntimePlatform platfo return result; } + public async Task EnsureCliVisibleFromShellAsync( + RuntimePlatform platform, + CancellationToken ct) + { + ct.ThrowIfCancellationRequested(); + + bool isVisible = await _cliInstallationDetector.IsCliVisibleFromShellAsync(platform, ct); + if (isVisible) + { + return new CliPathSetupFlowResult( + CliPathSetupFlowStatus.AlreadyVisible, + default, + ""); + } + + CliPathSetupPlan plan = await _nativeCliInstaller.GetGlobalCliPathSetupPlanAsync(platform, ct); + if (!plan.CanApplyAutomatically) + { + return new CliPathSetupFlowResult( + CliPathSetupFlowStatus.ManualSetupRequired, + plan, + ""); + } + + CliPathSetupApplyResult applyResult = _nativeCliInstaller.ApplyGlobalCliPathSetup(plan); + if (!applyResult.Success) + { + return new CliPathSetupFlowResult( + CliPathSetupFlowStatus.Failed, + plan, + applyResult.ErrorOutput); + } + + _cliInstallationDetector.InvalidateCache(); + bool isVisibleAfterApply = await _cliInstallationDetector.IsCliVisibleFromShellAsync(platform, ct); + if (!isVisibleAfterApply) + { + return new CliPathSetupFlowResult( + CliPathSetupFlowStatus.AppliedButStillMissing, + plan, + ""); + } + + CliPathSetupFlowStatus status = applyResult.Status == CliPathSetupApplyStatus.AlreadyConfigured + ? CliPathSetupFlowStatus.AlreadyConfiguredAndVisible + : CliPathSetupFlowStatus.AppliedAndVisible; + return new CliPathSetupFlowResult(status, plan, ""); + } + public async Task UninstallGlobalCliAsync(RuntimePlatform platform, CancellationToken ct) { ct.ThrowIfCancellationRequested(); @@ -221,6 +284,11 @@ public static Task ForceRefreshCliVersionAsync(CancellationToken ct) return GetService().ForceRefreshCliVersionAsync(ct); } + public static Task IsCliVisibleFromShellAsync(RuntimePlatform platform, CancellationToken ct) + { + return GetService().IsCliVisibleFromShellAsync(platform, ct); + } + public static void InvalidateCliCache() { GetService().InvalidateCliCache(); @@ -243,6 +311,11 @@ public static bool IsPackageOwnedCurrentUserInstallPath( return GetService().IsPackageOwnedCurrentUserInstallPath(cliExecutablePath, platform); } + public static bool HasPackageOwnedCurrentUserInstall(RuntimePlatform platform) + { + return GetService().HasPackageOwnedCurrentUserInstall(platform); + } + public static bool IsCliVersionLessThan(string leftVersion, string rightVersion) { return GetService().IsCliVersionLessThan(leftVersion, rightVersion); @@ -258,6 +331,13 @@ public static Task InstallGlobalCliAsync(RuntimePlatform platf return GetService().InstallGlobalCliAsync(platform, ct); } + public static Task EnsureCliVisibleFromShellAsync( + RuntimePlatform platform, + CancellationToken ct) + { + return GetService().EnsureCliVisibleFromShellAsync(platform, ct); + } + public static Task UninstallGlobalCliAsync(RuntimePlatform platform, CancellationToken ct) { return GetService().UninstallGlobalCliAsync(platform, ct); diff --git a/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs b/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs index 41272e3d0..088e55a2c 100644 --- a/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs +++ b/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs @@ -92,6 +92,25 @@ public async Task ForceRefreshCliVersionAsync(CancellationToken ct) _cacheInitialized = true; } + public Task IsCliVisibleFromShellAsync(RuntimePlatform platform, CancellationToken ct) + { + if (platform == RuntimePlatform.WindowsEditor) + { + return Task.FromResult(true); + } + + return Task.Run( + () => + { + CliInstallationDetection detection = DetectShellCliInstallationBlocking(platform, ct); + return IsShellDetectionUsableForPathSetup( + detection, + platform, + NativeCliInstaller.IsPackageOwnedCurrentUserInstallPath); + }, + ct); + } + public void InvalidateCache() { _cachedCliVersion = null; @@ -145,7 +164,7 @@ private static CliInstallationDetection DetectShellCliInstallationBlocking(Runti { if (platform != RuntimePlatform.WindowsEditor) { - return DetectShellCliInstallationFromLoginShell(ct); + return DetectShellCliInstallationFromLoginShell(platform, ct); } string executablePath = NodeEnvironmentResolver.FindExecutablePathAtPlatform( @@ -154,21 +173,78 @@ private static CliInstallationDetection DetectShellCliInstallationBlocking(Runti return DetectCliInstallationAtExecutablePath(executablePath, ct); } - private static CliInstallationDetection DetectShellCliInstallationFromLoginShell(CancellationToken ct) + private static CliInstallationDetection DetectShellCliInstallationFromLoginShell(RuntimePlatform platform, CancellationToken ct) { string shell = NodeEnvironmentResolver.GetUserShell(); + CliPathSetupPlan pathSetupPlan = CliPathSetupProfileResolver.ResolveCurrentUserPlan(platform); + ProcessStartInfo startInfo = BuildShellCliDetectionStartInfo( + shell, + platform, + pathSetupPlan, + Environment.GetEnvironmentVariable(CliConstants.POSIX_PATH_ENVIRONMENT_VARIABLE)); + + string output = ExecuteAndGetOutput(startInfo, ct); + return ParseShellCliInstallationOutput(output); + } + + internal static ProcessStartInfo BuildShellCliDetectionStartInfo( + string shell, + RuntimePlatform platform, + CliPathSetupPlan pathSetupPlan, + string currentPath) + { + UnityEngine.Debug.Assert(!string.IsNullOrWhiteSpace(shell), "shell must not be null or empty"); + ProcessStartInfo startInfo = new() { FileName = shell, - Arguments = "-l -i -c " + QuoteProcessArgument(BuildShellCliDetectionCommand(CliConstants.EXECUTABLE_NAME)), + Arguments = "-l -i -c " + QuoteProcessArgument(BuildShellCliDetectionCommandForPlan( + CliConstants.EXECUTABLE_NAME, + pathSetupPlan)), UseShellExecute = false, RedirectStandardOutput = true, RedirectStandardError = true, CreateNoWindow = true }; + if (platform != RuntimePlatform.WindowsEditor) + { + startInfo.EnvironmentVariables[CliConstants.POSIX_PATH_ENVIRONMENT_VARIABLE] = + NativeCliInstaller.BuildPathWithoutInstallDirectory( + currentPath, + pathSetupPlan.InstallDirectory, + platform); + } - string output = ExecuteAndGetOutput(startInfo, ct); - return ParseShellCliInstallationOutput(output); + return startInfo; + } + + internal static bool IsShellDetectionUsableForPathSetup( + CliInstallationDetection detection, + RuntimePlatform platform, + Func isPackageOwnedCurrentUserInstallPath) + { + UnityEngine.Debug.Assert(isPackageOwnedCurrentUserInstallPath != null, "isPackageOwnedCurrentUserInstallPath must not be null"); + + if (isPackageOwnedCurrentUserInstallPath(detection.ExecutablePath, platform)) + { + return true; + } + + return CliVersionComparer.IsVersionGreaterThanOrEqual( + detection.Version, + CliConstants.MINIMUM_REQUIRED_CLI_VERSION); + } + + internal static string BuildShellCliDetectionCommandForPlan( + string executableName, + CliPathSetupPlan pathSetupPlan) + { + if (pathSetupPlan.ShellKind == CliPathSetupShellKind.Fish) + { + return BuildFishShellCliDetectionCommand(executableName); + } + + return BuildShellCliDetectionCommand(executableName); } internal static string BuildShellCliDetectionCommand(string executableName) @@ -187,6 +263,22 @@ internal static string BuildShellCliDetectionCommand(string executableName) + "echo " + SHELL_VERSION_STATUS_END_MARKER; } + private static string BuildFishShellCliDetectionCommand(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" + + "set uloop_version_status $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) { string pathBlock = NodeEnvironmentResolver.ExtractBetweenMarkers( diff --git a/Packages/src/Editor/Infrastructure/CLI/CliPathSetupProfileResolver.cs b/Packages/src/Editor/Infrastructure/CLI/CliPathSetupProfileResolver.cs new file mode 100644 index 000000000..3387ac9e3 --- /dev/null +++ b/Packages/src/Editor/Infrastructure/CLI/CliPathSetupProfileResolver.cs @@ -0,0 +1,251 @@ +using System; +using System.IO; +using System.Text; + +using UnityEngine; + +using io.github.hatayama.UnityCliLoop.Application; +using io.github.hatayama.UnityCliLoop.Domain; +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.Infrastructure +{ + /// + /// Selects the shell profile target for the single PATH line owned by Unity CLI Loop. + /// + internal static class CliPathSetupProfileResolver + { + private const string HomeReference = "$HOME"; + private const string ZshProfileFileName = ".zshrc"; + private const string BashProfileFileName = ".bash_profile"; + private const string BashLoginFileName = ".bash_login"; + private const string PosixProfileFileName = ".profile"; + private const string FishConfigDirectoryName = "fish"; + private const string FishConfigFileName = "config.fish"; + private const string DefaultXdgConfigDirectoryName = ".config"; + + public static CliPathSetupPlan ResolveCurrentUserPlan(RuntimePlatform platform) + { + string installDirectory = NativeCliInstaller.GetCurrentUserGlobalCliInstallDirectory(platform); + if (string.IsNullOrWhiteSpace(installDirectory)) + { + return CreateUnsupportedPlan("unknown", CliConstants.EXECUTABLE_NAME); + } + + return ResolvePlan( + platform, + NodeEnvironmentResolver.GetUserShell(), + Environment.GetEnvironmentVariable(CliConstants.POSIX_HOME_ENVIRONMENT_VARIABLE), + Environment.GetEnvironmentVariable("ZDOTDIR"), + Environment.GetEnvironmentVariable("XDG_CONFIG_HOME"), + installDirectory, + File.Exists); + } + + internal static CliPathSetupPlan ResolvePlan( + RuntimePlatform platform, + string shellPath, + string homeDirectory, + string zDotDirectory, + string xdgConfigHome, + string installDirectory, + Func fileExists) + { + Debug.Assert(!string.IsNullOrWhiteSpace(installDirectory), "installDirectory must not be null or empty"); + Debug.Assert(fileExists != null, "fileExists must not be null"); + + if (platform == RuntimePlatform.WindowsEditor) + { + return CreateUnsupportedPlan("windows", installDirectory); + } + + string resolvedHomeDirectory = string.IsNullOrWhiteSpace(homeDirectory) + ? Environment.GetFolderPath(Environment.SpecialFolder.UserProfile) + : homeDirectory; + string shellName = GetShellName(shellPath); + string profileInstallDirectory = FormatInstallDirectoryForProfile( + installDirectory, + resolvedHomeDirectory); + + if (string.Equals(shellName, "zsh", StringComparison.Ordinal)) + { + string configurationRoot = string.IsNullOrWhiteSpace(zDotDirectory) + ? resolvedHomeDirectory + : zDotDirectory; + string configurationPath = Path.Combine(configurationRoot, ZshProfileFileName); + string configurationLine = BuildPosixExportLine(profileInstallDirectory); + return CreateSupportedPlan( + CliPathSetupShellKind.Zsh, + shellName, + installDirectory, + profileInstallDirectory, + configurationPath, + configurationLine); + } + + if (string.Equals(shellName, "bash", StringComparison.Ordinal)) + { + string configurationPath = SelectBashProfilePath(resolvedHomeDirectory, fileExists); + string configurationLine = BuildPosixExportLine(profileInstallDirectory); + return CreateSupportedPlan( + CliPathSetupShellKind.Bash, + shellName, + installDirectory, + profileInstallDirectory, + configurationPath, + configurationLine); + } + + if (string.Equals(shellName, "fish", StringComparison.Ordinal)) + { + string configRoot = string.IsNullOrWhiteSpace(xdgConfigHome) + ? Path.Combine(resolvedHomeDirectory, DefaultXdgConfigDirectoryName) + : xdgConfigHome; + string configurationPath = Path.Combine( + configRoot, + FishConfigDirectoryName, + FishConfigFileName); + string configurationLine = $"fish_add_path \"{EscapeDoubleQuotedPathValue(profileInstallDirectory, false)}\""; + return CreateSupportedPlan( + CliPathSetupShellKind.Fish, + shellName, + installDirectory, + profileInstallDirectory, + configurationPath, + configurationLine); + } + + return CreateUnsupportedPlan( + string.IsNullOrWhiteSpace(shellName) ? "unknown" : shellName, + installDirectory); + } + + private static CliPathSetupPlan CreateSupportedPlan( + CliPathSetupShellKind shellKind, + string shellName, + string installDirectory, + string profileInstallDirectory, + string configurationPath, + string configurationLine) + { + return new CliPathSetupPlan( + shellKind, + shellName, + true, + installDirectory, + profileInstallDirectory, + configurationPath, + configurationLine, + BuildManualCommand(configurationPath, configurationLine)); + } + + private static CliPathSetupPlan CreateUnsupportedPlan(string shellName, string installDirectory) + { + return new CliPathSetupPlan( + CliPathSetupShellKind.Unsupported, + shellName, + false, + installDirectory, + installDirectory, + "", + "", + "export PATH=" + QuotePosixShellValue(installDirectory) + ":\"$PATH\""); + } + + private static string SelectBashProfilePath(string homeDirectory, Func fileExists) + { + string bashProfilePath = Path.Combine(homeDirectory, BashProfileFileName); + if (fileExists(bashProfilePath)) + { + return bashProfilePath; + } + + string bashLoginPath = Path.Combine(homeDirectory, BashLoginFileName); + if (fileExists(bashLoginPath)) + { + return bashLoginPath; + } + + string profilePath = Path.Combine(homeDirectory, PosixProfileFileName); + return fileExists(profilePath) ? profilePath : bashProfilePath; + } + + private static string GetShellName(string shellPath) + { + return string.IsNullOrWhiteSpace(shellPath) + ? string.Empty + : Path.GetFileName(shellPath.Trim()).ToLowerInvariant(); + } + + private static string FormatInstallDirectoryForProfile(string installDirectory, string homeDirectory) + { + if (string.IsNullOrWhiteSpace(homeDirectory)) + { + return installDirectory; + } + + string normalizedHome = homeDirectory.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); + if (string.Equals(installDirectory, normalizedHome, StringComparison.Ordinal)) + { + return HomeReference; + } + + string homePrefix = normalizedHome + Path.DirectorySeparatorChar; + return installDirectory.StartsWith(homePrefix, StringComparison.Ordinal) + ? HomeReference + "/" + installDirectory.Substring(homePrefix.Length) + : installDirectory; + } + + private static string BuildPosixExportLine(string installDirectory) + { + return $"export PATH=\"{EscapeDoubleQuotedPathValue(installDirectory, true)}:$PATH\""; + } + + private static string EscapeDoubleQuotedPathValue(string value, bool escapeBacktick) + { + StringBuilder builder = new StringBuilder(); + int cursor = 0; + if (string.Equals(value, HomeReference, StringComparison.Ordinal) + || value.StartsWith(HomeReference + "/", StringComparison.Ordinal)) + { + builder.Append(HomeReference); + cursor = HomeReference.Length; + } + + while (cursor < value.Length) + { + char character = value[cursor]; + if (character == '\\' + || character == '"' + || character == '$' + || (escapeBacktick && character == '`')) + { + builder.Append('\\'); + } + + builder.Append(character); + cursor++; + } + + return builder.ToString(); + } + + private static string BuildManualCommand(string configurationPath, string configurationLine) + { + string configurationDirectory = Path.GetDirectoryName(configurationPath); + string appendCommand = "printf '\\n%s\\n' " + + $"{QuotePosixShellValue(configurationLine)} >> {QuotePosixShellValue(configurationPath)}"; + if (string.IsNullOrEmpty(configurationDirectory)) + { + return appendCommand; + } + + return $"mkdir -p {QuotePosixShellValue(configurationDirectory)} && " + appendCommand; + } + + private static string QuotePosixShellValue(string value) + { + return $"'{value.Replace("'", "'\"'\"'")}'"; + } + } +} diff --git a/Packages/src/Editor/Infrastructure/CLI/CliPathSetupProfileResolver.cs.meta b/Packages/src/Editor/Infrastructure/CLI/CliPathSetupProfileResolver.cs.meta new file mode 100644 index 000000000..c5251697e --- /dev/null +++ b/Packages/src/Editor/Infrastructure/CLI/CliPathSetupProfileResolver.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: a273da369deb7428bbea909bdb47fd54 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Infrastructure/CLI/CliPathSetupWriter.cs b/Packages/src/Editor/Infrastructure/CLI/CliPathSetupWriter.cs new file mode 100644 index 000000000..4c483f308 --- /dev/null +++ b/Packages/src/Editor/Infrastructure/CLI/CliPathSetupWriter.cs @@ -0,0 +1,159 @@ +using System; +using System.Collections.Generic; +using System.IO; + +using UnityEngine; + +using io.github.hatayama.UnityCliLoop.Application; + +namespace io.github.hatayama.UnityCliLoop.Infrastructure +{ + /// + /// Appends the single PATH setup line owned by Unity CLI Loop without interpreting full shell startup semantics. + /// + internal static class CliPathSetupWriter + { + public static CliPathSetupApplyResult ApplyToFileSystem(CliPathSetupPlan plan) + { + return Apply( + plan, + File.Exists, + File.ReadAllText, + Directory.CreateDirectory, + File.AppendAllText); + } + + internal static CliPathSetupApplyResult Apply( + CliPathSetupPlan plan, + Func fileExists, + Func readAllText, + Func createDirectory, + Action appendAllText) + { + Debug.Assert(fileExists != null, "fileExists must not be null"); + Debug.Assert(readAllText != null, "readAllText must not be null"); + Debug.Assert(createDirectory != null, "createDirectory must not be null"); + Debug.Assert(appendAllText != null, "appendAllText must not be null"); + + if (!plan.CanApplyAutomatically) + { + return new CliPathSetupApplyResult( + false, + CliPathSetupApplyStatus.Unsupported, + "This shell is not supported for automatic PATH setup."); + } + + string existingContent = fileExists(plan.ConfigurationFilePath) + ? readAllText(plan.ConfigurationFilePath) + : string.Empty; + if (ContainsExistingPathSetup(existingContent, plan)) + { + return new CliPathSetupApplyResult( + true, + CliPathSetupApplyStatus.AlreadyConfigured, + ""); + } + + string configurationDirectory = Path.GetDirectoryName(plan.ConfigurationFilePath); + if (!string.IsNullOrEmpty(configurationDirectory)) + { + createDirectory(configurationDirectory); + } + + string prefix = NeedsLeadingNewLine(existingContent) ? "\n" : string.Empty; + appendAllText( + plan.ConfigurationFilePath, + prefix + plan.ConfigurationLine + "\n"); + return new CliPathSetupApplyResult( + true, + CliPathSetupApplyStatus.Applied, + ""); + } + + internal static bool ContainsExistingPathSetup(string content, CliPathSetupPlan plan) + { + if (string.IsNullOrEmpty(content)) + { + return false; + } + + string[] references = BuildReferenceCandidates(plan); + string[] lines = content.Replace("\r\n", "\n").Split('\n'); + foreach (string line in lines) + { + string trimmedLine = line.Trim(); + if (string.IsNullOrEmpty(trimmedLine) + || trimmedLine.StartsWith("#", StringComparison.Ordinal)) + { + continue; + } + + if (string.Equals(trimmedLine, plan.ConfigurationLine, StringComparison.Ordinal)) + { + return true; + } + + if (LooksLikePathSetupLine(trimmedLine, plan.ShellKind) + && ContainsAnyReference(trimmedLine, references)) + { + return true; + } + } + + return false; + } + + private static string[] BuildReferenceCandidates(CliPathSetupPlan plan) + { + List candidates = new List + { + plan.InstallDirectory, + plan.ProfileInstallDirectory + }; + + if (plan.ProfileInstallDirectory.StartsWith("$HOME/", StringComparison.Ordinal)) + { + string suffix = plan.ProfileInstallDirectory.Substring("$HOME".Length); + candidates.Add("${HOME}" + suffix); + candidates.Add("~" + suffix); + } + + return candidates.ToArray(); + } + + private static bool LooksLikePathSetupLine(string line, CliPathSetupShellKind shellKind) + { + if (shellKind == CliPathSetupShellKind.Fish) + { + return line.StartsWith("fish_add_path", StringComparison.Ordinal) + || (line.StartsWith("set", StringComparison.Ordinal) + && line.Contains("PATH")); + } + + return line.StartsWith("PATH=", StringComparison.Ordinal) + || line.StartsWith("PATH =", StringComparison.Ordinal) + || line.StartsWith("export PATH=", StringComparison.Ordinal) + || line.StartsWith("export PATH =", StringComparison.Ordinal); + } + + private static bool ContainsAnyReference(string line, string[] references) + { + foreach (string reference in references) + { + if (!string.IsNullOrWhiteSpace(reference) + && line.Contains(reference)) + { + return true; + } + } + + return false; + } + + private static bool NeedsLeadingNewLine(string content) + { + return !string.IsNullOrEmpty(content) + && !content.EndsWith("\n", StringComparison.Ordinal); + } + } +} diff --git a/Packages/src/Editor/Infrastructure/CLI/CliPathSetupWriter.cs.meta b/Packages/src/Editor/Infrastructure/CLI/CliPathSetupWriter.cs.meta new file mode 100644 index 000000000..78ff5e01d --- /dev/null +++ b/Packages/src/Editor/Infrastructure/CLI/CliPathSetupWriter.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 9b1d19098368d4a3a925bb80fdcece03 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Infrastructure/CLI/NativeCliInstaller.cs b/Packages/src/Editor/Infrastructure/CLI/NativeCliInstaller.cs index d24e09044..c21c1a11f 100644 --- a/Packages/src/Editor/Infrastructure/CLI/NativeCliInstaller.cs +++ b/Packages/src/Editor/Infrastructure/CLI/NativeCliInstaller.cs @@ -680,6 +680,17 @@ internal static string GetCurrentUserGlobalCliInstallPath(RuntimePlatform platfo : GetGlobalCliInstallPath(installDirectory, platform); } + internal static string GetCurrentUserGlobalCliInstallDirectory(RuntimePlatform platform) + { + return GetInstallDirectoryForCurrentUser(platform); + } + + internal static bool HasPackageOwnedCurrentUserInstall(RuntimePlatform platform) + { + string executablePath = GetCurrentUserGlobalCliInstallPath(platform); + return !string.IsNullOrWhiteSpace(executablePath) && File.Exists(executablePath); + } + private static string GetInstallDirectoryForCurrentUser(RuntimePlatform platform) { string configuredInstallDirectory = Environment.GetEnvironmentVariable(CliConstants.INSTALL_DIR_ENVIRONMENT_VARIABLE); @@ -1087,6 +1098,11 @@ public bool IsPackageOwnedCurrentUserInstallPath(string cliExecutablePath, Runti return NativeCliInstaller.IsPackageOwnedCurrentUserInstallPath(cliExecutablePath, platform); } + public bool HasPackageOwnedCurrentUserInstall(RuntimePlatform platform) + { + return NativeCliInstaller.HasPackageOwnedCurrentUserInstall(platform); + } + public Task InstallGlobalCliAsync( RuntimePlatform platform, string cliReleaseTag, @@ -1102,6 +1118,17 @@ public Task UninstallGlobalCliAsync(RuntimePlatform platform, return NativeCliInstaller.UninstallAsync(platform, ct); } + public Task GetGlobalCliPathSetupPlanAsync(RuntimePlatform platform, CancellationToken ct) + { + ct.ThrowIfCancellationRequested(); + return Task.FromResult(CliPathSetupProfileResolver.ResolveCurrentUserPlan(platform)); + } + + public CliPathSetupApplyResult ApplyGlobalCliPathSetup(CliPathSetupPlan plan) + { + return CliPathSetupWriter.ApplyToFileSystem(plan); + } + public NativeCliInstallCommand GetGlobalCliInstallCommand( RuntimePlatform platform, string cliReleaseTag, diff --git a/Packages/src/Editor/Presentation/CliPathSetupPrompt.cs b/Packages/src/Editor/Presentation/CliPathSetupPrompt.cs new file mode 100644 index 000000000..a3b0fc822 --- /dev/null +++ b/Packages/src/Editor/Presentation/CliPathSetupPrompt.cs @@ -0,0 +1,98 @@ +using System.Threading; +using System.Threading.Tasks; + +using UnityEditor; +using UnityEngine; + +using io.github.hatayama.UnityCliLoop.Application; + +namespace io.github.hatayama.UnityCliLoop.Presentation +{ + /// + /// Shows the terminal PATH setup result without owning shell profile details. + /// + internal static class CliPathSetupPrompt + { + public static async Task EnsureVisibleAndShowResultAsync( + RuntimePlatform platform, + CancellationToken ct) + { + CliPathSetupFlowResult result = + await CliSetupApplicationFacade.EnsureCliVisibleFromShellAsync(platform, ct); + ShowResult(result); + return result; + } + + internal static void ShowResult(CliPathSetupFlowResult result) + { + if (result.Status == CliPathSetupFlowStatus.AlreadyVisible) + { + return; + } + + if (result.Status == CliPathSetupFlowStatus.ManualSetupRequired) + { + bool copyCommand = EditorUtility.DisplayDialog( + "Finish uLoop CLI PATH Setup", + BuildManualSetupMessage(result.Plan), + "Copy Command", + "OK"); + if (copyCommand) + { + EditorGUIUtility.systemCopyBuffer = result.Plan.ManualCommand; + } + + return; + } + + if (result.Status == CliPathSetupFlowStatus.Failed) + { + EditorUtility.DisplayDialog( + "PATH Setup Failed", + $"Could not update your shell profile.\n\n{result.ErrorOutput}\n\n" + + $"You can run this manually:\n{result.Plan.ManualCommand}", + "OK"); + return; + } + + if (result.Status == CliPathSetupFlowStatus.AppliedButStillMissing) + { + EditorUtility.DisplayDialog( + "PATH Setup Still Needed", + "PATH setup was updated, but a fresh terminal still cannot find uloop.\n\n" + + $"Profile: {result.Plan.ConfigurationFilePath}\n\n" + + $"You can run this manually:\n{result.Plan.ManualCommand}", + "OK"); + return; + } + + EditorUtility.DisplayDialog( + "PATH Setup Complete", + BuildCompleteMessage(result), + "OK"); + } + + private static string BuildManualSetupMessage(CliPathSetupPlan plan) + { + string shellName = string.IsNullOrWhiteSpace(plan.ShellName) + ? "your shell" + : plan.ShellName; + return "The uLoop CLI was installed, but your terminal cannot find the uloop command yet.\n\n" + + $"Detected shell: {shellName}\n" + + $"Install directory: {plan.InstallDirectory}\n\n" + + "Add the install directory to PATH in your shell profile.\n\n" + + plan.ManualCommand; + } + + private static string BuildCompleteMessage(CliPathSetupFlowResult result) + { + string action = result.Status == CliPathSetupFlowStatus.AlreadyConfiguredAndVisible + ? "Your shell profile already contains the PATH setup." + : "PATH setup was updated."; + return "The uLoop CLI is now available from a fresh terminal.\n\n" + + action + "\n\n" + + $"Profile: {result.Plan.ConfigurationFilePath}\n" + + $"Line: {result.Plan.ConfigurationLine}"; + } + } +} diff --git a/Packages/src/Editor/Presentation/CliPathSetupPrompt.cs.meta b/Packages/src/Editor/Presentation/CliPathSetupPrompt.cs.meta new file mode 100644 index 000000000..75b87b7ad --- /dev/null +++ b/Packages/src/Editor/Presentation/CliPathSetupPrompt.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 63945b651d76f4f41aa1281aa7cbb63a +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs b/Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs index 4df65d1eb..4eef9045a 100644 --- a/Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs +++ b/Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs @@ -239,6 +239,7 @@ private static UnityCliLoopEditorSettingsService GetEditorSettingsService() // State private bool _isInstallingCli; private bool _isInstallingSkills; + private bool _needsCliPathSetup; private bool _isApplyingContentSize; private bool _isSkillsTargetFieldInitialized; private bool _shouldUseFirstInstallSkillsUi; @@ -492,6 +493,7 @@ private async void RefreshUI(bool refreshSkillsSection = true) string requiredCliVersion = GetMinimumRequiredCliVersion(); bool cliInstalled = IsCliInstalled(cliVersion); bool cliVersionMatched = IsCliVersionSatisfied(cliVersion, requiredCliVersion) && cliInstalled; + _needsCliPathSetup = await ShouldRepairCliPathSetupAsync(CancellationToken.None); UpdateCliStep(cliInstalled, cliVersion, requiredCliVersion, cliVersionMatched); @@ -650,11 +652,13 @@ private void UpdateCliStep( _isInstallingCli, false, needsUpdate, + _needsCliPathSetup, cliVersion, requiredCliVersion); bool buttonEnabled = IsCliButtonEnabledForSetupWizard( cliInstalled, cliVersionMatched, + _needsCliPathSetup, _isInstallingCli, isChecking: false); @@ -688,6 +692,7 @@ internal static string GetCliButtonTextForSetupWizard( bool isInstallingCli, bool isChecking, bool needsUpdate, + bool needsCliPathSetup, string cliVersion, string requiredCliVersion) { @@ -701,6 +706,11 @@ internal static string GetCliButtonTextForSetupWizard( return "Installing..."; } + if (needsCliPathSetup) + { + return "Fix PATH"; + } + if (!cliInstalled) { return "Install CLI"; @@ -717,10 +727,36 @@ internal static string GetCliButtonTextForSetupWizard( internal static bool IsCliButtonEnabledForSetupWizard( bool cliInstalled, bool cliVersionMatched, + bool needsCliPathSetup, bool isInstallingCli, bool isChecking) { - return !isInstallingCli && !isChecking && (!cliInstalled || !cliVersionMatched); + return !isInstallingCli && !isChecking && (!cliInstalled || !cliVersionMatched || needsCliPathSetup); + } + + private static async Task ShouldRepairCliPathSetupAsync(CancellationToken ct) + { + bool hasPackageOwnedCurrentUserInstall = + CliSetupApplicationFacade.HasPackageOwnedCurrentUserInstall(UnityEngine.Application.platform); + if (!ShouldCheckCliPathSetupForSetupWizard( + UnityEngine.Application.platform, + hasPackageOwnedCurrentUserInstall)) + { + return false; + } + + bool isCliVisibleFromShell = await CliSetupApplicationFacade.IsCliVisibleFromShellAsync( + UnityEngine.Application.platform, + ct); + return !isCliVisibleFromShell; + } + + internal static bool ShouldCheckCliPathSetupForSetupWizard( + RuntimePlatform platform, + bool hasPackageOwnedCurrentUserInstall) + { + return platform != RuntimePlatform.WindowsEditor + && hasPackageOwnedCurrentUserInstall; } private static bool IsCliVersionSatisfied(string cliVersion, string requiredCliVersion) @@ -935,7 +971,16 @@ internal static string GetSkillInstallStatusClass( private async void HandleInstallCli() { + await RefreshCliPrimaryActionStateAsync(CancellationToken.None); + + if (ShouldRepairCliPathFromPrimaryButton(_needsCliPathSetup)) + { + await HandleRepairCliPathSetup(); + return; + } + bool wasCliInstalledBeforeInstall = CliSetupApplicationFacade.IsCliInstalled(); + _needsCliPathSetup = false; _isInstallingCli = true; UpdateCliStep(false, null, GetMinimumRequiredCliVersion(), false); @@ -957,6 +1002,11 @@ private async void HandleInstallCli() "OK"); return; } + + await CliPathSetupPrompt.EnsureVisibleAndShowResultAsync( + UnityEngine.Application.platform, + CancellationToken.None); + _needsCliPathSetup = await ShouldRepairCliPathSetupAsync(CancellationToken.None); } finally { @@ -966,6 +1016,48 @@ private async void HandleInstallCli() } } + private async Task RefreshCliPrimaryActionStateAsync(CancellationToken ct) + { + _installCliButton.SetEnabled(false); + _installCliButton.text = "Checking..."; + + await CliSetupApplicationFacade.ForceRefreshCliVersionAsync(ct); + string cliVersion = CliSetupApplicationFacade.GetCachedCliVersion(); + string requiredCliVersion = GetMinimumRequiredCliVersion(); + bool cliInstalled = IsCliInstalled(cliVersion); + bool cliVersionMatched = IsCliVersionSatisfied(cliVersion, requiredCliVersion) && cliInstalled; + _needsCliPathSetup = await ShouldRepairCliPathSetupAsync(ct); + UpdateCliStep(cliInstalled, cliVersion, requiredCliVersion, cliVersionMatched); + } + + internal static bool ShouldRepairCliPathFromPrimaryButton(bool needsCliPathSetup) + { + return needsCliPathSetup; + } + + private async Task HandleRepairCliPathSetup() + { + _isInstallingCli = true; + UpdateCliStep( + cliInstalled: true, + cliVersion: CliSetupApplicationFacade.GetCachedCliVersion(), + requiredCliVersion: GetMinimumRequiredCliVersion(), + cliVersionMatched: true); + + try + { + await CliPathSetupPrompt.EnsureVisibleAndShowResultAsync( + UnityEngine.Application.platform, + CancellationToken.None); + _needsCliPathSetup = await ShouldRepairCliPathSetupAsync(CancellationToken.None); + } + finally + { + _isInstallingCli = false; + RefreshUI(); + } + } + private async void HandleInstallSkills() { CancelSkillInstallStateRefresh(); diff --git a/Packages/src/Editor/Presentation/UIToolkit/Components/CliSetupSection.cs b/Packages/src/Editor/Presentation/UIToolkit/Components/CliSetupSection.cs index 512697a7d..2e21bbe3c 100644 --- a/Packages/src/Editor/Presentation/UIToolkit/Components/CliSetupSection.cs +++ b/Packages/src/Editor/Presentation/UIToolkit/Components/CliSetupSection.cs @@ -118,6 +118,7 @@ private void UpdateInstallCliButton(CliSetupData data) data.NeedsUpdate, data.NeedsDowngrade, data.CanUninstallCli, + data.NeedsCliPathSetup, data.CliVersion, data.RequiredCliVersion); bool enabled = IsInstallCliButtonEnabled( @@ -203,6 +204,7 @@ internal static string GetInstallCliButtonText( bool needsUpdate, bool needsDowngrade, bool canUninstallCli, + bool needsCliPathSetup, string cliVersion, string requiredCliVersion) { @@ -217,6 +219,11 @@ internal static string GetInstallCliButtonText( return isUninstallAction ? "Uninstalling..." : "Installing..."; } + if (needsCliPathSetup) + { + return "Fix PATH"; + } + if (!isCliInstalled) { return "Install CLI"; diff --git a/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs b/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs index 33affb364..5653ffb1e 100644 --- a/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs +++ b/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs @@ -39,6 +39,8 @@ public class UnityCliLoopSettingsWindow : EditorWindow private bool _isInstallingCli; private bool _isInstallingSkills; private bool _isRefreshingVersion; + private bool _isRefreshingCliPathSetup; + private bool _needsCliPathSetup; private bool _isToolSettingsCatalogDirty = true; private bool _isDeferredInitialRefreshScheduled; private bool _hasCompletedDeferredInitialRefresh; @@ -282,6 +284,7 @@ internal void RefreshAllSections( if (runExpensiveChecks) { RefreshCliVersionInBackground(); + RefreshCliPathSetupInBackground(); if (refreshSkillInstallState) { RefreshSelectedTargetInstallStateInBackground(); @@ -304,10 +307,41 @@ private async void RefreshCliVersionInBackground() } await CliSetupApplicationFacade.RefreshCliVersionAsync(CancellationToken.None); + RefreshCliPathSetupInBackground(); RefreshCliSetupSection(); RefreshSelectedTargetInstallStateInBackground(); } + private async void RefreshCliPathSetupInBackground() + { + if (_isRefreshingCliPathSetup) + { + return; + } + + if (!ShouldCheckCliPathSetup()) + { + _needsCliPathSetup = false; + return; + } + + _isRefreshingCliPathSetup = true; + RefreshCliSetupSection(); + + try + { + bool isCliVisibleFromShell = await CliSetupApplicationFacade.IsCliVisibleFromShellAsync( + UnityEngine.Application.platform, + CancellationToken.None); + _needsCliPathSetup = !isCliVisibleFromShell; + } + finally + { + _isRefreshingCliPathSetup = false; + RefreshCliSetupSection(); + } + } + private async void HandleRefreshCliVersion() { if (_isRefreshingVersion) @@ -323,6 +357,7 @@ private async void HandleRefreshCliVersion() Task forceRefresh = CliSetupApplicationFacade.ForceRefreshCliVersionAsync(CancellationToken.None); Task minimumDelay = Task.Delay(500); await Task.WhenAll(forceRefresh, minimumDelay); + RefreshCliPathSetupInBackground(); } finally { @@ -577,12 +612,13 @@ private CliSetupData CreateCliSetupData(bool includeSkillDirectoryChecks = true) string cliExecutablePath = CliSetupApplicationFacade.GetCachedCliExecutablePath(); string requiredCliVersion = GetMinimumRequiredCliVersion(); - bool isCliInstalled = cliVersion != null; + bool isCliInstalled = cliVersion != null || _needsCliPathSetup; bool canUninstallCli = CliSetupApplicationFacade.IsPackageOwnedCurrentUserInstallPath( cliExecutablePath, UnityEngine.Application.platform); bool isChecking = !CliSetupApplicationFacade.IsCliCheckCompleted() || _isRefreshingVersion + || _isRefreshingCliPathSetup || !includeSkillDirectoryChecks; bool needsUpdate = IsCliUpdateNeeded(cliVersion, requiredCliVersion); bool needsDowngrade = false; @@ -598,6 +634,7 @@ private CliSetupData CreateCliSetupData(bool includeSkillDirectoryChecks = true) needsUpdate, needsDowngrade, canUninstallCli, + _needsCliPathSetup, _isInstallingCli, isChecking, isClaudeSkillsInstalled: false, @@ -617,6 +654,20 @@ private static string GetMinimumRequiredCliVersion() return CliSetupApplicationFacade.GetMinimumRequiredCliVersion(); } + private static bool ShouldCheckCliPathSetup() + { + return ShouldCheckCliPathSetupForPlatform( + UnityEngine.Application.platform, + CliSetupApplicationFacade.HasPackageOwnedCurrentUserInstall(UnityEngine.Application.platform)); + } + + internal static bool ShouldCheckCliPathSetupForPlatform( + RuntimePlatform platform, + bool hasPackageOwnedCurrentUserInstall) + { + return platform != RuntimePlatform.WindowsEditor && hasPackageOwnedCurrentUserInstall; + } + private void RefreshSelectedTargetInstallStateFast() { if (!CliSetupApplicationFacade.IsCliInstalled()) @@ -709,6 +760,14 @@ private void CancelSkillInstallStateRefresh() private async void HandleInstallCli() { + await RefreshCliPrimaryActionStateAsync(CancellationToken.None); + + if (ShouldRepairCliPathFromPrimaryButton(_needsCliPathSetup)) + { + await HandleRepairCliPathSetup(); + return; + } + if (ShouldUninstallCliFromPrimaryButton()) { await HandleUninstallCli(); @@ -716,6 +775,7 @@ private async void HandleInstallCli() } bool wasCliInstalledBeforeInstall = CliSetupApplicationFacade.IsCliInstalled(); + _needsCliPathSetup = false; _isInstallingCli = true; RefreshCliSetupSection(); @@ -736,6 +796,11 @@ private async void HandleInstallCli() "OK"); return; } + + await CliPathSetupPrompt.EnsureVisibleAndShowResultAsync( + UnityEngine.Application.platform, + CancellationToken.None); + await RefreshCliPathSetupAsync(CancellationToken.None); } finally { @@ -769,6 +834,61 @@ internal static bool ShouldUninstallCliFromPrimaryButton( return CliSetupSection.IsUninstallCliAction(isCliInstalled, needsUpdate, needsDowngrade: false, canUninstallCli); } + internal static bool ShouldRepairCliPathFromPrimaryButton(bool needsCliPathSetup) + { + return needsCliPathSetup; + } + + private async Task RefreshCliPrimaryActionStateAsync(CancellationToken ct) + { + _isRefreshingVersion = true; + RefreshCliSetupSection(); + + try + { + await CliSetupApplicationFacade.ForceRefreshCliVersionAsync(ct); + await RefreshCliPathSetupAsync(ct); + } + finally + { + _isRefreshingVersion = false; + RefreshCliSetupSection(); + } + } + + private async Task RefreshCliPathSetupAsync(CancellationToken ct) + { + if (!ShouldCheckCliPathSetup()) + { + _needsCliPathSetup = false; + return; + } + + bool isCliVisibleFromShell = await CliSetupApplicationFacade.IsCliVisibleFromShellAsync( + UnityEngine.Application.platform, + ct); + _needsCliPathSetup = !isCliVisibleFromShell; + } + + private async Task HandleRepairCliPathSetup() + { + _isInstallingCli = true; + RefreshCliSetupSection(); + + try + { + await CliPathSetupPrompt.EnsureVisibleAndShowResultAsync( + UnityEngine.Application.platform, + CancellationToken.None); + await RefreshCliPathSetupAsync(CancellationToken.None); + } + finally + { + _isInstallingCli = false; + RefreshAllSections(); + } + } + internal static bool IsCliUpdateNeeded(string cliVersion, string requiredCliVersion) { return cliVersion != null diff --git a/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindowViewData.cs b/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindowViewData.cs index de2fa228f..a339ebfe2 100644 --- a/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindowViewData.cs +++ b/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindowViewData.cs @@ -83,6 +83,7 @@ public record CliSetupData public readonly bool NeedsUpdate; public readonly bool NeedsDowngrade; public readonly bool CanUninstallCli; + public readonly bool NeedsCliPathSetup; public readonly bool IsInstallingCli; public readonly bool IsChecking; public readonly bool IsClaudeSkillsInstalled; @@ -103,6 +104,7 @@ public CliSetupData( bool needsUpdate, bool needsDowngrade, bool canUninstallCli, + bool needsCliPathSetup, bool isInstallingCli, bool isChecking, bool isClaudeSkillsInstalled, @@ -122,6 +124,7 @@ public CliSetupData( NeedsUpdate = needsUpdate; NeedsDowngrade = needsDowngrade; CanUninstallCli = canUninstallCli; + NeedsCliPathSetup = needsCliPathSetup; IsInstallingCli = isInstallingCli; IsChecking = isChecking; IsClaudeSkillsInstalled = isClaudeSkillsInstalled; diff --git a/scripts/install.sh b/scripts/install.sh index d74f8e83b..f9d495ab6 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -115,6 +115,116 @@ print_legacy_npm_manual_removal() { echo " npm uninstall -g uloop-cli" } +quote_for_single_quoted_shell() { + printf "%s" "$1" | sed "s/'/'\\\\''/g" +} + +print_append_command() { + append_line=$1 + profile_path=$2 + quoted_line=$(quote_for_single_quoted_shell "$append_line") + quoted_profile=$(quote_for_single_quoted_shell "$profile_path") + + printf "%s\n" " printf '\n%s\n' '$quoted_line' >> '$quoted_profile'" +} + +detect_user_shell_name() { + shell_path=${SHELL:-} + + if [ -z "$shell_path" ]; then + echo "" + return + fi + + echo "${shell_path##*/}" +} + +detect_bash_profile_path() { + if [ -f "$HOME/.bash_profile" ]; then + echo "$HOME/.bash_profile" + return + fi + + if [ -f "$HOME/.bash_login" ]; then + echo "$HOME/.bash_login" + return + fi + + if [ -f "$HOME/.profile" ]; then + echo "$HOME/.profile" + return + fi + + echo "$HOME/.bash_profile" +} + +detect_zsh_profile_path() { + if [ -n "${ZDOTDIR:-}" ]; then + echo "$ZDOTDIR/.zshrc" + return + fi + + echo "$HOME/.zshrc" +} + +detect_fish_profile_path() { + if [ -n "${XDG_CONFIG_HOME:-}" ]; then + echo "$XDG_CONFIG_HOME/fish/config.fish" + return + fi + + echo "$HOME/.config/fish/config.fish" +} + +print_fish_append_command() { + append_line=$1 + profile_path=$2 + profile_dir=${profile_path%/*} + quoted_line=$(quote_for_single_quoted_shell "$append_line") + quoted_profile=$(quote_for_single_quoted_shell "$profile_path") + quoted_profile_dir=$(quote_for_single_quoted_shell "$profile_dir") + + printf "%s\n" " mkdir -p '$quoted_profile_dir' && printf '\n%s\n' '$quoted_line' >> '$quoted_profile'" +} + +print_path_setup_guidance() { + shell_name=$(detect_user_shell_name) + + echo "Installed uloop to $INSTALL_DIR, but that directory is not in PATH." + case "$shell_name" in + zsh) + profile_path=$(detect_zsh_profile_path) + echo "Detected shell: zsh" + echo "Add this line to $profile_path:" + echo " export PATH=\"$INSTALL_DIR:\$PATH\"" + echo "Or run:" + print_append_command "export PATH=\"$INSTALL_DIR:\$PATH\"" "$profile_path" + ;; + bash) + profile_path=$(detect_bash_profile_path) + echo "Detected shell: bash" + echo "Add this line to $profile_path:" + echo " export PATH=\"$INSTALL_DIR:\$PATH\"" + echo "Or run:" + print_append_command "export PATH=\"$INSTALL_DIR:\$PATH\"" "$profile_path" + ;; + fish) + profile_path=$(detect_fish_profile_path) + echo "Detected shell: fish" + echo "Add this line to $profile_path:" + echo " fish_add_path \"$INSTALL_DIR\"" + echo "Or run:" + print_fish_append_command "fish_add_path \"$INSTALL_DIR\"" "$profile_path" + ;; + *) + echo "Add this directory to PATH in your shell profile:" + echo " $INSTALL_DIR" + ;; + esac + + echo "Open a new terminal after updating the profile." +} + try_remove_legacy_npm_package() { legacy_uloop=$1 expected_uloop=$2 @@ -318,9 +428,7 @@ fi case ":$PATH:" in *":$INSTALL_DIR:"*) ;; *) - echo "Installed uloop to $INSTALL_DIR, but that directory is not in PATH." - echo "Add this to your shell profile:" - echo " export PATH=\"$INSTALL_DIR:\$PATH\"" + print_path_setup_guidance ;; esac diff --git a/scripts/test-install-release-filter.sh b/scripts/test-install-release-filter.sh index a310e283a..1911709d8 100755 --- a/scripts/test-install-release-filter.sh +++ b/scripts/test-install-release-filter.sh @@ -35,6 +35,15 @@ assert_not_contains() { fi } +assert_file_not_exists() { + file=$1 + + if [ -e "$file" ]; then + echo "Expected $file not to exist" >&2 + exit 1 + fi +} + write_releases_json() { output_path=$1 @@ -285,7 +294,7 @@ write_required_tool_links() { tool_bin=$1 mkdir -p "$tool_bin" - for command_name in awk cat chmod grep install mkdir mktemp mv readlink rm; do + for command_name in awk cat chmod grep install mkdir mktemp mv readlink rm sed; do command_path=$(command -v "$command_name") { printf '%s\n' '#!/bin/sh' @@ -361,6 +370,143 @@ test_posix_latest_beta_selects_prerelease_assets() { assert_not_contains "$npm_log" "uninstall -g uloop-cli" } +test_posix_prints_zsh_path_guidance_without_writing_profile() { + work_dir="$TMP_DIR/posix-zsh-path-guidance" + mock_bin="$work_dir/bin" + install_dir="$work_dir/install" + home_dir="$work_dir/home" + zdot_dir="$work_dir/zdot" + releases_json="$work_dir/releases.json" + curl_log="$work_dir/curl.log" + npm_log="$work_dir/npm.log" + profile_path="$zdot_dir/.zshrc" + mkdir -p "$work_dir" "$home_dir" "$zdot_dir" + : > "$curl_log" + : > "$npm_log" + write_releases_json "$releases_json" + write_mock_commands "$mock_bin" + + PATH="$mock_bin:/usr/bin:/bin:/usr/sbin:/sbin" \ + HOME="$home_dir" \ + ZDOTDIR="$zdot_dir" \ + SHELL="/bin/zsh" \ + ULOOP_VERSION=latest \ + ULOOP_INSTALL_DIR="$install_dir" \ + RELEASES_JSON="$releases_json" \ + CURL_LOG="$curl_log" \ + NPM_LOG="$npm_log" \ + LEGACY_ULOOP="" \ + "$ROOT_DIR/scripts/install.sh" > "$work_dir/output.txt" 2> "$work_dir/stderr.txt" + + assert_contains "$work_dir/output.txt" "Detected shell: zsh" + assert_contains "$work_dir/output.txt" "Add this line to $profile_path:" + assert_contains "$work_dir/output.txt" " export PATH=\"$install_dir:\$PATH\"" + assert_contains "$work_dir/output.txt" "printf '\\n%s\\n' 'export PATH=\"$install_dir:\$PATH\"' >> '$profile_path'" + assert_file_not_exists "$profile_path" +} + +test_posix_prints_bash_path_guidance_without_modifying_existing_profile() { + work_dir="$TMP_DIR/posix-bash-path-guidance" + mock_bin="$work_dir/bin" + install_dir="$work_dir/install" + home_dir="$work_dir/home" + releases_json="$work_dir/releases.json" + curl_log="$work_dir/curl.log" + npm_log="$work_dir/npm.log" + profile_path="$home_dir/.bash_login" + mkdir -p "$work_dir" "$home_dir" + : > "$curl_log" + : > "$npm_log" + printf '%s\n' "existing bash profile" > "$profile_path" + write_releases_json "$releases_json" + write_mock_commands "$mock_bin" + + PATH="$mock_bin:/usr/bin:/bin:/usr/sbin:/sbin" \ + HOME="$home_dir" \ + SHELL="/bin/bash" \ + ULOOP_VERSION=latest \ + ULOOP_INSTALL_DIR="$install_dir" \ + RELEASES_JSON="$releases_json" \ + CURL_LOG="$curl_log" \ + NPM_LOG="$npm_log" \ + LEGACY_ULOOP="" \ + "$ROOT_DIR/scripts/install.sh" > "$work_dir/output.txt" 2> "$work_dir/stderr.txt" + + assert_contains "$work_dir/output.txt" "Detected shell: bash" + assert_contains "$work_dir/output.txt" "Add this line to $profile_path:" + assert_contains "$work_dir/output.txt" " export PATH=\"$install_dir:\$PATH\"" + if [ "$(cat "$profile_path")" != "existing bash profile" ]; then + echo "Expected bash profile to remain unchanged: $profile_path" >&2 + exit 1 + fi +} + +test_posix_prints_fish_path_guidance_without_writing_profile() { + work_dir="$TMP_DIR/posix-fish-path-guidance" + mock_bin="$work_dir/bin" + install_dir="$work_dir/install" + home_dir="$work_dir/home" + xdg_config_home="$work_dir/xdg" + releases_json="$work_dir/releases.json" + curl_log="$work_dir/curl.log" + npm_log="$work_dir/npm.log" + profile_path="$xdg_config_home/fish/config.fish" + mkdir -p "$work_dir" "$home_dir" + : > "$curl_log" + : > "$npm_log" + write_releases_json "$releases_json" + write_mock_commands "$mock_bin" + + PATH="$mock_bin:/usr/bin:/bin:/usr/sbin:/sbin" \ + HOME="$home_dir" \ + XDG_CONFIG_HOME="$xdg_config_home" \ + SHELL="/opt/homebrew/bin/fish" \ + ULOOP_VERSION=latest \ + ULOOP_INSTALL_DIR="$install_dir" \ + RELEASES_JSON="$releases_json" \ + CURL_LOG="$curl_log" \ + NPM_LOG="$npm_log" \ + LEGACY_ULOOP="" \ + "$ROOT_DIR/scripts/install.sh" > "$work_dir/output.txt" 2> "$work_dir/stderr.txt" + + assert_contains "$work_dir/output.txt" "Detected shell: fish" + assert_contains "$work_dir/output.txt" "Add this line to $profile_path:" + assert_contains "$work_dir/output.txt" " fish_add_path \"$install_dir\"" + assert_contains "$work_dir/output.txt" "mkdir -p '$xdg_config_home/fish' && printf '\\n%s\\n' 'fish_add_path \"$install_dir\"' >> '$profile_path'" + assert_file_not_exists "$profile_path" +} + +test_posix_prints_generic_path_guidance_for_unknown_shell() { + work_dir="$TMP_DIR/posix-generic-path-guidance" + mock_bin="$work_dir/bin" + install_dir="$work_dir/install" + home_dir="$work_dir/home" + releases_json="$work_dir/releases.json" + curl_log="$work_dir/curl.log" + npm_log="$work_dir/npm.log" + mkdir -p "$work_dir" "$home_dir" + : > "$curl_log" + : > "$npm_log" + write_releases_json "$releases_json" + write_mock_commands "$mock_bin" + + PATH="$mock_bin:/usr/bin:/bin:/usr/sbin:/sbin" \ + HOME="$home_dir" \ + SHELL="/bin/ksh" \ + ULOOP_VERSION=latest \ + ULOOP_INSTALL_DIR="$install_dir" \ + RELEASES_JSON="$releases_json" \ + CURL_LOG="$curl_log" \ + NPM_LOG="$npm_log" \ + LEGACY_ULOOP="" \ + "$ROOT_DIR/scripts/install.sh" > "$work_dir/output.txt" 2> "$work_dir/stderr.txt" + + assert_contains "$work_dir/output.txt" "Add this directory to PATH in your shell profile:" + assert_contains "$work_dir/output.txt" " $install_dir" + assert_not_contains "$work_dir/output.txt" "Detected shell:" + assert_file_not_exists "$home_dir/.kshrc" +} + test_posix_skips_default_npm_cleanup_when_native_command_is_first() { work_dir="$TMP_DIR/posix-native-first" mock_bin="$work_dir/bin" @@ -616,6 +762,10 @@ test_powershell_installer_uses_non_installer_staged_executable_name() { test_posix_latest_skips_prerelease_assets test_posix_latest_beta_selects_prerelease_assets +test_posix_prints_zsh_path_guidance_without_writing_profile +test_posix_prints_bash_path_guidance_without_modifying_existing_profile +test_posix_prints_fish_path_guidance_without_writing_profile +test_posix_prints_generic_path_guidance_for_unknown_shell test_posix_skips_default_npm_cleanup_when_native_command_is_first test_posix_does_not_infer_npm_prefix_from_non_npm_command test_posix_prints_prefix_manual_cleanup_when_npm_is_unavailable From 1206dd470e4d3f1581a11445c5f42e5f3695d8b9 Mon Sep 17 00:00:00 2001 From: hatayama Date: Thu, 21 May 2026 00:15:12 +0900 Subject: [PATCH 3/5] Harden CLI PATH setup repair flow Keep the clicked Settings action stable across async state refreshes so a stale install click cannot turn into an uninstall action. Tighten PATH profile handling so Unity only rewrites shell configuration when the terminal cannot resolve uloop, fish guidance moves ~/.local/bin to the front, and duplicate detection avoids both stale PATH entries and unrelated trailing lines. --- Assets/Tests/Editor/CliPathSetupFlowTests.cs | 47 ++- .../Editor/CliPathSetupFlowTests.cs.meta | 6 +- .../CliPathSetupProfileResolverTests.cs | 6 +- .../CliPathSetupProfileResolverTests.cs.meta | 6 +- .../Tests/Editor/CliPathSetupWriterTests.cs | 216 +++++++++++- .../Editor/CliPathSetupWriterTests.cs.meta | 6 +- Assets/Tests/Editor/CliSetupSectionTests.cs | 1 + Assets/Tests/Editor/SetupWizardWindowTests.cs | 1 + ...nityCliLoopSettingsWindowCliActionTests.cs | 22 ++ .../Editor/Application/CliPathSetupPlan.cs | 4 +- .../Application/CliPathSetupPlan.cs.meta | 6 +- .../CLI/CliPathSetupProfileResolver.cs | 4 +- .../CLI/CliPathSetupProfileResolver.cs.meta | 6 +- .../Infrastructure/CLI/CliPathSetupWriter.cs | 322 ++++++++++++++++-- .../CLI/CliPathSetupWriter.cs.meta | 6 +- .../Editor/Presentation/CliPathSetupPrompt.cs | 31 +- .../Presentation/CliPathSetupPrompt.cs.meta | 6 +- .../Presentation/Setup/SetupWizardWindow.cs | 8 +- .../UIToolkit/Components/CliSetupSection.cs | 10 +- .../UnityCliLoopSettingsWindow.cs | 46 ++- .../cli-path-setup-rebuild-plan.md | 2 +- scripts/install.sh | 4 +- scripts/test-install-release-filter.sh | 4 +- 23 files changed, 684 insertions(+), 86 deletions(-) diff --git a/Assets/Tests/Editor/CliPathSetupFlowTests.cs b/Assets/Tests/Editor/CliPathSetupFlowTests.cs index ddb4b80a7..34b871d6a 100644 --- a/Assets/Tests/Editor/CliPathSetupFlowTests.cs +++ b/Assets/Tests/Editor/CliPathSetupFlowTests.cs @@ -18,7 +18,9 @@ public async Task EnsureCliVisibleFromShellAsync_WhenAlreadyVisibleDoesNotApplyP { // Verifies that fresh shell visibility is the authority and no profile write happens when uloop is visible. FakeCliInstallationDetector detector = new(true); - FakeNativeCliInstaller installer = new(CreateSupportedPlan()); + FakeNativeCliInstaller installer = new FakeNativeCliInstaller( + CreateSupportedPlan(), + CreateAppliedResult()); CliSetupApplicationService service = new(detector, installer); CliPathSetupFlowResult result = await service.EnsureCliVisibleFromShellAsync( @@ -34,7 +36,9 @@ public async Task EnsureCliVisibleFromShellAsync_WhenApplySucceedsRechecksShell( { // Verifies that PATH setup is only considered complete after a second fresh shell check passes. FakeCliInstallationDetector detector = new(false, true); - FakeNativeCliInstaller installer = new(CreateSupportedPlan()); + FakeNativeCliInstaller installer = new FakeNativeCliInstaller( + CreateSupportedPlan(), + CreateAppliedResult()); CliSetupApplicationService service = new(detector, installer); CliPathSetupFlowResult result = await service.EnsureCliVisibleFromShellAsync( @@ -51,7 +55,9 @@ public async Task EnsureCliVisibleFromShellAsync_WhenUnsupportedShellReturnsManu { // Verifies that unsupported shells never receive automatic profile edits. FakeCliInstallationDetector detector = new(false); - FakeNativeCliInstaller installer = new(CreateUnsupportedPlan()); + FakeNativeCliInstaller installer = new FakeNativeCliInstaller( + CreateUnsupportedPlan(), + CreateAppliedResult()); CliSetupApplicationService service = new(detector, installer); CliPathSetupFlowResult result = await service.EnsureCliVisibleFromShellAsync( @@ -62,6 +68,28 @@ public async Task EnsureCliVisibleFromShellAsync_WhenUnsupportedShellReturnsManu Assert.That(installer.ApplyCount, Is.EqualTo(0)); } + [Test] + public async Task EnsureCliVisibleFromShellAsync_WhenProfileApplyFailsReturnsFailure() + { + // Verifies that profile write failures become user-facing PATH setup failures. + FakeCliInstallationDetector detector = new(false); + CliPathSetupApplyResult applyResult = new CliPathSetupApplyResult( + false, + CliPathSetupApplyStatus.Failed, + "profile is read-only"); + FakeNativeCliInstaller installer = new FakeNativeCliInstaller(CreateSupportedPlan(), applyResult); + CliSetupApplicationService service = new CliSetupApplicationService(detector, installer); + + CliPathSetupFlowResult result = await service.EnsureCliVisibleFromShellAsync( + RuntimePlatform.OSXEditor, + CancellationToken.None); + + Assert.That(result.Status, Is.EqualTo(CliPathSetupFlowStatus.Failed)); + Assert.That(result.ErrorOutput, Does.Contain("profile is read-only")); + Assert.That(installer.ApplyCount, Is.EqualTo(1)); + Assert.That(detector.VisibilityCheckCount, Is.EqualTo(1)); + } + private static CliPathSetupPlan CreateUnsupportedPlan() { return new CliPathSetupPlan( @@ -72,7 +100,7 @@ private static CliPathSetupPlan CreateUnsupportedPlan() "/Users/ExampleUser/.local/bin", "", "", - "export PATH='/Users/ExampleUser/.local/bin':\"$PATH\""); + ""); } private static CliPathSetupPlan CreateSupportedPlan() @@ -88,6 +116,11 @@ private static CliPathSetupPlan CreateSupportedPlan() "printf '\\n%s\\n' 'export PATH=\"$HOME/.local/bin:$PATH\"' >> '/Users/ExampleUser/.zshrc'"); } + private static CliPathSetupApplyResult CreateAppliedResult() + { + return new CliPathSetupApplyResult(true, CliPathSetupApplyStatus.Applied, ""); + } + private sealed class FakeCliInstallationDetector : ICliInstallationDetector { private readonly bool[] _visibilityResults; @@ -119,10 +152,12 @@ public Task IsCliVisibleFromShellAsync(RuntimePlatform platform, Cancellat private sealed class FakeNativeCliInstaller : INativeCliInstaller { private readonly CliPathSetupPlan _plan; + private readonly CliPathSetupApplyResult _applyResult; - public FakeNativeCliInstaller(CliPathSetupPlan plan) + public FakeNativeCliInstaller(CliPathSetupPlan plan, CliPathSetupApplyResult applyResult) { _plan = plan; + _applyResult = applyResult; } public int ApplyCount { get; private set; } @@ -158,7 +193,7 @@ public Task GetGlobalCliPathSetupPlanAsync(RuntimePlatform pla public CliPathSetupApplyResult ApplyGlobalCliPathSetup(CliPathSetupPlan plan) { ApplyCount++; - return new CliPathSetupApplyResult(true, CliPathSetupApplyStatus.Applied, ""); + return _applyResult; } public NativeCliInstallCommand GetGlobalCliInstallCommand( diff --git a/Assets/Tests/Editor/CliPathSetupFlowTests.cs.meta b/Assets/Tests/Editor/CliPathSetupFlowTests.cs.meta index 1a948c48a..61dfdc7f7 100644 --- a/Assets/Tests/Editor/CliPathSetupFlowTests.cs.meta +++ b/Assets/Tests/Editor/CliPathSetupFlowTests.cs.meta @@ -6,6 +6,6 @@ MonoImporter: defaultReferences: [] executionOrder: 0 icon: {instanceID: 0} - userData: - assetBundleName: - assetBundleVariant: + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/CliPathSetupProfileResolverTests.cs b/Assets/Tests/Editor/CliPathSetupProfileResolverTests.cs index ba49472fb..cd2369a43 100644 --- a/Assets/Tests/Editor/CliPathSetupProfileResolverTests.cs +++ b/Assets/Tests/Editor/CliPathSetupProfileResolverTests.cs @@ -64,13 +64,13 @@ public void ResolvePlan_WhenFishUsesXdgConfigHome() Assert.That( plan.ConfigurationFilePath, Is.EqualTo("/Users/ExampleUser/Library/Application Support/fish/config.fish")); - Assert.That(plan.ConfigurationLine, Is.EqualTo("fish_add_path \"$HOME/.local/bin\"")); + Assert.That(plan.ConfigurationLine, Is.EqualTo("fish_add_path --move \"$HOME/.local/bin\"")); } [Test] public void ResolvePlan_WhenUnsupportedShellDisablesAutomaticApply() { - // Verifies that unknown shells fall back to a manual command instead of guessing a profile file. + // Verifies that unknown shells do not expose a command written for a different shell syntax. CliPathSetupPlan plan = CliPathSetupProfileResolver.ResolvePlan( RuntimePlatform.OSXEditor, "/bin/tcsh", @@ -82,7 +82,7 @@ public void ResolvePlan_WhenUnsupportedShellDisablesAutomaticApply() Assert.That(plan.ShellKind, Is.EqualTo(CliPathSetupShellKind.Unsupported)); Assert.That(plan.CanApplyAutomatically, Is.False); - Assert.That(plan.ManualCommand, Does.Contain("/Users/ExampleUser/.local/bin")); + Assert.That(plan.ManualCommand, Is.Empty); } } } diff --git a/Assets/Tests/Editor/CliPathSetupProfileResolverTests.cs.meta b/Assets/Tests/Editor/CliPathSetupProfileResolverTests.cs.meta index da098c85e..245447a0e 100644 --- a/Assets/Tests/Editor/CliPathSetupProfileResolverTests.cs.meta +++ b/Assets/Tests/Editor/CliPathSetupProfileResolverTests.cs.meta @@ -6,6 +6,6 @@ MonoImporter: defaultReferences: [] executionOrder: 0 icon: {instanceID: 0} - userData: - assetBundleName: - assetBundleVariant: + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/CliPathSetupWriterTests.cs b/Assets/Tests/Editor/CliPathSetupWriterTests.cs index 0e4aa3ddf..8d4c4600b 100644 --- a/Assets/Tests/Editor/CliPathSetupWriterTests.cs +++ b/Assets/Tests/Editor/CliPathSetupWriterTests.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.IO; @@ -50,6 +51,42 @@ public void Apply_WhenCanonicalLineExistsDoesNotAppend() Assert.That(appendCount, Is.EqualTo(0)); } + [Test] + public void Apply_WhenCanonicalLineIsFollowedByShadowingPathPrependerAppends() + { + // Verifies that stale earlier setup lines do not block a repair append at the end of the profile. + CliPathSetupPlan plan = CreateZshPlan(); + int appendCount = 0; + + CliPathSetupApplyResult result = CliPathSetupWriter.Apply( + plan, + path => true, + path => "export PATH=\"$HOME/.local/bin:$PATH\"\nexport PATH=\"/usr/local/bin:$PATH\"\n", + path => new DirectoryInfo(path), + (path, content) => appendCount++); + + Assert.That(result.Status, Is.EqualTo(CliPathSetupApplyStatus.Applied)); + Assert.That(appendCount, Is.EqualTo(1)); + } + + [Test] + public void Apply_WhenCanonicalLineIsFollowedByUnrelatedCommandDoesNotAppend() + { + // Verifies that unrelated later profile commands do not duplicate an effective PATH setup. + CliPathSetupPlan plan = CreateZshPlan(); + int appendCount = 0; + + CliPathSetupApplyResult result = CliPathSetupWriter.Apply( + plan, + path => true, + path => "export PATH=\"$HOME/.local/bin:$PATH\"\nalias ll='ls -la'\n", + path => new DirectoryInfo(path), + (path, content) => appendCount++); + + Assert.That(result.Status, Is.EqualTo(CliPathSetupApplyStatus.AlreadyConfigured)); + Assert.That(appendCount, Is.EqualTo(0)); + } + [Test] public void Apply_WhenLineIsCommentedAppends() { @@ -69,9 +106,9 @@ public void Apply_WhenLineIsCommentedAppends() } [Test] - public void Apply_WhenInstallDirectoryAppearsInPathAssignmentDoesNotAppend() + public void Apply_WhenInstallDirectoryAppearsAfterExistingPathAppends() { - // Verifies that an existing PATH assignment with the install directory is enough to avoid duplication. + // Verifies that appended install directories do not block a repair that needs to outrank old shims. CliPathSetupPlan plan = CreateZshPlan(); int appendCount = 0; @@ -82,10 +119,172 @@ public void Apply_WhenInstallDirectoryAppearsInPathAssignmentDoesNotAppend() path => new DirectoryInfo(path), (path, content) => appendCount++); + Assert.That(result.Status, Is.EqualTo(CliPathSetupApplyStatus.Applied)); + Assert.That(appendCount, Is.EqualTo(1)); + } + + [Test] + public void Apply_WhenInstallDirectoryIsNotFirstPathEntryAppends() + { + // Verifies that earlier PATH entries keep shadowing risk even when the install directory is before $PATH. + CliPathSetupPlan plan = CreateZshPlan(); + int appendCount = 0; + + CliPathSetupApplyResult result = CliPathSetupWriter.Apply( + plan, + path => true, + path => "export PATH=\"/opt/old:$HOME/.local/bin:$PATH\"\n", + path => new DirectoryInfo(path), + (path, content) => appendCount++); + + Assert.That(result.Status, Is.EqualTo(CliPathSetupApplyStatus.Applied)); + Assert.That(appendCount, Is.EqualTo(1)); + } + + [Test] + public void Apply_WhenInstallDirectoryAppearsBeforeExistingPathDoesNotAppend() + { + // Verifies that profile entries only count as configured when they put the install directory first. + CliPathSetupPlan plan = CreateZshPlan(); + int appendCount = 0; + + CliPathSetupApplyResult result = CliPathSetupWriter.Apply( + plan, + path => true, + path => "PATH=\"$HOME/.local/bin:$PATH\"\n", + path => new DirectoryInfo(path), + (path, content) => appendCount++); + Assert.That(result.Status, Is.EqualTo(CliPathSetupApplyStatus.AlreadyConfigured)); Assert.That(appendCount, Is.EqualTo(0)); } + [Test] + public void Apply_WhenSiblingDirectoryAppearsInPathAssignmentAppends() + { + // Verifies that similar directory names are not treated as the install directory. + CliPathSetupPlan plan = CreateZshPlan(); + int appendCount = 0; + + CliPathSetupApplyResult result = CliPathSetupWriter.Apply( + plan, + path => true, + path => "PATH=\"$PATH:$HOME/.local/bin-old\"\n", + path => new DirectoryInfo(path), + (path, content) => appendCount++); + + Assert.That(result.Status, Is.EqualTo(CliPathSetupApplyStatus.Applied)); + Assert.That(appendCount, Is.EqualTo(1)); + } + + [Test] + public void Apply_WhenFishProfileSetsUnrelatedPathVariableAppends() + { + // Verifies that unrelated fish variables containing PATH do not count as shell PATH setup. + CliPathSetupPlan plan = CreateFishPlan(); + int appendCount = 0; + + CliPathSetupApplyResult result = CliPathSetupWriter.Apply( + plan, + path => true, + path => "set -gx GOPATH \"$HOME/.local/bin\"\n", + path => new DirectoryInfo(path), + (path, content) => appendCount++); + + Assert.That(result.Status, Is.EqualTo(CliPathSetupApplyStatus.Applied)); + Assert.That(appendCount, Is.EqualTo(1)); + } + + [Test] + public void Apply_WhenFishProfileSetsPathVariableDoesNotAppend() + { + // Verifies that fish PATH variable setup with the install directory prevents duplication. + CliPathSetupPlan plan = CreateFishPlan(); + int appendCount = 0; + + CliPathSetupApplyResult result = CliPathSetupWriter.Apply( + plan, + path => true, + path => "set -gx fish_user_paths \"$HOME/.local/bin\" $fish_user_paths\n", + path => new DirectoryInfo(path), + (path, content) => appendCount++); + + Assert.That(result.Status, Is.EqualTo(CliPathSetupApplyStatus.AlreadyConfigured)); + Assert.That(appendCount, Is.EqualTo(0)); + } + + [Test] + public void Apply_WhenFishProfileAppendsInstallDirectoryAppends() + { + // Verifies that fish append-style setup does not block a repair that must prepend the install directory. + CliPathSetupPlan plan = CreateFishPlan(); + int appendCount = 0; + + CliPathSetupApplyResult result = CliPathSetupWriter.Apply( + plan, + path => true, + path => "set -gx fish_user_paths $fish_user_paths \"$HOME/.local/bin\"\n", + path => new DirectoryInfo(path), + (path, content) => appendCount++); + + Assert.That(result.Status, Is.EqualTo(CliPathSetupApplyStatus.Applied)); + Assert.That(appendCount, Is.EqualTo(1)); + } + + [Test] + public void Apply_WhenProfileWriteFailsReturnsFailedResult() + { + // Verifies that a denied shell profile write falls back to the manual setup result path. + CliPathSetupPlan plan = CreateZshPlan(); + + CliPathSetupApplyResult result = CliPathSetupWriter.Apply( + plan, + path => true, + path => string.Empty, + path => new DirectoryInfo(path), + (path, content) => throw new UnauthorizedAccessException("profile is read-only")); + + Assert.That(result.Success, Is.False); + Assert.That(result.Status, Is.EqualTo(CliPathSetupApplyStatus.Failed)); + Assert.That(result.ErrorOutput, Does.Contain("profile is read-only")); + } + + [Test] + public void Apply_WhenProfileReadFailsReturnsFailedResult() + { + // Verifies that a denied shell profile read falls back to the manual setup result path. + CliPathSetupPlan plan = CreateZshPlan(); + + CliPathSetupApplyResult result = CliPathSetupWriter.Apply( + plan, + path => true, + path => throw new IOException("profile cannot be read"), + path => new DirectoryInfo(path), + (path, content) => { }); + + Assert.That(result.Success, Is.False); + Assert.That(result.Status, Is.EqualTo(CliPathSetupApplyStatus.Failed)); + Assert.That(result.ErrorOutput, Does.Contain("profile cannot be read")); + } + + [Test] + public void Apply_WhenProfileDirectoryCreationFailsReturnsFailedResult() + { + // Verifies that denied profile directory creation falls back to the manual setup result path. + CliPathSetupPlan plan = CreateZshPlan(); + + CliPathSetupApplyResult result = CliPathSetupWriter.Apply( + plan, + path => false, + path => string.Empty, + path => throw new IOException("profile directory cannot be created"), + (path, content) => { }); + + Assert.That(result.Success, Is.False); + Assert.That(result.Status, Is.EqualTo(CliPathSetupApplyStatus.Failed)); + Assert.That(result.ErrorOutput, Does.Contain("profile directory cannot be created")); + } + private static CliPathSetupPlan CreateZshPlan() { return new CliPathSetupPlan( @@ -98,5 +297,18 @@ private static CliPathSetupPlan CreateZshPlan() "export PATH=\"$HOME/.local/bin:$PATH\"", "printf '\\n%s\\n' 'export PATH=\"$HOME/.local/bin:$PATH\"' >> '/Users/ExampleUser/.zshrc'"); } + + private static CliPathSetupPlan CreateFishPlan() + { + return new CliPathSetupPlan( + CliPathSetupShellKind.Fish, + "fish", + true, + "/Users/ExampleUser/.local/bin", + "$HOME/.local/bin", + "/Users/ExampleUser/.config/fish/config.fish", + "fish_add_path --move \"$HOME/.local/bin\"", + "mkdir -p '/Users/ExampleUser/.config/fish' && printf '\\n%s\\n' 'fish_add_path --move \"$HOME/.local/bin\"' >> '/Users/ExampleUser/.config/fish/config.fish'"); + } } } diff --git a/Assets/Tests/Editor/CliPathSetupWriterTests.cs.meta b/Assets/Tests/Editor/CliPathSetupWriterTests.cs.meta index f4fb57ca6..6f00761dd 100644 --- a/Assets/Tests/Editor/CliPathSetupWriterTests.cs.meta +++ b/Assets/Tests/Editor/CliPathSetupWriterTests.cs.meta @@ -6,6 +6,6 @@ MonoImporter: defaultReferences: [] executionOrder: 0 icon: {instanceID: 0} - userData: - assetBundleName: - assetBundleVariant: + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/CliSetupSectionTests.cs b/Assets/Tests/Editor/CliSetupSectionTests.cs index a06895c23..dd99bf5d1 100644 --- a/Assets/Tests/Editor/CliSetupSectionTests.cs +++ b/Assets/Tests/Editor/CliSetupSectionTests.cs @@ -21,6 +21,7 @@ public class CliSetupSectionTests [TestCase(true, false, false, true, false, true, false, "2.9.0", "3.0.0", "Update CLI (v2.9.0 \u2192 v3.0.0)")] [TestCase(true, false, false, false, true, true, false, "3.1.0", "3.0.0", "Downgrade CLI (v3.1.0 \u2192 v3.0.0)")] [TestCase(true, true, false, false, false, true, false, "3.0.0", "3.0.0", "Uninstalling...")] + [TestCase(true, true, false, false, false, true, true, "3.0.0", "3.0.0", "Fixing PATH...")] [TestCase(false, true, false, false, false, false, false, null, "3.0.0", "Installing...")] [TestCase(false, false, true, false, false, false, false, null, "3.0.0", "Checking...")] public void GetInstallCliButtonText_ReturnsExpectedText( diff --git a/Assets/Tests/Editor/SetupWizardWindowTests.cs b/Assets/Tests/Editor/SetupWizardWindowTests.cs index 633bc970b..16331baa3 100644 --- a/Assets/Tests/Editor/SetupWizardWindowTests.cs +++ b/Assets/Tests/Editor/SetupWizardWindowTests.cs @@ -363,6 +363,7 @@ public void ShouldShowSkillsTargetList_WhenCliInstalledAndNotFirstInstall_Return [TestCase(true, false, false, false, true, "3.0.0", "3.0.0", "Fix PATH")] [TestCase(true, false, false, true, false, "2.9.0", "3.0.0", "Update CLI (v2.9.0 \u2192 v3.0.0)")] [TestCase(true, true, false, false, false, "3.0.0", "3.0.0", "Installing...")] + [TestCase(true, true, false, false, true, "3.0.0", "3.0.0", "Fixing PATH...")] [TestCase(false, false, true, false, false, null, "3.0.0", "Checking...")] public void GetCliButtonTextForSetupWizard_ReturnsExpectedLabel( bool cliInstalled, diff --git a/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs b/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs index 244cac40a..b9550ee18 100644 --- a/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs +++ b/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs @@ -42,6 +42,28 @@ public void ShouldRepairCliPathFromPrimaryButton_ReturnsExpectedAction( Assert.That(result, Is.EqualTo(expected)); } + [TestCase(true, "3.0.0", "3.0.0", true, "RepairPath")] + [TestCase(false, "3.0.0", "3.0.0", true, "Uninstall")] + [TestCase(false, "2.9.0", "3.0.0", true, "InstallOrUpdate")] + [TestCase(false, null, "3.0.0", true, "InstallOrUpdate")] + public void ResolveCliPrimaryButtonAction_ReturnsClickedPrimaryAction( + bool needsCliPathSetup, + string cliVersion, + string requiredCliVersion, + bool canUninstallCli, + string expected) + { + // Verifies that the Settings window preserves the primary action chosen before refresh. + UnityCliLoopSettingsWindow.CliPrimaryButtonAction result = + UnityCliLoopSettingsWindow.ResolveCliPrimaryButtonAction( + needsCliPathSetup, + cliVersion, + requiredCliVersion, + canUninstallCli); + + Assert.That(result.ToString(), Is.EqualTo(expected)); + } + [TestCase(RuntimePlatform.OSXEditor, true, true)] [TestCase(RuntimePlatform.OSXEditor, false, false)] [TestCase(RuntimePlatform.LinuxEditor, true, true)] diff --git a/Packages/src/Editor/Application/CliPathSetupPlan.cs b/Packages/src/Editor/Application/CliPathSetupPlan.cs index e62ff66b4..0bfe70180 100644 --- a/Packages/src/Editor/Application/CliPathSetupPlan.cs +++ b/Packages/src/Editor/Application/CliPathSetupPlan.cs @@ -19,11 +19,11 @@ public CliPathSetupPlan( { Debug.Assert(!string.IsNullOrWhiteSpace(installDirectory), "installDirectory must not be null or empty"); Debug.Assert(!string.IsNullOrWhiteSpace(profileInstallDirectory), "profileInstallDirectory must not be null or empty"); - Debug.Assert(!string.IsNullOrWhiteSpace(manualCommand), "manualCommand must not be null or empty"); if (canApplyAutomatically) { Debug.Assert(!string.IsNullOrWhiteSpace(configurationFilePath), "configurationFilePath must not be null or empty"); Debug.Assert(!string.IsNullOrWhiteSpace(configurationLine), "configurationLine must not be null or empty"); + Debug.Assert(!string.IsNullOrWhiteSpace(manualCommand), "manualCommand must not be null or empty"); } ShellKind = shellKind; @@ -33,7 +33,7 @@ public CliPathSetupPlan( ProfileInstallDirectory = profileInstallDirectory; ConfigurationFilePath = configurationFilePath ?? string.Empty; ConfigurationLine = configurationLine ?? string.Empty; - ManualCommand = manualCommand; + ManualCommand = manualCommand ?? string.Empty; } public CliPathSetupShellKind ShellKind { get; } diff --git a/Packages/src/Editor/Application/CliPathSetupPlan.cs.meta b/Packages/src/Editor/Application/CliPathSetupPlan.cs.meta index e7f771389..74710fa7c 100644 --- a/Packages/src/Editor/Application/CliPathSetupPlan.cs.meta +++ b/Packages/src/Editor/Application/CliPathSetupPlan.cs.meta @@ -6,6 +6,6 @@ MonoImporter: defaultReferences: [] executionOrder: 0 icon: {instanceID: 0} - userData: - assetBundleName: - assetBundleVariant: + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Infrastructure/CLI/CliPathSetupProfileResolver.cs b/Packages/src/Editor/Infrastructure/CLI/CliPathSetupProfileResolver.cs index 3387ac9e3..b52010a28 100644 --- a/Packages/src/Editor/Infrastructure/CLI/CliPathSetupProfileResolver.cs +++ b/Packages/src/Editor/Infrastructure/CLI/CliPathSetupProfileResolver.cs @@ -105,7 +105,7 @@ internal static CliPathSetupPlan ResolvePlan( configRoot, FishConfigDirectoryName, FishConfigFileName); - string configurationLine = $"fish_add_path \"{EscapeDoubleQuotedPathValue(profileInstallDirectory, false)}\""; + string configurationLine = $"fish_add_path --move \"{EscapeDoubleQuotedPathValue(profileInstallDirectory, false)}\""; return CreateSupportedPlan( CliPathSetupShellKind.Fish, shellName, @@ -149,7 +149,7 @@ private static CliPathSetupPlan CreateUnsupportedPlan(string shellName, string i installDirectory, "", "", - "export PATH=" + QuotePosixShellValue(installDirectory) + ":\"$PATH\""); + ""); } private static string SelectBashProfilePath(string homeDirectory, Func fileExists) diff --git a/Packages/src/Editor/Infrastructure/CLI/CliPathSetupProfileResolver.cs.meta b/Packages/src/Editor/Infrastructure/CLI/CliPathSetupProfileResolver.cs.meta index c5251697e..e8dc1c5cf 100644 --- a/Packages/src/Editor/Infrastructure/CLI/CliPathSetupProfileResolver.cs.meta +++ b/Packages/src/Editor/Infrastructure/CLI/CliPathSetupProfileResolver.cs.meta @@ -6,6 +6,6 @@ MonoImporter: defaultReferences: [] executionOrder: 0 icon: {instanceID: 0} - userData: - assetBundleName: - assetBundleVariant: + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Infrastructure/CLI/CliPathSetupWriter.cs b/Packages/src/Editor/Infrastructure/CLI/CliPathSetupWriter.cs index 4c483f308..2499148be 100644 --- a/Packages/src/Editor/Infrastructure/CLI/CliPathSetupWriter.cs +++ b/Packages/src/Editor/Infrastructure/CLI/CliPathSetupWriter.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Security; using UnityEngine; @@ -43,31 +44,46 @@ internal static CliPathSetupApplyResult Apply( "This shell is not supported for automatic PATH setup."); } - string existingContent = fileExists(plan.ConfigurationFilePath) - ? readAllText(plan.ConfigurationFilePath) - : string.Empty; - if (ContainsExistingPathSetup(existingContent, plan)) + try { + string existingContent = fileExists(plan.ConfigurationFilePath) + ? readAllText(plan.ConfigurationFilePath) + : string.Empty; + if (ContainsExistingPathSetup(existingContent, plan)) + { + return new CliPathSetupApplyResult( + true, + CliPathSetupApplyStatus.AlreadyConfigured, + ""); + } + + string configurationDirectory = Path.GetDirectoryName(plan.ConfigurationFilePath); + if (!string.IsNullOrEmpty(configurationDirectory)) + { + createDirectory(configurationDirectory); + } + + string prefix = NeedsLeadingNewLine(existingContent) ? "\n" : string.Empty; + appendAllText( + plan.ConfigurationFilePath, + prefix + plan.ConfigurationLine + "\n"); return new CliPathSetupApplyResult( true, - CliPathSetupApplyStatus.AlreadyConfigured, + CliPathSetupApplyStatus.Applied, ""); } - - string configurationDirectory = Path.GetDirectoryName(plan.ConfigurationFilePath); - if (!string.IsNullOrEmpty(configurationDirectory)) + catch (IOException ex) { - createDirectory(configurationDirectory); + return CreateFailedResult(ex); + } + catch (UnauthorizedAccessException ex) + { + return CreateFailedResult(ex); + } + catch (SecurityException ex) + { + return CreateFailedResult(ex); } - - string prefix = NeedsLeadingNewLine(existingContent) ? "\n" : string.Empty; - appendAllText( - plan.ConfigurationFilePath, - prefix + plan.ConfigurationLine + "\n"); - return new CliPathSetupApplyResult( - true, - CliPathSetupApplyStatus.Applied, - ""); } internal static bool ContainsExistingPathSetup(string content, CliPathSetupPlan plan) @@ -79,6 +95,7 @@ internal static bool ContainsExistingPathSetup(string content, CliPathSetupPlan string[] references = BuildReferenceCandidates(plan); string[] lines = content.Replace("\r\n", "\n").Split('\n'); + string lastPathSetupLine = null; foreach (string line in lines) { string trimmedLine = line.Trim(); @@ -88,19 +105,17 @@ internal static bool ContainsExistingPathSetup(string content, CliPathSetupPlan continue; } - if (string.Equals(trimmedLine, plan.ConfigurationLine, StringComparison.Ordinal)) + if (!LooksLikePathSetupLine(trimmedLine, plan.ShellKind)) { - return true; + continue; } - if (LooksLikePathSetupLine(trimmedLine, plan.ShellKind) - && ContainsAnyReference(trimmedLine, references)) - { - return true; - } + lastPathSetupLine = trimmedLine; } - return false; + return lastPathSetupLine != null + && (string.Equals(lastPathSetupLine, plan.ConfigurationLine, StringComparison.Ordinal) + || ContainsPrependingReference(lastPathSetupLine, references, plan.ShellKind)); } private static string[] BuildReferenceCandidates(CliPathSetupPlan plan) @@ -126,8 +141,7 @@ private static bool LooksLikePathSetupLine(string line, CliPathSetupShellKind sh if (shellKind == CliPathSetupShellKind.Fish) { return line.StartsWith("fish_add_path", StringComparison.Ordinal) - || (line.StartsWith("set", StringComparison.Ordinal) - && line.Contains("PATH")); + || IsFishPathSetCommand(line); } return line.StartsWith("PATH=", StringComparison.Ordinal) @@ -136,24 +150,272 @@ private static bool LooksLikePathSetupLine(string line, CliPathSetupShellKind sh || line.StartsWith("export PATH =", StringComparison.Ordinal); } - private static bool ContainsAnyReference(string line, string[] references) + private static bool ContainsPrependingReference( + string line, + string[] references, + CliPathSetupShellKind shellKind) { + if (shellKind == CliPathSetupShellKind.Fish + && line.StartsWith("fish_add_path", StringComparison.Ordinal)) + { + return !IsFishAddPathAppendCommand(line) + && IndexOfFirstPathEntryReference(line, references) >= 0; + } + + int referenceIndex = IndexOfFirstPathEntryReference(line, references); + if (referenceIndex < 0) + { + return false; + } + + int pathVariableIndex = IndexOfFirstPathVariableReference(line, shellKind); + if (pathVariableIndex >= 0 && referenceIndex >= pathVariableIndex) + { + return false; + } + + int firstPathEntryIndex = IndexOfFirstPathEntryStart(line, shellKind); + return firstPathEntryIndex >= 0 && referenceIndex == firstPathEntryIndex; + } + + private static int IndexOfFirstPathEntryReference(string line, string[] references) + { + int firstIndex = -1; foreach (string reference in references) { if (!string.IsNullOrWhiteSpace(reference) - && line.Contains(reference)) + && TryFindPathEntryReference(line, reference, out int index) + && (firstIndex < 0 || index < firstIndex)) + { + firstIndex = index; + } + } + + return firstIndex; + } + + private static int IndexOfFirstPathEntryStart(string line, CliPathSetupShellKind shellKind) + { + if (shellKind == CliPathSetupShellKind.Fish) + { + return IndexOfFishSetFirstPathEntryStart(line); + } + + return IndexOfPosixPathFirstEntryStart(line); + } + + private static int IndexOfPosixPathFirstEntryStart(string line) + { + int assignmentIndex = line.IndexOf('='); + if (assignmentIndex < 0) + { + return -1; + } + + return SkipPathEntryDecorators(line, assignmentIndex + 1); + } + + private static int IndexOfFishSetFirstPathEntryStart(string line) + { + TokenPosition[] tokens = SplitTokenPositions(line); + for (int i = 0; i < tokens.Length; i++) + { + string token = tokens[i].Value; + if (!string.Equals(token, "PATH", StringComparison.Ordinal) + && !string.Equals(token, "fish_user_paths", StringComparison.Ordinal)) + { + continue; + } + + if (i + 1 >= tokens.Length) + { + return -1; + } + + return SkipPathEntryDecorators(line, tokens[i + 1].StartIndex); + } + + return -1; + } + + private static int IndexOfFirstPathVariableReference(string line, CliPathSetupShellKind shellKind) + { + string[] variables = shellKind == CliPathSetupShellKind.Fish + ? new[] { "$PATH", "${PATH}", "$fish_user_paths" } + : new[] { "$PATH", "${PATH}" }; + + int firstIndex = -1; + foreach (string variable in variables) + { + int index = line.IndexOf(variable, StringComparison.Ordinal); + if (index >= 0 && (firstIndex < 0 || index < firstIndex)) + { + firstIndex = index; + } + } + + return firstIndex; + } + + private static bool IsFishAddPathAppendCommand(string line) + { + string[] tokens = line.Split( + new[] { ' ', '\t' }, + StringSplitOptions.RemoveEmptyEntries); + foreach (string token in tokens) + { + if (string.Equals(token, "--append", StringComparison.Ordinal) + || string.Equals(token, "-a", StringComparison.Ordinal)) + { + return true; + } + } + + return false; + } + + private static bool IsFishPathSetCommand(string line) + { + string[] tokens = line.Split( + new[] { ' ', '\t' }, + StringSplitOptions.RemoveEmptyEntries); + if (tokens.Length < 2 || !string.Equals(tokens[0], "set", StringComparison.Ordinal)) + { + return false; + } + + for (int i = 1; i < tokens.Length; i++) + { + string token = tokens[i]; + if (token.StartsWith("-", StringComparison.Ordinal)) + { + continue; + } + + return string.Equals(token, "PATH", StringComparison.Ordinal) + || string.Equals(token, "fish_user_paths", StringComparison.Ordinal); + } + + return false; + } + + private static TokenPosition[] SplitTokenPositions(string line) + { + List tokens = new List(); + int index = 0; + while (index < line.Length) + { + while (index < line.Length && char.IsWhiteSpace(line[index])) + { + index++; + } + + if (index >= line.Length) + { + break; + } + + int startIndex = index; + while (index < line.Length && !char.IsWhiteSpace(line[index])) + { + index++; + } + + tokens.Add(new TokenPosition( + line.Substring(startIndex, index - startIndex), + startIndex)); + } + + return tokens.ToArray(); + } + + private static int SkipPathEntryDecorators(string line, int index) + { + int currentIndex = index; + while (currentIndex < line.Length && char.IsWhiteSpace(line[currentIndex])) + { + currentIndex++; + } + + if (currentIndex < line.Length + && (line[currentIndex] == '"' || line[currentIndex] == '\'')) + { + currentIndex++; + } + + return currentIndex; + } + + private static bool TryFindPathEntryReference(string line, string reference, out int referenceIndex) + { + int searchIndex = 0; + while (searchIndex < line.Length) + { + int index = line.IndexOf(reference, searchIndex, StringComparison.Ordinal); + if (index < 0) { + referenceIndex = -1; + return false; + } + + if (HasPathEntryBoundaryBefore(line, index) + && HasPathEntryBoundaryAfter(line, index + reference.Length)) + { + referenceIndex = index; return true; } + + searchIndex = index + reference.Length; } + referenceIndex = -1; return false; } + private static bool HasPathEntryBoundaryBefore(string line, int index) + { + return index == 0 || IsPathEntryBoundary(line[index - 1]); + } + + private static bool HasPathEntryBoundaryAfter(string line, int index) + { + return index == line.Length || IsPathEntryBoundary(line[index]); + } + + private static bool IsPathEntryBoundary(char character) + { + return character == ':' + || character == '=' + || character == '"' + || character == '\'' + || char.IsWhiteSpace(character); + } + private static bool NeedsLeadingNewLine(string content) { return !string.IsNullOrEmpty(content) && !content.EndsWith("\n", StringComparison.Ordinal); } + + private static CliPathSetupApplyResult CreateFailedResult(Exception exception) + { + Debug.Assert(exception != null, "exception must not be null"); + return new CliPathSetupApplyResult( + false, + CliPathSetupApplyStatus.Failed, + exception.Message); + } + + private readonly struct TokenPosition + { + public TokenPosition(string value, int startIndex) + { + Value = value; + StartIndex = startIndex; + } + + public string Value { get; } + public int StartIndex { get; } + } } } diff --git a/Packages/src/Editor/Infrastructure/CLI/CliPathSetupWriter.cs.meta b/Packages/src/Editor/Infrastructure/CLI/CliPathSetupWriter.cs.meta index 78ff5e01d..67f4541f4 100644 --- a/Packages/src/Editor/Infrastructure/CLI/CliPathSetupWriter.cs.meta +++ b/Packages/src/Editor/Infrastructure/CLI/CliPathSetupWriter.cs.meta @@ -6,6 +6,6 @@ MonoImporter: defaultReferences: [] executionOrder: 0 icon: {instanceID: 0} - userData: - assetBundleName: - assetBundleVariant: + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Presentation/CliPathSetupPrompt.cs b/Packages/src/Editor/Presentation/CliPathSetupPrompt.cs index a3b0fc822..28390d35a 100644 --- a/Packages/src/Editor/Presentation/CliPathSetupPrompt.cs +++ b/Packages/src/Editor/Presentation/CliPathSetupPrompt.cs @@ -32,6 +32,15 @@ internal static void ShowResult(CliPathSetupFlowResult result) if (result.Status == CliPathSetupFlowStatus.ManualSetupRequired) { + if (string.IsNullOrWhiteSpace(result.Plan.ManualCommand)) + { + EditorUtility.DisplayDialog( + "Finish uLoop CLI PATH Setup", + BuildManualSetupMessage(result.Plan), + "OK"); + return; + } + bool copyCommand = EditorUtility.DisplayDialog( "Finish uLoop CLI PATH Setup", BuildManualSetupMessage(result.Plan), @@ -50,7 +59,7 @@ internal static void ShowResult(CliPathSetupFlowResult result) EditorUtility.DisplayDialog( "PATH Setup Failed", $"Could not update your shell profile.\n\n{result.ErrorOutput}\n\n" - + $"You can run this manually:\n{result.Plan.ManualCommand}", + + BuildManualCommandFallback(result.Plan), "OK"); return; } @@ -61,7 +70,7 @@ internal static void ShowResult(CliPathSetupFlowResult result) "PATH Setup Still Needed", "PATH setup was updated, but a fresh terminal still cannot find uloop.\n\n" + $"Profile: {result.Plan.ConfigurationFilePath}\n\n" - + $"You can run this manually:\n{result.Plan.ManualCommand}", + + BuildManualCommandFallback(result.Plan), "OK"); return; } @@ -80,8 +89,22 @@ private static string BuildManualSetupMessage(CliPathSetupPlan plan) return "The uLoop CLI was installed, but your terminal cannot find the uloop command yet.\n\n" + $"Detected shell: {shellName}\n" + $"Install directory: {plan.InstallDirectory}\n\n" - + "Add the install directory to PATH in your shell profile.\n\n" - + plan.ManualCommand; + + "Add the install directory to PATH in your shell profile." + + BuildOptionalManualCommand(plan); + } + + private static string BuildOptionalManualCommand(CliPathSetupPlan plan) + { + return string.IsNullOrWhiteSpace(plan.ManualCommand) + ? "" + : "\n\n" + plan.ManualCommand; + } + + private static string BuildManualCommandFallback(CliPathSetupPlan plan) + { + return string.IsNullOrWhiteSpace(plan.ManualCommand) + ? $"Add this directory to PATH in your shell profile:\n{plan.InstallDirectory}" + : $"You can run this manually:\n{plan.ManualCommand}"; } private static string BuildCompleteMessage(CliPathSetupFlowResult result) diff --git a/Packages/src/Editor/Presentation/CliPathSetupPrompt.cs.meta b/Packages/src/Editor/Presentation/CliPathSetupPrompt.cs.meta index 75b87b7ad..cfaad5b69 100644 --- a/Packages/src/Editor/Presentation/CliPathSetupPrompt.cs.meta +++ b/Packages/src/Editor/Presentation/CliPathSetupPrompt.cs.meta @@ -6,6 +6,6 @@ MonoImporter: defaultReferences: [] executionOrder: 0 icon: {instanceID: 0} - userData: - assetBundleName: - assetBundleVariant: + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs b/Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs index 4eef9045a..9cefb40db 100644 --- a/Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs +++ b/Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs @@ -701,14 +701,14 @@ internal static string GetCliButtonTextForSetupWizard( return "Checking..."; } - if (isInstallingCli) + if (needsCliPathSetup) { - return "Installing..."; + return isInstallingCli ? "Fixing PATH..." : "Fix PATH"; } - if (needsCliPathSetup) + if (isInstallingCli) { - return "Fix PATH"; + return "Installing..."; } if (!cliInstalled) diff --git a/Packages/src/Editor/Presentation/UIToolkit/Components/CliSetupSection.cs b/Packages/src/Editor/Presentation/UIToolkit/Components/CliSetupSection.cs index 2e21bbe3c..e75840358 100644 --- a/Packages/src/Editor/Presentation/UIToolkit/Components/CliSetupSection.cs +++ b/Packages/src/Editor/Presentation/UIToolkit/Components/CliSetupSection.cs @@ -213,15 +213,15 @@ internal static string GetInstallCliButtonText( return "Checking..."; } - bool isUninstallAction = IsUninstallCliAction(isCliInstalled, needsUpdate, needsDowngrade, canUninstallCli); - if (isInstallingCli) + if (needsCliPathSetup) { - return isUninstallAction ? "Uninstalling..." : "Installing..."; + return isInstallingCli ? "Fixing PATH..." : "Fix PATH"; } - if (needsCliPathSetup) + bool isUninstallAction = IsUninstallCliAction(isCliInstalled, needsUpdate, needsDowngrade, canUninstallCli); + if (isInstallingCli) { - return "Fix PATH"; + return isUninstallAction ? "Uninstalling..." : "Installing..."; } if (!isCliInstalled) diff --git a/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs b/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs index 5653ffb1e..b5a10e9b5 100644 --- a/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs +++ b/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs @@ -17,6 +17,13 @@ namespace io.github.hatayama.UnityCliLoop.Presentation /// public class UnityCliLoopSettingsWindow : EditorWindow { + internal enum CliPrimaryButtonAction + { + InstallOrUpdate, + RepairPath, + Uninstall + } + private const bool ForceFlatSkillInstall = true; private const double DeferredInitialRefreshDelaySeconds = 0.05; private const double ToolSettingsRegistryWarmupInitialDelaySeconds = 0.05; @@ -760,15 +767,17 @@ private void CancelSkillInstallStateRefresh() private async void HandleInstallCli() { + CliPrimaryButtonAction clickedAction = GetCurrentCliPrimaryButtonAction(); + await RefreshCliPrimaryActionStateAsync(CancellationToken.None); - if (ShouldRepairCliPathFromPrimaryButton(_needsCliPathSetup)) + if (clickedAction == CliPrimaryButtonAction.RepairPath) { await HandleRepairCliPathSetup(); return; } - if (ShouldUninstallCliFromPrimaryButton()) + if (clickedAction == CliPrimaryButtonAction.Uninstall) { await HandleUninstallCli(); return; @@ -811,6 +820,20 @@ await CliPathSetupPrompt.EnsureVisibleAndShowResultAsync( } } + private CliPrimaryButtonAction GetCurrentCliPrimaryButtonAction() + { + string cliVersion = CliSetupApplicationFacade.GetCachedCliVersion(); + string cliExecutablePath = CliSetupApplicationFacade.GetCachedCliExecutablePath(); + bool canUninstallCli = CliSetupApplicationFacade.IsPackageOwnedCurrentUserInstallPath( + cliExecutablePath, + UnityEngine.Application.platform); + return ResolveCliPrimaryButtonAction( + _needsCliPathSetup, + cliVersion, + GetMinimumRequiredCliVersion(), + canUninstallCli); + } + private bool ShouldUninstallCliFromPrimaryButton() { string cliVersion = CliSetupApplicationFacade.GetCachedCliVersion(); @@ -834,6 +857,25 @@ internal static bool ShouldUninstallCliFromPrimaryButton( return CliSetupSection.IsUninstallCliAction(isCliInstalled, needsUpdate, needsDowngrade: false, canUninstallCli); } + internal static CliPrimaryButtonAction ResolveCliPrimaryButtonAction( + bool needsCliPathSetup, + string cliVersion, + string requiredCliVersion, + bool canUninstallCli) + { + if (ShouldRepairCliPathFromPrimaryButton(needsCliPathSetup)) + { + return CliPrimaryButtonAction.RepairPath; + } + + if (ShouldUninstallCliFromPrimaryButton(cliVersion, requiredCliVersion, canUninstallCli)) + { + return CliPrimaryButtonAction.Uninstall; + } + + return CliPrimaryButtonAction.InstallOrUpdate; + } + internal static bool ShouldRepairCliPathFromPrimaryButton(bool needsCliPathSetup) { return needsCliPathSetup; diff --git a/docs/architecture/cli-path-setup-rebuild-plan.md b/docs/architecture/cli-path-setup-rebuild-plan.md index dda7903c8..fc7d3d3e8 100644 --- a/docs/architecture/cli-path-setup-rebuild-plan.md +++ b/docs/architecture/cli-path-setup-rebuild-plan.md @@ -41,7 +41,7 @@ fish の `$status` だけは POSIX shell と異なるため、shell kind ごと 自動追記する行は canonical line だけにする。 - zsh / bash: `export PATH="$HOME/.local/bin:$PATH"` -- fish: `fish_add_path "$HOME/.local/bin"` +- fish: `fish_add_path --move "$HOME/.local/bin"` 既存設定判定は次の順で十分とする。 diff --git a/scripts/install.sh b/scripts/install.sh index f9d495ab6..35391863e 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -212,9 +212,9 @@ print_path_setup_guidance() { profile_path=$(detect_fish_profile_path) echo "Detected shell: fish" echo "Add this line to $profile_path:" - echo " fish_add_path \"$INSTALL_DIR\"" + echo " fish_add_path --move \"$INSTALL_DIR\"" echo "Or run:" - print_fish_append_command "fish_add_path \"$INSTALL_DIR\"" "$profile_path" + print_fish_append_command "fish_add_path --move \"$INSTALL_DIR\"" "$profile_path" ;; *) echo "Add this directory to PATH in your shell profile:" diff --git a/scripts/test-install-release-filter.sh b/scripts/test-install-release-filter.sh index 1911709d8..cdf455efb 100755 --- a/scripts/test-install-release-filter.sh +++ b/scripts/test-install-release-filter.sh @@ -471,8 +471,8 @@ test_posix_prints_fish_path_guidance_without_writing_profile() { assert_contains "$work_dir/output.txt" "Detected shell: fish" assert_contains "$work_dir/output.txt" "Add this line to $profile_path:" - assert_contains "$work_dir/output.txt" " fish_add_path \"$install_dir\"" - assert_contains "$work_dir/output.txt" "mkdir -p '$xdg_config_home/fish' && printf '\\n%s\\n' 'fish_add_path \"$install_dir\"' >> '$profile_path'" + assert_contains "$work_dir/output.txt" " fish_add_path --move \"$install_dir\"" + assert_contains "$work_dir/output.txt" "mkdir -p '$xdg_config_home/fish' && printf '\\n%s\\n' 'fish_add_path --move \"$install_dir\"' >> '$profile_path'" assert_file_not_exists "$profile_path" } From f07963c1e00252c5c86a11da42b5920fe44632b1 Mon Sep 17 00:00:00 2001 From: hatayama Date: Thu, 21 May 2026 00:46:59 +0900 Subject: [PATCH 4/5] Fix CLI PATH review feedback Resolve review feedback that could make terminal visibility checks use the wrong shell syntax, produce misleading manual PATH guidance when the install directory is unavailable, or leave the Setup Wizard CLI action stuck in a checking state. Remove the temporary rebuild plan document now that the implementation is represented by code and tests. --- .../Editor/CliInstallationDetectorTests.cs | 12 ++ .../CliPathSetupProfileResolverTests.cs | 20 ++++ .../Editor/Application/CliPathSetupPlan.cs | 8 +- .../CLI/CliInstallationDetector.cs | 13 +- .../CLI/CliPathSetupProfileResolver.cs | 21 ++-- .../Editor/Presentation/CliPathSetupPrompt.cs | 20 +++- .../Presentation/Setup/SetupWizardWindow.cs | 15 ++- .../cli-path-setup-rebuild-plan.md | 113 ------------------ 8 files changed, 87 insertions(+), 135 deletions(-) delete mode 100644 docs/architecture/cli-path-setup-rebuild-plan.md diff --git a/Assets/Tests/Editor/CliInstallationDetectorTests.cs b/Assets/Tests/Editor/CliInstallationDetectorTests.cs index 3199d06d6..e566d9fb6 100644 --- a/Assets/Tests/Editor/CliInstallationDetectorTests.cs +++ b/Assets/Tests/Editor/CliInstallationDetectorTests.cs @@ -119,6 +119,18 @@ public void BuildShellCliDetectionCommand_UsesShortVersionFlag() Assert.That(command, Does.Not.Contain("uloop --version")); } + [Test] + public void BuildShellCliDetectionCommandForShell_WhenRuntimeShellIsFish_UsesFishStatusSyntax() + { + // Verifies that command syntax follows the actual shell process, not PATH setup support. + string command = CliInstallationDetector.BuildShellCliDetectionCommandForShell( + "uloop", + "/opt/homebrew/bin/fish"); + + Assert.That(command, Does.Contain("set uloop_version_status $status")); + Assert.That(command, Does.Not.Contain("uloop_version_status=$?")); + } + [Test] public void ParseShellCliInstallationOutput_WhenPathAndVersionExist_ReturnsDetection() { diff --git a/Assets/Tests/Editor/CliPathSetupProfileResolverTests.cs b/Assets/Tests/Editor/CliPathSetupProfileResolverTests.cs index cd2369a43..851e9616c 100644 --- a/Assets/Tests/Editor/CliPathSetupProfileResolverTests.cs +++ b/Assets/Tests/Editor/CliPathSetupProfileResolverTests.cs @@ -84,5 +84,25 @@ public void ResolvePlan_WhenUnsupportedShellDisablesAutomaticApply() Assert.That(plan.CanApplyAutomatically, Is.False); Assert.That(plan.ManualCommand, Is.Empty); } + + [Test] + public void ResolvePlan_WhenInstallDirectoryIsMissingDoesNotUseExecutableNameAsDirectory() + { + // Verifies that missing install roots do not produce misleading PATH directory guidance. + CliPathSetupPlan plan = CliPathSetupProfileResolver.ResolvePlan( + RuntimePlatform.OSXEditor, + "/bin/zsh", + "/Users/ExampleUser", + null, + null, + "", + path => false); + + Assert.That(plan.ShellKind, Is.EqualTo(CliPathSetupShellKind.Unsupported)); + Assert.That(plan.CanApplyAutomatically, Is.False); + Assert.That(plan.InstallDirectory, Is.Empty); + Assert.That(plan.ProfileInstallDirectory, Is.Empty); + Assert.That(plan.ManualCommand, Is.Empty); + } } } diff --git a/Packages/src/Editor/Application/CliPathSetupPlan.cs b/Packages/src/Editor/Application/CliPathSetupPlan.cs index 0bfe70180..6cb8f9a22 100644 --- a/Packages/src/Editor/Application/CliPathSetupPlan.cs +++ b/Packages/src/Editor/Application/CliPathSetupPlan.cs @@ -17,10 +17,10 @@ public CliPathSetupPlan( string configurationLine, string manualCommand) { - Debug.Assert(!string.IsNullOrWhiteSpace(installDirectory), "installDirectory must not be null or empty"); - Debug.Assert(!string.IsNullOrWhiteSpace(profileInstallDirectory), "profileInstallDirectory must not be null or empty"); if (canApplyAutomatically) { + Debug.Assert(!string.IsNullOrWhiteSpace(installDirectory), "installDirectory must not be null or empty"); + Debug.Assert(!string.IsNullOrWhiteSpace(profileInstallDirectory), "profileInstallDirectory must not be null or empty"); Debug.Assert(!string.IsNullOrWhiteSpace(configurationFilePath), "configurationFilePath must not be null or empty"); Debug.Assert(!string.IsNullOrWhiteSpace(configurationLine), "configurationLine must not be null or empty"); Debug.Assert(!string.IsNullOrWhiteSpace(manualCommand), "manualCommand must not be null or empty"); @@ -29,8 +29,8 @@ public CliPathSetupPlan( ShellKind = shellKind; ShellName = shellName ?? string.Empty; CanApplyAutomatically = canApplyAutomatically; - InstallDirectory = installDirectory; - ProfileInstallDirectory = profileInstallDirectory; + InstallDirectory = installDirectory ?? string.Empty; + ProfileInstallDirectory = profileInstallDirectory ?? string.Empty; ConfigurationFilePath = configurationFilePath ?? string.Empty; ConfigurationLine = configurationLine ?? string.Empty; ManualCommand = manualCommand ?? string.Empty; diff --git a/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs b/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs index 088e55a2c..bfe6cc9b4 100644 --- a/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs +++ b/Packages/src/Editor/Infrastructure/CLI/CliInstallationDetector.cs @@ -198,9 +198,9 @@ internal static ProcessStartInfo BuildShellCliDetectionStartInfo( ProcessStartInfo startInfo = new() { FileName = shell, - Arguments = "-l -i -c " + QuoteProcessArgument(BuildShellCliDetectionCommandForPlan( + Arguments = "-l -i -c " + QuoteProcessArgument(BuildShellCliDetectionCommandForShell( CliConstants.EXECUTABLE_NAME, - pathSetupPlan)), + shell)), UseShellExecute = false, RedirectStandardOutput = true, RedirectStandardError = true, @@ -235,11 +235,14 @@ internal static bool IsShellDetectionUsableForPathSetup( CliConstants.MINIMUM_REQUIRED_CLI_VERSION); } - internal static string BuildShellCliDetectionCommandForPlan( + internal static string BuildShellCliDetectionCommandForShell( string executableName, - CliPathSetupPlan pathSetupPlan) + string shellPath) { - if (pathSetupPlan.ShellKind == CliPathSetupShellKind.Fish) + string shellName = string.IsNullOrWhiteSpace(shellPath) + ? string.Empty + : Path.GetFileName(shellPath.Trim()).ToLowerInvariant(); + if (string.Equals(shellName, "fish", StringComparison.Ordinal)) { return BuildFishShellCliDetectionCommand(executableName); } diff --git a/Packages/src/Editor/Infrastructure/CLI/CliPathSetupProfileResolver.cs b/Packages/src/Editor/Infrastructure/CLI/CliPathSetupProfileResolver.cs index b52010a28..7f9712554 100644 --- a/Packages/src/Editor/Infrastructure/CLI/CliPathSetupProfileResolver.cs +++ b/Packages/src/Editor/Infrastructure/CLI/CliPathSetupProfileResolver.cs @@ -27,14 +27,15 @@ internal static class CliPathSetupProfileResolver public static CliPathSetupPlan ResolveCurrentUserPlan(RuntimePlatform platform) { string installDirectory = NativeCliInstaller.GetCurrentUserGlobalCliInstallDirectory(platform); + string shellPath = NodeEnvironmentResolver.GetUserShell(); if (string.IsNullOrWhiteSpace(installDirectory)) { - return CreateUnsupportedPlan("unknown", CliConstants.EXECUTABLE_NAME); + return CreateUnsupportedPlan(GetShellName(shellPath), string.Empty); } return ResolvePlan( platform, - NodeEnvironmentResolver.GetUserShell(), + shellPath, Environment.GetEnvironmentVariable(CliConstants.POSIX_HOME_ENVIRONMENT_VARIABLE), Environment.GetEnvironmentVariable("ZDOTDIR"), Environment.GetEnvironmentVariable("XDG_CONFIG_HOME"), @@ -51,9 +52,14 @@ internal static CliPathSetupPlan ResolvePlan( string installDirectory, Func fileExists) { - Debug.Assert(!string.IsNullOrWhiteSpace(installDirectory), "installDirectory must not be null or empty"); Debug.Assert(fileExists != null, "fileExists must not be null"); + string shellName = GetShellName(shellPath); + if (string.IsNullOrWhiteSpace(installDirectory)) + { + return CreateUnsupportedPlan(shellName, string.Empty); + } + if (platform == RuntimePlatform.WindowsEditor) { return CreateUnsupportedPlan("windows", installDirectory); @@ -62,7 +68,6 @@ internal static CliPathSetupPlan ResolvePlan( string resolvedHomeDirectory = string.IsNullOrWhiteSpace(homeDirectory) ? Environment.GetFolderPath(Environment.SpecialFolder.UserProfile) : homeDirectory; - string shellName = GetShellName(shellPath); string profileInstallDirectory = FormatInstallDirectoryForProfile( installDirectory, resolvedHomeDirectory); @@ -141,12 +146,14 @@ private static CliPathSetupPlan CreateSupportedPlan( private static CliPathSetupPlan CreateUnsupportedPlan(string shellName, string installDirectory) { + string displayShellName = string.IsNullOrWhiteSpace(shellName) ? "unknown" : shellName; + string displayInstallDirectory = installDirectory ?? string.Empty; return new CliPathSetupPlan( CliPathSetupShellKind.Unsupported, - shellName, + displayShellName, false, - installDirectory, - installDirectory, + displayInstallDirectory, + displayInstallDirectory, "", "", ""); diff --git a/Packages/src/Editor/Presentation/CliPathSetupPrompt.cs b/Packages/src/Editor/Presentation/CliPathSetupPrompt.cs index 28390d35a..e7e60a198 100644 --- a/Packages/src/Editor/Presentation/CliPathSetupPrompt.cs +++ b/Packages/src/Editor/Presentation/CliPathSetupPrompt.cs @@ -88,11 +88,18 @@ private static string BuildManualSetupMessage(CliPathSetupPlan plan) : plan.ShellName; return "The uLoop CLI was installed, but your terminal cannot find the uloop command yet.\n\n" + $"Detected shell: {shellName}\n" - + $"Install directory: {plan.InstallDirectory}\n\n" + + BuildInstallDirectoryLine(plan) + "Add the install directory to PATH in your shell profile." + BuildOptionalManualCommand(plan); } + private static string BuildInstallDirectoryLine(CliPathSetupPlan plan) + { + return string.IsNullOrWhiteSpace(plan.InstallDirectory) + ? "Install directory: unavailable\n\n" + : $"Install directory: {plan.InstallDirectory}\n\n"; + } + private static string BuildOptionalManualCommand(CliPathSetupPlan plan) { return string.IsNullOrWhiteSpace(plan.ManualCommand) @@ -102,9 +109,14 @@ private static string BuildOptionalManualCommand(CliPathSetupPlan plan) private static string BuildManualCommandFallback(CliPathSetupPlan plan) { - return string.IsNullOrWhiteSpace(plan.ManualCommand) - ? $"Add this directory to PATH in your shell profile:\n{plan.InstallDirectory}" - : $"You can run this manually:\n{plan.ManualCommand}"; + if (!string.IsNullOrWhiteSpace(plan.ManualCommand)) + { + return $"You can run this manually:\n{plan.ManualCommand}"; + } + + return string.IsNullOrWhiteSpace(plan.InstallDirectory) + ? "Could not determine the CLI install directory. Add the directory that contains uloop to PATH manually." + : $"Add this directory to PATH in your shell profile:\n{plan.InstallDirectory}"; } private static string BuildCompleteMessage(CliPathSetupFlowResult result) diff --git a/Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs b/Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs index 9cefb40db..5bce5e0e0 100644 --- a/Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs +++ b/Packages/src/Editor/Presentation/Setup/SetupWizardWindow.cs @@ -1021,12 +1021,23 @@ private async Task RefreshCliPrimaryActionStateAsync(CancellationToken ct) _installCliButton.SetEnabled(false); _installCliButton.text = "Checking..."; - await CliSetupApplicationFacade.ForceRefreshCliVersionAsync(ct); + try + { + await CliSetupApplicationFacade.ForceRefreshCliVersionAsync(ct); + _needsCliPathSetup = await ShouldRepairCliPathSetupAsync(ct); + } + finally + { + RefreshCliStepFromCachedState(); + } + } + + private void RefreshCliStepFromCachedState() + { string cliVersion = CliSetupApplicationFacade.GetCachedCliVersion(); string requiredCliVersion = GetMinimumRequiredCliVersion(); bool cliInstalled = IsCliInstalled(cliVersion); bool cliVersionMatched = IsCliVersionSatisfied(cliVersion, requiredCliVersion) && cliInstalled; - _needsCliPathSetup = await ShouldRepairCliPathSetupAsync(ct); UpdateCliStep(cliInstalled, cliVersion, requiredCliVersion, cliVersionMatched); } diff --git a/docs/architecture/cli-path-setup-rebuild-plan.md b/docs/architecture/cli-path-setup-rebuild-plan.md deleted file mode 100644 index fc7d3d3e8..000000000 --- a/docs/architecture/cli-path-setup-rebuild-plan.md +++ /dev/null @@ -1,113 +0,0 @@ -# CLI PATH Setup Rebuild Plan - -## Goal - -Unity UI から CLI を入れる初心者向けフローでは、インストール後に fresh login interactive shell で `uloop` が解決できるかを実測し、見えない場合はユーザー操作を追加で求めずに shell 設定ファイルへ最小の PATH 行を追記する。 - -コマンドライン installer から入れる上級者向けフローでは、shell 設定ファイルを変更せず、`~/.local/bin` が `PATH` に無い場合だけ、ユーザーが自分で追記できる短い案内を表示する。 - -## Current Branch Reference - -参考実装は `/Users/masamichi/work/unity-cli-loop2` の `codex/fix-v3-cli-path-setup-ui` ブランチに置かれている。 - -そこから再利用する考え方は次の通り。 - -- Unity の起動時に持っていた stale `PATH` を信用しない。 -- fresh login interactive shell で `command -v uloop` と `uloop -v` を実行して判断する。 -- CLI installer は shell profile を勝手に変更しない。 -- UI からのインストールでは、shell profile 変更を初心者向け自動セットアップの一部として扱う。 -- zsh / bash / fish 以外は自動追記しない。 -- 追記前には既存設定を確認し、ファイル末尾に改行が無い場合でも行を連結しない。 - -捨てる複雑さは次の通り。 - -- shell profile 全体を静的解析して PATH の実効値を推測しない。 -- zsh / bash / fish のあらゆる PATH 記法を解釈しようとしない。 -- `ZDOTDIR` や login hook の推測を installer script 側へ過剰に持ち込まない。 -- UI と Setup Wizard の PATH repair 手順を別々に増やさない。 - -## Design - -### 1. Terminal visibility is authoritative - -CLI が terminal から使えるかどうかは、常に shell 実行結果で判断する。 - -POSIX ではユーザー shell を `-l -i -c` で起動し、`command -v uloop` と `uloop -v` を marker 付きで取得する。Unity プロセスが保持している `PATH` から package install directory を一時的に除外し、Unity 起動時点の stale `PATH` で誤判定しないようにする。 - -fish の `$status` だけは POSIX shell と異なるため、shell kind ごとの小さい command builder を用意する。 - -### 2. Profile writes are intentionally boring - -自動追記する行は canonical line だけにする。 - -- zsh / bash: `export PATH="$HOME/.local/bin:$PATH"` -- fish: `fish_add_path --move "$HOME/.local/bin"` - -既存設定判定は次の順で十分とする。 - -1. 自分たちが書く canonical line がコメントでない行として存在する。 -2. コメントでない行に install directory の literal path または `$HOME` 形式が PATH 設定として含まれる。 - -この判定は「重複追記を避けるための軽い確認」であり、正しさの最終判断には使わない。最終判断は追記後の fresh shell visibility check で行う。 - -### 3. Profile target selection is simple - -自動追記先は shell kind ごとに限定する。 - -- zsh: `$ZDOTDIR/.zshrc` が使えるならそれを使う。`ZDOTDIR` が無ければ `$HOME/.zshrc`。 -- bash: 既存の `$HOME/.bash_profile`、`$HOME/.bash_login`、`$HOME/.profile` の順で選び、無ければ `$HOME/.bash_profile`。 -- fish: `${XDG_CONFIG_HOME:-$HOME/.config}/fish/config.fish`。 - -`ZDOTDIR` を login shell から探りに行く処理は採用しない。必要なら将来、実際の不具合が出た時に小さく追加する。 - -### 4. One application flow owns PATH setup - -UI と Setup Wizard は、それぞれが shell profile の詳細を知らない。 - -共通の application service が次を行う。 - -1. CLI install または repair ボタン押下を受ける。 -2. fresh shell visibility check を実行する。 -3. 見えなければ shell kind に応じた profile plan を作る。 -4. 対応 shell なら profile に追記する。 -5. もう一度 fresh shell visibility check を実行する。 -6. 成功、未対応 shell、書き込み失敗、追記済みだが未解決、の結果を UI に返す。 - -### 5. Command installer remains read-only for profiles - -`scripts/install.sh` は `~/.local/bin` へ CLI を置く。`PATH` に含まれていない場合は案内だけ出す。 - -案内は zsh / bash / fish ごとに最低限の追記コマンドを表示する。設定ファイルの探索や推測は UI 側ほど行わない。 - -## ToDo - -- [ ] 参照 checkout の差分から必要な挙動だけを抽出する。 -- [ ] 方針ファイルを先に commit する。 -- [ ] `CliPathSetupPlan` 系 DTO を小さく作る。 -- [ ] `CliPathSetupProfileResolver` を作り、shell kind と設定ファイルだけを決める。 -- [ ] `CliPathSetupWriter` を作り、canonical line の重複防止と newline-safe append を行う。 -- [ ] `CliTerminalVisibilityChecker` を作り、fresh shell 実測を担当させる。 -- [ ] `CliSetupApplicationService` に install / repair 後の PATH setup flow を集約する。 -- [ ] Settings Window と Setup Wizard は共通 flow の結果表示だけに寄せる。 -- [ ] `scripts/install.sh` を profile 非変更、案内のみの実装へ更新する。 -- [ ] C# unit tests を Red-Green-Refactor で追加する。 -- [ ] shell script tests を追加し、profile を作成・変更しないことを確認する。 -- [ ] `sh -n scripts/install.sh` を通す。 -- [ ] `scripts/test-install-release-filter.sh` を通す。 -- [ ] focused EditMode tests を通す。 -- [ ] `Packages/src/Cli~/dist/darwin-arm64/uloop compile --project-path /Users/masamichi/work/unity-cli-loop` を通す。 -- [ ] 関心ごとごとに commit する。 -- [ ] commit 後に difit を表示する。 - -## Completion Criteria - -- 新ブランチが `origin/v3-beta` 由来である。 -- この方針ファイルが実装前の判断材料として残っている。 -- UI install / repair は stale Unity `PATH` を信用せず、fresh shell で `uloop` が見えない時だけ PATH setup を実行する。 -- UI 自動追記は zsh / bash / fish のみで行う。 -- 未対応 shell は自動追記せず、manual command を表示できる。 -- command installer は shell profile を作成・変更しない。 -- profile 追記は末尾改行なしファイルでも安全に行う。 -- 重複判定は canonical line または明確な install directory PATH 設定だけを対象にする。 -- 参照 checkout にあった巨大な shell profile 静的解析を持ち込まない。 -- 検証コマンドが通り、commit が作成されている。 From 61571939ff75f84176f312f5bc71a23602c6162e Mon Sep 17 00:00:00 2001 From: hatayama Date: Thu, 21 May 2026 01:04:00 +0900 Subject: [PATCH 5/5] Guard CLI action refresh edge cases Prevent the Settings primary button from executing a stale destructive action after refreshing CLI and PATH state. Return failed PATH setup results for invalid or unsupported profile paths so UI repair flows can surface the error instead of throwing. --- .../Tests/Editor/CliPathSetupWriterTests.cs | 36 +++++++++++++++++++ ...nityCliLoopSettingsWindowCliActionTests.cs | 27 ++++++++++++++ .../Infrastructure/CLI/CliPathSetupWriter.cs | 8 +++++ .../UnityCliLoopSettingsWindow.cs | 31 ++++++++++++++-- 4 files changed, 100 insertions(+), 2 deletions(-) diff --git a/Assets/Tests/Editor/CliPathSetupWriterTests.cs b/Assets/Tests/Editor/CliPathSetupWriterTests.cs index 8d4c4600b..a9db82b48 100644 --- a/Assets/Tests/Editor/CliPathSetupWriterTests.cs +++ b/Assets/Tests/Editor/CliPathSetupWriterTests.cs @@ -285,6 +285,42 @@ public void Apply_WhenProfileDirectoryCreationFailsReturnsFailedResult() Assert.That(result.ErrorOutput, Does.Contain("profile directory cannot be created")); } + [Test] + public void Apply_WhenProfilePathIsInvalidReturnsFailedResult() + { + // Verifies that invalid profile paths fall back to the manual setup result path. + CliPathSetupPlan plan = CreateZshPlan(); + + CliPathSetupApplyResult result = CliPathSetupWriter.Apply( + plan, + path => throw new ArgumentException("profile path is invalid"), + path => string.Empty, + path => new DirectoryInfo(path), + (path, content) => { }); + + Assert.That(result.Success, Is.False); + Assert.That(result.Status, Is.EqualTo(CliPathSetupApplyStatus.Failed)); + Assert.That(result.ErrorOutput, Does.Contain("profile path is invalid")); + } + + [Test] + public void Apply_WhenProfilePathIsNotSupportedReturnsFailedResult() + { + // Verifies that unsupported profile path formats fall back to the manual setup result path. + CliPathSetupPlan plan = CreateZshPlan(); + + CliPathSetupApplyResult result = CliPathSetupWriter.Apply( + plan, + path => throw new NotSupportedException("profile path is not supported"), + path => string.Empty, + path => new DirectoryInfo(path), + (path, content) => { }); + + Assert.That(result.Success, Is.False); + Assert.That(result.Status, Is.EqualTo(CliPathSetupApplyStatus.Failed)); + Assert.That(result.ErrorOutput, Does.Contain("profile path is not supported")); + } + private static CliPathSetupPlan CreateZshPlan() { return new CliPathSetupPlan( diff --git a/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs b/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs index b9550ee18..c71a9e070 100644 --- a/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs +++ b/Assets/Tests/Editor/UnityCliLoopSettingsWindowCliActionTests.cs @@ -64,6 +64,27 @@ public void ResolveCliPrimaryButtonAction_ReturnsClickedPrimaryAction( Assert.That(result.ToString(), Is.EqualTo(expected)); } + [TestCase("InstallOrUpdate", "InstallOrUpdate", "InstallOrUpdate")] + [TestCase("RepairPath", "RepairPath", "RepairPath")] + [TestCase("Uninstall", "Uninstall", "Uninstall")] + [TestCase("InstallOrUpdate", "RepairPath", "RepairPath")] + [TestCase("InstallOrUpdate", "Uninstall", "None")] + [TestCase("Uninstall", "InstallOrUpdate", "None")] + [TestCase("RepairPath", "Uninstall", "None")] + public void ResolveExecutableCliPrimaryButtonAction_IgnoresUnsafeStaleActions( + string clickedAction, + string refreshedAction, + string expected) + { + // Verifies that a refreshed Settings state cannot turn a stale click into a destructive action. + UnityCliLoopSettingsWindow.CliPrimaryButtonAction result = + UnityCliLoopSettingsWindow.ResolveExecutableCliPrimaryButtonAction( + ParseAction(clickedAction), + ParseAction(refreshedAction)); + + Assert.That(result.ToString(), Is.EqualTo(expected)); + } + [TestCase(RuntimePlatform.OSXEditor, true, true)] [TestCase(RuntimePlatform.OSXEditor, false, false)] [TestCase(RuntimePlatform.LinuxEditor, true, true)] @@ -94,5 +115,11 @@ public void IsCliUpdateNeeded_UsesMinimumRequiredCliVersion( Assert.That(result, Is.EqualTo(expected)); } + + private static UnityCliLoopSettingsWindow.CliPrimaryButtonAction ParseAction(string action) + { + return (UnityCliLoopSettingsWindow.CliPrimaryButtonAction) + System.Enum.Parse(typeof(UnityCliLoopSettingsWindow.CliPrimaryButtonAction), action); + } } } diff --git a/Packages/src/Editor/Infrastructure/CLI/CliPathSetupWriter.cs b/Packages/src/Editor/Infrastructure/CLI/CliPathSetupWriter.cs index 2499148be..8304fca21 100644 --- a/Packages/src/Editor/Infrastructure/CLI/CliPathSetupWriter.cs +++ b/Packages/src/Editor/Infrastructure/CLI/CliPathSetupWriter.cs @@ -76,6 +76,14 @@ internal static CliPathSetupApplyResult Apply( { return CreateFailedResult(ex); } + catch (ArgumentException ex) + { + return CreateFailedResult(ex); + } + catch (NotSupportedException ex) + { + return CreateFailedResult(ex); + } catch (UnauthorizedAccessException ex) { return CreateFailedResult(ex); diff --git a/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs b/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs index b5a10e9b5..d9535ada2 100644 --- a/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs +++ b/Packages/src/Editor/Presentation/UnityCliLoopSettingsWindow.cs @@ -19,6 +19,7 @@ public class UnityCliLoopSettingsWindow : EditorWindow { internal enum CliPrimaryButtonAction { + None, InstallOrUpdate, RepairPath, Uninstall @@ -770,14 +771,22 @@ private async void HandleInstallCli() CliPrimaryButtonAction clickedAction = GetCurrentCliPrimaryButtonAction(); await RefreshCliPrimaryActionStateAsync(CancellationToken.None); + CliPrimaryButtonAction refreshedAction = GetCurrentCliPrimaryButtonAction(); + CliPrimaryButtonAction executableAction = ResolveExecutableCliPrimaryButtonAction( + clickedAction, + refreshedAction); + if (executableAction == CliPrimaryButtonAction.None) + { + return; + } - if (clickedAction == CliPrimaryButtonAction.RepairPath) + if (executableAction == CliPrimaryButtonAction.RepairPath) { await HandleRepairCliPathSetup(); return; } - if (clickedAction == CliPrimaryButtonAction.Uninstall) + if (executableAction == CliPrimaryButtonAction.Uninstall) { await HandleUninstallCli(); return; @@ -876,6 +885,24 @@ internal static CliPrimaryButtonAction ResolveCliPrimaryButtonAction( return CliPrimaryButtonAction.InstallOrUpdate; } + internal static CliPrimaryButtonAction ResolveExecutableCliPrimaryButtonAction( + CliPrimaryButtonAction clickedAction, + CliPrimaryButtonAction refreshedAction) + { + if (clickedAction == refreshedAction) + { + return clickedAction; + } + + if (clickedAction == CliPrimaryButtonAction.InstallOrUpdate + && refreshedAction == CliPrimaryButtonAction.RepairPath) + { + return CliPrimaryButtonAction.RepairPath; + } + + return CliPrimaryButtonAction.None; + } + internal static bool ShouldRepairCliPathFromPrimaryButton(bool needsCliPathSetup) { return needsCliPathSetup;