Skip to content

feat: add safe hybrid local archive collection#139

Closed
hannesrudolph wants to merge 7 commits into
openclaw:mainfrom
hannesrudolph:codex/discrawl-hybrid-upstream-20260717
Closed

feat: add safe hybrid local archive collection#139
hannesrudolph wants to merge 7 commits into
openclaw:mainfrom
hannesrudolph:codex/discrawl-hybrid-upstream-20260717

Conversation

@hannesrudolph

@hannesrudolph hannesrudolph commented Jul 18, 2026

Copy link
Copy Markdown
Member

Summary

  • add native hybrid collection so an upstream Git snapshot can be refreshed into one local archive while preserving destination-only private rows
  • keep the broader local bot incremental through Gateway tail and bounded repair instead of repeatedly downloading complete history
  • apply channel/category message exclusions consistently across Discord sync, tail, desktop-cache import, coverage, and snapshot merge paths
  • harden writer locking and reconcile deleted/excluded/unknown-message update races
  • document the hybrid configuration and operational workflow

Safety properties

  • local-only private channels and DMs remain local and are not added to the upstream snapshot
  • routine upstream refreshes are non-forced and import canonical changes while preserving destination-only rows
  • excluded feed-channel message traffic is blocked across every ingestion and merge path while channel metadata remains available
  • writer operations are guarded after lock acquisition to prevent concurrent archive mutation

Validation

  • go test ./...
  • go test -race ./...
  • go vet ./...
  • golangci-lint, staticcheck, deadcode, and gosec
  • module tidy/verify/read-only checks
  • two byte-identical normal-checkout darwin/arm64 static builds: 37e13b24fb4f1c286da184c52554778243b3c2f600eb81d65e7c7b077bacf597
  • embedded vcs.revision=d7c07e25e60b7749c4a47de72349830099c52c1a and vcs.modified=false
  • credential-free isolated CLI smoke without touching live Discrawl state

Qualified against upstream 1d1b8a0a61ac060fc3d72a30221f32672b8e31dc.

@hannesrudolph
hannesrudolph requested a review from a team as a code owner July 18, 2026 00:12
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. 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 18, 2026
@clawsweeper

clawsweeper Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 18, 2026, 2:28 AM ET / 06:28 UTC.

Summary
The PR adds hybrid upstream-snapshot/local archive collection, channel-message exclusions across sync paths, tail-repair controls, post-lock resource guards, and related tests and documentation.

Reproducibility: not applicable. This PR proposes a new hybrid collection mode rather than reporting broken existing behavior. The supplied validation describes automated checks and an isolated CLI smoke, but no bug reproduction is required for this classification.

Review metrics: 2 noteworthy metrics.

  • Patch surface: 48 files affected; 5,541 additions and 484 deletions. The feature crosses configuration, collection, merge, storage, CLI, documentation, and test surfaces, so contract approval matters more than a narrow code-only review.
  • Automated checks: 9 successful check runs. The supplied PR state shows broad automated validation, while product and upgrade acceptance remain a separate maintainer decision.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🌊 off-meta tidepool
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

  • [P2] Record maintainer approval of the hybrid archive and upgrade contract before merge.

Risk before merge

  • [P1] Merging establishes a compatibility-sensitive contract for when archive refreshes preserve local rows, skip excluded channel traffic, and intentionally switch to destructive exact reconciliation under --force.
  • [P1] The new exclusions and hybrid update modes span Discord API sync, desktop cache import, tail repair, and snapshot merge, so maintainers should explicitly own the documented consistency and upgrade semantics before landing.

Maintainer options:

  1. Approve the preservation contract before merge (recommended)
    Document and explicitly accept the compatibility promise for safe updates, exclusions, and --force, then merge against that stable contract.
  2. Accept the broader operational surface
    Maintainers may intentionally accept the multi-path archive behavior as core functionality, including the associated configuration and upgrade responsibility.
  3. Pause for narrower product scope
    Defer the PR if Discrawl should not yet commit to hybrid archive persistence semantics in core.

Next step before merge

  • [P2] A maintainer must decide whether the hybrid archive behavior and its compatibility guarantees are supported core product direction; there is no narrow mechanical repair to dispatch first.

Maintainer decision needed

  • Question: Should Discrawl core support a native hybrid archive mode that combines canonical Git snapshots with locally retained private/archive rows and cross-path message exclusions?
  • Rationale: This is a new user-facing persistence and operational model rather than a repair to an established contract; choosing its supported guarantees, upgrade expectations, and core scope requires maintainer intent.
  • Likely owner: hannesrudolph — They authored the integrated feature series and are the clearest available routing candidate for clarifying the intended contract with repository decision-makers.
  • Options:
    • Sponsor a defined core contract (recommended): Approve hybrid collection as a core workflow after recording the guarantees for safe merge, exclusions, local-row preservation, and forced replacement.
    • Narrow the feature: Request a smaller first slice, such as documented safe snapshot preservation or collection exclusions, before accepting the full hybrid operational model.
    • Defer core adoption: Keep the PR open while maintainers decide whether this workflow should remain an external/operator pattern rather than a supported core mode.

Security
Cleared: The supplied diff summary adds no workflow, action, package-source, secret-handling, or third-party execution change; the dependency-file edit only changes the directness classification of an existing SQLite module.

Review details

Best possible solution:

Adopt a concise, maintainer-approved hybrid-archive contract that defines safe-update preservation, exclusion precedence, and --force behavior, then land this implementation only if that contract belongs in Discrawl core.

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

Not applicable: this PR proposes a new hybrid collection mode rather than reporting broken existing behavior. The supplied validation describes automated checks and an isolated CLI smoke, but no bug reproduction is required for this classification.

Is this the best way to solve the issue?

Unclear: the implementation is internally focused and has no discrete blocking code finding in the available review record, but whether native hybrid archive persistence is the right supported core contract requires maintainer direction.

AGENTS.md: unclear because the file could not be read completely.

Codex review notes: model internal, reasoning high; reviewed against 15af97146636.

Label changes

Label changes:

  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • remove rating: 🦐 gold shrimp: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

  • P2: This is a substantial new archive workflow with bounded user impact, not an emergency regression.
  • merge-risk: 🚨 compatibility: The PR adds persisted configuration and changes safe-update versus forced-replacement behavior for existing local archives.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: Not applicable to this MEMBER-authored PR; the body nevertheless reports real isolated CLI smoke validation alongside automated checks.
Evidence reviewed

What I checked:

  • Feature scope: The proposed head adds configuration, CLI flags, safe snapshot-merge filtering, desktop-cache filtering, gateway-tail behavior, and documentation for hybrid collection semantics. (internal/config/config.go:51, d7c07e25e60b)
  • Compatibility boundary: The PR explicitly changes how routine safe updates preserve local rows while omitting excluded message-scoped snapshot rows, with forced update retaining separate exact-reconciliation behavior. (internal/share/share.go:65, d7c07e25e60b)
  • Prior review continuity: The prior completed ClawSweeper review at this same head reported no discrete code findings and identified the remaining blocker as a human choice of supported product and compatibility contract. (internal/share/merge.go:192, d7c07e25e60b)
  • Current PR state: The hydrated GitHub state reports an open, cleanly mergeable PR with 48 changed files, seven commits, and successful analyze, dependency, Docker, lint, release, secret-scan, test, and CodeQL checks; it is not merged into the supplied current main SHA. (d7c07e25e60b)
  • Implementation provenance: The behavior appears to have been introduced by the hybrid collection commit and refined through six focused follow-up commits, ending with unknown-message update reconciliation. (internal/syncer/tail.go:13, 3eff325e1c6c)

Likely related people:

  • hannesrudolph: The available provenance shows authorship of all seven commits implementing and hardening the hybrid collection, merge, and tail behavior; no independent current-main ownership trail was available in the hydrated review data. (role: current feature integration author; confidence: medium; commits: 3eff325e1c6c, 9612267f7c82, d7c07e25e60b; files: internal/share/share.go, internal/share/merge.go, internal/syncer/tail.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 (2 earlier review cycles)
  • reviewed 2026-07-18T00:14:21.787Z sha d7c07e2 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-18T00:24:23.886Z sha d7c07e2 :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jul 18, 2026
@clawsweeper

clawsweeper Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper status: review started.

I am starting a fresh review of this pull request: feat: add safe hybrid local archive collection This is item 1/1 in the current shard. Shard 0/1.

This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.

@steipete

Copy link
Copy Markdown
Collaborator

Closing because main already has hybrid collection. This 48-file monolith combines exclusions, tail scheduling, writer guards, embeddings, CLI/config, and privacy behavior without changelog coverage or live boundary proof.

Please send only independently reproduced, bounded bugs as separate focused PRs with targeted proof.

@steipete steipete closed this Jul 18, 2026
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: 🐚 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants