fix(record_sim_video): reject incomplete recordings

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## [Unreleased]
+
+### Fixed
+
+- `record_sim_video` no longer reports success for an incomplete recording. A stop that needed a force-kill, or an MP4 with no `moov` box or a truncated box, now fails with the AXe output and the file left in place.
+
 ## [2.7.1]
 
 ### Changed

src/mcp/tools/simulator/__tests__/record_sim_video.test.ts

@@ -39,6 +39,7 @@ describe('record_sim_video tool - validation', () => {
 describe('record_sim_video logic - start behavior', () => {
   it('starts with default fps (30) and warns when outputFile is provided on start (ignored)', async () => {
     const video: any = {
+      checkMp4Finalized: async () => ({ finalized: true }),
       startSimulatorVideoCapture: async () => ({
         started: true,
         sessionId: 'sess-123',
@@ -86,6 +87,7 @@ describe('record_sim_video logic - start behavior', () => {
 
   it('keeps start failure summary short and preserves diagnostics', async () => {
     const video: any = {
+      checkMp4Finalized: async () => ({ finalized: true }),
       startSimulatorVideoCapture: async () => ({
         started: false,
         error: 'AXe stderr: permission denied',
@@ -124,6 +126,7 @@ describe('record_sim_video logic - start behavior', () => {
 describe('record_sim_video logic - end-to-end stop with rename', () => {
   it('stops, parses stdout path, and renames to outputFile', async () => {
     const video: any = {
+      checkMp4Finalized: async () => ({ finalized: true }),
       startSimulatorVideoCapture: async () => ({
         started: true,
         sessionId: 'sess-abc',
@@ -181,6 +184,7 @@ describe('record_sim_video logic - end-to-end stop with rename', () => {
 
   it('keeps missing parsed path summary short and preserves raw AXe output', async () => {
     const video: any = {
+      checkMp4Finalized: async () => ({ finalized: true }),
       startSimulatorVideoCapture: async () => ({
         started: true,
         sessionId: 'sess-abc',
@@ -226,6 +230,7 @@ describe('record_sim_video logic - version gate', () => {
     };
 
     const video: any = {
+      checkMp4Finalized: async () => ({ finalized: true }),
       startSimulatorVideoCapture: async () => ({
         started: true,
         sessionId: 'sess-xyz',
@@ -256,3 +261,67 @@ describe('record_sim_video logic - version gate', () => {
     expect(text).toContain('AXe v1.1.0');
   });
 });
+
+describe('record_sim_video logic - incomplete recordings', () => {
+  const axe = {
+    areAxeToolsAvailable: () => true,
+    isAxeAtLeastVersion: async () => true,
+  };
+
+  async function stopWith(stopResult: object, finalization: object) {
+    const copied: string[] = [];
+    const fs = createMockFileSystemExecutor({
+      cp: async (source: string) => {
+        copied.push(source);
+      },
+    });
+    const video: any = {
+      startSimulatorVideoCapture: async () => ({ started: true, sessionId: 's' }),
+      stopSimulatorVideoCapture: async () => ({ stopped: true, ...stopResult }),
+      checkMp4Finalized: async () => finalization,
+    };
+    const { result, run } = createMockToolHandlerContext();
+    await run(() =>
+      record_sim_videoLogic(
+        { simulatorId: VALID_SIM_ID, stop: true, outputFile: '/var/videos/final.mp4' } as any,
+        DUMMY_EXECUTOR,
+        axe,
+        video,
+        fs,
+      ),
+    );
+    return { result, copied };
+  }
+
+  it('fails without copying when the recording has no moov box', async () => {
+    const { result, copied } = await stopWith(
+      { parsedPath: '/tmp/recorded.mp4', stdout: 'Saved /tmp/recorded.mp4', exitCode: 0 },
+      { finalized: false, reason: 'no moov box found' },
+    );
+    expect(result.isError()).toBe(true);
+    expect(copied).toEqual([]);
+    const text = result.text();
+    expect(text).toContain('incomplete');
+    expect(text).toContain('no moov box found');
+    expect(text).toContain('Saved /tmp/recorded.mp4');
+  });
+
+  it('fails when AXe had to be force-killed even if the file looks finalized', async () => {
+    const { result, copied } = await stopWith(
+      { parsedPath: '/tmp/recorded.mp4', stdout: 'Saved /tmp/recorded.mp4', forcedKill: true },
+      { finalized: true },
+    );
+    expect(result.isError()).toBe(true);
+    expect(copied).toEqual([]);
+    expect(result.text()).toContain('force-killed');
+  });
+
+  it('copies a finalized recording', async () => {
+    const { result, copied } = await stopWith(
+      { parsedPath: '/tmp/recorded.mp4', stdout: 'Saved /tmp/recorded.mp4', exitCode: 0 },
+      { finalized: true },
+    );
+    expect(result.isError()).toBe(false);
+    expect(copied).toEqual(['/tmp/recorded.mp4']);
+  });
+});

src/mcp/tools/simulator/record_sim_video.ts

@@ -14,6 +14,7 @@ import {
   AXE_NOT_AVAILABLE_MESSAGE,
 } from '../../../utils/axe/index.ts';
 import {
+  checkMp4Finalized,
   startSimulatorVideoCapture,
   stopSimulatorVideoCapture,
 } from '../../../utils/video-capture/index.ts';
@@ -106,6 +107,7 @@ export function createRecordSimVideoExecutor(
   video: {
     startSimulatorVideoCapture: typeof startSimulatorVideoCapture;
     stopSimulatorVideoCapture: typeof stopSimulatorVideoCapture;
+    checkMp4Finalized: typeof checkMp4Finalized;
   },
   fs: FileSystemExecutor,
 ): NonStreamingExecutor<RecordSimVideoParams, RecordSimVideoResult> {
@@ -173,10 +175,36 @@ export function createRecordSimVideoExecutor(
       });
     }
 
+    const rawOutput = stopRes.stdout ?? '(no output captured)';
+    if (stopRes.parsedPath) {
+      const processOutcome = `exit code: ${stopRes.exitCode ?? 'none'}, signal: ${stopRes.signal ?? 'none'}`;
+      let incompleteReason: string | undefined;
+      if (stopRes.forcedKill) {
+        incompleteReason = 'AXe did not exit after the stop signal and was force-killed';
+      } else {
+        try {
+          const finalization = await video.checkMp4Finalized(stopRes.parsedPath);
+          if (!finalization.finalized) {
+            incompleteReason = finalization.reason;
+          }
+        } catch (error) {
+          incompleteReason = `could not inspect ${stopRes.parsedPath}: ${error instanceof Error ? error.message : String(error)}`;
+        }
+      }
+      if (incompleteReason) {
+        return createRecordSimVideoResult({
+          simulatorId: params.simulatorId,
+          didError: true,
+          error: 'Recording stopped but the video file is incomplete.',
+          diagnosticsMessage: `${incompleteReason} (${processOutcome}). Left in place: ${stopRes.parsedPath}. Raw output: ${rawOutput}`,
+        });
+      }
+    }
+
     try {
       if (params.outputFile) {
         if (!stopRes.parsedPath) {
-          const diagnosticMessage = `Recording stopped but could not determine the recorded file path from AXe output. Raw output: ${stopRes.stdout ?? '(no output captured)'}`;
+          const diagnosticMessage = `Recording stopped but could not determine the recorded file path from AXe output. Raw output: ${rawOutput}`;
           return createRecordSimVideoResult({
             simulatorId: params.simulatorId,
             didError: true,
@@ -228,9 +256,11 @@ export async function record_sim_videoLogic(
   video: {
     startSimulatorVideoCapture: typeof startSimulatorVideoCapture;
     stopSimulatorVideoCapture: typeof stopSimulatorVideoCapture;
+    checkMp4Finalized: typeof checkMp4Finalized;
   } = {
     startSimulatorVideoCapture,
     stopSimulatorVideoCapture,
+    checkMp4Finalized,
   },
   fs: FileSystemExecutor = getDefaultFileSystemExecutor(),
 ): Promise<void> {

src/utils/__tests__/video_capture.test.ts

@@ -0,0 +1,112 @@
+import { describe, expect, it } from 'vitest';
+import { mkdtempSync, writeFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+import { spawn } from 'node:child_process';
+import {
+  checkMp4Finalized,
+  startSimulatorVideoCapture,
+  stopSimulatorVideoCapture,
+} from '../video_capture.ts';
+
+function box(type: string, payload: Buffer): Buffer {
+  const header = Buffer.alloc(8);
+  header.writeUInt32BE(8 + payload.length, 0);
+  header.write(type, 4, 'latin1');
+  return Buffer.concat([header, payload]);
+}
+
+function writeMp4(parts: Buffer[]): string {
+  const path = join(mkdtempSync(join(tmpdir(), 'mp4-')), 'a.mp4');
+  writeFileSync(path, Buffer.concat(parts));
+  return path;
+}
+
+describe('checkMp4Finalized', () => {
+  const ftyp = box('ftyp', Buffer.from('isom0000', 'latin1'));
+  const mdat = box('mdat', Buffer.alloc(1000, 1));
+  const moov = box('moov', Buffer.alloc(100, 2));
+
+  it('accepts moov after mdat and moov before mdat', async () => {
+    expect(await checkMp4Finalized(writeMp4([ftyp, mdat, moov]))).toEqual({ finalized: true });
+    expect(await checkMp4Finalized(writeMp4([ftyp, moov, mdat]))).toEqual({ finalized: true });
+  });
+
+  it('rejects a recording with no moov box', async () => {
+    const result = await checkMp4Finalized(writeMp4([ftyp, mdat]));
+    expect(result).toEqual({ finalized: false, reason: 'no moov box found' });
+  });
+
+  it('rejects a truncated mdat and a truncated moov', async () => {
+    const cutMdat = await checkMp4Finalized(writeMp4([ftyp, mdat.subarray(0, 500)]));
+    expect(cutMdat.finalized).toBe(false);
+    const cutMoov = await checkMp4Finalized(writeMp4([ftyp, mdat, moov.subarray(0, 50)]));
+    expect(cutMoov.finalized).toBe(false);
+  });
+
+  it('rejects an extended-size box smaller than its own 16 byte header', async () => {
+    const extended = Buffer.alloc(16);
+    extended.writeUInt32BE(1, 0);
+    extended.write('moov', 4, 'latin1');
+    extended.writeBigUInt64BE(8n, 8);
+    const result = await checkMp4Finalized(writeMp4([ftyp, extended]));
+    expect(result.finalized).toBe(false);
+  });
+
+  it('accepts a valid extended-size moov box', async () => {
+    const extended = Buffer.alloc(16);
+    extended.writeUInt32BE(1, 0);
+    extended.write('moov', 4, 'latin1');
+    extended.writeBigUInt64BE(16n, 8);
+    expect(await checkMp4Finalized(writeMp4([ftyp, mdat, extended]))).toEqual({ finalized: true });
+  });
+
+  it('rejects an empty file', async () => {
+    expect((await checkMp4Finalized(writeMp4([]))).finalized).toBe(false);
+  });
+});
+
+describe('stopSimulatorVideoCapture process outcome', () => {
+  const helpers = { getAxePath: () => '/axe', getBundledAxeEnvironment: () => ({}) };
+  // resolves once the child has printed, which its scripts do after installing the SIGINT handler
+  const executorFor = (script: string, ready: { promise?: Promise<void> }) =>
+    (async () => {
+      const child = spawn(process.execPath, ['-e', script]);
+      ready.promise = new Promise<void>((resolve) => {
+        child.stdout.once('data', () => resolve());
+      });
+      return { success: true, output: '', process: child };
+    }) as never;
+
+  it('reports a child that ignores the stop signal as force-killed with its output', async () => {
+    const script =
+      "process.on('SIGINT',()=>{});console.log('Saved /tmp/t.mp4');setInterval(()=>{},1000)";
+    const ready: { promise?: Promise<void> } = {};
+    await startSimulatorVideoCapture(
+      { simulatorUuid: 'sim-kill' },
+      executorFor(script, ready),
+      helpers,
+    );
+    await ready.promise;
+    const result = await stopSimulatorVideoCapture({ simulatorUuid: 'sim-kill' }, null as never);
+    expect(result).toMatchObject({
+      stopped: true,
+      forcedKill: true,
+      parsedPath: '/tmp/t.mp4',
+    });
+  }, 15000);
+
+  it('reports a child that exits on the stop signal as not force-killed', async () => {
+    const script =
+      "process.on('SIGINT',()=>process.exit(0));console.log('Saved /tmp/t.mp4');setInterval(()=>{},1000)";
+    const ready: { promise?: Promise<void> } = {};
+    await startSimulatorVideoCapture(
+      { simulatorUuid: 'sim-ok' },
+      executorFor(script, ready),
+      helpers,
+    );
+    await ready.promise;
+    const result = await stopSimulatorVideoCapture({ simulatorUuid: 'sim-ok' }, null as never);
+    expect(result).toMatchObject({ stopped: true, forcedKill: false, exitCode: 0 });
+  }, 15000);
+});

src/utils/video-capture/index.ts

@@ -1,7 +1,9 @@
 export {
   startSimulatorVideoCapture,
   stopSimulatorVideoCapture,
+  checkMp4Finalized,
   type AxeHelpers,
+  type Mp4FinalizationResult,
   type StartVideoCaptureResult,
   type StopVideoCaptureResult,
 } from '../video_capture.ts';