fix: use yaml.safe_dump for virtual package apm.yml generation#707
fix: use yaml.safe_dump for virtual package apm.yml generation#707edenfunf wants to merge 4 commits intomicrosoft:mainfrom
Conversation
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 microsoft#703
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR fixes invalid apm.yml generation for virtual file and collection packages when YAML-special characters (notably :) appear in unquoted metadata fields, by switching from f-string interpolation to yaml_to_str() (yaml.safe_dump).
Changes:
- Replace hand-built
apm.ymlstrings withyaml_to_str()indownload_virtual_file_packageanddownload_collection_package. - Preserve collection
tagsby adding them to the YAML data structure rather than string concatenation. - Add unit tests to ensure generated
apm.ymlremains parseable when:appears indescriptionorname.
Show a summary per file
| File | Description |
|---|---|
src/apm_cli/deps/github_downloader.py |
Switches virtual package apm.yml generation to yaml_to_str() for correct quoting/escaping of special characters. |
tests/test_github_downloader.py |
Adds regression tests that reproduce the colon-in-metadata YAML failure and validate generated YAML via yaml.safe_load. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 3
- 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.
sergio-sisternes-epam
left a comment
There was a problem hiding this comment.
Clean fix replacing f-string YAML generation with yaml_to_str() (backed by yaml.safe_dump). Both affected code paths fixed — virtual file and collection packages.
6 thorough tests covering colons in descriptions, names, and tags, plus regression guard for normal values. All Copilot findings addressed in follow-up commits.
Good use of the existing yaml_to_str helper — follows codebase conventions.
Problem
When a virtual file or collection dependency (e.g. a single
.agent.md) has a:in itsdescriptionfield,apm installfails with:Root cause:
download_virtual_file_packageanddownload_collection_packageingithub_downloader.pygeneratedapm.ymlby string interpolation (f"""name: {name}\ndescription: {description}\n"""). Any:in a value produces syntactically invalid YAML.Concrete example from the issue — swe-subagent frontmatter:
After stripping the surrounding quotes and dropping into the f-string, the
:after "tasks" is parsed as a YAML mapping separator, causing the error.Fix
Replace both f-string blocks with
yaml_to_str()(backed byyaml.safe_dump), which correctly quotes strings that contain:,#,[, or other YAML special characters.yaml_to_stralready exists inapm_cli/utils/yaml_io.pyand is the established helper for this purpose throughout the codebase.Tests
Three new unit tests in
TestVirtualFilePackageYamlGeneration:test_yaml_with_colon_in_descriptiontest_yaml_with_colon_in_nametest_yaml_without_special_characters_still_validChecklist
yaml.safe_loadon generatedapm.ymlsucceeds after changedownload_virtual_file_packageanddownload_collection_package)Closes #703