Skip to content

feat: verify refreshed snapshots before publishing - #16

Open
gacevicljubisa wants to merge 12 commits into
mainfrom
feat/verify-snapshot
Open

feat: verify refreshed snapshots before publishing#16
gacevicljubisa wants to merge 12 commits into
mainfrom
feat/verify-snapshot

Conversation

@gacevicljubisa

@gacevicljubisa gacevicljubisa commented Aug 31, 2026

Copy link
Copy Markdown
Member

Adds a verify subcommand and wires it into the Batch Sync workflow as a regression gate: no snapshot reaches a commit or a tag in ethersphere/batch-archive unless it provably extends the archive file it is about to replace.

Why

The workflow resumes the archive snapshot, exports newer entries onto it, then commits and tags the result. Nothing checked that the refreshed file still held everything the old one did — a regression in the resume or gzip-append path could silently drop, mutate, or reorder historical entries and publish that as the new latest version.

What it does

  • batch-export verify --old <file> --new <file> proves the new snapshot strictly extends the old one: the old file's clean content must appear byte for byte at the start of the new file, and every appended entry must parse and advance the (blockNumber, logIndex) order past the old cursor. An empty tail is valid. Plain NDJSON and gzip are detected by content, in any combination.
  • On success it prints only the decimal last block number to stdout and exits 0; on failure stdout is empty and the exit code is non-zero. All logging goes to stderr.
  • The workflow gains a Verify snapshot step between Export and Publish, whose last_block output supplies the commit title. That replaces a gunzip | tail | sed regex which duplicated knowledge of the on-disk JSON format in shell, where a format change would have passed every Go test and broken only in CI.
  • pkg/verify leans on pkg/resume instead of re-deriving the format: ReadLine, ParseEntry, Cursor.Before and Cursor.OpenClean are exported, so the line reader, the entry parser and the (blockNumber, logIndex) ordering rule each exist in one place.

--old points at batch-archive/archive/export.ndjson.gzip — the file Publish overwrites — rather than the resumed tag's file. Those are the same bytes whenever the resumed tag is main's tip, and when an operator resumes an older tag it is the stricter check: it is the archive file a regression would actually destroy.

Also here

  • make test now runs ./... instead of ./pkg/.... The tests pinning the stdout contract (and the pre-existing cmd/export_test.go) were never running in CI.
  • The Publish step's git diff --quiet guard is documented as unable to fire: a resumed gzip export always appends a member, so the file always differs. Behaviour unchanged.

Testing

go test ./..., go vet ./... and golangci-lint are green; actionlint reports only the pre-existing custom bee runner-label warning. pkg/verify covers identical files, a proper superset, a corrupted prefix, a dropped entry, a short file, a duplicate cursor entry, out-of-order and malformed appended lines, an old file with a truncated tail, and all four plain/gzip combinations. The real binary was exercised end to end: stdout is exactly 3\n on success, empty with exit 1 on failure and on a missing required flag.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LqPjhcvFr2LLuBdhyjQwCe

gacevicljubisa and others added 12 commits August 31, 2026 16:12
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014WoaRY6SZi2gUPPmQEhJgr
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014WoaRY6SZi2gUPPmQEhJgr
- workflow: Verify now compares against
  batch-archive/archive/export.ndjson.gzip (the file Publish overwrites)
  instead of the resumed tag, closing the gap where a non-tip archive_tag
  plus a stale finalized head could publish a snapshot that loses entries
  main already has.
- Makefile: `make test` now runs `go test -v ./...` so cmd/verify_test.go
  and cmd/export_test.go run in CI via go.yml, not just pkg/...
- workflow: documented why the Publish step's `git diff --quiet` guard can
  never fire today (resumed gzip export always appends an empty member);
  behavior unchanged, comment only.
- pkg/resume: fixed OpenClean's doc comment (it returns decompressed
  content, not the same raw bytes PrepareOutput copies) and switched its
  two new error messages from "export file" to "resume file" to match the
  rest of the package.
- pkg/verify: comparePrefix now distinguishes a genuinely short new
  snapshot (io.EOF/io.ErrUnexpectedEOF, still ErrMismatch) from a real
  read failure (wrapped as "error reading new snapshot"); both paths stay
  fail-closed. Also documented why checkTail's missing-trailing-newline
  branch is unreachable in practice, without removing it.
- README: verify example now uses the ./dist/ prefix like every other
  command example.
- spec: narrowed the Problem section's claim — the gate does not catch a
  regression confined to the slim NDJSON encoding, since ParseEntry only
  requires blockNumber and logIndex; that encoding is pinned separately by
  pkg/filestore's own tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014WoaRY6SZi2gUPPmQEhJgr
The final review's fix pointed verify --old at the archive file the publish
step overwrites rather than the resumed tag's file; three spec passages and
the plan's Task 6 snippet still described the old behaviour.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014WoaRY6SZi2gUPPmQEhJgr
The verify gate's design and implementation plan lived only to produce the
change; the rationale that outlives them is at its point of use in the code
and the workflow.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LqPjhcvFr2LLuBdhyjQwCe
checkTail had transcribed scanMember's line loop, which is what forced
maxLineBytes to be exported; it now calls resume.ReadLine, and the cap is
unexported again. after() restated Cursor.Skip inverted, so both go through
the new Cursor.Before. parseCursor is renamed ParseEntry, dropping the
pass-through wrapper, and OpenClean becomes a Cursor method so its
precondition is in the signature.

LAST_BLOCK now crosses steps through GITHUB_OUTPUT rather than the job-wide
env, with the empty-value guard the old shell pipeline used to carry, and the
step comments are trimmed to what the code cannot say itself.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LqPjhcvFr2LLuBdhyjQwCe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant