Skip to content

fix(config/sessions): narrow reply-session initialization revision to identity fields#98835

Merged
jalehman merged 5 commits into
openclaw:mainfrom
moguangyu5-design:fix/98672-session-initialization-conflict
Jul 2, 2026
Merged

fix(config/sessions): narrow reply-session initialization revision to identity fields#98835
jalehman merged 5 commits into
openclaw:mainfrom
moguangyu5-design:fix/98672-session-initialization-conflict

Conversation

@moguangyu5-design

@moguangyu5-design moguangyu5-design commented Jul 2, 2026 •
edited by jalehman
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes #98672. Users observed reply session initialization conflicted for agent:main:main errors that broke ongoing sessions until the gateway was restarted. The error came from the guarded reply-session initialization path comparing the full persisted session entry between snapshot and commit.

Why This Change Was Made

The initialization guard only needs to detect whether another initializer rotated the session, meaning sessionId or sessionFile changed. Comparing the entire entry included volatile fields such as updatedAt, heartbeat metadata, pending-delivery metadata, and other fields that can legitimately change while a reply-session initialization is in flight.

The guard is narrowed to the identity fields sessionId and sessionFile. When that identity-only guard passes, the commit path carries forward metadata that changed since the snapshot via mergeConcurrentReplySessionMetadata. Fields the prepared turn entry explicitly modified relative to the snapshot still win, so /new and /reset continue to clear recovered auto-fallback provider/model overrides, runtime model caches, and context-budget state as before.

The merge also treats an absent field and an own undefined field as equivalent when deciding whether the prepared entry left the snapshot unchanged. That prevents incidental optional undefined properties from overwriting concurrent same-session additions.

User Impact

  • Sessions no longer fail with reply session initialization conflicted when same-session background activity touches metadata during reply initialization.
  • Real session rotations are still detected and retried as before.
  • Concurrent heartbeat, delivery, context, and optional metadata written after the snapshot is no longer overwritten by the initialization commit.
  • Explicit reset clearing behavior (/new, /reset) is preserved.

Evidence

Behavior proof: same-session concurrent metadata write

This branch adds src/config/sessions/session-accessor.reply-init-concurrency.test.ts, a deterministic two-process behavior proof for the affected race window:

  1. The parent process seeds agent:main:main in a real temporary sessions.json store.
  2. A child Node process loads the real loadReplySessionInitializationSnapshot and waits with that snapshot held.
  3. The parent process performs a production updateSessionEntry write that changes same-session metadata after the snapshot.
  4. The child process calls the real commitReplySessionInitialization with the held revision and snapshot.
  5. The expected fixed behavior is ok: true; a stale-snapshot result would reproduce the user-visible conflict class.

This is a deterministic multi-process proof at the production session-store/accessor boundary where the conflict is created and resolved.

$ node scripts/run-vitest.mjs src/config/sessions/session-accessor.reply-init-concurrency.test.ts --reporter=verbose

[test] starting test/vitest/vitest.runtime-config.config.ts

 RUN  v4.1.8 /Users/phaedrus/Projects/openclaw/.worktrees/pr-98835-thermonuclear

 ✓ |runtime-config| src/config/sessions/session-accessor.reply-init-concurrency.test.ts > reply session initialization concurrency > commits after same-session activity from another process 1114ms

 Test Files  1 passed (1)
      Tests  1 passed (1)

Local real-gateway proof, not committed

I also verified the same race through an actual spawned OpenClaw gateway process. This was an exploratory local proof only; the temporary test hook and e2e harness were removed and are not part of this PR.

Methodology:

  1. Added a temporary VITEST-only pause seam immediately after loadReplySessionInitializationSnapshot() in src/auto-reply/reply/session.ts.
  2. Added a temporary e2e test that starts a real gateway child process with createOpenClawTestInstance.
  3. Connected to that gateway with the real Gateway WebSocket client.
  4. Sent chat.send for agent:main:main.
  5. Paused the gateway child after it loaded the reply-session initialization snapshot.
  6. Mutated the same gateway-owned temporary sessions.json from the parent process with production updateSessionEntry, changing heartbeat metadata after the snapshot.
  7. Resumed the gateway and asserted the chat.send ACK started successfully, the final session entry preserved the concurrent heartbeat metadata, and gateway logs did not contain reply session initialization conflicted.

Proof commands and results:

$ node scripts/run-vitest.mjs test/gateway.reply-session-init.e2e.test.ts

 Test Files  1 passed (1)
      Tests  1 passed (1)
[test] passed 1 Vitest shard in 16.94s

$ node scripts/run-vitest.mjs src/config/sessions/session-accessor.reply-init-concurrency.test.ts src/config/sessions/session-accessor.test.ts test/gateway.reply-session-init.e2e.test.ts

Session accessor shard:
 Test Files  2 passed (2)
      Tests  65 passed (65)

Gateway e2e shard:
 Test Files  1 passed (1)
      Tests  1 passed (1)

[test] passed 2 Vitest shards in 32.12s

$ node scripts/run-oxlint.mjs src/auto-reply/reply/session.ts test/gateway.reply-session-init.e2e.test.ts
Passed.

$ git diff --check
Passed.

Focused regression coverage

$ node scripts/run-vitest.mjs src/config/sessions/session-accessor.test.ts -t 'commits reply session initialization despite active-turn metadata changes|commits reply session initialization despite non-identity metadata changes|preserves concurrent optional additions when prepared fields are undefined|does not restore pending final delivery metadata cleared after the snapshot|does not merge old-session delivery metadata into a rotated session|rejects stale reply session initialization snapshots without writing' --reporter=verbose

 ✓ rejects stale reply session initialization snapshots without writing
 ✓ commits reply session initialization despite active-turn metadata changes
 ✓ commits reply session initialization despite non-identity metadata changes
 ✓ preserves concurrent optional additions when prepared fields are undefined
 ✓ does not restore pending final delivery metadata cleared after the snapshot
 ✓ does not merge old-session delivery metadata into a rotated session

 Test Files  1 passed (1)
      Tests  6 passed | 58 skipped (64)

Local quality gates

$ node_modules/.bin/oxfmt --check src/config/sessions/session-accessor.ts src/config/sessions/session-accessor.test.ts src/config/sessions/session-accessor.reply-init-concurrency.test.ts
Checking formatting...
All matched files use the correct format.

$ node scripts/run-oxlint.mjs src/config/sessions/session-accessor.ts src/config/sessions/session-accessor.test.ts src/config/sessions/session-accessor.reply-init-concurrency.test.ts
Passed.

$ git diff --check
Passed.

Autoreview

Structured autoreview was run against the current branch diff after the optional-undefined merge fix:

$ .agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main

autoreview clean: no accepted/actionable findings reported
overall: patch is correct (0.79)

CI

Current head: 455615bd04a01fbaccc806581139805a91dcb7af.

gh pr checks 98835 is green for the active check set. Relevant passing checks include:

  • Real behavior proof
  • check-session-accessor-boundary
  • check-test-types
  • check-lint
  • check-prod-types
  • check-dependencies
  • check-shrinkwrap
  • QA Smoke CI
  • build-artifacts
  • OpenGrep OSS
  • Security High (core-auth-secrets)
  • Security High (channel-runtime-boundary)
  • Security High (network-ssrf-boundary)
  • Security High (mcp-process-tool-boundary)
  • Security High (process-exec-boundary)
  • Security High (plugin-trust-boundary)
  • Security High (actions)

PR merge state is CLEAN at the current head.

@clawsweeper

clawsweeper Bot commented Jul 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 2, 2026, 1:58 PM ET / 17:58 UTC.

Summary
The PR narrows reply-session initialization conflict detection to session identity fields, merges safe concurrent metadata drift, and adds file-backed regression/concurrency tests.

PR surface: Source +110, Tests +538. Total +648 across 4 files.

Reproducibility: yes. for the source path: current main hashes the full persisted session entry shape, so non-identity metadata drift can invalidate the snapshot before commit. I did not run tests in this read-only review, but the PR adds focused tests that exercise the same accessor race.

Review metrics: 1 noteworthy metric.

  • Session guard semantics: 1 revision guard narrowed, 1 metadata merge added. This measured behavior change decides whether same-session metadata writes are tolerated or block inbound replies.

Stored data model
Persistent data-model change detected: serialized state: src/auto-reply/reply/session.ts, serialized state: src/config/sessions/session-accessor.reply-init-concurrency.test.ts, serialized state: src/config/sessions/session-accessor.test.ts, serialized state: src/config/sessions/session-accessor.ts, unknown-data-model-change: src/config/sessions/session-accessor.reply-init-concurrency.test.ts, unknown-data-model-change: src/config/sessions/session-accessor.test.ts, and 2 more. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #98672
Summary: This PR is the candidate source-level fix for the open cross-channel reply-session initialization conflict, while related items cover package parity, symptoms, or retry mitigation.

Members:

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

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

  • [P1] The PR changes conflict acceptance semantics for shared reply-session state; a bad merge could preserve stale metadata or keep rejecting valid same-session turns.
  • [P1] Inbound reply dispatch depends on this accessor path across channels, so regressions here can surface as dropped or suppressed messages.
  • [P1] Merging this source fix does not by itself repair already-published v2026.6.11 packages; release/package parity remains tracked separately at [Bug] v2026.6.11 published dist missing reentrancy guard - reply session initialization conflicted #98416.

Maintainer options:

  1. Land With Release Coordination (recommended)
    Merge this focused source fix once required checks are green and maintainers accept the shared session-state behavior, while tracking package release parity separately.
  2. Request One More Transport Proof
    Maintainers can ask for an additional redacted Discord, webchat, or Telegram-style runtime proof if they want transport-level confidence beyond the accessor and spawned-gateway proof.
  3. Pause For Companion Sequencing
    Pause only if maintainers want the retry-budget mitigation or v2026.6.11 artifact investigation resolved before landing this source guard change.

Next step before merge

  • No automated repair lane is needed; maintainers should decide merge timing after required checks and release/package sequencing.

Security
Cleared: The final live diff changes TypeScript session-state logic and tests only; no dependency, workflow, secret, package, permission, or supply-chain surface change remains.

Review details

Best possible solution:

Land this accessor-boundary source fix after required checks and maintainer session-state review, keep v2026.6.11 package parity under #98416, and treat #99048 as a companion retry mitigation rather than a replacement.

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

Yes for the source path: current main hashes the full persisted session entry shape, so non-identity metadata drift can invalidate the snapshot before commit. I did not run tests in this read-only review, but the PR adds focused tests that exercise the same accessor race.

Is this the best way to solve the issue?

Yes. The fix is at the shared accessor boundary, narrows the false conflict predicate to session identity, and preserves safe concurrent metadata without replacing it with a broader retry-only workaround.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P1: The PR targets a cross-channel reply-session conflict that can stop or drop real user messages until retry or gateway restart.
  • merge-risk: 🚨 session-state: The diff changes guarded session-store initialization and concurrent metadata merge semantics for shared reply-session state.
  • merge-risk: 🚨 message-delivery: The stale-snapshot failure occurs during inbound reply dispatch and can suppress messages across channels.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix terminal proof from a spawned gateway/WebSocket race run plus focused accessor-boundary test output for the changed behavior.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal proof from a spawned gateway/WebSocket race run plus focused accessor-boundary test output for the changed behavior.
Evidence reviewed

PR surface:

Source +110, Tests +538. Total +648 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 2 118 8 +110
Tests 2 538 0 +538
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 656 8 +648

What I checked:

  • Repository policy: Root AGENTS.md requires full review beyond the diff and treats auth/session state and persisted preferences as compatibility-sensitive, which directly applies to this session-state PR. (AGENTS.md:31, 55ed57a4e39b)
  • Current main conflict predicate: Current main computes the reply-session initialization revision from the canonical persisted session entry, so same-session metadata drift can change the revision and reject the guarded commit before message handling proceeds. (src/config/sessions/session-accessor.ts:1159, 55ed57a4e39b)
  • Runtime caller: Current main loads a reply-session initialization snapshot before building the prepared entry and calls commitReplySessionInitialization from the shared reply startup path. (src/auto-reply/reply/session.ts:455, 55ed57a4e39b)
  • PR identity guard: PR head changes createReplySessionInitializationRevision to serialize only sessionId and sessionFile, preserving real session rotation detection while ignoring non-identity metadata churn. (src/config/sessions/session-accessor.ts:1248, 61fc7d2ece1a)
  • PR metadata merge: PR head adds mergeConcurrentReplySessionMetadata and uses it at the guarded commit boundary so fields changed since the snapshot are carried forward only when the prepared entry did not explicitly modify them. (src/config/sessions/session-accessor.ts:1200, 61fc7d2ece1a)
  • Caller passes snapshot: PR head passes initializationSnapshot.currentEntry into the commit call, giving the accessor the baseline it needs for snapshot-aware merge decisions. (src/auto-reply/reply/session.ts:930, 61fc7d2ece1a)

Likely related people:

  • jalehman: GitHub path history shows jalehman authored the guarded reply-session initialization refactor and several adjacent session-accessor migrations in the affected files. (role: reply-session initialization boundary contributor; confidence: high; commits: 95e37f8e9517, 7e2b2d2987b2, b58e6e0734f2; files: src/config/sessions/session-accessor.ts, src/auto-reply/reply/session.ts)
  • obviyus: GitHub path history shows obviyus authored the reentrant session-store writer support and reply initialization serialization that this guarded commit path depends on. (role: adjacent session writer-lane contributor; confidence: high; commits: d2da8c79d9b8; files: src/config/sessions/session-accessor.ts, src/auto-reply/reply/session.ts, src/config/sessions/store-writer.ts)
  • joshavant: Recent history shows joshavant worked on persisted-shape reply-init revisions and active implicit rollover behavior in the same session initialization area. (role: recent session-accessor contributor; confidence: high; commits: 75db48a17522, 6f1076351cbe; files: src/config/sessions/session-accessor.ts, src/config/sessions/session-accessor.test.ts, src/auto-reply/reply/session.ts)
  • vincentkoc: Recent path history shows vincentkoc touched scoped store path resolution, session-store cleanup, and shared writer queue helpers adjacent to the affected accessor boundary. (role: recent adjacent session-store contributor; confidence: medium; commits: 10d1af6867fc, e3058efa1042, 27e13933c0f3; files: src/config/sessions/session-accessor.ts, src/config/sessions/store-writer.ts, src/shared/store-writer-queue.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 Jul 2, 2026
@moguangyu5-design

moguangyu5-design commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the ClawSweeper review feedback from the latest review:

  • commitReplySessionInitialization now merges the current persisted row with the prepared turn entry via mergeSessionEntry, so concurrent non-identity metadata (heartbeat, delivery, context-budget, route fields) is preserved when the identity-only guard passes. The prepared entry still wins for fields it explicitly carries.
  • Extended the regression test to assert that accepted metadata drift (lastHeartbeatSentAt, lastHeartbeatText) survives the commit and is visible in the persisted entry.
  • Fixed the failing check-test-types CI shard by replacing the incorrectly typed contextBudgetStatus test values with type-safe lastHeartbeat* fields.
  • Added terminal proof from the focused regression test run to the PR body.

Ready for re-review.

@moguangyu5-design

moguangyu5-design commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Updated the fix to address the CI regression in src/auto-reply/reply/session.test.ts:

  • Replaced the full-row mergeSessionEntry merge with mergeConcurrentReplySessionMetadata, which carries forward only fields that changed since the snapshot and were not explicitly modified by the prepared turn entry.
  • This preserves the intended /new and /reset clearing behavior for auto-fallback provider/model overrides and runtime model caches, while still protecting concurrent heartbeat/delivery/context metadata from being rolled back.
  • Updated the regression test to use a prepared entry that inherits the pre-drift heartbeat values, matching how the real caller builds it.

Verification:

  • src/config/sessions/session-accessor.test.ts: 60/60 pass
  • src/auto-reply/reply/session.test.ts: 124/124 pass
  • pnpm tsgo:core, pnpm tsgo:core:test, pnpm tsgo:extensions:test, oxfmt, oxlint pass

The two remaining failures from the last CI run (render-maturity-docs and agent-bundle-mcp-runtime timeout) appear unrelated and the former passes locally.

@moguangyu5-design

moguangyu5-design commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 2, 2026 •
edited
Loading

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.

This was referenced Jul 2, 2026
Closed
Closed
Closed
@moguangyu5-design

moguangyu5-design commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@openclaw-mantis visual task: verify repeated main-session messages no longer hit reply session initialization conflicted while session metadata changes concurrently.

@clawsweeper clawsweeper Bot mentioned this pull request Jul 2, 2026
Closed
@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. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed 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. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jul 2, 2026
@moguangyu5-design @jalehman
fix(config/sessions): narrow reply-session initialization revision to…
237f3ad 
… identity fields

The initialization guard compared the full persisted session entry, so
background touches to updatedAt, heartbeat timestamps, context-budget
metadata, etc. produced false-positive stale-snapshot conflicts and the
"reply session initialization conflicted" error.

Only sessionId and sessionFile matter for detecting a session rotation.
Narrow the revision to those identity fields and add a regression test.

Fixes openclaw#98672
@moguangyu5-design @jalehman
fix(config/sessions): merge current metadata when reply-init identity…
365d5d1 
… guard passes
@moguangyu5-design @jalehman
fix(config/sessions): preserve only snapshot-drifted metadata in repl…
748d1b6 
…y-init commit
This was referenced Jul 5, 2026
Closed
Closed
Closed
Closed
Closed
Closed
Closed
@omarshahine omarshahine mentioned this pull request Jul 5, 2026
Merged
@amknight amknight mentioned this pull request Jul 6, 2026
Closed
This was referenced Jul 6, 2026
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Open
Closed
Closed
Closed
@syfvb syfvb mentioned this pull request Jul 10, 2026
Open
This was referenced Jul 11, 2026
Closed
Merged
@ouyangenlin ouyangenlin mentioned this pull request Jul 12, 2026
Open
@clawsweeper clawsweeper Bot mentioned this pull request Jul 13, 2026
Closed
@syfvb syfvb mentioned this pull request Jul 15, 2026
Open
@clawsweeper clawsweeper Bot mentioned this pull request Jul 16, 2026
Closed
@k176060444-lgtm k176060444-lgtm mentioned this pull request Jul 16, 2026
Open
@clawsweeper clawsweeper Bot mentioned this pull request Jul 17, 2026
Open
@jalehman jalehman mentioned this pull request Jul 20, 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. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: L status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Sessions breaking constantly

2 participants

@moguangyu5-design @jalehman