From 7bc72e4a0b3b09b266c6dacf7571b2a940954884 Mon Sep 17 00:00:00 2001 From: Anthony Shew Date: Tue, 7 Jul 2026 11:29:49 -0600 Subject: [PATCH 1/2] fix: Show toolchain tasks in the TUI and never run in silence Two fixes for TUI behavior in Cargo-enabled repos: The TUI task list came from a package.json scripts lookup, so Cargo tasks were never registered (resolving the standing TODO in tasks_with_command). Ask the package's toolchain via Toolchain::defines_task instead - the same authority execution uses - so the TUI, display, and execution cannot drift. JS behavior is unchanged (its defines_task is the scripts lookup). This also fixes the task list in watch mode. Worse than the missing panes: events for those unknown tasks killed the TUI event loop mid-run via TaskNotFound, after the terminal sink had already been disabled - the rest of the run executed with no live sink and every task's output silently vanished. Wrap the render-thread handle in a watchdog inside start_terminal_ui that re-enables the terminal sink the moment the TUI is gone, however it exits (normal shutdown, render error, panic). Both run and watch inherit it, and neither caller can forget it. --- crates/turborepo-lib/src/commands/run.rs | 15 ++--- crates/turborepo-lib/src/engine/mod.rs | 77 ++++++++++++++++++++++-- crates/turborepo-lib/src/run/mod.rs | 21 ++++++- crates/turborepo-lib/src/run/watch.rs | 9 +-- 4 files changed, 102 insertions(+), 20 deletions(-) diff --git a/crates/turborepo-lib/src/commands/run.rs b/crates/turborepo-lib/src/commands/run.rs index 6cec035ef9f83..103a75ab4ad0a 100644 --- a/crates/turborepo-lib/src/commands/run.rs +++ b/crates/turborepo-lib/src/commands/run.rs @@ -1,6 +1,6 @@ use std::{env, future::Future, sync::Arc}; -use tracing::{error, Instrument}; +use tracing::Instrument; use turborepo_api_client::SharedHttpClient; use turborepo_log::StructuredLogSink; use turborepo_query_api::QueryServer; @@ -140,7 +140,9 @@ pub async fn run( let (sender, handle) = { let _span = tracing::info_span!("start_ui").entered(); // The TUI needs a handle to the terminal sink so it can re-enable - // streamed output when the user toggles out of the alternate screen. + // streamed output when the user toggles out of the alternate + // screen, and its watchdog restores streamed output if the + // render thread exits mid-run. run.start_ui(sinks.terminal.clone())?.unzip() }; @@ -175,12 +177,11 @@ pub async fn run( sender.stop().await; } + // Wait for TUI cleanup (terminal restoration, task persistence) + // before printing anything else; render errors are logged by the + // watchdog inside `start_ui`. if let Some(handle) = handle { - match handle.await { - Ok(Err(e)) => error!("error encountered rendering tui: {e}"), - Err(e) => error!("render thread panicked: {e}"), - Ok(Ok(())) => {} - } + handle.await.ok(); } if let Some(path) = subscriber.stderr_redirect_path() { diff --git a/crates/turborepo-lib/src/engine/mod.rs b/crates/turborepo-lib/src/engine/mod.rs index 5cad47b044b29..33b2dfef21ef0 100644 --- a/crates/turborepo-lib/src/engine/mod.rs +++ b/crates/turborepo-lib/src/engine/mod.rs @@ -79,11 +79,17 @@ impl EngineExt for Engine { }) .filter_map(|task| { let pkg_name = PackageName::from(task.package()); - let json = pkg_graph.package_json(&pkg_name)?; - // TODO: delegate to command factory to filter down tasks to those that will - // have a runnable command. - (task.task() == "proxy" || json.command(task.task()).is_some()) - .then(|| task.to_string()) + let info = pkg_graph.package_info(&pkg_name)?; + // Ask the package's toolchain whether the task resolves to a + // runnable command — the same authority execution uses. For + // JS packages this is the package.json scripts lookup; for + // Cargo packages it consults the verb tables, so toolchain + // tasks appear in the TUI task list. + let defines_task = pkg_graph + .toolchains() + .get(&info.toolchain) + .is_some_and(|toolchain| toolchain.defines_task(info, task.task())); + (task.task() == "proxy" || defines_task).then(|| task.to_string()) }) .collect() } @@ -303,6 +309,67 @@ mod test { } } + #[tokio::test(flavor = "multi_thread")] + async fn test_tasks_with_command_asks_toolchains() { + // The TUI task list must come from the same authority execution + // uses: the package's toolchain. JS packages resolve via + // package.json scripts; Cargo packages resolve via the toolchain's + // verb tables — no scripts anywhere. + let tmp = tempfile::TempDir::with_prefix("tasks_with_command").unwrap(); + let root = AbsoluteSystemPath::from_std_path(tmp.path()).unwrap(); + + // A minimal Cargo workspace with one binary crate. + root.join_component("Cargo.toml") + .create_with_contents("[workspace]\nmembers = [\"crates/*\"]\nresolver = \"2\"\n") + .unwrap(); + let crate_dir = root.join_components(&["crates", "my-crate"]); + crate_dir.join_component("src").create_dir_all().unwrap(); + crate_dir + .join_component("Cargo.toml") + .create_with_contents( + "[package]\nname = \"my-crate\"\nversion = \"0.1.0\"\nedition = \"2021\"\n", + ) + .unwrap(); + crate_dir + .join_components(&["src", "main.rs"]) + .create_with_contents("fn main() {}\n") + .unwrap(); + + let mut engine: Engine = Engine::new(); + for (package, task) in [ + // JS package with a build script (DummyDiscovery gives "a" one). + ("a", "build"), + // JS package without any scripts. + ("c", "build"), + // The synthetic Cargo workspace package. + ("cargo", "test"), + // A binary crate. + ("my-crate", "build"), + ] { + let task_id = TaskId::new(package, task); + engine.get_index(&task_id); + engine.add_definition(task_id, TaskDefinition::default()); + } + let engine = engine.seal(); + + let graph = PackageGraph::builder(root, PackageJson::default()) + .with_package_discovery(DummyDiscovery( + turbopath::AbsoluteSystemPathBuf::try_from(tmp.path()).unwrap(), + )) + .with_toolchain(turborepo_repository::cargo::CargoToolchain::new( + root.to_owned(), + )) + .build() + .await + .unwrap(); + + let mut tasks = engine.tasks_with_command(&graph); + tasks.sort(); + // "c#build" is absent: no script defines it. Both Cargo tasks are + // present without any package.json involvement. + assert_eq!(tasks, vec!["a#build", "cargo#test", "my-crate#build"]); + } + #[tokio::test] async fn issue_4291() { // we had an issue where our engine validation would reject running persistent diff --git a/crates/turborepo-lib/src/run/mod.rs b/crates/turborepo-lib/src/run/mod.rs index eebe5e8a91bc1..95d309fcab88c 100644 --- a/crates/turborepo-lib/src/run/mod.rs +++ b/crates/turborepo-lib/src/run/mod.rs @@ -141,7 +141,9 @@ pub struct Run { shutdown_started_emitted: Arc, } -type UIResult = Result>)>, Error>; +// The join handle covers the render thread plus its sink-restoring +// watchdog; render errors are logged there, not surfaced to the caller. +type UIResult = Result)>, Error>; type TuiResult = UIResult; @@ -602,9 +604,24 @@ impl Run { repo_root, scrollback_len, Some(interrupt), - terminal_sink, + terminal_sink.clone(), )?; + // The terminal sink is disabled while the TUI owns the screen. + // Whatever ends the render thread — normal shutdown, a render + // error, a panic — output must return to the stream sink + // immediately: a mid-run TUI death would otherwise leave the rest + // of the run executing in silence, with every task's output + // dropped. Task output must always have a live sink. + let handle = tokio::spawn(async move { + match handle.await { + Ok(Err(e)) => tracing::error!("error encountered rendering tui: {e}"), + Err(e) => tracing::error!("render thread panicked: {e}"), + Ok(Ok(())) => {} + } + terminal_sink.enable(); + }); + Ok(Some((sender, handle))) } diff --git a/crates/turborepo-lib/src/run/watch.rs b/crates/turborepo-lib/src/run/watch.rs index e694854fb204a..7fe7f6397e9cb 100644 --- a/crates/turborepo-lib/src/run/watch.rs +++ b/crates/turborepo-lib/src/run/watch.rs @@ -153,7 +153,7 @@ pub struct WatchClient { telemetry: CommandEventBuilder, handler: SignalHandler, ui_sender: Option, - ui_handle: Option>>, + ui_handle: Option>, experimental_write_cache: bool, query_server: Option>, } @@ -661,12 +661,9 @@ impl WatchClient { if let Some(sender) = &self.ui_sender { sender.stop().await; } + // Render errors are logged by the watchdog inside `start_ui`. if let Some(handle) = self.ui_handle.take() { - match handle.await { - Ok(Err(err)) => tracing::error!("error encountered rendering tui: {err}"), - Err(err) => tracing::error!("render thread panicked: {err}"), - Ok(Ok(())) => {} - } + handle.await.ok(); } } From 1c567f2a1b7e0c9efb173622a6b1af6dfcd78e1e Mon Sep 17 00:00:00 2001 From: Anthony Shew Date: Tue, 7 Jul 2026 14:26:33 -0600 Subject: [PATCH 2/2] fix: Only mark a task as running once its process actually spawns (#13309) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why In a Cargo-enabled repo, the TUI showed a column of `»` running tasks whose processes didn't exist. The running marker fired at the very top of task execution — before the cache check and before the serial-group lock. For JS tasks that gap is milliseconds and invisible; Cargo's serial group (one cargo at a time, because they'd fight over the build directory) stretched it to minutes of tasks claiming to run while queued. ## What `task_output.start()` moves to immediately before the process spawn, after serial-group acquisition. States now tell the truth: | Situation | Before | After | |---|---|---| | Waiting on serial group | `»` running, empty pane | pending — same as waiting on a dependency | | Holding the lock / executing | `»` | `»` | | Cache hit | brief false `»` flash | pending → ✓ directly (it never ran; duration truthfully ~0) | | JS task wrapping cargo (e.g. napi builds) | unchanged | unchanged — its process is real, and cargo self-reports "Blocking waiting for file lock" | ## How Two consequences the move forces, both handled: - **Cache hits never start**, so `finish_task` learns the planned → finished transition (`start().finish()` back to back). Unknown task names remain a hard error — this is a legitimate state transition, not event-swallowing. Persistence on TUI exit is unaffected: `tasks_started()` reads the finished/running lists, which now include these tasks. - **`StartTask` was the only carrier of the task's `outputLogs` setting.** A never-started cache hit would have defaulted to full log persistence, ignoring `hash-only`/`new-only`. `Event::Status` — which every cache-restore path already emits before any start — now carries `output_logs` too, threaded through `TaskSender::status` → `UISender` → `TuiSender`. Exit-path audit: the only `execute_inner` return remaining before the new start location is the cache-hit return (verified — no `?` operators in between), and spawn-failure paths sit after the start call, keeping start→finish ordering. `start()` is a no-op outside the TUI, so stream/grouped log modes are untouched (their "cache miss, executing" lines come from the run cache, not this signal). Tests: `finish_task` planned→finished (+ unknown-name error + `tasks_started` inclusion), `set_status` delivering `output_logs`; existing UI/executor/run-cache/cargo-workspace suites green. Follow-ups deliberately not in this PR: run-summary durations still include serial-group wait (`tracker.start()` unmoved); batching contended cargo invocations into one process remains a parked design discussion. --- crates/turborepo-lockfiles/src/pnpm/data.rs | 163 +++++++++++++++++- crates/turborepo-run-cache/src/lib.rs | 2 +- crates/turborepo-task-executor/src/exec.rs | 8 +- crates/turborepo-ui/src/sender.rs | 15 +- crates/turborepo-ui/src/tui/app.rs | 143 +++++++++++++-- crates/turborepo-ui/src/tui/event.rs | 4 + crates/turborepo-ui/src/tui/handle.rs | 9 +- crates/turborepo/tests/common/mod.rs | 22 +++ crates/turborepo/tests/daemon_test.rs | 3 + crates/turborepo/tests/dry_json.rs | 3 + .../turborepo/tests/graceful_shutdown_test.rs | 19 +- crates/turborepo/tests/prune_test.rs | 54 ++++++ .../turborepo/tests/stdin_eof_startup_test.rs | 8 +- crates/turborepo/tests/watch_test.rs | 3 + 14 files changed, 423 insertions(+), 33 deletions(-) diff --git a/crates/turborepo-lockfiles/src/pnpm/data.rs b/crates/turborepo-lockfiles/src/pnpm/data.rs index 10c20b1e5dd81..266f55558ffe2 100644 --- a/crates/turborepo-lockfiles/src/pnpm/data.rs +++ b/crates/turborepo-lockfiles/src/pnpm/data.rs @@ -574,16 +574,25 @@ impl PnpmLockfile { for dependency in pruned_packages.keys() { let dp = DepPath::parse(self.version(), dependency.as_str())?; - let patch_key = format!("{}@{}", dp.name, dp.version); - if let Some(patch) = patches.get(&patch_key).filter(|patch| { + let hash_matches = |patch: &PatchFile| { // In V7 patch hash isn't included in packages key, so no need to check matches!(self.version(), SupportedLockfileVersion::V7AndV9) || dp.patch_hash() == Some(patch.hash()) - }) { + }; + + let patch_key = format!("{}@{}", dp.name, dp.version); + if let Some(patch) = patches.get(&patch_key).filter(|patch| hash_matches(patch)) { pruned_patches.insert(patch_key, patch.clone()); continue; } + if let Some((range_key, patch)) = Self::find_version_range_patch(patches, &dp) + && hash_matches(patch) + { + pruned_patches.insert(range_key.clone(), patch.clone()); + continue; + } + let version_less_key = dp.name.to_string(); if let Some(patch) = patches.get(&version_less_key) { pruned_patches.insert(version_less_key, patch.clone()); @@ -592,6 +601,31 @@ impl PnpmLockfile { Ok(pruned_patches) } + /// pnpm allows patch keys to target a semver range, e.g. `foo@^2.0.0` or + /// `foo@<=2.1.0`. Returns the first patch whose range matches the + /// dependency's version. + fn find_version_range_patch<'a>( + patches: &'a Map, + dp: &DepPath, + ) -> Option<(&'a String, &'a PatchFile)> { + let version = Version::parse(dp.version).ok()?; + patches.iter().find(|(key, _)| { + let Some(range) = key + .strip_prefix(dp.name) + .and_then(|rest| rest.strip_prefix('@')) + else { + return false; + }; + // A bare version key is an exact match in pnpm (handled by the + // direct lookup above), not a caret range like VersionReq would + // treat it. + if Version::parse(range).is_ok() { + return false; + } + semver::VersionReq::parse(range).is_ok_and(|req| req.matches(&version)) + }) + } + // Create a projection of all fields in the lockfile that could affect all // workspaces fn global_fields(&self) -> GlobalFields<'_> { @@ -2201,6 +2235,129 @@ snapshots: assert_eq!(pruned_lockfile.patched_dependencies, Some(BTreeMap::new())); } + /// Regression test for https://github.com/vercel/turborepo/issues/13301 + /// + /// pnpm supports semver ranges in `patchedDependencies` keys (e.g. + /// `is-odd@<=3.0.1`). Patches whose range matches a package in the pruned + /// closure must be kept. + #[test] + fn test_subgraph_keeps_version_range_patched_dependencies() { + let yaml = r#"lockfileVersion: '9.0' + +patchedDependencies: + is-odd@<=3.0.1: + hash: abc + path: patches/is-odd.patch + '@scope/pkg@^1.0.0': + hash: def + path: patches/scope-pkg.patch + lodash@>=5.0.0: + hash: ghi + path: patches/lodash.patch + +importers: + + .: {} + + apps/web: + dependencies: + is-odd: + specifier: 3.0.1 + version: 3.0.1 + '@scope/pkg': + specifier: 1.2.3 + version: 1.2.3 + lodash: + specifier: 4.17.21 + version: 4.17.21 + +packages: + + is-odd@3.0.1: + resolution: {integrity: sha512-bbb} + patched: true + + '@scope/pkg@1.2.3': + resolution: {integrity: sha512-ccc} + patched: true + + lodash@4.17.21: + resolution: {integrity: sha512-aaa} + +snapshots: + + is-odd@3.0.1: {} + + '@scope/pkg@1.2.3': {} + + lodash@4.17.21: {} +"#; + let lockfile = PnpmLockfile::from_bytes(yaml.as_bytes()).unwrap(); + + let workspace_packages = vec!["apps/web".to_string()]; + let resolved_packages = vec![ + "is-odd@3.0.1".to_string(), + "@scope/pkg@1.2.3".to_string(), + "lodash@4.17.21".to_string(), + ]; + let pruned = lockfile + .subgraph(&workspace_packages, &resolved_packages) + .unwrap(); + + let pruned_bytes = pruned.encode().unwrap(); + let pruned_lockfile = PnpmLockfile::from_bytes(&pruned_bytes).unwrap(); + let patches = pruned_lockfile + .patched_dependencies + .as_ref() + .expect("should have patched dependencies"); + + assert!(patches.contains_key("is-odd@<=3.0.1")); + assert!(patches.contains_key("@scope/pkg@^1.0.0")); + // lodash@4.17.21 does not satisfy >=5.0.0 + assert!(!patches.contains_key("lodash@>=5.0.0")); + } + + /// A bare version patch key is an exact match in pnpm, not a caret range. + /// `is-odd@3.0.0` must not apply to `is-odd@3.0.1`. + #[test] + fn test_subgraph_does_not_treat_exact_patch_key_as_range() { + let yaml = r#"lockfileVersion: '9.0' + +patchedDependencies: + is-odd@3.0.0: + hash: abc + path: patches/is-odd.patch + +importers: + + .: {} + + apps/web: + dependencies: + is-odd: + specifier: 3.0.1 + version: 3.0.1 + +packages: + + is-odd@3.0.1: + resolution: {integrity: sha512-bbb} + +snapshots: + + is-odd@3.0.1: {} +"#; + let lockfile = PnpmLockfile::from_bytes(yaml.as_bytes()).unwrap(); + + let pruned = lockfile + .subgraph(&["apps/web".to_string()], &["is-odd@3.0.1".to_string()]) + .unwrap(); + + let pruned_bytes = pruned.encode().unwrap(); + let pruned_lockfile = PnpmLockfile::from_bytes(&pruned_bytes).unwrap(); + assert_eq!(pruned_lockfile.patched_dependencies, Some(BTreeMap::new())); + } + /// Regression test for https://github.com/vercel/turborepo/issues/12252 /// /// The Lockfile trait returns `HashMap` from diff --git a/crates/turborepo-run-cache/src/lib.rs b/crates/turborepo-run-cache/src/lib.rs index f8ccd266c10a0..bba9c171c7bb3 100644 --- a/crates/turborepo-run-cache/src/lib.rs +++ b/crates/turborepo-run-cache/src/lib.rs @@ -417,7 +417,7 @@ impl TaskCache { result: turborepo_ui::tui::event::CacheResult, ) { if let Some(sender) = tui_sender { - sender.status(message, result); + sender.status(message, result, self.task_output_logs.into()); } if !message.is_empty() { let line = format!("{message}\n"); diff --git a/crates/turborepo-task-executor/src/exec.rs b/crates/turborepo-task-executor/src/exec.rs index e5f33cc42ad66..5547be8a503ce 100644 --- a/crates/turborepo-task-executor/src/exec.rs +++ b/crates/turborepo-task-executor/src/exec.rs @@ -363,8 +363,6 @@ where task_handle: &mut turborepo_log::grouping::TaskHandle, telemetry: &PackageTaskEventBuilder, ) -> Result { - task_output.start(self.task_cache.output_logs().into()); - if !self.task_cache.is_caching_disabled() { let missing_platform_env = self.platform_env.validate(&self.execution_env); if !missing_platform_env.is_empty() { @@ -430,6 +428,12 @@ where None => None, }; + // The task is only presented as running once its process is about + // to exist. Everything before this point — cache restore, waiting + // on the serial group — happens while the task is still pending, + // and cache hits finish without ever starting. + task_output.start(self.task_cache.output_logs().into()); + // Spawn the process let cmd = self.cmd.clone(); let mut process = diff --git a/crates/turborepo-ui/src/sender.rs b/crates/turborepo-ui/src/sender.rs index 3fba6412ce92e..e2d091e4c2cc7 100644 --- a/crates/turborepo-ui/src/sender.rs +++ b/crates/turborepo-ui/src/sender.rs @@ -30,9 +30,15 @@ impl UISender { } } - pub fn status(&self, task: String, status: String, result: CacheResult) { + pub fn status( + &self, + task: String, + status: String, + result: CacheResult, + output_logs: OutputLogs, + ) { match self { - UISender::Tui(sender) => sender.status(task, status, result), + UISender::Tui(sender) => sender.status(task, status, result, output_logs), } } fn set_stdin(&self, task: String, stdin: Box) { @@ -115,12 +121,13 @@ impl TaskSender { self.handle.set_stdin(self.name.clone(), stdin); } - pub fn status(&self, status: &str, result: CacheResult) { + pub fn status(&self, status: &str, result: CacheResult, output_logs: OutputLogs) { // Since this will be rendered via ratatui we any ANSI escape codes will not be // handled. // TODO: prevent the status from having ANSI codes in this scenario let status = console::strip_ansi_codes(status).into_owned(); - self.handle.status(self.name.clone(), status, result); + self.handle + .status(self.name.clone(), status, result, output_logs); } } diff --git a/crates/turborepo-ui/src/tui/app.rs b/crates/turborepo-ui/src/tui/app.rs index 9703f4b4377b9..bb8047a45b184 100644 --- a/crates/turborepo-ui/src/tui/app.rs +++ b/crates/turborepo-ui/src/tui/app.rs @@ -506,8 +506,11 @@ impl App { Ok(()) } - /// Mark the given running task as finished - /// Errors if given task wasn't a running task + /// Mark the given running or planned task as finished. + /// + /// A task is only marked as started once its process actually spawns, + /// so cache hits legitimately finish straight from the planned state — + /// they never ran. Errors if the task is unknown or already finished. #[tracing::instrument(skip(self, result))] pub fn finish_task(&mut self, task: &str, result: TaskResult) -> Result<(), Error> { debug!("finishing task {task}"); @@ -518,16 +521,32 @@ impl App { .task_name(self.selected_task_index)? .to_string(); - let running_idx = self + let finished = if let Some(running_idx) = self .tasks_by_status .running .iter() .position(|running| running.name() == task) - .ok_or_else(|| Error::TaskNotFound { name: task.into() })?; - - let running = self.tasks_by_status.running.remove(running_idx); - self.tasks_by_status - .insert_finished_task(running.finish(result)); + { + self.tasks_by_status + .running + .remove(running_idx) + .finish(result) + } else { + let planned_idx = self + .tasks_by_status + .planned + .iter() + .position(|planned| planned.name() == task) + .ok_or_else(|| Error::TaskNotFound { name: task.into() })?; + // start().finish() back to back: a task that never ran has a + // (truthful) zero duration. + self.tasks_by_status + .planned + .remove(planned_idx) + .start() + .finish(result) + }; + self.tasks_by_status.insert_finished_task(finished); self.tasks .get_mut(task) @@ -725,6 +744,7 @@ impl App { task: String, status: String, result: CacheResult, + output_logs: OutputLogs, ) -> Result<(), Error> { let task = self .tasks @@ -734,6 +754,9 @@ impl App { })?; task.status = Some(status); task.cache_result = Some(result); + // Cache hits finish without ever starting, so `StartTask` cannot be + // relied on to deliver the output verbosity before persistence. + task.output_logs = Some(output_logs); Ok(()) } @@ -1551,8 +1574,9 @@ fn update( task, status, result, + output_logs, } => { - app.set_status(task, status, result)?; + app.set_status(task, status, result, output_logs)?; } Event::InternalStop => { debug!("shutting down due to internal failure"); @@ -2137,7 +2161,12 @@ mod test { assert_eq!(app.task_list_scroll.selected(), Some(1), "selected b"); assert_eq!(app.tasks_by_status.task_name(1)?, "b", "selected b"); // set status for a - app.set_status("a".to_string(), "building".to_string(), CacheResult::Hit)?; + app.set_status( + "a".to_string(), + "building".to_string(), + CacheResult::Hit, + OutputLogs::Full, + )?; assert_eq!( app.tasks.get("a").unwrap().status.as_deref(), @@ -2147,6 +2176,77 @@ mod test { Ok(()) } + #[test] + fn test_finish_task_from_planned_state() -> Result<(), Error> { + let repo_root_tmp = tempdir()?; + let repo_root = AbsoluteSystemPathBuf::try_from(repo_root_tmp.path()) + .expect("Failed to create AbsoluteSystemPathBuf"); + + let mut app: App> = App::new_for_test( + 100, + 100, + vec!["a".to_string(), "b".to_string()], + PreferenceLoader::new(&repo_root), + 2048, + ); + + // Cache hits finish without ever starting: planned -> finished. + app.finish_task("a", TaskResult::CacheHit)?; + assert!( + app.tasks_by_status + .finished + .iter() + .any(|task| task.name() == "a"), + "task should be finished" + ); + assert!( + !app.tasks_by_status + .planned + .iter() + .any(|task| task.name() == "a"), + "task should no longer be planned" + ); + // Persistence on TUI exit covers finished tasks, so the replayed + // logs of a never-started cache hit still land in the terminal. + assert!( + app.tasks_by_status + .tasks_started() + .contains(&"a".to_string()) + ); + + // Unknown tasks remain a hard error. + assert!(app.finish_task("missing", TaskResult::Success).is_err()); + Ok(()) + } + + #[test] + fn test_set_status_carries_output_logs() -> Result<(), Error> { + let repo_root_tmp = tempdir()?; + let repo_root = AbsoluteSystemPathBuf::try_from(repo_root_tmp.path()) + .expect("Failed to create AbsoluteSystemPathBuf"); + + let mut app: App> = App::new_for_test( + 100, + 100, + vec!["a".to_string()], + PreferenceLoader::new(&repo_root), + 2048, + ); + + app.set_status( + "a".to_string(), + "cache hit, replaying logs".to_string(), + CacheResult::Hit, + OutputLogs::HashOnly, + )?; + assert_eq!( + app.tasks.get("a").unwrap().output_logs, + Some(OutputLogs::HashOnly), + "status must deliver output verbosity for tasks that never start" + ); + Ok(()) + } + #[test] fn test_restarting_task_no_scroll() -> Result<(), Error> { let repo_root_tmp = tempdir()?; @@ -2830,6 +2930,7 @@ mod test { fn test_should_start_terminal_on_cache_miss() { // Cache miss should trigger terminal start let miss_event = Event::Status { + output_logs: OutputLogs::Full, task: "task-a".to_string(), status: "building".to_string(), // This includes cache bypasses via `--force` @@ -2845,6 +2946,7 @@ mod test { fn test_should_not_start_terminal_on_cache_hit() { // Cache hit should NOT trigger terminal start let hit_event = Event::Status { + output_logs: OutputLogs::Full, task: "task-a".to_string(), status: "cached".to_string(), result: CacheResult::Hit, @@ -2915,8 +3017,18 @@ mod test { // Simulate a full cache hit scenario: // 1. Set status as cache hit (this doesn't start the task in running state) - app.set_status("a".to_string(), "cached".to_string(), CacheResult::Hit)?; - app.set_status("b".to_string(), "cached".to_string(), CacheResult::Hit)?; + app.set_status( + "a".to_string(), + "cached".to_string(), + CacheResult::Hit, + OutputLogs::Full, + )?; + app.set_status( + "b".to_string(), + "cached".to_string(), + CacheResult::Hit, + OutputLogs::Full, + )?; // 2. Start and finish tasks with CacheHit result app.start_task("a", OutputLogs::Full)?; @@ -3421,7 +3533,12 @@ mod test { // The run proceeds: each task gets StartTask, Status(Hit), EndTask(CacheHit). for task in &tasks { app.start_task(task, OutputLogs::Full)?; - app.set_status(task.clone(), "cached".to_string(), CacheResult::Hit)?; + app.set_status( + task.clone(), + "cached".to_string(), + CacheResult::Hit, + OutputLogs::Full, + )?; app.finish_task(task, TaskResult::CacheHit)?; } diff --git a/crates/turborepo-ui/src/tui/event.rs b/crates/turborepo-ui/src/tui/event.rs index 5697144b4910c..6a1573f6435d7 100644 --- a/crates/turborepo-ui/src/tui/event.rs +++ b/crates/turborepo-ui/src/tui/event.rs @@ -21,6 +21,10 @@ pub enum Event { task: String, status: String, result: CacheResult, + /// The task's configured output verbosity. Carried here (in + /// addition to `StartTask`) because cache hits finish without ever + /// starting, and log persistence must still respect the setting. + output_logs: OutputLogs, }, PaneSizeQuery(oneshot::Sender), Stop(oneshot::Sender<()>), diff --git a/crates/turborepo-ui/src/tui/handle.rs b/crates/turborepo-ui/src/tui/handle.rs index aa50621bb772e..eb95e5da0f554 100644 --- a/crates/turborepo-ui/src/tui/handle.rs +++ b/crates/turborepo-ui/src/tui/handle.rs @@ -68,12 +68,19 @@ impl TuiSender { self.primary.send(Event::EndTask { task, result }).ok(); } - pub fn status(&self, task: String, status: String, result: CacheResult) { + pub fn status( + &self, + task: String, + status: String, + result: CacheResult, + output_logs: OutputLogs, + ) { self.primary .send(Event::Status { task, status, result, + output_logs, }) .ok(); } diff --git a/crates/turborepo/tests/common/mod.rs b/crates/turborepo/tests/common/mod.rs index cffaf7baa335f..f099b9301cc1c 100644 --- a/crates/turborepo/tests/common/mod.rs +++ b/crates/turborepo/tests/common/mod.rs @@ -28,6 +28,25 @@ pub fn turbo_output_filters() -> Vec<(&'static str, &'static str)> { ] } +/// Env keys in the test process that can carry real turbo configuration +/// into spawned turbo children — TURBO_TEAM/TURBO_TOKEN exported by CI +/// credential steps, VERCEL_ARTIFACTS_* on Vercel builds, or a developer's +/// local settings. Tests assert against turbo's defaults, so every harness +/// that spawns turbo must remove these before applying its own vars; +/// per-test overrides (explicit env sets after removal) still win because +/// later ops on a key replace earlier ones. +pub fn ambient_turbo_env_keys() -> Vec { + std::env::vars_os() + .map(|(key, _)| key) + .filter(|key| { + let key = key.to_string_lossy(); + key.starts_with("TURBO_") + || key == "VERCEL_ARTIFACTS_OWNER" + || key == "VERCEL_ARTIFACTS_TOKEN" + }) + .collect() +} + /// Return a pre-configured `Command` for the turbo binary with all standard /// env var suppression applied. Callers can chain `.arg()`, `.env()`, etc. /// before calling `.output()`. @@ -38,6 +57,9 @@ pub fn turbo_command(test_dir: &Path) -> assert_cmd::Command { cmd.env("PATH", setup::prepend_to_path(&corepack_dir)) .env("COREPACK_HOME", setup::corepack_home()); } + for key in ambient_turbo_env_keys() { + cmd.env_remove(&key); + } cmd.env("TURBO_TELEMETRY_MESSAGE_DISABLED", "1") .env("TURBO_GLOBAL_WARNING_DISABLED", "1") .env("TURBO_PRINT_VERSION_DISABLED", "1") diff --git a/crates/turborepo/tests/daemon_test.rs b/crates/turborepo/tests/daemon_test.rs index a7af3b67eef1e..a90710be082e6 100644 --- a/crates/turborepo/tests/daemon_test.rs +++ b/crates/turborepo/tests/daemon_test.rs @@ -7,6 +7,9 @@ use common::setup; fn run_daemon_status(dir: &std::path::Path, env_val: &str, extra_args: &[&str]) -> String { let config_dir = tempfile::tempdir().unwrap(); let mut cmd = assert_cmd::Command::cargo_bin("turbo").unwrap(); + for key in common::ambient_turbo_env_keys() { + cmd.env_remove(&key); + } cmd.env("TURBO_TELEMETRY_MESSAGE_DISABLED", "1") .env("TURBO_GLOBAL_WARNING_DISABLED", "1") .env("TURBO_PRINT_VERSION_DISABLED", "1") diff --git a/crates/turborepo/tests/dry_json.rs b/crates/turborepo/tests/dry_json.rs index 2b8af53f249a4..a562d5112cd82 100644 --- a/crates/turborepo/tests/dry_json.rs +++ b/crates/turborepo/tests/dry_json.rs @@ -115,6 +115,9 @@ fn test_monorepo_env_var_in_summary() -> Result<(), anyhow::Error> { let config_dir = tempfile::tempdir()?; let mut cmd = assert_cmd::Command::cargo_bin("turbo")?; + for key in common::ambient_turbo_env_keys() { + cmd.env_remove(&key); + } cmd.env("TURBO_TELEMETRY_MESSAGE_DISABLED", "1") .env("TURBO_GLOBAL_WARNING_DISABLED", "1") .env("TURBO_PRINT_VERSION_DISABLED", "1") diff --git a/crates/turborepo/tests/graceful_shutdown_test.rs b/crates/turborepo/tests/graceful_shutdown_test.rs index 4316fd89daa97..9b98003c58881 100644 --- a/crates/turborepo/tests/graceful_shutdown_test.rs +++ b/crates/turborepo/tests/graceful_shutdown_test.rs @@ -180,10 +180,11 @@ mod unix { fn spawn_noninteractive_turbo(test_dir: &Path) -> ChildGuard { let mut cmd = Command::new(turbo_bin()); - cmd.arg("run") - .arg("dev") - .arg("--filter=app-a") - .env("TURBO_TELEMETRY_MESSAGE_DISABLED", "1") + cmd.arg("run").arg("dev").arg("--filter=app-a"); + for key in common::ambient_turbo_env_keys() { + cmd.env_remove(&key); + } + cmd.env("TURBO_TELEMETRY_MESSAGE_DISABLED", "1") .env("TURBO_GLOBAL_WARNING_DISABLED", "1") .env("TURBO_PRINT_VERSION_DISABLED", "1") .env("DO_NOT_TRACK", "1") @@ -202,8 +203,11 @@ mod unix { cmd.arg(turbo_node_wrapper()) .arg("run") .arg("dev") - .arg("--filter=app-a") - .env("TURBO_BINARY_PATH", turbo_bin()) + .arg("--filter=app-a"); + for key in common::ambient_turbo_env_keys() { + cmd.env_remove(&key); + } + cmd.env("TURBO_BINARY_PATH", turbo_bin()) .env("TURBO_TELEMETRY_MESSAGE_DISABLED", "1") .env("TURBO_GLOBAL_WARNING_DISABLED", "1") .env("TURBO_PRINT_VERSION_DISABLED", "1") @@ -258,6 +262,9 @@ mod unix { command.arg("dev"); command.arg("--filter=app-a"); command.cwd(test_dir); + for key in common::ambient_turbo_env_keys() { + command.env_remove(&key); + } command.env("TURBO_TELEMETRY_MESSAGE_DISABLED", "1"); command.env("TURBO_GLOBAL_WARNING_DISABLED", "1"); command.env("TURBO_PRINT_VERSION_DISABLED", "1"); diff --git a/crates/turborepo/tests/prune_test.rs b/crates/turborepo/tests/prune_test.rs index 44ada1dfa7133..3585348e4ea90 100644 --- a/crates/turborepo/tests/prune_test.rs +++ b/crates/turborepo/tests/prune_test.rs @@ -199,6 +199,60 @@ patchedDependencies: } } +/// Regression test for https://github.com/vercel/turborepo/issues/13301 +/// +/// pnpm supports semver ranges in `patchedDependencies` keys (e.g. +/// `is-number@<=7.0.0`). Prune must keep patches whose range matches a +/// package in the pruned closure. +#[test] +fn test_prune_docker_keeps_version_range_patches() { + let tempdir = tempfile::tempdir().unwrap(); + setup::copy_fixture("monorepo_with_root_dep", tempdir.path()).unwrap(); + + // Rewrite the exact patch key to a semver range everywhere it's declared. + for file in ["pnpm-lock.yaml", "package.json"] { + let path = tempdir.path().join(file); + let contents = fs::read_to_string(&path).unwrap(); + fs::write( + &path, + contents + .replace("is-number@7.0.0:", "is-number@<=7.0.0:") + .replace("\"is-number@7.0.0\"", "\"is-number@<=7.0.0\""), + ) + .unwrap(); + } + + let output = run_turbo(tempdir.path(), &["prune", "web", "--docker"]); + assert!( + output.status.success(), + "prune --docker failed: {}", + combined_output(&output) + ); + + let pruned_lockfile = fs::read_to_string(tempdir.path().join("out/pnpm-lock.yaml")).unwrap(); + assert!( + pruned_lockfile.contains("is-number@<=7.0.0"), + "pruned lockfile should retain range patch key:\n{pruned_lockfile}" + ); + + let pkg_json: serde_json::Value = serde_json::from_str( + &fs::read_to_string(tempdir.path().join("out/json/package.json")).unwrap(), + ) + .unwrap(); + assert_eq!( + pkg_json["pnpm"]["patchedDependencies"]["is-number@<=7.0.0"], + "patches/is-number@7.0.0.patch" + ); + + assert!( + tempdir + .path() + .join("out/json/patches/is-number@7.0.0.patch") + .exists(), + "patch file should be copied into pruned output" + ); +} + #[test] fn test_prune_rejects_patch_paths_with_parent_dir() { let tempdir = tempfile::tempdir().unwrap(); diff --git a/crates/turborepo/tests/stdin_eof_startup_test.rs b/crates/turborepo/tests/stdin_eof_startup_test.rs index 5bf33b4ccb366..1e2e587d0b1f9 100644 --- a/crates/turborepo/tests/stdin_eof_startup_test.rs +++ b/crates/turborepo/tests/stdin_eof_startup_test.rs @@ -22,9 +22,11 @@ fn setup_stdin_eof_fixture() -> tempfile::TempDir { fn spawn_turbo_run_dev(test_dir: &Path, config_dir: &Path) -> Child { let turbo_bin = assert_cmd::cargo::cargo_bin("turbo"); let mut cmd = Command::new(turbo_bin); - cmd.arg("run") - .arg("dev") - .env("TURBO_TELEMETRY_MESSAGE_DISABLED", "1") + cmd.arg("run").arg("dev"); + for key in common::ambient_turbo_env_keys() { + cmd.env_remove(&key); + } + cmd.env("TURBO_TELEMETRY_MESSAGE_DISABLED", "1") .env("TURBO_GLOBAL_WARNING_DISABLED", "1") .env("TURBO_PRINT_VERSION_DISABLED", "1") .env("TURBO_CONFIG_DIR_PATH", config_dir) diff --git a/crates/turborepo/tests/watch_test.rs b/crates/turborepo/tests/watch_test.rs index 00a9fc3ba9eac..a35934a1ea627 100644 --- a/crates/turborepo/tests/watch_test.rs +++ b/crates/turborepo/tests/watch_test.rs @@ -113,6 +113,9 @@ fn spawn_turbo_watch_with_tasks_and_stdio( for task in tasks { cmd.arg(task); } + for key in common::ambient_turbo_env_keys() { + cmd.env_remove(&key); + } cmd.env("TURBO_TELEMETRY_MESSAGE_DISABLED", "1") .env("TURBO_GLOBAL_WARNING_DISABLED", "1") .env("TURBO_PRINT_VERSION_DISABLED", "1")