diff --git a/internal/sync/api/client.go b/internal/sync/api/client.go index 14b5bb28..8df18c01 100644 --- a/internal/sync/api/client.go +++ b/internal/sync/api/client.go @@ -77,6 +77,25 @@ func NewClient(transport resources.Client, accessToken, baseURI string) Client { } } +// ModelConfig returns the latest version of one model config. +func (client Client) ModelConfig(projectKey, modelConfigKey string) (ModelConfig, error) { + endpoint, err := url.JoinPath(client.baseURI, "api/v2/projects", projectKey, "ai-configs/model-configs", modelConfigKey) + if err != nil { + return ModelConfig{}, fmt.Errorf("build model config endpoint: %w", err) + } + + response, err := client.transport.MakeRequest(client.accessToken, http.MethodGet, endpoint, "", nil, nil, false) + if err != nil { + return ModelConfig{}, fmt.Errorf("get model config %q: %w", modelConfigKey, err) + } + + var modelConfig ModelConfig + if err := json.Unmarshal(response, &modelConfig); err != nil { + return ModelConfig{}, fmt.Errorf("decode model config response: %w", err) + } + return modelConfig, nil +} + // ReadVariation returns one variation and its parent config mode. func (client Client) ReadVariation(projectKey, configKey, variationKey string) (VariationState, error) { config, err := NewCatalogClient(client.transport, client.accessToken, client.baseURI).Config(projectKey, configKey) diff --git a/internal/sync/fingerprint.go b/internal/sync/fingerprint.go index 263e9347..cbbd6568 100644 --- a/internal/sync/fingerprint.go +++ b/internal/sync/fingerprint.go @@ -5,6 +5,7 @@ import ( "encoding/hex" "encoding/json" "fmt" + "maps" ) const variationFingerprintSchema = "launchdarkly.config.variation/v1" @@ -17,9 +18,7 @@ func FingerprintVariation(projectKey, lookupKey string, variation Variation) (st } normalized := variation - if len(normalized.Model) == 0 { - normalized.Model = nil - } + normalized.Model = normalizeModelForFingerprint(normalized.Model) if len(normalized.Messages) == 0 { normalized.Messages = nil } @@ -57,6 +56,25 @@ func FingerprintVariation(projectKey, lookupKey string, variation Variation) (st return "sha256:" + hex.EncodeToString(sum[:]), nil } +// 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 { + if len(model) == 0 { + return nil + } + + normalized := maps.Clone(model) + for _, key := range []string{"parameters", "custom"} { + if value, ok := normalized[key].(map[string]any); ok && len(value) == 0 { + delete(normalized, key) + } + } + if len(normalized) == 0 { + return nil + } + return normalized +} + // ValidateDirectAPIVariation rejects fields that the existing variation APIs // cannot round-trip without the sync endpoints. func ValidateDirectAPIVariation(variation Variation) error { diff --git a/internal/sync/fingerprint_test.go b/internal/sync/fingerprint_test.go index edc6dcf8..38cfe4d9 100644 --- a/internal/sync/fingerprint_test.go +++ b/internal/sync/fingerprint_test.go @@ -107,6 +107,67 @@ func TestFingerprintVariationNormalizesPromptLineEndings(t *testing.T) { } } +func TestFingerprintVariationNormalizesModelDefaults(t *testing.T) { + withoutDefaults := Variation{ + Mode: VariationModeAgent, + Key: "agent", + Name: "Agent", + Model: map[string]any{ + "modelName": "claude", + "metadata": map[string]any{}, + "retries": 1, + }, + } + withDefaults := withoutDefaults + withDefaults.Model = map[string]any{ + "modelName": "claude", + "parameters": map[string]any{}, + "custom": map[string]any{}, + "metadata": map[string]any{}, + "retries": 1.0, + } + + withoutDefaultsFingerprint, err := FingerprintVariation("project", "config/agent", withoutDefaults) + require.NoError(t, err) + withDefaultsFingerprint, err := FingerprintVariation("project", "config/agent", withDefaults) + require.NoError(t, err) + + require.Equal(t, withoutDefaultsFingerprint, withDefaultsFingerprint) + require.Contains(t, withDefaults.Model, "parameters") + require.Contains(t, withDefaults.Model, "custom") +} + +func TestFingerprintVariationPreservesMeaningfulModelChanges(t *testing.T) { + base := Variation{ + Mode: VariationModeAgent, + Key: "agent", + Name: "Agent", + Model: map[string]any{ + "modelName": "claude", + }, + } + withEmptyMetadata := base + withEmptyMetadata.Model = map[string]any{ + "modelName": "claude", + "metadata": map[string]any{}, + } + withMetadata := base + withMetadata.Model = map[string]any{ + "modelName": "claude", + "metadata": map[string]any{"region": "us-east"}, + } + + baseFingerprint, err := FingerprintVariation("project", "config/agent", base) + require.NoError(t, err) + emptyMetadataFingerprint, err := FingerprintVariation("project", "config/agent", withEmptyMetadata) + require.NoError(t, err) + metadataFingerprint, err := FingerprintVariation("project", "config/agent", withMetadata) + require.NoError(t, err) + + require.NotEqual(t, baseFingerprint, emptyMetadataFingerprint) + require.NotEqual(t, emptyMetadataFingerprint, metadataFingerprint) +} + func TestValidateDirectAPIVariationSupportsModelConfigVersion(t *testing.T) { base := Variation{Mode: VariationModeAgent, Key: "default", Name: "Default"} diff --git a/internal/sync/prompt/acceptance_test.go b/internal/sync/prompt/acceptance_test.go index b2a24427..5d0ea9e6 100644 --- a/internal/sync/prompt/acceptance_test.go +++ b/internal/sync/prompt/acceptance_test.go @@ -17,16 +17,17 @@ import ( "github.com/launchdarkly/ldcli/internal/analytics" "github.com/launchdarkly/ldcli/internal/resources" syncdomain "github.com/launchdarkly/ldcli/internal/sync" + syncapi "github.com/launchdarkly/ldcli/internal/sync/api" synclocal "github.com/launchdarkly/ldcli/internal/sync/local" syncmanifest "github.com/launchdarkly/ldcli/internal/sync/manifest" syncreference "github.com/launchdarkly/ldcli/internal/sync/reference" ) type directAPI struct { - variation *syncdomain.Variation - variationState string - canonicalizeCreatedModel bool - requests []string + variation *syncdomain.Variation + variationState string + modelConfigs []syncapi.ModelConfig + requests []string } type directAPIVariation struct { @@ -46,6 +47,14 @@ func (api *directAPI) MakeRequest( _ bool, ) ([]byte, error) { api.requests = append(api.requests, method+" "+path) + if method == "GET" && strings.Contains(path, "/model-configs/") { + for _, modelConfig := range api.modelConfigs { + if strings.HasSuffix(path, "/"+modelConfig.Key) { + return json.Marshal(modelConfig) + } + } + return nil, fmt.Errorf("model config not found") + } switch method { case "GET": @@ -65,11 +74,6 @@ func (api *directAPI) MakeRequest( return nil, err } variation.Mode = syncdomain.VariationModeAgent - if api.canonicalizeCreatedModel { - variation.Model = map[string]any{ - "modelName": variation.ModelConfigKey, "parameters": map[string]any{}, "custom": map[string]any{}, - } - } api.variation = &variation api.variationState = "published" case "PATCH": @@ -140,38 +144,64 @@ func TestPromptFirstSyncAdoptsMatchingStateWithoutMutation(t *testing.T) { require.NoError(t, err) assertManifestFingerprint(t, root, local) - for _, request := range api.requests { - assert.True(t, strings.HasPrefix(request, "GET "), request) - } + requireOnlyReads(t, api.requests) } func TestPromptFirstSyncCreatesUpsertVariation(t *testing.T) { root := initRepository(t) local := variation("New") + local.ModelConfigKey = "gemini" + local.Model = map[string]any{"modelName": "gemini"} writeVariation(t, root, local, true) - api := &directAPI{} + api := &directAPI{modelConfigs: []syncapi.ModelConfig{{Key: "gemini", ID: "gemini", Version: 4}}} _, _, err := runPrompt(t, root, api, "--yes") require.NoError(t, err) require.NotNil(t, api.variation) assert.Equal(t, local.Name, api.variation.Name) + assert.Equal(t, 4, api.variation.ModelConfigVersion) + + local.ModelConfigVersion = 4 assertManifestFingerprint(t, root, local) + requireLocalModelConfigVersion(t, root, 0) + + api.requests = nil + _, _, err = runPrompt(t, root, api, "--yes") + require.NoError(t, err) + requireOnlyReads(t, api.requests) } -func TestPromptCreateAcceptsSuccessfulServerCanonicalization(t *testing.T) { +func TestPromptUpdateResolvesOmittedModelConfigVersionToLatest(t *testing.T) { root := initRepository(t) - local := variation("New") - local.ModelConfigKey = "gemini" + baseline := variation("Matching") + baseline.ModelConfigKey = "gemini" + baseline.ModelConfigVersion = 2 + baseline.Model = map[string]any{"modelName": "gemini"} + + local := baseline + local.ModelConfigVersion = 0 writeVariation(t, root, local, true) - api := &directAPI{canonicalizeCreatedModel: true} + writeManifest(t, root, baseline) + api := &directAPI{ + variation: pointer(baseline), + modelConfigs: []syncapi.ModelConfig{{Key: "gemini", ID: "gemini", Version: 4}}, + } _, _, err := runPrompt(t, root, api, "--yes") require.NoError(t, err) require.NotNil(t, api.variation) - assert.NotEmpty(t, api.variation.Model) + assert.Equal(t, 4, api.variation.ModelConfigVersion) + + local.ModelConfigVersion = 4 assertManifestFingerprint(t, root, local) + requireLocalModelConfigVersion(t, root, 0) + + api.requests = nil + _, _, err = runPrompt(t, root, api, "--yes") + require.NoError(t, err) + requireOnlyReads(t, api.requests) } func TestPromptPushesLocalChangeAndAdvancesManifest(t *testing.T) { @@ -211,6 +241,65 @@ func TestPromptPullsServerChangeAndAdvancesManifest(t *testing.T) { assertManifestFingerprint(t, root, server) } +func TestPromptPullPreservesLocalModelOverridesAcrossVersions(t *testing.T) { + root := initRepository(t) + baseline := variation("Baseline") + baseline.ModelConfigKey = "gemini" + baseline.ModelConfigVersion = 4 + baseline.Model = map[string]any{ + "modelName": "gemini-1", + "parameters": map[string]any{"temperature": 0.2}, + "custom": map[string]any{"tone": "friendly"}, + } + + local := baseline + local.ModelConfigVersion = 0 + local.Model = map[string]any{"custom": map[string]any{"tone": "friendly"}} + writeVariation(t, root, local, false) + writeManifest(t, root, baseline) + + server := baseline + server.Name = "Server" + server.Model = map[string]any{ + "modelName": "gemini-1", + "parameters": map[string]any{"temperature": 0.2}, + "custom": map[string]any{"tone": "formal"}, + } + api := &directAPI{ + variation: pointer(server), + modelConfigs: []syncapi.ModelConfig{{ + Key: "gemini", ID: "gemini-1", Version: 4, + Params: map[string]any{"temperature": 0.2}, + }}, + } + + _, _, err := runPrompt(t, root, api, "--yes") + + require.NoError(t, err) + requireLocalModelConfigVersion(t, root, 0) + assertManifestFingerprint(t, root, server) + + api.modelConfigs[0] = syncapi.ModelConfig{ + Key: "gemini", ID: "gemini-2", Version: 5, + Params: map[string]any{"temperature": 0.4}, + } + _, _, err = runPrompt(t, root, api, "--yes") + require.NoError(t, err) + require.NotNil(t, api.variation) + assert.Equal(t, 5, api.variation.ModelConfigVersion) + assert.Equal(t, map[string]any{ + "modelName": "gemini-2", + "parameters": map[string]any{"temperature": 0.4}, + "custom": map[string]any{"tone": "formal"}, + }, api.variation.Model) + requireLocalModelConfigVersion(t, root, 0) + + api.requests = nil + _, _, err = runPrompt(t, root, api, "--yes") + require.NoError(t, err) + requireOnlyReads(t, api.requests) +} + func TestPromptPushesLinkedFileContent(t *testing.T) { root := initRepository(t) local := variation("Linked") @@ -315,9 +404,7 @@ func TestPromptRejectsDivergentChangesWithoutMutation(t *testing.T) { _, _, err := runPrompt(t, root, api, "--yes") require.ErrorContains(t, err, "interactive conflict resolution requires a terminal") - for _, request := range api.requests { - assert.True(t, strings.HasPrefix(request, "GET "), request) - } + requireOnlyReads(t, api.requests) } func TestPromptRevalidatesBeforeWriting(t *testing.T) { @@ -330,9 +417,7 @@ func TestPromptRevalidatesBeforeWriting(t *testing.T) { _, _, err := runPrompt(t, root, api, "--yes") require.ErrorContains(t, err, "sync state changed after review") - for _, request := range api.requests { - assert.True(t, strings.HasPrefix(request, "GET "), request) - } + requireOnlyReads(t, api.requests) } func TestPromptAcceptsConfirmedWriteAfterAmbiguousError(t *testing.T) { @@ -568,6 +653,25 @@ func assertManifestFingerprints(t *testing.T, root string, values ...syncdomain. } } +func requireLocalModelConfigVersion(t *testing.T, root string, expected int) { + t.Helper() + resources, err := synclocal.CompileWorkspace(root) + require.NoError(t, err) + require.Len(t, resources, 1) + + var persisted syncdomain.Variation + require.NoError(t, json.Unmarshal(resources[0].Payload, &persisted)) + require.Equal(t, expected, persisted.ModelConfigVersion) +} + +func requireOnlyReads(t *testing.T, requests []string) { + t.Helper() + require.NotEmpty(t, requests) + for _, request := range requests { + require.True(t, strings.HasPrefix(request, "GET "), request) + } +} + func fingerprint(t *testing.T, value syncdomain.Variation) string { t.Helper() result, err := syncdomain.FingerprintVariation("production", "support/"+value.Key, value) diff --git a/internal/sync/prompt/execute.go b/internal/sync/prompt/execute.go index ea147fde..a80ea15f 100644 --- a/internal/sync/prompt/execute.go +++ b/internal/sync/prompt/execute.go @@ -12,12 +12,6 @@ import ( syncmanifest "github.com/launchdarkly/ldcli/internal/sync/manifest" ) -type currentResourceState struct { - localFingerprint string - serverFingerprint string - serverMode syncdomain.VariationMode -} - // executePlan applies each independently executable resource and advances the // manifest only for resources that succeed. func executePlan( @@ -26,6 +20,7 @@ func executePlan( client syncapi.Client, manifest syncmanifest.Manifest, plan Plan, + localFiles localFileResourcesByID, ) ([]ResourceOutcome, syncmanifest.Manifest, error) { // Keep the reviewed baseline immutable while successful resources advance // the result manifest independently. @@ -45,7 +40,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, resource, localFiles[resource.ID]); 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 @@ -61,15 +56,25 @@ func executePlan( return outcomes, manifest, errors.Join(failures...) } -// applyResourceChange verifies the reviewed state and applies one local or -// server mutation. -func applyResourceChange(repositoryRoot string, localStore synclocal.Store, client syncapi.Client, resource PlannedResource) error { - if err := verifyResourceUnchanged(repositoryRoot, client, resource); err != nil { - return err - } +// 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, + localFile syncdomain.SyncedResource, +) error { if changesServer(resource.Action) { return applyServerChange(client, resource) } + if resource.Action == ActionUpdateLocal { + variation, err := variationForLocalFile(*resource.Server, resource.Local, localFile) + if err != nil { + return err + } + resource.Server = &variation + } if err := applyLocalChange(localStore, resource); err != nil { return err } @@ -89,21 +94,6 @@ func recordSuccessfulChange(manifest *syncmanifest.Manifest, resource PlannedRes } } -// verifyResourceUnchanged prevents a reviewed action from using stale local or -// server state. -func verifyResourceUnchanged(repositoryRoot string, client syncapi.Client, reviewed PlannedResource) error { - current, err := readCurrentResourceState(repositoryRoot, client, reviewed.ID) - if err != nil { - return err - } - if current.localFingerprint != reviewed.LocalFingerprint || - current.serverFingerprint != reviewed.ServerFingerprint || - current.serverMode != reviewed.ServerMode { - return fmt.Errorf("resource changed after review; run sync again") - } - return nil -} - // applyServerChange performs one variation mutation through the existing // public config APIs. func applyServerChange(client syncapi.Client, resource PlannedResource) error { @@ -165,9 +155,16 @@ func verifyLocalResult(repositoryRoot string, resource PlannedResource) error { return err } - expectedFingerprint := resource.ServerFingerprint - if resource.Action == ActionDeleteLocal { - expectedFingerprint = "" + expectedFingerprint := "" + if resource.Action == ActionUpdateLocal { + expectedFingerprint, err = syncdomain.FingerprintVariation( + resource.ID.ProjectKey, + resource.ID.LookupKey, + *resource.Server, + ) + if err != nil { + return err + } } if actualFingerprint != expectedFingerprint { return fmt.Errorf("local variation did not match the expected state after sync") @@ -175,27 +172,6 @@ func verifyLocalResult(repositoryRoot string, resource PlannedResource) error { return nil } -// readCurrentResourceState reads the local and server fingerprints used for -// optimistic concurrency checks. -func readCurrentResourceState(repositoryRoot string, client syncapi.Client, id ResourceID) (currentResourceState, error) { - localFingerprint, err := readLocalFingerprint(repositoryRoot, id) - if err != nil { - return currentResourceState{}, err - } - - serverResource, err := readServerResource(client, id) - if err != nil { - return currentResourceState{}, err - } - serverFingerprint := "" - if serverResource.Variation != nil { - serverFingerprint, err = syncdomain.FingerprintVariation(id.ProjectKey, id.LookupKey, *serverResource.Variation) - } - return currentResourceState{ - localFingerprint: localFingerprint, serverFingerprint: serverFingerprint, serverMode: serverResource.ConfigMode, - }, err -} - // readLocalFingerprint returns the current fingerprint for one local resource, // or an empty fingerprint when the resource does not exist. func readLocalFingerprint(repositoryRoot string, id ResourceID) (string, error) { diff --git a/internal/sync/prompt/execute_test.go b/internal/sync/prompt/execute_test.go index f910412e..c6ad2c4a 100644 --- a/internal/sync/prompt/execute_test.go +++ b/internal/sync/prompt/execute_test.go @@ -31,7 +31,7 @@ func TestExecutePlanDoesNotMutateReviewedManifest(t *testing.T) { LocalFingerprint: "updated", }}} - _, updatedManifest, err := executePlan("", synclocal.Store{}, syncapi.Client{}, reviewedManifest, plan) + _, updatedManifest, err := executePlan("", synclocal.Store{}, syncapi.Client{}, reviewedManifest, plan, nil) require.NoError(t, err) assert.Equal(t, "reviewed", reviewedManifest.Resources[0].Fingerprint) diff --git a/internal/sync/prompt/local_changes.go b/internal/sync/prompt/local_changes.go index 677c29d5..68ca31c8 100644 --- a/internal/sync/prompt/local_changes.go +++ b/internal/sync/prompt/local_changes.go @@ -15,19 +15,20 @@ func applyLocalChange(store synclocal.Store, resource PlannedResource) error { switch resource.Action { case ActionUpdateLocal: + variation := *resource.Server if resource.Local == nil { _, err := store.Add([]synclocal.VariationFile{{ ProjectKey: resource.ID.ProjectKey, ConfigKey: configKey, Upsert: true, - Variation: *resource.Server, + Variation: variation, }}) return err } _, err := store.ReplaceVariations([]synclocal.VariationReplacement{{ ProjectKey: resource.ID.ProjectKey, ConfigKey: configKey, - Variation: *resource.Server, + Variation: variation, }}) return err case ActionDeleteLocal: diff --git a/internal/sync/prompt/model_config.go b/internal/sync/prompt/model_config.go new file mode 100644 index 00000000..1bf1a736 --- /dev/null +++ b/internal/sync/prompt/model_config.go @@ -0,0 +1,112 @@ +package prompt + +import ( + "encoding/json" + "fmt" + "maps" + "reflect" + "slices" + + syncdomain "github.com/launchdarkly/ldcli/internal/sync" + syncapi "github.com/launchdarkly/ldcli/internal/sync/api" +) + +type modelConfigGetter func(projectKey, modelConfigKey string) (syncapi.ModelConfig, error) + +type modelConfigID struct { + projectKey string + configKey string +} + +// canonicalizeLocalVariationModels returns local resources in the +// server-comparable shape used by planning. An omitted version uses the latest +// versioned model config. +func canonicalizeLocalVariationModels( + localFileResources []syncdomain.SyncedResource, + getModelConfig modelConfigGetter, +) ([]syncdomain.SyncedResource, error) { + // Clone the slice so canonical payloads cannot replace local file payloads. + canonicalResources := slices.Clone(localFileResources) + modelConfigs := make(map[modelConfigID]syncapi.ModelConfig) + + for index := range canonicalResources { + resource := &canonicalResources[index] + if resource.Kind != syncdomain.KindVariation { + continue + } + + var variation syncdomain.Variation + if err := json.Unmarshal(resource.Payload, &variation); err != nil { + return nil, fmt.Errorf("decode local variation %q: %w", resource.LookupKey, err) + } + if variation.ModelConfigKey == "" || variation.ModelConfigVersion != 0 { + continue + } + + id := modelConfigID{projectKey: resource.ProjectKey, configKey: variation.ModelConfigKey} + modelConfig, ok := modelConfigs[id] + if !ok { + var err error + modelConfig, err = getModelConfig(id.projectKey, id.configKey) + if err != nil { + return nil, err + } + modelConfigs[id] = modelConfig + } + if modelConfig.Version == 0 { + continue + } + + variation.ModelConfigVersion = modelConfig.Version + canonicalModel := modelConfig.VariationModel() + maps.Copy(canonicalModel, variation.Model) + variation.Model = canonicalModel + payload, err := json.Marshal(variation) + if err != nil { + return nil, fmt.Errorf("encode canonical variation %q: %w", resource.LookupKey, err) + } + resource.Payload = payload + } + + return canonicalResources, nil +} + +// variationForLocalFile converts server state into local file form. It retains +// model keys from the file and new server values as overrides. +func variationForLocalFile( + serverVariation syncdomain.Variation, + canonicalLocalVariation *syncdomain.Variation, + localFileResource syncdomain.SyncedResource, +) (syncdomain.Variation, error) { + if len(localFileResource.Payload) == 0 || canonicalLocalVariation == nil { + return serverVariation, nil + } + + var localFileVariation syncdomain.Variation + if err := json.Unmarshal(localFileResource.Payload, &localFileVariation); err != nil { + return syncdomain.Variation{}, fmt.Errorf("decode local file variation %q: %w", localFileResource.LookupKey, err) + } + if localFileVariation.ModelConfigKey == "" || localFileVariation.ModelConfigVersion != 0 { + return serverVariation, nil + } + // Preserve an explicit server reference when it differs from the canonical local reference. + if serverVariation.ModelConfigKey != canonicalLocalVariation.ModelConfigKey || + serverVariation.ModelConfigVersion != canonicalLocalVariation.ModelConfigVersion { + return serverVariation, nil + } + + serverVariation.ModelConfigVersion = 0 + // Clone the server model before removing canonical fields from the local form. + serverVariation.Model = maps.Clone(serverVariation.Model) + for key, value := range serverVariation.Model { + _, definedInLocalFile := localFileVariation.Model[key] + canonicalValue, presentInCanonical := canonicalLocalVariation.Model[key] + if !definedInLocalFile && presentInCanonical && reflect.DeepEqual(value, canonicalValue) { + delete(serverVariation.Model, key) + } + } + if len(serverVariation.Model) == 0 { + serverVariation.Model = nil + } + return serverVariation, nil +} diff --git a/internal/sync/prompt/model_config_test.go b/internal/sync/prompt/model_config_test.go new file mode 100644 index 00000000..d462811c --- /dev/null +++ b/internal/sync/prompt/model_config_test.go @@ -0,0 +1,96 @@ +package prompt + +import ( + "encoding/json" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + syncdomain "github.com/launchdarkly/ldcli/internal/sync" + syncapi "github.com/launchdarkly/ldcli/internal/sync/api" +) + +func TestCanonicalizeLocalVariationModels(t *testing.T) { + versioned := testVariation("versioned") + versioned.ModelConfigKey = "custom-model" + versioned.Model = map[string]any{ + "parameters": map[string]any{"temperature": 0.8}, + "custom": map[string]any{"region": "us-east"}, + } + unversioned := testVariation("unversioned") + unversioned.Key = "global" + unversioned.ModelConfigKey = "global-model" + unversioned.Model = map[string]any{"modelName": "global", "custom": map[string]any{"region": "us-east"}} + pinned := testVariation("pinned") + pinned.ModelConfigKey = "custom-model" + pinned.ModelConfigVersion = 2 + pinned.Model = map[string]any{"modelName": "claude-2", "parameters": map[string]any{"temperature": 0.4}} + + localFileResources := []syncdomain.SyncedResource{ + syncedVariation(t, "production", "support/versioned", versioned), + syncedVariation(t, "production", "support/global", unversioned), + syncedVariation(t, "production", "support/pinned", pinned), + } + var requested []string + + canonicalLocalResources, err := canonicalizeLocalVariationModels( + localFileResources, + func(projectKey, modelConfigKey string) (syncapi.ModelConfig, error) { + assert.Equal(t, "production", projectKey) + requested = append(requested, modelConfigKey) + return map[string]syncapi.ModelConfig{ + "custom-model": {Key: "custom-model", ID: "claude-4", Version: 4, Params: map[string]any{"temperature": 0.2}}, + "global-model": {Key: "global-model", ID: "global"}, + }[modelConfigKey], nil + }, + ) + + require.NoError(t, err) + require.Equal(t, []string{"custom-model", "global-model"}, requested) + + var canonicalVersioned syncdomain.Variation + require.NoError(t, json.Unmarshal(canonicalLocalResources[0].Payload, &canonicalVersioned)) + assert.Equal(t, 4, canonicalVersioned.ModelConfigVersion) + assert.Equal(t, map[string]any{ + "modelName": "claude-4", + "parameters": map[string]any{"temperature": 0.8}, + "custom": map[string]any{"region": "us-east"}, + }, canonicalVersioned.Model) + + var canonicalUnversioned syncdomain.Variation + require.NoError(t, json.Unmarshal(canonicalLocalResources[1].Payload, &canonicalUnversioned)) + assert.Equal(t, unversioned, canonicalUnversioned) + + var canonicalPinned syncdomain.Variation + require.NoError(t, json.Unmarshal(canonicalLocalResources[2].Payload, &canonicalPinned)) + assert.Equal(t, pinned, canonicalPinned) + + var localFileVersioned syncdomain.Variation + require.NoError(t, json.Unmarshal(localFileResources[0].Payload, &localFileVersioned)) + assert.Equal(t, versioned, localFileVersioned) +} + +func TestCanonicalizeLocalVariationModelsRejectsUnknownConfig(t *testing.T) { + variation := testVariation("local") + variation.ModelConfigKey = "missing" + + _, err := canonicalizeLocalVariationModels( + []syncdomain.SyncedResource{syncedVariation(t, "production", "support/default", variation)}, + func(string, string) (syncapi.ModelConfig, error) { + return syncapi.ModelConfig{}, errors.New("model config not found") + }, + ) + + require.ErrorContains(t, err, "model config not found") +} + +func syncedVariation(t *testing.T, projectKey, lookupKey string, variation syncdomain.Variation) syncdomain.SyncedResource { + t.Helper() + payload, err := json.Marshal(variation) + require.NoError(t, err) + return syncdomain.SyncedResource{ + Kind: syncdomain.KindVariation, ProjectKey: projectKey, LookupKey: lookupKey, Payload: payload, + } +} diff --git a/internal/sync/prompt/runner.go b/internal/sync/prompt/runner.go index 990de4b9..325e645f 100644 --- a/internal/sync/prompt/runner.go +++ b/internal/sync/prompt/runner.go @@ -53,6 +53,8 @@ type syncWorkspace struct { manifest syncmanifest.Store } +type localFileResourcesByID map[ResourceID]syncdomain.SyncedResource + // Runner coordinates prompt synchronization using existing config APIs. type Runner struct { client resources.Client @@ -205,7 +207,7 @@ func (runner Runner) runWorkspaceSync(options Options, workspace syncWorkspace) if err != nil { return err } - reviewedPlan, err := loadWorkspacePlan(workspace.root, baseline, apiClient) + reviewedPlan, _, err := loadWorkspacePlan(workspace.root, baseline, apiClient) if err != nil { return err } @@ -240,7 +242,7 @@ func (runner Runner) runWorkspaceSync(options Options, workspace syncWorkspace) if err != nil { return err } - currentPlan, err := loadWorkspacePlan(workspace.root, currentManifest, apiClient) + currentPlan, currentLocalFiles, err := loadWorkspacePlan(workspace.root, currentManifest, apiClient) if err != nil { return err } @@ -254,7 +256,14 @@ func (runner Runner) runWorkspaceSync(options Options, workspace syncWorkspace) // Apply the user's conflict choices to freshly read state, never to the // potentially stale objects that were rendered during review. currentPlan = applyConflictResolutions(currentPlan, conflictResult.resolutions) - outcomes, updatedManifest, executionErr := executePlan(workspace.root, workspace.local, apiClient, currentManifest, currentPlan) + outcomes, updatedManifest, executionErr := executePlan( + workspace.root, + workspace.local, + apiClient, + currentManifest, + currentPlan, + currentLocalFiles, + ) if err := workspace.manifest.Write(updatedManifest); err != nil { executionErr = errors.Join(executionErr, err) } @@ -287,16 +296,28 @@ func validateOptions(options Options) error { return nil } -// loadWorkspacePlan reads local and server state before building a three-way plan. -func loadWorkspacePlan(repositoryRoot string, baseline syncmanifest.Manifest, client syncapi.Client) (Plan, error) { - localResources, err := synclocal.CompileWorkspace(repositoryRoot) +// loadWorkspacePlan keeps local file data separate from the canonical local +// resources used by the three-way plan. +func loadWorkspacePlan( + repositoryRoot string, + baseline syncmanifest.Manifest, + client syncapi.Client, +) (Plan, localFileResourcesByID, error) { + localFileResources, err := synclocal.CompileWorkspace(repositoryRoot) + if err != nil { + return Plan{}, nil, err + } + canonicalLocalResources, err := canonicalizeLocalVariationModels(localFileResources, client.ModelConfig) if err != nil { - return Plan{}, err + return Plan{}, nil, err } - resourceIDs := make(map[ResourceID]struct{}, len(localResources)+len(baseline.Resources)) - for _, resource := range localResources { - resourceIDs[ResourceID{Kind: resource.Kind, ProjectKey: resource.ProjectKey, LookupKey: resource.LookupKey}] = struct{}{} + localFilesByID := make(localFileResourcesByID, len(localFileResources)) + resourceIDs := make(map[ResourceID]struct{}, len(localFileResources)+len(baseline.Resources)) + for _, resource := range localFileResources { + id := ResourceID{Kind: resource.Kind, ProjectKey: resource.ProjectKey, LookupKey: resource.LookupKey} + localFilesByID[id] = resource + resourceIDs[id] = struct{}{} } for _, resource := range baseline.Resources { resourceIDs[resource.ID()] = struct{}{} @@ -306,11 +327,11 @@ func loadWorkspacePlan(repositoryRoot string, baseline syncmanifest.Manifest, cl for id := range resourceIDs { resource, err := readServerResource(client, id) if err != nil { - return Plan{}, err + return Plan{}, nil, err } serverResources[id] = resource } - return BuildPlan(baseline, localResources, serverResources), nil + return BuildPlan(baseline, canonicalLocalResources, serverResources), localFilesByID, nil } // samePlanState reports whether every reviewed decision still has the same inputs.