fix(skills): report video generation failures with a nonzero exit status (#7034)

AGENTS.md

@@ -219,6 +219,8 @@ Repo-wide conventions:
   Public skill script regressions live in `tests/skills/` and run in Skill Review CI
   independently of package review. Exercise dependency fallbacks with the optional
   import unavailable and mock HTTP transport to keep tests offline.
+  Video-generation CLI failures must exit non-zero and write diagnostics to stderr;
+  keep success output on stdout and test the CLI boundary with offline transport.
 - **Before pushing** — backend `make format`, frontend `pnpm check`.
   Backend CI enforces `ruff format --check`.
 - **Skill text encoding** — use explicit UTF-8 for textual skill resources.

README.md

@@ -1600,6 +1600,10 @@ The [music-generation skill](skills/public/music-generation/SKILL.md#output-hand
 
 Skills are loaded progressively — only when the task needs them, not all at once. This keeps the context window lean and makes DeerFlow work well even with token-sensitive models.
 
+The bundled video-generation command reports failures on stderr and exits with
+status 1, including when the selected provider has no API key. Callers can check
+the exit status before presenting an output file.
+
 When deferred skill discovery is enabled, `describe_skill` ranks installed skills by bounded, Unicode-normalized intent-term coverage across names and descriptions. Natural multi-term requests can therefore find a relevant skill without requiring one exact phrase, while exact `select:` and required-name `+prefix` lookups remain available. Ranked searches use up to 256 characters and return up to five results; exact `select:` lists are not truncated and return all requested catalog matches.
 
 A skill directory is a package boundary: once DeerFlow finds its `SKILL.md`, nested `SKILL.md` files under that package (for example evaluation fixtures) remain supporting data and are not registered as runtime skills. This applies to managed integration packs as well as public and custom skills. Namespace directories without their own `SKILL.md` can still group nested skills.

skills/public/video-generation/SKILL.md

@@ -124,6 +124,10 @@ python /mnt/skills/public/video-generation/scripts/generate.py \
 
 ## Output Handling
 
+The generation command exits with status 1 and writes errors to stderr when
+generation fails, including missing provider credentials. Only present the video
+after the command succeeds; a file left by an earlier run is not a new result.
+
 After generation:
 
 - Videos are typically saved in `/mnt/user-data/outputs/`

skills/public/video-generation/scripts/generate.py

@@ -1,5 +1,6 @@
 import base64
 import os
+import sys
 import time
 
 import requests
@@ -16,10 +17,7 @@ def _resolve_provider(override_env: str, existing_provider: str, has_existing_cr
         return existing_provider
     if os.getenv("MINIMAX_API_KEY"):
         return "minimax"
-    raise ValueError(
-        f"No credentials found. Set GEMINI_API_KEY for {existing_provider}, "
-        f"or MINIMAX_API_KEY for minimax (optionally force with {override_env})."
-    )
+    raise ValueError(f"No credentials found. Set GEMINI_API_KEY for {existing_provider}, or MINIMAX_API_KEY for minimax (optionally force with {override_env}).")
 
 
 def _minimax_host() -> str:
@@ -56,8 +54,7 @@ def _to_data_url(image_path: str) -> str:
     return f"data:{_guess_mime(image_path)};base64,{b64}"
 
 
-def _poll_video_task(host: str, auth: str, task_id: str,
-                     max_attempts: int = 120, interval: int = 3) -> str:
+def _poll_video_task(host: str, auth: str, task_id: str, max_attempts: int = 120, interval: int = 3) -> str:
     for _ in range(max_attempts):
         response = requests.get(
             f"{host}/v1/query/video_generation",
@@ -72,10 +69,7 @@ def _poll_video_task(host: str, auth: str, task_id: str,
             return payload["file_id"]
         if status == "Fail":
             base = payload.get("base_resp") or {}
-            raise Exception(
-                f"MiniMax video task {task_id} failed: "
-                f"{base.get('status_code')} {base.get('status_msg')}"
-            )
+            raise Exception(f"MiniMax video task {task_id} failed: {base.get('status_code')} {base.get('status_msg')}")
         # Surface query-level errors (bad task_id, auth) that arrive as a non-zero
         # base_resp without a terminal status, then keep polling.
         _check_base_resp(payload)
@@ -104,12 +98,10 @@ def _download(url: str, output_file: str) -> None:
         f.write(response.content)
 
 
-def _generate_video_minimax(
-    prompt: str, reference_images: list[str], output_file: str
-) -> str:
+def _generate_video_minimax(prompt: str, reference_images: list[str], output_file: str) -> str:
     minimax_api_key = os.getenv("MINIMAX_API_KEY")
     if not minimax_api_key:
-        return "MINIMAX_API_KEY is not set"
+        raise ValueError("MINIMAX_API_KEY is not set")
     host = _minimax_host()
     auth = f"Bearer {minimax_api_key}"
     body = {"model": os.getenv("MINIMAX_VIDEO_MODEL", "MiniMax-Hailuo-2.3"), "prompt": prompt}
@@ -142,9 +134,7 @@ def download(url: str, output_file: str) -> None:
         f.write(response.content)
 
 
-def _generate_video_gemini(
-    prompt: str, reference_images: list[str], output_file: str, aspect_ratio: str = "16:9"
-) -> str:
+def _generate_video_gemini(prompt: str, reference_images: list[str], output_file: str, aspect_ratio: str = "16:9") -> str:
     reference_payload = []
     request_json = {
         "instances": [{"prompt": prompt}],
@@ -153,15 +143,12 @@ def _generate_video_gemini(
     for reference_image in reference_images:
         with open(reference_image, "rb") as f:
             image_b64 = base64.b64encode(f.read()).decode("utf-8")
-        reference_payload.append(
-            {"image": {"mimeType": "image/jpeg", "bytesBase64Encoded": image_b64},
-             "referenceType": "asset"}
-        )
+        reference_payload.append({"image": {"mimeType": "image/jpeg", "bytesBase64Encoded": image_b64}, "referenceType": "asset"})
     if reference_payload:
         request_json["instances"][0]["referenceImages"] = reference_payload
     gemini_api_key = os.getenv("GEMINI_API_KEY")
     if not gemini_api_key:
-        return "GEMINI_API_KEY is not set"
+        raise ValueError("GEMINI_API_KEY is not set")
     response = requests.post(
         "https://generativelanguage.googleapis.com/v1beta/models/veo-3.1-generate-preview:predictLongRunning",
         headers={"x-goog-api-key": gemini_api_key, "Content-Type": "application/json"},
@@ -193,18 +180,14 @@ def generate_video(
     output_file: str,
     aspect_ratio: str = "16:9",
 ) -> str:
-    with open(prompt_file, "r", encoding="utf-8") as f:
+    with open(prompt_file, encoding="utf-8") as f:
         prompt = f.read()
-    provider = _resolve_provider(
-        "VIDEO_GENERATION_PROVIDER", "gemini", bool(os.getenv("GEMINI_API_KEY"))
-    )
+    provider = _resolve_provider("VIDEO_GENERATION_PROVIDER", "gemini", bool(os.getenv("GEMINI_API_KEY")))
     if provider == "minimax":
         # MiniMax video uses resolution/duration, not aspect_ratio; aspect_ratio ignored.
         return _generate_video_minimax(prompt, reference_images, output_file)
     if provider in ("gemini", "google"):
-        return _generate_video_gemini(
-            prompt, reference_images, output_file, aspect_ratio
-        )
+        return _generate_video_gemini(prompt, reference_images, output_file, aspect_ratio)
     raise ValueError(f"Unknown video provider: {provider!r} (use 'gemini' or 'minimax')")
 
 
@@ -213,15 +196,13 @@ if __name__ == "__main__":
 
     parser = argparse.ArgumentParser(description="Generate videos using Gemini or MiniMax API")
     parser.add_argument("--prompt-file", required=True, help="Absolute path to JSON prompt file")
-    parser.add_argument("--reference-images", nargs="*", default=[],
-                        help="Absolute paths to reference images (space-separated)")
+    parser.add_argument("--reference-images", nargs="*", default=[], help="Absolute paths to reference images (space-separated)")
     parser.add_argument("--output-file", required=True, help="Output path for generated video")
-    parser.add_argument("--aspect-ratio", required=False, default="16:9",
-                        help="Aspect ratio of the generated video (Gemini only)")
+    parser.add_argument("--aspect-ratio", required=False, default="16:9", help="Aspect ratio of the generated video (Gemini only)")
     args = parser.parse_args()
 
     try:
-        print(generate_video(args.prompt_file, args.reference_images,
-                             args.output_file, args.aspect_ratio))
+        print(generate_video(args.prompt_file, args.reference_images, args.output_file, args.aspect_ratio))
     except Exception as e:
-        print(f"Error while generating video: {e}")
+        print(f"Error while generating video: {e}", file=sys.stderr)
+        sys.exit(1)

tests/skills/test_video_generation_cli.py

@@ -0,0 +1,104 @@
+"""The video CLI must expose generation failures to its shell caller."""
+
+import json
+import runpy
+import subprocess
+import sys
+from pathlib import Path
+
+import pytest
+import requests
+
+SCRIPT = Path(__file__).resolve().parents[2] / "skills/public/video-generation/scripts/generate.py"
+
+
+@pytest.fixture(autouse=True)
+def clean_credentials(monkeypatch):
+    for key in ("GEMINI_API_KEY", "MINIMAX_API_KEY", "VIDEO_GENERATION_PROVIDER"):
+        monkeypatch.delenv(key, raising=False)
+
+
+@pytest.mark.parametrize(
+    ("case", "error"),
+    [
+        ("missing-prompt", "Error while generating video:"),
+        ("no-credentials", "No credentials found"),
+        ("gemini", "GEMINI_API_KEY is not set"),
+        ("google", "GEMINI_API_KEY is not set"),
+        ("minimax", "MINIMAX_API_KEY is not set"),
+        ("unknown", "Unknown video provider"),
+    ],
+)
+def test_cli_generation_failure_is_nonzero(monkeypatch, tmp_path, case, error):
+    prompt = tmp_path / "prompt.json"
+    if case != "missing-prompt":
+        prompt.write_text('{"prompt": "A cat runs"}', encoding="utf-8")
+    if case not in ("missing-prompt", "no-credentials"):
+        monkeypatch.setenv("VIDEO_GENERATION_PROVIDER", case)
+    output = tmp_path / "video.mp4"
+
+    result = subprocess.run(
+        [sys.executable, str(SCRIPT), "--prompt-file", str(prompt), "--output-file", str(output)],
+        capture_output=True,
+        text=True,
+        check=False,
+        timeout=10,
+    )
+
+    assert result.returncode == 1
+    assert error in result.stderr
+    assert result.stdout == ""
+    assert not output.exists()
+
+
+@pytest.mark.parametrize("provider", ["gemini", "minimax"])
+@pytest.mark.parametrize("http_error", [False, True], ids=["success", "http-error"])
+def test_cli_provider_result(monkeypatch, tmp_path, capsys, provider, http_error):
+    monkeypatch.setenv("VIDEO_GENERATION_PROVIDER", provider)
+    monkeypatch.setenv("GEMINI_API_KEY", "test-key")
+    monkeypatch.setenv("MINIMAX_API_KEY", "test-key")
+    prompt = tmp_path / "prompt.json"
+    prompt.write_text('{"prompt": "A cat runs"}', encoding="utf-8")
+    output = tmp_path / "video.mp4"
+    original = b"existing video"
+    output.write_bytes(original)
+    monkeypatch.setattr(sys, "argv", [str(SCRIPT), "--prompt-file", str(prompt), "--output-file", str(output)])
+
+    def response(payload=None, *, status=200, content=b""):
+        result = requests.Response()
+        result.status_code = status
+        result._content = json.dumps(payload).encode("utf-8") if payload is not None else content
+        return result
+
+    def post(*args, **kwargs):
+        if http_error:
+            return response(status=500)
+        return response({"task_id": "task-1"} if provider == "minimax" else {"name": "operations/test"})
+
+    def get(url, **kwargs):
+        if url.endswith("/query/video_generation"):
+            return response({"status": "Success", "file_id": "file-1"})
+        if url.endswith("/files/retrieve"):
+            return response({"file": {"download_url": "https://example.test/video.mp4"}})
+        if url.endswith("/operations/test"):
+            return response({"done": True, "response": {"generateVideoResponse": {"generatedSamples": [{"video": {"uri": "https://example.test/video.mp4"}}]}}})
+        assert url == "https://example.test/video.mp4"
+        return response(content=b"test video")
+
+    monkeypatch.setattr(requests, "post", post)
+    monkeypatch.setattr(requests, "get", get)
+
+    if http_error:
+        with pytest.raises(SystemExit) as exc:
+            runpy.run_path(str(SCRIPT), run_name="__main__")
+        assert exc.value.code == 1
+        assert output.read_bytes() == original
+        captured = capsys.readouterr()
+        assert "Error while generating video:" in captured.err
+        assert captured.out == ""
+    else:
+        runpy.run_path(str(SCRIPT), run_name="__main__")
+        assert output.read_bytes() == b"test video"
+        captured = capsys.readouterr()
+        assert "generated successfully" in captured.out
+        assert captured.err == ""