Skip to content

fix(runtime): reconcile stale running sessions#96853

Open
moeedahmed wants to merge 7 commits into
openclaw:mainfrom
moeedahmed:codex/reconcile-stale-running-sessions
Open

fix(runtime): reconcile stale running sessions#96853
moeedahmed wants to merge 7 commits into
openclaw:mainfrom
moeedahmed:codex/reconcile-stale-running-sessions

Conversation

@moeedahmed

@moeedahmed moeedahmed commented Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

OpenClaw can leave a durable session row in status="running" after the underlying embedded run/task is already gone. In Telegram forum topics, that stale row makes the next message look like it is queued behind active work even when there is no live run to wait for, leaving the topic wedged.

Why This Change Was Made

The runtime already has several recovery paths for stuck sessions, restart recovery, abort handling, and delivery safety hooks, but they did not consistently reconcile persisted running session rows with the real active run/task registry. This PR adds a focused reconciliation path so rows that no longer have active work are closed instead of continuing to occupy a session lane.

User Impact

  • Telegram topics should recover when a previous run disappeared but the durable session row stayed running.
  • sessions.abort no-active-run cases should not leave the session permanently wedged.
  • Startup/stuck-session recovery gets a safer persisted-state cleanup path.
  • The change targets stale runtime bookkeeping; it does not change model routing, command syntax, or Telegram delivery semantics beyond avoiding stuck lanes.

Evidence

AI-assisted: yes. This came from local OpenClaw incident debugging where Hermes/Codex traced topic lanes stuck behind stale running rows.

Validation from the original PR branch:

  • node scripts/run-vitest.mjs run src/logging/diagnostic-stuck-session-recovery.integration.test.ts src/gateway/server-methods/sessions.abort-agent-scope.test.ts src/agents/main-session-restart-recovery.test.ts src/agents/command/delivery.test.ts — passed locally before PR creation
  • pnpm build — passed locally before PR creation

Runtime evidence from the local rollout that motivated the fix:

  • Telegram topic lane showed work ahead/active accounting even though no active run was present.
  • Abort/recovery paths reported no active cancellable work while persisted state still blocked subsequent turns.
  • After the reconciliation patch, stale running rows were cleared during recovery instead of leaving the topic wedged.

Known gap:

  • The PR body originally used older Summary / Root cause / Validation headings. This update converts it to OpenClaw's current contributor format so the real-behavior-proof gate has durable authored problem/evidence sections.

@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime agents Agent runtime and tooling size: L triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jun 25, 2026
@clawsweeper

clawsweeper Bot commented Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 7, 2026, 10:03 AM ET / 14:03 UTC.

Summary
The branch adds stale persisted running-session reconciliation across abort, startup recovery, diagnostic stuck-session recovery, and final-delivery fallback paths, with durable recovery metadata and regression tests.

PR surface: Source +529, Tests +512. Total +1041 across 13 files.

Reproducibility: yes. from source: seed a persisted running row with no active run or queued/running task and exercise sessions.abort no-active-run or startup recovery. I did not run the live Telegram scenario in this read-only review.

Review metrics: 2 noteworthy metrics.

  • Durable session metadata: 4 added. The PR persists new recovery markers on session rows, so upgrade and session-state behavior should be checked before merge.
  • Recovery entry points: 4 paths changed. Abort, startup, diagnostic recovery, and final-delivery fallback now share stale-session reconciliation, increasing the blast radius of a false stale decision.

Stored data model
Persistent data-model change detected: serialized state: src/agents/command/delivery.ts, serialized state: src/agents/session-running-reconciliation.ts, serialized state: src/config/sessions/types.ts, serialized state: src/gateway/server-methods/sessions.abort-agent-scope.test.ts, serialized state: src/gateway/server-methods/sessions.ts, serialized state: src/logging/diagnostic-stuck-session-recovery.integration.test.ts, and 4 more. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
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:

  • [P1] Add current-head redacted runtime logs, terminal output, copied live output, or Telegram Desktop proof showing stale-row recovery and next reply delivery.
  • Include before/after evidence that the row was running with no active work, then became non-running without blocking the follow-up turn.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: Local tests and rollout narrative are present, but the PR still needs current-head redacted runtime logs, terminal output, copied live output, or Telegram proof showing stale recovery and follow-up delivery; private details should be redacted before posting. 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
Native Telegram proof would show whether a wedged topic recovers and replies after stale-session reconciliation. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis telegram desktop proof: verify a topic wedged by a stale running session recovers and sends the next reply without manual reset.

Risk before merge

  • [P1] The PR terminalizes persisted running and processing rows from abort, startup, diagnostic, and final-delivery paths; a false stale decision could mark live work failed and change follow-up delivery.
  • [P1] The contributor proof gate is still unmet: current-head tests and CI are present, but no runtime or Telegram artifact shows stale recovery plus the next visible reply after the latest fixes.
  • [P1] The diff adds durable session metadata and changes interpretation of existing session rows, so upgrade/session-state behavior needs maintainer-visible proof before merge.

Maintainer options:

  1. Require Current-Head Proof (recommended)
    Ask for redacted runtime logs, terminal output, copied live output, or Telegram Desktop proof showing stale-row recovery and follow-up delivery on the latest head before merge.
  2. Accept The Runtime Risk
    Maintainers can intentionally land after reviewing source and CI only, while owning the remaining upgrade and delivery uncertainty.
  3. Pause For Cluster Choice
    Keep this PR paused if maintainers want to choose between this broader branch and the narrower overlapping stale-session repair first.

Next step before merge

  • [P1] The remaining blocker is current-head real behavior proof and maintainer proof sufficiency, not a narrow code repair ClawSweeper can safely supply.

Maintainer decision needed

  • Question: Should this PR wait for current-head runtime or Telegram proof before merge, or may maintainers accept source-only validation for this session recovery change?
  • Rationale: The patch appears code-correct, but it changes persisted session lifecycle and visible message recovery paths in an external PR without concrete current-head real-behavior proof.
  • Likely owner: jalehman — The remaining decision is mostly about persisted session lifecycle and accessor-boundary safety, where this history is strongest.
  • Options:
    • Require Current-Head Proof (recommended): Ask for redacted runtime logs, terminal output, copied live output, or Telegram Desktop proof showing the stale row is recovered and the next reply is delivered on this head.
    • Accept Source-Only Validation: A maintainer can intentionally override the proof gap and rely on green CI plus focused tests while owning the remaining runtime uncertainty.
    • Defer To The Stale-Session Cluster: Pause this PR while deciding whether the narrower overlapping stale-row PR should be repaired or replaced instead.

Security
Cleared: The diff changes internal TypeScript session recovery and tests without adding dependencies, workflows, package metadata, secret handling, downloads, or new code-execution surfaces.

Review details

Best possible solution:

Land a source fix that keeps reconciliation behind the session-accessor and task-status seams, preserves active and queued work, and includes current-head redacted runtime or Telegram proof for stale recovery and follow-up delivery.

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

Yes, from source: seed a persisted running row with no active run or queued/running task and exercise sessions.abort no-active-run or startup recovery. I did not run the live Telegram scenario in this read-only review.

Is this the best way to solve the issue?

Yes, with the proof gate still open. The generic runtime recovery boundary is the right layer and the latest head routes abort cleanup through the accessor, but merge should wait for real current-head behavior proof.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P1: The PR targets a stuck session lane that can block Telegram or other channel replies until manual recovery.
  • merge-risk: 🚨 compatibility: The diff changes how existing persisted running or processing session rows are interpreted during upgrades and recovery.
  • merge-risk: 🚨 session-state: The PR rewrites durable session lifecycle fields from abort, startup, diagnostic, and final-delivery paths.
  • merge-risk: 🚨 message-delivery: Incorrect stale-session reconciliation could unblock, suppress, or terminalize work in a way that changes follow-up reply delivery.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Local tests and rollout narrative are present, but the PR still needs current-head redacted runtime logs, terminal output, copied live output, or Telegram proof showing stale recovery and follow-up delivery; private details should be redacted before posting. 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 targets Telegram topic recovery and visible follow-up replies, which a short Telegram Desktop proof can demonstrate.
Evidence reviewed

PR surface:

Source +529, Tests +512. Total +1041 across 13 files.

View PR surface stats
Area Files Added Removed Net
Source 9 535 6 +529
Tests 4 514 2 +512
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 13 1049 8 +1041

What I checked:

Likely related people:

  • jalehman: Recent commits moved session access behind the accessor and added the boundary guard that this PR now has to satisfy. (role: session accessor and storage-boundary owner; confidence: high; commits: cb0d8a1294ea, 4e2a80cac34d, c7295e417d5d; files: src/config/sessions/session-accessor.ts, scripts/check-session-accessor-boundary.mjs)
  • steipete: Recent commits changed the gateway sessions surface and session lifecycle controls adjacent to the abort path touched by this PR. (role: recent gateway sessions contributor; confidence: medium; commits: 354a1515440b, 88f1ec38d4a5, 6df0fb818d67; files: src/gateway/server-methods/sessions.ts)
  • joshavant: Prior work introduced and maintained startup recovery behavior adjacent to the stale-running sweep this PR extends. (role: startup recovery contributor; confidence: medium; commits: e1978cf73cbd, 75db48a17522; files: src/agents/main-session-restart-recovery.ts, src/config/sessions/session-accessor.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.
Review history (4 earlier review cycles)
  • reviewed 2026-07-02T11:39:29.839Z sha e23aa6f :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-07T13:20:27.384Z sha 0daedfd :: needs real behavior proof before merge. :: [P1] Route abort reconciliation through the session accessor
  • reviewed 2026-07-07T13:48:28.690Z sha bfbfc10 :: needs real behavior proof before merge. :: [P1] Count tasks for every candidate session key
  • reviewed 2026-07-07T13:51:31.460Z sha bfbfc10 :: needs real behavior proof before merge. :: none

@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: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. 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 25, 2026
@moeedahmed

moeedahmed commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up local verification from the live incident:

  • Targeted stale-session regression suite passed locally: node scripts/run-vitest.mjs run src/logging/diagnostic-stuck-session-recovery.integration.test.ts src/gateway/server-methods/sessions.abort-agent-scope.test.ts src/agents/main-session-restart-recovery.test.ts src/agents/command/delivery.test.ts
  • Result: 3 Vitest shards passed, 112 tests passed.
  • Installed runtime bundle was rebuilt locally from this source branch and staged into the local OpenClaw install with a rollback backup.
  • Full pnpm build did not reach compilation locally because the wrapper stalled in its dependency/install phase; it was stopped rather than left running.

This PR remains the right upstream vehicle for the Telegram forum-topic stale running-session incident. The remaining red check is the Telegram visual proof workflow, not the targeted session lifecycle regression suite.

@moeedahmed

moeedahmed commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Review blocker follow-up pushed in 0a70727071e.

Addressed:

  • Restricted stale persisted-session reconciliation to actual in-progress states only: running and processing. Existing terminal timeout and killed rows are now preserved.
  • Typed and reserved the additional core-owned session metadata fields: staleRunningRecoveredAt and safeFallbackDelivered.
  • Added regression coverage for terminal status preservation in both sessions.abort no-active-run reconciliation and startup store-wide stale-running reconciliation.

Local verification:

  • node scripts/run-vitest.mjs run src/logging/diagnostic-stuck-session-recovery.integration.test.ts src/gateway/server-methods/sessions.abort-agent-scope.test.ts src/agents/main-session-restart-recovery.test.ts src/agents/command/delivery.test.ts
    • Passed: 3 Vitest shards, 5 files, 114 tests.
  • ./node_modules/.bin/oxfmt --check src/agents/session-running-reconciliation.ts src/config/sessions/types.ts src/plugins/session-entry-slot-keys.ts src/gateway/server-methods/sessions.abort-agent-scope.test.ts src/agents/main-session-restart-recovery.test.ts
    • Passed.
  • ./node_modules/.bin/oxlint src/agents/session-running-reconciliation.ts src/config/sessions/types.ts src/plugins/session-entry-slot-keys.ts src/gateway/server-methods/sessions.abort-agent-scope.test.ts src/agents/main-session-restart-recovery.test.ts
    • Passed.
  • node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.test.src.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/test-src.tsbuildinfo
    • Passed.

Remaining known blocker is still the Telegram visual proof workflow, not the code review blockers above.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 25, 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.

@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
@moeedahmed
moeedahmed marked this pull request as ready for review June 26, 2026 05:37
@moeedahmed
fix(runtime): keep stale session reconciliation behind task seams
f70d9bf 
Route running-task lookups through the approved task status access seam so the reconciliation helper does not import the task registry directly.
@moeedahmed

moeedahmed commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the failing task-boundaries CI check in a follow-up commit.

What changed:

  • Removed the direct task-registry.js import from src/agents/session-running-reconciliation.ts.
  • Routed the needed running-task lookups through the existing approved task-status-access seam.

Validation:

  • node scripts/run-vitest.mjs src/tasks/task-boundaries.test.ts src/logging/diagnostic-stuck-session-recovery.runtime.test.ts — passed, 26 tests
  • node_modules/.bin/oxfmt --check --threads=1 src/agents/session-running-reconciliation.ts src/tasks/task-status-access.ts src/tasks/task-boundaries.test.ts — passed
  • git diff --check — passed

This should clear the previous checks-node-compact-large-whole-1 failure.

@moeedahmed

moeedahmed commented Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the ClawSweeper P1 queued-task blocker in a follow-up commit.

What changed:

  • Stale persisted-session reconciliation now treats both queued and running task records as active work before marking a persisted running row failed.
  • Added abort-path coverage proving a queued session task preserves the persisted running row when abort finds no active run.
  • Added startup-recovery coverage proving a queued topic task blocks store-wide stale-running reconciliation.

Validation:

  • node scripts/run-vitest.mjs src/tasks/task-boundaries.test.ts src/gateway/server-methods/sessions.abort-agent-scope.test.ts src/agents/main-session-restart-recovery.test.ts — passed, 3 Vitest shards, 88 tests.
  • ./node_modules/.bin/oxfmt --check --threads=1 src/agents/session-running-reconciliation.ts src/gateway/server-methods/sessions.abort-agent-scope.test.ts src/agents/main-session-restart-recovery.test.ts — passed.
  • ./node_modules/.bin/oxlint src/agents/session-running-reconciliation.ts src/tasks/task-status-access.ts src/tasks/task-boundaries.test.ts src/gateway/server-methods/sessions.abort-agent-scope.test.ts src/agents/main-session-restart-recovery.test.ts — passed.
  • node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.test.src.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/test-src.tsbuildinfo — passed.

Remaining known blocker is still current-head Telegram/runtime visual proof, not this queued-task code finding.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 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.

This was referenced Jun 29, 2026
Closed
Closed
@clawsweeper clawsweeper Bot mentioned this pull request Jun 30, 2026
Closed
25 tasks
@clawsweeper clawsweeper Bot mentioned this pull request Jul 2, 2026
Closed
@moeedahmed

moeedahmed commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

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

@mjnkao mjnkao mentioned this pull request Jul 9, 2026
Closed
@barnacle-openclaw

barnacle-openclaw Bot commented Jul 22, 2026

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@barnacle-openclaw barnacle-openclaw Bot added the stale Marked as stale due to inactivity label Jul 22, 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

agents Agent runtime and tooling gateway Gateway runtime mantis: telegram-visible-proof Mantis should capture Telegram visible proof. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. 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. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XL stale Marked as stale due to inactivity 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