fix: skip setup commands on worktree reuse so reconciler doesn't fire npm ci every 30s

Story 1066 (merged 2026-05-14 23:39) introduced a periodic reconciler that
calls `reconcile_worktree_create` every 30 seconds (default
`reconcile_interval_secs`). The reconciler's docstring promises it is a no-op
for stories whose worktree already exists — but the implementation calls
`create_worktree`, whose reuse path was running `run_setup_commands`
unconditionally. Setup includes destructive `npm ci` (rm -rf node_modules
then reinstall), so every Coding story got `npm ci` fired every 30 seconds.

When story 1086 hit a gate-failure retry loop on 2026-05-15, the merge gate's
own `npm install`/`npm run build` raced one of these reconciler-driven
`npm ci` runs that was wiping node_modules — leaving `.bin/tsc` as a broken
symlink pointing into a half-populated `typescript/` package and producing
`sh: 1: tsc: not found`. 37 npm ci fires for 1086 in 5 hours against only
3 real Coding transitions, a 12x amplification driven entirely by the
30-second reconcile cadence.

Fix: align `create_worktree`'s behaviour with the contract `reconcile_worktree_create`
already documents — reuse is a no-op for setup commands. Sparse checkout
and `.mcp.json` rewrite still run (both cheap and idempotent).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

server/src/worktree/create.rs

@@ -14,8 +14,26 @@ use super::{WorktreeInfo, worktree_path, write_mcp_json};
 /// - Creates the worktree at `{project_root}/.huskies/worktrees/{story_id}`
 ///   on branch `feature/story-{story_id}`.
 /// - Writes `.mcp.json` in the worktree pointing to the MCP server at `port`.
-/// - Runs setup commands from the config for each component.
+/// - Runs setup commands from the config for each component **only on fresh
+///   creation** — see below.
 /// - If the worktree/branch already exists, reuses rather than errors.
+///
+/// **Idempotency on reuse:** when `wt_path` already exists, this function does
+/// **not** re-run [`run_setup_commands`].  Setup commands typically include
+/// destructive operations like `npm ci` (`rm -rf node_modules` then reinstall)
+/// that, if run concurrently with another reuse from a different caller, leave
+/// `node_modules` in a half-populated state (broken `.bin/*` symlinks pointing
+/// at empty package directories).  This used to be rare and tolerable, but
+/// after story 1066 added a 30-second periodic reconciler that calls
+/// `reconcile_worktree_create` → `create_worktree`, every Coding story got a
+/// destructive `npm ci` every 30s — racing the merge-gate's own frontend
+/// build and producing the `sh: 1: tsc: not found` failure that bricked
+/// story 1086 retries on 2026-05-15.
+///
+/// The reuse path now matches the documented contract of
+/// `reconcile_worktree_create`: "no-op for stories whose worktree already
+/// exists."  If a worktree is in a bad state and needs re-setup, the caller
+/// must explicitly delete it and call `create_worktree` again.
 pub async fn create_worktree(
     project_root: &Path,
     story_id: &str,
@@ -30,14 +48,15 @@ pub async fn create_worktree(
         .unwrap_or_else(|| detect_base_branch(project_root));
     let root = project_root.to_path_buf();
 
-    // Already exists — reuse (ensure sparse checkout is configured)
+    // Already exists — reuse without re-running destructive setup commands.
+    // Sparse checkout is reconfigured (cheap, idempotent) and `.mcp.json` is
+    // rewritten in case the server port changed across restarts.
     if wt_path.exists() {
         let wt_clone = wt_path.clone();
         tokio::task::spawn_blocking(move || configure_sparse_checkout(&wt_clone))
             .await
             .map_err(|e| format!("spawn_blocking: {e}"))??;
         write_mcp_json(&wt_path, port)?;
-        run_setup_commands(&wt_path, config).await;
         return Ok(WorktreeInfo {
             path: wt_path,
             branch,
@@ -374,32 +393,80 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn create_worktree_reuse_succeeds_despite_setup_failure() {
+    async fn create_worktree_reuse_does_not_rerun_setup_commands() {
+        // Regression for the 2026-05-15 1086 outage: the reuse path used to
+        // re-run setup commands (including destructive `npm ci`).  Combined
+        // with story 1066's 30-second periodic reconciler, this fired
+        // `npm ci` against every Coding story every 30s and caused
+        // `tsc: not found` gate failures.  The reuse path must now be a
+        // no-op for setup commands.
         let tmp = TempDir::new().unwrap();
         let project_root = tmp.path().join("my-project");
         fs::create_dir_all(&project_root).unwrap();
         init_git_repo(&project_root);
 
         // First creation — no setup commands, should succeed
-        create_worktree(&project_root, "173_reuse_fail", &empty_config(), 3001)
+        create_worktree(&project_root, "173_reuse_no_setup", &empty_config(), 3001)
             .await
             .unwrap();
 
-        // Second call — worktree exists, setup commands fail, must still succeed
+        // Second call — worktree exists.  Setup commands are configured to
+        // FAIL (`exit 1`); if the reuse path were still running them, the
+        // failure log would surface — but more importantly, this test
+        // documents that the reuse path is expected to NEVER reach
+        // `run_setup_commands` and therefore can never produce a setup
+        // failure regardless of how broken the setup config is.
         let result = create_worktree(
             &project_root,
-            "173_reuse_fail",
+            "173_reuse_no_setup",
             &failing_setup_config(),
             3002,
         )
         .await;
         assert!(
             result.is_ok(),
-            "create_worktree reuse must succeed even if setup commands fail: {:?}",
+            "reuse must succeed and must not run setup commands: {:?}",
             result.err()
         );
     }
 
+    #[tokio::test]
+    async fn create_worktree_reuse_does_not_create_setup_marker_file() {
+        // Stronger version of the above: assert that on reuse, a setup
+        // command that would have created a marker file does NOT run.
+        let tmp = TempDir::new().unwrap();
+        let project_root = tmp.path().join("my-project");
+        fs::create_dir_all(&project_root).unwrap();
+        init_git_repo(&project_root);
+
+        // First creation — no setup, so no marker yet.
+        let info = create_worktree(&project_root, "174_reuse_marker", &empty_config(), 3001)
+            .await
+            .unwrap();
+        let marker = info.path.join("__setup_ran__");
+        assert!(!marker.exists(), "no marker after empty-setup creation");
+
+        // Second call with a setup command that WOULD create the marker if
+        // run.  The reuse path must not run it.
+        let cfg = ProjectConfig {
+            component: vec![ComponentConfig {
+                name: "marker".to_string(),
+                path: ".".to_string(),
+                setup: vec!["touch __setup_ran__".to_string()],
+                teardown: vec![],
+            }],
+            ..empty_config()
+        };
+        create_worktree(&project_root, "174_reuse_marker", &cfg, 3002)
+            .await
+            .unwrap();
+
+        assert!(
+            !marker.exists(),
+            "reuse path must not run setup commands; marker file was created"
+        );
+    }
+
     #[test]
     fn install_pre_commit_hook_creates_executable_hook_and_sets_hookspath() {
         let tmp = TempDir::new().unwrap();