fix: address code review feedback

- Sort map keys in `env show` output for deterministic display
- Move values file loading before DB open in run.go (fail-fast)
- Detect env key collisions during uppercase normalization in values.go

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: michaelmcnees

packages/engine/internal/cli/env.go

@@ -2,6 +2,7 @@ package cli
 
 import (
 	"fmt"
+	"sort"
 	"text/tabwriter"
 
 	"github.com/dvflw/mantle/internal/config"
@@ -117,14 +118,24 @@ func newEnvShowCommand() *cobra.Command {
 			fmt.Fprintf(cmd.OutOrStdout(), "Name: %s\n", env.Name)
 			if len(env.Inputs) > 0 {
 				fmt.Fprintln(cmd.OutOrStdout(), "\nInputs:")
-				for k, v := range env.Inputs {
-					fmt.Fprintf(cmd.OutOrStdout(), "  %s: %v\n", k, v)
+				keys := make([]string, 0, len(env.Inputs))
+				for k := range env.Inputs {
+					keys = append(keys, k)
+				}
+				sort.Strings(keys)
+				for _, k := range keys {
+					fmt.Fprintf(cmd.OutOrStdout(), "  %s: %v\n", k, env.Inputs[k])
 				}
 			}
 			if len(env.Env) > 0 {
 				fmt.Fprintln(cmd.OutOrStdout(), "\nEnv:")
-				for k, v := range env.Env {
-					fmt.Fprintf(cmd.OutOrStdout(), "  %s: %s\n", k, v)
+				keys := make([]string, 0, len(env.Env))
+				for k := range env.Env {
+					keys = append(keys, k)
+				}
+				sort.Strings(keys)
+				for _, k := range keys {
+					fmt.Fprintf(cmd.OutOrStdout(), "  %s: %s\n", k, env.Env[k])
 				}
 			}
 			return nil

packages/engine/internal/cli/run.go

@@ -38,6 +38,28 @@ func newRunCommand() *cobra.Command {
 				return fmt.Errorf("config not loaded")
 			}
 
+			// Parse --input flags early (no I/O needed).
+			inputs := make(map[string]any)
+			for _, kv := range inputFlags {
+				key, value, ok := strings.Cut(kv, "=")
+				if !ok {
+					return fmt.Errorf("invalid input format %q — expected key=value", kv)
+				}
+				inputs[key] = value
+			}
+
+			// Load values file early so we fail fast on bad input (before DB open).
+			var valuesInputs map[string]any
+			var valuesEnv map[string]string
+			if valuesFile != "" {
+				vals, valErr := workflow.LoadValues(valuesFile)
+				if valErr != nil {
+					return fmt.Errorf("loading values file: %w", valErr)
+				}
+				valuesInputs = vals.Inputs
+				valuesEnv = vals.Env
+			}
+
 			database, err := db.Open(cfg.Database)
 			if err != nil {
 				return fmt.Errorf("failed to connect to database: %w", err)
@@ -66,28 +88,6 @@ func newRunCommand() *cobra.Command {
 				envEnvVars = storedEnv.Env
 			}
 
-			// Parse --input flags.
-			inputs := make(map[string]any)
-			for _, kv := range inputFlags {
-				key, value, ok := strings.Cut(kv, "=")
-				if !ok {
-					return fmt.Errorf("invalid input format %q — expected key=value", kv)
-				}
-				inputs[key] = value
-			}
-
-			// Load values file and merge all input layers.
-			var valuesInputs map[string]any
-			var valuesEnv map[string]string
-			if valuesFile != "" {
-				vals, valErr := workflow.LoadValues(valuesFile)
-				if valErr != nil {
-					return fmt.Errorf("loading values file: %w", valErr)
-				}
-				valuesInputs = vals.Inputs
-				valuesEnv = vals.Env
-			}
-
 			// NOTE: MergeInputs does not apply workflow defaults (that's the engine's job
 			// during execution). Pass nil for workflowInputs here — the engine handles it.
 			mergedInputs := workflow.MergeInputs(nil, envInputs, valuesInputs, inputs)

packages/engine/internal/workflow/values.go

@@ -45,10 +45,17 @@ func LoadValuesBytes(data []byte) (*Values, error) {
 	}
 
 	// Normalize env keys to uppercase (matches config.go behavior).
+	// Detect collisions where different-cased keys map to the same uppercase key.
 	if len(vals.Env) > 0 {
 		normalized := make(map[string]string, len(vals.Env))
+		sources := make(map[string]string, len(vals.Env)) // uppercase key -> original key
 		for k, v := range vals.Env {
-			normalized[strings.ToUpper(k)] = v
+			upper := strings.ToUpper(k)
+			if orig, exists := sources[upper]; exists && orig != k {
+				return nil, fmt.Errorf("env key collision: %q and %q both normalize to %q", orig, k, upper)
+			}
+			sources[upper] = k
+			normalized[upper] = v
 		}
 		vals.Env = normalized
 	}

packages/engine/internal/workflow/values_test.go

@@ -169,6 +169,21 @@ env:
 	}
 }
 
+func TestLoadValues_EnvKeyCollision(t *testing.T) {
+	path := writeValuesFile(t, `
+env:
+  api_key: lowercase_value
+  API_KEY: uppercase_value
+`)
+	_, err := LoadValues(path)
+	if err == nil {
+		t.Fatal("expected error for env key collision")
+	}
+	if !strings.Contains(err.Error(), "collision") {
+		t.Errorf("error = %q, want it to contain %q", err.Error(), "collision")
+	}
+}
+
 func TestLoadValues_EmptyFile(t *testing.T) {
 	path := writeValuesFile(t, "")