From 68a0951aa34967ea378125774874d4f702e7c4d9 Mon Sep 17 00:00:00 2001 From: Cliff Tawiah Date: Thu, 22 Oct 2026 21:06:57 +0000 Subject: [PATCH 1/4] fix: plan a tool or skill row when a variation newly references it A variation row writes the content of each tool and skill that it uses. The guard for tool and skill changes skips one that the overwritten variation does not use yet. So in a sync of prompts, two overwrites happened: - A push of a variation that newly attaches a tool published the old local copy over a change in LaunchDarkly. - A pull of a variation that newly references a tool wrote the old LaunchDarkly copy over a local edit. The plan now has a row for such a tool or skill when it exists on both sides with different content. The row decides the content, as in a sync of every kind: it pulls, pushes, or is a conflict. The variation row then uses that content and pins its version. A tool or skill with equal sides gets no row, so other plans do not change. --- internal/sync/prompt/acceptance_test.go | 146 ++++++++++++++++++++++++ internal/sync/prompt/plan.go | 17 +++ internal/sync/prompt/state.go | 88 +++++++++++--- 3 files changed, 236 insertions(+), 15 deletions(-) diff --git a/internal/sync/prompt/acceptance_test.go b/internal/sync/prompt/acceptance_test.go index ea22d9ee..a4defe69 100644 --- a/internal/sync/prompt/acceptance_test.go +++ b/internal/sync/prompt/acceptance_test.go @@ -682,6 +682,152 @@ func TestPromptUpdateResolvesOmittedModelConfigVersionToLatest(t *testing.T) { requireOnlyReads(t, api.requests) } +// A push of a variation that newly attaches a tracked tool must not publish +// the local copy over a change in LaunchDarkly. The plan pulls the tool, and +// the variation pins the pulled version. +func TestPromptPushOfANewToolReferenceKeepsTheLaunchDarklyChange(t *testing.T) { + root := initRepository(t) + baseline := variation("Baseline") + writeManifest(t, root, baseline) + trackTool(t, root, searchTool("old")) + local := baseline + local.Tools = []syncdomain.AttachmentRef{{Key: "search"}} + local.Attachments = []syncdomain.Attachment{toolAttachment(searchTool("old"), 0)} + writeVariation(t, root, local, true) + api := &directAPI{variation: pointer(baseline), tools: map[string]versionedTool{"search": {Tool: searchTool("new"), Version: 2}}} + + stdout, _, err := runPrompt(t, root, api, "--dry-run", "--output", "json") + require.NoError(t, err) + assert.Equal(t, []string{"tool search update_local", "variation support/default update_server"}, planRows(t, stdout)) + + _, _, err = runPrompt(t, root, api, "--yes") + + require.NoError(t, err) + assert.Zero(t, countRequests(api.requests, "PATCH", "/ai-tools/search"), "the push must not publish the old local copy") + assert.Equal(t, "new", *api.tools["search"].Description) + assert.Equal(t, []syncdomain.AttachmentRef{{Key: "search", Version: 2}}, api.variation.Tools) + assert.Contains(t, readLocalFile(t, root, "tools/search.json"), `"description": "new"`) +} + +// A pull of a variation that newly references a tracked tool must not write +// the LaunchDarkly copy over a local edit. The plan pushes the tool, and the +// file keeps the edit. The next sync only repins the variation. +func TestPromptPullOfANewToolReferenceKeepsTheLocalEdit(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, _, err := runPrompt(t, root, api, "--dry-run", "--output", "json") + require.NoError(t, err) + assert.Equal(t, []string{"tool search update_server", "variation support/default update_local"}, planRows(t, stdout)) + + _, _, err = runPrompt(t, root, api, "--yes") + + require.NoError(t, err) + assert.Contains(t, readLocalFile(t, root, "tools/search.json"), `"description": "edited"`, "the local edit stays") + assert.Equal(t, "edited", *api.tools["search"].Description) + assert.Equal(t, 2, api.tools["search"].Version) + stdout, _, err = runPrompt(t, root, api, "--dry-run", "--output", "json") + require.NoError(t, err) + assert.Equal(t, []string{"variation support/default update_server"}, planRows(t, stdout)) + assert.Contains(t, stdout, `"attachment versions"`, "the variation only repins the tool") +} + +// The same holds for a skill. +func TestPromptPullOfANewSkillReferenceKeepsTheLocalEdit(t *testing.T) { + root := initRepository(t) + baseline := variation("Baseline") + writeVariation(t, root, baseline, true) + writeManifest(t, root, baseline) + summarize := syncdomain.Skill{Key: "summarize", Name: "Summarize", Description: "Summarize", Markdown: "Old.\n"} + trackAttachment(t, root, syncdomain.Attachment{Kind: syncdomain.AttachmentSkill, Version: 1, Skill: &summarize}) + writeLocalFile(t, root, "skills/summarize.md", "---\nkey: summarize\ndescription: Summarize\n---\n\nEdited.\n") + server := baseline + server.Skills = []syncdomain.AttachmentRef{{Key: "summarize", Version: 1}} + api := &directAPI{variation: &server, skills: map[string]versionedSkill{"summarize": {Skill: summarize, Version: 1}}} + + _, _, err := runPrompt(t, root, api, "--yes") + + require.NoError(t, err) + assert.Contains(t, readLocalFile(t, root, "skills/summarize.md"), "Edited.", "the local edit stays") + assert.Equal(t, "Edited.\n", api.skills["summarize"].Markdown) +} + +// When both sides of a newly referenced tool changed, the tool row is a +// conflict, and one choice covers the tool and the variation. +func TestPromptNewToolReferenceWithTwoChangesIsAConflict(t *testing.T) { + root := initRepository(t) + baseline := variation("Baseline") + writeManifest(t, root, baseline) + trackTool(t, root, searchTool("old")) + local := baseline + local.Tools = []syncdomain.AttachmentRef{{Key: "search"}} + local.Attachments = []syncdomain.Attachment{toolAttachment(searchTool("edited"), 0)} + writeVariation(t, root, local, true) + api := &directAPI{variation: pointer(baseline), tools: map[string]versionedTool{"search": {Tool: searchTool("new"), Version: 2}}} + + _, stderr, err := runPrompt(t, root, api, "--yes", "--no-input") + + require.ErrorContains(t, err, "conflict resolution requires --conflict or --resolve without interactive input") + assert.Contains(t, stderr, "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.") + requireOnlyReads(t, api.requests) +} + +// A newly referenced tool whose two sides are equal cannot be overwritten, so +// the plan has no row for it. +func TestPromptNewToolReferenceWithEqualSidesPlansNoToolRow(t *testing.T) { + root := initRepository(t) + baseline := variation("Baseline") + writeManifest(t, root, baseline) + trackTool(t, root, searchTool("same")) + local := baseline + local.Tools = []syncdomain.AttachmentRef{{Key: "search"}} + local.Attachments = []syncdomain.Attachment{toolAttachment(searchTool("same"), 0)} + writeVariation(t, root, local, true) + api := &directAPI{variation: pointer(baseline), tools: map[string]versionedTool{"search": {Tool: searchTool("same"), Version: 1}}} + + stdout, _, err := runPrompt(t, root, api, "--dry-run", "--output", "json") + + require.NoError(t, err) + assert.Equal(t, []string{"variation support/default update_server"}, planRows(t, stdout)) +} + +func searchTool(description string) syncdomain.Tool { + return syncdomain.Tool{Key: "search", Description: pointer(description), Schema: map[string]any{"type": "object"}} +} + +// trackTool records the tool in sync.lock and in the remote manifest, as a +// sync that synced it does. +func trackTool(t *testing.T, root string, tool syncdomain.Tool) { + t.Helper() + trackAttachment(t, root, toolAttachment(tool, 1)) +} + +func trackAttachment(t *testing.T, root string, attachment syncdomain.Attachment) { + t.Helper() + lock := readLock(t, root) + require.NoError(t, lock.SetAttachments("production", []syncdomain.Attachment{attachment})) + require.NoError(t, syncmanifest.WriteLock(synclocal.NewStore(root), lock)) + fingerprint, err := syncdomain.FingerprintAttachment("production", attachment) + require.NoError(t, err) + manifestsByRoot[root].Items = append(manifestsByRoot[root].Items, syncapi.SyncManifestResource{ + ResourceKind: syncdomain.Kind(attachment.Kind), ResourceLookupKey: attachment.Key(), Fingerprint: fingerprint, Version: 1, + }) +} + +func readLocalFile(t *testing.T, root, projectPath string) string { + t.Helper() + return string(must(os.ReadFile(filepath.Join(root, syncdomain.RootDir, "production", filepath.FromSlash(projectPath))))) +} + func TestPromptPushesLocalChangeAndAdvancesManifest(t *testing.T) { root := initRepository(t) baseline := variation("Baseline") diff --git a/internal/sync/prompt/plan.go b/internal/sync/prompt/plan.go index ad6e281f..736a9a16 100644 --- a/internal/sync/prompt/plan.go +++ b/internal/sync/prompt/plan.go @@ -87,6 +87,23 @@ func (resource PlannedResource) hasServer() bool { return resource.Server != nil || resource.ServerAttachment != nil } +// writtenSides returns the variation that a variation row writes, and the +// variation that the write replaces. A push writes the local variation to +// LaunchDarkly, and a pull writes the LaunchDarkly variation to the local +// file. Both are nil for a row that writes no variation. +func (resource PlannedResource) writtenSides() (source, target *syncdomain.Variation) { + switch { + case resource.ID.Kind != syncdomain.KindVariation: + return nil, nil + case resource.Action.changesServer(): + return resource.Local, resource.Server + case resource.Action == ActionUpdateLocal: + return resource.Server, resource.Local + default: + return nil, nil + } +} + // Plan is the sync decision for each resource, in identity order. type Plan struct { Resources []PlannedResource diff --git a/internal/sync/prompt/state.go b/internal/sync/prompt/state.go index 4383abd0..39f14172 100644 --- a/internal/sync/prompt/state.go +++ b/internal/sync/prompt/state.go @@ -83,10 +83,15 @@ func loadWorkspacePlan( return Plan{}, nil, err } plan.Resources = append(plan.Resources, rows...) - slices.SortFunc(plan.Resources, func(left, right PlannedResource) int { - return syncdomain.CompareResourceIDs(left.ID, right.ID) - }) } + referenced, err := planReferencedAttachments(repositoryRoot, baseline.Lock, attachments, plan.Resources) + if err != nil { + return Plan{}, nil, err + } + plan.Resources = append(plan.Resources, referenced...) + slices.SortFunc(plan.Resources, func(left, right PlannedResource) int { + return syncdomain.CompareResourceIDs(left.ID, right.ID) + }) checkAttachmentBaselines(plan.Resources, baseline.Lock) checkAttachmentDeletes(plan.Resources, variations) @@ -180,6 +185,69 @@ func planAttachmentRows( return planAttachments(tracked, local, server), nil } +// planReferencedAttachments plans a row for each tool and skill that a +// variation row writes to a side that does not use it yet, when the tool or +// skill exists on both sides with different content. Without the row, the +// variation row would write its own copy over the other side. The row decides +// the content, and executePlan applies it before the variation rows. +// checkAttachmentBaselines covers the tools and skills that both sides use. +func planReferencedAttachments( + repositoryRoot string, + lock syncmanifest.Manifest, + attachments *attachmentCache, + resources []PlannedResource, +) ([]PlannedResource, error) { + referenced := make(map[ResourceID]bool) + for _, resource := range resources { + for _, id := range newReferences(resource) { + referenced[id] = true + } + } + for _, resource := range resources { + delete(referenced, resource.ID) + } + if len(referenced) == 0 { + return nil, nil + } + + rows, err := planAttachmentRows(repositoryRoot, lock, attachments, func(id ResourceID) bool { return referenced[id] }) + if err != nil { + return nil, err + } + return slices.DeleteFunc(rows, func(row PlannedResource) bool { + return !row.hasLocal() || !row.hasServer() || row.LocalFingerprint == row.ServerFingerprint + }), nil +} + +// newReferences returns each tool and skill that a variation row writes to a +// side that does not use it yet. A conflict can write either side. +func newReferences(resource PlannedResource) []ResourceID { + var ids []ResourceID + add := func(source, target *syncdomain.Variation) { + if source == nil { + return + } + for _, attachment := range source.Attachments { + if target == nil || !hasAttachment(*target, attachment) { + ids = append(ids, attachment.ID(resource.ID.ProjectKey)) + } + } + } + if resource.ID.Kind == syncdomain.KindVariation && resource.Action == ActionConflict { + add(resource.Local, resource.Server) + add(resource.Server, resource.Local) + } else { + add(resource.writtenSides()) + } + return ids +} + +// hasAttachment reports whether a variation uses the tool or skill of attachment. +func hasAttachment(variation syncdomain.Variation, attachment syncdomain.Attachment) bool { + _, ok := variation.Attachment(attachment.Kind, attachment.Key()) + return ok +} + // checkAttachmentBaselines makes sure that a variation row does not move a // tool or a skill past a change on the other side. sync.lock records the // state of each tool and skill on its own. A row becomes a conflict when the @@ -193,18 +261,8 @@ func checkAttachmentBaselines(resources []PlannedResource, lock syncmanifest.Man for index := range resources { resource := &resources[index] - var source, target *syncdomain.Variation - switch { - case resource.ID.Kind != syncdomain.KindVariation: - continue - case resource.Action.changesServer(): - source, target = resource.Local, resource.Server - case resource.Action == ActionUpdateLocal: - source, target = resource.Server, resource.Local - default: - continue - } - if target == nil { + source, target := resource.writtenSides() + if source == nil || target == nil { continue } for _, id := range resource.changedAttachments { From 0a8d6d07454a6a246a9a08bbead1735b6f67ed25 Mon Sep 17 00:00:00 2001 From: Cliff Tawiah Date: Fri, 23 Oct 2026 14:24:04 +0000 Subject: [PATCH 2/4] fix: plan no extra tool or skill row for a conflicted variation A conflicted variation planned a row for each tool and skill that one of its sides newly used. The conflict choice of the variation then also covered that row. So a choice of LaunchDarkly, whose version does not use the tool, pulled the tool over a local edit. A conflict writes no side until the user chooses one. So only a push or a pull of a variation now plans these rows. A conflict plans no extra rows. The comment of writtenSides is shorter. --- internal/sync/prompt/acceptance_test.go | 27 +++++++++++++++++++++++++ internal/sync/prompt/plan.go | 4 +--- internal/sync/prompt/state.go | 24 +++++++++------------- 3 files changed, 37 insertions(+), 18 deletions(-) diff --git a/internal/sync/prompt/acceptance_test.go b/internal/sync/prompt/acceptance_test.go index a4defe69..af4fa1e4 100644 --- a/internal/sync/prompt/acceptance_test.go +++ b/internal/sync/prompt/acceptance_test.go @@ -781,6 +781,33 @@ func TestPromptNewToolReferenceWithTwoChangesIsAConflict(t *testing.T) { requireOnlyReads(t, api.requests) } +// A conflict choice covers only the conflicted variation. A tool that only its +// local version newly uses gets no row, so a choice of LaunchDarkly, which +// does not use the tool, does not pull the tool over its local edit. +func TestPromptConflictChoiceDoesNotPullAToolThatOnlyTheLocalVersionUses(t *testing.T) { + root := initRepository(t) + baseline := variation("Baseline") + writeManifest(t, root, baseline) + trackTool(t, root, searchTool("old")) + local := variation("Local") + local.Tools = []syncdomain.AttachmentRef{{Key: "search"}} + local.Attachments = []syncdomain.Attachment{toolAttachment(searchTool("edited"), 0)} + writeVariation(t, root, local, true) + server := variation("LaunchDarkly") + api := &directAPI{variation: &server, tools: map[string]versionedTool{"search": {Tool: searchTool("old"), Version: 1}}} + + stdout, _, err := runPrompt(t, root, api, "--dry-run", "--output", "json") + require.ErrorContains(t, err, "cannot sync conflicted resource variation production/support/default") + assert.Equal(t, []string{"variation support/default conflict"}, planRows(t, stdout)) + + stdout, _, err = runPrompt(t, root, api, "--conflict", "launchdarkly", "--yes", "--no-input", "--output", "json") + + require.NoError(t, err) + assert.Equal(t, []string{"variation support/default update_local"}, planRows(t, stdout), "the choice does not pull the tool") + assert.Equal(t, "old", *api.tools["search"].Description) + assert.Zero(t, countRequests(api.requests, "PATCH", "/ai-tools/search")) +} + // A newly referenced tool whose two sides are equal cannot be overwritten, so // the plan has no row for it. func TestPromptNewToolReferenceWithEqualSidesPlansNoToolRow(t *testing.T) { diff --git a/internal/sync/prompt/plan.go b/internal/sync/prompt/plan.go index 736a9a16..eea0ebc8 100644 --- a/internal/sync/prompt/plan.go +++ b/internal/sync/prompt/plan.go @@ -88,9 +88,7 @@ func (resource PlannedResource) hasServer() bool { } // writtenSides returns the variation that a variation row writes, and the -// variation that the write replaces. A push writes the local variation to -// LaunchDarkly, and a pull writes the LaunchDarkly variation to the local -// file. Both are nil for a row that writes no variation. +// variation that it replaces. Both are nil for a row that writes none. func (resource PlannedResource) writtenSides() (source, target *syncdomain.Variation) { switch { case resource.ID.Kind != syncdomain.KindVariation: diff --git a/internal/sync/prompt/state.go b/internal/sync/prompt/state.go index 39f14172..2263d679 100644 --- a/internal/sync/prompt/state.go +++ b/internal/sync/prompt/state.go @@ -220,24 +220,18 @@ func planReferencedAttachments( } // newReferences returns each tool and skill that a variation row writes to a -// side that does not use it yet. A conflict can write either side. +// side that does not use it yet. A conflict has none, because it writes no +// side until the user chooses one, and that choice covers only the variation. func newReferences(resource PlannedResource) []ResourceID { + source, target := resource.writtenSides() + if source == nil { + return nil + } var ids []ResourceID - add := func(source, target *syncdomain.Variation) { - if source == nil { - return + for _, attachment := range source.Attachments { + if target == nil || !hasAttachment(*target, attachment) { + ids = append(ids, attachment.ID(resource.ID.ProjectKey)) } - for _, attachment := range source.Attachments { - if target == nil || !hasAttachment(*target, attachment) { - ids = append(ids, attachment.ID(resource.ID.ProjectKey)) - } - } - } - if resource.ID.Kind == syncdomain.KindVariation && resource.Action == ActionConflict { - add(resource.Local, resource.Server) - add(resource.Server, resource.Local) - } else { - add(resource.writtenSides()) } return ids } From def885801d60d4c6bf744b3c57a81a75c2cc71e2 Mon Sep 17 00:00:00 2001 From: Cliff Tawiah Date: Fri, 23 Oct 2026 14:36:05 +0000 Subject: [PATCH 3/4] fix: follow the conflict choice for a tool or skill that one side newly uses A conflicted variation planned no row for a tool or skill that one of its sides newly used. So a choice of local published the local copy over a LaunchDarkly change of the tool, and a tool that changed on both sides never became part of the conflict. Each side of a conflict now counts. A row that only conflicts need joins the conflict when the tool or skill changed on both sides. Otherwise it keeps its own action, and stays in the plan only when the chosen side uses the tool or skill. So a choice of local pulls the LaunchDarkly change first, and a choice of LaunchDarkly still leaves a tool that it does not use as it is. --- internal/sync/prompt/acceptance_test.go | 59 +++++++++++++++++++++++-- internal/sync/prompt/conflict.go | 28 ++++++++++-- internal/sync/prompt/plan.go | 10 +++++ internal/sync/prompt/state.go | 50 +++++++++++++++------ 4 files changed, 125 insertions(+), 22 deletions(-) diff --git a/internal/sync/prompt/acceptance_test.go b/internal/sync/prompt/acceptance_test.go index af4fa1e4..c64d4a69 100644 --- a/internal/sync/prompt/acceptance_test.go +++ b/internal/sync/prompt/acceptance_test.go @@ -781,9 +781,10 @@ func TestPromptNewToolReferenceWithTwoChangesIsAConflict(t *testing.T) { requireOnlyReads(t, api.requests) } -// A conflict choice covers only the conflicted variation. A tool that only its -// local version newly uses gets no row, so a choice of LaunchDarkly, which -// does not use the tool, does not pull the tool over its local edit. +// A tool that only the local version of a conflicted variation newly uses +// keeps its own row and action. A choice of LaunchDarkly, which does not use +// the tool, drops that row, so the choice does not pull the tool over its +// local edit. func TestPromptConflictChoiceDoesNotPullAToolThatOnlyTheLocalVersionUses(t *testing.T) { root := initRepository(t) baseline := variation("Baseline") @@ -798,7 +799,7 @@ func TestPromptConflictChoiceDoesNotPullAToolThatOnlyTheLocalVersionUses(t *test stdout, _, err := runPrompt(t, root, api, "--dry-run", "--output", "json") require.ErrorContains(t, err, "cannot sync conflicted resource variation production/support/default") - assert.Equal(t, []string{"variation support/default conflict"}, planRows(t, stdout)) + assert.Equal(t, []string{"tool search update_server", "variation support/default conflict"}, planRows(t, stdout)) stdout, _, err = runPrompt(t, root, api, "--conflict", "launchdarkly", "--yes", "--no-input", "--output", "json") @@ -808,6 +809,56 @@ func TestPromptConflictChoiceDoesNotPullAToolThatOnlyTheLocalVersionUses(t *test assert.Zero(t, countRequests(api.requests, "PATCH", "/ai-tools/search")) } +// A choice of local for a conflicted variation that newly uses a tool must not +// publish the old local copy over a LaunchDarkly change of the tool. The tool +// row keeps its own pull, and the variation pins the pulled version. +func TestPromptConflictChoiceOfLocalKeepsTheLaunchDarklyChangeOfANewTool(t *testing.T) { + root := initRepository(t) + baseline := variation("Baseline") + writeManifest(t, root, baseline) + trackTool(t, root, searchTool("old")) + local := variation("Local") + local.Tools = []syncdomain.AttachmentRef{{Key: "search"}} + local.Attachments = []syncdomain.Attachment{toolAttachment(searchTool("old"), 0)} + writeVariation(t, root, local, true) + server := variation("LaunchDarkly") + api := &directAPI{variation: &server, tools: map[string]versionedTool{"search": {Tool: searchTool("new"), Version: 2}}} + + _, _, err := runPrompt(t, root, api, "--conflict", "local", "--yes", "--no-input") + + require.NoError(t, err) + assert.Zero(t, countRequests(api.requests, "PATCH", "/ai-tools/search"), "the push must not publish the old local copy") + assert.Equal(t, "new", *api.tools["search"].Description) + assert.Equal(t, "Local", api.variation.Name) + assert.Equal(t, []syncdomain.AttachmentRef{{Key: "search", Version: 2}}, api.variation.Tools) + assert.Contains(t, readLocalFile(t, root, "tools/search.json"), `"description": "new"`) +} + +// When a tool that a conflicted variation newly uses changed on both sides, +// the tool and the variation form one conflict with one choice. +func TestPromptConflictWithANewToolThatChangedOnBothSidesIsOneChoice(t *testing.T) { + root := initRepository(t) + baseline := variation("Baseline") + writeManifest(t, root, baseline) + trackTool(t, root, searchTool("old")) + local := variation("Local") + local.Tools = []syncdomain.AttachmentRef{{Key: "search"}} + local.Attachments = []syncdomain.Attachment{toolAttachment(searchTool("edited"), 0)} + writeVariation(t, root, local, true) + server := variation("LaunchDarkly") + api := &directAPI{variation: &server, tools: map[string]versionedTool{"search": {Tool: searchTool("new"), Version: 2}}} + + _, stderr, err := runPrompt(t, root, api, "--yes", "--no-input") + require.ErrorContains(t, err, "conflict resolution requires --conflict or --resolve without interactive input") + assert.Contains(t, stderr, "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.") + + _, _, err = runPrompt(t, root, api, "--conflict", "local", "--yes", "--no-input") + require.NoError(t, err) + assert.Equal(t, "edited", *api.tools["search"].Description) + assert.Equal(t, []syncdomain.AttachmentRef{{Key: "search", Version: 3}}, api.variation.Tools) +} + // A newly referenced tool whose two sides are equal cannot be overwritten, so // the plan has no row for it. func TestPromptNewToolReferenceWithEqualSidesPlansNoToolRow(t *testing.T) { diff --git a/internal/sync/prompt/conflict.go b/internal/sync/prompt/conflict.go index 83eede28..273fa0f5 100644 --- a/internal/sync/prompt/conflict.go +++ b/internal/sync/prompt/conflict.go @@ -200,8 +200,11 @@ func groupConflicts(plan Plan) []conflictGroup { for next := 0; next < len(members); next++ { for candidate := range plan.Resources { row := plan.Resources[candidate] + // A row that only conflicts need keeps its own action, unless + // it is a conflict itself. applyConflictResolutions decides it. if visited[candidate] || row.Action == ActionError || + (len(row.conflictUses) != 0 && row.Action != ActionConflict) || (row.ID.Kind != syncdomain.KindVariation && row.Action == ActionDeleteLocal) || !sharesChangedAttachment(plan.Resources[members[next]], row) { continue @@ -305,17 +308,34 @@ 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: slices.Clone(plan.Resources)} - for index := range resolved.Resources { - resource := &resolved.Resources[index] + resolved := Plan{Resources: make([]PlannedResource, 0, len(plan.Resources))} + for _, resource := range plan.Resources { + if !chosenSideUses(resource, resolutions) { + continue + } if resolution, ok := resolutions[resource.ID]; ok { - resource.Action = resolvedConflictAction(*resource, resolution) + resource.Action = resolvedConflictAction(resource, resolution) } + resolved.Resources = append(resolved.Resources, resource) } checkResolvedAttachmentDeletes(resolved.Resources) return resolved } +// chosenSideUses reports whether a row stays after the conflict choices. A +// row that only conflicts need stays when the side that the user chose for +// one of them uses its tool or skill. Then the row keeps its own action, so +// that the variation does not write its own copy over the other side. +func chosenSideUses(resource PlannedResource, resolutions map[ResourceID]ConflictResolution) bool { + if len(resource.conflictUses) == 0 || resource.Action == ActionConflict { + return true + } + return slices.ContainsFunc(resource.conflictUses, func(use conflictUse) bool { + resolution := resolutions[use.variation] + return use.local && resolution == ConflictUseLocal || !use.local && resolution == ConflictUseLaunchDarkly + }) +} + // resolvedConflictAction returns the action that makes the chosen side win. func resolvedConflictAction(resource PlannedResource, resolution ConflictResolution) Action { switch resolution { diff --git a/internal/sync/prompt/plan.go b/internal/sync/prompt/plan.go index eea0ebc8..11b8f518 100644 --- a/internal/sync/prompt/plan.go +++ b/internal/sync/prompt/plan.go @@ -73,6 +73,9 @@ type PlannedResource struct { // changedAttachments are the tools and skills whose content differs // between the local file and LaunchDarkly. changedAttachments []ResourceID + // 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 // restoreRef is the link that a restored variation file keeps. restoreRef *syncdomain.Reference } @@ -102,6 +105,13 @@ func (resource PlannedResource) writtenSides() (source, target *syncdomain.Varia } } +// conflictUse is a conflicted variation that newly uses a tool or skill on one +// side. local is true when that side is the local file. +type conflictUse struct { + variation ResourceID + local bool +} + // Plan is the sync decision for each resource, in identity order. type Plan struct { Resources []PlannedResource diff --git a/internal/sync/prompt/state.go b/internal/sync/prompt/state.go index 2263d679..9de01381 100644 --- a/internal/sync/prompt/state.go +++ b/internal/sync/prompt/state.go @@ -191,46 +191,68 @@ func planAttachmentRows( // variation row would write its own copy over the other side. The row decides // the content, and executePlan applies it before the variation rows. // checkAttachmentBaselines covers the tools and skills that both sides use. +// +// A conflicted variation writes the side that the user chooses, so each side +// of it counts. A row that only conflicts need records them in conflictUses, +// and applyConflictResolutions keeps it only for the chosen side. func planReferencedAttachments( repositoryRoot string, lock syncmanifest.Manifest, attachments *attachmentCache, resources []PlannedResource, ) ([]PlannedResource, error) { - referenced := make(map[ResourceID]bool) + written := make(map[ResourceID]bool) + conflictUses := make(map[ResourceID][]conflictUse) for _, resource := range resources { for _, id := range newReferences(resource) { - referenced[id] = true + written[id] = true + } + if resource.ID.Kind == syncdomain.KindVariation && resource.Action == ActionConflict { + for _, id := range newAttachments(resource.ID.ProjectKey, resource.Local, resource.Server) { + conflictUses[id] = append(conflictUses[id], conflictUse{variation: resource.ID, local: true}) + } + for _, id := range newAttachments(resource.ID.ProjectKey, resource.Server, resource.Local) { + conflictUses[id] = append(conflictUses[id], conflictUse{variation: resource.ID}) + } } } - for _, resource := range resources { - delete(referenced, resource.ID) - } - if len(referenced) == 0 { - return nil, nil + referenced := func(id ResourceID) bool { + return (written[id] || len(conflictUses[id]) != 0) && + !slices.ContainsFunc(resources, func(resource PlannedResource) bool { return resource.ID == id }) } - rows, err := planAttachmentRows(repositoryRoot, lock, attachments, func(id ResourceID) bool { return referenced[id] }) + rows, err := planAttachmentRows(repositoryRoot, lock, attachments, referenced) if err != nil { return nil, err } - return slices.DeleteFunc(rows, func(row PlannedResource) bool { + rows = slices.DeleteFunc(rows, func(row PlannedResource) bool { return !row.hasLocal() || !row.hasServer() || row.LocalFingerprint == row.ServerFingerprint - }), nil + }) + for index := range rows { + if !written[rows[index].ID] { + rows[index].conflictUses = conflictUses[rows[index].ID] + } + } + return rows, nil } -// newReferences returns each tool and skill that a variation row writes to a -// side that does not use it yet. A conflict has none, because it writes no -// side until the user chooses one, and that choice covers only the variation. +// newReferences returns each tool and skill that a push or a pull writes to a +// side that does not use it yet. func newReferences(resource PlannedResource) []ResourceID { source, target := resource.writtenSides() + return newAttachments(resource.ID.ProjectKey, source, target) +} + +// newAttachments returns each tool and skill that source uses and target does +// not. +func newAttachments(projectKey string, source, target *syncdomain.Variation) []ResourceID { if source == nil { return nil } var ids []ResourceID for _, attachment := range source.Attachments { if target == nil || !hasAttachment(*target, attachment) { - ids = append(ids, attachment.ID(resource.ID.ProjectKey)) + ids = append(ids, attachment.ID(projectKey)) } } return ids From cf6adb57836fd12869244cea39ebb04e94357ecd Mon Sep 17 00:00:00 2001 From: Cliff Tawiah Date: Fri, 23 Oct 2026 14:42:20 +0000 Subject: [PATCH 4/4] test: say that the dry run changes nothing in the new reference tests The push and pull tests of a new tool reference start with a dry run. Their comments now say so, and each test checks after the dry run that it sent only reads and left the tool file as it was. --- internal/sync/prompt/acceptance_test.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/internal/sync/prompt/acceptance_test.go b/internal/sync/prompt/acceptance_test.go index c64d4a69..ee26b09e 100644 --- a/internal/sync/prompt/acceptance_test.go +++ b/internal/sync/prompt/acceptance_test.go @@ -683,8 +683,9 @@ func TestPromptUpdateResolvesOmittedModelConfigVersionToLatest(t *testing.T) { } // A push of a variation that newly attaches a tracked tool must not publish -// the local copy over a change in LaunchDarkly. The plan pulls the tool, and -// the variation pins the pulled version. +// the local copy over a change in LaunchDarkly. A dry run plans a pull of the +// tool and changes nothing. The sync then pulls the tool, and the variation +// pins the pulled version. func TestPromptPushOfANewToolReferenceKeepsTheLaunchDarklyChange(t *testing.T) { root := initRepository(t) baseline := variation("Baseline") @@ -699,6 +700,8 @@ func TestPromptPushOfANewToolReferenceKeepsTheLaunchDarklyChange(t *testing.T) { stdout, _, err := runPrompt(t, root, api, "--dry-run", "--output", "json") require.NoError(t, err) assert.Equal(t, []string{"tool search update_local", "variation support/default update_server"}, planRows(t, stdout)) + requireOnlyReads(t, api.requests) + assert.Contains(t, readLocalFile(t, root, "tools/search.json"), `"description": "old"`) _, _, err = runPrompt(t, root, api, "--yes") @@ -710,8 +713,9 @@ func TestPromptPushOfANewToolReferenceKeepsTheLaunchDarklyChange(t *testing.T) { } // A pull of a variation that newly references a tracked tool must not write -// the LaunchDarkly copy over a local edit. The plan pushes the tool, and the -// file keeps the edit. The next sync only repins the variation. +// the LaunchDarkly copy over a local edit. A dry run plans a push of the tool +// and changes nothing. The sync then pushes the tool, and the file keeps the +// edit. The next sync only repins the variation. func TestPromptPullOfANewToolReferenceKeepsTheLocalEdit(t *testing.T) { root := initRepository(t) baseline := variation("Baseline") @@ -727,6 +731,8 @@ func TestPromptPullOfANewToolReferenceKeepsTheLocalEdit(t *testing.T) { stdout, _, err := runPrompt(t, root, api, "--dry-run", "--output", "json") require.NoError(t, err) assert.Equal(t, []string{"tool search update_server", "variation support/default update_local"}, planRows(t, stdout)) + requireOnlyReads(t, api.requests) + assert.Contains(t, readLocalFile(t, root, "tools/search.json"), `"description": "edited"`) _, _, err = runPrompt(t, root, api, "--yes")