From 94cdc12f025b3565cac27470f9d1f4a77484b4fd Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:10:48 -0700 Subject: [PATCH] fix(systemd): KillMode=process preserves retained Codex sidecars across unit restarts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The server's graceful shutdown deliberately retains adopted terminal-pane Codex sidecars (SAFE-11), but systemd's default KillMode=control-group (and the live host's explicit one) kills them after the main process exits — every systemctl restart destroyed in-flight Codex conversations. - Ship KillMode=process in the unit (accepted trade-off, matching the setsid launcher's crash semantics: a crashed server's children are not reaped by systemd). - Point the SAFE-11 comment in main.rs at the shipped setting instead of a hypothetical adoption note. - Update the runbook's unit-coupling note: retention survives unit restarts when the installed copy carries the line; verify + re-copy + daemon-reload guidance included. Config/comment/doc only; no executable behavior changed in-repo. Host install staged separately (daemon-reload done; effective at the next approved restart). --- crates/freshell-server/src/main.rs | 13 ++++++++----- docs/development/self-hosted-launch-runbook.md | 14 +++++++++----- installers/systemd/freshell-rust.service | 10 ++++++++++ 3 files changed, 27 insertions(+), 10 deletions(-) diff --git a/crates/freshell-server/src/main.rs b/crates/freshell-server/src/main.rs index 694da8c56c..8c11aa1d4a 100644 --- a/crates/freshell-server/src/main.rs +++ b/crates/freshell-server/src/main.rs @@ -3440,11 +3440,14 @@ async fn main() -> ExitCode { // `prepare_retention("server-shutdown")` and their records flip to // Retained — instead of handing them to the teardown worker. // - // Supervisor caveat (recorded, no code — reports/V6.md NA-1): the in-repo - // systemd unit is NOT installed today (restarts are script-driven). If it - // is ever adopted, its KillMode must not be `control-group` — a cgroup - // kill would slaughter the retained sidecars this deliberately keeps - // alive (`process_group(0)` detaches the pgid, not the cgroup). + // Supervisor caveat (SAFE-11, reports/V6.md NA-1): the shipped systemd + // unit (installers/systemd/freshell-rust.service) sets + // `KillMode=process` for exactly this reason — a cgroup kill + // (`control-group`, the default; `mixed` likewise) would slaughter the + // retained sidecars this deliberately keeps alive (`process_group(0)` + // detaches the pgid, not the cgroup). Keep that setting: hosts whose + // installed copy predates it must re-copy the unit and daemon-reload, + // or unit restarts still defeat retention. freshell_codex::launch_lifecycle::CodexTerminalLaunchManager::global() .begin_shutdown_retention(); registry.kill_all(); diff --git a/docs/development/self-hosted-launch-runbook.md b/docs/development/self-hosted-launch-runbook.md index be3a62de25..7d7d7540ce 100644 --- a/docs/development/self-hosted-launch-runbook.md +++ b/docs/development/self-hosted-launch-runbook.md @@ -373,11 +373,15 @@ drop-in, plus `freshell-supervisor.service`): `freshell-supervisor.service`. Restarting the SUPERVISOR therefore also restarts the server in the same transaction — one `systemctl --user restart freshell-supervisor.service` redeploys both. - BEWARE: any server-unit restart terminates the server's RETAINED Codex - sidecars (see the restart-effects list below — the unit has no - `KillMode` override, so systemd's control-group kill defeats the - server's deliberate SAFE-11 retention); in-flight Codex conversations - are lost. + The shipped unit sets `KillMode=process` (SAFE-11: the server's graceful + shutdown deliberately RETAINS adopted terminal-pane Codex sidecars, and + a control-group kill would slaughter them), so unit restarts PRESERVE + in-flight Codex conversations — but only if the INSTALLED copy carries + that line. Verify `systemctl --user show freshell-rust -p KillMode` + reports `process`; an installed copy without it (or explicitly + `control-group`) must be re-copied from `installers/systemd/` and + `systemctl --user daemon-reload`d, or unit restarts still terminate + retained Codex conversations. The same `Type=simple` socket race as the strict sequence applies here: the server may start before the supervisor rebinds its control socket and come up failed; the unit's `Restart=on-failure` retries can then diff --git a/installers/systemd/freshell-rust.service b/installers/systemd/freshell-rust.service index 5676f6e1ae..df575a8b3c 100644 --- a/installers/systemd/freshell-rust.service +++ b/installers/systemd/freshell-rust.service @@ -56,6 +56,16 @@ Restart=on-failure RestartSec=2 # Give the graceful-shutdown path (WS close 4009 + PTY teardown) time to run. TimeoutStopSec=15 +# SAFE-11 retention: on stop, signal ONLY the main process. The server's +# graceful shutdown reaps every PTY and sidecar it owns EXCEPT adopted +# terminal-pane Codex sidecars, which it deliberately RETAINS across +# restarts (crates/freshell-server/src/main.rs, "SAFE-11 deliberate +# deviation"). systemd's default KillMode=control-group (and `mixed` +# likewise) would slaughter exactly those retained sidecars after the +# main process exits. Accepted trade-off (matches the setsid launcher's +# crash semantics): if the server CRASHES, systemd does not reap its +# children either. +KillMode=process [Install] WantedBy=default.target