Skip to content

Commit 534e118

Browse files
authored
fix(engine): session-end durability + raw KeyboardInterrupt handling (#62)
Session-end durability barrier: record completed session before post-session hooks; usage folded in afterward as best-effort metadata. Resume continuation consumes durably-recorded dev/review results into verify/decide instead of restart-rollback. Raw KeyboardInterrupt branch records a controlled stop. Replay preserves attempt/cycle/baseline. Closes #57
1 parent ea01d9e commit 534e118

4 files changed

Lines changed: 429 additions & 25 deletions

File tree

src/bmad_loop/engine.py

Lines changed: 135 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from __future__ import annotations
1010

1111
import contextlib
12+
import functools
1213
import shutil
1314
import signal
1415
import sys
@@ -245,6 +246,22 @@ def run(self) -> RunSummary:
245246
raise # nested auto-sweep: let the owner record the stop
246247
self.state.stopped = True
247248
self.journal.append("run-stop")
249+
except KeyboardInterrupt:
250+
# Some Windows console/control events can still surface as a raw
251+
# KeyboardInterrupt without routing through the installed signal
252+
# handler. Persist a controlled stop rather than letting the
253+
# engine disappear with stale state.
254+
self._stopping = True # swallow stop signals landing mid-teardown
255+
try:
256+
kill_session(self.state.run_id)
257+
except (
258+
BaseException
259+
): # noqa: BLE001 # nosec B110 - best-effort teardown; the stop must still record
260+
pass
261+
if self._is_nested:
262+
raise
263+
self.state.stopped = True
264+
self.journal.append("run-stop", reason="KeyboardInterrupt")
248265
except Exception as exc:
249266
# an unexpected exception escaped the loop (e.g. a transport
250267
# hang that leaked past the seam). Don't let it die to the lossy
@@ -1043,6 +1060,36 @@ def _finish_inflight(self) -> None:
10431060
self._integrate_unit(task, unit)
10441061
else:
10451062
self._review_and_commit(task)
1063+
elif (resumable := self._resumable_session(task)) is not None:
1064+
# the host died inside the post-session window: the session
1065+
# itself completed and its recorded result is on disk, so
1066+
# continue into the normal verify/decide pipeline instead of
1067+
# rolling the finished work back through resume-restart.
1068+
role, result = resumable
1069+
self.journal.append("resume-verify", story_key=task.story_key, role=role)
1070+
if role == "dev":
1071+
# deliberate reset like the restart arm: _dev_phase re-enters
1072+
# its loop and consumes the recorded result instead of
1073+
# running a session
1074+
task.phase = Phase.PENDING
1075+
continuation = functools.partial(self._drive_story, task, dev_resume=result)
1076+
else:
1077+
# deliberate reset to the legal pre-review phase
1078+
task.phase = Phase.DEV_VERIFY
1079+
continuation = functools.partial(
1080+
self._review_and_commit, task, resume_result=result
1081+
)
1082+
if isolated:
1083+
unit = self._reopen_unit(task)
1084+
prev = self.workspace
1085+
self.workspace = unit.workspace
1086+
try:
1087+
continuation()
1088+
finally:
1089+
self.workspace = prev
1090+
self._integrate_unit(task, unit)
1091+
else:
1092+
continuation()
10461093
else:
10471094
self.journal.append(
10481095
"resume-restart", story_key=task.story_key, phase=str(task.phase)
@@ -1062,6 +1109,33 @@ def _finish_inflight(self) -> None:
10621109
self._save()
10631110
self._run_story(task)
10641111

1112+
def _resumable_session(self, task: StoryTask) -> tuple[str, SessionResult] | None:
1113+
"""The in-flight session's durably-recorded result, when complete enough
1114+
to act on: the task died mid-phase but its current attempt/cycle record
1115+
is ``completed`` and carries the parsed result. Consumes only evidence
1116+
the adapter vouched for at session end — no artifact re-scan, no
1117+
loosening of completion authority. Anything less returns None and the
1118+
caller falls through to resume-restart."""
1119+
if task.phase == Phase.DEV_RUNNING:
1120+
role, seq = "dev", task.attempt
1121+
elif task.phase == Phase.REVIEW_RUNNING:
1122+
role, seq = "review", task.review_cycle
1123+
else:
1124+
return None
1125+
task_id = f"{task.story_key}-{role}-{seq}"
1126+
for record in reversed(task.sessions):
1127+
if record.task_id != task_id:
1128+
continue
1129+
if record.status != "completed" or record.result_json is None:
1130+
return None
1131+
return role, SessionResult(
1132+
status="completed",
1133+
result_json=record.result_json,
1134+
session_id=record.session_id,
1135+
transcript_path=record.transcript_path,
1136+
)
1137+
return None
1138+
10651139
# ------------------------------------------------------------- per story
10661140

10671141
def _gate_unit(self, task: StoryTask) -> bool:
@@ -1269,8 +1343,8 @@ def _run_story(self, task: StoryTask) -> None:
12691343
self._drive_story(task)
12701344
self._emit("post_story", task)
12711345

1272-
def _drive_story(self, task: StoryTask) -> None:
1273-
if not self._dev_phase(task):
1346+
def _drive_story(self, task: StoryTask, dev_resume: SessionResult | None = None) -> None:
1347+
if not self._dev_phase(task, resume_result=dev_resume):
12741348
return
12751349
if gates.pause_after_spec(self.policy):
12761350
gates.notify(
@@ -1286,24 +1360,40 @@ def _drive_story(self, task: StoryTask) -> None:
12861360
)
12871361
self._review_and_commit(task)
12881362

1289-
def _dev_phase(self, task: StoryTask) -> bool:
1363+
def _dev_phase(self, task: StoryTask, resume_result: SessionResult | None = None) -> bool:
12901364
if self._vetoed(self._emit("pre_dev_phase", task), task):
12911365
return False
1292-
task.baseline_commit = verify.rev_parse_head(self.workspace.root)
1293-
# snapshot untracked files now so a later rollback removes only what THIS
1294-
# attempt creates, never files the user already had on disk.
1295-
task.baseline_untracked = sorted(verify.untracked_files(self.workspace.root))
1366+
if resume_result is None:
1367+
task.baseline_commit = verify.rev_parse_head(self.workspace.root)
1368+
# snapshot untracked files now so a later rollback removes only what
1369+
# THIS attempt creates, never files the user already had on disk.
1370+
# A resumed result keeps the persisted baseline: re-capturing here
1371+
# would shift the rollback/squash reference onto the completed
1372+
# session's own tree.
1373+
task.baseline_untracked = sorted(verify.untracked_files(self.workspace.root))
12961374
feedback: Path | None = None
12971375
while True:
1298-
task.attempt += 1
1376+
if resume_result is None:
1377+
# a resumed result replays the attempt it was recorded under, so
1378+
# the counter (and the session task_id derived from it) must not
1379+
# advance; a second host death then still finds the record and
1380+
# re-enters this continuation instead of falling back to restart.
1381+
task.attempt += 1
12991382
advance(task, Phase.DEV_RUNNING)
13001383
self._save()
1301-
result = self._run_session(
1302-
task,
1303-
role="dev",
1304-
prompt=self._dev_prompt(task, feedback),
1305-
seq=task.attempt,
1306-
)
1384+
if resume_result is not None:
1385+
# the session already ran before the host died; its recorded
1386+
# result re-enters the verify/decide pipeline. Consumed exactly
1387+
# once — later iterations run sessions normally.
1388+
result = resume_result
1389+
resume_result = None
1390+
else:
1391+
result = self._run_session(
1392+
task,
1393+
role="dev",
1394+
prompt=self._dev_prompt(task, feedback),
1395+
seq=task.attempt,
1396+
)
13071397
advance(task, Phase.DEV_VERIFY)
13081398
outcome = None
13091399
if result.status == "completed":
@@ -1384,7 +1474,9 @@ def _record_dev_spec(self, task: StoryTask, result_json: dict | None) -> None:
13841474
if spec_path.is_file():
13851475
task.spec_file = str(spec_path)
13861476

1387-
def _review_and_commit(self, task: StoryTask) -> None:
1477+
def _review_and_commit(
1478+
self, task: StoryTask, resume_result: SessionResult | None = None
1479+
) -> None:
13881480
if not self.policy.review.enabled:
13891481
# review.enabled = false: the bmad-dev-auto session's own inline
13901482
# review is the only review; verify the deterministic gates + commit.
@@ -1418,16 +1510,27 @@ def _review_and_commit(self, task: StoryTask) -> None:
14181510
# only state the budget-exhaustion rescue below is allowed to commit.
14191511
refileable_followup = False
14201512
while task.review_cycle < self.policy.limits.max_review_cycles:
1421-
task.review_cycle += 1
1513+
if resume_result is None:
1514+
# a resumed result replays the cycle it was recorded under: the
1515+
# counter must not advance, or the replay burns a review-budget
1516+
# slot and mislabels its journal/session ids.
1517+
task.review_cycle += 1
14221518
refileable_followup = False # only a completed pass this cycle can set it
14231519
advance(task, Phase.REVIEW_RUNNING)
14241520
self._save()
1425-
result = self._run_session(
1426-
task,
1427-
role="review",
1428-
prompt=self._review_prompt(task),
1429-
seq=task.review_cycle,
1430-
)
1521+
if resume_result is not None:
1522+
# the session already ran before the host died; its recorded
1523+
# result re-enters the decision pipeline. Consumed exactly
1524+
# once — later cycles run sessions normally.
1525+
result = resume_result
1526+
resume_result = None
1527+
else:
1528+
result = self._run_session(
1529+
task,
1530+
role="review",
1531+
prompt=self._review_prompt(task),
1532+
seq=task.review_cycle,
1533+
)
14311534
advance(task, Phase.REVIEW_VERIFY)
14321535
self._save()
14331536
self._emit(
@@ -1835,23 +1938,31 @@ def _run_session(
18351938
self.journal.set_active_log(task_id)
18361939
self.journal.append("session-start", task_id=task_id, role=role, prompt=prompt)
18371940
result = adapter.run(spec)
1838-
usage = adapter.read_usage(result)
18391941
task.record_session(
18401942
SessionRecord(
18411943
task_id=task_id,
18421944
role=role,
18431945
status=result.status,
18441946
session_id=result.session_id,
18451947
transcript_path=result.transcript_path,
1846-
usage=usage,
1948+
result_json=result.result_json,
18471949
)
18481950
)
1951+
# Make the completed session durable before the usage read, post-session
1952+
# hooks, and follow-up verification. If the host kills the process in
1953+
# that window, the resume path can see the session instead of a stale
1954+
# dev-running task with no evidence; usage stays best-effort metadata,
1955+
# not a durability gate.
1956+
self._save()
1957+
usage = adapter.read_usage(result)
1958+
task.attach_session_usage(task_id, usage)
18491959
self.journal.append(
18501960
"session-end",
18511961
task_id=task_id,
18521962
status=result.status,
18531963
tokens=usage.total if usage else None,
18541964
)
1965+
self._save()
18551966
self._emit(
18561967
"post_session",
18571968
task,

src/bmad_loop/model.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,9 @@ class SessionRecord:
9191
session_id: str | None = None
9292
transcript_path: str | None = None
9393
usage: TokenUsage | None = None
94+
# the session's parsed result payload, persisted so a durably-saved
95+
# completed session is actionable on resume, not just forensics
96+
result_json: dict[str, Any] | None = None
9497

9598
def to_dict(self) -> dict[str, Any]:
9699
return {
@@ -100,6 +103,7 @@ def to_dict(self) -> dict[str, Any]:
100103
"session_id": self.session_id,
101104
"transcript_path": self.transcript_path,
102105
"usage": self.usage.to_dict() if self.usage else None,
106+
"result_json": self.result_json,
103107
}
104108

105109
@classmethod
@@ -112,6 +116,7 @@ def from_dict(cls, d: dict[str, Any]) -> "SessionRecord":
112116
session_id=d.get("session_id"),
113117
transcript_path=d.get("transcript_path"),
114118
usage=TokenUsage.from_dict(usage) if usage else None,
119+
result_json=d.get("result_json"),
115120
)
116121

117122

@@ -168,6 +173,21 @@ def record_session(self, record: SessionRecord) -> None:
168173
if record.usage:
169174
self.tokens.add(record.usage)
170175

176+
def attach_session_usage(self, task_id: str, usage: TokenUsage | None) -> None:
177+
"""Fold usage into the most recent session for `task_id`. Usage is
178+
best-effort metadata attached after the session itself is saved, so a
179+
failed usage read never costs the recorded session."""
180+
if usage is None:
181+
return
182+
for record in reversed(self.sessions):
183+
if record.task_id != task_id:
184+
continue
185+
if record.usage is None:
186+
record.usage = usage
187+
self.tokens.add(usage)
188+
return
189+
raise KeyError(task_id)
190+
171191
def to_dict(self) -> dict[str, Any]:
172192
return {
173193
"story_key": self.story_key,

0 commit comments

Comments
 (0)