fix(agents): guard session prompt ownership by TurboTheTurtle · Pull Request #85955 · openclaw/openclaw

TurboTheTurtle · 2026-05-24T06:28:36Z

Summary

serialize embedded runner prompt releases by resolved session JSONL file
keep the file-scoped prompt holder on true provider prompt windows only
release the post-prompt compaction wait without registering a prompt-active holder, so later same-file prompt releases cannot wait on stale ownership
add regressions for both same-file prompt serialization and the compaction-wait lifecycle

Verification

node scripts/run-vitest.mjs src/agents/pi-embedded-runner/run/attempt.session-lock.test.ts
- Passed: 2 files, 68 tests, Duration 1.24s after rebasing onto latest upstream/main.
pnpm check:test-types
- Passed after rebasing onto latest upstream/main.
node scripts/run-vitest.mjs src/agents/pi-embedded-runner/run/attempt.session-lock.test.ts src/logging/diagnostic-stuck-session-recovery.runtime.test.ts src/infra/heartbeat-runner.skips-busy-session-lane.test.ts src/agents/model-fallback.test.ts src/agents/failover-error.test.ts
- Passed before the upstream rebase: 7 files, 300 tests, Duration 51.12s.
git diff --check
pnpm exec oxfmt --check --threads=1 src/agents/pi-embedded-runner/run/attempt.session-lock.ts src/agents/pi-embedded-runner/run/attempt.session-lock.test.ts src/agents/pi-embedded-runner/run/attempt.ts

Real behavior proof

Behavior or issue addressed: duplicate embedded attempts for the same resolved session JSONL file could both release their write locks for provider prompt streaming at the same time. A follow-up review also found that the post-prompt compaction wait path must not leave a stale prompt holder after it reacquires with withSessionWriteLock().
Real environment tested: local OpenClaw checkout on macOS arm64, Node v24.15.0, branch fix/session-file-prompt-guard-85913, using the real acquireSessionWriteLock implementation against a temporary session JSONL file.
Exact steps or command run after this patch: node --import tsx --input-type=module with an inline runtime script that created two createEmbeddedAttemptSessionLockController instances for the same temp session.jsonl; the first controller called releaseForSessionIdleWait() and then reacquired through withSessionWriteLock(), then the second controller called releaseForPrompt().
Evidence after fix (screenshot, recording, terminal capture, console output, redacted runtime log, linked artifact, or copied live output): copied live terminal output from that command:

{
  "node": "v24.15.0",
  "platform": "darwin/arm64",
  "sessionFile": "<temp>/session.jsonl",
  "events": [
    "first released for compaction wait without prompt holder",
    "first reacquired through withSessionWriteLock",
    "second released for prompt after first compaction wait",
    "second released after compaction path: true",
    "takeover first=false second=false"
  ]
}

Observed result after fix: the second same-file controller released for prompt after the first controller's compaction-wait path reacquired through withSessionWriteLock(); it did not wait on a stale prompt holder. Both controllers reported hasSessionTakeover() === false.
What was not tested: no additional gaps.

Authorship

Please preserve author attribution if this PR is squashed or reworked, or include:

Co-authored-by: Andy Ye <35905412+TurboTheTurtle@users.noreply.github.com>

clawsweeper · 2026-05-24T06:30:37Z

Codex review: needs maintainer review before merge. Reviewed May 25, 2026, 5:20 PM ET / 21:20 UTC.

Summary
The PR adds per-session-file prompt-turn serialization to the embedded runner session-lock controller, adds a non-prompt idle-wait release path, updates the compaction wait caller, and extends session-lock regressions.

PR surface: Source +97, Tests +231. Total +328 across 3 files.

Reproducibility: yes. source-level: current main releases the held write lock around provider prompt streaming without a same-session-file prompt turn guard, and the linked issue plus PR tests describe the cross-lane race. I did not run a live gateway repro because this review was read-only.

Review metrics: 1 noteworthy metric.

Internal Lock Lifecycle Surface: 1 controller method added. The new releaseForSessionIdleWait method is the key lifecycle split reviewers should check because it keeps compaction waits out of prompt ownership.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

none

Risk before merge

The patch intentionally serializes same-session-file provider prompt windows; if an earlier prompt window never releases, later same-file attempts can wait behind it until the existing lock/attempt timeout path fires.
This PR addresses the cross-lane prompt-window ownership race, while the related same-lane ALS ownership problem remains tracked separately at Hoist withOwnedSessionTranscriptWrites ALS scope to span agent.prompt() to fix vanilla-openclaw same-lane fence trip #86572.

Maintainer options:

Land After Focused Lock Review (recommended)
Accept the session-state and availability tradeoff if maintainers agree the per-session-file prompt queue is the right narrow fix for the cross-lane prompt-window race and checks pass on the final head.
Require Broader Runtime Proof
Ask for a live gateway or packaged-run proof if maintainers want confidence beyond the supplied real lock-controller script and focused regressions before merging.
Pause For ALS Follow-up Direction
Pause only if maintainers decide the same-lane ALS ownership work should supersede this cross-lane guard instead of remaining a separate follow-up.

Next step before merge
No automated repair remains; the next step is maintainer review of the session-lock tradeoff and final checks before merge.

Security
Cleared: No concrete security or supply-chain concern found; the diff is limited to in-process session-lock coordination and colocated tests.

Review details

Best possible solution:

Land the narrow prompt-window guard after focused maintainer review and green checks, while keeping the separate same-lane ALS follow-up in its own canonical issue.

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

Yes, source-level: current main releases the held write lock around provider prompt streaming without a same-session-file prompt turn guard, and the linked issue plus PR tests describe the cross-lane race. I did not run a live gateway repro because this review was read-only.

Is this the best way to solve the issue?

Yes for the cross-lane race: the PR keeps the fix inside the embedded session-lock owner, adds focused cleanup and ordering regressions, and leaves the separate same-lane ALS issue to its own follow-up.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied live output from a local OpenClaw checkout using the real session write-lock implementation against a temporary session JSONL file, which is sufficient for this internal runtime behavior.
add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied live output from a local OpenClaw checkout using the real session write-lock implementation against a temporary session JSONL file, which is sufficient for this internal runtime behavior.
remove rating: 🧂 unranked krab: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

P1: The linked bug affects live embedded agent/channel turns through session takeover or lock timeout paths that can drop replies or stall runs.
merge-risk: 🚨 session-state: The PR changes how concurrent embedded attempts coordinate ownership of the same session JSONL file during prompt release windows.
merge-risk: 🚨 availability: The new queueing path can intentionally make same-file attempts wait behind an earlier prompt window, so release and timeout behavior must be reviewed carefully.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied live output from a local OpenClaw checkout using the real session write-lock implementation against a temporary session JSONL file, which is sufficient for this internal runtime behavior.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied live output from a local OpenClaw checkout using the real session write-lock implementation against a temporary session JSONL file, which is sufficient for this internal runtime behavior.

Evidence reviewed

PR surface:

Source +97, Tests +231. Total +328 across 3 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	2	116	19	+97
Tests	1	231	0	+231
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	3	347	19	+328

What I checked:

Repository policy read: Root AGENTS.md and the scoped agent runner AGENTS.md files were read; their guidance made this a session-state/availability-sensitive PR review rather than a cleanup close. (AGENTS.md:1, f4cfa012e117)
Current main behavior: Current main's releaseForPrompt captures a fence and immediately releases the held session write lock, with no same-file prompt-turn guard before the release window. (src/agents/pi-embedded-runner/run/attempt.session-lock.ts:752, f4cfa012e117)
PR implementation evidence: The merge ref adds PromptSessionFileTurn state, a per-session-file turn tail map, and releaseHeldLockForUnlockedSessionWindow so prompt releases queue behind earlier same-file prompt windows while idle waits do not register as prompt holders. (src/agents/pi-embedded-runner/run/attempt.session-lock.ts:396, f6ea30fdf817)
Caller split evidence: The compaction post-prompt wait caller changes from releaseForPrompt to releaseForSessionIdleWait, matching the PR's distinction between provider prompt windows and idle compaction waits. (src/agents/pi-embedded-runner/run/attempt.ts:4276, f6ea30fdf817)
Regression evidence: The PR adds regressions for same-file prompt serialization, multi-waiter ordering, queued waiter reacquire timeout cleanup, and compaction idle wait lifecycle. (src/agents/pi-embedded-runner/run/attempt.session-lock.test.ts:551, f6ea30fdf817)
Git history provenance: Current releaseForPrompt/reacquireAfterPrompt lines blame to commit 915c820; the PR's merge ref is f6ea30f, merging e84381f into base 5dccba7. (src/agents/pi-embedded-runner/run/attempt.session-lock.ts:752, 915c820c38a2)

Likely related people:

Sebastien Tardif: Blame and -S history point to commit 915c820 as the current source of the embedded session-lock release/reacquire implementation and EmbeddedAttemptSessionTakeoverError path. (role: introduced current behavior; confidence: medium; commits: 915c820c38a2; files: src/agents/pi-embedded-runner/run/attempt.session-lock.ts)
Peter Steinberger: Recent commit 77d9ac3 touched the same session-lock file in current main while refactoring helper use, making him a nearby routing candidate but not the prompt-window behavior author. (role: recent adjacent contributor; confidence: low; commits: 77d9ac30bb8d; files: src/agents/pi-embedded-runner/run/attempt.session-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.

clawsweeper · 2026-05-24T06:37:33Z

ClawSweeper PR egg

✨ Hatched: 🥚 common Frosted Proofling

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

Merged PRs are hatchable.
Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

Rarity: 🥚 common.
Trait: finds missing screenshots.
Image traits: location status garden; accessory little merge flag; palette rose quartz and slate; mood curious; pose curling around a status light; shell polished stone shell; lighting calm overcast light; background tiny artifact crates.
Share on X: post this hatch
Copy: My PR egg hatched a 🥚 common Frosted Proofling in ClawSweeper.

What is this egg doing here?

Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

TurboTheTurtle · 2026-05-24T06:55:26Z

@clawsweeper re-review\n\nAddressed the P1 stale prompt-holder lifecycle finding. The post-prompt compaction wait now uses a non-prompt release path, and the new regression covers release for compaction wait followed by withSessionWriteLock before a second same-file releaseForPrompt. Updated real behavior proof is in the PR body.

clawsweeper · 2026-05-24T06:55:29Z

🦞👀
ClawSweeper picked this up.

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

TurboTheTurtle · 2026-05-24T07:02:30Z

@clawsweeper re-review\n\nRebased onto current upstream/main to pick up the unrelated document extractor type-test fix, and pushed the stale prompt-holder lifecycle fix on the new head. Local post-rebase checks: targeted session-lock Vitest, pnpm check:test-types, oxfmt check, and git diff --check.

clawsweeper · 2026-05-24T07:02:33Z

🦞👀
ClawSweeper picked this up.

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

TurboTheTurtle · 2026-05-24T07:14:55Z

@clawsweeper re-review

clawsweeper · 2026-05-24T07:14:58Z

🦞👀
ClawSweeper picked this up.

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

clawsweeper · 2026-05-24T08:39:57Z

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

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/26356609351
Updated: 2026-05-24T08:50:29.540Z

TurboTheTurtle · 2026-05-25T21:11:56Z

Addressed the current ClawSweeper P1 queued prompt-turn cleanup finding in e84381f085.

What changed:

releaseHeldLockForUnlockedSessionWindow now treats a queued prompt turn as provisional until the prompt window is actually released.
If the waiting lock release, previous-turn wait, write-lock reacquire, fingerprint read, or fence snapshot setup fails before that release, the queued prompt turn is released/removed so later same-file waiters are not stuck behind it.
Added a regression where a queued same-file waiter times out while reacquiring and a later waiter still completes.

Validation:

node scripts/run-vitest.mjs src/agents/pi-embedded-runner/run/attempt.session-lock.test.ts - 2 files / 72 tests passed
pnpm exec oxfmt --check --threads=1 src/agents/pi-embedded-runner/run/attempt.session-lock.ts src/agents/pi-embedded-runner/run/attempt.session-lock.test.ts src/agents/pi-embedded-runner/run/attempt.ts - passed
pnpm check:test-types - passed
git diff --check - passed

Author check before push:

e84381f085 Andy Ye <35905412+TurboTheTurtle@users.noreply.github.com> Fix prompt holder waiter race
bcae241a17 Andy Ye <35905412+TurboTheTurtle@users.noreply.github.com> fix(agents): guard session prompt ownership

Raw Pulls API still reports maintainer_can_modify: true.

@clawsweeper re-review

clawsweeper · 2026-05-25T21:11:59Z

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

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/26419978848
Updated: 2026-05-25T21:24:48.236Z

joshavant · 2026-05-27T06:21:15Z

Thanks @TurboTheTurtle for the original prompt-ownership guard PR. This was the first concrete implementation pass on the right invariant: concurrent embedded prompt windows need to coordinate by the resolved session JSONL, not only by logical session id/key.

The final fix landed through #87159 in commit 3349fe2. It incorporates the same-session-file serialization direction from this PR, while extending the repair to the broader issue evidence: canonical file ownership, active embedded-run indexes by session file, same-file reply/recovery sibling checks, and diagnostic heartbeat recovery requests that preserve sessionFile.

The landed PR has focused regression coverage plus AWS Crabbox proof for the heartbeat/recovery path that was missing from the earlier attempts. I’m closing this PR as superseded by the merged #87159 fix. Thanks again for pushing the initial guard and tests forward.

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

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 24, 2026

TurboTheTurtle force-pushed the fix/session-file-prompt-guard-85913 branch from c22ae12 to 0fa3daa Compare May 24, 2026 06:36

TurboTheTurtle force-pushed the fix/session-file-prompt-guard-85913 branch from 0fa3daa to 36f6ae7 Compare May 24, 2026 06:54

openclaw-barnacle Bot added size: M and removed size: S labels May 24, 2026

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 24, 2026

TurboTheTurtle force-pushed the fix/session-file-prompt-guard-85913 branch from 36f6ae7 to a8cde4b Compare May 24, 2026 07:01

fix(agents): guard session prompt ownership

bcae241

TurboTheTurtle force-pushed the fix/session-file-prompt-guard-85913 branch from 0e25c91 to d716f20 Compare May 24, 2026 09:09

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 24, 2026

clawsweeper Bot mentioned this pull request May 24, 2026

[Bug]: Concurrent lane writes to same session file cause persistent SessionWriteLockTimeoutError (dual-lane race + lock lifetime mismatch) #86004

Closed

ubehera mentioned this pull request May 25, 2026

Hoist withOwnedSessionTranscriptWrites ALS scope to span agent.prompt() to fix vanilla-openclaw same-lane fence trip #86572

Open

Fix prompt holder waiter race

e84381f

TurboTheTurtle force-pushed the fix/session-file-prompt-guard-85913 branch from d716f20 to e84381f Compare May 25, 2026 21:11

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 25, 2026

joshavant closed this May 27, 2026

xuess mentioned this pull request May 30, 2026

[Bug]: isolated cron can still self-conflict on a dedicated cron agent with EmbeddedAttemptSessionTakeoverError #88369

Open

TurboTheTurtle deleted the fix/session-file-prompt-guard-85913 branch June 10, 2026 04:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(agents): guard session prompt ownership#85955

fix(agents): guard session prompt ownership#85955
TurboTheTurtle wants to merge 2 commits into
openclaw:mainfrom
TurboTheTurtle:fix/session-file-prompt-guard-85913

TurboTheTurtle commented May 24, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 24, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 24, 2026 •

edited

Loading

Uh oh!

TurboTheTurtle commented May 24, 2026

Uh oh!

clawsweeper Bot commented May 24, 2026

Uh oh!

TurboTheTurtle commented May 24, 2026

Uh oh!

clawsweeper Bot commented May 24, 2026

Uh oh!

TurboTheTurtle commented May 24, 2026

Uh oh!

clawsweeper Bot commented May 24, 2026

Uh oh!

clawsweeper Bot commented May 24, 2026 •

edited

Loading

Uh oh!

TurboTheTurtle commented May 25, 2026

Uh oh!

clawsweeper Bot commented May 25, 2026 •

edited

Loading

Uh oh!

joshavant commented May 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

TurboTheTurtle commented May 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Verification

Real behavior proof

Authorship

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Hatch command

Uh oh!

TurboTheTurtle commented May 24, 2026

Uh oh!

clawsweeper Bot commented May 24, 2026

Uh oh!

TurboTheTurtle commented May 24, 2026

Uh oh!

clawsweeper Bot commented May 24, 2026

Uh oh!

TurboTheTurtle commented May 24, 2026

Uh oh!

clawsweeper Bot commented May 24, 2026

Uh oh!

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

Uh oh!

Uh oh!

TurboTheTurtle commented May 25, 2026

Uh oh!

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

Uh oh!

Uh oh!

joshavant commented May 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

TurboTheTurtle commented May 24, 2026 •

edited

Loading

clawsweeper Bot commented May 24, 2026 •

edited

Loading

clawsweeper Bot commented May 24, 2026 •

edited

Loading

clawsweeper Bot commented May 24, 2026 •

edited

Loading

clawsweeper Bot commented May 25, 2026 •

edited

Loading