diff --git a/.github/workflows/windows-render.yml b/.github/workflows/windows-render.yml index db3100e127..869ea0e279 100644 --- a/.github/workflows/windows-render.yml +++ b/.github/workflows/windows-render.yml @@ -484,6 +484,11 @@ jobs: shell: pwsh run: bunx vitest run packages/producer/src/services/hyperframeLint.file-race.test.ts --maxWorkers=2 + - name: Verify Windows frame staging + if: matrix.lane == 'studio-engine-cli' + shell: pwsh + run: bunx vitest run packages/producer/src/services/windowsFrameStaging.test.ts --maxWorkers=2 + - name: Verify catalog source reads on Windows if: matrix.lane == 'studio-engine-cli' shell: pwsh diff --git a/packages/producer/src/services/render/shared.ts b/packages/producer/src/services/render/shared.ts index 69fc3fe83e..15eaf8226f 100644 --- a/packages/producer/src/services/render/shared.ts +++ b/packages/producer/src/services/render/shared.ts @@ -306,7 +306,7 @@ type MaterializePathModule = { type MaterializeFileSystem = { existsSync: (path: string) => boolean; mkdirSync: (path: string, options: { recursive: true }) => unknown; - symlinkSync: (target: string, path: string) => unknown; + symlinkSync: (target: string, path: string, type?: "dir" | "junction") => unknown; cpSync: (src: string, dest: string, options: { recursive: true }) => unknown; // Optional: only the stale-entry (EEXIST) recovery path calls it, and the // default fileSystem always supplies it. Test doubles that never trigger @@ -412,31 +412,55 @@ export function createMemorySampler(intervalMs: number = 250): MemorySampler { * external callers should use `executeRenderJob` instead. */ // Stage one video's extracted-frame dir into the compiled dir. Default is a -// single symlink (cheap; the in-process renderer); `materializeSymlinks` copies -// instead (distributed plan() needs a self-contained dir). On Windows without -// Developer Mode/Administrator symlink creation is rejected with EPERM/EACCES, -// which failed high/standard renders — degrade to a copy there rather than -// throwing. Non-permission errors still propagate so real failures aren't hidden. +// directory link (cheap; the in-process renderer); `materializeSymlinks` copies +// instead (distributed plan() needs a self-contained dir). On Windows, try a +// junction if symlink privileges are unavailable before falling back to a copy. // One-time guard for the symlink→copy fallback notice below. let warnedSymlinkFallback = false; +// Junctions avoid Windows symlink privileges. POSIX keeps its copy fallback. +function tryWindowsFrameJunction( + fileSystem: MaterializeFileSystem, + src: string, + dest: string, +): boolean { + if (process.platform !== "win32") return false; + try { + fileSystem.symlinkSync(src, dest, "junction"); + return true; + } catch (error) { + const code = + error instanceof Error && "code" in error && typeof error.code === "string" + ? error.code + : undefined; + if ( + ["EPERM", "EACCES", "UNKNOWN", "EINVAL", "ENOSYS", "EOPNOTSUPP", "ENOTSUP"].includes( + code ?? "", + ) + ) { + return false; + } + throw error; + } +} + // Create the symlink, degrading to a copy on Windows' no-symlink-privilege -// errors (EPERM/EACCES, plus UNKNOWN — some Windows builds surface a symlink -// privilege denial as an UNKNOWN-coded error rather than EPERM). Non-permission -// errors propagate. +// errors (EPERM/EACCES, plus UNKNOWN), trying a Windows junction first. +// Other symlink errors and non-capability junction errors propagate. function linkOrCopyFrameDir(fileSystem: MaterializeFileSystem, src: string, dest: string): void { try { - fileSystem.symlinkSync(src, dest); + fileSystem.symlinkSync(src, dest, "dir"); } catch (err) { const code = (err as NodeJS.ErrnoException | undefined)?.code; if (code !== "EPERM" && code !== "EACCES" && code !== "UNKNOWN") throw err; + if (tryWindowsFrameJunction(fileSystem, src, dest)) return; // Copying is measurably slower than symlinking, so surface the degrade once // — it explains a render that suddenly got heavier and saves a support // round-trip diagnosing slow frame staging on Windows. if (!warnedSymlinkFallback) { warnedSymlinkFallback = true; defaultLogger.info( - `[Render] Symlinking extracted frames was rejected (${code}); copying them into the compiled dir instead. Expected on Windows without Developer Mode/Administrator.`, + `[Render] Linking extracted frames was rejected (${code}); copying them into the compiled dir instead.`, ); } fileSystem.cpSync(src, dest, { recursive: true }); diff --git a/packages/producer/src/services/render/stages/extractVideosStage.test.ts b/packages/producer/src/services/render/stages/extractVideosStage.test.ts index 3df853e8ac..dc6e7ff0c8 100644 --- a/packages/producer/src/services/render/stages/extractVideosStage.test.ts +++ b/packages/producer/src/services/render/stages/extractVideosStage.test.ts @@ -15,7 +15,6 @@ import { buildHdrProbeStageError, resolveVideoExtractionPolicy, safeVideoExtractionSourceLogMetadata, - shouldCopyExtractedFrames, VideoExtractionStageError, } from "./extractVideosStage.js"; import { EncoderInterruptedError } from "../encoderInterruption.js"; @@ -210,17 +209,6 @@ describe("HDR probe src resolution (PRINFRA-349)", () => { }); }); -describe("shouldCopyExtractedFrames", () => { - it("copies frames on Windows (symlinkSync throws EPERM without Developer Mode)", () => { - expect(shouldCopyExtractedFrames("win32")).toBe(true); - }); - - it("symlinks on macOS and Linux (cheaper, symlinks allowed)", () => { - expect(shouldCopyExtractedFrames("darwin")).toBe(false); - expect(shouldCopyExtractedFrames("linux")).toBe(false); - }); -}); - describe("resolveVideoExtractionPolicy", () => { it("enforces extraction failures by default (#3372)", () => { expect(resolveVideoExtractionPolicy({})).toEqual({ diff --git a/packages/producer/src/services/render/stages/extractVideosStage.ts b/packages/producer/src/services/render/stages/extractVideosStage.ts index dc37c0c2b0..029c9905aa 100644 --- a/packages/producer/src/services/render/stages/extractVideosStage.ts +++ b/packages/producer/src/services/render/stages/extractVideosStage.ts @@ -112,18 +112,6 @@ export interface ExtractVideosStageResult { failureToEnforce: VideoExtractionStageError | null; } -/** - * Whether the extract stage should COPY frames into the compiled dir instead of - * symlinking them. Windows without Developer Mode / Administrator can't create - * symlinks (`symlinkSync` throws EPERM), which failed local video renders; copy - * there instead. Elsewhere symlinking is cheaper, so keep it. (The distributed - * `plan()` path already forces copying for a different reason — a self-contained - * planDir — by passing `materializeSymlinks: true` explicitly.) - */ -export function shouldCopyExtractedFrames(platform: NodeJS.Platform): boolean { - return platform === "win32"; -} - export type VideoExtractionStageErrorCode = "VIDEO_SOURCE_UNRENDERABLE" | "VIDEO_EXTRACTION_FAILED"; export interface VideoExtractionStageFailureSummary { diff --git a/packages/producer/src/services/renderOrchestrator.ts b/packages/producer/src/services/renderOrchestrator.ts index c7a373846d..37d8e5408b 100644 --- a/packages/producer/src/services/renderOrchestrator.ts +++ b/packages/producer/src/services/renderOrchestrator.ts @@ -161,10 +161,7 @@ import { import { runCompileStage } from "./render/stages/compileStage.js"; import { runProbeStage } from "./render/stages/probeStage.js"; import { validateRenderDuration } from "./render/planValidation.js"; -import { - runExtractVideosStage, - shouldCopyExtractedFrames, -} from "./render/stages/extractVideosStage.js"; +import { runExtractVideosStage } from "./render/stages/extractVideosStage.js"; import { runAudioStage } from "./render/stages/audioStage.js"; import { runCaptureStage } from "./render/stages/captureStage.js"; import { @@ -2482,9 +2479,8 @@ async function executeRenderPipeline(input: { composition, abortSignal: executionSignal, assertNotAborted, - // Copy (don't symlink) extracted frames on Windows — symlinkSync throws - // EPERM there without Developer Mode/admin, which failed local renders. - materializeSymlinks: shouldCopyExtractedFrames(process.platform), + // Local staging can use links; distributed plan() alone requires real copies. + materializeSymlinks: false, }), ); const { diff --git a/packages/producer/src/services/windowsFrameStaging.test.ts b/packages/producer/src/services/windowsFrameStaging.test.ts new file mode 100644 index 0000000000..81913794d2 --- /dev/null +++ b/packages/producer/src/services/windowsFrameStaging.test.ts @@ -0,0 +1,180 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { + cpSync, + existsSync, + lstatSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + symlinkSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { createRenderJob, executeRenderJob } from "./renderOrchestrator.js"; +import { materializeExtractedFramesForCompiledDir } from "./render/shared.js"; + +const staging = vi.hoisted((): { requestedCopy?: boolean } => ({})); +vi.mock("@hyperframes/engine", async (importOriginal) => ({ + ...(await importOriginal()), + assertConfiguredFfmpegBinariesExist: () => {}, +})); +vi.mock("./render/stages/compileStage.js", () => ({ + runCompileStage: async () => ({ + compiled: { html: "
" }, + composition: { width: 320, height: 180, duration: 1, videos: [], images: [], audios: [] }, + deviceScaleFactor: 1, + outputWidth: 320, + outputHeight: 180, + compileOnlyMs: 0, + forceScreenshot: true, + }), +})); +vi.mock("./render/stages/probeStage.js", () => ({ + runProbeStage: async ({ compiled }: { compiled: object }) => ({ + compiled, + fileServer: null, + probeSession: null, + lastBrowserConsole: [], + duration: 1, + totalFrames: 30, + browserProbeMs: 0, + }), +})); +vi.mock("./render/stages/extractVideosStage.js", async (importOriginal) => ({ + ...(await importOriginal()), + runExtractVideosStage: async ({ materializeSymlinks }: { materializeSymlinks?: boolean }) => { + staging.requestedCopy = materializeSymlinks; + throw new Error("staging reached"); + }, +})); + +const dirs: string[] = []; +function fixture() { + const root = mkdtempSync(join(tmpdir(), "hf-windows-staging-")); + dirs.push(root); + const source = join(root, "cache"); + mkdirSync(source); + const frame = join(source, "frame_000001.jpg"); + writeFileSync(frame, "frame bytes"); + return { + root, + source, + compiled: join(root, "compiled"), + extracted: { videoId: "clip", outputDir: source, framePaths: new Map([[0, frame]]) }, + }; +} +afterEach(() => { + for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true }); +}); + +it("the actual local Windows renderer does not request eager frame copies", async () => { + const { root } = fixture(); + writeFileSync(join(root, "index.html"), "
"); + const job = createRenderJob({ + fps: 30, + quality: "standard", + hdrMode: "force-sdr", + logger: { info() {}, warn() {}, error() {}, debug() {} }, + }); + const platform = Object.getOwnPropertyDescriptor(process, "platform")!; + try { + Object.defineProperty(process, "platform", { value: "win32" }); + await expect(executeRenderJob(job, root, join(root, "output.mp4"))).rejects.toThrow( + "staging reached", + ); + expect(staging.requestedCopy).not.toBe(true); + } finally { + Object.defineProperty(process, "platform", platform); + } +}); + +describe.skipIf(process.platform !== "win32")("real Windows frame staging", () => { + it("uses a real junction after a symlink privilege denial without copying frames", () => { + const { source, compiled, extracted } = fixture(); + const attempts: Array = []; + materializeExtractedFramesForCompiledDir([extracted], compiled, { + fileSystem: { + existsSync, + mkdirSync, + rmSync, + symlinkSync: (target, path, type) => { + attempts.push(type); + if (attempts.length === 1) + throw Object.assign(new Error("symlink denied"), { code: "EPERM" }); + symlinkSync(target, path, type); + }, + cpSync: () => { + throw new Error("junction-capable local staging must not copy"); + }, + }, + }); + expect(attempts).toEqual(["dir", "junction"]); + expect(lstatSync(extracted.outputDir).isSymbolicLink()).toBe(true); + writeFileSync(join(source, "frame_000001.jpg"), "updated cache bytes"); + expect(readFileSync(extracted.framePaths.get(0)!, "utf8")).toBe("updated cache bytes"); + rmSync(extracted.outputDir, { recursive: true }); + expect(readFileSync(join(source, "frame_000001.jpg"), "utf8")).toBe("updated cache bytes"); + }); +}); + +it("distributed materialization stays physically self-contained after cache removal", () => { + const { source, compiled, extracted } = fixture(); + materializeExtractedFramesForCompiledDir([extracted], compiled, { materializeSymlinks: true }); + expect(lstatSync(extracted.outputDir).isSymbolicLink()).toBe(false); + rmSync(source, { recursive: true }); + expect(readFileSync(extracted.framePaths.get(0)!, "utf8")).toBe("frame bytes"); +}); + +describe("Windows link fallbacks", () => { + const platform = Object.getOwnPropertyDescriptor(process, "platform")!; + beforeEach(() => Object.defineProperty(process, "platform", { value: "win32" })); + afterEach(() => Object.defineProperty(process, "platform", platform)); + + it.each(["EPERM", "EACCES", "UNKNOWN", "EINVAL", "ENOSYS", "EOPNOTSUPP", "ENOTSUP"])( + "copies only after the junction is rejected with %s", + (code) => { + const { compiled, extracted } = fixture(); + const attempts: Array = []; + materializeExtractedFramesForCompiledDir([extracted], compiled, { + fileSystem: { + existsSync, + mkdirSync, + cpSync, + rmSync, + symlinkSync: (_target, _path, type) => { + attempts.push(type); + throw Object.assign(new Error("link unavailable"), { + code: type === "junction" ? code : "EPERM", + }); + }, + }, + }); + expect(attempts).toEqual(["dir", "junction"]); + expect(lstatSync(extracted.outputDir).isSymbolicLink()).toBe(false); + expect(readFileSync(extracted.framePaths.get(0)!, "utf8")).toBe("frame bytes"); + }, + ); + + it("does not hide a disk failure from the junction attempt", () => { + const { compiled, extracted } = fixture(); + const copy = vi.fn(); + expect(() => + materializeExtractedFramesForCompiledDir([extracted], compiled, { + fileSystem: { + existsSync, + mkdirSync, + rmSync, + cpSync: copy, + symlinkSync: (_target, _path, type) => { + throw Object.assign(new Error(type === "junction" ? "disk full" : "denied"), { + code: type === "junction" ? "ENOSPC" : "EPERM", + }); + }, + }, + }), + ).toThrow("disk full"); + expect(copy).not.toHaveBeenCalled(); + }); +});