fix(plan): normalize command arrays through existing shell path

Join command array entries with && before planning so string[] shorthands preserve existing shell semantics instead of relying on partial cwd-change detection.

Author: jong-kyung

crates/vite_shell/src/lib.rs

@@ -4,8 +4,8 @@ use brush_parser::{
     Parser, ParserOptions,
     ast::{
         AndOr, Assignment, AssignmentName, AssignmentValue, Command, CommandPrefix,
-        CommandPrefixOrSuffixItem, CommandSuffix, CompoundCommand, CompoundList, CompoundListItem,
-        DoGroupCommand, Pipeline, Program, SeparatorOperator, SimpleCommand, SourceLocation, Word,
+        CommandPrefixOrSuffixItem, CommandSuffix, CompoundListItem, Pipeline, Program,
+        SeparatorOperator, SimpleCommand, SourceLocation, Word,
     },
     word::{WordPiece, WordPieceWithSource},
 };
@@ -135,139 +135,6 @@ fn pipeline_to_command(pipeline: &Pipeline) -> Option<(TaskParsedCommand, Range<
     Some((TaskParsedCommand { envs, program: unquote(program)?, args }, range))
 }
 
-#[derive(Clone, Copy, Debug, PartialEq, Eq)]
-enum CwdChangingCommand {
-    Cd,
-    Chdir,
-    Dot,
-    Source,
-    Eval,
-}
-
-impl CwdChangingCommand {
-    fn from_name(command_name: &str) -> Option<Self> {
-        Some(match command_name {
-            "cd" => Self::Cd,
-            "chdir" => Self::Chdir,
-            "." => Self::Dot,
-            "source" => Self::Source,
-            "eval" => Self::Eval,
-            _ => return None,
-        })
-    }
-}
-
-fn command_name_may_change_cwd(command_name: &str) -> bool {
-    CwdChangingCommand::from_name(command_name).is_some()
-}
-
-fn wrapper_command_may_change_cwd(suffix: Option<&CommandSuffix>) -> bool {
-    let Some(CommandSuffix(suffix_items)) = suffix else {
-        return false;
-    };
-
-    for suffix_item in suffix_items {
-        let CommandPrefixOrSuffixItem::Word(word) = suffix_item else {
-            continue;
-        };
-        let Some(arg) = unquote(word) else {
-            return true;
-        };
-        if arg == "--" || arg.starts_with('-') {
-            continue;
-        }
-        return command_name_may_change_cwd(arg.as_str());
-    }
-
-    false
-}
-
-fn simple_command_may_change_cwd(simple_command: &SimpleCommand) -> bool {
-    let Some(program) = &simple_command.word_or_name else {
-        return false;
-    };
-    let Some(program) = unquote(program) else {
-        return true;
-    };
-
-    match program.as_str() {
-        "command" | "builtin" | "time" => {
-            wrapper_command_may_change_cwd(simple_command.suffix.as_ref())
-        }
-        command_name => command_name_may_change_cwd(command_name),
-    }
-}
-
-fn and_or_list_may_change_cwd(and_or_list: &brush_parser::ast::AndOrList) -> bool {
-    pipeline_may_change_cwd(&and_or_list.first)
-        || and_or_list.additional.iter().any(|and_or| {
-            let pipeline = match and_or {
-                AndOr::And(pipeline) | AndOr::Or(pipeline) => pipeline,
-            };
-            pipeline_may_change_cwd(pipeline)
-        })
-}
-
-fn compound_list_may_change_cwd(compound_list: &CompoundList) -> bool {
-    compound_list
-        .0
-        .iter()
-        .any(|CompoundListItem(and_or_list, _)| and_or_list_may_change_cwd(and_or_list))
-}
-
-fn do_group_may_change_cwd(do_group: &DoGroupCommand) -> bool {
-    compound_list_may_change_cwd(&do_group.list)
-}
-
-fn compound_command_may_change_cwd(compound_command: &CompoundCommand) -> bool {
-    match compound_command {
-        CompoundCommand::Arithmetic(_) | CompoundCommand::Subshell(_) => false,
-        CompoundCommand::ArithmeticForClause(command) => do_group_may_change_cwd(&command.body),
-        CompoundCommand::BraceGroup(command) => compound_list_may_change_cwd(&command.list),
-        CompoundCommand::ForClause(command) => do_group_may_change_cwd(&command.body),
-        CompoundCommand::CaseClause(command) => command
-            .cases
-            .iter()
-            .any(|case| case.cmd.as_ref().is_some_and(compound_list_may_change_cwd)),
-        CompoundCommand::IfClause(command) => {
-            compound_list_may_change_cwd(&command.condition)
-                || compound_list_may_change_cwd(&command.then)
-                || command.elses.as_ref().is_some_and(|elses| {
-                    elses.iter().any(|else_clause| {
-                        else_clause.condition.as_ref().is_some_and(compound_list_may_change_cwd)
-                            || compound_list_may_change_cwd(&else_clause.body)
-                    })
-                })
-        }
-        CompoundCommand::WhileClause(command) | CompoundCommand::UntilClause(command) => {
-            compound_list_may_change_cwd(&command.0) || do_group_may_change_cwd(&command.1)
-        }
-    }
-}
-
-fn command_may_change_cwd(command: &Command) -> bool {
-    match command {
-        Command::Simple(simple_command) => simple_command_may_change_cwd(simple_command),
-        Command::Compound(compound_command, _) => compound_command_may_change_cwd(compound_command),
-        Command::Function(function) => compound_command_may_change_cwd(&function.body.0),
-        Command::ExtendedTest(..) => false,
-    }
-}
-
-fn pipeline_may_change_cwd(pipeline: &Pipeline) -> bool {
-    pipeline.seq.iter().any(command_may_change_cwd)
-}
-
-#[must_use]
-pub fn shell_command_may_change_cwd(cmd: &str) -> bool {
-    let mut parser = Parser::new(cmd.as_bytes(), &PARSER_OPTIONS);
-    let Ok(Program { complete_commands }) = parser.parse_program() else {
-        return true;
-    };
-
-    complete_commands.iter().any(compound_list_may_change_cwd)
-}
-
 #[must_use]
 pub fn try_parse_as_and_list(cmd: &str) -> Option<Vec<(TaskParsedCommand, Range<usize>)>> {
     let mut parser = Parser::new(cmd.as_bytes(), &PARSER_OPTIONS);
@@ -295,29 +162,6 @@ pub fn try_parse_as_and_list(cmd: &str) -> Option<Vec<(TaskParsedCommand, Range<
 mod tests {
     use super::*;
 
-    #[test]
-    fn test_shell_command_may_change_cwd_with_unresolved_arg() {
-        assert!(shell_command_may_change_cwd(r#"cd "$APP_DIR""#));
-        assert!(shell_command_may_change_cwd(r#"echo ok && cd "$APP_DIR""#));
-        assert!(shell_command_may_change_cwd(r#"FOO=bar 'cd' "$APP_DIR""#));
-    }
-
-    #[test]
-    fn test_shell_command_may_change_cwd_in_compound_commands() {
-        assert!(shell_command_may_change_cwd("if true; then cd snapshots; fi"));
-        assert!(shell_command_may_change_cwd("{ cd snapshots; }"));
-        assert!(shell_command_may_change_cwd("f(){ cd snapshots; }; f"));
-        assert!(shell_command_may_change_cwd("command cd snapshots"));
-        assert!(shell_command_may_change_cwd("time cd snapshots"));
-    }
-
-    #[test]
-    fn test_shell_command_may_change_cwd_ignores_non_current_shell_cd() {
-        assert!(!shell_command_may_change_cwd(r#"echo "cd $APP_DIR""#));
-        assert!(!shell_command_may_change_cwd("cdtool $APP_DIR"));
-        assert!(!shell_command_may_change_cwd("(cd snapshots)"));
-    }
-
     #[test]
     fn test_parse_single_command() {
         let source = r"A=B hello world";

crates/vite_task_plan/src/plan.rs

@@ -16,7 +16,7 @@ use futures_util::FutureExt;
 use petgraph::Direction;
 use rustc_hash::FxHashMap;
 use vite_path::{AbsolutePath, AbsolutePathBuf, RelativePathBuf, relative::InvalidPathDataError};
-use vite_shell::{TaskParsedCommand, shell_command_may_change_cwd, try_parse_as_and_list};
+use vite_shell::{TaskParsedCommand, try_parse_as_and_list};
 use vite_str::Str;
 use vite_task_graph::{
     TaskNodeIndex, TaskSource,
@@ -87,10 +87,9 @@ enum PlannedCommand {
 }
 
 #[expect(clippy::result_large_err, reason = "Error is large for diagnostics")]
-fn planned_commands(command: &TaskCommand) -> Result<Vec<PlannedCommand>, Error> {
-    let mut array_len = None;
-    let snippets: Box<dyn Iterator<Item = &Str> + '_> = match command {
-        TaskCommand::String(command) => Box::new(std::iter::once(command)),
+fn command_source(command: &TaskCommand) -> Result<Str, Error> {
+    match command {
+        TaskCommand::String(command) => Ok(command.clone()),
         TaskCommand::Array(commands) => {
             if commands.is_empty() {
                 return Err(Error::InvalidTaskCommand("command array must not be empty".into()));
@@ -100,36 +99,34 @@ fn planned_commands(command: &TaskCommand) -> Result<Vec<PlannedCommand>, Error>
                     "command array entries must not be empty".into(),
                 ));
             }
-            array_len = Some(commands.len());
-            Box::new(commands.iter())
-        }
-    };
 
-    let mut planned = Vec::new();
-    for (snippet_index, snippet) in snippets.enumerate() {
-        if let Some(parsed) = try_parse_as_and_list(snippet.as_str()) {
-            for (and_item, range) in parsed {
-                planned.push(PlannedCommand::Parsed {
-                    display: Str::from(&snippet.as_str()[range.clone()]),
-                    and_item,
-                    stack_frame: range,
-                });
-            }
-        } else {
-            // A shell fallback runs in a child shell, so any cwd change inside it cannot be
-            // reflected in the planner's cwd for following array entries.
-            if array_len.is_some_and(|len| snippet_index + 1 < len)
-                && shell_command_may_change_cwd(snippet.as_str())
-            {
-                return Err(Error::InvalidTaskCommand(
-                    "command array entries that may change directory in a shell must be the final entry"
-                        .into(),
-                ));
+            let mut source = Str::default();
+            for (index, command) in commands.iter().enumerate() {
+                if index > 0 {
+                    source.push_str(" && ");
+                }
+                source.push_str(command.as_str());
             }
-            planned.push(PlannedCommand::Shell(snippet.clone()));
+            Ok(source)
         }
     }
-    Ok(planned)
+}
+
+#[expect(clippy::result_large_err, reason = "Error is large for diagnostics")]
+fn planned_commands(command: &TaskCommand) -> Result<Vec<PlannedCommand>, Error> {
+    let source = command_source(command)?;
+    if let Some(parsed) = try_parse_as_and_list(source.as_str()) {
+        Ok(parsed
+            .into_iter()
+            .map(|(and_item, range)| PlannedCommand::Parsed {
+                display: Str::from(&source.as_str()[range.clone()]),
+                and_item,
+                stack_frame: range,
+            })
+            .collect())
+    } else {
+        Ok(vec![PlannedCommand::Shell(source)])
+    }
 }
 
 /// - `with_hooks`: whether to look up `preX`/`postX` lifecycle hooks for this task.

crates/vite_task_plan/tests/plan_snapshots/fixtures/task_command_shorthands/snapshots.toml

@@ -23,11 +23,11 @@ name = "array_cd_shell"
 args = ["run", "array_cd_shell"]
 
 [[plan]]
-name = "array_shell_cd_before_next_error"
+name = "array_shell_cd_before_next"
 args = ["run", "array_shell_cd_before_next"]
 
 [[plan]]
-name = "array_compound_cd_before_next_error"
+name = "array_compound_cd_before_next"
 args = ["run", "array_compound_cd_before_next"]
 
 [[plan]]

crates/vite_task_plan/tests/plan_snapshots/fixtures/task_command_shorthands/snapshots/query_array_cd_shell.jsonc

@@ -20,24 +20,24 @@
                 "task_name": "array_cd_shell",
                 "package_path": "<workspace>/"
               },
-              "command": "echo $PWD",
-              "and_item_index": 1,
-              "cwd": "<workspace>/snapshots"
+              "command": "cd snapshots && echo $PWD",
+              "and_item_index": null,
+              "cwd": "<workspace>/"
             },
             "kind": {
               "Leaf": {
                 "Spawn": {
                   "cache_metadata": {
                     "spawn_fingerprint": {
-                      "cwd": "snapshots",
+                      "cwd": "",
                       "program_fingerprint": {
                         "OutsideWorkspace": {
                           "program_name": "<os_shell_name>"
                         }
                       },
                       "args": [
                         "<os_shell_args>",
-                        "echo $PWD"
+                        "cd snapshots && echo $PWD"
                       ],
                       "env_fingerprints": {
                         "fingerprinted_envs": {},
@@ -49,7 +49,7 @@
                     "execution_cache_key": {
                       "UserTask": {
                         "task_name": "array_cd_shell",
-                        "and_item_index": 1,
+                        "and_item_index": 0,
                         "extra_args": [],
                         "package_path": ""
                       }
@@ -69,13 +69,13 @@
                     "program_path": "<os_shell_path>",
                     "args": [
                       "<os_shell_args>",
-                      "echo $PWD"
+                      "cd snapshots && echo $PWD"
                     ],
                     "all_envs": {
                       "FORCE_COLOR": "1",
                       "PATH": "<workspace>/node_modules/.bin:<tools>"
                     },
-                    "cwd": "<workspace>/snapshots"
+                    "cwd": "<workspace>/"
                   }
                 }
               }