Skip to content

fix: prevent stale skill snapshots blocking Discord replies#97657

Merged
joshavant merged 1 commit into
mainfrom
fix/96698-reply-init-persisted-revision
Jun 29, 2026
Merged

fix: prevent stale skill snapshots blocking Discord replies#97657
joshavant merged 1 commit into
mainfrom
fix/96698-reply-init-persisted-revision

Conversation

@joshavant

Copy link
Copy Markdown
Contributor

Closes #96698

What Problem This Solves

Fixes an issue where users sending a second Discord message in the same session could receive no reply when the first turn persisted a large skills snapshot as a prompt blob.

Why This Change Was Made

The reply-session initialization guard now compares the canonical persisted session-entry shape instead of the live hydrated runtime object. That keeps real stale-session protection intact while ignoring equivalent representation differences such as hydrated skillsSnapshot.prompt text and runtime-only resolvedSkills cache entries.

This change stays in the shared session file-backend boundary rather than adding Discord-specific handling, because the false conflict is caused by session snapshot revision comparison.

User Impact

Discord users can continue an existing agent conversation after a skills snapshot is persisted as a prompt blob. Other channel/session users benefit from the same shared guard behavior, and true concurrent durable session changes still reject as stale.

AI-assisted change.

Evidence

  • Autoreview: .agents/skills/autoreview/scripts/autoreview --mode local completed cleanly with no accepted/actionable findings before commit.
  • Focused test after rebase: node scripts/run-vitest.mjs src/config/sessions/session-accessor.test.ts passed: 1 file, 59 tests.
  • Fix proof, live Discord via Crabbox Azure: lease cbx_11c087803ec6, run run_f4d2ea8c6456.
    • OPENCLAW_TEST_FAST_CHILD=0
    • first Discord reply succeeded
    • session persisted skillsSnapshot.promptRef
    • second Discord reply succeeded
    • FIX_RESULT=passed-promptRef-present-second-replied
  • Regression repro, same live Discord scenario with the fix hunk reverted via Crabbox Azure: lease cbx_c94f033b0224, run run_ff0d0c251602.
    • OPENCLAW_TEST_FAST_CHILD=0
    • first Discord reply succeeded
    • session persisted skillsSnapshot.promptRef
    • second Discord reply was missing with stale-snapshot conflict
    • REPRO_RESULT=reproduced-promptRef-present-second-missing

Note: an initial focused test command with --runInBand failed before tests ran because that Vitest option is unsupported by the repo wrapper; it was rerun with the supported command above and passed.

@openclaw-barnacle openclaw-barnacle Bot added size: S maintainer Maintainer-authored PR labels Jun 29, 2026
@clawsweeper

clawsweeper Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 29, 2026, 12:41 AM ET / 04:41 UTC.

Summary
The PR changes reply-session initialization revision comparison to serialize a canonical persistence projection of the session entry and adds regression coverage for hydrated skillsSnapshot prompt blobs plus runtime-only resolvedSkills.

PR surface: Source +29, Tests +55. Total +84 across 4 files.

Reproducibility: yes. at source level: current main JSON-stringifies the whole entry while store loads can represent the same skillsSnapshot as promptRef, hydrated prompt, or resolvedSkills. The PR body also reports a live Discord repro with the fix hunk reverted.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: persistent cache schema: src/config/sessions/session-accessor.test.ts, serialized state: src/config/sessions/session-accessor.test.ts, serialized state: src/config/sessions/session-accessor.ts, serialized state: src/config/sessions/store-load.ts, serialized state: src/config/sessions/store.ts, unknown-data-model-change: src/config/sessions/session-accessor.test.ts, and 1 more. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #96698
Summary: This PR is a candidate fix for the open Discord skillsSnapshot false stale-snapshot issue; two other open PRs overlap the same root cause, while merged related PRs address adjacent serialization or retry behavior.

Members:

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

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:

  • [P2] Get required CI green or document unrelated failures before merge.
  • After landing this PR, close or supersede the remaining same-root-cause PRs.

Risk before merge

  • [P1] The diff changes optimistic revision identity in a shared session boundary; if the projection misses a meaningful persisted field, stale writes could be accepted or valid replies could still be rejected.
  • [P1] Required PR checks are currently failing or pending, and logs were unavailable while the workflow run was still in progress, so maintainers need final CI evidence before merge.
  • [P1] Multiple same-root-cause PRs remain open, so maintainers should choose one canonical landing path to avoid duplicate branches diverging.

Maintainer options:

  1. Finish Checks, Then Land This Fix (recommended)
    Require the current head to finish required CI cleanly, then merge this PR as the canonical promptRef hydration fix if maintainers accept the live Discord proof.
  2. Accept The Session Boundary Risk
    Maintainers could merge on source review and live proof before every broader check is clean, but that owns the shared session-state regression risk explicitly.
  3. Pause For A Different Candidate
    If maintainers prefer another same-root-cause PR, pause this branch and require the chosen candidate to carry the same regression coverage and real Discord proof.

Next step before merge

  • [P2] The remaining action is maintainer merge review for a protected-label shared session fix, plus final required-check resolution and canonical PR selection.

Security
Cleared: No concrete security or supply-chain concern found; the diff changes session revision comparison and tests without dependencies, workflows, secrets, permissions, or external code execution.

Review details

Best possible solution:

Land one canonical persisted-shape revision comparison after required checks are green, then close the linked issue and stale duplicate candidate PRs.

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

Yes at source level: current main JSON-stringifies the whole entry while store loads can represent the same skillsSnapshot as promptRef, hydrated prompt, or resolvedSkills. The PR body also reports a live Discord repro with the fix hunk reverted.

Is this the best way to solve the issue?

Yes: the PR uses the existing persistence projection boundary instead of changing runtime snapshot hydration or adding Discord-specific handling. The remaining blocker is merge validation, not a different code shape.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P1: The PR targets a broken Discord follow-up workflow where false session-state conflicts can suppress the second inbound reply.
  • add merge-risk: 🚨 message-delivery: A wrong shared reply-session initialization comparison can continue blocking visible channel follow-up replies.
  • add merge-risk: 🚨 session-state: The diff changes the optimistic revision identity used to guard writes to persisted session metadata.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live Discord proof and a reverted-fix repro with Crabbox lease/run IDs, including promptRef persistence and second-reply success after the fix.
  • 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): The PR body includes after-fix live Discord proof and a reverted-fix repro with Crabbox lease/run IDs, including promptRef persistence and second-reply success after the fix.

Label justifications:

  • P1: The PR targets a broken Discord follow-up workflow where false session-state conflicts can suppress the second inbound reply.
  • merge-risk: 🚨 session-state: The diff changes the optimistic revision identity used to guard writes to persisted session metadata.
  • merge-risk: 🚨 message-delivery: A wrong shared reply-session initialization comparison can continue blocking visible channel follow-up replies.
  • 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): The PR body includes after-fix live Discord proof and a reverted-fix repro with Crabbox lease/run IDs, including promptRef persistence and second-reply success after the fix.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live Discord proof and a reverted-fix repro with Crabbox lease/run IDs, including promptRef persistence and second-reply success after the fix.
Evidence reviewed

PR surface:

Source +29, Tests +55. Total +84 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 3 34 5 +29
Tests 1 55 0 +55
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 89 5 +84

What I checked:

Likely related people:

  • jalehman: Authored the merged guarded reply-session initialization refactor that introduced the load/commit revision boundary involved here. (role: reply-session initialization boundary contributor; confidence: high; commits: 95e37f8e9517; files: src/config/sessions/session-accessor.ts, src/auto-reply/reply/session.ts)
  • steipete: Authored the merged persisted skill prompt blob projection and hydration work that creates the durable promptRef versus hydrated prompt representation involved in the mismatch. (role: adjacent promptRef persistence contributor; confidence: high; commits: edd4c62da1c2; files: src/config/sessions/skill-prompt-blobs.ts, src/config/sessions/store-load.ts, src/config/sessions/store.ts)
  • obviyus: Authored and merged the reply-session initialization serialization fix in the same conflict family, which overlaps the runtime path without resolving this promptRef hydration mismatch. (role: recent adjacent session-initialization contributor; confidence: medium; commits: d2da8c79d9b8; files: src/config/sessions/session-accessor.ts, src/auto-reply/reply/session.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 proof: sufficient ClawSweeper judged the real behavior proof convincing. 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. 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 29, 2026
@joshavant
joshavant merged commit 75db48a into main Jun 29, 2026
145 of 162 checks passed
@joshavant
joshavant deleted the fix/96698-reply-init-persisted-revision branch June 29, 2026 04:45
scribe-dandelion-cult added a commit to karmaterminal/openclaw that referenced this pull request Jun 29, 2026
Bring upstream/main into frond-scribe/20260624/assembly-continuation-followons, including the canonical reply-init fix from upstream openclaw#97657, while preserving continuation assembly behavior and resolving the three drift conflicts.

Co-authored-by: Copilot <[email protected]>
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 29, 2026
QiuYuang pushed a commit to QiuYuang/openclaw that referenced this pull request Jul 1, 2026
chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 1, 2026
amittell pushed a commit to amittell/openclaw that referenced this pull request Jul 2, 2026
NOVA-Openclaw pushed a commit to NOVA-Openclaw/nova-openclaw that referenced this pull request Jul 3, 2026
chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 3, 2026
tsekula added a commit to tsekula/openclaw that referenced this pull request Jul 6, 2026
Cherry-picked from 75db48a (openclaw#97657) to prevent cache hydration
differences from causing false-positive stale-snapshot conflicts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer Maintainer-authored PR 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. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S 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.

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

1 participant