diff --git a/src/app_controller.py b/src/app_controller.py index 7d1cee32..dc1e1f82 100644 --- a/src/app_controller.py +++ b/src/app_controller.py @@ -837,6 +837,14 @@ class AppController: # sub-track 4 lands GUI-side error display); the instance list is # the durable data plane for sub-track 4 to drain. self._startup_timeline_errors: List[Tuple[str, ErrorInfo]] = [] # (op_name, error) + # --- GUI state-setter error state (Phase 6: Group 6.3) --- + # Each carries the first ErrorInfo when the corresponding setter/operation + # fails. The legacy wrapper stores the error here for sub-track 4 GUI to + # display; stderr write IS the visible-but-incomplete drain (full drain = + # GUI modal in sub-track 4). + self._inject_preview_error: Optional[ErrorInfo] = None + self._mcp_config_parse_error: Optional[ErrorInfo] = None + self._save_project_error: Optional[ErrorInfo] = None # --- Shared background pool + proactive warmup (startup_speedup_20260606) --- self._io_pool = make_io_pool() _install_sigint_exit_handler(self) @@ -1518,15 +1526,27 @@ class AppController: Updates the preview content based on the selected file and injection mode. [C: src/gui_2.py:App._render_text_viewer_window, tests/test_skeleton_injection.py:test_update_inject_preview_full, tests/test_skeleton_injection.py:test_update_inject_preview_skeleton, tests/test_skeleton_injection.py:test_update_inject_preview_truncation] """ + result = self._update_inject_preview_result() + if result.ok: + self._inject_preview = result.data + self._inject_preview_error = None + else: + err = result.errors[0] + self._inject_preview_error = err + self._inject_preview = f"Error reading file: {err.message}" + + def _update_inject_preview_result(self) -> "Result[str]": + """Read the file for the inject preview. Returns Result[str] (Phase 6 Group 6.3). + On failure: OSError/IOError/UnicodeDecodeError -> ErrorInfo(original=e). + Caller (`_update_inject_preview`) stores the error on + `self._inject_preview_error` for sub-track 4 GUI.""" if not self._inject_file_path: - self._inject_preview = "" - return + return Result(data="") target_path = self._inject_file_path if not os.path.isabs(target_path): target_path = os.path.join(self.active_project_root, target_path) if not os.path.exists(target_path): - self._inject_preview = "" - return + return Result(data="") try: with open(target_path, "r", encoding="utf-8") as f: content = f.read() @@ -1538,10 +1558,14 @@ class AppController: lines = preview.splitlines() if len(lines) > 500: preview = "\n".join(lines[:500]) + "\n... (truncated)" - self._inject_preview = preview + return Result(data=preview) except (OSError, IOError, UnicodeDecodeError) as e: - logging.getLogger(__name__).debug("inject preview file read failed: %s", e, extra={"source": "app_controller._update_inject_preview"}) - self._inject_preview = f"Error reading file: {e}" + return Result(data="", errors=[ErrorInfo( + kind=ErrorKind.INTERNAL, + message=str(e), + source="app_controller._update_inject_preview_result", + original=e, + )]) @property def ai_status(self) -> str: @@ -1684,11 +1708,30 @@ class AppController: return json.dumps(self.mcp_config.to_dict()) if self.mcp_config else "{}" @mcp_config_json.setter def mcp_config_json(self, value: str) -> None: + result = self._set_mcp_config_json_result(value) + if not result.ok: + err = result.errors[0] + self._mcp_config_parse_error = err + sys.stderr.write(f"mcp_config parse failed: {err.ui_message()}\n") + sys.stderr.flush() + + def _set_mcp_config_json_result(self, value: str) -> "Result[None]": + """Parse and assign mcp_config from a JSON string. Returns Result[None] (Phase 6 Group 6.3). + On failure: json.JSONDecodeError/ValueError/TypeError/KeyError/AttributeError -> ErrorInfo(original=e). + Property setters cannot return values; this sibling helper carries the + structured result. Caller (`mcp_config_json` setter) stores the error + on `self._mcp_config_parse_error` for sub-track 4 GUI.""" try: data = json.loads(value) self.mcp_config = models.MCPConfiguration.from_dict(data) + return OK except (json.JSONDecodeError, ValueError, TypeError, KeyError, AttributeError) as e: - logging.getLogger(__name__).debug("mcp config parse failed: %s", e, extra={"source": "app_controller.mcp_config_json"}) + return Result(data=None, errors=[ErrorInfo( + kind=ErrorKind.INVALID_INPUT, + message=str(e), + source="app_controller._set_mcp_config_json_result", + original=e, + )]) @property def ui_file_paths(self) -> list[str]: @@ -3125,13 +3168,32 @@ class AppController: """ [C: src/gui_2.py:App.delete_context_preset, src/gui_2.py:App.save_context_preset] """ - if self.active_project_path: - try: - cleaned = project_manager.clean_nones(self.project) - project_manager.save_project(cleaned, self.active_project_path) - except (OSError, IOError, ValueError, TypeError, KeyError, AttributeError) as e: - logging.getLogger(__name__).debug("save_project failed: %s", e, extra={"source": "app_controller._save_active_project"}) - self.ai_status = f"save error: {e}" + result = self._save_active_project_result() + if not result.ok: + err = result.errors[0] + self._save_project_error = err + self.ai_status = f"save error: {err.message}" + sys.stderr.write(f"save_project failed: {err.ui_message()}\n") + sys.stderr.flush() + + def _save_active_project_result(self) -> "Result[None]": + """Save the active project to its current path. Returns Result[None] (Phase 6 Group 6.3). + On failure: OSError/IOError/ValueError/TypeError/KeyError/AttributeError -> ErrorInfo(original=e). + Caller (`_save_active_project`) stores the error on + `self._save_project_error` for sub-track 4 GUI and updates self.ai_status.""" + if not self.active_project_path: + return OK + try: + cleaned = project_manager.clean_nones(self.project) + project_manager.save_project(cleaned, self.active_project_path) + return OK + except (OSError, IOError, ValueError, TypeError, KeyError, AttributeError) as e: + return Result(data=None, errors=[ErrorInfo( + kind=ErrorKind.INTERNAL, + message=str(e), + source="app_controller._save_active_project_result", + original=e, + )]) #endregion: Project diff --git a/tests/test_app_controller_result.py b/tests/test_app_controller_result.py index 696f9187..efbfb84f 100644 --- a/tests/test_app_controller_result.py +++ b/tests/test_app_controller_result.py @@ -278,3 +278,130 @@ def test_warmup_complete_timeline_records_error_on_stderr_failure(): 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