diff --git a/src/gui_2.py b/src/gui_2.py index ce4a62fb..378a9143 100644 --- a/src/gui_2.py +++ b/src/gui_2.py @@ -1465,12 +1465,35 @@ def _gui_func_entry_log_result(app: "App") -> Result[None]: def _close_vscode_diff(self) -> None: if hasattr(self, '_vscode_diff_process') and self._vscode_diff_process: - try: - self._vscode_diff_process.terminate() - except Exception: - pass + term_result = _close_vscode_diff_terminate_result(self) + if not term_result.ok: + if not hasattr(self, '_last_request_errors'): self._last_request_errors = [] + self._last_request_errors.append(("_close_vscode_diff.terminate", term_result.errors[0])) self._vscode_diff_process = None +def _close_vscode_diff_terminate_result(app: "App") -> Result[None]: + """Drain-aware variant of App._close_vscode_diff terminate try block (L1466 INTERNAL_SILENT_SWALLOW). + + Extracts the self._vscode_diff_process.terminate() try/except from + App._close_vscode_diff into a Result-returning helper. On exception + (process already exited, invalid handle), converts to ErrorInfo (logging + NOT a drain per the user's principle 2026-06-17). The legacy wrapper + drains to self._last_request_errors and proceeds to set + self._vscode_diff_process = None (preserving the original behavior). + + [C: src/gui_2.py:App._close_vscode_diff (L1466 legacy wrapper)] + """ + try: + app._vscode_diff_process.terminate() + return Result(data=None) + except Exception as e: + return Result(data=None, errors=[ErrorInfo( + kind=ErrorKind.INTERNAL, + message=f"_vscode_diff_process.terminate failed: {e}", + source="gui_2._close_vscode_diff_terminate_result", + original=e, + )]) + def _apply_pending_patch(self) -> None: if not self._pending_patch_text: self._patch_error_message = "No patch to apply" diff --git a/tests/test_gui_2_result.py b/tests/test_gui_2_result.py index d4521bfa..247d6114 100644 --- a/tests/test_gui_2_result.py +++ b/tests/test_gui_2_result.py @@ -1990,4 +1990,46 @@ def test_phase_10_l1152_gui_func_entry_log_result_failure(): assert "broken pipe" in err.message +def test_phase_10_l1466_close_vscode_diff_terminate_result_success(): + """ + L1466 _close_vscode_diff_terminate_result returns Result(data=None) on success. + + The helper extracts the self._vscode_diff_process.terminate() try/except + from App._close_vscode_diff into a Result-returning helper. On success, + returns Result(data=None). The legacy wrapper sets + self._vscode_diff_process = None (the original behavior preserved). + """ + from unittest.mock import MagicMock + import src.gui_2 as gui2_mod + app = MagicMock() + app._vscode_diff_process = MagicMock() + result = gui2_mod._close_vscode_diff_terminate_result(app) + assert result.ok, f"Expected ok=True on success, got errors: {result.errors}" + assert result.data is None + app._vscode_diff_process.terminate.assert_called_once() + + +def test_phase_10_l1466_close_vscode_diff_terminate_result_failure(): + """ + L1466 _close_vscode_diff_terminate_result returns Result(data=None, errors=[ErrorInfo]) on failure. + + When the process.terminate() call raises (e.g., process already exited, + invalid handle), the helper converts to ErrorInfo. The legacy wrapper + drains to self._last_request_errors and proceeds to set + self._vscode_diff_process = None (preserving the original behavior). + """ + from unittest.mock import MagicMock + import src.gui_2 as gui2_mod + app = MagicMock() + app._vscode_diff_process = MagicMock() + app._vscode_diff_process.terminate.side_effect = OSError("process already exited") + result = gui2_mod._close_vscode_diff_terminate_result(app) + assert not result.ok, f"Expected ok=False on failure, got data: {result.data}" + assert result.data is None + assert result.errors, "Expected at least one error on failure" + err = result.errors[0] + assert err.source == "gui_2._close_vscode_diff_terminate_result" + assert "process already exited" in err.message + +