From 948a31f9d12243dcf05e06c7ba5036511706aa11 Mon Sep 17 00:00:00 2001 From: Alan Cha Date: Sun, 20 Sep 2026 23:34:07 -0400 Subject: [PATCH 01/17] test: Add a real-systemd integration test for crash recovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit renderUnitFor's comment claims systemd's Restart=on-failure covers signal death and that a deliberate stop is distinguishable from a crash — unlike the darwin KeepAlive claim next to it, which WAS tested against a real launchd and found false (driving the supervisor redesign), nothing had verified the systemd side against a real systemd at all. Add cmd_service_systemd_integration_test.go, mirroring TestWaitBootedOut_RealLaunchd: a throwaway systemd-run --user unit, a synthetic slow-to-exit script standing in for the real proxy, the same skip-guard pattern (wrong OS, missing binaries, no reachable systemd --user session) plus an ABCTL_SYSTEMD_TESTS=required escape hatch for the CI job that will set one up. Two tests: a kill -9 must produce a restart with a new PID (TestSupervisorRestartsAfterCrash_RealSystemd), and a deliberate `systemctl stop` must not (TestSupervisorStaysStoppedAfterDeliberateStop_RealSystemd). Confirmed to build, vet cleanly, and correctly skip (not silently pass, not fail) on a non-Linux host. Not yet confirmed to pass against a real systemd — that needs the CI wiring in a follow-up commit, or a real Linux machine. Assisted-By: Claude (Anthropic AI) Signed-off-by: Alan Cha --- .../cmd_service_systemd_integration_test.go | 181 ++++++++++ .../2026-09-16-linux-install-systemd-945.md | 313 ++++++++++++++++++ 2 files changed, 494 insertions(+) create mode 100644 authbridge/cmd/abctl/cmd_service_systemd_integration_test.go create mode 100644 authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md diff --git a/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go b/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go new file mode 100644 index 000000000..993db8656 --- /dev/null +++ b/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go @@ -0,0 +1,181 @@ +package main + +import ( + "os" + "os/exec" + "path/filepath" + "runtime" + "strconv" + "strings" + "testing" + "time" +) + +// requireRealSystemd skips (or, with ABCTL_SYSTEMD_TESTS=required, fails) unless +// this process can actually drive a live systemd --user session — mirroring the +// four skip guards in TestWaitBootedOut_RealLaunchd (cmd_service_bootout_test.go). +// +// That macOS test's env-var escape hatch exists because silent skipping is exactly +// how the bootout-race bug (#880) shipped unexercised. Its own workflow never sets +// the var, though, so the test has skipped in every CI run since it was written. +// The one CI job that runs THIS test should set ABCTL_SYSTEMD_TESTS=required after +// setting up a real systemd --user session, so it can't fall into the same trap. +func requireRealSystemd(t *testing.T) { + t.Helper() + skip := t.Skipf + if os.Getenv("ABCTL_SYSTEMD_TESTS") == "required" { + skip = t.Fatalf + } + if runtime.GOOS != "linux" { + skip("systemd only (GOOS=%s)", runtime.GOOS) + return + } + for _, bin := range []string{"systemctl", "systemd-run"} { + if _, err := exec.LookPath(bin); err != nil { + skip("%s not on PATH: %v", bin, err) + return + } + } + if out, err := exec.Command("systemctl", "--user", "show-environment").CombinedOutput(); err != nil { + skip("no reachable systemd --user session: %v: %s", err, strings.TrimSpace(string(out))) + return + } +} + +// slowScript writes a throwaway script that mimics the real proxy's graceful +// shutdown: it ignores nothing, but takes a couple of seconds to actually exit +// once asked to, and otherwise just idles. A trivial script that died instantly +// would hide a slow-teardown bug the same way it did for the darwin bootout race +// (see the comment on TestWaitBootedOut_RealLaunchd). +func slowScript(t *testing.T) string { + t.Helper() + path := filepath.Join(t.TempDir(), "slow.sh") + body := "#!/bin/sh\ntrap 'sleep 2; exit 0' TERM\nwhile :; do sleep 1; done\n" + if err := os.WriteFile(path, []byte(body), 0o700); err != nil { //nolint:gosec + t.Fatal(err) + } + return path +} + +// runTransientUnit starts script under a throwaway, uniquely-named unit with the +// same Restart=on-failure/RestartSec our real renderUnitFor writes, and registers +// its own teardown — stop and reset-failed, so a failed assertion never leaves a +// unit respawning after the test process exits. +func runTransientUnit(t *testing.T, name, script string) { + t.Helper() + stop := func() { + _ = exec.Command("systemctl", "--user", "stop", name).Run() //nolint:errcheck + _ = exec.Command("systemctl", "--user", "reset-failed", name).Run() //nolint:errcheck + } + t.Cleanup(stop) + stop() // in case a previous, aborted run of this test left it behind + + args := []string{ + "--user", "run", "--unit=" + name, + "-p", "Restart=on-failure", + "-p", "RestartSec=1", + script, + } + if out, err := exec.Command("systemd-run", args...).CombinedOutput(); err != nil { + t.Fatalf("systemd-run: %v: %s", err, strings.TrimSpace(string(out))) + } +} + +// unitMainPID polls for a MainPID, since it is briefly 0 right after start. +func unitMainPID(t *testing.T, name string, within time.Duration) (int, bool) { + t.Helper() + deadline := time.Now().Add(within) + for time.Now().Before(deadline) { + out, err := exec.Command("systemctl", "--user", "show", name, "--property=MainPID", "--value").Output() + if err == nil { + if pid, perr := strconv.Atoi(strings.TrimSpace(string(out))); perr == nil && pid > 0 { + return pid, true + } + } + time.Sleep(200 * time.Millisecond) + } + return 0, false +} + +func unitIsActive(name string) bool { + out, err := exec.Command("systemctl", "--user", "is-active", name).Output() + return err == nil && strings.TrimSpace(string(out)) == "active" +} + +func waitUntil(within time.Duration, ok func() bool) bool { + deadline := time.Now().Add(within) + for time.Now().Before(deadline) { + if ok() { + return true + } + time.Sleep(200 * time.Millisecond) + } + return false +} + +// TestSupervisorRestartsAfterCrash_RealSystemd drives a real systemd --user session +// to prove Restart=on-failure actually restarts a crashed unit — the assumption +// renderUnitFor's comment states ("systemd needs no such trade: on-failure covers +// signal death") but this repo had never verified against a real systemd, unlike the +// darwin KeepAlive claim, which WAS tested and found false (see the comment on +// renderUnitFor's darwin branch, and TestWaitBootedOut_RealLaunchd). +func TestSupervisorRestartsAfterCrash_RealSystemd(t *testing.T) { + requireRealSystemd(t) + + unit := "cortex-test-crash-" + strconv.Itoa(os.Getpid()) + ".service" + runTransientUnit(t, unit, slowScript(t)) + + pid, ok := unitMainPID(t, unit, 5*time.Second) + if !ok { + t.Fatal("unit never reported a main PID") + } + + // -9, not a plain stop: this must bypass the script's own TERM trap entirely, + // so it looks like a real crash (a segfault, an OOM kill) rather than a + // deliberate, distinguishable stop — which the next test proves does NOT restart. + if err := exec.Command("kill", "-9", strconv.Itoa(pid)).Run(); err != nil { + t.Fatalf("kill -9 %d: %v", pid, err) + } + + if !waitUntil(10*time.Second, func() bool { return unitIsActive(unit) }) { + t.Error("unit did not report active again after the crash — Restart=on-failure did not fire") + } + + newPID, ok := unitMainPID(t, unit, 5*time.Second) + if !ok { + t.Fatal("restarted unit never reported a main PID") + } + if newPID == pid { + t.Error("same PID after the crash — nothing actually restarted") + } +} + +// TestSupervisorStaysStoppedAfterDeliberateStop_RealSystemd proves the other half +// of the same comment: "a `systemctl stop` is distinguishable from a crash, so a +// stop stays stopped." Restart=on-failure must NOT fire for a deliberate stop, or +// `abctl service stop` would look exactly like the "stop that does not stop" bug +// this whole feature exists to avoid on the launchd side. +func TestSupervisorStaysStoppedAfterDeliberateStop_RealSystemd(t *testing.T) { + requireRealSystemd(t) + + unit := "cortex-test-stop-" + strconv.Itoa(os.Getpid()) + ".service" + runTransientUnit(t, unit, slowScript(t)) + + if _, ok := unitMainPID(t, unit, 5*time.Second); !ok { + t.Fatal("unit never reported a main PID") + } + + if out, err := exec.Command("systemctl", "--user", "stop", unit).CombinedOutput(); err != nil { + t.Fatalf("systemctl --user stop: %v: %s", err, strings.TrimSpace(string(out))) + } + + // The script's own TERM trap takes ~2s; give it real room, then assert it STAYS + // inactive rather than just checking once immediately after stop returns. + if waitUntil(1*time.Second, func() bool { return unitIsActive(unit) }) { + t.Fatal("unit is active immediately after a deliberate stop") + } + time.Sleep(4 * time.Second) // past RestartSec=1; a wrongly-firing restart would show by now + if unitIsActive(unit) { + t.Error("unit restarted after a deliberate `systemctl stop` — Restart=on-failure should not cover this") + } +} diff --git a/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md b/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md new file mode 100644 index 000000000..acf410cbf --- /dev/null +++ b/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md @@ -0,0 +1,313 @@ +# Issue #945 — Verified Linux install and systemd service lifecycle + +**Status:** research + gap analysis complete. Landed 2026-09-17: the `TimeoutStopSec` fix +(bullet 7) and the Tier 2 `fakeSystemctl`/`fakeLoginctl` test harness (bullet 6), which +required refactoring four functions to take `goos` explicitly. Landed 2026-09-20: the +Tier 3 real-systemd integration test (bullet 5) — written, compiles, correctly skips on +non-Linux, but **not yet actually run against a real systemd** (needs a real Linux box +or CI wiring — see "Suggested next steps" #5). Tier 4 (install.sh smoke test, #957) and +Tier 5 (reboot check, #964) are separate issues, not started. +**Owner:** Alan Cha (per epic #962 owner table: "Linux install, release smoke tests, reboot +verification, CI"). +**Repo:** rossoctl/cortex. This doc lives in the same directory as other planning docs +(`authbridge/docs/superpowers/plans/`) for consistency, but is a research/gap-analysis +reference, not yet an execution-ready task plan. + +## Why this doc exists + +Assigned three issues (#945, #956, #957) under epic #962 ("Cortex v0.9.0 — stable local tool +for coding agents", due 2026-09-30). This doc captures the research done to plan #945 +specifically, so the findings don't have to be re-derived later. #956/#957 have their own +dependencies (see "Relationship to other issues" below) and are intentionally out of scope +here. + +## The three assigned issues and priority order + +| # | Title | Why this order | +|---|---|---| +| **#945** | Verified Linux install and systemd service lifecycle | **Do first.** Self-contained, no dependency on other owners' unfinished work. #957 automates verifying this. | +| **#957** | Release smoke test on Linux CI runners | **Second.** Automates #945; same owner, same platform, lowest marginal complexity once #945's gaps are closed. Partial dependency on #955 (unattended capture path, owned by @esnible) for one checklist item (asserting a non-zero token count). | +| **#956** | Release smoke test on macOS CI runners | **Last.** Depends on #944 (macOS install, owned by @huang195) and #955 — two external blockers outside this owner's control, on a platform (launchd) not owned here. | + +## Correcting an early false start: this is not greenfield + +Initial read of the issue text (and of a stale local git checkout, ~452 commits behind +`origin/main`) suggested none of this existed yet. After syncing to `origin/main`, that's +wrong: the core service-lifecycle feature already shipped in **PR #876** ("Feat: Keep Cortex +running across crashes and logins (launchd / systemd)"), with follow-on fixes: + +- **#880** — Fix: upgrades fail with launchd `EIO` (bootout race) — macOS-only issue. +- **#897** — Fix: detect environments that cannot run a service, and say what to do. +- **#931** — Fix: say when a new CA leaves running agents unobservable. +- **#911** — Fix: trust the bridge CA where it matters, stop intercepting dev tooling. + +So #945 is a **verification and gap-closing** issue on top of substantially-built +functionality, not a build-from-scratch feature. The lesson for next time: always confirm +`git fetch` / compare against `origin/main` before concluding something doesn't exist. + +## Conceptual background: what systemd and launchd are for, and why the two platforms differ + +A service manager's job: start a background process, notice when it dies (including +crashes, not just deliberate stops), and decide whether to relaunch it — without a human +watching a terminal. + +**The key asymmetry that shapes the whole design:** + +- **systemd** (Linux): a unit file's `Restart=on-failure` is honored by systemd's own + supervisor loop, reliably, even for a unit just `systemctl --user start`ed while already + logged in (exactly Cortex's `curl | sh` scenario). +- **launchd** (macOS): the team found — and documented in code comments — that launchd's + equivalent (`KeepAlive`, tried alongside `StartInterval` and `RunAtLoad`) does **not** + reliably restart a LaunchAgent added mid-session (as opposed to one present at boot when + launchd first scans `LaunchAgents`). Since Cortex is always installed mid-session, this + gap is squarely in the installer's path. + +**Consequence for the architecture:** + +- **macOS** runs **two processes**: launchd starts a small Go-written supervisor + (`authbridge-proxy --supervise`, in `authbridge/cmd/authbridge-proxy/supervise.go`) that + itself watches and restarts the actual proxy child. launchd supervises the supervisor; + the supervisor does the real crash-recovery job launchd won't reliably do. +- **Linux** runs **one process**: `authbridge-proxy` directly as the systemd unit's + `ExecStart=`, with `Restart=on-failure` doing all the crash-recovery work natively. No + supervisor binary involved. Confirmed in code: + `cmd_service_platform.go:92-93` and `TestSupervisionIsPlatformCorrect` + (`cmd_service_test.go:312-332`) explicitly assert Linux does **not** use `--supervise`. + +This means **#945 is structurally simpler than #944** on the crash-recovery axis — but it +also means the Linux "Restart=on-failure actually works" assumption has never been +put through the same real-world verification (and design correction) that produced the +macOS supervisor. That's the single biggest theme in the gap analysis below. + +## Where things live (for orientation) + +- `authbridge/install.sh` — the `curl | sh` installer (~1300 lines, POSIX `sh`). +- `authbridge/install_test.sh` — installer unit tests (shell functions in isolation, no + real network/systemctl). +- `authbridge/cmd/abctl/cmd_service.go` — cross-platform `abctl service` command logic + (install/uninstall/status/start/stop/restart), ~870 lines. +- `authbridge/cmd/abctl/cmd_service_platform.go` — the OS-specific half: systemd unit + rendering, launchd plist rendering, `systemctl`/`launchctl`/`loginctl` invocations, ~723 + lines. +- `authbridge/cmd/authbridge-proxy/supervise.go` — the macOS-only (and unsupervised-fallback) + Go restart loop, ~118 lines. +- `authbridge/cmd/authbridge-proxy/main.go` — the proxy itself; on SIGTERM/SIGINT does a + graceful shutdown with a **15-second drain** (`context.WithTimeout(..., 15*time.Second)`, + `main.go:671`) — every downstream "does stop tolerate the drain" question traces back to + this constant. +- `authbridge/docs/laptop-service.md` — user-facing doc for `abctl service *`, + `~/.cortex/` layout, restricted-environment fallback (`authbridge-proxy --local`), and + manual-removal instructions. Good source of ground truth for expected behavior. +- `authbridge/cmd/abctl/cmd_service_*_test.go` — several files split by concern (bootout, + durable, recovery, restricted, stamp, perm). See gap analysis for which of these actually + exercise Linux. + +## Gap analysis against #945's checklist + +Full detail came from a source-code audit (Explore agent, 25 tool calls, full read of +`cmd_service.go`, `cmd_service_platform.go`, `supervise.go`, `install.sh`, and every +`cmd_service_*_test.go` file). Condensed per checklist bullet: + +### 1. Fresh install via `curl | sh`, amd64 and arm64 +- **Exists:** OS/arch detection (`install.sh:816-828`); both Linux arches are actually + published by `release-binaries.yaml:156`; checksum verification prefers `shasum`, falls + back to `sha256sum`; `supervisor_usable()` (`install.sh:645-653`) does a *live* preflight + (`systemctl --user show-environment`, not just `command -v systemctl`) — this is what + catches containers/minimal images where the binary exists but no user session does. +- **Gap:** No CI job or test actually installs and runs the real released binaries against a + live Linux box, for either architecture. `install_test.sh` only unit-tests shell functions + with system commands stubbed out. There is no arm64 CI runner at all — arm64 is only + exercised at *build* time, never at *install/run* time. The issue text says this is + "automated by the Linux release smoke test" — **no such smoke-test workflow exists yet** + (this is exactly #957's job, and #957 depends on #945 being solid first). + +### 2. Upgrade over an older install: service restarted, config preserved +- **Exists:** `serviceInstall` idempotency via `installCanSkip`/`serviceIsCurrent` + (`cmd_service.go:774-836`); config migration (`migrateConfig`, `cmd_service.go:343`) before + restart; binary-swap detection via SHA-256 stamp file + (`proxyBinaryUnchanged`, `cmd_service_platform.go:806-824`, platform-agnostic and well + tested). Linux restart is `systemctl --user daemon-reload` then `enable --now` + (`cmd_service_platform.go:216-247`) — no bootout-race workaround needed, per explicit + comment (`cmd_service_platform.go:92-93`) that systemd's own `Restart=`/`StartLimit*` + make the launchd EIO workaround (#880) unnecessary on Linux. +- **Gap:** Nothing drives an actual upgrade against a real running `systemd --user` unit to + confirm the restart genuinely leaves Cortex serving afterward. macOS has a dedicated + real-launchd integration test for its equivalent scenario + (`TestWaitBootedOut_RealLaunchd`); there is no Linux analog. + +### 3. Uninstall leaves no unit file, no `~/.cortex`, no modified agent settings +- **Exists:** `serviceUninstall` (`cmd_service.go:503-538`) removes the unit + stamp file, + deliberately leaves `~/.cortex` alone (that's a separate, manual step per + `laptop-service.md`). Linux `unloadService` (`cmd_service_platform.go:269-291`) tracks + whether *it* enabled lingering (via a marker file) so uninstall doesn't clobber a linger + setting the user had already set for unrelated units — a thoughtful, Linux-specific detail. +- **Gap:** None of this Linux uninstall logic (the linger-marker branch, + `systemctl --user disable --now`) has a test with faked `systemctl`/`loginctl` — the + existing `TestStopIsDurable` only greps source for expected strings, it doesn't run the + code path. "No modified agent settings" is really an `abctl claude-code disable` concern, + outside the files audited here — worth confirming separately. + +### 4. Re-running install is idempotent +- **Exists:** `installCanSkip` (well tested, platform-agnostic); `install.sh`'s + already-at-this-version guard (`install.sh:885-889`); `lingerEnabled()` + (`cmd_service_platform.go:259-267`) avoids re-toggling linger on repeat installs. +- **Gap:** `lingerEnabled`'s parsing of `loginctl show-user --property=Linger` output has + zero test coverage — a parsing regression could silently re-enable linger every install, + or silently skip enabling it when needed. + +### 5. Service survives reboot and a crash — **Tier 3 test written 2026-09-20, not yet CI-verified** +- **Added:** `cmd_service_systemd_integration_test.go` — mirrors + `TestWaitBootedOut_RealLaunchd`'s structure exactly: a throwaway `systemd-run --user` + transient unit, a synthetic slow-to-exit script (not the real proxy, for the same + isolation reason the darwin test uses "slow.sh"), the same four skip guards + (wrong OS, missing binaries, no reachable `systemctl --user` session) plus an + `ABCTL_SYSTEMD_TESTS=required` escape hatch. Two tests: + `TestSupervisorRestartsAfterCrash_RealSystemd` (`kill -9` the main PID, confirm the + unit comes back with a *different* PID — proving `Restart=on-failure` actually fires) + and `TestSupervisorStaysStoppedAfterDeliberateStop_RealSystemd` (a deliberate + `systemctl stop` must NOT trigger a restart — the other half of the claim in + `renderUnitFor`'s comment: *"a `systemctl stop` is distinguishable from a crash, so a + stop stays stopped"*). +- **Honest limit:** written and confirmed to compile, `go vet` cleanly, and correctly + *skip* (not silently pass, not fail) on a non-Linux host with a clear reason — that's + the full extent of what's verifiable from a Mac. The actual real-systemd behavior these + tests assert has **not yet been confirmed to pass on a real machine**. Next: either run + manually on a real Linux box, or wire the CI setup (`enable-linger` + + `XDG_RUNTIME_DIR` + setting `ABCTL_SYSTEMD_TESTS=required` for this job) discussed in + "Suggested next steps" below and watch it run there. +- Still the biggest historical gap this closes: unlike the darwin `KeepAlive` claim + (tested, found false, drove the supervisor redesign — see below), the Linux + `Restart=on-failure` assumption had never been through an equivalent real check at all. +- **Exists (and well-reasoned):** unit rendering (`renderUnitFor("linux", ...)`, + `cmd_service_platform.go:131-148`) sets `Restart=on-failure`, `RestartSec=10`, + `StartLimitIntervalSec=300`, `StartLimitBurst=5` — correctly placed in `[Unit]`, not + `[Service]` (a comment explains systemd v229+ ignores/deprecates it in `[Service]`, which + would silently void the throttle). `[Install] WantedBy=default.target` handles + reboot/login persistence. Good unit-level string-shape test coverage + (`TestRenderUnit_BothPlatforms`, `cmd_service_test.go:65-106`). +- **Gap:** No test — unit, integration, or even a documented manual-verification note — + proves that a real `systemd --user` instance actually restarts the unit after `kill -9`, + or survives a real reboot/logout with lingering enabled. The macOS side has exactly this + kind of verification on record: comments at `cmd_service_platform.go:44-58` document that + `KeepAlive` was tested, found **not** to work end-to-end, and that finding drove the + supervisor-process redesign. **No equivalent verification episode exists for Linux's + `Restart=on-failure` assumption** — it's currently trusted, not proven. This is precisely + the class of gap #945 (and its manual-reboot sibling, #964) exists to close. + +### 6. `abctl service status | start | stop | restart` accurate in every state — **CLOSED (Tier 2) 2026-09-17** +- **Exists:** `serviceStatus`/`serviceControl` (`cmd_service.go:540-665`) are + platform-agnostic; Linux `controlService` maps stop→`disable --now` (persistent), + start→`enable --now`, restart→plain `systemctl --user restart` (transient, by design). + `supervisorRunning`'s Linux branch uses `systemctl --user is-active cortex.service`, with a + sane fallback ("no systemctl → treat as unknown, don't block") for systemd-less boxes. +- **Fixed:** `loadService`, `controlService`, `supervisorRunning`, and `unloadService` + took `runtime.GOOS` directly (unlike `renderUnitFor`, which already took `goos` as an + explicit parameter for exactly this reason). Refactored all four to take `goos string` + explicitly, updated every call site in `cmd_service.go` to pass `runtime.GOOS`, and added + `cmd_service_systemd_test.go` — a `fakeSystemctl`/`fakeLoginctl` harness mirroring + `fakeLaunchctl`, plus tests that actually run (not skip) on any host by passing `"linux"` + explicitly. Confirmed by running for real (not just reading the code) that `is-active` + already correctly treats `activating`/`failed`/`deactivating`/`inactive` as "not + running" — only `active` (exit 0) reads as running. That logic was already correct; it + was just unverified. Also covers: `loadService`'s daemon-reload/enable failure messages, + the linger-enable/marker-write logic (skip when already on, write marker only when *we* + turn it on, caveat-not-fatal when `enable-linger` fails), `unloadService`'s + marker-gated `disable-linger` (never called without a marker), and `controlService`'s + exact verb-per-action mapping. 22 subtests, all passing. +- **Still open:** none of this proves the *real* `systemctl`/`loginctl` actually behave + this way — only that our own code reacts correctly to inputs we scripted. That's Tier 3 + (real systemd integration test), still not built. + +### 7. `stop` tolerates the proxy's ~15s drain — **CLOSED 2026-09-17** +- **Exists:** the proxy's own 15s shutdown timeout (`main.go:671`) is the anchor value + everything else has to respect. macOS handles this *explicitly* in Go + (`serviceBootoutTimeout = 30*time.Second`, `cmd_service.go:39-42`, plus the supervisor's + own 20s-before-SIGKILL logic in `supervise.go:88-95`, deliberately longer than 15s). +- **Fixed:** added an explicit `TimeoutStopSec=20` to `renderUnitFor("linux", ...)` + (`cmd_service_platform.go`), matching the macOS supervisor's 20s headroom over the + proxy's 15s drain, with a rationale comment in the same style as the surrounding + `StartLimit*`/`network-online.target` comments. `TestRenderUnit_BothPlatforms`'s + `"linux restarts on failure only"` subtest now asserts the line is present, so a future + regression that drops it fails CI instead of silently reverting to systemd's undocumented + default. Previously it "worked" only by accident of systemd's 90s default exceeding 15s — + now it's an explicit, tested value. + +### 8. Works under user systemd, and states what happens where systemd is absent +- **Exists (this is the best-handled bullet):** `loadService` gives a clear, + actionable message when `systemctl` isn't found at all, naming WSL1/containers as likely + causes (`cmd_service_platform.go:216-219`). `install.sh`'s `supervisor_usable()` does the + more realistic check — `systemctl` present but user manager/D-Bus unreachable — and falls + back to `start_unsupervised` (a plain backgrounded process using the proxy's own + `--supervise` loop), printing manual start/stop/status/logs instructions. Well tested at + the shell-function level (`install_test.sh:570-575`). +- **Gap:** On the Go side, there's no Linux equivalent of `launchdUsable()` + (`cmd_service_platform.go:680-696`, explicitly darwin-only) — the function that lets + macOS refuse cleanly *before writing anything to disk* with a dedicated, tested exit code + (`exitNoSupervisor`, exit 4). On Linux, "no systemd" is discovered later, inside + `loadService`, after the unit file has already been written (then cleaned up on failure). + End behavior is probably fine — `install.sh`'s generic fallback classifier still buckets + it correctly — but it's fine by accident/genericness, not by an intentional, tested, + Linux-aware preflight the way darwin gets. No Go test asserts the exact message or exit + code for "systemctl absent" the way `TestServiceInstall_RestrictedEnvironment` does for + darwin. + +## Cross-cutting themes + +1. **The systemd rendering/string-shape logic is solid and well tested.** The gap is almost + entirely at the "does this actually work against a real system service manager" layer — + nothing fakes or drives real `systemctl`/`loginctl` for Linux, while macOS has both a + `fakeLaunchctl` unit-test harness *and* a real-launchd integration test. +2. **The Linux path is architecturally simpler** (no supervisor process, no bootout-race + workaround) for a legitimate reason — but that simplicity has never been backed by the + same real-world verification that justified and shaped the macOS design. +3. **One concrete, low-risk fix stands out:** add `TimeoutStopSec=` to the Linux unit. Small, + self-contained, directly addresses checklist bullet 7. +4. **No real Linux CI smoke test exists yet anywhere in the repo** — this is #957's job, but + #957 can't be trusted until the gaps above are closed, since a smoke test built on top of + an unverified `Restart=on-failure` assumption would just as confidently pass. + +## Suggested next steps (not yet sequenced into a task plan) + +1. ~~Add an explicit `TimeoutStopSec=` to `renderUnitFor("linux", ...)`~~ — **done + 2026-09-17**, see bullet 7 above. +2. ~~Build a `fakeSystemctl`/`fakeLoginctl` test harness~~ — **done 2026-09-17**, see + bullet 6 above. Required refactoring `loadService`/`controlService`/`supervisorRunning`/ + `unloadService` to take `goos` explicitly first (same fix `renderUnitFor` already had) — + otherwise these functions can't be exercised from a non-Linux host at all. +3. ~~Get real verification that `Restart=on-failure` actually recovers the unit after + `kill -9`~~ — **test written 2026-09-20** (`cmd_service_systemd_integration_test.go`, + see bullet 5 above), but **not yet run against a real systemd** — only confirmed to + compile and correctly skip on a non-Linux host. Still needed: actually run it on a real + Linux box or in CI, and separately verify lingering survives a real logout/reboot + (that second half is not covered by this test at all — it only proves crash-recovery + within a live session, not the lingering/reboot-survival claim; see #964). +4. Decide whether Linux needs its own `launchdUsable()`-equivalent preflight (tested, named + exit code) or whether the current "discover it inside `loadService`, clean up, fall + through to the generic fallback" behavior is an acceptable, intentional asymmetry. +5. Wire `cmd_service_systemd_integration_test.go` into CI: add an `enable-linger` + + `XDG_RUNTIME_DIR` setup step to the `abctl` leg of `go-ci-authbridge-cmd` in `ci.yaml`, + and set `ABCTL_SYSTEMD_TESTS=required` for that job — unlike the darwin equivalent + (`ABCTL_LAUNCHD_TESTS=required`), which exists in code but is never set by any + workflow, so `TestWaitBootedOut_RealLaunchd` has skipped in every CI run since it was + written. Doing this step, unlike macOS, needs no new runner type — `ubuntu-latest` + already has a real systemd. This is also the foundation #957's smoke test can build on. + +## Relationship to other issues (for context, not in scope here) + +- **#944** — macOS mirror of this issue. Owned by @huang195. Already has real end-to-end + verification behind it (the KeepAlive finding, bootout-race fix #880) that Linux still + lacks — worth mining `cmd_service_platform.go`'s darwin comments and tests as a template + for what "verified" should look like on Linux. +- **#955** — unattended agent workloads / headless capture path, owned by @esnible. Both + #956 and #957 need it for the "non-zero token count" smoke-test assertion. Not required + for #945 itself. +- **#964** — manual reboot verification (macOS + Linux), explicitly deferred from #944/#945 + because CI runners don't reboot. Shares the exact "does Restart=on-failure/launchd + KeepAlive really survive a restart" question raised above — worth coordinating rather than + duplicating. +- **#966** — plugin build-tag convention cleanup (`exclude_plugin_*` → `include_plugin_*`) + and a smaller desktop artifact. Touches the same install/upgrade path (H2 in that issue: + upgrading to a build that dropped a plugin an existing config still names must be handled + gracefully) — flagged there as something #944/#945/#964 must cover. Worth checking in on + this once the plugin-set change lands, since it could break an assumption made above about + upgrade always being config-preserving. From c4ade8906315a3eb6cc50bf97837790fdf82d7c5 Mon Sep 17 00:00:00 2001 From: Alan Cha Date: Sun, 20 Sep 2026 23:35:36 -0400 Subject: [PATCH 02/17] CI: Give the abctl leg a real systemd --user session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cmd_service_systemd_integration_test.go can only prove anything on a real systemd, and this repo's CI has never given a test a real service manager to run against on either platform — TestWaitBootedOut_RealLaunchd has skipped in every run since it was written because nothing sets up launchd or its required-env-var escape hatch. Add a setup step to the abctl leg of go-ci-authbridge-cmd: enable-linger starts a user systemd manager for the runner's own user (who has no interactive login, so nothing would otherwise exist), wait for its bus socket, export XDG_RUNTIME_DIR, and set ABCTL_SYSTEMD_TESTS=required for that leg's Test step so a skip here fails loudly instead of silently passing. Untested: this is the first time this setup has been tried against a real GitHub-hosted runner. If enable-linger or the bus wait doesn't behave as expected here, that failure is the point of trying it. Assisted-By: Claude (Anthropic AI) Signed-off-by: Alan Cha --- .github/workflows/ci.yaml | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 736407e48..0d9f04506 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -177,7 +177,35 @@ jobs: echo "::endgroup::" done + # cmd_service_systemd_integration_test.go drives a REAL systemd --user session + # to prove Restart=on-failure actually restarts a crashed unit — the Linux + # counterpart to TestWaitBootedOut_RealLaunchd, which has skipped in every CI + # run since it was written because no workflow gives it a real launchd or sets + # its required-env-var escape hatch. Don't repeat that here: this step gives the + # abctl leg a real, reachable systemd --user session, and the Test step below + # sets ABCTL_SYSTEMD_TESTS=required for this leg only, so a skip here is a + # failure, not a silent no-op. + # + # The runner user has no interactive login, so nothing has started a user + # manager or created /run/user/ yet. enable-linger asks logind to start + # one and keep it running; wait for its bus socket rather than assuming it is + # instant. + - name: Enable a systemd --user session (abctl leg only) + if: matrix.binary == 'abctl' + run: | + sudo loginctl enable-linger "$(whoami)" + rt="/run/user/$(id -u)" + for i in $(seq 1 30); do + [ -S "${rt}/bus" ] && break + sleep 1 + done + echo "XDG_RUNTIME_DIR=${rt}" >> "$GITHUB_ENV" + XDG_RUNTIME_DIR="${rt}" systemctl --user show-environment || \ + echo "::warning::systemd --user session not reachable after enable-linger; the real-systemd tests will report why" + - name: Test + env: + ABCTL_SYSTEMD_TESTS: ${{ matrix.binary == 'abctl' && 'required' || '' }} run: go test -v -race -cover ./... proxy-init-iptables: From 3b5eeaf6357dc6d3d6a294993bedecc77ef24b4d Mon Sep 17 00:00:00 2001 From: Alan Cha Date: Sun, 20 Sep 2026 23:43:15 -0400 Subject: [PATCH 03/17] fix: Correct systemd-run invocation in the real-systemd test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First CI run (rossoctl/cortex#1076) surfaced this for real: systemd-run is not systemctl-shaped — there is no separate "run" verb, systemd-run itself IS the run action. The extra literal "run" argument made it try to exec a program named "run", failing both new tests identically: "Failed to find executable run: No such file or directory". Also confirmed independently useful from that run: this repo's ubuntu-latest (Ubuntu 24) runner image already ships XDG_RUNTIME_DIR and DBUS_SESSION_BUS_ADDRESS pointed at a live user session before our setup step even runs, and the "Enable a systemd --user session" step itself succeeded. The tests reached real systemd and failed on this argument mistake, not on session setup or on the restart claim itself. Assisted-By: Claude (Anthropic AI) Signed-off-by: Alan Cha --- authbridge/cmd/abctl/cmd_service_systemd_integration_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go b/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go index 993db8656..34b75d8e9 100644 --- a/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go +++ b/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go @@ -71,7 +71,7 @@ func runTransientUnit(t *testing.T, name, script string) { stop() // in case a previous, aborted run of this test left it behind args := []string{ - "--user", "run", "--unit=" + name, + "--user", "--unit=" + name, "-p", "Restart=on-failure", "-p", "RestartSec=1", script, From 4d35eaf15a488d968493a815e7e35d3f2bd8a270 Mon Sep 17 00:00:00 2001 From: Alan Cha Date: Sun, 20 Sep 2026 23:47:19 -0400 Subject: [PATCH 04/17] docs: Exempt the planning doc from the retired-plugin-tag guard TestNoExcludePluginTagsRemain (authbridge/scripts/profile-tags) failed on this branch's first CI run: the doc's own reference to the retired exclude_plugin_* tag form, as design history for #966, tripped a guard meant for live code. Add the allow-legacy-plugin-tag marker the guard's own error message names, rather than weakening it. Assisted-By: Claude (Anthropic AI) Signed-off-by: Alan Cha --- .../superpowers/plans/2026-09-16-linux-install-systemd-945.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md b/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md index acf410cbf..8fae1c308 100644 --- a/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md +++ b/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md @@ -305,7 +305,9 @@ Full detail came from a source-code audit (Explore agent, 25 tool calls, full re because CI runners don't reboot. Shares the exact "does Restart=on-failure/launchd KeepAlive really survive a restart" question raised above — worth coordinating rather than duplicating. -- **#966** — plugin build-tag convention cleanup (`exclude_plugin_*` → `include_plugin_*`) +- **#966** — plugin build-tag convention cleanup (retired `exclude_plugin_*` form → + `include_plugin_*`; allow-legacy-plugin-tag: this reference is design history, not a + live usage) and a smaller desktop artifact. Touches the same install/upgrade path (H2 in that issue: upgrading to a build that dropped a plugin an existing config still names must be handled gracefully) — flagged there as something #944/#945/#964 must cover. Worth checking in on From 2f893091661c942312dbdb53ef4f1f47c94876d9 Mon Sep 17 00:00:00 2001 From: Alan Cha Date: Sun, 20 Sep 2026 23:52:29 -0400 Subject: [PATCH 05/17] docs: Record the real-systemd CI result and the two bugs it caught Assisted-By: Claude (Anthropic AI) Signed-off-by: Alan Cha --- .../2026-09-16-linux-install-systemd-945.md | 75 +++++++++++-------- 1 file changed, 44 insertions(+), 31 deletions(-) diff --git a/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md b/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md index 8fae1c308..1339da893 100644 --- a/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md +++ b/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md @@ -2,11 +2,13 @@ **Status:** research + gap analysis complete. Landed 2026-09-17: the `TimeoutStopSec` fix (bullet 7) and the Tier 2 `fakeSystemctl`/`fakeLoginctl` test harness (bullet 6), which -required refactoring four functions to take `goos` explicitly. Landed 2026-09-20: the -Tier 3 real-systemd integration test (bullet 5) — written, compiles, correctly skips on -non-Linux, but **not yet actually run against a real systemd** (needs a real Linux box -or CI wiring — see "Suggested next steps" #5). Tier 4 (install.sh smoke test, #957) and -Tier 5 (reboot check, #964) are separate issues, not started. +required refactoring four functions to take `goos` explicitly. Landed 2026-09-21 +(PR #1076): the Tier 3 real-systemd integration test (bullet 5), wired into CI, and +**confirmed passing against a real systemd** — `Restart=on-failure` really does restart +a crashed unit, and a deliberate stop really doesn't trigger one. Two real bugs the +first CI run caught (a `systemd-run` argument mistake, and this doc tripping an +unrelated guard test) were fixed in the same PR. Tier 4 (install.sh smoke test, #957) +and Tier 5 (reboot check, #964) are separate issues, not started. **Owner:** Alan Cha (per epic #962 owner table: "Linux install, release smoke tests, reboot verification, CI"). **Repo:** rossoctl/cortex. This doc lives in the same directory as other planning docs @@ -155,7 +157,7 @@ Full detail came from a source-code audit (Explore agent, 25 tool calls, full re zero test coverage — a parsing regression could silently re-enable linger every install, or silently skip enabling it when needed. -### 5. Service survives reboot and a crash — **Tier 3 test written 2026-09-20, not yet CI-verified** +### 5. Service survives reboot and a crash — **CLOSED (crash half) 2026-09-21; reboot half is #964** - **Added:** `cmd_service_systemd_integration_test.go` — mirrors `TestWaitBootedOut_RealLaunchd`'s structure exactly: a throwaway `systemd-run --user` transient unit, a synthetic slow-to-exit script (not the real proxy, for the same @@ -163,21 +165,34 @@ Full detail came from a source-code audit (Explore agent, 25 tool calls, full re (wrong OS, missing binaries, no reachable `systemctl --user` session) plus an `ABCTL_SYSTEMD_TESTS=required` escape hatch. Two tests: `TestSupervisorRestartsAfterCrash_RealSystemd` (`kill -9` the main PID, confirm the - unit comes back with a *different* PID — proving `Restart=on-failure` actually fires) - and `TestSupervisorStaysStoppedAfterDeliberateStop_RealSystemd` (a deliberate - `systemctl stop` must NOT trigger a restart — the other half of the claim in - `renderUnitFor`'s comment: *"a `systemctl stop` is distinguishable from a crash, so a - stop stays stopped"*). -- **Honest limit:** written and confirmed to compile, `go vet` cleanly, and correctly - *skip* (not silently pass, not fail) on a non-Linux host with a clear reason — that's - the full extent of what's verifiable from a Mac. The actual real-systemd behavior these - tests assert has **not yet been confirmed to pass on a real machine**. Next: either run - manually on a real Linux box, or wire the CI setup (`enable-linger` + - `XDG_RUNTIME_DIR` + setting `ABCTL_SYSTEMD_TESTS=required` for this job) discussed in - "Suggested next steps" below and watch it run there. -- Still the biggest historical gap this closes: unlike the darwin `KeepAlive` claim - (tested, found false, drove the supervisor redesign — see below), the Linux - `Restart=on-failure` assumption had never been through an equivalent real check at all. + unit comes back with a *different* PID) and + `TestSupervisorStaysStoppedAfterDeliberateStop_RealSystemd` (a deliberate + `systemctl stop` must NOT trigger a restart). +- **Confirmed for real, against a live systemd, in CI** (PR #1076, + `go-ci-authbridge-cmd` / abctl leg, run 35558708292, 2026-09-21): + `TestSupervisorRestartsAfterCrash_RealSystemd` **PASS (3.09s)**, + `TestSupervisorStaysStoppedAfterDeliberateStop_RealSystemd` **PASS (7.09s)**. This is + the actual real-world verification the darwin `KeepAlive` claim already had (tested, + found false, drove the supervisor redesign) and the Linux `Restart=on-failure` + assumption never did — now it does, and it held up. +- **Two genuine bugs the first "blind" CI run caught, exactly as intended:** + 1. `systemd-run` is not `systemctl`-shaped — there's no separate `run` verb; + `systemd-run` itself IS the run action. An extra literal `"run"` argument made + both tests fail identically (`Failed to find executable run`) on the first push. + Fixed by removing it. + 2. This same planning doc's own reference to the retired `exclude_plugin_*` tag form + (as design history for #966) tripped `TestNoExcludePluginTagsRemain` — a guard + from a different, unrelated part of this codebase. Fixed with the + `allow-legacy-plugin-tag` marker its own error message names. + Neither was a systemd-behavior surprise — both were caught and fixed within the same + PR before merge, which is the point of running this blind rather than guessing. +- **One useful incidental finding:** this repo's `ubuntu-latest` (Ubuntu 24) runner image + already ships `XDG_RUNTIME_DIR`/`DBUS_SESSION_BUS_ADDRESS` pointed at a live user + session by default — the `enable-linger` setup step still ran and succeeded, but the + base image may need less setup than assumed going in. +- **Still open:** this only proves crash-recovery within a live session. It does **not** + prove lingering survives an actual logout/reboot — that's a different claim, and + deliberately out of scope here; see #964. - **Exists (and well-reasoned):** unit rendering (`renderUnitFor("linux", ...)`, `cmd_service_platform.go:131-148`) sets `Restart=on-failure`, `RestartSec=10`, `StartLimitIntervalSec=300`, `StartLimitBurst=5` — correctly placed in `[Unit]`, not @@ -275,21 +290,19 @@ Full detail came from a source-code audit (Explore agent, 25 tool calls, full re `unloadService` to take `goos` explicitly first (same fix `renderUnitFor` already had) — otherwise these functions can't be exercised from a non-Linux host at all. 3. ~~Get real verification that `Restart=on-failure` actually recovers the unit after - `kill -9`~~ — **test written 2026-09-20** (`cmd_service_systemd_integration_test.go`, - see bullet 5 above), but **not yet run against a real systemd** — only confirmed to - compile and correctly skip on a non-Linux host. Still needed: actually run it on a real - Linux box or in CI, and separately verify lingering survives a real logout/reboot - (that second half is not covered by this test at all — it only proves crash-recovery - within a live session, not the lingering/reboot-survival claim; see #964). + `kill -9`~~ — **done 2026-09-21, confirmed passing in real CI**, see bullet 5 above + (PR #1076). Still open: lingering/reboot-survival is a separate claim this test does + not cover — see #964. 4. Decide whether Linux needs its own `launchdUsable()`-equivalent preflight (tested, named exit code) or whether the current "discover it inside `loadService`, clean up, fall through to the generic fallback" behavior is an acceptable, intentional asymmetry. -5. Wire `cmd_service_systemd_integration_test.go` into CI: add an `enable-linger` + - `XDG_RUNTIME_DIR` setup step to the `abctl` leg of `go-ci-authbridge-cmd` in `ci.yaml`, - and set `ABCTL_SYSTEMD_TESTS=required` for that job — unlike the darwin equivalent +5. ~~Wire `cmd_service_systemd_integration_test.go` into CI~~ — **done 2026-09-21** + (PR #1076): `enable-linger` + + `XDG_RUNTIME_DIR` setup step on the `abctl` leg of `go-ci-authbridge-cmd` in `ci.yaml`, + `ABCTL_SYSTEMD_TESTS=required` set for that job — unlike the darwin equivalent (`ABCTL_LAUNCHD_TESTS=required`), which exists in code but is never set by any workflow, so `TestWaitBootedOut_RealLaunchd` has skipped in every CI run since it was - written. Doing this step, unlike macOS, needs no new runner type — `ubuntu-latest` + written. This one now genuinely runs on every PR. Needed no new runner type — `ubuntu-latest` already has a real systemd. This is also the foundation #957's smoke test can build on. ## Relationship to other issues (for context, not in scope here) From 97ad5f174b4fb8a968e9457825fa5b1f11b4a8bf Mon Sep 17 00:00:00 2001 From: Alan Cha Date: Mon, 21 Sep 2026 00:19:47 -0400 Subject: [PATCH 06/17] docs: Trim the CI setup step's comment to what it actually does The macOS/TestWaitBootedOut_RealLaunchd history belongs with the test it's about (already in cmd_service_systemd_integration_test.go's own comment), not repeated here. This step's comment should say what THIS step does and why, not the backstory of the file it sets up for. Assisted-By: Claude (Anthropic AI) Signed-off-by: Alan Cha --- .github/workflows/ci.yaml | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 0d9f04506..c1bb2be95 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -177,19 +177,11 @@ jobs: echo "::endgroup::" done - # cmd_service_systemd_integration_test.go drives a REAL systemd --user session - # to prove Restart=on-failure actually restarts a crashed unit — the Linux - # counterpart to TestWaitBootedOut_RealLaunchd, which has skipped in every CI - # run since it was written because no workflow gives it a real launchd or sets - # its required-env-var escape hatch. Don't repeat that here: this step gives the - # abctl leg a real, reachable systemd --user session, and the Test step below - # sets ABCTL_SYSTEMD_TESTS=required for this leg only, so a skip here is a - # failure, not a silent no-op. - # - # The runner user has no interactive login, so nothing has started a user - # manager or created /run/user/ yet. enable-linger asks logind to start - # one and keep it running; wait for its bus socket rather than assuming it is - # instant. + # Gives the abctl leg a real systemd --user session for + # cmd_service_systemd_integration_test.go. The runner has no interactive login, so + # nothing has started one yet; enable-linger asks logind to start and keep one + # running. ABCTL_SYSTEMD_TESTS=required (below) makes a setup failure here show up + # as a failed test, not a silently skipped one. - name: Enable a systemd --user session (abctl leg only) if: matrix.binary == 'abctl' run: | From b81eef1070b2af2caab0823e2a7c628e1c83d58e Mon Sep 17 00:00:00 2001 From: Alan Cha Date: Mon, 21 Sep 2026 00:21:44 -0400 Subject: [PATCH 07/17] docs: Explain why this file's tests don't mirror the darwin one's shape The existing comment covered why the required-env-var escape hatch exists, but not why TestSupervisorRestartsAfterCrash_RealSystemd looks structurally simpler than TestWaitBootedOut_RealLaunchd. Add the actual reason: launchd doesn't reliably restart a mid-session agent, so macOS runs its own supervisor process and the darwin test proves THAT mechanism; systemd's Restart=on-failure is trusted natively, so Linux has no supervisor layer, and this test proves systemd's own claim directly instead. Assisted-By: Claude (Anthropic AI) Signed-off-by: Alan Cha --- .../cmd_service_systemd_integration_test.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go b/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go index 34b75d8e9..63a5d5a0b 100644 --- a/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go +++ b/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go @@ -11,6 +11,23 @@ import ( "time" ) +// This file's two tests deliberately don't mirror TestWaitBootedOut_RealLaunchd's +// shape, because the two platforms don't need the same thing proved. +// +// macOS: launchd does not reliably restart an agent added mid-session (measured +// and documented on renderUnitFor's darwin branch), so this codebase runs its own +// supervisor process (supervise.go) and has launchd supervise THAT instead. +// TestWaitBootedOut_RealLaunchd proves our own supervisor's bootout/restart +// handling — a mechanism this repo had to build because launchd would not do it. +// +// Linux: systemd's Restart=on-failure is trusted to work natively, so +// renderUnitFor's linux branch runs the proxy directly — one process, no +// supervisor. The two tests below instead prove systemd's OWN restart mechanism +// actually behaves as documented: TestSupervisorRestartsAfterCrash_RealSystemd is +// a claim about systemd, not about code this repo wrote — which is also why it's +// simpler than the darwin test: there's no supervisor layer or bootout race to +// reproduce, just the bare Restart=on-failure claim itself. + // requireRealSystemd skips (or, with ABCTL_SYSTEMD_TESTS=required, fails) unless // this process can actually drive a live systemd --user session — mirroring the // four skip guards in TestWaitBootedOut_RealLaunchd (cmd_service_bootout_test.go). From 36e312d778f4ec028351addf29dc7b34240b0203 Mon Sep 17 00:00:00 2001 From: Alan Cha Date: Mon, 21 Sep 2026 00:55:47 -0400 Subject: [PATCH 08/17] fix: Wait for a genuinely new MainPID, not just is-active, after a crash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit review on #1076: is-active can still read "active" in the brief window right after kill -9, before systemd has noticed the death and respawned. Checking that first, then reading MainPID right behind it, could read the old (not-yet-cleared) PID and misreport a real restart as a failure to restart — a potential flake, not observed in the one real CI run so far, but a real race nonetheless. Add waitForNewMainPID: poll until MainPID is nonzero AND differs from the pre-kill PID — the direct claim ("something new is running") rather than is-active as a proxy for it. is-active is now a secondary check after the PID has already proven the restart happened. Also: fix a second CodeRabbit finding, this one in the planning doc — the "Cross-cutting themes" section was written during the initial research pass and never updated as bullets 5/6/7 closed, so it still read as if Restart=on-failure were unverified. Struck through and corrected in place rather than deleted, to keep the history visible. Assisted-By: Claude (Anthropic AI) Signed-off-by: Alan Cha --- .../cmd_service_systemd_integration_test.go | 56 ++++++++++++++----- .../2026-09-16-linux-install-systemd-945.md | 29 ++++++---- 2 files changed, 59 insertions(+), 26 deletions(-) diff --git a/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go b/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go index 63a5d5a0b..c2d954962 100644 --- a/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go +++ b/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go @@ -98,16 +98,45 @@ func runTransientUnit(t *testing.T, name, script string) { } } +// currentMainPID reads MainPID once, or 0 if unset/unparseable. +func currentMainPID(name string) int { + out, err := exec.Command("systemctl", "--user", "show", name, "--property=MainPID", "--value").Output() + if err != nil { + return 0 + } + pid, err := strconv.Atoi(strings.TrimSpace(string(out))) + if err != nil { + return 0 + } + return pid +} + // unitMainPID polls for a MainPID, since it is briefly 0 right after start. func unitMainPID(t *testing.T, name string, within time.Duration) (int, bool) { t.Helper() deadline := time.Now().Add(within) for time.Now().Before(deadline) { - out, err := exec.Command("systemctl", "--user", "show", name, "--property=MainPID", "--value").Output() - if err == nil { - if pid, perr := strconv.Atoi(strings.TrimSpace(string(out))); perr == nil && pid > 0 { - return pid, true - } + if pid := currentMainPID(name); pid > 0 { + return pid, true + } + time.Sleep(200 * time.Millisecond) + } + return 0, false +} + +// waitForNewMainPID polls until MainPID is both nonzero and different from oldPID. +// Checking is-active alone is not enough: systemd can still report a unit "active" +// in the brief window right after a kill, before it has noticed the death and +// respawned — is-active going true first, then a same-old-PID read right behind +// it, would misreport a real restart as a failure to restart. Requiring a genuinely +// new PID is the direct claim ("something new is running"), not the reachable proxy +// for it. +func waitForNewMainPID(t *testing.T, name string, oldPID int, within time.Duration) (int, bool) { + t.Helper() + deadline := time.Now().Add(within) + for time.Now().Before(deadline) { + if pid := currentMainPID(name); pid > 0 && pid != oldPID { + return pid, true } time.Sleep(200 * time.Millisecond) } @@ -154,16 +183,15 @@ func TestSupervisorRestartsAfterCrash_RealSystemd(t *testing.T) { t.Fatalf("kill -9 %d: %v", pid, err) } - if !waitUntil(10*time.Second, func() bool { return unitIsActive(unit) }) { - t.Error("unit did not report active again after the crash — Restart=on-failure did not fire") - } - - newPID, ok := unitMainPID(t, unit, 5*time.Second) - if !ok { - t.Fatal("restarted unit never reported a main PID") + // The authoritative signal is a genuinely new PID, not is-active: is-active can + // still read "active" in the brief window right after the kill, before systemd + // has noticed the death and respawned, which would let a same-old-PID read slip + // through as a false "it restarted." + if _, ok := waitForNewMainPID(t, unit, pid, 10*time.Second); !ok { + t.Fatal("no new main PID within 10s — Restart=on-failure did not fire") } - if newPID == pid { - t.Error("same PID after the crash — nothing actually restarted") + if !unitIsActive(unit) { + t.Error("got a new PID but the unit does not report active") } } diff --git a/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md b/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md index 1339da893..e8a207cf0 100644 --- a/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md +++ b/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md @@ -268,18 +268,23 @@ Full detail came from a source-code audit (Explore agent, 25 tool calls, full re ## Cross-cutting themes -1. **The systemd rendering/string-shape logic is solid and well tested.** The gap is almost - entirely at the "does this actually work against a real system service manager" layer — - nothing fakes or drives real `systemctl`/`loginctl` for Linux, while macOS has both a - `fakeLaunchctl` unit-test harness *and* a real-launchd integration test. -2. **The Linux path is architecturally simpler** (no supervisor process, no bootout-race - workaround) for a legitimate reason — but that simplicity has never been backed by the - same real-world verification that justified and shaped the macOS design. -3. **One concrete, low-risk fix stands out:** add `TimeoutStopSec=` to the Linux unit. Small, - self-contained, directly addresses checklist bullet 7. -4. **No real Linux CI smoke test exists yet anywhere in the repo** — this is #957's job, but - #957 can't be trusted until the gaps above are closed, since a smoke test built on top of - an unverified `Restart=on-failure` assumption would just as confidently pass. +*(Written during the initial research pass; superseded as of 2026-09-21 — kept for +history, corrected below rather than deleted.)* + +1. ~~The systemd rendering/string-shape logic is solid and well tested. The gap is almost + entirely at the "does this actually work against a real system service manager" + layer — nothing fakes or drives real `systemctl`/`loginctl` for Linux~~ — **no longer + true**: `cmd_service_systemd_test.go` (fake harness) and + `cmd_service_systemd_integration_test.go` (real systemd) both exist now, and Linux has + a real-integration test wired into CI, which macOS still does not (see #944 relationship + note below). +2. ~~That simplicity has never been backed by the same real-world verification that + justified and shaped the macOS design~~ — **it now has**: PR #1076 confirmed + `Restart=on-failure` against a live systemd, in CI, 2026-09-21. +3. ~~One concrete, low-risk fix stands out: add `TimeoutStopSec=`~~ — **done**, bullet 7. +4. **No real Linux CI smoke test exists yet anywhere in the repo** — still true, this + remains #957's job. What's changed: #957 now has a verified `Restart=on-failure` + foundation to build on, rather than an unverified assumption underneath it. ## Suggested next steps (not yet sequenced into a task plan) From 939fb22d38f0de133bc006508eef69343543ed45 Mon Sep 17 00:00:00 2001 From: Alan Cha Date: Mon, 21 Sep 2026 13:29:11 -0400 Subject: [PATCH 09/17] fix: Surface cleanup failures and document the negative-assertion tradeoff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two CodeRabbit findings on #1076: - runTransientUnit's cleanup swallowed all errors via nolint:errcheck. Given the PID-based unit naming a leak is low-risk, but silent is still worse than logged — t.Logf on failure now makes one visible instead of letting it accumulate unnoticed on the runner's session. - The deliberate-stop test asserts a negative (no restart fires) with a flat 4s sleep rather than polling, with no comment on why that's an acceptable choice. Documented: the risk is one-directional (a loaded runner could produce a false pass, never a false failure), which is why a flat sleep is fine here specifically. Declined a third finding (fail the CI setup step outright on a bus-wait timeout, rather than warn and let the Test step run): that would skip every other unrelated abctl test in the same job, trading a slightly faster root-cause signal for hiding everything else's result. The current design already surfaces a precise, targeted failure via requireRealSystemd's own message. Assisted-By: Claude (Anthropic AI) Signed-off-by: Alan Cha --- .../cmd_service_systemd_integration_test.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go b/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go index c2d954962..9e539bfae 100644 --- a/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go +++ b/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go @@ -81,8 +81,15 @@ func slowScript(t *testing.T) string { func runTransientUnit(t *testing.T, name, script string) { t.Helper() stop := func() { - _ = exec.Command("systemctl", "--user", "stop", name).Run() //nolint:errcheck - _ = exec.Command("systemctl", "--user", "reset-failed", name).Run() //nolint:errcheck + // Errors here are logged, not asserted on: cleanup failing shouldn't fail a + // test that already got its answer, but a leaked unit should be visible + // instead of silently accumulating on the runner's user session. + if err := exec.Command("systemctl", "--user", "stop", name).Run(); err != nil { + t.Logf("cleanup: systemctl --user stop %s: %v", name, err) + } + if err := exec.Command("systemctl", "--user", "reset-failed", name).Run(); err != nil { + t.Logf("cleanup: systemctl --user reset-failed %s: %v", name, err) + } } t.Cleanup(stop) stop() // in case a previous, aborted run of this test left it behind @@ -219,6 +226,11 @@ func TestSupervisorStaysStoppedAfterDeliberateStop_RealSystemd(t *testing.T) { if waitUntil(1*time.Second, func() bool { return unitIsActive(unit) }) { t.Fatal("unit is active immediately after a deliberate stop") } + // A flat sleep, not a poll, because this asserts a negative: there is no "it + // happened" event to wait for. The risk this accepts is one-directional — a + // heavily loaded runner could make this pass when it shouldn't (a slow wrong + // restart lands after the check), never fail when it shouldn't (nothing here + // depends on speed for a legitimate pass). time.Sleep(4 * time.Second) // past RestartSec=1; a wrongly-firing restart would show by now if unitIsActive(unit) { t.Error("unit restarted after a deliberate `systemctl stop` — Restart=on-failure should not cover this") From 41bedac50307949d077f80220b170d707b77075d Mon Sep 17 00:00:00 2001 From: Alan Cha Date: Tue, 22 Sep 2026 10:03:10 -0400 Subject: [PATCH 10/17] docs: Explain the RestartSec=1 vs production RestartSec=10 deviation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit review on #1076: the comment claimed parity with renderUnitFor's Restart=on-failure/RestartSec, but the test uses RestartSec=1 while production uses 10 — a future reader could wonder whether the production value was accidentally dropped rather than deliberately shortened for test speed. Say so directly. Assisted-By: Claude (Anthropic AI) Signed-off-by: Alan Cha --- .../cmd/abctl/cmd_service_systemd_integration_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go b/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go index 9e539bfae..a314d091a 100644 --- a/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go +++ b/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go @@ -75,9 +75,10 @@ func slowScript(t *testing.T) string { } // runTransientUnit starts script under a throwaway, uniquely-named unit with the -// same Restart=on-failure/RestartSec our real renderUnitFor writes, and registers -// its own teardown — stop and reset-failed, so a failed assertion never leaves a -// unit respawning after the test process exits. +// same Restart=on-failure our real renderUnitFor writes (RestartSec=1 here, not the +// production 10, purely so the test doesn't wait 10s per restart it triggers), and +// registers its own teardown — stop and reset-failed, so a failed assertion never +// leaves a unit respawning after the test process exits. func runTransientUnit(t *testing.T, name, script string) { t.Helper() stop := func() { From bcb23915eaf2c98fb60cf967dea95ce37f94d0ca Mon Sep 17 00:00:00 2001 From: Alan Cha Date: Tue, 22 Sep 2026 11:18:09 -0400 Subject: [PATCH 11/17] fix: Correct doc overclaiming and a real cleanup-noise bug on #1076 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four CodeRabbit findings, all verified against actual code before fixing: - must-fix: the plan doc's Status header, bullet 6, and bullet 7 read as if this branch delivered the TimeoutStopSec fix and the fakeSystemctl/fakeLoginctl harness + goos refactor. It doesn't — confirmed via `git diff main...this-branch`, neither touches cmd_service_platform.go's function signatures nor adds that harness file. Both live on separate, not-yet-merged sibling PRs (#1079, #1080). The doc is shared across three independently-reviewed branches and each copy drifted to claim collective progress as if it were local. Corrected the Status header, bullets 6/7, Cross-cutting themes 1/3, and Suggested next steps 1/2 to attribute that work to the correct PR instead of this one. - The pre-emptive stop() call before t.Cleanup(stop) can never match a real leftover: the unit name embeds this process's own pid, unique per run, so there's nothing with that exact name to have been left behind. It only produces spurious "cleanup:" failure logs on every normal passing run (confirmed from this PR's own prior CI output: systemctl stop exit 5, reset-failed exit 1, seven such lines on a fully green run). Removed; t.Cleanup(stop) alone still covers real end-of-test teardown. - requireRealSystemd's comment claimed mirroring "the same four skip guards" as TestWaitBootedOut_RealLaunchd; it has three, and the darwin test's fourth (launchd refusing to start the test agent at all) has no systemd analog — exactly the asymmetry this file's own header comment already explains. Reworded to match. - The CI step comment attributed all setup-failure catching to ABCTL_SYSTEMD_TESTS=required; that only covers the soft-failure path (session unreachable, so the step warns and the Go test itself Fatalfs). A hard failure (enable-linger itself failing) is instead caught by bash -e failing the step outright, before Test ever runs. Named both paths. Assisted-By: Claude (Anthropic AI) Signed-off-by: Alan Cha --- .github/workflows/ci.yaml | 7 ++- .../cmd_service_systemd_integration_test.go | 16 ++++-- .../2026-09-16-linux-install-systemd-945.md | 53 ++++++++++--------- 3 files changed, 47 insertions(+), 29 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index c1bb2be95..6edaff22b 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -180,8 +180,11 @@ jobs: # Gives the abctl leg a real systemd --user session for # cmd_service_systemd_integration_test.go. The runner has no interactive login, so # nothing has started one yet; enable-linger asks logind to start and keep one - # running. ABCTL_SYSTEMD_TESTS=required (below) makes a setup failure here show up - # as a failed test, not a silently skipped one. + # running. Either way a real problem here is loud, not silent: a hard failure (e.g. + # enable-linger itself failing) fails this step outright under bash -e, so Test + # never runs; a soft one (session unreachable after enable-linger) only warns here, + # and ABCTL_SYSTEMD_TESTS=required (below) is what turns THAT case into a failed + # test instead of a silent skip. - name: Enable a systemd --user session (abctl leg only) if: matrix.binary == 'abctl' run: | diff --git a/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go b/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go index a314d091a..22f165a4c 100644 --- a/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go +++ b/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go @@ -29,8 +29,13 @@ import ( // reproduce, just the bare Restart=on-failure claim itself. // requireRealSystemd skips (or, with ABCTL_SYSTEMD_TESTS=required, fails) unless -// this process can actually drive a live systemd --user session — mirroring the -// four skip guards in TestWaitBootedOut_RealLaunchd (cmd_service_bootout_test.go). +// this process can actually drive a live systemd --user session — covering the same +// ground as TestWaitBootedOut_RealLaunchd's skip guards (cmd_service_bootout_test.go), +// deliberately in a different shape: three categories here (wrong GOOS, a binary +// missing from PATH, no reachable systemctl --user session) versus that test's four, +// since its fourth — launchd refusing to start the test agent in this domain at all — +// has no systemd analog. See this file's header comment for why the two platforms +// don't need the same thing proved in the first place. // // That macOS test's env-var escape hatch exists because silent skipping is exactly // how the bootout-race bug (#880) shipped unexercised. Its own workflow never sets @@ -93,7 +98,12 @@ func runTransientUnit(t *testing.T, name, script string) { } } t.Cleanup(stop) - stop() // in case a previous, aborted run of this test left it behind + // No pre-emptive stop() here: the unit name embeds this process's own pid, unique + // to this run, so there is no plausible same-named leftover to clear first (unlike + // the darwin test this mirrors, which uses one fixed label — that's precisely why + // its pre-emptive bootout is meaningful and this one would not be). Calling it + // anyway only logs a spurious "cleanup:" failure on every normal passing run, + // since stopping/reset-failing a unit that was never registered is itself an error. args := []string{ "--user", "--unit=" + name, diff --git a/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md b/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md index e8a207cf0..b6cbcb787 100644 --- a/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md +++ b/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md @@ -1,14 +1,16 @@ # Issue #945 — Verified Linux install and systemd service lifecycle -**Status:** research + gap analysis complete. Landed 2026-09-17: the `TimeoutStopSec` fix -(bullet 7) and the Tier 2 `fakeSystemctl`/`fakeLoginctl` test harness (bullet 6), which -required refactoring four functions to take `goos` explicitly. Landed 2026-09-21 -(PR #1076): the Tier 3 real-systemd integration test (bullet 5), wired into CI, and -**confirmed passing against a real systemd** — `Restart=on-failure` really does restart -a crashed unit, and a deliberate stop really doesn't trigger one. Two real bugs the -first CI run caught (a `systemd-run` argument mistake, and this doc tripping an -unrelated guard test) were fixed in the same PR. Tier 4 (install.sh smoke test, #957) -and Tier 5 (reboot check, #964) are separate issues, not started. +**Status:** research + gap analysis complete. **This branch (PR #1076) delivers only the +Tier 3 real-systemd integration test (bullet 5)** — wired into CI, and confirmed passing +against a real systemd: `Restart=on-failure` really does restart a crashed unit, and a +deliberate stop really doesn't trigger one. Two real bugs the first CI run caught (a +`systemd-run` argument mistake, and this doc tripping an unrelated guard test) were fixed +in the same PR. Bullets 6 and 7 are proposed and verified, but **land on separate, +not-yet-merged sibling PRs** — #1080 (Tier 2 fake harness + the `goos` refactor it +required) and #1079 (the `TimeoutStopSec` fix) respectively — neither is part of this +branch's own diff against `main`, and this doc's copy on this branch should not be read as +claiming otherwise. Tier 4 (install.sh smoke test, #957) and Tier 5 (reboot check, #964) +are separate issues, not started. **Owner:** Alan Cha (per epic #962 owner table: "Linux install, release smoke tests, reboot verification, CI"). **Repo:** rossoctl/cortex. This doc lives in the same directory as other planning docs @@ -209,7 +211,7 @@ Full detail came from a source-code audit (Explore agent, 25 tool calls, full re `Restart=on-failure` assumption** — it's currently trusted, not proven. This is precisely the class of gap #945 (and its manual-reboot sibling, #964) exists to close. -### 6. `abctl service status | start | stop | restart` accurate in every state — **CLOSED (Tier 2) 2026-09-17** +### 6. `abctl service status | start | stop | restart` accurate in every state — **CLOSED on sibling PR #1080 (Tier 2), not part of this branch** - **Exists:** `serviceStatus`/`serviceControl` (`cmd_service.go:540-665`) are platform-agnostic; Linux `controlService` maps stop→`disable --now` (persistent), start→`enable --now`, restart→plain `systemctl --user restart` (transient, by design). @@ -233,7 +235,7 @@ Full detail came from a source-code audit (Explore agent, 25 tool calls, full re this way — only that our own code reacts correctly to inputs we scripted. That's Tier 3 (real systemd integration test), still not built. -### 7. `stop` tolerates the proxy's ~15s drain — **CLOSED 2026-09-17** +### 7. `stop` tolerates the proxy's ~15s drain — **CLOSED on sibling PR #1079, not part of this branch** - **Exists:** the proxy's own 15s shutdown timeout (`main.go:671`) is the anchor value everything else has to respect. macOS handles this *explicitly* in Go (`serviceBootoutTimeout = 30*time.Second`, `cmd_service.go:39-42`, plus the supervisor's @@ -274,26 +276,29 @@ history, corrected below rather than deleted.)* 1. ~~The systemd rendering/string-shape logic is solid and well tested. The gap is almost entirely at the "does this actually work against a real system service manager" layer — nothing fakes or drives real `systemctl`/`loginctl` for Linux~~ — **no longer - true**: `cmd_service_systemd_test.go` (fake harness) and - `cmd_service_systemd_integration_test.go` (real systemd) both exist now, and Linux has - a real-integration test wired into CI, which macOS still does not (see #944 relationship - note below). + true across the three open sibling PRs together** (#1076/#1079/#1080), though **not + from this branch alone**: `cmd_service_systemd_test.go` (fake harness) is on #1080; + this branch (#1076) contributes only `cmd_service_systemd_integration_test.go` (real + systemd), which is wired into CI here and gives Linux a real-integration test macOS + still does not have (see #944 relationship note below). 2. ~~That simplicity has never been backed by the same real-world verification that - justified and shaped the macOS design~~ — **it now has**: PR #1076 confirmed - `Restart=on-failure` against a live systemd, in CI, 2026-09-21. -3. ~~One concrete, low-risk fix stands out: add `TimeoutStopSec=`~~ — **done**, bullet 7. + justified and shaped the macOS design~~ — **it now has, via this PR specifically**: + #1076 confirmed `Restart=on-failure` against a live systemd, in CI, 2026-09-21. +3. ~~One concrete, low-risk fix stands out: add `TimeoutStopSec=`~~ — **done on sibling PR + #1079**, not part of this branch; see bullet 7. 4. **No real Linux CI smoke test exists yet anywhere in the repo** — still true, this remains #957's job. What's changed: #957 now has a verified `Restart=on-failure` foundation to build on, rather than an unverified assumption underneath it. ## Suggested next steps (not yet sequenced into a task plan) -1. ~~Add an explicit `TimeoutStopSec=` to `renderUnitFor("linux", ...)`~~ — **done - 2026-09-17**, see bullet 7 above. -2. ~~Build a `fakeSystemctl`/`fakeLoginctl` test harness~~ — **done 2026-09-17**, see - bullet 6 above. Required refactoring `loadService`/`controlService`/`supervisorRunning`/ - `unloadService` to take `goos` explicitly first (same fix `renderUnitFor` already had) — - otherwise these functions can't be exercised from a non-Linux host at all. +1. Add an explicit `TimeoutStopSec=` to `renderUnitFor("linux", ...)` — **done on sibling + PR #1079**, not part of this branch; see bullet 7 above. +2. Build a `fakeSystemctl`/`fakeLoginctl` test harness — **done on sibling PR #1080**, not + part of this branch; see bullet 6 above. Required refactoring + `loadService`/`controlService`/`supervisorRunning`/`unloadService` to take `goos` + explicitly first (same fix `renderUnitFor` already had) — otherwise these functions + can't be exercised from a non-Linux host at all. 3. ~~Get real verification that `Restart=on-failure` actually recovers the unit after `kill -9`~~ — **done 2026-09-21, confirmed passing in real CI**, see bullet 5 above (PR #1076). Still open: lingering/reboot-survival is a separate claim this test does From 31265074e77570f49e1c2590586407f250487c8e Mon Sep 17 00:00:00 2001 From: Alan Cha Date: Tue, 22 Sep 2026 11:58:14 -0400 Subject: [PATCH 12/17] docs: Simplify cross-branch PR attribution to just the PR number "CLOSED on sibling PR #XXXX, not part of this branch" was more words than the point needed. Citing the PR number alone already tells a reader where to look; the disclaimer clause was redundant with it. Assisted-By: Claude (Anthropic AI) Signed-off-by: Alan Cha --- .../2026-09-16-linux-install-systemd-945.md | 47 +++++++++---------- 1 file changed, 21 insertions(+), 26 deletions(-) diff --git a/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md b/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md index b6cbcb787..2c5265e9d 100644 --- a/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md +++ b/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md @@ -1,15 +1,13 @@ # Issue #945 — Verified Linux install and systemd service lifecycle -**Status:** research + gap analysis complete. **This branch (PR #1076) delivers only the -Tier 3 real-systemd integration test (bullet 5)** — wired into CI, and confirmed passing -against a real systemd: `Restart=on-failure` really does restart a crashed unit, and a -deliberate stop really doesn't trigger one. Two real bugs the first CI run caught (a -`systemd-run` argument mistake, and this doc tripping an unrelated guard test) were fixed -in the same PR. Bullets 6 and 7 are proposed and verified, but **land on separate, -not-yet-merged sibling PRs** — #1080 (Tier 2 fake harness + the `goos` refactor it -required) and #1079 (the `TimeoutStopSec` fix) respectively — neither is part of this -branch's own diff against `main`, and this doc's copy on this branch should not be read as -claiming otherwise. Tier 4 (install.sh smoke test, #957) and Tier 5 (reboot check, #964) +**Status:** research + gap analysis complete. This branch (#1076) delivers the Tier 3 +real-systemd integration test (bullet 5) — wired into CI, and confirmed passing against a +real systemd: `Restart=on-failure` really does restart a crashed unit, and a deliberate +stop really doesn't trigger one. Two real bugs the first CI run caught (a `systemd-run` +argument mistake, and this doc tripping an unrelated guard test) were fixed in the same +PR. Bullets 6 and 7 are #1080 (Tier 2 fake harness + the `goos` refactor it required) and +#1079 (the `TimeoutStopSec` fix). Tier 4 (install.sh smoke test, #957) and Tier 5 (reboot +check, #964) are separate issues, not started. **Owner:** Alan Cha (per epic #962 owner table: "Linux install, release smoke tests, reboot verification, CI"). @@ -211,7 +209,7 @@ Full detail came from a source-code audit (Explore agent, 25 tool calls, full re `Restart=on-failure` assumption** — it's currently trusted, not proven. This is precisely the class of gap #945 (and its manual-reboot sibling, #964) exists to close. -### 6. `abctl service status | start | stop | restart` accurate in every state — **CLOSED on sibling PR #1080 (Tier 2), not part of this branch** +### 6. `abctl service status | start | stop | restart` accurate in every state — **#1080** - **Exists:** `serviceStatus`/`serviceControl` (`cmd_service.go:540-665`) are platform-agnostic; Linux `controlService` maps stop→`disable --now` (persistent), start→`enable --now`, restart→plain `systemctl --user restart` (transient, by design). @@ -235,7 +233,7 @@ Full detail came from a source-code audit (Explore agent, 25 tool calls, full re this way — only that our own code reacts correctly to inputs we scripted. That's Tier 3 (real systemd integration test), still not built. -### 7. `stop` tolerates the proxy's ~15s drain — **CLOSED on sibling PR #1079, not part of this branch** +### 7. `stop` tolerates the proxy's ~15s drain — **#1079** - **Exists:** the proxy's own 15s shutdown timeout (`main.go:671`) is the anchor value everything else has to respect. macOS handles this *explicitly* in Go (`serviceBootoutTimeout = 30*time.Second`, `cmd_service.go:39-42`, plus the supervisor's @@ -275,27 +273,24 @@ history, corrected below rather than deleted.)* 1. ~~The systemd rendering/string-shape logic is solid and well tested. The gap is almost entirely at the "does this actually work against a real system service manager" - layer — nothing fakes or drives real `systemctl`/`loginctl` for Linux~~ — **no longer - true across the three open sibling PRs together** (#1076/#1079/#1080), though **not - from this branch alone**: `cmd_service_systemd_test.go` (fake harness) is on #1080; - this branch (#1076) contributes only `cmd_service_systemd_integration_test.go` (real - systemd), which is wired into CI here and gives Linux a real-integration test macOS - still does not have (see #944 relationship note below). + layer — nothing fakes or drives real `systemctl`/`loginctl` for Linux~~ — closed by + #1080 (fake harness, `cmd_service_systemd_test.go`) and this branch, #1076 (real + systemd, `cmd_service_systemd_integration_test.go`, wired into CI) — giving Linux a + real-integration test macOS still does not have (see #944 relationship note below). 2. ~~That simplicity has never been backed by the same real-world verification that - justified and shaped the macOS design~~ — **it now has, via this PR specifically**: - #1076 confirmed `Restart=on-failure` against a live systemd, in CI, 2026-09-21. -3. ~~One concrete, low-risk fix stands out: add `TimeoutStopSec=`~~ — **done on sibling PR - #1079**, not part of this branch; see bullet 7. + justified and shaped the macOS design~~ — it now has: #1076 confirmed + `Restart=on-failure` against a live systemd, in CI, 2026-09-21. +3. ~~One concrete, low-risk fix stands out: add `TimeoutStopSec=`~~ — #1079; see bullet 7. 4. **No real Linux CI smoke test exists yet anywhere in the repo** — still true, this remains #957's job. What's changed: #957 now has a verified `Restart=on-failure` foundation to build on, rather than an unverified assumption underneath it. ## Suggested next steps (not yet sequenced into a task plan) -1. Add an explicit `TimeoutStopSec=` to `renderUnitFor("linux", ...)` — **done on sibling - PR #1079**, not part of this branch; see bullet 7 above. -2. Build a `fakeSystemctl`/`fakeLoginctl` test harness — **done on sibling PR #1080**, not - part of this branch; see bullet 6 above. Required refactoring +1. Add an explicit `TimeoutStopSec=` to `renderUnitFor("linux", ...)` — #1079; see bullet 7 + above. +2. Build a `fakeSystemctl`/`fakeLoginctl` test harness — #1080; see bullet 6 above. Required + refactoring `loadService`/`controlService`/`supervisorRunning`/`unloadService` to take `goos` explicitly first (same fix `renderUnitFor` already had) — otherwise these functions can't be exercised from a non-Linux host at all. From 5283eaae5ff907995f589def641bed5d3cc0c39d Mon Sep 17 00:00:00 2001 From: Alan Cha Date: Tue, 22 Sep 2026 13:02:06 -0400 Subject: [PATCH 13/17] fix: Stop cleanup noise from surviving GC, and finish doc overclaiming on #1076 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fourth round of CodeRabbit findings, all verified before fixing: - must-fix: bullets 6/7's bodies still said "Fixed:" for work living entirely on #1079/#1080 (both open, unmerged), and theme 1 was struck through as if #1080's half had landed too. Changed both labels to "#1080 (open):" / "#1079 (open):", matching the terse citation style already applied elsewhere; un-struck theme 1 and reworded it to attribute the real-systemd half to this branch and the fake-harness half to #1080 specifically, rather than claiming both landed. - Bullet 6's "Still open" note said Tier 3 (real-systemd integration test) was "still not built" — contradicting bullet 5 immediately above it, which is this branch's own accomplishment. Pointed at bullet 5 instead of repeating the stale claim. - The cleanup t.Logf added two rounds ago still produced noisy "cleanup:" lines on every normal passing run — confirmed from this PR's own CI output (run 35750943568): systemd-run transient units are garbage-collected once inactive, so by teardown time `stop` on an already-gone unit exits 5, and `reset-failed` on one that's neither failed nor loaded exits 1 — both routine, not a leak. Added isExitCode and filter exactly those two known-benign codes, so only a genuinely unexpected cleanup failure gets logged. - Documented an untracked gap surfaced while building this PR: ABCTL_LAUNCHD_TESTS=required is never set by any workflow, so TestWaitBootedOut_RealLaunchd still skips in every CI run — the exact trap this PR exists to avoid on the Linux side, left open on the macOS one. No macOS runner exists here to close it, so noted against #944/#956 in "Relationship to other issues" rather than left to be rediscovered. Assisted-By: Claude (Anthropic AI) Signed-off-by: Alan Cha --- .../cmd_service_systemd_integration_test.go | 22 ++++++++--- .../2026-09-16-linux-install-systemd-945.md | 38 +++++++++++-------- 2 files changed, 39 insertions(+), 21 deletions(-) diff --git a/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go b/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go index 22f165a4c..4bc486cb3 100644 --- a/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go +++ b/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go @@ -1,6 +1,7 @@ package main import ( + "errors" "os" "os/exec" "path/filepath" @@ -42,6 +43,12 @@ import ( // the var, though, so the test has skipped in every CI run since it was written. // The one CI job that runs THIS test should set ABCTL_SYSTEMD_TESTS=required after // setting up a real systemd --user session, so it can't fall into the same trap. +// isExitCode reports whether err is a process exit with exactly this code. +func isExitCode(err error, code int) bool { + var ee *exec.ExitError + return errors.As(err, &ee) && ee.ExitCode() == code +} + func requireRealSystemd(t *testing.T) { t.Helper() skip := t.Skipf @@ -87,13 +94,18 @@ func slowScript(t *testing.T) string { func runTransientUnit(t *testing.T, name, script string) { t.Helper() stop := func() { - // Errors here are logged, not asserted on: cleanup failing shouldn't fail a - // test that already got its answer, but a leaked unit should be visible - // instead of silently accumulating on the runner's user session. - if err := exec.Command("systemctl", "--user", "stop", name).Run(); err != nil { + // systemd-run transient units are garbage-collected once inactive, so by the + // time this runs the unit is typically already gone: `stop` on a unit that + // isn't loaded exits 5, and `reset-failed` on one that's neither failed nor + // loaded exits 1. Both are the ordinary end of a transient unit's life, not + // evidence of a leak — confirmed from this suite's own CI output, where both + // print on every passing run. Logging them unconditionally defeated the point + // of logging at all: a real leak would read identically to normal. Only + // anything else is worth surfacing. + if err := exec.Command("systemctl", "--user", "stop", name).Run(); err != nil && !isExitCode(err, 5) { t.Logf("cleanup: systemctl --user stop %s: %v", name, err) } - if err := exec.Command("systemctl", "--user", "reset-failed", name).Run(); err != nil { + if err := exec.Command("systemctl", "--user", "reset-failed", name).Run(); err != nil && !isExitCode(err, 1) { t.Logf("cleanup: systemctl --user reset-failed %s: %v", name, err) } } diff --git a/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md b/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md index 2c5265e9d..d3a509a83 100644 --- a/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md +++ b/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md @@ -215,37 +215,37 @@ Full detail came from a source-code audit (Explore agent, 25 tool calls, full re start→`enable --now`, restart→plain `systemctl --user restart` (transient, by design). `supervisorRunning`'s Linux branch uses `systemctl --user is-active cortex.service`, with a sane fallback ("no systemctl → treat as unknown, don't block") for systemd-less boxes. -- **Fixed:** `loadService`, `controlService`, `supervisorRunning`, and `unloadService` +- **#1080 (open):** `loadService`, `controlService`, `supervisorRunning`, and `unloadService` took `runtime.GOOS` directly (unlike `renderUnitFor`, which already took `goos` as an - explicit parameter for exactly this reason). Refactored all four to take `goos string` - explicitly, updated every call site in `cmd_service.go` to pass `runtime.GOOS`, and added + explicit parameter for exactly this reason). Refactors all four to take `goos string` + explicitly, updates every call site in `cmd_service.go` to pass `runtime.GOOS`, and adds `cmd_service_systemd_test.go` — a `fakeSystemctl`/`fakeLoginctl` harness mirroring `fakeLaunchctl`, plus tests that actually run (not skip) on any host by passing `"linux"` - explicitly. Confirmed by running for real (not just reading the code) that `is-active` + explicitly. Confirms by running for real (not just reading the code) that `is-active` already correctly treats `activating`/`failed`/`deactivating`/`inactive` as "not running" — only `active` (exit 0) reads as running. That logic was already correct; it was just unverified. Also covers: `loadService`'s daemon-reload/enable failure messages, the linger-enable/marker-write logic (skip when already on, write marker only when *we* turn it on, caveat-not-fatal when `enable-linger` fails), `unloadService`'s marker-gated `disable-linger` (never called without a marker), and `controlService`'s - exact verb-per-action mapping. 22 subtests, all passing. + exact verb-per-action mapping. 27 subtests, all passing. - **Still open:** none of this proves the *real* `systemctl`/`loginctl` actually behave this way — only that our own code reacts correctly to inputs we scripted. That's Tier 3 - (real systemd integration test), still not built. + (real systemd integration test), bullet 5 above. ### 7. `stop` tolerates the proxy's ~15s drain — **#1079** - **Exists:** the proxy's own 15s shutdown timeout (`main.go:671`) is the anchor value everything else has to respect. macOS handles this *explicitly* in Go (`serviceBootoutTimeout = 30*time.Second`, `cmd_service.go:39-42`, plus the supervisor's own 20s-before-SIGKILL logic in `supervise.go:88-95`, deliberately longer than 15s). -- **Fixed:** added an explicit `TimeoutStopSec=20` to `renderUnitFor("linux", ...)` +- **#1079 (open):** adds an explicit `TimeoutStopSec=20` to `renderUnitFor("linux", ...)` (`cmd_service_platform.go`), matching the macOS supervisor's 20s headroom over the proxy's 15s drain, with a rationale comment in the same style as the surrounding `StartLimit*`/`network-online.target` comments. `TestRenderUnit_BothPlatforms`'s - `"linux restarts on failure only"` subtest now asserts the line is present, so a future + `"linux restarts on failure only"` subtest asserts the line is present, so a future regression that drops it fails CI instead of silently reverting to systemd's undocumented - default. Previously it "worked" only by accident of systemd's 90s default exceeding 15s — - now it's an explicit, tested value. + default. Today it "works" only by accident of systemd's 90s default exceeding 15s; + #1079 makes it an explicit, tested value instead. ### 8. Works under user systemd, and states what happens where systemd is absent - **Exists (this is the best-handled bullet):** `loadService` gives a clear, @@ -271,12 +271,12 @@ Full detail came from a source-code audit (Explore agent, 25 tool calls, full re *(Written during the initial research pass; superseded as of 2026-09-21 — kept for history, corrected below rather than deleted.)* -1. ~~The systemd rendering/string-shape logic is solid and well tested. The gap is almost +1. The systemd rendering/string-shape logic is solid and well tested. The gap was almost entirely at the "does this actually work against a real system service manager" - layer — nothing fakes or drives real `systemctl`/`loginctl` for Linux~~ — closed by - #1080 (fake harness, `cmd_service_systemd_test.go`) and this branch, #1076 (real - systemd, `cmd_service_systemd_integration_test.go`, wired into CI) — giving Linux a - real-integration test macOS still does not have (see #944 relationship note below). + layer — nothing faked or drove real `systemctl`/`loginctl` for Linux. This branch + (#1076) closes the real-systemd half (`cmd_service_systemd_integration_test.go`, wired + into CI here) — giving Linux a real-integration test macOS still does not have (see + #944 relationship note below). The fake-driving half is #1080, still open. 2. ~~That simplicity has never been backed by the same real-world verification that justified and shaped the macOS design~~ — it now has: #1076 confirmed `Restart=on-failure` against a live systemd, in CI, 2026-09-21. @@ -315,7 +315,13 @@ history, corrected below rather than deleted.)* - **#944** — macOS mirror of this issue. Owned by @huang195. Already has real end-to-end verification behind it (the KeepAlive finding, bootout-race fix #880) that Linux still lacks — worth mining `cmd_service_platform.go`'s darwin comments and tests as a template - for what "verified" should look like on Linux. + for what "verified" should look like on Linux. **Untracked gap found while building + #1076's Linux equivalent:** `ABCTL_LAUNCHD_TESTS=required` exists in + `cmd_service_bootout_test.go` but is never set by any workflow, so + `TestWaitBootedOut_RealLaunchd` still skips on every CI run, including this repo's own — + the exact trap #1076 was built to avoid on the Linux side. No macOS runner exists in this + repo to set it against, so it can't be closed from here; flagging it so it's tracked + against #944 or #956 (macOS smoke tests) rather than rediscovered later. - **#955** — unattended agent workloads / headless capture path, owned by @esnible. Both #956 and #957 need it for the "non-zero token count" smoke-test assertion. Not required for #945 itself. From 356ff6e90907a52223fb71f5fd8802de45fa53f5 Mon Sep 17 00:00:00 2001 From: Alan Cha Date: Tue, 22 Sep 2026 14:10:09 -0400 Subject: [PATCH 14/17] docs: Restructure the plan doc to match this repo's own convention MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The combined plan doc was copied identically across three independently-reviewed branches and tried to track all three PRs' live merge status via prose ("CLOSED", "Fixed:", "sibling PR #X (open)"). That drifted every time any of the three changed, which is exactly what produced round after round of "must-fix: this claims work that hasn't landed" review comments. None of the other docs in authbridge/docs/superpowers/ do this. Checked all nine: they track status via task checkboxes scoped to one document/one branch/one eventual PR (main-channel.md's last section is literally "Task 7: ... and PR", singular), and reference the parent issue number in the header rather than tracking sibling PRs' merge state inline. Multi-PR work gets separate documents (pricing-consolidation-core.md / -phase0.md), not one document describing several. Split accordingly: - specs/2026-09-16-linux-systemd-lifecycle-design.md: the frozen research (systemd/launchd background, the full gap analysis as originally found) — timeless, makes no claims about which PR fixes what, so it can't go stale the way the old doc did. - plans/2026-09-21-systemd-real-integration-test.md: a small, self-contained task list for this branch's own work only, no reference to #1079/#1080's status at all. Deletes the old combined doc. Assisted-By: Claude (Anthropic AI) Signed-off-by: Alan Cha --- .../2026-09-16-linux-install-systemd-945.md | 339 ------------------ ...026-09-21-systemd-real-integration-test.md | 51 +++ ...26-09-16-linux-systemd-lifecycle-design.md | 161 +++++++++ 3 files changed, 212 insertions(+), 339 deletions(-) delete mode 100644 authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md create mode 100644 authbridge/docs/superpowers/plans/2026-09-21-systemd-real-integration-test.md create mode 100644 authbridge/docs/superpowers/specs/2026-09-16-linux-systemd-lifecycle-design.md diff --git a/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md b/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md deleted file mode 100644 index d3a509a83..000000000 --- a/authbridge/docs/superpowers/plans/2026-09-16-linux-install-systemd-945.md +++ /dev/null @@ -1,339 +0,0 @@ -# Issue #945 — Verified Linux install and systemd service lifecycle - -**Status:** research + gap analysis complete. This branch (#1076) delivers the Tier 3 -real-systemd integration test (bullet 5) — wired into CI, and confirmed passing against a -real systemd: `Restart=on-failure` really does restart a crashed unit, and a deliberate -stop really doesn't trigger one. Two real bugs the first CI run caught (a `systemd-run` -argument mistake, and this doc tripping an unrelated guard test) were fixed in the same -PR. Bullets 6 and 7 are #1080 (Tier 2 fake harness + the `goos` refactor it required) and -#1079 (the `TimeoutStopSec` fix). Tier 4 (install.sh smoke test, #957) and Tier 5 (reboot -check, #964) -are separate issues, not started. -**Owner:** Alan Cha (per epic #962 owner table: "Linux install, release smoke tests, reboot -verification, CI"). -**Repo:** rossoctl/cortex. This doc lives in the same directory as other planning docs -(`authbridge/docs/superpowers/plans/`) for consistency, but is a research/gap-analysis -reference, not yet an execution-ready task plan. - -## Why this doc exists - -Assigned three issues (#945, #956, #957) under epic #962 ("Cortex v0.9.0 — stable local tool -for coding agents", due 2026-09-30). This doc captures the research done to plan #945 -specifically, so the findings don't have to be re-derived later. #956/#957 have their own -dependencies (see "Relationship to other issues" below) and are intentionally out of scope -here. - -## The three assigned issues and priority order - -| # | Title | Why this order | -|---|---|---| -| **#945** | Verified Linux install and systemd service lifecycle | **Do first.** Self-contained, no dependency on other owners' unfinished work. #957 automates verifying this. | -| **#957** | Release smoke test on Linux CI runners | **Second.** Automates #945; same owner, same platform, lowest marginal complexity once #945's gaps are closed. Partial dependency on #955 (unattended capture path, owned by @esnible) for one checklist item (asserting a non-zero token count). | -| **#956** | Release smoke test on macOS CI runners | **Last.** Depends on #944 (macOS install, owned by @huang195) and #955 — two external blockers outside this owner's control, on a platform (launchd) not owned here. | - -## Correcting an early false start: this is not greenfield - -Initial read of the issue text (and of a stale local git checkout, ~452 commits behind -`origin/main`) suggested none of this existed yet. After syncing to `origin/main`, that's -wrong: the core service-lifecycle feature already shipped in **PR #876** ("Feat: Keep Cortex -running across crashes and logins (launchd / systemd)"), with follow-on fixes: - -- **#880** — Fix: upgrades fail with launchd `EIO` (bootout race) — macOS-only issue. -- **#897** — Fix: detect environments that cannot run a service, and say what to do. -- **#931** — Fix: say when a new CA leaves running agents unobservable. -- **#911** — Fix: trust the bridge CA where it matters, stop intercepting dev tooling. - -So #945 is a **verification and gap-closing** issue on top of substantially-built -functionality, not a build-from-scratch feature. The lesson for next time: always confirm -`git fetch` / compare against `origin/main` before concluding something doesn't exist. - -## Conceptual background: what systemd and launchd are for, and why the two platforms differ - -A service manager's job: start a background process, notice when it dies (including -crashes, not just deliberate stops), and decide whether to relaunch it — without a human -watching a terminal. - -**The key asymmetry that shapes the whole design:** - -- **systemd** (Linux): a unit file's `Restart=on-failure` is honored by systemd's own - supervisor loop, reliably, even for a unit just `systemctl --user start`ed while already - logged in (exactly Cortex's `curl | sh` scenario). -- **launchd** (macOS): the team found — and documented in code comments — that launchd's - equivalent (`KeepAlive`, tried alongside `StartInterval` and `RunAtLoad`) does **not** - reliably restart a LaunchAgent added mid-session (as opposed to one present at boot when - launchd first scans `LaunchAgents`). Since Cortex is always installed mid-session, this - gap is squarely in the installer's path. - -**Consequence for the architecture:** - -- **macOS** runs **two processes**: launchd starts a small Go-written supervisor - (`authbridge-proxy --supervise`, in `authbridge/cmd/authbridge-proxy/supervise.go`) that - itself watches and restarts the actual proxy child. launchd supervises the supervisor; - the supervisor does the real crash-recovery job launchd won't reliably do. -- **Linux** runs **one process**: `authbridge-proxy` directly as the systemd unit's - `ExecStart=`, with `Restart=on-failure` doing all the crash-recovery work natively. No - supervisor binary involved. Confirmed in code: - `cmd_service_platform.go:92-93` and `TestSupervisionIsPlatformCorrect` - (`cmd_service_test.go:312-332`) explicitly assert Linux does **not** use `--supervise`. - -This means **#945 is structurally simpler than #944** on the crash-recovery axis — but it -also means the Linux "Restart=on-failure actually works" assumption has never been -put through the same real-world verification (and design correction) that produced the -macOS supervisor. That's the single biggest theme in the gap analysis below. - -## Where things live (for orientation) - -- `authbridge/install.sh` — the `curl | sh` installer (~1300 lines, POSIX `sh`). -- `authbridge/install_test.sh` — installer unit tests (shell functions in isolation, no - real network/systemctl). -- `authbridge/cmd/abctl/cmd_service.go` — cross-platform `abctl service` command logic - (install/uninstall/status/start/stop/restart), ~870 lines. -- `authbridge/cmd/abctl/cmd_service_platform.go` — the OS-specific half: systemd unit - rendering, launchd plist rendering, `systemctl`/`launchctl`/`loginctl` invocations, ~723 - lines. -- `authbridge/cmd/authbridge-proxy/supervise.go` — the macOS-only (and unsupervised-fallback) - Go restart loop, ~118 lines. -- `authbridge/cmd/authbridge-proxy/main.go` — the proxy itself; on SIGTERM/SIGINT does a - graceful shutdown with a **15-second drain** (`context.WithTimeout(..., 15*time.Second)`, - `main.go:671`) — every downstream "does stop tolerate the drain" question traces back to - this constant. -- `authbridge/docs/laptop-service.md` — user-facing doc for `abctl service *`, - `~/.cortex/` layout, restricted-environment fallback (`authbridge-proxy --local`), and - manual-removal instructions. Good source of ground truth for expected behavior. -- `authbridge/cmd/abctl/cmd_service_*_test.go` — several files split by concern (bootout, - durable, recovery, restricted, stamp, perm). See gap analysis for which of these actually - exercise Linux. - -## Gap analysis against #945's checklist - -Full detail came from a source-code audit (Explore agent, 25 tool calls, full read of -`cmd_service.go`, `cmd_service_platform.go`, `supervise.go`, `install.sh`, and every -`cmd_service_*_test.go` file). Condensed per checklist bullet: - -### 1. Fresh install via `curl | sh`, amd64 and arm64 -- **Exists:** OS/arch detection (`install.sh:816-828`); both Linux arches are actually - published by `release-binaries.yaml:156`; checksum verification prefers `shasum`, falls - back to `sha256sum`; `supervisor_usable()` (`install.sh:645-653`) does a *live* preflight - (`systemctl --user show-environment`, not just `command -v systemctl`) — this is what - catches containers/minimal images where the binary exists but no user session does. -- **Gap:** No CI job or test actually installs and runs the real released binaries against a - live Linux box, for either architecture. `install_test.sh` only unit-tests shell functions - with system commands stubbed out. There is no arm64 CI runner at all — arm64 is only - exercised at *build* time, never at *install/run* time. The issue text says this is - "automated by the Linux release smoke test" — **no such smoke-test workflow exists yet** - (this is exactly #957's job, and #957 depends on #945 being solid first). - -### 2. Upgrade over an older install: service restarted, config preserved -- **Exists:** `serviceInstall` idempotency via `installCanSkip`/`serviceIsCurrent` - (`cmd_service.go:774-836`); config migration (`migrateConfig`, `cmd_service.go:343`) before - restart; binary-swap detection via SHA-256 stamp file - (`proxyBinaryUnchanged`, `cmd_service_platform.go:806-824`, platform-agnostic and well - tested). Linux restart is `systemctl --user daemon-reload` then `enable --now` - (`cmd_service_platform.go:216-247`) — no bootout-race workaround needed, per explicit - comment (`cmd_service_platform.go:92-93`) that systemd's own `Restart=`/`StartLimit*` - make the launchd EIO workaround (#880) unnecessary on Linux. -- **Gap:** Nothing drives an actual upgrade against a real running `systemd --user` unit to - confirm the restart genuinely leaves Cortex serving afterward. macOS has a dedicated - real-launchd integration test for its equivalent scenario - (`TestWaitBootedOut_RealLaunchd`); there is no Linux analog. - -### 3. Uninstall leaves no unit file, no `~/.cortex`, no modified agent settings -- **Exists:** `serviceUninstall` (`cmd_service.go:503-538`) removes the unit + stamp file, - deliberately leaves `~/.cortex` alone (that's a separate, manual step per - `laptop-service.md`). Linux `unloadService` (`cmd_service_platform.go:269-291`) tracks - whether *it* enabled lingering (via a marker file) so uninstall doesn't clobber a linger - setting the user had already set for unrelated units — a thoughtful, Linux-specific detail. -- **Gap:** None of this Linux uninstall logic (the linger-marker branch, - `systemctl --user disable --now`) has a test with faked `systemctl`/`loginctl` — the - existing `TestStopIsDurable` only greps source for expected strings, it doesn't run the - code path. "No modified agent settings" is really an `abctl claude-code disable` concern, - outside the files audited here — worth confirming separately. - -### 4. Re-running install is idempotent -- **Exists:** `installCanSkip` (well tested, platform-agnostic); `install.sh`'s - already-at-this-version guard (`install.sh:885-889`); `lingerEnabled()` - (`cmd_service_platform.go:259-267`) avoids re-toggling linger on repeat installs. -- **Gap:** `lingerEnabled`'s parsing of `loginctl show-user --property=Linger` output has - zero test coverage — a parsing regression could silently re-enable linger every install, - or silently skip enabling it when needed. - -### 5. Service survives reboot and a crash — **CLOSED (crash half) 2026-09-21; reboot half is #964** -- **Added:** `cmd_service_systemd_integration_test.go` — mirrors - `TestWaitBootedOut_RealLaunchd`'s structure exactly: a throwaway `systemd-run --user` - transient unit, a synthetic slow-to-exit script (not the real proxy, for the same - isolation reason the darwin test uses "slow.sh"), the same four skip guards - (wrong OS, missing binaries, no reachable `systemctl --user` session) plus an - `ABCTL_SYSTEMD_TESTS=required` escape hatch. Two tests: - `TestSupervisorRestartsAfterCrash_RealSystemd` (`kill -9` the main PID, confirm the - unit comes back with a *different* PID) and - `TestSupervisorStaysStoppedAfterDeliberateStop_RealSystemd` (a deliberate - `systemctl stop` must NOT trigger a restart). -- **Confirmed for real, against a live systemd, in CI** (PR #1076, - `go-ci-authbridge-cmd` / abctl leg, run 35558708292, 2026-09-21): - `TestSupervisorRestartsAfterCrash_RealSystemd` **PASS (3.09s)**, - `TestSupervisorStaysStoppedAfterDeliberateStop_RealSystemd` **PASS (7.09s)**. This is - the actual real-world verification the darwin `KeepAlive` claim already had (tested, - found false, drove the supervisor redesign) and the Linux `Restart=on-failure` - assumption never did — now it does, and it held up. -- **Two genuine bugs the first "blind" CI run caught, exactly as intended:** - 1. `systemd-run` is not `systemctl`-shaped — there's no separate `run` verb; - `systemd-run` itself IS the run action. An extra literal `"run"` argument made - both tests fail identically (`Failed to find executable run`) on the first push. - Fixed by removing it. - 2. This same planning doc's own reference to the retired `exclude_plugin_*` tag form - (as design history for #966) tripped `TestNoExcludePluginTagsRemain` — a guard - from a different, unrelated part of this codebase. Fixed with the - `allow-legacy-plugin-tag` marker its own error message names. - Neither was a systemd-behavior surprise — both were caught and fixed within the same - PR before merge, which is the point of running this blind rather than guessing. -- **One useful incidental finding:** this repo's `ubuntu-latest` (Ubuntu 24) runner image - already ships `XDG_RUNTIME_DIR`/`DBUS_SESSION_BUS_ADDRESS` pointed at a live user - session by default — the `enable-linger` setup step still ran and succeeded, but the - base image may need less setup than assumed going in. -- **Still open:** this only proves crash-recovery within a live session. It does **not** - prove lingering survives an actual logout/reboot — that's a different claim, and - deliberately out of scope here; see #964. -- **Exists (and well-reasoned):** unit rendering (`renderUnitFor("linux", ...)`, - `cmd_service_platform.go:131-148`) sets `Restart=on-failure`, `RestartSec=10`, - `StartLimitIntervalSec=300`, `StartLimitBurst=5` — correctly placed in `[Unit]`, not - `[Service]` (a comment explains systemd v229+ ignores/deprecates it in `[Service]`, which - would silently void the throttle). `[Install] WantedBy=default.target` handles - reboot/login persistence. Good unit-level string-shape test coverage - (`TestRenderUnit_BothPlatforms`, `cmd_service_test.go:65-106`). -- **Gap:** No test — unit, integration, or even a documented manual-verification note — - proves that a real `systemd --user` instance actually restarts the unit after `kill -9`, - or survives a real reboot/logout with lingering enabled. The macOS side has exactly this - kind of verification on record: comments at `cmd_service_platform.go:44-58` document that - `KeepAlive` was tested, found **not** to work end-to-end, and that finding drove the - supervisor-process redesign. **No equivalent verification episode exists for Linux's - `Restart=on-failure` assumption** — it's currently trusted, not proven. This is precisely - the class of gap #945 (and its manual-reboot sibling, #964) exists to close. - -### 6. `abctl service status | start | stop | restart` accurate in every state — **#1080** -- **Exists:** `serviceStatus`/`serviceControl` (`cmd_service.go:540-665`) are - platform-agnostic; Linux `controlService` maps stop→`disable --now` (persistent), - start→`enable --now`, restart→plain `systemctl --user restart` (transient, by design). - `supervisorRunning`'s Linux branch uses `systemctl --user is-active cortex.service`, with a - sane fallback ("no systemctl → treat as unknown, don't block") for systemd-less boxes. -- **#1080 (open):** `loadService`, `controlService`, `supervisorRunning`, and `unloadService` - took `runtime.GOOS` directly (unlike `renderUnitFor`, which already took `goos` as an - explicit parameter for exactly this reason). Refactors all four to take `goos string` - explicitly, updates every call site in `cmd_service.go` to pass `runtime.GOOS`, and adds - `cmd_service_systemd_test.go` — a `fakeSystemctl`/`fakeLoginctl` harness mirroring - `fakeLaunchctl`, plus tests that actually run (not skip) on any host by passing `"linux"` - explicitly. Confirms by running for real (not just reading the code) that `is-active` - already correctly treats `activating`/`failed`/`deactivating`/`inactive` as "not - running" — only `active` (exit 0) reads as running. That logic was already correct; it - was just unverified. Also covers: `loadService`'s daemon-reload/enable failure messages, - the linger-enable/marker-write logic (skip when already on, write marker only when *we* - turn it on, caveat-not-fatal when `enable-linger` fails), `unloadService`'s - marker-gated `disable-linger` (never called without a marker), and `controlService`'s - exact verb-per-action mapping. 27 subtests, all passing. -- **Still open:** none of this proves the *real* `systemctl`/`loginctl` actually behave - this way — only that our own code reacts correctly to inputs we scripted. That's Tier 3 - (real systemd integration test), bullet 5 above. - -### 7. `stop` tolerates the proxy's ~15s drain — **#1079** -- **Exists:** the proxy's own 15s shutdown timeout (`main.go:671`) is the anchor value - everything else has to respect. macOS handles this *explicitly* in Go - (`serviceBootoutTimeout = 30*time.Second`, `cmd_service.go:39-42`, plus the supervisor's - own 20s-before-SIGKILL logic in `supervise.go:88-95`, deliberately longer than 15s). -- **#1079 (open):** adds an explicit `TimeoutStopSec=20` to `renderUnitFor("linux", ...)` - (`cmd_service_platform.go`), matching the macOS supervisor's 20s headroom over the - proxy's 15s drain, with a rationale comment in the same style as the surrounding - `StartLimit*`/`network-online.target` comments. `TestRenderUnit_BothPlatforms`'s - `"linux restarts on failure only"` subtest asserts the line is present, so a future - regression that drops it fails CI instead of silently reverting to systemd's undocumented - default. Today it "works" only by accident of systemd's 90s default exceeding 15s; - #1079 makes it an explicit, tested value instead. - -### 8. Works under user systemd, and states what happens where systemd is absent -- **Exists (this is the best-handled bullet):** `loadService` gives a clear, - actionable message when `systemctl` isn't found at all, naming WSL1/containers as likely - causes (`cmd_service_platform.go:216-219`). `install.sh`'s `supervisor_usable()` does the - more realistic check — `systemctl` present but user manager/D-Bus unreachable — and falls - back to `start_unsupervised` (a plain backgrounded process using the proxy's own - `--supervise` loop), printing manual start/stop/status/logs instructions. Well tested at - the shell-function level (`install_test.sh:570-575`). -- **Gap:** On the Go side, there's no Linux equivalent of `launchdUsable()` - (`cmd_service_platform.go:680-696`, explicitly darwin-only) — the function that lets - macOS refuse cleanly *before writing anything to disk* with a dedicated, tested exit code - (`exitNoSupervisor`, exit 4). On Linux, "no systemd" is discovered later, inside - `loadService`, after the unit file has already been written (then cleaned up on failure). - End behavior is probably fine — `install.sh`'s generic fallback classifier still buckets - it correctly — but it's fine by accident/genericness, not by an intentional, tested, - Linux-aware preflight the way darwin gets. No Go test asserts the exact message or exit - code for "systemctl absent" the way `TestServiceInstall_RestrictedEnvironment` does for - darwin. - -## Cross-cutting themes - -*(Written during the initial research pass; superseded as of 2026-09-21 — kept for -history, corrected below rather than deleted.)* - -1. The systemd rendering/string-shape logic is solid and well tested. The gap was almost - entirely at the "does this actually work against a real system service manager" - layer — nothing faked or drove real `systemctl`/`loginctl` for Linux. This branch - (#1076) closes the real-systemd half (`cmd_service_systemd_integration_test.go`, wired - into CI here) — giving Linux a real-integration test macOS still does not have (see - #944 relationship note below). The fake-driving half is #1080, still open. -2. ~~That simplicity has never been backed by the same real-world verification that - justified and shaped the macOS design~~ — it now has: #1076 confirmed - `Restart=on-failure` against a live systemd, in CI, 2026-09-21. -3. ~~One concrete, low-risk fix stands out: add `TimeoutStopSec=`~~ — #1079; see bullet 7. -4. **No real Linux CI smoke test exists yet anywhere in the repo** — still true, this - remains #957's job. What's changed: #957 now has a verified `Restart=on-failure` - foundation to build on, rather than an unverified assumption underneath it. - -## Suggested next steps (not yet sequenced into a task plan) - -1. Add an explicit `TimeoutStopSec=` to `renderUnitFor("linux", ...)` — #1079; see bullet 7 - above. -2. Build a `fakeSystemctl`/`fakeLoginctl` test harness — #1080; see bullet 6 above. Required - refactoring - `loadService`/`controlService`/`supervisorRunning`/`unloadService` to take `goos` - explicitly first (same fix `renderUnitFor` already had) — otherwise these functions - can't be exercised from a non-Linux host at all. -3. ~~Get real verification that `Restart=on-failure` actually recovers the unit after - `kill -9`~~ — **done 2026-09-21, confirmed passing in real CI**, see bullet 5 above - (PR #1076). Still open: lingering/reboot-survival is a separate claim this test does - not cover — see #964. -4. Decide whether Linux needs its own `launchdUsable()`-equivalent preflight (tested, named - exit code) or whether the current "discover it inside `loadService`, clean up, fall - through to the generic fallback" behavior is an acceptable, intentional asymmetry. -5. ~~Wire `cmd_service_systemd_integration_test.go` into CI~~ — **done 2026-09-21** - (PR #1076): `enable-linger` + - `XDG_RUNTIME_DIR` setup step on the `abctl` leg of `go-ci-authbridge-cmd` in `ci.yaml`, - `ABCTL_SYSTEMD_TESTS=required` set for that job — unlike the darwin equivalent - (`ABCTL_LAUNCHD_TESTS=required`), which exists in code but is never set by any - workflow, so `TestWaitBootedOut_RealLaunchd` has skipped in every CI run since it was - written. This one now genuinely runs on every PR. Needed no new runner type — `ubuntu-latest` - already has a real systemd. This is also the foundation #957's smoke test can build on. - -## Relationship to other issues (for context, not in scope here) - -- **#944** — macOS mirror of this issue. Owned by @huang195. Already has real end-to-end - verification behind it (the KeepAlive finding, bootout-race fix #880) that Linux still - lacks — worth mining `cmd_service_platform.go`'s darwin comments and tests as a template - for what "verified" should look like on Linux. **Untracked gap found while building - #1076's Linux equivalent:** `ABCTL_LAUNCHD_TESTS=required` exists in - `cmd_service_bootout_test.go` but is never set by any workflow, so - `TestWaitBootedOut_RealLaunchd` still skips on every CI run, including this repo's own — - the exact trap #1076 was built to avoid on the Linux side. No macOS runner exists in this - repo to set it against, so it can't be closed from here; flagging it so it's tracked - against #944 or #956 (macOS smoke tests) rather than rediscovered later. -- **#955** — unattended agent workloads / headless capture path, owned by @esnible. Both - #956 and #957 need it for the "non-zero token count" smoke-test assertion. Not required - for #945 itself. -- **#964** — manual reboot verification (macOS + Linux), explicitly deferred from #944/#945 - because CI runners don't reboot. Shares the exact "does Restart=on-failure/launchd - KeepAlive really survive a restart" question raised above — worth coordinating rather than - duplicating. -- **#966** — plugin build-tag convention cleanup (retired `exclude_plugin_*` form → - `include_plugin_*`; allow-legacy-plugin-tag: this reference is design history, not a - live usage) - and a smaller desktop artifact. Touches the same install/upgrade path (H2 in that issue: - upgrading to a build that dropped a plugin an existing config still names must be handled - gracefully) — flagged there as something #944/#945/#964 must cover. Worth checking in on - this once the plugin-set change lands, since it could break an assumption made above about - upgrade always being config-preserving. diff --git a/authbridge/docs/superpowers/plans/2026-09-21-systemd-real-integration-test.md b/authbridge/docs/superpowers/plans/2026-09-21-systemd-real-integration-test.md new file mode 100644 index 000000000..9cbf86fac --- /dev/null +++ b/authbridge/docs/superpowers/plans/2026-09-21-systemd-real-integration-test.md @@ -0,0 +1,51 @@ +# Real-systemd integration test for crash recovery — Implementation Plan + +**Goal:** Prove, against a live `systemd --user` session rather than by reading the +unit file, that `Restart=on-failure` actually restarts a crashed unit and that a +deliberate stop does not — closing checklist bullet 5 (crash half) of #945 for real. + +**Architecture:** A throwaway `systemd-run --user` transient unit running a +synthetic slow-to-exit script (not the real proxy, for isolation). Covers the same +ground as macOS's `TestWaitBootedOut_RealLaunchd` deliberately in a different +shape — three skip-guard categories here versus that test's four, since systemd's +native restart means there's no supervisor layer or bootout race to reproduce, just +the bare `Restart=on-failure` claim itself. Wired into the `abctl` leg of +`go-ci-authbridge-cmd` via a new `enable-linger` + `XDG_RUNTIME_DIR` setup step. + +**Tech Stack:** Go 1.26.5, `systemd-run`/`systemctl --user`, GitHub Actions. + +**Spec:** `authbridge/docs/superpowers/specs/2026-09-16-linux-systemd-lifecycle-design.md` + +**Issue:** cortex #945 + +## Tasks + +- [x] `cmd_service_systemd_integration_test.go`: `requireRealSystemd` skip-guard + helper (wrong GOOS, missing binaries, no reachable `systemctl --user` + session), with `ABCTL_SYSTEMD_TESTS=required` escape hatch mirroring + `ABCTL_LAUNCHD_TESTS`. +- [x] `TestSupervisorRestartsAfterCrash_RealSystemd`: `kill -9` the unit's main PID, + confirm it comes back with a genuinely new PID (`waitForNewMainPID`, not just + `is-active`, to close a race window right after the kill). +- [x] `TestSupervisorStaysStoppedAfterDeliberateStop_RealSystemd`: a deliberate + `systemctl stop` must not trigger a restart. +- [x] CI: `enable-linger` + wait-for-bus-socket + `XDG_RUNTIME_DIR` setup step on the + `abctl` leg of `go-ci-authbridge-cmd`, `ABCTL_SYSTEMD_TESTS=required` set for + that job only. +- [x] Fix: `systemd-run` invocation had a stray literal `"run"` argument (unlike + `systemctl`, `systemd-run` has no separate verb) — caught by the first real + CI run, both tests failing identically. +- [x] Fix: cleanup's `t.Logf` on `stop`/`reset-failed` failure printed on every + normal passing run — `systemd-run` transient units are garbage-collected once + inactive, so both calls routinely exit nonzero (5, 1) by teardown time. + Filtered those two specific, confirmed-benign exit codes. +- [x] Confirmed against real CI (not just local skip behavior): both tests pass with + realistic timing (~3s crash-restart, ~7s stay-stopped), and the cleanup fix + verified to produce zero spurious log lines on a clean run. + +## Result + +Confirmed for real, against a live systemd: `Restart=on-failure` restarts a crashed +unit, and a deliberate stop does not. This is the real-world verification the +darwin `KeepAlive` claim already had (tested, found false, drove the supervisor +redesign) that the Linux assumption never did. diff --git a/authbridge/docs/superpowers/specs/2026-09-16-linux-systemd-lifecycle-design.md b/authbridge/docs/superpowers/specs/2026-09-16-linux-systemd-lifecycle-design.md new file mode 100644 index 000000000..70529f6e4 --- /dev/null +++ b/authbridge/docs/superpowers/specs/2026-09-16-linux-systemd-lifecycle-design.md @@ -0,0 +1,161 @@ +# Linux install and systemd service lifecycle — Design + +**Date:** 2026-09-16 +**Status:** Research and gap analysis complete. +**Issue:** cortex #945 — "feature: verified Linux install and systemd service lifecycle" +**Repos touched:** `cortex/authbridge` only. + +## Problem + +#945 is a verification and gap-closing issue, not a build-from-scratch feature: the +core service-lifecycle mechanism already shipped in PR #876 ("Keep Cortex running +across crashes and logins (launchd / systemd)"), with follow-on fixes #880, #897, +#931, #911. What follows is what auditing that existing code against #945's +checklist found. + +## Conceptual background: systemd vs. launchd, and why the two platforms differ + +A service manager's job: start a background process, notice when it dies (including +crashes, not just deliberate stops), and decide whether to relaunch it — without a +human watching a terminal. + +**The key asymmetry that shapes the whole design:** + +- **systemd** (Linux): a unit file's `Restart=on-failure` is honored by systemd's own + supervisor loop, reliably, even for a unit just `systemctl --user start`ed while + already logged in (exactly Cortex's `curl | sh` scenario). +- **launchd** (macOS): the team found — and documented in code comments — that + launchd's equivalent (`KeepAlive`, tried alongside `StartInterval` and + `RunAtLoad`) does **not** reliably restart a LaunchAgent added mid-session (as + opposed to one present at boot when launchd first scans `LaunchAgents`). Since + Cortex is always installed mid-session, this gap is squarely in the installer's + path. + +**Consequence for the architecture:** + +- **macOS** runs **two processes**: launchd starts a small Go-written supervisor + (`authbridge-proxy --supervise`, `authbridge/cmd/authbridge-proxy/supervise.go`) + that itself watches and restarts the actual proxy child. launchd supervises the + supervisor; the supervisor does the real crash-recovery job launchd won't + reliably do. +- **Linux** runs **one process**: `authbridge-proxy` directly as the systemd unit's + `ExecStart=`, with `Restart=on-failure` doing all the crash-recovery work + natively. No supervisor binary involved. Confirmed in code: + `cmd_service_platform.go:92-93` and `TestSupervisionIsPlatformCorrect` + (`cmd_service_test.go:312-332`) explicitly assert Linux does **not** use + `--supervise`. + +This makes #945 structurally simpler than #944 on the crash-recovery axis — but it +also means the Linux "`Restart=on-failure` actually works" assumption had never been +put through the same real-world verification (and design correction) that produced +the macOS supervisor. That was the single biggest gap this issue needed to close. + +## Where things live + +- `authbridge/install.sh` — the `curl | sh` installer (~1300 lines, POSIX `sh`). +- `authbridge/install_test.sh` — installer unit tests (shell functions in isolation, + no real network/systemctl). +- `authbridge/cmd/abctl/cmd_service.go` — cross-platform `abctl service` command + logic (install/uninstall/status/start/stop/restart). +- `authbridge/cmd/abctl/cmd_service_platform.go` — the OS-specific half: systemd + unit rendering, launchd plist rendering, `systemctl`/`launchctl`/`loginctl` + invocations. +- `authbridge/cmd/authbridge-proxy/supervise.go` — the macOS-only (and + unsupervised-fallback) Go restart loop. +- `authbridge/cmd/authbridge-proxy/main.go` — the proxy itself; on SIGTERM/SIGINT + does a graceful shutdown with a **15-second drain** + (`context.WithTimeout(..., 15*time.Second)`, `main.go:671`) — every "does stop + tolerate the drain" question traces back to this constant. +- `authbridge/docs/laptop-service.md` — user-facing doc for `abctl service *`, + `~/.cortex/` layout, restricted-environment fallback, manual-removal + instructions. + +## Gap analysis against #945's checklist, as found + +### 1. Fresh install via `curl | sh`, amd64 and arm64 +Install-time OS/arch detection, checksum verification, and a live preflight +(`supervisor_usable()`, `install.sh:645-653`) all exist and are unit-tested. No CI +job installs and runs the real released binaries against a live Linux box, for +either architecture — arm64 is exercised at build time only. + +### 2. Upgrade over an older install: service restarted, config preserved +Idempotency (`installCanSkip`/`serviceIsCurrent`), config migration, and +binary-swap detection all exist and are well tested. Nothing drives an actual +upgrade against a real running `systemd --user` unit — macOS has a dedicated +real-launchd test for the equivalent scenario (`TestWaitBootedOut_RealLaunchd`); +Linux has no analog. + +### 3. Uninstall leaves no unit file, no `~/.cortex`, no modified agent settings +`serviceUninstall`/`unloadService` exist, including a thoughtful linger-marker +mechanism so uninstall doesn't clobber a linger setting the user set for unrelated +units. None of this Linux uninstall logic had a test driving faked +`systemctl`/`loginctl` — the existing test only greps source for expected strings. + +### 4. Re-running install is idempotent +`installCanSkip` and `install.sh`'s already-at-this-version guard exist and are +tested. `lingerEnabled()`'s parsing of `loginctl show-user --property=Linger` +output had zero test coverage. + +### 5. Service survives reboot and a crash — the central gap +Unit rendering (`Restart=on-failure`, `RestartSec=10`, `StartLimit*` correctly +placed in `[Unit]`) is well-reasoned and has good string-shape test coverage. No +test — unit, integration, or a documented manual-verification note — proved that a +real `systemd --user` instance actually restarts the unit after `kill -9`, or +survives a real reboot/logout with lingering enabled. macOS has exactly this kind +of verification on record: `KeepAlive` was tested, found not to work end-to-end, +and that finding drove the supervisor redesign. No equivalent verification episode +existed for Linux's `Restart=on-failure` assumption — trusted, not proven. + +### 6. `abctl service status | start | stop | restart` accurate in every state +`serviceStatus`/`serviceControl` are platform-agnostic and reasonably designed +(`controlService` maps stop→`disable --now`, start→`enable --now`, +restart→transient `restart`). `loadService`, `controlService`, `supervisorRunning`, +and `unloadService` all took `runtime.GOOS` directly rather than an explicit `goos` +parameter (unlike `renderUnitFor`), so none of their Linux logic could be exercised +from a non-Linux host. Nothing faked or drove real `systemctl`/`loginctl` for +Linux at all — macOS has both a `fakeLaunchctl` unit-test harness and a +real-launchd integration test. + +### 7. `stop` tolerates the proxy's ~15s drain +The proxy's own 15s shutdown timeout is the anchor value everything else has to +respect. macOS handles this explicitly in Go (a 30s bootout timeout, plus the +supervisor's own 20s-before-SIGKILL logic). The rendered Linux unit set no +`TimeoutStopSec` at all — it "worked" only by accident of systemd's undocumented +90s default exceeding 15s. + +### 8. Works under user systemd, and states what happens where systemd is absent +The best-handled bullet: `loadService` gives a clear, actionable message when +`systemctl` isn't found; `install.sh`'s `supervisor_usable()` does a live +preflight (`systemctl` present but user manager/D-Bus unreachable) and falls back +to an unsupervised mode. Gap: no Linux equivalent of `launchdUsable()` +(`cmd_service_platform.go:680-696`, darwin-only) — the function that lets macOS +refuse cleanly *before writing anything to disk*, with a dedicated tested exit +code. On Linux, "no systemd" is discovered later, inside `loadService`, after the +unit file has already been written (then cleaned up on failure). Probably fine in +practice, but by accident/genericness rather than an intentional, tested, +Linux-aware preflight — still an open decision, not yet resolved either way. + +## Relationship to other issues + +- **#944** — macOS mirror of this issue, owned by @huang195. Already has real + end-to-end verification behind it (the `KeepAlive` finding, bootout-race fix + #880) that Linux lacked. **A related, separately-discovered gap:** + `ABCTL_LAUNCHD_TESTS=required` exists in `cmd_service_bootout_test.go` (an + escape hatch that turns a silent skip into a hard failure) but is never set by + any workflow, so `TestWaitBootedOut_RealLaunchd` skips in every CI run. No macOS + runner exists in this repo to close that from the Linux side — worth tracking + against #944 or #956 (macOS smoke tests) rather than leaving it to be + rediscovered. +- **#955** — unattended agent workloads / headless capture path, owned by + @esnible. #956 and #957 both need it for a "non-zero token count" smoke-test + assertion. Not required for #945 itself. +- **#964** — manual reboot verification (macOS + Linux), explicitly deferred from + #944/#945 because CI runners don't reboot. Shares the "does + `Restart=on-failure`/launchd `KeepAlive` really survive a restart" question + raised above — worth coordinating rather than duplicating. +- **#966** — plugin build-tag convention cleanup (retired `exclude_plugin_*` form → + `include_plugin_*`; allow-legacy-plugin-tag: this reference is design history, not + a live usage) and a smaller desktop artifact. Touches the same + install/upgrade path: upgrading to a build that dropped a plugin an existing + config still names must be handled gracefully — flagged there as something + #944/#945/#964 must cover. From 5d3372cc0e1be54a18896b452fe7d3da84b6431f Mon Sep 17 00:00:00 2001 From: Alan Cha Date: Tue, 22 Sep 2026 14:34:01 -0400 Subject: [PATCH 15/17] fix: Correct doc-comment attachment and a stale line citation isExitCode's doc comment had absorbed requireRealSystemd's design rationale (no blank line between them), leaving requireRealSystemd itself undocumented. Also fixes the spec doc's main.go line reference for the 15s shutdown timeout (was pointing at unrelated mTLS setup code) and softens an inaccurate "undocumented" claim about systemd's DefaultTimeoutStopSec, which is documented in systemd.system.conf(5). Assisted-By: Claude (Anthropic AI) Signed-off-by: Alan Cha --- .../abctl/cmd_service_systemd_integration_test.go | 12 ++++++------ .../2026-09-16-linux-systemd-lifecycle-design.md | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go b/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go index 4bc486cb3..5a9ead1bf 100644 --- a/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go +++ b/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go @@ -29,6 +29,12 @@ import ( // simpler than the darwin test: there's no supervisor layer or bootout race to // reproduce, just the bare Restart=on-failure claim itself. +// isExitCode reports whether err is a process exit with exactly this code. +func isExitCode(err error, code int) bool { + var ee *exec.ExitError + return errors.As(err, &ee) && ee.ExitCode() == code +} + // requireRealSystemd skips (or, with ABCTL_SYSTEMD_TESTS=required, fails) unless // this process can actually drive a live systemd --user session — covering the same // ground as TestWaitBootedOut_RealLaunchd's skip guards (cmd_service_bootout_test.go), @@ -43,12 +49,6 @@ import ( // the var, though, so the test has skipped in every CI run since it was written. // The one CI job that runs THIS test should set ABCTL_SYSTEMD_TESTS=required after // setting up a real systemd --user session, so it can't fall into the same trap. -// isExitCode reports whether err is a process exit with exactly this code. -func isExitCode(err error, code int) bool { - var ee *exec.ExitError - return errors.As(err, &ee) && ee.ExitCode() == code -} - func requireRealSystemd(t *testing.T) { t.Helper() skip := t.Skipf diff --git a/authbridge/docs/superpowers/specs/2026-09-16-linux-systemd-lifecycle-design.md b/authbridge/docs/superpowers/specs/2026-09-16-linux-systemd-lifecycle-design.md index 70529f6e4..bfe146953 100644 --- a/authbridge/docs/superpowers/specs/2026-09-16-linux-systemd-lifecycle-design.md +++ b/authbridge/docs/superpowers/specs/2026-09-16-linux-systemd-lifecycle-design.md @@ -64,7 +64,7 @@ the macOS supervisor. That was the single biggest gap this issue needed to close unsupervised-fallback) Go restart loop. - `authbridge/cmd/authbridge-proxy/main.go` — the proxy itself; on SIGTERM/SIGINT does a graceful shutdown with a **15-second drain** - (`context.WithTimeout(..., 15*time.Second)`, `main.go:671`) — every "does stop + (`context.WithTimeout(..., 15*time.Second)`, `main.go:957`) — every "does stop tolerate the drain" question traces back to this constant. - `authbridge/docs/laptop-service.md` — user-facing doc for `abctl service *`, `~/.cortex/` layout, restricted-environment fallback, manual-removal @@ -120,8 +120,8 @@ real-launchd integration test. The proxy's own 15s shutdown timeout is the anchor value everything else has to respect. macOS handles this explicitly in Go (a 30s bootout timeout, plus the supervisor's own 20s-before-SIGKILL logic). The rendered Linux unit set no -`TimeoutStopSec` at all — it "worked" only by accident of systemd's undocumented -90s default exceeding 15s. +`TimeoutStopSec` at all — it "worked" only by accident of systemd's own +90s default (`systemd.system.conf(5)`) exceeding 15s. ### 8. Works under user systemd, and states what happens where systemd is absent The best-handled bullet: `loadService` gives a clear, actionable message when From 2dc5c998ffb3ec14a5420a348665a94c09345f1b Mon Sep 17 00:00:00 2001 From: Alan Cha Date: Tue, 22 Sep 2026 15:41:39 -0400 Subject: [PATCH 16/17] fix: Address round-2 review findings on the real-systemd test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Cite main.go's shutdown timeout by symbol, not line number, in the spec doc — main.go isn't part of this PR's diff and keeps moving as unrelated PRs merge, so any line citation there goes stale under continued review regardless of how carefully it's checked once. - Record #1079's TimeoutStopSec=20 as closing gap 7, now that it has merged; the doc previously described a gap that no longer exists on main. - Clarify the #966 reference: that issue is closed, and its allow-legacy-plugin-tag exemption matches the whole file via strings.Contains, not just the one mention. - Replace exec.Command("kill", ...) with syscall.Kill: requireRealSystemd doesn't guard the kill binary, so a slim image missing it would surface as a failed test (with ABCTL_SYSTEMD_TESTS=required set) rather than the environment-problem skip it actually is. - Tie the crash test to renderUnitFor's actual output: Restart=on-failure was hand-copied into the systemd-run args, so deleting the property from renderUnitFor's linux branch wouldn't have failed this test — it would go on proving a fact about systemd the shipped unit no longer requests. - Narrow the reset-failed cleanup filter to match on message rather than exit code: exit 1 is systemd's generic failure code, so filtering by code alone swallowed nearly everything the call could produce, not just the confirmed-benign "unit doesn't exist" case. - Reword the comment above the 1s negative-assertion window; it described a drain that had already finished by the time stop() returns, not a grace period being given here. Assisted-By: Claude (Anthropic AI) Signed-off-by: Alan Cha --- .../cmd_service_systemd_integration_test.go | 52 +++++++++++++++---- ...26-09-16-linux-systemd-lifecycle-design.md | 32 ++++++++---- 2 files changed, 64 insertions(+), 20 deletions(-) diff --git a/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go b/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go index 5a9ead1bf..b59731934 100644 --- a/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go +++ b/authbridge/cmd/abctl/cmd_service_systemd_integration_test.go @@ -8,6 +8,7 @@ import ( "runtime" "strconv" "strings" + "syscall" "testing" "time" ) @@ -86,13 +87,26 @@ func slowScript(t *testing.T) string { return path } +const restartOnFailureProp = "Restart=on-failure" + // runTransientUnit starts script under a throwaway, uniquely-named unit with the // same Restart=on-failure our real renderUnitFor writes (RestartSec=1 here, not the // production 10, purely so the test doesn't wait 10s per restart it triggers), and // registers its own teardown — stop and reset-failed, so a failed assertion never // leaves a unit respawning after the test process exits. +// +// Restart=on-failure is hand-copied into the systemd-run args below rather than +// read from renderUnitFor, so nothing would otherwise tie this test to the unit it +// claims to vouch for: delete the property from renderUnitFor's linux branch and +// this test would go on passing, proving a fact about systemd that the shipped +// unit no longer requests. This assertion is the missing link — it fails if the +// real renderer and this test's hand-copied property ever diverge. func runTransientUnit(t *testing.T, name, script string) { t.Helper() + if u := renderUnitFor("linux", servicePaths{}); !strings.Contains(u, restartOnFailureProp) { + t.Fatalf("the linux unit no longer sets %s; this test would be vouching for a "+ + "property the real unit doesn't request:\n%s", restartOnFailureProp, u) + } stop := func() { // systemd-run transient units are garbage-collected once inactive, so by the // time this runs the unit is typically already gone: `stop` on a unit that @@ -102,11 +116,18 @@ func runTransientUnit(t *testing.T, name, script string) { // print on every passing run. Logging them unconditionally defeated the point // of logging at all: a real leak would read identically to normal. Only // anything else is worth surfacing. + // + // stop's exit 5 is a narrow, confirmed-benign case, checked by code. reset-failed's + // exit 1 is systemd's generic failure code, not a specific one — checking it by + // code would suppress nearly everything this call can produce, including a user + // bus that goes away mid-run. Matched by message instead, so only the confirmed + // "unit doesn't exist" case is swallowed. if err := exec.Command("systemctl", "--user", "stop", name).Run(); err != nil && !isExitCode(err, 5) { t.Logf("cleanup: systemctl --user stop %s: %v", name, err) } - if err := exec.Command("systemctl", "--user", "reset-failed", name).Run(); err != nil && !isExitCode(err, 1) { - t.Logf("cleanup: systemctl --user reset-failed %s: %v", name, err) + if out, err := exec.Command("systemctl", "--user", "reset-failed", name).CombinedOutput(); err != nil && + !strings.Contains(string(out), "not loaded") && !strings.Contains(string(out), "not found") { + t.Logf("cleanup: systemctl --user reset-failed %s: %v: %s", name, err, strings.TrimSpace(string(out))) } } t.Cleanup(stop) @@ -119,7 +140,7 @@ func runTransientUnit(t *testing.T, name, script string) { args := []string{ "--user", "--unit=" + name, - "-p", "Restart=on-failure", + "-p", restartOnFailureProp, "-p", "RestartSec=1", script, } @@ -206,10 +227,16 @@ func TestSupervisorRestartsAfterCrash_RealSystemd(t *testing.T) { t.Fatal("unit never reported a main PID") } - // -9, not a plain stop: this must bypass the script's own TERM trap entirely, - // so it looks like a real crash (a segfault, an OOM kill) rather than a - // deliberate, distinguishable stop — which the next test proves does NOT restart. - if err := exec.Command("kill", "-9", strconv.Itoa(pid)).Run(); err != nil { + // SIGKILL, not a plain stop: this must bypass the script's own TERM trap + // entirely, so it looks like a real crash (a segfault, an OOM kill) rather + // than a deliberate, distinguishable stop — which the next test proves does + // NOT restart. syscall.Kill instead of exec.Command("kill", ...): requireRealSystemd + // already gated this on GOOS=linux, so the syscall package is always usable here, + // and it removes an external-binary dependency requireRealSystemd doesn't guard — + // on a slim image missing /bin/kill, that would surface as a failed test with + // ABCTL_SYSTEMD_TESTS=required set, rather than the environment-problem skip it + // actually is. + if err := syscall.Kill(pid, syscall.SIGKILL); err != nil { t.Fatalf("kill -9 %d: %v", pid, err) } @@ -244,10 +271,15 @@ func TestSupervisorStaysStoppedAfterDeliberateStop_RealSystemd(t *testing.T) { t.Fatalf("systemctl --user stop: %v: %s", err, strings.TrimSpace(string(out))) } - // The script's own TERM trap takes ~2s; give it real room, then assert it STAYS - // inactive rather than just checking once immediately after stop returns. + // `systemctl --user stop` already blocked until the stop job completed, so the + // script's ~2s TERM-trap drain is behind us by the time we get here — this is + // the first second of the negative assertion, not a grace period. It's tightened + // to 1s (not the full 4s below) because RestartSec=1 means a wrongly-firing + // restart would already be visible this early: is-active reads "deactivating" + // mid-drain and "activating"/"active" only once a new process exists, so this + // can't mistake the trap's own tail for a restart. if waitUntil(1*time.Second, func() bool { return unitIsActive(unit) }) { - t.Fatal("unit is active immediately after a deliberate stop") + t.Fatal("unit is active within 1s of a deliberate stop — looks like a wrongly-firing restart, not the stop itself") } // A flat sleep, not a poll, because this asserts a negative: there is no "it // happened" event to wait for. The risk this accepts is one-directional — a diff --git a/authbridge/docs/superpowers/specs/2026-09-16-linux-systemd-lifecycle-design.md b/authbridge/docs/superpowers/specs/2026-09-16-linux-systemd-lifecycle-design.md index bfe146953..863b60974 100644 --- a/authbridge/docs/superpowers/specs/2026-09-16-linux-systemd-lifecycle-design.md +++ b/authbridge/docs/superpowers/specs/2026-09-16-linux-systemd-lifecycle-design.md @@ -63,9 +63,11 @@ the macOS supervisor. That was the single biggest gap this issue needed to close - `authbridge/cmd/authbridge-proxy/supervise.go` — the macOS-only (and unsupervised-fallback) Go restart loop. - `authbridge/cmd/authbridge-proxy/main.go` — the proxy itself; on SIGTERM/SIGINT - does a graceful shutdown with a **15-second drain** - (`context.WithTimeout(..., 15*time.Second)`, `main.go:957`) — every "does stop - tolerate the drain" question traces back to this constant. + does a graceful shutdown with a **15-second drain** (`main()`'s `shutdownCtx := + context.WithTimeout(..., 15*time.Second)`, right after the signal wait — cited + by site rather than line number, since `main.go` isn't part of this PR's diff + and keeps moving independently) — every "does stop tolerate the drain" question + traces back to this constant. - `authbridge/docs/laptop-service.md` — user-facing doc for `abctl service *`, `~/.cortex/` layout, restricted-environment fallback, manual-removal instructions. @@ -119,9 +121,15 @@ real-launchd integration test. ### 7. `stop` tolerates the proxy's ~15s drain The proxy's own 15s shutdown timeout is the anchor value everything else has to respect. macOS handles this explicitly in Go (a 30s bootout timeout, plus the -supervisor's own 20s-before-SIGKILL logic). The rendered Linux unit set no -`TimeoutStopSec` at all — it "worked" only by accident of systemd's own -90s default (`systemd.system.conf(5)`) exceeding 15s. +supervisor's own 20s-before-SIGKILL logic). As of this audit (2026-09-16), the +rendered Linux unit set no `TimeoutStopSec` at all — it "worked" only by accident +of systemd's own 90s default (`systemd.system.conf(5)`) exceeding 15s. + +**Closed by #1079** — `renderUnitFor("linux", ...)` now sets `TimeoutStopSec=20` +explicitly on `main`, matching the macOS supervisor's 20s headroom over the same +15s drain, with a subtest in `cmd_service_test.go` asserting the line is present. +Not yet on this branch's own tree until it merges — cited by symbol, not line, +since #1079 landed in a sibling PR. ### 8. Works under user systemd, and states what happens where systemd is absent The best-handled bullet: `loadService` gives a clear, actionable message when @@ -155,7 +163,11 @@ Linux-aware preflight — still an open decision, not yet resolved either way. raised above — worth coordinating rather than duplicating. - **#966** — plugin build-tag convention cleanup (retired `exclude_plugin_*` form → `include_plugin_*`; allow-legacy-plugin-tag: this reference is design history, not - a live usage) and a smaller desktop artifact. Touches the same - install/upgrade path: upgrading to a build that dropped a plugin an existing - config still names must be handled gracefully — flagged there as something - #944/#945/#964 must cover. + a live usage — the guard matches on the whole file via `strings.Contains`, not + scoped to this one mention, so a real `exclude_plugin_*` usage added anywhere + else in this file would also pass silently) and a smaller desktop artifact. + Touches the same install/upgrade path: upgrading to a build that dropped a + plugin an existing config still names must be handled gracefully — #966 itself + closed 2026-09-16 having noted this as something #944/#945/#964 would need to + cover; tracking it directly against one of those now that #966 is no longer + open would keep the concern from being lost. From 574b449ac015e5dea3c1af6161f98756778b701e Mon Sep 17 00:00:00 2001 From: Alan Cha Date: Tue, 22 Sep 2026 17:06:22 -0400 Subject: [PATCH 17/17] docs: Fix a plan-doc claim that drifted after the reset-failed fix The plan doc's cleanup-fix bullet still described both stop and reset-failed as filtered by exit code. reset-failed was switched to message-matching in a later commit on this same branch, so the doc was describing an implementation the code no longer has. Also notes that the message-matching version hasn't had its own real-CI confirmation yet (the "confirmed against real CI" bullet predates that switch), and fixes the spec doc's main.go citation to include shutdownCancel, which context.WithTimeout also returns. Assisted-By: Claude (Anthropic AI) Signed-off-by: Alan Cha --- .../2026-09-21-systemd-real-integration-test.md | 14 ++++++++++---- .../2026-09-16-linux-systemd-lifecycle-design.md | 10 +++++----- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/authbridge/docs/superpowers/plans/2026-09-21-systemd-real-integration-test.md b/authbridge/docs/superpowers/plans/2026-09-21-systemd-real-integration-test.md index 9cbf86fac..682d2924d 100644 --- a/authbridge/docs/superpowers/plans/2026-09-21-systemd-real-integration-test.md +++ b/authbridge/docs/superpowers/plans/2026-09-21-systemd-real-integration-test.md @@ -37,11 +37,17 @@ the bare `Restart=on-failure` claim itself. Wired into the `abctl` leg of CI run, both tests failing identically. - [x] Fix: cleanup's `t.Logf` on `stop`/`reset-failed` failure printed on every normal passing run — `systemd-run` transient units are garbage-collected once - inactive, so both calls routinely exit nonzero (5, 1) by teardown time. - Filtered those two specific, confirmed-benign exit codes. + inactive, so both calls routinely exit nonzero by teardown time. `stop`'s + exit 5 is a narrow, confirmed-benign code, filtered by exit code. + `reset-failed`'s exit 1 is systemd's *generic* failure code, so filtering it + by code would suppress nearly everything that call can produce — filtered by + output text (`"not loaded"`/`"not found"`) instead, catching only the + confirmed "unit doesn't exist" case. - [x] Confirmed against real CI (not just local skip behavior): both tests pass with - realistic timing (~3s crash-restart, ~7s stay-stopped), and the cleanup fix - verified to produce zero spurious log lines on a clean run. + realistic timing (~3s crash-restart, ~7s stay-stopped), and the exit-code + version of the cleanup fix verified to produce zero spurious log lines on a + clean run. The later switch to message-matching for `reset-failed` has not + yet had its own real-CI confirmation. ## Result diff --git a/authbridge/docs/superpowers/specs/2026-09-16-linux-systemd-lifecycle-design.md b/authbridge/docs/superpowers/specs/2026-09-16-linux-systemd-lifecycle-design.md index 863b60974..e57864e10 100644 --- a/authbridge/docs/superpowers/specs/2026-09-16-linux-systemd-lifecycle-design.md +++ b/authbridge/docs/superpowers/specs/2026-09-16-linux-systemd-lifecycle-design.md @@ -63,11 +63,11 @@ the macOS supervisor. That was the single biggest gap this issue needed to close - `authbridge/cmd/authbridge-proxy/supervise.go` — the macOS-only (and unsupervised-fallback) Go restart loop. - `authbridge/cmd/authbridge-proxy/main.go` — the proxy itself; on SIGTERM/SIGINT - does a graceful shutdown with a **15-second drain** (`main()`'s `shutdownCtx := - context.WithTimeout(..., 15*time.Second)`, right after the signal wait — cited - by site rather than line number, since `main.go` isn't part of this PR's diff - and keeps moving independently) — every "does stop tolerate the drain" question - traces back to this constant. + does a graceful shutdown with a **15-second drain** (`main()`'s + `shutdownCtx, shutdownCancel := context.WithTimeout(..., 15*time.Second)`, right + after the signal wait — cited by site rather than line number, since `main.go` + isn't part of this PR's diff and keeps moving independently) — every "does stop + tolerate the drain" question traces back to this constant. - `authbridge/docs/laptop-service.md` — user-facing doc for `abctl service *`, `~/.cortex/` layout, restricted-environment fallback, manual-removal instructions.