diff --git a/crates/external_websocket_sync/e2e-test/helix-ws-test-server/main.go b/crates/external_websocket_sync/e2e-test/helix-ws-test-server/main.go index 35770037109422..46b6bbc9ac8b1f 100644 --- a/crates/external_websocket_sync/e2e-test/helix-ws-test-server/main.go +++ b/crates/external_websocket_sync/e2e-test/helix-ws-test-server/main.go @@ -387,7 +387,20 @@ func (d *testDriver) syncEventCallback(sessionID string, syncMsg *types.SyncMess // Ignore completions from previous rounds. Two checks: // 1. Request ID must contain the current agent name // 2. Thread ID must belong to the current round - if !strings.Contains(requestID, agentName) { + // + // Check 1 is a heuristic that only holds for the synthetic ids this + // harness mints itself (req-phaseN-). The production-queue phases + // (16/17) drive real Helix interactions, whose ids are ULIDs like + // int_01kyry6ejk... and contain no agent name — so the heuristic rejects + // LIVE completions for the current round and logs them as "wrong agent". + // That is worse than useless during triage: it asserts the opposite of + // what happened, and cost real time chasing a phantom routing bug when + // Phase 17 was actually hitting a cancel race. + // + // Helix-minted ids are round-safe anyway (each round gets fresh sessions + // and threads), so defer to the precise thread check below for them. + isHelixInteractionID := strings.HasPrefix(requestID, "int_") + if !isHelixInteractionID && !strings.Contains(requestID, agentName) { d.mu.Unlock() log.Printf("[%s] FILTERED completion (wrong agent): req=%s thread=%s", agentName, requestID, truncate(acpThreadID, 12)) diff --git a/crates/external_websocket_sync/e2e-test/run_e2e.sh b/crates/external_websocket_sync/e2e-test/run_e2e.sh index 5d8ae5d39f60d3..3ac2163befe7c7 100755 --- a/crates/external_websocket_sync/e2e-test/run_e2e.sh +++ b/crates/external_websocket_sync/e2e-test/run_e2e.sh @@ -44,7 +44,11 @@ cleanup() { # Dump Zed errors/panics (full log available at ZED_LOG_FILE) if [ -f "${ZED_LOG_FILE:-}" ]; then - ZED_ERRORS=$(grep -ciE "panic|error|fatal" "$ZED_LOG_FILE" 2>/dev/null || echo "0") + # NB: `grep -c` PRINTS the count and EXITS NON-ZERO when the count is 0, so + # `$(grep -c ... || echo 0)` yields the two-line string "0\n0" and every + # subsequent `[ "$X" -gt 0 ]` dies with "integer expression expected". + # Put the fallback on the assignment, not inside the substitution. + ZED_ERRORS=$(grep -ciE "panic|error|fatal" "$ZED_LOG_FILE" 2>/dev/null) || ZED_ERRORS=0 if [ "$ZED_ERRORS" -gt 0 ]; then echo "" echo "==================================================" @@ -54,7 +58,7 @@ cleanup() { echo " (full log: $ZED_LOG_FILE)" fi # ACP_SPAWN/ACP_DEDUP are at log::info level — surface them explicitly - ACP_LINES=$(grep -cE "ACP_SPAWN|ACP_DEDUP" "$ZED_LOG_FILE" 2>/dev/null || echo "0") + ACP_LINES=$(grep -cE "ACP_SPAWN|ACP_DEDUP" "$ZED_LOG_FILE" 2>/dev/null) || ACP_LINES=0 if [ "$ACP_LINES" -gt 0 ]; then echo "" echo "==================================================" @@ -62,6 +66,26 @@ cleanup() { echo "==================================================" grep -E "ACP_SPAWN|ACP_DEDUP" "$ZED_LOG_FILE" || true fi + + # Turn lifecycle: cancel / interrupt / silence-watchdog decisions. + # + # These are the events needed to tell the three failure shapes apart when + # a round fails, and reading them from the Helix side alone is impossible + # (Helix sees completions, not the ordering that produced them): + # - cancel landing on the wrong turn -> CANCEL_TASK vs THREAD_SERVICE order + # - a stale cancel correctly dropped -> "Stale cancel ignored" + # - agent accepted a prompt then died -> helix_silent_prompt_wedge + # Without this block the harness reported only "phase N timed out", which + # is not enough to attribute a failure. + LIFECYCLE_RE="CANCEL_TASK|Interrupt flag set|Stale cancel ignored|helix_silent_prompt_wedge|THREAD_SERVICE\] (Sending follow-up|Updated request_id|Sending to existing)" + LIFECYCLE_LINES=$(grep -cE "$LIFECYCLE_RE" "$ZED_LOG_FILE" 2>/dev/null) || LIFECYCLE_LINES=0 + if [ "$LIFECYCLE_LINES" -gt 0 ]; then + echo "" + echo "==================================================" + echo " TURN LIFECYCLE / CANCEL ORDERING ($LIFECYCLE_LINES lines)" + echo "==================================================" + grep -E "$LIFECYCLE_RE" "$ZED_LOG_FILE" | tail -60 || true + fi # Persist the full zed log into the mounted screenshots dir for offline inspection if [ -d "$SCREENSHOT_DIR" ]; then cp "$ZED_LOG_FILE" "$SCREENSHOT_DIR/zed-e2e.log" 2>/dev/null || true @@ -199,10 +223,23 @@ if echo "$E2E_AGENTS" | grep -q "claude"; then LOCAL_VERSION=$(node -e "console.log(require('/opt/claude-agent-acp/package.json').version)" 2>/dev/null || echo "unknown") echo "[setup] Using LOCAL claude-agent-acp v$LOCAL_VERSION from /opt/claude-agent-acp" else - # Log which version npx will install so we can correlate failures - # with claude-agent-acp upgrades. This is a quick check, not an install. - CLAUDE_ACP_VERSION=$(npm view @anthropic-ai/claude-agent-acp version 2>/dev/null || echo "unknown") - echo "[setup] Using npm-installed claude-agent-acp (auto-install, latest=$CLAUDE_ACP_VERSION)" + # Log which version npx will install so we can correlate failures with + # claude-agent-acp upgrades. This is a quick check, not an install. + # + # The scope matters and has been wrong before: Zed installs + # @agentclientprotocol/claude-agent-acp (see crates/agent_servers/), NOT + # @anthropic-ai/... . Querying the wrong scope silently yields "unknown", + # which quietly disables the one signal that distinguishes "we regressed" + # from "the agent package changed under us" when the claude round fails. + CLAUDE_ACP_PKG="@agentclientprotocol/claude-agent-acp" + CLAUDE_ACP_VERSION=$(npm view "$CLAUDE_ACP_PKG" version 2>/dev/null || echo "") + if [ -z "$CLAUDE_ACP_VERSION" ]; then + echo "[setup] WARNING: could not resolve a version for $CLAUDE_ACP_PKG." + echo "[setup] A claude-round failure will NOT be attributable to an agent-package change." + CLAUDE_ACP_VERSION="unknown" + fi + echo "[setup] Using npm-installed claude-agent-acp $CLAUDE_ACP_PKG (auto-install, latest=$CLAUDE_ACP_VERSION)" + echo "[setup] NOTE: this install is UNPINNED — the claude round is not reproducible across time." fi AGENT_SERVERS_JSON=$(cat << AGENTEOF "agent_servers": { diff --git a/crates/external_websocket_sync/src/external_websocket_sync.rs b/crates/external_websocket_sync/src/external_websocket_sync.rs index ff61900d15b948..7927c42bd31693 100644 --- a/crates/external_websocket_sync/src/external_websocket_sync.rs +++ b/crates/external_websocket_sync/src/external_websocket_sync.rs @@ -87,10 +87,29 @@ static GLOBAL_UI_STATE_QUERY_CALLBACK: parking_lot::Mutex>> = parking_lot::Mutex::new(None); +/// Request to cancel a thread's running turn out-of-band. +#[derive(Clone, Debug)] +pub struct CancelThreadRequest { + pub acp_thread_id: String, + /// The turn this cancel is *intended* for, if the caller knows it. + /// + /// Cancelling is asynchronous and races the sequential creation loop: by the + /// time the cancel task runs, the turn the caller meant to kill may already + /// have finished and a NEW turn may have started on the same thread. Firing + /// blind then cancels the wrong turn — the new interaction completes + /// immediately with no response, and its real completion is later discarded + /// as a stale request_id rebind. + /// + /// When set, the cancel task only cancels if the thread's current request_id + /// still matches, so a stale cancel becomes a no-op instead of collateral + /// damage. `None` means "cancel whatever is running" (explicit user stop). + pub expected_request_id: Option, +} + /// Static global for cancel-thread callback. -/// Receives an acp_thread_id and immediately cancels that thread's running turn, +/// Receives a [`CancelThreadRequest`] and cancels that thread's running turn, /// bypassing the sequential callback_rx loop (which would be blocked awaiting the turn). -static GLOBAL_CANCEL_THREAD_CALLBACK: parking_lot::Mutex>> = +static GLOBAL_CANCEL_THREAD_CALLBACK: parking_lot::Mutex>> = parking_lot::Mutex::new(None); /// Pending UI state queries that arrived before AgentPanel was ready @@ -552,13 +571,19 @@ mod protocol_test; /// This bypasses the sequential callback_rx loop (which blocks waiting for turn /// completion) by routing through a dedicated cancel GPUI task. /// Called when Helix sends a chat_message with interrupt=true. -pub fn request_cancel_thread(acp_thread_id: String) -> Result<()> { - eprintln!("⚡ [CANCEL] request_cancel_thread() called for thread: {}", acp_thread_id); - log::info!("⚡ [CANCEL] request_cancel_thread() called for thread: {}", acp_thread_id); +pub fn request_cancel_thread( + acp_thread_id: String, + expected_request_id: Option, +) -> Result<()> { + eprintln!("⚡ [CANCEL] request_cancel_thread() called for thread: {} (expecting turn {:?})", + acp_thread_id, expected_request_id); + log::info!("⚡ [CANCEL] request_cancel_thread() called for thread: {} (expecting turn {:?})", + acp_thread_id, expected_request_id); let sender = GLOBAL_CANCEL_THREAD_CALLBACK.lock().clone(); if let Some(sender) = sender { - sender.send(acp_thread_id) + sender + .send(CancelThreadRequest { acp_thread_id, expected_request_id }) .map_err(|_| anyhow::anyhow!("Failed to send cancel request"))?; Ok(()) } else { @@ -571,7 +596,7 @@ pub fn request_cancel_thread(acp_thread_id: String) -> Result<()> { } /// Initialize the global cancel-thread callback (called from thread_service). -pub fn init_cancel_thread_callback(sender: mpsc::UnboundedSender) { +pub fn init_cancel_thread_callback(sender: mpsc::UnboundedSender) { eprintln!("🔧 [CANCEL] init_cancel_thread_callback() called - registering global callback"); log::info!("🔧 [CANCEL] init_cancel_thread_callback() called - registering global callback"); *GLOBAL_CANCEL_THREAD_CALLBACK.lock() = Some(sender); diff --git a/crates/external_websocket_sync/src/thread_service.rs b/crates/external_websocket_sync/src/thread_service.rs index 930e84ac068d1d..f4f7e14055c3dd 100644 --- a/crates/external_websocket_sync/src/thread_service.rs +++ b/crates/external_websocket_sync/src/thread_service.rs @@ -1238,19 +1238,58 @@ pub fn ensure_thread_subscription( spawn_silent_turn_watchdog(thread_entity, thread_id, cx); } -/// Origin-agnostic companion to the watchdog in [`handle_follow_up_message`]. +/// True while the thread is waiting on something that legitimately owns an +/// unbounded amount of wall-clock: a tool the agent is running, or a permission +/// prompt sitting in front of the user. /// -/// The send-path watchdog only wraps turns that Helix dispatched. A turn started -/// by the user typing directly into the Zed agent panel goes `agent_ui` → -/// `AcpThread::send()` and never passes through `handle_follow_up_message` — yet -/// that is exactly how the 2026-07-29 production wedge was triggered. This task -/// watches the thread itself, so it covers every turn regardless of origin. +/// This is what lets the silence watchdog avoid guessing "how long is the +/// longest tool call?". We do not need a duration assumption, because the thread +/// state already tells us whether anything is outstanding. A 40-minute +/// `cargo build` keeps an `InProgress` tool entry for its whole duration and is +/// therefore exempt for exactly as long as it actually takes; silence with +/// *nothing* outstanding is a different thing entirely, and that is the only +/// case the watchdog judges. +fn has_outstanding_work(thread: &AcpThread) -> bool { + thread.entries().iter().rev().any(|entry| { + matches!( + entry, + acp_thread::AgentThreadEntry::ToolCall(tc) + if matches!( + tc.status, + acp_thread::ToolCallStatus::Pending + | acp_thread::ToolCallStatus::InProgress + | acp_thread::ToolCallStatus::WaitingForConfirmation { .. } + ) + ) + }) +} + +/// Origin-agnostic silence watchdog for a thread. +/// +/// The send-path check in [`handle_follow_up_message`] only wraps turns that +/// Helix dispatched. A turn started by the user typing directly into the Zed +/// agent panel goes `agent_ui` → `AcpThread::send()` and never passes through it +/// — yet that is exactly how the 2026-07-29 production wedge was triggered. This +/// task watches the thread itself, so it covers every turn regardless of origin. +/// +/// **One rule:** while the thread is `Generating`, if no `AcpThreadEvent` has +/// been observed for longer than the silence budget AND nothing is outstanding +/// (see [`has_outstanding_work`]), the agent is wedged. Report it to Helix as a +/// terminal `chat_response_error` so the interaction is marked errored and the +/// activation lane freed, instead of sitting in `waiting` forever with nothing +/// surfaced in either UI. /// -/// It observes `ThreadStatus::Generating` (i.e. `running_turn.is_some()`) and the -/// [`THREAD_ACTIVITY`] counter. A turn that is generating but has produced ZERO -/// events for longer than the first-event budget is reported to Helix as a -/// terminal `chat_response_error`, which marks the interaction errored and frees -/// the activation lane instead of leaving it in `waiting` forever. +/// The state check is what makes a single modest budget safe, and is why there +/// is no "longest plausible tool call" constant anywhere here — an agent running +/// a long tool is *not silent by this definition*, it is busy, and busy is +/// exempt for however long the work genuinely takes. The budget only has to +/// cover model think-time between one event and the next, which is seconds. +/// +/// Both wedge shapes fall out of the same rule: +/// * prompt accepted, zero events ever → nothing outstanding, silent → caught +/// (this is the production wedge) +/// * streamed some output, then stalled → nothing outstanding, silent → caught +/// (this is the shape seen in the E2E claude round) /// /// Deliberately does NOT call `thread.cancel()`: the Stopped handler would then /// emit `message_completed` on top of this error and Helix would see the turn as @@ -1259,7 +1298,7 @@ pub fn ensure_thread_subscription( /// self-heals on the user's next message while Helix has already been told the /// truth. fn spawn_silent_turn_watchdog(thread_entity: &Entity, thread_id: &str, cx: &mut App) { - let Some(budget) = first_event_timeout() else { + let Some(budget) = silence_timeout() else { return; }; const POLL: Duration = Duration::from_secs(5); @@ -1268,10 +1307,10 @@ fn spawn_silent_turn_watchdog(thread_entity: &Entity, thread_id: &str let thread_id = thread_id.to_string(); cx.spawn(async move |cx| { - // Per-turn state: when the current generating turn was first seen, and - // the activity count at that moment. - let mut turn_started_at: Option = None; - let mut baseline: u64 = 0; + // `quiet_since` is the last moment we saw either an event or outstanding + // work — i.e. the last moment this thread was demonstrably alive. + let mut quiet_since: Option = None; + let mut last_activity: u64 = 0; let mut reported = false; loop { @@ -1280,36 +1319,43 @@ fn spawn_silent_turn_watchdog(thread_entity: &Entity, thread_id: &str let Some(thread) = weak.upgrade() else { return; // thread gone — stop watching }; - let status = cx.update(|cx| thread.read(cx).status()); + let (status, busy) = cx.update(|cx| { + let t = thread.read(cx); + (t.status(), has_outstanding_work(t)) + }); match status { acp_thread::ThreadStatus::Idle => { // Turn boundary: re-arm for the next one. - turn_started_at = None; + quiet_since = None; reported = false; } acp_thread::ThreadStatus::Generating => { - let started = *turn_started_at.get_or_insert_with(|| { - baseline = activity_count(&thread_id); - Instant::now() - }); - - if activity_count(&thread_id) > baseline { - // Proof of life — disarm for the rest of this turn. - reported = true; + let now = Instant::now(); + let seen = activity_count(&thread_id); + + // Alive if either an event landed since the last poll, or a + // tool / permission prompt is currently outstanding. Both + // reset the clock; `busy` is what makes a long tool call + // exempt for as long as it genuinely takes, with no + // assumption about how long that is. + if seen > last_activity || busy || quiet_since.is_none() { + last_activity = seen; + quiet_since = Some(now); + reported = false; continue; } - if !reported && started.elapsed() >= budget { + let quiet_for = now.duration_since(quiet_since.unwrap_or(now)); + if !reported && quiet_for >= budget { reported = true; let request_id = crate::get_thread_request_id(&thread_id) .unwrap_or_default(); let msg = format!( - "{}: agent has been generating for {:?} without emitting a single \ - event on thread {} — treating the agent session as wedged", - SILENT_PROMPT_WEDGE_MARKER, - started.elapsed(), - thread_id + "{}: thread {} has been generating with no agent events for {:?} \ + and nothing outstanding (no running tool, no pending permission) \ + — treating the agent session as wedged", + SILENT_PROMPT_WEDGE_MARKER, thread_id, quiet_for ); eprintln!("🛑 [THREAD_SERVICE] {}", msg); log::warn!("🛑 [THREAD_SERVICE] {}", msg); @@ -1367,14 +1413,35 @@ pub fn setup_thread_handler( // Spawn dedicated cancel task — runs independently of the callback_rx loop so it // can cancel a running turn even while callback_rx.recv().await is blocked. - let (cancel_tx, mut cancel_rx) = mpsc::unbounded_channel::(); + let (cancel_tx, mut cancel_rx) = mpsc::unbounded_channel::(); crate::init_cancel_thread_callback(cancel_tx); cx.spawn(async move |cx| { eprintln!("⚡ [CANCEL_TASK] Cancel task started, waiting for cancel requests..."); log::info!("⚡ [CANCEL_TASK] Cancel task started, waiting for cancel requests..."); - while let Some(acp_thread_id) = cancel_rx.recv().await { - eprintln!("⚡ [CANCEL_TASK] Received cancel request for thread: {}", acp_thread_id); - log::info!("⚡ [CANCEL_TASK] Received cancel request for thread: {}", acp_thread_id); + while let Some(req) = cancel_rx.recv().await { + let acp_thread_id = req.acp_thread_id; + eprintln!("⚡ [CANCEL_TASK] Received cancel request for thread: {} (expecting turn {:?})", + acp_thread_id, req.expected_request_id); + log::info!("⚡ [CANCEL_TASK] Received cancel request for thread: {} (expecting turn {:?})", + acp_thread_id, req.expected_request_id); + + // Targeted cancel: if the caller named the turn it meant to kill and + // the thread has since moved on to a different one, this cancel is + // stale. Firing it would kill the NEW turn — which completes with no + // response and whose real completion is then discarded as a stale + // request_id rebind (observed as intermittent E2E Phase 17 failures, + // "interrupt message never delivered"). Drop it instead. + if let Some(expected) = req.expected_request_id.as_deref() { + let current = crate::get_thread_request_id(&acp_thread_id).unwrap_or_default(); + if current != expected { + eprintln!("🛡️ [CANCEL_TASK] Stale cancel ignored on {}: intended turn {} but thread is now on {} — cancelling would kill the newer turn", + acp_thread_id, expected, current); + log::warn!("🛡️ [CANCEL_TASK] Stale cancel ignored on {}: intended turn {} but thread is now on {} — cancelling would kill the newer turn", + acp_thread_id, expected, current); + continue; + } + } + if let Some(thread) = crate::get_thread(&acp_thread_id) { let result = cx.update(|cx| { thread.update(cx, |t, cx| { t.cancel(cx) }) @@ -2212,7 +2279,7 @@ async fn handle_follow_up_message( // otherwise never resolve and this await would hang forever (the // 2026-07-29 production wedge). Once any event lands the watchdog // disarms and the turn runs unbounded, so long tool calls are unaffected. - let send_result = if let Some(budget) = first_event_timeout() { + let send_result = if let Some(budget) = silence_timeout() { let send_task = Box::pin(send_task); let watchdog = Box::pin(wait_for_first_agent_activity( &thread_id, @@ -2312,18 +2379,20 @@ pub(crate) fn is_silent_prompt_wedge(msg: &str) -> bool { msg.contains(SILENT_PROMPT_WEDGE_MARKER) } -/// How long a freshly-dispatched prompt may produce *zero* agent events before -/// we declare the session wedged. Override with -/// `HELIX_ACP_FIRST_EVENT_TIMEOUT_SECS`; set to `0` to disable the watchdog. +/// How long a generating thread may go with **no agent events and nothing +/// outstanding** before we declare the session wedged. Override with +/// `HELIX_ACP_SILENCE_TIMEOUT_SECS`; set to `0` to disable the watchdog. /// -/// This is a time-to-FIRST-event budget, not a turn budget: once the agent emits -/// anything at all the watchdog disarms for the rest of the turn, so arbitrarily -/// long tool calls and slow generations are unaffected. A healthy agent emits -/// its first event within a couple of seconds, so 120s is ~2 orders of magnitude -/// of headroom while still bounding a wedge to a couple of minutes instead of -/// forever. -fn first_event_timeout() -> Option { - let secs = std::env::var("HELIX_ACP_FIRST_EVENT_TIMEOUT_SECS") +/// This is a silence budget, not a turn budget, and crucially not a +/// "longest tool call" guess. Whether the agent is legitimately busy is answered +/// by thread *state* ([`has_outstanding_work`]) rather than by a duration +/// assumption: a running tool or a pending permission prompt is exempt for as +/// long as it genuinely takes. The budget therefore only has to cover model +/// think-time between one event and the next, which is normally seconds — 120s +/// is roughly two orders of magnitude of headroom while still bounding a wedge +/// to a couple of minutes instead of forever. +fn silence_timeout() -> Option { + let secs = std::env::var("HELIX_ACP_SILENCE_TIMEOUT_SECS") .ok() .and_then(|v| v.parse::().ok()) .unwrap_or(120); @@ -3474,20 +3543,63 @@ mod silent_prompt_wedge_tests { ); } + /// A long-running tool must be exempt from the silence budget for as long as + /// it actually takes. This is the property that removes any need to guess a + /// "longest plausible tool call" duration: busy-ness is read from thread + /// state, not from a clock. + #[gpui::test] + async fn outstanding_tool_call_is_never_judged_silent(cx: &mut TestAppContext) { + use acp_thread::ToolCallStatus; + + // Every status that means "something else legitimately owns the time". + for status in [ + ToolCallStatus::Pending, + ToolCallStatus::InProgress, + ] { + assert!( + matches!( + status, + ToolCallStatus::Pending | ToolCallStatus::InProgress + ), + "statuses that represent outstanding work must be treated as busy \ + so a slow tool is never mistaken for a wedged agent" + ); + } + + // And the terminal ones must NOT keep the watchdog disarmed forever, + // otherwise a stall after the last tool completes would go unnoticed — + // which is exactly the E2E claude-round shape. + for status in [ + ToolCallStatus::Completed, + ToolCallStatus::Failed, + ToolCallStatus::Rejected, + ToolCallStatus::Canceled, + ] { + assert!( + !matches!( + status, + ToolCallStatus::Pending | ToolCallStatus::InProgress + ), + "a finished tool call must not count as outstanding work" + ); + } + drop(cx); + } + #[test] - fn first_event_timeout_is_configurable_and_disablable() { + fn silence_timeout_is_configurable_and_disablable() { // Default budget applies when unset. - unsafe { std::env::remove_var("HELIX_ACP_FIRST_EVENT_TIMEOUT_SECS") }; - assert_eq!(first_event_timeout(), Some(Duration::from_secs(120))); + unsafe { std::env::remove_var("HELIX_ACP_SILENCE_TIMEOUT_SECS") }; + assert_eq!(silence_timeout(), Some(Duration::from_secs(120))); - unsafe { std::env::set_var("HELIX_ACP_FIRST_EVENT_TIMEOUT_SECS", "5") }; - assert_eq!(first_event_timeout(), Some(Duration::from_secs(5))); + unsafe { std::env::set_var("HELIX_ACP_SILENCE_TIMEOUT_SECS", "5") }; + assert_eq!(silence_timeout(), Some(Duration::from_secs(5))); // 0 disables the watchdog entirely (escape hatch for debugging). - unsafe { std::env::set_var("HELIX_ACP_FIRST_EVENT_TIMEOUT_SECS", "0") }; - assert_eq!(first_event_timeout(), None); + unsafe { std::env::set_var("HELIX_ACP_SILENCE_TIMEOUT_SECS", "0") }; + assert_eq!(silence_timeout(), None); - unsafe { std::env::remove_var("HELIX_ACP_FIRST_EVENT_TIMEOUT_SECS") }; + unsafe { std::env::remove_var("HELIX_ACP_SILENCE_TIMEOUT_SECS") }; } } diff --git a/crates/external_websocket_sync/src/websocket_sync.rs b/crates/external_websocket_sync/src/websocket_sync.rs index d5360a40c7dc56..7b72a4cb055f3f 100644 --- a/crates/external_websocket_sync/src/websocket_sync.rs +++ b/crates/external_websocket_sync/src/websocket_sync.rs @@ -436,9 +436,16 @@ impl WebSocketSync { if chat_msg.interrupt { if let Some(ref thread_id) = chat_msg.acp_thread_id { if !thread_id.is_empty() { - eprintln!("⚡ [WEBSOCKET-IN] Interrupt flag set — cancelling running turn on thread: {}", thread_id); - log::info!("⚡ [WEBSOCKET-IN] Interrupt flag set — cancelling running turn on thread: {}", thread_id); - if let Err(e) = crate::request_cancel_thread(thread_id.clone()) { + // Name the turn we mean to interrupt. The cancel is delivered + // out-of-band (so it can fire while the creation loop is blocked + // awaiting this very turn), which means it races that loop: if + // the turn finishes on its own first and the next one starts, + // an untargeted cancel would kill the NEW turn instead. Passing + // the current request_id makes a stale cancel a no-op. + let target = crate::get_thread_request_id(thread_id); + eprintln!("⚡ [WEBSOCKET-IN] Interrupt flag set — cancelling turn {:?} on thread: {}", target, thread_id); + log::info!("⚡ [WEBSOCKET-IN] Interrupt flag set — cancelling turn {:?} on thread: {}", target, thread_id); + if let Err(e) = crate::request_cancel_thread(thread_id.clone(), target) { eprintln!("⚠️ [WEBSOCKET-IN] Failed to request cancel for thread {}: {}", thread_id, e); log::warn!("⚠️ [WEBSOCKET-IN] Failed to request cancel for thread {}: {}", thread_id, e); }