diff --git a/internal/sync/attachment.go b/internal/sync/attachment.go index 5f6d62ff1..e65234010 100644 --- a/internal/sync/attachment.go +++ b/internal/sync/attachment.go @@ -101,6 +101,20 @@ type SyncedAttachment struct { Projection Projection } +// Fingerprint returns the fingerprint that sync.lock records for the tool or +// the skill. A descriptor-backed source records the fingerprint of its +// projection. A managed file records the fingerprint of the full content. +func (attachment SyncedAttachment) Fingerprint() (string, error) { + if attachment.Source == nil { + return FingerprintAttachment(attachment.ProjectKey, attachment.Attachment) + } + projection, err := ProjectAttachment(attachment.Attachment, *attachment.Source) + if err != nil { + return "", err + } + return FingerprintProjection(attachment.ID(), projection) +} + // ID returns the identity of the tool or the skill. func (attachment SyncedAttachment) ID() ResourceID { return attachment.Attachment.ID(attachment.ProjectKey) diff --git a/internal/sync/detach/detach.go b/internal/sync/detach/detach.go index 8ee4aa2c6..a7a874cc9 100644 --- a/internal/sync/detach/detach.go +++ b/internal/sync/detach/detach.go @@ -8,7 +8,10 @@ import ( "errors" "fmt" "io" + "os" + "path/filepath" "slices" + "strings" syncdomain "github.com/launchdarkly/ldcli/internal/sync" syncapi "github.com/launchdarkly/ldcli/internal/sync/api" @@ -200,10 +203,10 @@ func confirmDetach( } else { _ = console.Line("Resources to detach:") } - changed := localChanges(options.RepositoryRoot, lock, variations, selected) + notes := localChanges(options.RepositoryRoot, lock, variations, selected) for _, id := range selected { - if changed[id] { - _ = console.Printf(" %s (local changes will be lost)\n", id.Label()) + if note := notes[id]; note != "" { + _ = console.Printf(" %s (%s)\n", id.Label(), note) } else { _ = console.Printf(" %s\n", id.Label()) } @@ -217,44 +220,90 @@ func confirmDetach( return confirmed, err } -// localChanges reports each selected resource whose local file differs from -// its sync.lock entry, because the detach deletes that file. A file that sync -// cannot read counts as changed. A resource without a local file loses -// nothing. +// The notes that the detach review shows next to a resource. +const ( + // noteLocalChangesLost marks a managed file whose content differs from + // sync.lock. The detach deletes the file. + noteLocalChangesLost = "local changes will be lost" + // noteUnsyncedChanges marks a linked resource whose mapping, mapped + // source values, or relationships differ from sync.lock. The detach + // deletes the descriptor with its relationships, and keeps the source. + noteUnsyncedChanges = "unsynced changes" +) + +// localChanges returns a note for each selected resource whose local state +// differs from its sync.lock entry. A file that sync cannot read counts as +// changed. A resource without a local file loses nothing. func localChanges( repositoryRoot string, lock syncmanifest.Manifest, variations []syncdomain.SyncedResource, selected []syncdomain.ResourceID, -) map[syncdomain.ResourceID]bool { - local := make(map[syncdomain.ResourceID]string) - for _, variation := range variations { - if fingerprint, err := syncdomain.FingerprintVariation(variation.ProjectKey, variation.LookupKey, variation.Variation); err == nil { - local[variation.ID()] = fingerprint +) map[syncdomain.ResourceID]string { + baseline := make(map[syncdomain.ResourceID]syncmanifest.Resource, len(lock.Resources)) + for _, resource := range lock.Resources { + baseline[resource.ID()] = resource + } + // note returns the note of a resource, or an empty string when its lock + // entry still describes it. A linked resource compares its mapping and + // the fingerprint of its projection, which has the relationships. + note := func(id syncdomain.ResourceID, source *syncdomain.Reference, fingerprint func() (string, error)) string { + actual, err := fingerprint() + if source == nil { + if err == nil && actual == baseline[id].Fingerprint { + return "" + } + return noteLocalChangesLost } + digest, digestErr := syncdomain.ReferenceDigest(*source) + if err == nil && digestErr == nil && digest == baseline[id].ReferenceDigest && actual == baseline[id].Fingerprint { + return "" + } + return noteUnsyncedChanges + } + + notes := make(map[syncdomain.ResourceID]string) + for _, variation := range variations { + notes[variation.ID()] = note(variation.ID(), variation.Source, func() (string, error) { + if variation.Source == nil { + return syncdomain.FingerprintVariation(variation.ProjectKey, variation.LookupKey, variation.Variation) + } + projection, err := syncdomain.ProjectVariation(variation.Variation, *variation.Source) + if err != nil { + return "", err + } + return syncdomain.FingerprintProjection(variation.ID(), projection) + }) } isSelected := func(id syncdomain.ResourceID) bool { return slices.Contains(selected, id) } attachments, _ := synclocal.CompileAttachments(repositoryRoot, isSelected) for _, attachment := range attachments { - if fingerprint, err := syncdomain.FingerprintAttachment(attachment.ProjectKey, attachment.Attachment); err == nil { - local[attachment.ID()] = fingerprint - } - } - baseline := make(map[syncdomain.ResourceID]string, len(lock.Resources)) - for _, resource := range lock.Resources { - baseline[resource.ID()] = resource.Fingerprint + notes[attachment.ID()] = note(attachment.ID(), attachment.Source, attachment.Fingerprint) } files, _ := synclocal.SourceFiles(repositoryRoot) - changed := make(map[syncdomain.ResourceID]bool) + result := make(map[syncdomain.ResourceID]string) for _, file := range files { id, ok := synclocal.ParseManagedPath(file) - if ok && isSelected(id) { - fingerprint, readable := local[id] - changed[id] = !readable || fingerprint != baseline[id] + if !ok { + id, ok = synclocal.ParseReferencePath(file) + } + if !ok || !isSelected(id) { + continue + } + if link, err := leftoverLink(repositoryRoot, file); err != nil || link { + // A symbolic link at a managed path is not a definition, so a + // detach loses no content there. + continue + } + result[id] = noteLocalChangesLost + if _, readable := notes[id]; readable { + result[id] = notes[id] + } else if strings.HasSuffix(file, ".ref.yaml") { + result[id] = noteUnsyncedChanges } } - return changed + return result } // archiveVariations archives each variation that LaunchDarkly still has. It @@ -325,7 +374,20 @@ func loadResources( synced = append(synced, resource.ID()) } for _, file := range files { - if id, ok := synclocal.ParseManagedPath(file); ok { + id, ok := synclocal.ParseManagedPath(file) + if !ok { + id, ok = synclocal.ParseReferencePath(file) + } + if !ok { + continue + } + // A symbolic link at a managed path is not a definition, so it is + // not a synced resource to detach. + link, err := leftoverLink(repositoryRoot, file) + if err != nil { + return nil, syncmanifest.Baseline{}, err + } + if !link { synced = append(synced, id) } } @@ -352,16 +414,15 @@ func detachResources(options Options, original syncmanifest.Baseline, selected [ return errors.Join(cause, restoreErr) } - deletions, err := localAttachments(options.RepositoryRoot, selected) + deletions, err := localDefinitions(options.RepositoryRoot, selected) if err != nil { return restoreBaseline(err) } for _, id := range selected { - configKey, variationKey, err := id.VariationKeys() - if err != nil { + if slices.Contains(deletions, id) { continue } - exists, err := options.Store.VariationExists(id.ProjectKey, configKey, variationKey) + exists, err := options.Store.ResourceExists(id) if err != nil { return restoreBaseline(err) } @@ -369,23 +430,43 @@ func detachResources(options Options, original syncmanifest.Baseline, selected [ deletions = append(deletions, id) } } - if _, err := options.Store.DeleteResources(deletions); err != nil { + if _, err := options.Store.DeleteDefinitions(deletions); err != nil { return restoreBaseline(err) } return nil } -// localAttachments returns each selected tool and skill that has a local file. -func localAttachments(repositoryRoot string, selected []syncdomain.ResourceID) ([]syncdomain.ResourceID, error) { +// leftoverLink reports whether a repository file is a symbolic link. +func leftoverLink(repositoryRoot, file string) (bool, error) { + info, err := os.Lstat(filepath.Join(repositoryRoot, filepath.FromSlash(file))) + if err != nil { + return false, fmt.Errorf("inspect %s: %w", file, err) + } + return info.Mode()&os.ModeSymlink != 0, nil +} + +// localDefinitions returns each selected resource that has a managed file, +// a descriptor, or a leftover link at its managed path. +func localDefinitions(repositoryRoot string, selected []syncdomain.ResourceID) ([]syncdomain.ResourceID, error) { files, err := synclocal.SourceFiles(repositoryRoot) if err != nil { return nil, err } - var attachments []syncdomain.ResourceID + var definitions []syncdomain.ResourceID for _, file := range files { - if id, ok := synclocal.ParseManagedPath(file); ok && id.Kind != syncdomain.KindVariation && slices.Contains(selected, id) { - attachments = append(attachments, id) + id, ok := synclocal.ParseManagedPath(file) + if !ok { + id, ok = synclocal.ParseReferencePath(file) } - } - return attachments, nil + if !ok || !slices.Contains(selected, id) { + continue + } + // A symbolic link at a managed path is not a definition, but sync + // owns the managed directory and compile rejects the link, so the + // detach deletes it with the resource. The delete removes the link + // itself, not its target. + definitions = append(definitions, id) + } + slices.SortFunc(definitions, syncdomain.CompareResourceIDs) + return slices.Compact(definitions), nil } diff --git a/internal/sync/detach/detach_test.go b/internal/sync/detach/detach_test.go index 928fdb7f2..b98b3b3b6 100644 --- a/internal/sync/detach/detach_test.go +++ b/internal/sync/detach/detach_test.go @@ -97,6 +97,46 @@ func TestDetachResourcesKeepsAttachmentManifestEntries(t *testing.T) { require.NoError(t, err, "the tool file stays") } +func TestDetachResourcesRemovesDescriptorAndManifestButKeepsSource(t *testing.T) { + root := t.TempDir() + referencePath := filepath.Join(root, "prompts", "prompt.md") + require.NoError(t, os.MkdirAll(filepath.Dir(referencePath), 0o755)) + require.NoError(t, os.WriteFile(referencePath, []byte("Keep me.\n"), 0o644)) + + store := synclocal.NewStore(root) + resource := syncdomain.VariationID("project", "config", "prompt") + _, err := store.WriteDescriptor(resource, synclocal.Reference{ + File: "prompts/prompt.md", Format: "markdown", + }) + require.NoError(t, err) + + manifestStore := newMemoryManifestStore() + original := syncmanifest.Manifest{ + Resources: []syncmanifest.Resource{{ + ResourceKind: syncdomain.KindVariation, ProjectKey: "project", LookupKey: "config/prompt", + Fingerprint: testFingerprint(), + }}, + } + require.NoError(t, manifestStore.Write(original)) + + err = detachResources( + Options{RepositoryRoot: root, Store: store, Baselines: manifestStore}, + syncmanifest.Baseline{Lock: original}, + []syncdomain.ResourceID{resource}, + ) + + require.NoError(t, err) + exists, err := store.VariationExists("project", "config", "prompt") + require.NoError(t, err) + assert.False(t, exists) + _, err = os.Stat(referencePath) + require.NoError(t, err) + baseline, err := manifestStore.Load([]string{"project"}) + manifest := baseline.Lock + require.NoError(t, err) + assert.Empty(t, manifest.Resources) +} + func TestDetachResourcesRemovesManifestEntryWhenWrapperWasAlreadyDeleted(t *testing.T) { root := t.TempDir() store := synclocal.NewStore(root) @@ -123,6 +163,80 @@ func TestDetachResourcesRemovesManifestEntryWhenWrapperWasAlreadyDeleted(t *test assert.Empty(t, manifest.Resources) } +func TestDetachResourcesRemovesManifestEntryAndDeletesALeftoverLink(t *testing.T) { + root := t.TempDir() + store := synclocal.NewStore(root) + external := filepath.Join(root, "external.md") + require.NoError(t, os.WriteFile(external, []byte("---\nformatVersion: 1\nmode: agent\nkey: linked\n---\n\nOther.\n"), 0o644)) + resource := syncdomain.VariationID("project", "config", "linked") + managedPath, err := synclocal.ResourcePath(resource) + require.NoError(t, err) + target := filepath.Join(root, filepath.FromSlash(managedPath)) + require.NoError(t, os.MkdirAll(filepath.Dir(target), 0o755)) + require.NoError(t, os.Symlink(external, target)) + manifestStore := newMemoryManifestStore() + original := syncmanifest.Manifest{ + Resources: []syncmanifest.Resource{{ + ResourceKind: syncdomain.KindVariation, ProjectKey: "project", LookupKey: "config/linked", + Fingerprint: testFingerprint(), + }}, + } + require.NoError(t, manifestStore.Write(original)) + + err = detachResources( + Options{RepositoryRoot: root, Store: store, Baselines: manifestStore}, + syncmanifest.Baseline{Lock: original}, + []syncdomain.ResourceID{resource}, + ) + + require.NoError(t, err) + baseline, err := manifestStore.Load([]string{"project"}) + require.NoError(t, err) + assert.Empty(t, baseline.Lock.Resources) + _, err = os.Lstat(target) + assert.ErrorIs(t, err, os.ErrNotExist, "the detach deletes the leftover link, which compile would reject") + assert.FileExists(t, external, "the target of the link stays") +} + +func TestLocalChangesIgnoreALeftoverLink(t *testing.T) { + root := t.TempDir() + external := filepath.Join(root, "external.md") + require.NoError(t, os.WriteFile(external, []byte("Other.\n"), 0o644)) + linked := syncdomain.VariationID("project", "config", "linked") + managedPath, err := synclocal.ResourcePath(linked) + require.NoError(t, err) + target := filepath.Join(root, filepath.FromSlash(managedPath)) + require.NoError(t, os.MkdirAll(filepath.Dir(target), 0o755)) + require.NoError(t, os.Symlink(external, target)) + edited := syncdomain.VariationID("project", "config", "edited") + editedPath, err := synclocal.ResourcePath(edited) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(root, filepath.FromSlash(editedPath)), []byte("---\nformatVersion: 1\nmode: agent\nkey: edited\n---\n\nEdited.\n"), 0o644)) + + notes := localChanges(root, syncmanifest.Manifest{}, nil, []syncdomain.ResourceID{linked, edited}) + + assert.Equal(t, map[syncdomain.ResourceID]string{edited: noteLocalChangesLost}, notes, "a link is not a definition, so no content is lost there") +} + +func TestLoadResourcesSkipsALeftoverLink(t *testing.T) { + root := t.TempDir() + external := filepath.Join(root, "external.md") + require.NoError(t, os.WriteFile(external, []byte("Other.\n"), 0o644)) + linked := syncdomain.VariationID("project", "config", "linked") + managedPath, err := synclocal.ResourcePath(linked) + require.NoError(t, err) + target := filepath.Join(root, filepath.FromSlash(managedPath)) + require.NoError(t, os.MkdirAll(filepath.Dir(target), 0o755)) + require.NoError(t, os.Symlink(external, target)) + manifestStore := newMemoryManifestStore() + require.NoError(t, manifestStore.Write(syncmanifest.Manifest{})) + + synced, _, err := loadResources(root, manifestStore, []string{"project"}) + + require.NoError(t, err) + assert.Empty(t, synced, "a link is not a definition, so it is not a synced resource") +} + func TestDetachResourcesDeletesUnreadableWrapper(t *testing.T) { root := t.TempDir() wrapper := filepath.Join(root, syncdomain.RootDir, "project", "configs", "config", "broken.prompt.md") @@ -687,3 +801,51 @@ func testVariation(key string) syncdomain.Variation { func testFingerprint() string { return "sha256:" + strings.Repeat("0", 64) } + +// A detach deletes the descriptor of a linked resource, with its mapping and +// its relationships, and keeps the source. A descriptor that sync.lock does +// not describe has unsynced changes. +func TestRunMarksALinkedResourceWithUnsyncedChanges(t *testing.T) { + workspace := newArchiveWorkspace(t) + require.NoError(t, os.WriteFile(filepath.Join(workspace.root, "prompt.md"), []byte("Edited locally.\n"), 0o644)) + linked := syncdomain.VariationID("project", "config", "linked") + reference := synclocal.Reference{File: "prompt.md", Format: "markdown"} + _, err := workspace.store.WriteDescriptor(linked, reference) + require.NoError(t, err) + // A sync records the digest of the compiled descriptor and the + // fingerprint of its projection. + compiled, err := synclocal.CompileWorkspace(workspace.root) + require.NoError(t, err) + require.Len(t, compiled, 1) + projection, err := syncdomain.ProjectVariation(compiled[0].Variation, *compiled[0].Source) + require.NoError(t, err) + fingerprint, err := syncdomain.FingerprintProjection(linked, projection) + require.NoError(t, err) + workspace.baselines.manifest.SetFingerprint(linked, fingerprint) + require.NoError(t, workspace.baselines.manifest.SetSource(linked, compiled[0].Source, "markdown/1", nil)) + + run := func() string { + var output bytes.Buffer + options := workspace.options(&recordingArchiver{}) + options.Selections = []syncdomain.ResourceID{linked} + options.NoInput, options.Input, options.Output = false, strings.NewReader("n\n"), &output + options.isTerminal = func(io.Reader, io.Writer) bool { return true } + require.NoError(t, Run(options)) + return output.String() + } + + assert.Contains(t, run(), " variation project/config/linked\n", "the lock describes the descriptor") + + // A relationship edit lives in the descriptor that the detach deletes. + reference.Tools = &[]syncdomain.AttachmentRef{{Key: "search"}} + _, err = workspace.store.WriteDescriptor(linked, reference) + require.NoError(t, err) + assert.Contains(t, run(), " variation project/config/linked (unsynced changes)\n", "the relationship is not recorded") + + reference.Tools = nil + require.NoError(t, os.WriteFile(filepath.Join(workspace.root, "other.md"), []byte("Other source.\n"), 0o644)) + reference.File = "other.md" + _, err = workspace.store.WriteDescriptor(linked, reference) + require.NoError(t, err) + assert.Contains(t, run(), " variation project/config/linked (unsynced changes)\n", "the mapping is not recorded") +}