refactor(cli): dedupe interactivity gate and vite config probing, normalize -C paths

fengmk2 · fengmk2 · commit d131ea93ed93 · 2026-07-03T16:21:33.000+08:00
Post-review cleanup:

- hoist the CI/interactive-terminal gate into vite_shared and reuse it from
  the command picker, vite_install, and app-target elicitation (the new copy
  had drifted to 6 of 11 CI vars)
- expose vite_static_config::has_config_file and use it for runnable ranking
  (the hand-rolled list missed vite.config.cjs/.cts)
- normalize -C and defaultPackage joins with clean() so upward workspace
  walks never see ./ or .. components
- read defaultPackage before the workspace lookup so package.json-less
  framework roots (the RFC's motivating shape) work
- exempt -h/--help/-V/--version from elicitation so vp dev --help always
  reaches the tool
- accept -C=dir in bin.ts to match the clap grammar; simplify row
  construction, single-runnable check, and the target binding in mod.rs
diff --git a/crates/vite_global_cli/src/cli.rs b/crates/vite_global_cli/src/cli.rs
@@ -858,8 +858,9 @@ pub async fn run_command_with_options(
 ) -> 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.
     if let Some(dir) = &args.chdir {
-        cwd.push(dir);
+        cwd = cwd.join(dir).clean();
         if !cwd.as_path().is_dir() {
             return Err(Error::UserMessage(format!("directory not found: {dir}").into()));
         }
diff --git a/crates/vite_global_cli/src/command_picker.rs b/crates/vite_global_cli/src/command_picker.rs
@@ -1,7 +1,7 @@
 //! Interactive top-level command picker for `vp`.
 
 use std::{
-    io::{self, IsTerminal, Write},
+    io::{self, Write},
     ops::ControlFlow,
 };
 
@@ -114,20 +114,6 @@ const COMMANDS: &[CommandEntry] = &[
     },
 ];
 
-const CI_ENV_VARS: &[&str] = &[
-    "CI",
-    "CONTINUOUS_INTEGRATION",
-    "GITHUB_ACTIONS",
-    "GITLAB_CI",
-    "CIRCLECI",
-    "TRAVIS",
-    "JENKINS_URL",
-    "BUILDKITE",
-    "DRONE",
-    "CODEBUILD_BUILD_ID",
-    "TF_BUILD",
-];
-
 pub fn pick_top_level_command_if_interactive(
     cwd: &AbsolutePath,
 ) -> io::Result<TopLevelCommandPick> {
@@ -144,14 +130,7 @@ pub fn pick_top_level_command_if_interactive(
 }
 
 fn should_enable_picker() -> bool {
-    std::io::stdin().is_terminal()
-        && std::io::stdout().is_terminal()
-        && std::env::var("TERM").map_or(true, |term| term != "dumb")
-        && !is_ci_environment()
-}
-
-fn is_ci_environment() -> bool {
-    CI_ENV_VARS.iter().any(|key| std::env::var_os(key).is_some())
+    vite_shared::is_interactive_terminal()
 }
 
 fn run_picker(command_order: &[usize]) -> io::Result<Option<PickedCommand>> {
diff --git a/crates/vite_install/src/package_manager.rs b/crates/vite_install/src/package_manager.rs
@@ -1229,26 +1229,7 @@ async fn set_dev_engines_package_manager_field(
 }
 
 pub(crate) use vite_shared::format_path_prepended as format_path_env;
-
-/// Common CI environment variables
-const CI_ENV_VARS: &[&str] = &[
-    "CI",
-    "CONTINUOUS_INTEGRATION",
-    "GITHUB_ACTIONS",
-    "GITLAB_CI",
-    "CIRCLECI",
-    "TRAVIS",
-    "JENKINS_URL",
-    "BUILDKITE",
-    "DRONE",
-    "CODEBUILD_BUILD_ID", // AWS CodeBuild
-    "TF_BUILD",           // Azure Pipelines
-];
-
-/// Check if running in a CI environment
-fn is_ci_environment() -> bool {
-    CI_ENV_VARS.iter().any(|key| env::var(key).is_ok())
-}
+use vite_shared::is_ci_environment;
 
 /// Interactive menu for selecting a package manager with keyboard navigation
 fn interactive_package_manager_menu() -> Result<PackageManagerType, Error> {
diff --git a/crates/vite_shared/src/interactivity.rs b/crates/vite_shared/src/interactivity.rs
@@ -0,0 +1,32 @@
+//! Shared CI-environment and interactive-terminal detection.
+
+use std::io::IsTerminal;
+
+/// Common CI environment variables.
+const CI_ENV_VARS: &[&str] = &[
+    "CI",
+    "CONTINUOUS_INTEGRATION",
+    "GITHUB_ACTIONS",
+    "GITLAB_CI",
+    "CIRCLECI",
+    "TRAVIS",
+    "JENKINS_URL",
+    "BUILDKITE",
+    "DRONE",
+    "CODEBUILD_BUILD_ID", // AWS CodeBuild
+    "TF_BUILD",           // Azure Pipelines
+];
+
+/// Check if running in a CI environment.
+pub fn is_ci_environment() -> bool {
+    CI_ENV_VARS.iter().any(|key| std::env::var_os(key).is_some())
+}
+
+/// True when vp can show interactive prompts: stdin and stdout are terminals,
+/// the terminal is capable, and this is not a CI environment.
+pub fn is_interactive_terminal() -> bool {
+    std::io::stdin().is_terminal()
+        && std::io::stdout().is_terminal()
+        && std::env::var("TERM").map_or(true, |term| term != "dumb")
+        && !is_ci_environment()
+}
diff --git a/crates/vite_shared/src/lib.rs b/crates/vite_shared/src/lib.rs
@@ -11,6 +11,7 @@ mod env_config;
 pub mod env_vars;
 mod error;
 pub mod header;
+mod interactivity;
 mod home;
 mod http;
 mod json_edit;
@@ -24,6 +25,7 @@ mod tracing;
 pub use env_config::{EnvConfig, TestEnvGuard};
 pub use error::format_error_chain;
 pub use home::get_vp_home;
+pub use interactivity::{is_ci_environment, is_interactive_terminal};
 pub use http::shared_http_client;
 pub use json_edit::{JsonStyle, edit_json_object, insert_after};
 pub use package_json::{
diff --git a/crates/vite_static_config/src/lib.rs b/crates/vite_static_config/src/lib.rs
@@ -87,6 +87,11 @@ const CONFIG_FILE_NAMES: &[&str] = &[
     "vite.config.cts",
 ];
 
+/// Returns true when `dir` directly contains a Vite config file.
+pub fn has_config_file(dir: &AbsolutePath) -> bool {
+    resolve_config_path(dir).is_some()
+}
+
 /// Resolve the vite config file path in the given directory.
 ///
 /// Tries each config file name in priority order and returns the first one that exists.
diff --git a/packages/cli/binding/src/cli/app_target.rs b/packages/cli/binding/src/cli/app_target.rs
@@ -3,11 +3,9 @@
 //! A bare `vp dev`/`build`/`preview`/`pack` at a workspace root has no target
 //! and would silently run against the root. Resolution order (rfcs/cwd-flag.md):
 //! explicit `-C` and positional targets are handled before this code and skip
-//! elicitation entirely; then `defaultPackage` from the root config, then the
-//! workspace package listing (interactive picker planned once `vite_select`
-//! supports a custom prompt), then exit 1.
-
-use std::io::IsTerminal;
+//! elicitation entirely; then `defaultPackage` from the config in the
+//! invocation directory, then the workspace package listing (interactive
+//! picker planned once `vite_select` supports a custom prompt), then exit 1.
 
 use vite_error::Error;
 use vite_path::AbsolutePathBuf;
@@ -46,61 +44,49 @@ fn app_command_parts(subcommand: &SynthesizableSubcommand) -> Option<(&'static s
     }
 }
 
-/// Bare = no positional target. A non-flag token may be a flag value
-/// (`--port 3000`), so any non-flag argument conservatively disables
-/// elicitation and forwards the invocation unchanged.
+/// Bare = no positional target and no help-like flag. A non-flag token may be
+/// a flag value (`--port 3000`), so any non-flag argument conservatively
+/// disables elicitation; help/version requests are answered by the underlying
+/// tool and must never be redirected.
 fn is_bare(args: &[String]) -> bool {
-    args.iter().all(|arg| arg.starts_with('-'))
-}
-
-/// Mirror of the global command picker's interactivity gate.
-fn is_interactive() -> bool {
-    const CI_ENV_VARS: &[&str] =
-        &["CI", "CONTINUOUS_INTEGRATION", "GITHUB_ACTIONS", "GITLAB_CI", "BUILDKITE", "JENKINS_URL"];
-    std::io::stdin().is_terminal()
-        && std::io::stdout().is_terminal()
-        && std::env::var("TERM").map_or(true, |term| term != "dumb")
-        && !CI_ENV_VARS.iter().any(|key| std::env::var_os(key).is_some())
+    args.iter().all(|arg| {
+        arg.starts_with('-') && !matches!(arg.as_str(), "-h" | "--help" | "-V" | "--version")
+    })
 }
 
 /// Heuristic ranking signal: does `dir` look runnable for `command`?
 /// Used for ordering and single-candidate auto-selection, never for hiding.
 fn looks_runnable(dir: &AbsolutePathBuf, command: &str) -> bool {
-    const VITE_CONFIGS: &[&str] =
-        &["vite.config.ts", "vite.config.js", "vite.config.mts", "vite.config.mjs"];
-    let has_vite_config = VITE_CONFIGS.iter().any(|name| dir.as_path().join(name).is_file());
+    let has_vite_config = vite_static_config::has_config_file(dir);
     match command {
         "pack" => has_vite_config || dir.as_path().join("src/index.ts").is_file(),
         _ => has_vite_config || dir.as_path().join("index.html").is_file(),
     }
 }
 
-/// `defaultPackage` from the root `vite.config.*`, 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.
+/// `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.
 fn resolve_default_package(command: &str, cwd: &AbsolutePathBuf) -> Option<AppTarget> {
-    let fields = vite_static_config::resolve_static_config(cwd);
-    match fields.get("defaultPackage") {
+    let fail = |msg: &str| {
+        output::error(msg);
+        Some(AppTarget::Exit(ExitStatus(1)))
+    };
+    match vite_static_config::resolve_static_config(cwd).get("defaultPackage") {
         Some(vite_static_config::FieldValue::Json(serde_json::Value::String(dir))) => {
-            let mut target = cwd.clone();
-            target.push(dir.trim_start_matches("./"));
+            let target = cwd.join(&dir).clean();
             if !target.as_path().is_dir() {
-                output::error(&format!("defaultPackage points to a missing directory: {dir}"));
-                return Some(AppTarget::Exit(ExitStatus(1)));
+                return fail(&format!("defaultPackage points to a missing directory: {dir}"));
             }
             output::note(&format!("vp {command}: using {dir} (defaultPackage)"));
             Some(AppTarget::Dir(target))
         }
         Some(vite_static_config::FieldValue::Json(other)) => {
-            output::error(&format!("defaultPackage must be a string of a directory, got: {other}"));
-            Some(AppTarget::Exit(ExitStatus(1)))
-        }
-        Some(vite_static_config::FieldValue::NonStatic) => {
-            output::error(
-                "defaultPackage in vite.config.ts must be a static string literal so vp can read it without executing the config",
-            );
-            Some(AppTarget::Exit(ExitStatus(1)))
+            fail(&format!("defaultPackage must be a string of a directory, got: {other}"))
         }
+        Some(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,
     }
 }
@@ -116,18 +102,21 @@ pub(super) fn resolve_app_target(
         return Ok(AppTarget::CurrentDir);
     }
 
-    let (workspace_root, rel_from_root) = vite_workspace::find_workspace_root(cwd)?;
-    if !rel_from_root.as_str().is_empty() {
-        return Ok(AppTarget::CurrentDir);
-    }
-
+    // `defaultPackage` comes before any workspace lookup: the non-workspace
+    // framework shape (a Laravel-style root with a vite.config.ts pointer and
+    // no package.json up-tree) has no workspace metadata at all.
     if let Some(target) = resolve_default_package(command, cwd) {
         return Ok(target);
     }
 
-    // Only real workspaces have a package list to offer; a standalone package
-    // root keeps today's behavior.
-    if matches!(workspace_root.workspace_file, WorkspaceFile::NonWorkspacePackage(_)) {
+    // The package listing needs workspace metadata; anything unresolvable
+    // keeps today's behavior (the caller surfaces its own workspace errors).
+    let Ok((workspace_root, rel_from_root)) = vite_workspace::find_workspace_root(cwd) else {
+        return Ok(AppTarget::CurrentDir);
+    };
+    if !rel_from_root.as_str().is_empty()
+        || matches!(workspace_root.workspace_file, WorkspaceFile::NonWorkspacePackage(_))
+    {
         return Ok(AppTarget::CurrentDir);
     }
 
@@ -136,25 +125,26 @@ pub(super) fn resolve_app_target(
     let mut rows: Vec<PackageRow> = graph
         .node_weights()
         .filter(|info| !info.path.as_str().is_empty())
-        .map(|info| PackageRow {
-            name: info.package_json.name.to_string(),
-            path: info.path.as_str().to_string(),
-            absolute: info.absolute_path.to_absolute_path_buf(),
-            runnable: false,
+        .map(|info| {
+            let absolute = info.absolute_path.to_absolute_path_buf();
+            PackageRow {
+                name: info.package_json.name.to_string(),
+                path: info.path.as_str().to_string(),
+                runnable: looks_runnable(&absolute, command),
+                absolute,
+            }
         })
         .collect();
     if rows.is_empty() {
         return Ok(AppTarget::CurrentDir);
     }
-    for row in &mut rows {
-        row.runnable = looks_runnable(&row.absolute, command);
-    }
     rows.sort_by(|a, b| (!a.runnable, a.path.as_str()).cmp(&(!b.runnable, b.path.as_str())));
 
-    // With exactly one likely-runnable package, an interactive terminal
-    // auto-selects it (the degenerate picker).
-    if is_interactive() && rows.iter().filter(|row| row.runnable).count() == 1 {
-        let row = rows.iter().find(|row| row.runnable).expect("one runnable row exists");
+    // With exactly one likely-runnable package (rows are sorted runnable
+    // first), an interactive terminal auto-selects it (the degenerate picker).
+    let single_runnable = rows[0].runnable && rows.get(1).is_none_or(|row| !row.runnable);
+    if single_runnable && vite_shared::is_interactive_terminal() {
+        let row = &rows[0];
         println!("Selected package: {} ({})", row.name, row.path);
         println!("Tip: run this directly with `vp -C {} {command}`", row.path);
         return Ok(AppTarget::Dir(row.absolute.clone()));
@@ -168,7 +158,7 @@ pub(super) fn resolve_app_target(
         output::raw_stderr(&format!("    {:<name_width$}  {}", row.name, row.path));
     }
     output::raw_stderr("");
-    let example = &rows.first().expect("rows is non-empty").path;
+    let example = &rows[0].path;
     output::raw_stderr(&format!("  Pass a directory:  vp -C {example} {command}"));
     output::raw_stderr(&format!("  Or run every package's {command} script:  vp run -r {command}"));
     Ok(AppTarget::Exit(ExitStatus(1)))
@@ -179,7 +169,7 @@ mod tests {
     use super::*;
 
     #[test]
-    fn bare_means_no_positional_target() {
+    fn bare_means_no_positional_target_and_no_help() {
         let to_args = |args: &[&str]| args.iter().map(|s| (*s).to_string()).collect::<Vec<_>>();
         assert!(is_bare(&to_args(&[])));
         assert!(is_bare(&to_args(&["--watch"])));
@@ -189,6 +179,10 @@ mod tests {
         // A flag value is indistinguishable from a positional without knowing
         // the tool's flag arity, so it conservatively counts as non-bare.
         assert!(!is_bare(&to_args(&["--port", "3000"])));
+        // Help/version requests go to the underlying tool, never elicitation.
+        assert!(!is_bare(&to_args(&["--help"])));
+        assert!(!is_bare(&to_args(&["-h"])));
+        assert!(!is_bare(&to_args(&["--watch", "--version"])));
     }
 
     #[test]
diff --git a/packages/cli/binding/src/cli/mod.rs b/packages/cli/binding/src/cli/mod.rs
@@ -50,12 +50,14 @@ async fn execute_direct_subcommand(
     // A bare app command at a workspace root resolves its target first
     // (defaultPackage, package listing); the command then runs as if invoked
     // in the resolved directory (rfcs/cwd-flag.md).
-    let elicited_cwd = match app_target::resolve_app_target(&subcommand, cwd)? {
-        app_target::AppTarget::CurrentDir => None,
-        app_target::AppTarget::Dir(dir) => Some(dir),
-        app_target::AppTarget::Exit(status) => return Ok(status),
+    let target = app_target::resolve_app_target(&subcommand, cwd)?;
+    if let app_target::AppTarget::Exit(status) = target {
+        return Ok(status);
+    }
+    let cwd = match &target {
+        app_target::AppTarget::Dir(dir) => dir,
+        _ => cwd,
     };
-    let cwd = elicited_cwd.as_ref().unwrap_or(cwd);
 
     let (workspace_root, _) = vite_workspace::find_workspace_root(cwd)?;
     let workspace_path: Arc<AbsolutePath> = workspace_root.path.into();
diff --git a/packages/cli/src/bin.ts b/packages/cli/src/bin.ts
@@ -42,9 +42,10 @@ let args = process.argv.slice(2);
 // Global `-C <dir>` flag: run as if vp was started in <dir>. The global Rust
 // CLI parses this itself and spawns bin.js with the target cwd already set;
 // this branch covers direct local-bin invocations (`pnpm exec vp -C <dir> ...`).
-if (args[0] === '-C' || (args[0]?.startsWith('-C') && args[0].length > 2)) {
-  const inline = args[0] !== '-C';
-  const dir = inline ? args[0].slice(2) : args[1];
+// Accepts `-C dir`, `-Cdir`, and `-C=dir`, matching the clap grammar.
+if (args[0]?.startsWith('-C')) {
+  const inline = args[0].length > 2;
+  const dir = inline ? args[0].slice(args[0][2] === '=' ? 3 : 2) : args[1];
   if (!dir) {
     errorMsg('-C requires a directory argument');
     process.exit(1);
