feat: support SLACK_CLI_APP_ICON_PATH env var for icon override by srtaalej · Pull Request #519 · slackapi/slack-cli

srtaalej · 2026-05-01T18:57:01Z

Changelog

Add SLACK_CLI_APP_ICON_PATH environment variable to override the icon file path used during app install and local run. When set, the specified path takes priority over the manifest icon field and the default icon file search. A warning is displayed if the configured path does not exist.

Summary

Add SLACK_CLI_APP_ICON_PATH environment variable support to override the app icon path at install/deploy time
Enables setting different icons per environment (dev/staging/prod) without changing project files
Priority chain: SLACK_CLI_APP_ICON_PATH env var > manifest icon field > assets/ and root fallback (via icon.ResolveIconPath)
Extracts resolveIconPath wrapper in install.go that adds env var priority + path validation with warnings on top of icon.ResolveIconPath

Test plan

Unit tests

go test ./internal/config/ -run Test_DotEnv — env var loading (set, empty, whitespace trimming)
go test ./internal/pkg/apps/ -run Test_resolveIconPath — priority chain scenarios (env var, manifest, assets fallback, root fallback, missing paths)

Manual testing

From a Slack app project directory:

Override via env var

export SLACK_CLI_APP_ICON_PATH=icon.png
./bin/slack run -e set-icon

Confirm output shows Updated app icon: icon.png

Invalid env var path shows warning
```
export SLACK_CLI_APP_ICON_PATH=/nonexistent/icon.png
./bin/slack run -e set-icon
```
Confirm warning: icon path from SLACK_CLI_APP_ICON_PATH not found
Manifest icon field used when env var unset
```
unset SLACK_CLI_APP_ICON_PATH
./bin/slack run -e set-icon
```
If manifest returns "icon": "path/to/icon.png", confirm that path is used
Fallback to assets/icon.png or icon.png in project root
With no env var and no manifest icon field, confirm icon auto-detection finds assets/icon.png (or icon.png in root)

The icon upload fallback only checked for icon.png in the project root. This adds assets/icon.png as the first fallback path, matching the convention used by slack-samples template repos and the existing auto-detection in slack_yaml.go.

Extract resolveIconPath to centralize icon fallback logic across Install and InstallLocalApp. Search for icon.{png,jpg,jpeg,gif} in assets/ then project root, matching all formats the API accepts.

Consolidate duplicated icon path resolution logic from install.go and slack_yaml.go into a shared internal/icon package per review feedback.

Allow users to override the app icon path via the SLACK_CLI_APP_ICON_PATH environment variable, enabling different icons per environment without changing project files. Priority chain: env var > manifest icon > icon.png in project root. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

codecov · 2026-05-01T19:14:56Z

Codecov Report

❌ Patch coverage is 94.73684% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 71.35%. Comparing base (02805c7) to head (ac8245a).

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

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #519      +/-   ##
==========================================
+ Coverage   71.32%   71.35%   +0.03%     
==========================================
  Files         222      222              
  Lines       18714    18731      +17     
==========================================
+ Hits        13348    13366      +18     
- Misses       4186     4187       +1     
+ Partials     1180     1178       -2

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

zimeg

@srtaalej Sweeeet this is getting to a promising place 📸

I'm leaving a few comments on fallback behavior and organization before testing more but I'd love to hear what you think about some of these notes?

zimeg · 2026-05-04T03:05:51Z

+			return envIconPath
+		}
+		clients.IO.PrintDebug(ctx, "SLACK_CLI_APP_ICON_PATH file not found: %s", envIconPath)
+		_, _ = clients.IO.WriteOut().Write([]byte(style.SectionSecondaryf("Warning: icon path from SLACK_CLI_APP_ICON_PATH not found: %s", envIconPath)))


⚠️ issue: Falling through might be unexpected here if the SLACK_CLI_APP_ICON_PATH is set. IMHO we should print the warning and return no value instead?

zimeg · 2026-05-04T03:06:41Z

+		if _, err := os.Stat(envIconPath); !os.IsNotExist(err) {
+			return envIconPath
+		}


🪬 question: Is this a check we should also include for manifestIcon values?

zimeg · 2026-05-04T03:10:49Z

-	// upload icon, default to icon.png
-	var iconPath = slackManifest.Icon
-	if iconPath == "" {
-		if _, err := os.Stat("icon.png"); !os.IsNotExist(err) {
-			iconPath = "icon.png"
-		}
-	}


📸 thought: Keeping this inline might be alright to avoided redirected logic?

⛈️ ramble: I might suggest we instead also extract the "upload" logic that follows this since it's duplicated too, but I'd much prefer the Install and InstallLocalApp functions be merged because the logic is almost identical if this is changing-

zimeg · 2026-05-04T03:19:04Z

 	}

+	// Load app icon path from environment variables
+	var appIconPath = strings.TrimSpace(c.os.Getenv(slackAppIconPathEnv))


📠 issue: This might not capture values from the .env file also. I'm wondering if this extends to other values in this file so perhaps not a blocker?

yup all other env vars are read from system !

zimeg · 2026-05-04T03:21:47Z

 	APIHostFlag             string
 	APIHostResolved         string
 	AppFlag                 string
+	AppIconPathFlag         string


👾 thought: Interesting to find "app icon path" reads more clear than "CLI app icon path" but this ought not block decision on:

- SLACK_APP_ICON_PATH + SLACK_CLI_APP_ICON_PATH

🌲 ramble: I'm curious now again if "file" path needs to be specified in these values? Curious if path is clear enough on it's own...

i can change it to CLIAppIconPath! i was matching it to the other vars here but it doesnt have to

Co-authored-by: Eden Zimbelman <eden.zimbelman@salesforce.com>

- Update constant reference to slackCLIAppIconPathEnv - Stop falling through to manifest/icon.png when env var file is missing - Add file existence check for manifest icon values - Update tests for new behavior

zimeg

👋 @srtaalej Sharing another fallback case to return for but I'm curious still if we're wanting to read from ".env" files?

Co-authored-by: Eden Zimbelman <eden.zimbelman@salesforce.com>

zimeg

Baseline — default icon via slack run
./bin/slack run -e set-icon
Confirm output shows Updated app icon: assets/icon.png

⚠️ issue: When using the changes of slack-samples/bolt-js-starter-template#145 I don't find the "assets/icon.png" is uploaded as expected?

$ slack create --branch add-app-icon -t slack-samples/bolt-js-starter-template
$ slack version
Using slack v4.0.1-ale-icon-env-var-15-g2778c01

Would this be a change for this PR or can I test this with another pattern? 💡

zimeg · 2026-05-05T21:33:09Z

📚 suggestion: Would be good to note this as changelog as well for folks that've been uploading icons prior!

srtaalej · 2026-05-07T15:59:17Z

Baseline — default icon via slack run
./bin/slack run -e set-icon
Confirm output shows Updated app icon: assets/icon.png
⚠️ issue: When using the changes of slack-samples/bolt-js-starter-template#145 I don't find the "assets/icon.png" is uploaded as expected?
$ slack create --branch add-app-icon -t slack-samples/bolt-js-starter-template
$ slack version
Using slack v4.0.1-ale-icon-env-var-15-g2778c01
Would this be a change for this PR or can I test this with another pattern? 💡

@zimeg thanks for raising this that was an issue with the pr description. this pr doesnt check for assets/, that gets added in 509 actually. i updated the pr description! but i think i should consolidate that check into this PR to since resolveIconPath function is in diff places in both PRs - 509 was updated with that before I opened 519 😿

Merge ale-icon-assets-fallback into ale-icon-env-var. The resolveIconPath wrapper now delegates to icon.ResolveIconPath for fallback search, gaining assets/ directory support and multiple extensions (.png, .jpg, .jpeg, .gif). Tests updated to use afero.Fs mock instead of os.Chdir.

Take main's canonical versions of icon.go, icon_test.go, and slack_yaml.go. Keep resolveIconPath wrapper in install.go that adds SLACK_CLI_APP_ICON_PATH env var priority on top of icon.ResolveIconPath.

zimeg

📸 @srtaalej LGTM! Thanks for separating these changes for more focused PRs!

Approving now with confidence and curiousness of potential code improvements to make, but nothing's blocking!

zimeg · 2026-05-12T17:25:06Z

+		_, _ = clients.IO.WriteOut().Write([]byte(style.SectionSecondaryf("Warning: icon path from manifest not found: %s", manifestIcon)))
+		return ""
+	}
+	return icon.ResolveIconPath(clients.Fs, "")


Suggested change

return icon.ResolveIconPath(clients.Fs, "")

return icon.ResolveIconPath(clients.Fs)

🪓 suggestion: Let's remove arguments that aren't used as part of these changes!

🪬 ramble: It's confusing to have multiple functions to resolve icon path functions IMHO and might consider the one of this file as a source of truth because we're writing output here. I'd be curious in refactoring the rest of the icon package inline if that's reasonable but no blocker for this PR!

srtaalej and others added 5 commits April 27, 2026 15:45

feat: support png, jpg, jpeg, gif for icon auto-detection

5176129

Extract resolveIconPath to centralize icon fallback logic across Install and InstallLocalApp. Search for icon.{png,jpg,jpeg,gif} in assets/ then project root, matching all formats the API accepts.

refactor: extract icon resolution to internal/icon package

c9abb30

Consolidate duplicated icon path resolution logic from install.go and slack_yaml.go into a shared internal/icon package per review feedback.

Merge branch 'main' into ale-icon-assets-fallback

14c811f

srtaalej requested a review from a team as a code owner May 1, 2026 18:57

merge: resolve conflict with main in install_test.go

95ded37

Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>

zimeg reviewed May 4, 2026

View reviewed changes

srtaalej and others added 2 commits May 4, 2026 13:32

Update internal/config/config.go

d7e7ccd

Co-authored-by: Eden Zimbelman <eden.zimbelman@salesforce.com>

fix: address review feedback for icon path resolution

d599052

- Update constant reference to slackCLIAppIconPathEnv - Stop falling through to manifest/icon.png when env var file is missing - Add file existence check for manifest icon values - Update tests for new behavior

srtaalej requested a review from zimeg May 4, 2026 18:58

Merge branch 'main' into ale-icon-assets-fallback

8df3140

zimeg reviewed May 4, 2026

View reviewed changes

Comment thread internal/pkg/apps/install.go

srtaalej and others added 3 commits May 5, 2026 11:05

Update internal/pkg/apps/install.go

87a4646

Co-authored-by: Eden Zimbelman <eden.zimbelman@salesforce.com>

fix: update test to match manifest icon early-return behavior

061e54f

Merge branch 'main' into ale-icon-env-var

2778c01

srtaalej requested a review from zimeg May 5, 2026 18:19

srtaalej self-assigned this May 5, 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 May 5, 2026

srtaalej added this to the Next Release milestone May 5, 2026

zimeg reviewed May 5, 2026

View reviewed changes

srtaalej added the changelog Use on updates to be included in the release notes label May 7, 2026

srtaalej marked this pull request as draft May 7, 2026 16:29

srtaalej added 2 commits May 7, 2026 12:38

merge: resolve conflicts with main after #509 squash merge

805fdaf

Take main's canonical versions of icon.go, icon_test.go, and slack_yaml.go. Keep resolveIconPath wrapper in install.go that adds SLACK_CLI_APP_ICON_PATH env var priority on top of icon.ResolveIconPath.

srtaalej force-pushed the ale-icon-env-var branch from a044dbe to 805fdaf Compare May 12, 2026 14:51

Merge branch 'main' into ale-icon-env-var

ac8245a

srtaalej marked this pull request as ready for review May 12, 2026 16:24

srtaalej requested review from mwbrooks and zimeg May 12, 2026 16:24

zimeg approved these changes May 12, 2026

View reviewed changes

	return icon.ResolveIconPath(clients.Fs, "")
	return icon.ResolveIconPath(clients.Fs)

Conversation

srtaalej commented May 1, 2026 • edited by zimeg Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog

Summary

Test plan

Unit tests

Manual testing

Uh oh!

codecov Bot commented May 1, 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!

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!

Uh oh!

zimeg left a comment

Choose a reason for hiding this comment

Uh oh!

zimeg commented May 5, 2026

Uh oh!

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

Uh oh!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

srtaalej commented May 1, 2026 •

edited by zimeg

Loading

codecov Bot commented May 1, 2026 •

edited

Loading

srtaalej commented May 7, 2026 •

edited

Loading