Skip to content

fix(sessions): align reply init revision with persisted skillsSnapshot#96699

Closed
wiidz wants to merge 1 commit into
openclaw:mainfrom
wiidz:fix/reply-session-skills-snapshot-revision
Closed

fix(sessions): align reply init revision with persisted skillsSnapshot#96699
wiidz wants to merge 1 commit into
openclaw:mainfrom
wiidz:fix/reply-session-skills-snapshot-revision

Conversation

@wiidz

@wiidz wiidz commented Jun 25, 2026

Copy link
Copy Markdown

Summary

  • Compare reply session initialization revisions using the on-disk skillsSnapshot shape (promptRef) instead of hydrated writer-cache prompt text
  • Await inbound session meta recording before updateLastRoute to avoid concurrent session store writes

Fixes #96698

Problem

The second inbound Discord message (and likely any channel) fails with:

reply session initialization conflicted for agent:main:discord:channel:<channelId>

createReplySessionInitializationRevision() serializes the full session entry, but snapshot load and commit compared different skillsSnapshot shapes: persisted { skills, promptRef } vs hydrated { skills, prompt }. This produced false stale-snapshot conflicts after the first agent run.

Test plan

  • Send two consecutive messages in the same Discord channel; both succeed
  • No stale-snapshot / initialization conflict on second turn
  • Existing session store tests pass

Made with Cursor

@wiidz @cursoragent
fix(sessions): align reply init revision with persisted skillsSnapshot
ae5d763 
Compare reply session initialization revisions using the on-disk
skillsSnapshot shape (promptRef) instead of hydrated writer-cache
prompt text, which caused false stale-snapshot conflicts on the second
inbound turn. Also await inbound meta recording before last-route
updates to avoid concurrent session store writes.

Fixes openclaw#96698

Co-authored-by: Cursor <[email protected]>
@openclaw-barnacle openclaw-barnacle Bot added size: XS triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jun 25, 2026
@clawsweeper

clawsweeper Bot commented Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 25, 2026, 4:10 AM ET / 08:10 UTC.

Summary
The PR changes reply-session initialization revision comparison to use persisted skillsSnapshot shape and awaits inbound session metadata recording before updating the last route.

PR surface: Source +20. Total +20 across 2 files.

Reproducibility: yes. at source level: current main compares whole session-entry JSON revisions, and persisted promptRef versus hydrated prompt shapes can differ. I did not run live Discord in this read-only review.

Review metrics: 1 noteworthy metric.

  • Reply-session unhydrated reads: 2 added. Only revision comparison should need persisted shape; unhydrated runtime snapshot reads can change prompt delivery behavior before merge.

Stored data model
Persistent data-model change detected: serialized state: src/config/sessions/session-accessor.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until 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:

  • Keep runtime initialization snapshots hydrated while comparing revisions against a persisted-shape entry.
  • [P1] Add focused regression coverage for externalized skill prompt refs through reply-session initialization.
  • [P1] Add redacted Discord after-fix proof showing two consecutive messages succeed without a stale-snapshot conflict.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body claims local Discord verification but includes no screenshot, recording, terminal output, copied live output, redacted logs, or linked artifact showing the after-fix second-turn behavior. 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.

Risk before merge

  • [P1] Changing loadReplySessionInitializationSnapshot to an unhydrated load can pass promptRef-only skillsSnapshot data into runtime callers; downstream run preparation may then miss the skills prompt.
  • [P1] The PR body describes local Discord verification but includes no screenshot, recording, terminal output, copied live output, redacted logs, or linked artifact showing the after-fix second-turn behavior.
  • [P1] The open Telegram PR overlaps stale-snapshot handling, so maintainers should coordinate behavior, but it does not replace this promptRef hydration fix.

Maintainer options:

  1. Separate revision shape from runtime shape (recommended)
    Load or normalize a persisted-shape entry only for revision comparison while returning hydrated currentEntry and readEntry to reply-session runtime callers.
  2. Accept promptRef-only runtime risk
    Maintainers could intentionally allow promptRef-only runtime snapshots only after proving downstream prompt assembly reconstructs the skills prompt in every affected run path.
  3. Coordinate with Telegram conflict work
    Pause this PR if maintainers want the broader retry/backoff work in fix(telegram): topic replies stall after session conflicts #96550 to land first, then rebase this narrower hydration fix on top.
Copy recommended automerge instruction 
@clawsweeper automerge

Special instructions:
Keep runtime initialization snapshots hydrated; compute persisted/unhydrated skillsSnapshot shape only for the expected-revision comparison in load/commit paths; add focused regression coverage for a promptRef-backed skillsSnapshot.

Next step before merge

  • [P1] The PR needs contributor-side real behavior proof and a code revision; missing external proof makes it human-only rather than an automated repair lane.

Security
Cleared: No concrete security or supply-chain concern found; the diff changes session/channel runtime code without dependencies, workflows, secrets, permissions, or external code execution.

Review findings

  • [P1] Keep runtime session snapshots hydrated — src/config/sessions/session-accessor.ts:1381-1387
Review details

Best possible solution:

Keep runtime initialization snapshots hydrated, compute only the optimistic-concurrency revision from a persisted/unhydrated representation, then add focused regression coverage and redacted Discord proof before merge.

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

Yes at source level: current main compares whole session-entry JSON revisions, and persisted promptRef versus hydrated prompt shapes can differ. I did not run live Discord in this read-only review.

Is this the best way to solve the issue?

No. Persisted-shape revision comparison is the right direction, but this patch also changes runtime snapshot shape; the safer fix is to normalize only the revision while preserving hydrated currentEntry/readEntry behavior.

Full review comments:

  • [P1] Keep runtime session snapshots hydrated — src/config/sessions/session-accessor.ts:1381-1387
    This hunk makes the entire initialization snapshot load unhydrated, so currentEntry and readEntry can carry a promptRef-only skillsSnapshot into runtime reply setup. For prompt blobs, ensureSkillSnapshot can reuse that fresh existing snapshot, and run preparation then calls resolveSkillsPromptForRun without entries; that resolver only reads skillsSnapshot.prompt, so the follow-up run can lose the skills prompt. Compute the revision from persisted shape, but keep runtime snapshot entries hydrated.
    Confidence: 0.87

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P1: The PR targets a broken second inbound Discord message path and the current patch can regress prompt delivery for existing sessions.
  • merge-risk: 🚨 session-state: The diff changes which hydrated versus persisted skillsSnapshot shape crosses the reply-session initialization boundary.
  • merge-risk: 🚨 message-delivery: The diff can let follow-up inbound replies proceed with a session snapshot that lacks the skills prompt used to build agent instructions.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • 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 claims local Discord verification but includes no screenshot, recording, terminal output, copied live output, redacted logs, or linked artifact showing the after-fix second-turn behavior. 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

PR surface:

Source +20. Total +20 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 2 23 3 +20
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 23 3 +20

What I checked:

Likely related people:

  • Shakker: Blame and show history tie the current reply-session initialization, prompt blob hydration, session snapshot, and channel recorder code to commit 365279b. (role: current implementation contributor; confidence: high; commits: 365279b86f47; files: src/config/sessions/session-accessor.ts, src/channels/session.ts, src/config/sessions/store-load.ts)
  • wanglu241: Recent commits touched neighboring session-store pruning/loading behavior and make them a useful secondary routing candidate for session persistence review. (role: adjacent session-store contributor; confidence: medium; commits: fc198d862a80, 875e26e4bbb3; files: src/config/sessions/store-load.ts)
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.

@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. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 25, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 25, 2026
Merged
@CnBarrier404 CnBarrier404 mentioned this pull request Jun 25, 2026
Closed
@clawsweeper clawsweeper Bot mentioned this pull request Jun 26, 2026
Closed
This was referenced Jun 29, 2026
Closed
Closed
@clawsweeper clawsweeper Bot mentioned this pull request Jun 29, 2026
Merged
@joshavant

joshavant commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Thanks @wiidz for both the original issue report and this early fix direction.

The root cause identified here was correct: reply-session initialization was comparing equivalent session entries in different skillsSnapshot representations, causing false stale-snapshot conflicts after prompt blobs were persisted. The canonical fix has now landed in #97657 as 75db48a.

The merged version keeps runtime initialization snapshots hydrated and normalizes only the persisted-shape revision comparison, which addresses the review concern on this PR while fixing the Discord second-message failure. It also includes focused regression coverage and live Discord proof showing the failure with the fix reverted and success with the fix applied.

Closing this PR as superseded by #97657.

@joshavant joshavant closed this Jun 29, 2026
@yaaboo-gif yaaboo-gif mentioned this pull request Jul 1, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P1 High-priority user-facing bug, regression, or broken workflow. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Discord second message fails: false stale-snapshot from skillsSnapshot hydration mismatch

2 participants

@wiidz @joshavant