fix(engine): harden ffmpeg binary resolution

packages/engine/src/utils/ffmpegBinaries.test.ts

@@ -1,6 +1,8 @@
 // fallow-ignore-file code-duplication
-import { resolve } from "node:path";
-import { afterEach, describe, expect, it } from "vitest";
+import { chmodSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join, resolve } from "node:path";
+import { afterEach, describe, expect, it, vi } from "vitest";
 import {
   assertConfiguredFfmpegBinariesExist,
   getFfmpegBinary,
@@ -10,12 +12,17 @@ import {
 describe("ffmpeg binary env resolution", () => {
   const originalFfmpegPath = process.env.HYPERFRAMES_FFMPEG_PATH;
   const originalFfprobePath = process.env.HYPERFRAMES_FFPROBE_PATH;
+  const originalPath = process.env.PATH;
 
   afterEach(() => {
+    vi.resetModules();
+    vi.doUnmock("child_process");
     if (originalFfmpegPath === undefined) delete process.env.HYPERFRAMES_FFMPEG_PATH;
     else process.env.HYPERFRAMES_FFMPEG_PATH = originalFfmpegPath;
     if (originalFfprobePath === undefined) delete process.env.HYPERFRAMES_FFPROBE_PATH;
     else process.env.HYPERFRAMES_FFPROBE_PATH = originalFfprobePath;
+    if (originalPath === undefined) delete process.env.PATH;
+    else process.env.PATH = originalPath;
   });
 
   it("uses configured absolute paths when env vars are set", () => {
@@ -40,4 +47,43 @@ describe("ffmpeg binary env resolution", () => {
 
     expect(() => assertConfiguredFfmpegBinariesExist()).not.toThrow();
   });
+
+  it("prefers the real Windows exe when PATH lookup lists a cmd shim first", async () => {
+    vi.resetModules();
+    vi.doMock("child_process", () => ({
+      execFileSync: () => "C:\\tools\\ffmpeg.cmd\r\nC:\\tools\\ffmpeg.exe\r\n",
+    }));
+
+    const { getFfmpegBinary: getMockedFfmpegBinary } = await import("./ffmpegBinaries.js");
+
+    expect(getMockedFfmpegBinary()).toBe(resolve("C:\\tools\\ffmpeg.exe"));
+  });
+
+  it("falls back to scanning PATH when which/where fails", async () => {
+    const binDir = mkdtempSync(join(tmpdir(), "hyperframes-ffmpeg-path-"));
+    const ffmpegPath = join(binDir, process.platform === "win32" ? "ffmpeg.exe" : "ffmpeg");
+    const execFileSync = vi.fn(() => {
+      throw new Error("lookup command failed");
+    });
+    writeFileSync(ffmpegPath, "#!/bin/sh\n");
+    chmodSync(ffmpegPath, 0o755);
+    process.env.PATH = binDir;
+    vi.resetModules();
+    vi.doMock("child_process", () => ({ execFileSync }));
+
+    try {
+      const { getFfmpegBinary: getMockedFfmpegBinary } = await import("./ffmpegBinaries.js");
+
+      expect(getMockedFfmpegBinary()).toBe(resolve(ffmpegPath));
+      expect(execFileSync).toHaveBeenCalledOnce();
+    } finally {
+      rmSync(binDir, { force: true, recursive: true });
+    }
+  });
+
+  it("calls out a mangled replacement character in configured binary paths", () => {
+    process.env.HYPERFRAMES_FFMPEG_PATH = "/missing/ffmpeg�.exe";
+
+    expect(() => assertConfiguredFfmpegBinariesExist()).toThrow(/replacement character|mangled/i);
+  });
 });

packages/engine/src/utils/ffmpegBinaries.ts

@@ -1,15 +1,71 @@
 // fallow-ignore-file code-duplication
 import { execFileSync } from "child_process";
-import { existsSync } from "fs";
-import { resolve } from "path";
+import { accessSync, constants, existsSync } from "fs";
+import { delimiter, join, resolve } from "path";
 
 export const FFMPEG_PATH_ENV = "HYPERFRAMES_FFMPEG_PATH";
 export const FFPROBE_PATH_ENV = "HYPERFRAMES_FFPROBE_PATH";
 
 const pathCache = new Map<string, string | undefined>();
 
+function candidateFileName(candidate: string): string {
+  return candidate.split(/[\\/]/).at(-1)?.toLowerCase() ?? candidate.toLowerCase();
+}
+
+function chooseBestPathCandidate(
+  name: "ffmpeg" | "ffprobe",
+  candidates: readonly string[],
+): string | undefined {
+  const normalized = candidates.map((candidate) => candidate.trim()).filter(Boolean);
+  return (
+    normalized.find((candidate) => candidateFileName(candidate) === `${name}.exe`) ??
+    normalized.find((candidate) => candidateFileName(candidate) === name) ??
+    normalized.find((candidate) => !candidateFileName(candidate).match(/\.(cmd|bat)$/i)) ??
+    normalized[0]
+  );
+}
+
+function scanPath(name: "ffmpeg" | "ffprobe"): string | undefined {
+  const pathValue = process.env.PATH;
+  if (!pathValue) return undefined;
+
+  const extensions =
+    process.platform === "win32"
+      ? [
+          ".exe",
+          ...new Set(
+            (process.env.PATHEXT ?? ".COM;.EXE;.BAT;.CMD")
+              .split(";")
+              .map((ext) => ext.trim().toLowerCase())
+              .filter(Boolean),
+          ),
+          "",
+        ]
+      : [""];
+  const candidates: string[] = [];
+  for (const dir of pathValue.split(delimiter)) {
+    if (!dir) continue;
+    for (const ext of extensions) {
+      const candidate = join(dir, `${name}${ext}`);
+      if (isExecutablePathCandidate(candidate)) candidates.push(candidate);
+    }
+  }
+  return chooseBestPathCandidate(name, candidates);
+}
+
+function isExecutablePathCandidate(candidate: string): boolean {
+  if (process.platform === "win32") return existsSync(candidate);
+  try {
+    accessSync(candidate, constants.X_OK);
+    return true;
+  } catch {
+    return false;
+  }
+}
+
 function findOnPath(name: "ffmpeg" | "ffprobe"): string | undefined {
   if (pathCache.has(name)) return pathCache.get(name);
+  let found: string | undefined;
   try {
     const command = process.platform === "win32" ? "where" : "which";
     const output = execFileSync(command, [name], {
@@ -17,17 +73,13 @@ function findOnPath(name: "ffmpeg" | "ffprobe"): string | undefined {
       stdio: ["pipe", "pipe", "pipe"],
       timeout: 5000,
     });
-    const first = output
-      .split(/\r?\n/)
-      .map((s) => s.trim())
-      .find(Boolean);
-    const resolved = first ? resolve(first) : undefined;
-    pathCache.set(name, resolved);
-    return resolved;
+    found = chooseBestPathCandidate(name, output.split(/\r?\n/));
   } catch {
-    pathCache.set(name, undefined);
-    return undefined;
+    found = scanPath(name);
   }
+  const resolved = found ? resolve(found) : undefined;
+  pathCache.set(name, resolved);
+  return resolved;
 }
 
 function getConfiguredBinary(envName: string, binaryName: "ffmpeg" | "ffprobe"): string {
@@ -49,7 +101,7 @@ export function assertConfiguredFfmpegBinariesExist(): void {
   if (ffmpegPath && !existsSync(ffmpegPath)) {
     throw new Error(
       `[FFmpeg] FFmpeg binary not found at ${FFMPEG_PATH_ENV}="${ffmpegPath}". ` +
-        "Install FFmpeg or unset the override.",
+        `Install FFmpeg or unset the override.${pathEncodingHint(ffmpegPath)}`,
     );
   }
 
@@ -57,7 +109,12 @@ export function assertConfiguredFfmpegBinariesExist(): void {
   if (ffprobePath && !existsSync(ffprobePath)) {
     throw new Error(
       `[FFmpeg] FFprobe binary not found at ${FFPROBE_PATH_ENV}="${ffprobePath}". ` +
-        "Install FFmpeg or unset the override.",
+        `Install FFmpeg or unset the override.${pathEncodingHint(ffprobePath)}`,
     );
   }
 }
+
+function pathEncodingHint(configuredPath: string): string {
+  if (!configuredPath.includes("\uFFFD")) return "";
+  return " The path contains a Unicode replacement character, which usually means it was mangled while being copied or decoded.";
+}

packages/engine/src/utils/ffprobe.test.ts

@@ -166,12 +166,19 @@ function createSpawnSpy(outcomes: SpawnOutcome[]): {
 
 describe("ffprobe missing-binary fallback", () => {
   const originalFfprobePath = process.env.HYPERFRAMES_FFPROBE_PATH;
+  const originalPath = process.env.PATH;
+
+  function hidePathBinaries(): void {
+    process.env.PATH = "";
+  }
 
   afterEach(() => {
     vi.resetModules();
     vi.doUnmock("child_process");
     if (originalFfprobePath === undefined) delete process.env.HYPERFRAMES_FFPROBE_PATH;
     else process.env.HYPERFRAMES_FFPROBE_PATH = originalFfprobePath;
+    if (originalPath === undefined) delete process.env.PATH;
+    else process.env.PATH = originalPath;
   });
 
   it("spawns the configured absolute FFprobe path when HYPERFRAMES_FFPROBE_PATH is set", async () => {
@@ -198,6 +205,7 @@ describe("ffprobe missing-binary fallback", () => {
 
   it("extractMediaMetadata falls back to PNG cICP metadata when ffprobe is missing", async () => {
     const { spawn, calls } = createSpawnSpy([{ kind: "missing" }]);
+    hidePathBinaries();
     vi.resetModules();
     vi.doMock("child_process", () => ({ spawn }));
 
@@ -294,6 +302,7 @@ describe("ffprobe missing-binary fallback", () => {
 
   it("extractMediaMetadata rethrows ffprobe-missing error for non-image files without fallback", async () => {
     const { spawn } = createSpawnSpy([{ kind: "missing" }]);
+    hidePathBinaries();
     vi.resetModules();
     vi.doMock("child_process", () => ({ spawn }));
 
@@ -304,6 +313,7 @@ describe("ffprobe missing-binary fallback", () => {
 
   it("extractAudioMetadata surfaces a ffprobe-missing error verbatim", async () => {
     const { spawn, calls } = createSpawnSpy([{ kind: "missing" }]);
+    hidePathBinaries();
     vi.resetModules();
     vi.doMock("child_process", () => ({ spawn }));
 
@@ -318,6 +328,7 @@ describe("ffprobe missing-binary fallback", () => {
 
   it("analyzeKeyframeIntervals surfaces a ffprobe-missing error verbatim", async () => {
     const { spawn, calls } = createSpawnSpy([{ kind: "missing" }]);
+    hidePathBinaries();
     vi.resetModules();
     vi.doMock("child_process", () => ({ spawn }));
 
@@ -332,6 +343,7 @@ describe("ffprobe missing-binary fallback", () => {
 
   it("ffprobe-missing error message includes install hint", async () => {
     const { spawn } = createSpawnSpy([{ kind: "missing" }]);
+    hidePathBinaries();
     vi.resetModules();
     vi.doMock("child_process", () => ({ spawn }));