fix(test): address Codex round-7 findings

The push trigger's paths-ignore becomes a paths list with negations so
baseline-only .md pushes still run CI. list-dir hides dot entries like
plain ls (--all opts in) and the migrator TODOs ls flags. vp/vpr/vpx
resolve through the case PATH first so VP_HOME/bin shims shadow the
harness aliases. A failing step now stops the case by default (shell-like
&& semantics) with continue-on-failure as the opt-out; the migrator
restores legacy semantics exactly by marking each command line's final
step. Bare runtime-tool versions (Node 24.x, pnpm 10.x from vp create)
redact by tool-name context while user semver stays assertable.

Claude-Session: https://claude.ai/code/session_01NRgjMi2Vus3iJctudGEWPT

Author: fengmk2

.github/workflows/ci.yml

@@ -11,8 +11,13 @@ on:
   push:
     branches:
       - main
-    paths-ignore:
-      - '**/*.md'
+    # `paths` with negations instead of paths-ignore: PTY snapshot baselines
+    # are .md files (crates/vite_cli_snapshots/**/snapshots/*.md) and a
+    # baseline-only push must still run CI. Later patterns win.
+    paths:
+      - '**'
+      - '!**/*.md'
+      - 'crates/vite_cli_snapshots/**'
 
 concurrency:
   group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.sha }}

crates/vite_cli_snapshots/src/bin/vpt/list_dir.rs

@@ -18,6 +18,7 @@ pub fn run(args: &[String]) -> Result<(), Box<dyn std::error::Error>> {
     let mut dir: Option<&str> = None;
     let mut ext: Option<&str> = None;
     let mut recursive = false;
+    let mut all = false;
     let mut i = 0;
     while i < args.len() {
         match args[i].as_str() {
@@ -26,12 +27,13 @@ pub fn run(args: &[String]) -> Result<(), Box<dyn std::error::Error>> {
                 ext = Some(args.get(i).ok_or("--ext requires a value")?.as_str());
             }
             "--recursive" => recursive = true,
+            "--all" => all = true,
             other if dir.is_none() => dir = Some(other),
             other => return Err(format!("unexpected argument: {other}").into()),
         }
         i += 1;
     }
-    let dir = dir.ok_or("Usage: vpt list-dir <dir> [--ext <suffix>] [--recursive]")?;
+    let dir = dir.ok_or("Usage: vpt list-dir <dir> [--ext <suffix>] [--recursive] [--all]")?;
 
     // Like `ls <file>`, a file target prints its own name; legacy fixtures
     // use that form as an existence assertion.
@@ -42,7 +44,7 @@ pub fn run(args: &[String]) -> Result<(), Box<dyn std::error::Error>> {
     }
 
     let mut names: Vec<String> = Vec::new();
-    collect(path, ext, recursive, &mut names)?;
+    collect(path, ext, recursive, all, &mut names)?;
     names.sort();
     for name in names {
         println!("{name}");
@@ -57,15 +59,22 @@ fn collect(
     dir: &std::path::Path,
     ext: Option<&str>,
     recursive: bool,
+    all: bool,
     names: &mut Vec<String>,
 ) -> Result<(), Box<dyn std::error::Error>> {
     for entry in std::fs::read_dir(dir)? {
         let entry = entry?;
+        let name = entry.file_name().to_string_lossy().into_owned();
+        // Plain ls hides dot entries; --all shows them. Keeping them hidden
+        // by default also keeps snapshots free of package-manager internals
+        // like .pnpm.
+        if !all && name.starts_with('.') {
+            continue;
+        }
         if recursive && entry.file_type()?.is_dir() {
-            collect(&entry.path(), ext, recursive, names)?;
+            collect(&entry.path(), ext, recursive, all, &mut *names)?;
             continue;
         }
-        let name = entry.file_name().to_string_lossy().into_owned();
         if let Some(suffix) = ext
             && !name.ends_with(suffix)
         {

crates/vite_cli_snapshots/tests/cli_snapshots/README.md

@@ -97,6 +97,9 @@ A step is a bare argv array or a table:
   timeout = 120000,           # ms, default 50s
   snapshot = false,           # omit the screen while the step succeeds
                               #   (failures always keep their output)
+  continue-on-failure = true, # a failing step normally stops the case
+                              #   (shell-like &&); this lets later steps
+                              #   inspect post-failure state
   tty = false,                # piped stdio instead of a PTY (non-TTY tests)
   interactions = [ ... ] }
 ```

crates/vite_cli_snapshots/tests/cli_snapshots/flavor.rs

@@ -285,23 +285,18 @@ impl FlavorRuntime {
         case_path: &std::ffi::OsStr,
     ) -> Result<PathBuf, String> {
         match program {
-            "vp" | "vpr" | "vpx" | "vpt" | "oxfmt" | "oxlint" => {
-                let name = if cfg!(windows) {
-                    // Installed as either .exe or .cmd; try both.
-                    ["exe", "cmd"]
-                        .iter()
-                        .map(|ext| self.bin_dir.join(format!("{program}.{ext}")))
-                        .find(|p| p.is_file())
-                        .ok_or_else(|| format!("`{program}` is not available in this flavor"))?
-                } else {
-                    let p = self.bin_dir.join(program);
-                    if !p.is_file() && !p.is_symlink() {
-                        return Err(format!("`{program}` is not available in this flavor"));
-                    }
-                    p
-                };
-                Ok(name)
+            "vp" | "vpr" | "vpx" | "oxfmt" | "oxlint" => {
+                // Case PATH first: shims a case creates in $VP_HOME/bin must
+                // shadow the harness-installed aliases. The flavor bin dir is
+                // on that PATH too, so this is a pure precedence rule; the
+                // direct bin-dir lookup below only remains as the fallback
+                // for cases that override PATH entirely.
+                if let Ok(found) = which::which_in(program, Some(case_path), PathBuf::from(".")) {
+                    return Ok(found);
+                }
+                self.bin_dir_tool(program)
             }
+            "vpt" => self.bin_dir_tool(program),
             "node" | "git" | "npm" | "pnpm" | "yarn" | "bun" => {
                 which::which_in(program, Some(case_path), PathBuf::from("."))
                     .map_err(|e| format!("`{program}` not found on the case PATH: {e}"))
@@ -311,4 +306,22 @@ impl FlavorRuntime {
             )),
         }
     }
+
+    /// Looks a tool up directly in the flavor bin dir.
+    fn bin_dir_tool(&self, program: &str) -> Result<PathBuf, String> {
+        if cfg!(windows) {
+            // Installed as either .exe or .cmd; try both.
+            ["exe", "cmd"]
+                .iter()
+                .map(|ext| self.bin_dir.join(format!("{program}.{ext}")))
+                .find(|p| p.is_file())
+                .ok_or_else(|| format!("`{program}` is not available in this flavor"))
+        } else {
+            let p = self.bin_dir.join(program);
+            if !p.is_file() && !p.is_symlink() {
+                return Err(format!("`{program}` is not available in this flavor"));
+            }
+            Ok(p)
+        }
+    }
 }

crates/vite_cli_snapshots/tests/cli_snapshots/main.rs

@@ -86,6 +86,10 @@ struct StepConfig {
     /// that specifically assert non-TTY behaviour. Interactions require a PTY.
     #[serde(default = "default_true")]
     tty: bool,
+    /// By default a failing step stops the case (shell-like `&&` semantics);
+    /// set true when later steps deliberately inspect post-failure state.
+    #[serde(default, rename = "continue-on-failure")]
+    continue_on_failure: bool,
 }
 
 impl Step {
@@ -188,6 +192,13 @@ impl Step {
             Self::Simple(_) => true,
         }
     }
+
+    const fn continue_on_failure(&self) -> bool {
+        match self {
+            Self::Detailed(config) => config.continue_on_failure,
+            Self::Simple(_) => false,
+        }
+    }
 }
 
 #[derive(serde::Deserialize, Debug, Clone)]
@@ -650,7 +661,7 @@ fn run_case(
     }
 
     let mut timeout_error: Option<String> = None;
-    for step in &case.steps {
+    for (step_index, step) in case.steps.iter().enumerate() {
         let argv = step.argv();
         assert!(!argv.is_empty(), "step argv must not be empty");
 
@@ -828,6 +839,15 @@ fn run_case(
             );
             doc.push_str(&redacted);
         }
+
+        // Shell-like `&&` semantics: a failing step stops the case unless it
+        // opts out, so later steps never bless output from a broken setup.
+        if !succeeded && !step.continue_on_failure() {
+            if step_index + 1 < case.steps.len() {
+                doc.push_str("\n*(remaining steps skipped: step failed)*\n");
+            }
+            break;
+        }
     }
 
     // Cleanup steps: best-effort, never snapshotted. Per-step envs apply

crates/vite_cli_snapshots/tests/cli_snapshots/redact.rs

@@ -24,6 +24,15 @@ static VERSION_RE: LazyLock<regex::Regex> = LazyLock::new(|| {
 });
 static THREAD_RE: LazyLock<regex::Regex> =
     LazyLock::new(|| regex::Regex::new(r"\d+ threads").unwrap());
+// Some tool banners print runtime versions bare ("Node 24.18.0  pnpm 10.34.4"
+// in vp create); mask those by tool-name context so user semver elsewhere
+// stays assertable.
+static TOOL_VERSION_RE: LazyLock<regex::Regex> = LazyLock::new(|| {
+    regex::Regex::new(
+        r"\b((?i:node(?:\.js)?|npm|pnpm|yarn|bun|deno))([ /]+)\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?\b",
+    )
+    .unwrap()
+});
 // Output bytes differ across OSes (line endings, embedded paths), so byte
 // sizes and content-derived asset hashes can never be part of a shared
 // snapshot. The unit is kept ("<size> kB"): it only changes when content
@@ -153,6 +162,9 @@ pub fn redact_output(
     // Redact semver-shaped versions (bundled tool versions, Node versions).
     output = VERSION_RE.replace_all(&output, "<version>").into_owned();
 
+    // Redact bare runtime-tool versions by name context (see TOOL_VERSION_RE)
+    output = TOOL_VERSION_RE.replace_all(&output, "$1$2<version>").into_owned();
+
     // Redact thread counts like "16 threads" to "<n> threads"
     output = THREAD_RE.replace_all(&output, "<n> threads").into_owned();
 

crates/vite_cli_snapshots/tests/redact_unit.rs

@@ -45,6 +45,16 @@ fn masks_only_v_prefixed_versions() {
     );
 }
 
+#[test]
+fn masks_bare_runtime_tool_versions_by_name_context() {
+    // vp create prints these without the v prefix.
+    let input = "Node 24.18.0  pnpm 10.34.4 (agent npm/11.4.2)\n".to_owned();
+    assert_eq!(
+        redact_output(input, &[], true),
+        "Node <version>  pnpm <version> (agent npm/<version>)\n"
+    );
+}
+
 #[test]
 fn replaces_paths_with_labels() {
     let input = "built /tmp/stage-1/dist in 3ms\n".to_owned();

packages/tools/src/__tests__/migrate-snap-tests.spec.ts

@@ -82,6 +82,18 @@ describe('translateCommand', () => {
     expect(isTodo(translateCommand('PATH=$PATH vp check', ctx()))).toBe(true);
   });
 
+  it('marks only the line-final step continue-on-failure', () => {
+    // Legacy lines were independent; && within a line short-circuited.
+    const steps = translateCommand('vp add x && cat package.json', ctx());
+    expect(steps).toHaveLength(2);
+    expect(steps[0].continueOnFailure).toBeUndefined();
+    expect(steps[1].continueOnFailure).toBe(true);
+  });
+
+  it('TODOs ls flags that list-dir does not replicate', () => {
+    expect(isTodo(translateCommand('ls -la node_modules', ctx()))).toBe(true);
+  });
+
   it('turns leading cd chains into step cwd', () => {
     const steps = translateCommand('cd packages/web && vp run build', ctx());
     expect(steps).toHaveLength(1);

packages/tools/src/migrate-snap-tests.ts

@@ -25,6 +25,7 @@ interface NewStep {
   comment?: string;
   cwd?: string;
   envs?: [string, string][];
+  continueOnFailure?: boolean;
 }
 
 /** New-harness fixture/case names allow only `[A-Za-z0-9_]`. */
@@ -282,6 +283,9 @@ function translateSimple(command: string, ctx: TranslationContext): NewStep | nu
       } else {
         return todo('unsupported chmod invocation');
       }
+    } else if (program === 'ls' && args.some((a) => a.startsWith('-'))) {
+      // ls flags (-a, -l, ...) change semantics list-dir does not replicate.
+      return todo('ls flags need hand conversion');
     } else if (program in COREUTILS_MAP) {
       step = { argv: ['vpt', COREUTILS_MAP[program], ...args.filter((a) => !a.startsWith('-'))] };
     } else {
@@ -348,6 +352,7 @@ function translateCommand(raw: string, ctx: TranslationContext): NewStep[] {
       if (comment) {
         step.comment = comment;
       }
+      step.continueOnFailure = true;
       ctx.notes.push(`folded \`&& echo\` into stat-file assertion: \`${command}\``);
       return [step];
     }
@@ -378,6 +383,13 @@ function translateCommand(raw: string, ctx: TranslationContext): NewStep[] {
   if (comment && steps.length > 0) {
     steps[0].comment = steps[0].comment ? `${comment}; ${steps[0].comment}` : comment;
   }
+  // Legacy command LINES were independent (a failure did not stop the next
+  // line), while `&&` within a line short-circuited. The harness stops on
+  // failure by default, so only the line-final step opts back out; chain-
+  // internal failures still stop, exactly like the shell did.
+  if (steps.length > 0) {
+    steps[steps.length - 1].continueOnFailure = true;
+  }
   return steps;
 }
 
@@ -394,6 +406,7 @@ function emitStep(step: NewStep, extra: { timeout?: number; snapshot?: boolean }
     step.comment === undefined &&
     step.cwd === undefined &&
     step.envs === undefined &&
+    step.continueOnFailure !== true &&
     extra.timeout === undefined &&
     extra.snapshot === undefined;
   const argv = `[${step.argv.map(tomlString).join(', ')}]`;
@@ -417,6 +430,9 @@ function emitStep(step: NewStep, extra: { timeout?: number; snapshot?: boolean }
   if (extra.snapshot !== undefined) {
     fields.push(`snapshot = ${String(extra.snapshot)}`);
   }
+  if (step.continueOnFailure === true) {
+    fields.push('continue-on-failure = true');
+  }
   return `  { ${fields.join(', ')} },`;
 }
 

rfcs/interactive-snapshot-tests.md

@@ -232,7 +232,7 @@ The per-case bin dir fronts `packages/cli/bin` (the JS dispatch), which requires
 - **PTY and screen.** Each step spawns in a fresh PTY with a 500x500 grid (large enough that wrapping and scrolling do not distort output; scrollback capture is an open question for outputs beyond 500 rows). Output feeds a vt100 emulator continuously; screens are read from the emulator, never from raw bytes. Plain capture flattens all styling; `formatted-snapshot = true` preserves SGR color codes (rendered as escaped `\x1b[31m` text) for cases that assert color behavior, with the ConPTY parity fix (bare SGR-reset stripping) inherited from `pty_terminal`.
 - **Environment.** The child env is cleared and rebuilt: fixed baseline (`VP_CLI_TEST=1`, `VP_EMIT_MILESTONES=1`, `TERM=xterm-256color`, git identity, temp `VP_HOME`, temp `NPM_CONFIG_PREFIX`, controlled `HOME`), a small platform allow-list (Windows needs `SYSTEMROOT`, `APPDATA`, etc.), then case `env`/`unset-env`, then step `envs`. Today's ~40-pattern passthrough allow-list and its leakage risks go away. Notably absent from the baseline: `CI=true` and `NO_COLOR` (the PTY makes real interactive behavior the default, and the grid render makes color stripping unnecessary).
 - **Timeouts.** Per-step, default 50s (120s for `local-registry` cases), overridable per step. On timeout the child is killed, the exit is recorded as `timeout`, and remaining steps are skipped. No leaked processes.
-- **Exit codes.** Recorded in the snapshot when nonzero. Old `&&` chains that encoded "this must succeed before the next step" translate to sequential steps whose exit codes are asserted by the snapshot itself.
+- **Exit codes.** Recorded in the snapshot when nonzero. A failing step stops the case by default (shell-like `&&` semantics), so later steps never bless output from a broken setup; `continue-on-failure = true` opts a step out for deliberate post-failure inspection. The migrator preserves legacy semantics exactly: `&&` chain members keep the stopping default, while each legacy command line's final step gets the opt-out, because old lines were independent.
 - **Concurrency.** `libtest-mimic` runs trials in parallel. vite-task serializes on Linux because of ctrl-c signal-routing flakiness in parallel PTY tests; with a corpus this size we should scope that serialization to signal-sensitive cases (a `serial-signals` marker or a dedicated shard) rather than the whole suite. This needs measurement during implementation.
 
 ## Milestone protocol and CLI instrumentation