feat(adapters): post-kill reconcile rescues finished stalled/timeout sessions

CHANGELOG.md

@@ -31,6 +31,18 @@ breaking changes may land in a minor release.
 
 ### Fixed
 
+- **A finished session whose final `Stop` hook was lost no longer loses its work.** A dev/review
+  session that wrote its terminal spec but never delivered the `Stop` ended `stalled` — or `timeout`,
+  when hooks were misconfigured and no event ever arrived — and the on-disk result was discarded.
+  The adapter now re-reads the spec after the window is provably dead, rescuing a self-consistent
+  successful terminal; every rescue still faces the full deterministic verify, and the journal records
+  `session-rescued-post-kill` so it stays distinguishable from a live completion. (#95, closes #61)
+- **A corrupt terminal artifact no longer crashes the whole run.** A spec truncated mid-write (a
+  multi-byte UTF-8 sequence cut in half) raised out of the read-back and past the per-task boundary,
+  marking the run `CRASHED` and abandoning every remaining story. The read-back now degrades an
+  undecodable spec to "no result yet" — the session retries or keeps its verdict — and the post-kill
+  rescue additionally keeps its verdict on _any_ read fault, so a best-effort rescue can never make
+  things worse. The repair path still raises on purpose. (#95, closes #96)
 - **Windows installs now pull `psutil` automatically** — moved from the opt-in `non-linux` extra to a
   platform-scoped core dependency (`sys_platform == 'win32'`), so the TUI liveness column no longer
   shows every run as `?` on a stock install. macOS keeps the `non-linux` extra; Linux stays dep-free.

src/bmad_loop/adapters/base.py

@@ -91,6 +91,20 @@ def read_usage(self, result: SessionResult) -> TokenUsage | None:
     def run(self, spec: SessionSpec) -> SessionResult:
         handle = self.start_session(spec)
         try:
-            return self.wait_for_completion(handle, spec)
+            result = self.wait_for_completion(handle, spec)
         finally:
             self.kill(handle)
+        return self._post_kill_reconcile(handle, spec, result)
+
+    def _post_kill_reconcile(
+        self, handle: SessionHandle, spec: SessionSpec, result: SessionResult
+    ) -> SessionResult:
+        """Last-chance reconcile after the session's window has been torn down.
+
+        Runs only on the normal return path — a raising wait_for_completion
+        still kills the window and propagates without reaching this hook.
+        Base behavior: identity. Adapters whose completion trust keys on
+        window death (see GenericDevAdapter) may re-inspect on-disk state here,
+        now that the kill has settled the liveness question a live-window
+        verdict had to leave open."""
+        return result

src/bmad_loop/adapters/generic.py

@@ -459,12 +459,18 @@ def _artifact_dirs(self, cwd: Path) -> list[Path]:
         return dirs
 
     def _result_json(self, handle: SessionHandle, spec: SessionSpec, *, wait: bool) -> dict | None:
+        sr = self._synth_result(handle, spec, wait=wait)
+        return sr.result_json if sr is not None else None
+
+    def _synth_result(
+        self, handle: SessionHandle, spec: SessionSpec, *, wait: bool
+    ) -> devcontract.SynthResult | None:
         # Stories mode (folder+id dispatch): the story spec lives at a
         # deterministic id-keyed path, so resolve it directly instead of the
         # mtime-floor scan. The engine exports BMAD_LOOP_SPEC_FOLDER only for
         # stories runs, so sprint/sweep runs keep the scan path below unchanged.
         if spec.env.get("BMAD_LOOP_SPEC_FOLDER"):
-            return self._stories_result_json(handle, spec, wait=wait)
+            return self._stories_synth_result(handle, spec, wait=wait)
         # Mirror the base _await_result poll: the skill's terminal spec may not be
         # flushed to disk the instant the Stop event fires, so briefly await it when
         # wait=True instead of reading once and mis-reporting a stall.
@@ -482,14 +488,14 @@ def _result_json(self, handle: SessionHandle, spec: SessionSpec, *, wait: bool)
                     dw_ids = [tok for tok in (i.strip() for i in raw_dw_ids) if tok]
                     return devcontract.synthesize_result(
                         spec_path, story_key=story_key, dw_ids=dw_ids or None
-                    ).result_json
+                    )
             if not wait or time.monotonic() >= deadline:
                 return None
             time.sleep(RESULT_POLL_S)
 
-    def _stories_result_json(
+    def _stories_synth_result(
         self, handle: SessionHandle, spec: SessionSpec, *, wait: bool
-    ) -> dict | None:
+    ) -> devcontract.SynthResult | None:
         """Deterministic stories-mode read-back: resolve ``<spec-folder>/stories/
         <id>-*.md`` by id (never the mtime scan) and synthesize from it.
 
@@ -530,9 +536,9 @@ def _stories_result_json(
                 and self._written_this_session(state.path, handle.launched_ns)
             ):
                 try:
-                    result_json = devcontract.synthesize_result(
+                    sr = devcontract.synthesize_result(
                         state.path, story_key=story_key or None, plan_halt=plan_halt
-                    ).result_json
+                    )
                 except UnicodeDecodeError:
                     # A non-UTF-8 read is either a torn glimpse of a spec still
                     # being written (keep polling — a later pass sees the finished
@@ -541,9 +547,9 @@ def _stories_result_json(
                     # wedge (resolve_story_spec degrades an undecodable PRESENT
                     # spec to status "" → pause for resolve), never a crash of
                     # the read-back poll.
-                    result_json = None
-                if result_json is not None:
-                    return result_json
+                    sr = None
+                if sr is not None and sr.result_json is not None:
+                    return sr
             if not wait or time.monotonic() >= deadline:
                 return None
             time.sleep(RESULT_POLL_S)
@@ -560,6 +566,70 @@ def _written_this_session(spec_path: Path, launched_ns: int) -> bool:
         except OSError:
             return False
 
+    def _post_kill_reconcile(
+        self, handle: SessionHandle, spec: SessionSpec, result: SessionResult
+    ) -> SessionResult:
+        """Rescue a finished-but-unvouched session once its window is dead (#61).
+
+        A session that wrote its terminal spec but whose final Stop event was
+        lost ends ``stalled`` (nudge-unresponsive under a live window, where
+        the artifact is advisory — the #48/#53 invariant), or ``timeout`` when
+        no hook event ever arrived (hook misconfig, events-dir write failure —
+        that path never arms the stall grace at all). Both verdicts discard
+        the on-disk result solely because the window was alive to distrust;
+        ``run()``'s kill has since settled that the way window death already
+        vouches for the crash path. So: re-probe, and only on a provably dead
+        window re-run the same read-back a delivered Stop would have run.
+
+        The gate is deliberately stricter than the crash path's
+        accept-any-terminal: the synthesis must be self-consistent
+        (``status_consistent`` — "no active disagreement"; a blank frontmatter
+        with prose ``done`` passes, exactly what a delivered Stop would have
+        synthesized, and the engine's reconcile repairs the lag) and a
+        *successful* terminal — ``done``, or the stories plan-halt leg (a
+        deliberate widening of #61's literal done-only wording). A ``blocked``
+        terminal is never rescued: it carries no finished work, and
+        blocked-plus-nudge-unresponsive is weak evidence of anything. Every
+        rescue still runs the engine's full deterministic verify downstream,
+        so a bogus upgrade degrades into an ordinary verify-failed retry. A
+        cap-exhausted injected-workflow stall whose marker landed before the
+        kill is rescued by the same trust model."""
+        if result.status not in ("stalled", "timeout") or result.result_json is not None:
+            return result
+        try:
+            if self._window_alive(handle):
+                # The kill silently failed (best-effort teardown): the window
+                # is still alive, so the live-window invariant still applies.
+                return result
+        except MultiplexerError:
+            return result  # liveness unknowable: unknown is not dead
+        try:
+            sr = self._synth_result(handle, spec, wait=False)
+        except (OSError, UnicodeDecodeError):
+            # An unreadable artifact is not evidence a session finished. This
+            # hook runs right after run()'s finally-kill — the moment a spec the
+            # CLI was mid-write is truncated, possibly through a multi-byte UTF-8
+            # sequence — so a corrupt read is the *expected* fault here, not an
+            # anomaly. Keep the verdict: a best-effort rescue must never escalate
+            # a clean stall/timeout into an exception, which the engine does not
+            # contain per-task (it fails the whole run). UnicodeDecodeError is a
+            # ValueError, so both must be named.
+            return result
+        if sr is None or sr.result_json is None or not sr.status_consistent:
+            return result
+        rj = sr.result_json
+        if rj.get("escalations") or not (
+            rj.get("status") == devcontract.DONE or rj.get("plan_halt") is True
+        ):
+            return result
+        rj["post_kill_reconciled"] = True
+        return SessionResult(
+            status="completed",
+            result_json=rj,
+            session_id=result.session_id,
+            transcript_path=result.transcript_path,
+        )
+
 
 # Back-compat alias: the adapter was ``GenericTmuxAdapter`` before tmux moved
 # behind the multiplexer seam. Keeps existing imports stable.

src/bmad_loop/devcontract.py

@@ -174,6 +174,25 @@ class SynthResult:
     status_consistent: bool
 
 
+def _read_text_or_empty(path: Path) -> str:
+    """Read a spec on the *read-back* path, degrading an unreadable file to "".
+
+    An absent, binary/truncated, or unreadable spec carries no parseable result
+    section, so it reads exactly like a spec that has not terminated yet — the
+    caller then waits, nudges, or keeps its verdict. Never a crash: this runs on
+    the observation path, where the orchestrator's job is to classify what it
+    finds, not to trust it. (UnicodeDecodeError is a ValueError, not an OSError.)
+
+    The *repair* path deliberately does the opposite — `reset_spec_status` and
+    `strip_auto_run_result` let an unreadable spec raise, because silently
+    skipping a rewrite leaves the spec in a state the caller believes it fixed.
+    """
+    try:
+        return path.read_text(encoding="utf-8")
+    except (OSError, UnicodeDecodeError):
+        return ""
+
+
 def synthesize_result(
     spec_path: Path,
     *,
@@ -207,9 +226,7 @@ def synthesize_result(
     """
     fm = read_frontmatter(spec_path)
     fm_status = str(fm.get("status", "")).strip().lower()
-    arr = parse_auto_run_result(
-        spec_path.read_text(encoding="utf-8") if spec_path.is_file() else ""
-    )
+    arr = parse_auto_run_result(_read_text_or_empty(spec_path))
 
     terminal = (DONE, BLOCKED, PLAN_HALT_STATUS) if plan_halt else (DONE, BLOCKED)
     # Not terminal yet: no result section AND frontmatter not at a terminal state.
@@ -284,7 +301,11 @@ def find_result_artifact(impl_artifacts: Path, *, since_ns: int) -> Path | None:
         if not path.name.startswith(FALLBACK_RESULT_PREFIX):
             try:
                 text = path.read_text(encoding="utf-8")
-            except OSError:
+            except (OSError, UnicodeDecodeError):
+                # A binary/truncated candidate cannot be shown to carry a terminal
+                # section, so it does not qualify — skip it exactly like an
+                # unreadable one. UnicodeDecodeError is a ValueError, so a bare
+                # `except OSError` let a torn mid-write spec crash the scan.
                 continue
             if not _section_headings(text):
                 continue

src/bmad_loop/engine.py

@@ -2066,6 +2066,10 @@ def _run_session(
         self.journal.set_active_log(task_id)
         self.journal.append("session-start", task_id=task_id, role=role, prompt=prompt)
         result = adapter.run(spec)
+        # A post-kill rescue (#61) is otherwise indistinguishable from a normal
+        # completion in the journal; leave a breadcrumb for forensics.
+        if result.result_json is not None and result.result_json.get("post_kill_reconciled"):
+            self.journal.append("session-rescued-post-kill", task_id=task_id, role=role)
         # Only dev/review sessions are resumable — `_resumable_session` matches
         # exactly those task ids under DEV_RUNNING/REVIEW_RUNNING. For everything
         # else (triage/sweep, labeled plugin-workflow sessions) the payload is

tests/test_devcontract.py

@@ -1,5 +1,6 @@
 """Tests for the generic bmad-dev-auto -> result.json translation shim."""
 
+import os
 from pathlib import Path
 
 import pytest
@@ -339,6 +340,43 @@ def test_find_artifact_ignores_heading_in_longer_outer_fence(tmp_path):
     assert devcontract.find_result_artifact(tmp_path, since_ns=0) is None
 
 
+# The read-back decodes artifacts as UTF-8. A spec truncated mid-write (the CLI
+# was killed) can end inside a multi-byte sequence; `read_text(encoding="utf-8")`
+# then raises UnicodeDecodeError — a ValueError, NOT an OSError.
+_BAD_UTF8 = b"\xff\xfe\x00\x01 not utf-8 \x80\x81"
+
+
+def test_find_artifact_skips_non_utf8_spec(tmp_path):
+    """A binary/truncated candidate cannot be shown to carry a terminal section, so
+    it does not qualify — and must be skipped, not raised on, even though it is the
+    newest file. An older qualifying spec still wins."""
+    good = tmp_path / "spec-1-1-a.md"
+    good.write_text("---\nstatus: done\n---\n\n## Auto Run Result\n\nStatus: done\n")
+    torn = tmp_path / "spec-1-1-b.md"
+    torn.write_bytes(_BAD_UTF8)
+    os.utime(good, ns=(1_000_000_000, 1_000_000_000))
+    os.utime(torn, ns=(2_000_000_000, 2_000_000_000))  # newest, but unreadable
+    assert devcontract.find_result_artifact(tmp_path, since_ns=0) == good
+
+
+def test_find_artifact_skips_only_candidate_when_non_utf8(tmp_path):
+    (tmp_path / "spec-1-1-a.md").write_bytes(_BAD_UTF8)
+    assert devcontract.find_result_artifact(tmp_path, since_ns=0) is None
+
+
+def test_synthesize_result_non_utf8_fallback_marker_is_not_terminal(tmp_path):
+    """The no-spec fallback marker is matched by NAME, so the finder hands it back
+    without ever reading it — the decode fault lands here instead. An unreadable
+    spec carries no parseable result, so it reads exactly like one that has not
+    terminated yet: no result_json, no crash."""
+    marker = tmp_path / "bmad-dev-auto-result-unclear-1234.md"
+    marker.write_bytes(_BAD_UTF8)
+    assert devcontract.find_result_artifact(tmp_path, since_ns=0) == marker
+    sr = devcontract.synthesize_result(marker, story_key="1-1")
+    assert sr.result_json is None
+    assert sr.status_consistent is True
+
+
 # ----------------------------------------------------------- reset_spec_status
 
 

tests/test_engine.py

@@ -570,6 +570,35 @@ def test_happy_path(project):
     assert adapter.sessions[1].prompt.startswith("/bmad-dev-auto ")
 
 
+def test_post_kill_rescued_result_flows_and_journals(project):
+    """A result rescued by the adapter's post-kill reconcile (#61) reaches the
+    engine as an ordinary completed result — it must flow the completed path
+    (reconcile/verify/commit) unchanged, with the extra breadcrumb key riding
+    along harmlessly — plus one forensic journal entry, since the rescue is
+    otherwise indistinguishable from a live completion."""
+    write_sprint(project, {"epic-1": "backlog", "1-1-a": "ready-for-dev"})
+
+    inner = dev_effect(project, "1-1-a", followup_review=False)
+
+    def rescued_dev(spec):
+        result = inner(spec)
+        # what GenericDevAdapter._post_kill_reconcile returns: a synthesized
+        # result (status included) stamped with the rescue breadcrumb
+        result.result_json["status"] = "done"
+        result.result_json["post_kill_reconciled"] = True
+        return result
+
+    engine, _ = make_engine(project, [rescued_dev])
+    summary = engine.run()
+
+    assert summary.done == 1 and summary.deferred == 0 and not summary.paused
+    task = engine.state.tasks["1-1-a"]
+    assert task.phase == Phase.DONE
+    assert task.commit_sha and task.commit_sha != task.baseline_commit
+    kinds = [e["kind"] for e in engine.journal.entries()]
+    assert "session-rescued-post-kill" in kinds
+
+
 def test_inplace_ready_gate_veto_defers_before_any_session(project):
     """A plugin gating pre_ready_gate in non-isolated (in-place) mode — e.g. a
     shared-mode Unity engine waiting on the live Editor — defers the unit via the