feat: Add flags for prompt values to upgrade command #87

kkemple · 2025-05-10T14:58:16Z

Summary of changes

Added a new boolean flag --auto-approve to the upgrade command
Added a new method InstallUpdatesWithoutPrompt to the UpdateNotification type for handling auto-approved updates
Modified the dependencies' PrintUpdateNotification methods to check for the flag and bypass confirmation
Updated command examples and help text to document the new flag

This PR enables:

Automated CLI upgrade workflows in CI/CD pipelines
Programmatic upgrades through the MCP server tools API
Simplified upgrades for users who always want the latest version
Better integration with scripts and other automation tools

The flag follows established patterns in the codebase for command options, with appropriate documentation and test coverage to ensure reliability.

Tests Added

Updated TestUpgradeCommand to verify behavior with and without the flag
Added a test case specifically for the auto-approve flag path
Ensured all existing tests continue to pass

Docs Updated

Added the flag to the command documentation in docs/reference/commands/slack_upgrade.md
Added an example demonstrating usage of the flag
Updated the options section to include the new flag with its description

Reasoning for implementation

The upgrade command needed a way to be run non-interactively, especially for automation and scripting use cases. The --auto-approve flag provides this functionality by skipping the confirmation prompt when installing updates.

While we could have modified the existing confirmation flow directly, creating a dedicated method provides better separation of concerns:

Keeps the standard interactive flow clean and focused
Makes the auto-approve path more explicit and maintainable
Reduces the risk of regression in either flow when changes are made

Requirements

I've read and understood the Contributing Guidelines and have done my best effort to follow them.
I've read and agree to the Code of Conduct.

codecov · 2025-05-10T15:25:42Z

Codecov Report

Attention: Patch coverage is 47.61905% with 33 lines in your changes missing coverage. Please review.

Project coverage is 63.15%. Comparing base (1129877) to head (24ec54b).

Files with missing lines	Patch %	Lines
internal/update/update.go	60.00%	10 Missing and 2 partials ⚠️
internal/update/cli.go	0.00%	8 Missing ⚠️
cmd/upgrade/upgrade.go	63.15%	7 Missing ⚠️
internal/update/sdk.go	0.00%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #87      +/-   ##
==========================================
- Coverage   63.17%   63.15%   -0.02%     
==========================================
  Files         210      210              
  Lines       22186    22242      +56     
==========================================
+ Hits        14015    14048      +33     
- Misses       7084     7108      +24     
+ Partials     1087     1086       -1

☔ View full report in Codecov by Sentry.
📢 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.

kkemple · 2025-05-10T15:47:37Z

Last commit improves error handling in the automatic updates flow by:

Fixing linting error in cmd/upgrade/upgrade.go by properly checking the error returned from updateNotification.InstallUpdatesWithoutPrompt() when the --auto-approve flag is used.
Adding comprehensive test coverage for error scenarios:
- New TestUpgradeCommandWithAutoApproveError test in cmd/upgrade/upgrade_test.go
  to verify error propagation when auto-approve is enabled
- New Test_Update_InstallUpdatesWithoutPrompt test in internal/update/update_test.go
  to test different error scenarios in the update notification process
Improved test structure to correctly model the early-return behavior of the InstallUpdatesWithoutPrompt method, ensuring mock expectations are properly set when errors occur.

These changes ensure that if an error occurs during automatic updates, it will
be properly propagated back to the user rather than silently ignored, improving
reliability and user experience.

mwbrooks

Great idea @kkemple and thanks for putting together a PR 👏🏻

We already have some conventions for disabling prompts and seeding a default answer. There are also some good conventions in the CLI community. So, I think I'd lean toward adopting those rather than introducing a new flag.

@zimeg Do you think this is a good place for the --no-prompt flag? We could also use the common convention of --yes | -y to default all prompts to "yes".

zimeg · 2025-05-12T19:52:12Z

@mwbrooks Great suggestions and I also think this is a good idea to have for imperative automations 👾

Do you think this is a good place for the --no-prompt flag?

With our current default prompt being "no" for the upgrade command, I don't think this would be ideal. AFAICT this flag should instead error with the suggested flag alternative instead of make the default choice?

To me --yes is solid and --auto-approve makes sense too! I'd avoid --force since we're not overriding warnings here.

One concern I have with both options is that we sometimes have multiple prompts when upgrading, one for the CLI version and one for the SDK. Without being so verbose, matching one flag to one prompt might be the most sure, though developers using --yes might understand the effects of automating updates.

Could these boolean flags for the prompt make sense?

$ slack upgrade --cli
$ slack upgrade --sdk

zimeg · 2025-05-12T19:54:49Z

Matching specific confirmation prompts might be an interesting pattern to use with something like this as well:

$ slack upgrade --confirm=cli --confirm=sdk

mwbrooks · 2025-05-12T22:27:09Z

Great point @zimeg that --no-prompt always chooses the default, while --yes or --no would answer ALL prompts with a yes or no.

One concern I have with both options is that we sometimes have multiple prompts when upgrading, one for the CLI version and one for the SDK.

You also bring up a good point that the upgrade command can have 2 prompts: CLI and SDK.

I did some research and common approaches are:

Skip all prompts using defaults: --no-prompt, --no-interactivity, --quiet
Answer all prompts with a value: --yes or --no
Answer each prompt with a specific value: --foo bar

So, I think we can come back to something that we discussed a good year ago: each prompt should have a flag.

In that case, something like this makes sense:

Upgrade CLI: --cli=<bool>
Upgrade SDK: --sdk=<bool>

kkemple · 2025-05-13T13:35:51Z

Replace --auto-approve flag with component-specific flags

This change allows users to selectively upgrade components rather than using a single auto-approve flag.

Implementation Review

✅ Flag Definition: Implemented with clear, descriptive flag names and helpful descriptions
✅ Component Logic: The InstallUpdatesWithComponentFlags method correctly selects which components to update based on flags
✅ Documentation: All examples, help text, and reference docs have been updated to reflect the new flags
✅ Tests: Comprehensive tests cover all combinations and edge cases
✅ Code Cleanup: Removed the unused InstallUpdatesWithoutPrompt method and its tests since it was only created in this

kkemple · 2025-05-13T13:59:04Z

Once we're code complete here and everything looks good, I'll work on the e2e test!

zimeg

👁️‍🗨️ Quick note on some confusion I'm finding in outputs when testing this. I hope to review the code changes soon, but want to make sure this is updating as expected first!

zimeg · 2025-05-19T17:13:14Z

.cursor-notepads/command-flags.md

📝 Before this merges I think we will want to make a separate PR for these rules.

It might also be worth considering our review process for these, but I'm favoring quick review with welcomed follow up. Saving more rambles for the other PR though 🫡

zimeg · 2025-05-19T17:30:18Z

cmd/upgrade/upgrade.go

 		}, "\n"),
 		Example: style.ExampleCommandsf([]style.ExampleCommand{
 			{Command: "upgrade", Meaning: "Check for any available updates"},
+			{Command: "upgrade --cli", Meaning: "Check for CLI updates and automatically upgrade without confirmation"},


🤔 For some reason this flag is causing me to update twice which doesn't seem to be happening in earlier versions. This might be due to my setups, but please let me know if this is unexpected!

$ lack update --cli 🌱 A new version of the Slack CLI is available: v3.1.0-26-g24ec54b → 3.1.0 You can read the release notes at: https://docs.slack.dev/changelog To manually update, visit the download page: https://tools.slack.dev/slack-cli ✓ Installing update automatically... Starting the auto-update... Downloading version 3.1.0... Found current install path: /Users/eden.zimbelman/programming/tools/slack-cli/bin/slack Extracting the download: /Users/eden.zimbelman/.slack/bin/3.1.0 Backing up current install: /Users/eden.zimbelman/.slack/bin/backups/v3.1.0-26-g24ec54b Updating to version 3.1.0: /Users/eden.zimbelman/programming/tools/slack-cli/bin/slack Successfully updated to version 3.1.0 Verifying the update... 🌱 A new version of the Slack CLI is available: v3.1.0-26-g24ec54b → 3.1.0 You can read the release notes at: https://docs.slack.dev/changelog To manually update, visit the download page: https://tools.slack.dev/slack-cli Starting the auto-update... Downloading version 3.1.0... Found current install path: /Users/eden.zimbelman/programming/tools/slack-cli/bin/slack Extracting the download: /Users/eden.zimbelman/.slack/bin/3.1.0 Backing up current install: /Users/eden.zimbelman/.slack/bin/backups/v3.1.0-26-g24ec54b Updating to version 3.1.0: /Users/eden.zimbelman/programming/tools/slack-cli/bin/slack Successfully updated to version 3.1.0 Verifying the update... $ lack version Using lack v3.1.0

back on this today! will have to try it out locally and see what's going on!

lukegalbraithrussell · 2025-05-22T15:21:54Z

docs/reference/commands/slack_upgrade.md

+      --cli            automatically approve and install CLI updates without prompting
+  -h, --help           help for upgrade
+      --sdk            automatically approve and install SDK updates without prompting
 ```


these docs are auto-generated, updating in source will update this page

kkemple requested review from a team as code owners May 10, 2025 14:58

kkemple marked this pull request as draft May 10, 2025 15:01

kkemple added the enhancement M-T: A feature request for new functionality label May 10, 2025

mwbrooks reviewed May 12, 2025

View reviewed changes

kkemple changed the title ~~feat: Add --auto-approve flag to upgrade command~~ feat: Add flags for prompt values to upgrade command May 13, 2025

kkemple requested a review from mwbrooks May 13, 2025 13:59

kkemple added 9 commits May 14, 2025 08:55

feat: Add --auto-approve flag to upgrade command

b7ae43a

fix: properly check error return value in upgrade command

b1a0d39

fix: properly handle errors in upgrade auto-approve path

58e6775

move to two flag approach to match prompt

85efa63

remove unused function and related tests

6f5e67a

add license headers to cursor rules and accompanying rule

4a79622

rules set to always not applied to chat, changed to auto attach with *.*

4dc8226

add missing license headers

5c2a152

clean up rules

24ec54b

kkemple force-pushed the add-new-upgrade-command-flag branch from 21fe3f7 to 24ec54b Compare May 14, 2025 12:56

kkemple self-assigned this May 15, 2025

zimeg reviewed May 19, 2025

View reviewed changes

lukegalbraithrussell reviewed May 22, 2025

View reviewed changes

feat: Add flags for prompt values to upgrade command #87

Are you sure you want to change the base?

feat: Add flags for prompt values to upgrade command #87

Uh oh!

Conversation

kkemple commented May 10, 2025

Summary of changes

Tests Added

Docs Updated

Reasoning for implementation

Requirements

Uh oh!

codecov bot commented May 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kkemple commented May 10, 2025

Uh oh!

mwbrooks left a comment

Choose a reason for hiding this comment

Uh oh!

zimeg commented May 12, 2025

Uh oh!

zimeg commented May 12, 2025

Uh oh!

mwbrooks commented May 12, 2025

Uh oh!

kkemple commented May 13, 2025

Replace --auto-approve flag with component-specific flags

Implementation Review

Uh oh!

kkemple commented May 13, 2025

Uh oh!

zimeg left a comment

Choose a reason for hiding this comment

Uh oh!

zimeg May 19, 2025

Choose a reason for hiding this comment

Uh oh!

zimeg May 19, 2025

Choose a reason for hiding this comment

Uh oh!

kkemple Jun 16, 2025

Choose a reason for hiding this comment

Uh oh!

lukegalbraithrussell May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov bot commented May 10, 2025 •

edited

Loading

lukegalbraithrussell May 22, 2025 •

edited

Loading