fix: force-release session .jsonl.lock on dispose when retained-lock user is stuck by joelnishanth · Pull Request #95854 · openclaw/openclaw

joelnishanth · 2026-06-22T20:19:43Z

What Problem This Solves

Fixes an issue where sessions would permanently break with "Something went wrong" after a subagent abort-settle timeout, because the .jsonl.lock file was never released. Users had to manually delete the lock file to restore the session.

Why This Change Was Made

disposeHeldLockAfterRetainedIdle() is the final lock cleanup path called from the attempt's outer finally block. Previously, when waitForRetainedLockIdle() returned false (caller inside an active write-lock scope) or waited indefinitely (a stuck operation never released its retained lock), dispose would return without releasing the physical lock — leaving a stale .jsonl.lock that blocked the session permanently.

The fix introduces a bounded variant waitForRetainedLockIdleWithTimeout() that adds a 5-second timeout to the retained-lock-idle wait. If the wait returns false (either because the caller is inside an active write-lock scope or the timeout fires), dispose now force-releases the lock anyway — since dispose is the final cleanup path and no more writes will happen.

User Impact

Sessions that previously required manual .jsonl.lock removal after subagent abort-settle timeouts now recover automatically. The session lock is always released on every exit path of the attempt lifecycle.

Evidence

Test: pnpm test -- src/agents/embedded-agent-runner/run/attempt.session-lock.test.ts

$ node node_modules/.bin/vitest run src/agents/embedded-agent-runner/run/attempt.session-lock.test.ts --reporter=verbose
 ✓ force-releases on dispose when retained-lock user is stuck (#95833) 5082ms
 Test Files  1 passed (1)
      Tests  106 passed (106)

Regression test: The new test force-releases on dispose when retained-lock user is stuck (#95833) simulates a stuck withSessionWriteLock callback that never completes (like a hung model call). It verifies that dispose() force-releases the physical lock after the 5-second timeout instead of hanging indefinitely.

All related tests pass:

$ node node_modules/.bin/vitest run src/agents/embedded-agent-runner/run/attempt.session-lock.test.ts src/agents/embedded-agent-runner/run/attempt-abort.test.ts src/agents/embedded-agent-runner/run/attempt.subscription-cleanup.test.ts src/agents/embedded-agent-runner/runs.test.ts --reporter=verbose
 Test Files  4 passed (4)
      Tests  140 passed (140)

— Joel Nishanth · offlyn.AI

…user is stuck (openclaw#95833)

clawsweeper · 2026-06-23T16:13:15Z

Codex review: needs real behavior proof before merge. Reviewed June 23, 2026, 12:19 PM ET / 16:19 UTC.

Summary
The PR adds a 5-second bounded dispose wait that force-releases a retained session write lock and adds a regression test for the stuck retained-lock case.

PR surface: Source +42, Tests +35. Total +77 across 2 files.

Reproducibility: yes. Source inspection on current main shows a retained withSessionWriteLock() operation can keep retainedLockUseCount nonzero while dispose either waits indefinitely or returns before releasing the physical lock; the linked issue supplies production-style steps, but I did not run a live gateway reproduction in this read-only review.

Review metrics: 1 noteworthy metric.

Disposal Timeout: 1 added, 5,000 ms. This hard-coded lifecycle timeout changes when an advisory session lock can be released while retained users are still active.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #95833
Summary: This PR is the open implementation candidate for the stale retained session-lock issue.

Members:

canonical: Subagent abort-settle fails to release .jsonl.lock, permanently breaking the session #95833 - The PR body uses closing syntax for the issue, and the issue describes the same retained .jsonl.lock failure.

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦪 silver shellfish
Patch quality: 🧂 unranked krab
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] Fix or explicitly owner-approve the force-release semantics for active retained writers.
[P1] Add redacted live/runtime proof that the stale-lock recovery works in an affected session.
Update the PR body after adding proof so ClawSweeper can re-review automatically, or ask a maintainer to comment @clawsweeper re-review.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR body provides Vitest output and CI has checks green, but there is no redacted live/runtime proof that an affected session recovers after the fix; screenshots, terminal output, copied live output, recordings, linked artifacts, or redacted logs would count after private details are removed. 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

[P2] The 5-second timeout can release .jsonl.lock while a delayed retained writer still owns the logical write scope, allowing another attempt to acquire the sidecar before that writer resumes.
[P1] Contributor evidence is unit-test output only; no redacted live/runtime proof shows an affected session recovering after this cleanup path.

Maintainer options:

Preserve Active-Writer Safety (recommended)
Change the timeout path so force-release happens only after the retained writer is provably terminal, or add an owner-approved terminal state that prevents any late transcript mutation after release.
Accept Terminal Cleanup Tradeoff
Maintainers can intentionally accept the 5-second force-release behavior if they document and test the risk that a late retained writer is cut off or ignored.
Pause For Owner Decision
If the retained-writer semantics are not settled, leave the PR open or pause it until the session-lock owner chooses the recovery contract.

Next step before merge

[P1] The remaining blocker is a session-lock lifecycle decision plus contributor real behavior proof, not a safe automated repair task.

Security
Cleared: The diff only changes internal TypeScript lock lifecycle code and a focused test; I found no concrete security or supply-chain regression.

Review findings

[P1] Preserve retained-writer safety before force-releasing — src/agents/embedded-agent-runner/run/attempt.session-lock.ts:1693-1700

Review details

Best possible solution:

Keep the stale-lock recovery, but preserve or explicitly product-approve the active-writer safety invariant before merge, then add focused regression coverage plus redacted live/runtime proof of session recovery.

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

Yes. Source inspection on current main shows a retained withSessionWriteLock() operation can keep retainedLockUseCount nonzero while dispose either waits indefinitely or returns before releasing the physical lock; the linked issue supplies production-style steps, but I did not run a live gateway reproduction in this read-only review.

Is this the best way to solve the issue?

No, not yet. The patch targets the right owner path for the stale lock, but treating a slow retained writer as dead after 5 seconds conflicts with the existing active-writer invariant and the sidecar-lock recovery contract unless maintainers explicitly accept that terminal cleanup tradeoff.

Full review comments:

[P1] Preserve retained-writer safety before force-releasing — src/agents/embedded-agent-runner/run/attempt.session-lock.ts:1693-1700
This timeout branch releases the physical sidecar while retained-lock users are still active. Existing abort/cleanup paths wait for retained writers before releasing or taking the lock, and the sidecar-lock contract only allows recovery removal after proving the holder cannot still write; if the retained callback resumes after five seconds, another attempt may already hold the same session lock and both paths can mutate the transcript.
Confidence: 0.82

Overall correctness: patch is incorrect
Overall confidence: 0.82

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add P1: The linked bug and proposed fix affect real sessions becoming unusable or racing after stale transcript locks.
add merge-risk: 🚨 message-delivery: The stale-lock bug drops subsequent messages today, while the proposed force-release path could also race message persistence if the retained writer resumes late.
add merge-risk: 🚨 session-state: Merging this PR can let a late retained writer mutate session transcript state after the physical lock has been released.
add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
add 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 provides Vitest output and CI has checks green, but there is no redacted live/runtime proof that an affected session recovers after the fix; screenshots, terminal output, copied live output, recordings, linked artifacts, or redacted logs would count after private details are removed. 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.

Label justifications:

P1: The linked bug and proposed fix affect real sessions becoming unusable or racing after stale transcript locks.
merge-risk: 🚨 session-state: Merging this PR can let a late retained writer mutate session transcript state after the physical lock has been released.
merge-risk: 🚨 message-delivery: The stale-lock bug drops subsequent messages today, while the proposed force-release path could also race message persistence if the retained writer resumes late.
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 real behavior proof before merge: The PR body provides Vitest output and CI has checks green, but there is no redacted live/runtime proof that an affected session recovers after the fix; screenshots, terminal output, copied live output, recordings, linked artifacts, or redacted logs would count after private details are removed. 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 +42, Tests +35. Total +77 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	43	1	+42
Tests	1	35	0	+35
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	78	1	+77

What I checked:

Repository policy applied: Root and scoped agent-runner AGENTS.md files were read fully; the review applied the agents-path guidance for whole-path review, dependency contract checks, real behavior proof, and session-state merge risk. (AGENTS.md:1, 9ff7abc8989c)
Current main source reproduction: Current main has an unbounded retained-lock idle wait and disposeHeldLockAfterRetainedIdle() returns before releasing when that wait fails, matching the source-level stale .jsonl.lock failure. (src/agents/embedded-agent-runner/run/attempt.session-lock.ts:1311, 9ff7abc8989c)
Linked issue context: The linked issue includes production-style reproduction steps and a later correction narrowing the problem to the retained held-lock controller path; this PR uses closing syntax for that issue.
Patch changes active-writer release semantics: The PR's new timeout branch force-releases heldLock whenever retained users are not idle after 5 seconds, without proving the retained writer cannot still resume. (src/agents/embedded-agent-runner/run/attempt.session-lock.ts:1693, 5ca776e5aacd)
Sibling invariant currently waits for retained writers: Existing tests assert that abort release and cleanup acquisition wait for active retained-lock writes before releasing or taking the lock, so this PR creates a special dispose-only exception to that invariant. (src/agents/embedded-agent-runner/run/attempt.session-lock.test.ts:403, 9ff7abc8989c)
Dependency contract warns against unsafe stale removal: The bundled @openclaw/[email protected] sidecar-lock docs say stale lock removal should happen only through an application-owned recovery path after proving the holder cannot still be writing; the PR timeout path does not establish that proof.

Likely related people:

vincentkoc: Current-main blame for the retained-lock wait, dispose path, and write-lock path points to a recent commit authored and committed by vincentkoc; older provenance is limited by the grafted checkout. (role: recent current-source contributor; confidence: medium; commits: 7cc0879d0e80; files: src/agents/embedded-agent-runner/run/attempt.session-lock.ts, src/agents/embedded-agent-runner/run/attempt.session-lock.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.

sallyom · 2026-06-25T00:49:36Z

Thanks for taking this on. I rebased and reviewed this against latest main locally, including #96100.

I'm closing this PR rather than proceed with the force-release approach. The concern is that releasing the physical .jsonl.lock while a retained withSessionWriteLock callback may still resume can break the session write-lock serialization contract and allow late transcript writes after another owner has acquired the lock.

#96100 has now merged and addresses the retained-lock abort cleanup path we believe was gating #95833. We’ll keep #95833 open while the reporter validates whether that fully resolves the stale lock / heap growth behavior.

Thanks again for this contribution!

fix: force-release session .jsonl.lock on dispose when retained-lock …

be7758e

…user is stuck (openclaw#95833)

openclaw-barnacle Bot added the agents Agent runtime and tooling label Jun 22, 2026

joelnishanth mentioned this pull request Jun 22, 2026

Subagent abort-settle fails to release .jsonl.lock, permanently breaking the session #95833

Closed

openclaw-barnacle Bot added the size: S label Jun 22, 2026

fix: resolve no-promise-executor-return lint in test

5ca776e

clawsweeper Bot mentioned this pull request Jun 24, 2026

fix(agent): replace self-wait with deferred release in retained-lock abort cleanup #96100

Merged

sallyom mentioned this pull request Jun 25, 2026

fix: force-release session lock on dispose to prevent orphan locks #90419

Closed

sallyom closed this Jun 25, 2026

wangmiao0668000666 mentioned this pull request Jun 28, 2026

Heap not released on embedded run abort (released=0); session write lock persists after abort cleanup #95915

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix: force-release session .jsonl.lock on dispose when retained-lock user is stuck#95854

fix: force-release session .jsonl.lock on dispose when retained-lock user is stuck#95854
joelnishanth wants to merge 2 commits into
openclaw:mainfrom
joelnishanth:fix/abort-settle-lock-release-95833

joelnishanth commented Jun 22, 2026

Uh oh!

clawsweeper Bot commented Jun 23, 2026 •

edited

Loading

Uh oh!

sallyom commented Jun 25, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

joelnishanth commented Jun 22, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

clawsweeper Bot commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sallyom commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Jun 23, 2026 •

edited

Loading

sallyom commented Jun 25, 2026 •

edited

Loading