refactor: encapsulate CLI flag state to enable parallel testing

[jongio]

Code Smell: Inappropriate Intimacy (Couplers)

Severity: HIGH
Category: Couplers > Inappropriate Intimacy
File: cli/src/cmd/exec/main.go (lines 22-29), cli/src/cmd/exec/main_test.go

Description

The CLI flag values are stored as package-level mutable variables:

var (
    shell       string
    interactive bool
    stopOnKeyVaultError bool
)

Tests directly mutate these package-level variables to control behavior:

// main_test.go line 99-100
shell = ""
interactive = false

This creates tight coupling between the test and the internal implementation. The package-level state also means:

• Tests cannot run in parallel (t.Parallel()) because they share mutable state
• Test ordering matters — one test's mutations affect subsequent tests
• The newScriptExecutor function variable (line 36) is also package-level mutable state used for test injection

Refactoring Recommendation

Encapsulate flag state in a struct and pass it through the command tree:

type appConfig struct {
    shell               string
    interactive         bool
    stopOnKeyVaultError bool
    newExecutor         func(executor.Config) (scriptExecutor, error)
}

func newRootCmd(cfg *appConfig) *cobra.Command {
    rootCmd.Flags().StringVarP(&cfg.shell, "shell", "s", "", "...")
    // ...
}

Tests create their own appConfig instances without touching shared state:

func TestRunE(t *testing.T) {
    t.Parallel() // Now safe
    cfg := &appConfig{newExecutor: func(...) { ... }}
    cmd := newRootCmd(cfg)
    // ...
}

Impact

• Test isolation: Tests cannot safely run in parallel
• Hidden dependencies: Test behavior depends on execution order
• Fragile tests: Adding a new flag requires updating all tests that reset state