From 9bcccce2db91ceb4e1f93f9b4fc08ac24c0fc6b2 Mon Sep 17 00:00:00 2001
From: 0xDevNinja <manmit0x@gmail.com>
Date: Tue, 30 Jun 2026 12:05:23 +0530
Subject: [PATCH] fix(publish): validate optional assets and default export
 inside project

Addresses review feedback on export_bundle:

- If subtitles_path or thumbnail_path is provided but the file is missing, the
  tool now fails with an explicit error instead of silently producing a package
  without that asset (which could ship an approved deliverable missing part of
  its content).
- Default export location now stays inside the project workspace: when the
  render lives at projects/<name>/renders/..., the bundle defaults to
  projects/<name>/exports/ (alongside artifacts/, assets/, renders/) rather than
  a repo-root exports/<name>/. export_dir remains an explicit override.

Tests cover both: missing optional asset errors, and the project-workspace
default path.
---
 tests/tools/test_export_bundle.py | 25 +++++++++++++++++++++++++
 tools/publishers/export_bundle.py | 28 +++++++++++++++++++++++++++-
 2 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/tests/tools/test_export_bundle.py b/tests/tools/test_export_bundle.py
index be35b80d..74e30c49 100644
--- a/tests/tools/test_export_bundle.py
+++ b/tests/tools/test_export_bundle.py
@@ -123,6 +123,31 @@ def test_infer_project_name(tmp_path):
     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()
diff --git a/tools/publishers/export_bundle.py b/tools/publishers/export_bundle.py
index ca59be18..43cd21ef 100644
--- a/tools/publishers/export_bundle.py
+++ b/tools/publishers/export_bundle.py
@@ -162,7 +162,19 @@ class ExportBundle(BaseTool):
 
         title = inputs["title"]
         project_name = inputs.get("project_name") or self._infer_project_name(video_path)
-        export_root = Path(inputs["export_dir"]).expanduser() if inputs.get("export_dir") else Path("exports") / project_name
+
+        # Explicitly-provided optional assets must exist — silently dropping them
+        # would ship a publish package missing part of an approved deliverable.
+        for key in ("subtitles_path", "thumbnail_path"):
+            val = inputs.get(key)
+            if val and not Path(val).expanduser().is_file():
+                return ToolResult(success=False, error=f"{key} provided but not found: {val}")
+
+        export_root = (
+            Path(inputs["export_dir"]).expanduser()
+            if inputs.get("export_dir")
+            else self._default_export_dir(video_path, project_name)
+        )
 
         video_dir = export_root / "video"
         meta_dir = export_root / "metadata"
@@ -272,6 +284,20 @@ class ExportBundle(BaseTool):
             artifacts=[str(out_video)],
         )
 
+    @staticmethod
+    def _default_export_dir(video_path: Path, project_name: str) -> Path:
+        """Keep run output inside the project workspace.
+
+        When the render lives at ``projects/<name>/renders/...`` (the OpenMontage
+        convention), default the bundle to ``projects/<name>/exports/`` alongside
+        ``artifacts/``, ``assets/`` and ``renders/``. Otherwise fall back to a
+        top-level ``exports/<project_name>/``.
+        """
+        resolved = video_path.resolve()
+        if resolved.parent.name == "renders":
+            return resolved.parent.parent / "exports"
+        return Path("exports") / project_name
+
     @staticmethod
     def _infer_project_name(video_path: Path) -> str:
         # projects/<name>/renders/final.mp4 -> <name>; fall back to the file stem.