feat: add durable backup history#11
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed June 19, 2026, 6:57 AM ET / 10:57 UTC. Summary Reproducibility: not applicable. this is a feature PR rather than a current-main bug report. The changelog finding is source-reproducible from the PR diff. Review metrics: 3 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Land the backup history implementation after removing the Do we have a high-confidence way to reproduce the issue? Not applicable; this is a feature PR rather than a current-main bug report. The changelog finding is source-reproducible from the PR diff. Is this the best way to solve the issue? Mostly yes: the implementation path is coherent and covered by focused tests, but it is not merge-ready until the release-owned changelog hunk is removed and real backup-run proof is added. Full review comments:
Overall correctness: patch is correct AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning high; reviewed against c0b341495848. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cea247977b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| tag, err := tagSnapshot(ctx, cfg, opts.Tag) | ||
| if err != nil { | ||
| return Result{}, err |
There was a problem hiding this comment.
Preflight tag conflicts before committing snapshots
When the requested tag already exists on a different commit and this push produces a new snapshot, the immutable-tag error is discovered only here, after writeSnapshot and commitAndPush have already advanced the backup repo's HEAD. The command returns an error but leaves that untagged commit as the current backup, so a later unqualified backup pull or backup snapshots can use a snapshot from a failed backup push --tag .... Check for an existing conflicting tag before writing/committing, or roll back the local commit on this error path.
Useful? React with 👍 / 👎.
|
Landed as Proof before merge:
Post-merge checkout: |
Summary
backup pull --refrestores without changing the backup checkoutThe archive schema, backup manifest JSON shape, current restore behavior, and existing CLI JSON contracts remain unchanged.
Durability
--no-pushusable when the configured remote does not yet existProof
GOWORK=off go test -count=1 ./...GOWORK=off go test -race -count=1 ./...GOWORK=off go vet ./..../scripts/coverage.sh 35.0(56.1%)No release or tag requested.