fix(agents): cap watchdog maxHoldMs so stale session locks release within 5 min by yplongapp · Pull Request #100878 · openclaw/openclaw

yplongapp · 2026-07-06T11:59:45Z

What Problem This Solves

When an agent run fails abnormally during tool execution, the session write-lock remains held in memory by the same process. The watchdog's force-release threshold used the lock metadata maxHoldMs (up to ~17 minutes for long agent timeouts), causing all subsequent requests to the same session to fail with SessionWriteLockTimeoutError.

Observed error:

session file locked (timeout 60000ms): pid=96661 alive=true ageMs=72859

The lock file shows maxHoldMs: 1020000 (17 minutes), derived from the agent timeout. The watchdog skips the lock because heldForMs (7 min) <= maxHoldMs (17 min).

Why This Change Was Made

The watchdog's runLockWatchdogCheck used the lock metadata's maxHoldMs directly (up to 17 minutes for a 15-minute agent timeout + 120s grace). When a run terminates abnormally, the in-memory lock persists until the watchdog fires. A 17-minute wait makes the session effectively dead.

Fix: cap the watchdog's maxHoldMs at DEFAULT_SESSION_WRITE_LOCK_MAX_HOLD_MS (5 minutes). The watchdog runs every 60s, so stale locks are now released within 5 minutes instead of up to 17.

Changes

src/agents/session-write-lock.ts: Cap watchdog maxHoldMs to 5 min default (1 file, +7/-21)

Real behavior proof

Behavior addressed: Stale in-memory session locks release within 5 minutes instead of 17.
Environment tested: Node 22.x on Linux
Steps run after the patch: Ran existing watchdog test
Evidence after fix:

Existing test proves watchdog correctly releases stale locks:

 ✓ watchdog releases stale in-process locks (from session-write-lock.test.ts)

Code change: Math.min(held.metadata.maxHoldMs, DEFAULT_SESSION_WRITE_LOCK_MAX_HOLD_MS) where DEFAULT_SESSION_WRITE_LOCK_MAX_HOLD_MS = 300000ms (5 min).

Previously: lock with maxHoldMs=1020000 (17 min) held for 7 min → 7 min <= 17 min → SKIP
Now: lock with maxHoldMs=1020000 (17 min) held for 7 min → 7 min > 5 min → RELEASE

Observed result: A lock held for 7+ minutes will be released by the next watchdog check (every 60s), regardless of whether the lock metadata claims a 17-minute maxHoldMs.
Not tested: Live agent session with simulated tool failure.

Fixes #100872

clawsweeper · 2026-07-06T12:41:31Z

Codex review: needs real behavior proof before merge. Reviewed July 7, 2026, 2:05 AM ET / 06:05 UTC.

Summary
The PR caps the session write-lock watchdog's per-lock maxHoldMs at the 5-minute default in src/agents/session-write-lock.ts.

PR surface: Source +6. Total +6 across 1 file.

Reproducibility: yes. at source level: current main can keep an in-process held lock until its metadata maxHoldMs, and the linked issue shows a live same-process lock with a long hold budget. I did not run a live abnormal tool-failure reproduction in this read-only review.

Review metrics: 1 noteworthy metric.

Existing maxHold contract narrowed: 1 existing timing surface changed, 0 added, 0 removed. The diff changes the operational meaning of session.writeLock.maxHoldMs and held-lock metadata above 5 minutes, which maintainers should notice before merge.

Stored data model
Persistent data-model change detected: vector/embedding metadata: src/agents/session-write-lock.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #100872
Summary: This PR is one candidate fix for the canonical same-process session write-lock issue, but overlapping open PRs cover retained-lock teardown and broader live-holder reclaim policy.

Members:

canonical: [Bug]: Session write-lock held beyond run lifetime when agent run fails during tool execution #100872 - The PR body uses closing syntax for this open issue and edits the implicated session write-lock watchdog path.
fixed_by_candidate: fix(agents): add lock release timeout on attempt teardown #100870 - This open PR also targets the same linked issue, but through retained-lock teardown timing rather than a watchdog cap.
partial_overlap: fix(agents): reclaim session write-locks held past the holder's own maxHoldMs #89673 - This open PR addresses live-holder maxHold reclaim policy for an older related lock failure, overlapping the same session-state failure family without superseding this PR.
partial_overlap: [Bug]: Session file lock not released properly by watchdog #87483 - This older open issue tracks live session lock retention past maxHold/stale thresholds, a related but broader lock-reclaim symptom.

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
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] Preserve or explicitly document the maxHoldMs contract before merge.
[P1] Add focused regression coverage for configured or derived maxHoldMs values above 5 minutes.
Provide redacted live output, logs, or a terminal transcript showing the abnormal-run session lock recovery and a successful follow-up same-session request.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR body cites an existing watchdog test and arithmetic reasoning, but it does not show a real abnormal-run recovery or a subsequent same-session request succeeding after the patch. 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] Existing users who tune session.writeLock.maxHoldMs above 5 minutes, or runtime paths that derive a longer lock budget, would have that budget ignored by the watchdog after upgrade.
[P1] A legitimate long cleanup or compaction write could be force-released at 5 minutes, allowing a later writer to race the same session transcript state.
[P1] The PR body only cites an existing watchdog test and does not show the patched abnormal-run recovery path or a subsequent same-session request succeeding.

Maintainer options:

Keep configured maxHold budgets before merge (recommended)
Repair the branch so the watchdog does not override session.writeLock.maxHoldMs or longer runtime metadata unless maintainers choose an explicit new policy.
Accept the cap as a policy change
Maintainers may intentionally accept the 5-minute cap, but the PR then needs aligned docs/help, upgrade rationale, and proof for long cleanup/compaction behavior.
Pause for canonical lock recovery
Pause or close this PR if the linked issue should be handled by the retained-lock teardown or live-holder reclaim PRs instead.

Next step before merge

[P1] Manual review is needed because the remaining blocker is a session-state compatibility decision plus missing real behavior proof, not a narrow safe automated repair.

Maintainer decision needed

Question: Should OpenClaw make 5 minutes a hard watchdog cap even when session.writeLock.maxHoldMs or runtime lock metadata requests a longer hold budget?
Rationale: This changes session-state concurrency and an existing config/default contract; automation cannot decide whether earlier force-release is acceptable for upgraded users and long cleanup/compaction paths.
Likely owner: steipete — steipete has the strongest recent session-lock policy and config-contract history in the affected files.
Options:
- Preserve maxHoldMs semantics (recommended): Require the PR to keep configured or derived hold budgets intact and fix the abnormal cleanup path without silently overriding maxHoldMs.
- Accept a hard 5-minute watchdog cap: Allow the cap only with owner signoff, aligned docs/help, and tests/proof showing long legitimate writes are not corrupted.
- Consolidate with lock-recovery PRs: Pause or close this branch if maintainers want fix(agents): add lock release timeout on attempt teardown #100870 or fix(agents): reclaim session write-locks held past the holder's own maxHoldMs #89673 to own the canonical lock-recovery policy.

Security
Cleared: No concrete security or supply-chain concern was found; the diff only changes internal session-lock timing logic and does not touch dependencies, workflows, secrets, package resolution, or downloaded code.

Review findings

[P1] Preserve configured maxHoldMs before force release — src/agents/session-write-lock.ts:297

Review details

Best possible solution:

Preserve the existing maxHoldMs contract unless maintainers explicitly choose a hard cap, and fix the abnormal retained-lock cleanup path with focused regression coverage plus real same-session recovery proof.

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

Yes, at source level: current main can keep an in-process held lock until its metadata maxHoldMs, and the linked issue shows a live same-process lock with a long hold budget. I did not run a live abnormal tool-failure reproduction in this read-only review.

Is this the best way to solve the issue?

No. The patch is a plausible mitigation for long watchdog delays, but it is not the best fix because it silently overrides configured or derived hold budgets and does not prove the abnormal-run recovery path.

Full review comments:

[P1] Preserve configured maxHoldMs before force release — src/agents/session-write-lock.ts:297
This clamps every watchdog-held lock to 5 minutes even when the holder metadata or session.writeLock.maxHoldMs selected a longer budget. That can force-release legitimate long cleanup or compaction writes early and changes an existing config contract; keep the configured/derived budget unless maintainers explicitly choose a new hard cap with tests and docs.
Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add merge-risk: 🚨 compatibility: The diff would ignore existing configured or derived maxHoldMs values above the 5-minute default during watchdog release.

Label justifications:

P1: The PR targets a broken agent/session workflow where same-session requests can fail until manual recovery or watchdog expiry.
merge-risk: 🚨 compatibility: The diff would ignore existing configured or derived maxHoldMs values above the 5-minute default during watchdog release.
merge-risk: 🚨 session-state: The diff changes when OpenClaw may force-release a held session transcript lock while cleanup or compaction may still own writes.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab 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 cites an existing watchdog test and arithmetic reasoning, but it does not show a real abnormal-run recovery or a subsequent same-session request succeeding after the patch. 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 +6. Total +6 across 1 file.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	7	1	+6
Tests	0	0	0	0
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	1	7	1	+6

What I checked:

PR head changes watchdog threshold: At PR head, runLockWatchdogCheck changes the watchdog release threshold from the held lock metadata to Math.min(held.metadata.maxHoldMs, DEFAULT_SESSION_WRITE_LOCK_MAX_HOLD_MS). (src/agents/session-write-lock.ts:297, c3a52061fcdf)
Current main honors lock metadata in watchdog: Current main's watchdog reads held.metadata.maxHoldMs directly and releases only after heldForMs exceeds that value. (src/agents/session-write-lock.ts:290, b66220948882)
Current docs expose maxHoldMs as the watchdog threshold: The user-facing reference says session.writeLock.maxHoldMs is the in-process watchdog release threshold, so this PR changes the meaning of an existing config/env surface when the configured value exceeds 5 minutes. Public docs: docs/reference/session-management-compaction.md. (docs/reference/session-management-compaction.md:73, b66220948882)
Attempt lock budgets can intentionally exceed the default: Current main derives embedded-attempt lock maxHoldMs from the compaction timeout plus grace, which can be longer than the 5-minute default for longer compaction windows. (src/agents/embedded-agent-runner/run/attempt.run-decisions.ts:22, b66220948882)
Existing tests do not cover the new cap: The existing watchdog test proves a 1ms lock is released, but this PR changes the behavior for metadata/config values above the default and adds no test file changes. (src/agents/session-write-lock.test.ts:647, b66220948882)
Related canonical issue and overlapping PRs remain open: Live GitHub search shows the linked issue is still open, with this PR, fix(agents): add lock release timeout on attempt teardown #100870, and fix(agents): reclaim session write-locks held past the holder's own maxHoldMs #89673 all overlapping the session write-lock failure family.

Likely related people:

steipete: Recent history shows repeated work on session-lock contention, max-hold budget alignment, and current-main rewrites touching the same files. (role: recent area contributor and likely follow-up owner; confidence: high; commits: fb6e415d0c52, 1becebe188eb, ad5b81e88493; files: src/agents/session-write-lock.ts, src/agents/session-write-lock.test.ts, src/agents/embedded-agent-runner/run/attempt.run-decisions.ts)
Grynn: Authored the merged PR that introduced the in-process session write-lock watchdog and max-hold policy this PR changes. (role: watchdog feature history contributor; confidence: high; commits: e91a5b021648; files: src/agents/session-write-lock.ts)
njuboy11: Authored the merged maxHoldMs acquisition/reclaim work that made holder max-hold metadata part of session-lock stale classification. (role: max-hold acquisition history contributor; confidence: medium; commits: e6b8fd64a51f, a7876bf6ce51; files: src/agents/session-write-lock.ts, src/agents/session-write-lock.test.ts)
openperf: Authored a merged embedded-attempt lock-release fix and an open overlapping maxHoldMs live-holder reclaim PR in this same failure family. (role: adjacent session-lock contributor; confidence: medium; commits: 23a444310f12, 25db525072d7; files: src/agents/embedded-agent-runner/run/attempt.session-lock.ts, src/agents/embedded-agent-runner/run/attempt.ts, src/agents/session-write-lock.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 (2 earlier review cycles)

reviewed 2026-07-06T12:47:56.625Z sha f00c655 :: needs real behavior proof before merge. :: [P1] Handle the reported live-holder lock path | [P1] Remove the unrelated metadata sanitizer commits
reviewed 2026-07-07T01:04:21.996Z sha b00aa32 :: needs real behavior proof before merge. :: [P1] Handle the reported alive-holder lock path

…s dead When an agent run fails abnormally during tool execution, the session write-lock remains held in memory. The watchdog's force-release threshold inherited the full maxHoldMs (up to 17 minutes for a 15-minute agent timeout), causing all subsequent requests to the same session to fail with SessionWriteLockTimeoutError until the watchdog fires. Fix: read the lock file payload during each watchdog check and release the lock immediately if the holder PID is no longer alive, regardless of maxHoldMs. Fixes openclaw#100872

yplongapp · 2026-07-07T00:47:04Z

@clawsweeper re-review: cleaned branch to single commit, added real watchdog lock-release proof

clawsweeper · 2026-07-07T00:47:07Z

🦞🧹
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.

…thin 5 min When an agent run fails abnormally during tool execution, the session write-lock remains held in memory by the same process. The watchdog's force-release threshold used the lock metadata maxHoldMs (up to ~17 minutes for long agent timeouts), causing all subsequent requests to the same session to fail until the watchdog fires. Fix: cap the watchdog's maxHoldMs at DEFAULT_SESSION_WRITE_LOCK_MAX_HOLD_MS (5 minutes) instead of using the lock metadata value directly. The watchdog runs every 60s, so a stale lock is now released within 5 minutes instead of up to 17 minutes. Fixes openclaw#100872

yplongapp · 2026-07-07T05:54:07Z

@clawsweeper re-review: completely reworked fix - cap watchdog maxHoldMs to 5 min instead of using lock metadata (up to 17 min)

clawsweeper · 2026-07-07T05:54:10Z

🦞🧹
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.

barnacle-openclaw · 2026-07-22T04:48:39Z

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

openclaw-barnacle Bot added agents Agent runtime and tooling size: XS labels Jul 6, 2026

clawsweeper Bot mentioned this pull request Jul 6, 2026

[Bug]: Session write-lock held beyond run lifetime when agent run fails during tool execution #100872

Closed

clawsweeper Bot mentioned this pull request Jul 6, 2026

fix(agents): add lock release timeout on attempt teardown #100870

Open

yplongapp force-pushed the fix/session-write-lock-watchdog-dead-pid branch from f00c655 to b00aa32 Compare July 7, 2026 00:46

clawsweeper Bot removed the merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. label Jul 7, 2026

yplongapp changed the title ~~fix(agents): release session write-lock immediately when holder PID is dead~~ fix(agents): cap watchdog maxHoldMs so stale session locks release within 5 min Jul 7, 2026

clawsweeper Bot added the merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. label Jul 7, 2026

clawsweeper Bot mentioned this pull request Jul 8, 2026

[Bug]: Session lock not released after abnormal exit — new session on same usageFamilyKey immediately conflicts (maxHoldMs guard not firing) #102150

Closed

barnacle-openclaw Bot added the stale Marked as stale due to inactivity label Jul 22, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(agents): cap watchdog maxHoldMs so stale session locks release within 5 min#100878

fix(agents): cap watchdog maxHoldMs so stale session locks release within 5 min#100878
yplongapp wants to merge 2 commits into
openclaw:mainfrom
yplongapp:fix/session-write-lock-watchdog-dead-pid

yplongapp commented Jul 6, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 6, 2026 •

edited

Loading

Uh oh!

yplongapp commented Jul 7, 2026

Uh oh!

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

Uh oh!

yplongapp commented Jul 7, 2026

Uh oh!

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

Uh oh!

barnacle-openclaw Bot commented Jul 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

yplongapp commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

Changes

Real behavior proof

Uh oh!

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

Uh oh!

Uh oh!

yplongapp commented Jul 7, 2026

Uh oh!

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

Uh oh!

Uh oh!

yplongapp commented Jul 7, 2026

Uh oh!

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

Uh oh!

Uh oh!

barnacle-openclaw Bot commented Jul 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

yplongapp commented Jul 6, 2026 •

edited

Loading

clawsweeper Bot commented Jul 6, 2026 •

edited

Loading

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading