Fix: Report a stopped Cortex as stopped after systemd gave up on it

cmd/agentop/cmd_service_platform.go

@@ -604,6 +604,14 @@ func controlService(goos, action string, p servicePaths, progress io.Writer) err
 	if out, err := exec.Command("systemctl", args...).CombinedOutput(); err != nil {
 		return fmt.Errorf("systemctl %s: %v: %s", strings.Join(args, " "), err, strings.TrimSpace(string(out)))
 	}
+	// A stop of a unit systemd had already given up on is a no-op for systemd, so
+	// the unit stays failed, only now disabled, and status read the deliberate stop
+	// as a crash and advised a restart. Clearing it leaves inactive and disabled,
+	// which status reports as stopped. After the stop, never before: nothing is
+	// left running for the start limit to protect.
+	if action == "stop" {
+		clearStartLimit()
+	}
 	return nil
 }
 
@@ -615,10 +623,11 @@ func controlService(goos, action string, p servicePaths, progress io.Writer) err
 // tells the user to run failed with "start of the service was attempted too often",
 // and so did `service start` and a reinstall.
 //
-// Only before a start someone asked for, never on systemd's own restarts, so the
-// limit still stops a crash loop by itself. reset-failed on a unit that is not
-// failed changes nothing; its error, for a unit systemd has not loaded, is not
-// ours to report.
+// Only around a start or stop someone asked for, never on systemd's own restarts,
+// so the limit still stops a crash loop by itself. controlService also calls it
+// after a stop, for the state that leaves; see there. reset-failed on a unit that
+// is not failed changes nothing; its error, for a unit systemd has not loaded, is
+// not ours to report.
 func clearStartLimit() {
 	_ = exec.Command("systemctl", "--user", "reset-failed", systemdUnit).Run() //nolint:errcheck
 }
@@ -637,8 +646,8 @@ type unitState struct {
 	// activating/auto-restart between a crash and its restart, failed/failed once
 	// the restarts run out.
 	active, sub string
-	// UnitFileState: enabled after `service start`, disabled after `service stop`,
-	// which is what tells our stop from someone else's `systemctl --user stop`.
+	// UnitFileState: enabled after `service start`; disabled after `service stop`,
+	// or for a unit file that was never enabled, which systemd cannot tell apart.
 	enabled string
 	// Result, as systemd words why the unit last stopped: success, exit-code,
 	// signal, start-limit-hit.

cmd/agentop/cmd_service_systemd_integration_test.go

@@ -551,6 +551,27 @@ func TestServiceLifecycle_RealSystemd(t *testing.T) {
 	code, out = status()
 	expect("status after recovering", code, out, 0, "Cortex is healthy according to")
 
+	// A stop of a unit systemd has given up on. systemd's own stop is a no-op on a
+	// failed unit, so before stop cleared it the unit stayed failed, only disabled,
+	// and status called the deliberate stop a crash and advised a restart.
+	if code, out := control("stop"); code != 0 {
+		t.Fatalf("stop before second crash loop: exit %d:\n%s", code, out)
+	}
+	if err := os.WriteFile(bin, []byte("#!/bin/sh\nexit 3\n"), 0o700); err != nil { //nolint:gosec
+		t.Fatal(err)
+	}
+	_, _ = control("start")
+	if !waitUntil(limitWait, func() bool { return unit().active == "failed" }) {
+		t.Fatalf("the second crash loop did not hit the start limit within %s; systemd says %+v", limitWait, unit())
+	}
+	code, out = control("stop")
+	expect("stop of a unit systemd gave up on", code, out, 0)
+	if s := unit(); s.active != "inactive" || s.enabled != "disabled" {
+		t.Errorf("after stopping a failed unit systemd says %+v, want inactive and disabled", s)
+	}
+	code, out = status()
+	expect("status after stopping a failed unit", code, out, 0, "Cortex is stopped, and stays stopped")
+
 	code, out = run("uninstall", func(o, e io.Writer) int { return serviceUninstall(p, true, o, e) })
 	expect("uninstall", code, out, 0, "Removed.")
 	code, out = status()

cmd/agentop/cmd_service_systemd_test.go

@@ -689,13 +689,16 @@ func TestControlService_Linux(t *testing.T) {
 	// the right action.
 	//
 	// start and restart clear systemd's start limit first, so they can bring back a
-	// unit that crash-looped into start-limit-hit. stop does not: it has no start to
-	// unblock.
+	// unit that crash-looped into start-limit-hit. stop clears it after instead: it
+	// has no start to unblock, but it must not leave a deliberate stop reading as a
+	// crash.
 	for _, tc := range []struct {
 		action string
 		want   []string
 	}{
-		{"stop", []string{"--user disable --now cortex.service"}},
+		// stop clears the failed state AFTER it, so a unit systemd had given up on
+		// ends inactive and disabled, which status reports as stopped, not crashed.
+		{"stop", []string{"--user disable --now cortex.service", "--user reset-failed cortex.service"}},
 		{"start", []string{"--user reset-failed cortex.service", "--user enable --now cortex.service"}},
 		{"restart", []string{"--user reset-failed cortex.service", "--user restart cortex.service"}},
 	} {

docs/laptop-service.md

@@ -611,7 +611,8 @@ exited 1. On macOS it still does.
 
 `service start`, `service restart` and `service install` clear systemd's record of the
 crashes first, so they can start a unit systemd had given up on without waiting out
-its five-minute window.
+its five-minute window. `service stop` clears it after, so stopping a unit systemd had
+given up on reads as stopped, not as a crash.
 
 If it says `config: … will not load`, an edit to `~/.cortex/config.yaml` broke it
 (`config: no config at …` means the file is gone). A proxy that was already running