From f1726e1895a6400e12eb6cf85ff7407525b02317 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Sat, 22 Aug 2026 15:09:08 -0700 Subject: [PATCH 1/3] fix(media): bound the ffmpeg tool's child processes, inputs and scale targets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The copilot ffmpeg tool shells out to FFmpeg in the Sim app process — not in a sandbox — and nothing bounded the run. runCommand had no timer, no kill and no signal, so a transcode ran until it finished and survived the request that asked for it: a stopped copilot turn left the encode pinning both cores of a shared instance. Every operation now shares one 10-minute wall-clock deadline, and both the deadline and the caller's cancellation SIGKILL the child. The budget is per-operation rather than per-command because concat runs a full re-encode per input, so a per-command timeout would let N inputs multiply into N timeouts. Cancellation is wired to context.abortSignal, whose every abort reason is an explicit user stop — the copilot lifecycle tracks a passive client disconnect separately and does not abort on it — so an encode dies when the user asks and not before. (userStopSignal, which assertServerToolNotAborted reads, has no producer on this path.) ffprobe had the same shape in miniature: fluent-ffmpeg's static ffprobe hands back no process handle, so a timeout could only race the callback and leave a wedged prober alive, and concat probes once per input. It now runs through execFile, which takes timeout, killSignal and signal natively. Scale targets reached the filter graph unvalidated, where libavfilter sizes its per-frame buffers from them — scale=30000:30000 is ~2.7 GB a frame, allocated in a child that shares the instance's memory. Dimensions are now rejected outside 16-4096 with a message the model can act on, plus an area cap. concat's targets come from the source container rather than a caller assertion, so those are clamped instead. Two further holes: readOut buffered the output with no ceiling, and CRF-18 re-encodes routinely exceed their input, so the input budget did not bound it; and the handler accepted unlimited input files. Both are capped, and the byte check now runs against the recorded size before the download rather than after. FFMPEG_LIMITS is the single source for the four numbers the Go tool catalog mirrors into its schema, pinned by ffmpeg-schema-parity.test.ts so the model is never told a ceiling the executor does not enforce. Companion: simstudioai/copilot#PENDING Co-Authored-By: Claude Opus 5 (1M context) --- .../lib/copilot/generated/tool-catalog-v1.ts | 269 ++---------- .../lib/copilot/generated/tool-schemas-v1.ts | 287 ++----------- .../copilot/tools/server/media/ffmpeg.test.ts | 88 ++++ .../lib/copilot/tools/server/media/ffmpeg.ts | 53 ++- apps/sim/lib/media/ffmpeg-limits.ts | 19 + .../lib/media/ffmpeg-schema-parity.test.ts | 55 +++ apps/sim/lib/media/ffmpeg.test.ts | 188 +++++++- apps/sim/lib/media/ffmpeg.ts | 405 ++++++++++++++---- 8 files changed, 760 insertions(+), 604 deletions(-) create mode 100644 apps/sim/lib/media/ffmpeg-limits.ts create mode 100644 apps/sim/lib/media/ffmpeg-schema-parity.test.ts diff --git a/apps/sim/lib/copilot/generated/tool-catalog-v1.ts b/apps/sim/lib/copilot/generated/tool-catalog-v1.ts index ab7556186a3..bbb44542619 100644 --- a/apps/sim/lib/copilot/generated/tool-catalog-v1.ts +++ b/apps/sim/lib/copilot/generated/tool-catalog-v1.ts @@ -2348,67 +2348,32 @@ export const Ffmpeg: ToolCatalogEntry = { type: 'string', description: 'Target format/extension for convert (e.g. mp4, mp3, wav, gif).', }, - height: { type: 'number', description: 'Target height in pixels (scale_pad).' }, + height: { + type: 'number', + description: + 'Target height in pixels (scale_pad). 16-4096, and width x height must not exceed 4096 x 2304.', + minimum: 16, + maximum: 4096, + }, inputs: { type: 'object', description: - 'Workspace resources to mount into the sandbox. Copy paths verbatim from glob/read/grep output — they are percent-encoded per segment (spaces are %20, an in-name slash is %2F; parentheses and dots stay literal). Both the encoded path and the plain name resolve, so copy the returned path exactly rather than retyping or decoding it.', + 'Workspace files this tool reads. Copy paths verbatim from glob/read/grep output — they are percent-encoded per segment (spaces are %20, an in-name slash is %2F; parentheses and dots stay literal). Both the encoded path and the plain name resolve, so copy the returned path exactly rather than retyping or decoding it.', properties: { - directories: { - type: 'array', - description: - 'Workspace folders to mount recursively into the sandbox, including nested files and empty folders.', - items: { - type: 'object', - properties: { - path: { - type: 'string', - description: - 'Canonical VFS folder path, e.g. "files/Reports". By default this mounts at "/home/user/{path}".', - }, - sandboxPath: { - type: 'string', - description: - 'Optional full sandbox directory path override. Omit to mount at /home/user/{path}.', - }, - }, - required: ['path'], - }, - }, files: { type: 'array', - description: 'Workspace files to mount into the sandbox.', + description: 'Workspace files to read, in the order this operation expects them.', items: { type: 'object', properties: { path: { type: 'string', - description: - 'Canonical VFS file path, e.g. "files/Reports/sales.csv". By default this mounts at "/home/user/{path}".', - }, - sandboxPath: { - type: 'string', - description: - 'Full sandbox path to mount at, e.g. /home/user/inputs/data.csv. STRONGLY RECOMMENDED whenever the file name has spaces or special characters: the default mount path is the percent-ENCODED canonical path (e.g. /home/user/files/Q4%20Sales%20(Final).csv), which code using the human-readable name will not find. Set a simple sandboxPath and read exactly that.', + description: 'Canonical VFS file path, e.g. "files/Reports/clip.mp4".', }, }, required: ['path'], }, - }, - tables: { - type: 'array', - description: 'Workspace tables to mount as CSV files.', - items: { - type: 'object', - properties: { - path: { type: 'string', description: 'Canonical VFS table path when available.' }, - sandboxPath: { - type: 'string', - description: 'Optional full sandbox path for the mounted CSV.', - }, - tableId: { type: 'string', description: 'Workspace table ID.' }, - }, - }, + maxItems: 20, }, }, }, @@ -2440,8 +2405,7 @@ export const Ffmpeg: ToolCatalogEntry = { }, outputs: { type: 'object', - description: - 'Workspace files to create or overwrite from returned code results or sandbox-created files.', + description: "Workspace files to create or overwrite with this tool's result.", properties: { files: { type: 'array', @@ -2450,11 +2414,6 @@ export const Ffmpeg: ToolCatalogEntry = { items: { type: 'object', properties: { - format: { - type: 'string', - description: 'Optional serialization format for returned values.', - enum: ['json', 'csv', 'txt', 'md', 'html'], - }, mimeType: { type: 'string', description: 'Optional MIME type override when inference is not enough.', @@ -2466,12 +2425,7 @@ export const Ffmpeg: ToolCatalogEntry = { }, path: { type: 'string', - description: 'Canonical destination VFS path, e.g. "files/Reports/chart.png".', - }, - sandboxPath: { - type: 'string', - description: - 'Optional full path to a file created inside the sandbox. Omit to save the code return value.', + description: 'Canonical destination VFS path, e.g. "files/Reports/clip.mp4".', }, }, required: ['path', 'mode'], @@ -2490,7 +2444,13 @@ export const Ffmpeg: ToolCatalogEntry = { type: 'number', description: 'Volume multiplier for the primary track (mix_audio / overlay_audio).', }, - width: { type: 'number', description: 'Target width in pixels (scale_pad).' }, + width: { + type: 'number', + description: + 'Target width in pixels (scale_pad). 16-4096, and width x height must not exceed 4096 x 2304.', + minimum: 16, + maximum: 4096, + }, }, required: ['operation', 'inputs'], }, @@ -2567,64 +2527,22 @@ export const GenerateAudio: ToolCatalogEntry = { inputs: { type: 'object', description: - 'Workspace resources to mount into the sandbox. Copy paths verbatim from glob/read/grep output — they are percent-encoded per segment (spaces are %20, an in-name slash is %2F; parentheses and dots stay literal). Both the encoded path and the plain name resolve, so copy the returned path exactly rather than retyping or decoding it.', + 'Workspace files this tool reads. Copy paths verbatim from glob/read/grep output — they are percent-encoded per segment (spaces are %20, an in-name slash is %2F; parentheses and dots stay literal). Both the encoded path and the plain name resolve, so copy the returned path exactly rather than retyping or decoding it.', properties: { - directories: { - type: 'array', - description: - 'Workspace folders to mount recursively into the sandbox, including nested files and empty folders.', - items: { - type: 'object', - properties: { - path: { - type: 'string', - description: - 'Canonical VFS folder path, e.g. "files/Reports". By default this mounts at "/home/user/{path}".', - }, - sandboxPath: { - type: 'string', - description: - 'Optional full sandbox directory path override. Omit to mount at /home/user/{path}.', - }, - }, - required: ['path'], - }, - }, files: { type: 'array', - description: 'Workspace files to mount into the sandbox.', + description: 'Workspace files to read, in the order this operation expects them.', items: { type: 'object', properties: { path: { type: 'string', - description: - 'Canonical VFS file path, e.g. "files/Reports/sales.csv". By default this mounts at "/home/user/{path}".', - }, - sandboxPath: { - type: 'string', - description: - 'Full sandbox path to mount at, e.g. /home/user/inputs/data.csv. STRONGLY RECOMMENDED whenever the file name has spaces or special characters: the default mount path is the percent-ENCODED canonical path (e.g. /home/user/files/Q4%20Sales%20(Final).csv), which code using the human-readable name will not find. Set a simple sandboxPath and read exactly that.', + description: 'Canonical VFS file path, e.g. "files/Reports/clip.mp4".', }, }, required: ['path'], }, }, - tables: { - type: 'array', - description: 'Workspace tables to mount as CSV files.', - items: { - type: 'object', - properties: { - path: { type: 'string', description: 'Canonical VFS table path when available.' }, - sandboxPath: { - type: 'string', - description: 'Optional full sandbox path for the mounted CSV.', - }, - tableId: { type: 'string', description: 'Workspace table ID.' }, - }, - }, - }, }, }, instrumental: { @@ -2644,8 +2562,7 @@ export const GenerateAudio: ToolCatalogEntry = { }, outputs: { type: 'object', - description: - 'Workspace files to create or overwrite from returned code results or sandbox-created files.', + description: "Workspace files to create or overwrite with this tool's result.", properties: { files: { type: 'array', @@ -2654,11 +2571,6 @@ export const GenerateAudio: ToolCatalogEntry = { items: { type: 'object', properties: { - format: { - type: 'string', - description: 'Optional serialization format for returned values.', - enum: ['json', 'csv', 'txt', 'md', 'html'], - }, mimeType: { type: 'string', description: 'Optional MIME type override when inference is not enough.', @@ -2670,12 +2582,7 @@ export const GenerateAudio: ToolCatalogEntry = { }, path: { type: 'string', - description: 'Canonical destination VFS path, e.g. "files/Reports/chart.png".', - }, - sandboxPath: { - type: 'string', - description: - 'Optional full path to a file created inside the sandbox. Omit to save the code return value.', + description: 'Canonical destination VFS path, e.g. "files/Reports/clip.mp4".', }, }, required: ['path', 'mode'], @@ -2717,70 +2624,27 @@ export const GenerateImage: ToolCatalogEntry = { inputs: { type: 'object', description: - 'Workspace resources to mount into the sandbox. Copy paths verbatim from glob/read/grep output — they are percent-encoded per segment (spaces are %20, an in-name slash is %2F; parentheses and dots stay literal). Both the encoded path and the plain name resolve, so copy the returned path exactly rather than retyping or decoding it.', + 'Workspace files this tool reads. Copy paths verbatim from glob/read/grep output — they are percent-encoded per segment (spaces are %20, an in-name slash is %2F; parentheses and dots stay literal). Both the encoded path and the plain name resolve, so copy the returned path exactly rather than retyping or decoding it.', properties: { - directories: { - type: 'array', - description: - 'Workspace folders to mount recursively into the sandbox, including nested files and empty folders.', - items: { - type: 'object', - properties: { - path: { - type: 'string', - description: - 'Canonical VFS folder path, e.g. "files/Reports". By default this mounts at "/home/user/{path}".', - }, - sandboxPath: { - type: 'string', - description: - 'Optional full sandbox directory path override. Omit to mount at /home/user/{path}.', - }, - }, - required: ['path'], - }, - }, files: { type: 'array', - description: 'Workspace files to mount into the sandbox.', + description: 'Workspace files to read, in the order this operation expects them.', items: { type: 'object', properties: { path: { type: 'string', - description: - 'Canonical VFS file path, e.g. "files/Reports/sales.csv". By default this mounts at "/home/user/{path}".', - }, - sandboxPath: { - type: 'string', - description: - 'Full sandbox path to mount at, e.g. /home/user/inputs/data.csv. STRONGLY RECOMMENDED whenever the file name has spaces or special characters: the default mount path is the percent-ENCODED canonical path (e.g. /home/user/files/Q4%20Sales%20(Final).csv), which code using the human-readable name will not find. Set a simple sandboxPath and read exactly that.', + description: 'Canonical VFS file path, e.g. "files/Reports/clip.mp4".', }, }, required: ['path'], }, }, - tables: { - type: 'array', - description: 'Workspace tables to mount as CSV files.', - items: { - type: 'object', - properties: { - path: { type: 'string', description: 'Canonical VFS table path when available.' }, - sandboxPath: { - type: 'string', - description: 'Optional full sandbox path for the mounted CSV.', - }, - tableId: { type: 'string', description: 'Workspace table ID.' }, - }, - }, - }, }, }, outputs: { type: 'object', - description: - 'Workspace files to create or overwrite from returned code results or sandbox-created files.', + description: "Workspace files to create or overwrite with this tool's result.", properties: { files: { type: 'array', @@ -2789,11 +2653,6 @@ export const GenerateImage: ToolCatalogEntry = { items: { type: 'object', properties: { - format: { - type: 'string', - description: 'Optional serialization format for returned values.', - enum: ['json', 'csv', 'txt', 'md', 'html'], - }, mimeType: { type: 'string', description: 'Optional MIME type override when inference is not enough.', @@ -2805,12 +2664,7 @@ export const GenerateImage: ToolCatalogEntry = { }, path: { type: 'string', - description: 'Canonical destination VFS path, e.g. "files/Reports/chart.png".', - }, - sandboxPath: { - type: 'string', - description: - 'Optional full path to a file created inside the sandbox. Omit to save the code return value.', + description: 'Canonical destination VFS path, e.g. "files/Reports/clip.mp4".', }, }, required: ['path', 'mode'], @@ -2855,64 +2709,22 @@ export const GenerateVideo: ToolCatalogEntry = { inputs: { type: 'object', description: - 'Workspace resources to mount into the sandbox. Copy paths verbatim from glob/read/grep output — they are percent-encoded per segment (spaces are %20, an in-name slash is %2F; parentheses and dots stay literal). Both the encoded path and the plain name resolve, so copy the returned path exactly rather than retyping or decoding it.', + 'Workspace files this tool reads. Copy paths verbatim from glob/read/grep output — they are percent-encoded per segment (spaces are %20, an in-name slash is %2F; parentheses and dots stay literal). Both the encoded path and the plain name resolve, so copy the returned path exactly rather than retyping or decoding it.', properties: { - directories: { - type: 'array', - description: - 'Workspace folders to mount recursively into the sandbox, including nested files and empty folders.', - items: { - type: 'object', - properties: { - path: { - type: 'string', - description: - 'Canonical VFS folder path, e.g. "files/Reports". By default this mounts at "/home/user/{path}".', - }, - sandboxPath: { - type: 'string', - description: - 'Optional full sandbox directory path override. Omit to mount at /home/user/{path}.', - }, - }, - required: ['path'], - }, - }, files: { type: 'array', - description: 'Workspace files to mount into the sandbox.', + description: 'Workspace files to read, in the order this operation expects them.', items: { type: 'object', properties: { path: { type: 'string', - description: - 'Canonical VFS file path, e.g. "files/Reports/sales.csv". By default this mounts at "/home/user/{path}".', - }, - sandboxPath: { - type: 'string', - description: - 'Full sandbox path to mount at, e.g. /home/user/inputs/data.csv. STRONGLY RECOMMENDED whenever the file name has spaces or special characters: the default mount path is the percent-ENCODED canonical path (e.g. /home/user/files/Q4%20Sales%20(Final).csv), which code using the human-readable name will not find. Set a simple sandboxPath and read exactly that.', + description: 'Canonical VFS file path, e.g. "files/Reports/clip.mp4".', }, }, required: ['path'], }, }, - tables: { - type: 'array', - description: 'Workspace tables to mount as CSV files.', - items: { - type: 'object', - properties: { - path: { type: 'string', description: 'Canonical VFS table path when available.' }, - sandboxPath: { - type: 'string', - description: 'Optional full sandbox path for the mounted CSV.', - }, - tableId: { type: 'string', description: 'Workspace table ID.' }, - }, - }, - }, }, }, model: { @@ -2938,8 +2750,7 @@ export const GenerateVideo: ToolCatalogEntry = { }, outputs: { type: 'object', - description: - 'Workspace files to create or overwrite from returned code results or sandbox-created files.', + description: "Workspace files to create or overwrite with this tool's result.", properties: { files: { type: 'array', @@ -2948,11 +2759,6 @@ export const GenerateVideo: ToolCatalogEntry = { items: { type: 'object', properties: { - format: { - type: 'string', - description: 'Optional serialization format for returned values.', - enum: ['json', 'csv', 'txt', 'md', 'html'], - }, mimeType: { type: 'string', description: 'Optional MIME type override when inference is not enough.', @@ -2964,12 +2770,7 @@ export const GenerateVideo: ToolCatalogEntry = { }, path: { type: 'string', - description: 'Canonical destination VFS path, e.g. "files/Reports/chart.png".', - }, - sandboxPath: { - type: 'string', - description: - 'Optional full path to a file created inside the sandbox. Omit to save the code return value.', + description: 'Canonical destination VFS path, e.g. "files/Reports/clip.mp4".', }, }, required: ['path', 'mode'], diff --git a/apps/sim/lib/copilot/generated/tool-schemas-v1.ts b/apps/sim/lib/copilot/generated/tool-schemas-v1.ts index a6070e7f09f..c7f0cfcd3bf 100644 --- a/apps/sim/lib/copilot/generated/tool-schemas-v1.ts +++ b/apps/sim/lib/copilot/generated/tool-schemas-v1.ts @@ -2301,74 +2301,30 @@ export const TOOL_RUNTIME_SCHEMAS: Record = { }, height: { type: 'number', - description: 'Target height in pixels (scale_pad).', + description: + 'Target height in pixels (scale_pad). 16-4096, and width x height must not exceed 4096 x 2304.', + minimum: 16, + maximum: 4096, }, inputs: { type: 'object', description: - 'Workspace resources to mount into the sandbox. Copy paths verbatim from glob/read/grep output — they are percent-encoded per segment (spaces are %20, an in-name slash is %2F; parentheses and dots stay literal). Both the encoded path and the plain name resolve, so copy the returned path exactly rather than retyping or decoding it.', + 'Workspace files this tool reads. Copy paths verbatim from glob/read/grep output — they are percent-encoded per segment (spaces are %20, an in-name slash is %2F; parentheses and dots stay literal). Both the encoded path and the plain name resolve, so copy the returned path exactly rather than retyping or decoding it.', properties: { - directories: { - type: 'array', - description: - 'Workspace folders to mount recursively into the sandbox, including nested files and empty folders.', - items: { - type: 'object', - properties: { - path: { - type: 'string', - description: - 'Canonical VFS folder path, e.g. "files/Reports". By default this mounts at "/home/user/{path}".', - }, - sandboxPath: { - type: 'string', - description: - 'Optional full sandbox directory path override. Omit to mount at /home/user/{path}.', - }, - }, - required: ['path'], - }, - }, files: { type: 'array', - description: 'Workspace files to mount into the sandbox.', + description: 'Workspace files to read, in the order this operation expects them.', items: { type: 'object', properties: { path: { type: 'string', - description: - 'Canonical VFS file path, e.g. "files/Reports/sales.csv". By default this mounts at "/home/user/{path}".', - }, - sandboxPath: { - type: 'string', - description: - 'Full sandbox path to mount at, e.g. /home/user/inputs/data.csv. STRONGLY RECOMMENDED whenever the file name has spaces or special characters: the default mount path is the percent-ENCODED canonical path (e.g. /home/user/files/Q4%20Sales%20(Final).csv), which code using the human-readable name will not find. Set a simple sandboxPath and read exactly that.', + description: 'Canonical VFS file path, e.g. "files/Reports/clip.mp4".', }, }, required: ['path'], }, - }, - tables: { - type: 'array', - description: 'Workspace tables to mount as CSV files.', - items: { - type: 'object', - properties: { - path: { - type: 'string', - description: 'Canonical VFS table path when available.', - }, - sandboxPath: { - type: 'string', - description: 'Optional full sandbox path for the mounted CSV.', - }, - tableId: { - type: 'string', - description: 'Workspace table ID.', - }, - }, - }, + maxItems: 20, }, }, }, @@ -2400,8 +2356,7 @@ export const TOOL_RUNTIME_SCHEMAS: Record = { }, outputs: { type: 'object', - description: - 'Workspace files to create or overwrite from returned code results or sandbox-created files.', + description: "Workspace files to create or overwrite with this tool's result.", properties: { files: { type: 'array', @@ -2410,11 +2365,6 @@ export const TOOL_RUNTIME_SCHEMAS: Record = { items: { type: 'object', properties: { - format: { - type: 'string', - description: 'Optional serialization format for returned values.', - enum: ['json', 'csv', 'txt', 'md', 'html'], - }, mimeType: { type: 'string', description: 'Optional MIME type override when inference is not enough.', @@ -2426,12 +2376,7 @@ export const TOOL_RUNTIME_SCHEMAS: Record = { }, path: { type: 'string', - description: 'Canonical destination VFS path, e.g. "files/Reports/chart.png".', - }, - sandboxPath: { - type: 'string', - description: - 'Optional full path to a file created inside the sandbox. Omit to save the code return value.', + description: 'Canonical destination VFS path, e.g. "files/Reports/clip.mp4".', }, }, required: ['path', 'mode'], @@ -2458,7 +2403,10 @@ export const TOOL_RUNTIME_SCHEMAS: Record = { }, width: { type: 'number', - description: 'Target width in pixels (scale_pad).', + description: + 'Target width in pixels (scale_pad). 16-4096, and width x height must not exceed 4096 x 2304.', + minimum: 16, + maximum: 4096, }, }, required: ['operation', 'inputs'], @@ -2520,70 +2468,22 @@ export const TOOL_RUNTIME_SCHEMAS: Record = { inputs: { type: 'object', description: - 'Workspace resources to mount into the sandbox. Copy paths verbatim from glob/read/grep output — they are percent-encoded per segment (spaces are %20, an in-name slash is %2F; parentheses and dots stay literal). Both the encoded path and the plain name resolve, so copy the returned path exactly rather than retyping or decoding it.', + 'Workspace files this tool reads. Copy paths verbatim from glob/read/grep output — they are percent-encoded per segment (spaces are %20, an in-name slash is %2F; parentheses and dots stay literal). Both the encoded path and the plain name resolve, so copy the returned path exactly rather than retyping or decoding it.', properties: { - directories: { - type: 'array', - description: - 'Workspace folders to mount recursively into the sandbox, including nested files and empty folders.', - items: { - type: 'object', - properties: { - path: { - type: 'string', - description: - 'Canonical VFS folder path, e.g. "files/Reports". By default this mounts at "/home/user/{path}".', - }, - sandboxPath: { - type: 'string', - description: - 'Optional full sandbox directory path override. Omit to mount at /home/user/{path}.', - }, - }, - required: ['path'], - }, - }, files: { type: 'array', - description: 'Workspace files to mount into the sandbox.', + description: 'Workspace files to read, in the order this operation expects them.', items: { type: 'object', properties: { path: { type: 'string', - description: - 'Canonical VFS file path, e.g. "files/Reports/sales.csv". By default this mounts at "/home/user/{path}".', - }, - sandboxPath: { - type: 'string', - description: - 'Full sandbox path to mount at, e.g. /home/user/inputs/data.csv. STRONGLY RECOMMENDED whenever the file name has spaces or special characters: the default mount path is the percent-ENCODED canonical path (e.g. /home/user/files/Q4%20Sales%20(Final).csv), which code using the human-readable name will not find. Set a simple sandboxPath and read exactly that.', + description: 'Canonical VFS file path, e.g. "files/Reports/clip.mp4".', }, }, required: ['path'], }, }, - tables: { - type: 'array', - description: 'Workspace tables to mount as CSV files.', - items: { - type: 'object', - properties: { - path: { - type: 'string', - description: 'Canonical VFS table path when available.', - }, - sandboxPath: { - type: 'string', - description: 'Optional full sandbox path for the mounted CSV.', - }, - tableId: { - type: 'string', - description: 'Workspace table ID.', - }, - }, - }, - }, }, }, instrumental: { @@ -2603,8 +2503,7 @@ export const TOOL_RUNTIME_SCHEMAS: Record = { }, outputs: { type: 'object', - description: - 'Workspace files to create or overwrite from returned code results or sandbox-created files.', + description: "Workspace files to create or overwrite with this tool's result.", properties: { files: { type: 'array', @@ -2613,11 +2512,6 @@ export const TOOL_RUNTIME_SCHEMAS: Record = { items: { type: 'object', properties: { - format: { - type: 'string', - description: 'Optional serialization format for returned values.', - enum: ['json', 'csv', 'txt', 'md', 'html'], - }, mimeType: { type: 'string', description: 'Optional MIME type override when inference is not enough.', @@ -2629,12 +2523,7 @@ export const TOOL_RUNTIME_SCHEMAS: Record = { }, path: { type: 'string', - description: 'Canonical destination VFS path, e.g. "files/Reports/chart.png".', - }, - sandboxPath: { - type: 'string', - description: - 'Optional full path to a file created inside the sandbox. Omit to save the code return value.', + description: 'Canonical destination VFS path, e.g. "files/Reports/clip.mp4".', }, }, required: ['path', 'mode'], @@ -2673,76 +2562,27 @@ export const TOOL_RUNTIME_SCHEMAS: Record = { inputs: { type: 'object', description: - 'Workspace resources to mount into the sandbox. Copy paths verbatim from glob/read/grep output — they are percent-encoded per segment (spaces are %20, an in-name slash is %2F; parentheses and dots stay literal). Both the encoded path and the plain name resolve, so copy the returned path exactly rather than retyping or decoding it.', + 'Workspace files this tool reads. Copy paths verbatim from glob/read/grep output — they are percent-encoded per segment (spaces are %20, an in-name slash is %2F; parentheses and dots stay literal). Both the encoded path and the plain name resolve, so copy the returned path exactly rather than retyping or decoding it.', properties: { - directories: { - type: 'array', - description: - 'Workspace folders to mount recursively into the sandbox, including nested files and empty folders.', - items: { - type: 'object', - properties: { - path: { - type: 'string', - description: - 'Canonical VFS folder path, e.g. "files/Reports". By default this mounts at "/home/user/{path}".', - }, - sandboxPath: { - type: 'string', - description: - 'Optional full sandbox directory path override. Omit to mount at /home/user/{path}.', - }, - }, - required: ['path'], - }, - }, files: { type: 'array', - description: 'Workspace files to mount into the sandbox.', + description: 'Workspace files to read, in the order this operation expects them.', items: { type: 'object', properties: { path: { type: 'string', - description: - 'Canonical VFS file path, e.g. "files/Reports/sales.csv". By default this mounts at "/home/user/{path}".', - }, - sandboxPath: { - type: 'string', - description: - 'Full sandbox path to mount at, e.g. /home/user/inputs/data.csv. STRONGLY RECOMMENDED whenever the file name has spaces or special characters: the default mount path is the percent-ENCODED canonical path (e.g. /home/user/files/Q4%20Sales%20(Final).csv), which code using the human-readable name will not find. Set a simple sandboxPath and read exactly that.', + description: 'Canonical VFS file path, e.g. "files/Reports/clip.mp4".', }, }, required: ['path'], }, }, - tables: { - type: 'array', - description: 'Workspace tables to mount as CSV files.', - items: { - type: 'object', - properties: { - path: { - type: 'string', - description: 'Canonical VFS table path when available.', - }, - sandboxPath: { - type: 'string', - description: 'Optional full sandbox path for the mounted CSV.', - }, - tableId: { - type: 'string', - description: 'Workspace table ID.', - }, - }, - }, - }, }, }, outputs: { type: 'object', - description: - 'Workspace files to create or overwrite from returned code results or sandbox-created files.', + description: "Workspace files to create or overwrite with this tool's result.", properties: { files: { type: 'array', @@ -2751,11 +2591,6 @@ export const TOOL_RUNTIME_SCHEMAS: Record = { items: { type: 'object', properties: { - format: { - type: 'string', - description: 'Optional serialization format for returned values.', - enum: ['json', 'csv', 'txt', 'md', 'html'], - }, mimeType: { type: 'string', description: 'Optional MIME type override when inference is not enough.', @@ -2767,12 +2602,7 @@ export const TOOL_RUNTIME_SCHEMAS: Record = { }, path: { type: 'string', - description: 'Canonical destination VFS path, e.g. "files/Reports/chart.png".', - }, - sandboxPath: { - type: 'string', - description: - 'Optional full path to a file created inside the sandbox. Omit to save the code return value.', + description: 'Canonical destination VFS path, e.g. "files/Reports/clip.mp4".', }, }, required: ['path', 'mode'], @@ -2811,70 +2641,22 @@ export const TOOL_RUNTIME_SCHEMAS: Record = { inputs: { type: 'object', description: - 'Workspace resources to mount into the sandbox. Copy paths verbatim from glob/read/grep output — they are percent-encoded per segment (spaces are %20, an in-name slash is %2F; parentheses and dots stay literal). Both the encoded path and the plain name resolve, so copy the returned path exactly rather than retyping or decoding it.', + 'Workspace files this tool reads. Copy paths verbatim from glob/read/grep output — they are percent-encoded per segment (spaces are %20, an in-name slash is %2F; parentheses and dots stay literal). Both the encoded path and the plain name resolve, so copy the returned path exactly rather than retyping or decoding it.', properties: { - directories: { - type: 'array', - description: - 'Workspace folders to mount recursively into the sandbox, including nested files and empty folders.', - items: { - type: 'object', - properties: { - path: { - type: 'string', - description: - 'Canonical VFS folder path, e.g. "files/Reports". By default this mounts at "/home/user/{path}".', - }, - sandboxPath: { - type: 'string', - description: - 'Optional full sandbox directory path override. Omit to mount at /home/user/{path}.', - }, - }, - required: ['path'], - }, - }, files: { type: 'array', - description: 'Workspace files to mount into the sandbox.', + description: 'Workspace files to read, in the order this operation expects them.', items: { type: 'object', properties: { path: { type: 'string', - description: - 'Canonical VFS file path, e.g. "files/Reports/sales.csv". By default this mounts at "/home/user/{path}".', - }, - sandboxPath: { - type: 'string', - description: - 'Full sandbox path to mount at, e.g. /home/user/inputs/data.csv. STRONGLY RECOMMENDED whenever the file name has spaces or special characters: the default mount path is the percent-ENCODED canonical path (e.g. /home/user/files/Q4%20Sales%20(Final).csv), which code using the human-readable name will not find. Set a simple sandboxPath and read exactly that.', + description: 'Canonical VFS file path, e.g. "files/Reports/clip.mp4".', }, }, required: ['path'], }, }, - tables: { - type: 'array', - description: 'Workspace tables to mount as CSV files.', - items: { - type: 'object', - properties: { - path: { - type: 'string', - description: 'Canonical VFS table path when available.', - }, - sandboxPath: { - type: 'string', - description: 'Optional full sandbox path for the mounted CSV.', - }, - tableId: { - type: 'string', - description: 'Workspace table ID.', - }, - }, - }, - }, }, }, model: { @@ -2900,8 +2682,7 @@ export const TOOL_RUNTIME_SCHEMAS: Record = { }, outputs: { type: 'object', - description: - 'Workspace files to create or overwrite from returned code results or sandbox-created files.', + description: "Workspace files to create or overwrite with this tool's result.", properties: { files: { type: 'array', @@ -2910,11 +2691,6 @@ export const TOOL_RUNTIME_SCHEMAS: Record = { items: { type: 'object', properties: { - format: { - type: 'string', - description: 'Optional serialization format for returned values.', - enum: ['json', 'csv', 'txt', 'md', 'html'], - }, mimeType: { type: 'string', description: 'Optional MIME type override when inference is not enough.', @@ -2926,12 +2702,7 @@ export const TOOL_RUNTIME_SCHEMAS: Record = { }, path: { type: 'string', - description: 'Canonical destination VFS path, e.g. "files/Reports/chart.png".', - }, - sandboxPath: { - type: 'string', - description: - 'Optional full path to a file created inside the sandbox. Omit to save the code return value.', + description: 'Canonical destination VFS path, e.g. "files/Reports/clip.mp4".', }, }, required: ['path', 'mode'], diff --git a/apps/sim/lib/copilot/tools/server/media/ffmpeg.test.ts b/apps/sim/lib/copilot/tools/server/media/ffmpeg.test.ts index 26912076ead..0dc94d40797 100644 --- a/apps/sim/lib/copilot/tools/server/media/ffmpeg.test.ts +++ b/apps/sim/lib/copilot/tools/server/media/ffmpeg.test.ts @@ -309,3 +309,91 @@ describe('ffmpeg server tool secret provenance', () => { }) }) }) + +describe('ffmpeg server tool input admission', () => { + const context = { + userId: 'user-1', + workspaceId: 'workspace-1', + toolCallId: 'tool-1', + copilotToolExecution: true as const, + resolvedSecretTraceRegistry: new ResolvedSecretTraceRegistry([], { + userId: 'user-1', + workspaceId: 'workspace-1', + }), + } + + beforeEach(() => { + vi.clearAllMocks() + resolveWorkspaceFileReferenceMock.mockResolvedValue(file) + fetchWorkspaceFileBufferMock.mockResolvedValue(Buffer.from('media')) + getBoundWorkspaceFileSecretProvenanceMock.mockResolvedValue(EXACT_EMPTY) + mergeWorkspaceFileSecretProvenanceMock.mockImplementation(mergeProvenance) + runFfmpegOperationMock.mockResolvedValue({ + buffer: Buffer.from('output'), + ext: 'mp4', + contentType: 'video/mp4', + }) + writeWorkspaceFileByPathMock.mockResolvedValue({ + id: 'output-1', + name: 'converted.mp4', + vfsPath: 'files/converted.mp4', + downloadUrl: '/api/files/serve/converted.mp4', + mode: 'create', + }) + }) + + it('refuses more inputs than one call may transcode, before reading any of them', async () => { + const files = Array.from({ length: 21 }, () => ({ path: 'files/input.mp4' })) + + const result = await ffmpegServerTool.execute( + { operation: 'concat', inputs: { files } }, + context + ) + + expect(result.success).toBe(false) + expect(result.message).toContain('at most 20') + expect(resolveWorkspaceFileReferenceMock).not.toHaveBeenCalled() + expect(runFfmpegOperationMock).not.toHaveBeenCalled() + }) + + it('admits a batch at the input ceiling', async () => { + const files = Array.from({ length: 20 }, () => ({ path: 'files/input.mp4' })) + + const result = await ffmpegServerTool.execute( + { operation: 'concat', inputs: { files } }, + context + ) + + expect(result.success).toBe(true) + expect(runFfmpegOperationMock).toHaveBeenCalledTimes(1) + }) + + it('rejects on the recorded size before spending the download', async () => { + resolveWorkspaceFileReferenceMock.mockResolvedValue({ ...file, size: 300 * 1024 * 1024 }) + + const result = await ffmpegServerTool.execute( + { operation: 'convert', inputs: { files: [{ path: 'files/huge.mp4' }] } }, + context + ) + + expect(result.success).toBe(false) + expect(result.message).toContain('byte limit') + expect(fetchWorkspaceFileBufferMock).not.toHaveBeenCalled() + }) + + it('hands the caller cancellation signal to the transcode', async () => { + const controller = new AbortController() + + await ffmpegServerTool.execute( + { operation: 'convert', inputs: { files: [{ path: 'files/input.mp4' }] } }, + { ...context, abortSignal: controller.signal } + ) + + expect(runFfmpegOperationMock).toHaveBeenCalledWith( + 'convert', + expect.anything(), + expect.anything(), + { signal: controller.signal } + ) + }) +}) diff --git a/apps/sim/lib/copilot/tools/server/media/ffmpeg.ts b/apps/sim/lib/copilot/tools/server/media/ffmpeg.ts index 8032cf1cb02..3d966894619 100644 --- a/apps/sim/lib/copilot/tools/server/media/ffmpeg.ts +++ b/apps/sim/lib/copilot/tools/server/media/ffmpeg.ts @@ -13,6 +13,7 @@ import { import { writeCopilotWorkspaceFileByPath } from '@/lib/copilot/vfs/resource-writer' import { MAX_MEDIA_BYTES } from '@/lib/media/falai' import { type FfmpegOperation, type MediaFile, runFfmpegOperation } from '@/lib/media/ffmpeg' +import { FFMPEG_LIMITS } from '@/lib/media/ffmpeg-limits' import { createWorkspaceFileSecretProvenanceFromRegistry, getBoundWorkspaceFileSecretProvenance, @@ -26,6 +27,13 @@ import { projectResolvedSecretModelContent } from '@/executor/utils/resolved-sec const logger = createLogger('FfmpegTool') const MEDIA_OPERATION_FAILED_SAFELY = 'The media operation failed safely' +/** + * Backstops the `maxItems` the generated tool schema declares: the byte budget + * below does not bound a call that lists many small clips, and a caller that + * reaches this handler without passing Ajv still must not get an unbounded run. + */ +const { maxInputFiles: MAX_INPUT_FILES } = FFMPEG_LIMITS + const VALID_OPERATIONS: FfmpegOperation[] = [ 'overlay_audio', 'mux', @@ -93,6 +101,12 @@ export const ffmpegServerTool: BaseServerTool = { if (inputPaths.length === 0) { return { success: false, message: 'At least one input file is required in inputs.files' } } + if (inputPaths.length > MAX_INPUT_FILES) { + return { + success: false, + message: `${inputPaths.length} input files were requested; at most ${MAX_INPUT_FILES} are allowed per ffmpeg call. Combine them in batches.`, + } + } let inputRequiresOpaqueError = false try { @@ -108,6 +122,11 @@ export const ffmpegServerTool: BaseServerTool = { reference: filePath, } ) + // Reject on the recorded size before spending the download. The + // accumulated check below still backstops a stale size row. + if (totalInputBytes + fileRecord.size > MAX_MEDIA_BYTES) { + throw new Error(`Input files exceed the ${MAX_MEDIA_BYTES} byte limit`) + } const fileProvenance = await getBoundWorkspaceFileSecretProvenance(workspaceId, { fileId: fileRecord.id, key: fileRecord.key, @@ -141,19 +160,27 @@ export const ffmpegServerTool: BaseServerTool = { inputRequiresOpaqueError ||= inputProvenance.status !== 'exact' || inputProvenance.entries.length > 0 assertServerToolNotAborted(context) - const result = await runFfmpegOperation(params.operation, mediaFiles, { - text: params.text, - position: params.position, - start: params.start, - end: params.end, - width: params.width, - height: params.height, - aspectRatio: params.aspectRatio, - volume: params.volume, - musicVolume: params.musicVolume, - loopToVideo: params.loopToVideo, - format: params.format, - }) + const result = await runFfmpegOperation( + params.operation, + mediaFiles, + { + text: params.text, + position: params.position, + start: params.start, + end: params.end, + width: params.width, + height: params.height, + aspectRatio: params.aspectRatio, + volume: params.volume, + musicVolume: params.musicVolume, + loopToVideo: params.loopToVideo, + format: params.format, + }, + // Every abort of this signal is an explicit user stop — the copilot + // lifecycle tracks a passive client disconnect separately and does not + // abort on it — so a transcode dies when the user says stop, and only then. + { signal: context.abortSignal } + ) // probe reports metadata only — no file written. if (params.operation === 'probe') { diff --git a/apps/sim/lib/media/ffmpeg-limits.ts b/apps/sim/lib/media/ffmpeg-limits.ts new file mode 100644 index 00000000000..6167efa9ee5 --- /dev/null +++ b/apps/sim/lib/media/ffmpeg-limits.ts @@ -0,0 +1,19 @@ +/** + * Execution bounds the ffmpeg tool enforces. + * + * These are mirrored into the Go tool catalog + * (`copilot/internal/tools/catalog/other/ffmpeg.go`) so the model reads the + * limits off its own schema instead of discovering them as a failed tool call. + * `ffmpeg-schema-parity.test.ts` fails when the two copies drift. + * + * `maxScalePixels` has no JSON Schema equivalent, so it lives in the parameter + * description on the Go side and is enforced here only. + */ +export const FFMPEG_LIMITS = { + /** Every input costs a full re-encode pass in `concat`, the only multi-input operation. */ + maxInputFiles: 20, + minScaleDimension: 16, + maxScaleDimension: 4096, + /** DCI 4K in either orientation — bounds the square frames the per-axis cap alone allows. */ + maxScalePixels: 4096 * 2304, +} as const diff --git a/apps/sim/lib/media/ffmpeg-schema-parity.test.ts b/apps/sim/lib/media/ffmpeg-schema-parity.test.ts new file mode 100644 index 00000000000..f9057738bac --- /dev/null +++ b/apps/sim/lib/media/ffmpeg-schema-parity.test.ts @@ -0,0 +1,55 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { TOOL_RUNTIME_SCHEMAS } from '@/lib/copilot/generated/tool-schemas-v1' +import { FFMPEG_LIMITS } from '@/lib/media/ffmpeg-limits' + +/** + * The ffmpeg bounds live twice: here, where the executor enforces them, and in + * the Go tool catalog, where they become the JSON Schema the model reads and + * Ajv checks at the router. Drift between the two is silent and user-visible — + * the model is told one ceiling and the transcode refuses at another — so pin + * the generated schema against the executor's own numbers. + * + * When this fails, change `ffmpeg.go` in the copilot repo and regenerate; do + * not edit the generated schema. + */ +interface SchemaNode { + properties?: Record + items?: SchemaNode + maxItems?: number + minimum?: number + maximum?: number +} + +const ffmpegParameters = TOOL_RUNTIME_SCHEMAS.ffmpeg?.parameters as SchemaNode | undefined + +describe('ffmpeg tool schema parity', () => { + it('declares the tool in the generated catalog', () => { + expect(ffmpegParameters?.properties).toBeDefined() + }) + + it('caps inputs.files at the executor limit', () => { + expect(ffmpegParameters?.properties?.inputs?.properties?.files?.maxItems).toBe( + FFMPEG_LIMITS.maxInputFiles + ) + }) + + it('bounds the scale dimensions at the executor limits', () => { + for (const axis of ['width', 'height'] as const) { + expect(ffmpegParameters?.properties?.[axis]?.minimum).toBe(FFMPEG_LIMITS.minScaleDimension) + expect(ffmpegParameters?.properties?.[axis]?.maximum).toBe(FFMPEG_LIMITS.maxScaleDimension) + } + }) + + it('does not offer sandbox-only fields on a tool that runs in this process', () => { + const inputs = ffmpegParameters?.properties?.inputs?.properties + expect(Object.keys(inputs ?? {})).toEqual(['files']) + expect(inputs?.files?.items?.properties).toBeDefined() + expect(Object.keys(inputs?.files?.items?.properties ?? {})).toEqual(['path']) + + const outputItem = ffmpegParameters?.properties?.outputs?.properties?.files?.items?.properties + expect(Object.keys(outputItem ?? {}).sort()).toEqual(['mimeType', 'mode', 'path']) + }) +}) diff --git a/apps/sim/lib/media/ffmpeg.test.ts b/apps/sim/lib/media/ffmpeg.test.ts index c004f06081b..480f766bcfe 100644 --- a/apps/sim/lib/media/ffmpeg.test.ts +++ b/apps/sim/lib/media/ffmpeg.test.ts @@ -3,15 +3,29 @@ */ import fs from 'node:fs' import path from 'node:path' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -const { capturedVideoFilters, capturedCaptions } = vi.hoisted(() => ({ - capturedVideoFilters: [] as string[], - capturedCaptions: [] as string[], -})) +const { capturedVideoFilters, capturedCaptions, killSignals, probeReport, command, saves } = + vi.hoisted(() => ({ + capturedVideoFilters: [] as string[], + capturedCaptions: [] as string[], + killSignals: [] as string[], + probeReport: { json: '{"streams":[],"format":{}}' }, + command: { hang: false }, + saves: { waiters: [] as Array<() => void> }, + })) vi.mock('node:child_process', () => ({ - execSync: () => '/usr/bin/ffmpeg\n', + execSync: (cmd: string) => + String(cmd).includes('ffprobe') ? '/usr/bin/ffprobe\n' : '/usr/bin/ffmpeg\n', + // `promisify` honors this symbol the same way it does for the real execFile, + // so the module under test destructures `{ stdout }` exactly as in production. + execFile: Object.assign(() => undefined, { + [Symbol.for('nodejs.util.promisify.custom')]: async () => ({ + stdout: probeReport.json, + stderr: '', + }), + }), })) vi.mock('fluent-ffmpeg', () => { @@ -32,6 +46,9 @@ vi.mock('fluent-ffmpeg', () => { cmd.setDuration = chain() cmd.seekInput = chain() cmd.frames = chain() + cmd.kill = chain((signal) => { + killSignals.push(String(signal)) + }) cmd.videoFilters = chain((arg) => { const filter = String(arg) capturedVideoFilters.push(filter) @@ -48,6 +65,10 @@ vi.mock('fluent-ffmpeg', () => { return cmd } cmd.save = (outputPath: string) => { + for (const resolve of saves.waiters.splice(0)) resolve() + // A hung command never emits `end`, standing in for an encode that outlives + // the request that asked for it. + if (command.hang) return cmd fs.writeFileSync(outputPath, Buffer.from('stub-output')) handlers.end?.() return cmd @@ -57,10 +78,6 @@ vi.mock('fluent-ffmpeg', () => { const ffmpeg = ((_input?: unknown, options?: { cwd?: string }) => makeCommand(options?.cwd)) as unknown as Record & (() => unknown) ;(ffmpeg as Record).setFfmpegPath = () => {} - ;(ffmpeg as Record).ffprobe = ( - _path: string, - cb: (err: unknown, data: unknown) => void - ) => cb(null, { streams: [], format: {} }) return { default: ffmpeg } }) @@ -72,12 +89,25 @@ const videoInput = { name: 'clip.mp4', } -describe('runFfmpegOperation add_text filtergraph injection', () => { - beforeEach(() => { - capturedVideoFilters.length = 0 - capturedCaptions.length = 0 - }) +/** Resolves once the next FFmpeg command reaches `.save()`, i.e. once it is running. */ +function nextSave(): Promise { + return new Promise((resolve) => saves.waiters.push(resolve)) +} + +beforeEach(() => { + capturedVideoFilters.length = 0 + capturedCaptions.length = 0 + killSignals.length = 0 + saves.waiters.length = 0 + command.hang = false + probeReport.json = '{"streams":[],"format":{}}' +}) +afterEach(() => { + vi.useRealTimers() +}) + +describe('runFfmpegOperation add_text filtergraph injection', () => { it('routes the caption through textfile= so it never becomes filtergraph syntax', async () => { await runFfmpegOperation('add_text', [videoInput], { text: 'Hello World :) 100% done' }) @@ -114,3 +144,131 @@ describe('runFfmpegOperation add_text filtergraph injection', () => { expect(capturedCaptions).toEqual([payload]) }) }) + +describe('runFfmpegOperation scale targets', () => { + it('renders a known aspect ratio at its preset size', async () => { + await runFfmpegOperation('scale_pad', [videoInput], { aspectRatio: '9:16' }) + + expect(capturedVideoFilters[0]).toContain('scale=1080:1920') + }) + + it('accepts explicit dimensions up to the 4K ceiling', async () => { + await runFfmpegOperation('scale_pad', [videoInput], { width: 3840, height: 2160 }) + + expect(capturedVideoFilters[0]).toContain('scale=3840:2160') + }) + + it('rejects a dimension above the per-axis ceiling before spawning FFmpeg', async () => { + await expect( + runFfmpegOperation('scale_pad', [videoInput], { width: 30000, height: 30000 }) + ).rejects.toThrow(/width must be between 16 and 4096/) + expect(capturedVideoFilters).toHaveLength(0) + }) + + it('rejects a frame whose area exceeds the pixel budget even when both axes fit', async () => { + await expect( + runFfmpegOperation('scale_pad', [videoInput], { width: 4096, height: 4096 }) + ).rejects.toThrow(/pixel limit/) + expect(capturedVideoFilters).toHaveLength(0) + }) + + it('rejects a non-finite dimension', async () => { + await expect( + runFfmpegOperation('scale_pad', [videoInput], { + width: Number.POSITIVE_INFINITY, + height: 720, + }) + ).rejects.toThrow(/must be a finite number/) + expect(capturedVideoFilters).toHaveLength(0) + }) +}) + +describe('runFfmpegOperation process bounds', () => { + it('kills a command that outlives the operation budget', async () => { + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }) + command.hang = true + + const saved = nextSave() + const result = runFfmpegOperation('convert', [videoInput], { format: 'mp4' }) + await saved + await vi.advanceTimersByTimeAsync(60 * 60 * 1000) + + await expect(result).rejects.toThrow(/media operation limit/) + expect(killSignals).toEqual(['SIGKILL']) + }) + + it('kills a running command when the caller cancels', async () => { + command.hang = true + const controller = new AbortController() + + const saved = nextSave() + const result = runFfmpegOperation( + 'convert', + [videoInput], + { format: 'mp4' }, + { signal: controller.signal } + ) + await saved + controller.abort() + + await expect(result).rejects.toThrow('FFmpeg cancelled') + expect(killSignals).toEqual(['SIGKILL']) + }) + + it('refuses to spawn anything once the caller has already cancelled', async () => { + const controller = new AbortController() + controller.abort() + + await expect( + runFfmpegOperation('convert', [videoInput], { format: 'mp4' }, { signal: controller.signal }) + ).rejects.toThrow('FFmpeg cancelled') + expect(killSignals).toHaveLength(0) + }) + + it('leaves a command that finishes inside the budget untouched', async () => { + const result = await runFfmpegOperation('convert', [videoInput], { format: 'mp4' }) + + expect(result.buffer?.toString()).toBe('stub-output') + expect(killSignals).toHaveLength(0) + }) +}) + +describe('runFfmpegOperation probe', () => { + it('maps the ffprobe report onto the media probe shape', async () => { + probeReport.json = JSON.stringify({ + streams: [ + { codec_type: 'video', codec_name: 'h264', width: 1920, height: 1080 }, + { codec_type: 'audio', codec_name: 'aac' }, + ], + format: { duration: '12.5', format_name: 'mov,mp4,m4a' }, + }) + + const result = await runFfmpegOperation('probe', [videoInput]) + + expect(result.probe).toEqual({ + durationSeconds: 12.5, + format: 'mov,mp4,m4a', + width: 1920, + height: 1080, + videoCodec: 'h264', + audioCodec: 'aac', + hasAudio: true, + hasVideo: true, + }) + }) + + it('reports a container with no streams without inventing metadata', async () => { + const result = await runFfmpegOperation('probe', [videoInput]) + + expect(result.probe).toEqual({ + durationSeconds: 0, + format: 'unknown', + width: undefined, + height: undefined, + videoCodec: undefined, + audioCodec: undefined, + hasAudio: false, + hasVideo: false, + }) + }) +}) diff --git a/apps/sim/lib/media/ffmpeg.ts b/apps/sim/lib/media/ffmpeg.ts index 9cc94c84a19..93b062aa406 100644 --- a/apps/sim/lib/media/ffmpeg.ts +++ b/apps/sim/lib/media/ffmpeg.ts @@ -1,34 +1,71 @@ -import { execSync } from 'node:child_process' +import { execFile, execSync } from 'node:child_process' import fs from 'node:fs/promises' import os from 'node:os' import path from 'node:path' +import { promisify } from 'node:util' import { createLogger } from '@sim/logger' +import { toError } from '@sim/utils/errors' import ffmpeg from 'fluent-ffmpeg' +import { MAX_MEDIA_BYTES } from '@/lib/media/falai' +import { FFMPEG_LIMITS } from '@/lib/media/ffmpeg-limits' const logger = createLogger('MediaFfmpeg') -let ffmpegInitialized = false +const execFileAsync = promisify(execFile) + +const INSTALL_HINT = + 'Install: brew install ffmpeg (macOS) / apk add ffmpeg (Alpine) / apt-get install ffmpeg (Ubuntu)' + +/** + * Wall-clock budget for one media operation, shared by every child process it + * spawns. The budget is per-operation rather than per-command because `concat` + * runs a full re-encode per input: a per-command timeout would let N inputs + * multiply into N timeouts on a CPU-bound instance that serves every other + * request at the same time. + */ +const OPERATION_TIMEOUT_MS = 10 * 60 * 1000 + +/** Per-probe ceiling, additionally bounded by whatever remains of the operation budget. */ +const PROBE_TIMEOUT_MS = 30 * 1000 + +/** Headroom for ffprobe's JSON report on a container with many streams. */ +const PROBE_MAX_OUTPUT_BYTES = 4 * 1024 * 1024 + +const { + minScaleDimension: MIN_SCALE_DIMENSION, + maxScaleDimension: MAX_SCALE_DIMENSION, + maxScalePixels: MAX_SCALE_PIXELS, +} = FFMPEG_LIMITS + +let binariesInitialized = false let ffmpegPath: string | null = null +let ffprobePath: string | null = null + +function resolveBinary(binary: string): string | null { + try { + const cmd = process.platform === 'win32' ? `where ${binary}` : `which ${binary}` + return execSync(cmd, { encoding: 'utf-8' }).trim().split('\n')[0] || null + } catch { + return null + } +} /** Lazy system FFmpeg binary resolution, mirroring lib/audio/extractor.ts. */ function ensureFfmpeg(): void { - if (ffmpegInitialized) { - if (!ffmpegPath) { - throw new Error( - 'FFmpeg not found. Install: brew install ffmpeg (macOS) / apk add ffmpeg (Alpine) / apt-get install ffmpeg (Ubuntu)' - ) - } - return + if (!binariesInitialized) { + binariesInitialized = true + ffmpegPath = resolveBinary('ffmpeg') + ffprobePath = resolveBinary('ffprobe') + if (ffmpegPath) ffmpeg.setFfmpegPath(ffmpegPath) + else logger.warn('[FFmpeg] No FFmpeg binary found at init time') } - ffmpegInitialized = true + if (!ffmpegPath) throw new Error(`FFmpeg not found. ${INSTALL_HINT}`) +} - try { - const cmd = process.platform === 'win32' ? 'where ffmpeg' : 'which ffmpeg' - ffmpegPath = execSync(cmd, { encoding: 'utf-8' }).trim().split('\n')[0] - ffmpeg.setFfmpegPath(ffmpegPath) - } catch { - logger.warn('[FFmpeg] No FFmpeg binary found at init time') - } +function ensureFfprobe(): string { + ensureFfmpeg() + if (!ffprobePath) throw new Error(`FFprobe not found. ${INSTALL_HINT}`) + return ffprobePath } export type FfmpegOperation = @@ -84,6 +121,28 @@ export interface FfmpegResult { probe?: MediaProbe } +export interface FfmpegRunOptions { + /** + * Kills the running child process when the caller cancels. Copilot's tool + * signal fires only on an explicit user stop, never on a passive transport + * disconnect, so a wired encode stops when the user asks and not before. + */ + signal?: AbortSignal +} + +/** Per-operation execution bounds, shared by every child process the operation spawns. */ +interface FfmpegRunContext { + /** Absolute wall-clock deadline for the whole operation. */ + deadlineAt: number + signal?: AbortSignal +} + +const CANCELLED_MESSAGE = 'FFmpeg cancelled' + +function timedOutMessage(): string { + return `FFmpeg exceeded the ${Math.round(OPERATION_TIMEOUT_MS / 1000)}s media operation limit. Use shorter inputs, fewer clips, or a smaller target size.` +} + const MIME_TO_EXT: Record = { 'video/mp4': 'mp4', 'video/mpeg': 'mp4', @@ -186,44 +245,151 @@ async function writeInput(dir: string, file: MediaFile, index: number): Promise< return filePath } -function runCommand(command: ffmpeg.FfmpegCommand, outputPath: string): Promise { +/** + * Run one FFmpeg command under the operation's deadline and cancellation signal. + * + * Without this, a transcode has no bound at all: `.save()` resolves whenever + * FFmpeg happens to finish, so a long input or an oversized filter graph pins + * the instance's cores for as long as it likes and survives the request that + * asked for it. + */ +function runCommand( + ctx: FfmpegRunContext, + command: ffmpeg.FfmpegCommand, + outputPath: string +): Promise { return new Promise((resolve, reject) => { - command - .on('end', () => resolve()) - .on('error', (err) => reject(new Error(`FFmpeg error: ${err.message}`))) - .save(outputPath) + if (ctx.signal?.aborted) { + reject(new Error(CANCELLED_MESSAGE)) + return + } + const remaining = ctx.deadlineAt - Date.now() + if (remaining <= 0) { + reject(new Error(timedOutMessage())) + return + } + + let settled = false + const timer = setTimeout(() => terminate(new Error(timedOutMessage())), remaining) + + function cleanup() { + clearTimeout(timer) + ctx.signal?.removeEventListener('abort', onAbort) + } + function succeed() { + if (settled) return + settled = true + cleanup() + resolve() + } + function fail(error: Error) { + if (settled) return + settled = true + cleanup() + reject(error) + } + /** + * SIGKILL rather than SIGTERM: the process being torn down is either wedged + * or deliberately expensive, and neither deserves a chance to ignore it. + * Settling first makes the `error` event FFmpeg emits on death a no-op, so + * the caller sees why we killed it instead of "killed with signal SIGKILL". + */ + function terminate(error: Error) { + if (settled) return + settled = true + cleanup() + try { + command.kill('SIGKILL') + } catch { + // Already exited — nothing to signal. + } + reject(error) + } + function onAbort() { + terminate(new Error(CANCELLED_MESSAGE)) + } + + ctx.signal?.addEventListener('abort', onAbort, { once: true }) + + try { + command + .on('end', () => succeed()) + .on('error', (err) => fail(new Error(`FFmpeg error: ${err.message}`))) + .save(outputPath) + } catch (error) { + // Settle through `fail` rather than letting the executor throw, so the + // deadline timer is cleared instead of holding the event loop open. + fail(toError(error)) + } }) } -export async function probeMedia(file: MediaFile): Promise { - return withTempDir(async (dir) => { - const inputPath = await writeInput(dir, file, 0) - return probeFile(inputPath) - }) +interface FfprobeReport { + streams?: Array<{ + codec_type?: string + codec_name?: string + width?: number + height?: number + }> + format?: { duration?: string | number; format_name?: string } } -function probeFile(filePath: string): Promise { - ensureFfmpeg() - return new Promise((resolve, reject) => { - ffmpeg.ffprobe(filePath, (err, metadata) => { - if (err) { - reject(new Error(`FFprobe error: ${err.message}`)) - return +/** + * Probe with `execFile` rather than `fluent-ffmpeg`'s static `ffprobe`, which + * hands back no process handle: its callback can only be raced, leaving a + * wedged prober alive on the instance. `concat` probes once per input, so that + * leak scales with the request. + */ +async function probeFile(ctx: FfmpegRunContext, filePath: string): Promise { + const binary = ensureFfprobe() + const timeout = Math.min(PROBE_TIMEOUT_MS, ctx.deadlineAt - Date.now()) + if (timeout <= 0) throw new Error(timedOutMessage()) + + let stdout: string + try { + ;({ stdout } = await execFileAsync( + binary, + ['-v', 'error', '-print_format', 'json', '-show_format', '-show_streams', filePath], + { + timeout, + killSignal: 'SIGKILL', + maxBuffer: PROBE_MAX_OUTPUT_BYTES, + signal: ctx.signal, + encoding: 'utf-8', } - const video = metadata.streams.find((s) => s.codec_type === 'video') - const audio = metadata.streams.find((s) => s.codec_type === 'audio') - resolve({ - durationSeconds: Number(metadata.format?.duration) || 0, - format: metadata.format?.format_name || 'unknown', - width: video?.width, - height: video?.height, - videoCodec: video?.codec_name, - audioCodec: audio?.codec_name, - hasAudio: Boolean(audio), - hasVideo: Boolean(video), - }) - }) - }) + )) + } catch (error) { + const failure = error as NodeJS.ErrnoException & { killed?: boolean; stderr?: string } + if (failure.name === 'AbortError') throw new Error(CANCELLED_MESSAGE) + // Node kills the child for an oversized report too, so check that before + // reading `killed` as "we ran out of time". + if (failure.code === 'ERR_CHILD_PROCESS_STDIO_MAXBUFFER') { + throw new Error('FFprobe error: probe report was too large to read') + } + if (failure.killed) throw new Error(timedOutMessage()) + throw new Error(`FFprobe error: ${failure.stderr?.trim() || failure.message}`) + } + + let report: FfprobeReport + try { + report = JSON.parse(stdout) as FfprobeReport + } catch { + throw new Error('FFprobe error: probe output was not readable') + } + + const streams = report.streams ?? [] + const video = streams.find((s) => s.codec_type === 'video') + const audio = streams.find((s) => s.codec_type === 'audio') + return { + durationSeconds: Number(report.format?.duration) || 0, + format: report.format?.format_name || 'unknown', + width: video?.width, + height: video?.height, + videoCodec: video?.codec_name, + audioCodec: audio?.codec_name, + hasAudio: Boolean(audio), + hasVideo: Boolean(video), + } } /** @@ -233,14 +399,22 @@ function probeFile(filePath: string): Promise { export async function runFfmpegOperation( operation: FfmpegOperation, inputs: MediaFile[], - options: FfmpegOptions = {} + options: FfmpegOptions = {}, + runOptions: FfmpegRunOptions = {} ): Promise { if (inputs.length === 0) { throw new Error('At least one input file is required') } + const ctx: FfmpegRunContext = { + deadlineAt: Date.now() + OPERATION_TIMEOUT_MS, + signal: runOptions.signal, + } + if (operation === 'probe') { - return { probe: await probeMedia(inputs[0]) } + return withTempDir(async (dir) => ({ + probe: await probeFile(ctx, await writeInput(dir, inputs[0], 0)), + })) } return withTempDir(async (dir) => { @@ -249,39 +423,51 @@ export async function runFfmpegOperation( switch (operation) { case 'overlay_audio': case 'mux': - return overlayAudio(dir, inputPaths, options) + return overlayAudio(ctx, dir, inputPaths, options) case 'mix_audio': - return mixAudio(dir, inputPaths, options) + return mixAudio(ctx, dir, inputPaths, options) case 'concat': - return concat(dir, inputPaths) + return concat(ctx, dir, inputPaths) case 'trim': - return trim(dir, inputPaths[0], inputs[0], options) + return trim(ctx, dir, inputPaths[0], inputs[0], options) case 'scale_pad': - return scalePad(dir, inputPaths[0], options) + return scalePad(ctx, dir, inputPaths[0], options) case 'overlay_image': - return overlayImage(dir, inputPaths, options) + return overlayImage(ctx, dir, inputPaths, options) case 'add_text': - return addText(dir, inputPaths[0], options) + return addText(ctx, dir, inputPaths[0], options) case 'fade': - return fade(dir, inputPaths[0], inputs[0], options) + return fade(ctx, dir, inputPaths[0], inputs[0], options) case 'extract_audio': - return extractAudio(dir, inputPaths[0], options) + return extractAudio(ctx, dir, inputPaths[0], options) case 'convert': - return convert(dir, inputPaths[0], options) + return convert(ctx, dir, inputPaths[0], options) case 'thumbnail': - return thumbnail(dir, inputPaths[0], options) + return thumbnail(ctx, dir, inputPaths[0], options) default: throw new Error(`Unsupported ffmpeg operation: ${operation}`) } }) } +/** + * Size the output before buffering it. The input budget does not bound this: + * `concat` re-encodes at CRF 18 and `convert` can target a lossless format, so + * a bounded input routinely produces a much larger output. + */ async function readOut(outputPath: string, ext: string): Promise { + const { size } = await fs.stat(outputPath) + if (size > MAX_MEDIA_BYTES) { + throw new Error( + `FFmpeg produced ${size} bytes, above the ${MAX_MEDIA_BYTES} byte media limit. Use a shorter input or a smaller target size.` + ) + } const buffer = await fs.readFile(outputPath) return { buffer, ext, contentType: mimeFromExt(ext) } } async function overlayAudio( + ctx: FfmpegRunContext, dir: string, inputPaths: string[], options: FfmpegOptions @@ -305,11 +491,12 @@ async function overlayAudio( 'aac', '-shortest', ]) - await runCommand(command, outputPath) + await runCommand(ctx, command, outputPath) return readOut(outputPath, 'mp4') } async function mixAudio( + ctx: FfmpegRunContext, dir: string, inputPaths: string[], options: FfmpegOptions @@ -327,13 +514,17 @@ async function mixAudio( `[v][m]amix=inputs=2:duration=longest:dropout_transition=0[a]`, ]) .outputOptions(['-map', '[a]']) - await runCommand(command, outputPath) + await runCommand(ctx, command, outputPath) return readOut(outputPath, 'mp3') } -async function concat(dir: string, inputPaths: string[]): Promise { +async function concat( + ctx: FfmpegRunContext, + dir: string, + inputPaths: string[] +): Promise { if (inputPaths.length < 2) throw new Error('concat requires at least 2 clips') - const probes = await Promise.all(inputPaths.map(probeFile)) + const probes = await Promise.all(inputPaths.map((p) => probeFile(ctx, p))) probes.forEach((p, i) => { if (!p.hasVideo) { throw new Error( @@ -341,8 +532,11 @@ async function concat(dir: string, inputPaths: string[]): Promise ) } }) - const width = probes[0].width || 1280 - const height = probes[0].height || 720 + // Clamped, not rejected: these describe the caller's own file rather than a + // value they asserted, but a container is free to declare a frame size far + // larger than anything worth normalizing to. + const width = clampProbedDimension(probes[0].width || 1280) + const height = clampProbedDimension(probes[0].height || 720) const fps = 30 // Normalize every clip to identical codec/size/fps/pixfmt, and SYNTHESIZE silent @@ -392,7 +586,7 @@ async function concat(dir: string, inputPaths: string[]): Promise '2', ...extra, ]) - await runCommand(cmd, out) + await runCommand(ctx, cmd, out) normalized.push(out) } @@ -407,11 +601,12 @@ async function concat(dir: string, inputPaths: string[]): Promise .input(listPath) .inputOptions(['-f', 'concat', '-safe', '0']) .outputOptions(['-c', 'copy', '-movflags', '+faststart']) - await runCommand(concatCmd, outputPath) + await runCommand(ctx, concatCmd, outputPath) return readOut(outputPath, 'mp4') } async function trim( + ctx: FfmpegRunContext, dir: string, inputPath: string, input: MediaFile, @@ -424,35 +619,72 @@ async function trim( if (options.end !== undefined) { command.setDuration(Math.max(0, options.end - start)) } - await runCommand(command, outputPath) + await runCommand(ctx, command, outputPath) return readOut(outputPath, ext) } +function clampProbedDimension(value: number): number { + return Math.min(Math.max(Math.round(value), MIN_SCALE_DIMENSION), MAX_SCALE_DIMENSION) +} + +function resolveScaleDimension(value: number, label: string): number { + if (!Number.isFinite(value)) { + throw new Error(`scale_pad ${label} must be a finite number`) + } + const rounded = Math.round(value) + if (rounded < MIN_SCALE_DIMENSION || rounded > MAX_SCALE_DIMENSION) { + throw new Error( + `scale_pad ${label} must be between ${MIN_SCALE_DIMENSION} and ${MAX_SCALE_DIMENSION} pixels (received ${rounded})` + ) + } + return rounded +} + +/** + * Bound the scale targets before they reach the filter graph. libavfilter sizes + * its per-frame buffers from these numbers, so an unbounded pair — `scale=30000:30000` + * is ~2.7 GB a frame — is a multi-gigabyte allocation in a child process that + * shares the instance's memory, for every frame of the input. + */ async function scalePad( + ctx: FfmpegRunContext, dir: string, inputPath: string, options: FfmpegOptions ): Promise { - let width = options.width - let height = options.height - if ((!width || !height) && options.aspectRatio && ASPECT_TARGETS[options.aspectRatio]) { - width = ASPECT_TARGETS[options.aspectRatio].w - height = ASPECT_TARGETS[options.aspectRatio].h + let requestedWidth = options.width + let requestedHeight = options.height + if ( + (!requestedWidth || !requestedHeight) && + options.aspectRatio && + ASPECT_TARGETS[options.aspectRatio] + ) { + requestedWidth = ASPECT_TARGETS[options.aspectRatio].w + requestedHeight = ASPECT_TARGETS[options.aspectRatio].h } - if (!width || !height) { + if (!requestedWidth || !requestedHeight) { throw new Error('scale_pad requires width+height or a known aspectRatio (e.g. 9:16)') } + const width = resolveScaleDimension(requestedWidth, 'width') + const height = resolveScaleDimension(requestedHeight, 'height') + if (width * height > MAX_SCALE_PIXELS) { + throw new Error( + `scale_pad ${width}x${height} is ${width * height} pixels, above the ${MAX_SCALE_PIXELS} pixel limit (4K). Choose a smaller frame` + ) + } + const outputPath = path.join(dir, 'out.mp4') const command = ffmpeg(inputPath) .videoFilters( `scale=${width}:${height}:force_original_aspect_ratio=decrease,pad=${width}:${height}:(ow-iw)/2:(oh-ih)/2,setsar=1` ) .outputOptions(['-c:a', 'copy']) - await runCommand(command, outputPath) + await runCommand(ctx, command, outputPath) return readOut(outputPath, 'mp4') } async function overlayImage( + ctx: FfmpegRunContext, dir: string, inputPaths: string[], options: FfmpegOptions @@ -465,11 +697,12 @@ async function overlayImage( .input(inputPaths[1]) .complexFilter([`[0:v][1:v]overlay=${xy}[v]`]) .outputOptions(['-map', '[v]', '-map', '0:a?', '-c:a', 'copy']) - await runCommand(command, outputPath) + await runCommand(ctx, command, outputPath) return readOut(outputPath, 'mp4') } async function addText( + ctx: FfmpegRunContext, dir: string, inputPath: string, options: FfmpegOptions @@ -508,17 +741,18 @@ async function addText( const command = ffmpeg(inputPath, { cwd: dir }) .videoFilters(`drawtext=${drawtext}`) .outputOptions(['-c:a', 'copy']) - await runCommand(command, outputPath) + await runCommand(ctx, command, outputPath) return readOut(outputPath, 'mp4') } async function fade( + ctx: FfmpegRunContext, dir: string, inputPath: string, input: MediaFile, _options: FfmpegOptions ): Promise { - const probe = await probeFile(inputPath) + const probe = await probeFile(ctx, inputPath) const duration = probe.durationSeconds || 0 const fadeDur = Math.min(0.5, duration / 4 || 0.5) const outStart = Math.max(0, duration - fadeDur) @@ -530,11 +764,12 @@ async function fade( command.videoFilters([`fade=t=in:st=0:d=${fadeDur}`, `fade=t=out:st=${outStart}:d=${fadeDur}`]) } command.audioFilters([`afade=t=in:st=0:d=${fadeDur}`, `afade=t=out:st=${outStart}:d=${fadeDur}`]) - await runCommand(command, outputPath) + await runCommand(ctx, command, outputPath) return readOut(outputPath, ext) } async function extractAudio( + ctx: FfmpegRunContext, dir: string, inputPath: string, options: FfmpegOptions @@ -542,11 +777,12 @@ async function extractAudio( const ext = (options.format || 'mp3').toLowerCase() const outputPath = path.join(dir, `out.${ext}`) const command = ffmpeg(inputPath).noVideo() - await runCommand(command, outputPath) + await runCommand(ctx, command, outputPath) return readOut(outputPath, ext) } async function convert( + ctx: FfmpegRunContext, dir: string, inputPath: string, options: FfmpegOptions @@ -554,11 +790,12 @@ async function convert( if (!options.format) throw new Error('convert requires a target format') const ext = options.format.toLowerCase() const outputPath = path.join(dir, `out.${ext}`) - await runCommand(ffmpeg(inputPath), outputPath) + await runCommand(ctx, ffmpeg(inputPath), outputPath) return readOut(outputPath, ext) } async function thumbnail( + ctx: FfmpegRunContext, dir: string, inputPath: string, options: FfmpegOptions @@ -567,7 +804,7 @@ async function thumbnail( const command = ffmpeg(inputPath) .seekInput(options.start ?? 0) .frames(1) - await runCommand(command, outputPath) + await runCommand(ctx, command, outputPath) return readOut(outputPath, 'jpg') } From 627509fbb4b741a820fd7ad26b3b54da3bc19c55 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Sat, 22 Aug 2026 15:32:54 -0700 Subject: [PATCH 2/3] fix(media): fit concat inside the shared area budget and stop input prep on cancel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings, both real. concat clamped each probed axis independently, so a 4096x4096 source produced a normalization target of exactly that — nearly double the area budget scale_pad enforces through the same scale=/pad= graph. The pair is now scaled down together, so aspect ratio survives and one ceiling governs both entry points. Dimensions come back even too, which yuv420p requires and per-axis rounding did not guarantee. Preparing the inputs is itself the expensive half of a many-file call — up to the whole byte budget in storage reads — and the abort signal was only consulted after the loop, so an explicit stop was observed only once every download had already finished. The loop now checks it per input. Also corrects the rationale comments on the mirrored limits. maxItems/minimum/ maximum do not reach the model: copilot's NormalizeToolParameters allowlists type/properties/items/description/enum/required, so the bounds travel only the contract path, where Sim's router validates against them. That is still worth having — it turns an out-of-range argument into a structured rejection before any storage read or child process — but the model learns the limits from the parameter descriptions, and the comments now say so instead of claiming the schema teaches it. --- .../copilot/tools/server/media/ffmpeg.test.ts | 40 ++++++++++++++++ .../lib/copilot/tools/server/media/ffmpeg.ts | 6 +++ apps/sim/lib/media/ffmpeg-limits.ts | 10 ++-- apps/sim/lib/media/ffmpeg.test.ts | 46 +++++++++++++++++++ apps/sim/lib/media/ffmpeg.ts | 31 +++++++++++-- 5 files changed, 126 insertions(+), 7 deletions(-) diff --git a/apps/sim/lib/copilot/tools/server/media/ffmpeg.test.ts b/apps/sim/lib/copilot/tools/server/media/ffmpeg.test.ts index 0dc94d40797..40f40d9771b 100644 --- a/apps/sim/lib/copilot/tools/server/media/ffmpeg.test.ts +++ b/apps/sim/lib/copilot/tools/server/media/ffmpeg.test.ts @@ -381,6 +381,46 @@ describe('ffmpeg server tool input admission', () => { expect(fetchWorkspaceFileBufferMock).not.toHaveBeenCalled() }) + it('stops preparing inputs the moment the caller cancels', async () => { + const controller = new AbortController() + controller.abort() + + const result = await ffmpegServerTool.execute( + { + operation: 'concat', + inputs: { files: [{ path: 'files/a.mp4' }, { path: 'files/b.mp4' }] }, + }, + { ...context, abortSignal: controller.signal } + ) + + expect(result.success).toBe(false) + // Not one storage read, let alone all of them. + expect(resolveWorkspaceFileReferenceMock).not.toHaveBeenCalled() + expect(fetchWorkspaceFileBufferMock).not.toHaveBeenCalled() + expect(runFfmpegOperationMock).not.toHaveBeenCalled() + }) + + it('stops between inputs when the cancel lands mid-preparation', async () => { + const controller = new AbortController() + // Abort once the first input has been read, so the second is never fetched. + fetchWorkspaceFileBufferMock.mockImplementationOnce(async () => { + controller.abort() + return Buffer.from('media') + }) + + const result = await ffmpegServerTool.execute( + { + operation: 'concat', + inputs: { files: [{ path: 'files/a.mp4' }, { path: 'files/b.mp4' }] }, + }, + { ...context, abortSignal: controller.signal } + ) + + expect(result.success).toBe(false) + expect(fetchWorkspaceFileBufferMock).toHaveBeenCalledTimes(1) + expect(runFfmpegOperationMock).not.toHaveBeenCalled() + }) + it('hands the caller cancellation signal to the transcode', async () => { const controller = new AbortController() diff --git a/apps/sim/lib/copilot/tools/server/media/ffmpeg.ts b/apps/sim/lib/copilot/tools/server/media/ffmpeg.ts index 3d966894619..effb8ed1da0 100644 --- a/apps/sim/lib/copilot/tools/server/media/ffmpeg.ts +++ b/apps/sim/lib/copilot/tools/server/media/ffmpeg.ts @@ -114,6 +114,12 @@ export const ffmpegServerTool: BaseServerTool = { let totalInputBytes = 0 const inputProvenances: WorkspaceFileSecretProvenance[] = [] for (const filePath of inputPaths) { + // Preparing the inputs is itself the expensive part of a many-file call — + // up to the whole byte budget in storage reads. Without this the run only + // notices an explicit stop once every download has already finished. + if (context.abortSignal?.aborted) { + throw new Error('ffmpeg cancelled while preparing inputs') + } const fileRecord = await resolveCopilotWorkspaceFileReference( context, fileOperations.readContent, diff --git a/apps/sim/lib/media/ffmpeg-limits.ts b/apps/sim/lib/media/ffmpeg-limits.ts index 6167efa9ee5..06fc100377d 100644 --- a/apps/sim/lib/media/ffmpeg-limits.ts +++ b/apps/sim/lib/media/ffmpeg-limits.ts @@ -2,9 +2,13 @@ * Execution bounds the ffmpeg tool enforces. * * These are mirrored into the Go tool catalog - * (`copilot/internal/tools/catalog/other/ffmpeg.go`) so the model reads the - * limits off its own schema instead of discovering them as a failed tool call. - * `ffmpeg-schema-parity.test.ts` fails when the two copies drift. + * (`copilot/internal/tools/catalog/other/ffmpeg.go`), which is what lets the + * router reject an out-of-range argument structurally, before any storage read + * or child process. The model itself learns the limits from the parameter + * descriptions — copilot's `NormalizeToolParameters` drops every JSON Schema + * keyword outside its allowlist on the way to a provider, so the numbers are + * stated in prose there too. `ffmpeg-schema-parity.test.ts` fails when the two + * copies drift. * * `maxScalePixels` has no JSON Schema equivalent, so it lives in the parameter * description on the Go side and is enforced here only. diff --git a/apps/sim/lib/media/ffmpeg.test.ts b/apps/sim/lib/media/ffmpeg.test.ts index 480f766bcfe..0626251634c 100644 --- a/apps/sim/lib/media/ffmpeg.test.ts +++ b/apps/sim/lib/media/ffmpeg.test.ts @@ -183,6 +183,52 @@ describe('runFfmpegOperation scale targets', () => { }) }) +describe('runFfmpegOperation concat normalization target', () => { + const twoVideoStreams = JSON.stringify({ + streams: [{ codec_type: 'video', codec_name: 'h264', width: 4096, height: 4096 }], + format: { duration: '2', format_name: 'mp4' }, + }) + + it('fits an oversized square source inside the same area budget scale_pad enforces', async () => { + probeReport.json = twoVideoStreams + + await runFfmpegOperation('concat', [videoInput, videoInput]) + + // First filter is the normalization pass for input 0. + const target = capturedVideoFilters[0].match(/scale=(\d+):(\d+):/) + expect(target).not.toBeNull() + const width = Number(target![1]) + const height = Number(target![2]) + expect(width * height).toBeLessThanOrEqual(4096 * 2304) + // Aspect ratio of the square source survives the fit. + expect(width).toBe(height) + }) + + it('emits even dimensions, which yuv420p requires', async () => { + probeReport.json = JSON.stringify({ + streams: [{ codec_type: 'video', codec_name: 'h264', width: 1919, height: 1081 }], + format: { duration: '2', format_name: 'mp4' }, + }) + + await runFfmpegOperation('concat', [videoInput, videoInput]) + + const target = capturedVideoFilters[0].match(/scale=(\d+):(\d+):/) + expect(Number(target![1]) % 2).toBe(0) + expect(Number(target![2]) % 2).toBe(0) + }) + + it('leaves an ordinary source untouched', async () => { + probeReport.json = JSON.stringify({ + streams: [{ codec_type: 'video', codec_name: 'h264', width: 1920, height: 1080 }], + format: { duration: '2', format_name: 'mp4' }, + }) + + await runFfmpegOperation('concat', [videoInput, videoInput]) + + expect(capturedVideoFilters[0]).toContain('scale=1920:1080') + }) +}) + describe('runFfmpegOperation process bounds', () => { it('kills a command that outlives the operation budget', async () => { vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }) diff --git a/apps/sim/lib/media/ffmpeg.ts b/apps/sim/lib/media/ffmpeg.ts index 93b062aa406..20d89326874 100644 --- a/apps/sim/lib/media/ffmpeg.ts +++ b/apps/sim/lib/media/ffmpeg.ts @@ -535,8 +535,7 @@ async function concat( // Clamped, not rejected: these describe the caller's own file rather than a // value they asserted, but a container is free to declare a frame size far // larger than anything worth normalizing to. - const width = clampProbedDimension(probes[0].width || 1280) - const height = clampProbedDimension(probes[0].height || 720) + const { width, height } = clampProbedFrame(probes[0].width || 1280, probes[0].height || 720) const fps = 30 // Normalize every clip to identical codec/size/fps/pixfmt, and SYNTHESIZE silent @@ -623,8 +622,32 @@ async function trim( return readOut(outputPath, ext) } -function clampProbedDimension(value: number): number { - return Math.min(Math.max(Math.round(value), MIN_SCALE_DIMENSION), MAX_SCALE_DIMENSION) +/** + * Fit a probed source frame inside the same budget `scale_pad` enforces. + * + * Clamping each axis on its own is not enough: two axes at the per-axis ceiling + * are 4096x4096, nearly double the area limit, and that target is baked into the + * same `scale=`/`pad=` graph. Scale the pair down together instead, so aspect + * ratio survives and one ceiling governs both entry points. + * + * Dimensions come back even because the normalization encodes yuv420p, which + * has no odd-sized frame. + */ +function clampProbedFrame(width: number, height: number): { width: number; height: number } { + let w = clampProbedAxis(width) + let h = clampProbedAxis(height) + const area = w * h + if (area > MAX_SCALE_PIXELS) { + const ratio = Math.sqrt(MAX_SCALE_PIXELS / area) + w = clampProbedAxis(w * ratio) + h = clampProbedAxis(h * ratio) + } + return { width: w, height: h } +} + +function clampProbedAxis(value: number): number { + const bounded = Math.min(Math.max(Math.round(value), MIN_SCALE_DIMENSION), MAX_SCALE_DIMENSION) + return bounded - (bounded % 2) } function resolveScaleDimension(value: number, label: string): number { From 0dee35bbb44570d61500b544e313504587e162d0 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Sat, 22 Aug 2026 15:44:59 -0700 Subject: [PATCH 3/3] fix(media): floor the scaled concat axes so the area bound actually holds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The scale factor lands both axes on a product of exactly the budget, so an axis allowed to round up can put the pair back over it — and when both round up and both land even, nothing pulls them back. A brute force over every dimension pair in 16..4096 finds 219,280 that violate the bound under round-to-even, worst 2694x3520 -> 2688x3512, 3072 pixels over. Under floor-to-even: none. Flooring keeps each axis at or below its exact target, so the product cannot exceed the budget. Probed dimensions are already integers, so this changes only the scaled path. --- apps/sim/lib/media/ffmpeg.test.ts | 15 +++++++++++++++ apps/sim/lib/media/ffmpeg.ts | 11 ++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/apps/sim/lib/media/ffmpeg.test.ts b/apps/sim/lib/media/ffmpeg.test.ts index 0626251634c..749a0b279d1 100644 --- a/apps/sim/lib/media/ffmpeg.test.ts +++ b/apps/sim/lib/media/ffmpeg.test.ts @@ -204,6 +204,21 @@ describe('runFfmpegOperation concat normalization target', () => { expect(width).toBe(height) }) + it('holds the area bound for dimensions that round up on both axes', async () => { + // 2694x3520 is the worst case in the whole dimension space: the scale factor + // lands both axes on .5, and rounding both up to an even number put the pair + // 3072 pixels back over the budget it had just been scaled into. + probeReport.json = JSON.stringify({ + streams: [{ codec_type: 'video', codec_name: 'h264', width: 2694, height: 3520 }], + format: { duration: '2', format_name: 'mp4' }, + }) + + await runFfmpegOperation('concat', [videoInput, videoInput]) + + const target = capturedVideoFilters[0].match(/scale=(\d+):(\d+):/) + expect(Number(target![1]) * Number(target![2])).toBeLessThanOrEqual(4096 * 2304) + }) + it('emits even dimensions, which yuv420p requires', async () => { probeReport.json = JSON.stringify({ streams: [{ codec_type: 'video', codec_name: 'h264', width: 1919, height: 1081 }], diff --git a/apps/sim/lib/media/ffmpeg.ts b/apps/sim/lib/media/ffmpeg.ts index 20d89326874..5eb0c57751d 100644 --- a/apps/sim/lib/media/ffmpeg.ts +++ b/apps/sim/lib/media/ffmpeg.ts @@ -645,8 +645,17 @@ function clampProbedFrame(width: number, height: number): { width: number; heigh return { width: w, height: h } } +/** + * Floors rather than rounds, which is what makes the area bound hold. + * + * The scale factor lands both axes on a product of exactly the budget, so any + * axis allowed to round *up* can put the pair back over it — and when both round + * up and both land even, nothing pulls them back. Flooring keeps each axis at or + * below its exact target, so the product cannot exceed the budget. Probed + * dimensions are already integers, so this only ever bites on the scaled path. + */ function clampProbedAxis(value: number): number { - const bounded = Math.min(Math.max(Math.round(value), MIN_SCALE_DIMENSION), MAX_SCALE_DIMENSION) + const bounded = Math.min(Math.max(Math.floor(value), MIN_SCALE_DIMENSION), MAX_SCALE_DIMENSION) return bounded - (bounded % 2) }