From 63b17487591011eb7c8f09ad8086ce69b96775fe Mon Sep 17 00:00:00 2001 From: Clifford Tawiah Date: Thu, 8 Oct 2026 17:33:19 -0400 Subject: [PATCH 1/5] feat(sync): restore missing files and archive only with detach --archive A missing local file made sync archive the variation in LaunchDarkly. A file is often missing only because the working copy does not have it yet, for example after a Git pull or a branch switch. A plain sync could then archive a variation that other engineers use. Sync now restores a missing file from LaunchDarkly. The sync plan never archives a variation. "ldcli sync prompt detach --archive" is the only way to archive one. It archives each selected variation, and then stops syncing it. It asks for confirmation, and with --no-input it needs --yes. The archive runs first, and an archived variation does not fail, so the same command can run again after a later step fails. The confirmation prompt moves to the interactive package, so that sync and detach use the same prompt. --- cmd/sync/prompt.go | 13 ++++- internal/sync/detach/detach.go | 75 +++++++++++++++++++++++-- internal/sync/detach/detach_test.go | 67 ++++++++++++++++++++++ internal/sync/interactive/confirm.go | 59 +++++++++++++++++++ internal/sync/prompt/acceptance_test.go | 17 ++++-- internal/sync/prompt/action.go | 4 +- internal/sync/prompt/conflict.go | 9 +-- internal/sync/prompt/conflict_test.go | 1 - internal/sync/prompt/execute.go | 11 +--- internal/sync/prompt/output.go | 11 +--- internal/sync/prompt/output_test.go | 15 ----- internal/sync/prompt/plan.go | 40 ++++++------- internal/sync/prompt/plan_test.go | 13 ++--- internal/sync/prompt/render_diff.go | 9 +-- internal/sync/prompt/review.go | 59 +------------------ internal/sync/prompt/review_test.go | 13 +++-- internal/sync/prompt/runner.go | 3 + 17 files changed, 267 insertions(+), 152 deletions(-) create mode 100644 internal/sync/interactive/confirm.go diff --git a/cmd/sync/prompt.go b/cmd/sync/prompt.go index dd3760618..e938f7b56 100644 --- a/cmd/sync/prompt.go +++ b/cmd/sync/prompt.go @@ -19,6 +19,7 @@ import ( ) const ( + archiveFlag = "archive" conflictFlag = "conflict" contentFlag = "content" dryRunFlag = "dry-run" @@ -214,16 +215,22 @@ func newDetachCmd(client resources.Client) *cobra.Command { ldcli sync prompt detach # Stop syncing one variation - ldcli sync prompt detach production/support/default`, + ldcli sync prompt detach production/support/default + + # Stop syncing one variation and archive it in LaunchDarkly + ldcli sync prompt detach production/support/default --archive --yes --no-input`, Args: validatedArgs(cobra.MinimumNArgs(0)), - RunE: runPrompt(client, func(_ *cobra.Command, args []string) (syncprompt.CommandAction, error) { + RunE: runPrompt(client, func(cmd *cobra.Command, args []string) (syncprompt.CommandAction, error) { variations, err := parseVariationSelectors(args) if err != nil { return nil, err } - return syncprompt.DetachAction{Variations: variations}, nil + archive, _ := cmd.Flags().GetBool(archiveFlag) + return syncprompt.DetachAction{Variations: variations, Archive: archive}, nil }), } + cmd.Flags().Bool(archiveFlag, false, "Also archive the variations in LaunchDarkly") + cmd.Flags().Bool(yesFlag, false, "Archive without confirmation") cmd.Flags().Bool(noInputFlag, false, "Fail instead of prompting for missing input") return cmd } diff --git a/internal/sync/detach/detach.go b/internal/sync/detach/detach.go index c4ed1d823..18119ada2 100644 --- a/internal/sync/detach/detach.go +++ b/internal/sync/detach/detach.go @@ -1,20 +1,30 @@ // Package detach stops syncing selected variations. It removes their local -// files and their baseline. LaunchDarkly keeps the variations. +// files and their baseline. LaunchDarkly keeps the variations, unless the +// user asks to archive them. package detach import ( + "context" "errors" "fmt" "io" "slices" syncdomain "github.com/launchdarkly/ldcli/internal/sync" + syncapi "github.com/launchdarkly/ldcli/internal/sync/api" syncconsole "github.com/launchdarkly/ldcli/internal/sync/console" syncinteractive "github.com/launchdarkly/ldcli/internal/sync/interactive" synclocal "github.com/launchdarkly/ldcli/internal/sync/local" syncmanifest "github.com/launchdarkly/ldcli/internal/sync/manifest" ) +const archiveQuestion = "\nArchive these variations in LaunchDarkly? [y/N] " + +// Archiver archives a variation in LaunchDarkly. +type Archiver interface { + ArchiveVariation(projectKey, configKey, variationKey string) error +} + // Options are the dependencies and the input of one detach. type Options struct { RepositoryRoot string @@ -27,6 +37,11 @@ type Options struct { // Selections are the variations to detach. If it is empty, Run asks the user. Selections []syncdomain.ResourceID NoInput bool + // Archive also archives each selected variation in LaunchDarkly. Without + // Yes, the user must confirm the archive. + Archive bool + Archiver Archiver + Yes bool } // Run detaches the selected variations. @@ -60,11 +75,26 @@ func Run(options Options) error { return err } } + if options.Archive { + confirmed, err := confirmArchive(options, selected) + if err != nil || !confirmed { + return err + } + // Archive first. If a later step fails, the same command can run again, + // because a variation that is already archived does not fail. + if err := archiveVariations(options.Archiver, selected); err != nil { + return err + } + } if err := detachResources(options, baseline, selected); err != nil { return err } - _ = console.Line("Detached resources:") + if options.Archive { + _ = console.Line("Detached and archived resources:") + } else { + _ = console.Line("Detached resources:") + } for _, resource := range selected { _ = console.Printf("- %s %s\n", resource.Kind, resource) } @@ -85,9 +115,44 @@ func promptForResources(options Options, synced []syncdomain.ResourceID) ([]sync Title: resource.String(), Description: string(resource.Kind), Value: resource, }) } - return syncinteractive.MultiSelect( - options.Input, options.Output, "Select resources to detach", "Detached resources remain in LaunchDarkly.", choices, - ) + description := "Detached resources remain in LaunchDarkly." + if options.Archive { + description = "Detached resources are also archived in LaunchDarkly." + } + return syncinteractive.MultiSelect(options.Input, options.Output, "Select resources to detach", description, choices) +} + +// confirmArchive asks the user to agree to the archive, unless Yes is set. +func confirmArchive(options Options, selected []syncdomain.ResourceID) (bool, error) { + if options.Yes { + return true, nil + } + console := syncconsole.New(options.Output) + _ = console.Line("Variations to archive in LaunchDarkly:") + for _, resource := range selected { + _ = console.Printf("- %s\n", resource) + } + interactive := !options.NoInput && syncinteractive.StreamsAreTerminal(options.Input, options.Output) + confirmed, err := syncinteractive.Confirm(context.Background(), options.Input, options.Output, interactive, archiveQuestion) + if err == nil && !confirmed { + _ = console.Line("Detach canceled.") + } + return confirmed, err +} + +// archiveVariations archives each variation. A variation that LaunchDarkly +// does not have, or that is already archived, does not fail. +func archiveVariations(archiver Archiver, selected []syncdomain.ResourceID) error { + for _, resource := range selected { + configKey, variationKey, err := resource.VariationKeys() + if err != nil { + return err + } + if err := archiver.ArchiveVariation(resource.ProjectKey, configKey, variationKey); err != nil && !syncapi.IsNotFound(err) { + return fmt.Errorf("archive variation %s: %w", resource, err) + } + } + return nil } func validateSelections(synced, selected []syncdomain.ResourceID) error { diff --git a/internal/sync/detach/detach_test.go b/internal/sync/detach/detach_test.go index 7e6a27cb0..50ed89b91 100644 --- a/internal/sync/detach/detach_test.go +++ b/internal/sync/detach/detach_test.go @@ -2,6 +2,7 @@ package detach import ( "bytes" + "errors" "os" "path/filepath" "strings" @@ -256,6 +257,72 @@ func TestRunRejectsExplicitSelectionWhenNoResourcesAreSynced(t *testing.T) { assert.Empty(t, output.String()) } +func TestRunArchive(t *testing.T) { + notFound := errors.New(`{"code":"not_found","statusCode":404}`) + tests := map[string]struct { + archive bool + yes bool + archiveErr error + wantError string + wantArchived []string + wantDetached bool + }{ + "detach without archive keeps the variation in LaunchDarkly": {wantDetached: true}, + "archive with yes": {archive: true, yes: true, wantArchived: []string{"project/config/prompt"}, wantDetached: true}, + "variation already archived": {archive: true, yes: true, archiveErr: notFound, wantArchived: []string{"project/config/prompt"}, wantDetached: true}, + "archive needs a confirmation": {archive: true, wantError: "rerun with --yes"}, + "archive failure stops detach": { + archive: true, yes: true, archiveErr: errors.New("forbidden"), + wantError: "archive variation project/config/prompt", wantArchived: []string{"project/config/prompt"}, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + root := t.TempDir() + store := synclocal.NewStore(root) + _, err := store.Add([]synclocal.VariationFile{{ + ProjectKey: "project", ConfigKey: "config", Variation: testVariation("prompt"), + }}) + require.NoError(t, err) + archiver := &recordingArchiver{err: test.archiveErr} + + err = Run(Options{ + RepositoryRoot: root, + Store: store, + Baselines: newMemoryManifestStore(), + Input: bytes.NewBuffer(nil), + Output: bytes.NewBuffer(nil), + Selections: []syncdomain.ResourceID{syncdomain.VariationID("project", "config", "prompt")}, + NoInput: true, + Archive: test.archive, + Archiver: archiver, + Yes: test.yes, + }) + + if test.wantError != "" { + require.ErrorContains(t, err, test.wantError) + } else { + require.NoError(t, err) + } + assert.Equal(t, test.wantArchived, archiver.archived) + exists, err := store.VariationExists("project", "config", "prompt") + require.NoError(t, err) + assert.Equal(t, !test.wantDetached, exists) + }) + } +} + +type recordingArchiver struct { + archived []string + err error +} + +func (archiver *recordingArchiver) ArchiveVariation(projectKey, configKey, variationKey string) error { + archiver.archived = append(archiver.archived, projectKey+"/"+configKey+"/"+variationKey) + return archiver.err +} + type memoryManifestStore struct { manifest syncmanifest.Manifest loadedProjectKeys []string diff --git a/internal/sync/interactive/confirm.go b/internal/sync/interactive/confirm.go new file mode 100644 index 000000000..91520c421 --- /dev/null +++ b/internal/sync/interactive/confirm.go @@ -0,0 +1,59 @@ +package interactive + +import ( + "bufio" + "context" + "errors" + "fmt" + "io" + "strings" +) + +// Confirm asks a yes or no question and stops when ctx ends. A nil ctx never +// ends. Without a terminal, it returns an error that suggests --yes. +func Confirm(ctx context.Context, input io.Reader, output io.Writer, interactive bool, question string) (bool, error) { + if !interactive { + return false, errors.New("interactive confirmation requires a terminal; rerun with --yes to apply non-interactively") + } + if ctx == nil { + ctx = context.Background() + } + if err := ctx.Err(); err != nil { + return false, err + } + + // A read from io.Reader cannot stop when ctx ends, so read in a goroutine. + // The channel has a buffer, so the goroutine can finish after confirm + // returns. + type answer struct { + confirmed bool + err error + } + answers := make(chan answer, 1) + go func() { + confirmed, err := readConfirmation(input, output, question) + answers <- answer{confirmed: confirmed, err: err} + }() + + select { + case <-ctx.Done(): + return false, ctx.Err() + case result := <-answers: + if err := ctx.Err(); err != nil { + return false, err + } + return result.confirmed, result.err + } +} + +func readConfirmation(input io.Reader, output io.Writer, question string) (bool, error) { + if _, err := io.WriteString(output, question); err != nil { + return false, err + } + line, err := bufio.NewReader(input).ReadString('\n') + if err != nil && err != io.EOF { + return false, fmt.Errorf("read confirmation: %w", err) + } + line = strings.ToLower(strings.TrimSpace(line)) + return line == "y" || line == "yes", nil +} diff --git a/internal/sync/prompt/acceptance_test.go b/internal/sync/prompt/acceptance_test.go index 81bd5c891..e14f91fb6 100644 --- a/internal/sync/prompt/acceptance_test.go +++ b/internal/sync/prompt/acceptance_test.go @@ -789,17 +789,19 @@ func TestPromptStaleWorkingCopyPullsInsteadOfReverting(t *testing.T) { assert.Equal(t, other.Name, resources[0].Variation.Name) } -func TestPromptPropagatesTrackedLocalDeletion(t *testing.T) { +func TestPromptRestoresTrackedMissingFileInsteadOfArchiving(t *testing.T) { root := initRepository(t) baseline := variation("Baseline") writeVariation(t, root, baseline, false) writeManifest(t, root, baseline) api := &directAPI{variation: pointer(baseline)} - // The first sync writes sync.lock, which tracks the project after its - // files are gone. + // The first sync of a workspace from before sync.lock writes the lock. _, _, err := runPrompt(t, root, api, "--yes") require.NoError(t, err) + _, err = os.Stat(filepath.Join(root, syncdomain.RootDir, "sync.lock")) + require.NoError(t, err) + _, err = synclocal.NewStore(root).DeleteVariations([]synclocal.VariationDeletion{{ ProjectKey: "production", ConfigKey: "support", VariationKey: "default", }}) @@ -808,9 +810,12 @@ func TestPromptPropagatesTrackedLocalDeletion(t *testing.T) { _, _, err = runPrompt(t, root, api, "--yes") require.NoError(t, err) - require.NotNil(t, api.variation) - assert.Equal(t, "archived", api.variationState) - assert.Empty(t, manifestsByRoot[root].Items) + assert.NotEqual(t, "archived", api.variationState) + resources, err := synclocal.CompileWorkspace(root) + require.NoError(t, err) + require.Len(t, resources, 1) + assert.Equal(t, baseline.Name, resources[0].Variation.Name) + assert.NotEmpty(t, manifestsByRoot[root].Items) } func TestPromptPropagatesTrackedServerDeletion(t *testing.T) { diff --git a/internal/sync/prompt/action.go b/internal/sync/prompt/action.go index 201de5154..8549479f5 100644 --- a/internal/sync/prompt/action.go +++ b/internal/sync/prompt/action.go @@ -34,9 +34,11 @@ type AttachAction struct { Target *syncdomain.ResourceID } -// DetachAction stops syncing selected local variations. +// DetachAction stops syncing selected local variations. With Archive, it +// also archives them in LaunchDarkly. type DetachAction struct { Variations []syncdomain.ResourceID + Archive bool } // LinkAction creates a variation whose prompt is an external file, and then syncs. diff --git a/internal/sync/prompt/conflict.go b/internal/sync/prompt/conflict.go index 3abd0ac5b..d07ba8ce7 100644 --- a/internal/sync/prompt/conflict.go +++ b/internal/sync/prompt/conflict.go @@ -307,14 +307,11 @@ func resolvedConflictAction(resource PlannedResource, resolution ConflictResolut } return ActionUpdateLocal case ConflictUseLocal: - switch { - case resource.Local == nil: - return ActionArchiveServer - case resource.Server == nil: + // A conflict always has a local file, because a missing file is restored. + if resource.Server == nil { return ActionCreateServer - default: - return ActionUpdateServer } + return ActionUpdateServer default: return ActionConflict } diff --git a/internal/sync/prompt/conflict_test.go b/internal/sync/prompt/conflict_test.go index 26089b557..d00ab37a5 100644 --- a/internal/sync/prompt/conflict_test.go +++ b/internal/sync/prompt/conflict_test.go @@ -32,7 +32,6 @@ func TestResolvedConflictAction(t *testing.T) { {"LaunchDarkly deletion removes the local resource", PlannedResource{Local: &local}, ConflictUseLaunchDarkly, ActionDeleteLocal}, {"local updates an existing server resource", PlannedResource{Local: &local, Server: &server}, ConflictUseLocal, ActionUpdateServer}, {"local creates a missing server resource", PlannedResource{Local: &local}, ConflictUseLocal, ActionCreateServer}, - {"local deletion archives the server resource", PlannedResource{Server: &server}, ConflictUseLocal, ActionArchiveServer}, } for _, test := range tests { diff --git a/internal/sync/prompt/execute.go b/internal/sync/prompt/execute.go index 7180a04b9..a6e05a046 100644 --- a/internal/sync/prompt/execute.go +++ b/internal/sync/prompt/execute.go @@ -97,7 +97,7 @@ func attachmentManifestState(resource PlannedResource) *syncdomain.Variation { // recordSuccessfulChange records the state that a completed action chose. func recordSuccessfulChange(manifest *syncmanifest.Manifest, resource PlannedResource) { switch { - case resource.Action == ActionArchiveServer || resource.Action == ActionDeleteLocal: + case resource.Action == ActionDeleteLocal: manifest.Remove(resource.ID) case resource.Action.changesServer(): manifest.SetFingerprint(resource.ID, resource.LocalFingerprint) @@ -175,8 +175,6 @@ func applyServerChange(client syncapi.Client, attachments *attachmentResolver, r return err } writeErr = client.UpdateVariation(projectKey, configKey, variation) - case ActionArchiveServer: - writeErr = client.ArchiveVariation(projectKey, configKey, variationKey) default: return fmt.Errorf("action %q does not change LaunchDarkly", resource.Action) } @@ -197,13 +195,8 @@ func applyServerChange(client syncapi.Client, attachments *attachmentResolver, r return errors.Join(writeErr, err) } } - expected := resource.LocalFingerprint - if resource.Action == ActionArchiveServer { - expected = "" - } - switch actual { - case expected: + case resource.LocalFingerprint: return nil case resource.ServerFingerprint: return writeErr diff --git a/internal/sync/prompt/output.go b/internal/sync/prompt/output.go index 8f22c2d2b..1714c4e6b 100644 --- a/internal/sync/prompt/output.go +++ b/internal/sync/prompt/output.go @@ -220,8 +220,6 @@ func actionDescription(action Action) string { return "Create the variation in LaunchDarkly" case ActionUpdateServer: return "Update LaunchDarkly from the local file" - case ActionArchiveServer: - return "Archive the variation in LaunchDarkly" case ActionUpdateLocal: return "Update the local file from LaunchDarkly" case ActionDeleteLocal: @@ -241,9 +239,8 @@ func actionDescription(action Action) string { // variationDiffPresentation is the labels and the direction of a diff. type variationDiffPresentation struct { - beforeLabel string - afterLabel string - missingAfter string + beforeLabel string + afterLabel string // reverse shows the local file as "before", because the local file changes. reverse bool } @@ -255,10 +252,6 @@ func diffPresentation(action Action) variationDiffPresentation { return variationDiffPresentation{beforeLabel: "Local file now", afterLabel: "Local file after sync", reverse: true} case ActionCreateServer, ActionUpdateServer: return variationDiffPresentation{beforeLabel: "LaunchDarkly now", afterLabel: "LaunchDarkly after sync"} - case ActionArchiveServer: - return variationDiffPresentation{ - beforeLabel: "LaunchDarkly now", afterLabel: "LaunchDarkly after sync", missingAfter: "(archived in LaunchDarkly)", - } default: return variationDiffPresentation{beforeLabel: "LaunchDarkly now", afterLabel: "Local file now"} } diff --git a/internal/sync/prompt/output_test.go b/internal/sync/prompt/output_test.go index d09b661a7..be3c7e6aa 100644 --- a/internal/sync/prompt/output_test.go +++ b/internal/sync/prompt/output_test.go @@ -42,21 +42,6 @@ func TestWritePlanDescribesMissingLaunchDarklyVariation(t *testing.T) { assert.NotContains(t, output.String(), "-null") } -func TestWritePlanDescribesArchivedLaunchDarklyVariation(t *testing.T) { - variation := syncdomain.Variation{Mode: syncdomain.VariationModeCompletion, Key: "default", Name: "Default"} - plan := Plan{Resources: []PlannedResource{{ - ID: testResourceID(), - Action: ActionArchiveServer, - Diff: variationDiff(&variation, nil), - }}} - - var output bytes.Buffer - require.NoError(t, writePlanOutput(&output, "plaintext", plan)) - - assert.Contains(t, output.String(), "Action: Archive the variation in LaunchDarkly") - assert.Contains(t, output.String(), "(archived in LaunchDarkly)") -} - func TestWritePlanNotesResourceSyncedByAnotherWorkingCopy(t *testing.T) { plan := Plan{Resources: []PlannedResource{{ID: testResourceID(), Action: ActionUpdateLocal, SyncedElsewhere: true}}} diff --git a/internal/sync/prompt/plan.go b/internal/sync/prompt/plan.go index f58cfef12..05ee2e978 100644 --- a/internal/sync/prompt/plan.go +++ b/internal/sync/prompt/plan.go @@ -15,7 +15,6 @@ const ( ActionInSync Action = "in_sync" ActionCreateServer Action = "create_server" ActionUpdateServer Action = "update_server" - ActionArchiveServer Action = "archive_server" ActionUpdateLocal Action = "update_local" ActionDeleteLocal Action = "delete_local" ActionUpdateManifest Action = "update_manifest" @@ -26,7 +25,7 @@ const ( // changesServer reports whether the action writes to LaunchDarkly. func (action Action) changesServer() bool { - return action == ActionCreateServer || action == ActionUpdateServer || action == ActionArchiveServer + return action == ActionCreateServer || action == ActionUpdateServer } // changesLocal reports whether the action writes a local file. @@ -200,6 +199,15 @@ func chooseAction(tracked bool, resource PlannedResource) Action { } } + // A missing local file means that this working copy does not have the + // variation, for example after a Git pull or a branch switch. Sync restores + // the file. Only "detach --archive" archives a variation. + if !localExists && serverExists { + return ActionUpdateLocal + } + + // A tracked side that exists has a fingerprint, so "unchanged" also means + // that the side exists. localUnchanged := resource.LocalFingerprint == resource.BaselineFingerprint serverUnchanged := resource.ServerFingerprint == resource.BaselineFingerprint switch { @@ -207,30 +215,19 @@ func chooseAction(tracked bool, resource PlannedResource) Action { return ActionInSync case resource.LocalFingerprint == resource.ServerFingerprint: // Both sides changed to the same state, which can be a deletion. - if !localExists && !serverExists { + if !localExists { return ActionRemoveManifest } return ActionUpdateManifest case serverUnchanged: // Only the local file changed, so LaunchDarkly follows it. - switch { - case !localExists: - return ActionArchiveServer - case serverExists: - return ActionUpdateServer - default: - return ActionConflict - } + return ActionUpdateServer + case localUnchanged && !serverExists: + // The variation was archived in LaunchDarkly, so the local file goes too. + return ActionDeleteLocal case localUnchanged: // Only LaunchDarkly changed, so the local file follows it. - switch { - case !serverExists: - return ActionDeleteLocal - case localExists: - return ActionUpdateLocal - default: - return ActionConflict - } + return ActionUpdateLocal default: return ActionConflict } @@ -244,11 +241,10 @@ func (plan Plan) RequiresConfirmation() bool { }) } -// HasDestructiveActions reports whether the plan deletes a local file or -// archives a variation in LaunchDarkly. +// HasDestructiveActions reports whether the plan deletes a local file. func (plan Plan) HasDestructiveActions() bool { return slices.ContainsFunc(plan.Resources, func(resource PlannedResource) bool { - return resource.Action == ActionArchiveServer || resource.Action == ActionDeleteLocal + return resource.Action == ActionDeleteLocal }) } diff --git a/internal/sync/prompt/plan_test.go b/internal/sync/prompt/plan_test.go index 471a0503d..2d31c9cb8 100644 --- a/internal/sync/prompt/plan_test.go +++ b/internal/sync/prompt/plan_test.go @@ -25,11 +25,11 @@ func TestBuildPlanThreeWayMatrix(t *testing.T) { {"server edit", &baseline, &serverChange, ActionUpdateLocal}, {"same edit", &localChange, &localChange, ActionUpdateManifest}, {"divergent edits", &localChange, &serverChange, ActionConflict}, - {"local deletion", nil, &baseline, ActionArchiveServer}, + {"missing local file is restored", nil, &baseline, ActionUpdateLocal}, {"server deletion", &baseline, nil, ActionDeleteLocal}, {"both deleted", nil, nil, ActionRemoveManifest}, {"local edit after server deletion", &localChange, nil, ActionConflict}, - {"server edit after local deletion", nil, &serverChange, ActionConflict}, + {"missing local file after a server edit is restored", nil, &serverChange, ActionUpdateLocal}, } for _, test := range tests { @@ -138,11 +138,10 @@ func TestPlanHasDestructiveActions(t *testing.T) { action Action destructive bool }{ - "create server": {action: ActionCreateServer}, - "update server": {action: ActionUpdateServer}, - "archive server": {action: ActionArchiveServer, destructive: true}, - "update local": {action: ActionUpdateLocal}, - "delete local": {action: ActionDeleteLocal, destructive: true}, + "create server": {action: ActionCreateServer}, + "update server": {action: ActionUpdateServer}, + "update local": {action: ActionUpdateLocal}, + "delete local": {action: ActionDeleteLocal, destructive: true}, } for name, test := range tests { diff --git a/internal/sync/prompt/render_diff.go b/internal/sync/prompt/render_diff.go index 2a19e9f84..a702bc7e0 100644 --- a/internal/sync/prompt/render_diff.go +++ b/internal/sync/prompt/render_diff.go @@ -110,11 +110,11 @@ func diffSections(field string, diff variationFieldDiff, presentation variationD ) } - before, err := formatDiffValue(diff.Before, presentation.beforeLabel, "") + before, err := formatDiffValue(diff.Before, presentation.beforeLabel) if err != nil { return nil, err } - after, err := formatDiffValue(diff.After, presentation.afterLabel, presentation.missingAfter) + after, err := formatDiffValue(diff.After, presentation.afterLabel) if err != nil { return nil, err } @@ -473,11 +473,8 @@ func changedParts(before, after string) (prefix, removed, added, suffix string) // formatDiffValue pretty-prints JSON and substitutes readable absence markers // for missing local or LaunchDarkly resources. -func formatDiffValue(value json.RawMessage, label string, missingValue string) (string, error) { +func formatDiffValue(value json.RawMessage, label string) (string, error) { if len(value) == 0 || bytes.Equal(value, []byte("null")) { - if missingValue != "" { - return missingValue, nil - } if strings.HasPrefix(label, "LaunchDarkly") { return "(does not exist in LaunchDarkly)", nil } diff --git a/internal/sync/prompt/review.go b/internal/sync/prompt/review.go index 12bf0ab21..bee786e1e 100644 --- a/internal/sync/prompt/review.go +++ b/internal/sync/prompt/review.go @@ -1,16 +1,12 @@ package prompt import ( - "bufio" - "context" - "errors" - "fmt" - "io" "slices" "strings" syncdomain "github.com/launchdarkly/ldcli/internal/sync" syncconsole "github.com/launchdarkly/ldcli/internal/sync/console" + syncinteractive "github.com/launchdarkly/ldcli/internal/sync/interactive" synclocal "github.com/launchdarkly/ldcli/internal/sync/local" ) @@ -39,7 +35,7 @@ func reviewAndConfirmPlan(options Options, plan Plan, interactive bool) (bool, e return true, nil } - confirmed, err := confirm(options.Context, options.Input, options.ErrorOutput, interactive, applyQuestion) + confirmed, err := syncinteractive.Confirm(options.Context, options.Input, options.ErrorOutput, interactive, applyQuestion) if err != nil { return false, err } @@ -49,55 +45,6 @@ func reviewAndConfirmPlan(options Options, plan Plan, interactive bool) (bool, e return confirmed, nil } -// confirm asks a yes or no question and stops when ctx ends. A nil ctx never -// ends. -func confirm(ctx context.Context, input io.Reader, output io.Writer, interactive bool, question string) (bool, error) { - if !interactive { - return false, errors.New("interactive confirmation requires a terminal; rerun with --yes to apply non-interactively") - } - if ctx == nil { - ctx = context.Background() - } - if err := ctx.Err(); err != nil { - return false, err - } - - // A read from io.Reader cannot stop when ctx ends, so read in a goroutine. - // The channel has a buffer, so the goroutine can finish after confirm - // returns. - type answer struct { - confirmed bool - err error - } - answers := make(chan answer, 1) - go func() { - confirmed, err := readConfirmation(input, output, question) - answers <- answer{confirmed: confirmed, err: err} - }() - - select { - case <-ctx.Done(): - return false, ctx.Err() - case result := <-answers: - if err := ctx.Err(); err != nil { - return false, err - } - return result.confirmed, result.err - } -} - -func readConfirmation(input io.Reader, output io.Writer, question string) (bool, error) { - if err := syncconsole.New(output).Write(question); err != nil { - return false, err - } - line, err := bufio.NewReader(input).ReadString('\n') - if err != nil && err != io.EOF { - return false, fmt.Errorf("read apply confirmation: %w", err) - } - line = strings.ToLower(strings.TrimSpace(line)) - return line == "y" || line == "yes", nil -} - // cleanupOrphanedAttachments shows the tool and skill files that no local // variation uses, and deletes them after the user agrees. Without --yes and // without a terminal, it keeps the files. Before it deletes a file, it makes @@ -127,7 +74,7 @@ func cleanupOrphanedAttachments(options Options, store synclocal.Store, interact return nil } if !options.Yes { - confirmed, err := confirm( + confirmed, err := syncinteractive.Confirm( options.Context, options.Input, options.ErrorOutput, interactive, "\nDelete these unreferenced local files? [y/N] ", ) diff --git a/internal/sync/prompt/review_test.go b/internal/sync/prompt/review_test.go index 1175d5f55..a6a726fa7 100644 --- a/internal/sync/prompt/review_test.go +++ b/internal/sync/prompt/review_test.go @@ -15,6 +15,7 @@ import ( "github.com/stretchr/testify/require" syncdomain "github.com/launchdarkly/ldcli/internal/sync" + syncinteractive "github.com/launchdarkly/ldcli/internal/sync/interactive" synclocal "github.com/launchdarkly/ldcli/internal/sync/local" ) @@ -58,7 +59,7 @@ func TestConfirmApply(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { var prompt bytes.Buffer - confirmed, err := confirm(context.Background(), strings.NewReader(test.input), &prompt, test.terminal, applyQuestion) + confirmed, err := syncinteractive.Confirm(context.Background(), strings.NewReader(test.input), &prompt, test.terminal, applyQuestion) assert.Equal(t, test.confirmed, confirmed) if test.wantError != "" { @@ -174,7 +175,7 @@ func TestReviewAndConfirmPlanStopsWhenWatchContextIsCanceled(t *testing.T) { confirmed, err := reviewAndConfirmPlan(Options{ Action: SyncAction{Watch: true}, Context: ctx, Input: input, ErrorOutput: output, }, Plan{Resources: []PlannedResource{{ - ID: testResourceID(), Action: ActionArchiveServer, + ID: testResourceID(), Action: ActionDeleteLocal, }}}, true) results <- struct { confirmed bool @@ -208,7 +209,7 @@ func TestReviewAndConfirmPlanRejectsConfirmationWhenWatchContextIsAlreadyCancele confirmed, err := reviewAndConfirmPlan(Options{ Action: SyncAction{Watch: true}, Context: ctx, Input: strings.NewReader("yes\n"), ErrorOutput: io.Discard, }, Plan{Resources: []PlannedResource{{ - ID: testResourceID(), Action: ActionArchiveServer, + ID: testResourceID(), Action: ActionDeleteLocal, }}}, true) require.False(t, confirmed) @@ -228,7 +229,7 @@ func TestReviewAndConfirmPlanWatchPolicy(t *testing.T) { }{ {name: "updates apply automatically", action: ActionUpdateServer, wantContinue: true}, { - name: "server archive requires confirmation", action: ActionArchiveServer, input: "yes\n", + name: "local deletion requires confirmation", action: ActionDeleteLocal, input: "yes\n", interactive: true, wantContinue: true, wantPrompt: true, }, { @@ -236,11 +237,11 @@ func TestReviewAndConfirmPlanWatchPolicy(t *testing.T) { interactive: true, wantPrompt: true, }, { - name: "non-terminal destructive action is rejected", action: ActionArchiveServer, + name: "non-terminal destructive action is rejected", action: ActionDeleteLocal, wantError: "rerun with --yes", }, { - name: "explicit yes applies destructive action", action: ActionArchiveServer, + name: "explicit yes applies destructive action", action: ActionDeleteLocal, yes: true, wantContinue: true, }, } diff --git a/internal/sync/prompt/runner.go b/internal/sync/prompt/runner.go index f372147be..07524dca5 100644 --- a/internal/sync/prompt/runner.go +++ b/internal/sync/prompt/runner.go @@ -155,6 +155,9 @@ func (runner Runner) runDetach(options Options, workspace syncWorkspace, action Output: options.Output, Selections: action.Variations, NoInput: options.NoInput, + Archive: action.Archive, + Archiver: runner.api(options), + Yes: options.Yes, }) } From 431c79efab90f3ed69d4a4ba4aa258014265d012 Mon Sep 17 00:00:00 2001 From: Clifford Tawiah Date: Thu, 8 Oct 2026 19:31:12 -0400 Subject: [PATCH 2/5] fix(sync): keep links on restore and make detach --archive safe to rerun A restored variation file lost its link to the prompt file that it used. The lock now records the link of each variation, and a restore keeps it. Sync stops with a clear error, and writes nothing, when it cannot restore the link safely. This happens when the working copy has no sync.lock, when the linked file is not available, or when the linked file has local edits. Detach now reads each variation before it archives it, because the API rejects the archive of an archived variation. A rerun after a failed step skips the variations that are already archived and finishes. Detach also uses the command context, so a canceled command does not archive. The lock no longer stores the remote version of each entry. Each save still sends the version that sync read, so optimistic locking does not change. A lock entry now changes only when its content or its link changes, which keeps Git diffs and merge conflicts small. New tests cover linked restores, unsafe restores, a partial failure with several variations, a failed delete followed by a rerun, the interactive archive confirmation, and detach --archive through the CLI. --- internal/sync/detach/detach.go | 48 ++++- internal/sync/detach/detach_test.go | 237 ++++++++++++++++++++---- internal/sync/local/compile.go | 2 +- internal/sync/local/reference.go | 9 +- internal/sync/local/replace.go | 5 +- internal/sync/local/store.go | 4 +- internal/sync/local/variation.go | 1 + internal/sync/manifest/baseline.go | 8 +- internal/sync/manifest/baseline_test.go | 28 ++- internal/sync/manifest/model.go | 31 +++- internal/sync/prompt/acceptance_test.go | 123 ++++++++++++ internal/sync/prompt/execute.go | 2 + internal/sync/prompt/plan.go | 2 + internal/sync/prompt/plan_test.go | 11 ++ internal/sync/prompt/runner.go | 1 + internal/sync/prompt/state.go | 44 +++++ internal/sync/prompt/sync.go | 5 +- internal/sync/resource.go | 11 +- 18 files changed, 510 insertions(+), 62 deletions(-) diff --git a/internal/sync/detach/detach.go b/internal/sync/detach/detach.go index 18119ada2..764110510 100644 --- a/internal/sync/detach/detach.go +++ b/internal/sync/detach/detach.go @@ -20,8 +20,9 @@ import ( const archiveQuestion = "\nArchive these variations in LaunchDarkly? [y/N] " -// Archiver archives a variation in LaunchDarkly. +// Archiver reads and archives a variation in LaunchDarkly. type Archiver interface { + ReadVariation(projectKey, configKey, variationKey string) (syncapi.VariationState, error) ArchiveVariation(projectKey, configKey, variationKey string) error } @@ -42,6 +43,19 @@ type Options struct { Archive bool Archiver Archiver Yes bool + // Context stops the archive confirmation and the archive when it ends. + Context context.Context + + // isTerminal reports whether the streams are a terminal. Tests replace it. + isTerminal func(io.Reader, io.Writer) bool +} + +// context returns the context of the command, or a context that never ends. +func (options Options) context() context.Context { + if options.Context == nil { + return context.Background() + } + return options.Context } // Run detaches the selected variations. @@ -81,8 +95,8 @@ func Run(options Options) error { return err } // Archive first. If a later step fails, the same command can run again, - // because a variation that is already archived does not fail. - if err := archiveVariations(options.Archiver, selected); err != nil { + // because it skips a variation that is already archived. + if err := archiveVariations(options.context(), options.Archiver, selected); err != nil { return err } } @@ -132,22 +146,40 @@ func confirmArchive(options Options, selected []syncdomain.ResourceID) (bool, er for _, resource := range selected { _ = console.Printf("- %s\n", resource) } - interactive := !options.NoInput && syncinteractive.StreamsAreTerminal(options.Input, options.Output) - confirmed, err := syncinteractive.Confirm(context.Background(), options.Input, options.Output, interactive, archiveQuestion) + isTerminal := options.isTerminal + if isTerminal == nil { + isTerminal = syncinteractive.StreamsAreTerminal + } + interactive := !options.NoInput && isTerminal(options.Input, options.Output) + confirmed, err := syncinteractive.Confirm(options.context(), options.Input, options.Output, interactive, archiveQuestion) if err == nil && !confirmed { _ = console.Line("Detach canceled.") } return confirmed, err } -// archiveVariations archives each variation. A variation that LaunchDarkly -// does not have, or that is already archived, does not fail. -func archiveVariations(archiver Archiver, selected []syncdomain.ResourceID) error { +// archiveVariations archives each variation that LaunchDarkly still has. It +// reads each variation first, because the API rejects the archive of an +// archived variation. A variation that is already archived or gone does not +// fail, so the same command can run again after a later step fails. It stops +// before the next archive when ctx ends. +func archiveVariations(ctx context.Context, archiver Archiver, selected []syncdomain.ResourceID) error { for _, resource := range selected { + if err := ctx.Err(); err != nil { + return err + } configKey, variationKey, err := resource.VariationKeys() if err != nil { return err } + // ReadVariation treats an archived variation as absent. + state, err := archiver.ReadVariation(resource.ProjectKey, configKey, variationKey) + switch { + case syncapi.IsNotFound(err) || err == nil && !state.Exists: + continue + case err != nil: + return fmt.Errorf("read variation %s: %w", resource, err) + } if err := archiver.ArchiveVariation(resource.ProjectKey, configKey, variationKey); err != nil && !syncapi.IsNotFound(err) { return fmt.Errorf("archive variation %s: %w", resource, err) } diff --git a/internal/sync/detach/detach_test.go b/internal/sync/detach/detach_test.go index 50ed89b91..c5e261859 100644 --- a/internal/sync/detach/detach_test.go +++ b/internal/sync/detach/detach_test.go @@ -2,9 +2,12 @@ package detach import ( "bytes" + "context" "errors" + "io" "os" "path/filepath" + "slices" "strings" "testing" @@ -12,6 +15,7 @@ import ( "github.com/stretchr/testify/require" 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" @@ -262,43 +266,30 @@ func TestRunArchive(t *testing.T) { tests := map[string]struct { archive bool yes bool - archiveErr error + archiver recordingArchiver wantError string wantArchived []string wantDetached bool }{ "detach without archive keeps the variation in LaunchDarkly": {wantDetached: true}, "archive with yes": {archive: true, yes: true, wantArchived: []string{"project/config/prompt"}, wantDetached: true}, - "variation already archived": {archive: true, yes: true, archiveErr: notFound, wantArchived: []string{"project/config/prompt"}, wantDetached: true}, + "variation already archived": {archive: true, yes: true, archiver: recordingArchiver{preArchived: true}, wantDetached: true}, + "config gone in LaunchDarkly": {archive: true, yes: true, archiver: recordingArchiver{readErr: notFound}, wantDetached: true}, "archive needs a confirmation": {archive: true, wantError: "rerun with --yes"}, "archive failure stops detach": { - archive: true, yes: true, archiveErr: errors.New("forbidden"), - wantError: "archive variation project/config/prompt", wantArchived: []string{"project/config/prompt"}, + archive: true, yes: true, archiver: recordingArchiver{err: errors.New("forbidden")}, + wantError: "archive variation project/config/prompt", }, } for name, test := range tests { t.Run(name, func(t *testing.T) { - root := t.TempDir() - store := synclocal.NewStore(root) - _, err := store.Add([]synclocal.VariationFile{{ - ProjectKey: "project", ConfigKey: "config", Variation: testVariation("prompt"), - }}) - require.NoError(t, err) - archiver := &recordingArchiver{err: test.archiveErr} - - err = Run(Options{ - RepositoryRoot: root, - Store: store, - Baselines: newMemoryManifestStore(), - Input: bytes.NewBuffer(nil), - Output: bytes.NewBuffer(nil), - Selections: []syncdomain.ResourceID{syncdomain.VariationID("project", "config", "prompt")}, - NoInput: true, - Archive: test.archive, - Archiver: archiver, - Yes: test.yes, - }) + workspace := newArchiveWorkspace(t, "prompt") + archiver := &test.archiver + options := workspace.options(archiver) + options.Archive, options.Yes = test.archive, test.yes + + err := Run(options) if test.wantError != "" { require.ErrorContains(t, err, test.wantError) @@ -306,26 +297,207 @@ func TestRunArchive(t *testing.T) { require.NoError(t, err) } assert.Equal(t, test.wantArchived, archiver.archived) - exists, err := store.VariationExists("project", "config", "prompt") - require.NoError(t, err) - assert.Equal(t, !test.wantDetached, exists) + workspace.requireTracked(t, "prompt", !test.wantDetached) }) } } +// A failure on the second variation stops the detach of both. The next run +// skips the variation that is already archived and finishes. +func TestRunArchiveWithSeveralSelectionsCanRunAgainAfterAFailure(t *testing.T) { + workspace := newArchiveWorkspace(t, "first", "second") + archiver := &recordingArchiver{errFor: map[string]error{"project/config/second": errors.New("forbidden")}} + options := workspace.options(archiver) + options.Archive, options.Yes = true, true + + require.ErrorContains(t, Run(options), "archive variation project/config/second: forbidden") + assert.Equal(t, []string{"project/config/first"}, archiver.archived) + workspace.requireTracked(t, "first", true) + workspace.requireTracked(t, "second", true) + + archiver.errFor = nil + require.NoError(t, Run(options)) + assert.Equal(t, []string{"project/config/first", "project/config/second"}, archiver.archived) + workspace.requireTracked(t, "first", false) + workspace.requireTracked(t, "second", false) +} + +// The file delete runs after the archive and the baseline save. When it +// fails, detach restores the baseline, and the next run finishes. +func TestRunArchiveCanRunAgainAfterTheDeleteFails(t *testing.T) { + workspace := newArchiveWorkspace(t, "prompt") + archiver := &recordingArchiver{} + options := workspace.options(archiver) + options.Archive, options.Yes = true, true + + // A directory in place of the variation file makes the delete fail. + variationFile := filepath.Join(workspace.root, syncdomain.RootDir, "project", "configs", "config", "prompt.prompt.md") + content, err := os.ReadFile(variationFile) + require.NoError(t, err) + require.NoError(t, os.Remove(variationFile)) + require.NoError(t, os.Mkdir(variationFile, 0o755)) + + require.ErrorContains(t, Run(options), "is not a regular file") + assert.Equal(t, []string{"project/config/prompt"}, archiver.archived) + assert.NotEmpty(t, workspace.baselines.manifest.Resources) + + require.NoError(t, os.Remove(variationFile)) + require.NoError(t, os.WriteFile(variationFile, content, 0o644)) + require.NoError(t, Run(options)) + assert.Equal(t, []string{"project/config/prompt"}, archiver.archived) + workspace.requireTracked(t, "prompt", false) +} + +func TestRunArchiveAsksForConfirmation(t *testing.T) { + tests := map[string]struct { + answer string + wantArchived []string + wantOutput string + }{ + "yes archives": {answer: "y\n", wantArchived: []string{"project/config/prompt"}, wantOutput: "Detached and archived resources:"}, + "no changes nothing": {answer: "n\n", wantOutput: "Detach canceled."}, + "empty answer is a no": {answer: "\n", wantOutput: "Detach canceled."}, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + workspace := newArchiveWorkspace(t, "prompt") + archiver := &recordingArchiver{} + var output bytes.Buffer + options := workspace.options(archiver) + options.Archive, options.NoInput = true, false + options.Input, options.Output = strings.NewReader(test.answer), &output + options.isTerminal = func(io.Reader, io.Writer) bool { return true } + + require.NoError(t, Run(options)) + + assert.Contains(t, output.String(), "Archive these variations in LaunchDarkly? [y/N]") + assert.Contains(t, output.String(), test.wantOutput) + assert.Equal(t, test.wantArchived, archiver.archived) + workspace.requireTracked(t, "prompt", test.wantArchived == nil) + }) + } +} + +// archiveWorkspace is a workspace whose variations have local files and +// baseline entries. +type archiveWorkspace struct { + root string + store synclocal.Store + baselines *memoryManifestStore + keys []string +} + +func newArchiveWorkspace(t *testing.T, keys ...string) archiveWorkspace { + t.Helper() + workspace := archiveWorkspace{root: t.TempDir(), baselines: newMemoryManifestStore(), keys: keys} + workspace.store = synclocal.NewStore(workspace.root) + for _, key := range keys { + _, err := workspace.store.Add([]synclocal.VariationFile{{ + ProjectKey: "project", ConfigKey: "config", Variation: testVariation(key), + }}) + require.NoError(t, err) + workspace.baselines.manifest.SetFingerprint(syncdomain.VariationID("project", "config", key), testFingerprint()) + } + return workspace +} + +func (workspace archiveWorkspace) options(archiver Archiver) Options { + selections := make([]syncdomain.ResourceID, 0, len(workspace.keys)) + for _, key := range workspace.keys { + selections = append(selections, syncdomain.VariationID("project", "config", key)) + } + return Options{ + RepositoryRoot: workspace.root, + Store: workspace.store, + Baselines: workspace.baselines, + Input: bytes.NewBuffer(nil), + Output: bytes.NewBuffer(nil), + Selections: selections, + NoInput: true, + Archiver: archiver, + } +} + +// requireTracked checks both the local file and the baseline entry. +func (workspace archiveWorkspace) requireTracked(t *testing.T, key string, tracked bool) { + t.Helper() + exists, err := workspace.store.VariationExists("project", "config", key) + require.NoError(t, err) + assert.Equal(t, tracked, exists, "local file of %s", key) + id := syncdomain.VariationID("project", "config", key) + inBaseline := slices.ContainsFunc(workspace.baselines.manifest.Resources, func(resource syncmanifest.Resource) bool { + return resource.ID() == id + }) + assert.Equal(t, tracked, inBaseline, "baseline entry of %s", key) +} + +// The archive runs before the baseline save. When the save fails, the same +// command must finish on its next run, although the variation is archived. +func TestRunArchiveCanRunAgainAfterALaterStepFails(t *testing.T) { + workspace := newArchiveWorkspace(t, "prompt") + archiver := &recordingArchiver{} + options := workspace.options(archiver) + options.Archive, options.Yes = true, true + + workspace.baselines.saveErr = errors.New("save baseline") + require.ErrorContains(t, Run(options), "save baseline") + assert.Equal(t, []string{"project/config/prompt"}, archiver.archived) + workspace.requireTracked(t, "prompt", true) + + workspace.baselines.saveErr = nil + require.NoError(t, Run(options)) + assert.Equal(t, []string{"project/config/prompt"}, archiver.archived) + workspace.requireTracked(t, "prompt", false) +} + +func TestRunArchiveStopsWhenTheContextEnds(t *testing.T) { + workspace := newArchiveWorkspace(t, "prompt") + archiver := &recordingArchiver{} + ctx, cancel := context.WithCancel(context.Background()) + cancel() + options := workspace.options(archiver) + options.Archive, options.Yes, options.Context = true, true, ctx + + require.ErrorIs(t, Run(options), context.Canceled) + assert.Empty(t, archiver.archived) + workspace.requireTracked(t, "prompt", true) +} + +// recordingArchiver behaves like the API: an archived variation reads as +// absent, and a second archive of it fails. type recordingArchiver struct { - archived []string - err error + archived []string + err error + errFor map[string]error + readErr error + preArchived bool +} + +func (archiver *recordingArchiver) ReadVariation(projectKey, configKey, variationKey string) (syncapi.VariationState, error) { + exists := !archiver.preArchived && !slices.Contains(archiver.archived, projectKey+"/"+configKey+"/"+variationKey) + return syncapi.VariationState{Exists: exists}, archiver.readErr } func (archiver *recordingArchiver) ArchiveVariation(projectKey, configKey, variationKey string) error { - archiver.archived = append(archiver.archived, projectKey+"/"+configKey+"/"+variationKey) - return archiver.err + id := projectKey + "/" + configKey + "/" + variationKey + if archiver.preArchived || slices.Contains(archiver.archived, id) { + return errors.New(`{"code":"invalid_request","message":"cannot archive an archived variation","statusCode":400}`) + } + if archiver.err != nil { + return archiver.err + } + if err := archiver.errFor[id]; err != nil { + return err + } + archiver.archived = append(archiver.archived, id) + return nil } type memoryManifestStore struct { manifest syncmanifest.Manifest loadedProjectKeys []string + saveErr error } func newMemoryManifestStore() *memoryManifestStore { @@ -338,6 +510,9 @@ func (store *memoryManifestStore) Load(projectKeys []string) (syncmanifest.Basel } func (store *memoryManifestStore) Save(_ syncmanifest.Baseline, next syncmanifest.Manifest) (syncmanifest.Baseline, error) { + if store.saveErr != nil { + return syncmanifest.Baseline{}, store.saveErr + } store.manifest = next return syncmanifest.Baseline{Lock: next}, nil } diff --git a/internal/sync/local/compile.go b/internal/sync/local/compile.go index da285b96c..1c16f6fa6 100644 --- a/internal/sync/local/compile.go +++ b/internal/sync/local/compile.go @@ -41,7 +41,7 @@ func CompileWorkspace(repositoryRoot string) ([]syncdomain.SyncedResource, error return nil, fmt.Errorf("resolve repository root: %w", err) } return compile(workspaceFS{root: root}, func(reference Reference) ([]byte, error) { - return readWorkspaceReference(root, reference) + return ReadReference(root, reference) }) } diff --git a/internal/sync/local/reference.go b/internal/sync/local/reference.go index 4fbb35372..1ca8adeea 100644 --- a/internal/sync/local/reference.go +++ b/internal/sync/local/reference.go @@ -14,10 +14,7 @@ import ( // Reference is the "ref" field of a linked variation. It names a file in the // repository and the format of that file. -type Reference struct { - File string `yaml:"file"` - Format string `yaml:"format"` -} +type Reference = syncdomain.Reference // NewReference converts a path that the user entered to a reference. The // path can be relative to workingDirectory. The file must be in the repository. @@ -108,7 +105,9 @@ func readReferenceFromFS(fsys fs.FS, reference Reference) ([]byte, error) { return data, nil } -func readWorkspaceReference(repositoryRoot string, reference Reference) ([]byte, error) { +// ReadReference reads a linked file. It resolves symbolic links and makes sure +// that the file is a regular file in the repository. +func ReadReference(repositoryRoot string, reference Reference) ([]byte, error) { target, err := resolveReferencePath(repositoryRoot, reference) if err != nil { return nil, err diff --git a/internal/sync/local/replace.go b/internal/sync/local/replace.go index 0a08f2c59..d8aa4ed31 100644 --- a/internal/sync/local/replace.go +++ b/internal/sync/local/replace.go @@ -149,7 +149,8 @@ func (store Store) prepareReplacements(replacements []VariationReplacement) ([]s } // missingVariation describes a variation file that does not exist yet. A new -// file has upsert, so that a later sync can create the variation again. +// file has upsert, so that a later sync can create the variation again, and +// it keeps the link of the variation that it restores. func missingVariation(replacement VariationReplacement) (existingVariation, error) { relativePath, err := variationPath(replacement.ProjectKey, replacement.ConfigKey, replacement.Variation.Key) if err != nil { @@ -158,7 +159,7 @@ func missingVariation(replacement VariationReplacement) (existingVariation, erro return existingVariation{ relativePath: relativePath, mode: 0o644, - frontMatter: variationFrontMatter{Upsert: true}, + frontMatter: variationFrontMatter{Upsert: true, Ref: replacement.Ref}, }, nil } diff --git a/internal/sync/local/store.go b/internal/sync/local/store.go index 96ab51719..8b5182172 100644 --- a/internal/sync/local/store.go +++ b/internal/sync/local/store.go @@ -27,11 +27,13 @@ type VariationFile struct { } // VariationReplacement is new content for a variation file. If -// CreateIfMissing is true and the file does not exist, the replace creates it. +// CreateIfMissing is true and the file does not exist, the replace creates it +// with Ref as its link. An existing file keeps its own link. type VariationReplacement struct { ProjectKey string ConfigKey string CreateIfMissing bool + Ref *Reference Variation syncdomain.Variation } diff --git a/internal/sync/local/variation.go b/internal/sync/local/variation.go index d959be2bb..d31b9ea22 100644 --- a/internal/sync/local/variation.go +++ b/internal/sync/local/variation.go @@ -89,6 +89,7 @@ func parseVariation(file localFile) (syncdomain.SyncedResource, error) { ProjectKey: file.ProjectKey, LookupKey: configKey + "/" + variation.Key, Upsert: meta.Upsert, + Ref: meta.Ref, Variation: variation, }, nil } diff --git a/internal/sync/manifest/baseline.go b/internal/sync/manifest/baseline.go index c0343833a..f2b2d9c57 100644 --- a/internal/sync/manifest/baseline.go +++ b/internal/sync/manifest/baseline.go @@ -80,15 +80,15 @@ func (store BaselineStore) Load(projectKeys []string) (Baseline, error) { } // Save records next as the new baseline. It sends the entries that changed -// from current.Lock to next to the remote manifest, and then writes next to -// the sync.lock file with the versions that LaunchDarkly returned. If the -// remote save fails, the lock file does not change. +// from current.Lock to next to the remote manifest, with the versions that +// sync read, and then writes next to the sync.lock file. If the remote save +// fails, the lock file does not change. func (store BaselineStore) Save(current Baseline, next Manifest) (Baseline, error) { remote, err := store.remote.Update(current.remote, current.remote.WithChanges(current.Lock, next)) if err != nil { return Baseline{}, err } - saved := Baseline{Lock: next.WithVersionsFrom(remote), remote: remote, lockFileExists: true} + saved := Baseline{Lock: next.Clone(), remote: remote, lockFileExists: true} saved.Lock.Sort() if err := writeLock(store.lock, saved.Lock); err != nil { return Baseline{}, err diff --git a/internal/sync/manifest/baseline_test.go b/internal/sync/manifest/baseline_test.go index c84b5199f..0dfb4f16f 100644 --- a/internal/sync/manifest/baseline_test.go +++ b/internal/sync/manifest/baseline_test.go @@ -27,12 +27,10 @@ resources: project: production key: search fingerprint: `+fingerprint("b")+` - version: 2 - kind: variation project: production key: support/default fingerprint: `+fingerprint("a")+` - version: 6 `, string(content)) decoded, err := decodeLock(content) @@ -100,7 +98,7 @@ func TestBaselineStaleComparesFingerprints(t *testing.T) { // The version in each request must be the version that sync read from // LaunchDarkly, so that LaunchDarkly rejects a save that races another save. -func TestBaselineStoreSaveSendsRemoteVersionsAndWritesLock(t *testing.T) { +func TestBaselineStoreSaveSendsRemoteVersionsAndWritesLockWithoutThem(t *testing.T) { client := &manifestClient{ manifests: map[string]syncapi.SyncManifest{"production": remoteManifest(fingerprint("b"), 5)}, patchResponses: []syncapi.SyncManifest{{ @@ -130,7 +128,7 @@ func TestBaselineStoreSaveSendsRemoteVersionsAndWritesLock(t *testing.T) { written, _, err := ReadLock(lock) require.NoError(t, err) assert.Equal(t, saved.Lock, written) - assert.Equal(t, 6, written.Resources[written.index(variationID())].Version) + assert.NotContains(t, string(lock.content), "version:") assert.False(t, saved.Stale(variationID())) } @@ -220,3 +218,25 @@ func (lock *memoryLock) WriteLock(content []byte) error { lock.content = content return nil } + +func TestLockKeepsTheLinkOfAVariation(t *testing.T) { + manifest := Manifest{Resources: []Resource{{ + ResourceKind: syncdomain.KindVariation, ProjectKey: "production", LookupKey: "support/default", + Fingerprint: fingerprint("a"), Version: 1, + }}} + ref := &syncdomain.Reference{File: "prompts/support.md", Format: "plain-markdown"} + manifest.SetRefs([]syncdomain.SyncedResource{{ + Kind: syncdomain.KindVariation, ProjectKey: "production", LookupKey: "support/default", Ref: ref, + }}) + + content, err := encodeLock(manifest) + require.NoError(t, err) + assert.Contains(t, string(content), " ref:\n file: prompts/support.md\n format: plain-markdown\n") + decoded, err := decodeLock(content) + require.NoError(t, err) + assert.Equal(t, ref, decoded.Ref(variationID())) + + // A variation without a local file keeps its recorded link. + manifest.SetRefs(nil) + assert.Equal(t, ref, manifest.Ref(variationID())) +} diff --git a/internal/sync/manifest/model.go b/internal/sync/manifest/model.go index 92dd0ef67..4290f8963 100644 --- a/internal/sync/manifest/model.go +++ b/internal/sync/manifest/model.go @@ -19,15 +19,20 @@ type Manifest struct { Resources []Resource } -// Resource is the baseline of one resource. Version is the version of the -// remote manifest entry, which LaunchDarkly uses for optimistic locking. The -// YAML tags are the format of the sync.lock file. +// Resource is the baseline of one resource. The YAML tags are the format of +// the sync.lock file. type Resource struct { ResourceKind syncdomain.Kind `yaml:"kind"` ProjectKey string `yaml:"project"` LookupKey string `yaml:"key"` Fingerprint string `yaml:"fingerprint"` - Version int `yaml:"version"` + // 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. + Version int `yaml:"-"` + // Ref is the linked prompt file of a variation. Only sync.lock stores it, + // so that sync can restore a missing variation file with its link. + Ref *syncdomain.Reference `yaml:"ref,omitempty"` } // ID returns the identity of the resource. @@ -66,6 +71,24 @@ func (manifest *Manifest) Remove(id syncdomain.ResourceID) { } } +// SetRefs records the linked prompt file of each variation. A variation +// without a local file keeps its recorded link. +func (manifest *Manifest) SetRefs(variations []syncdomain.SyncedResource) { + for _, variation := range variations { + if index := manifest.index(variation.ID()); index >= 0 { + manifest.Resources[index].Ref = variation.Ref + } + } +} + +// Ref returns the recorded linked prompt file of a resource, or nil. +func (manifest Manifest) Ref(id syncdomain.ResourceID) *syncdomain.Reference { + if index := manifest.index(id); index >= 0 { + return manifest.Resources[index].Ref + } + return nil +} + // SetAttachments records the state of each tool and skill. Many variations // can share one attachment, but the manifest tracks it once. func (manifest *Manifest) SetAttachments(projectKey string, attachments []syncdomain.Attachment) error { diff --git a/internal/sync/prompt/acceptance_test.go b/internal/sync/prompt/acceptance_test.go index e14f91fb6..998d6080c 100644 --- a/internal/sync/prompt/acceptance_test.go +++ b/internal/sync/prompt/acceptance_test.go @@ -789,6 +789,129 @@ func TestPromptStaleWorkingCopyPullsInsteadOfReverting(t *testing.T) { assert.Equal(t, other.Name, resources[0].Variation.Name) } +func TestPromptRestoresLinkedVariationWithItsLink(t *testing.T) { + root := initRepository(t) + api := syncLinkedVariation(t, root) + deleteVariationFile(t, root) + + _, _, err := runPrompt(t, root, api, "--yes") + + require.NoError(t, err) + resources, err := synclocal.CompileWorkspace(root) + require.NoError(t, err) + require.Len(t, resources, 1) + require.NotNil(t, resources[0].Ref) + assert.Equal(t, "prompts/default.md", resources[0].Ref.File) + + // An edit in the linked file still reaches LaunchDarkly. + require.NoError(t, os.WriteFile(filepath.Join(root, "prompts", "default.md"), []byte("Edited prompt\n"), 0o644)) + _, _, err = runPrompt(t, root, api, "--yes") + require.NoError(t, err) + assert.Equal(t, "Edited prompt", api.variation.Instructions) +} + +func TestPromptStopsRestoreWhenLinkedFileIsNotSafe(t *testing.T) { + tests := map[string]struct { + change func(t *testing.T, linkedFile string) + wantError string + }{ + "linked file is missing": { + change: func(t *testing.T, linkedFile string) { require.NoError(t, os.Remove(linkedFile)) }, + wantError: `its linked file "prompts/default.md" is not available`, + }, + "linked file has local edits": { + change: func(t *testing.T, linkedFile string) { + require.NoError(t, os.WriteFile(linkedFile, []byte("Local edit\n"), 0o644)) + }, + wantError: `its linked file "prompts/default.md" differs from LaunchDarkly`, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + root := initRepository(t) + api := syncLinkedVariation(t, root) + deleteVariationFile(t, root) + test.change(t, filepath.Join(root, "prompts", "default.md")) + + _, _, err := runPrompt(t, root, api, "--yes") + + require.ErrorContains(t, err, test.wantError) + assert.Equal(t, "Linked prompt", api.variation.Instructions) + _, statErr := os.Stat(filepath.Join(root, syncdomain.RootDir, "production", "configs", "support", "default.prompt.md")) + assert.ErrorIs(t, statErr, os.ErrNotExist) + }) + } +} + +func TestPromptDetachArchive(t *testing.T) { + tests := map[string]struct { + arguments []string + wantError string + }{ + "archive with --yes": { + arguments: []string{"--archive", "--yes", "--no-input"}, + }, + "archive without --yes": { + arguments: []string{"--archive", "--no-input"}, + wantError: "rerun with --yes", + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + root := initRepository(t) + baseline := variation("Baseline") + writeVariation(t, root, baseline, false) + writeManifest(t, root, baseline) + api := &directAPI{variation: pointer(baseline)} + _, _, err := runPrompt(t, root, api, "--yes") + require.NoError(t, err) + + _, _, err = runPrompt(t, root, api, append([]string{"detach", "production/support/default"}, test.arguments...)...) + + variationFile := filepath.Join(root, syncdomain.RootDir, "production", "configs", "support", "default.prompt.md") + _, statErr := os.Stat(variationFile) + if test.wantError != "" { + require.ErrorContains(t, err, test.wantError) + assert.NotEqual(t, "archived", api.variationState) + assert.NoError(t, statErr) + assert.NotEmpty(t, manifestsByRoot[root].Items) + return + } + require.NoError(t, err) + assert.Equal(t, "archived", api.variationState) + assert.ErrorIs(t, statErr, os.ErrNotExist) + assert.Empty(t, manifestsByRoot[root].Items) + // The lock had only this variation, so detach removes the file. + _, statErr = os.Stat(filepath.Join(root, syncdomain.RootDir, "sync.lock")) + assert.ErrorIs(t, statErr, os.ErrNotExist) + }) + } +} + +// syncLinkedVariation writes a linked variation that matches LaunchDarkly and +// runs one sync, so that sync.lock records the link. +func syncLinkedVariation(t *testing.T, root string) *directAPI { + t.Helper() + linked := variation("Linked") + writeLinkedVariation(t, root, linked, "Linked prompt") + linked.Instructions = "Linked prompt" + writeManifest(t, root, linked) + api := &directAPI{variation: pointer(linked)} + _, _, err := runPrompt(t, root, api, "--yes") + require.NoError(t, err) + return api +} + +func deleteVariationFile(t *testing.T, root string) { + t.Helper() + _, err := synclocal.NewStore(root).DeleteVariations([]synclocal.VariationDeletion{{ + ProjectKey: "production", ConfigKey: "support", VariationKey: "default", + }}) + require.NoError(t, err) +} + func TestPromptRestoresTrackedMissingFileInsteadOfArchiving(t *testing.T) { root := initRepository(t) baseline := variation("Baseline") diff --git a/internal/sync/prompt/execute.go b/internal/sync/prompt/execute.go index a6e05a046..645fda533 100644 --- a/internal/sync/prompt/execute.go +++ b/internal/sync/prompt/execute.go @@ -76,6 +76,7 @@ func executePlan( failures = append(failures, err) } else { next.RemoveUnusedAttachments(variations) + next.SetRefs(variations) } } return outcomes, next, errors.Join(failures...) @@ -235,6 +236,7 @@ func applyLocalChange(store synclocal.Store, resource PlannedResource) error { ProjectKey: resource.ID.ProjectKey, ConfigKey: configKey, CreateIfMissing: resource.Local == nil, + Ref: resource.restoreRef, Variation: *resource.Server, }}) return err diff --git a/internal/sync/prompt/plan.go b/internal/sync/prompt/plan.go index 05ee2e978..5df33a296 100644 --- a/internal/sync/prompt/plan.go +++ b/internal/sync/prompt/plan.go @@ -68,6 +68,8 @@ type PlannedResource struct { // changedAttachments are the tools and skills whose content differs // between the local file and LaunchDarkly. changedAttachments []ResourceID + // restoreRef is the link that a restored variation file keeps. + restoreRef *syncdomain.Reference } // Plan is the sync decision for each resource, in identity order. diff --git a/internal/sync/prompt/plan_test.go b/internal/sync/prompt/plan_test.go index 2d31c9cb8..37fea8be9 100644 --- a/internal/sync/prompt/plan_test.go +++ b/internal/sync/prompt/plan_test.go @@ -3,6 +3,7 @@ package prompt import ( "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" syncdomain "github.com/launchdarkly/ldcli/internal/sync" @@ -178,3 +179,13 @@ func localResources(variation *syncdomain.Variation, upsert bool) []syncdomain.S Upsert: upsert, }} } + +func TestPlanRestoreStopsWithoutSyncLock(t *testing.T) { + server := testVariation("Server") + resource := PlannedResource{ID: testResourceID(), Action: ActionUpdateLocal, Server: &server} + + planRestore(t.TempDir(), syncmanifest.Baseline{}, &resource) + + assert.Equal(t, ActionError, resource.Action) + assert.Contains(t, resource.Error, "this working copy has no sync.lock") +} diff --git a/internal/sync/prompt/runner.go b/internal/sync/prompt/runner.go index 07524dca5..c83133b92 100644 --- a/internal/sync/prompt/runner.go +++ b/internal/sync/prompt/runner.go @@ -158,6 +158,7 @@ func (runner Runner) runDetach(options Options, workspace syncWorkspace, action Archive: action.Archive, Archiver: runner.api(options), Yes: options.Yes, + Context: options.Context, }) } diff --git a/internal/sync/prompt/state.go b/internal/sync/prompt/state.go index 083d09814..577fd08bd 100644 --- a/internal/sync/prompt/state.go +++ b/internal/sync/prompt/state.go @@ -2,12 +2,14 @@ package prompt import ( "errors" + "fmt" "slices" 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" ) // workspaceState is everything that one plan depends on. The plan compares @@ -81,10 +83,52 @@ func loadWorkspacePlan( plan := BuildPlan(baseline.Lock, local, server) for index := range plan.Resources { plan.Resources[index].SyncedElsewhere = baseline.Stale(plan.Resources[index].ID) + planRestore(repositoryRoot, baseline, &plan.Resources[index]) } return plan, localFilesByID, nil } +// planRestore checks a variation whose local file is missing. The restored +// file must keep the link that sync.lock records. When sync cannot restore +// the link safely, the resource becomes an error and sync writes nothing. +func planRestore(repositoryRoot string, baseline syncmanifest.Baseline, resource *PlannedResource) { + if resource.Action != ActionUpdateLocal || resource.Local != nil { + return + } + fail := func(problem, fix string) { + resource.Action = ActionError + resource.Error = "the variation file is missing, and " + problem + ". " + fix + } + if !baseline.HasLockFile() { + fail("this working copy has no sync.lock to show whether the file linked to a prompt file", + "Restore the file from Git, or stop syncing the variation with detach") + return + } + ref := baseline.Lock.Ref(resource.ID) + if ref == nil { + return + } + + content, err := synclocal.ReadReference(repositoryRoot, *ref) + if err != nil { + fail(fmt.Sprintf("its linked file %q is not available (%s)", ref.File, err), + "Restore the files from Git, or stop syncing the variation with detach") + return + } + // The restore writes LaunchDarkly's prompt to the linked file. Allow that + // only when the file already has that prompt, so that no local edit is lost. + linked := *resource.Server + if err := syncreference.ApplyToVariation(ref.Format, content, &linked); err == nil { + fingerprint, err := syncdomain.FingerprintVariation(resource.ID.ProjectKey, resource.ID.LookupKey, linked) + if err == nil && fingerprint == resource.ServerFingerprint { + resource.restoreRef = ref + return + } + } + fail(fmt.Sprintf("its linked file %q differs from LaunchDarkly", ref.File), + "Restore the variation file from Git, and then run sync again") +} + // readServerResource reads one variation from LaunchDarkly with the content // of its tools and skills. func readServerResource(client syncapi.Client, attachments *attachmentCache, id syncdomain.ResourceID) (ServerResource, error) { diff --git a/internal/sync/prompt/sync.go b/internal/sync/prompt/sync.go index 77e355352..614bbad37 100644 --- a/internal/sync/prompt/sync.go +++ b/internal/sync/prompt/sync.go @@ -2,6 +2,7 @@ package prompt import ( "errors" + "maps" "slices" syncdomain "github.com/launchdarkly/ldcli/internal/sync" @@ -62,7 +63,9 @@ func (runner Runner) runWorkspaceSync(options Options, workspace syncWorkspace, if !resolved.HasChanges() { // A workspace from before sync.lock gets the file on its first sync. if !reviewed.baseline.HasLockFile() { - if _, err := workspace.baselines.Save(reviewed.baseline, reviewed.baseline.Lock); err != nil { + lock := reviewed.baseline.Lock.Clone() + lock.SetRefs(slices.Collect(maps.Values(reviewed.localFiles))) + if _, err := workspace.baselines.Save(reviewed.baseline, lock); err != nil { return err } } diff --git a/internal/sync/resource.go b/internal/sync/resource.go index 5110133f1..96bd8dba4 100644 --- a/internal/sync/resource.go +++ b/internal/sync/resource.go @@ -92,13 +92,22 @@ func ValidateKey(key string) error { } } +// Reference links a variation to a prompt file elsewhere in the repository. +// Format names the adapter that reads and writes the file. +type Reference struct { + File string `yaml:"file"` + Format string `yaml:"format"` +} + // SyncedResource is one variation compiled from the local workspace. type SyncedResource struct { Kind Kind ProjectKey string LookupKey string // Upsert lets sync create the variation in LaunchDarkly when it is absent. - Upsert bool + Upsert bool + // Ref is the linked prompt file, or nil when the variation file holds the prompt. + Ref *Reference Variation Variation } From 3fc2ea9a77fac05cc420edbf56302a617e6a74c5 Mon Sep 17 00:00:00 2001 From: Clifford Tawiah Date: Fri, 9 Oct 2026 15:25:31 -0400 Subject: [PATCH 3/5] feat(sync): rename the sync prompt command to sync prompts Every other ldcli resource command uses a plural noun, such as flags, projects, and segments. The command also syncs every prompt variation in the workspace, not one. "ldcli sync prompts" now replaces "ldcli sync prompt", with no alias. --- cmd/sync/prompt.go | 36 ++++++++++++------------- cmd/sync/prompt_test.go | 8 +++--- cmd/sync/sync.go | 4 +-- internal/sync/prompt/acceptance_test.go | 2 +- internal/sync/prompt/runner.go | 2 +- 5 files changed, 26 insertions(+), 26 deletions(-) diff --git a/cmd/sync/prompt.go b/cmd/sync/prompt.go index e938f7b56..df449bea2 100644 --- a/cmd/sync/prompt.go +++ b/cmd/sync/prompt.go @@ -35,21 +35,21 @@ const ( // NewPromptCmd creates the prompt synchronization command. func NewPromptCmd(client resources.Client) *cobra.Command { cmd := &cobra.Command{ - Use: "prompt", + Use: "prompts", Short: "Synchronize local prompt variations with LaunchDarkly", Long: "Bootstrap local prompt variations from LaunchDarkly, add more variations, or synchronize changes using the LaunchDarkly manifest. " + "Sync rechecks state before every write, rerun sync after a change.", Example: ` # Preview synchronization changes - ldcli sync prompt --dry-run + ldcli sync prompts --dry-run # Add the first variation without prompting - ldcli sync prompt add production/support/default --no-input + ldcli sync prompts add production/support/default --no-input # Apply changes in an existing workspace without prompting - ldcli sync prompt --yes --no-input + ldcli sync prompts --yes --no-input # Prefer local changes when conflicts occur - ldcli sync prompt --yes --conflict=local`, + ldcli sync prompts --yes --conflict=local`, Args: func(cmd *cobra.Command, args []string) error { if err := cobra.NoArgs(cmd, args); err != nil { return err @@ -121,10 +121,10 @@ func newWatchCmd(client resources.Client) *cobra.Command { Use: "watch", Short: "Synchronize when managed files change", Example: ` # Watch files and confirm each set of changes - ldcli sync prompt watch + ldcli sync prompts watch # Apply watched changes without prompting and report unresolved conflicts - ldcli sync prompt watch --yes --no-input`, + ldcli sync prompts watch --yes --no-input`, Args: validatedArgs(cobra.NoArgs), RunE: runPrompt(client, func(*cobra.Command, []string) (syncprompt.CommandAction, error) { return syncprompt.SyncAction{Watch: true}, nil @@ -139,13 +139,13 @@ func newAddCmd(client resources.Client) *cobra.Command { Use: "add [project-key/config-key/variation-key...]", Short: "Add prompt variations from LaunchDarkly", Example: ` # Choose variations interactively - ldcli sync prompt add + ldcli sync prompts add # Add one variation by its full selector - ldcli sync prompt add production/support/default + ldcli sync prompts add production/support/default # Add multiple variations without prompting - ldcli sync prompt add production/support/default production/chat/concise --no-input`, + ldcli sync prompts add production/support/default production/chat/concise --no-input`, Args: validatedArgs(cobra.MinimumNArgs(0)), RunE: runPrompt(client, func(cmd *cobra.Command, args []string) (syncprompt.CommandAction, error) { variations, err := parseVariationSelectors(args) @@ -166,10 +166,10 @@ func newAttachCmd(client resources.Client) *cobra.Command { Use: "attach", Short: "Attach a tool or skill to a synced variation", Example: ` # Attach a tool - ldcli sync prompt attach tool search --to production/support/default --yes + ldcli sync prompts attach tool search --to production/support/default --yes # Attach a skill - ldcli sync prompt attach skill summarize --to production/support/default --yes`, + ldcli sync prompts attach skill summarize --to production/support/default --yes`, } cmd.AddCommand( newAttachKindCmd(client, syncdomain.AttachmentTool), @@ -183,10 +183,10 @@ func newAttachKindCmd(client resources.Client, kind syncdomain.AttachmentKind) * Use: string(kind) + " [key]", Short: "Attach a " + string(kind) + " to a synced variation", Example: fmt.Sprintf(` # Choose a %s and synced variation interactively - ldcli sync prompt attach %s + ldcli sync prompts attach %s # Attach a %s by key to a synced variation - ldcli sync prompt attach %s example-key \ + ldcli sync prompts attach %s example-key \ --to production/support/default \ --yes --no-input`, kind, kind, kind, kind), Args: validatedArgs(cobra.MaximumNArgs(1)), @@ -212,13 +212,13 @@ func newDetachCmd(client resources.Client) *cobra.Command { Use: "detach [project-key/config-key/variation-key...]", Short: "Stop syncing local prompt variations", Example: ` # Choose variations interactively - ldcli sync prompt detach + ldcli sync prompts detach # Stop syncing one variation - ldcli sync prompt detach production/support/default + ldcli sync prompts detach production/support/default # Stop syncing one variation and archive it in LaunchDarkly - ldcli sync prompt detach production/support/default --archive --yes --no-input`, + ldcli sync prompts detach production/support/default --archive --yes --no-input`, Args: validatedArgs(cobra.MinimumNArgs(0)), RunE: runPrompt(client, func(cmd *cobra.Command, args []string) (syncprompt.CommandAction, error) { variations, err := parseVariationSelectors(args) @@ -240,7 +240,7 @@ func newLinkCmd(client resources.Client) *cobra.Command { Use: "link ", Short: "Link an external prompt file", Example: ` # Link a Markdown file as a new variation - ldcli sync prompt link prompts/support.md \ + ldcli sync prompts link prompts/support.md \ --format plain-markdown \ --to production/support/default \ --model-config-key claude \ diff --git a/cmd/sync/prompt_test.go b/cmd/sync/prompt_test.go index 97fc5ce1d..702961d18 100644 --- a/cmd/sync/prompt_test.go +++ b/cmd/sync/prompt_test.go @@ -13,7 +13,7 @@ import ( func TestPromptCommandUsesActionSubcommands(t *testing.T) { command := NewPromptCmd(nil) - assert.Equal(t, "prompt", command.Use) + assert.Equal(t, "prompts", command.Use) for _, name := range []string{"watch", "add", "attach", "detach", "link"} { child, _, err := command.Find([]string{name}) require.NoError(t, err) @@ -34,13 +34,13 @@ func TestPromptCommandUsesActionSubcommands(t *testing.T) { func TestSyncCommandIncludesExamples(t *testing.T) { command := NewSyncCmd(nil, nil) - assert.Contains(t, command.Example, "ldcli sync prompt") + assert.Contains(t, command.Example, "ldcli sync prompts") } func TestPromptCommandHelpIncludesExamples(t *testing.T) { command := NewPromptCmd(nil) tests := map[string][]string{ - "prompt": {}, + "prompts": {}, "watch": {"watch"}, "add": {"add"}, "attach": {"attach"}, @@ -59,7 +59,7 @@ func TestPromptCommandHelpIncludesExamples(t *testing.T) { require.NoError(t, err) } - assert.Contains(t, target.Example, "ldcli sync prompt") + assert.Contains(t, target.Example, "ldcli sync prompts") }) } } diff --git a/cmd/sync/sync.go b/cmd/sync/sync.go index 14c4a9d85..0fd24677b 100644 --- a/cmd/sync/sync.go +++ b/cmd/sync/sync.go @@ -17,10 +17,10 @@ func NewSyncCmd(client resources.Client, analyticsTrackerFn analytics.TrackerFn) Use: "sync", Short: "Synchronize local resources with LaunchDarkly", Example: ` # Synchronize prompt variations - ldcli sync prompt + ldcli sync prompts # Preview prompt synchronization changes - ldcli sync prompt --dry-run`, + ldcli sync prompts --dry-run`, Args: cobra.MinimumNArgs(1), PersistentPreRun: func(cmd *cobra.Command, _ []string) { tracker := analyticsTrackerFn( diff --git a/internal/sync/prompt/acceptance_test.go b/internal/sync/prompt/acceptance_test.go index 998d6080c..5e305954d 100644 --- a/internal/sync/prompt/acceptance_test.go +++ b/internal/sync/prompt/acceptance_test.go @@ -1157,7 +1157,7 @@ func runPrompt(t *testing.T, root string, client resources.Client, arguments ... api.manifest = manifest } args := []string{ - "sync", "prompt", + "sync", "prompts", "--access-token", "token", "--base-uri", "https://example.test", } diff --git a/internal/sync/prompt/runner.go b/internal/sync/prompt/runner.go index c83133b92..07cc53cab 100644 --- a/internal/sync/prompt/runner.go +++ b/internal/sync/prompt/runner.go @@ -1,4 +1,4 @@ -// Package prompt runs the "ldcli sync prompt" commands. A sync compares the +// Package prompt runs the "ldcli sync prompts" commands. A sync compares the // local files and LaunchDarkly with the manifest, shows the plan, and applies // it after the user agrees. package prompt From 8dd652407173537a992260f58500fbb5d0b37741 Mon Sep 17 00:00:00 2001 From: Clifford Tawiah Date: Fri, 9 Oct 2026 15:58:08 -0400 Subject: [PATCH 4/5] fix(sync): track nothing when a working copy has no sync.lock The remote manifest is shared by every branch. A working copy without sync.lock used it as its baseline, so variations from other branches looked tracked and missing. Sync then restored or archived them. Now a missing sync.lock gives an empty baseline. The first sync adopts the local files and writes the lock. This removes the HasLockFile flag, the lock write on a sync without changes, and the restore error for a working copy without sync.lock. --- internal/sync/manifest/baseline.go | 27 +++++-------- internal/sync/manifest/baseline_test.go | 31 +++++++++++---- internal/sync/manifest/lock.go | 17 ++++----- internal/sync/prompt/acceptance_test.go | 50 +++++++++++++++++++++++-- internal/sync/prompt/plan_test.go | 11 ------ internal/sync/prompt/state.go | 7 +--- internal/sync/prompt/sync.go | 9 ----- 7 files changed, 87 insertions(+), 65 deletions(-) diff --git a/internal/sync/manifest/baseline.go b/internal/sync/manifest/baseline.go index f2b2d9c57..1b8002843 100644 --- a/internal/sync/manifest/baseline.go +++ b/internal/sync/manifest/baseline.go @@ -14,15 +14,8 @@ import ( // repository. It supplies the version of each entry, which LaunchDarkly uses // to reject two saves of one entry at the same time. type Baseline struct { - Lock Manifest - remote Manifest - lockFileExists bool -} - -// HasLockFile reports whether the working copy has a sync.lock file. A -// working copy without one uses the remote manifest as its lock. -func (baseline Baseline) HasLockFile() bool { - return baseline.lockFileExists + Lock Manifest + remote Manifest } // Stale reports whether another working copy synced a different state of the @@ -62,10 +55,11 @@ func NewBaselineStore(remote Store, lock LockFile) BaselineStore { } // Load reads the sync.lock file and the remote manifest of each project that -// projectKeys or the lock names. A working copy without a sync.lock file uses -// the remote manifest as its lock. Its next save then writes the file. +// projectKeys or the lock names. A working copy without a sync.lock file +// tracks nothing yet. The remote manifest is not its lock, because other +// branches add entries to it that this working copy never had. func (store BaselineStore) Load(projectKeys []string) (Baseline, error) { - lock, hasLock, err := ReadLock(store.lock) + lock, err := ReadLock(store.lock) if err != nil { return Baseline{}, err } @@ -73,10 +67,7 @@ func (store BaselineStore) Load(projectKeys []string) (Baseline, error) { if err != nil { return Baseline{}, err } - if !hasLock { - lock = remote.Clone() - } - return Baseline{Lock: lock, remote: remote, lockFileExists: hasLock}, nil + return Baseline{Lock: lock, remote: remote}, nil } // Save records next as the new baseline. It sends the entries that changed @@ -88,9 +79,9 @@ func (store BaselineStore) Save(current Baseline, next Manifest) (Baseline, erro if err != nil { return Baseline{}, err } - saved := Baseline{Lock: next.Clone(), remote: remote, lockFileExists: true} + saved := Baseline{Lock: next.Clone(), remote: remote} saved.Lock.Sort() - if err := writeLock(store.lock, saved.Lock); err != nil { + if err := WriteLock(store.lock, saved.Lock); err != nil { return Baseline{}, err } return saved, nil diff --git a/internal/sync/manifest/baseline_test.go b/internal/sync/manifest/baseline_test.go index 0dfb4f16f..2acb590e7 100644 --- a/internal/sync/manifest/baseline_test.go +++ b/internal/sync/manifest/baseline_test.go @@ -49,17 +49,33 @@ func TestDecodeLockRejectsInvalidContent(t *testing.T) { } } -func TestBaselineStoreUsesRemoteManifestWithoutLockFile(t *testing.T) { - client := &manifestClient{manifests: map[string]syncapi.SyncManifest{"production": remoteManifest(fingerprint("a"), 3)}} +// 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) { + client := &manifestClient{ + manifests: map[string]syncapi.SyncManifest{"production": remoteManifest(fingerprint("a"), 3)}, + patchResponses: []syncapi.SyncManifest{remoteManifest(fingerprint("b"), 4)}, + } lock := &memoryLock{} + store := NewBaselineStore(NewStore(client, testSource), lock) - baseline, err := NewBaselineStore(NewStore(client, testSource), lock).Load([]string{"production"}) + baseline, err := store.Load([]string{"production"}) require.NoError(t, err) - assert.False(t, baseline.HasLockFile()) - require.Len(t, baseline.Lock.Resources, 1) - assert.Equal(t, fingerprint("a"), baseline.Lock.Resources[0].Fingerprint) + assert.Empty(t, baseline.Lock.Resources) assert.False(t, baseline.Stale(variationID())) + + // A new entry for a resource that the remote manifest has must send the + // remote version that sync read. + next := baseline.Lock.Clone() + next.SetFingerprint(variationID(), fingerprint("b")) + _, err = store.Save(baseline, next) + require.NoError(t, err) + require.Len(t, client.patches, 1) + assert.Equal(t, []syncapi.SyncManifestUpsert{ + {ResourceKind: syncdomain.KindVariation, ResourceLookupKey: "support/default", Fingerprint: fingerprint("b"), Version: 3}, + }, client.patches[0].upserts) + assert.NotNil(t, lock.content) } func TestBaselineStoreUsesLockFileAsBaselineAndReportsStaleness(t *testing.T) { @@ -69,7 +85,6 @@ func TestBaselineStoreUsesLockFileAsBaselineAndReportsStaleness(t *testing.T) { baseline, err := NewBaselineStore(NewStore(client, testSource), lock).Load(nil) require.NoError(t, err) - assert.True(t, baseline.HasLockFile()) assert.Equal(t, fingerprint("a"), baseline.Lock.Resources[0].Fingerprint) assert.True(t, baseline.Stale(variationID())) } @@ -125,7 +140,7 @@ func TestBaselineStoreSaveSendsRemoteVersionsAndWritesLockWithoutThem(t *testing {ResourceKind: syncdomain.KindTool, ResourceLookupKey: "search", Fingerprint: fingerprint("d"), Version: 0}, {ResourceKind: syncdomain.KindVariation, ResourceLookupKey: "support/default", Fingerprint: fingerprint("c"), Version: 5}, }, client.patches[0].upserts) - written, _, err := ReadLock(lock) + written, err := ReadLock(lock) require.NoError(t, err) assert.Equal(t, saved.Lock, written) assert.NotContains(t, string(lock.content), "version:") diff --git a/internal/sync/manifest/lock.go b/internal/sync/manifest/lock.go index 800671c12..ac90a4430 100644 --- a/internal/sync/manifest/lock.go +++ b/internal/sync/manifest/lock.go @@ -29,27 +29,26 @@ type lockDocument struct { Resources []Resource `yaml:"resources"` } -// ReadLock decodes the sync.lock file. The bool result is false when the file -// does not exist. -func ReadLock(lock LockFile) (Manifest, bool, error) { +// ReadLock decodes the sync.lock file. A missing file gives an empty manifest. +func ReadLock(lock LockFile) (Manifest, error) { content, err := lock.ReadLock() if errors.Is(err, fs.ErrNotExist) { - return New(), false, nil + return New(), nil } if err != nil { - return Manifest{}, false, fmt.Errorf("read sync.lock: %w", err) + return Manifest{}, fmt.Errorf("read sync.lock: %w", err) } manifest, err := decodeLock(content) if err != nil { - return Manifest{}, false, fmt.Errorf("read sync.lock: %w", err) + return Manifest{}, fmt.Errorf("read sync.lock: %w", err) } - return manifest, true, nil + return manifest, nil } -// writeLock encodes the manifest to the sync.lock file. A manifest without +// WriteLock encodes the manifest to the sync.lock file. A manifest without // entries removes the file, so that a workspace without resources has no // .launchdarkly directory. -func writeLock(lock LockFile, manifest Manifest) error { +func WriteLock(lock LockFile, manifest Manifest) error { content, err := encodeLock(manifest) if err != nil { return err diff --git a/internal/sync/prompt/acceptance_test.go b/internal/sync/prompt/acceptance_test.go index 5e305954d..70e4d3811 100644 --- a/internal/sync/prompt/acceptance_test.go +++ b/internal/sync/prompt/acceptance_test.go @@ -772,7 +772,7 @@ func TestPromptStaleWorkingCopyPullsInsteadOfReverting(t *testing.T) { // Another working copy syncs a change. The shared remote manifest moves. other := variation("Other working copy") api.variation = pointer(other) - writeManifest(t, root, other) + writeRemoteManifest(t, root, other) manifestsByRoot[root].Items[0].Version = 2 _, review, err := runPrompt(t, root, api, "--yes") @@ -897,7 +897,6 @@ func syncLinkedVariation(t *testing.T, root string) *directAPI { linked := variation("Linked") writeLinkedVariation(t, root, linked, "Linked prompt") linked.Instructions = "Linked prompt" - writeManifest(t, root, linked) api := &directAPI{variation: pointer(linked)} _, _, err := runPrompt(t, root, api, "--yes") require.NoError(t, err) @@ -916,10 +915,9 @@ func TestPromptRestoresTrackedMissingFileInsteadOfArchiving(t *testing.T) { root := initRepository(t) baseline := variation("Baseline") writeVariation(t, root, baseline, false) - writeManifest(t, root, baseline) api := &directAPI{variation: pointer(baseline)} - // The first sync of a workspace from before sync.lock writes the lock. + // The first sync adopts the file that matches LaunchDarkly and writes the lock. _, _, err := runPrompt(t, root, api, "--yes") require.NoError(t, err) _, err = os.Stat(filepath.Join(root, syncdomain.RootDir, "sync.lock")) @@ -941,6 +939,38 @@ func TestPromptRestoresTrackedMissingFileInsteadOfArchiving(t *testing.T) { assert.NotEmpty(t, manifestsByRoot[root].Items) } +// The remote manifest is shared by every branch. A working copy without a +// sync.lock file must not restore, archive, or record a variation that only +// another branch syncs. +func TestPromptWithoutSyncLockIgnoresVariationsOfOtherBranches(t *testing.T) { + root := initRepository(t) + baseline := variation("Baseline") + writeVariation(t, root, baseline, false) + writeRemoteManifest(t, root, baseline) + remote := manifestsByRoot[root] + remote.Items = append(remote.Items, syncapi.SyncManifestResource{ + ResourceKind: syncdomain.KindVariation, + ResourceLookupKey: "billing/default", + Fingerprint: remote.Items[0].Fingerprint, + Version: 1, + }) + api := &directAPI{variation: pointer(baseline)} + + _, _, err := runPrompt(t, root, api, "--yes") + + require.NoError(t, err) + assert.False(t, slices.ContainsFunc(api.requests, func(request string) bool { + return strings.Contains(request, "/billing") + }), "sync read a variation that this working copy never synced") + _, statErr := os.Stat(filepath.Join(root, syncdomain.RootDir, "production", "configs", "billing")) + assert.ErrorIs(t, statErr, os.ErrNotExist) + assert.Len(t, manifestsByRoot[root].Items, 2) + lock, err := os.ReadFile(filepath.Join(root, syncdomain.RootDir, "sync.lock")) + require.NoError(t, err) + assert.Contains(t, string(lock), "key: support/default") + assert.NotContains(t, string(lock), "billing") +} + func TestPromptPropagatesTrackedServerDeletion(t *testing.T) { root := initRepository(t) baseline := variation("Baseline") @@ -1304,7 +1334,18 @@ func writeManifest(t *testing.T, root string, value syncdomain.Variation) { writeManifestResources(t, root, value) } +// writeManifestResources records values as the baseline of a working copy +// that synced them. It writes the sync.lock file and the remote manifest. func writeManifestResources(t *testing.T, root string, values ...syncdomain.Variation) { + t.Helper() + manifest := writeRemoteManifest(t, root, values...) + require.NoError(t, os.MkdirAll(filepath.Join(root, syncdomain.RootDir), 0o755)) + require.NoError(t, syncmanifest.WriteLock(synclocal.NewStore(root), manifest)) +} + +// writeRemoteManifest writes only the remote manifest, as another working +// copy does when it syncs. +func writeRemoteManifest(t *testing.T, root string, values ...syncdomain.Variation) syncmanifest.Manifest { t.Helper() manifest := syncmanifest.New() for _, value := range values { @@ -1323,6 +1364,7 @@ func writeManifestResources(t *testing.T, root string, values ...syncdomain.Vari }) } manifestsByRoot[root] = remote + return manifest } func assertManifestFingerprint(t *testing.T, root string, value syncdomain.Variation) { diff --git a/internal/sync/prompt/plan_test.go b/internal/sync/prompt/plan_test.go index 37fea8be9..2d31c9cb8 100644 --- a/internal/sync/prompt/plan_test.go +++ b/internal/sync/prompt/plan_test.go @@ -3,7 +3,6 @@ package prompt import ( "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" syncdomain "github.com/launchdarkly/ldcli/internal/sync" @@ -179,13 +178,3 @@ func localResources(variation *syncdomain.Variation, upsert bool) []syncdomain.S Upsert: upsert, }} } - -func TestPlanRestoreStopsWithoutSyncLock(t *testing.T) { - server := testVariation("Server") - resource := PlannedResource{ID: testResourceID(), Action: ActionUpdateLocal, Server: &server} - - planRestore(t.TempDir(), syncmanifest.Baseline{}, &resource) - - assert.Equal(t, ActionError, resource.Action) - assert.Contains(t, resource.Error, "this working copy has no sync.lock") -} diff --git a/internal/sync/prompt/state.go b/internal/sync/prompt/state.go index 577fd08bd..35ac6480d 100644 --- a/internal/sync/prompt/state.go +++ b/internal/sync/prompt/state.go @@ -99,11 +99,6 @@ func planRestore(repositoryRoot string, baseline syncmanifest.Baseline, resource resource.Action = ActionError resource.Error = "the variation file is missing, and " + problem + ". " + fix } - if !baseline.HasLockFile() { - fail("this working copy has no sync.lock to show whether the file linked to a prompt file", - "Restore the file from Git, or stop syncing the variation with detach") - return - } ref := baseline.Lock.Ref(resource.ID) if ref == nil { return @@ -159,7 +154,7 @@ func (workspace syncWorkspace) projectKeys() ([]string, error) { if err != nil { return nil, err } - lock, _, err := syncmanifest.ReadLock(workspace.local) + lock, err := syncmanifest.ReadLock(workspace.local) if err != nil { return nil, err } diff --git a/internal/sync/prompt/sync.go b/internal/sync/prompt/sync.go index 614bbad37..1a31cb479 100644 --- a/internal/sync/prompt/sync.go +++ b/internal/sync/prompt/sync.go @@ -2,7 +2,6 @@ package prompt import ( "errors" - "maps" "slices" syncdomain "github.com/launchdarkly/ldcli/internal/sync" @@ -61,14 +60,6 @@ func (runner Runner) runWorkspaceSync(options Options, workspace syncWorkspace, } if !proceed { if !resolved.HasChanges() { - // A workspace from before sync.lock gets the file on its first sync. - if !reviewed.baseline.HasLockFile() { - lock := reviewed.baseline.Lock.Clone() - lock.SetRefs(slices.Collect(maps.Values(reviewed.localFiles))) - if _, err := workspace.baselines.Save(reviewed.baseline, lock); err != nil { - return err - } - } return cleanupOrphanedAttachments(options, workspace.local, interactive) } return nil From 3d1afe521fcb1f940122770e62058293dd86a98d Mon Sep 17 00:00:00 2001 From: Clifford Tawiah Date: Fri, 9 Oct 2026 18:34:20 -0400 Subject: [PATCH 5/5] fix(sync): keep the conflict diff visible while the user chooses The conflict choice opened in the terminal's alternate screen. That screen hid the diff that sync writes just before the question, so the user saw only the three choices. The conflict choice now shows below the current output, and the diff stays above it. The project, config, and model config pickers keep the full screen, because their lists can be long. --- internal/sync/interactive/form.go | 43 +++++++++++++++++++++---------- internal/sync/prompt/conflict.go | 2 +- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/internal/sync/interactive/form.go b/internal/sync/interactive/form.go index 2a4ec4c79..aad45a6e8 100644 --- a/internal/sync/interactive/form.go +++ b/internal/sync/interactive/form.go @@ -332,11 +332,23 @@ func (model *searchModel[T]) rebuildList(loadedMore bool) { // Select asks the user to choose one value. func Select[T any](input io.Reader, output io.Writer, title string, choices []Choice[T]) (T, bool, error) { - return SelectContext(context.Background(), input, output, title, choices) + return selectChoice(context.Background(), input, output, title, choices, true) } -// SelectContext asks the user to choose one value and stops when the context ends. -func SelectContext[T any](ctx context.Context, input io.Reader, output io.Writer, title string, choices []Choice[T]) (T, bool, error) { +// SelectInline asks the user to choose one value below the current output, +// so that the text above the question stays visible. It stops when ctx ends. +func SelectInline[T any](ctx context.Context, input io.Reader, output io.Writer, title string, choices []Choice[T]) (T, bool, error) { + return selectChoice(ctx, input, output, title, choices, false) +} + +func selectChoice[T any]( + ctx context.Context, + input io.Reader, + output io.Writer, + title string, + choices []Choice[T], + fullScreen bool, +) (T, bool, error) { var zero T if len(choices) == 0 { return zero, false, fmt.Errorf("%s: no choices are available", title) @@ -351,15 +363,13 @@ func SelectContext[T any](ctx context.Context, input io.Reader, output io.Writer field := huh.NewSelect[int](). Title(title). Options(options...). - Height(selectionHeight). Value(&selected) + // A select with a height pads to it. Inline, it fits its options instead. + if fullScreen { + field.Height(selectionHeight) + } - canceled, err := RunFormContext( - ctx, - input, - output, - field, - ) + canceled, err := runForm(ctx, input, output, fullScreen, field) if err != nil || canceled { return zero, canceled, err } @@ -404,13 +414,18 @@ func MultiSelect[T any](input io.Reader, output io.Writer, title, description st // RunForm runs fields with the shared sync theme and terminal streams. func RunForm(input io.Reader, output io.Writer, fields ...huh.Field) (bool, error) { - return RunFormContext(context.Background(), input, output, fields...) + return runForm(context.Background(), input, output, true, fields...) } -// RunFormContext runs fields until they complete, abort, or the context ends. -func RunFormContext(ctx context.Context, input io.Reader, output io.Writer, fields ...huh.Field) (bool, error) { +// runForm runs fields until they complete, abort, or ctx ends. A full screen +// form gives a long list the whole terminal, but it hides the output above it. +func runForm(ctx context.Context, input io.Reader, output io.Writer, fullScreen bool, fields ...huh.Field) (bool, error) { + programOptions := []tea.ProgramOption{tea.WithReportFocus()} + if fullScreen { + programOptions = append(programOptions, tea.WithAltScreen()) + } err := huh.NewForm(huh.NewGroup(fields...)). - WithProgramOptions(tea.WithAltScreen(), tea.WithReportFocus()). + WithProgramOptions(programOptions...). WithInput(input). WithOutput(output). WithAccessible(false). diff --git a/internal/sync/prompt/conflict.go b/internal/sync/prompt/conflict.go index d07ba8ce7..06beaae72 100644 --- a/internal/sync/prompt/conflict.go +++ b/internal/sync/prompt/conflict.go @@ -213,7 +213,7 @@ func sharesChangedAttachment(left, right PlannedResource) bool { // promptConflictResolution asks the user which side wins one conflict. func promptConflictResolution(ctx context.Context, input io.Reader, output io.Writer) (conflictChoice, error) { - resolution, canceled, err := syncinteractive.SelectContext( + resolution, canceled, err := syncinteractive.SelectInline( ctx, input, output,