Commit guest metadata with configuration recovery

Stage guest metadata with the archive's settings and credentials, publish the live cache only after a successful commit, and serialize it with pending identity snapshots. Restore newly created targets and an original moved aside before a failed replacement. Preserve normalization, scope, archive compatibility and ordinary metadata-writer ownership.

Contract-Neutral: Repairs the existing all-or-nothing configuration recovery behaviour; archive formats, API response shape, tenant authority and normal metadata mutation contracts are unchanged.
Change-source: pulse-maintainer

internal/config/export.go

@@ -258,16 +258,11 @@ func (c *ConfigPersistence) ImportConfig(encryptedData string, passphrase string
 		}
 	}
 
-	if err := tx.Commit(); err != nil {
+	if err := c.GetGuestMetadataStore().commitImport(tx, exportData.GuestMetadata); err != nil {
 		return fmt.Errorf("failed to commit import transaction: %w", err)
 	}
 	committed = true
 
-	// Import guest metadata into the active persistence scope.
-	if err := c.GetGuestMetadataStore().ReplaceAll(exportData.GuestMetadata); err != nil {
-		log.Warn().Err(err).Msg("Failed to import guest metadata")
-	}
-
 	return nil
 }
 

internal/config/guest_metadata_import.go

@@ -0,0 +1,52 @@
+package config
+
+import (
+	"encoding/json"
+	"fmt"
+	"path/filepath"
+)
+
+// commitImport includes guest metadata in the same file transaction as settings
+// and credentials. Keep the store's writer and mutation locks through commit so
+// an older background snapshot cannot overwrite the imported file, and readers
+// cannot observe imported metadata before its configuration has committed.
+func (s *GuestMetadataStore) commitImport(tx *importTransaction, metadata map[string]*GuestMetadata) error {
+	s.writeMu.Lock()
+	defer s.writeMu.Unlock()
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	if s.closed.Load() {
+		return ErrGuestMetadataStoreClosed
+	}
+
+	working := make(map[string]*GuestMetadata, len(metadata))
+	for id, meta := range metadata {
+		if meta == nil {
+			continue
+		}
+		clone := cloneGuestMetadata(meta)
+		clone.ID = id
+		if clone.Tags == nil {
+			clone.Tags = []string{}
+		}
+		working[id] = clone
+	}
+	data, err := json.Marshal(working)
+	if err != nil {
+		return fmt.Errorf("serialize imported guest metadata: %w", err)
+	}
+	if err := tx.StageFile(filepath.Join(s.dataPath, "guest_metadata.json"), data, 0600); err != nil {
+		return fmt.Errorf("stage imported guest metadata: %w", err)
+	}
+	if err := tx.Commit(); err != nil {
+		return err
+	}
+
+	// Publish only after every file committed. A pending writer will now see
+	// that this revision is durable instead of replaying a pre-import snapshot.
+	s.metadata = working
+	s.revision++
+	s.attemptedRevision = s.revision
+	s.lastWriteErr = nil
+	return nil
+}

internal/config/guest_metadata_import_test.go

@@ -0,0 +1,134 @@
+package config
+
+import (
+	"encoding/json"
+	"os"
+	"path/filepath"
+	"reflect"
+	"sync"
+	"testing"
+	"time"
+)
+
+type importBlockedMetadataFS struct {
+	FileSystem
+	once    sync.Once
+	entered chan struct{}
+	release chan struct{}
+}
+
+func (f *importBlockedMetadataFS) WriteFile(path string, data []byte, perm os.FileMode) error {
+	if filepath.Base(path) == "guest_metadata.json.tmp" {
+		f.once.Do(func() { close(f.entered); <-f.release })
+	}
+	return f.FileSystem.WriteFile(path, data, perm)
+}
+
+func TestGuestMetadataImportOrdersPendingSnapshotsAndCopiesInput(t *testing.T) {
+	dir := t.TempDir()
+	fs := &importBlockedMetadataFS{FileSystem: defaultFileSystem{}, entered: make(chan struct{}), release: make(chan struct{})}
+	store := NewGuestMetadataStore(dir, fs)
+	var releaseOnce sync.Once
+	release := func() { releaseOnce.Do(func() { close(fs.release) }) }
+	t.Cleanup(func() {
+		release()
+		if err := store.Close(time.Second); err != nil {
+			t.Error(err)
+		}
+	})
+	if err := store.SetAsync("old:101", &GuestMetadata{Description: "Old snapshot"}); err != nil {
+		t.Fatal(err)
+	}
+	select {
+	case <-fs.entered:
+	case <-time.After(time.Second):
+		t.Fatal("pending metadata writer did not enter")
+	}
+	if err := store.RememberIdentity("old:101", "Old guest", "qemu"); err != nil {
+		t.Fatal(err)
+	}
+	tx, err := newImportTransaction(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer tx.Cleanup()
+	settings := filepath.Join(dir, "system.json")
+	if err := os.WriteFile(settings, []byte("old settings"), 0600); err != nil {
+		t.Fatal(err)
+	}
+	if err := tx.StageFile(settings, []byte("imported settings"), 0600); err != nil {
+		t.Fatal(err)
+	}
+	metadata := map[string]*GuestMetadata{"new:202": {ID: "wrong", Description: "Imported snapshot", Tags: []string{"imported"}, Notes: []string{"note"}}}
+	done := make(chan error, 1)
+	go func() { done <- store.commitImport(tx, metadata) }()
+	select {
+	case err := <-done:
+		t.Fatalf("import overtook an in-flight metadata snapshot: %v", err)
+	case <-time.After(50 * time.Millisecond):
+		// A bounded negative observation, followed by the positive joined proof.
+	}
+	if data, err := os.ReadFile(settings); err != nil || string(data) != "old settings" {
+		t.Fatal("configuration committed while the old metadata write was still in flight")
+	}
+	release()
+	select {
+	case err := <-done:
+		if err != nil {
+			t.Fatal(err)
+		}
+	case <-time.After(5 * time.Second):
+		t.Fatal("import did not complete after the prior snapshot")
+	}
+	if !store.WaitForPendingWrites(time.Second) {
+		t.Fatal("pending writer did not drain")
+	}
+	expected := map[string]*GuestMetadata{"new:202": {ID: "new:202", Description: "Imported snapshot", Tags: []string{"imported"}, Notes: []string{"note"}}}
+	metadata["new:202"].Tags[0] = "caller mutation"
+	metadata["new:202"].Notes[0] = "caller mutation"
+	if !reflect.DeepEqual(store.GetAll(), expected) {
+		t.Fatal("import retained old records or caller-owned slices")
+	}
+	data, err := os.ReadFile(filepath.Join(dir, "guest_metadata.json"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	var persisted map[string]*GuestMetadata
+	if err := json.Unmarshal(data, &persisted); err != nil {
+		t.Fatal(err)
+	}
+	if !reflect.DeepEqual(persisted, expected) {
+		t.Fatal("pending snapshot overwrote the imported metadata file")
+	}
+	if data, err := os.ReadFile(settings); err != nil || string(data) != "imported settings" {
+		t.Fatal("metadata import did not commit its settings")
+	}
+	if err := store.Load(); err != nil {
+		t.Fatalf("committed revision was left pending: %v", err)
+	}
+}
+
+func TestGuestMetadataImportStageFailurePreservesMemory(t *testing.T) {
+	cp := seedImportValidationState(t)
+	store := cp.GetGuestMetadataStore()
+	before := store.GetAll()
+	tx, err := newImportTransaction(cp.configDir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	// An occupied staging pathname is an actual filesystem failure before
+	// commit, not a simulated successful persistence result.
+	if err := os.Remove(tx.stagingDir); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(tx.stagingDir, []byte("block staging"), 0600); err != nil {
+		t.Fatal(err)
+	}
+	defer tx.Cleanup()
+	if err := store.commitImport(tx, map[string]*GuestMetadata{"new:202": {Description: "Do not publish"}}); err == nil {
+		t.Fatal("metadata staging failure was accepted")
+	}
+	if !reflect.DeepEqual(before, store.GetAll()) || tx.committed {
+		t.Fatal("staging failure changed live metadata or committed configuration")
+	}
+}

internal/config/import_transaction.go

@@ -170,6 +170,12 @@ func (tx *importTransaction) Commit() error {
 					continue
 				}
 				tx.backups[target] = ""
+			} else {
+				// This target did not exist before the import. Restoring its
+				// absence matters just as much as restoring an overwritten file.
+				if err := os.Remove(target); err != nil && !os.IsNotExist(err) {
+					joinRestoreErr(fmt.Errorf("remove new target %s during restore: %w", target, err))
+				}
 			}
 		}
 		return restoreErr
@@ -203,11 +209,12 @@ func (tx *importTransaction) Commit() error {
 			return failWithRestore(fmt.Errorf("stat destination %s: %w", target, err))
 		}
 
+		// Track the target before replacement: a failed rename after moving
+		// the original into a backup must restore that original too.
+		applied = append(applied, target)
 		if err := os.Rename(stagedPath, target); err != nil {
 			return failWithRestore(fmt.Errorf("apply staged file to %s: %w", target, err))
 		}
-
-		applied = append(applied, target)
 	}
 
 	// Successful commit: remove backups. Keep success behavior, but surface cleanup failures in logs.