feat: support loading a dotenv '.env' file for your app (#436)

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

Author: zimeg

.claude/CLAUDE.md

@@ -164,6 +164,12 @@ for name, tc := range tests {
 }
 ```
 
+### Error Handling
+
+- Wrap errors returned across package boundaries with `slackerror.Wrap(err, slackerror.ErrCode)` so they carry a structured error code
+- Register new error codes in `internal/slackerror/errors.go`: add a constant and an entry in `ErrorCodeMap`
+- Error codes are alphabetically ordered in both the constants block and `ErrorCodeMap`
+
 ## Version Management
 
 Versions use semantic versioning with git tags (format: `v*.*.*`).

cmd/platform/deploy.go

@@ -181,6 +181,7 @@ func deployHook(ctx context.Context, clients *shared.ClientFactory) error {
 	// so we instantiate the default here.
 	shell := hooks.HookExecutorDefaultProtocol{
 		IO: clients.IO,
+		Fs: clients.Fs,
 	}
 	if _, err := shell.Execute(ctx, hookExecOpts); err != nil {
 		return err

cmd/platform/deploy_test.go

@@ -279,7 +279,7 @@ func TestDeployCommand_DeployHook(t *testing.T) {
 
 			clients := shared.NewClientFactory(clientsMock.MockClientFactory(), func(clients *shared.ClientFactory) {
 				clients.SDKConfig = sdkConfigMock
-				clients.HookExecutor = hooks.GetHookExecutor(clientsMock.IO, sdkConfigMock)
+				clients.HookExecutor = hooks.GetHookExecutor(clientsMock.IO, clients.Fs, sdkConfigMock)
 			})
 			cmd := NewDeployCommand(clients)
 			cmd.PreRunE = func(cmd *cobra.Command, args []string) error { return nil }

internal/config/dotenv.go

@@ -17,24 +17,10 @@ package config
 import (
 	"strings"
 
-	"github.com/joho/godotenv"
 	"github.com/slackapi/slack-cli/internal/version"
-	"github.com/spf13/afero"
 )
 
-// GetDotEnvFileVariables collects only the variables in the .env file
-func (c *Config) GetDotEnvFileVariables() (map[string]string, error) {
-	variables := map[string]string{}
-	file, err := afero.ReadFile(c.fs, ".env")
-	if err != nil && !c.os.IsNotExist(err) {
-		return variables, err
-	}
-	return godotenv.UnmarshalBytes(file)
-}
-
 // LoadEnvironmentVariables sets flags based on their environment variable value
-//
-// Note: Values are not loaded from the .env file. Use: `GetDotEnvFileVariables`
 func (c *Config) LoadEnvironmentVariables() error {
 	// Skip when dependencies are not configured
 	if c.os == nil {

internal/config/dotenv_test.go

@@ -18,47 +18,9 @@ import (
 	"testing"
 
 	"github.com/slackapi/slack-cli/internal/slackdeps"
-	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
 )
 
-func Test_DotEnv_GetDotEnvFileVariables(t *testing.T) {
-	tests := map[string]struct {
-		globalVariableName  string
-		globalVariableValue string
-		localEnvFile        string
-		expectedValues      map[string]string
-	}{
-		"environment file variables are read": {
-			localEnvFile:   "SLACK_VARIABLE=12\n",
-			expectedValues: map[string]string{"SLACK_VARIABLE": "12"},
-		},
-		"variable casing is preserved on load": {
-			localEnvFile:   "secret_Token=Key123!\n",
-			expectedValues: map[string]string{"secret_Token": "Key123!"},
-		},
-		"global environment variables are ignored": {
-			globalVariableName:  "SLACK_VARIABLE",
-			globalVariableValue: "12",
-			expectedValues:      map[string]string{},
-		},
-	}
-	for name, tc := range tests {
-		t.Run(name, func(t *testing.T) {
-			fs := slackdeps.NewFsMock()
-			os := slackdeps.NewOsMock()
-			os.AddDefaultMocks()
-			os.Setenv(tc.globalVariableName, tc.globalVariableValue)
-			err := afero.WriteFile(fs, ".env", []byte(tc.localEnvFile), 0600)
-			assert.NoError(t, err)
-			config := NewConfig(fs, os)
-			variables, err := config.GetDotEnvFileVariables()
-			assert.NoError(t, err)
-			assert.Equal(t, tc.expectedValues, variables)
-		})
-	}
-}
-
 func Test_DotEnv_LoadEnvironmentVariables(t *testing.T) {
 	tableTests := map[string]struct {
 		envName        string

internal/hooks/hook_executor_default.go

@@ -21,18 +21,20 @@ import (
 
 	"github.com/slackapi/slack-cli/internal/iostreams"
 	"github.com/slackapi/slack-cli/internal/slackerror"
+	"github.com/spf13/afero"
 )
 
 // HookExecutorDefaultProtocol uses the original protocol between the CLI and the SDK where diagnostic info
 // and hook responses come in via stdout. Data outside the expected JSON payload is ignored, with the
 // exception of the 'start' hook, for which it is printed.
 type HookExecutorDefaultProtocol struct {
 	IO iostreams.IOStreamer
+	Fs afero.Fs
 }
 
 // Execute processes the data received by the SDK.
 func (e *HookExecutorDefaultProtocol) Execute(ctx context.Context, opts HookExecOpts) (string, error) {
-	cmdArgs, cmdArgVars, cmdEnvVars, err := processExecOpts(opts)
+	cmdArgs, cmdArgVars, cmdEnvVars, err := processExecOpts(ctx, opts, e.Fs, e.IO)
 	if err != nil {
 		return "", err
 	}

internal/hooks/hook_executor_default_test.go

@@ -25,6 +25,7 @@ import (
 	"github.com/slackapi/slack-cli/internal/slackcontext"
 	"github.com/slackapi/slack-cli/internal/slackdeps"
 	"github.com/slackapi/slack-cli/internal/slackerror"
+	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -80,6 +81,31 @@ func Test_Hook_Execute_Default_Protocol(t *testing.T) {
 				require.Contains(t, opts.Exec.(*MockExec).mockCommand.Env, `YIN=yang`)
 			},
 		},
+		"dotenv vars and hook vars are loaded into the environment": {
+			opts: HookExecOpts{
+				Hook: HookScript{Name: "happypath", Command: "echo {}"},
+				Env: map[string]string{
+					"OPTS_VAR": "from_opts",
+				},
+				Exec: &MockExec{
+					mockCommand: &MockCommand{
+						MockStdout: []byte("test output"),
+						Err:        nil,
+					},
+				},
+			},
+			handler: func(t *testing.T, ctx context.Context, executor HookExecutor, opts HookExecOpts) {
+				// Write a .env file to the mock filesystem
+				e := executor.(*HookExecutorDefaultProtocol)
+				_ = afero.WriteFile(e.Fs, ".env", []byte("DOTENV_VAR=from_dotenv\n"), 0600)
+
+				response, err := executor.Execute(ctx, opts)
+				require.Equal(t, "test output", response)
+				require.NoError(t, err)
+				require.Contains(t, opts.Exec.(*MockExec).mockCommand.Env, `DOTENV_VAR=from_dotenv`)
+				require.Contains(t, opts.Exec.(*MockExec).mockCommand.Env, `OPTS_VAR=from_opts`)
+			},
+		},
 		"failed execution": {
 			opts: HookExecOpts{
 				Hook: HookScript{Command: "boom", Name: "sadpath"},
@@ -156,6 +182,7 @@ func Test_Hook_Execute_Default_Protocol(t *testing.T) {
 			ios.AddDefaultMocks()
 			hookExecutor := &HookExecutorDefaultProtocol{
 				IO: ios,
+				Fs: afero.NewMemMapFs(),
 			}
 			if tc.handler != nil {
 				tc.handler(t, ctx, hookExecutor, tc.opts)

internal/hooks/hook_executor_v2.go

@@ -25,21 +25,23 @@ import (
 
 	"github.com/slackapi/slack-cli/internal/iostreams"
 	"github.com/slackapi/slack-cli/internal/slackerror"
+	"github.com/spf13/afero"
 )
 
 // HookExecutorMessageBoundaryProtocol uses a protocol between the CLI and the SDK where diagnostic info
 // and hook responses come in via stdout, and hook responses are wrapped in a string denoting the
 // message boundary. Only one message payload can be received.
 type HookExecutorMessageBoundaryProtocol struct {
 	IO iostreams.IOStreamer
+	Fs afero.Fs
 }
 
 // generateBoundary is a function for creating boundaries that can be mocked
 var generateBoundary = generateMD5FromRandomString
 
 // Execute processes the data received by the SDK.
 func (e *HookExecutorMessageBoundaryProtocol) Execute(ctx context.Context, opts HookExecOpts) (string, error) {
-	cmdArgs, cmdArgVars, cmdEnvVars, err := processExecOpts(opts)
+	cmdArgs, cmdArgVars, cmdEnvVars, err := processExecOpts(ctx, opts, e.Fs, e.IO)
 	if err != nil {
 		return "", err
 	}

internal/hooks/hook_executor_v2_test.go

@@ -25,6 +25,7 @@ import (
 	"github.com/slackapi/slack-cli/internal/slackcontext"
 	"github.com/slackapi/slack-cli/internal/slackdeps"
 	"github.com/slackapi/slack-cli/internal/slackerror"
+	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -41,6 +42,7 @@ func mockBoundaryStringGenerator() string {
 func Test_Hook_Execute_V2_Protocol(t *testing.T) {
 	tests := map[string]struct {
 		opts  HookExecOpts
+		setup func(afero.Fs)
 		check func(*testing.T, string, error, ExecInterface)
 	}{
 		"error if hook command unavailable": {
@@ -132,6 +134,29 @@ func Test_Hook_Execute_V2_Protocol(t *testing.T) {
 				)
 			},
 		},
+		"dotenv vars and hook vars are loaded into the environment": {
+			opts: HookExecOpts{
+				Hook: HookScript{Name: "happypath", Command: "echo {}"},
+				Env: map[string]string{
+					"OPTS_VAR": "from_opts",
+				},
+				Exec: &MockExec{
+					mockCommand: &MockCommand{
+						MockStdout: []byte(mockBoundaryString + `{"ok": true}` + mockBoundaryString),
+						Err:        nil,
+					},
+				},
+			},
+			setup: func(fs afero.Fs) {
+				_ = afero.WriteFile(fs, ".env", []byte("DOTENV_VAR=from_dotenv\n"), 0600)
+			},
+			check: func(t *testing.T, response string, err error, mockExec ExecInterface) {
+				require.NoError(t, err)
+				require.Equal(t, `{"ok": true}`, response)
+				require.Contains(t, mockExec.(*MockExec).mockCommand.Env, `DOTENV_VAR=from_dotenv`)
+				require.Contains(t, mockExec.(*MockExec).mockCommand.Env, `OPTS_VAR=from_opts`)
+			},
+		},
 		"fail to parse payload due to improper boundary strings": {
 			opts: HookExecOpts{
 				Hook: HookScript{Name: "happypath", Command: "echo {}"},
@@ -176,8 +201,13 @@ func Test_Hook_Execute_V2_Protocol(t *testing.T) {
 			config := config.NewConfig(fs, os)
 			ios := iostreams.NewIOStreamsMock(config, fs, os)
 			ios.AddDefaultMocks()
+			memFs := afero.NewMemMapFs()
 			hookExecutor := &HookExecutorMessageBoundaryProtocol{
 				IO: ios,
+				Fs: memFs,
+			}
+			if tc.setup != nil {
+				tc.setup(memFs)
 			}
 			response, err := hookExecutor.Execute(ctx, tc.opts)
 			tc.check(t, response, err, tc.opts.Exec)

internal/hooks/hooks.go

@@ -16,32 +16,34 @@ package hooks
 
 import (
 	"context"
-	"os"
 	"strings"
 
 	"github.com/slackapi/slack-cli/internal/goutils"
 	"github.com/slackapi/slack-cli/internal/iostreams"
+	"github.com/spf13/afero"
 )
 
 type HookExecutor interface {
 	Execute(ctx context.Context, opts HookExecOpts) (response string, err error)
 }
 
-func GetHookExecutor(ios iostreams.IOStreamer, cfg SDKCLIConfig) HookExecutor {
+func GetHookExecutor(ios iostreams.IOStreamer, fs afero.Fs, cfg SDKCLIConfig) HookExecutor {
 	protocol := cfg.Config.SupportedProtocols.Preferred()
 	switch protocol {
 	case HookProtocolV2:
 		return &HookExecutorMessageBoundaryProtocol{
 			IO: ios,
+			Fs: fs,
 		}
 	default:
 		return &HookExecutorDefaultProtocol{
 			IO: ios,
+			Fs: fs,
 		}
 	}
 }
 
-func processExecOpts(opts HookExecOpts) ([]string, []string, []string, error) {
+func processExecOpts(ctx context.Context, opts HookExecOpts, fs afero.Fs, io iostreams.IOStreamer) ([]string, []string, []string, error) {
 	cmdStr, err := opts.Hook.Get()
 	if err != nil {
 		return []string{}, []string{}, []string{}, err
@@ -53,13 +55,7 @@ func processExecOpts(opts HookExecOpts) ([]string, []string, []string, error) {
 	var cmdArgVars = cmdArgs[1:] // omit the first item because that is the command name
 	cmdArgVars = append(cmdArgVars, goutils.MapToStringSlice(opts.Args, "--")...)
 
-	// Whatever cmd.Env is set to will be the ONLY environment variables that the `cmd` will have access to when it runs.
-	// To avoid removing any environment variables that are set in the current environment, we first set the cmd.Env to the current environment.
-	// before adding any new environment variables.
-	var cmdEnvVars = os.Environ()
-	for name, value := range opts.Env {
-		cmdEnvVars = append(cmdEnvVars, name+"="+value)
-	}
+	cmdEnvVars := opts.ShellEnv(ctx, fs, io)
 
 	return cmdArgs, cmdArgVars, cmdEnvVars, nil
 }