Skip to content

feat(remote): add archive snapshot provenance#50

Merged
vincentkoc merged 14 commits into
mainfrom
feat/remote-snapshot-provenance
Jul 12, 2026
Merged

feat(remote): add archive snapshot provenance#50
vincentkoc merged 14 commits into
mainfrom
feat/remote-snapshot-provenance

Conversation

@vincentkoc

@vincentkoc vincentkoc commented Jul 12, 2026

Copy link
Copy Markdown
Member

Summary

  • add archive snapshot provenance fields to status, query, ingest, and cutover APIs
  • support atomic snapshot publishing with mutation-token continuation and explicit cutover
  • bind SQLite bundle manifests to the exact source database digest
  • keep source-keyed snapshot manifests deterministic while isolating compressed objects and parts under digest-qualified immutable keys

Tests\n- GOWORK=off go test ./... -count=1\n- GOWORK=off go test -race ./... -count=1\n- GOWORK=off go vet ./...\n- Windows amd64 cross-compile, tidy, formatting, and git diff --check

@clawsweeper

clawsweeper Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codex review: found issues before merge. Reviewed July 12, 2026, 6:01 AM ET / 10:01 UTC.

Summary
The PR adds archive snapshot provenance fields, pinned query and ingest behavior, explicit snapshot cutover, and opt-in content-addressed SQLite bundle publishing.

Reproducibility: not applicable. This PR adds a new remote snapshot lifecycle rather than fixing broken current-main behavior. The earlier source-visible legacy-key compatibility defect is fixed at the current head.

Review metrics: 3 noteworthy metrics.

  • Patch surface: 5 files, 797 additions, 149 deletions. The change materially expands a shared protocol, client API, test suite, persistent bundle representation, and release-owned changelog.
  • Review continuity: 2 earlier blockers fixed, 1 new P3 finding. The opt-in compatibility repairs remain present, while the latest commit reintroduced the previously removed changelog edit.
  • Exported path strategy: 1 opt-in builder and 1 cutover method added. The branch preserves the legacy builder while adding the new snapshot lifecycle as additive API surface.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Post redacted temporary-data terminal output for a released legacy consumer and the snapshot-aware publish, retry, query, and cutover path.
  • Remove the feature-specific CHANGELOG.md entry.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR lists automated checks but no after-change live or downstream consumer run; add redacted terminal output, update the PR body to trigger review, and ask a maintainer for @clawsweeper re-review if it does not rerun automatically.

Risk before merge

  • [P1] The shared remote protocol and immutable object-key scheme are consumed across repositories, but green crawlkit tests do not prove that a released legacy consumer and the new snapshot-aware consumer both work against the proposed API.
  • [P1] The related gitcrawl consumer work remains open, so merging requires either recorded coordinated upgrade proof or an explicit owner decision to accept cross-repository rollout risk.

Maintainer options:

  1. Prove both consumer generations (recommended)
    Run a released legacy reader and the snapshot-aware downstream branch against the proposed API with temporary data before merge.
  2. Own a coordinated rollout
    Merge only if the remote-area owner explicitly accepts synchronizing downstream consumers without recorded compatibility proof.

Next step before merge

  • [P2] The contributor should remove the changelog line and supply real downstream proof; a remote-area owner must then decide whether the compatibility evidence is sufficient for this member-authored draft.

Maintainer decision needed

  • Question: Must this shared protocol wait for recorded smoke proof from both a released legacy consumer and the snapshot-aware gitcrawl branch before merge?
  • Rationale: Repository policy requires downstream compatibility checking for exported API changes, and unit tests in this repository cannot establish cross-repository upgrade behavior.
  • Likely owner: Peter Steinberger — He introduced the current remote API and SQLite bundle surface in the v0.13.4 release commit.
  • Options:
    • Require both consumer smokes (recommended): Request redacted temporary-data output from the released legacy path and the snapshot-aware downstream lifecycle before merging.
    • Accept coordinated rollout risk: Merge without recorded smoke proof only if the remote-area owner will coordinate downstream upgrades and explicitly accepts the compatibility risk.

Security
Cleared: The Go-only API and test changes add no dependency, workflow, permission, secret-handling, publishing, or downloaded-code surface.

Review findings

  • [P3] Remove the feature-branch changelog entry — CHANGELOG.md:6
Review details

Best possible solution:

Preserve the additive APIs and legacy default layout, remove the feature-branch changelog entry, then record temporary-data downstream smoke output covering legacy bundle discovery plus snapshot publish, retry, pinned query, and cutover.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this PR adds a new remote snapshot lifecycle rather than fixing broken current-main behavior. The earlier source-visible legacy-key compatibility defect is fixed at the current head.

Is this the best way to solve the issue?

Yes, the separate opt-in snapshot builder and additive client fields are the narrowest maintainable approach because they retain the released legacy API and key layout. The remaining work is release-note cleanup and upgrade proof, not a competing implementation.

Full review comments:

  • [P3] Remove the feature-branch changelog entry — CHANGELOG.md:6
    CHANGELOG.md is release-owned for this review workflow, so feature PRs should keep release-note context in the PR body rather than editing the unreleased section. This line was added after the previous reviewed head; please drop it again.
    Confidence: 0.99

Overall correctness: patch is correct
Overall confidence: 0.93

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 17d1ba056e14.

Label changes

Label changes:

  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR lists automated checks but no after-change live or downstream consumer run; add redacted terminal output, update the PR body to trigger review, and ask a maintainer for @clawsweeper re-review if it does not rerun automatically.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦪 silver shellfish, so this older rating label is no longer current.
  • remove status: 👀 ready for maintainer look: Current PR status label is status: 📣 needs proof.

Label justifications:

  • P2: This is a bounded shared-library feature with meaningful but limited cross-repository compatibility impact and no current production outage.
  • merge-risk: 🚨 compatibility: Merging changes the shared remote protocol and persistent bundle addressing without recorded proof from both legacy and snapshot-aware consumers.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR lists automated checks but no after-change live or downstream consumer run; add redacted terminal output, update the PR body to trigger review, and ask a maintainer for @clawsweeper re-review if it does not rerun automatically.
Evidence reviewed

What I checked:

  • Additive client protocol: The branch adds snapshot DTOs and a new Cutover method while retaining the existing Query, Ingest, and SQLite upload entry points. (remote/remote.go:188, 5d8ad9db7f08)
  • Legacy layout remains default: Snapshot-addressed bundle generation is exposed through a separate BuildSnapshotGzipSQLiteBundle entry point, leaving BuildGzipSQLiteBundle on the legacy path. (remote/sqlite_bundle.go:92, 5d8ad9db7f08)
  • Release-owned changelog edit: The latest head adds a feature entry directly to the unreleased changelog section after an earlier review cycle had removed that edit. (CHANGELOG.md:6, 5d8ad9db7f08)
  • Current-main provenance: The existing remote contract, client, and SQLite bundle surface was introduced together in the v0.13.4 preparation commit, tying Peter Steinberger to the affected shared API history. (remote/contract.go:1, 216601c0c26b)
  • Repository compatibility policy: The repository requires additive public APIs and downstream compatibility checks for exported API changes; this PR changes shared remote protocol and bundle surfaces. (AGENTS.md:17, 17d1ba056e14)
  • Proof gap: The PR body reports unit, race, vet, cross-compile, tidy, formatting, and diff checks, but includes no redacted live output from a released legacy consumer or the snapshot-aware downstream consumer. (5d8ad9db7f08)

Likely related people:

  • Peter Steinberger: Commit 216601c introduced the current remote contract, HTTP client, and SQLite bundle implementation that this PR extends, and current-main history shows no other merged owner for those files. (role: introduced behavior and likely compatibility decision owner; confidence: high; commits: 216601c0c26b; files: remote/contract.go, remote/remote.go, remote/sqlite_bundle.go)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (3 earlier review cycles)
  • reviewed 2026-07-12T09:11:00.590Z sha 2a5e116 :: needs changes before merge. :: [P1] Keep the snapshot key layout opt-in | [P3] Remove the feature-branch changelog entry
  • reviewed 2026-07-12T09:20:29.645Z sha 1f28a67 :: needs changes before merge. :: [P1] Keep snapshot-scoped bundle keys opt-in | [P3] Remove the feature-branch changelog entry
  • reviewed 2026-07-12T09:51:02.450Z sha cb7e0fe :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jul 12, 2026
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jul 12, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 12, 2026
@vincentkoc
vincentkoc marked this pull request as ready for review July 12, 2026 10:58
@vincentkoc
vincentkoc merged commit a51e186 into main Jul 12, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant