diff --git a/.codacy.yml b/.codacy.yml new file mode 100644 index 0000000..d0b387d --- /dev/null +++ b/.codacy.yml @@ -0,0 +1,7 @@ +--- +# Test code legitimately exercises security-sensitive patterns (subprocess +# execution, fake SSH credentials, cleartext URLs fed to the SSRF validator), +# so security linters raise false positives there. Exclude tests from analysis; +# production code under pybreeze/ is still fully scanned. +exclude_paths: + - 'test/**' diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index f6aab6b..6263faf 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -29,8 +29,8 @@ jobs: run: python -m pip install --upgrade pip setuptools wheel - name: Install dev dependencies run: python -m pip install -r dev_requirements.txt - - name: Install pytest - run: python -m pip install pytest + - name: Install test tooling + run: python -m pip install pytest hypothesis - name: Run unit tests (pytest) run: python -m pytest test/test_utils/ -v --tb=short - name: Run AutomationEditor With Debug Mode diff --git a/.github/workflows/stable.yml b/.github/workflows/stable.yml index c3bf82a..072f3f5 100644 --- a/.github/workflows/stable.yml +++ b/.github/workflows/stable.yml @@ -29,8 +29,8 @@ jobs: run: python -m pip install --upgrade pip setuptools wheel - name: Install dependencies run: python -m pip install -r requirements.txt - - name: Install pytest - run: python -m pip install pytest + - name: Install test tooling + run: python -m pip install pytest hypothesis - name: Run unit tests (pytest) run: python -m pytest test/test_utils/ -v --tb=short - name: Run AutomationEditor With Debug Mode diff --git a/.sonarcloud.properties b/.sonarcloud.properties new file mode 100644 index 0000000..411fdbc --- /dev/null +++ b/.sonarcloud.properties @@ -0,0 +1,3 @@ +sonar.sources=pybreeze +sonar.tests=test +sonar.python.version=3.10, 3.11, 3.12, 3.13, 3.14 diff --git a/pybreeze/extend/process_executor/api_testka/api_testka_process.py b/pybreeze/extend/process_executor/api_testka/api_testka_process.py index 3d2af9d..7545126 100644 --- a/pybreeze/extend/process_executor/api_testka/api_testka_process.py +++ b/pybreeze/extend/process_executor/api_testka/api_testka_process.py @@ -2,13 +2,14 @@ from typing import TYPE_CHECKING -from pybreeze.extend.process_executor.process_executor_utils import build_process +from pybreeze.extend.process_executor.process_executor_utils import ( + build_process, run_dir_files_with_package, +) if TYPE_CHECKING: from pybreeze.pybreeze_ui.editor_main.main_ui import PyBreezeMainWindow -from pybreeze.utils.file_process.get_dir_file_list import ask_and_get_dir_files_as_list -from pybreeze.utils.logging.logger import pybreeze_logger +_PACKAGE = "je_api_testka" def call_api_testka( @@ -16,7 +17,7 @@ def call_api_testka( exec_str: str | None = None, program_buffer: int = 1024000 ): - build_process(main_window, "je_api_testka", exec_str, False, program_buffer) + build_process(main_window, _PACKAGE, exec_str, False, program_buffer) def call_api_testka_with_send( @@ -24,40 +25,18 @@ def call_api_testka_with_send( exec_str: str | None = None, program_buffer: int = 1024000 ): - build_process(main_window, "je_api_testka", exec_str, True, program_buffer) + build_process(main_window, _PACKAGE, exec_str, True, program_buffer) def call_api_testka_multi_file( main_window: PyBreezeMainWindow, program_buffer: int = 1024000 ): - try: - need_to_execute_list = ask_and_get_dir_files_as_list(main_window) - if need_to_execute_list is not None and len(need_to_execute_list) > 0: - for execute_file in need_to_execute_list: - with open(execute_file, encoding="utf-8") as test_script_json: - call_api_testka( - main_window, - test_script_json.read(), - program_buffer - ) - except Exception as error: - pybreeze_logger.error(f"api_testka multi file error: {error}") + run_dir_files_with_package(main_window, _PACKAGE, False, program_buffer) def call_api_testka_multi_file_and_send( main_window: PyBreezeMainWindow, program_buffer: int = 1024000 ): - try: - need_to_execute_list = ask_and_get_dir_files_as_list(main_window) - if need_to_execute_list is not None and len(need_to_execute_list) > 0: - for execute_file in need_to_execute_list: - with open(execute_file, encoding="utf-8") as test_script_json: - call_api_testka_with_send( - main_window, - test_script_json.read(), - program_buffer - ) - except Exception as error: - pybreeze_logger.error(f"api_testka multi file and send error: {error}") + run_dir_files_with_package(main_window, _PACKAGE, True, program_buffer) diff --git a/pybreeze/extend/process_executor/auto_control/auto_control_process.py b/pybreeze/extend/process_executor/auto_control/auto_control_process.py index ecb9028..f87ee12 100644 --- a/pybreeze/extend/process_executor/auto_control/auto_control_process.py +++ b/pybreeze/extend/process_executor/auto_control/auto_control_process.py @@ -2,13 +2,14 @@ from typing import TYPE_CHECKING -from pybreeze.extend.process_executor.process_executor_utils import build_process +from pybreeze.extend.process_executor.process_executor_utils import ( + build_process, run_dir_files_with_package, +) if TYPE_CHECKING: from pybreeze.pybreeze_ui.editor_main.main_ui import PyBreezeMainWindow -from pybreeze.utils.file_process.get_dir_file_list import ask_and_get_dir_files_as_list -from pybreeze.utils.logging.logger import pybreeze_logger +_PACKAGE = "je_auto_control" def call_auto_control( @@ -16,7 +17,7 @@ def call_auto_control( exec_str: str | None = None, program_buffer: int = 1024000 ): - build_process(main_window, "je_auto_control", exec_str, False, program_buffer) + build_process(main_window, _PACKAGE, exec_str, False, program_buffer) def call_auto_control_with_send( @@ -24,40 +25,18 @@ def call_auto_control_with_send( exec_str: str | None = None, program_buffer: int = 1024000 ): - build_process(main_window, "je_auto_control", exec_str, True, program_buffer) + build_process(main_window, _PACKAGE, exec_str, True, program_buffer) def call_auto_control_multi_file( main_window: PyBreezeMainWindow, program_buffer: int = 1024000 ): - try: - need_to_execute_list = ask_and_get_dir_files_as_list(main_window) - if need_to_execute_list is not None and len(need_to_execute_list) > 0: - for execute_file in need_to_execute_list: - with open(execute_file, encoding="utf-8") as test_script_json: - call_auto_control( - main_window, - test_script_json.read(), - program_buffer - ) - except Exception as error: - pybreeze_logger.error(f"auto control multi file error: {error}") + run_dir_files_with_package(main_window, _PACKAGE, False, program_buffer) def call_auto_control_multi_file_and_send( main_window: PyBreezeMainWindow, program_buffer: int = 1024000 ): - try: - need_to_execute_list = ask_and_get_dir_files_as_list(main_window) - if need_to_execute_list is not None and len(need_to_execute_list) > 0: - for execute_file in need_to_execute_list: - with open(execute_file, encoding="utf-8") as test_script_json: - call_auto_control_with_send( - main_window, - test_script_json.read(), - program_buffer - ) - except Exception as error: - pybreeze_logger.error(f"auto control multi file and send error: {error}") + run_dir_files_with_package(main_window, _PACKAGE, True, program_buffer) diff --git a/pybreeze/extend/process_executor/file_automation/file_automation_process.py b/pybreeze/extend/process_executor/file_automation/file_automation_process.py index 92fe6db..707f7b2 100644 --- a/pybreeze/extend/process_executor/file_automation/file_automation_process.py +++ b/pybreeze/extend/process_executor/file_automation/file_automation_process.py @@ -2,13 +2,14 @@ from typing import TYPE_CHECKING -from pybreeze.extend.process_executor.process_executor_utils import build_process +from pybreeze.extend.process_executor.process_executor_utils import ( + build_process, run_dir_files_with_package, +) if TYPE_CHECKING: from pybreeze.pybreeze_ui.editor_main.main_ui import PyBreezeMainWindow -from pybreeze.utils.file_process.get_dir_file_list import ask_and_get_dir_files_as_list -from pybreeze.utils.logging.logger import pybreeze_logger +_PACKAGE = "automation_file" def call_file_automation_test( @@ -16,7 +17,7 @@ def call_file_automation_test( exec_str: str | None = None, program_buffer: int = 1024000 ): - build_process(main_window, "automation_file", exec_str, False, program_buffer) + build_process(main_window, _PACKAGE, exec_str, False, program_buffer) def call_file_automation_test_with_send( @@ -24,40 +25,18 @@ def call_file_automation_test_with_send( exec_str: str | None = None, program_buffer: int = 1024000 ): - build_process(main_window, "automation_file", exec_str, True, program_buffer) + build_process(main_window, _PACKAGE, exec_str, True, program_buffer) def call_file_automation_test_multi_file( main_window: PyBreezeMainWindow, program_buffer: int = 1024000 ): - try: - need_to_execute_list = ask_and_get_dir_files_as_list(main_window) - if need_to_execute_list is not None and len(need_to_execute_list) > 0: - for execute_file in need_to_execute_list: - with open(execute_file, encoding="utf-8") as test_script_json: - call_file_automation_test( - main_window, - test_script_json.read(), - program_buffer - ) - except Exception as error: - pybreeze_logger.error(f"file automation multi file error: {error}") + run_dir_files_with_package(main_window, _PACKAGE, False, program_buffer) def call_file_automation_test_multi_file_and_send( main_window: PyBreezeMainWindow, program_buffer: int = 1024000 ): - try: - need_to_execute_list = ask_and_get_dir_files_as_list(main_window) - if need_to_execute_list is not None and len(need_to_execute_list) > 0: - for execute_file in need_to_execute_list: - with open(execute_file, encoding="utf-8") as test_script_json: - call_file_automation_test_with_send( - main_window, - test_script_json.read(), - program_buffer - ) - except Exception as error: - pybreeze_logger.error(f"file automation multi file and send error: {error}") + run_dir_files_with_package(main_window, _PACKAGE, True, program_buffer) diff --git a/pybreeze/extend/process_executor/file_runner_process.py b/pybreeze/extend/process_executor/file_runner_process.py index 8f9ca25..8a00911 100644 --- a/pybreeze/extend/process_executor/file_runner_process.py +++ b/pybreeze/extend/process_executor/file_runner_process.py @@ -21,6 +21,8 @@ from je_editor.pyside_ui.main_ui.save_settings.user_color_setting_file import actually_color_dict from pybreeze.pybreeze_ui.show_code_window.code_window import CodeWindow +from pybreeze.utils.logging.logger import pybreeze_logger +from pybreeze.utils.subprocess_util import no_window_creationflags, utf8_subprocess_env class FileRunnerProcess: @@ -81,6 +83,7 @@ def _compile_and_run(self, compiler: str, args: list, output_flag: str, file_pat compile_cmd, capture_output=True, timeout=60, + creationflags=no_window_creationflags(), ) except FileNotFoundError: self._append_text(f"[Error] Compiler not found: {compiler}\n", is_error=True) @@ -117,6 +120,8 @@ def _start_process(self, command: list[str], cleanup_binary: str | None = None) stderr=subprocess.PIPE, stdin=subprocess.PIPE, shell=False, + creationflags=no_window_creationflags(), + env=utf8_subprocess_env(self.program_encoding), ) except FileNotFoundError: self._append_text(f"[Error] Command not found: {command[0]}\n", is_error=True) @@ -189,8 +194,8 @@ def _finish(self) -> None: if self._cleanup_binary: try: os.remove(self._cleanup_binary) - except OSError: - pass + except OSError as error: + pybreeze_logger.debug("Could not remove compiled binary %s: %s", self._cleanup_binary, error) def _drain_queues(self) -> None: """Drain all remaining messages from output/error queues to UI.""" @@ -211,37 +216,29 @@ def _drain_queues(self) -> None: except queue.Empty: break - def _read_stdout(self) -> None: + def _read_stream(self, stream_name: str, target_queue: Queue) -> None: + # Empty read from readline means the pipe reached EOF (the child closed + # the stream), so stop immediately rather than spinning on a closed pipe + # until the process is reaped. try: while self.still_running: proc = self.process if proc is None: break - data = proc.stdout.readline(self.program_buffer_size) - if not data and proc.poll() is not None: + data = getattr(proc, stream_name).readline(self.program_buffer_size) + if not data: break if isinstance(data, bytes): data = data.decode(self.program_encoding, "replace") - if data: - self.output_queue.put(data) - except (OSError, ValueError): - pass + target_queue.put(data) + except (OSError, ValueError) as error: + pybreeze_logger.debug("Reader for %s stopped: %s", stream_name, error) + + def _read_stdout(self) -> None: + self._read_stream("stdout", self.output_queue) def _read_stderr(self) -> None: - try: - while self.still_running: - proc = self.process - if proc is None: - break - data = proc.stderr.readline(self.program_buffer_size) - if not data and proc.poll() is not None: - break - if isinstance(data, bytes): - data = data.decode(self.program_encoding, "replace") - if data: - self.error_queue.put(data) - except (OSError, ValueError): - pass + self._read_stream("stderr", self.error_queue) def _append_text(self, text: str, is_error: bool) -> None: """Append text to the code result widget.""" diff --git a/pybreeze/extend/process_executor/load_density/load_density_process.py b/pybreeze/extend/process_executor/load_density/load_density_process.py index 2d2a04a..21ee852 100644 --- a/pybreeze/extend/process_executor/load_density/load_density_process.py +++ b/pybreeze/extend/process_executor/load_density/load_density_process.py @@ -2,13 +2,14 @@ from typing import TYPE_CHECKING -from pybreeze.extend.process_executor.process_executor_utils import build_process +from pybreeze.extend.process_executor.process_executor_utils import ( + build_process, run_dir_files_with_package, +) if TYPE_CHECKING: from pybreeze.pybreeze_ui.editor_main.main_ui import PyBreezeMainWindow -from pybreeze.utils.file_process.get_dir_file_list import ask_and_get_dir_files_as_list -from pybreeze.utils.logging.logger import pybreeze_logger +_PACKAGE = "je_load_density" def call_load_density( @@ -16,7 +17,7 @@ def call_load_density( exec_str: str | None = None, program_buffer: int = 1024000 ): - build_process(main_window, "je_load_density", exec_str, False, program_buffer) + build_process(main_window, _PACKAGE, exec_str, False, program_buffer) def call_load_density_with_send( @@ -24,40 +25,18 @@ def call_load_density_with_send( exec_str: str | None = None, program_buffer: int = 1024000 ): - build_process(main_window, "je_load_density", exec_str, True, program_buffer) + build_process(main_window, _PACKAGE, exec_str, True, program_buffer) def call_load_density_multi_file( main_window: PyBreezeMainWindow, program_buffer: int = 1024000 ): - try: - need_to_execute_list = ask_and_get_dir_files_as_list(main_window) - if need_to_execute_list is not None and len(need_to_execute_list) > 0: - for execute_file in need_to_execute_list: - with open(execute_file, encoding="utf-8") as test_script_json: - call_load_density( - main_window, - test_script_json.read(), - program_buffer - ) - except Exception as error: - pybreeze_logger.error(f"load density multi file error: {error}") + run_dir_files_with_package(main_window, _PACKAGE, False, program_buffer) def call_load_density_multi_file_and_send( main_window: PyBreezeMainWindow, program_buffer: int = 1024000 ): - try: - need_to_execute_list = ask_and_get_dir_files_as_list(main_window) - if need_to_execute_list is not None and len(need_to_execute_list) > 0: - for execute_file in need_to_execute_list: - with open(execute_file, encoding="utf-8") as test_script_json: - call_load_density_with_send( - main_window, - test_script_json.read(), - program_buffer - ) - except Exception as error: - pybreeze_logger.error(f"load density multi file and send error: {error}") + run_dir_files_with_package(main_window, _PACKAGE, True, program_buffer) diff --git a/pybreeze/extend/process_executor/process_executor_utils.py b/pybreeze/extend/process_executor/process_executor_utils.py index 3be5fbf..2acaa68 100644 --- a/pybreeze/extend/process_executor/process_executor_utils.py +++ b/pybreeze/extend/process_executor/process_executor_utils.py @@ -10,6 +10,7 @@ from pybreeze.extend.process_executor.python_task_process_manager import TaskProcessManager from pybreeze.utils.exception.exception_tags import wrong_test_data_format_exception_tag from pybreeze.utils.exception.exceptions import ITETestExecutorException +from pybreeze.utils.file_process.get_dir_file_list import ask_and_get_dir_files_as_list from pybreeze.utils.logging.logger import pybreeze_logger if TYPE_CHECKING: @@ -31,7 +32,7 @@ def build_process( test_format_code = exec_str start_process(main_window, package, test_format_code, send_mail, program_buffer) except json.decoder.JSONDecodeError as error: - pybreeze_logger.error(f"{repr(error)}\n{wrong_test_data_format_exception_tag}") + pybreeze_logger.error(f"{error!r}\n{wrong_test_data_format_exception_tag}") except ITETestExecutorException as error: pybreeze_logger.error(repr(error)) @@ -70,6 +71,30 @@ def build_process_from_file( pybreeze_logger.error(repr(error)) +def run_dir_files_with_package( + main_window: PyBreezeMainWindow, + package: str, + send_mail: bool = False, + program_buffer: int = 1024000, +) -> None: + """Prompt for a directory and run every matching file through *package*. + + Each file is executed via its on-disk path (``--execute_file``) so large + action JSON never trips the Windows ~32K command-line limit, and one run + window is opened per file. ``build_process_from_file`` already logs and + contains per-file executor errors; the broad guard here only keeps a single + bad directory pick from crashing the menu callback. + """ + try: + execute_list = ask_and_get_dir_files_as_list(main_window) + if not execute_list: + return + for execute_file in execute_list: + build_process_from_file(main_window, package, execute_file, send_mail, program_buffer) + except Exception as error: # noqa: BLE001 — batch UI action must not abort on one bad entry + pybreeze_logger.error("%s multi file error: %r", package, error) + + def _build_task_process( main_window: PyBreezeMainWindow, send_mail: bool, diff --git a/pybreeze/extend/process_executor/python_task_process_manager.py b/pybreeze/extend/process_executor/python_task_process_manager.py index 6be4839..92040c9 100644 --- a/pybreeze/extend/process_executor/python_task_process_manager.py +++ b/pybreeze/extend/process_executor/python_task_process_manager.py @@ -13,11 +13,13 @@ from PySide6.QtCore import QTimer from PySide6.QtGui import QTextCharFormat from je_editor.pyside_ui.main_ui.save_settings.user_color_setting_file import actually_color_dict +from je_editor.utils.exception.exceptions import JEditorExecException from je_editor.utils.venv_check.check_venv import check_and_choose_venv from pybreeze.extend.process_executor.queue_pump import pump_message_queue from pybreeze.pybreeze_ui.show_code_window.code_window import CodeWindow from pybreeze.utils.logging.logger import pybreeze_logger +from pybreeze.utils.subprocess_util import no_window_creationflags, utf8_subprocess_env def find_venv_path() -> Path: @@ -65,15 +67,26 @@ def __init__( self.error_trigger_function: Callable = error_trigger_function self.program_buffer_size = program_buffer_size - def renew_path(self) -> None: + def renew_path(self) -> bool: + """Resolve the interpreter path. Returns False (without raising) when no + Python can be found, surfacing the error in the run window instead of + crashing the menu callback.""" if self.main_window.python_compiler is None: venv_path = find_venv_path() - self.compiler_path = check_and_choose_venv(venv_path) + try: + self.compiler_path = check_and_choose_venv(venv_path) + except JEditorExecException as error: + pybreeze_logger.error("No Python interpreter found for run: %r", error) + self._append_text(f"[Error] No Python interpreter found: {error}", is_error=True) + self.main_window.show() + return False else: self.compiler_path = self.main_window.python_compiler + return True def start_test_process(self, package: str, exec_str: str): - self.renew_path() + if not self.renew_path(): + return if sys.platform in ["win32", "cygwin", "msys"]: exec_str = json.dumps(exec_str) args = [ @@ -88,7 +101,8 @@ def start_test_process(self, package: str, exec_str: str): def start_test_process_file(self, package: str, file_path: str): # Pass the action JSON as a path so we never hit the Windows ~32K # command-line cap when scripts are large. Caller owns the file. - self.renew_path() + if not self.renew_path(): + return args = [ str(self.compiler_path), "-m", @@ -106,6 +120,8 @@ def _spawn_and_pump(self, package: str, args: list) -> None: args, stdout=subprocess.PIPE, stderr=subprocess.PIPE, + creationflags=no_window_creationflags(), + env=utf8_subprocess_env(self.program_encoding), ) self.still_run_program = True self.read_program_output_from_thread = Thread( @@ -186,24 +202,31 @@ def drain_and_display_queue(self): except queue.Empty: break - def read_program_output_from_process(self): + def _read_stream_into_queue(self, stream_name: str, target_queue: Queue) -> None: + # Block on readline until a line arrives or the pipe hits EOF. Stopping on + # EOF (empty read) is essential: without it the loop spins at 100% CPU + # re-reading a closed pipe until the QTimer notices the process exited. while self.still_run_program: proc = self.process if proc is None: break - program_output_data = proc.stdout.readline(self.program_buffer_size) - if isinstance(program_output_data, bytes): - program_output_data = program_output_data.decode(self.program_encoding, "replace") - if program_output_data.strip(): - self.run_output_queue.put(program_output_data) + stream = getattr(proc, stream_name) + if stream is None: + break + try: + line = stream.readline(self.program_buffer_size) + except (ValueError, OSError): + # Pipe closed underneath us during shutdown. + break + if not line: + break + if isinstance(line, bytes): + line = line.decode(self.program_encoding, "replace") + if line.strip(): + target_queue.put(line) + + def read_program_output_from_process(self): + self._read_stream_into_queue("stdout", self.run_output_queue) def read_program_error_output_from_process(self): - while self.still_run_program: - proc = self.process - if proc is None: - break - program_error_output_data = proc.stderr.readline(self.program_buffer_size) - if isinstance(program_error_output_data, bytes): - program_error_output_data = program_error_output_data.decode(self.program_encoding, "replace") - if program_error_output_data.strip(): - self.run_error_queue.put(program_error_output_data) + self._read_stream_into_queue("stderr", self.run_error_queue) diff --git a/pybreeze/extend/process_executor/queue_pump.py b/pybreeze/extend/process_executor/queue_pump.py index e3c13dc..bcd4eaa 100644 --- a/pybreeze/extend/process_executor/queue_pump.py +++ b/pybreeze/extend/process_executor/queue_pump.py @@ -11,23 +11,30 @@ from collections.abc import Callable from queue import Queue +# Drain up to this many messages per timer tick. Draining one line per ~100 ms +# tick caps throughput at ~10 lines/s and makes chatty scripts crawl; batching +# many lines per tick keeps up with bursty output while the bound keeps the UI +# thread from stalling when a process floods stdout. +MAX_MESSAGES_PER_PUMP = 256 + def pump_message_queue( q: Queue, append_fn: Callable[[str, bool], None], *, is_error: bool, + max_messages: int = MAX_MESSAGES_PER_PUMP, ) -> None: - """Drain one pending message from *q* and forward it to *append_fn*. + """Drain up to *max_messages* pending messages from *q* into *append_fn*. - Silently ignores ``queue.Empty`` (the ``empty()`` check is racy) and empty - strings after stripping. + Stops early when the queue empties. Empty strings (after stripping) are + skipped. ``queue.Empty`` from the racy non-blocking get is treated as a + clean stop, not an error. """ - try: - if q.empty(): + for _ in range(max_messages): + try: + message = str(q.get_nowait()).strip() + except queue.Empty: return - message = str(q.get_nowait()).strip() if message: append_fn(message, is_error) - except queue.Empty: - pass diff --git a/pybreeze/extend/process_executor/test_pioneer/test_pioneer_process_manager.py b/pybreeze/extend/process_executor/test_pioneer/test_pioneer_process_manager.py index 6062b9c..47735ab 100644 --- a/pybreeze/extend/process_executor/test_pioneer/test_pioneer_process_manager.py +++ b/pybreeze/extend/process_executor/test_pioneer/test_pioneer_process_manager.py @@ -14,6 +14,7 @@ from pybreeze.extend.process_executor.python_task_process_manager import find_venv_path from pybreeze.extend.process_executor.queue_pump import pump_message_queue from pybreeze.pybreeze_ui.show_code_window.code_window import CodeWindow +from pybreeze.utils.subprocess_util import no_window_creationflags, utf8_subprocess_env if TYPE_CHECKING: from pybreeze.pybreeze_ui.editor_main.main_ui import PyBreezeMainWindow @@ -61,6 +62,8 @@ def __init__( stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, + creationflags=no_window_creationflags(), + env=utf8_subprocess_env(self._program_encoding), ) def _append_text(self, text: str, is_error: bool = False) -> None: @@ -119,27 +122,34 @@ def drain_and_clear_queue(self): except queue.Empty: break - def read_program_output_from_process(self): + def _read_stream_into_queue(self, stream_name: str, target_queue: Queue) -> None: + # Block on readline until a line arrives or the pipe hits EOF. Stopping on + # EOF (empty read) is essential: without it the loop spins at 100% CPU + # re-reading a closed pipe until the QTimer notices the process exited. while self._still_run_program: proc = self._process if proc is None: break - program_output_data = proc.stdout.readline(self._program_buffer_size) - if isinstance(program_output_data, bytes): - program_output_data = program_output_data.decode(self._program_encoding, "replace") - if program_output_data.strip(): - self._run_output_queue.put(program_output_data) + stream = getattr(proc, stream_name) + if stream is None: + break + try: + line = stream.readline(self._program_buffer_size) + except (ValueError, OSError): + # Pipe closed underneath us during shutdown. + break + if not line: + break + if isinstance(line, bytes): + line = line.decode(self._program_encoding, "replace") + if line.strip(): + target_queue.put(line) + + def read_program_output_from_process(self): + self._read_stream_into_queue("stdout", self._run_output_queue) def read_program_error_output_from_process(self): - while self._still_run_program: - proc = self._process - if proc is None: - break - program_error_output_data = proc.stderr.readline(self._program_buffer_size) - if isinstance(program_error_output_data, bytes): - program_error_output_data = program_error_output_data.decode(self._program_encoding, "replace") - if program_error_output_data.strip(): - self._run_error_queue.put(program_error_output_data) + self._read_stream_into_queue("stderr", self._run_error_queue) def start_test_pioneer_process(self): self._still_run_program = True diff --git a/pybreeze/extend/process_executor/web_runner/web_runner_process.py b/pybreeze/extend/process_executor/web_runner/web_runner_process.py index 64e49d6..39078fe 100644 --- a/pybreeze/extend/process_executor/web_runner/web_runner_process.py +++ b/pybreeze/extend/process_executor/web_runner/web_runner_process.py @@ -3,14 +3,13 @@ from typing import TYPE_CHECKING from pybreeze.extend.process_executor.process_executor_utils import ( - build_process, build_process_from_file, + build_process, run_dir_files_with_package, ) if TYPE_CHECKING: from pybreeze.pybreeze_ui.editor_main.main_ui import PyBreezeMainWindow -from pybreeze.utils.file_process.get_dir_file_list import ask_and_get_dir_files_as_list -from pybreeze.utils.logging.logger import pybreeze_logger +_PACKAGE = "je_web_runner" def call_web_runner_test( @@ -18,7 +17,7 @@ def call_web_runner_test( exec_str: str | None = None, program_buffer: int = 1024000 ): - build_process(main_window, "je_web_runner", exec_str, False, program_buffer) + build_process(main_window, _PACKAGE, exec_str, False, program_buffer) def call_web_runner_test_with_send( @@ -26,36 +25,18 @@ def call_web_runner_test_with_send( exec_str: str | None = None, program_buffer: int = 1024000 ): - build_process(main_window, "je_web_runner", exec_str, True, program_buffer) + build_process(main_window, _PACKAGE, exec_str, True, program_buffer) def call_web_runner_test_multi_file( main_window: PyBreezeMainWindow, program_buffer: int = 1024000 ): - try: - need_to_execute_list = ask_and_get_dir_files_as_list(main_window) - if need_to_execute_list is not None and len(need_to_execute_list) > 0: - for execute_file in need_to_execute_list: - build_process_from_file( - main_window, "je_web_runner", execute_file, - False, program_buffer, - ) - except Exception as error: - pybreeze_logger.error(f"web runner multi file error: {error}") + run_dir_files_with_package(main_window, _PACKAGE, False, program_buffer) def call_web_runner_test_multi_file_and_send( main_window: PyBreezeMainWindow, program_buffer: int = 1024000 ): - try: - need_to_execute_list = ask_and_get_dir_files_as_list(main_window) - if need_to_execute_list is not None and len(need_to_execute_list) > 0: - for execute_file in need_to_execute_list: - build_process_from_file( - main_window, "je_web_runner", execute_file, - True, program_buffer, - ) - except Exception as error: - pybreeze_logger.error(f"web runner multi file and send error: {error}") + run_dir_files_with_package(main_window, _PACKAGE, True, program_buffer) diff --git a/pybreeze/extend_multi_language/extend_traditional_chinese.py b/pybreeze/extend_multi_language/extend_traditional_chinese.py index 0795ec2..a32e214 100644 --- a/pybreeze/extend_multi_language/extend_traditional_chinese.py +++ b/pybreeze/extend_multi_language/extend_traditional_chinese.py @@ -125,7 +125,7 @@ "ssh_command_widget_status_label_disconnected": "已斷線", "ssh_command_widget_log_message_connected": "已連線至", "ssh_command_widget_log_message_error": "[錯誤]", - "ssh_command_widget_error_message_decode_failed": "<解碼錯誤> {error}", + "ssh_command_widget_error_message_decode_failed": "<解碼錯誤> ", "ssh_command_widget_log_message_channel_closed": "[通道已關閉]", "ssh_command_widget_error_message_reader_failed": "讀取錯誤", "ssh_command_widget_log_message_reader_closed": "讀取已關閉", diff --git a/pybreeze/pybreeze_ui/connect_gui/ssh/ssh_command_widget.py b/pybreeze/pybreeze_ui/connect_gui/ssh/ssh_command_widget.py index 09562db..002b7e5 100644 --- a/pybreeze/pybreeze_ui/connect_gui/ssh/ssh_command_widget.py +++ b/pybreeze/pybreeze_ui/connect_gui/ssh/ssh_command_widget.py @@ -17,7 +17,22 @@ from pybreeze.pybreeze_ui.connect_gui.ssh.ssh_login_widget import LoginWidget from pybreeze.utils.logging.logger import pybreeze_logger -ANSI_ESCAPE_PATTERN = re.compile(r'\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])') +ANSI_ESCAPE_PATTERN = re.compile( + r'\x1B(?:' + r'\][^\x07\x1B]*(?:\x07|\x1B\\)?' # OSC; BEL/ST-terminated or implicitly ended by the next ESC / EOF + r'|[@-Z\\-_]' # other two-character C1 Fe sequences + r'|\[[0-?]*[ -/]*[@-~]' # CSI (colours, cursor movement) + r')' +) + +# Bound the terminal scrollback so an endless stream (``tail -f``, ``yes``) +# cannot grow the document without limit; oldest lines drop once exceeded. +TERMINAL_MAX_BLOCKS = 10000 + +# Send a keepalive packet after this many idle seconds so an idle session is not +# dropped by the TCP stack, a NAT/firewall, or the SSH server (≈ OpenSSH's +# ServerAliveInterval). +SSH_KEEPALIVE_SECONDS = 30 class SSHReaderThread(QThread): @@ -30,29 +45,31 @@ def __init__(self, chan: paramiko.Channel, parent=None): self._running = True self.word_dict = language_wrapper.language_word_dict + def _pump_once(self) -> bool: + """Forward any ready stdout/stderr; return False once the channel closes.""" + if self.chan.recv_ready(): + data = self.chan.recv(4096) + if data: + self.data_received.emit(data) + if self.chan.recv_stderr_ready(): + err = self.chan.recv_stderr(4096) + if err: + self.data_received.emit(err) + return not (self.chan.closed or self.chan.exit_status_ready()) + def run(self): + error_msg = None try: - while self._running: - if self.chan.recv_ready(): - data = self.chan.recv(4096) - if data: - self.data_received.emit(data) - - if self.chan.recv_stderr_ready(): - err = self.chan.recv_stderr(4096) - if err: - self.data_received.emit(err) - - if self.chan.closed or self.chan.exit_status_ready(): - break - + while self._running and self._pump_once(): self.msleep(10) - except Exception as e: - self.closed.emit( - f"{self.word_dict.get('ssh_command_widget_error_message_reader_failed')} {e}") + except Exception as e: # noqa: BLE001 — any reader failure must surface to the UI + pybreeze_logger.debug("SSH reader thread error: %r", e) + error_msg = f"{self.word_dict.get('ssh_command_widget_error_message_reader_failed')} {e}" finally: + # Emit exactly once: the error when one occurred, otherwise the + # normal close notice (previously the error path emitted twice). self.closed.emit( - self.word_dict.get("ssh_command_widget_log_message_reader_closed")) + error_msg or self.word_dict.get("ssh_command_widget_log_message_reader_closed")) def stop(self): self._running = False @@ -83,6 +100,7 @@ def __init__(self, external_login_widget: LoginWidget = None, add_login_widget: # 其他 UI 控制元件 self.terminal = QPlainTextEdit() self.terminal.setReadOnly(True) + self.terminal.setMaximumBlockCount(TERMINAL_MAX_BLOCKS) self.command_input_edit = QLineEdit() self.command_send_button = QPushButton( self.word_dict.get("ssh_command_widget_button_label_send_command")) @@ -140,6 +158,10 @@ def connect_ssh(self): return try: + # Tear down any prior session first: re-clicking Connect while already + # connected would otherwise leak the old SSH client and orphan its + # reader thread (which keeps appending to the terminal). + self._cleanup() self.ssh_client = paramiko.SSHClient() apply_host_key_policy(self.ssh_client, self) pybreeze_logger.info("SSH connecting to %s:%s", host, port) @@ -181,6 +203,9 @@ def _authenticate_with_key(self, host: str, port: int, user: str, key_path: str, return True def _start_shell(self, host: str, port: int, user: str) -> None: + transport = self.ssh_client.get_transport() + if transport is not None: + transport.set_keepalive(SSH_KEEPALIVE_SECONDS) self.shell_channel = self.ssh_client.invoke_shell(term='xterm', width=120, height=32) self.shell_channel.settimeout(0.0) self.reader_thread = SSHReaderThread(self.shell_channel) @@ -233,6 +258,9 @@ def disconnect_ssh(self): def _cleanup(self): try: if self.reader_thread: + # Block the thread's signals before tearing down so a data/closed + # signal emitted mid-shutdown can't land in a half-cleaned widget. + self.reader_thread.blockSignals(True) self.reader_thread.stop() self.reader_thread.wait(1000) except Exception as error: diff --git a/pybreeze/pybreeze_ui/connect_gui/ssh/ssh_file_viewer_widget.py b/pybreeze/pybreeze_ui/connect_gui/ssh/ssh_file_viewer_widget.py index 75cfd92..00764a2 100644 --- a/pybreeze/pybreeze_ui/connect_gui/ssh/ssh_file_viewer_widget.py +++ b/pybreeze/pybreeze_ui/connect_gui/ssh/ssh_file_viewer_widget.py @@ -1,8 +1,9 @@ from __future__ import annotations import os +import posixpath +import re import stat -from pathlib import Path import paramiko from PySide6.QtCore import Qt, QEvent @@ -18,6 +19,52 @@ from pybreeze.utils.logging.logger import pybreeze_logger +_SIZE_UNITS = ("B", "KB", "MB", "GB", "TB", "PB") + + +def format_size(num_bytes: int) -> str: + """Format a byte count for display, e.g. ``1536 -> '1.5 KB'`` (1024-based).""" + if num_bytes < 0: + return "" + size = float(num_bytes) + for unit in _SIZE_UNITS: + if size < 1024 or unit == _SIZE_UNITS[-1]: + return f"{int(size)} {unit}" if unit == "B" else f"{size:.1f} {unit}" + size /= 1024 + return "" + + +# Keepalive interval (seconds) so an idle SFTP session is not dropped by the +# TCP stack, a NAT/firewall, or the SSH server (≈ OpenSSH ServerAliveInterval). +SSH_KEEPALIVE_SECONDS = 30 + + +def natural_key(name: str) -> list: + """Sort key giving natural (human) order, e.g. ``img2`` before ``img10``. + + Embedded digit runs compare as integers and text compares case-insensitively, + matching how Windows Explorer and most file managers order file listings. + """ + # isdecimal (not isdigit): a part like "²³" or "①" is isdigit() but int() + # rejects it, which would crash sorting a file named with such characters. + return [ + int(part) if part.isdecimal() else part.lower() + for part in re.split(r"(\d+)", name) + ] + + +def remote_join(directory: str, name: str) -> str: + """Join *name* under a remote POSIX *directory* into an absolute path. + + Remote SFTP paths are always POSIX: ``os.path.join`` would emit ``\\`` on a + Windows client and break navigation/transfers on the server. The result + always uses ``/`` separators and has a leading slash. + """ + base = "" if directory == "/" else directory + joined = posixpath.join(base, name) + return joined if joined.startswith("/") else f"/{joined}" + + class SFTPClientWrapper: """ Lightweight wrapper around Paramiko SFTP client. @@ -41,16 +88,25 @@ def connect(self, host: str, port: int, username: str, password: str, self._ssh = paramiko.SSHClient() apply_host_key_policy(self._ssh, parent_widget) pybreeze_logger.info("SFTP connecting to %s:%s", host, port) - if use_key and key_path: - pkey = load_private_key(key_path, password, context="SFTP") - if pkey is None: - raise ValueError( - self.word_dict.get("ssh_command_widget_error_message_unsupported_private_key") - ) - self._ssh.connect(hostname=host, port=port, username=username, pkey=pkey, timeout=10) - else: - self._ssh.connect(hostname=host, port=port, username=username, password=password, timeout=10) - self._sftp = self._ssh.open_sftp() + try: + if use_key and key_path: + pkey = load_private_key(key_path, password, context="SFTP") + if pkey is None: + raise ValueError( + self.word_dict.get("ssh_command_widget_error_message_unsupported_private_key") + ) + self._ssh.connect(hostname=host, port=port, username=username, pkey=pkey, timeout=10) + else: + self._ssh.connect(hostname=host, port=port, username=username, password=password, timeout=10) + transport = self._ssh.get_transport() + if transport is not None: + transport.set_keepalive(SSH_KEEPALIVE_SECONDS) + self._sftp = self._ssh.open_sftp() + except Exception: + # A failure partway (auth, keepalive, or open_sftp) must not leak the + # half-open SSH transport: tear it down so connect() is all-or-nothing. + self.close() + raise def close(self): """ @@ -244,7 +300,8 @@ def make_item(self, name: str, typ: str, size: int, full_path: str) -> QTreeWidg Create a tree item with metadata. 建立帶有中繼資料的樹狀項目。 """ - item = QTreeWidgetItem([name, typ, str(size), full_path]) + size_text = "" if typ == "dir" else format_size(size) + item = QTreeWidgetItem([name, typ, size_text, full_path]) if typ == "dir": # Use QStyle enum for standard icons # 使用 QStyle 列舉取得標準資料夾圖示 @@ -304,19 +361,18 @@ def populate_children(self, parent_item: QTreeWidgetItem): @staticmethod def _sort_entries(entries): - """Return ``[(name, entry)]`` with directories before files.""" + """Return ``[(name, entry)]`` with directories first, each in natural order.""" dirs: list = [] files: list = [] for entry in entries: bucket = dirs if stat.S_ISDIR(entry.st_mode) else files bucket.append((entry.filename, entry)) + dirs.sort(key=lambda item: natural_key(item[0])) + files.sort(key=lambda item: natural_key(item[0])) return dirs + files def _add_entry_row(self, parent_item: QTreeWidgetItem, path: str, name: str, entry) -> None: - base = "" if path == "/" else path - full_path = os.path.join(base, name) - if not full_path.startswith("/"): - full_path = f"/{full_path}" + full_path = remote_join(path, name) typ = "dir" if stat.S_ISDIR(entry.st_mode) else "file" size = entry.st_size if typ == "file" else 0 child = self.make_item(name, typ, size, full_path) @@ -389,14 +445,13 @@ def action_create_folder(self, item: QTreeWidgetItem | None): return base_path = item.text(3) if item.text(1) != "dir": - base_path = os.path.dirname(base_path) + base_path = posixpath.dirname(base_path) name, ok = self.get_text( self.word_dict.get("ssh_file_viewer_dialog_title_create_folder"), self.word_dict.get("ssh_file_viewer_dialog_label_folder_name")) if not ok or not name.strip(): return - new_path = os.path.join(base_path if base_path != "/" else "", name.strip()) - new_path = new_path if new_path.startswith("/") else f"/{new_path}" + new_path = remote_join(base_path, name.strip()) self.client.mkdir(new_path) self.action_refresh(item) @@ -413,9 +468,8 @@ def action_rename(self, item: QTreeWidgetItem | None): f"{self.word_dict.get('ssh_file_viewer_dialog_label_new_name_for_item')}: {item.text(0)}") if not ok or not new_name.strip(): return - base = os.path.dirname(old_path) or "/" - new_path = os.path.join(base if base != "/" else "", new_name.strip()) - new_path = new_path if new_path.startswith("/") else f"/{new_path}" + base = posixpath.dirname(old_path) or "/" + new_path = remote_join(base, new_name.strip()) self.client.rename(old_path, new_path) # Update item display item.setText(0, new_name.strip()) @@ -460,7 +514,7 @@ def action_download(self, item: QTreeWidgetItem | None): self.word_dict.get("ssh_file_viewer_dialog_message_select_file_to_download")) return remote_path = item.text(3) - suggested = Path(remote_path).name + suggested = posixpath.basename(remote_path) # remote path uses POSIX separators local_path, _ = QFileDialog.getSaveFileName( self, self.word_dict.get("ssh_file_viewer_dialog_title_save_as"), @@ -480,14 +534,13 @@ def action_upload(self, item: QTreeWidgetItem | None): """ if item is None: return - target_dir = item.text(3) if item.text(1) == "dir" else os.path.dirname(item.text(3)) + target_dir = item.text(3) if item.text(1) == "dir" else posixpath.dirname(item.text(3)) local_path, _ = QFileDialog.getOpenFileName( self, self.word_dict.get("ssh_file_viewer_dialog_title_select_local_file"), "") if not local_path: return - filename = os.path.basename(local_path) - remote_path = os.path.join(target_dir if target_dir != "/" else "", filename) - remote_path = remote_path if remote_path.startswith("/") else f"/{remote_path}" + filename = os.path.basename(local_path) # local path: OS-native separator is correct + remote_path = remote_join(target_dir, filename) self.client.upload(local_path, remote_path) QMessageBox.information( self, diff --git a/pybreeze/pybreeze_ui/connect_gui/ssh/ssh_host_key_policy.py b/pybreeze/pybreeze_ui/connect_gui/ssh/ssh_host_key_policy.py index 0a6571a..84936c2 100644 --- a/pybreeze/pybreeze_ui/connect_gui/ssh/ssh_host_key_policy.py +++ b/pybreeze/pybreeze_ui/connect_gui/ssh/ssh_host_key_policy.py @@ -17,6 +17,7 @@ from je_editor import language_wrapper from PySide6.QtWidgets import QMessageBox +from pybreeze.utils.app_dirs import pybreeze_data_dir from pybreeze.utils.logging.logger import pybreeze_logger if TYPE_CHECKING: @@ -25,9 +26,7 @@ def _known_hosts_path() -> Path: """Return the PyBreeze-managed known_hosts file path, ensuring the parent dir exists.""" - home_dir = Path.home() / ".pybreeze" - home_dir.mkdir(parents=True, exist_ok=True) - return home_dir / "ssh_known_hosts" + return pybreeze_data_dir() / "ssh_known_hosts" def _fingerprint_sha256(key: paramiko.PKey) -> str: diff --git a/pybreeze/pybreeze_ui/connect_gui/url/ai_code_review_gui.py b/pybreeze/pybreeze_ui/connect_gui/url/ai_code_review_gui.py index 6dc6f3a..bb74f6c 100644 --- a/pybreeze/pybreeze_ui/connect_gui/url/ai_code_review_gui.py +++ b/pybreeze/pybreeze_ui/connect_gui/url/ai_code_review_gui.py @@ -9,6 +9,11 @@ ) from je_editor import language_wrapper +from pybreeze.utils.app_dirs import pybreeze_data_dir +from pybreeze.utils.logging.logger import pybreeze_logger +from pybreeze.utils.network.http_client import ( + ResponseTooLargeError, read_capped_text, CONNECT_TIMEOUT, +) from pybreeze.utils.network.url_validation import UnsafeURLError, validate_url @@ -23,10 +28,11 @@ def __init__(self): # 記錄接受/拒絕次數 self.accept_count = 0 self.reject_count = 0 - data_dir = os.path.join(os.getcwd(), ".pybreeze") - os.makedirs(data_dir, exist_ok=True) - self.stats_file = os.path.join(data_dir, "response_stats.txt") - self.url_file = os.path.join(data_dir, "urls.txt") + # Store under the user's home (like the SSH known_hosts) so the data is + # stable regardless of which directory the IDE was launched from. + data_dir = pybreeze_data_dir() + self.stats_file = str(data_dir / "response_stats.txt") + self.url_file = str(data_dir / "urls.txt") # 主佈局 (垂直) main_layout = QVBoxLayout() @@ -144,21 +150,22 @@ def send_request(self): try: if method == "GET": - response = requests.get(url, timeout=30, allow_redirects=False) + response = requests.get(url, timeout=(CONNECT_TIMEOUT, 30), allow_redirects=False, stream=True) elif method == "POST": - response = requests.post(url, data={"code": code_content}, timeout=30, allow_redirects=False) + response = requests.post(url, data={"code": code_content}, timeout=(CONNECT_TIMEOUT, 30), allow_redirects=False, stream=True) elif method == "PUT": - response = requests.put(url, data={"code": code_content}, timeout=30, allow_redirects=False) + response = requests.put(url, data={"code": code_content}, timeout=(CONNECT_TIMEOUT, 30), allow_redirects=False, stream=True) elif method == "DELETE": - response = requests.delete(url, timeout=30, allow_redirects=False) + response = requests.delete(url, timeout=(CONNECT_TIMEOUT, 30), allow_redirects=False, stream=True) else: self.response_panel.setPlainText( self.word_dict.get("ai_code_review_gui_message_unsupported_http_method")) return - self.response_panel.append(response.text) + self.response_panel.append(read_capped_text(response)) - except Exception as e: + except (requests.RequestException, ResponseTooLargeError) as e: + pybreeze_logger.error("AI code review request failed: %r", e) self.response_panel.setPlainText(f"{self.word_dict.get('ai_code_review_gui_message_error')}: {e}") def accept_response(self): diff --git a/pybreeze/pybreeze_ui/diagram_editor/diagram_editor_widget.py b/pybreeze/pybreeze_ui/diagram_editor/diagram_editor_widget.py index 0ebba53..153648b 100644 --- a/pybreeze/pybreeze_ui/diagram_editor/diagram_editor_widget.py +++ b/pybreeze/pybreeze_ui/diagram_editor/diagram_editor_widget.py @@ -457,6 +457,15 @@ def _get_content_rect(self) -> QRectF: rect = self._scene.itemsBoundingRect() return rect.marginsAdded(QMarginsF(40, 40, 40, 40)) + def _warn_export_failed(self, path: str, reason: str) -> None: + pybreeze_logger.error("Diagram export to %s failed: %s", path, reason) + QMessageBox.warning( + self, + _lang("diagram_editor_error_title", "Error"), + _lang("diagram_editor_export_failed", "Could not export the diagram to:\n{path}") + .format(path=path), + ) + def _export_png(self) -> None: path, _ = QFileDialog.getSaveFileName( self, _lang("diagram_editor_dialog_export_png", "Export PNG"), @@ -471,6 +480,14 @@ def _export_png(self) -> None: int(rect.width() * scale), int(rect.height() * scale), QImage.Format.Format_ARGB32_Premultiplied, ) + if image.isNull(): + self._warn_export_failed(path, "could not allocate image") + return + # Tag the 2x raster with matching DPI (96 base * scale) so viewers and + # print show it at the intended physical size instead of doubled. + dots_per_meter = int(scale * 96 / 0.0254) + image.setDotsPerMeterX(dots_per_meter) + image.setDotsPerMeterY(dots_per_meter) image.fill(Qt.GlobalColor.white) painter = QPainter(image) painter.setRenderHint(QPainter.RenderHint.Antialiasing) @@ -479,9 +496,12 @@ def _export_png(self) -> None: self._scene.clearSelection() self._scene.render(painter, QRectF(), rect) painter.end() - image.save(path) - except Exception as e: - pybreeze_logger.error(f"Export PNG failed: {e}") + # QImage.save returns False (without raising) on permission/path/format + # errors, so the result must be checked to avoid a silent failure. + if not image.save(path): + self._warn_export_failed(path, "QImage.save returned False") + except Exception as error: # noqa: BLE001 — export must not crash the editor + self._warn_export_failed(path, repr(error)) def _export_svg(self) -> None: path, _ = QFileDialog.getSaveFileName( @@ -497,13 +517,16 @@ def _export_svg(self) -> None: gen.setSize(QSizeF(rect.width(), rect.height()).toSize()) gen.setViewBox(QRectF(0, 0, rect.width(), rect.height())) painter = QPainter(gen) + if not painter.isActive(): + self._warn_export_failed(path, "could not open SVG for writing") + return painter.setRenderHint(QPainter.RenderHint.Antialiasing) painter.translate(-rect.topLeft()) self._scene.clearSelection() self._scene.render(painter, QRectF(), rect) painter.end() - except Exception as e: - pybreeze_logger.error(f"Export SVG failed: {e}") + except Exception as error: # noqa: BLE001 — export must not crash the editor + self._warn_export_failed(path, repr(error)) # ------------------------------------------------------------------ # Image operations diff --git a/pybreeze/pybreeze_ui/diagram_editor/diagram_items.py b/pybreeze/pybreeze_ui/diagram_editor/diagram_items.py index 546eea8..78366f7 100644 --- a/pybreeze/pybreeze_ui/diagram_editor/diagram_items.py +++ b/pybreeze/pybreeze_ui/diagram_editor/diagram_items.py @@ -103,9 +103,24 @@ def __init__(self, w: float, h: float): _DEFAULT_NODE_W = 140.0 _DEFAULT_NODE_H = 60.0 +_MIN_NODE_W = 40.0 +_MIN_NODE_H = 20.0 _NODE_PEN_COLOR = "#455a64" _NODE_BRUSH_COLOR = "#e3f2fd" _NODE_SELECTED_COLOR = "#1565c0" + + +def _safe_color(value: str | None, fallback: str) -> QColor: + """Return ``QColor(value)`` if it parses, else the fallback colour. + + A corrupted/hand-edited diagram can carry an unparseable colour string; + ``QColor`` would silently produce an invalid (black) colour, so validate it. + """ + if value: + color = QColor(value) + if color.isValid(): + return color + return QColor(fallback) _LABEL_FONT_FAMILY = "Segoe UI" _LABEL_FONT_SIZE = 10 _CONNECTION_COLOR = "#37474f" @@ -222,6 +237,10 @@ def __init__( border_color: str | None = None, font_size: int = _LABEL_FONT_SIZE, ): + # Clamp to a positive minimum so a zero/negative size from corrupted data + # can't cause a divide-by-zero when computing edge intersection points. + w = max(_MIN_NODE_W, w) + h = max(_MIN_NODE_H, h) super().__init__(0, 0, w, h) self.setPen(QPen(Qt.PenStyle.NoPen)) self.setBrush(QBrush(Qt.BrushStyle.NoBrush)) @@ -238,8 +257,8 @@ def __init__( self._resizing = False # Colors - self._fill_color = QColor(fill_color) if fill_color else QColor(_NODE_BRUSH_COLOR) - self._border_color = QColor(border_color) if border_color else QColor(_NODE_PEN_COLOR) + self._fill_color = _safe_color(fill_color, _NODE_BRUSH_COLOR) + self._border_color = _safe_color(border_color, _NODE_PEN_COLOR) self._font_size = font_size # Shape body (child) diff --git a/pybreeze/pybreeze_ui/diagram_editor/diagram_mermaid_parser.py b/pybreeze/pybreeze_ui/diagram_editor/diagram_mermaid_parser.py index d4902b8..76aeef8 100644 --- a/pybreeze/pybreeze_ui/diagram_editor/diagram_mermaid_parser.py +++ b/pybreeze/pybreeze_ui/diagram_editor/diagram_mermaid_parser.py @@ -2,6 +2,7 @@ import re from collections import defaultdict, deque +from collections.abc import Callable from dataclasses import dataclass from pybreeze.pybreeze_ui.diagram_editor.diagram_items import ( @@ -45,17 +46,18 @@ class _EdgeInfo: re.IGNORECASE, ) -# Arrow operators with optional pipe-label. -# Order matters: longer patterns first to avoid partial matches. +# Arrow / link operator with optional pipe-label. Handles every common mermaid +# link: normal/thick/dotted bodies, optional right head (arrow ``>``, circle +# ``o`` or cross ``x``) and an optional ``<`` left head for bidirectional links. +# The left head is restricted to ``<`` on purpose: allowing ``o``/``x`` as a left +# head would wrongly split node names ending in those letters (e.g. ``Box-->B``). _ARROW_SPLIT_RE = re.compile( r"\s*" r"(" - r"={2,}>(?:\|[^|]*\|)?" # ==> or ==>|label| - r"|-\.+->(?:\|[^|]*\|)?" # -.-> or -.->|label| - r"|--+>(?:\|[^|]*\|)?" # --> or -->|label| - r"|={2,}" # === - r"|-\.+-" # -.- - r"|---+" # --- + r"ox]?" # optional right head: arrow / circle / cross + r"(?:\|[^|]*\|)?" # optional |label| r")" r"\s*" ) @@ -77,23 +79,40 @@ def _normalize_inline_labels(line: str) -> str: return line -# Ordered longest-first so the double-bracket shapes match before single-bracket ones. +# Ordered longest-first so multi-char shapes match before their single-char +# prefixes (e.g. ``(((`` before ``((`` before ``(``). The editor has four node +# shapes, so mermaid shapes without an exact counterpart map to the nearest one +# (cylinder/subroutine/parallelogram/trapezoid/asymmetric -> rectangle, +# double-circle -> ellipse, hexagon -> diamond). _SHAPE_DELIMS: tuple[tuple[str, str, NodeShape], ...] = ( - ("((", "))", NodeShape.ELLIPSE), - ("([", "])", NodeShape.ROUNDED_RECT), - ("{{", "}}", NodeShape.DIAMOND), - ("[[", "]]", NodeShape.RECTANGLE), - ("(", ")", NodeShape.ROUNDED_RECT), - ("{", "}", NodeShape.DIAMOND), - ("[", "]", NodeShape.RECTANGLE), + ("(((", ")))", NodeShape.ELLIPSE), # double circle + ("((", "))", NodeShape.ELLIPSE), # circle + ("([", "])", NodeShape.ROUNDED_RECT), # stadium / pill + ("[[", "]]", NodeShape.RECTANGLE), # subroutine + ("[(", ")]", NodeShape.RECTANGLE), # cylinder / database + ("[/", "/]", NodeShape.RECTANGLE), # parallelogram + ("[\\", "\\]", NodeShape.RECTANGLE), # trapezoid + ("{{", "}}", NodeShape.DIAMOND), # hexagon + (">", "]", NodeShape.RECTANGLE), # asymmetric / flag + ("(", ")", NodeShape.ROUNDED_RECT), # round edges + ("{", "}", NodeShape.DIAMOND), # rhombus / decision + ("[", "]", NodeShape.RECTANGLE), # rectangle ) +def _unquote(text: str) -> str: + """Drop the surrounding double quotes mermaid uses to allow special chars.""" + if len(text) >= 2 and text[0] == '"' and text[-1] == '"': + return text[1:-1] + return text + + def _extract_shape(rest: str, default_text: str) -> tuple[str, NodeShape]: """Strip matching delimiters from ``rest`` and return ``(text, shape)``.""" for open_tok, close_tok, shape in _SHAPE_DELIMS: if rest.startswith(open_tok) and rest.endswith(close_tok): - return rest[len(open_tok):-len(close_tok)].strip(), shape + inner = rest[len(open_tok):-len(close_tok)].strip() + return _unquote(inner), shape return default_text, NodeShape.RECTANGLE @@ -109,11 +128,55 @@ def _parse_node_ref(raw: str, nodes: dict[str, _NodeInfo]) -> str | None: node_id = m.group(1) rest = m.group(2).strip() text, shape = _extract_shape(rest, default_text=node_id) - if node_id not in nodes: + existing = nodes.get(node_id) + if existing is None: nodes[node_id] = _NodeInfo(id=node_id, text=text, shape=shape) + elif rest: + # An explicit label/shape declaration updates a node that was first + # seen as a bare reference (mermaid commonly declares edges before + # labelling the nodes). A bare reference never clobbers a label. + existing.text = text + existing.shape = shape return node_id +def _split_node_group(raw: str) -> list[str]: + """Split a node group on top-level ``&`` (mermaid fan-in/out). + + ``&`` inside brackets or quotes is part of a label, not a separator, so the + split tracks bracket depth and quote state (e.g. ``A["Tom & Jerry"] & B`` + yields two members, not three). + """ + parts: list[str] = [] + current: list[str] = [] + depth = 0 + in_quote = False + for char in raw: + if char == '"': + in_quote = not in_quote + elif not in_quote and char in "([{": + depth += 1 + elif not in_quote and char in ")]}": + depth = max(0, depth - 1) + if char == "&" and depth == 0 and not in_quote: + parts.append("".join(current)) + current = [] + else: + current.append(char) + parts.append("".join(current)) + return parts + + +def _parse_node_group(raw: str, nodes: dict[str, _NodeInfo]) -> list[str]: + """Parse one ``&``-joined node group, returning the registered node IDs.""" + ids: list[str] = [] + for member in _split_node_group(raw): + node_id = _parse_node_ref(member, nodes) + if node_id is not None: + ids.append(node_id) + return ids + + def _parse_arrow(token: str) -> tuple[str, ConnectionStyle, float]: """Return ``(label, style, line_width)`` from an arrow token.""" label = "" @@ -122,9 +185,9 @@ def _parse_arrow(token: str) -> tuple[str, ConnectionStyle, float]: label = lm.group(1).strip() if "==" in token: - return label, ConnectionStyle.SOLID, 3.5 + return label, ConnectionStyle.SOLID, 3.5 # thick link if "-." in token: - return label, ConnectionStyle.DASHED, 2.0 + return label, ConnectionStyle.DOTTED, 2.0 # dotted link return label, ConnectionStyle.SOLID, 2.0 @@ -154,10 +217,16 @@ def _assign_layers( roots = [nid for nid, deg in in_deg.items() if deg == 0] or [next(iter(nodes))] layers: dict[str, int] = dict.fromkeys(roots, 0) queue: deque[str] = deque(roots) + # Longest simple path in a graph of N nodes spans at most N-1 edges. A + # back-edge (cycle or self-loop) would otherwise relax layers upward + # forever and hang the parser, so we never push a layer to N or beyond. + max_layer_bound = len(nodes) while queue: nid = queue.popleft() for child in adj.get(nid, []): new_layer = layers[nid] + 1 + if new_layer >= max_layer_bound: + continue if child not in layers or layers[child] < new_layer: layers[child] = new_layer queue.append(child) @@ -169,6 +238,115 @@ def _assign_layers( return layers +# Alternating top-down/bottom-up barycenter sweeps. Four passes is the usual +# sweet spot — most crossing reduction lands in the first couple of sweeps. +_LAYOUT_SWEEPS = 4 + + +def _sweep_layers( + layer_groups: dict[int, list[str]], + adj: dict[str, list[str]], + rev_adj: dict[str, list[str]], + apply_layer: Callable[[list[str], int, dict[str, list[str]]], None], +) -> None: + """Run alternating top-down / bottom-up Sugiyama sweeps. + + For each layer (in sweep order) calls ``apply_layer(group, fixed_layer, + neighbours)`` where *neighbours* is the adjacency toward the adjacent, + already-positioned *fixed_layer*. Centralises the sweep skeleton so the + ordering and coordinate passes stay flat and low-complexity. + """ + layer_indices = sorted(layer_groups.keys()) + for sweep in range(_LAYOUT_SWEEPS): + going_down = sweep % 2 == 0 + order = layer_indices if going_down else layer_indices[::-1] + neighbours = rev_adj if going_down else adj + for layer in order: + fixed_layer = layer - 1 if going_down else layer + 1 + apply_layer(layer_groups[layer], fixed_layer, neighbours) + + +def _order_layers( + layer_groups: dict[int, list[str]], + layers: dict[str, int], + adj: dict[str, list[str]], + rev_adj: dict[str, list[str]], +) -> None: + """Reorder nodes within each layer to reduce edge crossings, in place. + + Standard Sugiyama crossing-reduction barycenter heuristic: sweep layer by + layer (alternating direction) moving every node to the mean position of its + neighbours in the adjacent, already-fixed layer. + """ + pos: dict[str, int] = {} + for group in layer_groups.values(): + for index, nid in enumerate(group): + pos[nid] = index + + def barycenter(nid: str, neighbours: dict[str, list[str]], fixed_layer: int) -> float: + coords = [pos[n] for n in neighbours.get(nid, []) if layers.get(n) == fixed_layer] + return sum(coords) / len(coords) if coords else float(pos[nid]) + + def reorder(group: list[str], fixed_layer: int, neighbours: dict[str, list[str]]) -> None: + group.sort(key=lambda nid: barycenter(nid, neighbours, fixed_layer)) + for index, nid in enumerate(group): + pos[nid] = index + + _sweep_layers(layer_groups, adj, rev_adj, reorder) + + +def _resolve_offsets(group: list[str], desired: list[float], offset: dict[str, float]) -> None: + """Place *group* near *desired* offsets, preserving order, in place. + + Nodes keep their within-layer order (so no new crossings) and stay at least + one slot apart (so no overlaps); the layer is then re-centred on zero. + """ + placed: list[float] = [] + prev: float | None = None + for want in desired: + value = want if prev is None else max(want, prev + 1.0) + placed.append(value) + prev = value + if not placed: + return + # Shift the whole run so its centre matches the centre of the desired + # positions: this preserves alignment (a lone child stays under its parent) + # while cancelling the left-to-right spacing push. + shift = sum(desired) / len(desired) - sum(placed) / len(placed) + for nid, value in zip(group, placed): + offset[nid] = value + shift + + +def _assign_cross_offsets( + layer_groups: dict[int, list[str]], + layers: dict[str, int], + adj: dict[str, list[str]], + rev_adj: dict[str, list[str]], +) -> dict[str, float]: + """Assign a cross-axis offset to each node, aligning it with its neighbours. + + The Sugiyama coordinate-assignment phase: alternating sweeps pull each node + toward the mean offset of its neighbours in the adjacent fixed layer, which + straightens edges. ``_resolve_offsets`` keeps the result legal (ordered, + non-overlapping, centred). + """ + offset: dict[str, float] = {} + for group in layer_groups.values(): + count = len(group) + for i, nid in enumerate(group): + offset[nid] = float(i) - (count - 1) / 2 + + def realign(group: list[str], fixed_layer: int, neighbours: dict[str, list[str]]) -> None: + desired = [] + for nid in group: + coords = [offset[n] for n in neighbours.get(nid, []) if layers.get(n) == fixed_layer] + desired.append(sum(coords) / len(coords) if coords else offset[nid]) + _resolve_offsets(group, desired, offset) + + _sweep_layers(layer_groups, adj, rev_adj, realign) + return offset + + _NODE_H = 60.0 _GAP_MAIN = 120.0 _GAP_CROSS = 80.0 @@ -202,19 +380,24 @@ def _auto_layout( adj, in_deg = _build_adjacency(nodes, edges) layers = _assign_layers(nodes, adj, in_deg) + rev_adj: dict[str, list[str]] = defaultdict(list) + for source, targets in adj.items(): + for target in targets: + rev_adj[target].append(source) + layer_groups: dict[int, list[str]] = defaultdict(list) for nid, layer in layers.items(): layer_groups[layer].append(nid) + _order_layers(layer_groups, layers, adj, rev_adj) + offsets = _assign_cross_offsets(layer_groups, layers, adj, rev_adj) + horizontal = direction in ("LR", "RL") flip = direction in ("RL", "BT") for layer_idx in sorted(layer_groups.keys()): - group = layer_groups[layer_idx] - count = len(group) - for i, nid in enumerate(group): - cross_offset = i - (count - 1) / 2 - _position_node(nodes[nid], layer_idx, cross_offset, horizontal, flip) + for nid in layer_groups[layer_idx]: + _position_node(nodes[nid], layer_idx, offsets[nid], horizontal, flip) # --------------------------------------------------------------------------- @@ -234,27 +417,38 @@ def _parse_statement( parts = [p for p in _ARROW_SPLIT_RE.split(stmt) if p.strip()] if len(parts) < 3: if parts: - _parse_node_ref(parts[0], nodes) + _parse_node_group(parts[0], nodes) return idx = 0 while idx + 2 < len(parts): - src_id = _parse_node_ref(parts[idx], nodes) + src_ids = _parse_node_group(parts[idx], nodes) label, style, width = _parse_arrow(parts[idx + 1]) - tgt_id = _parse_node_ref(parts[idx + 2], nodes) - if src_id and tgt_id: - edges.append(_EdgeInfo( - source=src_id, target=tgt_id, - label=label, style=style, line_width=width, - )) + tgt_ids = _parse_node_group(parts[idx + 2], nodes) + # mermaid joins every source to every target ("A & B --> C & D"). + for src_id in src_ids: + for tgt_id in tgt_ids: + edges.append(_EdgeInfo( + source=src_id, target=tgt_id, + label=label, style=style, line_width=width, + )) idx += 2 -def _parse_direction(line: str) -> str | None: +def _parse_direction(line: str) -> tuple[str, str] | None: + """Return ``(direction, remainder)`` when *line* opens with a graph/flowchart + direction keyword, else ``None``. + + *remainder* is any ``;``-separated statement text that followed the keyword + on the same line (valid mermaid), so the caller can still parse it instead + of discarding the rest of the line. + """ match = _DIRECTION_RE.match(line) if match is None: return None direction = match.group(1).upper() - return "TD" if direction == "TB" else direction + direction = "TD" if direction == "TB" else direction + remainder = line[match.end():].lstrip(" ;").strip() + return direction, remainder def parse_mermaid(text: str) -> dict: @@ -280,10 +474,12 @@ def parse_mermaid(text: str) -> dict: line = _COMMENT_RE.sub("", raw_line).strip() if not line: continue - new_dir = _parse_direction(line) - if new_dir is not None: - direction = new_dir - continue + parsed_dir = _parse_direction(line) + if parsed_dir is not None: + direction, remainder = parsed_dir + if not remainder: + continue + line = remainder if _SKIP_RE.match(line): continue for stmt in line.split(";"): diff --git a/pybreeze/pybreeze_ui/diagram_editor/diagram_net_utils.py b/pybreeze/pybreeze_ui/diagram_editor/diagram_net_utils.py index 2150da1..26a5aac 100644 --- a/pybreeze/pybreeze_ui/diagram_editor/diagram_net_utils.py +++ b/pybreeze/pybreeze_ui/diagram_editor/diagram_net_utils.py @@ -8,7 +8,7 @@ """ from __future__ import annotations -from urllib.request import Request, urlopen +from urllib.request import HTTPRedirectHandler, Request, build_opener from pybreeze.utils.network.url_validation import UnsafeURLError, validate_url @@ -28,6 +28,49 @@ def _validate_url(url: str) -> str: raise ImageDownloadError(str(exc)) from exc +class _ValidatingRedirectHandler(HTTPRedirectHandler): + """Re-validate every redirect hop to prevent redirect-based SSRF. + + ``urlopen`` follows 3xx redirects by default; without this the initial URL + could be a public host that 302-redirects to an internal address (e.g. the + cloud metadata service). Each redirect target is run through the same SSRF + validation before the request is allowed to proceed. + """ + + def redirect_request(self, req, fp, code, msg, headers, newurl): + _validate_url(newurl) + return super().redirect_request(req, fp, code, msg, headers, newurl) + + +_OPENER = build_opener(_ValidatingRedirectHandler()) + + +def _parse_content_length(raw: str | None) -> int | None: + """Return a non-negative ``Content-Length``, or ``None`` if absent/malformed. + + A hostile or buggy server can send a non-numeric or negative header; the + real cap is still enforced by the bounded ``read`` below, so an unparseable + header is treated as "unknown" rather than crashing with ``ValueError``. + """ + if raw is None: + return None + try: + value = int(raw) + except ValueError: + return None + return value if value >= 0 else None + + +def _is_text_content_type(content_type: str) -> bool: + """True for a declared ``text/*`` response (e.g. an HTML error page). + + Only obvious text types are rejected so an image served as a generic binary + type (``application/octet-stream``) or with no header still passes through to + the pixmap validation. + """ + return content_type.split(";")[0].strip().lower().startswith("text/") + + def safe_download_image(url: str) -> bytes: """Download image data from *url* with security and size guards. @@ -36,17 +79,21 @@ def safe_download_image(url: str) -> bytes: url = _validate_url(url) req = Request(url, headers={"User-Agent": "PyBreeze-DiagramEditor/1.0"}) - resp = urlopen(req, timeout=TIMEOUT_SECONDS) # nosec B310 # noqa: S310 — URL validated above by _validate_url - - # Check Content-Length header if available - content_length = resp.headers.get("Content-Length") - if content_length is not None and int(content_length) > MAX_DOWNLOAD_BYTES: - raise ImageDownloadError( - f"Image too large ({int(content_length)} bytes, max {MAX_DOWNLOAD_BYTES})." - ) + with _OPENER.open(req, timeout=TIMEOUT_SECONDS) as resp: # nosec B310 # noqa: S310 — URL + redirects validated by _ValidatingRedirectHandler + content_type = resp.headers.get("Content-Type", "") + if _is_text_content_type(content_type): + raise ImageDownloadError( + f"Expected an image but the server returned '{content_type.strip()}'." + ) + declared_length = _parse_content_length(resp.headers.get("Content-Length")) + if declared_length is not None and declared_length > MAX_DOWNLOAD_BYTES: + raise ImageDownloadError( + f"Image too large ({declared_length} bytes, max {MAX_DOWNLOAD_BYTES})." + ) + # Read one byte past the cap so an undersized/absent header can't smuggle + # an oversized body past the check. + data = resp.read(MAX_DOWNLOAD_BYTES + 1) - # Read with size limit - data = resp.read(MAX_DOWNLOAD_BYTES + 1) if len(data) > MAX_DOWNLOAD_BYTES: raise ImageDownloadError( f"Image exceeds {MAX_DOWNLOAD_BYTES // (1024 * 1024)} MB limit." diff --git a/pybreeze/pybreeze_ui/diagram_editor/diagram_property_panel.py b/pybreeze/pybreeze_ui/diagram_editor/diagram_property_panel.py index 508f7b8..19a102a 100644 --- a/pybreeze/pybreeze_ui/diagram_editor/diagram_property_panel.py +++ b/pybreeze/pybreeze_ui/diagram_editor/diagram_property_panel.py @@ -248,20 +248,25 @@ def _on_selection_changed(self) -> None: self._current_conn = None self._current_img = None selected = self._scene.selectedItems() - nodes = [i for i in selected if isinstance(i, DiagramNode)] conns = [i for i in selected if isinstance(i, DiagramConnection)] imgs = [i for i in selected if isinstance(i, DiagramImage)] + self._show_for_selection(nodes, conns, imgs) + self._updating = False - if len(nodes) == 1 and len(conns) == 0 and len(imgs) == 0: + def _show_for_selection(self, nodes: list, conns: list, imgs: list) -> None: + # Tuple comparison instead of chained ``and`` keeps the panel showing a + # single node/connection/image (and nothing else categorised) while + # staying low-complexity. + counts = (len(nodes), len(conns), len(imgs)) + if counts == (1, 0, 0): self._show_node(nodes[0]) - elif len(conns) == 1 and len(nodes) == 0 and len(imgs) == 0: + elif counts == (0, 1, 0): self._show_connection(conns[0]) - elif len(imgs) == 1 and len(nodes) == 0 and len(conns) == 0: + elif counts == (0, 0, 1): self._show_image(imgs[0]) else: self._show_none() - self._updating = False def _show_none(self) -> None: self._no_sel_label.show() @@ -386,7 +391,7 @@ def _show_image(self, img: DiagramImage) -> None: self._img_caption.setText(img.text()) src = img.source() - self._img_source.setText(src if src else "") + self._img_source.setText(src or "") self._img_w.setValue(int(img.img_w)) self._img_h.setValue(int(img.img_h)) diff --git a/pybreeze/pybreeze_ui/diagram_editor/diagram_scene.py b/pybreeze/pybreeze_ui/diagram_editor/diagram_scene.py index 41f1de1..7a2ab3f 100644 --- a/pybreeze/pybreeze_ui/diagram_editor/diagram_scene.py +++ b/pybreeze/pybreeze_ui/diagram_editor/diagram_scene.py @@ -24,6 +24,13 @@ ) from pybreeze.utils.logging.logger import pybreeze_logger +# Allowlist of image extensions that a saved diagram may reference on disk. +# Defined once at module scope because it is a security boundary (only these +# local files are read back when reloading a ``.diagram.json``). +_VALID_IMAGE_SUFFIXES = frozenset( + {".png", ".jpg", ".jpeg", ".bmp", ".gif", ".svg", ".webp", ".ico"} +) + class ToolMode(Enum): """State pattern: each mode defines how mouse events behave on the canvas.""" @@ -84,7 +91,10 @@ def grid_enabled(self) -> bool: @grid_enabled.setter def grid_enabled(self, value: bool) -> None: self._grid_enabled = value + # Both item types snap independently, so keep their class flags in sync — + # otherwise images ignore the grid while nodes honour it. DiagramNode.grid_enabled = value + DiagramImage.grid_enabled = value @property def grid_size(self) -> int: @@ -94,6 +104,7 @@ def grid_size(self) -> int: def grid_size(self, value: int) -> None: self._grid_size = max(5, value) DiagramNode.grid_size = self._grid_size + DiagramImage.grid_size = self._grid_size # ------------------------------------------------------------------ # State management @@ -400,7 +411,7 @@ def paste_clipboard(self) -> None: label=cd.get("label", ""), line_color=cd.get("line_color"), line_width=cd.get("line_width", 2.0), - style=ConnectionStyle[cd.get("style", "SOLID")], + style=ConnectionStyle.__members__.get(cd.get("style", "SOLID"), ConnectionStyle.SOLID), ) self.addItem(conn) self.item_count_changed.emit() @@ -554,26 +565,53 @@ def _clear_items(self) -> None: self.removeItem(item) def _load_items(self, data: dict) -> None: - """Add items from serialised data.""" + """Add items from serialised data, skipping any malformed entry. + + A single corrupt node/connection/image must not abort the whole load + and lose the valid items alongside it (saved diagrams can be hand-edited + or partially written), so each entry is loaded defensively. + """ + id_to_node = self._load_nodes(data.get("nodes", [])) + self._load_connections(data.get("connections", []), id_to_node) + self._load_images(data.get("images", [])) + + def _load_nodes(self, node_dicts: list) -> dict[int, DiagramNode]: id_to_node: dict[int, DiagramNode] = {} - for nd in data.get("nodes", []): - node = DiagramNode.from_dict(nd) + for nd in node_dicts: + try: + node = DiagramNode.from_dict(nd) + except (KeyError, ValueError, TypeError) as err: + pybreeze_logger.debug("Skipping malformed diagram node %r: %s", nd, err) + continue self.addItem(node) - id_to_node[nd["id"]] = node - for cd in data.get("connections", []): - src = id_to_node.get(cd["source"]) - tgt = id_to_node.get(cd["target"]) - if src is not None and tgt is not None: - conn = DiagramConnection( - src, tgt, - label=cd.get("label", ""), - line_color=cd.get("line_color"), - line_width=cd.get("line_width", 2.0), - style=ConnectionStyle[cd.get("style", "SOLID")], - ) - self.addItem(conn) - for img_d in data.get("images", []): - img = DiagramImage.from_dict(img_d) + node_id = nd.get("id") + if node_id is not None: + id_to_node[node_id] = node + return id_to_node + + def _load_connections(self, conn_dicts: list, id_to_node: dict[int, DiagramNode]) -> None: + for cd in conn_dicts: + src = id_to_node.get(cd.get("source")) + tgt = id_to_node.get(cd.get("target")) + if src is None or tgt is None: + continue + style = ConnectionStyle.__members__.get(cd.get("style", "SOLID"), ConnectionStyle.SOLID) + conn = DiagramConnection( + src, tgt, + label=cd.get("label", ""), + line_color=cd.get("line_color"), + line_width=cd.get("line_width", 2.0), + style=style, + ) + self.addItem(conn) + + def _load_images(self, image_dicts: list) -> None: + for img_d in image_dicts: + try: + img = DiagramImage.from_dict(img_d) + except (KeyError, ValueError, TypeError) as err: + pybreeze_logger.debug("Skipping malformed diagram image %r: %s", img_d, err) + continue self.addItem(img) # Try to reload pixmap from source source = img_d.get("source", "") @@ -587,9 +625,8 @@ def _try_load_image_source(self, img: DiagramImage, source: str) -> None: URLs are validated and size-limited via ``safe_download_image``. """ path = Path(source) - # Only load if the file actually exists and has an image extension - _IMAGE_SUFFIXES = {".png", ".jpg", ".jpeg", ".bmp", ".gif", ".svg", ".webp", ".ico"} - if path.is_file() and path.suffix.lower() in _IMAGE_SUFFIXES: + # Only load if the file actually exists and has an allowlisted extension. + if path.is_file() and path.suffix.lower() in _VALID_IMAGE_SUFFIXES: pix = QPixmap(str(path)) if not pix.isNull(): img.set_pixmap(pix, source) diff --git a/pybreeze/pybreeze_ui/editor_main/file_tree_context_menu.py b/pybreeze/pybreeze_ui/editor_main/file_tree_context_menu.py index 5278024..a7aa7e5 100644 --- a/pybreeze/pybreeze_ui/editor_main/file_tree_context_menu.py +++ b/pybreeze/pybreeze_ui/editor_main/file_tree_context_menu.py @@ -4,6 +4,7 @@ import shutil import subprocess import sys +from collections.abc import Callable from pathlib import Path from PySide6.QtCore import Qt, QModelIndex @@ -15,6 +16,23 @@ from je_editor import language_wrapper from je_editor.pyside_ui.main_ui.editor.editor_widget import EditorWidget +from pybreeze.utils.logging.logger import pybreeze_logger + + +def _perform_file_op(tree_view: QTreeView, operation: Callable[[], None]) -> bool: + """Run a filesystem mutation, surfacing failures as a dialog, not a traceback. + + Returns ``True`` on success, ``False`` if the operation raised ``OSError``. + """ + word = language_wrapper.language_word_dict + try: + operation() + return True + except OSError as error: + pybreeze_logger.error("File tree operation failed: %r", error) + QMessageBox.warning(tree_view, word.get("file_tree_ctx_error"), str(error)) + return False + def setup_file_tree_context_menu(main_window) -> None: """ @@ -34,14 +52,18 @@ def patched_add_tab(*args, **kwargs): result = original_add_tab(*args, **kwargs) widget = args[0] if args else None if isinstance(widget, EditorWidget) and widget.project_treeview is not None: - if widget.project_treeview.contextMenuPolicy() != Qt.ContextMenuPolicy.CustomContextMenu: - _attach_context_menu(widget.project_treeview, main_window) + _attach_context_menu(widget.project_treeview, main_window) return result main_window.tab_widget.addTab = patched_add_tab def _attach_context_menu(tree_view: QTreeView, main_window) -> None: + # Idempotent: a tree view already switched to CustomContextMenu has our + # handler connected, so skip it — otherwise a repeated setup would connect + # the signal again and fire the menu multiple times per right-click. + if tree_view.contextMenuPolicy() == Qt.ContextMenuPolicy.CustomContextMenu: + return tree_view.setContextMenuPolicy(Qt.ContextMenuPolicy.CustomContextMenu) tree_view.customContextMenuRequested.connect( lambda pos, tv=tree_view, mw=main_window: _show_context_menu(pos, tv, mw) @@ -141,8 +163,11 @@ def _action_new_file(tree_view: QTreeView, path: Path | None) -> None: word.get("file_tree_ctx_already_exists").format(name=str(new_path)), ) return - new_path.parent.mkdir(parents=True, exist_ok=True) - new_path.touch() + def _create() -> None: + new_path.parent.mkdir(parents=True, exist_ok=True) + new_path.touch() + + _perform_file_op(tree_view, _create) def _action_new_folder(tree_view: QTreeView, path: Path | None) -> None: @@ -163,7 +188,7 @@ def _action_new_folder(tree_view: QTreeView, path: Path | None) -> None: word.get("file_tree_ctx_already_exists").format(name=str(new_path)), ) return - new_path.mkdir(parents=True) + _perform_file_op(tree_view, lambda: new_path.mkdir(parents=True)) def _find_editor_for_file(main_window, file_path: Path) -> EditorWidget | None: @@ -200,7 +225,8 @@ def _action_rename(tree_view: QTreeView, main_window, path: Path | None) -> None # If this file is currently open in an editor tab, update the tab editor = _find_editor_for_file(main_window, path) - path.rename(target) + if not _perform_file_op(tree_view, lambda: path.rename(target)): + return if editor is not None and target.is_file(): editor.current_file = str(target) editor.code_edit.current_file = str(target) @@ -228,10 +254,13 @@ def _action_delete(tree_view: QTreeView, main_window, path: Path | None) -> None editor.close() main_window.tab_widget.removeTab(idx) - if path.is_dir(): - shutil.rmtree(path) - else: - path.unlink() + def _delete() -> None: + if path.is_dir(): + shutil.rmtree(path) + else: + path.unlink() + + _perform_file_op(tree_view, _delete) def _action_copy_path(tree_view: QTreeView, path: Path | None, relative: bool = False) -> None: diff --git a/pybreeze/pybreeze_ui/extend_ai_gui/code_review/code_review_thread.py b/pybreeze/pybreeze_ui/extend_ai_gui/code_review/code_review_thread.py index e553f53..a8e48af 100644 --- a/pybreeze/pybreeze_ui/extend_ai_gui/code_review/code_review_thread.py +++ b/pybreeze/pybreeze_ui/extend_ai_gui/code_review/code_review_thread.py @@ -8,7 +8,11 @@ from pybreeze.pybreeze_ui.extend_ai_gui.ai_gui_global_variable import COT_TEMPLATE_RELATION from pybreeze.pybreeze_ui.extend_ai_gui.prompt_edit_gui.cot_code_review_prompt_templates.global_rule import \ build_global_rule_template -from pybreeze.utils.network.url_validation import validate_url +from pybreeze.utils.logging.logger import pybreeze_logger +from pybreeze.utils.network.http_client import ( + ResponseTooLargeError, read_capped_text, CONNECT_TIMEOUT, +) +from pybreeze.utils.network.url_validation import UnsafeURLError, validate_url class SenderThread(QThread): @@ -21,36 +25,53 @@ def __init__(self, files: list, code: str, url: str): self.url = url def run(self): - validate_url(self.url) - code = self.code + try: + validate_url(self.url) + except UnsafeURLError as error: + pybreeze_logger.error("CoT code review URL rejected: %r", error) + self.update_response.emit("error", str(error)) + return + # One session reuses a single TCP/TLS connection across all the + # sequential per-template POSTs to the same endpoint. + session = requests.Session() + try: + self._run_templates(session, self.code) + finally: + session.close() + + def _run_templates(self, session: requests.Session, code: str) -> None: first_code_review_result = None first_summary_result = None linter_result = None code_smell_result = None for file in self.files: + # Stop promptly if the widget is closing instead of firing off the + # remaining per-template POSTs. + if self.isInterruptionRequested(): + return match file: case "first_summary_prompt.md": - first_summary_prompt = COT_TEMPLATE_RELATION.get("first_summary_prompt.md") + first_summary_prompt = COT_TEMPLATE_RELATION["first_summary_prompt.md"] prompt = build_global_rule_template( prompt=first_summary_prompt.format(code_diff=code) ) case "first_code_review.md": - first_code_review_prompt = COT_TEMPLATE_RELATION.get("first_code_review.md") + first_code_review_prompt = COT_TEMPLATE_RELATION["first_code_review.md"] prompt = build_global_rule_template( prompt=first_code_review_prompt.format(code_diff=code) ) case "linter.md": - linter_prompt = COT_TEMPLATE_RELATION.get("linter.md") + linter_prompt = COT_TEMPLATE_RELATION["linter.md"] prompt = build_global_rule_template( prompt=linter_prompt.format(code_diff=code) ) case "code_smell_detector.md": - code_smell_detector_prompt = COT_TEMPLATE_RELATION.get("code_smell_detector.md") + code_smell_detector_prompt = COT_TEMPLATE_RELATION["code_smell_detector.md"] prompt = build_global_rule_template( prompt=code_smell_detector_prompt.format(code_diff=code) ) case "total_summary.md": - total_summary_prompt = COT_TEMPLATE_RELATION.get("total_summary.md") + total_summary_prompt = COT_TEMPLATE_RELATION["total_summary.md"] prompt = build_global_rule_template( prompt=total_summary_prompt.format( first_code_review=first_code_review_result, @@ -64,9 +85,12 @@ def run(self): continue try: - # 傳送到指定 URL - resp = requests.post(self.url, json={"prompt": prompt}, timeout=60, allow_redirects=False) - reply_text = resp.text + # 傳送到指定 URL（重用 session 連線） + resp = session.post( + self.url, json={"prompt": prompt}, + timeout=(CONNECT_TIMEOUT, 60), allow_redirects=False, stream=True, + ) + reply_text = read_capped_text(resp) match file: case "first_summary_prompt.md": first_summary_result = reply_text @@ -77,8 +101,12 @@ def run(self): case "code_smell_detector.md": code_smell_result = reply_text case _: - continue - except Exception as e: + # total_summary.md has no intermediate result to store but + # must still be emitted — `continue` here previously + # dropped the final summary before it reached the UI. + pass + except (requests.RequestException, ResponseTooLargeError) as e: + pybreeze_logger.error("CoT code review send failed for %s: %r", file, e) reply_text = f"{language_wrapper.language_word_dict.get('cot_gui_error_sending')} {file} {e}" # 發送訊號更新 UI diff --git a/pybreeze/pybreeze_ui/extend_ai_gui/code_review/cot_code_review_gui.py b/pybreeze/pybreeze_ui/extend_ai_gui/code_review/cot_code_review_gui.py index 9857fde..57d86b5 100644 --- a/pybreeze/pybreeze_ui/extend_ai_gui/code_review/cot_code_review_gui.py +++ b/pybreeze/pybreeze_ui/extend_ai_gui/code_review/cot_code_review_gui.py @@ -59,6 +59,7 @@ def __init__(self): # 儲存回覆 self.responses = {} + self.thread = None def show_response(self, filename): if filename in self.responses: @@ -76,9 +77,16 @@ def start_sending(self): QMessageBox.warning(self, "Warning", str(e)) return + # Ignore re-submits while a run is in flight so we never drop a running + # QThread or interleave two review passes into the same response store. + if self.thread is not None and self.thread.isRunning(): + return + # 啟動傳送 Thread + self.send_button.setEnabled(False) self.thread = SenderThread(files=self.files, code=self.code_paste_area.toPlainText(), url=url) self.thread.update_response.connect(self.handle_response) + self.thread.finished.connect(lambda: self.send_button.setEnabled(True)) self.thread.start() def handle_response(self, filename, response): @@ -88,3 +96,14 @@ def handle_response(self, filename, response): # 自動顯示最新回覆 self.response_selector.setCurrentText(filename) self.show_response(filename) + + def closeEvent(self, event): + thread = self.thread + if thread is not None and thread.isRunning(): + # Block slots so a late signal can't hit the dying widget, ask the + # worker to stop after its current request, then wait so the QThread + # is never destroyed while still running ("Destroyed while running"). + thread.blockSignals(True) + thread.requestInterruption() + thread.wait() + event.accept() diff --git a/pybreeze/pybreeze_ui/extend_ai_gui/prompt_edit_gui/cot_prompt_editor_widget.py b/pybreeze/pybreeze_ui/extend_ai_gui/prompt_edit_gui/cot_prompt_editor_widget.py index af3b832..84d79ce 100644 --- a/pybreeze/pybreeze_ui/extend_ai_gui/prompt_edit_gui/cot_prompt_editor_widget.py +++ b/pybreeze/pybreeze_ui/extend_ai_gui/prompt_edit_gui/cot_prompt_editor_widget.py @@ -11,6 +11,7 @@ from pybreeze.pybreeze_ui.extend_ai_gui.ai_gui_global_variable import COT_TEMPLATE_FILES, \ COT_TEMPLATE_RELATION +from pybreeze.pybreeze_ui.extend_ai_gui.prompt_edit_gui.prompt_file_io import save_prompt_text class CoTPromptEditor(QWidget): @@ -22,7 +23,7 @@ def __init__(self, prompt_files=None, parent=None): self.templates = COT_TEMPLATE_RELATION self.setWindowTitle(language_wrapper.language_word_dict.get( - "cot_cot_prompt_editor_window_title" + "cot_prompt_editor_window_title" )) # 視窗標題：Prompt 編輯器 # --- Layouts (版面配置) --- @@ -105,8 +106,11 @@ def create_file(self): return template_content = self.templates.get(filename, "") - with open(filename, "w", encoding="utf-8") as f: - f.write(template_content) + if not save_prompt_text( + self, filename, template_content, + language_wrapper.language_word_dict.get("cot_prompt_editor_msgbox_error_title"), + ): + return QMessageBox.information( self, @@ -125,12 +129,15 @@ def save_file(self): QMessageBox.warning( self, language_wrapper.language_word_dict.get("cot_prompt_editor_msgbox_error_title"), - language_wrapper.language_word_dict.get("cot_cot_prompt_editor_msgbox_no_file_selected")) + language_wrapper.language_word_dict.get("cot_prompt_editor_msgbox_no_file_selected")) return content = self.middle_editor.toPlainText() - with open(self.current_file, "w", encoding="utf-8") as f: - f.write(content) + if not save_prompt_text( + self, self.current_file, content, + language_wrapper.language_word_dict.get("cot_prompt_editor_msgbox_error_title"), + ): + return QMessageBox.information( self, language_wrapper.language_word_dict.get("cot_prompt_editor_msgbox_success_title"), diff --git a/pybreeze/pybreeze_ui/extend_ai_gui/prompt_edit_gui/prompt_file_io.py b/pybreeze/pybreeze_ui/extend_ai_gui/prompt_edit_gui/prompt_file_io.py new file mode 100644 index 0000000..9e1623a --- /dev/null +++ b/pybreeze/pybreeze_ui/extend_ai_gui/prompt_edit_gui/prompt_file_io.py @@ -0,0 +1,27 @@ +"""Shared file-write helper for the prompt template editors. + +The CoT and Skills prompt editors both write template text to disk on create and +save. Centralising the write keeps a single place that turns an I/O failure into +a user-facing dialog instead of an uncaught traceback. +""" +from __future__ import annotations + +from PySide6.QtWidgets import QMessageBox, QWidget + +from pybreeze.utils.logging.logger import pybreeze_logger + + +def save_prompt_text(parent: QWidget, path: str, content: str, error_title: str) -> bool: + """Write *content* to *path*; show a warning dialog on failure. + + Returns ``True`` on success, ``False`` if the write raised ``OSError`` (so the + caller can skip its success message). + """ + try: + with open(path, "w", encoding="utf-8") as file_handle: + file_handle.write(content) + return True + except OSError as error: + pybreeze_logger.error("Prompt file write failed for %s: %r", path, error) + QMessageBox.warning(parent, error_title, str(error)) + return False diff --git a/pybreeze/pybreeze_ui/extend_ai_gui/prompt_edit_gui/skills_prompt_editor_widget.py b/pybreeze/pybreeze_ui/extend_ai_gui/prompt_edit_gui/skills_prompt_editor_widget.py index 5d54914..d266995 100644 --- a/pybreeze/pybreeze_ui/extend_ai_gui/prompt_edit_gui/skills_prompt_editor_widget.py +++ b/pybreeze/pybreeze_ui/extend_ai_gui/prompt_edit_gui/skills_prompt_editor_widget.py @@ -11,6 +11,7 @@ from pybreeze.pybreeze_ui.extend_ai_gui.ai_gui_global_variable import SKILLS_TEMPLATE_FILES, \ SKILLS_TEMPLATE_RELATION +from pybreeze.pybreeze_ui.extend_ai_gui.prompt_edit_gui.prompt_file_io import save_prompt_text class SkillPromptEditor(QWidget): @@ -104,8 +105,11 @@ def create_file(self): return template_content = self.templates.get(filename, "") - with open(filename, "w", encoding="utf-8") as f: - f.write(template_content) + if not save_prompt_text( + self, filename, template_content, + language_wrapper.language_word_dict.get("skill_prompt_editor_msgbox_error_title"), + ): + return QMessageBox.information( self, @@ -129,8 +133,11 @@ def save_file(self): return content = self.middle_editor.toPlainText() - with open(self.current_file, "w", encoding="utf-8") as f: - f.write(content) + if not save_prompt_text( + self, self.current_file, content, + language_wrapper.language_word_dict.get("skill_prompt_editor_msgbox_error_title"), + ): + return QMessageBox.information( self, language_wrapper.language_word_dict.get("skill_prompt_editor_msgbox_success_title"), diff --git a/pybreeze/pybreeze_ui/extend_ai_gui/skills/skills_send_gui.py b/pybreeze/pybreeze_ui/extend_ai_gui/skills/skills_send_gui.py index fd9c81f..fc01c6a 100644 --- a/pybreeze/pybreeze_ui/extend_ai_gui/skills/skills_send_gui.py +++ b/pybreeze/pybreeze_ui/extend_ai_gui/skills/skills_send_gui.py @@ -9,7 +9,11 @@ from je_editor import language_wrapper from pybreeze.pybreeze_ui.extend_ai_gui.ai_gui_global_variable import SKILLS_TEMPLATE_FILES -from pybreeze.utils.network.url_validation import validate_url +from pybreeze.utils.logging.logger import pybreeze_logger +from pybreeze.utils.network.http_client import ( + ResponseTooLargeError, read_capped_text, CONNECT_TIMEOUT, truncate_for_display, +) +from pybreeze.utils.network.url_validation import UnsafeURLError, validate_url class RequestThread(QThread): @@ -24,16 +28,20 @@ def __init__(self, api_url, code_text): def run(self): try: validate_url(self.api_url) - response = requests.post(self.api_url, json={"code": self.code_text}, timeout=30, allow_redirects=False) + response = requests.post( + self.api_url, json={"code": self.code_text}, + timeout=(CONNECT_TIMEOUT, 30), allow_redirects=False, stream=True, + ) + body = read_capped_text(response) if response.ok: - self.finished.emit(response.text) + self.finished.emit(body) elif response.is_redirect: self.finished.emit( language_wrapper.language_word_dict.get( "skills_error_status").format( status_code=response.status_code, text=f"Redirect to {response.headers.get('Location', 'unknown')}")) - elif response.status_code == 401 or response.status_code == 403: + elif response.status_code in (401, 403): self.error.emit( language_wrapper.language_word_dict.get( "skills_error_status").format( @@ -44,12 +52,14 @@ def run(self): language_wrapper.language_word_dict.get( "skills_error_status").format( status_code=response.status_code, - text=f"Server error: {response.text}")) + text=f"Server error: {truncate_for_display(body)}")) else: self.finished.emit( language_wrapper.language_word_dict.get( - "skills_error_status").format(status_code=response.status_code, text=response.text)) - except Exception as e: + "skills_error_status").format( + status_code=response.status_code, text=truncate_for_display(body))) + except (requests.RequestException, ResponseTooLargeError, UnsafeURLError) as e: + pybreeze_logger.error("Skills send request failed: %r", e) self.error.emit(language_wrapper.language_word_dict.get("skills_exception").format(error=str(e))) @@ -96,6 +106,12 @@ def __init__(self): self.thread = None # 保存執行緒 def send_prompt(self): + # Ignore re-submits while a request is in flight: reassigning self.thread + # here would drop a still-running QThread (risking "destroyed while + # running") and let a stale worker overwrite the panel. + if self.thread is not None and self.thread.isRunning(): + return + api_url = self.api_url_input.text().strip() prompt_text = self.prompt_input.toPlainText().strip() @@ -107,6 +123,7 @@ def send_prompt(self): self.response_output.setPlainText(language_wrapper.language_word_dict.get("skills_generating")) # 啟動 QThread + self.send_button.setEnabled(False) self.thread = RequestThread(api_url, prompt_text) self.thread.finished.connect(self.on_finished) self.thread.error.connect(self.on_error) @@ -114,6 +131,17 @@ def send_prompt(self): def on_finished(self, result): self.response_output.setPlainText(result) + self.send_button.setEnabled(True) def on_error(self, error_msg): self.response_output.setPlainText(error_msg) + self.send_button.setEnabled(True) + + def closeEvent(self, event): + thread = self.thread + if thread is not None and thread.isRunning(): + # Block slots so a late finished/error emit can't hit the dying + # widget, then wait so the QThread is never destroyed while running. + thread.blockSignals(True) + thread.wait() + event.accept() diff --git a/pybreeze/pybreeze_ui/jupyter_lab_gui/jupyter_lab_thread.py b/pybreeze/pybreeze_ui/jupyter_lab_gui/jupyter_lab_thread.py index 9332da9..0241e5b 100644 --- a/pybreeze/pybreeze_ui/jupyter_lab_gui/jupyter_lab_thread.py +++ b/pybreeze/pybreeze_ui/jupyter_lab_gui/jupyter_lab_thread.py @@ -11,6 +11,7 @@ from je_editor import language_wrapper from pybreeze.utils.logging.logger import pybreeze_logger +from pybreeze.utils.subprocess_util import no_window_creationflags JUPYTER_STARTUP_TIMEOUT = 60 @@ -54,6 +55,7 @@ def is_jupyter_installed(python_exe: str) -> bool: [python_exe, "-m", "pip", "show", "jupyterlab"], capture_output=True, timeout=30, + creationflags=no_window_creationflags(), ) return result.returncode == 0 @@ -84,7 +86,8 @@ def run(self): "install", "jupyterlab", "-U" - ], capture_output=True, text=True, timeout=300) + ], capture_output=True, text=True, timeout=300, + creationflags=no_window_creationflags()) if result.returncode != 0: raise RuntimeError(result.stderr) @@ -95,47 +98,77 @@ def run(self): # Launch embedded JupyterLab. Server binds to localhost only (see # CLAUDE.md JupyterLab integration notes); shell=False. nosec B603. + # The bind address is pinned explicitly: with token/password empty, + # XSRF disabled and a wildcard origin, the loopback-only binding is + # the sole barrier, so we never rely on the jupyter default staying + # localhost. self.process = subprocess.Popen([ # nosec B603 # nosemgrep # noqa: S603 python_exe, "-m", "jupyterlab", "--no-browser", + "--ServerApp.ip=localhost", f"--ServerApp.port={port}", "--ServerApp.token=", "--ServerApp.password=", "--ServerApp.allow_origin=*", "--ServerApp.disable_check_xsrf=True", - ], stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) - - start_time = time.time() - - while True: - elapsed = time.time() - start_time - if elapsed > self.startup_timeout: - raise TimeoutError( - f"JupyterLab startup timeout ({self.startup_timeout}s)") - - self.status_update.emit( - f"{language_wrapper.language_word_dict.get('jupyterlab_loading')} " - f"({int(elapsed)}s / {self.startup_timeout}s)") - - try: - s = socket.create_connection(("localhost", port), timeout=0.5) - s.close() - break - except OSError: - time.sleep(0.2) + ], stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, + creationflags=no_window_creationflags()) + self._wait_until_ready(port) self.server_ready.emit(f"http://localhost:{port}/lab") except Exception: err = traceback.format_exc() + # Tear down a half-started server so a startup timeout doesn't leave an + # orphaned JupyterLab process running and holding the port. + self.stop() self.error_occurred.emit(err) pybreeze_logger.error(f"JupyterLab launch failed: {err}") + @staticmethod + def _port_open(port: int) -> bool: + try: + with socket.create_connection(("localhost", port), timeout=0.5): + return True + except OSError: + return False + + def _wait_until_ready(self, port: int) -> None: + """Block until the server accepts connections, or raise on failure.""" + process = self.process + if process is None: + raise RuntimeError("JupyterLab process was not started") + start_time = time.time() + while True: + elapsed = time.time() - start_time + if elapsed > self.startup_timeout: + raise TimeoutError( + f"JupyterLab startup timeout ({self.startup_timeout}s)") + + # Fail fast if the server died (port conflict, bad install, ...) + # instead of polling a dead port until the full timeout elapses. + if process.poll() is not None: + stderr_tail = "" + if process.stderr is not None: + # Bounded read: the process has exited so the pipe holds at + # most its buffer; cap explicitly and keep the tail message. + stderr_tail = process.stderr.read(65536)[-500:] + raise RuntimeError( + f"JupyterLab exited early (code {process.returncode}): {stderr_tail}") + + self.status_update.emit( + f"{language_wrapper.language_word_dict.get('jupyterlab_loading')} " + f"({int(elapsed)}s / {self.startup_timeout}s)") + + if self._port_open(port): + return + time.sleep(0.2) + def stop(self): if self.process is not None: try: self.process.terminate() - except OSError: - pass + except OSError as error: + pybreeze_logger.debug("JupyterLab terminate failed: %r", error) diff --git a/pybreeze/pybreeze_ui/jupyter_lab_gui/jupyter_lab_widget.py b/pybreeze/pybreeze_ui/jupyter_lab_gui/jupyter_lab_widget.py index 540d9e1..cadb171 100644 --- a/pybreeze/pybreeze_ui/jupyter_lab_gui/jupyter_lab_widget.py +++ b/pybreeze/pybreeze_ui/jupyter_lab_gui/jupyter_lab_widget.py @@ -32,7 +32,10 @@ def __init__(self): self.thread.start() def update_status(self, text): - self.status_label.setText(text) + # status_label is removed once the lab loads; a late status/error signal + # could still arrive, so guard against the None it leaves behind. + if self.status_label: + self.status_label.setText(text) def load_lab(self, url): if self.status_label: @@ -44,11 +47,15 @@ def load_lab(self, url): self.browser.show() def show_error(self, msg): - self.status_label.setText(language_wrapper.language_word_dict.get("jupyterlab_init_failed")) + if self.status_label: + self.status_label.setText(language_wrapper.language_word_dict.get("jupyterlab_init_failed")) pybreeze_logger.error(msg) def closeEvent(self, event): if self.thread.isRunning(): + # Block signals first so a late status/error emit can't reach a slot + # on the widget being torn down. + self.thread.blockSignals(True) self.thread.stop() self.thread.quit() self.thread.wait() diff --git a/pybreeze/pybreeze_ui/menu/automation_menu/test_pioneer_menu/build_test_pioneer_menu.py b/pybreeze/pybreeze_ui/menu/automation_menu/test_pioneer_menu/build_test_pioneer_menu.py index d458e6c..5bf7dbb 100644 --- a/pybreeze/pybreeze_ui/menu/automation_menu/test_pioneer_menu/build_test_pioneer_menu.py +++ b/pybreeze/pybreeze_ui/menu/automation_menu/test_pioneer_menu/build_test_pioneer_menu.py @@ -47,10 +47,11 @@ def set_test_pioneer_menu(ui_we_want_to_set: PyBreezeMainWindow): def check_file(ui_we_want_to_set: PyBreezeMainWindow): dialog = QFileDialog(ui_we_want_to_set) dialog.setNameFilter("Yaml (*.yml)") - file_tuple = dialog.getOpenFileName() + # getOpenFileName returns (filename, selected_filter); the tuple itself is + # always truthy, so check the filename — an empty one means the user cancelled. + file_path = dialog.getOpenFileName()[0] show_messagebox = False - if file_tuple: - file_path = file_tuple[0] + if file_path: if Path(file_path).is_file() and Path(file_path).suffix == ".yml": init_and_start_test_pioneer_process(ui_we_want_to_set, file_path) else: @@ -59,4 +60,4 @@ def check_file(ui_we_want_to_set: PyBreezeMainWindow): messagebox = QMessageBox(ui_we_want_to_set) messagebox.setWindowTitle(language_wrapper.language_word_dict.get("test_pioneer_not_choose_yaml")) messagebox.setText(language_wrapper.language_word_dict.get("test_pioneer_not_choose_yaml")) - messagebox.exec_() + messagebox.exec() diff --git a/pybreeze/pybreeze_ui/menu/plugin_menu/build_plugin_menu.py b/pybreeze/pybreeze_ui/menu/plugin_menu/build_plugin_menu.py index 9b22217..2ef6581 100644 --- a/pybreeze/pybreeze_ui/menu/plugin_menu/build_plugin_menu.py +++ b/pybreeze/pybreeze_ui/menu/plugin_menu/build_plugin_menu.py @@ -83,17 +83,16 @@ def _add_plugin_entry(ui_we_want_to_set: PyBreezeMainWindow, meta: dict) -> None # 多種副檔名：每個副檔名一個執行動作 # Multiple suffixes: one run action per suffix for suffix in suffixes: - _add_run_action(ui_we_want_to_set, sub_menu, run_config, suffix, + _add_run_action(ui_we_want_to_set, sub_menu, run_config, label_name=f"{config_name} ({suffix})") else: # 單一副檔名：一個執行動作 # Single suffix: one run action - suffix = suffixes[0] if suffixes else "" - _add_run_action(ui_we_want_to_set, sub_menu, run_config, suffix, label_name=config_name) + _add_run_action(ui_we_want_to_set, sub_menu, run_config, label_name=config_name) def _add_run_action(ui_we_want_to_set: PyBreezeMainWindow, parent_menu, - run_config: dict, suffix: str, label_name: str) -> None: + run_config: dict, label_name: str) -> None: run_action = QAction( language_wrapper.language_word_dict.get( "plugin_menu_run_with", "Run with {name}" @@ -101,7 +100,7 @@ def _add_run_action(ui_we_want_to_set: PyBreezeMainWindow, parent_menu, parent_menu, ) run_action.triggered.connect( - _make_run_callback(ui_we_want_to_set, run_config, suffix) + _make_run_callback(ui_we_want_to_set, run_config) ) parent_menu.addAction(run_action) @@ -135,7 +134,7 @@ def callback(): return callback -def _make_run_callback(ui_we_want_to_set: PyBreezeMainWindow, run_config: dict, suffix: str): +def _make_run_callback(ui_we_want_to_set: PyBreezeMainWindow, run_config: dict): """ 建立使用插件執行設定來執行程式的回呼函式。 Create a callback to run a program using plugin run config. @@ -156,6 +155,10 @@ def callback(): return file_path = widget.current_file + # The save dialog can be accepted without a path being set. + if not file_path: + return + # 檢查副檔名是否匹配 / Check suffix match file_suffix = Path(file_path).suffix.lower() supported = run_config.get("suffixes", ()) diff --git a/pybreeze/pybreeze_ui/show_code_window/code_window.py b/pybreeze/pybreeze_ui/show_code_window/code_window.py index 439fca6..e1a5657 100644 --- a/pybreeze/pybreeze_ui/show_code_window/code_window.py +++ b/pybreeze/pybreeze_ui/show_code_window/code_window.py @@ -3,6 +3,11 @@ from PySide6.QtGui import QGuiApplication from PySide6.QtWidgets import QWidget, QGridLayout, QTextEdit, QScrollArea +# Cap the output scrollback so a runaway script (e.g. an infinite print loop) +# cannot grow the document without bound and exhaust memory; the oldest lines +# are dropped once the limit is reached, like a terminal's scrollback buffer. +MAX_OUTPUT_BLOCKS = 10000 + class CodeWindow(QWidget): @@ -14,6 +19,7 @@ def __init__(self): self.code_result = QTextEdit() self.code_result.setLineWrapMode(self.code_result.LineWrapMode.NoWrap) self.code_result.setReadOnly(True) + self.code_result.document().setMaximumBlockCount(MAX_OUTPUT_BLOCKS) self.code_result_scroll_area = QScrollArea() self.code_result_scroll_area.setWidgetResizable(True) self.code_result_scroll_area.setViewportMargins(0, 0, 0, 0) diff --git a/pybreeze/pybreeze_ui/syntax/syntax_extend.py b/pybreeze/pybreeze_ui/syntax/syntax_extend.py index e96eba4..f18c1ba 100644 --- a/pybreeze/pybreeze_ui/syntax/syntax_extend.py +++ b/pybreeze/pybreeze_ui/syntax/syntax_extend.py @@ -17,8 +17,10 @@ def syntax_extend_package(main_window: PyBreezeMainWindow) -> None: # Register JSON syntax keywords for each automation package json_syntax_words = {} for package in package_manager.syntax_check_list: + # Default to an empty list so a package without a keyword list registers + # no words instead of crashing syntax setup with set(None). json_syntax_words[package] = { - "words": set(package_keyword_list.get(package)), + "words": set(package_keyword_list.get(package, [])), "color": QColor(255, 255, 0), } register_programming_language(".json", json_syntax_words) @@ -26,7 +28,7 @@ def syntax_extend_package(main_window: PyBreezeMainWindow) -> None: # Register YAML syntax keywords for test_pioneer yml_syntax_words = { "test_pioneer": { - "words": set(package_keyword_list.get("test_pioneer")), + "words": set(package_keyword_list.get("test_pioneer", [])), "color": QColor(255, 153, 0), } } diff --git a/pybreeze/pybreeze_ui/syntax/syntax_keyword.py b/pybreeze/pybreeze_ui/syntax/syntax_keyword.py index 50ed810..48bacc5 100644 --- a/pybreeze/pybreeze_ui/syntax/syntax_keyword.py +++ b/pybreeze/pybreeze_ui/syntax/syntax_keyword.py @@ -113,7 +113,7 @@ "AutoControlScreenException", "ImageNotFoundException", "AutoControlJsonActionException", "AutoControlRecordException", "AutoControlActionNullException", "AutoControlActionException", "record", "stop_record", "read_action_json", "write_action_json", "execute_action", "execute_files", "executor", - "add_command_to_executor", "multiprocess_timeout", "test_record_instance", "screenshot", "pil_screenshot", + "add_command_to_executor", "multiprocess_timeout", "test_record_instance", "pil_screenshot", "generate_html", "generate_html_report", "generate_json", "generate_json_report", "generate_xml", "generate_xml_report", "get_dir_files_as_list", "create_project_dir", "start_autocontrol_socket_server", "callback_executor", "package_manager", "get_special_table", "ShellManager", "default_shell_manager", diff --git a/pybreeze/utils/app_dirs.py b/pybreeze/utils/app_dirs.py new file mode 100644 index 0000000..3a2d02f --- /dev/null +++ b/pybreeze/utils/app_dirs.py @@ -0,0 +1,18 @@ +"""Per-user PyBreeze application directories.""" +from __future__ import annotations + +from pathlib import Path + +_DATA_DIR_NAME = ".pybreeze" + + +def pybreeze_data_dir() -> Path: + """Return the user-level PyBreeze data directory, creating it if needed. + + A single home-based location (``~/.pybreeze``) keeps persisted data — SSH + known hosts, AI-review stats — stable regardless of the directory the IDE + was launched from. + """ + data_dir = Path.home() / _DATA_DIR_NAME + data_dir.mkdir(parents=True, exist_ok=True) + return data_dir diff --git a/pybreeze/utils/file_process/get_dir_file_list.py b/pybreeze/utils/file_process/get_dir_file_list.py index f83e162..cecaad1 100644 --- a/pybreeze/utils/file_process/get_dir_file_list.py +++ b/pybreeze/utils/file_process/get_dir_file_list.py @@ -10,17 +10,22 @@ from pybreeze.utils.logging.logger import pybreeze_logger -def get_dir_files_as_list(dir_path: str = getcwd(), default_search_file_extension: str = ".json") -> list: +def get_dir_files_as_list(dir_path: str | None = None, default_search_file_extension: str = ".json") -> list: """ get dir file when end with default_search_file_extension - :param dir_path: which dir we want to walk and get file list + :param dir_path: which dir we want to walk and get file list; defaults to the + current working directory at call time when ``None`` :param default_search_file_extension: which extension we want to search + (matched case-insensitively) :return: [] if nothing searched or [file1, file2.... files] file was searched """ + if dir_path is None: + dir_path = getcwd() + extension = default_search_file_extension.lower() return [ - abspath(join(root, file)) for root, dirs, files in walk(dir_path) + abspath(join(root, file)) for root, _dirs, files in walk(dir_path) for file in files - if file.endswith(default_search_file_extension.lower()) + if file.lower().endswith(extension) ] diff --git a/pybreeze/utils/json_format/json_process.py b/pybreeze/utils/json_format/json_process.py index 3cd8351..39fc416 100644 --- a/pybreeze/utils/json_format/json_process.py +++ b/pybreeze/utils/json_format/json_process.py @@ -10,12 +10,14 @@ from pybreeze.utils.logging.logger import pybreeze_logger -def __process_json(json_string: str, **kwargs) -> str: +def _process_json(json_string: str, **kwargs) -> str: try: return dumps(loads(json_string), indent=4, sort_keys=True, **kwargs) except json.JSONDecodeError as error: + # Wrap in the project exception so reformat_json's caller sees a single, + # documented ITEJsonException type rather than a raw JSONDecodeError. pybreeze_logger.error(wrong_json_data_error) - raise error + raise ITEJsonException(wrong_json_data_error) from error except TypeError: try: return dumps(json_string, indent=4, sort_keys=True, **kwargs) @@ -25,6 +27,6 @@ def __process_json(json_string: str, **kwargs) -> str: def reformat_json(json_string: str, **kwargs) -> str: try: - return __process_json(json_string, **kwargs) + return _process_json(json_string, **kwargs) except ITEJsonException as err: raise ITEJsonException(cant_reformat_json_error) from err diff --git a/pybreeze/utils/logging/logger.py b/pybreeze/utils/logging/logger.py index 15171aa..416da75 100644 --- a/pybreeze/utils/logging/logger.py +++ b/pybreeze/utils/logging/logger.py @@ -4,8 +4,10 @@ import os from logging.handlers import RotatingFileHandler -# 設定 root logger 等級 Set root logger level -logging.root.setLevel(logging.DEBUG) +# A library must not reconfigure the root logger: forcing it to DEBUG makes +# every third-party logger verbose and robs the host application of control +# over log levels. PyBreeze logs only through its own named logger below, which +# carries its own DEBUG level and file handler. # 建立 AutoControlGUI 專用 logger Create dedicated logger pybreeze_logger = logging.getLogger("Pybreeze") diff --git a/pybreeze/utils/network/http_client.py b/pybreeze/utils/network/http_client.py new file mode 100644 index 0000000..806cd38 --- /dev/null +++ b/pybreeze/utils/network/http_client.py @@ -0,0 +1,63 @@ +"""Bounded HTTP response reading helpers. + +User-configured endpoints (AI code-review services, skill webhooks) are only +semi-trusted: a hostile or buggy server can return a multi-gigabyte body that +``requests`` would buffer entirely into memory. ``read_capped_text`` streams the +body and aborts once it crosses a size cap, and ``truncate_for_display`` keeps an +oversized body from being pasted whole into an error dialog. +""" +from __future__ import annotations + +import requests + +DEFAULT_MAX_RESPONSE_BYTES = 16 * 1024 * 1024 # 16 MB +DISPLAY_TRUNCATE_CHARS = 2000 +_CHUNK_SIZE = 65536 + +# Seconds to wait for the TCP/TLS connection to establish. Use it as a literal +# tuple ``timeout=(CONNECT_TIMEOUT, read)`` at call sites so an unreachable +# endpoint fails fast — and so Bandit's B113 recognises the timeout (it does not +# follow a helper function, only literal/constant values). +CONNECT_TIMEOUT = 10 + + +class ResponseTooLargeError(Exception): + """Raised when a streamed response body exceeds the configured cap.""" + + +def read_capped_text( + response: requests.Response, + *, + max_bytes: int = DEFAULT_MAX_RESPONSE_BYTES, + default_encoding: str = "utf-8", +) -> str: + """Read a streamed response body up to *max_bytes* and decode it to text. + + The request must be issued with ``stream=True`` so the body is read here + under the cap instead of being buffered in full by ``requests``. Raises + ``ResponseTooLargeError`` once the accumulated body exceeds *max_bytes*. The + response is always closed before returning. + """ + total = 0 + chunks: list[bytes] = [] + try: + for chunk in response.iter_content(chunk_size=_CHUNK_SIZE): + if not chunk: + continue + total += len(chunk) + if total > max_bytes: + raise ResponseTooLargeError( + f"Response body exceeds the {max_bytes}-byte limit." + ) + chunks.append(chunk) + finally: + response.close() + encoding = response.encoding or default_encoding + return b"".join(chunks).decode(encoding, "replace") + + +def truncate_for_display(text: str, limit: int = DISPLAY_TRUNCATE_CHARS) -> str: + """Shorten *text* for safe embedding in an error message / dialog.""" + if len(text) <= limit: + return text + return f"{text[:limit]}… [truncated, {len(text)} chars total]" diff --git a/pybreeze/utils/network/url_validation.py b/pybreeze/utils/network/url_validation.py index 1445785..bf91784 100644 --- a/pybreeze/utils/network/url_validation.py +++ b/pybreeze/utils/network/url_validation.py @@ -6,18 +6,66 @@ _ALLOWED_SCHEMES = frozenset({"http", "https"}) +# RFC 6598 shared address space (Carrier-Grade NAT). Not covered by +# ``is_private`` / ``is_reserved`` yet routinely abused for SSRF in cloud +# environments, so it is blocked explicitly. +_CGNAT_NETWORK = ipaddress.ip_network("100.64.0.0/10") + +# RFC 6052 NAT64 well-known prefix. The trailing 32 bits embed an IPv4 target +# that a NAT64 gateway routes to, so it must be decoded and re-checked. +_NAT64_NETWORK = ipaddress.ip_network("64:ff9b::/96") + class UnsafeURLError(Exception): """Raised when a URL fails security validation.""" +def _embedded_ipv4(ip: ipaddress.IPv6Address) -> list[ipaddress.IPv4Address]: + """Return any IPv4 addresses tunnelled inside an IPv6 transition form. + + Dual-stack and translated networks route these wrappers to the underlying + IPv4 endpoint, so an attacker can reach a blocked IPv4 (e.g. the cloud + metadata service) by wrapping it as IPv4-mapped, 6to4, Teredo or NAT64. + Each embedded address is returned so the caller can re-validate it. + """ + embedded: list[ipaddress.IPv4Address] = [] + for candidate in (ip.ipv4_mapped, ip.sixtofour): + if candidate is not None: + embedded.append(candidate) + teredo = ip.teredo + if teredo is not None: + embedded.append(teredo[1]) + if ip in _NAT64_NETWORK: + embedded.append(ipaddress.ip_address(int(ip) & 0xFFFFFFFF)) + return embedded + + +def _is_blocked_ip(ip: ipaddress.IPv4Address | ipaddress.IPv6Address) -> bool: + """Return ``True`` when *ip* (or an IPv4 it tunnels to) is non-public.""" + if ( + ip.is_private + or ip.is_loopback + or ip.is_link_local + or ip.is_reserved + or ip.is_multicast + or ip.is_unspecified + ): + return True + if isinstance(ip, ipaddress.IPv4Address): + return ip in _CGNAT_NETWORK + return any(_is_blocked_ip(embedded) for embedded in _embedded_ipv4(ip)) + + def validate_url(url: str) -> str: """Validate a user-supplied URL against SSRF rules. Checks: 1. Scheme must be ``http`` or ``https`` 2. Hostname must be present - 3. Resolved IP must not be private, loopback, link-local or reserved + 3. Every resolved IP must be a public address. Private, loopback, + link-local, reserved, multicast, unspecified and Carrier-Grade NAT + ranges are blocked, as are IPv6 transition forms (IPv4-mapped, 6to4, + Teredo, NAT64) that tunnel to a blocked IPv4 endpoint. Returns the original *url* on success; raises ``UnsafeURLError`` on failure. """ @@ -37,11 +85,11 @@ def validate_url(url: str) -> str: except socket.gaierror as exc: raise UnsafeURLError(f"Cannot resolve hostname '{hostname}': {exc}") from exc - for _family, _type, _proto, _canonname, sockaddr in infos: + for *_unused, sockaddr in infos: ip = ipaddress.ip_address(sockaddr[0]) - if ip.is_private or ip.is_loopback or ip.is_link_local or ip.is_reserved: + if _is_blocked_ip(ip): raise UnsafeURLError( - f"Access to private/internal address {ip} is blocked." + f"Access to non-public address {ip} is blocked." ) return url diff --git a/pybreeze/utils/subprocess_util.py b/pybreeze/utils/subprocess_util.py new file mode 100644 index 0000000..d8e1068 --- /dev/null +++ b/pybreeze/utils/subprocess_util.py @@ -0,0 +1,33 @@ +"""Helpers for spawning child processes cleanly across platforms.""" +from __future__ import annotations + +import os +import subprocess +import sys + + +def utf8_subprocess_env(encoding: str = "utf-8") -> dict[str, str]: + """Return a copy of ``os.environ`` forcing a child Python's stdio *encoding*. + + On Windows a child's piped stdout defaults to the console code page (e.g. + cp950 / cp1252), so non-ASCII output would be mis-decoded by the utf-8 reader + in the process managers and show up garbled. Pinning ``PYTHONIOENCODING`` + makes the child emit the same encoding the manager decodes with. + """ + env = os.environ.copy() + env["PYTHONIOENCODING"] = encoding + return env + + +def no_window_creationflags() -> int: + """Return ``creationflags`` that suppress a console window on Windows. + + PyBreeze is a GUI application. When it is launched without an attached + console (packaged ``.exe`` or ``pythonw``), every console child it spawns + (``python -m ``, ``pip``, a compiler) pops a transient console + window. ``CREATE_NO_WINDOW`` prevents that; the child's piped stdout/stderr + are unaffected. Returns ``0`` (a no-op flag) on non-Windows platforms. + """ + if sys.platform == "win32": + return subprocess.CREATE_NO_WINDOW + return 0 diff --git a/test/test_utils/test_ai_gui_lifecycle.py b/test/test_utils/test_ai_gui_lifecycle.py new file mode 100644 index 0000000..bd22554 --- /dev/null +++ b/test/test_utils/test_ai_gui_lifecycle.py @@ -0,0 +1,74 @@ +"""closeEvent must never leave a running worker QThread to be destroyed mid-run.""" +from __future__ import annotations + +import os + +os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") + +from unittest.mock import MagicMock + +from pybreeze.pybreeze_ui.extend_ai_gui.code_review.cot_code_review_gui import CoTCodeReviewGUI +from pybreeze.pybreeze_ui.extend_ai_gui.skills.skills_send_gui import SkillsSendGUI + + +def _running_thread(): + thread = MagicMock() + thread.isRunning.return_value = True + return thread + + +class TestCoTCloseEvent: + def test_running_thread_is_blocked_interrupted_and_awaited(self): + gui = CoTCodeReviewGUI.__new__(CoTCodeReviewGUI) + gui.thread = _running_thread() + event = MagicMock() + + CoTCodeReviewGUI.closeEvent(gui, event) + + gui.thread.blockSignals.assert_called_once_with(True) + gui.thread.requestInterruption.assert_called_once() + gui.thread.wait.assert_called_once() + event.accept.assert_called_once() + + def test_no_thread_just_accepts(self): + gui = CoTCodeReviewGUI.__new__(CoTCodeReviewGUI) + gui.thread = None + event = MagicMock() + + CoTCodeReviewGUI.closeEvent(gui, event) + + event.accept.assert_called_once() + + def test_finished_thread_is_not_awaited(self): + gui = CoTCodeReviewGUI.__new__(CoTCodeReviewGUI) + thread = MagicMock() + thread.isRunning.return_value = False + gui.thread = thread + event = MagicMock() + + CoTCodeReviewGUI.closeEvent(gui, event) + + thread.wait.assert_not_called() + event.accept.assert_called_once() + + +class TestSkillsCloseEvent: + def test_running_thread_is_blocked_and_awaited(self): + gui = SkillsSendGUI.__new__(SkillsSendGUI) + gui.thread = _running_thread() + event = MagicMock() + + SkillsSendGUI.closeEvent(gui, event) + + gui.thread.blockSignals.assert_called_once_with(True) + gui.thread.wait.assert_called_once() + event.accept.assert_called_once() + + def test_no_thread_just_accepts(self): + gui = SkillsSendGUI.__new__(SkillsSendGUI) + gui.thread = None + event = MagicMock() + + SkillsSendGUI.closeEvent(gui, event) + + event.accept.assert_called_once() diff --git a/test/test_utils/test_ansi_escape.py b/test/test_utils/test_ansi_escape.py new file mode 100644 index 0000000..de8d090 --- /dev/null +++ b/test/test_utils/test_ansi_escape.py @@ -0,0 +1,38 @@ +from __future__ import annotations + +import pytest + +from pybreeze.pybreeze_ui.connect_gui.ssh.ssh_command_widget import ANSI_ESCAPE_PATTERN + + +def _strip(text: str) -> str: + return ANSI_ESCAPE_PATTERN.sub("", text) + + +class TestAnsiEscapeStripping: + @pytest.mark.parametrize("raw,expected", [ + ("\x1b[31mred\x1b[0m", "red"), # SGR colour + ("\x1b[2J\x1b[H clear", " clear"), # cursor / erase + ("\x1b[1mbold\x1b[22m", "bold"), # bold on/off + ("a\x1bMb", "ab"), # two-char C1 (reverse index) + ("no escapes here", "no escapes here"), # plain text untouched + ]) + def test_csi_and_fe(self, raw, expected): + assert _strip(raw) == expected + + @pytest.mark.parametrize("raw,expected", [ + # OSC terminal-title sequence (BEL terminated) — common in shell prompts. + ("\x1b]0;user@host:~\x07ls output", "ls output"), + # OSC hyperlink (ST terminated, ESC backslash). + ("\x1b]8;;http://example.com\x1b\\link\x1b]8;;\x1b\\ done", "link done"), + # OSC mixed with colour. + ("\x1b]0;title\x07\x1b[32mok\x1b[0m", "ok"), + # Unterminated OSC ended by the next ESC (a CSI), not BEL/ST: the body + # used to leak as "8;;https://..." text; it must be consumed whole. + ("\x1b]8;;https://api.github.com/repos\x1b[0mtext", "text"), + # Unterminated OSC ended by end-of-stream. + ("before\x1b]0;unterminated title", "before"), + ]) + def test_osc_sequences_are_stripped(self, raw, expected): + # Regression: OSC bodies (e.g. "0;title") used to leak through as garbage. + assert _strip(raw) == expected diff --git a/test/test_utils/test_app_dirs.py b/test/test_utils/test_app_dirs.py new file mode 100644 index 0000000..c5f66cb --- /dev/null +++ b/test/test_utils/test_app_dirs.py @@ -0,0 +1,20 @@ +from __future__ import annotations + +from pathlib import Path + +from pybreeze.utils.app_dirs import pybreeze_data_dir + + +class TestPybreezeDataDir: + def test_under_home_and_created(self, tmp_path, monkeypatch): + monkeypatch.setattr(Path, "home", classmethod(lambda cls: tmp_path)) + result = pybreeze_data_dir() + assert result == tmp_path / ".pybreeze" + assert result.is_dir() + + def test_idempotent_when_already_exists(self, tmp_path, monkeypatch): + monkeypatch.setattr(Path, "home", classmethod(lambda cls: tmp_path)) + first = pybreeze_data_dir() + second = pybreeze_data_dir() + assert first == second + assert second.is_dir() diff --git a/test/test_utils/test_code_window.py b/test/test_utils/test_code_window.py new file mode 100644 index 0000000..fca1639 --- /dev/null +++ b/test/test_utils/test_code_window.py @@ -0,0 +1,45 @@ +from __future__ import annotations + +import os + +os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") + +import pytest + + +@pytest.fixture(scope="module") +def qt_app(): + from PySide6.QtWidgets import QApplication + app = QApplication.instance() + if app is None: + try: + app = QApplication([]) + except Exception as exc: # pragma: no cover - no usable Qt platform + pytest.skip(f"Cannot start QApplication: {exc}") + return app + + +class TestCodeWindowScrollbackCap: + def test_block_count_is_capped(self, qt_app): + from pybreeze.pybreeze_ui.show_code_window.code_window import ( + MAX_OUTPUT_BLOCKS, + CodeWindow, + ) + + window = CodeWindow() + assert window.code_result.document().maximumBlockCount() == MAX_OUTPUT_BLOCKS + + def test_old_lines_dropped_once_capped(self, qt_app): + from pybreeze.pybreeze_ui.show_code_window.code_window import CodeWindow + + window = CodeWindow() + window.code_result.document().setMaximumBlockCount(50) + cursor = window.code_result.textCursor() + for i in range(500): + cursor.insertText(f"line {i}") + cursor.insertBlock() + + assert window.code_result.document().blockCount() <= 50 + text = window.code_result.toPlainText() + assert "line 499" in text # newest kept + assert "line 0\n" not in text # oldest dropped diff --git a/test/test_utils/test_content_length_parsing.py b/test/test_utils/test_content_length_parsing.py new file mode 100644 index 0000000..d9d12fa --- /dev/null +++ b/test/test_utils/test_content_length_parsing.py @@ -0,0 +1,49 @@ +from __future__ import annotations + +import pytest + +from pybreeze.pybreeze_ui.diagram_editor.diagram_net_utils import ( + _is_text_content_type, + _parse_content_length, +) + + +class TestIsTextContentType: + @pytest.mark.parametrize("content_type", [ + "text/html", + "text/html; charset=utf-8", + "text/plain", + "TEXT/HTML", + ]) + def test_text_types_rejected(self, content_type): + assert _is_text_content_type(content_type) is True + + @pytest.mark.parametrize("content_type", [ + "image/png", + "image/jpeg", + "application/octet-stream", + "", + ]) + def test_non_text_types_allowed(self, content_type): + assert _is_text_content_type(content_type) is False + + +class TestParseContentLength: + @pytest.mark.parametrize("raw,expected", [ + ("0", 0), + ("1024", 1024), + ("20971520", 20 * 1024 * 1024), + ]) + def test_valid_values(self, raw, expected): + assert _parse_content_length(raw) == expected + + @pytest.mark.parametrize("raw", [ + None, # header absent + "abc", # non-numeric + "123abc", # trailing junk + "", # empty + "-5", # negative + "12.5", # float-like + ]) + def test_absent_or_malformed_returns_none(self, raw): + assert _parse_content_length(raw) is None diff --git a/test/test_utils/test_cot_session_reuse.py b/test/test_utils/test_cot_session_reuse.py new file mode 100644 index 0000000..d05d607 --- /dev/null +++ b/test/test_utils/test_cot_session_reuse.py @@ -0,0 +1,88 @@ +from __future__ import annotations + +import os + +os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") + + +class _FakeResponse: + def __init__(self, body: bytes): + self._body = body + self.encoding = "utf-8" + self.closed = False + + def iter_content(self, chunk_size: int = 65536): + yield self._body + + def close(self): + self.closed = True + + +class _FakeSession: + """Records every post call so we can assert one session served them all.""" + + def __init__(self): + self.post_calls = [] + self.closed = False + + def post(self, url, **kwargs): + self.post_calls.append((url, kwargs)) + return _FakeResponse(b"reply for prompt") + + def close(self): + self.closed = True + + +def _qt_app(): + from PySide6.QtWidgets import QApplication + return QApplication.instance() or QApplication([]) + + +def test_one_session_serves_all_templates(): + _qt_app() + from pybreeze.pybreeze_ui.extend_ai_gui.code_review.code_review_thread import SenderThread + from pybreeze.pybreeze_ui.extend_ai_gui.ai_gui_global_variable import COT_TEMPLATE_FILES + + thread = SenderThread(files=list(COT_TEMPLATE_FILES), code="print('x')", url="https://example.com/api") + received = [] + thread.update_response.connect(lambda name, resp: received.append((name, resp))) + + session = _FakeSession() + thread._run_templates(session, "print('x')") + + # Every template posts through the SAME session (connection reuse) ... + assert len(session.post_calls) == len(COT_TEMPLATE_FILES) + # ... always to the configured URL ... + assert all(url == "https://example.com/api" for url, _ in session.post_calls) + # ... with streaming + no redirects preserved. + assert all(kw.get("stream") is True and kw.get("allow_redirects") is False + for _, kw in session.post_calls) + # ... and each produced a UI response — including the final total summary, + # which a `case _: continue` used to drop before it reached the UI. + assert len(received) == len(COT_TEMPLATE_FILES) + assert "total_summary.md" in {name for name, _ in received} + + +def test_run_closes_session_even_on_error(monkeypatch): + _qt_app() + from pybreeze.pybreeze_ui.extend_ai_gui.code_review import code_review_thread as mod + from pybreeze.pybreeze_ui.extend_ai_gui.code_review.code_review_thread import SenderThread + + created = {} + + class _TrackedSession(_FakeSession): + def __init__(self): + super().__init__() + created["session"] = self + + monkeypatch.setattr(mod.requests, "Session", _TrackedSession) + # Make the work raise to prove the finally still closes the session. + monkeypatch.setattr(SenderThread, "_run_templates", + lambda self, session, code: (_ for _ in ()).throw(RuntimeError("boom"))) + + thread = SenderThread(files=[], code="", url="https://example.com/api") + try: + thread.run() + except RuntimeError: + pass + assert created["session"].closed is True diff --git a/test/test_utils/test_diagram_serialization.py b/test/test_utils/test_diagram_serialization.py new file mode 100644 index 0000000..7344b13 --- /dev/null +++ b/test/test_utils/test_diagram_serialization.py @@ -0,0 +1,241 @@ +from __future__ import annotations + +import os + +import pytest + +# Use Qt's offscreen platform so the widgets never need a real display. +os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") + + +@pytest.fixture(scope="module") +def qt_app(): + try: + from PySide6.QtWidgets import QApplication + except ImportError: # pragma: no cover - PySide6 is a hard dependency + pytest.skip("PySide6 not available") + app = QApplication.instance() + if app is None: + try: + app = QApplication([]) + except Exception as exc: # pragma: no cover - no usable Qt platform + pytest.skip(f"Cannot start QApplication: {exc}") + return app + + +class TestNodeRoundTrip: + def test_node_preserves_fields(self, qt_app): + from pybreeze.pybreeze_ui.diagram_editor.diagram_items import DiagramNode, NodeShape + + node = DiagramNode(x=12, y=34, w=140, h=70, text="Hello", shape=NodeShape.DIAMOND) + restored = DiagramNode.from_dict(node.to_dict(0)) + + assert restored.text() == "Hello" + assert restored.shape_type is NodeShape.DIAMOND + assert restored.node_w == 140 + assert restored.node_h == 70 + + def test_to_dict_and_from_dict_field_symmetry(self, qt_app): + from pybreeze.pybreeze_ui.diagram_editor.diagram_items import DiagramNode, NodeShape + + # Every persisted field (except the positional id) must be consumable by + # from_dict — guards against adding a to_dict field without restoring it. + node = DiagramNode(x=0, y=0, w=100, h=60, text="N", shape=NodeShape.RECTANGLE) + data = node.to_dict(0) + clone = DiagramNode.from_dict(data) + clone_data = clone.to_dict(0) + for key in ("x", "y", "w", "h", "text", "shape", "font_size"): + assert clone_data[key] == data[key], f"field {key} not round-tripped" + + +class TestImageRoundTrip: + def test_image_preserves_source_and_geometry(self, qt_app): + from pybreeze.pybreeze_ui.diagram_editor.diagram_items import DiagramImage + + img = DiagramImage(x=5, y=6, w=210, h=120, source="C:/pics/logo.png") + restored = DiagramImage.from_dict(img.to_dict(0)) + + assert restored._source == "C:/pics/logo.png" + assert restored.img_w == 210 + assert restored.img_h == 120 + + +class TestSceneLoadRobustness: + def test_valid_items_load_despite_malformed_entries(self, qt_app): + from pybreeze.pybreeze_ui.diagram_editor.diagram_scene import DiagramScene + + scene = DiagramScene() + data = { + "nodes": [ + {"id": 0, "x": 0, "y": 0, "text": "Good", "shape": "RECTANGLE"}, + {"id": 1}, # malformed: missing x/y + {"x": 5, "y": 5, "text": "NoId"}, # missing id -> loads, not mappable + ], + "connections": [ + {"source": 0, "target": 0, "style": "SOLID"}, + {"source": 0, "target": 99}, # dangling target -> skipped + {"target": 0}, # missing source -> skipped + ], + "images": [ + {"id": 0}, # malformed: missing x/y + ], + } + # Must not raise even though several entries are malformed. + scene.load_from_dict(data) + nodes = scene.get_all_nodes() + # "Good" and "NoId" load; the {"id": 1} node is skipped. + assert {n.text() for n in nodes} == {"Good", "NoId"} + # Only the valid self-connection survives. + assert len(scene.get_all_connections()) == 1 + + def test_invalid_connection_style_falls_back_to_solid(self, qt_app): + from pybreeze.pybreeze_ui.diagram_editor.diagram_scene import DiagramScene + from pybreeze.pybreeze_ui.diagram_editor.diagram_items import ConnectionStyle + + scene = DiagramScene() + scene.load_from_dict({ + "nodes": [ + {"id": 0, "x": 0, "y": 0, "text": "A"}, + {"id": 1, "x": 0, "y": 80, "text": "B"}, + ], + "connections": [{"source": 0, "target": 1, "style": "NOT_A_STYLE"}], + }) + conns = scene.get_all_connections() + assert len(conns) == 1 + assert conns[0]._style is ConnectionStyle.SOLID + + +class TestUndoRedo: + def test_add_node_undo_redo_cycle(self, qt_app): + from pybreeze.pybreeze_ui.diagram_editor.diagram_scene import DiagramScene + from pybreeze.pybreeze_ui.diagram_editor.diagram_items import DiagramNode, NodeShape + + scene = DiagramScene() + with scene.undo_scope("Add"): + scene.addItem(DiagramNode(x=0, y=0, w=100, h=60, text="X", shape=NodeShape.RECTANGLE)) + assert len(scene.get_all_nodes()) == 1 + + scene.undo_stack.undo() + assert len(scene.get_all_nodes()) == 0 + + scene.undo_stack.redo() + assert len(scene.get_all_nodes()) == 1 + + def test_no_op_scope_pushes_no_command(self, qt_app): + from pybreeze.pybreeze_ui.diagram_editor.diagram_scene import DiagramScene + + scene = DiagramScene() + with scene.undo_scope("Nothing"): + pass + # A scope that changes nothing must not create an undo entry. + assert scene.undo_stack.count() == 0 + + +class TestCopyPaste: + def _two_connected_nodes(self): + from pybreeze.pybreeze_ui.diagram_editor.diagram_scene import DiagramScene + from pybreeze.pybreeze_ui.diagram_editor.diagram_items import ( + DiagramConnection, DiagramNode, NodeShape, + ) + scene = DiagramScene() + n1 = DiagramNode(x=0, y=0, w=100, h=60, text="A", shape=NodeShape.RECTANGLE) + n2 = DiagramNode(x=200, y=0, w=100, h=60, text="B", shape=NodeShape.RECTANGLE) + scene.addItem(n1) + scene.addItem(n2) + conn = DiagramConnection(n1, n2) + scene.addItem(conn) + return scene, n1, n2, conn + + def test_copy_paste_duplicates_nodes_and_internal_connection(self, qt_app): + scene, n1, n2, conn = self._two_connected_nodes() + for item in (n1, n2, conn): + item.setSelected(True) + scene.copy_selected() + scene.paste_clipboard() + assert len(scene.get_all_nodes()) == 4 + assert len(scene.get_all_connections()) == 2 + + def test_paste_offsets_copies(self, qt_app): + scene, n1, n2, conn = self._two_connected_nodes() + n1.setSelected(True) + scene.copy_selected() + scene.paste_clipboard() + # The pasted copy of A sits at the original +30,+30. + offsets = {(round(n.pos().x()), round(n.pos().y())) for n in scene.get_all_nodes() if n.text() == "A"} + assert (0, 0) in offsets + assert (30, 30) in offsets + + def test_connection_with_external_endpoint_not_copied(self, qt_app): + # Only n1 selected; the connection touches n2 (unselected) so it must + # not be copied (avoids a dangling reference). + scene, n1, n2, conn = self._two_connected_nodes() + n1.setSelected(True) + scene.copy_selected() + scene.paste_clipboard() + assert len(scene.get_all_connections()) == 1 # only the original remains + + +class TestCorruptedNodeData: + def test_zero_size_node_is_clamped_and_edge_point_survives(self, qt_app): + from PySide6.QtCore import QPointF + from pybreeze.pybreeze_ui.diagram_editor.diagram_items import DiagramNode, NodeShape + + node = DiagramNode(x=0, y=0, w=0, h=0, text="X", shape=NodeShape.DIAMOND) + assert node.node_w >= 40 + assert node.node_h >= 20 + # Previously a zero-size diamond divided by zero here. + point = node.edge_point(QPointF(100, 100)) + assert point is not None + + def test_invalid_colour_falls_back(self, qt_app): + from pybreeze.pybreeze_ui.diagram_editor.diagram_items import DiagramNode + + node = DiagramNode.from_dict({ + "x": 0, "y": 0, "text": "N", "shape": "RECTANGLE", + "fill_color": "not-a-real-color", + }) + assert node._fill_color.isValid() + + def test_valid_colour_preserved(self, qt_app): + from pybreeze.pybreeze_ui.diagram_editor.diagram_items import DiagramNode + + node = DiagramNode(fill_color="#ff0000") + assert node._fill_color.name() == "#ff0000" + + +class TestGridSettings: + def test_grid_settings_sync_to_nodes_and_images(self, qt_app): + from pybreeze.pybreeze_ui.diagram_editor.diagram_scene import DiagramScene + from pybreeze.pybreeze_ui.diagram_editor.diagram_items import DiagramImage, DiagramNode + + scene = DiagramScene() + try: + scene.grid_enabled = True + scene.grid_size = 25 + # Both item types snap independently; the scene must update both, + # otherwise images ignore the grid that nodes honour. + assert DiagramNode.grid_enabled is True + assert DiagramImage.grid_enabled is True + assert DiagramNode.grid_size == 25 + assert DiagramImage.grid_size == 25 + finally: + # These are class-level flags; reset so other tests see defaults. + scene.grid_enabled = False + scene.grid_size = 20 + + +class TestMermaidImportEndToEnd: + def test_parse_then_load_preserves_shapes_arrows_styles(self, qt_app): + from pybreeze.pybreeze_ui.diagram_editor.diagram_mermaid_parser import parse_mermaid + from pybreeze.pybreeze_ui.diagram_editor.diagram_scene import DiagramScene + + data = parse_mermaid("graph LR\nA[(DB)] -.-> B & C\nB ==> D{{Decision}}") + scene = DiagramScene() + scene.load_from_dict(data) + + assert len(scene.get_all_nodes()) == 4 + assert len(scene.get_all_connections()) == 3 + texts = {n.text() for n in scene.get_all_nodes()} + assert {"DB", "B", "C", "Decision"} == texts + styles = sorted(c._style.name for c in scene.get_all_connections()) + assert styles == ["DOTTED", "DOTTED", "SOLID"] # two dotted fan-out, one thick diff --git a/test/test_utils/test_fuzz_pure_logic.py b/test/test_utils/test_fuzz_pure_logic.py new file mode 100644 index 0000000..0e7e1ed --- /dev/null +++ b/test/test_utils/test_fuzz_pure_logic.py @@ -0,0 +1,101 @@ +"""Property-based fuzzing: pure-logic helpers must never crash on hostile input.""" +from __future__ import annotations + +from hypothesis import HealthCheck, given, settings +from hypothesis import strategies as st + +from pybreeze.pybreeze_ui.connect_gui.ssh.ssh_file_viewer_widget import ( + format_size, + natural_key, + remote_join, +) +from pybreeze.pybreeze_ui.diagram_editor.diagram_mermaid_parser import parse_mermaid +from pybreeze.pybreeze_ui.diagram_editor.diagram_net_utils import ( + _is_text_content_type, + _parse_content_length, +) +from pybreeze.utils.exception.exceptions import ITEJsonException +from pybreeze.utils.json_format.json_process import reformat_json +from pybreeze.utils.network.http_client import truncate_for_display +from pybreeze.utils.network.url_validation import _embedded_ipv4, _is_blocked_ip + +_FUZZ = settings(max_examples=300, deadline=None, suppress_health_check=[HealthCheck.too_slow]) + + +class TestParseMermaidNeverCrashes: + @_FUZZ + @given(st.text()) + def test_arbitrary_text(self, text): + result = parse_mermaid(text) + assert isinstance(result, dict) + assert "nodes" in result and "connections" in result + + @_FUZZ + @given(st.text(alphabet="ABCD-->.=<>|&()[]{}\"/\\ \n;", max_size=120)) + def test_diagram_like_text(self, text): + result = parse_mermaid("graph TD\n" + text) + # connections must only reference node indices that exist + node_count = len(result["nodes"]) + for conn in result["connections"]: + assert 0 <= conn["source"] < node_count + assert 0 <= conn["target"] < node_count + + +class TestHelpersNeverCrash: + @_FUZZ + @given(st.text()) + def test_natural_key(self, name): + key = natural_key(name) + assert isinstance(key, list) + + @_FUZZ + @given(st.integers()) + def test_format_size(self, num): + assert isinstance(format_size(num), str) + + @_FUZZ + @given(st.text(), st.text()) + def test_remote_join_always_absolute(self, directory, name): + assert remote_join(directory, name).startswith("/") + + @_FUZZ + @given(st.text(), st.integers(min_value=0, max_value=5000)) + def test_truncate_for_display(self, text, limit): + out = truncate_for_display(text, limit=limit) + assert isinstance(out, str) + + @_FUZZ + @given(st.text()) + def test_reformat_json_only_raises_ite(self, text): + # Must return a str or raise the documented ITEJsonException — never an + # uncaught JSONDecodeError / ValueError / RecursionError. + try: + assert isinstance(reformat_json(text), str) + except ITEJsonException: + pass + + @_FUZZ + @given(st.one_of(st.none(), st.text())) + def test_parse_content_length(self, raw): + result = _parse_content_length(raw) + assert result is None or (isinstance(result, int) and result >= 0) + + @_FUZZ + @given(st.text()) + def test_is_text_content_type(self, raw): + assert isinstance(_is_text_content_type(raw), bool) + + +class TestSsrfLogicNeverCrashes: + @_FUZZ + @given(st.ip_addresses(v=4)) + def test_blocked_ipv4(self, ip): + assert isinstance(_is_blocked_ip(ip), bool) + + @_FUZZ + @given(st.ip_addresses(v=6)) + def test_blocked_ipv6(self, ip): + # Covers the IPv4-mapped / 6to4 / Teredo / NAT64 extraction paths. + assert isinstance(_is_blocked_ip(ip), bool) + for embedded in _embedded_ipv4(ip): + assert isinstance(_is_blocked_ip(embedded), bool) diff --git a/test/test_utils/test_get_dir_file_list.py b/test/test_utils/test_get_dir_file_list.py index 749d04d..5c8164a 100644 --- a/test/test_utils/test_get_dir_file_list.py +++ b/test/test_utils/test_get_dir_file_list.py @@ -63,15 +63,25 @@ def test_returns_absolute_paths(self, temp_dir_with_files): assert os.path.isabs(path) def test_case_insensitive_extension(self, temp_dir_with_files): - # Create a file with uppercase extension + # A file with an uppercase extension must still match a lowercase query + # (Windows-primary IDE; filesystem is case-insensitive). with open(os.path.join(temp_dir_with_files, "upper.JSON"), "w") as f: f.write("{}") result = get_dir_files_as_list(temp_dir_with_files, ".json") - # .json should match files with .json extension - # The function uses file.endswith(ext.lower()) so it checks lowercase ext - # But the file "upper.JSON" won't match ".json" because endswith is case-sensitive - json_files = [f for f in result if f.endswith(".json")] - assert len(json_files) == 3 # only lowercase .json files + assert len(result) == 4 # test1/test2/nested + upper.JSON + assert any(f.endswith("upper.JSON") for f in result) + + def test_case_insensitive_query_extension(self, temp_dir_with_files): + # An uppercase query extension must match lowercase files too. + result = get_dir_files_as_list(temp_dir_with_files, ".JSON") + assert len(result) == 3 # test1.json, test2.json, nested.json + + def test_default_dir_is_resolved_at_call_time(self, temp_dir_with_files, monkeypatch): + # The default must reflect the cwd when called, not when imported. + monkeypatch.chdir(temp_dir_with_files) + result = get_dir_files_as_list() + assert len(result) == 3 + assert all(os.path.isabs(path) for path in result) def test_walks_subdirectories(self, temp_dir_with_files): result = get_dir_files_as_list(temp_dir_with_files, ".json") diff --git a/test/test_utils/test_http_client.py b/test/test_utils/test_http_client.py new file mode 100644 index 0000000..9353ca7 --- /dev/null +++ b/test/test_utils/test_http_client.py @@ -0,0 +1,76 @@ +from __future__ import annotations + +import pytest + +from pybreeze.utils.network.http_client import ( + CONNECT_TIMEOUT, + ResponseTooLargeError, + read_capped_text, + truncate_for_display, +) + + +class TestConnectTimeout: + def test_is_a_positive_number(self): + assert isinstance(CONNECT_TIMEOUT, (int, float)) + assert CONNECT_TIMEOUT > 0 + + def test_shorter_than_typical_read_timeouts(self): + # Used as the connect half of timeout=(CONNECT_TIMEOUT, read); it should + # be well under the 30/60s read budgets so connect failures fail fast. + assert CONNECT_TIMEOUT < 30 + + +class FakeResponse: + """Minimal stand-in for a streamed requests.Response.""" + + def __init__(self, body: bytes, encoding: str | None = "utf-8", chunk: int = 8): + self._body = body + self.encoding = encoding + self._chunk = chunk + self.closed = False + + def iter_content(self, chunk_size: int = 65536): + step = self._chunk + for i in range(0, len(self._body), step): + yield self._body[i:i + step] + + def close(self): + self.closed = True + + +class TestReadCappedText: + def test_reads_full_body_under_cap(self): + resp = FakeResponse(b"hello world") + assert read_capped_text(resp, max_bytes=1024) == "hello world" + assert resp.closed is True + + def test_raises_when_over_cap(self): + resp = FakeResponse(b"x" * 5000) + with pytest.raises(ResponseTooLargeError): + read_capped_text(resp, max_bytes=1000) + assert resp.closed is True # closed even on abort + + def test_exact_cap_is_allowed(self): + resp = FakeResponse(b"x" * 100) + assert len(read_capped_text(resp, max_bytes=100)) == 100 + + def test_falls_back_to_default_encoding_when_none(self): + resp = FakeResponse("héllo".encode("utf-8"), encoding=None) + assert read_capped_text(resp, default_encoding="utf-8") == "héllo" + + def test_decode_errors_are_replaced_not_raised(self): + resp = FakeResponse(b"\xff\xfe bad bytes", encoding="utf-8") + # Should not raise; invalid bytes replaced. + assert isinstance(read_capped_text(resp), str) + + +class TestTruncateForDisplay: + def test_short_text_unchanged(self): + assert truncate_for_display("short", limit=100) == "short" + + def test_long_text_truncated_with_marker(self): + result = truncate_for_display("x" * 5000, limit=100) + assert result.startswith("x" * 100) + assert "truncated" in result + assert "5000" in result diff --git a/test/test_utils/test_json_process.py b/test/test_utils/test_json_process.py index 6dc1b7e..5bc8031 100644 --- a/test/test_utils/test_json_process.py +++ b/test/test_utils/test_json_process.py @@ -35,8 +35,10 @@ def test_json_array(self): parsed = json.loads(result) assert parsed == [1, 2, 3] - def test_invalid_json_raises_error(self): - with pytest.raises((json.JSONDecodeError, ITEJsonException)): + def test_invalid_json_raises_ite_exception(self): + # reformat_json must surface a single documented exception type, not a + # raw json.JSONDecodeError leaking through. + with pytest.raises(ITEJsonException): reformat_json("not valid json {{{") def test_empty_object(self): diff --git a/test/test_utils/test_jupyter_ready.py b/test/test_utils/test_jupyter_ready.py new file mode 100644 index 0000000..a4f4ba0 --- /dev/null +++ b/test/test_utils/test_jupyter_ready.py @@ -0,0 +1,98 @@ +from __future__ import annotations + +import os + +os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") + +import pytest + + +@pytest.fixture(scope="module") +def qt_app(): + from PySide6.QtWidgets import QApplication + app = QApplication.instance() + if app is None: + try: + app = QApplication([]) + except Exception as exc: # pragma: no cover - no usable Qt platform + pytest.skip(f"Cannot start QApplication: {exc}") + return app + + +class _DeadProcess: + returncode = 1 + + class _Stderr: + @staticmethod + def read(size=-1): + text = "ImportError: jupyterlab not installed" + return text if size < 0 else text[:size] + + stderr = _Stderr() + + @staticmethod + def poll(): + return 1 + + +class _AliveProcess: + returncode = None + stderr = None + + @staticmethod + def poll(): + return None + + +def _thread(qt_app): + from pybreeze.pybreeze_ui.jupyter_lab_gui.jupyter_lab_thread import JupyterLauncherThread + return JupyterLauncherThread(startup_timeout=5) + + +class TestWaitUntilReady: + def test_early_exit_fails_fast(self, qt_app): + thread = _thread(qt_app) + thread.process = _DeadProcess() + with pytest.raises(RuntimeError) as exc: + thread._wait_until_ready(59999) + assert "exited early" in str(exc.value) + assert "jupyterlab not installed" in str(exc.value) + + def test_returns_when_port_open(self, qt_app, monkeypatch): + thread = _thread(qt_app) + thread.process = _AliveProcess() + monkeypatch.setattr(thread, "_port_open", lambda port: True) + # Should return without raising and without sleeping. + assert thread._wait_until_ready(59999) is None + + +class TestRunCleansUpOnFailure: + def test_startup_failure_terminates_orphan_process(self, qt_app, monkeypatch): + import pybreeze.pybreeze_ui.jupyter_lab_gui.jupyter_lab_thread as mod + + class _RecordingProc: + terminated = False + + def terminate(self): + self.terminated = True + + proc = _RecordingProc() + monkeypatch.setattr(mod, "get_venv_python", lambda: "python") + monkeypatch.setattr(mod, "is_jupyter_installed", lambda exe: True) + monkeypatch.setattr(mod, "find_free_port", lambda: 59999) + monkeypatch.setattr(mod.subprocess, "Popen", lambda *a, **k: proc) + + thread = mod.JupyterLauncherThread(startup_timeout=5) + + def _boom(_port): + raise TimeoutError("startup timeout") + + monkeypatch.setattr(thread, "_wait_until_ready", _boom) + + errors = [] + thread.error_occurred.connect(errors.append) + thread.run() + + # The half-started server must be terminated, not left holding the port. + assert proc.terminated is True + assert errors diff --git a/test/test_utils/test_language_parity.py b/test/test_utils/test_language_parity.py new file mode 100644 index 0000000..26a206f --- /dev/null +++ b/test/test_utils/test_language_parity.py @@ -0,0 +1,63 @@ +from __future__ import annotations + +import pathlib +import re + +import pybreeze +from pybreeze.extend_multi_language.extend_english import ( + pybreeze_english_word_dict as EN, +) +from pybreeze.extend_multi_language.extend_traditional_chinese import ( + pybreeze_traditional_chinese_word_dict as ZH, +) + +_GET_KEY_RE = re.compile(r'language_word_dict\.get\(\s*["\']([A-Za-z0-9_]+)["\']') + + +def _placeholders(text: str) -> set[str]: + return set(re.findall(r"{(\w+)}", str(text))) + + +def _code_used_keys() -> dict[str, str]: + """Map every literal ``language_word_dict.get("key")`` key to a source file.""" + root = pathlib.Path(pybreeze.__file__).parent + used: dict[str, str] = {} + for path in root.rglob("*.py"): + for match in _GET_KEY_RE.finditer(path.read_text(encoding="utf-8")): + used.setdefault(match.group(1), path.name) + return used + + +class TestLanguageParity: + def test_same_keys_in_both_languages(self): + missing_in_zh = set(EN) - set(ZH) + missing_in_en = set(ZH) - set(EN) + assert not missing_in_zh, f"Keys present in English but missing in Chinese: {sorted(missing_in_zh)}" + assert not missing_in_en, f"Keys present in Chinese but missing in English: {sorted(missing_in_en)}" + + def test_no_empty_english_values(self): + empty = [k for k, v in EN.items() if not str(v).strip()] + assert not empty, f"English keys with empty values: {empty}" + + def test_no_empty_chinese_values(self): + empty = [k for k, v in ZH.items() if not str(v).strip()] + assert not empty, f"Chinese keys with empty values: {empty}" + + def test_placeholders_match_across_languages(self): + # A {placeholder} present in one language but not the other either crashes + # .format() or leaks a literal "{x}" when the code concatenates instead. + mismatched = { + k: (_placeholders(EN[k]), _placeholders(ZH[k])) + for k in EN + if k in ZH and _placeholders(EN[k]) != _placeholders(ZH[k]) + } + assert not mismatched, f"Placeholder mismatches between languages: {mismatched}" + + +class TestCodeKeysAreDefined: + def test_every_get_key_exists_in_dict(self): + # A typo'd key (e.g. the "cot_cot_..." double prefix) makes get() return + # None, so the widget silently shows a blank title / message. + used = _code_used_keys() + missing = {k: src for k, src in used.items() if k not in EN} + assert not missing, f"language_word_dict.get() keys missing from the dict: {missing}" diff --git a/test/test_utils/test_logger.py b/test/test_utils/test_logger.py index ed8e0f5..7880134 100644 --- a/test/test_utils/test_logger.py +++ b/test/test_utils/test_logger.py @@ -16,6 +16,17 @@ def test_logger_effective_level(self): def test_logger_has_handler(self): assert len(pybreeze_logger.handlers) > 0 + def test_does_not_reconfigure_root_logger(self): + # A library must not force the root logger level or call basicConfig; + # that would override the host application's logging configuration. + import inspect + + from pybreeze.utils.logging import logger as logger_module + + source = inspect.getsource(logger_module) + assert "root.setLevel" not in source + assert "basicConfig" not in source + def test_custom_handler_creation(self): with tempfile.TemporaryDirectory() as tmpdir: log_file = os.path.join(tmpdir, "test.log") diff --git a/test/test_utils/test_mermaid_parser.py b/test/test_utils/test_mermaid_parser.py new file mode 100644 index 0000000..6f5a373 --- /dev/null +++ b/test/test_utils/test_mermaid_parser.py @@ -0,0 +1,286 @@ +from __future__ import annotations + +import pytest + +from pybreeze.pybreeze_ui.diagram_editor.diagram_mermaid_parser import ( + _order_layers, + parse_mermaid, +) + + +def _count_crossings(result): + """Count edge crossings between adjacent layers in a TD layout.""" + nodes = {n["id"]: n for n in result["nodes"]} + conns = result["connections"] + crossings = 0 + for i in range(len(conns)): + for j in range(i + 1, len(conns)): + a, b = conns[i], conns[j] + ay = (round(nodes[a["source"]]["y"]), round(nodes[a["target"]]["y"])) + by = (round(nodes[b["source"]]["y"]), round(nodes[b["target"]]["y"])) + if set(ay) == set(by) and ay[0] != ay[1]: + ax = (nodes[a["source"]]["x"], nodes[a["target"]]["x"]) + bx = (nodes[b["source"]]["x"], nodes[b["target"]]["x"]) + if (ax[0] - bx[0]) * (ax[1] - bx[1]) < 0: + crossings += 1 + return crossings + + +def _texts(result): + return {n["text"] for n in result["nodes"]} + + +def _shapes(result): + return {n["text"]: n["shape"] for n in result["nodes"]} + + +class TestBasicParsing: + def test_simple_edge(self): + r = parse_mermaid("graph TD\nA-->B") + assert len(r["nodes"]) == 2 + assert len(r["connections"]) == 1 + assert _texts(r) == {"A", "B"} + + def test_chained_arrows(self): + r = parse_mermaid("graph TD\nA-->B-->C") + assert len(r["nodes"]) == 3 + assert len(r["connections"]) == 2 + + def test_node_text_overrides_id(self): + r = parse_mermaid("graph TD\nA[Start]-->B[End]") + assert _texts(r) == {"Start", "End"} + + def test_label_after_bare_reference_is_applied(self): + # Edges declared first, labels later — labels must still take effect. + r = parse_mermaid("graph TD\nA-->B\nA[Hello]\nB[World]") + assert _texts(r) == {"Hello", "World"} + + def test_bare_reference_does_not_clobber_label(self): + r = parse_mermaid("graph TD\nA[Hello]-->B\nA-->C") + shapes = _shapes(r) + assert "Hello" in shapes + assert shapes["Hello"] == "RECTANGLE" + + def test_empty_input(self): + r = parse_mermaid("") + assert r["nodes"] == [] + assert r["connections"] == [] + + def test_whitespace_and_comments_ignored(self): + r = parse_mermaid("graph TD\n%% a comment\n\n \nA-->B %% trailing") + assert len(r["nodes"]) == 2 + + +class TestShapes: + def test_all_shapes(self): + r = parse_mermaid( + "graph LR\n" + "A[Rect]-->B(Round)\n" + "B-->C{Diamond}\n" + "C-->D((Ellipse))" + ) + shapes = _shapes(r) + assert shapes["Rect"] == "RECTANGLE" + assert shapes["Round"] == "ROUNDED_RECT" + assert shapes["Diamond"] == "DIAMOND" + assert shapes["Ellipse"] == "ELLIPSE" + + +class TestArrowsAndLabels: + def test_pipe_label(self): + r = parse_mermaid("graph TD\nA-->|yes|B") + assert r["connections"][0]["label"] == "yes" + + def test_inline_label(self): + r = parse_mermaid("graph TD\nA -- maybe --> B") + assert r["connections"][0]["label"] == "maybe" + + def test_dotted_and_thick_styles(self): + r = parse_mermaid("graph TD\nA-.->B\nB==>C") + styles = {c["style"] for c in r["connections"]} + assert "DOTTED" in styles # -.-> dotted link + assert "SOLID" in styles # ==> thick link (solid, wider) + + +class TestExtendedShapes: + @pytest.mark.parametrize("text,label,shape", [ + ("graph TD\nA[(Database)]-->B", "Database", "RECTANGLE"), # cylinder + ("graph TD\nA(((Core)))-->B", "Core", "ELLIPSE"), # double circle + ("graph TD\nA[/Input/]-->B", "Input", "RECTANGLE"), # parallelogram + ("graph TD\nA[\\Trap\\]-->B", "Trap", "RECTANGLE"), # trapezoid + ("graph TD\nA>Flag]-->B", "Flag", "RECTANGLE"), # asymmetric/flag + ("graph TD\nA([Pill])-->B", "Pill", "ROUNDED_RECT"), # stadium + ("graph TD\nA[[Sub]]-->B", "Sub", "RECTANGLE"), # subroutine + ("graph TD\nA{{Hex}}-->B", "Hex", "DIAMOND"), # hexagon + ]) + def test_shape_text_is_clean(self, text, label, shape): + r = parse_mermaid(text) + match = next(n for n in r["nodes"] if n["text"] == label) + assert match["shape"] == shape + + @pytest.mark.parametrize("text,label", [ + ('graph TD\nA["Hello World"]-->B', "Hello World"), + ('graph TD\nA["a (b) [c]"]-->B', "a (b) [c]"), + ('graph TD\nA("quoted round")-->B', "quoted round"), + ('graph TD\nA{"decision?"}-->B', "decision?"), + ]) + def test_quoted_labels_are_unquoted(self, text, label): + r = parse_mermaid(text) + assert label in {n["text"] for n in r["nodes"]} + + +class TestExtendedArrows: + @pytest.mark.parametrize("text", [ + "graph TD\nA --o B", # circle edge + "graph TD\nA --x B", # cross edge + "graph TD\nA <--> B", # bidirectional + "graph TD\nA ==o B", # thick circle + "graph TD\nA o--o B", # bidirectional circle + ]) + def test_connection_not_lost(self, text): + # Regression: --o / --x / <--> used to fail the arrow split and drop the edge. + r = parse_mermaid(text) + assert len(r["nodes"]) == 2 + assert len(r["connections"]) == 1 + + @pytest.mark.parametrize("text,first_node", [ + ("graph TD\nBox-->B", "Box"), # node ending in 'x' must not be split + ("graph TD\nFox--xB", "Fox"), # 'x' cross head must not eat the 'x' in Fox + ("graph TD\nHippo-->B", "Hippo"), # node ending in 'o' + ]) + def test_node_names_ending_in_o_or_x_not_split(self, text, first_node): + r = parse_mermaid(text) + assert first_node in {n["text"] for n in r["nodes"]} + assert len(r["connections"]) == 1 + + +class TestDirections: + @pytest.mark.parametrize("direction", ["TD", "TB", "LR", "RL", "BT"]) + def test_direction_keywords_accepted(self, direction): + r = parse_mermaid(f"graph {direction}\nA-->B") + assert len(r["nodes"]) == 2 + + def test_direction_on_same_line_as_statements(self): + # Regression: a direction keyword followed by ';'-separated statements + # used to swallow the whole line and yield zero nodes. + r = parse_mermaid("graph TD; A-->B; B-->C") + assert len(r["nodes"]) == 3 + assert len(r["connections"]) == 2 + + +class TestAmpersandMultiNode: + def _edges(self, result): + return sorted((c["source"], c["target"]) for c in result["connections"]) + + def test_fan_out(self): + r = parse_mermaid("graph TD\nA --> B & C") + assert len(r["nodes"]) == 3 + assert self._edges(r) == [(0, 1), (0, 2)] # A->B, A->C + + def test_fan_in(self): + r = parse_mermaid("graph TD\nA & B --> C") + assert len(r["nodes"]) == 3 + assert self._edges(r) == [(0, 2), (1, 2)] # A->C, B->C + + def test_cartesian_product(self): + r = parse_mermaid("graph TD\nA & B --> C & D") + assert len(r["nodes"]) == 4 + assert len(r["connections"]) == 4 + + def test_chain_after_group(self): + r = parse_mermaid("graph TD\nA & B --> C --> D") + assert len(r["nodes"]) == 4 + assert self._edges(r) == [(0, 2), (1, 2), (2, 3)] + + def test_ampersand_inside_label_not_split(self): + # The '&' inside a quoted label must not act as a node separator. + r = parse_mermaid('graph TD\nA["Tom & Jerry"] & B --> C') + assert len(r["nodes"]) == 3 + assert "Tom & Jerry" in {n["text"] for n in r["nodes"]} + + +class TestCyclesTerminate: + """Regression: cyclic graphs used to hang the layout pass forever.""" + + def test_self_loop(self): + r = parse_mermaid("graph TD\nA-->A") + assert len(r["nodes"]) == 1 + + def test_two_cycle(self): + r = parse_mermaid("graph TD\nA-->B\nB-->A") + assert len(r["nodes"]) == 2 + assert len(r["connections"]) == 2 + + def test_three_cycle(self): + r = parse_mermaid("graph LR\nA-->B\nB-->C\nC-->A") + assert len(r["nodes"]) == 3 + + def test_cycle_with_tail(self): + r = parse_mermaid("graph TD\nA-->B\nB-->C\nC-->B\nC-->D") + assert len(r["nodes"]) == 4 + + +class TestLayout: + def test_layers_separate_chain_nodes(self): + r = parse_mermaid("graph TD\nA-->B-->C") + ys = {n["text"]: n["y"] for n in r["nodes"]} + # TD layout stacks successive layers along Y. + assert ys["A"] < ys["B"] < ys["C"] + + +def _layer_overlap(result): + """True if any two nodes sharing a layer (same y) sit within one slot.""" + from collections import defaultdict + layer = defaultdict(list) + for n in result["nodes"]: + layer[round(n["y"])].append(n["x"]) + for xs in layer.values(): + xs.sort() + if any(abs(b - a) < 1.0 for a, b in zip(xs, xs[1:])): + return True + return False + + +class TestCoordinateAlignment: + def _x(self, result): + return {n["text"]: n["x"] for n in result["nodes"]} + + def test_lone_child_aligns_under_parent(self): + # C has a single child D; the C->D edge should be straight (same x). + x = self._x(parse_mermaid("graph TD\nA-->B\nA-->C\nC-->D")) + assert abs(x["C"] - x["D"]) < 0.5 + + def test_chain_is_a_straight_line(self): + x = self._x(parse_mermaid("graph TD\nA-->B-->C-->D")) + assert len({round(v, 1) for v in x.values()}) == 1 + + def test_alignment_keeps_nodes_from_overlapping(self): + r = parse_mermaid("graph TD\nR-->A\nR-->B\nR-->C\nA-->X\nB-->X\nC-->Y") + assert _layer_overlap(r) is False + + def test_alignment_does_not_add_crossings(self): + r = parse_mermaid("graph TD\nA-->Y\nB-->X\nC-->Z\nA2-->X\nB2-->Y\nC2-->Z") + assert _count_crossings(r) == 0 + + +class TestCrossingReduction: + def test_order_layers_uncrosses(self): + # Initial order makes A->D and B->C cross; barycenter must swap C/D. + layer_groups = {0: ["A", "B"], 1: ["C", "D"]} + layers = {"A": 0, "B": 0, "C": 1, "D": 1} + adj = {"A": ["D"], "B": ["C"]} + rev_adj = {"D": ["A"], "C": ["B"]} + _order_layers(layer_groups, layers, adj, rev_adj) + assert layer_groups[1] == ["D", "C"] + + def test_complex_graph_has_no_crossings(self): + # This bipartite-ish graph can be drawn crossing-free; the layout must + # find such an ordering rather than leaving avoidable crossings. + r = parse_mermaid( + "graph TD\nA-->Y\nB-->X\nC-->Z\nA2-->X\nB2-->Y\nC2-->Z" + ) + assert _count_crossings(r) == 0 + + def test_tree_has_no_crossings(self): + r = parse_mermaid("graph TD\nR-->A\nR-->B\nA-->X\nA-->Y\nB-->Z") + assert _count_crossings(r) == 0 diff --git a/test/test_utils/test_process_reader_eof.py b/test/test_utils/test_process_reader_eof.py new file mode 100644 index 0000000..a14f3ac --- /dev/null +++ b/test/test_utils/test_process_reader_eof.py @@ -0,0 +1,91 @@ +"""Regression tests for the subprocess reader threads. + +These lock in the EOF fix: an empty read from ``readline`` means the pipe +closed, so the reader must stop instead of spinning on a closed pipe (which +previously pinned a CPU core until the process was reaped). +""" +from __future__ import annotations + +import io +from queue import Queue + +from pybreeze.extend.process_executor.file_runner_process import FileRunnerProcess +from pybreeze.extend.process_executor.python_task_process_manager import TaskProcessManager + + +class _CountingStream: + """A byte stream that records how many times readline was called.""" + + def __init__(self, data: bytes): + self._buf = io.BytesIO(data) + self.readline_calls = 0 + + def readline(self, size: int = -1) -> bytes: + self.readline_calls += 1 + return self._buf.readline(size) + + +class _FakeProc: + def __init__(self, stdout: bytes = b"", stderr: bytes = b""): + self.stdout = _CountingStream(stdout) + self.stderr = _CountingStream(stderr) + + +def _make_task_manager(proc) -> TaskProcessManager: + manager = TaskProcessManager.__new__(TaskProcessManager) + manager.still_run_program = True + manager.program_buffer_size = 1024 + manager.program_encoding = "utf-8" + manager.run_output_queue = Queue() + manager.run_error_queue = Queue() + manager.process = proc + return manager + + +def _drain(q: Queue) -> list[str]: + items = [] + while not q.empty(): + items.append(q.get()) + return items + + +class TestTaskProcessManagerReader: + def test_reads_all_lines_then_stops(self): + proc = _FakeProc(stdout=b"alpha\nbeta\n") + manager = _make_task_manager(proc) + manager._read_stream_into_queue("stdout", manager.run_output_queue) + assert _drain(manager.run_output_queue) == ["alpha\n", "beta\n"] + + def test_eof_does_not_spin(self): + # Pipe already at EOF: readline should be called exactly once, then break. + proc = _FakeProc(stdout=b"") + manager = _make_task_manager(proc) + manager._read_stream_into_queue("stdout", manager.run_output_queue) + assert proc.stdout.readline_calls == 1 + assert manager.run_output_queue.empty() + + +def _make_file_runner(proc) -> FileRunnerProcess: + runner = FileRunnerProcess.__new__(FileRunnerProcess) + runner.still_running = True + runner.program_buffer_size = 1024 + runner.program_encoding = "utf-8" + runner.output_queue = Queue() + runner.error_queue = Queue() + runner.process = proc + return runner + + +class TestFileRunnerReader: + def test_reads_all_lines_then_stops(self): + proc = _FakeProc(stderr=b"warn1\nwarn2\n") + runner = _make_file_runner(proc) + runner._read_stream("stderr", runner.error_queue) + assert _drain(runner.error_queue) == ["warn1\n", "warn2\n"] + + def test_eof_does_not_spin(self): + proc = _FakeProc(stdout=b"") + runner = _make_file_runner(proc) + runner._read_stream("stdout", runner.output_queue) + assert proc.stdout.readline_calls == 1 + assert runner.output_queue.empty() diff --git a/test/test_utils/test_queue_pump.py b/test/test_utils/test_queue_pump.py new file mode 100644 index 0000000..36b933d --- /dev/null +++ b/test/test_utils/test_queue_pump.py @@ -0,0 +1,51 @@ +from __future__ import annotations + +from queue import Queue + +from pybreeze.extend.process_executor.queue_pump import ( + MAX_MESSAGES_PER_PUMP, + pump_message_queue, +) + + +def _collect_pump(messages, *, max_messages=MAX_MESSAGES_PER_PUMP): + q: Queue = Queue() + for message in messages: + q.put(message) + received: list[tuple[str, bool]] = [] + pump_message_queue( + q, + lambda text, is_error: received.append((text, is_error)), + is_error=False, + max_messages=max_messages, + ) + return q, received + + +class TestPumpBatching: + def test_drains_many_messages_in_one_call(self): + q, received = _collect_pump([f"line {i}" for i in range(50)]) + assert len(received) == 50 + assert q.empty() + + def test_respects_max_messages_bound(self): + q, received = _collect_pump([f"line {i}" for i in range(10)], max_messages=4) + assert len(received) == 4 + assert q.qsize() == 6 # remaining drained on later ticks + + def test_empty_queue_is_noop(self): + _, received = _collect_pump([]) + assert received == [] + + def test_blank_messages_are_skipped(self): + _, received = _collect_pump([" ", "\n", "real"]) + assert received == [("real", False)] + + def test_is_error_flag_forwarded(self): + q: Queue = Queue() + q.put("boom") + received: list[tuple[str, bool]] = [] + pump_message_queue( + q, lambda text, is_error: received.append((text, is_error)), is_error=True + ) + assert received == [("boom", True)] diff --git a/test/test_utils/test_sftp_remote_path.py b/test/test_utils/test_sftp_remote_path.py new file mode 100644 index 0000000..ee34d68 --- /dev/null +++ b/test/test_utils/test_sftp_remote_path.py @@ -0,0 +1,143 @@ +from __future__ import annotations + +import stat +from unittest.mock import MagicMock + +import pytest + +from pybreeze.pybreeze_ui.connect_gui.ssh import ssh_file_viewer_widget as sftp_mod +from pybreeze.pybreeze_ui.connect_gui.ssh.ssh_file_viewer_widget import ( + SSHFileTreeManager, + SSH_KEEPALIVE_SECONDS, + SFTPClientWrapper, + format_size, + natural_key, + remote_join, +) + + +class TestKeepalive: + def test_connect_enables_keepalive(self, monkeypatch): + wrapper = SFTPClientWrapper.__new__(SFTPClientWrapper) + wrapper._ssh = None + wrapper._sftp = None + + transport = MagicMock() + fake_ssh = MagicMock() + fake_ssh.get_transport.return_value = transport + monkeypatch.setattr(sftp_mod.paramiko, "SSHClient", lambda: fake_ssh) + monkeypatch.setattr(sftp_mod, "apply_host_key_policy", lambda client, parent: None) + + wrapper.connect("host", 22, "user", "pw") + + transport.set_keepalive.assert_called_once_with(SSH_KEEPALIVE_SECONDS) + + +class _Entry: + def __init__(self, filename: str, is_dir: bool): + self.filename = filename + self.st_mode = (stat.S_IFDIR if is_dir else stat.S_IFREG) | 0o644 + + +class TestNaturalKey: + def test_numbers_compare_as_integers(self): + names = ["img10.png", "img2.png", "img1.png", "img100.png"] + assert sorted(names, key=natural_key) == [ + "img1.png", "img2.png", "img10.png", "img100.png", + ] + + def test_case_insensitive(self): + assert sorted(["beta", "Alpha", "gamma"], key=natural_key) == ["Alpha", "beta", "gamma"] + + @pytest.mark.parametrize("name", ["²³", "①②", "file².txt"]) + def test_non_decimal_digit_chars_do_not_crash(self, name): + # Regression (found by fuzzing): superscript/circled "digits" are + # str.isdigit() but int() rejects them; natural_key must not crash. + assert isinstance(natural_key(name), list) + + +class TestFormatSize: + @pytest.mark.parametrize("num_bytes,expected", [ + (0, "0 B"), + (512, "512 B"), + (1023, "1023 B"), + (1024, "1.0 KB"), + (1536, "1.5 KB"), + (1048576, "1.0 MB"), + (1073741824, "1.0 GB"), + (1099511627776, "1.0 TB"), + ]) + def test_human_readable(self, num_bytes, expected): + assert format_size(num_bytes) == expected + + def test_negative_is_blank(self): + assert format_size(-1) == "" + + +class TestSortEntries: + def test_directories_first_then_natural_order(self): + entries = [ + _Entry("file10.txt", is_dir=False), + _Entry("zeta_dir", is_dir=True), + _Entry("file2.txt", is_dir=False), + _Entry("alpha_dir", is_dir=True), + ] + ordered = [name for name, _ in SSHFileTreeManager._sort_entries(entries)] + # directories first (natural/case-insensitive), then files (natural) + assert ordered == ["alpha_dir", "zeta_dir", "file2.txt", "file10.txt"] + + +class TestRemoteJoin: + @pytest.mark.parametrize("directory,name,expected", [ + ("/", "home", "/home"), + ("/home/user", "file.txt", "/home/user/file.txt"), + ("", "x", "/x"), + ("/a/b", "c", "/a/b/c"), + ("/dir", "name with spaces.txt", "/dir/name with spaces.txt"), + ]) + def test_joins_with_posix_separators(self, directory, name, expected): + assert remote_join(directory, name) == expected + + def test_never_emits_backslash(self): + # Regression: os.path.join produced '\\' on Windows and broke remote nav. + result = remote_join("/home/user", "sub") + assert "\\" not in result + assert result == "/home/user/sub" + + def test_result_is_always_absolute(self): + assert remote_join("relative/dir", "f").startswith("/") + + +class TestConnectIsAtomic: + def test_open_sftp_failure_tears_down_ssh(self, monkeypatch): + # If open_sftp() fails after the SSH transport is up, connect() must not + # leak the half-open transport: it should close it and re-raise. + wrapper = SFTPClientWrapper.__new__(SFTPClientWrapper) + wrapper.word_dict = {} + wrapper._ssh = None + wrapper._sftp = None + wrapper.root_path = "/" + + closed = {"ssh": False} + + class _SSH: + def connect(self, **kwargs): + pass + + def get_transport(self): + return MagicMock() + + def open_sftp(self): + raise OSError("sftp subsystem disabled") + + def close(self): + closed["ssh"] = True + + monkeypatch.setattr(sftp_mod.paramiko, "SSHClient", _SSH) + monkeypatch.setattr(sftp_mod, "apply_host_key_policy", lambda client, parent: None) + + with pytest.raises(OSError): + wrapper.connect("host", 22, "user", "pw") + + assert closed["ssh"] is True + assert wrapper._ssh is None diff --git a/test/test_utils/test_ssh_reentrancy.py b/test/test_utils/test_ssh_reentrancy.py new file mode 100644 index 0000000..e8d5bf0 --- /dev/null +++ b/test/test_utils/test_ssh_reentrancy.py @@ -0,0 +1,40 @@ +"""Re-clicking Connect must tear down the prior SSH session, not leak it.""" +from __future__ import annotations + +import os + +os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") + +from unittest.mock import MagicMock + +from pybreeze.pybreeze_ui.connect_gui.ssh import ssh_command_widget as mod + + +def _widget_with_valid_inputs(): + widget = mod.SSHCommandWidget.__new__(mod.SSHCommandWidget) + widget.word_dict = {} + login = MagicMock() + login.host_edit.text.return_value = "host" + login.user_edit.text.return_value = "user" + login.port_spin.value.return_value = 22 + login.use_key_check.isChecked.return_value = False + login.key_edit.text.return_value = "" + login.pass_edit.text.return_value = "pw" + widget.login_widget = login + return widget + + +class TestConnectReentrancy: + def test_cleanup_runs_before_new_session(self, monkeypatch): + widget = _widget_with_valid_inputs() + order = [] + widget._cleanup = lambda: order.append("cleanup") + widget._start_shell = lambda *args: order.append("start_shell") + monkeypatch.setattr(mod.paramiko, "SSHClient", lambda: MagicMock()) + monkeypatch.setattr(mod, "apply_host_key_policy", lambda client, parent: None) + + widget.connect_ssh() + + # The prior session is torn down before a new client/shell is created, + # so re-connecting cannot leak the old client or orphan its reader. + assert order == ["cleanup", "start_shell"] diff --git a/test/test_utils/test_ssh_security.py b/test/test_utils/test_ssh_security.py new file mode 100644 index 0000000..f87e249 --- /dev/null +++ b/test/test_utils/test_ssh_security.py @@ -0,0 +1,50 @@ +from __future__ import annotations + +import base64 +import hashlib + +import paramiko + +from pybreeze.pybreeze_ui.connect_gui.ssh.ssh_host_key_policy import _fingerprint_sha256 +from pybreeze.pybreeze_ui.connect_gui.ssh.ssh_key_loader import load_private_key + + +class TestFingerprint: + def test_format_and_value(self): + key = paramiko.RSAKey.generate(2048) + fp = _fingerprint_sha256(key) + assert fp.startswith("SHA256:") + assert "=" not in fp # OpenSSH style strips base64 padding + expected = "SHA256:" + base64.b64encode( + hashlib.sha256(key.asbytes()).digest() + ).rstrip(b"=").decode("ascii") + assert fp == expected + + def test_distinct_keys_have_distinct_fingerprints(self): + fp1 = _fingerprint_sha256(paramiko.RSAKey.generate(2048)) + fp2 = _fingerprint_sha256(paramiko.RSAKey.generate(2048)) + assert fp1 != fp2 + + +class TestLoadPrivateKey: + def test_missing_file_returns_none(self, tmp_path): + assert load_private_key(str(tmp_path / "nope"), "") is None + + def test_rsa_round_trip(self, tmp_path): + path = tmp_path / "id_rsa" + paramiko.RSAKey.generate(2048).write_private_key_file(str(path)) + loaded = load_private_key(str(path), "") + assert isinstance(loaded, paramiko.RSAKey) + + def test_ecdsa_loads_via_fallback(self, tmp_path): + # ECDSA is the third key class tried, so this exercises the fallback loop. + path = tmp_path / "id_ecdsa" + paramiko.ECDSAKey.generate().write_private_key_file(str(path)) + loaded = load_private_key(str(path), "") + assert isinstance(loaded, paramiko.ECDSAKey) + + def test_encrypted_key_requires_correct_passphrase(self, tmp_path): + path = tmp_path / "id_rsa_enc" + paramiko.RSAKey.generate(2048).write_private_key_file(str(path), password="secret") + assert load_private_key(str(path), "wrong") is None + assert isinstance(load_private_key(str(path), "secret"), paramiko.RSAKey) diff --git a/test/test_utils/test_subprocess_util.py b/test/test_utils/test_subprocess_util.py new file mode 100644 index 0000000..647fdcb --- /dev/null +++ b/test/test_utils/test_subprocess_util.py @@ -0,0 +1,45 @@ +from __future__ import annotations + +import subprocess +import sys + +from pybreeze.utils.subprocess_util import no_window_creationflags, utf8_subprocess_env + + +class TestUtf8SubprocessEnv: + def test_sets_pythonioencoding(self): + env = utf8_subprocess_env("utf-8") + assert env["PYTHONIOENCODING"] == "utf-8" + + def test_honours_custom_encoding(self): + assert utf8_subprocess_env("cp950")["PYTHONIOENCODING"] == "cp950" + + def test_preserves_existing_environment(self, monkeypatch): + monkeypatch.setenv("PYBREEZE_TEST_MARKER", "kept") + env = utf8_subprocess_env() + assert env.get("PYBREEZE_TEST_MARKER") == "kept" + + def test_child_actually_uses_the_encoding(self): + # A real child must report the pinned stdout encoding. + result = subprocess.run( + [sys.executable, "-c", "import sys; print(sys.stdout.encoding)"], + capture_output=True, text=True, env=utf8_subprocess_env("utf-8"), + ) + assert result.stdout.strip().replace("-", "").lower() == "utf8" + + +class TestNoWindowCreationflags: + def test_returns_int(self): + assert isinstance(no_window_creationflags(), int) + + def test_matches_platform(self): + flags = no_window_creationflags() + if sys.platform == "win32": + assert flags == subprocess.CREATE_NO_WINDOW + else: + assert flags == 0 + + def test_is_a_noop_when_or_combined_on_posix_semantics(self): + # OR-ing the flag with other creationflags must never lose existing bits. + base = 0x4 + assert base | no_window_creationflags() >= base diff --git a/test/test_utils/test_syntax_keywords.py b/test/test_utils/test_syntax_keywords.py new file mode 100644 index 0000000..fb61218 --- /dev/null +++ b/test/test_utils/test_syntax_keywords.py @@ -0,0 +1,30 @@ +from __future__ import annotations + +from collections import Counter + +import pybreeze.pybreeze_ui.syntax.syntax_keyword as sk + + +def _keyword_lists(): + """Yield (name, list) for every module-level keyword sequence of strings.""" + for name in dir(sk): + if name.startswith("_"): + continue + value = getattr(sk, name) + if isinstance(value, (list, tuple)) and value and all(isinstance(x, str) for x in value): + yield name, value + + +class TestSyntaxKeywords: + def test_no_duplicate_keywords(self): + offenders = {} + for name, value in _keyword_lists(): + dups = {kw: n for kw, n in Counter(value).items() if n > 1} + if dups: + offenders[name] = dups + assert not offenders, f"Duplicate keywords found: {offenders}" + + def test_no_blank_keywords(self): + for name, value in _keyword_lists(): + blanks = [kw for kw in value if not kw.strip()] + assert not blanks, f"{name} contains blank keywords" diff --git a/test/test_utils/test_task_manager_venv.py b/test/test_utils/test_task_manager_venv.py new file mode 100644 index 0000000..f5420a0 --- /dev/null +++ b/test/test_utils/test_task_manager_venv.py @@ -0,0 +1,57 @@ +from __future__ import annotations + +import os + +os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") + +import pytest + + +@pytest.fixture(scope="module") +def qt_app(): + from PySide6.QtWidgets import QApplication + app = QApplication.instance() + if app is None: + try: + app = QApplication([]) + except Exception as exc: # pragma: no cover - no usable Qt platform + pytest.skip(f"Cannot start QApplication: {exc}") + return app + + +def _raise_no_python(_path): + from je_editor.utils.exception.exceptions import JEditorExecException + raise JEditorExecException("no python interpreter found") + + +class TestRenewPathNoInterpreter: + def test_returns_false_without_crashing(self, qt_app, monkeypatch): + from pybreeze.extend.process_executor import python_task_process_manager as mod + from pybreeze.extend.process_executor.python_task_process_manager import TaskProcessManager + from pybreeze.pybreeze_ui.show_code_window.code_window import CodeWindow + + window = CodeWindow() + window.python_compiler = None + + manager = TaskProcessManager.__new__(TaskProcessManager) + manager.main_window = window + + monkeypatch.setattr(mod, "find_venv_path", lambda: "ignored") + monkeypatch.setattr(mod, "check_and_choose_venv", _raise_no_python) + + # Must report (not raise) so the run menu callback does not crash. + assert manager.renew_path() is False + assert "No Python interpreter" in window.code_result.toPlainText() + + def test_returns_true_with_explicit_compiler(self, qt_app): + from pybreeze.extend.process_executor.python_task_process_manager import TaskProcessManager + from pybreeze.pybreeze_ui.show_code_window.code_window import CodeWindow + + window = CodeWindow() + window.python_compiler = "C:/python/python.exe" + + manager = TaskProcessManager.__new__(TaskProcessManager) + manager.main_window = window + + assert manager.renew_path() is True + assert manager.compiler_path == "C:/python/python.exe" diff --git a/test/test_utils/test_url_validation.py b/test/test_utils/test_url_validation.py new file mode 100644 index 0000000..3cf3a6b --- /dev/null +++ b/test/test_utils/test_url_validation.py @@ -0,0 +1,62 @@ +from __future__ import annotations + +import pytest + +from pybreeze.utils.network.url_validation import UnsafeURLError, validate_url + + +class TestSchemeValidation: + @pytest.mark.parametrize("url", [ + "file:///etc/passwd", + "ftp://example.com/x", + "gopher://example.com/", + "data:text/plain;base64,AAAA", + ]) + def test_non_http_schemes_rejected(self, url): + with pytest.raises(UnsafeURLError): + validate_url(url) + + def test_missing_hostname_rejected(self): + with pytest.raises(UnsafeURLError): + validate_url("http:///nohost") + + +class TestPrivateAddressBlocking: + @pytest.mark.parametrize("url", [ + "http://127.0.0.1/", # loopback + "http://10.0.0.1/", # RFC1918 + "http://192.168.1.1/", # RFC1918 + "http://172.16.0.1/", # RFC1918 + "http://169.254.169.254/", # link-local / cloud metadata + "http://0.0.0.0/", # unspecified + "http://100.64.0.1/", # RFC6598 CGNAT + "http://224.0.0.1/", # multicast + "http://[::1]/", # IPv6 loopback + "http://[fe80::1]/", # IPv6 link-local + ]) + def test_blocked(self, url): + with pytest.raises(UnsafeURLError): + validate_url(url) + + +class TestIPv6TransitionForms: + """IPv6 wrappers that route to the blocked cloud-metadata IPv4 (169.254.169.254).""" + + @pytest.mark.parametrize("url", [ + "http://[::ffff:169.254.169.254]/", # IPv4-mapped + "http://[2002:a9fe:a9fe::1]/", # 6to4 wrapping 169.254.169.254 + "http://[64:ff9b::a9fe:a9fe]/", # NAT64 well-known prefix + ]) + def test_transition_form_to_metadata_blocked(self, url): + with pytest.raises(UnsafeURLError): + validate_url(url) + + +class TestPublicAddressAllowed: + @pytest.mark.parametrize("url", [ + "http://8.8.8.8/", + "https://1.1.1.1/path?q=1", + "http://[2606:4700:4700::1111]/", # Cloudflare public IPv6 + ]) + def test_public_ip_allowed(self, url): + assert validate_url(url) == url