Python: create_harness_agent skills_paths accepts str | Path | Sequence[str | Path] | None#6717
Python: create_harness_agent skills_paths accepts str | Path | Sequence[str | Path] | None#6717Copilot wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the Python harness agent factory API by making create_harness_agent(..., skills_paths=...) accept the same path shapes as the lower-level skills APIs (str | Path | Sequence[str | Path] | None) and fixes the call-site to SkillsProvider.from_paths so multiple paths work.
Changes:
- Widened
create_harness_agent.skills_pathstyping to acceptstr,Path, or sequences of them (orNone). - Fixed
_assemble_context_providersto callSkillsProvider.from_paths(skills_paths)(instead of*skills_paths). - Added tests covering
skills_pathspassed as a singlestr, a singlePath, and aSequence[Path].
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| python/packages/core/agent_framework/_harness/_agent.py | Widens skills_paths type and fixes the SkillsProvider.from_paths call site; updates docstring accordingly. |
| python/packages/core/tests/core/test_harness_agent.py | Adds new unit tests validating the newly accepted skills_paths input shapes. |
| file_access_store: AgentFileStore | None = None, | ||
| skills_provider: SkillsProvider | None = None, | ||
| skills_paths: Sequence[str] | None = None, | ||
| skills_paths: str | Path | Sequence[str | Path] | None = None, |
westey-m
left a comment
There was a problem hiding this comment.
There are build failures, please address.
…tch public signature
Fixed in the latest commit — updated |
Motivation & Context
create_harness_agent'sskills_pathsparameter was typed asSequence[str] | None, forcing callers to wrap bare paths in a list and convertPathobjects to strings — inconsistent withSkillsProvider.from_pathsandFileSkillsSource, which already acceptstr | Path | Sequence[str | Path]. Closes #6705.Description & Review Guide
What are the major changes?
skills_pathstype widened fromSequence[str] | None→str | Path | Sequence[str | Path] | NoneSkillsProvider.from_paths(*skills_paths)→SkillsProvider.from_paths(skills_paths). The old spread-unpack was also latently broken:from_pathstakes a single positionalskill_pathsargument, so passing*["a", "b"]would raiseTypeErrorfor two or more pathsstr, singlePath, andSequence[Path]What is the impact of these changes? Callers can now write:
instead of
skills_paths=[str(SKILLS_DIR)].What do you want reviewers to focus on? The call-site fix (
*skills_paths→skills_paths) and whether the latent multi-path bug was intentional or overlooked.Related Issue
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.