Add units RFC: supervised sessions with lifecycle policy

[pcarrier]

Design only — no code. Adds docs/design/units.md.

What this proposes

A unit is a named, declarative supervisor for a blit session: an on-disk file saying what to run, when to start it, when to restart it, when it is ready, when it is healthy, and what it depends on. Systemd's model, down to the key names.

The framing that keeps it blit-shaped: a unit introduces no new object. C2S_RESTART already respawns an exited child in place, reusing the same pty id and driver (lib.rs:8683, pty_unix.rs:570), so blit already has a session identity that outlives a process. A unit is that identity made declarative — policy on an existing Pty entry, keyed by its existing tag. Clients subscribe once and follow a unit across restarts, with continuous scrollback as the blit-native journal. The alternative, a second process model bolted alongside the PTY map, is what the design exists to avoid.

Three primitives first

Units sit on three changes that fix real bugs and are worth having alone:

• Deadlines — every timeout is client-side today, so a hung command outlives a disconnected orchestrator forever. Worse than it looks: blanket_frame_interval returns None with no clients (lib.rs:2731), so the tick loop's next_deadline goes None and a silent runaway is never visited on any schedule. The 5 s reap_zombies task becomes a 1 s state-carrying supervisor tick.
• Group kill — C2S_KILL signals the leader pid only and C2S_CLOSE's SIGHUP misses anything that changed process group, even though the code already pgrp-signals for SIGWINCH (pty_unix.rs:224-230). KillMode=process-group becomes the default.
• GC — nothing frees an exited PTY slot (the only ptys.remove in the server is in the C2S_CLOSE arm, lib.rs:8883) and max_ptys is hardcoded unlimited (crates/cli/src/main.rs:802). Adjacent standalone bug: hitting the cap is a bare continue with no reply, and there is no error opcode in the protocol at all, so a nonce-bearing create hangs forever.

Decisions baked in

INI with sections and systemd's exact key names (four documented deviations); readiness and health both in v1; Backing=pipe still feeds the alacritty driver so scrollback/search/COPY_RANGE/rendering keep working; the full deadline surface including connection leases that resolve into the deadline primitive rather than adding a second kill path.

Health gates activation — departing from systemd, a unit with ExecHealthCheck= reaches active only after its first probe passes. That single rule makes After= mean "healthy" with no second ordering keyword, and with Type=oneshot/surface it makes this expressible:

# open-url.unit
[Unit]
Requires=api chromium
After=api chromium
[Service]
Type=oneshot
RemainAfterExit=yes
ExecStart=<browser-driver> open http://localhost:8080

Three different definitions of "up" — a sd_notify handshake plus a passing health probe, a Wayland surface, a command that exited zero — and the dependent expresses its requirement without knowing which.

Delivery

One axis per PR: (1) group kill, (2) supervisor tick + deadlines + leases, (3) GC + max_ptys + S2C_CREATE_FAILED, (4) units. The first three are the incident-shaped fixes and stand alone.

Every code claim in the doc is cited to path:line and verified against dc6a265. Open questions are called out inline, notably ReadySurface= matching by app_id rather than by process — surfaces are keyed by Wayland object id and the compositor records no client credentials today.

Tag @indent to continue the conversation here.

[github-actions]

Coverage

Crate	Lines	Functions	Regions
alacritty-driver	69.7% (698/1002)	72.0% (54/75)	71.7% (1050/1464)
browser	0.0% (0/807)	0.0% (0/65)	0.0% (0/1370)
cli	18.2% (1520/8342)	28.5% (200/701)	21.0% (2729/12984)
compositor	1.0% (93/9248)	2.0% (8/400)	1.2% (146/12403)
fonts	81.4% (721/886)	88.6% (70/79)	83.0% (1427/1719)
fssync	90.1% (3494/3878)	90.9% (288/317)	90.2% (6216/6894)
gateway	26.1% (375/1437)	29.9% (38/127)	19.9% (470/2362)
git	85.3% (2889/3385)	88.8% (231/260)	85.3% (4586/5379)
lsp	76.0% (2503/3295)	78.2% (248/317)	73.8% (3886/5266)
proxy	19.3% (172/892)	20.5% (26/127)	21.2% (293/1381)
remote	89.1% (7723/8665)	92.1% (574/623)	87.3% (12907/14791)
sd-notify	69.6% (64/92)	100.0% (6/6)	80.9% (106/131)
server	35.8% (5964/16646)	49.8% (585/1174)	38.6% (10279/26604)
ssh	1.9% (7/374)	3.2% (1/31)	0.7% (4/613)
upsidedown	31.4% (391/1247)	27.8% (55/198)	34.8% (797/2287)
webrtc-forwarder	2.7% (72/2624)	2.1% (4/187)	1.2% (50/4335)
webserver	59.1% (995/1684)	64.3% (153/238)	62.4% (1737/2782)
Total	42.9% (27681/64504)	51.6% (2541/4925)	45.4% (46683/102765)

[indent]

PR Summary

Design-only PR — adds docs/design/units.md, an RFC proposing systemd-style units: a declarative supervisor layer over blit's existing PTY sessions, built on three standalone primitives (group kill, a fully-reactive supervisor loop with deadlines/leases, and GC + a real max_ptys + an error opcode) each pitched as a bug fix worth having on its own. No code changes.

• Defines a small UnitRuntime registry (the PTY stays the only stream object), per-restart generations to make restart-on-leader-death race-safe, unit identity via a server-owned non-wire unit field, a strict execve-direct INI grammar (no /bin/sh -c, no specifiers, duplicate scalar keys are errors), the state machine, readiness/health probes with failure/success thresholds, dependency semantics with faithful Requires= stop-propagation, Backing=pipe, a 0x90 wire family (with a generation field), and the CLI.
• Reworks the supervisor into a reactive tokio::select! loop; SIGCHLD only wakes the supervisor to reap owned pids via targeted waitpid, and the global waitpid(-1) drain is deleted.
• Settles the security model: installing units through the fs family is intended, the trust boundary is the socket (with BLIT_UNITS=0 as a global kill switch), and permission bits are explicitly not relied on since an fs writer runs as the server's own user; adds load-time hygiene and audit logging. Operator intent (disabled / manually-stopped) persists under a reserved blit/units/ KV prefix.
• Adds live-reload atomic-swap semantics, a support matrix, Deferred/Testability sections, and splits delivery into a PTY-only core (PR 4) and a policy layer (PR 5).
• The doc's central quality claim is that every path:line citation is verified against base dc6a265; I audited them across all seven commits and they all check out.

Issues

All clear! No issues remaining. 🎉

2 issues already resolved

• The "flesh out the worked example" commit (174388e) also deleted the Pipe backing, Wire and CLI (the 0x90 opcode table, FEATURE_UNITS bit, CLI subcommands, learn.md), and Server restart sections — but Delivery item 4 and Constraints still reference that removed content, and the reworked example now says "setpgid(0,0) noted above" with no such note left, indicating the deletion was unintentional. (fixed by commit 23f3032)
• Citation drift: the doc cites the refuse_lsp_message refusal pattern at lib.rs:7411-7422, but those lines are the git open-rebase path; the actual refusal dispatch is lib.rs:7431-7439 and the function is defined at lib.rs:6928. (fixed by commit 23f3032)

CI Checks

All CI checks passed on 27b2d76.

[pcarrier]

🐮 Moo review — @indent, everything below is up for pushback. These are review hypotheses and suggested tradeoffs, not decrees; please challenge any premise, severity, or alternative that does not fit the intended shape of blit.

I reviewed PR head 4ee4989 against base dc6a265. The motivation is strong, and the three standalone primitives—group kill, server-side deadlines, and bounded exited-session retention—look independently valuable. Before implementing the unit layer, I think the RFC should resolve the lifecycle and trust-boundary questions below.

Highest-priority design issues

1. A unit cannot be only policy attached to an existing Pty

The central claim is that a unit “introduces no new object” and is keyed by the PTY’s existing tag. That seems incompatible with the rest of the design:

• An inactive, failed-to-load, dependency-blocked, or autostarting unit exists before any PTY does.
• A unit with RemainAfterExit=yes exists after its process exits.
• Reloading a definition requires retaining configuration and runtime state independently of a process generation.
• Existing tags are arbitrary, optional, and not unique. Creation accepts empty or duplicate tags without checking existing PTYs (crates/server/src/lib.rs:7777-7817).

I would introduce a small explicit registry:

UnitRuntime {
    name: UnitName,
    definition: UnitDefinition,
    state: UnitState,
    current_pty: Option<PtyId>,
    generation: u64,
}

The PTY can remain the sole stream/terminal object. A registry pointing to the current PTY is orchestration state, not a competing process model. Unit identity should be a validated unique name or reserved tag namespace, rather than an unconstrained client tag. The RFC should also define what happens if an ordinary client creates a PTY whose tag matches a loaded unit.

2. SIGCHLD must not blindly reuse the current global reaper

The RFC replaces polling with a reactive SIGCHLD handler, but does not specify the reaping algorithm. Today reap_zombies() calls waitpid(-1, …) and discards statuses for children not registered as PTYs (pty_unix.rs:246-284). Running that immediately on every SIGCHLD would make it much more likely to steal exit statuses from health-check helpers, audio children, LSP processes, and future Command-owned children. That directly conflicts with periodic ExecHealthCheck processes.

The signal handler could wake the supervisor, which then calls waitpid(known_pty_pid, WNOHANG) for each registered PTY child. A global waitpid(-1) drain seems safe only if child ownership is centralized for the whole server.

3. Leader death before PTY EOF creates stale-generation races

The RFC correctly identifies the case where the session leader dies while a grandchild keeps the slave open. Restarting immediately on SIGCHLD creates another problem:

• the old PTY reader can remain alive;
• old descendants can keep producing output;
• respawn_child opens a new master and starts another reader;
• eventual EOF/output from the old reader can arrive against the same PTY ID and mutate the new generation.

Today in-place respawn is safe largely because it occurs after the old PTY reaches the EOF/exited path. The proposal removes that precondition without defining replacement cleanup.

Every asynchronous event should probably carry (pty_id, generation). Before restart, the RFC should define a sequence such as:

1. Record leader exit status.
2. Terminate the remaining old process group.
3. Close the old master.
4. Retire/join the old reader.
5. Increment generation.
6. Spawn the replacement.
7. Ignore late events from previous generations.

4. Deadlines and leases do not yet compose

The RFC says all bounds resolve to one enforcement path, while lease disconnect arms a deadline and reconnect clears it. With one apparent deadline field, several races are undefined:

• An explicit C2S_DEADLINE can be overwritten by a lease-disconnect deadline.
• Reclaim could clear an unrelated explicit or RuntimeMaxSec deadline.
• An old connection can disconnect after a replacement connection reclaims the same lease, rearming its deadline.
• Two simultaneous connections presenting one lease ID have no ownership semantics.
• S2C_EXITED attribution is ambiguous when multiple constraints expire together.

I would model independent causes and enforce their minimum:

effective_deadline =
  min(explicit_deadline,
      lease_deadline_for_current_epoch,
      runtime_max_deadline,
      stop_escalation_deadline)

Reclaim should remove only the matching lease constraint. A connection epoch or holder count is needed so stale disconnects cannot revoke newer claims. A server-minted unguessable token may be safer than a client-selected bare u64; either way, issuance, collision, and concurrent-holder semantics should be explicit.

5. The persistence security control does not address the stated attacker

The RFC correctly says units add persistence and can be written through the filesystem protocol, then proposes rejecting group/world-writable files and directories. But a remote filesystem client writing through the server will normally create a server-user-owned, non-world-writable file in the user directory, so it passes those checks. Mode checks protect against another Unix account; they do not distinguish a trusted local operator from the remote client in the threat model.

Possible policies:

1. Exclude unit directories from the remote filesystem API.
2. Require unit installation through a local-only administrative CLI/control socket.
3. Autostart only system-owned /etc/blit/units by default; gate user autostart behind an explicit server option.
4. Treat remote unit installation as a separate capability from C2S_CREATE.
5. At minimum, document that filesystem capability grants durable code-execution persistence and that mode checks do not mitigate that case.

Symlinks, ownership, atomic open/validation, and writable parent-directory traversal also need definition.

6. “Use man systemd.service as the reference” is too ambiguous for a small custom parser

The proposed roughly 60-line parser plus systemd-compatible names leaves material questions:

• Is ExecStart shell-interpreted? systemd’s generally is not /bin/sh -c.
• Are quoting, escaping, continuations, specifiers, and environment expansion supported?
• Do repeated keys replace or append?
• Are dependency lists whitespace-separated?
• Are duration suffixes (30s, 500ms, infinity) accepted?
• Are unknown sections/keys fatal, warnings, or ignored?
• What are duplicate-section, malformed-line, and inline-comment semantics?

Because these files persistently execute code, permissive or surprising parsing is risky. I would either define a deliberately small grammar and say only the vocabulary transfers, use a mature parser for actual systemd semantics, or use a typed format such as TOML and consider an import tool later. “No parser crate currently exists in the workspace” does not seem like a strong reason to own a compatibility-sensitive parser.

Other clarifications worth making

Reject unsupported readiness modes instead of silently weakening them

Type=notify falling back to simple on Windows breaks the core promise that After= waits for real readiness. Prefer a load error, or require an explicit fallback. The RFC also needs a platform matrix covering process-tree kill, cgroups/Job Objects, notify sockets, and reactive child exit.

Make the state machine an event/transition table

The diagram omits restart backoff, reload, stop escalation, readiness failure, and generations. It also says Restart= applies on active → inactive|failed, but separately applies it to activation timeout. Please define transitions for spawn failure, readiness timeout, health failure while activating/active, watchdog expiry, explicit stop, config removal, stale-generation EOF, and start-limit exhaustion.

Clarify that health-based dependencies are only an initial gate

The headline can read as a maintained invariant—“that unit is answering its health check”—but v1 waits only for the first passing probe and explicitly excludes propagation. That is a reasonable policy, but I would state it directly:

After= gates initial activation on the dependency’s first healthy state; it does not maintain a continuous health invariant.

Consecutive success/failure thresholds may also be useful; restarting on one transient failed curl seems aggressive.

Document live-reload semantics

Define whether changes to command/environment/working directory, dependencies, health intervals, restart limits, autostart, removal, and parse failures apply immediately, on the next restart, or after an explicit restart. A safe pattern is to parse and validate a complete new registry, then atomically swap definitions; an invalid edit should not partially mutate live policy.

Existing inline findings

I agree with the existing review notes that:

• the worked-example commit appears to have removed the Pipe backing, Wire and CLI, and Server restart sections while Delivery still promises them; and
• the refuse_lsp_message citation has drifted.

The missing server-restart and wire sections are especially relevant because persistence and capability negotiation affect several issues above.

Possible narrower alternatives

A. Land only the incident-shaped primitives first

Proceed with group/process-tree kill, per-session monotonic deadlines, nonce-correlated create failures, and bounded exited-session retention. Defer leases and units until timer provenance, reconnect ownership, and process-generation handling have been exercised.

B. Start with a smaller unit MVP

Use an explicit UnitRegistry, initially supporting only:

• Type=simple | oneshot | notify;
• ExecStart;
• After and Requires;
• restart/backoff;
• start/stop timeouts;
• PTY backing only.

Defer surface matching, regex matching, health checks, pipe backing, leases, live reload, and watchdogs. This keeps the core value while reducing the first implementation’s race surface.

C. Separate stable unit identity from terminal/process generations

Unit → current ProcessGeneration → PTY stream

The existing terminal driver can remain attached across generations where safe, but lifecycle events carry a generation number. This preserves continuous scrollback without requiring unit identity, PTY identity, and process identity to be identical.

Again, @indent: all of this is open for pushback. The RFC has a compelling core; my main ask is to make the ownership, generation, composition, parsing, and persistence contracts explicit before implementation locks them in.

[indent]

Strong review. Six of these are things I got wrong rather than tradeoffs I chose, and I've taken them. Addressed in 8375171 (you reviewed 4ee4989; 23f3032 had already fixed the two inline findings).

Where I push back is narrower than you offered, and it's mostly about the two decisions that were made explicitly upstream of the RFC.

Taken

1. A unit is not policy on a Pty. You're right and my patch two commits ago was only half a fix — I'd moved identity off tag onto a unit: Option<String> field, which still can't represent a unit that is inactive, dependency-blocked, failed-to-load, or RemainAfterExit=yes past its process. Adopted UnitRuntime with current: Option<(PtyId, Generation)>. What survives is the narrower claim you granted: the PTY stays the only stream object and the registry points at it. A client PTY whose tag collides with a unit name is now explicitly left alone — not adopted, not refused, never in S2C_UNIT_LIST.

2. SIGCHLD must not reuse the global reaper. Confirmed against the source and this is a bug I was introducing: reap_zombies drains waitpid(-1, WNOHANG) and discards statuses for pids outside pty_pids() (pty_unix.rs:266-284). The audio pipeline already lives with that race at 5s; running it per-SIGCHLD widens it sharply, and I was adding periodic ExecHealthCheck= children straight into its path. Now: the handler only wakes the supervisor, which does targeted waitpid(pid, WNOHANG) over pids it owns (PTY + helper children, via the existing register_pty_pid/pty_pids() registry generalized). The global drain is deleted rather than rescheduled — Command-owned children keep being reaped by their own owners, and no status is collected by a party that didn't spawn it. Strictly safer than today.

3. Generations. Also right, and the sharpest finding. In-place respawn is safe today only because it happens after the EOF/exited path; restarting on leader death removes that precondition and I hadn't replaced it. Added Generation on every unit-owned PTY, (pty_id, generation) on every async event, and the ordered sequence you sketched — record status, kill old group, close master, retire and join the reader, bump generation, spawn, drop stale-generation events. That last step is the cheap backstop that makes the rest safe to get slightly wrong.

4. Deadlines and leases don't compose. "All resolve to one enforcement path" was hand-waving. Now independent causes with effective = min(explicit, lease[current_epoch], runtime_max, stop_escalation), each armed and cleared only by itself. Leases get server-minted ids (a client-chosen u64 is a namespace anyone on the socket can guess, for something that is a kill switch on other people's sessions), an epoch bumped on reclaim so a stale disconnect can't revoke a newer claim, and single-holder semantics. S2C_EXITED attributes to the cause that produced the minimum, ties broken in a fixed order, so it's deterministic rather than whichever timer fired first.

5. The security control didn't address its own attacker. Completely right and this was the worst of them. A remote fs client writes as the server's user, 0644, in the user's own directory — it passes every mode check. Mode bits distinguish another Unix account, not a socket from a keyboard. Rewrote around the distinction that actually matters: C2S_CREATE is ephemeral execution scoped to a connection, a unit file is durable execution that outlives the connection, the client, and the process. Treating them as one capability is how a session compromise becomes permanent. So — your options 1, 2, 3 and 5 together: unit directories excluded from the fs family; install is a local capability (START/STOP/RESTART stay on the wire, since exercising already-installed policy isn't introducing code); /etc/blit/units autostarts by default with the user directory gated behind an explicit option; mode/symlink/parent-traversal checks demoted to depth; and the equivalence documented plainly.

Also taken: state machine is now a transition table covering spawn failure, readiness timeout, health failure in both states, watchdog, explicit stop, config removal, stale generation, and limit exhaustion. Health failures are thresholded (HealthCheckFailureThreshold, default 3) — you're right that restarting on one lost curl race is worse than the problem. After= is documented as an initial gate, in your words. Live reload defined as validate-whole-registry-then-atomic-swap, with a per-key table of what applies now vs. at next start. Unsupported platform capabilities are now load errors, not warnings — you're right that Type=notify silently degrading breaks the one promise After= makes, and I'd traded a loud failure for a silent one.

Pushback

6. INI vs. TOML — the format is decided, the ambiguity is real. INI with systemd's key names was an explicit product decision upstream of this RFC, so I'm keeping it. But your substantive point stands and I've fixed it: "use man systemd.service as the reference" is not implementable, and I've deleted that sentence. systemd's syntax carries specifiers, ExecStart= prefixes (-, @, +, !), continuations, its own quoting, drop-in merge order and EnvironmentFile= expansion; implementing a subset of that in 60 lines while claiming compatibility is exactly how a code-executing file format acquires surprises.

So: only the vocabulary transfers, and there's now an explicit grammar table — no continuations, no inline comments, no quoting, argv split on whitespace and execve'd directly (never /bin/sh -c), no specifiers, repeated scalar keys are an error rather than last-wins, list keys append, durations take ms/s/m/h, unknown section or key is an error. Strict beats permissive throughout, and it can be relaxed later where permissive can't be tightened. I'd also push back on "use a mature parser for actual systemd semantics": that buys a compatibility surface we've deliberately scoped out, and the failure mode isn't parse errors, it's users reasonably expecting %i and drop-ins to work.

Alternative B, partially. Deferring health checks and live reload conflicts with an explicit "readiness and health from day 1" decision, so I can't take it as written. But your underlying point — the first implementation should have a smaller race surface — is right, so I applied it as sequencing instead of scope: PR 4 is now unit core (registry, generations, strict parser, state machine, restart, Requires=/After=, Type=simple|oneshot|notify, PTY backing only) and PR 5 is unit policy (health + thresholds + ActiveWhenHealthy=, watchdog, Type=match, Type=surface, Backing=pipe, live reload, KV operator intent). Nothing is dropped, but the generation and ownership contracts get exercised by a PTY-only, probe-free core before a second spawn path and live reload land on top.

Alternative A is already the plan — PRs 1-3 are exactly those primitives and land first. Alternative C is what the registry plus generations now implements.

One correction to the review: the missing Pipe backing / Wire and CLI / Server restart sections were my splice error and were restored in 23f3032, before this round. (I then managed to repeat the identical mistake while editing this revision and caught it on re-read — the failure mode is real enough that I'm now diffing section headers before every push.)