diff --git a/src/agents/sandbox/capabilities/skills.py b/src/agents/sandbox/capabilities/skills.py index b46a3468d9..567fbffdeb 100644 --- a/src/agents/sandbox/capabilities/skills.py +++ b/src/agents/sandbox/capabilities/skills.py @@ -380,6 +380,62 @@ def _get_manifest_entry_by_path(manifest: Manifest, path: Path) -> BaseEntry | N return None +_BLOCK_SCALAR_HEADERS = frozenset({">", ">-", ">+", "|", "|-", "|+"}) + + +def _indent_of(line: str) -> int: + """Return the width of the leading whitespace on a frontmatter line.""" + + return len(line) - len(line.lstrip()) + + +def _fold_lines(block: list[str]) -> str: + """Join lines the way YAML folds them, turning blank lines into line breaks.""" + + folded = "" + blank_lines = 0 + for line in block: + text = line.strip() + if not text: + blank_lines += 1 + continue + if folded: + folded += "\n" * blank_lines if blank_lines else " " + folded += text + blank_lines = 0 + return folded + + +def _join_block_lines(block: list[str], *, literal: bool) -> str: + """Render the body of a block scalar introduced by a `>` or `|` header.""" + + if not literal: + return _fold_lines(block) + + indent = min((_indent_of(line) for line in block if line.strip()), default=0) + return "\n".join(line[indent:] if line.strip() else "" for line in block) + + +def _take_continuation_lines( + lines: list[str], start: int, end: int, key_indent: int +) -> tuple[list[str], int]: + """Collect the lines that belong to the key opened on the preceding line. + + A value can run past its own line as a block scalar or as a wrapped plain scalar, and a + key can open a nested block. All three indent their remaining lines past the key, so those + lines belong to that key and must not be read as keys of their own. + """ + + index = start + while index < end and (not lines[index].strip() or _indent_of(lines[index]) > key_indent): + index += 1 + + block = lines[start:index] + while block and not block[-1].strip(): + block.pop() + return block, index + + def _parse_frontmatter(markdown: str) -> dict[str, str]: """Parse the simple YAML frontmatter shape used by skill indexes.""" @@ -396,19 +452,33 @@ def _parse_frontmatter(markdown: str) -> dict[str, str]: return {} metadata: dict[str, str] = {} - for line in lines[1:end_index]: + index = 1 + while index < end_index: + line = lines[index] + index += 1 stripped = line.strip() if stripped == "" or stripped.startswith("#") or ":" not in stripped: continue key, value = stripped.split(":", 1) parsed_key = key.strip() parsed_value = value.strip() - if ( - len(parsed_value) >= 2 - and parsed_value[0] == parsed_value[-1] - and parsed_value[0] in {"'", '"'} - ): - parsed_value = parsed_value[1:-1] + continuation, index = _take_continuation_lines(lines, index, end_index, _indent_of(line)) + + if parsed_value in _BLOCK_SCALAR_HEADERS: + parsed_value = _join_block_lines(continuation, literal=parsed_value[0] == "|").strip() + else: + # A comment line is content inside a block scalar but a comment anywhere else, so it + # must not extend a wrapped value or keep a quoted one from being unwrapped. + continuation = [item for item in continuation if not item.strip().startswith("#")] + if continuation and parsed_value: + parsed_value = _fold_lines([parsed_value, *continuation]) + elif not continuation and ( + len(parsed_value) >= 2 + and parsed_value[0] == parsed_value[-1] + and parsed_value[0] in {"'", '"'} + ): + parsed_value = parsed_value[1:-1] + metadata[parsed_key] = parsed_value return metadata diff --git a/tests/sandbox/capabilities/test_skills_capability.py b/tests/sandbox/capabilities/test_skills_capability.py index 417e32cff3..9f19b0e257 100644 --- a/tests/sandbox/capabilities/test_skills_capability.py +++ b/tests/sandbox/capabilities/test_skills_capability.py @@ -552,6 +552,137 @@ async def test_instructions_resolve_opt_in_lazy_local_dir_metadata( assert "Call `load_skill` with a single skill name from the list" in instructions assert "loaded on demand instead of being present up front" in instructions + @pytest.mark.parametrize( + ("frontmatter_description", "expected_description"), + [ + pytest.param( + "description: >\n Use for GitHub issue triage.\n Triggers: /triage, bug report", + "Use for GitHub issue triage. Triggers: /triage, bug report", + id="folded_block_scalar", + ), + pytest.param( + "description: |\n Use for GitHub issue triage.\n Triggers: /triage, bug report", + "Use for GitHub issue triage.\nTriggers: /triage, bug report", + id="literal_block_scalar", + ), + pytest.param( + "description: >-\n Use for GitHub issue triage.\n Triggers: /triage, bug report", + "Use for GitHub issue triage. Triggers: /triage, bug report", + id="folded_block_scalar_with_chomping_indicator", + ), + pytest.param( + "description: Use for GitHub issue\n triage, not for PR review.", + "Use for GitHub issue triage, not for PR review.", + id="wrapped_plain_scalar", + ), + pytest.param( + "description: >\n Use for GitHub issue triage.\n\n Not for PR review.", + "Use for GitHub issue triage.\nNot for PR review.", + id="folded_block_scalar_with_blank_line", + ), + ], + ) + @pytest.mark.asyncio + async def test_instructions_keep_multi_line_frontmatter_descriptions( + self, + tmp_path: Path, + frontmatter_description: str, + expected_description: str, + ) -> None: + src_root = tmp_path / "skills" + skill_dir = src_root / "dynamic-skill" + skill_dir.mkdir(parents=True) + # The name follows the description so the test also covers where the value ends. + (skill_dir / "SKILL.md").write_text( + f"---\n{frontmatter_description}\nname: discovered-skill\n---\n# Skill\n", + encoding="utf-8", + ) + + capability = Skills( + lazy_from=LocalDirLazySkillSource(source=LocalDir(src=src_root)), + ) + + instructions = await capability.instructions(_source_granted_manifest(source=src_root)) + + assert instructions is not None + assert ( + f"- discovered-skill: {expected_description} (file: .agents/dynamic-skill)" + in instructions + ) + + @pytest.mark.parametrize( + ("frontmatter", "expected_line"), + [ + pytest.param( + "name: discovered-skill\n # explanation\ndescription: local dir metadata", + "- discovered-skill: local dir metadata", + id="indented_comment_after_plain_value", + ), + pytest.param( + 'name: discovered-skill\ndescription: "local dir metadata"\n # note', + "- discovered-skill: local dir metadata", + id="indented_comment_after_quoted_value", + ), + pytest.param( + "name: discovered-skill\ndescription: >\n Use for triage.\n # kept as content", + "- discovered-skill: Use for triage. # kept as content", + id="comment_line_inside_block_scalar_is_content", + ), + ], + ) + @pytest.mark.asyncio + async def test_instructions_treat_comment_lines_the_way_yaml_does( + self, + tmp_path: Path, + frontmatter: str, + expected_line: str, + ) -> None: + src_root = tmp_path / "skills" + skill_dir = src_root / "dynamic-skill" + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text( + f"---\n{frontmatter}\n---\n# Skill\n", + encoding="utf-8", + ) + + capability = Skills( + lazy_from=LocalDirLazySkillSource(source=LocalDir(src=src_root)), + ) + + instructions = await capability.instructions(_source_granted_manifest(source=src_root)) + + assert instructions is not None + assert f"{expected_line} (file: .agents/dynamic-skill)" in instructions + + @pytest.mark.asyncio + async def test_instructions_keep_skill_name_when_a_description_line_looks_like_a_key( + self, tmp_path: Path + ) -> None: + src_root = tmp_path / "skills" + skill_dir = src_root / "dynamic-skill" + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text( + "---\n" + "name: discovered-skill\n" + "description: >\n" + " Use for GitHub issue triage.\n" + " name: not-the-skill-name\n" + "---\n# Skill\n", + encoding="utf-8", + ) + + capability = Skills( + lazy_from=LocalDirLazySkillSource(source=LocalDir(src=src_root)), + ) + + instructions = await capability.instructions(_source_granted_manifest(source=src_root)) + + assert instructions is not None + assert ( + "- discovered-skill: Use for GitHub issue triage. name: not-the-skill-name " + "(file: .agents/dynamic-skill)" + ) in instructions + @pytest.mark.asyncio async def test_lazy_local_dir_metadata_skips_symlinked_skill_directory( self, tmp_path: Path