From b3f75fa55f649c5d5d175a7a2ebf7de9fd03c71d Mon Sep 17 00:00:00 2001 From: M-Maciej <130112810+M-Maciej@users.noreply.github.com> Date: Fri, 21 Aug 2026 11:29:05 +0000 Subject: [PATCH 1/8] feat(tui): honor goal continuation delay for host-managed turns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Host-managed engines have no cross-turn scheduler, so the configured [goal] continuation_delay_seconds quiet period was never applied: the intra-turn continuation dispatch in turn_loop.rs continued immediately. - goal_loop.rs: shared continuation_wait(delay) computation (None = immediate, capped at MAX_GOAL_CONTINUATION_DELAY_SECONDS) plus await_continuation_wait with biased cancellation — the cancel token always wins over a racing expiry (same semantics as #5508). - turn_loop.rs: goal_continuation_message_if_needed now decides before spending the quiet period, awaits the cancellable wait for host-managed turns only, and re-decides on the live goal state after the wait so pause/clear or a terminal update_goal cancels the pending pass. Failed or interrupted turns never continue. Interactive engines keep their existing cross-turn cadence untouched. Verified: cargo test -p codewhale-tui goal_loop (15/15, incl. new delay>0, zero-delay-immediate, cancellation-wins tests), 454 goal-named engine/tool/UI tests, cargo clippy -p codewhale-tui --lib, cargo fmt. Signed-off-by: M-Maciej <130112810+M-Maciej@users.noreply.github.com> --- crates/tui/src/core/engine/turn_loop.rs | 89 +++++++++++++++----- crates/tui/src/goal_loop.rs | 106 ++++++++++++++++++++++++ 2 files changed, 173 insertions(+), 22 deletions(-) diff --git a/crates/tui/src/core/engine/turn_loop.rs b/crates/tui/src/core/engine/turn_loop.rs index ee8435e013..1b6a81bccc 100644 --- a/crates/tui/src/core/engine/turn_loop.rs +++ b/crates/tui/src/core/engine/turn_loop.rs @@ -4377,28 +4377,12 @@ impl Engine { Some(snapshot) } - async fn goal_continuation_message_if_needed( - &self, - tool_registry: Option<&crate::tools::ToolRegistry>, - continuations_this_turn: &mut u32, - current_turn_usage: &Usage, - ) -> Option { - let registry = tool_registry?; - if !registry.contains("update_goal") { - return None; - } - - let mut snapshot = self.goal_snapshot_with_current_turn_usage(current_turn_usage)?; - let current_turn_tokens = u64::from(current_turn_usage.input_tokens) - .saturating_add(u64::from(current_turn_usage.output_tokens)); - - // Route the continuation decision through the goal-loop decision core. - // A goal runs until complete/blocked or the user pauses it; token/time - // accounting is telemetry (#5052). The configurable run-level backstop - // ([goal] max_continuations) only halts a pathological - // loop. The per-turn guard (`per_turn_max`) only bounds how many - // continuation passes happen *within* a single turn before yielding - // back to the engine. + /// Run the goal-loop decision core against the live goal state merged with + /// this turn's usage. `Some(snapshot)` means the goal is still active and + /// should continue; `None` means no continuation (inactive goal, terminal + /// status, or continuation backstop), after emitting the terminal status. + async fn goal_continuation_allowed(&self, current_turn_usage: &Usage) -> Option { + let snapshot = self.goal_snapshot_with_current_turn_usage(current_turn_usage)?; let decision = crate::goal_loop::decide_continuation( crate::goal_loop::GoalRunStatus::Active, crate::goal_loop::GoalProgress { @@ -4417,6 +4401,67 @@ impl Engine { let _ = self.tx_event.send(Event::status(message)).await; return None; } + Some(snapshot) + } + + async fn goal_continuation_message_if_needed( + &self, + tool_registry: Option<&crate::tools::ToolRegistry>, + continuations_this_turn: &mut u32, + current_turn_usage: &Usage, + ) -> Option { + let registry = tool_registry?; + if !registry.contains("update_goal") { + return None; + } + + // Decide first so a terminal goal never spends the quiet period — + // failures never continue (host-managed cadence). + self.goal_continuation_allowed(current_turn_usage) + .await + .as_ref()?; + + // Host-managed turns have no cross-turn scheduler, so the configured + // between-continuation quiet period is awaited right here. The wait is + // cancellable: the cancel token (Esc) wins biased over the timer, and + // a pause/clear or terminal update_goal observed after the wait + // cancels the pending pass before anything is recorded or dispatched. + let wait = if self.host_managed_turns() { + crate::goal_loop::continuation_wait(self.config.goal_continuation_delay_seconds) + } else { + None + }; + let was_delayed = wait.is_some(); + if let Some(wait) = wait { + let _ = self + .tx_event + .send(Event::GoalContinuationWaiting { + delay_seconds: wait.as_secs(), + }) + .await; + } + if crate::goal_loop::await_continuation_wait(wait, &self.cancel_token).await + == crate::goal_loop::ContinuationWaitOutcome::Cancelled + { + let _ = self + .tx_event + .send(Event::GoalContinuationWaitEnded { interrupted: true }) + .await; + return None; + } + if was_delayed { + let _ = self + .tx_event + .send(Event::GoalContinuationWaitEnded { interrupted: false }) + .await; + } + + // Re-decide on the live state after the quiet period: /goal pause, + // /goal clear, or a terminal update_goal during the wait cancels the + // pending pass instead of dispatching a provider request. + let mut snapshot = self.goal_continuation_allowed(current_turn_usage).await?; + let current_turn_tokens = u64::from(current_turn_usage.input_tokens) + .saturating_add(u64::from(current_turn_usage.output_tokens)); *continuations_this_turn = (*continuations_this_turn).saturating_add(1); match self.config.goal_state.lock() { diff --git a/crates/tui/src/goal_loop.rs b/crates/tui/src/goal_loop.rs index d2f729cb0f..82e1bb3a84 100644 --- a/crates/tui/src/goal_loop.rs +++ b/crates/tui/src/goal_loop.rs @@ -20,6 +20,8 @@ //! (`turnBudget` per-task, resumable after budget-reached). Log when the //! backstop fires. +use std::time::Duration; + /// Default automatic cross-turn continuation policy for one goal run (#5052). /// /// Goals are unlimited by default: completion, blocked status, or explicit @@ -184,6 +186,53 @@ pub fn decide_continuation( ContinuationDecision::Continue } +/// Outcome of waiting out the between-continuation quiet period. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ContinuationWaitOutcome { + /// The quiet period elapsed — dispatch the continuation. + Elapsed, + /// Cancelled during the quiet period — never dispatch. + Cancelled, +} + +/// Compute the quiet-period wait for a configured between-continuation delay. +/// `None` continues immediately (unset or zero delay); a positive delay +/// returns the capped wait shared by every dispatch path so no caller can +/// construct an effectively uninterruptible schedule receipt. +#[must_use] +pub const fn continuation_wait(delay_seconds: u64) -> Option { + if delay_seconds == 0 { + None + } else { + Some(Duration::from_secs( + if delay_seconds > MAX_GOAL_CONTINUATION_DELAY_SECONDS { + MAX_GOAL_CONTINUATION_DELAY_SECONDS + } else { + delay_seconds + }, + )) + } +} + +/// Wait out the between-continuation quiet period, honoring cancellation. +/// `None` resolves to `Elapsed` immediately so callers have a single dispatch +/// gate. Cancellation is biased and always wins over a racing expiry — the +/// same semantics as the interactive cadence (#5508) for host-managed turns, +/// where the turn loop is the only continuation dispatcher. +pub async fn await_continuation_wait( + wait: Option, + cancel_token: &tokio_util::sync::CancellationToken, +) -> ContinuationWaitOutcome { + let Some(wait) = wait else { + return ContinuationWaitOutcome::Elapsed; + }; + tokio::select! { + biased; + () = cancel_token.cancelled() => ContinuationWaitOutcome::Cancelled, + () = tokio::time::sleep(wait) => ContinuationWaitOutcome::Elapsed, + } +} + /// Whether the durable token usage has reached the active goal's budget. /// /// Budgets are telemetry-only in unbounded goal mode. Keeping this shared @@ -381,4 +430,61 @@ mod tests { ContinuationDecision::Stop(StopReason::Completed) ); } + + #[test] + fn continuation_wait_honors_configured_delay() { + assert_eq!( + continuation_wait(300), + Some(Duration::from_secs(300)), + "a positive configured delay must become the quiet-period wait" + ); + assert_eq!( + continuation_wait(MAX_GOAL_CONTINUATION_DELAY_SECONDS + 1), + Some(Duration::from_secs(MAX_GOAL_CONTINUATION_DELAY_SECONDS)), + "the shared cap must bound an oversized configured delay" + ); + } + + #[test] + fn zero_delay_continues_immediately() { + assert_eq!( + continuation_wait(0), + None, + "an unset or zero delay must dispatch immediately" + ); + } + + #[tokio::test] + async fn cancellation_wins_over_pending_quiet_period() { + let cancel_token = tokio_util::sync::CancellationToken::new(); + let canceller = cancel_token.clone(); + tokio::spawn(async move { + tokio::time::sleep(Duration::from_millis(10)).await; + canceller.cancel(); + }); + assert_eq!( + await_continuation_wait( + continuation_wait(MAX_GOAL_CONTINUATION_DELAY_SECONDS), + &cancel_token, + ) + .await, + ContinuationWaitOutcome::Cancelled, + "an explicit cancel during the quiet period must win and never dispatch" + ); + } + + #[tokio::test] + async fn elapsed_quiet_period_dispatches() { + let cancel_token = tokio_util::sync::CancellationToken::new(); + assert_eq!( + await_continuation_wait(None, &cancel_token).await, + ContinuationWaitOutcome::Elapsed, + "an unset wait must gate the dispatch through immediately" + ); + assert_eq!( + await_continuation_wait(Some(Duration::from_millis(1)), &cancel_token).await, + ContinuationWaitOutcome::Elapsed, + "an expired quiet period must dispatch" + ); + } } From 5f7bbb6fd0466ec9a077d786fb6b17b9d1031518 Mon Sep 17 00:00:00 2001 From: M-Maciej <130112810+M-Maciej@users.noreply.github.com> Date: Fri, 21 Aug 2026 11:29:58 +0000 Subject: [PATCH 2/8] fix(tui): honor goal continuation delay for host-injected tokens The v1 fix awaited the quiet period inside the intra-turn ladder (turn_loop.rs), which is not the dispatch path for host-managed sessions: the engine never calls schedule_goal_continuation there (gated on !host_managed_turns), and the host injects Op::ContinueGoal (engine_schedule_id None) into the engine mailbox instead. The run() arm for that token dispatched immediately with no delay. Now the Op::ContinueGoal arm awaits the shared cancellable quiet period for host-injected tokens before dispatching: biased cancellation (Esc/steer/host cancel) always wins over a racing expiry, and the live goal is re-read only after the wait so pause/clear/complete/blocked cancels the pass and failures never continue. Engine-owned tokens (engine_schedule_id Some) keep the scheduler's ready_at semantics untouched. Tests: host-injected ContinueGoal with delay>0 waits out the quiet period before the provider request, delay=0 dispatches immediately, and cancellation during the wait never dispatches. Signed-off-by: M-Maciej <130112810+M-Maciej@users.noreply.github.com> --- crates/tui/src/core/engine.rs | 30 +++- crates/tui/src/core/engine/tests.rs | 233 ++++++++++++++++++++++++++++ 2 files changed, 261 insertions(+), 2 deletions(-) diff --git a/crates/tui/src/core/engine.rs b/crates/tui/src/core/engine.rs index 8962916c6f..d42a32a76d 100644 --- a/crates/tui/src/core/engine.rs +++ b/crates/tui/src/core/engine.rs @@ -2410,9 +2410,35 @@ impl Engine { else { continue; }; + // Host-injected tokens carry their own quiet period: + // host-managed sessions never run the engine-owned + // scheduler, so this arm is their only continuation + // dispatch site and must honor + // [goal] continuation_delay_seconds itself before + // dispatching. The wait is biased-cancellable (Esc, + // steer, or host cancel always wins over a racing + // expiry), and the live goal is re-read below only + // after it, so a pause/clear/complete/blocked landing + // during the quiet period cancels the pass and + // failures never continue. Engine-owned tokens (Some) + // already waited out ready_at in the scheduler and + // keep those semantics untouched. + if engine_schedule_id.is_none() + && crate::goal_loop::await_continuation_wait( + crate::goal_loop::continuation_wait( + self.config.goal_continuation_delay_seconds, + ), + &self.cancel_token, + ) + .await + == crate::goal_loop::ContinuationWaitOutcome::Cancelled + { + continue; + } // Status controls queued while the previous turn was - // running are processed before this operation. Re-read - // the live goal now so pause/clear/complete/blocked can + // running are processed before this operation, and a + // host-injected quiet period has now elapsed. Re-read + // the live goal so pause/clear/complete/blocked can // cancel a stale continuation without starting a turn. let (content, goal_snapshot) = match self.goal_continuation_if_active() { GoalContinuationAction::Inactive => continue, diff --git a/crates/tui/src/core/engine/tests.rs b/crates/tui/src/core/engine/tests.rs index a5e77602da..0860d87df5 100644 --- a/crates/tui/src/core/engine/tests.rs +++ b/crates/tui/src/core/engine/tests.rs @@ -1890,6 +1890,239 @@ async fn goal_pause_during_configured_delay_cancels_pending_continuation() { .expect("engine task"); } +#[tokio::test] +async fn host_injected_goal_continuation_waits_out_the_quiet_period() { + // Host-managed sessions never run the engine-owned scheduler + // (schedule_goal_continuation is gated on !host_managed_turns), so the + // host injects ContinueGoal with engine_schedule_id None and this arm is + // the only dispatch site. A positive configured delay must be awaited + // before the provider request starts. + let model = std::sync::Arc::new(FailingGoalModelClient { + calls: std::sync::atomic::AtomicUsize::new(0), + message: "the host-injected continuation must wait first".to_string(), + }); + let config = goal_custom_route_config(); + let client: crate::core::model_client::SharedModelClient = model.clone(); + let (engine, handle) = Engine::new_with_model_client( + EngineConfig { + model: "local-model".to_string(), + snapshots_enabled: false, + terminal_chrome_enabled: false, + goal_objective: Some("wait out the cadence".to_string()), + goal_continuation_delay_seconds: 1, + ..EngineConfig::default() + }, + &config, + client, + ); + engine + .config + .goal_state + .lock() + .expect("goal lock") + .sync_from_host_status( + Some("wait out the cadence"), + None, + crate::tools::goal::GoalStatus::Active, + ); + let run_task = tokio::spawn(engine.run()); + + let queued_at = Instant::now(); + handle + .send(Op::ContinueGoal { + dynamic_tools: Vec::new(), + engine_schedule_id: None, + }) + .await + .expect("queue host-injected continuation"); + { + let mut events = handle.rx_event.write().await; + tokio::time::timeout(Duration::from_secs(3), async { + while let Some(event) = events.recv().await { + if matches!(event, Event::TurnStarted { .. }) { + break; + } + } + }) + .await + .expect("host-injected continuation did not dispatch after the quiet period"); + } + let elapsed = queued_at.elapsed(); + assert!( + elapsed >= Duration::from_millis(900), + "dispatch must wait out the configured quiet period, dispatched after {elapsed:?}" + ); + + let _ = tokio::time::timeout(model_turn_event_timeout(), handle.get_session_snapshot()) + .await + .expect("host-injected delayed turn did not settle") + .expect("session snapshot after delayed host-injected dispatch"); + assert_eq!( + model.calls.load(std::sync::atomic::Ordering::SeqCst), + 1, + "the host-injected token must dispatch exactly one provider request" + ); + + handle.send(Op::Shutdown).await.expect("shutdown engine"); + tokio::time::timeout(model_turn_event_timeout(), run_task) + .await + .expect("engine did not shut down after host-injected delay") + .expect("engine task"); +} + +#[tokio::test] +async fn host_injected_goal_continuation_with_zero_delay_dispatches_immediately() { + let model = std::sync::Arc::new(FailingGoalModelClient { + calls: std::sync::atomic::AtomicUsize::new(0), + message: "zero-delay host-injected dispatch proof".to_string(), + }); + let config = goal_custom_route_config(); + let client: crate::core::model_client::SharedModelClient = model.clone(); + let (engine, handle) = Engine::new_with_model_client( + EngineConfig { + model: "local-model".to_string(), + snapshots_enabled: false, + terminal_chrome_enabled: false, + goal_objective: Some("dispatch without a cadence".to_string()), + goal_continuation_delay_seconds: 0, + ..EngineConfig::default() + }, + &config, + client, + ); + engine + .config + .goal_state + .lock() + .expect("goal lock") + .sync_from_host_status( + Some("dispatch without a cadence"), + None, + crate::tools::goal::GoalStatus::Active, + ); + let run_task = tokio::spawn(engine.run()); + + handle + .send(Op::ContinueGoal { + dynamic_tools: Vec::new(), + engine_schedule_id: None, + }) + .await + .expect("queue zero-delay host-injected continuation"); + { + let mut events = handle.rx_event.write().await; + tokio::time::timeout(Duration::from_secs(1), async { + while let Some(event) = events.recv().await { + if matches!(event, Event::TurnStarted { .. }) { + break; + } + } + }) + .await + .expect("a zero-delay host-injected continuation must dispatch immediately"); + } + + let _ = tokio::time::timeout(model_turn_event_timeout(), handle.get_session_snapshot()) + .await + .expect("zero-delay host-injected turn did not settle") + .expect("session snapshot after zero-delay dispatch"); + assert_eq!( + model.calls.load(std::sync::atomic::Ordering::SeqCst), + 1, + "zero delay must not suppress the host-injected dispatch" + ); + + handle.send(Op::Shutdown).await.expect("shutdown engine"); + tokio::time::timeout(model_turn_event_timeout(), run_task) + .await + .expect("engine did not shut down after zero-delay dispatch") + .expect("engine task"); +} + +#[tokio::test] +async fn cancellation_during_host_injected_continuation_wait_never_dispatches() { + let model = std::sync::Arc::new(FailingGoalModelClient { + calls: std::sync::atomic::AtomicUsize::new(0), + message: "a cancelled host-injected wait must never start".to_string(), + }); + let config = goal_custom_route_config(); + let client: crate::core::model_client::SharedModelClient = model.clone(); + let (engine, handle) = Engine::new_with_model_client( + EngineConfig { + model: "local-model".to_string(), + snapshots_enabled: false, + terminal_chrome_enabled: false, + goal_objective: Some("cancel mid-cadence".to_string()), + goal_continuation_delay_seconds: 1, + ..EngineConfig::default() + }, + &config, + client, + ); + engine + .config + .goal_state + .lock() + .expect("goal lock") + .sync_from_host_status( + Some("cancel mid-cadence"), + None, + crate::tools::goal::GoalStatus::Active, + ); + let run_task = tokio::spawn(engine.run()); + + handle + .send(Op::ContinueGoal { + dynamic_tools: Vec::new(), + engine_schedule_id: None, + }) + .await + .expect("queue host-injected continuation"); + // Enter the quiet period, then cancel: the biased wait must drop the + // pending pass instead of dispatching when the period would have elapsed. + tokio::time::sleep(Duration::from_millis(100)).await; + handle.cancel(); + + // A dispatch bug would surface a TurnStarted once the 1s period expires; + // a correct cancellation settles back into the mailbox loop silently. + let dispatched = { + let mut events = handle.rx_event.write().await; + tokio::time::timeout(Duration::from_secs(2), async { + loop { + let Some(event) = events.recv().await else { + break false; + }; + if matches!(event, Event::TurnStarted { .. }) { + break true; + } + } + }) + .await + .unwrap_or(false) + }; + assert!( + !dispatched, + "cancellation during the host-injected quiet period must never dispatch" + ); + + // The engine must keep accepting controls after the cancelled wait. + let _ = tokio::time::timeout(model_turn_event_timeout(), handle.get_session_snapshot()) + .await + .expect("engine did not accept controls after the cancelled wait") + .expect("session snapshot after cancelled wait"); + assert_eq!( + model.calls.load(std::sync::atomic::Ordering::SeqCst), + 0, + "the cancelled host-injected token must never reach the provider" + ); + + handle.send(Op::Shutdown).await.expect("shutdown engine"); + tokio::time::timeout(model_turn_event_timeout(), run_task) + .await + .expect("engine did not shut down after cancelled wait") + .expect("engine task"); +} + #[tokio::test] async fn queued_not_started_turn_cancels_older_goal_continuation() { let objective = "stop when the queued turn cannot start"; From b207ee03f1d4b6958a06bc7629511d0e9f2e7d01 Mon Sep 17 00:00:00 2001 From: M-Maciej <130112810+M-Maciej@users.noreply.github.com> Date: Fri, 21 Aug 2026 11:31:17 +0000 Subject: [PATCH 3/8] fix(tui): honor goal-continuation quiet period for non-host-managed sessions goal_continuation_message_if_needed gated the between-continuation quiet period on host_managed_turns(); the else-None branch assumed the cross-turn scheduler covered non-host-managed sessions, so a session resumed via `codewhale resume --last` (runtime_services.active_thread_id None) dispatched goal continuation instantly. This within-turn hook is the only dispatch site, so the wait is now unconditional: continuation_wait(goal_continuation_delay_seconds) for every session, keeping the biased-cancellable await, the post-wait re-decision on live state, and the GoalContinuationWaiting/WaitEnded events unchanged. Tests (turn_loop): a non-host-managed engine with a positive delay emits the wait receipt, does not dispatch before the quiet period elapses, and dispatches (one recorded continuation) afterwards; a zero delay still continues immediately with no wait receipt; a host-managed engine keeps the same wait contract. Signed-off-by: M-Maciej <130112810+M-Maciej@users.noreply.github.com> --- crates/tui/src/core/engine/turn_loop.rs | 172 ++++++++++++++++++++++-- 1 file changed, 162 insertions(+), 10 deletions(-) diff --git a/crates/tui/src/core/engine/turn_loop.rs b/crates/tui/src/core/engine/turn_loop.rs index 1b6a81bccc..3e014d655d 100644 --- a/crates/tui/src/core/engine/turn_loop.rs +++ b/crates/tui/src/core/engine/turn_loop.rs @@ -4421,16 +4421,15 @@ impl Engine { .await .as_ref()?; - // Host-managed turns have no cross-turn scheduler, so the configured - // between-continuation quiet period is awaited right here. The wait is - // cancellable: the cancel token (Esc) wins biased over the timer, and - // a pause/clear or terminal update_goal observed after the wait - // cancels the pending pass before anything is recorded or dispatched. - let wait = if self.host_managed_turns() { - crate::goal_loop::continuation_wait(self.config.goal_continuation_delay_seconds) - } else { - None - }; + // This within-turn hook is the only goal-continuation dispatch site + // for every session, so the configured between-continuation quiet + // period is awaited right here unconditionally — non-host-managed + // sessions (e.g. `codewhale resume --last`) must honor the delay too. + // The wait is cancellable: the cancel token (Esc) wins biased over the + // timer, and a pause/clear or terminal update_goal observed after the + // wait cancels the pending pass before anything is recorded or + // dispatched. + let wait = crate::goal_loop::continuation_wait(self.config.goal_continuation_delay_seconds); let was_delayed = wait.is_some(); if let Some(wait) = wait { let _ = self @@ -6605,4 +6604,157 @@ mod tests { assert!(fold.deny_reason.is_none()); assert!(fold.requires_approval); } + + // ── Goal continuation quiet period ─────────────────────────────── + + /// Engine fixture for the continuation-hook cadence tests. A non-empty + /// `goal_objective` with the default `Active` status leaves an active goal + /// in the shared state after `Engine::new`, so the within-turn hook has a + /// live goal to continue. `host_managed` sets `active_thread_id`, the flag + /// the hook previously used to decide whether to wait at all. + fn goal_continuation_cadence_engine( + tmp: &tempfile::TempDir, + delay_seconds: u64, + host_managed: bool, + ) -> (Engine, EngineHandle) { + let config = EngineConfig { + workspace: tmp.path().to_path_buf(), + goal_objective: Some("keep going".to_string()), + goal_continuation_delay_seconds: delay_seconds, + runtime_services: crate::tools::spec::RuntimeToolServices { + active_thread_id: host_managed.then(|| "host-managed-thread".to_string()), + ..Default::default() + }, + ..Default::default() + }; + Engine::new(config, &Config::default()) + } + + fn goal_continuation_registry(engine: &Engine) -> crate::tools::ToolRegistry { + crate::tools::ToolRegistryBuilder::new() + .with_goal_tools(engine.config.goal_state.clone()) + .build(crate::tools::spec::ToolContext::new( + engine.config.workspace.clone(), + )) + } + + /// Drive the within-turn hook on an engine whose configured quiet period + /// is positive, asserting the full dispatch contract: the hook emits its + /// wait receipt before dispatching, does not dispatch before the quiet + /// period elapses, and does dispatch (recording one continuation) after. + async fn assert_positive_delay_continuation_waits( + engine: Engine, + handle: EngineHandle, + delay_seconds: u64, + ) { + let registry = goal_continuation_registry(&engine); + let mut task = tokio::spawn(async move { + let mut continuations = 0u32; + let usage = Usage::default(); + let message = engine + .goal_continuation_message_if_needed(Some(®istry), &mut continuations, &usage) + .await; + (message, continuations) + }); + + // The wait receipt must arrive before anything is dispatched. If the + // hook skips the wait, it returns without one and the task finishes. + let mut events = handle.rx_event.write().await; + loop { + let event = tokio::select! { + event = events.recv() => event, + finished = &mut task => { + panic!( + "goal continuation dispatched before the quiet period: {finished:?}" + ); + } + }; + match event { + Some(Event::GoalContinuationWaiting { + delay_seconds: emitted, + }) => { + assert_eq!( + emitted, delay_seconds, + "wait receipt must carry the configured delay" + ); + break; + } + Some(_) => continue, + None => panic!("event channel closed before the continuation wait receipt"), + } + } + assert!( + !task.is_finished(), + "continuation must still be inside the quiet period after the wait receipt" + ); + + let started = std::time::Instant::now(); + let (message, continuations) = task.await.expect("continuation task panicked"); + let waited = started.elapsed(); + assert!( + waited >= Duration::from_millis(delay_seconds.saturating_mul(1000).saturating_sub(100)), + "continuation dispatched after only {waited:?}; the {delay_seconds}s quiet period was not honored" + ); + assert!( + message.is_some(), + "active goal must dispatch a continuation prompt after the quiet period" + ); + assert_eq!(continuations, 1); + } + + /// Regression: a CLI-resumed (non-host-managed) session has + /// `runtime_services.active_thread_id` unset and must still honor the + /// between-continuation quiet period before dispatching. + #[tokio::test] + async fn non_host_managed_goal_continuation_waits_for_quiet_period() { + let tmp = tempdir().expect("tempdir"); + let (engine, handle) = goal_continuation_cadence_engine(&tmp, 1, false); + assert_eq!( + engine.config.runtime_services.active_thread_id, None, + "fixture must be non-host-managed" + ); + assert_positive_delay_continuation_waits(engine, handle, 1).await; + } + + /// Host-managed sessions keep their existing cadence: the quiet period + /// still elapses before the continuation prompt dispatches. + #[tokio::test] + async fn host_managed_goal_continuation_still_waits_for_quiet_period() { + let tmp = tempdir().expect("tempdir"); + let (engine, handle) = goal_continuation_cadence_engine(&tmp, 1, true); + assert!( + engine.config.runtime_services.active_thread_id.is_some(), + "fixture must be host-managed" + ); + assert_positive_delay_continuation_waits(engine, handle, 1).await; + } + + /// A zero delay must continue immediately: no wait receipt is emitted and + /// the continuation prompt dispatches without any quiet period. + #[tokio::test] + async fn zero_goal_continuation_delay_dispatches_immediately() { + let tmp = tempdir().expect("tempdir"); + let (engine, handle) = goal_continuation_cadence_engine(&tmp, 0, false); + let registry = goal_continuation_registry(&engine); + let task = tokio::spawn(async move { + let mut continuations = 0u32; + let usage = Usage::default(); + let message = engine + .goal_continuation_message_if_needed(Some(®istry), &mut continuations, &usage) + .await; + (message, continuations) + }); + + let (message, continuations) = task.await.expect("continuation task panicked"); + assert!(message.is_some(), "zero delay must still continue the goal"); + assert_eq!(continuations, 1); + + let mut events = handle.rx_event.write().await; + while let Ok(event) = events.try_recv() { + assert!( + !matches!(event, Event::GoalContinuationWaiting { .. }), + "zero delay must not enter the quiet-period wait, got {event:?}" + ); + } + } } From 207c3e6d05a63150d55d42512455d4d742d4d745 Mon Sep 17 00:00:00 2001 From: M-Maciej <130112810+M-Maciej@users.noreply.github.com> Date: Fri, 21 Aug 2026 11:22:18 +0000 Subject: [PATCH 4/8] test(tui): neutralize SSH markers in the terminal-motion tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Production intentionally caps terminal motion over SSH (in_ssh_session -> low_motion), and four tests assert the un-capped state without neutralizing SSH_CLIENT/SSH_TTY — deterministic failures whenever the suite runs over SSH (the normal test-bed pattern). Each test now removes SSH_CLIENT, SSH_CONNECTION, and SSH_TTY via EnvVarGuard under the env lock, restoring them on drop. Signed-off-by: M-Maciej <130112810+M-Maciej@users.noreply.github.com> --- crates/tui/src/commands/groups/config/config.rs | 5 +++++ crates/tui/src/settings.rs | 15 +++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/crates/tui/src/commands/groups/config/config.rs b/crates/tui/src/commands/groups/config/config.rs index e111384c9c..a68717d2ef 100644 --- a/crates/tui/src/commands/groups/config/config.rs +++ b/crates/tui/src/commands/groups/config/config.rs @@ -3696,6 +3696,11 @@ mod tests { )); fs::create_dir_all(&temp_root).unwrap(); let _guard = EnvGuard::new(&temp_root); + // Neutralize the SSH markers: production intentionally caps motion + // over SSH, and the suite routinely runs inside one. + let _ssh_client = EnvVarGuard::remove("SSH_CLIENT"); + let _ssh_connection = EnvVarGuard::remove("SSH_CONNECTION"); + let _ssh_tty = EnvVarGuard::remove("SSH_TTY"); let prev_term_program = env::var_os("TERM_PROGRAM"); // Safety: test-only environment mutation guarded by EnvGuard's lock. unsafe { diff --git a/crates/tui/src/settings.rs b/crates/tui/src/settings.rs index aba868fb6a..a2f47660e8 100644 --- a/crates/tui/src/settings.rs +++ b/crates/tui/src/settings.rs @@ -4065,6 +4065,11 @@ mod tests { #[test] fn ghostty_term_program_keeps_full_motion_without_the_legacy_30_fps_cap() { let _g = term_program_test_guard(); + // Neutralize the SSH markers: production intentionally caps motion + // over SSH, and the suite routinely runs inside one. + let _ssh_client = crate::test_support::EnvVarGuard::remove("SSH_CLIENT"); + let _ssh_connection = crate::test_support::EnvVarGuard::remove("SSH_CONNECTION"); + let _ssh_tty = crate::test_support::EnvVarGuard::remove("SSH_TTY"); let prev = std::env::var_os("TERM_PROGRAM"); // SAFETY: serialised by the guard. unsafe { @@ -4088,6 +4093,11 @@ mod tests { #[test] fn ghostty_term_fallback_keeps_full_motion_without_the_legacy_30_fps_cap() { let _g = term_program_test_guard(); + // Neutralize the SSH markers: production intentionally caps motion + // over SSH, and the suite routinely runs inside one. + let _ssh_client = crate::test_support::EnvVarGuard::remove("SSH_CLIENT"); + let _ssh_connection = crate::test_support::EnvVarGuard::remove("SSH_CONNECTION"); + let _ssh_tty = crate::test_support::EnvVarGuard::remove("SSH_TTY"); let prev_program = std::env::var_os("TERM_PROGRAM"); let prev_term = std::env::var_os("TERM"); // SAFETY: serialised by the guard. @@ -4183,6 +4193,11 @@ mod tests { #[test] fn tilix_and_terminator_cap_redraws_without_disabling_motion() { let _g = term_program_test_guard(); + // Neutralize the SSH markers: production intentionally caps motion + // over SSH, and the suite routinely runs inside one. + let _ssh_client = crate::test_support::EnvVarGuard::remove("SSH_CLIENT"); + let _ssh_connection = crate::test_support::EnvVarGuard::remove("SSH_CONNECTION"); + let _ssh_tty = crate::test_support::EnvVarGuard::remove("SSH_TTY"); let prev_term_program = std::env::var_os("TERM_PROGRAM"); let prev_tilix_id = std::env::var_os("TILIX_ID"); let prev_terminator_uuid = std::env::var_os("TERMINATOR_UUID"); From b6fbeec22082348aa5a4176ca034710cc5a1399e Mon Sep 17 00:00:00 2001 From: M-Maciej <130112810+M-Maciej@users.noreply.github.com> Date: Fri, 21 Aug 2026 11:22:24 +0000 Subject: [PATCH 5/8] test(cli): preserve RUSTUP_HOME in the read-only diagnostic test On hosts with a system-wide rustup (cargo/rustc are rustup proxies), the env-cleared doctor probe defaults RUSTUP_HOME to the sealed fixture HOME and materializes toolchain state there, failing the read-only assertion. Mirror the TUI integration test's preserve_host_rustup_home guard. Signed-off-by: M-Maciej <130112810+M-Maciej@users.noreply.github.com> --- .../tests/diagnostic_dispatch_read_only.rs | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/crates/cli/tests/diagnostic_dispatch_read_only.rs b/crates/cli/tests/diagnostic_dispatch_read_only.rs index 6e70dabb5e..e8c0eae543 100644 --- a/crates/cli/tests/diagnostic_dispatch_read_only.rs +++ b/crates/cli/tests/diagnostic_dispatch_read_only.rs @@ -49,15 +49,16 @@ fn dispatcher_diagnostics_are_in_process_and_read_only() { // dispatches through `run_tui_in_process` -> `codewhale_tui::run`. No // `DEEPSEEK_TUI_BIN` sibling is spawned, so there is no receipt to read; // assert the in-process behavior and the read-only invariants instead. - let output = Command::new(codewhale_binary()) + let mut command = Command::new(codewhale_binary()); + command .args(args) .env_clear() .env("HOME", &sealed_home) .env("USERPROFILE", &sealed_home) .env("CODEWHALE_HOME", &codewhale_home) - .env("CODEWHALE_SECRET_BACKEND", "file") - .output() - .expect("run dispatcher diagnostic"); + .env("CODEWHALE_SECRET_BACKEND", "file"); + preserve_host_rustup_home(&mut command); + let output = command.output().expect("run dispatcher diagnostic"); assert!( output.status.success(), @@ -189,3 +190,20 @@ fn codewhale_binary() -> PathBuf { path.push(format!("codewhale{}", std::env::consts::EXE_SUFFIX)); path } + +/// A rustup shim may initialize its own toolchain state below `$HOME` when +/// `doctor` asks `rustc --version`. Preserve an already-configured toolchain +/// root so this test isolates Codewhale's own state contract. +fn preserve_host_rustup_home(command: &mut Command) { + let rustup_home = std::env::var_os("RUSTUP_HOME") + .map(PathBuf::from) + .or_else(|| { + std::env::var_os("HOME") + .map(PathBuf::from) + .map(|home| home.join(".rustup")) + .filter(|path| path.is_dir()) + }); + if let Some(rustup_home) = rustup_home { + command.env("RUSTUP_HOME", rustup_home); + } +} From 5d1be303492fe989f62ec12f645b3162f6b8fc36 Mon Sep 17 00:00:00 2001 From: M-Maciej <130112810+M-Maciej@users.noreply.github.com> Date: Fri, 21 Aug 2026 11:33:01 +0000 Subject: [PATCH 6/8] =?UTF-8?q?test(tui):=20harden=20route=5Fbudget=20para?= =?UTF-8?q?llel=20flake=20=E2=80=94=20env-barrier=20isolation=20for=20V4?= =?UTF-8?q?=20trigger=20and=20prompt=20prefix-leak=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit route_budget::tests::v4_trigger_uses_window_percent_when_it_fits_spendable_input asserts no-override output-budget values but read the process-global CODEWHALE_MAX_OUTPUT_TOKENS/DEEPSEEK_MAX_OUTPUT_TOKENS without holding lock_test_env(), so a concurrent sibling writer could flip the value mid-assertion. The test now holds the barrier and removes both overrides, matching every sibling test with the same dependency; assertions unchanged. Full-suite verification under --test-threads=1 exposed the same class of bug in prompts::tests::system_prompt_prefix_never_leaks_private_content: it read the real home via HOME/USERPROFILE, so ~/.codewhale/instructions.md leaked its absolute path into the prompt and failed the no-private-paths assertion unless a sibling's temporary HOME guard happened to be live. It now holds the barrier and pins HOME/USERPROFILE to a scratch dir. Assertions unchanged. Verified: 4 consecutive full cargo test -p codewhale-tui --lib runs green (10891 passed / 0 failed / 13 ignored each; 2 x default threads, 2 x --test-threads=1); fmt clean; clippy clean. Signed-off-by: M-Maciej <130112810+M-Maciej@users.noreply.github.com> --- CHANGELOG.md | 17 ++++++++ crates/tui/src/prompts.rs | 14 +++++++ crates/tui/src/route_budget.rs | 11 +++++ docs/changelog/changelog-0076.md | 70 ++++++++++++++++++++++++++++++++ 4 files changed, 112 insertions(+) create mode 100644 docs/changelog/changelog-0076.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 335d6d6527..2498ec7906 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -785,6 +785,23 @@ erase behavior, migration, security, compatibility, or verification details. restating it never changed anything. Shipped binaries were never affected; `release-artifacts.yml` builds `--profile dist` with fat LTO and `codegen-units = 1`. +- Two unit tests no longer depend on process-global environment state that + sibling tests mutate concurrently. + `route_budget::tests::v4_trigger_uses_window_percent_when_it_fits_spendable_input` + asserted no-override output-budget values while reading + `CODEWHALE_MAX_OUTPUT_TOKENS` / `DEEPSEEK_MAX_OUTPUT_TOKENS` without + holding `lock_test_env()`, so a concurrent writer could flip the value + mid-assertion (the order-dependent full-suite flake). + `prompts::tests::system_prompt_prefix_never_leaks_private_content` read the + real home via `HOME`/`USERPROFILE`, so a machine with + `~/.codewhale/instructions.md` leaked its absolute path into the prompt and + failed the no-private-paths assertion unless a sibling's temporary `HOME` + guard happened to be live. Both tests now hold the env barrier and pin the + variables (the route-budget test removes the overrides; the prompts test + points `HOME`/`USERPROFILE` at a scratch dir). Assertions unchanged. + Four consecutive full `cargo test -p codewhale-tui --lib` runs green + (10891 passed / 0 failed / 13 ignored each), covering default and + `--test-threads=1` modes. - Test debt: the transcript history-cell suite has been rebuilt. It was 123 tests across 3,964 lines, and about a third of it pinned the current skin diff --git a/crates/tui/src/prompts.rs b/crates/tui/src/prompts.rs index 0542201126..2dd6cc719f 100644 --- a/crates/tui/src/prompts.rs +++ b/crates/tui/src/prompts.rs @@ -3480,9 +3480,23 @@ mod tests { /// #4632 — The system prompt prefix (the byte-stable part cached by /// inference servers) must never contain private content: absolute /// filesystem paths, API keys, or home-directory references. + /// + /// Pin `HOME`/`USERPROFILE` to a scratch dir (and hold the env barrier) + /// so global `~/.codewhale/instructions.md` and home-resolved skills + /// cannot leak their real absolute paths into the prompt under test. + /// Without this a machine that has `~/.codewhale/instructions.md` fails + /// the absolute-path assertion, and the test only passes in parallel + /// runs when a sibling test happens to hold a temporary `HOME` guard at + /// the same moment — process-global env, so the result must never + /// depend on scheduling or the developer's machine. #[test] fn system_prompt_prefix_never_leaks_private_content() { + let _env_guard = crate::test_support::lock_test_env(); let tmp = tempdir().expect("tempdir"); + let home_tmp = tempdir().expect("home tempdir"); + let _home = EnvVarGuard::set("HOME", home_tmp.path().as_os_str()); + let _userprofile = EnvVarGuard::set("USERPROFILE", home_tmp.path().as_os_str()); + let _skills_dir = EnvVarGuard::remove("DEEPSEEK_SKILLS_DIR"); let workspace = tmp.path(); let prompt = match system_prompt_for_mode_with_context(workspace, None) { SystemPrompt::Text(text) => text, diff --git a/crates/tui/src/route_budget.rs b/crates/tui/src/route_budget.rs index 5b1651d695..7f2f1a46ac 100644 --- a/crates/tui/src/route_budget.rs +++ b/crates/tui/src/route_budget.rs @@ -517,8 +517,19 @@ mod tests { )); } + /// The assertion values here depend on `explicit_max_output_tokens_override` + /// seeing no ambient env override, and sibling tests in this binary + /// (this module, `client`, `vision/tools`, `core/engine`) set + /// `CODEWHALE_MAX_OUTPUT_TOKENS`/`DEEPSEEK_MAX_OUTPUT_TOKENS` while holding + /// `lock_test_env`. Without the lock and guards this test could read a + /// concurrent writer's value mid-assertion (process-global env, parallel + /// threads), which is the order-dependent flake this guards against. #[test] fn v4_trigger_uses_window_percent_when_it_fits_spendable_input() { + let _lock = crate::test_support::lock_test_env(); + let _codewhale = crate::test_support::EnvVarGuard::remove("CODEWHALE_MAX_OUTPUT_TOKENS"); + let _deepseek = crate::test_support::EnvVarGuard::remove("DEEPSEEK_MAX_OUTPUT_TOKENS"); + let budget = route_context_budget(ApiProvider::Deepseek, "deepseek-v4-pro", None, 0) .expect("V4 route budget"); diff --git a/docs/changelog/changelog-0076.md b/docs/changelog/changelog-0076.md new file mode 100644 index 0000000000..6865a45a11 --- /dev/null +++ b/docs/changelog/changelog-0076.md @@ -0,0 +1,70 @@ +# changelog-0076 — harden route_budget parallel test flake + +Date: 2026-08-20 + +## Summary + +Harden two unit tests in `crates/tui` whose outcome depended on +process-global environment state that sibling tests mutate concurrently: + +- `route_budget::tests::v4_trigger_uses_window_percent_when_it_fits_spendable_input` +- `prompts::tests::system_prompt_prefix_never_leaks_private_content` + +Both fixes are test-only environment isolation; no assertion was weakened or +changed, and no production code changed. + +## Root cause + +`route_context_budget(ApiProvider::Deepseek, "deepseek-v4-pro", None, 0)` +resolves the route output reservation through +`explicit_max_output_tokens_override()`, which reads the process-global +`CODEWHALE_MAX_OUTPUT_TOKENS` / `DEEPSEEK_MAX_OUTPUT_TOKENS` environment +variables. The V4 test asserts the no-override values (output cap 65 536, +input ceiling 933 440, trigger 800 000) but never took `lock_test_env()` and +never pinned those variables. Sibling tests in the same binary (this module, +`client`, `vision/tools`, `core/engine`) set those variables while holding +the env barrier; because the V4 test did not participate in that barrier, a +concurrent writer could flip the value between the test's reads — the +order-dependent flake seen once in a full parallel run (passes in isolation, +passes on reruns). + +The same class of bug surfaced during `--test-threads=1` verification in +`system_prompt_prefix_never_leaks_private_content`: the prompt is built with +`effective_home_dir()`, which reads process-global `HOME`/`USERPROFILE`. +The test never isolated those, so on a machine with +`~/.codewhale/instructions.md` the global instructions block leaked its real +absolute path into the prompt and failed the absolute-path assertion. The +test only passed in parallel runs when a sibling test's temporary `HOME` +guard happened to be live at the same moment; serialized, it failed +deterministically. + +## Fix + +- `crates/tui/src/route_budget.rs`: the V4 trigger test now holds + `lock_test_env()` and removes `CODEWHALE_MAX_OUTPUT_TOKENS` and + `DEEPSEEK_MAX_OUTPUT_TOKENS` for its duration — the established pattern + used by every sibling test with the same dependency. Assertions unchanged. +- `crates/tui/src/prompts.rs`: the prefix-leak test now holds + `lock_test_env()` and pins `HOME`/`USERPROFILE` to a scratch directory + (and removes `DEEPSEEK_SKILLS_DIR`), matching the neighboring + byte-stability tests, so global instructions and home-resolved skills + cannot leak real absolute paths. Assertions unchanged. + +## Verification + +Full suite, `cargo test -p codewhale-tui --lib` (10 904 tests): + +- Run A, `--test-threads=1`: 10891 passed / 0 failed / 13 ignored +- Run B, `--test-threads=1`: 10891 passed / 0 failed / 13 ignored +- Run C, default threads: 10891 passed / 0 failed / 13 ignored +- Run D, default threads: 10891 passed / 0 failed / 13 ignored + +Four consecutive full-suite runs green, covering both thread modes. Before +the prompts fix, a serialized run reproduced the second failure +deterministically (1 failed, `system_prompt_prefix_never_leaks_private_content`); +after both fixes that run mode is green. + +- `cargo fmt --all -- --check`: clean. +- `cargo clippy -p codewhale-tui --lib`: clean (exit 0). +- Focused checks: `route_budget::tests::v4_trigger_uses_window_percent_when_it_fits_spendable_input` + and the full `prompts::` module (109/109) green in isolation. From f9212bbb6b6d9e727545918c470d98849d0d624b Mon Sep 17 00:00:00 2001 From: M-Maciej <130112810+M-Maciej@users.noreply.github.com> Date: Sun, 23 Aug 2026 11:38:53 +0000 Subject: [PATCH 7/8] docs: changelog entries for the goal-continuation quiet-period fix (#5534) and the flake hardening Move the flake-hardening entry out of the released 0.9.11 section into Unreleased (0.9.11 already shipped without it) and add the #5534 entry. Regenerated crates/tui/CHANGELOG.md via scripts/sync-changelog.sh. Signed-off-by: M-Maciej <130112810+M-Maciej@users.noreply.github.com> --- CHANGELOG.md | 41 ++++++++++++++++++++++++----------------- crates/tui/CHANGELOG.md | 24 ++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2498ec7906..09cb57124d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +- The goal-continuation quiet period (`[goal] continuation_delay_seconds`, + added in #5508) now applies on every dispatch path. Previously the + within-turn dispatch hook fired the next continuation prompt immediately + whenever a model step ended with the goal still active, so CLI-resumed and + host-managed sessions never observed the configured delay (measured + end-to-start gaps of ~9 s with a 300 s delay configured). The wait now + runs unconditionally inside `goal_continuation_message_if_needed`, + remains cancellable, and pause/clear/terminal `update_goal` calls still + cancel a pending pass (#5534). +- Two unit tests no longer depend on process-global environment state that + sibling tests mutate concurrently. + `route_budget::tests::v4_trigger_uses_window_percent_when_it_fits_spendable_input` + asserted no-override output-budget values while reading + `CODEWHALE_MAX_OUTPUT_TOKENS` / `DEEPSEEK_MAX_OUTPUT_TOKENS` without + holding `lock_test_env()`, so a concurrent writer could flip the value + mid-assertion (the order-dependent full-suite flake). + `prompts::tests::system_prompt_prefix_never_leaks_private_content` read the + real home via `HOME`/`USERPROFILE`, so a machine with + `~/.codewhale/instructions.md` leaked its absolute path into the prompt and + failed the no-private-paths assertion unless a sibling's temporary `HOME` + guard happened to be live. Both tests now hold the env barrier and pin the + variables (the route-budget test removes the overrides; the prompts test + points `HOME`/`USERPROFILE` at a scratch dir). Assertions unchanged. + ## [0.9.11] - 2026-08-22 Codewhale v0.9.11 tightens the long-running agent loop, makes workflow @@ -785,23 +809,6 @@ erase behavior, migration, security, compatibility, or verification details. restating it never changed anything. Shipped binaries were never affected; `release-artifacts.yml` builds `--profile dist` with fat LTO and `codegen-units = 1`. -- Two unit tests no longer depend on process-global environment state that - sibling tests mutate concurrently. - `route_budget::tests::v4_trigger_uses_window_percent_when_it_fits_spendable_input` - asserted no-override output-budget values while reading - `CODEWHALE_MAX_OUTPUT_TOKENS` / `DEEPSEEK_MAX_OUTPUT_TOKENS` without - holding `lock_test_env()`, so a concurrent writer could flip the value - mid-assertion (the order-dependent full-suite flake). - `prompts::tests::system_prompt_prefix_never_leaks_private_content` read the - real home via `HOME`/`USERPROFILE`, so a machine with - `~/.codewhale/instructions.md` leaked its absolute path into the prompt and - failed the no-private-paths assertion unless a sibling's temporary `HOME` - guard happened to be live. Both tests now hold the env barrier and pin the - variables (the route-budget test removes the overrides; the prompts test - points `HOME`/`USERPROFILE` at a scratch dir). Assertions unchanged. - Four consecutive full `cargo test -p codewhale-tui --lib` runs green - (10891 passed / 0 failed / 13 ignored each), covering default and - `--test-threads=1` modes. - Test debt: the transcript history-cell suite has been rebuilt. It was 123 tests across 3,964 lines, and about a third of it pinned the current skin diff --git a/crates/tui/CHANGELOG.md b/crates/tui/CHANGELOG.md index 43cb061e80..d4cd18dda3 100644 --- a/crates/tui/CHANGELOG.md +++ b/crates/tui/CHANGELOG.md @@ -7,6 +7,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +- The goal-continuation quiet period (`[goal] continuation_delay_seconds`, + added in #5508) now applies on every dispatch path. Previously the + within-turn dispatch hook fired the next continuation prompt immediately + whenever a model step ended with the goal still active, so CLI-resumed and + host-managed sessions never observed the configured delay (measured + end-to-start gaps of ~9 s with a 300 s delay configured). The wait now + runs unconditionally inside `goal_continuation_message_if_needed`, + remains cancellable, and pause/clear/terminal `update_goal` calls still + cancel a pending pass (#5534). +- Two unit tests no longer depend on process-global environment state that + sibling tests mutate concurrently. + `route_budget::tests::v4_trigger_uses_window_percent_when_it_fits_spendable_input` + asserted no-override output-budget values while reading + `CODEWHALE_MAX_OUTPUT_TOKENS` / `DEEPSEEK_MAX_OUTPUT_TOKENS` without + holding `lock_test_env()`, so a concurrent writer could flip the value + mid-assertion (the order-dependent full-suite flake). + `prompts::tests::system_prompt_prefix_never_leaks_private_content` read the + real home via `HOME`/`USERPROFILE`, so a machine with + `~/.codewhale/instructions.md` leaked its absolute path into the prompt and + failed the no-private-paths assertion unless a sibling's temporary `HOME` + guard happened to be live. Both tests now hold the env barrier and pin the + variables (the route-budget test removes the overrides; the prompts test + points `HOME`/`USERPROFILE` at a scratch dir). Assertions unchanged. + ## [0.9.11] - 2026-08-22 Codewhale v0.9.11 tightens the long-running agent loop, makes workflow From c2629bacafac65349efc4373b01a885d5c3d39d3 Mon Sep 17 00:00:00 2001 From: M-Maciej <130112810+M-Maciej@users.noreply.github.com> Date: Sun, 23 Aug 2026 18:06:22 +0000 Subject: [PATCH 8/8] docs: drop the internal session changelog from the upstream pack docs/changelog/changelog-0076.md is an ecosystem-internal per-session build log (session-numbered convention); upstream keeps its changelog in the root CHANGELOG.md, where the flake-hardening entry already lives. Signed-off-by: M-Maciej <130112810+M-Maciej@users.noreply.github.com> --- docs/changelog/changelog-0076.md | 70 -------------------------------- 1 file changed, 70 deletions(-) delete mode 100644 docs/changelog/changelog-0076.md diff --git a/docs/changelog/changelog-0076.md b/docs/changelog/changelog-0076.md deleted file mode 100644 index 6865a45a11..0000000000 --- a/docs/changelog/changelog-0076.md +++ /dev/null @@ -1,70 +0,0 @@ -# changelog-0076 — harden route_budget parallel test flake - -Date: 2026-08-20 - -## Summary - -Harden two unit tests in `crates/tui` whose outcome depended on -process-global environment state that sibling tests mutate concurrently: - -- `route_budget::tests::v4_trigger_uses_window_percent_when_it_fits_spendable_input` -- `prompts::tests::system_prompt_prefix_never_leaks_private_content` - -Both fixes are test-only environment isolation; no assertion was weakened or -changed, and no production code changed. - -## Root cause - -`route_context_budget(ApiProvider::Deepseek, "deepseek-v4-pro", None, 0)` -resolves the route output reservation through -`explicit_max_output_tokens_override()`, which reads the process-global -`CODEWHALE_MAX_OUTPUT_TOKENS` / `DEEPSEEK_MAX_OUTPUT_TOKENS` environment -variables. The V4 test asserts the no-override values (output cap 65 536, -input ceiling 933 440, trigger 800 000) but never took `lock_test_env()` and -never pinned those variables. Sibling tests in the same binary (this module, -`client`, `vision/tools`, `core/engine`) set those variables while holding -the env barrier; because the V4 test did not participate in that barrier, a -concurrent writer could flip the value between the test's reads — the -order-dependent flake seen once in a full parallel run (passes in isolation, -passes on reruns). - -The same class of bug surfaced during `--test-threads=1` verification in -`system_prompt_prefix_never_leaks_private_content`: the prompt is built with -`effective_home_dir()`, which reads process-global `HOME`/`USERPROFILE`. -The test never isolated those, so on a machine with -`~/.codewhale/instructions.md` the global instructions block leaked its real -absolute path into the prompt and failed the absolute-path assertion. The -test only passed in parallel runs when a sibling test's temporary `HOME` -guard happened to be live at the same moment; serialized, it failed -deterministically. - -## Fix - -- `crates/tui/src/route_budget.rs`: the V4 trigger test now holds - `lock_test_env()` and removes `CODEWHALE_MAX_OUTPUT_TOKENS` and - `DEEPSEEK_MAX_OUTPUT_TOKENS` for its duration — the established pattern - used by every sibling test with the same dependency. Assertions unchanged. -- `crates/tui/src/prompts.rs`: the prefix-leak test now holds - `lock_test_env()` and pins `HOME`/`USERPROFILE` to a scratch directory - (and removes `DEEPSEEK_SKILLS_DIR`), matching the neighboring - byte-stability tests, so global instructions and home-resolved skills - cannot leak real absolute paths. Assertions unchanged. - -## Verification - -Full suite, `cargo test -p codewhale-tui --lib` (10 904 tests): - -- Run A, `--test-threads=1`: 10891 passed / 0 failed / 13 ignored -- Run B, `--test-threads=1`: 10891 passed / 0 failed / 13 ignored -- Run C, default threads: 10891 passed / 0 failed / 13 ignored -- Run D, default threads: 10891 passed / 0 failed / 13 ignored - -Four consecutive full-suite runs green, covering both thread modes. Before -the prompts fix, a serialized run reproduced the second failure -deterministically (1 failed, `system_prompt_prefix_never_leaks_private_content`); -after both fixes that run mode is green. - -- `cargo fmt --all -- --check`: clean. -- `cargo clippy -p codewhale-tui --lib`: clean (exit 0). -- Focused checks: `route_budget::tests::v4_trigger_uses_window_percent_when_it_fits_spendable_input` - and the full `prompts::` module (109/109) green in isolation.