From 46da5008cd6b101809bfa768d46349cdd11c25bd Mon Sep 17 00:00:00 2001 From: haotianqi666 Date: Sat, 24 Oct 2026 10:49:05 +0800 Subject: [PATCH] fix(skills): report podcast CLI failures with a nonzero status (#7236) Co-authored-by: haotianqi666 <250384991+haotianqi666@users.noreply.github.com> --- AGENTS.md | 2 +- README.md | 4 + skills/public/podcast-generation/SKILL.md | 4 + .../podcast-generation/scripts/generate.py | 4 +- tests/skills/test_podcast_cli.py | 145 ++++++++++++++++++ 5 files changed, 157 insertions(+), 2 deletions(-) create mode 100644 tests/skills/test_podcast_cli.py diff --git a/AGENTS.md b/AGENTS.md index 28da683125..9cfdc99173 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -226,7 +226,7 @@ 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; + Video- and podcast-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`. diff --git a/README.md b/README.md index c449574250..eba9d60a48 100644 --- a/README.md +++ b/README.md @@ -1629,6 +1629,10 @@ 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. +The bundled podcast-generation command returns a nonzero exit status and writes +diagnostics to stderr when generation fails. Callers can check the status before +presenting audio; an existing MP3 or generated transcript is not proof of success. + 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. diff --git a/skills/public/podcast-generation/SKILL.md b/skills/public/podcast-generation/SKILL.md index 23f5e05236..5aa1e81dce 100644 --- a/skills/public/podcast-generation/SKILL.md +++ b/skills/public/podcast-generation/SKILL.md @@ -163,6 +163,10 @@ The generated podcast follows the "Hello Deer" format: ## Output Handling +The command exits with status 1 and writes diagnostics to stderr if generation +fails. Only present the MP3 after a successful command; an existing audio file +or a newly written transcript does not establish that audio generation succeeded. + After generation: - Podcasts and transcripts are saved in `/mnt/user-data/outputs/` diff --git a/skills/public/podcast-generation/scripts/generate.py b/skills/public/podcast-generation/scripts/generate.py index 939e649e17..566ca59255 100644 --- a/skills/public/podcast-generation/scripts/generate.py +++ b/skills/public/podcast-generation/scripts/generate.py @@ -4,6 +4,7 @@ import json import logging import os import random +import sys import time import uuid from concurrent.futures import ThreadPoolExecutor, as_completed @@ -380,5 +381,6 @@ if __name__ == "__main__": print(result) except Exception as e: import traceback - print(f"Error generating podcast: {e}") + print(f"Error generating podcast: {e}", file=sys.stderr) traceback.print_exc() + sys.exit(1) diff --git a/tests/skills/test_podcast_cli.py b/tests/skills/test_podcast_cli.py new file mode 100644 index 0000000000..df97b13caa --- /dev/null +++ b/tests/skills/test_podcast_cli.py @@ -0,0 +1,145 @@ +"""Exercise the documented podcast command, including its process status.""" + +import base64 +import json +import runpy +import subprocess +import sys +from pathlib import Path + +import pytest +import requests + +SCRIPT = ( + Path(__file__).resolve().parents[2] + / "skills/public/podcast-generation/scripts/generate.py" +) +SCRIPT_DATA = {"lines": [{"speaker": "male", "paragraph": "Hello"}]} + + +@pytest.fixture(autouse=True) +def isolated_provider_env(monkeypatch): + for name in ( + "PODCAST_GENERATION_PROVIDER", + "VOLCENGINE_TTS_APPID", + "VOLCENGINE_TTS_ACCESS_TOKEN", + "MINIMAX_API_KEY", + ): + monkeypatch.delenv(name, raising=False) + monkeypatch.setenv("MINIMAX_TTS_MAX_RETRIES", "0") + + +@pytest.mark.parametrize( + ("case", "error"), + [ + ("missing-file", "FileNotFoundError"), + ("invalid-json", "JSONDecodeError"), + ("empty-script", "Script contains no lines"), + ("no-credentials", "No credentials found"), + ("volcengine", "VOLCENGINE_TTS_APPID"), + ("minimax", "MINIMAX_API_KEY is not set"), + ], +) +def test_cli_failure_status(monkeypatch, tmp_path, case, error): + script = tmp_path / "script.json" + if case != "missing-file": + data = {"lines": []} if case == "empty-script" else SCRIPT_DATA + script.write_text( + "{" if case == "invalid-json" else json.dumps(data), encoding="utf-8" + ) + if case in ("volcengine", "minimax"): + monkeypatch.setenv("PODCAST_GENERATION_PROVIDER", case) + output = tmp_path / "episode.mp3" + + result = subprocess.run( + [ + sys.executable, + str(SCRIPT), + "--script-file", + str(script), + "--output-file", + str(output), + ], + capture_output=True, + text=True, + timeout=20, + check=False, + ) + + assert result.returncode == 1 + assert result.stdout == "" + assert "Error generating podcast:" in result.stderr + assert error in result.stderr + assert not output.exists() + + +@pytest.mark.parametrize("provider", ["volcengine", "minimax"]) +@pytest.mark.parametrize( + "outcome", ["success", "http-error", "connection-error", "empty-audio"] +) +def test_cli_provider_outcome(monkeypatch, tmp_path, capsys, provider, outcome): + monkeypatch.setenv("PODCAST_GENERATION_PROVIDER", provider) + monkeypatch.setenv("VOLCENGINE_TTS_APPID", "test-app") + monkeypatch.setenv("VOLCENGINE_TTS_ACCESS_TOKEN", "test-token") + monkeypatch.setenv("MINIMAX_API_KEY", "test-key") + script = tmp_path / "script.json" + script.write_text(json.dumps(SCRIPT_DATA), encoding="utf-8") + output = tmp_path / "episode.mp3" + output.write_bytes(b"previous episode") + transcript = tmp_path / "episode.md" + monkeypatch.setattr( + sys, + "argv", + [ + str(SCRIPT), + "--script-file", + str(script), + "--output-file", + str(output), + "--transcript-file", + str(transcript), + ], + ) + + def post(*args, **kwargs): + if outcome == "connection-error": + raise requests.ConnectionError("provider unavailable") + response = requests.Response() + response.status_code = 403 if outcome == "http-error" else 200 + audio = b"" if outcome == "empty-audio" else b"new episode" + if provider == "volcengine": + payload = {"code": 3000, "data": base64.b64encode(audio).decode("ascii")} + else: + payload = {"base_resp": {"status_code": 0}, "data": {"audio": audio.hex()}} + response._content = json.dumps(payload).encode("utf-8") + return response + + monkeypatch.setattr(requests, "post", post) + + if outcome == "success": + runpy.run_path(str(SCRIPT), run_name="__main__") + assert output.read_bytes() == b"new episode" + assert "**Host (Male)**: Hello" in transcript.read_text(encoding="utf-8") + captured = capsys.readouterr() + assert "Successfully generated podcast" in captured.out + assert captured.err == "" + else: + with pytest.raises(SystemExit) as exc: + runpy.run_path(str(SCRIPT), run_name="__main__") + assert exc.value.code == 1 + assert output.read_bytes() == b"previous episode" + captured = capsys.readouterr() + assert captured.out == "" + assert "Error generating podcast:" in captured.err + + +def test_cli_argument_error_remains_nonzero(): + result = subprocess.run( + [sys.executable, str(SCRIPT)], + capture_output=True, + text=True, + timeout=20, + check=False, + ) + assert result.returncode == 2 + assert "--script-file" in result.stderr