stacklok / stacklok/mecatl

engine: failure-path session persistence races run-context cancellation, losing the snapshot that records the failure

Open
#1,588 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
Go
Stars
152
Forks
16
Avg merge
14h 48m
Merged PRs (30d)
536

Description

Bug description

When a run ends on an abrupt provider failure, the failure-path session persistence
races the run context's cancellation and loses the snapshot. engine/agent/loop.go
(Engine.save) guards the healthy path with a check-then-use:

if ctx.Err() == nil {
        err = e.deps.Store.Save(ctx, sess)        // ← cancellation can land HERE
} else {
        saveCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), sessionPersistenceTimeout)
        err = e.deps.Store.Save(saveCtx, sess)
        cancel()
}

The detached branch only engages when the context is already cancelled at the
check. On an error terminal the cancellation typically arrives milliseconds
after the check (the relay/client tears down on seeing the terminal event),
so Store.Save starts on the live context and then aborts mid-flight —
observed as jsonlstore failing its snapshot-family flock acquire with
context canceled. There is no detached retry, so the snapshot recording the
failure state is simply lost and the WARN admits it: "this session may not be
resumable after a restart."

This was not a one-off. In one interactive mecatui session, a burst of
provider HTTP 400s ended 9 runs over ~30 minutes, and all nine saves died
in this race — the WARN pair appears 15ms apart every time:

time=2026-09-14T11:43:17.395 level=INFO msg="llm stream failed with a non-retryable provider error; ending turn"
  provider=anthropic model=claude-opus-5 retry_disposition=permanent stream_progress=precommit
  http_status=400 session=c15f145c… run_serial=4 turn=11
time=2026-09-14T11:43:17.410 level=WARN msg="session persistence failed; this session may not be resumable after a restart"
  session=c15f145c… error="jsonlstore: acquire snapshot family lock \"…/sid-v1-c15f145c…family.lock\": context canceled"

(Same pair at 11:45:59, 11:48:59, 11:49:45, 11:54:49, 11:56:45, 11:57:48,
12:10:59 — run_serial 4–12, turn depths 2–11. 9/9 loss rate on the failure
path, exactly the snapshots that mattered.)

Steps to reproduce

  1. Run mecatui (embedded engine, jsonlstore) and drive a session.
  2. Have the provider return a permanent error mid-run (any non-retryable HTTP
    failure ends the turn; the relay/client teardown then cancels the run ctx).
  3. Observe the WARN pair: the terminal INFO line, then ~15ms later the
    persistence WARN with context canceled from the store's lock acquire.

Deterministic variant for a test: a SessionStore fake whose Save blocks
until the run context is cancelled, then returns ctx.Err() — asserting that
Engine.save still persists (via a detached retry) rather than warning.

Expected behavior

A failure-terminal save is exactly the save that must survive: the session's
failed state, the repaired history (closeOutInterruptedTurn), and
LastError (issue #332) all ride it. Cancellation of the run must not abort
the durable write — the same rationale already applied to appendEvent
(cancel-detached context.WithoutCancel so "a dead client's cancelled ctx
can't abort the durable write") and to lease release.

Actual behavior

The snapshot is lost whenever cancellation lands inside the Store.Save call
window. The session's last persisted state then still reads as the previous
snapshot (potentially running), pushing recovery onto the crash-orphan seams
(issue #475) instead of the clean failed → Recover path, and losing
LastError — the durable cause record — for background children whose end
emit already raced the seal.

Environment

  • mecatl @ ebb14c78, mecatui embedded engine, jsonlstore
  • macOS 26.5.1, session c15f145c0e948d006bb1f88129b8f6a9 in
    $XDG_STATE_HOME/mecatl/mecatui.log

Additional context

Suggested fix-shape, keeping the healthy path byte-identical: on a Save error
where the run context is (now) cancelled and errors.Is(err, context.Canceled),
retry ONCE through the existing detached branch (context.WithoutCancel +
sessionPersistenceTimeout) before warning. That closes the race without
detaching the healthy path from live cancellation semantics. The sticky
r.saveWarned behavior can stay as-is.

Secondary observation from the same incident (possibly its own issue): the
root-cause 400s themselves are undiagnosable from the operator log — the
resilience line carries only correlation_kind=request +
correlation_digest, never a sanitized provider error body, so the operator
sees nine dead turns with no way to learn what Anthropic rejected. A bounded,
secret-scrubbed error-body excerpt on failure_class=http terminal lines
would make this class debuggable.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in engine/agent/loop.go at Engine.save and trace the existing healthy and detached persistence branches. Reproduce the race with the described SessionStore fake, where Save blocks until cancellation, and verify that the failure snapshot still persists without the persistence warning.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.