From ef64ec56f221038eaa222b82956fc78fdd450bdf Mon Sep 17 00:00:00 2001 From: Rene Zander Date: Thu, 2 Jul 2026 05:07:46 +0000 Subject: [PATCH] fix(add-video/add-audio): de-collide different sources sharing a basename addVideo/addAudio copied sources into assets// and skipped the copy when the destination already existed. The skip is intentional (the Wikimedia fetch path pre-writes there, and re-adding the same file must not re-copy), but when a *different* source resolved to an existing basename (e.g. en/0_00.png vs jp/0_00.png) the copy was skipped silently and the draft pointed at the wrong content. Route both through a shared copyAssetDeduped(): byte-identical existing file keeps the silent no-op; a different source is written under a content-hashed name (0_00..png) and a warning is printed to stderr, so the draft references the correct content instead of a stale one. Adds test/asset-collision.test.mjs covering the collision and the idempotent re-add. Reported by @naoyashiga. Closes #37 --- src/factory.ts | 76 +++++++++++++++++++++++++++-------- test/asset-collision.test.mjs | 71 ++++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+), 16 deletions(-) create mode 100644 test/asset-collision.test.mjs diff --git a/src/factory.ts b/src/factory.ts index 9acf56a..35f5b00 100644 --- a/src/factory.ts +++ b/src/factory.ts @@ -1,5 +1,5 @@ -import { randomUUID } from "node:crypto"; -import { copyFileSync, cpSync, existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import { createHash, randomUUID } from "node:crypto"; +import { copyFileSync, cpSync, existsSync, mkdirSync, readFileSync, statSync, writeFileSync } from "node:fs"; import { basename, dirname, resolve } from "node:path"; import type { Draft, Segment, Timerange, Track } from "./draft.js"; import { findMaterialGlobal, findSegment } from "./draft.js"; @@ -41,6 +41,58 @@ export function uuid(): string { return randomUUID(); } +// --- Asset copy (collision-safe) --- + +function fileSha1(path: string): string { + return createHash("sha1").update(readFileSync(path)).digest("hex"); +} + +/** True if both paths exist and have byte-identical content. */ +function sameContent(a: string, b: string): boolean { + if (statSync(a).size !== statSync(b).size) return false; + return fileSha1(a) === fileSha1(b); +} + +/** + * Copy a source file into an assets directory, keyed by its basename. + * + * The skip-on-exists behaviour is intentional and load-bearing: the Wikimedia + * fetch path pre-writes into the same dir so this copy is a deliberate no-op, + * and re-adding the same file must not re-copy. But when a *different* source + * resolves to a basename already present (e.g. `en/0_00.png` vs `jp/0_00.png`), + * skipping silently leaves the draft pointing at the wrong content. In that case + * we de-collide by writing under a content-hashed name and warn on stderr. + * + * Returns the destination path the draft should reference. + */ +function copyAssetDeduped(srcPath: string, assetsDir: string, fallbackName: string): string { + mkdirSync(assetsDir, { recursive: true }); + const filename = basename(srcPath) || fallbackName; + const destPath = resolve(assetsDir, filename); + + if (!existsSync(destPath)) { + copyFileSync(srcPath, destPath); + return destPath; + } + // Destination exists — same source (intentional no-op) or basename collision? + if (sameContent(srcPath, destPath)) return destPath; + + // Different source resolving to the same basename: de-collide by content hash. + const dot = filename.lastIndexOf("."); + const stem = dot > 0 ? filename.slice(0, dot) : filename; + const ext = dot > 0 ? filename.slice(dot) : ""; + const dedupName = `${stem}.${fileSha1(srcPath).slice(0, 8)}${ext}`; + const dedupPath = resolve(assetsDir, dedupName); + if (!existsSync(dedupPath)) { + copyFileSync(srcPath, dedupPath); + console.warn( + `Warning: "assets/${basename(assetsDir)}/${filename}" already exists from a different source file; ` + + `copied "${srcPath}" to "${dedupName}" instead. The draft references the correct content.`, + ); + } + return dedupPath; +} + // --- Init (create new empty draft) --- export interface InitOptions { @@ -526,17 +578,13 @@ export function addAudio( const trackName = opts.trackName ?? "audio"; const volume = opts.volume ?? 1.0; - // Copy file into draft assets directory + // Copy file into draft assets directory (collision-safe) const draftDir = dirname(filePath); - const filename = basename(opts.path) || "audio.mp3"; const assetsDir = resolve(draftDir, "assets", "audio"); - mkdirSync(assetsDir, { recursive: true }); - const destPath = resolve(assetsDir, filename); - if (!existsSync(destPath)) { - copyFileSync(opts.path, destPath); - } + const destPath = copyAssetDeduped(opts.path, assetsDir, "audio.mp3"); // Use the local assets path — CapCut rewrites to placeholder on open const localPath = destPath; + const filename = basename(localPath); // Find or create audio track let track = draft.tracks.find((t) => t.type === "audio" && t.name === trackName); @@ -627,17 +675,13 @@ export function addVideo( const ext = opts.path.split(".").pop()?.toLowerCase() || ""; const materialType = opts.type ?? (["jpg", "jpeg", "png", "webp", "bmp", "tiff"].includes(ext) ? "photo" : "video"); - // Copy file into draft assets directory + // Copy file into draft assets directory (collision-safe) const draftDir = dirname(filePath); - const filename = basename(opts.path) || "media"; const assetsDir = resolve(draftDir, "assets", "video"); - mkdirSync(assetsDir, { recursive: true }); - const destPath = resolve(assetsDir, filename); - if (!existsSync(destPath)) { - copyFileSync(opts.path, destPath); - } + const destPath = copyAssetDeduped(opts.path, assetsDir, "media"); // Use the local assets path — CapCut rewrites to placeholder on open const localPath = destPath; + const filename = basename(localPath); // Find or create video track let track = draft.tracks.find((t) => t.type === "video" && t.name === trackName); diff --git a/test/asset-collision.test.mjs b/test/asset-collision.test.mjs new file mode 100644 index 0000000..f9bf7e8 --- /dev/null +++ b/test/asset-collision.test.mjs @@ -0,0 +1,71 @@ +import assert from "node:assert/strict"; +import { existsSync, mkdirSync, readdirSync, readFileSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { after, describe, it } from "node:test"; +import { spawnCli } from "./helpers/spawn-cli.mjs"; +import { tmpDraft } from "./helpers/tmp-draft.mjs"; + +// Collect the `path` of every material referenced in the draft (any track type). +function materialPaths(draftPath) { + const draft = JSON.parse(readFileSync(draftPath, "utf-8")); + const paths = []; + for (const arr of Object.values(draft.materials)) { + if (!Array.isArray(arr)) continue; + for (const m of arr) if (m && typeof m.path === "string") paths.push(m.path); + } + return paths; +} + +describe("asset copy: basename collision (issue #37)", () => { + it("de-collides two different sources sharing a basename, and warns", () => { + const fix = tmpDraft(); + after(() => fix.cleanup()); + + // Two DIFFERENT files that resolve to the same basename `0_00.png`. + mkdirSync(join(fix.dir, "jp"), { recursive: true }); + mkdirSync(join(fix.dir, "en"), { recursive: true }); + const jp = join(fix.dir, "jp", "0_00.png"); + const en = join(fix.dir, "en", "0_00.png"); + writeFileSync(jp, "JAPANESE-IMAGE-BYTES"); + writeFileSync(en, "ENGLISH-IMAGE-BYTES-different-length"); + + const r1 = spawnCli(["add-video", fix.path, jp, "0", "7s", "--no-probe"]); + assert.equal(r1.status, 0, `add jp failed: ${r1.stderr}`); + const r2 = spawnCli(["add-video", fix.path, en, "0", "7s", "--no-probe"]); + assert.equal(r2.status, 0, `add en failed: ${r2.stderr}`); + + // The collision is surfaced, not silent. + assert.match(r2.stderr, /already exists from a different source/i); + + // Both sources landed as distinct files in assets/video. + const videoDir = join(fix.dir, "assets", "video"); + const files = readdirSync(videoDir).filter((f) => f.endsWith(".png")); + assert.equal(files.length, 2, `expected 2 distinct assets, got: ${files.join(", ")}`); + + // The two newest video materials point at different, existing files whose + // content matches their original source (no stale reference). + const paths = [...new Set(materialPaths(fix.path))].filter((p) => p.includes("0_00")); + assert.equal(paths.length, 2, `expected 2 distinct material paths, got: ${paths.join(", ")}`); + const contents = paths.map((p) => readFileSync(p, "utf-8")).sort(); + assert.deepEqual(contents, ["ENGLISH-IMAGE-BYTES-different-length", "JAPANESE-IMAGE-BYTES"].sort()); + }); + + it("re-adding the SAME file is an idempotent no-op (no dup, no warning)", () => { + const fix = tmpDraft(); + after(() => fix.cleanup()); + + const src = join(fix.dir, "clip.png"); + writeFileSync(src, "SAME-BYTES"); + + const r1 = spawnCli(["add-video", fix.path, src, "0", "5s", "--no-probe"]); + assert.equal(r1.status, 0, `first add failed: ${r1.stderr}`); + const r2 = spawnCli(["add-video", fix.path, src, "0", "5s", "--no-probe"]); + assert.equal(r2.status, 0, `second add failed: ${r2.stderr}`); + + assert.doesNotMatch(r2.stderr, /already exists from a different source/i); + const videoDir = join(fix.dir, "assets", "video"); + const clips = readdirSync(videoDir).filter((f) => f.startsWith("clip")); + assert.equal(clips.length, 1, `expected a single copy, got: ${clips.join(", ")}`); + assert.ok(existsSync(join(videoDir, "clip.png"))); + }); +});