From 58e107ccf4858b5c55f30bd73d76a10413fb998c Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Wed, 13 May 2026 16:27:59 -0700 Subject: [PATCH] feat: add two-way manifest sync between project and app settings When the manifest-sync experiment is enabled, the CLI detects per-field differences between the local manifest.json and the remote app settings, lets the user resolve them interactively, and writes the merged result to both sides. This replaces the binary "overwrite?" prompt when divergence is detected during install/deploy. Adds `slack manifest sync` command and `slack sync` alias. Gated behind --experiment=manifest-sync. --- cmd/manifest/manifest.go | 1 + cmd/manifest/sync.go | 45 +++++++ cmd/root.go | 1 + internal/experiment/experiment.go | 4 + internal/pkg/apps/install.go | 15 ++- internal/pkg/manifest/diff.go | 100 +++++++++++++++ internal/pkg/manifest/diff_test.go | 143 +++++++++++++++++++++ internal/pkg/manifest/display.go | 155 +++++++++++++++++++++++ internal/pkg/manifest/flatten.go | 50 ++++++++ internal/pkg/manifest/flatten_test.go | 107 ++++++++++++++++ internal/pkg/manifest/merge.go | 159 ++++++++++++++++++++++++ internal/pkg/manifest/merge_test.go | 125 +++++++++++++++++++ internal/pkg/manifest/sync.go | 110 ++++++++++++++++ internal/pkg/manifest/writeback.go | 151 ++++++++++++++++++++++ internal/pkg/manifest/writeback_test.go | 85 +++++++++++++ 15 files changed, 1248 insertions(+), 3 deletions(-) create mode 100644 cmd/manifest/sync.go create mode 100644 internal/pkg/manifest/diff.go create mode 100644 internal/pkg/manifest/diff_test.go create mode 100644 internal/pkg/manifest/display.go create mode 100644 internal/pkg/manifest/flatten.go create mode 100644 internal/pkg/manifest/flatten_test.go create mode 100644 internal/pkg/manifest/merge.go create mode 100644 internal/pkg/manifest/merge_test.go create mode 100644 internal/pkg/manifest/sync.go create mode 100644 internal/pkg/manifest/writeback.go create mode 100644 internal/pkg/manifest/writeback_test.go diff --git a/cmd/manifest/manifest.go b/cmd/manifest/manifest.go index 9f9d3a57..5fe13fec 100644 --- a/cmd/manifest/manifest.go +++ b/cmd/manifest/manifest.go @@ -79,6 +79,7 @@ func NewCommand(clients *shared.ClientFactory) *cobra.Command { // Add child commands cmd.AddCommand(NewInfoCommand(clients)) + cmd.AddCommand(NewSyncCommand(clients)) cmd.AddCommand(NewValidateCommand(clients)) cmd.Flags().StringVar( diff --git a/cmd/manifest/sync.go b/cmd/manifest/sync.go new file mode 100644 index 00000000..a522bfb8 --- /dev/null +++ b/cmd/manifest/sync.go @@ -0,0 +1,45 @@ +package manifest + +import ( + "github.com/opentracing/opentracing-go" + "github.com/slackapi/slack-cli/internal/app" + "github.com/slackapi/slack-cli/internal/cmdutil" + "github.com/slackapi/slack-cli/internal/pkg/manifest" + "github.com/slackapi/slack-cli/internal/prompts" + "github.com/slackapi/slack-cli/internal/shared" + "github.com/slackapi/slack-cli/internal/style" + "github.com/spf13/cobra" +) + +var manifestSyncFunc = manifest.Sync + +func NewSyncCommand(clients *shared.ClientFactory) *cobra.Command { + cmd := &cobra.Command{ + Use: "sync", + Short: "Sync the app manifest between project and app settings", + Long: "Compare the local project manifest with app settings, resolve differences, and sync both to the same state.", + Example: style.ExampleCommandsf([]style.ExampleCommand{ + {Command: "manifest sync", Meaning: "Sync project manifest with app settings"}, + }), + Args: cobra.NoArgs, + PreRunE: func(cmd *cobra.Command, args []string) error { + return cmdutil.IsValidProjectDirectory(clients) + }, + RunE: func(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() + span, ctx := opentracing.StartSpanFromContext(ctx, "cmd.manifest.sync") + defer span.Finish() + + selection, err := appSelectPromptFunc(ctx, clients, prompts.ShowAllEnvironments, prompts.ShowInstalledAppsOnly) + if err != nil { + return err + } + + clients.Config.ManifestEnv = app.SetManifestEnvTeamVars(clients.Config.ManifestEnv, selection.App.TeamDomain, selection.App.IsDev) + + _, err = manifestSyncFunc(ctx, clients, selection.App, selection.Auth) + return err + }, + } + return cmd +} diff --git a/cmd/root.go b/cmd/root.go index b638502b..af2a36f8 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -80,6 +80,7 @@ var AliasMap = map[string]*AliasInfo{ "logout": {CommandFactory: auth.NewLogoutCommand, CanonicalName: "auth logout", ParentName: "auth"}, "run": {CommandFactory: platform.NewRunCommand, CanonicalName: "platform run", ParentName: "platform"}, "samples": {CommandFactory: project.NewSamplesCommand, CanonicalName: "project samples", ParentName: "project"}, + "sync": {CommandFactory: manifest.NewSyncCommand, CanonicalName: "manifest sync", ParentName: "manifest"}, "uninstall": {CommandFactory: app.NewUninstallCommand, CanonicalName: "app uninstall", ParentName: "app"}, } var processName = cmdutil.GetProcessName() diff --git a/internal/experiment/experiment.go b/internal/experiment/experiment.go index 80c661f9..2cf1687e 100644 --- a/internal/experiment/experiment.go +++ b/internal/experiment/experiment.go @@ -33,6 +33,9 @@ const ( // Lipgloss experiment shows pretty styles. Lipgloss Experiment = "lipgloss" + // ManifestSync experiment enables two-way manifest sync between local and remote. + ManifestSync Experiment = "manifest-sync" + // Placeholder experiment is a placeholder for testing and does nothing... or does it? Placeholder Experiment = "placeholder" @@ -47,6 +50,7 @@ const ( // Please also add here 👇 var AllExperiments = []Experiment{ Lipgloss, + ManifestSync, Placeholder, Sandboxes, SetIcon, diff --git a/internal/pkg/apps/install.go b/internal/pkg/apps/install.go index 76c69324..e660b9d3 100644 --- a/internal/pkg/apps/install.go +++ b/internal/pkg/apps/install.go @@ -25,7 +25,7 @@ import ( "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/experiment" "github.com/slackapi/slack-cli/internal/icon" - "github.com/slackapi/slack-cli/internal/pkg/manifest" + manifestpkg "github.com/slackapi/slack-cli/internal/pkg/manifest" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackerror" @@ -273,11 +273,11 @@ func printNonSuccessInstallState(ctx context.Context, clients *shared.ClientFact func validateManifestForInstall(ctx context.Context, clients *shared.ClientFactory, token string, app types.App, appManifest types.AppManifest) error { validationResult, err := clients.API().ValidateAppManifest(ctx, token, appManifest, app.AppID) - if retryValidate := manifest.HandleConnectorNotInstalled(ctx, clients, token, err); retryValidate { + if retryValidate := manifestpkg.HandleConnectorNotInstalled(ctx, clients, token, err); retryValidate { validationResult, err = clients.API().ValidateAppManifest(ctx, token, appManifest, app.AppID) } - if err := manifest.HandleConnectorApprovalRequired(ctx, clients, token, err); err != nil { + if err := manifestpkg.HandleConnectorApprovalRequired(ctx, clients, token, err); err != nil { return err } @@ -764,6 +764,15 @@ func shouldUpdateManifest(ctx context.Context, clients *shared.ClientFactory, ap default: notice = style.Yellow("The manifest on app settings has been changed since last update") } + + if clients.Config.WithExperimentOn(experiment.ManifestSync) { + _, err := manifestpkg.Sync(ctx, clients, app, auth) + if err != nil { + return false, err + } + return false, nil + } + clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{ Emoji: "books", Text: "App Manifest", diff --git a/internal/pkg/manifest/diff.go b/internal/pkg/manifest/diff.go new file mode 100644 index 00000000..0922d091 --- /dev/null +++ b/internal/pkg/manifest/diff.go @@ -0,0 +1,100 @@ +package manifest + +import ( + "encoding/json" + "fmt" + + "github.com/slackapi/slack-cli/internal/shared/types" +) + +// DiffType describes how a field differs between local and remote. +type DiffType int + +const ( + DiffModified DiffType = iota // Both sides have the field but with different values + DiffLocalOnly // Field exists only in local (added locally or deleted remotely) + DiffRemoteOnly // Field exists only in remote (added remotely or deleted locally) +) + +// FieldDiff represents a single difference between local and remote manifests. +type FieldDiff struct { + Path string + Type DiffType + LocalValue any + RemoteValue any +} + +// DiffResult holds all differences found between two manifests. +type DiffResult struct { + Diffs []FieldDiff +} + +// HasDifferences returns true if any differences were found. +func (dr *DiffResult) HasDifferences() bool { + return len(dr.Diffs) > 0 +} + +// Diff performs a two-way comparison between local and remote manifests, +// returning all fields that differ between them. +func Diff(local, remote types.AppManifest) (*DiffResult, error) { + localFlat, err := Flatten(local) + if err != nil { + return nil, fmt.Errorf("failed to flatten local manifest: %w", err) + } + remoteFlat, err := Flatten(remote) + if err != nil { + return nil, fmt.Errorf("failed to flatten remote manifest: %w", err) + } + return diffFlat(localFlat, remoteFlat), nil +} + +func diffFlat(local, remote map[string]any) *DiffResult { + result := &DiffResult{} + seen := make(map[string]bool) + + for path, localVal := range local { + seen[path] = true + remoteVal, exists := remote[path] + if !exists { + result.Diffs = append(result.Diffs, FieldDiff{ + Path: path, + Type: DiffLocalOnly, + LocalValue: localVal, + }) + continue + } + if !valuesEqual(localVal, remoteVal) { + result.Diffs = append(result.Diffs, FieldDiff{ + Path: path, + Type: DiffModified, + LocalValue: localVal, + RemoteValue: remoteVal, + }) + } + } + + for path, remoteVal := range remote { + if seen[path] { + continue + } + result.Diffs = append(result.Diffs, FieldDiff{ + Path: path, + Type: DiffRemoteOnly, + RemoteValue: remoteVal, + }) + } + + return result +} + +func valuesEqual(a, b any) bool { + aJSON, err := json.Marshal(a) + if err != nil { + return false + } + bJSON, err := json.Marshal(b) + if err != nil { + return false + } + return string(aJSON) == string(bJSON) +} diff --git a/internal/pkg/manifest/diff_test.go b/internal/pkg/manifest/diff_test.go new file mode 100644 index 00000000..a661c083 --- /dev/null +++ b/internal/pkg/manifest/diff_test.go @@ -0,0 +1,143 @@ +package manifest + +import ( + "testing" + + "github.com/slackapi/slack-cli/internal/shared/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_Diff(t *testing.T) { + tests := map[string]struct { + local types.AppManifest + remote types.AppManifest + expected []FieldDiff + }{ + "identical manifests produce no diffs": { + local: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App"}, + }, + remote: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App"}, + }, + expected: nil, + }, + "modified field detected": { + local: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App", Description: "Local desc"}, + }, + remote: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App", Description: "Remote desc"}, + }, + expected: []FieldDiff{ + {Path: "display_information.description", Type: DiffModified, LocalValue: "Local desc", RemoteValue: "Remote desc"}, + }, + }, + "local-only field detected": { + local: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App", Description: "Has desc"}, + }, + remote: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App"}, + }, + expected: []FieldDiff{ + {Path: "display_information.description", Type: DiffLocalOnly, LocalValue: "Has desc"}, + }, + }, + "remote-only field detected": { + local: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App"}, + }, + remote: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App", Description: "Remote only"}, + }, + expected: []FieldDiff{ + {Path: "display_information.description", Type: DiffRemoteOnly, RemoteValue: "Remote only"}, + }, + }, + "function added locally": { + local: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App"}, + Functions: map[string]types.ManifestFunction{ + "greet": {Title: "Greet", Description: "Hello"}, + }, + }, + remote: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App"}, + }, + expected: []FieldDiff{ + {Path: "functions.greet.description", Type: DiffLocalOnly, LocalValue: "Hello"}, + {Path: "functions.greet.title", Type: DiffLocalOnly, LocalValue: "Greet"}, + }, + }, + "array values compared as wholes": { + local: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App"}, + OAuthConfig: &types.OAuthConfig{ + Scopes: &types.ManifestScopes{ + Bot: []string{"chat:write", "users:read"}, + }, + }, + }, + remote: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App"}, + OAuthConfig: &types.OAuthConfig{ + Scopes: &types.ManifestScopes{ + Bot: []string{"chat:write", "files:read"}, + }, + }, + }, + expected: []FieldDiff{ + { + Path: "oauth_config.scopes.bot", + Type: DiffModified, + LocalValue: []any{"chat:write", "users:read"}, + RemoteValue: []any{"chat:write", "files:read"}, + }, + }, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + result, err := Diff(tc.local, tc.remote) + require.NoError(t, err) + if tc.expected == nil { + assert.False(t, result.HasDifferences()) + return + } + assert.True(t, result.HasDifferences()) + for _, expectedDiff := range tc.expected { + found := false + for _, actualDiff := range result.Diffs { + if actualDiff.Path == expectedDiff.Path { + found = true + assert.Equal(t, expectedDiff.Type, actualDiff.Type, "diff type mismatch for path %s", expectedDiff.Path) + if expectedDiff.LocalValue != nil { + assert.Equal(t, expectedDiff.LocalValue, actualDiff.LocalValue, "local value mismatch for path %s", expectedDiff.Path) + } + if expectedDiff.RemoteValue != nil { + assert.Equal(t, expectedDiff.RemoteValue, actualDiff.RemoteValue, "remote value mismatch for path %s", expectedDiff.Path) + } + break + } + } + assert.True(t, found, "expected diff not found for path %s", expectedDiff.Path) + } + }) + } +} + +func Test_DiffResult_HasDifferences(t *testing.T) { + t.Run("empty result has no differences", func(t *testing.T) { + result := &DiffResult{} + assert.False(t, result.HasDifferences()) + }) + + t.Run("result with diffs has differences", func(t *testing.T) { + result := &DiffResult{ + Diffs: []FieldDiff{{Path: "test", Type: DiffModified}}, + } + assert.True(t, result.HasDifferences()) + }) +} diff --git a/internal/pkg/manifest/display.go b/internal/pkg/manifest/display.go new file mode 100644 index 00000000..2a4d854a --- /dev/null +++ b/internal/pkg/manifest/display.go @@ -0,0 +1,155 @@ +package manifest + +import ( + "context" + "encoding/json" + "fmt" + "sort" + + "github.com/slackapi/slack-cli/internal/iostreams" + "github.com/slackapi/slack-cli/internal/style" +) + +// DisplayDiffs prints the differences to the terminal. +func DisplayDiffs(ctx context.Context, io iostreams.IOStreamer, diffs *DiffResult) { + if !diffs.HasDifferences() { + return + } + + sorted := make([]FieldDiff, len(diffs.Diffs)) + copy(sorted, diffs.Diffs) + sort.Slice(sorted, func(i, j int) bool { + return sorted[i].Path < sorted[j].Path + }) + + io.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{ + Emoji: "books", + Text: "App Manifest", + Secondary: []string{ + fmt.Sprintf("Found %d difference(s) between project and app settings", len(sorted)), + }, + })) + + for _, d := range sorted { + io.PrintInfo(ctx, false, "") + switch d.Type { + case DiffModified: + io.PrintInfo(ctx, false, " %s", style.Bold(d.Path)) + io.PrintInfo(ctx, false, " Project: %s", formatValue(d.LocalValue)) + io.PrintInfo(ctx, false, " App settings: %s", formatValue(d.RemoteValue)) + case DiffLocalOnly: + io.PrintInfo(ctx, false, " %s %s", style.Bold(d.Path), "(only in project)") + io.PrintInfo(ctx, false, " Value: %s", formatValue(d.LocalValue)) + case DiffRemoteOnly: + io.PrintInfo(ctx, false, " %s %s", style.Bold(d.Path), "(only in app settings)") + io.PrintInfo(ctx, false, " Value: %s", formatValue(d.RemoteValue)) + } + } + io.PrintInfo(ctx, false, "") +} + +// PromptResolutionStrategy asks the user how they want to resolve differences. +func PromptResolutionStrategy(ctx context.Context, io iostreams.IOStreamer) (MergeStrategy, error) { + options := []string{ + "Use all project values", + "Use all app settings values", + "Choose for each difference", + } + resp, err := io.SelectPrompt(ctx, "How would you like to resolve these differences?", options, iostreams.SelectPromptConfig{ + Required: true, + }) + if err != nil { + return 0, err + } + switch resp.Index { + case 0: + return MergeAllLocal, nil + case 1: + return MergeAllRemote, nil + default: + return MergePerField, nil + } +} + +// PromptFieldResolutions asks the user to resolve each difference individually. +func PromptFieldResolutions(ctx context.Context, io iostreams.IOStreamer, diffs *DiffResult) ([]FieldResolution, error) { + sorted := make([]FieldDiff, len(diffs.Diffs)) + copy(sorted, diffs.Diffs) + sort.Slice(sorted, func(i, j int) bool { + return sorted[i].Path < sorted[j].Path + }) + + resolutions := make([]FieldResolution, 0, len(sorted)) + for _, d := range sorted { + var options []string + switch d.Type { + case DiffModified: + options = []string{ + fmt.Sprintf("Use project: %s", formatValue(d.LocalValue)), + fmt.Sprintf("Use app settings: %s", formatValue(d.RemoteValue)), + } + case DiffLocalOnly: + options = []string{ + "Keep (include in merged manifest)", + "Remove (exclude from merged manifest)", + } + case DiffRemoteOnly: + options = []string{ + "Remove (exclude from merged manifest)", + "Keep (include in merged manifest)", + } + } + + resp, err := io.SelectPrompt(ctx, d.Path, options, iostreams.SelectPromptConfig{ + Required: true, + }) + if err != nil { + return nil, err + } + + var resolution Resolution + switch d.Type { + case DiffModified: + if resp.Index == 0 { + resolution = ResolveLocal + } else { + resolution = ResolveRemote + } + case DiffLocalOnly: + if resp.Index == 0 { + resolution = ResolveLocal + } else { + resolution = ResolveRemote + } + case DiffRemoteOnly: + if resp.Index == 0 { + resolution = ResolveLocal + } else { + resolution = ResolveRemote + } + } + + resolutions = append(resolutions, FieldResolution{Path: d.Path, Resolution: resolution}) + } + return resolutions, nil +} + +func formatValue(v any) string { + if v == nil { + return "(not present)" + } + switch val := v.(type) { + case string: + return fmt.Sprintf("%q", val) + default: + data, err := json.Marshal(val) + if err != nil { + return fmt.Sprintf("%v", val) + } + s := string(data) + if len(s) > 80 { + return s[:77] + "..." + } + return s + } +} diff --git a/internal/pkg/manifest/flatten.go b/internal/pkg/manifest/flatten.go new file mode 100644 index 00000000..8d706105 --- /dev/null +++ b/internal/pkg/manifest/flatten.go @@ -0,0 +1,50 @@ +package manifest + +import ( + "encoding/json" + "fmt" + "sort" +) + +// Flatten converts a manifest (as JSON-serializable struct) into a flat map +// where keys are dot-notation paths and values are the leaf values. +// Arrays are treated as leaf values (not recursed into individually) because +// array element identity is ambiguous without a key field. +func Flatten(manifest any) (map[string]any, error) { + data, err := json.Marshal(manifest) + if err != nil { + return nil, fmt.Errorf("failed to marshal manifest: %w", err) + } + var raw map[string]any + if err := json.Unmarshal(data, &raw); err != nil { + return nil, fmt.Errorf("failed to unmarshal manifest: %w", err) + } + result := make(map[string]any) + flattenRecursive("", raw, result) + return result, nil +} + +func flattenRecursive(prefix string, data map[string]any, result map[string]any) { + for key, value := range data { + fullKey := key + if prefix != "" { + fullKey = prefix + "." + key + } + switch v := value.(type) { + case map[string]any: + flattenRecursive(fullKey, v, result) + default: + result[fullKey] = value + } + } +} + +// SortedKeys returns the keys of a flat map in sorted order for deterministic output. +func SortedKeys(m map[string]any) []string { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + return keys +} diff --git a/internal/pkg/manifest/flatten_test.go b/internal/pkg/manifest/flatten_test.go new file mode 100644 index 00000000..d490d721 --- /dev/null +++ b/internal/pkg/manifest/flatten_test.go @@ -0,0 +1,107 @@ +package manifest + +import ( + "testing" + + "github.com/slackapi/slack-cli/internal/shared/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_Flatten(t *testing.T) { + tests := map[string]struct { + manifest types.AppManifest + expected map[string]any + }{ + "flattens display_information fields": { + manifest: types.AppManifest{ + DisplayInformation: types.DisplayInformation{ + Name: "My App", + Description: "A test app", + }, + }, + expected: map[string]any{ + "display_information.name": "My App", + "display_information.description": "A test app", + }, + }, + "flattens nested settings": { + manifest: types.AppManifest{ + DisplayInformation: types.DisplayInformation{ + Name: "App", + }, + Settings: &types.AppSettings{ + FunctionRuntime: types.LocallyRun, + }, + }, + expected: map[string]any{ + "display_information.name": "App", + "settings.function_runtime": "local", + }, + }, + "flattens functions map": { + manifest: types.AppManifest{ + DisplayInformation: types.DisplayInformation{ + Name: "App", + }, + Functions: map[string]types.ManifestFunction{ + "greet": { + Title: "Greet", + Description: "Greets a user", + }, + }, + }, + expected: map[string]any{ + "display_information.name": "App", + "functions.greet.title": "Greet", + "functions.greet.description": "Greets a user", + }, + }, + "treats arrays as leaf values": { + manifest: types.AppManifest{ + DisplayInformation: types.DisplayInformation{ + Name: "App", + }, + OAuthConfig: &types.OAuthConfig{ + Scopes: &types.ManifestScopes{ + Bot: []string{"chat:write", "channels:read"}, + }, + }, + }, + expected: map[string]any{ + "display_information.name": "App", + "oauth_config.scopes.bot": []any{"chat:write", "channels:read"}, + }, + }, + "empty manifest has only display_information.name": { + manifest: types.AppManifest{ + DisplayInformation: types.DisplayInformation{ + Name: "App", + }, + }, + expected: map[string]any{ + "display_information.name": "App", + }, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + result, err := Flatten(tc.manifest) + require.NoError(t, err) + for key, expectedVal := range tc.expected { + assert.Contains(t, result, key) + assert.Equal(t, expectedVal, result[key], "mismatch at key %s", key) + } + }) + } +} + +func Test_SortedKeys(t *testing.T) { + m := map[string]any{ + "z.field": "val", + "a.field": "val", + "m.field": "val", + } + keys := SortedKeys(m) + assert.Equal(t, []string{"a.field", "m.field", "z.field"}, keys) +} diff --git a/internal/pkg/manifest/merge.go b/internal/pkg/manifest/merge.go new file mode 100644 index 00000000..f17e73ac --- /dev/null +++ b/internal/pkg/manifest/merge.go @@ -0,0 +1,159 @@ +package manifest + +import ( + "encoding/json" + "fmt" + "maps" + + "github.com/slackapi/slack-cli/internal/shared/types" +) + +// Resolution represents which side the user chose for a given field. +type Resolution int + +const ( + ResolveLocal Resolution = iota // Use the local value + ResolveRemote // Use the remote value +) + +// MergeStrategy determines how all differences are resolved. +type MergeStrategy int + +const ( + MergeAllLocal MergeStrategy = iota // Use all local values + MergeAllRemote // Use all remote values + MergePerField // Resolve each field individually +) + +// FieldResolution pairs a diff path with the user's choice. +type FieldResolution struct { + Path string + Resolution Resolution +} + +// Merge applies resolutions to produce a final manifest. It starts with the +// remote manifest as a base, then applies local values for fields resolved as +// local and keeps remote values for fields resolved as remote. +func Merge(local, remote types.AppManifest, resolutions []FieldResolution) (types.AppManifest, error) { + localFlat, err := Flatten(local) + if err != nil { + return types.AppManifest{}, fmt.Errorf("failed to flatten local manifest: %w", err) + } + remoteFlat, err := Flatten(remote) + if err != nil { + return types.AppManifest{}, fmt.Errorf("failed to flatten remote manifest: %w", err) + } + + merged := make(map[string]any) + maps.Copy(merged, remoteFlat) + + // Apply resolutions + resolutionMap := make(map[string]Resolution) + for _, r := range resolutions { + resolutionMap[r.Path] = r.Resolution + } + + for path, resolution := range resolutionMap { + switch resolution { + case ResolveLocal: + if val, exists := localFlat[path]; exists { + merged[path] = val + } else { + delete(merged, path) + } + case ResolveRemote: + if val, exists := remoteFlat[path]; exists { + merged[path] = val + } else { + delete(merged, path) + } + } + } + + // Also include local-only paths that were resolved as local + for path, val := range localFlat { + if _, inRemote := remoteFlat[path]; !inRemote { + if res, hasResolution := resolutionMap[path]; hasResolution && res == ResolveLocal { + merged[path] = val + } + } + } + + return unflatten(merged) +} + +// MergeAllFrom resolves all differences from one side. +func MergeAllFrom(local, remote types.AppManifest, diffs *DiffResult, strategy MergeStrategy) (types.AppManifest, error) { + resolutions := make([]FieldResolution, 0, len(diffs.Diffs)) + for _, d := range diffs.Diffs { + var res Resolution + switch strategy { + case MergeAllLocal: + res = ResolveLocal + case MergeAllRemote: + res = ResolveRemote + } + resolutions = append(resolutions, FieldResolution{Path: d.Path, Resolution: res}) + } + return Merge(local, remote, resolutions) +} + +// unflatten converts a flat dot-notation map back into a nested structure, +// then marshals/unmarshals into AppManifest. +func unflatten(flat map[string]any) (types.AppManifest, error) { + nested := unflattenToMap(flat) + data, err := json.Marshal(nested) + if err != nil { + return types.AppManifest{}, fmt.Errorf("failed to marshal merged manifest: %w", err) + } + var result types.AppManifest + if err := json.Unmarshal(data, &result); err != nil { + return types.AppManifest{}, fmt.Errorf("failed to unmarshal merged manifest: %w", err) + } + return result, nil +} + +// unflattenToMap reconstructs a nested map from dot-notation paths. +func unflattenToMap(flat map[string]any) map[string]any { + result := make(map[string]any) + for path, value := range flat { + setNestedValue(result, path, value) + } + return result +} + +func setNestedValue(m map[string]any, path string, value any) { + parts := splitPath(path) + current := m + for i, part := range parts { + if i == len(parts)-1 { + current[part] = value + return + } + next, exists := current[part] + if !exists { + next = make(map[string]any) + current[part] = next + } + current = next.(map[string]any) + } +} + +func splitPath(path string) []string { + var parts []string + current := "" + for _, ch := range path { + if ch == '.' { + if current != "" { + parts = append(parts, current) + current = "" + } + } else { + current += string(ch) + } + } + if current != "" { + parts = append(parts, current) + } + return parts +} diff --git a/internal/pkg/manifest/merge_test.go b/internal/pkg/manifest/merge_test.go new file mode 100644 index 00000000..39b020d0 --- /dev/null +++ b/internal/pkg/manifest/merge_test.go @@ -0,0 +1,125 @@ +package manifest + +import ( + "testing" + + "github.com/slackapi/slack-cli/internal/shared/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_Merge(t *testing.T) { + tests := map[string]struct { + local types.AppManifest + remote types.AppManifest + resolutions []FieldResolution + expected types.AppManifest + }{ + "resolve all local keeps local values": { + local: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "Local App", Description: "Local desc"}, + }, + remote: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "Remote App", Description: "Remote desc"}, + }, + resolutions: []FieldResolution{ + {Path: "display_information.name", Resolution: ResolveLocal}, + {Path: "display_information.description", Resolution: ResolveLocal}, + }, + expected: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "Local App", Description: "Local desc"}, + }, + }, + "resolve all remote keeps remote values": { + local: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "Local App", Description: "Local desc"}, + }, + remote: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "Remote App", Description: "Remote desc"}, + }, + resolutions: []FieldResolution{ + {Path: "display_information.name", Resolution: ResolveRemote}, + {Path: "display_information.description", Resolution: ResolveRemote}, + }, + expected: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "Remote App", Description: "Remote desc"}, + }, + }, + "mixed resolution picks per field": { + local: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "Local App", Description: "Local desc"}, + }, + remote: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "Remote App", Description: "Remote desc"}, + }, + resolutions: []FieldResolution{ + {Path: "display_information.name", Resolution: ResolveLocal}, + {Path: "display_information.description", Resolution: ResolveRemote}, + }, + expected: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "Local App", Description: "Remote desc"}, + }, + }, + "local-only field resolved as local is included": { + local: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App", Description: "Has desc"}, + }, + remote: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App"}, + }, + resolutions: []FieldResolution{ + {Path: "display_information.description", Resolution: ResolveLocal}, + }, + expected: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App", Description: "Has desc"}, + }, + }, + "local-only field resolved as remote is excluded": { + local: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App", Description: "Has desc"}, + }, + remote: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App"}, + }, + resolutions: []FieldResolution{ + {Path: "display_information.description", Resolution: ResolveRemote}, + }, + expected: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App"}, + }, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + result, err := Merge(tc.local, tc.remote, tc.resolutions) + require.NoError(t, err) + assert.Equal(t, tc.expected.DisplayInformation.Name, result.DisplayInformation.Name) + assert.Equal(t, tc.expected.DisplayInformation.Description, result.DisplayInformation.Description) + }) + } +} + +func Test_MergeAllFrom(t *testing.T) { + local := types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "Local", Description: "Local desc"}, + } + remote := types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "Remote", Description: "Remote desc"}, + } + diffs, err := Diff(local, remote) + require.NoError(t, err) + + t.Run("merge all local", func(t *testing.T) { + result, err := MergeAllFrom(local, remote, diffs, MergeAllLocal) + require.NoError(t, err) + assert.Equal(t, "Local", result.DisplayInformation.Name) + assert.Equal(t, "Local desc", result.DisplayInformation.Description) + }) + + t.Run("merge all remote", func(t *testing.T) { + result, err := MergeAllFrom(local, remote, diffs, MergeAllRemote) + require.NoError(t, err) + assert.Equal(t, "Remote", result.DisplayInformation.Name) + assert.Equal(t, "Remote desc", result.DisplayInformation.Description) + }) +} diff --git a/internal/pkg/manifest/sync.go b/internal/pkg/manifest/sync.go new file mode 100644 index 00000000..c747ef1a --- /dev/null +++ b/internal/pkg/manifest/sync.go @@ -0,0 +1,110 @@ +package manifest + +import ( + "context" + "fmt" + + "github.com/slackapi/slack-cli/internal/shared" + "github.com/slackapi/slack-cli/internal/shared/types" + "github.com/slackapi/slack-cli/internal/slackerror" + "github.com/slackapi/slack-cli/internal/style" +) + +// SyncResult describes what happened during a sync operation. +type SyncResult struct { + Merged types.AppManifest + WriteBack WriteBackResult + HasDifferences bool +} + +// Sync performs two-way manifest sync between local and remote. It fetches +// both manifests, computes diffs, prompts the user for resolution, writes +// the merged result to both the API and the local file, and returns the result. +func Sync(ctx context.Context, clients *shared.ClientFactory, app types.App, auth types.SlackAuth) (*SyncResult, error) { + localManifest, err := clients.AppClient().Manifest.GetManifestLocal(ctx, clients.SDKConfig, clients.HookExecutor) + if err != nil { + return nil, slackerror.New("Failed to get local manifest").WithRootCause(err).WithCode(slackerror.ErrInvalidManifest) + } + + remoteManifest, err := clients.AppClient().Manifest.GetManifestRemote(ctx, auth.Token, app.AppID) + if err != nil { + return nil, slackerror.New("Failed to get remote manifest from app settings").WithRootCause(err) + } + + diffs, err := Diff(localManifest.AppManifest, remoteManifest.AppManifest) + if err != nil { + return nil, fmt.Errorf("failed to compute manifest differences: %w", err) + } + + if !diffs.HasDifferences() { + clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{ + Emoji: "books", + Text: "App Manifest", + Secondary: []string{"Project manifest and app settings are in sync"}, + })) + return &SyncResult{Merged: localManifest.AppManifest, HasDifferences: false}, nil + } + + DisplayDiffs(ctx, clients.IO, diffs) + + if !clients.IO.IsTTY() { + return nil, slackerror.New(slackerror.ErrAppManifestUpdate). + WithRemediation("Run %s interactively to resolve manifest differences, or use %s to overwrite app settings", + style.CommandText("slack manifest sync"), + style.CommandText("--force"), + ) + } + + merged, err := resolveInteractively(ctx, clients, localManifest.AppManifest, remoteManifest.AppManifest, diffs) + if err != nil { + return nil, err + } + + // Push merged manifest to API + clients.IO.PrintInfo(ctx, false, "\n Syncing manifest...") + _, err = clients.API().UpdateApp(ctx, auth.Token, app.AppID, merged, true, true) + if err != nil { + return nil, slackerror.New("Failed to update app settings with merged manifest").WithRootCause(err) + } + clients.IO.PrintInfo(ctx, false, " %s Updated app settings", style.Green("✓")) + + // Write back to local file + workingDir := clients.SDKConfig.WorkingDirectory + writeResult, err := WriteManifestLocal(clients.Fs, workingDir, merged) + if err != nil { + return nil, err + } + if writeResult.Written { + clients.IO.PrintInfo(ctx, false, " %s Updated %s", style.Green("✓"), manifestFileName) + } else if writeResult.Warning != "" { + clients.IO.PrintInfo(ctx, false, " %s %s", style.Yellow("!"), writeResult.Warning) + } + + clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{ + Emoji: "books", + Text: "App Manifest", + Secondary: []string{fmt.Sprintf("Finished manifest sync for %q", localManifest.DisplayInformation.Name)}, + })) + + return &SyncResult{Merged: merged, WriteBack: writeResult, HasDifferences: true}, nil +} + +func resolveInteractively(ctx context.Context, clients *shared.ClientFactory, local, remote types.AppManifest, diffs *DiffResult) (types.AppManifest, error) { + strategy, err := PromptResolutionStrategy(ctx, clients.IO) + if err != nil { + return types.AppManifest{}, err + } + + switch strategy { + case MergeAllLocal: + return MergeAllFrom(local, remote, diffs, MergeAllLocal) + case MergeAllRemote: + return MergeAllFrom(local, remote, diffs, MergeAllRemote) + default: + resolutions, err := PromptFieldResolutions(ctx, clients.IO, diffs) + if err != nil { + return types.AppManifest{}, err + } + return Merge(local, remote, resolutions) + } +} diff --git a/internal/pkg/manifest/writeback.go b/internal/pkg/manifest/writeback.go new file mode 100644 index 00000000..e32f0503 --- /dev/null +++ b/internal/pkg/manifest/writeback.go @@ -0,0 +1,151 @@ +package manifest + +import ( + "bytes" + "encoding/json" + "fmt" + "os" + "path/filepath" + + "github.com/slackapi/slack-cli/internal/shared/types" + "github.com/spf13/afero" +) + +const manifestFileName = "manifest.json" + +// WriteBackResult describes what happened during write-back. +type WriteBackResult struct { + Written bool + FilePath string + Warning string +} + +// WriteManifestLocal writes the merged manifest back to the project's +// manifest.json file, preserving the original file's key ordering by +// using the same JSON structure. +func WriteManifestLocal(fs afero.Fs, workingDir string, manifest types.AppManifest) (WriteBackResult, error) { + manifestPath := filepath.Join(workingDir, manifestFileName) + + exists, err := afero.Exists(fs, manifestPath) + if err != nil { + return WriteBackResult{}, fmt.Errorf("failed to check manifest file: %w", err) + } + if !exists { + return WriteBackResult{ + Warning: fmt.Sprintf("No %s found in project root — merged manifest was not written locally", manifestFileName), + }, nil + } + + original, err := afero.ReadFile(fs, manifestPath) + if err != nil { + return WriteBackResult{}, fmt.Errorf("failed to read %s: %w", manifestFileName, err) + } + + merged, err := marshalPreservingOrder(original, manifest) + if err != nil { + return WriteBackResult{}, fmt.Errorf("failed to serialize merged manifest: %w", err) + } + + if err := afero.WriteFile(fs, manifestPath, merged, os.FileMode(0644)); err != nil { + return WriteBackResult{}, fmt.Errorf("failed to write %s: %w", manifestFileName, err) + } + + return WriteBackResult{Written: true, FilePath: manifestPath}, nil +} + +// marshalPreservingOrder serializes the manifest to JSON, preserving the key +// order from the original file. It does this by unmarshaling the original into +// an ordered structure, applying the new values, then re-serializing. +func marshalPreservingOrder(original []byte, manifest types.AppManifest) ([]byte, error) { + var originalKeys []string + if err := extractTopLevelKeyOrder(original, &originalKeys); err != nil { + return marshalFresh(manifest) + } + + newData, err := json.Marshal(manifest) + if err != nil { + return nil, err + } + var newMap map[string]json.RawMessage + if err := json.Unmarshal(newData, &newMap); err != nil { + return marshalFresh(manifest) + } + + // Build output with original key order, then append any new keys + type kv struct { + Key string + Value json.RawMessage + } + var ordered []kv + seen := make(map[string]bool) + + for _, key := range originalKeys { + if val, exists := newMap[key]; exists { + ordered = append(ordered, kv{Key: key, Value: val}) + seen[key] = true + } + } + for key, val := range newMap { + if !seen[key] { + ordered = append(ordered, kv{Key: key, Value: val}) + } + } + + // Manually build JSON with preserved order + buf := []byte("{\n") + for i, item := range ordered { + keyJSON, _ := json.Marshal(item.Key) + indented, _ := json.MarshalIndent(json.RawMessage(item.Value), " ", " ") + buf = fmt.Appendf(buf, " %s: %s", keyJSON, indented) + if i < len(ordered)-1 { + buf = append(buf, ',') + } + buf = append(buf, '\n') + } + buf = append(buf, '}') + buf = append(buf, '\n') + + return buf, nil +} + +func extractTopLevelKeyOrder(data []byte, keys *[]string) error { + var raw map[string]json.RawMessage + if err := json.Unmarshal(data, &raw); err != nil { + return err + } + // json.Unmarshal into map doesn't preserve order, so we need to parse tokens + dec := json.NewDecoder(bytes.NewReader(data)) + // Read opening brace + t, err := dec.Token() + if err != nil { + return err + } + if delim, ok := t.(json.Delim); !ok || delim != '{' { + return fmt.Errorf("expected opening brace") + } + for dec.More() { + t, err := dec.Token() + if err != nil { + return err + } + key, ok := t.(string) + if !ok { + continue + } + *keys = append(*keys, key) + // Skip the value + var skip json.RawMessage + if err := dec.Decode(&skip); err != nil { + return err + } + } + return nil +} + +func marshalFresh(manifest types.AppManifest) ([]byte, error) { + data, err := json.MarshalIndent(manifest, "", " ") + if err != nil { + return nil, err + } + return append(data, '\n'), nil +} diff --git a/internal/pkg/manifest/writeback_test.go b/internal/pkg/manifest/writeback_test.go new file mode 100644 index 00000000..46ffea41 --- /dev/null +++ b/internal/pkg/manifest/writeback_test.go @@ -0,0 +1,85 @@ +package manifest + +import ( + "testing" + + "github.com/slackapi/slack-cli/internal/shared/types" + "github.com/spf13/afero" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_WriteManifestLocal(t *testing.T) { + t.Run("writes merged manifest to existing file", func(t *testing.T) { + fs := afero.NewMemMapFs() + original := `{ + "display_information": { + "name": "Original App" + } +} +` + require.NoError(t, afero.WriteFile(fs, "/project/manifest.json", []byte(original), 0644)) + + manifest := types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "Merged App", Description: "New desc"}, + } + + result, err := WriteManifestLocal(fs, "/project", manifest) + require.NoError(t, err) + assert.True(t, result.Written) + assert.Equal(t, "/project/manifest.json", result.FilePath) + + content, err := afero.ReadFile(fs, "/project/manifest.json") + require.NoError(t, err) + assert.Contains(t, string(content), "Merged App") + assert.Contains(t, string(content), "New desc") + }) + + t.Run("preserves original key order", func(t *testing.T) { + fs := afero.NewMemMapFs() + original := `{ + "settings": {"function_runtime": "local"}, + "display_information": {"name": "App"} +} +` + require.NoError(t, afero.WriteFile(fs, "/project/manifest.json", []byte(original), 0644)) + + manifest := types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App"}, + Settings: &types.AppSettings{FunctionRuntime: types.LocallyRun}, + } + + result, err := WriteManifestLocal(fs, "/project", manifest) + require.NoError(t, err) + assert.True(t, result.Written) + + content, err := afero.ReadFile(fs, "/project/manifest.json") + require.NoError(t, err) + contentStr := string(content) + // settings should come before display_information (matching original order) + settingsIdx := indexOf(contentStr, "settings") + displayIdx := indexOf(contentStr, "display_information") + assert.Less(t, settingsIdx, displayIdx, "key order should be preserved from original file") + }) + + t.Run("returns warning when manifest.json does not exist", func(t *testing.T) { + fs := afero.NewMemMapFs() + manifest := types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App"}, + } + + result, err := WriteManifestLocal(fs, "/project", manifest) + require.NoError(t, err) + assert.False(t, result.Written) + assert.Contains(t, result.Warning, "No manifest.json found") + }) +} + +func indexOf(s, substr string) int { + for i := range s { + if i+len(substr) <= len(s) && s[i:i+len(substr)] == substr { + return i + } + } + return -1 +}