From db4b42955208bf91eb887cb84b209edd1f88d8fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=A8=B1=E5=85=83=E8=B1=AA?= <146086744+edenfunf@users.noreply.github.com> Date: Tue, 14 Apr 2026 20:15:38 +0800 Subject: [PATCH 1/2] fix: use yaml.safe_dump for virtual package apm.yml generation When a virtual file or collection package has a `:` in its description, name, or tag values, the f-string interpolation produced invalid YAML (e.g. `description: tasks: feature development` triggers a mapping-values error on load). Replace both f-string blocks in `download_virtual_file_package` and `download_collection_package` with `yaml_to_str()` (backed by `yaml.safe_dump`), which correctly quotes strings containing special YAML characters. Fixes #703 --- src/apm_cli/deps/github_downloader.py | 32 +++++---- tests/test_github_downloader.py | 98 +++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 15 deletions(-) diff --git a/src/apm_cli/deps/github_downloader.py b/src/apm_cli/deps/github_downloader.py index ee5714f7..c89dd066 100644 --- a/src/apm_cli/deps/github_downloader.py +++ b/src/apm_cli/deps/github_downloader.py @@ -1515,11 +1515,14 @@ def download_virtual_file_package(self, dep_ref: DependencyReference, target_pat # If frontmatter parsing fails, use default description pass - apm_yml_content = f"""name: {package_name} -version: 1.0.0 -description: {description} -author: {dep_ref.repo_url.split('/')[0]} -""" + from ..utils.yaml_io import yaml_to_str + apm_yml_data = { + "name": package_name, + "version": "1.0.0", + "description": description, + "author": dep_ref.repo_url.split('/')[0], + } + apm_yml_content = yaml_to_str(apm_yml_data) apm_yml_path = target_path / "apm.yml" apm_yml_path.write_text(apm_yml_content, encoding='utf-8') @@ -1655,17 +1658,16 @@ def download_collection_package(self, dep_ref: DependencyReference, target_path: # Generate apm.yml with collection metadata package_name = dep_ref.get_virtual_package_name() - apm_yml_content = f"""name: {package_name} -version: 1.0.0 -description: {manifest.description} -author: {dep_ref.repo_url.split('/')[0]} -""" - - # Add tags if present + from ..utils.yaml_io import yaml_to_str + apm_yml_data = { + "name": package_name, + "version": "1.0.0", + "description": manifest.description, + "author": dep_ref.repo_url.split('/')[0], + } if manifest.tags: - apm_yml_content += f"\ntags:\n" - for tag in manifest.tags: - apm_yml_content += f" - {tag}\n" + apm_yml_data["tags"] = list(manifest.tags) + apm_yml_content = yaml_to_str(apm_yml_data) apm_yml_path = target_path / "apm.yml" apm_yml_path.write_text(apm_yml_content, encoding='utf-8') diff --git a/tests/test_github_downloader.py b/tests/test_github_downloader.py index a043c07e..eac68bfd 100644 --- a/tests/test_github_downloader.py +++ b/tests/test_github_downloader.py @@ -1656,5 +1656,103 @@ def test_try_raw_download_returns_content_on_200(self): assert result == b'hello world' +class TestVirtualFilePackageYamlGeneration: + """Tests that apm.yml for virtual packages is always valid YAML.""" + + def setup_method(self): + self.downloader = GitHubPackageDownloader() + self.temp_dir = Path(tempfile.mkdtemp()) + + def teardown_method(self): + if self.temp_dir.exists(): + shutil.rmtree(self.temp_dir, ignore_errors=True) + + def _make_dep_ref(self, virtual_path): + """Helper: build a minimal DependencyReference for a virtual file.""" + from apm_cli.models.apm_package import DependencyReference + dep_ref = Mock(spec=DependencyReference) + dep_ref.is_virtual = True + dep_ref.virtual_path = virtual_path + dep_ref.reference = "main" + dep_ref.repo_url = "github/awesome-copilot" + dep_ref.get_virtual_package_name.return_value = "awesome-copilot-swe-subagent" + dep_ref.to_github_url.return_value = f"https://github.com/github/awesome-copilot/blob/main/{virtual_path}" + dep_ref.is_virtual_file.return_value = True + dep_ref.VIRTUAL_FILE_EXTENSIONS = [".prompt.md", ".instructions.md", ".chatmode.md", ".agent.md"] + return dep_ref + + def test_yaml_with_colon_in_description(self): + """apm.yml must be valid when the agent description contains a colon.""" + import yaml + + agent_content = ( + b"---\n" + b"name: 'SWE'\n" + b"description: 'Senior software engineer subagent for implementation tasks:" + b" feature development, debugging, refactoring, and testing.'\n" + b"tools: ['vscode']\n" + b"---\n\n## Body\n" + ) + + dep_ref = self._make_dep_ref("agents/swe-subagent.agent.md") + target_path = self.temp_dir / "pkg" + + with patch.dict(os.environ, {}, clear=True), _CRED_FILL_PATCH: + with patch.object(self.downloader, "download_raw_file", return_value=agent_content): + pkg_info = self.downloader.download_virtual_file_package(dep_ref, target_path) + + apm_yml_path = target_path / "apm.yml" + assert apm_yml_path.exists(), "apm.yml was not created" + + content = apm_yml_path.read_text(encoding="utf-8") + parsed = yaml.safe_load(content) # must not raise + + expected = ( + "Senior software engineer subagent for implementation tasks:" + " feature development, debugging, refactoring, and testing." + ) + assert parsed["description"] == expected + + def test_yaml_with_colon_in_name(self): + """apm.yml must be valid even when the package name contains a colon.""" + import yaml + + dep_ref = self._make_dep_ref("agents/my-agent.agent.md") + dep_ref.get_virtual_package_name.return_value = "org-name: special" + + agent_content = b"---\nname: 'plain'\ndescription: 'plain'\n---\n" + target_path = self.temp_dir / "pkg2" + + with patch.dict(os.environ, {}, clear=True), _CRED_FILL_PATCH: + with patch.object(self.downloader, "download_raw_file", return_value=agent_content): + self.downloader.download_virtual_file_package(dep_ref, target_path) + + content = (target_path / "apm.yml").read_text(encoding="utf-8") + parsed = yaml.safe_load(content) + assert parsed["name"] == "org-name: special" + + def test_yaml_without_special_characters_still_valid(self): + """apm.yml generation must still work for ordinary descriptions.""" + import yaml + + agent_content = ( + b"---\n" + b"name: 'Simple Agent'\n" + b"description: 'A simple agent without special chars'\n" + b"---\n" + ) + + dep_ref = self._make_dep_ref("agents/simple.agent.md") + target_path = self.temp_dir / "pkg3" + + with patch.dict(os.environ, {}, clear=True), _CRED_FILL_PATCH: + with patch.object(self.downloader, "download_raw_file", return_value=agent_content): + self.downloader.download_virtual_file_package(dep_ref, target_path) + + content = (target_path / "apm.yml").read_text(encoding="utf-8") + parsed = yaml.safe_load(content) + assert parsed["description"] == "A simple agent without special chars" + + if __name__ == '__main__': pytest.main([__file__]) From 5b03f0dad3f1c54c72d82a9d47ba2425347cfff6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=A8=B1=E5=85=83=E8=B1=AA?= <146086744+edenfunf@users.noreply.github.com> Date: Wed, 15 Apr 2026 01:13:14 +0800 Subject: [PATCH 2/2] refactor: move yaml_to_str import to module level; expand YAML tests - Move `yaml_to_str` from inline function imports to the module-level import block, removing the duplicated local imports in `download_virtual_file_package` and `download_collection_package`. - Switch test temp-directory management from manual `tempfile.mkdtemp()` + `teardown_method` to pytest's `tmp_path` fixture for automatic, reliable cleanup. - Add two new tests covering the `download_collection_package` path: colon in `description` and colon inside `tags` values. --- src/apm_cli/deps/github_downloader.py | 3 +- tests/test_github_downloader.py | 118 +++++++++++++++++++++----- 2 files changed, 99 insertions(+), 22 deletions(-) diff --git a/src/apm_cli/deps/github_downloader.py b/src/apm_cli/deps/github_downloader.py index c89dd066..1b128786 100644 --- a/src/apm_cli/deps/github_downloader.py +++ b/src/apm_cli/deps/github_downloader.py @@ -42,6 +42,7 @@ is_azure_devops_hostname, is_github_hostname ) +from ..utils.yaml_io import yaml_to_str def normalize_collection_path(virtual_path: str) -> str: @@ -1515,7 +1516,6 @@ def download_virtual_file_package(self, dep_ref: DependencyReference, target_pat # If frontmatter parsing fails, use default description pass - from ..utils.yaml_io import yaml_to_str apm_yml_data = { "name": package_name, "version": "1.0.0", @@ -1658,7 +1658,6 @@ def download_collection_package(self, dep_ref: DependencyReference, target_path: # Generate apm.yml with collection metadata package_name = dep_ref.get_virtual_package_name() - from ..utils.yaml_io import yaml_to_str apm_yml_data = { "name": package_name, "version": "1.0.0", diff --git a/tests/test_github_downloader.py b/tests/test_github_downloader.py index eac68bfd..19de991b 100644 --- a/tests/test_github_downloader.py +++ b/tests/test_github_downloader.py @@ -1659,14 +1659,6 @@ def test_try_raw_download_returns_content_on_200(self): class TestVirtualFilePackageYamlGeneration: """Tests that apm.yml for virtual packages is always valid YAML.""" - def setup_method(self): - self.downloader = GitHubPackageDownloader() - self.temp_dir = Path(tempfile.mkdtemp()) - - def teardown_method(self): - if self.temp_dir.exists(): - shutil.rmtree(self.temp_dir, ignore_errors=True) - def _make_dep_ref(self, virtual_path): """Helper: build a minimal DependencyReference for a virtual file.""" from apm_cli.models.apm_package import DependencyReference @@ -1681,7 +1673,20 @@ def _make_dep_ref(self, virtual_path): dep_ref.VIRTUAL_FILE_EXTENSIONS = [".prompt.md", ".instructions.md", ".chatmode.md", ".agent.md"] return dep_ref - def test_yaml_with_colon_in_description(self): + def _make_collection_dep_ref(self, virtual_path): + """Helper: build a minimal DependencyReference for a virtual collection.""" + from apm_cli.models.apm_package import DependencyReference + dep_ref = Mock(spec=DependencyReference) + dep_ref.is_virtual = True + dep_ref.virtual_path = virtual_path + dep_ref.reference = "main" + dep_ref.repo_url = "github/my-org" + dep_ref.get_virtual_package_name.return_value = "my-org-my-collection" + dep_ref.to_github_url.return_value = f"https://github.com/github/my-org/blob/main/{virtual_path}" + dep_ref.is_virtual_collection.return_value = True + return dep_ref + + def test_yaml_with_colon_in_description(self, tmp_path): """apm.yml must be valid when the agent description contains a colon.""" import yaml @@ -1695,11 +1700,12 @@ def test_yaml_with_colon_in_description(self): ) dep_ref = self._make_dep_ref("agents/swe-subagent.agent.md") - target_path = self.temp_dir / "pkg" + target_path = tmp_path / "pkg" + downloader = GitHubPackageDownloader() with patch.dict(os.environ, {}, clear=True), _CRED_FILL_PATCH: - with patch.object(self.downloader, "download_raw_file", return_value=agent_content): - pkg_info = self.downloader.download_virtual_file_package(dep_ref, target_path) + with patch.object(downloader, "download_raw_file", return_value=agent_content): + downloader.download_virtual_file_package(dep_ref, target_path) apm_yml_path = target_path / "apm.yml" assert apm_yml_path.exists(), "apm.yml was not created" @@ -1713,7 +1719,7 @@ def test_yaml_with_colon_in_description(self): ) assert parsed["description"] == expected - def test_yaml_with_colon_in_name(self): + def test_yaml_with_colon_in_name(self, tmp_path): """apm.yml must be valid even when the package name contains a colon.""" import yaml @@ -1721,17 +1727,18 @@ def test_yaml_with_colon_in_name(self): dep_ref.get_virtual_package_name.return_value = "org-name: special" agent_content = b"---\nname: 'plain'\ndescription: 'plain'\n---\n" - target_path = self.temp_dir / "pkg2" + target_path = tmp_path / "pkg" + downloader = GitHubPackageDownloader() with patch.dict(os.environ, {}, clear=True), _CRED_FILL_PATCH: - with patch.object(self.downloader, "download_raw_file", return_value=agent_content): - self.downloader.download_virtual_file_package(dep_ref, target_path) + with patch.object(downloader, "download_raw_file", return_value=agent_content): + downloader.download_virtual_file_package(dep_ref, target_path) content = (target_path / "apm.yml").read_text(encoding="utf-8") parsed = yaml.safe_load(content) assert parsed["name"] == "org-name: special" - def test_yaml_without_special_characters_still_valid(self): + def test_yaml_without_special_characters_still_valid(self, tmp_path): """apm.yml generation must still work for ordinary descriptions.""" import yaml @@ -1743,16 +1750,87 @@ def test_yaml_without_special_characters_still_valid(self): ) dep_ref = self._make_dep_ref("agents/simple.agent.md") - target_path = self.temp_dir / "pkg3" + target_path = tmp_path / "pkg" + downloader = GitHubPackageDownloader() with patch.dict(os.environ, {}, clear=True), _CRED_FILL_PATCH: - with patch.object(self.downloader, "download_raw_file", return_value=agent_content): - self.downloader.download_virtual_file_package(dep_ref, target_path) + with patch.object(downloader, "download_raw_file", return_value=agent_content): + downloader.download_virtual_file_package(dep_ref, target_path) content = (target_path / "apm.yml").read_text(encoding="utf-8") parsed = yaml.safe_load(content) assert parsed["description"] == "A simple agent without special chars" + def test_collection_yaml_with_colon_in_description(self, tmp_path): + """apm.yml for collection packages must be valid when description contains a colon.""" + import yaml + + # A minimal .collection.yml whose description contains ":" + collection_manifest = ( + b"id: my-collection\n" + b"name: My Collection\n" + b"description: 'A collection for tasks: feature development, debugging.'\n" + b"items:\n" + b" - path: agents/my-agent.agent.md\n" + b" kind: agent\n" + ) + agent_file = b"---\nname: My Agent\n---\n## Body\n" + + dep_ref = self._make_collection_dep_ref("collections/my-collection") + target_path = tmp_path / "pkg" + + downloader = GitHubPackageDownloader() + + def _fake_download(dep_ref_arg, path, ref): + if "collection" in path: + return collection_manifest + return agent_file + + with patch.dict(os.environ, {}, clear=True), _CRED_FILL_PATCH: + with patch.object(downloader, "download_raw_file", side_effect=_fake_download): + downloader.download_collection_package(dep_ref, target_path) + + content = (target_path / "apm.yml").read_text(encoding="utf-8") + parsed = yaml.safe_load(content) # must not raise + + assert parsed["description"] == "A collection for tasks: feature development, debugging." + + def test_collection_yaml_with_colon_in_tags(self, tmp_path): + """apm.yml for collection packages must be valid when tags contain a colon.""" + import yaml + + collection_manifest = ( + b"id: tagged-collection\n" + b"name: Tagged\n" + b"description: Normal description\n" + b"tags:\n" + b" - 'scope: engineering'\n" + b" - plain-tag\n" + b"items:\n" + b" - path: agents/my-agent.agent.md\n" + b" kind: agent\n" + ) + agent_file = b"---\nname: My Agent\n---\n## Body\n" + + dep_ref = self._make_collection_dep_ref("collections/tagged-collection") + target_path = tmp_path / "pkg" + + downloader = GitHubPackageDownloader() + + def _fake_download(dep_ref_arg, path, ref): + if "collection" in path: + return collection_manifest + return agent_file + + with patch.dict(os.environ, {}, clear=True), _CRED_FILL_PATCH: + with patch.object(downloader, "download_raw_file", side_effect=_fake_download): + downloader.download_collection_package(dep_ref, target_path) + + content = (target_path / "apm.yml").read_text(encoding="utf-8") + parsed = yaml.safe_load(content) + + assert parsed["tags"] == ["scope: engineering", "plain-tag"] + if __name__ == '__main__': pytest.main([__file__])