From b021d22be253247aa90ecbe8a14d331cdbf01f06 Mon Sep 17 00:00:00 2001 From: Ale Mercado Date: Mon, 10 Aug 2026 13:49:44 -0400 Subject: [PATCH 1/8] feat: add --manifest-source flag to run and deploy commands --- cmd/platform/deploy.go | 4 +++ cmd/platform/run.go | 5 ++- internal/cmdutil/flags.go | 25 ++++++++++++++ internal/cmdutil/flags_test.go | 45 +++++++++++++++++++++++++ internal/config/config.go | 1 + internal/manifest/sync.go | 8 ++--- internal/manifest/sync_test.go | 60 ++++++++++++++++++++++++++++++++++ 7 files changed, 143 insertions(+), 5 deletions(-) diff --git a/cmd/platform/deploy.go b/cmd/platform/deploy.go index 88eed572..b96cde4c 100644 --- a/cmd/platform/deploy.go +++ b/cmd/platform/deploy.go @@ -59,6 +59,9 @@ func NewDeployCommand(clients *shared.ClientFactory) *cobra.Command { {Command: "platform deploy --team T0123456", Meaning: "Deploy to a specific team"}, }), PreRunE: func(cmd *cobra.Command, args []string) error { + if err := cmdutil.ValidateManifestSourceFlag(clients); err != nil { + return err + } return cmdutil.IsValidProjectDirectory(clients) }, RunE: func(cmd *cobra.Command, args []string) error { @@ -108,6 +111,7 @@ func NewDeployCommand(clients *shared.ClientFactory) *cobra.Command { } cmd.Flags().BoolVar(&deployFlags.hideTriggers, "hide-triggers", false, "do not list triggers and skip trigger creation prompts") + cmd.Flags().StringVar(&clients.Config.ManifestSourceFlag, "manifest-source", "", "resolve manifest differences using this source (project or remote)") cmd.Flags().StringVar(&deployFlags.orgGrantWorkspaceID, cmdutil.OrgGrantWorkspaceFlag, "", cmdutil.OrgGrantWorkspaceDescription()) return cmd diff --git a/cmd/platform/run.go b/cmd/platform/run.go index 35d258fa..b887ceed 100644 --- a/cmd/platform/run.go +++ b/cmd/platform/run.go @@ -58,7 +58,9 @@ func NewRunCommand(clients *shared.ClientFactory) *cobra.Command { {Command: "platform run --cleanup", Meaning: "Run a local development server with cleanup"}, }), PreRunE: func(cmd *cobra.Command, args []string) error { - // Verify command is run in a project directory + if err := cmdutil.ValidateManifestSourceFlag(clients); err != nil { + return err + } return cmdutil.IsValidProjectDirectory(clients) }, RunE: func(cmd *cobra.Command, args []string) error { @@ -70,6 +72,7 @@ func NewRunCommand(clients *shared.ClientFactory) *cobra.Command { cmd.Flags().StringVar(&runFlags.activityLevel, "activity-level", platform.ActivityMinLevelDefault, "activity level to display") cmd.Flags().BoolVar(&runFlags.noActivity, "no-activity", false, "hide Slack Platform log activity") cmd.Flags().BoolVar(&runFlags.cleanup, "cleanup", false, "uninstall the local app after exiting") + cmd.Flags().StringVar(&clients.Config.ManifestSourceFlag, "manifest-source", "", "resolve manifest differences using this source (project or remote)") cmd.Flags().StringVar(&runFlags.orgGrantWorkspaceID, cmdutil.OrgGrantWorkspaceFlag, "", cmdutil.OrgGrantWorkspaceDescription()) cmd.Flags().BoolVar(&runFlags.hideTriggers, "hide-triggers", false, "do not list triggers and skip trigger creation prompts") diff --git a/internal/cmdutil/flags.go b/internal/cmdutil/flags.go index cdbea12a..572edf2e 100644 --- a/internal/cmdutil/flags.go +++ b/internal/cmdutil/flags.go @@ -17,6 +17,8 @@ package cmdutil import ( "fmt" + "github.com/slackapi/slack-cli/internal/shared" + "github.com/slackapi/slack-cli/internal/slackerror" "github.com/slackapi/slack-cli/internal/style" "github.com/spf13/cobra" ) @@ -35,6 +37,29 @@ var OrgGrantWorkspaceDescription = func() string { style.Secondary("(or 'all' for all workspaces in the org)")) } +// ManifestSourceFlag values +const ( + ManifestSourceProject = "project" + ManifestSourceRemote = "remote" +) + +// ValidateManifestSourceFlag checks that --manifest-source has a valid value if set +func ValidateManifestSourceFlag(clients *shared.ClientFactory) error { + v := clients.Config.ManifestSourceFlag + if v == "" { + return nil + } + if v != ManifestSourceProject && v != ManifestSourceRemote { + return slackerror.New(slackerror.ErrInvalidFlag). + WithMessage("Invalid value %q for %s flag", v, style.CommandText("--manifest-source")). + WithRemediation("Valid values are %s or %s", + style.Highlight(ManifestSourceProject), + style.Highlight(ManifestSourceRemote), + ) + } + return nil +} + // IsFlagChanged checks if a certain flag has been set in the command func IsFlagChanged(cmd *cobra.Command, flag string) bool { IsFlagSet := cmd.Flags().Lookup(flag) diff --git a/internal/cmdutil/flags_test.go b/internal/cmdutil/flags_test.go index 2189bc95..7bfc6498 100644 --- a/internal/cmdutil/flags_test.go +++ b/internal/cmdutil/flags_test.go @@ -17,10 +17,55 @@ package cmdutil import ( "testing" + "github.com/slackapi/slack-cli/internal/config" + "github.com/slackapi/slack-cli/internal/shared" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +func Test_ValidateManifestSourceFlag(t *testing.T) { + tests := map[string]struct { + value string + expectErr bool + }{ + "flag not provided is valid": { + value: "", + expectErr: false, + }, + "project is valid": { + value: "project", + expectErr: false, + }, + "remote is valid": { + value: "remote", + expectErr: false, + }, + "invalid value returns error": { + value: "invalid", + expectErr: true, + }, + "local is not valid": { + value: "local", + expectErr: true, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + clients := &shared.ClientFactory{ + Config: &config.Config{ManifestSourceFlag: tc.value}, + } + err := ValidateManifestSourceFlag(clients) + if tc.expectErr { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.value) + } else { + require.NoError(t, err) + } + }) + } +} + func Test_IsFlagChanged(t *testing.T) { tests := map[string]struct { flag string diff --git a/internal/config/config.go b/internal/config/config.go index 979a0afe..e1112664 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -54,6 +54,7 @@ type Config struct { DisableTelemetryFlag bool ForceFlag bool ForceRemoteFlag bool + ManifestSourceFlag string LogstashHostResolved string NoColor bool RuntimeFlag string diff --git a/internal/manifest/sync.go b/internal/manifest/sync.go index feb30e15..bc7246a4 100644 --- a/internal/manifest/sync.go +++ b/internal/manifest/sync.go @@ -77,12 +77,12 @@ func Sync(ctx context.Context, clients *shared.ClientFactory, app types.App, aut var merged types.AppManifest switch { - case clients.Config.ForceFlag: + case clients.Config.ManifestSourceFlag == "project" || clients.Config.ForceFlag: merged, err = MergeAllFrom(localManifest.AppManifest, remoteManifest.AppManifest, diffs, MergeAllLocal) if err != nil { return nil, err } - case clients.Config.ForceRemoteFlag: + case clients.Config.ManifestSourceFlag == "remote" || clients.Config.ForceRemoteFlag: merged, err = MergeAllFrom(localManifest.AppManifest, remoteManifest.AppManifest, diffs, MergeAllRemote) if err != nil { return nil, err @@ -91,8 +91,8 @@ func Sync(ctx context.Context, clients *shared.ClientFactory, app types.App, aut return nil, slackerror.New(slackerror.ErrAppManifestUpdate). WithRemediation("Run %s interactively to resolve manifest differences, or pass %s to push the project manifest to app settings or %s to pull app settings to project", style.Commandf("manifest sync", false), - style.CommandText("--force"), - style.CommandText("--force-remote"), + style.CommandText("--manifest-source=project"), + style.CommandText("--manifest-source=remote"), ) default: merged, err = resolveInteractively(ctx, clients, localManifest.AppManifest, remoteManifest.AppManifest, diffs) diff --git a/internal/manifest/sync_test.go b/internal/manifest/sync_test.go index a9feeb6c..1cc9a244 100644 --- a/internal/manifest/sync_test.go +++ b/internal/manifest/sync_test.go @@ -220,6 +220,66 @@ func Test_Sync(t *testing.T) { assert.Equal(t, "Remote", result.Merged.DisplayInformation.Description) }) + t.Run("manifest-source=project merges all local and pushes to API", func(t *testing.T) { + f := newSyncTestFixture(t) + f.projectConfig.On("GetManifestSource", mock.Anything).Return(config.ManifestSourceLocal, nil) + f.manifestMock.On("GetManifestLocal", mock.Anything, mock.Anything, mock.Anything). + Return(localManifest, nil) + f.manifestMock.On("GetManifestRemote", mock.Anything, mock.Anything, mock.Anything). + Return(remoteManifest, nil) + f.clients.Config.ManifestSourceFlag = "project" + f.clientsMock.API.On("UpdateApp", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(api.UpdateAppResult{}, nil) + f.cacheMock.On("NewManifestHash", mock.Anything, mock.Anything).Return(cache.Hash("newhash"), nil) + f.cacheMock.On("SetManifestHash", mock.Anything, mock.Anything, mock.Anything).Return(nil) + _ = afero.WriteFile(f.fs, "/project/manifest.json", []byte(`{"display_information":{"name":"App"}}`), 0644) + + result, err := Sync(f.ctx, f.clients, testApp, testAuth) + + require.NoError(t, err) + require.NotNil(t, result) + assert.True(t, result.HasDifferences) + assert.Equal(t, "Local", result.Merged.DisplayInformation.Description) + }) + + t.Run("manifest-source=remote merges all remote and pushes to API", func(t *testing.T) { + f := newSyncTestFixture(t) + f.projectConfig.On("GetManifestSource", mock.Anything).Return(config.ManifestSourceLocal, nil) + f.manifestMock.On("GetManifestLocal", mock.Anything, mock.Anything, mock.Anything). + Return(localManifest, nil) + f.manifestMock.On("GetManifestRemote", mock.Anything, mock.Anything, mock.Anything). + Return(remoteManifest, nil) + f.clients.Config.ManifestSourceFlag = "remote" + f.clientsMock.API.On("UpdateApp", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(api.UpdateAppResult{}, nil) + f.cacheMock.On("NewManifestHash", mock.Anything, mock.Anything).Return(cache.Hash("newhash"), nil) + f.cacheMock.On("SetManifestHash", mock.Anything, mock.Anything, mock.Anything).Return(nil) + _ = afero.WriteFile(f.fs, "/project/manifest.json", []byte(`{"display_information":{"name":"App"}}`), 0644) + + result, err := Sync(f.ctx, f.clients, testApp, testAuth) + + require.NoError(t, err) + require.NotNil(t, result) + assert.True(t, result.HasDifferences) + assert.Equal(t, "Remote", result.Merged.DisplayInformation.Description) + }) + + t.Run("non-TTY error mentions --manifest-source in remediation", func(t *testing.T) { + f := newSyncTestFixture(t) + f.projectConfig.On("GetManifestSource", mock.Anything).Return(config.ManifestSourceLocal, nil) + f.manifestMock.On("GetManifestLocal", mock.Anything, mock.Anything, mock.Anything). + Return(localManifest, nil) + f.manifestMock.On("GetManifestRemote", mock.Anything, mock.Anything, mock.Anything). + Return(remoteManifest, nil) + + _, err := Sync(f.ctx, f.clients, testApp, testAuth) + + require.Error(t, err) + slackErr := slackerror.ToSlackError(err) + assert.Contains(t, slackErr.Remediation, "--manifest-source=project") + assert.Contains(t, slackErr.Remediation, "--manifest-source=remote") + }) + t.Run("API UpdateApp failure is propagated", func(t *testing.T) { f := newSyncTestFixture(t) f.projectConfig.On("GetManifestSource", mock.Anything).Return(config.ManifestSourceLocal, nil) From 7ab29be52f2601c98fb19e1f1afbbf212d4c92c5 Mon Sep 17 00:00:00 2001 From: Ale Mercado Date: Mon, 17 Aug 2026 16:50:41 -0400 Subject: [PATCH 2/8] fix: skip manifest overwrite prompt when --manifest-source is set --- internal/pkg/apps/install.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/pkg/apps/install.go b/internal/pkg/apps/install.go index 88f7237d..8b737b52 100644 --- a/internal/pkg/apps/install.go +++ b/internal/pkg/apps/install.go @@ -701,10 +701,10 @@ func shouldUpdateManifest(ctx context.Context, clients *shared.ClientFactory, ap if err != nil { return false, err } - if manifestSource.Equals(config.ManifestSourceRemote) { + if clients.Config.ManifestSourceFlag == "remote" || manifestSource.Equals(config.ManifestSourceRemote) { return false, nil } - if clients.Config.ForceFlag { + if clients.Config.ManifestSourceFlag == "project" || clients.Config.ForceFlag { return true, nil } manifest, err := clients.AppClient().Manifest.GetManifestLocal(ctx, clients.SDKConfig, clients.HookExecutor) From 6ec8dc76622103fceb269942b40069e8c5c0deb8 Mon Sep 17 00:00:00 2001 From: Ale Mercado Date: Fri, 4 Sep 2026 15:34:40 -0400 Subject: [PATCH 3/8] fix: update tests and flag validation logic --- cmd/platform/deploy_test.go | 18 ++++++++++++++++++ cmd/platform/run_test.go | 19 +++++++++++++++++++ internal/manifest/sync.go | 9 +++++---- internal/manifest/sync_test.go | 7 +++++-- internal/pkg/apps/install.go | 11 +++++++++-- 5 files changed, 56 insertions(+), 8 deletions(-) diff --git a/cmd/platform/deploy_test.go b/cmd/platform/deploy_test.go index 00a656e0..8c0e8f4e 100644 --- a/cmd/platform/deploy_test.go +++ b/cmd/platform/deploy_test.go @@ -106,6 +106,24 @@ func TestDeployCommand(t *testing.T) { deployPkgMock.AssertCalled(t, "Deploy", mock.Anything, mock.Anything, mock.Anything, mock.Anything) } +func TestDeployCommand_ManifestSourceFlag_InvalidValue(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + clientsMock := shared.NewClientsMock() + clientsMock.AddDefaultMocks() + clients := shared.NewClientFactory(clientsMock.MockClientFactory(), func(clients *shared.ClientFactory) { + clients.Config.ProjectConfig = config.NewProjectConfigMock() + clients.SDKConfig = hooks.NewSDKConfigMock() + }) + + cmd := NewDeployCommand(clients) + testutil.MockCmdIO(clients.IO, cmd) + cmd.SetArgs([]string{"--manifest-source", "invalid"}) + + err := cmd.ExecuteContext(ctx) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid") +} + func TestDeployCommand_HasValidDeploymentMethod(t *testing.T) { tests := map[string]struct { app types.App diff --git a/cmd/platform/run_test.go b/cmd/platform/run_test.go index 517b5c92..21777600 100644 --- a/cmd/platform/run_test.go +++ b/cmd/platform/run_test.go @@ -32,6 +32,7 @@ import ( "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" ) // Setup a mock for the package @@ -277,6 +278,24 @@ func TestRunCommand_Flags(t *testing.T) { } } +func TestRunCommand_ManifestSourceFlag_InvalidValue(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + clientsMock := shared.NewClientsMock() + clientsMock.IO.On("IsTTY").Return(true) + clientsMock.IO.AddDefaultMocks() + clients := shared.NewClientFactory(clientsMock.MockClientFactory(), func(clients *shared.ClientFactory) { + clients.SDKConfig = hooks.NewSDKConfigMock() + }) + + cmd := NewRunCommand(clients) + testutil.MockCmdIO(clients.IO, cmd) + cmd.SetArgs([]string{"--manifest-source", "invalid"}) + + err := cmd.ExecuteContext(ctx) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid") +} + func TestRunCommand_Help(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) clientsMock := shared.NewClientsMock() diff --git a/internal/manifest/sync.go b/internal/manifest/sync.go index bc7246a4..635f0c8d 100644 --- a/internal/manifest/sync.go +++ b/internal/manifest/sync.go @@ -18,6 +18,7 @@ import ( "context" "fmt" + "github.com/slackapi/slack-cli/internal/cmdutil" "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" @@ -77,12 +78,12 @@ func Sync(ctx context.Context, clients *shared.ClientFactory, app types.App, aut var merged types.AppManifest switch { - case clients.Config.ManifestSourceFlag == "project" || clients.Config.ForceFlag: + case clients.Config.ManifestSourceFlag == cmdutil.ManifestSourceProject || clients.Config.ForceFlag: merged, err = MergeAllFrom(localManifest.AppManifest, remoteManifest.AppManifest, diffs, MergeAllLocal) if err != nil { return nil, err } - case clients.Config.ManifestSourceFlag == "remote" || clients.Config.ForceRemoteFlag: + case clients.Config.ManifestSourceFlag == cmdutil.ManifestSourceRemote || clients.Config.ForceRemoteFlag: merged, err = MergeAllFrom(localManifest.AppManifest, remoteManifest.AppManifest, diffs, MergeAllRemote) if err != nil { return nil, err @@ -91,8 +92,8 @@ func Sync(ctx context.Context, clients *shared.ClientFactory, app types.App, aut return nil, slackerror.New(slackerror.ErrAppManifestUpdate). WithRemediation("Run %s interactively to resolve manifest differences, or pass %s to push the project manifest to app settings or %s to pull app settings to project", style.Commandf("manifest sync", false), - style.CommandText("--manifest-source=project"), - style.CommandText("--manifest-source=remote"), + style.CommandText("--manifest-source=project / --force"), + style.CommandText("--manifest-source=remote / --force-remote"), ) default: merged, err = resolveInteractively(ctx, clients, localManifest.AppManifest, remoteManifest.AppManifest, diffs) diff --git a/internal/manifest/sync_test.go b/internal/manifest/sync_test.go index 1cc9a244..862e8909 100644 --- a/internal/manifest/sync_test.go +++ b/internal/manifest/sync_test.go @@ -22,6 +22,7 @@ import ( "github.com/slackapi/slack-cli/internal/api" "github.com/slackapi/slack-cli/internal/app" "github.com/slackapi/slack-cli/internal/cache" + "github.com/slackapi/slack-cli/internal/cmdutil" "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/hooks" "github.com/slackapi/slack-cli/internal/iostreams" @@ -227,7 +228,7 @@ func Test_Sync(t *testing.T) { Return(localManifest, nil) f.manifestMock.On("GetManifestRemote", mock.Anything, mock.Anything, mock.Anything). Return(remoteManifest, nil) - f.clients.Config.ManifestSourceFlag = "project" + f.clients.Config.ManifestSourceFlag = cmdutil.ManifestSourceProject f.clientsMock.API.On("UpdateApp", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). Return(api.UpdateAppResult{}, nil) f.cacheMock.On("NewManifestHash", mock.Anything, mock.Anything).Return(cache.Hash("newhash"), nil) @@ -249,7 +250,7 @@ func Test_Sync(t *testing.T) { Return(localManifest, nil) f.manifestMock.On("GetManifestRemote", mock.Anything, mock.Anything, mock.Anything). Return(remoteManifest, nil) - f.clients.Config.ManifestSourceFlag = "remote" + f.clients.Config.ManifestSourceFlag = cmdutil.ManifestSourceRemote f.clientsMock.API.On("UpdateApp", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). Return(api.UpdateAppResult{}, nil) f.cacheMock.On("NewManifestHash", mock.Anything, mock.Anything).Return(cache.Hash("newhash"), nil) @@ -278,6 +279,8 @@ func Test_Sync(t *testing.T) { slackErr := slackerror.ToSlackError(err) assert.Contains(t, slackErr.Remediation, "--manifest-source=project") assert.Contains(t, slackErr.Remediation, "--manifest-source=remote") + assert.Contains(t, slackErr.Remediation, "--force") + assert.Contains(t, slackErr.Remediation, "--force-remote") }) t.Run("API UpdateApp failure is propagated", func(t *testing.T) { diff --git a/internal/pkg/apps/install.go b/internal/pkg/apps/install.go index 8b737b52..b6be8ad0 100644 --- a/internal/pkg/apps/install.go +++ b/internal/pkg/apps/install.go @@ -22,6 +22,7 @@ import ( "github.com/opentracing/opentracing-go" "github.com/slackapi/slack-cli/internal/api" + "github.com/slackapi/slack-cli/internal/cmdutil" "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/experiment" "github.com/slackapi/slack-cli/internal/icon" @@ -701,10 +702,16 @@ func shouldUpdateManifest(ctx context.Context, clients *shared.ClientFactory, ap if err != nil { return false, err } - if clients.Config.ManifestSourceFlag == "remote" || manifestSource.Equals(config.ManifestSourceRemote) { + if clients.Config.ManifestSourceFlag == cmdutil.ManifestSourceRemote { return false, nil } - if clients.Config.ManifestSourceFlag == "project" || clients.Config.ForceFlag { + if clients.Config.ManifestSourceFlag == cmdutil.ManifestSourceProject { + return true, nil + } + if manifestSource.Equals(config.ManifestSourceRemote) { + return false, nil + } + if clients.Config.ForceFlag { return true, nil } manifest, err := clients.AppClient().Manifest.GetManifestLocal(ctx, clients.SDKConfig, clients.HookExecutor) From 392db1e9cf42660a315376985cfb10dcc378f859 Mon Sep 17 00:00:00 2001 From: Ale Mercado Date: Tue, 8 Sep 2026 11:49:05 -0400 Subject: [PATCH 4/8] refactor: reuse config.ManifestSource constants for --manifest-source flag --- cmd/platform/deploy.go | 2 +- cmd/platform/run.go | 2 +- internal/cmdutil/flags.go | 13 ++++--------- internal/cmdutil/flags_test.go | 8 ++++---- internal/config/config.go | 2 +- internal/manifest/sync.go | 7 +++---- internal/manifest/sync_test.go | 9 ++++----- internal/pkg/apps/install.go | 5 ++--- 8 files changed, 20 insertions(+), 28 deletions(-) diff --git a/cmd/platform/deploy.go b/cmd/platform/deploy.go index b96cde4c..d19a9971 100644 --- a/cmd/platform/deploy.go +++ b/cmd/platform/deploy.go @@ -111,7 +111,7 @@ func NewDeployCommand(clients *shared.ClientFactory) *cobra.Command { } cmd.Flags().BoolVar(&deployFlags.hideTriggers, "hide-triggers", false, "do not list triggers and skip trigger creation prompts") - cmd.Flags().StringVar(&clients.Config.ManifestSourceFlag, "manifest-source", "", "resolve manifest differences using this source (project or remote)") + cmd.Flags().StringVar(&clients.Config.ManifestSourceFlag, "manifest-source", "", "resolve manifest differences using this source (local or remote)") cmd.Flags().StringVar(&deployFlags.orgGrantWorkspaceID, cmdutil.OrgGrantWorkspaceFlag, "", cmdutil.OrgGrantWorkspaceDescription()) return cmd diff --git a/cmd/platform/run.go b/cmd/platform/run.go index b887ceed..dcc683a4 100644 --- a/cmd/platform/run.go +++ b/cmd/platform/run.go @@ -72,7 +72,7 @@ func NewRunCommand(clients *shared.ClientFactory) *cobra.Command { cmd.Flags().StringVar(&runFlags.activityLevel, "activity-level", platform.ActivityMinLevelDefault, "activity level to display") cmd.Flags().BoolVar(&runFlags.noActivity, "no-activity", false, "hide Slack Platform log activity") cmd.Flags().BoolVar(&runFlags.cleanup, "cleanup", false, "uninstall the local app after exiting") - cmd.Flags().StringVar(&clients.Config.ManifestSourceFlag, "manifest-source", "", "resolve manifest differences using this source (project or remote)") + cmd.Flags().StringVar(&clients.Config.ManifestSourceFlag, "manifest-source", "", "resolve manifest differences using this source (local or remote)") cmd.Flags().StringVar(&runFlags.orgGrantWorkspaceID, cmdutil.OrgGrantWorkspaceFlag, "", cmdutil.OrgGrantWorkspaceDescription()) cmd.Flags().BoolVar(&runFlags.hideTriggers, "hide-triggers", false, "do not list triggers and skip trigger creation prompts") diff --git a/internal/cmdutil/flags.go b/internal/cmdutil/flags.go index 572edf2e..1eaac6c4 100644 --- a/internal/cmdutil/flags.go +++ b/internal/cmdutil/flags.go @@ -17,6 +17,7 @@ package cmdutil import ( "fmt" + "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/slackerror" "github.com/slackapi/slack-cli/internal/style" @@ -37,24 +38,18 @@ var OrgGrantWorkspaceDescription = func() string { style.Secondary("(or 'all' for all workspaces in the org)")) } -// ManifestSourceFlag values -const ( - ManifestSourceProject = "project" - ManifestSourceRemote = "remote" -) - // ValidateManifestSourceFlag checks that --manifest-source has a valid value if set func ValidateManifestSourceFlag(clients *shared.ClientFactory) error { v := clients.Config.ManifestSourceFlag if v == "" { return nil } - if v != ManifestSourceProject && v != ManifestSourceRemote { + if v != string(config.ManifestSourceLocal) && v != string(config.ManifestSourceRemote) { return slackerror.New(slackerror.ErrInvalidFlag). WithMessage("Invalid value %q for %s flag", v, style.CommandText("--manifest-source")). WithRemediation("Valid values are %s or %s", - style.Highlight(ManifestSourceProject), - style.Highlight(ManifestSourceRemote), + style.Highlight(string(config.ManifestSourceLocal)), + style.Highlight(string(config.ManifestSourceRemote)), ) } return nil diff --git a/internal/cmdutil/flags_test.go b/internal/cmdutil/flags_test.go index 7bfc6498..bc4727ba 100644 --- a/internal/cmdutil/flags_test.go +++ b/internal/cmdutil/flags_test.go @@ -33,8 +33,8 @@ func Test_ValidateManifestSourceFlag(t *testing.T) { value: "", expectErr: false, }, - "project is valid": { - value: "project", + "local is valid": { + value: "local", expectErr: false, }, "remote is valid": { @@ -45,8 +45,8 @@ func Test_ValidateManifestSourceFlag(t *testing.T) { value: "invalid", expectErr: true, }, - "local is not valid": { - value: "local", + "project is not valid": { + value: "project", expectErr: true, }, } diff --git a/internal/config/config.go b/internal/config/config.go index e1112664..ea732278 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -54,8 +54,8 @@ type Config struct { DisableTelemetryFlag bool ForceFlag bool ForceRemoteFlag bool - ManifestSourceFlag string LogstashHostResolved string + ManifestSourceFlag string NoColor bool RuntimeFlag string RuntimeName string diff --git a/internal/manifest/sync.go b/internal/manifest/sync.go index 635f0c8d..a6762dc6 100644 --- a/internal/manifest/sync.go +++ b/internal/manifest/sync.go @@ -18,7 +18,6 @@ import ( "context" "fmt" - "github.com/slackapi/slack-cli/internal/cmdutil" "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" @@ -78,12 +77,12 @@ func Sync(ctx context.Context, clients *shared.ClientFactory, app types.App, aut var merged types.AppManifest switch { - case clients.Config.ManifestSourceFlag == cmdutil.ManifestSourceProject || clients.Config.ForceFlag: + case clients.Config.ManifestSourceFlag == string(config.ManifestSourceLocal) || clients.Config.ForceFlag: merged, err = MergeAllFrom(localManifest.AppManifest, remoteManifest.AppManifest, diffs, MergeAllLocal) if err != nil { return nil, err } - case clients.Config.ManifestSourceFlag == cmdutil.ManifestSourceRemote || clients.Config.ForceRemoteFlag: + case clients.Config.ManifestSourceFlag == string(config.ManifestSourceRemote) || clients.Config.ForceRemoteFlag: merged, err = MergeAllFrom(localManifest.AppManifest, remoteManifest.AppManifest, diffs, MergeAllRemote) if err != nil { return nil, err @@ -92,7 +91,7 @@ func Sync(ctx context.Context, clients *shared.ClientFactory, app types.App, aut return nil, slackerror.New(slackerror.ErrAppManifestUpdate). WithRemediation("Run %s interactively to resolve manifest differences, or pass %s to push the project manifest to app settings or %s to pull app settings to project", style.Commandf("manifest sync", false), - style.CommandText("--manifest-source=project / --force"), + style.CommandText("--manifest-source=local / --force"), style.CommandText("--manifest-source=remote / --force-remote"), ) default: diff --git a/internal/manifest/sync_test.go b/internal/manifest/sync_test.go index 862e8909..d790afc4 100644 --- a/internal/manifest/sync_test.go +++ b/internal/manifest/sync_test.go @@ -22,7 +22,6 @@ import ( "github.com/slackapi/slack-cli/internal/api" "github.com/slackapi/slack-cli/internal/app" "github.com/slackapi/slack-cli/internal/cache" - "github.com/slackapi/slack-cli/internal/cmdutil" "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/hooks" "github.com/slackapi/slack-cli/internal/iostreams" @@ -221,14 +220,14 @@ func Test_Sync(t *testing.T) { assert.Equal(t, "Remote", result.Merged.DisplayInformation.Description) }) - t.Run("manifest-source=project merges all local and pushes to API", func(t *testing.T) { + t.Run("manifest-source=local merges all local and pushes to API", func(t *testing.T) { f := newSyncTestFixture(t) f.projectConfig.On("GetManifestSource", mock.Anything).Return(config.ManifestSourceLocal, nil) f.manifestMock.On("GetManifestLocal", mock.Anything, mock.Anything, mock.Anything). Return(localManifest, nil) f.manifestMock.On("GetManifestRemote", mock.Anything, mock.Anything, mock.Anything). Return(remoteManifest, nil) - f.clients.Config.ManifestSourceFlag = cmdutil.ManifestSourceProject + f.clients.Config.ManifestSourceFlag = string(config.ManifestSourceLocal) f.clientsMock.API.On("UpdateApp", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). Return(api.UpdateAppResult{}, nil) f.cacheMock.On("NewManifestHash", mock.Anything, mock.Anything).Return(cache.Hash("newhash"), nil) @@ -250,7 +249,7 @@ func Test_Sync(t *testing.T) { Return(localManifest, nil) f.manifestMock.On("GetManifestRemote", mock.Anything, mock.Anything, mock.Anything). Return(remoteManifest, nil) - f.clients.Config.ManifestSourceFlag = cmdutil.ManifestSourceRemote + f.clients.Config.ManifestSourceFlag = string(config.ManifestSourceRemote) f.clientsMock.API.On("UpdateApp", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). Return(api.UpdateAppResult{}, nil) f.cacheMock.On("NewManifestHash", mock.Anything, mock.Anything).Return(cache.Hash("newhash"), nil) @@ -277,7 +276,7 @@ func Test_Sync(t *testing.T) { require.Error(t, err) slackErr := slackerror.ToSlackError(err) - assert.Contains(t, slackErr.Remediation, "--manifest-source=project") + assert.Contains(t, slackErr.Remediation, "--manifest-source=local") assert.Contains(t, slackErr.Remediation, "--manifest-source=remote") assert.Contains(t, slackErr.Remediation, "--force") assert.Contains(t, slackErr.Remediation, "--force-remote") diff --git a/internal/pkg/apps/install.go b/internal/pkg/apps/install.go index b6be8ad0..460c30b6 100644 --- a/internal/pkg/apps/install.go +++ b/internal/pkg/apps/install.go @@ -22,7 +22,6 @@ import ( "github.com/opentracing/opentracing-go" "github.com/slackapi/slack-cli/internal/api" - "github.com/slackapi/slack-cli/internal/cmdutil" "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/experiment" "github.com/slackapi/slack-cli/internal/icon" @@ -702,10 +701,10 @@ func shouldUpdateManifest(ctx context.Context, clients *shared.ClientFactory, ap if err != nil { return false, err } - if clients.Config.ManifestSourceFlag == cmdutil.ManifestSourceRemote { + if clients.Config.ManifestSourceFlag == string(config.ManifestSourceRemote) { return false, nil } - if clients.Config.ManifestSourceFlag == cmdutil.ManifestSourceProject { + if clients.Config.ManifestSourceFlag == string(config.ManifestSourceLocal) { return true, nil } if manifestSource.Equals(config.ManifestSourceRemote) { From a965364247d6fa13f4ab922fd65622baff9194d4 Mon Sep 17 00:00:00 2001 From: Ale Mercado Date: Tue, 8 Sep 2026 16:51:24 -0400 Subject: [PATCH 5/8] refactor: replace --force/--force-remote with --manifest-source on manifest sync --- cmd/manifest/sync.go | 12 ++++++------ cmd/manifest/sync_test.go | 7 +++---- internal/manifest/sync.go | 4 ++-- internal/manifest/sync_test.go | 2 -- 4 files changed, 11 insertions(+), 14 deletions(-) diff --git a/cmd/manifest/sync.go b/cmd/manifest/sync.go index 3b78a359..6ea0e0ee 100644 --- a/cmd/manifest/sync.go +++ b/cmd/manifest/sync.go @@ -18,6 +18,7 @@ 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/config" "github.com/slackapi/slack-cli/internal/experiment" "github.com/slackapi/slack-cli/internal/manifest" "github.com/slackapi/slack-cli/internal/prompts" @@ -37,8 +38,8 @@ func NewSyncCommand(clients *shared.ClientFactory) *cobra.Command { Hidden: true, Example: style.ExampleCommandsf([]style.ExampleCommand{ {Command: "manifest sync", Meaning: "Sync project manifest with app settings"}, - {Command: "manifest sync --force", Meaning: "Push project manifest to app settings without prompting"}, - {Command: "manifest sync --force-remote", Meaning: "Pull app settings to project manifest without prompting"}, + {Command: "manifest sync --manifest-source=local", Meaning: "Push project manifest to app settings without prompting"}, + {Command: "manifest sync --manifest-source=remote", Meaning: "Pull app settings to project manifest without prompting"}, }), Args: cobra.NoArgs, PreRunE: func(cmd *cobra.Command, args []string) error { @@ -49,9 +50,8 @@ func NewSyncCommand(clients *shared.ClientFactory) *cobra.Command { style.CommandText("--experiment manifest-sync"), ) } - if clients.Config.ForceFlag && clients.Config.ForceRemoteFlag { - return slackerror.New(slackerror.ErrMismatchedFlags). - WithMessage("Cannot use both %s and %s flags", style.CommandText("--force"), style.CommandText("--force-remote")) + if err := cmdutil.ValidateManifestSourceFlag(clients); err != nil { + return err } return cmdutil.IsValidProjectDirectory(clients) }, @@ -71,6 +71,6 @@ func NewSyncCommand(clients *shared.ClientFactory) *cobra.Command { return err }, } - cmd.Flags().BoolVar(&clients.Config.ForceRemoteFlag, "force-remote", false, "use all app settings values without prompting") + cmd.Flags().StringVar(&clients.Config.ManifestSourceFlag, "manifest-source", "", "resolve manifest differences using this source ("+string(config.ManifestSourceLocal)+" or "+string(config.ManifestSourceRemote)+")") return cmd } diff --git a/cmd/manifest/sync_test.go b/cmd/manifest/sync_test.go index 2ca0da96..464f6b0c 100644 --- a/cmd/manifest/sync_test.go +++ b/cmd/manifest/sync_test.go @@ -45,15 +45,14 @@ func TestSyncCommand(t *testing.T) { // the gate itself should pass. ExpectedErrorStrings: []string{}, }, - "errors when both --force and --force-remote are set": { - CmdArgs: []string{"--force-remote"}, + "errors when --manifest-source has an invalid value": { + CmdArgs: []string{"--manifest-source=invalid"}, Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { cm.AddDefaultMocks() cf.Config.ExperimentsFlag = []string{string(experiment.ManifestSync)} cf.Config.LoadExperiments(ctx, cf.IO.PrintDebug) - cf.Config.ForceFlag = true }, - ExpectedErrorStrings: []string{"Cannot use both", "--force", "--force-remote"}, + ExpectedErrorStrings: []string{"Invalid value", "invalid", "--manifest-source"}, }, }, func(clients *shared.ClientFactory) *cobra.Command { return NewSyncCommand(clients) diff --git a/internal/manifest/sync.go b/internal/manifest/sync.go index a6762dc6..902fa976 100644 --- a/internal/manifest/sync.go +++ b/internal/manifest/sync.go @@ -91,8 +91,8 @@ func Sync(ctx context.Context, clients *shared.ClientFactory, app types.App, aut return nil, slackerror.New(slackerror.ErrAppManifestUpdate). WithRemediation("Run %s interactively to resolve manifest differences, or pass %s to push the project manifest to app settings or %s to pull app settings to project", style.Commandf("manifest sync", false), - style.CommandText("--manifest-source=local / --force"), - style.CommandText("--manifest-source=remote / --force-remote"), + style.CommandText("--manifest-source=local"), + style.CommandText("--manifest-source=remote"), ) default: merged, err = resolveInteractively(ctx, clients, localManifest.AppManifest, remoteManifest.AppManifest, diffs) diff --git a/internal/manifest/sync_test.go b/internal/manifest/sync_test.go index d790afc4..ec41163c 100644 --- a/internal/manifest/sync_test.go +++ b/internal/manifest/sync_test.go @@ -278,8 +278,6 @@ func Test_Sync(t *testing.T) { slackErr := slackerror.ToSlackError(err) assert.Contains(t, slackErr.Remediation, "--manifest-source=local") assert.Contains(t, slackErr.Remediation, "--manifest-source=remote") - assert.Contains(t, slackErr.Remediation, "--force") - assert.Contains(t, slackErr.Remediation, "--force-remote") }) t.Run("API UpdateApp failure is propagated", func(t *testing.T) { From 958b5685f1c642da3f964f1688e6fad7fe786603 Mon Sep 17 00:00:00 2001 From: Ale Mercado Date: Tue, 8 Sep 2026 17:05:32 -0400 Subject: [PATCH 6/8] refactor: move --manifest-source validation into Sync() --- cmd/manifest/sync.go | 3 --- cmd/manifest/sync_test.go | 9 ------- cmd/platform/deploy.go | 3 --- cmd/platform/deploy_test.go | 18 -------------- cmd/platform/run.go | 3 --- cmd/platform/run_test.go | 19 -------------- internal/cmdutil/flags.go | 20 --------------- internal/cmdutil/flags_test.go | 45 ---------------------------------- internal/manifest/sync.go | 11 ++++++++- internal/manifest/sync_test.go | 17 +++++++++++++ 10 files changed, 27 insertions(+), 121 deletions(-) diff --git a/cmd/manifest/sync.go b/cmd/manifest/sync.go index 6ea0e0ee..4cb4d551 100644 --- a/cmd/manifest/sync.go +++ b/cmd/manifest/sync.go @@ -50,9 +50,6 @@ func NewSyncCommand(clients *shared.ClientFactory) *cobra.Command { style.CommandText("--experiment manifest-sync"), ) } - if err := cmdutil.ValidateManifestSourceFlag(clients); err != nil { - return err - } return cmdutil.IsValidProjectDirectory(clients) }, RunE: func(cmd *cobra.Command, args []string) error { diff --git a/cmd/manifest/sync_test.go b/cmd/manifest/sync_test.go index 464f6b0c..63c12951 100644 --- a/cmd/manifest/sync_test.go +++ b/cmd/manifest/sync_test.go @@ -45,15 +45,6 @@ func TestSyncCommand(t *testing.T) { // the gate itself should pass. ExpectedErrorStrings: []string{}, }, - "errors when --manifest-source has an invalid value": { - CmdArgs: []string{"--manifest-source=invalid"}, - Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - cm.AddDefaultMocks() - cf.Config.ExperimentsFlag = []string{string(experiment.ManifestSync)} - cf.Config.LoadExperiments(ctx, cf.IO.PrintDebug) - }, - ExpectedErrorStrings: []string{"Invalid value", "invalid", "--manifest-source"}, - }, }, func(clients *shared.ClientFactory) *cobra.Command { return NewSyncCommand(clients) }) diff --git a/cmd/platform/deploy.go b/cmd/platform/deploy.go index d19a9971..d4b2638f 100644 --- a/cmd/platform/deploy.go +++ b/cmd/platform/deploy.go @@ -59,9 +59,6 @@ func NewDeployCommand(clients *shared.ClientFactory) *cobra.Command { {Command: "platform deploy --team T0123456", Meaning: "Deploy to a specific team"}, }), PreRunE: func(cmd *cobra.Command, args []string) error { - if err := cmdutil.ValidateManifestSourceFlag(clients); err != nil { - return err - } return cmdutil.IsValidProjectDirectory(clients) }, RunE: func(cmd *cobra.Command, args []string) error { diff --git a/cmd/platform/deploy_test.go b/cmd/platform/deploy_test.go index 8c0e8f4e..00a656e0 100644 --- a/cmd/platform/deploy_test.go +++ b/cmd/platform/deploy_test.go @@ -106,24 +106,6 @@ func TestDeployCommand(t *testing.T) { deployPkgMock.AssertCalled(t, "Deploy", mock.Anything, mock.Anything, mock.Anything, mock.Anything) } -func TestDeployCommand_ManifestSourceFlag_InvalidValue(t *testing.T) { - ctx := slackcontext.MockContext(t.Context()) - clientsMock := shared.NewClientsMock() - clientsMock.AddDefaultMocks() - clients := shared.NewClientFactory(clientsMock.MockClientFactory(), func(clients *shared.ClientFactory) { - clients.Config.ProjectConfig = config.NewProjectConfigMock() - clients.SDKConfig = hooks.NewSDKConfigMock() - }) - - cmd := NewDeployCommand(clients) - testutil.MockCmdIO(clients.IO, cmd) - cmd.SetArgs([]string{"--manifest-source", "invalid"}) - - err := cmd.ExecuteContext(ctx) - require.Error(t, err) - assert.Contains(t, err.Error(), "invalid") -} - func TestDeployCommand_HasValidDeploymentMethod(t *testing.T) { tests := map[string]struct { app types.App diff --git a/cmd/platform/run.go b/cmd/platform/run.go index dcc683a4..0ba4eccf 100644 --- a/cmd/platform/run.go +++ b/cmd/platform/run.go @@ -58,9 +58,6 @@ func NewRunCommand(clients *shared.ClientFactory) *cobra.Command { {Command: "platform run --cleanup", Meaning: "Run a local development server with cleanup"}, }), PreRunE: func(cmd *cobra.Command, args []string) error { - if err := cmdutil.ValidateManifestSourceFlag(clients); err != nil { - return err - } return cmdutil.IsValidProjectDirectory(clients) }, RunE: func(cmd *cobra.Command, args []string) error { diff --git a/cmd/platform/run_test.go b/cmd/platform/run_test.go index 21777600..517b5c92 100644 --- a/cmd/platform/run_test.go +++ b/cmd/platform/run_test.go @@ -32,7 +32,6 @@ import ( "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" ) // Setup a mock for the package @@ -278,24 +277,6 @@ func TestRunCommand_Flags(t *testing.T) { } } -func TestRunCommand_ManifestSourceFlag_InvalidValue(t *testing.T) { - ctx := slackcontext.MockContext(t.Context()) - clientsMock := shared.NewClientsMock() - clientsMock.IO.On("IsTTY").Return(true) - clientsMock.IO.AddDefaultMocks() - clients := shared.NewClientFactory(clientsMock.MockClientFactory(), func(clients *shared.ClientFactory) { - clients.SDKConfig = hooks.NewSDKConfigMock() - }) - - cmd := NewRunCommand(clients) - testutil.MockCmdIO(clients.IO, cmd) - cmd.SetArgs([]string{"--manifest-source", "invalid"}) - - err := cmd.ExecuteContext(ctx) - require.Error(t, err) - assert.Contains(t, err.Error(), "invalid") -} - func TestRunCommand_Help(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) clientsMock := shared.NewClientsMock() diff --git a/internal/cmdutil/flags.go b/internal/cmdutil/flags.go index 1eaac6c4..cdbea12a 100644 --- a/internal/cmdutil/flags.go +++ b/internal/cmdutil/flags.go @@ -17,9 +17,6 @@ package cmdutil import ( "fmt" - "github.com/slackapi/slack-cli/internal/config" - "github.com/slackapi/slack-cli/internal/shared" - "github.com/slackapi/slack-cli/internal/slackerror" "github.com/slackapi/slack-cli/internal/style" "github.com/spf13/cobra" ) @@ -38,23 +35,6 @@ var OrgGrantWorkspaceDescription = func() string { style.Secondary("(or 'all' for all workspaces in the org)")) } -// ValidateManifestSourceFlag checks that --manifest-source has a valid value if set -func ValidateManifestSourceFlag(clients *shared.ClientFactory) error { - v := clients.Config.ManifestSourceFlag - if v == "" { - return nil - } - if v != string(config.ManifestSourceLocal) && v != string(config.ManifestSourceRemote) { - return slackerror.New(slackerror.ErrInvalidFlag). - WithMessage("Invalid value %q for %s flag", v, style.CommandText("--manifest-source")). - WithRemediation("Valid values are %s or %s", - style.Highlight(string(config.ManifestSourceLocal)), - style.Highlight(string(config.ManifestSourceRemote)), - ) - } - return nil -} - // IsFlagChanged checks if a certain flag has been set in the command func IsFlagChanged(cmd *cobra.Command, flag string) bool { IsFlagSet := cmd.Flags().Lookup(flag) diff --git a/internal/cmdutil/flags_test.go b/internal/cmdutil/flags_test.go index bc4727ba..2189bc95 100644 --- a/internal/cmdutil/flags_test.go +++ b/internal/cmdutil/flags_test.go @@ -17,55 +17,10 @@ package cmdutil import ( "testing" - "github.com/slackapi/slack-cli/internal/config" - "github.com/slackapi/slack-cli/internal/shared" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) -func Test_ValidateManifestSourceFlag(t *testing.T) { - tests := map[string]struct { - value string - expectErr bool - }{ - "flag not provided is valid": { - value: "", - expectErr: false, - }, - "local is valid": { - value: "local", - expectErr: false, - }, - "remote is valid": { - value: "remote", - expectErr: false, - }, - "invalid value returns error": { - value: "invalid", - expectErr: true, - }, - "project is not valid": { - value: "project", - expectErr: true, - }, - } - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - clients := &shared.ClientFactory{ - Config: &config.Config{ManifestSourceFlag: tc.value}, - } - err := ValidateManifestSourceFlag(clients) - if tc.expectErr { - require.Error(t, err) - assert.Contains(t, err.Error(), tc.value) - } else { - require.NoError(t, err) - } - }) - } -} - func Test_IsFlagChanged(t *testing.T) { tests := map[string]struct { flag string diff --git a/internal/manifest/sync.go b/internal/manifest/sync.go index 902fa976..6c8d24c6 100644 --- a/internal/manifest/sync.go +++ b/internal/manifest/sync.go @@ -61,7 +61,7 @@ func Sync(ctx context.Context, clients *shared.ClientFactory, app types.App, aut diffs, err := Diff(localManifest.AppManifest, remoteManifest.AppManifest, app.IsDev) if err != nil { - return nil, fmt.Errorf("failed to compute manifest differences: %w", err) + return nil, fmt.Errorf("Failed to compute manifest differences: %w", err) } if !diffs.HasDifferences() { @@ -75,6 +75,15 @@ func Sync(ctx context.Context, clients *shared.ClientFactory, app types.App, aut DisplayDiffs(ctx, clients.IO, diffs) + if v := clients.Config.ManifestSourceFlag; v != "" && v != string(config.ManifestSourceLocal) && v != string(config.ManifestSourceRemote) { + return nil, slackerror.New(slackerror.ErrInvalidFlag). + WithMessage("Invalid value %q for %s flag", v, style.CommandText("--manifest-source")). + WithRemediation("Valid values are %s or %s", + style.Highlight(string(config.ManifestSourceLocal)), + style.Highlight(string(config.ManifestSourceRemote)), + ) + } + var merged types.AppManifest switch { case clients.Config.ManifestSourceFlag == string(config.ManifestSourceLocal) || clients.Config.ForceFlag: diff --git a/internal/manifest/sync_test.go b/internal/manifest/sync_test.go index ec41163c..78df65c0 100644 --- a/internal/manifest/sync_test.go +++ b/internal/manifest/sync_test.go @@ -264,6 +264,23 @@ func Test_Sync(t *testing.T) { assert.Equal(t, "Remote", result.Merged.DisplayInformation.Description) }) + t.Run("invalid manifest-source flag returns error", func(t *testing.T) { + f := newSyncTestFixture(t) + f.projectConfig.On("GetManifestSource", mock.Anything).Return(config.ManifestSourceLocal, nil) + f.manifestMock.On("GetManifestLocal", mock.Anything, mock.Anything, mock.Anything). + Return(localManifest, nil) + f.manifestMock.On("GetManifestRemote", mock.Anything, mock.Anything, mock.Anything). + Return(remoteManifest, nil) + f.clients.Config.ManifestSourceFlag = "invalid" + + result, err := Sync(f.ctx, f.clients, testApp, testAuth) + + require.Error(t, err) + assert.Nil(t, result) + assert.Contains(t, err.Error(), "invalid") + assert.Contains(t, err.Error(), "--manifest-source") + }) + t.Run("non-TTY error mentions --manifest-source in remediation", func(t *testing.T) { f := newSyncTestFixture(t) f.projectConfig.On("GetManifestSource", mock.Anything).Return(config.ManifestSourceLocal, nil) From 0cf12c19daeea7d404a7c43ce1912e43af0d75b8 Mon Sep 17 00:00:00 2001 From: Ale Mercado Date: Tue, 8 Sep 2026 17:09:24 -0400 Subject: [PATCH 7/8] fix: linter error --- internal/manifest/sync.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/manifest/sync.go b/internal/manifest/sync.go index 6c8d24c6..224881e5 100644 --- a/internal/manifest/sync.go +++ b/internal/manifest/sync.go @@ -61,7 +61,7 @@ func Sync(ctx context.Context, clients *shared.ClientFactory, app types.App, aut diffs, err := Diff(localManifest.AppManifest, remoteManifest.AppManifest, app.IsDev) if err != nil { - return nil, fmt.Errorf("Failed to compute manifest differences: %w", err) + return nil, fmt.Errorf("failed to compute manifest differences: %w", err) } if !diffs.HasDifferences() { From 6fc14dfe8e247a8a91b9fa4a7ae40a9af75cd440 Mon Sep 17 00:00:00 2001 From: Ale Mercado Date: Tue, 8 Sep 2026 17:46:03 -0400 Subject: [PATCH 8/8] fix: restore --manifest-source validation in PreRunE for run and deploy --- cmd/manifest/sync.go | 3 +++ cmd/manifest/sync_test.go | 9 +++++++ cmd/platform/deploy.go | 3 +++ cmd/platform/deploy_test.go | 18 ++++++++++++++ cmd/platform/run.go | 3 +++ cmd/platform/run_test.go | 19 ++++++++++++++ internal/cmdutil/flags.go | 20 +++++++++++++++ internal/cmdutil/flags_test.go | 45 ++++++++++++++++++++++++++++++++++ internal/manifest/sync.go | 18 +++++++------- 9 files changed, 129 insertions(+), 9 deletions(-) diff --git a/cmd/manifest/sync.go b/cmd/manifest/sync.go index 4cb4d551..6ea0e0ee 100644 --- a/cmd/manifest/sync.go +++ b/cmd/manifest/sync.go @@ -50,6 +50,9 @@ func NewSyncCommand(clients *shared.ClientFactory) *cobra.Command { style.CommandText("--experiment manifest-sync"), ) } + if err := cmdutil.ValidateManifestSourceFlag(clients); err != nil { + return err + } return cmdutil.IsValidProjectDirectory(clients) }, RunE: func(cmd *cobra.Command, args []string) error { diff --git a/cmd/manifest/sync_test.go b/cmd/manifest/sync_test.go index 63c12951..464f6b0c 100644 --- a/cmd/manifest/sync_test.go +++ b/cmd/manifest/sync_test.go @@ -45,6 +45,15 @@ func TestSyncCommand(t *testing.T) { // the gate itself should pass. ExpectedErrorStrings: []string{}, }, + "errors when --manifest-source has an invalid value": { + CmdArgs: []string{"--manifest-source=invalid"}, + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + cm.AddDefaultMocks() + cf.Config.ExperimentsFlag = []string{string(experiment.ManifestSync)} + cf.Config.LoadExperiments(ctx, cf.IO.PrintDebug) + }, + ExpectedErrorStrings: []string{"Invalid value", "invalid", "--manifest-source"}, + }, }, func(clients *shared.ClientFactory) *cobra.Command { return NewSyncCommand(clients) }) diff --git a/cmd/platform/deploy.go b/cmd/platform/deploy.go index d4b2638f..d19a9971 100644 --- a/cmd/platform/deploy.go +++ b/cmd/platform/deploy.go @@ -59,6 +59,9 @@ func NewDeployCommand(clients *shared.ClientFactory) *cobra.Command { {Command: "platform deploy --team T0123456", Meaning: "Deploy to a specific team"}, }), PreRunE: func(cmd *cobra.Command, args []string) error { + if err := cmdutil.ValidateManifestSourceFlag(clients); err != nil { + return err + } return cmdutil.IsValidProjectDirectory(clients) }, RunE: func(cmd *cobra.Command, args []string) error { diff --git a/cmd/platform/deploy_test.go b/cmd/platform/deploy_test.go index 00a656e0..8c0e8f4e 100644 --- a/cmd/platform/deploy_test.go +++ b/cmd/platform/deploy_test.go @@ -106,6 +106,24 @@ func TestDeployCommand(t *testing.T) { deployPkgMock.AssertCalled(t, "Deploy", mock.Anything, mock.Anything, mock.Anything, mock.Anything) } +func TestDeployCommand_ManifestSourceFlag_InvalidValue(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + clientsMock := shared.NewClientsMock() + clientsMock.AddDefaultMocks() + clients := shared.NewClientFactory(clientsMock.MockClientFactory(), func(clients *shared.ClientFactory) { + clients.Config.ProjectConfig = config.NewProjectConfigMock() + clients.SDKConfig = hooks.NewSDKConfigMock() + }) + + cmd := NewDeployCommand(clients) + testutil.MockCmdIO(clients.IO, cmd) + cmd.SetArgs([]string{"--manifest-source", "invalid"}) + + err := cmd.ExecuteContext(ctx) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid") +} + func TestDeployCommand_HasValidDeploymentMethod(t *testing.T) { tests := map[string]struct { app types.App diff --git a/cmd/platform/run.go b/cmd/platform/run.go index 0ba4eccf..dcc683a4 100644 --- a/cmd/platform/run.go +++ b/cmd/platform/run.go @@ -58,6 +58,9 @@ func NewRunCommand(clients *shared.ClientFactory) *cobra.Command { {Command: "platform run --cleanup", Meaning: "Run a local development server with cleanup"}, }), PreRunE: func(cmd *cobra.Command, args []string) error { + if err := cmdutil.ValidateManifestSourceFlag(clients); err != nil { + return err + } return cmdutil.IsValidProjectDirectory(clients) }, RunE: func(cmd *cobra.Command, args []string) error { diff --git a/cmd/platform/run_test.go b/cmd/platform/run_test.go index 517b5c92..21777600 100644 --- a/cmd/platform/run_test.go +++ b/cmd/platform/run_test.go @@ -32,6 +32,7 @@ import ( "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" ) // Setup a mock for the package @@ -277,6 +278,24 @@ func TestRunCommand_Flags(t *testing.T) { } } +func TestRunCommand_ManifestSourceFlag_InvalidValue(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + clientsMock := shared.NewClientsMock() + clientsMock.IO.On("IsTTY").Return(true) + clientsMock.IO.AddDefaultMocks() + clients := shared.NewClientFactory(clientsMock.MockClientFactory(), func(clients *shared.ClientFactory) { + clients.SDKConfig = hooks.NewSDKConfigMock() + }) + + cmd := NewRunCommand(clients) + testutil.MockCmdIO(clients.IO, cmd) + cmd.SetArgs([]string{"--manifest-source", "invalid"}) + + err := cmd.ExecuteContext(ctx) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid") +} + func TestRunCommand_Help(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) clientsMock := shared.NewClientsMock() diff --git a/internal/cmdutil/flags.go b/internal/cmdutil/flags.go index cdbea12a..098444db 100644 --- a/internal/cmdutil/flags.go +++ b/internal/cmdutil/flags.go @@ -17,6 +17,9 @@ package cmdutil import ( "fmt" + "github.com/slackapi/slack-cli/internal/config" + "github.com/slackapi/slack-cli/internal/shared" + "github.com/slackapi/slack-cli/internal/slackerror" "github.com/slackapi/slack-cli/internal/style" "github.com/spf13/cobra" ) @@ -35,6 +38,23 @@ var OrgGrantWorkspaceDescription = func() string { style.Secondary("(or 'all' for all workspaces in the org)")) } +// ValidateManifestSourceFlag checks that --manifest-source has a valid value if set +func ValidateManifestSourceFlag(clients *shared.ClientFactory) error { + v := clients.Config.ManifestSourceFlag + if v == "" { + return nil + } + if !(v == string(config.ManifestSourceLocal) || v == string(config.ManifestSourceRemote)) { + return slackerror.New(slackerror.ErrInvalidFlag). + WithMessage("Invalid value %q for %s flag", v, style.CommandText("--manifest-source")). + WithRemediation("Valid values are %s or %s", + style.Highlight(string(config.ManifestSourceLocal)), + style.Highlight(string(config.ManifestSourceRemote)), + ) + } + return nil +} + // IsFlagChanged checks if a certain flag has been set in the command func IsFlagChanged(cmd *cobra.Command, flag string) bool { IsFlagSet := cmd.Flags().Lookup(flag) diff --git a/internal/cmdutil/flags_test.go b/internal/cmdutil/flags_test.go index 2189bc95..bc4727ba 100644 --- a/internal/cmdutil/flags_test.go +++ b/internal/cmdutil/flags_test.go @@ -17,10 +17,55 @@ package cmdutil import ( "testing" + "github.com/slackapi/slack-cli/internal/config" + "github.com/slackapi/slack-cli/internal/shared" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +func Test_ValidateManifestSourceFlag(t *testing.T) { + tests := map[string]struct { + value string + expectErr bool + }{ + "flag not provided is valid": { + value: "", + expectErr: false, + }, + "local is valid": { + value: "local", + expectErr: false, + }, + "remote is valid": { + value: "remote", + expectErr: false, + }, + "invalid value returns error": { + value: "invalid", + expectErr: true, + }, + "project is not valid": { + value: "project", + expectErr: true, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + clients := &shared.ClientFactory{ + Config: &config.Config{ManifestSourceFlag: tc.value}, + } + err := ValidateManifestSourceFlag(clients) + if tc.expectErr { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.value) + } else { + require.NoError(t, err) + } + }) + } +} + func Test_IsFlagChanged(t *testing.T) { tests := map[string]struct { flag string diff --git a/internal/manifest/sync.go b/internal/manifest/sync.go index 224881e5..a7aed42d 100644 --- a/internal/manifest/sync.go +++ b/internal/manifest/sync.go @@ -36,6 +36,15 @@ type SyncResult struct { // both manifests, computes diffs, prompts the user for resolution, writes // the merged result to both the API and the local file, and returns the result. func Sync(ctx context.Context, clients *shared.ClientFactory, app types.App, auth types.SlackAuth) (*SyncResult, error) { + if v := clients.Config.ManifestSourceFlag; v != "" && !(v == string(config.ManifestSourceLocal) || v == string(config.ManifestSourceRemote)) { + return nil, slackerror.New(slackerror.ErrInvalidFlag). + WithMessage("Invalid value %q for %s flag", v, style.CommandText("--manifest-source")). + WithRemediation("Valid values are %s or %s", + style.Highlight(string(config.ManifestSourceLocal)), + style.Highlight(string(config.ManifestSourceRemote)), + ) + } + manifestSource, err := clients.Config.ProjectConfig.GetManifestSource(ctx) if err != nil { return nil, err @@ -75,15 +84,6 @@ func Sync(ctx context.Context, clients *shared.ClientFactory, app types.App, aut DisplayDiffs(ctx, clients.IO, diffs) - if v := clients.Config.ManifestSourceFlag; v != "" && v != string(config.ManifestSourceLocal) && v != string(config.ManifestSourceRemote) { - return nil, slackerror.New(slackerror.ErrInvalidFlag). - WithMessage("Invalid value %q for %s flag", v, style.CommandText("--manifest-source")). - WithRemediation("Valid values are %s or %s", - style.Highlight(string(config.ManifestSourceLocal)), - style.Highlight(string(config.ManifestSourceRemote)), - ) - } - var merged types.AppManifest switch { case clients.Config.ManifestSourceFlag == string(config.ManifestSourceLocal) || clients.Config.ForceFlag: