diff --git a/internal/cli/run.go b/internal/cli/run.go index dccc32e1d..d0e9d5702 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -1140,7 +1140,7 @@ func componentPathsWithWorkspaceScoped(homeDir, workspaceDir string, scope Insta if skills.IsSDDSkill(skillID) { continue } - path := skills.SkillPathForAgent(homeDir, adapter, skillID) + path := skills.SkillPathForAgent(targetDir, adapter, skillID) if path != "" { paths = append(paths, path) } diff --git a/internal/cli/run_component_paths_test.go b/internal/cli/run_component_paths_test.go index 49456e25d..05fed129a 100644 --- a/internal/cli/run_component_paths_test.go +++ b/internal/cli/run_component_paths_test.go @@ -197,6 +197,7 @@ func TestComponentPathsWithWorkspaceOpenClawSDDUsesWorkspaceScopedSkills(t *test func TestComponentPathsOpenClawSkillsSkipsSDDPhaseSkills(t *testing.T) { home := t.TempDir() + workspace := t.TempDir() adapters := resolveAdapters([]model.AgentID{model.AgentOpenClaw}) selection := model.Selection{ Skills: []model.SkillID{ @@ -206,16 +207,17 @@ func TestComponentPathsOpenClawSkillsSkipsSDDPhaseSkills(t *testing.T) { }, } - paths := componentPathsWithWorkspace(home, t.TempDir(), selection, adapters, model.ComponentSkills) + // OpenClaw always uses workspaceDir when set, independent of scope. + paths := componentPathsWithWorkspace(home, workspace, selection, adapters, model.ComponentSkills) - want := filepath.Join(home, ".openclaw", "skills", "go-testing", "SKILL.md") + want := filepath.Join(workspace, ".openclaw", "skills", "go-testing", "SKILL.md") if !containsPath(paths, want) { t.Fatalf("componentPaths(skills,openclaw) missing portable skill path %q\npaths=%v", want, paths) } for _, unwanted := range []string{ - filepath.Join(home, ".openclaw", "skills", "sdd-init", "SKILL.md"), - filepath.Join(home, ".openclaw", "skills", "sdd-onboard", "SKILL.md"), + filepath.Join(workspace, ".openclaw", "skills", "sdd-init", "SKILL.md"), + filepath.Join(workspace, ".openclaw", "skills", "sdd-onboard", "SKILL.md"), } { if containsPath(paths, unwanted) { t.Fatalf("componentPaths(skills,openclaw) must not verify SDD phase skill path %q\npaths=%v", unwanted, paths) @@ -223,6 +225,80 @@ func TestComponentPathsOpenClawSkillsSkipsSDDPhaseSkills(t *testing.T) { } } +func TestComponentPathsWorkspaceScopedSkillsUsesWorkspaceDir(t *testing.T) { + home := t.TempDir() + workspace := t.TempDir() + adapters := resolveAdapters([]model.AgentID{model.AgentClaudeCode}) + selection := model.Selection{ + Skills: []model.SkillID{ + model.SkillGoTesting, + model.SkillBranchPR, + }, + } + + paths := componentPathsWithWorkspaceScoped(home, workspace, ScopeWorkspace, selection, adapters, model.ComponentSkills) + + for _, want := range []string{ + filepath.Join(workspace, ".claude", "skills", "go-testing", "SKILL.md"), + filepath.Join(workspace, ".claude", "skills", "branch-pr", "SKILL.md"), + } { + if !containsPath(paths, want) { + t.Fatalf("componentPathsWithWorkspaceScoped(skills,claude-code,workspace) missing workspace-scoped path %q\npaths=%v", want, paths) + } + } + + for _, unwanted := range []string{ + filepath.Join(home, ".claude", "skills", "go-testing", "SKILL.md"), + filepath.Join(home, ".claude", "skills", "branch-pr", "SKILL.md"), + } { + if containsPath(paths, unwanted) { + t.Fatalf("componentPathsWithWorkspaceScoped(skills,claude-code,workspace) must not include home-scoped path %q\npaths=%v", unwanted, paths) + } + } +} + +// TestInstallWorkspaceScopeVerificationWithNoGlobalSkills verifies that +// post-apply verification succeeds when --scope=workspace is used and no +// global skill files exist. This is a regression test for issue #785: +// the verifier used to check home-scoped paths even when workspace scope +// was active, causing false failures when only workspace skills existed. +func TestInstallWorkspaceScopeVerificationWithNoGlobalSkills(t *testing.T) { + home := t.TempDir() + workspace := t.TempDir() + adapters := resolveAdapters([]model.AgentID{model.AgentClaudeCode}) + selection := model.Selection{ + Skills: []model.SkillID{ + model.SkillGoTesting, + model.SkillBranchPR, + }, + } + + // Simulate workspace-scoped install: skills are written to workspace only. + // The verification should check workspace paths, not home paths. + paths := componentPathsWithWorkspaceScoped(home, workspace, ScopeWorkspace, selection, adapters, model.ComponentSkills) + + // Verify that workspace paths are included (these should exist after install). + for _, want := range []string{ + filepath.Join(workspace, ".claude", "skills", "go-testing", "SKILL.md"), + filepath.Join(workspace, ".claude", "skills", "branch-pr", "SKILL.md"), + } { + if !containsPath(paths, want) { + t.Fatalf("workspace-scoped verification missing workspace path %q\npaths=%v", want, paths) + } + } + + // Verify that home paths are NOT included (these would cause false failures + // if checked when only workspace skills exist). + for _, unwanted := range []string{ + filepath.Join(home, ".claude", "skills", "go-testing", "SKILL.md"), + filepath.Join(home, ".claude", "skills", "branch-pr", "SKILL.md"), + } { + if containsPath(paths, unwanted) { + t.Fatalf("workspace-scoped verification must not check home path %q when scope=workspace\npaths=%v", unwanted, paths) + } + } +} + func TestComponentPathsSDDKimiIncludesAgentFilesAndGlobalSkills(t *testing.T) { home := t.TempDir() adapters := resolveAdapters([]model.AgentID{model.AgentKimi}) diff --git a/internal/cli/run_integration_test.go b/internal/cli/run_integration_test.go index fc25d923d..2032c194e 100644 --- a/internal/cli/run_integration_test.go +++ b/internal/cli/run_integration_test.go @@ -2217,3 +2217,69 @@ func TestRunInstallKimiAlreadyInstalledDoesNotRequireUV(t *testing.T) { t.Fatalf("expected no install commands when Kimi is already installed, got: %v", got) } } + +// TestRunInstallWorkspaceScopeVerification verifies the user-visible 'install --scope=workspace' +// behavior from issue #785. It ensures that when installing with workspace scope: +// 1. Verification files are written to the workspace directory, NOT the home directory. +// 2. Post-apply verification succeeds because it checks the workspace skill paths. +func TestRunInstallWorkspaceScopeVerification(t *testing.T) { + home := t.TempDir() + workspace := t.TempDir() + + originalCwd, err := os.Getwd() + if err != nil { + t.Fatalf("failed to get current wd: %v", err) + } + + restoreHome := osUserHomeDir + restoreCommand := runCommand + restoreLookPath := cmdLookPath + t.Cleanup(func() { + osUserHomeDir = restoreHome + runCommand = restoreCommand + cmdLookPath = restoreLookPath + if err := os.Chdir(originalCwd); err != nil { + t.Errorf("failed to restore working directory: %v", err) + } + }) + + osUserHomeDir = func() (string, error) { return home, nil } + runCommand = func(string, ...string) error { return nil } + cmdLookPath = func(name string) (string, error) { + return "/usr/local/bin/" + name, nil + } + + if err := os.Chdir(workspace); err != nil { + t.Fatalf("failed to change working directory to temp workspace: %v", err) + } + + // Run install with workspace scope, installing Claude Code agent and skills component + args := []string{ + "--scope", "workspace", + "--agent", "claude-code", + "--component", "skills", + "--preset", "custom", + "--skill", "go-testing,branch-pr", + } + + result, err := RunInstall(args, system.DetectionResult{}) + if err != nil { + t.Fatalf("RunInstall() error = %v", err) + } + + if !result.Verify.Ready { + t.Fatalf("post-apply verification failed, report = %#v", result.Verify) + } + + // Assert that skill files were written to the workspace directory. + expectedWorkspaceSkillFile := filepath.Join(workspace, ".claude", "skills", "go-testing", "SKILL.md") + if _, err := os.Stat(expectedWorkspaceSkillFile); err != nil { + t.Errorf("expected skill file in workspace %q, but was missing: %v", expectedWorkspaceSkillFile, err) + } + + // Assert that no skill files were written to the home directory. + unexpectedHomeSkillFile := filepath.Join(home, ".claude", "skills", "go-testing", "SKILL.md") + if _, err := os.Stat(unexpectedHomeSkillFile); err == nil { + t.Errorf("unexpected skill file found in home directory: %q", unexpectedHomeSkillFile) + } +}