Add E2E tests for skills overwrite protection and CLI

test/e2e/api_skills_test.go

@@ -677,6 +677,86 @@ var _ = Describe("Skills API", Label("api", "skills", "e2e"), func() {
 		})
 	})
 
+	Describe("Overwrite protection", func() {
+		It("should reject install over existing skill without force", func() {
+			skillName := "overwrite-noflag"
+
+			By("Installing the skill for the first time")
+			resp := installSkill(apiServer, installSkillRequest{Name: skillName})
+			defer resp.Body.Close()
+			Expect(resp.StatusCode).To(Equal(http.StatusCreated))
+
+			By("Uninstalling via API so the DB record is gone but leave the concept of a conflict test")
+			// Instead we test duplicate detection: installing the same name again
+			// should return 409 Conflict (the DB record still exists).
+			resp2 := installSkill(apiServer, installSkillRequest{Name: skillName})
+			defer resp2.Body.Close()
+
+			By("Verifying response status is 409 Conflict")
+			Expect(resp2.StatusCode).To(Equal(http.StatusConflict))
+		})
+
+		It("should allow reinstall after uninstall", func() {
+			skillName := "overwrite-reinstall"
+
+			By("Installing the skill")
+			r1 := installSkill(apiServer, installSkillRequest{Name: skillName})
+			defer r1.Body.Close()
+			Expect(r1.StatusCode).To(Equal(http.StatusCreated))
+
+			By("Uninstalling the skill")
+			r2 := uninstallSkill(apiServer, skillName)
+			defer r2.Body.Close()
+			Expect(r2.StatusCode).To(Equal(http.StatusNoContent))
+
+			By("Re-installing the skill (should succeed since DB record was removed)")
+			r3 := installSkill(apiServer, installSkillRequest{Name: skillName})
+			defer r3.Body.Close()
+			Expect(r3.StatusCode).To(Equal(http.StatusCreated))
+		})
+
+		It("should still reject duplicate DB record even with force flag", func() {
+			skillName := "overwrite-force-dup"
+
+			By("Installing the skill for the first time")
+			r1 := installSkill(apiServer, installSkillRequest{Name: skillName})
+			defer r1.Body.Close()
+			Expect(r1.StatusCode).To(Equal(http.StatusCreated))
+
+			By("Force-installing the same skill again (force is for filesystem conflicts, not DB duplicates)")
+			r2 := installSkill(apiServer, installSkillRequest{Name: skillName, Force: true})
+			defer r2.Body.Close()
+
+			By("Verifying response is still 409 Conflict (DB record exists)")
+			Expect(r2.StatusCode).To(Equal(http.StatusConflict))
+		})
+	})
+
+	Describe("Build and validate lifecycle", func() {
+		It("should build, then validate, the same skill directory", func() {
+			skillName := "build-validate-lifecycle"
+
+			By("Creating a valid skill directory")
+			skillDir := createTestSkillDir(skillName, "A skill for build-validate lifecycle")
+
+			By("Validating the skill")
+			vResp := validateSkill(apiServer, skillDir)
+			defer vResp.Body.Close()
+			Expect(vResp.StatusCode).To(Equal(http.StatusOK))
+			var vResult validationResultResponse
+			Expect(json.NewDecoder(vResp.Body).Decode(&vResult)).To(Succeed())
+			Expect(vResult.Valid).To(BeTrue())
+
+			By("Building the skill")
+			bResp := buildSkill(apiServer, skillDir, "v0.1.0")
+			defer bResp.Body.Close()
+			Expect(bResp.StatusCode).To(Equal(http.StatusOK))
+			var bResult buildResultResponse
+			Expect(json.NewDecoder(bResp.Body).Decode(&bResult)).To(Succeed())
+			Expect(bResult.Reference).ToNot(BeEmpty())
+		})
+	})
+
 	Describe("Full lifecycle integration", func() {
 		It("should support install → list → info → uninstall → list → info", func() {
 			skillName := "lifecycle-test"

test/e2e/cli_skills_test.go

@@ -0,0 +1,189 @@
+// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package e2e_test
+
+import (
+	"encoding/json"
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+
+	"github.com/stacklok/toolhive/test/e2e"
+)
+
+var _ = Describe("Skills CLI", Label("api", "cli", "skills", "e2e"), func() {
+	var (
+		config    *e2e.ServerConfig
+		apiServer *e2e.Server
+		thvConfig *e2e.TestConfig
+	)
+
+	BeforeEach(func() {
+		config = e2e.NewServerConfig()
+		apiServer = e2e.StartServer(config)
+		thvConfig = e2e.NewTestConfig()
+	})
+
+	// thvSkillCmd creates a THVCommand for `thv skill <args>` with
+	// TOOLHIVE_API_URL pointing to the test server.
+	thvSkillCmd := func(args ...string) *e2e.THVCommand {
+		fullArgs := append([]string{"skill"}, args...)
+		return e2e.NewTHVCommand(thvConfig, fullArgs...).
+			WithEnv("TOOLHIVE_API_URL=" + apiServer.BaseURL())
+	}
+
+	Describe("thv skill validate", func() {
+		It("should succeed for a valid skill directory", func() {
+			skillDir := createTestSkillDir("cli-valid-skill", "A valid skill for CLI testing")
+
+			stdout, _ := thvSkillCmd("validate", skillDir).ExpectSuccess()
+			// Text output should not contain "Error:" lines for a valid skill
+			Expect(stdout).ToNot(ContainSubstring("Error:"))
+		})
+
+		It("should succeed with JSON output", func() {
+			skillDir := createTestSkillDir("cli-valid-json", "A valid skill for JSON output")
+
+			stdout, _ := thvSkillCmd("validate", "--format", "json", skillDir).ExpectSuccess()
+
+			var result validationResultResponse
+			Expect(json.Unmarshal([]byte(stdout), &result)).To(Succeed())
+			Expect(result.Valid).To(BeTrue())
+		})
+
+		It("should fail for an invalid skill directory", func() {
+			emptyDir := GinkgoT().TempDir()
+
+			_, _, err := thvSkillCmd("validate", emptyDir).Run()
+			Expect(err).To(HaveOccurred(), "validate should fail for directory without SKILL.md")
+		})
+	})
+
+	Describe("thv skill build", func() {
+		It("should build a valid skill and print the reference", func() {
+			skillDir := createTestSkillDir("cli-build-skill", "A skill for CLI build testing")
+
+			stdout, _ := thvSkillCmd("build", skillDir).ExpectSuccess()
+			// The build command should output something (the reference)
+			Expect(strings.TrimSpace(stdout)).ToNot(BeEmpty())
+		})
+	})
+
+	Describe("thv skill install and list", func() {
+		It("should install a skill and list it", func() {
+			skillName := fmt.Sprintf("cli-install-%d", GinkgoRandomSeed())
+
+			By("Installing the skill")
+			thvSkillCmd("install", skillName).ExpectSuccess()
+
+			By("Listing skills in text format — should show the installed skill")
+			stdout, _ := thvSkillCmd("list").ExpectSuccess()
+			Expect(stdout).To(ContainSubstring(skillName))
+
+			By("Listing skills in JSON format")
+			jsonOut, _ := thvSkillCmd("list", "--format", "json").ExpectSuccess()
+			var skills []json.RawMessage
+			Expect(json.Unmarshal([]byte(jsonOut), &skills)).To(Succeed())
+			Expect(skills).ToNot(BeEmpty())
+		})
+	})
+
+	Describe("thv skill info", func() {
+		It("should show info for an installed skill", func() {
+			skillName := fmt.Sprintf("cli-info-%d", GinkgoRandomSeed())
+
+			By("Installing the skill")
+			thvSkillCmd("install", skillName).ExpectSuccess()
+
+			By("Getting info in text format")
+			stdout, _ := thvSkillCmd("info", skillName).ExpectSuccess()
+			Expect(stdout).To(ContainSubstring(skillName))
+
+			By("Getting info in JSON format")
+			jsonOut, _ := thvSkillCmd("info", "--format", "json", skillName).ExpectSuccess()
+			Expect(jsonOut).To(ContainSubstring(skillName))
+		})
+
+		It("should fail for a non-existent skill", func() {
+			_, _, err := thvSkillCmd("info", "no-such-skill-xyz").Run()
+			Expect(err).To(HaveOccurred())
+		})
+	})
+
+	Describe("thv skill uninstall", func() {
+		It("should uninstall an installed skill", func() {
+			skillName := fmt.Sprintf("cli-uninstall-%d", GinkgoRandomSeed())
+
+			By("Installing the skill")
+			thvSkillCmd("install", skillName).ExpectSuccess()
+
+			By("Uninstalling the skill")
+			thvSkillCmd("uninstall", skillName).ExpectSuccess()
+
+			By("Verifying the skill is no longer listed")
+			stdout, _ := thvSkillCmd("list").ExpectSuccess()
+			Expect(stdout).ToNot(ContainSubstring(skillName))
+		})
+
+		It("should fail for a non-existent skill", func() {
+			_, _, err := thvSkillCmd("uninstall", "no-such-skill-xyz").Run()
+			Expect(err).To(HaveOccurred())
+		})
+	})
+
+	Describe("CLI full lifecycle", func() {
+		It("should support validate → build → install → list → info → uninstall → list", func() {
+			skillName := fmt.Sprintf("cli-lifecycle-%d", GinkgoRandomSeed())
+
+			By("Creating a valid skill directory")
+			parentDir := GinkgoT().TempDir()
+			skillDir := filepath.Join(parentDir, skillName)
+			Expect(os.MkdirAll(skillDir, 0o755)).To(Succeed())
+
+			skillMD := fmt.Sprintf(`---
+name: %s
+description: Full lifecycle CLI test
+version: 1.0.0
+---
+
+# %s
+
+A test skill for the full CLI lifecycle.
+`, skillName, skillName)
+			Expect(os.WriteFile(
+				filepath.Join(skillDir, "SKILL.md"),
+				[]byte(skillMD),
+				0o644,
+			)).To(Succeed())
+
+			By("Validating the skill")
+			thvSkillCmd("validate", skillDir).ExpectSuccess()
+
+			By("Building the skill")
+			thvSkillCmd("build", skillDir).ExpectSuccess()
+
+			By("Installing the skill by name (pending)")
+			thvSkillCmd("install", skillName).ExpectSuccess()
+
+			By("Listing skills — should contain the skill")
+			listOut, _ := thvSkillCmd("list").ExpectSuccess()
+			Expect(listOut).To(ContainSubstring(skillName))
+
+			By("Getting skill info")
+			infoOut, _ := thvSkillCmd("info", skillName).ExpectSuccess()
+			Expect(infoOut).To(ContainSubstring(skillName))
+
+			By("Uninstalling the skill")
+			thvSkillCmd("uninstall", skillName).ExpectSuccess()
+
+			By("Listing skills — should no longer contain the skill")
+			listOut2, _ := thvSkillCmd("list").ExpectSuccess()
+			Expect(listOut2).ToNot(ContainSubstring(skillName))
+		})
+	})
+})