Repair half-removed installations in the installer update paths

A box with the binary still at /opt/pulse/bin/pulse but /etc/pulse, the
systemd unit and the /usr/local/bin symlink deleted by hand takes the
update path ("Reinstalling version ..."), which assumed the previous
install's environment still existed. With auto-updates enabled the run
died at setup_auto_updates writing system.json into the missing config
dir; without them it printed a success completion while systemctl
enable/start had failed with "Unit pulse.service could not be found",
softened into the unprivileged-container note (#1663).

Both the --version and menu update flows now run setup_directories and
recreate the unit file when it is missing (an existing unit stays
untouched so user customizations survive normal updates), and
start_pulse refuses to report success when the unit does not exist at
all. Documented as a deployment-installability contract invariant.

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

@@ -2005,6 +2005,26 @@ is `safe_systemctl daemon-reload`, which returns 0 by design, so an unchecked
 write reported success over a truncated unit. The staged helper must also be
 rejected before the swap unless it is non-empty and begins with `#!`.
 
+Every update-shaped flow over an existing box (the `--version` path and the
+interactive update action, like the reinstall action before them) must repair
+a half-removed installation rather than assume the previous install's
+environment survived: the flow runs `setup_directories` before anything
+writes into the config dir, and recreates the systemd unit via
+`ensure_systemd_service_installed` when
+`/etc/systemd/system/<service>.service` is missing — while leaving an
+existing unit untouched so operator customizations (e.g. a non-default
+`FRONTEND_PORT`) survive normal updates. A box whose operator deleted
+`/etc/pulse`, the unit file and the binary symlink but kept
+`/opt/pulse/bin/pulse` takes exactly this path ("Reinstalling version ..."),
+and previously crashed writing `system.json` into the missing config dir
+when auto-updates were enabled, or — without them — printed a success
+completion while `systemctl enable/start` had failed with "Unit
+pulse.service could not be found" behind the unprivileged-container note
+(issue #1663). `start_pulse` must never report success when the unit does
+not exist at all: the installer always writes the unit file itself, even
+where systemctl cannot run, so a missing unit is a broken installation
+rather than an unprivileged-container quirk and must fail the run loudly.
+
 Changes to the generated units must be able to reach already-deployed boxes.
 A box installed before the sandbox was widened runs the installer from a
 `pulse-update.service` whose `ReadWritePaths` excludes the helper and unit

install.sh

@@ -4504,6 +4504,18 @@ EOF
     safe_systemctl daemon-reload
 }
 
+# Updates keep an existing unit file untouched (it may carry user
+# customizations such as FRONTEND_PORT), but a half-removed installation —
+# binary still present, unit deleted by hand (#1663) — must get a fresh unit
+# or the enable/start below fails with "Unit ... could not be found".
+ensure_systemd_service_installed() {
+    command -v systemctl >/dev/null 2>&1 || return 0
+    if [[ ! -f "/etc/systemd/system/${SERVICE_NAME}.service" ]]; then
+        print_warn "systemd unit ${SERVICE_NAME}.service is missing; recreating it"
+        install_systemd_service
+    fi
+}
+
 # Tracks whether Pulse was running before an update-time stop, so start_pulse can
 # guarantee it comes back up afterward instead of silently leaving it stopped
 # (#1323: on unprivileged LXC the installer's restart can silently fail).
@@ -4561,7 +4573,20 @@ ensure_pulse_running_after_update() {
 
 start_pulse() {
     print_info "Starting Pulse..."
-    
+
+    # A missing unit is a broken installation, not an unprivileged-container
+    # quirk: "Unit ... could not be found" must never be softened into the
+    # container notes below and then reported as success (#1663). The
+    # installer always writes /etc/systemd/system/<name>.service; systemctl
+    # cat covers units living elsewhere.
+    if command -v systemctl >/dev/null 2>&1 \
+        && [[ ! -f "/etc/systemd/system/${SERVICE_NAME}.service" ]] \
+        && ! timeout 5 systemctl cat "${SERVICE_NAME}.service" >/dev/null 2>&1; then
+        print_error "systemd unit ${SERVICE_NAME}.service does not exist; Pulse was not started"
+        print_info "Re-run the installer to recreate the service unit"
+        return 1
+    fi
+
     # Try to enable/start service (may fail in unprivileged containers)
     if ! safe_systemctl enable $SERVICE_NAME; then
         print_info "Note: systemctl enable failed (common in unprivileged containers)"
@@ -4816,8 +4841,14 @@ main() {
             stop_pulse_for_update
             create_user
             download_pulse
+            # A half-removed installation (binary present, /etc/pulse or the
+            # unit file deleted, #1663) reaches this update path; recreate
+            # the environment the steps below assume instead of crashing on
+            # a missing config dir or reporting success over a missing unit.
+            setup_directories
             setup_update_command
-            
+            ensure_systemd_service_installed
+
             # Setup auto-updates if requested
             if [[ "$ENABLE_AUTO_UPDATES" == "true" ]]; then
                 setup_auto_updates
@@ -5018,8 +5049,13 @@ main() {
                 stop_pulse_for_update
                 create_user
                 download_pulse
+                # Same repair as the --version path: a half-removed
+                # installation (#1663) must get its config dir and unit
+                # file back before auto-update setup and start.
+                setup_directories
                 setup_update_command
-                
+                ensure_systemd_service_installed
+
                 # 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

scripts/installtests/root_install_sh_test.go

@@ -2035,3 +2035,92 @@ func TestRootInstallScriptRepairAutoUpdateUnitsEntryPoint(t *testing.T) {
 	}
 	assertNoAutoUpdateStagingLitter(t, filepath.Dir(autoUpdateDest), filepath.Dir(servicePath))
 }
+
+// Regression tests for #1663: a half-removed installation (binary still at
+// /opt/pulse/bin/pulse, but /etc/pulse and the systemd unit deleted by hand)
+// re-ran the installer, which took the update path ("Reinstalling version
+// ..."). With auto-updates enabled it crashed writing system.json into the
+// missing config dir; without them it printed a success completion while
+// `systemctl enable/start` had failed with "Unit pulse.service could not be
+// found", softened into the unprivileged-container note. The update flows
+// must recreate the config dir and the unit, and start_pulse must fail
+// loudly when the unit does not exist at all.
+func TestRootInstallScriptUpdateFlowsRepairHalfRemovedInstall(t *testing.T) {
+	content, err := os.ReadFile(filepath.Join("..", "..", "install.sh"))
+	if err != nil {
+		t.Fatalf("read root install.sh: %v", err)
+	}
+
+	wired := regexp.MustCompile(`(?m)^\s*download_pulse\n(?:\s*#[^\n]*\n)*\s*setup_directories\n\s*setup_update_command\n\s*ensure_systemd_service_installed$`)
+	if got := len(wired.FindAll(content, -1)); got != 2 {
+		t.Fatalf("expected both update flows (--version and menu update) to run setup_directories and ensure_systemd_service_installed after download_pulse, found %d", got)
+	}
+}
+
+func TestRootInstallEnsureSystemdServiceRecreatesMissingUnit(t *testing.T) {
+	script := `
+set -euo pipefail
+SERVICE_NAME="pulse-missing-unit-1663"
+print_warn() { echo "WARN: $*"; }
+systemctl() { return 0; }
+install_systemd_service() { echo "INSTALL_SYSTEMD_SERVICE_CALLED"; }
+` + extractRootInstallShellFunction(t, "ensure_systemd_service_installed") + `
+ensure_systemd_service_installed
+`
+	out, err := exec.Command("bash", "-c", script).CombinedOutput()
+	if err != nil {
+		t.Fatalf("ensure_systemd_service_installed failed: %v\n%s", err, out)
+	}
+	if !strings.Contains(string(out), "INSTALL_SYSTEMD_SERVICE_CALLED") {
+		t.Fatalf("missing unit was not recreated:\n%s", out)
+	}
+}
+
+func TestRootInstallStartPulseFailsWhenUnitMissing(t *testing.T) {
+	stubs := `
+set -euo pipefail
+PULSE_WAS_ACTIVE="false"
+print_info() { echo "INFO: $*"; }
+print_error() { echo "ERROR: $*"; }
+print_success() { echo "SUCCESS: $*"; }
+safe_systemctl() { return 0; }
+timeout() { shift; "$@"; }
+sleep() { :; }
+journalctl() { return 0; }
+ensure_pulse_running_after_update() { return 0; }
+` + extractRootInstallShellFunction(t, "start_pulse") + "\n"
+
+	// Unit genuinely absent: no unit file and `systemctl cat` cannot find it.
+	// start_pulse must fail with a clear error instead of reporting success
+	// behind the unprivileged-container note.
+	missing := stubs + `
+SERVICE_NAME="pulse-missing-unit-1663"
+systemctl() { return 1; }
+start_pulse
+`
+	out, err := exec.Command("bash", "-c", missing).CombinedOutput()
+	if err == nil {
+		t.Fatalf("start_pulse reported success with no unit installed:\n%s", out)
+	}
+	if !strings.Contains(string(out), "does not exist") {
+		t.Fatalf("start_pulse did not explain the missing unit:\n%s", out)
+	}
+	if strings.Contains(string(out), "SUCCESS:") {
+		t.Fatalf("start_pulse printed success for a missing unit:\n%s", out)
+	}
+
+	// Unit resolvable via systemctl cat: the guard must fall through and the
+	// normal start path must succeed.
+	present := stubs + `
+SERVICE_NAME="pulse-missing-unit-1663"
+systemctl() { return 0; }
+start_pulse
+`
+	out, err = exec.Command("bash", "-c", present).CombinedOutput()
+	if err != nil {
+		t.Fatalf("start_pulse failed with a resolvable unit: %v\n%s", err, out)
+	}
+	if !strings.Contains(string(out), "SUCCESS:") {
+		t.Fatalf("start_pulse did not report a successful start:\n%s", out)
+	}
+}