fix(update): count local changes one way for the preflight and the pull

The automatic update's preflight ignored mode-only changes and anything
whose status line contained plugins/ or plugin-repos/, then promised
"Automatic updates will not stash your changes". perform_core_update
used plain git status (modes count) and ignored only 'plugins/', then
ran 'git stash push -- :!plugins', which nothing ever pops. So an edit
to a bundled plugin under plugin-repos/, or the installer's chmods on
tracked scripts, passed the preflight and was stashed away for good.

- auto_update.local_changes() is the one predicate both use:
  core.fileMode=false, porcelain -z, and plugins/ and plugin-repos/
  excluded by leading folder rather than substring (a core file under
  web_interface/static/v3/js/plugins/ now counts).
- Update Code's explicit stash leaves out both plugin folders; the
  pull's --autostash carries their edits and mode changes across and
  reapplies them.
- The automatic updater calls perform_core_update(stash_local_changes=
  False), which refuses instead of stashing edits that appeared after
  the preflight; update_core reports that as 'blocked'.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

docs/WEB_INTERFACE_GUIDE.md

@@ -102,7 +102,9 @@ Configure basic system settings:
   check are shown under the toggle, and anything other than success raises a
   banner on **Overview**.
   - *Checks first:* the code update is skipped, with the reason shown, if
-    tracked files were edited locally, the checkout has local commits, a
+    tracked files were edited locally (permission-only changes and edits under
+    `plugins/` or `plugin-repos/` don't count; the pull carries those across
+    and puts them back), the checkout has local commits, a
     rebase/merge is in progress, the branch has no upstream, less than 300 MB
     is free, or the newest version already failed once. A failed fetch is
     retried the next day.

scripts/utils/auto_update_verify.py

@@ -191,8 +191,11 @@ class Verifier:
         if not old:
             return False, 'the commit to roll back to is unknown'
         requirements = self.changed_requirements(old, new) if new else list(REQUIREMENT_FILES)
-        # Safe to --hard: the updater refuses to run with local edits to
-        # tracked files, so the only thing this discards is the update.
+        # --hard: the updater refuses to run with local edits to tracked core
+        # files (web_interface/auto_update.local_changes), so outside the
+        # plugin folders the only thing this discards is the update. Edits
+        # under plugins/ and plugin-repos/, which that check leaves to the
+        # pull's --autostash, are reset along with it.
         result = self._run(['git', 'reset', '--hard', old], timeout=120)
         if result.returncode != 0:
             return False, (f'"git reset --hard {old}" failed: '

test/test_auto_update.py

@@ -72,7 +72,7 @@ class Repo:
 
 
 def real_pull(device, **extra):
-    def core_update():
+    def core_update(**kwargs):
         before = git(device, 'rev-parse', 'HEAD')
         r = subprocess.run(['git', 'pull', '-q', '--rebase', '--autostash'],
                            cwd=str(device), capture_output=True, text=True)
@@ -339,6 +339,140 @@ class TestPreflightRefuses:
         assert repo.head() == old and h.sudo == []
 
 
+# -- what counts as a local change ---------------------------------------------
+
+def _device_with_plugin_and_script(tmp_path):
+    """A clone carrying a bundled plugin and an executable script, as LEDMatrix does."""
+    repo = Repo(tmp_path)
+    for rel, text in (('plugin-repos/web-ui-info/manager.py', 'x = 1\n'),
+                      ('web_interface/static/v3/js/plugins/store.js', 'var a;\n'),
+                      ('scripts/run.sh', 'echo hi\n')):
+        (repo.seed / rel).parent.mkdir(parents=True, exist_ok=True)
+        (repo.seed / rel).write_text(text)
+    git(repo.seed, 'add', '.')
+    git(repo.seed, 'update-index', '--chmod=+x', 'scripts/run.sh')
+    git(repo.seed, 'commit', '-qm', 'layout')
+    git(repo.seed, 'push', '-q', 'origin', 'main')
+    git(repo.device, 'pull', '-q')
+    return repo
+
+
+def _chmod_like_the_installer(device):
+    """first_time_install.sh sets tracked 100755 files to 644: a mode-only change.
+
+    Windows has no exec bit, so there core.fileMode=true alone shows it."""
+    import os
+    git(device, 'config', 'core.fileMode', 'true')
+    os.chmod(device / 'scripts' / 'run.sh', 0o644)
+    assert 'mode change 100755 => 100644 scripts/run.sh' in git(device, 'diff', '--summary')
+
+
+@needs_git
+class TestLocalChangesAreCountedOnce:
+    """The preflight promises the pull will not stash. That holds only if both
+    count local changes the same way (auto_update.local_changes)."""
+
+    def test_mode_changes_and_plugin_folders_do_not_count(self, tmp_path):
+        repo = _device_with_plugin_and_script(tmp_path)
+        _chmod_like_the_installer(repo.device)
+        (repo.device / 'plugin-repos/web-ui-info/manager.py').write_text('x = 2  # store update\n')
+        assert au.local_changes(repo.device) == []
+
+    def test_a_core_folder_named_plugins_still_counts(self, tmp_path):
+        """The old filter matched 'plugins/' anywhere in the line."""
+        repo = _device_with_plugin_and_script(tmp_path)
+        (repo.device / 'web_interface/static/v3/js/plugins/store.js').write_text('var mine;\n')
+        assert au.local_changes(repo.device) == ['web_interface/static/v3/js/plugins/store.js']
+
+    def test_a_staged_rename_is_reported_by_its_new_name(self, tmp_path):
+        repo = Repo(tmp_path)
+        git(repo.device, 'mv', 'app.py', 'main.py')
+        assert au.local_changes(repo.device) == ['main.py']
+
+    def test_the_preflight_refuses_a_core_edit_under_a_plugins_folder(self, tmp_path):
+        repo = _device_with_plugin_and_script(tmp_path)
+        old = repo.head()
+        repo.publish()
+        (repo.device / 'web_interface/static/v3/js/plugins/store.js').write_text('var mine;\n')
+        result = Harness(tmp_path, repo).updater.run()
+        assert result['core_outcome'] == 'blocked'
+        assert 'store.js' in result['core_message']
+        assert repo.head() == old
+
+    @pytest.fixture
+    def core_update(self, monkeypatch):
+        """The real perform_core_update, pointed at a test clone."""
+        from web_interface.blueprints import api_v3 as pkg
+        from web_interface.blueprints.api_v3 import system
+
+        def point_at(device):
+            monkeypatch.setattr(system, 'PROJECT_ROOT', device)
+            monkeypatch.setattr(pkg.api_v3, 'plugin_store_manager', None, raising=False)
+            monkeypatch.setattr(system, '_pip_install_requirements',
+                                lambda *a, **k: pytest.fail('no requirements changed'))
+            return system.perform_core_update
+        return point_at
+
+    @pytest.mark.parametrize('change', [
+        'bundled_plugin_edit',
+        pytest.param('installer_chmod', marks=pytest.mark.skipif(
+            sys.platform == 'win32',
+            reason='no exec bit: the reset inside --autostash cannot clear a mode change, '
+                   'so git will not reapply it (on Linux it reapplies cleanly)')),
+    ])
+    def test_an_update_that_passed_the_preflight_leaves_no_stash(self, tmp_path, core_update, change):
+        """Found by audit: a bundled-plugin edit or the installer's chmods
+        passed the preflight, then perform_core_update stashed them for good.
+        Now --autostash carries them across the pull and puts them back."""
+        repo = _device_with_plugin_and_script(tmp_path)
+        new = repo.publish()
+        if change == 'installer_chmod':
+            _chmod_like_the_installer(repo.device)
+        else:
+            (repo.device / 'plugin-repos/web-ui-info/manager.py').write_text('x = 2  # store update\n')
+        h = Harness(tmp_path, repo, core_update=core_update(repo.device))
+        assert h.updater.preflight({})[0] == 'ready'
+
+        result = h.updater.run()
+        assert result['core_outcome'] == 'verifying', result['core_message']
+        assert repo.head() == new
+        assert git(repo.device, 'stash', 'list') == ''
+        if change == 'installer_chmod':
+            assert 'mode change 100755 => 100644 scripts/run.sh' in git(repo.device, 'diff', '--summary')
+        else:
+            assert 'store update' in (repo.device / 'plugin-repos/web-ui-info/manager.py').read_text()
+
+    def test_edits_made_after_the_preflight_are_refused_not_stashed(self, tmp_path, core_update):
+        repo = Repo(tmp_path)
+        old = repo.head()
+        repo.publish()
+        perform = core_update(repo.device)
+
+        def edit_then_update(**kwargs):
+            (repo.device / 'app.py').write_text('mine\n')
+            return perform(**kwargs)
+        result = Harness(tmp_path, repo, core_update=edit_then_update).updater.run()
+        assert result['core_outcome'] == 'blocked'
+        assert 'app.py' in result['core_message'] and 'will not stash' in result['core_message']
+        assert repo.head() == old
+        assert git(repo.device, 'stash', 'list') == ''
+        assert (repo.device / 'app.py').read_text() == 'mine\n'
+
+    def test_update_code_still_stashes_core_edits_but_not_plugin_folders(self, tmp_path, core_update):
+        repo = _device_with_plugin_and_script(tmp_path)
+        new = repo.publish()
+        (repo.device / 'scripts/run.sh').write_text('echo mine\n')
+        (repo.device / 'plugin-repos/web-ui-info/manager.py').write_text('x = 2  # store update\n')
+        payload = core_update(repo.device)()
+        assert payload['status'] == 'success', payload['message']
+        assert 'stashed' in payload['message']
+        assert repo.head() == new
+        assert 'LEDMatrix auto-stash before update' in git(repo.device, 'stash', 'list')
+        stashed = git(repo.device, 'stash', 'show', '--name-only', 'stash@{0}')
+        assert stashed.split() == ['scripts/run.sh']
+        assert 'store update' in (repo.device / 'plugin-repos/web-ui-info/manager.py').read_text()
+
+
 # -- the update and its hand-off to the health check ---------------------------
 
 @needs_git
@@ -395,7 +529,7 @@ class TestUpdateIsVerified:
         old = repo.head()
         repo.publish()
         h = Harness(tmp_path, repo,
-                    core_update=lambda: {'status': 'error', 'message': 'Update failed: network'})
+                    core_update=lambda **kw: {'status': 'error', 'message': 'Update failed: network'})
         result = h.updater.run()
         assert result['core_outcome'] == 'error'
         assert repo.head() == old
@@ -407,7 +541,7 @@ class TestUpdateIsVerified:
         repo.publish()
         pull = real_pull(repo.device)
 
-        def half_failed():
+        def half_failed(**kwargs):
             pull()
             return {'status': 'error', 'message': 'Update failed: interrupted'}
         result = Harness(tmp_path, repo, core_update=half_failed).updater.run()

web_interface/auto_update.py

@@ -263,6 +263,56 @@ def start_setup_if_needed(was_enabled, config):
     return 'Could not restart the display to finish automatic update setup; restart it from the Overview tab.'
 
 
+#: Top-level folders of separately installed plugins. Edits there are not the
+#: core's local changes: the Plugin Store updates plugins in place, including
+#: the bundled ones committed under plugin-repos/, and the pull's --autostash
+#: carries those edits across and reapplies them.
+SEPARATE_INSTALL_DIRS = ('plugins', 'plugin-repos')
+
+
+def local_changes(project_dir, run=None, timeout=30):
+    """Tracked files edited in the checkout, as both code updates count them.
+
+    The one definition shared by the automatic update's preflight and
+    ``perform_core_update`` (Update Code), so the preflight's promise that
+    nothing will be stashed holds for the pull that follows it.
+
+    * Mode-only changes do not count (``core.fileMode=false``): installers
+      chmod tracked scripts, and --autostash carries modes across the pull.
+    * Paths under ``SEPARATE_INSTALL_DIRS`` do not count. The match is on
+      the leading folder, not a substring, so
+      ``web_interface/static/v3/js/plugins/x.js`` is still a core edit.
+
+    Returns the changed paths, or None when git could not tell. Raises what
+    ``run`` raises (e.g. ``subprocess.TimeoutExpired``).
+    """
+    run = run or subprocess.run
+    result = run(['git', '-c', 'core.fileMode=false', 'status', '--porcelain',
+                  '--untracked-files=no', '-z'],
+                 cwd=str(project_dir), capture_output=True, text=True, timeout=timeout)
+    if result.returncode != 0:
+        return None
+    changed = []
+    entries = iter((result.stdout or '').split('\0'))
+    for entry in entries:
+        if len(entry) < 4:
+            continue
+        paths = [entry[3:]]
+        if entry[0] in 'RC' or entry[1] in 'RC':
+            paths.append(next(entries, ''))  # -z puts a rename's source next
+        if any(p and p.split('/', 1)[0] not in SEPARATE_INSTALL_DIRS for p in paths):
+            changed.append(paths[0])
+    return changed
+
+
+def describe_local_changes(changed):
+    """Why an automatic update refused to touch a checkout with local edits."""
+    example = f' (for example {changed[0]})' if changed else ''
+    return (f'{len(changed or ()) or "Some"} tracked file(s) in the LEDMatrix folder were edited '
+            f'locally{example}. Automatic updates will not stash your changes; '
+            'commit or revert them, or update manually with Update Code.')
+
+
 def _load_verifier(path):
     spec = importlib.util.spec_from_file_location('ledmatrix_auto_update_verifier', str(path))
     module = importlib.util.module_from_spec(spec)
@@ -429,17 +479,11 @@ class AutoUpdater:
             return 'blocked', ('The current branch has no upstream to update from (or HEAD is detached). '
                                'Use Update Code once, or Tools -> Switch branch.'), {}
 
-        # Mode-only changes are ignored: older installers chmod tracked
-        # scripts, and --autostash in the pull carries those across. Plugin
-        # folders are separate installs, as in perform_core_update.
-        status = self._git('-c', 'core.fileMode=false', 'status', '--porcelain', '--untracked-files=no')
-        changed = [line[3:] for line in status.stdout.splitlines()
-                   if line.strip() and 'plugins/' not in line and 'plugin-repos/' not in line]
-        if status.returncode != 0 or changed:
-            example = f' (for example {changed[0]})' if changed else ''
-            return 'blocked', (f'{len(changed) or "Some"} tracked file(s) in the LEDMatrix folder were edited '
-                               f'locally{example}. Automatic updates will not stash your changes; '
-                               'commit or revert them, or update manually with Update Code.'), {}
+        # The same predicate perform_core_update refuses on when called from
+        # here, so what passes this check is never stashed by the pull.
+        changed = local_changes(self.project_root, run=self.run_command)
+        if changed is None or changed:
+            return 'blocked', describe_local_changes(changed), {}
 
         free = self.disk_free(str(self.project_root))
         if free < MIN_FREE_BYTES:
@@ -484,11 +528,15 @@ class AutoUpdater:
             return {'outcome': 'error', 'message': f'Could not prepare the update health check: {e}.'}
 
         try:
-            core = self.core_update()
+            # Refuse rather than stash: edits made since the preflight are the
+            # user's, and nothing would ever restore a stash taken here.
+            core = self.core_update(stash_local_changes=False)
         except Exception as e:
             logger.exception("perform_core_update raised")
             core = {'status': 'error', 'message': f'Update failed: {e}'}
         new_head = self._git('rev-parse', 'HEAD').stdout.strip()
+        if core.get('local_changes') is not None and new_head == old_head:
+            return {'outcome': 'blocked', 'message': describe_local_changes(core['local_changes'])}
         pending = {
             'status': 'pending',
             'old_head': old_head,

web_interface/blueprints/api_v3/system.py

@@ -235,13 +235,17 @@ _core_update_lock = threading.Lock()
 CORE_REQUIREMENT_FILES = ('requirements.txt', 'web_interface/requirements.txt')
 
 
-def perform_core_update():
+def perform_core_update(stash_local_changes=True):
     """Pull the latest LEDMatrix code and sync its dependencies.
 
     Shared by the Overview "Update Code" button and the weekly automatic
     updater (web_interface/auto_update.py), so both take exactly the same
     path. Returns the JSON-able payload the button has always received:
     ``status``, ``message`` and ``restart_required``.
+
+    Update Code stashes local edits before pulling. The automatic updater
+    passes ``stash_local_changes=False``: then local edits make this return
+    an error carrying ``local_changes`` (the edited paths) without pulling.
     """
     # The button and the scheduler can fire together; two pulls racing
     # over one checkout (and one stash) is how local changes get lost.
@@ -249,12 +253,12 @@ def perform_core_update():
         return {'status': 'error', 'restart_required': False,
                 'message': 'An update is already in progress; try again shortly.'}
     try:
-        return _perform_core_update_locked()
+        return _perform_core_update_locked(stash_local_changes)
     finally:
         _core_update_lock.release()
 
 
-def _perform_core_update_locked():
+def _perform_core_update_locked(stash_local_changes=True):
     project_dir = str(PROJECT_ROOT)
 
     # Decide how to pull BEFORE stashing. If this checkout cannot be
@@ -265,37 +269,35 @@ def _perform_core_update_locked():
         logger.warning("git pull not attempted: %s", pull_error)
         return {'status': 'error', 'message': pull_error, 'restart_required': False}
 
-    # Check if there are local changes that need to be stashed
-    # Exclude plugins directory - plugins are separate repos and shouldn't be stashed with base project
-    # Use --untracked-files=no to skip untracked files check (much faster with symlinked plugins)
+    # Local changes, counted exactly as the automatic update's preflight
+    # counts them (auto_update.local_changes): mode-only changes and the
+    # plugin folders don't count, and the pull's --autostash carries those
+    # across and reapplies them.
+    from web_interface import auto_update
     try:
-        status_result = subprocess.run(
-            ['git', 'status', '--porcelain', '--untracked-files=no'],
-            capture_output=True,
-            text=True,
-            timeout=30,
-            cwd=project_dir
-        )
-        # Filter out any changes in plugins directory - plugins are separate repositories
-        # Git status format: XY filename (where X is status of index, Y is status of work tree)
-        status_lines = [line for line in status_result.stdout.strip().split('\n')
-                       if line.strip() and 'plugins/' not in line]
-        has_changes = bool('\n'.join(status_lines).strip())
-    except subprocess.TimeoutExpired:
-        # If status check times out, assume there might be changes and proceed
-        # This is safer than failing the update
-        has_changes = True
-        status_result = type('obj', (object,), {'stdout': '', 'stderr': 'Status check timed out'})()
+        changed = auto_update.local_changes(project_dir)
+    except (subprocess.SubprocessError, OSError) as status_err:
+        logger.warning("git status failed before pull: %s", status_err)
+        changed = None
+    # When git cannot say, assume there are changes rather than pull over them.
+    has_changes = changed is None or bool(changed)
+
+    if has_changes and not stash_local_changes:
+        # The automatic updater: it promised not to stash, and nothing would
+        # ever restore a stash taken on its behalf.
+        return {'status': 'error', 'restart_required': False, 'dependency_failures': [],
+                'local_changes': list(changed or []),
+                'message': auto_update.describe_local_changes(changed)}
 
     stash_info = ""
 
-    # Stash local changes if they exist (excluding plugins)
-    # Plugins are separate repositories and shouldn't be stashed with base project updates
+    # Stash local changes if they exist. The plugin folders are left out:
+    # plugins are separate installs, and --autostash carries their edits.
     if has_changes:
         try:
-            # Use pathspec to exclude plugins directory from stash
             stash_result = subprocess.run(
-                ['git', 'stash', 'push', '-m', 'LEDMatrix auto-stash before update', '--', ':!plugins'],
+                ['git', 'stash', 'push', '-m', 'LEDMatrix auto-stash before update', '--',
+                 *(f':!{folder}' for folder in auto_update.SEPARATE_INSTALL_DIRS)],
                 capture_output=True,
                 text=True,
                 timeout=30,