backup: #904 v16 - correct v14 Redis multi-DB fan-out (codex P2 v14)

v14 added an enumerate-and-fan-out path for redis/db_<N>/ to address
codex P1 v13's silent-data-loss concern. Codex's v14 follow-up
clarified that the fan-out is itself incorrect, and two P2 findings
landed (L452 + L427).

## Codex P2 v14 L452: Reject Redis DB fan-out until DBs are encoded

The Redis MVCC key prefixes (!redis|str|, !redis|hll|, !redis|ttl|,
plus the collection helpers' analogues) carry NO database component.
Feeding two distinct DBs into the same snapshotBuilder would:
- collide on same-named keys across DBs (b.Add returns ErrDuplicate),
  or
- merge keys under db_0 at decode time (DecodeOptions.RedisDBIndex
  defaults to 0, so a db_3-only self-test would decode under db_0
  and the structural diff would fail).

In either case the produced .fsm is mis-scoped: the v14 fan-out
replaces silent-data-loss with silent-merge-or-collide.

Fix: encodeAllRedisDBs now enumerates and decides:
- 0 indices (no redis/, or empty redis/) → no-op.
- [0] only → proceed with NewRedisEncoder(_, 0) (the legacy path).
- anything else → fail closed with new sentinel
  ErrRedisEncodeMultiDBUnsupported.

The sentinel is marked at the runAdapterEncoders boundary with
ErrEncodeAdapterData so the CLI routes it to exit-2. Until Phase 1
makes native keys DB-aware, multi-DB inputs are quarantined as
data-correctness failures rather than silently mis-scoped output.

## Codex P2 v14 L427: Fail closed on malformed Redis db_N entries

The v14 parseRedisDBDir(ent) returned (_, false) whenever
!ent.IsDir(), so a regular file or symlink at redis/db_0 would be
silently skipped and the encoder would publish a header-only .fsm.

Fix: split into parseRedisDBName (pure name parser) and shift the
ent.IsDir() check into enumerateRedisDBs. When the name matches the
canonical db_<N> pattern AND the entry is not a directory,
enumerateRedisDBs returns ErrRedisEncodeNotDir (mirroring the per-DB
encoder's existing Lstat guard).

## Pinned by

- TestEncodeSnapshotRedisRejectsNonZeroDB: redis/db_3-only fixture
  through EncodeSnapshot → errors.Is ErrRedisEncodeMultiDBUnsupported
  AND errors.Is ErrEncodeAdapterData (both fire).
- TestEncodeSnapshotRedisRejectsMultipleDBs: redis/db_0 + redis/db_3
  → errors.Is ErrRedisEncodeMultiDBUnsupported.
- TestEnumerateRedisDBsRejectsNonDirDBEntry: redis/db_2 as a regular
  file → errors.Is ErrRedisEncodeNotDir.
- The replaced TestEncodeSnapshotRedisMultiDB (v14) was reformulated
  as TestEncodeSnapshotRedisRejectsNonZeroDB; the prior "more bytes
  than baseline" assertion no longer holds because the encoder
  rejects the fixture upfront.

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

- encodeAllRedisDBs: sole caller is adapterRunners.redis. Semantic
  change: was "fan out across all db_<N>"; now "encode db_0 only,
  fail closed on anything else." Error contract:
  - errors.Is(err, ErrEncodeAdapterData) — still true (mark applied
    at runAdapterEncoders, unchanged).
  - errors.Is(err, ErrRedisEncodeMultiDBUnsupported) — new path.
  - errors.Is(err, ErrRedisEncodeNotDir) — newly reachable via
    enumerateRedisDBs for malformed db_<N> entries.
  All in-tree NewRedisEncoder direct callers (encode_redis_test.go,
  encode_redis_coll_test.go, encode_redis_hardlink_unix_test.go) are
  unaffected — they bypass the adapter runner.
- enumerateRedisDBs: was "silently skip non-dir db_<N>"; now "fail
  closed with ErrRedisEncodeNotDir." No production callers exist
  outside encodeAllRedisDBs; tests in encode_snapshot_test.go cover
  the new fail-closed path.
- parseRedisDBDir → parseRedisDBName: signature change (DirEntry →
  string). Sole caller is enumerateRedisDBs (updated in lock-step).
- v14's TestEncodeSnapshotRedisMultiDB is replaced — see above.
  TestEnumerateRedisDBsMixedEntries (v14) continues to pass: its
  positive entries (db_0, db_1, db_5) are all directories, so the
  IsDir check it never exercises remains exercised by the new
  TestEnumerateRedisDBsRejectsNonDirDBEntry.

Tests + lint green.

Author: bootjp

internal/backup/encode_snapshot.go

@@ -41,6 +41,22 @@ var ErrSelfTestLowerLastCommitTS = errors.New("backup: --last-commit-ts T < mani
 // until the encoder learns the JSONL layout (M7 / future milestone).
 var ErrEncodeUnsupportedDynamoDBLayout = errors.New("backup: DynamoDB JSONL layout not supported by encoder")
 
+// ErrRedisEncodeMultiDBUnsupported is returned when the input tree
+// contains a Redis db_<N>/ for any N != 0, or contains multiple
+// db_<N> directories. The current Redis MVCC key prefixes
+// (!redis|str|, !redis|hll|, !redis|ttl|, …) carry NO database
+// component, so feeding two distinct DBs into the same snapshot
+// builder would either collide on same-named keys or silently merge
+// both DBs under db_0 on restore (DecodeOptions.RedisDBIndex
+// defaults to 0). Failing closed preserves correctness until Phase 1
+// makes the native keys DB-aware (codex P2 v14 #904).
+//
+// v14 originally fanned out across db_<N> to address codex P1 v13's
+// silent-data-loss concern; codex's v14 follow-up clarified that
+// fan-out under the current key format produces mis-scoped output.
+// The corrected fix replaces fan-out with fail-closed.
+var ErrRedisEncodeMultiDBUnsupported = errors.New("backup: redis encoder requires single db_0 (multi-DB or non-zero DB not yet supported)")
+
 // ErrEncodeAdapterData marks every error returned by an adapter
 // encoder (Redis / DynamoDB / S3 / SQS) so callers can distinguish
 // "the input tree contained content the encoder cannot translate"
@@ -387,9 +403,20 @@ func enumerateRedisDBs(inRoot string) ([]int, error) {
 	}
 	var indices []int
 	for _, ent := range entries {
-		if idx, ok := parseRedisDBDir(ent); ok {
-			indices = append(indices, idx)
+		idx, ok := parseRedisDBName(ent.Name())
+		if !ok {
+			continue
 		}
+		// Canonical db_<N> name; entry MUST be a directory.
+		// Silently skipping a regular file or symlink at
+		// redis/db_<N> would let a malformed dump publish a
+		// header-only/partial FSM (codex P2 v14 #904 L427).
+		if !ent.IsDir() {
+			return nil, errors.Wrapf(ErrRedisEncodeNotDir,
+				"redis/%s exists but is not a directory (mode=%s)",
+				ent.Name(), ent.Type())
+		}
+		indices = append(indices, idx)
 	}
 	sort.Ints(indices)
 	return indices, nil
@@ -416,17 +443,19 @@ func checkRedisRoot(redisDir string) error {
 	return nil
 }
 
-// parseRedisDBDir returns (dbIndex, true) when ent names a canonical
-// db_<N> directory (N is a non-negative decimal with no leading zeros).
-// Non-matching entries return (0, false) so the caller can skip without
-// erroring — they cannot have been produced by the canonical decoder.
-// Reject non-canonical decimals so a hypothetical Phase 1 dumper cannot
-// double-emit the same db under two distinct directory names.
-func parseRedisDBDir(ent os.DirEntry) (int, bool) {
-	if !ent.IsDir() {
-		return 0, false
-	}
-	name := ent.Name()
+// parseRedisDBName returns (dbIndex, true) when name matches the
+// canonical db_<N> pattern (N is a non-negative decimal with no
+// leading zeros). Non-matching names return (0, false) so the caller
+// can skip them without erroring — they cannot have been produced by
+// the canonical decoder. Reject non-canonical decimals so a
+// hypothetical Phase 1 dumper cannot double-emit the same db under
+// two distinct directory names.
+//
+// This is a pure name parser; the caller is responsible for
+// validating the directory-entry shape (codex P2 v14 #904 L427
+// shifted the IsDir check to enumerateRedisDBs so a regular file
+// at redis/db_<N> fails closed instead of being silently skipped).
+func parseRedisDBName(name string) (int, bool) {
 	if !strings.HasPrefix(name, redisDBDirPrefix) {
 		return 0, false
 	}
@@ -438,20 +467,35 @@ func parseRedisDBDir(ent os.DirEntry) (int, bool) {
 	return idx, true
 }
 
-// encodeAllRedisDBs invokes NewRedisEncoder per discovered db_<N>
-// directory in ascending index order. A missing redis/ directory is a
-// no-op. Codex P1 v13 #904: replaces the prior hardcoded db_0 fan-out
-// which would silently drop non-default DBs from any Phase 1 multi-DB
-// dump. Per-DB errors are wrapped with the db index for traceability.
+// encodeAllRedisDBs invokes NewRedisEncoder for redis/db_0/ when the
+// input tree has exactly that DB (or no Redis content at all). A
+// missing redis/ directory is a no-op. Any non-zero DB or the
+// presence of multiple db_<N> directories fails closed with
+// ErrRedisEncodeMultiDBUnsupported.
+//
+// Codex P1 v13 #904 originally asked for a per-DB fan-out to address
+// the prior hardcoded db_0 dispatch silently dropping non-default
+// DBs. Codex P2 v14 #904 (L452) clarified that fan-out under the
+// current MVCC key prefixes (!redis|str|, !redis|hll|, !redis|ttl|,
+// …, none of which carry a database component) would either collide
+// on same-named keys across DBs or merge everything under db_0 at
+// decode time. The corrected fix replaces the silent drop and the
+// incorrect fan-out with a fail-closed sentinel until Phase 1
+// makes the native keys DB-aware.
 func encodeAllRedisDBs(b *snapshotBuilder, inRoot string) error {
 	indices, err := enumerateRedisDBs(inRoot)
 	if err != nil {
 		return errors.Wrap(err, "redis encoder enumerate")
 	}
-	for _, idx := range indices {
-		if err := NewRedisEncoder(inRoot, idx).Encode(b); err != nil {
-			return errors.Wrapf(err, "redis encoder db_%d", idx)
-		}
+	if len(indices) == 0 {
+		return nil
+	}
+	if len(indices) > 1 || indices[0] != 0 {
+		return errors.Wrapf(ErrRedisEncodeMultiDBUnsupported,
+			"redis encoder enumerated db indices %v", indices)
+	}
+	if err := NewRedisEncoder(inRoot, 0).Encode(b); err != nil {
+		return errors.Wrap(err, "redis encoder db_0")
 	}
 	return nil
 }

internal/backup/encode_snapshot_test.go

@@ -459,30 +459,20 @@ func TestEnumerateRedisDBsRedisIsRegularFile(t *testing.T) {
 	}
 }
 
-// TestEncodeSnapshotRedisMultiDB pins codex P1 v13 #904: the Redis
-// fan-out in adapterRunners enumerates redis/db_<N>/ and invokes the
-// per-DB encoder for each. The fixture places a single string under
-// redis/db_3/ ONLY (no db_0). Pre-fix, the encoder hardcoded
-// NewRedisEncoder(root, 0) and produced a header-only .fsm for this
-// input — silent data loss. Post-fix, the db_3 string is included.
+// TestEncodeSnapshotRedisRejectsNonZeroDB pins codex P2 v14 #904
+// L452: the Redis MVCC key prefixes (!redis|str|, !redis|hll|,
+// !redis|ttl|, …) carry no database component, so feeding a
+// non-zero DB through the encoder would mis-scope the produced
+// .fsm — same-named keys collide and a db_3-only self-test would
+// decode under db_0. Until Phase 1 makes native keys DB-aware,
+// non-zero-DB inputs MUST fail closed.
 //
-// Assertion is content-free: compare encoded byte count against an
-// empty-redis baseline. With multi-DB fan-out, the db_3 fixture
-// produces MORE bytes than an empty tree. Without it, both encodes
-// would produce identical header-only output.
-func TestEncodeSnapshotRedisMultiDB(t *testing.T) {
+// The fixture places a single string under redis/db_3/ ONLY.
+// EncodeSnapshot must reject with ErrRedisEncodeMultiDBUnsupported
+// and write no bytes. (v14 originally attempted to fan out per DB;
+// codex's L452 follow-up established the correct fix is fail-closed.)
+func TestEncodeSnapshotRedisRejectsNonZeroDB(t *testing.T) {
 	t.Parallel()
-	emptyIn := t.TempDir()
-	var emptyBuf bytes.Buffer
-	emptyResult, err := EncodeSnapshot(EncodeOptions{
-		InputRoot:    emptyIn,
-		Adapters:     AdapterSet{Redis: true},
-		LastCommitTS: 1,
-	}, &emptyBuf)
-	if err != nil {
-		t.Fatalf("EncodeSnapshot empty: %v", err)
-	}
-
 	in := t.TempDir()
 	encKey := EncodeSegment([]byte("k3"))
 	db3Strings := filepath.Join(in, "redis", "db_3", "strings")
@@ -493,17 +483,68 @@ func TestEncodeSnapshotRedisMultiDB(t *testing.T) {
 		t.Fatalf("WriteFile db_3 string: %v", err)
 	}
 	var buf bytes.Buffer
-	result, err := EncodeSnapshot(EncodeOptions{
+	_, err := EncodeSnapshot(EncodeOptions{
 		InputRoot:    in,
 		Adapters:     AdapterSet{Redis: true},
 		LastCommitTS: 1,
 	}, &buf)
-	if err != nil {
-		t.Fatalf("EncodeSnapshot db_3-only: %v", err)
+	if err == nil {
+		t.Fatalf("EncodeSnapshot accepted db_3-only Redis input; want ErrRedisEncodeMultiDBUnsupported")
+	}
+	if !errors.Is(err, ErrRedisEncodeMultiDBUnsupported) {
+		t.Errorf("err = %v, want errors.Is ErrRedisEncodeMultiDBUnsupported", err)
+	}
+	// Marked as adapter-data so the CLI routes it to exit-2.
+	if !errors.Is(err, ErrEncodeAdapterData) {
+		t.Errorf("err = %v, want errors.Is ErrEncodeAdapterData (mark from runAdapterEncoders)", err)
+	}
+}
+
+// TestEncodeSnapshotRedisRejectsMultipleDBs pins the multi-DB case:
+// redis/db_0 + redis/db_3 → ErrRedisEncodeMultiDBUnsupported (the
+// fan-out would collide on same-named keys or merge both DBs under
+// db_0 on restore; codex P2 v14 #904 L452).
+func TestEncodeSnapshotRedisRejectsMultipleDBs(t *testing.T) {
+	t.Parallel()
+	in := t.TempDir()
+	for _, name := range []string{"db_0", "db_3"} {
+		if err := os.MkdirAll(filepath.Join(in, "redis", name, "strings"), 0o755); err != nil {
+			t.Fatalf("MkdirAll %s: %v", name, err)
+		}
 	}
-	if result.BytesWritten <= emptyResult.BytesWritten {
-		t.Errorf("BytesWritten with redis/db_3 fixture (%d) <= empty (%d); pre-fix, hardcoded db_0 fan-out dropped db_3 silently",
-			result.BytesWritten, emptyResult.BytesWritten)
+	var buf bytes.Buffer
+	_, err := EncodeSnapshot(EncodeOptions{
+		InputRoot:    in,
+		Adapters:     AdapterSet{Redis: true},
+		LastCommitTS: 1,
+	}, &buf)
+	if err == nil {
+		t.Fatalf("EncodeSnapshot accepted db_0 + db_3; want ErrRedisEncodeMultiDBUnsupported")
+	}
+	if !errors.Is(err, ErrRedisEncodeMultiDBUnsupported) {
+		t.Errorf("err = %v, want errors.Is ErrRedisEncodeMultiDBUnsupported", err)
+	}
+}
+
+// TestEnumerateRedisDBsRejectsNonDirDBEntry pins codex P2 v14 #904
+// L427: when a canonical db_<N> name resolves to a regular file
+// (or symlink) instead of a directory, enumerateRedisDBs must fail
+// closed with ErrRedisEncodeNotDir — silently skipping would let a
+// malformed dump publish a header-only/partial FSM.
+func TestEnumerateRedisDBsRejectsNonDirDBEntry(t *testing.T) {
+	t.Parallel()
+	in := t.TempDir()
+	if err := os.MkdirAll(filepath.Join(in, "redis"), 0o755); err != nil {
+		t.Fatalf("MkdirAll: %v", err)
+	}
+	// redis/db_2 is a regular file — name matches the canonical
+	// pattern but the entry shape is wrong.
+	if err := os.WriteFile(filepath.Join(in, "redis", "db_2"), []byte("not a dir"), 0o600); err != nil {
+		t.Fatalf("WriteFile: %v", err)
+	}
+	_, err := enumerateRedisDBs(in)
+	if !errors.Is(err, ErrRedisEncodeNotDir) {
+		t.Errorf("err = %v, want errors.Is ErrRedisEncodeNotDir", err)
 	}
 }