From 24191c827d1af9f965e4d1c88772d006dfce05c2 Mon Sep 17 00:00:00 2001 From: Ed_ Date: Sat, 20 Jun 2026 00:52:01 -0400 Subject: [PATCH] TIER-2 READ conductor/code_styleguides/error_handling.md end-to-end before Phase 10: refactor(gui_2): migrate L1466 _close_vscode_diff terminate to Result[T] (Phase 10 site 7) Extracted _close_vscode_diff_terminate_result(app) -> Result[None] helper above the App._close_vscode_diff method. ANTI-SLIMING: full Result[T] propagation (NO except+pass). The helper returns Result(data=None) on success or Result(data=None, errors=[ErrorInfo]) on exception (logging NOT a drain per the user's principle 2026-06-17). The legacy _close_vscode_diff method preserves its signature, calls the helper, drains errors to self._last_request_errors, and proceeds to set self._vscode_diff_process = None (preserving the original post-error behavior of clearing the handle). Tests: 2 new tests verify both paths (success and OSError). Audit: L1466 reclassified from INTERNAL_SILENT_SWALLOW (7 sites remaining, was 8). New helper L1466 is INTERNAL_COMPLIANT. --- src/gui_2.py | 31 ++++++++++++++++++++++++---- tests/test_gui_2_result.py | 42 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 4 deletions(-) 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 + +