[dotnet-port-api] Port skill tool auto-approval helpers - #1045
Quim Muntal (qmuntal) merged 4 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…-20260709-0a5188c6deb256e2
There was a problem hiding this comment.
🟡 Changes recommended
The new “approve all skills tools” helper and sample should include an explicit warning about auto-approving script execution to reduce unsafe copy/paste usage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Ports the public API surface for skills tool auto-approval helpers from the upstream .NET change, adding exported tool-name constants and reusable auto-approval rule helpers in agent/skills, plus updating docs and providing a sample demonstrating usage.
Changes:
- Added exported skill tool-name constants and two reusable auto-approval rule helpers (
ReadOnlyToolsAutoApprovalRule,AllToolsAutoApprovalRule). - Updated skills provider tests to use the new constants and added focused tests for the new helper rules.
- Added an example demonstrating skills tool auto-approval usage and updated the .NET/Go feature comparison note.
File summaries
| File | Description |
|---|---|
| agent/skills/provider.go | Exports tool-name constants, adds auto-approval helper rules, and uses constants for tool registration. |
| agent/skills/provider_test.go | Adds coverage for the new auto-approval helpers and updates existing tool-name references to constants. |
| docs/dotnet-go-sdk-feature-comparison.md | Documents the new exported constants and reusable auto-approval helpers as part of skills parity notes. |
| examples/02-agents/skills/step04_skills_auto_approval/main.go | New sample demonstrating skills tool auto-approval configuration. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Generated by Go API Consistency Review Agent for #1045 · copilot · auto · 70.1 AIC · ⌖ 6.71 AIC · ⊞ 9.6K
|
Copilot address PR feedbackl |
Co-authored-by: michelle-clayton-work <262183035+michelle-clayton-work@users.noreply.github.com>
Addressed in |
This comment has been minimized.
This comment has been minimized.
| LoadSkillToolName = "load_skill" | ||
| // ReadSkillResourceToolName is the name of the tool that reads a skill resource. | ||
| ReadSkillResourceToolName = "read_skill_resource" | ||
| // RunSkillScriptToolName is the name of the tool that runs a skill script. | ||
| RunSkillScriptToolName = "run_skill_script" |
There was a problem hiding this comment.
Unexport this for now, not sure they provide enough value.
|
Scope: public API, examples Changed Go contract: New exported Upstream evidence reviewed: Result: findings reported (one gap; behavior otherwise aligned). The auto-approval rule helpers themselves match .NET semantics well (read-only vs. all-tools approval, same tool set, same "unmatched call → false, let other rules decide" contract). One gap: .NET makes the three skill tool names (
|
There was a problem hiding this comment.
Generated by Go API Consistency Review Agent for #1045 · copilot · auto · 41.9 AIC · ⌖ 8.14 AIC · ⊞ 9.6K
Summary
Ported the narrow public API portion of the upstream AgentSkillsProvider approval change from microsoft/agent-framework#6729 by adding exported skill-tool name constants plus reusable
skills.ReadOnlyToolsAutoApprovalRuleandskills.AllToolsAutoApprovalRulehelpers in Go. This also adds a focused skills auto-approval sample and updates the feature-comparison note.The upstream source of truth for this port is commit
e3b64fdc4749256fa2d559be18a41f1a008dd7f6. Go already matched the broader behavioral part of that .NET change because all three skill tools already required approval by default, so this PR keeps scope to the missing exported helper surface.Ported .NET PRs
e3b64fdc4749256fa2d559be18a41f1a008dd7f6Breaking Changes
No. Go already required approval for
load_skill,read_skill_resource, andrun_skill_scriptby default, so this PR only adds exported constants, reusable auto-approval helpers, tests, and a sample.Tests and Examples
go test ./agent/skills ./agent/harness/toolapproval ./examples/02-agents/skills/...examples/02-agents/skills/step04_skills_auto_approvalskills.ReadOnlyToolsAutoApprovalRuleandskills.AllToolsAutoApprovalRuleNotes
Closes #457