diff --git a/synodic_client/application/qt.py b/synodic_client/application/qt.py index a737efb..3aab74a 100644 --- a/synodic_client/application/qt.py +++ b/synodic_client/application/qt.py @@ -5,14 +5,15 @@ import logging import signal import sys +import traceback import types from collections.abc import Callable import qasync from porringer.api import API from porringer.schema import LocalConfiguration -from PySide6.QtCore import Qt, QTimer -from PySide6.QtWidgets import QApplication +from PySide6.QtCore import QEvent, QObject, Qt, QTimer +from PySide6.QtWidgets import QApplication, QWidget from synodic_client.application.icon import app_icon from synodic_client.application.init import run_startup_preamble @@ -90,6 +91,34 @@ def _exception_hook( sys.excepthook = _exception_hook +class _TopLevelShowFilter(QObject): + """[DIAG] Application-wide event filter that logs Show/WindowActivate on top-level widgets.""" + + _diag_logger = logging.getLogger('synodic_client.diag.window') + + def eventFilter(self, obj: QObject, event: QEvent) -> bool: # noqa: N802 + if ( + event.type() in {QEvent.Type.Show, QEvent.Type.WindowActivate} + and isinstance(obj, QWidget) + and obj.isWindow() + ): + geo = obj.geometry() + stack = ''.join(traceback.format_stack(limit=12)) + self._diag_logger.warning( + '[DIAG] Top-level window %s: class=%s title=%r geo=(%d,%d %dx%d) visible=%s\n%s', + event.type().name, + type(obj).__qualname__, + obj.windowTitle(), + geo.x(), + geo.y(), + geo.width(), + geo.height(), + obj.isVisible(), + stack, + ) + return False + + def _init_app() -> QApplication: """Create and configure the ``QApplication``.""" # Set the App User Model ID so Windows uses our icon on the taskbar @@ -104,6 +133,10 @@ def _init_app() -> QApplication: app.setWindowIcon(app_icon()) app.setAttribute(Qt.ApplicationAttribute.AA_CompressHighFrequencyEvents) + # [DIAG] Install a global event filter to log every top-level window show. + diag_filter = _TopLevelShowFilter(app) # parented to app, prevented from GC + app.installEventFilter(diag_filter) + # Allow Ctrl+C in the terminal to terminate the application. # Qt's event loop blocks Python's default SIGINT handling, so we # install our own handler and use a short timer to let Python diff --git a/synodic_client/application/screen/install.py b/synodic_client/application/screen/install.py index f07d4d6..a699581 100644 --- a/synodic_client/application/screen/install.py +++ b/synodic_client/application/screen/install.py @@ -13,6 +13,7 @@ import asyncio import logging +import traceback from pathlib import Path from typing import Any @@ -27,6 +28,7 @@ SyncStrategy, ) from PySide6.QtCore import Qt, QTimer, Signal +from PySide6.QtGui import QShowEvent from PySide6.QtWidgets import ( QFileDialog, QFrame, @@ -890,6 +892,21 @@ def __init__( self._init_ui() + def showEvent(self, event: QShowEvent) -> None: # noqa: N802 + """[DIAG] Log every show event with a stack trace.""" + geo = self.geometry() + stack = ''.join(traceback.format_stack(limit=10)) + logger.warning( + '[DIAG] InstallPreviewWindow.showEvent: geo=(%d,%d %dx%d) visible=%s\n%s', + geo.x(), + geo.y(), + geo.width(), + geo.height(), + self.isVisible(), + stack, + ) + super().showEvent(event) + def _init_ui(self) -> None: """Build the UI layout.""" central = QWidget() @@ -952,11 +969,6 @@ def _on_browse_project_dir(self) -> None: # --- Lifecycle --- - def showEvent(self, event: Any) -> None: - """Log when the window becomes visible.""" - super().showEvent(event) - logger.info('Install preview window shown (visible=%s)', self.isVisible()) - def closeEvent(self, event: Any) -> None: """Clean up the temp directory when the window is closed.""" logger.info('Install preview window closing') diff --git a/synodic_client/application/screen/plugin_row.py b/synodic_client/application/screen/plugin_row.py index f23aa71..e83cb56 100644 --- a/synodic_client/application/screen/plugin_row.py +++ b/synodic_client/application/screen/plugin_row.py @@ -37,8 +37,10 @@ PLUGIN_ROW_PROJECT_TAG_STYLE, PLUGIN_ROW_PROJECT_TAG_TRANSITIVE_STYLE, PLUGIN_ROW_REMOVE_STYLE, + PLUGIN_ROW_STATUS_MIN_WIDTH, PLUGIN_ROW_STATUS_STYLE, PLUGIN_ROW_STYLE, + PLUGIN_ROW_TIMESTAMP_MIN_WIDTH, PLUGIN_ROW_TIMESTAMP_STYLE, PLUGIN_ROW_TOGGLE_STYLE, PLUGIN_ROW_UPDATE_STYLE, @@ -341,7 +343,7 @@ def _build_controls(self, layout: QHBoxLayout, data: PluginRowData) -> None: Controls are always created in the same order with fixed widths so that columns align vertically across all rows. Hidden - controls still reserve space. + controls reserve space via ``retainSizeWhenHidden``. """ if data.show_toggle: self._build_toggle(layout, data) @@ -352,23 +354,25 @@ def _build_controls(self, layout: QHBoxLayout, data: PluginRowData) -> None: # Inline auto-update status (e.g. "Up to date", "v1.2 available") self._update_status_label = QLabel() self._update_status_label.setStyleSheet(PLUGIN_ROW_STATUS_STYLE) + self._update_status_label.setMinimumWidth(PLUGIN_ROW_STATUS_MIN_WIDTH) self._update_status_label.hide() layout.addWidget(self._update_status_label) + self._retain_size(layout) - # Version - if data.version: - version_label = QLabel(data.version) - version_label.setStyleSheet(PLUGIN_ROW_VERSION_STYLE) - version_label.setMinimumWidth(PLUGIN_ROW_VERSION_MIN_WIDTH) - version_label.setAlignment(Qt.AlignmentFlag.AlignRight | Qt.AlignmentFlag.AlignVCenter) - layout.addWidget(version_label) + # Version — always created so column width is reserved + version_label = QLabel(data.version) + version_label.setStyleSheet(PLUGIN_ROW_VERSION_STYLE) + version_label.setMinimumWidth(PLUGIN_ROW_VERSION_MIN_WIDTH) + version_label.setAlignment(Qt.AlignmentFlag.AlignRight | Qt.AlignmentFlag.AlignVCenter) + layout.addWidget(version_label) - # Timestamp + # Timestamp — always created so column width is reserved + self._timestamp_label = QLabel(_format_relative_time(data.last_updated) if data.last_updated else '') + self._timestamp_label.setStyleSheet(PLUGIN_ROW_TIMESTAMP_STYLE) + self._timestamp_label.setMinimumWidth(PLUGIN_ROW_TIMESTAMP_MIN_WIDTH) if data.last_updated: - self._timestamp_label = QLabel(_format_relative_time(data.last_updated)) - self._timestamp_label.setStyleSheet(PLUGIN_ROW_TIMESTAMP_STYLE) self._timestamp_label.setToolTip(f'Last updated: {data.last_updated}') - layout.addWidget(self._timestamp_label) + layout.addWidget(self._timestamp_label) # Transient inline error label (hidden by default) self._error_label = QLabel() @@ -409,6 +413,18 @@ def _build_update_button(self, layout: QHBoxLayout, data: PluginRowData) -> None update_btn.setVisible(data.has_update) self._update_btn = update_btn layout.addWidget(update_btn) + self._retain_size(layout) + + @staticmethod + def _retain_size(layout: QHBoxLayout) -> None: + """Mark the most recently added widget as size-retaining when hidden.""" + item = layout.itemAt(layout.count() - 1) + if item is not None: + widget = item.widget() + if widget is not None: + policy = widget.sizePolicy() + policy.setRetainSizeWhenHidden(True) + widget.setSizePolicy(policy) def _build_remove_button(self, layout: QHBoxLayout, data: PluginRowData) -> None: """Add the remove button — enabled only for global packages.""" diff --git a/synodic_client/application/screen/screen.py b/synodic_client/application/screen/screen.py index 4146d1e..78de44b 100644 --- a/synodic_client/application/screen/screen.py +++ b/synodic_client/application/screen/screen.py @@ -2,6 +2,7 @@ import asyncio import logging +import traceback from collections import OrderedDict from pathlib import Path @@ -20,6 +21,7 @@ ) from porringer.schema.plugin import PluginKind from PySide6.QtCore import Qt, QTimer, Signal +from PySide6.QtGui import QShowEvent from PySide6.QtWidgets import ( QHBoxLayout, QLineEdit, @@ -197,6 +199,8 @@ def refresh(self) -> None: """Schedule an asynchronous rebuild of the tool list.""" if self._refresh_in_progress: return + caller = ''.join(traceback.format_stack(limit=4)) + logger.info('[DIAG] ToolsView.refresh() called, parent_visible=%s\n%s', self.isVisible(), caller) asyncio.create_task(self._async_refresh()) async def _async_refresh(self) -> None: @@ -1104,6 +1108,21 @@ def __init__( # Update banner — always available, starts hidden. self._update_banner = UpdateBanner(self) + def showEvent(self, event: QShowEvent) -> None: # noqa: N802 + """[DIAG] Log every show event with a stack trace.""" + geo = self.geometry() + stack = ''.join(traceback.format_stack(limit=10)) + logger.warning( + '[DIAG] MainWindow.showEvent: geo=(%d,%d %dx%d) visible=%s\n%s', + geo.x(), + geo.y(), + geo.width(), + geo.height(), + self.isVisible(), + stack, + ) + super().showEvent(event) + @property def porringer(self) -> API | None: """Return the porringer API instance, if available.""" diff --git a/synodic_client/application/screen/settings.py b/synodic_client/application/screen/settings.py index 4fb9791..69b5684 100644 --- a/synodic_client/application/screen/settings.py +++ b/synodic_client/application/screen/settings.py @@ -7,11 +7,12 @@ import logging import sys +import traceback from collections.abc import Iterator from contextlib import contextmanager from PySide6.QtCore import Qt, QUrl, Signal -from PySide6.QtGui import QDesktopServices +from PySide6.QtGui import QDesktopServices, QShowEvent from PySide6.QtWidgets import ( QCheckBox, QComboBox, @@ -53,6 +54,21 @@ class SettingsWindow(QMainWindow): check_updates_requested = Signal() """Emitted when the user clicks the *Check for Updates* button.""" + def showEvent(self, event: QShowEvent) -> None: # noqa: N802 + """[DIAG] Log every show event with a stack trace.""" + geo = self.geometry() + stack = ''.join(traceback.format_stack(limit=10)) + logger.warning( + '[DIAG] SettingsWindow.showEvent: geo=(%d,%d %dx%d) visible=%s\n%s', + geo.x(), + geo.y(), + geo.width(), + geo.height(), + self.isVisible(), + stack, + ) + super().showEvent(event) + def __init__( self, config: ResolvedConfig, diff --git a/synodic_client/application/screen/tool_update_controller.py b/synodic_client/application/screen/tool_update_controller.py index 2f91f28..d9599ae 100644 --- a/synodic_client/application/screen/tool_update_controller.py +++ b/synodic_client/application/screen/tool_update_controller.py @@ -54,6 +54,7 @@ def __init__( window: MainWindow, config_resolver: Callable[[], ResolvedConfig], tray: QSystemTrayIcon, + is_user_active: Callable[[], bool] | None = None, ) -> None: """Set up the controller. @@ -61,10 +62,14 @@ def __init__( window: The main application window. config_resolver: Callable returning the current resolved config. tray: System tray icon for notification messages. + is_user_active: Predicate returning ``True`` when the user + has a visible window. Periodic tool updates are + deferred while active. """ self._window = window self._resolve_config = config_resolver self._tray = tray + self._is_user_active = is_user_active or (lambda: False) self._tool_task: asyncio.Task[None] | None = None self._tool_update_timer: QTimer | None = None @@ -108,10 +113,17 @@ def restart_tool_update_timer(self) -> None: self._tool_update_timer = self._restart_timer( self._tool_update_timer, config.tool_update_interval_minutes, - self.on_tool_update, + self._on_periodic_tool_update, 'Automatic tool updating', ) + def _on_periodic_tool_update(self) -> None: + """Timer callback — deferred when the user has a visible window.""" + if self._is_user_active(): + logger.debug('Periodic tool update deferred — user is active') + return + self.on_tool_update() + # -- ToolsView signal wiring -- def connect_tools_view(self, tools_view: ToolsView) -> None: @@ -293,6 +305,12 @@ def _on_tool_update_finished( # Clear updating state on widgets tools_view = self._window.tools_view + logger.info( + '[DIAG] _on_tool_update_finished: manual=%s, tools_view_exists=%s, window_visible=%s', + manual, + tools_view is not None, + self._window.isVisible(), + ) if tools_view is not None: if updating_plugin is not None: tools_view.set_plugin_updating(updating_plugin, False) diff --git a/synodic_client/application/screen/tray.py b/synodic_client/application/screen/tray.py index 16fa599..e82bcd4 100644 --- a/synodic_client/application/screen/tray.py +++ b/synodic_client/application/screen/tray.py @@ -72,15 +72,17 @@ def __init__( app, client, self._banner, - self._settings_window, - config, + settings_window=self._settings_window, + config=config, ) + self._update_controller.set_user_active_predicate(self._is_user_active) # Tool update orchestrator - owns tool/package update lifecycle self._tool_orchestrator = ToolUpdateOrchestrator( window, self._resolve_config, self.tray, + is_user_active=self._is_user_active, ) self._tool_orchestrator.restart_tool_update_timer() @@ -128,6 +130,14 @@ def _show_settings(self) -> None: """Show the settings window.""" self._settings_window.show() + def _is_user_active(self) -> bool: + """Return ``True`` when the user has a visible window. + + Used by the update controllers to defer automatic updates + while the user is actively interacting with the application. + """ + return self._window.isVisible() or self._settings_window.isVisible() + def _on_settings_changed(self, config: ResolvedConfig) -> None: """React to a change made in the settings window.""" self._config = config diff --git a/synodic_client/application/theme.py b/synodic_client/application/theme.py index 0ed0dd5..d457d01 100644 --- a/synodic_client/application/theme.py +++ b/synodic_client/application/theme.py @@ -231,6 +231,12 @@ PLUGIN_ROW_VERSION_MIN_WIDTH = 60 """Minimum width for the version label column.""" +PLUGIN_ROW_STATUS_MIN_WIDTH = 90 +"""Minimum width for the inline auto-update status label.""" + +PLUGIN_ROW_TIMESTAMP_MIN_WIDTH = 40 +"""Minimum width for the relative timestamp label.""" + PLUGIN_ROW_SPACING = 1 """Pixels between individual tool/package rows.""" diff --git a/synodic_client/application/update_controller.py b/synodic_client/application/update_controller.py index 3ec38b7..5a988a4 100644 --- a/synodic_client/application/update_controller.py +++ b/synodic_client/application/update_controller.py @@ -10,6 +10,7 @@ import asyncio import logging +from collections.abc import Callable from datetime import UTC, datetime from typing import TYPE_CHECKING @@ -53,6 +54,9 @@ class UpdateController: The ``SettingsWindow`` (receives status text + colour). config: Optional pre-resolved configuration. ``None`` resolves from disk. + is_user_active: + Predicate returning ``True`` when the user has a visible window. + Automatic checks and auto-apply are deferred while active. """ def __init__( @@ -60,6 +64,7 @@ def __init__( app: QApplication, client: Client, banner: UpdateBanner, + *, settings_window: SettingsWindow, config: ResolvedConfig | None = None, ) -> None: @@ -77,6 +82,7 @@ def __init__( self._banner = banner self._settings_window = settings_window self._config = config + self._is_user_active: Callable[[], bool] = lambda: False self._update_task: asyncio.Task[None] | None = None # Derive auto-apply preference from config @@ -94,6 +100,14 @@ def __init__( # Wire settings check-updates button self._settings_window.check_updates_requested.connect(self._on_manual_check) + def set_user_active_predicate(self, predicate: Callable[[], bool]) -> None: + """Set the predicate used to defer automatic checks when the user is active. + + Args: + predicate: Returns ``True`` when the user has a visible window. + """ + self._is_user_active = predicate + # ------------------------------------------------------------------ # Config helpers # ------------------------------------------------------------------ @@ -104,6 +118,14 @@ def _resolve_config(self) -> ResolvedConfig: return self._config return resolve_config() + def _can_auto_apply(self) -> bool: + """Return whether a downloaded update should be applied automatically. + + Auto-apply is suppressed when the user has a visible window so + the application is never force-restarted during active use. + """ + return self._auto_apply and not self._is_user_active() + # ------------------------------------------------------------------ # Timer management # ------------------------------------------------------------------ @@ -176,7 +198,14 @@ def _on_manual_check(self) -> None: self._do_check(silent=False) def _on_auto_check(self) -> None: - """Handle automatic (periodic) check — silent.""" + """Handle automatic (periodic) check — silent. + + Skipped when the user has a visible window to avoid disruptive + downloads and auto-apply restarts. The next timer tick retries. + """ + if self._is_user_active(): + logger.debug('Automatic update check deferred — user is active') + return self._do_check(silent=True) def _do_check(self, *, silent: bool) -> None: @@ -193,9 +222,11 @@ def _do_check(self, *, silent: bool) -> None: async def _async_check(self, *, silent: bool) -> None: """Run the update check coroutine and route results.""" + logger.info('[DIAG] Self-update check starting (silent=%s)', silent) try: result = await check_for_update(self._client) self._on_check_finished(result, silent=silent) + logger.info('[DIAG] Self-update check completed (silent=%s)', silent) except Exception as exc: logger.exception('Update check failed') self._on_check_error(str(exc), silent=silent) @@ -277,7 +308,7 @@ def _on_download_finished(self, success: bool, version: str) -> None: # Persist the client update timestamp update_user_config(last_client_update=datetime.now(UTC).isoformat()) - if self._auto_apply: + if self._can_auto_apply(): # Silently apply and restart — no banner, no user interaction logger.info('Auto-applying update v%s', version) self._settings_window.set_update_status( @@ -287,7 +318,7 @@ def _on_download_finished(self, success: bool, version: str) -> None: self._apply_update(silent=True) return - # Manual mode — show ready banner and let user choose when to restart + # Manual mode (or user is active) — show ready banner and let user choose when to restart self._banner.show_ready(version) self._settings_window.set_update_status( f'v{version} ready', diff --git a/tests/unit/qt/test_update_controller.py b/tests/unit/qt/test_update_controller.py index ef1745c..0906755 100644 --- a/tests/unit/qt/test_update_controller.py +++ b/tests/unit/qt/test_update_controller.py @@ -49,6 +49,7 @@ def _make_controller( *, auto_apply: bool = True, auto_update_interval_minutes: int = 0, + is_user_active: bool = False, ) -> tuple[UpdateController, MagicMock, MagicMock, UpdateBanner, MagicMock]: """Build an ``UpdateController`` with mocked collaborators. @@ -69,7 +70,14 @@ def _make_controller( mock_ucfg.return_value = MagicMock( auto_update_interval_minutes=auto_update_interval_minutes, ) - controller = UpdateController(app, client, banner, settings, config) + controller = UpdateController( + app, + client, + banner, + settings_window=settings, + config=config, + ) + controller.set_user_active_predicate(lambda: is_user_active) return controller, app, client, banner, settings @@ -199,6 +207,90 @@ def test_download_failure_shows_error() -> None: settings.set_update_status.assert_called_with('Download failed', UPDATE_STATUS_ERROR_STYLE) +# --------------------------------------------------------------------------- +# User-active gating +# --------------------------------------------------------------------------- + + +class TestUserActiveGating: + """Verify that automatic actions are deferred when the user is active.""" + + @staticmethod + def test_auto_check_skipped_when_user_active() -> None: + """_on_auto_check should not call _do_check when user is active.""" + ctrl, _app, _client, banner, settings = _make_controller(is_user_active=True) + + with patch.object(ctrl, '_do_check') as mock_check: + ctrl._on_auto_check() + + mock_check.assert_not_called() + + @staticmethod + def test_auto_check_proceeds_when_user_inactive() -> None: + """_on_auto_check should call _do_check when user is NOT active.""" + ctrl, _app, _client, banner, settings = _make_controller(is_user_active=False) + + with patch.object(ctrl, '_do_check') as mock_check: + ctrl._on_auto_check() + + mock_check.assert_called_once_with(silent=True) + + @staticmethod + def test_manual_check_unaffected_by_active_user() -> None: + """_on_manual_check should always call _do_check regardless of user activity.""" + ctrl, _app, _client, banner, settings = _make_controller(is_user_active=True) + + with patch.object(ctrl, '_do_check') as mock_check: + ctrl._on_manual_check() + + mock_check.assert_called_once_with(silent=False) + + @staticmethod + def test_auto_apply_deferred_when_user_active() -> None: + """When auto_apply=True but user is active, show READY banner instead of applying.""" + ctrl, app, client, banner, settings = _make_controller( + auto_apply=True, + is_user_active=True, + ) + + with patch.object(ctrl, '_apply_update') as mock_apply: + ctrl._on_download_finished(True, '2.0.0') + + mock_apply.assert_not_called() + assert banner.state.name == 'READY' + + @staticmethod + def test_auto_apply_proceeds_when_user_inactive() -> None: + """When auto_apply=True and user is inactive, _apply_update is called.""" + ctrl, app, client, banner, settings = _make_controller( + auto_apply=True, + is_user_active=False, + ) + + with patch.object(ctrl, '_apply_update') as mock_apply: + ctrl._on_download_finished(True, '2.0.0') + + mock_apply.assert_called_once_with(silent=True) + + @staticmethod + def test_can_auto_apply_false_when_user_active() -> None: + """_can_auto_apply should return False when auto_apply=True but user is active.""" + ctrl, *_ = _make_controller(auto_apply=True, is_user_active=True) + assert ctrl._can_auto_apply() is False + + @staticmethod + def test_can_auto_apply_false_when_disabled() -> None: + """_can_auto_apply should return False when auto_apply=False.""" + ctrl, *_ = _make_controller(auto_apply=False, is_user_active=False) + assert ctrl._can_auto_apply() is False + + @staticmethod + def test_can_auto_apply_true_when_enabled_and_inactive() -> None: + """_can_auto_apply should return True only when auto_apply=True and user is inactive.""" + ctrl, *_ = _make_controller(auto_apply=True, is_user_active=False) + assert ctrl._can_auto_apply() is True + + # --------------------------------------------------------------------------- # Apply update # --------------------------------------------------------------------------- diff --git a/tool/pyinstaller/rthook_no_console.py b/tool/pyinstaller/rthook_no_console.py index e49d4fc..577c64e 100644 --- a/tool/pyinstaller/rthook_no_console.py +++ b/tool/pyinstaller/rthook_no_console.py @@ -27,6 +27,9 @@ _STARTF_USESHOWWINDOW = _sp.STARTF_USESHOWWINDOW _CREATE_NO_WINDOW = _sp.CREATE_NO_WINDOW + # [DIAG] Toggle to log every subprocess spawn to stderr. + _SUBPROCESS_LOGGING = True + _original_init = subprocess.Popen.__init__ def _patched_init(self: subprocess.Popen, *args: Any, **kwargs: Any) -> None: @@ -37,6 +40,10 @@ def _patched_init(self: subprocess.Popen, *args: Any, **kwargs: Any) -> None: startupinfo.wShowWindow = _SW_HIDE kwargs['startupinfo'] = startupinfo + if _SUBPROCESS_LOGGING: + cmd = args[0] if args else kwargs.get('args', '') + print(f'[DIAG] subprocess.Popen: {cmd}', file=sys.stderr, flush=True) + _original_init(self, *args, **kwargs) subprocess.Popen.__init__ = _patched_init