From b6414721c93cb5911ac9b655fd531ead0ab0d3aa Mon Sep 17 00:00:00 2001 From: Taylor Silenzio Date: Mon, 13 Jul 2026 13:38:24 -0500 Subject: [PATCH 01/11] docs: add ADR-0013 turn-span activity model and phase-1 plan Records the decision to replace the timeout-driven release with a turn-span hold (UserPromptSubmit until Stop/interrupt/death/awaiting-input), motivated by the 2026-07-13 mid-turn display-sleep incident and the verification that no incremental signal survives a long thinking block. Supersedes ADR-0006 and dissolves the commit-aware timeout of ADR-0005. Adds TODO-002 with the phase-1 work breakdown and the battery-floor invariant. --- docs/TODO.md | 61 ++++++- docs/adr/0013-turn-span-activity-model.md | 191 ++++++++++++++++++++++ 2 files changed, 251 insertions(+), 1 deletion(-) create mode 100644 docs/adr/0013-turn-span-activity-model.md diff --git a/docs/TODO.md b/docs/TODO.md index 2e0f885..717209e 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -1,9 +1,68 @@ # vigil TODO -Last Updated: 2026-07-02 +Last Updated: 2026-07-13 Planned work and deferred decisions. +## TODO-002: Turn-span activity model (phase 1) + +**Status:** Planned. 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..5ec4604 --- /dev/null +++ b/docs/adr/0013-turn-span-activity-model.md @@ -0,0 +1,191 @@ +# 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`, or `elicitation_dialog` is treated as +awaiting the user and is not active. The next `PreToolUse` (after approval) or a +new `UserPromptSubmit` becomes the newest line and the session is active again. + +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. + +### 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) From ba1381e771e5c7b9a36bb14c2591fe5bed689591 Mon Sep 17 00:00:00 2001 From: Taylor Silenzio Date: Mon, 13 Jul 2026 13:46:44 -0500 Subject: [PATCH 02/11] feat: turn-span activity model (ADR-0013 phase 1) Replace the timeout-driven release with a turn-span hold: a session is active while its log exists, its process is alive, its transcript is not interrupted, its newest line is not an awaiting-input Notification, and its age is under a long safety cap. Fixes the mid-turn display sleep where a >120s tool-free gap elapsed the idle timeout and released the hold. - Wire Notification as a seventh hook event; record notification_type and treat permission_prompt/agent_needs_input/elicitation_dialog as awaiting-input, active only through a 90s grace. - Remove STANDARD_TIMEOUT, COMMIT_TIMEOUT, GC_THRESHOLD, and commit detection; the commit hold now falls out of turn-span. - Add a stateless 12h safety cap (log age) as the missed-Stop backstop, and a scan-time interrupt-marker check for a marker written before daemon start. - Extract the battery cap into pure battery_cap_latch/hold_wanted functions with the floor-vetoes-active-hold invariant locked by a unit test. - status reports working / tool-in-flight / awaiting-input. --- src/config.rs | 28 ++++---- src/daemon.rs | 188 ++++++++++++++++++++++++++++++++++++------------- src/event.rs | 135 +++++++++++++++-------------------- src/install.rs | 25 ++++--- 4 files changed, 222 insertions(+), 154 deletions(-) diff --git a/src/config.rs b/src/config.rs index 6c3ecdc..1f5a1f7 100644 --- a/src/config.rs +++ b/src/config.rs @@ -7,18 +7,19 @@ 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; +/// 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; diff --git a/src/daemon.rs b/src/daemon.rs index 77402bb..0b427e1 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -1,8 +1,9 @@ //! 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}; @@ -103,18 +104,10 @@ pub fn run() -> Result { // 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() { @@ -141,8 +134,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 +145,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 +162,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 +188,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 +226,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 +298,29 @@ 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) +} + /// Print current sessions, assertion state, and power state. Read-only. pub fn status() -> Result<(), Error> { let now = event::now_secs(); @@ -309,7 +336,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 +349,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 +362,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 { "" }, ); } @@ -381,28 +415,82 @@ 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 commit_in_flight_extends_timeout() { - let commit = event("PreToolUse", Some("git commit -m x")); - assert_eq!(timeout_for(&commit), config::COMMIT_TIMEOUT); + 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 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 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] diff --git a/src/event.rs b/src/event.rs index b6d6434..bfd26ce 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,24 @@ 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 waiting on the user, so +/// the turn is no longer advancing and the hold should release after its grace +/// (ADR-0013). `agent_completed` and `auth_success` are notifications that do not +/// signal awaiting-input and are not listed. +const AWAITING_INPUT_TYPES: [&str; 3] = [ + "permission_prompt", + "agent_needs_input", + "elicitation_dialog", +]; + +/// True when the session's newest line is an awaiting-input `Notification`: the +/// turn has surfaced a permission or elicitation prompt and is waiting on the user. +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 +243,43 @@ 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")))); + // A notification that does not await input, or none at all. + assert!(!is_awaiting_input(¬if(Some("agent_completed")))); + 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..bfebe34 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 @@ -27,8 +27,9 @@ pub fn bootstrap() -> Result<(), Error> { let plan = build_plan(&settings, &desired_bin, 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!(" hooks: {n} of {n} wired"); println!(" symlink: {}", config::symlink_path().display()); println!( " daemon: {}", @@ -68,7 +69,10 @@ pub fn install(dir: Option, force: bool, assume_yes: bool) -> Result<() if plan.is_noop() { println!("vigil is already installed at {}", desired_bin.display()); - println!(" 6 hooks wired, binary present, symlink ok"); + println!( + " {} hooks wired, binary present, symlink ok", + EventKind::ALL.len() + ); println!(" (re-run with --force to overwrite the binary after a rebuild)"); return Ok(()); } @@ -228,7 +232,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 +267,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 +317,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"); @@ -564,7 +568,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!(); } @@ -604,8 +611,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()) From 025842c9568a8fc2a713d32ab482edeecce99b33 Mon Sep 17 00:00:00 2001 From: Taylor Silenzio Date: Mon, 13 Jul 2026 16:57:48 -0500 Subject: [PATCH 03/11] docs: add ADR-0014 for safe binary replacement and daemon self-upgrade Records three mechanisms: atomic-rename install (removes the in-place-overwrite signature corruption behind the install --force SIGKILL), inode-change daemon self-upgrade, and a reactive .disabled flag. Notes Homebrew and cargo install behavior and coverage. --- docs/adr/0014-safe-self-upgrade.md | 122 +++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 docs/adr/0014-safe-self-upgrade.md diff --git a/docs/adr/0014-safe-self-upgrade.md b/docs/adr/0014-safe-self-upgrade.md new file mode 100644 index 0000000..b9dfa84 --- /dev/null +++ b/docs/adr/0014-safe-self-upgrade.md @@ -0,0 +1,122 @@ +# ADR-0014: 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 (`rm` then copy) to force a fresh inode. + +Two separate problems sit behind this: + +1. **Corruption.** Overwriting the running binary in place invalidates its + signature. This is a property of the write technique, independent of the daemon. +2. **Freshness.** Even after a safe replacement, the old daemon keeps executing its + old inode until it restarts, so new code is not live until then. + +A third concern is coordination during an upgrade: in the lazy-spawn model a hook +can spawn a daemon at any instant, so any "stop the daemon, then swap the binary" +sequence has a window where a fresh old-binary daemon can start mid-swap. + +External installers behave differently. Homebrew installs each version into a new +Cellar directory and repoints `/opt/homebrew/bin/vigil`, leaving the running file +untouched (no corruption, but the launch-path inode does not change). `cargo +install` stages the build and renames it into `~/.cargo/bin/vigil`, which is +corruption-safe on Unix and does change the inode at a fixed path. + +## Decision + +Three mechanisms, applied in order. + +### 1. Atomic rename on install + +`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 is running, so +it is the floor the other two mechanisms build on. + +### 2. Daemon self-upgrade via inode change + +At startup the daemon records the `(device, inode)` of its own executable +(`current_exe()`). On the slow-housekeeping cadence (`POWER_POLL_INTERVAL`) it +re-stats that path. When the pair differs, a new binary has been installed at the +path, so the daemon stops its caffeinate, releases its lock, spawns a fresh detached +daemon (which execs the new inode), and exits. The inode change is the version +signal; no version directories or version strings are involved. + +The handoff releases the lock before spawning, so the new daemon acquires it +immediately rather than racing the old daemon's exit. A hook-spawned daemon that +wins the lock first is an equally valid new-binary daemon, so the handoff is +self-healing. + +Coverage by installer: native `install --force` (mechanism 1) and `cargo install` +both land a new inode at a fixed path and are caught. Homebrew repoints a symlink +and keeps the old Cellar inode, so the launch-path inode does not change and +auto-upgrade does not fire; freshness there comes from the daemon's idle-restart or +an explicit re-run of `vigil install`. Watching the front-door symlink to catch the +Homebrew case is a possible later refinement. + +### 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, which mechanism 2 handles on its own. + +## Consequences + +**Corruption removed.** Mechanism 1 makes `install --force` safe whether or not the +daemon runs, and matches the atomic-write pattern already used for `settings.json`. + +**Upgrades go live on their own for the fixed-path installers.** Mechanism 2 makes a +native or cargo upgrade take effect within `POWER_POLL_INTERVAL` without a manual +restart, and keeps the lazy-spawn model (install does not orchestrate the daemon). + +**Clean shutdown path.** Mechanisms 2 and 3 both 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 is safe but not auto-fresh.** A `brew upgrade` cannot corrupt the running +binary, but the daemon does not auto-restart on it. Documented as a nudge +(`vigil install` or a natural idle-restart), with the symlink-watch refinement left +open. + +**Disable flag is sticky until removed.** While `.disabled` exists, every spawned +daemon exits at startup, so vigil holds nothing. 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: + +- "Single-instance (flock)" / "Self-exit & crash recovery": add the inode-change + self-upgrade handoff and the `.disabled` flag as daemon exit paths. +- "CLI Interface" / install: note that `install` replaces the binary via atomic + rename. +- "Timeouts & Configuration": the binary-change check runs on the + `POWER_POLL_INTERVAL` cadence. + +## 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; `cargo install` rename-into-place on Unix +- ADR-0007 (daemon lifecycle and lazy spawn), ADR-0012 (directory watch reused for + the disable flag), ADR-0002 (single daemon-owned caffeinate) From 47e865561df4f0fc7dff8954a1fc4fc23bcca15e Mon Sep 17 00:00:00 2001 From: Taylor Silenzio Date: Mon, 13 Jul 2026 17:47:01 -0500 Subject: [PATCH 04/11] fix: replace the installed binary via atomic rename copy_self wrote the new binary over the destination in place with fs::copy, which rewrites the bytes of the inode a running daemon holds and invalidates the Mach-O signature, so macOS SIGKILLs the next exec (observed exit 137 on an install --force over a live daemon). Copy to a temp file beside the destination and rename over it: rename repoints the directory entry to a new inode, leaving the running daemon's inode intact. Verified by reproducing install --force over a running daemon and exec'ing the result. --- src/install.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/install.rs b/src/install.rs index bfebe34..8923416 100644 --- a/src/install.rs +++ b/src/install.rs @@ -434,18 +434,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(()) } From bc650b62626d11dc7a9a89adc750fd7a450f80a7 Mon Sep 17 00:00:00 2001 From: Taylor Silenzio Date: Mon, 13 Jul 2026 17:49:56 -0500 Subject: [PATCH 05/11] docs: revise ADR-0014 for hybrid install modes Adds the install-mode model (own-copy default, native cargo and Homebrew when detected from current_exe) and reframes the self-upgrade watch to follow the front-door symlink so brew upgrades are caught too. Flags the Homebrew end-to-end as unverified until a tap exists. --- docs/adr/0014-safe-self-upgrade.md | 137 ++++++++++++++++------------- 1 file changed, 78 insertions(+), 59 deletions(-) diff --git a/docs/adr/0014-safe-self-upgrade.md b/docs/adr/0014-safe-self-upgrade.md index b9dfa84..4e76d5e 100644 --- a/docs/adr/0014-safe-self-upgrade.md +++ b/docs/adr/0014-safe-self-upgrade.md @@ -1,4 +1,4 @@ -# ADR-0014: Safe binary replacement and daemon self-upgrade +# ADR-0014: Install modes, safe binary replacement, and daemon self-upgrade **Status:** Draft @@ -12,30 +12,43 @@ 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 (`rm` then copy) to force a fresh inode. +file first to force a fresh inode. -Two separate problems sit behind this: +Three problems sit behind this and the broader upgrade story: 1. **Corruption.** Overwriting the running binary in place invalidates its - signature. This is a property of the write technique, independent of the daemon. -2. **Freshness.** Even after a safe replacement, the old daemon keeps executing its - old inode until it restarts, so new code is not live until then. + 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. -A third concern is coordination during an upgrade: in the lazy-spawn model a hook -can spawn a daemon at any instant, so any "stop the daemon, then swap the binary" -sequence has a window where a fresh old-binary daemon can start mid-swap. +## Decision -External installers behave differently. Homebrew installs each version into a new -Cellar directory and repoints `/opt/homebrew/bin/vigil`, leaving the running file -untouched (no corruption, but the launch-path inode does not change). `cargo -install` stages the build and renames it into `~/.cargo/bin/vigil`, which is -corruption-safe on Unix and does change the inode at a fixed path. +### Install modes -## Decision +`vigil install` classifies where it is running from and wires the hooks at the +appropriate stable path, rather than always copying to one location: -Three mechanisms, applied in order. +- **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. -### 1. Atomic rename on install +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 @@ -43,29 +56,26 @@ directory entry to a new inode; it never rewrites the bytes of the inode a runni 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 is running, so -it is the floor the other two mechanisms build on. +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 records the `(device, inode)` of its own executable -(`current_exe()`). On the slow-housekeeping cadence (`POWER_POLL_INTERVAL`) it -re-stats that path. When the pair differs, a new binary has been installed at the -path, so the daemon stops its caffeinate, releases its lock, spawns a fresh detached -daemon (which execs the new inode), and exits. The inode change is the version -signal; no version directories or version strings are involved. - -The handoff releases the lock before spawning, so the new daemon acquires it -immediately rather than racing the old daemon's exit. A hook-spawned daemon that -wins the lock first is an equally valid new-binary daemon, so the handoff is -self-healing. - -Coverage by installer: native `install --force` (mechanism 1) and `cargo install` -both land a new inode at a fixed path and are caught. Homebrew repoints a symlink -and keeps the old Cellar inode, so the launch-path inode does not change and -auto-upgrade does not fire; freshness there comes from the daemon's idle-restart or -an explicit re-run of `vigil install`. Watching the front-door symlink to catch the -Homebrew case is a possible later refinement. +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 @@ -75,48 +85,57 @@ 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, which mechanism 2 handles on its own. +switch, separate from the upgrade path. ## Consequences -**Corruption removed.** Mechanism 1 makes `install --force` safe whether or not the -daemon runs, and matches the atomic-write pattern already used for `settings.json`. +**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 for the fixed-path installers.** Mechanism 2 makes a -native or cargo upgrade take effect within `POWER_POLL_INTERVAL` without a manual -restart, and keeps the lazy-spawn model (install does not orchestrate the daemon). +**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 both 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`. +**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 is safe but not auto-fresh.** A `brew upgrade` cannot corrupt the running -binary, but the daemon does not auto-restart on it. Documented as a nudge -(`vigil install` or a natural idle-restart), with the symlink-watch refinement left -open. +**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, so vigil holds nothing. 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`. +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. -- "CLI Interface" / install: note that `install` replaces the binary via atomic - rename. -- "Timeouts & Configuration": the binary-change check runs on the - `POWER_POLL_INTERVAL` cadence. +- "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; `cargo install` rename-into-place on Unix +- 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) From 5a949efc30d53d5bacdc30592b3e7903a1a2a247 Mon Sep 17 00:00:00 2001 From: Taylor Silenzio Date: Mon, 13 Jul 2026 17:54:42 -0500 Subject: [PATCH 06/11] feat: detect cargo and Homebrew installs and wire hooks natively vigil now classifies how it was obtained from current_exe: a cargo binary (${CARGO_HOME:-~/.cargo}/bin/vigil) or a Homebrew Cellar path wires the hooks at that package-managed path with no copy and no vigil symlink, and uninstall leaves the binary for cargo/brew. Anything else (build artifact, --dir, VIGIL_INSTALL_DIR) keeps the own-copy behavior. The Homebrew front-door derivation is pure string surgery and unit-tested; its end-to-end is unverified until a tap exists. --- src/config.rs | 50 ++++++++++++- src/install.rs | 197 +++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 208 insertions(+), 39 deletions(-) diff --git a/src/config.rs b/src/config.rs index 1f5a1f7..d82d3c7 100644 --- a/src/config.rs +++ b/src/config.rs @@ -5,7 +5,7 @@ //! `$CLAUDE_CONFIG_DIR`, `$XDG_DATA_HOME`). use std::env; -use std::path::PathBuf; +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 @@ -98,6 +98,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") @@ -108,3 +128,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/install.rs b/src/install.rs index 8923416..fc6e169 100644 --- a/src/install.rs +++ b/src/install.rs @@ -23,14 +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: {n} of {n} wired"); - println!(" symlink: {}", config::symlink_path().display()); + 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() { @@ -64,16 +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!( - " {} hooks wired, binary present, symlink ok", - EventKind::ALL.len() - ); - 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(()); } @@ -91,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 = { @@ -99,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(); @@ -123,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()); @@ -155,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, @@ -172,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, @@ -189,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> { @@ -537,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 { @@ -590,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")); From 14397c491420a2db1dfdb36db13d58fad3309eed Mon Sep 17 00:00:00 2001 From: Taylor Silenzio Date: Tue, 14 Jul 2026 10:59:29 -0500 Subject: [PATCH 07/11] feat: daemon self-upgrade on binary inode change The daemon records the (device, inode) of its watch path at startup (the Homebrew front door when running from a Cellar, else current_exe) and re-checks it on the power-poll cadence. When it changes (install --force or cargo install renames a new inode, or a brew upgrade repoints the front-door symlink) it stops its caffeinate, releases the lock, and spawns a fresh daemon from the watch path, which follows a brew symlink flip to the new version. Verified end-to-end: a daemon handed off to a new binary within the poll interval after an atomic rename over its watched path. --- src/daemon.rs | 44 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/src/daemon.rs b/src/daemon.rs index 0b427e1..bec8d81 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -7,6 +7,7 @@ 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}; @@ -24,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") @@ -46,6 +50,23 @@ 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 { @@ -68,6 +89,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 @@ -98,6 +123,19 @@ pub fn run() -> Result { 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); From cd07669af0e55ce9192f359a1dabf4302cfb2672 Mon Sep 17 00:00:00 2001 From: Taylor Silenzio Date: Tue, 14 Jul 2026 11:03:57 -0500 Subject: [PATCH 08/11] feat: reactive .disabled flag stands the daemon down A sentinel file in the runtime dir disables the daemon: checked at startup so a hook-spawned daemon exits at once, and each loop pass so creating the file (which wakes the existing log-dir watch) releases within a wake. The daemon clean-exits, stopping its caffeinate, so unlike a pkill it leaves no orphan. status reports the disabled state. Verified end-to-end: creating the flag exits the daemon and stops its caffeinate with no orphan, and a fresh spawn is blocked while the flag exists. --- src/config.rs | 6 ++++++ src/daemon.rs | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/config.rs b/src/config.rs index d82d3c7..e4e4245 100644 --- a/src/config.rs +++ b/src/config.rs @@ -81,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") diff --git a/src/daemon.rs b/src/daemon.rs index bec8d81..ddae8f4 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -72,6 +72,13 @@ fn binary_ident(path: &Path) -> Option<(u64, u64)> { 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() { @@ -117,6 +124,13 @@ 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() { @@ -416,6 +430,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) => { From 8af1538f5549ac54295733815dfa5c6c324545f2 Mon Sep 17 00:00:00 2001 From: Taylor Silenzio Date: Tue, 14 Jul 2026 11:29:27 -0500 Subject: [PATCH 09/11] fix: release the hold on idle_prompt and agent_completed notifications The awaiting-input release set was missing idle_prompt (the user has gone idle at the prompt) and agent_completed (the turn finished). A session ending on an idle_prompt Notification stayed active under the 12h safety cap and held the display awake overnight (2026-07-13, 20:27 to 10:44 per pmset -g log) on an abandoned session. Both now release after the grace, so an idle session lets the display sleep ~90s after the prompt. auth_success and elicitation_complete/ _response still mean work is resuming and do not release. --- docs/adr/0013-turn-span-activity-model.md | 15 +++++++++--- src/event.rs | 29 ++++++++++++++++------- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/docs/adr/0013-turn-span-activity-model.md b/docs/adr/0013-turn-span-activity-model.md index 5ec4604..4686d6d 100644 --- a/docs/adr/0013-turn-span-activity-model.md +++ b/docs/adr/0013-turn-span-activity-model.md @@ -78,9 +78,18 @@ the duration of the turn, so the Touch ID sheet is covered without a special cas `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`, or `elicitation_dialog` is treated as -awaiting the user and is not active. The next `PreToolUse` (after approval) or a -new `UserPromptSubmit` becomes the newest line and the session is active again. +`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 diff --git a/src/event.rs b/src/event.rs index bfd26ce..e56d613 100644 --- a/src/event.rs +++ b/src/event.rs @@ -218,18 +218,24 @@ fn last_complete_line(buf: &[u8]) -> Option<&str> { (!line.is_empty()).then_some(line) } -/// The `notification_type` values that mean a session is waiting on the user, so -/// the turn is no longer advancing and the hold should release after its grace -/// (ADR-0013). `agent_completed` and `auth_success` are notifications that do not -/// signal awaiting-input and are not listed. -const AWAITING_INPUT_TYPES: [&str; 3] = [ +/// 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 an awaiting-input `Notification`: the -/// turn has surfaced a permission or elicitation prompt and is waiting on the user. +/// 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 @@ -253,8 +259,13 @@ mod tests { 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")))); - // A notification that does not await input, or none at all. - assert!(!is_awaiting_input(¬if(Some("agent_completed")))); + // 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 { From 4460ad8e6402276e1ddb7c4ab866cbd4a7a764bd Mon Sep 17 00:00:00 2001 From: Taylor Silenzio Date: Tue, 14 Jul 2026 13:22:48 -0500 Subject: [PATCH 10/11] fix: battery max-hold counts battery time, not total hold time The 3h BATTERY_MAX_HOLD guard measured now - hold_since, where hold_since marked the start of the continuous hold across AC and battery. Turn-span makes a hold span a whole turn, so a multi-hour AC hold consumed the battery budget before an unplug: a ~14h continuous hold (mostly on AC) tripped the guard the instant the machine was unplugged, releasing at 100% charge. Clear hold_since on AC and set it on the first battery tick (extracted into next_hold_since, unit-tested), so the guard counts battery time only and each unplug starts a fresh 3h budget, matching ADR-0009's intent. --- docs/adr/0013-turn-span-activity-model.md | 11 ++++++++ src/daemon.rs | 34 ++++++++++++++++++++--- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/docs/adr/0013-turn-span-activity-model.md b/docs/adr/0013-turn-span-activity-model.md index 4686d6d..858f0b8 100644 --- a/docs/adr/0013-turn-span-activity-model.md +++ b/docs/adr/0013-turn-span-activity-model.md @@ -121,6 +121,17 @@ including the case where a session is active, on battery, and at or below the fl 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 diff --git a/src/daemon.rs b/src/daemon.rs index ddae8f4..4dfd7f4 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -162,14 +162,14 @@ pub fn run() -> Result { 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. @@ -373,6 +373,19 @@ 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(); @@ -551,6 +564,19 @@ mod tests { )); } + #[test] + 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] fn parse_charge_from_pmset_line() { let line = " -InternalBattery-0 (id=1234) 94%; discharging; 2:33 remaining present: true"; From c75c62a6a5e257f99c73e26bf964a666fd121605 Mon Sep 17 00:00:00 2001 From: Taylor Silenzio Date: Tue, 14 Jul 2026 13:31:19 -0500 Subject: [PATCH 11/11] docs: add TODO-003 (daemon introspection) and mark turn-span phase 1 built --- docs/TODO.md | 62 +++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 57 insertions(+), 5 deletions(-) diff --git a/docs/TODO.md b/docs/TODO.md index 717209e..7e66123 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -1,15 +1,67 @@ # vigil TODO -Last Updated: 2026-07-13 +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:** Planned. 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. +**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