Skip to content

fix(sessions): compare reply init revisions using persisted shape#96936

Closed
moeedahmed wants to merge 1 commit into
openclaw:mainfrom
moeedahmed:fix/reply-init-persisted-revision
Closed

fix(sessions): compare reply init revisions using persisted shape#96936
moeedahmed wants to merge 1 commit into
openclaw:mainfrom
moeedahmed:fix/reply-init-persisted-revision

Conversation

@moeedahmed

@moeedahmed moeedahmed commented Jun 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

Telegram forum-topic /new and /reset can be accepted by the bot but fail to rotate the topic session. In the affected runtime path, OpenClaw logs repeated reply session initialization conflicted errors and keeps retrying the Telegram update from the ingress spool even though no real concurrent session edit occurred.

Why This Change Was Made

Reply-session initialization uses a guarded revision check. The snapshot side reads the persisted session entry shape from disk, while the commit side may read the live writer cache. Those two representations can be semantically equivalent but structurally different:

  • persisted disk entries never include runtime-only skillsSnapshot.resolvedSkills
  • persisted disk entries can store a large skills prompt as promptRef
  • the live cache can hold hydrated prompt content and runtime-only resolved skill data

The old revision check used raw JSON.stringify(entry), so equivalent persisted/runtime shapes compared as different and produced a false stale-snapshot conflict. This PR compares the canonical persisted shape instead.

User Impact

  • /new and /reset are less likely to get stuck in Telegram topics after skills prompt hydration or runtime skill-cache activity.
  • Spurious Telegram ingress spool retries from false session-init conflicts are avoided.
  • The change stays within the session-store/reply-session boundary and does not alter Telegram command parsing, provider routing, model selection, or user-facing command text.

Evidence

AI-assisted: yes. Hermes investigated the production symptom, implemented the minimal source fix, and verified it locally before opening this PR.

Local validation on this PR branch:

  • node scripts/run-vitest.mjs src/config/sessions/session-accessor.test.ts -t "runtime-only resolvedSkills" — passed, 1 regression test selected
  • node scripts/run-vitest.mjs src/config/sessions/session-accessor.test.ts src/config/sessions/store.skills-stripping.test.ts src/auto-reply/reply/session.test.ts src/config/sessions/store-writer.test.ts — passed, 199 related tests
  • node_modules/.bin/oxfmt --check --threads=1 src/config/sessions/session-accessor.ts src/config/sessions/session-accessor.test.ts src/config/sessions/store-load.ts src/config/sessions/store.ts — passed
  • pnpm tsgo:core — passed
  • pnpm tsgo:core:test — passed

Runtime evidence from the local OpenClaw deployment after applying the same fix and restarting the gateway:

  • health endpoint returned {"ok":true,"status":"live"}
  • gateway connectivity probe was ok
  • running task count was 0
  • Telegram ingress spool files were 0
  • post-fix log scan found 0 reply session initialization conflicted entries and 0 failed spooled updates

Notes for Reviewers

The regression test creates a persisted skills snapshot, takes the reply-session initialization snapshot, mutates the live cached entry with runtime-only resolvedSkills, then verifies commitReplySessionInitialization still commits. The implementation projects both sides to the canonical persisted entry shape before revision comparison, including prompt-ref projection for hydrated prompts.

@openclaw-barnacle openclaw-barnacle Bot added size: S triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jun 26, 2026
@clawsweeper

clawsweeper Bot commented Jun 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks for the context here. I did a careful shell check against current main, and this is already implemented.

Current main already contains the central persisted-shape reply-session revision fix from merged #97657, with matching source, regression coverage, and live before/after proof; this PR now has no meaningful unique user-facing fix left to merge.

So I’m closing this as already implemented rather than keeping a duplicate issue open.

Review details

Best possible solution:

Keep the merged main implementation from #97657 and close this redundant branch rather than merging the remaining comment/export churn.

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

Yes for the original bug: the merged sibling PR documents a live Discord repro with the fix hunk reverted, and current main has a focused regression test for the persisted-versus-runtime skills snapshot mismatch. I did not rerun the live transport flow during this read-only cleanup review.

Is this the best way to solve the issue?

Yes, the solution now on main is the best layer: it normalizes only the shared session-accessor revision comparison instead of adding channel-specific handling. This PR's remaining diff is no longer the best way to solve the issue because the implementation already landed elsewhere.

Security review:

Security review cleared: No concrete security or supply-chain concern was found; the remaining diff only touches session-store comments/tests and an internal export, with no dependencies, workflows, secrets, permissions, or external code execution.

AGENTS.md: found and applied where relevant.

What I checked:

  • Current main source implements the requested comparison: createReplySessionInitializationRevision now serializes projectSessionEntryForPersistenceRevision({ storePath, entry }), so the guard compares the canonical persisted entry shape instead of raw hydrated runtime objects. (src/config/sessions/session-accessor.ts:1167, ff1e7e1305cc)
  • Current main has regression coverage: The session accessor test covers committing reply-session initialization after adding runtime-only skillsSnapshot.resolvedSkills to the live cached entry. (src/config/sessions/session-accessor.test.ts:537, ff1e7e1305cc)
  • Persistence projection normalizes prompt and cache shape: projectSessionEntryForPersistenceRevision strips resolvedSkills and projects large hydrated prompts through the persistence projection before revision comparison. (src/config/sessions/store.ts:351, ff1e7e1305cc)
  • Git blame ties the current implementation to the merged fix commit: Blame for the revision helper, projection helper, and regression test points to commit 75db48a175220b51bfeba79ce2070e7ef29e3b56, the merge commit for the canonical fix. (src/config/sessions/session-accessor.ts:1159, 75db48a17522)
  • Merged PR provenance: fix: prevent stale skill snapshots blocking Discord replies #97657 merged the same persisted-shape revision fix, closed the Discord stale-snapshot issue, and recorded focused tests plus live Discord repro/fix proof. (75db48a17522)
  • Release provenance: No release tag contains 75db48a175220b51bfeba79ce2070e7ef29e3b56; the fix is present on current main only as of 2026-06-29T04:45:50Z. (75db48a17522)

Likely related people:

  • joshavant: Authored and merged the canonical persisted-shape revision fix now blamed on the session accessor, store projection, and regression test lines. (role: current-main fix author; confidence: high; commits: 75db48a17522, 0f68f1292cf7; files: src/config/sessions/session-accessor.ts, src/config/sessions/store.ts, src/config/sessions/store-load.ts)
  • obviyus: Merged the related reply-session initialization serialization fix in the same session-init/runtime path shortly before the persisted-shape fix landed. (role: recent adjacent contributor; confidence: medium; commits: d2da8c79d9b8; files: src/auto-reply/reply/session.ts, src/config/sessions/session-accessor.ts, src/config/sessions/store.ts)
  • VACInc: Authored the merged Telegram retry backoff follow-up for the same reply session initialization conflicted symptom on Telegram lanes. (role: adjacent Telegram retry-path contributor; confidence: medium; commits: 13ecca5408cb; files: extensions/telegram/src/polling-session.ts, extensions/telegram/src/polling-session.test.ts, extensions/telegram/src/telegram-ingress-spool.ts)

Codex review notes: model internal, reasoning high; reviewed against ff1e7e1305cc; fix evidence: commit 75db48a17522, main fix timestamp 2026-06-29T04:45:50Z.

@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. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. P1 High-priority user-facing bug, regression, or broken workflow. labels Jun 26, 2026
@moeedahmed

moeedahmed commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Updated the PR description to OpenClaw contributor format with What Problem This Solves, Why This Change Was Made, User Impact, and Evidence sections, including local validation and runtime proof.

@openclaw-barnacle openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 26, 2026
@clawsweeper clawsweeper Bot added 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 26, 2026
This was referenced Jun 26, 2026
Closed
Closed
Closed
@moeedahmed
moeedahmed force-pushed the fix/reply-init-persisted-revision branch from 526fedb to f511843 Compare June 29, 2026 08:31
@clawsweeper clawsweeper Bot removed merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jun 29, 2026
@clawsweeper

clawsweeper Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper applied the proposed close for this PR.

@clawsweeper clawsweeper Bot closed this Jun 29, 2026
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

mantis: telegram-visible-proof Mantis should capture Telegram visible proof. P1 High-priority user-facing bug, regression, or broken workflow. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@moeedahmed