diff --git a/docs/TODO.md b/docs/TODO.md index 2e0f885..7e66123 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -1,9 +1,120 @@ # vigil TODO -Last Updated: 2026-07-02 +Last Updated: 2026-07-14 Planned work and deferred decisions. +## TODO-003: Daemon decision introspection + +**Status:** Planned. Motivated by the 2026-07-14 investigation into a daemon that +reported active sessions but held nothing. The daemon runs detached with +`/dev/null` stdio, so its decision state is invisible: diagnosing it needed a +`sample` stack trace and a restart experiment, and two wrong guesses (a legitimate +battery cap, then a deadlock) before `pmset -g log` and a restart isolated the real +causes (see session `2026-07-14_1329_turn-span-safe-upgrade-and-overnight-fix`). + +Expose the daemon's live decision so `vigil status` can answer "why is it (not) +holding" in one command, distinct from status's own independent recomputation. + +### Design sketch + +Each housekeeping tick (or on each decision change), the daemon writes a small +state file to the runtime dir, for example `${VIGIL_RUNTIME_DIR}/daemon-state.json`, +with the fields it just computed: `ts`, `active`, `want_hold`, `battery_capped`, +`hold_since`, `on_ac`, `charge`, and the daemon pid. `vigil status` reads it and +prints the daemon's actual decision and the reason it holds or releases. A stale +`ts` (older than a few `POLL_INTERVAL`s) signals a wedged or dead daemon, which the +current pgrep-based "daemon running" line cannot distinguish from a healthy idle +one. + +### Work + +- Add a serde state struct and an atomic write (temp + rename, as + `write_settings_backed_up` does) each tick in `daemon::run`. +- Read and render it in `daemon::status`, including a staleness note. +- Decide write cadence: every tick is simplest; on-change avoids churn but adds + state. Every tick is fine (one small file write per `POLL_INTERVAL`). +- Clean up the state file on clean exit (self-exit, disable flag, self-upgrade). +- Consider an opt-in append-only debug log (`$VIGIL_DEBUG` env) for a rolling + history rather than only the latest snapshot, if the snapshot proves too thin. + +### Acceptance + +- `vigil status` shows why the daemon holds or releases (active, want_hold, and the + battery-cap reason) from the daemon's own last decision, not a recomputation. +- A wedged or dead daemon is distinguishable from a healthy idle one via state + staleness. + +### Related + +- Session `2026-07-14_1329_turn-span-safe-upgrade-and-overnight-fix` (the diagnosis + that motivated this) +- ADR-0013 (battery interactions), ADR-0007 (daemon lifecycle, detached stdio) + +## TODO-002: Turn-span activity model (phase 1) + +**Status:** Phase 1 built on branch `feat/turn-span-model`, not yet merged or +deployed (session `2026-07-14_1329_turn-span-safe-upgrade-and-overnight-fix`). Two +follow-up fixes landed on the same branch after real-use testing: the awaiting-input +release set was missing `idle_prompt`/`agent_completed` (held the display overnight), +and the battery max-hold counted total hold instead of battery-only hold. Design in +ADR-0013. Supersedes the timeout-driven release from ADR-0006 and dissolves the +commit-aware timeout from ADR-0005. Motivated by the 2026-07-13 mid-turn +display-sleep incident (session `684e315d`), where a 245s gap with no hook events +elapsed the 120s timeout and released the hold. + +Replace the daemon's activity test with turn-span: a session is active while its +log exists, its process is alive, its transcript's newest line is not the interrupt +marker, its newest log line is not an awaiting-input `Notification`, and its log age +is under a long safety cap. Verification on 2026-07-13 established that no +incremental signal (hooks or transcript) survives a long thinking block, so the +turn boundary is the only reliable span marker (full evidence in ADR-0013). + +### Phase 1 work + +- **Recorder / event schema.** Wire `Notification` as a seventh hook event; append a + line carrying `notification_type`. Add the optional `notification_type` field to + the `Event` schema (`event.rs`). Keep `Stop`/`StopFailure`/`SessionEnd` as + log-deleting terminal events. +- **Activity test.** Rewrite the freshness check in `evaluate_sessions` (`daemon.rs`) + to turn-span. A session whose newest line is one of `permission_prompt`, + `agent_needs_input`, `elicitation_dialog` is awaiting-input and not active, with a + ~90s grace before release. +- **Remove the commit apparatus.** Delete `COMMIT_TIMEOUT`, `STANDARD_TIMEOUT`, + `is_unmatched_commit`, `is_commit_command`, `is_git_commit_segment`, and their + tests. The commit hold falls out of turn-span. +- **Safety cap.** A single long absolute cap on log age (target 12h), derived from + file state so it survives daemon restarts. Replaces `GC_THRESHOLD` as the primary + backstop, or sits alongside it. +- **Scan-time interrupt check.** Check the interrupt marker when a session is first + evaluated, alongside the liveness check, so a missed marker is caught at scan time + rather than held to the safety cap. +- **Battery-floor invariant.** Keep the battery cap a veto over the hold decision + (`want_hold = active AND (on_ac OR NOT battery_capped)`). Extract the cap decision + into a pure function and unit-test the active-on-battery-at-floor release case, so + the invariant cannot silently regress. This extraction also serves TODO-001. +- **status.** Report per-session state (working, tool-in-flight, awaiting-input). + +### Acceptance + +- Unit tests: turn-span activity given a synthetic newest line (each state), + awaiting-input release, battery-floor veto while active, safety-cap release. +- `cargo test`, `clippy -D warnings`, `fmt --check` green. +- Manual: re-run the incident shape (a turn with a >120s tool-free gap) and confirm + the hold spans it; confirm a permission prompt releases within the grace; confirm + on battery at the floor the hold releases mid-turn. + +### Deferred to phase 2 (only if warranted) + +- A shorter cap keyed on the newest event type (long for `PreToolUse` tool-in-flight, + medium for between-tools states), gated on real usage showing missed-`Stop` leaks. +- Live confirmation of `idle_prompt` and `StopFailure` (documented, not captured). + +### Related + +- ADR-0013 (`adr/0013-turn-span-activity-model.md`) +- ADR-0006 (superseded), ADR-0005 (dissolved), ADR-0009 (battery cap, retained) + ## TODO-001: Graduated battery response (charge-tiered max-hold) **Status:** Deferred. v1 ships the two-guard form (floor + single max-hold) from diff --git a/docs/adr/0013-turn-span-activity-model.md b/docs/adr/0013-turn-span-activity-model.md new file mode 100644 index 0000000..858f0b8 --- /dev/null +++ b/docs/adr/0013-turn-span-activity-model.md @@ -0,0 +1,211 @@ +# ADR-0013: Turn-span activity model + +**Status:** Draft + +**Date:** 2026-07-13 + +## Context + +The release model to date treats a session as active while the idle time since +its newest log line is under a timeout (ADR-0006, demoted to a backstop by +ADR-0010/0011 but still the freshness signal in `evaluate_sessions`). The timeout +is `STANDARD_TIMEOUT` (120s), or `COMMIT_TIMEOUT` (300s) while the newest line is +an in-flight commit (ADR-0005). + +On 2026-07-13 a session on battery slept the display mid-turn. Session +`684e315d-aeb4-4a7b-a63c-404d96fae6fc` produced no hook events for 245s while the +model worked (a burst of reads at 10:36:06, then the next event at 10:40:11). The +120s timeout elapsed at 10:38:07, vigil released, powerd began its 2-minute +display-off grace, and the display turned off at 10:40:17, six seconds after vigil +re-acquired at 10:40:13. The pmset log records the release +(`PID 94896(caffeinate) ClientDied ... 10:38:07`) and the re-acquire +(`PID 1677(caffeinate) Created ... 10:40:13`). A `PreventUserIdleDisplaySleep` +assertion does not cancel a display-off already in progress, so the late +re-acquire did not relight the screen. + +The stated goal is a hold that spans a whole turn: from `UserPromptSubmit` until +the turn ends or the session is waiting on the user, with no mid-turn release. + +### Verification (2026-07-13) + +Whether an incremental activity signal could carry a mid-turn hold was tested +directly. + +- **Transcript writes are not a mid-turn signal.** During the incident's 245s hook + gap the session transcript was also silent: one write at 10:36:06, the next at + 10:40:09 (243s). The earlier gap showed 139s of transcript silence. A headless + `claude -p` prose session (1500-word essay, no tools) held its transcript at + 18323 bytes for 68s, then wrote the whole body in one append to 35369 bytes at + completion. The transcript is written at message-block and tool completion, not + during token streaming, so it goes quiet during the same long thinking or + generation stretch that produces no hooks. +- **Turn boundaries are reliable.** Clean turn completion fired `Stop` in every + observed case (two headless sessions, one interactive). `SessionEnd` fired on + every session close, with `reason:"other"` on normal end and + `reason:"prompt_input_exit"` on `/exit`. +- **`Notification` reports awaiting-input.** An interactive session showing a + permission dialog fired `Notification` with + `{notification_type:"permission_prompt", message:"Claude needs your permission"}`. + The documented awaiting-input types are `permission_prompt`, `idle_prompt`, + `agent_needs_input`, and the `elicitation_*` set (Claude Code docs, hooks + reference, retrieved 2026-07-13). +- **Not reproduced live.** `idle_prompt` did not fire during a 75s wait while a + permission dialog was already open. `StopFailure` (documented for API errors) was + not induced. `PermissionRequest` wiring was added after the interactive probe ran. + +The finding that no incremental signal survives a long thinking block removes the +option of proving liveness mid-turn. What remains reliable is the turn boundary +(`UserPromptSubmit` start, `Stop`/`StopFailure`/`SessionEnd` end), process +liveness (ADR-0010), and the interrupt marker (ADR-0011). + +## Decision + +Replace the timeout-driven freshness test with a turn-span activity test. A +session is active when all of the following hold: + +- its log exists (the recorder creates it on `UserPromptSubmit` and deletes it on + `Stop`/`StopFailure`/`SessionEnd`, so an existing log means a turn is in flight), +- its `claude` process is alive (ADR-0010), +- its transcript's newest line is not the interrupt marker (ADR-0011), +- its newest log line is not an awaiting-input `Notification`, and +- its log age is under a long safety cap. + +The freshness timeout, the commit-extended timeout, and commit detection are +removed. An in-flight commit is a tool in flight like any other and is held for +the duration of the turn, so the Touch ID sheet is covered without a special case. + +### Awaiting-input + +`Notification` is wired as a seventh hook event. The recorder appends a line +carrying `notification_type`. A session whose newest line is one of +`permission_prompt`, `agent_needs_input`, `elicitation_dialog` (the user must act), +`idle_prompt` (the user has gone idle at the prompt), or `agent_completed` (the turn +finished) is not actively working and is not active. `auth_success` and +`elicitation_complete`/`_response` mean work is resuming and do not release. The next +`PreToolUse` (after approval) or a new `UserPromptSubmit` becomes the newest line and +the session is active again. + +`idle_prompt` was omitted from the release set in the first implementation. It held +the display awake overnight on 2026-07-13: an abandoned session whose newest line was +an `idle_prompt` Notification stayed active under the 12h safety cap and pinned the +assertion from 20:27 to 10:44 (per `pmset -g log`). Adding `idle_prompt` and +`agent_completed` releases such a session after the grace. + +Awaiting-input uses a short grace before release (target 90s) rather than +immediate release, so the display does not sleep while the user reads a dialog +they have not yet answered. An unattended session releases roughly 90s after the +prompt. + +### Safety cap + +A single long absolute cap on log age (target 12h) is the backstop for a turn that +never ends: a `Stop` that did not fire while the process stays alive with no +interrupt marker. No shorter cap is applied, because a long single tool (a build, +a test run) can leave the newest line unchanged for a long time while genuinely +working, and vigil cannot distinguish that from a turn whose `Stop` was missed. The +cap is derived from log age (file state), not held in daemon memory, so it survives +the daemon self-exit and respawn that a multi-turn `/loop` causes. + +### Battery floor invariant + +The battery cap (ADR-0009) is applied as a veto over the hold decision, not as part +of the activity test. `want_hold = active AND (on_ac OR NOT battery_capped)`. The +turn-span change alters only the `active` term. On battery, once charge reaches +`BATTERY_FLOOR_PCT`, `battery_capped` is set from the live `pmset` read and the hold +is released regardless of how long the turn has been active. A mega-turn held for +hours on battery is released at the floor. The floor re-derives from the current +charge on every poll, so it re-latches after a daemon restart. + +The battery-cap decision is extracted into a pure function and unit-tested, +including the case where a session is active, on battery, and at or below the floor, +and the hold is not taken. This locks the invariant against regression during the +turn-span change. + +The `BATTERY_MAX_HOLD` guard (ADR-0009) measures `now - hold_since`, where +`hold_since` marks the start of the continuous hold. Turn-span makes holds span a +whole turn rather than resetting every 120s, so a hold can run for hours on AC. The +first implementation left `hold_since` running across AC, so a multi-hour AC hold +consumed the battery budget before an unplug: on 2026-07-13 a ~14h continuous hold +(mostly on AC) tripped the 3h guard the instant the machine was unplugged, releasing +at 100% charge. `hold_since` is now cleared while on AC and set on the first battery +tick, so the max-hold guard counts battery time only and each unplug starts a fresh +budget. This matches ADR-0009's stated intent ("continuous battery-powered +holding"). Extracted into a pure `next_hold_since` and unit-tested. + +### Scan-time interrupt check + +The interrupt marker is checked when a session is first evaluated, alongside the +liveness check, in addition to the existing reactive transcript-write check. Under +the timeout model a missed marker released after 120s. Under turn-span there is no +short timeout, so a marker that is a transcript's last line with no following write +(a daemon that starts after the interrupt) is caught at scan time rather than held +to the safety cap. + +## Consequences + +**Covers the incident.** Both gaps in session `684e315d` were mid-turn with the log +present, the process alive, and no interrupt marker. Under turn-span the hold spans +the gap. + +**Failure direction inverts.** Under the timeout model a missed signal false-releases, +bounded at 120s. Under turn-span a missed end signal false-holds, bounded by the +safety cap on AC and by the battery guards on battery. `Stop` was reliable in every +observed clean end, so the missed-`Stop` case is rare; its cost is a held display +until the user returns (their next input or prompt resolves it) or the cap fires. + +**Overnight, one mega-turn.** A single long instruction is one `UserPromptSubmit` +and one `Stop`, held throughout. An un-preauthorized permission prompt releases +(awaiting-input) and the display sleeps until the user returns and approves, which +is the intended behavior for a genuinely blocked run. + +**Overnight, many turns (`/loop`).** Each iteration ends with `Stop`, releases, and +the display sleeps during the gap. Once the display sleeps and the lock engages, +Touch ID keychain reads fail until the user returns regardless of vigil, so this +matches the existing headless workflow (unsigned commits between iterations). + +**Battery unchanged.** A long turn on battery was already held continuously under +the timeout model (tool events refreshed it), so the floor was already the sole +guard against draining an unattended battery session. It remains so, now enforced +by an explicit invariant and a test. + +**Commit apparatus removed.** `COMMIT_TIMEOUT`, `is_unmatched_commit`, and the +git-commit command parser are deleted. ADR-0005 dissolves. The commit hold is a +consequence of turn-span rather than a detection heuristic. + +**Deferred.** A shorter cap keyed on the newest event type (a build sitting on +`PreToolUse` held long, a between-tools state held for a medium interval) is not +built. It keys on a reliable signal (event type) rather than the refuted transcript +signal, so it is a viable later refinement, gated on real usage showing that +missed-`Stop` leaks occur. `idle_prompt` and `StopFailure` behavior are confirmed +by documentation but not by live capture. + +## SPEC impact + +To be applied in a dedicated spec-update session: + +- "Reference counting & the staleness backstop" and "Commit-aware timeout": replace + the timeout-driven activity test with turn-span (log present, alive, not + interrupted, not awaiting-input, under the safety cap). Remove commit detection. +- "Timeouts & Configuration": remove `STANDARD_TIMEOUT` and `COMMIT_TIMEOUT`; add + the awaiting-input grace and the safety cap; note the battery-floor invariant. +- "Hook Contract / Wired Events": add `Notification` as a seventh event; record the + awaiting-input `notification_type` set. +- "Event Log / Line Schema": add `notification_type` (optional). +- "Daemon / Supervisor loop": the activity test changes; the battery veto and the + self-exit logic are unchanged. +- "CLI status": report per-session state (working, tool-in-flight, awaiting-input). + +## References + +- `../SPEC.md` sections "Reference counting & the staleness backstop", + "Commit-aware timeout", "Power source & battery cap", "Hook Contract" +- Incident 2026-07-13, session `684e315d-aeb4-4a7b-a63c-404d96fae6fc`; pmset log + release 10:38:07, re-acquire 10:40:13, display off 10:40:17 +- Verification 2026-07-13: transcript-silence measurement, headless batched-write + measurement, `Notification` permission_prompt capture +- Claude Code hooks reference (retrieved 2026-07-13): `Notification` types, + `StopFailure`, `PermissionRequest` +- ADR-0006 (staleness release, superseded by this ADR), ADR-0005 (commit-aware + timeout, dissolved), ADR-0010 (process liveness, retained), ADR-0011 (reactive + loop and interrupt marker, retained), ADR-0009 (battery cap, retained as the hold + veto), ADR-0012 (reactive log-directory watch, retained) diff --git a/docs/adr/0014-safe-self-upgrade.md b/docs/adr/0014-safe-self-upgrade.md new file mode 100644 index 0000000..4e76d5e --- /dev/null +++ b/docs/adr/0014-safe-self-upgrade.md @@ -0,0 +1,141 @@ +# ADR-0014: Install modes, safe binary replacement, and daemon self-upgrade + +**Status:** Draft + +**Date:** 2026-07-13 + +## Context + +`vigil install --force` copied the new binary over the installed path with +`fs::copy`, which opens the destination with truncate and rewrites its bytes in +place. On 2026-07-13, running `install --force` while the daemon was executing +that path produced a binary that was SIGKILLed on every exec (observed exit 137). +macOS invalidates the code signature of a Mach-O whose bytes are rewritten while a +process maps it, and the kernel kills the next exec. Recovery required removing the +file first to force a fresh inode. + +Three problems sit behind this and the broader upgrade story: + +1. **Corruption.** Overwriting the running binary in place invalidates its + signature. A property of the write technique, independent of the daemon. +2. **Freshness.** After a safe replacement, the old daemon keeps executing its old + inode until it restarts, so new code is not live until then. +3. **Distribution.** vigil should install from a local build, from `cargo install` + (which places the binary at `${CARGO_HOME:-~/.cargo}/bin/vigil`), or from + Homebrew (which places each version under `/Cellar/vigil//…` and + points `/bin/vigil` at it). The binary lands in a different, and for + Homebrew a versioned, location per channel. + +## Decision + +### Install modes + +`vigil install` classifies where it is running from and wires the hooks at the +appropriate stable path, rather than always copying to one location: + +- **Homebrew** when `current_exe()` resolves under a `/Cellar/` path. The hooks are + wired at the stable front door `/bin/vigil` (the part of the path before + `/Cellar/`, plus `bin/vigil`), and no copy is made. Homebrew owns the binary. +- **cargo** when `current_exe()` is `${CARGO_HOME:-~/.cargo}/bin/vigil`. The hooks + are wired there, no copy. cargo owns the binary. +- **own-copy** otherwise (a `target/` build artifact, a manually placed binary, or + an explicit `--dir` / `$VIGIL_INSTALL_DIR`). The binary is copied to + `${VIGIL_INSTALL_DIR:-~/.local/share/vigil}/bin/vigil` with a `~/.local/bin/vigil` + PATH symlink, and the hooks are wired there. This is the pre-existing behavior. + +An explicit `--dir` or `$VIGIL_INSTALL_DIR` forces own-copy. For the managed modes, +the binary already sits on a stable, on-`PATH` path, so vigil neither copies it nor +creates its own symlink, and `vigil uninstall` removes only the hooks and runtime +state, leaving the binary for `cargo uninstall` / `brew uninstall`. + +### 1. Atomic rename on install (own-copy) + +`copy_self` writes the new binary to a temporary file in the destination directory, +sets its mode, then `fs::rename`s it over the destination. `rename` repoints the +directory entry to a new inode; it never rewrites the bytes of the inode a running +process holds. The old daemon keeps executing its old (now-unlinked) inode with a +valid signature, and any new exec of the path resolves to the new inode. The +temporary must share the destination's filesystem for the rename to be atomic. This +removes corruption as a possibility regardless of whether the daemon runs, and is +the floor the other mechanisms build on. It applies to the own-copy path; Homebrew +and cargo install their own binaries and never overwrite a running file in place. + +### 2. Daemon self-upgrade via inode change + +At startup the daemon computes its watch path: the Homebrew front door if it is +running from a Cellar, otherwise `current_exe()`. It records the `(device, inode)` +that path resolves to (`fs::metadata` follows symlinks) and re-stats it on the +slow-housekeeping cadence (`POWER_POLL_INTERVAL`). A changed pair means a new binary +is in place, so the daemon stops its caffeinate, releases its lock, spawns a fresh +detached daemon, and exits; the new daemon acquires the lock and re-acquires the +hold. The lock is released before the spawn so the successor acquires it without +racing the old daemon's exit, and a hook-spawned successor is equally valid. + +Following symlinks on the watch path unifies all three channels. own-copy and cargo +replace a real file, so the resolved inode changes on `install --force` / +`cargo install`. Homebrew repoints the `/bin/vigil` symlink, so re-resolving +the front door yields the new Cellar inode. In every case the inode the watch path +resolves to changes, and no version string is involved. + +### 3. Reactive disable flag + +A sentinel file (`${VIGIL_RUNTIME_DIR}/.disabled`) disables the daemon. The daemon +checks for it at startup (so a daemon spawned while disabled exits at once) and on +each loop pass. Because the flag lives in the log directory the daemon already +watches with `EVFILT_VNODE`/`NOTE_WRITE` (ADR-0012), creating it wakes the daemon +reactively, and it clean-exits (killing its caffeinate) rather than being signaled. +Removing the flag lets the next hook spawn a daemon again. This is a manual off +switch, separate from the upgrade path. + +## Consequences + +**Corruption removed.** Mechanism 1 makes own-copy `install --force` safe whether or +not the daemon runs, matching the atomic-write pattern already used for +`settings.json`. Homebrew and cargo do not overwrite in place, so they are safe by +construction. + +**Native package integration.** Hooks point at the package manager's own path for +cargo and Homebrew, so a `cargo install` upgrade or a `brew upgrade` is picked up by +the self-upgrade watch without a manual step, and uninstalling vigil does not delete +a package-managed binary. + +**Upgrades go live on their own.** Mechanism 2 applies the new binary within +`POWER_POLL_INTERVAL` of the replacement across all three channels, and the daemon +still spawns lazily (install does not orchestrate the daemon). + +**Clean shutdown path.** Mechanisms 2 and 3 exit through the daemon's own cleanup +(caffeinate stopped, lock released), so neither orphans a caffeinate the way an +external SIGTERM does. This also addresses the orphaned-caffeinate failure mode of a +plain `pkill`. + +**Homebrew end-to-end is unverified until a tap exists.** The Cellar path detection +and front-door derivation are unit-tested against synthetic paths, but no crate or +tap is published yet, so a real `brew install`/`brew upgrade` cannot be exercised. +Until then a Homebrew user falls back safely to own-copy (vigil copies the binary +out of the Cellar), which functions but is not native. + +**Disable flag is sticky until removed.** While `.disabled` exists, every spawned +daemon exits at startup. Removing the file restores normal operation on the next +hook. A stranded flag does not survive a reboot, since the runtime dir is under +`/tmp`. + +## SPEC impact + +To be applied in a dedicated spec-update session: + +- "CLI Interface" / install: describe the three install modes and their hook + targets; note atomic-rename replacement for own-copy. +- "Single-instance (flock)" / "Self-exit & crash recovery": add the inode-change + self-upgrade handoff and the `.disabled` flag as daemon exit paths. +- "Timeouts & Configuration": the binary-change check runs on `POWER_POLL_INTERVAL`. + +## References + +- `../SPEC.md` sections "Daemon", "Single-instance (flock)", "CLI Interface" +- Incident 2026-07-13: `install --force` over a running daemon produced a + SIGKILLed binary (exit 137); recovered by removing the file first +- Homebrew Cellar/symlink layout (`/Cellar//`, `` + is `/opt/homebrew` on Apple Silicon, `/usr/local` on Intel); `cargo install` + rename-into-place at `${CARGO_HOME:-~/.cargo}/bin` +- ADR-0007 (daemon lifecycle and lazy spawn), ADR-0012 (directory watch reused for + the disable flag), ADR-0002 (single daemon-owned caffeinate) diff --git a/src/config.rs b/src/config.rs index 6c3ecdc..e4e4245 100644 --- a/src/config.rs +++ b/src/config.rs @@ -5,20 +5,21 @@ //! `$CLAUDE_CONFIG_DIR`, `$XDG_DATA_HOME`). use std::env; -use std::path::PathBuf; - -/// Idle release backstop. Death (`EVFILT_PROC`) and Esc-interrupt -/// (`EVFILT_VNODE`) release reactively, and `Stop`/`SessionEnd` delete the log -/// directly, so this only covers the residual: a session whose process could not -/// be watched, or activity that stops with no signal. Above the typical gap -/// between tool events, well under the 10-minute AC display-sleep timer. -pub const STANDARD_TIMEOUT: u64 = 120; - -/// Applied while a commit is in flight. No reactive signal fires during a blocked -/// commit (the tool has not returned, the process is alive, no interrupt), so this -/// timeout holds the session active through the Touch ID sheet and any -/// password-fallback entry. -pub const COMMIT_TIMEOUT: u64 = 300; +use std::path::{Path, PathBuf}; + +/// Absolute hold cap on a single turn, seconds. Turn-span holds from +/// `UserPromptSubmit` until `Stop`/interrupt/death/awaiting-input, so a session +/// with none of those signals (a `Stop` that never fired while the process stays +/// alive with no interrupt marker) is released and GC'd only here. Sized well above +/// any real turn; on battery the floor and max-hold guards bound the case first. +/// Derived from log age, so it survives a daemon restart. ADR-0013. +pub const SAFETY_CAP: u64 = 43_200; + +/// Grace before an awaiting-input session releases, seconds. A session whose newest +/// line is a permission or elicitation `Notification` is waiting on the user; this +/// holds the display briefly so it does not sleep while the user reads the dialog, +/// then releases if no answer comes. ADR-0013. +pub const AWAITING_INPUT_GRACE: u64 = 90; /// Housekeeping tick cadence, seconds, and the kqueue poll timeout. The daemon /// blocks up to this long for a reactive event, then does power, battery, and @@ -30,9 +31,6 @@ pub const POLL_INTERVAL: u64 = 2; /// Consecutive idle polls before the daemon self-exits. pub const EXIT_GRACE: u32 = 2; -/// Delete logs whose newest line is older than this, seconds. -pub const GC_THRESHOLD: u64 = 300; - /// caffeinate self-expiry backstop if the daemon dies without cleanup, seconds. pub const SAFETY_SECS: u64 = 1800; @@ -83,6 +81,12 @@ pub fn lock_path() -> PathBuf { vigil_dir().join("daemon.lock") } +/// Sentinel that disables the daemon while present. Lives in the watched runtime +/// dir so creating it wakes the daemon reactively (ADR-0014). Reboot-cleared. +pub fn disable_flag_path() -> PathBuf { + vigil_dir().join(".disabled") +} + /// Install root. `$VIGIL_INSTALL_DIR` overrides `${XDG_DATA_HOME}/vigil`. pub fn install_dir() -> PathBuf { env::var_os("VIGIL_INSTALL_DIR") @@ -100,6 +104,26 @@ pub fn symlink_path() -> PathBuf { home().join(".local").join("bin").join(BIN_NAME) } +/// Where `cargo install` places the binary: `${CARGO_HOME:-~/.cargo}/bin/vigil`. +pub fn cargo_bin_path() -> PathBuf { + env::var_os("CARGO_HOME") + .map(PathBuf::from) + .unwrap_or_else(|| home().join(".cargo")) + .join("bin") + .join(BIN_NAME) +} + +/// The Homebrew front-door path for a binary running from a Cellar, or `None` if +/// `exe` is not under a `/Cellar/` path. Homebrew installs each version under +/// `/Cellar/vigil//bin/vigil` and points `/bin/vigil` at +/// it, so the stable path is the prefix (everything before `/Cellar/`) plus +/// `bin/vigil`. Pure string surgery, so no `brew` shell-out (ADR-0014). +pub fn homebrew_front_door(exe: &Path) -> Option { + let s = exe.to_str()?; + let idx = s.find("/Cellar/")?; + Some(Path::new(&s[..idx]).join("bin").join(BIN_NAME)) +} + /// Claude Code config dir. `$CLAUDE_CONFIG_DIR` overrides `~/.claude`. pub fn claude_config_dir() -> PathBuf { env::var_os("CLAUDE_CONFIG_DIR") @@ -110,3 +134,31 @@ pub fn claude_config_dir() -> PathBuf { pub fn settings_path() -> PathBuf { claude_config_dir().join("settings.json") } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn homebrew_front_door_from_cellar_path() { + // Apple Silicon prefix. + assert_eq!( + homebrew_front_door(Path::new("/opt/homebrew/Cellar/vigil/0.1.0/bin/vigil")), + Some(PathBuf::from("/opt/homebrew/bin/vigil")) + ); + // Intel prefix. + assert_eq!( + homebrew_front_door(Path::new("/usr/local/Cellar/vigil/0.1.0/bin/vigil")), + Some(PathBuf::from("/usr/local/bin/vigil")) + ); + // Not a Cellar path. + assert_eq!( + homebrew_front_door(Path::new("/Users/x/.cargo/bin/vigil")), + None + ); + assert_eq!( + homebrew_front_door(Path::new("/Users/x/.local/share/vigil/bin/vigil")), + None + ); + } +} diff --git a/src/daemon.rs b/src/daemon.rs index 77402bb..4dfd7f4 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -1,11 +1,13 @@ //! The supervisor: single-instance lock, the reactive kqueue event loop, //! reference counting, self-exit, power-source polling, and the battery hold cap. -//! Release on process death is reactive (ADR-0011); the housekeeping tick handles -//! power, battery timers, caffeinate respawn, self-exit, and the staleness -//! backstop. +//! Activity is turn-span (ADR-0013): a session is held from `UserPromptSubmit` +//! until `Stop`/death/interrupt/awaiting-input. Release on process death is +//! reactive (ADR-0011); the housekeeping tick handles power, battery timers, +//! caffeinate respawn, self-exit, and the safety-cap backstop. use std::collections::HashSet; use std::fs::{self, File}; +use std::os::unix::fs::MetadataExt; use std::os::unix::process::CommandExt; use std::path::{Path, PathBuf}; use std::process::{Command, ExitCode, Stdio}; @@ -23,10 +25,13 @@ use crate::watch::{SessionWatch, Wake}; /// Best-effort: the single-instance lock makes a redundant spawn a no-op, so /// failures here never block a turn. pub fn ensure_running() { - let Ok(exe) = std::env::current_exe() else { - return; - }; + if let Ok(exe) = std::env::current_exe() { + spawn_daemon(&exe); + } +} +/// Spawn `vigil daemon` from `exe`, detached in a new session with null stdio. +fn spawn_daemon(exe: &Path) { let mut command = Command::new(exe); command .arg("daemon") @@ -45,11 +50,35 @@ pub fn ensure_running() { let _ = command.spawn(); } +/// The path whose inode signals that a new binary was installed: the Homebrew +/// front door when running from a Cellar (a `brew upgrade` repoints it), otherwise +/// this executable's own path (an `install --force` or `cargo install` renames a +/// new inode over it). The daemon respawns from this path so it follows a brew +/// symlink flip to the new version. ADR-0014. +fn self_watch_path() -> Option { + let exe = std::env::current_exe().ok()?; + Some(config::homebrew_front_door(&exe).unwrap_or(exe)) +} + +/// The `(device, inode)` the watch path resolves to, following symlinks. `None` if +/// it cannot be stat'd. +fn binary_ident(path: &Path) -> Option<(u64, u64)> { + let meta = fs::metadata(path).ok()?; + Some((meta.dev(), meta.ino())) +} + /// Run the supervisor loop. Acquires the single-instance lock or exits 0 if /// another daemon already holds it. pub fn run() -> Result { fs::create_dir_all(config::vigil_dir())?; + // Disabled: a sentinel file stands the daemon down (a manual off switch). + // Checked here so a hook-spawned daemon exits at once, and in the loop so + // creating the file releases reactively via the log-dir watch (ADR-0014). + if config::disable_flag_path().exists() { + return Ok(ExitCode::SUCCESS); + } + // Held for the daemon's lifetime; the flock releases on drop at return. let lock = File::create(config::lock_path())?; if lock.try_lock().is_err() { @@ -67,6 +96,10 @@ pub fn run() -> Result { let mut hold_since: Option = None; let mut idle_ticks: u32 = 0; + // Identity of the binary at our watch path, to notice a self-upgrade. + let watch_path = self_watch_path(); + let self_ident = watch_path.as_deref().and_then(binary_ident); + loop { // Reactive wait: block up to POLL_INTERVAL for a watched process to exit. // With nothing registered the kqueue would return immediately, so sleep @@ -91,40 +124,52 @@ pub fn run() -> Result { } }; + // The disable flag can appear at any time; creating it wakes the log-dir + // watch, so this releases within a wake of the file landing. + if config::disable_flag_path().exists() { + caffeinate.stop(); + return Ok(ExitCode::SUCCESS); + } + let now = event::now_secs(); if now.saturating_sub(last_power_poll) >= config::POWER_POLL_INTERVAL { if let Some(fresh) = read_power() { power = fresh; } last_power_poll = now; + + // Self-upgrade: a new binary at the watch path (a new inode there, or a + // brew front-door symlink repointed to a new Cellar) means an install + // happened, so hand off to it. Release the lock before spawning so the + // successor acquires it without racing this exit (ADR-0014). + if let (Some(orig), Some(path)) = (self_ident, watch_path.as_deref()) + && binary_ident(path).is_some_and(|cur| cur != orig) + { + caffeinate.stop(); + drop(lock); + spawn_daemon(path); + return Ok(ExitCode::SUCCESS); + } } let active = evaluate_sessions(now, &mut watch); // Battery cap: two guards OR'd, whichever fires first. The latch clears // only on AC; on battery a fired guard stays latched until AC returns or - // the daemon idle-exits. - if power.on_ac { - battery_capped = false; - } else if power.charge <= config::BATTERY_FLOOR_PCT { - battery_capped = true; - } else if let Some(started) = hold_since - && now.saturating_sub(started) > config::BATTERY_MAX_HOLD - { - battery_capped = true; - } - - let want_hold = active && (power.on_ac || !battery_capped); + // the daemon idle-exits. Applied as a veto over the activity result, never + // folded into it, so a live turn still releases at the floor (ADR-0013). + battery_capped = battery_cap_latch(&power, now, hold_since, battery_capped); + let want_hold = hold_wanted(active, &power, battery_capped); if want_hold { - if hold_since.is_none() { - hold_since = Some(now); - } caffeinate.ensure_running()?; } else { - hold_since = None; caffeinate.stop(); } + // hold_since marks the start of the continuous hold ON BATTERY, so the + // max-hold guard counts battery time only. A long hold on AC does not + // consume the battery budget before an unplug (ADR-0013). + hold_since = next_hold_since(want_hold, power.on_ac, hold_since, now); // Self-exit advances only on housekeeping ticks, not on reactive death // wakeups, so the grace window stays ~EXIT_GRACE * interval. @@ -141,8 +186,8 @@ pub fn run() -> Result { } /// Scan every session log once: register any newly-seen live PID for reactive -/// exit notification, drop a session whose process is already gone, GC logs past -/// the staleness backstop, and return whether any session is active. +/// exit notification, drop a session whose process is already gone or interrupted, +/// GC logs past the safety cap, and return whether any session is active. fn evaluate_sessions(now: u64, watch: &mut SessionWatch) -> bool { let mut active = false; let mut live_transcripts = HashSet::new(); @@ -152,7 +197,7 @@ fn evaluate_sessions(now: u64, watch: &mut SessionWatch) -> bool { }; // Liveness: register a new live PID; a PID already dead or reused at first - // sight releases here instead of waiting for the staleness backstop. + // sight releases here instead of waiting for the safety cap. if let Some(pid) = last.pid && !watch.is_pid_watched(pid) { @@ -169,18 +214,25 @@ fn evaluate_sessions(now: u64, watch: &mut SessionWatch) -> bool { } // Watch the transcript so an Esc interrupt marker is seen reactively on its - // next write; the release itself happens in release_if_interrupted. + // next write, and check it now: a marker already written before this daemon + // started has no future write to react to (ADR-0013 scan-time check). if let Some(transcript) = &last.transcript { let transcript = PathBuf::from(transcript); + if event::is_interrupt_transcript(&transcript) { + let _ = fs::remove_file(&path); + continue; + } watch.watch_transcript(&transcript); live_transcripts.insert(transcript); } - let idle = now.saturating_sub(last.ts); - if idle < timeout_for(&last) { - active = true; - } else if idle > config::GC_THRESHOLD { + // Turn-span: an existing, live, un-interrupted session is active for the + // whole turn. A turn that never ends is released and GC'd at the cap. + let age = now.saturating_sub(last.ts); + if age > config::SAFETY_CAP { let _ = fs::remove_file(&path); + } else if is_active(&last, now) { + active = true; } } // Close transcript watches whose session log is gone, bounding open fds. @@ -188,6 +240,18 @@ fn evaluate_sessions(now: u64, watch: &mut SessionWatch) -> bool { active } +/// Whether a session, already checked alive and not interrupted, counts as active +/// under turn-span. An awaiting-input session is active only through its grace; any +/// other in-flight turn is active up to the safety cap (ADR-0013). +fn is_active(last: &Event, now: u64) -> bool { + let age = now.saturating_sub(last.ts); + if event::is_awaiting_input(last) { + age < config::AWAITING_INPUT_GRACE + } else { + age < config::SAFETY_CAP + } +} + /// Delete the log of the session whose recorded PID matches an exited process. fn remove_logs_for_pid(pid: u32) { for path in session_logs() { @@ -214,14 +278,6 @@ fn release_if_interrupted(transcript: &Path) { } } -fn timeout_for(last: &Event) -> u64 { - if event::is_unmatched_commit(last) { - config::COMMIT_TIMEOUT - } else { - config::STANDARD_TIMEOUT - } -} - fn session_logs() -> Vec { let mut logs = Vec::new(); if let Ok(entries) = fs::read_dir(config::vigil_dir()) { @@ -294,6 +350,42 @@ fn parse_charge(line: &str) -> Option { }) } +/// The battery-cap latch after one poll. On AC the latch clears. On battery it +/// latches at the floor or once the continuous hold exceeds the max, and otherwise +/// keeps its prior value (latched until AC returns or the daemon idle-exits). +fn battery_cap_latch(power: &PowerState, now: u64, hold_since: Option, capped: bool) -> bool { + if power.on_ac { + false + } else if power.charge <= config::BATTERY_FLOOR_PCT + || hold_since.is_some_and(|s| now.saturating_sub(s) > config::BATTERY_MAX_HOLD) + { + // Floor guard (death prevention) OR the max continuous-hold guard. + true + } else { + capped + } +} + +/// The hold decision: hold only while a session is active and the battery cap is +/// not vetoing. The battery term is applied here, over the activity result, so an +/// active turn on battery still releases once capped (ADR-0013 invariant). +fn hold_wanted(active: bool, power: &PowerState, battery_capped: bool) -> bool { + active && (power.on_ac || !battery_capped) +} + +/// The hold-start timestamp after one tick. `hold_since` marks the start of the +/// continuous hold ON BATTERY, so the max-hold guard measures battery time only. It +/// is cleared whenever the hold is not wanted or the daemon is on AC, and set on the +/// first battery tick of a hold. This keeps a long hold on AC from consuming the +/// battery budget before an unplug (ADR-0013). +fn next_hold_since(want_hold: bool, on_ac: bool, hold_since: Option, now: u64) -> Option { + if want_hold && !on_ac { + hold_since.or(Some(now)) + } else { + None + } +} + /// Print current sessions, assertion state, and power state. Read-only. pub fn status() -> Result<(), Error> { let now = event::now_secs(); @@ -309,7 +401,8 @@ pub fn status() -> Result<(), Error> { continue; }; let idle = now.saturating_sub(last.ts); - let committing = event::is_unmatched_commit(&last); + let awaiting = event::is_awaiting_input(&last); + let tool_in_flight = last.event == "PreToolUse"; let alive = last.pid.map(|pid| { proc::is_alive(&ProcId { pid, @@ -321,8 +414,9 @@ pub fn status() -> Result<(), Error> { .as_deref() .is_some_and(|t| event::is_interrupt_transcript(Path::new(t))); - // A session is active when its process is live, not interrupted, and fresh. - let active = alive != Some(false) && !interrupted && idle < timeout_for(&last); + // A session is active when its process is live, not interrupted, and the + // turn is still in flight under turn-span (ADR-0013). + let active = alive != Some(false) && !interrupted && is_active(&last, now); any_active |= active; let sid = path.file_stem().and_then(|s| s.to_str()).unwrap_or("?"); @@ -333,10 +427,15 @@ pub fn status() -> Result<(), Error> { (None, _) => "pid=?".to_string(), }; println!( - " {sid} last={} idle={idle}s {pid} {}{}{}", + " {sid} last={} idle={idle}s {pid} {}{}{}{}", last.event, if active { "active" } else { "idle" }, - if committing { " commit-in-flight" } else { "" }, + if tool_in_flight { + " tool-in-flight" + } else { + "" + }, + if awaiting { " awaiting-input" } else { "" }, if interrupted { " interrupted" } else { "" }, ); } @@ -344,6 +443,12 @@ pub fn status() -> Result<(), Error> { println!("active: {any_active}"); println!("assertion held: {}", pgrep("caffeinate -di")); println!("daemon running: {}", pgrep("vigil daemon")); + if config::disable_flag_path().exists() { + println!( + "disabled: yes (remove {} to re-enable)", + config::disable_flag_path().display() + ); + } match read_power() { Some(power) => { @@ -381,28 +486,95 @@ fn pgrep(pattern: &str) -> bool { mod tests { use super::*; - fn event(event: &str, command: Option<&str>) -> Event { - Event { + fn battery(charge: u8) -> PowerState { + PowerState { + on_ac: false, + charge, + state: "discharging".to_string(), + remaining: None, + } + } + + fn on_ac() -> PowerState { + PowerState { + on_ac: true, + charge: 100, + state: "charged".to_string(), + remaining: None, + } + } + + #[test] + fn turn_span_stays_active_far_past_the_old_timeout() { + let working = Event { ts: 0, - event: event.to_string(), - tool: command.map(|_| "Bash".to_string()), - command: command.map(str::to_string), + event: "PostToolUse".to_string(), ..Default::default() - } + }; + // A gap that the old 120s timeout would have released is still active. + assert!(is_active(&working, 1_000)); + assert!(is_active(&working, config::SAFETY_CAP - 1)); + // Past the safety cap it is not active (and evaluate_sessions GCs it). + assert!(!is_active(&working, config::SAFETY_CAP + 1)); + } + + #[test] + fn awaiting_input_is_active_only_through_its_grace() { + let awaiting = Event { + ts: 0, + event: "Notification".to_string(), + notification_type: Some("permission_prompt".to_string()), + ..Default::default() + }; + assert!(is_active(&awaiting, config::AWAITING_INPUT_GRACE - 1)); + assert!(!is_active(&awaiting, config::AWAITING_INPUT_GRACE + 1)); + } + + #[test] + fn battery_floor_vetoes_hold_while_active() { + // The invariant: an active turn on battery at the floor is not held. + let power = battery(config::BATTERY_FLOOR_PCT); + let capped = battery_cap_latch(&power, 100, Some(0), false); + assert!(capped); + assert!(!hold_wanted(true, &power, capped)); + } + + #[test] + fn ac_clears_the_cap_and_allows_an_active_hold() { + let power = on_ac(); + let capped = battery_cap_latch(&power, 100, Some(0), true); + assert!(!capped); + assert!(hold_wanted(true, &power, capped)); } #[test] - fn commit_in_flight_extends_timeout() { - let commit = event("PreToolUse", Some("git commit -m x")); - assert_eq!(timeout_for(&commit), config::COMMIT_TIMEOUT); + fn max_hold_latches_above_the_floor() { + let power = battery(90); + assert!(battery_cap_latch( + &power, + config::BATTERY_MAX_HOLD + 1, + Some(0), + false + )); + assert!(!battery_cap_latch( + &power, + config::BATTERY_MAX_HOLD - 1, + Some(0), + false + )); } #[test] - fn ordinary_event_uses_standard_timeout() { - let normal = event("PreToolUse", Some("git status")); - assert_eq!(timeout_for(&normal), config::STANDARD_TIMEOUT); - let prompt = event("UserPromptSubmit", None); - assert_eq!(timeout_for(&prompt), config::STANDARD_TIMEOUT); + fn hold_since_counts_battery_time_not_ac_time() { + // Holding on AC keeps no battery clock, so a long AC hold never accrues. + assert_eq!(next_hold_since(true, true, None, 100), None); + assert_eq!(next_hold_since(true, true, Some(50), 100), None); + // The clock starts on the first battery tick and then holds its start, so + // the max-hold budget begins at the unplug, not at the original AC hold. + assert_eq!(next_hold_since(true, false, None, 100), Some(100)); + assert_eq!(next_hold_since(true, false, Some(50), 100), Some(50)); + // Releasing clears it. + assert_eq!(next_hold_since(false, false, Some(50), 100), None); } #[test] diff --git a/src/event.rs b/src/event.rs index b6d6434..e56d613 100644 --- a/src/event.rs +++ b/src/event.rs @@ -1,6 +1,6 @@ //! The per-session event log: the JSONL line schema, its file I/O, the recorder -//! entry point, and commit detection. The recorder writes raw events only; the -//! daemon derives all policy from them (ADR-0004). +//! entry point, and the awaiting-input and interrupt-marker checks. The recorder +//! writes raw events only; the daemon derives all policy from them (ADR-0004). use std::fs::{self, File, OpenOptions}; use std::io::{Read, Seek, SeekFrom, Write}; @@ -18,8 +18,9 @@ use crate::proc; /// one small event each, so the last complete line lives well inside the window. const TAIL_CHUNK: u64 = 8192; -/// The six lifecycle events wired as hooks. Terminal events end the turn or -/// session and delete the log instead of appending. +/// The lifecycle events wired as hooks. Terminal events end the turn or session +/// and delete the log instead of appending. `Notification` carries the +/// awaiting-input signal (ADR-0013). #[derive(Clone, Copy, Debug, ValueEnum)] pub enum EventKind { #[value(name = "UserPromptSubmit")] @@ -28,6 +29,8 @@ pub enum EventKind { PreToolUse, #[value(name = "PostToolUse")] PostToolUse, + #[value(name = "Notification")] + Notification, #[value(name = "Stop")] Stop, #[value(name = "StopFailure")] @@ -37,11 +40,12 @@ pub enum EventKind { } impl EventKind { - /// The six events an install wires as hooks, and the order they are listed. - pub const ALL: [EventKind; 6] = [ + /// The events an install wires as hooks, and the order they are listed. + pub const ALL: [EventKind; 7] = [ Self::UserPromptSubmit, Self::PreToolUse, Self::PostToolUse, + Self::Notification, Self::Stop, Self::StopFailure, Self::SessionEnd, @@ -52,6 +56,7 @@ impl EventKind { Self::UserPromptSubmit => "UserPromptSubmit", Self::PreToolUse => "PreToolUse", Self::PostToolUse => "PostToolUse", + Self::Notification => "Notification", Self::Stop => "Stop", Self::StopFailure => "StopFailure", Self::SessionEnd => "SessionEnd", @@ -84,6 +89,8 @@ pub struct Event { pub pid_start: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub transcript: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub notification_type: Option, } /// The raw hook JSON on stdin. Only the fields the recorder extracts are named; @@ -99,6 +106,8 @@ struct HookPayload { agent_id: Option, #[serde(default)] transcript_path: Option, + #[serde(default)] + notification_type: Option, } #[derive(Deserialize)] @@ -134,6 +143,7 @@ pub fn record(kind: EventKind, hook_json: &str) -> Result<(), Error> { pid: identity.as_ref().map(|id| id.pid), pid_start: identity.map(|id| id.start), transcript: payload.transcript_path, + notification_type: payload.notification_type, }; append(&payload.session_id, &event) } @@ -208,45 +218,30 @@ fn last_complete_line(buf: &[u8]) -> Option<&str> { (!line.is_empty()).then_some(line) } -/// True when the session's newest line is an in-flight commit: a `PreToolUse` -/// for a `Bash` `git commit` with no `PostToolUse` after it yet. -pub fn is_unmatched_commit(last: &Event) -> bool { - last.event == "PreToolUse" - && last.tool.as_deref() == Some("Bash") - && last.command.as_deref().is_some_and(is_commit_command) -} - -/// True when `command` is a `git commit` invocation. Splits on shell separators -/// so `git add -A && git commit` matches, and skips leading `git` option tokens -/// so `git -C commit` matches. A commit run through a shell alias is not -/// detected. Mirrors the behavioral spec in SPEC.md "Commit-aware timeout". -pub fn is_commit_command(command: &str) -> bool { - command.split([';', '|', '&']).any(is_git_commit_segment) -} - -fn is_git_commit_segment(segment: &str) -> bool { - let mut tokens = segment.split_whitespace(); - if tokens.next() != Some("git") { - return false; - } - - while let Some(token) = tokens.next() { - if token == "commit" { - return true; - } - // `-C ` and `-c ` take a following value; skip it. - if token == "-C" || token == "-c" { - tokens.next(); - continue; - } - // Any other pre-subcommand option flag. - if token.starts_with('-') { - continue; - } - // A different subcommand (`log`, `status`, ...). - return false; - } - false +/// The `notification_type` values that mean a session is no longer actively +/// working, so the hold should release after its grace (ADR-0013): the user must +/// act (`permission_prompt`, `agent_needs_input`, `elicitation_dialog`), the user +/// has gone idle at the prompt (`idle_prompt`), or the turn finished +/// (`agent_completed`). `auth_success` and `elicitation_complete`/`_response` mean +/// work is resuming, so they are not listed. `idle_prompt` was added after it held +/// the display awake overnight on an abandoned session (2026-07-13). +const AWAITING_INPUT_TYPES: [&str; 5] = [ + "permission_prompt", + "agent_needs_input", + "elicitation_dialog", + "idle_prompt", + "agent_completed", +]; + +/// True when the session's newest line is a `Notification` that means the session +/// is not actively working (a prompt awaiting the user, the user idle at the +/// prompt, or the turn finished), so the hold should release after its grace. +pub fn is_awaiting_input(last: &Event) -> bool { + last.event == "Notification" + && last + .notification_type + .as_deref() + .is_some_and(|t| AWAITING_INPUT_TYPES.contains(&t)) } #[cfg(test)] @@ -254,57 +249,48 @@ mod tests { use super::*; #[test] - fn commit_commands_match() { - assert!(is_commit_command("git commit")); - assert!(is_commit_command("git commit -m \"x\"")); - assert!(is_commit_command("git -C /path commit")); - assert!(is_commit_command("git add -A && git commit")); - assert!(is_commit_command("git commit --amend")); - assert!(is_commit_command("git -c user.name=x commit -m y")); - } - - #[test] - fn non_commit_commands_reject() { - assert!(!is_commit_command("git log")); - assert!(!is_commit_command("git status")); - assert!(!is_commit_command("echo git commit")); - assert!(!is_commit_command("git commit-tree")); - } - - #[test] - fn unmatched_commit_needs_pretooluse_bash() { - let commit = |event: &str| Event { + fn awaiting_input_needs_notification_with_a_listed_type() { + let notif = |ty: Option<&str>| Event { ts: 1, - event: event.to_string(), - tool: Some("Bash".to_string()), - command: Some("git commit -m x".to_string()), + event: "Notification".to_string(), + notification_type: ty.map(str::to_string), ..Default::default() }; - assert!(is_unmatched_commit(&commit("PreToolUse"))); - // A PostToolUse for the same command means the commit returned. - assert!(!is_unmatched_commit(&commit("PostToolUse"))); - - let non_bash = Event { - tool: Some("Edit".to_string()), - ..commit("PreToolUse") + assert!(is_awaiting_input(¬if(Some("permission_prompt")))); + assert!(is_awaiting_input(¬if(Some("agent_needs_input")))); + assert!(is_awaiting_input(¬if(Some("elicitation_dialog")))); + // Idle at the prompt and turn-finished also release; idle_prompt held the + // display awake overnight before it was added to the set. + assert!(is_awaiting_input(¬if(Some("idle_prompt")))); + assert!(is_awaiting_input(¬if(Some("agent_completed")))); + // A notification that means work is resuming, or none at all, does not. + assert!(!is_awaiting_input(¬if(Some("auth_success")))); + assert!(!is_awaiting_input(¬if(Some("elicitation_complete")))); + assert!(!is_awaiting_input(¬if(None))); + // A listed type on a non-Notification event does not count. + let tool = Event { + event: "PreToolUse".to_string(), + notification_type: Some("permission_prompt".to_string()), + ..Default::default() }; - assert!(!is_unmatched_commit(&non_bash)); + assert!(!is_awaiting_input(&tool)); } #[test] fn event_round_trips() { let event = Event { ts: 1751490000, - event: "PreToolUse".to_string(), - tool: Some("Bash".to_string()), - command: Some("git commit -m \"x\"".to_string()), + event: "Notification".to_string(), + notification_type: Some("permission_prompt".to_string()), ..Default::default() }; let line = serde_json::to_string(&event).unwrap(); let back: Event = serde_json::from_str(&line).unwrap(); assert_eq!(event, back); - // agent_id serializes even when null; tool/command omitted when absent. + // agent_id serializes even when null; absent optionals are omitted. assert!(line.contains("\"agent_id\":null")); + assert!(line.contains("\"notification_type\":\"permission_prompt\"")); + assert!(!line.contains("\"tool\":")); } #[test] diff --git a/src/install.rs b/src/install.rs index e35a8ba..fc6e169 100644 --- a/src/install.rs +++ b/src/install.rs @@ -1,4 +1,4 @@ -//! Install and uninstall: place the binary, wire the six Claude Code hooks into +//! Install and uninstall: place the binary, wire the Claude Code lifecycle hooks into //! `settings.json`, and reverse both. All settings edits are surgical, only //! vigil's own hook entries are added or removed, so hooks and settings the user //! added stay intact. Install is idempotent: a fully consistent install is a @@ -23,13 +23,20 @@ use crate::event::{self, EventKind}; /// install or repair interactively. pub fn bootstrap() -> Result<(), Error> { let settings = read_settings()?; - let desired_bin = resolve_target(None, &settings); - let plan = build_plan(&settings, &desired_bin, false); + let (desired_bin, mode) = resolve_target(None, &settings); + let plan = build_plan(&settings, &desired_bin, mode, false); if plan.is_noop() { + let n = EventKind::ALL.len(); println!("vigil is installed at {}", desired_bin.display()); - println!(" hooks: 6 of 6 wired"); - println!(" symlink: {}", config::symlink_path().display()); + println!(" hooks: {n} of {n} wired"); + match mode { + InstallMode::OwnCopy => { + println!(" symlink: {}", config::symlink_path().display()) + } + InstallMode::Cargo => println!(" managed: cargo"), + InstallMode::Homebrew => println!(" managed: homebrew"), + } println!( " daemon: {}", if daemon_running() { @@ -63,13 +70,20 @@ pub fn bootstrap() -> Result<(), Error> { /// consistent unless `force` overwrites the binary. pub fn install(dir: Option, force: bool, assume_yes: bool) -> Result<(), Error> { let settings = read_settings()?; - let desired_bin = resolve_target(dir.as_deref(), &settings); - let plan = build_plan(&settings, &desired_bin, force); + let (desired_bin, mode) = resolve_target(dir.as_deref(), &settings); + let plan = build_plan(&settings, &desired_bin, mode, force); if plan.is_noop() { + let n = EventKind::ALL.len(); println!("vigil is already installed at {}", desired_bin.display()); - println!(" 6 hooks wired, binary present, symlink ok"); - println!(" (re-run with --force to overwrite the binary after a rebuild)"); + match mode { + InstallMode::OwnCopy => { + println!(" {n} hooks wired, binary present, symlink ok"); + println!(" (re-run with --force to overwrite the binary after a rebuild)"); + } + InstallMode::Cargo => println!(" {n} hooks wired, cargo-managed binary"), + InstallMode::Homebrew => println!(" {n} hooks wired, homebrew-managed binary"), + } return Ok(()); } @@ -87,6 +101,7 @@ pub fn install(dir: Option, force: bool, assume_yes: bool) -> Result<() /// binary, symlink, and runtime state, and stop the daemon. pub fn uninstall(assume_yes: bool) -> Result<(), Error> { let settings = read_settings()?; + let mode = detect_mode(); let bin = consistent_hook_bin(&settings).unwrap_or_else(config::install_bin_path); let stripped = { @@ -95,9 +110,12 @@ pub fn uninstall(assume_yes: bool) -> Result<(), Error> { s }; let remove_hooks = stripped != settings; - let bin_present = binary_present(&bin); + // Managed installs (cargo/brew) own the binary and its PATH entry; vigil leaves + // both for `cargo uninstall` / `brew uninstall` and removes only its own state. + let manage_binary = !mode.is_managed(); + let bin_present = manage_binary && binary_present(&bin); let link = config::symlink_path(); - let link_is_ours = symlink_ok(&bin); + let link_is_ours = manage_binary && symlink_ok(&bin); let runtime = config::vigil_dir(); let runtime_present = runtime.exists(); @@ -119,6 +137,14 @@ pub fn uninstall(assume_yes: bool) -> Result<(), Error> { if link_is_ours { println!(" remove symlink {}", link.display()); } + if mode.is_managed() { + let pm = if mode == InstallMode::Cargo { + "cargo uninstall vigil" + } else { + "brew uninstall vigil" + }; + println!(" keep binary {} (remove with `{pm}`)", bin.display()); + } println!(" stop daemon pkill -f 'vigil daemon'"); if runtime_present { println!(" clean runtime {}", runtime.display()); @@ -151,8 +177,45 @@ pub fn uninstall(assume_yes: bool) -> Result<(), Error> { Ok(()) } +/// How vigil was obtained, which decides binary placement and hook target +/// (ADR-0014). Managed modes leave the binary to the package manager. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +enum InstallMode { + /// Copy self to vigil's own dir and manage a PATH symlink. + OwnCopy, + /// Running from cargo's bin; hooks point there, cargo owns the binary. + Cargo, + /// Running from a Homebrew Cellar; hooks point at the brew bin symlink. + Homebrew, +} + +impl InstallMode { + /// A package-managed binary vigil neither copies nor removes. + fn is_managed(self) -> bool { + matches!(self, Self::Cargo | Self::Homebrew) + } +} + +/// Classify how the running binary was installed. An explicit `$VIGIL_INSTALL_DIR` +/// forces own-copy; otherwise the running binary's location selects the mode. +fn detect_mode() -> InstallMode { + if env::var_os("VIGIL_INSTALL_DIR").is_some() { + return InstallMode::OwnCopy; + } + if let Ok(exe) = env::current_exe() { + if config::homebrew_front_door(&exe).is_some() { + return InstallMode::Homebrew; + } + if canon_eq(&exe, &config::cargo_bin_path()) { + return InstallMode::Cargo; + } + } + InstallMode::OwnCopy +} + /// The set of changes an install needs. Empty (`is_noop`) means fully consistent. struct Plan { + mode: InstallMode, desired_bin: PathBuf, desired_settings: Value, write_settings: bool, @@ -168,14 +231,17 @@ impl Plan { } } -fn build_plan(settings: &Value, desired_bin: &Path, force: bool) -> Plan { +fn build_plan(settings: &Value, desired_bin: &Path, mode: InstallMode, force: bool) -> Plan { let desired_settings = desired_settings(settings, desired_bin); let write_settings = &desired_settings != settings; - let copy_binary = force || !binary_present(desired_bin); - let fix_symlink = !symlink_ok(desired_bin); + // Managed installs (cargo/brew) own the binary and are already on PATH, so + // vigil neither copies the binary nor manages a PATH symlink for them. + let copy_binary = !mode.is_managed() && (force || !binary_present(desired_bin)); + let fix_symlink = !mode.is_managed() && !symlink_ok(desired_bin); let stale = consistent_hook_bin(settings).is_some_and(|p| p != desired_bin); Plan { + mode, desired_bin: desired_bin.to_path_buf(), desired_settings, write_settings, @@ -185,16 +251,29 @@ fn build_plan(settings: &Value, desired_bin: &Path, force: bool) -> Plan { } } -/// Where the binary should live: an explicit `--dir`, then `$VIGIL_INSTALL_DIR`, -/// then wherever the existing hooks consistently point, then the default. -fn resolve_target(cli_dir: Option<&Path>, settings: &Value) -> PathBuf { +/// The hook target and install mode: an explicit `--dir` or `$VIGIL_INSTALL_DIR` +/// forces own-copy at that location; a cargo or Homebrew binary points the hooks at +/// its own stable path; otherwise own-copy to wherever the existing hooks point, or +/// the default (ADR-0014). +fn resolve_target(cli_dir: Option<&Path>, settings: &Value) -> (PathBuf, InstallMode) { if let Some(dir) = cli_dir { - return dir.join("bin").join(config::BIN_NAME); - } - if env::var_os("VIGIL_INSTALL_DIR").is_some() { - return config::install_bin_path(); - } - consistent_hook_bin(settings).unwrap_or_else(config::install_bin_path) + return (dir.join("bin").join(config::BIN_NAME), InstallMode::OwnCopy); + } + let mode = detect_mode(); + let target = match mode { + InstallMode::Homebrew => env::current_exe() + .ok() + .and_then(|e| config::homebrew_front_door(&e)) + .unwrap_or_else(config::install_bin_path), + InstallMode::Cargo => config::cargo_bin_path(), + InstallMode::OwnCopy if env::var_os("VIGIL_INSTALL_DIR").is_some() => { + config::install_bin_path() + } + InstallMode::OwnCopy => { + consistent_hook_bin(settings).unwrap_or_else(config::install_bin_path) + } + }; + (target, mode) } fn apply(plan: &Plan) -> Result<(), Error> { @@ -228,7 +307,7 @@ fn command_is_vigil(cmd: &str) -> bool { is_vigil && tokens.next() == Some("record") } -/// The binary paths every vigil hook entry references, across the six events. +/// The binary paths every vigil hook entry references, across all wired events. fn hook_bins(settings: &Value) -> Vec { let mut out = Vec::new(); let Some(hooks) = settings.get("hooks").and_then(Value::as_object) else { @@ -263,7 +342,7 @@ fn consistent_hook_bin(settings: &Value) -> Option { bins.iter().all(|b| *b == first).then_some(first) } -/// Remove every vigil hook entry from the six event arrays, pruning any group, +/// Remove every vigil hook entry from the wired event arrays, pruning any group, /// event array, or the `hooks` object left empty. fn strip_vigil(settings: &mut Value) { let Some(obj) = settings.as_object_mut() else { @@ -313,7 +392,7 @@ fn strip_vigil(settings: &mut Value) { } } -/// Append a fresh vigil hook group to each of the six events, creating the +/// Append a fresh vigil hook group to each wired event, creating the /// `hooks` object and event arrays as needed. fn insert_vigil(settings: &mut Value, bin: &Path) { let obj = settings.as_object_mut().expect("settings is a JSON object"); @@ -430,18 +509,24 @@ fn prune_backups(dir: &Path) { fn copy_self(dest: &Path) -> Result<(), Error> { let src = env::current_exe()?; - // Running the installed binary already: nothing to copy, and copying onto a - // live executable would fail with ETXTBSY. + // Running the installed binary already: nothing to copy. if canon_eq(&src, dest) { return Ok(()); } - if let Some(parent) = dest.parent() { - fs::create_dir_all(parent)?; - } - fs::copy(&src, dest)?; - let mut perms = fs::metadata(dest)?.permissions(); + let parent = dest.parent().unwrap_or_else(|| Path::new(".")); + fs::create_dir_all(parent)?; + + // Copy to a temp file beside the destination, then rename over it. rename + // repoints the directory entry to a new inode instead of rewriting the bytes + // of the inode a running daemon holds. An in-place overwrite invalidates the + // Mach-O signature and macOS SIGKILLs the next exec of the path. The temp + // shares the destination directory so the rename stays on one filesystem. + let tmp = dest.with_file_name(format!("{}.vigil-tmp", config::BIN_NAME)); + fs::copy(&src, &tmp)?; + let mut perms = fs::metadata(&tmp)?.permissions(); perms.set_mode(0o755); - fs::set_permissions(dest, perms)?; + fs::set_permissions(&tmp, perms)?; + fs::rename(&tmp, dest)?; Ok(()) } @@ -527,24 +612,37 @@ fn print_install_plan(settings: &Value, plan: &Plan) { }; println!("{header}\n"); - if plan.copy_binary { - match env::current_exe() { - Ok(src) => println!(" copy binary {}", src.display()), - Err(_) => println!(" copy binary "), - } - println!(" -> {}", plan.desired_bin.display()); - } else { - println!(" binary present at {}", plan.desired_bin.display()); - } - - if plan.fix_symlink { + if plan.mode.is_managed() { + let pm = if plan.mode == InstallMode::Cargo { + "cargo" + } else { + "homebrew" + }; println!( - " symlink {} -> {}", - config::symlink_path().display(), + " binary {}-managed at {}", + pm, plan.desired_bin.display() ); } else { - println!(" symlink ok"); + if plan.copy_binary { + match env::current_exe() { + Ok(src) => println!(" copy binary {}", src.display()), + Err(_) => println!(" copy binary "), + } + println!(" -> {}", plan.desired_bin.display()); + } else { + println!(" binary present at {}", plan.desired_bin.display()); + } + + if plan.fix_symlink { + println!( + " symlink {} -> {}", + config::symlink_path().display(), + plan.desired_bin.display() + ); + } else { + println!(" symlink ok"); + } } if plan.write_settings { @@ -564,7 +662,10 @@ fn print_install_plan(settings: &Value, plan: &Plan) { println!(" backup settings.json.bak- (all other hooks preserved)"); } } else { - println!(" hooks all 6 already wired"); + println!( + " hooks all {} already wired", + EventKind::ALL.len() + ); } println!(); } @@ -577,6 +678,39 @@ mod tests { PathBuf::from("/opt/vigil/bin/vigil") } + #[test] + fn managed_modes_leave_the_binary() { + assert!(InstallMode::Cargo.is_managed()); + assert!(InstallMode::Homebrew.is_managed()); + assert!(!InstallMode::OwnCopy.is_managed()); + } + + #[test] + fn dir_forces_own_copy_at_that_path() { + let (target, mode) = resolve_target(Some(Path::new("/opt/vigil")), &json!({})); + assert_eq!(target, PathBuf::from("/opt/vigil/bin/vigil")); + assert_eq!(mode, InstallMode::OwnCopy); + } + + #[test] + fn managed_plan_skips_copy_and_symlink() { + // A cargo install with hooks already wired at the cargo path is a noop: + // no binary copy, no symlink, only settings if they differ. + let cargo_bin = config::cargo_bin_path(); + let mut settings = json!({}); + insert_vigil(&mut settings, &cargo_bin); + let plan = build_plan(&settings, &cargo_bin, InstallMode::Cargo, true); + assert!( + !plan.copy_binary, + "managed install must not copy the binary" + ); + assert!( + !plan.fix_symlink, + "managed install must not manage a symlink" + ); + assert!(plan.is_noop(), "already-wired cargo install is a noop"); + } + #[test] fn command_matching() { assert!(command_is_vigil("/opt/vigil/bin/vigil record PreToolUse")); @@ -604,8 +738,8 @@ mod tests { let pre = settings["hooks"]["PreToolUse"].as_array().unwrap(); assert_eq!(pre.len(), 2); assert_eq!(pre[0]["hooks"][0]["command"], json!("/other/log.sh")); - // All six wired at the target path. - assert_eq!(hook_bins(&settings).len(), 6); + // All events wired at the target path. + assert_eq!(hook_bins(&settings).len(), EventKind::ALL.len()); assert_eq!( consistent_hook_bin(&settings).as_deref(), Some(bin().as_path())