From d85be3dcc3243da0966244fcc4d1a4d90261755b Mon Sep 17 00:00:00 2001 From: Asher Norland Date: Sat, 21 Mar 2026 10:31:02 -0700 Subject: [PATCH 1/2] Bootstrap Fix --- synodic_client/application/bootstrap.py | 19 +++++- synodic_client/cli/update.py | 9 ++- tests/unit/qt/test_update_controller.py | 17 ++++++ tests/unit/test_bootstrap.py | 79 +++++++++++++++++++++++++ tests/unit/test_cli.py | 29 ++++++++- 5 files changed, 148 insertions(+), 5 deletions(-) create mode 100644 tests/unit/test_bootstrap.py diff --git a/synodic_client/application/bootstrap.py b/synodic_client/application/bootstrap.py index c206a98..3adc7a3 100644 --- a/synodic_client/application/bootstrap.py +++ b/synodic_client/application/bootstrap.py @@ -8,9 +8,11 @@ Import order matters: 1. stdlib + config (pure-Python, fast) 2. configure_logging() — now Qt-free - 3. initialize_velopack() — hooks run with logging active - 4. run_startup_preamble() — protocol, config seed, auto-startup - 5. import qt.application — PySide6 / porringer loaded here + 3. sync_startup() — refresh Windows auto-startup registry **before** + Velopack, which may exit the process during post-update hooks + 4. initialize_velopack() — hooks run with logging active + 5. run_startup_preamble() — protocol, config seed, auto-startup + 6. import qt.application — PySide6 / porringer loaded here """ import logging @@ -47,6 +49,17 @@ def bootstrap() -> None: logger = logging.getLogger(__name__) logger.info('Bootstrap started (exe=%s, argv=%s)', sys.executable, sys.argv) + # Refresh the Windows auto-startup registry entry BEFORE Velopack + # initialisation. App.run() may exit the current process during + # post-update lifecycle hooks, so sync_startup must run first to + # ensure the registry path stays current after an update. + if not dev_mode: + from synodic_client.resolution import resolve_config + from synodic_client.startup import sync_startup + + config = resolve_config() + sync_startup(sys.executable, auto_start=config.auto_start) + initialize_velopack() if not dev_mode: diff --git a/synodic_client/cli/update.py b/synodic_client/cli/update.py index 6e0b615..08c56ca 100644 --- a/synodic_client/cli/update.py +++ b/synodic_client/cli/update.py @@ -8,6 +8,7 @@ from __future__ import annotations import asyncio +import sys from typing import Annotated import typer @@ -75,7 +76,13 @@ def update_apply( """Apply a downloaded self-update.""" from synodic_client.cli.context import get_services from synodic_client.operations.update import apply_self_update + from synodic_client.startup import sync_startup + + client, _, config = get_services() + + # Refresh the Windows auto-startup registry entry before the update + # replaces the executable, so the path stays current. + sync_startup(sys.executable, auto_start=config.auto_start) - client, _, _ = get_services() apply_self_update(client, restart=not no_restart, silent=silent) typer.echo('Update applied.') diff --git a/tests/unit/qt/test_update_controller.py b/tests/unit/qt/test_update_controller.py index 6d79660..2027300 100644 --- a/tests/unit/qt/test_update_controller.py +++ b/tests/unit/qt/test_update_controller.py @@ -40,6 +40,7 @@ def __init__(self, model: UpdateModel) -> None: def _make_controller( *, auto_apply: bool = True, + auto_start: bool = True, auto_update_interval_minutes: int = 0, is_user_active: bool = False, ) -> tuple[UpdateController, MagicMock, MagicMock, UpdateBanner, UpdateModel]: @@ -49,6 +50,7 @@ def _make_controller( """ config = make_resolved_config( auto_apply=auto_apply, + auto_start=auto_start, auto_update_interval_minutes=auto_update_interval_minutes, ) @@ -352,6 +354,21 @@ def test_apply_update_refreshes_startup_registry_when_frozen() -> None: client.apply_update_on_exit.assert_called_once() app.quit.assert_called_once() + @staticmethod + def test_apply_update_passes_auto_start_false_from_config() -> None: + """sync_startup receives auto_start=False when config says so.""" + ctrl, app, client, banner, model = _make_controller(auto_start=False) + ctrl._pending_version = '2.0.0' + + with ( + patch('synodic_client.application.update_controller.sync_startup') as mock_sync, + patch('synodic_client.application.update_controller.sys') as mock_sys, + ): + mock_sys.executable = r'C:\app\synodic.exe' + ctrl._apply_update() + + mock_sync.assert_called_once_with(r'C:\app\synodic.exe', auto_start=False) + # --------------------------------------------------------------------------- # Settings changed → immediate check diff --git a/tests/unit/test_bootstrap.py b/tests/unit/test_bootstrap.py new file mode 100644 index 0000000..0315bde --- /dev/null +++ b/tests/unit/test_bootstrap.py @@ -0,0 +1,79 @@ +"""Tests for the bootstrap entry point startup-sync ordering.""" + +from __future__ import annotations + +import importlib +import sys +from unittest.mock import MagicMock, patch + +_MODULE = 'synodic_client.application.bootstrap' + + +def _run_bootstrap(*, argv: list[str]) -> None: + """Import (or reload) the bootstrap module, triggering ``bootstrap()``. + + The module-level ``bootstrap()`` call runs on every import/reload, + so all patches must be in place before calling this. + """ + # Ensure sys.argv is set for the bootstrap function + with patch.object(sys, 'argv', argv): + if _MODULE in sys.modules: + importlib.reload(sys.modules[_MODULE]) + else: + importlib.import_module(_MODULE) + + +class TestBootstrapStartupSync: + """Verify sync_startup runs before initialize_velopack in bootstrap.""" + + @staticmethod + def test_sync_startup_called_before_velopack_init() -> None: + """sync_startup must execute before initialize_velopack. + + Velopack's App.run() may exit the process during post-update + hooks, so the startup registry must already be refreshed. + """ + call_order: list[str] = [] + + def _record_sync(*args: object, **kwargs: object) -> None: + call_order.append('sync_startup') + + def _record_velopack() -> None: + call_order.append('initialize_velopack') + + mock_config = MagicMock(auto_start=True) + + with ( + patch('synodic_client.config.set_dev_mode'), + patch('synodic_client.logging.configure_logging'), + patch('synodic_client.subprocess_patch.apply'), + patch('synodic_client.updater.initialize_velopack', side_effect=_record_velopack), + patch('synodic_client.resolution.resolve_config', return_value=mock_config), + patch('synodic_client.startup.sync_startup', side_effect=_record_sync) as mock_sync, + patch('synodic_client.application.init.run_startup_preamble'), + patch('synodic_client.application.qt.application'), + patch('synodic_client.protocol.extract_uri_from_args', return_value=None), + ): + _run_bootstrap(argv=[r'C:\app\synodic.exe']) + + assert call_order == ['sync_startup', 'initialize_velopack'] + mock_sync.assert_called_once() + assert mock_sync.call_args.kwargs['auto_start'] is True + + @staticmethod + def test_sync_startup_skipped_in_dev_mode() -> None: + """sync_startup is not called when --dev flag is passed.""" + with ( + patch('synodic_client.config.set_dev_mode'), + patch('synodic_client.logging.configure_logging'), + patch('synodic_client.subprocess_patch.apply'), + patch('synodic_client.updater.initialize_velopack'), + patch('synodic_client.resolution.resolve_config') as mock_resolve, + patch('synodic_client.startup.sync_startup') as mock_sync, + patch('synodic_client.application.qt.application'), + patch('synodic_client.protocol.extract_uri_from_args', return_value=None), + ): + _run_bootstrap(argv=[r'C:\app\synodic.exe', '--dev']) + + mock_resolve.assert_not_called() + mock_sync.assert_not_called() diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index a28db8b..67cb8c7 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -295,15 +295,42 @@ def test_update_download() -> None: @staticmethod def test_update_apply() -> None: """Update apply calls apply_self_update.""" + mock_config = MagicMock(auto_start=True) with ( - patch('synodic_client.cli.context.get_services', return_value=(MagicMock(), None, None)), + patch('synodic_client.cli.context.get_services', return_value=(MagicMock(), None, mock_config)), patch('synodic_client.operations.update.apply_self_update') as mock_apply, + patch('synodic_client.startup.sync_startup'), ): result = runner.invoke(app, ['update', 'apply']) assert result.exit_code == 0 assert 'applied' in result.output.lower() mock_apply.assert_called_once() + @staticmethod + def test_update_apply_calls_sync_startup_before_apply() -> None: + """sync_startup is called with the config's auto_start before apply_self_update.""" + call_order: list[str] = [] + mock_config = MagicMock(auto_start=False) + + def _record_sync(*args: object, **kwargs: object) -> None: + call_order.append('sync_startup') + + def _record_apply(*args: object, **kwargs: object) -> None: + call_order.append('apply_self_update') + + with ( + patch('synodic_client.cli.context.get_services', return_value=(MagicMock(), None, mock_config)), + patch('synodic_client.startup.sync_startup', side_effect=_record_sync) as mock_sync, + patch('synodic_client.operations.update.apply_self_update', side_effect=_record_apply), + ): + result = runner.invoke(app, ['update', 'apply']) + assert result.exit_code == 0 + + mock_sync.assert_called_once() + # auto_start=False should be forwarded from config + assert mock_sync.call_args.kwargs['auto_start'] is False + assert call_order == ['sync_startup', 'apply_self_update'] + # --------------------------------------------------------------------------- # Debug subcommands From 5c989b93838c1dbc99b470513144f5622f790e7c Mon Sep 17 00:00:00 2001 From: Asher Norland Date: Sat, 21 Mar 2026 11:27:32 -0700 Subject: [PATCH 2/2] Update Chore Regression Fix --- .../application/update_controller.py | 8 +- synodic_client/resolution.py | 8 +- synodic_client/updater.py | 117 +++++++++++++++--- tests/unit/test_resolution.py | 8 +- tests/unit/test_updater.py | 12 +- 5 files changed, 120 insertions(+), 33 deletions(-) diff --git a/synodic_client/application/update_controller.py b/synodic_client/application/update_controller.py index cd259fd..49335a7 100644 --- a/synodic_client/application/update_controller.py +++ b/synodic_client/application/update_controller.py @@ -124,7 +124,13 @@ def _set_task(self, coro: Coroutine[Any, Any, None]) -> None: """Cancel any in-flight task and start *coro* as the active task.""" if self._update_task is not None and not self._update_task.done(): self._update_task.cancel() - self._update_task = asyncio.create_task(coro) + try: + self._update_task = asyncio.create_task(coro) + except RuntimeError: + # No running event loop yet (e.g. during early init). + # The periodic timer will retry once the loop is running. + coro.close() + logger.debug('Deferred update check — event loop not yet running') # ------------------------------------------------------------------ # Config helpers diff --git a/synodic_client/resolution.py b/synodic_client/resolution.py index 3d24a92..0d7fc06 100644 --- a/synodic_client/resolution.py +++ b/synodic_client/resolution.py @@ -31,7 +31,6 @@ UpdateConfig, UserConfig, ) -from synodic_client.updater import github_release_asset_url logger = logging.getLogger(__name__) @@ -171,14 +170,9 @@ def resolve_update_config(config: ResolvedConfig) -> UpdateConfig: """ channel = UpdateChannel.DEVELOPMENT if config.update_channel == 'dev' else UpdateChannel.STABLE - repo_url = github_release_asset_url( - config.update_source or GITHUB_REPO_URL, - channel, - ) - return UpdateConfig( channel=channel, - repo_url=repo_url, + repo_url=config.update_source or GITHUB_REPO_URL, auto_update_interval_minutes=config.auto_update_interval_minutes, tool_update_interval_minutes=config.tool_update_interval_minutes, ) diff --git a/synodic_client/updater.py b/synodic_client/updater.py index 4d2972e..753c434 100644 --- a/synodic_client/updater.py +++ b/synodic_client/updater.py @@ -8,8 +8,10 @@ """ import contextlib +import json import logging import sys +import urllib.request from collections.abc import Callable from typing import Any @@ -173,7 +175,19 @@ def check_for_update(self) -> UpdateInfo: error='Not installed via Velopack', ) - velopack_info = manager.check_for_updates() + try: + velopack_info = manager.check_for_updates() + except Exception as sdk_err: + if '404' in str(sdk_err): + logger.debug('SDK check failed with 404, trying manifest fallback: %s', sdk_err) + velopack_info = self._check_manifest_fallback() + else: + raise + + if velopack_info is None: + # SDK returned no update; try the manual manifest fallback + # in case the SDK's GithubSource skipped prerelease entries. + velopack_info = self._check_manifest_fallback() if velopack_info is not None: latest = Version(velopack_info.TargetFullRelease.Version) @@ -202,20 +216,6 @@ def check_for_update(self) -> UpdateInfo: return self._update_info except Exception as e: - if '404' in str(e): - channel = self._config.channel_name - msg = ( - f"No releases found for the '{channel}' channel. " - "Try switching to the 'Development' channel in Settings \u2192 Channel." - ) - logger.debug('No releases for channel %s: %s', channel, e) - self._state = UpdateState.NO_UPDATE - return UpdateInfo( - available=False, - current_version=self._current_version, - error=msg, - ) - logger.exception('Failed to check for updates') self._state = UpdateState.FAILED return UpdateInfo( @@ -224,6 +224,93 @@ def check_for_update(self) -> UpdateInfo: error=str(e), ) + def _check_manifest_fallback(self) -> Any: + """Download the release manifest directly and check for updates. + + The Velopack SDK's ``GithubSource`` handler cannot discover + updates from prerelease GitHub Releases. This fallback + downloads ``releases.{channel}.json`` via Python's stdlib and + constructs a ``velopack.UpdateInfo`` when a newer version + exists. + + Returns: + A ``velopack.UpdateInfo`` if an update is available, + ``None`` otherwise. + """ + asset_base = github_release_asset_url(self._config.repo_url, self._config.channel) + manifest_url = f'{asset_base}/releases.{self._config.channel_name}.json' + logger.debug('Manifest fallback: fetching %s', manifest_url) + + try: + req = urllib.request.Request(manifest_url, headers={'User-Agent': 'synodic-client'}) + with urllib.request.urlopen(req, timeout=30) as resp: # noqa: S310 — URL is derived from a known repo constant + data = json.loads(resp.read()) + except Exception: + logger.debug('Manifest fallback failed for %s', manifest_url, exc_info=True) + return None + + current_semver = pep440_to_semver(str(self._current_version)) + best: dict[str, Any] | None = None + best_ver: str | None = None + + for asset in data.get('Assets', []): + if asset.get('Type') != 'Full': + continue + ver = asset.get('Version', '') + if not ver: + continue + # Simple semver comparison via packaging.version (accepts + # semver pre-release tags like ``0.1.0-dev.79``). + try: + if Version(ver) > Version(current_semver): + if best_ver is None or Version(ver) > Version(best_ver): + best = asset + best_ver = ver + except Exception: + continue + + if best is None: + logger.debug('Manifest fallback: no newer version found') + return None + + logger.debug('Manifest fallback: found %s', best_ver) + + target = velopack.VelopackAsset( + PackageId=best['PackageId'], + Version=best['Version'], + Type=best['Type'], + FileName=best['FileName'], + SHA1=best.get('SHA1', ''), + SHA256=best.get('SHA256', ''), + Size=best.get('Size', 0), + NotesMarkdown='', + NotesHtml='', + ) + + # Collect matching delta assets for the same version. + deltas = [] + for asset in data.get('Assets', []): + if asset.get('Type') == 'Delta' and asset.get('Version') == best['Version']: + deltas.append( + velopack.VelopackAsset( + PackageId=asset['PackageId'], + Version=asset['Version'], + Type=asset['Type'], + FileName=asset['FileName'], + SHA1=asset.get('SHA1', ''), + SHA256=asset.get('SHA256', ''), + Size=asset.get('Size', 0), + NotesMarkdown='', + NotesHtml='', + ) + ) + + return velopack.UpdateInfo( + TargetFullRelease=target, + DeltasToTarget=deltas, + IsDowngrade=False, + ) + def download_update(self, progress_callback: Callable[[int], None] | None = None) -> bool: """Download the update. diff --git a/tests/unit/test_resolution.py b/tests/unit/test_resolution.py index eb4bfc7..6aaf476 100644 --- a/tests/unit/test_resolution.py +++ b/tests/unit/test_resolution.py @@ -364,17 +364,17 @@ def test_custom_source_non_github() -> None: @staticmethod def test_default_source_dev() -> None: - """Verify default dev source uses GitHub download path with dev tag.""" + """Verify default dev source uses raw GitHub repo URL.""" config = _make_resolved(update_channel='dev') result = resolve_update_config(config) - assert result.repo_url == f'{GITHUB_REPO_URL}/releases/download/dev' + assert result.repo_url == GITHUB_REPO_URL @staticmethod def test_default_source_stable() -> None: - """Verify default stable source uses GitHub latest download path.""" + """Verify default stable source uses raw GitHub repo URL.""" config = _make_resolved(update_channel='stable') result = resolve_update_config(config) - assert result.repo_url == f'{GITHUB_REPO_URL}/releases/latest/download' + assert result.repo_url == GITHUB_REPO_URL @staticmethod def test_default_auto_update_interval() -> None: diff --git a/tests/unit/test_updater.py b/tests/unit/test_updater.py index e927a90..438326f 100644 --- a/tests/unit/test_updater.py +++ b/tests/unit/test_updater.py @@ -227,18 +227,18 @@ def test_check_error(updater: Updater) -> None: @staticmethod def test_check_404_returns_friendly_message(updater: Updater) -> None: - """Verify a 404 from GitHub returns a friendly no-releases message.""" + """Verify a 404 from GitHub falls back to manifest check gracefully.""" mock_manager = MagicMock(spec=velopack.UpdateManager) mock_manager.check_for_updates.side_effect = RuntimeError('Network error: Http error: http status: 404') - with patch.object(updater, '_get_velopack_manager', return_value=mock_manager): + with ( + patch.object(updater, '_get_velopack_manager', return_value=mock_manager), + patch.object(updater, '_check_manifest_fallback', return_value=None), + ): info = updater.check_for_update() assert info.available is False - assert info.error is not None - assert 'No releases found' in info.error - assert updater._config.channel_name in info.error - # A missing channel is informational, not a hard failure + # Fallback returned None, so no error — just no update. assert updater.state == UpdateState.NO_UPDATE @staticmethod