fix(statewrite): route orchestrate and external state writes through WriteManifestState

internal/external/command.go

@@ -10,7 +10,6 @@ import (
 	"time"
 
 	"github.com/spf13/cobra"
-	"gopkg.in/yaml.v3"
 
 	"github.com/stablekernel/cascade/internal/config"
 	"github.com/stablekernel/cascade/internal/git"
@@ -280,24 +279,20 @@ func commitWithApplicationRetry(filePath, commitMsg string, maxAttempts int, app
 	return fmt.Errorf("git push failed after %d attempts: %s: %w", maxAttempts, strings.TrimSpace(string(lastPushOut)), lastPushErr)
 }
 
-// writeManifest writes the CICD file back to the manifest under the specified key.
+// writeManifest writes the updated external deploy state back to the manifest
+// under the specified key. It rewrites only the mutable state subtree, so every
+// other manifest key, including config fields this binary does not model (for
+// example cli_version_sha on a SHA-pinned repo running an older cascade) and the
+// internal runtime fields, is preserved verbatim rather than dropped on the
+// round-trip. The external update mutates only state, so a full re-marshal of
+// the typed config is never needed here.
 func writeManifest(path, key string, file *config.CICDFile) error {
-	// Read existing manifest to preserve other keys
-	data, err := os.ReadFile(path)
+	current, err := os.ReadFile(path)
 	if err != nil {
 		return fmt.Errorf("reading manifest: %w", err)
 	}
 
-	var manifest map[string]any
-	if err := yaml.Unmarshal(data, &manifest); err != nil {
-		return fmt.Errorf("parsing manifest: %w", err)
-	}
-
-	// Update the CI key
-	manifest[key] = file
-
-	// Write back
-	out, err := yaml.Marshal(manifest)
+	out, err := config.WriteManifestState(current, key, file.State, file.LatestRelease)
 	if err != nil {
 		return fmt.Errorf("marshaling manifest: %w", err)
 	}

internal/external/command_test.go

@@ -102,6 +102,53 @@ other_key: preserved
 	assert.Contains(t, content, "preserved")
 }
 
+// TestWriteManifest_PreservesFullConfig guards the external state write against
+// dropping manifest configuration on the round-trip (issue #372). The write must
+// touch only the mutable state subtree: modeled config fields (cli_version_sha),
+// config keys this binary does not model, and the internal runtime fields
+// (manifest_file/manifest_key) must all be left untouched in the committed file.
+func TestWriteManifest_PreservesFullConfig(t *testing.T) {
+	tmpDir := t.TempDir()
+	manifestPath := filepath.Join(tmpDir, "manifest.yaml")
+
+	const sha = "5702d41a1234567890abcdef1234567890abcdef"
+	initial := `ci:
+  config:
+    trunk_branch: main
+    environments: [dev]
+    cli_version: v0.6.0
+    pin_mode: sha
+    cli_version_sha: ` + sha + `
+    future_field: keep-me
+  state:
+    dev:
+      sha: abc123
+`
+	require.NoError(t, os.WriteFile(manifestPath, []byte(initial), 0644))
+
+	// Parse the on-disk manifest, mutate only state, and write it back through
+	// the same path the external update command uses.
+	cicdFile, err := config.ParseManifestFile(manifestPath, "ci")
+	require.NoError(t, err)
+	cicdFile.State["dev"].External = map[string]*config.ExternalDeployState{
+		"cdk": {Repo: "org/cdk-infra", SHA: "cdk123"},
+	}
+
+	require.NoError(t, writeManifest(manifestPath, "ci", cicdFile))
+
+	data, err := os.ReadFile(manifestPath)
+	require.NoError(t, err)
+	content := string(data)
+
+	assert.Contains(t, content, "cdk123", "state not updated")
+	assert.Contains(t, content, "cli_version_sha: "+sha, "cli_version_sha dropped")
+	assert.Contains(t, content, "cli_version: v0.6.0", "cli_version dropped")
+	assert.Contains(t, content, "pin_mode: sha", "pin_mode dropped")
+	assert.Contains(t, content, "future_field: keep-me", "unmodeled config field dropped")
+	assert.NotContains(t, content, "manifest_file:", "internal runtime field leaked")
+	assert.NotContains(t, content, "manifest_key:", "internal runtime field leaked")
+}
+
 func TestUpdateCommand_RequiredFlags(t *testing.T) {
 	cmd := NewCommand()
 

internal/orchestrate/orchestrator.go

@@ -8,8 +8,6 @@ import (
 	"strings"
 	"time"
 
-	"gopkg.in/yaml.v3"
-
 	"github.com/stablekernel/cascade/internal/changelog"
 	"github.com/stablekernel/cascade/internal/config"
 	"github.com/stablekernel/cascade/internal/git"
@@ -462,9 +460,24 @@ func (o *Orchestrator) calculateChangelogRefs() (string, string) {
 	return initialCommit, ""
 }
 
-// writeConfig writes the updated cicd.yaml file.
+// writeConfig writes the updated manifest back to disk. It rewrites only the
+// mutable state subtree of the on-disk manifest, so the top-level manifest key
+// wrapper and every config key, including ones this binary does not model (for
+// example cli_version_sha on a SHA-pinned repo running an older cascade), are
+// preserved verbatim rather than dropped on the round-trip. Finalize mutates
+// only state, so a full re-marshal of the typed config is never needed here.
 func (o *Orchestrator) writeConfig() error {
-	data, err := yaml.Marshal(o.cicdFile)
+	key := config.DefaultManifestKey
+	if o.cicdFile.Config != nil && o.cicdFile.Config.ManifestKey != "" {
+		key = o.cicdFile.Config.ManifestKey
+	}
+
+	current, err := os.ReadFile(o.configPath)
+	if err != nil {
+		return fmt.Errorf("failed to read config: %w", err)
+	}
+
+	data, err := config.WriteManifestState(current, key, o.cicdFile.State, o.cicdFile.LatestRelease)
 	if err != nil {
 		return fmt.Errorf("failed to marshal config: %w", err)
 	}

internal/orchestrate/state_preserve_test.go

@@ -0,0 +1,93 @@
+package orchestrate
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// TestWriteConfig_PreservesFullManifest guards against the state-write that
+// silently dropped manifest configuration on the round-trip (issue #372).
+//
+// The finalize state write must touch only the mutable state subtree and leave
+// every other manifest key byte-for-byte intact: the top-level manifest key
+// wrapper, modeled config fields (cli_version_sha, which a SHA-pin repo relies
+// on), config keys this binary does not model (a newer cascade release may add
+// fields an older pinned binary cannot round-trip), and the internal runtime
+// fields (manifest_file/manifest_key) must never leak into the committed file.
+func TestWriteConfig_PreservesFullManifest(t *testing.T) {
+	dir := t.TempDir()
+	mfDir := filepath.Join(dir, ".github")
+	if err := os.MkdirAll(mfDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	path := filepath.Join(mfDir, "manifest.yaml")
+
+	const sha = "5702d41a1234567890abcdef1234567890abcdef"
+	orig := `ci:
+  config:
+    trunk_branch: main
+    environments:
+      - dev
+    cli_version: v0.6.0
+    pin_mode: sha
+    cli_version_sha: ` + sha + `
+    future_field: keep-me
+    deploys:
+      - name: app
+        run: echo deploy
+  state:
+    dev:
+      sha: oldsha
+`
+	if err := os.WriteFile(path, []byte(orig), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	o, err := NewOrchestrator(path, "ci", "dev")
+	if err != nil {
+		t.Fatalf("NewOrchestrator: %v", err)
+	}
+
+	// Mutate state exactly as Finalize does, then persist.
+	o.cicdFile.State["dev"].SHA = "newsha"
+	o.cicdFile.State["dev"].Version = "v0.6.0-rc.1"
+	if err := o.writeConfig(); err != nil {
+		t.Fatalf("writeConfig: %v", err)
+	}
+
+	out, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	got := string(out)
+
+	// State must be updated.
+	if !strings.Contains(got, "newsha") {
+		t.Errorf("state not updated; missing newsha:\n%s", got)
+	}
+	// Top-level manifest key wrapper must survive.
+	if !strings.Contains(got, "ci:") {
+		t.Errorf("top-level 'ci:' wrapper dropped:\n%s", got)
+	}
+	// Modeled SHA-pin field must survive (the drift that started this).
+	if !strings.Contains(got, "cli_version_sha: "+sha) {
+		t.Errorf("cli_version_sha dropped:\n%s", got)
+	}
+	// Sibling config the round-trip should never touch.
+	if !strings.Contains(got, "cli_version: v0.6.0") {
+		t.Errorf("cli_version dropped:\n%s", got)
+	}
+	if !strings.Contains(got, "pin_mode: sha") {
+		t.Errorf("pin_mode dropped:\n%s", got)
+	}
+	// Config keys this binary does not model must survive (older-binary case).
+	if !strings.Contains(got, "future_field: keep-me") {
+		t.Errorf("unmodeled config field dropped:\n%s", got)
+	}
+	// Internal runtime fields must never leak into the committed manifest.
+	if strings.Contains(got, "manifest_file:") || strings.Contains(got, "manifest_key:") {
+		t.Errorf("internal runtime field leaked into manifest:\n%s", got)
+	}
+}