feat: migrate experiments config from array to bools (#398)

* feat: migrate experiments config from array to map

Change the experiments field in SystemConfig, ProjectConfig, and Config
from []experiment.Experiment to map[string]bool. This enables dot-notation
access for the upcoming config command and allows explicit enable/disable
of experiments.

Includes backwards-compatible JSON unmarshaling that auto-converts the old
array format (["charm"]) to the new map format ({"charm": true}).

Updates help output to show all available experiments with ENABLED/DISABLED
status for better discoverability.

* revert: remove help menu experiment display changes

Reverts the EXPERIMENTS section changes from the experiments
migration commit to keep PR #1 focused on the array-to-map
refactor. Help menu improvements will be done in a separate PR.

* test: tighten up the tests

* test: tighten up the tests

* refactor: use experiment.Experiment type as experiments map key

Use the stricter experiment.Experiment type instead of string for the
internal Config.experiments map key to improve type safety.

* fix: reorder experiment loading so flags have highest priority

Reorder experiment source precedence so that higher-priority sources
override lower ones: permanently enabled < system config < project
config < flags.

* fix: remove backwards compatibility for old experiment array format

Remove custom UnmarshalJSON methods and the unmarshalExperimentsField
helper that supported the deprecated array format for experiments in
config.json. The old format (["exp1"]) now produces a parse error with
a remediation hint to update to the object format ({"exp1": true}).

Author: mwbrooks

internal/config/config.go

@@ -63,7 +63,7 @@ type Config struct {
 
 	// Feature experiments
 	ExperimentsFlag []string
-	experiments     []experiment.Experiment
+	experiments     map[experiment.Experiment]bool
 
 	// Eventually this will also load the global and project slack config files
 	DomainAuthTokens string

internal/config/experiments.go

@@ -17,59 +17,100 @@ package config
 import (
 	"context"
 	"fmt"
-	"slices"
+	"maps"
+	"sort"
+	"strings"
 
 	"github.com/slackapi/slack-cli/internal/experiment"
 	"github.com/slackapi/slack-cli/internal/slackerror"
 )
 
+// experimentsFormatHint is a remediation hint for config files that use the
+// deprecated array format for experiments instead of the current object format.
+const experimentsFormatHint = `If the "experiments" field is an array (e.g. ["exp1"]), update it to an object (e.g. {"exp1": true})`
+
 // LoadExperiments parses experiments from the command flags and configuration
 // files and stores the findings in Config
 func (c *Config) LoadExperiments(
 	ctx context.Context,
 	printDebug func(ctx context.Context, format string, a ...interface{}),
 ) {
-	experiments := []experiment.Experiment{}
-	// Load from flags
-	for _, flagValue := range c.ExperimentsFlag {
-		experiments = append(experiments, experiment.Experiment(flagValue))
+	experiments := map[experiment.Experiment]bool{}
+	// Load from permanently enabled list (lowest priority)
+	for _, exp := range experiment.EnabledExperiments {
+		experiments[exp] = true
+	}
+	printDebug(ctx, fmt.Sprintf("active permanently enabled experiments: %s", experiment.EnabledExperiments))
+	// Load from system config file
+	userConfig, err := c.SystemConfig.UserConfig(ctx)
+	if err != nil {
+		printDebug(ctx, fmt.Sprintf("failed to parse system-level config file: %s", err))
+	} else {
+		printDebug(ctx, fmt.Sprintf("active system experiments: %s", formatExperimentMap(toExperimentMap(userConfig.Experiments))))
+		maps.Copy(experiments, toExperimentMap(userConfig.Experiments))
 	}
-	printDebug(ctx, fmt.Sprintf("active flag experiments: %s", experiments))
 	// Load from project config file
 	projectConfig, err := ReadProjectConfigFile(ctx, c.fs, c.os)
 	if err != nil && slackerror.ToSlackError(err).Code != slackerror.ErrInvalidAppDirectory {
 		printDebug(ctx, fmt.Sprintf("failed to parse project-level config file: %s", err))
 	} else if err == nil {
-		printDebug(ctx, fmt.Sprintf("active project experiments: %s", projectConfig.Experiments))
-		experiments = append(experiments, projectConfig.Experiments...)
+		printDebug(ctx, fmt.Sprintf("active project experiments: %s", formatExperimentMap(toExperimentMap(projectConfig.Experiments))))
+		maps.Copy(experiments, toExperimentMap(projectConfig.Experiments))
 	}
-	// Load from system config file
-	userConfig, err := c.SystemConfig.UserConfig(ctx)
-	if err != nil {
-		printDebug(ctx, fmt.Sprintf("failed to parse system-level config file: %s", err))
-	} else {
-		printDebug(ctx, fmt.Sprintf("active system experiments: %s", userConfig.Experiments))
-		experiments = append(experiments, userConfig.Experiments...)
+	// Load from flags (highest priority)
+	for _, flagValue := range c.ExperimentsFlag {
+		experiments[experiment.Experiment(flagValue)] = true
 	}
-	// Load from permanently enabled list
-	experiments = append(experiments, experiment.EnabledExperiments...)
-	printDebug(ctx, fmt.Sprintf("active permanently enabled experiments: %s", experiment.EnabledExperiments))
-	// Sort, trim, and audit the experiments list
-	slices.Sort(experiments)
-	c.experiments = slices.Compact(experiments)
-	for _, exp := range c.experiments {
-		if !experiment.Includes(exp) {
-			printDebug(ctx, fmt.Sprintf("invalid experiment found: %s", exp))
+	printDebug(ctx, fmt.Sprintf("active flag experiments: %s", formatExperimentMap(experiments)))
+	// Audit the experiments
+	c.experiments = experiments
+	for name := range c.experiments {
+		if !experiment.Includes(name) {
+			printDebug(ctx, fmt.Sprintf("invalid experiment found: %s", name))
 		}
 	}
 }
 
 // GetExperiments returns the set of active experiments
 func (c *Config) GetExperiments() []experiment.Experiment {
-	return c.experiments
+	result := make([]experiment.Experiment, 0, len(c.experiments))
+	for name, enabled := range c.experiments {
+		if enabled {
+			result = append(result, name)
+		}
+	}
+	sort.Slice(result, func(i, j int) bool {
+		return result[i] < result[j]
+	})
+	return result
 }
 
 // WithExperimentOn checks whether an experiment is currently toggled on
 func (c *Config) WithExperimentOn(experimentToCheck experiment.Experiment) bool {
-	return slices.Contains(c.experiments, experimentToCheck)
+	return c.experiments[experimentToCheck]
+}
+
+// toExperimentMap converts a map[string]bool to map[experiment.Experiment]bool
+func toExperimentMap(m map[string]bool) map[experiment.Experiment]bool {
+	result := make(map[experiment.Experiment]bool, len(m))
+	for name, enabled := range m {
+		result[experiment.Experiment(name)] = enabled
+	}
+	return result
+}
+
+// formatExperimentMap returns a string representation of the experiments map
+// for debug logging, formatted similar to the old slice format.
+func formatExperimentMap(m map[experiment.Experiment]bool) string {
+	if len(m) == 0 {
+		return "[]"
+	}
+	names := make([]string, 0, len(m))
+	for name, enabled := range m {
+		if enabled {
+			names = append(names, string(name))
+		}
+	}
+	sort.Strings(names)
+	return "[" + strings.Join(names, " ") + "]"
 }

internal/config/experiments_test.go

@@ -19,7 +19,6 @@ import (
 	"context"
 	"fmt"
 	"path/filepath"
-	"slices"
 	"testing"
 
 	"github.com/slackapi/slack-cli/internal/experiment"
@@ -40,7 +39,7 @@ func Test_Config_WithExperimentOn(t *testing.T) {
 		mockOutput, mockPrintDebug := setupMockPrintDebug()
 
 		// Write our test script to the memory file system used by the mock
-		jsonContents := []byte(fmt.Sprintf("{\"last_update_checked_at\":\"2023-05-11T15:41:07.799619-07:00\",\"experiments\":[\"%s\"]}", validExperiment))
+		jsonContents := []byte(fmt.Sprintf(`{"last_update_checked_at":"2023-05-11T15:41:07.799619-07:00","experiments":{"%s":true}}`, validExperiment))
 		_ = afero.WriteFile(fs, pathToConfigJSON, jsonContents, 0600)
 
 		config.LoadExperiments(ctx, mockPrintDebug)
@@ -50,13 +49,34 @@ func Test_Config_WithExperimentOn(t *testing.T) {
 			fmt.Sprintf("active system experiments: [%s]", validExperiment))
 	})
 
+	t.Run("Returns a parse error with remediation for old array format in config.json", func(t *testing.T) {
+		// Setup
+		ctx, fs, _, config, pathToConfigJSON, teardown := setup(t)
+		defer teardown(t)
+		mockOutput, mockPrintDebug := setupMockPrintDebug()
+
+		// Write old array format
+		jsonContents := []byte(
+			fmt.Sprintf(
+				`{"last_update_checked_at":"2023-05-11T15:41:07.799619-07:00","experiments":["%s"]}`,
+				validExperiment,
+			),
+		)
+		_ = afero.WriteFile(fs, pathToConfigJSON, jsonContents, 0600)
+
+		config.LoadExperiments(ctx, mockPrintDebug)
+		experimentOn := config.WithExperimentOn(validExperiment)
+		assert.Equal(t, false, experimentOn)
+		assert.Contains(t, mockOutput.String(), "failed to parse system-level config file")
+	})
+
 	t.Run("Correctly returns false when experiments are not in config.json", func(t *testing.T) {
 		// Setup
 		ctx, fs, _, config, pathToConfigJSON, teardown := setup(t)
 		defer teardown(t)
 		mockOutput, mockPrintDebug := setupMockPrintDebug()
 
-		jsonContents := []byte("{\"last_update_checked_at\":\"2023-05-11T15:41:07.799619-07:00\",\"experiments\":[]}")
+		jsonContents := []byte(`{"last_update_checked_at":"2023-05-11T15:41:07.799619-07:00","experiments":{}}`)
 		_ = afero.WriteFile(fs, pathToConfigJSON, jsonContents, 0600)
 
 		config.LoadExperiments(ctx, mockPrintDebug)
@@ -72,7 +92,7 @@ func Test_Config_WithExperimentOn(t *testing.T) {
 		mockOutput, mockPrintDebug := setupMockPrintDebug()
 
 		// Write no contents via config.json
-		jsonContents := []byte("{\"last_update_checked_at\":\"2023-05-11T15:41:07.799619-07:00\",\"experiments\":[]}")
+		jsonContents := []byte(`{"last_update_checked_at":"2023-05-11T15:41:07.799619-07:00","experiments":{}}`)
 		_ = afero.WriteFile(fs, pathToConfigJSON, jsonContents, 0600)
 		config.ExperimentsFlag = []string{string(validExperiment)}
 
@@ -89,7 +109,7 @@ func Test_Config_WithExperimentOn(t *testing.T) {
 		defer teardown(t)
 		mockOutput, mockPrintDebug := setupMockPrintDebug()
 
-		jsonContents := []byte(fmt.Sprintf("{\"last_update_checked_at\":\"2023-05-11T15:41:07.799619-07:00\",\"experiments\":[\"%s\"]}", invalidExperiment))
+		jsonContents := []byte(fmt.Sprintf(`{"last_update_checked_at":"2023-05-11T15:41:07.799619-07:00","experiments":{"%s":true}}`, invalidExperiment))
 		_ = afero.WriteFile(fs, pathToConfigJSON, jsonContents, 0600)
 		config.LoadExperiments(ctx, mockPrintDebug)
 		assert.Contains(t, mockOutput.String(),
@@ -120,7 +140,7 @@ func Test_Config_WithExperimentOn(t *testing.T) {
 				mockOutput, mockPrintDebug := setupMockPrintDebug()
 
 				// Write contents via config.json
-				jsonContents := []byte(fmt.Sprintf("{\"last_update_checked_at\":\"2023-05-11T15:41:07.799619-07:00\",\"experiments\":[\"%s\"]}", tc.experiment))
+				jsonContents := []byte(fmt.Sprintf(`{"last_update_checked_at":"2023-05-11T15:41:07.799619-07:00","experiments":{"%s":true}}`, tc.experiment))
 				_ = afero.WriteFile(fs, pathToConfigJSON, jsonContents, 0600)
 
 				config.LoadExperiments(ctx, mockPrintDebug)
@@ -172,7 +192,7 @@ func Test_Config_WithExperimentOn(t *testing.T) {
 		require.NoError(t, err)
 		err = afero.WriteFile(fs, GetProjectHooksJSONFilePath(slackdeps.MockWorkingDirectory), []byte("{}\n"), 0600)
 		require.NoError(t, err)
-		jsonContents := []byte(fmt.Sprintf(`{"experiments":["%s"]}`, experiment.Placeholder))
+		jsonContents := []byte(fmt.Sprintf(`{"experiments":{"%s":true}}`, experiment.Placeholder))
 		err = afero.WriteFile(fs, GetProjectConfigJSONFilePath(slackdeps.MockWorkingDirectory), []byte(jsonContents), 0600)
 		require.NoError(t, err)
 
@@ -185,36 +205,37 @@ func Test_Config_WithExperimentOn(t *testing.T) {
 	t.Run("Loads valid experiments from project configs and removes duplicates", func(t *testing.T) {
 		ctx, fs, _, config, _, teardown := setup(t)
 		defer teardown(t)
-		mockOutput, mockPrintDebug := setupMockPrintDebug()
+		_, mockPrintDebug := setupMockPrintDebug()
 		err := fs.Mkdir(slackdeps.MockWorkingDirectory, 0755)
 		require.NoError(t, err)
 		err = fs.Mkdir(filepath.Join(slackdeps.MockWorkingDirectory, ProjectConfigDirName), 0755)
 		require.NoError(t, err)
 		err = afero.WriteFile(fs, GetProjectHooksJSONFilePath(slackdeps.MockWorkingDirectory), []byte("{}\n"), 0600)
 		require.NoError(t, err)
-		jsonContents := []byte(fmt.Sprintf(
-			`{"experiments":["%s", "%s", "%s"]}`,
-			experiment.Placeholder,
-			experiment.Placeholder,
-			experiment.Placeholder,
-		))
+		jsonContents := []byte(
+			fmt.Sprintf(`{"experiments":{"%s":true, "%s":true, "%s":true}}`,
+				experiment.Placeholder,
+				experiment.Placeholder,
+				experiment.Placeholder,
+			),
+		)
 		err = afero.WriteFile(fs, GetProjectConfigJSONFilePath(slackdeps.MockWorkingDirectory), []byte(jsonContents), 0600)
 		require.NoError(t, err)
 
+		// Also set via flag to test dedup
+		config.ExperimentsFlag = []string{string(experiment.Placeholder)}
+
 		config.LoadExperiments(ctx, mockPrintDebug)
 		assert.True(t, config.WithExperimentOn(experiment.Placeholder))
-		assert.Contains(t, mockOutput.String(), fmt.Sprintf(
-			"active project experiments: [%s %s %s]",
-			experiment.Placeholder,
-			experiment.Placeholder,
-			experiment.Placeholder,
-		))
-		// Assert that duplicates are removed and slice length is reduced
-		// Add in the permanently enabled experiments before comparing
-		activeExperiments := append(experiment.EnabledExperiments, experiment.Placeholder)
-		slices.Sort(activeExperiments)
-		activeExperiments = slices.Compact(activeExperiments)
-		assert.Equal(t, activeExperiments, config.experiments)
+		// Assert that experiments are deduplicated via map
+		exps := config.GetExperiments()
+		count := 0
+		for _, exp := range exps {
+			if exp == experiment.Placeholder {
+				count++
+			}
+		}
+		assert.Equal(t, 1, count, "experiment should appear exactly once")
 	})
 
 	t.Run("Loads valid experiments and enabled default experiments", func(t *testing.T) {
@@ -227,7 +248,7 @@ func Test_Config_WithExperimentOn(t *testing.T) {
 		require.NoError(t, err)
 		err = afero.WriteFile(fs, GetProjectHooksJSONFilePath(slackdeps.MockWorkingDirectory), []byte("{}\n"), 0600)
 		require.NoError(t, err)
-		jsonContents := []byte(`{"experiments":[]}`) // No experiments
+		jsonContents := []byte(`{"experiments":{}}`) // No experiments
 		err = afero.WriteFile(fs, GetProjectConfigJSONFilePath(slackdeps.MockWorkingDirectory), []byte(jsonContents), 0600)
 		require.NoError(t, err)
 
@@ -244,7 +265,7 @@ func Test_Config_WithExperimentOn(t *testing.T) {
 		assert.True(t, config.WithExperimentOn(experiment.Placeholder))
 		assert.Contains(t, mockOutput.String(), "active project experiments: []")
 		assert.Contains(t, mockOutput.String(), fmt.Sprintf("active permanently enabled experiments: [%s]", experiment.Placeholder))
-		assert.Equal(t, []experiment.Experiment{experiment.Placeholder}, config.experiments)
+		assert.ElementsMatch(t, []experiment.Experiment{experiment.Placeholder}, config.GetExperiments())
 	})
 
 	t.Run("Logs an invalid experiments in project configs", func(t *testing.T) {
@@ -257,7 +278,7 @@ func Test_Config_WithExperimentOn(t *testing.T) {
 		require.NoError(t, err)
 		err = afero.WriteFile(fs, GetProjectHooksJSONFilePath(slackdeps.MockWorkingDirectory), []byte("{}\n"), 0600)
 		require.NoError(t, err)
-		jsonContents := []byte(fmt.Sprintf("{\"experiments\":[\"%s\"]}", "experiment-37"))
+		jsonContents := []byte(`{"experiments":{"experiment-37":true}}`)
 		err = afero.WriteFile(fs, GetProjectConfigJSONFilePath(slackdeps.MockWorkingDirectory), []byte(jsonContents), 0600)
 		require.NoError(t, err)
 
@@ -283,7 +304,7 @@ func Test_Config_GetExperiments(t *testing.T) {
 		_, mockPrintDebug := setupMockPrintDebug()
 
 		// Write contents via config.json
-		var configJSON = []byte(fmt.Sprintf("{\"last_update_checked_at\":\"2023-05-11T15:41:07.799619-07:00\",\"experiments\":[\"%s\",\"%s\"]}", validExperiment, validExperiment))
+		var configJSON = []byte(fmt.Sprintf(`{"last_update_checked_at":"2023-05-11T15:41:07.799619-07:00","experiments":{"%s":true}}`, validExperiment))
 		_ = afero.WriteFile(fs, pathToConfigJSON, configJSON, 0600)
 
 		// Set contexts of experiment flag

internal/config/project.go

@@ -25,7 +25,6 @@ import (
 	"github.com/google/uuid"
 	"github.com/opentracing/opentracing-go"
 	"github.com/slackapi/slack-cli/internal/cache"
-	"github.com/slackapi/slack-cli/internal/experiment"
 	"github.com/slackapi/slack-cli/internal/shared/types"
 	"github.com/slackapi/slack-cli/internal/slackerror"
 	"github.com/slackapi/slack-cli/internal/style"
@@ -60,7 +59,7 @@ type ProjectConfigManager interface {
 
 // ProjectConfig is the project-level config file
 type ProjectConfig struct {
-	Experiments []experiment.Experiment `json:"experiments,omitempty"`
+	Experiments map[string]bool         `json:"experiments,omitempty"`
 	Manifest    *ManifestConfig         `json:"manifest,omitempty"`
 	ProjectID   string                  `json:"project_id,omitempty"`
 	Surveys     map[string]SurveyConfig `json:"surveys,omitempty"`
@@ -260,7 +259,11 @@ func ReadProjectConfigFile(ctx context.Context, fs afero.Fs, os types.Os) (Proje
 		return projectConfig, slackerror.New(slackerror.ErrUnableToParseJSON).
 			WithMessage("Failed to parse contents of project-level config file").
 			WithRootCause(err).
-			WithRemediation("Check that %s is valid JSON", style.HomePath(projectConfigFilePath))
+			WithRemediation(
+				"Check that %s is valid JSON. %s",
+				style.HomePath(projectConfigFilePath),
+				experimentsFormatHint,
+			)
 	}
 	if projectConfig.Surveys == nil {
 		projectConfig.Surveys = map[string]SurveyConfig{}

internal/config/system.go

@@ -26,7 +26,6 @@ import (
 
 	"github.com/google/uuid"
 	"github.com/opentracing/opentracing-go"
-	"github.com/slackapi/slack-cli/internal/experiment"
 	"github.com/slackapi/slack-cli/internal/shared/types"
 	"github.com/slackapi/slack-cli/internal/slackerror"
 	"github.com/slackapi/slack-cli/internal/style"
@@ -63,7 +62,7 @@ type SystemConfigManager interface {
 
 // SystemConfig contains the system-level config file
 type SystemConfig struct {
-	Experiments         []experiment.Experiment `json:"experiments,omitempty"`
+	Experiments         map[string]bool         `json:"experiments,omitempty"`
 	LastUpdateCheckedAt time.Time               `json:"last_update_checked_at,omitempty"`
 	Surveys             map[string]SurveyConfig `json:"surveys,omitempty"`
 	SystemID            string                  `json:"system_id,omitempty"`
@@ -133,7 +132,11 @@ func (c *SystemConfig) UserConfig(ctx context.Context) (*SystemConfig, error) {
 		return &config, slackerror.New(slackerror.ErrUnableToParseJSON).
 			WithMessage("Failed to parse contents of system-level config file").
 			WithRootCause(err).
-			WithRemediation("Check that %s is valid JSON", style.HomePath(path))
+			WithRemediation(
+				"Check that %s is valid JSON. %s",
+				style.HomePath(path),
+				experimentsFormatHint,
+			)
 	}
 
 	if config.Surveys == nil {