Fix: Phase 0: Add --skill infrastructure (#473)

* Fix: Phase 0: Add --skill infrastructure

* Apply changes from Holon

* Apply changes from Holon

* Delete pkg/builtin/skills/github/solve/SKILL.md

* Apply changes from Holon

---------

Co-authored-by: holonbot[bot] <250454749+holonbot[bot]@users.noreply.github.com>
Co-authored-by: jolestar <jolestar@gmail.com>

Author: holonbot[bot]

.gitignore

@@ -37,3 +37,7 @@ dist
 
 # Exception: allow .claude in agents/claude (needed for agent development)
 !agents/claude/.claude/
+
+# Builtin skills (generated copy from repository-level skills/)
+# Run 'go generate ./pkg/builtin' to regenerate
+pkg/builtin/skills/

Makefile

@@ -21,12 +21,16 @@ build: build-host
 ## build-host: Build runner CLI for current OS/Arch with version info
 build-host:
 	@echo "Building runner CLI (Version: $(VERSION), Commit: $(COMMIT))..."
+	@echo "Generating builtin skills..."
+	@go generate ./pkg/builtin
 	@mkdir -p $(BIN_DIR)
 	go build $(LDFLAGS) -o $(BIN_DIR)/$(BINARY_NAME) ./cmd/holon
 
 ## release-build: Build binaries for multiple platforms
 release-build:
 	@echo "Building release binaries..."
+	@echo "Generating builtin skills..."
+	@go generate ./pkg/builtin
 	@mkdir -p $(BIN_DIR)
 	@echo "Building for linux/amd64..."
 	GOOS=linux GOARCH=amd64 go build $(LDFLAGS) -o $(BIN_DIR)/$(BINARY_NAME)-linux-amd64 ./cmd/holon
@@ -42,6 +46,8 @@ test: test-go
 ## test-go: Run Go tests with structured output
 test-go:
 	@echo "Running Go tests..."
+	@echo "Generating builtin skills..."
+	@go generate ./pkg/builtin
 	@if command -v gotestfmt > /dev/null 2>&1; then \
 		go test ./... -json -v 2>&1 | gotestfmt; \
 	else \
@@ -52,11 +58,15 @@ test-go:
 ## test-raw: Run Go tests without gotestfmt (plain output)
 test-raw:
 	@echo "Running Go tests with plain output..."
+	@echo "Generating builtin skills..."
+	@go generate ./pkg/builtin
 	go test ./... -v
 
 ## test-unit: Run unit tests (non-integration) with structured output
 test-unit:
 	@echo "Running unit tests..."
+	@echo "Generating builtin skills..."
+	@go generate ./pkg/builtin
 	@if command -v gotestfmt > /dev/null 2>&1; then \
 		go test $$(go list ./... | grep -v '^github.com/holon-run/holon/tests/') -short -json -v 2>&1 | gotestfmt; \
 	else \
@@ -67,6 +77,8 @@ test-unit:
 ## test-unit-raw: Run unit tests (non-integration) without gotestfmt
 test-unit-raw:
 	@echo "Running unit tests with plain output..."
+	@echo "Generating builtin skills..."
+	@go generate ./pkg/builtin
 	go test $$(go list ./... | grep -v '^github.com/holon-run/holon/tests/') -short -v
 
 ## test-agent: Run agent TypeScript tests

cmd/holon/main.go

@@ -220,6 +220,21 @@ var runCmd = &cobra.Command{
 	RunE: func(cmd *cobra.Command, args []string) error {
 		ctx := context.Background()
 
+		// Validate --skill/--skills and --mode mutual exclusivity
+		// Parse CLI skills
+		cliSkills := skillPaths
+		for _, s := range skills.ParseSkillsList(skillsList) {
+			cliSkills = append(cliSkills, s)
+		}
+		// Check if --skill/--skills was explicitly provided
+		hasSkills := cmd.Flags().Changed("skill") || cmd.Flags().Changed("skills")
+		// Check if --mode was explicitly provided (not default)
+		hasMode := cmd.Flags().Changed("mode")
+		if hasSkills && hasMode {
+			return fmt.Errorf("--skill/--skills and --mode flags are mutually exclusive (both were provided)")
+		}
+		// Note: If only one of --mode or --skill/--skills is provided, it will control the behavior
+
 		// Resolve workspace path early for auto-detection
 		absWorkspace, err := filepath.Abs(workspacePath)
 		if err != nil {

pkg/builtin/embed.go

@@ -0,0 +1,142 @@
+// Package builtin provides embedded builtin skills for Holon.
+// Builtin skills are embedded in the binary using Go's embed package
+// and can be loaded by reference (e.g., "github/solve").
+//
+//go:generate sh -c "rm -rf skills && cp -r ../../skills ."
+package builtin
+
+import (
+	"embed"
+	"errors"
+	"io/fs"
+	"path/filepath"
+)
+
+//go:embed skills/*
+var builtinSkills embed.FS
+
+// FS returns the embedded skills filesystem, rooted at the skills directory.
+// The returned FS provides access to all builtin skills.
+func FS() fs.FS {
+	// Create a subfilesystem rooted at the skills directory
+	sub, err := fs.Sub(builtinSkills, "skills")
+	if err != nil {
+		// This should never happen if the embed path is correct
+		// Return a nil filesystem so callers can detect unavailability
+		return nil
+	}
+	return sub
+}
+
+// Has checks if a builtin skill exists at the given reference.
+// The reference is a slash-separated path like "github/solve".
+// Returns true if the skill directory exists and contains SKILL.md.
+func Has(ref string) bool {
+	f := FS()
+	if f == nil {
+		return false
+	}
+
+	// Check if SKILL.md exists in the skill directory
+	skillManifestPath := filepath.Join(ref, "SKILL.md")
+	_, err := fs.Stat(f, skillManifestPath)
+	return err == nil
+}
+
+// Load reads the SKILL.md file for a builtin skill.
+// The reference is a slash-separated path like "github/solve".
+// Returns the contents of SKILL.md or an error if the skill doesn't exist.
+func Load(ref string) ([]byte, error) {
+	f := FS()
+	if f == nil {
+		return nil, errors.New("builtin skills filesystem not available")
+	}
+
+	skillManifestPath := filepath.Join(ref, "SKILL.md")
+	return fs.ReadFile(f, skillManifestPath)
+}
+
+// LoadDir reads the entire skill directory for a builtin skill.
+// Returns a map of filename to content for all files in the skill directory.
+func LoadDir(ref string) (map[string][]byte, error) {
+	f := FS()
+	if f == nil {
+		return nil, errors.New("builtin skills filesystem not available")
+	}
+
+	files := make(map[string][]byte)
+
+	// Walk the skill directory and read all files
+	err := fs.WalkDir(f, ref, func(path string, d fs.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+
+		// Skip directories
+		if d.IsDir() {
+			return nil
+		}
+
+		// Read the file
+		content, err := fs.ReadFile(f, path)
+		if err != nil {
+			return err
+		}
+
+		// Store with relative path from skill root
+		relPath := path
+		if ref != "." {
+			// Ensure path is long enough to avoid slicing panic
+			if len(path) > len(ref)+1 {
+				relPath = path[len(ref)+1:]
+			} else {
+				relPath = filepath.Base(path)
+			}
+		}
+		files[relPath] = content
+
+		return nil
+	})
+
+	if err != nil {
+		return nil, err
+	}
+
+	return files, nil
+}
+
+// List returns all builtin skill references.
+// A skill reference is returned if the directory contains a SKILL.md file.
+func List() ([]string, error) {
+	f := FS()
+	if f == nil {
+		return nil, errors.New("builtin skills filesystem not available")
+	}
+
+	var skills []string
+
+	// Walk the entire filesystem
+	err := fs.WalkDir(f, ".", func(path string, d fs.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+
+		// Check if this is a SKILL.md file
+		if !d.IsDir() && d.Name() == "SKILL.md" {
+			// Get the parent directory as the skill reference
+			skillRef := filepath.Dir(path)
+			if skillRef == "." {
+				skillRef = ""
+			}
+			skills = append(skills, skillRef)
+		}
+
+		return nil
+	})
+
+	if err != nil {
+		return nil, err
+	}
+
+	return skills, nil
+}