Skip to content

[dotnet-code] Clarify file script run internals#534

Open
michelle-clayton-work wants to merge 1 commit into
mainfrom
dotnet-code-file-script-runner-d69ce61f4bcf05e1
Open

[dotnet-code] Clarify file script run internals#534
michelle-clayton-work wants to merge 1 commit into
mainfrom
dotnet-code-file-script-runner-d69ce61f4bcf05e1

Conversation

@michelle-clayton-work

Copy link
Copy Markdown
Contributor

Summary

Extracted the file-backed script run closure into small unexported helpers. This keeps the Go file-skill script execution shape closer to the .NET AgentFileSkillScript.RunAsync guard sequence while preserving the existing public API and behavior.

.NET Reference

  • dotnet/src/Microsoft.Agents.AI/Skills/File/AgentFileSkillScript.cs - validates the owning file skill, checks for a configured runner, then invokes the file script runner.

Public API and Behavior

No public Go API changed. No intentional behavior change was made.

Tests

  • go test ./agent/skills/...
  • gofmt -w agent/skills/fsskills/source.go

Notes

Rejected candidates:

  • dotnet/src/Microsoft.Agents.AI/Skills/Programmatic/AgentInlineSkillResource.cs - Go inline skills are represented directly by skills.Skill values and test helpers rather than a corresponding internal resource type, so there was no narrow production cleanup to make.
  • dotnet/src/Microsoft.Agents.AI/Evaluation/ExpectedToolCall.cs - no matching Go evaluation surface was present in the sampled implementation, so changing Go would risk adding a missing feature rather than refactoring existing internals.
  • dotnet/src/Microsoft.Agents.AI.Workflows/ExternalResponse.cs - the close Go equivalent would require adding exported convenience methods to workflow.ExternalResponse, which would change the public Go API and violate the task constraints.

Open [dotnet-code] PRs were checked before editing; the existing workflow edge PR does not cover this file-skill script candidate.

Generated by .NET-to-Go Code Portability Refactoring Agent · 277 AIC · ⌖ 41.7 AIC · ⊞ 20.8K ·

Closes #499

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Refactors the file-backed script Run closure in fsskills into small unexported helpers to make the guard/validation sequence clearer and closer in shape to the referenced .NET implementation, without changing the public Go API.

Changes:

  • Extracted the inline Run closure into newFileScriptRunFunc(...).
  • Centralized the “owner must be a file-backed skill” validation into requireFileSkill(...).

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

@github-actions

Copy link
Copy Markdown
Contributor

API Consistency Review: ✅ No parity issues

This PR refactors unexported internals in agent/skills/fsskills/source.go — extracting a file script run closure into two unexported helpers (newFileScriptRunFunc, requireFileSkill). No exported Go identifiers, option shapes, or observable runtime behaviors changed.

Scope verdict: Out of scope for cross-repo consistency review. The change is a pure internal refactor that intentionally mirrors the guard-sequence structure of the .NET AgentFileSkillScript.RunAsync method without modifying the public Go contract.

Generated by Go API Consistency Review Agent for issue #534 · 28.6 AIC · ⌖ 11.7 AIC · ⊞ 28.4K ·

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[dotnet-code] Clarify file script run internals

2 participants