From 077958a774d188ae46493c483182a1c4d3ffda18 Mon Sep 17 00:00:00 2001 From: hatayama Date: Sun, 25 Oct 2026 22:10:43 +0900 Subject: [PATCH 1/9] Let uloop skills install at the Git repository root Teams split on where their agents start: some open the Unity project, others open the Git repository that contains it, and skills installed under the Unity root are invisible to the latter. - Add --root unity|git to skills list/install/uninstall and the V3 migration subcommands; git resolves to the nearest ancestor of the Unity project root (inclusive) holding a .git directory or file, and fails instead of falling back when there is none - Without --root, follow installRoot saved by the Editor in .uloop/settings.skills.json so the CLI and the Editor install into the same place; --global and --output-dir ignore it - Only the destination moves: skill sources and disabledTools still come from the Unity project root - Move the target definitions into skills_targets.go to keep skills.go under the 500-line limit --- README.md | 3 + README_ja.md | 3 + cli/dispatcher/internal/dispatcher/skills.go | 84 ++-- .../internal/dispatcher/skills_display.go | 12 +- .../dispatcher/skills_install_root.go | 208 +++++++++ .../dispatcher/skills_install_root_test.go | 395 ++++++++++++++++++ .../internal/dispatcher/skills_options.go | 22 +- .../internal/dispatcher/skills_targets.go | 36 ++ .../internal/dispatcher/skills_test.go | 18 +- .../dispatcher/skills_v3_migration.go | 20 +- docs/glossary.md | 10 + 11 files changed, 730 insertions(+), 81 deletions(-) create mode 100644 cli/dispatcher/internal/dispatcher/skills_install_root.go create mode 100644 cli/dispatcher/internal/dispatcher/skills_install_root_test.go diff --git a/README.md b/README.md index 46b5dc1e5c..411e454372 100644 --- a/README.md +++ b/README.md @@ -114,6 +114,9 @@ uloop skills install --claude # Codex and other tools that read .agents/skills uloop skills install --agents +# Install at the root of the Git repository that contains the Unity project +uloop skills install --claude --root git + # Install globally instead of into the project uloop skills install --claude --global diff --git a/README_ja.md b/README_ja.md index a3c611b75e..0149a0425c 100644 --- a/README_ja.md +++ b/README_ja.md @@ -114,6 +114,9 @@ uloop skills install --claude # Codex など .agents/skills を読むツール uloop skills install --agents +# Unity プロジェクトを含む Git リポジトリのルートにインストール +uloop skills install --claude --root git + # プロジェクトではなくグローバルにインストール uloop skills install --claude --global diff --git a/cli/dispatcher/internal/dispatcher/skills.go b/cli/dispatcher/internal/dispatcher/skills.go index ed10aea6c9..b45bdff53e 100644 --- a/cli/dispatcher/internal/dispatcher/skills.go +++ b/cli/dispatcher/internal/dispatcher/skills.go @@ -27,42 +27,6 @@ const ( lineFeedCodeUnit = 0x000a ) -var targetConfigs = map[string]skillTarget{ - "claude": {id: "claude", displayName: "Claude Code", projectDir: ".claude"}, - "codex": {id: "codex", displayName: "Codex CLI", projectDir: ".codex"}, - "agents": {id: "agents", displayName: "Common", projectDir: ".agents"}, - "windsurf": {id: "windsurf", displayName: "Windsurf", projectDir: ".agents"}, - "antigravity": {id: "antigravity", displayName: "Antigravity", projectDir: ".agent"}, -} - -// allSkillTargetIDs enumerates every target id in the display order used by -// help output and flag parsing. targetConfigs is an unordered map, so this -// slice is the single source of truth for iteration order and for the set of -// accepted -- flags; help lines and parseSkillsOptions must both derive -// from it to avoid drift. -var allSkillTargetIDs = []string{"claude", "codex", "agents", "windsurf", "antigravity"} - -// nonDefaultSkillTargetIDs lists targets that are excluded from the default -// target set and are only installed when explicitly requested via their -// -- flag. -var nonDefaultSkillTargetIDs = map[string]bool{"windsurf": true} - -// defaultSkillTargetIDs is derived from allSkillTargetIDs so its order matches -// help output and so any new target added to allSkillTargetIDs is included by -// default unless it is explicitly listed in nonDefaultSkillTargetIDs. -var defaultSkillTargetIDs = buildDefaultSkillTargetIDs() - -func buildDefaultSkillTargetIDs() []string { - ids := make([]string, 0, len(allSkillTargetIDs)) - for _, id := range allSkillTargetIDs { - if nonDefaultSkillTargetIDs[id] { - continue - } - ids = append(ids, id) - } - return ids -} - // deprecatedSkillNames lists previously installed skill directory names that // skills install/uninstall runs clean up. Keep cleanup names here and in the // Unity-side SkillTargetInstaller; agent-facing V2-to-V3 CLI migration @@ -89,7 +53,11 @@ type skillCommandOptions struct { global bool flat bool outputDir string - targets []skillTarget + // root is the install root requested with --root; empty until resolved + // against the saved setting. installRoot is its resolved directory. + root skillsInstallRoot + installRoot string + targets []skillTarget } type skillDefinition struct { @@ -138,6 +106,12 @@ func tryHandleSkillsRequest(args []string, startPath string, globalProjectPath s return true, 1 } + options, err = resolveSkillsInstallLocation(projectRoot, options) + if err != nil { + clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ProjectRoot: projectRoot, Command: clicore.SkillsCommandName}) + return true, 1 + } + if isV3MigrationSkillSubcommand(subcommand) { return true, runV3MigrationSkillsSubcommand(subcommand, projectRoot, options, stdout, stderr) } @@ -219,10 +193,7 @@ func runSkillsList(projectRoot string, skills []skillDefinition, options skillCo targets = defaultSkillTargets() } - location := "Project" - if options.global { - location = "Global" - } + location := skillLocationLabel(options) disabledTools := loadDisabledToolsForSkills(projectRoot, options.global) @@ -230,7 +201,7 @@ func runSkillsList(projectRoot string, skills []skillDefinition, options skillCo clicore.WriteLine(stdout, "uloop Skills Status:") clicore.WriteLine(stdout, "") for _, target := range targets { - baseDir, err := getSkillsBaseDir(projectRoot, target, options.global) + baseDir, err := getSkillsBaseDir(skillsInstallRootFor(projectRoot, options), target, options.global) if err != nil { clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ProjectRoot: projectRoot, Command: clicore.SkillsCommandName}) return 1 @@ -258,7 +229,7 @@ func runSkillsList(projectRoot string, skills []skillDefinition, options skillCo func runSkillsInstall(projectRoot string, skills []skillDefinition, options skillCommandOptions, stdout io.Writer, stderr io.Writer) int { clicore.WriteLine(stdout, "") - clicore.WriteFormat(stdout, "Installing uloop skills (%s)...\n", skillLocationName(options.global)) + clicore.WriteFormat(stdout, "Installing uloop skills (%s)...\n", skillLocationName(options)) clicore.WriteLine(stdout, "") // Targets installed earlier but omitted from this invocation would keep stale // skill copies that contradict the CLI, so refresh every detected install. @@ -271,7 +242,7 @@ func runSkillsInstall(projectRoot string, skills []skillDefinition, options skil clicore.WriteFormat(stdout, "Auto-refreshing %s: an existing uloop skill install was detected there.\n\n", autoTarget.displayName) } for _, target := range append(append([]skillTarget{}, options.targets...), autoRefreshTargets...) { - result, err := installSkillsForTarget(projectRoot, target, skills, options.global, groupManagedSkillsForOptions(options)) + result, err := installSkillsForTarget(projectRoot, skillsInstallRootFor(projectRoot, options), target, skills, options.global, groupManagedSkillsForOptions(options)) if err != nil { clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ProjectRoot: projectRoot, Command: clicore.SkillsCommandName}) return 1 @@ -283,7 +254,7 @@ func runSkillsInstall(projectRoot string, skills []skillDefinition, options skil if result.deprecatedRemoved > 0 { clicore.WriteFormat(stdout, " Deprecated removed: %d\n", result.deprecatedRemoved) } - baseDir, err := getSkillsBaseDir(projectRoot, target, options.global) + baseDir, err := getSkillsBaseDir(skillsInstallRootFor(projectRoot, options), target, options.global) if err != nil { clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ProjectRoot: projectRoot, Command: clicore.SkillsCommandName}) return 1 @@ -295,11 +266,11 @@ func runSkillsInstall(projectRoot string, skills []skillDefinition, options skil func runSkillsUninstall(projectRoot string, skills []skillDefinition, options skillCommandOptions, stdout io.Writer, stderr io.Writer) int { clicore.WriteLine(stdout, "") - clicore.WriteFormat(stdout, "Uninstalling uloop skills (%s)...\n", skillLocationName(options.global)) + clicore.WriteFormat(stdout, "Uninstalling uloop skills (%s)...\n", skillLocationName(options)) clicore.WriteLine(stdout, "") for _, target := range options.targets { grouped := groupManagedSkillsForOptions(options) - removed, notFound, err := uninstallSkillsForTarget(projectRoot, target, skills, options.global, grouped) + removed, notFound, err := uninstallSkillsForTarget(skillsInstallRootFor(projectRoot, options), target, skills, options.global, grouped) if err != nil { clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ProjectRoot: projectRoot, Command: clicore.SkillsCommandName}) return 1 @@ -307,7 +278,7 @@ func runSkillsUninstall(projectRoot string, skills []skillDefinition, options sk clicore.WriteFormat(stdout, "%s:\n", target.displayName) clicore.WriteFormat(stdout, " Removed: %d\n", removed) clicore.WriteFormat(stdout, " Not found: %d\n", notFound) - baseDir, err := getSkillsBaseDir(projectRoot, target, options.global) + baseDir, err := getSkillsBaseDir(skillsInstallRootFor(projectRoot, options), target, options.global) if err != nil { clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ProjectRoot: projectRoot, Command: clicore.SkillsCommandName}) return 1 @@ -331,7 +302,7 @@ func detectInstalledSkillTargets(projectRoot string, skills []skillDefinition, o if requestedDirs[target.projectDir] { continue } - baseDir, err := getSkillsBaseDir(projectRoot, target, options.global) + baseDir, err := getSkillsBaseDir(skillsInstallRootFor(projectRoot, options), target, options.global) if err != nil { return nil, err } @@ -368,9 +339,16 @@ type skillInstallResult struct { deprecatedRemoved int } -func installSkillsForTarget(projectRoot string, target skillTarget, skills []skillDefinition, global bool, grouped bool) (skillInstallResult, error) { +func installSkillsForTarget( + projectRoot string, + installRoot string, + target skillTarget, + skills []skillDefinition, + global bool, + grouped bool, +) (skillInstallResult, error) { result := skillInstallResult{} - baseDir, err := getSkillsBaseDir(projectRoot, target, global) + baseDir, err := getSkillsBaseDir(installRoot, target, global) if err != nil { return skillInstallResult{}, err } @@ -457,10 +435,10 @@ func installSkillForTarget( return nil } -func uninstallSkillsForTarget(projectRoot string, target skillTarget, skills []skillDefinition, global bool, grouped bool) (int, int, error) { +func uninstallSkillsForTarget(installRoot string, target skillTarget, skills []skillDefinition, global bool, grouped bool) (int, int, error) { removed := 0 notFound := 0 - baseDir, err := getSkillsBaseDir(projectRoot, target, global) + baseDir, err := getSkillsBaseDir(installRoot, target, global) if err != nil { return removed, notFound, err } diff --git a/cli/dispatcher/internal/dispatcher/skills_display.go b/cli/dispatcher/internal/dispatcher/skills_display.go index 3ee5d808de..5419858709 100644 --- a/cli/dispatcher/internal/dispatcher/skills_display.go +++ b/cli/dispatcher/internal/dispatcher/skills_display.go @@ -38,13 +38,6 @@ func isSafeSkillName(name string) bool { !strings.Contains(name, "/") && !strings.Contains(name, `\`) } -func skillLocationName(global bool) string { - if global { - return "global" - } - return "project" -} - func statusIcon(status string) string { switch status { case "installed": @@ -93,6 +86,7 @@ func printSkillsSubcommandHelp(command string, stdout io.Writer) { clicore.WriteLine(stdout, "Options:") clicore.WriteLine(stdout, " -g, --global") clicore.WriteLine(stdout, " --flat") + clicore.WriteLine(stdout, " "+skillsRootFlagName+" ") if skillsSubcommandSupportsOutputDir(command) { clicore.WriteLine(stdout, " "+skillsOutputDirFlagName+" ") } @@ -103,6 +97,10 @@ func printSkillsSubcommandHelp(command string, stdout io.Writer) { clicore.WriteLine(stdout, "even when their flag is omitted, so previously installed copies never go stale.") clicore.WriteLine(stdout, "") } + clicore.WriteLine(stdout, "Project-local skills go under the Unity project root by default. --root git") + clicore.WriteLine(stdout, "uses the root of the Git repository that contains the Unity project instead.") + clicore.WriteLine(stdout, "Without --root, the install root chosen in the Unity Editor settings is used.") + clicore.WriteLine(stdout, "") if skillsSubcommandSupportsOutputDir(command) { clicore.WriteLine(stdout, "With --output-dir, skills are managed flat as / with no") clicore.WriteLine(stdout, "target subdirectories. Top-level files uloop does not manage are left untouched,") diff --git a/cli/dispatcher/internal/dispatcher/skills_install_root.go b/cli/dispatcher/internal/dispatcher/skills_install_root.go new file mode 100644 index 0000000000..8a4d294496 --- /dev/null +++ b/cli/dispatcher/internal/dispatcher/skills_install_root.go @@ -0,0 +1,208 @@ +package dispatcher + +// The install root is the directory whose agent directories (.claude/skills, +// .agents/skills, ...) receive project-local skills. Teams split on where +// their agents start: some open the Unity project itself, others open the +// Git repository that contains it, so both are supported. The choice made in +// the Editor is saved in the project's .uloop/settings.skills.json, and a +// flagless `uloop skills` command follows it so the CLI and the Editor never +// install into different places; --root overrides it for one invocation. +// Only the destination moves: skill sources and tool settings still come from +// the Unity project root. + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "strings" + + clierrors "github.com/hatayama/unity-cli-loop/common/errors" + + "github.com/hatayama/unity-cli-loop/common/clicore" +) + +type skillsInstallRoot string + +const ( + skillsInstallRootUnity skillsInstallRoot = "unity" + skillsInstallRootGit skillsInstallRoot = "git" + + // skillsRootFlagName is the single definition of the --root token, shared + // by parsing, validation, errors, and help output. + skillsRootFlagName = "--root" + + skillSettingsDir = ".uloop" + skillSettingsFile = "settings.skills.json" + gitMarkerName = ".git" +) + +type skillSettingsData struct { + InstallRoot string `json:"installRoot"` +} + +func parseSkillsInstallRoot(value string) (skillsInstallRoot, bool) { + switch skillsInstallRoot(value) { + case skillsInstallRootUnity, skillsInstallRootGit: + return skillsInstallRoot(value), true + default: + return "", false + } +} + +// parseRootOption consumes the --root option at args[index] in either value +// form and returns the index of the last consumed token. +func parseRootOption(options *skillCommandOptions, args []string, index int) (int, error) { + if options.root != "" { + return index, &clierrors.ArgumentError{ + Message: "Duplicate skills option: " + skillsRootFlagName, + Option: skillsRootFlagName, + Command: clicore.SkillsCommandName, + NextActions: []string{"Pass the install root only once."}, + } + } + value, joined := strings.CutPrefix(args[index], skillsRootFlagName+"=") + nextIndex := index + if !joined { + // A flag-like next token must not be swallowed as the value, or + // `--root --claude` would silently drop the target. + if index+1 >= len(args) || strings.HasPrefix(args[index+1], "-") { + return index, invalidSkillsRootValueError("") + } + value = args[index+1] + nextIndex = index + 1 + } + root, ok := parseSkillsInstallRoot(value) + if !ok { + return index, invalidSkillsRootValueError(value) + } + options.root = root + return nextIndex, nil +} + +func invalidSkillsRootValueError(received string) error { + return &clierrors.ArgumentError{ + Message: "The " + skillsRootFlagName + " option requires unity or git.", + Option: skillsRootFlagName, + Received: received, + Command: clicore.SkillsCommandName, + NextActions: []string{"Pass `--root unity` for the Unity project root or `--root git` for the Git repository root."}, + } +} + +// loadSavedSkillsInstallRoot returns the install root saved by the Editor, or +// unity when none is saved. An unreadable or unknown value is an error rather +// than a silent fallback: installing at the Unity root when the team chose the +// Git root would leave agents reading stale skills with no visible failure. +func loadSavedSkillsInstallRoot(projectRoot string) (skillsInstallRoot, error) { + settingsPath := filepath.Join(projectRoot, skillSettingsDir, skillSettingsFile) + content, err := os.ReadFile(settingsPath) + if os.IsNotExist(err) { + return skillsInstallRootUnity, nil + } + if err != nil { + return "", err + } + if strings.TrimSpace(string(content)) == "" { + return skillsInstallRootUnity, nil + } + settings := skillSettingsData{} + if err := json.Unmarshal(content, &settings); err != nil { + return "", fmt.Errorf("failed to parse %s: %w", settingsPath, err) + } + if settings.InstallRoot == "" { + return skillsInstallRootUnity, nil + } + root, ok := parseSkillsInstallRoot(settings.InstallRoot) + if !ok { + return "", fmt.Errorf("%s has an unknown installRoot %q; expected unity or git", settingsPath, settings.InstallRoot) + } + return root, nil +} + +// findGitRepositoryRoot walks up from the Unity project root, inclusive, to the +// nearest directory holding .git. A .git file counts too, because worktree and +// submodule checkouts mark their root with one. +func findGitRepositoryRoot(projectRoot string) (string, error) { + currentPath, err := filepath.Abs(projectRoot) + if err != nil { + return "", err + } + for { + if _, err := os.Lstat(filepath.Join(currentPath, gitMarkerName)); err == nil { + return currentPath, nil + } else if !os.IsNotExist(err) { + return "", err + } + parentPath := filepath.Dir(currentPath) + if parentPath == currentPath { + return "", gitRepositoryNotFoundError(projectRoot) + } + currentPath = parentPath + } +} + +func gitRepositoryNotFoundError(projectRoot string) error { + return &clierrors.ArgumentError{ + Message: "No Git repository contains the Unity project " + projectRoot + ", so skills cannot be installed at the Git root.", + Option: skillsRootFlagName, + Command: clicore.SkillsCommandName, + NextActions: []string{"Pass `--root unity` to install at the Unity project root, or run the command inside a Git repository."}, + } +} + +// resolveSkillsInstallLocation fills in the effective install root and its +// directory. Global installs (under the home directory) and --output-dir syncs +// already name their destination, so they neither read the saved setting nor +// resolve a project directory. +func resolveSkillsInstallLocation(projectRoot string, options skillCommandOptions) (skillCommandOptions, error) { + if options.global || options.outputDir != "" { + return options, nil + } + root := options.root + if root == "" { + savedRoot, err := loadSavedSkillsInstallRoot(projectRoot) + if err != nil { + return skillCommandOptions{}, err + } + root = savedRoot + } + resolved := options + resolved.root = root + if root == skillsInstallRootUnity { + resolved.installRoot = projectRoot + return resolved, nil + } + gitRoot, err := findGitRepositoryRoot(projectRoot) + if err != nil { + return skillCommandOptions{}, err + } + resolved.installRoot = gitRoot + return resolved, nil +} + +// skillsInstallRootFor returns the directory that receives project-local agent +// directories. An unresolved option set means the Unity project root, the +// default install root. +func skillsInstallRootFor(projectRoot string, options skillCommandOptions) string { + if options.installRoot == "" { + return projectRoot + } + return options.installRoot +} + +// skillLocationName names the install location in progress messages. +func skillLocationName(options skillCommandOptions) string { + return strings.ToLower(skillLocationLabel(options)) +} + +// skillLocationLabel names the install location in list headings. +func skillLocationLabel(options skillCommandOptions) string { + if options.global { + return "Global" + } + if options.root == skillsInstallRootGit { + return "Git root" + } + return "Project" +} diff --git a/cli/dispatcher/internal/dispatcher/skills_install_root_test.go b/cli/dispatcher/internal/dispatcher/skills_install_root_test.go new file mode 100644 index 0000000000..05e35cd264 --- /dev/null +++ b/cli/dispatcher/internal/dispatcher/skills_install_root_test.go @@ -0,0 +1,395 @@ +package dispatcher + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" +) + +// Tests that both value forms of --root are accepted. +func TestParseSkillsOptionsAcceptsRootValues(t *testing.T) { + separate, err := parseSkillsOptions([]string{"--claude", "--root", "git"}) + if err != nil { + t.Fatalf("--root git should parse: %v", err) + } + if separate.root != skillsInstallRootGit { + t.Fatalf("root = %q, want git", separate.root) + } + + joined, err := parseSkillsOptions([]string{"--root=unity", "--claude"}) + if err != nil { + t.Fatalf("--root=unity should parse: %v", err) + } + if joined.root != skillsInstallRootUnity { + t.Fatalf("root = %q, want unity", joined.root) + } +} + +// Tests that --root rejects values other than unity and git, including other casings. +func TestParseSkillsOptionsRejectsUnknownRootValue(t *testing.T) { + for _, args := range [][]string{ + {"--root", "repo"}, + {"--root=Git"}, + {"--root="}, + } { + if _, err := parseSkillsOptions(args); err == nil { + t.Fatalf("%v should be rejected", args) + } + } +} + +// Tests that --root without a value, or followed by another flag, is rejected instead of swallowing the flag. +func TestParseSkillsOptionsRejectsMissingRootValue(t *testing.T) { + for _, args := range [][]string{ + {"--root"}, + {"--root", "--claude"}, + } { + if _, err := parseSkillsOptions(args); err == nil { + t.Fatalf("%v should be rejected", args) + } + } +} + +// Tests that passing --root twice is rejected even when both values agree. +func TestParseSkillsOptionsRejectsDuplicateRoot(t *testing.T) { + if _, err := parseSkillsOptions([]string{"--root", "git", "--root", "git"}); err == nil { + t.Fatal("duplicate --root should be rejected") + } +} + +// Tests that --root conflicts with --global and with --output-dir, which already name a destination. +func TestParseSkillsOptionsRejectsRootWithOtherDestinations(t *testing.T) { + if _, err := parseSkillsOptions([]string{"--root", "git", "--global", "--claude"}); err == nil { + t.Fatal("--root with --global should be rejected") + } + if _, err := parseSkillsOptions([]string{"--root", "git", "--output-dir", "store"}); err == nil { + t.Fatal("--root with --output-dir should be rejected") + } +} + +// Tests that the Git root is the nearest ancestor holding a .git directory, starting at the Unity project root. +func TestFindGitRepositoryRootFindsAncestorWithGitDirectory(t *testing.T) { + repositoryRoot := t.TempDir() + mustMkdir(t, filepath.Join(repositoryRoot, ".git")) + projectRoot := filepath.Join(repositoryRoot, "client", "unity") + mustMkdir(t, projectRoot) + + found, err := findGitRepositoryRoot(projectRoot) + if err != nil { + t.Fatalf("git root should be found: %v", err) + } + if found != repositoryRoot { + t.Fatalf("git root = %q, want %q", found, repositoryRoot) + } +} + +// Tests that a .git file (a worktree or submodule checkout) also marks the Git root. +func TestFindGitRepositoryRootAcceptsGitFile(t *testing.T) { + repositoryRoot := t.TempDir() + if err := os.WriteFile(filepath.Join(repositoryRoot, ".git"), []byte("gitdir: elsewhere\n"), 0o644); err != nil { + t.Fatalf("failed to write .git file: %v", err) + } + projectRoot := filepath.Join(repositoryRoot, "unity") + mustMkdir(t, projectRoot) + + found, err := findGitRepositoryRoot(projectRoot) + if err != nil { + t.Fatalf("git root should be found: %v", err) + } + if found != repositoryRoot { + t.Fatalf("git root = %q, want %q", found, repositoryRoot) + } +} + +// Tests that a Unity project that is itself the repository root resolves to itself. +func TestFindGitRepositoryRootReturnsProjectRootWhenItHoldsGit(t *testing.T) { + projectRoot := t.TempDir() + mustMkdir(t, filepath.Join(projectRoot, ".git")) + + found, err := findGitRepositoryRoot(projectRoot) + if err != nil { + t.Fatalf("git root should be found: %v", err) + } + if found != projectRoot { + t.Fatalf("git root = %q, want %q", found, projectRoot) + } +} + +// Tests that a project outside any Git repository reports an error instead of falling back to the Unity root. +func TestFindGitRepositoryRootFailsOutsideRepository(t *testing.T) { + projectRoot := filepath.Join(t.TempDir(), "unity") + mustMkdir(t, projectRoot) + + if _, err := findGitRepositoryRoot(projectRoot); err == nil { + t.Fatal("a project outside any repository should be rejected") + } +} + +// Tests that a project without a saved skills setting installs at the Unity project root. +func TestLoadSavedSkillsInstallRootDefaultsToUnity(t *testing.T) { + root, err := loadSavedSkillsInstallRoot(t.TempDir()) + if err != nil { + t.Fatalf("missing settings should not fail: %v", err) + } + if root != skillsInstallRootUnity { + t.Fatalf("root = %q, want unity", root) + } +} + +// Tests that the install root saved by the Editor is read back. +func TestLoadSavedSkillsInstallRootReadsSavedValue(t *testing.T) { + projectRoot := t.TempDir() + writeSkillSettings(t, projectRoot, `{"installRoot":"git"}`) + + root, err := loadSavedSkillsInstallRoot(projectRoot) + if err != nil { + t.Fatalf("saved settings should load: %v", err) + } + if root != skillsInstallRootGit { + t.Fatalf("root = %q, want git", root) + } +} + +// Tests that an unreadable or unknown saved install root fails instead of silently installing at the Unity root. +func TestLoadSavedSkillsInstallRootRejectsInvalidSettings(t *testing.T) { + for _, content := range []string{`{"installRoot":"repo"}`, `{not json`} { + projectRoot := t.TempDir() + writeSkillSettings(t, projectRoot, content) + + if _, err := loadSavedSkillsInstallRoot(projectRoot); err == nil { + t.Fatalf("settings %q should be rejected", content) + } + } +} + +// Tests that install with --root git writes into the Git root while tool settings still come from the Unity project. +func TestTryHandleSkillsRequestInstallsAtGitRoot(t *testing.T) { + repositoryRoot, projectRoot := setUpNestedUnityProjectWithSkills(t) + + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + handled, code := tryHandleSkillsRequest( + []string{"skills", "install", "--claude", "--root", "git"}, + "", + projectRoot, + stdout, + stderr) + + if !handled || code != 0 { + t.Fatalf("install should succeed: handled=%v code=%d stderr=%s", handled, code, stderr.String()) + } + assertSkillInstalledAt(t, repositoryRoot, "uloop-enabled-skill") + assertSkillAbsentAt(t, repositoryRoot, "uloop-disabled-skill") + assertSkillAbsentAt(t, projectRoot, "uloop-enabled-skill") + if !strings.Contains(stdout.String(), filepath.Join(repositoryRoot, ".claude", "skills")) { + t.Fatalf("output should report the Git root location:\n%s", stdout.String()) + } +} + +// Tests that install without --root follows the install root saved by the Editor. +func TestTryHandleSkillsRequestFollowsSavedInstallRoot(t *testing.T) { + repositoryRoot, projectRoot := setUpNestedUnityProjectWithSkills(t) + writeSkillSettings(t, projectRoot, `{"installRoot":"git"}`) + + stderr := &bytes.Buffer{} + _, code := tryHandleSkillsRequest( + []string{"skills", "install", "--claude"}, + "", + projectRoot, + &bytes.Buffer{}, + stderr) + + if code != 0 { + t.Fatalf("install should succeed: code=%d stderr=%s", code, stderr.String()) + } + assertSkillInstalledAt(t, repositoryRoot, "uloop-enabled-skill") + assertSkillAbsentAt(t, projectRoot, "uloop-enabled-skill") +} + +// Tests that --root unity overrides a saved Git install root for one invocation. +func TestTryHandleSkillsRequestRootFlagOverridesSavedInstallRoot(t *testing.T) { + repositoryRoot, projectRoot := setUpNestedUnityProjectWithSkills(t) + writeSkillSettings(t, projectRoot, `{"installRoot":"git"}`) + + stderr := &bytes.Buffer{} + _, code := tryHandleSkillsRequest( + []string{"skills", "install", "--claude", "--root", "unity"}, + "", + projectRoot, + &bytes.Buffer{}, + stderr) + + if code != 0 { + t.Fatalf("install should succeed: code=%d stderr=%s", code, stderr.String()) + } + assertSkillInstalledAt(t, projectRoot, "uloop-enabled-skill") + assertSkillAbsentAt(t, repositoryRoot, "uloop-enabled-skill") +} + +// Tests that --root git fails without writing anything when the project is not inside a Git repository. +func TestTryHandleSkillsRequestRejectsGitRootOutsideRepository(t *testing.T) { + projectRoot := filepath.Join(t.TempDir(), "unity") + writeUnityProjectWithSkills(t, projectRoot) + + stderr := &bytes.Buffer{} + _, code := tryHandleSkillsRequest( + []string{"skills", "install", "--claude", "--root", "git"}, + "", + projectRoot, + &bytes.Buffer{}, + stderr) + + if code != 1 { + t.Fatalf("install should fail: code=%d", code) + } + if !strings.Contains(stderr.String(), "Git repository") { + t.Fatalf("error should explain the missing Git repository:\n%s", stderr.String()) + } + assertSkillAbsentAt(t, projectRoot, "uloop-enabled-skill") +} + +// Tests that list with --root git reports skills installed at the Git root. +func TestTryHandleSkillsRequestListsGitRootInstall(t *testing.T) { + repositoryRoot, projectRoot := setUpNestedUnityProjectWithSkills(t) + _, installCode := tryHandleSkillsRequest( + []string{"skills", "install", "--claude", "--root", "git"}, + "", + projectRoot, + &bytes.Buffer{}, + &bytes.Buffer{}) + if installCode != 0 { + t.Fatalf("install should succeed: code=%d", installCode) + } + + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + _, code := tryHandleSkillsRequest( + []string{"skills", "list", "--claude", "--root", "git"}, + "", + projectRoot, + stdout, + stderr) + + if code != 0 { + t.Fatalf("list should succeed: code=%d stderr=%s", code, stderr.String()) + } + output := stdout.String() + if !strings.Contains(output, "Claude Code (Git root):") { + t.Fatalf("list should label the Git root location:\n%s", output) + } + if !strings.Contains(output, filepath.Join(repositoryRoot, ".claude", "skills")) { + t.Fatalf("list should report the Git root location:\n%s", output) + } + if !strings.Contains(output, "uloop-enabled-skill (installed)") { + t.Fatalf("list should report the Git root install:\n%s", output) + } +} + +// Tests that the V3 migration skill honors --root git like the regular skills. +func TestTryHandleSkillsRequestInstallsV3MigrationAtGitRoot(t *testing.T) { + repositoryRoot := t.TempDir() + mustMkdir(t, filepath.Join(repositoryRoot, ".git")) + projectRoot := filepath.Join(repositoryRoot, "unity") + writeUnityProjectWithSkills(t, projectRoot) + writeV3MigrationSkillFixture(t, projectRoot, "---\nname: v3-cli-invocation-migration\n---\n") + + stderr := &bytes.Buffer{} + _, code := tryHandleSkillsRequest( + []string{"skills", "install-v3-migration", "--claude", "--root", "git"}, + "", + projectRoot, + &bytes.Buffer{}, + stderr) + + if code != 0 { + t.Fatalf("install-v3-migration should succeed: code=%d stderr=%s", code, stderr.String()) + } + assertSkillInstalledAt(t, repositoryRoot, v3MigrationSkillName) + assertSkillAbsentAt(t, projectRoot, v3MigrationSkillName) +} + +// setUpNestedUnityProjectWithSkills builds a Git repository whose Unity project +// sits one level below the repository root, with one enabled and one disabled skill. +func setUpNestedUnityProjectWithSkills(t *testing.T) (string, string) { + t.Helper() + repositoryRoot := t.TempDir() + mustMkdir(t, filepath.Join(repositoryRoot, ".git")) + projectRoot := filepath.Join(repositoryRoot, "unity") + writeUnityProjectWithSkills(t, projectRoot) + return repositoryRoot, projectRoot +} + +func writeUnityProjectWithSkills(t *testing.T, projectRoot string) { + t.Helper() + mustMkdir(t, filepath.Join(projectRoot, "Assets")) + mustMkdir(t, filepath.Join(projectRoot, "ProjectSettings")) + writeTestSkill(t, projectRoot, "Assets/Editor/Enabled/Skill", "---\nname: uloop-enabled-skill\n---\n") + writeTestSkill(t, projectRoot, "Assets/Editor/Disabled/Skill", "---\nname: uloop-disabled-skill\n---\n") + writeToolSettings(t, projectRoot, `{"disabledTools":["disabled-skill"]}`) +} + +func writeSkillSettings(t *testing.T, projectRoot string, content string) { + t.Helper() + settingsDir := filepath.Join(projectRoot, ".uloop") + mustMkdir(t, settingsDir) + if err := os.WriteFile(filepath.Join(settingsDir, "settings.skills.json"), []byte(content), 0o644); err != nil { + t.Fatalf("failed to write skill settings: %v", err) + } +} + +func assertSkillInstalledAt(t *testing.T, installRoot string, skillName string) { + t.Helper() + skillFile := filepath.Join(installRoot, ".claude", "skills", skillName, "SKILL.md") + if _, err := os.Stat(skillFile); err != nil { + t.Fatalf("%s should be installed at %s: %v", skillName, installRoot, err) + } +} + +func assertSkillAbsentAt(t *testing.T, installRoot string, skillName string) { + t.Helper() + skillDir := filepath.Join(installRoot, ".claude", "skills", skillName) + if _, err := os.Stat(skillDir); !os.IsNotExist(err) { + t.Fatalf("%s should not exist at %s: %v", skillName, installRoot, err) + } +} + +func mustMkdir(t *testing.T, directory string) { + t.Helper() + if err := os.MkdirAll(directory, 0o755); err != nil { + t.Fatalf("failed to create %s: %v", directory, err) + } +} + +// Tests that --output-dir ignores a saved Git install root, since it names its own destination. +func TestTryHandleSkillsRequestOutputDirIgnoresSavedInstallRoot(t *testing.T) { + projectRoot := filepath.Join(t.TempDir(), "unity") + writeUnityProjectWithSkills(t, projectRoot) + writeSkillSettings(t, projectRoot, `{"installRoot":"git"}`) + storeDir := filepath.Join(t.TempDir(), "store") + + stderr := &bytes.Buffer{} + _, code := tryHandleSkillsRequest( + []string{"skills", "install", "--output-dir", storeDir}, + "", + projectRoot, + &bytes.Buffer{}, + stderr) + + if code != 0 { + t.Fatalf("install should succeed: code=%d stderr=%s", code, stderr.String()) + } + if _, err := os.Stat(filepath.Join(storeDir, "uloop-enabled-skill", "SKILL.md")); err != nil { + t.Fatalf("skill should be synced into the store: %v", err) + } +} + +// Tests that subcommand help documents the --root option. +func TestPrintSkillsSubcommandHelpListsRootOption(t *testing.T) { + stdout := &bytes.Buffer{} + printSkillsSubcommandHelp("install", stdout) + if !strings.Contains(stdout.String(), skillsRootFlagName+" ") { + t.Fatalf("help should list --root:\n%s", stdout.String()) + } +} diff --git a/cli/dispatcher/internal/dispatcher/skills_options.go b/cli/dispatcher/internal/dispatcher/skills_options.go index 3938e4fffa..3b42777999 100644 --- a/cli/dispatcher/internal/dispatcher/skills_options.go +++ b/cli/dispatcher/internal/dispatcher/skills_options.go @@ -31,6 +31,12 @@ func parseSkillsOptions(args []string) (skillCommandOptions, error) { return skillCommandOptions{}, err } index = nextIndex + case arg == skillsRootFlagName || strings.HasPrefix(arg, skillsRootFlagName+"="): + nextIndex, err := parseRootOption(&options, args, index) + if err != nil { + return skillCommandOptions{}, err + } + index = nextIndex default: if err := appendSkillTarget(&options, seenTargets, arg); err != nil { return skillCommandOptions{}, err @@ -112,12 +118,24 @@ func missingSkillsOutputDirValueError() error { // validateSkillsOptions rejects flag combinations whose destinations conflict: // --output-dir already names one exact destination and always installs flat, so -// combining it with the location flags (--global or a target flag) or the layout -// flag (--flat) would make the request ambiguous. +// combining it with the location flags (--global, --root, or a target flag) or +// the layout flag (--flat) would make the request ambiguous. --root picks a +// project-local install root, which a --global install does not have. func validateSkillsOptions(options skillCommandOptions) error { + if options.root != "" && options.global { + return &clierrors.ArgumentError{ + Message: "The " + skillsRootFlagName + " option cannot be combined with --global.", + Option: skillsRootFlagName, + Command: clicore.SkillsCommandName, + NextActions: []string{"Use --global alone, or drop it to install into the chosen project-local root."}, + } + } if options.outputDir == "" { return nil } + if options.root != "" { + return skillsOutputDirConflictError(skillsRootFlagName) + } if len(options.targets) > 0 { return skillsOutputDirConflictError("--" + options.targets[0].id) } diff --git a/cli/dispatcher/internal/dispatcher/skills_targets.go b/cli/dispatcher/internal/dispatcher/skills_targets.go index 559a038886..4ce9102c53 100644 --- a/cli/dispatcher/internal/dispatcher/skills_targets.go +++ b/cli/dispatcher/internal/dispatcher/skills_targets.go @@ -10,6 +10,42 @@ import ( var userHomeDir = os.UserHomeDir +var targetConfigs = map[string]skillTarget{ + "claude": {id: "claude", displayName: "Claude Code", projectDir: ".claude"}, + "codex": {id: "codex", displayName: "Codex CLI", projectDir: ".codex"}, + "agents": {id: "agents", displayName: "Common", projectDir: ".agents"}, + "windsurf": {id: "windsurf", displayName: "Windsurf", projectDir: ".agents"}, + "antigravity": {id: "antigravity", displayName: "Antigravity", projectDir: ".agent"}, +} + +// allSkillTargetIDs enumerates every target id in the display order used by +// help output and flag parsing. targetConfigs is an unordered map, so this +// slice is the single source of truth for iteration order and for the set of +// accepted -- flags; help lines and parseSkillsOptions must both derive +// from it to avoid drift. +var allSkillTargetIDs = []string{"claude", "codex", "agents", "windsurf", "antigravity"} + +// nonDefaultSkillTargetIDs lists targets that are excluded from the default +// target set and are only installed when explicitly requested via their +// -- flag. +var nonDefaultSkillTargetIDs = map[string]bool{"windsurf": true} + +// defaultSkillTargetIDs is derived from allSkillTargetIDs so its order matches +// help output and so any new target added to allSkillTargetIDs is included by +// default unless it is explicitly listed in nonDefaultSkillTargetIDs. +var defaultSkillTargetIDs = buildDefaultSkillTargetIDs() + +func buildDefaultSkillTargetIDs() []string { + ids := make([]string, 0, len(allSkillTargetIDs)) + for _, id := range allSkillTargetIDs { + if nonDefaultSkillTargetIDs[id] { + continue + } + ids = append(ids, id) + } + return ids +} + func getSkillsBaseDir(projectRoot string, target skillTarget, global bool) (string, error) { if global { homeDir, err := userHomeDir() diff --git a/cli/dispatcher/internal/dispatcher/skills_test.go b/cli/dispatcher/internal/dispatcher/skills_test.go index 6a02284371..5d07d547f0 100644 --- a/cli/dispatcher/internal/dispatcher/skills_test.go +++ b/cli/dispatcher/internal/dispatcher/skills_test.go @@ -316,7 +316,7 @@ name: uloop-sample } target := targetConfigs["claude"] - result, err := installSkillsForTarget(projectRoot, target, []skillDefinition{skill}, false, true) + result, err := installSkillsForTarget(projectRoot, projectRoot, target, []skillDefinition{skill}, false, true) if err != nil { t.Fatalf("installSkillsForTarget failed: %v", err) } @@ -494,7 +494,7 @@ func TestInstallSkillsNormalizesCRLFLineEndings(t *testing.T) { sourceDirectory: sourceDir, } - result, err := installSkillsForTarget(projectRoot, targetConfigs["claude"], []skillDefinition{skill}, false, true) + result, err := installSkillsForTarget(projectRoot, projectRoot, targetConfigs["claude"], []skillDefinition{skill}, false, true) if err != nil { t.Fatalf("installSkillsForTarget failed: %v", err) } @@ -561,7 +561,7 @@ name: uloop-disabled-skill writeSkillFile(t, deprecatedFlatDir, "---\nname: uloop-capture-window\n---\n") writeSkillFile(t, deprecatedGroupedDir, "---\nname: uloop-capture-window\n---\n") - result, err := installSkillsForTarget(projectRoot, target, []skillDefinition{ + result, err := installSkillsForTarget(projectRoot, projectRoot, target, []skillDefinition{ { name: "uloop-enabled-skill", content: []byte("---\nname: uloop-enabled-skill\n---\n\n# enabled\n"), @@ -602,7 +602,7 @@ func TestInstallSkillsForTargetRemovesRetiredRecordInputSkill(t *testing.T) { writeSkillFile(t, flatDir, "---\nname: uloop-record-input\n---\n") writeSkillFile(t, groupedDir, "---\nname: uloop-record-input\n---\n") - result, err := installSkillsForTarget(projectRoot, target, []skillDefinition{}, false, true) + result, err := installSkillsForTarget(projectRoot, projectRoot, target, []skillDefinition{}, false, true) if err != nil { t.Fatalf("installSkillsForTarget failed: %v", err) } @@ -642,7 +642,7 @@ name: uloop-sample groupedDir := getPreferredSkillDir(baseDir, skill.name, true) writeSkillFile(t, flatDir, skillContent) - result, err := installSkillsForTarget(projectRoot, target, []skillDefinition{skill}, false, true) + result, err := installSkillsForTarget(projectRoot, projectRoot, target, []skillDefinition{skill}, false, true) if err != nil { t.Fatalf("installSkillsForTarget failed: %v", err) } @@ -682,7 +682,7 @@ name: uloop-sample flatDir := getPreferredSkillDir(baseDir, skill.name, false) writeSkillFile(t, groupedDir, "# grouped\n") - result, err := installSkillsForTarget(projectRoot, target, []skillDefinition{skill}, false, false) + result, err := installSkillsForTarget(projectRoot, projectRoot, target, []skillDefinition{skill}, false, false) if err != nil { t.Fatalf("installSkillsForTarget failed: %v", err) } @@ -1546,7 +1546,7 @@ func TestInstallSkillsForTargetGlobalIgnoresProjectToolSettings(t *testing.T) { homeDir := t.TempDir() stubSkillsUserHomeDir(t, homeDir, nil) - result, err := installSkillsForTarget(projectRoot, targetConfigs["claude"], []skillDefinition{sampleSkillWithSource(t)}, true, false) + result, err := installSkillsForTarget(projectRoot, projectRoot, targetConfigs["claude"], []skillDefinition{sampleSkillWithSource(t)}, true, false) if err != nil || result.installed != 1 { t.Fatalf("unexpected result: %+v err=%v", result, err) @@ -1604,7 +1604,7 @@ func TestInstallSkillsForTargetReportsFailures(t *testing.T) { skill := sampleSkillWithSource(t) testCase.setup(t, baseDir, &skill) - _, err := installSkillsForTarget(projectRoot, targetConfigs["claude"], []skillDefinition{skill}, false, testCase.grouped) + _, err := installSkillsForTarget(projectRoot, projectRoot, targetConfigs["claude"], []skillDefinition{skill}, false, testCase.grouped) if err == nil || !strings.Contains(err.Error(), testCase.wantMessage) { t.Fatalf("expected an error containing %q, got %v", testCase.wantMessage, err) } @@ -1616,7 +1616,7 @@ func TestInstallSkillsForTargetReportsHomeLookupFailure(t *testing.T) { // Verifies a global install fails when the home directory cannot be resolved. stubSkillsUserHomeDir(t, "", errors.New("home unavailable")) - _, err := installSkillsForTarget(t.TempDir(), targetConfigs["claude"], nil, true, false) + _, err := installSkillsForTarget(t.TempDir(), t.TempDir(), targetConfigs["claude"], nil, true, false) if err == nil || !strings.Contains(err.Error(), "home unavailable") { t.Fatalf("expected the home error, got %v", err) diff --git a/cli/dispatcher/internal/dispatcher/skills_v3_migration.go b/cli/dispatcher/internal/dispatcher/skills_v3_migration.go index c15cd9853b..6efebccc46 100644 --- a/cli/dispatcher/internal/dispatcher/skills_v3_migration.go +++ b/cli/dispatcher/internal/dispatcher/skills_v3_migration.go @@ -12,10 +12,10 @@ import ( func runV3MigrationSkillInstall(projectRoot string, skills []skillDefinition, options skillCommandOptions, stdout io.Writer, stderr io.Writer) int { clicore.WriteLine(stdout, "") - clicore.WriteFormat(stdout, "Installing V3 CLI invocation migration skill (%s)...\n", skillLocationName(options.global)) + clicore.WriteFormat(stdout, "Installing V3 CLI invocation migration skill (%s)...\n", skillLocationName(options)) clicore.WriteLine(stdout, "") for _, target := range options.targets { - result, err := installV3MigrationSkillForTarget(projectRoot, target, skills, options.global, groupManagedSkillsForOptions(options)) + result, err := installV3MigrationSkillForTarget(skillsInstallRootFor(projectRoot, options), target, skills, options.global, groupManagedSkillsForOptions(options)) if err != nil { clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ProjectRoot: projectRoot, Command: clicore.SkillsCommandName}) return 1 @@ -24,7 +24,7 @@ func runV3MigrationSkillInstall(projectRoot string, skills []skillDefinition, op clicore.WriteFormat(stdout, " Installed: %d\n", result.installed) clicore.WriteFormat(stdout, " Updated: %d\n", result.updated) clicore.WriteFormat(stdout, " Skipped: %d\n", result.skipped) - baseDir, err := getSkillsBaseDir(projectRoot, target, options.global) + baseDir, err := getSkillsBaseDir(skillsInstallRootFor(projectRoot, options), target, options.global) if err != nil { clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ProjectRoot: projectRoot, Command: clicore.SkillsCommandName}) return 1 @@ -36,11 +36,11 @@ func runV3MigrationSkillInstall(projectRoot string, skills []skillDefinition, op func runV3MigrationSkillUninstall(projectRoot string, options skillCommandOptions, stdout io.Writer, stderr io.Writer) int { clicore.WriteLine(stdout, "") - clicore.WriteFormat(stdout, "Uninstalling V3 CLI invocation migration skill (%s)...\n", skillLocationName(options.global)) + clicore.WriteFormat(stdout, "Uninstalling V3 CLI invocation migration skill (%s)...\n", skillLocationName(options)) clicore.WriteLine(stdout, "") for _, target := range options.targets { grouped := groupManagedSkillsForOptions(options) - removed, notFound, err := uninstallV3MigrationSkillForTarget(projectRoot, target, options.global, grouped) + removed, notFound, err := uninstallV3MigrationSkillForTarget(skillsInstallRootFor(projectRoot, options), target, options.global, grouped) if err != nil { clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ProjectRoot: projectRoot, Command: clicore.SkillsCommandName}) return 1 @@ -48,7 +48,7 @@ func runV3MigrationSkillUninstall(projectRoot string, options skillCommandOption clicore.WriteFormat(stdout, "%s:\n", target.displayName) clicore.WriteFormat(stdout, " Removed: %d\n", removed) clicore.WriteFormat(stdout, " Not found: %d\n", notFound) - baseDir, err := getSkillsBaseDir(projectRoot, target, options.global) + baseDir, err := getSkillsBaseDir(skillsInstallRootFor(projectRoot, options), target, options.global) if err != nil { clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ProjectRoot: projectRoot, Command: clicore.SkillsCommandName}) return 1 @@ -58,9 +58,9 @@ func runV3MigrationSkillUninstall(projectRoot string, options skillCommandOption return 0 } -func installV3MigrationSkillForTarget(projectRoot string, target skillTarget, skills []skillDefinition, global bool, grouped bool) (skillInstallResult, error) { +func installV3MigrationSkillForTarget(installRoot string, target skillTarget, skills []skillDefinition, global bool, grouped bool) (skillInstallResult, error) { result := skillInstallResult{} - baseDir, err := getSkillsBaseDir(projectRoot, target, global) + baseDir, err := getSkillsBaseDir(installRoot, target, global) if err != nil { return skillInstallResult{}, err } @@ -92,10 +92,10 @@ func installV3MigrationSkillForTarget(projectRoot string, target skillTarget, sk return result, nil } -func uninstallV3MigrationSkillForTarget(projectRoot string, target skillTarget, global bool, grouped bool) (int, int, error) { +func uninstallV3MigrationSkillForTarget(installRoot string, target skillTarget, global bool, grouped bool) (int, int, error) { removed := 0 notFound := 0 - baseDir, err := getSkillsBaseDir(projectRoot, target, global) + baseDir, err := getSkillsBaseDir(installRoot, target, global) if err != nil { return removed, notFound, err } diff --git a/docs/glossary.md b/docs/glossary.md index 81c8fe454f..54cac82ba5 100644 --- a/docs/glossary.md +++ b/docs/glossary.md @@ -135,6 +135,16 @@ the contract first when the rule changes. A destination agent environment into which skills are installed (for example Claude Code, Codex CLI). Each target defines where its skill copies live and which folder layout it uses. +### Skill install root + +The directory whose agent directories (`.claude/skills`, `.agents/skills`, ...) receive +project-local skills: the Unity project root (`unity`, the default) or the root of the Git +repository that contains it (`git`, the nearest ancestor of the Unity project root, inclusive, +holding a `.git` directory or file). The Editor saves the choice in +`.uloop/settings.skills.json`; `uloop skills` follows it and `--root` overrides it for one +invocation. Only the destination moves — skill sources and tool settings always come from the +Unity project root. Distinct from the *project root*, which always means the Unity project. + ### UseCase An Application-layer orchestration class that coordinates one tool execution flow across From e0c59630c7902fde9dbc32ea6b1ce141c4234c71 Mon Sep 17 00:00:00 2001 From: hatayama Date: Sun, 25 Oct 2026 22:20:27 +0900 Subject: [PATCH 2/9] Install Editor skills under the saved skill install root The Editor wrote agent skill folders under the Unity project root only, so teams whose agents start at the Git repository root could not use the Editor installer. - Save the choice in .uloop/settings.skills.json with the installRoot spelling the CLI reads, so both installers target the same root - ToolSkillSetupService resolves the install root on every call, so every caller (Settings, Setup Wizard, migration wizard, tool toggles) follows the saved choice without knowing about it; skill sources and disabled tools still come from the Unity project root - An unavailable Git root makes explicit installs throw, detection report nothing, and tool-toggle syncs skip with a warning - Synchronizer methods that only touch target folders now take the install root and are named AtInstallRoot --- ...etupWizardStartupFlowVersionChangeTests.cs | 6 + .../SetupWizardWorkflowControllersTests.cs | 6 + .../Editor/SkillInstallRootResolverTests.cs | 246 ++++++++++++++++++ .../SkillInstallRootResolverTests.cs.meta | 11 + ...lMigrationWizardWorkflowControllerTests.cs | 6 + ...tyToolMigrationWizardWorkflowStateTests.cs | 6 + .../ToolSkillSynchronizerRemovalTests.cs | 2 +- .../Editor/ToolSkillSynchronizerTests.cs | 66 ++++- ...liLoopSettingsSkillsPresenterStateTests.cs | 18 ++ .../Editor/V3MigrationSkillInstallerTests.cs | 2 +- .../Application/UseCases/SkillSetupUseCase.cs | 10 + Packages/src/Editor/Domain/ISkillSetupPort.cs | 6 + .../src/Editor/Domain/SkillInstallRoot.cs | 11 + .../Editor/Domain/SkillInstallRoot.cs.meta | 11 + .../Domain/SkillInstallRootSelection.cs | 41 +++ .../Domain/SkillInstallRootSelection.cs.meta | 11 + .../SkillSetup/SkillInstallRootResolver.cs | 149 +++++++++++ .../SkillInstallRootResolver.cs.meta | 11 + .../SkillSetup/SkillTargetDetector.cs | 8 +- .../SkillSetup/SkillTargetInstaller.cs | 12 +- .../SkillSetup/ToolSkillSetupService.cs | 109 +++++++- .../SkillSetup/ToolSkillSynchronizer.cs | 129 +++------ .../SkillSetup/V3MigrationSkillInstaller.cs | 52 ++-- 23 files changed, 782 insertions(+), 147 deletions(-) create mode 100644 Assets/Tests/Editor/SkillInstallRootResolverTests.cs create mode 100644 Assets/Tests/Editor/SkillInstallRootResolverTests.cs.meta create mode 100644 Packages/src/Editor/Domain/SkillInstallRoot.cs create mode 100644 Packages/src/Editor/Domain/SkillInstallRoot.cs.meta create mode 100644 Packages/src/Editor/Domain/SkillInstallRootSelection.cs create mode 100644 Packages/src/Editor/Domain/SkillInstallRootSelection.cs.meta create mode 100644 Packages/src/Editor/Infrastructure/SkillSetup/SkillInstallRootResolver.cs create mode 100644 Packages/src/Editor/Infrastructure/SkillSetup/SkillInstallRootResolver.cs.meta diff --git a/Assets/Tests/Editor/SetupWizardStartupFlowVersionChangeTests.cs b/Assets/Tests/Editor/SetupWizardStartupFlowVersionChangeTests.cs index 44282b600a..639a7293c8 100644 --- a/Assets/Tests/Editor/SetupWizardStartupFlowVersionChangeTests.cs +++ b/Assets/Tests/Editor/SetupWizardStartupFlowVersionChangeTests.cs @@ -589,6 +589,12 @@ private sealed class RecordingSkillSetupPort : ISkillSetupPort internal List DetectGroupFlags { get; } = new List(); internal bool ReturnOutdatedTarget { get; set; } + public SkillInstallRootSelection GetSkillInstallRootSelection(string projectRoot) => + new(SkillInstallRoot.UnityProjectRoot, projectRoot, null); + + public void SaveSkillInstallRoot(string projectRoot, SkillInstallRoot root) => + throw new NotSupportedException(); + public void RemoveSkillFiles(string toolName) => throw new NotSupportedException(); public bool IsSkillInstalled(string toolName) => throw new NotSupportedException(); diff --git a/Assets/Tests/Editor/SetupWizardWorkflowControllersTests.cs b/Assets/Tests/Editor/SetupWizardWorkflowControllersTests.cs index 598fb7fde7..10fc7f06fb 100644 --- a/Assets/Tests/Editor/SetupWizardWorkflowControllersTests.cs +++ b/Assets/Tests/Editor/SetupWizardWorkflowControllersTests.cs @@ -993,6 +993,12 @@ public List DetectSkillTargetsForLayoutAtProjectRoot( return FullTargets; } + public SkillInstallRootSelection GetSkillInstallRootSelection(string projectRoot) => + new(SkillInstallRoot.UnityProjectRoot, projectRoot, null); + + public void SaveSkillInstallRoot(string projectRoot, SkillInstallRoot root) => + throw new NotSupportedException(); + public void RemoveSkillFiles(string toolName) => throw new NotSupportedException(); public bool IsSkillInstalled(string toolName) => throw new NotSupportedException(); diff --git a/Assets/Tests/Editor/SkillInstallRootResolverTests.cs b/Assets/Tests/Editor/SkillInstallRootResolverTests.cs new file mode 100644 index 0000000000..6b98db42ce --- /dev/null +++ b/Assets/Tests/Editor/SkillInstallRootResolverTests.cs @@ -0,0 +1,246 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Threading; +using System.Threading.Tasks; + +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.Domain; +using io.github.hatayama.UnityCliLoop.Infrastructure; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Verifies how the skill install root is saved, read back, and resolved to a directory. + /// + [TestFixture] + public sealed class SkillInstallRootResolverTests + { + private string _temporaryRoot; + + [SetUp] + public void SetUpTemporaryRoot() + { + _temporaryRoot = Path.Combine(Path.GetTempPath(), "uloop-install-root-" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(_temporaryRoot); + } + + [TearDown] + public void TearDownTemporaryRoot() + { + if (Directory.Exists(_temporaryRoot)) + { + Directory.Delete(_temporaryRoot, true); + } + } + + /// + /// Verifies that a project without saved settings installs at the Unity project root. + /// + [Test] + public void GetSelection_WhenNothingIsSaved_SelectsUnityProjectRoot() + { + string projectRoot = CreateNestedProject(withGitDirectory: true); + + SkillInstallRootSelection selection = new SkillInstallRootResolver().GetSelection(projectRoot); + + Assert.That(selection.SelectedRoot, Is.EqualTo(SkillInstallRoot.UnityProjectRoot)); + Assert.That(selection.SelectedRootPath, Is.EqualTo(Path.GetFullPath(projectRoot))); + } + + /// + /// Verifies that a saved Git repository root is read back and resolves to the repository directory. + /// + [Test] + public void SaveInstallRoot_WhenGitRepositoryRootIsSaved_ResolvesToRepositoryRoot() + { + string projectRoot = CreateNestedProject(withGitDirectory: true); + SkillInstallRootResolver resolver = new(); + + resolver.SaveInstallRoot(projectRoot, SkillInstallRoot.GitRepositoryRoot); + SkillInstallRootSelection selection = resolver.GetSelection(projectRoot); + + Assert.That(selection.SelectedRoot, Is.EqualTo(SkillInstallRoot.GitRepositoryRoot)); + Assert.That(selection.IsSelectedRootAvailable, Is.True); + Assert.That(selection.SelectedRootPath, Is.EqualTo(Path.GetFullPath(_temporaryRoot))); + } + + /// + /// Verifies that the saved file uses the installRoot spelling the uloop CLI reads. + /// + [Test] + public void SaveInstallRoot_WritesTheValueTheCliReads() + { + string projectRoot = CreateNestedProject(withGitDirectory: true); + + new SkillInstallRootResolver().SaveInstallRoot(projectRoot, SkillInstallRoot.GitRepositoryRoot); + + string json = File.ReadAllText(Path.Combine(projectRoot, ".uloop", "settings.skills.json")); + Assert.That(json.Replace(" ", string.Empty), Does.Contain("\"installRoot\":\"git\"")); + } + + /// + /// Verifies that a saved Git repository root is reported unavailable when no repository contains the project. + /// + [Test] + public void GetSelection_WhenGitRootIsSavedOutsideRepository_ReportsUnavailable() + { + string projectRoot = CreateNestedProject(withGitDirectory: false); + SkillInstallRootResolver resolver = new(); + resolver.SaveInstallRoot(projectRoot, SkillInstallRoot.GitRepositoryRoot); + + SkillInstallRootSelection selection = resolver.GetSelection(projectRoot); + + Assert.That(selection.IsGitRepositoryRootAvailable, Is.False); + Assert.That(selection.IsSelectedRootAvailable, Is.False); + Assert.That(selection.SelectedRootPath, Is.Null); + } + + /// + /// Verifies that a .git file, as used by worktrees and submodules, marks the repository root. + /// + [Test] + public void FindGitRepositoryRoot_WhenAncestorHasGitFile_ReturnsThatAncestor() + { + string projectRoot = CreateNestedProject(withGitDirectory: false); + File.WriteAllText(Path.Combine(_temporaryRoot, ".git"), "gitdir: elsewhere\n"); + + string gitRoot = SkillInstallRootResolver.FindGitRepositoryRoot(projectRoot); + + Assert.That(gitRoot, Is.EqualTo(Path.GetFullPath(_temporaryRoot))); + } + + /// + /// Verifies that the search starts at the Unity project root itself. + /// + [Test] + public void FindGitRepositoryRoot_WhenProjectRootHoldsGit_ReturnsProjectRoot() + { + string projectRoot = CreateNestedProject(withGitDirectory: true); + Directory.CreateDirectory(Path.Combine(projectRoot, ".git")); + + string gitRoot = SkillInstallRootResolver.FindGitRepositoryRoot(projectRoot); + + Assert.That(gitRoot, Is.EqualTo(Path.GetFullPath(projectRoot))); + } + + /// + /// Verifies that detection with a saved Git repository root reports targets found under the repository root. + /// + [Test] + public void DetectSkillTargets_WhenGitRootIsSaved_DetectsTargetsUnderRepositoryRoot() + { + string projectRoot = CreateNestedProject(withGitDirectory: true); + Directory.CreateDirectory(Path.Combine(_temporaryRoot, ".codex")); + Directory.CreateDirectory(Path.Combine(projectRoot, ".agent")); + ToolSkillSetupService service = new(new NoDisabledToolsPort()); + service.SaveSkillInstallRoot(projectRoot, SkillInstallRoot.GitRepositoryRoot); + + List targets = service.DetectSkillTargetsForLayoutAtProjectRoot( + projectRoot, + groupSkillsUnderUnityCliLoop: false); + + Assert.That(targets.Count, Is.EqualTo(1)); + Assert.That(targets[0].DirName, Is.EqualTo(".codex")); + } + + /// + /// Verifies that detection reports no targets when the saved Git repository root does not exist. + /// + [Test] + public void DetectSkillTargets_WhenGitRootIsUnavailable_ReportsNoTargets() + { + string projectRoot = CreateNestedProject(withGitDirectory: false); + Directory.CreateDirectory(Path.Combine(projectRoot, ".codex")); + ToolSkillSetupService service = new(new NoDisabledToolsPort()); + service.SaveSkillInstallRoot(projectRoot, SkillInstallRoot.GitRepositoryRoot); + + List targets = service.DetectSkillTargetsForLayoutFastAtProjectRoot( + projectRoot, + groupSkillsUnderUnityCliLoop: false); + + Assert.That(targets, Is.Empty); + } + + /// + /// Verifies that installing the migration skill into an unavailable Git repository root fails without writing files. + /// + [Test] + public void InstallV3MigrationSkillFiles_WhenGitRootIsUnavailable_ThrowsWithoutWriting() + { + string projectRoot = CreateNestedProject(withGitDirectory: false); + ToolSkillSetupService service = new(new NoDisabledToolsPort()); + service.SaveSkillInstallRoot(projectRoot, SkillInstallRoot.GitRepositoryRoot); + SkillSetupTargetInfo target = new( + "Claude Code", + ".claude", + "--claude", + hasSkillsDirectory: true, + hasExistingSkills: false); + + Assert.ThrowsAsync(() => service.InstallV3MigrationSkillFilesAsync( + projectRoot, + new List { target }, + groupSkillsUnderUnityCliLoop: false, + CancellationToken.None)); + Assert.That(Directory.Exists(Path.Combine(projectRoot, ".claude")), Is.False); + } + + /// + /// Verifies that the migration skill is installed under the Git repository root when it is saved. + /// + [Test] + public async Task InstallV3MigrationSkillFiles_WhenGitRootIsSaved_InstallsUnderRepositoryRoot() + { + string projectRoot = CreateNestedProject(withGitDirectory: true); + ToolSkillSetupService service = new(new NoDisabledToolsPort()); + service.SaveSkillInstallRoot(projectRoot, SkillInstallRoot.GitRepositoryRoot); + SkillSetupTargetInfo target = new( + "Claude Code", + ".claude", + "--claude", + hasSkillsDirectory: true, + hasExistingSkills: false); + + await service.InstallV3MigrationSkillFilesAsync( + projectRoot, + new List { target }, + groupSkillsUnderUnityCliLoop: false, + CancellationToken.None); + + Assert.That( + File.Exists(Path.Combine(_temporaryRoot, ".claude", "skills", "v3-cli-invocation-migration", "SKILL.md")), + Is.True); + Assert.That(Directory.Exists(Path.Combine(projectRoot, ".claude")), Is.False); + } + + // Builds /unity as the Unity project, optionally with .git at the temporary root. + private string CreateNestedProject(bool withGitDirectory) + { + if (withGitDirectory) + { + Directory.CreateDirectory(Path.Combine(_temporaryRoot, ".git")); + } + + string projectRoot = Path.Combine(_temporaryRoot, "unity"); + Directory.CreateDirectory(projectRoot); + return projectRoot; + } + + private sealed class NoDisabledToolsPort : IToolSettingsPort + { + public bool IsToolEnabled(string toolName) => true; + + public void SetToolEnabled(string toolName, bool enabled) + { + } + + public string[] GetDisabledTools() => Array.Empty(); + + public void InvalidateCache() + { + } + } + } +} diff --git a/Assets/Tests/Editor/SkillInstallRootResolverTests.cs.meta b/Assets/Tests/Editor/SkillInstallRootResolverTests.cs.meta new file mode 100644 index 0000000000..f0bab9709f --- /dev/null +++ b/Assets/Tests/Editor/SkillInstallRootResolverTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 380b83483e724465087eb2cb67c25022 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/ThirdPartyToolMigrationWizardWorkflowControllerTests.cs b/Assets/Tests/Editor/ThirdPartyToolMigrationWizardWorkflowControllerTests.cs index 02009281c5..cc6663bd5a 100644 --- a/Assets/Tests/Editor/ThirdPartyToolMigrationWizardWorkflowControllerTests.cs +++ b/Assets/Tests/Editor/ThirdPartyToolMigrationWizardWorkflowControllerTests.cs @@ -396,6 +396,12 @@ public Task ApplyMigrationAsync( private sealed class NoOpSkillSetupPort : ISkillSetupPort { + public SkillInstallRootSelection GetSkillInstallRootSelection(string projectRoot) => + new(SkillInstallRoot.UnityProjectRoot, projectRoot, null); + + public void SaveSkillInstallRoot(string projectRoot, SkillInstallRoot root) => + throw new NotSupportedException(); + public void RemoveSkillFiles(string toolName) { } diff --git a/Assets/Tests/Editor/ThirdPartyToolMigrationWizardWorkflowStateTests.cs b/Assets/Tests/Editor/ThirdPartyToolMigrationWizardWorkflowStateTests.cs index 84c79e0a0a..e9756c4e96 100644 --- a/Assets/Tests/Editor/ThirdPartyToolMigrationWizardWorkflowStateTests.cs +++ b/Assets/Tests/Editor/ThirdPartyToolMigrationWizardWorkflowStateTests.cs @@ -436,6 +436,12 @@ public Task RemoveV3MigrationSkillFilesAsync( return Task.CompletedTask; } + public SkillInstallRootSelection GetSkillInstallRootSelection(string projectRoot) => + new(SkillInstallRoot.UnityProjectRoot, projectRoot, null); + + public void SaveSkillInstallRoot(string projectRoot, SkillInstallRoot root) => + throw new NotSupportedException(); + public void RemoveSkillFiles(string toolName) { throw new NotSupportedException(); diff --git a/Assets/Tests/Editor/ToolSkillSynchronizerRemovalTests.cs b/Assets/Tests/Editor/ToolSkillSynchronizerRemovalTests.cs index 76daf6ef3b..be7f7f7889 100644 --- a/Assets/Tests/Editor/ToolSkillSynchronizerRemovalTests.cs +++ b/Assets/Tests/Editor/ToolSkillSynchronizerRemovalTests.cs @@ -43,7 +43,7 @@ public void RemoveSkillFilesAtProjectRoot_WhenInstalledSkillsBelongToOtherTools_ WriteInstalledSkillFile(groupedSkillDirectory, "---\nname: uloop-get-logs\n---\n"); WriteInstalledSkillFile(flatSkillDirectory, "---\nname: uloop-run-tests\n---\n"); - ToolSkillSynchronizer.RemoveSkillFilesAtProjectRoot(_temporaryProjectRoot, "compile"); + ToolSkillSynchronizer.RemoveSkillFilesAtInstallRoot(_temporaryProjectRoot, "compile"); Assert.That(File.Exists(Path.Combine(groupedSkillDirectory, SkillInstallLayout.SkillFileName)), Is.True); Assert.That(File.Exists(Path.Combine(flatSkillDirectory, SkillInstallLayout.SkillFileName)), Is.True); diff --git a/Assets/Tests/Editor/ToolSkillSynchronizerTests.cs b/Assets/Tests/Editor/ToolSkillSynchronizerTests.cs index 9fb6d64bb1..e2c798d56d 100644 --- a/Assets/Tests/Editor/ToolSkillSynchronizerTests.cs +++ b/Assets/Tests/Editor/ToolSkillSynchronizerTests.cs @@ -86,12 +86,15 @@ public async Task InstallSkillFiles_DoesNotCreateNonExistentTargetDirectories() // Act List targets = SkillTargetDetector.DetectTargetsForLayoutStateAtProjectRoot( + _projectRoot, _projectRoot, requireSkillsDirectory: true, groupSkillsUnderUnityCliLoop: false, includeFreshnessCheck: false, disabledTools: Array.Empty()); - await ToolSkillSynchronizer.InstallSkillFiles( + await ToolSkillSynchronizer.InstallSkillFilesAtProjectRoot( + _projectRoot, + _projectRoot, targets, groupSkillsUnderUnityCliLoop: false, Array.Empty(), @@ -123,7 +126,9 @@ public async Task InstallSkillFiles_WhenToolIsDisabled_DoesNotRecreateSkill() hasExistingSkills: true); ToolSkillSynchronizer.SkillInstallResult result = - await ToolSkillSynchronizer.InstallSkillFiles( + await ToolSkillSynchronizer.InstallSkillFilesAtProjectRoot( + _projectRoot, + _projectRoot, new List { target }, groupSkillsUnderUnityCliLoop: false, disabledTools: new[] { "compile" }, @@ -156,6 +161,7 @@ public async Task InstallSkillFilesAtProjectRoot_WhenManagedSkillWasDeleted_Rest ToolSkillSynchronizer.SkillInstallResult result = await ToolSkillSynchronizer.InstallSkillFilesAtProjectRoot( + temporaryRoot, temporaryRoot, new[] { target }, groupSkillsUnderUnityCliLoop: true, @@ -205,6 +211,7 @@ public async Task InstallSkillFilesAtProjectRoot_WhenFlatLayoutRequested_Install ToolSkillSynchronizer.SkillInstallResult result = await ToolSkillSynchronizer.InstallSkillFilesAtProjectRoot( + temporaryRoot, temporaryRoot, new[] { target }, groupSkillsUnderUnityCliLoop: false, @@ -257,6 +264,7 @@ public async Task InstallSkillFilesAtProjectRoot_WhenFlatLayoutRequested_Removes ToolSkillSynchronizer.SkillInstallResult result = await ToolSkillSynchronizer.InstallSkillFilesAtProjectRoot( + temporaryRoot, temporaryRoot, new[] { target }, groupSkillsUnderUnityCliLoop: false, @@ -301,6 +309,7 @@ public async Task InstallSkillFilesAtProjectRoot_WhenFlatLayoutRequested_Removes ToolSkillSynchronizer.SkillInstallResult result = await ToolSkillSynchronizer.InstallSkillFilesAtProjectRoot( + temporaryRoot, temporaryRoot, new[] { target }, groupSkillsUnderUnityCliLoop: false, @@ -353,6 +362,7 @@ public async Task InstallSkillFilesAtProjectRoot_WhenDisabledSkillExistsInBothLa ToolSkillSynchronizer.SkillInstallResult result = await ToolSkillSynchronizer.InstallSkillFilesAtProjectRoot( + temporaryRoot, temporaryRoot, new[] { target }, groupSkillsUnderUnityCliLoop: false, @@ -396,6 +406,7 @@ public async Task InstallSkillFilesAtProjectRoot_WhenFlatLayoutRequested_Install ToolSkillSynchronizer.SkillInstallResult result = await ToolSkillSynchronizer.InstallSkillFilesAtProjectRoot( + temporaryRoot, temporaryRoot, new[] { target }, groupSkillsUnderUnityCliLoop: false, @@ -441,6 +452,7 @@ public async Task InstallSkillFilesForToolAtProjectRoot_DoesNotUpdateUnrelatedSk ToolSkillSynchronizer.SkillInstallResult result = await ToolSkillSynchronizer.InstallSkillFilesForToolAtProjectRoot( + temporaryRoot, temporaryRoot, "enabled-skill", groupSkillsUnderUnityCliLoop: false, @@ -476,6 +488,7 @@ public async Task InstallSkillFilesForToolAtProjectRoot_WhenRequestedToolIsDisab ToolSkillSynchronizer.SkillInstallResult result = await ToolSkillSynchronizer.InstallSkillFilesForToolAtProjectRoot( + temporaryRoot, temporaryRoot, "disabled-skill", groupSkillsUnderUnityCliLoop: false, @@ -566,6 +579,7 @@ public async Task InstallSkillFilesForToolAtProjectRoot_RemovesDisabledAndDeprec ToolSkillSynchronizer.SkillInstallResult result = await ToolSkillSynchronizer.InstallSkillFilesForToolAtProjectRoot( + temporaryRoot, temporaryRoot, "enabled-skill", groupSkillsUnderUnityCliLoop: false, @@ -617,6 +631,7 @@ public async Task InstallSkillFilesForToolAtProjectRoot_WhenFlatLayoutRequested_ ToolSkillSynchronizer.SkillInstallResult result = await ToolSkillSynchronizer.InstallSkillFilesForToolAtProjectRoot( + temporaryRoot, temporaryRoot, "enabled-skill", groupSkillsUnderUnityCliLoop: false, @@ -661,6 +676,7 @@ public async Task InstallSkillFilesAtProjectRoot_WhenFlatLayoutRequested_Preserv ToolSkillSynchronizer.SkillInstallResult result = await ToolSkillSynchronizer.InstallSkillFilesAtProjectRoot( + temporaryRoot, temporaryRoot, new[] { target }, groupSkillsUnderUnityCliLoop: false, @@ -708,6 +724,7 @@ public async Task InstallSkillFilesAtProjectRoot_WhenManagedSkillsParentOnlyHasE ToolSkillSynchronizer.SkillInstallResult result = await ToolSkillSynchronizer.InstallSkillFilesAtProjectRoot( + temporaryRoot, temporaryRoot, new[] { target }, groupSkillsUnderUnityCliLoop: false, @@ -730,6 +747,7 @@ public void DetectTargets_DoesNotIncludeTargetsWithOnlyParentDirectory() // Act string[] detectedTargetDirs = SkillTargetDetector.DetectTargetsForLayoutStateAtProjectRoot( + temporaryRoot, temporaryRoot, requireSkillsDirectory: true, groupSkillsUnderUnityCliLoop: false, @@ -756,6 +774,7 @@ public void DetectTargets_WhenParentDirectoryExists_ReportsTargetAsNotOptedIn() } ToolSkillSynchronizer.SkillTargetInfo[] detectedTargets = SkillTargetDetector.DetectTargetsForLayoutStateAtProjectRoot( + temporaryRoot, temporaryRoot, requireSkillsDirectory: false, groupSkillsUnderUnityCliLoop: false, @@ -787,6 +806,7 @@ public void DetectTargets_IncludesTargetsWhenSkillsDirectoryExists() // Act ToolSkillSynchronizer.SkillTargetInfo[] detectedTargets = SkillTargetDetector.DetectTargetsForLayoutStateAtProjectRoot( + temporaryRoot, temporaryRoot, requireSkillsDirectory: true, groupSkillsUnderUnityCliLoop: false, @@ -813,7 +833,7 @@ public void RemoveSkillFiles_DoesNotCreateNonExistentTargetDirectories() string temporaryRoot = CreateTemporaryProjectRoot(); string testToolName = "compile"; - ToolSkillSynchronizer.RemoveSkillFilesAtProjectRoot(temporaryRoot, testToolName); + ToolSkillSynchronizer.RemoveSkillFilesAtInstallRoot(temporaryRoot, testToolName); foreach (string dir in SkillTargetDetector.SkillTargetDirs) { @@ -830,7 +850,7 @@ public void IsSkillInstalled_DoesNotCreateNonExistentTargetDirectories() string testToolName = "compile"; // Act - ToolSkillSynchronizer.IsSkillInstalled(testToolName); + ToolSkillSynchronizer.IsSkillInstalledAtInstallRoot(_projectRoot, testToolName); // Assert: directories that didn't exist before should still not exist foreach (string dir in _nonExistentDirsBefore) @@ -857,6 +877,7 @@ public void DetectTargets_WhenManagedSkillsDirectoryContainsSkills_ReportsInstal } ToolSkillSynchronizer.SkillTargetInfo[] detectedTargets = SkillTargetDetector.DetectTargetsForLayoutStateAtProjectRoot( + temporaryRoot, temporaryRoot, requireSkillsDirectory: true, groupSkillsUnderUnityCliLoop: true, @@ -883,6 +904,7 @@ public void DetectTargets_WhenGroupedLayoutRequested_IgnoresFlatInstalledSkills( } ToolSkillSynchronizer.SkillTargetInfo[] detectedTargets = SkillTargetDetector.DetectTargetsForLayoutStateAtProjectRoot( + temporaryRoot, temporaryRoot, requireSkillsDirectory: true, groupSkillsUnderUnityCliLoop: true, @@ -907,6 +929,7 @@ public void DetectTargets_WhenGroupedLayoutRequested_DetectsEmptyFlatManagedDire Directory.CreateDirectory(Path.Combine(targetRoot, SkillInstallLayout.SkillsDirName, "uloop-compile")); ToolSkillSynchronizer.SkillTargetInfo[] detectedTargets = SkillTargetDetector.DetectTargetsForLayoutStateAtProjectRoot( + temporaryRoot, temporaryRoot, requireSkillsDirectory: true, groupSkillsUnderUnityCliLoop: true, @@ -937,6 +960,7 @@ public void DetectTargets_WhenFlatLayoutRequested_IgnoresGroupedInstalledSkills( } ToolSkillSynchronizer.SkillTargetInfo[] detectedTargets = SkillTargetDetector.DetectTargetsForLayoutStateAtProjectRoot( + temporaryRoot, temporaryRoot, requireSkillsDirectory: true, groupSkillsUnderUnityCliLoop: false, @@ -966,6 +990,7 @@ public void DetectTargets_WhenLegacyThirdPartySkillsExist_ReportsInstalled() } ToolSkillSynchronizer.SkillTargetInfo[] detectedTargets = SkillTargetDetector.DetectTargetsForLayoutStateAtProjectRoot( + temporaryRoot, temporaryRoot, requireSkillsDirectory: true, groupSkillsUnderUnityCliLoop: false, @@ -998,6 +1023,7 @@ public void DetectTargets_WhenOnlyGroupedThirdPartySkillsExist_DoesNotReportInst } ToolSkillSynchronizer.SkillTargetInfo[] detectedTargets = SkillTargetDetector.DetectTargetsForLayoutStateAtProjectRoot( + temporaryRoot, temporaryRoot, requireSkillsDirectory: true, groupSkillsUnderUnityCliLoop: true, @@ -1026,6 +1052,7 @@ public void DetectTargets_WhenOnlyManualLegacySkillsExist_DoesNotReportInstalled } ToolSkillSynchronizer.SkillTargetInfo[] detectedTargets = SkillTargetDetector.DetectTargetsForLayoutStateAtProjectRoot( + temporaryRoot, temporaryRoot, requireSkillsDirectory: true, groupSkillsUnderUnityCliLoop: false, @@ -1073,6 +1100,7 @@ public void DetectTargets_WhenSkillExistsInDependentPackageCache_ReportsInstalle File.WriteAllText(Path.Combine(installedSkillDir, "reference.md"), "reference"); ToolSkillSynchronizer.SkillTargetInfo[] detectedTargets = SkillTargetDetector.DetectTargetsForLayoutStateAtProjectRoot( + temporaryRoot, temporaryRoot, requireSkillsDirectory: true, groupSkillsUnderUnityCliLoop: true, @@ -1106,6 +1134,7 @@ public void DetectTargets_WhenExpectedLayoutMatchesSourceContent_ReportsInstalle "reference.md"), "reference"); ToolSkillSynchronizer.SkillTargetInfo[] detectedTargets = SkillTargetDetector.DetectTargetsForLayoutStateAtProjectRoot( + temporaryRoot, temporaryRoot, requireSkillsDirectory: true, groupSkillsUnderUnityCliLoop: true, @@ -1134,6 +1163,7 @@ public void DetectTargets_WhenExpectedLayoutDiffersFromSourceContent_ReportsOutd File.WriteAllText(Path.Combine(installedSkillDir, "reference.md"), "reference"); ToolSkillSynchronizer.SkillTargetInfo[] detectedTargets = SkillTargetDetector.DetectTargetsForLayoutStateAtProjectRoot( + temporaryRoot, temporaryRoot, requireSkillsDirectory: true, groupSkillsUnderUnityCliLoop: true, @@ -1169,6 +1199,7 @@ public void DetectTargets_WhenOnlyInternalSkillsAreMissing_IgnoresThem() File.WriteAllText(Path.Combine(installedSkillDir, "reference.md"), "reference"); ToolSkillSynchronizer.SkillTargetInfo[] detectedTargets = SkillTargetDetector.DetectTargetsForLayoutStateAtProjectRoot( + temporaryRoot, temporaryRoot, requireSkillsDirectory: true, groupSkillsUnderUnityCliLoop: true, @@ -1203,6 +1234,7 @@ public void DetectTargets_WhenSourceOnlyHasMetaSidecars_IgnoresThem() File.WriteAllText(Path.Combine(installedSkillDir, "reference.md"), "reference"); ToolSkillSynchronizer.SkillTargetInfo[] detectedTargets = SkillTargetDetector.DetectTargetsForLayoutStateAtProjectRoot( + temporaryRoot, temporaryRoot, requireSkillsDirectory: true, groupSkillsUnderUnityCliLoop: true, @@ -1237,6 +1269,7 @@ public void DetectTargets_WhenInstalledSkillHasExtraFiles_ReportsOutdated() File.WriteAllText(Path.Combine(installedSkillDir, "stale.md"), "stale"); ToolSkillSynchronizer.SkillTargetInfo[] detectedTargets = SkillTargetDetector.DetectTargetsForLayoutStateAtProjectRoot( + temporaryRoot, temporaryRoot, requireSkillsDirectory: true, groupSkillsUnderUnityCliLoop: true, @@ -1270,6 +1303,7 @@ public void DetectTargets_WhenInstalledSkillRootContainsExtraNonMarkdownFile_Rep File.WriteAllText(Path.Combine(installedSkillDir, "extra.yml"), "name: uloop-fake-skill\n"); ToolSkillSynchronizer.SkillTargetInfo[] detectedTargets = SkillTargetDetector.DetectTargetsForLayoutStateAtProjectRoot( + temporaryRoot, temporaryRoot, requireSkillsDirectory: true, groupSkillsUnderUnityCliLoop: true, @@ -1304,6 +1338,7 @@ public void DetectTargets_WhenShippedScriptInSubdirectoryDiffers_ReportsOutdated File.WriteAllText(installedScriptPath, "echo installed"); ToolSkillSynchronizer.SkillTargetInfo[] detectedTargets = SkillTargetDetector.DetectTargetsForLayoutStateAtProjectRoot( + temporaryRoot, temporaryRoot, requireSkillsDirectory: true, groupSkillsUnderUnityCliLoop: true, @@ -1337,6 +1372,7 @@ public void DetectTargets_WhenInstalledSkillUsesCrlfLineEndings_ReportsInstalled File.WriteAllText(Path.Combine(installedSkillDir, "reference.md"), "line1\r\nline2\r\n"); ToolSkillSynchronizer.SkillTargetInfo[] detectedTargets = SkillTargetDetector.DetectTargetsForLayoutStateAtProjectRoot( + temporaryRoot, temporaryRoot, requireSkillsDirectory: true, groupSkillsUnderUnityCliLoop: true, @@ -1369,6 +1405,7 @@ public async Task InstallSkillFilesAtProjectRoot_WhenSourceSkillUsesCrlfLineEndi hasExistingSkills: false); await ToolSkillSynchronizer.InstallSkillFilesAtProjectRoot( + temporaryRoot, temporaryRoot, new[] { target }, groupSkillsUnderUnityCliLoop: true, @@ -1442,6 +1479,7 @@ public void DetectTargets_WhenDeprecatedManagedSkillDirectoryExists_DoesNotRepor "uloop-execute-menu-item"); WriteSkillFile(executeMenuItemSkillDir, "---\nname: uloop-execute-menu-item\n---\n"); ToolSkillSynchronizer.SkillTargetInfo[] detectedTargets = SkillTargetDetector.DetectTargetsForLayoutStateAtProjectRoot( + temporaryRoot, temporaryRoot, requireSkillsDirectory: true, groupSkillsUnderUnityCliLoop: true, @@ -1501,6 +1539,7 @@ public async Task InstallSkillFilesAtProjectRoot_WhenDeprecatedManagedSkillDirec ToolSkillSynchronizer.SkillInstallResult result = await ToolSkillSynchronizer.InstallSkillFilesAtProjectRoot( + temporaryRoot, temporaryRoot, new[] { target }, groupSkillsUnderUnityCliLoop: true, @@ -1551,6 +1590,7 @@ public async Task InstallSkillFilesAtProjectRoot_WhenSettingsUpdateButtonTargetI ToolSkillSynchronizer.SkillInstallResult result = await ToolSkillSynchronizer.InstallSkillFilesAtProjectRoot( + temporaryRoot, temporaryRoot, new[] { target }, groupSkillsUnderUnityCliLoop: false, @@ -1577,6 +1617,7 @@ public void DetectTargets_WhenSourceSkillNameIsNotSafePathComponent_IgnoresIt() SkillInstallLayout.SkillsDirName)); ToolSkillSynchronizer.SkillTargetInfo[] detectedTargets = SkillTargetDetector.DetectTargetsForLayoutStateAtProjectRoot( + temporaryRoot, temporaryRoot, requireSkillsDirectory: true, groupSkillsUnderUnityCliLoop: true, @@ -1606,6 +1647,7 @@ public void DetectTargets_WhenSourceSkillNameContainsUnsafePathCharacters_Ignore SkillInstallLayout.SkillsDirName)); ToolSkillSynchronizer.SkillTargetInfo[] detectedTargets = SkillTargetDetector.DetectTargetsForLayoutStateAtProjectRoot( + temporaryRoot, temporaryRoot, requireSkillsDirectory: true, groupSkillsUnderUnityCliLoop: true, @@ -1639,7 +1681,7 @@ public async Task InstallSpecificSkillFilesAtProjectRoot_WhenSingleSkillSourceIs }); ToolSkillSynchronizer.SkillInstallResult result = - await V3MigrationSkillInstaller.InstallSpecificSkillFilesAtProjectRoot( + await V3MigrationSkillInstaller.InstallSpecificSkillFilesAtInstallRoot( temporaryRoot, new[] { target }, skill, @@ -1651,7 +1693,7 @@ await V3MigrationSkillInstaller.InstallSpecificSkillFilesAtProjectRoot( ".codex", SkillInstallLayout.SkillsDirName, CliConstants.V3_CLI_INVOCATION_MIGRATION_SKILL_NAME); - SkillInstallState installState = V3MigrationSkillInstaller.GetSkillInstallStateAtProjectRoot( + SkillInstallState installState = V3MigrationSkillInstaller.GetSkillInstallStateAtInstallRoot( temporaryRoot, target, skill, @@ -1734,13 +1776,13 @@ public async Task GetV3MigrationSkillInstallStateAtProjectRoot_WhenSkillExistsIn hasExistingSkills: false); ToolSkillSynchronizer.SkillInstallResult result = - await V3MigrationSkillInstaller.InstallV3MigrationSkillFilesAtProjectRoot( + await V3MigrationSkillInstaller.InstallV3MigrationSkillFilesAtInstallRoot( temporaryRoot, new[] { target }, groupSkillsUnderUnityCliLoop: true, ct: CancellationToken.None); - SkillInstallState installState = V3MigrationSkillInstaller.GetV3MigrationSkillInstallStateAtProjectRoot( + SkillInstallState installState = V3MigrationSkillInstaller.GetV3MigrationSkillInstallStateAtInstallRoot( temporaryRoot, target, groupSkillsUnderUnityCliLoop: false); @@ -1767,7 +1809,7 @@ public void GetV3MigrationSkillInstallStateAtProjectRoot_WhenSkillDirectoryLacks groupSkillsUnderUnityCliLoop: false); Directory.CreateDirectory(Path.Combine(migrationSkillDir, "references")); - SkillInstallState installState = V3MigrationSkillInstaller.GetV3MigrationSkillInstallStateAtProjectRoot( + SkillInstallState installState = V3MigrationSkillInstaller.GetV3MigrationSkillInstallStateAtInstallRoot( temporaryRoot, target, groupSkillsUnderUnityCliLoop: false); @@ -1799,7 +1841,7 @@ public async Task RemoveSpecificSkillFilesAtProjectRoot_WhenSkillExists_RemovesO WriteSkillFile(unrelatedSkillDir, "---\nname: uloop-compile\n---\n"); ToolSkillSynchronizer.SkillInstallResult result = - await V3MigrationSkillInstaller.RemoveSpecificSkillFilesAtProjectRoot( + await V3MigrationSkillInstaller.RemoveSpecificSkillFilesAtInstallRoot( temporaryRoot, new[] { target }, CliConstants.V3_CLI_INVOCATION_MIGRATION_SKILL_NAME, @@ -1842,7 +1884,7 @@ public async Task RemoveSpecificSkillFilesAtProjectRoot_WithMultipleTargets_Repo WriteSkillFile(claudeSkillDir, "---\nname: v3-cli-invocation-migration\n---\n"); ToolSkillSynchronizer.SkillInstallResult result = - await V3MigrationSkillInstaller.RemoveSpecificSkillFilesAtProjectRoot( + await V3MigrationSkillInstaller.RemoveSpecificSkillFilesAtInstallRoot( temporaryRoot, new[] { codexTarget, claudeTarget }, CliConstants.V3_CLI_INVOCATION_MIGRATION_SKILL_NAME, @@ -1885,7 +1927,7 @@ public async Task RemoveV3MigrationSkillFilesAtProjectRoot_WhenSkillExistsInBoth WriteSkillFile(unrelatedSkillDir, "---\nname: uloop-compile\n---\n"); ToolSkillSynchronizer.SkillInstallResult result = - await V3MigrationSkillInstaller.RemoveV3MigrationSkillFilesAtProjectRoot( + await V3MigrationSkillInstaller.RemoveV3MigrationSkillFilesAtInstallRoot( temporaryRoot, new[] { target }, groupSkillsUnderUnityCliLoop: false, diff --git a/Assets/Tests/Editor/UnityCliLoopSettingsSkillsPresenterStateTests.cs b/Assets/Tests/Editor/UnityCliLoopSettingsSkillsPresenterStateTests.cs index f1f13109c4..10aca4e7a0 100644 --- a/Assets/Tests/Editor/UnityCliLoopSettingsSkillsPresenterStateTests.cs +++ b/Assets/Tests/Editor/UnityCliLoopSettingsSkillsPresenterStateTests.cs @@ -626,6 +626,24 @@ public List DetectSkillTargetsForLayoutAtProjectRoot( return FullTargets; } + public SkillInstallRootSelection InstallRootSelection { get; set; } + + public List SavedInstallRoots { get; } = new(); + + public SkillInstallRootSelection GetSkillInstallRootSelection(string projectRoot) => + string.IsNullOrEmpty(InstallRootSelection.UnityProjectRootPath) + ? new SkillInstallRootSelection(SkillInstallRoot.UnityProjectRoot, projectRoot, null) + : InstallRootSelection; + + public void SaveSkillInstallRoot(string projectRoot, SkillInstallRoot root) + { + SavedInstallRoots.Add(root); + InstallRootSelection = new SkillInstallRootSelection( + root, + GetSkillInstallRootSelection(projectRoot).UnityProjectRootPath, + GetSkillInstallRootSelection(projectRoot).GitRepositoryRootPath); + } + public void RemoveSkillFiles(string toolName) { RemovedTools.Add(toolName); diff --git a/Assets/Tests/Editor/V3MigrationSkillInstallerTests.cs b/Assets/Tests/Editor/V3MigrationSkillInstallerTests.cs index 790d141714..511f1cfda5 100644 --- a/Assets/Tests/Editor/V3MigrationSkillInstallerTests.cs +++ b/Assets/Tests/Editor/V3MigrationSkillInstallerTests.cs @@ -53,7 +53,7 @@ public void GetSkillInstallStateAtProjectRoot_WhenTargetDirectoryIsMissing_Retur ["SKILL.md"] = new byte[] { 0x41 } }); - SkillInstallState state = V3MigrationSkillInstaller.GetSkillInstallStateAtProjectRoot( + SkillInstallState state = V3MigrationSkillInstaller.GetSkillInstallStateAtInstallRoot( _temporaryProjectRoot, target, skill, diff --git a/Packages/src/Editor/Application/UseCases/SkillSetupUseCase.cs b/Packages/src/Editor/Application/UseCases/SkillSetupUseCase.cs index 66cc2a9cab..60e41990cf 100644 --- a/Packages/src/Editor/Application/UseCases/SkillSetupUseCase.cs +++ b/Packages/src/Editor/Application/UseCases/SkillSetupUseCase.cs @@ -22,6 +22,16 @@ public SkillSetupUseCase(ISkillSetupPort skillSetupPort) _skillSetupPort = skillSetupPort ?? throw new ArgumentNullException(nameof(skillSetupPort)); } + public SkillInstallRootSelection GetSkillInstallRootSelection(string projectRoot) + { + return _skillSetupPort.GetSkillInstallRootSelection(projectRoot); + } + + public void SaveSkillInstallRoot(string projectRoot, SkillInstallRoot root) + { + _skillSetupPort.SaveSkillInstallRoot(projectRoot, root); + } + public void RemoveSkillFiles(string toolName) { _skillSetupPort.RemoveSkillFiles(toolName); diff --git a/Packages/src/Editor/Domain/ISkillSetupPort.cs b/Packages/src/Editor/Domain/ISkillSetupPort.cs index fb824e1692..64ea298f44 100644 --- a/Packages/src/Editor/Domain/ISkillSetupPort.cs +++ b/Packages/src/Editor/Domain/ISkillSetupPort.cs @@ -6,9 +6,15 @@ namespace io.github.hatayama.UnityCliLoop.Domain { /// /// Defines the file-system boundary for skill setup operations. + /// Every operation reads skill sources from the Unity project root and writes agent skill + /// folders under the saved skill install root, so callers never resolve the install root. + /// Installs into an unavailable install root (the Git repository root when no Git repository + /// contains the project) throw InvalidOperationException; detection reports no targets there. /// public interface ISkillSetupPort { + SkillInstallRootSelection GetSkillInstallRootSelection(string projectRoot); + void SaveSkillInstallRoot(string projectRoot, SkillInstallRoot root); void RemoveSkillFiles(string toolName); bool IsSkillInstalled(string toolName); List DetectSkillTargetsForLayoutAtProjectRoot( diff --git a/Packages/src/Editor/Domain/SkillInstallRoot.cs b/Packages/src/Editor/Domain/SkillInstallRoot.cs new file mode 100644 index 0000000000..09582bb652 --- /dev/null +++ b/Packages/src/Editor/Domain/SkillInstallRoot.cs @@ -0,0 +1,11 @@ +namespace io.github.hatayama.UnityCliLoop.Domain +{ + /// + /// Selects which directory receives project-local agent skill folders (.claude/skills, ...). + /// + public enum SkillInstallRoot + { + UnityProjectRoot, + GitRepositoryRoot + } +} diff --git a/Packages/src/Editor/Domain/SkillInstallRoot.cs.meta b/Packages/src/Editor/Domain/SkillInstallRoot.cs.meta new file mode 100644 index 0000000000..694d41e0bd --- /dev/null +++ b/Packages/src/Editor/Domain/SkillInstallRoot.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: af31a354708324d308494a6dc3f5d177 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Domain/SkillInstallRootSelection.cs b/Packages/src/Editor/Domain/SkillInstallRootSelection.cs new file mode 100644 index 0000000000..4b7947e28f --- /dev/null +++ b/Packages/src/Editor/Domain/SkillInstallRootSelection.cs @@ -0,0 +1,41 @@ +namespace io.github.hatayama.UnityCliLoop.Domain +{ + /// + /// Describes the saved skill install root together with the directories each choice resolves to. + /// + public readonly struct SkillInstallRootSelection + { + public readonly SkillInstallRoot SelectedRoot; + public readonly string UnityProjectRootPath; + + // Null when no Git repository contains the Unity project. + public readonly string GitRepositoryRootPath; + + public SkillInstallRootSelection( + SkillInstallRoot selectedRoot, + string unityProjectRootPath, + string gitRepositoryRootPath) + { + System.Diagnostics.Debug.Assert( + !string.IsNullOrEmpty(unityProjectRootPath), + "unityProjectRootPath must not be null or empty"); + + SelectedRoot = selectedRoot; + UnityProjectRootPath = unityProjectRootPath; + GitRepositoryRootPath = gitRepositoryRootPath; + } + + public bool IsGitRepositoryRootAvailable => !string.IsNullOrEmpty(GitRepositoryRootPath); + + public bool IsSelectedRootAvailable => + SelectedRoot == SkillInstallRoot.UnityProjectRoot || IsGitRepositoryRootAvailable; + + /// + /// Directory that receives agent skill folders, or null when the Git repository root is + /// selected but no Git repository contains the Unity project. + /// + public string SelectedRootPath => SelectedRoot == SkillInstallRoot.GitRepositoryRoot + ? GitRepositoryRootPath + : UnityProjectRootPath; + } +} diff --git a/Packages/src/Editor/Domain/SkillInstallRootSelection.cs.meta b/Packages/src/Editor/Domain/SkillInstallRootSelection.cs.meta new file mode 100644 index 0000000000..a1f310cd28 --- /dev/null +++ b/Packages/src/Editor/Domain/SkillInstallRootSelection.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 985c8292379824adba5743c7dc0b2255 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Infrastructure/SkillSetup/SkillInstallRootResolver.cs b/Packages/src/Editor/Infrastructure/SkillSetup/SkillInstallRootResolver.cs new file mode 100644 index 0000000000..2e9e6d437c --- /dev/null +++ b/Packages/src/Editor/Infrastructure/SkillSetup/SkillInstallRootResolver.cs @@ -0,0 +1,149 @@ +using System; +using System.IO; + +using UnityEngine; + +using io.github.hatayama.UnityCliLoop.Domain; +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.Infrastructure +{ + /// + /// Reads and saves the skill install root in .uloop/settings.skills.json and resolves it to a directory. + /// The file is shared with the uloop CLI, which installs into the same root when no --root is given. + /// + internal sealed class SkillInstallRootResolver + { + internal const string SettingsFileName = "settings.skills.json"; + + // These spellings are the file contract with the CLI (cli/dispatcher skills_install_root.go). + private const string UnityProjectRootValue = "unity"; + private const string GitRepositoryRootValue = "git"; + private const string GitMarkerName = ".git"; + + [Serializable] + private sealed class SkillSettingsJsonData + { + public string installRoot = string.Empty; + } + + internal SkillInstallRootSelection GetSelection(string projectRoot) + { + Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty"); + + string fullProjectRoot = Path.GetFullPath(projectRoot); + return new SkillInstallRootSelection( + LoadSavedRoot(fullProjectRoot), + fullProjectRoot, + FindGitRepositoryRoot(fullProjectRoot)); + } + + internal void SaveInstallRoot(string projectRoot, SkillInstallRoot root) + { + Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty"); + + string settingsPath = GetSettingsPath(projectRoot); + Directory.CreateDirectory(Path.GetDirectoryName(settingsPath)); + SkillSettingsJsonData data = new() { installRoot = ToSettingsValue(root) }; + AtomicFileWriter.Write(settingsPath, JsonUtility.ToJson(data, true)); + AtomicFileWriter.CleanupBackup(settingsPath + AtomicFileWriter.BackupFileSuffix); + } + + /// + /// Returns the nearest directory at or above the Unity project root that holds .git, or null. + /// A .git file counts too, because worktree and submodule checkouts mark their root with one. + /// + internal static string FindGitRepositoryRoot(string projectRoot) + { + Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty"); + + DirectoryInfo current = new(Path.GetFullPath(projectRoot)); + while (current != null) + { + string marker = Path.Combine(current.FullName, GitMarkerName); + if (Directory.Exists(marker) || File.Exists(marker)) + { + return current.FullName; + } + + current = current.Parent; + } + + return null; + } + + private static SkillInstallRoot LoadSavedRoot(string projectRoot) + { + string settingsPath = GetSettingsPath(projectRoot); + AtomicFileWriter.RecoverSidecarFiles(settingsPath); + if (!File.Exists(settingsPath)) + { + return SkillInstallRoot.UnityProjectRoot; + } + + string json = File.ReadAllText(settingsPath); + if (string.IsNullOrWhiteSpace(json)) + { + return SkillInstallRoot.UnityProjectRoot; + } + + string value = ReadInstallRootValue(json); + if (value == null) + { + WarnUnusableSettings(settingsPath, "cannot be parsed"); + return SkillInstallRoot.UnityProjectRoot; + } + + if (value == string.Empty || value == UnityProjectRootValue) + { + return SkillInstallRoot.UnityProjectRoot; + } + + if (value == GitRepositoryRootValue) + { + return SkillInstallRoot.GitRepositoryRoot; + } + + WarnUnusableSettings(settingsPath, $"has an unknown installRoot '{value}'"); + return SkillInstallRoot.UnityProjectRoot; + } + + // The Editor keeps working so the user can pick a root again, which rewrites the file; + // the CLI rejects the same file, so the problem cannot go unnoticed there. + private static void WarnUnusableSettings(string settingsPath, string problem) + { + Debug.LogWarning( + $"[UnityCliLoop] {settingsPath} {problem}. " + + "Using the Unity project root until an install location is chosen again."); + } + + // Returns null when the JSON is malformed. + private static string ReadInstallRootValue(string json) + { + try + { + SkillSettingsJsonData data = JsonUtility.FromJson(json); + return data == null || data.installRoot == null ? string.Empty : data.installRoot; + } + catch (ArgumentException) + { + return null; + } + } + + private static string GetSettingsPath(string projectRoot) + { + return Path.Combine(projectRoot, UnityCliLoopConstants.ULOOP_DIR, SettingsFileName); + } + + private static string ToSettingsValue(SkillInstallRoot root) + { + return root switch + { + SkillInstallRoot.UnityProjectRoot => UnityProjectRootValue, + SkillInstallRoot.GitRepositoryRoot => GitRepositoryRootValue, + _ => throw new ArgumentOutOfRangeException(nameof(root), root, "Unknown skill install root") + }; + } + } +} diff --git a/Packages/src/Editor/Infrastructure/SkillSetup/SkillInstallRootResolver.cs.meta b/Packages/src/Editor/Infrastructure/SkillSetup/SkillInstallRootResolver.cs.meta new file mode 100644 index 0000000000..51f1887795 --- /dev/null +++ b/Packages/src/Editor/Infrastructure/SkillSetup/SkillInstallRootResolver.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 5e1a1686cefac435abedfe977ba14cc3 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Infrastructure/SkillSetup/SkillTargetDetector.cs b/Packages/src/Editor/Infrastructure/SkillSetup/SkillTargetDetector.cs index 5f4ab0700e..d8c8a4011c 100644 --- a/Packages/src/Editor/Infrastructure/SkillSetup/SkillTargetDetector.cs +++ b/Packages/src/Editor/Infrastructure/SkillSetup/SkillTargetDetector.cs @@ -39,19 +39,21 @@ internal SkillTargetDefinition(string dirName, string flag, string displayName) internal static List DetectTargetsForLayoutStateAtProjectRoot( string projectRoot, + string installRoot, bool requireSkillsDirectory, bool groupSkillsUnderUnityCliLoop, bool includeFreshnessCheck, IReadOnlyCollection disabledTools) { Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty"); + Debug.Assert(!string.IsNullOrEmpty(installRoot), "installRoot must not be null or empty"); Debug.Assert(disabledTools != null, "disabledTools must not be null"); List targets = new(); foreach (SkillTargetDefinition target in SkillTargets) { - string targetRoot = Path.Combine(projectRoot, target.DirName); + string targetRoot = Path.Combine(installRoot, target.DirName); if (!Directory.Exists(targetRoot)) { continue; @@ -88,12 +90,12 @@ internal SkillTargetDefinition(string dirName, string flag, string displayName) return targets; } - internal static List DetectTargetsWithSkillsDirectory(string projectRoot) + internal static List DetectTargetsWithSkillsDirectory(string installRoot) { List targets = new(); foreach (SkillTargetDefinition target in SkillTargets) { - string targetRoot = Path.Combine(projectRoot, target.DirName); + string targetRoot = Path.Combine(installRoot, target.DirName); if (!Directory.Exists(targetRoot)) { continue; diff --git a/Packages/src/Editor/Infrastructure/SkillSetup/SkillTargetInstaller.cs b/Packages/src/Editor/Infrastructure/SkillSetup/SkillTargetInstaller.cs index 9bedebf937..95e22527fc 100644 --- a/Packages/src/Editor/Infrastructure/SkillSetup/SkillTargetInstaller.cs +++ b/Packages/src/Editor/Infrastructure/SkillSetup/SkillTargetInstaller.cs @@ -34,7 +34,7 @@ private enum SkillSyncScope } internal static void InstallSkillsForTarget( - string projectRoot, + string installRoot, ToolSkillSynchronizer.SkillTargetInfo target, IReadOnlyCollection disabledSkills, IReadOnlyCollection enabledSkills, @@ -42,7 +42,7 @@ internal static void InstallSkillsForTarget( CancellationToken ct) { InstallSkillsForTargetCore( - projectRoot, + installRoot, target, disabledSkills, enabledSkills, @@ -52,7 +52,7 @@ internal static void InstallSkillsForTarget( } internal static void InstallSpecificSkillsForTarget( - string projectRoot, + string installRoot, ToolSkillSynchronizer.SkillTargetInfo target, IReadOnlyCollection disabledSkills, IReadOnlyCollection skills, @@ -60,7 +60,7 @@ internal static void InstallSpecificSkillsForTarget( CancellationToken ct) { InstallSkillsForTargetCore( - projectRoot, + installRoot, target, disabledSkills, skills, @@ -70,7 +70,7 @@ internal static void InstallSpecificSkillsForTarget( } private static void InstallSkillsForTargetCore( - string projectRoot, + string installRoot, ToolSkillSynchronizer.SkillTargetInfo target, IReadOnlyCollection disabledSkills, IReadOnlyCollection skills, @@ -79,7 +79,7 @@ private static void InstallSkillsForTargetCore( CancellationToken ct) { ct.ThrowIfCancellationRequested(); - string targetRoot = Path.Combine(projectRoot, target.DirName); + string targetRoot = Path.Combine(installRoot, target.DirName); string skillsRoot = SkillInstallLayout.GetSkillsRoot(targetRoot); Directory.CreateDirectory(skillsRoot); diff --git a/Packages/src/Editor/Infrastructure/SkillSetup/ToolSkillSetupService.cs b/Packages/src/Editor/Infrastructure/SkillSetup/ToolSkillSetupService.cs index 44a7c3a835..bc825c86f9 100644 --- a/Packages/src/Editor/Infrastructure/SkillSetup/ToolSkillSetupService.cs +++ b/Packages/src/Editor/Infrastructure/SkillSetup/ToolSkillSetupService.cs @@ -5,6 +5,7 @@ using System.Threading.Tasks; using io.github.hatayama.UnityCliLoop.Domain; +using io.github.hatayama.UnityCliLoop.ToolContracts; namespace io.github.hatayama.UnityCliLoop.Infrastructure { @@ -15,6 +16,7 @@ namespace io.github.hatayama.UnityCliLoop.Infrastructure public sealed class ToolSkillSetupService : ISkillSetupPort { private readonly IToolSettingsPort _toolSettingsPort; + private readonly SkillInstallRootResolver _installRootResolver = new(); public ToolSkillSetupService(IToolSettingsPort toolSettingsPort) { @@ -23,23 +25,48 @@ public ToolSkillSetupService(IToolSettingsPort toolSettingsPort) _toolSettingsPort = toolSettingsPort ?? throw new System.ArgumentNullException(nameof(toolSettingsPort)); } + public SkillInstallRootSelection GetSkillInstallRootSelection(string projectRoot) + { + return _installRootResolver.GetSelection(projectRoot); + } + + public void SaveSkillInstallRoot(string projectRoot, SkillInstallRoot root) + { + _installRootResolver.SaveInstallRoot(projectRoot, root); + } + public void RemoveSkillFiles(string toolName) { - ToolSkillSynchronizer.RemoveSkillFiles(toolName); + string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); + if (!TryGetInstallRootForToolSync(projectRoot, toolName, out string installRoot)) + { + return; + } + + ToolSkillSynchronizer.RemoveSkillFilesAtInstallRoot(installRoot, toolName); } public bool IsSkillInstalled(string toolName) { - return ToolSkillSynchronizer.IsSkillInstalled(toolName); + string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); + string installRoot = _installRootResolver.GetSelection(projectRoot).SelectedRootPath; + return installRoot != null && ToolSkillSynchronizer.IsSkillInstalledAtInstallRoot(installRoot, toolName); } public List DetectSkillTargetsForLayoutAtProjectRoot( string projectRoot, bool groupSkillsUnderUnityCliLoop) { + string installRoot = _installRootResolver.GetSelection(projectRoot).SelectedRootPath; + if (installRoot == null) + { + return new List(); + } + List targets = ToolSkillSynchronizer.DetectTargetsForLayoutAtProjectRoot( projectRoot, + installRoot, groupSkillsUnderUnityCliLoop, _toolSettingsPort.GetDisabledTools()); return targets.Select(ToDomainInfo).ToList(); @@ -49,9 +76,16 @@ public List DetectSkillTargetsForLayoutFastAtProjectRoot( string projectRoot, bool groupSkillsUnderUnityCliLoop) { + string installRoot = _installRootResolver.GetSelection(projectRoot).SelectedRootPath; + if (installRoot == null) + { + return new List(); + } + List targets = ToolSkillSynchronizer.DetectTargetsForLayoutFastAtProjectRoot( projectRoot, + installRoot, groupSkillsUnderUnityCliLoop); return targets.Select(ToDomainInfo).ToList(); } @@ -64,10 +98,14 @@ public async Task InstallSkillFilesAsync( Debug.Assert(targets != null, "targets must not be null"); ct.ThrowIfCancellationRequested(); + string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); + string installRoot = GetInstallRootOrThrow(projectRoot); List synchronizerTargets = targets .Select(ToSynchronizerInfo) .ToList(); - await ToolSkillSynchronizer.InstallSkillFiles( + await ToolSkillSynchronizer.InstallSkillFilesAtProjectRoot( + projectRoot, + installRoot, synchronizerTargets, groupSkillsUnderUnityCliLoop, _toolSettingsPort.GetDisabledTools(), @@ -82,7 +120,15 @@ public async Task InstallSkillFilesForToolAsync( Debug.Assert(!string.IsNullOrEmpty(toolName), "toolName must not be null or empty"); ct.ThrowIfCancellationRequested(); - await ToolSkillSynchronizer.InstallSkillFilesForToolWithDisabledTools( + string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); + if (!TryGetInstallRootForToolSync(projectRoot, toolName, out string installRoot)) + { + return; + } + + await ToolSkillSynchronizer.InstallSkillFilesForToolAtProjectRoot( + projectRoot, + installRoot, toolName, groupSkillsUnderUnityCliLoop, _toolSettingsPort.GetDisabledTools(), @@ -96,8 +142,14 @@ public SkillInstallState GetV3MigrationSkillInstallStateAtProjectRoot( { Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty"); - return ToolSkillSynchronizer.GetV3MigrationSkillInstallStateAtProjectRoot( - projectRoot, + string installRoot = _installRootResolver.GetSelection(projectRoot).SelectedRootPath; + if (installRoot == null) + { + return SkillInstallState.Missing; + } + + return ToolSkillSynchronizer.GetV3MigrationSkillInstallStateAtInstallRoot( + installRoot, ToSynchronizerInfo(target), groupSkillsUnderUnityCliLoop); } @@ -112,11 +164,12 @@ public async Task InstallV3MigrationSkillFilesAsync( Debug.Assert(targets != null, "targets must not be null"); ct.ThrowIfCancellationRequested(); + string installRoot = GetInstallRootOrThrow(projectRoot); List synchronizerTargets = targets .Select(ToSynchronizerInfo) .ToList(); - await ToolSkillSynchronizer.InstallV3MigrationSkillFilesAtProjectRoot( - projectRoot, + await ToolSkillSynchronizer.InstallV3MigrationSkillFilesAtInstallRoot( + installRoot, synchronizerTargets, groupSkillsUnderUnityCliLoop, ct); @@ -132,16 +185,52 @@ public async Task RemoveV3MigrationSkillFilesAsync( Debug.Assert(targets != null, "targets must not be null"); ct.ThrowIfCancellationRequested(); + string installRoot = _installRootResolver.GetSelection(projectRoot).SelectedRootPath; + if (installRoot == null) + { + return; + } + List synchronizerTargets = targets .Select(ToSynchronizerInfo) .ToList(); - await ToolSkillSynchronizer.RemoveV3MigrationSkillFilesAtProjectRoot( - projectRoot, + await ToolSkillSynchronizer.RemoveV3MigrationSkillFilesAtInstallRoot( + installRoot, synchronizerTargets, groupSkillsUnderUnityCliLoop, ct); } + // An explicit install must not silently land somewhere the user did not choose. + private string GetInstallRootOrThrow(string projectRoot) + { + SkillInstallRootSelection selection = _installRootResolver.GetSelection(projectRoot); + if (!selection.IsSelectedRootAvailable) + { + throw new System.InvalidOperationException( + $"No Git repository contains the Unity project '{projectRoot}', so skills cannot be " + + "installed at the Git repository root. Choose the Unity project root as the install location."); + } + + return selection.SelectedRootPath; + } + + // Toggling a tool must keep working when skills cannot be synced, so an unavailable + // install root only skips the sync instead of failing the toggle. + private bool TryGetInstallRootForToolSync(string projectRoot, string toolName, out string installRoot) + { + installRoot = _installRootResolver.GetSelection(projectRoot).SelectedRootPath; + if (installRoot != null) + { + return true; + } + + UnityEngine.Debug.LogWarning( + $"[UnityCliLoop] Skipped syncing the skill for '{toolName}': no Git repository contains the " + + "Unity project, but the Git repository root is chosen as the skill install location."); + return false; + } + private static SkillSetupTargetInfo ToDomainInfo(ToolSkillSynchronizer.SkillTargetInfo target) { return new SkillSetupTargetInfo( diff --git a/Packages/src/Editor/Infrastructure/SkillSetup/ToolSkillSynchronizer.cs b/Packages/src/Editor/Infrastructure/SkillSetup/ToolSkillSynchronizer.cs index d53db8ba91..65570ba9ff 100644 --- a/Packages/src/Editor/Infrastructure/SkillSetup/ToolSkillSynchronizer.cs +++ b/Packages/src/Editor/Infrastructure/SkillSetup/ToolSkillSynchronizer.cs @@ -21,66 +21,54 @@ namespace io.github.hatayama.UnityCliLoop.Infrastructure /// public static partial class ToolSkillSynchronizer { - public static void RemoveSkillFiles(string toolName) + // Skill sources (and the tool settings that filter them) always come from the Unity + // project root, while target folders live under the install root, which is either the + // project root or the Git repository root containing it. Methods that only touch target + // folders therefore take the install root alone. + internal static void RemoveSkillFilesAtInstallRoot(string installRoot, string toolName) { + Debug.Assert(!string.IsNullOrEmpty(installRoot), "installRoot must not be null or empty"); Debug.Assert(!string.IsNullOrEmpty(toolName), "toolName must not be null or empty"); - string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); - RemoveSkillFilesAtProjectRoot(projectRoot, toolName); - } - - internal static void RemoveSkillFilesAtProjectRoot(string projectRoot, string toolName) - { - Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty"); - Debug.Assert(!string.IsNullOrEmpty(toolName), "toolName must not be null or empty"); - - foreach (string targetDir in SkillTargetDetector.SkillTargetDirs) + foreach (string skillDir in EnumerateInstalledToolSkillDirectories(installRoot, toolName)) { - string targetRoot = Path.Combine(projectRoot, targetDir); - if (!Directory.Exists(targetRoot)) - { - continue; - } - - foreach (string skillDir in SkillInstallLayout.EnumerateInstalledSkillDirectories(targetRoot)) - { - if (SkillInstallLayout.SkillMatchesTool(skillDir, toolName)) - { - Debug.Log($"[UnityCliLoop] Removing skill '{toolName}' from '{skillDir}'"); - Directory.Delete(skillDir, true); - } - } + Debug.Log($"[UnityCliLoop] Removing skill '{toolName}' from '{skillDir}'"); + Directory.Delete(skillDir, true); } } - public static bool IsSkillInstalled(string toolName) + internal static bool IsSkillInstalledAtInstallRoot(string installRoot, string toolName) { + Debug.Assert(!string.IsNullOrEmpty(installRoot), "installRoot must not be null or empty"); Debug.Assert(!string.IsNullOrEmpty(toolName), "toolName must not be null or empty"); - string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); + return EnumerateInstalledToolSkillDirectories(installRoot, toolName).Any(); + } + private static IEnumerable EnumerateInstalledToolSkillDirectories(string installRoot, string toolName) + { foreach (string targetDir in SkillTargetDetector.SkillTargetDirs) { - string targetRoot = Path.Combine(projectRoot, targetDir); + string targetRoot = Path.Combine(installRoot, targetDir); if (!Directory.Exists(targetRoot)) { continue; } - foreach (string skillDir in SkillInstallLayout.EnumerateInstalledSkillDirectories(targetRoot)) + // Materialized so callers may delete the yielded directories while iterating. + foreach (string skillDir in SkillInstallLayout.EnumerateInstalledSkillDirectories(targetRoot).ToList()) { if (SkillInstallLayout.SkillMatchesTool(skillDir, toolName)) { - return true; + yield return skillDir; } } } - - return false; } internal static List DetectTargetsForLayoutAtProjectRoot( string projectRoot, + string installRoot, bool groupSkillsUnderUnityCliLoop, IReadOnlyCollection disabledTools) { @@ -89,6 +77,7 @@ internal static List DetectTargetsForLayoutAtProjectRoot( return SkillTargetDetector.DetectTargetsForLayoutStateAtProjectRoot( projectRoot, + installRoot, requireSkillsDirectory: false, groupSkillsUnderUnityCliLoop, includeFreshnessCheck: true, @@ -97,6 +86,7 @@ internal static List DetectTargetsForLayoutAtProjectRoot( internal static List DetectTargetsForLayoutFastAtProjectRoot( string projectRoot, + string installRoot, bool groupSkillsUnderUnityCliLoop) { Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty"); @@ -105,61 +95,23 @@ internal static List DetectTargetsForLayoutFastAtProjectRoot( // cannot change its result. return SkillTargetDetector.DetectTargetsForLayoutStateAtProjectRoot( projectRoot, + installRoot, requireSkillsDirectory: false, groupSkillsUnderUnityCliLoop, includeFreshnessCheck: false, System.Array.Empty()); } - internal static async Task InstallSkillFilesForToolWithDisabledTools( - string toolName, - bool groupSkillsUnderUnityCliLoop, - string[] disabledTools, - CancellationToken ct) - { - Debug.Assert(!string.IsNullOrEmpty(toolName), "toolName must not be null or empty"); - Debug.Assert(disabledTools != null, "disabledTools must not be null"); - ct.ThrowIfCancellationRequested(); - - string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); - Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty"); - - return await InstallSkillFilesForToolAtProjectRoot( - projectRoot, - toolName, - groupSkillsUnderUnityCliLoop, - disabledTools, - ct); - } - - internal static async Task InstallSkillFiles( - List targets, - bool groupSkillsUnderUnityCliLoop, - string[] disabledTools, - CancellationToken ct) - { - Debug.Assert(targets != null, "targets must not be null"); - Debug.Assert(disabledTools != null, "disabledTools must not be null"); - ct.ThrowIfCancellationRequested(); - string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); - Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty"); - - return await InstallSkillFilesAtProjectRoot( - projectRoot, - targets, - groupSkillsUnderUnityCliLoop, - disabledTools, - ct); - } - internal static async Task InstallSkillFilesAtProjectRoot( string projectRoot, + string installRoot, IEnumerable targets, bool groupSkillsUnderUnityCliLoop, string[] disabledTools, CancellationToken ct) { Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty"); + Debug.Assert(!string.IsNullOrEmpty(installRoot), "installRoot must not be null or empty"); Debug.Assert(targets != null, "targets must not be null"); Debug.Assert(disabledTools != null, "disabledTools must not be null"); ct.ThrowIfCancellationRequested(); @@ -182,7 +134,7 @@ internal static async Task InstallSkillFilesAtProjectRoot( { ct.ThrowIfCancellationRequested(); SkillTargetInstaller.InstallSkillsForTarget( - projectRoot, + installRoot, target, disabledSkills, enabledSkills, @@ -197,6 +149,7 @@ internal static async Task InstallSkillFilesAtProjectRoot( internal static async Task InstallSkillFilesForToolAtProjectRoot( string projectRoot, + string installRoot, string toolName, bool groupSkillsUnderUnityCliLoop, string[] disabledTools, @@ -212,7 +165,7 @@ internal static async Task InstallSkillFilesForToolAtProject return new SkillInstallResult(0, 0); } - SkillTargetInfo[] targetArray = SkillTargetDetector.DetectTargetsWithSkillsDirectory(projectRoot).ToArray(); + SkillTargetInfo[] targetArray = SkillTargetDetector.DetectTargetsWithSkillsDirectory(installRoot).ToArray(); return await Task.Run(() => { List allSkills = SkillInstallLayout.GetSkillSourceInfos(projectRoot); @@ -234,7 +187,7 @@ internal static async Task InstallSkillFilesForToolAtProject { ct.ThrowIfCancellationRequested(); SkillTargetInstaller.InstallSpecificSkillsForTarget( - projectRoot, + installRoot, target, disabledSkills, toolSkills, @@ -247,38 +200,38 @@ internal static async Task InstallSkillFilesForToolAtProject }, ct); } - internal static SkillInstallState GetV3MigrationSkillInstallStateAtProjectRoot( - string projectRoot, + internal static SkillInstallState GetV3MigrationSkillInstallStateAtInstallRoot( + string installRoot, SkillTargetInfo target, bool groupSkillsUnderUnityCliLoop) { - return V3MigrationSkillInstaller.GetV3MigrationSkillInstallStateAtProjectRoot( - projectRoot, + return V3MigrationSkillInstaller.GetV3MigrationSkillInstallStateAtInstallRoot( + installRoot, target, groupSkillsUnderUnityCliLoop); } - internal static async Task InstallV3MigrationSkillFilesAtProjectRoot( - string projectRoot, + internal static async Task InstallV3MigrationSkillFilesAtInstallRoot( + string installRoot, IEnumerable targets, bool groupSkillsUnderUnityCliLoop, CancellationToken ct) { - return await V3MigrationSkillInstaller.InstallV3MigrationSkillFilesAtProjectRoot( - projectRoot, + return await V3MigrationSkillInstaller.InstallV3MigrationSkillFilesAtInstallRoot( + installRoot, targets, groupSkillsUnderUnityCliLoop, ct); } - internal static async Task RemoveV3MigrationSkillFilesAtProjectRoot( - string projectRoot, + internal static async Task RemoveV3MigrationSkillFilesAtInstallRoot( + string installRoot, IEnumerable targets, bool groupSkillsUnderUnityCliLoop, CancellationToken ct) { - return await V3MigrationSkillInstaller.RemoveV3MigrationSkillFilesAtProjectRoot( - projectRoot, + return await V3MigrationSkillInstaller.RemoveV3MigrationSkillFilesAtInstallRoot( + installRoot, targets, groupSkillsUnderUnityCliLoop, ct); diff --git a/Packages/src/Editor/Infrastructure/SkillSetup/V3MigrationSkillInstaller.cs b/Packages/src/Editor/Infrastructure/SkillSetup/V3MigrationSkillInstaller.cs index ad839f2dbe..87dfd76158 100644 --- a/Packages/src/Editor/Infrastructure/SkillSetup/V3MigrationSkillInstaller.cs +++ b/Packages/src/Editor/Infrastructure/SkillSetup/V3MigrationSkillInstaller.cs @@ -23,14 +23,14 @@ internal static class V3MigrationSkillInstaller CliConstants.V3_CLI_INVOCATION_MIGRATION_SKILL_NAME, "Skill"); - internal static SkillInstallState GetV3MigrationSkillInstallStateAtProjectRoot( - string projectRoot, + internal static SkillInstallState GetV3MigrationSkillInstallStateAtInstallRoot( + string installRoot, ToolSkillSynchronizer.SkillTargetInfo target, bool groupSkillsUnderUnityCliLoop) { SkillInstallLayout.SkillSourceInfo skill = GetV3MigrationSkillSourceInfo(); - SkillInstallState preferredLayoutState = GetSkillInstallStateAtProjectRoot( - projectRoot, + SkillInstallState preferredLayoutState = GetSkillInstallStateAtInstallRoot( + installRoot, target, skill, groupSkillsUnderUnityCliLoop); @@ -39,35 +39,35 @@ internal static SkillInstallState GetV3MigrationSkillInstallStateAtProjectRoot( return preferredLayoutState; } - return GetSkillInstallStateAtProjectRoot( - projectRoot, + return GetSkillInstallStateAtInstallRoot( + installRoot, target, skill, !groupSkillsUnderUnityCliLoop); } - internal static async Task InstallV3MigrationSkillFilesAtProjectRoot( - string projectRoot, + internal static async Task InstallV3MigrationSkillFilesAtInstallRoot( + string installRoot, IEnumerable targets, bool groupSkillsUnderUnityCliLoop, CancellationToken ct) { SkillInstallLayout.SkillSourceInfo skill = GetV3MigrationSkillSourceInfo(); - return await InstallSpecificSkillFilesAtProjectRoot( - projectRoot, + return await InstallSpecificSkillFilesAtInstallRoot( + installRoot, targets, skill, groupSkillsUnderUnityCliLoop, ct); } - internal static async Task RemoveV3MigrationSkillFilesAtProjectRoot( - string projectRoot, + internal static async Task RemoveV3MigrationSkillFilesAtInstallRoot( + string installRoot, IEnumerable targets, bool groupSkillsUnderUnityCliLoop, CancellationToken ct) { - Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty"); + Debug.Assert(!string.IsNullOrEmpty(installRoot), "installRoot must not be null or empty"); Debug.Assert(targets != null, "targets must not be null"); ct.ThrowIfCancellationRequested(); @@ -76,7 +76,7 @@ internal static SkillInstallState GetV3MigrationSkillInstallStateAtProjectRoot( targets, (ToolSkillSynchronizer.SkillTargetInfo target, CancellationToken targetCt) => { - string targetRoot = Path.Combine(projectRoot, target.DirName); + string targetRoot = Path.Combine(installRoot, target.DirName); SkillTargetInstaller.DeleteSkillDirectoryIfExists( targetRoot, skill.Name, @@ -91,16 +91,16 @@ internal static SkillInstallState GetV3MigrationSkillInstallStateAtProjectRoot( ct); } - internal static SkillInstallState GetSkillInstallStateAtProjectRoot( - string projectRoot, + internal static SkillInstallState GetSkillInstallStateAtInstallRoot( + string installRoot, ToolSkillSynchronizer.SkillTargetInfo target, SkillInstallLayout.SkillSourceInfo skill, bool groupSkillsUnderUnityCliLoop) { - Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty"); + Debug.Assert(!string.IsNullOrEmpty(installRoot), "installRoot must not be null or empty"); Debug.Assert(!string.IsNullOrEmpty(target.DirName), "target dir name must not be null or empty"); - string targetRoot = Path.Combine(projectRoot, target.DirName); + string targetRoot = Path.Combine(installRoot, target.DirName); if (!Directory.Exists(targetRoot)) { return SkillInstallState.Missing; @@ -112,14 +112,14 @@ internal static SkillInstallState GetSkillInstallStateAtProjectRoot( groupSkillsUnderUnityCliLoop); } - internal static async Task InstallSpecificSkillFilesAtProjectRoot( - string projectRoot, + internal static async Task InstallSpecificSkillFilesAtInstallRoot( + string installRoot, IEnumerable targets, SkillInstallLayout.SkillSourceInfo skill, bool groupSkillsUnderUnityCliLoop, CancellationToken ct) { - Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty"); + Debug.Assert(!string.IsNullOrEmpty(installRoot), "installRoot must not be null or empty"); Debug.Assert(targets != null, "targets must not be null"); ct.ThrowIfCancellationRequested(); @@ -128,7 +128,7 @@ internal static SkillInstallState GetSkillInstallStateAtProjectRoot( (ToolSkillSynchronizer.SkillTargetInfo target, CancellationToken targetCt) => { SkillTargetInstaller.InstallSpecificSkillsForTarget( - projectRoot, + installRoot, target, Array.Empty(), new[] { skill }, @@ -138,14 +138,14 @@ internal static SkillInstallState GetSkillInstallStateAtProjectRoot( ct); } - internal static async Task RemoveSpecificSkillFilesAtProjectRoot( - string projectRoot, + internal static async Task RemoveSpecificSkillFilesAtInstallRoot( + string installRoot, IEnumerable targets, string skillName, bool groupSkillsUnderUnityCliLoop, CancellationToken ct) { - Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty"); + Debug.Assert(!string.IsNullOrEmpty(installRoot), "installRoot must not be null or empty"); Debug.Assert(targets != null, "targets must not be null"); Debug.Assert(!string.IsNullOrEmpty(skillName), "skillName must not be null or empty"); ct.ThrowIfCancellationRequested(); @@ -154,7 +154,7 @@ internal static SkillInstallState GetSkillInstallStateAtProjectRoot( targets, (ToolSkillSynchronizer.SkillTargetInfo target, CancellationToken targetCt) => { - string targetRoot = Path.Combine(projectRoot, target.DirName); + string targetRoot = Path.Combine(installRoot, target.DirName); SkillTargetInstaller.DeleteSkillDirectoryIfExists( targetRoot, skillName, From 1ba5b192c16d364e7a5e43584ee6449e922dd101 Mon Sep 17 00:00:00 2001 From: hatayama Date: Sun, 25 Oct 2026 22:30:02 +0900 Subject: [PATCH 3/9] Let Settings and Setup Wizard choose the skill install location Users need to pick between the Unity project root and the Git repository root in the GUI, and the installed-skills status must be checked at the same place the skills are installed. - Add a Location picker to the shared skills panel, so both Settings and Setup Wizard save the choice and rescan the status list there - Show the resolved directory under the picker; when no Git repository contains the project the picker is disabled unless the unavailable Git root is the saved choice, so the user can switch back - Installing into an unavailable Git root shows a dialog instead of letting the install throw - The Settings presenter rereads the saved choice on every refresh so a choice made in the Setup Wizard shows up without reopening --- Assets/Tests/Editor/CliSetupSectionTests.cs | 5 +- .../SetupWizardWorkflowControllersTests.cs | 67 +++++++++- .../Editor/SkillsInstallRootTextTests.cs | 54 ++++++++ .../Editor/SkillsInstallRootTextTests.cs.meta | 11 ++ .../SkillsTargetSelectionResolverTests.cs | 3 +- ...iLoopSettingsCliSetupPresenterFlowTests.cs | 3 +- ...liLoopSettingsSkillsPresenterStateTests.cs | 116 +++++++++++++++++- .../SetupWizardSkillsWorkflowController.cs | 25 ++++ .../Shared/SkillsInstallRootText.cs | 34 +++++ .../Shared/SkillsInstallRootText.cs.meta | 11 ++ .../Shared/SkillsInstallRootView.cs | 84 +++++++++++++ .../Shared/SkillsInstallRootView.cs.meta | 11 ++ .../Presentation/Shared/SkillsSetupPanel.uss | 16 +++ .../Presentation/Shared/SkillsSetupPanel.uxml | 5 + .../Shared/SkillsSetupPanelView.cs | 9 ++ .../UIToolkit/Components/CliSetupSection.cs | 5 + .../UIToolkit/UnityCliLoopSettingsWindowUI.cs | 2 + .../UnityCliLoopSettingsCliSetupPresenter.cs | 15 ++- .../UnityCliLoopSettingsSkillsPresenter.cs | 55 ++++++++- .../UnityCliLoopSettingsWindow.cs | 1 + .../UnityCliLoopSettingsWindowViewData.cs | 5 +- 21 files changed, 516 insertions(+), 21 deletions(-) create mode 100644 Assets/Tests/Editor/SkillsInstallRootTextTests.cs create mode 100644 Assets/Tests/Editor/SkillsInstallRootTextTests.cs.meta create mode 100644 Packages/src/Editor/Presentation/Shared/SkillsInstallRootText.cs create mode 100644 Packages/src/Editor/Presentation/Shared/SkillsInstallRootText.cs.meta create mode 100644 Packages/src/Editor/Presentation/Shared/SkillsInstallRootView.cs create mode 100644 Packages/src/Editor/Presentation/Shared/SkillsInstallRootView.cs.meta diff --git a/Assets/Tests/Editor/CliSetupSectionTests.cs b/Assets/Tests/Editor/CliSetupSectionTests.cs index bcbc0d8096..46c03bf3a7 100644 --- a/Assets/Tests/Editor/CliSetupSectionTests.cs +++ b/Assets/Tests/Editor/CliSetupSectionTests.cs @@ -589,6 +589,8 @@ private static VisualElement CreateRootElement() root.Add(new Button { name = "refresh-skills-state-button" }); VisualElement skillsSubsection = new() { name = "skills-subsection" }; VisualElement skillsSetupPanel = new() { name = "skills-setup-panel" }; + skillsSetupPanel.Add(new EnumField { name = "skills-install-root-field" }); + skillsSetupPanel.Add(new Label { name = "skills-install-root-location" }); skillsSetupPanel.Add(new VisualElement { name = "skill-target-status-list" }); skillsSetupPanel.Add(new VisualElement { name = "skill-target-status-divider" }); skillsSetupPanel.Add(new Label { name = "skill-target-status-summary" }); @@ -642,7 +644,8 @@ private static CliSetupData CreateData( SkillsTarget.Claude, groupSkillsUnderUnityCliLoop: false, isInstallingSkills: false, - installableSkillTargets: installableSkillTargets ?? new List()); + installableSkillTargets: installableSkillTargets ?? new List(), + skillInstallRootSelection: default); } } } diff --git a/Assets/Tests/Editor/SetupWizardWorkflowControllersTests.cs b/Assets/Tests/Editor/SetupWizardWorkflowControllersTests.cs index 10fc7f06fb..eb33579179 100644 --- a/Assets/Tests/Editor/SetupWizardWorkflowControllersTests.cs +++ b/Assets/Tests/Editor/SetupWizardWorkflowControllersTests.cs @@ -433,6 +433,52 @@ public void SkillsRefreshSection_WhenCancelledDuringTheFullScan_KeepsTheFastScan Assert.That(_root.Q("skill-target-status-list").childCount, Is.EqualTo(1)); } + /// + /// Verifies a bulk install into an unavailable Git repository root explains why and installs nothing. + /// + [Test] + public async Task SkillsHandleInstallSkillsAsync_WhenTheGitRootIsUnavailable_ExplainsAndInstallsNothing() + { + _cliDetector.CliVersion = "3.1.0"; + _skillPort.InstallRootSelection = new SkillInstallRootSelection( + SkillInstallRoot.GitRepositoryRoot, + "/unity", + null); + _skillPort.FullTargets = new List + { + CreateTarget("Claude Code", ".claude", SkillInstallState.Missing) + }; + SetupWizardSkillsWorkflowController controller = CreateSkillsWorkflow(); + + await controller.HandleInstallSkillsAsync(isBulkInstall: true, CancellationToken.None); + + Assert.That(_skillPort.InstalledTargetDirs, Is.Empty); + Assert.That(_dialogs.MessageTitles, Is.EqualTo(new List { "No Git Repository" })); + } + + /// + /// Verifies choosing an install location in the wizard saves it and shows where skills go. + /// + [Test] + public void SkillsHandleInstallRootChanged_SavesTheRootAndShowsTheLocation() + { + _cliDetector.CliVersion = "3.1.0"; + _skillPort.InstallRootSelection = new SkillInstallRootSelection( + SkillInstallRoot.UnityProjectRoot, + "/unity", + ""); + SetupWizardSkillsWorkflowController controller = CreateSkillsWorkflow(); + + controller.HandleInstallRootChanged(SkillInstallRoot.GitRepositoryRoot); + + Assert.That( + _skillPort.SavedInstallRoots, + Is.EqualTo(new List { SkillInstallRoot.GitRepositoryRoot })); + Assert.That( + _root.Q