From 73e17d51f37e4df108f6a60d5d8e6352b0f4d052 Mon Sep 17 00:00:00 2001 From: Anders Olsson Date: Fri, 7 Aug 2026 15:10:48 +0200 Subject: [PATCH] docs(plan): implementation plan for the wait-for readiness gate Six TDD tasks: schema + KDL parsing, the Waiting state and WaitInfo, a ready module that evaluates one condition, the interruptible gate inside do_start, CLI surfacing, and the README. Also corrects the spec: xy-protocol has no etcetera dependency, so ~ is expanded from $HOME; and in KDL `command`/`args` are sibling nodes inside the wait-for block, which changes how the parser counts conditions. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EGntTHCW3sEPy1VBRopNNp --- .../plans/2026-08-07-xy-readiness-gate.md | 1176 +++++++++++++++++ .../2026-08-07-xy-readiness-gate-design.md | 23 +- 2 files changed, 1195 insertions(+), 4 deletions(-) create mode 100644 docs/superpowers/plans/2026-08-07-xy-readiness-gate.md diff --git a/docs/superpowers/plans/2026-08-07-xy-readiness-gate.md b/docs/superpowers/plans/2026-08-07-xy-readiness-gate.md new file mode 100644 index 0000000..8a3937a --- /dev/null +++ b/docs/superpowers/plans/2026-08-07-xy-readiness-gate.md @@ -0,0 +1,1176 @@ +# Readiness Gate (`wait-for`) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Let a supervised server declare a precondition that must hold before it is spawned, so a slow infrastructure dependency costs patience rather than the crash-retry budget. + +**Architecture:** `xy-protocol` gains the `wait-for` schema, its KDL parsing, a `Waiting` state and a `WaitInfo` RPC field. `xy-supervisor` gains a `ready` module that evaluates a condition, and a gate inside `do_start` that polls it — interruptibly — before spawning. `xy` renders the new state. Waiting never calls `RetryWindow::record`, which is the property that fixes the bug. + +**Tech Stack:** Rust 2024, `kdl`, `humantime`/`humantime-serde`, `tokio` (`net`, `process`, `time`), `serde`. + +**Spec:** `docs/superpowers/specs/2026-08-07-xy-readiness-gate-design.md` + +## Global Constraints + +- Format with `cargo +nightly fmt` before every commit. NEVER plain `cargo fmt`. +- `cargo clippy --workspace --all-targets` must be clean before moving on. +- Run tests with `cargo nextest run`, never `cargo test`. +- TDD: write the failing test, run it, confirm it fails for the expected reason, then the minimal implementation. +- All `#[cfg(test)] mod tests` blocks go at the END of the source file. Append to the existing block; never create a second one in a file. +- Naming: short but descriptive. `err` not `e`, `result` not `r`. Single-char names only for loop indices (`i`/`j`/`k`) and counts (`n`). +- Blank line after every statement by default; multi-line `let` bindings ALWAYS get a blank line after. Adjacent asserts stay grouped with no blanks between them. +- No comments unless the *why* is non-obvious. +- `pub(crate)` for internals; `pub` only at a crate's API boundary. +- No `mod.rs` for new modules — `foo.rs` module root plus a `foo/` directory for submodules. +- Default wait timeout is **120s**; default poll interval is **1s**. +- Test timeouts and intervals are in **milliseconds** — no test may wait on wall-clock seconds. + +## File Structure + +| File | Responsibility | +|---|---| +| `crates/xy-protocol/src/config.rs` | *Modify:* `WaitCondition`, `WaitForConfig`, `ServerConfig.wait_for` | +| `crates/xy-protocol/src/kdl_parse.rs` | *Modify:* parse the `wait-for` block, expand `~/` | +| `crates/xy-protocol/src/state.rs` | *Modify:* add `ServerState::Waiting` | +| `crates/xy-protocol/src/rpc.rs` | *Modify:* `WaitInfo`, `StatusDetail.wait_for` | +| `crates/xy-supervisor/src/ready.rs` | *New:* evaluate one condition, once | +| `crates/xy-supervisor/src/lib.rs` | *Modify:* declare `mod ready` | +| `crates/xy-supervisor/src/supervisor.rs` | *Modify:* `StartFailure`, `WaitProgress`, the gate, call sites | +| `crates/xy/src/daemon/handlers.rs` | *Modify:* populate `wait_for` on `StatusDetail` | +| `crates/xy/src/cli/format.rs` | *Modify:* render the `wait-for` line in status | +| `README.md` | *Modify:* document `wait-for` | + +--- + +### Task 1: Schema and KDL parsing + +**Files:** +- Modify: `crates/xy-protocol/src/config.rs` +- Modify: `crates/xy-protocol/src/kdl_parse.rs` +- Test: both files' existing `#[cfg(test)] mod tests` blocks + +**Interfaces:** +- Consumes: nothing from earlier tasks. +- Produces, all `pub`: + - `enum WaitCondition { Path(PathBuf), Tcp(String), Command { command: PathBuf, args: Vec } }` + - `struct WaitForConfig { condition: WaitCondition, timeout: Duration, interval: Duration }` + - `ServerConfig.wait_for: Option` + +- [ ] **Step 1: Write the failing parser tests** + +Append to the `#[cfg(test)] mod tests` block at the end of `crates/xy-protocol/src/kdl_parse.rs`: + +```rust +#[test] +fn parses_a_path_condition_with_defaults() { + let text = "command \"/bin/x\"\nport 1\nwait-for { path \"/var/run/d.sock\" }"; + let cfg = parse_server_config("foo", text, p()).unwrap(); + + let wait = cfg.wait_for.unwrap(); + + assert_eq!( + wait.condition, + WaitCondition::Path(PathBuf::from("/var/run/d.sock")) + ); + assert_eq!(wait.timeout, Duration::from_secs(120)); + assert_eq!(wait.interval, Duration::from_secs(1)); +} + +#[test] +fn expands_a_leading_tilde_in_a_path_condition() { + unsafe { std::env::set_var("HOME", "/home/someone") }; + + let text = "command \"/bin/x\"\nport 1\nwait-for { path \"~/.orbstack/run/docker.sock\" }"; + let cfg = parse_server_config("foo", text, p()).unwrap(); + + assert_eq!( + cfg.wait_for.unwrap().condition, + WaitCondition::Path(PathBuf::from("/home/someone/.orbstack/run/docker.sock")) + ); +} + +#[test] +fn parses_a_tcp_condition_with_overrides() { + let text = "command \"/bin/x\"\nport 1\nwait-for { tcp \"127.0.0.1:5432\"\ntimeout \"30s\"\ninterval \"250ms\" }"; + let cfg = parse_server_config("foo", text, p()).unwrap(); + + let wait = cfg.wait_for.unwrap(); + + assert_eq!(wait.condition, WaitCondition::Tcp("127.0.0.1:5432".into())); + assert_eq!(wait.timeout, Duration::from_secs(30)); + assert_eq!(wait.interval, Duration::from_millis(250)); +} + +#[test] +fn parses_a_command_condition_from_sibling_nodes() { + let text = "command \"/bin/x\"\nport 1\nwait-for { command \"docker\"\nargs \"info\" }"; + let cfg = parse_server_config("foo", text, p()).unwrap(); + + assert_eq!( + cfg.wait_for.unwrap().condition, + WaitCondition::Command { + command: PathBuf::from("docker"), + args: vec!["info".to_string()], + } + ); +} + +#[test] +fn absent_wait_for_leaves_no_gate() { + let text = "command \"/bin/x\"\nport 1"; + let cfg = parse_server_config("foo", text, p()).unwrap(); + + assert!(cfg.wait_for.is_none()); +} + +#[test] +fn wait_for_without_a_condition_fails() { + let text = "command \"/bin/x\"\nport 1\nwait-for { timeout \"5s\" }"; + let err = parse_server_config("foo", text, p()).unwrap_err(); + + assert!(matches!( + err, + ConfigError::InvalidValue { + field: "wait-for", + .. + } + )); +} + +#[test] +fn wait_for_with_two_conditions_fails() { + let text = "command \"/bin/x\"\nport 1\nwait-for { path \"/a\"\ntcp \"127.0.0.1:1\" }"; + let err = parse_server_config("foo", text, p()).unwrap_err(); + + assert!(matches!( + err, + ConfigError::InvalidValue { + field: "wait-for", + .. + } + )); +} + +#[test] +fn wait_for_args_without_command_fails() { + let text = "command \"/bin/x\"\nport 1\nwait-for { args \"info\" }"; + let err = parse_server_config("foo", text, p()).unwrap_err(); + + assert!(matches!( + err, + ConfigError::InvalidValue { + field: "wait-for", + .. + } + )); +} + +#[test] +fn unknown_wait_for_key_fails() { + let text = "command \"/bin/x\"\nport 1\nwait-for { path \"/a\"\nnope \"x\" }"; + let err = parse_server_config("foo", text, p()).unwrap_err(); + + assert!(matches!( + err, + ConfigError::InvalidValue { + field: "wait-for", + .. + } + )); +} +``` + +Add `use crate::WaitCondition;` to that test module's imports if `use super::*;` does not already bring it in. + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `cargo nextest run -p xy-protocol -E 'test(/wait_for|wait-for|tilde|condition/)'` + +Expected: FAIL to compile, with `cannot find type 'WaitCondition'` / `no field 'wait_for' on type 'ServerConfig'`. + +- [ ] **Step 3: Add the schema** + +In `crates/xy-protocol/src/config.rs`, alongside the existing config types: + +```rust +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub enum WaitCondition { + Path(PathBuf), + Tcp(String), + Command { + command: PathBuf, + args: Vec, + }, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct WaitForConfig { + pub condition: WaitCondition, + #[serde(default = "default_wait_timeout", with = "humantime_serde")] + pub timeout: Duration, + #[serde(default = "default_wait_interval", with = "humantime_serde")] + pub interval: Duration, +} + +pub fn default_wait_timeout() -> Duration { + Duration::from_secs(120) +} + +pub fn default_wait_interval() -> Duration { + Duration::from_secs(1) +} +``` + +Add to `ServerConfig`, after `stop`: + +```rust + #[serde(default)] + pub wait_for: Option, +``` + +Re-export `WaitCondition` and `WaitForConfig` from `crates/xy-protocol/src/lib.rs` wherever `ServerConfig`, `RestartConfig` and `StopConfig` are already re-exported. + +- [ ] **Step 4: Parse the block** + +In `crates/xy-protocol/src/kdl_parse.rs`, add to the `use crate::{…}` list: `WaitCondition, WaitForConfig, default_wait_interval, default_wait_timeout`. + +Add the call in `parse_server_config`, after `let stop = parse_stop(...)?;`: + +```rust + let wait_for = parse_wait_for(&doc, source_path)?; +``` + +and the field `wait_for,` in the returned `ServerConfig`. + +Then add these functions: + +```rust +fn expand_tilde(raw: &str) -> PathBuf { + let Some(rest) = raw.strip_prefix("~/") else { + return PathBuf::from(raw); + }; + + match std::env::var_os("HOME") { + Some(home) => PathBuf::from(home).join(rest), + None => PathBuf::from(raw), + } +} + +fn parse_wait_for(doc: &KdlDocument, path: &Path) -> Result, ConfigError> { + let Some(node) = find_node(doc, "wait-for") else { + return Ok(None); + }; + + let invalid = |message: String| ConfigError::InvalidValue { + path: path.to_path_buf(), + field: "wait-for", + message, + }; + + let Some(children) = node.children() else { + return Err(invalid("expected a block with exactly one condition".into())); + }; + + let mut conditions: Vec = Vec::new(); + let mut command: Option = None; + let mut args: Vec = Vec::new(); + let mut timeout = default_wait_timeout(); + let mut interval = default_wait_interval(); + + for child in children.nodes() { + match child.name().value() { + "path" => { + let raw = string_arg(child, "wait-for", path)?; + + conditions.push(WaitCondition::Path(expand_tilde(&raw))); + } + "tcp" => { + let addr = string_arg(child, "wait-for", path)?; + + conditions.push(WaitCondition::Tcp(addr)); + } + "command" => { + command = Some(PathBuf::from(string_arg(child, "wait-for", path)?)); + } + "args" => { + args = child + .entries() + .iter() + .filter_map(|e| e.value().as_string().map(str::to_string)) + .collect(); + } + "timeout" => { + timeout = parse_duration_arg(child, "wait-for", path)?; + } + "interval" => { + interval = parse_duration_arg(child, "wait-for", path)?; + } + other => { + return Err(invalid(format!("unknown key `{other}`"))); + } + } + } + + if let Some(command) = command { + conditions.push(WaitCondition::Command { command, args }); + } else if !args.is_empty() { + return Err(invalid("`args` requires `command`".into())); + } + + match conditions.len() { + 1 => Ok(Some(WaitForConfig { + condition: conditions.remove(0), + timeout, + interval, + })), + 0 => Err(invalid( + "expected exactly one of `path`, `tcp` or `command`".into(), + )), + n => Err(invalid(format!( + "expected exactly one condition, found {n}" + ))), + } +} +``` + +`conditions` must be declared `let mut conditions` and matched by length before `remove(0)`; the `match` above does that. + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `cargo nextest run -p xy-protocol` + +Expected: PASS, including the 9 new tests. The pre-existing `parses_full_config` and `parses_minimal_config` must still pass — if `ServerConfig` construction elsewhere in the workspace now fails to compile because of the new field, fix those construction sites by adding `wait_for: None`. + +- [ ] **Step 6: Format, lint** + +```bash +cargo +nightly fmt +cargo clippy --workspace --all-targets +``` + +- [ ] **Step 7: Commit** + +```bash +git add crates/xy-protocol/ +git commit -m "feat(protocol): wait-for schema and KDL parsing" +``` + +--- + +### Task 2: `Waiting` state and the RPC field + +**Files:** +- Modify: `crates/xy-protocol/src/state.rs` +- Modify: `crates/xy-protocol/src/rpc.rs` +- Test: `crates/xy-protocol/src/state.rs` + +**Interfaces:** +- Consumes: nothing. +- Produces: `ServerState::Waiting`; `pub struct WaitInfo { description: String, elapsed_secs: u64, timeout_secs: u64 }`; `StatusDetail.wait_for: Option`. + +- [ ] **Step 1: Write the failing test** + +Append to the `#[cfg(test)] mod tests` block at the end of `crates/xy-protocol/src/state.rs`: + +```rust +#[test] +fn waiting_state_round_trips_as_json() { + let json = serde_json::to_string(&ServerState::Waiting).unwrap(); + + assert_eq!(json, "\"waiting\""); + + let back: ServerState = serde_json::from_str(&json).unwrap(); + + assert_eq!(back, ServerState::Waiting); +} +``` + +If the existing `ServerState` derives do not already include `PartialEq`, this test tells you — add it rather than weakening the assertion. + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cargo nextest run -p xy-protocol -E 'test(/waiting_state/)'` + +Expected: FAIL to compile, `no variant named 'Waiting' found for enum 'ServerState'`. + +- [ ] **Step 3: Add the variant and the RPC type** + +In `crates/xy-protocol/src/state.rs`, add `Waiting,` to `ServerState`. Place it between `Stopped` and `Starting`, since that is the lifecycle order. + +In `crates/xy-protocol/src/rpc.rs`: + +```rust +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct WaitInfo { + pub description: String, + pub elapsed_secs: u64, + pub timeout_secs: u64, +} +``` + +and add to `StatusDetail`: + +```rust + #[serde(default)] + pub wait_for: Option, +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cargo nextest run -p xy-protocol` + +Expected: PASS. Any `StatusDetail { … }` construction elsewhere now needs `wait_for: None` — fix those sites. + +- [ ] **Step 5: Format, lint, commit** + +```bash +cargo +nightly fmt +cargo clippy --workspace --all-targets +git add crates/xy-protocol/ +git commit -m "feat(protocol): Waiting state and WaitInfo on StatusDetail" +``` + +--- + +### Task 3: Evaluating a condition + +**Files:** +- Create: `crates/xy-supervisor/src/ready.rs` +- Modify: `crates/xy-supervisor/src/lib.rs` +- Test: `crates/xy-supervisor/src/ready.rs` + +**Interfaces:** +- Consumes: `WaitCondition` from Task 1. +- Produces: `pub(crate) async fn is_ready(condition: &WaitCondition, budget: Duration) -> bool` and `pub(crate) fn describe(condition: &WaitCondition) -> String`. + +`budget` bounds a single evaluation — the caller passes the poll `interval`, so neither a TCP connect nor a hung command can outlast one tick. + +- [ ] **Step 1: Write the failing tests** + +Create `crates/xy-supervisor/src/ready.rs` containing only the test module: + +```rust +#[cfg(test)] +mod tests { + use super::*; + use std::path::PathBuf; + + fn ms(n: u64) -> Duration { + Duration::from_millis(n) + } + + #[tokio::test] + async fn an_existing_path_is_ready() { + let tmp = tempfile::tempdir().unwrap(); + let file = tmp.path().join("sock"); + + std::fs::write(&file, b"").unwrap(); + + assert!(is_ready(&WaitCondition::Path(file), ms(50)).await); + } + + #[tokio::test] + async fn a_missing_path_is_not_ready() { + let condition = WaitCondition::Path(PathBuf::from("/definitely/not/here.sock")); + + assert!(!is_ready(&condition, ms(50)).await); + } + + #[tokio::test] + async fn a_listening_port_is_ready() { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap().to_string(); + + assert!(is_ready(&WaitCondition::Tcp(addr), ms(500)).await); + } + + #[tokio::test] + async fn a_closed_port_is_not_ready() { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap().to_string(); + + drop(listener); + + assert!(!is_ready(&WaitCondition::Tcp(addr), ms(500)).await); + } + + #[tokio::test] + async fn a_command_exiting_zero_is_ready() { + let condition = WaitCondition::Command { + command: PathBuf::from("/usr/bin/true"), + args: vec![], + }; + + assert!(is_ready(&condition, ms(500)).await); + } + + #[tokio::test] + async fn a_command_exiting_nonzero_is_not_ready() { + let condition = WaitCondition::Command { + command: PathBuf::from("/usr/bin/false"), + args: vec![], + }; + + assert!(!is_ready(&condition, ms(500)).await); + } + + #[tokio::test] + async fn a_command_that_outlasts_its_budget_is_not_ready() { + let condition = WaitCondition::Command { + command: PathBuf::from("/bin/sleep"), + args: vec!["5".to_string()], + }; + + assert!(!is_ready(&condition, ms(100)).await); + } + + #[test] + fn descriptions_name_the_condition() { + assert_eq!( + describe(&WaitCondition::Path(PathBuf::from("/a/b.sock"))), + "path /a/b.sock" + ); + assert_eq!( + describe(&WaitCondition::Tcp("127.0.0.1:5432".into())), + "tcp 127.0.0.1:5432" + ); + assert_eq!( + describe(&WaitCondition::Command { + command: PathBuf::from("docker"), + args: vec!["info".to_string()], + }), + "command docker info" + ); + } +} +``` + +Declare the module in `crates/xy-supervisor/src/lib.rs` alongside the existing ones: + +```rust +mod ready; +``` + +`tempfile` must be a dev-dependency of `xy-supervisor`; it is already used by `logs.rs` tests, so no Cargo change should be needed. If it is missing, add `tempfile.workspace = true` under `[dev-dependencies]`. + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `cargo nextest run -p xy-supervisor -E 'test(/ready::/)'` + +Expected: FAIL to compile, `cannot find function 'is_ready' in this scope`. + +- [ ] **Step 3: Write the implementation** + +Prepend to `crates/xy-supervisor/src/ready.rs`: + +```rust +use std::process::Stdio; +use std::time::Duration; +use xy_protocol::WaitCondition; + +pub(crate) async fn is_ready(condition: &WaitCondition, budget: Duration) -> bool { + match condition { + WaitCondition::Path(p) => p.exists(), + WaitCondition::Tcp(addr) => { + let connect = tokio::net::TcpStream::connect(addr); + + matches!(tokio::time::timeout(budget, connect).await, Ok(Ok(_))) + } + WaitCondition::Command { command, args } => { + let mut cmd = tokio::process::Command::new(command); + + cmd.args(args) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .kill_on_drop(true); + + match tokio::time::timeout(budget, cmd.status()).await { + Ok(Ok(status)) => status.success(), + _ => false, + } + } + } +} + +pub(crate) fn describe(condition: &WaitCondition) -> String { + match condition { + WaitCondition::Path(p) => format!("path {}", p.display()), + WaitCondition::Tcp(addr) => format!("tcp {addr}"), + WaitCondition::Command { command, args } => { + let mut out = format!("command {}", command.display()); + + for arg in args { + out.push(' '); + out.push_str(arg); + } + + out + } + } +} +``` + +`kill_on_drop(true)` is load-bearing: without it a command that outlasts its budget is left running as an orphan on every poll tick. + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cargo nextest run -p xy-supervisor -E 'test(/ready::/)'` + +Expected: PASS, 8 tests. + +- [ ] **Step 5: Format, lint, commit** + +```bash +cargo +nightly fmt +cargo clippy --workspace --all-targets +git add crates/xy-supervisor/ +git commit -m "feat(supervisor): evaluate wait-for conditions" +``` + +--- + +### Task 4: The gate + +The heart of the feature. `do_start` grows a gate before the spawn and a richer error type, and all three call sites are updated. + +**Files:** +- Modify: `crates/xy-supervisor/src/supervisor.rs` +- Test: `crates/xy-supervisor/src/supervisor.rs` + +**Interfaces:** +- Consumes: `is_ready` / `describe` (Task 3), `ServerState::Waiting` (Task 2), `WaitForConfig` (Task 1). +- Produces: `Status.waiting: Option` where `pub struct WaitProgress { pub description: String, pub started_at: Instant, pub timeout: Duration }`. Task 5 consumes it. + +- [ ] **Step 1: Write the failing tests** + +Append to the `#[cfg(test)] mod tests` block at the end of `crates/xy-supervisor/src/supervisor.rs`. Extend `cfg(...)` usage by building on the existing helper — add this alongside it: + +```rust + fn cfg_with_wait(name: &str, wait: WaitForConfig) -> ServerConfig { + let mut c = cfg(name, RestartPolicy::Never, 5); + c.wait_for = Some(wait); + c + } +``` + +Then the tests: + +```rust + #[tokio::test] + async fn a_met_condition_spawns_immediately() { + let tmp = tempfile::tempdir().unwrap(); + let file = tmp.path().join("sock"); + + std::fs::write(&file, b"").unwrap(); + + let cfg = cfg_with_wait( + "x", + WaitForConfig { + condition: WaitCondition::Path(file), + timeout: Duration::from_millis(500), + interval: Duration::from_millis(10), + }, + ); + + let (mock, ctl) = MockChild::new(1); + let queue = Arc::new(Mutex::new(vec![mock])); + let spawner = QueueSpawner { queue }; + + let (status_tx, mut status_rx) = watch::channel(initial_status(&cfg)); + let (cmd_tx, cmd_rx) = mpsc::channel(8); + let task = SupervisorTask::new(cfg, sink("x"), spawner, status_tx, cmd_rx); + let h = tokio::spawn(task.run()); + + let (ack_tx, ack_rx) = oneshot::channel(); + cmd_tx + .send(SupervisorCmd::Start { ack: ack_tx }) + .await + .unwrap(); + assert_eq!(ack_rx.await.unwrap(), StartAck::Started); + wait_for(&mut status_rx, ServerState::Running).await; + + drop(ctl); + + let (ack_tx, ack_rx) = oneshot::channel(); + cmd_tx + .send(SupervisorCmd::Shutdown { ack: ack_tx }) + .await + .unwrap(); + ack_rx.await.unwrap(); + h.await.unwrap(); + } + + #[tokio::test] + async fn a_timed_out_gate_fails_without_spawning_or_touching_the_retry_budget() { + let cfg = cfg_with_wait( + "x", + WaitForConfig { + condition: WaitCondition::Path(PathBuf::from("/definitely/not/here.sock")), + timeout: Duration::from_millis(80), + interval: Duration::from_millis(10), + }, + ); + + // An empty queue: any spawn attempt panics, which is the assertion that + // a timed-out gate never reaches the spawner. + let queue = Arc::new(Mutex::new(Vec::new())); + let spawner = QueueSpawner { queue }; + + let (status_tx, mut status_rx) = watch::channel(initial_status(&cfg)); + let (cmd_tx, cmd_rx) = mpsc::channel(8); + let task = SupervisorTask::new(cfg, sink("x"), spawner, status_tx, cmd_rx); + let h = tokio::spawn(task.run()); + + let (ack_tx, _ack_rx) = oneshot::channel(); + cmd_tx + .send(SupervisorCmd::Start { ack: ack_tx }) + .await + .unwrap(); + + wait_for(&mut status_rx, ServerState::Waiting).await; + wait_for(&mut status_rx, ServerState::Failed).await; + + assert_eq!( + status_rx.borrow().restart_count, + 0, + "waiting must not consume the restart budget" + ); + + let (ack_tx, ack_rx) = oneshot::channel(); + cmd_tx + .send(SupervisorCmd::Shutdown { ack: ack_tx }) + .await + .unwrap(); + ack_rx.await.unwrap(); + h.await.unwrap(); + } + + #[tokio::test] + async fn stop_during_a_wait_cancels_promptly() { + let cfg = cfg_with_wait( + "x", + WaitForConfig { + condition: WaitCondition::Path(PathBuf::from("/definitely/not/here.sock")), + timeout: Duration::from_secs(30), + interval: Duration::from_millis(10), + }, + ); + + let queue = Arc::new(Mutex::new(Vec::new())); + let spawner = QueueSpawner { queue }; + + let (status_tx, mut status_rx) = watch::channel(initial_status(&cfg)); + let (cmd_tx, cmd_rx) = mpsc::channel(8); + let task = SupervisorTask::new(cfg, sink("x"), spawner, status_tx, cmd_rx); + let h = tokio::spawn(task.run()); + + let (ack_tx, _ack_rx) = oneshot::channel(); + cmd_tx + .send(SupervisorCmd::Start { ack: ack_tx }) + .await + .unwrap(); + wait_for(&mut status_rx, ServerState::Waiting).await; + + let (ack_tx, ack_rx) = oneshot::channel(); + cmd_tx + .send(SupervisorCmd::Stop { ack: ack_tx }) + .await + .unwrap(); + + // The 30s timeout would swamp this if the poll loop were not interruptible. + tokio::time::timeout(Duration::from_secs(2), ack_rx) + .await + .expect("stop must not wait for the gate timeout") + .unwrap(); + + wait_for(&mut status_rx, ServerState::Stopped).await; + + let (ack_tx, ack_rx) = oneshot::channel(); + cmd_tx + .send(SupervisorCmd::Shutdown { ack: ack_tx }) + .await + .unwrap(); + ack_rx.await.unwrap(); + h.await.unwrap(); + } +``` + +Add `use std::path::PathBuf;` and `use xy_protocol::{WaitCondition, WaitForConfig};` to the test module imports. + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `cargo nextest run -p xy-supervisor -E 'test(/spawns_immediately|retry_budget|cancels_promptly/)'` + +Expected: FAIL to compile, with `cannot find function 'cfg_with_wait'` resolved by your own helper, then `no field 'waiting' on type 'Status'`. Once it compiles, `a_timed_out_gate_fails_without_spawning_or_touching_the_retry_budget` must fail by panicking inside `QueueSpawner::spawn` on `q.remove(0)` from the empty queue — that panic IS the assertion that no gate exists yet. If it instead fails on a missing symbol, fix the typo and re-run until you see the spawn panic. + +- [ ] **Step 3: Add `WaitProgress` to `Status`** + +In `crates/xy-supervisor/src/supervisor.rs`: + +```rust +#[derive(Debug, Clone)] +pub struct WaitProgress { + pub description: String, + pub started_at: Instant, + pub timeout: Duration, +} +``` + +Add `pub waiting: Option,` to `Status`, add `waiting: None` to the test helper `initial_status`, to `crates/xy/src/daemon/mod.rs`'s initial `Status`, and carry `self.waiting.clone()` in `set_state`. Add the field `waiting: Option` to `SupervisorTask`, initialised `None` in `new`. + +- [ ] **Step 4: Add the gate** + +```rust +enum StartFailure { + Spawn(std::io::Error), + WaitTimedOut, + Cancelled, + Shutdown, +} + +enum GateOutcome { + Ready, + TimedOut, + Cancelled, + Shutdown, +} + +impl SupervisorTask { + async fn await_ready(&mut self) -> GateOutcome { + let Some(wait) = self.cfg.wait_for.clone() else { + return GateOutcome::Ready; + }; + + let started_at = Instant::now(); + + self.waiting = Some(WaitProgress { + description: crate::ready::describe(&wait.condition), + started_at, + timeout: wait.timeout, + }); + + self.set_state(ServerState::Waiting); + + let outcome = loop { + if crate::ready::is_ready(&wait.condition, wait.interval).await { + break GateOutcome::Ready; + } + + if started_at.elapsed() >= wait.timeout { + break GateOutcome::TimedOut; + } + + let mut tick = std::pin::pin!(sleep(wait.interval)); + + let interrupted = tokio::select! { + _ = &mut tick => None, + cmd = self.cmd_rx.recv() => match cmd { + None => Some(GateOutcome::Shutdown), + Some(SupervisorCmd::Stop { ack }) => { + let _ = ack.send(StopAck::NotRunning); + Some(GateOutcome::Cancelled) + } + Some(SupervisorCmd::Shutdown { ack }) => { + let _ = ack.send(()); + Some(GateOutcome::Shutdown) + } + Some(SupervisorCmd::Start { ack }) => { + let _ = ack.send(StartAck::Started); + None + } + Some(SupervisorCmd::Restart { ack }) => { + let _ = ack.send(()); + None + } + Some(SupervisorCmd::Reconfigure { new, ack }) => { + self.cfg = new; + let _ = ack.send(()); + None + } + }, + }; + + if let Some(outcome) = interrupted { + break outcome; + } + }; + + self.waiting = None; + + outcome + } +} +``` + +A `Reconfigure` arriving mid-wait replaces `self.cfg` but the loop continues on the cloned `wait` it started with; the new condition applies from the next start. That is deliberate — swapping the condition mid-poll would make the elapsed/timeout accounting meaningless. + +- [ ] **Step 5: Rewire `do_start` and its call sites** + +Change the signature and prepend the gate: + +```rust + async fn do_start(&mut self, cause: StartCause) -> Result { + match self.await_ready().await { + GateOutcome::Ready => {} + GateOutcome::TimedOut => return Err(StartFailure::WaitTimedOut), + GateOutcome::Cancelled => return Err(StartFailure::Cancelled), + GateOutcome::Shutdown => return Err(StartFailure::Shutdown), + } + + self.set_state(ServerState::Starting); + + let c = self + .spawner + .spawn(&self.cfg, self.log_sink.clone()) + .await + .map_err(StartFailure::Spawn)?; +``` + +The rest of `do_start` is unchanged. + +At each of the three call sites, replace the existing `Err(err) => { … }` arm with a match over `StartFailure`: + +```rust + Err(StartFailure::Spawn(err)) => { + warn!(name = %self.cfg.name, error = %err, "spawn failed"); + self.set_state(ServerState::Failed); + // at the Start call site only, also: + // let _ = ack.send(StartAck::SpawnFailed(err.to_string())); + } + Err(StartFailure::WaitTimedOut) => { + warn!(name = %self.cfg.name, "wait-for timed out"); + self.set_state(ServerState::Failed); + } + Err(StartFailure::Cancelled) => { + self.set_state(ServerState::Stopped); + } + Err(StartFailure::Shutdown) => return, +``` + +At the `Start` call site the `ack` must still be answered on every path — send `StartAck::SpawnFailed` for `Spawn`, and `StartAck::SpawnFailed` with a "wait-for timed out" message for `WaitTimedOut`, so a caller of `xy start` is never left waiting. + +- [ ] **Step 6: Run tests to verify they pass** + +Run: `cargo nextest run -p xy-supervisor` + +Expected: PASS, all tests including the three new ones. The pre-existing restart-count and lifecycle tests must still pass — they have no `wait_for`, so `await_ready` returns `Ready` immediately. + +- [ ] **Step 7: Format, lint, commit** + +```bash +cargo +nightly fmt +cargo clippy --workspace --all-targets +git add crates/xy-supervisor/ crates/xy/ +git commit -m "feat(supervisor): gate spawns on the wait-for condition" +``` + +--- + +### Task 5: Surfacing it in the CLI + +**Files:** +- Modify: `crates/xy/src/daemon/handlers.rs` +- Modify: `crates/xy/src/cli/format.rs` +- Test: `crates/xy/src/cli/format.rs` + +**Interfaces:** +- Consumes: `Status.waiting` (Task 4), `WaitInfo` (Task 2). +- Produces: nothing later tasks depend on. + +`xy list` needs no change: it renders state via `format!("{:?}", state).to_lowercase()`, so `Waiting` prints as `waiting` automatically. Confirm that in Step 1 rather than assuming. + +- [ ] **Step 1: Write the failing tests** + +`crates/xy/src/cli/format.rs` has no test module today (verified), so create one at the end of the file: + +```rust +#[cfg(test)] +mod tests { + use super::*; + use xy_protocol::{ServerState, rpc::WaitInfo}; + + // tests below +} +``` + +with these tests inside it: + +```rust + #[test] + fn list_renders_the_waiting_state() { + let rows = vec![ServerSummary { + name: "gitea".to_string(), + state: ServerState::Waiting, + pid: None, + port: 8181, + uptime_secs: None, + restart_count: 0, + last_exit: None, + }]; + + let out = list_table(&rows); + + assert!(out.contains("waiting"), "got: {out}"); + } + + #[test] + fn status_renders_the_wait_condition_and_elapsed() { + let info = WaitInfo { + description: "path /Users/me/.orbstack/run/docker.sock".to_string(), + elapsed_secs: 43, + timeout_secs: 120, + }; + + let out = wait_line(&info); + + assert!(out.contains("path /Users/me/.orbstack/run/docker.sock"), "got: {out}"); + assert!(out.contains("43s elapsed"), "got: {out}"); + assert!(out.contains("timeout 120s"), "got: {out}"); + } +``` + +Import `ServerState` and `WaitInfo` in that test module as needed. + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `cargo nextest run -p xy -E 'test(/waiting_state|wait_condition/)'` + +Expected: FAIL to compile, `cannot find function 'wait_line' in this scope`. + +- [ ] **Step 3: Implement** + +In `crates/xy/src/cli/format.rs`: + +```rust +pub(crate) fn wait_line(info: &WaitInfo) -> String { + format!( + " wait-for: {}\n {}s elapsed, timeout {}s\n", + info.description, info.elapsed_secs, info.timeout_secs + ) +} +``` + +In `crates/xy/src/daemon/handlers.rs`, populate the new field in the `status` handler, deriving elapsed at read time exactly as `uptime_secs` now does: + +```rust + wait_for: s.waiting.as_ref().map(|w| WaitInfo { + description: w.description.clone(), + elapsed_secs: w.started_at.elapsed().as_secs(), + timeout_secs: w.timeout.as_secs(), + }), +``` + +`list` does not carry `WaitInfo` — `ServerSummary` is unchanged. + +Then surface it in `crates/xy/src/cli/mod.rs`. The `status` function currently ends with `println!("{:#?}", d);` — print the wait line above that dump: + +```rust + if let Some(info) = &d.wait_for { + print!("{}", format::wait_line(info)); + } + + println!("{:#?}", d); +``` + +`format::wait_line` is `pub(crate)`, and `cli/mod.rs` already declares `mod format;`, so no import change is needed. + +- [ ] **Step 4: Run tests, then the full suite** + +```bash +cargo nextest run -p xy -E 'test(/waiting_state|wait_condition/)' +cargo nextest run +``` + +Expected: both PASS. + +- [ ] **Step 5: Format, lint, commit** + +```bash +cargo +nightly fmt +cargo clippy --workspace --all-targets +git add crates/xy/ +git commit -m "feat(cli): surface the waiting state and its condition" +``` + +--- + +### Task 6: Documentation + +**Files:** +- Modify: `README.md` + +**Interfaces:** +- Consumes: nothing. Produces: nothing. + +- [ ] **Step 1: Document `wait-for`** + +Add to `README.md`, after the server-config description and matching the file's existing four-space code-block convention: + +```markdown +## Waiting on a dependency + +A server can declare a precondition that must hold before it is started: + + wait-for { + path "~/.orbstack/run/docker.sock" + timeout "180s" + interval "1s" + } + +Exactly one condition per block — `path "

"` (the path exists), +`tcp ""` (a connection succeeds), or `command ""` with optional +`args` (the process exits 0). `timeout` defaults to 120s and `interval` to 1s. + +While waiting the server reports state `waiting`, and `xy status ` shows +which condition it is blocked on and for how long. If the timeout expires the +server is marked `failed` and is never spawned. + +Waiting does **not** consume the restart budget: a slow dependency costs +patience, not retries. This is what stops a Docker-backed server from being +marked failed at login while the Docker daemon is still starting. +``` + +- [ ] **Step 2: Verify the tree is healthy** + +```bash +cargo nextest run +cargo +nightly fmt +cargo clippy --workspace +``` + +- [ ] **Step 3: Commit** + +```bash +git add README.md +git commit -m "docs(readme): document the wait-for readiness gate" +``` + +--- + +## Manual acceptance + +The feature exists for one concrete case, so verify that one: + +```bash +cargo install --path crates/xy +``` + +Add to `~/.config/xy/servers/gitea.kdl` and `~/.config/xy/servers/signoz.kdl`: + +```kdl +wait-for { + path "~/.orbstack/run/docker.sock" + timeout "180s" +} +``` + +Then `xy reload` and confirm both still come up normally. The real test is a +logout/login with OrbStack cold: both should sit in `waiting` for ~20 seconds +and then start, rather than being marked `failed` after 15. Check with: + +```bash +xy list # expect: waiting, then running +xy status gitea # expect: the wait-for line while waiting +``` diff --git a/docs/superpowers/specs/2026-08-07-xy-readiness-gate-design.md b/docs/superpowers/specs/2026-08-07-xy-readiness-gate-design.md index 9c37dd7..d0a4312 100644 --- a/docs/superpowers/specs/2026-08-07-xy-readiness-gate-design.md +++ b/docs/superpowers/specs/2026-08-07-xy-readiness-gate-design.md @@ -66,10 +66,25 @@ Exactly one condition per `wait-for` block. Zero or more than one is a parse error, so `xy reload` exits 3 and changes nothing — matching how invalid config already behaves. -`path` values get `~` expanded at parse time via `etcetera::home_dir()`, since -the motivating case is literally `~/.orbstack/run/docker.sock`. The expanded -path is what `xy status` displays, so there is no ambiguity about what was -checked. +`path` values get a leading `~/` expanded at parse time from `$HOME`, since the +motivating case is literally `~/.orbstack/run/docker.sock`. `$HOME` rather than +`etcetera::home_dir()` deliberately: `xy-protocol` is a leaf crate with no +directory dependencies, and adding one for a two-line prefix substitution is +not worth it. If `$HOME` is unset the literal path is kept, which simply never +exists and surfaces as a normal timeout. The expanded path is what `xy status` +displays, so there is no ambiguity about what was checked. + +Note the KDL shape of the `command` kind: `command` and `args` are **sibling +nodes** inside the block, mirroring the top level, not one node with trailing +arguments: + + wait-for { + command "docker" + args "info" + } + +so the parser accumulates them separately and combines them after the loop. +`args` without `command` is a parse error. The `command` kind is bounded by the poll `interval`: each invocation is run under a timeout of `interval` and a non-exit is treated as not-ready, so a hung