refactor(app_controller): migrate 2 signal handler sites to Result (Phase 6 Group 6.1)

Replaces the silent-swallow logging.debug bodies in _on_sigint and _install_sigint_exit_handler with proper Result[T] propagation: - _shutdown_io_pool_result() -> Result[None]: wraps io_pool.shutdown with OSError/RuntimeError/ValueError -> ErrorInfo(original=e) - _install_signal_handler_result(handler) -> Result[None]: wraps signal.signal() with ValueError/OSError -> ErrorInfo(original=e) - _install_sigint_exit_handler stores result.errors[0] on self._signal_handler_error: Optional[ErrorInfo] for sub-track 4 GUI The os._exit(0) inside the signal handler IS the drain (Pattern 3: intentional termination per error_handling.md:419). The stderr write before os._exit is part of the termination pattern (Heuristic D match). TIER-2 READ conductor/code_styleguides/error_handling.md before Phase 6. Audit: INTERNAL_SILENT_SWALLOW for src/app_controller.py: 30 -> 28.
2026-06-19 15:49:04 -04:00
parent eec44a09ed
commit 108e77e11d
2 changed files with 147 additions and 12 deletions
@@ -761,21 +761,23 @@ def _install_sigint_exit_handler(controller: 'AppController') -> None:
  [SDM: src/app_controller.py:_install_sigint_exit_handler]
  Best-effort: ``signal.signal`` may fail with ``ValueError`` on
  non-main threads (e.g. some conftest warmup paths). The failure
-  is swallowed because production (main thread) is the only case
-  that matters; the conftest's own atexit fix at commit 8957c9a5
-  covers the test fixture's normal-exit path.
+  is drained via `_install_signal_handler_result` -> ErrorInfo
+  stored on `controller._signal_handler_error`; the os._exit(0)
+  in the inner closure IS the drain (Pattern 3: intentional
+  termination per `error_handling.md` §"The 5 drain point patterns").
  [C: src/app_controller.py:AppController.__init__]
 """
 def _on_sigint(signum: int, frame: Any) -> None:
-  try:
-   controller._io_pool.shutdown(wait=False)
-  except (OSError, RuntimeError, ValueError) as e:
-   logging.getLogger(__name__).debug("io_pool shutdown on sigint: %s", e, extra={"source": "app_controller._on_sigint"})
+  result = controller._shutdown_io_pool_result()
+  if not result.ok:
+   sys.stderr.write(f"FATAL: {result.errors[0].ui_message()}\n")
+   sys.stderr.flush()
  os._exit(0)
- try:
-  signal.signal(signal.SIGINT, _on_sigint)
- except (ValueError, OSError) as e:
-   logging.getLogger(__name__).debug("signal handler install failed: %s", e, extra={"source": "app_controller._install_sigint_exit_handler"})
+ install_result = controller._install_signal_handler_result(_on_sigint)
+ if not install_result.ok:
+  controller._signal_handler_error = install_result.errors[0]
+  sys.stderr.write(f"signal handler install failed: {install_result.errors[0].ui_message()}\n")
+  sys.stderr.flush()


 class AppController:
@@ -823,6 +825,12 @@ class AppController:
  # degraded state via /api/gui_health and fail fast.
  self._gui_degraded_reason:         Optional[str] = None
  self._last_imgui_assert:           Optional[str] = None
+  # --- Signal handler state (Phase 6: Group 6.1) ---
+  # Set to the first ErrorInfo when signal.signal() fails to install
+  # the SIGINT exit handler (e.g. non-main thread). The os._exit(0)
+  # inside the signal handler IS the drain (Pattern 3: intentional
+  # termination). Stored for sub-track 4 GUI to display in degraded mode.
+  self._signal_handler_error:          Optional[ErrorInfo] = None
  # --- Shared background pool + proactive warmup (startup_speedup_20260606) ---
  self._io_pool = make_io_pool()
  _install_sigint_exit_handler(self)
@@ -1286,6 +1294,38 @@ class AppController:
  """Timestamp of the first GUI frame; None until the App has rendered once. [SDM: src/app_controller.py:first_frame_ts]"""
  return self._first_frame_ts

+ def _shutdown_io_pool_result(self) -> "Result[None]":
+  """Drain the IO pool for SIGINT. Returns Result[None] (Phase 6 Group 6.1).
+  On failure: OSError/RuntimeError/ValueError -> ErrorInfo(original=e).
+  Caller (the signal handler closure) writes the error to stderr and then
+  calls os._exit(0) (Pattern 3 drain: intentional termination)."""
+  try:
+   self._io_pool.shutdown(wait=False)
+   return OK
+  except (OSError, RuntimeError, ValueError) as e:
+   return Result(data=None, errors=[ErrorInfo(
+    kind=ErrorKind.INTERNAL,
+    message=str(e),
+    source="app_controller._shutdown_io_pool_result",
+    original=e,
+   )])
+
+ def _install_signal_handler_result(self, handler: "Callable[[int, Any], None]") -> "Result[None]":
+  """Install a SIGINT handler. Returns Result[None] (Phase 6 Group 6.1).
+  On failure: ValueError/OSError -> ErrorInfo(original=e). Caller
+  (`_install_sigint_exit_handler`) stores the error on
+  `self._signal_handler_error` for downstream consumers."""
+  try:
+   signal.signal(signal.SIGINT, handler)
+   return OK
+  except (ValueError, OSError) as e:
+   return Result(data=None, errors=[ErrorInfo(
+    kind=ErrorKind.INTERNAL,
+    message=str(e),
+    source="app_controller._install_signal_handler_result",
+    original=e,
+   )])
+
 def mark_first_frame_rendered(self, ts: "Optional[float]" = None) -> None:
  """Called by the App on the first frame render. Stamps first_frame_ts and logs the timeline to stderr. [SDM: src/app_controller.py:mark_first_frame_rendered] [C: src/gui_2.py:render_main_interface]"""
  if self._first_frame_ts is not None: return
@@ -20,7 +20,7 @@ The tests are pattern templates:
 """
 import pytest
 from unittest.mock import MagicMock, patch
-from src.app_controller import AppController
+from src.app_controller import AppController, _install_sigint_exit_handler
 from src.result_types import Result, ErrorInfo, ErrorKind


@@ -111,3 +111,98 @@ def test_offload_entry_payload_preserves_unchanged_payload():
  entry = {"kind": "request", "payload": {"message": "hi"}, "ts": "12:00:00"}
  out = ctrl._offload_entry_payload(entry)
 assert out == entry
+
+
+# --- Phase 6: Group 6.1 (signal handlers; Pattern 3 drain via os._exit) ---
+
+def test_shutdown_io_pool_result_returns_ok_when_pool_shuts_down_cleanly():
+ """
+ Pattern 3 drain: _shutdown_io_pool_result returns Result[None] with no errors
+ when the IO pool shuts down without raising.
+ """
+ from src.app_controller import AppController
+ ctrl = AppController()
+ ctrl._io_pool = MagicMock()
+ ctrl._io_pool.shutdown = MagicMock()
+ result = ctrl._shutdown_io_pool_result()
+ assert isinstance(result, Result)
+ assert result.ok is True
+ assert result.errors == []
+
+
+def test_shutdown_io_pool_result_returns_error_when_pool_raises():
+ """
+ Pattern 3 drain: _shutdown_io_pool_result converts OSError/RuntimeError/ValueError
+ to ErrorInfo(original=e) in Result.errors.
+ """
+ from src.app_controller import AppController
+ ctrl = AppController()
+ ctrl._io_pool = MagicMock()
+ ctrl._io_pool.shutdown = MagicMock(side_effect=RuntimeError("pool broken"))
+ result = ctrl._shutdown_io_pool_result()
+ assert isinstance(result, Result)
+ assert result.ok is False
+ assert len(result.errors) == 1
+ assert isinstance(result.errors[0], ErrorInfo)
+ assert "pool broken" in result.errors[0].message
+ assert result.errors[0].original is not None
+
+
+def test_install_signal_handler_result_returns_ok_when_signal_installs():
+ """
+ Pattern 3 drain: _install_signal_handler_result returns Result[None] on success
+ (no errors).
+ """
+ from src.app_controller import AppController
+ ctrl = AppController()
+ handler = lambda signum, frame: None
+ with patch("src.app_controller.signal.signal") as mock_signal:
+  result = ctrl._install_signal_handler_result(handler)
+ assert isinstance(result, Result)
+ assert result.ok is True
+ assert result.errors == []
+ assert mock_signal.called
+
+
+def test_install_signal_handler_result_returns_error_when_signal_raises():
+ """
+ Pattern 3 drain: _install_signal_handler_result converts ValueError/OSError
+ to ErrorInfo(original=e).
+ """
+ from src.app_controller import AppController
+ ctrl = AppController()
+ handler = lambda signum, frame: None
+ with patch("src.app_controller.signal.signal", side_effect=ValueError("not main thread")):
+  result = ctrl._install_signal_handler_result(handler)
+ assert isinstance(result, Result)
+ assert result.ok is False
+ assert len(result.errors) == 1
+ assert isinstance(result.errors[0], ErrorInfo)
+ assert "not main thread" in result.errors[0].message
+
+
+def test_install_sigint_exit_handler_stores_error_when_signal_install_fails():
+ """
+ Drains the Result to instance state: when the helper returns errors,
+ _install_sigint_exit_handler stores the first error on
+ ctrl._signal_handler_error for downstream consumers (e.g., sub-track 4 GUI).
+ """
+ from src.app_controller import AppController
+ ctrl = AppController()
+ with patch("src.app_controller.signal.signal", side_effect=ValueError("not main thread")):
+  _install_sigint_exit_handler(ctrl)
+ assert ctrl._signal_handler_error is not None
+ assert isinstance(ctrl._signal_handler_error, ErrorInfo)
+ assert "not main thread" in ctrl._signal_handler_error.message
+ assert ctrl._signal_handler_error.kind == ErrorKind.INTERNAL
+
+
+def test_install_sigint_exit_handler_no_error_when_signal_install_succeeds():
+ """
+ On success, _signal_handler_error stays as None.
+ """
+ from src.app_controller import AppController
+ ctrl = AppController()
+ with patch("src.app_controller.signal.signal"):
+  _install_sigint_exit_handler(ctrl)
+ assert ctrl._signal_handler_error is None