From fb3aa95b3c723346546735b17ddf08175161bbdc Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Sat, 13 Jun 2026 16:14:35 -0700 Subject: [PATCH 01/16] feat: add slack manifest diff command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Compares the project manifest against app settings and prints any differences. Read-only — no API mutations, no file writes, no prompts. Useful for CI guardrails, pre-flight visibility before deploy, and debugging manifest drift. Carved out from the larger two-way manifest sync work (#543) so the diff capability can land independently with a smaller diff. --- cmd/manifest/diff.go | 80 +++++++++++++++ cmd/manifest/diff_test.go | 90 +++++++++++++++++ cmd/manifest/manifest.go | 1 + internal/manifest/diff.go | 118 ++++++++++++++++++++++ internal/manifest/diff_test.go | 157 ++++++++++++++++++++++++++++++ internal/manifest/display.go | 83 ++++++++++++++++ internal/manifest/flatten.go | 77 +++++++++++++++ internal/manifest/flatten_test.go | 139 ++++++++++++++++++++++++++ 8 files changed, 745 insertions(+) create mode 100644 cmd/manifest/diff.go create mode 100644 cmd/manifest/diff_test.go create mode 100644 internal/manifest/diff.go create mode 100644 internal/manifest/diff_test.go create mode 100644 internal/manifest/display.go create mode 100644 internal/manifest/flatten.go create mode 100644 internal/manifest/flatten_test.go diff --git a/cmd/manifest/diff.go b/cmd/manifest/diff.go new file mode 100644 index 00000000..7e53e906 --- /dev/null +++ b/cmd/manifest/diff.go @@ -0,0 +1,80 @@ +// Copyright 2022-2026 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +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/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" +) + +func NewDiffCommand(clients *shared.ClientFactory) *cobra.Command { + return &cobra.Command{ + Use: "diff", + Short: "Show differences between the project manifest and app settings", + Long: "Compare the project manifest with app settings and print any differences.", + Example: style.ExampleCommandsf([]style.ExampleCommand{ + {Command: "manifest diff", Meaning: "Show differences between project manifest and 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.diff") + 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) + + localManifest, err := clients.AppClient().Manifest.GetManifestLocal(ctx, clients.SDKConfig, clients.HookExecutor) + if err != nil { + return err + } + + remoteManifest, err := clients.AppClient().Manifest.GetManifestRemote(ctx, selection.Auth.Token, selection.App.AppID) + if err != nil { + return err + } + + diffs, err := manifest.Diff(localManifest.AppManifest, remoteManifest.AppManifest) + if err != nil { + return 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 nil + } + + manifest.DisplayDiffs(ctx, clients.IO, diffs) + return nil + }, + } +} diff --git a/cmd/manifest/diff_test.go b/cmd/manifest/diff_test.go new file mode 100644 index 00000000..cbfd8470 --- /dev/null +++ b/cmd/manifest/diff_test.go @@ -0,0 +1,90 @@ +// Copyright 2022-2026 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package manifest + +import ( + "context" + "testing" + + "github.com/slackapi/slack-cli/internal/app" + "github.com/slackapi/slack-cli/internal/hooks" + "github.com/slackapi/slack-cli/internal/prompts" + "github.com/slackapi/slack-cli/internal/shared" + "github.com/slackapi/slack-cli/internal/shared/types" + "github.com/slackapi/slack-cli/test/testutil" + "github.com/spf13/cobra" + "github.com/stretchr/testify/mock" +) + +func TestDiffCommand(t *testing.T) { + testutil.TableTestCommand(t, testutil.CommandTests{ + "prints in-sync message when manifests match": { + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + appSelectMock := prompts.NewAppSelectMock() + appSelectPromptFunc = appSelectMock.AppSelectPrompt + appSelectMock.On("AppSelectPrompt", mock.Anything, mock.Anything, prompts.ShowAllEnvironments, prompts.ShowInstalledAppsOnly).Return( + prompts.SelectedApp{ + App: types.App{AppID: "A001"}, + Auth: types.SlackAuth{Token: "xapp"}, + }, nil) + manifestMock := &app.ManifestMockObject{} + manifestMock.On("GetManifestLocal", mock.Anything, mock.Anything, mock.Anything).Return(types.SlackYaml{ + AppManifest: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App"}, + }, + }, nil) + manifestMock.On("GetManifestRemote", mock.Anything, mock.Anything, mock.Anything).Return(types.SlackYaml{ + AppManifest: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App"}, + }, + }, nil) + cf.AppClient().Manifest = manifestMock + cf.SDKConfig = hooks.NewSDKConfigMock() + }, + ExpectedOutputs: []string{"in sync"}, + }, + "prints differences when project and app settings disagree": { + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + appSelectMock := prompts.NewAppSelectMock() + appSelectPromptFunc = appSelectMock.AppSelectPrompt + appSelectMock.On("AppSelectPrompt", mock.Anything, mock.Anything, prompts.ShowAllEnvironments, prompts.ShowInstalledAppsOnly).Return( + prompts.SelectedApp{ + App: types.App{AppID: "A002"}, + Auth: types.SlackAuth{Token: "xapp"}, + }, nil) + manifestMock := &app.ManifestMockObject{} + manifestMock.On("GetManifestLocal", mock.Anything, mock.Anything, mock.Anything).Return(types.SlackYaml{ + AppManifest: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "Project Name"}, + }, + }, nil) + manifestMock.On("GetManifestRemote", mock.Anything, mock.Anything, mock.Anything).Return(types.SlackYaml{ + AppManifest: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "Remote Name"}, + }, + }, nil) + cf.AppClient().Manifest = manifestMock + cf.SDKConfig = hooks.NewSDKConfigMock() + }, + ExpectedOutputs: []string{ + "display_information.name", + "Project Name", + "Remote Name", + }, + }, + }, func(clients *shared.ClientFactory) *cobra.Command { + return NewDiffCommand(clients) + }) +} diff --git a/cmd/manifest/manifest.go b/cmd/manifest/manifest.go index c2c2f3d7..e96df539 100644 --- a/cmd/manifest/manifest.go +++ b/cmd/manifest/manifest.go @@ -80,6 +80,7 @@ func NewCommand(clients *shared.ClientFactory) *cobra.Command { } // Add child commands + cmd.AddCommand(NewDiffCommand(clients)) cmd.AddCommand(NewInfoCommand(clients)) cmd.AddCommand(NewValidateCommand(clients)) diff --git a/internal/manifest/diff.go b/internal/manifest/diff.go new file mode 100644 index 00000000..7fc4e9c2 --- /dev/null +++ b/internal/manifest/diff.go @@ -0,0 +1,118 @@ +// Copyright 2022-2026 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +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) +} + +func diffFlat(local, remote map[string]any) (*DiffResult, error) { + 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 + } + equal, err := valuesEqual(localVal, remoteVal) + if err != nil { + return nil, fmt.Errorf("failed to compare manifest values at %q: %w", path, err) + } + if !equal { + 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, nil +} + +func valuesEqual(a, b any) (bool, error) { + aJSON, err := json.Marshal(a) + if err != nil { + return false, err + } + bJSON, err := json.Marshal(b) + if err != nil { + return false, err + } + return string(aJSON) == string(bJSON), nil +} diff --git a/internal/manifest/diff_test.go b/internal/manifest/diff_test.go new file mode 100644 index 00000000..1d7f00b8 --- /dev/null +++ b/internal/manifest/diff_test.go @@ -0,0 +1,157 @@ +// Copyright 2022-2026 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +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/manifest/display.go b/internal/manifest/display.go new file mode 100644 index 00000000..1a3c9514 --- /dev/null +++ b/internal/manifest/display.go @@ -0,0 +1,83 @@ +// Copyright 2022-2026 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +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, "") +} + +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/manifest/flatten.go b/internal/manifest/flatten.go new file mode 100644 index 00000000..21f4853a --- /dev/null +++ b/internal/manifest/flatten.go @@ -0,0 +1,77 @@ +// Copyright 2022-2026 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package manifest + +import ( + "encoding/json" + "fmt" + "sort" + "strings" +) + +// 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. +// +// Keys that contain literal dots (e.g. function IDs like "slack.users.lookup") +// have those dots backslash-escaped in the path so flatten/unflatten round- +// trip cleanly. splitPath honors the same escape sequence. +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 := escapePathSegment(key) + if prefix != "" { + fullKey = prefix + "." + fullKey + } + switch v := value.(type) { + case map[string]any: + flattenRecursive(fullKey, v, result) + default: + result[fullKey] = value + } + } +} + +// escapePathSegment backslash-escapes the path delimiter and the escape +// character so a key containing a literal dot survives flatten/splitPath. +func escapePathSegment(s string) string { + s = strings.ReplaceAll(s, `\`, `\\`) + s = strings.ReplaceAll(s, `.`, `\.`) + return s +} + +// 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/manifest/flatten_test.go b/internal/manifest/flatten_test.go new file mode 100644 index 00000000..4a41ecb2 --- /dev/null +++ b/internal/manifest/flatten_test.go @@ -0,0 +1,139 @@ +// Copyright 2022-2026 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +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_Flatten_EscapesDotsInKeys(t *testing.T) { + // Manifest function IDs may contain literal dots (e.g. "slack.users.lookup"). + // Flatten must backslash-escape those dots so the path remains parseable. + manifest := types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App"}, + Functions: map[string]types.ManifestFunction{ + "slack.users.lookup": {Title: "Lookup", Description: "Lookup a user"}, + }, + } + + flat, err := Flatten(manifest) + require.NoError(t, err) + + assert.Contains(t, flat, `functions.slack\.users\.lookup.title`) + assert.Equal(t, "Lookup", flat[`functions.slack\.users\.lookup.title`]) + assert.Equal(t, "Lookup a user", flat[`functions.slack\.users\.lookup.description`]) +} + +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) +} From 1a9940340dd9f692659dc186fb214201a9e429b7 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Sat, 13 Jun 2026 16:25:52 -0700 Subject: [PATCH 02/16] docs: add CI/CD example for manifest diff Shows the non-interactive form using --app and --token, matching the intended CI guardrail use case. --- cmd/manifest/diff.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/manifest/diff.go b/cmd/manifest/diff.go index 7e53e906..b7c40505 100644 --- a/cmd/manifest/diff.go +++ b/cmd/manifest/diff.go @@ -32,6 +32,7 @@ func NewDiffCommand(clients *shared.ClientFactory) *cobra.Command { Long: "Compare the project manifest with app settings and print any differences.", Example: style.ExampleCommandsf([]style.ExampleCommand{ {Command: "manifest diff", Meaning: "Show differences between project manifest and app settings"}, + {Command: "manifest diff --app A0123456789 --token xoxp-...", Meaning: "Detect manifest drift in CI without prompts"}, }), Args: cobra.NoArgs, PreRunE: func(cmd *cobra.Command, args []string) error { From 5e518f0448239bd63bc49d75cef561aaf64ce07d Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Sat, 13 Jun 2026 16:28:31 -0700 Subject: [PATCH 03/16] docs: reword non-interactive manifest diff example Replaces "Detect manifest drift in CI" with "Show manifest differences without prompts" so the language is plain and accessible to developers who haven't encountered drift terminology before. --- cmd/manifest/diff.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/manifest/diff.go b/cmd/manifest/diff.go index b7c40505..575554d2 100644 --- a/cmd/manifest/diff.go +++ b/cmd/manifest/diff.go @@ -32,7 +32,7 @@ func NewDiffCommand(clients *shared.ClientFactory) *cobra.Command { Long: "Compare the project manifest with app settings and print any differences.", Example: style.ExampleCommandsf([]style.ExampleCommand{ {Command: "manifest diff", Meaning: "Show differences between project manifest and app settings"}, - {Command: "manifest diff --app A0123456789 --token xoxp-...", Meaning: "Detect manifest drift in CI without prompts"}, + {Command: "manifest diff --app A0123456789 --token xoxp-...", Meaning: "Show manifest differences without prompts"}, }), Args: cobra.NoArgs, PreRunE: func(cmd *cobra.Command, args []string) error { From 79d0468cfd2ef11d5ef79627116583cd926831f3 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Sat, 13 Jun 2026 22:17:51 -0700 Subject: [PATCH 04/16] docs: add godoc for NewDiffCommand --- cmd/manifest/diff.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/manifest/diff.go b/cmd/manifest/diff.go index 575554d2..9dde22f2 100644 --- a/cmd/manifest/diff.go +++ b/cmd/manifest/diff.go @@ -25,6 +25,8 @@ import ( "github.com/spf13/cobra" ) +// NewDiffCommand implements the "manifest diff" command, which prints the +// differences between the project manifest and the app settings on Slack. func NewDiffCommand(clients *shared.ClientFactory) *cobra.Command { return &cobra.Command{ Use: "diff", From 2a8816b89404c02fda9d9a821ccd1e0d824a4d49 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Sat, 13 Jun 2026 22:19:47 -0700 Subject: [PATCH 05/16] docs: add godoc for unexported manifest helpers Documents flattenRecursive, diffFlat, valuesEqual, and formatValue to match the project's existing convention (e.g. cmd/manifest/info.go documents both exported and unexported helpers). --- internal/manifest/diff.go | 5 +++++ internal/manifest/display.go | 3 +++ internal/manifest/flatten.go | 3 +++ 3 files changed, 11 insertions(+) diff --git a/internal/manifest/diff.go b/internal/manifest/diff.go index 7fc4e9c2..a920a275 100644 --- a/internal/manifest/diff.go +++ b/internal/manifest/diff.go @@ -62,6 +62,8 @@ func Diff(local, remote types.AppManifest) (*DiffResult, error) { return diffFlat(localFlat, remoteFlat) } +// diffFlat compares two flattened manifests and returns one FieldDiff per +// path that differs (modified, local-only, or remote-only). func diffFlat(local, remote map[string]any) (*DiffResult, error) { result := &DiffResult{} seen := make(map[string]bool) @@ -105,6 +107,9 @@ func diffFlat(local, remote map[string]any) (*DiffResult, error) { return result, nil } +// valuesEqual reports whether two leaf values from a flattened manifest are +// equivalent. It compares their JSON encodings so type-equivalent values +// (e.g. matching arrays or nested objects) compare equal. func valuesEqual(a, b any) (bool, error) { aJSON, err := json.Marshal(a) if err != nil { diff --git a/internal/manifest/display.go b/internal/manifest/display.go index 1a3c9514..c2e8fc66 100644 --- a/internal/manifest/display.go +++ b/internal/manifest/display.go @@ -62,6 +62,9 @@ func DisplayDiffs(ctx context.Context, io iostreams.IOStreamer, diffs *DiffResul io.PrintInfo(ctx, false, "") } +// formatValue renders a leaf value for display. Strings are quoted, other +// values are JSON-encoded, and any value longer than 80 characters is +// truncated with an ellipsis. func formatValue(v any) string { if v == nil { return "(not present)" diff --git a/internal/manifest/flatten.go b/internal/manifest/flatten.go index 21f4853a..049351f4 100644 --- a/internal/manifest/flatten.go +++ b/internal/manifest/flatten.go @@ -43,6 +43,9 @@ func Flatten(manifest any) (map[string]any, error) { return result, nil } +// flattenRecursive walks a map produced by json.Unmarshal and writes one +// dot-notation entry per leaf into result. Nested maps recurse; arrays and +// scalars are leaves. func flattenRecursive(prefix string, data map[string]any, result map[string]any) { for key, value := range data { fullKey := escapePathSegment(key) From d58fd8e52b039b00f96b29bec7f2f2e7ddaf3f09 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Sat, 13 Jun 2026 22:31:07 -0700 Subject: [PATCH 06/16] fix: drop extra blank line before the first manifest diff entry DisplayDiffs printed a leading blank inside the loop, which combined with the trailing newline from style.Sectionf to produce a double blank between the header and the first difference. Skip the blank on the first iteration so spacing only appears between entries. --- internal/manifest/display.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/manifest/display.go b/internal/manifest/display.go index c2e8fc66..9fa77e80 100644 --- a/internal/manifest/display.go +++ b/internal/manifest/display.go @@ -44,8 +44,10 @@ func DisplayDiffs(ctx context.Context, io iostreams.IOStreamer, diffs *DiffResul }, })) - for _, d := range sorted { - io.PrintInfo(ctx, false, "") + for i, d := range sorted { + if i > 0 { + io.PrintInfo(ctx, false, "") + } switch d.Type { case DiffModified: io.PrintInfo(ctx, false, " %s", style.Bold(d.Path)) From db6aa4722d76cb64504c87918b17cfc89071cbab Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Sat, 13 Jun 2026 22:37:43 -0700 Subject: [PATCH 07/16] fix: filter _metadata paths out of manifest diff output apps.manifest.export does not echo _metadata back, so projects that declared it (e.g. SDK schema annotations) saw a noisy "(only in project)" entry on every diff run. Apply a small ignored-paths filter inside Diff so these never reach the user, and leave the list extensible for any future one-sided fields. --- internal/manifest/diff.go | 35 ++++++++++++++++++++++++-- internal/manifest/diff_test.go | 45 ++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/internal/manifest/diff.go b/internal/manifest/diff.go index a920a275..dac2e137 100644 --- a/internal/manifest/diff.go +++ b/internal/manifest/diff.go @@ -17,10 +17,18 @@ package manifest import ( "encoding/json" "fmt" + "strings" "github.com/slackapi/slack-cli/internal/shared/types" ) +// ignoredDiffPaths are top-level manifest fields that the project may declare +// but Slack's apps.manifest.export does not echo back. Diffs at or under these +// paths are dropped to avoid spurious "only in project" entries on every run. +var ignoredDiffPaths = []string{ + "_metadata", // SDK-side schema annotations; not stored in app settings +} + // DiffType describes how a field differs between local and remote. type DiffType int @@ -49,7 +57,8 @@ func (dr *DiffResult) HasDifferences() bool { } // Diff performs a two-way comparison between local and remote manifests, -// returning all fields that differ between them. +// returning all fields that differ between them. Paths under ignoredDiffPaths +// are excluded. func Diff(local, remote types.AppManifest) (*DiffResult, error) { localFlat, err := Flatten(local) if err != nil { @@ -59,7 +68,29 @@ func Diff(local, remote types.AppManifest) (*DiffResult, error) { if err != nil { return nil, fmt.Errorf("failed to flatten remote manifest: %w", err) } - return diffFlat(localFlat, remoteFlat) + result, err := diffFlat(localFlat, remoteFlat) + if err != nil { + return nil, err + } + filtered := result.Diffs[:0] + for _, d := range result.Diffs { + if !isIgnoredPath(d.Path) { + filtered = append(filtered, d) + } + } + result.Diffs = filtered + return result, nil +} + +// isIgnoredPath reports whether a flattened manifest path is at or under any +// entry in ignoredDiffPaths. +func isIgnoredPath(path string) bool { + for _, prefix := range ignoredDiffPaths { + if path == prefix || strings.HasPrefix(path, prefix+".") { + return true + } + } + return false } // diffFlat compares two flattened manifests and returns one FieldDiff per diff --git a/internal/manifest/diff_test.go b/internal/manifest/diff_test.go index 1d7f00b8..f82be357 100644 --- a/internal/manifest/diff_test.go +++ b/internal/manifest/diff_test.go @@ -142,6 +142,51 @@ func Test_Diff(t *testing.T) { } } +func Test_Diff_IgnoresMetadataPaths(t *testing.T) { + // _metadata is project-side annotation that apps.manifest.export does not + // echo back. Without filtering, projects using _metadata see noisy + // "(only in project)" entries on every diff run. + local := types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App"}, + Metadata: &types.ManifestMetadata{ + MajorVersion: 1, + MinorVersion: 2, + }, + } + remote := types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App"}, + } + + result, err := Diff(local, remote) + require.NoError(t, err) + assert.False(t, result.HasDifferences(), "metadata-only differences should be filtered, got %+v", result.Diffs) +} + +func Test_Diff_FiltersMetadataButKeepsOtherDiffs(t *testing.T) { + local := types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "Project"}, + Metadata: &types.ManifestMetadata{MajorVersion: 1}, + } + remote := types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "Remote"}, + } + + result, err := Diff(local, remote) + require.NoError(t, err) + require.True(t, result.HasDifferences()) + for _, d := range result.Diffs { + assert.False(t, isIgnoredPath(d.Path), "unexpected ignored path in result: %s", d.Path) + } + // The display_information.name diff must still be reported. + var sawNameDiff bool + for _, d := range result.Diffs { + if d.Path == "display_information.name" { + sawNameDiff = true + } + } + assert.True(t, sawNameDiff, "display_information.name diff was unexpectedly filtered") +} + func Test_DiffResult_HasDifferences(t *testing.T) { t.Run("empty result has no differences", func(t *testing.T) { result := &DiffResult{} From 1e703304d6e3cd73d0391002a53ab101f7b3855b Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Sat, 13 Jun 2026 22:41:39 -0700 Subject: [PATCH 08/16] fix: truncate manifest diff values by rune, not byte formatValue used a byte-based slice (s[:77]) for the truncation, which could cut a multi-byte UTF-8 character in half and produce an invalid string in the middle of a localized field value. Switch to a rune-based truncate helper and cover it with a test that includes multi-byte input. --- internal/manifest/display.go | 20 +++++++--- internal/manifest/display_test.go | 61 +++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 5 deletions(-) create mode 100644 internal/manifest/display_test.go diff --git a/internal/manifest/display.go b/internal/manifest/display.go index 9fa77e80..f9b2a2b9 100644 --- a/internal/manifest/display.go +++ b/internal/manifest/display.go @@ -65,7 +65,7 @@ func DisplayDiffs(ctx context.Context, io iostreams.IOStreamer, diffs *DiffResul } // formatValue renders a leaf value for display. Strings are quoted, other -// values are JSON-encoded, and any value longer than 80 characters is +// values are JSON-encoded, and any value longer than 80 runes is // truncated with an ellipsis. func formatValue(v any) string { if v == nil { @@ -79,10 +79,20 @@ func formatValue(v any) string { if err != nil { return fmt.Sprintf("%v", val) } - s := string(data) - if len(s) > 80 { - return s[:77] + "..." - } + return truncateRunes(string(data), 80) + } +} + +// truncateRunes returns s unchanged if it is at most max runes, otherwise it +// returns the first max-3 runes followed by "...". Splitting on runes (rather +// than bytes) avoids cutting through a multi-byte UTF-8 character. +func truncateRunes(s string, max int) string { + if max <= 3 { + return s + } + runes := []rune(s) + if len(runes) <= max { return s } + return string(runes[:max-3]) + "..." } diff --git a/internal/manifest/display_test.go b/internal/manifest/display_test.go new file mode 100644 index 00000000..232e8c84 --- /dev/null +++ b/internal/manifest/display_test.go @@ -0,0 +1,61 @@ +// Copyright 2022-2026 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package manifest + +import ( + "testing" + "unicode/utf8" + + "github.com/stretchr/testify/assert" +) + +func Test_truncateRunes(t *testing.T) { + tests := map[string]struct { + input string + max int + expected string + }{ + "shorter than max returns unchanged": { + input: "hello", + max: 80, + expected: "hello", + }, + "exactly max runes returns unchanged": { + input: "abcdefghij", + max: 10, + expected: "abcdefghij", + }, + "longer than max truncates with ellipsis": { + input: "abcdefghijklmno", + max: 10, + expected: "abcdefg...", + }, + "multi-byte runes are not cut mid-character": { + // Each emoji is 4 bytes in UTF-8 but one rune. Byte-based + // slicing would split the middle emoji. + input: "🐶🐱🐭🐹🐰🦊🐻🐼🐨🐯🦁🐮", + max: 6, + expected: "🐶🐱🐭...", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + got := truncateRunes(tc.input, tc.max) + assert.Equal(t, tc.expected, got) + // In every case the result must remain valid UTF-8. + assert.True(t, utf8.ValidString(got), "result is not valid UTF-8: %q", got) + }) + } +} From ea650695a8910445c5da6c7526dddf79ad1b73f4 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Sat, 13 Jun 2026 22:43:38 -0700 Subject: [PATCH 09/16] test: assert exact diff and flatten counts to catch spurious entries The Test_Diff and Test_Flatten loops previously only verified that expected entries were present, so a regression that produced extra unrelated entries would not have failed. Add a Len assertion before the per-entry checks. Also update the function-related cases to account for ManifestFunction.InputParameters and OutputParameters (which lack the omitempty tag and therefore always serialize as null). --- internal/manifest/diff_test.go | 6 ++++++ internal/manifest/flatten_test.go | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/internal/manifest/diff_test.go b/internal/manifest/diff_test.go index f82be357..a77854b0 100644 --- a/internal/manifest/diff_test.go +++ b/internal/manifest/diff_test.go @@ -83,6 +83,11 @@ func Test_Diff(t *testing.T) { expected: []FieldDiff{ {Path: "functions.greet.description", Type: DiffLocalOnly, LocalValue: "Hello"}, {Path: "functions.greet.title", Type: DiffLocalOnly, LocalValue: "Greet"}, + // ManifestFunction.InputParameters/OutputParameters lack + // `omitempty`, so an added function flattens with nil values + // for both — they show as local-only too. + {Path: "functions.greet.input_parameters", Type: DiffLocalOnly, LocalValue: nil}, + {Path: "functions.greet.output_parameters", Type: DiffLocalOnly, LocalValue: nil}, }, }, "array values compared as wholes": { @@ -121,6 +126,7 @@ func Test_Diff(t *testing.T) { return } assert.True(t, result.HasDifferences()) + assert.Len(t, result.Diffs, len(tc.expected), "unexpected number of diffs: got %+v", result.Diffs) for _, expectedDiff := range tc.expected { found := false for _, actualDiff := range result.Diffs { diff --git a/internal/manifest/flatten_test.go b/internal/manifest/flatten_test.go index 4a41ecb2..f2236ce8 100644 --- a/internal/manifest/flatten_test.go +++ b/internal/manifest/flatten_test.go @@ -69,6 +69,10 @@ func Test_Flatten(t *testing.T) { "display_information.name": "App", "functions.greet.title": "Greet", "functions.greet.description": "Greets a user", + // ManifestFunction.InputParameters/OutputParameters lack `omitempty` + // so they always serialize as null. Flatten captures the nulls. + "functions.greet.input_parameters": nil, + "functions.greet.output_parameters": nil, }, }, "treats arrays as leaf values": { @@ -102,6 +106,7 @@ func Test_Flatten(t *testing.T) { t.Run(name, func(t *testing.T) { result, err := Flatten(tc.manifest) require.NoError(t, err) + assert.Len(t, result, len(tc.expected), "unexpected number of flattened keys: got %+v", result) for key, expectedVal := range tc.expected { assert.Contains(t, result, key) assert.Equal(t, expectedVal, result[key], "mismatch at key %s", key) From 9cab9ad91e10017ec6c241ff0e85461dc67370ca Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Sat, 13 Jun 2026 23:01:32 -0700 Subject: [PATCH 10/16] fix: suppress " (local)" suffix differences in manifest diff Slack's apps.manifest.export appends " (local)" to display_information.name and features.bot_user.display_name when returning the manifest of a dev-installed app. Fresh installs would otherwise show two phantom diffs immediately. Suppress these specifically when removing the suffix would make the values equal, so genuine renames still surface. --- internal/manifest/diff.go | 48 ++++++++++++++++++++++++++++++++-- internal/manifest/diff_test.go | 40 ++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/internal/manifest/diff.go b/internal/manifest/diff.go index dac2e137..705697ce 100644 --- a/internal/manifest/diff.go +++ b/internal/manifest/diff.go @@ -29,6 +29,17 @@ var ignoredDiffPaths = []string{ "_metadata", // SDK-side schema annotations; not stored in app settings } +// devLocalSuffixPaths are flattened paths where Slack's apps.manifest.export +// appends " (local)" for dev-installed apps. Diffs at these paths are +// dropped only when removing the suffix would make the values equal; real +// renames still surface. +var devLocalSuffixPaths = []string{ + "display_information.name", + "features.bot_user.display_name", +} + +const devLocalSuffix = " (local)" + // DiffType describes how a field differs between local and remote. type DiffType int @@ -74,9 +85,13 @@ func Diff(local, remote types.AppManifest) (*DiffResult, error) { } filtered := result.Diffs[:0] for _, d := range result.Diffs { - if !isIgnoredPath(d.Path) { - filtered = append(filtered, d) + if isIgnoredPath(d.Path) { + continue } + if isDevLocalSuffixDiff(d) { + continue + } + filtered = append(filtered, d) } result.Diffs = filtered return result, nil @@ -93,6 +108,35 @@ func isIgnoredPath(path string) bool { return false } +// isDevLocalSuffixDiff reports whether a Modified diff is purely the result +// of Slack's apps.manifest.export appending " (local)" to a name field for a +// dev-installed app. Real renames are not suppressed because trimming the +// suffix from RemoteValue would not produce LocalValue. +func isDevLocalSuffixDiff(d FieldDiff) bool { + if d.Type != DiffModified { + return false + } + matched := false + for _, p := range devLocalSuffixPaths { + if d.Path == p { + matched = true + break + } + } + if !matched { + return false + } + local, ok := d.LocalValue.(string) + if !ok { + return false + } + remote, ok := d.RemoteValue.(string) + if !ok { + return false + } + return strings.TrimSuffix(remote, devLocalSuffix) == local +} + // diffFlat compares two flattened manifests and returns one FieldDiff per // path that differs (modified, local-only, or remote-only). func diffFlat(local, remote map[string]any) (*DiffResult, error) { diff --git a/internal/manifest/diff_test.go b/internal/manifest/diff_test.go index a77854b0..e1d070dd 100644 --- a/internal/manifest/diff_test.go +++ b/internal/manifest/diff_test.go @@ -193,6 +193,46 @@ func Test_Diff_FiltersMetadataButKeepsOtherDiffs(t *testing.T) { assert.True(t, sawNameDiff, "display_information.name diff was unexpectedly filtered") } +func Test_Diff_SuppressesDevLocalSuffix(t *testing.T) { + // apps.manifest.export appends " (local)" to display_information.name and + // features.bot_user.display_name for dev-installed apps. The diff command + // suppresses these so users don't see noise on every run. + local := types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "romantic-dolphin-526"}, + Features: &types.AppFeatures{ + BotUser: types.BotUser{DisplayName: "romantic-dolphin-526"}, + }, + } + remote := types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "romantic-dolphin-526 (local)"}, + Features: &types.AppFeatures{ + BotUser: types.BotUser{DisplayName: "romantic-dolphin-526 (local)"}, + }, + } + result, err := Diff(local, remote) + require.NoError(t, err) + assert.False(t, result.HasDifferences(), "dev-local suffix differences should be suppressed, got %+v", result.Diffs) +} + +func Test_Diff_PreservesRealRenames(t *testing.T) { + // A genuine rename (suffix-trimmed remote does not equal local) must + // continue to surface as a diff. + local := types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "new-name"}, + } + remote := types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "old-name (local)"}, + } + result, err := Diff(local, remote) + require.NoError(t, err) + require.True(t, result.HasDifferences()) + require.Len(t, result.Diffs, 1) + assert.Equal(t, "display_information.name", result.Diffs[0].Path) + assert.Equal(t, DiffModified, result.Diffs[0].Type) + assert.Equal(t, "new-name", result.Diffs[0].LocalValue) + assert.Equal(t, "old-name (local)", result.Diffs[0].RemoteValue) +} + func Test_DiffResult_HasDifferences(t *testing.T) { t.Run("empty result has no differences", func(t *testing.T) { result := &DiffResult{} From 413f2727cf5a3b829b939124d1b81526b0df8341 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Sat, 13 Jun 2026 23:18:50 -0700 Subject: [PATCH 11/16] fix: suppress remote-only is_mcp_enabled=false from manifest diff apps.manifest.export emits settings.is_mcp_enabled: false for every app even when the project never declared the field, so users would otherwise see a phantom "(only in app settings)" entry on every run. Suppress it specifically when the value is false; if a user sets it to true locally, the resulting Modified diff still surfaces. --- internal/manifest/diff.go | 36 ++++++++++++++++++++++++++++ internal/manifest/diff_test.go | 43 ++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/internal/manifest/diff.go b/internal/manifest/diff.go index 705697ce..2b5d38b9 100644 --- a/internal/manifest/diff.go +++ b/internal/manifest/diff.go @@ -40,6 +40,16 @@ var devLocalSuffixPaths = []string{ const devLocalSuffix = " (local)" +// remoteFalseDefaultPaths are flattened paths where Slack's +// apps.manifest.export emits a default `false` for every app, even when the +// project has not declared the field. Remote-only diffs at these paths are +// dropped when the value is false so users do not see a phantom entry on +// every run; a real disagreement (e.g. local sets the field to true) still +// surfaces as a Modified diff. +var remoteFalseDefaultPaths = []string{ + "settings.is_mcp_enabled", +} + // DiffType describes how a field differs between local and remote. type DiffType int @@ -91,6 +101,9 @@ func Diff(local, remote types.AppManifest) (*DiffResult, error) { if isDevLocalSuffixDiff(d) { continue } + if isRemoteFalseDefaultDiff(d) { + continue + } filtered = append(filtered, d) } result.Diffs = filtered @@ -108,6 +121,29 @@ func isIgnoredPath(path string) bool { return false } +// isRemoteFalseDefaultDiff reports whether a remote-only diff is purely the +// result of Slack's apps.manifest.export emitting a default `false` for a +// field the project did not declare. Real disagreements (e.g. local sets +// the field to true) are not suppressed because they would surface as a +// Modified diff, not a remote-only diff. +func isRemoteFalseDefaultDiff(d FieldDiff) bool { + if d.Type != DiffRemoteOnly { + return false + } + matched := false + for _, p := range remoteFalseDefaultPaths { + if d.Path == p { + matched = true + break + } + } + if !matched { + return false + } + v, ok := d.RemoteValue.(bool) + return ok && !v +} + // isDevLocalSuffixDiff reports whether a Modified diff is purely the result // of Slack's apps.manifest.export appending " (local)" to a name field for a // dev-installed app. Real renames are not suppressed because trimming the diff --git a/internal/manifest/diff_test.go b/internal/manifest/diff_test.go index e1d070dd..b9622e95 100644 --- a/internal/manifest/diff_test.go +++ b/internal/manifest/diff_test.go @@ -233,6 +233,49 @@ func Test_Diff_PreservesRealRenames(t *testing.T) { assert.Equal(t, "old-name (local)", result.Diffs[0].RemoteValue) } +func Test_Diff_SuppressesRemoteFalseDefaultIsMcpEnabled(t *testing.T) { + // apps.manifest.export emits settings.is_mcp_enabled: false for every app + // even when the project never declared the field. Suppress the + // remote-only diff so users do not see a phantom entry on every run. + local := types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App"}, + } + remote := types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App"}, + Settings: &types.AppSettings{ + IsMCPEnabled: ptrBool(false), + }, + } + result, err := Diff(local, remote) + require.NoError(t, err) + assert.False(t, result.HasDifferences(), "remote-only is_mcp_enabled=false should be suppressed, got %+v", result.Diffs) +} + +func Test_Diff_SurfacesRealIsMcpEnabledDisagreement(t *testing.T) { + // A genuine disagreement — local sets is_mcp_enabled to true while + // remote returns false — must still surface as a Modified diff. + local := types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App"}, + Settings: &types.AppSettings{ + IsMCPEnabled: ptrBool(true), + }, + } + remote := types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "App"}, + Settings: &types.AppSettings{ + IsMCPEnabled: ptrBool(false), + }, + } + result, err := Diff(local, remote) + require.NoError(t, err) + require.True(t, result.HasDifferences()) + require.Len(t, result.Diffs, 1) + assert.Equal(t, "settings.is_mcp_enabled", result.Diffs[0].Path) + assert.Equal(t, DiffModified, result.Diffs[0].Type) +} + +func ptrBool(b bool) *bool { return &b } + func Test_DiffResult_HasDifferences(t *testing.T) { t.Run("empty result has no differences", func(t *testing.T) { result := &DiffResult{} From d1cf988e81d633e6754b94f00d84357faaea0195 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Sun, 14 Jun 2026 19:47:06 -0700 Subject: [PATCH 12/16] docs: reword in-sync message to "match" "In sync" reads as version-control jargon and is awkward when paired with the diff command's read-only contract. Use plainer language that any developer can parse without translation. --- cmd/manifest/diff.go | 2 +- cmd/manifest/diff_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/manifest/diff.go b/cmd/manifest/diff.go index 9dde22f2..2339a58f 100644 --- a/cmd/manifest/diff.go +++ b/cmd/manifest/diff.go @@ -71,7 +71,7 @@ func NewDiffCommand(clients *shared.ClientFactory) *cobra.Command { 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"}, + Secondary: []string{"Project manifest and app settings match"}, })) return nil } diff --git a/cmd/manifest/diff_test.go b/cmd/manifest/diff_test.go index cbfd8470..647d547d 100644 --- a/cmd/manifest/diff_test.go +++ b/cmd/manifest/diff_test.go @@ -30,7 +30,7 @@ import ( func TestDiffCommand(t *testing.T) { testutil.TableTestCommand(t, testutil.CommandTests{ - "prints in-sync message when manifests match": { + "prints match message when manifests match": { Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { appSelectMock := prompts.NewAppSelectMock() appSelectPromptFunc = appSelectMock.AppSelectPrompt @@ -53,7 +53,7 @@ func TestDiffCommand(t *testing.T) { cf.AppClient().Manifest = manifestMock cf.SDKConfig = hooks.NewSDKConfigMock() }, - ExpectedOutputs: []string{"in sync"}, + ExpectedOutputs: []string{"match"}, }, "prints differences when project and app settings disagree": { Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { From 79d3a36cde1e43c3ba029728f6aa722e4a0c9cd1 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Sun, 14 Jun 2026 19:57:02 -0700 Subject: [PATCH 13/16] feat: render manifest diff side-by-side for all difference types Local-only and remote-only fields previously printed in a different shape than modified fields, forcing the reader to mentally translate "only in app settings" into "Project doesn't have it." Render every diff with the same Project/App settings layout and use "(not set)" on the absent side. The header still announces the count and the direction is conveyed by which side reads "(not set)". --- internal/manifest/display.go | 25 +++++++---- internal/manifest/display_test.go | 74 +++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 8 deletions(-) diff --git a/internal/manifest/display.go b/internal/manifest/display.go index f9b2a2b9..5303d8c6 100644 --- a/internal/manifest/display.go +++ b/internal/manifest/display.go @@ -48,22 +48,31 @@ func DisplayDiffs(ctx context.Context, io iostreams.IOStreamer, diffs *DiffResul if i > 0 { io.PrintInfo(ctx, false, "") } + var local, remote string 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)) + local = formatValue(d.LocalValue) + remote = absentValue 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)) + local = absentValue + remote = formatValue(d.RemoteValue) + default: + local = formatValue(d.LocalValue) + remote = formatValue(d.RemoteValue) } + io.PrintInfo(ctx, false, " %s", style.Bold(d.Path)) + io.PrintInfo(ctx, false, " Project: %s", local) + io.PrintInfo(ctx, false, " App settings: %s", remote) } io.PrintInfo(ctx, false, "") } +// absentValue is shown opposite a present value when a field exists on only +// one side of a diff. It is intentionally distinct from formatValue(nil)'s +// "(not present)", which represents a JSON null on a side that does have +// the field. +const absentValue = "(not set)" + // formatValue renders a leaf value for display. Strings are quoted, other // values are JSON-encoded, and any value longer than 80 runes is // truncated with an ellipsis. diff --git a/internal/manifest/display_test.go b/internal/manifest/display_test.go index 232e8c84..385dd539 100644 --- a/internal/manifest/display_test.go +++ b/internal/manifest/display_test.go @@ -18,9 +18,83 @@ import ( "testing" "unicode/utf8" + "github.com/slackapi/slack-cli/internal/shared" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/stretchr/testify/assert" ) +func Test_DisplayDiffs(t *testing.T) { + tests := map[string]struct { + diffs []FieldDiff + expectedSubstrs []string + forbiddenSubstrs []string + }{ + "modified field shows both values side-by-side": { + diffs: []FieldDiff{ + { + Path: "display_information.name", + Type: DiffModified, + LocalValue: "Project Name", + RemoteValue: "Remote Name", + }, + }, + expectedSubstrs: []string{ + "display_information.name", + `Project: "Project Name"`, + `App settings: "Remote Name"`, + }, + forbiddenSubstrs: []string{"(only in", "Value:", "(not set)"}, + }, + "local-only field shows (not set) on the app settings side": { + diffs: []FieldDiff{ + { + Path: "functions.greet.title", + Type: DiffLocalOnly, + LocalValue: "Greet", + }, + }, + expectedSubstrs: []string{ + "functions.greet.title", + `Project: "Greet"`, + "App settings: (not set)", + }, + forbiddenSubstrs: []string{"(only in", "Value:"}, + }, + "remote-only field shows (not set) on the project side": { + diffs: []FieldDiff{ + { + Path: "settings.is_mcp_enabled", + Type: DiffRemoteOnly, + RemoteValue: false, + }, + }, + expectedSubstrs: []string{ + "settings.is_mcp_enabled", + "Project: (not set)", + "App settings: false", + }, + forbiddenSubstrs: []string{"(only in", "Value:"}, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + cm := shared.NewClientsMock() + cm.AddDefaultMocks() + + DisplayDiffs(ctx, cm.IO, &DiffResult{Diffs: tc.diffs}) + + out := cm.GetStdoutOutput() + for _, want := range tc.expectedSubstrs { + assert.Contains(t, out, want, "expected output to contain %q", want) + } + for _, forbidden := range tc.forbiddenSubstrs { + assert.NotContains(t, out, forbidden, "expected output not to contain %q", forbidden) + } + }) + } +} + func Test_truncateRunes(t *testing.T) { tests := map[string]struct { input string From 8370ed75983d46f44ea70cea559303ac38560b39 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Sun, 14 Jun 2026 21:18:52 -0700 Subject: [PATCH 14/16] docs: pluralize manifest diff count using style.Pluralize Replaces "Found N difference(s)" with the proper singular/plural form via the existing style.Pluralize helper. --- internal/manifest/display.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/manifest/display.go b/internal/manifest/display.go index 5303d8c6..7ece8508 100644 --- a/internal/manifest/display.go +++ b/internal/manifest/display.go @@ -40,7 +40,7 @@ func DisplayDiffs(ctx context.Context, io iostreams.IOStreamer, diffs *DiffResul Emoji: "books", Text: "App Manifest", Secondary: []string{ - fmt.Sprintf("Found %d difference(s) between project and app settings", len(sorted)), + fmt.Sprintf("Found %d %s between project and app settings", len(sorted), style.Pluralize("difference", "differences", len(sorted))), }, })) From e585fa96ab0fa1e5083e1d68a3e2072b1d3776ab Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Sun, 14 Jun 2026 21:22:17 -0700 Subject: [PATCH 15/16] test: raise display.go coverage above 80% Adds focused test cases for the remaining uncovered branches: DisplayDiffs's empty-result early return and multi-diff sort path, formatValue's nil and long-value paths, and truncateRunes's short-max early return. The json.Marshal error path inside formatValue stays uncovered because reproducing it requires a value that the marshaller cannot encode (channels, funcs); the contortion is not worth the line of coverage. --- internal/manifest/display_test.go | 66 +++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/internal/manifest/display_test.go b/internal/manifest/display_test.go index 385dd539..d33a42f6 100644 --- a/internal/manifest/display_test.go +++ b/internal/manifest/display_test.go @@ -75,6 +75,29 @@ func Test_DisplayDiffs(t *testing.T) { }, forbiddenSubstrs: []string{"(only in", "Value:"}, }, + "multiple diffs are sorted by path and rendered in order": { + // Intentionally unsorted in the input. DisplayDiffs sorts by path, + // so display_information.name must appear before features.bot_user. + diffs: []FieldDiff{ + {Path: "features.bot_user.display_name", Type: DiffModified, LocalValue: "App", RemoteValue: "App (local)"}, + {Path: "display_information.name", Type: DiffModified, LocalValue: "App", RemoteValue: "App (local)"}, + }, + expectedSubstrs: []string{ + // Header reflects the count and uses singular/plural correctly. + "Found 2 differences between project and app settings", + "display_information.name", + "features.bot_user.display_name", + }, + }, + "empty result prints nothing": { + diffs: nil, + forbiddenSubstrs: []string{ + "Found", + "App Manifest", + "Project:", + "App settings:", + }, + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { @@ -95,6 +118,42 @@ func Test_DisplayDiffs(t *testing.T) { } } +func Test_formatValue(t *testing.T) { + tests := map[string]struct { + input any + expected string + }{ + "nil renders as (not present)": { + input: nil, + expected: "(not present)", + }, + "strings are quoted": { + input: "hello", + expected: `"hello"`, + }, + "booleans are JSON-encoded": { + input: false, + expected: "false", + }, + "long non-string values are rune-truncated": { + // Strings short-circuit through the `case string` arm and are + // quoted as-is with no truncation. Non-string values go + // through json.Marshal and then truncateRunes, so use a + // long array to exercise the truncation path. + input: []string{ + "alpha", "bravo", "charlie", "delta", "echo", "foxtrot", + "golf", "hotel", "india", "juliet", "kilo", + }, + expected: `["alpha","bravo","charlie","delta","echo","foxtrot","golf","hotel","india","j...`, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + assert.Equal(t, tc.expected, formatValue(tc.input)) + }) + } +} + func Test_truncateRunes(t *testing.T) { tests := map[string]struct { input string @@ -116,6 +175,13 @@ func Test_truncateRunes(t *testing.T) { max: 10, expected: "abcdefg...", }, + "max less than ellipsis budget returns input unchanged": { + // max <= 3 leaves no room for the "..." sentinel, so the + // helper short-circuits and returns the input as-is. + input: "abcdef", + max: 3, + expected: "abcdef", + }, "multi-byte runes are not cut mid-character": { // Each emoji is 4 bytes in UTF-8 but one rune. Byte-based // slicing would split the middle emoji. From 2093f5321a28aaa87bbe22833f3fd154a313d166 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Sun, 14 Jun 2026 22:07:27 -0700 Subject: [PATCH 16/16] refactor: rename display.go to diff_display.go The bare "display" name was ambiguous now that the package sits alongside cmd/manifest/info.go, which also "displays" a manifest. Prefix the file with the command it belongs to so a reader navigating the package can place the file at a glance. --- internal/manifest/{display.go => diff_display.go} | 0 internal/manifest/{display_test.go => diff_display_test.go} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename internal/manifest/{display.go => diff_display.go} (100%) rename internal/manifest/{display_test.go => diff_display_test.go} (100%) diff --git a/internal/manifest/display.go b/internal/manifest/diff_display.go similarity index 100% rename from internal/manifest/display.go rename to internal/manifest/diff_display.go diff --git a/internal/manifest/display_test.go b/internal/manifest/diff_display_test.go similarity index 100% rename from internal/manifest/display_test.go rename to internal/manifest/diff_display_test.go