diff --git a/src/bmad_loop/adapters/generic.py b/src/bmad_loop/adapters/generic.py index 6fdda519..d02136e7 100644 --- a/src/bmad_loop/adapters/generic.py +++ b/src/bmad_loop/adapters/generic.py @@ -637,6 +637,15 @@ def wait_for_completion(self, handle: SessionHandle, spec: SessionSpec) -> Sessi # wall clock stepped backward must not stretch the session). wall_deadline = time.time() + spec.timeout_s session_id: str | None = None + # The first identified SessionStart belongs to the CLI session this + # adapter launched. Nested CLIs can inherit the hook relay environment + # and write into the same task event stream, so their lifecycle events + # must never be allowed to replace this identity or score this session. + # Events without an ID deliberately retain the legacy compatibility + # path below: there is no reliable attribution signal to enforce. + outer_session_id: str | None = None + session_start_seen = False + expects_session_start = "SessionStart" in self.profile.hooks.events.values() transcript_path: str | None = None nudges_left = self._stop_nudges # Positive grace arms at launch for dev/review sessions, so a CLI that @@ -999,6 +1008,61 @@ def wait_for_completion(self, handle: SessionHandle, spec: SessionSpec) -> Sessi stop_seen=stop_seen, ) continue + # Bind task attribution only from the launched session's first + # identified SessionStart. Once bound, reject a differently + # identified event before it can change identity, transcript, + # completion, or retry-driving state. This is intentionally ahead + # of profile-specific subagent filtering too: the diagnostic should + # cover every foreign lifecycle event while exposing no transcript + # or prompt payload. + if event.event == "SessionStart": + if event.session_id and outer_session_id is None: + outer_session_id = event.session_id + session_start_seen = True + elif event.session_id and event.session_id != outer_session_id: + self._note_lifecycle( + handle.task_id, + "foreign-hook-event-ignored", + hook_event=event.event, + foreign_session_id=event.session_id, + ) + continue + elif event.session_id: + session_start_seen = True + elif ( + outer_session_id is not None + and event.session_id + and event.session_id != outer_session_id + ): + self._note_lifecycle( + handle.task_id, + "foreign-hook-event-ignored", + hook_event=event.event, + foreign_session_id=event.session_id, + ) + continue + elif ( + expects_session_start + and not session_start_seen + and event.event == "SessionEnd" + and event.session_id + ): + # A profile that declares SessionStart has not established even + # the beginning of its launched session yet. An identified + # session-death event on the shared task channel is therefore + # unattributable and must fail closed instead of crashing on a + # child. Stop remains on the compatibility path: several CLIs + # and established adapters can validly deliver it without an + # observed SessionStart, while SessionEnd is the crash signal + # that caused this regression. Stop-only profiles likewise + # cannot establish this proof and retain their existing path. + self._note_lifecycle( + handle.task_id, + "unattributed-hook-event-ignored", + hook_event=event.event, + foreign_session_id=event.session_id, + ) + continue if ( event.event == "Stop" and self.profile.subagent_stop_without_transcript diff --git a/tests/test_generic_tmux.py b/tests/test_generic_tmux.py index 3022698a..e21e99c7 100644 --- a/tests/test_generic_tmux.py +++ b/tests/test_generic_tmux.py @@ -659,10 +659,10 @@ def wait_for(self, task_id, kinds, timeout_s, since_ns=0): return self._events.pop(0) if self._events else None -def _stop_event(task_id, session_id, transcript_path): +def _hook_event(task_id, event, session_id=None, transcript_path=None): return HookEvent( ts=1, - event="Stop", + event=event, task_id=task_id, session_id=session_id, transcript_path=transcript_path, @@ -670,6 +670,10 @@ def _stop_event(task_id, session_id, transcript_path): ) +def _stop_event(task_id, session_id, transcript_path): + return _hook_event(task_id, "Stop", session_id, transcript_path) + + def _dev_handle(launched_ns=0) -> SessionHandle: return SessionHandle(task_id="3-1-dev-1", native_id="@1", launched_ns=launched_ns) @@ -1185,6 +1189,170 @@ def flush_terminal_spec(call_n): assert result.session_id == "main-sess" # the subagent's toolu_ id is never recorded +def test_wait_for_completion_ignores_foreign_identified_lifecycle_events(tmp_path): + """A nested CLI may inherit the outer task's hook relay, but must not be + allowed to overwrite its session identity or terminate its completion loop.""" + adapter, impl = make_dev_adapter(tmp_path) + (impl / "spec-3-1-foo.md").write_text( + "---\nstatus: done\n---\n\n## Auto Run Result\n\nStatus: done\n" + ) + outer_id = "outer-session" + child_id = "nested-child" + adapter.watcher = _ScriptedWatcher( + [ + _hook_event("3-1-dev-1", "SessionStart", outer_id, "/outer.jsonl"), + _hook_event("3-1-dev-1", "SessionStart", child_id, "/child.jsonl"), + _stop_event("3-1-dev-1", child_id, "/child.jsonl"), + _hook_event("3-1-dev-1", "SessionEnd", child_id, "/child.jsonl"), + _stop_event("3-1-dev-1", outer_id, "/outer.jsonl"), + ] + ) + + result = adapter.wait_for_completion(_dev_handle(), _dev_spec(tmp_path)) + + assert result.status == "completed" + assert result.session_id == outer_id + assert result.transcript_path == "/outer.jsonl" + assert result.stop_seen is True + ignored = [ + entry + for entry in _lifecycle_lines(adapter) + if entry["event"] == "foreign-hook-event-ignored" + ] + assert [entry["hook_event"] for entry in ignored] == ["SessionStart", "Stop", "SessionEnd"] + assert [entry["foreign_session_id"] for entry in ignored] == [child_id] * 3 + assert all( + set(entry) == {"ts", "event", "hook_event", "foreign_session_id"} for entry in ignored + ) + assert "/child.jsonl" not in json.dumps(ignored) + + +def test_wait_for_completion_ignores_identified_session_end_before_session_start(tmp_path): + """An identified SessionEnd cannot crash a SessionStart-capable profile + before the launched session has emitted its own start evidence.""" + adapter, impl = make_dev_adapter(tmp_path) + (impl / "spec-3-1-foo.md").write_text( + "---\nstatus: done\n---\n\n## Auto Run Result\n\nStatus: done\n" + ) + outer_id = "outer-session" + child_id = "early-nested-child" + adapter.watcher = _ScriptedWatcher( + [ + _hook_event("3-1-dev-1", "SessionEnd", child_id, "/child.jsonl"), + _hook_event("3-1-dev-1", "SessionStart", outer_id, "/outer.jsonl"), + _stop_event("3-1-dev-1", outer_id, "/outer.jsonl"), + ] + ) + + result = adapter.wait_for_completion(_dev_handle(), _dev_spec(tmp_path)) + + assert result.status == "completed" + assert result.session_id == outer_id + assert result.transcript_path == "/outer.jsonl" + ignored = [ + entry + for entry in _lifecycle_lines(adapter) + if entry["event"] == "unattributed-hook-event-ignored" + ] + assert [entry["hook_event"] for entry in ignored] == ["SessionEnd"] + assert all( + set(entry) == {"ts", "event", "hook_event", "foreign_session_id"} for entry in ignored + ) + assert "/child.jsonl" not in json.dumps(ignored) + + +def test_wait_for_completion_ignores_child_end_after_unidentified_session_start(tmp_path): + """An unidentified SessionStart cannot attribute a later identified end. + + The launched parent must remain live until its own identified start and stop + arrive, even when a nested child shares the hook relay in between. + """ + adapter, impl = make_dev_adapter(tmp_path) + (impl / "spec-3-1-foo.md").write_text( + "---\nstatus: done\n---\n\n## Auto Run Result\n\nStatus: done\n" + ) + outer_id = "outer-session" + child_id = "early-nested-child" + adapter.watcher = _ScriptedWatcher( + [ + _hook_event("3-1-dev-1", "SessionStart", transcript_path="/unknown.jsonl"), + _hook_event("3-1-dev-1", "SessionEnd", child_id, "/child.jsonl"), + _hook_event("3-1-dev-1", "SessionStart", outer_id, "/outer.jsonl"), + _stop_event("3-1-dev-1", outer_id, "/outer.jsonl"), + ] + ) + + result = adapter.wait_for_completion(_dev_handle(), _dev_spec(tmp_path)) + + assert result.status == "completed" + assert result.session_id == outer_id + assert result.transcript_path == "/outer.jsonl" + ignored = [ + entry + for entry in _lifecycle_lines(adapter) + if entry["event"] == "unattributed-hook-event-ignored" + ] + assert [entry["hook_event"] for entry in ignored] == ["SessionEnd"] + assert [entry["foreign_session_id"] for entry in ignored] == [child_id] + + +def test_wait_for_completion_keeps_identified_stop_for_stop_only_profile(tmp_path): + """Profiles without SessionStart cannot supply the attribution proof, so + their established identified-Stop completion behavior must remain intact.""" + adapter, impl = make_dev_adapter(tmp_path, profile_name="antigravity") + (impl / "spec-3-1-foo.md").write_text( + "---\nstatus: done\n---\n\n## Auto Run Result\n\nStatus: done\n" + ) + adapter.watcher = _ScriptedWatcher( + [_stop_event("3-1-dev-1", "stop-only-session", "/legacy.jsonl")] + ) + + result = adapter.wait_for_completion(_dev_handle(), _dev_spec(tmp_path)) + + assert result.status == "completed" + assert result.session_id == "stop-only-session" + assert result.transcript_path == "/legacy.jsonl" + assert _lifecycle_lines(adapter) == [] + + +def test_wait_for_completion_keeps_matching_parent_session_end_crash(tmp_path): + adapter, _ = make_dev_adapter(tmp_path) + outer_id = "outer-session" + adapter.watcher = _ScriptedWatcher( + [ + _hook_event("3-1-dev-1", "SessionStart", outer_id, "/outer.jsonl"), + _hook_event("3-1-dev-1", "SessionEnd", outer_id, "/outer.jsonl"), + ] + ) + + result = adapter.wait_for_completion(_dev_handle(), _dev_spec(tmp_path)) + + assert result.status == "crashed" + assert result.session_id == outer_id + assert result.transcript_path == "/outer.jsonl" + assert _lifecycle_lines(adapter) == [] + + +def test_wait_for_completion_preserves_no_id_hook_compatibility(tmp_path): + adapter, impl = make_dev_adapter(tmp_path) + (impl / "spec-3-1-foo.md").write_text( + "---\nstatus: done\n---\n\n## Auto Run Result\n\nStatus: done\n" + ) + adapter.watcher = _ScriptedWatcher( + [ + _hook_event("3-1-dev-1", "SessionStart", transcript_path="/legacy.jsonl"), + _stop_event("3-1-dev-1", None, "/legacy.jsonl"), + ] + ) + + result = adapter.wait_for_completion(_dev_handle(), _dev_spec(tmp_path)) + + assert result.status == "completed" + assert result.session_id is None + assert result.transcript_path == "/legacy.jsonl" + assert _lifecycle_lines(adapter) == [] + + def test_wait_for_completion_transcriptless_stop_is_terminal_without_flag(tmp_path): """Gating: a profile without subagent_stop_without_transcript (claude) still treats every Stop as the main turn-end, so a result-less one stalls the dev