diff --git a/CHANGELOG.md b/CHANGELOG.md index ecd0e8b8..a2c7a897 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,16 @@ Entries before v0.5.0 were written retroactively as summaries. ## [Unreleased] +### Fixed + +- **AWS: uploaded custom templates now apply to PPTX generation** — the remote + server's template resolution only searched builtin templates, so a deck + referencing an uploaded user template silently fell back to + `blank-dark.pptx`. Generation now resolves user templates first (same order + as `analyze_template`), and an unresolvable template name raises an explicit + error listing available templates instead of silently using the wrong + design. (#206) + ## [0.7.0] - 2026-08-03 ### Added diff --git a/servers/remote/tools/generate.py b/servers/remote/tools/generate.py index 532bbf4e..4df732a9 100644 --- a/servers/remote/tools/generate.py +++ b/servers/remote/tools/generate.py @@ -248,13 +248,31 @@ def _collect_import_refs(value: object) -> set[str]: tmpl_name = "" # signal: resolved if tmpl_name: normalized = tmpl_name.removesuffix(".pptx") - for t in storage.list_templates(): - if t.get("name") == normalized: - template_key = t.get("s3Key", "") - break + # User templates take precedence (same order as analyze_template). + if storage.get_user_template_metadata(user_id, normalized): + template_path.write_bytes( + storage.download_user_template(user_id, normalized) + ) + else: + for t in storage.list_templates(): + if t.get("name") == normalized: + template_key = t.get("s3Key", "") + break + if not template_key: + # Do NOT silently fall back to a stock template: the deck + # explicitly references a template, so building with a + # different one would silently produce the wrong design. + available = [t.get("name", "") for t in storage.list_templates()] + available += [ + t.get("name", "") for t in storage.list_user_templates(user_id) + ] + raise ValueError( + f"Template '{tmpl_name}' not found. " + f"Available: {', '.join(available)}" + ) if not template_path.exists(): if not template_key: - template_key = deck.get("templateS3Key", "templates/blank-dark.pptx") + template_key = "templates/blank-dark.pptx" template_path.write_bytes(storage.download_file(key=template_key)) # Fonts diff --git a/tests/test_mcp_generate.py b/tests/test_mcp_generate.py index fd0d1719..457bed48 100644 --- a/tests/test_mcp_generate.py +++ b/tests/test_mcp_generate.py @@ -27,12 +27,12 @@ def mock_storage(): """Storage mock materializing a one-slide deck.""" storage = MagicMock() storage.pptx_bucket = "pptx-bucket" - storage.get_deck.return_value = { - "deckId": "d1", "name": "Test", "templateS3Key": "templates/blank-dark.pptx", - } + storage.get_deck.return_value = {"deckId": "d1", "name": "Test"} storage.get_deck_json.return_value = { "template": "", "fonts": {"fullwidth": "", "halfwidth": ""}, } + storage.get_user_template_metadata.return_value = None + storage.list_user_templates.return_value = [] def list_files(prefix: str, bucket: str = ""): if prefix.endswith("/slides/"): @@ -81,3 +81,50 @@ def test_generate_pptx_missing_deck(mock_storage): mock_storage.get_deck.return_value = None with pytest.raises(ValueError, match="not found"): generate_mod.generate_pptx(deck_id="dX", user_id="u1", storage=mock_storage) + + +def test_prepare_workspace_resolves_user_template(mock_storage): + """A deck referencing an uploaded user template uses it (Issue #206).""" + mock_storage.get_deck_json.return_value = { + "template": "my-brand.pptx", "fonts": {"fullwidth": "", "halfwidth": ""}, + } + mock_storage.get_user_template_metadata.return_value = { + "name": "my-brand", "s3Key": "user-templates/u1/my-brand.pptx", + } + mock_storage.download_user_template.return_value = _TEMPLATE.read_bytes() + + tmpdir, _slides, build_kwargs = generate_mod._prepare_workspace( + "d1", "u1", mock_storage, + ) + + mock_storage.get_user_template_metadata.assert_called_once_with("u1", "my-brand") + mock_storage.download_user_template.assert_called_once_with("u1", "my-brand") + assert build_kwargs["template_path"].read_bytes()[:2] == b"PK" + # Builtin download path must not be used for the template + for call in mock_storage.download_file.call_args_list: + assert not call.kwargs.get("key", "").startswith("templates/") + + +def test_prepare_workspace_unknown_template_raises(mock_storage): + """An unresolvable template name fails loudly instead of silently + falling back to blank-dark (Issue #206).""" + mock_storage.get_deck_json.return_value = { + "template": "ghost.pptx", "fonts": {"fullwidth": "", "halfwidth": ""}, + } + mock_storage.list_templates.return_value = [ + {"name": "blank-dark", "s3Key": "templates/blank-dark.pptx"}, + ] + mock_storage.list_user_templates.return_value = [{"name": "my-brand"}] + + with pytest.raises(ValueError, match=r"'ghost\.pptx' not found.*blank-dark.*my-brand"): + generate_mod._prepare_workspace("d1", "u1", mock_storage) + + +def test_prepare_workspace_empty_template_defaults_to_blank_dark(mock_storage): + """No template specified → blank-dark default (regression guard).""" + tmpdir, _slides, build_kwargs = generate_mod._prepare_workspace( + "d1", "u1", mock_storage, + ) + + mock_storage.download_file.assert_any_call(key="templates/blank-dark.pptx") + assert build_kwargs["template_path"].read_bytes()[:2] == b"PK"