Skip to content

fix(sessions): restore owned-write fence gate for same-lane takeover race (#86584 regression)#44

Merged
augusteo merged 3 commits into
boonfrom
fix/session-takeover-fence-6-11-boon
Jul 14, 2026
Merged

fix(sessions): restore owned-write fence gate for same-lane takeover race (#86584 regression)#44
augusteo merged 3 commits into
boonfrom
fix/session-takeover-fence-6-11-boon

Conversation

@augusteo

@augusteo augusteo commented Jul 13, 2026

Copy link
Copy Markdown

Summary

Fixes a regression in 2026.6.11-boon.* where long agent turns (notably cron turns) throw EmbeddedAttemptSessionTakeoverError on their own transcript append — the paired-lane / same-lane race (upstream openclaw#86572). On the gandalf fleet box this is firing ~192×/day across 54 of 84 crons.

This restores the fork [openclaw#86584] fence mechanism that was dropped in the 5.18→6.11 merge (#31) on the assumption that "upstream's fence work now covers it." It does not: 6.11 carries the owned-write machinery (transcript-write-context, publishOwnedSessionFileFenceSync) but the message-persist path registers owned writes via refreshAfterOwnedSessionWrite(), which keys its beforeWrite off the controller's stale fenceFingerprint field rather than a fingerprint read immediately before the append. On a long turn whose two nested async lanes each append, that stale key diverges from the real pre-append on-disk state, so the run's own write is never recognized as owned and the peer lane's fence throws a takeover on it.

The fix

Register the owned write keyed on a fresh pre-append fingerprint captured at the persist callsite, reusing 6.11's existing hardened publishOwnedSessionFileFenceSync trust gate:

  • session-tool-result-guard{,.-wrapper}.ts: widen onMessagePersisted to (message, { beforeWriteSnapshot }) and add a beforeMessagePersist hook captured immediately before the append.
  • attempt.session-lock.ts: new controller method publishOwnedPostMessageWrite(beforeWrite) — on the fence-active path delegates to publishOwnedSessionFileFenceSync keyed on the fresh snapshot; on the pre-fence path records the user-message write as trusted state (superseding refreshAfterOwnedSessionWrite on this callsite). Export SessionFileFingerprint + readSessionFileFingerprintSync.
  • attempt.ts: wire beforeMessagePersist + call only publishOwnedPostMessageWrite from onMessagePersisted (drops the stale-keyed refreshAfterOwnedSessionWrite here — a Codex-review finding: calling it first could launder a real external takeover into a silently dropped reply).

Fail-closed property (why this can't hide a real takeover)

publishOwnedSessionFileFenceSync records nothing and does not advance the fence unless beforeWrite matches the active fence or a recorded trusted state. If an external mutation lands before the lane's append, the freshly captured beforeWrite reflects that untrusted state → no publish → the subsequent fence still trips the genuine takeover. Covered by a negative regression test.

Tests

3 new regression tests in attempt.session-lock.test.ts:

  • own append registered via publishOwnedPostMessageWrite does not trip the fence;
  • external-write-before-own-append still trips (fail-closed);
  • exact production hook sequence (publish-only, no refresh) does not launder an external takeover.

tsgo:core clean; 360 tests pass across the session-lock + guard + write-lock suites.

Follow-ups (out of scope, filed separately)

Two adjacent hardening gaps surfaced during review, deferred to keep this hotfix minimal/revertible:

  1. installPromptSubmissionLockRelease's unconditional finally { reacquireAfterPrompt() } can mask a provider error with a takeover error (attribution bug).
  2. classifySessionFenceAdvance (async fence-assert path) reads only the appended region and does not re-validate the prefix — the prefix-launder guard from fork commit f11af41c18 covers the rewrite path but not the advance path.

🤖 Generated with Claude Code


Summary by cubic

Fixes a 6.11-boon regression where long agent turns threw session takeovers on their own transcript append. Restores the owned-write fence keyed on a fresh pre-append fingerprint to stop false takeovers while still catching real ones.

  • Bug Fixes
    • Restores the owned-write trust gate for the same-lane takeover race by capturing a fresh pre-append session-file fingerprint and publishing via publishOwnedSessionFileFenceSync.
    • Adds beforeMessagePersist and widens onMessagePersisted to (message, { beforeWriteSnapshot }); updates the guard and wrapper to pass this snapshot.
    • Adds publishOwnedPostMessageWrite on the controller and uses it instead of refreshAfterOwnedSessionWrite at the persist site; handles pre-fence trusted-state; exports SessionFileFingerprint and readSessionFileFingerprintSync.
    • Wires attempt.ts to snapshot before append and call only publishOwnedPostMessageWrite to avoid laundering real takeovers.
    • Adds 3 regression tests; runs the laundering test through the real guard wiring (beforeMessagePersist → append → onMessagePersisted) and fully types the assistant message for stricter type checks, proving a genuine external write still trips.

Written for commit 7e874f3. Summary will update on new commits.

Review in cubic

…race (openclaw#86584)

Long agent turns (notably cron turns) on 2026.6.11-boon.* throw
EmbeddedAttemptSessionTakeoverError on their OWN transcript append — the
paired-lane / same-lane race (upstream openclaw#86572). On the gandalf fleet box this
fires ~192x/day across 54 of 84 crons.

Root cause: the 5.18→6.11 merge (#31) dropped the fork [openclaw#86584] fence patch on
the assumption "upstream's fence work now covers it." It does not. 6.11 carries
the owned-write machinery, but the message-persist path registers owned writes
via refreshAfterOwnedSessionWrite(), which keys beforeWrite off the controller's
STALE fenceFingerprint field rather than a fingerprint read immediately before
the append. On a long turn whose two nested async lanes each append, that stale
key diverges from the real pre-append on-disk state, so the run's own write is
never recognized as owned and the peer lane's fence throws a takeover on it.

Fix: register the owned write keyed on a FRESH pre-append fingerprint captured
at the persist callsite, reusing 6.11's hardened publishOwnedSessionFileFenceSync
trust gate.
- session-tool-result-guard{,-wrapper}.ts: widen onMessagePersisted to
  (message, {beforeWriteSnapshot}); add beforeMessagePersist hook captured
  immediately before originalAppend.
- attempt.session-lock.ts: new controller method publishOwnedPostMessageWrite —
  fence-active path delegates to publishOwnedSessionFileFenceSync keyed on the
  fresh snapshot; pre-fence path records the user-message write as trusted state
  (supersedes refreshAfterOwnedSessionWrite on this callsite). Export
  SessionFileFingerprint + readSessionFileFingerprintSync.
- attempt.ts: wire beforeMessagePersist; call ONLY publishOwnedPostMessageWrite
  from onMessagePersisted. Dropping the stale-keyed refreshAfterOwnedSessionWrite
  here is required — calling it first could launder a real external takeover into
  a silently dropped reply (Codex review finding).

Fail-closed: publishOwnedSessionFileFenceSync records nothing and does not
advance the fence unless beforeWrite matches the active fence or a recorded
trusted state, so a genuine external takeover still trips.

Tests: 3 regression tests (own append absorbed; external-before-append still
trips; exact production hook sequence not laundered). tsgo:core clean; 360
tests pass across session-lock + guard + write-lock suites.

Codex-reviewed (2 rounds); v1 laundering bug found + fixed; verdict ship-ready.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@claude

claude Bot commented Jul 13, 2026

Copy link
Copy Markdown

Claude encountered an error —— View job


I'll analyze this and get back to you.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 5 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread src/agents/embedded-agent-runner/run/attempt.ts
Comment thread src/agents/embedded-agent-runner/run/attempt.session-lock.test.ts Outdated
augusteo and others added 2 commits July 14, 2026 09:46
…ersist wiring

Addresses cubic-dev-ai P3: the production-sequence test called
publishOwnedPostMessageWrite directly, duplicating the prior controller-level
test — reintroducing refreshAfterOwnedSessionWrite() at the attempt.ts persist
callsite would have left it green.

Rewrite it as a guard/runner composition test: build a file-backed
guardSessionManager wired exactly as runEmbeddedAttempt (beforeMessagePersist
snapshot -> append -> onMessagePersisted -> publishOwnedPostMessageWrite), let a
foreign writer mutate the transcript, then persist the lane's own message
through the guard and assert the fence still trips. Mutation-tested: injecting
controller.refreshAfterOwnedSessionWrite() into onMessagePersisted flips this
test red (withSessionWriteLock resolves instead of throwing), proving it now
guards the exact regression class the old test could not.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…tion test

check:test-types (stricter than tsgo:core) requires AssistantMessage to carry
api/provider/model/usage/stopReason. Populate them so the laundering-regression
test typechecks under test/tsconfig/tsconfig.core.test.json. No behavior change —
mutation test still flips red when refreshAfterOwnedSessionWrite() is reintroduced.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@augusteo
augusteo merged commit 4a63a70 into boon Jul 14, 2026
144 checks passed
@augusteo
augusteo deleted the fix/session-takeover-fence-6-11-boon branch July 14, 2026 03:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant