feat: create skills during sync, and read again after a 409 on create

internal/sync/api/attachment.go

@@ -29,11 +29,19 @@ type toolMutationRequest struct {
 	Tags             []string       `json:"tags"`
 }
 
+// skillMutationRequest updates a skill. It has no name, because LaunchDarkly
+// owns the name of a skill after it creates the skill.
 type skillMutationRequest struct {
 	Description string `json:"description"`
 	Markdown    string `json:"markdown"`
 }
 
+type skillCreateRequest struct {
+	Key  string `json:"key"`
+	Name string `json:"name"`
+	skillMutationRequest
+}
+
 // SearchAttachments returns one page of the latest tools or skills that match query.
 func (client Client) SearchAttachments(
 	projectKey string,
@@ -100,11 +108,8 @@ func (client Client) ReadAttachment(projectKey string, kind syncdomain.Attachmen
 	return attachment, nil
 }
 
-// CreateAttachment creates the first version of a tool. Sync cannot create a skill.
+// CreateAttachment creates the first version of a tool or a skill.
 func (client Client) CreateAttachment(projectKey string, attachment syncdomain.Attachment) error {
-	if attachment.Kind != syncdomain.AttachmentTool {
-		return fmt.Errorf("creating %ss is not supported", attachment.Kind)
-	}
 	return client.mutateAttachment(projectKey, attachment, true)
 }
 
@@ -135,7 +140,11 @@ func (client Client) mutateAttachment(projectKey string, attachment syncdomain.A
 		}
 		request.body = body
 	case attachment.Kind == syncdomain.AttachmentSkill && attachment.Skill != nil:
-		request.body = skillMutationRequest{Description: attachment.Skill.Description, Markdown: attachment.Skill.Markdown}
+		body := skillMutationRequest{Description: attachment.Skill.Description, Markdown: attachment.Skill.Markdown}
+		request.body = body
+		if create {
+			request.body = skillCreateRequest{Key: attachment.Key(), Name: attachment.Skill.CreateName(), skillMutationRequest: body}
+		}
 	default:
 		return fmt.Errorf("%s %q has no content", attachment.Kind, attachment.Key())
 	}

internal/sync/api/attachment_test.go

@@ -183,6 +183,36 @@ func TestCreateToolIncludesItsKey(t *testing.T) {
 	assert.Equal(t, "search", body["key"])
 }
 
+func TestCreateSkillSendsKeyNameAndContent(t *testing.T) {
+	tests := map[string]struct {
+		name string
+		want string
+	}{
+		"name from the skill file": {name: "Human handoff", want: "Human handoff"},
+		"name from the key":        {want: "Incident summary"},
+	}
+
+	for title, test := range tests {
+		t.Run(title, func(t *testing.T) {
+			transport := &recordingClient{Responses: [][]byte{[]byte(`{}`)}}
+			client := NewClient(transport, "token", "https://example.com")
+			skill := syncdomain.Skill{Key: "incident-summary", Name: test.name, Description: "Summarize", Markdown: "# Summary\n"}
+
+			err := client.CreateAttachment("project", syncdomain.Attachment{Kind: syncdomain.AttachmentSkill, Skill: &skill})
+
+			require.NoError(t, err)
+			require.Len(t, transport.Requests, 1)
+			assert.Equal(t, http.MethodPost, transport.Requests[0].Method)
+			assert.Equal(t, "https://example.com/api/v2/projects/project/ai-configs/skills", transport.Requests[0].Path)
+			var body map[string]any
+			require.NoError(t, json.Unmarshal(transport.Requests[0].Body, &body))
+			assert.Equal(t, map[string]any{
+				"key": "incident-summary", "name": test.want, "description": "Summarize", "markdown": "# Summary\n",
+			}, body)
+		})
+	}
+}
+
 func TestReadAttachmentRequiresMatchingVersionedIdentity(t *testing.T) {
 	for name, response := range map[string]string{
 		"key":     `{"key":"other","schema":{},"version":2}`,

internal/sync/attachment.go

@@ -38,22 +38,33 @@ type Tool struct {
 }
 
 // Skill is one Markdown file and its catalog metadata. Sync owns Key,
-// Description, and Markdown. LaunchDarkly owns Name.
+// Description, and Markdown. LaunchDarkly owns Name. A local skill file can
+// set Name, and sync sends it only when it creates the skill.
 type Skill struct {
 	Key         string `json:"key"`
 	Name        string `json:"name"`
 	Description string `json:"description,omitempty"`
 	Markdown    string `json:"markdown"`
 }
 
+// CreateName returns the name of a new skill: the name in the skill file, or a
+// name made from the key.
+func (skill Skill) CreateName() string {
+	if skill.Name != "" {
+		return skill.Name
+	}
+	return DisplayName(skill.Key)
+}
+
 // Attachment is the content of one tool or one skill, with the latest version
 // that sync read from LaunchDarkly. Exactly one of Tool and Skill is set.
 //
 // The JSON encoding of this struct is part of the variation fingerprint.
 type Attachment struct {
 	Kind    AttachmentKind
 	Version int
-	// Upsert lets sync create the tool in LaunchDarkly when it is absent.
+	// Upsert lets sync create the tool or the skill in LaunchDarkly when it
+	// is absent.
 	Upsert bool
 	Tool   *Tool
 	Skill  *Skill

internal/sync/link/link.go

@@ -12,7 +12,6 @@ import (
 	"path/filepath"
 	"slices"
 	"strings"
-	"unicode"
 
 	"github.com/charmbracelet/huh"
 
@@ -151,7 +150,7 @@ func targetSelection(options Options, prompt linkedPrompt, target Target) (Selec
 		return Selection{}, "", fmt.Errorf("model config %q was not found", target.ModelConfigKey)
 	}
 
-	name := cmp.Or(prompt.parsed.Name, target.Name, displayName(variationKey))
+	name := cmp.Or(prompt.parsed.Name, target.Name, syncdomain.DisplayName(variationKey))
 	return Selection{
 		Project:     syncapi.Project{Key: projectKey},
 		Config:      config,
@@ -213,7 +212,7 @@ func promptForSelection(options Options, prompt linkedPrompt) (Selection, string
 		fields = append(fields, huh.NewInput().Title("Variation key").Value(&key).Validate(validateDerivedKey))
 	}
 	if name == "" {
-		name = displayName(key)
+		name = syncdomain.DisplayName(key)
 		fields = append(fields, huh.NewInput().Title("Variation name").Value(&name).Validate(requiredValue("variation name")))
 	}
 	if len(prompt.parsed.Messages) == 0 {
@@ -366,12 +365,3 @@ func requiredValue(label string) func(string) error {
 		return nil
 	}
 }
-
-// displayName converts a key such as "support-agent" to "Support agent".
-func displayName(key string) string {
-	runes := []rune(strings.NewReplacer("-", " ", "_", " ").Replace(key))
-	if len(runes) != 0 {
-		runes[0] = unicode.ToUpper(runes[0])
-	}
-	return string(runes)
-}

internal/sync/local/attachment.go

@@ -19,7 +19,9 @@ import (
 )
 
 // A tool file is JSON. A skill file is Markdown with YAML front matter that
-// has the key and the description.
+// has the key and the description. Both can set upsert, and a skill file can
+// set the name that sync sends when it creates the skill. Neither is part of
+// the fingerprint.
 const toolFormatVersion = 1
 
 type toolFile struct {
@@ -31,6 +33,8 @@ type toolFile struct {
 type skillFrontMatter struct {
 	Key         string  `yaml:"key"`
 	Description *string `yaml:"description"`
+	Name        string  `yaml:"name,omitempty"`
+	Upsert      bool    `yaml:"upsert,omitempty"`
 }
 
 // OrphanedAttachment is a tool or skill file that no variation uses.
@@ -121,17 +125,10 @@ func readSkill(fsys fs.FS, projectKey, key string) (syncdomain.Attachment, error
 		return syncdomain.Attachment{}, err
 	}
 
-	var metadata skillFrontMatter
-	body, err := parseYAMLFrontMatter(data, &metadata)
+	metadata, body, err := parseSkillFile(data)
 	if err != nil {
 		return syncdomain.Attachment{}, fmt.Errorf("parse skill %q: %w", key, err)
 	}
-	// Rendering puts one blank line after the front matter. Remove it.
-	if bytes.HasPrefix(body, []byte("\r\n")) {
-		body = body[2:]
-	} else {
-		body = bytes.TrimPrefix(body, []byte("\n"))
-	}
 
 	switch {
 	case metadata.Key != key:
@@ -141,8 +138,23 @@ func readSkill(fsys fs.FS, projectKey, key string) (syncdomain.Attachment, error
 	case strings.TrimSpace(string(body)) == "":
 		return syncdomain.Attachment{}, fmt.Errorf("skill %q markdown is required", key)
 	}
-	skill := syncdomain.Skill{Key: key, Description: *metadata.Description, Markdown: string(body)}
-	return syncdomain.Attachment{Kind: syncdomain.AttachmentSkill, Skill: &skill}, nil
+	skill := syncdomain.Skill{Key: key, Name: metadata.Name, Description: *metadata.Description, Markdown: string(body)}
+	return syncdomain.Attachment{Kind: syncdomain.AttachmentSkill, Upsert: metadata.Upsert, Skill: &skill}, nil
+}
+
+// parseSkillFile decodes the front matter of a skill file and returns the
+// Markdown that follows it.
+func parseSkillFile(data []byte) (skillFrontMatter, []byte, error) {
+	var metadata skillFrontMatter
+	body, err := parseYAMLFrontMatter(data, &metadata)
+	if err != nil {
+		return skillFrontMatter{}, nil, err
+	}
+	// Rendering puts one blank line after the front matter. Remove it.
+	if bytes.HasPrefix(body, []byte("\r\n")) {
+		return metadata, body[2:], nil
+	}
+	return metadata, bytes.TrimPrefix(body, []byte("\n")), nil
 }
 
 // readAttachmentFile reads one tool or skill file. It rejects a symbolic link
@@ -210,7 +222,10 @@ func renderAttachment(attachment syncdomain.Attachment) ([]byte, error) {
 		}
 		return renderToolFile(toolFile{FormatVersion: toolFormatVersion, Upsert: attachment.Upsert, Tool: *attachment.Tool})
 	case attachment.Kind == syncdomain.AttachmentSkill && attachment.Skill != nil:
-		return renderSkill(*attachment.Skill)
+		// LaunchDarkly owns the name of a skill, so the file does not get it.
+		// A pull keeps the name of an existing file.
+		skill := *attachment.Skill
+		return renderSkillFile(skillFrontMatter{Key: skill.Key, Description: &skill.Description, Upsert: attachment.Upsert}, skill.Markdown)
 	default:
 		return nil, fmt.Errorf("%s %q has no content", attachment.Kind, attachment.Key())
 	}
@@ -227,41 +242,55 @@ func renderToolFile(file toolFile) ([]byte, error) {
 	return content.Bytes(), nil
 }
 
-func renderSkill(skill syncdomain.Skill) ([]byte, error) {
-	if strings.TrimSpace(skill.Markdown) == "" {
-		return nil, fmt.Errorf("skill %q markdown is required", skill.Key)
+func renderSkillFile(metadata skillFrontMatter, markdown string) ([]byte, error) {
+	if strings.TrimSpace(markdown) == "" {
+		return nil, fmt.Errorf("skill %q markdown is required", metadata.Key)
 	}
-	metadata, err := marshalYAML(skillFrontMatter{Key: skill.Key, Description: &skill.Description})
+	frontMatter, err := marshalYAML(metadata)
 	if err != nil {
-		return nil, fmt.Errorf("marshal skill %q metadata: %w", skill.Key, err)
+		return nil, fmt.Errorf("marshal skill %q metadata: %w", metadata.Key, err)
 	}
 
 	var content bytes.Buffer
 	content.WriteString("---\n")
-	content.Write(metadata)
+	content.Write(frontMatter)
 	content.WriteString("---\n\n")
-	content.WriteString(skill.Markdown)
+	content.WriteString(markdown)
 	return content.Bytes(), nil
 }
 
-// preserveToolUpsert keeps the upsert flag of an existing tool file. The flag
-// is a local choice, so new content from LaunchDarkly does not remove it.
-func preserveToolUpsert(filePath string, original, replacement []byte) ([]byte, error) {
-	if !strings.HasSuffix(filePath, toolFileSuffix) {
-		return replacement, nil
+// preserveLocalFields keeps the local choices of an existing tool or skill
+// file: the upsert flag, and the name of a skill. New content from
+// LaunchDarkly does not remove them.
+func preserveLocalFields(filePath string, original, replacement []byte) ([]byte, error) {
+	if strings.HasSuffix(filePath, toolFileSuffix) {
+		var local, updated toolFile
+		if err := json.Unmarshal(original, &local); err != nil {
+			return nil, fmt.Errorf("parse existing tool %q: %w", filePath, err)
+		}
+		if !local.Upsert {
+			return replacement, nil
+		}
+		if err := json.Unmarshal(replacement, &updated); err != nil {
+			return nil, fmt.Errorf("parse replacement tool %q: %w", filePath, err)
+		}
+		updated.Upsert = true
+		return renderToolFile(updated)
 	}
-	var local, updated toolFile
-	if err := json.Unmarshal(original, &local); err != nil {
-		return nil, fmt.Errorf("parse existing tool %q: %w", filePath, err)
+
+	local, _, err := parseSkillFile(original)
+	if err != nil {
+		return nil, fmt.Errorf("parse existing skill %q: %w", filePath, err)
 	}
-	if !local.Upsert {
+	if local.Name == "" && !local.Upsert {
 		return replacement, nil
 	}
-	if err := json.Unmarshal(replacement, &updated); err != nil {
-		return nil, fmt.Errorf("parse replacement tool %q: %w", filePath, err)
+	updated, markdown, err := parseSkillFile(replacement)
+	if err != nil {
+		return nil, fmt.Errorf("parse replacement skill %q: %w", filePath, err)
 	}
-	updated.Upsert = true
-	return renderToolFile(updated)
+	updated.Name, updated.Upsert = local.Name, updated.Upsert || local.Upsert
+	return renderSkillFile(updated, string(markdown))
 }
 
 // OrphanedAttachments returns each tool and skill file that no local

internal/sync/local/attachment_test.go

@@ -63,12 +63,33 @@ Help the customer.
 }
 
 func TestRenderSkillIncludesReadOnlyKeyAndEditableDescription(t *testing.T) {
-	rendered, err := renderSkill(syncdomain.Skill{
-		Key: "support", Description: "Follow the standard support process.", Markdown: "Follow the support process.\n",
-	})
+	skill := syncdomain.Skill{
+		Key: "support", Name: "Support", Description: "Follow the standard support process.", Markdown: "Follow the support process.\n",
+	}
+
+	rendered, err := renderAttachment(syncdomain.Attachment{Kind: syncdomain.AttachmentSkill, Skill: &skill})
+	require.NoError(t, err)
+	assert.Equal(t, "---\nkey: support\ndescription: Follow the standard support process.\n---\n\nFollow the support process.\n", string(rendered),
+		"LaunchDarkly owns the name, so the file does not get it")
+
+	rendered, err = renderAttachment(syncdomain.Attachment{Kind: syncdomain.AttachmentSkill, Upsert: true, Skill: &skill})
+	require.NoError(t, err)
+	assert.Contains(t, string(rendered), "\nupsert: true\n")
+}
+
+func TestReadSkillReadsTheOptionalNameAndUpsert(t *testing.T) {
+	fsys := fstest.MapFS{
+		".launchdarkly/project/skills/handoff.md": {
+			Data: []byte("---\nkey: handoff\ndescription: Hand off\nname: Human handoff\nupsert: true\n---\n\n# Handoff\n"),
+		},
+	}
+
+	attachment, err := readSkill(fsys, "project", "handoff")
 
 	require.NoError(t, err)
-	assert.Equal(t, "---\nkey: support\ndescription: Follow the standard support process.\n---\n\nFollow the support process.\n", string(rendered))
+	assert.True(t, attachment.Upsert)
+	assert.Equal(t, "Human handoff", attachment.Skill.Name)
+	assert.Equal(t, "# Handoff\n", attachment.Skill.Markdown)
 }
 
 func TestReadSkillRejectsKeyThatDoesNotMatchFilename(t *testing.T) {
@@ -144,7 +165,7 @@ func TestPreserveToolUpsertAcrossServerWrites(t *testing.T) {
 	})
 	require.NoError(t, err)
 
-	preserved, err := preserveToolUpsert("tools/search.json", original, replacement)
+	preserved, err := preserveLocalFields("tools/search.json", original, replacement)
 
 	require.NoError(t, err)
 	var file toolFile
@@ -153,6 +174,23 @@ func TestPreserveToolUpsertAcrossServerWrites(t *testing.T) {
 	assert.Equal(t, "New", *file.Description)
 }
 
+func TestPreserveSkillNameAndUpsertAcrossServerWrites(t *testing.T) {
+	original := []byte("---\nkey: handoff\ndescription: Old\nname: Human handoff\nupsert: true\n---\n\n# Old\n")
+	server := syncdomain.Skill{Key: "handoff", Name: "Renamed in LaunchDarkly", Description: "New", Markdown: "# New\n"}
+	replacement, err := renderAttachment(syncdomain.Attachment{Kind: syncdomain.AttachmentSkill, Skill: &server})
+	require.NoError(t, err)
+
+	preserved, err := preserveLocalFields("project/skills/handoff.md", original, replacement)
+
+	require.NoError(t, err)
+	assert.Equal(t, "---\nkey: handoff\ndescription: New\nname: Human handoff\nupsert: true\n---\n\n# New\n", string(preserved))
+
+	plain := []byte("---\nkey: handoff\ndescription: Old\n---\n\n# Old\n")
+	preserved, err = preserveLocalFields("project/skills/handoff.md", plain, replacement)
+	require.NoError(t, err)
+	assert.Equal(t, replacement, preserved)
+}
+
 func TestReplaceVariationsRemovesNewAttachmentWhenVariationUpdateFails(t *testing.T) {
 	root := t.TempDir()
 	store := NewStore(root)

internal/sync/local/replace.go

@@ -189,7 +189,7 @@ func (store Store) prepareLinkedFile(ref Reference, replacement VariationReplace
 }
 
 // prepareAttachmentFiles renders each tool and skill file that changed. A new
-// file is created. A tool file keeps its local upsert flag.
+// file is created. An existing file keeps its local fields.
 func (store Store) prepareAttachmentFiles(files []VariationFile) ([]stagedFile, error) {
 	rendered, err := renderAttachmentFiles(files)
 	if err != nil {
@@ -214,7 +214,7 @@ func (store Store) prepareAttachmentFiles(files []VariationFile) ([]stagedFile,
 			if err != nil {
 				return nil, fmt.Errorf("inspect attachment %q: %w", file.Path, err)
 			}
-			if next.replacement, err = preserveToolUpsert(file.Path, original, file.Content); err != nil {
+			if next.replacement, err = preserveLocalFields(file.Path, original, file.Content); err != nil {
 				return nil, err
 			}
 			next.original, next.originalExists, next.mode = original, true, info.Mode().Perm()