diff --git a/CHANGELOG.md b/CHANGELOG.md index 85b6853395..3dfe5d9c94 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Heavy pull requests now run the real Linux workspace nextest, doctest, and lockfile lanes directly on GitHub Actions instead of reporting a green placeholder while waiting for a branch-specific CNB mirror (#5547). +- 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 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); + } +} diff --git a/crates/tui/CHANGELOG.md b/crates/tui/CHANGELOG.md index 303d8d94bf..b68e777ef0 100644 --- a/crates/tui/CHANGELOG.md +++ b/crates/tui/CHANGELOG.md @@ -10,6 +10,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Heavy pull requests now run the real Linux workspace nextest, doctest, and lockfile lanes directly on GitHub Actions instead of reporting a green placeholder while waiting for a branch-specific CNB mirror (#5547). +- 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 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/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"; diff --git a/crates/tui/src/core/engine/turn_loop.rs b/crates/tui/src/core/engine/turn_loop.rs index ee8435e013..3e014d655d 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,66 @@ 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()?; + + // 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 + .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() { @@ -6560,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:?}" + ); + } + } } 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" + ); + } } 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/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");