fix(install): make unattended auto-updates actually run after install and upgrade

Two GA-blocking bugs in the auto-update install path:

The pulse-update.service unit was rendered through an unquoted heredoc
containing $${PULSE_SERVICE_NAME}, which bash expands to the installer's
PID, so the unit shipped with ExecCondition checking a service like
"24757{PULSE_SERVICE_NAME}". The condition always failed and systemd
silently skipped every scheduled run, for fresh installs and upgrades
alike. The heredoc now renders the detected service name directly:
ExecCondition=/bin/sh -c 'systemctl is-active --quiet pulse'.

Upgrades also never refreshed the updater assets: a v5 box with
auto-updates enabled keeps pulse-update.timer and autoUpdateEnabled=true,
so the update/reinstall flows never re-ran setup_auto_updates and the
v5.1-pinned /usr/local/bin/pulse-auto-update.sh stayed in place, logging
"Already running latest version" forever instead of selecting v6
releases. The asset-install half of setup_auto_updates is now a shared
install_auto_update_assets(), and a new refresh_auto_updates() rewrites
the helper script and units unconditionally whenever the timer already
exists, without touching system.json or the timer's enabled state. All
five install flows (update, reinstall, --version, --source, fresh) are
wired.

Tests now render the real unit and execute the rendered ExecCondition
against a recording systemctl stub instead of asserting source-text
fragments, pin refresh_auto_updates behavior (stale helper replaced,
system.json and enablement untouched), and pin the call-site wiring.
The deployment-installability contract records both invariants.

Note: rewritten message only — the original local commit carried a
parallel agent's licensing commit message due to a shared-index race;
the tree is byte-identical to the hook-verified original.

docs/release-control/v6/internal/subsystems/deployment-installability.md

@@ -1119,6 +1119,28 @@ installs too: the root `install.sh`, generated update helper, and
 identity across install, update, reset, uninstall, and timer/service wiring so
 stable and preview Pulse runtimes can coexist on one host without drifting back
 onto the default `pulse.service` paths.
+The generated auto-update systemd wiring is itself contract surface: the root
+`install.sh` writes the `pulse-update.service` / `pulse-update.timer` units
+(or the service-scoped equivalents) through one shared
+`install_auto_update_assets` helper, and the rendered units must contain no
+unexpanded `$` reference — every variable is substituted at render time, and
+in particular `$$` (which bash expands to the installer's PID inside the
+unquoted heredoc) must never reach the unit, because a PID-corrupted
+`ExecCondition` makes systemd silently skip every scheduled run. The rendered
+`ExecCondition` must gate the run on the detected Pulse service identity
+being active. Because updates and reinstalls only run the opt-in
+`setup_auto_updates` flow when the operator asks for it, every install flow
+over an existing box (update, reinstall, `--version`, `--source`, and the
+fresh-install tail behind a leftover timer) must instead refresh
+already-installed auto-update assets unconditionally via
+`refresh_auto_updates` when the update timer already exists — replacing the
+helper script and rewriting the units so a version-pinned helper from a
+previous major (which never selects newer releases and reports "Already
+running latest version" forever) cannot survive an upgrade — while leaving
+`system.json` and the timer's enabled/started state untouched. The
+rendered-unit execution, refresh-behavior, and call-site wiring tests in
+`scripts/installtests/root_install_sh_test.go` are the owned proof surface
+for these invariants.
 That same server-installer uninstall must also leave no legacy companion
 footprint behind on the host: `install.sh --uninstall` removes the local
 `pulse-sensor-proxy` artifacts a v5-era Proxmox host may still carry — the

install.sh

@@ -3893,10 +3893,11 @@ configure_auto_update_script_repo() {
     chmod +x "$dest"
 }
 
-setup_auto_updates() {
-    print_info "Setting up automatic updates..."
-    local desired_channel
-    desired_channel=$(selected_update_channel)
+# Installs the auto-update helper script and rewrites the systemd
+# service/timer units. Shared by setup_auto_updates (first-time enable) and
+# refresh_auto_updates (updates/reinstalls where the timer already exists).
+# Does not touch system.json or the timer's enabled/started state.
+install_auto_update_assets() {
     local service_name="${SERVICE_NAME:-${PULSE_SERVICE_NAME:-pulse}}"
     local install_dir="${INSTALL_DIR:-${PULSE_INSTALL_DIR:-/opt/pulse}}"
     local config_dir="${CONFIG_DIR:-${PULSE_CONFIG_DIR:-/etc/pulse}}"
@@ -3914,21 +3915,20 @@ setup_auto_updates() {
         print_info "Downloading auto-update script..."
         if ! download_auto_update_script; then
             print_warn "Could not download the auto-update helper after multiple attempts."
-            print_warn "Continuing without automatic updates. Re-run install.sh with --enable-auto-updates once connectivity is stable."
-            ENABLE_AUTO_UPDATES=false
-            return 0
+            return 1
         fi
     fi
 
     if ! configure_auto_update_script_repo "$auto_update_dest"; then
         print_warn "Could not configure the auto-update helper for the selected release repo."
-        print_warn "Continuing without automatic updates. Re-run install.sh once filesystem access is stable."
         rm -f "$auto_update_dest"
-        ENABLE_AUTO_UPDATES=false
-        return 0
+        return 1
     fi
-    
-    # Install systemd timer and service
+
+    # Install systemd timer and service. The heredoc is unquoted so the
+    # installer substitutes paths/names; the rendered unit must contain no
+    # unexpanded $ (in particular never $$, which bash expands to the
+    # installer's PID and which broke ExecCondition before v6.0.0).
     cat > "$update_service_path" <<EOF
 [Unit]
 Description=Automatic Pulse update check and install
@@ -3940,8 +3940,8 @@ Wants=network-online.target
 Type=oneshot
 User=root
 Group=root
-# Skip auto-update run unless a supported Pulse service is active
-ExecCondition=/bin/sh -c 'systemctl is-active --quiet "$${PULSE_SERVICE_NAME}"'
+# Skip auto-update run unless the Pulse service it updates is active
+ExecCondition=/bin/sh -c 'systemctl is-active --quiet ${service_name}'
 ExecStart=${auto_update_dest}
 Restart=no
 TimeoutStartSec=600
@@ -3983,10 +3983,27 @@ EOF
 
     # Reload systemd daemon
     safe_systemctl daemon-reload
-    
-    # Enable timer but don't start it yet  
+}
+
+setup_auto_updates() {
+    print_info "Setting up automatic updates..."
+    local desired_channel
+    desired_channel=$(selected_update_channel)
+    local service_name="${SERVICE_NAME:-${PULSE_SERVICE_NAME:-pulse}}"
+    local config_dir="${CONFIG_DIR:-${PULSE_CONFIG_DIR:-/etc/pulse}}"
+    local update_timer_path="${UPDATE_TIMER_PATH:-${PULSE_UPDATE_TIMER_PATH:-/etc/systemd/system/${service_name}-update.timer}}"
+    local update_timer_unit
+    update_timer_unit="$(basename "$update_timer_path")"
+
+    if ! install_auto_update_assets; then
+        print_warn "Continuing without automatic updates. Re-run install.sh with --enable-auto-updates once the issue above is resolved."
+        ENABLE_AUTO_UPDATES=false
+        return 0
+    fi
+
+    # Enable timer but don't start it yet
     safe_systemctl enable "$update_timer_unit" || true
-    
+
     # Update system.json to enable auto-updates
     if [[ -f "$config_dir/system.json" ]]; then
         # Update existing file
@@ -4018,10 +4035,25 @@ EOF
     
     # Start the timer
     safe_systemctl start "$update_timer_unit" || true
-    
+
     print_success "Automatic updates enabled (daily check with 2-6 hour random delay)"
 }
 
+# Updates and reinstalls only run setup_auto_updates when the user opts in,
+# but a box that already has the timer keeps whatever helper script and units
+# a previous install wrote — e.g. a v5.1-pinned helper that never selects a
+# v6 release, so unattended updates silently never resume after upgrading.
+# Refresh the installed assets unconditionally instead, without changing the
+# user's choices: system.json and the timer's enabled state stay untouched.
+refresh_auto_updates() {
+    print_info "Refreshing the installed auto-update helper..."
+    if ! install_auto_update_assets; then
+        print_warn "Could not refresh the auto-update helper; the previously installed one may be stale."
+        print_warn "Re-run install.sh with --enable-auto-updates to repair it."
+    fi
+    return 0
+}
+
 install_systemd_service() {
     print_info "Installing systemd service..."
     
@@ -4320,6 +4352,8 @@ main() {
 
             if [[ "$ENABLE_AUTO_UPDATES" == "true" ]]; then
                 setup_auto_updates
+            elif update_timer_exists; then
+                refresh_auto_updates
             fi
 
             start_pulse
@@ -4395,8 +4429,10 @@ main() {
             # Setup auto-updates if requested
             if [[ "$ENABLE_AUTO_UPDATES" == "true" ]]; then
                 setup_auto_updates
+            elif update_timer_exists; then
+                refresh_auto_updates
             fi
-            
+
             start_pulse
             create_marker_file
             print_completion
@@ -4592,9 +4628,13 @@ main() {
                 download_pulse
                 setup_update_command
                 
-                # Setup auto-updates if requested during update
+                # Setup auto-updates if requested during update; otherwise
+                # refresh assets a previous install already put in place so a
+                # stale helper (e.g. v5-pinned) doesn't survive the upgrade
                 if [[ "$ENABLE_AUTO_UPDATES" == "true" ]]; then
                     setup_auto_updates
+                elif update_timer_exists; then
+                    refresh_auto_updates
                 fi
                 
                 start_pulse
@@ -4649,9 +4689,12 @@ main() {
                 setup_update_command
                 install_systemd_service
                 
-                # Setup auto-updates if requested during reinstall
+                # Setup auto-updates if requested during reinstall; otherwise
+                # refresh assets a previous install already put in place
                 if [[ "$ENABLE_AUTO_UPDATES" == "true" ]]; then
                     setup_auto_updates
+                elif update_timer_exists; then
+                    refresh_auto_updates
                 fi
                 
                 start_pulse
@@ -4768,9 +4811,12 @@ main() {
         setup_update_command
         install_systemd_service
         
-        # Setup auto-updates if requested
+        # Setup auto-updates if requested; a leftover timer from a previous
+        # install still gets its helper script and units refreshed
         if [[ "$ENABLE_AUTO_UPDATES" == "true" ]]; then
             setup_auto_updates
+        elif update_timer_exists; then
+            refresh_auto_updates
         fi
         
         start_pulse

scripts/installtests/install_sh_test.go

@@ -1230,6 +1230,7 @@ func extractSetupAutoUpdatesShellFunctions(t *testing.T) string {
 	return extractSelectedUpdateChannelShellFunctions(t) + "\n" +
 		extractRootInstallShellFunction(t, "repo_web_url") + "\n" +
 		extractRootInstallShellFunction(t, "configure_auto_update_script_repo") + "\n" +
+		extractRootInstallShellFunction(t, "install_auto_update_assets") + "\n" +
 		extractRootInstallShellFunction(t, "setup_auto_updates")
 }
 

scripts/installtests/root_install_sh_test.go

@@ -4,6 +4,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"testing"
 )
@@ -842,3 +843,224 @@ func TestRootInstallDeployAgentScriptsDeploysSignatureSidecars(t *testing.T) {
 		}
 	}
 }
+
+// Regression test for the corrupted ExecCondition: setup_auto_updates used to
+// render the pulse-update.service unit through an unquoted heredoc containing
+// `$${PULSE_SERVICE_NAME}`, which bash expanded to the installer's PID. The
+// resulting condition always failed, so systemd silently skipped every
+// scheduled auto-update run. This test renders the real unit and executes the
+// rendered ExecCondition command, instead of asserting source-text fragments.
+func TestSetupAutoUpdatesRendersExecutableExecCondition(t *testing.T) {
+	for _, tc := range []struct {
+		name        string
+		serviceName string // empty = rely on the default
+		want        string
+	}{
+		{name: "default service name", serviceName: "", want: "pulse"},
+		{name: "instance-scoped service name", serviceName: "pulse-blue", want: "pulse-blue"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			tmpDir := t.TempDir()
+			configDir := filepath.Join(tmpDir, "config")
+			installDir := filepath.Join(tmpDir, "install")
+			autoUpdateSrc := filepath.Join(installDir, "scripts", "pulse-auto-update.sh")
+			autoUpdateDest, servicePath, timerPath := prepareAutoUpdatePaths(t, tmpDir)
+
+			if err := os.MkdirAll(configDir, 0755); err != nil {
+				t.Fatalf("mkdir config dir: %v", err)
+			}
+			if err := os.MkdirAll(filepath.Dir(autoUpdateSrc), 0755); err != nil {
+				t.Fatalf("mkdir auto-update src dir: %v", err)
+			}
+			if err := os.WriteFile(autoUpdateSrc, []byte("#!/usr/bin/env bash\n"), 0755); err != nil {
+				t.Fatalf("write auto-update src: %v", err)
+			}
+
+			serviceNameLine := ""
+			if tc.serviceName != "" {
+				serviceNameLine = `SERVICE_NAME="` + tc.serviceName + `"`
+			}
+			script := `
+		CONFIG_DIR="` + configDir + `"
+		INSTALL_DIR="` + installDir + `"
+		PULSE_AUTO_UPDATE_DEST="` + autoUpdateDest + `"
+		PULSE_UPDATE_SERVICE_PATH="` + servicePath + `"
+		PULSE_UPDATE_TIMER_PATH="` + timerPath + `"
+		` + serviceNameLine + `
+		FORCE_CHANNEL=""
+		UPDATE_CHANNEL=""
+		GITHUB_REPO="rcourtman/Pulse"
+		print_info() { :; }
+		print_warn() { :; }
+		print_success() { :; }
+		safe_systemctl() { :; }
+		systemctl() { return 0; }
+		chown() { :; }
+` + extractSetupAutoUpdatesShellFunctions(t) + `
+		setup_auto_updates
+	`
+
+			out, err := exec.Command("bash", "-c", script).CombinedOutput()
+			if err != nil {
+				t.Fatalf("bash: %v\n%s", err, out)
+			}
+
+			unitBytes, err := os.ReadFile(servicePath)
+			if err != nil {
+				t.Fatalf("read rendered service unit: %v", err)
+			}
+			unit := string(unitBytes)
+
+			wantLine := `ExecCondition=/bin/sh -c 'systemctl is-active --quiet ` + tc.want + `'`
+			if !strings.Contains(unit, wantLine+"\n") {
+				t.Fatalf("rendered unit missing %q:\n%s", wantLine, unit)
+			}
+			// The heredoc substitutes every variable at render time; any $
+			// left in the unit means an unexpanded (or PID-corrupted)
+			// reference leaked through again.
+			if strings.Contains(unit, "$") {
+				t.Fatalf("rendered unit contains an unexpanded $:\n%s", unit)
+			}
+
+			// Execute the rendered condition the way systemd would, with a
+			// recording systemctl stub, to prove the command itself is sound.
+			binDir := filepath.Join(tmpDir, "stub-bin")
+			if err := os.MkdirAll(binDir, 0755); err != nil {
+				t.Fatalf("mkdir stub bin: %v", err)
+			}
+			recordPath := filepath.Join(tmpDir, "systemctl-args")
+			stub := "#!/bin/sh\nprintf '%s' \"$*\" > \"" + recordPath + "\"\nexit 0\n"
+			if err := os.WriteFile(filepath.Join(binDir, "systemctl"), []byte(stub), 0755); err != nil {
+				t.Fatalf("write systemctl stub: %v", err)
+			}
+			condition := strings.TrimPrefix(wantLine, "ExecCondition=")
+			condOut, err := exec.Command("bash", "-c", `PATH="`+binDir+`:$PATH" `+condition).CombinedOutput()
+			if err != nil {
+				t.Fatalf("rendered ExecCondition failed to execute: %v\n%s", err, condOut)
+			}
+			recorded, err := os.ReadFile(recordPath)
+			if err != nil {
+				t.Fatalf("ExecCondition never invoked systemctl: %v", err)
+			}
+			if got, want := string(recorded), "is-active --quiet "+tc.want; got != want {
+				t.Fatalf("ExecCondition invoked systemctl %q, want %q", got, want)
+			}
+		})
+	}
+}
+
+// Regression test for stale updater scripts surviving upgrades: a v5 box with
+// auto-updates already enabled keeps pulse-update.timer, so the update flow
+// never re-ran setup_auto_updates and the v5.1-pinned helper script stayed in
+// place, logging "Already running latest version" forever. refresh_auto_updates
+// must replace the helper and units without touching system.json or the
+// timer's enabled/started state.
+func TestRefreshAutoUpdatesReplacesStaleHelperWithoutChangingEnablement(t *testing.T) {
+	tmpDir := t.TempDir()
+	configDir := filepath.Join(tmpDir, "config")
+	installDir := filepath.Join(tmpDir, "install")
+	autoUpdateSrc := filepath.Join(installDir, "scripts", "pulse-auto-update.sh")
+	autoUpdateDest, servicePath, timerPath := prepareAutoUpdatePaths(t, tmpDir)
+	callsPath := filepath.Join(tmpDir, "systemctl-calls")
+
+	if err := os.MkdirAll(configDir, 0755); err != nil {
+		t.Fatalf("mkdir config dir: %v", err)
+	}
+	if err := os.MkdirAll(filepath.Dir(autoUpdateSrc), 0755); err != nil {
+		t.Fatalf("mkdir auto-update src dir: %v", err)
+	}
+	if err := os.WriteFile(autoUpdateSrc, []byte("#!/usr/bin/env bash\necho v6-helper\n"), 0755); err != nil {
+		t.Fatalf("write auto-update src: %v", err)
+	}
+	// The stale v5.1-pinned helper and a v5-style unit without ExecCondition.
+	if err := os.WriteFile(autoUpdateDest, []byte("#!/usr/bin/env bash\necho v5-stale-helper\n"), 0755); err != nil {
+		t.Fatalf("write stale auto-update dest: %v", err)
+	}
+	if err := os.WriteFile(servicePath, []byte("[Service]\nExecStart="+autoUpdateDest+"\n"), 0644); err != nil {
+		t.Fatalf("write stale service unit: %v", err)
+	}
+	// The user explicitly disabled auto-updates; a refresh must not flip it.
+	systemJSON := `{"autoUpdateEnabled":false,"updateChannel":"stable"}`
+	if err := os.WriteFile(filepath.Join(configDir, "system.json"), []byte(systemJSON), 0644); err != nil {
+		t.Fatalf("write system.json: %v", err)
+	}
+
+	script := `
+		CONFIG_DIR="` + configDir + `"
+		INSTALL_DIR="` + installDir + `"
+		PULSE_AUTO_UPDATE_DEST="` + autoUpdateDest + `"
+		PULSE_UPDATE_SERVICE_PATH="` + servicePath + `"
+		PULSE_UPDATE_TIMER_PATH="` + timerPath + `"
+		GITHUB_REPO="rcourtman/Pulse"
+		print_info() { :; }
+		print_warn() { :; }
+		safe_systemctl() { printf '%s\n' "$*" >> "` + callsPath + `"; }
+` + extractRootInstallShellFunction(t, "repo_web_url") + `
+` + extractRootInstallShellFunction(t, "configure_auto_update_script_repo") + `
+` + extractRootInstallShellFunction(t, "install_auto_update_assets") + `
+` + extractRootInstallShellFunction(t, "refresh_auto_updates") + `
+		refresh_auto_updates
+	`
+
+	out, err := exec.Command("bash", "-c", script).CombinedOutput()
+	if err != nil {
+		t.Fatalf("bash: %v\n%s", err, out)
+	}
+
+	helper, err := os.ReadFile(autoUpdateDest)
+	if err != nil {
+		t.Fatalf("read refreshed helper: %v", err)
+	}
+	if strings.Contains(string(helper), "v5-stale-helper") {
+		t.Fatalf("refresh left the stale helper in place:\n%s", helper)
+	}
+	if !strings.Contains(string(helper), "v6-helper") {
+		t.Fatalf("refresh did not install the release helper:\n%s", helper)
+	}
+
+	unit, err := os.ReadFile(servicePath)
+	if err != nil {
+		t.Fatalf("read refreshed service unit: %v", err)
+	}
+	if !strings.Contains(string(unit), "ExecCondition=/bin/sh -c 'systemctl is-active --quiet pulse'") {
+		t.Fatalf("refresh did not rewrite the service unit:\n%s", unit)
+	}
+	if _, err := os.Stat(timerPath); err != nil {
+		t.Fatalf("refresh did not write the timer unit: %v", err)
+	}
+
+	gotJSON, err := os.ReadFile(filepath.Join(configDir, "system.json"))
+	if err != nil {
+		t.Fatalf("read system.json: %v", err)
+	}
+	if string(gotJSON) != systemJSON {
+		t.Fatalf("refresh modified system.json:\n got: %s\nwant: %s", gotJSON, systemJSON)
+	}
+
+	calls, err := os.ReadFile(callsPath)
+	if err != nil {
+		t.Fatalf("read recorded systemctl calls: %v", err)
+	}
+	if string(calls) != "daemon-reload\n" {
+		t.Fatalf("refresh changed systemd state beyond daemon-reload:\n%s", calls)
+	}
+}
+
+// Pins the wiring: every existing-install flow (update, reinstall, --version,
+// --source) and the fresh-install tail must refresh already-installed
+// auto-update assets when the user did not opt into a full re-setup.
+func TestRootInstallScriptUpdateFlowsRefreshExistingAutoUpdateAssets(t *testing.T) {
+	content, err := os.ReadFile(filepath.Join("..", "..", "install.sh"))
+	if err != nil {
+		t.Fatalf("read root install.sh: %v", err)
+	}
+
+	if !strings.Contains(string(content), "refresh_auto_updates() {") {
+		t.Fatal("install.sh missing refresh_auto_updates definition")
+	}
+
+	wired := regexp.MustCompile(`(?m)^\s*elif update_timer_exists; then\n\s*refresh_auto_updates$`)
+	if got := len(wired.FindAll(content, -1)); got < 5 {
+		t.Fatalf("expected at least 5 install flows to refresh existing auto-update assets, found %d", got)
+	}
+}