Skip to content

Commit b577d27

Browse files
claudebranchseer
authored andcommitted
feat: handle workspace root self-recursion in task planning
When a workspace root task's command contains a `vp run` invocation that expands back to include itself, the planner now handles this gracefully instead of treating it as a fatal recursion error. Two rules are implemented: - Rule 1 (Skip): When a nested `vp run` query matches the parent query exactly, the subcommand is skipped (e.g., root's "build": "vp run -r build" when invoked via `vp run -r build`). - Rule 2 (Prune): When the expanding task appears in a different query's expansion result, it is pruned from the graph and edges are reconnected through it (e.g., root's "build": "vp run -r build" when invoked via `vp run build` from root directory). Mutual recursion (A→B→A) remains a fatal error via check_recursion. To enable TaskQuery comparison for Rule 1, a GlobPattern wrapper type is introduced that implements PartialEq by comparing source strings, and PartialEq is derived through the entire type chain from PackageFilter up to TaskQuery. https://claude.ai/code/session_01QdDRLrGeycdzQ4g1FWX4pZ
1 parent 2537b34 commit b577d27

36 files changed

Lines changed: 753 additions & 29 deletions

crates/vite_task_graph/src/query/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ use crate::{IndexedTaskGraph, TaskDependencyType, TaskId, TaskNodeIndex};
3131
pub type TaskExecutionGraph = DiGraphMap<TaskNodeIndex, ()>;
3232

3333
/// A query for which tasks to run.
34-
#[derive(Debug)]
34+
#[derive(Debug, PartialEq)]
3535
pub struct TaskQuery {
3636
/// Which packages to select.
3737
pub package_query: PackageQuery,

crates/vite_task_plan/src/context.rs

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ use std::{env::JoinPathsError, ffi::OsStr, ops::Range, sync::Arc};
33
use rustc_hash::FxHashMap;
44
use vite_path::AbsolutePath;
55
use vite_str::Str;
6-
use vite_task_graph::{IndexedTaskGraph, TaskNodeIndex, config::ResolvedGlobalCacheConfig};
6+
use vite_task_graph::{
7+
IndexedTaskGraph, TaskNodeIndex, config::ResolvedGlobalCacheConfig, query::TaskQuery,
8+
};
79

810
use crate::{PlanRequestParser, path_env::prepend_path_env};
911

@@ -42,6 +44,10 @@ pub struct PlanContext<'a> {
4244

4345
/// Final resolved global cache config, combining the graph's config with any CLI override.
4446
resolved_global_cache: ResolvedGlobalCacheConfig,
47+
48+
/// The query that caused the current expansion.
49+
/// Used by the skip rule to detect and skip duplicate nested expansions.
50+
parent_query: Arc<TaskQuery>,
4551
}
4652

4753
impl<'a> PlanContext<'a> {
@@ -52,6 +58,7 @@ impl<'a> PlanContext<'a> {
5258
callbacks: &'a mut (dyn PlanRequestParser + 'a),
5359
indexed_task_graph: &'a IndexedTaskGraph,
5460
resolved_global_cache: ResolvedGlobalCacheConfig,
61+
parent_query: Arc<TaskQuery>,
5562
) -> Self {
5663
Self {
5764
workspace_path,
@@ -62,6 +69,7 @@ impl<'a> PlanContext<'a> {
6269
indexed_task_graph,
6370
extra_args: Arc::default(),
6471
resolved_global_cache,
72+
parent_query,
6573
}
6674
}
6775

@@ -128,6 +136,20 @@ impl<'a> PlanContext<'a> {
128136
self.resolved_global_cache = config;
129137
}
130138

139+
pub fn parent_query(&self) -> &TaskQuery {
140+
&self.parent_query
141+
}
142+
143+
pub fn set_parent_query(&mut self, query: Arc<TaskQuery>) {
144+
self.parent_query = query;
145+
}
146+
147+
/// Returns the task currently being expanded (whose command triggered a nested query).
148+
/// This is the last task on the call stack, or `None` at the top level.
149+
pub fn expanding_task(&self) -> Option<TaskNodeIndex> {
150+
self.task_call_stack.last().map(|(idx, _)| *idx)
151+
}
152+
131153
pub fn duplicate(&mut self) -> PlanContext<'_> {
132154
PlanContext {
133155
workspace_path: self.workspace_path,
@@ -138,6 +160,7 @@ impl<'a> PlanContext<'a> {
138160
indexed_task_graph: self.indexed_task_graph,
139161
extra_args: Arc::clone(&self.extra_args),
140162
resolved_global_cache: self.resolved_global_cache,
163+
parent_query: Arc::clone(&self.parent_query),
141164
}
142165
}
143166
}

crates/vite_task_plan/src/lib.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,15 +205,18 @@ pub async fn plan_query(
205205
query_plan_request.plan_options.cache_override,
206206
);
207207

208+
let QueryPlanRequest { query, plan_options } = query_plan_request;
209+
let query = Arc::new(query);
208210
let context = PlanContext::new(
209211
workspace_path,
210212
Arc::clone(cwd),
211213
envs.clone(),
212214
plan_request_parser,
213215
indexed_task_graph,
214216
resolved_global_cache,
217+
Arc::clone(&query),
215218
);
216-
plan_query_request(query_plan_request, context).await
219+
plan_query_request(query, plan_options, context).await
217220
}
218221

219222
const fn resolve_cache_with_override(

crates/vite_task_plan/src/plan.rs

Lines changed: 67 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use std::{
1212
};
1313

1414
use futures_util::FutureExt;
15+
use petgraph::Direction;
1516
use rustc_hash::FxHashMap;
1617
use vite_path::{AbsolutePath, AbsolutePathBuf, RelativePathBuf, relative::InvalidPathDataError};
1718
use vite_shell::try_parse_as_and_list;
@@ -22,6 +23,7 @@ use vite_task_graph::{
2223
CacheConfig, ResolvedGlobalCacheConfig, ResolvedTaskOptions,
2324
user::{UserCacheConfig, UserTaskOptions},
2425
},
26+
query::TaskQuery,
2527
};
2628

2729
use crate::{
@@ -34,7 +36,8 @@ use crate::{
3436
in_process::InProcessExecution,
3537
path_env::get_path_env,
3638
plan_request::{
37-
CacheOverride, PlanRequest, QueryPlanRequest, ScriptCommand, SyntheticPlanRequest,
39+
CacheOverride, PlanOptions, PlanRequest, QueryPlanRequest, ScriptCommand,
40+
SyntheticPlanRequest,
3841
},
3942
resolve_cache_with_override,
4043
};
@@ -197,17 +200,27 @@ async fn plan_task_as_execution_node(
197200
let execution_item_kind: ExecutionItemKind = match plan_request {
198201
// Expand task query like `vp run -r build`
199202
Some(PlanRequest::Query(query_plan_request)) => {
203+
// Rule 1: skip if this nested query is the same as the parent expansion.
204+
// This handles workspace root tasks like `"build": "vp run -r build"` —
205+
// re-entering the same query would just re-expand the same tasks.
206+
if query_plan_request.query == *context.parent_query() {
207+
continue;
208+
}
209+
200210
// Save task name before consuming the request
201211
let task_name = query_plan_request.query.task_name.clone();
202212
// Add prefix envs to the context
203213
context.add_envs(and_item.envs.iter());
204-
let execution_graph = plan_query_request(query_plan_request, context)
205-
.await
206-
.map_err(|error| Error::NestPlan {
207-
task_display: task_node.task_display.clone(),
208-
command: Str::from(&command_str[add_item_span.clone()]),
209-
error: Box::new(error),
210-
})?;
214+
let QueryPlanRequest { query, plan_options } = query_plan_request;
215+
let query = Arc::new(query);
216+
let execution_graph =
217+
plan_query_request(Arc::clone(&query), plan_options, context)
218+
.await
219+
.map_err(|error| Error::NestPlan {
220+
task_display: task_node.task_display.clone(),
221+
command: Str::from(&command_str[add_item_span.clone()]),
222+
error: Box::new(error),
223+
})?;
211224
// An empty execution graph means no tasks matched the query.
212225
// At the top level the session shows the task selector UI,
213226
// but in a nested context there is no UI — propagate as an error.
@@ -552,17 +565,24 @@ fn plan_spawn_execution(
552565
///
553566
/// Builds a `DiGraph` of task executions, then validates it is acyclic via
554567
/// `ExecutionGraph::try_from_graph`. Returns `CycleDependencyDetected` if a cycle is found.
568+
///
569+
/// **Rule 2 (prune self):** If the expanding task (the task whose command triggered
570+
/// this nested query) appears in the expansion result, it is pruned from the graph
571+
/// and its predecessors are wired directly to its successors. This prevents
572+
/// workspace root tasks like `"build": "vp run build"` from infinitely re-expanding
573+
/// themselves when a different query reaches them.
555574
#[expect(clippy::future_not_send, reason = "PlanContext contains !Send dyn PlanRequestParser")]
556575
pub async fn plan_query_request(
557-
query_plan_request: QueryPlanRequest,
576+
query: Arc<TaskQuery>,
577+
plan_options: PlanOptions,
558578
mut context: PlanContext<'_>,
559579
) -> Result<ExecutionGraph, Error> {
560580
// Apply cache override from `--cache` / `--no-cache` flags on this request.
561581
//
562582
// When `None`, we skip the update so the context keeps whatever the parent
563583
// resolved — this is how `vp run --cache outer` propagates to a nested
564584
// `vp run inner` that has no flags of its own.
565-
let cache_override = query_plan_request.plan_options.cache_override;
585+
let cache_override = plan_options.cache_override;
566586
if cache_override != CacheOverride::None {
567587
// Override is relative to the *workspace* config, not the parent's
568588
// resolved config. This means `vp run --no-cache outer` where outer
@@ -574,11 +594,13 @@ pub async fn plan_query_request(
574594
);
575595
context.set_resolved_global_cache(final_cache);
576596
}
577-
context.set_extra_args(Arc::clone(&query_plan_request.plan_options.extra_args));
597+
context.set_extra_args(plan_options.extra_args);
598+
context.set_parent_query(Arc::clone(&query));
599+
578600
// Query matching tasks from the task graph.
579601
// An empty graph means no tasks matched; the caller (session) handles
580602
// empty graphs by showing the task selector.
581-
let task_query_result = context.indexed_task_graph().query_tasks(&query_plan_request.query)?;
603+
let task_query_result = context.indexed_task_graph().query_tasks(&query)?;
582604

583605
#[expect(clippy::print_stderr, reason = "user-facing warning for typos in --filter")]
584606
for selector in &task_query_result.unmatched_selectors {
@@ -587,6 +609,12 @@ pub async fn plan_query_request(
587609

588610
let task_node_index_graph = task_query_result.execution_graph;
589611

612+
// Rule 2: if the expanding task appears in the expansion, prune it.
613+
// This handles cases like root `"build": "vp run build"` — the root's build
614+
// task is in the result but expanding it would recurse, so we remove it and
615+
// reconnect its predecessors directly to its successors.
616+
let pruned_task = context.expanding_task().filter(|t| task_node_index_graph.contains_node(*t));
617+
590618
let mut execution_node_indices_by_task_index =
591619
FxHashMap::<TaskNodeIndex, ExecutionNodeIndex>::with_capacity_and_hasher(
592620
task_node_index_graph.node_count(),
@@ -599,23 +627,48 @@ pub async fn plan_query_request(
599627
task_node_index_graph.edge_count(),
600628
);
601629

602-
// Plan each task node as execution nodes
630+
// Plan each task node as execution nodes, skipping the pruned task
603631
for task_index in task_node_index_graph.nodes() {
632+
if Some(task_index) == pruned_task {
633+
continue;
634+
}
604635
let task_execution =
605636
plan_task_as_execution_node(task_index, context.duplicate()).boxed_local().await?;
606637
execution_node_indices_by_task_index
607638
.insert(task_index, inner_graph.add_node(task_execution));
608639
}
609640

610-
// Add edges between execution nodes according to task dependencies
641+
// Add edges between execution nodes according to task dependencies,
642+
// skipping edges involving the pruned task.
611643
for (from_task_index, to_task_index, ()) in task_node_index_graph.all_edges() {
644+
if Some(from_task_index) == pruned_task || Some(to_task_index) == pruned_task {
645+
continue;
646+
}
612647
inner_graph.add_edge(
613648
execution_node_indices_by_task_index[&from_task_index],
614649
execution_node_indices_by_task_index[&to_task_index],
615650
(),
616651
);
617652
}
618653

654+
// Reconnect through the pruned node: wire each predecessor directly to each successor.
655+
if let Some(pruned) = pruned_task {
656+
let preds: Vec<_> =
657+
task_node_index_graph.neighbors_directed(pruned, Direction::Incoming).collect();
658+
let succs: Vec<_> =
659+
task_node_index_graph.neighbors_directed(pruned, Direction::Outgoing).collect();
660+
for &pred in &preds {
661+
for &succ in &succs {
662+
if let (Some(&pe), Some(&se)) = (
663+
execution_node_indices_by_task_index.get(&pred),
664+
execution_node_indices_by_task_index.get(&succ),
665+
) {
666+
inner_graph.add_edge(pe, se, ());
667+
}
668+
}
669+
}
670+
}
671+
619672
// Validate the graph is acyclic.
620673
// `try_from_graph` performs a DFS; if a cycle is found, it returns
621674
// `CycleError` containing the full cycle path as node indices.
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"name": "test-workspace",
3+
"private": true,
4+
"scripts": {
5+
"build": "vp run -r build",
6+
"lint": "echo linting"
7+
}
8+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"name": "@test/a",
3+
"version": "1.0.0",
4+
"scripts": {
5+
"build": "echo building-a",
6+
"lint": "echo linting-a"
7+
}
8+
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
packages:
2+
- 'packages/*'
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
# Tests that dependsOn works through a passthrough root task.
2+
# Root build = "vp run -r build", with dependsOn: ["lint"].
3+
# Rule 1 skips the recursive part, but the dependsOn lint tasks still run.
4+
5+
[[plan]]
6+
name = "depends on through passthrough"
7+
args = ["run", "-r", "build"]
8+
compact = true
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
source: crates/vite_task_plan/tests/plan_snapshots/main.rs
3+
expression: "&compact_plan"
4+
info:
5+
args:
6+
- run
7+
- "-r"
8+
- build
9+
input_file: crates/vite_task_plan/tests/plan_snapshots/fixtures/workspace-root-depends-on-passthrough
10+
---
11+
{
12+
"#build": [
13+
"#lint"
14+
],
15+
"#lint": [],
16+
"packages/a#build": []
17+
}

0 commit comments

Comments
 (0)