feat: verify refreshed snapshots before publishing - #16
Open
gacevicljubisa wants to merge 12 commits into
Open
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014WoaRY6SZi2gUPPmQEhJgr
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a
verifysubcommand and wires it into the Batch Sync workflow as a regression gate: no snapshot reaches a commit or a tag inethersphere/batch-archiveunless 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.Verify snapshotstep between Export and Publish, whoselast_blockoutput supplies the commit title. That replaces agunzip | tail | sedregex 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/verifyleans onpkg/resumeinstead of re-deriving the format:ReadLine,ParseEntry,Cursor.BeforeandCursor.OpenCleanare exported, so the line reader, the entry parser and the(blockNumber, logIndex)ordering rule each exist in one place.--oldpoints atbatch-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 testnow runs./...instead of./pkg/.... The tests pinning the stdout contract (and the pre-existingcmd/export_test.go) were never running in CI.git diff --quietguard 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 ./...andgolangci-lintare green;actionlintreports only the pre-existing custombeerunner-label warning.pkg/verifycovers 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 exactly3\non success, empty with exit 1 on failure and on a missing required flag.🤖 Generated with Claude Code
https://claude.ai/code/session_01LqPjhcvFr2LLuBdhyjQwCe