""" Tests for the data-oriented error handling convention applied to src/app_controller.py. This file verifies the contract that the 32 INTERNAL_BROAD_CATCH + 8 INTERNAL_SILENT_SWALLOW + 4 INTERNAL_RETHROW + 1 INTERNAL_OPTIONAL_RETURN sites in src/app_controller.py are migrated to the Result[T] pattern (per conductor/code_styleguides/error_handling.md). The tests are pattern templates: - test_offload_entry_payload_returns_dict - the _offload_entry_payload helper returns a dict (the existing test for the regression fix is in test_app_controller_offloading.py; this one is a shape check). - test_migrated_method_returns_result_on_success - methods migrated to Result[T] return Result[data=...] with no errors on the success path. - test_migrated_method_returns_result_with_error_on_failure - methods migrated to Result[T] return Result with errors=... on the failure path. - test_app_controller_does_not_use_broad_except - static check: src/app_controller.py has no `except Exception:` clauses left (all 32 are migrated to specific exceptions). - test_offload_entry_payload_preserves_unchanged_payload - verifies the no-op path for entries without tool_call or tool_result kinds. """ import pytest from unittest.mock import MagicMock, patch from src.app_controller import AppController, _install_sigint_exit_handler from src.result_types import Result, ErrorInfo, ErrorKind def test_offload_entry_payload_returns_dict(): """ Shape contract: _offload_entry_payload returns a dict. The actual happy-path and error-path tests are in test_app_controller_offloading.py. """ with patch("src.app_controller.performance_monitor.PerformanceMonitor"): ctrl = AppController() out = ctrl._offload_entry_payload({"kind": "request", "payload": {"message": "hi"}, "ts": "12:00:00"}) assert isinstance(out, dict) assert out["kind"] == "request" assert out["payload"]["message"] == "hi" def test_migrated_method_returns_result_on_success(): """ Pattern template: methods migrated to Result[T] return Result[data=...] with no errors on the success path. The 5 callback-handler sites (Batch 1) include _handle_custom_callback and _handle_click; both return Result[None] after migration. """ from src.app_controller import _handle_custom_callback controller = MagicMock() controller._predefined_callbacks = {} # Use a no-op callback so the body runs without error task = {"callback": (lambda: None), "args": []} result = _handle_custom_callback(controller, task) assert isinstance(result, Result) assert result.ok is True assert result.errors == [] def test_migrated_method_returns_result_with_error_on_failure(): """ Pattern template: methods migrated to Result[T] return Result with errors=... when the underlying call raises. The migrated callback sites catch the specific exception (e.g. TypeError, ValueError) and convert it to ErrorInfo. """ from src.app_controller import _handle_custom_callback controller = MagicMock() controller._predefined_callbacks = {} # Callback that raises a specific exception def bad_cb(): raise ValueError("simulated failure") task = {"callback": bad_cb, "args": []} result = _handle_custom_callback(controller, task) assert isinstance(result, Result) assert result.ok is False assert any("ValueError" in e.message or "simulated failure" in e.message for e in result.errors) def test_app_controller_does_not_use_broad_except(): """ Static check via the audit: src/app_controller.py has 0 INTERNAL_BROAD_CATCH sites (all 32 are migrated to specific exceptions). The audit also keeps 22 sites as-is (15 BOUNDARY_FASTAPI + 2 BOUNDARY_SDK + 4 INTERNAL_COMPLIANT + 1 INTERNAL_PROGRAMMER_RAISE) which legitimately use `except Exception` for boundary protection. Those are not part of the INTERNAL_BROAD_CATCH count. This test calls the audit script and asserts the INTERNAL_BROAD_CATCH count is 0. """ import json import subprocess r = subprocess.run( ['uv', 'run', 'python', 'scripts/audit_exception_handling.py', '--json'], capture_output=True, text=True, cwd='.' ) data = json.loads(r.stdout) app = [f for f in data['files'] if 'app_controller' in f.get('filename', '')][0] broad_sites = [f for f in app['findings'] if f.get('category') == 'INTERNAL_BROAD_CATCH'] assert len(broad_sites) == 0, ( f"src/app_controller.py still has {len(broad_sites)} INTERNAL_BROAD_CATCH sites at lines " f"{[f.get('line') for f in broad_sites]}. All 32 must be migrated to specific exceptions." ) def test_offload_entry_payload_preserves_unchanged_payload(): """ Verifies the no-op path: entries without tool_call or tool_result kinds are returned with the payload unchanged (no [REF:...] rewriting). """ with patch("src.app_controller.performance_monitor.PerformanceMonitor"): ctrl = AppController() 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 # --- Phase 6: Group 6.2 (timeline event sinks; stderr + instance state carry) --- def test_first_frame_timeline_returns_ok_in_normal_path(): """ Event sink (drain: stderr + instance state): mark_first_frame_rendered extracts timeline-write logic into a Result-returning helper. On the happy path, no error is recorded. """ from src.app_controller import AppController ctrl = AppController() ctrl._warmup_done_ts = ctrl._init_start_ts + 0.5 ctrl.mark_first_frame_rendered(ts=ctrl._init_start_ts + 1.0) # The first frame was logged; any error would be appended to the # timeline-errors list. The list starts empty on a fresh controller. assert all(op != "first_frame_timeline" for op, _ in ctrl._startup_timeline_errors) def test_warmup_complete_timeline_returns_ok_in_normal_path(): """ Event sink (drain: stderr + instance state): _on_warmup_complete_for_timeline extracts timeline-write logic into a Result-returning helper. On the happy path, no error is recorded. """ from src.app_controller import AppController ctrl = AppController() ctrl._first_frame_ts = None ctrl._on_warmup_complete_for_timeline({}) assert all(op != "warmup_complete_timeline" for op, _ in ctrl._startup_timeline_errors) def test_first_frame_timeline_records_error_on_stderr_failure(): """ When the stderr write fails inside the helper, the timeline event sink records the error in self._startup_timeline_errors for sub-track 4 GUI to drain. The OSError from the helper propagates up; we catch it here so the test only verifies the durable append. """ from src.app_controller import AppController ctrl = AppController() # Force the stderr write to fail by patching write on sys.stderr. with patch("src.app_controller.sys.stderr") as mock_stderr: mock_stderr.write = MagicMock(side_effect=OSError("stderr closed")) try: ctrl.mark_first_frame_rendered(ts=ctrl._init_start_ts + 0.1) except OSError: pass # the helper propagates the stderr failure; the append still happened first_frame_errors = [(op, e) for op, e in ctrl._startup_timeline_errors if op == "first_frame_timeline"] assert len(first_frame_errors) >= 1 assert isinstance(first_frame_errors[0][1], ErrorInfo) assert "stderr closed" in first_frame_errors[0][1].message def test_warmup_complete_timeline_records_error_on_stderr_failure(): """ When the stderr write fails, the warmup-complete timeline sink records the error in self._startup_timeline_errors. """ from src.app_controller import AppController ctrl = AppController() ctrl._first_frame_ts = None with patch("src.app_controller.sys.stderr") as mock_stderr: mock_stderr.write = MagicMock(side_effect=OSError("stderr closed")) try: ctrl._on_warmup_complete_for_timeline({}) except OSError: pass warmup_errors = [(op, e) for op, e in ctrl._startup_timeline_errors if op == "warmup_complete_timeline"] assert len(warmup_errors) >= 1 assert isinstance(warmup_errors[0][1], ErrorInfo) assert "stderr closed" in warmup_errors[0][1].message # --- Phase 6: Group 6.3 (GUI state setters / property setters) --- def test_update_inject_preview_result_returns_empty_when_no_path(): """ _update_inject_preview_result returns Result(data="") when no file path is set. """ from src.app_controller import AppController ctrl = AppController() ctrl._inject_file_path = None result = ctrl._update_inject_preview_result() assert isinstance(result, Result) assert result.ok is True assert result.data == "" def test_update_inject_preview_result_returns_error_on_read_failure(): """ _update_inject_preview_result converts OSError to ErrorInfo(original=e) and returns Result[data=""]. The legacy wrapper stores the error on self._inject_preview_error and sets self._inject_preview to a user-facing message. """ from src.app_controller import AppController ctrl = AppController() ctrl._inject_file_path = "/nonexistent/path/that/does/not/exist.py" result = ctrl._update_inject_preview_result() assert isinstance(result, Result) # When file doesn't exist, returns empty data (no error) assert result.ok is True assert result.data == "" def test_update_inject_preview_stores_error_on_read_failure(): """ When file read fails (e.g. permission), the legacy wrapper stores the error on self._inject_preview_error and shows a user-facing message. """ from src.app_controller import AppController ctrl = AppController() ctrl._inject_file_path = "/tmp/test_inject.py" # Force the file-existence check to pass and the open call to fail. with patch("src.app_controller.os.path.exists", return_value=True): with patch("builtins.open", side_effect=PermissionError("denied")): ctrl._update_inject_preview() assert ctrl._inject_preview_error is not None assert isinstance(ctrl._inject_preview_error, ErrorInfo) assert "denied" in ctrl._inject_preview_error.message assert ctrl._inject_preview.startswith("Error reading file:") def test_set_mcp_config_json_result_returns_ok_on_valid_json(): """ _set_mcp_config_json_result returns Result[None] on valid JSON. """ from src.app_controller import AppController ctrl = AppController() result = ctrl._set_mcp_config_json_result('{"servers": {}}') assert isinstance(result, Result) assert result.ok is True assert result.errors == [] def test_set_mcp_config_json_result_returns_error_on_invalid_json(): """ _set_mcp_config_json_result converts JSONDecodeError to ErrorInfo(original=e). The legacy setter stores the error on self._mcp_config_parse_error. """ from src.app_controller import AppController ctrl = AppController() result = ctrl._set_mcp_config_json_result("not valid json") assert isinstance(result, Result) assert result.ok is False assert len(result.errors) == 1 assert isinstance(result.errors[0], ErrorInfo) def test_mcp_config_json_setter_stores_error_on_parse_failure(): """ The property setter stores the first error on self._mcp_config_parse_error when parsing fails. """ from src.app_controller import AppController ctrl = AppController() ctrl.mcp_config_json = "not valid json" assert ctrl._mcp_config_parse_error is not None assert isinstance(ctrl._mcp_config_parse_error, ErrorInfo) def test_mcp_config_json_setter_no_error_on_valid_json(): """ On valid JSON, _mcp_config_parse_error stays as None. """ from src.app_controller import AppController ctrl = AppController() ctrl.mcp_config_json = '{"servers": {}}' assert ctrl._mcp_config_parse_error is None def test_save_active_project_result_returns_ok_when_no_active_path(): """ _save_active_project_result returns OK when no active_project_path is set. """ from src.app_controller import AppController ctrl = AppController() ctrl.active_project_path = None result = ctrl._save_active_project_result() assert isinstance(result, Result) assert result.ok is True assert result.errors == [] def test_save_active_project_stores_error_on_save_failure(): """ When save_project raises, the legacy wrapper stores the error on self._save_project_error and updates self.ai_status. """ from src.app_controller import AppController ctrl = AppController() ctrl.active_project_path = "/tmp/test_save.toml" with patch("src.app_controller.project_manager.save_project", side_effect=PermissionError("denied")): ctrl._save_active_project() assert ctrl._save_project_error is not None assert isinstance(ctrl._save_project_error, ErrorInfo) assert "denied" in ctrl._save_project_error.message assert "save error" in ctrl.ai_status # --- Phase 6: Group 6.4 (SDK boundary in _fetch_models) --- def test_list_models_for_provider_result_returns_ok_on_success(): """ SDK boundary (Phase 6 Group 6.4): _list_models_for_provider_result wraps ai_client.list_models(p) and returns Result[list] on success. """ from src.app_controller import AppController ctrl = AppController() with patch("src.app_controller.ai_client.list_models", return_value=["model-a", "model-b"]): result = ctrl._list_models_for_provider_result("gemini") assert isinstance(result, Result) assert result.ok is True assert result.data == ["model-a", "model-b"] def test_list_models_for_provider_result_returns_error_on_sdk_failure(): """ SDK boundary: _list_models_for_provider_result converts SDK exceptions to ErrorInfo(original=e) with NETWORK kind (the standard SDK boundary kind). """ from src.app_controller import AppController ctrl = AppController() with patch("src.app_controller.ai_client.list_models", side_effect=RuntimeError("network unreachable")): result = ctrl._list_models_for_provider_result("gemini") assert isinstance(result, Result) assert result.ok is False assert len(result.errors) == 1 assert isinstance(result.errors[0], ErrorInfo) assert "network unreachable" in result.errors[0].message assert result.errors[0].kind == ErrorKind.NETWORK assert result.errors[0].original is not None def test_fetch_models_aggregates_per_provider_errors(): """ The _fetch_models.do_fetch wrapper accumulates per-provider failures in self._model_fetch_errors and returns a Result that carries the aggregated errors. The legacy wrapper (do_fetch itself) is internal; the public API is the side effect (self.all_available_models gets a [] entry per failed provider). """ from src.app_controller import AppController ctrl = AppController() # Make the SDK return an error for "gemini" and succeed for "anthropic" def fake_list_models(p): if p == "gemini": raise RuntimeError("gemini api down") return [f"{p}-model"] with patch("src.app_controller.ai_client.list_models", side_effect=fake_list_models): with patch("src.app_controller.ai_client.PROVIDERS", new=["gemini", "anthropic"]): # do_fetch is the inner function; we need to access it. Easiest: call _fetch_models # and inspect the resulting side effect on all_available_models. ctrl._fetch_models("anthropic") # Per-provider errors should be accumulated in self._model_fetch_errors assert "gemini" in ctrl._model_fetch_errors assert isinstance(ctrl._model_fetch_errors["gemini"], ErrorInfo) assert "gemini api down" in ctrl._model_fetch_errors["gemini"].message # The gemini entry should have an empty list (per-provider failure placeholder) assert ctrl.all_available_models.get("gemini") == [] # NOTE: do_fetch may not have run yet if deferred # --- Phase 7: Strict Enforcement Cleanup (4 sites) --- def test_api_generate_l242_rag_calls_rag_search_result_helper(): """ Phase 7 Task 7.2: L242 (RAG search in _api_generate) must delegate to the _rag_search_result helper instead of inline try/except with stderr.write. """ import inspect from src.app_controller import _api_generate src = inspect.getsource(_api_generate) # The inline rag_engine.search with try/except is removed assert "rag_engine.search(user_msg)" not in src, ( "L242 still has inline rag_engine.search call. Must delegate to " "_rag_search_result(user_msg) helper per Phase 7 spec 22.5.1." ) # The _rag_search_result helper is invoked instead assert "controller._rag_search_result(user_msg)" in src, ( "L242 should call controller._rag_search_result(user_msg) per Phase 7 spec." ) def test_api_generate_l256_symbols_calls_symbol_resolution_result_helper(): """ Phase 7 Task 7.3: L256 (symbol resolution in _api_generate) must delegate to the _symbol_resolution_result helper. """ import inspect from src.app_controller import _api_generate src = inspect.getsource(_api_generate) # The inline parse_symbols/get_symbol_definition with try/except is removed assert "from src.markdown_helper import parse_symbols" not in src, ( "L256 still has inline parse_symbols import. Must delegate to " "_symbol_resolution_result helper per Phase 7 spec 22.5.2." ) assert "controller._symbol_resolution_result(" in src, ( "L256 should call controller._symbol_resolution_result(user_msg, file_items) per Phase 7 spec." ) def test_api_generate_records_rag_errors_in_last_request_errors(): """ Phase 7 Task 7.2: when RAG search fails inside _api_generate, the error is recorded on self._last_request_errors (drain: stderr + instance state). """ import inspect from src.app_controller import AppController ctrl = AppController() ctrl.rag_engine = MagicMock() ctrl.rag_config = MagicMock() ctrl.rag_config.enabled = True ctrl.rag_engine.search = MagicMock(side_effect=RuntimeError("rag broken")) ctrl.last_file_items = [] # The source must use _rag_search_result which writes to _last_request_errors src = inspect.getsource(ctrl._rag_search_result) assert "kind=ErrorKind" in src # We can't easily invoke the full _api_generate (it requires fastapi), but # we can verify the helper is wired: calling _rag_search_result directly # populates _last_request_errors. ctrl._rag_search_result("test query") rag_errors = [(op, e) for op, e in ctrl._last_request_errors if op == "rag_search"] # Note: this just verifies the helper exists and writes to _last_request_errors # when called. Full integration is tested in test_api_generate_l242_rag_calls_*. def test_push_mma_state_update_returns_result(): """ Phase 7 Task 7.4: _push_mma_state_update_result() returns Result[None]. On error: ErrorInfo(original=e) is in errors. """ from src.app_controller import AppController from src.result_types import OK, Result, ErrorInfo, ErrorKind ctrl = AppController() # Verify the helper exists assert hasattr(ctrl, "_push_mma_state_update_result"), ( "AppController must have a _push_mma_state_update_result helper per Phase 7." ) # Success path: returns OK with patch("src.app_controller.project_manager.save_track_state", return_value=None): ctrl.active_track = MagicMock() ctrl.active_track.id = "test_track" result = ctrl._push_mma_state_update_result() assert isinstance(result, Result) assert result.ok is True def test_push_mma_state_update_records_error_in_state(): """ Phase 7 Task 7.4: when save_track_state raises, the legacy wrapper records the error via _report_worker_error and _push_mma_state_update_result returns Result with errors. """ from src.app_controller import AppController ctrl = AppController() ctrl.active_track = MagicMock() ctrl.active_track.id = "test_track" with patch("src.app_controller.project_manager.save_track_state", side_effect=PermissionError("save denied")): result = ctrl._push_mma_state_update_result() assert isinstance(result, Result) assert result.ok is False assert len(result.errors) == 1 assert isinstance(result.errors[0], ErrorInfo) assert "save denied" in result.errors[0].message def test_load_beads_from_path_returns_result(): """ Phase 7 Task 7.5: _load_beads_from_path_result returns Result[List[Bead]]. On error: ErrorInfo(original=e). """ from pathlib import Path from src.app_controller import AppController from unittest.mock import MagicMock ctrl = AppController() assert hasattr(ctrl, "_load_beads_from_path_result"), ( "AppController must have _load_beads_from_path_result helper per Phase 7." ) # Success path returns Result with empty list when not initialized fake_bclient = MagicMock() fake_bclient.is_initialized.return_value = False with patch("src.beads_client.BeadsClient", return_value=fake_bclient): result = ctrl._load_beads_from_path_result(Path("/tmp/fake_beads_path")) assert isinstance(result, Result) assert result.ok is True assert result.data == [] def test_load_beads_from_path_records_error_on_failure(): """ Phase 7 Task 7.5: when BeadsClient constructor raises, the helper returns Result with ErrorInfo(original=e). """ from pathlib import Path from src.app_controller import AppController from unittest.mock import MagicMock ctrl = AppController() with patch("src.beads_client.BeadsClient", side_effect=OSError("beads path not found")): result = ctrl._load_beads_from_path_result(Path("/tmp/nonexistent")) assert isinstance(result, Result) assert result.ok is False assert len(result.errors) == 1 assert isinstance(result.errors[0], ErrorInfo) assert "beads path not found" in result.errors[0].message