From fb3aa95b3c723346546735b17ddf08175161bbdc Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Sat, 13 Jun 2026 16:14:35 -0700 Subject: [PATCH 01/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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{}