Merge branch 'main' into copilot/fix-flaky-workflow-test

This commit is contained in:
Jacob Alber
2026-05-14 10:55:38 -04:00
committed by GitHub
Unverified
7 changed files with 1760 additions and 165 deletions
+3 -3
View File
@@ -1,14 +1,14 @@
<Project>
<PropertyGroup>
<!-- Central version prefix - applies to all nuget packages. -->
<VersionPrefix>1.6.0</VersionPrefix>
<VersionPrefix>1.6.1</VersionPrefix>
<RCNumber>1</RCNumber>
<DateSuffix>260512</DateSuffix>
<DateSuffix>260514</DateSuffix>
<PackageVersion Condition="'$(IsReleaseCandidate)' == 'true'">$(VersionPrefix)-rc$(RCNumber)</PackageVersion>
<PackageVersion Condition="'$(IsReleaseCandidate)' != 'true' AND '$(VersionSuffix)' != ''">$(VersionPrefix)-$(VersionSuffix).$(DateSuffix).1</PackageVersion>
<PackageVersion Condition="'$(IsReleaseCandidate)' != 'true' AND '$(VersionSuffix)' == ''">$(VersionPrefix)-preview.$(DateSuffix).1</PackageVersion>
<PackageVersion Condition="'$(IsReleased)' == 'true'">$(VersionPrefix)</PackageVersion>
<GitTag>1.6.0</GitTag>
<GitTag>1.6.1</GitTag>
<Configurations>Debug;Release;Publish</Configurations>
<IsPackable>true</IsPackable>
@@ -122,6 +122,7 @@ public sealed class FileSystemAgentFileStore : AgentFileStore
}
var files = Directory.GetFiles(fullDir)
.Where(f => (File.GetAttributes(f) & FileAttributes.ReparsePoint) == 0)
.Select(Path.GetFileName)
.Where(name => name is not null)
.ToList();
@@ -157,6 +158,12 @@ public sealed class FileSystemAgentFileStore : AgentFileStore
foreach (string filePath in Directory.GetFiles(fullDir))
{
// Skip files that are symlinks/reparse points to prevent reading outside the root.
if ((File.GetAttributes(filePath) & FileAttributes.ReparsePoint) != 0)
{
continue;
}
string? fileName = Path.GetFileName(filePath);
if (fileName is null)
{
@@ -231,7 +238,7 @@ public sealed class FileSystemAgentFileStore : AgentFileStore
/// <summary>
/// Resolves a relative file path to a safe absolute path under the root directory.
/// Rejects paths that would escape the root via traversal or rooted paths.
/// Rejects paths that would escape the root via traversal, rooted paths, or symbolic links.
/// </summary>
private string ResolveSafePath(string relativePath)
{
@@ -250,9 +257,55 @@ public sealed class FileSystemAgentFileStore : AgentFileStore
nameof(relativePath));
}
// Reject symlinks/reparse points in any path segment to prevent escaping the root.
ThrowIfContainsSymlink(fullPath, this._rootPath);
return fullPath;
}
/// <summary>
/// Checks each path segment between the trusted root and the resolved path for symbolic links
/// or reparse points. Throws <see cref="ArgumentException"/> if any segment is a symlink.
/// Stops checking at the first segment that does not exist on disk (for write scenarios).
/// Uses <see cref="File.GetAttributes(string)"/> directly so that dangling symlinks (whose targets
/// do not exist) are still detected via their <see cref="FileAttributes.ReparsePoint"/> flag.
/// </summary>
private static void ThrowIfContainsSymlink(string fullPath, string rootPath)
{
string rootTrimmed = rootPath.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);
string relative = fullPath.Substring(rootTrimmed.Length);
string[] segments = relative.Split(
[Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar],
StringSplitOptions.RemoveEmptyEntries);
string current = rootTrimmed;
foreach (string segment in segments)
{
current = Path.Combine(current, segment);
FileAttributes attributes;
try
{
attributes = File.GetAttributes(current);
}
catch (FileNotFoundException)
{
// Segment does not exist on disk (write scenario); stop checking.
break;
}
catch (DirectoryNotFoundException)
{
break;
}
if ((attributes & FileAttributes.ReparsePoint) != 0)
{
throw new ArgumentException(
"Invalid path: the resolved path contains a symbolic link or reparse point.");
}
}
}
/// <summary>
/// Resolves a relative directory path to a safe absolute path under the root directory.
/// An empty string resolves to the root directory itself.
@@ -218,7 +218,7 @@ public class DevUIIntegrationTests
Assert.Contains(discoveryResponse.Entities, e => e.Name == "default-workflow" && e.Type == "workflow");
}
[Fact]
[Fact(Skip = "Flaky in merge_group; see https://github.com/microsoft/agent-framework/issues/5845")]
public async Task TestServerWithDevUI_ResolvesMixedAgentsAndWorkflows_AllRegistrationsAsync()
{
// Arrange
@@ -334,4 +334,475 @@ public sealed class FileSystemAgentFileStoreTests : IDisposable
}
#endregion
#region Symlink Escape Rejection
#if NET
/// <summary>
/// Attempts to create a file symlink. Returns false if the platform does not support
/// symlink creation (e.g., Windows without developer mode) or if creation fails.
/// </summary>
private static bool TryCreateFileSymbolicLink(string linkPath, string targetPath)
{
try
{
File.CreateSymbolicLink(linkPath, targetPath);
}
catch (IOException)
{
return false;
}
// Verify the symlink was actually created as a reparse point.
return File.Exists(linkPath)
&& (File.GetAttributes(linkPath) & FileAttributes.ReparsePoint) != 0;
}
/// <summary>
/// Attempts to create a directory symlink. Returns false if the platform does not support
/// symlink creation (e.g., Windows without developer mode) or if creation fails.
/// </summary>
private static bool TryCreateDirectorySymbolicLink(string linkPath, string targetPath)
{
try
{
Directory.CreateSymbolicLink(linkPath, targetPath);
}
catch (IOException)
{
return false;
}
// Verify the symlink was actually created as a reparse point.
return Directory.Exists(linkPath)
&& (File.GetAttributes(linkPath) & FileAttributes.ReparsePoint) != 0;
}
[Fact]
public async Task ReadFileAsync_SymlinkedFile_ThrowsAsync()
{
// Arrange — create a file outside the root and symlink to it from inside.
string outsideFile = Path.Combine(Path.GetTempPath(), "symlink_target_read_" + Guid.NewGuid().ToString("N") + ".txt");
File.WriteAllText(outsideFile, "SECRET_OUTSIDE_ROOT");
string linkPath = Path.Combine(this._rootDir, "leak.txt");
try
{
if (!TryCreateFileSymbolicLink(linkPath, outsideFile))
{
return; // Cannot create symlinks in this environment; skip.
}
// Act & Assert — reading through the symlink should be rejected.
await Assert.ThrowsAsync<ArgumentException>(() => this._store.ReadFileAsync("leak.txt"));
}
finally
{
if (File.Exists(linkPath))
{
File.Delete(linkPath);
}
File.Delete(outsideFile);
}
}
[Fact]
public async Task WriteFileAsync_SymlinkedFile_ThrowsAsync()
{
// Arrange — create a file outside the root and symlink to it from inside.
string outsideFile = Path.Combine(Path.GetTempPath(), "symlink_target_write_" + Guid.NewGuid().ToString("N") + ".txt");
File.WriteAllText(outsideFile, "ORIGINAL_CONTENT");
string linkPath = Path.Combine(this._rootDir, "overwrite.txt");
try
{
if (!TryCreateFileSymbolicLink(linkPath, outsideFile))
{
return;
}
// Act & Assert — writing through the symlink should be rejected.
await Assert.ThrowsAsync<ArgumentException>(() => this._store.WriteFileAsync("overwrite.txt", "EVIL_CONTENT"));
// Verify the outside file was NOT modified.
Assert.Equal("ORIGINAL_CONTENT", await File.ReadAllTextAsync(outsideFile));
}
finally
{
if (File.Exists(linkPath))
{
File.Delete(linkPath);
}
File.Delete(outsideFile);
}
}
[Fact]
public async Task DeleteFileAsync_SymlinkedFile_ThrowsAsync()
{
// Arrange
string outsideFile = Path.Combine(Path.GetTempPath(), "symlink_target_delete_" + Guid.NewGuid().ToString("N") + ".txt");
File.WriteAllText(outsideFile, "DO_NOT_DELETE");
string linkPath = Path.Combine(this._rootDir, "trap.txt");
try
{
if (!TryCreateFileSymbolicLink(linkPath, outsideFile))
{
return;
}
// Act & Assert
await Assert.ThrowsAsync<ArgumentException>(() => this._store.DeleteFileAsync("trap.txt"));
// Verify the outside file still exists.
Assert.True(File.Exists(outsideFile));
}
finally
{
if (File.Exists(linkPath))
{
File.Delete(linkPath);
}
File.Delete(outsideFile);
}
}
[Fact]
public async Task FileExistsAsync_SymlinkedFile_ThrowsAsync()
{
// Arrange
string outsideFile = Path.Combine(Path.GetTempPath(), "symlink_target_exists_" + Guid.NewGuid().ToString("N") + ".txt");
File.WriteAllText(outsideFile, "EXISTS_OUTSIDE");
string linkPath = Path.Combine(this._rootDir, "phantom.txt");
try
{
if (!TryCreateFileSymbolicLink(linkPath, outsideFile))
{
return;
}
// Act & Assert
await Assert.ThrowsAsync<ArgumentException>(() => this._store.FileExistsAsync("phantom.txt"));
}
finally
{
if (File.Exists(linkPath))
{
File.Delete(linkPath);
}
File.Delete(outsideFile);
}
}
[Fact]
public async Task WriteFileAsync_DanglingSymlink_ThrowsAsync()
{
// Arrange — create a symlink pointing to a non-existent target.
string nonExistentTarget = Path.Combine(Path.GetTempPath(), "dangling_target_" + Guid.NewGuid().ToString("N") + ".txt");
string linkPath = Path.Combine(this._rootDir, "dangling.txt");
try
{
if (!TryCreateFileSymbolicLink(linkPath, nonExistentTarget))
{
return;
}
// Act & Assert — even a dangling symlink must be rejected.
await Assert.ThrowsAsync<ArgumentException>(() => this._store.WriteFileAsync("dangling.txt", "CONTENT"));
// Verify the target was NOT created by following the dangling link.
Assert.False(File.Exists(nonExistentTarget));
}
finally
{
// Dangling symlinks: File.Exists returns false, but the link entry still exists.
// Use FileInfo to delete the link itself.
var linkInfo = new FileInfo(linkPath);
if (linkInfo.Exists || (linkInfo.Attributes & FileAttributes.ReparsePoint) != 0)
{
linkInfo.Delete();
}
}
}
[Fact]
public async Task ListFilesAsync_SymlinkedDirectory_ThrowsAsync()
{
// Arrange — create a directory outside root and symlink a directory inside root to it.
string outsideDir = Path.Combine(Path.GetTempPath(), "symlink_dir_target_" + Guid.NewGuid().ToString("N"));
Directory.CreateDirectory(outsideDir);
File.WriteAllText(Path.Combine(outsideDir, "secret.txt"), "SECRET");
string linkDir = Path.Combine(this._rootDir, "linked-dir");
try
{
if (!TryCreateDirectorySymbolicLink(linkDir, outsideDir))
{
return;
}
// Act & Assert
await Assert.ThrowsAsync<ArgumentException>(() => this._store.ListFilesAsync("linked-dir"));
}
finally
{
if (Directory.Exists(linkDir))
{
Directory.Delete(linkDir);
}
Directory.Delete(outsideDir, recursive: true);
}
}
[Fact]
public async Task SearchFilesAsync_SymlinkedDirectory_ThrowsAsync()
{
// Arrange
string outsideDir = Path.Combine(Path.GetTempPath(), "symlink_search_target_" + Guid.NewGuid().ToString("N"));
Directory.CreateDirectory(outsideDir);
File.WriteAllText(Path.Combine(outsideDir, "data.txt"), "SENSITIVE_DATA");
string linkDir = Path.Combine(this._rootDir, "search-link");
try
{
if (!TryCreateDirectorySymbolicLink(linkDir, outsideDir))
{
return;
}
// Act & Assert
await Assert.ThrowsAsync<ArgumentException>(() => this._store.SearchFilesAsync("search-link", "SENSITIVE"));
}
finally
{
if (Directory.Exists(linkDir))
{
Directory.Delete(linkDir);
}
Directory.Delete(outsideDir, recursive: true);
}
}
[Fact]
public async Task ReadFileAsync_ThroughDirectorySymlink_ThrowsAsync()
{
// Arrange — directory symlink inside root pointing outside; read a file through it.
string outsideDir = Path.Combine(Path.GetTempPath(), "symlink_dir_read_" + Guid.NewGuid().ToString("N"));
Directory.CreateDirectory(outsideDir);
File.WriteAllText(Path.Combine(outsideDir, "secret.txt"), "DIR_SYMLINK_SECRET");
string linkDir = Path.Combine(this._rootDir, "linked-output");
try
{
if (!TryCreateDirectorySymbolicLink(linkDir, outsideDir))
{
return;
}
// Act & Assert — reading through a directory symlink should be rejected.
await Assert.ThrowsAsync<ArgumentException>(() => this._store.ReadFileAsync("linked-output/secret.txt"));
}
finally
{
if (Directory.Exists(linkDir))
{
Directory.Delete(linkDir);
}
Directory.Delete(outsideDir, recursive: true);
}
}
[Fact]
public async Task WriteFileAsync_ThroughDirectorySymlink_ThrowsAsync()
{
// Arrange — directory symlink; attempt to create/overwrite a file through it.
string outsideDir = Path.Combine(Path.GetTempPath(), "symlink_dir_write_" + Guid.NewGuid().ToString("N"));
Directory.CreateDirectory(outsideDir);
string linkDir = Path.Combine(this._rootDir, "linked-output");
try
{
if (!TryCreateDirectorySymbolicLink(linkDir, outsideDir))
{
return;
}
// Act & Assert
await Assert.ThrowsAsync<ArgumentException>(() => this._store.WriteFileAsync("linked-output/created-by-agent.txt", "CONTENT"));
// Verify no file was created outside.
Assert.False(File.Exists(Path.Combine(outsideDir, "created-by-agent.txt")));
}
finally
{
if (Directory.Exists(linkDir))
{
Directory.Delete(linkDir);
}
Directory.Delete(outsideDir, recursive: true);
}
}
[Fact]
public async Task DeleteFileAsync_ThroughDirectorySymlink_ThrowsAsync()
{
// Arrange — directory symlink; attempt to delete a file through it.
string outsideDir = Path.Combine(Path.GetTempPath(), "symlink_dir_delete_" + Guid.NewGuid().ToString("N"));
Directory.CreateDirectory(outsideDir);
string outsideFile = Path.Combine(outsideDir, "delete-me.txt");
File.WriteAllText(outsideFile, "DO_NOT_DELETE");
string linkDir = Path.Combine(this._rootDir, "linked-output");
try
{
if (!TryCreateDirectorySymbolicLink(linkDir, outsideDir))
{
return;
}
// Act & Assert
await Assert.ThrowsAsync<ArgumentException>(() => this._store.DeleteFileAsync("linked-output/delete-me.txt"));
// Verify the outside file was NOT deleted.
Assert.True(File.Exists(outsideFile));
}
finally
{
if (Directory.Exists(linkDir))
{
Directory.Delete(linkDir);
}
Directory.Delete(outsideDir, recursive: true);
}
}
[Fact]
public async Task CreateDirectoryAsync_ThroughDirectorySymlink_ThrowsAsync()
{
// Arrange — directory symlink; attempt to create a subdirectory through it.
string outsideDir = Path.Combine(Path.GetTempPath(), "symlink_dir_mkdir_" + Guid.NewGuid().ToString("N"));
Directory.CreateDirectory(outsideDir);
string linkDir = Path.Combine(this._rootDir, "linked-output");
try
{
if (!TryCreateDirectorySymbolicLink(linkDir, outsideDir))
{
return;
}
// Act & Assert
await Assert.ThrowsAsync<ArgumentException>(() => this._store.CreateDirectoryAsync("linked-output/created-directory"));
// Verify no directory was created outside.
Assert.False(Directory.Exists(Path.Combine(outsideDir, "created-directory")));
}
finally
{
if (Directory.Exists(linkDir))
{
Directory.Delete(linkDir);
}
Directory.Delete(outsideDir, recursive: true);
}
}
[Fact]
public async Task SearchFilesAsync_RootWithSymlinkedFile_DoesNotLeakContentAsync()
{
// Arrange — symlinked file at root level; search should not return its content.
string outsideFile = Path.Combine(Path.GetTempPath(), "symlink_search_root_" + Guid.NewGuid().ToString("N") + ".txt");
File.WriteAllText(outsideFile, "ROOT_LEVEL_SECRET_CONTENT");
string linkPath = Path.Combine(this._rootDir, "env-link.txt");
try
{
if (!TryCreateFileSymbolicLink(linkPath, outsideFile))
{
return;
}
// Also add a normal file to confirm search still works for non-symlinks.
await this._store.WriteFileAsync("normal.txt", "NORMAL_CONTENT");
// Act — search at root should skip the symlinked file.
var results = await this._store.SearchFilesAsync("", "SECRET_CONTENT");
// Assert — no results from the symlinked file.
Assert.Empty(results);
}
finally
{
if (File.Exists(linkPath))
{
File.Delete(linkPath);
}
File.Delete(outsideFile);
}
}
[Fact]
public async Task ListFilesAsync_RootWithSymlinkedFile_ExcludesSymlinkAsync()
{
// Arrange — symlinked file at root level; listing should not include it.
string outsideFile = Path.Combine(Path.GetTempPath(), "symlink_list_root_" + Guid.NewGuid().ToString("N") + ".txt");
File.WriteAllText(outsideFile, "OUTSIDE");
string linkPath = Path.Combine(this._rootDir, "hidden-link.txt");
try
{
if (!TryCreateFileSymbolicLink(linkPath, outsideFile))
{
return;
}
// Also add a normal file.
await this._store.WriteFileAsync("visible.txt", "VISIBLE");
// Act
var files = await this._store.ListFilesAsync("");
// Assert — symlinked file should not appear in listing.
Assert.DoesNotContain("hidden-link.txt", files);
Assert.Contains("visible.txt", files);
}
finally
{
if (File.Exists(linkPath))
{
File.Delete(linkPath);
}
File.Delete(outsideFile);
}
}
#endif
#endregion
}
@@ -0,0 +1,546 @@
// Copyright (c) Microsoft. All rights reserved.
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using FluentAssertions;
using Microsoft.Agents.AI.Workflows.Execution;
namespace Microsoft.Agents.AI.Workflows.UnitTests;
public sealed class RouteBuilderTests
{
public enum HandlerOverload
{
SyncWithCancellation = 0,
SyncWithoutCancellation = 1,
AsyncWithCancellation = 2,
AsyncWithoutCancellation = 3,
}
private sealed record TestPayload(string Value);
private sealed class HandlerInvocation
{
public object? Message { get; private set; }
public IWorkflowContext? Context { get; private set; }
public CancellationToken CancellationToken { get; private set; }
public int InvocationCount { get; private set; }
public void Capture(object? message, IWorkflowContext context, CancellationToken cancellationToken = default)
{
this.Message = message;
this.Context = context;
this.CancellationToken = cancellationToken;
this.InvocationCount++;
}
}
private sealed class TestExternalRequestContext : IExternalRequestContext, IExternalRequestSink
{
public List<RequestPort> RegisteredPorts { get; } = [];
public List<ExternalRequest> PostedRequests { get; } = [];
public IExternalRequestSink RegisterPort(RequestPort port)
{
this.RegisteredPorts.Add(port);
return this;
}
public ValueTask PostAsync(ExternalRequest request)
{
this.PostedRequests.Add(request);
return default;
}
}
[Theory]
[InlineData(HandlerOverload.SyncWithCancellation)]
[InlineData(HandlerOverload.SyncWithoutCancellation)]
[InlineData(HandlerOverload.AsyncWithCancellation)]
[InlineData(HandlerOverload.AsyncWithoutCancellation)]
public async Task AddHandler_VoidOverloads_RouteExpectedMessageAsync(HandlerOverload overload)
{
// Arrange
RouteBuilder routeBuilder = new(null);
HandlerInvocation invocation = new();
CancellationToken cancellationToken = new CancellationTokenSource().Token;
RegisterVoidHandler(routeBuilder, invocation, overload);
MessageRouter router = routeBuilder.Build();
TestWorkflowContext context = new("executor");
// Act
CallResult? result = await router.RouteMessageAsync("hello", context, cancellationToken: cancellationToken);
// Assert
result.Should().NotBeNull();
result!.IsSuccess.Should().BeTrue();
result.IsVoid.Should().BeTrue();
result.Result.Should().BeNull();
invocation.InvocationCount.Should().Be(1);
invocation.Message.Should().Be("hello");
invocation.Context.Should().BeSameAs(context);
if (UsesCancellationToken(overload))
{
invocation.CancellationToken.Should().Be(cancellationToken);
}
}
[Theory]
[InlineData(HandlerOverload.SyncWithCancellation)]
[InlineData(HandlerOverload.SyncWithoutCancellation)]
[InlineData(HandlerOverload.AsyncWithCancellation)]
[InlineData(HandlerOverload.AsyncWithoutCancellation)]
public async Task AddHandler_ResultOverloads_RouteExpectedMessageAsync(HandlerOverload overload)
{
// Arrange
RouteBuilder routeBuilder = new(null);
HandlerInvocation invocation = new();
CancellationToken cancellationToken = new CancellationTokenSource().Token;
RegisterResultHandler(routeBuilder, invocation, overload);
MessageRouter router = routeBuilder.Build();
TestWorkflowContext context = new("executor");
// Act
CallResult? result = await router.RouteMessageAsync("hello", context, cancellationToken: cancellationToken);
// Assert
result.Should().NotBeNull();
result!.IsSuccess.Should().BeTrue();
result.IsVoid.Should().BeFalse();
result.Result.Should().Be("HELLO");
router.DefaultOutputTypes.Should().Contain(typeof(string));
invocation.InvocationCount.Should().Be(1);
invocation.Message.Should().Be("hello");
invocation.Context.Should().BeSameAs(context);
if (UsesCancellationToken(overload))
{
invocation.CancellationToken.Should().Be(cancellationToken);
}
}
[Theory]
[InlineData(HandlerOverload.SyncWithCancellation)]
[InlineData(HandlerOverload.SyncWithoutCancellation)]
[InlineData(HandlerOverload.AsyncWithCancellation)]
[InlineData(HandlerOverload.AsyncWithoutCancellation)]
public async Task AddCatchAll_VoidOverloads_RouteUnexpectedMessageAsync(HandlerOverload overload)
{
// Arrange
RouteBuilder routeBuilder = new(null);
HandlerInvocation invocation = new();
CancellationToken cancellationToken = new CancellationTokenSource().Token;
TestPayload payload = new("hello");
RegisterVoidCatchAll(routeBuilder, invocation, overload);
MessageRouter router = routeBuilder.Build();
TestWorkflowContext context = new("executor");
// Act
CallResult? result = await router.RouteMessageAsync(payload, context, cancellationToken: cancellationToken);
// Assert
result.Should().NotBeNull();
result!.IsSuccess.Should().BeTrue();
result.IsVoid.Should().BeTrue();
result.Result.Should().BeNull();
invocation.InvocationCount.Should().Be(1);
invocation.Message.Should().BeEquivalentTo(new PortableValue(payload));
invocation.Context.Should().BeSameAs(context);
if (UsesCancellationToken(overload))
{
invocation.CancellationToken.Should().Be(cancellationToken);
}
}
[Theory]
[InlineData(HandlerOverload.SyncWithCancellation)]
[InlineData(HandlerOverload.SyncWithoutCancellation)]
[InlineData(HandlerOverload.AsyncWithCancellation)]
[InlineData(HandlerOverload.AsyncWithoutCancellation)]
public async Task AddCatchAll_ResultOverloads_RouteUnexpectedMessageAsync(HandlerOverload overload)
{
// Arrange
RouteBuilder routeBuilder = new(null);
HandlerInvocation invocation = new();
CancellationToken cancellationToken = new CancellationTokenSource().Token;
TestPayload payload = new("hello");
RegisterResultCatchAll(routeBuilder, invocation, overload);
MessageRouter router = routeBuilder.Build();
TestWorkflowContext context = new("executor");
// Act
CallResult? result = await router.RouteMessageAsync(payload, context, cancellationToken: cancellationToken);
// Assert
result.Should().NotBeNull();
result!.IsSuccess.Should().BeTrue();
result.IsVoid.Should().BeFalse();
result.Result.Should().Be("HELLO");
invocation.InvocationCount.Should().Be(1);
invocation.Message.Should().BeEquivalentTo(new PortableValue(payload));
invocation.Context.Should().BeSameAs(context);
if (UsesCancellationToken(overload))
{
invocation.CancellationToken.Should().Be(cancellationToken);
}
}
[Fact]
public async Task AddHandlerUntyped_VoidAndResultOverloads_RouteExpectedMessageAsync()
{
// Arrange
RouteBuilder routeBuilder = new(null);
HandlerInvocation voidInvocation = new();
HandlerInvocation resultInvocation = new();
CancellationToken cancellationToken = new CancellationTokenSource().Token;
routeBuilder.AddHandlerUntyped(typeof(string), (message, context, token) =>
{
voidInvocation.Capture(message, context, token);
return default;
});
routeBuilder.AddHandlerUntyped<int>(typeof(int), (message, context, token) =>
{
resultInvocation.Capture(message, context, token);
return new((int)message + 1);
});
MessageRouter router = routeBuilder.Build();
TestWorkflowContext context = new("executor");
// Act
CallResult? voidResult = await router.RouteMessageAsync("hello", context, cancellationToken: cancellationToken);
CallResult? typedResult = await router.RouteMessageAsync(41, context, cancellationToken: cancellationToken);
// Assert
voidResult.Should().NotBeNull();
voidResult!.IsVoid.Should().BeTrue();
voidInvocation.Message.Should().Be("hello");
voidInvocation.Context.Should().BeSameAs(context);
voidInvocation.CancellationToken.Should().Be(cancellationToken);
typedResult.Should().NotBeNull();
typedResult!.Result.Should().Be(42);
router.DefaultOutputTypes.Should().Contain(typeof(int));
resultInvocation.Message.Should().Be(41);
resultInvocation.Context.Should().BeSameAs(context);
resultInvocation.CancellationToken.Should().Be(cancellationToken);
}
[Fact]
public void AddHandler_ForPortableValue_ThrowsInvalidOperationException()
{
// Arrange
RouteBuilder routeBuilder = new(null);
// Act
Action act = () => routeBuilder.AddHandler<PortableValue>((message, context) => { });
// Assert
act.Should().Throw<InvalidOperationException>()
.WithMessage("*Use AddCatchAll()*");
}
[Fact]
public void AddHandler_DuplicateRegistrationWithoutOverwrite_ThrowsArgumentException()
{
// Arrange
RouteBuilder routeBuilder = new(null);
routeBuilder.AddHandler<string>((message, context) => { });
// Act
Action act = () => routeBuilder.AddHandler<string>((message, context) => { });
// Assert
act.Should().Throw<ArgumentException>()
.WithMessage("*already registered*");
}
[Fact]
public void AddHandler_OverwriteWithoutExistingRegistration_ThrowsArgumentException()
{
// Arrange
RouteBuilder routeBuilder = new(null);
// Act
Action act = () => routeBuilder.AddHandler<string>((message, context) => { }, overwrite: true);
// Assert
act.Should().Throw<ArgumentException>()
.WithMessage("*has not yet been registered*");
}
[Fact]
public async Task AddHandler_OverwriteExistingRegistration_RoutesUpdatedHandlerAsync()
{
// Arrange
RouteBuilder routeBuilder = new(null);
routeBuilder.AddHandler<string>((message, context) => context.SendMessageAsync("first"));
routeBuilder.AddHandler<string>((message, context) => context.SendMessageAsync("second"), overwrite: true);
MessageRouter router = routeBuilder.Build();
TestWorkflowContext context = new("executor");
// Act
_ = await router.RouteMessageAsync("hello", context);
// Assert
context.SentMessages.Should().ContainSingle().Which.Should().Be("second");
}
[Fact]
public void AddCatchAll_DuplicateRegistrationWithoutOverwrite_ThrowsInvalidOperationException()
{
// Arrange
RouteBuilder routeBuilder = new(null);
routeBuilder.AddCatchAll((message, context) => { });
// Act
Action act = () => routeBuilder.AddCatchAll((message, context) => { });
// Assert
act.Should().Throw<InvalidOperationException>()
.WithMessage("*already registered*");
}
[Fact]
public async Task AddCatchAll_OverwriteExistingRegistration_RoutesUpdatedHandlerAsync()
{
// Arrange
RouteBuilder routeBuilder = new(null);
routeBuilder.AddCatchAll((message, context) => context.SendMessageAsync("first"));
routeBuilder.AddCatchAll((message, context) => context.SendMessageAsync("second"), overwrite: true);
MessageRouter router = routeBuilder.Build();
TestWorkflowContext context = new("executor");
// Act
_ = await router.RouteMessageAsync(new TestPayload("hello"), context);
// Assert
context.SentMessages.Should().ContainSingle().Which.Should().Be("second");
}
[Fact]
public void AddPortHandler_WithoutExternalRequestContext_ThrowsInvalidOperationException()
{
// Arrange
RouteBuilder routeBuilder = new(null);
// Act
Action act = () => routeBuilder.AddPortHandler<string, int>("port", (response, context, cancellationToken) => default, out _);
// Assert
act.Should().Throw<InvalidOperationException>()
.WithMessage("*external request context is required*");
}
[Fact]
public async Task AddPortHandler_RoutesMatchingExternalResponseAsync()
{
// Arrange
TestExternalRequestContext externalRequestContext = new();
RouteBuilder routeBuilder = new(externalRequestContext);
HandlerInvocation invocation = new();
routeBuilder.AddPortHandler<string, int>("port", (response, context, cancellationToken) =>
{
invocation.Capture(response, context, cancellationToken);
return default;
}, out PortBinding portBinding);
await portBinding.PostRequestAsync("request", requestId: "req-1");
MessageRouter router = routeBuilder.Build();
TestWorkflowContext context = new("executor");
CancellationToken cancellationToken = new CancellationTokenSource().Token;
ExternalResponse response = externalRequestContext.PostedRequests.Single().CreateResponse(42);
// Act
CallResult? result = await router.RouteMessageAsync(response, context, cancellationToken: cancellationToken);
// Assert
externalRequestContext.RegisteredPorts.Should().ContainSingle(port => port.Id == "port");
externalRequestContext.PostedRequests.Should().ContainSingle(request => request.RequestId == "req-1");
result.Should().NotBeNull();
result!.IsSuccess.Should().BeTrue();
result.Result.Should().BeSameAs(response);
invocation.InvocationCount.Should().Be(1);
invocation.Message.Should().Be(42);
invocation.Context.Should().BeSameAs(context);
invocation.CancellationToken.Should().Be(cancellationToken);
}
[Fact]
public async Task AddPortHandler_UnknownPort_ReturnsExceptionResultAsync()
{
// Arrange
TestExternalRequestContext externalRequestContext = new();
RouteBuilder routeBuilder = new(externalRequestContext);
routeBuilder.AddPortHandler<string, int>("port", (response, context, cancellationToken) => default, out _);
MessageRouter router = routeBuilder.Build();
ExternalRequest request = ExternalRequest.Create(RequestPort.Create<string, int>("other"), "request", requestId: "req-1");
// Act
CallResult? result = await router.RouteMessageAsync(request.CreateResponse(42), new TestWorkflowContext("executor"));
// Assert
result.Should().NotBeNull();
result!.IsSuccess.Should().BeFalse();
result.Exception.Should().BeOfType<InvalidOperationException>();
result.Exception!.Message.Should().Contain("Unknown port");
}
private static void RegisterVoidHandler(RouteBuilder routeBuilder, HandlerInvocation invocation, HandlerOverload overload)
{
switch (overload)
{
case HandlerOverload.SyncWithCancellation:
routeBuilder.AddHandler<string>((message, context, cancellationToken) => invocation.Capture(message, context, cancellationToken));
break;
case HandlerOverload.SyncWithoutCancellation:
routeBuilder.AddHandler<string>((message, context) => invocation.Capture(message, context));
break;
case HandlerOverload.AsyncWithCancellation:
routeBuilder.AddHandler<string>((message, context, cancellationToken) =>
{
invocation.Capture(message, context, cancellationToken);
return default;
});
break;
case HandlerOverload.AsyncWithoutCancellation:
routeBuilder.AddHandler<string>((message, context) =>
{
invocation.Capture(message, context);
return default;
});
break;
default:
throw new ArgumentOutOfRangeException(nameof(overload));
}
}
private static void RegisterResultHandler(RouteBuilder routeBuilder, HandlerInvocation invocation, HandlerOverload overload)
{
switch (overload)
{
case HandlerOverload.SyncWithCancellation:
routeBuilder.AddHandler<string, string>((message, context, cancellationToken) =>
{
invocation.Capture(message, context, cancellationToken);
return NormalizeHandlerResult(message);
});
break;
case HandlerOverload.SyncWithoutCancellation:
routeBuilder.AddHandler<string, string>((message, context) =>
{
invocation.Capture(message, context);
return NormalizeHandlerResult(message);
});
break;
case HandlerOverload.AsyncWithCancellation:
Func<string, IWorkflowContext, CancellationToken, ValueTask<string>> asyncHandlerWithCancellation = (message, context, cancellationToken) =>
{
invocation.Capture(message, context, cancellationToken);
return new ValueTask<string>(NormalizeHandlerResult(message));
};
routeBuilder.AddHandler(asyncHandlerWithCancellation);
break;
case HandlerOverload.AsyncWithoutCancellation:
Func<string, IWorkflowContext, ValueTask<string>> asyncHandler = (message, context) =>
{
invocation.Capture(message, context);
return new ValueTask<string>(NormalizeHandlerResult(message));
};
routeBuilder.AddHandler(asyncHandler);
break;
default:
throw new ArgumentOutOfRangeException(nameof(overload));
}
}
private static void RegisterVoidCatchAll(RouteBuilder routeBuilder, HandlerInvocation invocation, HandlerOverload overload)
{
switch (overload)
{
case HandlerOverload.SyncWithCancellation:
routeBuilder.AddCatchAll((message, context, cancellationToken) => invocation.Capture(message, context, cancellationToken));
break;
case HandlerOverload.SyncWithoutCancellation:
routeBuilder.AddCatchAll((message, context) => invocation.Capture(message, context));
break;
case HandlerOverload.AsyncWithCancellation:
routeBuilder.AddCatchAll((message, context, cancellationToken) =>
{
invocation.Capture(message, context, cancellationToken);
return default;
});
break;
case HandlerOverload.AsyncWithoutCancellation:
routeBuilder.AddCatchAll((message, context) =>
{
invocation.Capture(message, context);
return default;
});
break;
default:
throw new ArgumentOutOfRangeException(nameof(overload));
}
}
private static void RegisterResultCatchAll(RouteBuilder routeBuilder, HandlerInvocation invocation, HandlerOverload overload)
{
switch (overload)
{
case HandlerOverload.SyncWithCancellation:
routeBuilder.AddCatchAll((message, context, cancellationToken) =>
{
invocation.Capture(message, context, cancellationToken);
return NormalizeCatchAllResult(message);
});
break;
case HandlerOverload.SyncWithoutCancellation:
routeBuilder.AddCatchAll((message, context) =>
{
invocation.Capture(message, context);
return NormalizeCatchAllResult(message);
});
break;
case HandlerOverload.AsyncWithCancellation:
Func<PortableValue, IWorkflowContext, CancellationToken, ValueTask<string>> asyncCatchAllWithCancellation = (message, context, cancellationToken) =>
{
invocation.Capture(message, context, cancellationToken);
return new ValueTask<string>(NormalizeCatchAllResult(message));
};
routeBuilder.AddCatchAll(asyncCatchAllWithCancellation);
break;
case HandlerOverload.AsyncWithoutCancellation:
Func<PortableValue, IWorkflowContext, ValueTask<string>> asyncCatchAll = (message, context) =>
{
invocation.Capture(message, context);
return new ValueTask<string>(NormalizeCatchAllResult(message));
};
routeBuilder.AddCatchAll(asyncCatchAll);
break;
default:
throw new ArgumentOutOfRangeException(nameof(overload));
}
}
private static bool UsesCancellationToken(HandlerOverload overload) =>
overload is HandlerOverload.SyncWithCancellation or HandlerOverload.AsyncWithCancellation;
private static string NormalizeHandlerResult(string message) => message.ToUpperInvariant();
private static string NormalizeCatchAllResult(PortableValue message) => GetPayloadValue(message).ToUpperInvariant();
private static string GetPayloadValue(PortableValue message)
{
return message.As<TestPayload>() is TestPayload payload
? payload.Value
: throw new InvalidOperationException("Expected catch-all message payload to deserialize as TestPayload.");
}
}
+269 -53
View File
@@ -1430,6 +1430,13 @@ DEFAULT_RESOURCE_EXTENSIONS: Final[tuple[str, ...]] = (
)
DEFAULT_SCRIPT_EXTENSIONS: Final[tuple[str, ...]] = (".py",)
# "." means the skill directory root itself (files directly in the skill folder).
ROOT_DIRECTORY_INDICATOR: Final[str] = "."
# Standard subdirectory names per https://agentskills.io/specification#directory-structure
DEFAULT_RESOURCE_DIRECTORIES: Final[tuple[str, ...]] = ("references", "assets")
DEFAULT_SCRIPT_DIRECTORIES: Final[tuple[str, ...]] = ("scripts",)
# region Patterns and prompt template
# Matches YAML frontmatter delimited by "---" lines.
@@ -1650,6 +1657,8 @@ class SkillsProvider(ContextProvider):
script_runner: SkillScriptRunner | None = None,
resource_extensions: tuple[str, ...] | None = None,
script_extensions: tuple[str, ...] | None = None,
resource_directories: Sequence[str] | None = None,
script_directories: Sequence[str] | None = None,
instruction_template: str | None = None,
require_script_approval: bool = False,
disable_caching: bool = False,
@@ -1672,6 +1681,15 @@ class SkillsProvider(ContextProvider):
``(".md", ".json", ".yaml", ".yml", ".csv", ".xml", ".txt")``.
script_extensions: File extensions recognized as discoverable
scripts. Defaults to ``(".py",)``.
resource_directories: Relative directory paths to scan for
resource files within each skill directory. Use ``"."``
to include files at the skill root level. Defaults to
``("references", "assets")`` per the agentskills.io
specification.
script_directories: Relative directory paths to scan for
script files within each skill directory. Use ``"."``
to include files at the skill root level. Defaults to
``("scripts",)`` per the agentskills.io specification.
instruction_template: Custom system-prompt template for
advertising skills. Must contain a ``{skills}`` placeholder.
Uses a built-in template when ``None``.
@@ -1701,6 +1719,8 @@ class SkillsProvider(ContextProvider):
script_runner=script_runner,
resource_extensions=resource_extensions,
script_extensions=script_extensions,
resource_directories=resource_directories,
script_directories=script_directories,
)
)
return cls(
@@ -2186,7 +2206,15 @@ class FileSkillsSource(SkillsSource):
Recursively scans the configured *skill_paths* directories for
``SKILL.md`` files (up to 2 levels deep), parses their YAML frontmatter,
and discovers associated resource and script files from subdirectories.
and discovers associated resource and script files from spec-defined
subdirectories.
By default, resources are discovered from ``references/`` and ``assets/``
subdirectories, and scripts from ``scripts/``, per the
`agentskills.io specification
<https://agentskills.io/specification>`_. Use *resource_directories*
and *script_directories* to customize which subdirectories are scanned.
Pass ``"."`` to include files at the skill root level.
Security: file-based metadata is XML-escaped before prompt injection,
and resource reads are guarded against path traversal and symlink escape.
@@ -2200,14 +2228,15 @@ class FileSkillsSource(SkillsSource):
source = FileSkillsSource(skill_paths="./skills")
skills = await source.get_skills()
With a script runner and custom extensions:
With a script runner and custom directories:
.. code-block:: python
source = FileSkillsSource(
skill_paths=["./skills", "./more-skills"],
script_runner=my_runner,
script_extensions=(".py", ".sh"),
resource_directories=[".", "references", "assets"],
script_directories=["scripts"],
)
"""
@@ -2218,6 +2247,8 @@ class FileSkillsSource(SkillsSource):
script_runner: SkillScriptRunner | None = None,
resource_extensions: tuple[str, ...] | None = None,
script_extensions: tuple[str, ...] | None = None,
resource_directories: Sequence[str] | None = None,
script_directories: Sequence[str] | None = None,
) -> None:
"""Initialize a FileSkillsSource.
@@ -2237,6 +2268,18 @@ class FileSkillsSource(SkillsSource):
``(".md", ".json", ".yaml", ".yml", ".csv", ".xml", ".txt")``.
script_extensions: File extensions recognized as discoverable
scripts. Defaults to ``(".py",)``.
resource_directories: Relative directory paths to scan for
resource files within each skill directory. Use ``"."``
to include files at the skill root level. Defaults to
``("references", "assets")`` per the
`agentskills.io specification
<https://agentskills.io/specification>`_.
script_directories: Relative directory paths to scan for
script files within each skill directory. Use ``"."``
to include files at the skill root level. Defaults to
``("scripts",)`` per the
`agentskills.io specification
<https://agentskills.io/specification>`_.
"""
if isinstance(skill_paths, (str, Path)):
self._skill_paths: list[str] = [str(skill_paths)]
@@ -2247,6 +2290,17 @@ class FileSkillsSource(SkillsSource):
self._resource_extensions = resource_extensions or DEFAULT_RESOURCE_EXTENSIONS
self._script_extensions = script_extensions or DEFAULT_SCRIPT_EXTENSIONS
self._resource_directories: tuple[str, ...] = (
tuple(FileSkillsSource._validate_and_normalize_directory_names(resource_directories))
if resource_directories is not None
else DEFAULT_RESOURCE_DIRECTORIES
)
self._script_directories: tuple[str, ...] = (
tuple(FileSkillsSource._validate_and_normalize_directory_names(script_directories))
if script_directories is not None
else DEFAULT_SCRIPT_DIRECTORIES
)
async def get_skills(self) -> list[Skill]:
"""Discover and return all file-based skills from configured paths.
@@ -2284,12 +2338,16 @@ class FileSkillsSource(SkillsSource):
)
# Discover and attach file-based resources
for rn in FileSkillsSource._discover_resource_files(skill_path, self._resource_extensions):
for rn in FileSkillsSource._discover_resource_files(
skill_path, self._resource_extensions, self._resource_directories
):
resource_full_path = FileSkillsSource._get_validated_resource_path(skill_path, rn)
file_skill.resources.append(_FileSkillResource(name=rn, full_path=resource_full_path))
# Discover and attach file-based scripts as SkillScript instances
for sn in FileSkillsSource._discover_script_files(skill_path, self._script_extensions):
for sn in FileSkillsSource._discover_script_files(
skill_path, self._script_extensions, self._script_directories
):
script_full_path = os.path.normpath(os.path.join(skill_path, sn)) # noqa: ASYNC240
file_skill.scripts.append(
FileSkillScript(name=sn, full_path=script_full_path, runner=self._script_runner)
@@ -2369,116 +2427,274 @@ class FileSkillsSource(SkillsSource):
return True
return False
@staticmethod
def _validate_and_normalize_directory_names(
directories: Sequence[str],
) -> list[str]:
"""Validate and normalize relative directory names.
Ensures each entry is a safe relative path. The ``"."`` root indicator
is passed through unchanged. Entries containing ``..`` segments or
representing absolute paths are rejected with a warning and skipped.
Empty or whitespace-only entries raise :class:`ValueError`.
Args:
directories: Sequence of relative directory names to validate.
Returns:
A list of validated, normalized directory names.
Raises:
ValueError: If any entry is empty or whitespace-only.
"""
result: list[str] = []
for directory in directories:
if not directory or not directory.strip():
raise ValueError("Directory names must not be empty or whitespace.")
# Normalize separators: backslash → forward slash, strip leading ./ and trailing /
normalized = PurePosixPath(directory.replace("\\", "/")).as_posix()
# "." and "./" both normalize to "." — treat as root indicator
if normalized == ROOT_DIRECTORY_INDICATOR:
result.append(ROOT_DIRECTORY_INDICATOR)
continue
# Reject absolute paths (check both POSIX and Windows-style roots
# so validation is consistent regardless of the host OS)
if (
os.path.isabs(directory)
or normalized.startswith("/")
or re.match(r"^[A-Za-z]:[/\\]", directory)
):
logger.warning(
"Skipping directory '%s': absolute paths are not allowed.",
directory,
)
continue
# Reject paths containing ".." segments
if any(segment == ".." for segment in normalized.split("/")):
logger.warning(
"Skipping directory '%s': parent traversal ('..') is not allowed.",
directory,
)
continue
result.append(normalized)
return result
@staticmethod
def _discover_resource_files(
skill_dir_path: str,
extensions: tuple[str, ...] = DEFAULT_RESOURCE_EXTENSIONS,
directories: tuple[str, ...] = DEFAULT_RESOURCE_DIRECTORIES,
) -> list[str]:
"""Scan a skill directory for resource files matching *extensions*.
"""Scan configured subdirectories for resource files matching *extensions*.
Recursively walks *skill_dir_path* and collects files whose extension
is in *extensions*, excluding ``SKILL.md`` itself. Each candidate is
validated against path-traversal and symlink-escape checks; unsafe
files are skipped with a warning.
Scans each directory in *directories* within *skill_dir_path* for files
whose extension is in *extensions*, excluding ``SKILL.md`` itself.
Use ``"."`` in *directories* to include files at the skill root level.
Each candidate is validated against path-traversal and symlink-escape
checks; unsafe files are skipped with a warning.
Args:
skill_dir_path: Absolute path to the skill directory to scan.
extensions: Tuple of allowed file extensions (e.g. ``(".md", ".json")``).
directories: Relative subdirectory paths to scan for resources.
Returns:
Relative resource paths (forward-slash-separated) for every
Sorted relative resource paths (forward-slash-separated) for every
discovered file that passes security checks.
"""
skill_dir = Path(skill_dir_path).absolute()
root_directory_path = str(skill_dir)
resources: list[str] = []
normalized_extensions = {e.lower() for e in extensions}
seen_directories: set[str] = set()
for resource_file in skill_dir.rglob("*"):
if not resource_file.is_file():
for directory in directories:
is_root = directory == ROOT_DIRECTORY_INDICATOR
target_dir = skill_dir if is_root else (skill_dir / directory)
# Deduplicate after resolving to avoid scanning the same directory twice.
# Use normcase for case-insensitive dedup on case-insensitive filesystems.
resolved_target = str(Path(os.path.normpath(target_dir)).absolute())
dedup_key = os.path.normcase(resolved_target)
if dedup_key in seen_directories:
continue
seen_directories.add(dedup_key)
if not target_dir.is_dir():
continue
if resource_file.name.upper() == SKILL_FILE_NAME.upper():
continue
# Directory-level containment and symlink checks for non-root directories
if not is_root:
if not FileSkillsSource._is_path_within_directory(resolved_target, root_directory_path):
logger.warning(
"Skipping resource directory '%s': resolves outside skill directory '%s'",
directory,
skill_dir_path,
)
continue
if resource_file.suffix.lower() not in normalized_extensions:
continue
if FileSkillsSource._has_symlink_in_path(resolved_target, root_directory_path):
logger.warning(
"Skipping resource directory '%s': symlink detected in path under skill directory '%s'",
directory,
skill_dir_path,
)
continue
resource_full_path = str(Path(os.path.normpath(resource_file)).absolute())
if not FileSkillsSource._is_path_within_directory(resource_full_path, root_directory_path):
# Scan top-level files only (non-recursive) within this directory
try:
entries = list(target_dir.iterdir())
except OSError:
logger.warning(
"Skipping resource '%s': resolves outside skill directory '%s'",
resource_file,
"Failed to list resource directory '%s' in skill directory '%s'; skipping.",
directory,
skill_dir_path,
)
continue
if FileSkillsSource._has_symlink_in_path(resource_full_path, root_directory_path):
logger.warning(
"Skipping resource '%s': symlink detected in path under skill directory '%s'",
resource_file,
skill_dir_path,
)
continue
for resource_file in entries:
if not resource_file.is_file():
continue
rel_path = resource_file.relative_to(skill_dir)
resources.append(FileSkillsSource._normalize_resource_path(str(rel_path)))
if resource_file.name.upper() == SKILL_FILE_NAME.upper():
continue
if resource_file.suffix.lower() not in normalized_extensions:
continue
resource_full_path = str(Path(os.path.normpath(resource_file)).absolute())
# Containment check: file must resolve within the target directory
if not FileSkillsSource._is_path_within_directory(resource_full_path, resolved_target):
logger.warning(
"Skipping resource '%s': resolves outside target directory '%s'",
resource_file,
directory,
)
continue
if FileSkillsSource._has_symlink_in_path(resource_full_path, root_directory_path):
logger.warning(
"Skipping resource '%s': symlink detected in path under skill directory '%s'",
resource_file,
skill_dir_path,
)
continue
rel_path = resource_file.relative_to(skill_dir)
resources.append(FileSkillsSource._normalize_resource_path(str(rel_path)))
resources.sort()
return resources
@staticmethod
def _discover_script_files(
skill_dir_path: str,
extensions: tuple[str, ...] = DEFAULT_SCRIPT_EXTENSIONS,
directories: tuple[str, ...] = DEFAULT_SCRIPT_DIRECTORIES,
) -> list[str]:
"""Scan a skill directory for script files matching *extensions*.
"""Scan configured subdirectories for script files matching *extensions*.
Recursively walks *skill_dir_path* and collects files whose extension
is in *extensions*. Each candidate is validated against path-traversal
and symlink-escape checks; unsafe files are skipped with a warning.
Scans each directory in *directories* within *skill_dir_path* for files
whose extension is in *extensions*. Use ``"."`` in *directories* to
include files at the skill root level. Each candidate is validated
against path-traversal and symlink-escape checks; unsafe files are
skipped with a warning.
Args:
skill_dir_path: Absolute path to the skill directory to scan.
extensions: Tuple of allowed script extensions (e.g. ``(".py",)``).
directories: Relative subdirectory paths to scan for scripts.
Returns:
Relative script paths (forward-slash-separated) for every
Sorted relative script paths (forward-slash-separated) for every
discovered file that passes security checks.
"""
skill_dir = Path(skill_dir_path).absolute()
root_directory_path = str(skill_dir)
scripts: list[str] = []
normalized_extensions = {e.lower() for e in extensions}
seen_directories: set[str] = set()
for script_file in skill_dir.rglob("*"):
if not script_file.is_file():
for directory in directories:
is_root = directory == ROOT_DIRECTORY_INDICATOR
target_dir = skill_dir if is_root else (skill_dir / directory)
# Deduplicate after resolving to avoid scanning the same directory twice.
# Use normcase for case-insensitive dedup on case-insensitive filesystems.
resolved_target = str(Path(os.path.normpath(target_dir)).absolute())
dedup_key = os.path.normcase(resolved_target)
if dedup_key in seen_directories:
continue
seen_directories.add(dedup_key)
if not target_dir.is_dir():
continue
if script_file.suffix.lower() not in normalized_extensions:
continue
# Directory-level containment and symlink checks for non-root directories
if not is_root:
if not FileSkillsSource._is_path_within_directory(resolved_target, root_directory_path):
logger.warning(
"Skipping script directory '%s': resolves outside skill directory '%s'",
directory,
skill_dir_path,
)
continue
script_full_path = str(Path(os.path.normpath(script_file)).absolute())
if FileSkillsSource._has_symlink_in_path(resolved_target, root_directory_path):
logger.warning(
"Skipping script directory '%s': symlink detected in path under skill directory '%s'",
directory,
skill_dir_path,
)
continue
if not FileSkillsSource._is_path_within_directory(script_full_path, root_directory_path):
# Scan top-level files only (non-recursive) within this directory
try:
entries = list(target_dir.iterdir())
except OSError:
logger.warning(
"Skipping script '%s': resolves outside skill directory '%s'",
script_file,
"Failed to list script directory '%s' in skill directory '%s'; skipping.",
directory,
skill_dir_path,
)
continue
if FileSkillsSource._has_symlink_in_path(script_full_path, root_directory_path):
logger.warning(
"Skipping script '%s': symlink detected in path under skill directory '%s'",
script_file,
skill_dir_path,
)
continue
for script_file in entries:
if not script_file.is_file():
continue
rel_path = script_file.relative_to(skill_dir)
scripts.append(FileSkillsSource._normalize_resource_path(str(rel_path)))
if script_file.suffix.lower() not in normalized_extensions:
continue
script_full_path = str(Path(os.path.normpath(script_file)).absolute())
# Containment check: file must resolve within the target directory
if not FileSkillsSource._is_path_within_directory(script_full_path, resolved_target):
logger.warning(
"Skipping script '%s': resolves outside target directory '%s'",
script_file,
directory,
)
continue
if FileSkillsSource._has_symlink_in_path(script_full_path, root_directory_path):
logger.warning(
"Skipping script '%s': symlink detected in path under skill directory '%s'",
script_file,
skill_dir_path,
)
continue
rel_path = script_file.relative_to(skill_dir)
scripts.append(FileSkillsSource._normalize_resource_path(str(rel_path)))
scripts.sort()
return scripts
@staticmethod
+416 -107
View File
@@ -31,8 +31,11 @@ from agent_framework import (
SkillsProvider,
)
from agent_framework._skills import (
DEFAULT_RESOURCE_DIRECTORIES,
DEFAULT_RESOURCE_EXTENSIONS,
DEFAULT_SCRIPT_DIRECTORIES,
DEFAULT_SCRIPT_EXTENSIONS,
ROOT_DIRECTORY_INDICATOR,
InlineSkillResource,
InlineSkillScript,
_create_resource_element,
@@ -143,6 +146,8 @@ async def _discover_file_skills_for_test(
*,
resource_extensions: tuple[str, ...] | None = None,
script_extensions: tuple[str, ...] | None = None,
resource_directories: Sequence[str] | None = None,
script_directories: Sequence[str] | None = None,
script_runner: Any = None,
) -> dict[str, FileSkill]:
"""Test helper: discover file skills and return as a dict keyed by name.
@@ -155,6 +160,10 @@ async def _discover_file_skills_for_test(
kwargs["resource_extensions"] = resource_extensions
if script_extensions is not None:
kwargs["script_extensions"] = script_extensions
if resource_directories is not None:
kwargs["resource_directories"] = resource_directories
if script_directories is not None:
kwargs["script_directories"] = script_directories
if script_runner is not None:
kwargs["script_runner"] = script_runner
@@ -191,59 +200,103 @@ class TestNormalizeResourcePath:
class TestDiscoverResourceFiles:
"""Tests for _discover_resource_files (filesystem-based resource discovery)."""
def test_discovers_md_files(self, tmp_path: Path) -> None:
def test_discovers_md_files_in_references(self, tmp_path: Path) -> None:
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text("---\nname: s\ndescription: d\n---\n", encoding="utf-8")
refs = skill_dir / "refs"
refs = skill_dir / "references"
refs.mkdir()
(refs / "FAQ.md").write_text("FAQ content", encoding="utf-8")
resources = FileSkillsSource._discover_resource_files(str(skill_dir))
assert "refs/FAQ.md" in resources
assert "references/FAQ.md" in resources
def test_excludes_skill_md(self, tmp_path: Path) -> None:
def test_discovers_md_files_in_assets(self, tmp_path: Path) -> None:
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
assets = skill_dir / "assets"
assets.mkdir()
(assets / "guide.md").write_text("guide", encoding="utf-8")
resources = FileSkillsSource._discover_resource_files(str(skill_dir))
assert "assets/guide.md" in resources
def test_excludes_skill_md_at_root(self, tmp_path: Path) -> None:
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text("content", encoding="utf-8")
resources = FileSkillsSource._discover_resource_files(str(skill_dir))
resources = FileSkillsSource._discover_resource_files(str(skill_dir), directories=(".",))
assert len(resources) == 0
def test_discovers_multiple_extensions(self, tmp_path: Path) -> None:
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
(skill_dir / "data.json").write_text("{}", encoding="utf-8")
(skill_dir / "config.yaml").write_text("key: val", encoding="utf-8")
(skill_dir / "notes.txt").write_text("notes", encoding="utf-8")
refs = skill_dir / "references"
refs.mkdir(parents=True)
(refs / "data.json").write_text("{}", encoding="utf-8")
(refs / "config.yaml").write_text("key: val", encoding="utf-8")
(refs / "notes.txt").write_text("notes", encoding="utf-8")
resources = FileSkillsSource._discover_resource_files(str(skill_dir))
assert len(resources) == 3
names = set(resources)
assert "data.json" in names
assert "config.yaml" in names
assert "notes.txt" in names
assert "references/data.json" in names
assert "references/config.yaml" in names
assert "references/notes.txt" in names
def test_ignores_unsupported_extensions(self, tmp_path: Path) -> None:
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
(skill_dir / "image.png").write_bytes(b"\x89PNG")
(skill_dir / "binary.exe").write_bytes(b"\x00")
refs = skill_dir / "references"
refs.mkdir(parents=True)
(refs / "image.png").write_bytes(b"\x89PNG")
(refs / "binary.exe").write_bytes(b"\x00")
resources = FileSkillsSource._discover_resource_files(str(skill_dir))
assert len(resources) == 0
def test_custom_extensions(self, tmp_path: Path) -> None:
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
(skill_dir / "data.json").write_text("{}", encoding="utf-8")
(skill_dir / "notes.txt").write_text("notes", encoding="utf-8")
refs = skill_dir / "references"
refs.mkdir(parents=True)
(refs / "data.json").write_text("{}", encoding="utf-8")
(refs / "notes.txt").write_text("notes", encoding="utf-8")
resources = FileSkillsSource._discover_resource_files(str(skill_dir), extensions=(".json",))
assert resources == ["data.json"]
assert resources == ["references/data.json"]
def test_discovers_nested_files(self, tmp_path: Path) -> None:
def test_does_not_discover_nested_files(self, tmp_path: Path) -> None:
"""Non-recursive: files inside subdirectories of configured dirs are not discovered."""
skill_dir = tmp_path / "my-skill"
sub = skill_dir / "refs" / "deep"
sub = skill_dir / "references" / "deep"
sub.mkdir(parents=True)
(sub / "doc.md").write_text("deep doc", encoding="utf-8")
resources = FileSkillsSource._discover_resource_files(str(skill_dir))
assert "refs/deep/doc.md" in resources
assert len(resources) == 0
def test_root_directory_discovers_root_files(self, tmp_path: Path) -> None:
"""The '.' root indicator discovers files at the skill root level."""
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
(skill_dir / "data.json").write_text("{}", encoding="utf-8")
resources = FileSkillsSource._discover_resource_files(str(skill_dir), directories=(".",))
assert "data.json" in resources
def test_root_does_not_discover_by_default(self, tmp_path: Path) -> None:
"""Files at skill root are not discovered with default directories."""
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
(skill_dir / "data.json").write_text("{}", encoding="utf-8")
resources = FileSkillsSource._discover_resource_files(str(skill_dir))
assert len(resources) == 0
def test_custom_directories(self, tmp_path: Path) -> None:
"""Custom directory names override defaults."""
skill_dir = tmp_path / "my-skill"
custom = skill_dir / "docs"
custom.mkdir(parents=True)
(custom / "readme.md").write_text("readme", encoding="utf-8")
resources = FileSkillsSource._discover_resource_files(str(skill_dir), directories=("docs",))
assert "docs/readme.md" in resources
def test_nonexistent_directory_silently_skipped(self, tmp_path: Path) -> None:
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
resources = FileSkillsSource._discover_resource_files(str(skill_dir), directories=("nonexistent",))
assert resources == []
def test_empty_directory(self, tmp_path: Path) -> None:
skill_dir = tmp_path / "my-skill"
@@ -260,6 +313,27 @@ class TestDiscoverResourceFiles:
assert ".xml" in DEFAULT_RESOURCE_EXTENSIONS
assert ".txt" in DEFAULT_RESOURCE_EXTENSIONS
def test_duplicate_directories_deduplicated(self, tmp_path: Path) -> None:
"""Duplicate directory entries should not produce duplicate resources."""
skill_dir = tmp_path / "my-skill"
refs = skill_dir / "references"
refs.mkdir(parents=True)
(refs / "doc.md").write_text("content", encoding="utf-8")
resources = FileSkillsSource._discover_resource_files(
str(skill_dir), directories=("references", "references")
)
assert resources == ["references/doc.md"]
def test_results_are_sorted(self, tmp_path: Path) -> None:
"""Results should be sorted for stable ordering."""
skill_dir = tmp_path / "my-skill"
refs = skill_dir / "references"
refs.mkdir(parents=True)
(refs / "zebra.md").write_text("z", encoding="utf-8")
(refs / "alpha.md").write_text("a", encoding="utf-8")
resources = FileSkillsSource._discover_resource_files(str(skill_dir))
assert resources == ["references/alpha.md", "references/zebra.md"]
class TestTryParseSkillDocument:
"""Tests for _extract_frontmatter."""
@@ -413,11 +487,11 @@ class TestDiscoverAndLoadSkills:
tmp_path,
"my-skill",
body="Instructions here.",
resources={"refs/FAQ.md": "FAQ content"},
resources={"references/FAQ.md": "FAQ content"},
)
skills = await _discover_file_skills_for_test([str(tmp_path)])
assert "my-skill" in skills
assert [r.name for r in skills["my-skill"].resources] == ["refs/FAQ.md"]
assert [r.name for r in skills["my-skill"].resources] == ["references/FAQ.md"]
async def test_skill_discovers_all_resource_files(self, tmp_path: Path) -> None:
"""Resources are discovered by filesystem scan, not by markdown links."""
@@ -425,13 +499,13 @@ class TestDiscoverAndLoadSkills:
tmp_path,
"my-skill",
body="No links here.",
resources={"data.json": '{"key": "val"}', "refs/doc.md": "doc content"},
resources={"references/data.json": '{"key": "val"}', "assets/doc.md": "doc content"},
)
skills = await _discover_file_skills_for_test([str(tmp_path)])
assert "my-skill" in skills
resource_names = sorted(r.name for r in skills["my-skill"].resources)
assert "data.json" in resource_names
assert "refs/doc.md" in resource_names
assert "assets/doc.md" in resource_names
assert "references/data.json" in resource_names
# ---------------------------------------------------------------------------
@@ -446,12 +520,12 @@ class TestReadSkillResource:
_write_skill(
tmp_path,
"my-skill",
body="See [doc](refs/FAQ.md).",
resources={"refs/FAQ.md": "FAQ content here"},
body="See [doc](references/FAQ.md).",
resources={"references/FAQ.md": "FAQ content here"},
)
skill_dir = tmp_path / "my-skill"
full_path = str(skill_dir / "refs" / "FAQ.md")
resource = _FileSkillResource(name="refs/FAQ.md", full_path=full_path)
full_path = str(skill_dir / "references" / "FAQ.md")
resource = _FileSkillResource(name="references/FAQ.md", full_path=full_path)
content = await resource.read()
assert content == "FAQ content here"
@@ -468,12 +542,12 @@ class TestReadSkillResource:
_write_skill(
tmp_path,
"my-skill",
body="See [doc](refs/FAQ.md).",
resources={"refs/FAQ.md": "FAQ content"},
body="See [doc](references/FAQ.md).",
resources={"references/FAQ.md": "FAQ content"},
)
skill_dir = tmp_path / "my-skill"
full_path = str(skill_dir / "refs" / "FAQ.md")
resource = _FileSkillResource(name="refs/FAQ.md", full_path=full_path)
full_path = str(skill_dir / "references" / "FAQ.md")
resource = _FileSkillResource(name="references/FAQ.md", full_path=full_path)
content = await resource.read()
assert content == "FAQ content"
@@ -486,7 +560,7 @@ class TestReadSkillResource:
skill_dir = tmp_path / "skill"
skill_dir.mkdir()
(tmp_path / "secret.md").write_text("secret", encoding="utf-8")
resources = FileSkillsSource._discover_resource_files(str(skill_dir))
resources = FileSkillsSource._discover_resource_files(str(skill_dir), directories=(".",))
assert not any("secret" in r for r in resources)
@@ -633,13 +707,13 @@ class TestSkillsProvider:
_write_skill(
tmp_path,
"my-skill",
body="See [doc](refs/FAQ.md).",
resources={"refs/FAQ.md": "FAQ content"},
body="See [doc](references/FAQ.md).",
resources={"references/FAQ.md": "FAQ content"},
)
provider = SkillsProvider.from_paths(str(tmp_path))
await _init_provider(provider)
result = provider._load_skill(_raw_skills(provider), "my-skill")
assert "See [doc](refs/FAQ.md)." in result
assert "See [doc](references/FAQ.md)." in result
async def test_load_skill_unknown_returns_error(self, tmp_path: Path) -> None:
provider = SkillsProvider.from_paths(str(tmp_path))
@@ -657,12 +731,12 @@ class TestSkillsProvider:
_write_skill(
tmp_path,
"my-skill",
body="See [doc](refs/FAQ.md).",
resources={"refs/FAQ.md": "FAQ content"},
body="See [doc](references/FAQ.md).",
resources={"references/FAQ.md": "FAQ content"},
)
provider = SkillsProvider.from_paths(str(tmp_path))
await _init_provider(provider)
result = await provider._read_skill_resource(_raw_skills(provider), "my-skill", "refs/FAQ.md")
result = await provider._read_skill_resource(_raw_skills(provider), "my-skill", "references/FAQ.md")
assert result == "FAQ content"
async def test_read_skill_resource_unknown_skill_returns_error(self, tmp_path: Path) -> None:
@@ -791,7 +865,7 @@ class TestSymlinkDetection:
"---\nname: my-skill\ndescription: A test skill.\n---\nInstructions.\n",
encoding="utf-8",
)
refs_dir = skill_dir / "refs"
refs_dir = skill_dir / "references"
refs_dir.mkdir()
(refs_dir / "leak.md").symlink_to(outside_file)
# Also add a safe resource
@@ -800,8 +874,8 @@ class TestSymlinkDetection:
skills = await _discover_file_skills_for_test([str(tmp_path)])
assert "my-skill" in skills
resource_names = [r.name for r in skills["my-skill"].resources]
assert "refs/leak.md" not in resource_names
assert "refs/safe.md" in resource_names
assert "references/leak.md" not in resource_names
assert "references/safe.md" in resource_names
def test_discover_resource_files_rejects_symlinked_resource(self, tmp_path: Path) -> None:
"""_discover_resource_files should exclude a symlinked resource file."""
@@ -811,12 +885,12 @@ class TestSymlinkDetection:
outside_file = tmp_path / "secret.md"
outside_file.write_text("secret content", encoding="utf-8")
refs_dir = skill_dir / "refs"
refs_dir = skill_dir / "references"
refs_dir.mkdir()
(refs_dir / "leak.md").symlink_to(outside_file)
resources = FileSkillsSource._discover_resource_files(str(skill_dir))
assert "refs/leak.md" not in resources
assert "references/leak.md" not in resources
def test_discover_skips_symlinked_script(self, tmp_path: Path) -> None:
"""_discover_script_files should skip scripts with symlinks in their path."""
@@ -1361,21 +1435,22 @@ class TestSkillsProviderCodeSkill:
async def test_custom_resource_extensions(self, tmp_path: Path) -> None:
"""SkillsProvider accepts custom resource_extensions."""
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
refs = skill_dir / "references"
refs.mkdir(parents=True)
(skill_dir / "SKILL.md").write_text(
"---\nname: my-skill\ndescription: A test skill.\n---\nBody.",
encoding="utf-8",
)
(skill_dir / "data.json").write_text("{}", encoding="utf-8")
(skill_dir / "notes.txt").write_text("notes", encoding="utf-8")
(refs / "data.json").write_text("{}", encoding="utf-8")
(refs / "notes.txt").write_text("notes", encoding="utf-8")
# Only discover .json files
provider = SkillsProvider.from_paths(str(tmp_path), resource_extensions=(".json",))
await _init_provider(provider)
skill = _ctx(provider)[0]["my-skill"]
resource_names = [r.name for r in skill.resources]
assert "data.json" in resource_names
assert "notes.txt" not in resource_names
assert "references/data.json" in resource_names
assert "references/notes.txt" not in resource_names
# ---------------------------------------------------------------------------
@@ -1407,11 +1482,11 @@ class TestFileBasedSkillParsing:
assert skill.path == str(tmp_path / "my-skill")
async def test_resources_populated(self, tmp_path: Path) -> None:
_write_skill(tmp_path, "my-skill", resources={"refs/doc.md": "content"})
_write_skill(tmp_path, "my-skill", resources={"references/doc.md": "content"})
skills = await _discover_file_skills_for_test([str(tmp_path)])
assert "my-skill" in skills
resource_names = [r.name for r in skills["my-skill"].resources]
assert "refs/doc.md" in resource_names
assert "references/doc.md" in resource_names
# ---------------------------------------------------------------------------
@@ -1467,12 +1542,12 @@ class TestDiscoverResourceFilesEdgeCases:
"""Additional edge-case tests for filesystem resource discovery."""
def test_excludes_skill_md_case_insensitive(self, tmp_path: Path) -> None:
"""SKILL.md in any casing is excluded."""
"""SKILL.md in any casing is excluded when scanning root."""
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
(skill_dir / "skill.md").write_text("lowercase name", encoding="utf-8")
(skill_dir / "other.md").write_text("keep me", encoding="utf-8")
resources = FileSkillsSource._discover_resource_files(str(skill_dir))
resources = FileSkillsSource._discover_resource_files(str(skill_dir), directories=(".",))
names = [r.lower() for r in resources]
assert "skill.md" not in names
assert "other.md" in resources
@@ -1480,19 +1555,221 @@ class TestDiscoverResourceFilesEdgeCases:
def test_skips_directories(self, tmp_path: Path) -> None:
"""Directories are not included as resources even if their name matches an extension."""
skill_dir = tmp_path / "my-skill"
subdir = skill_dir / "data.json"
subdir.mkdir(parents=True)
refs = skill_dir / "references"
refs.mkdir(parents=True)
subdir = refs / "data.json"
subdir.mkdir()
resources = FileSkillsSource._discover_resource_files(str(skill_dir))
assert resources == []
def test_extension_matching_is_case_insensitive(self, tmp_path: Path) -> None:
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
(skill_dir / "NOTES.TXT").write_text("caps", encoding="utf-8")
refs = skill_dir / "references"
refs.mkdir(parents=True)
(refs / "NOTES.TXT").write_text("caps", encoding="utf-8")
resources = FileSkillsSource._discover_resource_files(str(skill_dir))
assert len(resources) == 1
class TestDiscoverFilesOSErrorWarning:
"""OSError during directory listing should log a warning, not fail silently."""
def test_resource_discovery_warns_on_oserror(self, tmp_path: Path, caplog: pytest.LogCaptureFixture) -> None:
"""_discover_resource_files logs a warning when iterdir() raises OSError."""
skill_dir = tmp_path / "my-skill"
refs = skill_dir / "references"
refs.mkdir(parents=True)
(refs / "guide.md").write_text("content", encoding="utf-8")
original_iterdir = Path.iterdir
def _patched_iterdir(self: Path) -> Any:
if self.name == "references":
raise PermissionError("access denied")
return original_iterdir(self)
import unittest.mock
with unittest.mock.patch.object(Path, "iterdir", _patched_iterdir):
resources = FileSkillsSource._discover_resource_files(str(skill_dir))
assert resources == []
assert any("Failed to list resource directory" in r.message for r in caplog.records)
def test_script_discovery_warns_on_oserror(self, tmp_path: Path, caplog: pytest.LogCaptureFixture) -> None:
"""_discover_script_files logs a warning when iterdir() raises OSError."""
skill_dir = tmp_path / "my-skill"
scripts_dir = skill_dir / "scripts"
scripts_dir.mkdir(parents=True)
(scripts_dir / "run.py").write_text("print('hi')", encoding="utf-8")
original_iterdir = Path.iterdir
def _patched_iterdir(self: Path) -> Any:
if self.name == "scripts":
raise PermissionError("access denied")
return original_iterdir(self)
import unittest.mock
with unittest.mock.patch.object(Path, "iterdir", _patched_iterdir):
scripts = FileSkillsSource._discover_script_files(str(skill_dir))
assert scripts == []
assert any("Failed to list script directory" in r.message for r in caplog.records)
class TestValidateAndNormalizeDirectoryNames:
"""Tests for _validate_and_normalize_directory_names."""
def test_simple_directory_name(self) -> None:
result = FileSkillsSource._validate_and_normalize_directory_names(["references"])
assert result == ["references"]
def test_root_indicator(self) -> None:
result = FileSkillsSource._validate_and_normalize_directory_names(["."])
assert result == ["."]
def test_dot_slash_normalizes_to_root(self) -> None:
result = FileSkillsSource._validate_and_normalize_directory_names(["./"])
assert result == ["."]
def test_backslash_dot_normalizes_to_root(self) -> None:
result = FileSkillsSource._validate_and_normalize_directory_names([".\\"])
assert result == ["."]
def test_backslashes_normalized(self) -> None:
result = FileSkillsSource._validate_and_normalize_directory_names(["sub\\scripts"])
assert result == ["sub/scripts"]
def test_trailing_slash_stripped(self) -> None:
result = FileSkillsSource._validate_and_normalize_directory_names(["scripts/"])
assert result == ["scripts"]
def test_leading_dot_slash_stripped(self) -> None:
result = FileSkillsSource._validate_and_normalize_directory_names(["./references"])
assert result == ["references"]
def test_rejects_parent_traversal(self) -> None:
result = FileSkillsSource._validate_and_normalize_directory_names(["../secrets"])
assert result == []
def test_rejects_embedded_parent_traversal(self) -> None:
result = FileSkillsSource._validate_and_normalize_directory_names(["sub/../secrets"])
assert result == []
def test_rejects_absolute_path(self) -> None:
result = FileSkillsSource._validate_and_normalize_directory_names(["/etc/passwd"])
assert result == []
def test_rejects_windows_absolute_path(self) -> None:
result = FileSkillsSource._validate_and_normalize_directory_names(["C:\\Windows"])
assert result == []
def test_empty_string_raises(self) -> None:
with pytest.raises(ValueError, match="empty or whitespace"):
FileSkillsSource._validate_and_normalize_directory_names([""])
def test_whitespace_only_raises(self) -> None:
with pytest.raises(ValueError, match="empty or whitespace"):
FileSkillsSource._validate_and_normalize_directory_names([" "])
def test_multiple_directories(self) -> None:
result = FileSkillsSource._validate_and_normalize_directory_names(
[".", "references", "assets", "scripts"]
)
assert result == [".", "references", "assets", "scripts"]
def test_default_resource_directories(self) -> None:
assert DEFAULT_RESOURCE_DIRECTORIES == ("references", "assets")
def test_default_script_directories(self) -> None:
assert DEFAULT_SCRIPT_DIRECTORIES == ("scripts",)
def test_root_directory_indicator_is_dot(self) -> None:
assert ROOT_DIRECTORY_INDICATOR == "."
class TestFileSkillsSourceDirectories:
"""Tests for resource_directories and script_directories parameters."""
async def test_custom_resource_directories(self, tmp_path: Path) -> None:
"""Custom resource_directories controls which dirs are scanned."""
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text(
"---\nname: my-skill\ndescription: test\n---\nBody",
encoding="utf-8",
)
# Put resource in a custom directory
docs = skill_dir / "docs"
docs.mkdir()
(docs / "guide.md").write_text("guide", encoding="utf-8")
# Also put one in default references/ — should not be found
refs = skill_dir / "references"
refs.mkdir()
(refs / "ref.md").write_text("ref", encoding="utf-8")
source = FileSkillsSource(str(tmp_path), resource_directories=["docs"])
skills = await source.get_skills()
resource_names = [r.name for r in skills[0].resources]
assert "docs/guide.md" in resource_names
assert "references/ref.md" not in resource_names
async def test_custom_script_directories(self, tmp_path: Path) -> None:
"""Custom script_directories controls which dirs are scanned."""
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text(
"---\nname: my-skill\ndescription: test\n---\nBody",
encoding="utf-8",
)
# Put script in a custom directory
tools = skill_dir / "tools"
tools.mkdir()
(tools / "run.py").write_text("print('run')", encoding="utf-8")
source = FileSkillsSource(str(tmp_path), script_directories=["tools"])
skills = await source.get_skills()
script_names = [s.name for s in skills[0].scripts]
assert "tools/run.py" in script_names
async def test_root_indicator_discovers_root_files(self, tmp_path: Path) -> None:
"""The '.' root indicator discovers files at the skill root."""
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text(
"---\nname: my-skill\ndescription: test\n---\nBody",
encoding="utf-8",
)
(skill_dir / "data.json").write_text("{}", encoding="utf-8")
source = FileSkillsSource(str(tmp_path), resource_directories=[".", "references"])
skills = await source.get_skills()
resource_names = [r.name for r in skills[0].resources]
assert "data.json" in resource_names
async def test_from_paths_passes_directories(self, tmp_path: Path) -> None:
"""from_paths passes resource_directories and script_directories through."""
skill_dir = tmp_path / "my-skill"
docs = skill_dir / "docs"
docs.mkdir(parents=True)
(skill_dir / "SKILL.md").write_text(
"---\nname: my-skill\ndescription: test\n---\nBody",
encoding="utf-8",
)
(docs / "guide.md").write_text("guide", encoding="utf-8")
provider = SkillsProvider.from_paths(
str(tmp_path),
resource_directories=["docs"],
)
await _init_provider(provider)
skill = _ctx(provider)[0]["my-skill"]
resource_names = [r.name for r in skill.resources]
assert "docs/guide.md" in resource_names
# ---------------------------------------------------------------------------
# Tests: _is_path_within_directory
# ---------------------------------------------------------------------------
@@ -2928,12 +3205,13 @@ class TestSkillsProviderFactories:
assert isinstance(_CustomRunner(), SkillScriptRunner)
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
scripts_dir = skill_dir / "scripts"
scripts_dir.mkdir(parents=True)
(skill_dir / "SKILL.md").write_text(
"---\nname: my-skill\ndescription: test\n---\nBody",
encoding="utf-8",
)
(skill_dir / "run.py").write_text("print('hi')", encoding="utf-8")
(scripts_dir / "run.py").write_text("print('hi')", encoding="utf-8")
provider = SkillsProvider.from_paths(
str(tmp_path),
@@ -2949,12 +3227,13 @@ class TestSkillsProviderFactories:
assert isinstance(sync_runner, SkillScriptRunner)
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
scripts_dir = skill_dir / "scripts"
scripts_dir.mkdir(parents=True)
(skill_dir / "SKILL.md").write_text(
"---\nname: my-skill\ndescription: test\n---\nBody",
encoding="utf-8",
)
(skill_dir / "run.py").write_text("print('hi')", encoding="utf-8")
(scripts_dir / "run.py").write_text("print('hi')", encoding="utf-8")
provider = SkillsProvider.from_paths(
str(tmp_path),
@@ -2966,12 +3245,13 @@ class TestSkillsProviderFactories:
async def test_file_script_with_sync_runner_executes(self, tmp_path: Path) -> None:
"""A sync script_runner is awaitable through the provider's run_skill_script."""
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
scripts_dir = skill_dir / "scripts"
scripts_dir.mkdir(parents=True)
(skill_dir / "SKILL.md").write_text(
"---\nname: my-skill\ndescription: test\n---\nBody",
encoding="utf-8",
)
(skill_dir / "run.py").write_text("print('hi')", encoding="utf-8")
(scripts_dir / "run.py").write_text("print('hi')", encoding="utf-8")
def sync_runner(skill, script, args=None):
return f"sync: {script.name} args={args}"
@@ -2982,17 +3262,18 @@ class TestSkillsProviderFactories:
)
await _init_provider(provider)
run_tool = next(t for t in _ctx(provider)[2] if hasattr(t, "name") and t.name == "run_skill_script")
result = await run_tool.func(skill_name="my-skill", script_name="run.py", args={"key": "val"})
assert result == "sync: run.py args={'key': 'val'}"
result = await run_tool.func(skill_name="my-skill", script_name="scripts/run.py", args={"key": "val"})
assert result == "sync: scripts/run.py args={'key': 'val'}"
async def test_file_skills_with_callback_runner(self, tmp_path: Path) -> None:
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
scripts_dir = skill_dir / "scripts"
scripts_dir.mkdir(parents=True)
(skill_dir / "SKILL.md").write_text(
"---\nname: my-skill\ndescription: test\n---\nBody",
encoding="utf-8",
)
(skill_dir / "run.py").write_text("print('hi')", encoding="utf-8")
(scripts_dir / "run.py").write_text("print('hi')", encoding="utf-8")
provider = SkillsProvider.from_paths(
str(tmp_path),
@@ -3028,12 +3309,13 @@ class TestSkillsProviderFactories:
async def test_file_scripts_without_runner_no_error_at_init(self, tmp_path: Path) -> None:
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
scripts_dir = skill_dir / "scripts"
scripts_dir.mkdir(parents=True)
(skill_dir / "SKILL.md").write_text(
"---\nname: my-skill\ndescription: test\n---\nBody",
encoding="utf-8",
)
(skill_dir / "run.py").write_text("print('hi')", encoding="utf-8")
(scripts_dir / "run.py").write_text("print('hi')", encoding="utf-8")
provider = SkillsProvider.from_paths(str(tmp_path))
# Initialization succeeds; the error now surfaces at script.run() time
@@ -3309,7 +3591,23 @@ class TestSkillsProviderFactories:
class TestFileScriptDiscovery:
"""Tests for automatic .py script discovery in skill directories."""
async def test_discovers_py_files(self, tmp_path: Path) -> None:
async def test_discovers_py_files_in_scripts_dir(self, tmp_path: Path) -> None:
skill_dir = tmp_path / "my-skill"
scripts_dir = skill_dir / "scripts"
scripts_dir.mkdir(parents=True)
(skill_dir / "SKILL.md").write_text(
"---\nname: my-skill\ndescription: test\n---\nBody",
encoding="utf-8",
)
(scripts_dir / "analyze.py").write_text("print('hi')", encoding="utf-8")
skills = await _discover_file_skills_for_test(str(tmp_path))
assert "my-skill" in skills
assert len(skills["my-skill"].scripts) == 1
assert skills["my-skill"].scripts[0].name == "scripts/analyze.py"
async def test_root_py_files_not_discovered_by_default(self, tmp_path: Path) -> None:
"""Scripts at the skill root are NOT discovered with default directories."""
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
(skill_dir / "SKILL.md").write_text(
@@ -3320,8 +3618,7 @@ class TestFileScriptDiscovery:
skills = await _discover_file_skills_for_test(str(tmp_path))
assert "my-skill" in skills
assert len(skills["my-skill"].scripts) == 1
assert skills["my-skill"].scripts[0].name == "analyze.py"
assert len(skills["my-skill"].scripts) == 0
async def test_discovered_script_has_absolute_full_path(self, tmp_path: Path) -> None:
skill_dir = tmp_path / "my-skill"
@@ -3340,7 +3637,8 @@ class TestFileScriptDiscovery:
expected = str(Path(str(skill_dir), "scripts", "generate.py"))
assert script.full_path == expected
async def test_discovers_nested_scripts(self, tmp_path: Path) -> None:
async def test_scripts_not_discovered_recursively(self, tmp_path: Path) -> None:
"""Scripts inside subdirectories of scripts/ are NOT discovered (non-recursive)."""
skill_dir = tmp_path / "my-skill"
scripts_dir = skill_dir / "scripts"
scripts_dir.mkdir(parents=True)
@@ -3348,11 +3646,16 @@ class TestFileScriptDiscovery:
"---\nname: my-skill\ndescription: test\n---\nBody",
encoding="utf-8",
)
(scripts_dir / "generate.py").write_text("print('gen')", encoding="utf-8")
# File directly in scripts/ is discovered
(scripts_dir / "top.py").write_text("print('top')", encoding="utf-8")
# File in scripts/sub/ is NOT discovered
sub_dir = scripts_dir / "sub"
sub_dir.mkdir()
(sub_dir / "nested.py").write_text("print('nested')", encoding="utf-8")
skills = await _discover_file_skills_for_test(str(tmp_path))
assert len(skills["my-skill"].scripts) == 1
assert skills["my-skill"].scripts[0].name == "scripts/generate.py"
assert skills["my-skill"].scripts[0].name == "scripts/top.py"
async def test_no_scripts_when_no_py_files(self, tmp_path: Path) -> None:
skill_dir = tmp_path / "my-skill"
@@ -3373,36 +3676,38 @@ class TestCustomScriptExtensions:
async def test_custom_script_extensions_via_get_skills(self, tmp_path: Path) -> None:
"""get_skills() forwards script_extensions to _discover_script_files."""
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
scripts_dir = skill_dir / "scripts"
scripts_dir.mkdir(parents=True)
(skill_dir / "SKILL.md").write_text(
"---\nname: my-skill\ndescription: test\n---\nBody",
encoding="utf-8",
)
(skill_dir / "analyze.py").write_text("print('hi')", encoding="utf-8")
(skill_dir / "run.sh").write_text("#!/bin/bash", encoding="utf-8")
(scripts_dir / "analyze.py").write_text("print('hi')", encoding="utf-8")
(scripts_dir / "run.sh").write_text("#!/bin/bash", encoding="utf-8")
# Default: only .py discovered
skills_default = await _discover_file_skills_for_test(str(tmp_path))
script_names_default = [s.name for s in skills_default["my-skill"].scripts]
assert "analyze.py" in script_names_default
assert "run.sh" not in script_names_default
assert "scripts/analyze.py" in script_names_default
assert "scripts/run.sh" not in script_names_default
# Custom: only .sh discovered
skills_custom = await _discover_file_skills_for_test(str(tmp_path), script_extensions=(".sh",))
script_names_custom = [s.name for s in skills_custom["my-skill"].scripts]
assert "run.sh" in script_names_custom
assert "analyze.py" not in script_names_custom
assert "scripts/run.sh" in script_names_custom
assert "scripts/analyze.py" not in script_names_custom
async def test_custom_script_extensions_via_provider(self, tmp_path: Path) -> None:
"""SkillsProvider accepts custom script_extensions."""
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
scripts_dir = skill_dir / "scripts"
scripts_dir.mkdir(parents=True)
(skill_dir / "SKILL.md").write_text(
"---\nname: my-skill\ndescription: test\n---\nBody",
encoding="utf-8",
)
(skill_dir / "analyze.py").write_text("print('hi')", encoding="utf-8")
(skill_dir / "run.sh").write_text("#!/bin/bash", encoding="utf-8")
(scripts_dir / "analyze.py").write_text("print('hi')", encoding="utf-8")
(scripts_dir / "run.sh").write_text("#!/bin/bash", encoding="utf-8")
# Only discover .sh scripts
provider = SkillsProvider.from_paths(
@@ -3413,20 +3718,21 @@ class TestCustomScriptExtensions:
await _init_provider(provider)
skill = _ctx(provider)[0]["my-skill"]
script_names = [s.name for s in skill.scripts]
assert "run.sh" in script_names
assert "analyze.py" not in script_names
assert "scripts/run.sh" in script_names
assert "scripts/analyze.py" not in script_names
async def test_multiple_script_extensions(self, tmp_path: Path) -> None:
"""Multiple script extensions can be specified."""
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
scripts_dir = skill_dir / "scripts"
scripts_dir.mkdir(parents=True)
(skill_dir / "SKILL.md").write_text(
"---\nname: my-skill\ndescription: test\n---\nBody",
encoding="utf-8",
)
(skill_dir / "analyze.py").write_text("print('hi')", encoding="utf-8")
(skill_dir / "run.sh").write_text("#!/bin/bash", encoding="utf-8")
(skill_dir / "notes.txt").write_text("notes", encoding="utf-8")
(scripts_dir / "analyze.py").write_text("print('hi')", encoding="utf-8")
(scripts_dir / "run.sh").write_text("#!/bin/bash", encoding="utf-8")
(scripts_dir / "notes.txt").write_text("notes", encoding="utf-8")
provider = SkillsProvider.from_paths(
str(tmp_path),
@@ -3436,9 +3742,9 @@ class TestCustomScriptExtensions:
await _init_provider(provider)
skill = _ctx(provider)[0]["my-skill"]
script_names = [s.name for s in skill.scripts]
assert "analyze.py" in script_names
assert "run.sh" in script_names
assert "notes.txt" not in script_names
assert "scripts/analyze.py" in script_names
assert "scripts/run.sh" in script_names
assert "scripts/notes.txt" not in script_names
def test_default_script_extensions_unchanged(self) -> None:
"""DEFAULT_SCRIPT_EXTENSIONS contains only .py."""
@@ -4597,21 +4903,22 @@ class TestSkillsSource:
async def test_file_skills_source_with_extensions(self, tmp_path: Path) -> None:
"""FileSkillsSource resource_extensions controls extension filtering."""
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
refs = skill_dir / "references"
refs.mkdir(parents=True)
(skill_dir / "SKILL.md").write_text(
"---\nname: my-skill\ndescription: Test skill.\n---\nBody.",
encoding="utf-8",
)
(skill_dir / "data.json").write_text("{}", encoding="utf-8")
(skill_dir / "data.csv").write_text("a,b", encoding="utf-8")
(refs / "data.json").write_text("{}", encoding="utf-8")
(refs / "data.csv").write_text("a,b", encoding="utf-8")
# Only allow .json resources
source = FileSkillsSource(str(tmp_path), resource_extensions=(".json",))
skills = await source.get_skills()
assert len(skills) == 1
resource_names = [r.name for r in skills[0].resources]
assert "data.json" in resource_names
assert "data.csv" not in resource_names
assert "references/data.json" in resource_names
assert "references/data.csv" not in resource_names
async def test_in_memory_skills_source_returns_all_skills(self) -> None:
"""InMemorySkillsSource returns all provided skills."""
@@ -4840,12 +5147,13 @@ class TestSourceComposition:
async def test_file_source_with_script_runner(self, tmp_path: Path) -> None:
"""FileSkillsSource with script_runner enables script execution."""
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
scripts_dir = skill_dir / "scripts"
scripts_dir.mkdir(parents=True)
(skill_dir / "SKILL.md").write_text(
"---\nname: my-skill\ndescription: test\n---\nBody",
encoding="utf-8",
)
(skill_dir / "run.py").write_text("print('hi')", encoding="utf-8")
(scripts_dir / "run.py").write_text("print('hi')", encoding="utf-8")
source = DeduplicatingSkillsSource(FileSkillsSource(str(tmp_path), script_runner=_noop_script_runner))
provider = SkillsProvider(source)
@@ -4875,12 +5183,13 @@ class TestSourceComposition:
async def test_per_source_runner(self, tmp_path: Path) -> None:
"""Per-source script runner is used when set on FileSkillsSource."""
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
scripts_dir = skill_dir / "scripts"
scripts_dir.mkdir(parents=True)
(skill_dir / "SKILL.md").write_text(
"---\nname: my-skill\ndescription: test\n---\nBody",
encoding="utf-8",
)
(skill_dir / "run.py").write_text("print('hi')", encoding="utf-8")
(scripts_dir / "run.py").write_text("print('hi')", encoding="utf-8")
call_log: list[str] = []
@@ -4894,7 +5203,7 @@ class TestSourceComposition:
# The source-level runner should be discovered and used
run_tool = next(t for t in _ctx(provider)[2] if hasattr(t, "name") and t.name == "run_skill_script")
result = await run_tool.func(skill_name="my-skill", script_name="run.py")
result = await run_tool.func(skill_name="my-skill", script_name="scripts/run.py")
assert result == "source"
assert call_log == ["source"]