From 2e130cc60d2a9753e900ea3b9f3fb788588a71c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C3=ADas=20D=2E?= Date: Tue, 16 Jun 2026 00:51:29 -0300 Subject: [PATCH 1/3] fix(verify): use scoped targetDir for Skills component paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ComponentSkills case in componentPathsWithWorkspaceScoped passed homeDir instead of the already-computed targetDir to skills.SkillPathForAgent. When --scope=workspace, the installer writes skills to the workspace directory but post-apply verification checked the global home directory, causing false failures. The install path at run.go:788 already uses targetDir correctly, so this aligns verify with install behavior. Also fixes TestComponentPathsOpenClawSkillsSkipsSDDPhaseSkills which passed an unused workspace dir but expected home paths — OpenClaw always uses workspaceDir when set, so the test now reflects that. Closes #785 --- internal/cli/run.go | 18 +++++----- internal/cli/run_component_paths_test.go | 42 +++++++++++++++++++++--- 2 files changed, 47 insertions(+), 13 deletions(-) diff --git a/internal/cli/run.go b/internal/cli/run.go index dccc32e1d..98f1f28c3 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -1135,16 +1135,16 @@ func componentPathsWithWorkspaceScoped(homeDir, workspaceDir string, scope Insta } } paths = append(paths, sddSubAgentPaths(targetDir, adapter)...) - case model.ComponentSkills: - for _, skillID := range selectedSkillIDs(selection) { - if skills.IsSDDSkill(skillID) { - continue - } - path := skills.SkillPathForAgent(homeDir, adapter, skillID) - if path != "" { - paths = append(paths, path) - } + case model.ComponentSkills: + for _, skillID := range selectedSkillIDs(selection) { + if skills.IsSDDSkill(skillID) { + continue + } + path := skills.SkillPathForAgent(targetDir, adapter, skillID) + if path != "" { + paths = append(paths, path) } + } case model.ComponentContext7: switch adapter.MCPStrategy() { case model.StrategySeparateMCPFiles: diff --git a/internal/cli/run_component_paths_test.go b/internal/cli/run_component_paths_test.go index 49456e25d..bb0126229 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,38 @@ 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) + } + } +} + func TestComponentPathsSDDKimiIncludesAgentFilesAndGlobalSkills(t *testing.T) { home := t.TempDir() adapters := resolveAdapters([]model.AgentID{model.AgentKimi}) From df9623910c4e2210d9c498cdbd1d1a7df249590c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C3=ADas=20D=2E?= Date: Wed, 17 Jun 2026 10:16:59 -0300 Subject: [PATCH 2/3] test(verify): add workspace-scope verification regression test for #785 Verifies that post-apply verification checks workspace paths (not home paths) when --scope=workspace is used and no global skill files exist. Also applies gofmt to internal/cli/run.go. Addresses review feedback from @Alan-TheGentleman --- internal/cli/run.go | 18 +++++----- internal/cli/run_component_paths_test.go | 42 ++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 9 deletions(-) diff --git a/internal/cli/run.go b/internal/cli/run.go index 98f1f28c3..d0e9d5702 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -1135,16 +1135,16 @@ func componentPathsWithWorkspaceScoped(homeDir, workspaceDir string, scope Insta } } paths = append(paths, sddSubAgentPaths(targetDir, adapter)...) - case model.ComponentSkills: - for _, skillID := range selectedSkillIDs(selection) { - if skills.IsSDDSkill(skillID) { - continue - } - path := skills.SkillPathForAgent(targetDir, adapter, skillID) - if path != "" { - paths = append(paths, path) + case model.ComponentSkills: + for _, skillID := range selectedSkillIDs(selection) { + if skills.IsSDDSkill(skillID) { + continue + } + path := skills.SkillPathForAgent(targetDir, adapter, skillID) + if path != "" { + paths = append(paths, path) + } } - } case model.ComponentContext7: switch adapter.MCPStrategy() { case model.StrategySeparateMCPFiles: diff --git a/internal/cli/run_component_paths_test.go b/internal/cli/run_component_paths_test.go index bb0126229..05fed129a 100644 --- a/internal/cli/run_component_paths_test.go +++ b/internal/cli/run_component_paths_test.go @@ -257,6 +257,48 @@ func TestComponentPathsWorkspaceScopedSkillsUsesWorkspaceDir(t *testing.T) { } } +// 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}) From f41b95629fd1aafed5c2dad7f53050f31cb95bb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C3=ADas=20D=2E?= Date: Tue, 23 Jun 2026 01:04:43 -0300 Subject: [PATCH 3/3] test(cli): add integration test for install --scope=workspace verification --- internal/cli/run_integration_test.go | 66 ++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) 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) + } +}