Skip to content

fix(agent): stop load-time repairs from masquerading as snapshot conflicts#6095

Merged
SivanCola merged 4 commits into
esengine:main-v2from
SivanCola:fix/snapshot-interrupted-tool-tail
Jul 6, 2026
Merged

fix(agent): stop load-time repairs from masquerading as snapshot conflicts#6095
SivanCola merged 4 commits into
esengine:main-v2from
SivanCola:fix/snapshot-interrupted-tool-tail

Conversation

@SivanCola

Copy link
Copy Markdown
Collaborator

Field incident

A session running on latest main-v2 (dce3b3a) forked a "conflict copy" 30 seconds after its first mid-turn autosave, with no second runtime involved (same writer_id/pid throughout):

  • 17:59:02 first 30s mid-turn snapshot bootstraps the event log: 21 messages, revision 1. The transcript tail is an assistant message whose tool call was still running — a legitimately dangling call.
  • 17:59:32 next autosave: memory holds 34 messages whose first 21 are byte-identical to disk (verified by per-message JSON hashing), i.e. a pure append — yet it was rejected as diverged and forked …-recovery-748ca7f3….

Root cause: checkSnapshotWrite compares the pending snapshot against LoadSession's view of disk, and load-time normalization had already fabricated a placeholder result for the dangling call (sessionToolResults). The live session's real tool result collides positionally with the placeholder, both prefix checks fail, and a pure append misreads as divergence. Any tool-heavy turn whose tool outlives the 30s autosave interval reproduces this deterministically.

Fixes (one per commit, same root cause family)

  1. checkSnapshotWrite falls back to the raw transcript (09ce7df). LoadSession now preserves the pre-repair messages when normalization changed them; append shape, owned-rewrite ownership, and conflict-kind classification retry against the bytes actually on disk before declaring conflict. The mid-turn compaction rewrite variant (ownership revoked by the repaired digest) is covered by the same fallback.
  2. Dirty-resume saves no longer take the append shortcut (a1fecd3). appendFrom was measured against the normalized view while the event log replays the raw transcript; the chain-broken append event was silently discarded on the next load, dropping the whole post-resume turn from disk despite a successful save. A pending repair now falls through to the full rewrite, which persists the repair and the new turn together. Same commit: SaveRecoveryBranch's "recovery not needed" coverage check gets the same raw fallback.
  3. Recovery forks transplant the in-flight turn marker (7932bc1). The marker written at turn start stayed on the forked-from path; turn end cleared the (absent) marker on the recovery path. The next open of the original branch fired recoverInterruptedTurn against a turn that had completed on the recovery branch, stripping the transcript back to the turn boundary and force-saving the truncation.
  4. Fork-orphaned markers self-heal on open (1437d5a). Every fork made by an older runtime already left such a marker behind. A recovery child whose parent_id is this session and whose created_at postdates the marker means the marked turn moved rather than died: clear the marker, keep the messages. A genuine crash (no such child) strips exactly as before. This is the migration path for existing users — no manual cleanup needed.

Tests

10 regression tests across internal/agent and internal/control; each was verified to FAIL with its fix stashed and PASS with it applied. Full go test ./..., go vet, gofmt clean.

Related

#6074, #6080, #6082 fixed the revision-CAS/ledger-drift/adoption conflict sources; this covers the normalization-induced ones they don't reach.

SivanCola added 4 commits July 6, 2026 18:42
A mid-turn autosave can legitimately cut an assistant tool call from its
still-running result, leaving a dangling call at the tail of the saved
transcript. checkSnapshotWrite compared the next snapshot against the
LoadSession view of disk, where normalization has already fabricated a
placeholder answer for that call. The live session's real tool result
collides with the placeholder, so a pure append misread as diverged and
forked a "snapshot conflict" recovery branch 30 seconds after the first
mid-turn save of any tool-heavy turn.

Keep the pre-repair transcript on loaded sessions and fall back to it in
checkSnapshotWrite when the normalized prefix checks fail: append shape,
the owned-rewrite digest, and conflict-kind classification are now judged
against the bytes actually on disk. Appending the real results also lands
the transcript well-formed, so no repair is left pending in that path.
…k recovery need

Two more sites judged the normalized load as if it were the disk truth:

- checkSnapshotWrite let a session resumed from a repaired transcript take
  the append-only shortcut with appendFrom measured against the normalized
  view. The event log replays the raw transcript, so the append event's
  index broke the replay chain and the whole appended turn silently
  vanished from disk on the next load (until a later save healed the log).
  A pending repair now falls through to the full rewrite, which persists
  the repair and the new turn together.

- SaveRecoveryBranch decided "recovery not needed" only against the
  normalized view, so a mid-history repair (e.g. an empty tool-call name
  backfilled on load) made a fully-covered snapshot look uncovered and
  forked a pointless recovery branch. Coverage now falls back to the raw
  transcript, mirroring checkSnapshotWrite.
A snapshot-conflict recovery fork switches the controller onto the new
branch mid-turn, but the in-flight turn marker written at turn start
stayed on the original path — turn end then cleared the (absent) marker
on the recovery path, leaving the original's marker behind forever. The
next open of the original branch fired recoverInterruptedTurn against a
turn that in fact kept running on the recovery branch, stripping every
message after the turn boundary and force-saving the truncation.

Transplant the marker when the fork switches paths: mark the recovery
branch (so a crash before turn end still strips correctly there) and
clear the forked-from branch.
The marker transplant stops new recovery forks from leaving in-flight
turn markers behind, but every fork made by an older runtime already
left one on its forked-from branch. Opening such a branch fired
recoverInterruptedTurn against a turn that completed on the recovery
child, stripping the transcript back to the turn boundary and
force-saving the truncation.

recoverInterruptedTurn now recognizes the leftover: a recovery branch
whose parent is this session and whose creation postdates the marker
means the marked turn moved there rather than dying with a runtime.
Clear the stale marker and keep the messages; a marker with no such
child still strips the partial tail as before.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent Core agent loop (internal/agent, internal/control) v2 Go rewrite (1.x) — main-v2 branch, active development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant