From 720df6fb56c57ca8b4f4d6b3e514554ba9316b22 Mon Sep 17 00:00:00 2001 From: Cliff Tawiah Date: Thu, 22 Oct 2026 21:08:03 +0000 Subject: [PATCH] feat: route sync, add, and detach by scope The scope moves from SyncAction to Options, so that every action has it. A sync of prompts, tools, skills, or all kinds plans only its scope. An add or a detach takes resources of the kind of the scope, and the add of tools and skills uses the add from the bootstrap package. A sync of every kind in a repository without sync files fails with a message that names the three add commands. A sync of tools or skills starts its add, as a sync of prompts does. An empty scope names its add command. A sync of prompts notes the tool and skill files that no variation uses, because its plan leaves them out. The note counts paths and reads no file. After a sync of tools or skills pushes a tool or skill, a note names each local variation that repins on the next sync of prompts. Without a terminal, an unresolved conflict outside the prompts scope names only --conflict, because only sync prompts has --resolve. --- cmd/sync/prompt.go | 2 +- internal/sync/prompt/acceptance_test.go | 287 ++++++++++++++++++++++-- internal/sync/prompt/action.go | 64 +++--- internal/sync/prompt/conflict.go | 16 +- internal/sync/prompt/output.go | 52 ++++- internal/sync/prompt/output_test.go | 14 ++ internal/sync/prompt/plan.go | 9 + internal/sync/prompt/runner.go | 26 ++- internal/sync/prompt/state.go | 56 ++++- internal/sync/prompt/sync.go | 5 +- 10 files changed, 465 insertions(+), 66 deletions(-) diff --git a/cmd/sync/prompt.go b/cmd/sync/prompt.go index beb0806f..0ac39361 100644 --- a/cmd/sync/prompt.go +++ b/cmd/sync/prompt.go @@ -156,7 +156,7 @@ func newAddCmd(client resources.Client) *cobra.Command { return nil, err } dryRun, _ := cmd.Flags().GetBool(dryRunFlag) - return syncprompt.AddAction{Variations: variations, Filter: targetFilter(cmd), DryRun: dryRun}, nil + return syncprompt.AddAction{Resources: variations, Filter: targetFilter(cmd), DryRun: dryRun}, nil }), } addTargetFlags(cmd, questionFlagHelp) diff --git a/internal/sync/prompt/acceptance_test.go b/internal/sync/prompt/acceptance_test.go index 9a4bb516..f0b4dda9 100644 --- a/internal/sync/prompt/acceptance_test.go +++ b/internal/sync/prompt/acceptance_test.go @@ -5,7 +5,6 @@ import ( "encoding/json" "errors" "fmt" - "io" "net/http" "net/url" "os" @@ -1523,7 +1522,7 @@ func TestPromptPlansToolAndSkillRowsByScope(t *testing.T) { t.Run(string(scope), func(t *testing.T) { api.requests = nil - stdout, err := runSyncAction(t, root, api, syncprompt.SyncAction{DryRun: true, Scope: scope}, false) + stdout, err := runSyncAction(t, root, api, scope, syncprompt.SyncAction{DryRun: true}, false) require.NoError(t, err) assert.Equal(t, want, planRows(t, stdout)) @@ -1540,7 +1539,7 @@ func TestPromptFiltersToolRowsBeforeAnyRead(t *testing.T) { root, api := writeStandaloneAttachmentWorkspace(t) filter := syncdomain.Filter{Project: "production", Keys: []string{"lookup"}} - stdout, err := runSyncAction(t, root, api, syncprompt.SyncAction{DryRun: true, Scope: syncdomain.ScopeTools, Filter: filter}, false) + stdout, err := runSyncAction(t, root, api, syncdomain.ScopeTools, syncprompt.SyncAction{DryRun: true, Filter: filter}, false) require.NoError(t, err) assert.Equal(t, []string{"tool lookup update_manifest"}, planRows(t, stdout)) @@ -1564,7 +1563,7 @@ func TestPromptFiltersToolRowsBeforeAnyRead(t *testing.T) { }, } { api.requests = nil - _, err := runSyncAction(t, root, api, syncprompt.SyncAction{DryRun: true, Scope: syncdomain.ScopeTools, Filter: test.filter}, false) + _, err := runSyncAction(t, root, api, syncdomain.ScopeTools, syncprompt.SyncAction{DryRun: true, Filter: test.filter}, false) require.EqualError(t, err, test.want) assert.Empty(t, api.requests) } @@ -1573,11 +1572,11 @@ func TestPromptFiltersToolRowsBeforeAnyRead(t *testing.T) { func TestPromptSyncsStandaloneToolsAndSkills(t *testing.T) { root, api := writeStandaloneAttachmentWorkspace(t) writeLocalFile(t, root, "tools/search.json", `{"formatVersion": 1, "key": "search", "description": "Search", "schema": {"type": "object"}}`) - all := syncprompt.SyncAction{Scope: syncdomain.ScopeAll} + all := syncprompt.SyncAction{} // One plan pushes the tool once, creates the new tool, records the rest, // and then pins the new tool version in the variation. - stdout, err := runSyncAction(t, root, api, all, true) + stdout, err := runSyncAction(t, root, api, syncdomain.ScopeAll, all, true) require.NoError(t, err) assert.Contains(t, stdout, `"lookupKey": "search", "action": "update_server", @@ -1593,7 +1592,7 @@ func TestPromptSyncsStandaloneToolsAndSkills(t *testing.T) { assert.Len(t, readLock(t, root).Resources, 6, "the lock keeps every tool and skill, also the ones that no variation uses") api.requests = nil - stdout, err = runSyncAction(t, root, api, syncprompt.SyncAction{DryRun: true, Scope: syncdomain.ScopeAll}, false) + stdout, err = runSyncAction(t, root, api, syncdomain.ScopeAll, syncprompt.SyncAction{DryRun: true}, false) require.NoError(t, err) for _, row := range planRows(t, stdout) { assert.True(t, strings.HasSuffix(row, " in_sync"), row) @@ -1604,7 +1603,7 @@ func TestPromptSyncsStandaloneToolsAndSkills(t *testing.T) { lookup.Description = pointer("Look up an order") lookup.Version++ api.tools["lookup"] = lookup - _, err = runSyncAction(t, root, api, syncprompt.SyncAction{Scope: syncdomain.ScopeTools}, true) + _, err = runSyncAction(t, root, api, syncdomain.ScopeTools, syncprompt.SyncAction{}, true) require.NoError(t, err) content, err := os.ReadFile(filepath.Join(root, syncdomain.RootDir, "production", "tools", "lookup.json")) require.NoError(t, err) @@ -1612,7 +1611,7 @@ func TestPromptSyncsStandaloneToolsAndSkills(t *testing.T) { // A hand-written skill file with upsert creates the skill. writeLocalFile(t, root, "skills/handoff.md", "---\nkey: handoff\ndescription: Hand off\nname: Human handoff\nupsert: true\n---\n\n# Handoff\n") - _, err = runSyncAction(t, root, api, syncprompt.SyncAction{Scope: syncdomain.ScopeSkills}, true) + _, err = runSyncAction(t, root, api, syncdomain.ScopeSkills, syncprompt.SyncAction{}, true) require.NoError(t, err) assert.Equal(t, "Human handoff", api.skills["handoff"].Name) @@ -1621,13 +1620,13 @@ func TestPromptSyncsStandaloneToolsAndSkills(t *testing.T) { handoff := api.skills["handoff"] handoff.Name = "Renamed in LaunchDarkly" api.skills["handoff"] = handoff - stdout, err = runSyncAction(t, root, api, syncprompt.SyncAction{DryRun: true, Scope: syncdomain.ScopeSkills}, false) + stdout, err = runSyncAction(t, root, api, syncdomain.ScopeSkills, syncprompt.SyncAction{DryRun: true}, false) require.NoError(t, err) assert.Contains(t, planRows(t, stdout), "skill handoff in_sync", "a rename in LaunchDarkly is not a change") writeLocalFile(t, root, "skills/handoff.md", "---\nkey: handoff\ndescription: Hand off to a person\nname: Human handoff\nupsert: true\n---\n\n# Handoff\n") api.requests = nil - _, err = runSyncAction(t, root, api, syncprompt.SyncAction{Scope: syncdomain.ScopeSkills}, true) + _, err = runSyncAction(t, root, api, syncdomain.ScopeSkills, syncprompt.SyncAction{}, true) require.NoError(t, err) assert.Equal(t, 1, countRequests(api.requests, "PATCH", "/ai-configs/skills/handoff")) assert.Equal(t, "Hand off to a person", api.skills["handoff"].Description) @@ -1639,13 +1638,244 @@ func TestPromptSyncsStandaloneToolsAndSkills(t *testing.T) { // A deletion in LaunchDarkly deletes the standalone file and its baseline. delete(api.tools, "lookup") - _, err = runSyncAction(t, root, api, syncprompt.SyncAction{Scope: syncdomain.ScopeTools}, true) + _, err = runSyncAction(t, root, api, syncdomain.ScopeTools, syncprompt.SyncAction{}, true) require.NoError(t, err) _, err = os.Stat(filepath.Join(root, syncdomain.RootDir, "production", "tools", "lookup.json")) assert.ErrorIs(t, err, os.ErrNotExist) assert.NotContains(t, string(must(os.ReadFile(filepath.Join(root, syncdomain.RootDir, "sync.lock")))), "key: lookup") } +// A workspace with only a tool file, or only a skill file, syncs with its own +// scope and with the scope of every kind. A sync of prompts leaves it out and +// says so. +func TestScopesSyncAWorkspaceWithOnlyOneTool(t *testing.T) { + for _, test := range []struct { + scope syncdomain.Scope + file string + row string + note string + api *directAPI + }{ + { + scope: syncdomain.ScopeTools, file: "tools/lookup.json", row: "tool lookup", + note: "Note: 1 standalone tool is not in this plan. Run ldcli sync all to include them.\n", + api: &directAPI{tools: map[string]versionedTool{ + "lookup": {Tool: syncdomain.Tool{Key: "lookup", Schema: map[string]any{"type": "object"}}, Version: 2}, + }}, + }, + { + scope: syncdomain.ScopeSkills, file: "skills/triage.md", row: "skill triage", + note: "Note: 1 standalone skill is not in this plan. Run ldcli sync all to include them.\n", + api: &directAPI{skills: map[string]versionedSkill{ + "triage": {Skill: syncdomain.Skill{Key: "triage", Description: "Triage", Markdown: "Pick a queue.\n"}, Version: 3}, + }}, + }, + } { + t.Run(string(test.scope), func(t *testing.T) { + root := initRepository(t) + writeLocalFile(t, root, "tools/lookup.json", `{"formatVersion": 1, "key": "lookup", "schema": {"type": "object"}}`) + writeLocalFile(t, root, "skills/triage.md", "---\nkey: triage\ndescription: Triage\n---\n\nPick a queue.\n") + require.NoError(t, os.Remove(filepath.Join(root, syncdomain.RootDir, "production", otherFile(test.file)))) + + stdout, err := runSyncAction(t, root, test.api, test.scope, syncprompt.SyncAction{}, true) + require.NoError(t, err) + assert.Contains(t, stdout, `"action": "update_manifest", + "status": "succeeded"`) + + for _, scope := range []syncdomain.Scope{test.scope, syncdomain.ScopeAll} { + stdout, err = runSyncAction(t, root, test.api, scope, syncprompt.SyncAction{DryRun: true}, false) + require.NoError(t, err) + assert.Equal(t, []string{test.row + " in_sync"}, planRows(t, stdout), "scope %s", scope) + } + + stdout, notes, err := runScoped(t, root, test.api, syncprompt.Options{Action: syncprompt.SyncAction{DryRun: true}}) + require.NoError(t, err) + assert.Empty(t, planRows(t, stdout)) + assert.Equal(t, test.note, notes) + }) + } +} + +// otherFile returns the standalone file of the other kind in +// TestScopesSyncAWorkspaceWithOnlyOneTool. +func otherFile(file string) string { + if file == "tools/lookup.json" { + return "skills/triage.md" + } + return "tools/lookup.json" +} + +// The count of standalone files reads only their paths, so an invalid file +// does not break a sync of prompts. +func TestSyncPromptsCountsStandaloneFilesWithoutReadingThem(t *testing.T) { + root, api := writeStandaloneAttachmentWorkspace(t) + writeLocalFile(t, root, "tools/broken.json", "not JSON") + + _, notes, err := runScoped(t, root, api, syncprompt.Options{Action: syncprompt.SyncAction{DryRun: true}}) + + require.NoError(t, err) + assert.Equal(t, "Note: 1 standalone skill and 3 standalone tools are not in this plan. Run ldcli sync all to include them.\n", notes) +} + +// --config-key and --key select variations, so a filtered sync of prompts +// counts the standalone files of its project. +func TestSyncPromptsWithAConfigFilterCountsStandaloneFiles(t *testing.T) { + for _, filter := range []syncdomain.Filter{ + {Project: "production"}, + {Project: "production", ConfigKey: "support"}, + {Project: "production", ConfigKey: "support", Keys: []string{"default"}}, + } { + root, api := writeStandaloneAttachmentWorkspace(t) + + stdout, notes, err := runScoped(t, root, api, syncprompt.Options{Action: syncprompt.SyncAction{Filter: filter, DryRun: true}}) + + require.NoError(t, err) + assert.Equal(t, []string{"variation support/default in_sync"}, planRows(t, stdout), "filter %+v", filter) + assert.Equal(t, "Note: 1 standalone skill and 2 standalone tools are not in this plan. Run ldcli sync all to include them.\n", + notes, "filter %+v", filter) + } +} + +// A tool that a variation pull newly references has a row in the plan, so +// the count of standalone files leaves it out. +func TestSyncPromptsDoesNotCountAToolThatHasARow(t *testing.T) { + root := initRepository(t) + baseline := variation("Baseline") + writeVariation(t, root, baseline, true) + writeManifest(t, root, baseline) + trackTool(t, root, searchTool("old")) + writeLocalFile(t, root, "tools/search.json", + `{"formatVersion": 1, "key": "search", "description": "edited", "schema": {"type": "object"}}`) + server := baseline + server.Tools = []syncdomain.AttachmentRef{{Key: "search", Version: 1}} + api := &directAPI{variation: &server, tools: map[string]versionedTool{"search": {Tool: searchTool("old"), Version: 1}}} + + stdout, notes, err := runScoped(t, root, api, syncprompt.Options{Action: syncprompt.SyncAction{DryRun: true}}) + + require.NoError(t, err) + assert.Contains(t, planRows(t, stdout), "tool search update_server") + assert.Empty(t, notes) +} + +func TestScopesStartFromARepositoryWithoutSyncFiles(t *testing.T) { + root := initRepository(t) + api := &directAPI{tools: map[string]versionedTool{ + "lookup": {Tool: syncdomain.Tool{Key: "lookup", Schema: map[string]any{"type": "object"}}, Version: 2}, + }} + + _, err := runSyncAction(t, root, api, syncdomain.ScopeAll, syncprompt.SyncAction{}, true) + require.EqualError(t, err, "Nothing to sync in this repository. "+ + "To start, run ldcli sync prompts add, ldcli sync tools add, or ldcli sync skills add.") + + _, err = runSyncAction(t, root, api, syncdomain.ScopeTools, syncprompt.SyncAction{}, true) + require.EqualError(t, err, "--project and --key are required with --no-input", "a sync of tools starts the add of tools") + + filter := syncdomain.Filter{Project: "production", Keys: []string{"lookup"}} + stdout, err := runSyncAction(t, root, api, syncdomain.ScopeTools, syncprompt.SyncAction{Filter: filter}, true) + require.NoError(t, err) + assert.Equal(t, "Bootstrapped 1 tool file in .launchdarkly.\n", stdout) + assert.Equal(t, []syncdomain.ResourceID{{Kind: syncdomain.KindTool, ProjectKey: "production", LookupKey: "lookup"}}, + lockResourceIDs(readLock(t, root))) +} + +func TestScopesAddAndDetachTools(t *testing.T) { + root, api := writeStandaloneAttachmentWorkspace(t) + require.NoError(t, os.Remove(filepath.Join(root, syncdomain.RootDir, "production", "tools", "lookup.json"))) + lookup := syncdomain.Filter{Project: "production", Keys: []string{"lookup"}} + lookupFile := filepath.Join(root, syncdomain.RootDir, "production", "tools", "lookup.json") + + stdout, _, err := runScoped(t, root, api, syncprompt.Options{ + Scope: syncdomain.ScopeTools, Action: syncprompt.AddAction{Filter: lookup, DryRun: true}, + }) + require.NoError(t, err) + assert.Equal(t, "1 file to create:\n .launchdarkly/production/tools/lookup.json +7\n", stdout) + _, err = os.Stat(lookupFile) + require.ErrorIs(t, err, os.ErrNotExist) + + _, _, err = runScoped(t, root, api, syncprompt.Options{Scope: syncdomain.ScopeTools, Action: syncprompt.AddAction{Filter: lookup}}) + require.NoError(t, err) + require.FileExists(t, lookupFile) + lookupID := syncdomain.ResourceID{Kind: syncdomain.KindTool, ProjectKey: "production", LookupKey: "lookup"} + assert.Contains(t, lockResourceIDs(readLock(t, root)), lookupID) + + search := syncdomain.Filter{Project: "production", Keys: []string{"search"}} + _, _, err = runScoped(t, root, api, syncprompt.Options{ + Scope: syncdomain.ScopeTools, Action: syncprompt.DetachAction{Filter: search}, Yes: true, + }) + require.EqualError(t, err, + "tool production/search is used by variation production/support/default. Remove it from that variation first.") + + stdout, _, err = runScoped(t, root, api, syncprompt.Options{ + Scope: syncdomain.ScopeTools, Action: syncprompt.DetachAction{Filter: lookup}, Yes: true, + }) + require.NoError(t, err) + assert.Equal(t, "Detached resources:\n- tool production/lookup\n", stdout) + _, err = os.Stat(lookupFile) + require.ErrorIs(t, err, os.ErrNotExist) + assert.NotContains(t, lockResourceIDs(readLock(t, root)), lookupID) + assert.Contains(t, api.tools, "lookup", "LaunchDarkly keeps the tool") +} + +// Only variations have positional selectors, so a tools or skills scope +// rejects them before any request, and the error names the kind of the scope. +func TestScopesOfToolsAndSkillsRejectPositionalSelectors(t *testing.T) { + selector := []syncdomain.ResourceID{syncdomain.VariationID("production", "support", "default")} + for _, scope := range []syncdomain.Scope{syncdomain.ScopeTools, syncdomain.ScopeSkills} { + for _, filter := range []syncdomain.Filter{{}, {Project: "production"}} { + for _, action := range []syncprompt.CommandAction{ + syncprompt.AddAction{Resources: selector, Filter: filter}, + syncprompt.DetachAction{Resources: selector, Filter: filter}, + } { + root, api := writeStandaloneAttachmentWorkspace(t) + api.requests = nil + + _, _, err := runScoped(t, root, api, syncprompt.Options{Scope: scope, Action: action, Yes: true}) + + require.EqualError(t, err, "only variations have positional selectors. Pass "+string(scope)+" as --project and --key.", + "%s %T %+v", scope, action, filter) + assert.Empty(t, api.requests) + } + } + } +} + +// A tool and the variation that uses it form one conflict in a sync of every +// kind. A sync of only that tool resolves it, and the next sync of every kind +// only repins the variation. +func TestScopesResolveASharedToolConflict(t *testing.T) { + root, api := writeStandaloneAttachmentWorkspace(t) + _, err := runSyncAction(t, root, api, syncdomain.ScopeAll, syncprompt.SyncAction{}, true) + require.NoError(t, err) + writeLocalFile(t, root, "tools/search.json", `{"formatVersion": 1, "key": "search", "description": "Local", "schema": {"type": "object"}}`) + search := api.tools["search"] + search.Description, search.Version = pointer("LaunchDarkly"), search.Version+1 + api.tools["search"] = search + + _, notes, err := runScoped(t, root, api, syncprompt.Options{Scope: syncdomain.ScopeAll, Action: syncprompt.SyncAction{}, Yes: true}) + require.EqualError(t, err, "conflict resolution requires --conflict without interactive input") + assert.Contains(t, notes, "Conflict: tool production/search changed on both sides. "+ + "It shares a changed tool or skill with variation production/support/default, so one choice covers them all.") + + api.requests = nil + _, notes, err = runScoped(t, root, api, syncprompt.Options{ + Scope: syncdomain.ScopeTools, + Action: syncprompt.SyncAction{Filter: syncdomain.Filter{Project: "production", Keys: []string{"search"}}}, + ConflictPolicy: syncprompt.ConflictPolicy{Default: syncprompt.ConflictUseLocal}, + Yes: true, + }) + require.NoError(t, err) + assert.Equal(t, 1, countRequests(api.requests, "PATCH", "/ai-tools/search")) + assert.Zero(t, countRequests(api.requests, "PATCH", "/variations/default"), "a sync of tools never writes a variation") + assert.Contains(t, notes, "Note: Prompt production/support/default uses tool search. "+ + "It repins on the next ldcli sync prompts or ldcli sync all.\n") + + stdout, err := runSyncAction(t, root, api, syncdomain.ScopeAll, syncprompt.SyncAction{DryRun: true}, false) + require.NoError(t, err) + assert.Contains(t, planRows(t, stdout), "variation support/default update_server") + assert.Contains(t, planRows(t, stdout), "tool search in_sync") + assert.Contains(t, stdout, `"attachment versions"`, "the variation only repins the tool") +} + func must[T any](value T, err error) T { if err != nil { panic(err) @@ -1694,17 +1924,36 @@ func writeStandaloneAttachmentWorkspace(t *testing.T) (string, *directAPI) { // runSyncAction runs one sync through Runner.Run, because no command selects // a scope yet. It returns the JSON output. -func runSyncAction(t *testing.T, root string, api *directAPI, action syncprompt.SyncAction, yes bool) (string, error) { +func runSyncAction( + t *testing.T, root string, api *directAPI, scope syncdomain.Scope, action syncprompt.CommandAction, yes bool, +) (string, error) { + t.Helper() + stdout, _, err := runScoped(t, root, api, syncprompt.Options{Scope: scope, Action: action, Yes: yes}) + return stdout, err +} + +// runScoped runs options through Runner.Run without a terminal, and returns +// the JSON output and the notes. A question fails the run. +func runScoped(t *testing.T, root string, api *directAPI, options syncprompt.Options) (string, string, error) { t.Helper() + if manifestsByRoot[root] == nil { + manifestsByRoot[root] = &syncapi.SyncManifest{Source: "git:example/repo", Items: []syncapi.SyncManifestResource{}} + } api.manifest = manifestsByRoot[root] - var output bytes.Buffer - err := syncprompt.NewRunner(api).Run(syncprompt.Options{ - WorkingDirectory: root, AccessToken: "token", BaseURI: "https://example.test", OutputKind: "json", - Action: action, Yes: yes, Input: strings.NewReader(""), Output: &output, ErrorOutput: io.Discard, - }) - return output.String(), err + var output, notes bytes.Buffer + options.WorkingDirectory, options.AccessToken, options.BaseURI = root, "token", "https://example.test" + options.OutputKind, options.NoInput, options.Input = "json", true, failingReader{} + options.Output, options.ErrorOutput = &output, ¬es + err := syncprompt.NewRunner(api).Run(options) + return output.String(), notes.String(), err } +// failingReader fails every read, so a test fails if the runner asks a +// question. +type failingReader struct{} + +func (failingReader) Read([]byte) (int, error) { return 0, errors.New("unexpected read of input") } + // planRows returns "kind lookup-key action" for each row of a JSON plan. func planRows(t *testing.T, plan string) []string { t.Helper() diff --git a/internal/sync/prompt/action.go b/internal/sync/prompt/action.go index df9c7cbe..edb7e63e 100644 --- a/internal/sync/prompt/action.go +++ b/internal/sync/prompt/action.go @@ -11,28 +11,26 @@ import ( syncreference "github.com/launchdarkly/ldcli/internal/sync/reference" ) -// CommandAction is one prompt sync operation. +// CommandAction is one sync operation. scope is the scope of the command. type CommandAction interface { - validate() error + validate(scope syncdomain.Scope) error } // SyncAction reconciles the workspace once, or each time a watched file -// changes. Scope is the resource kinds that the sync plans, and Filter limits -// the sync to the resources that it names. +// changes. Filter limits the sync to the resources that it names. type SyncAction struct { Watch bool DryRun bool - Scope syncdomain.Scope Filter syncdomain.Filter } -// AddAction writes selected LaunchDarkly variations as local files. The -// variations come from positional selectors or from the target flags in -// Filter, which also skip the project and config questions. +// AddAction writes selected LaunchDarkly resources of the kind of the scope +// as local files. The resources come from positional selectors or from the +// target flags in Filter, which also skip the project and config questions. type AddAction struct { - Variations []syncdomain.ResourceID - Filter syncdomain.Filter - DryRun bool + Resources []syncdomain.ResourceID + Filter syncdomain.Filter + DryRun bool } // AttachAction adds a tool or a skill to a local variation, and then syncs. @@ -62,22 +60,22 @@ type LinkAction struct { Target *synclink.Target } -func (action SyncAction) validate() error { +func (action SyncAction) validate(scope syncdomain.Scope) error { if action.Watch && action.DryRun { return errors.New("watch does not support --dry-run") } - return action.Filter.Validate(action.Scope.KeyKind()) + return action.Filter.Validate(scope.KeyKind()) } -func (action AddAction) validate() error { - return validateSelections(action.Variations, action.Filter) +func (action AddAction) validate(scope syncdomain.Scope) error { + return validateSelections(action.Resources, action.Filter, scope.KeyKind()) } -func (action DetachAction) validate() error { - return validateSelections(action.Resources, action.Filter) +func (action DetachAction) validate(scope syncdomain.Scope) error { + return validateSelections(action.Resources, action.Filter, scope.KeyKind()) } -func (action AttachAction) validate() error { +func (action AttachAction) validate(syncdomain.Scope) error { if !action.Kind.Valid() { return errors.New("attachment kind must be tool or skill") } @@ -87,7 +85,7 @@ func (action AttachAction) validate() error { return validateProjectFlag(action.ProjectKey, action.Target) } -func (action LinkAction) validate() error { +func (action LinkAction) validate(syncdomain.Scope) error { switch { case action.File == "": return errors.New("linked file is required") @@ -106,13 +104,19 @@ func (action LinkAction) validate() error { return syncreference.ValidateFormat(action.Format) } -// validateSelections rejects variations that come both from positional -// selectors and from target flags. -func validateSelections(selectors []syncdomain.ResourceID, filter syncdomain.Filter) error { - if err := filter.Validate(syncdomain.KindVariation); err != nil { +// validateSelections rejects positional selectors outside the prompts scope, +// and resources that come both from positional selectors and from target +// flags. kind is the kind that --key names. +func validateSelections(selectors []syncdomain.ResourceID, filter syncdomain.Filter, kind syncdomain.Kind) error { + if err := filter.Validate(kind); err != nil { return err } - if len(selectors) != 0 && !filter.IsZero() { + switch { + case len(selectors) == 0: + return nil + case kind != syncdomain.KindVariation: + return fmt.Errorf("only variations have positional selectors. Pass %ss as --project and --key.", kindNoun(kind)) + case !filter.IsZero(): return errors.New("pass variations as --project, --config-key, and --key, " + "or as project-key/config-key/variation-key arguments, not both") } @@ -131,12 +135,13 @@ func validateProjectFlag(projectKey string, target *syncdomain.ResourceID) error return nil } -// selections returns the variations that positional selectors or --key name. -func selections(selectors []syncdomain.ResourceID, filter syncdomain.Filter) []syncdomain.ResourceID { +// selections returns the resources that positional selectors or --key name. +// kind is the kind that --key names. +func selections(selectors []syncdomain.ResourceID, filter syncdomain.Filter, kind syncdomain.Kind) []syncdomain.ResourceID { if len(selectors) != 0 { return selectors } - return filter.IDs(syncdomain.KindVariation) + return filter.IDs(kind) } // Options are the command input and the streams of one prompt sync command. @@ -145,6 +150,9 @@ type Options struct { AccessToken string BaseURI string OutputKind string + // Scope is the resource kinds that the command syncs, adds, or detaches. + // The empty scope is prompt variations. + Scope syncdomain.Scope // Action is the operation to run. A nil action is a SyncAction. Action CommandAction ConflictPolicy ConflictPolicy @@ -166,7 +174,7 @@ func validateOptions(options Options) error { if options.Action == nil { return nil } - return options.Action.validate() + return options.Action.validate(options.Scope) } // withDefaults returns the options with a context and an action. diff --git a/internal/sync/prompt/conflict.go b/internal/sync/prompt/conflict.go index 273fa0f5..4fe26f81 100644 --- a/internal/sync/prompt/conflict.go +++ b/internal/sync/prompt/conflict.go @@ -94,6 +94,15 @@ type conflictChoice struct { sourcesChanged bool } +// missingConflictChoiceError names the flags that choose a side without a +// terminal. Only sync prompts has --resolve. +func missingConflictChoiceError(scope syncdomain.Scope) error { + if scope == "" || scope == syncdomain.ScopePrompts { + return errors.New("conflict resolution requires --conflict or --resolve without interactive input") + } + return errors.New("conflict resolution requires --conflict without interactive input") +} + // resolveConflicts shows each conflict and gets a choice from the policy or // from the user. In watch mode, a file change during the choice stops it. func resolveConflicts( @@ -128,9 +137,7 @@ func resolveConflicts( return conflictResolutionResult{}, errConflictAborted } case !interactive: - return conflictResolutionResult{}, errors.New( - "conflict resolution requires --conflict or --resolve without interactive input", - ) + return conflictResolutionResult{}, missingConflictChoiceError(options.Scope) case watched != nil: choice, err = promptWatchedConflictResolution(options.Context, options.Input, options.ErrorOutput, *watched) default: @@ -308,7 +315,8 @@ func writeConflictChoice(output io.Writer, choice conflictChoice) { // resource has the action for its chosen side. The original plan stays as // the reviewed record. func applyConflictResolutions(plan Plan, resolutions map[ResourceID]ConflictResolution) Plan { - resolved := Plan{Resources: make([]PlannedResource, 0, len(plan.Resources))} + resolved := plan + resolved.Resources = make([]PlannedResource, 0, len(plan.Resources)) for _, resource := range plan.Resources { if !chosenSideUses(resource, resolutions) { continue diff --git a/internal/sync/prompt/output.go b/internal/sync/prompt/output.go index 90bf18cb..20786661 100644 --- a/internal/sync/prompt/output.go +++ b/internal/sync/prompt/output.go @@ -164,7 +164,7 @@ func checkOutputKind(outputKind string) (string, error) { func writePlanReview(out io.Writer, outputKind string, plan Plan, width int, showDiffs bool) error { console := syncconsole.New(out) if len(plan.Resources) == 0 { - _ = console.Line("No prompt variations are synced in this repository. To start, run ldcli sync prompts add.") + _ = console.Line(emptyPlanMessage(plan.Scope)) return nil } _ = console.Line(planSummary(plan)) @@ -198,8 +198,21 @@ func writePlanReview(out io.Writer, outputKind string, plan Plan, width int, sho return nil } +// emptyPlanMessage tells the user how to start syncing the kinds of a scope. +func emptyPlanMessage(scope syncdomain.Scope) string { + switch scope { + case syncdomain.ScopeAll: + return nothingToSync + case syncdomain.ScopeTools, syncdomain.ScopeSkills: + return fmt.Sprintf("No %s are synced in this repository. To start, run ldcli sync %s add.", scope, scope) + default: + return "No prompt variations are synced in this repository. To start, run ldcli sync prompts add." + } +} + // writePlanNotes writes one line for each resource that another working copy -// synced after this working copy wrote its sync.lock file. +// synced after this working copy wrote its sync.lock file, and one line for +// the standalone tool and skill files that a prompts plan leaves out. func writePlanNotes(out io.Writer, plan Plan) { console := syncconsole.New(out) for _, resource := range plan.Resources { @@ -209,6 +222,41 @@ func writePlanNotes(out io.Writer, plan Plan) { ) } } + + var counts []string + total := 0 + for _, kind := range []syncdomain.Kind{syncdomain.KindSkill, syncdomain.KindTool} { + if count := plan.standalone[kind]; count != 0 { + counts = append(counts, countOf(count, "standalone "+kindNoun(kind))) + total += count + } + } + if total != 0 { + verb := "are" + if total == 1 { + verb = "is" + } + _ = console.Printf("Note: %s %s not in this plan. Run ldcli sync all to include them.\n", strings.Join(counts, " and "), verb) + } +} + +// writeRepinNotes writes one line for each local variation that uses a tool or +// a skill that a sync of tools or skills pushed. That sync does not write +// variations, so the variation repins on the next sync of prompts. +func writeRepinNotes(out io.Writer, plan Plan, outcomes []ResourceOutcome) { + if plan.Scope.Includes(syncdomain.KindVariation) { + return + } + console := syncconsole.New(out) + for index, outcome := range outcomes { + if outcome.Status != OutcomeSucceeded || !outcome.Action.changesServer() { + continue + } + for _, variation := range plan.Resources[index].usedBy { + _ = console.Printf("Note: Prompt %s uses %s %s. It repins on the next ldcli sync prompts or ldcli sync all.\n", + variation, outcome.ID.Kind, outcome.ID.LookupKey) + } + } } // planSummary describes the plan in one line, for example diff --git a/internal/sync/prompt/output_test.go b/internal/sync/prompt/output_test.go index 4563ae0a..e16544de 100644 --- a/internal/sync/prompt/output_test.go +++ b/internal/sync/prompt/output_test.go @@ -253,6 +253,20 @@ func TestWritePlanReviewSummarizesPlansWithoutChanges(t *testing.T) { assert.Equal(t, "No changes. 2 in sync.\n", output.String()) } +func TestWritePlanReviewNamesTheAddOfAnEmptyScope(t *testing.T) { + for scope, want := range map[syncdomain.Scope]string{ + syncdomain.ScopePrompts: "No prompt variations are synced in this repository. To start, run ldcli sync prompts add.\n", + syncdomain.ScopeTools: "No tools are synced in this repository. To start, run ldcli sync tools add.\n", + syncdomain.ScopeSkills: "No skills are synced in this repository. To start, run ldcli sync skills add.\n", + syncdomain.ScopeAll: "Nothing to sync in this repository. " + + "To start, run ldcli sync prompts add, ldcli sync tools add, or ldcli sync skills add.\n", + } { + var output bytes.Buffer + require.NoError(t, writePlanReview(&output, "plaintext", Plan{Scope: scope}, 0, false)) + assert.Equal(t, want, output.String(), "scope %s", scope) + } +} + func TestPlanReasonNamesTheChange(t *testing.T) { variation := testVariation("Support") tests := map[string]struct { diff --git a/internal/sync/prompt/plan.go b/internal/sync/prompt/plan.go index 11b8f518..0520ff89 100644 --- a/internal/sync/prompt/plan.go +++ b/internal/sync/prompt/plan.go @@ -76,6 +76,8 @@ type PlannedResource struct { // conflictUses are the conflicted variations that newly use the tool or // skill of this row on one side, when no push or pull needs the row. conflictUses []conflictUse + // usedBy are the local variations that use the tool or skill of this row. + usedBy []ResourceID // restoreRef is the link that a restored variation file keeps. restoreRef *syncdomain.Reference } @@ -115,6 +117,13 @@ type conflictUse struct { // Plan is the sync decision for each resource, in identity order. type Plan struct { Resources []PlannedResource + // Scope is the resource kinds that the plan covers. The empty scope is + // prompt variations. + Scope syncdomain.Scope + // standalone counts the tool and skill files, by kind, that no local + // variation uses. Only a prompts plan counts them, because it leaves them + // out. + standalone map[syncdomain.Kind]int } // BuildPlan compares the baseline with the local and the server state. The diff --git a/internal/sync/prompt/runner.go b/internal/sync/prompt/runner.go index 4dfa69c3..ebec7b2c 100644 --- a/internal/sync/prompt/runner.go +++ b/internal/sync/prompt/runner.go @@ -99,9 +99,17 @@ func (runner Runner) Run(options Options) error { return runner.runWorkspaceSync(options, workspace, nil) } +// nothingToSync tells the user how to start a sync of every kind. +const nothingToSync = "Nothing to sync in this repository. " + + "To start, run ldcli sync prompts add, ldcli sync tools add, or ldcli sync skills add." + +// errNothingToSync is the error of a sync of every kind in a repository +// without sync files. A sync of one kind starts its add instead. +var errNothingToSync = errors.New(nothingToSync) + // runSync syncs once, or watches the workspace. A workspace without any sync -// files starts with an add in the filter project, so that the user can choose -// the first variations. +// files starts with an add of the scope's kind in the filter project, so that +// the user can choose the first resources. func (runner Runner) runSync(options Options, workspace syncWorkspace, action SyncAction) error { exists, err := workspace.local.Exists() if err != nil { @@ -112,8 +120,10 @@ func (runner Runner) runSync(options Options, workspace syncWorkspace, action Sy return err } if !exists && len(synced) == 0 { - add := AddAction{Filter: action.Filter, DryRun: action.DryRun} - if err := runner.bootstrap(runner.bootstrapOptions(options, workspace, true, add)); err != nil { + if options.Scope == syncdomain.ScopeAll { + return errNothingToSync + } + if err := runner.runAdd(options, workspace, AddAction{Filter: action.Filter, DryRun: action.DryRun}); err != nil { return err } if !action.Watch { @@ -155,9 +165,9 @@ func (runner Runner) runDetach(options Options, workspace syncWorkspace, action ProjectKeys: projectKeys(synced, syncdomain.Filter{}), Input: options.Input, Output: options.Output, - Kind: syncdomain.KindVariation, + Kind: options.Scope.KeyKind(), Filter: action.Filter, - Selections: selections(action.Resources, action.Filter), + Selections: selections(action.Resources, action.Filter, options.Scope.KeyKind()), NoInput: options.NoInput, Archive: action.Archive, Archiver: runner.api(options), @@ -222,7 +232,9 @@ func (runner Runner) runAttach(options Options, workspace syncWorkspace, action func (runner Runner) bootstrapOptions(options Options, workspace syncWorkspace, initial bool, action AddAction) syncbootstrap.Options { api := runner.api(options) + kind := options.Scope.KeyKind() return syncbootstrap.Options{ + Kind: kind, Catalog: api, Attachments: api, Store: workspace.local, @@ -233,7 +245,7 @@ func (runner Runner) bootstrapOptions(options Options, workspace syncWorkspace, DryRun: action.DryRun, ProjectKey: action.Filter.Project, ConfigKey: action.Filter.ConfigKey, - Selections: selections(action.Variations, action.Filter), + Selections: selections(action.Resources, action.Filter, kind), NoInput: options.NoInput, Diff: options.Diff, } diff --git a/internal/sync/prompt/state.go b/internal/sync/prompt/state.go index 9de01381..cf4ae4c9 100644 --- a/internal/sync/prompt/state.go +++ b/internal/sync/prompt/state.go @@ -69,12 +69,14 @@ func loadWorkspacePlan( // reads it from LaunchDarkly once. attachments := newAttachmentCache(client) - var plan Plan + plan := Plan{Scope: scope} var localFiles map[ResourceID]syncdomain.SyncedResource if scope.Includes(syncdomain.KindVariation) { - if plan, localFiles, err = planVariationRows(repositoryRoot, baseline, client, attachments, filter, variations); err != nil { + var rows Plan + if rows, localFiles, err = planVariationRows(repositoryRoot, baseline, client, attachments, filter, variations); err != nil { return Plan{}, nil, err } + plan.Resources = rows.Resources } if scope.Includes(syncdomain.KindTool) || scope.Includes(syncdomain.KindSkill) { include := func(id ResourceID) bool { return scope.Includes(id.Kind) && filter.Includes(id) } @@ -96,11 +98,59 @@ func loadWorkspacePlan( checkAttachmentBaselines(plan.Resources, baseline.Lock) checkAttachmentDeletes(plan.Resources, variations) for index := range plan.Resources { - plan.Resources[index].SyncedElsewhere = baseline.Stale(plan.Resources[index].ID) + resource := &plan.Resources[index] + resource.SyncedElsewhere = baseline.Stale(resource.ID) + if resource.ID.Kind != syncdomain.KindVariation { + resource.usedBy = usersOf(variations, resource.ID) + } + } + if scope.Includes(syncdomain.KindVariation) && !scope.Includes(syncdomain.KindTool) { + if plan.standalone, err = countStandalone(repositoryRoot, variations, plan.Resources, filter.Project); err != nil { + return Plan{}, nil, err + } } return plan, localFiles, nil } +// usersOf returns the local variations that use a tool or a skill. +func usersOf(variations []syncdomain.SyncedResource, id ResourceID) []ResourceID { + var users []ResourceID + for _, variation := range variations { + if uses(variation, id) { + users = append(users, variation.ID()) + } + } + return users +} + +// countStandalone counts the tool and skill files, by kind, in project (or in +// every project when it is empty), that no local variation uses, and that have +// no row in the plan. --config-key and --key select only variations, so they +// do not change the count. It reads only paths, so an invalid file does not +// stop the count. +func countStandalone( + repositoryRoot string, + variations []syncdomain.SyncedResource, + planned []PlannedResource, + project string, +) (map[syncdomain.Kind]int, error) { + files, err := synclocal.SourceFiles(repositoryRoot) + if err != nil { + return nil, err + } + counts := make(map[syncdomain.Kind]int) + for _, file := range files { + id, ok := synclocal.ParseManagedPath(file) + standalone := ok && id.Kind != syncdomain.KindVariation && (project == "" || id.ProjectKey == project) && + len(usersOf(variations, id)) == 0 && + !slices.ContainsFunc(planned, func(resource PlannedResource) bool { return resource.ID == id }) + if standalone { + counts[id.Kind]++ + } + } + return counts, nil +} + // planVariationRows reads the variations that the filter keeps from // LaunchDarkly, and compares both sides with the lock. func planVariationRows( diff --git a/internal/sync/prompt/sync.go b/internal/sync/prompt/sync.go index cec9598a..a7463404 100644 --- a/internal/sync/prompt/sync.go +++ b/internal/sync/prompt/sync.go @@ -32,7 +32,7 @@ func (runner Runner) runWorkspaceSync(options Options, workspace syncWorkspace, } target := options.syncAction() - reviewed, err := workspace.loadState(client, target.Scope, target.Filter) + reviewed, err := workspace.loadState(client, options.Scope, target.Filter) if err != nil { return err } @@ -65,7 +65,7 @@ func (runner Runner) runWorkspaceSync(options Options, workspace syncWorkspace, } // Read both sides again, so that no action uses state from before the review. - current, err := workspace.loadState(client, target.Scope, target.Filter) + current, err := workspace.loadState(client, options.Scope, target.Filter) if err != nil { return err } @@ -111,5 +111,6 @@ func (runner Runner) applyPlan( if err := writeOutcomeOutput(options.Output, options.OutputKind, plan, outcomes); err != nil { failures = append(failures, err) } + writeRepinNotes(options.ErrorOutput, plan, outcomes) return errors.Join(failures...) }