test(scaffold): add frontmatter spec gate for skills and agents

CHANGELOG.md

@@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/), and this
 
 ### Added
 
+- **Frontmatter spec gate for skills & agents** ([#55](https://github.com/All-The-Vibes/ATV-StarterKit/pull/55)). New `pkg/scaffold/skillspec_test.go` structural regression test that validates the frontmatter of every shipped Copilot definition under `.github/skills`, `.github/agents`, and `pkg/scaffold/templates/skills` stays well-formed and complete — standalone `---` delimiters, required keys (`name`/`description` for skills, `description`/`user-invocable` for agents), `name == folder` and unique names for skills, no duplicate keys, non-placeholder descriptions, and a non-empty body — so definitions cannot silently drift out of spec as the set grows. Runs under the existing `go test ./...` job; no new CI wiring, dependencies, or product-code changes. Calibrated to current conventions (green on `main`): no description length cap, no required sections, no naming style beyond `name == folder`. `pkg/scaffold/templates/agents` is intentionally excluded for now (those files are stored single-line/collapsed and are not standard line-delimited YAML frontmatter — flagged as follow-up).
 - **`docs/atv-security.md` — full user reference for `/atv-security`** ([#49](https://github.com/All-The-Vibes/ATV-StarterKit/pull/49)). New top-level documentation surface for the security skill: all 33 config rules described individually across 5 categories (Secrets, Permissions, Hooks, MCP, Agents & Skills), OWASP Top 10 (2021) coverage matrix, STRIDE mapping, scoring formula and A–F grading scale, sample report layout, `fix`-mode semantics (which rules auto-remediate vs advise-only), persistence behaviour, and the v2.5.9 `/cso` → `/atv-security` heritage block. Linked from `README.md` and the `DOCS.md` command index. Closes the gap where the canonical 517-line `SKILL.md` was only readable post-install. No code changes.
 - **README `/atv-security` section expanded** — now lists the three surfaces (33 config rules, OWASP A01-A10, STRIDE 6-class), the two-tier detection model, persistence behaviour, and concrete `fix`-mode examples.
 

pkg/scaffold/skillspec_test.go

@@ -0,0 +1,340 @@
+package scaffold
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// skillspec_test.go is a structural regression guard for ATV's Copilot skill
+// and agent definitions. It asserts that the frontmatter on every shipped
+// definition stays well-formed and complete, so the repo's growing set of
+// definitions cannot silently drift out of the shape the Copilot harness
+// expects (a missing closing `---`, an empty `description`, a `name` that no
+// longer matches its folder, a duplicated key, etc.).
+//
+// It is deliberately MINIMAL and calibrated to ATV's existing conventions so
+// it passes green on the current tree. It intentionally does NOT enforce
+// stylistic policy that ATV does not already follow — there is no description
+// length cap, no required body-section list, no naming-style rule beyond the
+// `name == folder` invariant the skills already satisfy, and no banned-word
+// list. Its only job is to catch accidental structural breakage.
+//
+// Scope: the canonical dogfood definitions under `.github/skills` and
+// `.github/agents`, plus the consumer-facing templates under
+// `pkg/scaffold/templates/skills` and `pkg/scaffold/templates/agents`. The
+// agent templates were previously stored in a single-line minified form and
+// were excluded; once that corruption was repaired they are now held to the
+// same structural contract as every other shipped definition.
+
+// parsedFrontmatter holds the top-level frontmatter keys and the document body
+// that follow a strict `---` / `---` delimited block.
+type parsedFrontmatter struct {
+	keys map[string]string
+	body string
+}
+
+// parseFrontmatter parses a strict, line-delimited YAML-style frontmatter block
+// from raw markdown. Line endings are normalized first so CRLF files (`.github`)
+// and LF files (templates) are treated identically. It returns a non-empty
+// problem string describing the first structural defect found.
+//
+// Only unindented `key: value` lines are treated as frontmatter keys; indented
+// lines and list items (continuations of a value) are ignored so that inline
+// list values do not register spurious keys.
+func parseFrontmatter(raw string) (*parsedFrontmatter, string) {
+	s := strings.ReplaceAll(raw, "\r\n", "\n")
+	s = strings.ReplaceAll(s, "\r", "\n")
+
+	if !strings.HasPrefix(s, "---\n") {
+		return nil, "missing opening '---' delimiter on the first line"
+	}
+	rest := s[len("---\n"):]
+	lines := strings.Split(rest, "\n")
+
+	closing := -1
+	for i, line := range lines {
+		if line == "---" {
+			closing = i
+			break
+		}
+	}
+	if closing == -1 {
+		return nil, "missing closing '---' delimiter on its own line"
+	}
+
+	keys := make(map[string]string)
+	for _, line := range lines[:closing] {
+		if strings.TrimSpace(line) == "" {
+			continue
+		}
+		// Skip indented lines and list items: these are value continuations,
+		// not top-level keys.
+		if line[0] == ' ' || line[0] == '\t' || line[0] == '-' {
+			continue
+		}
+		idx := strings.Index(line, ":")
+		if idx <= 0 {
+			continue
+		}
+		key := strings.TrimSpace(line[:idx])
+		if !isFrontmatterKey(key) {
+			continue
+		}
+		if _, dup := keys[key]; dup {
+			return nil, "duplicate frontmatter key '" + key + "'"
+		}
+		keys[key] = strings.TrimSpace(line[idx+1:])
+	}
+
+	body := ""
+	if closing+1 < len(lines) {
+		body = strings.Join(lines[closing+1:], "\n")
+	}
+	return &parsedFrontmatter{keys: keys, body: body}, ""
+}
+
+// isFrontmatterKey reports whether s is a plausible top-level YAML key
+// (letters, digits, hyphen, underscore only).
+func isFrontmatterKey(s string) bool {
+	if s == "" {
+		return false
+	}
+	for _, r := range s {
+		switch {
+		case r >= 'a' && r <= 'z':
+		case r >= 'A' && r <= 'Z':
+		case r >= '0' && r <= '9':
+		case r == '-' || r == '_':
+		default:
+			return false
+		}
+	}
+	return true
+}
+
+// isPlaceholder reports whether a frontmatter value is an obvious unfilled
+// placeholder rather than real content.
+func isPlaceholder(v string) bool {
+	switch strings.ToLower(strings.TrimSpace(v)) {
+	case "todo", "tbd", "fixme", "changeme", "change me", "description":
+		return true
+	}
+	return false
+}
+
+// TestParseFrontmatterRejectsMalformed proves the gate is not vacuous: each
+// class of structural defect it is meant to catch is actually rejected. This
+// guards the validator itself against silently degrading into a no-op.
+func TestParseFrontmatterRejectsMalformed(t *testing.T) {
+	cases := []struct {
+		name    string
+		raw     string
+		wantErr bool
+	}{
+		{
+			name:    "valid",
+			raw:     "---\nname: foo\ndescription: A real description.\n---\nbody text\n",
+			wantErr: false,
+		},
+		{
+			name:    "missing opening delimiter (single-line minified)",
+			raw:     "---description: x user-invocable: true---body",
+			wantErr: true,
+		},
+		{
+			name:    "no opening delimiter at all",
+			raw:     "name: foo\ndescription: x\nbody",
+			wantErr: true,
+		},
+		{
+			name:    "missing closing delimiter",
+			raw:     "---\nname: foo\ndescription: x\nbody without close",
+			wantErr: true,
+		},
+		{
+			name:    "duplicate key",
+			raw:     "---\nname: foo\nname: bar\ndescription: x\n---\nbody",
+			wantErr: true,
+		},
+		{
+			name:    "crlf is normalized and parses",
+			raw:     "---\r\nname: foo\r\ndescription: A real description.\r\n---\r\nbody\r\n",
+			wantErr: false,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			fm, problem := parseFrontmatter(tc.raw)
+			if tc.wantErr {
+				if problem == "" {
+					t.Fatalf("expected a structural problem, got none (parsed %+v)", fm)
+				}
+				return
+			}
+			if problem != "" {
+				t.Fatalf("unexpected problem: %s", problem)
+			}
+			// A normalized value must not carry a trailing CR.
+			if got := fm.keys["name"]; strings.ContainsRune(got, '\r') {
+				t.Fatalf("name value retained a carriage return: %q", got)
+			}
+		})
+	}
+}
+
+// TestFrontmatterValueChecks covers the small value-level predicates.
+func TestFrontmatterValueChecks(t *testing.T) {
+	for _, v := range []string{"TODO", "tbd", "  FIXME ", "description", "change me"} {
+		if !isPlaceholder(v) {
+			t.Errorf("expected %q to be detected as a placeholder", v)
+		}
+	}
+	for _, v := range []string{"A real, specific description.", "Run linting before push."} {
+		if isPlaceholder(v) {
+			t.Errorf("did not expect %q to be a placeholder", v)
+		}
+	}
+
+	for _, k := range []string{"name", "user-invocable", "allowed_tools", "argument-hint"} {
+		if !isFrontmatterKey(k) {
+			t.Errorf("expected %q to be a valid key", k)
+		}
+	}
+	for _, k := range []string{"", "- Bash(x)", "a key", "key:sub"} {
+		if isFrontmatterKey(k) {
+			t.Errorf("did not expect %q to be a valid key", k)
+		}
+	}
+}
+
+// TestSkillFrontmatterSpec validates the frontmatter of every shipped skill in
+// the canonical dogfood tree and the consumer-facing template tree.
+func TestSkillFrontmatterSpec(t *testing.T) {
+	root := repoRoot(t)
+
+	trees := []string{
+		filepath.Join(".github", "skills"),
+		filepath.Join("pkg", "scaffold", "templates", "skills"),
+	}
+
+	for _, tree := range trees {
+		treeDir := filepath.Join(root, tree)
+		entries, err := os.ReadDir(treeDir)
+		if err != nil {
+			t.Fatalf("reading skill tree %s: %v", tree, err)
+		}
+
+		seen := make(map[string]string) // name -> folder that first claimed it
+		for _, entry := range entries {
+			if !entry.IsDir() {
+				continue
+			}
+			folder := entry.Name()
+			skillPath := filepath.Join(treeDir, folder, "SKILL.md")
+			rel := filepath.ToSlash(filepath.Join(tree, folder, "SKILL.md"))
+
+			raw, err := os.ReadFile(skillPath)
+			if err != nil {
+				t.Errorf("%s: skill directory %q has no readable SKILL.md: %v", tree, folder, err)
+				continue
+			}
+
+			fm, problem := parseFrontmatter(string(raw))
+			if problem != "" {
+				t.Errorf("%s: %s", rel, problem)
+				continue
+			}
+
+			name, ok := fm.keys["name"]
+			if !ok || strings.TrimSpace(name) == "" {
+				t.Errorf("%s: missing or empty required frontmatter key 'name'", rel)
+			} else {
+				if name != folder {
+					t.Errorf("%s: name %q does not match its folder %q", rel, name, folder)
+				}
+				if prev, dup := seen[name]; dup {
+					t.Errorf("%s: duplicate skill name %q (also defined by %q)", rel, name, prev)
+				} else {
+					seen[name] = folder
+				}
+			}
+
+			desc := strings.TrimSpace(fm.keys["description"])
+			if desc == "" {
+				t.Errorf("%s: missing or empty required frontmatter key 'description'", rel)
+			} else if isPlaceholder(desc) {
+				t.Errorf("%s: description is an unfilled placeholder %q", rel, desc)
+			}
+
+			if strings.TrimSpace(fm.body) == "" {
+				t.Errorf("%s: body after frontmatter is empty", rel)
+			}
+		}
+	}
+}
+
+// TestAgentFrontmatterSpec validates the frontmatter of every agent definition
+// in the canonical dogfood tree (.github/agents) and the consumer-facing
+// template tree (pkg/scaffold/templates/agents).
+func TestAgentFrontmatterSpec(t *testing.T) {
+	root := repoRoot(t)
+
+	trees := []string{
+		filepath.Join(".github", "agents"),
+		filepath.Join("pkg", "scaffold", "templates", "agents"),
+	}
+
+	for _, tree := range trees {
+		agentsDir := filepath.Join(root, tree)
+		entries, err := os.ReadDir(agentsDir)
+		if err != nil {
+			t.Fatalf("reading agents tree %s: %v", tree, err)
+		}
+
+		count := 0
+		for _, entry := range entries {
+			if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".agent.md") {
+				continue
+			}
+			count++
+			rel := filepath.ToSlash(filepath.Join(tree, entry.Name()))
+
+			raw, err := os.ReadFile(filepath.Join(agentsDir, entry.Name()))
+			if err != nil {
+				t.Errorf("%s: unreadable: %v", rel, err)
+				continue
+			}
+
+			fm, problem := parseFrontmatter(string(raw))
+			if problem != "" {
+				t.Errorf("%s: %s", rel, problem)
+				continue
+			}
+
+			desc := strings.TrimSpace(fm.keys["description"])
+			if desc == "" {
+				t.Errorf("%s: missing or empty required frontmatter key 'description'", rel)
+			} else if isPlaceholder(desc) {
+				t.Errorf("%s: description is an unfilled placeholder %q", rel, desc)
+			}
+
+			ui, ok := fm.keys["user-invocable"]
+			if !ok {
+				t.Errorf("%s: missing required frontmatter key 'user-invocable'", rel)
+			} else if ui != "true" && ui != "false" {
+				t.Errorf("%s: user-invocable must be 'true' or 'false', got %q", rel, ui)
+			}
+
+			if strings.TrimSpace(fm.body) == "" {
+				t.Errorf("%s: body after frontmatter is empty", rel)
+			}
+		}
+
+		if count == 0 {
+			t.Fatalf("no .agent.md definitions found under %s — path drift?", tree)
+		}
+	}
+}