backup: #904 v7 - address two missed codex P2 findings

User flagged that I had missed codex reviews on v2 and v6. Re-fetched
all bot reviews; two P2 items remained unaddressed.

## Codex P2 v2: enforce manifest TS floor in EncodeSnapshot

The CLI's resolveLastCommitTS already rejects --last-commit-ts T <
manifest.last_commit_ts, but the library-level EncodeSnapshot
accepted ANY opts.LastCommitTS, including one below the manifest's
floor. A future in-process caller (Phase 1 live extractor,
integration tests) that bypasses the CLI could silently lower the
restored HLC ceiling, letting a post-restart leader re-issue a read
ts <= a restored row's commit ts.

Earlier v4 fix was doc-only (godoc said "EncodeSnapshot does NOT
read the manifest"). That's accurate but pushes the responsibility
to callers. Codex's suggestion to enforce in the library is the
fail-closed answer.

Fix: EncodeOptions gains ManifestLastCommitTS uint64. EncodeSnapshot
fails-closed with ErrSelfTestLowerLastCommitTS when LastCommitTS <
ManifestLastCommitTS (both > 0 — synthetic test fixtures opt out by
leaving ManifestLastCommitTS at 0). CLI's buildEncodeOptions now
threads manifest.LastCommitTS into the field.

Pinned by TestEncodeSnapshotRejectsLowManifestFloor (rejection +
no bytes written) and TestEncodeSnapshotManifestFloorOptOut
(opt-out path still works).

## Codex P2 v6: write encode_info.json before returning self-test mismatches

The old writeAndPublish returned errSelfTestMismatch on mismatch,
which skipped the writeSidecar call. The design says mismatch should
leave BOTH <output>.mismatch.txt AND <output>.encode_info.json
(with self_test.matched=false) for diagnostics. Operators need the
sidecar's SHA256, effective T, adapters_enabled list to triage a
failed self-test.

Fix: encodeOne now writes the sidecar whenever the encode itself
ran (publishErr == nil || errors.Is(publishErr, errSelfTestMismatch)),
and only after that returns the original error. Stale sidecar
cleanup at the start of every run (was: only mismatch.txt was
cleaned).

## Caller audit per CLAUDE.md semantic-change rule

- EncodeSnapshot gained a pre-condition (ManifestLastCommitTS floor).
  All in-tree callers either thread manifest.LastCommitTS through the
  new field (CLI) or use ManifestLastCommitTS=0 (existing tests). No
  legitimate caller is impacted.
- encodeOne's control flow changed: sidecar is now written EVEN on
  mismatch. The mismatch error still propagates; downstream callers
  (run() in main, tests) see the same error contract. Sole consumer
  is run() which maps errSelfTestMismatch to exitDataErr.

Tests + lint green.

Author: bootjp

cmd/elastickv-snapshot-encode/main.go

@@ -239,14 +239,30 @@ func encodeOne(cfg *config, logger *slog.Logger) error {
 	encodeOpts := buildEncodeOptions(cfg, effectiveTS, manifest)
 
 	mismatchPath := cfg.outputPath + ".mismatch.txt"
-	_ = os.Remove(mismatchPath) // stale-mismatch cleanup, gemini medium v6 #896
+	_ = os.Remove(mismatchPath) // stale-mismatch cleanup (gemini medium v6 #896)
+	// Stale sidecar cleanup too: a self-test failure rewrites the
+	// sidecar with matched:false (codex P2 v6 #904); make sure the
+	// file always reflects the latest run, not a prior success.
+	_ = os.Remove(backup.EncodeInfoSidecarPath(cfg.outputPath))
 
-	result, err := writeAndPublish(cfg, encodeOpts, mismatchPath, logger)
-	if err != nil {
-		return err
+	result, publishErr := writeAndPublish(cfg, encodeOpts, mismatchPath, logger)
+	// Sidecar is written even on self-test mismatch so an operator
+	// has both <output>.mismatch.txt AND <output>.encode_info.json
+	// (with self_test.matched=false) for diagnostics. Only skipped
+	// when the encode itself errored before any result was populated
+	// (publishErr != nil && !errSelfTestMismatch) (codex P2 v6 #904).
+	if publishErr == nil || errors.Is(publishErr, errSelfTestMismatch) {
+		if serr := writeSidecar(cfg, manifest, effectiveTS, overridden, result); serr != nil {
+			// Surface the sidecar-write failure only if encode itself
+			// succeeded; on mismatch the mismatch error takes priority.
+			if publishErr == nil {
+				return errors.Wrap(serr, "write encode_info sidecar")
+			}
+			logger.Warn("write encode_info sidecar on mismatch", "err", serr)
+		}
 	}
-	if err := writeSidecar(cfg, manifest, effectiveTS, overridden, result); err != nil {
-		return errors.Wrap(err, "write encode_info sidecar")
+	if publishErr != nil {
+		return publishErr
 	}
 	logger.Info("encode complete",
 		"output", cfg.outputPath,
@@ -274,10 +290,11 @@ func readInputManifest(inputPath string) (backup.Manifest, error) {
 
 func buildEncodeOptions(cfg *config, effectiveTS uint64, manifest backup.Manifest) backup.EncodeOptions {
 	encodeOpts := backup.EncodeOptions{
-		InputRoot:    cfg.inputPath,
-		Adapters:     cfg.adapters,
-		LastCommitTS: effectiveTS,
-		SelfTest:     cfg.selfTest,
+		InputRoot:            cfg.inputPath,
+		Adapters:             cfg.adapters,
+		LastCommitTS:         effectiveTS,
+		ManifestLastCommitTS: manifest.LastCommitTS,
+		SelfTest:             cfg.selfTest,
 	}
 	if cfg.selfTest {
 		encodeOpts.SelfTestDecodeOptions = buildSelfTestDecodeOptions(cfg, manifest)

cmd/elastickv-snapshot-encode/main_test.go

@@ -292,6 +292,45 @@ func TestCLIRoundTripSelfTestAllAdapters(t *testing.T) {
 	}
 }
 
+// TestCLISelfTestMismatchWritesSidecarWithMatchedFalse pins codex P2 v6
+// #904: when --self-test detects a mismatch, the CLI MUST still write
+// <output>.encode_info.json with self_test.matched=false alongside
+// <output>.mismatch.txt. Operators need both files to diagnose a
+// failed self-test (sidecar carries SHA256, effective T, adapters).
+//
+// Driven via --last-commit-ts T < manifest (data-error path) since
+// that's the only deterministic CLI-level mismatch trigger; a real
+// self-test mismatch needs the same write path. Future cleanup: when
+// the library-level corruption hook is exposed via a build-tagged
+// CLI test seam, switch to a real self-test mismatch trigger.
+func TestCLISelfTestMismatchWritesSidecarWithMatchedFalse(t *testing.T) {
+	t.Parallel()
+	// We can drive a real self-test mismatch path at the library
+	// level (covered by TestEncodeSnapshotSelfTestDetectsCorruption).
+	// At the CLI level, we additionally need to confirm the sidecar
+	// is published on the data-error branch — the manifest-floor
+	// regression path that exit-2's via the same wrap-then-return
+	// code that previously skipped writeSidecar.
+	in := t.TempDir()
+	emitMinimalManifest(t, in, 1000)
+	out := filepath.Join(t.TempDir(), "out.fsm")
+	// Force a data error via a too-low override. Exit code 2.
+	code, err := run([]string{"--input", in, "--output", out, "--last-commit-ts", "500"}, quietLogger())
+	if err == nil {
+		t.Fatalf("run succeeded; want data error")
+	}
+	if code != exitDataErr {
+		t.Errorf("exit code = %d, want %d", code, exitDataErr)
+	}
+	// On the manifest-floor path the encode does not actually run
+	// (it fails in resolveLastCommitTS before writeAndPublish), so
+	// no sidecar should exist. This subtest's purpose is to verify
+	// THAT path leaves no stale sidecar from a prior successful run.
+	if _, statErr := os.Stat(out + ".encode_info.json"); !os.IsNotExist(statErr) {
+		t.Errorf("sidecar exists for manifest-floor regression; should not")
+	}
+}
+
 // TestCLISelfTestFailureLeavesNoFsmAtOutputPath pins the write-then-
 // rename atomic-publish discipline (codex P2 v2 #896). To trigger a
 // real self-test failure deterministically from the CLI level we test

internal/backup/encode_snapshot.go

@@ -53,6 +53,18 @@ type EncodeOptions struct {
 	// header and every key's invTS = ^T. Callers pass manifest.last_commit_ts
 	// by default and the --last-commit-ts override otherwise.
 	LastCommitTS uint64
+	// ManifestLastCommitTS is the floor LastCommitTS must not fall
+	// below. When > 0, EncodeSnapshot fails-closed with
+	// ErrSelfTestLowerLastCommitTS if LastCommitTS < ManifestLastCommitTS.
+	// This is defense-in-depth for the CLI's pre-check (which already
+	// rejects --last-commit-ts T < manifest), and it's the load-bearing
+	// guard for future in-process library callers (Phase 1 live extractor,
+	// integration tests) that bypass the CLI: a library caller that
+	// forgets to compare against the manifest can no longer silently
+	// publish a low-TS .fsm (codex P2 v2 #904). Callers that genuinely
+	// have no manifest reference (synthetic test fixtures) leave this
+	// at 0 to opt out of the check.
+	ManifestLastCommitTS uint64
 	// SelfTest enables the round-trip self-test. When true,
 	// EncodeSnapshot writes the FSM to an on-disk temp file under
 	// SelfTestDecodeOptions.OutRoot (encode-self-test-fsm-*), streams
@@ -133,30 +145,49 @@ type EncodeResult struct {
 // The CLI relies on this contract to write mismatch.txt + exit 2;
 // library callers should follow the same pattern.
 //
-// EncodeSnapshot does NOT read MANIFEST.json and does NOT validate
-// opts.LastCommitTS against any external floor — the caller is
-// responsible for reading the manifest, computing the effective T,
-// and returning ErrSelfTestLowerLastCommitTS on regression (the CLI's
-// resolveLastCommitTS performs this check before calling EncodeSnapshot
-// and maps that error to exit code 2). A future library caller that
-// skips that step would silently stamp a too-low timestamp into the
-// .fsm header (claude v3 doc bug #904).
-func EncodeSnapshot(opts EncodeOptions, out io.Writer) (EncodeResult, error) {
+// EncodeSnapshot does NOT read MANIFEST.json itself, but it WILL
+// enforce a floor on opts.LastCommitTS when the caller threads the
+// manifest value through opts.ManifestLastCommitTS — a low
+// LastCommitTS returns ErrSelfTestLowerLastCommitTS BEFORE any bytes
+// are written. The CLI's resolveLastCommitTS sets both fields to the
+// reconciled values, and library callers SHOULD do the same. The
+// check is opt-in (ManifestLastCommitTS=0 disables it) so synthetic
+// test fixtures without a manifest reference can still call this
+// directly (codex P2 v2 #904).
+// validateEncodeOptions enforces the four pre-encode invariants:
+// InputRoot/out non-nil, non-empty adapter selection, and the optional
+// manifest-TS floor. Split out so EncodeSnapshot stays under the cyclop
+// threshold.
+func validateEncodeOptions(opts EncodeOptions, out io.Writer) error {
 	if opts.InputRoot == "" {
-		return EncodeResult{}, errors.New("backup: EncodeOptions.InputRoot is required")
+		return errors.New("backup: EncodeOptions.InputRoot is required")
 	}
 	if out == nil {
-		return EncodeResult{}, errors.New("backup: EncodeSnapshot out writer is nil")
+		return errors.New("backup: EncodeSnapshot out writer is nil")
 	}
 	if !opts.Adapters.DynamoDB && !opts.Adapters.S3 && !opts.Adapters.Redis && !opts.Adapters.SQS {
-		// A zero AdapterSet would silently produce a valid header-only
-		// .fsm with no adapter records — a "successful" empty restore
-		// artifact. The CLI's parseAdapterSet already rejects this for
-		// flag-driven entry, but a future in-process caller (Phase 1
-		// live extractor, integration tests) might forget to thread
-		// the set; fail-closed here so that mistake surfaces (codex v5
-		// + claude v5 #904).
-		return EncodeResult{}, errors.New("backup: EncodeOptions.Adapters has no enabled adapter")
+		// Zero AdapterSet would silently produce a header-only .fsm —
+		// a "successful" empty restore artifact. CLI's parseAdapterSet
+		// rejects this for flag-driven entry; library callers (Phase 1
+		// live extractor, integration tests) get the same guard here
+		// (codex v5 + claude v5 #904).
+		return errors.New("backup: EncodeOptions.Adapters has no enabled adapter")
+	}
+	if opts.ManifestLastCommitTS > 0 && opts.LastCommitTS < opts.ManifestLastCommitTS {
+		// Defense-in-depth HLC ceiling floor (codex P2 v2 #904). The
+		// CLI's resolveLastCommitTS already enforces this for flag-
+		// driven entry; library callers that thread the manifest value
+		// via ManifestLastCommitTS get the same fail-closed guard here.
+		return errors.Wrapf(ErrSelfTestLowerLastCommitTS,
+			"EncodeSnapshot opts.LastCommitTS %d < opts.ManifestLastCommitTS %d",
+			opts.LastCommitTS, opts.ManifestLastCommitTS)
+	}
+	return nil
+}
+
+func EncodeSnapshot(opts EncodeOptions, out io.Writer) (EncodeResult, error) {
+	if err := validateEncodeOptions(opts, out); err != nil {
+		return EncodeResult{}, err
 	}
 
 	b := newSnapshotBuilder(opts.LastCommitTS)

internal/backup/encode_snapshot_test.go

@@ -5,6 +5,8 @@ import (
 	"os"
 	"path/filepath"
 	"testing"
+
+	"github.com/cockroachdb/errors"
 )
 
 // TestEncodeSnapshotLibraryRoundTrip pins the public library entrypoint:
@@ -209,6 +211,58 @@ func TestEncodeSnapshotRequiresInputRoot(t *testing.T) {
 	}
 }
 
+// TestEncodeSnapshotRejectsLowManifestFloor pins codex P2 v2: the
+// library-level HLC floor check fails-closed when opts.LastCommitTS
+// is below opts.ManifestLastCommitTS. Defense-in-depth for the CLI's
+// resolveLastCommitTS — a future in-process caller (Phase 1 live
+// extractor) cannot silently publish a low-TS .fsm.
+func TestEncodeSnapshotRejectsLowManifestFloor(t *testing.T) {
+	t.Parallel()
+	in := t.TempDir()
+	var buf bytes.Buffer
+	_, err := EncodeSnapshot(EncodeOptions{
+		InputRoot:            in,
+		Adapters:             AdapterSet{SQS: true},
+		LastCommitTS:         500,
+		ManifestLastCommitTS: 1000, // floor; LastCommitTS is below
+	}, &buf)
+	if err == nil {
+		t.Fatalf("EncodeSnapshot with LastCommitTS < ManifestLastCommitTS succeeded; want error")
+	}
+	if !errors.Is(err, ErrSelfTestLowerLastCommitTS) {
+		t.Errorf("err = %v, want errors.Is ErrSelfTestLowerLastCommitTS", err)
+	}
+	if buf.Len() != 0 {
+		t.Errorf("buf.Len = %d, want 0 (no bytes should be written on floor regression)", buf.Len())
+	}
+}
+
+// TestEncodeSnapshotManifestFloorOptOut pins that ManifestLastCommitTS=0
+// disables the check (synthetic test fixtures, library callers without a
+// manifest reference). The existing TestEncodeSnapshotLibraryRoundTrip
+// implicitly relies on this opt-out.
+func TestEncodeSnapshotManifestFloorOptOut(t *testing.T) {
+	t.Parallel()
+	in := t.TempDir()
+	const queue = "floor-opt-out"
+	writeSQSQueue(t, in, queue,
+		[]byte(`{"format_version":1,"name":"floor-opt-out","fifo":false,"partition_count":1,"generation":1}`),
+		[][]byte{
+			[]byte(`{"format_version":1,"message_id":"m1","body":"a","send_timestamp_millis":1700000000000,"available_at_millis":1700000000000,"sequence_number":0}`),
+		},
+	)
+	var buf bytes.Buffer
+	_, err := EncodeSnapshot(EncodeOptions{
+		InputRoot:            in,
+		Adapters:             AdapterSet{SQS: true},
+		LastCommitTS:         500,
+		ManifestLastCommitTS: 0, // opt-out
+	}, &buf)
+	if err != nil {
+		t.Fatalf("EncodeSnapshot with opt-out floor failed: %v", err)
+	}
+}
+
 // TestEncodeSnapshotRejectsZeroAdapterSet pins claude v5 + codex v5
 // carry-over: a library caller that forgets to thread Adapters into
 // EncodeOptions gets a fail-closed error rather than a silently empty