Skip to content

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

Open
srtaalej wants to merge 6 commits into
mainfrom
ale-manifest-source-flag
Open

feat: add --manifest-source flag to run and deploy commands#630
srtaalej wants to merge 6 commits into
mainfrom
ale-manifest-source-flag

Conversation

@srtaalej

@srtaalej srtaalej commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

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

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

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.30435% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.20%. Comparing base (73dde37) to head (d327435).

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.18%   78.20%   +0.01%     
==========================================
  Files         239      239              
  Lines       18136    18157      +21     
==========================================
+ Hits        14179    14199      +20     
- Misses       3957     3958       +1     

☔ 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.

@srtaalej srtaalej self-assigned this Aug 10, 2026
@srtaalej 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
srtaalej requested a review from zimeg August 17, 2026 20:57
@srtaalej srtaalej added this to the Next Release milestone Aug 17, 2026
@zimeg zimeg modified the milestones: v4.7.0, Next Release Aug 28, 2026

@zimeg zimeg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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 🚀 🔮

Comment thread internal/manifest/sync.go
Comment on lines +95 to +96
style.CommandText("--manifest-source=project / --force"),
style.CommandText("--manifest-source=remote / --force-remote"),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread internal/config/config.go
Comment on lines +57 to 58
ManifestSourceFlag string
LogstashHostResolved string

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread internal/cmdutil/flags.go
Comment on lines +41 to +44
const (
ManifestSourceProject = "project"
ManifestSourceRemote = "remote"
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

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

Comment thread cmd/platform/deploy.go
Comment on lines +62 to +64
if err := cmdutil.ValidateManifestSourceFlag(clients); err != nil {
return err
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement M-T: A feature request for new functionality semver:minor Use on pull requests to describe the release version increment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add --force/--force-remote flags to run and deploy commands

2 participants