Contain main-thread question/todos render crashes

_ask_question_blocking is now a containment boundary: a crash while
rendering a model-controlled question answers the pending question
with an error string (the worker blocked in _ask_sync unblocks), and
the message lands on the status bar instead of killing the TUI from
the main thread. Non-string question prompts are coerced before rich
rendering. Session.update_todos normalizes model-supplied todo items
(non-dict items, non-str fields) and _todos_panel tolerates malformed
data, fixing the same crash class for TodoWrite.

Author: beacoder

python_agent_harness/session/session.py

@@ -448,8 +448,30 @@ def connect_mcp(self) -> list[tuple[str, str]]:
     # ToolContext-facing API
     # ------------------------------------------------------------------
     def update_todos(self, todos: list[dict]) -> None:
-        """Store TODOS so the pinned TUI panel shows the current list."""
-        self.todos = list(todos)
+        """Store TODOS so the pinned TUI panel shows the current list.
+
+        Normalizes model-supplied items at the boundary: the panel and
+        row-cap math on the main render thread do ``todo.get(...)``
+        every frame, so anything that is not a {content, status}-like
+        dict would crash the TUI there (outside every worker-side tool
+        containment).  Non-dict items and missing "content" become a
+        deterministic string form instead.
+        """
+        normalized: list[dict] = []
+        if isinstance(todos, (list, tuple)):
+            for t in todos:
+                if isinstance(t, dict):
+                    item = dict(t)
+                    # coerce non-str fields: the panel does dict lookups
+                    # and str() rendering on the main thread every frame
+                    if "content" in item and not isinstance(item["content"], str):
+                        item["content"] = str(item["content"])
+                    if "status" in item and not isinstance(item["status"], str):
+                        item["status"] = str(item["status"])
+                    normalized.append(item)
+                else:
+                    normalized.append({"content": repr(t), "status": ""})
+        self.todos = normalized
         self.notify("todos")
 
     def clear_todos(self) -> None:

python_agent_harness/tui/input.py

@@ -354,6 +354,8 @@ def _render_frame(self) -> Any: ...
 
         def _flush(self) -> None: ...
 
+        def _on_log(self, msg: str) -> None: ...
+
     def _input_prompt(self) -> FormattedText:
         """Styled input prompt: short model name + caret.
 
@@ -396,18 +398,52 @@ def _read_multiline(self) -> str | None:
         return text
 
     def _ask_question_blocking(self) -> None:
+        """Answer the pending question on the main thread.
+
+        Containment boundary for the whole method: the question text is
+        model-controlled, and a render/resolve crash here would kill the
+        TUI from the main thread — outside every tool-containment
+        try/except in the worker (the worker only blocks on
+        ``q.event.wait`` in ``_ask_sync``).  On any unexpected exception
+        the question is answered with an error string (the blocked
+        worker unblocks and the failure reaches the model as a tool
+        result instead of wedging it forever), and the message goes to
+        the status bar.
+        """
         q = self.question
         if q is None:
             return
+        try:
+            answer = self._render_and_ask(q)
+        except (EOFError, KeyboardInterrupt):
+            raise
+        except Exception as e:  # noqa: BLE001 - containment boundary
+            self.question = None
+            q.answer = f"Error: question render failed — {e}"
+            q.event.set()
+            self._data_event.set()
+            self._on_log(f"error: question render failed — {e}")
+            return
+        q.answer = answer
+        q.event.set()
+        self.question = None
+        self._data_event.set()  # re-render promptly after the answer
+
+    def _render_and_ask(self, q: UiQuestion) -> str:
+        """Render the question UI and read one answer (may raise)."""
         self.console.print(self._render_frame())
         self.console.print()
         self._flush()
+        # prompt/options come from the model: coerce to str so a
+        # non-string "question" field can never raise rich's
+        # TypeError from Text.append / concatenation on the main thread
+        prompt_text = str(q.prompt)
         options = q.options or []
         keys = q.keys or []
         if keys and options and len(keys) == len(options):
             # keyed choices (e.g. y/n confirm): type the key to pick —
             # same list look as the Question tool, keys instead of numbers
-            self.console.print(Text(q.prompt))
+            self.console.print(Text(prompt_text))
             for key, opt in zip(keys, options, strict=True):
                 line = Text(f"  {key}) ", style="cyan")
                 line.append(opt)
@@ -419,7 +455,7 @@ def _ask_question_blocking(self) -> None:
             prompt = "> "
         elif options:
             # option labels get a numbered list: type the number to pick
-            self.console.print(Text(q.prompt))
+            self.console.print(Text(prompt_text))
             for i, opt in enumerate(options, 1):
                 line = Text(f"  {i}) ", style="cyan")
                 line.append(opt)
@@ -430,19 +466,15 @@ def _ask_question_blocking(self) -> None:
             self.console.print(f"[dim]{hint}[/dim]")
             prompt = "> "
         else:
-            prompt = q.prompt + " > "
+            prompt = prompt_text + " > "
         try:
             with _safe_patch_stdout():
                 answer = self.prompt_session.prompt(prompt, multiline=False)
         except (EOFError, KeyboardInterrupt):
             answer = ""
         if keys:
-            q.answer = _resolve_keyed_choice(answer, options, keys)
-        else:
-            q.answer = _resolve_numbered_choice(answer, options)
-        q.event.set()
-        self.question = None
-        self._data_event.set()  # re-render promptly after the answer
+            return _resolve_keyed_choice(answer, options, keys)
+        return _resolve_numbered_choice(answer, options)
 
     def _ask_sync(self, q: UiQuestion) -> str:
         """Block the worker thread until the main thread answers.

python_agent_harness/tui/render.py

@@ -270,14 +270,26 @@ def _build_history_rows(self, full: bool = False) -> list[Any]:
 
     def _todos_panel(self) -> Group | None:
         """Todos section — rebuilt every frame (not cached), so a
-        TodoWrite call shows up immediately even mid-run."""
+        TodoWrite call shows up immediately even mid-run.
+
+        Defensive against non-dict items / non-str fields: todos come
+        from the model, and this runs on the main render thread where a
+        raise would kill the TUI (session.update_todos already
+        normalizes at the boundary; this covers direct assignment).
+        """
         if not self._controller.todos:
             return None
         t = Table.grid(padding=(0, 1))
-        for todo in self._controller.todos[-8:]:
-            status = todo.get("status", "")
-            mark = {"completed": "✅", "in_progress": "⏳", "pending": "⬜"}.get(status, "•")
-            t.add_row(mark, todo.get("content", ""))
+        for raw in self._controller.todos[-8:]:
+            if isinstance(raw, dict):
+                status = raw.get("status", "")
+                if not isinstance(status, str):
+                    status = ""  # dict.get needs a hashable key
+                mark = {"completed": "✅", "in_progress": "⏳", "pending": "⬜"}.get(status, "•")
+                content = str(raw.get("content", ""))
+            else:
+                mark, content = "•", repr(raw)
+            t.add_row(mark, content)
         return Group(Text("Todos", style="bold"), t)
 
     def _history_rows(self) -> list[Any]:

tests/test_session.py

@@ -770,6 +770,40 @@ def test_clear_todos_drops_and_notifies(self):
         self.assertEqual(session.todos, [])
         self.assertEqual(notified, ["todos"])
 
+    def test_update_todos_normalizes_non_dict_items(self):
+        """Non-dict items (model schema violation) become deterministic
+        string rows instead of crashing the render thread's todo.get."""
+        session = RecordingSession()
+        session.update_todos(["junk", 42, ["nested"], {"content": "real", "status": "pending"}])
+        self.assertEqual(
+            session.todos,
+            [
+                {"content": "'junk'", "status": ""},
+                {"content": "42", "status": ""},
+                {"content": "['nested']", "status": ""},
+                {"content": "real", "status": "pending"},
+            ],
+        )
+
+    def test_update_todos_normalizes_bad_content_and_status(self):
+        """Dicts with non-str content or non-str status are coerced in
+        place; str fields pass through untouched."""
+        session = RecordingSession()
+        session.update_todos([{"content": 7, "status": "x"}, {"content": "ok", "status": 3}])
+        self.assertEqual(
+            session.todos,
+            [{"content": "7", "status": "x"}, {"content": "ok", "status": "3"}],
+        )
+
+    def test_update_todos_rejects_non_sequence(self):
+        """A model sending todos as a bare string/list-like junk stores
+        an empty list instead of blowing up the render loop."""
+        session = RecordingSession()
+        session.update_todos("abc")  # type: ignore[arg-type]
+        self.assertEqual(session.todos, [])
+        session.update_todos(None)  # type: ignore[arg-type]
+        self.assertEqual(session.todos, [])
+
 
 class TestPlanExit(unittest.TestCase):
     """PlanExit: approved switches to build and queues the approved

tests/tui/test_tui_questions.py

@@ -271,6 +271,66 @@ def test_option_label_coercion(self):
         self.assertEqual(_option_label(42), "42")
         self.assertEqual(_option_label(None), "None")
 
+    # ------------------------------------------------------------------
+    # containment: a crash while rendering a model-controlled question
+    # must resolve the pending question instead of killing the TUI
+    # (the worker blocks on q.event in _ask_sync and only tool execution
+    # is guarded — this render runs on the main thread)
+    # ------------------------------------------------------------------
+    def test_ask_question_crash_resolves_question_and_reports_error(self):
+        """A render crash inside _ask_question_blocking answers the
+        question with an error string, clears tui.question, sets the
+        worker's event, and surfaces the message on the status bar."""
+        tui, _ = make_tui()
+        q = UiQuestion("Pick", options=["a", "b"])
+        tui.question = q
+        with mock.patch.object(tui, "_render_and_ask", side_effect=TypeError("boom")):
+            tui._ask_question_blocking()
+        self.assertIsNone(tui.question)
+        self.assertTrue(q.event.is_set())
+        self.assertEqual(q.answer, "Error: question render failed — boom")
+        self.assertIn("error", tui.status.lower())
+        self.assertIn("question render failed", tui.status)
+
+    def test_ask_question_eof_propagates_out_of_containment(self):
+        """EOFError/KeyboardInterrupt pass through the containment: they
+        are interactive-cancel signals, not render bugs, and the
+        pending question stays for a later retry (run loop re-prompts)."""
+        tui, _ = make_tui()
+        q = UiQuestion("Pick", options=["a", "b"])
+        tui.question = q
+        with (
+            mock.patch.object(tui, "_render_and_ask", side_effect=EOFError),
+            self.assertRaises(EOFError),
+        ):
+            tui._ask_question_blocking()
+        self.assertIs(tui.question, q)
+        self.assertIsNone(q.answer)
+        self.assertFalse(q.event.is_set())
+
+    def test_ask_question_non_string_prompt_renders(self):
+        """A model sending a non-string 'question' field must not crash
+        the render thread with rich's TypeError."""
+        tui, buf = make_tui()
+        q = UiQuestion(123, options=["a", "b"])
+        tui.question = q
+        with mock.patch.object(tui.prompt_session, "prompt", return_value="1"):
+            tui._ask_question_blocking()
+        self.assertEqual(q.answer, "a")
+        self.assertIn("123", buf.getvalue())
+
+    def test_ask_question_still_answers_on_success_path(self):
+        """Success path unchanged after the containment refactor."""
+        tui, buf = make_tui()
+        q = UiQuestion("Proceed?", options=["y", "n"])
+        tui.question = q
+        with mock.patch.object(tui.prompt_session, "prompt", return_value="2"):
+            tui._ask_question_blocking()
+        self.assertEqual(q.answer, "n")
+        self.assertIsNone(tui.question)
+        self.assertTrue(q.event.is_set())
+        self.assertIn("1) y", buf.getvalue())
+
 
 if __name__ == "__main__":
     unittest.main()

tests/tui/test_tui_render.py

@@ -892,6 +892,21 @@ def test_todos_panel_hidden_when_empty(self):
         out = buf.getvalue()
         self.assertNotIn("Todos", out)
 
+    def test_todos_panel_survives_malformed_items(self):
+        """Regression: todos are model-controlled; a non-dict item or a
+        non-str content/status (which bypasses the session normalizer
+        via direct assignment) must not crash the main render thread."""
+        tui, buf = make_tui()
+        tui.session.todos = ["junk", 42, {"content": "real", "status": "pending"}]  # type: ignore[list-item]
+        tui.console.print(tui._render_frame())
+        out = buf.getvalue()
+        self.assertIn("Todos", out)
+        self.assertIn("real", out)
+        self.assertIn("'junk'", out)
+        tui.session.todos = [{"content": "a", "status": ["weird", "list"]}]  # type: ignore[dict-item]
+        tui.console.print(tui._render_frame())
+        self.assertIn("a", buf.getvalue())
+
     # ------------------------------------------------------------------
     # status bar markers / compacted summary rendering
     # ------------------------------------------------------------------