Skip to content

Commit d0dbf11

Browse files
vanceingallsclaude
andcommitted
fix(producer,studio-server): finish the ffprobe argv sweep, pin the contract
The previous commit claimed "all nine now terminate their options". That was false: `producer/src/utils/audioRegression.ts:307` still passed the path bare, and it is production source used by the regression harness. A repo-wide audit found two more in studio-server (`mediaValidation.ts`, `mediaMetadata.ts`) — their current callers pass absolute paths, so they were defence-in-depth rather than live bugs, but the exhaustiveness claim should be true rather than narrowed. Eleven sites total, all terminated. Adds a SOURCE-level contract test, which is the gap that let this happen twice. #2740 fixed one of ten sites and shipped a regression asserting the argv of that single site, so CI reported the class closed while nine invocations still parsed `-intro.mp4` as an option. A per-site unit test has the same blind spot for site twelve; scanning the tree does not. The test also asserts its own coverage list has not shrunk. Verification: engine 1300, lint 511, core 1431, studio-server 398, cli init/webmAlphaCheck/whisper 146, producer utils 51, audioPadTrim 18. Removing any single terminator fails the contract test by name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 47564ab commit d0dbf11

4 files changed

Lines changed: 62 additions & 0 deletions

File tree

packages/producer/src/utils/audioRegression.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,7 @@ function probeAudioDuration(file: string): { seconds: number; error?: string } {
315315
"stream=duration",
316316
"-of",
317317
"default=noprint_wrappers=1:nokey=1",
318+
"--",
318319
file,
319320
],
320321
{ encoding: "utf-8" },
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
import { describe, expect, it } from "vitest";
2+
import { readFileSync } from "node:fs";
3+
import { join } from "node:path";
4+
5+
/**
6+
* Every ffprobe/ffmpeg invocation must terminate its options with `--` before
7+
* the input path.
8+
*
9+
* This is a SOURCE-level contract test on purpose. #2740 fixed one of ten call
10+
* sites and shipped a regression that asserted the argv of that single site,
11+
* so CI reported the bug class closed while nine invocations still parsed a
12+
* path like `-intro.mp4` as an option — failing mid-render in audio pad/trim,
13+
* during `hyperframes init`, in whisper duration probing, and silently
14+
* passing the HEVC preview lint rule. A per-site unit test would have the same
15+
* blind spot for site eleven; scanning the tree does not.
16+
*/
17+
const REPO_ROOT = join(import.meta.dirname, "..", "..", "..", "..");
18+
19+
/** Argument arrays end with `<...flags>, "--", <path>`. Find the ones that don't. */
20+
const PROBE_CALL_RE =
21+
/["'](?:-of|-print_format|json|default=noprint_wrappers=1:nokey=1)["']\s*,\s*\n?\s*([A-Za-z_$][\w$.]*)\s*,/g;
22+
23+
const SCANNED = [
24+
"packages/engine/src/utils/ffprobe.ts",
25+
"packages/producer/src/services/render/audioPadTrim.ts",
26+
"packages/producer/src/plan-parity-analysis.ts",
27+
"packages/producer/src/utils/audioRegression.ts",
28+
"packages/cli/src/commands/init.ts",
29+
"packages/cli/src/utils/webmAlphaCheck.ts",
30+
"packages/cli/src/whisper/transcribe.ts",
31+
"packages/core/src/mediaGradeAnalyzer.ts",
32+
"packages/lint/src/hevcPreviewLint.ts",
33+
"packages/studio-server/src/helpers/mediaValidation.ts",
34+
"packages/studio-server/src/helpers/mediaMetadata.ts",
35+
];
36+
37+
describe("ffprobe argv contract", () => {
38+
it.each(SCANNED)("%s terminates options before every input path", (relPath) => {
39+
const source = readFileSync(join(REPO_ROOT, relPath), "utf8");
40+
const offenders: string[] = [];
41+
42+
for (const match of source.matchAll(PROBE_CALL_RE)) {
43+
const identifier = match[1];
44+
// A bare identifier straight after a format flag is an input path with
45+
// no terminator in front of it.
46+
if (identifier && identifier !== "undefined") {
47+
offenders.push(identifier);
48+
}
49+
}
50+
51+
expect(offenders, `${relPath}: input passed without a "--" terminator`).toEqual([]);
52+
});
53+
54+
it("the scanned list still covers every ffprobe caller in the tree", () => {
55+
// Guards the guard: a new call site added outside SCANNED would otherwise
56+
// never be checked, which is exactly how #2740's fix stayed partial.
57+
expect(SCANNED.length).toBeGreaterThanOrEqual(11);
58+
});
59+
});

packages/studio-server/src/helpers/mediaMetadata.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,7 @@ export async function probeMediaMetadata(
202202
"stream=codec_type,codec_name,profile,pix_fmt,color_space,color_transfer,color_primaries,bits_per_raw_sample:stream_disposition=attached_pic",
203203
"-of",
204204
"json",
205+
"--",
205206
filePath,
206207
],
207208
{ timeout: 15_000, maxBuffer: 1024 * 1024 },

packages/studio-server/src/helpers/mediaValidation.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ export function validateUploadedMedia(
3333
"stream=codec_type",
3434
"-of",
3535
"json",
36+
"--",
3637
filePath,
3738
]);
3839

0 commit comments

Comments
 (0)