diff --git a/agent-notes/improve-startup-registration.md b/agent-notes/improve-startup-registration.md new file mode 100644 index 0000000..5fbde0e --- /dev/null +++ b/agent-notes/improve-startup-registration.md @@ -0,0 +1,34 @@ +# Goal + +Correct the existing Windows startup panel and registration command. + +# State + +PR #4 is open. Initial CI ran 11 tests; ten passed and the save-failure regression +errored because its get_executable double returned Mock instead of a path string. +Corrected that fixture so it reaches the intended CreateKeyEx permission error. +No production relaxation or added guard; waiting for repaired-head CI. + +Isolated branch based on improve/integration-contract (PR #1). Public panel and +get_startup/set_startup names are unchanged. No system registry writes performed +by development/tests; tests use a controlled winreg boundary. + +# Decisions and why + +Set the rendered checkbox's live value. Missing key/value means disabled; other +read failures propagate. Enable with CreateKeyEx/KEY_SET_VALUE and REG_SZ; disable +only this application's value with KEY_SET_VALUE. A missing disable is idempotent. +Use stdlib subprocess.list2cmdline for the Windows command; source launches include +sys.executable because platform_utils returns the main script when not frozen. +No existing platform_utils command builder was found. Low-level write errors +propagate; the panel logs and displays a translated warning on save failure. +The startup and audio labels now explicitly bind their app_elements domain. + +# Surprises + +The package still supports Python 3 only, so the Python 2 _winreg import fallback +is removed. Startup remains explicitly Windows-only. No startup launch measured. + +# Next step + +Publish focused PR and read CI. No live Windows startup/AT claim, merge or release. diff --git a/app_elements/startup.py b/app_elements/startup.py index 47021e6..b380c97 100644 --- a/app_elements/startup.py +++ b/app_elements/startup.py @@ -1,17 +1,20 @@ from logging import getLogger +import subprocess +import sys +import winreg logger = getLogger("app_elements.startup") -try: - import winreg as _winreg -except ImportError: - import _winreg +import i18n_core from gui_builder import fields from platform_utils import paths from wx_utils import forms as wx_forms +from wx_utils import popups import app_elements +i18n_core.install_module_translation("app_elements", module=sys.modules[__name__]) application = app_elements.find_application_module() +RUN_KEY = r"Software\Microsoft\Windows\CurrentVersion\Run" class RunAtStartupPanel(wx_forms.AutoSizedPanel): @@ -19,37 +22,42 @@ class RunAtStartupPanel(wx_forms.AutoSizedPanel): def render(self, *args, **kwargs): super(RunAtStartupPanel, self).render(*args, **kwargs) - self.run_at_startup.default_value = get_startup() + self.run_at_startup.set_value(get_startup()) def set_config_values(self): - old_startup = get_startup() - if self.run_at_startup.get_value() != old_startup: - set_startup(self.run_at_startup.get_value()) + try: + old_startup = get_startup() + if self.run_at_startup.get_value() != old_startup: + set_startup(self.run_at_startup.get_value()) + except OSError: + logger.exception("Unable to change startup registration") + popups.warning_message( + title=_("Startup setting not saved"), + message=_("Unable to change the Windows startup setting. Check your account permissions and try again."), + ) def get_startup(): - with _winreg.OpenKey( - _winreg.HKEY_CURRENT_USER, r"Software\Microsoft\Windows\CurrentVersion\Run" - ) as key: - try: - _winreg.QueryValueEx(key, application.name) + try: + with winreg.OpenKey(winreg.HKEY_CURRENT_USER, RUN_KEY, 0, winreg.KEY_READ) as key: + winreg.QueryValueEx(key, application.name) return True - except WindowsError: - pass + except FileNotFoundError: + pass return False def set_startup(value): - with _winreg.OpenKey( - _winreg.HKEY_CURRENT_USER, - r"Software\Microsoft\Windows\CurrentVersion\Run", - 0, - _winreg.KEY_ALL_ACCESS, - ) as key: + if value: + arguments = [paths.get_executable()] + if not paths.is_frozen(): + arguments.insert(0, sys.executable) + command = subprocess.list2cmdline(arguments) + with winreg.CreateKeyEx(winreg.HKEY_CURRENT_USER, RUN_KEY, 0, winreg.KEY_SET_VALUE) as key: + winreg.SetValueEx(key, application.name, 0, winreg.REG_SZ, command) + else: try: - if value: - _winreg.SetValueEx(key, application.name, 0, 1, paths.get_executable()) - else: - _winreg.DeleteValue(key, application.name) - except WindowsError as e: - logger.exception("Unable to set startup to %r" % value) + with winreg.OpenKey(winreg.HKEY_CURRENT_USER, RUN_KEY, 0, winreg.KEY_SET_VALUE) as key: + winreg.DeleteValue(key, application.name) + except FileNotFoundError: + pass diff --git a/tests/test_startup.py b/tests/test_startup.py new file mode 100644 index 0000000..44668ac --- /dev/null +++ b/tests/test_startup.py @@ -0,0 +1,88 @@ +import unittest +from unittest.mock import MagicMock, patch + +from support import load_component + + +class StartupTests(unittest.TestCase): + def registry_key(self, registry, operation): + context = MagicMock() + getattr(registry, operation).return_value = context + return context.__enter__.return_value + + def test_missing_run_key_and_missing_value_are_disabled(self): + for missing in ("OpenKey", "QueryValueEx"): + with self.subTest(missing=missing): + with load_component("app_elements.startup") as (component, _, modules): + registry = modules["winreg"] + self.registry_key(registry, "OpenKey") + getattr(registry, missing).side_effect = FileNotFoundError() + self.assertFalse(component.get_startup()) + + def test_render_sets_live_checkbox_without_changing_registry(self): + with load_component("app_elements.startup") as (component, _, modules): + self.registry_key(modules["winreg"], "OpenKey") + panel = component.RunAtStartupPanel() + panel.render() + panel.run_at_startup.set_value.assert_called_once_with(True) + modules["winreg"].SetValueEx.assert_not_called() + + def test_frozen_path_with_spaces_is_quoted_and_uses_write_only_rights(self): + with load_component("app_elements.startup") as (component, app, modules): + registry = modules["winreg"] + key = self.registry_key(registry, "CreateKeyEx") + modules["platform_utils.paths"].is_frozen.return_value = True + modules["platform_utils.paths"].get_executable.return_value = r"C:\Program Files\Example\Example.exe" + component.set_startup(True) + registry.CreateKeyEx.assert_called_once_with(registry.HKEY_CURRENT_USER, component.RUN_KEY, 0, registry.KEY_SET_VALUE) + registry.SetValueEx.assert_called_once_with(key, app.name, 0, registry.REG_SZ, + '"C:\\Program Files\\Example\\Example.exe"') + + def test_source_launch_includes_interpreter_and_script(self): + with load_component("app_elements.startup") as (component, app, modules): + registry = modules["winreg"] + key = self.registry_key(registry, "CreateKeyEx") + modules["platform_utils.paths"].get_executable.return_value = r"C:\My Apps\Example\main.pyw" + with patch.object(component.sys, "executable", r"C:\Python Install\pythonw.exe"): + component.set_startup(True) + registry.SetValueEx.assert_called_once_with(key, app.name, 0, registry.REG_SZ, + '"C:\\Python Install\\pythonw.exe" "C:\\My Apps\\Example\\main.pyw"') + + def test_disabling_deletes_only_this_application_value(self): + with load_component("app_elements.startup") as (component, app, modules): + registry = modules["winreg"] + key = self.registry_key(registry, "OpenKey") + component.set_startup(False) + registry.OpenKey.assert_called_once_with(registry.HKEY_CURRENT_USER, component.RUN_KEY, 0, registry.KEY_SET_VALUE) + registry.DeleteValue.assert_called_once_with(key, app.name) + registry.SetValueEx.assert_not_called() + + def test_disabling_missing_registration_is_idempotent(self): + for missing in ("OpenKey", "DeleteValue"): + with self.subTest(missing=missing): + with load_component("app_elements.startup") as (component, _, modules): + registry = modules["winreg"] + self.registry_key(registry, "OpenKey") + getattr(registry, missing).side_effect = FileNotFoundError() + component.set_startup(False) + + def test_access_denial_is_not_misreported_as_disabled(self): + with load_component("app_elements.startup") as (component, _, modules): + modules["winreg"].OpenKey.side_effect = PermissionError("denied") + with self.assertRaises(PermissionError): + component.get_startup() + + def test_save_failure_shows_a_warning_and_does_not_claim_success(self): + with load_component("app_elements.startup") as (component, _, modules): + registry = modules["winreg"] + modules["platform_utils.paths"].get_executable.return_value = r"C:\Example\main.pyw" + registry.OpenKey.side_effect = FileNotFoundError() + registry.CreateKeyEx.side_effect = PermissionError("denied") + panel = component.RunAtStartupPanel() + panel.run_at_startup.get_value.return_value = True + with self.assertLogs("app_elements.startup", level="ERROR"): + panel.set_config_values() + warning = modules["wx_utils.popups"].warning_message + warning.assert_called_once() + self.assertIn("not saved", warning.call_args.kwargs["title"]) + registry.SetValueEx.assert_not_called()