crates/xy-protocol now rejects a one-line KDL block with more than one
key, and Harness::write_server wrote its restart block on exactly one
line with four keys, so every integration test using it would fail to
parse its config under the new parser behavior. Split onto separate
lines so the intended 10ms/50ms/3 restart config actually takes
effect, instead of silently falling back to the 1s/30s/5 defaults as
it had been doing all along.
In KDL, a `{ }` block's children are newline- or semicolon-separated,
so a block like `restart { policy "always" backoff-initial "10ms" }`
written on one line parses as a SINGLE node named `policy` whose
remaining words become extra arguments, not sibling keys. The parser
only ever read the first argument of each child node, so every key
after the first was silently discarded with no error.
Add `single_arg`, applied to every restart/stop/wait-for key that
takes exactly one value (all except `wait-for`'s `args`, which
legitimately takes zero or more), so a one-line block with 2+ keys
now fails to parse instead of silently keeping defaults.
`optional_string_map` (the `env { }` block) has the identical trap
but is out of scope here: it returns a bare BTreeMap with no error
path and would need a signature change to report a config error.
The spec asked for an integration test that a server whose condition can
never hold reaches failed without ever spawning; every gate test so far
drove SupervisorTask in isolation, which is exactly why the daemon boot
loop's ack await went unnoticed. This asserts both halves: `xy list`
answers well inside the gate, and the gated server fails on timeout
without the sleep server ever reaching running.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EGntTHCW3sEPy1VBRopNNp
The autostart loop awaited each Start ack, and a gated start does not ack
until its wait-for condition resolves. Because the accept loop and the
signal handler are installed after that loop, a server gated on a socket
that is not there yet left the daemon deaf for the whole gate: `xy list`
hung in the backlog and SIGTERM went unhandled. Nothing consumed the ack,
so drop the await and let the gates run concurrently with the accept loop.
Also clear started_at before publishing Waiting. A crash-and-regate kept
the dead process's start instant, so `xy list` showed a growing uptime for
a server that had been down for minutes -- in the one state whose whole
purpose is to explain what is going on.
And log the spawn error when a wait-for command cannot be run at all, so a
misspelled binary is distinguishable from a condition that is merely not
met yet.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EGntTHCW3sEPy1VBRopNNp
A second `command` node silently overwrote the first via a scalar
Option<PathBuf>, and a second `args` node silently overwrote the first
via a plain Vec, so duplicate keys last-won instead of tripping the
exactly-one-condition rule. Track args as Option<Vec<String>> and
error immediately on a repeated command or args key.
set_state computed uptime_secs from started_at.elapsed() and pushed the
result into the watch channel. do_start sets started_at immediately before
calling set_state(Running), so the published value was always ~0, and the
list/status handlers copied that frozen snapshot straight out. A server
that stayed healthy reported 0s forever, because the value was only
recomputed on the next state transition.
Status now carries started_at (an Instant, in-process only) and the
handlers derive uptime_secs when they build ServerSummary. The wire type
is unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EGntTHCW3sEPy1VBRopNNp
do_start incremented restart_count on every spawn, including the initial
one, so a server that had never restarted reported 1. do_start now takes a
StartCause: the explicit Start command (which is also how servers auto-start
at daemon boot) is Initial and does not count; the explicit Restart command
and the automatic post-crash respawn are Restart and do.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EGntTHCW3sEPy1VBRopNNp
A daemon that fails during startup reported the reason via eprintln! in
main's error arm. Under launchd stderr is discarded, so the failure was
invisible: daemon.log was created and left empty — exactly the case the
log file was added for.
Found by the manual acceptance run: the launchd agent loaded correctly
(RunAtLoad fired, runs=9) but every spawn exited 1 because a previously
started daemon held the pidfile, and nothing recorded why.
Fatal errors on the daemon path now go through tracing::error!, reaching
both the log file and stderr. Other subcommands keep eprintln!, since
their stderr is the user's terminal.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EGntTHCW3sEPy1VBRopNNp
service-manager 0.11 writes Disabled: true into every plist carrying
KeepAlive, and our RestartPolicy::Always guarantees KeepAlive, so every
agent we installed was born disabled. launchctl load honours the key while
still exiting 0, so install reported success on an agent that would never
start — not then and not at the next login.
service::start now removes the Disabled key itself before loading, via a
pure enable_plist() that rewrites nothing when the key is absent and
preserves every other key, including the EnvironmentVariables PATH
snapshot. That makes start self-healing for plists left disabled by an
earlier build. The crate's own start() is still not used, since without
Disabled it degrades to launchctl start, which fails on an unloaded job.
Because launchctl load exits 0 on failure, start also checks a
post-condition: it asks launchd whether the job now exists and reports a
diagnostic if it does not. stop keeps no such check, since a benign unload
of an already-stopped job also prints a failure while exiting 0.
status now treats the plist on disk as the definition of installed, as the
spec says: a plist that exists but is not loaded reports stopped with its
program, PATH and snapshot date intact instead of collapsing to
not-installed with every field cleared. That is precisely the state the
Disabled bug left users in, so it is the state status most needs to
describe.
status also gains the log path the spec always listed, and the daemon's own
log is renamed xy.log -> daemon.log so a supervised server named xy cannot
share a file, and two rotation counters, with the daemon.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EGntTHCW3sEPy1VBRopNNp
Three review findings on task 4:
- read_plist_fields now parses the plist properly via the plist crate
(already a transitive dep of service-manager, promoted to direct) instead
of slicing raw XML, so a PATH or program path containing & or < no longer
round-trips as literal &/< through cli::service::render_status.
- launchctl's error path now includes both stdout and stderr, trimmed and
joined only on non-empty parts, so a failure never surfaces as
"launchctl load failed: " with nothing after the colon.
- status() now returns identical None fields (program, path_env,
snapshotted, pid) whether the plist is absent or the plist exists but the
crate reports NotInstalled (e.g. written but never loaded, or booted out
of band) - extracted via a shared not_installed_status/build_status split
so both paths run the same code.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EGntTHCW3sEPy1VBRopNNp
PidFile::acquire used create_new(true) with Drop-based cleanup, so a
pidfile surviving power loss or SIGKILL made the daemon refuse to start
until the file was deleted by hand.
On AlreadyExists, read the recorded PID and probe it with kill(pid, 0):
ESRCH (or unparseable content) means stale, so remove the file and
retry the atomic create. A live PID keeps the refusal and now names the
holding process. The retry loop is bounded to stay race-safe against a
concurrent starter.
Closes#1
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ensure_dirs() now creates config_dir alongside state_dir and log_dir,
so first daemon run materializes $XDG_CONFIG_HOME/xy/servers/ — making
it obvious where to drop server .kdl files.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add StartAck::SpawnFailed(String) so callers can distinguish a successful
start from a failed spawn. The Start command arm now sends SpawnFailed on
io::Error rather than the misleading Started. handlers.rs maps the new
variant to an RpcErrorCode::SpawnFailed JSON-RPC error response.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the bare sleep(delay).await in the Restart backoff arm with a
tokio::select! over the timer and cmd_rx. Stop/Shutdown are now handled
immediately during backoff (Stop → Stopped, Shutdown → clean exit);
Start/Restart/Reconfigure skip the remaining delay and retry at once.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace watch::Receiver<ServerState> on SupervisorHandle with watch::Receiver<Status>,
a richer snapshot type that carries pid, port, uptime_secs, restart_count and last_exit.
SupervisorTask maintains current_pid and publishes a fresh Status on every state
transition; handlers.rs reads the full Status so list/status no longer return
zeroed/None fields.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fix a deadlock in the log-stream handler that caused all logs
requests to hang: Connection used a single Mutex<JsonFramed> for
both reads and writes, so the serve loop holding the read lock
blocked the spawned notification task from writing. Split
Connection into separate reader and writer mutexes.
Also fix a response/notification ordering race: the log task now
waits for an explicit ready signal sent by serve after writing the
LOGS response, ensuring notifications never arrive at the client
before their initiating response.
Replace bail!("not implemented") stubs with real RPC calls over the Unix
socket; add format::list_table for fixed-width list output.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implement per-connection ConnState tracking active subscriptions, and the
logs/logs_cancel RPC handlers. Snapshot-only streams terminate with a
log_end notification; follow streams forward broadcast lines until
cancelled or connection close.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Implements the `reload` JSON-RPC method: diffs the on-disk config dir
against the in-memory registry and reconciles — stops removed servers,
restarts changed servers (shutdown-then-respawn), and starts new ones.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Per-connection JSON-RPC dispatch in daemon/handlers.rs — list, status,
start, stop, and restart are fully implemented; reload, logs, and
logs_cancel are stubbed with -32601 for later tasks.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
One async task per managed server owns all state transitions via a
tokio::select! loop over cmd_rx and wait_child. Includes RealSpawner
and a smoke test covering the Start → Running → exit → Stopped →
Shutdown happy path.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Append RealChild (real tokio::process::Child wrapper) and spawn_with_logs
to child.rs. Uses nix::unistd::setpgid via tokio's re-exported pre_exec
to create an own process group, and fires per-stream log pump tasks that
drain stdout/stderr into the provided LogSink. terminate/kill signal the
whole process group via kill(-pgid, SIG*).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>