fix: require valid VpDirs output

Author: fengmk2

dist/azure/index.mjs

@@ -37,6 +37,31 @@ setup_vp_export_env() {
   printf "\n" >> "$SETUP_VP_ENV_FILE"
 }
 
+setup_vp_read_bin_dir() {
+  awk '
+    {
+      separator = index($0, "\t")
+      if (separator == 0) next
+      key = substr($0, 1, separator - 1)
+      value = substr($0, separator + 1)
+      sub(/^[[:space:]]+/, "", value)
+      sub(/[[:space:]]+$/, "", value)
+      if (key == "data") data = value
+      if (key == "bin") bin = value
+      if (key == "cache") cache = value
+      if (key == "config") config = value
+      if (key == "state") state = value
+    }
+    END {
+      if (data != "" && bin != "" && cache != "" && config != "" && state != "") {
+        print bin
+        exit 0
+      }
+      exit 1
+    }
+  ' "$1"
+}
+
 setup_vp_install_viteplus_from() {
   setup_vp_url="$1"
   setup_vp_download "$setup_vp_url" "$setup_vp_install_tmp" || return 1
@@ -162,12 +187,13 @@ setup_vp_runtime_tmp="$(mktemp "${TMPDIR:-/tmp}/setup-vp-gitlab-runtime.XXXXXX.m
 trap 'rm -f "$setup_vp_install_tmp" "$setup_vp_dirs_tmp" "$setup_vp_runtime_tmp"' EXIT
 
 setup_vp_install_viteplus
-setup_vp_bin_dir=""
 if [ "$setup_vp_detect_dirs" = "true" ]; then
-  setup_vp_bin_dir="$(awk -F '\t' '$1 == "bin" { print $2; exit }' "$setup_vp_dirs_tmp")"
-fi
-if [ -z "$setup_vp_bin_dir" ]; then
-  # Vite+ releases before VpDirs always use the monolithic layout.
+  if ! setup_vp_bin_dir="$(setup_vp_read_bin_dir "$setup_vp_dirs_tmp")"; then
+    echo "setup-vp: Vite+ was installed successfully, but setup-vp could not resolve its VpDirs." >&2
+    return 1 2>/dev/null || exit 1
+  fi
+else
+  # Vite+ releases before VpDirs use the monolithic layout.
   setup_vp_bin_dir="$HOME/.vite-plus/bin"
 fi
 export PATH="$setup_vp_bin_dir:$PATH"

dist/index.mjs

@@ -19,12 +19,9 @@ describe("installVitePlus", () => {
   it("uses PowerShell installers on Windows and bash installers on Unix", async () => {
     const calls: NodeJS.Platform[] = [];
     const runInstall = vi.fn(
-      (
-        _url: string,
-        _env: Record<string, string>,
-        platform: NodeJS.Platform = process.platform,
-      ) => {
+      (_url: string, env: Record<string, string>, platform: NodeJS.Platform = process.platform) => {
         calls.push(platform);
+        writeDirsFile(env, `/test/${platform}/bin`);
         return 0;
       },
     );
@@ -82,7 +79,10 @@ describe("installVitePlus", () => {
   });
 
   it("routes pkg.pr.new commit builds through VP_PR_VERSION", async () => {
-    const runInstall = vi.fn(() => 0);
+    const runInstall = vi.fn((_url: string, env: Record<string, string>) => {
+      writeDirsFile(env, "/test/data/bin");
+      return 0;
+    });
     const sha = "a".repeat(40);
 
     await installVitePlus(`0.0.0-commit.${sha}`, {
@@ -102,6 +102,34 @@ describe("installVitePlus", () => {
     expect(installCalls[0]?.[1]?.SETUP_VP_DIRS_FILE).toMatch(/setup-vp-dirs-.*\.txt$/);
   });
 
+  it.each([
+    { version: "0.3.0", output: undefined },
+    { version: `0.0.0-commit.${"a".repeat(40)}`, output: "bin\t/test/data/bin\n" },
+    { version: "latest", output: "" },
+  ])("fails when $version does not report valid VpDirs", async ({ version, output }) => {
+    const prependPath = vi.fn();
+    const runInstall = vi.fn((_url: string, env: Record<string, string>) => {
+      if (output !== undefined) writeFileSync(env.SETUP_VP_DIRS_FILE, output);
+      return 0;
+    });
+
+    await expect(
+      installVitePlus(version, {
+        platform: "linux",
+        env: { PATH: "/usr/bin" },
+        prependPath,
+        sleep: async () => undefined,
+        runInstall,
+        logWarningFn: () => undefined,
+      }),
+    ).rejects.toThrow(
+      "Vite+ was installed successfully, but setup-vp could not resolve its VpDirs.",
+    );
+
+    expect(runInstall).toHaveBeenCalledTimes(1);
+    expect(prependPath).not.toHaveBeenCalled();
+  });
+
   it("prepends the bin directory reported by the installed payload", async () => {
     const prependPath = vi.fn();
     const env = { PATH: "/usr/bin" };

gitlab/bootstrap.sh

@@ -6,8 +6,8 @@ import { getInstallScriptUrls, pkgPrNewCommitSha } from "../ci/install-script-ur
 import {
   createVitePlusDirsFile,
   getInstallScriptCommand,
-  readVitePlusDirs,
   removeVitePlusDirsFile,
+  resolveVitePlusBinDir,
   supportsVitePlusDirs,
   VP_DIRS_FILE_ENV,
 } from "../ci/vp-dirs.js";
@@ -120,10 +120,7 @@ export async function installVitePlus(
   };
 
   const ensureBinInPath = (): void => {
-    // Pre-VpDirs releases cannot emit a dump and always use this legacy path.
-    const binDir =
-      (dirsFile ? readVitePlusDirs(dirsFile)?.bin : undefined) ??
-      join(getVitePlusHome(platform), "bin");
+    const binDir = resolveVitePlusBinDir(dirsFile, join(getVitePlusHome(platform), "bin"));
     const separator = platform === "win32" ? ";" : ":";
     if (!targetEnv.PATH?.split(separator).includes(binDir)) {
       targetEnv.PATH = `${binDir}${separator}${targetEnv.PATH || ""}`;

src/azure/install-viteplus.test.ts

@@ -50,6 +50,16 @@ export function readVitePlusDirs(filePath: string): VitePlusDirs | undefined {
   }
 }
 
+export function resolveVitePlusBinDir(dirsFile: string | undefined, legacyBinDir: string): string {
+  if (!dirsFile) return legacyBinDir;
+
+  const dirs = readVitePlusDirs(dirsFile);
+  if (!dirs) {
+    throw new Error("Vite+ was installed successfully, but setup-vp could not resolve its VpDirs.");
+  }
+  return dirs.bin;
+}
+
 export function parseVitePlusDirs(output: string): VitePlusDirs | undefined {
   const dirs = new Map<string, string>();
 

src/azure/install-viteplus.ts

@@ -17,10 +17,20 @@ describe("GitLab bootstrap", () => {
   it("uses the bin directory reported by the installed payload", () => {
     expect(bootstrap).toContain('export VP_VPDIRS_AWARE="1"');
     expect(bootstrap).toContain('VP_DUMP_DIRS=1 "$SHIM_DIR/vp"');
-    expect(bootstrap).toContain("setup_vp_bin_dir=\"$(awk -F '\\t'");
+    expect(bootstrap).toContain('setup_vp_bin_dir="$(setup_vp_read_bin_dir "$setup_vp_dirs_tmp")"');
     expect(bootstrap).toContain('export PATH="$setup_vp_bin_dir:$PATH"');
   });
 
+  it("fails when a VpDirs-aware install does not report valid directories", () => {
+    for (const name of ["data", "bin", "cache", "config", "state"]) {
+      expect(bootstrap).toContain(`if (key == "${name}") ${name} = value`);
+    }
+    expect(bootstrap).toContain(
+      "Vite+ was installed successfully, but setup-vp could not resolve its VpDirs.",
+    );
+    expect(bootstrap).toContain("return 1 2>/dev/null || exit 1");
+  });
+
   it("limits VpDirs detection to supported versions", () => {
     expect(bootstrap).toContain('setup_vp_detect_dirs="true"');
     expect(bootstrap).toContain('[ "$setup_vp_minor" -lt 3 ]');

src/ci/vp-dirs.ts

@@ -48,6 +48,14 @@ function writeDirsFile(options: unknown, bin: string): void {
   );
 }
 
+function mockSuccessfulInstallOnce(): void {
+  vi.mocked(exec).mockImplementationOnce(async (_command, _args, options) => {
+    const env = (options as { env: Record<string, string> }).env;
+    if (env.SETUP_VP_DIRS_FILE) writeDirsFile(options, "/test/data/bin");
+    return 0;
+  });
+}
+
 describe("installVitePlus", () => {
   // installVitePlus spreads process.env into the child env, so a VP_PR_VERSION
   // inherited from the runner (setup-vp's own CI sets it) would make these tests
@@ -82,7 +90,7 @@ describe("installVitePlus", () => {
     vi.stubEnv("PATH", "/usr/bin");
     vi.stubEnv("VP_VPDIRS_AWARE", "1");
     vi.stubEnv("SETUP_VP_DIRS_FILE", "/tmp/stale-vp-dirs");
-    vi.mocked(exec).mockResolvedValueOnce(0);
+    mockSuccessfulInstallOnce();
 
     await installVitePlus({ ...baseInputs, version: "0.2.9" });
 
@@ -94,8 +102,30 @@ describe("installVitePlus", () => {
     expect((options as { env: Record<string, string> }).env.SETUP_VP_DIRS_FILE).toBeUndefined();
   });
 
+  it.each([
+    { version: "0.3.0", output: undefined },
+    { version: `0.0.0-commit.${commitSha}`, output: "bin\t/test/data/bin\n" },
+    { version: "latest", output: "" },
+  ])("should fail when $version does not report valid VpDirs", async ({ version, output }) => {
+    vi.mocked(exec).mockImplementationOnce(async (_command, _args, options) => {
+      if (output !== undefined) {
+        const env = (options as { env: Record<string, string> }).env;
+        writeFileSync(env.SETUP_VP_DIRS_FILE, output);
+      }
+      return 0;
+    });
+
+    await expect(installVitePlus({ ...baseInputs, version })).rejects.toThrow(
+      "Vite+ was installed successfully, but setup-vp could not resolve its VpDirs.",
+    );
+
+    expect(exec).toHaveBeenCalledTimes(1);
+    expect(addPath).not.toHaveBeenCalled();
+  });
+
   it("should retry on transient failure and eventually succeed", async () => {
-    vi.mocked(exec).mockResolvedValueOnce(6).mockResolvedValueOnce(6).mockResolvedValueOnce(0);
+    vi.mocked(exec).mockResolvedValueOnce(6).mockResolvedValueOnce(6);
+    mockSuccessfulInstallOnce();
 
     await installVitePlus(baseInputs);
 
@@ -112,7 +142,8 @@ describe("installVitePlus", () => {
   });
 
   it("should retry when exec itself throws (e.g. process spawn error)", async () => {
-    vi.mocked(exec).mockRejectedValueOnce(new Error("spawn bash ENOENT")).mockResolvedValueOnce(0);
+    vi.mocked(exec).mockRejectedValueOnce(new Error("spawn bash ENOENT"));
+    mockSuccessfulInstallOnce();
 
     await installVitePlus(baseInputs);
 
@@ -121,7 +152,8 @@ describe("installVitePlus", () => {
   });
 
   it("should fall back to the GitHub install URL after a single primary failure", async () => {
-    vi.mocked(exec).mockResolvedValueOnce(35).mockResolvedValueOnce(0);
+    vi.mocked(exec).mockResolvedValueOnce(35);
+    mockSuccessfulInstallOnce();
 
     await installVitePlus(baseInputs);
 
@@ -157,7 +189,7 @@ describe("installVitePlus", () => {
   ])(
     "should install $desc with the install script from their git ref",
     async ({ version, ref }) => {
-      vi.mocked(exec).mockResolvedValueOnce(0);
+      mockSuccessfulInstallOnce();
 
       await installVitePlus({ ...baseInputs, version });
 
@@ -169,7 +201,8 @@ describe("installVitePlus", () => {
   );
 
   it("should fall back to the jsDelivr mirror of the pinned script on failure", async () => {
-    vi.mocked(exec).mockResolvedValueOnce(35).mockResolvedValueOnce(0);
+    vi.mocked(exec).mockResolvedValueOnce(35);
+    mockSuccessfulInstallOnce();
 
     await installVitePlus({ ...baseInputs, version: "0.2.9" });
 
@@ -188,8 +221,8 @@ describe("installVitePlus", () => {
       .mockResolvedValueOnce(22)
       .mockResolvedValueOnce(22)
       .mockResolvedValueOnce(22)
-      .mockResolvedValueOnce(22)
-      .mockResolvedValueOnce(0);
+      .mockResolvedValueOnce(22);
+    mockSuccessfulInstallOnce();
 
     await installVitePlus({ ...baseInputs, version: "0.2.9" });
 
@@ -215,7 +248,7 @@ describe("installVitePlus", () => {
   });
 
   it("should source the downloaded bash installer and dump its resolved directories", async () => {
-    vi.mocked(exec).mockResolvedValueOnce(0);
+    mockSuccessfulInstallOnce();
 
     await installVitePlus(baseInputs);
 
@@ -232,7 +265,7 @@ describe("installVitePlus", () => {
   });
 
   it("should declare VpDirs support to the installer", async () => {
-    vi.mocked(exec).mockResolvedValueOnce(0);
+    mockSuccessfulInstallOnce();
 
     await installVitePlus(baseInputs);
 
@@ -259,7 +292,7 @@ describe("installVitePlus", () => {
       expected: undefined,
     },
   ])("$desc", async ({ version, expected }) => {
-    vi.mocked(exec).mockResolvedValueOnce(0);
+    mockSuccessfulInstallOnce();
 
     await installVitePlus({ ...baseInputs, version });
 
@@ -284,7 +317,7 @@ describe("installVitePlus", () => {
       expected: undefined,
     },
   ])("$desc", async ({ nodeManager, expected }) => {
-    vi.mocked(exec).mockResolvedValueOnce(0);
+    mockSuccessfulInstallOnce();
 
     await installVitePlus({ ...baseInputs, nodeManager });
 

src/gitlab/bootstrap.test.ts

@@ -6,8 +6,8 @@ import { getInstallScriptUrls, pkgPrNewCommitSha } from "./ci/install-script-url
 import {
   createVitePlusDirsFile,
   getInstallScriptCommand,
-  readVitePlusDirs,
   removeVitePlusDirsFile,
+  resolveVitePlusBinDir,
   supportsVitePlusDirs,
   VP_DIRS_FILE_ENV,
 } from "./ci/vp-dirs.js";
@@ -93,7 +93,7 @@ export async function installVitePlus(inputs: Inputs): Promise<void> {
   try {
     if (pinned.length > 0) {
       if (await tryUrls(pinned)) {
-        ensureVitePlusBinInPath(dirsFile ? readVitePlusDirs(dirsFile)?.bin : undefined);
+        ensureVitePlusBinInPath(dirsFile);
         return;
       }
       warning(
@@ -102,7 +102,7 @@ export async function installVitePlus(inputs: Inputs): Promise<void> {
     }
 
     if (await tryUrls(latest)) {
-      ensureVitePlusBinInPath(dirsFile ? readVitePlusDirs(dirsFile)?.bin : undefined);
+      ensureVitePlusBinInPath(dirsFile);
       return;
     }
 
@@ -124,10 +124,8 @@ async function runInstallCommand(url: string, env: { [key: string]: string }): P
   return exec(command, args, options);
 }
 
-function ensureVitePlusBinInPath(resolvedBinDir?: string): void {
-  // Vite+ releases before VpDirs cannot emit a directory dump and always use
-  // the legacy monolithic layout.
-  const binDir = resolvedBinDir ?? join(getVitePlusHome(), "bin");
+function ensureVitePlusBinInPath(dirsFile: string | undefined): void {
+  const binDir = resolveVitePlusBinDir(dirsFile, join(getVitePlusHome(), "bin"));
   if (!process.env.PATH?.split(delimiter).includes(binDir)) {
     addPath(binDir);
   }