Skip to content

fix stale app-state recovery for chat writes#299

Open
TomySpagnoletti wants to merge 4 commits into
openclaw:mainfrom
TomySpagnoletti:fix/app-state-key-recovery
Open

fix stale app-state recovery for chat writes#299
TomySpagnoletti wants to merge 4 commits into
openclaw:mainfrom
TomySpagnoletti:fix/app-state-key-recovery

Conversation

@TomySpagnoletti

@TomySpagnoletti TomySpagnoletti commented Jul 13, 2026

Copy link
Copy Markdown

Summary

  • catch up the collection used by each chat-state write (regular_low for archive, pin, and read state; regular_high for mute)
  • persist app-state events consumed by one-shot catch-up through the same local handlers used by sync
  • keep the linked-device connection alive while the primary device shares missing keys
  • request the primary-device recovery snapshot once when a delta has a mismatching LTHash
  • use context-bounded exponential backoff (250 ms to 5 s) while waiting, avoiding WhatsApp rate limits
  • cover missing-key recovery, LTHash recovery, collection routing, event persistence, and retry backoff with regression tests

Root cause

whatsmeow requests missing app-state keys asynchronously when decoding a patch fails with appstate.ErrKeyNotFound. Chat-state commands previously proceeded directly to SendAppState, so a stale collection could receive a 409 conflict and the one-shot process could exit before the primary device delivered the key.

Live validation exposed a second state of the same failure path: the local collection can have the expected version but a divergent LTHash. A normal delta cannot repair that condition; the primary device must provide its recovery snapshot. That response can take more than two minutes. Retrying every 250 ms while waiting also reproduced a server 429 rate-overlimit.

The shared pre-write helper now targets the collection used by each mutation, waits for missing keys, requests one official recovery snapshot on an LTHash mismatch, and backs retries off to a five-second cap. During catch-up it reuses the same app-state persistence handlers as normal sync, so decoded remote mutations are not consumed without updating wacli.db.

Redacted live proof

The test used a private temporary copy of an authenticated store. Phone numbers, JIDs, request IDs, endpoints, and key IDs are redacted. The temporary stores were deleted after verification.

Missing-key delivery and connection retention

Nine recent app-state keys were deliberately removed from the temporary copy while its collection remained at version 87:

regular_low             87
sync_keys_before        41
keys_removed             9
sync_keys_after         32

The one-shot process remained connected for 120.90 seconds. Before it exited on the independently discovered LTHash mismatch, the primary device had restored eight requested keys:

regular_low             87
sync_keys_after_run     40
terminal condition      mismatching LTHash
real                    120.90 s

This demonstrates that the command no longer exits before asynchronous key delivery. It also provided the live reproduction for the recovery-snapshot addition.

Recovery snapshot and successful mutation

With one recovery request and exponential backoff enabled, the same stale version-87 store waited for the primary-device snapshot and completed the native archive command:

$ wacli ... chats archive --chat <redacted>
{"success":true,"data":{"action":"archive","chat":"<redacted>","ok":true},"error":null}
real 162.21
user 0.14
sys 0.11

regular_low             89
archived                 1

No 429 occurred. An immediate native unarchive then succeeded and left the test chat in its requested final state:

$ wacli ... chats unarchive --chat <redacted>
{"success":true,"data":{"action":"unarchive","chat":"<redacted>","ok":true},"error":null}
real 1.63
user 0.02
sys 0.02

regular_low             90
archived                 0

Propagation of the archive path was also manually confirmed in both WhatsApp Desktop and WhatsApp for iPhone.

Final-head regular_high recovery and persistence

This test ran the exact clean final head and used only a private temporary copy of the authenticated store:

binary_version          0.12.1
vcs.revision            eed6914f7d2f82c4955ed13106ccdd1fb6f31f3c
vcs.modified            false

The temporary copy started from regular_high version 3. Its 128-byte LTHash was deliberately replaced with zeros, while the redacted test chat was locally unmuted. A temporary SQLite trigger, scoped to that chat, recorded every actual muted_until transition:

temporary_regular_high  3
temporary_hash_prefix   0000000000000000
temporary_archived      0
temporary_muted_until   0
audit_rows_before       0

A reference mute was first written from the main store:

$ wacli ... chats mute --chat <redacted>
{"success":true,"data":{"action":"mute","chat":"<redacted>","ok":true},"error":null}
real                    1.61 s
main_regular_high       6

The final-head binary then ran an unmute from the corrupted version-3 copy. It recovered regular_high, persisted the fetched remote mute event, and completed the unmute:

$ wacli --store <temporary-redacted-store> ... chats unmute --chat <redacted>
{"success":true,"data":{"action":"unmute","chat":"<redacted>","ok":true},"error":null}
real                    2.85 s

temporary_regular_high  7
temporary_hash_valid    true
temporary_archived      0
temporary_muted_until   0

audit_seq  old_muted  new_muted
1          0          -1
2         -1           0

Because the requested command was unmute, the first 0 -> -1 transition can only be the remote mute event emitted during pre-write recovery and persisted by the temporary handler. The second -1 -> 0 transition is the requested local unmute. This demonstrates both final-head regular_high recovery and wacli.db persistence before the write.

A final idempotent unmute reconciled the main store and left the requested user-visible state:

main_regular_high       8
final_archived          0
final_muted_until       0

The authenticated temporary store, audit database, safety backup, and generated build artifact were deleted after verification.

Validation

Passed:

  • go test ./internal/app -count=1
  • go test ./... -count=1
  • go test -tags sqlite_fts5 ./... -count=1
  • pnpm format:check
  • pnpm lint
  • pnpm build
  • git diff --check

pnpm test also completed its Go, sqlite_fts5, Windows-lock, and CGO-required stages successfully. Its documentation/release-workflow stage passed 40/41 tests; the sole failure was the known local endpoint-protection block on the temporary Windows a.out.exe (operation not permitted).

@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. P2 Normal priority bug or improvement with limited blast radius. labels Jul 13, 2026
@clawsweeper

clawsweeper Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 13, 2026, 11:36 AM ET / 15:36 UTC.

Summary
The PR adds collection-aware pre-write app-state recovery with bounded backoff and recovery-snapshot requests, persists catch-up events through existing handlers, and adds focused regression coverage.

Reproducibility: yes. The contributor reproduced missing-key and mismatching-LTHash failures with deliberately stale authenticated store copies, then repeated the recovery and persistence path on the exact clean final SHA.

Review metrics: 3 noteworthy metrics.

  • Patch surface: 4 files, +316/-11. The change is concentrated in app-state write handling and tests rather than spreading across CLI wiring, store schema, or dependencies.
  • Regression coverage: 230 test lines added. Most of the patch is focused coverage for collection routing, persistence, recovery, cleanup, and retry behavior.
  • Production code: 81 additions, 11 deletions across 2 files. The runtime change is bounded to chat-state recovery and shared app-state persistence dispatch.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
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:

  • none.

Risk before merge

  • [P1] The patch intentionally advances and persists WhatsApp app-state during one-shot write commands; incorrect event ordering or handler-lifecycle assumptions could stale or mis-associate local chat state even when unit tests pass.
  • [P1] The live proof covers archive/unarchive and mute/unmute on one authenticated account, but does not independently exercise every regular_low mutation such as pin and mark-read against a corrupted live store.

Maintainer options:

  1. Merge after focused app-state review (recommended)
    Accept the patch once a maintainer confirms that temporary handler installation and removal cannot miss or double-apply events emitted during FetchAppState.
  2. Request broader live mutation proof
    Ask for one additional redacted corrupted-store run covering pin or mark-read if live coverage beyond archive and mute collections is required.
  3. Pause for handler-order confirmation
    Hold the PR if maintainers cannot establish that fetched events finish dispatching before the temporary handler is removed.

Next step before merge

  • No automated repair remains; prior findings are addressed and final-head proof is sufficient, leaving an ordinary maintainer merge decision.

Security
Cleared: The diff adds no dependency, workflow, secret, permission, artifact-download, or new code-execution surface and remains within existing WhatsApp app-state and SQLite persistence paths.

Review details

Best possible solution:

Land the focused recovery and persistence changes after normal maintainer review confirms the temporary handler lifecycle, while retaining the collection-specific routing and regression coverage.

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

Yes. The contributor reproduced missing-key and mismatching-LTHash failures with deliberately stale authenticated store copies, then repeated the recovery and persistence path on the exact clean final SHA.

Is this the best way to solve the issue?

Yes. A shared collection-aware pre-write catch-up that reuses established persistence handlers is narrower and safer than duplicating recovery logic in each command or proceeding with writes against stale app-state.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. Redacted exact-head terminal and SQLite audit output shows after-fix regular_high recovery, remote-event persistence before the local write, successful mutation, and final-state reconciliation.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): Redacted exact-head terminal and SQLite audit output shows after-fix regular_high recovery, remote-event persistence before the local write, successful mutation, and final-state reconciliation.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.
  • remove rating: 🦐 gold shrimp: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

  • P2: This fixes a real but bounded chat-state write failure involving stale WhatsApp app-state, with focused code and live proof.
  • merge-risk: 🚨 session-state: Merging changes when app-state cursors advance and when remote archive, pin, mute, and read-state events are persisted during one-shot writes.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): Redacted exact-head terminal and SQLite audit output shows after-fix regular_high recovery, remote-event persistence before the local write, successful mutation, and final-state reconciliation.
  • proof: sufficient: Contributor real behavior proof is sufficient. Redacted exact-head terminal and SQLite audit output shows after-fix regular_high recovery, remote-event persistence before the local write, successful mutation, and final-state reconciliation.
Evidence reviewed

What I checked:

  • Final-head collection routing: Archive, pin, and read-state writes catch up regular_low, while mute and unmute catch up regular_high before sending their app-state mutation. (internal/app/chat_state.go:20, eed6914f7d2f)
  • Persistence path reuse: Normal sync and temporary pre-write catch-up share handleAppStatePersistenceEvent for app-state-derived events, preventing fetched deltas from advancing the cursor without updating wacli.db. (internal/app/sync_events.go:93, eed6914f7d2f)
  • Regression coverage: The final patch tests regular_high mute and unmute recovery, mismatching-LTHash recovery requests, pre-write event persistence, non-chat app-state persistence, handler cleanup, and retry behavior. (internal/app/sync_test.go:935, eed6914f7d2f)
  • Exact-head real behavior proof: The PR body identifies vcs.revision eed6914 with vcs.modified=false and shows a deliberately corrupted regular_high LTHash recovering from version 3 to 7 before a successful unmute. (eed6914f7d2f)
  • Fetched-event persistence proof: The redacted SQLite audit records the remote mute transition from 0 to -1 before the requested local unmute transition from -1 to 0, directly demonstrating persistence during final-head catch-up. (eed6914f7d2f)
  • Current-main comparison: The supplied PR metadata compares final head eed6914 against current main b1eb335 as a four-file patch, so the requested fix is not already implemented on main. (internal/app/chat_state.go:20, b1eb33538df3)

Likely related people:

  • dinakars777: GitHub surfaced this person in the review timeline when the app-state patch became ready for maintainer review, making them a plausible routing candidate for the affected internal/app behavior; deeper local provenance was unavailable in this review environment. (role: likely app-state area reviewer; confidence: low; files: internal/app/chat_state.go, internal/app/sync_events.go, internal/app/sync_test.go)
  • steipete: GitHub surfaced this person alongside the app-state review routing, making them a plausible candidate for accepting a session-state-sensitive behavioral fix; the available context does not establish file-level ownership. (role: repository-level review candidate; confidence: low; files: internal/app/chat_state.go, internal/app/sync_events.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 (6 earlier review cycles)
  • reviewed 2026-07-13T09:54:29.685Z sha 896a2ed :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-13T10:46:13.929Z sha 19c73fd :: needs changes before merge. :: [P3] Remove the PR-owned changelog entry
  • reviewed 2026-07-13T10:58:25.938Z sha 6ca72ee :: needs maintainer review before merge. :: none
  • reviewed 2026-07-13T11:12:12.510Z sha 6ca72ee :: needs changes before merge. :: [P2] Sync the mute app-state collection before muting | [P1] Persist deltas consumed by pre-write catch-up
  • reviewed 2026-07-13T14:08:05.348Z sha eed6914 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-13T14:22:51.130Z sha eed6914 :: needs real behavior proof before merge. :: none

@TomySpagnoletti TomySpagnoletti changed the title fix chat state writes with missing app-state keys fix stale app-state recovery for chat writes Jul 13, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

Added redacted live runtime proof for missing-key delivery, the primary-device recovery snapshot, bounded backoff without a 429, and successful archive/unarchive mutations. The branch now includes focused regression coverage for all three paths.

@clawsweeper

clawsweeper Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. 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: 🦪 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 Jul 13, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

Removed the PR-owned CHANGELOG.md entry as requested. The release-note context remains in the PR body; the functional patch and validation are unchanged.

@clawsweeper

clawsweeper Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@TomySpagnoletti TomySpagnoletti marked this pull request as ready for review July 13, 2026 10:55

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6ca72ee981

ℹ️ 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".

Comment thread internal/app/chat_state.go Outdated
Comment thread internal/app/chat_state.go Outdated
@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. merge-risk: 🚨 session-state 🚨 Merging this PR could lose, corrupt, stale, or mis-associate session or agent state. 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 13, 2026

Copy link
Copy Markdown
Author

@codex review

Addressed both findings in eed6914:

  • chat-state catch-up is now collection-aware: archive, pin, and read state use regular_low; mute and unmute use and recover regular_high
  • one-shot catch-up installs a temporary handler that reuses the same app-state persistence path as normal sync, then removes it before the mutation write
  • focused regressions cover mute/unmute LTHash recovery, chat-state delta persistence, non-chat app-state persistence, and handler cleanup

Validation passes for standard and sqlite_fts5 Go tests, formatting, vet, build, and diff checks. The only aggregate pnpm test failure is the documented local endpoint-protection block on its temporary Windows a.out.exe.

Copy link
Copy Markdown
Author

@clawsweeper re-review

The two app-state review findings are addressed in eed6914: collection-aware recovery routes mute/unmute through regular_high, and pre-write catch-up now reuses the normal app-state persistence path so fetched deltas update wacli.db. Focused regression coverage and the PR validation section were updated.

@clawsweeper

clawsweeper Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 👍

Reviewed commit: eed6914f7d

ℹ️ 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".

@clawsweeper clawsweeper Bot removed the proof: sufficient Contributor real behavior proof is sufficient. label Jul 13, 2026
@clawsweeper clawsweeper Bot added status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jul 13, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

Added redacted final-head live proof for the exact clean SHA eed6914.

The new PR-body section demonstrates:

  • forced regular_high recovery from version 3 with a deliberately invalid 128-byte LTHash to version 7 with a valid hash
  • successful final-head unmute after recovery
  • SQLite-audited persistence of the fetched remote mute event (0 -> -1) before the requested local unmute (-1 -> 0)
  • final main-store reconciliation at regular_high version 8 with the chat unarchived and unmuted

All authenticated temporary stores, the audit database, safety backup, and generated build artifact were deleted after verification.

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. 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: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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. proof: sufficient Contributor real behavior proof is sufficient. 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.

1 participant