Skip to content

fix: apply encrypted message edits during sync - #363

Open
goutamadwant wants to merge 1 commit into
openclaw:mainfrom
goutamadwant:fix/decrypt-encrypted-message-edits
Open

fix: apply encrypted message edits during sync#363
goutamadwant wants to merge 1 commit into
openclaw:mainfrom
goutamadwant:fix/decrypt-encrypted-message-edits

Conversation

@goutamadwant

Copy link
Copy Markdown
Contributor

Summary

  • decrypt SecretEncryptedMessage{MESSAGE_EDIT} envelopes before parsing and persistence
  • validate the decrypted protocol edit and preserve the envelope's target message key
  • cover live sync, history sync, target mismatches, and history ordering

Closes #362

Behavior proof

The regression fixtures seed an original message and then deliver a secret-encrypted edit. Both live sync and history sync now update the original row with the edited body, mark it edited, and keep the database at one row instead of persisting the opaque envelope. A mismatched decrypted target is rejected without changing the original row.

Validation

  • go test ./internal/app -run 'Test(LiveSyncDecryptsSecretMessageEditBeforeStorage|LiveSyncRejectsSecretMessageEditTargetMismatch|HistorySyncDecryptsSecretMessageEditBeforeStorage|HistorySyncEditedMessageSurvivesOlderOriginal|SyncEventHandlerPanicHasStackAndCounter)$' -count=1
  • go test ./internal/wa ./internal/app
  • pnpm format:check
  • pnpm lint
  • pnpm test
  • pnpm build
  • git diff --check

Limitations

No live WhatsApp account was used for validation; behavior proof is provided by deterministic sync fixtures using the project client interface and storage layer.

Disclosure: AI was used to understand the codebase and review the fix.

@clawsweeper

clawsweeper Bot commented Aug 15, 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: 🚨 message-delivery 🚨 Merging this PR could drop, duplicate, misroute, suppress, or wrongly target messages. P1 Urgent regression or broken agent/channel workflow affecting real users now. 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. labels Aug 15, 2026
@clawsweeper

clawsweeper Bot commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed August 15, 2026, 2:46 PM ET / 18:46 UTC.

ClawSweeper review

What this changes

The PR decrypts WhatsApp secret-encrypted edit envelopes during live and history sync, validates their target key, and routes them through existing edit storage.

Merge readiness

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

Keep open: current v0.17.1 still lacks encrypted-edit decryption, and this focused patch follows the established edit-reconciliation path. It needs redacted after-fix proof from a real WACLI/WhatsApp sync before merge because its current evidence is fixture-only.

Priority: P1
Reviewed head: c2253b86c27dbea0c10ca8f1e7af6ea5c2e61290

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The implementation and focused regression coverage are solid, but real behavior proof remains a merge gate.
Proof confidence 🦪 silver shellfish (2/6) Needs real behavior proof before merge: The PR explicitly reports no live WhatsApp account run; its claimed outcomes come from fake-client fixtures and storage tests, so redacted real sync output is still required before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: The PR explicitly reports no live WhatsApp account run; its claimed outcomes come from fake-client fixtures and storage tests, so redacted real sync output is still required before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 6 items Current main lacks the requested decrypt step: The live handler parses an incoming event immediately, and the history handler parses the stored Web message directly; neither path detects or decrypts secret edit envelopes.
Existing edit reconciliation is the correct downstream path: The parser already recognizes ordinary MESSAGE_EDIT protocol messages, applies their key to the parsed message, and marks them edited.
Patch preserves the envelope target before parsing: The branch decrypts only MESSAGE_EDIT envelopes, rejects invalid or mismatched targets, assigns the validated envelope target to the decrypted protocol edit, and returns it to the normal parser.
Findings None None.
Security None None.

How this fits together

WACLI converts WhatsApp live and history sync events into parsed messages and persists them in its local SQLite store. Protocol edit parsing redirects an edit to the original message row, so this change prepares encrypted edit envelopes for that existing reconciliation path.

flowchart LR
A[WhatsApp sync event] --> B[Detect encrypted edit envelope]
B --> C[Decrypt envelope]
C --> D[Validate target key]
D --> E[Parse protocol edit]
E --> F[Reconcile original message row]
F --> G[Stored edited message]
Loading

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The PR explicitly reports no live WhatsApp account run; its claimed outcomes come from fake-client fixtures and storage tests, so redacted real sync output is still required before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Resolve merge risk (P1) - Merging changes how encrypted incoming edits select and overwrite stored message rows; fixture-only validation does not prove that real WhatsApp decrypt results and history events preserve the routing metadata assumed by this path.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production versus regression coverage production +71, tests +162 The added production path is bounded and is covered by more than twice as many focused regression-test lines.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #362
Summary: This PR is a focused candidate fix for the reported encrypted-edit reconciliation failure.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

  1. Require real encrypted-edit proof (recommended)
    Before merge, obtain a redacted WACLI live or history sync run showing the encrypted edit updates the original stored row rather than persisting an opaque envelope.

Technical review

Best possible solution:

Land the narrow decrypt-and-reconcile approach after a redacted real-device sync demonstrates that both live and history encrypted edits update exactly one original row while preserving target-mismatch rejection.

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

Yes, at source level: current main sends these envelopes directly into parsing while ordinary edit reconciliation only receives decrypted protocol edits. The supplied fixtures demonstrate the intended path, but not a real WhatsApp transport run.

Is this the best way to solve the issue?

Yes: decrypting only MESSAGE_EDIT envelopes and then reusing the established protocol-edit parser is the narrowest maintainable solution, with target validation preventing a decrypted payload from retargeting another row.

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

  • add P1: The linked report shows a real sync workflow retaining stale messages and opaque edit rows rather than corrected message content.
  • add merge-risk: 🚨 message-delivery: The patch changes how encrypted inbound edits are targeted and persisted against existing message rows.
  • 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 explicitly reports no live WhatsApp account run; its claimed outcomes come from fake-client fixtures and storage tests, so redacted real sync output is still required before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P1: The linked report shows a real sync workflow retaining stale messages and opaque edit rows rather than corrected message content.
  • merge-risk: 🚨 message-delivery: The patch changes how encrypted inbound edits are targeted and persisted against existing message rows.
  • 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 explicitly reports no live WhatsApp account run; its claimed outcomes come from fake-client fixtures and storage tests, so redacted real sync output is still required before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

  • Current main lacks the requested decrypt step: The live handler parses an incoming event immediately, and the history handler parses the stored Web message directly; neither path detects or decrypts secret edit envelopes. (internal/app/sync_events.go:528, 97e14efdf91a)
  • Existing edit reconciliation is the correct downstream path: The parser already recognizes ordinary MESSAGE_EDIT protocol messages, applies their key to the parsed message, and marks them edited. (internal/wa/messages.go:266, 97e14efdf91a)
  • Patch preserves the envelope target before parsing: The branch decrypts only MESSAGE_EDIT envelopes, rejects invalid or mismatched targets, assigns the validated envelope target to the decrypted protocol edit, and returns it to the normal parser. (internal/app/sync_events.go:833, c2253b86c27d)
  • Regression coverage is focused: Tests cover live and history reconciliation, an edit-before-original history order, and a decrypted-target mismatch without creating a second row. (internal/app/sync_test.go:76, c2253b86c27d)
  • Relevant feature history: History identifies the earlier historical-edit unwrapping work as Mathias Maisberger's commit, while Peter Steinberger is the dominant contributor across the current sync/parser files. (internal/app/sync_events.go:647, 72db4db72ef8)
  • Not yet released: The current release tag v0.17.1 points at current main, while no release tag contains the PR head. (c2253b86c27d)

Likely related people:

  • Peter Steinberger: Current parser and storage lines blame to the v0.17.1 release commit, and feature history shows the most sustained work in the affected sync and parsing files. (role: recent sync-path contributor; confidence: high; commits: 97e14efdf91a, bc55bd7dcef7; files: internal/app/sync_events.go, internal/wa/messages.go, internal/app/sync.go)
  • Mathias Maisberger: Feature history records the earlier 'fix: unwrap edited history messages' change in the same sync/edit area. (role: introduced adjacent history-edit behavior; confidence: medium; commits: 72db4db72ef8; files: internal/app/sync_events.go, internal/app/sync_test.go)

Rank-up moves

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

  • Add a redacted live WACLI sync or history-sync transcript showing the original row becomes edited and no opaque envelope row remains.
  • Update the PR body with that proof; if no fresh review appears, ask a maintainer to comment @clawsweeper re-review.

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: 🚨 message-delivery 🚨 Merging this PR could drop, duplicate, misroute, suppress, or wrongly target messages. P1 Urgent regression or broken agent/channel workflow affecting real users now. 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.

SecretEncryptedMessage{MESSAGE_EDIT} edits are stored as opaque rows instead of being applied

1 participant