feat: add --manifest-source flag to run and deploy commands by srtaalej · Pull Request #630 · slackapi/slack-cli

srtaalej · 2026-08-10T17:49:58Z

Summary

Adds a --manifest-source=project|remote flag to slack run and slack deploy commands
When manifest sync detects differences during run/deploy, this flag allows non-interactive resolution without requiring slack manifest sync to be run separately
Skips the "Overwrite manifest on app settings?" confirmation prompt during install when --manifest-source is set — project auto-approves the overwrite, remote skips it entirely
Updates the non-TTY error remediation to reference --manifest-source instead of --force/--force-remote (which are only available on manifest sync)

Closes #628

Test plan

make lint passes
make test passes
Manual test: slack run --manifest-source=project pushes local manifest without prompting
Manual test: slack run --manifest-source=remote pulls app settings without prompting
Manual test: slack deploy --manifest-source=project works in non-TTY (CI) environments
Manual test: slack run --manifest-source=invalid returns a clear validation error
Manual test: slack run --manifest-source=remote skips the "Overwrite manifest?" prompt on reinstall

codecov · 2026-08-10T17:54:14Z

Codecov Report

❌ Patch coverage is 94.28571% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.22%. Comparing base (9b4235c) to head (6fc14df).

Files with missing lines	Patch %	Lines
internal/pkg/apps/install.go	50.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #630      +/-   ##
==========================================
+ Coverage   78.19%   78.22%   +0.03%     
==========================================
  Files         239      239              
  Lines       18149    18176      +27     
==========================================
+ Hits        14192    14219      +27     
  Misses       3957     3957

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

zimeg

@srtaalej I appreciate the most patient request for review 🙏 ✨ I'm requesting a few changes with hopes these comments move this in evermore stable directions. I call out:

Removing --force-remote flag altogether alongside this change: I'd like to avoid multiple options unless follow up is planned to remove this too?
Favoring the existing terms and implementations of manifest source: Our configuration file has some logic we might reuse here!

If I can share more to these please let me know! I'm optimistic we include this in upcoming release 🚀 🔮

zimeg · 2026-09-04T20:30:55Z

+				style.CommandText("--manifest-source=project / --force"),
+				style.CommandText("--manifest-source=remote / --force-remote"),


🪓 question: Are we alright to replace the --force and --force-remote options altogether while the sync command is under experiment?

yes i think thats a nice consolidation!

zimeg · 2026-09-04T20:32:42Z

+	ManifestSourceFlag      string
 	LogstashHostResolved    string


🧮 suggestion: Let's keep this in alphabetical order!

zimeg · 2026-09-04T20:35:21Z

+const (
+	ManifestSourceProject = "project"
+	ManifestSourceRemote  = "remote"
+)


Suggested change

const (

ManifestSourceProject = "project"

ManifestSourceRemote = "remote"

)

const (

ManifestSourceProject = "local"

ManifestSourceRemote = "remote"

)

🪬 suggestion(blocking): Earlier suggestion might've hinted at "project" terms but we should match existing configuration options I realize. Perhaps reusing logic from this package instead of validations here?

slack-cli/internal/config/manifest.go

Lines 20 to 23 in 73dde37

const (

ManifestSourceLocal ManifestSource = "local"

ManifestSourceRemote ManifestSource = "remote"

)

zimeg · 2026-09-04T20:38:34Z

+			if err := cmdutil.ValidateManifestSourceFlag(clients); err != nil {
+				return err
+			}


Suggested change

if err := cmdutil.ValidateManifestSourceFlag(clients); err != nil {

return err

}

🪓 quibble: I'd favor this validation happening with the switch case in internal/manifest/sync.go to avoid duplicate checks in code, although I understand this might error earlier.

we actually do need the check to happen in run and deploy as well otherwise invalid values in the flag will fall through

… flag

…nifest sync

zimeg

💬 @srtaalej Handfulls more comment around consolidating on config and flag options across commands! The PRs looking solid but I'd like to keep internal representations together with this!

zimeg · 2026-09-09T19:10:34Z

@@ -55,6 +55,7 @@ type Config struct {
 	ForceFlag               bool
 	ForceRemoteFlag         bool


Suggested change

ForceRemoteFlag bool

🪓 note: I'm eager to replace this as part of these changes while the initial implementation is under experiment still. IMHO a replacement of these instances makes safer review without handling multiple cases.

zimeg · 2026-09-09T19:50:05Z

 	var merged types.AppManifest
 	switch {
-	case clients.Config.ForceFlag:
+	case clients.Config.ManifestSourceFlag == string(config.ManifestSourceLocal) || clients.Config.ForceFlag:


Suggested change

case clients.Config.ManifestSourceFlag == string(config.ManifestSourceLocal) || clients.Config.ForceFlag:

case config.ManifestSource(clients.Config.ManifestSourceFlag).Equals(config.ManifestSourceLocal):

🪬 suggestion: Am hoping to compare objects without converting to string here.

zimeg · 2026-09-09T19:52:41Z

 		if err != nil {
 			return nil, err
 		}
 	case !clients.IO.IsTTY():


Suggested change

case config.ManifestSource(clients.Config.ManifestSourceFlag).Exists() && !config.ManifestSource(clients.Config.ManifestSourceFlag).IsValid():

return nil, slackerror.New(slackerror.ErrInvalidFlag).

WithMessage("Invalid value %q for %s flag", clients.Config.ManifestSourceFlag, style.CommandText("--manifest-source")).

WithRemediation("Valid values are %s or %s",

style.Highlight(string(config.ManifestSourceLocal)),

style.Highlight(string(config.ManifestSourceRemote)),

)

case !clients.IO.IsTTY():

🍾 suggestion: We might introduce a new "IsValid" method to the manifest source configurations to compare and error here?

zimeg · 2026-09-09T19:56:22Z

+	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)),
+			)
+	}
+


Suggested change

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)),

)

}

🪓 suggestion: Am hoping the checks below can cover this validation without causing issue?

🌲 note: I lean towards keeping validation in cmd overall which I might also have suggestion these files shouldn't be in internal... But that can be a later change!

zimeg · 2026-09-09T19:59:14Z

+// 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
+}


👁️‍🗨️ note: Adjacent comment suggests adding "IsValid" to the manifest source configurations that I think might be useful instead of flag specific checks here?

👾 note: I'm not so confident with PreRunE flag checks but am thinking that might return both truth values with an optional error:

func (ms ManifestSource) IsValid() (bool, error)

feat: add --manifest-source flag to run and deploy commands

b021d22

srtaalej requested a review from a team as a code owner August 10, 2026 17:49

srtaalej self-assigned this Aug 10, 2026

srtaalej added enhancement M-T: A feature request for new functionality semver:minor Use on pull requests to describe the release version increment labels Aug 10, 2026

srtaalej and others added 2 commits August 17, 2026 16:27

Merge branch 'main' into ale-manifest-source-flag

a8d89ea

fix: skip manifest overwrite prompt when --manifest-source is set

7ab29be

srtaalej requested a review from zimeg August 17, 2026 20:57

srtaalej added this to the Next Release milestone Aug 17, 2026

Merge branch 'main' into ale-manifest-source-flag

4837889

zimeg modified the milestones: v4.7.0, Next Release Aug 28, 2026

srtaalej and others added 2 commits September 4, 2026 15:34

fix: update tests and flag validation logic

6ec8dc7

Merge branch 'main' into ale-manifest-source-flag

d327435

zimeg requested changes Sep 4, 2026

View reviewed changes

srtaalej and others added 5 commits September 8, 2026 11:49

refactor: reuse config.ManifestSource constants for --manifest-source…

392db1e

… flag

refactor: replace --force/--force-remote with --manifest-source on ma…

a965364

…nifest sync

refactor: move --manifest-source validation into Sync()

958b568

Merge branch 'main' into ale-manifest-source-flag

edf1599

fix: linter error

0cf12c1

srtaalej requested review from zimeg and removed request for zimeg September 8, 2026 21:13

fix: restore --manifest-source validation in PreRunE for run and deploy

6fc14df

srtaalej requested a review from zimeg September 8, 2026 21:51

zimeg reviewed Sep 9, 2026

View reviewed changes

		style.CommandText("--manifest-source=project / --force"),
		style.CommandText("--manifest-source=remote / --force-remote"),

	const (
	ManifestSourceLocal ManifestSource = "local"
	ManifestSourceRemote ManifestSource = "remote"
	)

	if err := cmdutil.ValidateManifestSourceFlag(clients); err != nil {
	return err
	}

		@@ -55,6 +55,7 @@ type Config struct {
		ForceFlag bool
		ForceRemoteFlag bool

	case clients.Config.ManifestSourceFlag == string(config.ManifestSourceLocal) \|\| clients.Config.ForceFlag:
	case config.ManifestSource(clients.Config.ManifestSourceFlag).Equals(config.ManifestSourceLocal):

Uh oh!

Conversation

srtaalej commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

codecov Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

zimeg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zimeg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

srtaalej commented Aug 10, 2026 •

edited

Loading

codecov Bot commented Aug 10, 2026 •

edited

Loading