refactor: flag string splitting to use 'StringSliceVar'

cmd/check.go

@@ -13,8 +13,8 @@ import (
 )
 
 var (
-	checkOnly                  string
-	checkSkip                  string
+	checkOnly                  []string
+	checkSkip                  []string
 	perFileCheck               bool
 	checkSkipOrphans           bool
 	strictCheck                bool
@@ -32,8 +32,8 @@ var checkCmd = &cobra.Command{
 }
 
 func init() {
-	checkCmd.Flags().StringVar(&checkOnly, "only", "", "comma-separated list of check groups to run: structure,links,content,contamination")
-	checkCmd.Flags().StringVar(&checkSkip, "skip", "", "comma-separated list of check groups to skip: structure,links,content,contamination")
+	checkCmd.Flags().StringSliceVar(&checkOnly, "only", nil, "check groups to run: structure,links,content,contamination (comma-separated or repeatable)")
+	checkCmd.Flags().StringSliceVar(&checkSkip, "skip", nil, "check groups to skip: structure,links,content,contamination (comma-separated or repeatable)")
 	checkCmd.Flags().BoolVar(&perFileCheck, "per-file", false, "show per-file reference analysis")
 	checkCmd.Flags().BoolVar(&checkSkipOrphans, "skip-orphans", false,
 		"skip orphan file detection (unreferenced files in scripts/, references/, assets/)")
@@ -55,7 +55,7 @@ var validGroups = map[orchestrate.CheckGroup]bool{
 }
 
 func runCheck(cmd *cobra.Command, args []string) error {
-	if checkOnly != "" && checkSkip != "" {
+	if len(checkOnly) > 0 && len(checkSkip) > 0 {
 		return fmt.Errorf("--only and --skip are mutually exclusive")
 	}
 
@@ -98,15 +98,15 @@ func runCheck(cmd *cobra.Command, args []string) error {
 	return nil
 }
 
-func resolveCheckGroups(only, skip string) (map[orchestrate.CheckGroup]bool, error) {
+func resolveCheckGroups(only, skip []string) (map[orchestrate.CheckGroup]bool, error) {
 	enabled := orchestrate.AllGroups()
 
-	if only != "" {
+	if len(only) > 0 {
 		// Reset all to false, enable only specified
 		for k := range enabled {
 			enabled[k] = false
 		}
-		for g := range strings.SplitSeq(only, ",") {
+		for _, g := range only {
 			g = strings.TrimSpace(g)
 			cg := orchestrate.CheckGroup(g)
 			if !validGroups[cg] {
@@ -116,8 +116,8 @@ func resolveCheckGroups(only, skip string) (map[orchestrate.CheckGroup]bool, err
 		}
 	}
 
-	if skip != "" {
-		for g := range strings.SplitSeq(skip, ",") {
+	if len(skip) > 0 {
+		for _, g := range skip {
 			g = strings.TrimSpace(g)
 			cg := orchestrate.CheckGroup(g)
 			if !validGroups[cg] {

cmd/cmd_test.go

@@ -315,7 +315,7 @@ func TestReadSkillRaw_MissingFile(t *testing.T) {
 
 func TestResolveCheckGroups(t *testing.T) {
 	t.Run("default all enabled", func(t *testing.T) {
-		enabled, err := resolveCheckGroups("", "")
+		enabled, err := resolveCheckGroups(nil, nil)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -330,7 +330,7 @@ func TestResolveCheckGroups(t *testing.T) {
 	})
 
 	t.Run("only structure,links", func(t *testing.T) {
-		enabled, err := resolveCheckGroups("structure,links", "")
+		enabled, err := resolveCheckGroups([]string{"structure", "links"}, nil)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -343,7 +343,7 @@ func TestResolveCheckGroups(t *testing.T) {
 	})
 
 	t.Run("skip contamination", func(t *testing.T) {
-		enabled, err := resolveCheckGroups("", "contamination")
+		enabled, err := resolveCheckGroups(nil, []string{"contamination"})
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -356,7 +356,7 @@ func TestResolveCheckGroups(t *testing.T) {
 	})
 
 	t.Run("invalid group", func(t *testing.T) {
-		_, err := resolveCheckGroups("structure,bogus", "")
+		_, err := resolveCheckGroups([]string{"structure", "bogus"}, nil)
 		if err == nil {
 			t.Error("expected error for invalid group")
 		}

cmd/exitcode_integration_test.go

@@ -5,6 +5,7 @@ import (
 	"os/exec"
 	"path/filepath"
 	"runtime"
+	"strings"
 	"testing"
 )
 
@@ -104,3 +105,93 @@ func TestExitCodes(t *testing.T) {
 		})
 	}
 }
+
+func TestSliceFlags(t *testing.T) {
+	bin := buildBinary(t)
+
+	tests := []struct {
+		name       string
+		args       []string
+		wantCode   int
+		wantStdout string // substring that must appear in combined output
+		noStdout   string // substring that must NOT appear in combined output
+	}{
+		// --only: comma-separated
+		{
+			name:       "only comma-separated runs selected groups",
+			args:       []string{"check", "--only=structure,content", fixture(t, "valid-skill")},
+			wantCode:   0,
+			wantStdout: "SKILL.md found",
+		},
+		// --only: repeated flag
+		{
+			name:       "only repeated flag runs selected groups",
+			args:       []string{"check", "--only=structure", "--only=content", fixture(t, "valid-skill")},
+			wantCode:   0,
+			wantStdout: "SKILL.md found",
+		},
+		// --skip: comma-separated
+		{
+			name:       "skip comma-separated excludes groups",
+			args:       []string{"check", "--skip=links,content,contamination", fixture(t, "valid-skill")},
+			wantCode:   0,
+			wantStdout: "SKILL.md found",
+		},
+		// --skip: repeated flag
+		{
+			name:       "skip repeated flag excludes groups",
+			args:       []string{"check", "--skip=links", "--skip=content", "--skip=contamination", fixture(t, "valid-skill")},
+			wantCode:   0,
+			wantStdout: "SKILL.md found",
+		},
+		// --only and --skip mutual exclusion
+		{
+			name:     "only and skip mutual exclusion",
+			args:     []string{"check", "--only=structure", "--skip=links", fixture(t, "valid-skill")},
+			wantCode: 3,
+		},
+		// --allow-dirs: comma-separated
+		{
+			name:     "allow-dirs comma-separated suppresses warnings",
+			args:     []string{"check", "--only=structure", "--allow-dirs=evals,testing", fixture(t, "allowed-dirs-skill")},
+			wantCode: 0,
+			noStdout: "unknown directory",
+		},
+		// --allow-dirs: repeated flag
+		{
+			name:     "allow-dirs repeated flag suppresses warnings",
+			args:     []string{"check", "--only=structure", "--allow-dirs=evals", "--allow-dirs=testing", fixture(t, "allowed-dirs-skill")},
+			wantCode: 0,
+			noStdout: "unknown directory",
+		},
+		// --allow-dirs: partial (only one of two unknown dirs)
+		{
+			name:       "allow-dirs partial still warns for non-allowed",
+			args:       []string{"check", "--only=structure", "--allow-dirs=evals", fixture(t, "allowed-dirs-skill")},
+			wantCode:   2,
+			wantStdout: "unknown directory: testing/",
+			noStdout:   "unknown directory: evals/",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cmd := exec.Command(bin, tt.args...)
+			out, _ := cmd.CombinedOutput()
+			got := cmd.ProcessState.ExitCode()
+			if got != tt.wantCode {
+				t.Errorf("exit code = %d, want %d (args: %v)\noutput: %s", got, tt.wantCode, tt.args, out)
+			}
+			if tt.wantStdout != "" {
+				if !strings.Contains(string(out), tt.wantStdout) {
+					t.Errorf("expected output to contain %q, got:\n%s", tt.wantStdout, out)
+				}
+			}
+			if tt.noStdout != "" {
+				if strings.Contains(string(out), tt.noStdout) {
+					t.Errorf("expected output NOT to contain %q, got:\n%s", tt.noStdout, out)
+				}
+			}
+		})
+	}
+}