backup: #904 v8 - fail-closed on DynamoDB JSONL + fix stale godoc

Two findings on v7.

## Codex P2 v7: silent data loss on DynamoDB JSONL inputs

The decoder writes items/data-*.jsonl when --dynamodb-bundle-mode jsonl
is set, and the manifest records this as `dynamodb_layout: "jsonl"`.
The reverse encoder, however, only walks per-item files
(items/*.json, items/*/*.json) and silently skips every JSONL file.
Result: a JSONL dump round-trips through the encoder as an .fsm with
ONLY table metadata and ZERO items — a "successful" empty restore
artifact.

Fix:
- New ErrEncodeUnsupportedDynamoDBLayout sentinel.
- EncodeOptions gains DynamoDBBundleJSONL bool. validateEncodeOptions
  rejects when DynamoDBBundleJSONL && Adapters.DynamoDB.
- CLI's buildEncodeOptions sets the field from
  manifest.DynamoDBLayout == DynamoDBLayoutJSONL.
- CLI's run() maps the error to exit-2 (data-correctness, like the
  HLC ceiling regression).

Guard fires only when DDB is in scope so a caller encoding ONLY
Redis/SQS/S3 with the flag accidentally set is unaffected; pinned by
TestEncodeSnapshotJSONLOnlyRejectedWhenDDBEnabled. Rejection path
pinned by TestEncodeSnapshotRejectsDynamoDBJSONLLayout.

When the encoder learns the JSONL layout (future milestone), this
field switches from a fail-closed guard to a layout selector.

## Claude v7 doc bug: ErrSelfTestLowerLastCommitTS comment is stale

The var comment at lines 17-22 was accurate after v4's doc fix
("EncodeSnapshot itself does NOT enforce this floor") but v7 added
validateEncodeOptions which DOES enforce it when ManifestLastCommitTS > 0.
Comment now describes both layers (CLI + library) and points at the
codex P2 v2 plus claude v3 history.

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

- EncodeOptions gained DynamoDBBundleJSONL bool. CLI sets it from
  manifest; existing same-package tests pass EncodeOptions{} so the
  zero default (false) preserves their behavior. No legitimate caller
  impacted.
- validateEncodeOptions split into validateEncodeOptions +
  validateEncodeOptionsData to keep both under the cyclop threshold.
  Pure refactor: identical control flow, same errors returned.
- run() gained a third errors.Is branch for the new sentinel; same
  exit-2 mapping pattern as the prior two.

Tests + lint green.

Author: bootjp

cmd/elastickv-snapshot-encode/main.go

@@ -87,6 +87,9 @@ func run(argv []string, logger *slog.Logger) (int, error) {
 		if errors.Is(err, backup.ErrSelfTestLowerLastCommitTS) {
 			return exitDataErr, err
 		}
+		if errors.Is(err, backup.ErrEncodeUnsupportedDynamoDBLayout) {
+			return exitDataErr, err
+		}
 		if errors.Is(err, errSelfTestMismatch) {
 			return exitDataErr, err
 		}
@@ -294,6 +297,7 @@ func buildEncodeOptions(cfg *config, effectiveTS uint64, manifest backup.Manifes
 		Adapters:             cfg.adapters,
 		LastCommitTS:         effectiveTS,
 		ManifestLastCommitTS: manifest.LastCommitTS,
+		DynamoDBBundleJSONL:  manifest.DynamoDBLayout == backup.DynamoDBLayoutJSONL,
 		SelfTest:             cfg.selfTest,
 	}
 	if cfg.selfTest {

internal/backup/encode_snapshot.go

@@ -14,20 +14,33 @@ import (
 	"github.com/cockroachdb/errors"
 )
 
-// ErrSelfTestLowerLastCommitTS is the sentinel callers should return
-// when the operator-supplied T is below the manifest's last_commit_ts.
-// The HLC ceiling invariant (CLAUDE.md "Timestamp Oracle") forbids
-// lowering the ceiling on restore: a lower T would let a post-restart
-// leader issue a read ts ≤ a restored row's commit ts.
+// ErrSelfTestLowerLastCommitTS is returned when the operator-supplied
+// T is below the manifest's last_commit_ts. The HLC ceiling invariant
+// (CLAUDE.md "Timestamp Oracle") forbids lowering the ceiling on
+// restore: a lower T would let a post-restart leader issue a read
+// ts ≤ a restored row's commit ts.
 //
-// EncodeSnapshot itself does NOT read MANIFEST.json or enforce this
-// floor — the comparison happens in the CLI's resolveLastCommitTS
-// BEFORE EncodeSnapshot is called, and a future library caller (Phase 1
-// live extractor, integration tests) must perform its own comparison
-// and return this sentinel on regression so callers can errors.Is on
-// it to map to the right exit code (claude v3 doc bug #904).
+// Enforced at two layers:
+//   - CLI (`resolveLastCommitTS`) rejects --last-commit-ts T < manifest
+//     before EncodeSnapshot is called (exit code 2).
+//   - Library (`validateEncodeOptions`) rejects when the caller threads
+//     `opts.ManifestLastCommitTS > 0` and `opts.LastCommitTS` is below
+//     it — defense-in-depth for in-process callers (Phase 1 live
+//     extractor, integration tests) that bypass the CLI.
+//
+// Callers can errors.Is on this sentinel to map to the right exit code
+// (claude v3 doc bug #904 + claude v7 doc bug #904 + codex P2 v2 #904).
 var ErrSelfTestLowerLastCommitTS = errors.New("backup: --last-commit-ts T < manifest.last_commit_ts (HLC ceiling regression)")
 
+// ErrEncodeUnsupportedDynamoDBLayout is returned when an input dump
+// declares `dynamodb_layout: "jsonl"` in MANIFEST.json. The DynamoDB
+// reverse encoder only walks per-item files (items/*.json,
+// items/*/*.json) and would silently skip every items/data-*.jsonl
+// file, producing an .fsm with only table metadata and no items —
+// a silent-data-loss restore artifact (codex P2 v7 #904). Fail closed
+// until the encoder learns the JSONL layout (M7 / future milestone).
+var ErrEncodeUnsupportedDynamoDBLayout = errors.New("backup: DynamoDB JSONL layout not supported by encoder")
+
 // The encoder dispatch order (redis → dynamodb → s3 → sqs) is encoded
 // inside adapterRunners() and is intentionally distinct from decode.go's
 // finalize order (dynamodb → s3 → redis → sqs). The final .fsm byte
@@ -53,6 +66,15 @@ 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
+	// DynamoDBBundleJSONL is true when the input dump's MANIFEST.json
+	// has `dynamodb_layout: "jsonl"`. The reverse encoder does not
+	// support that layout — it would silently skip every
+	// items/data-*.jsonl file and publish an .fsm with only table
+	// metadata. Fail-closed via ErrEncodeUnsupportedDynamoDBLayout
+	// when true (codex P2 v7 #904). When the encoder gains JSONL
+	// support, this field will switch from a guard to a control.
+	DynamoDBBundleJSONL bool
+
 	// ManifestLastCommitTS is the floor LastCommitTS must not fall
 	// below. When > 0, EncodeSnapshot fails-closed with
 	// ErrSelfTestLowerLastCommitTS if LastCommitTS < ManifestLastCommitTS.
@@ -155,9 +177,10 @@ type EncodeResult struct {
 // 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.
+// InputRoot/out non-nil, non-empty adapter selection, optional
+// manifest-TS floor, and DDB JSONL guard. Split out so EncodeSnapshot
+// stays under the cyclop threshold; per-check helpers below keep each
+// branch's intent narrow.
 func validateEncodeOptions(opts EncodeOptions, out io.Writer) error {
 	if opts.InputRoot == "" {
 		return errors.New("backup: EncodeOptions.InputRoot is required")
@@ -167,21 +190,27 @@ func validateEncodeOptions(opts EncodeOptions, out io.Writer) error {
 	}
 	if !opts.Adapters.DynamoDB && !opts.Adapters.S3 && !opts.Adapters.Redis && !opts.Adapters.SQS {
 		// 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).
+		// a "successful" empty restore artifact (codex v5 + claude v5 #904).
 		return errors.New("backup: EncodeOptions.Adapters has no enabled adapter")
 	}
+	return validateEncodeOptionsData(opts)
+}
+
+// validateEncodeOptionsData covers the data-correctness pre-conditions:
+// HLC ceiling floor and DynamoDB JSONL guard. Kept separate from the
+// nil/empty-args checks so each function stays cyclop-clean.
+func validateEncodeOptionsData(opts EncodeOptions) error {
 	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.
+		// Defense-in-depth HLC ceiling floor (codex P2 v2 #904).
 		return errors.Wrapf(ErrSelfTestLowerLastCommitTS,
 			"EncodeSnapshot opts.LastCommitTS %d < opts.ManifestLastCommitTS %d",
 			opts.LastCommitTS, opts.ManifestLastCommitTS)
 	}
+	if opts.DynamoDBBundleJSONL && opts.Adapters.DynamoDB {
+		// The DynamoDB reverse encoder only walks per-item files;
+		// JSONL items would be silently skipped (codex P2 v7 #904).
+		return errors.WithStack(ErrEncodeUnsupportedDynamoDBLayout)
+	}
 	return nil
 }
 

internal/backup/encode_snapshot_test.go

@@ -263,6 +263,61 @@ func TestEncodeSnapshotManifestFloorOptOut(t *testing.T) {
 	}
 }
 
+// TestEncodeSnapshotRejectsDynamoDBJSONLLayout pins codex P2 v7 #904:
+// the DynamoDB reverse encoder does not support the JSONL bundle
+// layout, so a caller that threads DynamoDBBundleJSONL=true must be
+// rejected with ErrEncodeUnsupportedDynamoDBLayout before any bytes
+// are written. The CLI hits this path automatically when MANIFEST.json
+// has `dynamodb_layout: "jsonl"`; library callers that mirror that
+// thread the field themselves.
+func TestEncodeSnapshotRejectsDynamoDBJSONLLayout(t *testing.T) {
+	t.Parallel()
+	in := t.TempDir()
+	var buf bytes.Buffer
+	_, err := EncodeSnapshot(EncodeOptions{
+		InputRoot:           in,
+		Adapters:            AdapterSet{DynamoDB: true},
+		LastCommitTS:        1,
+		DynamoDBBundleJSONL: true,
+	}, &buf)
+	if err == nil {
+		t.Fatalf("EncodeSnapshot with DynamoDBBundleJSONL accepted; want error")
+	}
+	if !errors.Is(err, ErrEncodeUnsupportedDynamoDBLayout) {
+		t.Errorf("err = %v, want errors.Is ErrEncodeUnsupportedDynamoDBLayout", err)
+	}
+	if buf.Len() != 0 {
+		t.Errorf("buf.Len = %d, want 0 (no bytes should be written when JSONL is rejected)", buf.Len())
+	}
+}
+
+// TestEncodeSnapshotJSONLOnlyRejectedWhenDDBEnabled pins that the JSONL
+// guard fires only when DynamoDB is in the adapter set — a caller that
+// happens to set DynamoDBBundleJSONL=true while encoding ONLY Redis (or
+// any other adapter) is unaffected. Prevents the guard from becoming
+// over-zealous for callers who simply mirror the manifest field.
+func TestEncodeSnapshotJSONLOnlyRejectedWhenDDBEnabled(t *testing.T) {
+	t.Parallel()
+	in := t.TempDir()
+	const queue = "no-ddb"
+	writeSQSQueue(t, in, queue,
+		[]byte(`{"format_version":1,"name":"no-ddb","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}, // DDB NOT in scope
+		LastCommitTS:        1,
+		DynamoDBBundleJSONL: true, // would be rejected if DDB were enabled
+	}, &buf)
+	if err != nil {
+		t.Fatalf("EncodeSnapshot rejected JSONL flag when DDB not in scope: %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