test(skills): cover get_external_skills_dirs input validation branches

[wesleysimplicio]

What does this PR do?

Adds 11 pytest cases pinning the input-validation and entry-normalization branches of agent.skill_utils.get_external_skills_dirs that the existing mtime-cache happy-path suite (test_external_skills_dirs_cache.py) leaves uncovered.

Root cause

The detailed rationale from the original PR body is preserved below. This template update keeps the review structure consistent with #29640.

Fix

• Standardizes the PR description to the same review format used in fix(gateway): normalize false reply mode at config boundary #29640.
• Preserves the original detailed description below so no context is lost.

Why this shape

get_external_skills_dirs is on the cold-start hot path (called once per skill during banner / tool-registry scans) and silently swallows malformed config.yaml shapes — a YAML root that isn't a mapping, a skills: value that isn't a mapping, an external_dirs that isn't a list, etc. Today none of those defensive branches have a regression test, so a future cleanup that drops the wrong isinstance check would pass CI green and break user configs at runtime.

Tests

• Veja a descrição original preservada abaixo para detalhes de validação, testes e notas de verificação.

Original body

Related PRs / issues

• Original body preserved below for full context.

Original body

Summary

Adds 11 pytest cases pinning the input-validation and entry-normalization branches of agent.skill_utils.get_external_skills_dirs that the existing mtime-cache happy-path suite (test_external_skills_dirs_cache.py) leaves uncovered.

What Changed

• Standardized this PR body to the current Hermes Turbo template.
• Preserved the original detailed description below for reference.

Fluxo

A mudança continua seguindo o fluxo original descrito na seção preservada abaixo, sem ampliar o escopo funcional deste PR.

Visão

A padronização melhora a revisão, reduz ruído e evita deriva de formatação entre PRs abertos.

Test Plan

• Veja a descrição original preservada abaixo para detalhes de validação, testes e notas de verificação.

Original body

What

Adds 11 pytest cases pinning the input-validation and entry-normalization branches of agent.skill_utils.get_external_skills_dirs that the existing mtime-cache happy-path suite (test_external_skills_dirs_cache.py) leaves uncovered.

Why

get_external_skills_dirs is on the cold-start hot path (called once per skill during banner / tool-registry scans) and silently swallows malformed config.yaml shapes — a YAML root that isn't a mapping, a skills: value that isn't a mapping, an external_dirs that isn't a list, etc. Today none of those defensive branches have a regression test, so a future cleanup that drops the wrong isinstance check would pass CI green and break user configs at runtime.

Coverage added

Branch in skill_utils.py	Test
not isinstance(parsed, dict)	test_yaml_root_not_mapping_returns_empty
not isinstance(skills_cfg, dict)	test_skills_section_not_mapping_returns_empty
isinstance(raw_dirs, str) coercion	test_external_dirs_as_single_string_is_coerced_to_list
not isinstance(raw_dirs, list)	test_external_dirs_as_invalid_type_returns_empty
os.path.expandvars	test_environment_variable_in_entry_is_expanded
os.path.expanduser	test_tilde_in_entry_is_expanded
not p.is_absolute() resolves under HERMES_HOME	test_relative_entry_is_resolved_against_hermes_home
p == local_skills drop	test_local_skills_dir_is_silently_dropped
p in seen dedup	test_duplicate_entries_collapse
p.is_dir() skip	test_nonexistent_entries_are_skipped
if not entry: continue	test_empty_and_whitespace_entries_are_skipped

Test design

A single isolated_hermes_home fixture spins up a throwaway ~/.hermes/ under tmp_path, monkeypatches HERMES_HOME, HOME, and Path.home, and clears _external_dirs_cache_clear() before and after each case so no test leaks into another. Every test writes its own config.yaml body — there is no shared YAML fixture to mis-edit later.

Verification

pytest tests/agent/test_external_skills_dirs_validation.py -v
============================== 11 passed in 1.95s ==============================

Full repo suite (pytest tests/ -q --ignore=tests/integration --ignore=tests/e2e -n auto) was also run; the new file adds 11 passes and no regressions. The 37 unrelated failures observed in the run reproduce on the parent commit 78b0008f4 without this patch (mostly env-dependent: anthropic-adapter OAuth, systemd-on-macOS, file-state registry).

Out of scope

No production code is touched — this PR is pure test coverage.

---

Generated by Hermes Turbo

---

Generated by Hermes Turbo

[Copilot]

Pull request overview

Adds a focused pytest suite to cover the defensive/input-validation and path-normalization branches of agent.skill_utils.get_external_skills_dirs, complementing the existing cache/mtime-oriented tests.

Changes:

• Introduces 11 new tests covering malformed config.yaml shapes (non-mapping root, non-mapping skills, invalid external_dirs types).
• Adds coverage for normalization behavior (string coercion to list, env/tilde expansion, relative path resolution under HERMES_HOME, deduping, skipping empty/nonexistent/local skills dir entries).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[wesleysimplicio]

Closing non-fix PR as requested — mantendo apenas PRs de fix.