Merge main into Sora provider branch

tests/tools/test_audio_mixer_ducking.py

@@ -0,0 +1,89 @@
+"""Regression tests for audio_mixer full_mix ducking filtergraph.
+
+The ducking branch built an `acopy[speech_dup]` filter whose output pad was
+never consumed, leaving the FFmpeg filtergraph with a dangling output. FFmpeg
+rejects that, so `full_mix` with the most common shape — a single narration
+track plus one music bed, with ducking enabled (the default) — always failed.
+"""
+
+import shutil
+import subprocess
+import sys
+from pathlib import Path
+
+import pytest
+
+PROJECT_ROOT = Path(__file__).resolve().parent.parent.parent
+sys.path.insert(0, str(PROJECT_ROOT))
+
+from tools.audio.audio_mixer import AudioMixer  # noqa: E402
+
+pytestmark = pytest.mark.skipif(
+    shutil.which("ffmpeg") is None, reason="ffmpeg required for full_mix"
+)
+
+
+def _sine(path: Path, freq: int, dur: int) -> None:
+    subprocess.run(
+        ["ffmpeg", "-y", "-f", "lavfi", "-i", f"sine=frequency={freq}:duration={dur}", str(path)],
+        capture_output=True,
+        check=True,
+        timeout=30,
+    )
+
+
+def _has_audio(path: Path) -> bool:
+    out = subprocess.run(
+        ["ffprobe", "-v", "error", "-select_streams", "a",
+         "-show_entries", "stream=codec_type", "-of", "csv=p=0", str(path)],
+        capture_output=True, text=True, timeout=30,
+    )
+    return "audio" in out.stdout
+
+
+def test_full_mix_single_narration_plus_music_with_ducking(tmp_path):
+    speech = tmp_path / "speech.wav"
+    music = tmp_path / "music.wav"
+    _sine(speech, 440, 2)
+    _sine(music, 220, 3)
+    out = tmp_path / "mixed.wav"
+
+    result = AudioMixer().execute(
+        {
+            "operation": "full_mix",
+            "tracks": [
+                {"path": str(speech), "role": "speech"},
+                {"path": str(music), "role": "music"},
+            ],
+            "ducking": {"enabled": True},
+            "output_path": str(out),
+        }
+    )
+
+    assert result.success is True, result.error
+    assert out.exists() and _has_audio(out)
+
+
+def test_full_mix_multi_narration_plus_music_with_ducking(tmp_path):
+    s1, s2 = tmp_path / "s1.wav", tmp_path / "s2.wav"
+    music = tmp_path / "music.wav"
+    _sine(s1, 440, 2)
+    _sine(s2, 330, 2)
+    _sine(music, 220, 3)
+    out = tmp_path / "mixed_multi.wav"
+
+    result = AudioMixer().execute(
+        {
+            "operation": "full_mix",
+            "tracks": [
+                {"path": str(s1), "role": "speech"},
+                {"path": str(s2), "role": "speech"},
+                {"path": str(music), "role": "music"},
+            ],
+            "ducking": {"enabled": True},
+            "output_path": str(out),
+        }
+    )
+
+    assert result.success is True, result.error
+    assert out.exists() and _has_audio(out)

tests/tools/test_delivery_promise.py

@@ -0,0 +1,26 @@
+from lib.delivery_promise import PromiseType, classify_from_brief
+
+
+def test_classify_from_brief_source_led_reclassification_clears_motion_requirement() -> None:
+    promise = classify_from_brief("talking-head", {"has_footage": True})
+
+    assert promise.promise_type == PromiseType.SOURCE_LED
+    assert promise.source_required is True
+    assert promise.motion_required is False
+
+
+def test_classify_from_brief_explicit_motion_override_survives_reclassification() -> None:
+    promise = classify_from_brief(
+        "talking-head",
+        {"has_footage": True, "motion_required": True},
+    )
+
+    assert promise.promise_type == PromiseType.SOURCE_LED
+    assert promise.motion_required is True
+
+
+def test_classify_from_brief_avatar_defaults_stay_motion_required_without_footage() -> None:
+    promise = classify_from_brief("talking-head", {})
+
+    assert promise.promise_type == PromiseType.AVATAR_PRESENTER
+    assert promise.motion_required is True

tests/tools/test_documentary_governance.py

@@ -4,7 +4,9 @@ from pathlib import Path
 
 from tools.base_tool import ToolStatus
 from tools.tool_registry import ToolRegistry
+from tools.video.stock_sources import Candidate
 from tools.video.corpus_builder import CorpusBuilder
+from tools.video.direct_clip_search import DirectClipSearch
 from tools.video.video_compose import VideoCompose
 
 
@@ -196,3 +198,196 @@ def test_provider_menu_preserves_tool_discovery_metadata(monkeypatch):
     assert entry["name"] == "corpus_builder"
     assert entry["source_provider_summary"]["configured"] == 1
     assert entry["source_provider_menu"][0]["name"] == "archive_org"
+
+
+def test_direct_clip_search_honors_overall_timeout(monkeypatch, tmp_path):
+    """F-13 regression: direct clip search must stop on its own deadline and
+    return partial progress instead of relying on an external PTY interrupt."""
+    import tools.video.direct_clip_search as direct_clip_search
+    import tools.video.stock_sources as stock_sources
+
+    class SlowSource(_DummySource):
+        def search(self, query: str, filters):
+            return [
+                Candidate(
+                    source=self.name,
+                    source_id="slow-1",
+                    source_url="https://example.test/slow-1",
+                    download_url="https://example.test/slow-1.mp4",
+                    kind="video",
+                )
+            ]
+
+        def download(self, candidate, out_path: Path):
+            out_path.parent.mkdir(parents=True, exist_ok=True)
+            out_path.write_bytes(b"0" * 2048)
+            return out_path
+
+    source = SlowSource("slow_source", True)
+    monkeypatch.setattr(stock_sources, "all_sources", lambda: [source])
+    monkeypatch.setattr(stock_sources, "available_sources", lambda: [source])
+    monkeypatch.setattr(
+        stock_sources,
+        "source_summary",
+        lambda: {
+            "configured": 1,
+            "total": 1,
+            "available_source_names": ["slow_source"],
+            "unavailable_source_names": [],
+        },
+    )
+
+    ticks = iter([0.0, 2.0, 2.0, 2.0])
+    monkeypatch.setattr(direct_clip_search.time, "time", lambda: next(ticks, 2.0))
+
+    result = DirectClipSearch().execute(
+        {
+            "output_dir": str(tmp_path / "clips"),
+            "queries": [{"query": "foggy harbor", "slot_id": "sc5"}],
+            "timeout_seconds": 1,
+            "extract_thumbnails": False,
+        }
+    )
+
+    assert not result.success
+    assert "timed out" in (result.error or "").lower()
+    assert result.data["timed_out"] is True
+    assert result.data["phase"] in {"query", "search", "download"}
+    assert result.data["clips"] == []
+
+
+def test_direct_clip_search_times_out_streaming_download(monkeypatch, tmp_path):
+    """F-13 regression: a streaming adapter download must not run past the
+    tool-level deadline just because bytes keep arriving."""
+    import tools.video.direct_clip_search as direct_clip_search
+    import tools.video.stock_sources as stock_sources
+    import requests
+
+    clock = {"now": 0.0}
+
+    class StreamingResponse:
+        def __enter__(self):
+            return self
+
+        def __exit__(self, exc_type, exc, tb):
+            return False
+
+        def raise_for_status(self):
+            return None
+
+        def iter_content(self, chunk_size=1024):
+            clock["now"] = 2.0
+            yield b"0" * 2048
+
+    class StreamingSource(_DummySource):
+        def search(self, query: str, filters):
+            return [
+                Candidate(
+                    source=self.name,
+                    source_id="stream-1",
+                    source_url="https://example.test/stream-1",
+                    download_url="https://example.test/stream-1.mp4",
+                    kind="video",
+                )
+            ]
+
+        def download(self, candidate, out_path: Path):
+            out_path.parent.mkdir(parents=True, exist_ok=True)
+            with requests.get(candidate.download_url, stream=True, timeout=300) as response:
+                response.raise_for_status()
+                with out_path.open("wb") as f:
+                    for chunk in response.iter_content(chunk_size=1024):
+                        if chunk:
+                            f.write(chunk)
+            return out_path
+
+    source = StreamingSource("streaming_source", True)
+    monkeypatch.setattr(stock_sources, "all_sources", lambda: [source])
+    monkeypatch.setattr(stock_sources, "available_sources", lambda: [source])
+    monkeypatch.setattr(
+        stock_sources,
+        "source_summary",
+        lambda: {
+            "configured": 1,
+            "total": 1,
+            "available_source_names": ["streaming_source"],
+            "unavailable_source_names": [],
+        },
+    )
+    monkeypatch.setattr(direct_clip_search.time, "time", lambda: clock["now"])
+    monkeypatch.setattr(requests, "get", lambda *args, **kwargs: StreamingResponse())
+
+    result = DirectClipSearch().execute(
+        {
+            "output_dir": str(tmp_path / "clips"),
+            "queries": [{"query": "foggy harbor", "slot_id": "sc5"}],
+            "timeout_seconds": 1,
+            "extract_thumbnails": False,
+        }
+    )
+
+    assert not result.success
+    assert result.data["timed_out"] is True
+    assert result.data["phase"] == "download"
+    assert result.data["clips"] == []
+
+
+def test_direct_clip_search_reports_downloaded_clip_when_thumbnail_times_out(
+    monkeypatch, tmp_path
+):
+    """F-13 regression: timeout data should include a clip that was already
+    downloaded and validated before thumbnail extraction hit the deadline."""
+    import tools.video.direct_clip_search as direct_clip_search
+    import tools.video.stock_sources as stock_sources
+
+    clock = {"now": 0.0}
+
+    class SlowThumbnailSource(_DummySource):
+        def search(self, query: str, filters):
+            return [
+                Candidate(
+                    source=self.name,
+                    source_id="thumb-1",
+                    source_url="https://example.test/thumb-1",
+                    download_url="https://example.test/thumb-1.mp4",
+                    kind="video",
+                )
+            ]
+
+        def download(self, candidate, out_path: Path):
+            out_path.parent.mkdir(parents=True, exist_ok=True)
+            out_path.write_bytes(b"0" * 2048)
+            clock["now"] = 2.0
+            return out_path
+
+    source = SlowThumbnailSource("thumb_source", True)
+    monkeypatch.setattr(stock_sources, "all_sources", lambda: [source])
+    monkeypatch.setattr(stock_sources, "available_sources", lambda: [source])
+    monkeypatch.setattr(
+        stock_sources,
+        "source_summary",
+        lambda: {
+            "configured": 1,
+            "total": 1,
+            "available_source_names": ["thumb_source"],
+            "unavailable_source_names": [],
+        },
+    )
+    monkeypatch.setattr(direct_clip_search.time, "time", lambda: clock["now"])
+
+    result = DirectClipSearch().execute(
+        {
+            "output_dir": str(tmp_path / "clips"),
+            "queries": [{"query": "foggy harbor", "slot_id": "sc5"}],
+            "timeout_seconds": 1,
+            "extract_thumbnails": True,
+        }
+    )
+
+    assert not result.success
+    assert result.data["timed_out"] is True
+    assert result.data["phase"] == "thumbnail"
+    assert result.data["clips_downloaded"] == 1
+    assert result.data["total_clips"] == 1
+    assert result.data["clips"][0]["clip_id"] == "thumb_source_thumb-1"
+    assert result.data["clips"][0]["thumbnail"] == ""

tests/tools/test_export_bundle.py

@@ -0,0 +1,155 @@
+"""Tests for the export_bundle publisher tool.
+
+Covers the tool contract, registry discovery, the export bundle layout, a
+schema-valid publish_log, chapter formatting, and the missing-video error path.
+"""
+
+import json
+import sys
+from pathlib import Path
+
+import pytest
+
+PROJECT_ROOT = Path(__file__).resolve().parent.parent.parent
+sys.path.insert(0, str(PROJECT_ROOT))
+
+from tools.publishers.export_bundle import ExportBundle
+from tools.base_tool import ToolStatus, ToolTier
+from tools.tool_registry import ToolRegistry
+from schemas.artifacts import validate_artifact
+
+
+def _make_video(path: Path) -> None:
+    path.parent.mkdir(parents=True, exist_ok=True)
+    path.write_bytes(b"\x00\x00\x00\x18ftypmp42fakevideo")
+
+
+def test_contract_metadata():
+    tool = ExportBundle()
+    info = tool.get_info()
+    assert info["name"] == "export_bundle"
+    assert info["capability"] == "publish"
+    assert info["tier"] == ToolTier.PUBLISH.value
+    assert info["provider"] == "local"
+    assert info["resource_profile"]["network_required"] is False
+    assert tool.get_status() == ToolStatus.AVAILABLE
+    assert tool.estimate_cost({}) == 0.0
+
+
+def test_missing_video_errors(tmp_path):
+    result = ExportBundle().execute(
+        {"video_path": str(tmp_path / "nope.mp4"), "title": "X"}
+    )
+    assert result.success is False
+    assert "not found" in (result.error or "")
+
+
+def test_export_bundle_layout_and_publish_log(tmp_path):
+    video = tmp_path / "projects" / "demo" / "renders" / "final.mp4"
+    _make_video(video)
+    subs = tmp_path / "subs.srt"
+    subs.write_text("1\n00:00:00,000 --> 00:00:01,000\nhi\n", encoding="utf-8")
+
+    result = ExportBundle().execute(
+        {
+            "video_path": str(video),
+            "title": "Vector Databases Explained in 60 Seconds",
+            "export_dir": str(tmp_path / "out"),
+            "description": "A quick explainer.",
+            "tags": ["vector db", "explainer"],
+            "hashtags": ["#ai", "#database"],
+            "chapters": [
+                {"start_seconds": 0, "title": "Intro"},
+                {"start_seconds": 75, "title": "How it works"},
+            ],
+            "subtitles_path": str(subs),
+            "thumbnail_concept": {"text_overlay": "100x FASTER"},
+            "platform": "youtube",
+            "visibility": "unlisted",
+            "timestamp": "2026-06-29T10:30:00+00:00",
+        }
+    )
+    assert result.success is True
+    root = Path(result.data["export_path"])
+
+    # Layout
+    assert (root / "video" / "output.mp4").is_file()
+    assert (root / "video" / "subtitles.srt").is_file()
+    assert (root / "metadata" / "metadata.json").is_file()
+    assert (root / "metadata" / "description.txt").is_file()
+    assert (root / "metadata" / "tags.txt").is_file()
+    assert (root / "metadata" / "chapters.txt").is_file()
+    assert (root / "thumbnails" / "concept.json").is_file()
+
+    # tags one-per-line
+    assert (root / "metadata" / "tags.txt").read_text().splitlines() == ["vector db", "explainer"]
+    # chapter formatting (75s -> 1:15)
+    assert "1:15 - How it works" in (root / "metadata" / "chapters.txt").read_text()
+
+    # publish_log is schema-valid and shaped right
+    plog = result.data["publish_log"]
+    validate_artifact("publish_log", plog)
+    entry = plog["entries"][0]
+    assert entry["status"] == "exported"
+    assert entry["platform"] == "youtube"
+    assert entry["visibility"] == "unlisted"
+    assert entry["export_path"] == str(root)
+    assert entry["metadata_used"]["title"].startswith("Vector Databases")
+
+
+def test_chapter_time_formatting_hours(tmp_path):
+    video = tmp_path / "p" / "renders" / "final.mp4"
+    _make_video(video)
+    result = ExportBundle().execute(
+        {
+            "video_path": str(video),
+            "title": "Long",
+            "export_dir": str(tmp_path / "out"),
+            "chapters": [{"time_seconds": 3725, "label": "Deep dive"}],  # 1:02:05
+        }
+    )
+    assert result.success is True
+    txt = (Path(result.data["export_path"]) / "metadata" / "chapters.txt").read_text()
+    assert "1:02:05 - Deep dive" in txt
+
+
+def test_infer_project_name(tmp_path):
+    video = tmp_path / "projects" / "my-cool-video" / "renders" / "final.mp4"
+    _make_video(video)
+    result = ExportBundle().execute(
+        {"video_path": str(video), "title": "T", "export_dir": str(tmp_path / "out")}
+    )
+    # export still works; project name inference exercised via no-export_dir path below
+    assert result.success is True
+
+
+def test_missing_optional_asset_errors(tmp_path):
+    video = tmp_path / "p" / "renders" / "final.mp4"
+    _make_video(video)
+    for key in ("subtitles_path", "thumbnail_path"):
+        result = ExportBundle().execute(
+            {
+                "video_path": str(video),
+                "title": "T",
+                "export_dir": str(tmp_path / "out"),
+                key: str(tmp_path / "does_not_exist.x"),
+            }
+        )
+        assert result.success is False, key
+        assert key in (result.error or "")
+
+
+def test_default_export_dir_inside_project_workspace(tmp_path):
+    # projects/<name>/renders/final.mp4 -> projects/<name>/exports (no export_dir given)
+    video = tmp_path / "projects" / "demo" / "renders" / "final.mp4"
+    _make_video(video)
+    result = ExportBundle().execute({"video_path": str(video), "title": "T"})
+    assert result.success is True
+    assert Path(result.data["export_path"]) == (tmp_path / "projects" / "demo" / "exports").resolve()
+
+
+def test_registry_discovers_export_bundle():
+    reg = ToolRegistry()
+    reg.discover()
+    assert reg.get("export_bundle") is not None
+    assert reg.get_by_capability("publish")[0].name == "export_bundle"

tests/tools/test_hyperframes_compose.py

@@ -870,6 +870,45 @@ def test_video_compose_blocks_hyperframes_when_runtime_unavailable(
     assert "blocker" in err or "not available" in err
 
 
+def test_video_compose_honors_hyperframes_runtime_before_atelier_mode(
+    tmp_path, monkeypatch
+):
+    """Regression for F-14: composition_mode='atelier' must not force the
+    Remotion atelier branch when render_runtime='hyperframes' is locked."""
+
+    monkeypatch.setattr(
+        VideoCompose, "_hyperframes_available", lambda self: False, raising=True
+    )
+
+    result = VideoCompose().execute(
+        {
+            "operation": "render",
+            "edit_decisions": {
+                "version": "1.0",
+                "cuts": [
+                    {
+                        "id": "c1",
+                        "source": "a1",
+                        "in_seconds": 0,
+                        "out_seconds": 3,
+                    }
+                ],
+                "render_runtime": "hyperframes",
+                "composition_mode": "atelier",
+                "renderer_family": "animation-first",
+            },
+            "asset_manifest": {"assets": [{"id": "a1", "path": "does-not-matter.png"}]},
+            "output_path": str(tmp_path / "out.mp4"),
+        }
+    )
+
+    assert not result.success
+    err = (result.error or "").lower()
+    assert "hyperframes" in err
+    assert "not available" in err or "blocker" in err
+    assert "remotion entry" not in err
+
+
 # ------------------------------------------------------------------
 # Scaffold / workspace generation (no CLI invocation)
 # ------------------------------------------------------------------

tests/tools/test_math_animate_safety.py

@@ -0,0 +1,190 @@
+"""Tests for math_animate scene_code safety scan (issue #219).
+
+math_animate executes caller-supplied Python via Manim. The static scan blocks
+the constructs an attack needs (system/network/subprocess/secret access) while
+leaving genuine math-animation scenes untouched, and can be bypassed only with
+an explicit allow_unsafe_code opt-out.
+"""
+
+import sys
+from pathlib import Path
+
+import pytest
+
+PROJECT_ROOT = Path(__file__).resolve().parent.parent.parent
+sys.path.insert(0, str(PROJECT_ROOT))
+
+from tools.graphics.math_animate import MathAnimate  # noqa: E402
+
+SAFE_SCENE = (
+    "from manim import *\n"
+    "import numpy as np\n"
+    "import math\n"
+    "class Demo(Scene):\n"
+    "    def construct(self):\n"
+    "        self.play(Create(Circle(radius=np.pi / math.tau)))\n"
+)
+
+
+def test_safe_scene_passes_scan():
+    assert MathAnimate._scan_scene_code(SAFE_SCENE) == []
+
+
+@pytest.mark.parametrize(
+    "snippet, needle",
+    [
+        ("import os\nos.environ", "import 'os'"),
+        ("import subprocess", "import 'subprocess'"),
+        ("import socket", "import 'socket'"),
+        ("from urllib.request import urlopen", "from 'urllib.request' import ..."),
+        ("import requests", "import 'requests'"),
+    ],
+)
+def test_blocks_dangerous_imports(snippet, needle):
+    code = f"from manim import *\n{snippet}\nclass S(Scene):\n    def construct(self):\n        pass\n"
+    violations = MathAnimate._scan_scene_code(code)
+    assert needle in violations
+
+
+@pytest.mark.parametrize("call", ["eval", "exec", "compile", "open", "__import__"])
+def test_blocks_dangerous_calls(call):
+    code = (
+        "from manim import *\n"
+        "class S(Scene):\n"
+        "    def construct(self):\n"
+        f"        {call}('x')\n"
+    )
+    assert f"use of '{call}'" in MathAnimate._scan_scene_code(code)
+
+
+def test_blocks_no_import_builtins_secret_read():
+    # Regression for the reported bypass: no dangerous import, secret read via
+    # __builtins__ indexing. The whole expression roots on the bare __builtins__
+    # name (the 'open' inside [] is a string literal), so blocking that name
+    # blocks the payload.
+    code = (
+        "from manim import *\n"
+        "class S(Scene):\n"
+        "    def construct(self):\n"
+        "        __builtins__['open']('.env').read()\n"
+    )
+    assert "use of '__builtins__'" in MathAnimate._scan_scene_code(code)
+
+
+def test_blocks_getattr_reflection_bypass():
+    # getattr-based attribute reflection is a classic denylist evasion; blocking
+    # the getattr name removes the primitive.
+    code = (
+        "from manim import *\n"
+        "class S(Scene):\n"
+        "    def construct(self):\n"
+        "        cls = getattr(object(), '__class__')\n"
+    )
+    assert "use of 'getattr'" in MathAnimate._scan_scene_code(code)
+
+
+def test_blocks_aliased_dangerous_builtin():
+    # Binding a blocked builtin to another name must still trip on the name use.
+    code = (
+        "from manim import *\n"
+        "class S(Scene):\n"
+        "    def construct(self):\n"
+        "        f = open\n"
+        "        f('.env')\n"
+    )
+    assert "use of 'open'" in MathAnimate._scan_scene_code(code)
+
+
+def test_blocks_sandbox_escape_dunders():
+    code = (
+        "from manim import *\n"
+        "class S(Scene):\n"
+        "    def construct(self):\n"
+        "        ().__class__.__bases__[0].__subclasses__()\n"
+    )
+    violations = MathAnimate._scan_scene_code(code)
+    assert "dunder attribute access '.__class__'" in violations
+    assert "dunder attribute access '.__bases__'" in violations
+    assert "dunder attribute access '.__subclasses__'" in violations
+
+
+def test_blocks_builtins_module_via_print_self():
+    # Regression for the reported no-import bypass: print.__self__ is the
+    # builtins module, reachable without an import, a bare open/__builtins__/
+    # getattr, or a blocked name. Blocking all reflection dunders closes it.
+    code = (
+        "from manim import *\n"
+        "class S(Scene):\n"
+        "    def construct(self):\n"
+        "        print.__self__.open('.env').read()\n"
+    )
+    assert "dunder attribute access '.__self__'" in MathAnimate._scan_scene_code(code)
+
+
+def test_super_init_is_allowed():
+    # A legitimate custom Mobject with super().__init__() must not be blocked —
+    # __init__ (and __name__) are the only permitted dunders.
+    code = (
+        "from manim import *\n"
+        "class Widget(VGroup):\n"
+        "    def __init__(self, **kwargs):\n"
+        "        super().__init__(**kwargs)\n"
+        "        self.add(Circle())\n"
+        "class S(Scene):\n"
+        "    def construct(self):\n"
+        "        self.add(Widget())\n"
+    )
+    assert MathAnimate._scan_scene_code(code) == []
+
+
+def test_syntax_error_defers_to_manim():
+    # A parse failure must not mask as a safety violation; Manim reports it.
+    assert MathAnimate._scan_scene_code("class S(Scene):\n  def construct(self)\n") == []
+
+
+def test_execute_blocks_dangerous_code_before_running_manim(monkeypatch):
+    # Pretend manim is installed so execute() reaches the safety gate rather
+    # than short-circuiting on a missing binary. The scan must reject before any
+    # subprocess runs.
+    monkeypatch.setattr("shutil.which", lambda _: "/usr/bin/manim")
+
+    def boom(*a, **k):  # subprocess must never be reached
+        raise AssertionError("subprocess.run should not be called for blocked code")
+
+    monkeypatch.setattr("subprocess.run", boom)
+
+    dangerous = (
+        "from manim import *\n"
+        "import os\n"
+        "class S(Scene):\n"
+        "    def construct(self):\n"
+        "        print(os.environ)\n"
+    )
+    result = MathAnimate().execute({"scene_code": dangerous})
+    assert result.success is False
+    assert "safety scan" in result.error
+    assert "allow_unsafe_code" in result.error
+
+
+def test_allow_unsafe_code_bypasses_scan(monkeypatch):
+    # With the opt-out, execution proceeds past the scan to Manim (which we stub
+    # to fail); the failure must NOT be the safety-scan message.
+    monkeypatch.setattr("shutil.which", lambda _: "/usr/bin/manim")
+
+    class FakeProc:
+        returncode = 1
+        stderr = "manim ran"
+        stdout = ""
+
+    monkeypatch.setattr("subprocess.run", lambda *a, **k: FakeProc())
+
+    dangerous = (
+        "from manim import *\n"
+        "import os\n"
+        "class S(Scene):\n"
+        "    def construct(self):\n"
+        "        print(os.environ)\n"
+    )
+    result = MathAnimate().execute({"scene_code": dangerous, "allow_unsafe_code": True})
+    assert result.success is False
+    assert "safety scan" not in (result.error or "")

tests/tools/test_remotion_diagnostics.py

@@ -0,0 +1,128 @@
+"""Tests for Remotion render debuggability in video_compose (issue #217).
+
+Two creator-facing gaps:
+  1. A failed `npx remotion render` surfaced only "returned non-zero exit
+     status 1"; the useful Remotion diagnostics in stderr were dropped.
+  2. There was no pass-through for Remotion's `--timeout`, so a slow headless
+     browser setup failed opaquely with no way to raise the limit.
+"""
+
+import subprocess
+import sys
+from pathlib import Path
+
+import pytest
+
+PROJECT_ROOT = Path(__file__).resolve().parent.parent.parent
+sys.path.insert(0, str(PROJECT_ROOT))
+
+from tools.video.video_compose import VideoCompose  # noqa: E402
+
+
+@pytest.fixture
+def tool(monkeypatch):
+    monkeypatch.setattr("shutil.which", lambda _: "/usr/bin/npx")
+    return VideoCompose()
+
+
+def test_render_failure_surfaces_remotion_stderr_tail(tool, tmp_path, monkeypatch):
+    stderr = "some npm noise\nError: Delayed render timed out\nRemotion actual cause here"
+
+    def fake_run_command(cmd, *a, **k):
+        raise subprocess.CalledProcessError(returncode=1, cmd=cmd, output="", stderr=stderr)
+
+    monkeypatch.setattr(tool, "run_command", fake_run_command)
+    result = tool._remotion_render(
+        {"composition_data": {"cuts": []}, "output_path": str(tmp_path / "out.mp4")}
+    )
+
+    assert result.success is False
+    assert "exit 1" in result.error
+    assert "Remotion actual cause here" in result.error
+
+
+def test_timeout_expired_gives_actionable_message(tool, tmp_path, monkeypatch):
+    def fake_run_command(cmd, *a, **k):
+        raise subprocess.TimeoutExpired(cmd=cmd, timeout=600)
+
+    monkeypatch.setattr(tool, "run_command", fake_run_command)
+    result = tool._remotion_render(
+        {"composition_data": {"cuts": []}, "output_path": str(tmp_path / "out.mp4")}
+    )
+
+    assert result.success is False
+    assert "timed out" in result.error.lower()
+    assert "remotion_timeout_ms" in result.error
+
+
+def test_remotion_timeout_ms_is_passed_through(tool, tmp_path, monkeypatch):
+    seen = {}
+
+    def fake_run_command(cmd, *a, **k):
+        seen["cmd"] = cmd
+        seen["timeout"] = k.get("timeout")
+        return None  # output file intentionally absent
+
+    monkeypatch.setattr(tool, "run_command", fake_run_command)
+    tool._remotion_render(
+        {
+            "composition_data": {"cuts": []},
+            "output_path": str(tmp_path / "out.mp4"),
+            "remotion_timeout_ms": 120000,
+        }
+    )
+
+    assert "--timeout=120000" in seen["cmd"]
+    # subprocess timeout widened past the 120s render budget so run_command
+    # does not kill Remotion before its own timeout fires.
+    assert seen["timeout"] >= 180
+
+
+def test_high_level_render_forwards_timeout_to_remotion(tool, tmp_path, monkeypatch):
+    # The gap in the first cut: execute(operation="render") -> _render() builds a
+    # fresh remotion_inputs dict, so the option must be forwarded there, not only
+    # on a direct _remotion_render() call.
+    captured = {}
+    monkeypatch.setattr(tool, "_pre_compose_validation", lambda *a, **k: None)
+    monkeypatch.setattr(tool, "_needs_remotion", lambda *a, **k: True)
+
+    def fake_remotion_render(inputs):
+        captured.update(inputs)
+        from tools.base_tool import ToolResult
+
+        return ToolResult(success=True, data={}, artifacts=[])
+
+    monkeypatch.setattr(tool, "_remotion_render", fake_remotion_render)
+    monkeypatch.setattr(tool, "_run_final_review", lambda *a, **k: {})
+
+    tool._render(
+        {
+            "edit_decisions": {
+                "render_runtime": "remotion",
+                "renderer_family": "explainer-data",
+                "cuts": [{"id": "c1", "source": "a1", "in_seconds": 0, "out_seconds": 2}],
+            },
+            "asset_manifest": {"assets": [{"id": "a1", "path": "/tmp/a1.mp4"}]},
+            "output_path": str(tmp_path / "out.mp4"),
+            "remotion_timeout_ms": 120000,
+        }
+    )
+
+    assert captured.get("remotion_timeout_ms") == 120000
+
+
+def test_no_timeout_flag_when_not_requested(tool, tmp_path, monkeypatch):
+    seen = {}
+
+    def fake_run_command(cmd, *a, **k):
+        seen["cmd"] = cmd
+        seen["timeout"] = k.get("timeout")
+        return None
+
+    monkeypatch.setattr(tool, "run_command", fake_run_command)
+    tool._remotion_render(
+        {"composition_data": {"cuts": []}, "output_path": str(tmp_path / "out.mp4")}
+    )
+
+    assert not any(str(c).startswith("--timeout") for c in seen["cmd"])
+    assert seen["timeout"] == 600

tests/tools/test_scoring.py

@@ -0,0 +1,47 @@
+"""Regression tests for provider scoring tokenization."""
+
+from __future__ import annotations
+
+from lib.scoring import _tokenize_text, score_provider
+from tools.base_tool import ToolStatus
+
+
+class _FakeVideoTool:
+    name = "fake-video"
+
+    def get_info(self) -> dict[str, object]:
+        return {
+            "name": "fake-video",
+            "provider": "fake",
+            "best_for": ["cinematic video"],
+            "supports": {
+                "native_audio": True,
+                "multi_shot": True,
+                "camera_direction": True,
+                "lip_sync": True,
+                "cinematic_quality": True,
+            },
+            "stability": "production",
+            "runtime": "api",
+        }
+
+    def get_status(self) -> ToolStatus:
+        return ToolStatus.AVAILABLE
+
+    def estimate_cost(self, inputs: dict[str, object]) -> float:
+        return 0.0
+
+
+def test_tokenize_text_strips_trailing_punctuation() -> None:
+    assert _tokenize_text("cinematic.") == ["cinematic"]
+    assert _tokenize_text("v1.5.") == ["v1.5"]
+    assert _tokenize_text("gpt-4.1") == ["gpt-4.1"]
+
+
+def test_cinematic_bonus_ignores_adjacent_punctuation() -> None:
+    tool = _FakeVideoTool()
+    plain = score_provider(tool, {"asset_type": "video", "intent": "make it cinematic and fast"})
+    punctuated = score_provider(tool, {"asset_type": "video", "intent": "make it cinematic, and fast"})
+
+    assert punctuated.task_fit == plain.task_fit
+    assert punctuated.output_quality == plain.output_quality