feat: add personal skill resolver (#25362)

> Mux updated this PR on behalf of Mike. ## Stack Context This stack splits experimental personal skills into smaller reviewable PRs. Personal skills are user-owned `SKILL.md` files stored by Coder and injected into chatd alongside workspace skills. Stack order: 1. #25362 personal skill resolver 2. #25363 storage, permissions, API, and SDK 3. #25365 API test coverage 4. #25366 chattool and chatd integration 5. #25066 settings UI and docs 6. #25386 personal skills slash menu ## What? Adds the shared personal skill parser and resolver package, plus reusable skill-name validation exported from `workspacesdk`. The parser enforces the full personal skill contract: max raw size, kebab-case name, max name length, and non-empty body. ## Why? The rest of the stack needs one source-aware resolver for personal and workspace skills, including collision handling and qualified aliases. Keeping personal skill constraints in the parser prevents callers from accidentally parsing invalid personal skills. ## Validation - `go test ./coderd/x/skills ./codersdk/workspacesdk` - pre-commit hooks on this branch
2026-09-24 15:04:27 +08:00 · 2026-05-16 15:33:43 +00:00
parent 191dd230ae
commit 792f0b4902
7 changed files with 615 additions and 11 deletions
@@ -15,7 +15,7 @@ import (
 )

 const (
-	maxSkillMetaBytes = 64 * 1024
+	maxSkillMetaBytes = workspacesdk.MaxSkillMetaBytes
 	maxSkillFileBytes = 512 * 1024
 )

@@ -0,0 +1,52 @@
+// Package skills defines the shared model for personal and workspace skills
+// used by chatd.
+//
+// Glossary:
+//
+//   - Personal skill: A user-owned skill that follows the user across Coder
+//     chats and workspaces, stored by Coder rather than discovered from a
+//     workspace filesystem.
+//   - Workspace skill: A skill discovered from the workspace filesystem,
+//     currently under .agents/skills by default.
+//   - Skill source: The origin of a skill available to chatd, such as personal
+//     storage or workspace filesystem discovery.
+//   - Skill alias: A chat or tool lookup name for a skill. Bare aliases use the
+//     skill name. Qualified aliases use personal/<name> or workspace/<name>.
+//
+// Decision:
+//
+// Personal skills are stored by Coder. For each chat turn, chatd fetches
+// personal skill metadata fresh, combines it with workspace skill metadata, and
+// injects the available skills into the existing skill prompt.
+// When chatd needs skill content, it resolves personal skills through the
+// read_skill flow instead of syncing files into workspace filesystems.
+//
+// If a personal skill and workspace skill share the same kebab-case name, both
+// are exposed with qualified aliases: personal/<name> for the personal skill
+// and workspace/<name> for the workspace skill. One source must not silently
+// override the other.
+//
+// Site admins can read and modify personal skill content. Personal skills are
+// user-authored instructions, not secret material. Audit records can include
+// raw Markdown content diffs alongside the actor, target user, and relevant
+// metadata.
+//
+// Personal skill edits affect the next chat turn. Old chat turns are not exact
+// snapshots of the personal skill state that existed when they ran.
+//
+// The v1 design does not include CLI support, web UI support, supporting files,
+// organization-scoped personal skills, syncing personal skills into workspace
+// filesystems, or stable public API documentation.
+//
+// Consequences:
+//
+// Chatd can use personal and workspace skills through one prompt and one read
+// path, while storage remains owned by Coder instead of individual workspace
+// filesystems. Fresh metadata keeps skill changes responsive, but chat history
+// is less reproducible because old turns do not capture an exact copy of
+// personal skill content.
+//
+// Explicit qualified aliases make ambiguous names visible to users and tools.
+// Admin access improves operability and abuse handling, but it creates a
+// privacy trade-off that must remain clear in product and support expectations.
+package skills
@@ -0,0 +1,219 @@
+package skills
+
+import (
+	"maps"
+	"slices"
+	"strings"
+
+	"golang.org/x/xerrors"
+
+	"github.com/coder/coder/v2/codersdk/workspacesdk"
+)
+
+// MaxPersonalSkillSizeBytes is the maximum raw Markdown size accepted for a
+// personal skill upload.
+const MaxPersonalSkillSizeBytes = workspacesdk.MaxSkillMetaBytes
+
+// MaxPersonalSkillNameBytes is the maximum skill name length accepted for a
+// personal skill upload. Skill names are also used in URL paths.
+const MaxPersonalSkillNameBytes = 256
+
+// Source identifies where a skill came from.
+type Source string
+
+const (
+	// SourcePersonal identifies a user-owned, DB-backed skill.
+	SourcePersonal Source = "personal"
+	// SourceWorkspace identifies a filesystem-discovered workspace skill.
+	SourceWorkspace Source = "workspace"
+)
+
+var (
+	// ErrInvalidSkillName indicates that a skill name is missing, not valid
+	// kebab-case, or exceeds the maximum length.
+	ErrInvalidSkillName = xerrors.New("invalid skill name")
+	// ErrSkillBodyRequired indicates that the skill has no body after frontmatter.
+	ErrSkillBodyRequired = xerrors.New("skill body is required")
+	// ErrSkillTooLarge indicates that the raw skill Markdown is too large.
+	ErrSkillTooLarge = xerrors.New("skill is too large")
+	// ErrSkillNotFound indicates that a skill lookup did not match any alias.
+	ErrSkillNotFound = xerrors.New("skill not found")
+	// ErrSkillAmbiguous indicates that a skill lookup matched multiple sources.
+	ErrSkillAmbiguous = xerrors.New("skill lookup is ambiguous")
+)
+
+// Skill is the source-aware metadata needed to list and resolve a skill.
+type Skill struct {
+	Name        string
+	Description string
+	Source      Source
+}
+
+// ParsedSkill is a parsed skill with the Markdown body after frontmatter.
+// Body has HTML comments stripped and surrounding whitespace trimmed.
+type ParsedSkill struct {
+	Skill
+	Body string
+}
+
+// ResolvedSkill is a skill with the alias exposed to chat tools.
+type ResolvedSkill struct {
+	Skill
+	Alias string
+}
+
+// ParsePersonalSkillMarkdown parses raw personal skill Markdown and enforces
+// the personal skill contract. The raw size must not exceed
+// MaxPersonalSkillSizeBytes, frontmatter must contain a valid kebab-case name,
+// the skill name must not exceed MaxPersonalSkillNameBytes, and the body after
+// frontmatter must be non-empty.
+func ParsePersonalSkillMarkdown(raw []byte) (ParsedSkill, error) {
+	if len(raw) > MaxPersonalSkillSizeBytes {
+		return ParsedSkill{}, xerrors.Errorf(
+			"%w: got %d bytes, maximum is %d bytes",
+			ErrSkillTooLarge,
+			len(raw),
+			MaxPersonalSkillSizeBytes,
+		)
+	}
+
+	name, description, body, err := workspacesdk.ParseSkillFrontmatter(string(raw))
+	if err != nil {
+		if xerrors.Is(err, workspacesdk.ErrFrontmatterNameRequired) {
+			return ParsedSkill{}, xerrors.Errorf("%w: frontmatter must contain a 'name' field", ErrInvalidSkillName)
+		}
+		return ParsedSkill{}, xerrors.Errorf("parse skill frontmatter: %w", err)
+	}
+	if !workspacesdk.SkillNamePattern.MatchString(name) {
+		return ParsedSkill{}, xerrors.Errorf(
+			"%w: %q must match %s",
+			ErrInvalidSkillName,
+			name,
+			workspacesdk.SkillNameRegex,
+		)
+	}
+	nameBytes := len(name)
+	if nameBytes > MaxPersonalSkillNameBytes {
+		return ParsedSkill{}, xerrors.Errorf(
+			"%w: %q is %d bytes, maximum is %d bytes",
+			ErrInvalidSkillName,
+			name,
+			nameBytes,
+			MaxPersonalSkillNameBytes,
+		)
+	}
+	if strings.TrimSpace(body) == "" {
+		return ParsedSkill{}, xerrors.Errorf(
+			"%w: skill %q has no content after frontmatter",
+			ErrSkillBodyRequired,
+			name,
+		)
+	}
+
+	return ParsedSkill{
+		Skill: Skill{
+			Name:        name,
+			Description: description,
+			Source:      SourcePersonal,
+		},
+		Body: body,
+	}, nil
+}
+
+// MergeSkills combines personal and workspace skills into a deterministic list
+// with aliases for chat tool display and lookup. Skill names must already be
+// valid kebab-case names because qualified aliases use / as a separator. If a
+// source contains duplicate names, the first skill for that source wins.
+func MergeSkills(personalSkills, workspaceSkills []Skill) []ResolvedSkill {
+	personalByName := skillsByName(personalSkills, SourcePersonal)
+	workspaceByName := skillsByName(workspaceSkills, SourceWorkspace)
+
+	names := make(map[string]struct{}, len(personalByName)+len(workspaceByName))
+	for name := range personalByName {
+		names[name] = struct{}{}
+	}
+	for name := range workspaceByName {
+		names[name] = struct{}{}
+	}
+
+	resolved := make([]ResolvedSkill, 0, len(personalByName)+len(workspaceByName))
+	for _, name := range slices.Sorted(maps.Keys(names)) {
+		personal, hasPersonal := personalByName[name]
+		workspace, hasWorkspace := workspaceByName[name]
+		if hasPersonal && hasWorkspace {
+			resolved = append(resolved,
+				ResolvedSkill{
+					Skill: personal,
+					Alias: QualifiedAlias(SourcePersonal, name),
+				},
+				ResolvedSkill{
+					Skill: workspace,
+					Alias: QualifiedAlias(SourceWorkspace, name),
+				},
+			)
+			continue
+		}
+		if hasPersonal {
+			resolved = append(resolved, ResolvedSkill{
+				Skill: personal,
+				Alias: name,
+			})
+			continue
+		}
+		resolved = append(resolved, ResolvedSkill{
+			Skill: workspace,
+			Alias: name,
+		})
+	}
+	return resolved
+}
+
+// Lookup finds a resolved skill by bare alias or qualified source alias. It
+// returns ErrSkillNotFound if no alias matches, or ErrSkillAmbiguous if a bare
+// name matches skills from multiple sources.
+func Lookup(resolved []ResolvedSkill, lookup string) (ResolvedSkill, error) {
+	var (
+		bareNameMatch ResolvedSkill
+		matches       []string
+	)
+	for _, skill := range resolved {
+		qualifiedAlias := QualifiedAlias(skill.Source, skill.Name)
+		if lookup == skill.Alias || lookup == qualifiedAlias {
+			return skill, nil
+		}
+		if lookup == skill.Name {
+			bareNameMatch = skill
+			matches = append(matches, qualifiedAlias)
+		}
+	}
+	switch len(matches) {
+	case 0:
+		return ResolvedSkill{}, xerrors.Errorf("%w: %q", ErrSkillNotFound, lookup)
+	case 1:
+		return bareNameMatch, nil
+	default:
+		return ResolvedSkill{}, xerrors.Errorf(
+			"%w: %q matches %s",
+			ErrSkillAmbiguous,
+			lookup,
+			strings.Join(matches, ", "),
+		)
+	}
+}
+
+// QualifiedAlias returns the stable source-qualified alias for a skill name.
+func QualifiedAlias(source Source, name string) string {
+	return string(source) + "/" + name
+}
+
+func skillsByName(skills []Skill, source Source) map[string]Skill {
+	byName := make(map[string]Skill, len(skills))
+	for _, skill := range skills {
+		if _, ok := byName[skill.Name]; ok {
+			continue
+		}
+		skill.Source = source
+		byName[skill.Name] = skill
+	}
+	return byName
+}
@@ -0,0 +1,326 @@
+package skills_test
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/coder/coder/v2/coderd/x/skills"
+)
+
+func TestParsePersonalSkillMarkdown(t *testing.T) {
+	t.Parallel()
+
+	t.Run("ValidWithDescription", func(t *testing.T) {
+		t.Parallel()
+
+		content, err := skills.ParsePersonalSkillMarkdown([]byte(
+			"---\nname: my-skill\ndescription: Does a thing\n---\nUse this skill.\n",
+		))
+
+		require.NoError(t, err)
+		require.Equal(t, "my-skill", content.Name)
+		require.Equal(t, "Does a thing", content.Description)
+		require.Equal(t, skills.SourcePersonal, content.Source)
+		require.Equal(t, "Use this skill.", content.Body)
+	})
+
+	t.Run("ValidWithoutDescription", func(t *testing.T) {
+		t.Parallel()
+
+		content, err := skills.ParsePersonalSkillMarkdown([]byte(
+			"---\nname: my-skill\n---\nUse this skill.\n",
+		))
+
+		require.NoError(t, err)
+		require.Equal(t, "my-skill", content.Name)
+		require.Empty(t, content.Description)
+		require.Equal(t, skills.SourcePersonal, content.Source)
+		require.Equal(t, "Use this skill.", content.Body)
+	})
+
+	t.Run("MissingOpeningDelimiter", func(t *testing.T) {
+		t.Parallel()
+
+		_, err := skills.ParsePersonalSkillMarkdown([]byte("name: my-skill\n---\nBody.\n"))
+
+		require.ErrorContains(t, err, "missing opening frontmatter delimiter")
+	})
+
+	t.Run("MissingClosingDelimiter", func(t *testing.T) {
+		t.Parallel()
+
+		_, err := skills.ParsePersonalSkillMarkdown([]byte("---\nname: my-skill\nBody.\n"))
+
+		require.ErrorContains(t, err, "missing closing frontmatter delimiter")
+	})
+
+	t.Run("MissingName", func(t *testing.T) {
+		t.Parallel()
+
+		_, err := skills.ParsePersonalSkillMarkdown([]byte(
+			"---\ndescription: No name\n---\nBody.\n",
+		))
+
+		require.ErrorIs(t, err, skills.ErrInvalidSkillName)
+		require.ErrorContains(t, err, "frontmatter must contain a 'name' field")
+	})
+
+	t.Run("NonKebabCaseName", func(t *testing.T) {
+		t.Parallel()
+
+		_, err := skills.ParsePersonalSkillMarkdown([]byte(
+			"---\nname: Not_Kebab\n---\nBody.\n",
+		))
+
+		require.ErrorIs(t, err, skills.ErrInvalidSkillName)
+		require.ErrorContains(t, err, "Not_Kebab")
+	})
+
+	t.Run("NameTooLong", func(t *testing.T) {
+		t.Parallel()
+
+		_, err := skills.ParsePersonalSkillMarkdown([]byte(personalSkillMarkdownForTest(
+			strings.Repeat("a", skills.MaxPersonalSkillNameBytes+1),
+			"Too long",
+			"Body.",
+		)))
+
+		require.ErrorIs(t, err, skills.ErrInvalidSkillName)
+		require.ErrorContains(t, err, "maximum is 256 bytes")
+	})
+
+	t.Run("EmptyBody", func(t *testing.T) {
+		t.Parallel()
+
+		_, err := skills.ParsePersonalSkillMarkdown([]byte(
+			"---\nname: my-skill\n---\n\n",
+		))
+
+		require.ErrorIs(t, err, skills.ErrSkillBodyRequired)
+		require.ErrorContains(t, err, "my-skill")
+	})
+
+	t.Run("OversizedContent", func(t *testing.T) {
+		t.Parallel()
+
+		raw := []byte(strings.Repeat("a", skills.MaxPersonalSkillSizeBytes+1))
+		_, err := skills.ParsePersonalSkillMarkdown(raw)
+
+		require.ErrorIs(t, err, skills.ErrSkillTooLarge)
+	})
+}
+
+func personalSkillMarkdownForTest(name string, description string, body string) string {
+	return "---\nname: " + name + "\ndescription: " + description + "\n---\n\n" + body + "\n"
+}
+
+func TestMergeSkills(t *testing.T) {
+	t.Parallel()
+
+	t.Run("PersonalOnlyUsesBareAlias", func(t *testing.T) {
+		t.Parallel()
+
+		resolved := skills.MergeSkills(
+			[]skills.Skill{{Name: "my-skill", Description: "Mine"}},
+			nil,
+		)
+
+		require.Equal(t, []skills.ResolvedSkill{{
+			Skill: skills.Skill{
+				Name:        "my-skill",
+				Description: "Mine",
+				Source:      skills.SourcePersonal,
+			},
+			Alias: "my-skill",
+		}}, resolved)
+	})
+
+	t.Run("WorkspaceOnlyUsesBareAlias", func(t *testing.T) {
+		t.Parallel()
+
+		resolved := skills.MergeSkills(
+			nil,
+			[]skills.Skill{{Name: "my-skill", Description: "Workspace"}},
+		)
+
+		require.Equal(t, []skills.ResolvedSkill{{
+			Skill: skills.Skill{
+				Name:        "my-skill",
+				Description: "Workspace",
+				Source:      skills.SourceWorkspace,
+			},
+			Alias: "my-skill",
+		}}, resolved)
+	})
+
+	t.Run("NonCollidingSkillsUseBareAliases", func(t *testing.T) {
+		t.Parallel()
+
+		resolved := skills.MergeSkills(
+			[]skills.Skill{{Name: "personal-skill"}},
+			[]skills.Skill{{Name: "workspace-skill"}},
+		)
+
+		require.Equal(t, []skills.ResolvedSkill{
+			{
+				Skill: skills.Skill{
+					Name:   "personal-skill",
+					Source: skills.SourcePersonal,
+				},
+				Alias: "personal-skill",
+			},
+			{
+				Skill: skills.Skill{
+					Name:   "workspace-skill",
+					Source: skills.SourceWorkspace,
+				},
+				Alias: "workspace-skill",
+			},
+		}, resolved)
+	})
+
+	t.Run("CollidingSkillsUseQualifiedAliases", func(t *testing.T) {
+		t.Parallel()
+
+		resolved := skills.MergeSkills(
+			[]skills.Skill{{Name: "shared-skill", Description: "Mine"}},
+			[]skills.Skill{{Name: "shared-skill", Description: "Workspace"}},
+		)
+
+		require.Equal(t, []skills.ResolvedSkill{
+			{
+				Skill: skills.Skill{
+					Name:        "shared-skill",
+					Description: "Mine",
+					Source:      skills.SourcePersonal,
+				},
+				Alias: "personal/shared-skill",
+			},
+			{
+				Skill: skills.Skill{
+					Name:        "shared-skill",
+					Description: "Workspace",
+					Source:      skills.SourceWorkspace,
+				},
+				Alias: "workspace/shared-skill",
+			},
+		}, resolved)
+
+		personal, err := skills.Lookup(resolved, "personal/shared-skill")
+		require.NoError(t, err)
+		require.Equal(t, skills.SourcePersonal, personal.Source)
+		require.Equal(t, "shared-skill", personal.Name)
+
+		workspace, err := skills.Lookup(resolved, "workspace/shared-skill")
+		require.NoError(t, err)
+		require.Equal(t, skills.SourceWorkspace, workspace.Source)
+		require.Equal(t, "shared-skill", workspace.Name)
+
+		_, err = skills.Lookup(resolved, "shared-skill")
+		require.ErrorIs(t, err, skills.ErrSkillAmbiguous)
+		require.ErrorContains(t, err, "personal/shared-skill")
+		require.ErrorContains(t, err, "workspace/shared-skill")
+	})
+
+	t.Run("DuplicatesWithinSourceKeepFirst", func(t *testing.T) {
+		t.Parallel()
+
+		resolved := skills.MergeSkills(
+			[]skills.Skill{
+				{Name: "duplicate-skill", Description: "First"},
+				{Name: "duplicate-skill", Description: "Second"},
+			},
+			[]skills.Skill{
+				{Name: "workspace-skill", Description: "Workspace"},
+				{Name: "workspace-skill", Description: "Workspace duplicate"},
+			},
+		)
+
+		require.Equal(t, []skills.ResolvedSkill{
+			{
+				Skill: skills.Skill{
+					Name:        "duplicate-skill",
+					Description: "First",
+					Source:      skills.SourcePersonal,
+				},
+				Alias: "duplicate-skill",
+			},
+			{
+				Skill: skills.Skill{
+					Name:        "workspace-skill",
+					Description: "Workspace",
+					Source:      skills.SourceWorkspace,
+				},
+				Alias: "workspace-skill",
+			},
+		}, resolved)
+	})
+}
+
+func TestLookup(t *testing.T) {
+	t.Parallel()
+
+	t.Run("BareNameOnNonCollidingSkill", func(t *testing.T) {
+		t.Parallel()
+
+		resolved := skills.MergeSkills(
+			[]skills.Skill{{Name: "personal-skill"}},
+			[]skills.Skill{{Name: "workspace-skill"}},
+		)
+
+		personal, err := skills.Lookup(resolved, "personal-skill")
+		require.NoError(t, err)
+		require.Equal(t, skills.SourcePersonal, personal.Source)
+		require.Equal(t, "personal-skill", personal.Name)
+
+		workspace, err := skills.Lookup(resolved, "workspace-skill")
+		require.NoError(t, err)
+		require.Equal(t, skills.SourceWorkspace, workspace.Source)
+		require.Equal(t, "workspace-skill", workspace.Name)
+	})
+
+	t.Run("QualifiedAliasWorksWithoutCollision", func(t *testing.T) {
+		t.Parallel()
+
+		resolved := skills.MergeSkills(
+			[]skills.Skill{{Name: "personal-skill"}},
+			[]skills.Skill{{Name: "workspace-skill"}},
+		)
+
+		personal, err := skills.Lookup(resolved, "personal/personal-skill")
+		require.NoError(t, err)
+		require.Equal(t, skills.SourcePersonal, personal.Source)
+		require.Equal(t, "personal-skill", personal.Name)
+
+		workspace, err := skills.Lookup(resolved, "workspace/workspace-skill")
+		require.NoError(t, err)
+		require.Equal(t, skills.SourceWorkspace, workspace.Source)
+		require.Equal(t, "workspace-skill", workspace.Name)
+	})
+
+	t.Run("BareNameFallsBackToSingleQualifiedAliasMatch", func(t *testing.T) {
+		t.Parallel()
+
+		resolved := []skills.ResolvedSkill{{
+			Skill: skills.Skill{Name: "personal-skill", Source: skills.SourcePersonal},
+			Alias: "personal/personal-skill",
+		}}
+
+		personal, err := skills.Lookup(resolved, "personal-skill")
+
+		require.NoError(t, err)
+		require.Equal(t, skills.SourcePersonal, personal.Source)
+		require.Equal(t, "personal-skill", personal.Name)
+	})
+
+	t.Run("UnknownLookupReturnsNotFound", func(t *testing.T) {
+		t.Parallel()
+
+		_, err := skills.Lookup(nil, "missing-skill")
+
+		require.ErrorIs(t, err, skills.ErrSkillNotFound)
+		require.ErrorContains(t, err, "missing-skill")
+	})
+}