From 172938a9ad8f3692b139cce2dbbe4363a543d71d Mon Sep 17 00:00:00 2001 From: Clifford Tawiah Date: Wed, 4 Nov 2026 15:03:33 -0500 Subject: [PATCH] feat(sync): add projection and problem core Add typed projections for variations, tools, and skills, with the field states that the planner compares. Add structured sync problems with stable codes and next steps. Record the descriptor digest, the adapter behavior, and the indexed selections in sync.lock format version 1. Co-Authored-By: Claude Fable 5.1 --- internal/sync/api/client_test.go | 16 + internal/sync/api/manifest.go | 47 +- internal/sync/attachment.go | 9 + internal/sync/error.go | 106 +++ internal/sync/manifest/baseline_test.go | 43 +- internal/sync/manifest/lock.go | 2 +- internal/sync/manifest/model.go | 92 ++- internal/sync/manifest/model_test.go | 22 + internal/sync/manifest/store.go | 32 +- internal/sync/manifest/store_test.go | 34 +- internal/sync/projection.go | 730 ++++++++++++++++++ internal/sync/projection_parameters_test.go | 170 ++++ internal/sync/projection_test.go | 66 ++ internal/sync/prompt/acceptance_test.go | 6 +- internal/sync/relationship_projection_test.go | 76 ++ internal/sync/resource.go | 45 +- internal/sync/variation.go | 39 +- 17 files changed, 1494 insertions(+), 41 deletions(-) create mode 100644 internal/sync/error.go create mode 100644 internal/sync/projection.go create mode 100644 internal/sync/projection_parameters_test.go create mode 100644 internal/sync/projection_test.go create mode 100644 internal/sync/relationship_projection_test.go diff --git a/internal/sync/api/client_test.go b/internal/sync/api/client_test.go index 16d6e8fdb..48de862da 100644 --- a/internal/sync/api/client_test.go +++ b/internal/sync/api/client_test.go @@ -463,6 +463,7 @@ func TestClientArchiveVariationReturnsTransportError(t *testing.T) { func TestClientGetsSyncManifest(t *testing.T) { transport := &recordingClient{Responses: [][]byte{[]byte(`{ + "formatVersion": 1, "source": "git:example.com/acme/repo", "items": [{ "resourceKind": "variation", @@ -487,6 +488,7 @@ func TestClientGetsSyncManifest(t *testing.T) { func TestClientPatchesSyncManifest(t *testing.T) { transport := &recordingClient{Responses: [][]byte{[]byte(`{ + "formatVersion": 1, "source": "git:example.com/acme/repo", "items": [] }`)}} @@ -508,6 +510,7 @@ func TestClientPatchesSyncManifest(t *testing.T) { assert.Equal(t, http.MethodPatch, request.Method) assert.Equal(t, "application/json", request.ContentType) assert.JSONEq(t, `{ + "formatVersion": 1, "source": "git:example.com/acme/repo", "upserts": [{ "resourceKind": "variation", @@ -519,6 +522,19 @@ func TestClientPatchesSyncManifest(t *testing.T) { }`, string(request.Body)) } +func TestClientRejectsUnknownSyncManifestVersion(t *testing.T) { + transport := &recordingClient{Responses: [][]byte{[]byte(`{ + "formatVersion": 99, + "source": "git:example.com/acme/repo", + "items": [] + }`)}} + client := NewClient(transport, "token", "https://example.com") + + _, err := client.GetSyncManifest("project", "git:example.com/acme/repo") + + require.ErrorContains(t, err, "unsupported formatVersion 99") +} + func TestIsConflictRecognizesAPIStatus(t *testing.T) { assert.True(t, IsConflict(errors.New(`{"code":"conflict","statusCode":409}`))) assert.False(t, IsConflict(errors.New(`{"code":"invalid_request","statusCode":400}`))) diff --git a/internal/sync/api/manifest.go b/internal/sync/api/manifest.go index ad997bc9d..84fe6207e 100644 --- a/internal/sync/api/manifest.go +++ b/internal/sync/api/manifest.go @@ -9,11 +9,15 @@ import ( syncdomain "github.com/launchdarkly/ldcli/internal/sync" ) +// SyncManifestFormatVersion is the remote sync manifest schema. +const SyncManifestFormatVersion = 1 + // SyncManifest is the sync baseline that LaunchDarkly stores for one project // and one repository source. type SyncManifest struct { - Source string `json:"source"` - Items []SyncManifestResource `json:"items"` + FormatVersion int `json:"formatVersion"` + Source string `json:"source"` + Items []SyncManifestResource `json:"items"` } // SyncManifestResource is one versioned baseline entry. @@ -22,6 +26,9 @@ type SyncManifestResource struct { ResourceLookupKey string `json:"resourceLookupKey"` Fingerprint string `json:"fingerprint"` Version int `json:"version"` + FingerprintSchema string `json:"fingerprintSchema,omitempty"` + ReferenceDigest string `json:"referenceDigest,omitempty"` + AdapterBehavior string `json:"adapterBehavior,omitempty"` } // SyncManifestUpsert creates an entry, or updates the entry at Version. @@ -30,6 +37,9 @@ type SyncManifestUpsert struct { ResourceLookupKey string `json:"resourceLookupKey"` Fingerprint string `json:"fingerprint"` Version int `json:"version"` + FingerprintSchema string `json:"fingerprintSchema,omitempty"` + ReferenceDigest string `json:"referenceDigest,omitempty"` + AdapterBehavior string `json:"adapterBehavior,omitempty"` } // SyncManifestDeletion removes the entry at Version. @@ -48,7 +58,14 @@ func (client Client) GetSyncManifest(projectKey, source string) (SyncManifest, e if err != nil { return SyncManifest{}, err } - return decodeJSON[SyncManifest](response, fmt.Sprintf("sync manifest for project %q", projectKey)) + manifest, err := decodeJSON[SyncManifest](response, fmt.Sprintf("sync manifest for project %q", projectKey)) + if err != nil { + return SyncManifest{}, err + } + if err := validateSyncManifest(manifest); err != nil { + return SyncManifest{}, fmt.Errorf("sync manifest for project %q: %w", projectKey, err) + } + return manifest, nil } // PatchSyncManifest applies versioned changes and returns the new baseline. @@ -68,10 +85,16 @@ func (client Client) PatchSyncManifest( request := mutation{ method: http.MethodPatch, action: "update", resource: "sync manifest for project", key: projectKey, body: struct { - Source string `json:"source"` - Upserts []SyncManifestUpsert `json:"upserts"` - Deletions []SyncManifestDeletion `json:"deletions"` - }{Source: source, Upserts: upserts, Deletions: deletions}, + FormatVersion int `json:"formatVersion"` + Source string `json:"source"` + Upserts []SyncManifestUpsert `json:"upserts"` + Deletions []SyncManifestDeletion `json:"deletions"` + }{ + FormatVersion: SyncManifestFormatVersion, + Source: source, + Upserts: upserts, + Deletions: deletions, + }, } response, err := client.mutate(request, manifestPath(projectKey)...) if err != nil { @@ -84,9 +107,19 @@ func (client Client) PatchSyncManifest( // caller must read the manifest again to find its state. return SyncManifest{}, newMutationError("decode updated", request.resource, projectKey, err) } + if err := validateSyncManifest(manifest); err != nil { + return SyncManifest{}, newMutationError("decode updated", request.resource, projectKey, err) + } return manifest, nil } +func validateSyncManifest(manifest SyncManifest) error { + if manifest.FormatVersion != SyncManifestFormatVersion { + return fmt.Errorf("unsupported formatVersion %d", manifest.FormatVersion) + } + return nil +} + func manifestPath(projectKey string) []string { return projectPath(projectKey, "configs/sync/manifests") } diff --git a/internal/sync/attachment.go b/internal/sync/attachment.go index 8062cd15b..5f6d62ff1 100644 --- a/internal/sync/attachment.go +++ b/internal/sync/attachment.go @@ -63,6 +63,9 @@ func (skill Skill) CreateName() string { type Attachment struct { Kind AttachmentKind Version int + // Projection records the fields that a linked definition owns. It lets a + // dependency keep unowned LaunchDarkly fields when sync publishes it. + Projection Projection `json:"-" yaml:"-"` // Upsert lets sync create the tool or the skill in LaunchDarkly when it // is absent. Upsert bool @@ -91,6 +94,11 @@ func (attachment Attachment) ID(projectKey string) ResourceID { type SyncedAttachment struct { ProjectKey string Attachment Attachment + // Source is the descriptor-backed repository source, or nil for a managed file. + Source *Reference + Dependencies []string + IndexedSelections []IndexedSelection + Projection Projection } // ID returns the identity of the tool or the skill. @@ -103,6 +111,7 @@ func (attachment SyncedAttachment) ID() ResourceID { // the same content. func CanonicalAttachment(attachment Attachment) Attachment { attachment.Version = 0 + attachment.Projection = nil attachment.Upsert = false if attachment.Tool != nil { tool := *attachment.Tool diff --git a/internal/sync/error.go b/internal/sync/error.go new file mode 100644 index 000000000..c5a9c5050 --- /dev/null +++ b/internal/sync/error.go @@ -0,0 +1,106 @@ +package sync + +import ( + "errors" + "fmt" +) + +// ErrorCode identifies one stable sync failure. +type ErrorCode string + +const ( + ErrorInvalidPathSyntax ErrorCode = "invalid_path_syntax" + ErrorPathNotFound ErrorCode = "path_not_found" + ErrorPathAmbiguous ErrorCode = "path_ambiguous" + ErrorPathTypeMismatch ErrorCode = "path_type_mismatch" + ErrorFilterTypeMismatch ErrorCode = "filter_value_type_mismatch" + ErrorMapTypeMismatch ErrorCode = "map_value_type_mismatch" + ErrorIndexOutOfRange ErrorCode = "index_out_of_range" + ErrorIndexedSelectionChanged ErrorCode = "indexed_selection_changed" + ErrorUnsupportedFormat ErrorCode = "unsupported_source_format" + ErrorFieldReadOnly ErrorCode = "field_read_only" + ErrorDuplicateKey ErrorCode = "duplicate_key" + ErrorInvalidSource ErrorCode = "invalid_source" + ErrorInvalidReference ErrorCode = "invalid_reference" + ErrorDefinitionConflict ErrorCode = "resource_definition_conflict" + ErrorLinkedSourceMissing ErrorCode = "linked_source_missing" + ErrorAttachmentDefinitionMissing ErrorCode = "attachment_definition_missing" + ErrorSyncSettingUnavailable ErrorCode = "sync_setting_unavailable" + ErrorBindingStale ErrorCode = "binding_stale" + ErrorStateChanged ErrorCode = "state_changed" + ErrorSourceNotWritable ErrorCode = "source_not_writable" + ErrorOverlappingEdits ErrorCode = "overlapping_edits" + ErrorResourceDeleted ErrorCode = "resource_deleted_in_launchdarkly" + ErrorPlaceholderAmbiguous ErrorCode = "placeholder_translation_ambiguous" + ErrorImportUnresolved ErrorCode = "import_unresolved" +) + +// Problem describes one actionable sync error. +type Problem struct { + Code ErrorCode `json:"code"` + Message string `json:"message"` + Resource string `json:"resource,omitempty"` + File string `json:"file,omitempty"` + Line int `json:"line,omitempty"` + Column int `json:"column,omitempty"` + Suggestion string `json:"suggestion,omitempty"` + Cause error `json:"-"` +} + +// Error returns the message without embedding the stable code. +func (problem *Problem) Error() string { + if problem == nil { + return "" + } + if problem.Suggestion == "" { + return problem.Message + } + return problem.Message + ". " + problem.Suggestion +} + +// Unwrap returns the lower-level cause. +func (problem *Problem) Unwrap() error { + if problem == nil { + return nil + } + return problem.Cause +} + +// NewProblem creates one sync problem. +func NewProblem(code ErrorCode, message, suggestion string) *Problem { + return &Problem{Code: code, Message: message, Suggestion: suggestion} +} + +// ProblemFrom returns the first sync problem in an error chain. +func ProblemFrom(err error) (*Problem, bool) { + var problem *Problem + ok := errors.As(err, &problem) + return problem, ok +} + +// WithResource adds a resource identity to a problem. +func (problem *Problem) WithResource(id ResourceID) *Problem { + if problem != nil { + problem.Resource = id.Label() + } + return problem +} + +// WithFile adds a source location to a problem. +func (problem *Problem) WithFile(file string, line, column int) *Problem { + if problem != nil { + problem.File, problem.Line, problem.Column = file, line, column + } + return problem +} + +// WrapProblem adds a lower-level cause. +func WrapProblem(problem *Problem, cause error) *Problem { + if problem != nil { + problem.Cause = cause + if problem.Message == "" && cause != nil { + problem.Message = fmt.Sprint(cause) + } + } + return problem +} diff --git a/internal/sync/manifest/baseline_test.go b/internal/sync/manifest/baseline_test.go index 870752377..49a6c86b8 100644 --- a/internal/sync/manifest/baseline_test.go +++ b/internal/sync/manifest/baseline_test.go @@ -41,7 +41,7 @@ resources: func TestDecodeLockRejectsInvalidContent(t *testing.T) { for name, content := range map[string]string{ "unknown field": "formatVersion: 1\nresources: []\nextra: true\n", - "unknown version": "formatVersion: 2\nresources: []\n", + "unknown version": "formatVersion: 3\nresources: []\n", "bad fingerprint": "formatVersion: 1\nresources:\n - kind: tool\n project: p\n key: k\n fingerprint: x\n", } { _, err := decodeLock([]byte(content)) @@ -49,6 +49,33 @@ func TestDecodeLockRejectsInvalidContent(t *testing.T) { } } +func TestLockFormatOneContainsSourceMetadata(t *testing.T) { + content, err := encodeLock(Manifest{Resources: []Resource{{ + ResourceKind: syncdomain.KindSkill, + ProjectKey: "production", + LookupKey: "refunds", + Fingerprint: fingerprint("a"), + FingerprintSchema: syncdomain.ProjectionFingerprintSchema, + ReferenceDigest: fingerprint("b"), + AdapterBehavior: "markdown/1", + IndexedSelections: []syncdomain.IndexedSelection{{ + Path: "select#0", CollectionLength: 2, Fingerprint: fingerprint("c"), + }}, + }}}) + + require.NoError(t, err) + assert.Contains(t, string(content), "formatVersion: 1") + assert.Contains(t, string(content), "fingerprintSchema: launchdarkly.config.projection/1") + assert.Contains(t, string(content), "indexedSelections:") + assert.Contains(t, string(content), "collectionLength: 2") + assert.NotContains(t, string(content), "file:") + assert.NotContains(t, string(content), "direction:") + decoded, err := decodeLock(content) + require.NoError(t, err) + require.Len(t, decoded.Resources[0].IndexedSelections, 1) + assert.Equal(t, 2, decoded.Resources[0].IndexedSelections[0].CollectionLength) +} + // Other branches add entries to the shared remote manifest. A working copy // without a sync.lock file must not track them. func TestBaselineStoreWithoutLockFileTracksNothing(t *testing.T) { @@ -97,7 +124,7 @@ func TestBaselineStaleComparesFingerprints(t *testing.T) { "newer version with a different fingerprint": {remote: remoteManifest(fingerprint("b"), 5), stale: true}, "newer version with the same fingerprint": {remote: remoteManifest(fingerprint("a"), 5)}, "same version with a different fingerprint": {remote: remoteManifest(fingerprint("b"), 4), stale: true}, - "remote entry removed": {remote: syncapi.SyncManifest{Source: testSource}}, + "remote entry removed": {remote: syncapi.SyncManifest{FormatVersion: syncapi.SyncManifestFormatVersion, Source: testSource}}, } for name, test := range tests { @@ -117,7 +144,8 @@ func TestBaselineStoreSaveSendsRemoteVersionsAndWritesLockWithoutThem(t *testing client := &manifestClient{ manifests: map[string]syncapi.SyncManifest{"production": remoteManifest(fingerprint("b"), 5)}, patchResponses: []syncapi.SyncManifest{{ - Source: testSource, + FormatVersion: syncapi.SyncManifestFormatVersion, + Source: testSource, Items: []syncapi.SyncManifestResource{ {ResourceKind: syncdomain.KindVariation, ResourceLookupKey: "support/default", Fingerprint: fingerprint("c"), Version: 6}, {ResourceKind: syncdomain.KindTool, ResourceLookupKey: "search", Fingerprint: fingerprint("d"), Version: 1}, @@ -181,8 +209,11 @@ func TestBaselineStoreSaveDoesNotWriteLockWhenRemoteSaveFails(t *testing.T) { func TestBaselineStoreSaveRemovesEmptyLock(t *testing.T) { client := &manifestClient{ - manifests: map[string]syncapi.SyncManifest{"production": remoteManifest(fingerprint("a"), 4)}, - patchResponses: []syncapi.SyncManifest{{Source: testSource}}, + manifests: map[string]syncapi.SyncManifest{"production": remoteManifest(fingerprint("a"), 4)}, + patchResponses: []syncapi.SyncManifest{{ + FormatVersion: syncapi.SyncManifestFormatVersion, + Source: testSource, + }}, } lock := lockWith(t, fingerprint("a"), 4) store := NewBaselineStore(NewStore(client, testSource), lock) @@ -246,7 +277,7 @@ func variationID() syncdomain.ResourceID { } func remoteManifest(fingerprint string, version int) syncapi.SyncManifest { - return syncapi.SyncManifest{Source: testSource, Items: []syncapi.SyncManifestResource{{ + return syncapi.SyncManifest{FormatVersion: syncapi.SyncManifestFormatVersion, Source: testSource, Items: []syncapi.SyncManifestResource{{ ResourceKind: syncdomain.KindVariation, ResourceLookupKey: "support/default", Fingerprint: fingerprint, Version: version, }}} } diff --git a/internal/sync/manifest/lock.go b/internal/sync/manifest/lock.go index ac90a4430..44c0b332f 100644 --- a/internal/sync/manifest/lock.go +++ b/internal/sync/manifest/lock.go @@ -12,7 +12,7 @@ import ( // The sync.lock file stores the baseline of one working copy. Engineers commit // it with the .launchdarkly files, so that the baseline moves with the files. const ( - lockFormatVersion = 1 + lockFormatVersion = FormatVersion lockHeader = "# Written by ldcli sync. Commit this file with the .launchdarkly files.\n" ) diff --git a/internal/sync/manifest/model.go b/internal/sync/manifest/model.go index b7b860ea8..0db449a6d 100644 --- a/internal/sync/manifest/model.go +++ b/internal/sync/manifest/model.go @@ -14,6 +14,9 @@ import ( var fingerprintPattern = regexp.MustCompile(`^sha256:[0-9a-f]{64}$`) +// FormatVersion is the local sync manifest and sync.lock schema. +const FormatVersion = 1 + // Manifest is the baseline of every tracked resource. type Manifest struct { Resources []Resource @@ -26,6 +29,15 @@ type Resource struct { ProjectKey string `yaml:"project"` LookupKey string `yaml:"key"` Fingerprint string `yaml:"fingerprint"` + // FingerprintSchema identifies the meaning of a descriptor-backed fingerprint. + FingerprintSchema string `yaml:"fingerprintSchema,omitempty"` + // ReferenceDigest detects a descriptor mapping change. + ReferenceDigest string `yaml:"referenceDigest,omitempty"` + // AdapterBehavior is the behavior version of the adapter that produced + // the fingerprint, such as python/1. A new version needs a review. + AdapterBehavior string `yaml:"adapterBehavior,omitempty"` + // IndexedSelections detect a changed item at an indexed source path. + IndexedSelections []syncdomain.IndexedSelection `yaml:"indexedSelections,omitempty"` // Version is the version of the remote manifest entry, which LaunchDarkly // uses for optimistic locking. Only the remote manifest has it. The lock // does not store it, so that a lock entry changes only with its content. @@ -61,6 +73,40 @@ func (manifest *Manifest) SetFingerprint(id syncdomain.ResourceID, fingerprint s }) } +// SetSource records how a descriptor-backed resource was read: the digest of +// its mapping, the behavior version of its adapter, and the identity of each +// indexed item on its paths. A nil reference clears them for a managed file. +func (manifest *Manifest) SetSource( + id syncdomain.ResourceID, + reference *syncdomain.Reference, + behavior string, + selections []syncdomain.IndexedSelection, +) error { + index := manifest.index(id) + if index < 0 { + return nil + } + resource := &manifest.Resources[index] + if reference == nil { + resource.FingerprintSchema, resource.ReferenceDigest = "", "" + resource.AdapterBehavior, resource.IndexedSelections = "", nil + return nil + } + digest, err := syncdomain.ReferenceDigest(*reference) + if err != nil { + return err + } + selections = slices.Clone(selections) + slices.SortFunc(selections, func(left, right syncdomain.IndexedSelection) int { + return strings.Compare(left.Path, right.Path) + }) + resource.FingerprintSchema = syncdomain.ProjectionFingerprintSchema + resource.ReferenceDigest = digest + resource.AdapterBehavior = behavior + resource.IndexedSelections = selections + return nil +} + // Remove stops tracking one resource. func (manifest *Manifest) Remove(id syncdomain.ResourceID) { if index := manifest.index(id); index >= 0 { @@ -101,8 +147,8 @@ func (manifest *Manifest) setAttachments(projectKey string, attachments []syncdo func (manifest Manifest) WithChanges(before, after Manifest) Manifest { result := manifest.Clone() for _, resource := range after.Resources { - if index := before.index(resource.ID()); index < 0 || before.Resources[index].Fingerprint != resource.Fingerprint { - result.SetFingerprint(resource.ID(), resource.Fingerprint) + if index := before.index(resource.ID()); index < 0 || !before.Resources[index].SameContent(resource) { + result.put(resource) } } for _, resource := range before.Resources { @@ -113,6 +159,27 @@ func (manifest Manifest) WithChanges(before, after Manifest) Manifest { return result } +// SameContent reports whether two entries record the same state. The version +// is not content. +func (resource Resource) SameContent(other Resource) bool { + return resource.Fingerprint == other.Fingerprint && + resource.FingerprintSchema == other.FingerprintSchema && + resource.ReferenceDigest == other.ReferenceDigest && + resource.AdapterBehavior == other.AdapterBehavior && + slices.Equal(resource.IndexedSelections, other.IndexedSelections) +} + +// put records the content of one entry. An existing entry keeps its version. +func (manifest *Manifest) put(resource Resource) { + if index := manifest.index(resource.ID()); index >= 0 { + resource.Version = manifest.Resources[index].Version + manifest.Resources[index] = resource + return + } + resource.Version = 0 + manifest.Resources = append(manifest.Resources, resource) +} + // WithVersionsFrom returns a copy of the manifest in which each entry has the // version of the same entry in source, or 0 if source does not have it. func (manifest Manifest) WithVersionsFrom(source Manifest) Manifest { @@ -157,9 +224,30 @@ func (manifest Manifest) Validate() error { switch { case !fingerprintPattern.MatchString(resource.Fingerprint): return fmt.Errorf("invalid fingerprint for %s", id) + case resource.FingerprintSchema != "" && resource.FingerprintSchema != syncdomain.ProjectionFingerprintSchema: + return fmt.Errorf("unsupported fingerprint schema %q for %s", resource.FingerprintSchema, id) + case resource.FingerprintSchema != "" && (resource.ReferenceDigest == "" || resource.AdapterBehavior == ""): + return fmt.Errorf("incomplete source metadata for %s", id) + case resource.FingerprintSchema == "" && + (resource.ReferenceDigest != "" || resource.AdapterBehavior != "" || len(resource.IndexedSelections) != 0): + return fmt.Errorf("source metadata without a fingerprint schema for %s", id) + case resource.ReferenceDigest != "" && !fingerprintPattern.MatchString(resource.ReferenceDigest): + return fmt.Errorf("invalid reference digest for %s", id) case resource.Version < 0: return fmt.Errorf("invalid version for %s", id) } + for index, selection := range resource.IndexedSelections { + switch { + case selection.Path == "": + return fmt.Errorf("indexed selection path is empty for %s", id) + case selection.CollectionLength <= 0: + return fmt.Errorf("invalid indexed collection length for %s", id) + case !fingerprintPattern.MatchString(selection.Fingerprint): + return fmt.Errorf("invalid indexed selection fingerprint for %s", id) + case index > 0 && resource.IndexedSelections[index-1].Path >= selection.Path: + return fmt.Errorf("indexed selections are not in path order for %s", id) + } + } if _, duplicate := seen[id]; duplicate { return fmt.Errorf("duplicate manifest resource %s", id) } diff --git a/internal/sync/manifest/model_test.go b/internal/sync/manifest/model_test.go index 0efbd8682..94a53b4fb 100644 --- a/internal/sync/manifest/model_test.go +++ b/internal/sync/manifest/model_test.go @@ -1,6 +1,7 @@ package manifest import ( + "strings" "testing" "github.com/stretchr/testify/assert" @@ -59,3 +60,24 @@ func TestManifestPreservesExistingAttachmentBaselineWhenAddingConsumer(t *testin require.Len(t, manifest.Resources, 1) assert.Equal(t, "sha256:existing", manifest.Resources[0].Fingerprint) } + +func TestWithChangesCopiesSourceMetadata(t *testing.T) { + id := syncdomain.VariationID("project", "config", "default") + fingerprint := "sha256:" + strings.Repeat("a", 64) + before := Manifest{Resources: []Resource{{ + ResourceKind: id.Kind, ProjectKey: id.ProjectKey, LookupKey: id.LookupKey, Fingerprint: fingerprint, + }}} + remote := Manifest{Resources: []Resource{{ + ResourceKind: id.Kind, ProjectKey: id.ProjectKey, LookupKey: id.LookupKey, Fingerprint: fingerprint, Version: 3, + }}} + after := before.Clone() + require.NoError(t, after.SetSource(id, &syncdomain.Reference{File: "prompt.md", Format: "markdown"}, "markdown/1", nil)) + + result := remote.WithChanges(before, after) + + require.Len(t, result.Resources, 1) + assert.Equal(t, "markdown/1", result.Resources[0].AdapterBehavior) + assert.Equal(t, syncdomain.ProjectionFingerprintSchema, result.Resources[0].FingerprintSchema) + assert.NotEmpty(t, result.Resources[0].ReferenceDigest) + assert.Equal(t, 3, result.Resources[0].Version, "the remote version survives a content update") +} diff --git a/internal/sync/manifest/store.go b/internal/sync/manifest/store.go index 66c8d9f66..8b3dc7a92 100644 --- a/internal/sync/manifest/store.go +++ b/internal/sync/manifest/store.go @@ -133,6 +133,13 @@ func (store Store) patch( } func (store Store) resourcesFromRemote(projectKey string, remote syncapi.SyncManifest) ([]Resource, error) { + if remote.FormatVersion != syncapi.SyncManifestFormatVersion { + return nil, fmt.Errorf( + "sync manifest for project %q has unsupported formatVersion %d", + projectKey, + remote.FormatVersion, + ) + } if remote.Source != "" && remote.Source != store.source { return nil, fmt.Errorf( "sync manifest for project %q returned source %q instead of %q", projectKey, remote.Source, store.source, @@ -147,11 +154,14 @@ func (store Store) resourcesFromRemote(projectKey string, remote syncapi.SyncMan ) } resources = append(resources, Resource{ - ResourceKind: item.ResourceKind, - ProjectKey: projectKey, - LookupKey: item.ResourceLookupKey, - Fingerprint: item.Fingerprint, - Version: item.Version, + ResourceKind: item.ResourceKind, + ProjectKey: projectKey, + LookupKey: item.ResourceLookupKey, + Fingerprint: item.Fingerprint, + Version: item.Version, + FingerprintSchema: item.FingerprintSchema, + ReferenceDigest: item.ReferenceDigest, + AdapterBehavior: item.AdapterBehavior, }) } return resources, nil @@ -169,7 +179,7 @@ func changes(before, after []Resource) ([]syncapi.SyncManifestUpsert, []syncapi. for _, resource := range after { current, exists := remaining[resource.ID()] delete(remaining, resource.ID()) - if exists && current.Fingerprint == resource.Fingerprint { + if exists && sameManifestContent(current, resource) { continue } upserts = append(upserts, syncapi.SyncManifestUpsert{ @@ -177,6 +187,9 @@ func changes(before, after []Resource) ([]syncapi.SyncManifestUpsert, []syncapi. ResourceLookupKey: resource.LookupKey, Fingerprint: resource.Fingerprint, Version: current.Version, + FingerprintSchema: resource.FingerprintSchema, + ReferenceDigest: resource.ReferenceDigest, + AdapterBehavior: resource.AdapterBehavior, }) } @@ -193,6 +206,13 @@ func changes(before, after []Resource) ([]syncapi.SyncManifestUpsert, []syncapi. return upserts, deletions } +func sameManifestContent(left, right Resource) bool { + return left.Fingerprint == right.Fingerprint && + left.FingerprintSchema == right.FingerprintSchema && + left.ReferenceDigest == right.ReferenceDigest && + left.AdapterBehavior == right.AdapterBehavior +} + // changesApplied reports whether the remote manifest contains every change. func changesApplied( remote syncapi.SyncManifest, diff --git a/internal/sync/manifest/store_test.go b/internal/sync/manifest/store_test.go index bc19553a7..c4513641e 100644 --- a/internal/sync/manifest/store_test.go +++ b/internal/sync/manifest/store_test.go @@ -52,14 +52,16 @@ func (client *manifestClient) PatchSyncManifest( func TestStoreLoadsProjectManifests(t *testing.T) { client := &manifestClient{manifests: map[string]syncapi.SyncManifest{ "zeta": { - Source: "git:example/repo", + FormatVersion: syncapi.SyncManifestFormatVersion, + Source: "git:example/repo", Items: []syncapi.SyncManifestResource{{ ResourceKind: syncdomain.KindVariation, ResourceLookupKey: "config/b", Fingerprint: fingerprint("b"), Version: 4, }}, }, "alpha": { - Source: "git:example/repo", + FormatVersion: syncapi.SyncManifestFormatVersion, + Source: "git:example/repo", Items: []syncapi.SyncManifestResource{{ ResourceKind: syncdomain.KindVariation, ResourceLookupKey: "config/a", Fingerprint: fingerprint("a"), Version: 2, @@ -78,9 +80,15 @@ func TestStoreLoadsProjectManifests(t *testing.T) { func TestStoreUpdatesWithRemoteVersions(t *testing.T) { client := &manifestClient{patchResponses: []syncapi.SyncManifest{{ - Source: "git:example/repo", + FormatVersion: syncapi.SyncManifestFormatVersion, + Source: "git:example/repo", Items: []syncapi.SyncManifestResource{ - {ResourceKind: syncdomain.KindVariation, ResourceLookupKey: "config/existing", Fingerprint: fingerprint("c"), Version: 4}, + { + ResourceKind: syncdomain.KindVariation, ResourceLookupKey: "config/existing", + Fingerprint: fingerprint("c"), Version: 4, + FingerprintSchema: syncdomain.ProjectionFingerprintSchema, ReferenceDigest: fingerprint("e"), + AdapterBehavior: "markdown/1", + }, {ResourceKind: syncdomain.KindTool, ResourceLookupKey: "search", Fingerprint: fingerprint("d"), Version: 1}, }, }}} @@ -90,7 +98,12 @@ func TestStoreUpdatesWithRemoteVersions(t *testing.T) { {ResourceKind: syncdomain.KindSkill, ProjectKey: "project", LookupKey: "old", Fingerprint: fingerprint("b"), Version: 2}, }} next := Manifest{Resources: []Resource{ - {ResourceKind: syncdomain.KindVariation, ProjectKey: "project", LookupKey: "config/existing", Fingerprint: fingerprint("c"), Version: 3}, + { + ResourceKind: syncdomain.KindVariation, ProjectKey: "project", LookupKey: "config/existing", + Fingerprint: fingerprint("c"), Version: 3, + FingerprintSchema: syncdomain.ProjectionFingerprintSchema, ReferenceDigest: fingerprint("e"), + AdapterBehavior: "markdown/1", + }, {ResourceKind: syncdomain.KindTool, ProjectKey: "project", LookupKey: "search", Fingerprint: fingerprint("d")}, }} @@ -100,12 +113,18 @@ func TestStoreUpdatesWithRemoteVersions(t *testing.T) { require.Len(t, client.patches, 1) assert.Equal(t, []syncapi.SyncManifestUpsert{ {ResourceKind: syncdomain.KindTool, ResourceLookupKey: "search", Fingerprint: fingerprint("d"), Version: 0}, - {ResourceKind: syncdomain.KindVariation, ResourceLookupKey: "config/existing", Fingerprint: fingerprint("c"), Version: 3}, + { + ResourceKind: syncdomain.KindVariation, ResourceLookupKey: "config/existing", + Fingerprint: fingerprint("c"), Version: 3, + FingerprintSchema: syncdomain.ProjectionFingerprintSchema, ReferenceDigest: fingerprint("e"), + AdapterBehavior: "markdown/1", + }, }, client.patches[0].upserts) assert.Equal(t, []syncapi.SyncManifestDeletion{{ ResourceKind: syncdomain.KindSkill, ResourceLookupKey: "old", Version: 2, }}, client.patches[0].deletions) assert.Equal(t, 4, updated.Resources[1].Version) + assert.Equal(t, syncdomain.ProjectionFingerprintSchema, updated.Resources[1].FingerprintSchema) } func TestStoreReportsOptimisticConflict(t *testing.T) { @@ -126,7 +145,8 @@ func TestStoreReportsOptimisticConflict(t *testing.T) { func TestStoreReadsBackAnUncertainPatchThatSucceeded(t *testing.T) { applied := syncapi.SyncManifest{ - Source: "git:example/repo", + FormatVersion: syncapi.SyncManifestFormatVersion, + Source: "git:example/repo", Items: []syncapi.SyncManifestResource{{ ResourceKind: syncdomain.KindVariation, ResourceLookupKey: "config/variation", Fingerprint: fingerprint("b"), Version: 2, diff --git a/internal/sync/projection.go b/internal/sync/projection.go new file mode 100644 index 000000000..b6185d725 --- /dev/null +++ b/internal/sync/projection.go @@ -0,0 +1,730 @@ +package sync + +import ( + "encoding/json" + "fmt" + "maps" + "slices" + "strings" +) + +// ProjectionFingerprintSchema is the schema of descriptor-backed fingerprints. +const ProjectionFingerprintSchema = "launchdarkly.config.projection/1" + +// Field identifies one LaunchDarkly value that a repository source owns. +type Field string + +const ( + FieldPrompt Field = "prompt" + FieldMessages Field = "messages" + FieldName Field = "name" + FieldModel Field = "model" + FieldModelParameters Field = "model.parameters" + FieldTools Field = "tools" + FieldSkills Field = "skills" + FieldDescription Field = "description" + FieldSchema Field = "schema" + FieldCustomParameters Field = "customParameters" + FieldTags Field = "tags" + FieldMarkdown Field = "markdown" +) + +// Capability describes whether sync can write one mapped field back to its +// source. A read-only field blocks the whole resource. +type Capability string + +const ( + CapabilityDirect Capability = "direct" + CapabilityReadOnly Capability = "read-only" +) + +// FieldState records the write capability of one mapped field and whether +// the source gave it a value. A read-only field explains itself with Reason, +// and with Code and File when another file holds the value. +type FieldState struct { + Capability Capability + Known bool + Reason string + Code ErrorCode + File string +} + +// Projection is the mapped content of one repository source. Each kind has +// one typed projection. The methods let the planner compare and merge +// projections without knowing the kind. +type Projection interface { + Kind() Kind + // States returns the state of every mapped field. + States() map[Field]FieldState + // Value returns one known mapped value. + Value(Field) (any, bool) + // set returns a copy with one field set to value in state. + set(Field, any, FieldState) (Projection, error) + // empty returns a projection of the same kind with no fields. + empty() Projection +} + +// VariationProjection is the mapped content of one variation. Mode is the +// mode that the source implies: completion with Messages, else agent. +type VariationProjection struct { + Mode VariationMode `json:"mode"` + Instructions *string `json:"instructions,omitempty"` + Messages *[]Message `json:"messages,omitempty"` + Name *string `json:"name,omitempty"` + Model *string `json:"model,omitempty"` + Parameters *map[string]any `json:"parameters,omitempty"` + Tools *[]string `json:"tools,omitempty"` + Skills *[]string `json:"skills,omitempty"` + FieldStates map[Field]FieldState `json:"-"` +} + +// Kind returns KindVariation. +func (VariationProjection) Kind() Kind { return KindVariation } + +// States returns the state of every mapped field. +func (projection VariationProjection) States() map[Field]FieldState { return projection.FieldStates } + +// Value returns one known mapped value. FieldPrompt and FieldMessages both +// return the prompt: instructions or messages. +func (projection VariationProjection) Value(field Field) (any, bool) { + switch field { + case FieldPrompt, FieldMessages: + if projection.Instructions != nil { + return *projection.Instructions, true + } + if projection.Messages != nil { + return *projection.Messages, true + } + case FieldName: + return deref(projection.Name) + case FieldModel: + return deref(projection.Model) + case FieldModelParameters: + if projection.Parameters != nil { + return cloneMap(*projection.Parameters), true + } + case FieldTools: + return deref(projection.Tools) + case FieldSkills: + return deref(projection.Skills) + default: + if key, ok := ModelParameterKey(field); ok && projection.Parameters != nil { + parameter, present := (*projection.Parameters)[key] + return cloneJSONValue(parameter), present + } + } + return nil, false +} + +func (projection VariationProjection) set(field Field, raw any, state FieldState) (Projection, error) { + projection.FieldStates = withState(projection.FieldStates, field, state) + switch field { + case FieldPrompt, FieldMessages: + switch prompt := NormalizePrompt(raw).(type) { + case string: + projection.Instructions, projection.Messages = &prompt, nil + case []Message: + projection.Instructions, projection.Messages = nil, &prompt + default: + return nil, fmt.Errorf("field %q needs a string or messages", field) + } + case FieldName: + return projection, setString(field, raw, &projection.Name) + case FieldModel: + return projection, setString(field, raw, &projection.Model) + case FieldModelParameters: + return projection, setMap(field, raw, &projection.Parameters) + case FieldTools: + return projection, setStrings(field, raw, &projection.Tools) + case FieldSkills: + return projection, setStrings(field, raw, &projection.Skills) + default: + key, ok := ModelParameterKey(field) + if !ok { + return nil, fmt.Errorf("field %q is not valid for a variation", field) + } + parameters := map[string]any{} + if projection.Parameters != nil { + parameters = cloneMap(*projection.Parameters) + } + parameters[key] = cloneJSONValue(raw) + projection.Parameters = ¶meters + } + return projection, nil +} + +func (projection VariationProjection) empty() Projection { + return VariationProjection{Mode: projection.Mode, FieldStates: make(map[Field]FieldState)} +} + +// ForMode returns the projection in the mode of a config. One system message +// and agent instructions are the same prompt in the two modes, and an empty +// prompt is empty in both. Any other message list cannot become instructions. +func (projection VariationProjection) ForMode(mode VariationMode) (VariationProjection, error) { + if !mode.Valid() || projection.Mode == mode { + return projection, nil + } + projection.Mode = mode + switch { + case mode == VariationModeCompletion && projection.Instructions != nil: + messages := []Message{} + if *projection.Instructions != "" { + messages = append(messages, Message{Role: RoleSystem, Content: *projection.Instructions}) + } + projection.Instructions, projection.Messages = nil, &messages + case mode == VariationModeAgent && projection.Messages != nil: + messages := *projection.Messages + content := "" + switch { + case len(messages) == 0: + case len(messages) == 1 && messages[0].Role == RoleSystem: + content = messages[0].Content + default: + return VariationProjection{}, fmt.Errorf( + "the source has %d messages, but an agent config has only instructions", len(messages), + ) + } + projection.Instructions, projection.Messages = &content, nil + } + return projection, nil +} + +// ToolProjection is the mapped content of one tool. +type ToolProjection struct { + Description *string `json:"description,omitempty"` + Schema *map[string]any `json:"schema,omitempty"` + CustomParameters *map[string]any `json:"customParameters,omitempty"` + Tags *[]string `json:"tags,omitempty"` + FieldStates map[Field]FieldState `json:"-"` +} + +// Kind returns KindTool. +func (ToolProjection) Kind() Kind { return KindTool } + +// States returns the state of every mapped field. +func (projection ToolProjection) States() map[Field]FieldState { return projection.FieldStates } + +// Value returns one known mapped value. +func (projection ToolProjection) Value(field Field) (any, bool) { + switch field { + case FieldDescription: + return deref(projection.Description) + case FieldSchema: + return derefMap(projection.Schema) + case FieldCustomParameters: + return derefMap(projection.CustomParameters) + case FieldTags: + return deref(projection.Tags) + } + return nil, false +} + +func (projection ToolProjection) set(field Field, raw any, state FieldState) (Projection, error) { + projection.FieldStates = withState(projection.FieldStates, field, state) + switch field { + case FieldDescription: + return projection, setString(field, raw, &projection.Description) + case FieldSchema: + return projection, setMap(field, raw, &projection.Schema) + case FieldCustomParameters: + return projection, setMap(field, raw, &projection.CustomParameters) + case FieldTags: + return projection, setStrings(field, raw, &projection.Tags) + default: + return nil, fmt.Errorf("field %q is not valid for a tool", field) + } +} + +func (ToolProjection) empty() Projection { + return ToolProjection{FieldStates: make(map[Field]FieldState)} +} + +// SkillProjection is the mapped content of one skill. +type SkillProjection struct { + Description *string `json:"description,omitempty"` + Markdown *string `json:"markdown,omitempty"` + FieldStates map[Field]FieldState `json:"-"` +} + +// Kind returns KindSkill. +func (SkillProjection) Kind() Kind { return KindSkill } + +// States returns the state of every mapped field. +func (projection SkillProjection) States() map[Field]FieldState { return projection.FieldStates } + +// Value returns one known mapped value. +func (projection SkillProjection) Value(field Field) (any, bool) { + switch field { + case FieldDescription: + return deref(projection.Description) + case FieldMarkdown: + return deref(projection.Markdown) + } + return nil, false +} + +func (projection SkillProjection) set(field Field, raw any, state FieldState) (Projection, error) { + projection.FieldStates = withState(projection.FieldStates, field, state) + switch field { + case FieldDescription: + return projection, setString(field, raw, &projection.Description) + case FieldMarkdown: + return projection, setString(field, raw, &projection.Markdown) + default: + return nil, fmt.Errorf("field %q is not valid for a skill", field) + } +} + +func (SkillProjection) empty() Projection { + return SkillProjection{FieldStates: make(map[Field]FieldState)} +} + +// ProjectionFields returns the fields that a reference maps, in order. +func ProjectionFields(reference Reference) ([]Field, error) { + fields := make([]Field, 0, len(reference.Map)) + for name := range reference.Map { + field := Field(name) + if !field.valid() { + return nil, fmt.Errorf("unsupported mapped field %q", name) + } + fields = append(fields, field) + } + slices.Sort(fields) + return fields, nil +} + +func (field Field) valid() bool { + switch field { + case FieldPrompt, FieldMessages, FieldName, FieldModel, FieldModelParameters, FieldTools, FieldSkills, + FieldDescription, FieldSchema, FieldCustomParameters, FieldTags, FieldMarkdown: + return true + default: + _, ok := ModelParameterKey(field) + return ok + } +} + +// ModelParameterKey returns the exact key of a model.parameters["key"] field. +func ModelParameterKey(field Field) (string, bool) { + const prefix = `model.parameters[` + text := string(field) + if !strings.HasPrefix(text, prefix) || !strings.HasSuffix(text, "]") { + return "", false + } + quoted := text[len(prefix) : len(text)-1] + if len(quoted) < 2 || quoted[0] != '"' { + return "", false + } + var key string + if err := json.Unmarshal([]byte(quoted), &key); err != nil { + return "", false + } + return key, true +} + +// ProjectVariation selects the mapped fields of a reference from a variation. +func ProjectVariation(variation Variation, reference Reference) (VariationProjection, error) { + fields, err := ProjectionFields(reference) + if err != nil { + return VariationProjection{}, err + } + projection := VariationProjection{Mode: variation.Mode, FieldStates: make(map[Field]FieldState)} + for _, field := range fields { + projection.FieldStates[field] = directState + switch field { + case FieldPrompt, FieldMessages: + if variation.Mode == VariationModeCompletion { + messages := NormalizeMessages(variation.Messages) + projection.Messages = &messages + } else { + value := NormalizePromptText(variation.Instructions) + projection.Instructions = &value + } + case FieldName: + value := variation.Name + projection.Name = &value + case FieldModel: + if value, ok := variation.Model["modelName"].(string); ok { + projection.Model = &value + } + case FieldModelParameters: + parameters := canonicalModelParameters(variation) + projection.Parameters = ¶meters + default: + key, ok := ModelParameterKey(field) + if !ok { + return VariationProjection{}, fmt.Errorf("field %q is not valid for a variation", field) + } + parameters := canonicalModelParameters(variation) + if parameter, present := parameters[key]; present { + if projection.Parameters == nil { + value := map[string]any{} + projection.Parameters = &value + } + (*projection.Parameters)[key] = cloneJSONValue(parameter) + } + } + } + if reference.Tools != nil { + values := referenceKeys(variation.Tools) + projection.Tools = &values + projection.FieldStates[FieldTools] = directState + } + if reference.Skills != nil { + values := referenceKeys(variation.Skills) + projection.Skills = &values + projection.FieldStates[FieldSkills] = directState + } + markKnown(projection) + return projection, nil +} + +// markKnown sets Known on each direct field state from whether the projection +// has a value for the field. A server can lack a mapped value, such as a +// model name or a tool description. +func markKnown(projection Projection) { + for field, state := range projection.States() { + _, state.Known = projection.Value(field) + projection.States()[field] = state + } +} + +// ApplyVariationProjection merges the mapped fields of a projection into a +// variation. Fields that the projection does not map stay unchanged. +func ApplyVariationProjection(variation Variation, projection VariationProjection) Variation { + if projection.Mode.Valid() { + variation.Mode = projection.Mode + } + if projection.Instructions != nil { + variation.Instructions, variation.Messages = *projection.Instructions, nil + } + if projection.Messages != nil { + variation.Instructions, variation.Messages = "", slices.Clone(*projection.Messages) + } + if projection.Name != nil { + variation.Name = *projection.Name + } + if projection.Model != nil { + variation.Model = cloneMap(variation.Model) + variation.Model["modelName"] = *projection.Model + } + if projection.Parameters != nil { + variation.Model = cloneMap(variation.Model) + if _, whole := projection.FieldStates[FieldModelParameters]; whole { + variation.Model["parameters"] = cloneMap(*projection.Parameters) + } else { + parameters, _ := variation.Model["parameters"].(map[string]any) + parameters = cloneMap(parameters) + for _, field := range sortedFields(projection.FieldStates) { + key, parameter := ModelParameterKey(field) + if !parameter { + continue + } + if value, present := (*projection.Parameters)[key]; present { + parameters[key] = cloneJSONValue(value) + } + } + variation.Model["parameters"] = parameters + } + } + if projection.Tools != nil { + variation.Tools = attachmentReferences(*projection.Tools) + } + if projection.Skills != nil { + variation.Skills = attachmentReferences(*projection.Skills) + } + variation.Model = CanonicalModel(variation.Model, variation.Tools) + return variation +} + +// ProjectAttachment selects the mapped fields of a reference from a tool or +// a skill. +func ProjectAttachment(attachment Attachment, reference Reference) (Projection, error) { + fields, err := ProjectionFields(reference) + if err != nil { + return nil, err + } + switch { + case attachment.Tool != nil: + projection := ToolProjection{FieldStates: make(map[Field]FieldState)} + for _, field := range fields { + projection.FieldStates[field] = directState + switch field { + case FieldDescription: + projection.Description = attachment.Tool.Description + case FieldSchema: + value := cloneMap(attachment.Tool.Schema) + projection.Schema = &value + case FieldCustomParameters: + value := cloneMap(attachment.Tool.CustomParameters) + projection.CustomParameters = &value + case FieldTags: + value := sortedUnique(attachment.Tool.Tags) + projection.Tags = &value + default: + return nil, fmt.Errorf("field %q is not valid for a tool", field) + } + } + markKnown(projection) + return projection, nil + case attachment.Skill != nil: + projection := SkillProjection{FieldStates: make(map[Field]FieldState)} + for _, field := range fields { + projection.FieldStates[field] = directState + switch field { + case FieldDescription: + value := attachment.Skill.Description + projection.Description = &value + case FieldMarkdown: + value := attachment.Skill.Markdown + projection.Markdown = &value + default: + return nil, fmt.Errorf("field %q is not valid for a skill", field) + } + } + markKnown(projection) + return projection, nil + default: + return nil, fmt.Errorf("%s %q has no content", attachment.Kind, attachment.Key()) + } +} + +// ApplyAttachmentProjection merges the mapped fields of a projection into a +// tool or a skill. +func ApplyAttachmentProjection(attachment Attachment, projection Projection) (Attachment, error) { + switch value := projection.(type) { + case ToolProjection: + if attachment.Tool == nil { + return Attachment{}, fmt.Errorf("tool %q has no content", attachment.Key()) + } + tool := *attachment.Tool + if value.Description != nil { + description := *value.Description + tool.Description = &description + } + if value.Schema != nil { + tool.Schema = cloneMap(*value.Schema) + } + if value.CustomParameters != nil { + tool.CustomParameters = cloneMap(*value.CustomParameters) + } + if value.Tags != nil { + tool.Tags = sortedUnique(*value.Tags) + } + attachment.Tool = &tool + case SkillProjection: + if attachment.Skill == nil { + return Attachment{}, fmt.Errorf("skill %q has no content", attachment.Key()) + } + skill := *attachment.Skill + if value.Description != nil { + skill.Description = *value.Description + } + if value.Markdown != nil { + skill.Markdown = *value.Markdown + } + attachment.Skill = &skill + default: + return Attachment{}, fmt.Errorf("unsupported projection %T", projection) + } + return attachment, nil +} + +// FingerprintProjection returns a stable fingerprint of the mapped fields. +func FingerprintProjection(id ResourceID, projection Projection) (string, error) { + return fingerprint("projection", struct { + Schema string `json:"schema"` + ResourceKind Kind `json:"resourceKind"` + ProjectKey string `json:"projectKey"` + LookupKey string `json:"lookupKey"` + Projection Projection `json:"projection"` + }{ + Schema: ProjectionFingerprintSchema, + ResourceKind: id.Kind, + ProjectKey: id.ProjectKey, + LookupKey: id.LookupKey, + Projection: projection, + }) +} + +// ReferenceDigest returns a stable digest of the mapping of a descriptor. The +// relationship lists are content, so they are not part of it. +func ReferenceDigest(reference Reference) (string, error) { + return fingerprint("reference descriptor", struct { + Schema string `json:"schema"` + File string `json:"file"` + Format string `json:"format"` + Select string `json:"select,omitempty"` + Map map[string]string `json:"map,omitempty"` + }{ + Schema: "launchdarkly.config.reference/1", + File: reference.File, Format: reference.Format, Select: reference.Select, Map: reference.Map, + }) +} + +// FillReadOnlyFields copies the server value of each read-only field that +// the source cannot read, so that the two sides compare on equal terms. +func FillReadOnlyFields(local, server Projection) (Projection, error) { + result := local + for _, field := range sortedFields(local.States()) { + state := local.States()[field] + if state.Capability != CapabilityReadOnly || state.Known { + continue + } + value, ok := server.Value(field) + if !ok { + continue + } + var err error + if result, err = result.set(field, value, state); err != nil { + return nil, err + } + } + return result, nil +} + +// ChangedProjection returns the server values of the mapped fields whose +// values differ from the local projection. +func ChangedProjection(local, server Projection) (Projection, error) { + result := server.empty() + for _, field := range sortedFields(local.States()) { + left, leftOK := local.Value(field) + right, rightOK := server.Value(field) + if leftOK == rightOK && sameJSON(left, right) { + continue + } + if !rightOK { + continue + } + var err error + if result, err = result.set(field, right, directState); err != nil { + return nil, err + } + } + return result, nil +} + +// ProjectionIncludes reports whether actual has every known value of expected. +func ProjectionIncludes(actual, expected Projection) bool { + for _, field := range sortedFields(expected.States()) { + want, wantOK := expected.Value(field) + have, haveOK := actual.Value(field) + if wantOK != haveOK || !sameJSON(want, have) { + return false + } + } + return true +} + +var directState = FieldState{Capability: CapabilityDirect, Known: true} + +func sortedFields(states map[Field]FieldState) []Field { + return slices.Sorted(maps.Keys(states)) +} + +func withState(states map[Field]FieldState, field Field, state FieldState) map[Field]FieldState { + states = maps.Clone(states) + if states == nil { + states = make(map[Field]FieldState) + } + states[field] = state + return states +} + +func setString(field Field, raw any, target **string) error { + value, ok := raw.(string) + if !ok { + return fmt.Errorf("field %q needs a string", field) + } + *target = &value + return nil +} + +func setStrings(field Field, raw any, target **[]string) error { + value, ok := raw.([]string) + if !ok { + return fmt.Errorf("field %q needs a string list", field) + } + value = slices.Clone(value) + *target = &value + return nil +} + +func setMap(field Field, raw any, target **map[string]any) error { + value, ok := raw.(map[string]any) + if !ok { + return fmt.Errorf("field %q needs an object", field) + } + value = cloneMap(value) + *target = &value + return nil +} + +func deref[T any](value *T) (any, bool) { + if value == nil { + return nil, false + } + return *value, true +} + +func derefMap(value *map[string]any) (any, bool) { + if value == nil { + return nil, false + } + return cloneMap(*value), true +} + +func sameJSON(left, right any) bool { + leftJSON, _ := json.Marshal(left) + rightJSON, _ := json.Marshal(right) + return string(leftJSON) == string(rightJSON) +} + +func sortedUnique(values []string) []string { + return slices.Compact(slices.Sorted(slices.Values(values))) +} + +func referenceKeys(values []AttachmentRef) []string { + keys := make([]string, len(values)) + for index := range values { + keys[index] = values[index].Key + } + slices.Sort(keys) + return keys +} + +func attachmentReferences(keys []string) []AttachmentRef { + references := make([]AttachmentRef, len(keys)) + for index := range keys { + references[index] = AttachmentRef{Key: keys[index]} + } + return references +} + +func cloneMap(source map[string]any) map[string]any { + if source == nil { + return map[string]any{} + } + data, _ := json.Marshal(source) + var result map[string]any + _ = json.Unmarshal(data, &result) + return result +} + +func cloneJSONValue(source any) any { + data, err := json.Marshal(source) + if err != nil { + return source + } + var result any + if err := json.Unmarshal(data, &result); err != nil { + return source + } + return result +} + +func canonicalModelParameters(variation Variation) map[string]any { + model := CanonicalModel(variation.Model, variation.Tools) + parameters, _ := model["parameters"].(map[string]any) + return cloneMap(parameters) +} diff --git a/internal/sync/projection_parameters_test.go b/internal/sync/projection_parameters_test.go new file mode 100644 index 000000000..a2d79ea7e --- /dev/null +++ b/internal/sync/projection_parameters_test.go @@ -0,0 +1,170 @@ +package sync + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestModelParameterKeyPreservesExactKeys(t *testing.T) { + tests := []struct { + target string + key string + valid bool + }{ + {target: `model.parameters["temperature"]`, key: "temperature", valid: true}, + {target: `model.parameters["top_k"]`, key: "top_k", valid: true}, + {target: `model.parameters["top_p"]`, key: "top_p", valid: true}, + {target: `model.parameters["max_tokens"]`, key: "max_tokens", valid: true}, + {target: `model.parameters["Provider.Future-Key"]`, key: "Provider.Future-Key", valid: true}, + {target: `model.parameters["Case\u0053ensitive"]`, key: "CaseSensitive", valid: true}, + {target: `model.parameters[temperature]`}, + {target: `model.parameters.temperature`}, + {target: `model.parameters["temperature"].nested`}, + } + for _, test := range tests { + t.Run(test.target, func(t *testing.T) { + key, valid := ModelParameterKey(Field(test.target)) + assert.Equal(t, test.valid, valid) + assert.Equal(t, test.key, key) + }) + } +} + +func TestProjectVariationPreservesJSONModelParameterValues(t *testing.T) { + tests := []struct { + name string + key string + value any + }{ + {name: "temperature", key: "temperature", value: 0.2}, + {name: "top k", key: "top_k", value: 40.0}, + {name: "top p", key: "top_p", value: 0.9}, + {name: "max tokens", key: "max_tokens", value: 1024.0}, + {name: "future provider key", key: "Provider.Future-Key", value: "fast"}, + {name: "string", key: "response_style", value: "brief"}, + {name: "boolean", key: "reasoning", value: true}, + {name: "null", key: "seed", value: nil}, + {name: "nested object", key: "provider", value: map[string]any{"mode": "fast", "retry": true}}, + {name: "array", key: "stop", value: []any{"END", map[string]any{"token": 2.0}}}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + target := Field(`model.parameters["` + test.key + `"]`) + variation := Variation{ + Mode: VariationModeAgent, + Model: map[string]any{"parameters": map[string]any{test.key: test.value}}, + } + projection, err := ProjectVariation(variation, Reference{ + Map: map[string]string{string(target): "value"}, + }) + require.NoError(t, err) + require.NotNil(t, projection.Parameters) + assert.Equal(t, test.value, (*projection.Parameters)[test.key]) + value, present := projection.Value(target) + assert.True(t, present) + assert.Equal(t, test.value, value) + }) + } +} + +func TestApplyVariationProjectionUsesWholeOrIndividualParameterOwnership(t *testing.T) { + base := Variation{ + Mode: VariationModeAgent, + Model: map[string]any{"parameters": map[string]any{ + "temperature": 0.2, + "top_k": 20.0, + "keep": true, + }}, + } + tests := []struct { + name string + projection VariationProjection + want map[string]any + }{ + { + name: "individual mapping merges one key", + projection: VariationProjection{ + Parameters: &map[string]any{"top_k": 50.0}, + FieldStates: map[Field]FieldState{`model.parameters["top_k"]`: {Known: true}}, + }, + want: map[string]any{"temperature": 0.2, "top_k": 50.0, "keep": true}, + }, + { + name: "individual mapping preserves null", + projection: VariationProjection{ + Parameters: &map[string]any{"top_p": nil}, + FieldStates: map[Field]FieldState{`model.parameters["top_p"]`: {Known: true}}, + }, + want: map[string]any{"temperature": 0.2, "top_k": 20.0, "top_p": nil, "keep": true}, + }, + { + name: "whole mapping replaces canonical parameters", + projection: VariationProjection{ + Parameters: &map[string]any{"max_tokens": 512.0}, + FieldStates: map[Field]FieldState{FieldModelParameters: {Known: true}}, + }, + want: map[string]any{"max_tokens": 512.0}, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + applied := ApplyVariationProjection(base, test.projection) + assert.Equal(t, test.want, applied.Model["parameters"]) + }) + } +} + +func TestModelParameterProjectionChangeAndFingerprint(t *testing.T) { + id := VariationID("project", "config", "default") + target := Field(`model.parameters["temperature"]`) + state := map[Field]FieldState{target: {Capability: CapabilityDirect, Known: true}} + localParameters := map[string]any{"temperature": 0.2} + serverParameters := map[string]any{"temperature": 0.7} + local := VariationProjection{Mode: VariationModeAgent, Parameters: &localParameters, FieldStates: state} + server := VariationProjection{Mode: VariationModeAgent, Parameters: &serverParameters, FieldStates: state} + + localFingerprint, err := FingerprintProjection(id, local) + require.NoError(t, err) + serverFingerprint, err := FingerprintProjection(id, server) + require.NoError(t, err) + assert.NotEqual(t, localFingerprint, serverFingerprint) + + changed, err := ChangedProjection(local, server) + require.NoError(t, err) + value, present := changed.Value(target) + assert.True(t, present) + assert.Equal(t, 0.7, value) + assert.False(t, ProjectionIncludes(local, server)) + assert.True(t, ProjectionIncludes(server, changed)) +} + +func TestModelParameterProjectionExcludesExpandedTools(t *testing.T) { + variation := Variation{ + Mode: VariationModeAgent, + Tools: []AttachmentRef{{Key: "search"}}, + Model: map[string]any{"parameters": map[string]any{ + "temperature": 0.2, + "tools": []any{map[string]any{"name": "search"}}, + }}, + } + projection, err := ProjectVariation(variation, Reference{ + Map: map[string]string{string(FieldModelParameters): "parameters"}, + }) + require.NoError(t, err) + require.NotNil(t, projection.Parameters) + assert.Equal(t, map[string]any{"temperature": 0.2}, *projection.Parameters) + + parameters := map[string]any{ + "temperature": 0.3, + "tools": []any{map[string]any{"name": "search"}}, + } + applied := ApplyVariationProjection(variation, VariationProjection{ + Parameters: ¶meters, + FieldStates: map[Field]FieldState{ + FieldModelParameters: {Known: true}, + }, + }) + assert.Equal(t, map[string]any{"temperature": 0.3}, applied.Model["parameters"]) +} diff --git a/internal/sync/projection_test.go b/internal/sync/projection_test.go new file mode 100644 index 000000000..be032c0f6 --- /dev/null +++ b/internal/sync/projection_test.go @@ -0,0 +1,66 @@ +package sync + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestProjectionFingerprintIgnoresLineEndingsAndSurroundingSpace(t *testing.T) { + id := VariationID("project", "support", "default") + reference := Reference{File: "prompt.md", Format: "markdown", Map: map[string]string{string(FieldPrompt): "body"}} + clean := Variation{Mode: VariationModeAgent, Key: "default", Name: "Default", Instructions: "Ask for an order ID.\nKeep it short."} + noisy := clean + noisy.Instructions = "\n\nAsk for an order ID.\r\nKeep it short. \n" + + left, err := ProjectVariation(clean, reference) + require.NoError(t, err) + right, err := ProjectVariation(noisy, reference) + require.NoError(t, err) + leftFingerprint, err := FingerprintProjection(id, left) + require.NoError(t, err) + rightFingerprint, err := FingerprintProjection(id, right) + require.NoError(t, err) + assert.Equal(t, leftFingerprint, rightFingerprint) + + messages := VariationProjection{Mode: VariationModeCompletion} + set, err := messages.set(FieldMessages, []Message{{Role: RoleSystem, Content: "Hello\r\n"}}, directState) + require.NoError(t, err) + assert.Equal(t, "Hello", (*set.(VariationProjection).Messages)[0].Content) +} + +func TestForModeKeepsEmptyPromptsEmpty(t *testing.T) { + empty := "" + agent := VariationProjection{Mode: VariationModeAgent, Instructions: &empty} + completion, err := agent.ForMode(VariationModeCompletion) + require.NoError(t, err) + assert.Empty(t, *completion.Messages) + + back, err := completion.ForMode(VariationModeAgent) + require.NoError(t, err) + assert.Equal(t, "", *back.Instructions) + + user := VariationProjection{Mode: VariationModeCompletion, Messages: &[]Message{{Role: RoleUser, Content: "Hi"}}} + _, err = user.ForMode(VariationModeAgent) + require.Error(t, err) +} + +func TestProjectionsMarkAbsentServerValuesAsUnknown(t *testing.T) { + reference := Reference{File: "agent.py", Format: "python", Map: map[string]string{ + string(FieldPrompt): "instructions", string(FieldModel): "model", `model.parameters["temperature"]`: "temperature", + }} + projection, err := ProjectVariation(Variation{Mode: VariationModeAgent, Key: "default", Name: "Default"}, reference) + require.NoError(t, err) + assert.True(t, projection.States()[FieldPrompt].Known) + assert.False(t, projection.States()[FieldModel].Known) + assert.False(t, projection.States()[Field(`model.parameters["temperature"]`)].Known) + + tool := Attachment{Kind: AttachmentTool, Tool: &Tool{Key: "search", Schema: map[string]any{"type": "object"}}} + attachment, err := ProjectAttachment(tool, Reference{File: "tools.json", Format: "json", Map: map[string]string{ + string(FieldDescription): "description", string(FieldSchema): "schema", + }}) + require.NoError(t, err) + assert.False(t, attachment.States()[FieldDescription].Known) + assert.True(t, attachment.States()[FieldSchema].Known) +} diff --git a/internal/sync/prompt/acceptance_test.go b/internal/sync/prompt/acceptance_test.go index 3da41cbe9..050de741d 100644 --- a/internal/sync/prompt/acceptance_test.go +++ b/internal/sync/prompt/acceptance_test.go @@ -1960,7 +1960,7 @@ func runScoped(t *testing.T, root string, api *directAPI, options syncprompt.Opt func runWith(t *testing.T, root string, api *directAPI, runner syncprompt.Runner, options syncprompt.Options) (string, string, error) { t.Helper() if manifestsByRoot[root] == nil { - manifestsByRoot[root] = &syncapi.SyncManifest{Source: "git:example/repo", Items: []syncapi.SyncManifestResource{}} + manifestsByRoot[root] = &syncapi.SyncManifest{FormatVersion: syncapi.SyncManifestFormatVersion, Source: "git:example/repo", Items: []syncapi.SyncManifestResource{}} } api.manifest = manifestsByRoot[root] var output, notes bytes.Buffer @@ -2273,7 +2273,7 @@ func runPrompt(t *testing.T, root string, client resources.Client, arguments ... t.Setenv("XDG_CONFIG_HOME", t.TempDir()) manifest := manifestsByRoot[root] if manifest == nil { - manifest = &syncapi.SyncManifest{Source: "git:example/repo", Items: []syncapi.SyncManifestResource{}} + manifest = &syncapi.SyncManifest{FormatVersion: syncapi.SyncManifestFormatVersion, Source: "git:example/repo", Items: []syncapi.SyncManifestResource{}} manifestsByRoot[root] = manifest } switch api := client.(type) { @@ -2436,7 +2436,7 @@ func writeRemoteManifest(t *testing.T, root string, values ...syncdomain.Variati Kind: syncdomain.KindVariation, ProjectKey: "production", LookupKey: "support/" + value.Key, }, fingerprint(t, value)) } - remote := &syncapi.SyncManifest{Source: "git:example/repo"} + remote := &syncapi.SyncManifest{FormatVersion: syncapi.SyncManifestFormatVersion, Source: "git:example/repo"} for _, resource := range manifest.Resources { remote.Items = append(remote.Items, syncapi.SyncManifestResource{ ResourceKind: resource.ResourceKind, diff --git a/internal/sync/relationship_projection_test.go b/internal/sync/relationship_projection_test.go new file mode 100644 index 000000000..907ec918e --- /dev/null +++ b/internal/sync/relationship_projection_test.go @@ -0,0 +1,76 @@ +package sync + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestReferenceRelationshipsPreserveOwnershipAndExactKeys(t *testing.T) { + empty := []AttachmentRef{} + keys := []AttachmentRef{{Key: "Tool.Case_sensitive"}} + variation := Variation{ + Mode: VariationModeAgent, + Tools: []AttachmentRef{{Key: "Tool.Case_sensitive", Version: 9}}, + } + + unowned, err := ProjectVariation(variation, Reference{}) + require.NoError(t, err) + assert.Nil(t, unowned.Tools) + _, owned := unowned.FieldStates[FieldTools] + assert.False(t, owned) + + ownedEmpty, err := ProjectVariation(variation, Reference{Tools: &empty}) + require.NoError(t, err) + require.NotNil(t, ownedEmpty.Tools) + assert.Equal(t, []string{"Tool.Case_sensitive"}, *ownedEmpty.Tools) + + ownedKeys, err := ProjectVariation(variation, Reference{Tools: &keys}) + require.NoError(t, err) + require.NotNil(t, ownedKeys.Tools) + assert.Equal(t, []string{"Tool.Case_sensitive"}, *ownedKeys.Tools) + + emptyKeys := []string{} + applied := ApplyVariationProjection(Variation{}, VariationProjection{ + Tools: &emptyKeys, + FieldStates: map[Field]FieldState{ + FieldTools: {Capability: CapabilityDirect, Known: true}, + }, + }) + require.NotNil(t, applied.Tools) + assert.Empty(t, applied.Tools) + applied = ApplyVariationProjection(Variation{}, ownedKeys) + assert.Equal(t, []AttachmentRef{{Key: "Tool.Case_sensitive"}}, applied.Tools) +} + +func TestReferenceDigestExcludesRelationships(t *testing.T) { + first := []AttachmentRef{{Key: "search"}} + second := []AttachmentRef{{Key: "lookup"}} + base := Reference{File: "prompt.md", Format: "markdown"} + + left, err := ReferenceDigest(Reference{ + File: base.File, Format: base.Format, Tools: &first, + }) + require.NoError(t, err) + right, err := ReferenceDigest(Reference{ + File: base.File, Format: base.Format, Tools: &second, + }) + require.NoError(t, err) + assert.Equal(t, left, right) +} + +func TestValidateAttachmentRefsAllowsCompletionToolsAndRejectsSkills(t *testing.T) { + require.NoError(t, ValidateAttachmentRefs( + VariationModeCompletion, + []AttachmentRef{{Key: "search"}}, + nil, + )) + + err := ValidateAttachmentRefs( + VariationModeCompletion, + nil, + []AttachmentRef{{Key: "support"}}, + ) + require.EqualError(t, err, "skills can only be attached to agent-mode configs") +} diff --git a/internal/sync/resource.go b/internal/sync/resource.go index 5dcfb77ea..b80743366 100644 --- a/internal/sync/resource.go +++ b/internal/sync/resource.go @@ -22,6 +22,22 @@ const ( KindSkill Kind = "skill" ) +// SyncDirection is the LaunchDarkly setting that controls allowed writes. +type SyncDirection string + +const ( + SyncDirectionCodeCanonical SyncDirection = "code_canonical" + SyncDirectionServerCanonical SyncDirection = "server_canonical" + SyncDirectionBoth SyncDirection = "both" +) + +// Valid reports whether LaunchDarkly returned a supported sync direction. +func (direction SyncDirection) Valid() bool { + return direction == SyncDirectionCodeCanonical || + direction == SyncDirectionServerCanonical || + direction == SyncDirectionBoth +} + // ResourceID identifies one synchronized resource. // // For a variation, LookupKey is "config-key/variation-key". For a tool or a @@ -143,11 +159,23 @@ func ValidateCreate(kind Kind, key, name, description string) error { } } -// Reference links a variation to a prompt file elsewhere in the repository. -// Format names the adapter that reads and writes the file. +// Reference links one resource to a file elsewhere in the repository. +// Select identifies one value in the file. Map connects resource fields to +// paths below that value. type Reference struct { - File string `yaml:"file"` - Format string `yaml:"format"` + File string `json:"file" yaml:"file"` + Format string `json:"format" yaml:"format"` + Select string `json:"select,omitempty" yaml:"select,omitempty"` + Map map[string]string `json:"map,omitempty" yaml:"map,omitempty"` + Tools *[]AttachmentRef `json:"tools,omitempty" yaml:"tools,omitempty"` + Skills *[]AttachmentRef `json:"skills,omitempty" yaml:"skills,omitempty"` +} + +// IndexedSelection identifies one indexed item in a repository source. +type IndexedSelection struct { + Path string `yaml:"path"` + CollectionLength int `yaml:"collectionLength"` + Fingerprint string `yaml:"fingerprint"` } // SyncedResource is one variation compiled from the local workspace. @@ -156,8 +184,13 @@ type SyncedResource struct { ProjectKey string LookupKey string // Upsert lets sync create the variation in LaunchDarkly when it is absent. - Upsert bool - Variation Variation + Upsert bool + // Source is the descriptor-backed repository source, or nil for a managed file. + Source *Reference + Dependencies []string + IndexedSelections []IndexedSelection + Projection Projection + Variation Variation } // ID returns the identity of the compiled resource. diff --git a/internal/sync/variation.go b/internal/sync/variation.go index 22b853045..886d697ce 100644 --- a/internal/sync/variation.go +++ b/internal/sync/variation.go @@ -50,6 +50,28 @@ func NormalizePromptText(content string) string { return strings.TrimSpace(content) } +// NormalizeMessages returns the messages with normalized content. +func NormalizeMessages(messages []Message) []Message { + result := make([]Message, len(messages)) + for index, message := range messages { + result[index] = Message{Role: message.Role, Content: NormalizePromptText(message.Content)} + } + return result +} + +// NormalizePrompt normalizes a prompt value: instructions as a string, or +// messages as a list. Any other value is returned as it is. +func NormalizePrompt(value any) any { + switch prompt := value.(type) { + case string: + return NormalizePromptText(prompt) + case []Message: + return NormalizeMessages(prompt) + default: + return value + } +} + // Variation is the prompt variation model that every sync component uses. // // The JSON encoding of this struct is part of the fingerprint. Do not rename a @@ -82,8 +104,6 @@ func (variation Variation) Validate() error { return errors.New("key is required") case variation.Name == "": return errors.New("name is required") - case variation.Mode != VariationModeAgent && len(variation.Skills) != 0: - return errors.New("skills can only be attached to agent-mode configs") } // LaunchDarkly drops a value of these keys that is not an object. @@ -95,9 +115,22 @@ func (variation Variation) Validate() error { } } + return ValidateAttachmentRefs(variation.Mode, variation.Tools, variation.Skills) +} + +// ValidateAttachmentRefs makes sure that relationship keys are usable for the +// variation mode. +func ValidateAttachmentRefs(mode VariationMode, tools, skills []AttachmentRef) error { + if mode != VariationModeAgent && len(skills) != 0 { + return errors.New("skills can only be attached to agent-mode configs") + } for _, kind := range AttachmentKinds { seen := make(map[string]struct{}) - for _, ref := range variation.Refs(kind) { + refs := tools + if kind == AttachmentSkill { + refs = skills + } + for _, ref := range refs { if strings.TrimSpace(ref.Key) == "" { return fmt.Errorf("%s key is required", kind) }