diff --git a/CLAUDE.md b/CLAUDE.md index 89e8336..9ca3e97 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -34,6 +34,13 @@ Bach is a CLI-first personal agentic development session launcher that turns dai - `/board` (REPL) - Kanban board now shows liveness dots (●/◐/✓) and observer summary lines on executing/qa cards; `⚑ needs you` marker when observer flags needs_human - `/show ` (REPL) - Task detail now includes an Observer section (summary, confidence, needs_human, session state) when the observer has data +### PRD#10 — Persistent Watcher Daemon +- `uv run bach daemon run` - Start the foreground supervisor loop; reconciles live task-linked sessions vs supervised watcher processes every `daemon-reconcile-seconds` (default 30); single-instance guarded; exits cleanly on SIGTERM/SIGINT +- `uv run bach daemon status` - Report whether the daemon is running (pid) and list the supervised artifact set (derived from the process table) +- `uv run bach daemon stop` - Send SIGTERM to the recorded daemon pid and clear the pidfile +- `uv run bach daemon install` - Install `com.bach.daemon` as a macOS LaunchAgent (RunAtLoad + KeepAlive); prints `bach daemon run` fallback if launchctl fails +- `uv run bach daemon uninstall` - Unload and remove the LaunchAgent plist; idempotent + ## File References - `docs/internal/DISCOVERY.md` - real goal and scope - `docs/internal/PRD.md` - requirements diff --git a/docs/adr/017-persistent-watcher-daemon.md b/docs/adr/017-persistent-watcher-daemon.md new file mode 100644 index 0000000..2622992 --- /dev/null +++ b/docs/adr/017-persistent-watcher-daemon.md @@ -0,0 +1,239 @@ +# ADR-017 — Persistent Watcher Daemon (PRD #10) + +**Status:** Accepted +**Date:** 2026-06-13 +**Extends:** ADR-015 (session observability, watcher-per-session run-and-exit), + ADR-016 (loop control, AFK runner). + +## Context + +ADR-015 introduced a per-session watcher (`session_watcher.py`) that runs as a +foreground, run-and-exit process. It is spawned in one of two ways: + +1. **watch_on_launch** — `task_service.launch_task` spawns a detached watcher + when the user has set `watch_on_launch: on`. This is per-launch, per-user. +2. **`bach sessions watch`** — the user manually runs the watcher for a named + session. + +Both mechanisms share the same limitation: if the user restarts their machine, +closes their terminal, or starts a new session from a different terminal window, +no watcher is running and observations go dark. There is no process that +reconciles the desired supervision state against what is actually running. + +PRD #10 adds a **persistent daemon** that closes this gap. The daemon is a +long-running supervisor that continuously ensures one watcher is alive for every +live, task-linked session — without requiring the user to manually spawn or +re-spawn watchers. + +Two design questions arose that required explicit decisions: + +1. **What is the supervision state model?** Naively, a bookkeeping file could + record which artifacts have a running watcher. But a state file introduces + failure modes: it can go stale after a crash, diverge from reality, and + cannot be queried atomically with process existence checks. + +2. **Who terminates supervised watchers?** The daemon could actively kill + watchers when their session ends. But watchers already self-terminate on + session end (via spool events and transcript EOF). An active kill path would + duplicate responsibility and risk killing a watcher in mid-observation. + +## Decision + +### 1. Supervisor-of-subprocesses: thin reconcile loop + +The daemon runs a **single-threaded synchronous reconcile loop** with no +threads and no asyncio. Each cycle: + +1. Compute the **desired set** — artifact paths whose sessions are live and + task-linked (not ended, not in a terminal status). +2. Compute the **supervised set** — artifact paths that currently have a live + `session-watch` child process (see §2 below). +3. Compute the **gap** = desired − supervised. +4. Call `spawn_watcher(artifact)` for each artifact in the gap. +5. Sleep `daemon_reconcile_seconds` (default 30 s). + +Each session watcher is a separate detached process (`bach internal +session-watch `) — the existing `session_watcher.py` run-and-exit +process, completely unchanged. The daemon is a thin coordination layer above +the already-working watcher processes; it adds no new observation logic. + +Rationale for single-threaded synchronous design: +- One cycle is O(n) in the number of active sessions, typically < 5 processes + for a single developer. No concurrency benefit exists at this scale. +- Threads or asyncio would add cancellation complexity, exception propagation + issues, and test infrastructure overhead that far exceeds the value. +- The supervisor loop is fully injectable (all side-effecting deps are + keyword-only args to `run_supervisor`) so it is trivially unit-testable with + fakes and no real subprocess/time involvement. + +### 2. Supervision state is derived from the OS process table + +**The single source of truth for "which artifacts are supervised" is the OS +process table, not a bookkeeping file.** + +`list_supervised_artifacts()` (in `runtimes/watcher_processes.py`) calls +`ps -axo pid,command`, scans for lines containing the unique marker string +`"internal session-watch"`, and extracts the artifact path argument. The result +is a `set[Path]` of currently-watched artifacts. + +This is the load-bearing invariant. The reasons it was chosen over a state +file: + +- **Auto-dedup vs `watch_on_launch`.** If the user has `watch_on_launch: on` + and the daemon is also running, the per-launch watcher and the daemon's + watcher would both try to supervise the same artifact. The ps-derived + supervised set naturally deduplicates: if `watch_on_launch` already spawned + a watcher, it appears in `ps` and the daemon's reconcile skips it. No + coordination between the two spawn paths is needed. +- **Restart-safe.** If the daemon crashes and restarts, the supervised set is + reconstructed from the actual running processes. Orphaned watchers from the + previous daemon run are discovered and counted; there is no orphan list to + reconcile with. +- **No state file to corrupt.** A bookkeeping file can go out of sync with + reality. `ps` cannot. + +The consequence of this design is the **no-kill-path invariant**: because the +supervised set is derived from `ps`, the daemon does not track watcher PIDs +individually. It sees "artifact X has a watcher" but not "watcher PID is +12345". There is therefore no mechanism to selectively terminate a specific +watcher. Watchers self-terminate when their session ends (via spool events or +transcript EOF from ADR-015 §3). Ended or terminal-status artifacts drop from +the desired set; they are simply ignored on the next cycle. The watcher dies +on its own, and on the cycle after that, it disappears from `ps` and is not +missed. + +### 3. Crash backoff + +If a watcher crashes immediately after spawning (e.g. bad artifact state, disk +error), the reconcile loop would respawn it every cycle, hammering the process +table. The daemon uses per-artifact exponential backoff to prevent this. + +After each spawn attempt (success or failure), a backoff entry is recorded in +an **in-memory dict** (not a file). The next spawn for that artifact is +suppressed until the cooldown window expires. The window starts at 10 s and +doubles on each successive crash, capped at 5 minutes. + +The backoff state is reset on daemon restart because it is in-memory. This is +deliberate: a fresh daemon restart (via KeepAlive or manual `bach daemon run`) +gives every artifact a clean slate. The KeepAlive restart boundary naturally +bounds how long stale backoff state persists. + +### 4. Single-instance guard + +`acquire()` in `services/daemon_lifecycle.py` reads a pidfile at +`~/.bach/daemon.pid`, checks liveness of the recorded pid via `os.kill(pid, 0)`, +and either: +- Writes our PID and returns success. +- Returns a refusal with the live PID. +- Reclaims a stale pidfile (dead PID or corrupt content) and returns success. + +Stale pidfile reclaim is automatic so a crashed daemon never permanently +blocks a restart. + +### 5. macOS launchd integration + +`bach daemon install` writes a LaunchAgent plist to `~/Library/LaunchAgents/` +and loads it via `launchctl load`. The plist uses: + +- `RunAtLoad: true` — starts the daemon on user login without manual action. +- `KeepAlive: true` — launchd restarts the daemon if it crashes. +- `StandardOutPath` / `StandardErrorPath` — daemon output goes to + `~/.bach/logs/daemon.stdout.log` / `daemon.stderr.log`. + +On `launchctl` failure (non-macOS, SIP restrictions, sandboxed environment), +the plist is still written and a fallback command (`bach daemon run`) is +printed so the user always has a manual path. + +`bach daemon uninstall` unloads and removes the plist. Both are idempotent. + +## Known limitations / accepted trade-offs + +*These are deliberate design choices, not bugs.* + +**(a) Pidfile acquire has a TOCTOU window.** +Between reading the pidfile and writing it, another process could write its own +PID (a race). This is accepted for a single-user macOS development tool: +the only realistic concurrent caller is the user running `bach daemon run` +twice in rapid succession, which is a user error that results in one of them +refusing cleanly. Using `flock()` would add platform-specific code and a +dependency on `fcntl` with no real safety gain for a single-user tool. + +**(b) `ps` may truncate long artifact paths.** +On some platforms, `ps` truncates command lines beyond ~1000 characters. A +task artifact with a path longer than this would appear in `ps` with a +truncated command string, causing `list_supervised_artifacts` to miss it and +the daemon to attempt a duplicate spawn. At most one extra spawn would occur: +on the following cycle, the newly-spawned watcher appears in `ps` with the +full path (it was spawned from this machine with the full argv), so it is +detected and the artifact is no longer in the gap. The consequence is one +redundant spawn, not an infinite loop. This is accepted over using +platform-specific proc APIs (Linux `/proc//cmdline`, macOS +`libproc.h`) which would fragment the codebase for a purely theoretical +edge case. + +**(c) Backoff state is in-memory; it does not clear when a watcher recovers.** +If a watcher crashes once, accumulates a backoff cooldown, then succeeds on the +next attempt, the backoff entry persists in memory until the daemon restarts. +On the next restart the backoff dict is empty, so the artifact gets a clean +slate immediately. The `KeepAlive` restart boundary means staleness is bounded +by the daemon restart frequency. This is conservative-by-design: a slightly +extended cooldown after a recovery is preferable to hammering a flaky artifact +with no delay. + +## Consequences + +- **`bach daemon run` is a foreground command.** It runs until SIGTERM or + SIGINT. The launchd LaunchAgent is the intended production deployment; + foreground is for development and testing. + +- **No daemon required for basic use.** The existing `watch_on_launch` path + and `bach sessions watch` remain fully functional. The daemon is an opt-in + enhancement for persistent supervision. + +- **`daemon_reconcile_seconds` is a new config knob** (default 30). Lower + values mean faster gap detection but more `ps` calls. Tunable via + `bach config set daemon-reconcile-seconds `. + +- **Watcher logs accumulate.** Each watcher's stdout/stderr is appended to + `~/.bach/logs/watcher-.log`. The daemon's own output goes to + `daemon.stdout.log` / `daemon.stderr.log` under the same directory. GC is + a future concern. + +- **SSH check-in workflow is pull-only.** The daemon writes observer verdicts + to task artifacts and the board. There are no push notifications. A user + returning over SSH runs `bach sessions list`, `/board`, or `bach daemon status` + to pull current state. See `docs/how-to/run-the-daemon.md`. + +## Alternatives rejected + +- **Daemon with in-process threads / asyncio.** Threading or asyncio would + allow concurrent watcher goroutines but introduce cancellation complexity, + shared-state races, and test overhead that far exceeds the benefit for a + single-developer tool running O(5) concurrent sessions. + +- **Bookkeeping file as supervision state.** A JSON or YAML file recording + watched artifacts would be simpler to query but would go stale after crashes + and require its own GC and consistency maintenance. The process table is + always fresh and never lies. + +- **Daemon kills watchers on session end.** Adding a kill path would require + tracking individual watcher PIDs and handling races between kill and natural + self-termination. Watchers already self-terminate reliably (ADR-015 §3); + adding a redundant kill path would increase complexity with no correctness + benefit. + +- **flock() for single-instance guard.** Adds `fcntl` import (POSIX-only), + held-lock semantics, and platform-specific cleanup. The pidfile approach with + stale reclaim gives identical single-user safety at half the complexity. + +## References + +- ADR-015: `adr/015-session-observability-codex-parity.md` +- ADR-016: `adr/016-loop-control.md` +- Implementation: `src/bach/services/daemon_supervisor.py` (reconcile + run_supervisor + backoff) +- Implementation: `src/bach/runtimes/watcher_processes.py` (spawn_watcher, list_supervised_artifacts) +- Implementation: `src/bach/services/daemon_lifecycle.py` (pidfile single-instance guard) +- Implementation: `src/bach/services/launchd_service.py` (plist render + install/uninstall) +- Implementation: `src/bach/cli/daemon_cmd.py` (run/status/stop/install/uninstall) +- Config: `src/bach/config/settings.py` (`daemon_reconcile_seconds`) +- How-to: `docs/how-to/run-the-daemon.md` diff --git a/docs/how-to/run-the-daemon.md b/docs/how-to/run-the-daemon.md new file mode 100644 index 0000000..095296b --- /dev/null +++ b/docs/how-to/run-the-daemon.md @@ -0,0 +1,165 @@ +# Run the Persistent Watcher Daemon + +Set up Bach's background daemon in about 2 minutes: one install command, and +Bach will keep a session watcher alive for every live task — even across +reboots and terminal restarts. + +--- + +## Prerequisites + +- Bach installed and `uv run bach --help` works. +- At least one project registered (`bach project add `). +- Hooks installed for the runtimes you use (`bach hooks install claude-code --project `). +- macOS (the launchd path). Linux / other POSIX: use the foreground fallback. + +--- + +## Step-by-step + +### 1. Install the daemon as a LaunchAgent + +```bash +bach daemon install +``` + +This writes `~/Library/LaunchAgents/com.bach.daemon.plist` and loads it via +`launchctl`. The daemon starts immediately and will restart automatically on +login or crash. + +If `launchctl` fails (sandboxed environment, non-macOS, etc.) the plist is +still written and Bach prints the manual fallback: + +``` +Run manually (or under tmux / screen): bach daemon run +``` + +Keep that command handy — you can paste it into a persistent tmux session. + +Verify the daemon is running: + +```bash +bach daemon status +``` + +You should see something like: + +``` +running pid 12345 + +supervised artifacts (2): + my-task.md + other-task.md +``` + +--- + +### 2. Walk away + +Once the daemon is running you do not need to do anything else. For every task +session that is launched: + +- If `watch_on_launch` is on, a watcher is spawned at launch time as usual. +- If no watcher is present (session started outside Bach, machine restarted, + watcher crashed), the daemon detects the gap within one reconcile cycle + (default 30 seconds) and spawns a replacement. + +Watchers self-terminate when their session ends. The daemon never kills +watchers — it only fills gaps. + +--- + +### 3. Return over SSH and check in + +The daemon writes observer verdicts to task artifacts and the board. There are +**no push notifications** — you pull status when you return. + +Open a new SSH session and run any of these: + +```bash +# Quick overview: running state + supervised artifact count +bach daemon status + +# Full session list with liveness dots and task links +bach sessions list + +# Kanban board with observer summary lines and ⚑ needs you markers +bach +bach > /board +``` + +If an artifact shows `⚑ needs you` on the board or in `bach sessions list`, +the observer has flagged that the agent is blocked. Resume the session to +unblock it. + +--- + +### 4. Resume an idle session + +When you see a session you want to re-enter, grab its session ID from +`bach sessions list` (e.g. `s3`) and run: + +```bash +bach sessions resume s3 +``` + +This opens the session in a new iTerm window. If iTerm automation is +unavailable, Bach prints the resume command so you can paste it manually: + +``` +Resume manually: claude --resume +``` + +--- + +## Tuning + +The reconcile interval is how often the daemon checks for gaps: + +```bash +bach config set daemon-reconcile-seconds 10 # check every 10 s (faster) +bach config set daemon-reconcile-seconds 60 # check every 60 s (quieter) +``` + +Default is 30 seconds. Lower values mean faster watcher recovery but more +`ps` calls. + +--- + +## Stopping and removing the daemon + +Stop a running daemon without removing the LaunchAgent: + +```bash +bach daemon stop +``` + +Fully remove the LaunchAgent (stop + uninstall): + +```bash +bach daemon uninstall +``` + +After uninstall, `bach daemon install` re-registers it from scratch. + +--- + +## Troubleshooting + +| Symptom | Check | +|---------|-------| +| `bach daemon status` shows not running after install | Check `~/.bach/logs/daemon.stderr.log` for startup errors | +| Supervised artifact count is 0 when sessions are live | Confirm hooks are installed: `bach hooks status --project ` | +| Daemon stops supervising after a reboot | Confirm the LaunchAgent loaded: `launchctl list \| grep com.bach.daemon` | +| Watcher keeps crashing and not recovering | Backoff is active — check `~/.bach/logs/watcher-.log`; daemon retries after a cooldown | +| `bach daemon stop` says no daemon running | Pidfile was already cleared (crash or prior stop) — `bach daemon run` or reinstall | + +--- + +## Related docs + +- `enable-observer.md` — hooks, watch-on-launch, board liveness dots +- `wire-up-session-hook.md` — full hook reference, loop-gate +- `use-the-repl.md` — /board, /show, /open commands +- `../adr/017-persistent-watcher-daemon.md` — daemon architecture +- `../adr/015-session-observability-codex-parity.md` — watcher + observer design diff --git a/src/bach/cli/app.py b/src/bach/cli/app.py index 24576d3..b4b3f4e 100644 --- a/src/bach/cli/app.py +++ b/src/bach/cli/app.py @@ -7,6 +7,7 @@ import typer from rich.console import Console +from bach.cli.daemon_cmd import daemon_app from bach.cli.hooks_cmd import hooks_app from bach.cli.internal_sessions import register_internal_sessions from bach.cli.issue import issue_app @@ -73,6 +74,8 @@ # like issue_app and hooks_app. The old try/except guard was for partial-phase dev. app.add_typer(sessions_app, name="sessions") app.add_typer(hooks_app, name="hooks") +# Phase 17 (issue #12): daemon supervisor sub-app. +app.add_typer(daemon_app, name="daemon") # Internal hook-event + session-watch + session-stop commands. register_internal_sessions(internal_app) # Phase 16 L1: `bach afk run` sub-app. afk_runner.py (L2) provides the logic. diff --git a/src/bach/cli/daemon_cmd.py b/src/bach/cli/daemon_cmd.py new file mode 100644 index 0000000..c37573e --- /dev/null +++ b/src/bach/cli/daemon_cmd.py @@ -0,0 +1,378 @@ +"""Typer sub-app for `bach daemon ...` (PRD #10, issue #15). + +Subcommands: + run — foreground supervisor: keeps one watcher alive per live task session. + status — report daemon running state + supervised artifacts. + stop — gracefully stop the running daemon via SIGTERM. + install — install the daemon as a macOS LaunchAgent (RunAtLoad + KeepAlive). + uninstall — remove the LaunchAgent. + +Registration: wired in cli/app.py via: + app.add_typer(daemon_app, name="daemon") + +Architecture notes: + - Handler is THIN: just loads config, wires seams, calls service functions. + - All heavy logic lives in services/daemon_supervisor.py (loop + reconcile). + - Single-instance guard lives in services/daemon_lifecycle.py (pidfile). + - SIGTERM/SIGINT handler sets a threading.Event; should_stop_fn checks it — + the loop exits cleanly at the top of the next cycle rather than mid-spawn. + - `run` acquires single-instance ownership on start; clears pidfile on exit + (via finally block) so a crashed daemon never permanently blocks restart. + - `status` reads the pidfile + queries ps for supervised artifacts; read-only. + - `stop` sends SIGTERM; daemon_lifecycle handles the actual kill + cleanup. + - No real sleep / real process spawning in tests — all seams are injectable. +""" + +import logging +import os +import signal +import threading +import time +from datetime import UTC, datetime +from pathlib import Path + +import typer +from rich.console import Console + +from bach.config.paths import bach_home +from bach.config.projects import ProjectRegistry +from bach.config.settings import load_config +from bach.config.themes import THEMES, ThemePalette, get_theme +from bach.runtimes.session_discovery import default_process_probe, discover_sessions +from bach.runtimes.transcript import read_tail_events +from bach.runtimes.watcher_processes import ( + list_supervised_artifacts, + spawn_watcher, +) +from bach.services.daemon_lifecycle import ( + DaemonStatus, + acquire, + daemon_status, + stop, +) +from bach.services.daemon_supervisor import run_supervisor +from bach.services.launchd_service import ( + install_launch_agent, + uninstall_launch_agent, +) +from bach.services.sessions_service import ( + list_sessions, + list_supervisable_artifacts, + scan_registry_artifact_paths, +) + +logger = logging.getLogger(__name__) + +# Typer sub-app mounted at `bach daemon` by cli/app.py. +daemon_app = typer.Typer(help="Background daemon supervisor for session watchers.") + +_console = Console(highlight=False) + + +def _theme() -> ThemePalette: + """Load the active theme palette (same pattern as cli/sessions.py).""" + return get_theme(load_config().theme) or THEMES["default"] + + +def _pidfile_path() -> Path: + """Canonical path for the daemon pidfile under bach_home(). + + Centralised so tests can patch a single name to redirect all pidfile I/O. + """ + return bach_home() / "daemon.pid" + + +# --------------------------------------------------------------------------- +# `bach daemon run` +# --------------------------------------------------------------------------- + + +@daemon_app.command("run") +def daemon_run() -> None: + """Start the foreground daemon supervisor. + + Acquires single-instance ownership via a pidfile. Refuses to start if a + live daemon is already recorded — use `bach daemon stop` first. + + Monitors all live, task-linked sessions and ensures each has an active + watcher process. Exits cleanly on SIGTERM or SIGINT (Ctrl-C), clearing + the pidfile on shutdown. + + This is a FOREGROUND command — it runs until interrupted. For a background + deployment, wrap it in a process supervisor (launchd, systemd, etc.). + Use `daemon-reconcile-seconds` config knob to tune the cycle interval. + + Examples: + bach daemon run # defaults from ~/.bach/config.yaml + bach config set daemon-reconcile-seconds 10 + bach daemon run # faster cycles + """ + cfg = load_config() + t = _theme() + pidfile = _pidfile_path() + + # --------------------------------------------------------------------------- + # Single-instance guard: refuse if a live daemon is already running. + # Uses the pidfile path returned by _pidfile_path() — centralised so tests + # can redirect all pidfile I/O with a single patch. The real liveness probe + # (default) is used here; tests inject a fake acquire callable instead. + # --------------------------------------------------------------------------- + ok, existing_pid = acquire( + pidfile=pidfile, + our_pid=os.getpid(), + ) + if not ok: + _console.print( + f"[{t.error}]error[/{t.error}] " + f"daemon is already running (pid {existing_pid}). " + "Use `bach daemon stop` to stop it first." + ) + raise typer.Exit(code=1) + + logger.info("event=daemon_acquire_ok pid=%d", os.getpid()) + + # --------------------------------------------------------------------------- + # Clean shutdown: threading.Event shared between the signal handler and the + # should_stop_fn. Using an Event (not a plain bool) makes the set() visible + # across Python's signal-delivery boundary without needing a lock. + # The loop checks the flag at the top of each cycle so a SIGTERM received + # mid-spawn does not interrupt a spawn in progress. + # --------------------------------------------------------------------------- + _stop_event = threading.Event() + + def _signal_handler(signum: int, frame: object) -> None: + """Set the stop flag; the supervisor loop exits at the next cycle top.""" + logger.info("event=daemon_signal_received signum=%d", signum) + _stop_event.set() + + # Register for both SIGTERM (systemd/launchd stop) and SIGINT (Ctrl-C). + signal.signal(signal.SIGTERM, _signal_handler) + signal.signal(signal.SIGINT, _signal_handler) + + def should_stop() -> bool: + return _stop_event.is_set() + + # --------------------------------------------------------------------------- + # enumerate_desired_fn: builds the desired set each cycle by joining live + # session discovery with registered project artifacts, then filtering to + # supervisable rows (task-linked, not ended, not in a terminal status). + # Config is re-read inside the closure so a hot change to liveness thresholds + # takes effect on the next cycle without restarting the daemon. + # --------------------------------------------------------------------------- + def enumerate_desired() -> set[Path]: + """Enumerate artifacts that need a live watcher this cycle.""" + registry = ProjectRegistry.default() + artifact_paths = scan_registry_artifact_paths(registry) + + # Discover live sessions with the same thresholds as `bach sessions list`. + current_cfg = load_config() + discovered_sessions = discover_sessions( + claude_projects_dir=Path.home() / ".claude" / "projects", + codex_home=Path.home() / ".codex", + process_probe=default_process_probe, + now=datetime.now(UTC), + running_threshold_s=current_cfg.liveness_running_seconds, + idle_threshold_s=current_cfg.liveness_idle_minutes * 60, + ) + + # Join sessions with artifacts to get SessionRows. + rows = list_sessions( + sessions_dir=bach_home() / "sessions", + artifacts_dir=bach_home(), + discover_fn=lambda: discovered_sessions, + read_events_fn=read_tail_events, + artifact_paths=artifact_paths, + # Include all non-ended sessions — filter to supervisable below. + max_ended_age_s=0, # exclude ended immediately + ) + + return list_supervisable_artifacts(rows=rows) + + # --------------------------------------------------------------------------- + # spawn_fn: adapter that wraps spawn_watcher's (ok, log_path) return value + # into a simple no-return callable expected by run_supervisor. The supervisor + # catches exceptions from spawn_fn, so we only log here — never raise. + # --------------------------------------------------------------------------- + def spawn(artifact: Path) -> None: + """Spawn a detached watcher for the given artifact; log the result.""" + ok, log_path = spawn_watcher(artifact) + if ok: + logger.info( + "event=daemon_watcher_spawned artifact=%s log=%s", + artifact, + log_path, + ) + else: + logger.warning("event=daemon_watcher_spawn_failed artifact=%s", artifact) + + logger.info( + "event=daemon_run_start reconcile_seconds=%d", + cfg.daemon_reconcile_seconds, + ) + + try: + run_supervisor( + enumerate_desired_fn=enumerate_desired, + list_supervised_fn=list_supervised_artifacts, + spawn_fn=spawn, + sleep_fn=time.sleep, + should_stop_fn=should_stop, + reconcile_seconds=cfg.daemon_reconcile_seconds, + ) + finally: + # Always clear the pidfile on exit so `daemon run` can restart cleanly. + pidfile.unlink(missing_ok=True) + logger.info("event=daemon_run_done") + + +# --------------------------------------------------------------------------- +# `bach daemon status` +# --------------------------------------------------------------------------- + + +@daemon_app.command("status") +def daemon_status_cmd() -> None: + """Report whether the daemon is running and which artifacts it supervises. + + Shows: + - Running state (live pid or 'not running'). + - The set of task artifacts currently being watched. + + Examples: + bach daemon status + """ + t = _theme() + pidfile = _pidfile_path() + + status: DaemonStatus = daemon_status(pidfile=pidfile) + + if status.running: + _console.print(f"[{t.success}]running[/{t.success}] pid {status.pid}") + else: + _console.print(f"[{t.muted}]not running[/{t.muted}]") + + # Report the supervised artifact set regardless of daemon state so the + # user can see orphaned watchers if any are still alive. + supervised = list_supervised_artifacts() + if supervised: + _console.print(f"\n[{t.accent}]supervised artifacts ({len(supervised)}):[/{t.accent}]") + for artifact in sorted(supervised): + _console.print(f" {artifact.name}") + else: + _console.print(f"[{t.muted}]no artifacts currently supervised[/{t.muted}]") + + +# --------------------------------------------------------------------------- +# `bach daemon stop` +# --------------------------------------------------------------------------- + + +@daemon_app.command("stop") +def daemon_stop_cmd() -> None: + """Stop the running daemon by sending SIGTERM to the recorded pid. + + Reads the pidfile, checks liveness, and either: + - Sends SIGTERM to the live daemon (and clears the pidfile). + - Prints an informative message if no daemon is running. + + Safe to call even when no daemon is running — never raises. + + Examples: + bach daemon stop + """ + t = _theme() + pidfile = _pidfile_path() + + status: DaemonStatus = daemon_status(pidfile=pidfile) + + if not status.running: + _console.print(f"[{t.muted}]no daemon running[/{t.muted}]") + return + + # Delegate to daemon_lifecycle.stop for the actual SIGTERM + cleanup. + stop(pidfile=pidfile) + _console.print( + f"[{t.success}]stopped[/{t.success}] daemon (pid {status.pid})" + ) + logger.info("event=daemon_stop_cmd pid=%d", status.pid) + + +# --------------------------------------------------------------------------- +# `bach daemon install` +# --------------------------------------------------------------------------- + + +@daemon_app.command("install") +def daemon_install() -> None: + """Install the Bach daemon as a macOS LaunchAgent. + + Writes a plist to ~/Library/LaunchAgents/ and loads it via launchctl so + the daemon starts on login and self-restarts on crash (RunAtLoad + KeepAlive). + + This is an EXPLICIT install — nothing auto-installs the LaunchAgent. + Run `bach daemon uninstall` to remove it. + + If launchctl is unavailable (non-macOS, sandboxed environment, etc.) the + plist is still written and the manual fallback command is printed so you + always have a path to run the daemon. + + Examples: + bach daemon install + """ + t = _theme() + result = install_launch_agent() + + if not result.installed: + # Unexpected failure writing the plist itself — surface the fallback. + _console.print(f"[{t.error}]error[/{t.error}] failed to write LaunchAgent plist.") + if result.fallback_command: + _console.print( + f"[{t.muted}]Run manually (or under tmux):[/{t.muted}] " + f"{result.fallback_command}" + ) + logger.warning("event=daemon_install_failed") + raise typer.Exit(code=1) + + if result.fallback_command: + # Plist written but launchctl load failed — print fallback prominently. + _console.print( + f"[{t.accent}]installed[/{t.accent}] LaunchAgent plist written. " + "However, launchctl failed to load it automatically." + ) + _console.print( + f"[{t.muted}]Run manually (or under tmux / screen):[/{t.muted}] " + f"{result.fallback_command}" + ) + logger.warning( + "event=daemon_install_launchctl_failed fallback=%r", result.fallback_command + ) + else: + _console.print( + f"[{t.success}]installed[/{t.success}] Bach daemon LaunchAgent loaded successfully. " + "The daemon will start on login and self-restart on crash." + ) + logger.info("event=daemon_install_ok") + + +# --------------------------------------------------------------------------- +# `bach daemon uninstall` +# --------------------------------------------------------------------------- + + +@daemon_app.command("uninstall") +def daemon_uninstall() -> None: + """Remove the Bach daemon LaunchAgent. + + Unloads the agent from launchctl and deletes the plist file from + ~/Library/LaunchAgents/. Idempotent — safe to call when the agent is + not installed. + + Examples: + bach daemon uninstall + """ + t = _theme() + uninstall_launch_agent() + _console.print( + f"[{t.success}]uninstalled[/{t.success}] Bach daemon LaunchAgent removed." + ) + logger.info("event=daemon_uninstall_ok") diff --git a/src/bach/config/settings.py b/src/bach/config/settings.py index 8d91ad8..945293c 100644 --- a/src/bach/config/settings.py +++ b/src/bach/config/settings.py @@ -108,6 +108,14 @@ class BachConfig: # Hard time budget (minutes) for the AFK runner. Enforced alongside # afk_max_turns — whichever limit fires first stops the loop. afk_time_budget_minutes: int = 60 + # --------------------------------------------------------------------------- + # Phase 17 — Daemon supervisor (issue #12) + # --------------------------------------------------------------------------- + # Seconds between reconcile cycles in `bach daemon run`. A shorter interval + # means new sessions are picked up faster; a longer interval reduces process + # table scan overhead. 30s is a sensible default — new sessions are adopted + # within one cycle, and one extra scan per 30s is negligible overhead. + daemon_reconcile_seconds: int = 30 def _default_config_path() -> Path: @@ -179,6 +187,9 @@ def load_config(path: Path | None = None) -> BachConfig: afk_time_budget_minutes = _coerce_positive_int( parsed.get("afk_time_budget_minutes"), "afk_time_budget_minutes", default=60 ) + daemon_reconcile_seconds = _coerce_positive_int( + parsed.get("daemon_reconcile_seconds"), "daemon_reconcile_seconds", default=30 + ) config = BachConfig( iterm_layout=layout, concurrency=concurrency, @@ -194,6 +205,7 @@ def load_config(path: Path | None = None) -> BachConfig: liveness_idle_minutes=liveness_idle_minutes, afk_max_turns=afk_max_turns, afk_time_budget_minutes=afk_time_budget_minutes, + daemon_reconcile_seconds=daemon_reconcile_seconds, ) logger.info( @@ -201,7 +213,8 @@ def load_config(path: Path | None = None) -> BachConfig: "runtime_default=%s normalize_model=%s theme=%s user_name=%s " "watch_on_launch=%s observer_moves=%s judge_confidence_threshold=%s " "judge_interval_seconds=%d liveness_running_seconds=%d " - "liveness_idle_minutes=%d afk_max_turns=%d afk_time_budget_minutes=%d", + "liveness_idle_minutes=%d afk_max_turns=%d afk_time_budget_minutes=%d " + "daemon_reconcile_seconds=%d", config_path, config.iterm_layout.value, config.concurrency, @@ -217,6 +230,7 @@ def load_config(path: Path | None = None) -> BachConfig: config.liveness_idle_minutes, config.afk_max_turns, config.afk_time_budget_minutes, + config.daemon_reconcile_seconds, ) return config @@ -517,6 +531,8 @@ def _parse_positive_int(raw: str) -> int: "liveness-idle-minutes": _parse_positive_int, "afk-max-turns": _parse_positive_int, "afk-time-budget-minutes": _parse_positive_int, + # Phase 17 — daemon supervisor cycle interval (issue #12) + "daemon-reconcile-seconds": _parse_positive_int, } # Map CLI-spelling (hyphen) → YAML/dataclass field name (underscore). diff --git a/src/bach/runtimes/watcher_processes.py b/src/bach/runtimes/watcher_processes.py new file mode 100644 index 0000000..0c413e8 --- /dev/null +++ b/src/bach/runtimes/watcher_processes.py @@ -0,0 +1,223 @@ +"""Process-table adapter for Bach session-watcher supervision. + +This module owns two things: + 1. spawn_watcher — detached Popen of `bach internal session-watch `. + Extracted from TaskService._spawn_session_watcher so it can be reused + outside the service layer (e.g. the AFK runner, supervision daemon). + + 2. list_supervised_artifacts — process-table scan that returns the set of + artifact Paths currently being watched by a live session-watch process. + Uses an injectable process_probe so tests stay fully hermetic. + +Why the process table is the supervision source of truth (not a bookkeeping file): + - The OS is always authoritative about what is actually running. + - A file-based approach would go stale after a crash and require its own GC. + - ps-based detection auto-deduplicates: if watch_on_launch already spawned a + watcher, the daemon sees it in ps and does not spawn a duplicate. + - Accepted trade-off: ps may truncate command lines for paths >~1000 chars, + causing at most one extra spawn before detection. See ADR-017 §2. + +Design mirrors session_discovery.py: + - process_probe is injected; default_process_probe() is the real impl. + - Pure parse helper (parse_supervised_artifacts) is separate so it can be + unit-tested with fake input without touching any OS state. + - warn-not-fail on every exception: caller's primary job must not be blocked. + +Session-watch process command signature (produced by spawn_watcher): + -m bach internal session-watch + +The marker string "internal session-watch" uniquely identifies these processes +in the process table — no other bach subcommand uses that exact phrase. +""" + +import logging +import subprocess +import sys +from pathlib import Path + +from bach.config.paths import bach_home +from bach.runtimes.session_discovery import ProcessProbe, default_process_probe + +logger = logging.getLogger(__name__) + +# The unique substring that identifies a live session-watch process in `ps` output. +# Anchored to the internal subcommand so unrelated Python processes don't match. +_SESSION_WATCH_MARKER = "internal session-watch" + + +# --------------------------------------------------------------------------- +# Pure parse helper — no I/O, fully unit-testable +# --------------------------------------------------------------------------- + + +def parse_supervised_artifacts( + processes: list[tuple[int, str]], +) -> set[Path]: + """Extract watched artifact Paths from a process-table snapshot. + + Scans (pid, command_line) pairs for lines containing the session-watch + marker and extracts the artifact path that follows it. Returns a set + so duplicate artifacts (same path, different PIDs) are deduplicated. + + Ignores: + - Lines without the session-watch marker. + - Lines where the marker appears but no argument follows it. + - Lines with an empty command string. + + Args: + processes: list of (pid, command_line) as returned by a ProcessProbe. + + Returns: + Set of artifact Paths extracted from matching process command lines. + """ + artifacts: set[Path] = set() + + for _pid, cmdline in processes: + if not cmdline: + continue + + marker_idx = cmdline.find(_SESSION_WATCH_MARKER) + if marker_idx == -1: + # Not a session-watch process — skip it. + continue + + # Everything after the marker + one space is the artifact path argument. + # e.g. "python -m bach internal session-watch /path/to/task.md" + # marker ends at idx+len(marker), then one space, then the path. + after_marker = cmdline[marker_idx + len(_SESSION_WATCH_MARKER):] + artifact_str = after_marker.strip() + + if not artifact_str: + # Marker present but no argument — malformed or incomplete line. + # This can happen if ps truncated a very long command line exactly at + # the marker boundary. Log at DEBUG because it is not actionable. + logger.debug( + "event=watcher_parse_no_arg cmdline=%r", + cmdline, + ) + continue + + # The path is everything after the marker in the ps output. + # spawn_watcher passes the path as a distinct argv element (no shell + # quoting), so ps shows it verbatim. We take the full remainder as the + # path — no further splitting needed. + artifacts.add(Path(artifact_str)) + + return artifacts + + +# --------------------------------------------------------------------------- +# Supervision scan — injectable probe +# --------------------------------------------------------------------------- + + +def list_supervised_artifacts( + *, + process_probe: ProcessProbe = default_process_probe, +) -> set[Path]: + """Return the set of artifact Paths currently watched by a live session-watch. + + Calls process_probe() once to get a process-table snapshot, then delegates + to parse_supervised_artifacts for the pure extraction logic. + + The default probe (default_process_probe) uses `ps -axo pid,command`. + Inject a fake probe in tests to keep tests hermetic and fast. + + Args: + process_probe: callable returning [(pid, cmdline)]. Defaults to the + real ps-based probe from session_discovery. + + Returns: + Set of artifact Paths with at least one live watcher process. + Returns an empty set when the probe fails — never raises. + """ + try: + processes = process_probe() + except Exception as exc: # noqa: BLE001 — probe failure must not crash caller + logger.warning( + "event=watcher_probe_failed reason=%s", + type(exc).__name__, + ) + return set() + + return parse_supervised_artifacts(processes) + + +# --------------------------------------------------------------------------- +# Detached watcher spawn +# --------------------------------------------------------------------------- + + +def spawn_watcher(artifact: Path) -> tuple[bool, Path | None]: + """Spawn `bach internal session-watch ` as a detached child. + + This is the single canonical place for spawning a watcher process. + Extracted from TaskService._spawn_session_watcher so any caller + (service layer, AFK runner, supervision daemon) can use it without + duplicating the log-redirect logic. + + Uses the current Python interpreter (-m bach) so the child inherits + the same venv and PATH without requiring `bach` to be on PATH. + + stdout/stderr are redirected (append mode) to: + bach_home() / "logs" / "watcher-.log" + + This is the ONLY trace of the detached process. DEVNULL would make + watcher crashes completely invisible — always use a real file. + + close_fds=True detaches the child from the parent's open file table + so it outlives this process cleanly. + + Best-effort / warn-not-fail: a spawn exception logs a warning but + NEVER blocks the caller. The caller decides whether to notify the user. + + Args: + artifact: Path to the task artifact that the watcher should observe. + + Returns: + (True, log_path) on successful spawn. + (False, None) on any exception. + """ + # Use the artifact stem for the log name so it's easy to correlate. + # Fallback to the full name if the stem is somehow empty. + stem = artifact.stem if artifact.stem else artifact.name + logs_dir = bach_home() / "logs" + logs_dir.mkdir(parents=True, exist_ok=True) # create on demand; idempotent + log_path = logs_dir / f"watcher-{stem}.log" + + try: + # Re-use the current interpreter so the watcher inherits the same + # venv environment without requiring `bach` to be on PATH. + cmd = [sys.executable, "-m", "bach", "internal", "session-watch", str(artifact)] + + # Append mode: successive watcher restarts accumulate in one file + # rather than overwriting previous diagnostic output. + log_fh = log_path.open("a", encoding="utf-8") + + subprocess.Popen( + cmd, + stdin=subprocess.DEVNULL, + stdout=log_fh, + stderr=log_fh, + close_fds=True, + ) + # Close the parent's copy of the fd immediately after fork. + # The child has its own inherited copy at fd 1/fd 2 via Popen. + # Leaving log_fh open in the parent leaks one fd per spawned watcher + # until CPython GC collects it. Explicit close is the correct practice. + log_fh.close() + + logger.info( + "event=session_watcher_spawned artifact=%s log=%s", + artifact, + log_path, + ) + return True, log_path + + except Exception as exc: # noqa: BLE001 — warn-not-fail, never block caller + logger.warning( + "event=session_watcher_spawn_failed artifact=%s reason=%s", + artifact, + type(exc).__name__, + ) + return False, None diff --git a/src/bach/services/daemon_lifecycle.py b/src/bach/services/daemon_lifecycle.py new file mode 100644 index 0000000..0ae09fa --- /dev/null +++ b/src/bach/services/daemon_lifecycle.py @@ -0,0 +1,233 @@ +"""Single-instance guard and lifecycle control for the Bach daemon. + +Public API: + DaemonStatus — dataclass returned by daemon_status() + acquire() — write our pid to pidfile; refuses if a live daemon is already running + stop() — SIGTERM the recorded pid; clear the pidfile + daemon_status() — read pidfile + liveness check; return DaemonStatus + +Design: + - All side effects are injectable (liveness_probe, kill_fn, pidfile path). + Tests use tmp_path + fake probes; production code uses real os.kill semantics. + - Liveness probe receives a pid (int) and returns True if live, False if dead. + Default: os.kill(pid, 0) — raises OSError with errno.ESRCH if the pid is gone. + - Stale pidfiles (dead pid) are reclaimed automatically in acquire() so a crashed + daemon never permanently blocks a new one from starting. + - The pidfile path is always passed explicitly; callers typically pass + bach_home() / "daemon.pid" (see daemon_cmd.py). + +Why pidfile and not flock(): + flock() (or fcntl.LOCK_EX) would eliminate the TOCTOU window between read and + write, but it adds a platform-specific import (fcntl is POSIX-only), held-lock + semantics, and cleanup concerns with no real safety gain for a single-user tool. + The only realistic concurrent caller is the user running `bach daemon run` twice + in rapid succession — both processes see the pidfile and one refuses cleanly. + Accepted trade-off: there is a microsecond TOCTOU window (ADR-017 §Known limits). + +Invariants: + - acquire() is the ONLY writer. stop() is the only deleter. + - acquire() must not raise even if the pidfile is corrupted — it reclaims it. + - stop() must not raise even if pidfile is absent or pid is already dead. +""" + +import errno +import logging +import os +import signal +from collections.abc import Callable +from dataclasses import dataclass +from pathlib import Path + +logger = logging.getLogger(__name__) + + +# --------------------------------------------------------------------------- +# Domain type +# --------------------------------------------------------------------------- + + +@dataclass(frozen=True) +class DaemonStatus: + """Snapshot of the daemon's recorded state. + + Attributes: + running: True if a live daemon pid is recorded. + pid: The live pid, or None if no daemon is running. + """ + + running: bool + pid: int | None + + +# --------------------------------------------------------------------------- +# Internal helpers +# --------------------------------------------------------------------------- + + +def _default_liveness_probe(pid: int) -> bool: + """Return True if the process with 'pid' is alive. + + Uses os.kill(pid, 0) — signal 0 does not kill; it only checks if the + kernel would accept a signal for this pid. Two meaningful error cases: + - ESRCH: no such process → dead → return False. + - EPERM: process exists but we can't signal it (different user/root) → + still alive, so return True (conservative: don't reclaim a live + process we cannot signal). + Any other OSError also returns False (treat as dead, allowing reclaim). + """ + try: + os.kill(pid, 0) + return True + except OSError as exc: + if exc.errno == errno.EPERM: + # Process exists but we lack permission → still alive. + return True + # ESRCH or anything else → process is gone. + return False + + +def _read_pidfile(pidfile: Path) -> int | None: + """Read and parse the pidfile; return pid int or None if absent/corrupt.""" + if not pidfile.exists(): + return None + try: + text = pidfile.read_text().strip() + if not text: + return None + return int(text) + except (ValueError, OSError): + logger.warning("event=daemon_pidfile_corrupt path=%s", pidfile) + return None + + +def _write_pidfile(pidfile: Path, pid: int) -> None: + """Write pid to the pidfile, creating parent directories as needed.""" + pidfile.parent.mkdir(parents=True, exist_ok=True) + pidfile.write_text(str(pid)) + + +def _clear_pidfile(pidfile: Path) -> None: + """Remove the pidfile if it exists; log but do not raise on failure.""" + try: + pidfile.unlink(missing_ok=True) + except OSError as exc: + logger.warning("event=daemon_pidfile_clear_failed path=%s reason=%s", pidfile, exc) + + +# --------------------------------------------------------------------------- +# Public API +# --------------------------------------------------------------------------- + + +def acquire( + *, + pidfile: Path, + our_pid: int, + liveness_probe: Callable[[int], bool] = _default_liveness_probe, +) -> tuple[bool, int]: + """Attempt to acquire single-instance ownership. + + Reads the pidfile (if any), checks liveness of the recorded pid, and either: + - Writes our_pid and returns (True, our_pid) when ownership is acquired. + - Returns (False, live_pid) when a live daemon is already running. + + Stale pidfiles (dead pid or corrupt content) are reclaimed automatically: + the file is overwritten with our_pid. + + Idempotent: if our_pid is already written AND is live, returns (True, our_pid). + + Args: + pidfile: Path to the pid file (e.g. bach_home() / "daemon.pid"). + our_pid: The pid of the calling process (typically os.getpid()). + liveness_probe: Callable(pid) → bool. Defaults to os.kill(pid, 0). + + Returns: + (True, our_pid) — ownership acquired. + (False, live_pid) — refused; live_pid is the existing daemon's pid. + """ + existing_pid = _read_pidfile(pidfile) + + if existing_pid is not None: + if existing_pid == our_pid: + # We already own the pidfile — idempotent acquire. + logger.debug("event=daemon_acquire_idempotent pid=%d", our_pid) + return True, our_pid + + if liveness_probe(existing_pid): + # Another live process holds the pidfile — refuse. + logger.info("event=daemon_acquire_refused existing_pid=%d", existing_pid) + return False, existing_pid + + # Existing pid is dead — stale; reclaim it. + logger.info("event=daemon_stale_pidfile_reclaimed stale_pid=%d", existing_pid) + + # No live daemon: write our pid. + _write_pidfile(pidfile, our_pid) + logger.info("event=daemon_acquire_ok pid=%d", our_pid) + return True, our_pid + + +def stop( + *, + pidfile: Path, + liveness_probe: Callable[[int], bool] = _default_liveness_probe, + kill_fn: Callable[[int, int], None] = lambda pid, sig: os.kill(pid, sig), +) -> None: + """Stop the running daemon by sending SIGTERM to the recorded pid. + + Clears the pidfile whether or not the process was live: + - Live pid → SIGTERM then clear. + - Stale pid → just clear (no kill). + - No pidfile → no-op. + + Never raises. All errors are logged as warnings. + + Args: + pidfile: Path to the pid file. + liveness_probe: Callable(pid) → bool. Defaults to os.kill(pid, 0). + kill_fn: Callable(pid, sig) → None. Defaults to os.kill. + """ + pid = _read_pidfile(pidfile) + + if pid is None: + logger.info("event=daemon_stop_no_pidfile path=%s", pidfile) + return + + if liveness_probe(pid): + # Process is live — send SIGTERM. + try: + kill_fn(pid, signal.SIGTERM) + logger.info("event=daemon_stop_sigterm pid=%d", pid) + except OSError as exc: + logger.warning("event=daemon_stop_kill_failed pid=%d reason=%s", pid, exc) + else: + logger.info("event=daemon_stop_stale_pid pid=%d", pid) + + # Always clear the pidfile after attempting to stop. + _clear_pidfile(pidfile) + + +def daemon_status( + *, + pidfile: Path, + liveness_probe: Callable[[int], bool] = _default_liveness_probe, +) -> DaemonStatus: + """Return the current daemon liveness status. + + Reads the pidfile and checks whether the recorded pid is alive. + Does NOT modify any state — purely read-only. + + Args: + pidfile: Path to the pid file. + liveness_probe: Callable(pid) → bool. Defaults to os.kill(pid, 0). + + Returns: + DaemonStatus(running=True, pid=N) — live daemon found. + DaemonStatus(running=False, pid=None) — no daemon or stale entry. + """ + pid = _read_pidfile(pidfile) + + if pid is not None and liveness_probe(pid): + return DaemonStatus(running=True, pid=pid) + + return DaemonStatus(running=False, pid=None) diff --git a/src/bach/services/daemon_supervisor.py b/src/bach/services/daemon_supervisor.py new file mode 100644 index 0000000..007f0d7 --- /dev/null +++ b/src/bach/services/daemon_supervisor.py @@ -0,0 +1,296 @@ +"""Foreground daemon supervisor that keeps one watcher alive per live task session. + +Architecture: + - Single-threaded synchronous reconcile loop. NO threads, NO asyncio. + Why: at single-developer scale (O(5) concurrent sessions) the cycle is fast + enough synchronously; threads/asyncio would add cancellation complexity and + test infrastructure with no throughput benefit. + - Desired set is computed fresh each cycle by enumerate_desired_fn. + - Supervised set is the process-table snapshot from list_supervised_fn. + Why the process table and not a bookkeeping file: the OS is always up to date, + survives crashes, and naturally deduplicates against watch_on_launch spawns. + - Gap = desired - supervised → spawn_fn is called for each. + - Watchers self-terminate when their session ends; ended sessions simply + drop out of desired, so there is NO kill path here. Why: watchers already + self-terminate reliably (ADR-015); tracking individual watcher PIDs for an + active kill path would duplicate responsibility and risk races. + - Bounded exponential crash backoff prevents respawning a dying watcher + every cycle. Backoff state is in-memory and advisory — losing it on + restart results in at most one extra spawn attempt. Conservative-by-design: + KeepAlive restart boundaries bound staleness (ADR-017 §3 accepted trade-off). + +Public API: + BackoffState — type alias for the per-artifact backoff tracking dict. + SpawnPlan — named result of reconcile(), holding the to_spawn set. + reconcile() — pure function, no I/O; determines what to spawn this cycle. + run_supervisor() — the loop; every side-effecting dep is injected. + +Injected seams in run_supervisor (all keyword-only, all have real defaults +so the CLI wires them in one place and tests inject fakes): + enumerate_desired_fn — returns set[Path] of artifact paths that need a watcher. + list_supervised_fn — returns set[Path] of artifacts with a live watcher. + spawn_fn — called with a Path to start a detached watcher. + sleep_fn — pauses between cycles (injected so tests don't block). + should_stop_fn — returns True when the loop must exit. + reconcile_seconds — sleep duration between cycles. +""" + +import logging +import time +from collections.abc import Callable +from dataclasses import dataclass, field +from pathlib import Path +from typing import Any + +logger = logging.getLogger(__name__) + +# --------------------------------------------------------------------------- +# Backoff constants +# --------------------------------------------------------------------------- + +# Initial cooldown in seconds after a first failed spawn attempt. +_BACKOFF_INITIAL_S: float = 10.0 +# Multiplier applied on each successive failed attempt. +_BACKOFF_MULTIPLIER: float = 2.0 +# Hard cap so backoff never grows beyond this. +_BACKOFF_MAX_S: float = 300.0 # 5 minutes + + +# --------------------------------------------------------------------------- +# Public types +# --------------------------------------------------------------------------- + +# BackoffState maps artifact Path → per-artifact tracking dict. +# Keys in the inner dict: +# "last_spawn_at" — float monotonic clock value of the last spawn attempt. +# "attempt" — int count of consecutive failed attempts (watcher kept dying). +# "cooldown" — float seconds to wait before next spawn attempt. +# Using Any for the inner dict value types to keep mypy simple without a +# nested TypedDict — this is internal state, not part of the public API. +BackoffState = dict[Path, dict[str, Any]] + + +@dataclass(frozen=True) +class SpawnPlan: + """Result of a reconcile() call: the set of artifacts to spawn this cycle. + + Frozen so callers cannot mutate it after reconcile() returns. + `to_spawn` is a plain set — order doesn't matter; all are independent. + """ + + to_spawn: set[Path] = field(default_factory=set) + + +# --------------------------------------------------------------------------- +# Pure reconcile — no I/O, no side effects, fully unit-testable +# --------------------------------------------------------------------------- + + +def reconcile( + desired: set[Path], + supervised: set[Path], + backoff: BackoffState, + now: float, +) -> SpawnPlan: + """Determine which artifacts need a new watcher spawned this cycle. + + Rules (applied in order): + 1. Only artifacts in `desired` are candidates — ended sessions have + already been filtered from `desired` by the caller. + 2. Artifacts already in `supervised` are skipped — the duplicate-watcher + invariant; only one watcher per artifact at a time. + 3. Artifacts in backoff cooldown are skipped until the window expires. + 4. All remaining candidates → to_spawn. + + The backoff dict is READ but NOT mutated here. Mutation happens in + run_supervisor after each spawn attempt so the reconcile step stays pure. + + Args: + desired: artifacts that need a live watcher (not ended, not terminal). + supervised: artifacts with an already-running watcher process. + backoff: per-artifact crash backoff tracking (read-only here). + now: current monotonic clock value for backoff comparison. + + Returns: + SpawnPlan with the set of artifacts to spawn this cycle. + """ + to_spawn: set[Path] = set() + + for artifact in desired: + # Duplicate-watcher invariant: skip if already supervised. + if artifact in supervised: + continue + + # Backoff check: if we have a cooldown record and it hasn't expired, skip. + entry = backoff.get(artifact) + if entry is not None: + last_spawn = entry["last_spawn_at"] + cooldown = entry["cooldown"] + elapsed = now - last_spawn + if elapsed < cooldown: + logger.debug( + "event=daemon_backoff_skip artifact=%s elapsed=%.1fs cooldown=%.1fs", + artifact, + elapsed, + cooldown, + ) + continue + + to_spawn.add(artifact) + + return SpawnPlan(to_spawn=to_spawn) + + +# --------------------------------------------------------------------------- +# Backoff state updater — called after each spawn attempt +# --------------------------------------------------------------------------- + + +def _update_backoff_after_spawn(artifact: Path, backoff: BackoffState, now: float) -> None: + """Record a spawn attempt in the backoff state. + + On the first attempt for an artifact, creates a fresh entry with the + initial cooldown. On each successive attempt, doubles the cooldown + (bounded by _BACKOFF_MAX_S). + + The backoff entry is cleared externally (when the artifact drops from + desired, meaning its session ended naturally) — we do NOT clear it here + because reconcile's next cycle will see `desired` without this artifact + and simply never check its backoff entry. + + Args: + artifact: the artifact path that was just spawned. + backoff: mutable BackoffState dict — updated in place. + now: current monotonic clock value. + """ + entry = backoff.get(artifact) + if entry is None: + # First attempt: create a fresh entry. + backoff[artifact] = { + "last_spawn_at": now, + "attempt": 1, + "cooldown": _BACKOFF_INITIAL_S, + } + else: + # Successive attempt: increase attempt count and double cooldown. + new_attempt = entry["attempt"] + 1 + new_cooldown = min(entry["cooldown"] * _BACKOFF_MULTIPLIER, _BACKOFF_MAX_S) + backoff[artifact] = { + "last_spawn_at": now, + "attempt": new_attempt, + "cooldown": new_cooldown, + } + logger.info( + "event=daemon_backoff_updated artifact=%s attempt=%d cooldown=%.1fs", + artifact, + new_attempt, + new_cooldown, + ) + + +# --------------------------------------------------------------------------- +# Supervisor loop — all side effects injected +# --------------------------------------------------------------------------- + + +def run_supervisor( + *, + enumerate_desired_fn: Callable[[], set[Path]], + list_supervised_fn: Callable[[], set[Path]], + spawn_fn: Callable[[Path], Any], + sleep_fn: Callable[[float], None] = time.sleep, + should_stop_fn: Callable[[], bool], + reconcile_seconds: float, +) -> None: + """Run the foreground supervisor loop. + + Each cycle: + 1. Check should_stop_fn — exit cleanly if True. + 2. Enumerate desired artifacts (task-linked, not ended, not terminal). + 3. List currently supervised artifacts (process table scan). + 4. Reconcile to find the gap (desired - supervised, minus backoff). + 5. Spawn a watcher for each artifact in the gap. + 6. Sleep reconcile_seconds. + 7. Repeat. + + Watchers self-terminate when their sessions end — there is no kill path. + An artifact whose watcher keeps crashing is handled by the backoff state + so we back off exponentially instead of hammering the process table. + + All side-effecting deps are keyword-only args so the CLI wires them once + and tests inject fakes without touching real processes or real time. + + Args: + enumerate_desired_fn: returns set[Path] of artifacts needing a watcher. + list_supervised_fn: returns set[Path] of already-supervised artifacts. + spawn_fn: called with a Path to start a detached watcher. + sleep_fn: sleep between cycles (real: time.sleep, fake in tests). + should_stop_fn: returns True when the loop must exit cleanly. + reconcile_seconds: sleep duration; also logged per cycle for diagnostics. + """ + # In-memory backoff state — intentionally not persisted to disk. + # Rationale: a fresh daemon restart (via KeepAlive or manual run) should give + # every artifact a clean slate immediately. Persisting backoff across restarts + # would delay recovery when the underlying issue was transient. At worst, losing + # the state produces one extra spawn attempt before backoff re-accumulates. + backoff: BackoffState = {} + + logger.info( + "event=daemon_supervisor_start reconcile_seconds=%s", + reconcile_seconds, + ) + + while True: + # Enumerate desired artifacts for this cycle. + try: + desired = enumerate_desired_fn() + except Exception as exc: # noqa: BLE001 — best-effort; log and continue + logger.warning("event=daemon_enumerate_error reason=%s", type(exc).__name__) + desired = set() + + # Get the current supervised set from the process table. + try: + supervised = list_supervised_fn() + except Exception as exc: # noqa: BLE001 + logger.warning("event=daemon_supervised_error reason=%s", type(exc).__name__) + supervised = set() + + # Monotonic clock for backoff comparison. + now = time.monotonic() + plan = reconcile(desired, supervised, backoff, now) + + # Spawn watchers for the gap. + for artifact in plan.to_spawn: + logger.info("event=daemon_spawn artifact=%s", artifact) + try: + spawn_fn(artifact) + # Record the spawn attempt in backoff state so a watcher that + # crashes immediately doesn't get respawned on the very next cycle. + _update_backoff_after_spawn(artifact, backoff, now) + except Exception as exc: # noqa: BLE001 — warn-not-fail + logger.warning( + "event=daemon_spawn_error artifact=%s reason=%s", + artifact, + type(exc).__name__, + ) + _update_backoff_after_spawn(artifact, backoff, now) + + logger.debug( + "event=daemon_cycle desired=%d supervised=%d spawned=%d", + len(desired), + len(supervised), + len(plan.to_spawn), + ) + + # Check stop signal AFTER doing work so: (1) at least one full cycle + # always runs before evaluating the stop flag, and (2) a SIGTERM received + # mid-cycle exits cleanly without sleeping again. Tests rely on this + # ordering: enumerate is always called before the first stop check. + if should_stop_fn(): + logger.info("event=daemon_supervisor_stop reason=should_stop") + break + + # Sleep before next cycle. + sleep_fn(reconcile_seconds) + + logger.info("event=daemon_supervisor_done") diff --git a/src/bach/services/launchd_service.py b/src/bach/services/launchd_service.py new file mode 100644 index 0000000..ed6298a --- /dev/null +++ b/src/bach/services/launchd_service.py @@ -0,0 +1,278 @@ +"""macOS launchd LaunchAgent management for the Bach daemon. + +Provides idempotent install/uninstall of a LaunchAgent plist that keeps +`bach daemon run` alive across logins on macOS. + +Design mirrors hooks_service.py: + - `render_launch_agent` is a PURE function — no I/O, easy to test. + - `install_launch_agent` / `uninstall_launch_agent` accept injected `runner` + and `launch_agents_dir` so tests NEVER call real launchctl or touch + ~/Library/LaunchAgents. + - On launchctl failure, a `fallback_command` is returned (NEVER raise + unrecoverably) — callers print it so the user always has a manual path. + Why: launchctl can fail in sandboxed environments, on non-macOS, or under + SIP. The plist being on disk is still useful (e.g. for `launchctl load` + later) so we do not treat launchctl failure as a hard error. + - Idempotent: calling install twice unloads the existing agent before + reloading, preventing duplicate label registration errors. + +Architecture rule: install is EXPLICIT — only triggered by `bach daemon install`. +Nothing auto-installs the LaunchAgent on import or first run. +""" + +import logging +import plistlib +import subprocess +import sys +from collections.abc import Callable +from dataclasses import dataclass +from pathlib import Path +from typing import Any + +from bach.config.paths import bach_home + +logger = logging.getLogger(__name__) + +# Stable reverse-DNS label for the LaunchAgent. Never change this — it is +# used as the plist filename, the launchctl service name, and for idempotency. +LAUNCH_AGENT_LABEL = "com.bach.daemon" + +# Type alias: runner takes an argv list and returns (returncode, output). +RunnerFn = Callable[[list[str]], tuple[int, str]] + +# The fallback command users can run manually if launchctl is unavailable. +_FALLBACK_CMD = "bach daemon run" + + +@dataclass(frozen=True) +class LaunchdInstallResult: + """Outcome of an install_launch_agent call. + + Attributes: + installed: True when the plist was written successfully. + fallback_command: Set when launchctl failed; the user should run this + command manually (or under tmux/screen). None on + full success. + """ + + installed: bool + fallback_command: str | None = None + + +def _default_launch_agents_dir() -> Path: + """Return the standard macOS LaunchAgents directory for the current user.""" + return Path.home() / "Library" / "LaunchAgents" + + +def _default_log_dir() -> Path: + """Return the Bach log directory for daemon stdout/stderr.""" + return bach_home() / "logs" + + +def render_launch_agent(*, log_dir: Path) -> str: + """Return a valid plist XML string for the Bach daemon LaunchAgent. + + The plist contains: + Label — LAUNCH_AGENT_LABEL (com.bach.daemon) + ProgramArguments — [sys.executable, "-m", "bach", "daemon", "run"] + (uses the current interpreter so no PATH dependency) + RunAtLoad — True (auto-start on login) + KeepAlive — True (restart on crash) + StandardOutPath — /daemon.stdout.log + StandardErrorPath — /daemon.stderr.log + + Args: + log_dir: Directory where stdout/stderr logs are written. The caller + is responsible for creating it if needed. + + Returns: + A plist XML string suitable for writing to disk. + """ + # Use sys.executable so the correct venv Python is invoked even when + # the launchd environment has no PATH — same rationale as hooks_service. + program_args = [sys.executable, "-m", "bach", "daemon", "run"] + + data: dict[str, Any] = { + "Label": LAUNCH_AGENT_LABEL, + "ProgramArguments": program_args, + "RunAtLoad": True, + "KeepAlive": True, + "StandardOutPath": str(log_dir / "daemon.stdout.log"), + "StandardErrorPath": str(log_dir / "daemon.stderr.log"), + } + + # plistlib.dumps produces valid Apple plist XML — no hand-rolled XML needed. + return plistlib.dumps(data, fmt=plistlib.FMT_XML).decode("utf-8") + + +def install_launch_agent( + *, + launch_agents_dir: Path | None = None, + log_dir: Path | None = None, + runner: RunnerFn | None = None, +) -> LaunchdInstallResult: + """Write the LaunchAgent plist and load it via launchctl. + + Idempotent: if the plist already exists it is overwritten with the current + config, then launchctl unloads the old instance before loading the new one + (prevents duplicate agent registration). + + On launchctl failure (unavailable binary, SIP restriction, etc.) the plist + IS still written to disk and a `fallback_command` is returned — the user + can load it manually or run `bach daemon run` under a process supervisor. + + Args: + launch_agents_dir: Override for ~/Library/LaunchAgents (tests use tmp_path). + log_dir: Override for log directory (defaults to bach_home()/logs). + runner: Injectable launchctl runner: (argv) -> (returncode, output). + Defaults to the real subprocess-based runner. + + Returns: + LaunchdInstallResult with installed=True on plist write success. + fallback_command is set when launchctl load failed. + """ + # Resolve defaults (injectable for tests). + resolved_dir = launch_agents_dir or _default_launch_agents_dir() + resolved_log_dir = log_dir or _default_log_dir() + resolved_runner = runner or _subprocess_runner + + # Ensure the LaunchAgents directory exists. + resolved_dir.mkdir(parents=True, exist_ok=True) + + # Ensure the log directory exists so launchd can open the log files. + resolved_log_dir.mkdir(parents=True, exist_ok=True) + + plist_path = resolved_dir / f"{LAUNCH_AGENT_LABEL}.plist" + + # Render the plist XML from the pure render function. + xml = render_launch_agent(log_dir=resolved_log_dir) + + # Idempotency: unload before overwriting the plist. If we wrote the new plist + # first and then loaded, launchctl would complain about a duplicate label + # because the old instance is still registered. Unload first (ignore return + # code — the agent may not be loaded yet on first install), then write, then + # load. This is safe to repeat any number of times. + if plist_path.exists(): + _try_unload(plist_path=plist_path, runner=resolved_runner) + + # Write the plist to disk. + plist_path.write_text(xml, encoding="utf-8") + logger.info("event=launchd_plist_written path=%s", plist_path) + + # Load the agent via launchctl. + fallback: str | None = None + load_rc, load_out = _try_load(plist_path=plist_path, runner=resolved_runner) + if load_rc != 0: + # launchctl failed — surface the fallback so the CLI can print it. + fallback = _FALLBACK_CMD + logger.warning( + "event=launchd_load_failed plist=%s rc=%d output=%r fallback=%r", + plist_path, + load_rc, + load_out, + fallback, + ) + else: + logger.info("event=launchd_load_ok plist=%s", plist_path) + + return LaunchdInstallResult(installed=True, fallback_command=fallback) + + +def uninstall_launch_agent( + *, + launch_agents_dir: Path | None = None, + runner: RunnerFn | None = None, +) -> None: + """Unload and remove the LaunchAgent plist. + + Idempotent: if the plist does not exist, this is a no-op (no error raised). + + Args: + launch_agents_dir: Override for ~/Library/LaunchAgents (tests use tmp_path). + runner: Injectable launchctl runner: (argv) -> (returncode, output). + """ + resolved_dir = launch_agents_dir or _default_launch_agents_dir() + resolved_runner = runner or _subprocess_runner + + plist_path = resolved_dir / f"{LAUNCH_AGENT_LABEL}.plist" + + if not plist_path.exists(): + # Already absent — idempotent no-op. + logger.info("event=launchd_uninstall_noop plist=%s (not found)", plist_path) + return + + # Unload from launchd before removing the file. + # Ignore the return code — the agent may not be loaded (e.g. after a crash). + _try_unload(plist_path=plist_path, runner=resolved_runner) + + # Remove the plist file. + plist_path.unlink(missing_ok=True) + logger.info("event=launchd_plist_removed path=%s", plist_path) + + +def status_launch_agent( + *, + launch_agents_dir: Path | None = None, +) -> dict[str, Any]: + """Return the installation status of the LaunchAgent. + + Currently a best-effort check: we report `installed` based on whether the + plist file is present on disk. Full launchd query (launchctl list) would + require parsing output and is not needed for the issue-14 acceptance criteria. + + Args: + launch_agents_dir: Override for ~/Library/LaunchAgents (tests use tmp_path). + + Returns: + dict with keys: + installed: bool — plist file is present at the expected path. + plist_path: str — the expected plist path (for display). + """ + resolved_dir = launch_agents_dir or _default_launch_agents_dir() + plist_path = resolved_dir / f"{LAUNCH_AGENT_LABEL}.plist" + return { + "installed": plist_path.exists(), + "plist_path": str(plist_path), + } + + +# --------------------------------------------------------------------------- +# Internal helpers — launchctl wrappers +# --------------------------------------------------------------------------- + + +def _subprocess_runner(argv: list[str]) -> tuple[int, str]: + """Default runner: execute argv via subprocess, return (returncode, stdout).""" + try: + proc = subprocess.run( + argv, + capture_output=True, + text=True, + timeout=10, + ) + return proc.returncode, proc.stdout + proc.stderr + except (FileNotFoundError, OSError) as exc: + # launchctl binary not found (non-macOS, sandboxed env, etc.). + logger.warning("event=launchctl_not_found argv=%r reason=%s", argv, exc) + return 1, str(exc) + + +def _try_load(*, plist_path: Path, runner: RunnerFn) -> tuple[int, str]: + """Attempt to load the plist via `launchctl load `. + + Returns (returncode, combined output). Never raises. + """ + argv = ["launchctl", "load", str(plist_path)] + logger.debug("event=launchctl_load argv=%r", argv) + return runner(argv) + + +def _try_unload(*, plist_path: Path, runner: RunnerFn) -> tuple[int, str]: + """Attempt to unload the plist via `launchctl unload `. + + Returns (returncode, combined output). Never raises. Errors are logged + at DEBUG level because the agent may not be loaded yet. + """ + argv = ["launchctl", "unload", str(plist_path)] + logger.debug("event=launchctl_unload argv=%r", argv) + return runner(argv) diff --git a/src/bach/services/sessions_service.py b/src/bach/services/sessions_service.py index a3b41e8..5dde132 100644 --- a/src/bach/services/sessions_service.py +++ b/src/bach/services/sessions_service.py @@ -669,3 +669,62 @@ def close_session( pids[0], ) return CloseResult(session.session_id, pids[0], [], "terminated") + + +# --------------------------------------------------------------------------- +# Daemon supervisor enumeration — Phase 17 (issue #12) +# --------------------------------------------------------------------------- + +# Terminal statuses: tasks in these states do not need a live watcher. +# Matches VALID_STATUSES terminal states in status_service.py. +_TERMINAL_STATUSES: frozenset[str] = frozenset({"done", "carried_forward"}) + + +def list_supervisable_artifacts(rows: list[SessionRow]) -> set[Path]: + """Return artifact Paths that the daemon supervisor should keep watched. + + Filters the given rows to only those that: + 1. Are task-linked (task_id is not None AND artifact_path is not None). + 2. Have a session liveness that is NOT ended — ended sessions are gone. + 3. Have a task status that is NOT in _TERMINAL_STATUSES (done/carried_forward). + + The status is read from the artifact's frontmatter (via _read_artifact_frontmatter) + so the check reflects the latest on-disk state, not a stale row snapshot. + An unreadable artifact is treated as having an unknown (non-terminal) status and + is included rather than silently skipped — the watcher will log the missing + artifact and exit itself, which is the correct behavior. + + Args: + rows: list of SessionRow as produced by list_sessions(). The caller is + responsible for building the row list (e.g. from scan_registry_artifact_paths). + + Returns: + Set of artifact Paths that need a live watcher process. + """ + result: set[Path] = set() + + for row in rows: + # Must be task-linked to be supervisable. + if row.task_id is None or row.artifact_path is None: + continue + + # Ended sessions have no live process — nothing to watch. + if row.session.liveness is SessionLiveness.ended: + continue + + # Check task status against terminal states. + payload = _read_artifact_frontmatter(row.artifact_path) + if payload is not None: + status = str(payload.get("status", "")) + if status in _TERMINAL_STATUSES: + logger.debug( + "event=supervisable_skip_terminal artifact=%s status=%s", + row.artifact_path, + status, + ) + continue + + result.add(row.artifact_path) + + logger.debug("event=list_supervisable_artifacts count=%d", len(result)) + return result diff --git a/src/bach/services/task_service.py b/src/bach/services/task_service.py index 04d27eb..9bcc5b6 100644 --- a/src/bach/services/task_service.py +++ b/src/bach/services/task_service.py @@ -14,19 +14,17 @@ """ import logging -import subprocess -import sys from collections.abc import Callable from pathlib import Path import typer -from bach.config.paths import bach_home from bach.config.projects import ProjectRegistry from bach.config.settings import load_config from bach.domain.models import AgentRuntime, Project, SessionMode, TaskArtifact from bach.runtimes.commands import build_launch_command, build_resume_command from bach.runtimes.iterm import launch_in_iterm +from bach.runtimes.watcher_processes import spawn_watcher from bach.services.session_tracker import record_session_start from bach.storage.artifacts import TaskArtifactStore @@ -234,60 +232,14 @@ def launch_task( def _spawn_session_watcher(self, artifact: Path) -> tuple[bool, Path | None]: """Spawn `bach internal session-watch ` as a detached child. - Uses the same Python interpreter so the watcher inherits the same - environment (venv, PATH, etc.) without requiring `bach` to be on PATH. - subprocess.Popen with close_fds=True + redirected stdout/stderr - detaches the child from the parent terminal. We do NOT wait for it. + Thin delegation to runtimes.watcher_processes.spawn_watcher, which owns + the detach logic, log-file routing, and warn-not-fail error handling. + This method exists so callers in this service (launch_task) and tests + can patch it at the service level when needed. - stdout and stderr are redirected to an append-mode log file at: - bach_home() / "logs" / "watcher-.log" - This is the ONLY trace of a detached process — DEVNULL would make - watcher crashes completely invisible and unrecoverable to diagnose. - - Best-effort: a spawn failure logs a warning but NEVER blocks the - primary launch (warn-not-fail). The user has their iTerm tab; the - watcher is bonus. - - Returns (True, log_path) on successful spawn, (False, None) on - failure. The caller decides whether to print a confirmation line — - no output happens here so the output channel stays with launch_task. + Returns (True, log_path) on success, (False, None) on failure. """ - # Build the log file path: bach_home/logs/watcher-.log - # Using the artifact stem makes the log easy to correlate to the task. - # Fallback to a runtime timestamp if the artifact path is unusually short. - stem = artifact.stem if artifact.stem else artifact.name - logs_dir = bach_home() / "logs" - logs_dir.mkdir(parents=True, exist_ok=True) # create on demand - log_path = logs_dir / f"watcher-{stem}.log" - - try: - # Construct the same invocation as `bach internal session-watch` - # by re-using the current interpreter and module entry point. - cmd = [sys.executable, "-m", "bach", "internal", "session-watch", str(artifact)] - # Open in append mode so successive watchers for the same artifact - # (e.g. re-launch after crash) accumulate in one file rather than - # overwriting the previous run's output. - log_fh = log_path.open("a", encoding="utf-8") - subprocess.Popen( - cmd, - stdin=subprocess.DEVNULL, - stdout=log_fh, - stderr=log_fh, - close_fds=True, - ) - logger.info( - "event=session_watcher_spawned artifact=%s log=%s", - artifact, - log_path, - ) - return True, log_path - except Exception as exc: # noqa: BLE001 — warn-not-fail, never block launch - logger.warning( - "event=session_watcher_spawn_failed artifact=%s reason=%s", - artifact, - type(exc).__name__, - ) - return False, None + return spawn_watcher(artifact) def validate_task(self, artifact: Path) -> list[str]: """Return a list of validation errors for the artifact, empty if OK.""" diff --git a/tests/unit/test_config_set.py b/tests/unit/test_config_set.py index dc2a707..87dcfe7 100644 --- a/tests/unit/test_config_set.py +++ b/tests/unit/test_config_set.py @@ -35,9 +35,12 @@ # --------------------------------------------------------------------------- -def test_observer_settable_keys_has_eight_entries() -> None: - """The allowlist must contain exactly the 8 observer knobs specified in the DAG.""" - assert len(OBSERVER_SETTABLE_KEYS) == 8 +def test_observer_settable_keys_has_nine_entries() -> None: + """The allowlist must contain exactly 9 observer knobs. + + Phase 17 added daemon-reconcile-seconds to the original 8. + """ + assert len(OBSERVER_SETTABLE_KEYS) == 9 def test_observer_settable_keys_contains_expected_keys() -> None: @@ -51,6 +54,8 @@ def test_observer_settable_keys_contains_expected_keys() -> None: "liveness-idle-minutes", "afk-max-turns", "afk-time-budget-minutes", + # Phase 17 (issue #12) — daemon supervisor cycle interval + "daemon-reconcile-seconds", } assert set(OBSERVER_SETTABLE_KEYS.keys()) == expected diff --git a/tests/unit/test_daemon_cmd.py b/tests/unit/test_daemon_cmd.py new file mode 100644 index 0000000..c0ccf73 --- /dev/null +++ b/tests/unit/test_daemon_cmd.py @@ -0,0 +1,71 @@ +"""Smoke/wiring tests for cli/daemon_cmd.py. + +Tests that: + - `bach daemon run` is registered and callable via Typer. + - The `run` command wires run_supervisor with the correct seams. + - No real processes or real time.sleep are called in these tests. + - Clean SIGTERM handling: the stop event causes a clean exit. +""" + +from pathlib import Path +from unittest.mock import MagicMock, patch + +from typer.testing import CliRunner + +from bach.cli.app import app + +runner = CliRunner() + + +def test_daemon_run_is_registered() -> None: + """daemon run subcommand must appear in --help output.""" + result = runner.invoke(app, ["daemon", "--help"]) + assert result.exit_code == 0 + assert "run" in result.output + + +def test_daemon_run_wires_supervisor_and_exits_cleanly(tmp_path: Path) -> None: + """daemon run invokes run_supervisor; with a patched should_stop that fires + immediately, the command exits with code 0. + + We patch: + - run_supervisor (the core loop) — to verify it is called with the right args. + - list_supervisable_artifacts (the enumerate_desired seam). + - list_supervised_artifacts (the list_supervised seam). + - spawn_watcher (the spawn seam). + - load_config — returns a config pointing at tmp_path so no ~/.bach is touched. + """ + from bach.config.settings import BachConfig + + mock_supervisor = MagicMock() + + with ( + patch("bach.cli.daemon_cmd.run_supervisor", mock_supervisor), + patch("bach.cli.daemon_cmd.load_config", return_value=BachConfig()), + ): + result = runner.invoke(app, ["daemon", "run"]) + + # run_supervisor was called once + assert mock_supervisor.call_count == 1, f"Expected 1 call, got {mock_supervisor.call_count}" + + # Exit code must be 0 (clean) + assert result.exit_code == 0, f"Non-zero exit: {result.output}" + + +def test_daemon_run_passes_reconcile_seconds_from_config(tmp_path: Path) -> None: + """reconcile_seconds kwarg is sourced from cfg.daemon_reconcile_seconds.""" + from bach.config.settings import BachConfig + + custom_cfg = BachConfig(daemon_reconcile_seconds=99) + captured_kwargs: dict[str, object] = {} + + def fake_supervisor(**kwargs): # type: ignore[no-untyped-def] + captured_kwargs.update(kwargs) + + with ( + patch("bach.cli.daemon_cmd.run_supervisor", fake_supervisor), + patch("bach.cli.daemon_cmd.load_config", return_value=custom_cfg), + ): + runner.invoke(app, ["daemon", "run"]) + + assert captured_kwargs.get("reconcile_seconds") == 99 diff --git a/tests/unit/test_daemon_config.py b/tests/unit/test_daemon_config.py new file mode 100644 index 0000000..3a831a2 --- /dev/null +++ b/tests/unit/test_daemon_config.py @@ -0,0 +1,83 @@ +"""Tests for the daemon_reconcile_seconds config knob. + +Verifies: + - BachConfig has daemon_reconcile_seconds with a sensible default. + - load_config reads it from YAML correctly. + - Invalid values fall back to the default. + - set_observer_key("daemon-reconcile-seconds", ...) works via the allowlist. +""" + +from pathlib import Path + +from bach.config.settings import ( + _OBSERVER_KEY_TO_FIELD, + OBSERVER_SETTABLE_KEYS, + BachConfig, + load_config, + set_observer_key, +) + +# Default must be a sensible positive int (>=1). +_EXPECTED_DEFAULT = 30 + + +def test_daemon_reconcile_seconds_has_sensible_default() -> None: + """BachConfig() default is a positive int.""" + cfg = BachConfig() + assert cfg.daemon_reconcile_seconds >= 1 + assert cfg.daemon_reconcile_seconds == _EXPECTED_DEFAULT + + +def test_load_config_reads_daemon_reconcile_seconds(tmp_path: Path) -> None: + """A YAML file with daemon_reconcile_seconds: 60 → config value is 60.""" + config_path = tmp_path / "config.yaml" + config_path.write_text("daemon_reconcile_seconds: 60\n") + + cfg = load_config(path=config_path) + assert cfg.daemon_reconcile_seconds == 60 + + +def test_load_config_invalid_daemon_reconcile_falls_back(tmp_path: Path) -> None: + """Non-positive values fall back to default.""" + config_path = tmp_path / "config.yaml" + config_path.write_text("daemon_reconcile_seconds: 0\n") + + cfg = load_config(path=config_path) + assert cfg.daemon_reconcile_seconds == _EXPECTED_DEFAULT + + +def test_daemon_reconcile_seconds_in_observer_allowlist() -> None: + """daemon-reconcile-seconds must be in the observer settable keys allowlist.""" + assert "daemon-reconcile-seconds" in OBSERVER_SETTABLE_KEYS + + +def test_daemon_reconcile_seconds_field_mapping() -> None: + """The CLI key maps to the correct dataclass field name.""" + assert _OBSERVER_KEY_TO_FIELD["daemon-reconcile-seconds"] == "daemon_reconcile_seconds" + + +def test_set_observer_key_daemon_reconcile_seconds(tmp_path: Path) -> None: + """set_observer_key writes daemon_reconcile_seconds correctly.""" + from unittest.mock import patch + + with patch("bach.config.settings._default_config_path", return_value=tmp_path / "config.yaml"): + path = set_observer_key("daemon-reconcile-seconds", "45") + + cfg = load_config(path=path) + assert cfg.daemon_reconcile_seconds == 45 + + +def test_set_observer_key_rejects_zero() -> None: + """Zero is not a positive int — must raise ValueError.""" + import pytest + + with pytest.raises(ValueError, match="positive"): + set_observer_key("daemon-reconcile-seconds", "0") + + +def test_set_observer_key_rejects_negative() -> None: + """Negative values must raise ValueError.""" + import pytest + + with pytest.raises(ValueError, match="positive"): + set_observer_key("daemon-reconcile-seconds", "-5") diff --git a/tests/unit/test_daemon_launchd_cmd.py b/tests/unit/test_daemon_launchd_cmd.py new file mode 100644 index 0000000..314e8c6 --- /dev/null +++ b/tests/unit/test_daemon_launchd_cmd.py @@ -0,0 +1,149 @@ +"""Tests for `bach daemon install` and `bach daemon uninstall` CLI commands. + +Strategy: + - All launchd service calls are patched — no real launchctl or + ~/Library/LaunchAgents touched. + - Tests verify: subcommand registration, success output, fallback output + when launchctl fails, and idempotent-call behaviour. +""" + +from pathlib import Path +from unittest.mock import MagicMock, patch + +from typer.testing import CliRunner + +from bach.cli.app import app +from bach.services.launchd_service import LaunchdInstallResult + +runner = CliRunner() + +# Patch targets inside daemon_cmd module. +_INSTALL_PATCH = "bach.cli.daemon_cmd.install_launch_agent" +_UNINSTALL_PATCH = "bach.cli.daemon_cmd.uninstall_launch_agent" + + +# --------------------------------------------------------------------------- +# Subcommand registration +# --------------------------------------------------------------------------- + + +def test_daemon_install_is_registered() -> None: + """bach daemon install must appear in --help output.""" + result = runner.invoke(app, ["daemon", "--help"]) + assert result.exit_code == 0, result.output + assert "install" in result.output + + +def test_daemon_uninstall_is_registered() -> None: + """bach daemon uninstall must appear in --help output.""" + result = runner.invoke(app, ["daemon", "--help"]) + assert result.exit_code == 0, result.output + assert "uninstall" in result.output + + +# --------------------------------------------------------------------------- +# `bach daemon install` — success path +# --------------------------------------------------------------------------- + + +def test_daemon_install_calls_service(tmp_path: Path) -> None: + """daemon install must call install_launch_agent once.""" + mock_install = MagicMock( + return_value=LaunchdInstallResult(installed=True, fallback_command=None) + ) + with patch(_INSTALL_PATCH, mock_install): + result = runner.invoke(app, ["daemon", "install"]) + + assert mock_install.call_count == 1 + assert result.exit_code == 0, result.output + + +def test_daemon_install_prints_success_message(tmp_path: Path) -> None: + """On success, daemon install must print a success-indicating message.""" + mock_install = MagicMock( + return_value=LaunchdInstallResult(installed=True, fallback_command=None) + ) + with patch(_INSTALL_PATCH, mock_install): + result = runner.invoke(app, ["daemon", "install"]) + + # Output must mention some form of success / installed. + assert any( + word in result.output.lower() + for word in ("installed", "success", "loaded", "install") + ), f"Expected success message, got: {result.output!r}" + + +# --------------------------------------------------------------------------- +# `bach daemon install` — fallback path (launchctl failure) +# --------------------------------------------------------------------------- + + +def test_daemon_install_prints_fallback_on_launchctl_failure(tmp_path: Path) -> None: + """When launchctl fails, daemon install must print the manual fallback command.""" + mock_install = MagicMock( + return_value=LaunchdInstallResult( + installed=True, + fallback_command="bach daemon run", + ) + ) + with patch(_INSTALL_PATCH, mock_install): + result = runner.invoke(app, ["daemon", "install"]) + + # The fallback command must appear in the output. + assert "bach daemon run" in result.output, ( + f"Fallback command not printed. Output: {result.output!r}" + ) + + +def test_daemon_install_exit_code_nonzero_on_install_failure(tmp_path: Path) -> None: + """When install_launch_agent returns installed=False (unexpected error), + daemon install must exit with a non-zero code. + """ + mock_install = MagicMock( + return_value=LaunchdInstallResult(installed=False, fallback_command="bach daemon run") + ) + with patch(_INSTALL_PATCH, mock_install): + result = runner.invoke(app, ["daemon", "install"]) + + # Non-zero exit on failure. + assert result.exit_code != 0, ( + f"Expected non-zero exit on failure, got 0. Output: {result.output!r}" + ) + + +# --------------------------------------------------------------------------- +# `bach daemon uninstall` +# --------------------------------------------------------------------------- + + +def test_daemon_uninstall_calls_service(tmp_path: Path) -> None: + """daemon uninstall must call uninstall_launch_agent once.""" + mock_uninstall = MagicMock(return_value=None) + with patch(_UNINSTALL_PATCH, mock_uninstall): + result = runner.invoke(app, ["daemon", "uninstall"]) + + assert mock_uninstall.call_count == 1 + assert result.exit_code == 0, result.output + + +def test_daemon_uninstall_prints_confirmation(tmp_path: Path) -> None: + """daemon uninstall must print a message confirming the action.""" + mock_uninstall = MagicMock(return_value=None) + with patch(_UNINSTALL_PATCH, mock_uninstall): + result = runner.invoke(app, ["daemon", "uninstall"]) + + assert any( + word in result.output.lower() + for word in ("uninstalled", "removed", "uninstall") + ), f"Expected uninstall message, got: {result.output!r}" + + +def test_daemon_uninstall_is_idempotent_from_cli(tmp_path: Path) -> None: + """Calling daemon uninstall twice must succeed both times (exit 0).""" + mock_uninstall = MagicMock(return_value=None) + with patch(_UNINSTALL_PATCH, mock_uninstall): + r1 = runner.invoke(app, ["daemon", "uninstall"]) + r2 = runner.invoke(app, ["daemon", "uninstall"]) + + assert r1.exit_code == 0, r1.output + assert r2.exit_code == 0, r2.output diff --git a/tests/unit/test_daemon_lifecycle.py b/tests/unit/test_daemon_lifecycle.py new file mode 100644 index 0000000..ce5e593 --- /dev/null +++ b/tests/unit/test_daemon_lifecycle.py @@ -0,0 +1,279 @@ +"""Tests for services/daemon_lifecycle.py — single-instance guard, stop, status. + +Strategy: + - All filesystem ops use tmp_path (never ~/.bach). + - kill_fn and liveness probe are injected fakes — no real processes. + - Tests follow TDD: written against the intended public API before implementation. + +Covered cases: + acquire: + - writes pidfile when no daemon is running → returns (True, our_pid) + - refuses when live daemon is already recorded → returns (False, live_pid) + - reclaims stale pidfile (dead pid) → writes our pid, returns (True, our_pid) + - idempotent: calling acquire again with our own pid → returns (True, our_pid) + + stop: + - SIGTERMs the recorded pid; clears the pidfile + - graceful when pidfile does not exist (no-op) + - graceful when recorded pid is stale (clears pidfile, no kill) + + daemon_status: + - returns (running=True, pid=N) when live daemon recorded + - returns (running=False, pid=None) when no pidfile + - returns (running=False, pid=None) when pidfile is stale +""" + +import os +from collections.abc import Callable +from pathlib import Path + +import pytest + +from bach.services.daemon_lifecycle import ( + DaemonStatus, + acquire, + daemon_status, + stop, +) + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +_OUR_PID = os.getpid() # real pid of the test process (always live) +_DEAD_PID = 99999999 # astronomically large — almost certainly not running + + +def _live_probe(pid: int) -> bool: + """Fake liveness probe: PID 1..9999 = live, anything else = dead.""" + return 1 <= pid <= 9999 + + +def _never_kill(pid: int, sig: int) -> None: + """Fake kill that raises if ever called (unexpected).""" + pytest.fail(f"kill({pid}, {sig}) was called unexpectedly") + + +def _recording_kill(calls: list[tuple[int, int]]) -> Callable[[int, int], None]: + """Return a kill_fn that records (pid, sig) pairs.""" + def _kill(pid: int, sig: int) -> None: + calls.append((pid, sig)) + return _kill + + +# --------------------------------------------------------------------------- +# acquire +# --------------------------------------------------------------------------- + + +def test_acquire_writes_pidfile_when_no_daemon(tmp_path: Path) -> None: + """First acquire: no pidfile → writes our pid, returns (True, our_pid).""" + pidfile = tmp_path / "daemon.pid" + + ok, recorded_pid = acquire( + pidfile=pidfile, + our_pid=42, + liveness_probe=_live_probe, + ) + + assert ok is True + assert recorded_pid == 42 + assert pidfile.exists() + assert pidfile.read_text().strip() == "42" + + +def test_acquire_refuses_when_live_daemon_running(tmp_path: Path) -> None: + """Live daemon already recorded → returns (False, live_pid), no overwrite.""" + pidfile = tmp_path / "daemon.pid" + live_pid = 1234 # within _live_probe's live range + + pidfile.write_text(str(live_pid)) + + ok, recorded_pid = acquire( + pidfile=pidfile, + our_pid=9999, + liveness_probe=_live_probe, + ) + + assert ok is False + assert recorded_pid == live_pid + # Pidfile must still hold the live pid, not be overwritten. + assert pidfile.read_text().strip() == str(live_pid) + + +def test_acquire_reclaims_stale_pidfile(tmp_path: Path) -> None: + """Pidfile exists but pid is dead → reclaim: write our pid, return (True, our_pid).""" + pidfile = tmp_path / "daemon.pid" + pidfile.write_text(str(_DEAD_PID)) # dead pid + + ok, recorded_pid = acquire( + pidfile=pidfile, + our_pid=42, + liveness_probe=_live_probe, + ) + + assert ok is True + assert recorded_pid == 42 + assert pidfile.read_text().strip() == "42" + + +def test_acquire_idempotent_own_pid(tmp_path: Path) -> None: + """Calling acquire with our own pid already written → treated as stale-or-live. + + If the recorded pid IS us (live), we own it — returns (True, our_pid). + This avoids refusing to 'start' when we already wrote the file. + """ + our_pid = 42 + pidfile = tmp_path / "daemon.pid" + pidfile.write_text(str(our_pid)) + + # Make our own pid appear live in the probe. + def probe_own_live(pid: int) -> bool: + return pid == our_pid + + ok, recorded_pid = acquire( + pidfile=pidfile, + our_pid=our_pid, + liveness_probe=probe_own_live, + ) + + assert ok is True + assert recorded_pid == our_pid + + +def test_acquire_creates_parent_dirs(tmp_path: Path) -> None: + """acquire must create missing parent directories for the pidfile.""" + pidfile = tmp_path / "subdir" / "daemon.pid" + + ok, _ = acquire( + pidfile=pidfile, + our_pid=42, + liveness_probe=_live_probe, + ) + + assert ok is True + assert pidfile.exists() + + +# --------------------------------------------------------------------------- +# stop +# --------------------------------------------------------------------------- + + +def test_stop_sigterms_recorded_pid(tmp_path: Path) -> None: + """stop() sends SIGTERM to the pid in the pidfile and clears it.""" + import signal + + pidfile = tmp_path / "daemon.pid" + live_pid = 1234 + pidfile.write_text(str(live_pid)) + + kill_calls: list[tuple[int, int]] = [] + + stop( + pidfile=pidfile, + liveness_probe=_live_probe, + kill_fn=_recording_kill(kill_calls), + ) + + assert (live_pid, signal.SIGTERM) in kill_calls + # Pidfile must be cleared after stop. + assert not pidfile.exists() or pidfile.read_text().strip() == "" + + +def test_stop_no_pidfile_is_graceful(tmp_path: Path) -> None: + """stop() when no pidfile exists must not raise.""" + pidfile = tmp_path / "daemon.pid" + assert not pidfile.exists() + + stop( + pidfile=pidfile, + liveness_probe=_live_probe, + kill_fn=_never_kill, # kill must NOT be called + ) + # No assertion needed beyond 'no exception raised'. + + +def test_stop_stale_pidfile_no_kill(tmp_path: Path) -> None: + """stop() with a stale pidfile: clears file, does NOT call kill.""" + pidfile = tmp_path / "daemon.pid" + pidfile.write_text(str(_DEAD_PID)) + + stop( + pidfile=pidfile, + liveness_probe=_live_probe, + kill_fn=_never_kill, # kill must NOT be called for dead pid + ) + + # File should be cleaned up. + assert not pidfile.exists() or pidfile.read_text().strip() == "" + + +def test_stop_clears_pidfile_after_kill(tmp_path: Path) -> None: + """After SIGTERM is sent, the pidfile is removed.""" + import signal + + pidfile = tmp_path / "daemon.pid" + pidfile.write_text("1234") + kill_calls: list[tuple[int, int]] = [] + + stop( + pidfile=pidfile, + liveness_probe=_live_probe, + kill_fn=_recording_kill(kill_calls), + ) + + assert len(kill_calls) == 1 + assert kill_calls[0] == (1234, signal.SIGTERM) + assert not pidfile.exists() + + +# --------------------------------------------------------------------------- +# daemon_status +# --------------------------------------------------------------------------- + + +def test_daemon_status_running_live_pid(tmp_path: Path) -> None: + """Live pid recorded → DaemonStatus(running=True, pid=N).""" + pidfile = tmp_path / "daemon.pid" + pidfile.write_text("5678") + + def probe(pid: int) -> bool: + return pid == 5678 + + status = daemon_status(pidfile=pidfile, liveness_probe=probe) + + assert isinstance(status, DaemonStatus) + assert status.running is True + assert status.pid == 5678 + + +def test_daemon_status_no_pidfile(tmp_path: Path) -> None: + """No pidfile → DaemonStatus(running=False, pid=None).""" + pidfile = tmp_path / "daemon.pid" + + status = daemon_status(pidfile=pidfile, liveness_probe=_live_probe) + + assert status.running is False + assert status.pid is None + + +def test_daemon_status_stale_pid(tmp_path: Path) -> None: + """Pidfile with dead pid → DaemonStatus(running=False, pid=None).""" + pidfile = tmp_path / "daemon.pid" + pidfile.write_text(str(_DEAD_PID)) + + status = daemon_status(pidfile=pidfile, liveness_probe=_live_probe) + + assert status.running is False + assert status.pid is None + + +def test_daemon_status_returns_dataclass(tmp_path: Path) -> None: + """Return type must be DaemonStatus with .running and .pid attributes.""" + pidfile = tmp_path / "daemon.pid" + + status = daemon_status(pidfile=pidfile, liveness_probe=_live_probe) + + assert hasattr(status, "running") + assert hasattr(status, "pid") diff --git a/tests/unit/test_daemon_lifecycle_cmd.py b/tests/unit/test_daemon_lifecycle_cmd.py new file mode 100644 index 0000000..9579280 --- /dev/null +++ b/tests/unit/test_daemon_lifecycle_cmd.py @@ -0,0 +1,207 @@ +"""CLI integration tests for daemon status, daemon stop, and run's single-instance guard. + +Tests: + daemon status: + - prints "running" + pid when daemon is live + - prints "not running" when no daemon is running + + daemon stop: + - prints "stopped" when live daemon is found and SIGTERM sent + - prints "no daemon running" when no pidfile + + bach daemon run (guard): + - refuses (non-zero exit) when a live daemon is already recorded + - starts normally (calls run_supervisor) when no prior daemon is running + - clears pidfile on clean shutdown +""" + +from pathlib import Path +from unittest.mock import MagicMock, patch + +from typer.testing import CliRunner + +from bach.cli.app import app +from bach.config.settings import BachConfig + +runner = CliRunner() + + +# --------------------------------------------------------------------------- +# daemon status +# --------------------------------------------------------------------------- + + +def test_daemon_status_reports_running(tmp_path: Path) -> None: + """daemon status must say 'running' with the pid when daemon is live.""" + from bach.services.daemon_lifecycle import DaemonStatus + + live_status = DaemonStatus(running=True, pid=1234) + + with ( + patch("bach.cli.daemon_cmd.daemon_status", return_value=live_status), + patch("bach.cli.daemon_cmd.list_supervised_artifacts", return_value=set()), + patch("bach.cli.daemon_cmd._pidfile_path", return_value=tmp_path / "daemon.pid"), + ): + result = runner.invoke(app, ["daemon", "status"]) + + assert result.exit_code == 0, result.output + assert "1234" in result.output + # Some affirmative word about running state + assert any(w in result.output.lower() for w in ("running", "live", "active")) + + +def test_daemon_status_reports_not_running(tmp_path: Path) -> None: + """daemon status must say 'not running' when no live daemon is found.""" + from bach.services.daemon_lifecycle import DaemonStatus + + dead_status = DaemonStatus(running=False, pid=None) + + with ( + patch("bach.cli.daemon_cmd.daemon_status", return_value=dead_status), + patch("bach.cli.daemon_cmd.list_supervised_artifacts", return_value=set()), + patch("bach.cli.daemon_cmd._pidfile_path", return_value=tmp_path / "daemon.pid"), + ): + result = runner.invoke(app, ["daemon", "status"]) + + assert result.exit_code == 0, result.output + assert any(w in result.output.lower() for w in ("not running", "stopped", "no daemon")) + + +def test_daemon_status_shows_supervised_artifacts(tmp_path: Path) -> None: + """daemon status includes the supervised artifact set.""" + from bach.services.daemon_lifecycle import DaemonStatus + + live_status = DaemonStatus(running=True, pid=5678) + artifact = tmp_path / "task-001.md" + artifact.touch() + + with ( + patch("bach.cli.daemon_cmd.daemon_status", return_value=live_status), + patch( + "bach.cli.daemon_cmd.list_supervised_artifacts", + return_value={artifact}, + ), + patch("bach.cli.daemon_cmd._pidfile_path", return_value=tmp_path / "daemon.pid"), + ): + result = runner.invoke(app, ["daemon", "status"]) + + assert result.exit_code == 0 + assert "task-001" in result.output + + +# --------------------------------------------------------------------------- +# daemon stop +# --------------------------------------------------------------------------- + + +def test_daemon_stop_sends_sigterm_and_reports(tmp_path: Path) -> None: + """daemon stop calls stop() and prints a success message.""" + stop_called = [] + + def _fake_stop(*, pidfile: Path, **kwargs: object) -> None: + stop_called.append(pidfile) + + from bach.services.daemon_lifecycle import DaemonStatus + + live_status = DaemonStatus(running=True, pid=1234) + + with ( + patch("bach.cli.daemon_cmd.daemon_status", return_value=live_status), + patch("bach.cli.daemon_cmd.stop", side_effect=_fake_stop), + patch("bach.cli.daemon_cmd._pidfile_path", return_value=tmp_path / "daemon.pid"), + ): + result = runner.invoke(app, ["daemon", "stop"]) + + assert result.exit_code == 0, result.output + assert len(stop_called) == 1 + # Some word indicating success + assert any(w in result.output.lower() for w in ("stopped", "sent sigterm", "stop")) + + +def test_daemon_stop_no_daemon_graceful(tmp_path: Path) -> None: + """daemon stop with no live daemon: prints informative message, exit 0.""" + from bach.services.daemon_lifecycle import DaemonStatus + + dead_status = DaemonStatus(running=False, pid=None) + + with ( + patch("bach.cli.daemon_cmd.daemon_status", return_value=dead_status), + patch("bach.cli.daemon_cmd._pidfile_path", return_value=tmp_path / "daemon.pid"), + ): + result = runner.invoke(app, ["daemon", "stop"]) + + assert result.exit_code == 0, result.output + assert any(w in result.output.lower() for w in ("no daemon", "not running", "nothing")) + + +# --------------------------------------------------------------------------- +# daemon run — single-instance guard integration +# --------------------------------------------------------------------------- + + +def test_daemon_run_refuses_when_live_daemon(tmp_path: Path) -> None: + """daemon run exits non-zero when a live daemon is already recorded.""" + + # Simulate acquire failing (daemon already running) + def _fake_acquire(*, pidfile: Path, our_pid: int, **kwargs: object) -> tuple[bool, int]: + return False, 9999 + + with ( + patch("bach.cli.daemon_cmd.acquire", side_effect=_fake_acquire), + patch("bach.cli.daemon_cmd._pidfile_path", return_value=tmp_path / "daemon.pid"), + patch("bach.cli.daemon_cmd.load_config", return_value=BachConfig()), + ): + result = runner.invoke(app, ["daemon", "run"]) + + assert result.exit_code != 0, f"Expected non-zero exit, got 0. Output: {result.output}" + assert "9999" in result.output or any( + w in result.output.lower() for w in ("already running", "daemon is", "running") + ) + + +def test_daemon_run_starts_when_no_prior_daemon(tmp_path: Path) -> None: + """daemon run calls run_supervisor when acquire succeeds.""" + mock_supervisor = MagicMock() + + def _fake_acquire(*, pidfile: Path, our_pid: int, **kwargs: object) -> tuple[bool, int]: + # Simulate successful acquire + pidfile.parent.mkdir(parents=True, exist_ok=True) + pidfile.write_text(str(our_pid)) + return True, our_pid + + with ( + patch("bach.cli.daemon_cmd.acquire", side_effect=_fake_acquire), + patch("bach.cli.daemon_cmd._pidfile_path", return_value=tmp_path / "daemon.pid"), + patch("bach.cli.daemon_cmd.run_supervisor", mock_supervisor), + patch("bach.cli.daemon_cmd.load_config", return_value=BachConfig()), + ): + result = runner.invoke(app, ["daemon", "run"]) + + assert result.exit_code == 0, f"Unexpected non-zero exit. Output: {result.output}" + assert mock_supervisor.call_count == 1 + + +def test_daemon_run_clears_pidfile_on_clean_shutdown(tmp_path: Path) -> None: + """After run_supervisor exits, the pidfile must be cleared.""" + pidfile = tmp_path / "daemon.pid" + + def _fake_acquire(*, pidfile: Path, our_pid: int, **kwargs: object) -> tuple[bool, int]: + pidfile.parent.mkdir(parents=True, exist_ok=True) + pidfile.write_text(str(our_pid)) + return True, our_pid + + def _fake_supervisor(**kwargs: object) -> None: + # Supervisor runs and exits immediately. + pass + + with ( + patch("bach.cli.daemon_cmd.acquire", side_effect=_fake_acquire), + patch("bach.cli.daemon_cmd._pidfile_path", return_value=pidfile), + patch("bach.cli.daemon_cmd.run_supervisor", side_effect=_fake_supervisor), + patch("bach.cli.daemon_cmd.load_config", return_value=BachConfig()), + ): + result = runner.invoke(app, ["daemon", "run"]) + + assert result.exit_code == 0 + # Pidfile must be cleaned up after run. + assert not pidfile.exists() or pidfile.read_text().strip() == "" diff --git a/tests/unit/test_daemon_sessions_filter.py b/tests/unit/test_daemon_sessions_filter.py new file mode 100644 index 0000000..d7ba597 --- /dev/null +++ b/tests/unit/test_daemon_sessions_filter.py @@ -0,0 +1,242 @@ +"""Tests for list_supervisable_artifacts in sessions_service.py. + +Verifies the filtering logic: + - Includes task-linked rows with non-ended liveness and non-terminal status. + - Excludes rows with no task_id (unlinked). + - Excludes rows whose session liveness is `ended`. + - Excludes rows whose task status is a terminal status (done, carried_forward). +""" + +from datetime import UTC, datetime +from pathlib import Path +from unittest.mock import patch + +from bach.domain.models import ( + AgentRuntime, + DiscoveredSession, + SessionLiveness, +) +from bach.services.sessions_service import SessionRow, list_supervisable_artifacts + + +def _make_session( + session_id: str, + liveness: SessionLiveness = SessionLiveness.running, + project_dir: Path | None = None, +) -> DiscoveredSession: + """Helper to build a DiscoveredSession for tests.""" + return DiscoveredSession( + runtime=AgentRuntime.claude_code, + session_id=session_id, + transcript_path=Path("/fake/transcript.jsonl"), + project_dir=project_dir or Path("/fake/project"), + liveness=liveness, + last_activity=datetime.now(UTC), + started_at=datetime.now(UTC), + ) + + +def _make_row( + session_id: str, + task_id: str | None, + artifact_path: Path | None, + liveness: SessionLiveness = SessionLiveness.running, + status: str = "executing", +) -> SessionRow: + """Helper to build a SessionRow for tests.""" + return SessionRow( + session=_make_session(session_id, liveness=liveness), + task_id=task_id, + artifact_path=artifact_path, + preview=None, + ) + + +# --------------------------------------------------------------------------- +# Helpers to simulate artifact frontmatter reads +# --------------------------------------------------------------------------- + + +def _patch_frontmatter(artifact_path: Path, status: str) -> dict[str, str]: + """Return a fake frontmatter dict for the given artifact path.""" + return {"status": status} + + +# --------------------------------------------------------------------------- +# Tests for list_supervisable_artifacts +# --------------------------------------------------------------------------- + + +def test_supervisable_includes_linked_running_artifact(tmp_path: Path) -> None: + """A task-linked, running, non-terminal row should be included.""" + artifact = tmp_path / "task-001.md" + artifact.write_text("---\nstatus: executing\n---\n") + + row = _make_row( + session_id="abc123", + task_id="t1", + artifact_path=artifact, + liveness=SessionLiveness.running, + status="executing", + ) + + with patch( + "bach.services.sessions_service._read_artifact_frontmatter", + return_value={"status": "executing"}, + ): + result = list_supervisable_artifacts(rows=[row]) + + assert artifact in result + + +def test_supervisable_excludes_unlinked_row(tmp_path: Path) -> None: + """A row with no task_id (no artifact) is excluded.""" + row = _make_row( + session_id="abc123", + task_id=None, + artifact_path=None, + liveness=SessionLiveness.running, + status="executing", + ) + + result = list_supervisable_artifacts(rows=[row]) + assert result == set() + + +def test_supervisable_excludes_ended_session(tmp_path: Path) -> None: + """A row whose session is ended is excluded.""" + artifact = tmp_path / "task-001.md" + artifact.write_text("---\nstatus: executing\n---\n") + + row = _make_row( + session_id="abc123", + task_id="t1", + artifact_path=artifact, + liveness=SessionLiveness.ended, + status="executing", + ) + + result = list_supervisable_artifacts(rows=[row]) + assert result == set() + + +def test_supervisable_excludes_done_task(tmp_path: Path) -> None: + """A task with status=done is terminal and excluded.""" + artifact = tmp_path / "task-001.md" + artifact.write_text("---\nstatus: done\n---\n") + + row = _make_row( + session_id="abc123", + task_id="t1", + artifact_path=artifact, + liveness=SessionLiveness.idle, + status="done", + ) + + with patch( + "bach.services.sessions_service._read_artifact_frontmatter", + return_value={"status": "done"}, + ): + result = list_supervisable_artifacts(rows=[row]) + + assert result == set() + + +def test_supervisable_excludes_carried_forward_task(tmp_path: Path) -> None: + """A task with status=carried_forward is terminal and excluded.""" + artifact = tmp_path / "task-002.md" + artifact.write_text("---\nstatus: carried_forward\n---\n") + + row = _make_row( + session_id="abc456", + task_id="t2", + artifact_path=artifact, + liveness=SessionLiveness.idle, + status="carried_forward", + ) + + with patch( + "bach.services.sessions_service._read_artifact_frontmatter", + return_value={"status": "carried_forward"}, + ): + result = list_supervisable_artifacts(rows=[row]) + + assert result == set() + + +def test_supervisable_includes_idle_non_terminal(tmp_path: Path) -> None: + """Idle but not ended — if task is non-terminal, include it.""" + artifact = tmp_path / "task-003.md" + artifact.write_text("---\nstatus: executing\n---\n") + + row = _make_row( + session_id="abc789", + task_id="t3", + artifact_path=artifact, + liveness=SessionLiveness.idle, + status="executing", + ) + + with patch( + "bach.services.sessions_service._read_artifact_frontmatter", + return_value={"status": "executing"}, + ): + result = list_supervisable_artifacts(rows=[row]) + + assert artifact in result + + +def test_supervisable_returns_set_of_paths(tmp_path: Path) -> None: + """Return type is a set of Paths (not rows, not session IDs).""" + a1 = tmp_path / "task-001.md" + a2 = tmp_path / "task-002.md" + a1.write_text("---\nstatus: executing\n---\n") + a2.write_text("---\nstatus: qa\n---\n") + + rows = [ + _make_row("s1", "t1", a1, SessionLiveness.running, "executing"), + _make_row("s2", "t2", a2, SessionLiveness.running, "qa"), + ] + + frontmatter_map = { + a1: {"status": "executing"}, + a2: {"status": "qa"}, + } + + def fake_read(path: Path) -> dict[str, str]: + return frontmatter_map.get(path, {}) + + with patch("bach.services.sessions_service._read_artifact_frontmatter", side_effect=fake_read): + result = list_supervisable_artifacts(rows=rows) + + assert result == {a1, a2} + + +def test_supervisable_includes_unreadable_artifact(tmp_path: Path) -> None: + """An artifact whose frontmatter cannot be read is included, not silently dropped. + + Design decision: an unreadable artifact is treated as having an unknown + (non-terminal) status and is included in the supervised set. The watcher + process will log the missing artifact and exit itself — that is the correct + failure path, not silently skipping supervision and leaving a session unwatched. + """ + artifact = tmp_path / "task-corrupt.md" + artifact.write_text("this is not valid yaml frontmatter") + + row = _make_row( + session_id="abc999", + task_id="t9", + artifact_path=artifact, + liveness=SessionLiveness.running, + status="executing", + ) + + # _read_artifact_frontmatter returns None for unreadable artifacts. + with patch( + "bach.services.sessions_service._read_artifact_frontmatter", + return_value=None, + ): + result = list_supervisable_artifacts(rows=[row]) + + # Must be INCLUDED — watcher will handle the unreadable file gracefully. + assert artifact in result diff --git a/tests/unit/test_daemon_supervisor.py b/tests/unit/test_daemon_supervisor.py new file mode 100644 index 0000000..61b70b7 --- /dev/null +++ b/tests/unit/test_daemon_supervisor.py @@ -0,0 +1,350 @@ +"""Tests for services/daemon_supervisor.py. + +Strategy: + - Test reconcile() as a pure function — no I/O. + - Test run_supervisor with fully injected fakes (no real processes, no real sleep). + - Use a fake clock for backoff time control. + - Verify the duplicate-watcher invariant holds strictly. + - Verify clean loop exit when should_stop_fn fires. +""" + +from pathlib import Path + +import pytest + +from bach.services.daemon_supervisor import ( + BackoffState, + reconcile, + run_supervisor, +) + +# --------------------------------------------------------------------------- +# reconcile — pure function tests +# --------------------------------------------------------------------------- + + +def test_reconcile_spawns_desired_not_supervised(tmp_path: Path) -> None: + """A desired artifact not yet in supervised set → plan includes it.""" + artifact = tmp_path / "task-001.md" + desired = {artifact} + supervised: set[Path] = set() + backoff: BackoffState = {} + now = 1000.0 + + plan = reconcile(desired, supervised, backoff, now) + + assert artifact in plan.to_spawn + + +def test_reconcile_no_spawn_when_already_supervised(tmp_path: Path) -> None: + """Duplicate-watcher invariant: already supervised → NOT spawned again.""" + artifact = tmp_path / "task-001.md" + desired = {artifact} + supervised = {artifact} # already has a live watcher + backoff: BackoffState = {} + now = 1000.0 + + plan = reconcile(desired, supervised, backoff, now) + + assert artifact not in plan.to_spawn + + +def test_reconcile_empty_desired_produces_no_spawn() -> None: + """No desired sessions → nothing to spawn.""" + plan = reconcile(set(), set(), {}, 1000.0) + assert plan.to_spawn == set() + + +def test_reconcile_spawns_only_the_gap(tmp_path: Path) -> None: + """Two desired, one already supervised → only the gap is spawned.""" + a1 = tmp_path / "task-001.md" + a2 = tmp_path / "task-002.md" + desired = {a1, a2} + supervised = {a2} # a2 already has a watcher + backoff: BackoffState = {} + now = 1000.0 + + plan = reconcile(desired, supervised, backoff, now) + + assert a1 in plan.to_spawn + assert a2 not in plan.to_spawn + + +def test_reconcile_respects_backoff(tmp_path: Path) -> None: + """An artifact in backoff cooldown is NOT re-spawned before window expires.""" + artifact = tmp_path / "task-001.md" + desired = {artifact} + supervised: set[Path] = set() + # Mark artifact as having been spawned very recently (t=900, now=1000). + # With a typical backoff window of >=30s, this should still be on cooldown. + backoff: BackoffState = { + artifact: {"last_spawn_at": 990.0, "attempt": 1, "cooldown": 60.0} + } + now = 1000.0 # only 10s elapsed, cooldown is 60s + + plan = reconcile(desired, supervised, backoff, now) + + assert artifact not in plan.to_spawn + + +def test_reconcile_spawns_after_backoff_expires(tmp_path: Path) -> None: + """An artifact whose backoff window has elapsed IS included in the plan.""" + artifact = tmp_path / "task-001.md" + desired = {artifact} + supervised: set[Path] = set() + backoff: BackoffState = { + artifact: {"last_spawn_at": 100.0, "attempt": 1, "cooldown": 60.0} + } + now = 200.0 # 100s elapsed, cooldown is 60s → expired + + plan = reconcile(desired, supervised, backoff, now) + + assert artifact in plan.to_spawn + + +def test_reconcile_no_backoff_on_first_spawn(tmp_path: Path) -> None: + """An artifact with no backoff entry at all is spawned immediately.""" + artifact = tmp_path / "task-001.md" + desired = {artifact} + supervised: set[Path] = set() + backoff: BackoffState = {} # no entry → first attempt, no delay + now = 1000.0 + + plan = reconcile(desired, supervised, backoff, now) + + assert artifact in plan.to_spawn + + +def test_reconcile_supervised_not_in_desired_produces_no_spawn(tmp_path: Path) -> None: + """Supervised but not desired (ended session) → not in to_spawn.""" + artifact = tmp_path / "task-ended.md" + desired: set[Path] = set() + supervised = {artifact} # watcher still alive but session ended + backoff: BackoffState = {} + now = 1000.0 + + plan = reconcile(desired, supervised, backoff, now) + + assert plan.to_spawn == set() + + +# --------------------------------------------------------------------------- +# run_supervisor — injected seams, fake clock, no real processes +# --------------------------------------------------------------------------- + + +def test_run_supervisor_exits_on_first_stop(tmp_path: Path) -> None: + """should_stop_fn returning True on first call → loop exits, no spawn.""" + calls: list[str] = [] + + def enumerate_desired() -> set[Path]: + calls.append("enumerate") + return set() + + def list_supervised() -> set[Path]: + return set() + + def spawn(artifact: Path) -> None: + calls.append(f"spawn:{artifact}") + + def sleep(seconds: float) -> None: + calls.append("sleep") + + stop_calls = [True] # stop immediately + + def should_stop() -> bool: + return stop_calls.pop(0) + + run_supervisor( + enumerate_desired_fn=enumerate_desired, + list_supervised_fn=list_supervised, + spawn_fn=spawn, + sleep_fn=sleep, + should_stop_fn=should_stop, + reconcile_seconds=1, + ) + + # enumerate was called once before stop check, sleep never reached + assert "enumerate" in calls + assert "sleep" not in calls + + +def test_run_supervisor_spawns_new_session(tmp_path: Path) -> None: + """A desired+unsupervised artifact gets spawned in the first cycle.""" + artifact = tmp_path / "task-001.md" + spawned: list[Path] = [] + cycle = [0] # mutable counter for stop logic + + def enumerate_desired() -> set[Path]: + # Only return the artifact on the first cycle + return {artifact} if cycle[0] == 0 else set() + + def list_supervised() -> set[Path]: + return set() + + def spawn(a: Path) -> None: + spawned.append(a) + + def sleep(seconds: float) -> None: + cycle[0] += 1 # advance to next cycle + + def should_stop() -> bool: + return cycle[0] >= 1 # stop after first cycle+sleep + + run_supervisor( + enumerate_desired_fn=enumerate_desired, + list_supervised_fn=list_supervised, + spawn_fn=spawn, + sleep_fn=sleep, + should_stop_fn=should_stop, + reconcile_seconds=1, + ) + + assert artifact in spawned + + +def test_run_supervisor_does_not_spawn_already_supervised(tmp_path: Path) -> None: + """Duplicate-watcher invariant: already supervised → spawn never called.""" + artifact = tmp_path / "task-001.md" + spawned: list[Path] = [] + cycle = [0] + + def enumerate_desired() -> set[Path]: + return {artifact} + + def list_supervised() -> set[Path]: + return {artifact} # already supervised + + def spawn(a: Path) -> None: + spawned.append(a) + + def sleep(seconds: float) -> None: + cycle[0] += 1 + + def should_stop() -> bool: + return cycle[0] >= 1 + + run_supervisor( + enumerate_desired_fn=enumerate_desired, + list_supervised_fn=list_supervised, + spawn_fn=spawn, + sleep_fn=sleep, + should_stop_fn=should_stop, + reconcile_seconds=1, + ) + + assert spawned == [] + + +def test_run_supervisor_stops_when_should_stop_returns_true(tmp_path: Path) -> None: + """Loop exits cleanly after should_stop_fn fires.""" + artifact = tmp_path / "task.md" + cycle = [0] + stop_after = 3 # run for 3 sleep cycles then stop + + def enumerate_desired() -> set[Path]: + return {artifact} + + def list_supervised() -> set[Path]: + return {artifact} # always supervised, so no spawning + + def spawn(a: Path) -> None: + pytest.fail("spawn should not be called when artifact is supervised") + + def sleep(seconds: float) -> None: + cycle[0] += 1 + + def should_stop() -> bool: + return cycle[0] >= stop_after + + run_supervisor( + enumerate_desired_fn=enumerate_desired, + list_supervised_fn=list_supervised, + spawn_fn=spawn, + sleep_fn=sleep, + should_stop_fn=should_stop, + reconcile_seconds=5, + ) + + assert cycle[0] == stop_after + + +def test_run_supervisor_uses_injected_sleep_duration(tmp_path: Path) -> None: + """sleep_fn is called with the configured reconcile_seconds.""" + sleep_durations: list[float] = [] + cycle = [0] + + def enumerate_desired() -> set[Path]: + return set() + + def list_supervised() -> set[Path]: + return set() + + def spawn(a: Path) -> None: + pass + + def sleep(seconds: float) -> None: + sleep_durations.append(seconds) + cycle[0] += 1 + + def should_stop() -> bool: + return cycle[0] >= 2 + + run_supervisor( + enumerate_desired_fn=enumerate_desired, + list_supervised_fn=list_supervised, + spawn_fn=spawn, + sleep_fn=sleep, + should_stop_fn=should_stop, + reconcile_seconds=42, + ) + + assert all(d == 42 for d in sleep_durations) + assert len(sleep_durations) == 2 + + +def test_run_supervisor_backoff_prevents_immediate_respawn(tmp_path: Path) -> None: + """After spawn, the supervisor must NOT re-spawn the same artifact on the + very next cycle if the watcher crashed immediately (watcher absent from + supervised before backoff window expires). + + This is the core crash-backoff invariant: spawn_fn is called at most once + per artifact per backoff window, even if the watcher disappears between cycles. + """ + artifact = tmp_path / "task-crash.md" + spawn_calls: list[int] = [] # record which cycle each spawn happened on + cycle = [0] + + def enumerate_desired() -> set[Path]: + return {artifact} + + def list_supervised() -> set[Path]: + # Watcher is never seen as supervised — simulates immediate crash. + return set() + + def spawn(a: Path) -> None: + spawn_calls.append(cycle[0]) + + def sleep(seconds: float) -> None: + cycle[0] += 1 + + def should_stop() -> bool: + # Run 3 cycles: cycle 0 spawns, cycles 1 and 2 should be skipped by backoff. + return cycle[0] >= 3 + + run_supervisor( + enumerate_desired_fn=enumerate_desired, + list_supervised_fn=list_supervised, + spawn_fn=spawn, + sleep_fn=sleep, + should_stop_fn=should_stop, + reconcile_seconds=1, + ) + + # spawn must only fire ONCE in cycles 0..2 because backoff cooldown > 1s (reconcile). + # The initial backoff is _BACKOFF_INITIAL_S=10s which is >> reconcile_seconds=1s. + assert len(spawn_calls) == 1, ( + f"Expected 1 spawn across 3 cycles (backoff should prevent re-spawn), " + f"got {len(spawn_calls)} at cycles {spawn_calls}" + ) + assert spawn_calls[0] == 0, "Spawn must occur on the first cycle" diff --git a/tests/unit/test_launchd_service.py b/tests/unit/test_launchd_service.py new file mode 100644 index 0000000..1e2b64b --- /dev/null +++ b/tests/unit/test_launchd_service.py @@ -0,0 +1,324 @@ +"""Tests for services/launchd_service.py — macOS launchd LaunchAgent management. + +Strategy (mirrors test_hooks_service.py): + - `render_launch_agent` is a pure function — assert plist content directly. + - `install` / `uninstall` / `get_status` accept an injected `runner` callable + so tests NEVER call real launchctl or write to ~/Library/LaunchAgents. + - Plist-file writes are directed at tmp_path via an injected `launch_agents_dir`. + - Idempotency: install/uninstall twice must not produce errors or duplicate state. + - Fallback: when the launchctl runner returns failure, install must surface the + manual `bach daemon run` fallback command (returned in the result). +""" + +import plistlib +import sys +from pathlib import Path +from typing import Any + +from bach.services.launchd_service import ( + LAUNCH_AGENT_LABEL, + LaunchdInstallResult, + install_launch_agent, + render_launch_agent, + status_launch_agent, + uninstall_launch_agent, +) + +# --------------------------------------------------------------------------- +# Constants +# --------------------------------------------------------------------------- + +# The expected ProgramArguments must start with the current interpreter and end +# with ["daemon", "run"]. We do not hard-code the exact path so tests pass +# regardless of which venv pytest runs from. +_EXPECTED_ARGS_TAIL = ["-m", "bach", "daemon", "run"] + +# A fake runner that always succeeds (exit-code 0, empty output). +_OK_RUNNER = lambda argv: (0, "") # noqa: E731 + +# A fake runner that always fails (exit-code 1). +_FAIL_RUNNER = lambda argv: (1, "service not found") # noqa: E731 + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _parse_plist(xml: str) -> dict[str, Any]: + """Parse plist XML string into a Python dict.""" + result: dict[str, Any] = plistlib.loads(xml.encode()) + return result + + +def _write_plist(launch_agents_dir: Path, label: str, data: dict[str, Any]) -> Path: + """Write a plist file and return its path (used for pre-condition setup).""" + launch_agents_dir.mkdir(parents=True, exist_ok=True) + plist_path = launch_agents_dir / f"{label}.plist" + plist_path.write_bytes(plistlib.dumps(data)) + return plist_path + + +# --------------------------------------------------------------------------- +# render_launch_agent — plist content assertions +# --------------------------------------------------------------------------- + + +def test_render_returns_string() -> None: + """render_launch_agent must return a non-empty string.""" + xml = render_launch_agent(log_dir=Path("/tmp/logs")) + assert isinstance(xml, str) + assert len(xml) > 0 + + +def test_render_plist_is_valid_xml() -> None: + """The rendered output must be valid plist XML parseable by plistlib.""" + xml = render_launch_agent(log_dir=Path("/tmp/logs")) + data = _parse_plist(xml) + assert isinstance(data, dict) + + +def test_render_plist_has_correct_label() -> None: + """The plist Label must equal LAUNCH_AGENT_LABEL constant.""" + xml = render_launch_agent(log_dir=Path("/tmp/logs")) + data = _parse_plist(xml) + assert data["Label"] == LAUNCH_AGENT_LABEL + + +def test_render_plist_has_program_arguments() -> None: + """ProgramArguments must be a list starting with sys.executable.""" + xml = render_launch_agent(log_dir=Path("/tmp/logs")) + data = _parse_plist(xml) + args = data["ProgramArguments"] + assert isinstance(args, list) + assert args[0] == sys.executable, ( + f"First arg must be sys.executable, got: {args[0]!r}" + ) + + +def test_render_plist_program_arguments_invoke_daemon_run() -> None: + """ProgramArguments tail must be ['-m', 'bach', 'daemon', 'run'].""" + xml = render_launch_agent(log_dir=Path("/tmp/logs")) + data = _parse_plist(xml) + args = data["ProgramArguments"] + assert args[-4:] == _EXPECTED_ARGS_TAIL, ( + f"Expected tail {_EXPECTED_ARGS_TAIL!r}, got: {args!r}" + ) + + +def test_render_plist_has_run_at_load_true() -> None: + """RunAtLoad must be True so the agent starts on login.""" + xml = render_launch_agent(log_dir=Path("/tmp/logs")) + data = _parse_plist(xml) + assert data.get("RunAtLoad") is True + + +def test_render_plist_has_keep_alive_true() -> None: + """KeepAlive must be True so launchd restarts the daemon on crash.""" + xml = render_launch_agent(log_dir=Path("/tmp/logs")) + data = _parse_plist(xml) + assert data.get("KeepAlive") is True + + +def test_render_plist_has_stdout_path() -> None: + """StandardOutPath must point under the given log_dir.""" + log_dir = Path("/tmp/bach/logs") + xml = render_launch_agent(log_dir=log_dir) + data = _parse_plist(xml) + stdout = data.get("StandardOutPath", "") + assert str(log_dir) in stdout, ( + f"StandardOutPath {stdout!r} does not reference log_dir {log_dir}" + ) + + +def test_render_plist_has_stderr_path() -> None: + """StandardErrorPath must point under the given log_dir.""" + log_dir = Path("/tmp/bach/logs") + xml = render_launch_agent(log_dir=log_dir) + data = _parse_plist(xml) + stderr = data.get("StandardErrorPath", "") + assert str(log_dir) in stderr, ( + f"StandardErrorPath {stderr!r} does not reference log_dir {log_dir}" + ) + + +# --------------------------------------------------------------------------- +# LAUNCH_AGENT_LABEL constant +# --------------------------------------------------------------------------- + + +def test_label_is_reverse_dns() -> None: + """The label must be a stable reverse-DNS string starting with 'com.'.""" + assert LAUNCH_AGENT_LABEL.startswith("com."), ( + f"Label {LAUNCH_AGENT_LABEL!r} must start with 'com.'" + ) + + +# --------------------------------------------------------------------------- +# install_launch_agent — happy path +# --------------------------------------------------------------------------- + + +def test_install_creates_plist_file(tmp_path: Path) -> None: + """install must write a plist file at launch_agents_dir/