Codex review: found issues before merge. Reviewed June 24, 2026, 3:07 PM ET / 19:07 UTC.

Summary
The PR adds a pre-append session-file fingerprint callback for guarded message persistence, replaces unconditional owned-write fence refresh with trust-gated publishing, and adds focused session-lock regression tests.

PR surface: Source +126, Tests +60. Total +186 across 5 files.

Reproducibility: yes. source-level reproduction is high confidence. The PR proof exercises the message append interleave, and current main plus related reports show sibling runtime-owned session entry paths sharing the same released-lock fence invariant.

Review metrics: 1 noteworthy metric.

• Current-main publication seams: 1 PR message callback changed; 3 sibling seam families still relevant. Message, compaction/custom, snapshot, and prompt-stream transcript writes all share the session-file ownership invariant before merge.

Stored data model
Persistent data-model change detected: serialized state: src/agents/session-tool-result-guard-wrapper.ts, serialized state: src/agents/session-tool-result-guard.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #86572
Summary: This PR is a candidate interim fix for the canonical same-lane EmbeddedAttemptSessionTakeoverError tracker, while the broader durable storage/fence removal remains under the SQLite migration issue.

Members:

• canonical: Hoist withOwnedSessionTranscriptWrites ALS scope to span agent.prompt() to fix vanilla-openclaw same-lane fence trip #86572 - The PR body uses closing syntax for this issue and both discuss the same same-lane prompt-release session-write ownership failure.
• partial_overlap: Fix embedded session file ownership race #87159 - That merged PR fixed the adjacent cross-lane session-file ownership boundary but did not terminally resolve the same-lane publish-boundary class.
• partial_overlap: fix(agents): move session write lock into owned session runtime #87409 - That merged PR moved session write locking into owned session runtime and defines part of the current-main surface this branch must preserve.
• partial_overlap: Track core session/transcript SQLite migration via accessor seam #88838 - The migration tracker is broader, but maintainers cite it as the durable path for eliminating the JSONL fingerprint-fence failure class.
• duplicate: Embedded agent run deterministically self-takeovers (EmbeddedAttemptSessionTakeoverError) on any multi-step turn — owned-write refresh is ineffective #95228 - That issue was closed back to the canonical same-lane self-takeover tracker and adds v2026.6.1 evidence for the same class.

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦞 diamond lobster
Patch quality: 🧂 unranked krab
Result: blocked by patch quality or review findings.

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

Rank-up moves:

• Rebase onto current main and resolve the conflict.
• Port the trust gate through the current owned session-file publication APIs instead of only the message callback.
• [P2] Add or update regressions covering message, compaction/custom, snapshot, and prompt-stream transcript entries during prompt-lock release.

Risk before merge

• [P1] The branch is currently CONFLICTING/DIRTY against main, so the submitted diff is not the real merge result.
• [P1] Current main has snapshot, compaction/custom, and prompt-stream transcript publication paths sharing the same fence invariant; a message-only rebase can leave non-message owned writes outside the trust gate.
• [P1] Maintainers still need to choose whether to land another JSONL fence-hardening slice now or defer this class to the active SQLite session/transcript migration.

Maintainer options:

1. Port Gate Into Current Publication APIs (recommended)
   Rebase and move the trust decision into current owned session-file publication paths so all runtime-owned entries share one fence invariant before merge.
2. Defer To SQLite Migration
   If maintainers do not want another JSONL fence stopgap, pause or close this PR and keep the remaining work under the active session/transcript SQLite migration tracker.
3. Accept Message-Only Hardening With Follow-Up
   Maintainers could intentionally land only the message path after rebase, but should explicitly own the remaining non-message false-takeover surface as tracked follow-up risk.

Next step before merge

• [P2] Human review is needed because the branch conflicts with current main and maintainers must choose the current publication-boundary fix or defer to the SQLite migration path.

Security
Cleared: The diff is confined to TypeScript session-lock runtime and tests and does not add dependency, workflow, secret, package, or external code-execution surfaces.

Review findings

• [P1] Preserve current owned session publication — src/agents/embedded-agent-runner/run/attempt.ts:1979-1983

@clawsweeper re-review

PR body updated with harness output (run-refresh-before-hook-lock-proof.mjs) showing deterministic before/after on a real temp session JSONL: pre-fix throws EmbeddedAttemptSessionTakeoverError, post-fix hook fires cleanly with hasSessionTakeover()=false.

🦞🧹
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/26412335751
• Updated: 2026-05-25T17:44:42.441Z

ClawSweeper PR egg

✨ Hatched: 🌱 uncommon Gilded 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: 🌱 uncommon.
Trait: polishes edge cases.
Image traits: location diff observatory; accessory release bell; palette pearl, teal, and neon green; mood curious; pose peeking out from the egg shell; shell frosted glass shell; lighting moonlit rim light; background soft code-shaped tiles.
Share on X: post this hatch
Copy: My PR egg hatched a 🌱 uncommon Gilded Proofling in ClawSweeper.

Heads-up: the checks-node-agentic-agents failure on this PR is a pre-existing flake on main, not caused by this PR.

The failing test is src/agents/model-catalog-visibility.test.ts:29 > resolveVisibleModelCatalog > limits visible catalog to provider wildcard entries after default discovery — Error: Test timed out in 120000ms.

The same test, same line, same timeout fails on main CI run 26412030433 (push fix(security): audit Claude permission overrides under YOLO (#86557)) at 17:18Z — 13 minutes before this PR's run. The most recent successful main run (#86571 at 17:24Z) happens to pass it; the flake is timing-dependent.

This PR touches src/agents/pi-embedded-runner/run/attempt.session-lock.ts (+1 line) and the matching test file — completely separate code path from the model-catalog-visibility test. Local node scripts/run-vitest.mjs src/agents/pi-embedded-runner/run/attempt.session-lock.test.ts exits 0.

Per AGENTS.md "If [a failing check is] unrelated on latest origin/main, say so with scoped proof" — flagging this so it doesn't become a maintainer-side speed bump.

Redesigned per ClawSweeper review on 91ba92c9ab (force-pushed to d824be0298).

Change in approach:

• Old (91ba92c9ab): installSessionExternalHookWriteLock got a refreshBeforeLock param that called refreshAfterOwnedSessionWrite() before each hook acquired the lock. That blanket-refreshed the fence even for genuine external mutations, weakening takeover detection.
• New (d824be0298): A new publishOwnedPostMessageWrite() method is wired into onMessagePersisted (the existing pi _persist callback point). It records the post-write fingerprint as owned in ownedSessionFileWrites, gated on isTrustedSessionFileState(sessionFileFenceKey, fenceFingerprint). External mutations that bypass the callback never get published; the fence still trips on them via assertSessionFileFence's existing owned-write match path.

Addresses bot guidance verbatim:

"Please only advance the fence for writes proven to be this controller's own pi/session-manager persistence, and add a negative test for an external same-file append before a hook lock."

• Trust gate: The new method only records when the prior fenceFingerprint is in trustedSessionFileStates (set at releaseForPrompt time). Same-file external mutations cannot bypass this — they don't go through onMessagePersisted.
• Negative test added: trips takeover on a same-file external write that bypasses publishOwnedPostMessageWrite — verifies the fail-closed invariant.

Proof: Three-scenario harness covers pre-fix bug, post-fix success, and the negative case. Verbatim output in the PR body.

@clawsweeper re-review

🦞🧹
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/26413924744
• Updated: 2026-05-25T18:22:25.140Z

CI status: checks-node-core-runtime-cron-service fails on the unrelated test src/cron/service/timer.regression.test.ts:989 — records per-job start time and duration for batched due jobs. Assertion: expected 70 to be 50 // Object.is equality.

This is a timing-sensitive flake in the cron-service batched-due-jobs path (uses a mutable let now shared between two synchronous mock job runs). It has no dependency on the files this PR touches (src/agents/pi-embedded-runner/run/attempt.session-lock.ts, attempt.session-lock.test.ts, attempt.ts).

All other checks pass on d824be0298, including checks-node-agentic-agents (which covers the embedded-runner test surface this PR modifies).

Force-pushed 584ba0e65b addressing ClawSweeper's correctness finding on d824be0298.

The bot's exact concern, addressed:

"This records the current fingerprint as owned whenever the previous fence fingerprint was trusted. If another lane/process appends to the same session file after releaseForPrompt() and then this lane appends its pi message before onMessagePersisted() runs, current contains both writes; recording that combined fingerprint makes the later hook-lock fence accept the external mutation as owned."

Fix shape:

• publishOwnedPostMessageWrite() now takes a beforeWrite: SessionFileFingerprint | undefined argument and gates trust on the pre-append fingerprint, not on the current fenceFingerprint.
• The pre-append fingerprint is captured by a new beforeMessagePersist hook in session-tool-result-guard, invoked before originalAppend(message). The snapshot flows back to the caller through onMessagePersisted's new context arg (typed unknown to keep the guard module-agnostic).
• runEmbeddedAttempt wires beforeMessagePersist: () => readSessionFileFingerprintSync(params.sessionFile) and forwards the snapshot to publishOwnedPostMessageWrite(beforeWriteSnapshot).

Mixed-interleaving negative test added:

it("trips takeover on a mixed external-then-pi append (publish refuses to launder external mutation)", async () => {
  await controller.releaseForPrompt();                            // F0 trusted
  await fs.appendFile(sessionFile, '...external...', "utf8");     // F0 -> F1
  const beforeWrite = readSessionFileFingerprintSync(sessionFile);// F1
  await fs.appendFile(sessionFile, '...pi-after-external...');    // F1 -> F2
  controller.publishOwnedPostMessageWrite(beforeWrite);           // F1 NOT trusted: skip
  await expect(session.agent.beforeToolCall()).rejects.toBeInstanceOf(
    EmbeddedAttemptSessionTakeoverError,                          // fail closed
  );
});

Proof harness updated to a 4-scenario harness; Scenario D is the mixed-interleave fail-closed verification. All 4 PASS. Verbatim stdout in the PR body.

80 tests across the session-lock surface green (+1 from the new mixed-interleave test). 46 cases across the session-tool-result-guard surfaces also green — no regressions from the new beforeMessagePersist opt.

@clawsweeper re-review

🦞🧹
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/26417580857
• Updated: 2026-05-25T20:09:16.503Z

@clawsweeper re-review

Force-pushed 43baac80c5 — rebased onto current upstream/main and added the checked-in regression coverage flagged in the last pass (the "0 new test cases added in the latest diff" metric / maintainer option "Ask For Checked-In Regression Coverage").

Added (src/agents/embedded-agent-runner/run/attempt.session-lock.test.ts): a new describe("publishOwnedPostMessageWrite trust gate (#86572)") block with 3 cases that drive the publish path directly:

1. skips publish → trips — a same-lane write that never calls publishOwnedPostMessageWrite still trips the fence (fail-closed default; publish is the required opt-in).
2. publish + trusted baseline → accepts — pre-append fingerprint F0 is trusted, so the lane's own write is recorded as owned and the hook fires cleanly (hasSessionTakeover() === false).
3. mixed interleave → still trips — an external write advances F0→F1 first, so the captured baseline (F1) was never trusted; publishOwnedPostMessageWrite refuses to record the combined F2 state and the takeover fence still fires. This is the same-lane/mixed-interleave scenario from the proof harness, now checked in.

Harness Scenario C (pure external advance) is intentionally not duplicated — it is already covered by the existing rejects post-prompt writes when another owner advances the session file test.

Verification on the rebased branch (43baac80c5):

• node scripts/run-vitest.mjs src/agents/embedded-agent-runner/run/attempt.session-lock.test.ts → 45/45 pass.
• Full touched surface (attempt.session-lock + session-tool-result-guard{,.transcript-events,.tool-result-persist-hook}) → 113/113 pass.
• oxlint exit 0; oxfmt --check clean.
• Mutation-checked for teeth: test 2 fails if the owned-write recording is removed; test 3 fails if the fail-closed trust gate is removed — confirming each test guards the fix rather than restating current behavior.

No source behavior changed in this push — the diff vs the prior head is the rebase plus the three test cases.

🦞🧹
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/26808434401
• Updated: 2026-06-02T08:43:37.445Z

Latest-head broad gate — pnpm check:changed green on Linux

Ran the current-head broad changed gate (the latest-head verification flagged in the last review) on 43baac80c5, via crabbox local-container (Linux, ubuntu:26.04, Node 24), scoped to this PR's 5 changed files:

node scripts/crabbox-wrapper.mjs run --provider local-container -- \
  env CI=1 OPENCLAW_TEST_PROJECTS_PARALLEL=6 OPENCLAW_VITEST_MAX_WORKERS=1 \
  OPENCLAW_VITEST_NO_OUTPUT_TIMEOUT_MS=900000 pnpm check:changed

Result: exit=0, 0 failures (command 3m08s / total 5m36s). Changed-gate scope confirmed = exactly the 5 changed files (lanes=core, coreTests); no unrelated drift. Green steps:

• typecheck core + typecheck core tests
• lint core changed files — Found 0 warnings and 0 errors
• dependency pin guard (464 specs, 0 violations), package patch guard
• conflict markers, changelog attributions, plugin-sdk / extension wildcard re-export guards, duplicate-coverage, runtime import cycles, media / sidecar / webhook / pairing guards

Test execution (separate from check:changed): focused run-vitest on the touched surface is green — attempt.session-lock.test.ts 45/45 (including the 3 new publishOwnedPostMessageWrite trust-gate cases) plus the three session-tool-result-guard suites; 103 tests across 4 files. All GitHub checks on 43baac80c5 are green as well.

Proof note: this ran on a local-container Linux runner (Node 24, CI-matching), not the brokered runner. A maintainer can re-run the brokered broad gate for the canonical environment if desired.

Timeline correction on the production corroboration attributed to me — my 30-event corpus is PRE-#87159 in our install, not "post-#87159 still firing."

Thanks for carrying the production data. One clarification I should make before it weighs on the merge, because the current PR body reads our corpus as the class "continues firing in real installs after #87159 landed" / "post-#87159 code" — that isn't accurate for our install:

• Fix embedded session file ownership race #87159 merged upstream 05-27, but it didn't reach us until we upgraded to v2026.5.27 on 2026-05-29. All 30 of our EmbeddedAttemptSessionTakeoverError events were on the prior 2026.5.26 build (no Fix embedded session file ownership race #87159) — the last one 2026-05-27 11:03 UTC.
• Since deploying Fix embedded session file ownership race #87159 (05-29) the signature is zero in our install — ~6 days now, still 20 distinct on-disk events total (all pre-upgrade), verified three ways against the raw cron ledgers. Former hot-spot jobs now run clean through to delivery, so the fence is being exercised and holding.

So our corpus documents the class while it was active pre-#87159; it isn't evidence that it persists after #87159. The "rate accelerating / spread widening" in my 05-28 comment was the tail of the pre-upgrade period and shouldn't be read as current.

What our data can't do is corroborate that the same-lane path this PR targets is still firing post-#87159 — once the cross-lane boundary is closed, our production window can't isolate a same-lane-only race. So I'd weight this PR on its own source-level proof (the 4-scenario harness — A trips pre-fix, B clean post-fix, C/D fail-closed — independently demonstrates the same-lane race and the trust gate's fail-closed behavior), rather than on our corpus as a post-#87159 signal. The 30 anonymized session .jsonl files (all pre-#87159) remain available if a diff-driven repro is still useful.

Thanks @htuyer121 — appreciate the careful correction, and you're right. I've updated the PR body accordingly: the 30-event corpus is now described as pre-#87159 (build 2026.5.26), and the "continues firing after #87159" framing is gone. The "Production context" section now leads with your correction — zero events over ~6 days post-#87159, and the point that a same-lane-only race can't be isolated from production once #87159 closes the cross-lane boundary — and explicitly weights the PR on its source-level proof rather than the corpus, per your recommendation.

For clarity on what the PR now claims: defense-in-depth hardening of the same-lane publish boundary (fail-open unconditional fence refresh → fail-closed trust gate), not a fix for an actively-firing production class. The meaningful source-level scenario is the mixed-interleave case (an external mutation interleaved with the lane's own write being laundered into the fence), which the trust gate refuses to record.

The offer of the anonymized .jsonl files is still genuinely useful for a future diff-driven repro — thank you for that.

@clawsweeper re-review

One cleanup commit plus a PR-body correction since the last pass on 43baac80c5. New head b06e2b72d0 (fast-forward, not a force-push — the reviewed feature commit 43baac80c5 is untouched).

1. Removed dead refreshAfterOwnedSessionWrite (method + interface entry) — src/agents/embedded-agent-runner/run/attempt.session-lock.ts

This is upstream main's method (introduced in 2bb00f6726 "fix(agents): fence embedded session writes", carried through the #85341 rename). Its only caller was onMessagePersisted, which this PR's core fix already rewires to publishOwnedPostMessageWrite(beforeWriteSnapshot) (attempt.ts). Superseding that single call site left the method with zero production references — rg -uu (including generated/ignored files) confirms no remaining source reference on this branch; the only hits are regenerated build artifacts and git internals. Removing it completes the refactor to one canonical path: the fail-open unconditional refresh is exactly the behavior this PR replaces, so leaving it defined would strand a dead fail-open path beside the new fail-closed gate. Not a plugin-sdk export (the type is internal; its only structural consumer, attempt-abort.ts, uses Pick<…, "releaseHeldLockForAbort">, unaffected).

2. Reworked the two tests that referenced it — attempt.session-lock.test.ts

• Rewrote "keeps post-provider transcript writes owned after prompt stream returns" onto the production method publishOwnedPostMessageWrite(beforeWrite), capturing the pre-append fingerprint exactly as beforeMessagePersist does. This preserves its unique post-reacquireAfterPrompt coverage and is mutation-sensitive: if the publish were a no-op, acquireForCleanup's assertSessionFileFence sees current=F1 ≠ fence=F0, trips, and the test fails. Added a comment marking it as the deliberate post-reacquire path.
• Deleted "refreshes the prompt fence after an owned session manager append" as redundant — it tested the removed method, and its production-API analog is the existing "accepts a same-lane write published with a trusted pre-append fingerprint" trust-gate case.

3. PR body corrected (no code impact)

Removed the inaccurate "@htuyer121's corpus shows the class continues firing after #87159" framing per their correction (corpus is pre-#87159; zero events post-#87159). Reframed the headline as fail-open → fail-closed publish-boundary hardening, and added a Fix shape vs. #86572 note (publish boundary, not the proposed ALS-scope hoist) plus a Known limitation note (the narrow pre-append window, owned by #87159's file-ownership boundary).

Verification on b06e2b72d0:

• node scripts/run-vitest.mjs src/agents/embedded-agent-runner/run/attempt.session-lock.test.ts → 44/44 (was 45; −1 from the redundant-test deletion).
• oxlint exit 0; oxfmt --check clean.
• crabbox local-container pnpm check:changed (Ubuntu 26.04, Node 24) → exit=0 (command 3m16s / total 5m40s): typecheck core + core tests clean, lint 0 warnings / 0 errors on the 5 changed files, 0 runtime import cycles, all guards (dependency pins, package patches, media/sidecar/webhook/pairing) pass.

Net −23 LOC (source −7, tests −16). No production behavior change vs the prior head — this commit removes a now-dead path that the fix had already superseded and refreshes its coverage onto the production seam.

🦞🧹
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/26828060667
• Updated: 2026-06-02T15:01:46.227Z

Coverage note for the publish-boundary fix shape.

A runtime-owned custom session entry can advance the same session file during the prompt-lock release window before the next message append.

Observed ordering from a packaged [email protected] Docker gateway run:

• thinking_level_change
• custom entry model-snapshot
• user message
• subsequent fence check raised EmbeddedAttemptSessionTakeoverError: session file changed while embedded prompt lock was released

If the owned-write publish path is applied only to sessionManager.appendMessage, the later user-message append captures a pre-write fingerprint that already includes the unpublished custom entry. The fail-closed trust gate then refuses to publish the message append, so the fence still trips.

In this scenario, the publish-boundary coverage needs to include runtime-owned custom session entries as well as message entries, or those custom writes need an equivalent ownership/locking path so they cannot advance the session file unrecorded during the prompt-lock release window.

This assigned pull request has been automatically marked as stale after being open for 27 days.
Please add updates or it will be closed.

Closing due to inactivity.
If you believe this PR should be revived, post in #clawtributors on Discord to talk to a maintainer.
That channel is the escape hatch for high-quality PRs that get auto-closed.

Closing-state update (2026-07-18): this branch should remain closed rather than be rebased.

Its message-only publish gate was incomplete against the later compaction/custom, snapshot, and prompt-stream ownership seams. Current main subsequently covered those boundaries through #85341 and #90775, then replaced active JSON/JSONL session and transcript persistence with SQLite in #98236. The canonical issue #86572 was closed after that storage change.

If EmbeddedAttemptSessionTakeoverError still reproduces on current main or a post-#98236 release, please file a fresh report with current logs. This stale branch is no longer the right fix surface.