feat: update 'collaborator' command to support read-only permission type by vegeris · Pull Request #272 · slackapi/slack-cli

vegeris · 2025-11-20T20:23:22Z

Summary

A while back we added support for setting some app collaborators as read-only for deployed apps but never released the experiment

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.

…geris-release-read-only-app-collabs

vegeris · 2025-11-20T20:25:46Z


-// promptCollaboratorsAddPermissionPrompts gathers the collaborator permission
-// from selection if the experiment allows
-func promptCollaboratorsAddPermissionPrompts(


Changed the behaviour here: before when adding a new collaborator we required the permission type to be set, but I think we can default to 'owner' unless otherwise specified

Sounds good!

Since we're now defaulting to the owner, we should update the description of the --permission-type to default to owner as well. I'll make a suggestion for that!

vegeris · 2025-11-20T20:26:33Z

+		slackUser.PermissionType, err = promptCollaboratorPermissionSelection(ctx, clients)
+		if err != nil {
+			return err
+		}


When updating an existing collaborator however, maybe it would make sense to prompt for the permission type rather than error out

If we choose to prompt, than we should handle non-TTY (no interactivity) as well. In that case, I think we would error when there is no flag value provided.

suggestion: Personally, I suggest we keep the error when the flag is not provided because it's been tested and works. If we switch to a prompt, I'd rather see it as a separate PR because it changes the functionality rather than simply unhides the experiment (purpose of this PR).

Sounds good; I'll flip this back to erroring out if the flag isn't provided. I'm fine with keeping that behaviour if the CLI team doesn't see adding the prompt as an enhancement to the user experience

codecov · 2025-11-20T20:27:38Z

Codecov Report

❌ Patch coverage is 81.25000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.85%. Comparing base (f768cc2) to head (ea994f2).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/collaborators/add.go	70.00%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #272      +/-   ##
==========================================
+ Coverage   64.73%   64.85%   +0.11%     
==========================================
  Files         213      213              
  Lines       17582    17550      -32     
==========================================
  Hits        11382    11382              
+ Misses       5134     5103      -31     
+ Partials     1066     1065       -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.

lukegalbraithrussell

docs changes are good!

mwbrooks

🙇🏻 Thanks for unhiding and releasing the read-only-collaborators experiment!

✏️ The majority of the changes look good.

I've left 2 suggestions that we should merge to display the --permission-type description consistent with other commands.
I've marked this as Request changes because I don't think we should introduce a prompt for permission types when updating collaborators. I'd a nice UX improvement, but it has a high risk of introducing bugs for non-interactivity environments. I'd rather see this added as a separate PR that we can review, test, and feel confident in.

mwbrooks · 2025-11-26T22:37:50Z


-// promptCollaboratorsAddPermissionPrompts gathers the collaborator permission
-// from selection if the experiment allows
-func promptCollaboratorsAddPermissionPrompts(


Sounds good!

Since we're now defaulting to the owner, we should update the description of the --permission-type to default to owner as well. I'll make a suggestion for that!

mwbrooks · 2025-11-26T22:57:51Z

+		slackUser.PermissionType, err = promptCollaboratorPermissionSelection(ctx, clients)
+		if err != nil {
+			return err
+		}


If we choose to prompt, than we should handle non-TTY (no interactivity) as well. In that case, I think we would error when there is no flag value provided.

suggestion: Personally, I suggest we keep the error when the flag is not provided because it's been tested and works. If we switch to a prompt, I'd rather see it as a separate PR because it changes the functionality rather than simply unhides the experiment (purpose of this PR).

mwbrooks · 2025-11-26T23:00:46Z

+	response, err := clients.IO.SelectPrompt(
+		ctx,
+		"Select a permission type",
+		permissionLabels,
+		iostreams.SelectPromptConfig{
+			Required: true,
+		},
+	)


suggestion: I think this needs to be more robust. The iostreams.SelectPromptConfig should be bound to the flag and the flag, so that non-interactive environments can work properly with the prompt.

Removed the prompt for now but will make note of this if we want to bring it back

Co-authored-by: Michael Brooks <mbrooks@slack-corp.com>

vegeris · 2025-11-27T16:50:20Z

 	case "reader":
 		return READER, nil
 	default:
-		return "", fmt.Errorf("invalid")


Updated this error message to be more specific

mwbrooks

✅ Thanks a bunch @vegeris for releasing this feature out of experimentation! 🚀

🧪 Manual tests and code review look good to me 👌🏻

unhide read-only collaborator setting for deployed apps

22ed643

vegeris changed the title ~~feat: Unhide the read-only app collaborator setting~~ feat: unhide read-only app collaborator setting Nov 20, 2025

Merge branch 'main' of https://github.com/slackapi/slack-cli into eve…

ccaf2df

…geris-release-read-only-app-collabs

vegeris commented Nov 20, 2025

View reviewed changes

vegeris changed the title ~~feat: unhide read-only app collaborator setting~~ feat: release read-only-collaborators experiment Nov 20, 2025

vegeris marked this pull request as ready for review November 20, 2025 20:39

vegeris requested review from a team as code owners November 20, 2025 20:39

update

1100c37

lukegalbraithrussell approved these changes Nov 20, 2025

View reviewed changes

vegeris added 2 commits November 20, 2025 17:08

update prompt wording

48841e0

Merge branch 'main' into evegeris-release-read-only-app-collabs

236dad9

vegeris requested review from mwbrooks and zimeg November 21, 2025 17:41

mwbrooks self-assigned this Nov 26, 2025

mwbrooks added enhancement M-T: A feature request for new functionality semver:minor Use on pull requests to describe the release version increment labels Nov 26, 2025

mwbrooks added this to the Next Release milestone Nov 26, 2025

Merge branch 'main' into evegeris-release-read-only-app-collabs

9c46e30

mwbrooks requested changes Nov 26, 2025

View reviewed changes

vegeris and others added 4 commits November 26, 2025 19:28

Update cmd/collaborators/add.go

6c020f7

Co-authored-by: Michael Brooks <mbrooks@slack-corp.com>

Update cmd/collaborators/update.go

8e68452

Co-authored-by: Michael Brooks <mbrooks@slack-corp.com>

error out when flag not provided

b12ed2b

update error message

fa544dd

vegeris commented Nov 27, 2025

View reviewed changes

vegeris requested a review from mwbrooks November 27, 2025 16:50

Merge branch 'main' into evegeris-release-read-only-app-collabs

ea994f2

mwbrooks changed the title ~~feat: release read-only-collaborators experiment~~ feat: update 'collaborator' command to support read-only permission type Nov 27, 2025

mwbrooks approved these changes Nov 27, 2025

View reviewed changes

mwbrooks merged commit e20c404 into main Nov 27, 2025
8 checks passed

mwbrooks deleted the evegeris-release-read-only-app-collabs branch November 27, 2025 22:27

Conversation

vegeris commented Nov 20, 2025

Summary

Requirements

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!

codecov Bot commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lukegalbraithrussell left a comment

Choose a reason for hiding this comment

Uh oh!

mwbrooks left a comment

Choose a reason for hiding this comment

Uh oh!

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mwbrooks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Nov 20, 2025 •

edited

Loading