fix(state): preserve state.components on flat single-component write

CHANGELOG.md

@@ -16,6 +16,18 @@ A `Migration` section is added to any release that bumps `schema_version`.
 
 ### Fixed
 
+- **state:** A single-component (flat) state write on a manifest that carries
+  per-component state no longer destroys that state. The flat state map models
+  environments only, so parsing a component-scoped manifest lifted
+  `state.components` into it as an empty entry keyed `components`; rebuilding the
+  whole `state` node from that map then collapsed `state.components` to `{}` and
+  silently dropped every recorded component row while the write returned success.
+  A finalize invoked without `--component` on such a manifest (a stale
+  pre-migration workflow or a manual invocation) could land that loss on trunk.
+  The flat write path now treats `components` as a reserved, unowned subtree: it
+  is never rebuilt from the flat map and any existing subtree is preserved
+  verbatim. A single-component manifest with no `components` subtree emits
+  byte-identical output.
 - **generate:** A callback declaring `retries` is now judged on its ladder's
   effective result, so a deploy that fails and is then rescued by a retry no
   longer fails the run or gets denied in recorded state. A GitHub Actions job

internal/config/statemerge.go

@@ -108,16 +108,37 @@ func WriteScopedState(current []byte, manifestKey string, writes ...StateWrite)
 	return data, nil
 }
 
+// componentsStateKey is the reserved child of a `state` node that holds the
+// per-component state subtree (state.components.<name>.<env>). It is never an
+// environment leaf. The typed flat state map (CICDFile.State) is
+// map[string]*EnvState, so parsing a component-scoped manifest lifts
+// state.components into that map as a bogus, empty EnvState keyed
+// "components". A flat writer that rebuilt the whole state node from that map
+// would emit `state.components: {}` and silently destroy every recorded
+// component row. The single-component write path treats this key as a reserved,
+// unowned sibling: it is never rebuilt from the flat map, and any existing
+// subtree is preserved verbatim across the rebuild.
+const componentsStateKey = "components"
+
 // applySingleComponentWrites reconciles the whole single-component `state` node
 // from the union of set writes (byte-identical to the historical whole-node
-// replacement) and applies the top-level latest_release directive.
+// replacement) and applies the top-level latest_release directive. The reserved
+// `components` subtree (state.components) is never treated as an env leaf: it is
+// excluded from the rebuilt map and any existing subtree is carried over
+// verbatim, so a flat writer on a component-scoped manifest can never wipe it.
 func applySingleComponentWrites(section *yaml.Node, writes []StateWrite) error {
 	state := make(map[string]*EnvState)
 	haveStateDirective := false
 	for _, w := range writes {
 		if w.Env == "" {
 			continue // latest_release directive, handled below
 		}
+		if w.Env == componentsStateKey {
+			// The per-component state subtree, not an env leaf. A flat writer does
+			// not own it; skip it so the rebuild neither emits a bogus components
+			// row nor drops the real subtree (preserved verbatim below).
+			continue
+		}
 		haveStateDirective = true
 		if w.State != nil {
 			state[w.Env] = w.State
@@ -126,13 +147,23 @@ func applySingleComponentWrites(section *yaml.Node, writes []StateWrite) error {
 	}
 
 	if haveStateDirective {
-		if len(state) == 0 {
+		// Preserve an existing state.components subtree verbatim across the rebuild:
+		// the flat writer replaces the whole state node, but the components subtree
+		// is an unmodeled sibling it does not own.
+		var componentsNode *yaml.Node
+		if existing := mappingValue(section, "state"); existing != nil && existing.Kind == yaml.MappingNode {
+			componentsNode = mappingValue(existing, componentsStateKey)
+		}
+		if len(state) == 0 && componentsNode == nil {
 			deleteMappingKey(section, "state")
 		} else {
 			node, err := valueNode(state)
 			if err != nil {
 				return fmt.Errorf("encoding state for state write: %w", err)
 			}
+			if componentsNode != nil {
+				setMappingValue(node, componentsStateKey, componentsNode)
+			}
 			setMappingValue(section, "state", node)
 		}
 	}
@@ -334,6 +365,11 @@ func fetchedStateEnvKeys(current []byte, manifestKey string) []string {
 	}
 	keys := make([]string, 0, len(state.Content)/2)
 	for i := 0; i+1 < len(state.Content); i += 2 {
+		if state.Content[i].Value == componentsStateKey {
+			// Reserved per-component subtree, not an env row: excluded so the
+			// wrapper never derives a delete directive that would drop it.
+			continue
+		}
 		keys = append(keys, state.Content[i].Value)
 	}
 	return keys

internal/config/statemerge_components_preserve_test.go

@@ -0,0 +1,140 @@
+package config
+
+import (
+	"strings"
+	"testing"
+)
+
+// componentStateManifest is a component-scoped manifest carrying recorded state
+// under state.components.<name>.<env>. Parsing it into the flat CICDFile.State
+// map (map[string]*EnvState) lifts state.components into that map as a bogus,
+// empty EnvState keyed "components".
+const componentStateManifest = `ci:
+  config:
+    schema_version: 1
+    trunk_branch: main
+    components:
+      api:
+        environments:
+          - name: staging
+          - name: prod
+  state:
+    components:
+      api:
+        staging:
+          sha: aaa111
+          version: v1.0.0
+        prod:
+          sha: bbb222
+          version: v0.9.0
+`
+
+// TestWriteManifestState_FlatWriteOnComponentManifest_PreservesComponents drives
+// the real production flat-write path (ParseManifestBytes then WriteManifestState,
+// exactly what a finalize invoked without --component runs) against a manifest
+// that carries per-component state. Before the fix, the flat rebuild replaced the
+// whole state node from the typed map, whose bogus "components" EnvState collapsed
+// state.components to {} and silently destroyed every recorded component row while
+// the write returned success. The write must now preserve the subtree verbatim.
+func TestWriteManifestState_FlatWriteOnComponentManifest_PreservesComponents(t *testing.T) {
+	file, err := ParseManifestBytes([]byte(componentStateManifest), "ci")
+	if err != nil {
+		t.Fatalf("ParseManifestBytes: %v", err)
+	}
+	// The flat parse lifts state.components into State as a bogus "components" key.
+	if _, lifted := file.State["components"]; !lifted {
+		t.Fatalf("precondition: expected flat parse to lift a bogus 'components' key, got keys %v", keysOfState(file.State))
+	}
+
+	got, err := WriteManifestState([]byte(componentStateManifest), "ci", file.State, file.LatestRelease)
+	if err != nil {
+		t.Fatalf("WriteManifestState: %v", err)
+	}
+	out := string(got)
+
+	for _, want := range []string{"aaa111", "bbb222", "v1.0.0", "v0.9.0"} {
+		if !strings.Contains(out, want) {
+			t.Errorf("flat write destroyed component state (missing %q):\n%s", want, out)
+		}
+	}
+	if strings.Contains(out, "components: {}") {
+		t.Errorf("flat write collapsed state.components to an empty mapping:\n%s", out)
+	}
+}
+
+// TestWriteManifestState_SingleComponentManifest_ByteIdentical pins that a genuine
+// single-component manifest (no components: subtree) round-trips byte-identically
+// through the fixed flat path, so the reserved-key handling adds nothing when no
+// components subtree exists.
+func TestWriteManifestState_SingleComponentManifest_ByteIdentical(t *testing.T) {
+	const flat = `ci:
+  config:
+    schema_version: 1
+    trunk_branch: main
+    environments:
+      - name: staging
+      - name: prod
+  state:
+    staging:
+      sha: aaa111
+      version: v1.0.0
+    prod:
+      sha: bbb222
+      version: v0.9.0
+`
+	final := map[string]*EnvState{
+		"staging": {SHA: "aaa111", Version: "v1.0.0"},
+		"prod":    {SHA: "bbb222", Version: "v0.9.0"},
+	}
+	got, err := WriteManifestState([]byte(flat), "ci", final, nil)
+	if err != nil {
+		t.Fatalf("WriteManifestState: %v", err)
+	}
+	oracle := referenceWholeNodeReplace(t, []byte(flat), "ci", final, nil)
+	if string(got) != string(oracle) {
+		t.Fatalf("single-component flat write drifted from whole-node-replace oracle\n--- got ---\n%s\n--- want ---\n%s", got, oracle)
+	}
+	if strings.Contains(string(got), "components") {
+		t.Fatalf("single-component write leaked a components key:\n%s", got)
+	}
+}
+
+// TestWriteScopedState_ComponentWrite_PreservesSiblingComponents pins the
+// #614-class invariant on the component-scoped path: writing one component's env
+// leaf must not disturb a sibling component's recorded rows.
+func TestWriteScopedState_ComponentWrite_PreservesSiblingComponents(t *testing.T) {
+	const twoComponents = `ci:
+  config:
+    schema_version: 1
+    trunk_branch: main
+  state:
+    components:
+      api:
+        staging:
+          sha: aaa111
+      web:
+        staging:
+          sha: ccc333
+`
+	got, err := WriteScopedState([]byte(twoComponents), "ci",
+		StateWrite{Component: "api", Env: "staging", State: &EnvState{SHA: "updated"}},
+	)
+	if err != nil {
+		t.Fatalf("WriteScopedState: %v", err)
+	}
+	out := string(got)
+	if !strings.Contains(out, "updated") {
+		t.Errorf("component write did not apply its own leaf:\n%s", out)
+	}
+	if !strings.Contains(out, "ccc333") {
+		t.Errorf("component write destroyed sibling component 'web':\n%s", out)
+	}
+}
+
+func keysOfState(m map[string]*EnvState) []string {
+	ks := make([]string, 0, len(m))
+	for k := range m {
+		ks = append(ks, k)
+	}
+	return ks
+}