From 064762faa3b4f9d663aa6410fbfc5405be7879cd Mon Sep 17 00:00:00 2001 From: Daniel Vianna <1708810+pasunboneleve@users.noreply.github.com> Date: Wed, 26 Aug 2026 21:54:54 +1000 Subject: [PATCH 1/3] Make watcher shutdown idempotent Context: Ctrl-C could return an error before managed-process cleanup when native watch backends rejected redundant teardown of overlapping recursive and literal targets. Decision: Quiesce watcher callbacks and let the watcher backend drop as one unit. Keep process cleanup running if a watcher adapter nevertheless reports a shutdown error. Cover the failure with a mock that emits the observed missing-watch error and with explicit shutdown documentation. Alternatives considered: Rejecting or normalizing overlapping client patterns would move backend correctness into configuration. Reversing individual unwatch calls would remain dependent on platform-specific traversal and removal order. Tradeoffs: The watcher object remains allocated until the engine scope ends, after managed processes stop. Shutdown callbacks are suppressed during that short interval. Architectural impact: The runtime interpreter now treats watcher teardown as a recoverable edge failure and preserves the core shutdown invariant that all remaining cleanup effects execute. This is a PATCH-level bug fix. --- CHANGELOG.md | 6 ++++ docs/behavior.md | 12 ++++++-- src/engine.rs | 75 +++++++++++++++++++++++++++++++++++++++--------- 3 files changed, 77 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 02f1e2c..125b9ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to `devloop` will be recorded in this file. ## [Unreleased] +### Fixed + +- Made `ctrl-c` shutdown tolerate redundant or overlapping watch targets, so + native watcher teardown cannot skip managed-process cleanup or turn a normal + exit into an error. + ## [0.10.2] - 2026-08-26 ### Fixed diff --git a/docs/behavior.md b/docs/behavior.md index 910cfb6..c767286 100644 --- a/docs/behavior.md +++ b/docs/behavior.md @@ -314,9 +314,15 @@ output-derived updates. On `ctrl-c`, `devloop`: 1. marks itself as shutting down -2. stops all managed processes -3. suppresses further automatic restarts -4. exits +2. stops watching without requiring configured watch targets to be + disjoint or unique +3. stops all managed processes +4. suppresses further automatic restarts +5. exits successfully + +Overlapping recursive and literal watch patterns are valid. A redundant +watch registration or an already-removed backend watch cannot interrupt +process cleanup during shutdown. ## Known non-goals diff --git a/src/engine.rs b/src/engine.rs index b4fcb3b..d320bd6 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -13,7 +13,7 @@ use notify::{ use serde_json::{Map, Value}; use tokio::signal; use tokio::time::{Instant, sleep}; -use tracing::{error, info}; +use tracing::{error, info, warn}; use unicode_width::UnicodeWidthStr; use crate::browser_reload::{BrowserReloadSender, BrowserReloadServer, notify_browser_reload}; @@ -76,7 +76,6 @@ struct LiveRuntimeAdapter<'a, 'b> { watcher: &'a mut Box, watcher_shutdown: Arc, watched_targets: Vec, - active_watch_targets: Vec, external_event_tx: tokio::sync::mpsc::UnboundedSender, external_event_server: Option, browser_reload_server: Option, @@ -135,7 +134,6 @@ impl Engine { watcher: &mut watcher, watcher_shutdown, watched_targets, - active_watch_targets: Vec::new(), external_event_tx, external_event_server: None, browser_reload_server: None, @@ -318,7 +316,6 @@ impl RuntimeEffectAdapter for LiveRuntimeAdapter<'_, '_> { } async fn start_watching(&mut self) -> Result<()> { - self.active_watch_targets.clear(); let mut registrations = BTreeMap::::new(); for target in &self.watched_targets { for registration in resolve_watch_registrations(target, self.config.watcher.kind)? { @@ -339,7 +336,6 @@ impl RuntimeEffectAdapter for LiveRuntimeAdapter<'_, '_> { RecursiveMode::NonRecursive }, )?; - self.active_watch_targets.push(registration.clone()); info!( "watching {}{}", registration.path.display(), @@ -369,11 +365,11 @@ impl RuntimeEffectAdapter for LiveRuntimeAdapter<'_, '_> { } async fn stop_watching(&mut self) -> Result<()> { + // Native watcher backends may represent overlapping recursive and + // literal registrations with the same underlying OS watches. Dropping + // the whole watcher is the idempotent shutdown operation; unregistering + // each configured target can remove a descendant twice. self.watcher_shutdown.store(true, Ordering::Relaxed); - for target in &self.active_watch_targets { - self.watcher.unwatch(&target.path)?; - } - self.active_watch_targets.clear(); Ok(()) } @@ -437,7 +433,14 @@ async fn execute_runtime_effects( } } RuntimeEffect::LogInfo { message } => adapter.log_info(message).await?, - RuntimeEffect::StopWatching => adapter.stop_watching().await?, + RuntimeEffect::StopWatching => { + if let Err(error) = adapter.stop_watching().await { + warn!( + error = %error, + "watcher teardown failed; continuing process cleanup" + ); + } + } RuntimeEffect::StopAllProcesses => adapter.stop_all_processes().await?, RuntimeEffect::Exit => return Ok(true), } @@ -456,6 +459,9 @@ fn forward_watcher_event( mut result: notify::Result, ignored_paths: &[PathBuf], ) { + if shutting_down.load(Ordering::Relaxed) { + return; + } if let Ok(event) = &mut result { event.paths.retain(|path| { !ignored_paths @@ -1595,6 +1601,7 @@ mod tests { calls: Vec, changed_hooks: BTreeMap, workflow_errors: BTreeMap, + stop_watching_error: Option, watching: bool, } @@ -1604,6 +1611,7 @@ mod tests { calls: Vec::new(), changed_hooks: BTreeMap::new(), workflow_errors: BTreeMap::new(), + stop_watching_error: None, watching: false, } } @@ -1668,6 +1676,9 @@ mod tests { async fn stop_watching(&mut self) -> Result<()> { self.calls.push("stop_watch".into()); + if let Some(message) = &self.stop_watching_error { + return Err(anyhow!(message.clone())); + } self.watching = false; Ok(()) } @@ -1763,11 +1774,47 @@ mod tests { ); } + #[tokio::test] + async fn ctrl_c_continues_cleanup_when_watcher_teardown_reports_missing_watch() { + let config = Config { + root: PathBuf::from("."), + debounce_ms: 100, + watcher: crate::config::WatcherConfig::default(), + state_file: Some(PathBuf::from("./state.json")), + startup_workflows: vec![], + watch: BTreeMap::new(), + process: BTreeMap::new(), + hook: BTreeMap::new(), + event_server: crate::config::EventServerConfig::default(), + browser_reload_server: crate::config::BrowserReloadServerConfig::default(), + event: BTreeMap::new(), + workflow: BTreeMap::new(), + }; + let mut runtime = RuntimeMachine::new(&config); + let mut adapter = MockRuntimeAdapter::new(); + adapter.stop_watching_error = + Some("No watch was found. about [\"content/banner.html\"]".into()); + + runtime.handle_event(RuntimeEvent::CtrlC); + let exit = execute_runtime_effects(&mut runtime, &mut adapter) + .await + .expect("watcher teardown must not abort the remaining shutdown effects"); + + assert!(exit); + assert_eq!( + adapter.calls, + vec![ + "log:received ctrl-c, shutting down", + "stop_watch", + "stop_all", + ] + ); + } + #[test] - fn forward_watcher_event_ignores_send_failures_after_shutdown() { - let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); + fn forward_watcher_event_ignores_events_after_shutdown() { + let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel(); let shutdown = AtomicBool::new(true); - drop(rx); forward_watcher_event( &tx, @@ -1779,6 +1826,8 @@ mod tests { }), &[], ); + + assert!(rx.try_recv().is_err()); } #[test] From ee066ffec96df8908ca2d5d5048a1eea44dae752 Mon Sep 17 00:00:00 2001 From: Daniel Vianna <1708810+pasunboneleve@users.noreply.github.com> Date: Wed, 26 Aug 2026 21:58:56 +1000 Subject: [PATCH 2/3] Prepare v0.10.3 release Context: The Ctrl-C watcher teardown fix is a backwards-compatible correction to the 0.10 series and is ready for an artifact release. Decision: Advance Cargo metadata to 0.10.3 and move the complete unreleased fix into the dated 0.10.3 changelog section while restoring an empty Unreleased section. Alternatives considered: A minor release would overstate a bug fix that adds no new capability. Leaving the fix unreleased would not deliver the corrected installed binary requested by the user. Tradeoffs: This patch release depends on protected PR CI, post-merge main CI, and both tag-triggered platform artifact workflows before it is complete. Architectural impact: No additional runtime boundary changes are introduced here; this commit aligns version metadata, release notes, and the intended v0.10.3 tag. --- CHANGELOG.md | 2 ++ Cargo.lock | 2 +- Cargo.toml | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 125b9ae..1d856ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ All notable changes to `devloop` will be recorded in this file. ## [Unreleased] +## [0.10.3] - 2026-08-26 + ### Fixed - Made `ctrl-c` shutdown tolerate redundant or overlapping watch targets, so diff --git a/Cargo.lock b/Cargo.lock index b2ae63c..2f44a52 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -235,7 +235,7 @@ checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570" [[package]] name = "devloop" -version = "0.10.2" +version = "0.10.3" dependencies = [ "anyhow", "axum", diff --git a/Cargo.toml b/Cargo.toml index ba86581..59c9e60 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "devloop" -version = "0.10.2" +version = "0.10.3" edition = "2024" [dependencies] From 16e4ac6fa39a448b95282e0d3bfd2dcd7233c09d Mon Sep 17 00:00:00 2001 From: Daniel Vianna <1708810+pasunboneleve@users.noreply.github.com> Date: Wed, 26 Aug 2026 22:07:29 +1000 Subject: [PATCH 3/3] Make CI smoke state reads retryable --- scripts/ci-smoke.sh | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/scripts/ci-smoke.sh b/scripts/ci-smoke.sh index e858890..506f5f1 100755 --- a/scripts/ci-smoke.sh +++ b/scripts/ci-smoke.sh @@ -85,7 +85,11 @@ state_path = pathlib.Path(sys.argv[1]) deadline = time.time() + 15 while time.time() < deadline: if state_path.exists(): - data = json.loads(state_path.read_text()) + try: + data = json.loads(state_path.read_text()) + except json.JSONDecodeError: + time.sleep(0.1) + continue if data.get("current_value") == "initial": sys.exit(0) time.sleep(0.1) @@ -123,7 +127,11 @@ while time.time() < deadline: watched_path.write_text("updated\n") next_write = now + 0.5 if state_path.exists(): - data = json.loads(state_path.read_text()) + try: + data = json.loads(state_path.read_text()) + except json.JSONDecodeError: + time.sleep(0.1) + continue if ( data.get("current_value") == "updated" and data.get("current_url") == "devloop://updated"