fix(plugins): keep a plugin's tokens and local files across store updates

CHANGELOG.md

@@ -1417,6 +1417,17 @@ read any of them:
 
 ### Fixes
 
+- Updating a plugin from the store no longer deletes the files it wrote
+  beside itself. A monorepo update replaces the plugin directory with the
+  fresh download and deletes the old copy, so calendar's Google OAuth files
+  (`token.pickle`, `credentials.json`) were lost on every update and the
+  calendar stopped until they were restored by hand. Before the old copy is
+  removed, the update now copies over anything the plugin's `.gitignore`
+  excludes plus known secret/state files (`*.pickle`, `token.json`,
+  `credentials.json`, `config_secrets.json`, `.pkce_code_verifier`); files the
+  new release ships are never overwritten, and byte code is not carried. A
+  plugin updated with `git pull` no longer sweeps an untracked token into the
+  auto-stash, which is never popped (`src/plugin_system/plugin_local_files.py`).
 - Quieter routine logging. Every rotation logged each mode twice
   ("Switching to mode", then "Processing mode"), and a mode with nothing to
   show added "display() returned False" and "No content to display". Those

src/plugin_system/plugin_local_files.py

@@ -0,0 +1,204 @@
+"""
+Files a plugin writes beside itself at runtime, which an update must keep.
+
+A store update replaces a plugin's directory with a fresh download and then
+deletes the old copy. Anything the plugin created there -- OAuth tokens, a
+client-secrets file, a PKCE verifier, cached state -- is in no release, so the
+fresh download does not contain it and deleting the old copy destroys it. On
+2026-10-04 updating calendar 1.2.9 -> 1.2.12 that way deleted its
+``token.pickle`` and ``credentials.json``, and the calendar stopped until they
+were restored from a backup.
+
+What counts as "the plugin's own local file" is the union of:
+
+* :data:`KNOWN_STATE_PATTERNS` -- secret and state files plugins are known to
+  write, kept even when a plugin forgot to gitignore them; and
+* whatever the plugin's own ``.gitignore`` (old copy or new) excludes. A file
+  the author ignores is by definition not part of a release.
+
+A file the new release ships is never overwritten: tracked content wins. Byte
+code (``__pycache__``, ``*.pyc``) and ``.git`` are never carried, since they
+belong to the old code rather than to the user.
+"""
+
+from __future__ import annotations
+
+import fnmatch
+import os
+import re
+import shutil
+from pathlib import Path
+from typing import Iterable, List, Optional, Pattern, Tuple
+
+__all__ = [
+    'KNOWN_STATE_PATTERNS',
+    'carry_over_local_files',
+    'is_known_state_file',
+    'local_files_to_keep',
+]
+
+# Basename globs. Kept even when the plugin's .gitignore does not list them.
+KNOWN_STATE_PATTERNS: Tuple[str, ...] = (
+    'token.pickle',
+    '*.pickle',
+    'token.json',
+    'credentials.json',
+    'config_secrets.json',
+    '.pkce_code_verifier',
+)
+
+_NEVER_CARRY_DIRS = frozenset({'.git', '__pycache__'})
+_NEVER_CARRY_SUFFIXES = ('.pyc', '.pyo')
+
+
+def is_known_state_file(rel_path: str) -> bool:
+    """True when ``rel_path``'s basename is a known secret/state file."""
+    name = rel_path.replace('\\', '/').rsplit('/', 1)[-1]
+    return any(fnmatch.fnmatchcase(name, p) for p in KNOWN_STATE_PATTERNS)
+
+
+class _GitIgnore:
+    """The subset of gitignore semantics plugin .gitignore files use.
+
+    Supports comments, ``!`` negation (last match wins), a trailing ``/`` for
+    directory-only patterns, anchoring by a leading or embedded ``/``, ``*``,
+    ``?``, ``[...]`` and ``**``. As in git, a file under an ignored directory
+    is ignored regardless of later negations.
+    """
+
+    def __init__(self, lines: Iterable[str]):
+        self._rules: List[Tuple[Pattern[str], bool, bool]] = []
+        for raw in lines:
+            line = raw.rstrip('\n').rstrip()
+            if not line or line.startswith('#'):
+                continue
+            negate = line.startswith('!')
+            if negate:
+                line = line[1:]
+            elif line.startswith('\\'):
+                line = line[1:]
+            dir_only = line.endswith('/')
+            line = line.rstrip('/')
+            if not line:
+                continue
+            anchored = '/' in line
+            line = line.lstrip('/')
+            body = self._translate(line)
+            regex = body if anchored else r'(?:.*/)?' + body
+            self._rules.append((re.compile(r'\A' + regex + r'\Z'), negate, dir_only))
+
+    @staticmethod
+    def _translate(pattern: str) -> str:
+        out, i, n = [], 0, len(pattern)
+        while i < n:
+            if pattern.startswith('**/', i):
+                out.append(r'(?:.*/)?')
+                i += 3
+            elif pattern.startswith('/**', i) and i + 3 == n:
+                out.append(r'/.*')
+                i += 3
+            elif pattern.startswith('**', i):
+                out.append(r'.*')
+                i += 2
+            elif pattern[i] == '*':
+                out.append(r'[^/]*')
+                i += 1
+            elif pattern[i] == '?':
+                out.append(r'[^/]')
+                i += 1
+            elif pattern[i] == '[':
+                end = pattern.find(']', i + 1)
+                if end == -1:
+                    out.append(re.escape('['))
+                    i += 1
+                else:
+                    cls = pattern[i + 1:end]
+                    if cls.startswith('!'):
+                        cls = '^' + cls[1:]
+                    out.append('[' + cls.replace('\\', '\\\\') + ']')
+                    i = end + 1
+            else:
+                out.append(re.escape(pattern[i]))
+                i += 1
+        return ''.join(out)
+
+    def _decide(self, rel: str, is_dir: bool) -> Optional[bool]:
+        verdict = None
+        for regex, negate, dir_only in self._rules:
+            if dir_only and not is_dir:
+                continue
+            if regex.match(rel):
+                verdict = not negate
+        return verdict
+
+    def ignores(self, rel_path: str) -> bool:
+        if not self._rules:
+            return False
+        parts = rel_path.replace('\\', '/').split('/')
+        for depth in range(1, len(parts)):
+            if self._decide('/'.join(parts[:depth]), True):
+                return True
+        return bool(self._decide('/'.join(parts), False))
+
+
+def _read_gitignore(plugin_dir: Path) -> List[str]:
+    try:
+        return (plugin_dir / '.gitignore').read_text(
+            encoding='utf-8', errors='replace').splitlines()
+    except OSError:
+        return []
+
+
+def local_files_to_keep(old_dir: Path, new_dir: Path) -> List[str]:
+    """Relative paths (``/``-separated) in ``old_dir`` to copy into ``new_dir``.
+
+    Regular files only; symlinks and anything the new release already ships
+    are skipped.
+    """
+    old_dir, new_dir = Path(old_dir), Path(new_dir)
+    ignore = _GitIgnore(_read_gitignore(old_dir) + _read_gitignore(new_dir))
+    keep: List[str] = []
+    for root, dirs, files in os.walk(old_dir):
+        dirs[:] = sorted(d for d in dirs if d not in _NEVER_CARRY_DIRS
+                         and not os.path.islink(os.path.join(root, d)))
+        rel_root = os.path.relpath(root, old_dir)
+        for name in sorted(files):
+            if name.endswith(_NEVER_CARRY_SUFFIXES):
+                continue
+            full = os.path.join(root, name)
+            if os.path.islink(full) or not os.path.isfile(full):
+                continue
+            rel = name if rel_root == '.' else f"{rel_root}/{name}".replace('\\', '/')
+            if not (is_known_state_file(rel) or ignore.ignores(rel)):
+                continue
+            if os.path.lexists(new_dir / rel):
+                continue
+            keep.append(rel)
+    return keep
+
+
+def carry_over_local_files(
+    old_dir: Path, new_dir: Path
+) -> Tuple[List[str], List[Tuple[str, str]]]:
+    """Copy the plugin's local files from ``old_dir`` into ``new_dir``.
+
+    Copies rather than moves, so ``old_dir`` stays a complete copy until the
+    caller deletes it. Returns ``(copied, failed)`` where ``failed`` pairs a
+    relative path with the error; the caller should keep ``old_dir`` when
+    anything failed.
+    """
+    copied: List[str] = []
+    failed: List[Tuple[str, str]] = []
+    try:
+        candidates = local_files_to_keep(old_dir, new_dir)
+    except OSError as e:
+        return copied, [('.', str(e))]
+    for rel in candidates:
+        dest = Path(new_dir) / rel
+        try:
+            dest.parent.mkdir(parents=True, exist_ok=True)
+            shutil.copy2(Path(old_dir) / rel, dest)
+            copied.append(rel)
+        except OSError as e:
+            failed.append((rel, str(e)))
+    return copied, failed

src/plugin_system/store_install.py

@@ -22,6 +22,7 @@
     contained_plugin_dir, requirements_to_install,
 )
 from src.plugin_system.plugin_dirs import BACKUP_MARKER
+from src.plugin_system.plugin_local_files import carry_over_local_files
 from src.plugin_system.repo_urls import (
     USER_AGENT, github_api_headers, github_owner_repo, normalize_repo_url,
 )
@@ -92,7 +93,9 @@ def install_plugin(self, plugin_id: str, branch: Optional[str] = None) -> bool:
                 raise
 
             if installed:
-                self._discard_backup(plugin_id, backup_path, "install")
+                self._discard_backup(
+                    plugin_id, backup_path, "install",
+                    new_path=self._existing_install(plugin_id) or plugin_path)
                 return True
 
             self._restore_backup(plugin_id, plugin_path, backup_path, "Install")
@@ -133,8 +136,33 @@ def _set_aside(self, plugin_path: Path, backup_path: Path) -> Optional[str]:
             return f"could not set aside {plugin_path}: {e}"
         return None
 
-    def _discard_backup(self, plugin_id: str, backup_path: Path, action: str) -> None:
-        """Remove the set-aside copy after a successful (re)install."""
+    def _discard_backup(
+        self, plugin_id: str, backup_path: Path, action: str,
+        new_path: Optional[Path] = None,
+    ) -> None:
+        """Remove the set-aside copy after a successful (re)install.
+
+        With ``new_path`` (where the new copy landed), first carries the
+        plugin's own runtime files -- OAuth tokens, client secrets, anything
+        its .gitignore excludes -- from the old copy into the new one: no
+        release contains them, so deleting the old copy would destroy them.
+        See src/plugin_system/plugin_local_files.py. If any could not be
+        copied the old copy is kept, so nothing is lost.
+        """
+        if new_path is not None and new_path.is_dir():
+            copied, failed = carry_over_local_files(backup_path, new_path)
+            if copied:
+                self.logger.info(
+                    "Kept %d local file(s) of %s across the %s: %s",
+                    len(copied), plugin_id, action, ", ".join(copied))
+            if failed:
+                self.logger.error(
+                    "Could not carry %s's local files into the new copy (%s); "
+                    "the previous copy is kept at %s -- copy them back by hand",
+                    plugin_id,
+                    "; ".join(f"{rel}: {err}" for rel, err in failed),
+                    backup_path)
+                return
         if not self._safe_remove_directory(backup_path):
             self.logger.warning(
                 "%s of %s succeeded but the previous copy at %s could not be "
@@ -542,7 +570,8 @@ def install_from_url(self, repo_url: str, plugin_id: str = None, plugin_path: st
                     raise
                 temp_dir = None  # Prevent cleanup since we moved it
                 if backup_path is not None:
-                    self._discard_backup(plugin_id, backup_path, "install")
+                    self._discard_backup(
+                        plugin_id, backup_path, "install", new_path=final_path)
 
             # Install dependencies
             self._install_dependencies(final_path)

src/plugin_system/store_update.py

@@ -10,6 +10,9 @@
 from pathlib import Path
 from typing import Dict, Optional, Tuple
 from src.plugin_system.plugin_dirs import BACKUP_MARKER
+from src.plugin_system.plugin_local_files import (
+    KNOWN_STATE_PATTERNS, is_known_state_file,
+)
 from src.plugin_system.repo_urls import same_repo
 
 
@@ -302,7 +305,11 @@ def _reinstall_with_rollback(self, plugin_id: str, plugin_path: Path) -> bool:
                 installed = False
 
             if installed:
-                self._discard_backup(plugin_id, backup_path, "update")
+                # install_plugin may land the new copy under the manifest id
+                # rather than the old directory name.
+                self._discard_backup(
+                    plugin_id, backup_path, "update",
+                    new_path=self._existing_install(plugin_id) or plugin_path)
                 return True
 
             # Bad network, registry error...: the user keeps a working plugin.
@@ -509,8 +516,12 @@ def update_plugin(self, plugin_id: str) -> bool:
                             for line in untracked_result.stdout.strip().split('\n'):
                                 if line.startswith('??'):
                                     # Untracked file
-                                    file_path = line[3:].strip()
-                                    untracked_files.append(file_path)
+                                    file_path = line[3:].strip().strip('"')
+                                    # Tokens and secrets stay out of the
+                                    # stash (see below), so they alone are
+                                    # not a reason to stash.
+                                    if not is_known_state_file(file_path):
+                                        untracked_files.append(file_path)
 
                         # Check for tracked file changes
                         status_result = subprocess.run(
@@ -537,9 +548,17 @@ def update_plugin(self, plugin_id: str) -> bool:
                     if has_changes:
                         self.logger.info(f"Stashing local changes in {plugin_id} before update")
                         try:
-                            # Use -u to include untracked files in stash
+                            # Use -u to include untracked files in stash --
+                            # except the plugin's tokens and secrets, which a
+                            # repo may have forgotten to gitignore. The stash
+                            # is never popped, so a stashed token.pickle would
+                            # vanish from the plugin and break it.
+                            stash_cmd = (
+                                ['git', '-C', str(plugin_path), 'stash', 'push', '-u',
+                                 '-m', f'LEDMatrix auto-stash before update {plugin_id}', '--', '.']
+                                + [f':(exclude,glob)**/{p}' for p in KNOWN_STATE_PATTERNS])
                             stash_result = subprocess.run(
-                                ['git', '-C', str(plugin_path), 'stash', 'push', '-u', '-m', f'LEDMatrix auto-stash before update {plugin_id}'],
+                                stash_cmd,
                                 capture_output=True,
                                 text=True,
                                 timeout=30,

test/test_on_demand_live_and_restore.py

@@ -21,6 +21,18 @@
                 'ncaa_fb_live', 'ncaa_fb_recent', 'ncaa_fb_upcoming']
 
 
+def _last_write(cache_manager, key):
+    """The last ``cache_manager.set(key, ...)`` call.
+
+    Not simply the last ``set`` call: the controller's font-usage publisher
+    thread writes ``font_usage_snapshot`` to the same cache manager whenever
+    it wakes, so on a slow runner it can land after the write under test.
+    """
+    writes = [c for c in cache_manager.set.call_args_list if c.args and c.args[0] == key]
+    assert writes, f"nothing was written to {key!r}"
+    return writes[-1]
+
+
 def _sports_plugin(has_live_content=False):
     plugin = MagicMock(spec=['display', 'has_live_content', 'has_live_priority',
                              'get_live_modes'])
@@ -87,8 +99,7 @@ def test_a_named_second_live_mode_with_content_leads(self, test_display_controll
     def test_the_named_mode_survives_a_restart(self, football):
         football._activate_on_demand({'plugin_id': 'football-scoreboard',
                                       'mode': 'ncaa_fb_live'})
-        saved = football.cache_manager.set.call_args_list[-1]
-        assert saved.args[0] == 'display_on_demand_config'
+        saved = _last_write(football.cache_manager, 'display_on_demand_config')
         config = saved.args[1]
         assert config['named_mode'] == 'ncaa_fb_live'
 
@@ -120,10 +131,7 @@ def test_the_session_ends(self, restored):
     def test_it_is_reported_as_an_error(self, restored):
         assert restored.on_demand_status == 'error'
         assert restored.on_demand_last_error == 'restore-failed'
-        # The last on-demand state write, not the last write of any key: the
-        # font-usage publisher thread writes its own key at its own pace.
-        published = [c for c in restored.cache_manager.set.call_args_list
-                     if c.args and c.args[0] == 'display_on_demand_state'][-1]
+        published = _last_write(restored.cache_manager, 'display_on_demand_state')
         assert published.args[1]['status'] == 'error'
         assert published.args[1]['error'] == 'restore-failed'