Recursively order nested with/env/secrets maps during YAML serialization

docs/adr/40362-order-nested-maps-during-yaml-serialization.md

@@ -0,0 +1,38 @@
+# ADR-40362: Recursively Order Nested `with`/`env`/`secrets` Maps During YAML Serialization
+
+**Date**: 2026-06-19
+**Status**: Draft
+
+## Context
+
+The compiler serializes workflow data structures (`map[string]any`) to YAML, and reproducible output is required so that repeated compilations and wasm/frontmatter round-trips produce byte-identical results. Top-level step fields were already ordered deterministically, but nested `with:`, `env:`, and `secrets:` maps were emitted in Go's randomized map-iteration order. This caused the same input to serialize differently across runs, breaking determinism guarantees and round-trip stability tests. The ordering needed to apply recursively, since nested values can themselves contain maps (e.g. a `with` input whose value is another map).
+
+## Decision
+
+We will normalize nested map values inside the YAML serialization path rather than at each call site. `OrderMapFields` now routes every value through `prepareNestedMapValueForYAML`, which recursively converts `with`, `env`, and `secrets` sub-maps into key-sorted `yaml.MapSlice` values via `recursivelyOrderYAMLValue`. This centralizes deterministic ordering in one place, preserves existing top-level ordering behavior, and guarantees stable nested output (e.g. `alpha-*` before `zeta-*`) independent of Go map iteration.
+
+## Alternatives Considered
+
+### Alternative 1: Sort at each construction call site
+Each place that builds a `with`/`env`/`secrets` map could emit an ordered structure directly. Rejected because it scatters ordering logic across many call sites, is easy to forget for new call sites, and does not handle arbitrarily nested values uniformly.
+
+### Alternative 2: Thread an ordered map type through the whole pipeline
+Replace `map[string]any` with `yaml.MapSlice` (or a custom ordered map) everywhere these structures are built and passed around. Rejected as too invasive for the scope of fixing determinism: it would touch large amounts of unrelated code and change many function signatures, with high regression risk.
+
+## Consequences
+
+### Positive
+- Compiled YAML is deterministic for nested `with`/`env`/`secrets` blocks, fixing round-trip and repeated-compilation stability.
+- Ordering logic is centralized in the serialization path, so new call sites benefit automatically.
+
+### Negative
+- Each marshal performs extra allocation and recursive copying of nested values.
+- The special-cased field list (`with`, `env`, `secrets`) is hardcoded; adding another field that needs ordering requires editing `prepareNestedMapValueForYAML`.
+
+### Neutral
+- Non-string `yaml.MapSlice` keys are passed through without assuming string type, preserving correctness for already-ordered inputs.
+- Behavior is exercised by a focused `yaml_test.go` case and an expanded wasm golden round-trip test.
+
+---
+
+*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/27840723836) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*

pkg/workflow/wasm_golden_test.go

@@ -162,6 +162,18 @@ on:
   workflow_dispatch:
 permissions:
   contents: read
+env:
+  ZETA_WORKFLOW: zeta
+  ALPHA_WORKFLOW: alpha
+steps:
+  - name: Deterministic uses step
+    uses: actions/cache/restore@v4
+    with:
+      zeta-input: zeta
+      alpha-input: alpha
+    env:
+      ZETA_STEP: zeta
+      ALPHA_STEP: alpha
 engine: copilot
 timeout-minutes: 10
 ---
@@ -189,6 +201,21 @@ This workflow tests that compilation is deterministic.
 
 	require.Equal(t, normalizeHeredocDelimiters(results[0]), normalizeHeredocDelimiters(results[1]), "compilation 1 and 2 differ")
 	require.Equal(t, normalizeHeredocDelimiters(results[1]), normalizeHeredocDelimiters(results[2]), "compilation 2 and 3 differ")
+
+	assertOrder := func(before, after string) {
+		t.Helper()
+		beforeIndex := strings.Index(results[0], before)
+		afterIndex := strings.Index(results[0], after)
+		require.NotEqual(t, -1, beforeIndex, "expected %q in compiled YAML", before)
+		require.NotEqual(t, -1, afterIndex, "expected %q in compiled YAML", after)
+		if beforeIndex >= afterIndex {
+			t.Fatalf("expected %q before %q in compiled YAML:\n%s", before, after, results[0])
+		}
+	}
+
+	assertOrder("  ALPHA_WORKFLOW:", "  ZETA_WORKFLOW:")
+	assertOrder("      alpha-input:", "      zeta-input:")
+	assertOrder("      ALPHA_STEP:", "      ZETA_STEP:")
 }
 
 // TestWasmGolden_NativeVsStringAPI compiles a workflow using both the native

pkg/workflow/yaml.go

@@ -314,7 +314,7 @@ func OrderMapFields(data map[string]any, priorityFields []string) yaml.MapSlice
 	// This ensures important fields like "name", "on", "jobs" appear first
 	for _, fieldName := range priorityFields {
 		if value, exists := data[fieldName]; exists {
-			orderedData = append(orderedData, yaml.MapItem{Key: fieldName, Value: value})
+			orderedData = append(orderedData, yaml.MapItem{Key: fieldName, Value: prepareNestedMapValueForYAML(fieldName, value)})
 		}
 	}
 
@@ -334,12 +334,86 @@ func OrderMapFields(data map[string]any, priorityFields []string) yaml.MapSlice
 
 	// Phase 4: Add remaining fields to the ordered map
 	for _, key := range remainingKeys {
-		orderedData = append(orderedData, yaml.MapItem{Key: key, Value: data[key]})
+		orderedData = append(orderedData, yaml.MapItem{Key: key, Value: prepareNestedMapValueForYAML(key, data[key])})
 	}
 
 	return orderedData
 }
 
+func prepareNestedMapValueForYAML(fieldName string, value any) any {
+	switch v := value.(type) {
+	case map[string]any:
+		if fieldName == "with" || fieldName == "env" || fieldName == "secrets" {
+			return recursivelyOrderYAMLValue(v)
+		}
+		copied := make(map[string]any, len(v))
+		for key, childValue := range v {
+			copied[key] = prepareNestedMapValueForYAML(key, childValue)
+		}
+		return copied
+	case map[string]string:
+		if fieldName == "with" || fieldName == "env" || fieldName == "secrets" {
+			return recursivelyOrderYAMLValue(v)
+		}
+		return value
+	case []any:
+		copied := make([]any, len(v))
+		for i, childValue := range v {
+			copied[i] = prepareNestedMapValueForYAML("", childValue)
+		}
+		return copied
+	case yaml.MapSlice:
+		copied := make(yaml.MapSlice, 0, len(v))
+		for _, item := range v {
+			key, ok := item.Key.(string)
+			if !ok {
+				copied = append(copied, yaml.MapItem{Key: item.Key, Value: prepareNestedMapValueForYAML("", item.Value)})
+				continue
+			}
+			copied = append(copied, yaml.MapItem{Key: item.Key, Value: prepareNestedMapValueForYAML(key, item.Value)})
+		}
+		return copied
+	default:
+		return value
+	}
+}
+
+func recursivelyOrderYAMLValue(value any) any {
+	switch v := value.(type) {
+	case map[string]any:
+		orderedData := make(yaml.MapSlice, 0, len(v))
+		var keys []string
+		for key := range v {
+			keys = append(keys, key)
+		}
+		sort.Strings(keys)
+		for _, key := range keys {
+			orderedData = append(orderedData, yaml.MapItem{Key: key, Value: recursivelyOrderYAMLValue(v[key])})
+		}
+		return orderedData
+	case map[string]string:
+		orderedData := make(yaml.MapSlice, 0, len(v))
+		for _, key := range sortedMapKeys(v) {
+			orderedData = append(orderedData, yaml.MapItem{Key: key, Value: v[key]})
+		}
+		return orderedData
+	case []any:
+		copied := make([]any, len(v))
+		for i, childValue := range v {
+			copied[i] = recursivelyOrderYAMLValue(childValue)
+		}
+		return copied
+	case yaml.MapSlice:
+		copied := make(yaml.MapSlice, 0, len(v))
+		for _, item := range v {
+			copied = append(copied, yaml.MapItem{Key: item.Key, Value: recursivelyOrderYAMLValue(item.Value)})
+		}
+		return copied
+	default:
+		return value
+	}
+}
+
 // CleanYAMLNullValues removes " null" from YAML key-value pairs where the value is null.
 //
 // GitHub Actions YAML treats workflow_dispatch: and workflow_dispatch: null identically,

pkg/workflow/yaml_test.go

@@ -346,6 +346,60 @@ func TestMarshalWithFieldOrder(t *testing.T) {
 	}
 }
 
+func TestMarshalWithFieldOrder_OrdersNestedEnvWithSecretsRecursively(t *testing.T) {
+	data := map[string]any{
+		"env": map[string]string{
+			"ZETA_WORKFLOW":  "zeta",
+			"ALPHA_WORKFLOW": "alpha",
+		},
+		"secrets": map[string]string{
+			"ZETA_SECRET":  "${{ secrets.ZETA_SECRET }}",
+			"ALPHA_SECRET": "${{ secrets.ALPHA_SECRET }}",
+		},
+		"steps": []any{
+			map[string]any{
+				"name": "Deterministic action",
+				"uses": "actions/checkout@v4",
+				"with": map[string]any{
+					"zeta-input": "zeta",
+					"alpha-input": map[string]any{
+						"zeta-child":  "zeta",
+						"alpha-child": "alpha",
+					},
+				},
+				"env": map[string]string{
+					"ZETA_STEP":  "zeta",
+					"ALPHA_STEP": "alpha",
+				},
+			},
+		},
+	}
+
+	yamlBytes, err := MarshalWithFieldOrder(data, []string{"env", "secrets", "steps"})
+	if err != nil {
+		t.Fatalf("MarshalWithFieldOrder() error = %v", err)
+	}
+
+	yamlStr := string(yamlBytes)
+	assertOrder := func(before, after string) {
+		t.Helper()
+		beforeIndex := strings.Index(yamlStr, before)
+		afterIndex := strings.Index(yamlStr, after)
+		if beforeIndex == -1 || afterIndex == -1 {
+			t.Fatalf("expected both %q and %q in YAML:\n%s", before, after, yamlStr)
+		}
+		if beforeIndex >= afterIndex {
+			t.Fatalf("expected %q before %q in YAML:\n%s", before, after, yamlStr)
+		}
+	}
+
+	assertOrder("  ALPHA_WORKFLOW:", "  ZETA_WORKFLOW:")
+	assertOrder("  ALPHA_SECRET:", "  ZETA_SECRET:")
+	assertOrder("    alpha-input:", "    zeta-input:")
+	assertOrder("      alpha-child:", "      zeta-child:")
+	assertOrder("    ALPHA_STEP:", "    ZETA_STEP:")
+}
+
 func TestExtractTopLevelYAMLSectionWithOrdering(t *testing.T) {
 	compiler := NewCompiler()