feat(install): updates refresh systemd units; new installs run the newest release (#729)

Updates that move HEAD now install changed systemd units through a root-owned helper (/usr/local/sbin/ledmatrix-refresh-units, two literal sudo lines), with a backup restored on rollback; a refresh that fails part-way puts the old units back. Devices without the new sudo rule keep updating and are told to re-run the installer once. The one-shot installer now checks out the newest vX.Y.Z release (LEDMATRIX_CHANNEL=beta keeps main) and never moves an existing checkout backwards.

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

test/conftest.py

@@ -328,6 +328,21 @@ def _hermetic_control_socket(monkeypatch):
     monkeypatch.setenv(SOCKET_PATH_ENV, 'off')
 
 
+@pytest.fixture(autouse=True)
+def _hermetic_unit_refresh(monkeypatch, tmp_path_factory):
+    """Keep updates' systemd unit refresh off the host.
+
+    perform_core_update runs web_interface/unit_refresh.py after any update
+    that moves HEAD, and several tests run the real one against a test clone.
+    On a device -- or a machine where install_service.sh was tried out -- it
+    would compare the clone's templates with the real /etc/systemd/system and
+    run the real sudo helper. Point it at a folder that does not exist: no units
+    installed, nothing to do. The unit refresh tests pass their own.
+    """
+    from web_interface import unit_refresh
+    monkeypatch.setattr(unit_refresh, 'SYSTEMD_DIR', str(tmp_path_factory.getbasetemp() / 'no-systemd'))
+
+
 @pytest.fixture(autouse=True)
 def reset_logging():
     """Reset logging configuration before each test."""

test/test_auto_update.py

@@ -517,6 +517,32 @@ class TestUpdateIsVerified:
         h.updater.run()
         assert h.pending['dependency_failures'] == ['requirements.txt']
 
+    @pytest.mark.parametrize('unit_refresh, expected', [
+        ({'status': 'refreshed', 'message': '', 'units': ['ledmatrix.service']}, True),
+        ({'status': 'needs_reinstall', 'message': '', 'units': ['ledmatrix.service']}, False),
+        (None, False),
+    ])
+    def test_the_health_check_learns_whether_the_update_installed_units(self, tmp_path, unit_refresh,
+                                                                          expected):
+        """Its rollback restores the previous units only when this update replaced them."""
+        repo = Repo(tmp_path)
+        repo.publish()
+        h = Harness(tmp_path, repo, core_update=real_pull(repo.device, unit_refresh=unit_refresh))
+        h.updater.run()
+        assert h.pending['units_refreshed'] is expected
+
+    def test_a_health_check_that_never_starts_also_restores_the_units(self, tmp_path):
+        repo = Repo(tmp_path)
+        old = repo.head()
+        repo.publish()
+        refreshed = {'status': 'refreshed', 'message': '', 'units': ['ledmatrix.service']}
+        h = Harness(tmp_path, repo, pickup=False,
+                    core_update=real_pull(repo.device, unit_refresh=refreshed))
+        h.updater.run()
+        assert repo.head() == old
+        assert [a for a in h.sudo if a[-1] == '--restore'] == [
+            ['sudo', '-n', '/usr/local/sbin/ledmatrix-refresh-units', '--restore']]
+
     def test_a_health_check_that_never_starts_means_the_update_is_undone(self, tmp_path):
         repo = Repo(tmp_path)
         old = repo.head()

test/test_auto_update_verify.py

@@ -80,6 +80,8 @@ class FakeHost:
         self.nrestarts = 0
         self.heartbeat = heartbeat
         self.display_started_at = -1000.0  # the pre-update display, long running
+        self.unit_restores = []  # (argv, commit checked out, restarts so far)
+        self.restore_ok = True
 
     def broken(self, kind):
         if self.running_head is None:
@@ -103,6 +105,10 @@ class FakeHost:
             if self.pip:
                 return self.pip(args, self)
             return done(args, rc=0 if self.pip_ok else 1)
+        if args[:3] == ['sudo', '-n', av.REFRESH_UNITS_PATH]:
+            self.unit_restores.append((list(args), git(self.repo, 'rev-parse', 'HEAD'),
+                                       len(self.restarts)))
+            return done(args, rc=0 if self.restore_ok else 1)
         if args[:4] == ['sudo', '-n', 'systemctl', 'restart']:
             if self.restart_failures:
                 self.restart_failures -= 1
@@ -405,3 +411,45 @@ def test_the_heartbeat_location_and_freshness_match_the_display():
     # window it has to stay healthy for.
     assert av.HEARTBEAT_FRESH_SECONDS + av.POLL_SECONDS < av.STABLE_SECONDS
     assert av.HEARTBEAT_FRESH_SECONDS > display_watchdog.BEAT_INTERVAL_SECONDS * 2
+
+
+# -- systemd units the update installed ------------------------------------------
+
+def test_a_rollback_restores_the_units_the_update_installed(tmp_path):
+    """The update installed new units (web_interface/unit_refresh.py); the
+    rollback puts the old ones back before restarting onto the old code."""
+    code, result, host, head, old, new = check(tmp_path, 'display_down', units_refreshed=True)
+    assert result['status'] == 'rolled_back' and head == old
+    assert len(host.unit_restores) == 1
+    argv, commit, restarts_before = host.unit_restores[0]
+    assert argv == ['sudo', '-n', av.REFRESH_UNITS_PATH, '--restore']
+    assert commit == old, 'restored after the code was rolled back'
+    assert restarts_before == 2, 'restored before the services restart onto the old code'
+    assert host.restarts[-2:] == [('ledmatrix.service', old), ('ledmatrix-web.service', old)]
+    assert result['detail'] is None
+
+
+@pytest.mark.parametrize('pending', [{}, {'units_refreshed': False}])
+def test_a_rollback_leaves_units_alone_when_the_update_did_not_change_them(tmp_path, pending):
+    # {} is what an updater from before this change writes.
+    code, result, host, head, old, new = check(tmp_path, 'display_down', **pending)
+    assert result['status'] == 'rolled_back' and host.unit_restores == []
+
+
+def test_a_healthy_update_keeps_its_new_units(tmp_path):
+    code, result, host, head, old, new = check(tmp_path, units_refreshed=True)
+    assert result['status'] == 'success' and host.unit_restores == []
+
+
+def test_a_failed_unit_restore_is_reported_but_the_rollback_stands(tmp_path):
+    repo, old, new = updated_repo(tmp_path)
+    av.write_pending(av.pending_path(repo), {'status': 'pending', 'old_head': old, 'new_head': new,
+                                             'display_was_active': True, 'dependency_failures': [],
+                                             'units_refreshed': True})
+    host = FakeHost(repo, new, 'display_down')
+    host.restore_ok = False
+    host.verifier().verify()
+    result = av.read_pending(av.pending_path(repo))
+    assert result['status'] == 'rolled_back'
+    assert git(repo, 'rev-parse', 'HEAD') == old
+    assert 'install_service.sh' in result['detail']

test/test_install_release_checkout.py

@@ -0,0 +1,370 @@
+"""New installs run the newest release; re-running the installer never moves backwards.
+
+#684 made devices update along a channel -- stable follows the newest vX.Y.Z
+tag, beta follows main -- but a new install still cloned main's tip, so it ran
+unreleased code until the next release caught up with it. The one-shot
+installer (scripts/install/one-shot-install.sh, which is where the clone
+happens) now checks out the newest release after cloning, unless
+LEDMATRIX_CHANNEL=beta. Re-running it on an existing checkout moves a stable
+device forward to the newest release only when that release contains its
+commit, as update_channel.checkout_release() does, and leaves beta devices
+(and stable ones newer than every release) on the fast-forward pull they
+always had. first_time_install.sh writes an explicitly chosen channel
+(--beta / LEDMATRIX_CHANNEL) into config.json.
+
+These run the installer's own bash, under its strict mode, against real git
+repositories.
+"""
+import json
+import re
+import subprocess
+import sys
+from pathlib import Path
+
+import pytest
+
+ROOT = Path(__file__).resolve().parent.parent
+sys.path.insert(0, str(ROOT))
+ONE_SHOT = ROOT / "scripts" / "install" / "one-shot-install.sh"
+INSTALLER = ROOT / "first_time_install.sh"
+BEGIN = "# --- release checkout helpers"
+END = "# --- end release checkout helpers"
+
+from web_interface import update_channel  # noqa: E402
+
+pytestmark = pytest.mark.skipif(
+    not sys.platform.startswith("linux"), reason="runs the installer's bash under Linux"
+)
+
+
+def helper_block() -> str:
+    text = ONE_SHOT.read_text(encoding="utf-8")
+    assert text.count(BEGIN) == 1 and text.count(END) == 1, "helper block markers missing or duplicated"
+    return text[text.index(BEGIN): text.index(END)]
+
+
+def git(*args, cwd, env):
+    result = subprocess.run(["git", *args], cwd=cwd, env=env, capture_output=True, text=True)
+    assert result.returncode == 0, f"git {' '.join(args)} failed: {result.stderr}"
+    return result.stdout.strip()
+
+
+@pytest.fixture
+def git_env(tmp_path):
+    config = tmp_path / "gitconfig"
+    config.write_text(
+        "[user]\n\tname = t\n\temail = t@t\n"
+        "[protocol \"file\"]\n\tallow = always\n"
+        "[init]\n\tdefaultBranch = main\n"
+        "[advice]\n\tdetachedHead = false\n",
+        encoding="utf-8",
+    )
+    return {
+        "PATH": "/usr/bin:/bin:/usr/sbin:/sbin",
+        "HOME": str(tmp_path),
+        "GIT_CONFIG_GLOBAL": str(config),
+        "GIT_CONFIG_NOSYSTEM": "1",
+    }
+
+
+#: Tags and the commit (index into the history) each points at. The newest
+#: release is v3.10.0: 10 > 8 numerically, the rc and the zero-padded tag are
+#: not releases, and v3.12 and nightly are not vX.Y.Z at all.
+TAGS = {
+    "v3.7.0": 0, "v3.8.0": 1, "v3.10.0": 2,
+    "v3.11.0-rc1": 3, "v03.12.0": 3, "v3.12": 3, "nightly": 3,
+}
+NEWEST = "v3.10.0"
+
+
+@pytest.fixture
+def origin(tmp_path, git_env):
+    """A stand-in for GitHub: five commits on main (the last newer than any release)."""
+    seed = tmp_path / "seed"
+    seed.mkdir()
+    git("init", "-q", ".", cwd=seed, env=git_env)
+    commits = []
+    for i in range(5):
+        (seed / "version.txt").write_text(str(i), encoding="utf-8")
+        git("add", ".", cwd=seed, env=git_env)
+        git("commit", "-qm", f"c{i}", cwd=seed, env=git_env)
+        commits.append(git("rev-parse", "HEAD", cwd=seed, env=git_env))
+    for tag, index in TAGS.items():
+        git("tag", tag, commits[index], cwd=seed, env=git_env)
+    bare = tmp_path / "origin.git"
+    git("clone", "-q", "--bare", str(seed), str(bare), cwd=tmp_path, env=git_env)
+    return bare, commits, seed
+
+
+def run_block(snippet, cwd, env, channel=None):
+    env = dict(env)
+    if channel is not None:
+        env["LEDMATRIX_CHANNEL"] = channel
+    script = (
+        "set -Eeuo pipefail\n"
+        "trap 'echo ERR_TRAP_FIRED >&2; exit 99' ERR\n"
+        'print_success() { echo "OK: $*"; }\n'
+        'print_warning() { echo "W: $*"; }\n'
+        f"{helper_block()}\n"
+        f"{snippet}\n"
+    )
+    result = subprocess.run(["bash", "-c", script], cwd=cwd, capture_output=True, text=True, env=env)
+    assert "ERR_TRAP_FIRED" not in result.stderr, result.stdout + result.stderr
+    return result
+
+
+def clone(origin, tmp_path, env, name="LEDMatrix"):
+    bare, _, _ = origin
+    target = tmp_path / name
+    git("clone", "-q", str(bare), str(target), cwd=tmp_path, env=env)
+    return target
+
+
+def head(repo, env):
+    return git("rev-parse", "HEAD", cwd=repo, env=env)
+
+
+def branch(repo, env):
+    result = subprocess.run(["git", "symbolic-ref", "--quiet", "--short", "HEAD"], cwd=repo, env=env,
+                            capture_output=True, text=True)
+    return result.stdout.strip()
+
+
+def set_channel(repo, channel):
+    (repo / "config").mkdir(exist_ok=True)
+    (repo / "config" / "config.json").write_text(
+        json.dumps({"auto_update": {"enabled": False, "channel": channel}}), encoding="utf-8")
+
+
+# -- a fresh install -------------------------------------------------------------
+
+def test_a_fresh_clone_checks_out_the_newest_release(origin, tmp_path, git_env):
+    _, commits, _ = origin
+    repo = clone(origin, tmp_path, git_env)
+    out = run_block("_lm_checkout_release_after_clone", repo, git_env)
+    assert out.returncode == 0
+    assert head(repo, git_env) == commits[TAGS[NEWEST]]
+    assert branch(repo, git_env) == "", "a release is checked out detached, as Update Code does"
+    assert f"Installing release {NEWEST}" in out.stdout
+
+
+@pytest.mark.parametrize("channel", ["beta", "BETA", " beta "])
+def test_a_fresh_beta_install_stays_on_main(origin, tmp_path, git_env, channel):
+    _, commits, _ = origin
+    repo = clone(origin, tmp_path, git_env)
+    run_block("_lm_checkout_release_after_clone", repo, git_env, channel=channel)
+    assert head(repo, git_env) == commits[-1] and branch(repo, git_env) == "main"
+
+
+def test_an_unknown_channel_falls_back_to_stable_and_says_so(origin, tmp_path, git_env):
+    _, commits, _ = origin
+    repo = clone(origin, tmp_path, git_env)
+    out = run_block("_lm_checkout_release_after_clone", repo, git_env, channel="nightly")
+    assert head(repo, git_env) == commits[TAGS[NEWEST]]
+    assert "not stable or beta" in out.stdout + out.stderr
+
+
+def test_a_repository_without_releases_installs_main(tmp_path, git_env):
+    seed = tmp_path / "seed"
+    seed.mkdir()
+    git("init", "-q", ".", cwd=seed, env=git_env)
+    (seed / "f").write_text("x", encoding="utf-8")
+    git("add", ".", cwd=seed, env=git_env)
+    git("commit", "-qm", "only", cwd=seed, env=git_env)
+    git("tag", "v3.0", cwd=seed, env=git_env)  # not a release tag
+    repo = tmp_path / "LEDMatrix"
+    git("clone", "-q", str(seed), str(repo), cwd=tmp_path, env=git_env)
+    out = run_block("_lm_checkout_release_after_clone", repo, git_env)
+    assert branch(repo, git_env) == "main" and "No release found" in out.stdout
+
+
+# -- re-running on an existing checkout -----------------------------------------------
+
+def existing(origin, tmp_path, env, at, detached):
+    """An installed checkout, at commit index ``at``, on main or detached."""
+    _, commits, _ = origin
+    repo = clone(origin, tmp_path, env)
+    if detached:
+        git("checkout", "-q", "--detach", commits[at], cwd=repo, env=env)
+    else:
+        git("reset", "-q", "--hard", commits[at], cwd=repo, env=env)
+    return repo
+
+
+def update(repo, env, channel=None):
+    out = run_block("if _lm_update_existing_checkout; then echo RESULT=handled; "
+                    "else echo RESULT=pull; fi", repo, env, channel=channel)
+    return re.search(r"RESULT=(\w+)", out.stdout).group(1), out
+
+
+def test_a_device_on_an_older_release_moves_to_the_newest(origin, tmp_path, git_env):
+    _, commits, _ = origin
+    repo = existing(origin, tmp_path, git_env, at=TAGS["v3.7.0"], detached=True)
+    result, out = update(repo, git_env)
+    assert result == "handled"
+    assert head(repo, git_env) == commits[TAGS[NEWEST]]
+    assert f"Updated to release {NEWEST}" in out.stdout
+
+
+def test_a_device_already_on_the_newest_release_stays(origin, tmp_path, git_env):
+    _, commits, _ = origin
+    repo = existing(origin, tmp_path, git_env, at=TAGS[NEWEST], detached=True)
+    result, out = update(repo, git_env)
+    assert result == "handled" and head(repo, git_env) == commits[TAGS[NEWEST]]
+    assert "Already on the newest release" in out.stdout
+
+
+def test_a_device_on_main_behind_the_newest_release_moves_to_it(origin, tmp_path, git_env):
+    _, commits, _ = origin
+    repo = existing(origin, tmp_path, git_env, at=TAGS["v3.8.0"], detached=False)
+    result, _ = update(repo, git_env)
+    assert result == "handled" and head(repo, git_env) == commits[TAGS[NEWEST]]
+
+
+def test_a_device_on_main_newer_than_every_release_is_not_moved_back(origin, tmp_path, git_env):
+    """It keeps the fast-forward pull it always had, and waits for a release to contain it."""
+    _, commits, _ = origin
+    repo = existing(origin, tmp_path, git_env, at=4, detached=False)
+    result, _ = update(repo, git_env)
+    assert result == "pull"
+    assert head(repo, git_env) == commits[4] and branch(repo, git_env) == "main"
+
+
+def test_a_detached_device_newer_than_every_release_is_left_alone(origin, tmp_path, git_env):
+    _, commits, _ = origin
+    repo = existing(origin, tmp_path, git_env, at=3, detached=True)
+    result, out = update(repo, git_env)
+    assert result == "handled" and head(repo, git_env) == commits[3]
+    assert "newer than the newest release" in out.stdout
+
+
+def test_a_higher_version_on_an_older_commit_is_not_a_downgrade(origin, tmp_path, git_env):
+    """Newest by version is not newest by history: never move to a tag that does not contain HEAD."""
+    bare, commits, seed = origin
+    git("tag", "v9.0.0", commits[0], cwd=seed, env=git_env)
+    git("push", "-q", str(bare), "v9.0.0", cwd=seed, env=git_env)
+    repo = existing(origin, tmp_path, git_env, at=TAGS[NEWEST], detached=True)
+    result, _ = update(repo, git_env)
+    assert result == "handled" and head(repo, git_env) == commits[TAGS[NEWEST]]
+
+
+def test_a_new_release_published_since_the_clone_is_fetched(origin, tmp_path, git_env):
+    bare, commits, seed = origin
+    repo = existing(origin, tmp_path, git_env, at=TAGS[NEWEST], detached=True)
+    git("tag", "v3.11.0", commits[4], cwd=seed, env=git_env)
+    git("push", "-q", str(bare), "v3.11.0", cwd=seed, env=git_env)
+    result, _ = update(repo, git_env)
+    assert result == "handled" and head(repo, git_env) == commits[4]
+
+
+def test_a_beta_device_keeps_its_pull(origin, tmp_path, git_env):
+    _, commits, _ = origin
+    repo = existing(origin, tmp_path, git_env, at=TAGS["v3.8.0"], detached=False)
+    set_channel(repo, "beta")
+    result, _ = update(repo, git_env)
+    assert result == "pull" and head(repo, git_env) == commits[TAGS["v3.8.0"]]
+
+
+def test_the_environment_overrides_the_configured_channel(origin, tmp_path, git_env):
+    _, commits, _ = origin
+    repo = existing(origin, tmp_path, git_env, at=TAGS["v3.8.0"], detached=False)
+    set_channel(repo, "stable")
+    result, _ = update(repo, git_env, channel="beta")
+    assert result == "pull" and head(repo, git_env) == commits[TAGS["v3.8.0"]]
+
+
+def test_local_edits_that_block_the_move_keep_the_checkout(origin, tmp_path, git_env):
+    _, commits, _ = origin
+    repo = existing(origin, tmp_path, git_env, at=TAGS["v3.7.0"], detached=True)
+    (repo / "version.txt").write_text("my edit", encoding="utf-8")
+    result, out = update(repo, git_env)
+    assert result == "handled" and head(repo, git_env) == commits[TAGS["v3.7.0"]]
+    assert (repo / "version.txt").read_text(encoding="utf-8") == "my edit"
+    assert "Could not move to release" in out.stdout
+
+
+def test_an_unreachable_origin_keeps_the_checkout(origin, tmp_path, git_env):
+    _, commits, _ = origin
+    repo = existing(origin, tmp_path, git_env, at=TAGS["v3.7.0"], detached=True)
+    git("remote", "set-url", "origin", str(tmp_path / "gone.git"), cwd=repo, env=git_env)
+    result, out = update(repo, git_env)
+    assert result == "handled" and head(repo, git_env) == commits[TAGS["v3.7.0"]]
+    assert "Could not fetch" in out.stdout
+
+
+# -- same rules as the web interface -------------------------------------------------
+
+NAMES = ["v1.2.3", "v1.10.0", "v1.9.9", "v2.0.0-rc1", "v02.0.0", "v2.0", "v10.0.0", "v9.99.99",
+         "release-11", "v10.0.0+build", "v0.0.0", "v10.0.1", "v1.2.03", "V11.0.0"]
+
+
+@pytest.mark.parametrize("subset", [NAMES, NAMES[:4], ["v2.0", "nightly"], NAMES[::-1][:6]])
+def test_the_newest_tag_matches_update_channel(tmp_path, git_env, subset):
+    repo = tmp_path / "tags"
+    repo.mkdir()
+    git("init", "-q", ".", cwd=repo, env=git_env)
+    git("commit", "-q", "--allow-empty", "-m", "x", cwd=repo, env=git_env)
+    for name in subset:
+        git("tag", name, cwd=repo, env=git_env)
+    out = run_block("_lm_newest_release_tag", repo, git_env).stdout.strip()
+    assert out == (update_channel.newest_release_tag(subset) or "")
+
+
+# -- wiring --------------------------------------------------------------------------
+
+def test_every_clone_is_followed_by_the_release_checkout():
+    text = ONE_SHOT.read_text(encoding="utf-8")
+    clones = [m.start() for m in re.finditer(r'retry git clone "\$REPO_URL" "\$REPO_DIR"\n', text)]
+    assert clones
+    for pos in clones:
+        following = text[pos:].splitlines()[1]
+        assert '_lm_checkout_release_after_clone' in following, following
+
+
+def test_the_existing_checkout_is_handled_before_the_old_pull():
+    text = ONE_SHOT.read_text(encoding="utf-8")
+    assert re.search(r'if _lm_update_existing_checkout; then\n\s+PULL_SUCCESS=true\n'
+                     r'\s+elif git pull --ff-only origin "\$CURRENT_BRANCH"', text)
+
+
+def test_the_one_shot_passes_the_channel_to_the_installer():
+    text = ONE_SHOT.read_text(encoding="utf-8")
+    assert 'LEDMATRIX_CHANNEL="${LEDMATRIX_CHANNEL:-}"' in text
+
+
+# -- first_time_install.sh records the chosen channel ----------------------------------
+
+CHANNEL_BEGIN = 'case "$UPDATE_CHANNEL" in'
+CHANNEL_END = 'set it from the General tab instead"\n    fi\nfi\n'
+
+
+def channel_block():
+    text = INSTALLER.read_text(encoding="utf-8")
+    start = text.index(CHANNEL_BEGIN)
+    return text[start: text.index(CHANNEL_END, start) + len(CHANNEL_END)]
+
+
+@pytest.mark.parametrize("auto_update, channel, expected", [
+    ("", "beta", {"enabled": False, "channel": "beta"}),
+    ("", "stable", {"enabled": False, "channel": "stable"}),
+    ("1", "", {"enabled": True, "channel": "stable"}),
+    ("", "", {"enabled": False, "channel": "stable"}),       # nothing asked: untouched
+    ("", "nightly", {"enabled": False, "channel": "stable"}),  # nonsense: untouched
+])
+def test_the_installer_writes_only_an_explicit_channel(tmp_path, auto_update, channel, expected):
+    (tmp_path / "config").mkdir()
+    config = tmp_path / "config" / "config.json"
+    config.write_text(json.dumps({"auto_update": {"enabled": False, "channel": "stable"}, "x": 1}))
+    script = (f'set -Eeuo pipefail\nPROJECT_ROOT_DIR="{tmp_path}"\nAUTO_UPDATE="{auto_update}"\n'
+              f'UPDATE_CHANNEL="{channel}"\n{channel_block()}')
+    result = subprocess.run(["bash", "-c", script], capture_output=True, text=True)
+    assert result.returncode == 0, result.stdout + result.stderr
+    data = json.loads(config.read_text())
+    assert data["auto_update"] == expected and data["x"] == 1
+
+
+def test_the_installer_accepts_beta_as_a_flag_and_from_the_environment():
+    text = INSTALLER.read_text(encoding="utf-8")
+    assert re.search(r"^\s*--beta\) UPDATE_CHANNEL=beta ;;", text, re.M)
+    assert 'UPDATE_CHANNEL=$(printf \'%s\' "${LEDMATRIX_CHANNEL:-}"' in text
+    assert "LEDMATRIX_CHANNEL=stable|beta" in text, "documented in --help"

test/test_refresh_units.py

@@ -0,0 +1,525 @@
+"""Updates refresh the installed systemd units: the root helper and its callers.
+
+An update moved the checkout, and with it systemd/*.service, but systemd runs
+the copies in /etc/systemd/system, which only the installer wrote. So unit
+settings added after a device was installed (#687's render-loop watchdog)
+never reached it. scripts/install/ledmatrix_refresh_units.py, installed
+root-owned as /usr/local/sbin/ledmatrix-refresh-units and granted to the web
+user by exact command line, now installs changed units after an update, and
+puts the previous ones back when the automatic update rolls back.
+
+The helper runs as root on input the web user can edit (the templates), so
+most of these are about what it refuses.
+"""
+import importlib.util
+import json
+import os
+import re
+import shutil
+import subprocess
+import sys
+from pathlib import Path
+
+import pytest
+
+ROOT = Path(__file__).resolve().parent.parent
+sys.path.insert(0, str(ROOT))
+
+_spec = importlib.util.spec_from_file_location(
+    'ledmatrix_refresh_units', ROOT / 'scripts' / 'install' / 'ledmatrix_refresh_units.py')
+ru = importlib.util.module_from_spec(_spec)
+_spec.loader.exec_module(ru)
+
+from web_interface import unit_refresh  # noqa: E402
+
+try:
+    import pwd
+    # The web side checks the account exists, so use one that does.
+    WEB_USER = pwd.getpwuid(os.getuid()).pw_name
+except ImportError:  # Windows
+    WEB_USER = 'ledpi'
+
+# An account a hostile template switches the web unit to. Root, unless the
+# tests themselves run as root: then root *is* the web user and switching to
+# it changes nothing, so use another account.
+OTHER_USER = 'root' if WEB_USER != 'root' else 'nobody'
+
+
+class Host:
+    """A project checkout, an /etc/systemd/system, and a fake systemctl."""
+
+    def __init__(self, tmp_path, web_user=WEB_USER):
+        self.project = tmp_path / 'LEDMatrix'
+        shutil.copytree(ROOT / 'systemd', self.project / 'systemd')
+        self.systemd = tmp_path / 'etc-systemd-system'
+        self.systemd.mkdir()
+        self.backup = tmp_path / 'var-lib-ledmatrix' / 'unit-backup'
+        self.web_user = web_user
+        self.calls = []
+        self.path_active = True
+        self.root = True
+        for name in ru.UNITS:
+            self.install(name)
+
+    def template(self, name):
+        return (self.project / 'systemd' / name).read_text(encoding='utf-8')
+
+    def set_template(self, name, text):
+        (self.project / 'systemd' / name).write_text(text, encoding='utf-8', newline='\n')
+
+    def rendered(self, name, text=None):
+        user = 'root' if name == ru.DISPLAY_UNIT else self.web_user
+        return ru.render(text if text is not None else self.template(name), str(self.project), user)
+
+    def install(self, name, text=None):
+        (self.systemd / name).write_text(self.rendered(name, text), encoding='utf-8', newline='\n')
+
+    def installed(self, name):
+        path = self.systemd / name
+        return path.read_text(encoding='utf-8') if path.exists() else None
+
+    def run(self, args, **kwargs):
+        self.calls.append(list(args))
+        if args[:2] == ['systemctl', 'is-active']:
+            out = 'active\n' if self.path_active else 'inactive\n'
+            return subprocess.CompletedProcess(args, 0, stdout=out, stderr='')
+        return subprocess.CompletedProcess(args, 0, stdout='', stderr='')
+
+    def refresher(self, log=None):
+        return ru.Refresher(systemd_dir=str(self.systemd), backup_dir=str(self.backup), run=self.run,
+                            is_root=lambda: self.root, user_exists=lambda user: True,
+                            log=log or (lambda m: None))
+
+    @property
+    def reloads(self):
+        return self.calls.count(['systemctl', 'daemon-reload'])
+
+
+def watchdog_added(text):
+    """The kind of change #687 made: a new directive in [Service]."""
+    return text.replace('[Service]\n', '[Service]\nWatchdogSec=60\n', 1)
+
+
+@pytest.fixture
+def host(tmp_path):
+    return Host(tmp_path)
+
+
+# -- refresh ------------------------------------------------------------------
+
+def test_units_that_match_are_left_alone(host):
+    assert host.refresher().refresh() == []
+    assert host.reloads == 0
+
+
+def test_a_changed_template_is_installed_and_systemd_reloaded(host):
+    old = host.installed(ru.DISPLAY_UNIT)
+    host.set_template(ru.DISPLAY_UNIT, watchdog_added(host.template(ru.DISPLAY_UNIT)))
+
+    assert host.refresher().refresh() == [ru.DISPLAY_UNIT]
+    assert 'WatchdogSec=60' in host.installed(ru.DISPLAY_UNIT)
+    assert host.installed(ru.DISPLAY_UNIT) == host.rendered(ru.DISPLAY_UNIT)
+    assert host.reloads == 1
+    # Only the unit that changed is replaced, and the one it replaced is kept.
+    assert (host.backup / ru.DISPLAY_UNIT).read_text(encoding='utf-8') == old
+    assert json.loads((host.backup / ru.MANIFEST).read_text()) == {'units': [ru.DISPLAY_UNIT]}
+
+
+def test_the_web_unit_keeps_the_web_users_account(host):
+    host.set_template(ru.WEB_UNIT, watchdog_added(host.template(ru.WEB_UNIT)))
+    host.refresher().refresh()
+    assert ru.directive_values(host.installed(ru.WEB_UNIT), 'User') == [WEB_USER]
+    assert ru.directive_values(host.installed(ru.DISPLAY_UNIT), 'User') == ['root']
+
+
+def test_comment_only_changes_are_not_a_refresh(host):
+    host.set_template(ru.DISPLAY_UNIT, '# a new comment\n\n' + host.template(ru.DISPLAY_UNIT))
+    assert host.refresher().refresh() == []
+    assert host.reloads == 0
+
+
+def test_a_unit_that_was_never_installed_is_not_installed(host):
+    (host.systemd / ru.VERIFY_SERVICE).unlink()
+    (host.systemd / ru.VERIFY_PATH).unlink()
+    host.set_template(ru.VERIFY_SERVICE, watchdog_added(host.template(ru.VERIFY_SERVICE)))
+    host.refresher().refresh()
+    assert host.installed(ru.VERIFY_SERVICE) is None
+
+
+def test_a_changed_path_unit_is_restarted_so_it_watches_the_new_path(host):
+    host.set_template(ru.VERIFY_PATH, host.template(ru.VERIFY_PATH).replace(
+        '[Path]\n', '[Path]\nMakeDirectory=yes\n'))
+    host.refresher().refresh()
+    assert ['systemctl', 'restart', ru.VERIFY_PATH] in host.calls
+
+
+def test_it_must_run_as_root(host):
+    host.root = False
+    host.set_template(ru.DISPLAY_UNIT, watchdog_added(host.template(ru.DISPLAY_UNIT)))
+    with pytest.raises(ru.RefreshError, match='root'):
+        host.refresher().refresh()
+    assert 'WatchdogSec=60' not in host.installed(ru.DISPLAY_UNIT)
+
+
+def _failing_reload(host):
+    real = host.run
+    def run(args, **kwargs):
+        if args == ['systemctl', 'daemon-reload'] and host.reloads == 0:
+            host.calls.append(list(args))
+            return subprocess.CompletedProcess(args, 1, stdout='', stderr='boom')
+        return real(args, **kwargs)
+    return run
+
+
+def test_a_failed_daemon_reload_puts_the_old_units_back(host):
+    # The web side reports this as a failure and records no units_refreshed,
+    # so a rollback would not --restore: the helper must undo it itself.
+    old = host.installed(ru.DISPLAY_UNIT)
+    host.set_template(ru.DISPLAY_UNIT, watchdog_added(host.template(ru.DISPLAY_UNIT)))
+    host.run = _failing_reload(host)
+
+    with pytest.raises(ru.RefreshError):
+        host.refresher().refresh()
+    assert host.installed(ru.DISPLAY_UNIT) == old
+    assert host.reloads == 2  # the failed one, then one after putting it back
+    assert not (host.backup / ru.MANIFEST).exists()
+
+
+def test_a_failed_write_puts_back_the_units_already_written(host, monkeypatch):
+    old = {name: host.installed(name) for name in (ru.DISPLAY_UNIT, ru.WEB_UNIT)}
+    for name in old:
+        host.set_template(name, watchdog_added(host.template(name)))
+    refresher = host.refresher()
+    real_write = refresher._write_unit
+    writes = []
+
+    def write(name, text):
+        writes.append(name)
+        if len(writes) == 2:
+            raise OSError('disk full')
+        real_write(name, text)
+    monkeypatch.setattr(refresher, '_write_unit', write)
+
+    with pytest.raises(OSError):
+        refresher.refresh()
+    first = writes[0]
+    assert host.installed(first) == old[first]
+    assert all(host.installed(name) == old[name] for name in old)
+
+# -- what it refuses ------------------------------------------------------------
+
+@pytest.mark.parametrize('unit, edit', [
+    # The web interface's unit switched to root by a template edit.
+    (ru.WEB_UNIT, lambda t: t.replace('User=__USER__', f'User={OTHER_USER}')),
+    # The display's unit switched to another account.
+    (ru.DISPLAY_UNIT, lambda t: t.replace('User=root', 'User=nobody')),
+    # A second User= line.
+    (ru.VERIFY_SERVICE, lambda t: t.replace('[Service]\n', '[Service]\nUser=root\n', 1)),
+    # Run from somewhere else.
+    (ru.DISPLAY_UNIT, lambda t: t.replace('WorkingDirectory=__PROJECT_ROOT_DIR__', 'WorkingDirectory=/tmp')),
+    # A second User= written with spaces, which systemd accepts (last one wins).
+    # Placed in [Service] (before [Install]), where it is not a layout problem.
+    (ru.WEB_UNIT, lambda t: t.replace('\n[Install]', 'User = root\n\n[Install]')),
+    # The web user's User= moved to [Unit], where systemd ignores it (so root).
+    (ru.WEB_UNIT, lambda t: t.replace('User=__USER__\n', '').replace('[Unit]\n', '[Unit]\nUser=__USER__\n')),
+    # The User= line hidden inside a continued line, where systemd does not see it.
+    (ru.WEB_UNIT, lambda t: t.replace('User=__USER__\n', '').replace(
+        'Description=LED Matrix Web Interface Service\n',
+        'Description=LED Matrix Web Interface Service \\\\\nUser=__USER__\n')),
+    # A path unit that starts something else.
+    (ru.VERIFY_PATH, lambda t: t.replace('Unit=ledmatrix-update-verify.service', 'Unit=ledmatrix.service')),
+])
+def test_a_template_that_changes_who_or_where_is_refused_and_nothing_changes(host, unit, edit):
+    before = {name: host.installed(name) for name in ru.UNITS}
+    # A legitimate change alongside, which must not go in either.
+    host.set_template(ru.DISPLAY_UNIT, watchdog_added(host.template(ru.DISPLAY_UNIT)))
+    host.set_template(unit, edit(host.template(unit)))
+    with pytest.raises(ru.RefreshError):
+        host.refresher().refresh()
+    assert {name: host.installed(name) for name in ru.UNITS} == before
+    assert host.reloads == 0
+
+
+def test_the_project_folder_comes_from_the_installed_unit_not_the_caller(host, tmp_path):
+    # The installed display unit names the project; a WorkingDirectory that
+    # is not an existing absolute folder is refused before any template is read.
+    host.install(ru.DISPLAY_UNIT, host.template(ru.DISPLAY_UNIT).replace(
+        'WorkingDirectory=__PROJECT_ROOT_DIR__', 'WorkingDirectory=relative/path'))
+    with pytest.raises(ru.RefreshError, match='cannot be used'):
+        host.refresher().plan()
+
+
+def test_without_the_display_unit_installed_nothing_is_done(host):
+    (host.systemd / ru.DISPLAY_UNIT).unlink()
+    with pytest.raises(ru.RefreshError, match='not installed'):
+        host.refresher().plan()
+
+
+@pytest.mark.skipif(not hasattr(os, 'O_NOFOLLOW'), reason='POSIX only')
+def test_a_template_symlink_is_not_followed(host, tmp_path):
+    secret = tmp_path / 'secret'
+    secret.write_text(host.template(ru.DISPLAY_UNIT) + 'Environment=SECRET=1\n', encoding='utf-8')
+    target = host.project / 'systemd' / ru.DISPLAY_UNIT
+    target.unlink()
+    target.symlink_to(secret)
+    with pytest.raises(ru.RefreshError):
+        host.refresher().plan()
+
+
+@pytest.mark.skipif(not hasattr(os, 'O_NOFOLLOW'), reason='POSIX only')
+def test_a_symlinked_systemd_folder_is_not_followed(host, tmp_path):
+    elsewhere = tmp_path / 'elsewhere'
+    shutil.move(str(host.project / 'systemd'), str(elsewhere))
+    (host.project / 'systemd').symlink_to(elsewhere, target_is_directory=True)
+    with pytest.raises(ru.RefreshError):
+        host.refresher().plan()
+
+
+def test_an_oversized_template_is_refused(host):
+    host.set_template(ru.DISPLAY_UNIT, host.template(ru.DISPLAY_UNIT) + '#' * (ru.MAX_TEMPLATE_BYTES + 1))
+    with pytest.raises(ru.RefreshError, match='larger'):
+        host.refresher().plan()
+
+
+def test_main_leaves_the_callers_environment_alone(host):
+    """main() runs in-process in these tests; pinning PATH belongs to the installed program."""
+    before = os.environ.get('PATH')
+    ru.main(['ledmatrix-refresh-units', '--check'], refresher=host.refresher())
+    assert os.environ.get('PATH') == before
+
+
+@pytest.mark.parametrize('argv', [
+    ['--restore', 'x'], ['--refresh'], ['/etc/passwd'], ['--check', '--restore'], ['']])
+def test_any_other_command_line_is_refused(argv):
+    class Boom:
+        def __getattr__(self, name):
+            raise AssertionError('must not run')
+    assert ru.main(['ledmatrix-refresh-units', *argv], refresher=Boom()) == ru.EXIT_USAGE
+
+
+def test_main_reports_a_refusal_as_a_failure(host, capsys):
+    host.set_template(ru.WEB_UNIT, host.template(ru.WEB_UNIT).replace('User=__USER__', f'User={OTHER_USER}'))
+    assert ru.main(['ledmatrix-refresh-units'], refresher=host.refresher()) == ru.EXIT_FAILED
+    assert 'refusing' in capsys.readouterr().err
+
+
+# -- restore ------------------------------------------------------------------
+
+def test_restore_puts_back_exactly_what_the_refresh_replaced(host):
+    # A hand-edited installed unit: the rollback must give back this file,
+    # not a rendering of the old template.
+    hand_edited = host.installed(ru.DISPLAY_UNIT) + '# edited by hand\n'
+    (host.systemd / ru.DISPLAY_UNIT).write_text(hand_edited, encoding='utf-8', newline='\n')
+    untouched = host.installed(ru.WEB_UNIT)
+    host.set_template(ru.DISPLAY_UNIT, watchdog_added(host.template(ru.DISPLAY_UNIT)))
+    host.refresher().refresh()
+
+    assert host.refresher().restore() == [ru.DISPLAY_UNIT]
+    assert host.installed(ru.DISPLAY_UNIT) == hand_edited
+    assert host.installed(ru.WEB_UNIT) == untouched
+    assert host.reloads == 2
+    assert not (host.backup / ru.MANIFEST).exists(), 'a second restore must not repeat it'
+    assert host.refresher().restore() == []
+
+
+def test_restore_after_an_update_that_changed_no_units_restores_nothing(host):
+    host.set_template(ru.DISPLAY_UNIT, watchdog_added(host.template(ru.DISPLAY_UNIT)))
+    host.refresher().refresh()          # an earlier update...
+    newer = host.installed(ru.DISPLAY_UNIT)
+    host.refresher().refresh()          # ...then one that changed no units
+    assert host.refresher().restore() == []
+    assert host.installed(ru.DISPLAY_UNIT) == newer
+
+
+def test_restore_must_run_as_root(host):
+    host.root = False
+    with pytest.raises(ru.RefreshError, match='root'):
+        host.refresher().restore()
+
+
+# -- the real templates and install_service.sh ----------------------------------
+
+def test_every_shipped_template_passes_the_helpers_checks(tmp_path):
+    host = Host(tmp_path)
+    for name in ru.UNITS:
+        host.set_template(name, watchdog_added(host.template(name)) if name.endswith('.service')
+                          else host.template(name))
+    assert host.refresher().refresh() == sorted(n for n in ru.UNITS if n.endswith('.service'))
+
+
+@pytest.mark.skipif(not sys.platform.startswith('linux'), reason='runs sed as install_service.sh does')
+def test_rendering_matches_install_service_sh(tmp_path):
+    """Same text as the installer's sed, including characters sed treats specially."""
+    lib = ROOT / 'scripts' / 'install' / 'lib_systemd_render.sh'
+    for project in ('/home/pi/LEDMatrix', '/opt/led matrix&co'):
+        for name in ru.UNITS:
+            user = 'root' if name == ru.DISPLAY_UNIT else 'pi'
+            script = (f'source "{lib}"; R=$(sed_escape_replacement "$1"); U=$(sed_escape_replacement "$2"); '
+                      f'sed "s|__PROJECT_ROOT_DIR__|$R|g; s|__USER__|$U|g" "$3"')
+            out = subprocess.run(['bash', '-c', script, 'x', project, user, str(ROOT / 'systemd' / name)],
+                                 capture_output=True, text=True, check=True).stdout
+            template = (ROOT / 'systemd' / name).read_text(encoding='utf-8')
+            assert ru.render(template, project, user) == out, name
+
+
+def test_install_service_installs_the_helper_root_owned_at_the_granted_path():
+    text = (ROOT / 'scripts' / 'install' / 'install_service.sh').read_text(encoding='utf-8')
+    assert 'scripts/install/ledmatrix_refresh_units.py' in text
+    m = re.search(r'install -D -o root -g root -m 0755 "\$REFRESH_UNITS_SRC" "\$REFRESH_UNITS_DEST"', text)
+    assert m, 'install_service.sh must install the helper root:root 0755'
+    assert f'REFRESH_UNITS_DEST={ru.INSTALLED_PATH}' in text
+
+
+def test_every_caller_names_the_same_helper_path():
+    lib = (ROOT / 'scripts' / 'install' / 'lib_sudoers.sh').read_text(encoding='utf-8')
+    verifier = (ROOT / 'scripts' / 'utils' / 'auto_update_verify.py').read_text(encoding='utf-8')
+    assert f'LEDMATRIX_REFRESH_UNITS_PATH={ru.INSTALLED_PATH}' in lib
+    assert unit_refresh.HELPER_PATH == ru.INSTALLED_PATH
+    assert f"REFRESH_UNITS_PATH = '{ru.INSTALLED_PATH}'" in verifier
+    assert ru.INSTALLED_PATH.startswith('/usr/local/sbin/'), 'must live outside the user-owned checkout'
+
+
+def test_the_helper_imports_nothing_from_the_checkout():
+    source = (ROOT / 'scripts' / 'install' / 'ledmatrix_refresh_units.py').read_text(encoding='utf-8')
+    imports = re.findall(r'^\s*(?:from|import)\s+([\w.]+)', source, re.M)
+    assert not [m for m in imports if m.split('.')[0] in ('src', 'web_interface', 'scripts')]
+    assert source.startswith('#!/usr/bin/python3 -I\n'), 'isolated mode: no PYTHON* env, no user site'
+
+
+# -- the web interface's side (web_interface/unit_refresh.py) ---------------------
+
+class Sudo:
+    """sudo: refuses (``rc``/``stderr``), or runs the real helper as root against ``host``."""
+
+    def __init__(self, host=None, rc=0, stderr=''):
+        self.host, self.rc, self.stderr, self.calls = host, rc, stderr, []
+        self.as_root = False
+
+    def __call__(self, args, **kwargs):
+        self.calls.append(list(args))
+        if self.rc or self.host is None:
+            return subprocess.CompletedProcess(args, self.rc, stdout='', stderr=self.stderr)
+        lines = []
+        self.as_root = True
+        try:
+            rc = ru.main(['ledmatrix-refresh-units', *args[3:]], refresher=self.host.refresher(lines.append))
+        finally:
+            self.as_root = False
+        return subprocess.CompletedProcess(args, rc, stdout='\n'.join(lines) + '\n', stderr='')
+
+
+def _web(host, tmp_path, sudo, helper_installed=True):
+    helper = tmp_path / 'usr-local-sbin' / 'ledmatrix-refresh-units'
+    if helper_installed:
+        helper.parent.mkdir(exist_ok=True)
+        helper.write_text('#!/bin/true\n')
+    return unit_refresh.refresh_after_update(run=sudo, systemd_dir=str(host.systemd),
+                                             helper_path=str(helper))
+
+
+def _stale(host):
+    # The web side compares the installed units with the checkout's templates.
+    host.set_template(ru.DISPLAY_UNIT, watchdog_added(host.template(ru.DISPLAY_UNIT)))
+
+
+def test_web_side_does_nothing_when_the_units_match(host, tmp_path):
+    sudo = Sudo(host)
+    result = _web(host, tmp_path, sudo)
+    assert result['status'] == unit_refresh.CURRENT and sudo.calls == []
+    assert result['message'] == ''
+
+
+def test_web_side_runs_the_helper_through_sudo_with_no_arguments(host, tmp_path):
+    _stale(host)
+    sudo = Sudo(host)
+    result = _web(host, tmp_path, sudo)
+    assert result['status'] == unit_refresh.REFRESHED
+    assert result['units'] == [ru.DISPLAY_UNIT]
+    assert len(sudo.calls) == 1 and sudo.calls[0][:2] == ['sudo', '-n'] and len(sudo.calls[0]) == 3
+    assert 'ledmatrix.service' in result['message']
+    assert 'WatchdogSec=60' in host.installed(ru.DISPLAY_UNIT)
+
+
+@pytest.fixture
+def unreadable(monkeypatch, host):
+    """Installed units only root can read (install_service.sh used to leave them 0600)."""
+    real = ru._read_installed
+    sudo = Sudo(host)
+
+    def read(systemd_dir, name):
+        if not sudo.as_root:
+            raise ru.UnitsUnreadable(f'cannot read the installed {name}: Permission denied')
+        return real(systemd_dir, name)
+    monkeypatch.setattr(ru, '_read_installed', read)
+    monkeypatch.setattr(unit_refresh, '_load_helper', lambda path=None: ru)
+    return sudo
+
+
+def test_web_side_lets_the_helper_decide_when_it_cannot_read_the_units(host, tmp_path, unreadable):
+    _stale(host)
+    result = _web(host, tmp_path, unreadable)
+    assert len(unreadable.calls) == 1
+    assert result['status'] == unit_refresh.REFRESHED and result['units'] == [ru.DISPLAY_UNIT]
+
+
+def test_web_side_unreadable_and_already_current_is_current(host, tmp_path, unreadable):
+    result = _web(host, tmp_path, unreadable)
+    assert result['status'] == unit_refresh.CURRENT and result['message'] == ''
+
+
+def test_web_side_unreadable_without_the_rule_asks_for_a_reinstall(host, tmp_path, unreadable, caplog):
+    unreadable.rc, unreadable.stderr = 1, 'sudo: a password is required'
+    result = _web(host, tmp_path, unreadable)
+    assert result['status'] == unit_refresh.NEEDS_REINSTALL
+    assert 'could not be checked' in result['message']
+
+
+def test_web_side_trusts_the_helper_about_what_changed(host, tmp_path):
+    """An older installed helper that renders differently changed nothing: nothing to roll back."""
+    _stale(host)
+
+    def older_helper(args, **kwargs):
+        return subprocess.CompletedProcess(args, 0, stdout='units: up to date\n', stderr='')
+    result = _web(host, tmp_path, older_helper)
+    assert result['status'] == unit_refresh.CURRENT
+
+
+def test_web_side_without_the_helper_asks_for_a_reinstall(host, tmp_path, caplog):
+    _stale(host)
+    sudo = Sudo()
+    result = _web(host, tmp_path, sudo, helper_installed=False)
+    assert result['status'] == unit_refresh.NEEDS_REINSTALL and sudo.calls == []
+    assert 'first_time_install.sh' in result['message']
+    assert 'reinstall' in caplog.text
+
+
+@pytest.mark.parametrize('stderr', [
+    'sudo: a password is required',
+    'Sorry, user ledpi is not allowed to run \'/usr/local/sbin/ledmatrix-refresh-units\' as root on ledpi.',
+])
+def test_web_side_without_the_sudo_rule_asks_for_a_reinstall(host, tmp_path, stderr, caplog):
+    _stale(host)
+    result = _web(host, tmp_path, Sudo(rc=1, stderr=stderr))
+    assert result['status'] == unit_refresh.NEEDS_REINSTALL
+    assert 'configure_web_sudo.sh' in result['message']
+    assert 'no sudo rule' in caplog.text
+
+
+def test_web_side_reports_a_helper_refusal_as_a_failure(host, tmp_path):
+    _stale(host)
+    result = _web(host, tmp_path, Sudo(rc=1, stderr='ledmatrix-refresh-units: systemd/x refusing'))
+    assert result['status'] == unit_refresh.FAILED
+    assert 'refusing' in result['message']
+
+
+def test_web_side_on_a_machine_without_the_units_does_nothing(tmp_path):
+    sudo = Sudo()
+    result = unit_refresh.refresh_after_update(run=sudo, systemd_dir=str(tmp_path))
+    assert result['status'] == unit_refresh.SKIPPED and sudo.calls == []
+
+
+def test_web_side_never_raises_on_a_broken_template(host, tmp_path):
+    host.set_template(ru.WEB_UNIT, host.template(ru.WEB_UNIT).replace('User=__USER__', f'User={OTHER_USER}'))
+    sudo = Sudo()
+    result = _web(host, tmp_path, sudo)
+    assert result['status'] == unit_refresh.FAILED and sudo.calls == []

test/test_update_prompts_restart.py

@@ -82,3 +82,37 @@ class TestRestartIsRequestedWhenCodeChanged:
             data = _pull(client)
         assert data['status'] == 'error'
         assert data['restart_required'] is False
+
+
+class TestUnitsAreRefreshedWithTheCode:
+    """New code may bring new systemd unit settings; installing them is part of the update."""
+
+    @pytest.fixture
+    def refresh(self, monkeypatch):
+        from web_interface import unit_refresh
+        calls = []
+
+        def fake():
+            calls.append(True)
+            return {'status': 'refreshed', 'message': 'Service settings updated (ledmatrix.service).',
+                    'units': ['ledmatrix.service']}
+        monkeypatch.setattr(unit_refresh, 'refresh_after_update', fake)
+        return calls
+
+    def test_an_update_that_moved_head_refreshes_the_units(self, client, refresh):
+        with patch.object(mod.subprocess, 'run', _git(['aaa111', 'bbb222'])):
+            data = _pull(client)
+        assert refresh == [True]
+        assert data['unit_refresh']['status'] == 'refreshed'
+        assert 'Service settings updated' in data['message']
+
+    def test_nothing_new_touches_no_units(self, client, refresh):
+        with patch.object(mod.subprocess, 'run',
+                          _git(['aaa111', 'aaa111'], pull_out='Already up to date.\n')):
+            data = _pull(client)
+        assert refresh == [] and data['unit_refresh'] is None
+
+    def test_a_failed_pull_touches_no_units(self, client, refresh):
+        with patch.object(mod.subprocess, 'run', _git(['aaa111'], pull_rc=1)):
+            data = _pull(client)
+        assert refresh == [] and data['unit_refresh'] is None

test/test_web_sudoers_installers_agree.py

@@ -54,6 +54,10 @@ EXPECTED_GRANTS = frozenset({
     ("NOPASSWD:", "$SYSTEMCTL_PATH restart ledmatrix-web.service"),
     ("NOPASSWD:", "$BASH_PATH $PROJECT_ROOT/scripts/fix_perms/safe_plugin_rm.sh *"),
     ("NOPASSWD:", "$BASH_PATH $PROJECT_ROOT/scripts/fix_perms/safe_pip_install.sh *"),
+    # The unit refresh helper (scripts/install/ledmatrix_refresh_units.py),
+    # with no arguments (`""`; RULE below drops the closing quote) or --restore.
+    ("NOPASSWD:", '$LEDMATRIX_REFRESH_UNITS_PATH "'),
+    ("NOPASSWD:", "$LEDMATRIX_REFRESH_UNITS_PATH --restore"),
     ("NOPASSWD:NOEXEC:", "$JOURNALCTL_PATH -u ledmatrix.service *"),
     ("NOPASSWD:NOEXEC:", "$JOURNALCTL_PATH -u ledmatrix *"),
     ("NOPASSWD:NOEXEC:", "$JOURNALCTL_PATH -t ledmatrix *"),
@@ -145,7 +149,8 @@ def test_installer_call_renders_the_expected_rules(installer, tmp_path):
             rendered.add((m.group(2), m.group(3)))
     subst = {"$SYSTEMCTL_PATH": "/x/systemctl", "$BASH_PATH": "/x/bash",
              "$REBOOT_PATH": "/x/reboot", "$POWEROFF_PATH": "/x/poweroff",
-             "$JOURNALCTL_PATH": "/x/journalctl", "$PROJECT_ROOT": "/srv/led root"}
+             "$JOURNALCTL_PATH": "/x/journalctl", "$PROJECT_ROOT": "/srv/led root",
+             "$LEDMATRIX_REFRESH_UNITS_PATH": "/usr/local/sbin/ledmatrix-refresh-units"}
     expected = set()
     for tags, command in EXPECTED_GRANTS:
         for var, value in subst.items():