From 0cde3feaba640ac1b85778b71fbe1bbc1893f305 Mon Sep 17 00:00:00 2001 From: Clifford Tawiah Date: Fri, 2 Oct 2026 16:38:25 -0400 Subject: [PATCH] feat(sync): reconcile variation attachments --- internal/sync/api/client.go | 38 ++- internal/sync/bootstrap/bootstrap.go | 84 +++--- internal/sync/bootstrap/bootstrap_test.go | 66 ++++- internal/sync/detach/detach.go | 20 +- internal/sync/detach/detach_test.go | 57 +++- internal/sync/fingerprint.go | 114 +++++++- internal/sync/fingerprint_test.go | 124 +++++++++ internal/sync/local/attachment.go | 300 ++++++++++++++++++++++ internal/sync/local/attachment_test.go | 238 +++++++++++++++++ internal/sync/local/compile.go | 36 ++- internal/sync/local/render.go | 101 +++++++- internal/sync/local/replace.go | 68 +++++ internal/sync/local/store.go | 48 +++- internal/sync/local/variation.go | 50 +++- internal/sync/manifest/model.go | 49 ++++ internal/sync/manifest/model_test.go | 53 ++++ internal/sync/prompt/acceptance_test.go | 254 ++++++++++++++++-- internal/sync/prompt/conflict.go | 95 +++++-- internal/sync/prompt/conflict_test.go | 184 ++++++++++++- internal/sync/prompt/execute.go | 254 +++++++++++++++++- internal/sync/prompt/execute_test.go | 145 ++++++++++- internal/sync/prompt/local_changes.go | 2 +- internal/sync/prompt/plan.go | 165 +++++++++++- internal/sync/prompt/plan_test.go | 38 +++ internal/sync/prompt/runner.go | 88 ++++++- internal/sync/prompt/runner_test.go | 15 ++ internal/sync/resource.go | 102 +++++++- 27 files changed, 2606 insertions(+), 182 deletions(-) create mode 100644 internal/sync/local/attachment.go create mode 100644 internal/sync/local/attachment_test.go diff --git a/internal/sync/api/client.go b/internal/sync/api/client.go index 2bba1674..41d89b6d 100644 --- a/internal/sync/api/client.go +++ b/internal/sync/api/client.go @@ -27,22 +27,26 @@ type VariationState struct { } type createVariationRequest struct { - Key string `json:"key"` - Name string `json:"name"` - Instructions string `json:"instructions,omitempty"` - ModelConfigKey string `json:"modelConfigKey,omitempty"` - ModelConfigVersion int `json:"modelConfigVersion,omitempty"` - Model map[string]any `json:"model,omitempty"` - Messages []syncdomain.Message `json:"messages,omitempty"` + Key string `json:"key"` + Name string `json:"name"` + Instructions string `json:"instructions,omitempty"` + ModelConfigKey string `json:"modelConfigKey,omitempty"` + ModelConfigVersion int `json:"modelConfigVersion,omitempty"` + Model map[string]any `json:"model,omitempty"` + Messages []syncdomain.Message `json:"messages,omitempty"` + Tools []syncdomain.AttachmentRef `json:"tools,omitempty"` + Skills []syncdomain.AttachmentRef `json:"skills,omitempty"` } type updateVariationRequest struct { - Name string `json:"name"` - Instructions *string `json:"instructions,omitempty"` - ModelConfigKey string `json:"modelConfigKey"` - ModelConfigVersion int `json:"modelConfigVersion,omitempty"` - Model map[string]any `json:"model"` - Messages *[]syncdomain.Message `json:"messages,omitempty"` + Name string `json:"name"` + Instructions *string `json:"instructions,omitempty"` + ModelConfigKey string `json:"modelConfigKey"` + ModelConfigVersion int `json:"modelConfigVersion,omitempty"` + Model map[string]any `json:"model"` + Messages *[]syncdomain.Message `json:"messages,omitempty"` + Tools *[]syncdomain.AttachmentRef `json:"tools,omitempty"` + Skills *[]syncdomain.AttachmentRef `json:"skills,omitempty"` } type mutationError struct { @@ -133,6 +137,8 @@ func (client Client) CreateVariation(projectKey, configKey string, variation syn ModelConfigKey: variation.ModelConfigKey, ModelConfigVersion: variation.ModelConfigVersion, Model: variation.Model, + Tools: variation.Tools, + Skills: variation.Skills, } if variation.Mode == syncdomain.VariationModeAgent { request.Instructions = variation.Instructions @@ -182,6 +188,12 @@ func (client Client) UpdateVariation(projectKey, configKey string, variation syn ModelConfigVersion: variation.ModelConfigVersion, Model: model, } + if variation.Tools != nil { + request.Tools = &variation.Tools + } + if variation.Skills != nil { + request.Skills = &variation.Skills + } // Agent and completion configs reject fields owned by the other mode, even // when those fields are empty. Include only the prompt field for this mode. diff --git a/internal/sync/bootstrap/bootstrap.go b/internal/sync/bootstrap/bootstrap.go index 5de9b8f8..e06e99ab 100644 --- a/internal/sync/bootstrap/bootstrap.go +++ b/internal/sync/bootstrap/bootstrap.go @@ -23,6 +23,10 @@ type Catalog interface { SearchConfigs(projectKey, query string, modes []syncdomain.VariationMode, limit, offset int) (syncapi.Page[syncapi.Config], error) } +type AttachmentReader interface { + ReadAttachment(projectKey string, kind syncdomain.AttachmentKind, key string) (syncdomain.Attachment, error) +} + // ManifestStore persists the synchronization baseline after local files are written. type ManifestStore interface { Load() (syncmanifest.Manifest, bool, error) @@ -31,13 +35,14 @@ type ManifestStore interface { // Options contains the dependencies and streams for one bootstrap flow. type Options struct { - Catalog Catalog - Store synclocal.Store - Manifest ManifestStore - Input io.Reader - Output io.Writer - Initial bool - DryRun bool + Catalog Catalog + Attachments AttachmentReader + Store synclocal.Store + Manifest ManifestStore + Input io.Reader + Output io.Writer + Initial bool + DryRun bool } // Run interactively selects prompt variations and writes their local wrappers. @@ -131,6 +136,9 @@ func selectVariationFiles(options Options) ([]synclocal.VariationFile, bool, err files := make([]synclocal.VariationFile, 0, len(variations)) for _, variation := range variations { + if err := hydrateAttachments(options.Attachments, project.Key, &variation); err != nil { + return nil, false, err + } files = append(files, synclocal.VariationFile{ ProjectKey: project.Key, ConfigKey: config.Key, @@ -141,6 +149,26 @@ func selectVariationFiles(options Options) ([]synclocal.VariationFile, bool, err return files, false, nil } +func hydrateAttachments(reader AttachmentReader, projectKey string, variation *syncdomain.Variation) error { + for index := range variation.Tools { + attachment, err := reader.ReadAttachment(projectKey, syncdomain.AttachmentTool, variation.Tools[index].Key) + if err != nil { + return err + } + variation.Tools[index].Version = attachment.Version + variation.SetAttachment(attachment) + } + for index := range variation.Skills { + attachment, err := reader.ReadAttachment(projectKey, syncdomain.AttachmentSkill, variation.Skills[index].Key) + if err != nil { + return err + } + variation.Skills[index].Version = attachment.Version + variation.SetAttachment(attachment) + } + return variation.NormalizeAttachments() +} + // variationChoices returns unsynchronized variations in stable display order // and separately counts variations that already have local wrappers. func variationChoices( @@ -194,13 +222,13 @@ func finishSelection(options Options, files []synclocal.VariationFile) error { writeNoChangeSummary(options.Output, options.DryRun) return nil } - for _, file := range files { - if err := syncdomain.ValidateDirectAPIVariation(file.Variation); err != nil { - return err - } - } if options.DryRun { - previews, err := options.Store.RenderVariations(files) + for _, file := range files { + if err := syncdomain.ValidateDirectAPIVariation(file.Variation); err != nil { + return err + } + } + previews, err := options.Store.RenderResources(files) if err != nil { return err } @@ -221,42 +249,32 @@ func finishSelection(options Options, files []synclocal.VariationFile) error { manifest.SetFingerprint(syncdomain.ResourceID{ Kind: syncdomain.KindVariation, ProjectKey: file.ProjectKey, LookupKey: lookupKey, }, fingerprint) + if err := manifest.SetAttachmentsIfMissing(file.ProjectKey, file.Variation.Attachments); err != nil { + return err + } } - var paths []string + var creation synclocal.Creation if options.Initial { - paths, err = options.Store.Bootstrap(files) + creation, err = options.Store.BootstrapResources(files) } else { - paths, err = options.Store.Add(files) + creation, err = options.Store.AddResources(files) } if err != nil { return err } // The manifest is written last so it never claims a wrapper exists before - // that wrapper reaches disk. Roll back the wrappers if persistence fails. + // that wrapper reaches disk. Roll back every file this operation created if + // persistence fails, including shared dependencies that did not exist before. if err := options.Manifest.Write(manifest); err != nil { - return errors.Join(err, rollbackVariationFiles(options.Store, files)) + return errors.Join(err, options.Store.RollbackCreation(creation)) } - writeSummary(options.Output, options.Initial, len(paths)) + writeSummary(options.Output, options.Initial, len(creation.VariationPaths)) return nil } -// rollbackVariationFiles removes wrappers created by a failed bootstrap or add. -func rollbackVariationFiles(store synclocal.Store, files []synclocal.VariationFile) error { - deletions := make([]synclocal.VariationDeletion, 0, len(files)) - for _, file := range files { - deletions = append(deletions, synclocal.VariationDeletion{ - ProjectKey: file.ProjectKey, - ConfigKey: file.ConfigKey, - VariationKey: file.Variation.Key, - }) - } - _, err := store.DeleteVariations(deletions) - return err -} - // writeNoChangeSummary explains that every available variation is already local. func writeNoChangeSummary(output io.Writer, dryRun bool) { message := "No variations added; every variation in that config is already synced." diff --git a/internal/sync/bootstrap/bootstrap_test.go b/internal/sync/bootstrap/bootstrap_test.go index 54cb4908..3d354bfa 100644 --- a/internal/sync/bootstrap/bootstrap_test.go +++ b/internal/sync/bootstrap/bootstrap_test.go @@ -137,25 +137,42 @@ func TestFinishSelectionWritesInitialManifest(t *testing.T) { var output bytes.Buffer variation := syncdomain.Variation{ Mode: syncdomain.VariationModeAgent, Key: "variation", Name: "Variation", Instructions: "Be helpful.", + Tools: []syncdomain.AttachmentRef{{Key: "search"}}, + Attachments: []syncdomain.Attachment{{ + Kind: syncdomain.AttachmentTool, + Tool: &syncdomain.Tool{Key: "search", Schema: map[string]any{"type": "object"}}, + }}, } + secondVariation := variation + secondVariation.Key = "variation-2" + secondVariation.Name = "Variation 2" err := finishSelection(Options{ Store: synclocal.NewStore(root), Manifest: syncmanifest.NewStore(root), Output: &output, Initial: true, - }, []synclocal.VariationFile{{ - ProjectKey: "project", ConfigKey: "config", Upsert: true, Variation: variation, - }}) + }, []synclocal.VariationFile{ + {ProjectKey: "project", ConfigKey: "config", Upsert: true, Variation: variation}, + {ProjectKey: "project", ConfigKey: "config", Upsert: true, Variation: secondVariation}, + }) require.NoError(t, err) manifest, exists, err := syncmanifest.NewStore(root).Load() require.NoError(t, err) require.True(t, exists) - require.Len(t, manifest.Resources, 1) - expected, err := syncdomain.FingerprintVariation("project", "config/variation", variation) + require.Len(t, manifest.Resources, 3) + attachmentFingerprint, err := syncdomain.FingerprintAttachment("project", variation.Attachments[0]) + require.NoError(t, err) + assert.Equal(t, syncdomain.KindTool, manifest.Resources[0].ResourceKind) + assert.Equal(t, "search", manifest.Resources[0].LookupKey) + assert.Equal(t, attachmentFingerprint, manifest.Resources[0].Fingerprint) + variationFingerprint, err := syncdomain.FingerprintVariation("project", "config/variation", variation) require.NoError(t, err) - assert.Equal(t, expected, manifest.Resources[0].Fingerprint) + assert.Equal(t, syncdomain.KindVariation, manifest.Resources[1].ResourceKind) + assert.Equal(t, variationFingerprint, manifest.Resources[1].Fingerprint) + assert.Equal(t, syncdomain.KindVariation, manifest.Resources[2].ResourceKind) + assert.Equal(t, "config/variation-2", manifest.Resources[2].LookupKey) } func TestFinishSelectionAddsMultipleVersionedVariationsToExistingManifest(t *testing.T) { @@ -199,6 +216,11 @@ func TestFinishSelectionRollsBackFilesWhenManifestWriteFails(t *testing.T) { root := t.TempDir() variation := syncdomain.Variation{ Mode: syncdomain.VariationModeAgent, Key: "variation", Name: "Variation", Instructions: "Be helpful.", + Tools: []syncdomain.AttachmentRef{{Key: "search"}}, + Attachments: []syncdomain.Attachment{{ + Kind: syncdomain.AttachmentTool, + Tool: &syncdomain.Tool{Key: "search", Schema: map[string]any{"type": "object"}}, + }}, } err := finishSelection(Options{ @@ -215,6 +237,38 @@ func TestFinishSelectionRollsBackFilesWhenManifestWriteFails(t *testing.T) { require.ErrorIs(t, statErr, os.ErrNotExist) } +func TestFinishSelectionRollsBackNewAttachmentWithoutRemovingExistingWorkspace(t *testing.T) { + root := t.TempDir() + store := synclocal.NewStore(root) + _, err := store.Bootstrap([]synclocal.VariationFile{{ + ProjectKey: "project", ConfigKey: "config", + Variation: syncdomain.Variation{ + Mode: syncdomain.VariationModeAgent, Key: "existing", Name: "Existing", + }, + }}) + require.NoError(t, err) + variation := syncdomain.Variation{ + Mode: syncdomain.VariationModeAgent, Key: "new", Name: "New", + Tools: []syncdomain.AttachmentRef{{Key: "search"}}, + Attachments: []syncdomain.Attachment{{ + Kind: syncdomain.AttachmentTool, + Tool: &syncdomain.Tool{Key: "search", Schema: map[string]any{"type": "object"}}, + }}, + } + + err = finishSelection(Options{ + Store: store, Manifest: failingManifestStore{}, Output: io.Discard, + }, []synclocal.VariationFile{{ + ProjectKey: "project", ConfigKey: "config", Variation: variation, + }}) + + require.ErrorContains(t, err, "write manifest") + _, err = os.Stat(filepath.Join(root, ".launchdarkly", "project", "configs", "config", "existing.prompt.md")) + require.NoError(t, err) + _, err = os.Stat(filepath.Join(root, ".launchdarkly", "project", "tools", "search.json")) + require.ErrorIs(t, err, os.ErrNotExist) +} + func TestWriteSummary(t *testing.T) { var output bytes.Buffer diff --git a/internal/sync/detach/detach.go b/internal/sync/detach/detach.go index 90a1513e..f54f07db 100644 --- a/internal/sync/detach/detach.go +++ b/internal/sync/detach/detach.go @@ -83,7 +83,9 @@ func loadResources(repositoryRoot string, manifestStore syncmanifest.Store) ([]R resources := make(map[Resource]struct{}, len(manifest.Resources)) for _, resource := range manifest.Resources { - resources[resource.ID()] = struct{}{} + if resource.ResourceKind == syncdomain.KindVariation { + resources[resource.ID()] = struct{}{} + } } files, err := synclocal.SourceFiles(repositoryRoot) @@ -131,6 +133,22 @@ func detachResources(options Options, original syncmanifest.Manifest, manifestEx updated.Resources = append(updated.Resources, resource) } } + if resources, err := synclocal.CompileWorkspace(options.RepositoryRoot); err == nil { + referenced := make(map[syncdomain.ResourceID]struct{}) + for _, resource := range resources { + if _, detach := selectedSet[syncdomain.ResourceID{ + Kind: resource.Kind, ProjectKey: resource.ProjectKey, LookupKey: resource.LookupKey, + }]; detach { + continue + } + for _, attachment := range resource.Attachments { + referenced[syncdomain.ResourceID{ + Kind: syncdomain.Kind(attachment.Kind), ProjectKey: resource.ProjectKey, LookupKey: attachment.Key(), + }] = struct{}{} + } + } + updated.RemoveUnreferencedAttachments(referenced) + } if err := options.Manifest.Write(updated); err != nil { return err } diff --git a/internal/sync/detach/detach_test.go b/internal/sync/detach/detach_test.go index eede1e82..b17ca002 100644 --- a/internal/sync/detach/detach_test.go +++ b/internal/sync/detach/detach_test.go @@ -27,10 +27,16 @@ func TestLoadResourcesUnionsLocalAndManifestResources(t *testing.T) { manifestStore := syncmanifest.NewStore(root) require.NoError(t, manifestStore.Write(syncmanifest.Manifest{ FormatVersion: syncmanifest.FormatVersion, - Resources: []syncmanifest.Resource{{ - ResourceKind: syncdomain.KindVariation, ProjectKey: "project", LookupKey: "config/manifest-only", - Fingerprint: testFingerprint(), - }}, + Resources: []syncmanifest.Resource{ + { + ResourceKind: syncdomain.KindVariation, ProjectKey: "project", LookupKey: "config/manifest-only", + Fingerprint: testFingerprint(), + }, + { + ResourceKind: syncdomain.KindTool, ProjectKey: "project", LookupKey: "search", + Fingerprint: testFingerprint(), + }, + }, })) resources, _, exists, err := loadResources(root, manifestStore) @@ -43,6 +49,49 @@ func TestLoadResourcesUnionsLocalAndManifestResources(t *testing.T) { }, resources) } +func TestDetachResourcesPrunesUnreferencedAttachmentManifestEntries(t *testing.T) { + root := t.TempDir() + store := synclocal.NewStore(root) + description := "Search documentation" + variation := testVariation("prompt") + variation.Tools = []syncdomain.AttachmentRef{{Key: "search"}} + variation.Attachments = []syncdomain.Attachment{{ + Kind: syncdomain.AttachmentTool, + Tool: &syncdomain.Tool{Key: "search", Description: &description, Schema: map[string]any{"type": "object"}}, + }} + _, err := store.Add([]synclocal.VariationFile{{ProjectKey: "project", ConfigKey: "config", Variation: variation}}) + require.NoError(t, err) + + manifestStore := syncmanifest.NewStore(root) + original := syncmanifest.Manifest{ + FormatVersion: syncmanifest.FormatVersion, + Resources: []syncmanifest.Resource{ + { + ResourceKind: syncdomain.KindVariation, ProjectKey: "project", LookupKey: "config/prompt", + Fingerprint: testFingerprint(), + }, + { + ResourceKind: syncdomain.KindTool, ProjectKey: "project", LookupKey: "search", + Fingerprint: testFingerprint(), + }, + }, + } + require.NoError(t, manifestStore.Write(original)) + + resource := Resource{Kind: syncdomain.KindVariation, ProjectKey: "project", LookupKey: "config/prompt"} + err = detachResources( + Options{RepositoryRoot: root, Store: store, Manifest: manifestStore}, + original, + true, + []Resource{resource}, + ) + + require.NoError(t, err) + manifest, _, err := manifestStore.Load() + require.NoError(t, err) + assert.Empty(t, manifest.Resources) +} + func TestDetachResourcesRemovesWrapperAndManifestButKeepsReferencedFile(t *testing.T) { root := t.TempDir() referencePath := filepath.Join(root, "prompts", "prompt.md") diff --git a/internal/sync/fingerprint.go b/internal/sync/fingerprint.go index cbbd6568..53f367c5 100644 --- a/internal/sync/fingerprint.go +++ b/internal/sync/fingerprint.go @@ -6,18 +6,38 @@ import ( "encoding/json" "fmt" "maps" + "slices" ) -const variationFingerprintSchema = "launchdarkly.config.variation/v1" +const ( + variationFingerprintSchema = "launchdarkly.config.variation/v1" + attachmentFingerprintSchema = "launchdarkly.config.attachment/v1" +) // FingerprintVariation returns a stable fingerprint for the variation fields // supported by the existing config variation APIs. func FingerprintVariation(projectKey, lookupKey string, variation Variation) (string, error) { - if err := ValidateDirectAPIVariation(variation); err != nil { + normalized := variation + // A value copy still shares slice backing arrays; clone before canonical + // normalization so fingerprinting never mutates the caller's variation. + normalized.Tools = append([]AttachmentRef(nil), variation.Tools...) + normalized.Skills = append([]AttachmentRef(nil), variation.Skills...) + normalized.Attachments = append([]Attachment(nil), variation.Attachments...) + if err := normalized.NormalizeAttachments(); err != nil { return "", err } - - normalized := variation + if err := validateDirectAPIVariationFields(normalized); err != nil { + return "", err + } + for index := range normalized.Attachments { + normalized.Attachments[index] = CanonicalAttachment(normalized.Attachments[index]) + } + for index := range normalized.Tools { + normalized.Tools[index].Version = 0 + } + for index := range normalized.Skills { + normalized.Skills[index].Version = 0 + } normalized.Model = normalizeModelForFingerprint(normalized.Model) if len(normalized.Messages) == 0 { normalized.Messages = nil @@ -28,6 +48,7 @@ func FingerprintVariation(projectKey, lookupKey string, variation Variation) (st normalized.Messages = nil case VariationModeCompletion: normalized.Instructions = "" + // Clone messages before normalizing their text to preserve caller state. normalized.Messages = append([]Message(nil), normalized.Messages...) for index := range normalized.Messages { normalized.Messages[index].Content = NormalizePromptText(normalized.Messages[index].Content) @@ -35,27 +56,87 @@ func FingerprintVariation(projectKey, lookupKey string, variation Variation) (st } value := struct { - Schema string `json:"schema"` - ResourceKind Kind `json:"resourceKind"` - ProjectKey string `json:"projectKey"` - LookupKey string `json:"lookupKey"` - Variation Variation `json:"variation"` + Schema string `json:"schema"` + ResourceKind Kind `json:"resourceKind"` + ProjectKey string `json:"projectKey"` + LookupKey string `json:"lookupKey"` + Variation Variation `json:"variation"` + Attachments []Attachment `json:"attachments,omitempty"` }{ Schema: variationFingerprintSchema, ResourceKind: KindVariation, ProjectKey: projectKey, LookupKey: lookupKey, Variation: normalized, + Attachments: normalized.Attachments, + } + + return fingerprint(value, "variation") +} + +// FingerprintAttachment returns the stable baseline for one project-scoped +// tool or skill, excluding runtime version and file-only metadata. +func FingerprintAttachment(projectKey string, attachment Attachment) (string, error) { + if attachment.Kind != AttachmentTool && attachment.Kind != AttachmentSkill { + return "", fmt.Errorf("unsupported attachment kind %q", attachment.Kind) + } + if attachment.Key() == "" { + return "", fmt.Errorf("%s key is required", attachment.Kind) } + value := struct { + Schema string `json:"schema"` + ResourceKind Kind `json:"resourceKind"` + ProjectKey string `json:"projectKey"` + LookupKey string `json:"lookupKey"` + Attachment Attachment `json:"attachment"` + }{ + Schema: attachmentFingerprintSchema, + ResourceKind: Kind(attachment.Kind), + ProjectKey: projectKey, + LookupKey: attachment.Key(), + Attachment: CanonicalAttachment(attachment), + } + return fingerprint(value, string(attachment.Kind)) +} + +func fingerprint(value any, resource string) (string, error) { canonical, err := json.Marshal(value) if err != nil { - return "", fmt.Errorf("encode variation fingerprint: %w", err) + return "", fmt.Errorf("encode %s fingerprint: %w", resource, err) } sum := sha256.Sum256(canonical) return "sha256:" + hex.EncodeToString(sum[:]), nil } +// CanonicalAttachment removes runtime versions, API-only metadata, and +// ordering differences before comparison or fingerprinting. +func CanonicalAttachment(attachment Attachment) Attachment { + attachment.Version = 0 + attachment.Upsert = false + if attachment.Tool != nil { + tool := *attachment.Tool + if len(tool.CustomParameters) == 0 { + tool.CustomParameters = nil + } + if len(tool.Tags) == 0 { + tool.Tags = nil + } else { + // Clone tags before sorting so canonicalization preserves caller order. + tool.Tags = append([]string(nil), tool.Tags...) + slices.Sort(tool.Tags) + } + attachment.Tool = &tool + } + if attachment.Skill != nil { + // Name is catalog metadata and is not editable from the local skill file. + skill := *attachment.Skill + skill.Name = "" + attachment.Skill = &skill + } + return attachment +} + // normalizeModelForFingerprint removes only defaults that the variation API // adds without changing model behavior. Other empty objects remain meaningful. func normalizeModelForFingerprint(model map[string]any) map[string]any { @@ -78,6 +159,19 @@ func normalizeModelForFingerprint(model map[string]any) map[string]any { // ValidateDirectAPIVariation rejects fields that the existing variation APIs // cannot round-trip without the sync endpoints. func ValidateDirectAPIVariation(variation Variation) error { + if err := validateDirectAPIVariationFields(variation); err != nil { + return err + } + if err := normalizeAttachmentRefs(AttachmentTool, variation.Tools); err != nil { + return err + } + if err := normalizeAttachmentRefs(AttachmentSkill, variation.Skills); err != nil { + return err + } + return validateAttachmentMode(variation) +} + +func validateDirectAPIVariationFields(variation Variation) error { switch { case !variation.Mode.Valid(): return fmt.Errorf("unsupported variation mode %q", variation.Mode) diff --git a/internal/sync/fingerprint_test.go b/internal/sync/fingerprint_test.go index 38cfe4d9..17c4f550 100644 --- a/internal/sync/fingerprint_test.go +++ b/internal/sync/fingerprint_test.go @@ -168,6 +168,121 @@ func TestFingerprintVariationPreservesMeaningfulModelChanges(t *testing.T) { require.NotEqual(t, emptyMetadataFingerprint, metadataFingerprint) } +func TestFingerprintVariationTracksAttachmentContentButNotRuntimeVersions(t *testing.T) { + description := "Search documentation" + variation := Variation{ + Mode: VariationModeAgent, Key: "default", Name: "Default", + Tools: []AttachmentRef{{Key: "search", Version: 2}}, + Attachments: []Attachment{{ + Kind: AttachmentTool, + Tool: &Tool{Key: "search", Description: &description, Schema: map[string]any{"type": "object"}}, + }}, + } + original, err := FingerprintVariation("project", "config/default", variation) + require.NoError(t, err) + + variation.Tools[0].Version = 3 + variation.Attachments[0].Upsert = true + repinned, err := FingerprintVariation("project", "config/default", variation) + require.NoError(t, err) + require.Equal(t, original, repinned) + + updatedDescription := "Search all documentation" + variation.Attachments[0].Tool.Description = &updatedDescription + updated, err := FingerprintVariation("project", "config/default", variation) + require.NoError(t, err) + require.NotEqual(t, original, updated) +} + +func TestFingerprintVariationTracksEditableSkillContentButNotServerOwnedName(t *testing.T) { + description := "Support guidance" + variation := Variation{ + Mode: VariationModeAgent, Key: "default", Name: "Default", + Skills: []AttachmentRef{{Key: "support"}}, + Attachments: []Attachment{{ + Kind: AttachmentSkill, + Skill: &Skill{Key: "support", Name: "Support", Description: description, Markdown: "# Support\n"}, + }}, + } + original, err := FingerprintVariation("project", "config/default", variation) + require.NoError(t, err) + + variation.Attachments[0].Skill.Name = "Renamed" + nameChanged, err := FingerprintVariation("project", "config/default", variation) + require.NoError(t, err) + require.Equal(t, original, nameChanged) + + updatedDescription := "Updated support guidance" + variation.Attachments[0].Skill.Description = updatedDescription + descriptionChanged, err := FingerprintVariation("project", "config/default", variation) + require.NoError(t, err) + require.NotEqual(t, original, descriptionChanged) + + variation.Attachments[0].Skill.Markdown = "# Updated support\n" + contentChanged, err := FingerprintVariation("project", "config/default", variation) + require.NoError(t, err) + require.NotEqual(t, original, contentChanged) +} + +func TestFingerprintVariationNormalizesEmptySkillDescription(t *testing.T) { + variation := Variation{ + Mode: VariationModeAgent, Key: "default", Name: "Default", + Skills: []AttachmentRef{{Key: "support"}}, + Attachments: []Attachment{{ + Kind: AttachmentSkill, + Skill: &Skill{Key: "support", Markdown: "# Support\n"}, + }}, + } + withoutDescription, err := FingerprintVariation("project", "config/default", variation) + require.NoError(t, err) + + variation.Attachments[0].Skill.Description = "" + withEmptyDescription, err := FingerprintVariation("project", "config/default", variation) + require.NoError(t, err) + + require.Equal(t, withoutDescription, withEmptyDescription) +} + +func TestFingerprintAttachmentTracksCanonicalContentOnly(t *testing.T) { + description := "Search documentation" + tool := Attachment{ + Kind: AttachmentTool, Version: 2, + Tool: &Tool{Key: "search", Description: &description, Schema: map[string]any{"type": "object"}}, + } + originalTool, err := FingerprintAttachment("project", tool) + require.NoError(t, err) + + tool.Version = 3 + tool.Upsert = true + repinnedTool, err := FingerprintAttachment("project", tool) + require.NoError(t, err) + require.Equal(t, originalTool, repinnedTool) + + updatedDescription := "Search all documentation" + tool.Tool.Description = &updatedDescription + updatedTool, err := FingerprintAttachment("project", tool) + require.NoError(t, err) + require.NotEqual(t, originalTool, updatedTool) + + skill := Attachment{ + Kind: AttachmentSkill, Version: 2, + Skill: &Skill{Key: "support", Name: "Support", Description: "Support guidance", Markdown: "# Support\n"}, + } + originalSkill, err := FingerprintAttachment("project", skill) + require.NoError(t, err) + + skill.Version = 3 + skill.Skill.Name = "Renamed" + repinnedSkill, err := FingerprintAttachment("project", skill) + require.NoError(t, err) + require.Equal(t, originalSkill, repinnedSkill) + + skill.Skill.Markdown = "# Updated support\n" + updatedSkill, err := FingerprintAttachment("project", skill) + require.NoError(t, err) + require.NotEqual(t, originalSkill, updatedSkill) +} + func TestValidateDirectAPIVariationSupportsModelConfigVersion(t *testing.T) { base := Variation{Mode: VariationModeAgent, Key: "default", Name: "Default"} @@ -179,3 +294,12 @@ func TestValidateDirectAPIVariationSupportsModelConfigVersion(t *testing.T) { withOutput.OutputFormat = map[string]any{"type": "json"} require.ErrorContains(t, ValidateDirectAPIVariation(withOutput), "outputFormat") } + +func TestValidateDirectAPIVariationRejectsSkillsForCompletionMode(t *testing.T) { + variation := Variation{ + Mode: VariationModeCompletion, Key: "default", Name: "Default", + Skills: []AttachmentRef{{Key: "support"}}, + } + + require.ErrorContains(t, ValidateDirectAPIVariation(variation), "skills can only be attached to agent-mode configs") +} diff --git a/internal/sync/local/attachment.go b/internal/sync/local/attachment.go new file mode 100644 index 00000000..ac47b110 --- /dev/null +++ b/internal/sync/local/attachment.go @@ -0,0 +1,300 @@ +package local + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "io/fs" + "path" + "slices" + "strings" + + syncdomain "github.com/launchdarkly/ldcli/internal/sync" + "gopkg.in/yaml.v3" +) + +const ( + toolsDir = "tools" + skillsDir = "skills" + toolFileSuffix = ".json" + skillFileSuffix = ".md" +) + +type toolFile struct { + FormatVersion int `json:"formatVersion"` + Upsert bool `json:"upsert,omitempty"` + syncdomain.Tool +} + +type skillFrontMatter struct { + Key string `yaml:"key"` + Description *string `yaml:"description"` +} + +// readTool loads and validates the deterministic local file for one tool key. +func readTool(fsys fs.FS, projectKey, key string) (syncdomain.Attachment, error) { + if err := validatePathSegment(key); err != nil { + return syncdomain.Attachment{}, fmt.Errorf("invalid tool key %q: %w", key, err) + } + + relativePath, _ := attachmentPath(projectKey, syncdomain.AttachmentTool, key) + filePath := path.Join(syncdomain.RootDir, relativePath) + data, err := readAttachmentFile(fsys, filePath) + if err != nil { + return syncdomain.Attachment{}, fmt.Errorf("read tool %q: %w", key, err) + } + + var file toolFile + decoder := json.NewDecoder(bytes.NewReader(data)) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&file); err != nil { + return syncdomain.Attachment{}, fmt.Errorf("parse tool %q: %w", key, err) + } + var trailing any + if err := decoder.Decode(&trailing); err != io.EOF { + if err == nil { + return syncdomain.Attachment{}, fmt.Errorf("parse tool %q: multiple JSON values are not supported", key) + } + return syncdomain.Attachment{}, fmt.Errorf("parse tool %q: %w", key, err) + } + switch { + case file.FormatVersion != 1: + return syncdomain.Attachment{}, fmt.Errorf("tool %q has unsupported formatVersion %d", key, file.FormatVersion) + case file.Key != key: + return syncdomain.Attachment{}, fmt.Errorf("tool key %q does not match filename %q", file.Key, key) + case file.Schema == nil: + return syncdomain.Attachment{}, fmt.Errorf("tool %q schema is required", key) + } + return syncdomain.Attachment{ + Kind: syncdomain.AttachmentTool, Upsert: file.Upsert, Tool: &file.Tool, + }, nil +} + +// readSkill loads one Markdown skill and its editable description. The filename +// remains authoritative for the read-only key shown in front matter. +func readSkill(fsys fs.FS, projectKey, key string) (syncdomain.Attachment, error) { + if err := validatePathSegment(key); err != nil { + return syncdomain.Attachment{}, fmt.Errorf("invalid skill key %q: %w", key, err) + } + + relativePath, _ := attachmentPath(projectKey, syncdomain.AttachmentSkill, key) + filePath := path.Join(syncdomain.RootDir, relativePath) + data, err := readAttachmentFile(fsys, filePath) + if err != nil { + return syncdomain.Attachment{}, fmt.Errorf("read skill %q: %w", key, err) + } + + var metadata skillFrontMatter + body, err := parseYAMLFrontMatter(data, &metadata) + if err != nil { + return syncdomain.Attachment{}, fmt.Errorf("parse skill %q: %w", key, err) + } + body = trimSkillBodySeparator(body) + switch { + case metadata.Key != key: + return syncdomain.Attachment{}, fmt.Errorf("skill key %q does not match filename %q", metadata.Key, key) + case metadata.Description == nil: + return syncdomain.Attachment{}, fmt.Errorf("skill %q description is required in front matter", key) + 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 +} + +func readAttachment(fsys fs.FS, projectKey string, kind syncdomain.AttachmentKind, key string) (syncdomain.Attachment, error) { + switch kind { + case syncdomain.AttachmentTool: + return readTool(fsys, projectKey, key) + case syncdomain.AttachmentSkill: + return readSkill(fsys, projectKey, key) + default: + return syncdomain.Attachment{}, fmt.Errorf("unsupported attachment kind %q", kind) + } +} + +func trimSkillBodySeparator(body []byte) []byte { + if bytes.HasPrefix(body, []byte("\r\n")) { + return body[2:] + } + return bytes.TrimPrefix(body, []byte("\n")) +} + +// readAttachmentFile rejects links and non-regular files before reading +// managed dependency content. This keeps all reads inside the workspace tree. +func readAttachmentFile(fsys fs.FS, filePath string) ([]byte, error) { + info, err := fs.Stat(fsys, filePath) + if err != nil { + return nil, err + } + if info.Mode()&fs.ModeSymlink != 0 { + return nil, fmt.Errorf("symbolic links are not supported") + } + if !info.Mode().IsRegular() { + return nil, fmt.Errorf("not a regular file") + } + return fs.ReadFile(fsys, filePath) +} + +// renderAttachmentFiles renders each shared dependency once, rejecting two +// variations that claim different local content for the same key. +func renderAttachmentFiles(resources []VariationFile) ([]RenderedVariationFile, error) { + files := make(map[string][]byte) + + for _, resource := range resources { + if err := validatePathSegment(resource.ProjectKey); err != nil { + return nil, fmt.Errorf("invalid project key %q: %w", resource.ProjectKey, err) + } + for _, attachment := range resource.Variation.Attachments { + var content []byte + var err error + + switch attachment.Kind { + case syncdomain.AttachmentTool: + content, err = renderTool(attachment) + case syncdomain.AttachmentSkill: + if attachment.Skill == nil { + return nil, fmt.Errorf("skill content is required") + } + content, err = renderSkill(*attachment.Skill) + default: + err = fmt.Errorf("unsupported attachment kind %q", attachment.Kind) + } + if err != nil { + return nil, err + } + filePath, err := attachmentPath(resource.ProjectKey, attachment.Kind, attachment.Key()) + if err != nil { + return nil, err + } + if err := addAttachmentFile(files, filePath, content); err != nil { + return nil, err + } + } + } + + paths := make([]string, 0, len(files)) + for filePath := range files { + paths = append(paths, filePath) + } + slices.Sort(paths) + + rendered := make([]RenderedVariationFile, 0, len(paths)) + for _, filePath := range paths { + rendered = append(rendered, RenderedVariationFile{Path: filePath, Content: files[filePath]}) + } + return rendered, nil +} + +func renderTool(attachment syncdomain.Attachment) ([]byte, error) { + tool := attachment.Tool + if tool == nil { + return nil, fmt.Errorf("tool content is required") + } + if err := validatePathSegment(tool.Key); err != nil { + return nil, fmt.Errorf("invalid tool key %q: %w", tool.Key, err) + } + if tool.Schema == nil { + return nil, fmt.Errorf("tool %q schema is required", tool.Key) + } + + return renderToolFile(toolFile{FormatVersion: 1, Upsert: attachment.Upsert, Tool: *tool}) +} + +func renderToolFile(file toolFile) ([]byte, error) { + var content bytes.Buffer + encoder := json.NewEncoder(&content) + encoder.SetEscapeHTML(false) + encoder.SetIndent("", " ") + if err := encoder.Encode(file); err != nil { + return nil, fmt.Errorf("marshal tool %q: %w", file.Key, err) + } + return content.Bytes(), nil +} + +func renderSkill(skill syncdomain.Skill) ([]byte, error) { + if err := validatePathSegment(skill.Key); err != nil { + return nil, fmt.Errorf("invalid skill key %q: %w", skill.Key, err) + } + if strings.TrimSpace(skill.Markdown) == "" { + return nil, fmt.Errorf("skill %q markdown is required", skill.Key) + } + + var metadata bytes.Buffer + encoder := yaml.NewEncoder(&metadata) + encoder.SetIndent(2) + if err := encoder.Encode(skillFrontMatter{Key: skill.Key, Description: &skill.Description}); err != nil { + return nil, fmt.Errorf("marshal skill %q metadata: %w", skill.Key, err) + } + + var file bytes.Buffer + file.WriteString("---\n") + file.Write(metadata.Bytes()) + file.WriteString("---\n\n") + file.WriteString(skill.Markdown) + return file.Bytes(), nil +} + +func addAttachmentFile(files map[string][]byte, filePath string, content []byte) error { + if existing, ok := files[filePath]; ok && !bytes.Equal(existing, content) { + return fmt.Errorf("attachment %q has conflicting local definitions", filePath) + } + files[filePath] = content + return nil +} + +func preserveToolUpsert(filePath string, original, replacement []byte) ([]byte, error) { + if !strings.HasSuffix(filePath, toolFileSuffix) { + return replacement, nil + } + 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) +} + +// AttachVariation commits dependency files and their consuming variation in +// the same staged transaction. +func (store Store) AttachVariation(projectKey, configKey string, variation syncdomain.Variation) error { + _, err := store.ReplaceVariations([]VariationReplacement{{ + ProjectKey: projectKey, + ConfigKey: configKey, + Variation: variation, + }}) + return err +} + +func attachmentPath(projectKey string, kind syncdomain.AttachmentKind, key string) (string, error) { + if err := validatePathSegment(projectKey); err != nil { + return "", fmt.Errorf("invalid project key %q: %w", projectKey, err) + } + if err := validatePathSegment(key); err != nil { + return "", fmt.Errorf("invalid %s key %q: %w", kind, key, err) + } + dir, suffix, err := attachmentLayout(kind) + if err != nil { + return "", err + } + return path.Join(projectKey, dir, key+suffix), nil +} + +func attachmentLayout(kind syncdomain.AttachmentKind) (string, string, error) { + switch kind { + case syncdomain.AttachmentTool: + return toolsDir, toolFileSuffix, nil + case syncdomain.AttachmentSkill: + return skillsDir, skillFileSuffix, nil + default: + return "", "", fmt.Errorf("unsupported attachment kind %q", kind) + } +} diff --git a/internal/sync/local/attachment_test.go b/internal/sync/local/attachment_test.go new file mode 100644 index 00000000..4d5ddfe1 --- /dev/null +++ b/internal/sync/local/attachment_test.go @@ -0,0 +1,238 @@ +package local + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + "testing/fstest" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + syncdomain "github.com/launchdarkly/ldcli/internal/sync" +) + +func TestCompileHydratesVariationAttachments(t *testing.T) { + fsys := fstest.MapFS{ + ".launchdarkly/project/configs/config/default.prompt.md": { + Data: []byte(`--- +formatVersion: 1 +mode: agent +key: default +name: Default +tools: + - key: search +skills: + - key: support +--- + +Help the customer. +`), + }, + ".launchdarkly/project/tools/search.json": { + Data: []byte(`{ + "formatVersion": 1, + "upsert": true, + "key": "search", + "description": "Search documentation", + "schema": {"type": "object"} +}`), + }, + ".launchdarkly/project/skills/support.md": { + Data: []byte("---\nkey: support\ndescription: Follow the standard support process.\n---\n\nFollow the support process.\n"), + }, + } + + resources, err := Compile(fsys) + + require.NoError(t, err) + require.Len(t, resources, 1) + var variation syncdomain.Variation + require.NoError(t, json.Unmarshal(resources[0].Payload, &variation)) + assert.Equal(t, []syncdomain.AttachmentRef{{Key: "search"}}, variation.Tools) + assert.Equal(t, []syncdomain.AttachmentRef{{Key: "support"}}, variation.Skills) + require.Len(t, resources[0].Attachments, 2) + assert.Equal(t, "Search documentation", *resources[0].Attachments[0].Tool.Description) + assert.True(t, resources[0].Attachments[0].Upsert) + assert.Equal(t, "Follow the standard support process.", resources[0].Attachments[1].Skill.Description) + assert.Contains(t, resources[0].Attachments[1].Skill.Markdown, "Follow the support process.") +} + +func TestRenderSkillIncludesReadOnlyKeyAndEditableDescription(t *testing.T) { + rendered, err := renderSkill(syncdomain.Skill{ + Key: "support", Description: "Follow the standard support process.", Markdown: "Follow the support process.\n", + }) + + require.NoError(t, err) + assert.Equal(t, "---\nkey: support\ndescription: Follow the standard support process.\n---\n\nFollow the support process.\n", string(rendered)) +} + +func TestReadSkillRejectsKeyThatDoesNotMatchFilename(t *testing.T) { + fsys := fstest.MapFS{ + ".launchdarkly/project/skills/support.md": { + Data: []byte("---\nkey: other\ndescription: Support\n---\n\nFollow the support process.\n"), + }, + } + + _, err := readSkill(fsys, "project", "support") + + require.ErrorContains(t, err, `skill key "other" does not match filename "support"`) +} + +func TestReadSkillRequiresDescriptionFrontMatter(t *testing.T) { + fsys := fstest.MapFS{ + ".launchdarkly/project/skills/support.md": { + Data: []byte("---\nkey: support\n---\n\nFollow the support process.\n"), + }, + } + + _, err := readSkill(fsys, "project", "support") + + require.ErrorContains(t, err, "description is required") +} + +func TestCompileRejectsSkillsForCompletionVariation(t *testing.T) { + fsys := fstest.MapFS{ + ".launchdarkly/project/configs/config/default.prompt.md": { + Data: []byte(`--- +formatVersion: 1 +mode: completion +key: default +name: Default +skills: + - key: support +--- + + +Help the customer. + +`), + }, + } + + _, err := Compile(fsys) + + require.ErrorContains(t, err, "skills can only be attached to agent-mode configs") +} + +func TestReadToolRejectsMultipleJSONValues(t *testing.T) { + fsys := fstest.MapFS{ + ".launchdarkly/project/tools/search.json": { + Data: []byte(`{"formatVersion":1,"key":"search","schema":{}} {}`), + }, + } + + _, err := readTool(fsys, "project", "search") + + require.ErrorContains(t, err, "multiple JSON values") +} + +func TestPreserveToolUpsertAcrossServerWrites(t *testing.T) { + oldDescription, newDescription := "Old", "New" + original, err := renderToolFile(toolFile{ + FormatVersion: 1, Upsert: true, + Tool: syncdomain.Tool{Key: "search", Description: &oldDescription, Schema: map[string]any{}}, + }) + require.NoError(t, err) + replacement, err := renderToolFile(toolFile{ + FormatVersion: 1, + Tool: syncdomain.Tool{Key: "search", Description: &newDescription, Schema: map[string]any{}}, + }) + require.NoError(t, err) + + preserved, err := preserveToolUpsert("tools/search.json", original, replacement) + + require.NoError(t, err) + var file toolFile + require.NoError(t, json.Unmarshal(preserved, &file)) + assert.True(t, file.Upsert) + assert.Equal(t, "New", *file.Description) +} + +func TestReplaceVariationsLeavesAttachmentUnchangedWhenPreflightFails(t *testing.T) { + root := t.TempDir() + store := NewStore(root) + oldDescription, newDescription := "Old", "New" + tool := syncdomain.Tool{Key: "search", Description: &oldDescription, Schema: map[string]any{"type": "object"}} + existing := localVariation("default") + existing.Variation.Tools = []syncdomain.AttachmentRef{{Key: "search"}} + existing.Variation.Attachments = []syncdomain.Attachment{{Kind: syncdomain.AttachmentTool, Tool: &tool}} + _, err := store.Add([]VariationFile{existing}) + require.NoError(t, err) + + updated := existing.Variation + updated.Attachments[0].Tool.Description = &newDescription + _, err = store.ReplaceVariations([]VariationReplacement{ + {ProjectKey: "project", ConfigKey: "config", Variation: updated}, + {ProjectKey: "project", ConfigKey: "config", Variation: localVariation("missing").Variation}, + }) + + require.Error(t, err) + attachment, err := readAttachment(os.DirFS(root), "project", syncdomain.AttachmentTool, "search") + require.NoError(t, err) + assert.Equal(t, oldDescription, *attachment.Tool.Description) +} + +func TestCompileWorkspaceRejectsAttachmentSymlink(t *testing.T) { + root := t.TempDir() + wrapperPath := filepath.Join(root, ".launchdarkly", "project", "configs", "config", "default.prompt.md") + require.NoError(t, os.MkdirAll(filepath.Dir(wrapperPath), 0o755)) + require.NoError(t, os.WriteFile(wrapperPath, []byte(`--- +formatVersion: 1 +mode: agent +key: default +name: Default +tools: + - key: search +--- + +Help the customer. +`), 0o644)) + + external := filepath.Join(root, "external.json") + require.NoError(t, os.WriteFile(external, []byte(`{"formatVersion":1,"key":"search","schema":{}}`), 0o644)) + toolPath := filepath.Join(root, ".launchdarkly", "project", "tools", "search.json") + require.NoError(t, os.MkdirAll(filepath.Dir(toolPath), 0o755)) + require.NoError(t, os.Symlink(external, toolPath)) + + _, err := CompileWorkspace(root) + + require.ErrorContains(t, err, "symbolic links are not supported") +} + +func TestCompileWorkspaceRejectsAttachmentParentSymlink(t *testing.T) { + root := t.TempDir() + wrapperPath := filepath.Join(root, ".launchdarkly", "project", "configs", "config", "default.prompt.md") + require.NoError(t, os.MkdirAll(filepath.Dir(wrapperPath), 0o755)) + require.NoError(t, os.WriteFile(wrapperPath, []byte(`--- +formatVersion: 1 +mode: agent +key: default +name: Default +skills: + - key: support +--- + +Help the customer. +`), 0o644)) + + externalDir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(externalDir, "support.md"), []byte("# Support\n"), 0o644)) + skillsPath := filepath.Join(root, ".launchdarkly", "project", "skills") + require.NoError(t, os.MkdirAll(filepath.Dir(skillsPath), 0o755)) + require.NoError(t, os.Symlink(externalDir, skillsPath)) + + _, err := CompileWorkspace(root) + + require.ErrorContains(t, err, "symbolic links are not supported") + + skill := syncdomain.Skill{Key: "support", Markdown: "# Updated\n"} + variation := syncdomain.Variation{ + Mode: syncdomain.VariationModeAgent, Key: "default", Name: "Default", Instructions: "Help the customer.", + Skills: []syncdomain.AttachmentRef{{Key: "support"}}, + Attachments: []syncdomain.Attachment{{Kind: syncdomain.AttachmentSkill, Skill: &skill}}, + } + err = NewStore(root).AttachVariation("project", "config", variation) + require.ErrorContains(t, err, "symbolic links are not supported") +} diff --git a/internal/sync/local/compile.go b/internal/sync/local/compile.go index f1226173..3d901eb6 100644 --- a/internal/sync/local/compile.go +++ b/internal/sync/local/compile.go @@ -3,9 +3,11 @@ package local import ( "cmp" "errors" + "fmt" "io/fs" "os" "path" + "path/filepath" "slices" "strings" @@ -41,11 +43,38 @@ func Compile(fsys fs.FS) ([]syncdomain.SyncedResource, error) { // CompileWorkspace compiles local resources and safely resolves references // within the Git repository. func CompileWorkspace(repositoryRoot string) ([]syncdomain.SyncedResource, error) { - return compile(os.DirFS(repositoryRoot), func(reference Reference) ([]byte, error) { - return readWorkspaceReference(repositoryRoot, reference) + root, err := filepath.EvalSymlinks(repositoryRoot) + if err != nil { + return nil, fmt.Errorf("resolve repository root: %w", err) + } + return compile(workspaceFS{root: root}, func(reference Reference) ([]byte, error) { + return readWorkspaceReference(root, reference) }) } +// workspaceFS rejects symlinks anywhere in a managed path. Managed files are +// owned by sync and must not redirect reads outside (or elsewhere within) the +// repository. +type workspaceFS struct { + root string +} + +func (fsys workspaceFS) Open(name string) (fs.File, error) { + if !fs.ValidPath(name) { + return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrInvalid} + } + + target := filepath.Join(fsys.root, filepath.FromSlash(name)) + resolved, err := filepath.EvalSymlinks(target) + if err != nil { + return nil, err + } + if filepath.Clean(resolved) != filepath.Clean(target) { + return nil, &fs.PathError{Op: "open", Path: name, Err: errors.New("symbolic links are not supported")} + } + return os.Open(target) +} + // compile walks every managed project and delegates reference loading to the // caller so tests and real workspaces share the same parser. func compile(fsys fs.FS, readReference func(Reference) ([]byte, error)) ([]syncdomain.SyncedResource, error) { @@ -119,6 +148,9 @@ func compileProjectVariations( RelPath: relPath, Data: data, ReadReference: readReference, + ReadAttachment: func(kind syncdomain.AttachmentKind, key string) (syncdomain.Attachment, error) { + return readAttachment(fsys, projectKey, kind, key) + }, }) if err != nil { // Preserve the managed path so users can locate malformed content diff --git a/internal/sync/local/render.go b/internal/sync/local/render.go index df6f5e85..a1826eef 100644 --- a/internal/sync/local/render.go +++ b/internal/sync/local/render.go @@ -39,30 +39,72 @@ func (store Store) RenderVariations(resources []VariationFile) ([]RenderedVariat return rendered, nil } +// RenderResources validates and renders dependency files before their +// consuming variation wrappers. +func (store Store) RenderResources(resources []VariationFile) ([]RenderedVariationFile, error) { + variations, err := store.RenderVariations(resources) + if err != nil { + return nil, err + } + attachments, err := renderAttachmentFiles(resources) + if err != nil { + return nil, err + } + return append(attachments, variations...), nil +} + // createVariations writes a prevalidated batch and rolls back files created // before the first failure. -func (store Store) createVariations(resources []VariationFile) ([]string, error) { +func (store Store) createVariations(resources []VariationFile) (Creation, error) { // Render the complete batch first so validation failures cannot leave a // partially created workspace. - rendered, err := store.RenderVariations(resources) + files, err := store.RenderResources(resources) if err != nil { - return nil, err + return Creation{}, err } var createdPaths []string - for _, file := range rendered { + for _, file := range files { absolutePath := filepath.Join(store.root, filepath.FromSlash(file.Path)) - if err := createFile(absolutePath, file.Content); err != nil { - // Creates are independent filesystem operations. Remove earlier - // files in reverse order to recover the pre-call state. - for index := len(createdPaths) - 1; index >= 0; index-- { - _ = os.Remove(filepath.Join(store.root, filepath.FromSlash(createdPaths[index]))) - } - return nil, errors.Join(err, store.RemoveEmptyDirectories()) + created, err := createResourceFile(store.root, absolutePath, file.Content) + if err != nil { + rollbackErr := store.RollbackCreation(Creation{CreatedPaths: createdPaths}) + return Creation{}, errors.Join(err, rollbackErr) + } + if created { + createdPaths = append(createdPaths, file.Path) + } + } + + var variationPaths []string + for _, file := range files { + if strings.HasSuffix(file.Path, variationFileSuffix) { + variationPaths = append(variationPaths, file.Path) + } + } + return Creation{VariationPaths: variationPaths, CreatedPaths: createdPaths}, nil +} + +// createResourceFile preserves an existing dependency file so adding another +// consuming variation never overwrites local edits. +func createResourceFile(root, path string, data []byte) (bool, error) { + _, err := os.Stat(path) + switch { + case err == nil: + if err := rejectSymlinkedPath(root, path); err != nil { + return false, err } - createdPaths = append(createdPaths, file.Path) + if strings.HasSuffix(path, variationFileSuffix) { + return false, fmt.Errorf("%w: %s", ErrVariationExists, path) + } + return false, nil + case !errors.Is(err, os.ErrNotExist): + return false, fmt.Errorf("inspect resource file %s: %w", filepath.Base(path), err) + } + if err := createFile(root, path, data); err != nil { + return false, err } - return createdPaths, nil + return true, nil } // marshalVariationFile converts the canonical variation into front matter and @@ -71,6 +113,9 @@ func marshalVariationFile(resource VariationFile) ([]byte, error) { if !resource.Variation.Mode.Valid() { return nil, fmt.Errorf("variation %q has unsupported mode %q", resource.Variation.Key, resource.Variation.Mode) } + if err := resource.Variation.NormalizeAttachments(); err != nil { + return nil, fmt.Errorf("variation %q attachments: %w", resource.Variation.Key, err) + } switch resource.Variation.Mode { case syncdomain.VariationModeAgent: if len(resource.Variation.Messages) != 0 { @@ -142,7 +187,10 @@ func escapeMessageContent(content, role string) string { // createFile stages content beside its destination and hard-links it into // place, which guarantees an existing wrapper is never overwritten. -func createFile(path string, data []byte) error { +func createFile(root, path string, data []byte) error { + if err := rejectSymlinkedPath(root, path); err != nil { + return err + } if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { return fmt.Errorf("create variation directory: %w", err) } @@ -175,3 +223,28 @@ func createFile(path string, data []byte) error { } return nil } + +// rejectSymlinkedPath prevents managed writes from being redirected through +// an existing file or parent-directory symlink. +func rejectSymlinkedPath(root, target string) error { + relative, err := filepath.Rel(root, target) + if err != nil || relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) { + return fmt.Errorf("managed path %s is outside %s", target, root) + } + + current := root + for _, component := range append([]string{""}, strings.Split(relative, string(filepath.Separator))...) { + current = filepath.Join(current, component) + info, err := os.Lstat(current) + if errors.Is(err, os.ErrNotExist) { + return nil + } + if err != nil { + return fmt.Errorf("inspect managed path %s: %w", current, err) + } + if info.Mode()&os.ModeSymlink != 0 { + return fmt.Errorf("symbolic links are not supported: %s", current) + } + } + return nil +} diff --git a/internal/sync/local/replace.go b/internal/sync/local/replace.go index fc87dcc8..177e57f9 100644 --- a/internal/sync/local/replace.go +++ b/internal/sync/local/replace.go @@ -17,6 +17,7 @@ type stagedVariation struct { replacementContent []byte mode os.FileMode stagedPath string + originalExists bool report bool } @@ -76,6 +77,7 @@ func (store Store) prepareReplacements(resources []VariationReplacement) ([]stag originalContent: existing.content, replacementContent: content, mode: existing.mode, + originalExists: true, report: true, }) @@ -112,6 +114,59 @@ func (store Store) prepareReplacements(resources []VariationReplacement) ([]stag originalContent: originalContent, replacementContent: referenceContent, mode: info.Mode().Perm(), + originalExists: true, + }) + } + + attachmentFiles := make([]VariationFile, 0, len(resources)) + for _, resource := range resources { + attachmentFiles = append(attachmentFiles, VariationFile{ + ProjectKey: resource.ProjectKey, + Variation: resource.Variation, + }) + } + files, err := renderAttachmentFiles(attachmentFiles) + if err != nil { + return nil, err + } + for _, file := range files { + destination := filepath.Join(store.root, filepath.FromSlash(file.Path)) + if err := rejectSymlinkedPath(store.root, destination); err != nil { + return nil, err + } + if _, duplicate := seenPaths[destination]; duplicate { + return nil, fmt.Errorf("resource file %q was selected more than once", file.Path) + } + seenPaths[destination] = struct{}{} + + original, err := os.ReadFile(destination) + exists := err == nil + if err != nil && !errors.Is(err, os.ErrNotExist) { + return nil, fmt.Errorf("read attachment %q: %w", file.Path, err) + } + replacement := file.Content + mode := os.FileMode(0o644) + if exists { + replacement, err = preserveToolUpsert(file.Path, original, replacement) + if err != nil { + return nil, err + } + info, err := os.Stat(destination) + if err != nil { + return nil, fmt.Errorf("stat attachment %q: %w", file.Path, err) + } + mode = info.Mode().Perm() + } + if exists && bytes.Equal(original, replacement) { + continue + } + replacements = append(replacements, stagedVariation{ + relativePath: file.Path, + destinationPath: destination, + originalContent: original, + replacementContent: replacement, + mode: mode, + originalExists: exists, }) } return replacements, nil @@ -121,6 +176,10 @@ func (store Store) prepareReplacements(resources []VariationReplacement) ([]stag // before the first original file is changed. func stageReplacements(replacements []stagedVariation) error { for index := range replacements { + if err := os.MkdirAll(filepath.Dir(replacements[index].destinationPath), 0o755); err != nil { + removeStagedVariations(replacements) + return fmt.Errorf("create resource directory %s: %w", replacements[index].relativePath, err) + } stagedPath, err := stageReplacement(replacements[index]) if err != nil { removeStagedVariations(replacements) @@ -136,6 +195,9 @@ func stageReplacements(replacements []stagedVariation) error { func verifyReplacementSources(replacements []stagedVariation) error { for _, replacement := range replacements { current, err := os.ReadFile(replacement.destinationPath) + if !replacement.originalExists && errors.Is(err, os.ErrNotExist) { + continue + } if err != nil { return fmt.Errorf("recheck variation %s: %w", replacement.relativePath, err) } @@ -241,6 +303,12 @@ func rollbackVariations(replacements []stagedVariation) error { // partially restored state. for index := len(replacements) - 1; index >= 0; index-- { replacement := replacements[index] + if !replacement.originalExists { + if err := os.Remove(replacement.destinationPath); err != nil && !errors.Is(err, os.ErrNotExist) { + rollbackErr = errors.Join(rollbackErr, fmt.Errorf("roll back variation %s: %w", replacement.relativePath, err)) + } + continue + } tempPath, err := stageReplacement(stagedVariation{ relativePath: replacement.relativePath, destinationPath: replacement.destinationPath, replacementContent: replacement.originalContent, mode: replacement.mode, diff --git a/internal/sync/local/store.go b/internal/sync/local/store.go index 4bdc75a1..5ac8c667 100644 --- a/internal/sync/local/store.go +++ b/internal/sync/local/store.go @@ -44,6 +44,13 @@ type RenderedVariationFile struct { Content []byte } +// Creation records the variation paths returned to users and every file that +// can be safely removed if a later manifest write fails. +type Creation struct { + VariationPaths []string + CreatedPaths []string +} + // Store reads and writes resources under a repository's .launchdarkly directory. type Store struct { repositoryRoot string @@ -107,36 +114,63 @@ func (store Store) VariationExists(projectKey, configKey, variationKey string) ( // Bootstrap atomically creates a new .launchdarkly directory. func (store Store) Bootstrap(resources []VariationFile) ([]string, error) { + creation, err := store.BootstrapResources(resources) + return creation.VariationPaths, err +} + +// BootstrapResources creates a workspace and returns its rollback record. +func (store Store) BootstrapResources(resources []VariationFile) (Creation, error) { if _, err := os.Stat(store.root); err == nil { - return nil, fmt.Errorf("%s already exists", store.root) + return Creation{}, fmt.Errorf("%s already exists", store.root) } else if !errors.Is(err, os.ErrNotExist) { - return nil, fmt.Errorf("inspect %s: %w", store.root, err) + return Creation{}, fmt.Errorf("inspect %s: %w", store.root, err) } stagingDirectory, err := os.MkdirTemp(filepath.Dir(store.root), ".launchdarkly.tmp-") if err != nil { - return nil, fmt.Errorf("create bootstrap staging directory: %w", err) + return Creation{}, fmt.Errorf("create bootstrap staging directory: %w", err) } defer func() { _ = os.RemoveAll(stagingDirectory) }() // Build the entire workspace in a sibling directory. The final rename is a // single commit point because source and destination share a filesystem. stagedStore := Store{repositoryRoot: store.repositoryRoot, root: stagingDirectory} - paths, err := stagedStore.createVariations(resources) + creation, err := stagedStore.createVariations(resources) if err != nil { - return nil, err + return Creation{}, err } if err := os.Rename(stagingDirectory, store.root); err != nil { - return nil, fmt.Errorf("finish bootstrap: %w", err) + return Creation{}, fmt.Errorf("finish bootstrap: %w", err) } - return paths, nil + return creation, nil } // Add creates a batch of variation wrappers without overwriting existing files. func (store Store) Add(resources []VariationFile) ([]string, error) { + creation, err := store.AddResources(resources) + return creation.VariationPaths, err +} + +// AddResources creates resources and returns the exact files published by the call. +func (store Store) AddResources(resources []VariationFile) (Creation, error) { return store.createVariations(resources) } +// RollbackCreation removes only files published by the corresponding create. +func (store Store) RollbackCreation(creation Creation) error { + var failures []error + for index := len(creation.CreatedPaths) - 1; index >= 0; index-- { + filePath := filepath.Join(store.root, filepath.FromSlash(creation.CreatedPaths[index])) + if err := os.Remove(filePath); err != nil && !errors.Is(err, os.ErrNotExist) { + failures = append(failures, fmt.Errorf("remove created resource %s: %w", creation.CreatedPaths[index], err)) + } + } + if err := store.RemoveEmptyDirectories(); err != nil { + failures = append(failures, err) + } + return errors.Join(failures...) +} + type existingVariation struct { relativePath string absolutePath string diff --git a/internal/sync/local/variation.go b/internal/sync/local/variation.go index e8f11224..7c987b21 100644 --- a/internal/sync/local/variation.go +++ b/internal/sync/local/variation.go @@ -20,10 +20,11 @@ const ( ) type localFile struct { - ProjectKey string - RelPath string - Data []byte - ReadReference func(Reference) ([]byte, error) + ProjectKey string + RelPath string + Data []byte + ReadReference func(Reference) ([]byte, error) + ReadAttachment func(syncdomain.AttachmentKind, string) (syncdomain.Attachment, error) } type variationFrontMatter struct { @@ -99,6 +100,10 @@ func parseVariation(file localFile) (syncdomain.SyncedResource, error) { } } + if err := hydrateLocalAttachments(&variation, file); err != nil { + return syncdomain.SyncedResource{}, err + } + payload, err := marshalPayload(variation) if err != nil { return syncdomain.SyncedResource{}, err @@ -107,14 +112,41 @@ func parseVariation(file localFile) (syncdomain.SyncedResource, error) { configKey := path.Dir(file.RelPath) return syncdomain.SyncedResource{ - Kind: syncdomain.KindVariation, - ProjectKey: file.ProjectKey, - LookupKey: configKey + "/" + meta.Key, - Payload: payload, - Upsert: meta.Upsert, + Kind: syncdomain.KindVariation, + ProjectKey: file.ProjectKey, + LookupKey: configKey + "/" + meta.Key, + Payload: payload, + Attachments: variation.Attachments, + Upsert: meta.Upsert, }, nil } +// hydrateLocalAttachments resolves key-only wrapper references into the +// canonical dependency content included in variation fingerprints. +func hydrateLocalAttachments(variation *syncdomain.Variation, file localFile) error { + if err := variation.NormalizeAttachments(); err != nil { + return err + } + + variation.Attachments = make([]syncdomain.Attachment, 0, len(variation.Tools)+len(variation.Skills)) + for _, ref := range variation.Tools { + attachment, err := file.ReadAttachment(syncdomain.AttachmentTool, ref.Key) + if err != nil { + return err + } + variation.Attachments = append(variation.Attachments, attachment) + } + + for _, ref := range variation.Skills { + attachment, err := file.ReadAttachment(syncdomain.AttachmentSkill, ref.Key) + if err != nil { + return err + } + variation.Attachments = append(variation.Attachments, attachment) + } + return nil +} + // validateVariation checks the file format and binds the declared key to the filename. func validateVariation(relPath string, meta variationFrontMatter) error { switch { diff --git a/internal/sync/manifest/model.go b/internal/sync/manifest/model.go index f3db682c..d46b17ab 100644 --- a/internal/sync/manifest/model.go +++ b/internal/sync/manifest/model.go @@ -56,6 +56,55 @@ func (manifest *Manifest) SetFingerprint(id syncdomain.ResourceID, fingerprint s }) } +// SetAttachments records the canonical state of shared dependencies once, +// regardless of how many variations reference them. +func (manifest *Manifest) SetAttachments(projectKey string, attachments []syncdomain.Attachment) error { + return manifest.setAttachments(projectKey, attachments, true) +} + +// SetAttachmentsIfMissing establishes baselines for newly tracked +// dependencies without advancing existing baselines past unsynchronized edits. +func (manifest *Manifest) SetAttachmentsIfMissing(projectKey string, attachments []syncdomain.Attachment) error { + return manifest.setAttachments(projectKey, attachments, false) +} + +// setAttachments writes canonical dependency baselines with explicit overwrite behavior. +func (manifest *Manifest) setAttachments(projectKey string, attachments []syncdomain.Attachment, overwrite bool) error { + for _, attachment := range attachments { + id := syncdomain.ResourceID{ + Kind: syncdomain.Kind(attachment.Kind), + ProjectKey: projectKey, + LookupKey: attachment.Key(), + } + if !overwrite && manifest.has(id) { + continue + } + fingerprint, err := syncdomain.FingerprintAttachment(projectKey, attachment) + if err != nil { + return err + } + manifest.SetFingerprint(id, fingerprint) + } + return nil +} + +// has reports whether one resource identity is already tracked. +func (manifest Manifest) has(id syncdomain.ResourceID) bool { + return slices.ContainsFunc(manifest.Resources, func(resource Resource) bool { return resource.ID() == id }) +} + +// RemoveUnreferencedAttachments removes dependency baselines that no managed +// variation references after a successful synchronization. +func (manifest *Manifest) RemoveUnreferencedAttachments(referenced map[syncdomain.ResourceID]struct{}) { + manifest.Resources = slices.DeleteFunc(manifest.Resources, func(resource Resource) bool { + if resource.ResourceKind != syncdomain.KindTool && resource.ResourceKind != syncdomain.KindSkill { + return false + } + _, ok := referenced[resource.ID()] + return !ok + }) +} + // Remove deletes one resource from the manifest. func (manifest *Manifest) Remove(id syncdomain.ResourceID) { for index, resource := range manifest.Resources { diff --git a/internal/sync/manifest/model_test.go b/internal/sync/manifest/model_test.go index e1f9c3b3..3a3e04a2 100644 --- a/internal/sync/manifest/model_test.go +++ b/internal/sync/manifest/model_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" syncdomain "github.com/launchdarkly/ldcli/internal/sync" ) @@ -25,3 +26,55 @@ func TestManifestSetFingerprintAndRemove(t *testing.T) { Fingerprint: "sha256:second", }}, manifest.Resources) } + +func TestManifestTracksEachSharedAttachmentOnce(t *testing.T) { + description := "Search documentation" + attachment := syncdomain.Attachment{ + Kind: syncdomain.AttachmentTool, + Tool: &syncdomain.Tool{Key: "search", Description: &description, Schema: map[string]any{"type": "object"}}, + } + manifest := New() + + require.NoError(t, manifest.SetAttachments("project", []syncdomain.Attachment{attachment, attachment})) + + require.Len(t, manifest.Resources, 1) + assert.Equal(t, syncdomain.KindTool, manifest.Resources[0].ResourceKind) + assert.Equal(t, "search", manifest.Resources[0].LookupKey) +} + +func TestManifestPreservesExistingAttachmentBaselineWhenAddingConsumer(t *testing.T) { + description := "Current server content" + attachment := syncdomain.Attachment{ + Kind: syncdomain.AttachmentTool, + Tool: &syncdomain.Tool{Key: "search", Description: &description, Schema: map[string]any{"type": "object"}}, + } + manifest := New() + manifest.SetFingerprint( + syncdomain.ResourceID{Kind: syncdomain.KindTool, ProjectKey: "project", LookupKey: "search"}, + "sha256:existing", + ) + + require.NoError(t, manifest.SetAttachmentsIfMissing("project", []syncdomain.Attachment{attachment})) + + require.Len(t, manifest.Resources, 1) + assert.Equal(t, "sha256:existing", manifest.Resources[0].Fingerprint) +} + +func TestManifestRemovesOnlyUnreferencedAttachments(t *testing.T) { + manifest := New() + manifest.Resources = []Resource{ + {ResourceKind: syncdomain.KindVariation, ProjectKey: "project", LookupKey: "config/default"}, + {ResourceKind: syncdomain.KindTool, ProjectKey: "project", LookupKey: "search"}, + {ResourceKind: syncdomain.KindSkill, ProjectKey: "project", LookupKey: "support"}, + } + referenced := map[syncdomain.ResourceID]struct{}{ + {Kind: syncdomain.KindTool, ProjectKey: "project", LookupKey: "search"}: {}, + } + + manifest.RemoveUnreferencedAttachments(referenced) + + assert.Equal(t, []Resource{ + {ResourceKind: syncdomain.KindVariation, ProjectKey: "project", LookupKey: "config/default"}, + {ResourceKind: syncdomain.KindTool, ProjectKey: "project", LookupKey: "search"}, + }, manifest.Resources) +} diff --git a/internal/sync/prompt/acceptance_test.go b/internal/sync/prompt/acceptance_test.go index d800d7a9..54947666 100644 --- a/internal/sync/prompt/acceptance_test.go +++ b/internal/sync/prompt/acceptance_test.go @@ -3,6 +3,7 @@ package prompt_test import ( "encoding/json" "fmt" + "net/http" "net/url" "os" "os/exec" @@ -27,9 +28,21 @@ type directAPI struct { variation *syncdomain.Variation variationState string modelConfigs []syncapi.ModelConfig + tools map[string]versionedTool + skills map[string]versionedSkill requests []string } +type versionedTool struct { + syncdomain.Tool + Version int `json:"version"` +} + +type versionedSkill struct { + syncdomain.Skill + Version int `json:"version"` +} + type directAPIVariation struct { syncdomain.Variation State string `json:"state,omitempty"` @@ -47,6 +60,50 @@ func (api *directAPI) MakeRequest( _ bool, ) ([]byte, error) { api.requests = append(api.requests, method+" "+path) + if strings.HasSuffix(path, "/ai-tools") && method == http.MethodPost { + var tool versionedTool + if err := json.Unmarshal(body, &tool.Tool); err != nil { + return nil, err + } + tool.Version = 1 + if api.tools == nil { + api.tools = make(map[string]versionedTool) + } + api.tools[tool.Key] = tool + return json.Marshal(tool) + } + if strings.Contains(path, "/ai-tools/") { + key := path[strings.LastIndex(path, "/")+1:] + tool, ok := api.tools[key] + if !ok { + return nil, fmt.Errorf(`{"code":"not_found","message":"AI tool not found","statusCode":404}`) + } + if method == "PATCH" { + if err := json.Unmarshal(body, &tool.Tool); err != nil { + return nil, err + } + tool.Key = key + tool.Version++ + api.tools[key] = tool + } + return json.Marshal(tool) + } + if strings.Contains(path, "/ai-configs/skills/") { + key := path[strings.LastIndex(path, "/")+1:] + skill, ok := api.skills[key] + if !ok { + return nil, fmt.Errorf("skill not found") + } + if method == "PATCH" { + if err := json.Unmarshal(body, &skill.Skill); err != nil { + return nil, err + } + skill.Key = key + skill.Version++ + api.skills[key] = skill + } + return json.Marshal(skill) + } if method == "GET" && strings.Contains(path, "/model-configs/") { for _, modelConfig := range api.modelConfigs { if strings.HasSuffix(path, "/"+modelConfig.Key) { @@ -89,11 +146,13 @@ func (api *directAPI) MakeRequest( } var update struct { - Name string `json:"name"` - Instructions string `json:"instructions"` - ModelConfigKey string `json:"modelConfigKey"` - ModelConfigVersion int `json:"modelConfigVersion"` - Model map[string]any `json:"model"` + Name string `json:"name"` + Instructions string `json:"instructions"` + ModelConfigKey string `json:"modelConfigKey"` + ModelConfigVersion int `json:"modelConfigVersion"` + Model map[string]any `json:"model"` + Tools *[]syncdomain.AttachmentRef `json:"tools"` + Skills *[]syncdomain.AttachmentRef `json:"skills"` } if err := json.Unmarshal(body, &update); err != nil { return nil, err @@ -102,10 +161,21 @@ func (api *directAPI) MakeRequest( if api.variation != nil { key = api.variation.Key } + tools, skills := []syncdomain.AttachmentRef(nil), []syncdomain.AttachmentRef(nil) + if api.variation != nil { + tools, skills = api.variation.Tools, api.variation.Skills + } + if update.Tools != nil { + tools = *update.Tools + } + if update.Skills != nil { + skills = *update.Skills + } api.variation = &syncdomain.Variation{ Mode: syncdomain.VariationModeAgent, Key: key, Name: update.Name, Instructions: update.Instructions, ModelConfigKey: update.ModelConfigKey, ModelConfigVersion: update.ModelConfigVersion, Model: update.Model, + Tools: tools, Skills: skills, } api.variationState = "published" } @@ -172,6 +242,114 @@ func TestPromptFirstSyncCreatesUpsertVariation(t *testing.T) { requireOnlyReads(t, api.requests) } +func TestPromptCreatesMissingToolWhenUpsertIsEnabled(t *testing.T) { + root := initRepository(t) + baseline := variation("Baseline") + writeVariation(t, root, baseline, true) + writeManifest(t, root, baseline) + attachLocalTool(t, root, baseline, true) + api := &directAPI{variation: pointer(baseline), tools: map[string]versionedTool{}} + + _, _, err := runPrompt(t, root, api, "--yes") + + require.NoError(t, err) + require.Contains(t, api.tools, "my-second-tool") + assert.Equal(t, 1, api.tools["my-second-tool"].Version) + require.Equal(t, []syncdomain.AttachmentRef{{Key: "my-second-tool", Version: 1}}, api.variation.Tools) +} + +func TestPromptExplainsUpsertForMissingTool(t *testing.T) { + root := initRepository(t) + baseline := variation("Baseline") + writeVariation(t, root, baseline, true) + writeManifest(t, root, baseline) + attachLocalTool(t, root, baseline, false) + api := &directAPI{variation: pointer(baseline), tools: map[string]versionedTool{}} + + output, _, err := runPrompt(t, root, api, "--yes") + + require.ErrorContains(t, err, `add "upsert": true`) + assert.Contains(t, output, `add "upsert": true`) + assert.NotContains(t, err.Error(), "unknown error") +} + +func TestPromptVersionsChangedToolBeforeUpdatingVariation(t *testing.T) { + root := initRepository(t) + originalDescription := "Search documentation" + updatedDescription := "Search all documentation" + tool := syncdomain.Tool{ + Key: "search", Description: &originalDescription, + Schema: map[string]any{"type": "object"}, + } + baseline := variation("Baseline") + baseline.Tools = []syncdomain.AttachmentRef{{Key: "search", Version: 2}} + baseline.Attachments = []syncdomain.Attachment{toolAttachment(tool, 2)} + writeVariation(t, root, baseline, true) + writeManifest(t, root, baseline) + api := &directAPI{ + variation: pointer(baseline), + tools: map[string]versionedTool{ + "search": {Tool: tool, Version: 2}, + }, + } + toolPath := filepath.Join(root, ".launchdarkly", "production", "tools", "search.json") + require.NoError(t, os.WriteFile(toolPath, []byte(`{ + "formatVersion": 1, + "key": "search", + "description": "Search all documentation", + "schema": {"type": "object"} +} +`), 0o644)) + + _, _, err := runPrompt(t, root, api, "--yes") + + require.NoError(t, err) + assert.Equal(t, 3, api.tools["search"].Version) + assert.Equal(t, updatedDescription, *api.tools["search"].Description) + require.Equal(t, []syncdomain.AttachmentRef{{Key: "search", Version: 3}}, api.variation.Tools) + + expected := baseline + expected.Attachments[0].Tool.Description = &updatedDescription + assertManifestFingerprint(t, root, expected) +} + +func TestPromptPullsLatestToolAndAdvancesVariationPin(t *testing.T) { + root := initRepository(t) + oldDescription := "Search documentation" + newDescription := "Search all documentation" + oldTool := syncdomain.Tool{ + Key: "search", Description: &oldDescription, + Schema: map[string]any{"type": "object"}, + } + latestTool := oldTool + latestTool.Description = &newDescription + baseline := variation("Baseline") + baseline.Tools = []syncdomain.AttachmentRef{{Key: "search", Version: 2}} + baseline.Attachments = []syncdomain.Attachment{toolAttachment(oldTool, 2)} + writeVariation(t, root, baseline, true) + writeManifest(t, root, baseline) + serverVariation := baseline + serverVariation.Attachments = nil + api := &directAPI{ + variation: &serverVariation, + tools: map[string]versionedTool{ + "search": {Tool: latestTool, Version: 3}, + }, + } + + _, _, err := runPrompt(t, root, api, "--yes") + + require.NoError(t, err) + require.Equal(t, []syncdomain.AttachmentRef{{Key: "search", Version: 3}}, api.variation.Tools) + content, err := os.ReadFile(filepath.Join(root, ".launchdarkly", "production", "tools", "search.json")) + require.NoError(t, err) + assert.Contains(t, string(content), "Search all documentation") + + expected := baseline + expected.Attachments[0] = toolAttachment(latestTool, 3) + assertManifestFingerprint(t, root, expected) +} + func TestPromptUpdateResolvesOmittedModelConfigVersionToLatest(t *testing.T) { root := initRepository(t) baseline := variation("Matching") @@ -570,6 +748,27 @@ func writeVariation(t *testing.T, root string, value syncdomain.Variation, upser require.NoError(t, err) } +func attachLocalTool(t *testing.T, root string, variation syncdomain.Variation, upsert bool) { + t.Helper() + variation.Tools = []syncdomain.AttachmentRef{{Key: "my-second-tool"}} + _, err := synclocal.NewStore(root).ReplaceVariations([]synclocal.VariationReplacement{{ + ProjectKey: "production", ConfigKey: "support", Variation: variation, + }}) + require.NoError(t, err) + + toolPath := filepath.Join(root, ".launchdarkly", "production", "tools", "my-second-tool.json") + require.NoError(t, os.MkdirAll(filepath.Dir(toolPath), 0o755)) + content := fmt.Sprintf(`{ + "formatVersion": 1, + "upsert": %t, + "key": "my-second-tool", + "description": "This is a test", + "schema": {"type": "object"} +} +`, upsert) + require.NoError(t, os.WriteFile(toolPath, []byte(content), 0o644)) +} + func writeLinkedVariation(t *testing.T, root string, value syncdomain.Variation, content string) { t.Helper() referencePath := filepath.Join(root, "prompts", value.Key+".md") @@ -595,19 +794,14 @@ func writeManifest(t *testing.T, root string, value syncdomain.Variation) { func writeManifestResources(t *testing.T, root string, values ...syncdomain.Variation) { t.Helper() - resources := make([]syncmanifest.Resource, 0, len(values)) + manifest := syncmanifest.New() for _, value := range values { - resources = append(resources, syncmanifest.Resource{ - ResourceKind: syncdomain.KindVariation, - ProjectKey: "production", - LookupKey: "support/" + value.Key, - Fingerprint: fingerprint(t, value), - }) + require.NoError(t, manifest.SetAttachments("production", value.Attachments)) + manifest.SetFingerprint(syncdomain.ResourceID{ + Kind: syncdomain.KindVariation, ProjectKey: "production", LookupKey: "support/" + value.Key, + }, fingerprint(t, value)) } - require.NoError(t, syncmanifest.NewStore(root).Write(syncmanifest.Manifest{ - FormatVersion: syncmanifest.FormatVersion, - Resources: resources, - })) + require.NoError(t, syncmanifest.NewStore(root).Write(manifest)) } func assertManifestFingerprint(t *testing.T, root string, value syncdomain.Variation) { @@ -619,10 +813,28 @@ func assertManifestFingerprints(t *testing.T, root string, values ...syncdomain. manifest, exists, err := syncmanifest.NewStore(root).Load() require.NoError(t, err) require.True(t, exists) - require.Len(t, manifest.Resources, len(values)) - for index, value := range values { - assert.Equal(t, fingerprint(t, value), manifest.Resources[index].Fingerprint) + variations := make(map[string]string) + attachments := make(map[syncdomain.ResourceID]string) + for _, resource := range manifest.Resources { + if resource.ResourceKind == syncdomain.KindVariation { + variations[resource.LookupKey] = resource.Fingerprint + } else { + attachments[resource.ID()] = resource.Fingerprint + } + } + require.Len(t, variations, len(values)) + expectedAttachments := make(map[syncdomain.ResourceID]string) + for _, value := range values { + assert.Equal(t, fingerprint(t, value), variations["support/"+value.Key]) + for _, attachment := range value.Attachments { + id := syncdomain.ResourceID{ + Kind: syncdomain.Kind(attachment.Kind), ProjectKey: "production", LookupKey: attachment.Key(), + } + expectedAttachments[id], err = syncdomain.FingerprintAttachment("production", attachment) + require.NoError(t, err) + } } + assert.Equal(t, expectedAttachments, attachments) } func requireLocalModelConfigVersion(t *testing.T, root string, expected int) { @@ -661,6 +873,10 @@ func variationWithKey(key, name string) syncdomain.Variation { } } +func toolAttachment(tool syncdomain.Tool, version int) syncdomain.Attachment { + return syncdomain.Attachment{Kind: syncdomain.AttachmentTool, Version: version, Tool: &tool} +} + func pointer[T any](value T) *T { return &value } diff --git a/internal/sync/prompt/conflict.go b/internal/sync/prompt/conflict.go index 24a40c8e..ebcf15bb 100644 --- a/internal/sync/prompt/conflict.go +++ b/internal/sync/prompt/conflict.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "slices" "time" syncconsole "github.com/launchdarkly/ldcli/internal/sync/console" @@ -32,6 +33,11 @@ type conflictChoice struct { sourcesChanged bool } +type conflictGroup struct { + resources []PlannedResource + attachment bool +} + // resolveConflicts shows each conflict and asks which side should win. func resolveConflicts( options Options, @@ -40,12 +46,7 @@ func resolveConflicts( interactive bool, watched *watchedSources, ) (conflictResolutionResult, error) { - var conflicts []PlannedResource - for _, resource := range plan.Resources { - if resource.Action == ActionConflict { - conflicts = append(conflicts, resource) - } - } + conflicts := groupConflicts(plan) if len(conflicts) == 0 { return conflictResolutionResult{}, nil } @@ -55,12 +56,28 @@ func resolveConflicts( ) } - result := conflictResolutionResult{resolutions: make(map[ResourceID]conflictResolution, len(conflicts))} - for _, resource := range conflicts { - conflict := Plan{Resources: []PlannedResource{resource}} + result := conflictResolutionResult{resolutions: make(map[ResourceID]conflictResolution)} + for _, group := range conflicts { + visible := make([]PlannedResource, 0, len(group.resources)) + for _, resource := range group.resources { + if len(resource.Diff) != 0 { + visible = append(visible, resource) + } + } + if len(visible) == 0 { + visible = append(visible, group.resources[0]) + } + conflict := Plan{Resources: visible} if err := writePlanReview(options.ErrorOutput, "plaintext", conflict, terminalWidth(options.ErrorOutput)); err != nil { return conflictResolutionResult{}, err } + if group.attachment && len(group.resources) > 1 { + console := syncconsole.New(options.ErrorOutput) + _ = console.Printf("This attachment conflict affects %d variations:\n", len(group.resources)) + for _, resource := range group.resources { + _ = console.Printf("- %s/%s\n", resource.ID.ProjectKey, resource.ID.LookupKey) + } + } choice, err := readConflictChoice(options, reader, watched) if err != nil { @@ -74,11 +91,56 @@ func resolveConflicts( result.aborted = true return result, nil } - result.resolutions[resource.ID] = choice.resolution + for _, resource := range group.resources { + result.resolutions[resource.ID] = choice.resolution + } } return result, nil } +// groupConflicts presents one choice for each connected set of variations that +// share a changed dependency, including conflicts with other local edits. +func groupConflicts(plan Plan) []conflictGroup { + var groups []conflictGroup + visited := make([]bool, len(plan.Resources)) + for start := range plan.Resources { + if visited[start] || plan.Resources[start].Action != ActionConflict { + continue + } + visited[start] = true + indices := []int{start} + for next := 0; next < len(indices); next++ { + for candidate := range plan.Resources { + if visited[candidate] || + plan.Resources[candidate].Action == ActionError || + !sharesChangedAttachment(plan.Resources[indices[next]], plan.Resources[candidate]) { + continue + } + visited[candidate] = true + indices = append(indices, candidate) + } + } + + group := conflictGroup{attachment: len(indices) > 1} + for _, index := range indices { + group.resources = append(group.resources, plan.Resources[index]) + } + groups = append(groups, group) + } + return groups +} + +// sharesChangedAttachment identifies variations that must resolve their shared +// dependency in the same direction. +func sharesChangedAttachment(left, right PlannedResource) bool { + for _, leftID := range left.changedAttachments { + if slices.Contains(right.changedAttachments, leftID) { + return true + } + } + return false +} + // readConflictChoice waits for a regular terminal choice or a watch-aware // choice that can be interrupted by another source change. func readConflictChoice(options Options, reader io.Reader, watched *watchedSources) (conflictChoice, error) { @@ -89,11 +151,7 @@ func readConflictChoice(options Options, reader io.Reader, watched *watchedSourc } // promptConflictResolution asks which side should win one conflict. -func promptConflictResolution( - ctx context.Context, - input io.Reader, - output io.Writer, -) (conflictChoice, error) { +func promptConflictResolution(ctx context.Context, input io.Reader, output io.Writer) (conflictChoice, error) { resolution, canceled, err := syncinteractive.SelectContext( ctx, input, @@ -193,13 +251,16 @@ func (watched watchedSources) WaitForChange(ctx context.Context) error { } } -// applyConflictResolutions replaces conflict actions with the selected direction. +// applyConflictResolutions applies one direction to every resource in each +// conflict group, including non-conflicted consumers of a shared attachment. func applyConflictResolutions(plan Plan, resolutions map[ResourceID]conflictResolution) Plan { + // Clone the resource slice before changing actions so the reviewed plan + // remains an immutable record for post-review revalidation. resolved := Plan{Resources: append([]PlannedResource(nil), plan.Resources...)} for index := range resolved.Resources { resource := &resolved.Resources[index] resolution, ok := resolutions[resource.ID] - if !ok || resource.Action != ActionConflict { + if !ok { continue } resource.Action = resolvedConflictAction(*resource, resolution) diff --git a/internal/sync/prompt/conflict_test.go b/internal/sync/prompt/conflict_test.go index f1241a6c..9aa48382 100644 --- a/internal/sync/prompt/conflict_test.go +++ b/internal/sync/prompt/conflict_test.go @@ -43,15 +43,20 @@ func TestResolvedConflictAction(t *testing.T) { func TestApplyConflictResolutionsDoesNotChangeReviewedPlan(t *testing.T) { id := testResourceID() + sharedID := id + sharedID.LookupKey = "config/shared" local, server := testVariation("local"), testVariation("server") - reviewed := Plan{Resources: []PlannedResource{{ - ID: id, Action: ActionConflict, Local: &local, Server: &server, - }}} + reviewed := Plan{Resources: []PlannedResource{ + {ID: id, Action: ActionConflict, Local: &local, Server: &server}, + {ID: sharedID, Action: ActionUpdateServer, Local: &local, Server: &server}, + }} - resolved := applyConflictResolutions(reviewed, map[ResourceID]conflictResolution{id: useLocal}) + resolved := applyConflictResolutions(reviewed, map[ResourceID]conflictResolution{id: useLaunchDarkly, sharedID: useLaunchDarkly}) assert.Equal(t, ActionConflict, reviewed.Resources[0].Action) - assert.Equal(t, ActionUpdateServer, resolved.Resources[0].Action) + assert.Equal(t, ActionUpdateServer, reviewed.Resources[1].Action) + assert.Equal(t, ActionUpdateLocal, resolved.Resources[0].Action) + assert.Equal(t, ActionUpdateLocal, resolved.Resources[1].Action) } func TestApplyLocalChangeRestoresMissingConflictFile(t *testing.T) { @@ -105,6 +110,68 @@ func TestResolveConflictsRequiresTerminalEvenWithYes(t *testing.T) { require.ErrorContains(t, err, "interactive conflict resolution requires a terminal") } +func TestGroupConflictsDeduplicatesSharedAttachment(t *testing.T) { + server := testVariation("support") + local := server + server.Tools = []syncdomain.AttachmentRef{{Key: "search"}} + local.Tools = []syncdomain.AttachmentRef{{Key: "search"}} + server.Attachments = []syncdomain.Attachment{{ + Kind: syncdomain.AttachmentTool, + Tool: &syncdomain.Tool{Key: "search", Schema: map[string]any{"type": "object"}}, + }} + local.Attachments = []syncdomain.Attachment{{ + Kind: syncdomain.AttachmentTool, + Tool: &syncdomain.Tool{Key: "search", Schema: map[string]any{"type": "string"}}, + }} + local.Name = "Locally renamed" + first := PlannedResource{ + ID: ResourceID{Kind: syncdomain.KindVariation, ProjectKey: "project", LookupKey: "config/first"}, + Action: ActionConflict, Local: &local, Server: &server, Diff: variationDiff(&server, &local), + } + first.changedAttachments = changedAttachmentIDs(first) + second := first + second.ID.LookupKey = "config/second" + + groups := groupConflicts(Plan{Resources: []PlannedResource{first, second}}) + + require.Len(t, groups, 1) + assert.True(t, groups[0].attachment) + assert.Len(t, groups[0].resources, 2) +} + +func TestGroupConflictsConnectsMixedChangesAcrossSharedAttachments(t *testing.T) { + tool := attachmentID{projectKey: "project", kind: syncdomain.AttachmentTool, key: "search"} + skill := attachmentID{projectKey: "project", kind: syncdomain.AttachmentSkill, key: "support"} + resources := []PlannedResource{ + { + ID: ResourceID{Kind: syncdomain.KindVariation, ProjectKey: "project", LookupKey: "config/first"}, + Action: ActionConflict, changedAttachments: []attachmentID{tool}, + }, + { + ID: ResourceID{Kind: syncdomain.KindVariation, ProjectKey: "project", LookupKey: "config/second"}, + Action: ActionConflict, changedAttachments: []attachmentID{tool, skill}, + }, + { + ID: ResourceID{Kind: syncdomain.KindVariation, ProjectKey: "project", LookupKey: "config/third"}, + Action: ActionConflict, changedAttachments: []attachmentID{skill}, + }, + { + ID: ResourceID{Kind: syncdomain.KindVariation, ProjectKey: "project", LookupKey: "config/fourth"}, + Action: ActionUpdateServer, changedAttachments: []attachmentID{skill}, + }, + { + ID: ResourceID{Kind: syncdomain.KindVariation, ProjectKey: "project", LookupKey: "config/error"}, + Action: ActionError, changedAttachments: []attachmentID{skill}, + }, + } + + groups := groupConflicts(Plan{Resources: resources}) + + require.Len(t, groups, 1) + assert.True(t, groups[0].attachment) + assert.Len(t, groups[0].resources, 4) +} + func TestRunWorkspaceSyncAppliesConflictChoiceAfterRevalidation(t *testing.T) { root := t.TempDir() baseline, local, server := testVariation("baseline"), testVariation("local"), testVariation("server") @@ -154,6 +221,96 @@ func TestRunWorkspaceSyncAbortsConflictWithoutWriting(t *testing.T) { assert.Contains(t, output.String(), "Sync canceled; conflict left unresolved.") } +func TestRunWorkspaceSyncAbortsAttachmentConflictWithoutWriting(t *testing.T) { + root := t.TempDir() + baseline, local, server := attachmentConflictVariations() + localStore, manifestStore := writeConflictWorkspace(t, root, baseline, local) + api := &conflictAPI{variation: &server, tool: server.Attachments[0]} + runner := NewRunner(api) + input := strings.NewReader("\x1b[B\x1b[B\r") + runner.isTerminal = func(actual io.Reader, _ io.Writer) bool { return actual == input } + var output bytes.Buffer + + err := runner.runWorkspaceSync( + Options{ + AccessToken: "token", BaseURI: "https://example.test", Yes: true, Input: input, + Output: &output, ErrorOutput: &output, + }, + syncWorkspace{root: root, local: localStore, manifest: manifestStore}, + ) + + require.NoError(t, err) + assert.NotContains(t, api.requests, "PATCH") + assert.Equal(t, "Changed in LaunchDarkly", *api.tool.Tool.Description) + + resources, err := synclocal.CompileWorkspace(root) + require.NoError(t, err) + require.Len(t, resources, 1) + require.Len(t, resources[0].Attachments, 1) + assert.Equal(t, "Changed locally", *resources[0].Attachments[0].Tool.Description) + assert.Contains(t, output.String(), "Sync canceled; conflict left unresolved.") +} + +func TestRunWorkspaceSyncUsesLaunchDarklyForAttachmentConflict(t *testing.T) { + root := t.TempDir() + baseline, local, server := attachmentConflictVariations() + localStore, manifestStore := writeConflictWorkspace(t, root, baseline, local) + api := &conflictAPI{variation: &server, tool: server.Attachments[0]} + runner := NewRunner(api) + input := strings.NewReader("\r") + runner.isTerminal = func(actual io.Reader, _ io.Writer) bool { return actual == input } + var output bytes.Buffer + + err := runner.runWorkspaceSync( + Options{ + AccessToken: "token", BaseURI: "https://example.test", Yes: true, Input: input, + Output: &output, ErrorOutput: &output, + }, + syncWorkspace{root: root, local: localStore, manifest: manifestStore}, + ) + + require.NoError(t, err) + assert.NotContains(t, api.requests, "PATCH") + assert.Equal(t, "Changed in LaunchDarkly", *api.tool.Tool.Description) + + resources, err := synclocal.CompileWorkspace(root) + require.NoError(t, err) + require.Len(t, resources, 1) + require.Len(t, resources[0].Attachments, 1) + assert.Equal(t, "Changed in LaunchDarkly", *resources[0].Attachments[0].Tool.Description) + assert.Contains(t, output.String(), "Using LaunchDarkly.") +} + +func attachmentConflictVariations() (syncdomain.Variation, syncdomain.Variation, syncdomain.Variation) { + description := "Baseline" + tool := syncdomain.Tool{ + Key: "search", Description: &description, Schema: map[string]any{"type": "object"}, + } + baseline := testVariation("support") + baseline.Tools = []syncdomain.AttachmentRef{{Key: "search", Version: 1}} + baseline.Attachments = []syncdomain.Attachment{{ + Kind: syncdomain.AttachmentTool, Version: 1, Tool: &tool, + }} + + local := baseline + localTool := tool + localDescription := "Changed locally" + localTool.Description = &localDescription + local.Attachments = []syncdomain.Attachment{{ + Kind: syncdomain.AttachmentTool, Tool: &localTool, + }} + + server := baseline + serverTool := tool + serverDescription := "Changed in LaunchDarkly" + serverTool.Description = &serverDescription + server.Tools = []syncdomain.AttachmentRef{{Key: "search", Version: 2}} + server.Attachments = []syncdomain.Attachment{{ + Kind: syncdomain.AttachmentTool, Version: 2, Tool: &serverTool, + }} + return baseline, local, server +} + func divergentPlan(t *testing.T) Plan { t.Helper() @@ -200,19 +357,34 @@ func writeConflictWorkspace(t *testing.T, root string, baseline, local syncdomai type conflictAPI struct { variation *syncdomain.Variation + tool syncdomain.Attachment requests []string } func (api *conflictAPI) MakeRequest( _ string, method string, - _ string, + path string, _ string, _ url.Values, body []byte, _ bool, ) ([]byte, error) { api.requests = append(api.requests, method) + if strings.Contains(path, "/ai-tools/") { + if method == "PATCH" { + var tool syncdomain.Tool + if err := json.Unmarshal(body, &tool); err != nil { + return nil, err + } + api.tool.Tool = &tool + api.tool.Version++ + } + return json.Marshal(struct { + syncdomain.Tool + Version int `json:"version"` + }{Tool: *api.tool.Tool, Version: api.tool.Version}) + } if method == "GET" { return json.Marshal(map[string]any{ "key": "support", "name": "Support", "mode": "agent", "variations": []syncdomain.Variation{*api.variation}, diff --git a/internal/sync/prompt/execute.go b/internal/sync/prompt/execute.go index dd2310e0..9da8624b 100644 --- a/internal/sync/prompt/execute.go +++ b/internal/sync/prompt/execute.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "fmt" + "slices" "strings" syncdomain "github.com/launchdarkly/ldcli/internal/sync" @@ -12,6 +13,151 @@ import ( syncmanifest "github.com/launchdarkly/ldcli/internal/sync/manifest" ) +// attachmentResolver versions shared dependencies once per execution and +// reuses both successful results and failures across every consumer. +type attachmentResolver struct { + client syncapi.Client + resolved map[attachmentID]syncdomain.Attachment + failures map[attachmentID]error +} + +func newAttachmentResolver(client syncapi.Client) *attachmentResolver { + return &attachmentResolver{ + client: client, + resolved: make(map[attachmentID]syncdomain.Attachment), + failures: make(map[attachmentID]error), + } +} + +func (resolver *attachmentResolver) resolveVariation(projectKey string, variation syncdomain.Variation) (syncdomain.Variation, error) { + // A value copy still shares slice backing arrays with the reviewed plan; + // clone pins before assigning server versions so review state stays immutable. + variation.Tools = slices.Clone(variation.Tools) + variation.Skills = slices.Clone(variation.Skills) + if err := resolver.resolveReferences(projectKey, syncdomain.AttachmentTool, variation.Tools, variation); err != nil { + return syncdomain.Variation{}, err + } + if err := resolver.resolveReferences(projectKey, syncdomain.AttachmentSkill, variation.Skills, variation); err != nil { + return syncdomain.Variation{}, err + } + return variation, nil +} + +func (resolver *attachmentResolver) resolveReferences( + projectKey string, + kind syncdomain.AttachmentKind, + references []syncdomain.AttachmentRef, + variation syncdomain.Variation, +) error { + for index := range references { + local, ok := variation.Attachment(kind, references[index].Key) + if !ok { + return fmt.Errorf("%s %q content is missing", kind, references[index].Key) + } + resolved, err := resolver.resolve(projectKey, local) + if err != nil { + return err + } + references[index].Version = resolved.Version + } + return nil +} + +func (resolver *attachmentResolver) resolve(projectKey string, local syncdomain.Attachment) (syncdomain.Attachment, error) { + id := attachmentID{projectKey: projectKey, kind: local.Kind, key: local.Key()} + if err, failed := resolver.failures[id]; failed { + return syncdomain.Attachment{}, err + } + if attachment, ok := resolver.resolved[id]; ok { + return attachment, nil + } + + remote, err := resolver.client.ReadAttachment(projectKey, local.Kind, local.Key()) + var mutationErr error + switch { + case err == nil && sameAttachmentContent(local, remote): + resolver.resolved[id] = remote + return remote, nil + case err == nil: + mutationErr = resolver.client.UpdateAttachment(projectKey, local) + case syncapi.IsNotFound(err) && local.Kind == syncdomain.AttachmentTool && local.Upsert: + mutationErr = resolver.client.CreateAttachment(projectKey, local) + case syncapi.IsNotFound(err) && local.Kind == syncdomain.AttachmentTool: + err = fmt.Errorf( + "tool %q does not exist in LaunchDarkly; add \"upsert\": true to its local JSON file to create it during sync", + local.Key(), + ) + resolver.failures[id] = err + return syncdomain.Attachment{}, err + default: + resolver.failures[id] = err + return syncdomain.Attachment{}, err + } + + if mutationErr != nil && !syncapi.MutationMayHaveSucceeded(mutationErr) { + resolver.failures[id] = mutationErr + return syncdomain.Attachment{}, mutationErr + } + + // Version allocation belongs to LaunchDarkly, so trust only a subsequent + // read. It also verifies writes whose response was lost or malformed. + observed, err := resolver.client.ReadAttachment(projectKey, local.Kind, local.Key()) + if err != nil { + err = errors.Join(mutationErr, err) + resolver.failures[id] = err + return syncdomain.Attachment{}, err + } + if !sameAttachmentContent(local, observed) { + err = mutationErr + if err == nil { + err = fmt.Errorf("%s %q changed concurrently", local.Kind, local.Key()) + } + resolver.failures[id] = err + return syncdomain.Attachment{}, err + } + resolver.resolved[id] = observed + return observed, nil +} + +func sameAttachmentContent(left, right syncdomain.Attachment) bool { + leftJSON, _ := json.Marshal(syncdomain.CanonicalAttachment(left)) + rightJSON, _ := json.Marshal(syncdomain.CanonicalAttachment(right)) + return string(leftJSON) == string(rightJSON) +} + +// variationForServerUpdate distinguishes omitted attachment fields from an +// explicit request to detach every existing item. +func variationForServerUpdate(local syncdomain.Variation, server *syncdomain.Variation) syncdomain.Variation { + if server == nil { + return local + } + if local.Tools == nil && len(server.Tools) != 0 { + local.Tools = []syncdomain.AttachmentRef{} + } + if local.Skills == nil && len(server.Skills) != 0 { + local.Skills = []syncdomain.AttachmentRef{} + } + return local +} + +func variationPinnedToLatest(variation syncdomain.Variation) syncdomain.Variation { + // A value copy still shares slice backing arrays with the reviewed plan; + // clone pins before assigning latest versions. + variation.Tools = slices.Clone(variation.Tools) + variation.Skills = slices.Clone(variation.Skills) + for index := range variation.Tools { + if attachment, ok := variation.Attachment(syncdomain.AttachmentTool, variation.Tools[index].Key); ok { + variation.Tools[index].Version = attachment.Version + } + } + for index := range variation.Skills { + if attachment, ok := variation.Attachment(syncdomain.AttachmentSkill, variation.Skills[index].Key); ok { + variation.Skills[index].Version = attachment.Version + } + } + return variation +} + // executePlan applies each independently executable resource and advances the // manifest only for resources that succeed. func executePlan( @@ -21,12 +167,20 @@ func executePlan( manifest syncmanifest.Manifest, plan Plan, ) ([]ResourceOutcome, syncmanifest.Manifest, error) { + // Conflict resolution is a plan-wide decision. Refuse every mutation until + // all conflicts have a direction so a shared attachment cannot advance + // while one of its consumers remains unresolved. + if err := plan.BlockingError(); err != nil { + return nil, manifest, err + } + // Keep the reviewed baseline immutable while successful resources advance // the result manifest independently. manifest.Resources = append([]syncmanifest.Resource(nil), manifest.Resources...) outcomes := make([]ResourceOutcome, 0, len(plan.Resources)) var failures []error + attachments := newAttachmentResolver(client) for _, resource := range plan.Resources { outcome := ResourceOutcome{ID: resource.ID, Action: resource.Action, Status: OutcomeSucceeded} @@ -39,7 +193,7 @@ func executePlan( case ActionRemoveManifest: manifest.Remove(resource.ID) case ActionCreateServer, ActionUpdateServer, ActionArchiveServer, ActionUpdateLocal, ActionDeleteLocal: - if err := applyResourceChange(repositoryRoot, localStore, client, resource); err != nil { + if err := applyResourceChange(repositoryRoot, localStore, client, attachments, resource); err != nil { outcome.Status, outcome.Error = OutcomeFailed, err.Error() failures = append(failures, fmt.Errorf("%s/%s: %w", resource.ID.ProjectKey, resource.ID.LookupKey, err)) break @@ -49,22 +203,85 @@ func executePlan( outcome.Status, outcome.Error = OutcomeSkipped, "resource is not executable" } + if outcome.Status == OutcomeSucceeded { + if variation := attachmentManifestState(resource); variation != nil { + if err := manifest.SetAttachments(resource.ID.ProjectKey, variation.Attachments); err != nil { + outcome.Status, outcome.Error = OutcomeFailed, err.Error() + failures = append(failures, err) + } + } + } outcomes = append(outcomes, outcome) } + if len(failures) == 0 { + if err := pruneAttachmentManifest(repositoryRoot, &manifest); err != nil { + failures = append(failures, err) + } + } return outcomes, manifest, errors.Join(failures...) } +func attachmentManifestState(resource PlannedResource) *syncdomain.Variation { + switch resource.Action { + case ActionInSync, ActionUpdateManifest, ActionCreateServer, ActionUpdateServer: + return resource.Local + case ActionUpdateLocal: + return resource.Server + default: + return nil + } +} + +func pruneAttachmentManifest(repositoryRoot string, manifest *syncmanifest.Manifest) error { + resources, err := synclocal.CompileWorkspace(repositoryRoot) + if errors.Is(err, synclocal.ErrNoDirectory) { + resources = nil + err = nil + } + if err != nil { + return err + } + referenced := make(map[syncdomain.ResourceID]struct{}) + for _, resource := range resources { + for _, attachment := range resource.Attachments { + referenced[syncdomain.ResourceID{ + Kind: syncdomain.Kind(attachment.Kind), + ProjectKey: resource.ProjectKey, + LookupKey: attachment.Key(), + }] = struct{}{} + } + } + manifest.RemoveUnreferencedAttachments(referenced) + return nil +} + // applyResourceChange applies one local or server mutation from the plan // revalidated after review. -func applyResourceChange(repositoryRoot string, localStore synclocal.Store, client syncapi.Client, resource PlannedResource) error { +func applyResourceChange( + repositoryRoot string, + localStore synclocal.Store, + client syncapi.Client, + attachments *attachmentResolver, + resource PlannedResource, +) error { if changesServer(resource.Action) { - return applyServerChange(client, resource) + return applyServerChange(client, attachments, resource) } if err := applyLocalChange(localStore, resource); err != nil { return err } - return verifyLocalResult(repositoryRoot, resource) + if err := verifyLocalResult(repositoryRoot, resource); err != nil { + return err + } + if resource.ServerHasStaleAttachmentPins && resource.Server != nil { + configKey, _, err := splitVariationLookupKey(resource.ID.LookupKey) + if err != nil { + return err + } + return client.UpdateVariation(resource.ID.ProjectKey, configKey, variationPinnedToLatest(*resource.Server)) + } + return nil } // recordSuccessfulChange updates the manifest to the state selected by the @@ -82,7 +299,7 @@ func recordSuccessfulChange(manifest *syncmanifest.Manifest, resource PlannedRes // applyServerChange performs one variation mutation through the existing // public config APIs. -func applyServerChange(client syncapi.Client, resource PlannedResource) error { +func applyServerChange(client syncapi.Client, attachments *attachmentResolver, resource PlannedResource) error { configKey, variationKey, err := splitVariationLookupKey(resource.ID.LookupKey) if err != nil { return err @@ -91,9 +308,20 @@ func applyServerChange(client syncapi.Client, resource PlannedResource) error { var mutationErr error switch resource.Action { case ActionCreateServer: - mutationErr = client.CreateVariation(resource.ID.ProjectKey, configKey, *resource.Local) + variation, err := attachments.resolveVariation(resource.ID.ProjectKey, *resource.Local) + if err != nil { + return err + } + mutationErr = client.CreateVariation(resource.ID.ProjectKey, configKey, variation) case ActionUpdateServer: - mutationErr = client.UpdateVariation(resource.ID.ProjectKey, configKey, *resource.Local) + variation, err := attachments.resolveVariation( + resource.ID.ProjectKey, + variationForServerUpdate(*resource.Local, resource.Server), + ) + if err != nil { + return err + } + mutationErr = client.UpdateVariation(resource.ID.ProjectKey, configKey, variation) case ActionArchiveServer: mutationErr = client.ArchiveVariation(resource.ID.ProjectKey, configKey, variationKey) } @@ -113,6 +341,9 @@ func applyServerChange(client syncapi.Client, resource PlannedResource) error { actualFingerprint := "" if state.Exists { + if err := hydrateServerAttachments(client, resource.ID.ProjectKey, &state.Variation); err != nil { + return errors.Join(mutationErr, err) + } actualFingerprint, readErr = syncdomain.FingerprintVariation(resource.ID.ProjectKey, resource.ID.LookupKey, state.Variation) if readErr != nil { return errors.Join(mutationErr, readErr) @@ -174,13 +405,15 @@ func readLocalFingerprint(repositoryRoot string, id ResourceID) (string, error) if err := json.Unmarshal(resource.Payload, &variation); err != nil { return "", err } + variation.Attachments = resource.Attachments return syncdomain.FingerprintVariation(id.ProjectKey, id.LookupKey, variation) } return "", nil } -// readServerResource reads one supported resource from LaunchDarkly. -func readServerResource(client syncapi.Client, id ResourceID) (ServerResource, error) { +// readServerResource reads one supported resource and hydrates its shared +// dependencies through the plan-local cache. +func readServerResource(client syncapi.Client, attachments *attachmentHydrator, id ResourceID) (ServerResource, error) { if id.Kind != syncdomain.KindVariation { return ServerResource{}, fmt.Errorf("unsupported sync resource kind %q", id.Kind) } @@ -195,6 +428,9 @@ func readServerResource(client syncapi.Client, id ResourceID) (ServerResource, e resource := ServerResource{ConfigMode: state.ConfigMode} if state.Exists { + if err := attachments.hydrate(id.ProjectKey, &state.Variation); err != nil { + return ServerResource{}, err + } resource.Variation = &state.Variation } return resource, nil diff --git a/internal/sync/prompt/execute_test.go b/internal/sync/prompt/execute_test.go index f910412e..de1738d7 100644 --- a/internal/sync/prompt/execute_test.go +++ b/internal/sync/prompt/execute_test.go @@ -1,6 +1,7 @@ package prompt import ( + "encoding/json" "errors" "net/url" "testing" @@ -47,12 +48,154 @@ func TestApplyServerChangeDoesNotRereadAfterDefinitiveAPIError(t *testing.T) { Local: &variation, } - err := applyServerChange(syncapi.NewClient(transport, "token", "https://example.com"), resource) + client := syncapi.NewClient(transport, "token", "https://example.com") + err := applyServerChange(client, newAttachmentResolver(client), resource) require.ErrorContains(t, err, `"statusCode":400`) assert.Zero(t, transport.reads) } +func TestExecutePlanDoesNotMutateWhileConflictIsUnresolved(t *testing.T) { + transport := &attachmentMutationAPI{ + current: syncdomain.Tool{Key: "search", Schema: map[string]any{"type": "object"}}, + version: 2, + } + client := syncapi.NewClient(transport, "token", "https://example.com") + variation := testVariation("local") + variation.Tools = []syncdomain.AttachmentRef{{Key: "search"}} + variation.Attachments = []syncdomain.Attachment{{ + Kind: syncdomain.AttachmentTool, + Tool: &syncdomain.Tool{Key: "search", Schema: map[string]any{"type": "string"}}, + }} + plan := Plan{Resources: []PlannedResource{ + { + ID: ResourceID{Kind: syncdomain.KindVariation, ProjectKey: "project", LookupKey: "config/first"}, + Action: ActionUpdateServer, Local: &variation, + }, + { + ID: ResourceID{Kind: syncdomain.KindVariation, ProjectKey: "project", LookupKey: "config/second"}, + Action: ActionConflict, Local: &variation, + }, + }} + + outcomes, _, err := executePlan("", synclocal.Store{}, client, syncmanifest.New(), plan) + + require.ErrorContains(t, err, "cannot sync conflicted resource") + assert.Empty(t, outcomes) + assert.Zero(t, transport.reads) + assert.Zero(t, transport.updates) +} + +func TestExecutePlanSkipsEveryConsumerWhenSharedAttachmentFails(t *testing.T) { + transport := &attachmentMutationAPI{ + current: syncdomain.Tool{Key: "search", Schema: map[string]any{"type": "object"}}, + version: 2, + updateErr: errors.New(`{"code":"invalid_request","statusCode":400}`), + } + client := syncapi.NewClient(transport, "token", "https://example.com") + description := "Search all documentation" + variation := syncdomain.Variation{ + Mode: syncdomain.VariationModeAgent, Key: "default", Name: "Support", + Tools: []syncdomain.AttachmentRef{{Key: "search"}}, + Attachments: []syncdomain.Attachment{{ + Kind: syncdomain.AttachmentTool, + Tool: &syncdomain.Tool{ + Key: "search", Description: &description, Schema: map[string]any{"type": "object"}, + }, + }}, + } + plan := Plan{Resources: []PlannedResource{ + { + ID: ResourceID{Kind: syncdomain.KindVariation, ProjectKey: "project", LookupKey: "config/first"}, + Action: ActionUpdateServer, Local: &variation, + }, + { + ID: ResourceID{Kind: syncdomain.KindVariation, ProjectKey: "project", LookupKey: "config/second"}, + Action: ActionUpdateServer, Local: &variation, + }, + }} + + originalManifest := syncmanifest.New() + originalManifest.SetFingerprint( + syncdomain.ResourceID{Kind: syncdomain.KindTool, ProjectKey: "project", LookupKey: "search"}, + "sha256:reviewed", + ) + outcomes, manifest, err := executePlan("", synclocal.Store{}, client, originalManifest, plan) + + require.Error(t, err) + require.Len(t, outcomes, 2) + assert.Equal(t, OutcomeFailed, outcomes[0].Status) + assert.Equal(t, OutcomeFailed, outcomes[1].Status) + require.Len(t, manifest.Resources, 1) + assert.Equal(t, "sha256:reviewed", manifest.Resources[0].Fingerprint) + assert.Equal(t, 1, transport.updates) + assert.Equal(t, 1, transport.reads) +} + +func TestAttachmentVersioningDoesNotMutateReviewedVariation(t *testing.T) { + transport := &attachmentMutationAPI{ + current: syncdomain.Tool{Key: "search", Schema: map[string]any{"type": "object"}}, + version: 2, + } + client := syncapi.NewClient(transport, "token", "https://example.com") + variation := testVariation("local") + variation.Tools = []syncdomain.AttachmentRef{{Key: "search", Version: 1}} + variation.Attachments = []syncdomain.Attachment{{ + Kind: syncdomain.AttachmentTool, + Version: 2, + Tool: &syncdomain.Tool{Key: "search", Schema: map[string]any{"type": "object"}}, + }} + + resolved, err := newAttachmentResolver(client).resolveVariation("project", variation) + require.NoError(t, err) + pinned := variationPinnedToLatest(variation) + + assert.Equal(t, 1, variation.Tools[0].Version) + assert.Equal(t, 2, resolved.Tools[0].Version) + assert.Equal(t, 2, pinned.Tools[0].Version) +} + +type attachmentMutationAPI struct { + current syncdomain.Tool + version int + reads int + updates int + updateErr error +} + +func (api *attachmentMutationAPI) MakeRequest( + _ string, + method string, + _ string, + _ string, + _ url.Values, + body []byte, + _ bool, +) ([]byte, error) { + switch method { + case "GET": + api.reads++ + case "PATCH": + api.updates++ + if api.updateErr != nil { + return nil, api.updateErr + } + if err := json.Unmarshal(body, &api.current); err != nil { + return nil, err + } + api.current.Key = "search" + api.version++ + } + return json.Marshal(struct { + syncdomain.Tool + Version int `json:"version"` + }{Tool: api.current, Version: api.version}) +} + +func (*attachmentMutationAPI) MakeUnauthenticatedRequest(string, string, []byte) ([]byte, error) { + return nil, nil +} + type definitiveMutationAPI struct { reads int } diff --git a/internal/sync/prompt/local_changes.go b/internal/sync/prompt/local_changes.go index 46e9de31..a54b2da7 100644 --- a/internal/sync/prompt/local_changes.go +++ b/internal/sync/prompt/local_changes.go @@ -15,7 +15,7 @@ func applyLocalChange(store synclocal.Store, resource PlannedResource) error { switch resource.Action { case ActionUpdateLocal: - variation := *resource.Server + variation := variationPinnedToLatest(*resource.Server) if resource.Local != nil && resource.LocalFollowsLatestModelConfig { variation.ModelConfigVersion = 0 } diff --git a/internal/sync/prompt/plan.go b/internal/sync/prompt/plan.go index 1ed1612c..362eccba 100644 --- a/internal/sync/prompt/plan.go +++ b/internal/sync/prompt/plan.go @@ -48,8 +48,10 @@ type PlannedResource struct { Local *syncdomain.Variation Server *syncdomain.Variation LocalFollowsLatestModelConfig bool + ServerHasStaleAttachmentPins bool Diff variationDiffFields Error string + changedAttachments []attachmentID } // Plan contains sync decisions in deterministic resource order. @@ -72,6 +74,9 @@ func BuildPlan(baseline syncmanifest.Manifest, local []syncdomain.SyncedResource baselineByID := make(map[ResourceID]string, len(baseline.Resources)) for _, resource := range baseline.Resources { + if resource.ResourceKind != syncdomain.KindVariation { + continue + } id := resource.ID() baselineByID[id] = resource.Fingerprint resourceIDs[id] = struct{}{} @@ -115,6 +120,7 @@ func buildPlannedResource( resource.Action, resource.Error = ActionError, fmt.Sprintf("decode local variation: %s", err) return resource } + variation.Attachments = localResource.Attachments if variation.Mode != serverResource.ConfigMode { resource.Action = ActionError resource.Error = fmt.Sprintf("local mode %q does not match config mode %q", variation.Mode, serverResource.ConfigMode) @@ -140,13 +146,111 @@ func buildPlannedResource( } resource.Action = chooseAction(tracked, resource) + currentPins, latestPins, stalePins := attachmentPinDiff(resource.Server) + resource.ServerHasStaleAttachmentPins = stalePins + if resource.ServerHasStaleAttachmentPins && + (resource.Action == ActionInSync || resource.Action == ActionUpdateManifest) { + resource.Action = ActionUpdateServer + } if resource.Action == ActionError { resource.Error = "variation does not exist in LaunchDarkly; set upsert: true to create it" } - resource.Diff = variationDiff(resource.Server, resource.Local) + // Diffs explain semantic changes. API-added defaults can make the raw JSON + // differ even when the canonical fingerprints—and therefore behavior—match. + if resource.LocalFingerprint != resource.ServerFingerprint { + resource.Diff = variationDiff(resource.Server, resource.Local) + } + resource.changedAttachments = changedAttachmentIDs(resource) + if resource.Action != ActionConflict && stalePins { + if resource.Diff == nil { + resource.Diff = variationDiffFields{} + } + resource.Diff["attachment versions"] = variationFieldDiff{Before: currentPins, After: latestPins} + } return resource } +// changedAttachmentIDs returns shared dependencies whose canonical local and +// server content differs for this variation. +func changedAttachmentIDs(resource PlannedResource) []attachmentID { + var conflicts []attachmentID + for _, kind := range []syncdomain.AttachmentKind{syncdomain.AttachmentTool, syncdomain.AttachmentSkill} { + for _, key := range changedAttachmentKeys(resource.Server, resource.Local, kind) { + conflicts = append(conflicts, attachmentID{ + projectKey: resource.ID.ProjectKey, + kind: kind, + key: key, + }) + } + } + return conflicts +} + +// changedAttachmentKeys compares one attachment kind by stable key. +func changedAttachmentKeys(before, after *syncdomain.Variation, kind syncdomain.AttachmentKind) []string { + beforeByKey := attachmentsByKey(before, kind) + afterByKey := attachmentsByKey(after, kind) + keys := make([]string, 0, len(beforeByKey)+len(afterByKey)) + + for key, attachment := range beforeByKey { + other, exists := afterByKey[key] + if !exists || !sameAttachmentContent(attachment, other) { + keys = append(keys, key) + } + } + for key := range afterByKey { + if _, exists := beforeByKey[key]; !exists { + keys = append(keys, key) + } + } + slices.Sort(keys) + return keys +} + +// attachmentsByKey indexes hydrated canonical content for comparison. +func attachmentsByKey(variation *syncdomain.Variation, kind syncdomain.AttachmentKind) map[string]syncdomain.Attachment { + attachments := map[string]syncdomain.Attachment{} + if variation == nil { + return attachments + } + for _, attachment := range variation.Attachments { + if attachment.Kind == kind { + attachments[attachment.Key()] = attachment + } + } + return attachments +} + +// attachmentPinDiff reports server references that do not use the latest +// hydrated version. +func attachmentPinDiff(variation *syncdomain.Variation) (json.RawMessage, json.RawMessage, bool) { + if variation == nil { + return nil, nil, false + } + current := map[string]map[string]int{"tools": {}, "skills": {}} + latest := map[string]map[string]int{"tools": {}, "skills": {}} + for _, ref := range variation.Tools { + if attachment, ok := variation.Attachment(syncdomain.AttachmentTool, ref.Key); ok && + attachment.Version != ref.Version { + current["tools"][ref.Key] = ref.Version + latest["tools"][ref.Key] = attachment.Version + } + } + for _, ref := range variation.Skills { + if attachment, ok := variation.Attachment(syncdomain.AttachmentSkill, ref.Key); ok && + attachment.Version != ref.Version { + current["skills"][ref.Key] = ref.Version + latest["skills"][ref.Key] = attachment.Version + } + } + if len(current["tools"]) == 0 && len(current["skills"]) == 0 { + return nil, nil, false + } + currentJSON, _ := json.Marshal(current) + latestJSON, _ := json.Marshal(latest) + return currentJSON, latestJSON, true +} + // RequiresConfirmation reports whether the plan changes local or server // resources. Manifest-only bookkeeping is safe to perform without prompting. func (plan Plan) RequiresConfirmation() bool { @@ -257,19 +361,68 @@ func variationDiff(before, after *syncdomain.Variation) variationDiffFields { if before == nil && after == nil { return nil } - beforeJSON, _ := json.Marshal(before) - afterJSON, _ := json.Marshal(after) + + fields := variationDiffFields{} + beforeJSON, _ := json.Marshal(variationForDiff(before)) + afterJSON, _ := json.Marshal(variationForDiff(after)) if before == nil { beforeJSON = nil } if after == nil { afterJSON = nil } - if string(beforeJSON) == string(afterJSON) { + if string(beforeJSON) != string(afterJSON) { + fields["variation"] = variationFieldDiff{Before: beforeJSON, After: afterJSON} + } + + beforeTools, beforeSkills := attachmentDiffValues(before) + afterTools, afterSkills := attachmentDiffValues(after) + if string(beforeTools) != string(afterTools) { + fields["tools"] = variationFieldDiff{Before: beforeTools, After: afterTools} + } + if string(beforeSkills) != string(afterSkills) { + fields["skills"] = variationFieldDiff{Before: beforeSkills, After: afterSkills} + } + if len(fields) == 0 { return nil } + return fields +} + +func variationForDiff(variation *syncdomain.Variation) *syncdomain.Variation { + if variation == nil { + return nil + } + normalized := *variation + // Attachments have their own content-aware diff. Omitting their references + // here prevents the same attach or detach operation appearing twice. + normalized.Tools = nil + normalized.Skills = nil + return &normalized +} - return variationDiffFields{ - "variation": {Before: beforeJSON, After: afterJSON}, +func attachmentDiffValues(variation *syncdomain.Variation) (json.RawMessage, json.RawMessage) { + if variation == nil { + return nil, nil + } + + var tools []syncdomain.Tool + var skills []syncdomain.Skill + for _, attachment := range variation.Attachments { + canonical := syncdomain.CanonicalAttachment(attachment) + if canonical.Tool != nil { + tools = append(tools, *canonical.Tool) + } else if canonical.Skill != nil { + skills = append(skills, *canonical.Skill) + } + } + + var toolJSON, skillJSON json.RawMessage + if len(tools) != 0 { + toolJSON, _ = json.Marshal(tools) + } + if len(skills) != 0 { + skillJSON, _ = json.Marshal(skills) } + return toolJSON, skillJSON } diff --git a/internal/sync/prompt/plan_test.go b/internal/sync/prompt/plan_test.go index b82ec26a..d21c1ca8 100644 --- a/internal/sync/prompt/plan_test.go +++ b/internal/sync/prompt/plan_test.go @@ -85,6 +85,44 @@ func TestBuildPlanFirstSync(t *testing.T) { } } +func TestBuildPlanDoesNotTreatManifestAttachmentsAsIndependentTargets(t *testing.T) { + manifest := syncmanifest.New() + manifest.Resources = []syncmanifest.Resource{{ + ResourceKind: syncdomain.KindTool, + ProjectKey: "project", + LookupKey: "search", + Fingerprint: "sha256:ignored", + }} + + plan := BuildPlan(manifest, nil, nil) + + require.Empty(t, plan.Resources) +} + +func TestBuildPlanOmitsDiffForEquivalentAPIDefaults(t *testing.T) { + local := testVariation("example") + local.Model = map[string]any{"modelName": "gpt-5"} + server := local + server.Model = map[string]any{ + "modelName": "gpt-5", + "custom": map[string]any{}, + "parameters": map[string]any{}, + } + + id := testResourceID() + fingerprint, err := syncdomain.FingerprintVariation(id.ProjectKey, id.LookupKey, server) + require.NoError(t, err) + manifest := syncmanifest.New() + manifest.SetFingerprint(id, fingerprint) + + plan := BuildPlan(manifest, localResources(&local, false), map[ResourceID]ServerResource{ + id: {Variation: &server, ConfigMode: syncdomain.VariationModeAgent}, + }) + + require.Equal(t, ActionInSync, plan.Resources[0].Action) + require.Empty(t, plan.Resources[0].Diff) +} + func TestBuildPlanRejectsParentConfigModeMismatch(t *testing.T) { local := testVariation("local") id := testResourceID() diff --git a/internal/sync/prompt/runner.go b/internal/sync/prompt/runner.go index f1ab5489..625576fd 100644 --- a/internal/sync/prompt/runner.go +++ b/internal/sync/prompt/runner.go @@ -7,6 +7,7 @@ import ( "io" "os" "os/signal" + "slices" "syscall" "github.com/launchdarkly/ldcli/internal/resources" @@ -53,6 +54,12 @@ type syncWorkspace struct { manifest syncmanifest.Store } +type attachmentID struct { + projectKey string + kind syncdomain.AttachmentKind + key string +} + // Runner coordinates prompt synchronization using existing config APIs. type Runner struct { client resources.Client @@ -128,13 +135,14 @@ func (runner Runner) Run(options Options) error { if !localDirectoryExists || options.Add { if err := runner.bootstrap(syncbootstrap.Options{ - Catalog: apiClient, - Store: workspace.local, - Manifest: workspace.manifest, - Input: options.Input, - Output: options.Output, - Initial: !localDirectoryExists, - DryRun: options.DryRun, + Catalog: apiClient, + Attachments: apiClient, + Store: workspace.local, + Manifest: workspace.manifest, + Input: options.Input, + Output: options.Output, + Initial: !localDirectoryExists, + DryRun: options.DryRun, }); err != nil { return err } @@ -303,12 +311,15 @@ func loadWorkspacePlan(repositoryRoot string, baseline syncmanifest.Manifest, cl resourceIDs[ResourceID{Kind: resource.Kind, ProjectKey: resource.ProjectKey, LookupKey: resource.LookupKey}] = struct{}{} } for _, resource := range baseline.Resources { - resourceIDs[resource.ID()] = struct{}{} + if resource.ResourceKind == syncdomain.KindVariation { + resourceIDs[resource.ID()] = struct{}{} + } } serverResources := make(map[ResourceID]ServerResource, len(resourceIDs)) + attachments := newAttachmentHydrator(client) for id := range resourceIDs { - resource, err := readServerResource(client, id) + resource, err := readServerResource(client, attachments, id) if err != nil { return Plan{}, err } @@ -321,6 +332,52 @@ func loadWorkspacePlan(repositoryRoot string, baseline syncmanifest.Manifest, cl return plan, nil } +// attachmentHydrator reads each shared dependency once while building a plan. +type attachmentHydrator struct { + client syncapi.Client + cache map[attachmentID]syncdomain.Attachment +} + +func newAttachmentHydrator(client syncapi.Client) *attachmentHydrator { + return &attachmentHydrator{client: client, cache: make(map[attachmentID]syncdomain.Attachment)} +} + +func (hydrator *attachmentHydrator) read(projectKey string, kind syncdomain.AttachmentKind, key string) (syncdomain.Attachment, error) { + id := attachmentID{projectKey: projectKey, kind: kind, key: key} + if attachment, ok := hydrator.cache[id]; ok { + return attachment, nil + } + attachment, err := hydrator.client.ReadAttachment(projectKey, kind, key) + if err != nil { + return syncdomain.Attachment{}, err + } + hydrator.cache[id] = attachment + return attachment, nil +} + +func (hydrator *attachmentHydrator) hydrate(projectKey string, variation *syncdomain.Variation) error { + variation.Attachments = make([]syncdomain.Attachment, 0, len(variation.Tools)+len(variation.Skills)) + for _, ref := range variation.Tools { + attachment, err := hydrator.read(projectKey, syncdomain.AttachmentTool, ref.Key) + if err != nil { + return err + } + variation.Attachments = append(variation.Attachments, attachment) + } + for _, ref := range variation.Skills { + attachment, err := hydrator.read(projectKey, syncdomain.AttachmentSkill, ref.Key) + if err != nil { + return err + } + variation.Attachments = append(variation.Attachments, attachment) + } + return variation.NormalizeAttachments() +} + +func hydrateServerAttachments(client syncapi.Client, projectKey string, variation *syncdomain.Variation) error { + return newAttachmentHydrator(client).hydrate(projectKey, variation) +} + // samePlanState reports whether every reviewed decision still has the same inputs. func samePlanState(reviewed, current Plan) bool { if len(reviewed.Resources) != len(current.Resources) { @@ -344,5 +401,16 @@ func samePlannedResourceState(reviewed, current PlannedResource) bool { reviewed.ServerFingerprint == current.ServerFingerprint && reviewed.ServerMode == current.ServerMode && reviewed.Upsert == current.Upsert && - reviewed.LocalFollowsLatestModelConfig == current.LocalFollowsLatestModelConfig + reviewed.LocalFollowsLatestModelConfig == current.LocalFollowsLatestModelConfig && + reviewed.ServerHasStaleAttachmentPins == current.ServerHasStaleAttachmentPins && + sameAttachmentPins(reviewed.Server, current.Server) +} + +// sameAttachmentPins compares exact server references because canonical +// fingerprints intentionally exclude runtime versions. +func sameAttachmentPins(reviewed, current *syncdomain.Variation) bool { + if reviewed == nil || current == nil { + return reviewed == current + } + return slices.Equal(reviewed.Tools, current.Tools) && slices.Equal(reviewed.Skills, current.Skills) } diff --git a/internal/sync/prompt/runner_test.go b/internal/sync/prompt/runner_test.go index 0579bcb3..0ec85e46 100644 --- a/internal/sync/prompt/runner_test.go +++ b/internal/sync/prompt/runner_test.go @@ -207,6 +207,21 @@ func TestValidateOptions(t *testing.T) { require.ErrorContains(t, validateOptions(Options{Detach: true, Add: true}), "--detach cannot be combined") } +func TestSamePlannedResourceStateDetectsChangedAttachmentPins(t *testing.T) { + reviewedVariation := testVariation("reviewed") + reviewedVariation.Tools = []syncdomain.AttachmentRef{{Key: "search", Version: 1}} + currentVariation := reviewedVariation + currentVariation.Tools = []syncdomain.AttachmentRef{{Key: "search", Version: 2}} + reviewed := PlannedResource{ + ID: ResourceID{Kind: syncdomain.KindVariation, ProjectKey: "project", LookupKey: "config/default"}, + Action: ActionUpdateServer, Server: &reviewedVariation, ServerHasStaleAttachmentPins: true, + } + current := reviewed + current.Server = ¤tVariation + + assert.False(t, samePlannedResourceState(reviewed, current)) +} + type noopResourceClient struct{} var _ resources.Client = noopResourceClient{} diff --git a/internal/sync/resource.go b/internal/sync/resource.go index 44fba785..b646ede2 100644 --- a/internal/sync/resource.go +++ b/internal/sync/resource.go @@ -2,6 +2,8 @@ package sync import ( "encoding/json" + "fmt" + "slices" "strings" ) @@ -13,6 +15,8 @@ type Kind string const ( KindVariation Kind = "variation" + KindTool Kind = "tool" + KindSkill Kind = "skill" ) // ResourceID uniquely identifies a synchronized resource. @@ -35,11 +39,12 @@ func CompareResourceIDs(left, right ResourceID) int { // SyncedResource contains one compiled local resource. type SyncedResource struct { - Kind Kind - ProjectKey string - LookupKey string - Payload json.RawMessage - Upsert bool + Kind Kind + ProjectKey string + LookupKey string + Payload json.RawMessage + Attachments []Attachment + Upsert bool } // VariationMode identifies how a prompt variation stores its content. @@ -135,13 +140,82 @@ func NormalizePromptText(content string) string { // Variation is the common prompt variation representation used by sync. type Variation struct { - Mode VariationMode `json:"mode" yaml:"mode"` - Key string `json:"key" yaml:"key"` - Name string `json:"name" yaml:"name"` - Instructions string `json:"instructions,omitempty" yaml:"-"` - ModelConfigKey string `json:"modelConfigKey,omitempty" yaml:"modelConfigKey,omitempty"` - ModelConfigVersion int `json:"modelConfigVersion,omitempty" yaml:"modelConfigVersion,omitempty"` - Model map[string]any `json:"model,omitempty" yaml:"model,omitempty"` - OutputFormat map[string]any `json:"outputFormat,omitempty" yaml:"outputFormat,omitempty"` - Messages []Message `json:"messages,omitempty" yaml:"-"` + Mode VariationMode `json:"mode" yaml:"mode"` + Key string `json:"key" yaml:"key"` + Name string `json:"name" yaml:"name"` + Instructions string `json:"instructions,omitempty" yaml:"-"` + ModelConfigKey string `json:"modelConfigKey,omitempty" yaml:"modelConfigKey,omitempty"` + ModelConfigVersion int `json:"modelConfigVersion,omitempty" yaml:"modelConfigVersion,omitempty"` + Model map[string]any `json:"model,omitempty" yaml:"model,omitempty"` + OutputFormat map[string]any `json:"outputFormat,omitempty" yaml:"outputFormat,omitempty"` + Messages []Message `json:"messages,omitempty" yaml:"-"` + Tools []AttachmentRef `json:"tools,omitempty" yaml:"tools,omitempty"` + Skills []AttachmentRef `json:"skills,omitempty" yaml:"skills,omitempty"` + Attachments []Attachment `json:"-" yaml:"-"` +} + +// NormalizeAttachments validates, sorts, and de-duplicates attachment keys so +// local ordering never produces fingerprint drift. +func (variation *Variation) NormalizeAttachments() error { + if err := normalizeAttachmentRefs(AttachmentTool, variation.Tools); err != nil { + return err + } + if err := normalizeAttachmentRefs(AttachmentSkill, variation.Skills); err != nil { + return err + } + if err := validateAttachmentMode(*variation); err != nil { + return err + } + + slices.SortFunc(variation.Tools, func(a, b AttachmentRef) int { return strings.Compare(a.Key, b.Key) }) + slices.SortFunc(variation.Skills, func(a, b AttachmentRef) int { return strings.Compare(a.Key, b.Key) }) + slices.SortFunc(variation.Attachments, func(a, b Attachment) int { + if result := strings.Compare(string(a.Kind), string(b.Kind)); result != 0 { + return result + } + return strings.Compare(a.Key(), b.Key()) + }) + return nil +} + +// Attachment returns canonical content and the latest observed version for one reference. +func (variation Variation) Attachment(kind AttachmentKind, key string) (Attachment, bool) { + for _, attachment := range variation.Attachments { + if attachment.Kind == kind && attachment.Key() == key { + return attachment, true + } + } + return Attachment{}, false +} + +// SetAttachment inserts or replaces one canonical dependency. +func (variation *Variation) SetAttachment(attachment Attachment) { + for index := range variation.Attachments { + if variation.Attachments[index].Kind == attachment.Kind && variation.Attachments[index].Key() == attachment.Key() { + variation.Attachments[index] = attachment + return + } + } + variation.Attachments = append(variation.Attachments, attachment) +} + +func normalizeAttachmentRefs(kind AttachmentKind, refs []AttachmentRef) error { + seen := make(map[string]struct{}, len(refs)) + for _, ref := range refs { + if strings.TrimSpace(ref.Key) == "" { + return fmt.Errorf("%s key is required", kind) + } + if _, duplicate := seen[ref.Key]; duplicate { + return fmt.Errorf("%s key %q is duplicated", kind, ref.Key) + } + seen[ref.Key] = struct{}{} + } + return nil +} + +func validateAttachmentMode(variation Variation) error { + if variation.Mode == VariationModeCompletion && len(variation.Skills) != 0 { + return fmt.Errorf("skills can only be attached to agent-mode configs") + } + return nil }