From 23afd93b3859347127ff50210c9fd941101f54cc Mon Sep 17 00:00:00 2001 From: Ale Mercado Date: Mon, 27 Apr 2026 15:45:32 -0400 Subject: [PATCH 1/8] fix: add assets/icon.png fallback for icon auto-detection 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. --- internal/pkg/apps/install.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/internal/pkg/apps/install.go b/internal/pkg/apps/install.go index 9dcd68ee..feaea2cc 100644 --- a/internal/pkg/apps/install.go +++ b/internal/pkg/apps/install.go @@ -218,10 +218,12 @@ func Install(ctx context.Context, clients *shared.ClientFactory, auth types.Slac } } - // upload icon, default to icon.png + // upload icon, default to assets/icon.png or icon.png var iconPath = slackManifest.Icon if iconPath == "" { - if _, err := os.Stat("icon.png"); !os.IsNotExist(err) { + if _, err := os.Stat("assets/icon.png"); !os.IsNotExist(err) { + iconPath = "assets/icon.png" + } else if _, err := os.Stat("icon.png"); !os.IsNotExist(err) { iconPath = "icon.png" } } @@ -526,7 +528,9 @@ func InstallLocalApp(ctx context.Context, clients *shared.ClientFactory, orgGran if clients.Config.WithExperimentOn(experiment.SetIcon) { var iconPath = slackManifest.Icon if iconPath == "" { - if _, err := os.Stat("icon.png"); !os.IsNotExist(err) { + if _, err := os.Stat("assets/icon.png"); !os.IsNotExist(err) { + iconPath = "assets/icon.png" + } else if _, err := os.Stat("icon.png"); !os.IsNotExist(err) { iconPath = "icon.png" } } From 5176129057b9f4378be9a49eb7f68417d8f11c68 Mon Sep 17 00:00:00 2001 From: Ale Mercado Date: Tue, 28 Apr 2026 16:31:53 -0400 Subject: [PATCH 2/8] feat: support png, jpg, jpeg, gif for icon auto-detection 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. --- internal/pkg/apps/install.go | 40 ++++++++------- internal/pkg/apps/install_test.go | 64 ++++++++++++++++++++++++ internal/shared/types/slack_yaml.go | 14 ++++-- internal/shared/types/slack_yaml_test.go | 25 +++++++++ 4 files changed, 120 insertions(+), 23 deletions(-) diff --git a/internal/pkg/apps/install.go b/internal/pkg/apps/install.go index feaea2cc..b89ce904 100644 --- a/internal/pkg/apps/install.go +++ b/internal/pkg/apps/install.go @@ -17,7 +17,7 @@ package apps import ( "context" "fmt" - "os" + "path/filepath" "strings" "time" @@ -30,6 +30,7 @@ import ( "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackerror" "github.com/slackapi/slack-cli/internal/style" + "github.com/spf13/afero" ) // Constants for onlyCreateUpdateAppManifest parameter @@ -40,6 +41,23 @@ const ( const additionalManifestInfoNotice = "App manifest contains some components that may require additional information" +var supportedIconExtensions = []string{".png", ".jpg", ".jpeg", ".gif"} + +func resolveIconPath(fs afero.Fs, manifestIcon string) string { + if manifestIcon != "" { + return manifestIcon + } + for _, dir := range []string{"assets", "."} { + for _, ext := range supportedIconExtensions { + candidate := filepath.Join(dir, "icon"+ext) + if _, err := fs.Stat(candidate); err == nil { + return candidate + } + } + } + return "" +} + // Install installs the app to a team func Install(ctx context.Context, clients *shared.ClientFactory, auth types.SlackAuth, onlyCreateUpdateAppManifest bool, app types.App, orgGrantWorkspaceID string) (types.App, types.InstallState, error) { span, ctx := opentracing.StartSpanFromContext(ctx, "pkg.apps.install") @@ -218,15 +236,8 @@ func Install(ctx context.Context, clients *shared.ClientFactory, auth types.Slac } } - // upload icon, default to assets/icon.png or icon.png - var iconPath = slackManifest.Icon - if iconPath == "" { - if _, err := os.Stat("assets/icon.png"); !os.IsNotExist(err) { - iconPath = "assets/icon.png" - } else if _, err := os.Stat("icon.png"); !os.IsNotExist(err) { - iconPath = "icon.png" - } - } + // upload icon, default to assets/icon.{png,jpg,jpeg,gif} or icon.{png,jpg,jpeg,gif} + iconPath := resolveIconPath(clients.Fs, slackManifest.Icon) if iconPath != "" { err = updateIcon(ctx, clients, iconPath, app.AppID, token) if err != nil { @@ -526,14 +537,7 @@ func InstallLocalApp(ctx context.Context, clients *shared.ClientFactory, orgGran // upload icon for non-hosted apps (gated behind set-icon experiment) if clients.Config.WithExperimentOn(experiment.SetIcon) { - var iconPath = slackManifest.Icon - if iconPath == "" { - if _, err := os.Stat("assets/icon.png"); !os.IsNotExist(err) { - iconPath = "assets/icon.png" - } else if _, err := os.Stat("icon.png"); !os.IsNotExist(err) { - iconPath = "icon.png" - } - } + iconPath := resolveIconPath(clients.Fs, slackManifest.Icon) if iconPath != "" { _, iconErr := clients.API().IconSet(ctx, clients.Fs, token, app.AppID, iconPath) if iconErr != nil { diff --git a/internal/pkg/apps/install_test.go b/internal/pkg/apps/install_test.go index 2c44dc94..2be011a4 100644 --- a/internal/pkg/apps/install_test.go +++ b/internal/pkg/apps/install_test.go @@ -26,6 +26,7 @@ import ( "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/slackerror" + "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -1724,3 +1725,66 @@ func TestContinueDespiteWarning(t *testing.T) { }) } } + +func Test_resolveIconPath(t *testing.T) { + tests := map[string]struct { + manifestIcon string + files []string + expected string + }{ + "manifest icon set returns it directly": { + manifestIcon: "custom/my-icon.png", + expected: "custom/my-icon.png", + }, + "assets/icon.png found": { + files: []string{"assets/icon.png"}, + expected: "assets/icon.png", + }, + "assets/icon.jpg found": { + files: []string{"assets/icon.jpg"}, + expected: "assets/icon.jpg", + }, + "assets/icon.jpeg found": { + files: []string{"assets/icon.jpeg"}, + expected: "assets/icon.jpeg", + }, + "assets/icon.gif found": { + files: []string{"assets/icon.gif"}, + expected: "assets/icon.gif", + }, + "png wins over gif in assets": { + files: []string{"assets/icon.png", "assets/icon.gif"}, + expected: "assets/icon.png", + }, + "jpg wins over gif in assets": { + files: []string{"assets/icon.jpg", "assets/icon.gif"}, + expected: "assets/icon.jpg", + }, + "root icon.png found when no assets": { + files: []string{"icon.png"}, + expected: "icon.png", + }, + "root icon.jpg found when no assets": { + files: []string{"icon.jpg"}, + expected: "icon.jpg", + }, + "assets takes priority over root": { + files: []string{"assets/icon.gif", "icon.png"}, + expected: "assets/icon.gif", + }, + "no icon files returns empty": { + files: []string{}, + expected: "", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + fs := afero.NewMemMapFs() + for _, f := range tc.files { + require.NoError(t, afero.WriteFile(fs, f, []byte("img"), 0o644)) + } + result := resolveIconPath(fs, tc.manifestIcon) + assert.Equal(t, tc.expected, result) + }) + } +} diff --git a/internal/shared/types/slack_yaml.go b/internal/shared/types/slack_yaml.go index 106135b2..1f8fea08 100644 --- a/internal/shared/types/slack_yaml.go +++ b/internal/shared/types/slack_yaml.go @@ -27,15 +27,19 @@ type SlackYaml struct { Hash string } +var supportedIconExtensions = []string{".png", ".jpg", ".jpeg", ".gif"} + // hasValidIconPath returns false if icon path is provided but is not valid and true otherwise func (sy *SlackYaml) hasValidIconPath() bool { - // verify icon path is valid if exists var wd, err = os.Getwd() if err == nil { - if sy.Icon == "" { // icon was not provided. Let's check if the default one exists - var defaultIconPath = "assets/icon.png" - if _, err := os.Stat(filepath.Join(wd, defaultIconPath)); !os.IsNotExist(err) { - sy.Icon = defaultIconPath + if sy.Icon == "" { + for _, ext := range supportedIconExtensions { + candidate := filepath.Join(wd, "assets", "icon"+ext) + if _, err := os.Stat(candidate); !os.IsNotExist(err) { + sy.Icon = filepath.Join("assets", "icon"+ext) + break + } } } else { if _, err := os.Stat(filepath.Join(wd, sy.Icon)); os.IsNotExist(err) { diff --git a/internal/shared/types/slack_yaml_test.go b/internal/shared/types/slack_yaml_test.go index f9264288..7276a284 100644 --- a/internal/shared/types/slack_yaml_test.go +++ b/internal/shared/types/slack_yaml_test.go @@ -50,6 +50,31 @@ func Test_SlackYaml_hasValidIconPath(t *testing.T) { }, expected: true, }, + "no icon with default assets/icon.jpg present returns true": { + icon: "", + setup: func(t *testing.T, dir string) { + require.NoError(t, os.MkdirAll(filepath.Join(dir, "assets"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "assets", "icon.jpg"), []byte("img"), 0o644)) + }, + expected: true, + }, + "no icon with default assets/icon.gif present returns true": { + icon: "", + setup: func(t *testing.T, dir string) { + require.NoError(t, os.MkdirAll(filepath.Join(dir, "assets"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "assets", "icon.gif"), []byte("img"), 0o644)) + }, + expected: true, + }, + "png takes priority over jpg in assets": { + icon: "", + setup: func(t *testing.T, dir string) { + require.NoError(t, os.MkdirAll(filepath.Join(dir, "assets"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "assets", "icon.png"), []byte("img"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "assets", "icon.jpg"), []byte("img"), 0o644)) + }, + expected: true, + }, "no icon and no default returns true": { icon: "", setup: func(t *testing.T, dir string) {}, From c9abb3046b301ad85f67a9402bd11d3857ecb1a8 Mon Sep 17 00:00:00 2001 From: Ale Mercado Date: Fri, 1 May 2026 11:33:48 -0400 Subject: [PATCH 3/8] refactor: extract icon resolution to internal/icon package Consolidate duplicated icon path resolution logic from install.go and slack_yaml.go into a shared internal/icon package per review feedback. --- internal/icon/icon.go | 38 +++++++++++ internal/icon/icon_test.go | 86 ++++++++++++++++++++++++ internal/pkg/apps/install.go | 24 +------ internal/pkg/apps/install_test.go | 64 ------------------ internal/shared/types/slack_yaml.go | 34 ++++------ internal/shared/types/slack_yaml_test.go | 7 +- 6 files changed, 145 insertions(+), 108 deletions(-) create mode 100644 internal/icon/icon.go create mode 100644 internal/icon/icon_test.go diff --git a/internal/icon/icon.go b/internal/icon/icon.go new file mode 100644 index 00000000..05a81e9e --- /dev/null +++ b/internal/icon/icon.go @@ -0,0 +1,38 @@ +// Copyright 2022-2026 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package icon + +import ( + "path/filepath" + + "github.com/spf13/afero" +) + +var SupportedExtensions = []string{".png", ".jpg", ".jpeg", ".gif"} + +func ResolveIconPath(fs afero.Fs, manifestIcon string) string { + if manifestIcon != "" { + return manifestIcon + } + for _, dir := range []string{"assets", "."} { + for _, ext := range SupportedExtensions { + candidate := filepath.Join(dir, "icon"+ext) + if _, err := fs.Stat(candidate); err == nil { + return candidate + } + } + } + return "" +} diff --git a/internal/icon/icon_test.go b/internal/icon/icon_test.go new file mode 100644 index 00000000..e694bc7c --- /dev/null +++ b/internal/icon/icon_test.go @@ -0,0 +1,86 @@ +// Copyright 2022-2026 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package icon + +import ( + "testing" + + "github.com/spf13/afero" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_ResolveIconPath(t *testing.T) { + tests := map[string]struct { + manifestIcon string + files []string + expected string + }{ + "manifest icon set returns it directly": { + manifestIcon: "custom/my-icon.png", + expected: "custom/my-icon.png", + }, + "assets/icon.png found": { + files: []string{"assets/icon.png"}, + expected: "assets/icon.png", + }, + "assets/icon.jpg found": { + files: []string{"assets/icon.jpg"}, + expected: "assets/icon.jpg", + }, + "assets/icon.jpeg found": { + files: []string{"assets/icon.jpeg"}, + expected: "assets/icon.jpeg", + }, + "assets/icon.gif found": { + files: []string{"assets/icon.gif"}, + expected: "assets/icon.gif", + }, + "png wins over gif in assets": { + files: []string{"assets/icon.png", "assets/icon.gif"}, + expected: "assets/icon.png", + }, + "jpg wins over gif in assets": { + files: []string{"assets/icon.jpg", "assets/icon.gif"}, + expected: "assets/icon.jpg", + }, + "root icon.png found when no assets": { + files: []string{"icon.png"}, + expected: "icon.png", + }, + "root icon.jpg found when no assets": { + files: []string{"icon.jpg"}, + expected: "icon.jpg", + }, + "assets takes priority over root": { + files: []string{"assets/icon.gif", "icon.png"}, + expected: "assets/icon.gif", + }, + "no icon files returns empty": { + files: []string{}, + expected: "", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + fs := afero.NewMemMapFs() + for _, f := range tc.files { + require.NoError(t, afero.WriteFile(fs, f, []byte("img"), 0o644)) + } + result := ResolveIconPath(fs, tc.manifestIcon) + assert.Equal(t, tc.expected, result) + }) + } +} diff --git a/internal/pkg/apps/install.go b/internal/pkg/apps/install.go index b89ce904..29b5f5d0 100644 --- a/internal/pkg/apps/install.go +++ b/internal/pkg/apps/install.go @@ -17,7 +17,6 @@ package apps import ( "context" "fmt" - "path/filepath" "strings" "time" @@ -25,12 +24,12 @@ import ( "github.com/slackapi/slack-cli/internal/api" "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/experiment" + "github.com/slackapi/slack-cli/internal/icon" "github.com/slackapi/slack-cli/internal/pkg/manifest" "github.com/slackapi/slack-cli/internal/shared" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackerror" "github.com/slackapi/slack-cli/internal/style" - "github.com/spf13/afero" ) // Constants for onlyCreateUpdateAppManifest parameter @@ -41,23 +40,6 @@ const ( const additionalManifestInfoNotice = "App manifest contains some components that may require additional information" -var supportedIconExtensions = []string{".png", ".jpg", ".jpeg", ".gif"} - -func resolveIconPath(fs afero.Fs, manifestIcon string) string { - if manifestIcon != "" { - return manifestIcon - } - for _, dir := range []string{"assets", "."} { - for _, ext := range supportedIconExtensions { - candidate := filepath.Join(dir, "icon"+ext) - if _, err := fs.Stat(candidate); err == nil { - return candidate - } - } - } - return "" -} - // Install installs the app to a team func Install(ctx context.Context, clients *shared.ClientFactory, auth types.SlackAuth, onlyCreateUpdateAppManifest bool, app types.App, orgGrantWorkspaceID string) (types.App, types.InstallState, error) { span, ctx := opentracing.StartSpanFromContext(ctx, "pkg.apps.install") @@ -237,7 +219,7 @@ func Install(ctx context.Context, clients *shared.ClientFactory, auth types.Slac } // upload icon, default to assets/icon.{png,jpg,jpeg,gif} or icon.{png,jpg,jpeg,gif} - iconPath := resolveIconPath(clients.Fs, slackManifest.Icon) + iconPath := icon.ResolveIconPath(clients.Fs, slackManifest.Icon) if iconPath != "" { err = updateIcon(ctx, clients, iconPath, app.AppID, token) if err != nil { @@ -537,7 +519,7 @@ func InstallLocalApp(ctx context.Context, clients *shared.ClientFactory, orgGran // upload icon for non-hosted apps (gated behind set-icon experiment) if clients.Config.WithExperimentOn(experiment.SetIcon) { - iconPath := resolveIconPath(clients.Fs, slackManifest.Icon) + iconPath := icon.ResolveIconPath(clients.Fs, slackManifest.Icon) if iconPath != "" { _, iconErr := clients.API().IconSet(ctx, clients.Fs, token, app.AppID, iconPath) if iconErr != nil { diff --git a/internal/pkg/apps/install_test.go b/internal/pkg/apps/install_test.go index 2be011a4..2c44dc94 100644 --- a/internal/pkg/apps/install_test.go +++ b/internal/pkg/apps/install_test.go @@ -26,7 +26,6 @@ import ( "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/slackerror" - "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -1725,66 +1724,3 @@ func TestContinueDespiteWarning(t *testing.T) { }) } } - -func Test_resolveIconPath(t *testing.T) { - tests := map[string]struct { - manifestIcon string - files []string - expected string - }{ - "manifest icon set returns it directly": { - manifestIcon: "custom/my-icon.png", - expected: "custom/my-icon.png", - }, - "assets/icon.png found": { - files: []string{"assets/icon.png"}, - expected: "assets/icon.png", - }, - "assets/icon.jpg found": { - files: []string{"assets/icon.jpg"}, - expected: "assets/icon.jpg", - }, - "assets/icon.jpeg found": { - files: []string{"assets/icon.jpeg"}, - expected: "assets/icon.jpeg", - }, - "assets/icon.gif found": { - files: []string{"assets/icon.gif"}, - expected: "assets/icon.gif", - }, - "png wins over gif in assets": { - files: []string{"assets/icon.png", "assets/icon.gif"}, - expected: "assets/icon.png", - }, - "jpg wins over gif in assets": { - files: []string{"assets/icon.jpg", "assets/icon.gif"}, - expected: "assets/icon.jpg", - }, - "root icon.png found when no assets": { - files: []string{"icon.png"}, - expected: "icon.png", - }, - "root icon.jpg found when no assets": { - files: []string{"icon.jpg"}, - expected: "icon.jpg", - }, - "assets takes priority over root": { - files: []string{"assets/icon.gif", "icon.png"}, - expected: "assets/icon.gif", - }, - "no icon files returns empty": { - files: []string{}, - expected: "", - }, - } - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - fs := afero.NewMemMapFs() - for _, f := range tc.files { - require.NoError(t, afero.WriteFile(fs, f, []byte("img"), 0o644)) - } - result := resolveIconPath(fs, tc.manifestIcon) - assert.Equal(t, tc.expected, result) - }) - } -} diff --git a/internal/shared/types/slack_yaml.go b/internal/shared/types/slack_yaml.go index 1f8fea08..6c0fc072 100644 --- a/internal/shared/types/slack_yaml.go +++ b/internal/shared/types/slack_yaml.go @@ -18,7 +18,9 @@ import ( "os" "path/filepath" + "github.com/slackapi/slack-cli/internal/icon" "github.com/slackapi/slack-cli/internal/slackerror" + "github.com/spf13/afero" ) type SlackYaml struct { @@ -27,33 +29,25 @@ type SlackYaml struct { Hash string } -var supportedIconExtensions = []string{".png", ".jpg", ".jpeg", ".gif"} - // hasValidIconPath returns false if icon path is provided but is not valid and true otherwise -func (sy *SlackYaml) hasValidIconPath() bool { - var wd, err = os.Getwd() - if err == nil { - if sy.Icon == "" { - for _, ext := range supportedIconExtensions { - candidate := filepath.Join(wd, "assets", "icon"+ext) - if _, err := os.Stat(candidate); !os.IsNotExist(err) { - sy.Icon = filepath.Join("assets", "icon"+ext) - break - } - } - } else { - if _, err := os.Stat(filepath.Join(wd, sy.Icon)); os.IsNotExist(err) { - return false - } +func (sy *SlackYaml) hasValidIconPath(fs afero.Fs) bool { + wd, err := os.Getwd() + if err != nil { + return true + } + if sy.Icon == "" { + sy.Icon = icon.ResolveIconPath(afero.NewBasePathFs(fs, wd), "") + } else { + if _, err := fs.Stat(filepath.Join(wd, sy.Icon)); os.IsNotExist(err) { + return false } } - return true } // Verify checks that the app manifest meets some basic requirements -func (sy *SlackYaml) Verify() error { - if !sy.hasValidIconPath() { +func (sy *SlackYaml) Verify(fs afero.Fs) error { + if !sy.hasValidIconPath(fs) { return slackerror.New("Please specify a valid icon path in app manifest") } return nil diff --git a/internal/shared/types/slack_yaml_test.go b/internal/shared/types/slack_yaml_test.go index 7276a284..e41584ad 100644 --- a/internal/shared/types/slack_yaml_test.go +++ b/internal/shared/types/slack_yaml_test.go @@ -19,6 +19,7 @@ import ( "path/filepath" "testing" + "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -92,7 +93,7 @@ func Test_SlackYaml_hasValidIconPath(t *testing.T) { defer func() { require.NoError(t, os.Chdir(origDir)) }() sy := &SlackYaml{Icon: tc.icon} - assert.Equal(t, tc.expected, sy.hasValidIconPath()) + assert.Equal(t, tc.expected, sy.hasValidIconPath(afero.NewOsFs())) }) } } @@ -128,9 +129,9 @@ func Test_SlackYaml_Verify(t *testing.T) { sy := &SlackYaml{Icon: tc.icon} if tc.expectErr { - assert.Error(t, sy.Verify()) + assert.Error(t, sy.Verify(afero.NewOsFs())) } else { - assert.NoError(t, sy.Verify()) + assert.NoError(t, sy.Verify(afero.NewOsFs())) } }) } From b626cecd3d6c7956c547780dc604008c70aa634b Mon Sep 17 00:00:00 2001 From: Ale Mercado Date: Fri, 1 May 2026 14:56:43 -0400 Subject: [PATCH 4/8] feat: support SLACK_CLI_APP_ICON_PATH env var for icon override 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 --- internal/config/config.go | 2 + internal/config/dotenv.go | 6 +++ internal/config/dotenv_test.go | 21 ++++++++ internal/pkg/apps/install.go | 34 +++++++----- internal/pkg/apps/install_test.go | 88 +++++++++++++++++++++++++++++++ 5 files changed, 138 insertions(+), 13 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 9993d742..db9e70cf 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -22,6 +22,7 @@ import ( ) // Environment Variable constants +const slackAppIconPathEnv = "SLACK_CLI_APP_ICON_PATH" const slackAutoRequestAAAEnv = "SLACK_AUTO_REQUEST_AAA" const slackConfigDirEnv = "SLACK_CONFIG_DIR" const slackDisableTelemetryEnv = "SLACK_DISABLE_TELEMETRY" @@ -41,6 +42,7 @@ type Config struct { APIHostFlag string APIHostResolved string AppFlag string + AppIconPathFlag string AutoRequestAAAFlag bool ConfigDirFlag string DebugEnabled bool diff --git a/internal/config/dotenv.go b/internal/config/dotenv.go index 9ae6333a..4241392e 100644 --- a/internal/config/dotenv.go +++ b/internal/config/dotenv.go @@ -45,6 +45,12 @@ func (c *Config) LoadEnvironmentVariables() error { c.ConfigDirFlag = configDir } + // Load app icon path from environment variables + var appIconPath = strings.TrimSpace(c.os.Getenv(slackAppIconPathEnv)) + if appIconPath != "" { + c.AppIconPathFlag = appIconPath + } + // Disable telemetry if either disable-telemetry or test-version environment variables var disableTelemetry = strings.TrimSpace(c.os.Getenv(slackDisableTelemetryEnv)) var testVersion = strings.TrimSpace(c.os.Getenv(version.EnvTestVersion)) diff --git a/internal/config/dotenv_test.go b/internal/config/dotenv_test.go index 77d1773b..ab927346 100644 --- a/internal/config/dotenv_test.go +++ b/internal/config/dotenv_test.go @@ -153,6 +153,27 @@ func Test_DotEnv_LoadEnvironmentVariables(t *testing.T) { assert.Equal(t, false, cfg.AutoRequestAAAFlag) }, }, + "SLACK_CLI_APP_ICON_PATH=/path/to/icon.png should set AppIconPathFlag": { + envName: "SLACK_CLI_APP_ICON_PATH", + envValue: "/path/to/icon.png", + assertOnConfig: func(t *testing.T, cfg *Config) { + assert.Equal(t, "/path/to/icon.png", cfg.AppIconPathFlag) + }, + }, + "SLACK_CLI_APP_ICON_PATH= should not set AppIconPathFlag": { + envName: "SLACK_CLI_APP_ICON_PATH", + envValue: "", + assertOnConfig: func(t *testing.T, cfg *Config) { + assert.Equal(t, "", cfg.AppIconPathFlag) + }, + }, + "SLACK_CLI_APP_ICON_PATH with whitespace should be trimmed": { + envName: "SLACK_CLI_APP_ICON_PATH", + envValue: " /path/to/icon.png ", + assertOnConfig: func(t *testing.T, cfg *Config) { + assert.Equal(t, "/path/to/icon.png", cfg.AppIconPathFlag) + }, + }, "SLACK_CONFIG_DIR=/path/to/config should set the config dir": { envName: "SLACK_CONFIG_DIR", envValue: "/path/to/config", diff --git a/internal/pkg/apps/install.go b/internal/pkg/apps/install.go index 9dcd68ee..8ca3c2ec 100644 --- a/internal/pkg/apps/install.go +++ b/internal/pkg/apps/install.go @@ -218,13 +218,7 @@ func Install(ctx context.Context, clients *shared.ClientFactory, auth types.Slac } } - // upload icon, default to icon.png - var iconPath = slackManifest.Icon - if iconPath == "" { - if _, err := os.Stat("icon.png"); !os.IsNotExist(err) { - iconPath = "icon.png" - } - } + iconPath := resolveIconPath(ctx, clients, slackManifest.Icon) if iconPath != "" { err = updateIcon(ctx, clients, iconPath, app.AppID, token) if err != nil { @@ -524,12 +518,7 @@ func InstallLocalApp(ctx context.Context, clients *shared.ClientFactory, orgGran // upload icon for non-hosted apps (gated behind set-icon experiment) if clients.Config.WithExperimentOn(experiment.SetIcon) { - var iconPath = slackManifest.Icon - if iconPath == "" { - if _, err := os.Stat("icon.png"); !os.IsNotExist(err) { - iconPath = "icon.png" - } - } + iconPath := resolveIconPath(ctx, clients, slackManifest.Icon) if iconPath != "" { _, iconErr := clients.API().IconSet(ctx, clients.Fs, token, app.AppID, iconPath) if iconErr != nil { @@ -649,6 +638,25 @@ func appendLocalToDisplayName(manifest *types.AppManifest) { } } +// resolveIconPath determines the icon file path using the priority chain: +// SLACK_CLI_APP_ICON_PATH env var > manifest icon field > icon.png in project root +func resolveIconPath(ctx context.Context, clients *shared.ClientFactory, manifestIcon string) string { + if envIconPath := clients.Config.AppIconPathFlag; envIconPath != "" { + if _, err := os.Stat(envIconPath); !os.IsNotExist(err) { + 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))) + } + if manifestIcon != "" { + return manifestIcon + } + if _, err := os.Stat("icon.png"); !os.IsNotExist(err) { + return "icon.png" + } + return "" +} + // updateIcon will upload the new icon to the Slack API func updateIcon(ctx context.Context, clients *shared.ClientFactory, iconPath, appID string, token string) error { var span opentracing.Span diff --git a/internal/pkg/apps/install_test.go b/internal/pkg/apps/install_test.go index 2c44dc94..5a3367ad 100644 --- a/internal/pkg/apps/install_test.go +++ b/internal/pkg/apps/install_test.go @@ -16,6 +16,8 @@ package apps import ( "bytes" + "os" + "path/filepath" "testing" "github.com/slackapi/slack-cli/internal/api" @@ -1724,3 +1726,89 @@ func TestContinueDespiteWarning(t *testing.T) { }) } } + +func Test_resolveIconPath(t *testing.T) { + tests := map[string]struct { + envIconPath string + manifestIcon string + setupFiles func(t *testing.T, dir string) + expected string + }{ + "env var takes priority over manifest icon": { + envIconPath: "env-icon.png", + manifestIcon: "manifest-icon.png", + setupFiles: func(t *testing.T, dir string) { + require.NoError(t, os.WriteFile(filepath.Join(dir, "env-icon.png"), []byte("img"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "manifest-icon.png"), []byte("img"), 0o644)) + }, + expected: "env-icon.png", + }, + "env var takes priority over icon.png fallback": { + envIconPath: "env-icon.png", + setupFiles: func(t *testing.T, dir string) { + require.NoError(t, os.WriteFile(filepath.Join(dir, "env-icon.png"), []byte("img"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "icon.png"), []byte("img"), 0o644)) + }, + expected: "env-icon.png", + }, + "manifest icon used when no env var": { + manifestIcon: "manifest-icon.png", + setupFiles: func(t *testing.T, dir string) { + require.NoError(t, os.WriteFile(filepath.Join(dir, "manifest-icon.png"), []byte("img"), 0o644)) + }, + expected: "manifest-icon.png", + }, + "falls back to icon.png when no env var or manifest": { + setupFiles: func(t *testing.T, dir string) { + require.NoError(t, os.WriteFile(filepath.Join(dir, "icon.png"), []byte("img"), 0o644)) + }, + expected: "icon.png", + }, + "returns empty when no icon found": { + setupFiles: func(t *testing.T, dir string) {}, + expected: "", + }, + "env var file not found falls back to manifest": { + envIconPath: "missing-icon.png", + manifestIcon: "manifest-icon.png", + setupFiles: func(t *testing.T, dir string) { + require.NoError(t, os.WriteFile(filepath.Join(dir, "manifest-icon.png"), []byte("img"), 0o644)) + }, + expected: "manifest-icon.png", + }, + "env var file not found falls back to icon.png": { + envIconPath: "missing-icon.png", + setupFiles: func(t *testing.T, dir string) { + require.NoError(t, os.WriteFile(filepath.Join(dir, "icon.png"), []byte("img"), 0o644)) + }, + expected: "icon.png", + }, + "env var file not found and no fallback returns empty": { + envIconPath: "missing-icon.png", + setupFiles: func(t *testing.T, dir string) {}, + expected: "", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + dir := t.TempDir() + tc.setupFiles(t, dir) + + origDir, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(dir)) + defer func() { require.NoError(t, os.Chdir(origDir)) }() + + ctx := slackcontext.MockContext(t.Context()) + clientsMock := shared.NewClientsMock() + clientsMock.AddDefaultMocks() + clientsMock.Config.AppIconPathFlag = tc.envIconPath + output := &bytes.Buffer{} + clientsMock.IO.Stdout.SetOutput(output) + clients := shared.NewClientFactory(clientsMock.MockClientFactory()) + + result := resolveIconPath(ctx, clients, tc.manifestIcon) + assert.Equal(t, tc.expected, result) + }) + } +} From d7e7ccd1c0da53f1881e766a90d9b898fb06369f Mon Sep 17 00:00:00 2001 From: Ale Mercado <104795114+srtaalej@users.noreply.github.com> Date: Mon, 4 May 2026 13:32:12 -0400 Subject: [PATCH 5/8] Update internal/config/config.go Co-authored-by: Eden Zimbelman --- internal/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/config/config.go b/internal/config/config.go index db9e70cf..61b22d0d 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -22,7 +22,7 @@ import ( ) // Environment Variable constants -const slackAppIconPathEnv = "SLACK_CLI_APP_ICON_PATH" +const slackCLIAppIconPathEnv = "SLACK_CLI_APP_ICON_PATH" const slackAutoRequestAAAEnv = "SLACK_AUTO_REQUEST_AAA" const slackConfigDirEnv = "SLACK_CONFIG_DIR" const slackDisableTelemetryEnv = "SLACK_DISABLE_TELEMETRY" From d59905249fc3c776d960e6c6c7c4f51610ca208a Mon Sep 17 00:00:00 2001 From: Ale Mercado Date: Mon, 4 May 2026 14:57:31 -0400 Subject: [PATCH 6/8] fix: address review feedback for icon path resolution - 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 --- internal/config/dotenv.go | 2 +- internal/pkg/apps/install.go | 5 ++++- internal/pkg/apps/install_test.go | 16 ++++++++-------- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/internal/config/dotenv.go b/internal/config/dotenv.go index 4241392e..e553419a 100644 --- a/internal/config/dotenv.go +++ b/internal/config/dotenv.go @@ -46,7 +46,7 @@ func (c *Config) LoadEnvironmentVariables() error { } // Load app icon path from environment variables - var appIconPath = strings.TrimSpace(c.os.Getenv(slackAppIconPathEnv)) + var appIconPath = strings.TrimSpace(c.os.Getenv(slackCLIAppIconPathEnv)) if appIconPath != "" { c.AppIconPathFlag = appIconPath } diff --git a/internal/pkg/apps/install.go b/internal/pkg/apps/install.go index 3654587c..3dc3d0a9 100644 --- a/internal/pkg/apps/install.go +++ b/internal/pkg/apps/install.go @@ -647,9 +647,12 @@ func resolveIconPath(ctx context.Context, clients *shared.ClientFactory, manifes } 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))) + return "" } if manifestIcon != "" { - return manifestIcon + if _, err := os.Stat(manifestIcon); !os.IsNotExist(err) { + return manifestIcon + } } if _, err := os.Stat("icon.png"); !os.IsNotExist(err) { return "icon.png" diff --git a/internal/pkg/apps/install_test.go b/internal/pkg/apps/install_test.go index ae1b5ebb..939b9716 100644 --- a/internal/pkg/apps/install_test.go +++ b/internal/pkg/apps/install_test.go @@ -1771,25 +1771,25 @@ func Test_resolveIconPath(t *testing.T) { setupFiles: func(t *testing.T, dir string) {}, expected: "", }, - "env var file not found falls back to manifest": { + "env var file not found returns empty": { envIconPath: "missing-icon.png", manifestIcon: "manifest-icon.png", setupFiles: func(t *testing.T, dir string) { require.NoError(t, os.WriteFile(filepath.Join(dir, "manifest-icon.png"), []byte("img"), 0o644)) }, - expected: "manifest-icon.png", + expected: "", }, - "env var file not found falls back to icon.png": { - envIconPath: "missing-icon.png", + "manifest icon file not found falls back to icon.png": { + manifestIcon: "missing-manifest-icon.png", setupFiles: func(t *testing.T, dir string) { require.NoError(t, os.WriteFile(filepath.Join(dir, "icon.png"), []byte("img"), 0o644)) }, expected: "icon.png", }, - "env var file not found and no fallback returns empty": { - envIconPath: "missing-icon.png", - setupFiles: func(t *testing.T, dir string) {}, - expected: "", + "manifest icon file not found and no icon.png returns empty": { + manifestIcon: "missing-manifest-icon.png", + setupFiles: func(t *testing.T, dir string) {}, + expected: "", }, } for name, tc := range tests { From 87a464684929001696402eedfea3849666f66b3a Mon Sep 17 00:00:00 2001 From: Ale Mercado <104795114+srtaalej@users.noreply.github.com> Date: Tue, 5 May 2026 11:05:31 -0400 Subject: [PATCH 7/8] Update internal/pkg/apps/install.go Co-authored-by: Eden Zimbelman --- internal/pkg/apps/install.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/pkg/apps/install.go b/internal/pkg/apps/install.go index 3dc3d0a9..1f25d622 100644 --- a/internal/pkg/apps/install.go +++ b/internal/pkg/apps/install.go @@ -653,6 +653,9 @@ func resolveIconPath(ctx context.Context, clients *shared.ClientFactory, manifes if _, err := os.Stat(manifestIcon); !os.IsNotExist(err) { return manifestIcon } + clients.IO.PrintDebug(ctx, "manifest icon file not found: %s", manifestIcon) + _, _ = clients.IO.WriteOut().Write([]byte(style.SectionSecondaryf("Warning: icon path from manifest not found: %s", manifestIcon))) + return "" } if _, err := os.Stat("icon.png"); !os.IsNotExist(err) { return "icon.png" From 061e54f10c3dc17e3a49919687fbdb056020c753 Mon Sep 17 00:00:00 2001 From: Ale Mercado Date: Tue, 5 May 2026 11:10:00 -0400 Subject: [PATCH 8/8] fix: update test to match manifest icon early-return behavior --- internal/pkg/apps/install_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/pkg/apps/install_test.go b/internal/pkg/apps/install_test.go index 939b9716..46c71e46 100644 --- a/internal/pkg/apps/install_test.go +++ b/internal/pkg/apps/install_test.go @@ -1779,12 +1779,12 @@ func Test_resolveIconPath(t *testing.T) { }, expected: "", }, - "manifest icon file not found falls back to icon.png": { + "manifest icon file not found returns empty with warning": { manifestIcon: "missing-manifest-icon.png", setupFiles: func(t *testing.T, dir string) { require.NoError(t, os.WriteFile(filepath.Join(dir, "icon.png"), []byte("img"), 0o644)) }, - expected: "icon.png", + expected: "", }, "manifest icon file not found and no icon.png returns empty": { manifestIcon: "missing-manifest-icon.png",