Skip to content

fix(agents): update session store with rotated session id after post-compaction transcript rotation#88055

Merged
steipete merged 1 commit into
openclaw:mainfrom
1052326311:fix/session-store-stale-session-id
May 29, 2026
Merged

fix(agents): update session store with rotated session id after post-compaction transcript rotation#88055
steipete merged 1 commit into
openclaw:mainfrom
1052326311:fix/session-store-stale-session-id

Conversation

@1052326311

@1052326311 1052326311 commented May 29, 2026 •
edited by steipete
Loading

Copy link
Copy Markdown
Contributor

Summary

When truncateAfterCompaction: true rotates an OpenClaw transcript during an agent run, the embedded runner reports the new OpenClaw session ID and transcript file in result.meta.agentMeta.sessionId/sessionFile. The previous patch updated only the first session-store write, leaving later transcript persistence, CLI compaction lifecycle, and delivery freshness checks on the old pre-rotation session ID.

This change carries one effective rotated OpenClaw session identity through the full post-run path, persists the rotated transcript file/family metadata, and keeps backend CLI session IDs in their existing CLI binding fields instead of treating them as OpenClaw session IDs.

Closes #88040

Change Type

  • Bug fix

Scope

  • Agents / session store

Real behavior proof

Behavior addressed: automatic compaction transcript rotation no longer leaves post-run session persistence on the stale pre-rotation OpenClaw session ID.

Real environment tested: local OpenClaw checkout on macOS; branch rebased onto origin/main at 07870dff45.

Exact steps or command run after this patch: node scripts/run-vitest.mjs src/agents/agent-command.compaction-rotation.test.ts src/agents/agent-command.live-model-switch.test.ts src/agents/command/session-store.test.ts

Evidence after fix: 6 Vitest project-files passed, 128 tests passed. The new regression runs two agentCommand turns through real session resolution, sessions.json, and transcript persistence: the first turn simulates automatic rotation to a successor transcript, then the second turn resumes by the rotated session ID and verifies it uses the rotated transcript file.

Observed result after fix: post-run store update, transcript persistence, CLI compaction lifecycle, and delivery freshness all receive the rotated OpenClaw session ID only when a rotated sessionFile is reported; backend CLI session IDs remain CLI bindings.

What was not tested: no live model/provider compaction run. pnpm check:changed was attempted through Blacksmith Testbox tbx_01kstjya6zkexr2xqpv0mfth91, but current origin/main failed core-test typecheck in unrelated src/config/sessions.cache.test.ts lines 743, 772, 791, and 796.

Autoreview: /Users/steipete/Projects/agent-scripts/skills/autoreview/scripts/autoreview --mode local on the added regression reported no accepted/actionable findings. Earlier branch review of the implementation also reported no accepted/actionable findings.

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 29, 2026
@clawsweeper

clawsweeper Bot commented May 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed May 29, 2026, 4:04 PM ET / 20:04 UTC.

Summary
The branch carries a rotated embedded session id/file through agent-command post-run persistence, updates session-store family metadata, and adds regression tests for rotated transcript resume and CLI session-id separation.

PR surface: Source +23, Tests +422. Total +445 across 5 files.

Reproducibility: yes. from source inspection: current main's embedded runner reports rotated sessionId/sessionFile after transcript rotation, while the post-run caller still uses the original session id for store/transcript/compaction/delivery paths. I did not run a live compaction scenario.

Review metrics: none identified.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • [P1] Persist rotated sessionId/sessionFile through preserveUserFacingSessionModelState metadata patches and add a focused regression test for that path.
  • [P1] Add redacted real behavior proof showing an after-fix compaction rotation updates sessions.json and a subsequent turn resumes from the rotated transcript file.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body supplies structured proof, but the after-fix evidence is a Vitest run with mocked runner behavior; it still needs redacted real runtime output, logs, terminal output, or a recording showing transcript rotation, updated sessions.json, and a successful next turn. 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] The diff changes canonical session id/file propagation after transcript rotation; if preserved-state or heartbeat runs are not covered, users can still keep stale transcript pointers and hit the same blocked next-turn behavior.
  • [P1] The PR body's proof is still a targeted Vitest run with mocked runner behavior, not a real after-fix compaction rotation plus next-turn resume in a live or redacted runtime setup.
  • [P1] There is an open competing fix at fix(agents): adopt rotated session id and file after embedded compaction rotation #88106 for the same linked issue, so landing two similar session-identity patches could create review and maintenance drift.

Maintainer options:

  1. Persist preserved-state rotations before merge (recommended)
    Update the session-store metadata patch so rotated sessionId/sessionFile reach disk even when preserveUserFacingSessionModelState is true, and cover that path with a focused regression test.
  2. Defer to the competing session-store branch
    If maintainers prefer fix(agents): adopt rotated session id and file after embedded compaction rotation #88106, pause this branch and use one canonical PR to avoid two similar session-identity fixes diverging.

Next step before merge

  • [P1] Needs contributor or maintainer handling for the preserved-state code gap and real behavior proof; automation cannot provide the contributor's real setup proof.

Security
Cleared: The diff does not touch dependencies, workflows, credentials, permissions, or other supply-chain/code-execution surfaces beyond local agent session persistence.

Review findings

  • [P2] Persist rotations through preserved-state writes — src/agents/command/session-store.ts:137
Review details

Best possible solution:

Land one canonical session-store fix that adopts embedded rotated id/file for all post-run persistence, including preserved-state/heartbeat metadata patches, keeps CLI backend IDs as CLI bindings, and includes real after-fix proof.

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

Yes, from source inspection: current main's embedded runner reports rotated sessionId/sessionFile after transcript rotation, while the post-run caller still uses the original session id for store/transcript/compaction/delivery paths. I did not run a live compaction scenario.

Is this the best way to solve the issue?

No, not as written. Carrying an effective rotated id through agent-command is the right direction, but the session-store write must also persist rotated id/file through preserved-state metadata patches so heartbeat or preserved user-facing runs do not retain stale transcript pointers.

Full review comments:

  • [P2] Persist rotations through preserved-state writes — src/agents/command/session-store.ts:137
    This branch assigns the rotated id/file to the normal session entry, but updateSessionStoreAfterAgentRun still persists only updatedAt and lastInteractionAt when preserveUserFacingSessionModelState is true. Agent-command passes that mode for preserved user-facing state, so an automatic truncateAfterCompaction rotation there can still leave sessions.json on the stale transcript file and the next turn can hit the same lock/deadlock this PR is meant to fix. Include the rotated identity in that preserved-state metadata patch while keeping model/token fields preserved, and add regression coverage for that mode.
    Confidence: 0.83

Overall correctness: patch is incorrect
Overall confidence: 0.84

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against f10bad944ffc.

Label changes

Label justifications:

  • P2: This is a normal-priority agent session-state bug fix with limited blast radius but real next-turn deadlock impact for rotated transcripts.
  • merge-risk: 🚨 session-state: The PR changes canonical session id/file propagation after compaction rotation, and an incomplete path can leave sessions.json pointing at stale transcript state.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • 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 supplies structured proof, but the after-fix evidence is a Vitest run with mocked runner behavior; it still needs redacted real runtime output, logs, terminal output, or a recording showing transcript rotation, updated sessions.json, and a successful next turn. 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 +23, Tests +422. Total +445 across 5 files.

View PR surface stats
Area Files Added Removed Net
Source 2 33 10 +23
Tests 3 422 0 +422
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 5 455 10 +445

What I checked:

Likely related people:

  • steipete: Current-main blame for the central agent-command/session-store/embedded-runner paths points to Peter Steinberger's 58c46ec graft commit, and the PR timeline shows steipete assigned and force-pushed the branch. (role: recent area contributor and likely follow-up owner; confidence: medium; commits: 58c46ec03b84, 1ca7f5c0a057, 467b068fdc7b; files: src/agents/agent-command.ts, src/agents/command/session-store.ts, src/agents/embedded-agent-runner/run/attempt.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. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels May 29, 2026
@coder999999999 coder999999999 mentioned this pull request May 29, 2026
Closed
@steipete steipete self-assigned this May 29, 2026
@steipete steipete force-pushed the fix/session-store-stale-session-id branch from 04f95be to 2874138 Compare May 29, 2026 19:35
@openclaw-barnacle openclaw-barnacle Bot added size: S proof: supplied External PR includes structured after-fix real behavior proof. and removed size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 29, 2026
@1052326311 @steipete
fix(agents): update session store with rotated session id after compa…
c3d3c77 
…ction

When truncateAfterCompaction rotates the transcript to a new session file,
rotateTranscriptAfterCompaction creates a new session ID. The run records
this correctly in result.meta.agentMeta.sessionId, but
updateSessionStoreAfterAgentRun was called with the original (pre-rotation)
sessionId, leaving sessions.json pointing at the old file.

Use result.meta.agentMeta.sessionId when available so the session store
reflects the rotated file path.

Closes openclaw#88040
@steipete steipete force-pushed the fix/session-store-stale-session-id branch from 2874138 to c3d3c77 Compare May 29, 2026 19:58
@steipete

steipete commented May 29, 2026

Copy link
Copy Markdown
Contributor

Landing proof for c3d3c77ddf675bbba0b9ba6681b030a2f69a898c.

Behavior addressed: sessions.json now records the rotated OpenClaw session id and transcript file after compaction, while backend CLI session ids stay backend-only.

Exact steps or command run after this patch:

  • node scripts/run-vitest.mjs src/agents/agent-command.compaction-rotation.test.ts src/agents/agent-command.live-model-switch.test.ts src/agents/command/session-store.test.ts
  • $autoreview local review on the branch changes
  • GitHub CI on PR head c3d3c77ddf675bbba0b9ba6681b030a2f69a898c

Evidence after fix:

  • Local Vitest: 6 project files, 128 tests passed.
  • Autoreview: no accepted/actionable findings.
  • CI: PR rollup complete, 115 completed checks, 0 failed, 0 pending; CodeQL Critical Quality run 26659178698 passed, including agent-runtime-boundary and network-runtime-boundary.

Known proof gap:

  • Broad pnpm check:changed via Testbox tbx_01kstjya6zkexr2xqpv0mfth91 hit unrelated current-main typecheck errors in src/config/sessions.cache.test.ts; targeted regression proof and PR CI are green.

Thanks @1052326311.

@steipete steipete merged commit 9601172 into openclaw:main May 29, 2026
111 of 115 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@steipete steipete

Labels

agents Agent runtime and tooling merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: M 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.

fix: sessions.json stale after compaction transcript rotation (deadlock)

2 participants

@1052326311 @steipete