Capture and surface the settings migrate_v016.py does NOT migrate

Ran the converter against a real production settings corpus pulled from the
instance this tool is being built to migrate (secrets scrubbed on that host;
nothing sensitive transited). The result reframes what "a successful
migration" means:

    total settings:     12401
    NOT migrated:       12182  (98.2%)
    actually migrated:  219  (1.8%)

Stalwart's own script reports this in an unmigrated.txt it writes beside its
output - and this tool was throwing that file away. Worse, RunSettingsConvert
never set cmd.Dir, so the script wrote unmigrated.txt into whatever directory
the operator happened to launch from, or failed the whole convert when that
directory wasn't writable. Both reproduced.

The largest groups left behind on production are spam-filter rules, DNSBLs,
trusted-domain and URL-redirector lookups, queue scheduling and TLS
settings - and server.listener. That last one explains something that had
been puzzling from an earlier smoke run: a freshly migrated 0.16 instance
answered on none of the ports the old one did, and served nothing but
/admin. Its listeners never migrated.

So:

- SettingsConvertOptions gains WorkDir, and the convert runs there. The
  report lands somewhere known and an unwritable cwd can't fail the step.
- ReadUnmigratedReport parses it; UnmigratedReport.Summary renders the
  largest groups first.
- The dry run records it as an "unmigrated-settings" artifact with a
  checksum, notes the count in the checkpoint step, and prints it as a
  standing warning rather than a footnote.

ARCHITECTURE 4.3 anticipated a "best-effort apply-plan" gap here. The gap is
not a few stragglers needing review; it is effectively the entire
configuration, and the tool has to say so where nobody can miss it.

Verified against the smoke VM: even a default `stalwart --init` instance
reports 3505 of ~3514 settings unmigrated, listeners included.

cmd/stalwart-migrate/run.go

@@ -196,21 +196,48 @@ func runRun(args []string) (err error) {
 	}
 	fmt.Printf("cloned verified backup into sandbox: %s\n", sandboxDataDir)
 
+	unmigratedPath := filepath.Join(runWorkDir, "unmigrated.txt")
 	if _, err := store.RunStep(rs, checkpoint.PhaseStage, "convert-settings", func() (checkpoint.StepOutcome, error) {
 		if err := backup.RunSettingsConvert(ctx, backup.SettingsConvertOptions{
 			PythonPath: *pythonPath, ScriptPath: scriptDest,
 			SettingsPath: settingsPath, PrincipalsPath: principalsPath,
 			ConfigPath: sandboxConfigPath, OutputPath: sandboxExportPath,
 			PatchPaths: map[string]string{*dataDir: sandboxDataDir},
+			// Without this the script writes unmigrated.txt into whatever
+			// directory this command was launched from - or fails outright
+			// if that isn't writable.
+			WorkDir: runWorkDir,
 		}); err != nil {
 			return checkpoint.StepOutcome{}, err
 		}
-		return checkpoint.StepOutcome{Detail: fmt.Sprintf("generated %s and %s, patched to point at the sandbox", sandboxConfigPath, sandboxExportPath)}, nil
+		detail := fmt.Sprintf("generated %s and %s, patched to point at the sandbox", sandboxConfigPath, sandboxExportPath)
+		if report, err := backup.ReadUnmigratedReport(unmigratedPath); err == nil && report != nil && report.TotalKeys > 0 {
+			detail += fmt.Sprintf("; %d setting(s) were NOT migrated", report.TotalKeys)
+		}
+		return checkpoint.StepOutcome{Detail: detail, Extra: unmigratedPath}, nil
 	}); err != nil {
 		return fmt.Errorf("convert settings: %w", err)
 	}
 	fmt.Println("generated sandbox config.json and export.json")
 
+	// What the converter could NOT carry over matters more than what it
+	// could: against a real instance this is the overwhelming majority of
+	// the configuration, including the listeners, and an operator who
+	// doesn't read it will bring up a server that answers on no ports.
+	unmigrated, err := backup.ReadUnmigratedReport(unmigratedPath)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "warning: couldn't read the unmigrated-settings report: %v\n", err)
+	} else if unmigrated != nil && unmigrated.TotalKeys > 0 {
+		if sum, size, hashErr := backup.HashFile(unmigratedPath); hashErr == nil {
+			rs.RecordArtifact("unmigrated-settings", checkpoint.Artifact{Path: unmigratedPath, SHA256: sum, SizeBytes: size})
+			if saveErr := store.Save(rs); saveErr != nil {
+				fmt.Fprintf(os.Stderr, "warning: couldn't record the unmigrated-settings artifact: %v\n", saveErr)
+			}
+		}
+		fmt.Printf("\n  !! %s\n", unmigrated.Summary(10))
+		fmt.Println("     These do not carry over. Recreate them on the migrated instance before it serves mail.")
+	}
+
 	fmt.Println("\n--- recovery-mode migration (against the sandbox) ---")
 	listenURL := fmt.Sprintf("http://127.0.0.1:%d/", *recoveryPort)
 	recReport, err := recovery.Run(ctx, store, rs, recovery.Options{

internal/backup/backup.go

@@ -91,7 +91,7 @@ func Run(ctx context.Context, store *checkpoint.Store, rs *checkpoint.RunState,
 		if err != nil {
 			return checkpoint.StepOutcome{}, err
 		}
-		sum, size, err := hashFile(preserved)
+		sum, size, err := HashFile(preserved)
 		if err != nil {
 			return checkpoint.StepOutcome{}, fmt.Errorf("backup: hash preserved binary %s: %w", preserved, err)
 		}
@@ -146,7 +146,7 @@ func Run(ctx context.Context, store *checkpoint.Store, rs *checkpoint.RunState,
 			if err := RunPgDump(ctx, opts.SQL); err != nil {
 				return checkpoint.StepOutcome{}, err
 			}
-			sum, size, err := hashFile(opts.SQL.OutPath)
+			sum, size, err := HashFile(opts.SQL.OutPath)
 			if err != nil {
 				return checkpoint.StepOutcome{}, err
 			}
@@ -161,7 +161,7 @@ func Run(ctx context.Context, store *checkpoint.Store, rs *checkpoint.RunState,
 			if err := RunMySQLDump(ctx, opts.SQL); err != nil {
 				return checkpoint.StepOutcome{}, err
 			}
-			sum, size, err := hashFile(opts.SQL.OutPath)
+			sum, size, err := HashFile(opts.SQL.OutPath)
 			if err != nil {
 				return checkpoint.StepOutcome{}, err
 			}
@@ -214,11 +214,11 @@ func Run(ctx context.Context, store *checkpoint.Store, rs *checkpoint.RunState,
 		}); err != nil {
 			return checkpoint.StepOutcome{}, err
 		}
-		settingsSum, settingsSize, err := hashFile(opts.SettingsDumpPath)
+		settingsSum, settingsSize, err := HashFile(opts.SettingsDumpPath)
 		if err != nil {
 			return checkpoint.StepOutcome{}, err
 		}
-		principalsSum, principalsSize, err := hashFile(opts.PrincipalsDumpPath)
+		principalsSum, principalsSize, err := HashFile(opts.PrincipalsDumpPath)
 		if err != nil {
 			return checkpoint.StepOutcome{}, err
 		}
@@ -247,7 +247,7 @@ func Run(ctx context.Context, store *checkpoint.Store, rs *checkpoint.RunState,
 		}
 		var totalSize int64
 		for i, f := range files {
-			sum, size, err := hashFile(f)
+			sum, size, err := HashFile(f)
 			if err != nil {
 				return checkpoint.StepOutcome{}, err
 			}

internal/backup/hash.go

@@ -11,9 +11,9 @@ import (
 	"os"
 )
 
-// hashFile returns a file's SHA256 and size, for recording as a
+// HashFile returns a file's SHA256 and size, for recording as a
 // checkpoint.Artifact.
-func hashFile(path string) (sha256Hex string, size int64, err error) {
+func HashFile(path string) (sha256Hex string, size int64, err error) {
 	f, err := os.Open(path)
 	if err != nil {
 		return "", 0, fmt.Errorf("backup: hash %s: %w", path, err)

internal/backup/settingsdump.go

@@ -12,7 +12,9 @@ import (
 	"net/http"
 	"os"
 	"os/exec"
+	"regexp"
 	"sort"
+	"strconv"
 	"strings"
 	"time"
 )
@@ -135,6 +137,14 @@ type SettingsConvertOptions struct {
 	// config.json's contents directly, which would require depending on its
 	// exact schema.
 	PatchPaths map[string]string
+
+	// WorkDir is where the script runs. It matters more than it looks:
+	// migrate_v016.py writes its unmigrated.txt report into the current
+	// working directory, so without this the convert either fails outright
+	// (an unwritable CWD - which is what happens running as a service from
+	// /) or silently drops the single most important output of the whole
+	// migration wherever the operator happened to be standing.
+	WorkDir string
 }
 
 // RunSettingsConvert runs migrate_v016.py's convert subcommand.
@@ -159,9 +169,97 @@ func RunSettingsConvert(ctx context.Context, o SettingsConvertOptions) error {
 		args = append(args, "--patch-paths", strings.Join(pairs, ","))
 	}
 	cmd := exec.CommandContext(ctx, python, args...)
+	if o.WorkDir != "" {
+		if err := os.MkdirAll(o.WorkDir, 0o750); err != nil {
+			return fmt.Errorf("backup: create convert working directory %s: %w", o.WorkDir, err)
+		}
+		cmd.Dir = o.WorkDir
+	}
 	out, err := cmd.CombinedOutput()
 	if err != nil {
 		return fmt.Errorf("backup: migrate_v016.py convert failed: %w (output: %s)", err, out)
 	}
 	return nil
 }
+
+// UnmigratedPrefix is one group of v0.15 settings the conversion did not
+// carry over, as reported by migrate_v016.py's unmigrated.txt.
+type UnmigratedPrefix struct {
+	Prefix string
+	Keys   int
+}
+
+// UnmigratedReport summarizes what a conversion left behind.
+//
+// This is not a footnote. Against a real production instance - 12,401
+// settings - Stalwart's own converter migrated 219 of them, 1.8%, and left
+// 12,182 for the operator to recreate by hand: spam-filter rules, DNSBLs,
+// trusted-domain and URL-redirector lookups, queue scheduling and TLS
+// settings, and server.listener itself, which is why a freshly migrated
+// instance answers on none of the ports the old one did. A migration that
+// reported success while silently discarding this would be worse than one
+// that failed.
+type UnmigratedReport struct {
+	Path      string
+	TotalKeys int
+	Prefixes  []UnmigratedPrefix
+}
+
+// Summary renders the report for an operator, largest groups first.
+func (r *UnmigratedReport) Summary(maxPrefixes int) string {
+	if r == nil || r.TotalKeys == 0 {
+		return "no unmigrated settings were reported"
+	}
+	var b strings.Builder
+	fmt.Fprintf(&b, "%d v0.15 setting(s) were NOT migrated and must be recreated by hand (full list: %s)", r.TotalKeys, r.Path)
+	prefixes := r.Prefixes
+	if len(prefixes) > maxPrefixes {
+		prefixes = prefixes[:maxPrefixes]
+	}
+	for _, p := range prefixes {
+		fmt.Fprintf(&b, "\n      %-32s %d keys", p.Prefix, p.Keys)
+	}
+	if len(r.Prefixes) > len(prefixes) {
+		fmt.Fprintf(&b, "\n      ... and %d more prefix(es)", len(r.Prefixes)-len(prefixes))
+	}
+	return b.String()
+}
+
+// unmigratedPattern matches the report's per-prefix lines, e.g.
+// "  spam-filter.rule                       424 keys".
+var unmigratedPattern = regexp.MustCompile(`^\s+(\S+)\s+(\d+) keys\s*$`)
+
+// ReadUnmigratedReport parses the unmigrated.txt migrate_v016.py writes
+// beside its output. A missing file is not an error - an older script, or
+// a conversion with nothing left over, simply won't produce one - so
+// callers get a nil report rather than a failure.
+func ReadUnmigratedReport(path string) (*UnmigratedReport, error) {
+	data, err := os.ReadFile(path)
+	if os.IsNotExist(err) {
+		return nil, nil
+	}
+	if err != nil {
+		return nil, fmt.Errorf("backup: read %s: %w", path, err)
+	}
+	report := &UnmigratedReport{Path: path}
+	for _, line := range strings.Split(string(data), "\n") {
+		if m := unmigratedPattern.FindStringSubmatch(line); m != nil {
+			n, convErr := strconv.Atoi(m[2])
+			if convErr != nil {
+				continue
+			}
+			report.Prefixes = append(report.Prefixes, UnmigratedPrefix{Prefix: m[1], Keys: n})
+			continue
+		}
+		if strings.HasPrefix(strings.TrimSpace(line), "Total unmigrated keys:") {
+			fields := strings.Fields(line)
+			if len(fields) >= 4 {
+				if n, convErr := strconv.Atoi(fields[3]); convErr == nil {
+					report.TotalKeys = n
+				}
+			}
+		}
+	}
+	sort.Slice(report.Prefixes, func(i, j int) bool { return report.Prefixes[i].Keys > report.Prefixes[j].Keys })
+	return report, nil
+}

internal/backup/settingsdump_test.go

@@ -176,3 +176,89 @@ func TestRunSettingsConvertPropagatesFailure(t *testing.T) {
 		t.Errorf("error = %v, want it to include the script's stderr", err)
 	}
 }
+
+// The report this parses is the most consequential output of a real
+// migration: against a production instance with 12,401 settings,
+// migrate_v016.py migrated 219 of them and listed the other 12,182 here.
+// Losing or ignoring this file means bringing up a server that answers on
+// no ports, since server.listener is among the settings that don't carry.
+const sampleUnmigrated = `# Unmigrated v0.15 settings
+
+These v0.15 settings were not migrated by the script and must be
+reviewed manually.
+
+Total unmigrated keys: 12182 across 69 prefixes.
+
+  server.blocked-ip                     8547 keys
+  lookup.url-redirectors                1076 keys
+  spam-filter.rule                       424 keys
+  server.listener                         26 keys
+  asn.expires                              1 keys
+`
+
+func TestReadUnmigratedReportParsesTotalsAndPrefixes(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "unmigrated.txt")
+	if err := os.WriteFile(path, []byte(sampleUnmigrated), 0o640); err != nil {
+		t.Fatal(err)
+	}
+
+	report, err := ReadUnmigratedReport(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if report.TotalKeys != 12182 {
+		t.Errorf("TotalKeys = %d, want 12182", report.TotalKeys)
+	}
+	if len(report.Prefixes) != 5 {
+		t.Fatalf("parsed %d prefixes, want 5", len(report.Prefixes))
+	}
+	// Largest first, so the summary leads with what matters most.
+	if report.Prefixes[0].Prefix != "server.blocked-ip" || report.Prefixes[0].Keys != 8547 {
+		t.Errorf("first prefix = %+v, want server.blocked-ip 8547", report.Prefixes[0])
+	}
+
+	summary := report.Summary(3)
+	if !strings.Contains(summary, "12182") || !strings.Contains(summary, "must be recreated by hand") {
+		t.Errorf("summary should lead with the scale of the problem:\n%s", summary)
+	}
+	if !strings.Contains(summary, "and 2 more prefix(es)") {
+		t.Errorf("summary should say how much it elided:\n%s", summary)
+	}
+}
+
+// An older script, or a conversion with nothing left over, writes no file.
+// That is not an error.
+func TestReadUnmigratedReportTreatsMissingFileAsNoReport(t *testing.T) {
+	report, err := ReadUnmigratedReport(filepath.Join(t.TempDir(), "absent.txt"))
+	if err != nil {
+		t.Errorf("missing report should not be an error: %v", err)
+	}
+	if report != nil {
+		t.Errorf("report = %+v, want nil", report)
+	}
+}
+
+// migrate_v016.py writes unmigrated.txt into its working directory, so the
+// convert has to run somewhere writable that the caller knows about.
+func TestRunSettingsConvertRunsInTheGivenWorkDir(t *testing.T) {
+	dir := t.TempDir()
+	workDir := filepath.Join(dir, "work")
+	log := argsFile(t, dir)
+	withFakeExecutable(t, "python3", fakeScriptLoggingArgs(log, "pwd >> "+log+"\ntouch unmigrated.txt"))
+
+	err := RunSettingsConvert(context.Background(), SettingsConvertOptions{
+		ScriptPath: "/tmp/migrate_v016.py", SettingsPath: "/tmp/s.json", PrincipalsPath: "/tmp/p.json",
+		ConfigPath: filepath.Join(dir, "c.json"), OutputPath: filepath.Join(dir, "e.json"),
+		WorkDir: workDir,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(readArgsFile(t, log), workDir) {
+		t.Errorf("script did not run in WorkDir; log:\n%s", readArgsFile(t, log))
+	}
+	if _, err := os.Stat(filepath.Join(workDir, "unmigrated.txt")); err != nil {
+		t.Errorf("unmigrated.txt should land in WorkDir, not the caller's cwd: %v", err)
+	}
+}