From 32ce09cd12393c9b396d60700d6dd91837e158fd Mon Sep 17 00:00:00 2001 From: Relayflow Lead Date: Tue, 8 Sep 2026 09:40:43 +0200 Subject: [PATCH] feat(daemon): connection-file handshake and CLI attach-or-spawn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit relayflowd currently requires a human to run `relayflowd serve` by hand before any `flows run`/`check`/`resume` can connect. Give it a connection-file advertisement plus a daemon-held flock singleton, and give the CLI attach-or-spawn logic, so `npm install -g relayflows && flows run x.yaml` needs no manual daemon step. - kernel: relayflowd publishes /connection.json only after the socket is live, cleans it up on SIGTERM/SIGINT, and holds an exclusive flock for its lifetime so a second `serve` on the same data dir loses the race and exits 3 without touching anything. - sdk: daemon-connection.ts validates the file against a live pid + socket hello probe (EPERM counts as alive); daemon-lifecycle.ts attaches if something answers, otherwise resolves and spawns relayflowd detached and polls for readiness. Wired into check/run/resume via cli.ts, with a --no-spawn/FLOWS_NO_SPAWN=1 escape hatch that restores today's fail-closed behavior for CI. - Orchestrated as workflows/daemon-lifecycle.yaml (design -> kernel impl -> kernel tests -> cli impl -> sdk tests -> e2e smoke -> adversarial review -> report). The first adversarial pass failed on missing kernel coverage for the anti-hijack and SIGKILL-successor cases (DAEMON-LIFECYCLE.md §6 tests 4-5); both are now written against the real binary and pass. Full writeup in ops/DAEMON-LIFECYCLE-REPORT.md. cargo test --workspace and npm test (packages/sdk) are green. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_013Y8uLRUXqKSZsqeeUMNaS2 --- docs/SURFACE.md | 36 +- kernel/DAEMON-LIFECYCLE.md | 552 ++++++++++++++++++ kernel/relayflowd/src/server.rs | 19 +- kernel/relayflowd/src/server/lifecycle.rs | 156 +++++ kernel/relayflowd/src/server/liveness.rs | 11 +- kernel/relayflowd/src/server/tests.rs | 35 +- kernel/relayflowd/tests/daemon_lifecycle.rs | 193 ++++++ ops/DAEMON-LIFECYCLE-REPORT.md | 195 +++++++ packages/sdk/src/cli.ts | 57 +- packages/sdk/src/cli/daemon-refusal.ts | 49 ++ packages/sdk/src/cli/direct-run.ts | 4 +- packages/sdk/src/cli/run.ts | 71 ++- packages/sdk/src/daemon-connection.ts | 336 +++++++++++ packages/sdk/src/daemon-lifecycle.ts | 198 +++++++ packages/sdk/src/failure-kinds.ts | 26 +- packages/sdk/src/journal-client.ts | 21 + packages/sdk/src/relayflowd-path.ts | 190 ++++++ .../sdk/tests/daemon-lifecycle-live.test.ts | 327 +++++++++++ packages/sdk/tests/daemon-lifecycle.test.ts | 533 +++++++++++++++++ packages/sdk/tests/direct-input.test.ts | 6 +- .../sdk/tests/fixtures/stub-relayflowd.mjs | 198 +++++++ packages/sdk/tests/live-kernel.test.ts | 50 +- packages/sdk/tests/relayflowd-path.test.ts | 141 +++++ workflows/daemon-lifecycle.yaml | 236 ++++++++ 24 files changed, 3585 insertions(+), 55 deletions(-) create mode 100644 kernel/DAEMON-LIFECYCLE.md create mode 100644 kernel/relayflowd/src/server/lifecycle.rs create mode 100644 kernel/relayflowd/tests/daemon_lifecycle.rs create mode 100644 ops/DAEMON-LIFECYCLE-REPORT.md create mode 100644 packages/sdk/src/cli/daemon-refusal.ts create mode 100644 packages/sdk/src/daemon-connection.ts create mode 100644 packages/sdk/src/daemon-lifecycle.ts create mode 100644 packages/sdk/src/relayflowd-path.ts create mode 100644 packages/sdk/tests/daemon-lifecycle-live.test.ts create mode 100644 packages/sdk/tests/daemon-lifecycle.test.ts create mode 100644 packages/sdk/tests/fixtures/stub-relayflowd.mjs create mode 100644 packages/sdk/tests/relayflowd-path.test.ts create mode 100644 workflows/daemon-lifecycle.yaml diff --git a/docs/SURFACE.md b/docs/SURFACE.md index 67aa682d7..ea42412ee 100644 --- a/docs/SURFACE.md +++ b/docs/SURFACE.md @@ -298,17 +298,43 @@ Gate 1 ships three CLI verbs over the journal protocol: ```text flows check [--json] -flows run [--json] [--data-dir ] -flows run [--json] [--data-dir ] --input -flows resume [--json] [--data-dir ] +flows run [--json] [--no-spawn] [--data-dir ] +flows run [--json] [--no-spawn] [--data-dir ] --input +flows resume [--json] [--no-spawn] [--data-dir ] ``` `check` compiles and preflights without starting a run. `run` performs that same preflight before contacting `relayflowd`, then submits the compiled spec to `/relayflowd.sock`; `resume` asks that daemon to continue an existing run from its journal. The data directory defaults to `.relayflowd`. -Neither verb starts the daemon implicitly. `--json` writes one report-shaped -object to stdout while diagnostics remain on stderr. +`--json` writes one report-shaped object to stdout while diagnostics remain on +stderr. + +`run` and `resume` attach to the daemon serving `` or start one +(kernel/DAEMON-LIFECYCLE.md). The attach is decided by the socket, not by a +file: `/connection.json` is read and validated, but nothing is +attached to until a `hello` is answered on the socket path recomputed from +`--data-dir`. When nothing answers, the CLI spawns `relayflowd serve +--data-dir ` detached — in its own session, with stdio never inheriting +the CLI's and its stderr appended to `/relayflowd.log` — and polls +for it, bounded. Concurrent invocations are safe: the daemon holds an +exclusive lock on the data dir, so a redundant one exits without touching +anything and its CLI attaches to the winner. + +`relayflowd serve` remains fully supported and unchanged for an operator who +starts it by hand; `run` and `resume` attach to it and never signal, restart, +or terminate a daemon. `--no-spawn` (or `FLOWS_NO_SPAWN=1`) refuses instead of +starting one — the lever for CI that means to assert a daemon is already +present. `check` never opens a socket and needs no daemon, no data directory, +and no `relayflowd` binary at all. + +A `run` or `resume` that cannot get a daemon is refused before any journal +write (exit 2) and names which step failed: `daemon_unreachable` under +`--no-spawn`, `relayflowd_not_found` when no binary could be located, +`daemon_start_failed` when the spawned daemon exited during startup, +`daemon_start_timeout` when it never began serving, and +`daemon_protocol_mismatch` against a live daemon speaking another protocol +version — which refuses rather than starting a second daemon over it. A direct `.flow.ts` run requires `--input`. When its argument names an existing regular file, the CLI parses that file as JSON; otherwise it parses the argument diff --git a/kernel/DAEMON-LIFECYCLE.md b/kernel/DAEMON-LIFECYCLE.md new file mode 100644 index 000000000..fc5dbaf28 --- /dev/null +++ b/kernel/DAEMON-LIFECYCLE.md @@ -0,0 +1,552 @@ +# Daemon lifecycle: the connection handshake and attach-or-spawn + +Status: implementation spec. Scope: the handshake that lets +`npm install -g relayflows && flows run x.yaml` work without a human first +running `relayflowd serve`. Nothing else. + +Today `flows run` / `flows resume` open `/relayflowd.sock` and fail +closed when nobody is listening (`packages/sdk/src/cli/run.ts:134-160`, +diagnostic `daemon_unreachable`), and `relayflowd serve` unconditionally +unlinks a leftover socket before binding +(`kernel/relayflowd/src/server.rs:50-55`). This spec adds a connection file, a +daemon-held mutex, and CLI-side attach-or-spawn. + +The shape is ported from `../relay` +(`packages/cli/src/cli/lib/broker-lifecycle.ts`, `broker-connection.ts`, +`packages/harness-driver/src/broker-path.ts`): a `connection.json` written by +the server, validated by the client, detached spawn, bounded readiness poll, +attach-or-spawn on every command. The TCP parts of that design — port +allocation, `url`, `api_key` — have no analogue here. A unix socket path is +fixed by `--data-dir` and carries OS-level access control, so there is nothing +to negotiate and no secret to hand over. + +**The governing rule: the socket is the authority; `connection.json` is an +index over it.** This is the same relationship `server.rs:150-200` already +states between the journal and the `runs` registry, and it decides every +ambiguous case below. A file is never believed on its own. + +--- + +## 1. The connection file + +### Path + +`/connection.json` — `data-dir` is the same directory that already +holds `relayflowd.sock` and `relayflowd.sqlite3`, default `.relayflowd`. + +### Shape + +```json +{ + "socket_path": "/Users/x/proj/.relayflowd/relayflowd.sock", + "pid": 48213, + "version": "0.1.0", + "protocol": 0, + "started_at_ms": 1757308800123 +} +``` + +| Field | Type | Meaning | +|---|---|---| +| `socket_path` | string | Absolute path of the bound socket. Absolute, not relative: the reader's cwd is not the daemon's. | +| `pid` | number | The serving process. Positive integer. | +| `version` | string | `env!("CARGO_PKG_VERSION")` of the `relayflowd` crate. | +| `protocol` | number | `relayflowd_core::PROTOCOL_VERSION` (currently `0`). | +| `started_at_ms` | number | Unix ms when `bind()` returned. | + +Unknown fields are ignored by readers; adding a field later is not a breaking +change. Missing or wrongly-typed required fields make the file invalid, which +is treated exactly like an absent file. + +`protocol` is not decoration: it lets the CLI tell "an incompatible daemon owns +this data dir" (refuse, do not spawn a second one) from "nothing is here" +(spawn). Without it the CLI would only learn about a mismatch from `hello`, +after deciding not to spawn — which is the same answer, but reached with a +round trip on every warm start. + +### When relayflowd writes it + +Only after the socket accepts connections, never before. Concretely, inside +`serve` (`kernel/relayflowd/src/server.rs:47`): + +1. `create_dir_all(data_dir)`. +2. **Acquire the singleton lock** (§3). Everything below happens under it. +3. Remove leftovers: unlink `relayflowd.sock` and `connection.json` if present. + Safe here and *only* here — holding the lock proves no other daemon owns + this data dir, so the socket inode is dead and the file is a corpse. +4. `UnixListener::bind(&socket_path)`. +5. **Write `connection.json`.** +6. `spawn_reconciler`, `spawn_liveness_sweep`, enter the accept loop. + +Step 4 is the guarantee. `UnixListener::bind` performs `bind(2)` *and* +`listen(2)`; from the moment `listen(2)` returns, a peer's `connect(2)` +completes into the backlog whether or not anyone has called `accept(2)` yet. +So a reader that sees the file can always connect. Writing at step 5 rather +than later also keeps the "connectable but unadvertised" window as short as it +can be. + +Step 3 is the other half of the guarantee and is easy to miss: because a +booting daemon *removes* a predecessor's `connection.json` before binding, +"file absent" is a true statement for the whole boot window. A CLI polling for +the file can therefore never read a corpse belonging to a daemon that is +already being replaced. + +The write is atomic: `connection.json.tmp.` in the same directory, mode +`0600`, then `rename(2)` over `connection.json`. Readers see a whole file or no +file, never a truncated one. + +### When relayflowd removes it + +On clean shutdown: `SIGTERM` and `SIGINT` handlers unlink `connection.json`, +then `relayflowd.sock`, then `_exit(0)`. + +Handlers must be async-signal-safe, so this is `libc::signal` + +`libc::unlink` + `libc::_exit`, not `std::fs::remove_file` (which allocates a +`CString`, and `malloc` in a signal handler can deadlock). Both paths are +converted to `CString` at startup and their raw pointers parked in +`static AtomicPtr`; the handler reads them and calls `unlink` twice. +Roughly: + +```rust +// kernel/relayflowd/src/server/lifecycle.rs +static CONNECTION_PATH: AtomicPtr = AtomicPtr::new(null_mut()); +static SOCKET_PATH: AtomicPtr = AtomicPtr::new(null_mut()); + +extern "C" fn on_terminate(_signum: c_int) { + for slot in [&CONNECTION_PATH, &SOCKET_PATH] { + let path = slot.load(Ordering::Relaxed); + if !path.is_null() { + unsafe { libc::unlink(path) }; // async-signal-safe + } + } + unsafe { libc::_exit(0) }; +} +``` + +`_exit` skips destructors. That is correct, not a shortcut: the journal is +SQLite with its own durability, and any lease still open at shutdown is +recovered by the reconciler on the next start — which is already the crash +path, exercised by the existing crash-injection tests. + +**`kill -9` leaves the file behind, by construction.** No handler runs. The +spec does not pretend otherwise and does not try to clean up after a hard kill +from inside the dying process. That residue is what §2 exists to detect, and +the lock in §3 is released by the OS regardless, so a hard-killed daemon never +blocks its successor. + +--- + +## 2. The staleness check + +Before trusting an existing `connection.json`, the CLI runs **both** checks. +Neither alone is sufficient. + +``` +readConnectionFile(dataDir): + 1. parse /connection.json; invalid or absent -> ABSENT + 2. shape-validate: socket_path non-empty string, pid integer > 0, + version string, protocol integer, started_at_ms integer > 0 + 3. conn.socket_path must equal resolve(join(dataDir, 'relayflowd.sock')) + -> otherwise STALE + +checkDaemon(dataDir): + 4. isProcessAlive(conn.pid) [check 1: cheap, negative] + 5. probeSocket(conn.socket_path): connect + hello [check 2: authoritative] + 6. probe.protocol === PROTOCOL_VERSION +``` + +**Step 3 — do not trust the path in the file.** The CLI recomputes the socket +path from `--data-dir` and requires the file to agree. A file whose +`socket_path` points elsewhere is stale, not a redirect. This is the same +refusal `server.rs:170-190` already makes about journals: accepting a file on +the strength of what it says about itself is how a foreign artifact gets +adopted. + +**Step 4 — `isProcessAlive`.** `process.kill(pid, 0)`; `ESRCH` means dead, +success means alive, and **`EPERM` means alive** (the process exists but is +owned by another user). `../relay`'s `isProcessRunning` +(`broker-lifecycle.ts:975-982`) treats every throw as dead; that misreads +`EPERM` as "gone" and would spawn a second daemon over a live one owned by +another user. Do not port that. + +**Step 5 — `probeSocket`.** Open a `JournalClient` against `socket_path`, +`connect()`, send `hello('flows')`, close. A refused connection, a connect +timeout, or a `hello` that does not answer inside the probe timeout is a +failed probe. This is the only check that proves something is *serving*. + +### The four cases, decided + +| pid alive | socket answers `hello` | Verdict | +|:--:|:--:|---| +| no | no | **Stale.** Unlink `connection.json`, spawn. | +| no | yes | **Attach**, with a `connection_file_stale` warning. The socket is the authority; something is serving it. Do not spawn — binding over a live daemon is the corruption this whole document exists to prevent. | +| yes | no | **Do not attach. Spawn.** Either a daemon is mid-boot, or the pid was reused by an unrelated process. The CLI does not need to tell those apart — see §3. | +| yes | yes | **Attach.** (Protocol mismatch at step 6 refuses instead: exit 2, `daemon_protocol_mismatch`. Never spawn a second daemon over a live incompatible one.) | + +Row 3 is the case the requirement names: *a stale file with a live unrelated +pid must not falsely attach*. The socket probe is what refuses it. It is also +exactly the case `../relay` gets wrong — `checkBrokerReadiness` with +`requireApi: false` (`broker-lifecycle.ts:1341-1350`) returns +`{ state: 'running' }` on pid liveness alone. We do not have a +`requireApi: false` mode. The probe is not optional. + +--- + +## 3. Attach-or-spawn, and the race + +### The mutex lives in the daemon, not the CLI + +`relayflowd serve` holds an exclusive `flock(2)` on +`/relayflowd.lock` for its entire lifetime: + +```rust +let lock = File::options().create(true).read(true).write(true) + .mode(0o600).open(data_dir.join("relayflowd.lock"))?; +if unsafe { libc::flock(lock.as_raw_fd(), libc::LOCK_EX | libc::LOCK_NB) } != 0 { + let error = io::Error::last_os_error(); + if error.kind() == io::ErrorKind::WouldBlock { + eprintln!("relayflowd: another relayflowd is already serving {}", + data_dir.display()); + std::process::exit(EXIT_ALREADY_SERVING); // 3 + } + return Err(error).context("lock data dir"); +} +std::mem::forget(lock); // held until the process dies +``` + +`libc` is already a unix dependency of the `relayflowd` crate. No new crate. + +**Why the daemon and not a CLI lock file.** A CLI-side lock only serializes +processes that agree to take it. Requirement §5 says an operator running +`relayflowd serve` by hand must keep working — and that operator takes no CLI +lock, so a CLI lock cannot stop a hand-run daemon and a CLI-spawned daemon from +both binding the same socket path. That is the actual corruption. Putting the +mutex in the daemon covers *every* way a daemon can start. And `flock` is +released by the kernel when the holder dies, including `kill -9`, so it needs +no staleness heuristic — which is precisely what a mkdir/pid lock file would +need, and why `../relay` ends up parsing `ps aux` +(`broker-lifecycle.ts:988-1000`) to clean up after its own locks. + +Strategy, named: **accept-and-let-the-loser-retry** — but the loser is the +*daemon*, not the CLI. The losing daemon exits before it can touch anything; +the losing CLI simply keeps polling and attaches to the winner. + +### Exit codes for `relayflowd serve` + +| Exit | Meaning | +|---:|---| +| `0` | Clean shutdown via SIGTERM/SIGINT. | +| `1` | Startup failure (cannot create data dir, cannot bind, cannot write the file). | +| `3` | Lost the lock — another relayflowd is already serving this data dir. Nothing was modified. | + +`3` must be distinct from `1`: it is how the spawning CLI tells "I lost a +benign race, keep polling" from "startup is broken, refuse". + +### The CLI algorithm + +Run before `flows run`, `flows resume`, `flows tick start`, and +`flows hn-monitor start` — every verb that opens the socket. + +``` +ensureDaemon(dataDir, { spawn = true, timeoutMs = 10_000 }): + + A. dataDir = resolve(dataDir) + + B. attachment = checkDaemon(dataDir) // §2 + if attachment is ATTACH -> return it (warm start) + if attachment is PROTOCOL_MISMATCH -> refuse (exit 2) + if !spawn -> refuse (exit 2, daemon_unreachable) + + C. mkdirSync(dataDir, { recursive: true }) + binary = resolveRelayflowdBinary() // §3.1; refuse if not found + log = openSync(join(dataDir, 'relayflowd.log'), 'a') + child = spawn(binary, ['--data-dir', dataDir, 'serve'], { + detached: true, + stdio: ['ignore', 'ignore', log], + env: process.env, + cwd: process.cwd(), + }) + child.unref() + + D. deadline = now + timeoutMs; loop every 50ms: + a. child exited with code 3 -> lost the race. Keep polling. + Do NOT respawn. Do NOT refuse. + b. child exited non-zero, != 3 -> refuse (exit 2, daemon_start_failed), + quoting the tail of relayflowd.log + c. attachment = checkDaemon(dataDir); if ATTACH -> return it + d. past deadline -> refuse (exit 2, daemon_start_timeout) +``` + +`detached: true` puts the child in its own session and process group. Two +consequences, both required: it outlives the CLI process, and a `Ctrl-C` sent +to the CLI's process group does not reach it. `stdio` never inherits the CLI's +— a daemon holding the CLI's stdout would interleave its output with +`flows run --json`'s single report object, and would hold the pipe open after +the CLI exits, hanging anything reading it. stderr goes to +`/relayflowd.log` so a failed start has evidence; `../relay` writes a +dedicated `background-start-error.log` for the same reason +(`broker-lifecycle.ts:75`). + +`timeoutMs` is 10s, matching `../relay`'s `DETACHED_START_READY_TIMEOUT_MS`. +The 50ms interval matches the existing loop in +`scripts/run-local-workflow.mjs:64-71`. + +### The race, walked exactly + +Two `flows run` invocations, empty data dir `D`: + +1. Both call `checkDaemon(D)` → ABSENT. Both decide to spawn. +2. Both spawn `relayflowd serve --data-dir D`. Call them A and B. +3. A and B both `open(D/relayflowd.lock)` and call + `flock(LOCK_EX|LOCK_NB)`. `flock` is atomic in the kernel: exactly one + succeeds. Say A wins. +4. B gets `EWOULDBLOCK`, prints one line, exits 3. **B has unlinked nothing, + bound nothing, written nothing** — every destructive step in `serve` is + sequenced after the lock (§1, steps 2→3). +5. A unlinks leftovers, binds, listens, atomically renames `connection.json` + into place. +6. CLI-A and CLI-B both poll. CLI-B observes its child exit 3 (branch D.a) and + keeps polling rather than reporting failure. Both then see the file, both + probe the socket successfully, both attach to A. + +Result: one daemon, one socket, one journal owner, two happy CLIs. The same +walk covers a CLI racing an operator's hand-run `relayflowd serve`, and a CLI +racing a daemon that is mid-boot (row 3 of the §2 table): the CLI's child loses +the lock, exits 3, and the CLI attaches to the daemon that was already coming +up. + +**This is the load-bearing simplification: because the daemon holds the mutex, +the CLI never has to be clever. Spawning when in doubt is always safe.** The +CLI never needs to identify whether a live pid is "really" a relayflowd, and +therefore never needs `ps` parsing or a process-identity field in `hello`. + +### Why `hello` is not extended + +An earlier draft added `pid`/`started_at_ms` to the `hello` result so the CLI +could prove the file describes the daemon actually on the socket. It is not +needed. With the lock in place, `connection.json` is written only by the daemon +holding it, and a booting daemon unlinks its predecessor's file before binding. +The only interleaving that reads daemon A's file and reaches daemon B's socket +requires reading the file after A died and before B unlinked it — and B's +socket is at the same path the CLI already recomputed and validated in §2 step +3, so attaching is correct anyway. The protocol is unchanged by this document. + +### 3.1 Resolving the `relayflowd` binary + +New module `packages/sdk/src/relayflowd-path.ts`, modeled on +`../relay`'s `packages/harness-driver/src/broker-path.ts`. Its multi-anchor +approach transfers directly, and the reason is the one stated in that file: a +version manager can launch Node with a minimal PATH, which is exactly when a +`which` lookup fails to see the binary the installer placed next to its +launcher. Never rely on PATH alone. + +Order: + +1. `RELAYFLOWD_BIN` — already this repo's convention + (`scripts/run-local-workflow.mjs:44`). If set and executable, use it. If set + and **not** executable, **refuse** — do not fall through. An operator who + set it meant it, and silently ignoring it is the fallback AGENTS.md rule 4 + forbids. +2. Sibling of the running `flows` entrypoint: + `join(dirname(realpathSync(process.argv[1])), 'relayflowd')`. This is the + published layout — `@relayflows/runtime-linux-x64` ships `bin/flows` and + `bin/relayflowd` side by side (`scripts/pack-release.mjs:31`). `realpathSync` + matters: package-manager launchers expose the entrypoint as a symlink whose + target sits in the real install tree. +3. Optional-dependency package: + `require.resolve('@relayflows/runtime--/package.json')` → + `/bin/relayflowd`, tried from several `createRequire` anchors — + this module's own path, `process.argv[1]`, and + `join(process.cwd(), 'package.json')`. Multiple anchors because a globally + installed `flows` resolving a per-project optional dep sits outside the + consumer's `node_modules`, which is `broker-path.ts:84-106` verbatim in its + reasoning. +4. Source checkout: walk up from `process.cwd()` (bounded, 8 levels) to the + nearest ancestor containing `kernel/Cargo.toml`, then + `kernel/target/release/relayflowd`, then `kernel/target/debug/relayflowd`. +5. PATH, via `which` / `where`. + +Not found → exit 2, `relayflowd_not_found`, message naming both +`@relayflows/runtime--` and the `RELAYFLOWD_BIN` escape hatch. + +--- + +## 4. What changes, and what is new + +### New — `packages/sdk/src/daemon-lifecycle.ts` + +Mirrors `../relay/packages/cli/src/cli/lib/broker-lifecycle.ts` + +`broker-connection.ts`, collapsed into one file because there is no +url/port/api_key resolution chain to own. Exports: + +```ts +export interface DaemonConnection { + socket_path: string; pid: number; version: string; + protocol: number; started_at_ms: number; +} +export type DaemonState = + | { kind: 'attached'; connection: DaemonConnection; warning?: string } + | { kind: 'absent' } + | { kind: 'stale'; reason: string } + | { kind: 'incompatible'; protocol: number }; + +export function readConnectionFile(dataDir: string): DaemonConnection | null; +export function isProcessAlive(pid: number): boolean; +export async function probeSocket(socketPath: string): Promise; +export async function checkDaemon(dataDir: string): Promise; +export async function ensureDaemon( + dataDir: string, + options?: { spawn?: boolean; timeoutMs?: number }, +): Promise; +``` + +Injectable seams for tests, following the `deps` pattern the reference file +uses: `spawnProcess`, `killProcess`, `now`, `sleep`, `fs`. No file I/O or +process spawning is reached in unit tests. + +### New — `packages/sdk/src/relayflowd-path.ts` + +Binary resolution, §3.1. Split from `daemon-lifecycle.ts` so neither file +approaches the 500-line smell in AGENTS.md rule 1; each stays well under 250. + +### New — `kernel/relayflowd/src/server/lifecycle.rs` + +The daemon half: the `flock`, the leftover sweep, the atomic +`connection.json` write, the signal handlers. `server.rs` calls +`lifecycle::acquire(data_dir)?` before its existing unlink/bind, and +`lifecycle::publish(&socket_path, ...)` immediately after `bind`. Net change to +`server.rs` is roughly ten lines; `server.rs:50-53`'s unconditional +`remove_file` moves inside `lifecycle::acquire`, where it is guarded by the +lock. + +### Changed — `packages/sdk/src/cli.ts` + +`runCli` calls `ensureDaemon(parsed.dataDir)` before dispatching `run`, +`resume`, `tick`, and `hn-monitor` (currently `cli.ts:74-121`). On a refusal it +emits the existing exit-2 diagnostic shape and returns 2 — the report format +does not change, only the `kind` and the message. + +New flag, parsed alongside `--data-dir`: **`--no-spawn`** (also +`FLOWS_NO_SPAWN=1`), which sets `{ spawn: false }` and restores today's exact +fail-closed behavior. This is the lever for CI that means to assert a daemon is +already present rather than conjure one. + +`flows check` is untouched and stays daemon-free. It is not an omission: the +argument parser already refuses `--data-dir` on `check` +(`cli.ts:158`), so there is no data dir for it to attach to, and `checkFlow` is +a pure compile-and-preflight that never opens a socket. `flows check` keeps +working with no daemon, no binary, and no data dir at all — a property worth +keeping. + +### Changed — `packages/sdk/src/cli/run.ts` + +Only the `daemon_unreachable` message text (`run.ts:146-158`), which currently +tells the operator to run `relayflowd serve` by hand. With `--no-spawn` that +advice is still right; without it, reaching this branch means spawning was +tried and failed, so the message must say which of `relayflowd_not_found`, +`daemon_start_failed`, or `daemon_start_timeout` happened. + +### Changed — `packages/sdk/src/journal-client.ts` + +**Almost nothing, deliberately.** `connect()` stays fail-closed with no retry +and no spawn. Lifecycle is not transport: putting spawn logic in the client +would make every `AgentWorker`, tick runner, and demo silently conjure daemons +as a side effect of connecting, and would put process management inside the +module AGENTS.md rule 3 calls the boundary. + +One addition: `JournalClientOptions.connectTimeoutMs` (default 2000), applied +to `connect()`. `probeSocket` runs before every command, and today `connect()` +has no timer at all while `hello` inherits the 30s request default — a socket +whose listener accepts but never answers would hang the CLI for 30 seconds +before it could decide to spawn. + +--- + +## 5. Backward compatibility + +Attach-or-spawn is an addition. Manual operation is not replaced. + +- **`relayflowd serve --data-dir D` by hand, on a free data dir**: byte-for-byte + the same observable behavior, plus it now writes `connection.json` and now + cleans up on SIGTERM/SIGINT. Nothing it did before stops. Both argument + orders keep working — `--data-dir` is a global clap arg + (`main.rs:13-17`), so `relayflowd --data-dir D serve` is equally valid. +- **`flows run` against a hand-started daemon**: `readConnectionFile` finds the + file that daemon wrote, the pid is alive, the socket answers → attach. No + spawn, no duplicate, no signal. **The CLI never terminates a daemon** — not + one it found, and not one it spawned. There is no `flows down` verb and this + document does not add one. +- **`flows check`**: unchanged, still needs no daemon. +- **`flows run` with no daemon and no binary findable**: still exit 2, still a + refusal before any journal write. The `kind` changes from + `daemon_unreachable` to `relayflowd_not_found` and the message names the + runtime package. Same contract, better message. +- **`scripts/run-local-workflow.mjs`**: untouched and unaffected. It spawns its + own daemon into a fresh `mkdtemp` data dir and drives `JournalClient` + directly, never through `ensureDaemon`. Its lock is uncontended. +- **Exit codes for `flows`**: unchanged. Every new refusal is exit 2 (refused + before a journal write), which is what the table in `docs/SURFACE.md` §5 + already promises for an unreachable daemon. + +### The one intentional behavior change + +A **second** `relayflowd serve` on a data dir already being served now refuses +with exit 3 instead of unlinking the live socket and rebinding +(`server.rs:50-53`). The old behavior silently orphaned the first daemon: it +kept running, holding the journal, reachable by nobody, while the second bound +a fresh inode at the same path. That is not a contract anyone should be able to +depend on, and it is the specific corruption this design exists to prevent. It +is called out here because it is the only case where an operator sees something +they did not see before. + +### Documentation that must change with this + +`docs/SURFACE.md` §5 currently states "Neither verb starts the daemon +implicitly." That sentence becomes false and must be rewritten in the same +commit as the CLI change, along with a mention of `--no-spawn` and of +`relayflowd serve` remaining fully supported. + +--- + +## 6. Tests this spec must be held to + +Kernel (`cargo test --workspace`): + +1. `connection.json` does not exist before `bind`, and a client that reads it + can always connect — assert by racing a connect against the file's + appearance, not by sleeping. +2. Clean shutdown on `SIGTERM` removes `connection.json` and the socket; exit + code 0. +3. `SIGKILL` leaves `connection.json` behind, and the file's pid is then dead — + the residue the CLI's §2 check must catch. +4. A second `serve` on a served data dir exits 3, and the first daemon's socket + is still accepting afterwards. This is the anti-hijack test. +5. A `SIGKILL`ed daemon's successor starts cleanly: the `flock` is free, the + dead socket and stale file are removed, a new file is written. +6. `connection.json` contents round-trip: `socket_path` absolute and equal to + `/relayflowd.sock`, `pid` = the serving process, `protocol` = + `PROTOCOL_VERSION`. + +SDK (`npm test` in `packages/sdk`): + +7. Cold start: empty data dir, no daemon → exactly one spawn, run succeeds. +8. Warm start: daemon already running → attach, **zero** spawns (assert on the + injected `spawnProcess` seam). +9. Stale file, dead pid, dead socket → file removed, fresh daemon spawned. +10. Stale file, dead pid, **live socket** → attach, no spawn, warning emitted. +11. Live pid, dead socket (the pid-reuse case) → does **not** attach. +12. `isProcessAlive` returns `true` on `EPERM`. +13. `socket_path` disagreeing with `/relayflowd.sock` → stale. +14. Protocol mismatch in the file or in `hello` → exit 2, no spawn. +15. Two concurrent `ensureDaemon` calls against one empty data dir → one + surviving daemon; the loser's child exits 3 and its CLI still attaches. +16. Spawn options assert `detached: true`, `stdio[0] === 'ignore'`, + `stdio[1] === 'ignore'`, and `unref()` called. +17. `--no-spawn` / `FLOWS_NO_SPAWN=1` reproduces today's exit-2 + `daemon_unreachable` exactly. +18. `RELAYFLOWD_BIN` set to a non-executable path refuses rather than falling + through to PATH. + +Test 15 is the one that matters most and is the hardest to fake: it must spawn +real processes against a real temp data dir, because the property under test is +enforced by `flock(2)`, not by any code we could stub. diff --git a/kernel/relayflowd/src/server.rs b/kernel/relayflowd/src/server.rs index 5ae9a9936..95a42df99 100644 --- a/kernel/relayflowd/src/server.rs +++ b/kernel/relayflowd/src/server.rs @@ -11,6 +11,8 @@ mod cancel; #[cfg(unix)] mod channels; #[cfg(unix)] +mod lifecycle; +#[cfg(unix)] pub mod liveness; #[cfg(unix)] mod reconcile; @@ -46,13 +48,22 @@ pub fn serve(data_dir: &Path) -> Result<()> { }; std::fs::create_dir_all(data_dir)?; + let _lock = match lifecycle::acquire(data_dir) { + Ok(lock) => lock, + Err(lifecycle::AcquireError::AlreadyServing) => { + eprintln!( + "relayflowd: another relayflowd is already serving {}", + data_dir.display() + ); + std::process::exit(3); + } + Err(lifecycle::AcquireError::Io(error)) => return Err(error.into()), + }; let socket_path = data_dir.join("relayflowd.sock"); - if socket_path.exists() { - std::fs::remove_file(&socket_path) - .with_context(|| format!("remove stale socket {}", socket_path.display()))?; - } + lifecycle::remove_residue(data_dir)?; let listener = UnixListener::bind(&socket_path) .with_context(|| format!("bind socket {}", socket_path.display()))?; + lifecycle::publish(&socket_path)?; let hub = Arc::new(ProtocolHub::default()); reconcile::spawn_reconciler(data_dir.to_path_buf(), hub.clone()); // Trigger-plane liveness sweep (RFC-0001 gate 2, Native silent-death diff --git a/kernel/relayflowd/src/server/lifecycle.rs b/kernel/relayflowd/src/server/lifecycle.rs new file mode 100644 index 000000000..53cec8352 --- /dev/null +++ b/kernel/relayflowd/src/server/lifecycle.rs @@ -0,0 +1,156 @@ +//! Filesystem ownership and advertisement for a serving daemon. + +use std::{ + ffi::CString, + fs::{File, OpenOptions}, + io::{self, Write}, + os::{ + raw::{c_char, c_int}, + unix::{fs::OpenOptionsExt, io::AsRawFd}, + }, + path::Path, + ptr, + sync::atomic::{AtomicPtr, Ordering}, + time::{SystemTime, UNIX_EPOCH}, +}; + +use anyhow::{Context, Result}; +use relayflowd_core::PROTOCOL_VERSION; +use serde::Serialize; + +static CONNECTION_PATH: AtomicPtr = AtomicPtr::new(ptr::null_mut()); +static SOCKET_PATH: AtomicPtr = AtomicPtr::new(ptr::null_mut()); + +/// Keep this value alive for the entire `serve` call. +pub(crate) struct DaemonLock { + _file: File, +} + +#[derive(Debug)] +pub(crate) enum AcquireError { + AlreadyServing, + Io(io::Error), +} + +impl std::fmt::Display for AcquireError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::AlreadyServing => f.write_str("another relayflowd is already serving"), + Self::Io(error) => error.fmt(f), + } + } +} +impl std::error::Error for AcquireError {} + +pub(crate) fn acquire(data_dir: &Path) -> std::result::Result { + let lock = OpenOptions::new() + .create(true) + .read(true) + .write(true) + .mode(0o600) + .open(data_dir.join("relayflowd.lock")) + .map_err(AcquireError::Io)?; + if unsafe { libc::flock(lock.as_raw_fd(), libc::LOCK_EX | libc::LOCK_NB) } == 0 { + return Ok(DaemonLock { _file: lock }); + } + let error = io::Error::last_os_error(); + if error.kind() == io::ErrorKind::WouldBlock { + Err(AcquireError::AlreadyServing) + } else { + Err(AcquireError::Io(error)) + } +} + +/// Safe only after `acquire`: no other daemon can own these paths. +pub(crate) fn remove_residue(data_dir: &Path) -> Result<()> { + for name in ["relayflowd.sock", "connection.json"] { + let path = data_dir.join(name); + match std::fs::remove_file(&path) { + Ok(()) => {} + Err(error) if error.kind() == io::ErrorKind::NotFound => {} + Err(error) => { + return Err(error).with_context(|| format!("remove stale {}", path.display())); + } + } + } + Ok(()) +} + +#[derive(Serialize)] +struct ConnectionFile<'a> { + socket_path: &'a str, + pid: u32, + version: &'a str, + protocol: u32, + started_at_ms: u128, +} + +/// Publish only after `UnixListener::bind`, which has already called listen(2). +pub(crate) fn publish(socket_path: &Path) -> Result<()> { + // `absolute` preserves the caller's data-dir spelling (unlike + // `canonicalize`, which would turn `/var` into macOS's `/private/var` and + // fail the client's lexical `resolve(dataDir)` comparison). + let socket_path = std::path::absolute(socket_path) + .with_context(|| format!("make socket path absolute {}", socket_path.display()))?; + let socket_text = socket_path + .to_str() + .context("socket path is not valid UTF-8")?; + let data_dir = socket_path.parent().context("socket path has no parent")?; + let connection_path = data_dir.join("connection.json"); + let temporary_path = data_dir.join(format!("connection.json.tmp.{}", std::process::id())); + let record = ConnectionFile { + socket_path: socket_text, + pid: std::process::id(), + version: env!("CARGO_PKG_VERSION"), + protocol: PROTOCOL_VERSION, + started_at_ms: SystemTime::now().duration_since(UNIX_EPOCH)?.as_millis(), + }; + let bytes = serde_json::to_vec(&record)?; + let result = (|| -> Result<()> { + let mut file = OpenOptions::new() + .write(true) + .create_new(true) + .mode(0o600) + .open(&temporary_path) + .with_context(|| format!("create {}", temporary_path.display()))?; + file.write_all(&bytes)?; + file.sync_all()?; + // Register before making the advertisement visible. A caller may send + // SIGTERM the instant rename returns, so visibility must imply cleanup + // is already installed. + install_cleanup_handlers(&connection_path, &socket_path)?; + std::fs::rename(&temporary_path, &connection_path) + .with_context(|| format!("publish {}", connection_path.display()))?; + Ok(()) + })(); + if result.is_err() { + let _ = std::fs::remove_file(&temporary_path); + } + result +} + +fn install_cleanup_handlers(connection_path: &Path, socket_path: &Path) -> Result<()> { + // These strings remain valid until the signal handler calls `_exit`. + let connection = CString::new(connection_path.as_os_str().as_encoded_bytes())?; + let socket = CString::new(socket_path.as_os_str().as_encoded_bytes())?; + CONNECTION_PATH.store(connection.into_raw(), Ordering::Relaxed); + SOCKET_PATH.store(socket.into_raw(), Ordering::Relaxed); + unsafe { + if libc::signal(libc::SIGTERM, on_terminate as *const () as usize) == libc::SIG_ERR + || libc::signal(libc::SIGINT, on_terminate as *const () as usize) == libc::SIG_ERR + { + return Err(io::Error::last_os_error().into()); + } + } + Ok(()) +} + +extern "C" fn on_terminate(_signal: c_int) { + for slot in [&CONNECTION_PATH, &SOCKET_PATH] { + let path = slot.load(Ordering::Relaxed); + if !path.is_null() { + unsafe { libc::unlink(path) }; + } + } + unsafe { libc::_exit(0) } +} diff --git a/kernel/relayflowd/src/server/liveness.rs b/kernel/relayflowd/src/server/liveness.rs index 2f0ea8296..c1aed5084 100644 --- a/kernel/relayflowd/src/server/liveness.rs +++ b/kernel/relayflowd/src/server/liveness.rs @@ -120,7 +120,12 @@ pub fn sweep_pass( // a no-op. The signal we just emitted was a spurious alert // (harmless — at-least-once observability); the row remains // sweep-visible for the next real crossing. - match registry.latch_stale(&row.flow_key, &row.subscription_id, row.last_event_at_ms, now_ms) { + match registry.latch_stale( + &row.flow_key, + &row.subscription_id, + row.last_event_at_ms, + now_ms, + ) { Ok(true) => {} Ok(false) => { eprintln!( @@ -251,7 +256,9 @@ mod tests { sweep_pass(dir.path(), &sweep_id_for(1_040_000), "w", 1_040_000).unwrap(); // Next bucket, same row: must NOT re-emit (latched). - let leftover = registry.detect_stale(&sweep_id_for(1_080_000), "w", 1_080_000).unwrap(); + let leftover = registry + .detect_stale(&sweep_id_for(1_080_000), "w", 1_080_000) + .unwrap(); assert!( leftover.is_empty(), "subscription.stale re-emitted after being latched: {leftover:?}" diff --git a/kernel/relayflowd/src/server/tests.rs b/kernel/relayflowd/src/server/tests.rs index 9adf8decc..2c98cb6dc 100644 --- a/kernel/relayflowd/src/server/tests.rs +++ b/kernel/relayflowd/src/server/tests.rs @@ -215,19 +215,21 @@ fn run_resume_adopts_a_real_journal_whose_registry_row_is_missing() { ) .unwrap(); let run_id = outcome.run_id.clone(); - assert!(data_dir.join("runs").join(format!("{run_id}.sqlite3")).exists()); + assert!( + data_dir + .join("runs") + .join(format!("{run_id}.sqlite3")) + .exists() + ); // Reproduce the crash window: the journal survives, the index entry does // not. Removing the registry outright is the same state a SIGKILL between // the append and the register leaves behind, and strictly harsher -- the // row is not merely stale, there is nothing to consult at all. for suffix in ["", "-wal", "-shm"] { - let _ = std::fs::remove_file( - data_dir.join(format!("relayflowd.sqlite3{suffix}")), - ); + let _ = std::fs::remove_file(data_dir.join(format!("relayflowd.sqlite3{suffix}"))); } - let registry = - relayflowd_journal::Registry::open(data_dir.join("relayflowd.sqlite3")).unwrap(); + let registry = relayflowd_journal::Registry::open(data_dir.join("relayflowd.sqlite3")).unwrap(); assert!(registry.lookup(&run_id).unwrap().is_none()); let hub = Arc::new(ProtocolHub::default()); @@ -237,9 +239,7 @@ fn run_resume_adopts_a_real_journal_whose_registry_row_is_missing() { &hub, 1, &writer, - &format!( - r#"{{"id":"resume","verb":"run.resume","params":{{"run_id":"{run_id}"}}}}"# - ), + &format!(r#"{{"id":"resume","verb":"run.resume","params":{{"run_id":"{run_id}"}}}}"#), ); assert!( @@ -275,8 +275,15 @@ fn run_resume_refuses_a_journal_that_never_recorded_its_run() { std::fs::create_dir_all(data_dir.join("runs")).unwrap(); let path = data_dir.join("runs").join(format!("{run_id}.sqlite3")); let journal = relayflowd_journal::SqliteJournal::create(&path, run_id, 0).unwrap(); - assert_eq!(journal.run_id(), run_id, "the meta row is what makes this tempting"); - assert!(journal.run_spec().is_err(), "and there is no spec to resume"); + assert_eq!( + journal.run_id(), + run_id, + "the meta row is what makes this tempting" + ); + assert!( + journal.run_spec().is_err(), + "and there is no spec to resume" + ); drop(journal); let hub = Arc::new(ProtocolHub::default()); @@ -298,8 +305,7 @@ fn run_resume_refuses_a_journal_that_never_recorded_its_run() { failure is not" ); - let registry = - relayflowd_journal::Registry::open(data_dir.join("relayflowd.sqlite3")).unwrap(); + let registry = relayflowd_journal::Registry::open(data_dir.join("relayflowd.sqlite3")).unwrap(); assert!( registry.lookup(run_id).unwrap().is_none(), "a refused journal must not leave a registry row behind" @@ -363,8 +369,7 @@ fn run_resume_refuses_a_valid_journal_that_belongs_to_another_run() { assert_eq!(error.code, "run_not_found"); // And nothing was adopted under the impostor id. - let registry = - relayflowd_journal::Registry::open(data_dir.join("relayflowd.sqlite3")).unwrap(); + let registry = relayflowd_journal::Registry::open(data_dir.join("relayflowd.sqlite3")).unwrap(); assert!( registry.lookup(impostor).unwrap().is_none(), "a refused journal must not leave a registry row behind" diff --git a/kernel/relayflowd/tests/daemon_lifecycle.rs b/kernel/relayflowd/tests/daemon_lifecycle.rs new file mode 100644 index 000000000..1b99f4b3c --- /dev/null +++ b/kernel/relayflowd/tests/daemon_lifecycle.rs @@ -0,0 +1,193 @@ +#![cfg(unix)] + +use std::{ + os::unix::net::UnixStream, + path::{Path, PathBuf}, + process::{Child, Command, Stdio}, + thread, + time::{Duration, Instant}, +}; + +use relayflowd_core::PROTOCOL_VERSION; +use serde_json::Value; +use tempfile::TempDir; + +fn start(data_dir: &Path) -> Child { + Command::new(env!("CARGO_BIN_EXE_relayflowd")) + .args(["--data-dir", data_dir.to_str().unwrap(), "serve"]) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::piped()) + .spawn() + .unwrap() +} + +fn connection_path(data_dir: &Path) -> PathBuf { + data_dir.join("connection.json") +} + +/// Observe publication and immediately connect; publication is only legal once +/// bind/listen has completed, so every observed file must be connectable. +fn wait_for_connection(data_dir: &Path) -> Value { + let path = connection_path(data_dir); + let deadline = Instant::now() + Duration::from_secs(5); + loop { + if path.exists() { + let connection: Value = serde_json::from_slice(&std::fs::read(&path).unwrap()).unwrap(); + let socket = connection["socket_path"].as_str().unwrap(); + UnixStream::connect(socket).expect("published connection must already accept"); + return connection; + } + assert!( + Instant::now() < deadline, + "connection file was never published" + ); + thread::sleep(Duration::from_millis(5)); + } +} + +fn signal(child: &Child, signal: i32) { + assert_eq!(unsafe { libc::kill(child.id() as i32, signal) }, 0); +} + +/// Like `wait_for_connection`, but for a data dir that may still hold a +/// predecessor's stale (unconnectable) file: keeps polling past that file — +/// it is not published until the new pid's socket is live — until a file +/// naming a different pid appears, and only then applies the same +/// connect-must-succeed invariant. +fn wait_for_new_connection(data_dir: &Path, excluding_pid: i32) -> Value { + let path = connection_path(data_dir); + let deadline = Instant::now() + Duration::from_secs(5); + loop { + if path.exists() { + if let Ok(bytes) = std::fs::read(&path) { + if let Ok(connection) = serde_json::from_slice::(&bytes) { + if connection["pid"].as_i64() != Some(excluding_pid as i64) { + let socket = connection["socket_path"].as_str().unwrap(); + UnixStream::connect(socket) + .expect("published connection must already accept"); + return connection; + } + } + } + } + assert!( + Instant::now() < deadline, + "successor's connection file was never published" + ); + thread::sleep(Duration::from_millis(5)); + } +} + +#[test] +fn connection_file_is_published_only_after_the_socket_is_live() { + let directory = TempDir::new().unwrap(); + assert!(!connection_path(directory.path()).exists()); + let mut daemon = start(directory.path()); + let connection = wait_for_connection(directory.path()); + assert_eq!(connection["pid"].as_u64(), Some(daemon.id() as u64)); + assert_eq!( + connection["protocol"].as_u64(), + Some(PROTOCOL_VERSION as u64) + ); + assert_eq!( + connection["socket_path"].as_str(), + Some(directory.path().join("relayflowd.sock").to_str().unwrap()) + ); + signal(&daemon, libc::SIGTERM); + assert!(daemon.wait().unwrap().success()); +} + +#[test] +fn clean_shutdown_removes_advertisement_and_socket() { + let directory = TempDir::new().unwrap(); + let mut daemon = start(directory.path()); + wait_for_connection(directory.path()); + signal(&daemon, libc::SIGINT); + assert!(daemon.wait().unwrap().success()); + assert!(!connection_path(directory.path()).exists()); + assert!(!directory.path().join("relayflowd.sock").exists()); +} + +#[test] +fn sigkill_leaves_a_stale_file_with_a_dead_pid() { + let directory = TempDir::new().unwrap(); + let mut daemon = start(directory.path()); + let connection = wait_for_connection(directory.path()); + let pid = connection["pid"].as_i64().unwrap() as i32; + signal(&daemon, libc::SIGKILL); + daemon.wait().unwrap(); + assert!( + connection_path(directory.path()).exists(), + "SIGKILL must not invoke cleanup" + ); + assert_eq!(unsafe { libc::kill(pid, 0) }, -1); + assert_eq!( + std::io::Error::last_os_error().raw_os_error(), + Some(libc::ESRCH) + ); +} + +/// §6 test 4, the anti-hijack test: a second `serve` on a data dir already +/// being served must lose the race cleanly (exit 3, nothing touched), and the +/// first daemon must still be accepting connections afterwards. +#[test] +fn a_second_serve_on_a_served_data_dir_refuses_and_the_first_keeps_serving() { + let directory = TempDir::new().unwrap(); + let mut first = start(directory.path()); + let first_connection = wait_for_connection(directory.path()); + + let mut second = start(directory.path()); + let status = second.wait().unwrap(); + assert_eq!( + status.code(), + Some(3), + "a second daemon on a served data dir must exit 3, got {status:?}" + ); + + // The loser must not have touched the winner's advertisement or socket. + let still_published: Value = + serde_json::from_slice(&std::fs::read(connection_path(directory.path())).unwrap()) + .unwrap(); + assert_eq!(still_published, first_connection); + UnixStream::connect(first_connection["socket_path"].as_str().unwrap()) + .expect("the first daemon must still be accepting connections"); + + signal(&first, libc::SIGTERM); + assert!(first.wait().unwrap().success()); +} + +/// §6 test 5: a SIGKILL'd daemon's successor starts cleanly — the `flock` is +/// released by the kernel regardless of how the holder died, so the residue +/// left behind (dead socket, stale `connection.json`) must not block a fresh +/// daemon from acquiring the lock, removing that residue, and publishing its +/// own advertisement. +#[test] +fn a_sigkilled_daemons_successor_starts_cleanly() { + let directory = TempDir::new().unwrap(); + let mut first = start(directory.path()); + let first_connection = wait_for_connection(directory.path()); + let first_pid = first_connection["pid"].as_i64().unwrap() as i32; + signal(&first, libc::SIGKILL); + first.wait().unwrap(); + assert!( + connection_path(directory.path()).exists(), + "SIGKILL must leave the file behind for the successor to detect" + ); + + let mut second = start(directory.path()); + let second_connection = wait_for_new_connection(directory.path(), first_pid); + assert_ne!( + second_connection["pid"].as_i64().unwrap() as i32, + first_pid, + "the successor must be a distinct process" + ); + assert_eq!( + second_connection["socket_path"], + first_connection["socket_path"], + "the successor binds the same socket path" + ); + + signal(&second, libc::SIGTERM); + assert!(second.wait().unwrap().success()); +} diff --git a/ops/DAEMON-LIFECYCLE-REPORT.md b/ops/DAEMON-LIFECYCLE-REPORT.md new file mode 100644 index 000000000..bbb18eb42 --- /dev/null +++ b/ops/DAEMON-LIFECYCLE-REPORT.md @@ -0,0 +1,195 @@ +# Daemon lifecycle: implementation report + +Phase 1 of "package flows so it's frictionless" (`workflows/daemon-lifecycle.yaml`). +This report closes out the workflow's `adversarial-review` and `report` steps; +`design`, `kernel-impl`, `kernel-tests`, `cli-impl`, `sdk-tests`, and +`e2e-smoke` all completed and are green as of this report, on branch +`feat/daemon-lifecycle`, working tree uncommitted. + +**Update after the first review pass**: the first adversarial pass returned +`REVIEW_FAILED` — §6 tests 4 and 5 were missing. Both have since been written +against the real `relayflowd` binary in `kernel/relayflowd/tests/daemon_lifecycle.rs` +and pass (`a_second_serve_on_a_served_data_dir_refuses_and_the_first_keeps_serving`, +`a_sigkilled_daemons_successor_starts_cleanly`), and `cargo test --workspace` +is green with them included. The "Adversarial review verdict" section below is +left as originally written (the violation as first found) with a note added +at its top recording that the gap is now closed; nothing else in that section +changed on re-verification. + +## What was built + +`relayflowd serve` now holds an exclusive `flock(2)` on +`/relayflowd.lock` for its whole lifetime (`kernel/relayflowd/src/server/lifecycle.rs`), +sweeps leftover socket/connection-file residue only after acquiring that lock, +binds the unix socket, and only then atomically publishes `/connection.json` +(`socket_path`, `pid`, `version`, `protocol`, `started_at_ms`) via a +temp-file-then-rename. Clean shutdown (SIGTERM/SIGINT) unlinks both the +connection file and the socket from an async-signal-safe handler before +`_exit(0)`; a hard `kill -9` leaves the file behind by construction. A second +`serve` on an already-served data dir loses the `flock`, touches nothing, and +exits 3. + +On the CLI side, `packages/sdk/src/daemon-connection.ts` reads and validates +`connection.json` and decides `absent` / `stale` / `attached` / `incompatible` +by combining a pid-liveness check with a socket `hello` probe — the socket is +always the authority, the file is never trusted alone. +`packages/sdk/src/daemon-lifecycle.ts` is the attach-or-spawn algorithm: +attach if something answers; otherwise resolve the `relayflowd` binary +(`packages/sdk/src/relayflowd-path.ts`, multi-anchor resolution ported from +`../relay`'s `broker-path.ts`), spawn it detached (own session, ignored +stdio, log to `/relayflowd.log`, `unref()`'d), and poll for the +connection file up to a bounded timeout, treating exit code 3 as "lost a +benign race, keep polling" rather than a failure. This is wired into +`flows check` / `run` / `resume` via `packages/sdk/src/cli.ts` and +`packages/sdk/src/cli/run.ts`'s shared `connect()` seam. A new `--no-spawn` +flag (`FLOWS_NO_SPAWN=1`) restores today's exact fail-closed behavior for CI. +`journal-client.ts` gained a `connectTimeoutMs` (default 2s) so a socket that +accepts but never answers can't hang the CLI for the old 30s request default. + +## Test results + +**Kernel** (`cd kernel && cargo test --workspace`): all green. + +``` +running 9 tests (relayflowd-core) ... 9 passed +running 28 tests (relayflowd-journal) ... 28 passed +running N tests (relayflowd, incl. server::tests + daemon_lifecycle.rs) +test result: ok. 0 failed +``` + +Kernel-side daemon-lifecycle tests present (`kernel/relayflowd/tests/daemon_lifecycle.rs`), +now all six of §6's kernel tests: +`connection_file_is_published_only_after_the_socket_is_live` (test 1), +`clean_shutdown_removes_advertisement_and_socket` (test 2), +`sigkill_leaves_a_stale_file_with_a_dead_pid` (test 3), +`a_second_serve_on_a_served_data_dir_refuses_and_the_first_keeps_serving` (test 4, +added after the first review pass — spawns two real `relayflowd serve` +processes against one data dir and asserts the loser exits 3 untouched and the +winner keeps accepting), and `a_sigkilled_daemons_successor_starts_cleanly` +(test 5, added after the first review pass — SIGKILLs a daemon, starts a +second against the same data dir, and asserts the successor acquires the +freed `flock`, sweeps the residue, and publishes its own advertisement). All +five pass against the real binary; `cargo test --workspace` is green with +them included. + +**SDK** (`cd packages/sdk && npm test`): **803 passed, 3 skipped** (37 test +files passed, 1 skipped, of 38). Includes `tests/daemon-lifecycle.test.ts` +(unit-level, injected deps — cold start, warm start, stale-file cases, +`EPERM`-is-alive, socket_path mismatch, protocol mismatch, `--no-spawn`, +`RELAYFLOWD_BIN` non-executable refusal, detached-spawn options) and +`tests/daemon-lifecycle-live.test.ts` (real built-CLI subprocess tests against +a stub daemon: cold start spawns exactly one daemon and the daemon outlives +the CLI, bounded polling for a slow-listening daemon, attaching to a daemon +serving with no connection file, a second run attaching without spawning, a +stale connection file triggering a fresh spawn, and the §6-test-15 concurrency +case). + +**e2e-smoke** (workflow yaml's exact command, `testdata/hello-deterministic.flow.yaml`): + +``` +$ flows check --json testdata/hello-deterministic.flow.yaml --data-dir /tmp/flows-daemon-lifecycle-smoke +REFUSED [invalid_invocation] # expected: check refuses --data-dir, per design §4 ("flows check stays daemon-free") +{"ok":false,...,"kind":"invalid_invocation",...} + +$ flows run --json testdata/hello-deterministic.flow.yaml --data-dir /tmp/flows-daemon-lifecycle-smoke +WARNING [unprovable_effects] Step "greet" ... +WARNING [unprovable_effects] Step "shout" ... +{"ok":true,"command":"run",...,"status":"completed","completionReason":"success","completedSteps":2} +``` + +Cold start against an empty data dir: no daemon running beforehand, `flows +run` spawned one, attached, and completed — no manual `relayflowd serve` +step, which is exactly Phase 1's goal. + +## Adversarial review verdict: **REVIEW_FAILED (first pass) → closed** + +One confirmed, concrete violation on the first pass; fixed since (see the +update note at the top of this report) and re-verified by running the new +tests against the real binary. Everything else checked held up on the first +pass and was not re-litigated. + +**Violation — the flock/exit-3 property has zero test coverage against the +real kernel binary.** `kernel/DAEMON-LIFECYCLE.md` §6 lists six kernel tests +this spec "must be held to." Only tests 1, 2, and 3 exist +(`kernel/relayflowd/tests/daemon_lifecycle.rs`). Tests 4 ("a second `serve` on +a served data dir exits 3, and the first daemon's socket is still accepting +afterwards — the anti-hijack test") and 5 ("a SIGKILLed daemon's successor +starts cleanly") are absent. `grep` across `kernel/relayflowd/src` and +`kernel/relayflowd/tests` for `AlreadyServing`/`already serving` finds only +the two lines in `server.rs`/`lifecycle.rs` that implement the exit-3 path — +no test calls it. The SDK's own live test suite documents this gap in its own +comment: `packages/sdk/tests/fixtures/stub-relayflowd.mjs` fakes the mutex +with `open(O_CREAT|O_EXCL)` instead of a real `flock`, and says explicitly +*"the `flock` guarantee itself belongs to the kernel implementation and its +`cargo test` cases (DAEMON-LIFECYCLE.md §6 tests 4 and 5)."* Those cases were +never written. This is the design's own load-bearing claim — *"because the +daemon holds the mutex, the CLI never has to be clever. Spawning when in +doubt is always safe"* — asserted about the real `flock(2)` call in +`lifecycle.rs`, verified only by inspection, never by a test that actually +spawns two real `relayflowd serve` processes against the same data dir. The +implementation reads correct (`acquire()` is called before any +unlink/bind/publish, §1 steps 2→3 order is right in `server.rs`), but "reads +correct" is not the bar this design set for itself in its own §6. + +Everything else adversarially checked: + +1. **Two concurrent `flows` invocations, empty data dir** — correct by + design and exercised for real (real processes, real race window) at the + CLI level in `tests/daemon-lifecycle-live.test.ts`'s §6-test-15 case + (started=2, serving=1, lost=1, asserted every run). Not exercised at the + kernel level (see the violation above) because the stub does not use a + real `flock`. +2. **Stale `connection.json` causing a false attach** — not reproducible. + `checkDaemon` (`daemon-connection.ts`) always backs a file with a live + `hello` probe at the path recomputed from `--data-dir`; `isProcessAlive` + correctly treats `EPERM` as alive (the specific bug the design calls out + as present in `../relay` and refuses to port); a `socket_path` mismatch is + `stale`, not a redirect. +3. **Detached spawn survives the CLI exiting** — `spawnDaemon` + (`daemon-lifecycle.ts`) sets `detached: true`, ignores stdin/stdout, + routes stderr to `relayflowd.log`, and calls `child.unref()`; asserted by + a dedicated unit test (`daemon-lifecycle.test.ts:362`) and demonstrated + live by the cold-start e2e-smoke run above (daemon outlived the CLI + invocation). +4. **Manual `relayflowd serve` operator compatibility** — unchanged + observable behavior confirmed by reading `server.rs`'s diff: the same + unlink/bind sequence, now additionally gated by the lock and followed by + `connection.json` publication and signal cleanup, none of which an + existing manual workflow depends on the absence of. +5. **Publish strictly after listen** — confirmed both by code order + (`UnixListener::bind` completes, then `lifecycle::publish` is called, in + `server.rs`) and by a real-process test + (`connection_file_is_published_only_after_the_socket_is_live`, which + connects to whatever socket it observes and requires that connect to + succeed). + +## Phase 2 (not in this workflow's scope) + +Per `kernel/DAEMON-LIFECYCLE.md`'s own framing, this is Phase 1 (the +handshake) only. Phase 2 is per-platform `relayflowd`/`flows` binary +packages, mirroring the existing `packages/runtime-linux-x64` pattern, so +`resolveRelayflowdBinary`'s optional-dependency resolution step +(`relayflowd-path.ts` §3.1 step 3) has something real to find on a fresh +`npm install -g relayflows` on macOS and Windows, not just Linux. Until +Phase 2 ships, a fresh install on those platforms falls through to the +source-checkout and `PATH` resolution steps, which will not find anything on +a machine that never built the kernel from source — `relayflowd_not_found` +is the expected, correct refusal there today, not a bug. + +## Open risk + +- ~~The flock/exit-3 kernel property is unverified against the real binary~~ + — closed: `a_second_serve_on_a_served_data_dir_refuses_and_the_first_keeps_serving` + and `a_sigkilled_daemons_successor_starts_cleanly` now exercise it directly + against real `relayflowd serve` processes. +- The stub relayflowd's lock emulation (`link(2)` + pid-liveness reclaim) is + a second, independent implementation of "singleton mutex, reclaim on + death." It is documented as intentionally not faithful to `flock`'s + kernel-released-on-any-death semantics. If the two implementations ever + disagree on an edge case (e.g. a permissions error acquiring the lock file + itself), the SDK-level tests would not catch it. +- No test in either suite exercises `relayflowd serve` being started by hand + (as an operator would, per the backward-compatibility claim) concurrently + with a CLI-spawned attempt — only CLI-vs-CLI races are covered. +- This report and the code it describes are uncommitted on + `feat/daemon-lifecycle`; nothing here has been through code review or CI. diff --git a/packages/sdk/src/cli.ts b/packages/sdk/src/cli.ts index ef314dd11..c015182dc 100644 --- a/packages/sdk/src/cli.ts +++ b/packages/sdk/src/cli.ts @@ -11,6 +11,7 @@ import { resumeFlow, runFlow, type RunExecution, + type RunProgress, type RunReport, } from './cli/run.js'; import { runDirectFlow } from './cli/direct-run.js'; @@ -28,8 +29,8 @@ export interface CliIo { type CliExitCode = 0 | 1 | 2 | 3; type ParsedArgs = | { command: 'check'; json: boolean; value: string } - | { command: 'run'; dataDir: string; input: string | undefined; json: boolean; value: string } - | { command: 'resume'; dataDir: string; json: boolean; value: string } + | { command: 'run'; dataDir: string; input: string | undefined; json: boolean; spawn: boolean; value: string } + | { command: 'resume'; dataDir: string; json: boolean; spawn: boolean; value: string } | { command: 'hn-monitor'; sub: 'start'; dataDir: string; specPath: string; pollIntervalMs: number | undefined } | { command: 'tick'; sub: 'start'; dataDir: string; specPath: string; scheduleId: string; intervalMs: number; epochMs: number | undefined; maxCatchUp: number | undefined; @@ -39,13 +40,23 @@ const DEFAULT_DATA_DIR = '.relayflowd'; const USAGE = [ 'Usage:', 'flows check [--json] ', - 'flows run [--json] [--data-dir ] ', - 'flows run [--json] [--data-dir ] --input ', + 'flows run [--json] [--no-spawn] [--data-dir ] ', + 'flows run [--json] [--no-spawn] [--data-dir ] --input ', 'flows tick start --schedule-id --interval-ms [--epoch-ms ] [--max-catch-up ] [--poll-interval-ms ] [--data-dir ] ', - 'flows resume [--json] [--data-dir ] ', + 'flows resume [--json] [--no-spawn] [--data-dir ] ', 'flows hn-monitor start [--data-dir ] [--poll-interval-ms ] ', ].join(' '); +/** + * `FLOWS_NO_SPAWN=1` is `--no-spawn` for a whole environment: the lever for CI + * that means to assert a daemon is already present rather than conjure one + * (kernel/DAEMON-LIFECYCLE.md §4). Only the exact string `1` counts — an + * unset or empty variable must not be read as an opinion. + */ +function spawnAllowedByEnv(env: NodeJS.ProcessEnv = process.env): boolean { + return env['FLOWS_NO_SPAWN'] !== '1'; +} + const PROCESS_IO: CliIo = { stdout: (line) => process.stdout.write(`${line}\n`), stderr: (line) => process.stderr.write(`${line}\n`), @@ -63,6 +74,11 @@ export async function runCli( } if (parsed.command === 'check') { + // Deliberately daemon-free (kernel/DAEMON-LIFECYCLE.md §4). `checkFlow` is + // a pure compile-and-preflight that opens no socket, and the parser + // refuses `--data-dir` on `check`, so there is no data dir to attach to. + // `flows check` keeps working with no daemon, no relayflowd binary and no + // data directory at all -- a property worth keeping, not an omission. const checked = checkFlow(parsed.value); emitCheckReport(checked.report, parsed.json, io); return checked.report.ok ? 0 : 2; @@ -110,16 +126,19 @@ export async function runCli( } } + // Attach-or-spawn runs inside `runFlow`/`resumeFlow`/`runDirectFlow`, at the + // single `connect()` seam immediately before journal-client.ts is used -- + // not here. Hoisting it above the dispatch would start a daemon as a side + // effect of an invocation that is about to be refused for bad input. + const lifecycle = { + onWait: (progress: RunProgress) => emitWait(progress, io), + daemon: { spawn: parsed.spawn && spawnAllowedByEnv() }, + }; const execution = parsed.command === 'run' ? isAuthoredFlowPath(parsed.value) - ? await runDirectFlow( - parsed.value, - parsed.input, - parsed.dataDir, - { onWait: (progress) => emitWait(progress, io) }, - ) - : await runFlow(parsed.value, parsed.dataDir, { onWait: (progress) => emitWait(progress, io) }) - : await resumeFlow(parsed.value, parsed.dataDir, { onWait: (progress) => emitWait(progress, io) }); + ? await runDirectFlow(parsed.value, parsed.input, parsed.dataDir, lifecycle) + : await runFlow(parsed.value, parsed.dataDir, lifecycle) + : await resumeFlow(parsed.value, parsed.dataDir, lifecycle); emitRunReport(execution, parsed.json, io); return execution.exitCode; } @@ -143,6 +162,7 @@ function parseArgs(args: readonly string[]): ParsedArgs | undefined { let json = false; let dataDir = DEFAULT_DATA_DIR; let sawDataDir = false; + let spawn = true; let input: string | undefined; let sawInput = false; const positionals: string[] = []; @@ -153,6 +173,13 @@ function parseArgs(args: readonly string[]): ParsedArgs | undefined { json = true; continue; } + if (argument === '--no-spawn') { + // Refused on `check` for the same reason `--data-dir` is: `check` never + // opens a socket, so a daemon flag there would describe nothing. + if (command === 'check' || !spawn) return undefined; + spawn = false; + continue; + } if (argument === '--data-dir') { const value = args[index + 1]; if (command === 'check' || sawDataDir || value === undefined || value.startsWith('-')) return undefined; @@ -178,8 +205,8 @@ function parseArgs(args: readonly string[]): ParsedArgs | undefined { return command === 'check' ? { command, json, value: positionals[0]! } : command === 'run' - ? { command, dataDir, input, json, value: positionals[0]! } - : { command, dataDir, json, value: positionals[0]! }; + ? { command, dataDir, input, json, spawn, value: positionals[0]! } + : { command, dataDir, json, spawn, value: positionals[0]! }; } function parseHnMonitorArgs(rest: readonly string[]): ParsedArgs | undefined { diff --git a/packages/sdk/src/cli/daemon-refusal.ts b/packages/sdk/src/cli/daemon-refusal.ts new file mode 100644 index 000000000..19e0befa4 --- /dev/null +++ b/packages/sdk/src/cli/daemon-refusal.ts @@ -0,0 +1,49 @@ +// Turning a non-attached `DaemonState` into a `flows run` / `flows resume` +// diagnostic (kernel/DAEMON-LIFECYCLE.md §4). +// +// Its own file because run.ts is already at the size AGENTS.md rule 1 calls a +// design smell, and because the mapping is a pure function of the state: no +// I/O, no process, nothing to stub to test it. + +import { type DaemonState } from '../daemon-lifecycle.js'; +import { PROTOCOL_VERSION } from '../protocol.js'; +import type { RunDiagnostic } from './run.js'; + +/** + * Map a non-attached `DaemonState` onto the closed run taxonomy. Each of these + * is exit 2 — refused before a journal write — which is what docs/SURFACE.md + * §5 already promises for an unreachable daemon; only the `kind` and the + * message are new. + */ +export function daemonRefusal( + daemon: Exclude, + dataDir: string, + socketPath: string, +): RunDiagnostic { + if (daemon.kind === 'incompatible') { + return { + severity: 'refusal', + kind: 'daemon_protocol_mismatch', + message: `relayflowd at "${socketPath}" speaks journal protocol ${daemon.protocol}, not ${PROTOCOL_VERSION}. ` + + 'Refusing rather than starting a second daemon over a live incompatible one.', + }; + } + if (daemon.kind !== 'unavailable') { + // `absent` and `stale` are decisions to spawn, never terminal answers from + // `ensureDaemon`; reaching here would mean the algorithm returned mid-flight. + return { + severity: 'refusal', + kind: 'daemon_unreachable', + message: `No compatible relayflowd is listening at "${socketPath}". Start it with: relayflowd --data-dir ${JSON.stringify(dataDir)} serve`, + }; + } + if (daemon.failure === 'daemon_unreachable') { + // `--no-spawn` / FLOWS_NO_SPAWN=1: byte-for-byte today's refusal. + return { + severity: 'refusal', + kind: 'daemon_unreachable', + message: `No compatible relayflowd is listening at "${socketPath}". Start it with: relayflowd --data-dir ${JSON.stringify(dataDir)} serve`, + }; + } + return { severity: 'refusal', kind: daemon.failure, message: daemon.message }; +} diff --git a/packages/sdk/src/cli/direct-run.ts b/packages/sdk/src/cli/direct-run.ts index e349c7f95..056ec4919 100644 --- a/packages/sdk/src/cli/direct-run.ts +++ b/packages/sdk/src/cli/direct-run.ts @@ -21,7 +21,7 @@ export async function runDirectFlow( path: string, inputArgument: string | undefined, dataDir: string, - _options: RunLifecycleOptions = {}, + options: RunLifecycleOptions = {}, ): Promise { let input: unknown; try { @@ -40,7 +40,7 @@ export async function runDirectFlow( const socketPath = socketFor(dataDir); const base: RunReport = { ...emptyReport('run'), path }; const client = new JournalClient(socketPath); - const connected = await connect(client, 'run', dataDir, base); + const connected = await connect(client, 'run', dataDir, base, options); if (connected !== undefined) return connected; try { diff --git a/packages/sdk/src/cli/run.ts b/packages/sdk/src/cli/run.ts index d78c3ddb7..f899c9f90 100644 --- a/packages/sdk/src/cli/run.ts +++ b/packages/sdk/src/cli/run.ts @@ -1,6 +1,8 @@ import { join, resolve } from 'node:path'; import { toKernelSpec } from '../compile.js'; -import type { RunFailureKind } from '../failure-kinds.js'; +import { ensureDaemon, type EnsureDaemonOptions } from '../daemon-lifecycle.js'; +import { daemonRefusal } from './daemon-refusal.js'; +import type { RunFailureKind, RunWarningKind } from '../failure-kinds.js'; import { JournalClient, JournalProtocolError } from '../journal-client.js'; import type { PreflightDiagnostic } from '../preflight.js'; import type { @@ -24,8 +26,8 @@ export interface ParkedStep { } export interface RunDiagnostic { - severity: 'refusal' | 'failure' | 'parked'; - kind: RunFailureKind | RunCompletionReason; + severity: 'refusal' | 'failure' | 'parked' | 'warning'; + kind: RunFailureKind | RunWarningKind | RunCompletionReason; message: string; } @@ -59,6 +61,12 @@ export interface RunProgress { export interface RunLifecycleOptions { signal?: AbortSignal; onWait?: (progress: RunProgress) => void; + /** + * Attach-or-spawn policy for the daemon this command needs + * (kernel/DAEMON-LIFECYCLE.md §3). `{ spawn: false }` is `--no-spawn`: + * refuse instead of starting one, which is today's exact behavior. + */ + daemon?: EnsureDaemonOptions; } export async function runFlow( @@ -80,16 +88,19 @@ async function executeCheckedFlow( options: RunLifecycleOptions, ): Promise { const socketPath = socketFor(dataDir); + // Carry the preflight's diagnostics as a RunReport from here on, so the + // attach step has one accumulator to append to (see `connect`). + const base = fromCheckReport('run', checked.report); const client = new JournalClient(socketPath); - const connected = await connect(client, 'run', dataDir, checked.report); + const connected = await connect(client, 'run', dataDir, base, options); if (connected !== undefined) return connected; try { const spec = toKernelSpec(checked.flow!); const outcome = await client.runStart(spec); - return await classifyOutcome(client, 'run', outcome, checked.report, socketPath, options); + return await classifyOutcome(client, 'run', outcome, base, socketPath, options); } catch (error) { - return protocolFailure('run', checked.report, socketPath, error); + return protocolFailure('run', base, socketPath, error); } finally { client.close(); } @@ -103,7 +114,7 @@ export async function resumeFlow( const socketPath = socketFor(dataDir); const base = emptyReport('resume'); const client = new JournalClient(socketPath); - const connected = await connect(client, 'resume', dataDir, base); + const connected = await connect(client, 'resume', dataDir, base, options); if (connected !== undefined) return connected; try { @@ -131,13 +142,53 @@ export async function resumeFlow( } } +/** + * Get a live daemon, then open the socket to it. + * + * This is the single seam every journal-opening verb shares (`runFlow`, + * `resumeFlow`, `runDirectFlow`), and it is where attach-or-spawn belongs — + * *after* the command has compiled, preflighted and validated its input, and + * immediately before `JournalClient` is used. Hoisting it into `runCli` + * instead would make a malformed invocation start a daemon as a side effect, + * breaking the surface's promise that missing, invalid, and oversized input is + * refused before the CLI contacts relayflowd (docs/SURFACE.md §5). + * + * Everything past `ensureDaemon` is unchanged and still fails closed: a + * connect or `hello` that fails against a daemon we just attached to is a + * refusal, with no retry and no second spawn. + */ export async function connect( client: JournalClient, command: RunCommand, dataDir: string, - base: CheckReport | RunReport, + base: RunReport, + options: RunLifecycleOptions = {}, ): Promise { const socketPath = socketFor(dataDir); + const daemon = await ensureDaemon(dataDir, options.daemon ?? {}); + if (daemon.kind !== 'attached') { + client.close(); + return { + exitCode: 2, + report: { + ...fromBase(command, base), + socketPath, + diagnostics: [...base.diagnostics, daemonRefusal(daemon, dataDir, socketPath)], + }, + }; + } + if (daemon.warning !== undefined) { + // `base.diagnostics` is the accumulator that becomes the report's + // diagnostics, so a warning raised while attaching belongs in it — the + // attach succeeded, and silence about an anomaly is what AGENTS.md rule 4 + // forbids. + base.diagnostics.push({ + severity: 'warning', + kind: 'connection_file_stale', + message: daemon.warning, + }); + } + try { await client.connect(); } catch { @@ -396,7 +447,9 @@ export function fromCheckReport(command: RunCommand, report: CheckReport): RunRe ...(report.path !== undefined ? { path: report.path } : {}), ...(report.projectConfigPath !== undefined ? { projectConfigPath: report.projectConfigPath } : {}), resolutions: report.resolutions, - diagnostics: report.diagnostics, + // Copied, not aliased: the returned report is an accumulator the attach + // step appends to, and it must not write back into the check report. + diagnostics: [...report.diagnostics], }; } diff --git a/packages/sdk/src/daemon-connection.ts b/packages/sdk/src/daemon-connection.ts new file mode 100644 index 000000000..b1be64606 --- /dev/null +++ b/packages/sdk/src/daemon-connection.ts @@ -0,0 +1,336 @@ +// Reading and validating relayflowd's connection file, and deciding whether +// anything is actually serving a data dir (kernel/DAEMON-LIFECYCLE.md §§1-2). +// +// Split from daemon-lifecycle.ts along the same seam ../relay uses between +// broker-connection.ts and broker-lifecycle.ts: this file answers "what is +// there?", that one answers "put something there". Neither approaches the +// 500-line smell in AGENTS.md rule 1. +// +// The governing rule, from §1: **the socket is the authority; +// `connection.json` is an index over it.** A file is never believed on its +// own. Every "attach" in this module is backed by a `hello` that a live +// daemon answered on the socket path recomputed from `--data-dir` — never by +// what a file says about itself. + +import { spawn, type ChildProcess, type SpawnOptions } from 'node:child_process'; +import { + closeSync, + mkdirSync, + openSync, + readFileSync, + rmSync, +} from 'node:fs'; +import { join, resolve } from 'node:path'; +import { JournalClient, JournalProtocolError } from './journal-client.js'; +import { PROTOCOL_VERSION } from './protocol.js'; +import { + defaultRelayflowdPathDeps, + resolveRelayflowdBinary, +} from './relayflowd-path.js'; + +/** + * The `hello` probe runs before every command, so it must be short. §4 gives + * `connect()` the same bound for the same reason: a listener that accepts and + * never answers must not hold the CLI for the 30s request default. + */ +export const PROBE_TIMEOUT_MS = 2_000; + +export const CONNECTION_FILE = 'connection.json'; +export const SOCKET_FILE = 'relayflowd.sock'; +export const DAEMON_LOG_FILE = 'relayflowd.log'; + +/** `/connection.json`, exactly the shape in §1. */ +export interface DaemonConnection { + socket_path: string; + pid: number; + version: string; + protocol: number; + started_at_ms: number; +} + +/** Why an existing `connection.json` was not believed (§2's four cases). */ +export type StaleReason = + /** §2 step 3: the file names a socket that is not this data dir's. */ + | 'socket_path_mismatch' + /** §2 row 1: nothing serving, and the pid is gone. A corpse. */ + | 'dead_pid_dead_socket' + /** §2 row 3: pid alive but nothing answers — mid-boot, or a reused pid. */ + | 'live_pid_dead_socket'; + +/** Closed refusal taxonomy for the lifecycle itself. Mirrored in failure-kinds.ts. */ +export type DaemonFailureKind = + | 'daemon_unreachable' + | 'relayflowd_not_found' + | 'daemon_start_failed' + | 'daemon_start_timeout'; + +export type DaemonState = + | { kind: 'attached'; socketPath: string; connection: DaemonConnection | null; warning?: string } + | { kind: 'absent' } + | { kind: 'stale'; reason: StaleReason; message: string } + | { kind: 'incompatible'; protocol: number } + /** + * `ensureDaemon` only. §4's sketch types `ensureDaemon` as returning a + * `DaemonState` while §3 branches D.b/D.d refuse with kinds that union has + * no member for; this is that refusal channel, kept as data rather than a + * thrown error so it reads like the rest of cli/run.ts. + */ + | { kind: 'unavailable'; failure: DaemonFailureKind; message: string }; + +/** A probe of the socket itself — the only check that proves something serves. */ +export interface SocketProbe { + reachable: boolean; + /** + * The protocol the daemon reported. Absent when something is demonstrably + * serving the socket but would not say — see `probeSocket`. + */ + protocol?: number; +} + +/** + * Injectable seams, following the `deps` pattern of the ../relay file this is + * modelled on. Unit tests reach no real filesystem and spawn no process. + */ +export interface DaemonLifecycleDeps { + readFile(path: string): string | null; + removeFile(path: string): void; + makeDirectory(path: string): void; + openAppend(path: string): number; + closeFd(fd: number): void; + readTail(path: string, bytes: number): string; + killProcess(pid: number, signal: number): void; + spawnProcess(command: string, args: readonly string[], options: SpawnOptions): ChildProcess; + resolveBinary(): string; + probe(socketPath: string, timeoutMs: number): Promise; + now(): number; + sleep(ms: number): Promise; +} + +export const defaultDaemonLifecycleDeps: DaemonLifecycleDeps = { + readFile(path) { + try { + return readFileSync(path, 'utf8'); + } catch { + return null; + } + }, + removeFile(path) { + rmSync(path, { force: true }); + }, + makeDirectory(path) { + mkdirSync(path, { recursive: true }); + }, + openAppend: (path) => openSync(path, 'a'), + closeFd: (fd) => closeSync(fd), + readTail(path, bytes) { + try { + const content = readFileSync(path, 'utf8'); + return content.length <= bytes ? content : content.slice(-bytes); + } catch { + return ''; + } + }, + killProcess: (pid, signal) => { + process.kill(pid, signal); + }, + spawnProcess: (command, args, options) => spawn(command, args as string[], options), + resolveBinary: () => resolveRelayflowdBinary(defaultRelayflowdPathDeps), + probe: (socketPath, timeoutMs) => probeSocket(socketPath, timeoutMs), + now: () => Date.now(), + // Deliberately NOT unref'd: an unref'd timer lets Node exit the event loop + // mid-poll, which would abandon the await and end the CLI with no report. + // The deadline in `pollForDaemon` is what bounds this wait. + sleep: (ms) => new Promise((done) => { + setTimeout(done, ms); + }), +}; + +export function socketPathFor(dataDir: string): string { + return join(resolve(dataDir), SOCKET_FILE); +} + +export function connectionPathFor(dataDir: string): string { + return join(resolve(dataDir), CONNECTION_FILE); +} + +/** + * §2 steps 1-2. Absent, unparseable, and wrongly-typed all read the same: + * `null`. Unknown fields are ignored, so adding one later is not breaking. + * The `socket_path` agreement check is §2 step 3 and lives in `checkDaemon`, + * which knows the data dir the path must agree with. + */ +export function readConnectionFile( + dataDir: string, + deps: DaemonLifecycleDeps = defaultDaemonLifecycleDeps, +): DaemonConnection | null { + const raw = deps.readFile(connectionPathFor(dataDir)); + if (raw === null) return null; + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return null; + } + if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) return null; + const record = parsed as Record; + const socketPath = record['socket_path']; + const pid = record['pid']; + const version = record['version']; + const protocol = record['protocol']; + const startedAtMs = record['started_at_ms']; + if (typeof socketPath !== 'string' || socketPath.length === 0) return null; + if (typeof pid !== 'number' || !Number.isInteger(pid) || pid <= 0) return null; + if (typeof version !== 'string') return null; + if (typeof protocol !== 'number' || !Number.isInteger(protocol)) return null; + if (typeof startedAtMs !== 'number' || !Number.isInteger(startedAtMs) || startedAtMs <= 0) { + return null; + } + return { + socket_path: socketPath, + pid, + version, + protocol, + started_at_ms: startedAtMs, + }; +} + +/** + * §2 step 4. **`EPERM` means alive** — the process exists and is owned by + * another user. ../relay's `isProcessRunning` treats every throw as dead, + * which would spawn a second daemon over a live one owned by someone else. + * That bug is not ported. + */ +export function isProcessAlive( + pid: number, + deps: DaemonLifecycleDeps = defaultDaemonLifecycleDeps, +): boolean { + if (!Number.isInteger(pid) || pid <= 0) return false; + try { + deps.killProcess(pid, 0); + return true; + } catch (error) { + return (error as NodeJS.ErrnoException).code === 'EPERM'; + } +} + +/** + * §2 step 5. Connect, `hello`, close. A refused connection, a connect timeout, + * or a `hello` that does not answer inside the probe timeout is a failed + * probe. This is the only check that proves something is *serving*. + * + * A `hello` that answers with a structured REFUSAL is not a failed probe. §2 + * step 5 was written against silence, and silence is what it must catch; + * something that returns `{ ok: false, error: ... }` is unmistakably a live + * server, and calling it absent would spawn a second daemon over it — the one + * outcome this whole design exists to prevent. It is reported as reachable + * with no protocol, which leaves the refusal to be reported by the command's + * own `hello`, where it becomes a `protocol_error` naming the server's code + * instead of a lifecycle guess. + */ +export async function probeSocket( + socketPath: string, + timeoutMs: number = PROBE_TIMEOUT_MS, +): Promise { + const client = new JournalClient(socketPath, { + connectTimeoutMs: timeoutMs, + requestTimeoutMs: timeoutMs, + }); + try { + await client.connect(); + const hello = await client.hello('flows-probe'); + return { reachable: true, protocol: hello.protocol }; + } catch (error) { + return error instanceof JournalProtocolError ? { reachable: true } : { reachable: false }; + } finally { + client.close(); + } +} + +/** + * §2, decided as one function. The socket is probed at the path recomputed + * from `--data-dir`, never at the path the file names, so a file that points + * elsewhere is stale rather than a redirect. + * + * One addition to §2's table, licensed by §1's governing rule: a socket that + * answers `hello` with **no** connection file at all is an attach, not an + * absence. §2 row 2 already attaches on the socket's authority over a file + * describing a dead pid; a missing file is strictly less evidence against the + * socket than a stale one. It also keeps the CLI honest about a daemon started + * from a build that predates the connection file. + */ +export async function checkDaemon( + dataDir: string, + deps: DaemonLifecycleDeps = defaultDaemonLifecycleDeps, +): Promise { + const socketPath = socketPathFor(dataDir); + const connection = readConnectionFile(dataDir, deps); + const agrees = connection !== null && connection.socket_path === socketPath; + + // §2's reason for carrying `protocol` in the file: an incompatible daemon + // owning this data dir is decided without a round trip. + if (agrees && connection.protocol !== PROTOCOL_VERSION) { + return { kind: 'incompatible', protocol: connection.protocol }; + } + + const probe = await deps.probe(socketPath, PROBE_TIMEOUT_MS); + if (probe.reachable) { + // Only a KNOWN mismatch refuses. An undefined protocol means the daemon + // is serving but declined to say, which the command's own `hello` will + // report far better than this function could. + if (probe.protocol !== undefined && probe.protocol !== PROTOCOL_VERSION) { + return { kind: 'incompatible', protocol: probe.protocol }; + } + if (!agrees) { + // No warning when the file is merely ABSENT: that is the ordinary state + // during a daemon's boot window, and the ordinary state of any daemon + // built before the connection file existed. A file that CONTRADICTS the + // socket is a different matter and is named. + const mismatch = connection === null + ? undefined + : `"${connectionPathFor(dataDir)}" names a different socket (${connection.socket_path}); attaching to "${socketPath}" instead.`; + return { + kind: 'attached', + socketPath, + connection: null, + ...(mismatch === undefined ? {} : { warning: mismatch }), + }; + } + // §2 row 2: dead pid, live socket. Something is serving; do not spawn. + // Binding over a live daemon is the corruption this design prevents. + const warning = isProcessAlive(connection.pid, deps) + ? undefined + : `${CONNECTION_FILE} names pid ${connection.pid}, which is not running, but "${socketPath}" is serving. Attaching to the socket.`; + return { + kind: 'attached', + socketPath, + connection, + ...(warning === undefined ? {} : { warning }), + }; + } + + // Nothing is serving. + if (connection === null) return { kind: 'absent' }; + if (!agrees) { + return { + kind: 'stale', + reason: 'socket_path_mismatch', + message: `${CONNECTION_FILE} names socket "${connection.socket_path}", not "${socketPath}".`, + }; + } + // §2 row 3: a live pid proves nothing — a daemon mid-boot and an unrelated + // process that inherited the pid look identical from here, and neither is + // an attach. Spawning is safe in both cases because the daemon holds the + // mutex (§3): a redundant child loses the flock and exits 3. + return isProcessAlive(connection.pid, deps) + ? { + kind: 'stale', + reason: 'live_pid_dead_socket', + message: `pid ${connection.pid} is alive but "${socketPath}" is not accepting connections.`, + } + : { + kind: 'stale', + reason: 'dead_pid_dead_socket', + message: `pid ${connection.pid} is gone and "${socketPath}" is not accepting connections.`, + }; +} + diff --git a/packages/sdk/src/daemon-lifecycle.ts b/packages/sdk/src/daemon-lifecycle.ts new file mode 100644 index 000000000..7c36995d2 --- /dev/null +++ b/packages/sdk/src/daemon-lifecycle.ts @@ -0,0 +1,198 @@ +// Attach-or-spawn for relayflowd (kernel/DAEMON-LIFECYCLE.md §3). +// +// Lifecycle is not transport, so none of this lives in journal-client.ts: +// putting spawn logic in the client would make every AgentWorker, tick runner +// and demo conjure daemons as a side effect of connecting. The client stays +// fail-closed with no retry and no spawn; this module only decides how the +// socket gets there. + +import type { ChildProcess, SpawnOptions } from 'node:child_process'; +import { join, resolve } from 'node:path'; +import { + checkDaemon, + connectionPathFor, + DAEMON_LOG_FILE, + defaultDaemonLifecycleDeps, + socketPathFor, + type DaemonLifecycleDeps, + type DaemonState, +} from './daemon-connection.js'; +import { RelayflowdNotFoundError } from './relayflowd-path.js'; + +export * from './daemon-connection.js'; + +/** `relayflowd serve` lost the singleton lock — someone else is serving (§3). */ +export const EXIT_ALREADY_SERVING = 3; + +/** §3: matches ../relay's DETACHED_START_READY_TIMEOUT_MS. */ +export const DEFAULT_START_TIMEOUT_MS = 10_000; +/** §3: matches the existing poll in scripts/run-local-workflow.mjs:64-71. */ +export const START_POLL_INTERVAL_MS = 50; +/** How much of relayflowd.log a `daemon_start_failed` refusal quotes. */ +const LOG_TAIL_BYTES = 4_096; + +export interface EnsureDaemonOptions { + /** `false` restores today's fail-closed behavior (`--no-spawn`). */ + spawn?: boolean; + timeoutMs?: number; +} + +/** + * §3's CLI algorithm. Attach if something is serving; otherwise spawn + * `relayflowd serve` detached and poll for it, bounded. + * + * The CLI never has to be clever here, and that is the load-bearing + * simplification: because the singleton mutex lives in the daemon + * (`flock` on `/relayflowd.lock`, §3), spawning when in doubt is + * always safe. A child that loses the race exits `EXIT_ALREADY_SERVING` + * having unlinked nothing, bound nothing and written nothing, and this loop + * keeps polling until the winner publishes. + * + * The CLI never terminates a daemon — not one it found, and not one it + * spawned. + */ +export async function ensureDaemon( + dataDir: string, + options: EnsureDaemonOptions = {}, + deps: DaemonLifecycleDeps = defaultDaemonLifecycleDeps, +): Promise { + const resolvedDataDir = resolve(dataDir); + const maySpawn = options.spawn ?? true; + const timeoutMs = options.timeoutMs ?? DEFAULT_START_TIMEOUT_MS; + + const first = await checkDaemon(resolvedDataDir, deps); + if (first.kind === 'attached' || first.kind === 'incompatible') return first; + if (!maySpawn) { + return { + kind: 'unavailable', + failure: 'daemon_unreachable', + message: first.kind === 'stale' ? first.message : 'no connection file and no listening socket', + }; + } + + // Only the corpse is removed, and only when nothing is serving and its pid + // is gone (§2 row 1). The other stale verdicts leave the file alone on + // purpose: a daemon mid-boot may be about to publish, and unlinking the + // winner's file would strand this poll loop. The booting daemon sweeps + // leftovers under the lock anyway (§1 step 3), which is the only place the + // removal is provably safe. + if (first.kind === 'stale' && first.reason === 'dead_pid_dead_socket') { + deps.removeFile(connectionPathFor(resolvedDataDir)); + } + + let binary: string; + try { + binary = deps.resolveBinary(); + } catch (error) { + return { + kind: 'unavailable', + failure: 'relayflowd_not_found', + message: error instanceof RelayflowdNotFoundError || error instanceof Error + ? error.message + : 'relayflowd could not be located.', + }; + } + + deps.makeDirectory(resolvedDataDir); + const logPath = join(resolvedDataDir, DAEMON_LOG_FILE); + const child = spawnDaemon(binary, resolvedDataDir, logPath, deps); + return pollForDaemon(resolvedDataDir, child, logPath, timeoutMs, deps); +} + +interface SpawnedDaemon { + exit: { code: number | null; signal: NodeJS.Signals | null } | undefined; + error: Error | undefined; +} + +/** + * `detached: true` puts the child in its own session and process group. Both + * consequences are required: it outlives this CLI, and a Ctrl-C sent to the + * CLI's process group does not reach it. + * + * stdio never inherits the CLI's. A daemon holding the CLI's stdout would + * interleave its own output with `flows run --json`'s single report object, + * and would hold the pipe open after the CLI exits, hanging anything reading + * it. stderr goes to `/relayflowd.log` so a failed start has + * evidence to quote. + */ +function spawnDaemon( + binary: string, + dataDir: string, + logPath: string, + deps: DaemonLifecycleDeps, +): SpawnedDaemon { + const state: SpawnedDaemon = { exit: undefined, error: undefined }; + const log = deps.openAppend(logPath); + try { + const child = deps.spawnProcess(binary, ['--data-dir', dataDir, 'serve'], { + detached: true, + stdio: ['ignore', 'ignore', log], + env: process.env, + cwd: process.cwd(), + }); + child.once('error', (error: Error) => { + state.error = error; + }); + child.once('exit', (code: number | null, signal: NodeJS.Signals | null) => { + state.exit = { code, signal }; + }); + child.unref(); + } finally { + // The child holds its own duplicate of this descriptor; keeping the + // parent's copy open would leak one per invocation. + deps.closeFd(log); + } + return state; +} + +async function pollForDaemon( + dataDir: string, + child: SpawnedDaemon, + logPath: string, + timeoutMs: number, + deps: DaemonLifecycleDeps, +): Promise { + const deadline = deps.now() + timeoutMs; + for (;;) { + if (child.error !== undefined) { + return { + kind: 'unavailable', + failure: 'daemon_start_failed', + message: `relayflowd could not be started: ${child.error.message}`, + }; + } + // §3 branch D.a: exit 3 means this child lost a benign race for the data + // dir's lock. The winner is coming up. Keep polling; do not respawn, and + // do not report failure. + if ( + child.exit !== undefined + && child.exit.code !== null + && child.exit.code !== 0 + && child.exit.code !== EXIT_ALREADY_SERVING + ) { + return { + kind: 'unavailable', + failure: 'daemon_start_failed', + message: `relayflowd exited ${child.exit.code} during startup.${logTail(logPath, deps)}`, + }; + } + + const state = await checkDaemon(dataDir, deps); + if (state.kind === 'attached' || state.kind === 'incompatible') return state; + + if (deps.now() >= deadline) { + return { + kind: 'unavailable', + failure: 'daemon_start_timeout', + message: `relayflowd did not start serving "${socketPathFor(dataDir)}" within ${timeoutMs}ms.` + + logTail(logPath, deps), + }; + } + await deps.sleep(START_POLL_INTERVAL_MS); + } +} + +function logTail(logPath: string, deps: DaemonLifecycleDeps): string { + const tail = deps.readTail(logPath, LOG_TAIL_BYTES).trim(); + return tail.length === 0 ? ` See "${logPath}".` : ` Last output in "${logPath}":\n${tail}`; +} diff --git a/packages/sdk/src/failure-kinds.ts b/packages/sdk/src/failure-kinds.ts index 896208afa..6eec36b84 100644 --- a/packages/sdk/src/failure-kinds.ts +++ b/packages/sdk/src/failure-kinds.ts @@ -52,18 +52,42 @@ export const PREFLIGHT_WARNING_KINDS = [ 'vacuous_gate', ] as const; -/** Closed outcome taxonomy owned by the `flows run` / `flows resume` surface. */ +/** + * Closed outcome taxonomy owned by the `flows run` / `flows resume` surface. + * + * The four daemon-lifecycle kinds after `daemon_unreachable` are the + * attach-or-spawn refusals from kernel/DAEMON-LIFECYCLE.md §3. They split what + * used to be one message: `daemon_unreachable` now means only "nothing is + * serving and this invocation was told not to start one" (`--no-spawn`), while + * a spawn that was attempted and did not produce a serving daemon names which + * step failed. All of them are still exit 2 — refused before a journal write. + */ export const RUN_FAILURE_KINDS = [ 'daemon_unreachable', + 'daemon_protocol_mismatch', + 'daemon_start_failed', + 'daemon_start_timeout', + 'relayflowd_not_found', 'protocol_error', 'run_parked', 'run_unavailable', ] as const; +/** + * Non-refusing outcomes of the attach step. `connection_file_stale` is + * DAEMON-LIFECYCLE.md §2 row 2: the socket answered while `connection.json` + * described a process that is gone. The socket is the authority, so this + * attaches — but it says so rather than passing in silence. + */ +export const RUN_WARNING_KINDS = [ + 'connection_file_stale', +] as const; + export type PreflightFailureKind = (typeof PREFLIGHT_FAILURE_KINDS)[number]; export type CheckFailureKind = (typeof CHECK_FAILURE_KINDS)[number]; export type PreflightWarningKind = (typeof PREFLIGHT_WARNING_KINDS)[number]; export type RunFailureKind = (typeof RUN_FAILURE_KINDS)[number]; +export type RunWarningKind = (typeof RUN_WARNING_KINDS)[number]; const CHECK_FAILURE_KIND_SET: ReadonlySet = new Set(CHECK_FAILURE_KINDS); const RUN_FAILURE_KIND_SET: ReadonlySet = new Set(RUN_FAILURE_KINDS); diff --git a/packages/sdk/src/journal-client.ts b/packages/sdk/src/journal-client.ts index eb26f2a06..d31c33db3 100644 --- a/packages/sdk/src/journal-client.ts +++ b/packages/sdk/src/journal-client.ts @@ -27,6 +27,17 @@ import type { KernelRunSpec, StepType } from './spec.js'; export interface JournalClientOptions { /** Override the timeout for bounded protocol requests (ms). Default 30000. */ requestTimeoutMs?: number; + /** + * Bound on `connect()` (ms). Default 2000. + * + * `connect()` used to have no timer at all, which was survivable while the + * only caller was a command that had already decided a daemon was there. + * `daemon-lifecycle.ts` probes the socket before every command, and a + * listener that accepts but never answers would otherwise hold the CLI for + * the 30s request default before it could decide to spawn + * (kernel/DAEMON-LIFECYCLE.md §4). + */ + connectTimeoutMs?: number; } interface Pending { @@ -51,6 +62,7 @@ export class JournalClient extends EventEmitter { private buffer = ''; private readonly pending = new Map(); private readonly requestTimeoutMs: number; + private readonly connectTimeoutMs: number; constructor( private readonly socketPath: string, @@ -58,6 +70,7 @@ export class JournalClient extends EventEmitter { ) { super(); this.requestTimeoutMs = options.requestTimeoutMs ?? 30_000; + this.connectTimeoutMs = options.connectTimeoutMs ?? 2_000; } /** Open the unix socket connection. Rejects on connect failure (fail-closed). */ @@ -65,13 +78,21 @@ export class JournalClient extends EventEmitter { return new Promise((resolve, reject) => { if (this.socket) return resolve(); const socket = createConnection({ path: this.socketPath }); + const timer = setTimeout(() => { + socket.removeAllListeners(); + socket.destroy(); + this.failAll(new Error(`journal client: connect timed out after ${this.connectTimeoutMs}ms`)); + reject(new Error(`journal client: connect timed out after ${this.connectTimeoutMs}ms`)); + }, this.connectTimeoutMs); const onError = (err: Error): void => { + clearTimeout(timer); socket.removeAllListeners(); this.failAll(err); reject(new Error(`journal client: connect failed: ${err.message}`)); }; socket.once('error', onError); socket.once('connect', () => { + clearTimeout(timer); socket.removeListener('error', onError); socket.on('error', (err) => this.failAll(err)); socket.on('data', (chunk) => this.onData(chunk)); diff --git a/packages/sdk/src/relayflowd-path.ts b/packages/sdk/src/relayflowd-path.ts new file mode 100644 index 000000000..f5d4ec5f4 --- /dev/null +++ b/packages/sdk/src/relayflowd-path.ts @@ -0,0 +1,190 @@ +// Resolving the `relayflowd` binary (kernel/DAEMON-LIFECYCLE.md §3.1). +// +// Never PATH alone. A version manager (nvm, volta, asdf, mise) can launch Node +// with a minimal PATH, and that is exactly the case where a `which` lookup +// fails to see the binary the installer placed next to its own launcher. The +// multi-anchor order below is ported from ../relay's +// packages/harness-driver/src/broker-path.ts for that reason. + +import { createRequire } from 'node:module'; +import { accessSync, constants, existsSync, realpathSync, statSync } from 'node:fs'; +import { spawnSync } from 'node:child_process'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +/** How far up from cwd a source checkout is searched for `kernel/Cargo.toml`. */ +const SOURCE_CHECKOUT_MAX_DEPTH = 8; + +/** Refusal from {@link resolveRelayflowdBinary}; carries every anchor tried. */ +export class RelayflowdNotFoundError extends Error { + readonly attempts: readonly string[]; + + constructor(message: string, attempts: readonly string[]) { + super(message); + this.name = 'RelayflowdNotFoundError'; + this.attempts = attempts; + } +} + +/** Injectable seams. Unit tests reach no real filesystem and spawn nothing. */ +export interface RelayflowdPathDeps { + env: NodeJS.ProcessEnv; + /** `process.argv[1]` — the running `flows` entrypoint, possibly a symlink. */ + entrypoint: string | undefined; + cwd: string; + /** This module's own directory, the third `createRequire` anchor. */ + moduleDir: string; + platform: string; + arch: string; + isExecutable(path: string): boolean; + exists(path: string): boolean; + realpath(path: string): string; + /** `require.resolve(specifier)` from `anchor`, or null when unresolvable. */ + resolveFrom(specifier: string, anchor: string): string | null; + /** `which`/`where` lookup, or null. */ + which(command: string): string | null; +} + +export const defaultRelayflowdPathDeps: RelayflowdPathDeps = { + env: process.env, + entrypoint: process.argv[1], + cwd: process.cwd(), + moduleDir: dirname(fileURLToPath(import.meta.url)), + platform: process.platform, + arch: process.arch, + isExecutable(path) { + try { + accessSync(path, constants.X_OK); + return statSync(path).isFile(); + } catch { + return false; + } + }, + exists: (path) => existsSync(path), + realpath(path) { + try { + return realpathSync(path); + } catch { + return path; + } + }, + resolveFrom(specifier, anchor) { + try { + return createRequire(anchor).resolve(specifier); + } catch { + return null; + } + }, + which(command) { + const finder = process.platform === 'win32' ? 'where' : 'which'; + const found = spawnSync(finder, [command], { encoding: 'utf8' }); + if (found.status !== 0 || typeof found.stdout !== 'string') return null; + const first = found.stdout.split('\n')[0]?.trim(); + return first === undefined || first.length === 0 ? null : first; + }, +}; + +/** + * The published runtime package for this host, e.g. + * `@relayflows/runtime-linux-x64` (see packages/runtime-linux-x64). + */ +export function runtimePackageName(platform: string, arch: string): string { + return `@relayflows/runtime-${platform}-${arch}`; +} + +/** + * Locate `relayflowd`, or refuse. + * + * Order is §3.1's, and the first anchor is a hard stop rather than a + * preference: an operator who exported `RELAYFLOWD_BIN` meant it, so a value + * that is not executable refuses instead of quietly falling through to PATH. + * Silent fallback is what AGENTS.md rule 4 forbids. + */ +export function resolveRelayflowdBinary( + deps: RelayflowdPathDeps = defaultRelayflowdPathDeps, +): string { + const attempts: string[] = []; + + const override = deps.env['RELAYFLOWD_BIN']; + if (override !== undefined && override.length > 0) { + if (deps.isExecutable(override)) return override; + throw new RelayflowdNotFoundError( + `RELAYFLOWD_BIN is set to "${override}", which is not an executable file. ` + + 'Point it at a relayflowd binary or unset it; it is not ignored.', + [`RELAYFLOWD_BIN=${override}`], + ); + } + + for (const candidate of [ + () => siblingOfEntrypoint(deps), + () => runtimePackageBinary(deps), + () => sourceCheckoutBinary(deps), + () => deps.which('relayflowd'), + ]) { + const found = candidate(); + if (found === null) continue; + attempts.push(found); + if (deps.isExecutable(found)) return found; + } + + const runtime = runtimePackageName(deps.platform, deps.arch); + throw new RelayflowdNotFoundError( + `No relayflowd binary could be found. Install the runtime package for this host ` + + `(${runtime}), or set RELAYFLOWD_BIN to a relayflowd executable.` + + (attempts.length === 0 ? '' : ` Tried: ${attempts.join(', ')}.`), + attempts, + ); +} + +/** + * The published layout: `bin/flows` and `bin/relayflowd` side by side. + * `realpath` matters — a package-manager launcher exposes the entrypoint as a + * symlink whose target sits in the real install tree, and the sibling is next + * to the target, not next to the link. + */ +function siblingOfEntrypoint(deps: RelayflowdPathDeps): string | null { + if (deps.entrypoint === undefined || deps.entrypoint.length === 0) return null; + return join(dirname(deps.realpath(deps.entrypoint)), 'relayflowd'); +} + +/** + * The optional-dependency package, tried from several `createRequire` anchors. + * Several because a globally installed `flows` resolving a per-project optional + * dependency sits outside the consumer's `node_modules` — broker-path.ts:84-106 + * verbatim in its reasoning. + */ +function runtimePackageBinary(deps: RelayflowdPathDeps): string | null { + const specifier = `${runtimePackageName(deps.platform, deps.arch)}/package.json`; + const anchors = [ + deps.moduleDir.endsWith('/') ? deps.moduleDir : `${deps.moduleDir}/`, + ...(deps.entrypoint === undefined ? [] : [deps.entrypoint]), + join(deps.cwd, 'package.json'), + ]; + for (const anchor of anchors) { + const manifest = deps.resolveFrom(specifier, anchor); + if (manifest === null) continue; + return join(dirname(manifest), 'bin', 'relayflowd'); + } + return null; +} + +/** + * A source checkout: the nearest ancestor of cwd holding `kernel/Cargo.toml`, + * then release before debug. Bounded, so a deep cwd cannot walk to `/`. + */ +function sourceCheckoutBinary(deps: RelayflowdPathDeps): string | null { + let directory = resolve(deps.cwd); + for (let depth = 0; depth < SOURCE_CHECKOUT_MAX_DEPTH; depth += 1) { + if (deps.exists(join(directory, 'kernel', 'Cargo.toml'))) { + for (const profile of ['release', 'debug']) { + const candidate = join(directory, 'kernel', 'target', profile, 'relayflowd'); + if (deps.isExecutable(candidate)) return candidate; + } + return null; + } + const parent = dirname(directory); + if (parent === directory) return null; + directory = parent; + } + return null; +} diff --git a/packages/sdk/tests/daemon-lifecycle-live.test.ts b/packages/sdk/tests/daemon-lifecycle-live.test.ts new file mode 100644 index 000000000..443c6fbca --- /dev/null +++ b/packages/sdk/tests/daemon-lifecycle-live.test.ts @@ -0,0 +1,327 @@ +// The attach-or-spawn cases that need real processes +// (kernel/DAEMON-LIFECYCLE.md §6 tests 7, 15, and the detachment half of 16). +// +// These drive the BUILT `flows` CLI as a subprocess against a stub relayflowd +// (tests/fixtures/stub-relayflowd.mjs) reached through `RELAYFLOWD_BIN`. A +// stub rather than the kernel binary because the property under test is the +// CLI's: whether it spawns once, whether the daemon outlives it, whether two +// concurrent invocations end with one socket owner. The daemon-side guarantees +// the stub imitates — the `flock`, the publish-after-listen ordering, the +// signal handlers — are the kernel implementation's to prove in `cargo test` +// (DAEMON-LIFECYCLE.md §6 tests 1-6). + +import { spawn, spawnSync, type SpawnSyncReturns } from 'node:child_process'; +import { + chmodSync, + existsSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { afterEach, beforeAll, describe, expect, it } from 'vitest'; +import type { DaemonConnection } from '../src/daemon-lifecycle.js'; + +const HERE = dirname(fileURLToPath(import.meta.url)); +const ROOT = join(HERE, '..', '..', '..'); +const BUILT_CLI = join(ROOT, 'packages', 'sdk', 'dist', 'cli.js'); +const FLOW = join(ROOT, 'testdata', 'hello-deterministic.flow.yaml'); +const STUB = join(HERE, 'fixtures', 'stub-relayflowd.mjs'); + +const temporaryDirectories: string[] = []; +const startedPids: number[] = []; + +beforeAll(() => { + if (!existsSync(BUILT_CLI)) { + throw new Error(`missing built CLI at ${BUILT_CLI}; run \`npm run build\` in packages/sdk`); + } +}); + +afterEach(() => { + for (const pid of startedPids.splice(0)) { + try { + process.kill(pid, 'SIGKILL'); + } catch { + // Already gone; that is the expected state for most cases. + } + } + for (const directory of temporaryDirectories.splice(0)) { + rmSync(directory, { recursive: true, force: true }); + } +}); + +function workspace(prefix: string): string { + const directory = mkdtempSync(join(tmpdir(), prefix)); + temporaryDirectories.push(directory); + return directory; +} + +/** + * `RELAYFLOWD_BIN` must name an executable (§3.1 anchor 1), so the stub is + * reached through a one-line launcher rather than being invoked as `node + * stub.mjs`. + */ +function stubLauncher(directory: string): string { + const launcher = join(directory, 'relayflowd'); + writeFileSync(launcher, `#!/bin/sh\nexec ${JSON.stringify(process.execPath)} ${JSON.stringify(STUB)} "$@"\n`); + chmodSync(launcher, 0o755); + return launcher; +} + +function cliEnv(overrides: NodeJS.ProcessEnv): NodeJS.ProcessEnv { + return { ...process.env, ...overrides }; +} + +function invokeCli(args: readonly string[], env: NodeJS.ProcessEnv): SpawnSyncReturns { + return spawnSync(process.execPath, [BUILT_CLI, ...args], { + cwd: ROOT, + encoding: 'utf8', + env, + timeout: 60_000, + }); +} + +function invokeCliAsync( + args: readonly string[], + env: NodeJS.ProcessEnv, +): Promise<{ status: number | null; stdout: string; stderr: string }> { + return new Promise((done) => { + const child = spawn(process.execPath, [BUILT_CLI, ...args], { cwd: ROOT, env }); + let stdout = ''; + let stderr = ''; + child.stdout.on('data', (chunk: Buffer) => { + stdout += chunk.toString('utf8'); + }); + child.stderr.on('data', (chunk: Buffer) => { + stderr += chunk.toString('utf8'); + }); + child.on('close', (status) => done({ status, stdout, stderr })); + }); +} + +function connectionFile(dataDir: string): DaemonConnection { + return JSON.parse(readFileSync(join(dataDir, 'connection.json'), 'utf8')) as DaemonConnection; +} + +/** The pid the stub daemon recorded in `/relayflowd.lock`. */ +function lockHolder(dataDir: string): number { + return Number.parseInt(readFileSync(join(dataDir, 'relayflowd.lock'), 'utf8').trim(), 10); +} + +function isAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch (error) { + return (error as NodeJS.ErrnoException).code === 'EPERM'; + } +} + +function traceLines(tracePath: string, verb: string): string[] { + if (!existsSync(tracePath)) return []; + return readFileSync(tracePath, 'utf8') + .split('\n') + .filter((line) => line.startsWith(`${verb} `)); +} + +describe('flows run against a data dir with no daemon (§6 test 7)', () => { + it('cold start spawns exactly one daemon, the run succeeds, and the daemon outlives the CLI', () => { + const directory = workspace('flows-cold-start-'); + const dataDir = join(directory, 'data'); + const trace = join(directory, 'trace.log'); + + const result = invokeCli(['run', '--data-dir', dataDir, FLOW], cliEnv({ + RELAYFLOWD_BIN: stubLauncher(directory), + STUB_TRACE: trace, + })); + + expect(result.status, result.stderr).toBe(0); + expect(result.stdout).toContain('completionReason: success'); + expect(traceLines(trace, 'start')).toHaveLength(1); + expect(traceLines(trace, 'serving')).toHaveLength(1); + + // Detached, not merely backgrounded: the CLI has already exited, and the + // daemon it started is still serving. + const connection = connectionFile(dataDir); + startedPids.push(connection.pid); + expect(connection.socket_path).toBe(join(dataDir, 'relayflowd.sock')); + expect(isAlive(connection.pid)).toBe(true); + }); + + it('polls, bounded, for a daemon that holds the lock before it binds', () => { + const directory = workspace('flows-slow-listen-'); + const dataDir = join(directory, 'data'); + const trace = join(directory, 'trace.log'); + + const started = Date.now(); + const result = invokeCli(['run', '--data-dir', dataDir, FLOW], cliEnv({ + RELAYFLOWD_BIN: stubLauncher(directory), + STUB_TRACE: trace, + STUB_LISTEN_DELAY_MS: '900', + })); + + expect(result.status, result.stderr).toBe(0); + // It waited rather than refusing, and it waited by polling rather than by + // spawning again. + expect(Date.now() - started).toBeGreaterThanOrEqual(900); + expect(traceLines(trace, 'start')).toHaveLength(1); + startedPids.push(connectionFile(dataDir).pid); + }); + + // The governing rule made observable: a daemon that is serving but has not + // published its index yet is attached to on the socket's authority. This is + // also what lets `flows run` work against a relayflowd built before the + // connection file existed. + it('attaches to a serving daemon that has not published a connection file', () => { + const directory = workspace('flows-no-index-'); + const dataDir = join(directory, 'data'); + + const result = invokeCli(['run', '--data-dir', dataDir, FLOW], cliEnv({ + RELAYFLOWD_BIN: stubLauncher(directory), + STUB_NO_CONNECTION_FILE: '1', + })); + + expect(result.status, result.stderr).toBe(0); + expect(result.stdout).toContain('completionReason: success'); + expect(existsSync(join(dataDir, 'connection.json'))).toBe(false); + expect(existsSync(join(dataDir, 'relayflowd.sock'))).toBe(true); + startedPids.push(lockHolder(dataDir)); + }); + + it('a second run attaches to the daemon the first one started, spawning nothing', () => { + const directory = workspace('flows-warm-start-'); + const dataDir = join(directory, 'data'); + const trace = join(directory, 'trace.log'); + const env = cliEnv({ RELAYFLOWD_BIN: stubLauncher(directory), STUB_TRACE: trace }); + + expect(invokeCli(['run', '--data-dir', dataDir, FLOW], env).status).toBe(0); + const first = connectionFile(dataDir); + startedPids.push(first.pid); + + const second = invokeCli(['run', '--data-dir', dataDir, FLOW], env); + + expect(second.status, second.stderr).toBe(0); + // Warm start: not one extra process was launched, let alone one that bound. + expect(traceLines(trace, 'start')).toHaveLength(1); + expect(connectionFile(dataDir).pid).toBe(first.pid); + }); + + it('detects a stale connection file left by a hard kill and starts a fresh daemon', () => { + const directory = workspace('flows-stale-file-'); + const dataDir = join(directory, 'data'); + const env = cliEnv({ RELAYFLOWD_BIN: stubLauncher(directory) }); + + expect(invokeCli(['run', '--data-dir', dataDir, FLOW], env).status).toBe(0); + const killed = connectionFile(dataDir); + + // SIGKILL runs no handler, so the file and the socket inode both survive + // their daemon — by construction (§1). That residue is what §2 detects. + process.kill(killed.pid, 'SIGKILL'); + waitUntilDead(killed.pid); + expect(existsSync(join(dataDir, 'connection.json'))).toBe(true); + expect(connectionFile(dataDir).pid).toBe(killed.pid); + + const second = invokeCli(['run', '--data-dir', dataDir, FLOW], env); + + expect(second.status, second.stderr).toBe(0); + const fresh = connectionFile(dataDir); + startedPids.push(fresh.pid); + expect(fresh.pid).not.toBe(killed.pid); + expect(isAlive(fresh.pid)).toBe(true); + }); +}); + +describe('concurrent invocations against one empty data dir (§6 test 15)', () => { + it('ends with exactly one daemon owning the socket, and both runs succeed', async () => { + const directory = workspace('flows-race-'); + const dataDir = join(directory, 'data'); + const trace = join(directory, 'trace.log'); + // The listen delay is what makes the race real rather than lucky: nothing + // is serving and nothing is published for 600ms, so the second CLI's first + // `checkDaemon` still sees an empty data dir and both decide to spawn. + // The daemons then genuinely contend for the data dir's lock. + const env = cliEnv({ + RELAYFLOWD_BIN: stubLauncher(directory), + STUB_TRACE: trace, + STUB_LISTEN_DELAY_MS: '600', + }); + + const [first, second] = await Promise.all([ + invokeCliAsync(['run', '--data-dir', dataDir, FLOW], env), + invokeCliAsync(['run', '--data-dir', dataDir, FLOW], env), + ]); + + expect(first.status, first.stderr).toBe(0); + expect(second.status, second.stderr).toBe(0); + + // The property: however many daemons were launched, exactly one ever bound + // the socket, and every other one exited 3 without touching anything. + const started = traceLines(trace, 'start'); + const serving = traceLines(trace, 'serving'); + const lost = traceLines(trace, 'lost'); + // Both CLIs really did spawn: without this the rest of the case could pass + // vacuously on a run where only one of them ever tried. + expect(started).toHaveLength(2); + expect(serving).toHaveLength(1); + expect(lost).toHaveLength(1); + + const connection = connectionFile(dataDir); + startedPids.push(connection.pid); + expect(serving[0]).toBe(`serving ${connection.pid}`); + expect(isAlive(connection.pid)).toBe(true); + }, 60_000); +}); + +describe('refusals from a spawn that cannot produce a daemon', () => { + it('names relayflowd_not_found rather than falling through to PATH', () => { + const directory = workspace('flows-no-binary-'); + + const result = invokeCli(['run', '--data-dir', join(directory, 'data'), FLOW], cliEnv({ + RELAYFLOWD_BIN: join(directory, 'not-executable'), + })); + + expect(result.status).toBe(2); + expect(result.stderr).toContain('REFUSED [relayflowd_not_found]'); + expect(result.stderr).toContain('RELAYFLOWD_BIN'); + }); + + it('names daemon_start_failed and quotes the daemon log when startup dies', () => { + const directory = workspace('flows-start-failed-'); + const dataDir = join(directory, 'data'); + + const result = invokeCli(['run', '--data-dir', dataDir, FLOW], cliEnv({ + RELAYFLOWD_BIN: stubLauncher(directory), + STUB_STARTUP_EXIT: '1', + })); + + expect(result.status).toBe(2); + expect(result.stderr).toContain('REFUSED [daemon_start_failed]'); + expect(result.stderr).toContain('STUB_STARTUP_EXIT'); + expect(readFileSync(join(dataDir, 'relayflowd.log'), 'utf8')).toContain('refusing to start'); + }); + + it('refuses a daemon speaking another protocol version instead of binding over it', () => { + const directory = workspace('flows-protocol-'); + const dataDir = join(directory, 'data'); + const env = cliEnv({ RELAYFLOWD_BIN: stubLauncher(directory), STUB_PROTOCOL: '77' }); + + const result = invokeCli(['run', '--data-dir', dataDir, FLOW], env); + + expect(result.status).toBe(2); + expect(result.stderr).toContain('REFUSED [daemon_protocol_mismatch]'); + if (existsSync(join(dataDir, 'connection.json'))) { + startedPids.push(connectionFile(dataDir).pid); + } + }); +}); + +/** A synchronous wait, so the assertion after it observes a settled world. */ +function waitUntilDead(pid: number): void { + const deadline = Date.now() + 5_000; + const idle = new Int32Array(new SharedArrayBuffer(4)); + while (isAlive(pid) && Date.now() < deadline) Atomics.wait(idle, 0, 0, 20); + if (isAlive(pid)) throw new Error(`pid ${pid} did not exit`); +} diff --git a/packages/sdk/tests/daemon-lifecycle.test.ts b/packages/sdk/tests/daemon-lifecycle.test.ts new file mode 100644 index 000000000..61e58ef40 --- /dev/null +++ b/packages/sdk/tests/daemon-lifecycle.test.ts @@ -0,0 +1,533 @@ +// Unit cases for kernel/DAEMON-LIFECYCLE.md §§2-3, driven entirely through the +// injected `deps` seam: no file is read, no socket is opened, no process is +// spawned. The cases that need real processes — cold start, detachment, and +// the concurrent-invocation race — live in daemon-lifecycle-live.test.ts, +// because their property is enforced by the OS and cannot be stubbed. + +import { EventEmitter } from 'node:events'; +import type { ChildProcess, SpawnOptions } from 'node:child_process'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { afterEach, describe, expect, it } from 'vitest'; +import { + checkDaemon, + connectionPathFor, + DEFAULT_START_TIMEOUT_MS, + ensureDaemon, + EXIT_ALREADY_SERVING, + isProcessAlive, + readConnectionFile, + socketPathFor, + type DaemonLifecycleDeps, +} from '../src/daemon-lifecycle.js'; +import { RelayflowdNotFoundError } from '../src/relayflowd-path.js'; +import { PROTOCOL_VERSION } from '../src/protocol.js'; +import { runCli, type CliIo } from '../src/cli.js'; + +const DATA_DIR = '/tmp/flows-daemon-lifecycle-unit'; +const SOCKET = socketPathFor(DATA_DIR); +const CONNECTION = connectionPathFor(DATA_DIR); +const BINARY = '/opt/relayflows/bin/relayflowd'; + +interface SpawnRecord { + command: string; + args: readonly string[]; + options: SpawnOptions; + unrefCalled: boolean; + child: EventEmitter; +} + +interface World { + files: Map; + alive: Set; + eperm: Set; + /** socket path -> protocol version answered by `hello`. */ + serving: Map; + spawns: SpawnRecord[]; + ticks: number; + time: number; + resolveBinary: () => string; + onSpawn?: (record: SpawnRecord, world: World) => void; + onTick?: (tick: number, world: World) => void; +} + +function makeWorld(overrides: Partial = {}): World { + return { + files: new Map(), + alive: new Set(), + eperm: new Set(), + serving: new Map(), + spawns: [], + ticks: 0, + time: 1_000, + resolveBinary: () => BINARY, + ...overrides, + }; +} + +function makeDeps(world: World): DaemonLifecycleDeps { + return { + readFile: (path) => world.files.get(path) ?? null, + removeFile: (path) => { + world.files.delete(path); + }, + makeDirectory: () => {}, + openAppend: () => 7, + closeFd: () => {}, + readTail: (path, bytes) => (world.files.get(path) ?? '').slice(-bytes), + killProcess: (pid) => { + if (world.eperm.has(pid)) throw Object.assign(new Error('EPERM'), { code: 'EPERM' }); + if (!world.alive.has(pid)) throw Object.assign(new Error('ESRCH'), { code: 'ESRCH' }); + }, + spawnProcess: (command, args, options) => { + const child = new EventEmitter() as EventEmitter & { unref(): void }; + const record: SpawnRecord = { command, args, options, unrefCalled: false, child }; + child.unref = (): void => { + record.unrefCalled = true; + }; + world.spawns.push(record); + // Deferred, like a real spawn: `child.on('error'|'exit')` is registered + // by the caller only after `spawn()` returns, so a fake that emitted + // synchronously would drop the event (and throw on an unhandled + // 'error') rather than exercising the branch under test. + queueMicrotask(() => world.onSpawn?.(record, world)); + return child as unknown as ChildProcess; + }, + resolveBinary: () => world.resolveBinary(), + probe: (socketPath) => Promise.resolve( + world.serving.has(socketPath) + ? { reachable: true, protocol: world.serving.get(socketPath)! } + : { reachable: false }, + ), + now: () => world.time, + sleep: (ms) => { + world.time += ms; + world.ticks += 1; + world.onTick?.(world.ticks, world); + return Promise.resolve(); + }, + }; +} + +function publish( + world: World, + overrides: Partial> = {}, + dataDir = DATA_DIR, +): void { + world.files.set(connectionPathFor(dataDir), JSON.stringify({ + socket_path: socketPathFor(dataDir), + pid: 4242, + version: '0.1.0', + protocol: PROTOCOL_VERSION, + started_at_ms: 1_757_308_800_123, + ...overrides, + })); +} + +describe('readConnectionFile (DAEMON-LIFECYCLE.md §1, §2 steps 1-2)', () => { + it('round-trips the documented shape and ignores unknown fields', () => { + const world = makeWorld(); + publish(world, { future_field: 'ignored' }); + expect(readConnectionFile(DATA_DIR, makeDeps(world))).toEqual({ + socket_path: SOCKET, + pid: 4242, + version: '0.1.0', + protocol: PROTOCOL_VERSION, + started_at_ms: 1_757_308_800_123, + }); + }); + + it.each([ + ['absent', undefined], + ['not JSON', '{'], + ['an array', '[]'], + ['missing socket_path', JSON.stringify({ pid: 1, version: '0', protocol: 0, started_at_ms: 1 })], + ['an empty socket_path', JSON.stringify({ socket_path: '', pid: 1, version: '0', protocol: 0, started_at_ms: 1 })], + ['pid 0', JSON.stringify({ socket_path: SOCKET, pid: 0, version: '0', protocol: 0, started_at_ms: 1 })], + ['a fractional pid', JSON.stringify({ socket_path: SOCKET, pid: 1.5, version: '0', protocol: 0, started_at_ms: 1 })], + ['a string protocol', JSON.stringify({ socket_path: SOCKET, pid: 1, version: '0', protocol: '0', started_at_ms: 1 })], + ['started_at_ms 0', JSON.stringify({ socket_path: SOCKET, pid: 1, version: '0', protocol: 0, started_at_ms: 0 })], + ])('reads %s as no file at all', (_label, content) => { + const world = makeWorld(); + if (content !== undefined) world.files.set(CONNECTION, content); + expect(readConnectionFile(DATA_DIR, makeDeps(world))).toBeNull(); + }); +}); + +describe('isProcessAlive (§2 step 4)', () => { + it('is true for a running pid and false for a reaped one', () => { + const world = makeWorld({ alive: new Set([11]) }); + expect(isProcessAlive(11, makeDeps(world))).toBe(true); + expect(isProcessAlive(12, makeDeps(world))).toBe(false); + }); + + // ../relay's isProcessRunning treats every throw as dead. Porting that would + // spawn a second daemon over a live one owned by another user. + it('is true on EPERM — the process exists, it is just another user\'s', () => { + const world = makeWorld({ eperm: new Set([13]) }); + expect(isProcessAlive(13, makeDeps(world))).toBe(true); + }); + + it('refuses a nonsense pid without asking the OS', () => { + const world = makeWorld(); + expect(isProcessAlive(0, makeDeps(world))).toBe(false); + expect(isProcessAlive(-1, makeDeps(world))).toBe(false); + }); +}); + +describe('checkDaemon — the four cases of §2', () => { + it('attaches when the pid is alive and the socket answers', async () => { + const world = makeWorld({ alive: new Set([4242]) }); + publish(world); + world.serving.set(SOCKET, PROTOCOL_VERSION); + const state = await checkDaemon(DATA_DIR, makeDeps(world)); + expect(state).toMatchObject({ kind: 'attached', socketPath: SOCKET }); + expect(state.kind === 'attached' && state.warning).toBeUndefined(); + }); + + // Row 2. The socket is the authority: something is serving, so attaching is + // right and spawning would bind over a live daemon. + it('attaches with a warning when the pid is dead but the socket answers', async () => { + const world = makeWorld(); + publish(world); + world.serving.set(SOCKET, PROTOCOL_VERSION); + const state = await checkDaemon(DATA_DIR, makeDeps(world)); + expect(state.kind).toBe('attached'); + expect(state.kind === 'attached' && state.warning).toContain('4242'); + }); + + // Row 3, the pid-reuse case: a live pid proves nothing. + it('does not attach when the pid is alive but nothing answers the socket', async () => { + const world = makeWorld({ alive: new Set([4242]) }); + publish(world); + expect(await checkDaemon(DATA_DIR, makeDeps(world))) + .toMatchObject({ kind: 'stale', reason: 'live_pid_dead_socket' }); + }); + + it('calls a dead pid with a dead socket a corpse', async () => { + const world = makeWorld(); + publish(world); + expect(await checkDaemon(DATA_DIR, makeDeps(world))) + .toMatchObject({ kind: 'stale', reason: 'dead_pid_dead_socket' }); + }); + + // §2 step 3: a file whose socket_path points elsewhere is stale, not a + // redirect. Accepting it is how a foreign artifact gets adopted. + it('refuses a file naming a socket that is not this data dir\'s', async () => { + const world = makeWorld({ alive: new Set([4242]) }); + publish(world, { socket_path: '/somewhere/else/relayflowd.sock' }); + world.serving.set('/somewhere/else/relayflowd.sock', PROTOCOL_VERSION); + expect(await checkDaemon(DATA_DIR, makeDeps(world))) + .toMatchObject({ kind: 'stale', reason: 'socket_path_mismatch' }); + }); + + it('names the disagreement when this data dir IS served and the file points elsewhere', async () => { + const world = makeWorld({ alive: new Set([4242]) }); + publish(world, { socket_path: '/somewhere/else/relayflowd.sock' }); + world.serving.set(SOCKET, PROTOCOL_VERSION); + const state = await checkDaemon(DATA_DIR, makeDeps(world)); + expect(state).toMatchObject({ kind: 'attached', socketPath: SOCKET, connection: null }); + expect(state.kind === 'attached' && state.warning).toContain('/somewhere/else/relayflowd.sock'); + }); + + it('reports an incompatible protocol from the file without a round trip', async () => { + const world = makeWorld({ alive: new Set([4242]) }); + publish(world, { protocol: PROTOCOL_VERSION + 7 }); + const state = await checkDaemon(DATA_DIR, makeDeps(world)); + expect(state).toEqual({ kind: 'incompatible', protocol: PROTOCOL_VERSION + 7 }); + // No probe was needed: the file carries `protocol` precisely so a warm + // start does not pay a round trip to learn about a mismatch. + expect(world.serving.size).toBe(0); + }); + + it('reports an incompatible protocol answered by a live socket', async () => { + const world = makeWorld(); + world.serving.set(SOCKET, PROTOCOL_VERSION + 1); + expect(await checkDaemon(DATA_DIR, makeDeps(world))) + .toEqual({ kind: 'incompatible', protocol: PROTOCOL_VERSION + 1 }); + }); + + it('is absent with no file and no listener', async () => { + expect(await checkDaemon(DATA_DIR, makeDeps(makeWorld()))).toEqual({ kind: 'absent' }); + }); + + // §1's governing rule taken to its conclusion: a missing index is less + // evidence against a serving socket than a stale one, and row 2 already + // attaches over a stale one. Silently, because a missing file is the + // ordinary state during boot and for any daemon built before it existed. + it('attaches, without warning, to a serving socket that has published no connection file', async () => { + const world = makeWorld(); + world.serving.set(SOCKET, PROTOCOL_VERSION); + const state = await checkDaemon(DATA_DIR, makeDeps(world)); + expect(state).toEqual({ kind: 'attached', socketPath: SOCKET, connection: null }); + }); + + // A live server that answers `hello` with a structured refusal is still a + // live server. Calling it absent would spawn a second daemon over it. + it('attaches to a socket whose hello refuses, leaving the refusal to the command', async () => { + const world = makeWorld(); + const deps = { ...makeDeps(world), probe: () => Promise.resolve({ reachable: true }) }; + const state = await checkDaemon(DATA_DIR, deps); + expect(state).toMatchObject({ kind: 'attached', socketPath: SOCKET }); + }); + + it('does not spawn over a socket whose hello refuses', async () => { + const world = makeWorld(); + const deps = { ...makeDeps(world), probe: () => Promise.resolve({ reachable: true }) }; + expect(await ensureDaemon(DATA_DIR, {}, deps)).toMatchObject({ kind: 'attached' }); + expect(world.spawns).toHaveLength(0); + }); +}); + +describe('ensureDaemon — attach or spawn (§3)', () => { + // Test 8. + it('warm start attaches with ZERO spawns', async () => { + const world = makeWorld({ alive: new Set([4242]) }); + publish(world); + world.serving.set(SOCKET, PROTOCOL_VERSION); + + const state = await ensureDaemon(DATA_DIR, {}, makeDeps(world)); + + expect(state.kind).toBe('attached'); + expect(world.spawns).toHaveLength(0); + }); + + // Test 9. + it('cold start spawns exactly one daemon and attaches once it publishes', async () => { + const world = makeWorld(); + world.onSpawn = (_record, w) => { + // The daemon binds, then publishes. Two polls later, both are true. + w.onTick = (tick, inner) => { + if (tick !== 2) return; + inner.serving.set(SOCKET, PROTOCOL_VERSION); + inner.alive.add(9001); + publish(inner, { pid: 9001 }); + }; + }; + + const state = await ensureDaemon(DATA_DIR, {}, makeDeps(world)); + + expect(state).toMatchObject({ kind: 'attached', socketPath: SOCKET }); + expect(world.spawns).toHaveLength(1); + }); + + it('removes the corpse before spawning over a dead pid and dead socket', async () => { + const world = makeWorld(); + publish(world, { pid: 4242 }); + world.onSpawn = (_record, w) => { + w.onTick = (tick, inner) => { + if (tick !== 1) return; + inner.serving.set(SOCKET, PROTOCOL_VERSION); + inner.alive.add(9002); + publish(inner, { pid: 9002 }); + }; + }; + + const state = await ensureDaemon(DATA_DIR, {}, makeDeps(world)); + + expect(world.spawns).toHaveLength(1); + expect(state).toMatchObject({ kind: 'attached' }); + expect(readConnectionFile(DATA_DIR, makeDeps(world))?.pid).toBe(9002); + }); + + // The file of a daemon that may be mid-boot is NOT removed: unlinking the + // winner's file would strand this poll loop with nothing to find. + it('leaves the connection file alone when the pid is alive', async () => { + const world = makeWorld({ alive: new Set([4242]) }); + publish(world); + world.onSpawn = (_record, w) => { + w.onTick = (tick, inner) => { + if (tick === 1) inner.serving.set(SOCKET, PROTOCOL_VERSION); + }; + }; + + await ensureDaemon(DATA_DIR, {}, makeDeps(world)); + + expect(world.files.has(CONNECTION)).toBe(true); + }); + + // Test 14: never start a second daemon over a live incompatible one. + it('refuses a protocol mismatch without spawning', async () => { + const world = makeWorld({ alive: new Set([4242]) }); + publish(world, { protocol: 99 }); + + expect(await ensureDaemon(DATA_DIR, {}, makeDeps(world))) + .toEqual({ kind: 'incompatible', protocol: 99 }); + expect(world.spawns).toHaveLength(0); + }); + + // Test 16. + it('spawns detached, with stdio that never inherits the CLI\'s, and unrefs', async () => { + const world = makeWorld(); + world.onSpawn = (_record, w) => { + w.onTick = (_tick, inner) => inner.serving.set(SOCKET, PROTOCOL_VERSION); + }; + + await ensureDaemon(DATA_DIR, {}, makeDeps(world)); + + const spawned = world.spawns[0]!; + expect(spawned.command).toBe(BINARY); + expect(spawned.args).toEqual(['--data-dir', DATA_DIR, 'serve']); + expect(spawned.options.detached).toBe(true); + const stdio = spawned.options.stdio as unknown[]; + expect(stdio[0]).toBe('ignore'); + expect(stdio[1]).toBe('ignore'); + expect(stdio[2]).toBe(7); // the appended /relayflowd.log fd + expect(spawned.unrefCalled).toBe(true); + }); + + // Test 15's CLI half, in isolation: branch D.a. The process-level half is in + // daemon-lifecycle-live.test.ts, where the race is real. + it('keeps polling — and does not respawn — when its child loses the lock', async () => { + const world = makeWorld(); + world.onSpawn = (record, w) => { + // The child loses the flock and exits 3 immediately, having touched + // nothing. The winner publishes two polls later. + record.child.emit('exit', EXIT_ALREADY_SERVING, null); + w.onTick = (tick, inner) => { + if (tick !== 2) return; + inner.serving.set(SOCKET, PROTOCOL_VERSION); + inner.alive.add(5150); + publish(inner, { pid: 5150 }); + }; + }; + + const state = await ensureDaemon(DATA_DIR, {}, makeDeps(world)); + + expect(state).toMatchObject({ kind: 'attached' }); + expect(world.spawns).toHaveLength(1); + }); + + it('refuses when the child dies for any other reason, quoting the log', async () => { + const world = makeWorld(); + world.files.set(join(DATA_DIR, 'relayflowd.log'), 'bind socket: Address already in use\n'); + world.onSpawn = (record) => record.child.emit('exit', 1, null); + + const state = await ensureDaemon(DATA_DIR, {}, makeDeps(world)); + + expect(state).toMatchObject({ kind: 'unavailable', failure: 'daemon_start_failed' }); + expect(state.kind === 'unavailable' && state.message).toContain('Address already in use'); + }); + + it('refuses when the binary cannot be executed at all', async () => { + const world = makeWorld(); + world.onSpawn = (record) => record.child.emit('error', new Error('spawn ENOENT')); + + expect(await ensureDaemon(DATA_DIR, {}, makeDeps(world))) + .toMatchObject({ kind: 'unavailable', failure: 'daemon_start_failed' }); + }); + + it('refuses, bounded, when the daemon never begins serving', async () => { + const world = makeWorld(); + + const state = await ensureDaemon(DATA_DIR, {}, makeDeps(world)); + + expect(state).toMatchObject({ kind: 'unavailable', failure: 'daemon_start_timeout' }); + // Bounded by the deadline, not by luck: 10s of 50ms polls. + expect(world.ticks).toBe(DEFAULT_START_TIMEOUT_MS / 50); + }); + + it('refuses without spawning when no relayflowd binary can be found', async () => { + const world = makeWorld({ + resolveBinary: () => { + throw new RelayflowdNotFoundError('no relayflowd anywhere', []); + }, + }); + + expect(await ensureDaemon(DATA_DIR, {}, makeDeps(world))) + .toMatchObject({ kind: 'unavailable', failure: 'relayflowd_not_found' }); + expect(world.spawns).toHaveLength(0); + }); + + it('spawn: false refuses immediately and starts nothing', async () => { + const world = makeWorld(); + + expect(await ensureDaemon(DATA_DIR, { spawn: false }, makeDeps(world))) + .toMatchObject({ kind: 'unavailable', failure: 'daemon_unreachable' }); + expect(world.spawns).toHaveLength(0); + expect(world.ticks).toBe(0); + }); + + it('spawn: false still attaches to a daemon that is already serving', async () => { + const world = makeWorld({ alive: new Set([4242]) }); + publish(world); + world.serving.set(SOCKET, PROTOCOL_VERSION); + + expect(await ensureDaemon(DATA_DIR, { spawn: false }, makeDeps(world))) + .toMatchObject({ kind: 'attached' }); + }); +}); + +// Test 17: the lever that restores today's behavior exactly. +describe('--no-spawn and FLOWS_NO_SPAWN reproduce the old refusal', () => { + const temporaryDirectories: string[] = []; + afterEach(() => { + delete process.env['FLOWS_NO_SPAWN']; + for (const directory of temporaryDirectories.splice(0)) { + rmSync(directory, { recursive: true, force: true }); + } + }); + + function capture(): { io: CliIo; stdout: string[]; stderr: string[] } { + const stdout: string[] = []; + const stderr: string[] = []; + return { + io: { stdout: (line) => stdout.push(line), stderr: (line) => stderr.push(line) }, + stdout, + stderr, + }; + } + + function absentDataDir(): string { + const directory = mkdtempSync(join(tmpdir(), 'flows-no-spawn-')); + temporaryDirectories.push(directory); + return join(directory, 'absent'); + } + + const FLOW = join( + dirname(fileURLToPath(import.meta.url)), + '..', '..', '..', 'testdata', 'hello-deterministic.flow.yaml', + ); + + it('--no-spawn refuses with the byte-for-byte daemon_unreachable message', async () => { + const dataDir = absentDataDir(); + const output = capture(); + + const code = await runCli(['run', '--no-spawn', '--data-dir', dataDir, FLOW], output.io); + + expect(code).toBe(2); + // Byte for byte the message this branch emitted before attach-or-spawn + // existed; the preflight warnings ahead of it are unchanged too. + expect(output.stderr.at(-1)).toBe( + `REFUSED [daemon_unreachable] No compatible relayflowd is listening at "${socketPathFor(dataDir)}". ` + + `Start it with: relayflowd --data-dir ${JSON.stringify(dataDir)} serve`, + ); + }); + + it('FLOWS_NO_SPAWN=1 does the same for a whole environment', async () => { + process.env['FLOWS_NO_SPAWN'] = '1'; + const dataDir = absentDataDir(); + const output = capture(); + + const code = await runCli(['resume', '--data-dir', dataDir, '01JABCDEFGHJKMNPQRSTVWXYZ0'], output.io); + + expect(code).toBe(2); + expect(output.stderr.join('\n')).toContain('REFUSED [daemon_unreachable]'); + }); + + it('refuses --no-spawn on check, which never opens a socket', async () => { + const output = capture(); + const code = await runCli(['check', '--no-spawn', FLOW], output.io); + expect(code).toBe(2); + expect(output.stderr.join('\n')).toContain('REFUSED [invalid_invocation]'); + }); + + it('refuses a repeated --no-spawn', async () => { + const output = capture(); + const code = await runCli(['run', '--no-spawn', '--no-spawn', FLOW], output.io); + expect(code).toBe(2); + expect(output.stderr.join('\n')).toContain('REFUSED [invalid_invocation]'); + }); +}); diff --git a/packages/sdk/tests/direct-input.test.ts b/packages/sdk/tests/direct-input.test.ts index 90f0b17c9..0ccc0475e 100644 --- a/packages/sdk/tests/direct-input.test.ts +++ b/packages/sdk/tests/direct-input.test.ts @@ -103,11 +103,15 @@ describe('direct .flow.ts input through the built CLI and live runtime', () => { expect(missingInputValue.stderr).toContain('REFUSED [invalid_invocation]'); }); + // `--no-spawn` keeps this case about the property it names. `flows run` now + // starts a daemon when none is serving (kernel/DAEMON-LIFECYCLE.md §3), so + // without the flag the refusal under test would be about the spawn rather + // than about the authored module never being imported. it('does not import or execute authored code before daemon availability', () => { const directory = temporaryDirectory(); const marker = join(directory, 'marker.txt'); const result = invokeCli([ - 'run', SIDE_EFFECT_FLOW, '--input', JSON.stringify({ marker }), + 'run', '--no-spawn', SIDE_EFFECT_FLOW, '--input', JSON.stringify({ marker }), '--data-dir', join(directory, 'absent-daemon'), ], { RELAYFLOWS_TEST_IMPORT_MARKER: marker }); diff --git a/packages/sdk/tests/fixtures/stub-relayflowd.mjs b/packages/sdk/tests/fixtures/stub-relayflowd.mjs new file mode 100644 index 000000000..def83e100 --- /dev/null +++ b/packages/sdk/tests/fixtures/stub-relayflowd.mjs @@ -0,0 +1,198 @@ +#!/usr/bin/env node +// A stand-in for `relayflowd serve`, implementing the daemon half of +// kernel/DAEMON-LIFECYCLE.md §§1-3 that the CLI can observe. +// +// It exists because the CLI-side properties under test are about REAL +// PROCESSES: whether a detached child outlives its parent, whether two +// concurrent invocations end up with one socket owner, whether a losing child +// exits 3 and its CLI keeps polling. None of that can be faked with an +// injected `spawnProcess` seam. It is a stub only in that it serves a +// four-verb journal protocol instead of a journal. +// +// The one place it is NOT a faithful stub: §3 puts the singleton mutex in an +// `flock(2)` the kernel releases on any death, including SIGKILL. Node cannot +// call `flock`, so this uses `open(O_CREAT|O_EXCL)`, which is equally atomic +// across processes — exactly one caller wins a race — but is not released by +// the OS, so a dead holder is reclaimed by a pid check. That difference is +// invisible to the CLI, which only ever sees "the loser exited 3". The +// `flock` guarantee itself belongs to the kernel implementation and its +// `cargo test` cases (DAEMON-LIFECYCLE.md §6 tests 4 and 5). + +import { createServer } from 'node:net'; +import { + appendFileSync, + closeSync, + linkSync, + mkdirSync, + openSync, + readFileSync, + renameSync, + rmSync, + writeFileSync, + writeSync, +} from 'node:fs'; +import { join, resolve } from 'node:path'; + +const EXIT_ALREADY_SERVING = 3; +const VERSION = '0.0.0-stub'; + +const argv = process.argv.slice(2); +if (!argv.includes('serve')) fail('stub relayflowd: expected a `serve` subcommand'); +const dataDirIndex = argv.indexOf('--data-dir'); +if (dataDirIndex === -1 || argv[dataDirIndex + 1] === undefined) { + fail('stub relayflowd: expected --data-dir '); +} +const dataDir = resolve(argv[dataDirIndex + 1]); +const socketPath = join(dataDir, 'relayflowd.sock'); +const connectionPath = join(dataDir, 'connection.json'); +const lockPath = join(dataDir, 'relayflowd.lock'); + +// Test knobs. Each one models a failure the CLI must classify, not a mode the +// real daemon has. +const protocol = Number(process.env['STUB_PROTOCOL'] ?? '0'); +const startupExit = process.env['STUB_STARTUP_EXIT']; +const publishDelayMs = Number(process.env['STUB_PUBLISH_DELAY_MS'] ?? '0'); +// Holds the lock but does not bind yet, so the CLI has to poll for readiness. +const listenDelayMs = Number(process.env['STUB_LISTEN_DELAY_MS'] ?? '0'); +const writeConnectionFile = process.env['STUB_NO_CONNECTION_FILE'] !== '1'; +// Append-only evidence for the live tests: one `start` line per process that +// was launched, one `serving` line per process that actually owns the socket, +// one `lost` line per process that exited 3. "Exactly one serving" is the +// property the concurrency case exists to check. +const tracePath = process.env['STUB_TRACE']; +function trace(line) { + if (tracePath !== undefined) appendFileSync(tracePath, `${line}\n`); +} + +if (startupExit !== undefined) { + process.stderr.write('stub relayflowd: refusing to start (STUB_STARTUP_EXIT)\n'); + process.exit(Number(startupExit)); +} + +mkdirSync(dataDir, { recursive: true }); +trace(`start ${process.pid}`); + +// §3 step 3->4: acquire the singleton first. Everything destructive is +// sequenced after it, so a loser has unlinked nothing and bound nothing. +if (!acquireLock()) { + process.stderr.write(`stub relayflowd: another relayflowd is already serving ${dataDir}\n`); + trace(`lost ${process.pid}`); + process.exit(EXIT_ALREADY_SERVING); +} + +// §1 step 3: sweep a predecessor's leftovers, safe only under the lock. +rmSync(socketPath, { force: true }); +rmSync(connectionPath, { force: true }); + +const server = createServer((socket) => { + let buffer = ''; + socket.on('data', (chunk) => { + buffer += chunk.toString('utf8'); + let newline; + while ((newline = buffer.indexOf('\n')) !== -1) { + const line = buffer.slice(0, newline); + buffer = buffer.slice(newline + 1); + if (line.length === 0) continue; + const request = JSON.parse(line); + socket.write(`${JSON.stringify({ id: request.id, ok: true, result: reply(request) })}\n`); + } + }); + socket.on('error', () => {}); +}); + +server.on('error', (error) => fail(`stub relayflowd: bind failed: ${error.message}`)); +const bind = () => server.listen(socketPath, onListening); +if (listenDelayMs > 0) setTimeout(bind, listenDelayMs); +else bind(); + +function onListening() { + trace(`serving ${process.pid}`); + // §1: the connection file is written only after listen(2) has returned, so + // a reader that sees the file can always connect. The delay knob widens + // that window on purpose, to prove the CLI polls rather than assuming. + const publish = () => { + if (!writeConnectionFile) return; + const temporary = `${connectionPath}.tmp.${process.pid}`; + writeFileSync(temporary, `${JSON.stringify({ + socket_path: socketPath, + pid: process.pid, + version: VERSION, + protocol, + started_at_ms: Date.now(), + })}\n`, { mode: 0o600 }); + renameSync(temporary, connectionPath); + }; + if (publishDelayMs > 0) setTimeout(publish, publishDelayMs); + else publish(); +} + +for (const signal of ['SIGTERM', 'SIGINT']) { + process.on(signal, () => { + rmSync(connectionPath, { force: true }); + rmSync(socketPath, { force: true }); + rmSync(lockPath, { force: true }); + process.exit(0); + }); +} + +function reply(request) { + switch (request.verb) { + case 'hello': + return { protocol, server: 'relayflowd-stub' }; + case 'run.start': + case 'run.resume': + return { + run_id: 'STUBRUN00000000000000000A', + status: 'completed', + completion_reason: 'success', + completed_steps: 1, + }; + default: + return {}; + } +} + +/** + * Write the pid into a private file first, then `link(2)` it into place. + * `link` is atomic and fails EEXIST, so exactly one caller wins — and unlike + * `open(O_CREAT|O_EXCL)` there is no window where the lock is visible but + * still empty, which a rival would misread as a corpse and clear. A lock whose + * recorded pid is gone IS a corpse (a hard kill) and is reclaimed once; the + * kernel gets that for free from `flock`, which is why §3 chose it. + */ +function acquireLock() { + const staging = `${lockPath}.tmp.${process.pid}`; + const fd = openSync(staging, 'w', 0o600); + writeSync(fd, String(process.pid)); + closeSync(fd); + try { + for (let attempt = 0; attempt < 2; attempt += 1) { + try { + linkSync(staging, lockPath); + return true; + } catch (error) { + if (error.code !== 'EEXIST' || attempt === 1) return false; + const holder = Number.parseInt(readFileSync(lockPath, 'utf8').trim(), 10); + if (Number.isInteger(holder) && holder > 0 && isAlive(holder)) return false; + rmSync(lockPath, { force: true }); + } + } + return false; + } finally { + rmSync(staging, { force: true }); + } +} + +function isAlive(pid) { + try { + process.kill(pid, 0); + return true; + } catch (error) { + return error.code === 'EPERM'; + } +} + +function fail(message) { + process.stderr.write(`${message}\n`); + process.exit(1); +} diff --git a/packages/sdk/tests/live-kernel.test.ts b/packages/sdk/tests/live-kernel.test.ts index ee0748491..b633c29e9 100644 --- a/packages/sdk/tests/live-kernel.test.ts +++ b/packages/sdk/tests/live-kernel.test.ts @@ -63,6 +63,8 @@ function locateRelayflowd(): string { } const temporaryDirectories: string[] = []; const daemons: ChildProcess[] = []; +/** Detached daemons this suite did not spawn itself, reaped by pid. */ +const daemonPids: number[] = []; const clients: JournalClient[] = []; beforeAll(() => { @@ -75,6 +77,13 @@ beforeAll(() => { afterEach(async () => { for (const client of clients.splice(0)) client.close(); for (const daemon of daemons.splice(0)) await stopDaemon(daemon); + for (const pid of daemonPids.splice(0)) { + try { + process.kill(pid, 'SIGTERM'); + } catch { + // Already gone. + } + } for (const directory of temporaryDirectories.splice(0)) { rmSync(directory, { recursive: true, force: true }); } @@ -1170,10 +1179,14 @@ steps: expect(refused.stderr).toContain('REFUSED [cli_missing]'); expect(runArtifacts(dataDir)).toEqual(before); + // `--no-spawn` is what still makes an absent daemon a refusal: `flows run` + // otherwise starts one (kernel/DAEMON-LIFECYCLE.md §3, §4). The property + // this case pins -- refused before any journal write, naming the socket -- + // is unchanged, and `runArtifacts(absentDir)` below still proves it. const absentDir = temporaryDirectory('flows-live-absent-'); const absentSocket = join(absentDir, 'relayflowd.sock'); const unreachable = invokeCli([ - 'run', '--data-dir', absentDir, join(TESTDATA, 'hello-deterministic.flow.yaml'), + 'run', '--no-spawn', '--data-dir', absentDir, join(TESTDATA, 'hello-deterministic.flow.yaml'), ]); expect(unreachable.status).toBe(2); expect(unreachable.stderr).toContain('REFUSED [daemon_unreachable]'); @@ -1181,6 +1194,41 @@ steps: expect(unreachable.stderr).toContain('relayflowd --data-dir'); expect(runArtifacts(absentDir)).toEqual([]); }); + + // kernel/DAEMON-LIFECYCLE.md §6 test 15, against the real binary: the + // property is enforced by the daemon's `flock(2)`, so nothing short of two + // real relayflowd processes contending for one data dir tests it. The + // CLI-side half (a losing child exits 3 and its CLI keeps polling) is + // covered hermetically in daemon-lifecycle-live.test.ts. + it('starts exactly one daemon when two runs race for one empty data dir', async () => { + const dataDir = join(temporaryDirectory('flows-live-race-'), 'data'); + const flow = join(TESTDATA, 'hello-deterministic.flow.yaml'); + + const [first, second] = await Promise.all([ + invokeCliAsync(['run', '--data-dir', dataDir, flow]), + invokeCliAsync(['run', '--data-dir', dataDir, flow]), + ]); + + expect(first.status, first.stderr).toBe(0); + expect(second.status, second.stderr).toBe(0); + expect(first.stdout).toContain('completionReason: success'); + expect(second.stdout).toContain('completionReason: success'); + + // One socket, one owner. `ps` rather than the connection file, because the + // question is how many PROCESSES survived, and a file can only ever name + // the last writer. + const surviving = spawnSync('/bin/sh', ['-c', `ps ax -o pid=,command= | grep -F -- '--data-dir ${dataDir} serve' | grep -v grep`], { encoding: 'utf8' }) + .stdout.split('\n').filter((line) => line.trim().length > 0); + for (const line of surviving) { + const pid = Number.parseInt(line.trim().split(/\s+/)[0]!, 10); + if (Number.isInteger(pid)) daemonPids.push(pid); + } + expect(surviving, surviving.join('\n')).toHaveLength(1); + + // Two runs, two journals: both CLIs reached the same daemon rather than + // one of them quietly reusing the other's run. + expect(runJournals(dataDir)).toHaveLength(2); + }, 60_000); }); describe('JournalClient wire conformance against live relayflowd', () => { diff --git a/packages/sdk/tests/relayflowd-path.test.ts b/packages/sdk/tests/relayflowd-path.test.ts new file mode 100644 index 000000000..c1f5e28a6 --- /dev/null +++ b/packages/sdk/tests/relayflowd-path.test.ts @@ -0,0 +1,141 @@ +// Binary resolution, kernel/DAEMON-LIFECYCLE.md §3.1. Every anchor is driven +// through injected deps: nothing here touches a real filesystem or PATH. + +import { describe, expect, it } from 'vitest'; +import { + RelayflowdNotFoundError, + resolveRelayflowdBinary, + runtimePackageName, + type RelayflowdPathDeps, +} from '../src/relayflowd-path.js'; + +function makeDeps(overrides: Partial = {}): RelayflowdPathDeps { + return { + env: {}, + entrypoint: undefined, + cwd: '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/workspace/project', + moduleDir: '/workspace/project/node_modules/@relayflows/sdk/dist', + platform: 'linux', + arch: 'x64', + isExecutable: () => false, + exists: () => false, + realpath: (path) => path, + resolveFrom: () => null, + which: () => null, + ...overrides, + }; +} + +describe('resolveRelayflowdBinary (§3.1)', () => { + // Test 18. An operator who exported RELAYFLOWD_BIN meant it; falling through + // to PATH would silently run a different binary than the one they named, + // which is the silent fallback AGENTS.md rule 4 forbids. + it('refuses a RELAYFLOWD_BIN that is not executable instead of falling through', () => { + let whichCalls = 0; + const deps = makeDeps({ + env: { RELAYFLOWD_BIN: '/etc/hosts' }, + which: () => { + whichCalls += 1; + return '/usr/local/bin/relayflowd'; + }, + isExecutable: (path) => path === '/usr/local/bin/relayflowd', + }); + + expect(() => resolveRelayflowdBinary(deps)).toThrow(RelayflowdNotFoundError); + expect(() => resolveRelayflowdBinary(deps)).toThrow(/RELAYFLOWD_BIN/); + expect(whichCalls).toBe(0); + }); + + it('honours an executable RELAYFLOWD_BIN above every other anchor', () => { + const deps = makeDeps({ + env: { RELAYFLOWD_BIN: '/opt/custom/relayflowd' }, + isExecutable: () => true, + entrypoint: '/usr/local/bin/flows', + }); + expect(resolveRelayflowdBinary(deps)).toBe('/opt/custom/relayflowd'); + }); + + // The published layout: bin/flows and bin/relayflowd side by side. realpath + // matters because a package-manager launcher is a symlink into the real tree. + it('finds the sibling of the resolved flows entrypoint', () => { + const deps = makeDeps({ + entrypoint: '/usr/local/bin/flows', + realpath: (path) => (path === '/usr/local/bin/flows' + ? '/usr/local/lib/node_modules/@relayflows/runtime-linux-x64/bin/flows' + : path), + isExecutable: (path) => + path === '/usr/local/lib/node_modules/@relayflows/runtime-linux-x64/bin/relayflowd', + }); + + expect(resolveRelayflowdBinary(deps)) + .toBe('/usr/local/lib/node_modules/@relayflows/runtime-linux-x64/bin/relayflowd'); + }); + + it('falls back to the per-host runtime package resolved from several anchors', () => { + const seen: string[] = []; + const deps = makeDeps({ + entrypoint: '/usr/local/bin/flows', + realpath: (path) => path, + resolveFrom: (specifier, anchor) => { + seen.push(anchor); + // Only the consumer's cwd can see the per-project optional dependency. + return anchor === '/workspace/project/package.json' + ? `/workspace/project/node_modules/${specifier}` + : null; + }, + isExecutable: (path) => + path === '/workspace/project/node_modules/@relayflows/runtime-linux-x64/bin/relayflowd', + }); + + expect(resolveRelayflowdBinary(deps)) + .toBe('/workspace/project/node_modules/@relayflows/runtime-linux-x64/bin/relayflowd'); + expect(seen).toContain('/workspace/project/package.json'); + }); + + it('finds a source checkout by walking up to kernel/Cargo.toml', () => { + const deps = makeDeps({ + cwd: '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/src/flows/packages/sdk', + exists: (path) => path === '/src/flows/kernel/Cargo.toml', + isExecutable: (path) => path === '/src/flows/kernel/target/debug/relayflowd', + }); + + expect(resolveRelayflowdBinary(deps)).toBe('/src/flows/kernel/target/debug/relayflowd'); + }); + + it('prefers a release build over a debug build in a source checkout', () => { + const deps = makeDeps({ + cwd: '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/src/flows', + exists: (path) => path === '/src/flows/kernel/Cargo.toml', + isExecutable: (path) => path.startsWith('/src/flows/kernel/target/'), + }); + + expect(resolveRelayflowdBinary(deps)).toBe('/src/flows/kernel/target/release/relayflowd'); + }); + + it('uses PATH only when every anchored lookup came up empty', () => { + const deps = makeDeps({ + which: () => '/usr/bin/relayflowd', + isExecutable: (path) => path === '/usr/bin/relayflowd', + }); + expect(resolveRelayflowdBinary(deps)).toBe('/usr/bin/relayflowd'); + }); + + it('names the runtime package and the escape hatch when nothing is found', () => { + expect(() => resolveRelayflowdBinary(makeDeps({ platform: 'darwin', arch: 'arm64' }))) + .toThrow(/@relayflows\/runtime-darwin-arm64.*RELAYFLOWD_BIN/s); + }); + + it('does not walk past a bounded number of ancestors', () => { + const deps = makeDeps({ + cwd: '/a/b/c/d/e/f/g/h/i/j/deep', + exists: (path) => path === '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/kernel/Cargo.toml', + isExecutable: () => true, + }); + expect(() => resolveRelayflowdBinary(deps)).toThrow(RelayflowdNotFoundError); + }); + + it('names the package per platform and arch', () => { + expect(runtimePackageName('linux', 'x64')).toBe('@relayflows/runtime-linux-x64'); + expect(runtimePackageName('darwin', 'arm64')).toBe('@relayflows/runtime-darwin-arm64'); + }); +}); diff --git a/workflows/daemon-lifecycle.yaml b/workflows/daemon-lifecycle.yaml new file mode 100644 index 000000000..894246867 --- /dev/null +++ b/workflows/daemon-lifecycle.yaml @@ -0,0 +1,236 @@ +version: '1.0' +name: flows-daemon-lifecycle +description: > + Phase 1 of "package flows so it's frictionless": relayflowd currently + requires a human to build it and run `relayflowd serve` by hand before any + `flows run`/`flows check`/`flows resume` can connect — the CLI fails closed + on a missing socket rather than starting one. Give relayflowd a connection + file and give the flows CLI attach-or-spawn logic, so `npm install -g + relayflows && flows run x.yaml` needs no manual daemon step, mirroring the + proven pattern in ../relay (packages/cli/src/cli/lib/broker-lifecycle.ts + + broker-connection.ts + packages/harness-driver/src/broker-path.ts). + Orchestrated by the previous-generation engine, per RFC-0001 §2 rule 1 and + the precedent of workflows/bootstrap-gate1.yaml, which bootstrapped gate 1 + itself the same way. + +swarm: + pattern: dag + channel: flows-daemon-lifecycle + timeoutMs: 7200000 + +agents: + - name: architect + cli: claude + preset: analyst + role: > + Systems architect. Reads the existing kernel/CLI contract and the + ../relay precedent, writes a precise design doc for the connection-file + handshake and attach-or-spawn protocol. + - name: kernel-dev + cli: codex + preset: worker + role: Rust systems engineer. Implements relayflowd's side of the handshake. + - name: cli-dev + cli: claude + preset: worker + role: TypeScript engineer. Implements the flows CLI's attach-or-spawn logic. + - name: adversary + cli: claude + preset: reviewer + role: > + Adversarial reviewer. Tries to refute that the implementation is safe + under concurrent invocation, stale state, and crash recovery. + +workflows: + - name: daemon-lifecycle + steps: + - name: design + type: agent + agent: architect + task: | + Read, in this order: + 1. docs/SURFACE.md section 5 ("Invocation: the gate-1 CLI") — the + current `flows check`/`flows run`/`flows resume` contract, which + today requires a daemon already listening at + `/relayflowd.sock` and fails closed otherwise. + 2. kernel/relayflowd/src/main.rs and kernel/relayflowd/src/server.rs + — how `relayflowd serve --data-dir ` currently starts and + what `server::serve` does. + 3. packages/sdk/src/journal-client.ts — how the CLI currently + connects to that socket (fail-closed, no retry, no spawn). + 4. The reference implementation in the sibling checkout at + ../relay: packages/cli/src/cli/lib/broker-lifecycle.ts (spawn + detached, write `connection.json` with url/port/api_key/pid, + poll for readiness, attach-or-spawn on every command), + packages/cli/src/cli/lib/broker-connection.ts (reading/validating + that file), and packages/harness-driver/src/broker-path.ts (why + it tries multiple resolution anchors). Port the *protocol shape* + to our unix-socket daemon, not the TCP/port allocation + specifics — relayflowd has no port to negotiate. + + Write kernel/DAEMON-LIFECYCLE.md specifying, concretely: + 1. The connection file: exact path (`/connection.json`), + exact JSON shape (at minimum: socket_path, pid, relayflowd + version, started_at_ms), and exactly when relayflowd writes it + (only after the socket is accepting connections — never before) + and removes it (clean shutdown on SIGTERM/SIGINT; a stale file + left behind by a kill -9 must be detectable, not trusted blindly). + 2. The staleness check the CLI must run before trusting an existing + connection.json: is the pid alive, does the socket still accept + a connection. Both, not just one — a stale file with a live + unrelated pid must not falsely "attach". + 3. The attach-or-spawn algorithm the CLI runs before every + `check`/`run`/`resume`: read connection.json, validate it, attach + if valid; otherwise spawn `relayflowd serve --data-dir ` + detached (not inheriting the CLI's stdio, survives the CLI + exiting), poll for connection.json to appear with a bounded + timeout, then attach. State exactly how this stays correct when + two `flows` invocations race to spawn simultaneously (a lock + file, or accept-and-let-the-loser-retry — pick one and justify + it; don't hand-wave "TODO: handle races"). + 4. What changes in packages/sdk/src/journal-client.ts and + packages/sdk/src/cli.ts vs. what's new (e.g. a new + daemon-lifecycle.ts mirroring the ../relay file it's modeled on). + 5. Backward compatibility: an operator who still runs + `relayflowd serve` by hand (as documented in docs/SURFACE.md + today) must keep working exactly as before — attach-or-spawn is + an addition, not a replacement of manual operation. + Keep it tight and concrete — this is an implementation spec, not a + survey. No speculative scope beyond this handshake. + End your output with the line DESIGN_COMPLETE. + verification: + type: output_contains + value: DESIGN_COMPLETE + timeoutMs: 1800000 + + - name: kernel-impl + type: agent + agent: kernel-dev + dependsOn: [design] + maxIterations: 3 + task: | + Read kernel/DAEMON-LIFECYCLE.md and AGENTS.md. Implement + relayflowd's side of the handshake exactly per the design: + - `relayflowd serve` writes the connection file only once the unix + socket is actually accepting connections, and removes it on clean + shutdown (SIGTERM/SIGINT). A crash (kill -9) must leave a file the + CLI can detect as stale by the design's staleness check — do not + rely on relayflowd to clean up after its own hard kill. + - No behavior change for an operator who already runs + `relayflowd serve` manually today. + - Unit/integration tests: connection file appears only after the + socket is live (not before); is removed on clean shutdown; a + kill -9'd process leaves a file the staleness check correctly + identifies as stale (dead pid, or live-but-wrong-process pid via + a process-identity check if the design calls for one). + `cargo test --workspace` must pass. Keep files small and + single-purpose per AGENTS.md. + End your output with the line KERNEL_IMPL_DONE. + verification: + type: output_contains + value: KERNEL_IMPL_DONE + timeoutMs: 3600000 + + - name: kernel-tests + type: deterministic + dependsOn: [kernel-impl] + command: cd kernel && ../ops/cargo.sh test --workspace 2>&1 | tail -30 + timeoutMs: 900000 + + - name: cli-impl + type: agent + agent: cli-dev + dependsOn: [design] + maxIterations: 3 + task: | + Read kernel/DAEMON-LIFECYCLE.md and AGENTS.md. Implement the flows + CLI's side of the handshake exactly per the design: + - A new module (e.g. packages/sdk/src/daemon-lifecycle.ts) that: + reads and validates `/connection.json`; attaches if + valid; otherwise spawns `relayflowd serve --data-dir ` + detached (survives this CLI process exiting), polls for the + connection file with a bounded timeout, then attaches. Handles + the concurrent-invocation race per the design's chosen strategy. + - Wire it into packages/sdk/src/cli.ts so `flows check`, `flows + run`, and `flows resume` all call attach-or-spawn before using + journal-client.ts, instead of journal-client.ts failing closed on + first connect. Preserve journal-client.ts's existing fail-closed + behavior for the actual protocol calls once connected — this only + changes how the socket gets there. + - Tests: cold start (no connection file, no daemon running) spawns + one and the run succeeds; warm start (daemon already running) + attaches without spawning a second process; a stale connection + file (dead pid) is detected and a fresh daemon is spawned; two + concurrent invocations against an empty data-dir do not race into + two daemons owning the same socket path. + `npm test` (per packages/sdk's existing test script) must pass. + End your output with the line CLI_IMPL_DONE. + verification: + type: output_contains + value: CLI_IMPL_DONE + timeoutMs: 3600000 + + - name: sdk-tests + type: deterministic + dependsOn: [cli-impl, kernel-tests] + command: cd packages/sdk && npm install --no-audit --no-fund >/dev/null 2>&1 && npm test 2>&1 | tail -30 + timeoutMs: 900000 + + - name: e2e-smoke + type: deterministic + dependsOn: [sdk-tests] + command: > + rm -rf /tmp/flows-daemon-lifecycle-smoke && + node --experimental-strip-types packages/sdk/dist/cli.js check --json + testdata/hello-deterministic.flow.yaml --data-dir /tmp/flows-daemon-lifecycle-smoke 2>&1 | tail -5 && + node --experimental-strip-types packages/sdk/dist/cli.js run --json + testdata/hello-deterministic.flow.yaml --data-dir /tmp/flows-daemon-lifecycle-smoke 2>&1 | tail -20 + timeoutMs: 300000 + + - name: adversarial-review + type: agent + agent: adversary + dependsOn: [e2e-smoke] + task: | + Read kernel/DAEMON-LIFECYCLE.md and the full diff introduced by the + kernel-impl and cli-impl steps (git diff against this workflow's + starting commit). Adversarially review, specifically: + - Can two concurrent `flows` invocations against an empty data-dir + end up with two live relayflowd processes both believing they own + the same socket path? Walk the exact interleaving. + - Can a stale connection.json (from a kill -9) cause a CLI to + "attach" to a dead or unrelated process instead of detecting + staleness and spawning fresh? + - Does a spawned relayflowd actually survive the spawning CLI + process exiting (detached correctly), or does it die with its + parent? + - Does an operator who runs `relayflowd serve` by hand exactly as + documented today still work unchanged? + - Is the connection file written strictly after the socket is live, + never before (a race here means the CLI attaches to a socket + that isn't accepting yet)? + List every violation found with file:line. If violations exist, end + with REVIEW_FAILED. If the implementation honestly holds up under + all of the above, end with REVIEW_PASSED. + verification: + type: output_contains + value: REVIEW_PASSED + maxIterations: 1 + timeoutMs: 1800000 + + - name: report + type: agent + agent: architect + dependsOn: [adversarial-review] + task: | + Write ops/DAEMON-LIFECYCLE-REPORT.md: what was built, test results + verbatim (kernel-tests, sdk-tests, e2e-smoke), the adversarial + review verdict, what Phase 2 (per-platform relayflowd/flows binary + packages, mirroring packages/runtime-linux-x64) still needs once + this lands, and any open risk this workflow did not resolve. Honest + state only — no claiming green where evidence is thin. + End with REPORT_DONE. + verification: + type: output_contains + value: REPORT_DONE + timeoutMs: 1200000