Summary
When a dev/review session finishes, the engine records the completed SessionRecord only in memory and then runs post-session hooks + follow-up verification before the run state is ever flushed to disk. If the host kills the process (or a Windows console-control event surfaces as a raw KeyboardInterrupt) inside that window, the completed session is lost: on resume the TUI reads a stale dev-running task with no session evidence, and — because status classification then re-drives — this can feed the destructive auto-rollback path with a task that had, in fact, already completed.
This is platform-neutral, but the window is materially wider on Windows, where a completed claude.exe can be slower to release the ConPTY and where console-ctrl events can arrive as a bare KeyboardInterrupt rather than through the installed signal handler.
Where it happens (current main)
1. No durability barrier between record_session and post_session —
src/bmad_loop/engine.py _run_session (~L1799–1823):
result = adapter.run(spec)
usage = adapter.read_usage(result)
task.record_session(SessionRecord(..., usage=usage)) # in-memory only
self.journal.append("session-end", ...)
self._emit("post_session", task, ...) # hooks + verification run here
return result
There is no self._save() between recording the completed session and emitting post_session. The completed session only becomes durable at some later _save() after verification/hooks have run — anything that kills the process in between loses it. Usage tally is bundled into the same record_session call, so session durability is implicitly gated on usage metadata being available, even though usage is not a correctness input.
2. Raw KeyboardInterrupt has no branch in the run loop —
src/bmad_loop/engine.py main loop (~L239–289):
except RunStopped:
kill_session(self.state.run_id); ...; self.state.stopped = True
except Exception as exc: # KeyboardInterrupt is BaseException, not caught here
...
finally:
self._save()
A bare KeyboardInterrupt (e.g. a Windows GenerateConsoleCtrlEvent that does not route through the signal handler as RunStopped) escapes both except arms. finally: _save() does persist some state, but stopped/crashed are never set and kill_session(...) never runs — so the run is left in an ambiguous status with an orphaned agent window still alive.
Impact
- Lost completed-session record → resume/TUI misreads a finished task as
dev-running.
- That misread can trigger re-drive / destructive auto-rollback on work that was done.
- Orphaned agent session on raw-
KeyboardInterrupt teardown.
Severity: low frequency (narrow kill window), but high blast radius when it hits (destructive rollback of completed work). Sibling to #48 (premature-completion → agent kill), same "engine acts on a mis-read of session state" family.
Proposed fix
A. Make the completed session durable before post-session work, and decouple it from usage tally.
In _run_session, reorder to:
task.record_session(SessionRecord(...)) without usage.
self._save() — the completed session is now on disk.
- Compute/attach usage as metadata via a new
StoryTask.attach_session_usage(task_id, usage)
that finds the just-recorded session and folds usage into the token totals.
New model.py helper (src/bmad_loop/model.py, alongside record_session at ~L166):
def attach_session_usage(self, task_id: str, usage: TokenUsage | None) -> None:
if usage is None:
return
for record in reversed(self.sessions):
if record.task_id != task_id:
continue
if record.usage is None:
record.usage = usage
self.tokens.add(usage)
return
raise KeyError(task_id)
Keep the existing durability _save() before post_session as well, so hooks/verification never run against an unflushed completed session.
B. Add an explicit except KeyboardInterrupt branch in the run loop that mirrors except RunStopped: kill_session(...), set self.state.stopped = True, journal a run-stop with reason="KeyboardInterrupt", and re-raise when nested. This closes the orphan-window / ambiguous-status gap on the Windows console-ctrl path.
Tests
- Session-end durability: assert
_save() is called (session on disk) before post_session is emitted, and that a simulated host-kill after record_session leaves a recoverable completed session rather than a stale dev-running task.
attach_session_usage: usage folds into totals for the matching session; KeyError on unknown task_id; no-op on None.
KeyboardInterrupt branch: run loop sets stopped, kills the session, journals the stop, and re-raises when nested.
Scope / non-goals
- Do not port the
SESSION_END_TRACE_FILE / trace_run breadcrumb scaffolding that accompanied the reference patches — it was diagnostic weight for a since-closed engine-death investigation and should not land.
- Usage remains best-effort metadata; it must never gate session-progress durability.
Summary
When a dev/review session finishes, the engine records the completed
SessionRecordonly in memory and then runs post-session hooks + follow-up verification before the run state is ever flushed to disk. If the host kills the process (or a Windows console-control event surfaces as a rawKeyboardInterrupt) inside that window, the completed session is lost: on resume the TUI reads a staledev-runningtask with no session evidence, and — because status classification then re-drives — this can feed the destructive auto-rollback path with a task that had, in fact, already completed.This is platform-neutral, but the window is materially wider on Windows, where a completed
claude.execan be slower to release the ConPTY and where console-ctrl events can arrive as a bareKeyboardInterruptrather than through the installed signal handler.Where it happens (current
main)1. No durability barrier between
record_sessionandpost_session—src/bmad_loop/engine.py_run_session(~L1799–1823):There is no
self._save()between recording the completed session and emittingpost_session. The completed session only becomes durable at some later_save()after verification/hooks have run — anything that kills the process in between loses it. Usage tally is bundled into the samerecord_sessioncall, so session durability is implicitly gated on usage metadata being available, even though usage is not a correctness input.2. Raw
KeyboardInterrupthas no branch in the run loop —src/bmad_loop/engine.pymain loop (~L239–289):A bare
KeyboardInterrupt(e.g. a WindowsGenerateConsoleCtrlEventthat does not route through the signal handler asRunStopped) escapes bothexceptarms.finally: _save()does persist some state, butstopped/crashedare never set andkill_session(...)never runs — so the run is left in an ambiguous status with an orphaned agent window still alive.Impact
dev-running.KeyboardInterruptteardown.Severity: low frequency (narrow kill window), but high blast radius when it hits (destructive rollback of completed work). Sibling to #48 (premature-completion → agent kill), same "engine acts on a mis-read of session state" family.
Proposed fix
A. Make the completed session durable before post-session work, and decouple it from usage tally.
In
_run_session, reorder to:task.record_session(SessionRecord(...))withoutusage.self._save()— the completed session is now on disk.StoryTask.attach_session_usage(task_id, usage)that finds the just-recorded session and folds usage into the token totals.
New
model.pyhelper (src/bmad_loop/model.py, alongsiderecord_sessionat ~L166):Keep the existing durability
_save()beforepost_sessionas well, so hooks/verification never run against an unflushed completed session.B. Add an explicit
except KeyboardInterruptbranch in the run loop that mirrorsexcept RunStopped:kill_session(...), setself.state.stopped = True, journal arun-stopwithreason="KeyboardInterrupt", and re-raise when nested. This closes the orphan-window / ambiguous-status gap on the Windows console-ctrl path.Tests
_save()is called (session on disk) beforepost_sessionis emitted, and that a simulated host-kill afterrecord_sessionleaves a recoverable completed session rather than a staledev-runningtask.attach_session_usage: usage folds into totals for the matching session;KeyErroron unknowntask_id; no-op onNone.KeyboardInterruptbranch: run loop setsstopped, kills the session, journals the stop, and re-raises when nested.Scope / non-goals
SESSION_END_TRACE_FILE/trace_runbreadcrumb scaffolding that accompanied the reference patches — it was diagnostic weight for a since-closed engine-death investigation and should not land.