Repository navigation
fix(session): fence turn-failure writes under the run lease; detect paused turns without the .paused marker (Eve2-F2, Eve2-F3) - #617
Merged
Conversation
…nd treat an awaiting-approval turn checkpoint as paused (Eve2-F2, Eve2-F3)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes two HIGH durability findings from the eve2 runtime-tools audit.
Eve2-F2 —
recordFailure()poisoned a foreign checkpointAgentSession.recordFailure()did a rawload+savemerginglastError { retryable: false }/attempts + 1onto whatever checkpoint occupied<id>.turn-<n>, bypassing the run-lease fencing thatsaveStepCheckpoint/RunLease.saveuse. A run that stalled past its lease, then failed permanently, stamped its error on the checkpoint of the process that had taken the turn over — bricking that turn forever withLOUSHO_SESSION_TURN_FAILED.Fix: the session now takes the turn's run lease itself before the run starts (
holdRunLease), hands it to the run via the internalHELD_RUN_LEASEoption (so the executor saves under it instead of taking its own), keeps it untilrecord()is done, and writes the failure throughRunLease.save— a fenced save that throwsLOUSHO_SESSION_BUSY, writing nothing, once the lease is lost. Stores without lease support behave exactly as before.Eve2-F3 —
.pausedmarker gapThe
<id>.pausedpointer is written bymarkPaused()after the executor already saved the awaiting-approval checkpoint — two non-atomic durable writes.assertNotPausedconsulted only the marker, so aagent.send(msg, { sessionId })in the gap (or after a crash between the writes, permanently) ran a full turn beside the paused one.Fix (option a, defense-in-depth):
pausedSessionTurnnow falls back to scanning<id>.turn-*checkpoints viaCheckpointStore.list({ status: 'awaiting-approval' })when the pointer is absent (or stale), returning the newest paused turn. Stores withoutlist()keep the pointer-only behavior; alist()that is present but unavailable (KV binding withoutlist) degrades gracefully.Tests
src/session/sessionLease.test.ts: stalled session turn (file + SQLite stores) fails permanently after losing its lease — verifies the new holder's checkpoint keeps its ownattempts: 1, lastError: { retryable: true }and stays resumable.src/session/pausedSessionLock.test.ts:.pausedmarker deleted after a pause (the crash window) —agent.send(msg, { sessionId }),session.pending()andsession.send()all still refuse withSessionAwaitingApprovalError.Gates
npx tsc --noEmit -p tsconfig.jsonclean (tests tsconfig too)npm run lintcleannpx vitest run src/session src/execution— 85 files / 1188 tests passNo public API changes (
HELD_RUN_LEASE/RunLeaseOptionsalready existed internally; no signature changes).