From 918f18c2007cb1f6a640dd9f8506d03e94f3703a Mon Sep 17 00:00:00 2001 From: dave Date: Tue, 19 May 2026 18:14:51 +0000 Subject: [PATCH] =?UTF-8?q?huskies:=20merge=201151=20bug=20install=5Fpre?= =?UTF-8?q?=5Fcommit=5Fhook=20blocks=20the=20tokio=20executor=20=E2=80=94?= =?UTF-8?q?=20sync=20std::process::Command::output()=20in=20an=20async=20p?= =?UTF-8?q?ath=20stalls=20worktree-create-sub?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- server/src/agents/pool/worktree_lifecycle.rs | 8 +++- server/src/worktree/create.rs | 48 ++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/server/src/agents/pool/worktree_lifecycle.rs b/server/src/agents/pool/worktree_lifecycle.rs index 51ab35d2..77749d36 100644 --- a/server/src/agents/pool/worktree_lifecycle.rs +++ b/server/src/agents/pool/worktree_lifecycle.rs @@ -129,7 +129,13 @@ pub(crate) async fn on_coding_transition(project_root: &Path, port: u16, story_i "[worktree-create-sub] Worktree ready for '{story_id}' at {}", info.path.display() ); - if let Err(e) = crate::worktree::install_pre_commit_hook(&info.path) { + let hook_path = info.path.clone(); + let hook_result = tokio::task::spawn_blocking(move || { + crate::worktree::install_pre_commit_hook(&hook_path) + }) + .await + .unwrap_or_else(|e| Err(format!("spawn_blocking panicked: {e}"))); + if let Err(e) = hook_result { slog_warn!( "[worktree-create-sub] Pre-commit hook install failed for '{story_id}': {e}" ); diff --git a/server/src/worktree/create.rs b/server/src/worktree/create.rs index 9355ec99..a78c4c61 100644 --- a/server/src/worktree/create.rs +++ b/server/src/worktree/create.rs @@ -475,6 +475,54 @@ mod tests { ); } + /// Regression: before the fix, `install_pre_commit_hook` was called directly + /// from an async context, monopolising a tokio worker thread per call. With + /// `worker_threads=2` and two concurrent installs the executor would stall. + /// After the fix the installs run on `spawn_blocking`'s thread pool, leaving + /// both worker threads free to drive async tasks like the sleep below. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn install_pre_commit_hook_does_not_block_tokio_executor() { + let tmp = TempDir::new().unwrap(); + + // Helper: create a git repo and a linked worktree inside tmp. + let setup = |repo: &str, wt: &str, branch: &str| -> std::path::PathBuf { + let repo_path = tmp.path().join(repo); + fs::create_dir_all(&repo_path).unwrap(); + init_git_repo(&repo_path); + Command::new("git") + .args(["config", "extensions.worktreeConfig", "true"]) + .current_dir(&repo_path) + .output() + .unwrap(); + let wt_path = tmp.path().join(wt); + Command::new("git") + .args(["worktree", "add", wt_path.to_str().unwrap(), "-b", branch]) + .current_dir(&repo_path) + .output() + .unwrap(); + wt_path + }; + + let wt1 = setup("repo1", "wt1", "feature/noblock-1"); + let wt2 = setup("repo2", "wt2", "feature/noblock-2"); + + // Both hook installs run on the blocking pool — executor threads stay free. + let h1 = tokio::task::spawn_blocking(move || install_pre_commit_hook(&wt1)); + let h2 = tokio::task::spawn_blocking(move || install_pre_commit_hook(&wt2)); + + let start = std::time::Instant::now(); + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + let elapsed = start.elapsed(); + + h1.await.unwrap().expect("hook install 1 must succeed"); + h2.await.unwrap().expect("hook install 2 must succeed"); + + assert!( + elapsed < std::time::Duration::from_millis(500), + "tokio executor was starved; 100 ms sleep took {elapsed:?}" + ); + } + #[test] fn install_pre_commit_hook_creates_executable_hook_and_sets_hookspath() { let tmp = TempDir::new().unwrap();