From 83f6b1c64cac8313b443265357f7c61cfcbb09c3 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 6 Oct 2026 22:29:56 +0900 Subject: [PATCH 1/3] Add failing tests for skills install run without a target flag The help text promises that targets already holding uloop skills are refreshed even when their flag is omitted, but install without any target flag prints the target guidance and exits before detection runs. The first test pins the promised refresh and fails today; the second pins that guidance is still printed when no target holds a uloop skill. --- .../dispatcher/skills_dispatch_test.go | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/cli/dispatcher/internal/dispatcher/skills_dispatch_test.go b/cli/dispatcher/internal/dispatcher/skills_dispatch_test.go index 88ffab8bc0..918d6d4dd8 100644 --- a/cli/dispatcher/internal/dispatcher/skills_dispatch_test.go +++ b/cli/dispatcher/internal/dispatcher/skills_dispatch_test.go @@ -85,6 +85,72 @@ func TestTryHandleSkillsRequestPrintsTargetGuidanceWithoutTargets(t *testing.T) } } +func TestRunSkillsSubcommandInstallWithoutTargetRefreshesDetectedInstall(t *testing.T) { + // Verifies install without a target flag refreshes a target that already holds a uloop skill instead of printing guidance. + root := t.TempDir() + skill := writeDirModeSkillSource(t, root, "uloop-sample") + skills := []skillDefinition{skill} + claudeOptions := skillCommandOptions{targets: []skillTarget{targetConfigs["claude"]}} + var setupStderr bytes.Buffer + if code := runSkillsSubcommand("install", root, skills, claudeOptions, &bytes.Buffer{}, &setupStderr); code != 0 { + t.Fatalf("initial install failed: code=%d stderr=%s", code, setupStderr.String()) + } + baseDir, err := getSkillsBaseDir(root, targetConfigs["claude"], claudeOptions.global) + if err != nil { + t.Fatalf("failed to resolve the skills base dir: %v", err) + } + installedSkillFile := filepath.Join(getPreferredSkillDir(baseDir, skill.name, groupManagedSkillsForOptions(claudeOptions)), "SKILL.md") + writeDispatcherTestFile(t, installedSkillFile, "stale") + sourceContent, err := os.ReadFile(filepath.Join(skill.sourceDirectory, "SKILL.md")) + if err != nil { + t.Fatalf("failed to read the skill source: %v", err) + } + var stdout bytes.Buffer + var stderr bytes.Buffer + + code := runSkillsSubcommand("install", root, skills, skillCommandOptions{}, &stdout, &stderr) + + if code != 0 { + t.Fatalf("install without a target failed: code=%d stdout=%s stderr=%s", code, stdout.String(), stderr.String()) + } + if !strings.Contains(stdout.String(), "Auto-refreshing") || strings.Contains(stdout.String(), "Please specify at least one target") { + t.Fatalf("install without a target must refresh the detected install instead of printing guidance:\n%s", stdout.String()) + } + assertFileContent(t, installedSkillFile, string(sourceContent)) +} + +func TestRunSkillsSubcommandInstallWithoutTargetAndNoInstallPrintsGuidance(t *testing.T) { + // Verifies install without a target flag still prints guidance and writes no skill when no target holds a uloop skill. + root := t.TempDir() + skill := writeDirModeSkillSource(t, root, "uloop-sample") + var stdout bytes.Buffer + var stderr bytes.Buffer + + code := runSkillsSubcommand("install", root, []skillDefinition{skill}, skillCommandOptions{}, &stdout, &stderr) + + if code != 0 { + t.Fatalf("install without a target failed: code=%d stdout=%s stderr=%s", code, stdout.String(), stderr.String()) + } + if !strings.Contains(stdout.String(), "Please specify at least one target for 'install'") || strings.Contains(stdout.String(), "Auto-refreshing") { + t.Fatalf("install without a target must only print guidance when nothing is installed:\n%s", stdout.String()) + } + walkErr := filepath.WalkDir(root, func(path string, entry os.DirEntry, err error) error { + if err != nil { + return err + } + if entry.IsDir() && path == skill.sourceDirectory { + return filepath.SkipDir + } + if !entry.IsDir() && entry.Name() == "SKILL.md" { + t.Errorf("install without a target must not write a skill file: %s", path) + } + return nil + }) + if walkErr != nil { + t.Fatalf("failed to walk the project root: %v", walkErr) + } +} + func TestTryHandleSkillsRequestInstallsIntoOutputDir(t *testing.T) { // Verifies --output-dir routes the request to dir mode and installs and removes the skill there. projectRoot := createSkillsTestProject(t) From 6847f038e4c4546b86069bc7052d97ef8e79bcb7 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 6 Oct 2026 22:30:50 +0900 Subject: [PATCH 2/3] Refresh existing skill installs when install is run without a target flag Install without any target flag now runs the same installed-target detection that a flagged install uses for auto-refresh: when a target already holds a uloop skill it is refreshed, and the target guidance is printed only when no target holds one. This makes the command do what its help text already promises. A detection error is reported with code 1 in the same form as the flagged install, and a test pins it. --- .../internal/dispatcher/skills_dispatch.go | 13 +++++++++++-- .../internal/dispatcher/skills_dispatch_test.go | 14 ++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/cli/dispatcher/internal/dispatcher/skills_dispatch.go b/cli/dispatcher/internal/dispatcher/skills_dispatch.go index 0220937aa0..607bcd6190 100644 --- a/cli/dispatcher/internal/dispatcher/skills_dispatch.go +++ b/cli/dispatcher/internal/dispatcher/skills_dispatch.go @@ -209,9 +209,18 @@ func runSkillsInstallWithGuidance( stdout io.Writer, stderr io.Writer, ) int { + // The help text promises that targets already holding uloop skills never go stale, even + // when their flag is omitted, so only ask where to install when no target holds one yet. if len(options.targets) == 0 { - printSkillsTargetGuidance("install", stdout) - return 0 + detected, err := detectInstalledSkillTargets(projectRoot, skills, options) + if err != nil { + clierrors.WriteClassifiedError(stderr, err, clierrors.ErrorContext{ProjectRoot: projectRoot, Command: clicore.SkillsCommandName}) + return 1 + } + if len(detected) == 0 { + printSkillsTargetGuidance("install", stdout) + return 0 + } } return runSkillsInstall(projectRoot, skills, options, stdout, stderr) } diff --git a/cli/dispatcher/internal/dispatcher/skills_dispatch_test.go b/cli/dispatcher/internal/dispatcher/skills_dispatch_test.go index 918d6d4dd8..49e78de79d 100644 --- a/cli/dispatcher/internal/dispatcher/skills_dispatch_test.go +++ b/cli/dispatcher/internal/dispatcher/skills_dispatch_test.go @@ -151,6 +151,20 @@ func TestRunSkillsSubcommandInstallWithoutTargetAndNoInstallPrintsGuidance(t *te } } +func TestRunSkillsSubcommandInstallWithoutTargetReportsDetectionErrors(t *testing.T) { + // Verifies install without a target flag fails with code 1 instead of printing guidance when installed targets cannot be detected. + stubSkillsUserHomeDir(t, "", errors.New("home unavailable")) + skill := skillDefinition{name: "uloop-sample", content: []byte(sampleSkillContent)} + var stdout bytes.Buffer + var stderr bytes.Buffer + + code := runSkillsSubcommand("install", t.TempDir(), []skillDefinition{skill}, skillCommandOptions{global: true}, &stdout, &stderr) + + if code != 1 || !strings.Contains(stderr.String(), "home unavailable") || strings.Contains(stdout.String(), "Please specify at least one target") { + t.Fatalf("expected the detection error: code=%d stdout=%s stderr=%s", code, stdout.String(), stderr.String()) + } +} + func TestTryHandleSkillsRequestInstallsIntoOutputDir(t *testing.T) { // Verifies --output-dir routes the request to dir mode and installs and removes the skill there. projectRoot := createSkillsTestProject(t) From 4e3eb7dde4c70cc9532f67e7d8eeeb1fd02ff550 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 6 Oct 2026 22:53:58 +0900 Subject: [PATCH 3/3] Narrow the guidance test comment to projects with no installed skills Install without a target flag now refreshes targets that already hold uloop skills, so printing only guidance holds just when none does. --- cli/dispatcher/internal/dispatcher/skills_dispatch_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/dispatcher/internal/dispatcher/skills_dispatch_test.go b/cli/dispatcher/internal/dispatcher/skills_dispatch_test.go index 49e78de79d..ab018e2d25 100644 --- a/cli/dispatcher/internal/dispatcher/skills_dispatch_test.go +++ b/cli/dispatcher/internal/dispatcher/skills_dispatch_test.go @@ -72,7 +72,7 @@ func TestTryHandleSkillsRequestInstallListUninstallRoundTrip(t *testing.T) { } func TestTryHandleSkillsRequestPrintsTargetGuidanceWithoutTargets(t *testing.T) { - // Verifies install and uninstall without target flags only print guidance and change nothing. + // Verifies install and uninstall without target flags only print guidance and change nothing when no target holds a uloop skill yet. projectRoot := createSkillsTestProject(t) for _, subcommand := range []string{"install", "uninstall"} { code, stdout, stderr := runSkillsRequestForTest(t, projectRoot, subcommand)