refactor(cli): apply cleanup review across the -C and elicitation code

- one apply_chdir helper (join/clean/validate/chdir/PWD) replaces the four
  drifting copies in vite_global_cli: main's normal path, the completion
  peek, vpr, and the clap-path fallback in cli.rs — which previously
  skipped set_current_dir and the PWD sync, silently weakening the cd
  equivalence on orderings like a second -C
- one classify() core (bare -> defaultPackage-at-root -> workspace root)
  now feeds both resolve_app_target and needs_elicitation, so the RFC's
  resolution order is written once and the two entry points cannot drift;
  the extracted defaultPackage value rides the enum, removing a duplicate
  config parse
- bin/vpr is a grammar-free shim: bin.ts inserts 'run' itself via argv0
  after its existing -C consumption, deleting the hand-synchronized second
  JS copy of the -C token grammar
- pack runnable check stats src/index.ts before parsing the config (runs
  per workspace package); PWD in the direct-subcommand env is only
  overridden when elicitation retargeted (plain runs keep the caller's
  possibly-symlinked PWD verbatim); vpr parses -C once; identical
  FieldMapInner arms collapse via or-patterns; handler's -C predicate
  simplifies to starts_with

Author: fengmk2

crates/vite_global_cli/src/cli.rs

@@ -856,14 +856,13 @@ pub async fn run_command_with_options(
     args: Args,
     render_options: RenderOptions,
 ) -> Result<ExitStatus, Error> {
-    // Apply the global `-C <dir>` flag before anything reads cwd, so local CLI
-    // resolution and command execution behave as if vp was started in <dir>.
-    // `clean()` normalizes `.`/`..` so upward workspace walks never see them.
+    // Apply the global `-C <dir>` flag before anything reads cwd. main
+    // normally consumes a leading `-C` pre-parse; this covers orderings that
+    // reach clap (e.g. a second `-C`), with identical semantics — including
+    // the process-cwd change and PWD sync the shared helper performs.
     if let Some(dir) = &args.chdir {
-        cwd = cwd.join(dir).clean();
-        if !cwd.as_path().is_dir() {
-            return Err(Error::UserMessage(format!("directory not found: {dir}").into()));
-        }
+        cwd =
+            crate::apply_chdir(&cwd, dir).map_err(|message| Error::UserMessage(message.into()))?;
     }
 
     // Handle --version flag (Category B: delegates to JS)

crates/vite_global_cli/src/commands/vpr.rs

@@ -11,33 +11,26 @@ use vite_shared::output;
 /// Called from shim dispatch when `argv[0]` is `vpr`.
 pub async fn execute_vpr(args: &[String], cwd: &AbsolutePath) -> i32 {
     // `vpr -C <dir> <task>` mirrors `vp -C <dir> run <task>`: consume the
-    // global flag before treating the rest as run arguments.
+    // global flag before treating the rest as run arguments. There is no
+    // clap parse on this path, so a missing value is reported here.
     let mut args = args;
     let mut cwd_buf = cwd.to_absolute_path_buf();
-    // A bare `-C` with no value must error like the vp binary would, not
-    // fall through as a run argument (there is no clap parse on this path).
-    if args.first().is_some_and(|arg| matches!(arg.as_str(), "-C" | "-C="))
-        && crate::parse_leading_chdir(args).is_none()
-    {
-        output::raw_stderr("-C requires a directory argument");
-        return 1;
-    }
-    if let Some((dir, consumed)) = crate::parse_leading_chdir(args) {
-        cwd_buf = cwd_buf.join(&dir).clean();
-        if !cwd_buf.as_path().is_dir() {
-            output::raw_stderr(&format!("directory not found: {dir}"));
-            return 1;
+    match crate::parse_leading_chdir(args) {
+        Some((dir, consumed)) => {
+            cwd_buf = match crate::apply_chdir(cwd, &dir) {
+                Ok(target) => target,
+                Err(message) => {
+                    output::raw_stderr(&message);
+                    return 1;
+                }
+            };
+            args = &args[consumed..];
         }
-        if std::env::set_current_dir(cwd_buf.as_path()).is_err() {
-            output::error(&format!("Failed to change directory to {dir}"));
+        None if args.first().is_some_and(|arg| arg.starts_with("-C")) => {
+            output::raw_stderr("-C requires a directory argument");
             return 1;
         }
-        #[cfg(unix)]
-        // SAFETY: single-threaded startup, before any command logic runs.
-        unsafe {
-            std::env::set_var("PWD", cwd_buf.as_path());
-        }
-        args = &args[consumed..];
+        None => {}
     }
 
     if crate::help::maybe_print_unified_delegate_help("run", args, true) {

crates/vite_global_cli/src/main.rs

@@ -62,6 +62,30 @@ pub(crate) fn parse_leading_chdir(user_args: &[String]) -> Option<(String, usize
 /// - `vp rebuild ...` → `vp pm rebuild ...`
 /// - `vp help [command] [args...]` → `vp [command] [args...] --help`
 /// - `vp node [args...]` → `vp env exec node [args...]`
+/// Apply `-C <dir>`: resolve against `cwd`, validate, change the process
+/// cwd, and keep the POSIX `PWD` in sync like a real `cd`. Returns the new
+/// cwd, or the user-facing error message (not printed here: the completion
+/// peek must stay silent, and clap-path callers wrap it in their error type).
+pub(crate) fn apply_chdir(
+    cwd: &vite_path::AbsolutePath,
+    dir: &str,
+) -> Result<vite_path::AbsolutePathBuf, String> {
+    let target = cwd.join(dir).clean();
+    if !target.as_path().is_dir() {
+        return Err(format!("directory not found: {dir}"));
+    }
+    if let Err(e) = std::env::set_current_dir(target.as_path()) {
+        return Err(format!("Failed to change directory to {dir}: {e}"));
+    }
+    // Node tools commonly read process.env.PWD.
+    #[cfg(unix)]
+    // SAFETY: single-threaded startup, before any command logic runs.
+    unsafe {
+        std::env::set_var("PWD", target.as_path());
+    }
+    Ok(target)
+}
+
 fn normalize_args(args: Vec<String>) -> Vec<String> {
     let mut normalized = args;
     loop {
@@ -333,10 +357,8 @@ async fn main() -> ExitCode {
         && let Some((dir, _)) = parse_leading_chdir(&args[1..])
         && let Ok(current) = vite_path::current_dir()
     {
-        let target = current.join(&dir).clean();
-        if target.as_path().is_dir() {
-            let _ = std::env::set_current_dir(target.as_path());
-        }
+        // Best-effort and silent: mid-typing values are often not (yet) dirs.
+        let _ = apply_chdir(&current, &dir);
     }
     CompleteEnv::with_factory(command_with_help).var("VP_COMPLETE").complete();
 
@@ -367,22 +389,13 @@ async fn main() -> ExitCode {
     // cwd (before any command logic runs) keeps `vite_path::current_dir()`
     // callers deep inside command implementations equivalent to the cd form.
     if let Some((dir, consumed)) = parse_leading_chdir(&args[1..]) {
-        cwd = cwd.join(&dir).clean();
-        if !cwd.as_path().is_dir() {
-            output::raw_stderr(&format!("directory not found: {dir}"));
-            return ExitCode::FAILURE;
-        }
-        if let Err(e) = std::env::set_current_dir(cwd.as_path()) {
-            output::error(&format!("Failed to change directory to {dir}: {e}"));
-            return ExitCode::FAILURE;
-        }
-        // Keep the POSIX PWD in sync, like a real `cd`: Node tools commonly
-        // read process.env.PWD.
-        #[cfg(unix)]
-        // SAFETY: single-threaded startup, before any command logic runs.
-        unsafe {
-            std::env::set_var("PWD", cwd.as_path());
-        }
+        cwd = match apply_chdir(&cwd, &dir) {
+            Ok(target) => target,
+            Err(message) => {
+                output::raw_stderr(&message);
+                return ExitCode::FAILURE;
+            }
+        };
         args.drain(1..=consumed);
     }
 

crates/vite_static_config/src/lib.rs

@@ -79,10 +79,8 @@ impl FieldMap {
     /// unanalyzable config would break unrelated commands).
     #[must_use]
     pub fn get_declared(&self, key: &str) -> Option<FieldValue> {
-        match &self.0 {
-            FieldMapInner::Closed(map) => map.get(key).cloned(),
-            FieldMapInner::Open(map) => map.get(key).cloned(),
-        }
+        let (FieldMapInner::Closed(map) | FieldMapInner::Open(map)) = &self.0;
+        map.get(key).cloned()
     }
 }
 
@@ -347,21 +345,12 @@ fn extract_object_fields(obj: &oxc_ast::ast::ObjectExpression<'_>) -> FieldMap {
             continue;
         };
 
-        match &mut inner {
-            FieldMapInner::Closed(map) => {
-                let value =
-                    expr_to_json(&prop.value).map_or(FieldValue::NonStatic, FieldValue::Json);
-                map.insert(Box::from(key.as_ref()), value);
-            }
-            FieldMapInner::Open(map) => {
-                // Record explicit declarations, including NonStatic ones:
-                // `get_declared` must distinguish a written `key: expr` from a
-                // key that merely might exist behind the spread.
-                let value =
-                    expr_to_json(&prop.value).map_or(FieldValue::NonStatic, FieldValue::Json);
-                map.insert(Box::from(key.as_ref()), value);
-            }
-        }
+        // Both variants record explicit declarations, including NonStatic
+        // ones: `get_declared` must distinguish a written `key: expr` from a
+        // key that merely might exist behind a spread.
+        let (FieldMapInner::Closed(map) | FieldMapInner::Open(map)) = &mut inner;
+        let value = expr_to_json(&prop.value).map_or(FieldValue::NonStatic, FieldValue::Json);
+        map.insert(Box::from(key.as_ref()), value);
     }
 
     FieldMap(inner)

packages/cli/bin/vpr

@@ -4,19 +4,4 @@ import module from 'node:module';
 if (module.enableCompileCache) {
   module.enableCompileCache();
 }
-
-// Insert `run` after a leading global `-C <dir>` so `vpr -C <dir> <task>`
-// behaves like `vp -C <dir> run <task>` (bin.ts consumes -C before dispatch).
-// A bare `vpr -C` (missing value) inserts nothing: bin.ts then reports the
-// missing-argument error instead of misreading `run` as the directory.
-const first = process.argv[2];
-if (first === '-C') {
-  if (process.argv[3] !== undefined) {
-    process.argv.splice(4, 0, 'run');
-  }
-} else if (first?.startsWith('-C') && first.length > 2) {
-  process.argv.splice(3, 0, 'run');
-} else {
-  process.argv.splice(2, 0, 'run');
-}
-await import('../dist/bin.js');
+import '../dist/bin.js';

packages/cli/binding/src/cli/app_target.rs

@@ -134,47 +134,49 @@ fn is_bare(command: &str, args: &[String]) -> bool {
 /// silent root build this feature exists to prevent.
 fn looks_runnable(dir: &AbsolutePathBuf, command: &str, is_root: bool) -> bool {
     match command {
-        // Bare `vp pack` succeeds when the config explicitly declares a
-        // `pack` block or tsdown's default entry exists. A spread that only
+        // Bare `vp pack` succeeds when tsdown's default entry exists or the
+        // config explicitly declares a `pack` block (a spread that only
         // might contain `pack` does not count: auto-select acts on this
-        // signal, so a false positive runs tsdown in a non-packable package.
+        // signal, so a false positive runs tsdown in a non-packable
+        // package). The one-stat entry check runs first: this executes per
+        // workspace package, and the config check reads and parses a file.
         "pack" => {
-            vite_static_config::resolve_static_config(dir).get_declared("pack").is_some()
-                || dir.as_path().join("src/index.ts").is_file()
+            dir.as_path().join("src/index.ts").is_file()
+                || vite_static_config::resolve_static_config(dir).get_declared("pack").is_some()
         }
         _ if is_root => dir.as_path().join("index.html").is_file(),
         _ => vite_static_config::has_config_file(dir) || dir.as_path().join("index.html").is_file(),
     }
 }
 
-/// `defaultPackage` from the `vite.config.*` in `cwd`, read via static
-/// extraction so it works at roots without a vite-plus install (non-workspace
-/// framework repos). The value must be a static string literal.
-///
-/// `get_declared` keeps this to explicitly written fields: a config that is
-/// unanalyzable or hides fields behind a spread simply falls through to the
-/// picker/current-dir resolution instead of failing every bare app command.
-fn resolve_default_package(command: &str, cwd: &AbsolutePathBuf) -> Option<AppTarget> {
+/// Resolve the `defaultPackage` value [`classify`] extracted from the
+/// invocation root's `vite.config.*` (static extraction, so it works at
+/// roots without a vite-plus install). The value must be a static string
+/// literal naming an existing directory.
+fn resolve_default_package(
+    command: &str,
+    cwd: &AbsolutePathBuf,
+    value: vite_static_config::FieldValue,
+) -> AppTarget {
     let fail = |msg: &str| {
         output::error(msg);
-        Some(AppTarget::Exit(ExitStatus(1)))
+        AppTarget::Exit(ExitStatus(1))
     };
-    match vite_static_config::resolve_static_config(cwd).get_declared("defaultPackage") {
-        Some(vite_static_config::FieldValue::Json(serde_json::Value::String(dir))) => {
+    match value {
+        vite_static_config::FieldValue::Json(serde_json::Value::String(dir)) => {
             let target = cwd.join(&dir).clean();
             if !target.as_path().is_dir() {
                 return fail(&format!("defaultPackage points to a missing directory: {dir}"));
             }
             output::note(&format!("vp {command}: using {dir} (defaultPackage)"));
-            Some(AppTarget::Dir(target))
+            AppTarget::Dir(target)
         }
-        Some(vite_static_config::FieldValue::Json(other)) => {
+        vite_static_config::FieldValue::Json(other) => {
             fail(&format!("defaultPackage must be a string of a directory, got: {other}"))
         }
-        Some(vite_static_config::FieldValue::NonStatic) => fail(
+        vite_static_config::FieldValue::NonStatic => fail(
             "defaultPackage in vite.config.ts must be a static string literal so vp can read it without executing the config",
         ),
-        None => None,
     }
 }
 
@@ -239,67 +241,68 @@ pub(super) fn needs_elicitation(
     subcommand: &SynthesizableSubcommand,
     cwd: &AbsolutePathBuf,
 ) -> bool {
-    let Some((command, args)) = app_command_parts(subcommand) else {
-        return false;
-    };
-    if !is_bare(command, args) {
-        return false;
-    }
-    let workspace = vite_workspace::find_workspace_root(cwd);
-    if at_invocation_root(workspace.as_ref().ok().map(|(_, rel)| rel.as_str()))
-        && vite_static_config::resolve_static_config(cwd).get_declared("defaultPackage").is_some()
-    {
-        return true;
-    }
-    let Ok((workspace_root, rel_from_root)) = workspace else {
-        return false;
-    };
-    rel_from_root.as_str().is_empty()
-        && !matches!(workspace_root.workspace_file, WorkspaceFile::NonWorkspacePackage(_))
+    classify(subcommand, cwd).is_some()
 }
 
-/// `defaultPackage` is a root-pointer concept: it applies where the
-/// invocation directory is its own root (a workspace root, a standalone
-/// package, or a framework directory with no package.json ancestry — pass
-/// the workspace lookup's `rel_from_root`, or `None` when the lookup
-/// failed). Below a workspace root the current directory already identifies
-/// the target package, so a member's own config must not redirect.
-fn at_invocation_root(rel_from_root: Option<&str>) -> bool {
-    rel_from_root.is_none_or(str::is_empty)
+/// Why a bare app command needs target elicitation.
+enum Elicitation {
+    /// The invocation root's config explicitly declares `defaultPackage`
+    /// (with this value — possibly invalid, which the resolver reports).
+    DefaultPackage(vite_static_config::FieldValue),
+    /// Bare app command at a real workspace root: picker/listing territory.
+    WorkspaceRoot(vite_workspace::WorkspaceRoot),
 }
 
-pub(super) fn resolve_app_target(
+/// The RFC's resolution order, written once for both entry points: bare app
+/// command, then `defaultPackage` at the invocation root, then the workspace
+/// root itself. `defaultPackage` is a root-pointer concept: it applies where
+/// the invocation directory is its own root (a workspace root, a standalone
+/// package, or a framework directory with no package.json ancestry); below a
+/// workspace root the current directory already identifies the target, so a
+/// member's own config must not redirect.
+fn classify(
     subcommand: &SynthesizableSubcommand,
     cwd: &AbsolutePathBuf,
-) -> Result<AppTarget, Error> {
-    let Some((command, args)) = app_command_parts(subcommand) else {
-        return Ok(AppTarget::CurrentDir);
-    };
+) -> Option<(&'static str, Elicitation)> {
+    let (command, args) = app_command_parts(subcommand)?;
     if !is_bare(command, args) {
-        return Ok(AppTarget::CurrentDir);
+        return None;
     }
-
-    // `defaultPackage` is consulted before the workspace-shape dispatch (the
-    // non-workspace framework shape has no workspace metadata at all), but
-    // only at the invocation root: a member package's config must not
-    // redirect a command already running in that member.
     let workspace = vite_workspace::find_workspace_root(cwd);
-    if at_invocation_root(workspace.as_ref().ok().map(|(_, rel)| rel.as_str()))
-        && let Some(target) = resolve_default_package(command, cwd)
+    let at_invocation_root =
+        workspace.as_ref().map_or(true, |(_, rel_from_root)| rel_from_root.as_str().is_empty());
+    if at_invocation_root
+        && let Some(value) =
+            vite_static_config::resolve_static_config(cwd).get_declared("defaultPackage")
     {
-        return Ok(target);
+        return Some((command, Elicitation::DefaultPackage(value)));
     }
-
-    // The package listing needs workspace metadata; anything unresolvable
+    // The picker/listing needs workspace metadata; anything unresolvable
     // keeps today's behavior (the caller surfaces its own workspace errors).
     let Ok((workspace_root, rel_from_root)) = workspace else {
-        return Ok(AppTarget::CurrentDir);
+        return None;
     };
     if !rel_from_root.as_str().is_empty()
         || matches!(workspace_root.workspace_file, WorkspaceFile::NonWorkspacePackage(_))
     {
-        return Ok(AppTarget::CurrentDir);
+        return None;
     }
+    Some((command, Elicitation::WorkspaceRoot(workspace_root)))
+}
+
+pub(super) fn resolve_app_target(
+    subcommand: &SynthesizableSubcommand,
+    cwd: &AbsolutePathBuf,
+) -> Result<AppTarget, Error> {
+    let Some((command, elicitation)) = classify(subcommand, cwd) else {
+        return Ok(AppTarget::CurrentDir);
+    };
+    let workspace_root = match elicitation {
+        Elicitation::DefaultPackage(value) => {
+            return Ok(resolve_default_package(command, cwd, value));
+        }
+        Elicitation::WorkspaceRoot(workspace_root) => workspace_root,
+    };
 
     let graph =
         vite_workspace::load_package_graph(&workspace_root).map_err(|e| Error::Anyhow(e.into()))?;