Skip to content

Python: clarify skill script argument guidance - #7695

Merged
Giles Odigwe (giles17) merged 3 commits into
microsoft:mainfrom
uuzzrm:uuzzrm/issue-7691
Aug 17, 2026
Merged

Python: clarify skill script argument guidance#7695
Giles Odigwe (giles17) merged 3 commits into
microsoft:mainfrom
uuzzrm:uuzzrm/issue-7691

Conversation

@uuzzrm

@uuzzrm Ruiming Zhao (uuzzrm) commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Motivation & Context

SCRIPT_RUNNER_INSTRUCTIONS currently tells agents to pass args as an object, while the run_skill_script schema also accepts string arrays for file-based CLI scripts. That guidance can steer agents away from a supported argument shape. This change aligns the default prompt with the existing schema and runtime behavior.

Description & Review Guide

  • What are the major changes?
    • Clarify that named arguments, including inline scripts, use a JSON object inside args.
    • Clarify that string arrays are for file-based scripts that document CLI-style positional arguments.
    • Add a regression test for the generated default prompt.
    • Close the inline-code example correctly so the generated guidance renders as intended.
  • What is the impact of these changes?
    • No runtime or API behavior changes; the default prompt now distinguishes the two supported argument forms.
    • The focused test_skills.py suite passes with 537 passed and 11 skipped; Ruff check and format checks pass.
    • uv build successfully produced the core sdist and wheel. The repository poe build -P core wrapper then stops at its sh-based dist move because sh is unavailable in this Windows environment.
  • What do you want reviewers to focus on?
    • Whether the wording matches the file-based and inline script behavior.
    • Whether the test asserts the distinction without coupling to an entire sentence.

This PR was prepared with AI assistance; I reviewed the patch and verified the focused behavior locally.

Related Issue

Fixes #7691

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix) — a workflow keeps the label and the title prefix in sync automatically.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates the skills prompt guidance to clarify how to pass script arguments for inline vs file-based scripts, and adds a regression test to ensure the prompt includes this distinction.

Changes:

  • Expand SCRIPT_RUNNER_INSTRUCTIONS to describe args as either a JSON object (named args) or an array (CLI positional args).
  • Add a test asserting the default prompt contains the new guidance.

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/_skills.py Clarifies prompt instructions for run_skill_script argument shapes (object vs array).
python/packages/core/tests/core/test_skills.py Adds a regression test to ensure the prompt includes the new argument-shape guidance.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread python/packages/core/tests/core/test_skills.py Outdated
@agent-framework-automation agent-framework-automation Bot added the python Usage: [Issues, PRs], Target: Python label Aug 17, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Python Test Coverage

Python Test Coverage Report •
FileStmtsMissCoverMissing
packages/core/agent_framework
   _skills.py13567094%321, 591, 1109, 1124, 1126–1127, 1493–1494, 1736, 1765, 2283, 2473, 2991–2992, 3094, 3102, 3107, 3110, 3116, 3136, 3148, 3153, 3249, 3257, 3262, 3265, 3271, 3291, 3300, 3305, 3598–3599, 4063, 4066, 4068–4069, 4071, 4222, 4549, 4564, 4572, 4599, 4603, 4607, 4670–4671, 4673–4674, 4686, 4690, 4726–4727, 4731–4732, 4778–4780, 4786, 4835, 4839, 4843, 4853–4855, 5057–5058, 5080–5081, 5088–5089
TOTAL46779435790% 

Python Unit Test Overview

Tests Skipped Failures Errors Time
9453 36 💤 0 ❌ 0 🔥 2m 32s ⏱️

@giles17
Giles Odigwe (giles17) added this pull request to the merge queue Aug 17, 2026
Merged via the queue into microsoft:main with commit 925d722 Aug 17, 2026
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: [Bug]: Skill runner instructions contradict array-capable args schema

3 participants