Skip to content

feat: add opt-in optimized sync retention - #368

Open
theharshith wants to merge 1 commit into
openclaw:mainfrom
theharshith:feat/optimized-sync
Open

feat: add opt-in optimized sync retention#368
theharshith wants to merge 1 commit into
openclaw:mainfrom
theharshith:feat/optimized-sync

Conversation

@theharshith

Copy link
Copy Markdown

Summary

  • add a persisted --optimized sync policy for bounded large-account indexes
  • retain the 100 most-active non-archived chats and 50 newest messages per chat
  • request reduced initial history during pairing and disable automatic media, call, and Status persistence
  • require --confirm before first cleanup of a populated local store

Verification

  • pnpm test
  • pnpm build
  • pnpm format:check
  • pnpm lint
  • git diff --check

Notes

Existing local data is pruned only after explicit optimized activation; no WhatsApp phone history is changed.

@clawsweeper

clawsweeper Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@clawsweeper clawsweeper Bot added merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 session-state 🚨 Merging this PR could lose, corrupt, stale, or mis-associate session or agent state. P2 Normal priority bug or improvement with limited blast radius. 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. labels Aug 24, 2026
@clawsweeper

clawsweeper Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed August 23, 2026, 8:28 PM ET / August 24, 2026, 00:28 UTC.

ClawSweeper review

What this changes

The PR adds an account-persisted optimized-sync policy that bounds retained chats and messages, limits pairing history, and disables selected local persistence by default.

Merge readiness

Blocked until real behavior proof from a real setup is added - 7 items remain

Keep this PR open: it adds a useful opt-in retention feature, but partial tuning silently resets persisted boolean preferences and can subsequently delete call or Status rows. It also needs maintainer product sponsorship and real after-fix behavior proof.

Priority: P2
Reviewed head: e5ac78677b83a00efc1176867aff3fe449cfddbf
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) The implementation has useful scope and tests, but a P1 saved-settings defect and missing real behavior proof prevent merge readiness.
Proof confidence 🧂 unranked krab (1/6) Needs real behavior proof before merge: The PR body lists automated checks but provides no after-fix runtime evidence for activation on a populated store and later retained sync behavior. Please add a redacted terminal transcript, recording, or runtime log, redact private account data, and update the PR body to trigger re-review.
Patch quality 🦐 gold shrimp (3/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: The PR body lists automated checks but provides no after-fix runtime evidence for activation on a populated store and later retained sync behavior. Please add a redacted terminal transcript, recording, or runtime log, redact private account data, and update the PR body to trigger re-review.
Evidence reviewed 5 items Partial policy updates overwrite saved booleans: When any tuning flag is supplied, this assignment writes every boolean from Cobra defaults, even when that boolean flag was not supplied. For example, changing only max chats resets PersistCalls and PersistStatuses to false.
Reset preferences trigger cleanup: The overwritten policy is saved and immediately applied; disabled call or Status persistence deletes the corresponding existing local rows.
Current sync behavior provenance: The current-main live-message path and local-store behavior are attributed to the v0.17.1 base snapshot; the available shallow history limits earlier feature provenance.
Findings 1 actionable finding [P1] Preserve boolean settings during partial tuning
Security None None.

How this fits together

wacli authentication and sync commands receive WhatsApp history and live events, then persist an account-local SQLite index used by CLI search and history. The new policy sits between those incoming events and local message, media, call, and Status storage.

flowchart LR
  A[WhatsApp account] --> B[Auth and sync commands]
  B --> C[Persisted optimization policy]
  C --> D[History and live events]
  D --> E[Retention decision]
  E --> F[SQLite local index]
  F --> G[CLI search and history]
Loading

Decision needed

Question Recommendation
Should wacli own a persisted, destructive account-level optimized-sync retention policy as a core command feature? Sponsor bounded retention in core: Accept the account-level policy direction after the preference-reset defect is fixed and populated-store behavior is proven.

Why: This creates new persisted configuration, cleanup semantics, and a broad CLI surface; code correctness cannot decide whether that is the intended core product boundary.

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The PR body lists automated checks but provides no after-fix runtime evidence for activation on a populated store and later retained sync behavior. Please add a redacted terminal transcript, recording, or runtime log, redact private account data, and update the PR body to trigger re-review.
  • Preserve boolean settings during partial tuning (P1) - Changing only --max-chats reaches this assignment with the other Cobra booleans at defaults, so an existing policy with --persist-calls or --persist-statuses enabled is saved as disabled. The next retention pass then deletes those rows. Update only flags explicitly changed by the user and add a partial-update regression test.
  • Resolve merge risk (P1) - Changing only one tuning value currently overwrites saved call, Status, archived-chat, and eviction preferences; the resulting retention pass can erase call or Status rows.
  • Resolve merge risk (P1) - The feature introduces a persisted, destructive local-retention workflow whose core product scope needs explicit sponsorship.
  • Resolve merge risk (P1) - The PR supplies only claimed automated checks, not an after-fix run showing activation and later retained sync behavior.
  • Complete next step (P2) - A maintainer must sponsor the persistent retention direction, and the contributor must repair the saved-settings defect and add real behavior proof.

Findings

  • [P1] Preserve boolean settings during partial tuning — cmd/wacli/optimized.go:60
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Implementation and test delta production +543/-6, tests +71, docs +34 The feature spans CLI wiring, SQLite migration/storage, sync event handling, and user documentation.

Merge-risk options

Maintainer options:

  1. Preserve untouched saved options (recommended)
    Update policy resolution so each boolean changes only when its corresponding flag was explicitly supplied, with a regression test for partial tuning.
  2. Sponsor the destructive policy
    Explicitly approve the persisted cleanup model only after deciding that this expanded sync configuration belongs in core.

Technical review

Best possible solution:

If maintainers want optimized retention in core, preserve every untouched persisted option, add regression coverage for partial updates, and provide redacted populated-store proof before merging.

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

Yes for the review finding: start with a saved policy that enables calls or Statuses, then pass only --max-chats; the resolver’s default boolean values overwrite those saved settings.

Is this the best way to solve the issue?

No: partial tuning must retain all persisted options whose flags were not supplied, rather than reapplying Cobra defaults to them.

Full review comments:

  • [P1] Preserve boolean settings during partial tuning — cmd/wacli/optimized.go:60
    Changing only --max-chats reaches this assignment with the other Cobra booleans at defaults, so an existing policy with --persist-calls or --persist-statuses enabled is saved as disabled. The next retention pass then deletes those rows. Update only flags explicitly changed by the user and add a partial-update regression test.
    Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 97e14efdf91a.

Labels

Label changes:

  • add P2: This is a normal-priority new retention feature with a concrete saved-settings correctness defect.
  • add merge-risk: 🚨 compatibility: An existing optimized account can have its saved boolean preferences silently replaced by default values.
  • add merge-risk: 🚨 session-state: Those replacement values can cause the retention pass to delete locally persisted call or Status state.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • 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 body lists automated checks but provides no after-fix runtime evidence for activation on a populated store and later retained sync behavior. Please add a redacted terminal transcript, recording, or runtime log, redact private account data, and update the PR body to trigger re-review.

Label justifications:

  • P2: This is a normal-priority new retention feature with a concrete saved-settings correctness defect.
  • merge-risk: 🚨 compatibility: An existing optimized account can have its saved boolean preferences silently replaced by default values.
  • merge-risk: 🚨 session-state: Those replacement values can cause the retention pass to delete locally persisted call or Status state.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body lists automated checks but provides no after-fix runtime evidence for activation on a populated store and later retained sync behavior. Please add a redacted terminal transcript, recording, or runtime log, redact private account data, and update the PR body to trigger re-review.

Evidence

What I checked:

  • Partial policy updates overwrite saved booleans: When any tuning flag is supplied, this assignment writes every boolean from Cobra defaults, even when that boolean flag was not supplied. For example, changing only max chats resets PersistCalls and PersistStatuses to false. (cmd/wacli/optimized.go:60, e5ac78677b83)
  • Reset preferences trigger cleanup: The overwritten policy is saved and immediately applied; disabled call or Status persistence deletes the corresponding existing local rows. (internal/store/chats.go:126, e5ac78677b83)
  • Current sync behavior provenance: The current-main live-message path and local-store behavior are attributed to the v0.17.1 base snapshot; the available shallow history limits earlier feature provenance. (internal/app/sync_events.go:531, 97e14efdf91a)
  • Main and release check: The PR head is one commit beyond current main, which is v0.17.1; no release tag contains the PR head, so this feature is neither implemented on main nor released. (97e14efdf91a)
  • Existing webhook contract: Current documentation defines webhook delivery as successfully stored live messages, confirming that retention changes must preserve deliberate storage semantics. (docs/sync.md:27, 97e14efdf91a)

Likely related people:

  • Peter Steinberger: The available current-main blame assigns the central live-sync and store paths to the v0.17.1 base snapshot authored by Peter Steinberger; older history is shallow. (role: recent sync and storage-area contributor; confidence: medium; commits: 97e14efdf91a; files: internal/app/sync_events.go, internal/store/chats.go, cmd/wacli/sync.go)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Change partial policy updates to preserve every boolean whose flag was not explicitly supplied, and add a regression test.
  • Provide redacted after-fix proof of first activation on a populated store and a subsequent sync run with the saved policy.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • 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.

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. merge-risk: 🚨 session-state 🚨 Merging this PR could lose, corrupt, stale, or mis-associate session or agent state. P2 Normal priority bug or improvement with limited blast radius. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant