fix(media): fit concat inside the shared area budget and stop input prep on cancel

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.

Author: icecrasher321

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()
 

apps/sim/lib/copilot/tools/server/media/ffmpeg.ts

@@ -114,6 +114,12 @@ export const ffmpegServerTool: BaseServerTool<FfmpegArgs, FfmpegResult> = {
       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,

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.

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'] })

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 {