fix(migration): gate staged migration on hook preflight, normalize hooks dir, surface staged errors

fengmk2 · fengmk2 · commit 12bf2bba3d58 · 2026-03-06T00:33:47.000+08:00
- Extract preflightGitHooksSetup() from setupGitHooks() to run
  deterministic checks before migration rewrites lint-staged config.
  When hooks setup would fail (other hook tools, subdirectory project,
  unsupported lint-staged config), migration now preserves lint-staged
  scripts and config in package.json instead of rewriting them.

- Fix hookScript() path segment counting: filter out '.' segments so
  paths like "./.config/husky" produce the same depth as ".config/husky".

- Surface real resolveViteConfig errors in `vp staged` instead of
  masking them as "No staged config found".
diff --git a/packages/cli/snap-tests-global/command-staged-broken-config/snap.txt b/packages/cli/snap-tests-global/command-staged-broken-config/snap.txt
@@ -0,0 +1,13 @@
+> printf 'export default {\n  staged: {\n    "*.ts": "vp check --fix",\n  },\n  // syntax error: missing closing brace\n' > vite.config.ts
+[1]> vp staged # should show actual config error, not 'No staged config found'
+failed to load config from <cwd>/vite.config.ts
+Failed to load vite.config: Build failed with 1 error:
+
+[PARSE_ERROR] Error: Unexpected token
+   ╭─[ vite.config.ts:5:42 ]
+   │
+ 5 │   // syntax error: missing closing brace
+   │                                          │ 
+   │                                          ╰─ 
+───╯
+
diff --git a/packages/cli/snap-tests-global/command-staged-broken-config/steps.json b/packages/cli/snap-tests-global/command-staged-broken-config/steps.json
@@ -0,0 +1,9 @@
+{
+  "commands": [
+    {
+      "command": "printf 'export default {\\n  staged: {\\n    \"*.ts\": \"vp check --fix\",\\n  },\\n  // syntax error: missing closing brace\\n' > vite.config.ts",
+      "ignoreOutput": true
+    },
+    "vp staged # should show actual config error, not 'No staged config found'"
+  ]
+}
diff --git a/packages/cli/snap-tests-global/migration-lint-staged-ts-config/snap.txt b/packages/cli/snap-tests-global/migration-lint-staged-ts-config/snap.txt
@@ -9,10 +9,6 @@ pnpm@latest installing...
 
 pnpm@<semver> installed
 
-✘ lint-staged.config.ts — please migrate to "staged" in vite.config.ts manually
-
-Only "staged" in vite.config.ts is supported. See https://viteplus.dev/migration/#lint-staged
-
 ⚠ Unsupported lint-staged config format — skipping git hooks setup. Please configure git hooks manually.
 
 Wrote agent instructions to AGENTS.md
diff --git a/packages/cli/snap-tests-global/migration-lintstagedrc-not-support/snap.txt b/packages/cli/snap-tests-global/migration-lintstagedrc-not-support/snap.txt
@@ -9,14 +9,6 @@ pnpm@latest installing...
 
 pnpm@<semver> installed
 
-✘ .lintstagedrc is not JSON format — please migrate to "staged" in vite.config.ts manually
-
-✘ .lintstagedrc.yaml — please migrate to "staged" in vite.config.ts manually
-
-✘ lint-staged.config.mjs — please migrate to "staged" in vite.config.ts manually
-
-Only "staged" in vite.config.ts is supported. See https://viteplus.dev/migration/#lint-staged
-
 ⚠ Unsupported lint-staged config format — skipping git hooks setup. Please configure git hooks manually.
 
 Wrote agent instructions to AGENTS.md
diff --git a/packages/cli/snap-tests-global/migration-other-hook-tool-with-lint-staged/package.json b/packages/cli/snap-tests-global/migration-other-hook-tool-with-lint-staged/package.json
@@ -0,0 +1,14 @@
+{
+  "name": "migration-other-hook-tool-with-lint-staged",
+  "scripts": {
+    "check-staged": "lint-staged"
+  },
+  "devDependencies": {
+    "lint-staged": "^16.2.6",
+    "simple-git-hooks": "^2.11.1",
+    "vite": "^7.0.0"
+  },
+  "lint-staged": {
+    "*.ts": "eslint --fix"
+  }
+}
diff --git a/packages/cli/snap-tests-global/migration-other-hook-tool-with-lint-staged/snap.txt b/packages/cli/snap-tests-global/migration-other-hook-tool-with-lint-staged/snap.txt
@@ -0,0 +1,39 @@
+> vp migrate --no-interactive # hooks should be skipped due to simple-git-hooks
+VITE+ - The Unified Toolchain for the Web
+
+
+Using default package manager: pnpm
+
+pnpm@latest installing...
+
+pnpm@<semver> installed
+
+⚠ Detected simple-git-hooks — skipping git hooks setup. Please configure git hooks manually.
+
+Wrote agent instructions to AGENTS.md
+✔ Migration completed!
+
+
+> cat package.json # lint-staged config and scripts should be preserved
+{
+  "name": "migration-other-hook-tool-with-lint-staged",
+  "scripts": {
+    "check-staged": "lint-staged"
+  },
+  "devDependencies": {
+    "lint-staged": "^16.2.6",
+    "simple-git-hooks": "^2.11.1",
+    "vite": "npm:@voidzero-dev/vite-plus-core@latest",
+    "vite-plus": "latest"
+  },
+  "lint-staged": {
+    "*.ts": "eslint --fix"
+  },
+  "pnpm": {
+    "overrides": {
+      "vite": "npm:@voidzero-dev/vite-plus-core@latest",
+      "vitest": "npm:@voidzero-dev/vite-plus-test@latest"
+    }
+  },
+  "packageManager": "pnpm@<semver>"
+}
diff --git a/packages/cli/snap-tests-global/migration-other-hook-tool-with-lint-staged/steps.json b/packages/cli/snap-tests-global/migration-other-hook-tool-with-lint-staged/steps.json
@@ -0,0 +1,6 @@
+{
+  "commands": [
+    "vp migrate --no-interactive # hooks should be skipped due to simple-git-hooks",
+    "cat package.json # lint-staged config and scripts should be preserved"
+  ]
+}
diff --git a/packages/cli/snap-tests-global/migration-subpath/snap.txt b/packages/cli/snap-tests-global/migration-subpath/snap.txt
@@ -9,10 +9,6 @@ pnpm@latest installing...
 
 pnpm@<semver> installed
 
-✔ Created vite.config.ts in foo/vite.config.ts
-
-✔ Merged staged config into foo/vite.config.ts
-
 ⚠ Subdirectory project detected — skipping git hooks setup. Configure hooks at the repository root.
 
 Wrote agent instructions to AGENTS.md
@@ -22,6 +18,15 @@ Wrote agent instructions to AGENTS.md
 > cat foo/package.json # check package.json
 {
   "name": "migration-subpath",
+  "lint-staged": {
+    "*.@(js|ts|tsx|yml|yaml|md|json|html|toml)": [
+      "oxfmt --staged",
+      "eslint --fix"
+    ],
+    "*.@(js|ts|tsx)": [
+      "oxlint --fix"
+    ]
+  },
   "pnpm": {
     "overrides": {
       "vite": "npm:@voidzero-dev/vite-plus-core@latest",
@@ -34,21 +39,8 @@ Wrote agent instructions to AGENTS.md
   "packageManager": "pnpm@<semver>"
 }
 
-> cat foo/vite.config.ts # check vite.config.ts
-import { defineConfig } from 'vite-plus';
-
-export default defineConfig({
-  staged: {
-    "*.@(js|ts|tsx|yml|yaml|md|json|html|toml)": [
-      "vp fmt --staged",
-      "eslint --fix"
-    ],
-    "*.@(js|ts|tsx)": [
-      "vp lint --fix"
-    ]
-  },
-  
-});
+[1]> cat foo/vite.config.ts # check vite.config.ts
+cat: foo/vite.config.ts: No such file or directory
 
 > git config --local core.hooksPath || echo 'core.hooksPath is not set' # should NOT be set
 core.hooksPath is not set
diff --git a/packages/cli/src/config/__tests__/hooks.spec.ts b/packages/cli/src/config/__tests__/hooks.spec.ts
@@ -0,0 +1,44 @@
+import { describe, expect, it } from 'vitest';
+
+import { hookScript } from '../hooks.js';
+
+function countDirnameCalls(script: string): number {
+  // Count nested dirname calls in the `d=...` line
+  const match = script.match(/^d=(.+)$/m);
+  if (!match) {
+    return 0;
+  }
+  return (match[1].match(/dirname/g) ?? []).length;
+}
+
+describe('hookScript', () => {
+  it('should compute correct depth for simple dir', () => {
+    // ".vite-hooks" → 1 segment → depth 3
+    const script = hookScript('.vite-hooks');
+    expect(countDirnameCalls(script)).toBe(3);
+  });
+
+  it('should compute correct depth for nested dir', () => {
+    // ".config/husky" → 2 segments → depth 4
+    const script = hookScript('.config/husky');
+    expect(countDirnameCalls(script)).toBe(4);
+  });
+
+  it('should handle ./ prefix correctly (bug case)', () => {
+    // "./.config/husky" should produce same depth as ".config/husky"
+    // Before fix: filter(Boolean) kept "." → 3 segments → depth 5 (wrong)
+    // After fix: filter out "." → 2 segments → depth 4 (correct)
+    const withDot = hookScript('./.config/husky');
+    const withoutDot = hookScript('.config/husky');
+    expect(countDirnameCalls(withDot)).toBe(countDirnameCalls(withoutDot));
+    expect(countDirnameCalls(withDot)).toBe(4);
+  });
+
+  it('should handle ./ prefix for simple dir', () => {
+    // "./custom-hooks" should produce same depth as "custom-hooks"
+    const withDot = hookScript('./custom-hooks');
+    const withoutDot = hookScript('custom-hooks');
+    expect(countDirnameCalls(withDot)).toBe(countDirnameCalls(withoutDot));
+    expect(countDirnameCalls(withDot)).toBe(3);
+  });
+});
diff --git a/packages/cli/src/config/hooks.ts b/packages/cli/src/config/hooks.ts
@@ -30,9 +30,10 @@ function nestedDirname(depth: number): string {
 
 // The shell script that dispatches to user-defined hooks in <dir>/
 // `depth` = number of path segments in `dir` + 2 (for `_` subdir + hook filename)
-function hookScript(dir: string): string {
+export function hookScript(dir: string): string {
   // Count segments: ".vite-hooks" → 1, ".config/husky" → 2
-  const segments = dir.split('/').filter(Boolean).length;
+  // Filter out empty strings and '.' to handle paths like "./.config/husky"
+  const segments = dir.split('/').filter((s) => s !== '' && s !== '.').length;
   const depth = segments + 2; // +2 for _ subdir and hook filename
   const rootExpr = nestedDirname(depth);
   return `#!/usr/bin/env sh
diff --git a/packages/cli/src/migration/bin.ts b/packages/cli/src/migration/bin.ts
@@ -28,8 +28,8 @@ import {
   checkVitestVersion,
   checkViteVersion,
   installGitHooks,
+  preflightGitHooksSetup,
   rewriteMonorepo,
-  hasUnsupportedHuskyVersion,
   rewriteStandaloneProject,
 } from './migrator.js';
 
@@ -225,14 +225,15 @@ async function main() {
 
   let shouldSetupHooks = await promptGitHooks(options);
 
-  if (shouldSetupHooks && hasUnsupportedHuskyVersion(workspaceInfo.rootDir)) {
-    prompts.log.warn(
-      '⚠ Detected husky <9.0.0 — please upgrade to husky v9+ first, then re-run migration.',
-    );
-    shouldSetupHooks = false;
+  if (shouldSetupHooks) {
+    const reason = preflightGitHooksSetup(workspaceInfo.rootDir);
+    if (reason) {
+      prompts.log.warn(`⚠ ${reason}`);
+      shouldSetupHooks = false;
+    }
   }
 
-  // Skip staged migration when hooks are disabled (--no-hooks or unsupported husky).
+  // Skip staged migration when hooks are disabled (--no-hooks or preflight failed).
   // Without hooks, lint-staged config must stay in package.json so existing
   // .husky/pre-commit scripts that invoke `npx lint-staged` keep working.
   const skipStagedMigration = !shouldSetupHooks;
diff --git a/packages/cli/src/migration/migrator.ts b/packages/cli/src/migration/migrator.ts
@@ -793,15 +793,13 @@ function rewriteAllImports(projectPath: string): void {
 /**
  * Check if the project has an unsupported husky version (<9.0.0).
  * Uses `semver.coerce` to handle ranges like `^8.0.0` → `8.0.0`.
+ * Accepts pre-loaded deps to avoid re-reading package.json when called
+ * from contexts that already parsed it.
  */
-export function hasUnsupportedHuskyVersion(rootDir: string): boolean {
-  const packageJsonPath = path.join(rootDir, 'package.json');
-  if (!fs.existsSync(packageJsonPath)) {
-    return false;
-  }
-  const pkg = readJsonFile(packageJsonPath);
-  const deps = pkg.devDependencies as Record<string, string> | undefined;
-  const prodDeps = pkg.dependencies as Record<string, string> | undefined;
+function checkUnsupportedHuskyVersion(
+  deps: Record<string, string> | undefined,
+  prodDeps: Record<string, string> | undefined,
+): boolean {
   const huskyVersion = deps?.husky ?? prodDeps?.husky;
   if (!huskyVersion) {
     return false;
@@ -892,57 +890,58 @@ export function getOldHooksDir(rootDir: string): string | undefined {
 }
 
 /**
- * Set up git hooks with husky + lint-staged via vp commands.
- * Skips if another hook tool is detected (warns user).
- * Returns true if hooks were successfully set up, false if skipped.
+ * Pre-flight check: verify that git hooks can be set up for this project.
+ * Returns `null` if hooks setup can proceed, or a warning reason string
+ * explaining why hooks setup should be skipped.
+ *
+ * These checks are deterministic and read-only — they do not modify
+ * the project in any way, making them safe to call before migration.
  */
-export function setupGitHooks(projectPath: string, oldHooksDir?: string): boolean {
-  // Check git root first — subdirectory projects must not set core.hooksPath
-  // (running vp config from a subdirectory would hijack the repo-wide hooksPath)
+export function preflightGitHooksSetup(projectPath: string): string | null {
   const gitRoot = findGitRoot(projectPath);
   if (gitRoot && path.resolve(projectPath) !== path.resolve(gitRoot)) {
-    prompts.log.warn(
-      '⚠ Subdirectory project detected — skipping git hooks setup. Configure hooks at the repository root.',
-    );
-    return false;
+    return 'Subdirectory project detected — skipping git hooks setup. Configure hooks at the repository root.';
   }
-
   const packageJsonPath = path.join(projectPath, 'package.json');
   if (!fs.existsSync(packageJsonPath)) {
-    return false;
+    return null; // silently skip
   }
-
-  // Check for other hook tools → warn and skip
   const pkgContent = readJsonFile(packageJsonPath);
   const deps = pkgContent.devDependencies as Record<string, string> | undefined;
   const prodDeps = pkgContent.dependencies as Record<string, string> | undefined;
   for (const tool of OTHER_HOOK_TOOLS) {
     if (deps?.[tool] || prodDeps?.[tool] || pkgContent[tool]) {
-      prompts.log.warn(
-        `⚠ Detected ${tool} — skipping git hooks setup. Please configure git hooks manually.`,
-      );
-      return false;
+      return `Detected ${tool} — skipping git hooks setup. Please configure git hooks manually.`;
     }
   }
+  if (checkUnsupportedHuskyVersion(deps, prodDeps)) {
+    return 'Detected husky <9.0.0 — please upgrade to husky v9+ first, then re-run migration.';
+  }
+  if (hasUnsupportedLintStagedConfig(projectPath)) {
+    return 'Unsupported lint-staged config format — skipping git hooks setup. Please configure git hooks manually.';
+  }
+  return null;
+}
 
-  // Check for unsupported husky version (<9.0.0) → warn and skip
-  if (hasUnsupportedHuskyVersion(projectPath)) {
-    prompts.log.warn(
-      '⚠ Detected husky <9.0.0 — please upgrade to husky v9+ first, then re-run migration.',
-    );
+/**
+ * Set up git hooks with husky + lint-staged via vp commands.
+ * Skips if another hook tool is detected (warns user).
+ * Returns true if hooks were successfully set up, false if skipped.
+ */
+export function setupGitHooks(projectPath: string, oldHooksDir?: string): boolean {
+  const reason = preflightGitHooksSetup(projectPath);
+  if (reason) {
+    prompts.log.warn(`⚠ ${reason}`);
     return false;
   }
 
-  // Skip hook setup if lint-staged config exists in a format that can't be
-  // auto-migrated — the config still references old commands (oxlint/oxfmt)
-  // that migration can't rewrite, so vp staged would fail on the next commit.
-  if (hasUnsupportedLintStagedConfig(projectPath)) {
-    prompts.log.warn(
-      '⚠ Unsupported lint-staged config format — skipping git hooks setup. Please configure git hooks manually.',
-    );
+  const packageJsonPath = path.join(projectPath, 'package.json');
+  if (!fs.existsSync(packageJsonPath)) {
     return false;
   }
 
+  const gitRoot = findGitRoot(projectPath);
+
   // Custom husky dirs (e.g. .config/husky) stay unchanged;
   // only the default .husky dir gets migrated to .vite-hooks.
   const isCustomDir = oldHooksDir != null && oldHooksDir !== '.husky';
diff --git a/packages/cli/src/staged/bin.ts b/packages/cli/src/staged/bin.ts
@@ -141,8 +141,12 @@ if (args.help) {
   try {
     const viteConfig = await resolveViteConfig(args.cwd ?? process.cwd());
     stagedConfig = viteConfig.staged;
-  } catch {
-    // vite.config.ts not found or resolve failed — fall through
+  } catch (err) {
+    // Surface real errors (syntax errors, missing imports, etc.)
+    // instead of masking them as "no config found"
+    const message = err instanceof Error ? err.message : String(err);
+    log(`Failed to load vite.config: ${message}`);
+    process.exit(1);
   }
   if (stagedConfig) {
     options.config = stagedConfig as Configuration;
