Skip to content

[dotnet-port-fixes] Harden file skill discovery - #833

Merged
Quim Muntal (qmuntal) merged 3 commits into
mainfrom
copilot/dotnet-port-fsskills-hardening-20260811-2d23946e974262e7
Aug 20, 2026
Merged

[dotnet-port-fixes] Harden file skill discovery#833
Quim Muntal (qmuntal) merged 3 commits into
mainfrom
copilot/dotnet-port-fsskills-hardening-20260811-2d23946e974262e7

Conversation

@michelle-clayton-work

Copy link
Copy Markdown
Contributor

Summary

Port the .NET file-skill discovery hardening from microsoft/agent-framework#7540 into the Go agent/skills/fsskills loader. The Go change now skips symlinked SKILL.md files, resources, and scripts during discovery while still allowing a caller-configured root path that is itself a symlink.

Ported .NET PRs

Breaking Changes

No. Exported Go APIs are unchanged; the loader only rejects unsafe file-skill paths that previously could be followed through symlinks.

Tests and Examples

  • Added parity tests for symlinked SKILL.md, resource, and script paths, plus configured-root symlink support
  • Ran go test ./agent/skills/fsskills

Notes

  • This PR keeps the scope to the file-skill symlink hardening that maps cleanly to the existing Go API surface.
  • The existing Go discovery logic already continues past unreadable directories without aborting sibling skill discovery, so no additional API-safe change was needed there.
  • Upstream commit: microsoft/agent-framework@94bbfb2

Generated by .NET to Go Fixes and Test Porting Agent · gpt54 · 215.2 AIC · ⌖ 13.2 AIC · ⊞ 24.2K ·

Closes #822

Port .NET file-skill discovery hardening so symlinked skill files, resources, and scripts are skipped while a symlinked configured root remains supported.

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

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

This PR ports the .NET “file-skill discovery hardening” behavior into the Go agent/skills/fsskills loader by adding symlink/inspection checks so discovery skips symlinked SKILL.md files, resources, and scripts while still allowing a caller-provided root that is itself a symlink.

Changes:

  • Hardened skill directory discovery to avoid treating symlinked SKILL.md as a valid skill marker.
  • Hardened resource/script discovery to skip symlinked (or uninspectable) entries during scan.
  • Added tests covering symlinked SKILL.md, resources, scripts, and symlinked configured-root behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
agent/skills/fsskills/source.go Adds symlink/inspection checks during skill, resource, and script discovery.
agent/skills/fsskills/source_test.go Adds tests for symlinked SKILL.md, symlinked configured root, and symlinked resources.
agent/skills/fsskills/source_script_test.go Adds a test ensuring symlinked scripts are skipped.

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

Comment thread agent/skills/fsskills/source.go
@github-actions

This comment has been minimized.

@github-actions github-actions Bot added the parity-approved Go API consistency review found no parity issues label Aug 12, 2026
@michelle-clayton-work

Copy link
Copy Markdown
Contributor Author

Copilot resolve the merge conflicts in this pull request

Copilot AI and others added 2 commits August 18, 2026 21:04
…sskills-hardening-20260811-2d23946e974262e7

# Conflicts:
#	agent/skills/fsskills/source.go
#	agent/skills/fsskills/source_script_test.go
#	agent/skills/fsskills/source_test.go

Co-authored-by: michelle-clayton-work <262183035+michelle-clayton-work@users.noreply.github.com>
Co-authored-by: michelle-clayton-work <262183035+michelle-clayton-work@users.noreply.github.com>

Copilot AI commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Copilot resolve the merge conflicts in this pull request

Resolved by merging main and keeping the more complete hardening implementation from this branch. Also fixed the earlier return-vs-continue bug flagged in review: an unsafe/symlinked SKILL.md now only stops the marker-check loop, so nested skill directories below it are still discovered (added regression test TestFileSource_SymlinkedSkillFile_DoesNotAbortNestedDiscovery).

@github-actions github-actions Bot added area:agent Changes files in the agent area size:large At most 300 changed lines across at most 10 files failed-auto-risk Automatic risk classification was inconclusive or failed labels Aug 18, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Parity Review: ✅ Approved

This PR ports symlink-hardening behavior from upstream .NET PR microsoft/agent-framework#7540 into the Go agent/skills/fsskills loader.

Scope: agent/skills/fsskills/source.go — internal discovery logic only. No exported Go APIs were added, removed, or changed.

Parity check:

  • The Go change now skips symlinked SKILL.md files, resource paths, and script paths during discovery, directly matching the .NET hardening behavior from commit 94bbfb2.
  • The caller-configured root being a symlink continues to work in both Go and .NET, preserving the existing opt-in flexibility.
  • No equivalent Python skill-file-discovery surface was found in python/packages/core/agent_framework/; this behavior appears .NET/Go-specific.

Conclusion: The change preserves cross-repo semantic parity with the .NET implementation. No public-api-change label is warranted. The parity-approved label correctly reflects the state of this PR.

Generated by Go API Consistency Review Agent for #833 · sonnet46 · 16.5 AIC · ⌖ 4.69 AIC · ⊞ 5.7K ·

@qmuntal
Quim Muntal (qmuntal) added this pull request to the merge queue Aug 20, 2026
Merged via the queue into main with commit 2d93075 Aug 20, 2026
37 checks passed
@qmuntal
Quim Muntal (qmuntal) deleted the copilot/dotnet-port-fsskills-hardening-20260811-2d23946e974262e7 branch August 20, 2026 06:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:agent Changes files in the agent area failed-auto-risk Automatic risk classification was inconclusive or failed parity-approved Go API consistency review found no parity issues size:large At most 300 changed lines across at most 10 files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[dotnet-port-fixes] Harden file skill discovery

5 participants