Skip to content

fix stale app-state recovery for chat writes#299

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

fix stale app-state recovery for chat writes#299
TomySpagnoletti wants to merge 20 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 real behavior proof before merge. Reviewed July 17, 2026, 8:04 AM ET / 12:04 UTC.

Summary
The PR adds durable generation-based app-state replay intents, ordered persistence, collection-aware stale-state recovery, SQLite migrations, and regression coverage for chat-state writes.

Reproducibility: yes. Authenticated corrupted-store runs reproduced missing-key and divergent-LTHash failures, and final-head attempts exercised the retained-recovery-debt failure path; only successful final-head completion remains unobserved.

Review metrics: 3 noteworthy metrics.

  • Changed surface: 15 files; +2663/-57. The repair crosses app sequencing, WhatsApp sends, SQLite state, migrations, and tests, so runtime proof matters despite green checks.
  • Stored model: 2 additive migrations. Recovery markers and generation intents persist across upgrades and process restarts.
  • Race stress: 700 focused executions. The repeated race suite directly targets the ordering windows identified during maintainer review.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦐 gold shrimp
Patch quality: 🦞 diamond lobster
Result: blocked until stronger 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:

  • Provide or explicitly override a redacted exact-head authenticated success run showing recovered collection state, persisted remote mutation, matching-generation intent clearing, and the requested archive, mute, or read mutation completing.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: Earlier live output strongly proves the original recovery behavior and final-head output proves failure-safe retained debt, but the required successful corrupted-store recovery did not complete on f0a227d; post redacted exact-head terminal/live output, update the PR body, and ask a maintainer for @clawsweeper re-review if automatic review does not run.

Risk before merge

  • [P1] A successful corrupted-store recovery, recovered-event persistence, matching-intent clearing, and requested write have not all been observed together on final head f0a227d3078dd1055c1d20e1f6aa84b533b8c95a.
  • [P1] The patch intentionally relies on durable replay debt for asynchronous post-send events; merging without the success-path run accepts residual uncertainty in live provider timing that unit and race tests cannot fully settle.

Maintainer options:

  1. Require successful recovery proof (recommended)
    Do not merge until exact-head live output shows recovery and database persistence complete without losing or prematurely clearing replay debt.
  2. Accept the tested residual risk
    Merge only with an explicit maintainer proof override acknowledging that final-head success was not observed against the live provider.

Next step before merge

  • [P1] The branch already contains the maintainer-authored repair; the remaining action is a human proof-sufficiency decision, not another automated code change.

Maintainer decision needed

  • Question: Should this PR wait for a successful exact-head corrupted-store recovery run, or may the remaining runtime proof gap be explicitly waived based on the failure-safe marker evidence, focused race stress, and green full gate?
  • Rationale: The provider disconnected before both final-head success attempts, so code inspection and automated tests cannot determine whether maintainers are willing to accept the remaining external-runtime uncertainty.
  • Likely owner: steipete — They designed and pushed the final repair, ran the independent verification, and documented the exact remaining proof boundary.
  • Options:
    • Wait for successful exact-head proof (recommended): Keep the PR open until a redacted authenticated run on f0a227d3078dd1055c1d20e1f6aa84b533b8c95a completes recovery, persists the fetched mutation, clears the matching intent, and completes the requested write.
    • Grant an explicit proof override: Accept the green full gate, 700 focused race executions, and retained-debt failure proof as sufficient despite the provider-blocked success rerun.

Security
Cleared: The final diff adds local SQLite recovery state and concurrency control but no new dependency, workflow permission, secret access, artifact download, or supply-chain execution path.

Review details

Best possible solution:

Ship the durable replay-intent design after a clean exact-head authenticated run proves successful corrupted-store recovery, persistence of the recovered remote mutation, safe clearing of only the matching generation, and completion of the requested chat write.

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

Yes. Authenticated corrupted-store runs reproduced missing-key and divergent-LTHash failures, and final-head attempts exercised the retained-recovery-debt failure path; only successful final-head completion remains unobserved.

Is this the best way to solve the issue?

Yes. Durable per-generation replay intents and a shared persistence sequencer are narrower and safer than synchronizing correctness to upstream callback-drain timing, and the final branch covers the previously identified collection, persistence, and race failures.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a normal-priority reliability fix for bounded chat-state write failures, with strong code validation and one remaining live-proof decision.
  • merge-risk: 🚨 session-state: A mistake in recovery-intent clearing or event ordering could advance upstream app state while leaving wacli.db stale or misordered.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦞 diamond lobster.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Earlier live output strongly proves the original recovery behavior and final-head output proves failure-safe retained debt, but the required successful corrupted-store recovery did not complete on f0a227d; post redacted exact-head terminal/live output, update the PR body, and ask a maintainer for @clawsweeper re-review if automatic review does not run.
Evidence reviewed

What I checked:

  • Final-head repair design: The final branch reserves durable recovery intent before whatsmeow can advance app state and retains replay debt for delayed asynchronous dispatch rather than depending on temporary-handler timing. (internal/wa/appstate.go:23, f0a227d3078d)
  • Persistent recovery state: The branch adds generation-based recovery-intent storage and additive migrations 21 and 22 so incomplete or delayed persistence remains replayable across process restarts. (internal/store/migrations.go:37, f0a227d3078d)
  • Ordering and race coverage: The final repair covers pre-dispatch removal, delayed post-write dispatch, post-send/live ordering, generation-safe clearing, failed persistence, cancellation, and read-receipt boundaries; the maintainer reports 700 focused race executions and a passing full gate. (internal/app/sync_test.go:933, f0a227d3078d)
  • Repair provenance: The final behavior appears to date to the maintainer repair series from replay-safe recovery through the read-receipt boundary fix, ending at the current PR head. (internal/app/chat_state.go:20, f0a227d3078d)
  • Remaining proof gap: The exact-head authenticated reruns demonstrated failure-safe retained recovery debt, but both attempts disconnected before the app-state information query, so successful corrupted-store recovery and mutation were not observed on the final head. (f0a227d3078d)

Likely related people:

  • steipete: Performed the deep review, identified the dispatch race, and authored the final replay-safety, persistence, sequencing, and race-closing commit series. (role: repair author and likely follow-up owner; confidence: high; commits: e8a29be009ce, cb80c42914e7, 1caa6b032193; files: internal/app/app_state.go, 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 (8 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
  • reviewed 2026-07-13T15:36:42.905Z sha eed6914 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-17T05:04:14.514Z sha cb80c42 :: 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 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
@steipete

Copy link
Copy Markdown
Collaborator

Maintainer review (deep pass with independent verification): the recovery direction is right and the corrupted-store live proof is appreciated, but the current head has an actionable race we can't ship.

The temporary persistence handler can be removed after recovery advances durable whatsmeow app-state, but before asynchronous event dispatch begins. In that window, events that only exist because recovery advanced the state are dropped, which can permanently desynchronize wacli.db from the advanced upstream state. We confirmed the pre-dispatch removal window against upstream FetchAppState/recovery/event-dispatch source.

To land this we need one of: durable replay-marker semantics (events persisted with a marker so a re-run replays anything dispatched after recovery advanced state), or explicit recovery-completion synchronization that keeps the handler installed until dispatch has drained. Plus a repeat of the corrupted-store live proof on the fixed head.

Happy to land quickly once that hole is closed - full test/lint/race gates already pass on this head otherwise.

@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: e8a29be009

ℹ️ 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 on lines +94 to +96
err := a.wa.FetchAppState(ctx, string(collection), false, false)
if err == nil {
return nil

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Propagate catch-up persistence errors

When this normal delta catch-up fetch succeeds after decoding app-state events, the added handler can return a database error (for example while storing an archive/mute/star/delete event), but the tracker is never active around this fetch so record ignores it and the method returns nil. In that failure case whatsmeow has advanced the app-state cursor while wacli.db missed the remote mutation, so a one-shot chat write can still leave local state permanently stale; bracket this fetch with the tracker and check it before returning success.

Useful? React with 👍 / 👎.

Comment on lines +142 to +143
if !errors.Is(fetchErr, appstate.ErrKeyNotFound) {
return fmt.Errorf("replay WhatsApp app state recovery for %s: %w", collection, fetchErr)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Handle LTHash mismatches during full replay

If the initial delta fetch hits ErrMismatchingLTHash, this path enters full replay, but a full replay that also returns ErrMismatchingLTHash immediately exits here without sending RequestAppStateRecovery or waiting for the primary-device snapshot. For stores whose corrupted LTHash cannot be repaired by a server full sync, the persisted recovery marker makes every later archive/mute/read write repeat the same failing full replay and the chat-state command remains blocked.

Useful? React with 👍 / 👎.

@clawsweeper clawsweeper Bot added 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. and removed 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. labels Jul 17, 2026

@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: f0a227d307

ℹ️ 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/wa/appstate.go
// durable recovery intent remains after return because canonical sends
// dispatch their resulting events asynchronously.
beforeApply()
return nil, c.SendAppState(ctx, patch)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Return post-send app-state events

When a one-shot chat-state command runs, there is no sync event handler installed to catch whatsmeow's asynchronous post-send app-state dispatch, and this real client path always returns nil for postSendEvents. That means completeLocalAppStateWrite never persists extra mutations produced by the send's follow-up fetch (for example archive's automatic unpin mutation or conflict catch-up events), so the command can return success while wacli.db remains stale until some later full replay.

Useful? React with 👍 / 👎.

@steipete

Copy link
Copy Markdown
Collaborator

Maintainer repair pushed directly to the contributor branch. No contributor action needed.

Exact head: f0a227d3078dd1055c1d20e1f6aa84b533b8c95a

Design: durable per-generation replay intents, not handler-drain timing. A marker is written before a local app-state write or replay can advance state, fetched/live/local mutations share one persistence sequencer, and only the matching completed generation is cleared. Delayed dispatch and newer generations therefore remain replayable after restart. This is cleaner than coupling correctness to upstream callback timing. Read-self receipts stay synchronous and outside app-state replay because they are not replayable collection events.

Regression coverage includes the pre-dispatch removal window, delayed post-write dispatch, post-send/live ordering, generation-safe clearing, failed persistence retaining debt, request cancellation, and canonical send boundaries. Focused race proof passed 700 executions:

go test -race ./internal/app -run 'Test(MarkChatReadOrdersPreSendReceiptBeforeLocalMutation|ArchiveChatUsesSynchronousFullReplayForMismatch|ArchiveChatOrdersPostSendEventsBeforeNewerLiveEventDuringApply|ArchiveChatReplaysEventDispatchedAfterWriteCompletion|LocalAppStateWriteFinishesAfterRequestCancellation|ReadReceiptDoesNotWaitForAppStateQueue|ReadReceiptDoesNotClearPendingLocalRecoveryIntent)$' -count=100
ok github.com/openclaw/wacli/internal/app

Full exact-head gate passed:

pnpm format:check && pnpm lint && pnpm test && pnpm build && git diff --check && go test -race ./...

Branch AutoReview passed with no findings. Hosted exact-head checks are green: CI and Docker.

Live corrupted-store rerun used the exact clean binary (SHA-256 4433995b36f5b4f410c4f29dbad5f67c581b24a5fd1b65fcf529b0bd6dd21541) and a private authenticated store copy. Connection diagnostics succeeded. Two isolated regular_high attempts started at version 6 with the LTHash replaced by zeros, but the provider closed the websocket before the app-state info query on both the reference and corrupted-copy commands:

websocket disconnected before info query returned response
candidate_version_after=6
candidate_hash_valid=0
recovery_marker_count=1
audit_rows=0
candidate_final_unmuted=1

The commands failed before remote mutation. The failure-path evidence shows the exact-head durable marker remains set and the corrupted local state is not advanced or falsely audited. A second authenticated live store was not available, so successful exact-head recovery remains externally blocked; I am not claiming the earlier success-path proof as final-head proof.

Contributor commits remain intact. CHANGELOG.md now thanks @TomySpagnoletti. Not merged.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants