Skip to content

fix(sessions): avoid stale restart continuation reuse#87378

Merged
steipete merged 3 commits into
openclaw:mainfrom
TurboTheTurtle:fix/terminal-session-routing-86593
May 27, 2026
Merged

fix(sessions): avoid stale restart continuation reuse#87378
steipete merged 3 commits into
openclaw:mainfrom
TurboTheTurtle:fix/terminal-session-routing-86593

Conversation

@TurboTheTurtle

@TurboTheTurtle TurboTheTurtle commented May 27, 2026 •
edited by steipete
Loading

Copy link
Copy Markdown
Contributor

Summary

  • keep normal completed-run lifecycle fields (status, endedAt, runtimeMs) out of session freshness decisions so ordinary follow-up turns continue the existing conversation
  • clear stale lifecycle markers when a new reply session row is intentionally created over an existing row
  • make queued restart agentTurn deliveries carry the session id they were queued for, then fall back to a system wake if the session key now points at a different session

Closes #86593.

Validation

  • node scripts/run-vitest.mjs src/gateway/server-restart-sentinel.test.ts src/auto-reply/reply/session.test.ts src/agents/command/session-store.test.ts
  • node_modules/.bin/oxfmt --check src/config/sessions/types.ts src/agents/command/session.ts src/auto-reply/reply/session.ts src/infra/session-delivery-queue-storage.ts src/gateway/server-restart-sentinel.ts src/agents/command/session-store.test.ts src/auto-reply/reply/session.test.ts src/gateway/server-restart-sentinel.test.ts
  • git diff --check
  • /Users/steipete/Projects/agent-scripts/skills/autoreview/scripts/autoreview --mode branch --base origin/main

Real behavior proof

Behavior addressed: stale restart continuation delivery must not dispatch an agentTurn into a session key after that key has rotated to a different session id. Completed run metadata alone must not force normal fresh sessions to rotate.

Real environment tested: Real local OpenClaw checkout on branch pr-87378, head SHA 4521a0119da537b91d7402e8f07898785f3f7d9e, using a temporary JSON session store and the real resolveSession implementation.

Exact steps or command run after this patch: Created a temporary sessions.json containing a fresh row for agent:main:explicit:proof-completed-entry with sessionId=proof-completed-session, status=done, numeric startedAt, numeric endedAt, and runtimeMs. Then ran resolveSession against that real store path and printed the resolved session state.

Evidence after fix:

sessionKey=agent:main:explicit:proof-completed-entry
existingSessionId=proof-completed-session
resolvedSessionId=proof-completed-session
isNewSession=false
reusedExistingSession=true
completedStatusSeen=done
completedEndedAtSeen=true

Observed result after fix: A fresh completed-run row continues with its existing session id (reusedExistingSession=true) instead of rotating just because status=done and endedAt are present. The restart-sentinel fallback path is covered by the focused test suite and now keys queued agent turns by expectedSessionId.

What was not tested: No live Telegram Desktop restart was run locally. The focused restart-sentinel test exercises the routing and fallback behavior with preserved delivery context.

Attribution

If maintainers squash/rework this PR, please preserve author attribution or include:

Co-authored-by: Andy Ye <[email protected]>

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

clawsweeper Bot commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed May 27, 2026, 6:45 PM ET / 22:45 UTC.

Summary
The PR changes session and restart-sentinel handling by clearing terminal lifecycle fields on new reply sessions, adding expectedSessionId to queued restart deliveries, and adding tests around terminal session rows.

PR surface: Source -2, Tests +256. Total +254 across 7 files.

Reproducibility: yes. from source inspection: current main evaluates session freshness and queues restart continuations without terminal status or endedAt checks. I did not run tests because this review had to keep the checkout read-only.

Review metrics: 1 noteworthy metric.

  • Queued delivery payload field: 1 optional field added. expectedSessionId becomes persisted restart-delivery state, so maintainers should review upgrade/recovery semantics in addition to the runtime guard.

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

  • Flip the CLI and reply-session tests to require fresh sessions for terminal rows and implement the shared terminal lifecycle guard.
  • Add a terminal-row guard to restart-sentinel startup and queued recovery before dispatching agentTurn continuations.
  • Update the PR body with current-head real behavior proof that shows terminal session rotation and restart fallback behavior.

Proof guidance:
Needs stronger real behavior proof before merge: The PR body includes useful terminal/log proof, but it is for older head 66895600a3f27e9464f87e5620abae697c3687f0; the live head is 4521a0119da537b91d7402e8f07898785f3f7d9e and now contains tests that contradict the claimed terminal-skip behavior, so current-head proof is required before merge. Screenshots or videos are useful when they show the behavior; terminal output, copied live output, linked artifacts, recordings, and redacted logs also count, with private data redacted. After proof is added, updating the PR body should trigger a fresh ClawSweeper review; if it does not, ask a maintainer to comment @clawsweeper re-review. 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.

Mantis proof suggestion
A native Telegram topic restart recording would show whether /new plus gateway restart avoids resuming the terminal session and keeps the fallback wake in the expected topic. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

telegram desktop proof: verify that a gateway restart after /new in a Telegram topic does not resume into the terminal session and preserves the fallback wake in the same topic.

Risk before merge

  • Merging the current head would still allow timestamp-fresh terminal rows to be reused, preserving the session-state wedge that the linked issue reports.
  • The restart-sentinel change only detects a changed session id; it can still dispatch an agentTurn into the same terminal row, so channel-visible continuation delivery can remain misrouted or wedged.
  • The real behavior proof is stale and contradicts the live diff: it proves terminal skip behavior on an older head while the current head's tests expect terminal reuse and dispatch.

Maintainer options:

  1. Repair Terminal Guards Before Merge (recommended)
    Add a shared terminal lifecycle guard to CLI/reply freshness and restart continuation delivery, update the tests to expect rotation/fallback, and require proof against the new head.
  2. Accept Same-Session Continuation Policy
    Maintainers could explicitly decide that same-id terminal rows should receive continuations, but that would leave the linked session-state wedge outside this PR's stated fix.
  3. Pause For Policy Rework
    If the desired terminal restart behavior is still unsettled, pause this PR and keep the linked issue open until the fallback-vs-fresh-session policy is chosen.

Next step before merge
Human review is needed because the current head has P1 correctness blockers and stale/conflicting real behavior proof; automation should not repair or merge until the contributor/maintainer supplies a corrected branch and current-head proof.

Security
Cleared: Cleared: the diff touches TypeScript session/restart runtime and tests only, with no dependency, workflow, lockfile, secret, or package-publishing changes.

Review findings

  • [P1] Do not assert terminal rows are reusable — src/agents/command/session-store.test.ts:624-625
  • [P1] Skip continuations when the current row is terminal — src/gateway/server-restart-sentinel.ts:265-268
Review details

Best possible solution:

Repair the PR so terminal status values and finite endedAt make CLI/reply resolution rotate to a fresh session and make restart-sentinel startup/recovery fall back instead of dispatching agentTurn into terminal rows, then add current-head real behavior proof.

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

Yes from source inspection: current main evaluates session freshness and queues restart continuations without terminal status or endedAt checks. I did not run tests because this review had to keep the checkout read-only.

Is this the best way to solve the issue?

No: the current PR head is not the best fix because it asserts terminal rows are reusable and only guards restart continuations when the session id changes. The maintainable fix is a shared terminal lifecycle guard plus tests that expect fresh-session rotation and fallback wake behavior.

Full review comments:

  • [P1] Do not assert terminal rows are reusable — src/agents/command/session-store.test.ts:624-625
    This new regression test expects a status: "done" row with endedAt to return isNewSession=false and reuse the old session id. That is the central failure reported by the linked issue: a terminal row can remain timestamp-fresh and keep receiving the next CLI/session resolution. The test and production resolver need to treat terminal status or finite endedAt as stale so a fresh session id is allocated.
    Confidence: 0.96
  • [P1] Skip continuations when the current row is terminal — src/gateway/server-restart-sentinel.ts:265-268
    The new guard only falls back when the stored sessionId differs from expectedSessionId. In the reported /new case the same terminal row can remain under the routing key, so entry.sessionId === expectedSessionId and the code continues to dispatch the queued agentTurn into a done session. Check terminal status/endedAt here before dispatching and fall back to the restart wake path for terminal rows.
    Confidence: 0.95

Overall correctness: patch is incorrect
Overall confidence: 0.95

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
  • remove rating: 🦐 gold shrimp: Current PR rating is rating: 🧂 unranked krab, so this older rating label is no longer current.

Label justifications:

  • P1: The PR targets a real broken session/channel workflow where terminal session reuse can wedge inbound replies after /new and gateway restart.
  • merge-risk: 🚨 session-state: The diff changes persisted session reuse and restart recovery behavior, and the current head still risks reusing terminal session state.
  • merge-risk: 🚨 message-delivery: The diff changes restart-sentinel agentTurn delivery and currently can still route a continuation into a terminal session.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body includes useful terminal/log proof, but it is for older head 66895600a3f27e9464f87e5620abae697c3687f0; the live head is 4521a0119da537b91d7402e8f07898785f3f7d9e and now contains tests that contradict the claimed terminal-skip behavior, so current-head proof is required before merge. Screenshots or videos are useful when they show the behavior; terminal output, copied live output, linked artifacts, recordings, and redacted logs also count, with private data redacted. After proof is added, updating the PR body should trigger a fresh ClawSweeper review; if it does not, ask a maintainer to comment @clawsweeper re-review. 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.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The PR affects visible Telegram topic restart behavior through continuation routing and fallback wake delivery, so a short Telegram proof would materially help review after the code is corrected.
Evidence reviewed

PR surface:

Source -2, Tests +256. Total +254 across 7 files.

View PR surface stats
Area Files Added Removed Net
Source 4 29 31 -2
Tests 3 256 0 +256
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 7 285 31 +254

What I checked:

Likely related people:

  • steipete: Assigned on the live PR, authored the two latest PR commits, and appears repeatedly in recent session and restart history around the touched files. (role: recent area contributor and PR follow-up owner; confidence: high; commits: 4521a0119da5, 147801f0b141, 5f68291f4f54; files: src/gateway/server-restart-sentinel.ts, src/gateway/server-restart-sentinel.test.ts, src/agents/command/session.ts)
  • 狼哥: Current blame for the central session freshness and restart-continuation source lines points to commit b789e71e57d9c023e91e8bbb318230bd8d0cb39a. (role: current source provenance; confidence: medium; commits: b789e71e57d9; files: src/agents/command/session.ts, src/auto-reply/reply/session.ts, src/gateway/server-restart-sentinel.ts)
  • gumadeiras: Recent history shows restart-sentinel routing and account-preservation changes in the same gateway area. (role: adjacent restart-sentinel contributor; confidence: medium; commits: 9b44929f285b, 9c44f100266e; files: src/gateway/server-restart-sentinel.ts, src/gateway/server-restart-sentinel.test.ts)
  • mcaxtr: Recent session routing/default work touched initSessionState and related tests near the reply-session side of this bug. (role: adjacent session-init contributor; confidence: medium; commits: 458a52610a4d; files: src/auto-reply/reply/session.ts, src/auto-reply/reply/session.test.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. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. 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 May 27, 2026
@clawsweeper

clawsweeper Bot commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?
  • The egg game starts only after the PR passes the real-behavior proof check.
  • Before that, no creature or rarity is rolled. The treat waits for real proof.
  • This is still just collectible flavor: proof affects review readiness, not creature quality.

@TurboTheTurtle
TurboTheTurtle force-pushed the fix/terminal-session-routing-86593 branch from ad627c7 to b5a8736 Compare May 27, 2026 18:52
@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 27, 2026
@TurboTheTurtle
TurboTheTurtle marked this pull request as ready for review May 27, 2026 18:59
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels May 27, 2026
@TurboTheTurtle
TurboTheTurtle force-pushed the fix/terminal-session-routing-86593 branch from b5a8736 to 6689560 Compare May 27, 2026 19:14
@TurboTheTurtle

TurboTheTurtle commented May 27, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented May 27, 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.

@TurboTheTurtle

TurboTheTurtle commented May 27, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.

Re-review progress:

@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. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels May 27, 2026
@steipete steipete self-assigned this May 27, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 27, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed 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: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels May 27, 2026
@openclaw-barnacle openclaw-barnacle Bot added triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. proof: supplied External PR includes structured after-fix real behavior proof. and removed proof: supplied External PR includes structured after-fix real behavior proof. triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels May 27, 2026
@steipete steipete changed the title fix(sessions): avoid terminal session reuse fix(sessions): avoid stale restart continuation reuse May 27, 2026
@steipete

steipete commented May 27, 2026

Copy link
Copy Markdown
Contributor

Verification for final head 4521a0119da537b91d7402e8f07898785f3f7d9e:

Known proof gap: no live Telegram Desktop restart was run locally. The restart-sentinel routing/fallback behavior is covered by focused tests and the PR body includes copied real local resolveSession output for completed-run session reuse.

@steipete
steipete merged commit d8641a6 into openclaw:main May 27, 2026
157 of 171 checks passed
@steipete steipete mentioned this pull request May 27, 2026
Closed
@TurboTheTurtle
TurboTheTurtle deleted the fix/terminal-session-routing-86593 branch May 28, 2026 02:06
@Haderach-Ram Haderach-Ram mentioned this pull request May 28, 2026
Open
@clawsweeper clawsweeper Bot mentioned this pull request Jul 5, 2026
Draft
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 gateway Gateway runtime mantis: telegram-visible-proof Mantis should capture Telegram visible proof. 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: 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.

Inbound routing and restart-sentinel resume reattach to status:done sessions

2 participants

@TurboTheTurtle @steipete