Skip to content

Commit 48b0b0e

Browse files
committed
backup: #904 v13 - claude v12 doc fix + two carry-over cleanups
## Claude v12 mandatory: writeAndPublish godoc missed v12 cleanup The godoc described only the temp-file deferred cleanup but omitted the new stale-<output>.fsm removal added in v12. A future reader auditing the on-disk state after self-test mismatch would see only the temp-file cleanup documented and need to grep for cfg.outputPath to discover the second removal. Expanded the godoc to call out both. ## Claude v12 carry-over: canonicalizeInput swallowed os.Open error The test helper used "f, _ := os.Open(tmpOut)" and passed nil to DecodeSnapshot if the open failed, producing a panic or misleading decode error instead of a clean t.Fatalf. Fixed: check the error explicitly and defer the Close. CodeRabbit flagged this on v11/v12. ## Claude v12 carry-over: test name contradicted its assertion TestCLISelfTestMismatchWritesSidecarWithMatchedFalse asserted os.IsNotExist on the sidecar — the encode never ran on the manifest-floor path it exercises. Name renamed to TestCLIManifestFloorLeavesNoStaleSidecar to match the actual assertion. The real "sidecar IS written with matched=false on self-test mismatch" behavior is now pinned end-to-end by TestCLIWriteAndPublishRemovesStaleFSMOnSelfTestMismatch (v12) which drives a real self-test mismatch through the new corruption seam. Docstring updated to explain the rename history. ## Caller audit per CLAUDE.md semantic-change rule All v13 changes are doc/identifier/error-handling only. No public or package-private function signature, error semantics, or return contract changed. Renamed test has no callers (Go testing framework discovers by prefix). canonicalizeInput's error handling is strictly safer — any open failure now surfaces at the failure site. Tests + lint green.
1 parent 5a2f214 commit 48b0b0e

2 files changed

Lines changed: 28 additions & 21 deletions

File tree

cmd/elastickv-snapshot-encode/main.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -314,8 +314,10 @@ func buildEncodeOptions(cfg *config, effectiveTS uint64, manifest backup.Manifes
314314

315315
// writeAndPublish writes the .fsm to a temp path, runs the optional
316316
// self-test via EncodeSnapshot, and renames temp → output on success.
317-
// On self-test failure: writes mismatch.txt, removes the temp file via
318-
// the deferred cleanup, returns errSelfTestMismatch.
317+
// On self-test failure: writes mismatch.txt, removes any stale
318+
// <output>.fsm left by a prior successful run (codex P2 v10 #904),
319+
// removes the temp file via the deferred cleanup, returns
320+
// errSelfTestMismatch.
319321
func writeAndPublish(cfg *config, encodeOpts backup.EncodeOptions, mismatchPath string, logger *slog.Logger) (backup.EncodeResult, error) {
320322
tempPath, err := tempOutputPath(cfg.outputPath)
321323
if err != nil {

cmd/elastickv-snapshot-encode/main_test.go

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -275,14 +275,17 @@ func canonicalizeInput(t *testing.T, rawIn string, lastCommitTS uint64) string {
275275
if err != nil || code != exitSuccess {
276276
t.Fatalf("canonical encode: code=%d err=%v", code, err)
277277
}
278-
f, _ := os.Open(tmpOut)
278+
f, oerr := os.Open(tmpOut)
279+
if oerr != nil {
280+
t.Fatalf("open canonical output: %v", oerr)
281+
}
282+
defer func() { _ = f.Close() }()
279283
if _, err := backup.DecodeSnapshot(f, backup.DecodeOptions{
280284
OutRoot: canonicalIn,
281285
Adapters: backup.AdapterSet{SQS: true},
282286
}); err != nil {
283287
t.Fatalf("canonical decode: %v", err)
284288
}
285-
_ = f.Close()
286289
emitMinimalManifest(t, canonicalIn, lastCommitTS)
287290
return canonicalIn
288291
}
@@ -457,25 +460,27 @@ func TestCLIRoundTripSelfTestAllAdapters(t *testing.T) {
457460
}
458461
}
459462

460-
// TestCLISelfTestMismatchWritesSidecarWithMatchedFalse pins codex P2 v6
461-
// #904: when --self-test detects a mismatch, the CLI MUST still write
462-
// <output>.encode_info.json with self_test.matched=false alongside
463-
// <output>.mismatch.txt. Operators need both files to diagnose a
464-
// failed self-test (sidecar carries SHA256, effective T, adapters).
463+
// TestCLIManifestFloorLeavesNoStaleSidecar pins that the
464+
// manifest-floor preflight failure (--last-commit-ts T < manifest;
465+
// fails in resolveLastCommitTS BEFORE writeAndPublish) leaves NO
466+
// <output>.encode_info.json on disk — neither a fresh one nor a
467+
// stale one from a prior run (the pre-encode cleanup at the start
468+
// of encodeOne removes it).
465469
//
466-
// Driven via --last-commit-ts T < manifest (data-error path) since
467-
// that's the only deterministic CLI-level mismatch trigger; a real
468-
// self-test mismatch needs the same write path. Future cleanup: when
469-
// the library-level corruption hook is exposed via a build-tagged
470-
// CLI test seam, switch to a real self-test mismatch trigger.
471-
func TestCLISelfTestMismatchWritesSidecarWithMatchedFalse(t *testing.T) {
470+
// Note: the test name was previously TestCLISelfTestMismatchWritesSidecarWithMatchedFalse,
471+
// which contradicted the assertion (the encode does NOT run on this
472+
// path, so no sidecar is written). The actual sidecar-on-mismatch
473+
// behavior is now pinned end-to-end by
474+
// TestCLIWriteAndPublishRemovesStaleFSMOnSelfTestMismatch using the
475+
// CLI-level corruption seam (codex P2 v6/v10 #904; claude v12 rename).
476+
func TestCLIManifestFloorLeavesNoStaleSidecar(t *testing.T) {
472477
t.Parallel()
473-
// We can drive a real self-test mismatch path at the library
474-
// level (covered by TestEncodeSnapshotSelfTestDetectsCorruption).
475-
// At the CLI level, we additionally need to confirm the sidecar
476-
// is published on the data-error branch — the manifest-floor
477-
// regression path that exit-2's via the same wrap-then-return
478-
// code that previously skipped writeSidecar.
478+
// The pre-encode cleanup at the top of encodeOne removes any
479+
// stale <output>.encode_info.json before writeAndPublish runs.
480+
// On the manifest-floor path, resolveLastCommitTS exits with
481+
// exit-2 BEFORE that cleanup even runs (it's the second step in
482+
// encodeOne after readInputManifest). So the assertion is: a
483+
// fresh TempDir produces no sidecar at all.
479484
in := t.TempDir()
480485
emitMinimalManifest(t, in, 1000)
481486
out := filepath.Join(t.TempDir(), "out.fsm")

0 commit comments

Comments
 (0)