Merge pull request #12 from LINUXexpert-org/keep-the-runs-plan

Keep the four files a run cannot produce again

README.md

@@ -374,7 +374,13 @@ if the gap matters to you.
 - **The original service definition is preserved** as `<unit>.pre-<run-id>`
   before cutover rewrites it, so you aren't reconstructing a unit file from
   memory.
-- **The settings and principals dumps** taken during backup stay on disk.
+- **The settings and principals dumps, the apply plan and its supplement**
+  are kept in `<state-dir>/<run-id>` — `/var/lib/stalwart-migrator/runs/<run-id>`
+  unless you moved it. These four are the only files in a run that cannot be
+  produced again afterwards: the dumps need a live pre-migration instance,
+  and the plan is what was actually replayed into your store. They are kept
+  whether or not the run succeeded and whether or not you passed
+  `--keep-artifacts`.
 - **Every artifact path and checksum is in the checkpoint**, and
   `stalwart-migrate status <run-id>` prints exactly which steps completed
   and which failed — which is the first thing you want when deciding what to
@@ -383,6 +389,31 @@ if the gap matters to you.
 None of this is a substitute for the snapshot. It's what makes the twenty
 minutes after restoring one less unpleasant.
 
+### Do not boot recovery mode again afterwards
+
+The migration works by starting the new version once in recovery mode,
+replaying your settings into it, and stopping it. That is a one-time step
+in a migration, and it is worth knowing that it is not a general-purpose
+maintenance mode.
+
+An operator who booted recovery mode again — the same way the migration
+does, `STALWART_RECOVERY_MODE=1` against the same data directory — for
+reasons unrelated to the migration, on a server that had migrated
+successfully days earlier, found that `Domain` and `Account` queries came
+back empty on the next normal start. This happened twice, on two different
+servers. It was not a stale read: creating a domain that had certainly
+existed a moment earlier succeeded, with no `primaryKeyViolation`, so the
+records were genuinely gone. Disk usage did not change.
+
+What recovered it both times was re-applying that run's `export.json` and
+`supplement.json` against a fresh recovery boot, which is why those two
+files are now kept for you. If you need to change something after a
+migration, use the admin API or `stalwart-cli` against the running server.
+
+This is Stalwart's behaviour rather than this tool's, and it is reported
+here because this tool is where you learned the technique. Reported by
+[@kaya-eu](https://github.com/LINUXexpert-org/stalwart-migrator/issues/1).
+
 ### Why it works this way
 
 An earlier version of this tool implemented rollback itself: it restored the

cmd/stalwart-migrate/preserveplan.go

@@ -0,0 +1,71 @@
+// SPDX-FileCopyrightText: 2026 LINUXexpert-org
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package main
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"sort"
+
+	"github.com/LINUXexpert-org/stalwart-migrator/internal/backup"
+	"github.com/LINUXexpert-org/stalwart-migrator/internal/checkpoint"
+)
+
+// preservePlan lifts the run's irreplaceable inputs out of the scratch
+// directory into the run's state directory, which is never cleaned up, and
+// records each as a checkpoint artifact.
+//
+// These four are not intermediate files. The settings and principals dumps
+// can only be taken from a live pre-migration instance, and the apply plan
+// and its supplement are what was actually replayed into the store - so
+// after cutover there is no way to produce any of them again. Until this
+// existed they lived only in --work-dir, which a successful run deletes,
+// and `rehearse` kept more of its conclusions than `run` did: the
+// read-only command preserved the plan and the destructive one threw it
+// away.
+//
+// What made that concrete: an operator who booted recovery mode again
+// after a completed migration, for an unrelated reason, and found Domain
+// and Account queries coming back empty. Re-applying the original run's
+// export.json and supplement.json against a fresh recovery boot is what
+// got their server back, twice, on two different machines - and they had
+// them only because they had passed --keep-artifacts. Nobody should need
+// to have guessed that in advance. Reported by @kaya-eu in #1.
+//
+// A file that isn't there is skipped rather than failing the step: a patch
+// bump converts nothing, and a supplement that couldn't be generated is
+// already a warning of its own.
+func preservePlan(stateDir string, files map[string]string, rs *checkpoint.RunState) ([]string, error) {
+	if stateDir == "" {
+		return nil, fmt.Errorf("no state directory to preserve the run's plan in")
+	}
+	names := make([]string, 0, len(files))
+	for name := range files {
+		names = append(names, name)
+	}
+	sort.Strings(names)
+
+	var kept []string
+	for _, name := range names {
+		src := files[name]
+		if _, err := os.Stat(src); err != nil {
+			continue
+		}
+		dst := filepath.Join(stateDir, filepath.Base(src))
+		if err := copyFile(src, dst); err != nil {
+			return nil, fmt.Errorf("preserve %s as %s: %w", src, dst, err)
+		}
+		sum, size, err := backup.HashFile(dst)
+		if err != nil {
+			return nil, fmt.Errorf("hash %s: %w", dst, err)
+		}
+		rs.RecordArtifact(name, checkpoint.Artifact{Path: dst, SHA256: sum, SizeBytes: size})
+		kept = append(kept, filepath.Base(dst))
+	}
+	if len(kept) == 0 {
+		return nil, fmt.Errorf("none of the run's plan files exist to preserve - the conversion produced nothing")
+	}
+	return kept, nil
+}

cmd/stalwart-migrate/preserveplan_test.go

@@ -0,0 +1,110 @@
+// SPDX-FileCopyrightText: 2026 LINUXexpert-org
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package main
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/LINUXexpert-org/stalwart-migrator/internal/checkpoint"
+)
+
+func writeTemp(t *testing.T, dir, name, body string) string {
+	t.Helper()
+	p := filepath.Join(dir, name)
+	if err := os.WriteFile(p, []byte(body), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	return p
+}
+
+func newRunState(t *testing.T) *checkpoint.RunState {
+	t.Helper()
+	rs, err := checkpoint.NewStore(t.TempDir()).Create("0.15.5", "0.16.19")
+	if err != nil {
+		t.Fatal(err)
+	}
+	return rs
+}
+
+// The dumps can only be taken from a live pre-migration instance and the
+// apply plan is what was replayed into the store, so after cutover none of
+// them can be produced again. They used to live only in the work dir,
+// which a successful run deletes.
+func TestPreservePlanKeepsTheRunsInputsOutOfTheScratchDirectory(t *testing.T) {
+	work, state := t.TempDir(), t.TempDir()
+	rs := newRunState(t)
+
+	kept, err := preservePlan(state, map[string]string{
+		"settings-dump":    writeTemp(t, work, "settings.json", `{"settings":1}`),
+		"principals-dump":  writeTemp(t, work, "principals.json", `{"principals":1}`),
+		"converted-export": writeTemp(t, work, "export.json", `[{"@type":"create"}]`),
+		"supplement":       writeTemp(t, work, "supplement.json", `[]`),
+	}, rs)
+	if err != nil {
+		t.Fatalf("preservePlan: %v", err)
+	}
+	if len(kept) != 4 {
+		t.Errorf("kept %v, want all four", kept)
+	}
+
+	// The work dir is what gets deleted; what matters is that the state
+	// dir now stands on its own.
+	if err := os.RemoveAll(work); err != nil {
+		t.Fatal(err)
+	}
+	for _, name := range []string{"settings-dump", "principals-dump", "converted-export", "supplement"} {
+		art, ok := rs.Artifacts[name]
+		if !ok {
+			t.Errorf("%s was not recorded as an artifact", name)
+			continue
+		}
+		if !strings.HasPrefix(art.Path, state) {
+			t.Errorf("%s kept at %s, want it inside the state dir", name, art.Path)
+		}
+		if _, err := os.Stat(art.Path); err != nil {
+			t.Errorf("%s does not survive the work dir being cleaned: %v", name, err)
+		}
+		if art.SHA256 == "" || art.SizeBytes == 0 {
+			t.Errorf("%s recorded without a checksum or size: %+v", name, art)
+		}
+	}
+}
+
+// A patch bump converts nothing and a supplement that could not be
+// generated is already a warning of its own, so an absent file is skipped
+// rather than failing the step.
+func TestPreservePlanSkipsWhatIsNotThere(t *testing.T) {
+	work, state := t.TempDir(), t.TempDir()
+	rs := newRunState(t)
+
+	kept, err := preservePlan(state, map[string]string{
+		"converted-export": writeTemp(t, work, "export.json", `[]`),
+		"supplement":       filepath.Join(work, "never-written.json"),
+	}, rs)
+	if err != nil {
+		t.Fatalf("preservePlan: %v", err)
+	}
+	if len(kept) != 1 || kept[0] != "export.json" {
+		t.Errorf("kept %v, want just export.json", kept)
+	}
+	if _, ok := rs.Artifacts["supplement"]; ok {
+		t.Error("a file that was never written should not be recorded as an artifact")
+	}
+}
+
+// Nothing at all to preserve means the conversion produced nothing, which
+// is not a state to carry quietly into a migration.
+func TestPreservePlanRefusesWhenThereIsNothingToKeep(t *testing.T) {
+	work, state := t.TempDir(), t.TempDir()
+	rs := newRunState(t)
+
+	if _, err := preservePlan(state, map[string]string{
+		"converted-export": filepath.Join(work, "absent.json"),
+	}, rs); err == nil {
+		t.Fatal("want an error when none of the plan files exist")
+	}
+}

cmd/stalwart-migrate/run.go

@@ -11,6 +11,7 @@ import (
 	"os"
 	"path"
 	"path/filepath"
+	"strings"
 	"time"
 
 	"github.com/LINUXexpert-org/stalwart-migrator/internal/applyplan"
@@ -74,7 +75,9 @@ func runRun(args []string) (err error) {
 	binarySHA := fs.String("target-binary-sha256", "", "pinned sha256 of the target release archive")
 	minFree := fs.Float64("min-free-multiple", 2.0, "required free disk space as a multiple of the data directory size")
 	recalcQuotas := fs.Bool("recalculate-quotas", true, "schedule the post-migration quota rebuild")
-	keepArtifacts := fs.Bool("keep-artifacts", false, "don't delete work-dir/<run-id> afterward")
+	keepArtifacts := fs.Bool("keep-artifacts", false, "don't delete work-dir/<run-id> afterward. The run's own inputs - the "+
+		"settings and principals dumps, the apply plan and its supplement - are kept in state-dir/<run-id> either way; this "+
+		"additionally keeps the scratch files around them")
 	resume := fs.String("resume", "", "resume an interrupted run by id instead of starting a new one (see `status` for ids)")
 	yes := fs.Bool("yes", false, "actually perform the migration")
 	containerUnproven := fs.Bool("container-path-unproven", false,
@@ -158,7 +161,12 @@ func runRun(args []string) (err error) {
 		}
 		if rmErr := os.RemoveAll(runWorkDir); rmErr != nil {
 			fmt.Fprintf(os.Stderr, "warning: couldn't clean up %s: %v\n", runWorkDir, rmErr)
+			return
 		}
+		// What is deleted here is scratch. The run's inputs were copied to
+		// the state directory before the store was touched, because they
+		// cannot be produced again once it has been.
+		fmt.Printf("\ncleaned up %s; the run's dumps and apply plan are kept in %s\n", runWorkDir, runStateDir)
 	}()
 
 	fmt.Println("\n--- preflight ---")
@@ -416,6 +424,22 @@ func runRun(args []string) (err error) {
 			applyFiles = append(applyFiles, supplementPath)
 		}
 
+		if _, err := store.RunStep(rs, checkpoint.PhaseBackup, "preserve-plan", func() (checkpoint.StepOutcome, error) {
+			kept, err := preservePlan(runStateDir, map[string]string{
+				"settings-dump":    settingsPath,
+				"principals-dump":  principalsPath,
+				"converted-export": convertedExport,
+				"supplement":       supplementPath,
+			}, rs)
+			if err != nil {
+				return checkpoint.StepOutcome{}, err
+			}
+			return checkpoint.StepOutcome{Detail: fmt.Sprintf("kept %s in %s", strings.Join(kept, ", "), runStateDir)}, nil
+		}); err != nil {
+			return fmt.Errorf("preserve the run's plan: %w", err)
+		}
+		fmt.Println(rs.Outcome(checkpoint.PhaseBackup, "preserve-plan").Detail)
+
 		fmt.Println("\n--- recovery-mode migration (the store is migrated IN PLACE) ---")
 		recOpts := recovery.Options{
 			BinaryPath: staged, ConfigPath: convertedConfig,