Correct Windows startup registration and save feedback

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.

app_elements/startup.py

@@ -1,55 +1,63 @@
 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):
     run_at_startup = fields.CheckBox(label=__("&Run at startup"))
 
     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

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()