diff --git a/dotnet/agent-framework-dotnet.slnx b/dotnet/agent-framework-dotnet.slnx index e2ae05c3b4..c6480cf02a 100644 --- a/dotnet/agent-framework-dotnet.slnx +++ b/dotnet/agent-framework-dotnet.slnx @@ -174,6 +174,7 @@ + diff --git a/dotnet/samples/02-agents/AgentsWithFoundry/Agent_Step26_FoundryToolboxMcpSkills/Agent_Step26_FoundryToolboxMcpSkills.csproj b/dotnet/samples/02-agents/AgentsWithFoundry/Agent_Step26_FoundryToolboxMcpSkills/Agent_Step26_FoundryToolboxMcpSkills.csproj new file mode 100644 index 0000000000..5243b24af0 --- /dev/null +++ b/dotnet/samples/02-agents/AgentsWithFoundry/Agent_Step26_FoundryToolboxMcpSkills/Agent_Step26_FoundryToolboxMcpSkills.csproj @@ -0,0 +1,22 @@ + + + + Exe + net10.0 + + enable + enable + + + + + + + + + + + + + + diff --git a/dotnet/samples/02-agents/AgentsWithFoundry/Agent_Step26_FoundryToolboxMcpSkills/Program.cs b/dotnet/samples/02-agents/AgentsWithFoundry/Agent_Step26_FoundryToolboxMcpSkills/Program.cs new file mode 100644 index 0000000000..6efb8ce40e --- /dev/null +++ b/dotnet/samples/02-agents/AgentsWithFoundry/Agent_Step26_FoundryToolboxMcpSkills/Program.cs @@ -0,0 +1,93 @@ +// Copyright (c) Microsoft. All rights reserved. + +// Foundry Toolbox MCP Skills. +// +// Uses AgentSkillsProviderBuilder to discover MCP-based skills from a Foundry +// Toolbox endpoint and inject them as AIContextProviders so the agent can +// discover and use them at runtime. + +using System.Net.Http.Headers; +using Azure.AI.Projects; +using Azure.Core; +using Azure.Identity; +using Microsoft.Agents.AI; +using ModelContextProtocol.Client; + +// --- Configuration --- +string endpoint = Environment.GetEnvironmentVariable("AZURE_AI_PROJECT_ENDPOINT") + ?? throw new InvalidOperationException("AZURE_AI_PROJECT_ENDPOINT is not set."); +string deploymentName = Environment.GetEnvironmentVariable("AZURE_AI_MODEL_DEPLOYMENT_NAME") ?? "gpt-5.4-mini"; +string toolboxMcpServerUrl = Environment.GetEnvironmentVariable("FOUNDRY_TOOLBOX_MCP_SERVER_URL") + ?? throw new InvalidOperationException("FOUNDRY_TOOLBOX_MCP_SERVER_URL is not set."); + +// WARNING: DefaultAzureCredential is convenient for development but requires careful consideration in production. +// In production, consider using a specific credential (e.g., ManagedIdentityCredential) to avoid +// latency issues, unintended credential probing, and potential security risks from fallback mechanisms. +TokenCredential credential = new DefaultAzureCredential(); + +using var httpClient = new HttpClient(new BearerTokenHandler(credential, "https://ai.azure.com/.default") +{ + InnerHandler = new HttpClientHandler(), +}); + +// --- Connect to the Foundry Toolbox MCP endpoint --- +await using McpClient mcpClient = await McpClient.CreateAsync( + new HttpClientTransport( + new HttpClientTransportOptions + { + Endpoint = new Uri(toolboxMcpServerUrl), + Name = "foundry_toolbox", + TransportMode = HttpTransportMode.StreamableHttp, + AdditionalHeaders = new Dictionary + { + ["Foundry-Features"] = "Toolboxes=V1Preview", + }, + }, + httpClient)); + +// --- Discover MCP-based skills --- +var skillsProvider = new AgentSkillsProviderBuilder() + .UseMcpSkills(mcpClient) + .Build(); + +// --- Create the agent --- +AIProjectClient aiProjectClient = new(new Uri(endpoint), credential); + +AIAgent agent = aiProjectClient.AsAIAgent( + options: new ChatClientAgentOptions + { + Name = "ToolboxMcpSkillsAgent", + ChatOptions = new() + { + ModelId = deploymentName, + Instructions = "You are a helpful assistant. Use available skills to answer the user.", + }, + AIContextProviders = [skillsProvider], + }); + +// --- Interactive prompt --- +Console.Write("User: "); +string? query = Console.ReadLine(); + +if (string.IsNullOrWhiteSpace(query)) +{ + Console.WriteLine("No input provided."); + return; +} + +Console.WriteLine($"Assistant: {await agent.RunAsync(query)}"); + +// --------------------------------------------------------------------------- +// DelegatingHandler: attaches a fresh Foundry bearer token to every request +// --------------------------------------------------------------------------- +internal sealed class BearerTokenHandler(TokenCredential credential, string scope) : DelegatingHandler +{ + private readonly TokenRequestContext _tokenContext = new([scope]); + + protected override async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + AccessToken token = await credential.GetTokenAsync(this._tokenContext, cancellationToken).ConfigureAwait(false); + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token.Token); + return await base.SendAsync(request, cancellationToken).ConfigureAwait(false); + } +} diff --git a/dotnet/samples/02-agents/AgentsWithFoundry/Agent_Step26_FoundryToolboxMcpSkills/README.md b/dotnet/samples/02-agents/AgentsWithFoundry/Agent_Step26_FoundryToolboxMcpSkills/README.md new file mode 100644 index 0000000000..2e8efef8cc --- /dev/null +++ b/dotnet/samples/02-agents/AgentsWithFoundry/Agent_Step26_FoundryToolboxMcpSkills/README.md @@ -0,0 +1,32 @@ +# Foundry Toolbox MCP Skills + +This sample uses +`AgentSkillsProviderBuilder` to discover MCP-based skills from a Foundry Toolbox endpoint +and inject them as `AIContextProviders` so the agent can discover and use them at runtime. + +## What this sample demonstrates + +- Connecting to a Foundry toolbox's MCP endpoint via Streamable HTTP transport +- Injecting a fresh Azure AI bearer token (`https://ai.azure.com/.default`) on every MCP request +- Using `AgentSkillsProviderBuilder.UseMcpSkills(client)` to discover skills from the toolbox +- Injecting the discovered skills into `AIProjectClient.AsAIAgent(...)` via `AIContextProviders` + +## Prerequisites + +- A Microsoft Foundry project with a toolbox already configured +- The toolbox MCP endpoint must expose `skill://index.json` with `skill-md` entries (SEP-2640). If the resource is absent, the sample runs but the skills provider will be empty. +- Azure CLI installed and authenticated (`az login`) + +Set the following environment variables: + +```powershell +$env:AZURE_AI_PROJECT_ENDPOINT="https://your-foundry-service.services.ai.azure.com/api/projects/your-foundry-project" +$env:AZURE_AI_MODEL_DEPLOYMENT_NAME="gpt-5.4-mini" +$env:FOUNDRY_TOOLBOX_MCP_SERVER_URL="https://your-foundry-service.services.ai.azure.com/api/projects/your-project/toolboxes/your-toolbox/mcp?api-version=v1" +``` + +## Run the sample + +```powershell +dotnet run +``` diff --git a/dotnet/samples/02-agents/AgentsWithFoundry/README.md b/dotnet/samples/02-agents/AgentsWithFoundry/README.md index 3e203c1fc6..dda1e476f0 100644 --- a/dotnet/samples/02-agents/AgentsWithFoundry/README.md +++ b/dotnet/samples/02-agents/AgentsWithFoundry/README.md @@ -74,6 +74,7 @@ Some samples require extra tool-specific environment variables. See each sample | [Local MCP](./Agent_Step23_LocalMCP/) | Local MCP client with HTTP transport | | [Code interpreter file download](./Agent_Step24_CodeInterpreterFileDownload/) | Download container files generated by code interpreter | | [Foundry toolbox via MCP](./Agent_Step25_FoundryToolboxMcp/) | Use a Foundry Toolbox from a non-hosted agent via its MCP endpoint | +| [Foundry toolbox MCP skills](./Agent_Step26_FoundryToolboxMcpSkills/) | Use a Foundry Toolbox with MCP-based skills discovery (SEP-2640) via AIContextProviders | ## Running the samples diff --git a/dotnet/samples/02-agents/Harness/ConsoleReactiveComponents/AnsiEscapes.cs b/dotnet/samples/02-agents/Harness/ConsoleReactiveComponents/AnsiEscapes.cs index cf916938e7..6e5d51380d 100644 --- a/dotnet/samples/02-agents/Harness/ConsoleReactiveComponents/AnsiEscapes.cs +++ b/dotnet/samples/02-agents/Harness/ConsoleReactiveComponents/AnsiEscapes.cs @@ -72,6 +72,95 @@ public static class AnsiEscapes /// public static string ResetAttributes => "\x1b[0m"; + /// + /// Returns the visible (printed) length of a string after stripping ANSI escape sequences. + /// Escape sequences are zero-width on screen but occupy characters in the raw string. + /// + /// + /// This counts UTF-16 code units (chars) rather than terminal display cells. Emoji, + /// combining characters, variation selectors, and East Asian wide characters may be + /// measured incorrectly. For the console harness this is acceptable since content is + /// predominantly ASCII, and emoji are padded with surrounding spaces. + /// + public static int VisibleLength(string text) + { + if (string.IsNullOrEmpty(text)) + { + return 0; + } + + int length = 0; + for (int i = 0; i < text.Length; i++) + { + if (text[i] == '\x1b' && i + 1 < text.Length && text[i + 1] == '[') + { + // Skip the ESC[ and all characters up to and including the final byte (0x40–0x7E). + i += 2; + while (i < text.Length && text[i] < 0x40) + { + i++; + } + + // i now points to the final byte of the escape sequence; the for-loop will advance past it. + } + else if (text[i] != '\n' && text[i] != '\r') + { + length++; + } + } + + return length; + } + + /// + /// Counts the number of physical terminal rows a text item will occupy, + /// accounting for both explicit newlines and terminal line wrapping. + /// + /// The text to measure. + /// The terminal width in columns. If <= 0, wrapping is ignored (1 row per logical line). + /// The number of physical rows the text occupies. + public static int CountPhysicalLines(string text, int terminalWidth) + { + if (string.IsNullOrEmpty(text)) + { + return 0; + } + + int physicalLines = 0; + int lineStart = 0; + + for (int i = 0; i <= text.Length; i++) + { + if (i == text.Length || text[i] == '\n') + { + if (terminalWidth <= 0) + { + // No wrapping — each logical line is one physical row + physicalLines += 1; + } + else + { + string logicalLine = text[lineStart..i]; + int visibleWidth = VisibleLength(logicalLine); + + physicalLines += visibleWidth == 0 + ? 1 + : (visibleWidth - 1) / terminalWidth + 1; + } + + lineStart = i + 1; + } + } + + // If text ends with a newline, don't count the trailing empty line + if (text[text.Length - 1] == '\n') + { + physicalLines--; + } + + return physicalLines; + } + private static int ConsoleColorToAnsi(ConsoleColor color) => color switch { ConsoleColor.Black => 30, diff --git a/dotnet/samples/02-agents/Harness/ConsoleReactiveComponents/TextPanel.cs b/dotnet/samples/02-agents/Harness/ConsoleReactiveComponents/TextPanel.cs index 5692b58266..d63c243108 100644 --- a/dotnet/samples/02-agents/Harness/ConsoleReactiveComponents/TextPanel.cs +++ b/dotnet/samples/02-agents/Harness/ConsoleReactiveComponents/TextPanel.cs @@ -23,16 +23,18 @@ public record TextPanelProps : ConsoleReactiveProps public class TextPanel : ConsoleReactiveComponent { /// - /// Calculates the height (in lines) needed to render all items. + /// Calculates the height (in lines) needed to render all items, + /// accounting for terminal line wrapping at the specified width. /// /// The items to measure. - /// The total number of lines all items will occupy. - public static int CalculateHeight(IReadOnlyList items) + /// The terminal width in columns. When 0 or negative, wrapping is ignored. + /// The total number of physical lines all items will occupy. + public static int CalculateHeight(IReadOnlyList items, int terminalWidth = 0) { int total = 0; for (int i = 0; i < items.Count; i++) { - total += CountLines(items[i]); + total += AnsiEscapes.CountPhysicalLines(items[i], terminalWidth); } return total; @@ -47,13 +49,20 @@ public class TextPanel : ConsoleReactiveComponent 0 + ? Math.Max(1, (AnsiEscapes.VisibleLength(lines[j]) - 1) / props.Width + 1) + : 1; + Console.Write(AnsiEscapes.MoveAndEraseLine(props.Y + currentRow)); Console.Write(lines[j]); - currentRow++; + + currentRow += linePhysicalRows; + itemRow += linePhysicalRows; } } @@ -66,29 +75,4 @@ public class TextPanel : ConsoleReactiveComponent 0 ? lastItemLines - 1 : 0; // Update rendered count this._renderedCount = props.Items.Count; } - - private static int CountLines(string text) - { - if (string.IsNullOrEmpty(text)) - { - return 0; - } - - int count = 1; - for (int i = 0; i < text.Length; i++) - { - if (text[i] == '\n') - { - count++; - } - } - - // If text ends with a newline, don't count the trailing empty line - if (text[text.Length - 1] == '\n') - { - count--; - } - - return count; - } } diff --git a/dotnet/samples/02-agents/Harness/ConsoleReactiveFramework/ConsoleReactiveComponent.cs b/dotnet/samples/02-agents/Harness/ConsoleReactiveFramework/ConsoleReactiveComponent.cs index d71436e5e6..fa923efdf8 100644 --- a/dotnet/samples/02-agents/Harness/ConsoleReactiveFramework/ConsoleReactiveComponent.cs +++ b/dotnet/samples/02-agents/Harness/ConsoleReactiveFramework/ConsoleReactiveComponent.cs @@ -13,6 +13,11 @@ public abstract class ConsoleReactiveComponent { } + /// + /// Gets the shared render lock across all component types to prevent ANSI escape sequence interleaving. + /// + protected static object RenderLock { get; } = new(); + /// /// Gets or sets the component's props as the base type. /// Used by parent components to set layout (X, Y, Width, Height) on children without @@ -40,7 +45,6 @@ public abstract class ConsoleReactiveComponent : ConsoleReactive where TProps : ConsoleReactiveProps where TState : ConsoleReactiveState { - private readonly object _renderLock = new(); private TProps? _lastRenderedProps; private TState? _lastRenderedState; @@ -74,7 +78,7 @@ public abstract class ConsoleReactiveComponent : ConsoleReactive /// public override void Render() { - lock (this._renderLock) + lock (RenderLock) { if (this.Props is null) { @@ -97,7 +101,7 @@ public abstract class ConsoleReactiveComponent : ConsoleReactive /// public override void Invalidate() { - lock (this._renderLock) + lock (RenderLock) { this._lastRenderedProps = default; this._lastRenderedState = default; diff --git a/dotnet/samples/02-agents/Harness/Harness_Shared_Console/HarnessAppComponent.cs b/dotnet/samples/02-agents/Harness/Harness_Shared_Console/HarnessAppComponent.cs index d100c9d081..b2104aa5fd 100644 --- a/dotnet/samples/02-agents/Harness/Harness_Shared_Console/HarnessAppComponent.cs +++ b/dotnet/samples/02-agents/Harness/Harness_Shared_Console/HarnessAppComponent.cs @@ -28,6 +28,7 @@ public class HarnessAppComponent : ConsoleReactiveComponent /// Initializes a new instance of the class. @@ -341,7 +342,7 @@ public class HarnessAppComponent : ConsoleReactiveComponent(collectedText); } - catch (JsonException ex) + catch (JsonException) { - await ux.WriteInfoLineAsync($"❌ Failed to parse planning response: {ex.Message}", ConsoleColor.Red); - await ux.WriteInfoLineAsync($"(raw response) {collectedText}", ConsoleColor.DarkYellow); + // JSON parsing failed — fall back to rendering as regular text output. + await ux.WriteTextAsync(collectedText).ConfigureAwait(false); return null; } if (planningResponse is null) { - await ux.WriteInfoLineAsync("(no structured response from agent)", ConsoleColor.DarkYellow); + // Null result — fall back to rendering as regular text output. + await ux.WriteTextAsync(collectedText).ConfigureAwait(false); return null; } @@ -118,7 +119,8 @@ public sealed class PlanningOutputObserver : ConsoleObserver return new List { this.BuildApprovalAction(question, session) }; } - await ux.WriteInfoLineAsync($"(unexpected response type: {planningResponse.Type})", ConsoleColor.DarkYellow); + // Unexpected type — fall back to rendering as regular text output. + await ux.WriteTextAsync(collectedText).ConfigureAwait(false); return null; } diff --git a/dotnet/samples/02-agents/Harness/Harness_Shared_Console_OpenAI/OpenAIResponsesErrorObserver.cs b/dotnet/samples/02-agents/Harness/Harness_Shared_Console_OpenAI/OpenAIResponsesErrorObserver.cs index 9db00c9a65..8cf5abee57 100644 --- a/dotnet/samples/02-agents/Harness/Harness_Shared_Console_OpenAI/OpenAIResponsesErrorObserver.cs +++ b/dotnet/samples/02-agents/Harness/Harness_Shared_Console_OpenAI/OpenAIResponsesErrorObserver.cs @@ -2,6 +2,7 @@ #pragma warning disable OPENAI001 // Suppress experimental API warnings for Responses API usage. +using System.Text.Json; using Harness.Shared.Console.Observers; using Microsoft.Agents.AI; using Microsoft.Extensions.AI; @@ -53,9 +54,94 @@ public sealed class OpenAIResponsesErrorObserver : ConsoleObserver case StreamingResponseIncompleteUpdate incompleteUpdate: string? reason = incompleteUpdate.Response?.IncompleteStatusDetails?.Reason?.ToString(); - string incompleteText = $"⚠️ Response incomplete: {reason ?? "unknown reason"}"; - await ux.WriteInfoLineAsync(incompleteText, ConsoleColor.Yellow); + if (string.Equals(reason, "content_filter", StringComparison.OrdinalIgnoreCase)) + { + string detail = GetContentFilterDetails(incompleteUpdate); + const string Message = "🛡️ The service's built-in content filter guardrails were triggered and the response was cut short."; + await ux.WriteInfoLineAsync( + string.IsNullOrEmpty(detail) ? Message : $"{Message}\n{detail}", + ConsoleColor.Yellow); + } + else + { + string incompleteText = $"⚠️ Response incomplete: {reason ?? "unknown reason"}"; + await ux.WriteInfoLineAsync(incompleteText, ConsoleColor.Yellow); + } + break; } } + + /// + /// Extracts content filter details from the serialized response JSON and returns + /// a formatted string showing which specific categories were triggered. + /// Returns if details cannot be extracted. + /// + private static string GetContentFilterDetails(StreamingResponseIncompleteUpdate incompleteUpdate) + { + try + { + var data = System.ClientModel.Primitives.ModelReaderWriter.Write(incompleteUpdate); + using var doc = JsonDocument.Parse(data.ToString()); + var root = doc.RootElement; + + // Navigate into the nested response object if present. + JsonElement responseElement = root.TryGetProperty("response", out var resp) ? resp : root; + + if (!responseElement.TryGetProperty("content_filters", out var filtersArray) + || filtersArray.ValueKind != JsonValueKind.Array) + { + return string.Empty; + } + + foreach (var filter in filtersArray.EnumerateArray()) + { + if (!filter.TryGetProperty("content_filter_results", out var results) + || results.ValueKind != JsonValueKind.Object) + { + continue; + } + + // Collect category data for aligned output. + var categories = new List<(string Name, bool Filtered, string? Severity)>(); + foreach (var category in results.EnumerateObject()) + { + if (category.Value.ValueKind != JsonValueKind.Object) + { + continue; + } + + bool filtered = category.Value.TryGetProperty("filtered", out var f) && f.GetBoolean(); + string? severity = category.Value.TryGetProperty("severity", out var s) ? s.GetString() : null; + categories.Add((category.Name, filtered, severity)); + } + + // Build all category lines into a single string. + int maxNameLen = categories.Count > 0 ? categories.Max(c => c.Name.Length) : 0; + var lines = new List(); + + foreach (var (name, filtered, severity) in categories) + { + string paddedName = name.PadRight(maxNameLen); + string icon = filtered ? "❌" : "✅"; + string statusText = filtered ? "Filtered " : "Not Filtered"; + string severityText = severity is not null ? $" Severity: {severity}" : ""; + + lines.Add($" {icon} {paddedName} {statusText}{severityText}"); + } + + if (lines.Count > 0) + { + return string.Join("\n", lines); + } + } + + return string.Empty; + } + catch + { + // Parsing not critical — skip silently if it fails. + return string.Empty; + } + } } diff --git a/python/packages/core/agent_framework/_skills.py b/python/packages/core/agent_framework/_skills.py index 6268c00879..683302b13a 100644 --- a/python/packages/core/agent_framework/_skills.py +++ b/python/packages/core/agent_framework/_skills.py @@ -507,38 +507,45 @@ class Skill(ABC): """ ... - @property @abstractmethod - def content(self) -> str: - """The full skill content. + async def get_content(self) -> str: + """Get the full skill content. For file-based skills this is the raw SKILL.md file content, optionally augmented with a synthesized scripts block when scripts are present. For code-defined skills this is a synthesized XML document containing name, description, and body (instructions, resources, scripts). + + Returns: + The full skill content string. """ ... - @property - def resources(self) -> list[SkillResource]: - """Resources associated with this skill. + async def get_resource(self, name: str) -> SkillResource | None: + """Get a resource owned by this skill by name. - The default implementation returns an empty list. - Override this property in derived classes to provide skill-specific - resources. + Args: + name: The resource name (e.g. an identifier or a relative path + referenced inside the skill content). + + Returns: + The :class:`SkillResource`, or ``None`` when no resource with the + given name exists. """ - return [] + return None - @property - def scripts(self) -> list[SkillScript]: - """Scripts associated with this skill. + async def get_script(self, name: str) -> SkillScript | None: + """Get a script owned by this skill by name. - The default implementation returns an empty list. - Override this property in derived classes to provide skill-specific - scripts. + Args: + name: The script name. + + Returns: + The :class:`SkillScript`, or ``None`` when no script with the + given name exists. """ - return [] + return None @experimental(feature_id=ExperimentalFeature.SKILLS) @@ -767,12 +774,14 @@ class InlineSkill(Skill): """The L1 discovery metadata for this skill.""" return self._frontmatter - @property - def content(self) -> str: + async def get_content(self) -> str: """Synthesized XML content with name, description, instructions, resources, and scripts. The result is cached after the first access. Adding resources or scripts after the first access will not be reflected. + + Returns: + The synthesized XML content string. """ if self._cached_content is not None: return self._cached_content @@ -786,15 +795,31 @@ class InlineSkill(Skill): ) return self._cached_content - @property - def resources(self) -> list[SkillResource]: - """Mutable list of :class:`SkillResource` instances.""" - return self._resources + async def get_resource(self, name: str) -> SkillResource | None: + """Get a resource by name. - @property - def scripts(self) -> list[SkillScript]: - """Mutable list of :class:`SkillScript` instances.""" - return self._scripts + Args: + name: The resource name to look up (case-insensitive). + + Returns: + The :class:`SkillResource`, or ``None`` when no resource with the + given name exists. + """ + name_lower = name.lower() + return next((r for r in self._resources if r.name.lower() == name_lower), None) + + async def get_script(self, name: str) -> SkillScript | None: + """Get a script by name. + + Args: + name: The script name to look up (case-insensitive). + + Returns: + The :class:`SkillScript`, or ``None`` when no script with the + given name exists. + """ + name_lower = name.lower() + return next((s for s in self._scripts if s.name.lower() == name_lower), None) def resource( self, @@ -1318,11 +1343,13 @@ class ClassSkill(Skill, ABC): self._cached_scripts = scripts return list(self._cached_scripts) - @property - def content(self) -> str: + async def get_content(self) -> str: """Synthesized XML content containing name, description, instructions, resources, and scripts. The result is cached after the first access. + + Returns: + The synthesized XML content string. """ if self._cached_content is not None: return self._cached_content @@ -1336,6 +1363,32 @@ class ClassSkill(Skill, ABC): ) return self._cached_content + async def get_resource(self, name: str) -> SkillResource | None: + """Get a resource by name from the :attr:`resources` list. + + Args: + name: The resource name to look up (case-insensitive). + + Returns: + The :class:`SkillResource`, or ``None`` when no resource with the + given name exists. + """ + name_lower = name.lower() + return next((r for r in self.resources if r.name.lower() == name_lower), None) + + async def get_script(self, name: str) -> SkillScript | None: + """Get a script by name from the :attr:`scripts` list. + + Args: + name: The script name to look up (case-insensitive). + + Returns: + The :class:`SkillScript`, or ``None`` when no script with the + given name exists. + """ + name_lower = name.lower() + return next((s for s in self.scripts if s.name.lower() == name_lower), None) + @experimental(feature_id=ExperimentalFeature.SKILLS) class FileSkill(Skill): @@ -1378,8 +1431,7 @@ class FileSkill(Skill): """The L1 discovery metadata for this skill.""" return self._frontmatter - @property - def content(self) -> str: + async def get_content(self) -> str: """The skill content with appended scripts block. When scripts are present, a ```` XML block is appended @@ -1388,6 +1440,9 @@ class FileSkill(Skill): The result is cached after the first access. Adding scripts after the first access will not be reflected. + + Returns: + The skill content string. """ if self._cached_content is not None: return self._cached_content @@ -1398,15 +1453,31 @@ class FileSkill(Skill): self._cached_content = f"{self._content}\n\n\n{script_lines}\n" return self._cached_content - @property - def resources(self) -> list[SkillResource]: - """Resources discovered for this skill.""" - return self._resources + async def get_resource(self, name: str) -> SkillResource | None: + """Get a resource by name. - @property - def scripts(self) -> list[SkillScript]: - """Scripts discovered for this skill.""" - return self._scripts + Args: + name: The resource name to look up (case-insensitive). + + Returns: + The :class:`SkillResource`, or ``None`` when no resource with the + given name exists. + """ + name_lower = name.lower() + return next((r for r in self._resources if r.name.lower() == name_lower), None) + + async def get_script(self, name: str) -> SkillScript | None: + """Get a script by name. + + Args: + name: The script name to look up (case-insensitive). + + Returns: + The :class:`SkillScript`, or ``None`` when no script with the + given name exists. + """ + name_lower = name.lower() + return next((s for s in self._scripts if s.name.lower() == name_lower), None) # endregion @@ -1734,13 +1805,13 @@ class SkillsProvider(ContextProvider): Keyword Args: instruction_template: Custom system-prompt template for advertising skills. Must contain a ``{skills}`` placeholder for the - generated skills list. If the provider includes file-based script - execution instructions, the template must also contain - ``{runner_instructions}``. If the provider includes resource-reading - instructions, the template must also contain - ``{resource_instructions}``. Omitting any placeholder required by - the resolved skills configuration can raise :class:`ValueError` at - runtime. Uses a built-in template when ``None``. + generated skills list. May optionally contain + ``{runner_instructions}`` and/or ``{resource_instructions}`` + placeholders; when present, they are filled with built-in + guidance for script execution and resource reading respectively. + When omitted, those instructions are simply not included in the + rendered prompt (the corresponding tools are still registered). + Uses a built-in template when ``None``. require_script_approval: When ``True``, skill script execution requires explicit user approval before running. Instead of executing immediately, the agent pauses and returns a @@ -1867,29 +1938,20 @@ class SkillsProvider(ContextProvider): def _create_instructions( prompt_template: str | None, skills: Sequence[Skill], - include_script_runner_instructions: bool = False, - include_resource_instructions: bool = False, ) -> str | None: """Create the system-prompt text that advertises available skills. Generates an XML list of ```` elements (sorted by name) and inserts it into *prompt_template* at the ``{skills}`` placeholder. - When *include_script_runner_instructions* is ``True``, executor-provided - instructions are inserted at the ``{runner_instructions}`` placeholder. - When *include_resource_instructions* is ``True``, resource-reading - instructions are inserted at the ``{resource_instructions}`` placeholder. + Script-runner instructions are inserted at the + ``{runner_instructions}`` placeholder and resource-reading + instructions at the ``{resource_instructions}`` placeholder. Args: prompt_template: Custom template string with ``{skills}`` and optional ``{runner_instructions}`` and ``{resource_instructions}`` placeholders, or ``None`` to use the built-in default. skills: Registered skills. - include_script_runner_instructions: When ``True``, include - script-runner instructions in the generated prompt. - Defaults to ``False``. - include_resource_instructions: When ``True``, include - resource-reading instructions in the generated prompt. - Defaults to ``False``. Returns: The formatted instruction string, or ``None`` when *skills* is empty. @@ -1898,8 +1960,8 @@ class SkillsProvider(ContextProvider): ValueError: If *prompt_template* is not a valid format string (e.g. missing ``{skills}`` placeholder). """ - runner_instructions = SCRIPT_RUNNER_INSTRUCTIONS if include_script_runner_instructions else None - resource_instructions = RESOURCE_INSTRUCTIONS if include_resource_instructions else None + runner_instructions = SCRIPT_RUNNER_INSTRUCTIONS + resource_instructions = RESOURCE_INSTRUCTIONS template = DEFAULT_SKILLS_INSTRUCTION_PROMPT if prompt_template is not None: @@ -1921,16 +1983,6 @@ class SkillsProvider(ContextProvider): raise ValueError( "The provided instruction_template must contain a '{skills}' placeholder." # noqa: RUF027 ) - if runner_instructions and "__EXEC_PROBE__" not in result: - raise ValueError( - "The provided instruction_template must contain an '{runner_instructions}' placeholder " # noqa: RUF027 - "when a script runner is configured." - ) - if resource_instructions and "__RES_PROBE__" not in result: - raise ValueError( - "The provided instruction_template must contain a '{resource_instructions}' placeholder " # noqa: RUF027 - "when skills have resources." - ) template = prompt_template if not skills: @@ -1964,20 +2016,13 @@ class SkillsProvider(ContextProvider): if not skills: return skills, None, [] - has_scripts = any(s.scripts for s in skills) - has_resources = any(s.resources for s in skills) - instructions = self._create_instructions( prompt_template=self._instruction_template, skills=skills, - include_script_runner_instructions=has_scripts, - include_resource_instructions=has_resources, ) tools = self._create_tools( skills=skills, - include_script_runner_tool=has_scripts, - include_resource_tool=has_resources, require_script_approval=self._require_script_approval, ) @@ -2046,23 +2091,15 @@ class SkillsProvider(ContextProvider): def _create_tools( self, skills: Sequence[Skill], - include_script_runner_tool: bool, - include_resource_tool: bool, require_script_approval: bool = False, ) -> list[FunctionTool]: """Create the tool definitions for skill interaction. - Always includes ``load_skill``. Conditionally includes - ``read_skill_resource`` (when *include_resource_tool* is ``True``) - and ``run_skill_script`` (when *include_script_runner_tool* is - ``True``). + Always includes ``load_skill``, ``read_skill_resource``, and + ``run_skill_script``. Args: skills: The skills to bind to tool handlers. - include_script_runner_tool: Whether to include the - ``run_skill_script`` tool in the returned list. - include_resource_tool: Whether to include the - ``read_skill_resource`` tool in the returned list. require_script_approval: When ``True``, the ``run_skill_script`` tool pauses for user approval before each invocation. @@ -2070,11 +2107,23 @@ class SkillsProvider(ContextProvider): Returns: A list of :class:`FunctionTool` instances. """ - tools = [ + + async def _load(skill_name: str) -> str: + return await self._load_skill(skills, skill_name) + + async def _read_resource(skill_name: str, resource_name: str, **kwargs: Any) -> Any: + return await self._read_skill_resource(skills, skill_name, resource_name, **kwargs) + + async def _run_script( + skill_name: str, script_name: str, args: dict[str, Any] | list[str] | None = None, **kwargs: Any + ) -> Any: + return await self._run_skill_script(skills, skill_name, script_name, args, **kwargs) + + return [ FunctionTool( name="load_skill", description="Loads the full instructions for a specific skill.", - func=lambda skill_name: self._load_skill(skills, skill_name), # pyright: ignore[reportUnknownArgumentType, reportUnknownLambdaType] + func=_load, input_model={ "type": "object", "properties": { @@ -2083,108 +2132,88 @@ class SkillsProvider(ContextProvider): "required": ["skill_name"], }, ), + FunctionTool( + name="read_skill_resource", + description=( + "Reads a resource associated with a skill, such as references, assets, or dynamic data." + ), + func=_read_resource, + input_model={ + "type": "object", + "properties": { + "skill_name": {"type": "string", "description": "The name of the skill."}, + "resource_name": { + "type": "string", + "description": "The name of the resource.", + }, + }, + "required": ["skill_name", "resource_name"], + }, + ), + FunctionTool( + name="run_skill_script", + description="Runs a script associated with a skill.", + func=_run_script, + approval_mode="always_require" if require_script_approval else "never_require", + input_model={ + "type": "object", + "properties": { + "skill_name": {"type": "string", "description": "The name of the skill."}, + "script_name": { + "type": "string", + "description": ( + "The name of the script to run as listed in the skill, " + "preserving any directory prefix exactly as shown. " + "Do not add or remove path prefixes." + ), + }, + "args": { + "oneOf": [ + { + "type": "object", + "additionalProperties": True, + "description": ( + "Named arguments as key-value pairs " + '(e.g. {"length": 24, "uppercase": true}).' + ), + }, + { + "type": "array", + "items": {"type": "string"}, + "description": ( + "Positional CLI arguments as a string array " + '(e.g. ["input.docx", "--output", "result.idx"]).' + ), + }, + {"type": "null"}, + ], + "default": None, + "description": ( + "Arguments to pass to the script. " + "Use an array of strings for CLI-style positional arguments " + '(e.g. ["input.docx", "--output", "result.idx"]), ' + "or an object for named parameters " + '(e.g. {"length": 24, "uppercase": true}). ' + "How these values are mapped to the underlying script " + "is determined by the script implementation or configured runner." + ), + }, + }, + "required": ["skill_name", "script_name"], + }, + ), ] - if include_resource_tool: - - async def _read_resource(skill_name: str, resource_name: str, **kwargs: Any) -> Any: - return await self._read_skill_resource(skills, skill_name, resource_name, **kwargs) - - tools.append( - FunctionTool( - name="read_skill_resource", - description=( - "Reads a resource associated with a skill, such as references, assets, or dynamic data." - ), - func=_read_resource, - input_model={ - "type": "object", - "properties": { - "skill_name": {"type": "string", "description": "The name of the skill."}, - "resource_name": { - "type": "string", - "description": "The name of the resource.", - }, - }, - "required": ["skill_name", "resource_name"], - }, - ) - ) - - if include_script_runner_tool: - - async def _run_script( - skill_name: str, script_name: str, args: dict[str, Any] | list[str] | None = None, **kwargs: Any - ) -> Any: - return await self._run_skill_script(skills, skill_name, script_name, args, **kwargs) - - tools.append( - FunctionTool( - name="run_skill_script", - description="Runs a script associated with a skill.", - func=_run_script, - approval_mode="always_require" if require_script_approval else "never_require", - input_model={ - "type": "object", - "properties": { - "skill_name": {"type": "string", "description": "The name of the skill."}, - "script_name": { - "type": "string", - "description": ( - "The name of the script to run as listed in the skill, " - "preserving any directory prefix exactly as shown. " - "Do not add or remove path prefixes." - ), - }, - "args": { - "oneOf": [ - { - "type": "object", - "additionalProperties": True, - "description": ( - "Named arguments as key-value pairs " - '(e.g. {"length": 24, "uppercase": true}).' - ), - }, - { - "type": "array", - "items": {"type": "string"}, - "description": ( - "Positional CLI arguments as a string array " - '(e.g. ["input.docx", "--output", "result.idx"]).' - ), - }, - {"type": "null"}, - ], - "default": None, - "description": ( - "Arguments to pass to the script. " - "Use an array of strings for CLI-style positional arguments " - '(e.g. ["input.docx", "--output", "result.idx"]), ' - "or an object for named parameters " - '(e.g. {"length": 24, "uppercase": true}). ' - "How these values are mapped to the underlying script " - "is determined by the script implementation or configured runner." - ), - }, - }, - "required": ["skill_name", "script_name"], - }, - ) - ) - - return tools - @staticmethod def _find_skill(skills: Sequence[Skill], name: str) -> Skill | None: """Find a skill by name (case-insensitive linear scan).""" name_lower = name.lower() return next((s for s in skills if s.frontmatter.name.lower() == name_lower), None) - def _load_skill(self, skills: Sequence[Skill], skill_name: str) -> str: + async def _load_skill(self, skills: Sequence[Skill], skill_name: str) -> str: """Return the full content for the named skill. - Delegates to the skill's :attr:`~Skill.content` property, which + Delegates to the skill's :meth:`~Skill.get_content` method, which handles format differences between file-based and code-defined skills. Args: @@ -2204,7 +2233,7 @@ class SkillsProvider(ContextProvider): logger.info("Loading skill: %s", skill_name) - return skill.content + return await skill.get_content() async def _run_skill_script( self, @@ -2243,7 +2272,7 @@ class SkillsProvider(ContextProvider): if not skill: return f"Error: Skill '{skill_name}' not found." - script = next((s for s in skill.scripts if s.name.lower() == script_name.lower()), None) + script = await skill.get_script(script_name) if not script: return f"Error: Script '{script_name}' not found in skill '{skill_name}'." @@ -2284,12 +2313,8 @@ class SkillsProvider(ContextProvider): if skill is None: return f"Error: Skill '{skill_name}' not found." - # Find resource by name (case-insensitive) - resource_name_lower = resource_name.lower() - for resource in skill.resources: - if resource.name.lower() == resource_name_lower: - break - else: + resource = await skill.get_resource(resource_name) + if resource is None: return f"Error: Resource '{resource_name}' not found in skill '{skill_name}'." try: @@ -2481,27 +2506,29 @@ class FileSkillsSource(SkillsSource): ) continue - file_skill = FileSkill( - frontmatter=frontmatter, - content=content, - path=skill_path, - ) - - # Discover and attach file-based resources + # Discover file-based resources + resources: list[SkillResource] = [] 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)) + resources.append(_FileSkillResource(name=rn, full_path=resource_full_path)) - # Discover and attach file-based scripts as SkillScript instances + # Discover file-based scripts + scripts: list[SkillScript] = [] 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) - ) + scripts.append(FileSkillScript(name=sn, full_path=script_full_path, runner=self._script_runner)) + + file_skill = FileSkill( + frontmatter=frontmatter, + content=content, + path=skill_path, + resources=resources, + scripts=scripts, + ) skills[file_skill.frontmatter.name] = file_skill logger.info("Loaded skill: %s", file_skill.frontmatter.name) diff --git a/python/packages/core/tests/core/test_skills.py b/python/packages/core/tests/core/test_skills.py index 415d6ea857..17fb2cf5ce 100644 --- a/python/packages/core/tests/core/test_skills.py +++ b/python/packages/core/tests/core/test_skills.py @@ -451,7 +451,7 @@ class TestDiscoverAndLoadSkills: _write_skill(dir2, "my-skill", body="Second") skills = await _discover_file_skills_for_test([str(dir1), str(dir2)]) assert len(skills) == 1 - assert "First" in skills["my-skill"].content + assert "First" in (await skills["my-skill"].get_content()) async def test_empty_directory(self, tmp_path: Path) -> None: skills = await _discover_file_skills_for_test([str(tmp_path)]) @@ -489,7 +489,7 @@ class TestDiscoverAndLoadSkills: ) 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] == ["references/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.""" @@ -501,7 +501,7 @@ class TestDiscoverAndLoadSkills: ) 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) + resource_names = sorted(r.name for r in skills["my-skill"]._resources) assert "assets/doc.md" in resource_names assert "references/data.json" in resource_names @@ -676,9 +676,9 @@ class TestSkillsProvider: assert len(context.instructions) == 1 assert "my-skill" in context.instructions[0] - assert len(context.tools) == 1 tool_names = {t.name for t in context.tools} - assert tool_names == {"load_skill"} + assert len(context.tools) == 3 + assert tool_names == {"load_skill", "read_skill_resource", "run_skill_script"} async def test_before_run_without_skills(self, tmp_path: Path) -> None: provider = SkillsProvider.from_paths(str(tmp_path)) @@ -698,7 +698,7 @@ class TestSkillsProvider: _write_skill(tmp_path, "my-skill", body="Skill body content.") provider = SkillsProvider.from_paths(str(tmp_path)) await _init_provider(provider) - result = provider._load_skill(_raw_skills(provider), "my-skill") + result = await provider._load_skill(_raw_skills(provider), "my-skill") assert "Skill body content." in result async def test_load_skill_preserves_file_skill_content(self, tmp_path: Path) -> None: @@ -710,19 +710,19 @@ class TestSkillsProvider: ) provider = SkillsProvider.from_paths(str(tmp_path)) await _init_provider(provider) - result = provider._load_skill(_raw_skills(provider), "my-skill") + result = await provider._load_skill(_raw_skills(provider), "my-skill") 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)) await _init_provider(provider) - result = provider._load_skill(_raw_skills(provider), "nonexistent") + result = await provider._load_skill(_raw_skills(provider), "nonexistent") assert result.startswith("Error:") async def test_load_skill_empty_name_returns_error(self, tmp_path: Path) -> None: provider = SkillsProvider.from_paths(str(tmp_path)) await _init_provider(provider) - result = provider._load_skill(_raw_skills(provider), "") + result = await provider._load_skill(_raw_skills(provider), "") assert result.startswith("Error:") async def test_read_skill_resource_returns_content(self, tmp_path: Path) -> None: @@ -871,7 +871,7 @@ 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] + resource_names = [r.name for r in skills["my-skill"]._resources] assert "references/leak.md" not in resource_names assert "references/safe.md" in resource_names @@ -1032,7 +1032,7 @@ class TestInlineSkill: assert skill.frontmatter.name == "my-skill" assert skill.frontmatter.description == "A test skill." assert skill.instructions == "Instructions." - assert skill.resources == [] + assert skill._resources == [] def test_construction_with_static_resources(self) -> None: skill = InlineSkill( @@ -1042,8 +1042,8 @@ class TestInlineSkill: InlineSkillResource(name="ref", content="Reference content"), ], ) - assert len(skill.resources) == 1 - assert skill.resources[0].name == "ref" + assert len(skill._resources) == 1 + assert skill._resources[0].name == "ref" def test_empty_name_raises(self) -> None: with pytest.raises(ValueError, match="cannot be empty"): @@ -1083,11 +1083,11 @@ class TestInlineSkill: """Get the database schema.""" return "CREATE TABLE users (id INT)" - assert len(skill.resources) == 1 - assert skill.resources[0].name == "get_schema" - assert skill.resources[0].description is None - assert isinstance(skill.resources[0], InlineSkillResource) - assert skill.resources[0].function is get_schema + assert len(skill._resources) == 1 + assert skill._resources[0].name == "get_schema" + assert skill._resources[0].description is None + assert isinstance(skill._resources[0], InlineSkillResource) + assert skill._resources[0].function is get_schema def test_resource_decorator_with_args(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="A skill."), instructions="Body") @@ -1096,9 +1096,9 @@ class TestInlineSkill: def my_resource() -> Any: return "data" - assert len(skill.resources) == 1 - assert skill.resources[0].name == "custom-name" - assert skill.resources[0].description == "Custom description" + assert len(skill._resources) == 1 + assert skill._resources[0].name == "custom-name" + assert skill._resources[0].description == "Custom description" def test_resource_decorator_returns_function(self) -> None: """Decorator should return the original function unchanged.""" @@ -1122,8 +1122,8 @@ class TestInlineSkill: def resource_b() -> Any: return "B" - assert len(skill.resources) == 2 - names = [r.name for r in skill.resources] + assert len(skill._resources) == 2 + names = [r.name for r in skill._resources] assert "resource_a" in names assert "resource_b" in names @@ -1134,9 +1134,9 @@ class TestInlineSkill: async def get_async_data() -> Any: return "async data" - assert len(skill.resources) == 1 - assert isinstance(skill.resources[0], InlineSkillResource) - assert skill.resources[0].function is get_async_data + assert len(skill._resources) == 1 + assert isinstance(skill._resources[0], InlineSkillResource) + assert skill._resources[0].function is get_async_data # --------------------------------------------------------------------------- @@ -1163,7 +1163,7 @@ class TestSkillsProviderCodeSkill: ) provider = SkillsProvider([skill]) await _init_provider(provider) - result = provider._load_skill(_raw_skills(provider), "prog-skill") + result = await provider._load_skill(_raw_skills(provider), "prog-skill") assert "prog-skill" in result assert "A skill." in result assert "\nCode-defined instructions.\n" in result @@ -1180,7 +1180,7 @@ class TestSkillsProviderCodeSkill: ) provider = SkillsProvider([skill]) await _init_provider(provider) - result = provider._load_skill(_raw_skills(provider), "prog-skill") + result = await provider._load_skill(_raw_skills(provider), "prog-skill") assert "prog-skill" in result assert "A skill." in result assert "Do things." in result @@ -1194,7 +1194,7 @@ class TestSkillsProviderCodeSkill: ) provider = SkillsProvider([skill]) await _init_provider(provider) - result = provider._load_skill(_raw_skills(provider), "prog-skill") + result = await provider._load_skill(_raw_skills(provider), "prog-skill") assert "Body only." in result assert "" not in result @@ -1364,7 +1364,8 @@ class TestSkillsProviderCodeSkill: assert len(context.instructions) == 1 assert "prog-skill" in context.instructions[0] - assert len(context.tools) == 1 + assert len(context.tools) == 3 + assert {t.name for t in context.tools} == {"load_skill", "read_skill_resource", "run_skill_script"} async def test_before_run_empty_provider(self) -> None: provider = SkillsProvider([]) @@ -1407,7 +1408,7 @@ class TestSkillsProviderCodeSkill: ) await _init_provider(provider) # File-based is loaded first, so it wins - assert "File version" in _ctx(provider)[0]["my-skill"].content + assert "File version" in (await _ctx(provider)[0]["my-skill"].get_content()) async def test_combined_prompt_includes_both(self, tmp_path: Path) -> None: _write_skill(tmp_path, "file-skill") @@ -1446,7 +1447,7 @@ class TestSkillsProviderCodeSkill: 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] + resource_names = [r.name for r in skill._resources] assert "references/data.json" in resource_names assert "references/notes.txt" not in resource_names @@ -1459,14 +1460,14 @@ class TestSkillsProviderCodeSkill: class TestFileBasedSkillParsing: """Tests for file-based skills parsed from SKILL.md.""" - def test_content_contains_full_raw_file(self, tmp_path: Path) -> None: + async def test_content_contains_full_raw_file(self, tmp_path: Path) -> None: """content stores the entire SKILL.md file including frontmatter.""" _write_skill(tmp_path, "my-skill", description="A test skill.", body="Instructions here.") skill = _read_and_parse_skill_file_for_test(tmp_path / "my-skill") - assert "---" in skill.content - assert "name: my-skill" in skill.content - assert "description: A test skill." in skill.content - assert "Instructions here." in skill.content + assert "---" in (await skill.get_content()) + assert "name: my-skill" in (await skill.get_content()) + assert "description: A test skill." in (await skill.get_content()) + assert "Instructions here." in (await skill.get_content()) def test_name_and_description_from_frontmatter(self, tmp_path: Path) -> None: _write_skill(tmp_path, "my-skill", description="Skill desc.") @@ -1483,7 +1484,7 @@ class TestFileBasedSkillParsing: _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] + resource_names = [r.name for r in skills["my-skill"]._resources] assert "references/doc.md" in resource_names @@ -1500,7 +1501,7 @@ class TestLoadSkillFormatting: _write_skill(tmp_path, "my-skill", body="Do the thing.") provider = SkillsProvider.from_paths(str(tmp_path)) await _init_provider(provider) - result = provider._load_skill(_raw_skills(provider), "my-skill") + result = await provider._load_skill(_raw_skills(provider), "my-skill") assert "Do the thing." in result assert "" not in result assert "" not in result @@ -1512,7 +1513,7 @@ class TestLoadSkillFormatting: ) provider = SkillsProvider([skill]) await _init_provider(provider) - result = provider._load_skill(_raw_skills(provider), "prog-skill") + result = await provider._load_skill(_raw_skills(provider), "prog-skill") assert "prog-skill" in result assert "A skill." in result assert "\nDo stuff.\n" in result @@ -1526,7 +1527,7 @@ class TestLoadSkillFormatting: ) provider = SkillsProvider([skill]) await _init_provider(provider) - result = provider._load_skill(_raw_skills(provider), "prog-skill") + result = await provider._load_skill(_raw_skills(provider), "prog-skill") assert '' in result assert "description=" not in result @@ -1708,7 +1709,7 @@ class TestFileSkillsSourceDirectories: source = FileSkillsSource(str(tmp_path), resource_directories=["docs"]) skills = await source.get_skills() - resource_names = [r.name for r in skills[0].resources] + 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 @@ -1727,7 +1728,7 @@ class TestFileSkillsSourceDirectories: source = FileSkillsSource(str(tmp_path), script_directories=["tools"]) skills = await source.get_skills() - script_names = [s.name for s in skills[0].scripts] + 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: @@ -1742,7 +1743,7 @@ class TestFileSkillsSourceDirectories: source = FileSkillsSource(str(tmp_path), resource_directories=[".", "references"]) skills = await source.get_skills() - resource_names = [r.name for r in skills[0].resources] + 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: @@ -1762,7 +1763,7 @@ class TestFileSkillsSourceDirectories: ) await _init_provider(provider) skill = _ctx(provider)[0]["my-skill"] - resource_names = [r.name for r in skill.resources] + resource_names = [r.name for r in skill._resources] assert "docs/guide.md" in resource_names @@ -2602,40 +2603,46 @@ class TestCreateInstructionsEdgeCases: assert alpha_pos < bravo_pos < charlie_pos def test_custom_template_missing_runner_instructions_raises(self) -> None: - """Custom template without {runner_instructions} raises when scripts are enabled.""" + """Custom templates may omit {runner_instructions}.""" skills = [ InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="Skill."), instructions="Body"), ] template = "Skills: {skills}" - with pytest.raises(ValueError, match="runner_instructions"): - SkillsProvider._create_instructions(template, skills, include_script_runner_instructions=True) + result = SkillsProvider._create_instructions(template, skills) + assert result is not None + assert result.startswith("Skills: ") + assert "my-skill" in result + assert "Skill." in result def test_custom_template_missing_resource_instructions_raises(self) -> None: - """Custom template without {resource_instructions} raises when resources exist.""" + """Custom templates may omit {resource_instructions}.""" skills = [ InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="Skill."), instructions="Body"), ] template = "Skills: {skills}" - with pytest.raises(ValueError, match="resource_instructions"): - SkillsProvider._create_instructions(template, skills, include_resource_instructions=True) + result = SkillsProvider._create_instructions(template, skills) + assert result is not None + assert result.startswith("Skills: ") + assert "my-skill" in result + assert "Skill." in result def test_include_resource_instructions_true_adds_resource_text(self) -> None: - """When include_resource_instructions is True, resource instructions appear in the prompt.""" + """Resource instructions always appear in the prompt.""" skills = [ InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="Skill."), instructions="Body"), ] - result = SkillsProvider._create_instructions(None, skills, include_resource_instructions=True) + result = SkillsProvider._create_instructions(None, skills) assert result is not None assert "read_skill_resource" in result def test_include_resource_instructions_false_omits_resource_text(self) -> None: - """When include_resource_instructions is False, resource instructions do not appear.""" + """Resource instructions are still included by default.""" skills = [ InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="Skill."), instructions="Body"), ] - result = SkillsProvider._create_instructions(None, skills, include_resource_instructions=False) + result = SkillsProvider._create_instructions(None, skills) assert result is not None - assert "read_skill_resource" not in result + assert "read_skill_resource" in result def test_custom_template_with_unknown_placeholder_raises(self) -> None: """Template with an unknown placeholder raises ValueError.""" @@ -2646,6 +2653,39 @@ class TestCreateInstructionsEdgeCases: with pytest.raises(ValueError, match="valid format string"): SkillsProvider._create_instructions(template, skills) + def test_custom_template_with_all_placeholders_fills_them(self) -> None: + """Custom template with all three placeholders fills each one.""" + skills = [ + InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="Skill."), instructions="Body"), + ] + template = "SKILLS:{skills}\nRUNNER:{runner_instructions}\nRESOURCE:{resource_instructions}" + result = SkillsProvider._create_instructions(template, skills) + assert result is not None + assert "my-skill" in result + assert "run_skill_script" in result + assert "read_skill_resource" in result + + def test_custom_template_omitting_runner_excludes_runner_text(self) -> None: + """Omitting {runner_instructions} from a custom template excludes script guidance.""" + skills = [ + InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="Skill."), instructions="Body"), + ] + template = "Skills: {skills}" + result = SkillsProvider._create_instructions(template, skills) + assert result is not None + assert "run_skill_script" not in result + + def test_custom_template_omitting_resource_excludes_resource_text(self) -> None: + """Omitting {resource_instructions} from a custom template excludes resource guidance.""" + skills = [ + InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="Skill."), instructions="Body"), + ] + template = "Skills: {skills} {runner_instructions}" + result = SkillsProvider._create_instructions(template, skills) + assert result is not None + assert "run_skill_script" in result + assert "read_skill_resource" not in result + # --------------------------------------------------------------------------- # Tests: SkillsProvider edge cases @@ -2665,7 +2705,7 @@ class TestSkillsProviderEdgeCases: _write_skill(tmp_path, "my-skill") provider = SkillsProvider.from_paths(str(tmp_path)) await _init_provider(provider) - result = provider._load_skill(_raw_skills(provider), " ") + result = await provider._load_skill(_raw_skills(provider), " ") assert result.startswith("Error:") assert "empty" in result @@ -2716,7 +2756,7 @@ class TestSkillsProviderEdgeCases: ) provider = SkillsProvider([skill]) await _init_provider(provider) - result = provider._load_skill(_raw_skills(provider), "my-skill") + result = await provider._load_skill(_raw_skills(provider), "my-skill") assert "<tags>" in result assert "&" in result @@ -2738,9 +2778,9 @@ class TestSkillsProviderEdgeCases: await provider.before_run(agent=AsyncMock(), session=AsyncMock(), context=context, state={}) - assert len(context.tools) == 1 tool_names = {t.name for t in context.tools} - assert "load_skill" in tool_names + assert len(context.tools) == 3 + assert tool_names == {"load_skill", "read_skill_resource", "run_skill_script"} # --------------------------------------------------------------------------- @@ -2850,7 +2890,7 @@ class TestSkillResourceDecoratorEdgeCases: def no_docs() -> Any: return "data" - assert skill.resources[0].description is None + assert skill._resources[0].description is None def test_decorator_with_name_only(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="A skill."), instructions="Body") @@ -2860,9 +2900,9 @@ class TestSkillResourceDecoratorEdgeCases: """Some docs.""" return "data" - assert skill.resources[0].name == "custom-name" + assert skill._resources[0].name == "custom-name" # description is None when not explicitly provided - assert skill.resources[0].description is None + assert skill._resources[0].description is None def test_decorator_with_description_only(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="A skill."), instructions="Body") @@ -2871,8 +2911,8 @@ class TestSkillResourceDecoratorEdgeCases: def get_data() -> Any: return "data" - assert skill.resources[0].name == "get_data" - assert skill.resources[0].description == "Custom desc" + assert skill._resources[0].name == "get_data" + assert skill._resources[0].description == "Custom desc" def test_decorator_preserves_original_function_identity(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="A skill."), instructions="Body") @@ -3045,11 +3085,11 @@ class TestSkillScriptDecorator: """Run analysis.""" return "result" - assert len(skill.scripts) == 1 - assert skill.scripts[0].name == "analyze" - assert skill.scripts[0].description is None - assert isinstance(skill.scripts[0], InlineSkillScript) - assert skill.scripts[0].function is analyze + assert len(skill._scripts) == 1 + assert skill._scripts[0].name == "analyze" + assert skill._scripts[0].description is None + assert isinstance(skill._scripts[0], InlineSkillScript) + assert skill._scripts[0].function is analyze def test_parameterized_decorator(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") @@ -3058,11 +3098,11 @@ class TestSkillScriptDecorator: def my_func() -> str: return "data" - assert len(skill.scripts) == 1 - assert skill.scripts[0].name == "custom-name" - assert skill.scripts[0].description == "Custom desc" - assert isinstance(skill.scripts[0], InlineSkillScript) - assert skill.scripts[0].function is my_func + assert len(skill._scripts) == 1 + assert skill._scripts[0].name == "custom-name" + assert skill._scripts[0].description == "Custom desc" + assert isinstance(skill._scripts[0], InlineSkillScript) + assert skill._scripts[0].function is my_func def test_multiple_scripts(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") @@ -3075,9 +3115,9 @@ class TestSkillScriptDecorator: def script_b() -> str: return "b" - assert len(skill.scripts) == 2 - assert skill.scripts[0].name == "script_a" - assert skill.scripts[1].name == "script_b" + assert len(skill._scripts) == 2 + assert skill._scripts[0].name == "script_a" + assert skill._scripts[1].name == "script_b" def test_async_script(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") @@ -3087,10 +3127,10 @@ class TestSkillScriptDecorator: """Fetch remote data.""" return "data" - assert len(skill.scripts) == 1 - assert skill.scripts[0].name == "fetch_data" - assert isinstance(skill.scripts[0], InlineSkillScript) - assert skill.scripts[0].function is fetch_data + assert len(skill._scripts) == 1 + assert skill._scripts[0].name == "fetch_data" + assert isinstance(skill._scripts[0], InlineSkillScript) + assert skill._scripts[0].function is fetch_data def test_decorator_returns_original_function(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") @@ -3117,15 +3157,15 @@ class TestSkillWithScripts: def test_default_empty_scripts(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - assert skill.scripts == [] + assert skill._scripts == [] def test_scripts_at_construction(self) -> None: scripts = [InlineSkillScript(name="s1", function=lambda: None)] skill = InlineSkill( frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body", scripts=scripts ) - assert len(skill.scripts) == 1 - assert skill.scripts[0].name == "s1" + assert len(skill._scripts) == 1 + assert skill._scripts[0].name == "s1" # --------------------------------------------------------------------------- @@ -3147,7 +3187,7 @@ class TestSkillScriptRunnerProtocol: skill = InlineSkill(frontmatter=SkillFrontmatter(name="test-skill", description="test"), instructions="body") script = FileSkillScript(name="my-script", full_path=f"{_ABS}/test/scripts/run.py") - skill.scripts.append(script) + skill._scripts.append(script) result = await my_runner(skill, script, args={"key": "val"}) @@ -3165,7 +3205,7 @@ class TestSkillScriptRunnerProtocol: skill = InlineSkill(frontmatter=SkillFrontmatter(name="test-skill", description="test"), instructions="body") script = InlineSkillScript(name="my-script", function=lambda: None) - skill.scripts.append(script) + skill._scripts.append(script) result = await runner(skill, script, args={"key": "val"}) assert result == "custom result" @@ -3201,7 +3241,7 @@ class TestSkillScriptRunnerProtocol: skill = InlineSkill(frontmatter=SkillFrontmatter(name="test-skill", description="test"), instructions="body") script = FileSkillScript(name="my-script", full_path=f"{_ABS}/test/scripts/run.py") - skill.scripts.append(script) + skill._scripts.append(script) result = my_runner(skill, script, args={"key": "val"}) @@ -3219,7 +3259,7 @@ class TestSkillScriptRunnerProtocol: skill = InlineSkill(frontmatter=SkillFrontmatter(name="test-skill", description="test"), instructions="body") script = InlineSkillScript(name="my-script", function=lambda: None) - skill.scripts.append(script) + skill._scripts.append(script) result = runner(skill, script, args={"key": "val"}) assert result == "sync result" @@ -3255,7 +3295,7 @@ class TestSkillsProviderFactories: async def test_code_skills_with_scripts_creates_provider(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="s1", function=lambda: None)) + skill._scripts.append(InlineSkillScript(name="s1", function=lambda: None)) provider = SkillsProvider([skill]) await _init_provider(provider) @@ -3267,16 +3307,18 @@ class TestSkillsProviderFactories: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") provider = SkillsProvider([skill]) await _init_provider(provider) - # No scripts with functions, no runner, no resources — only load_skill - assert len(_ctx(provider)[2]) == 1 - assert not any(hasattr(t, "name") and t.name == "run_skill_script" for t in _ctx(provider)[2]) + assert {t.name for t in _ctx(provider)[2] if hasattr(t, "name")} == { + "load_skill", + "read_skill_resource", + "run_skill_script", + } async def test_code_script_runs_directly(self) -> None: def my_function(key: str = "") -> str: return f"executed: {key}" skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="s1", function=my_function)) + skill._scripts.append(InlineSkillScript(name="s1", function=my_function)) provider = SkillsProvider([skill]) await _init_provider(provider) @@ -3287,22 +3329,21 @@ class TestSkillsProviderFactories: async def test_no_scripts_no_tool(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - # No scripts at all — no run_skill_script tool provider = SkillsProvider([skill]) await _init_provider(provider) - assert not any(hasattr(t, "name") and t.name == "run_skill_script" for t in _ctx(provider)[2]) + assert any(hasattr(t, "name") and t.name == "run_skill_script" for t in _ctx(provider)[2]) async def test_no_resources_no_read_skill_resource_tool(self) -> None: - """When no skill has resources, read_skill_resource tool is not advertised.""" + """read_skill_resource is advertised even when no skill has resources.""" skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") provider = SkillsProvider([skill]) await _init_provider(provider) - assert not any(hasattr(t, "name") and t.name == "read_skill_resource" for t in _ctx(provider)[2]) + assert any(hasattr(t, "name") and t.name == "read_skill_resource" for t in _ctx(provider)[2]) async def test_resources_present_includes_read_skill_resource_tool(self) -> None: """When a skill has resources, read_skill_resource tool is advertised.""" skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.resources.append(InlineSkillResource(name="ref", content="reference data")) + skill._resources.append(InlineSkillResource(name="ref", content="reference data")) provider = SkillsProvider([skill]) await _init_provider(provider) assert any(hasattr(t, "name") and t.name == "read_skill_resource" for t in _ctx(provider)[2]) @@ -3310,22 +3351,22 @@ class TestSkillsProviderFactories: async def test_resources_present_includes_resource_instructions(self) -> None: """When a skill has resources, instructions mention read_skill_resource.""" skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.resources.append(InlineSkillResource(name="ref", content="reference data")) + skill._resources.append(InlineSkillResource(name="ref", content="reference data")) provider = SkillsProvider([skill]) await _init_provider(provider) assert "read_skill_resource" in (_ctx(provider)[1] or "") async def test_no_resources_excludes_resource_instructions(self) -> None: - """When no skill has resources, instructions do not mention read_skill_resource.""" + """read_skill_resource instructions are included even without resources.""" skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") provider = SkillsProvider([skill]) await _init_provider(provider) - assert "read_skill_resource" not in (_ctx(provider)[1] or "") + assert "read_skill_resource" in (_ctx(provider)[1] or "") async def test_read_skill_resource_tool_returns_content(self) -> None: """The read_skill_resource tool returns resource content when invoked.""" skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.resources.append(InlineSkillResource(name="ref", content="reference data")) + skill._resources.append(InlineSkillResource(name="ref", content="reference data")) provider = SkillsProvider([skill]) await _init_provider(provider) read_tool = next(t for t in _ctx(provider)[2] if hasattr(t, "name") and t.name == "read_skill_resource") @@ -3428,7 +3469,7 @@ class TestSkillsProviderFactories: code_skill = InlineSkill( frontmatter=SkillFrontmatter(name="code-skill", description="test"), instructions="body" ) - code_skill.scripts.append(InlineSkillScript(name="s1", function=lambda: None)) + code_skill._scripts.append(InlineSkillScript(name="s1", function=lambda: None)) provider = SkillsProvider( DeduplicatingSkillsSource( @@ -3459,8 +3500,8 @@ class TestSkillsProviderFactories: async def test_file_script_error_without_runner(self) -> None: # A skill with both a code script and a file-based script skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="code-s", function=lambda: "ok")) - skill.scripts.append(FileSkillScript(name="file-s", full_path=f"{_ABS}/test/scripts/s1.py")) + skill._scripts.append(InlineSkillScript(name="code-s", function=lambda: "ok")) + skill._scripts.append(FileSkillScript(name="file-s", full_path=f"{_ABS}/test/scripts/s1.py")) provider = SkillsProvider([skill]) await _init_provider(provider) @@ -3480,7 +3521,7 @@ class TestSkillsProviderFactories: return f"async: {x}" skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="s1", function=async_func)) + skill._scripts.append(InlineSkillScript(name="s1", function=async_func)) provider = SkillsProvider([skill]) await _init_provider(provider) @@ -3495,7 +3536,7 @@ class TestSkillsProviderFactories: return {"status": "ok", "value": 42} skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="s1", function=returns_dict)) + skill._scripts.append(InlineSkillScript(name="s1", function=returns_dict)) provider = SkillsProvider([skill]) await _init_provider(provider) @@ -3506,7 +3547,7 @@ class TestSkillsProviderFactories: async def test_code_script_returns_none(self) -> None: """Code-defined scripts returning None pass through as None.""" skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="s1", function=lambda: None)) + skill._scripts.append(InlineSkillScript(name="s1", function=lambda: None)) provider = SkillsProvider([skill]) await _init_provider(provider) @@ -3517,8 +3558,8 @@ class TestSkillsProviderFactories: async def test_script_with_path_errors_without_runner(self) -> None: """A file-based script without a runner should return an error.""" skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="code-s", function=lambda: "ok")) - skill.scripts.append(FileSkillScript(name="path-s", full_path=f"{_ABS}/test/scripts/s1.py")) + skill._scripts.append(InlineSkillScript(name="code-s", function=lambda: "ok")) + skill._scripts.append(FileSkillScript(name="path-s", full_path=f"{_ABS}/test/scripts/s1.py")) provider = SkillsProvider([skill]) await _init_provider(provider) @@ -3535,7 +3576,7 @@ class TestSkillsProviderFactories: async def test_run_skill_script_error_on_missing_skill(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="s1", function=lambda: None)) + skill._scripts.append(InlineSkillScript(name="s1", function=lambda: None)) provider = SkillsProvider([skill]) await _init_provider(provider) @@ -3606,7 +3647,7 @@ class TestSkillsProviderFactories: async def test_run_skill_script_error_on_missing_script(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="s1", function=lambda: None)) + skill._scripts.append(InlineSkillScript(name="s1", function=lambda: None)) provider = SkillsProvider([skill]) await _init_provider(provider) @@ -3617,7 +3658,7 @@ class TestSkillsProviderFactories: async def test_run_skill_script_error_on_empty_names(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="s1", function=lambda: None)) + skill._scripts.append(InlineSkillScript(name="s1", function=lambda: None)) provider = SkillsProvider([skill]) await _init_provider(provider) @@ -3631,7 +3672,7 @@ class TestSkillsProviderFactories: async def test_instructions_include_script_runner_hints(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="s1", function=lambda: None)) + skill._scripts.append(InlineSkillScript(name="s1", function=lambda: None)) provider = SkillsProvider([skill]) await _init_provider(provider) @@ -3642,12 +3683,11 @@ class TestSkillsProviderFactories: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") provider = SkillsProvider([skill]) await _init_provider(provider) - # No scripts and no runner — instructions should not mention run_skill_script - assert "run_skill_script" not in (_ctx(provider)[1] or "") + assert "run_skill_script" in (_ctx(provider)[1] or "") async def test_tool_schema_args_description_mentions_key_format(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="s1", function=lambda: None)) + skill._scripts.append(InlineSkillScript(name="s1", function=lambda: None)) provider = SkillsProvider([skill]) await _init_provider(provider) @@ -3658,7 +3698,7 @@ class TestSkillsProviderFactories: async def test_require_script_approval_sets_approval_mode(self) -> None: """When require_script_approval=True, the run_skill_script tool has approval_mode='always_require'.""" skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="s1", function=lambda: None)) + skill._scripts.append(InlineSkillScript(name="s1", function=lambda: None)) provider = SkillsProvider([skill], require_script_approval=True) await _init_provider(provider) @@ -3668,7 +3708,7 @@ class TestSkillsProviderFactories: async def test_require_script_approval_false_by_default(self) -> None: """By default, the run_skill_script tool has approval_mode='never_require'.""" skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="s1", function=lambda: None)) + skill._scripts.append(InlineSkillScript(name="s1", function=lambda: None)) provider = SkillsProvider([skill]) await _init_provider(provider) @@ -3676,14 +3716,14 @@ class TestSkillsProviderFactories: assert run_tool.approval_mode == "never_require" async def test_require_script_approval_does_not_affect_other_tools(self) -> None: - """The load_skill tool should never require approval.""" + """Non-script tools should never require approval.""" skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="s1", function=lambda: None)) + skill._scripts.append(InlineSkillScript(name="s1", function=lambda: None)) provider = SkillsProvider([skill], require_script_approval=True) await _init_provider(provider) other_tools = [t for t in _ctx(provider)[2] if hasattr(t, "name") and t.name != "run_skill_script"] - assert len(other_tools) == 1 + assert len(other_tools) == 2 for t in other_tools: assert t.approval_mode == "never_require" @@ -3694,7 +3734,7 @@ class TestSkillsProviderFactories: raise RuntimeError("Something went wrong") skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="boom", function=failing_script)) + skill._scripts.append(InlineSkillScript(name="boom", function=failing_script)) provider = SkillsProvider([skill]) await _init_provider(provider) @@ -3705,16 +3745,20 @@ class TestSkillsProviderFactories: assert "Something went wrong" not in result async def test_custom_template_without_runner_placeholder_raises(self) -> None: - """Provider with code scripts and custom template missing {runner_instructions} raises.""" + """Providers accept custom templates without {runner_instructions}.""" skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="s1", function=lambda: None)) + skill._scripts.append(InlineSkillScript(name="s1", function=lambda: None)) provider = SkillsProvider( [skill], instruction_template="Skills: {skills}", ) - with pytest.raises(ValueError, match="runner_instructions"): - await _init_provider(provider) + await _init_provider(provider) + instructions = _ctx(provider)[1] + assert instructions is not None + assert instructions.startswith("Skills: ") + assert "my-skill" in instructions + assert "test" in instructions # --------------------------------------------------------------------------- @@ -3737,8 +3781,8 @@ 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 == "scripts/analyze.py" + 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.""" @@ -3752,7 +3796,7 @@ class TestFileScriptDiscovery: skills = await _discover_file_skills_for_test(str(tmp_path)) assert "my-skill" in skills - assert len(skills["my-skill"].scripts) == 0 + 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" @@ -3765,7 +3809,7 @@ class TestFileScriptDiscovery: (scripts_dir / "generate.py").write_text("print('gen')", encoding="utf-8") skills = await _discover_file_skills_for_test(str(tmp_path)) - script = skills["my-skill"].scripts[0] + script = skills["my-skill"]._scripts[0] assert script.full_path is not None assert os.path.isabs(script.full_path) expected = str(Path(str(skill_dir), "scripts", "generate.py")) @@ -3788,8 +3832,8 @@ class TestFileScriptDiscovery: (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/top.py" + assert len(skills["my-skill"]._scripts) == 1 + 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" @@ -3801,7 +3845,7 @@ class TestFileScriptDiscovery: (skill_dir / "readme.md").write_text("# Docs", encoding="utf-8") skills = await _discover_file_skills_for_test(str(tmp_path)) - assert len(skills["my-skill"].scripts) == 0 + assert len(skills["my-skill"]._scripts) == 0 class TestCustomScriptExtensions: @@ -3821,13 +3865,13 @@ class TestCustomScriptExtensions: # 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] + script_names_default = [s.name for s in skills_default["my-skill"]._scripts] 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] + script_names_custom = [s.name for s in skills_custom["my-skill"]._scripts] assert "scripts/run.sh" in script_names_custom assert "scripts/analyze.py" not in script_names_custom @@ -3851,7 +3895,7 @@ class TestCustomScriptExtensions: ) await _init_provider(provider) skill = _ctx(provider)[0]["my-skill"] - script_names = [s.name for s in skill.scripts] + script_names = [s.name for s in skill._scripts] assert "scripts/run.sh" in script_names assert "scripts/analyze.py" not in script_names @@ -3875,7 +3919,7 @@ class TestCustomScriptExtensions: ) await _init_provider(provider) skill = _ctx(provider)[0]["my-skill"] - script_names = [s.name for s in skill.scripts] + script_names = [s.name for s in skill._scripts] assert "scripts/analyze.py" in script_names assert "scripts/run.sh" in script_names assert "scripts/notes.txt" not in script_names @@ -3895,7 +3939,7 @@ class TestCreateInstructionsWithScripts: def test_excludes_script_count(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="s1", function=lambda: None)) + skill._scripts.append(InlineSkillScript(name="s1", function=lambda: None)) result = SkillsProvider._create_instructions(None, [skill]) assert result is not None @@ -3919,11 +3963,11 @@ class TestLoadSkillWithScripts: async def test_code_skill_includes_scripts_element(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="analyze", description="Run analysis", function=lambda: None)) + skill._scripts.append(InlineSkillScript(name="analyze", description="Run analysis", function=lambda: None)) provider = SkillsProvider([skill]) await _init_provider(provider) - result = provider._load_skill(_raw_skills(provider), "my-skill") + result = await provider._load_skill(_raw_skills(provider), "my-skill") assert "" in result assert 'name="analyze"' in result @@ -3933,7 +3977,7 @@ class TestLoadSkillWithScripts: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") provider = SkillsProvider([skill]) await _init_provider(provider) - result = provider._load_skill(_raw_skills(provider), "my-skill") + result = await provider._load_skill(_raw_skills(provider), "my-skill") assert "" not in result @@ -4000,25 +4044,25 @@ class TestClassSkill: skill = _MinimalClassSkill() assert skill.scripts == [] - def test_minimal_skill_content_contains_name(self) -> None: + async def test_minimal_skill_content_contains_name(self) -> None: skill = _MinimalClassSkill() - assert "minimal-skill" in skill.content + assert "minimal-skill" in (await skill.get_content()) - def test_minimal_skill_content_contains_description(self) -> None: + async def test_minimal_skill_content_contains_description(self) -> None: skill = _MinimalClassSkill() - assert "A minimal skill." in skill.content + assert "A minimal skill." in (await skill.get_content()) - def test_minimal_skill_content_contains_instructions(self) -> None: + async def test_minimal_skill_content_contains_instructions(self) -> None: skill = _MinimalClassSkill() - assert "Do minimal things." in skill.content + assert "Do minimal things." in (await skill.get_content()) - def test_minimal_skill_content_no_resources_element(self) -> None: + async def test_minimal_skill_content_no_resources_element(self) -> None: skill = _MinimalClassSkill() - assert "" not in skill.content + assert "" not in (await skill.get_content()) - def test_minimal_skill_content_no_scripts_element(self) -> None: + async def test_minimal_skill_content_no_scripts_element(self) -> None: skill = _MinimalClassSkill() - assert "" not in skill.content + assert "" not in (await skill.get_content()) def test_full_skill_has_resources(self) -> None: skill = _FullClassSkill() @@ -4030,20 +4074,20 @@ class TestClassSkill: assert len(skill.scripts) == 1 assert skill.scripts[0].name == "test-script" - def test_full_skill_content_contains_resources(self) -> None: + async def test_full_skill_content_contains_resources(self) -> None: skill = _FullClassSkill() - assert "" in skill.content - assert 'name="test-resource"' in skill.content + assert "" in (await skill.get_content()) + assert 'name="test-resource"' in (await skill.get_content()) - def test_full_skill_content_contains_scripts(self) -> None: + async def test_full_skill_content_contains_scripts(self) -> None: skill = _FullClassSkill() - assert "" in skill.content - assert 'name="test-script"' in skill.content + assert "" in (await skill.get_content()) + assert 'name="test-script"' in (await skill.get_content()) - def test_content_is_cached(self) -> None: + async def test_content_is_cached(self) -> None: skill = _MinimalClassSkill() - content1 = skill.content - content2 = skill.content + content1 = (await skill.get_content()) + content2 = (await skill.get_content()) assert content1 is content2 def test_resources_are_lazy_cached(self) -> None: @@ -4081,7 +4125,7 @@ class TestClassSkill: provider = SkillsProvider([skill]) await _init_provider(provider) - result = provider._load_skill(_raw_skills(provider), "full-skill") + result = await provider._load_skill(_raw_skills(provider), "full-skill") assert "Use this skill for full tasks." in result assert "" in result assert "" in result @@ -4299,15 +4343,15 @@ class TestClassSkillDecoratorDiscovery: assert s1 == s2 assert s1 is not s2 # defensive copy - def test_content_includes_discovered_resources(self) -> None: + async def test_content_includes_discovered_resources(self) -> None: skill = _DecoratorClassSkill() - assert "" in skill.content - assert 'name="lookup-table"' in skill.content + assert "" in (await skill.get_content()) + assert 'name="lookup-table"' in (await skill.get_content()) - def test_content_includes_discovered_scripts(self) -> None: + async def test_content_includes_discovered_scripts(self) -> None: skill = _DecoratorClassSkill() - assert "" in skill.content - assert 'name="convert"' in skill.content + assert "" in (await skill.get_content()) + assert 'name="convert"' in (await skill.get_content()) def test_duplicate_resource_name_raises(self) -> None: skill = _DuplicateResourceSkill() @@ -4361,9 +4405,9 @@ class TestClassSkillDecoratorDiscovery: skill = _PropertyResourceSkill() assert skill.resources[0].description is None - def test_property_resource_in_content(self) -> None: + async def test_property_resource_in_content(self) -> None: skill = _PropertyResourceSkill() - assert 'name="static-table"' in skill.content + assert 'name="static-table"' in (await skill.get_content()) async def test_mixed_property_and_method_resources(self) -> None: """Property and method resources can coexist.""" @@ -4386,11 +4430,11 @@ class TestClassSkillDecoratorDiscovery: scr = next(s for s in skill.scripts if s.name == "described-scr") assert scr.description == "A described script." - def test_explicit_description_in_content_xml(self) -> None: + async def test_explicit_description_in_content_xml(self) -> None: """Explicit descriptions appear in the skill content XML.""" skill = _ExplicitDescriptionSkill() - assert 'description="A described resource."' in skill.content - assert 'description="A described script."' in skill.content + assert 'description="A described resource."' in (await skill.get_content()) + assert 'description="A described script."' in (await skill.get_content()) def test_property_getter_not_called_during_discovery(self) -> None: """Property getter must NOT be evaluated when resources are discovered.""" @@ -4698,11 +4742,11 @@ class _MixedPropertyMethodSkill(ClassSkill): return "result" skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="analyze", description="Run analysis", function=analyze)) + skill._scripts.append(InlineSkillScript(name="analyze", description="Run analysis", function=analyze)) provider = SkillsProvider([skill]) await _init_provider(provider) - result = provider._load_skill(_raw_skills(provider), "my-skill") + result = await provider._load_skill(_raw_skills(provider), "my-skill") assert "" in result assert 'name="analyze"' in result @@ -4715,7 +4759,7 @@ class TestReadSkillResourceWithScripts: async def test_reads_script_with_static_content(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="generate.py", function=lambda: "print('hello')")) + skill._scripts.append(InlineSkillScript(name="generate.py", function=lambda: "print('hello')")) provider = SkillsProvider([skill]) await _init_provider(provider) @@ -4725,7 +4769,7 @@ class TestReadSkillResourceWithScripts: async def test_script_not_accessible_via_read_resource(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="run.py", function=lambda: "script output")) + skill._scripts.append(InlineSkillScript(name="run.py", function=lambda: "script output")) provider = SkillsProvider([skill]) await _init_provider(provider) @@ -4738,7 +4782,7 @@ class TestReadSkillResourceWithScripts: return "async output" skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="run.py", function=async_script)) + skill._scripts.append(InlineSkillScript(name="run.py", function=async_script)) provider = SkillsProvider([skill]) await _init_provider(provider) @@ -4747,7 +4791,7 @@ class TestReadSkillResourceWithScripts: async def test_script_case_insensitive_not_in_resources(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="Generate.py", function=lambda: "code")) + skill._scripts.append(InlineSkillScript(name="Generate.py", function=lambda: "code")) provider = SkillsProvider([skill]) await _init_provider(provider) @@ -4756,8 +4800,8 @@ class TestReadSkillResourceWithScripts: async def test_resource_takes_priority_over_script(self) -> None: skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.resources.append(InlineSkillResource(name="data.py", content="resource content")) - skill.scripts.append(InlineSkillScript(name="data.py", function=lambda: "script content")) + skill._resources.append(InlineSkillResource(name="data.py", content="resource content")) + skill._scripts.append(InlineSkillScript(name="data.py", function=lambda: "script content")) provider = SkillsProvider([skill]) await _init_provider(provider) @@ -4769,7 +4813,7 @@ class TestReadSkillResourceWithScripts: raise RuntimeError("boom") skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="bad.py", function=failing_script)) + skill._scripts.append(InlineSkillScript(name="bad.py", function=failing_script)) provider = SkillsProvider([skill]) await _init_provider(provider) @@ -5056,7 +5100,7 @@ class TestSkillsSource: 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] + resource_names = [r.name for r in skills[0]._resources] assert "references/data.json" in resource_names assert "references/data.csv" not in resource_names @@ -5304,7 +5348,7 @@ class TestSourceComposition: async def test_script_approval_on_provider(self) -> None: """SkillsProvider with require_script_approval sets the approval mode.""" skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="s1", function=lambda: None)) + skill._scripts.append(InlineSkillScript(name="s1", function=lambda: None)) provider = SkillsProvider( DeduplicatingSkillsSource(InMemorySkillsSource([skill])), @@ -5540,11 +5584,11 @@ class TestSkillsProviderConstructorEdgeCases: class TestInlineSkillContentCaching: """Tests for InlineSkill.content caching.""" - def test_content_cached_after_first_access(self) -> None: + async def test_content_cached_after_first_access(self) -> None: """InlineSkill.content returns the same object on subsequent accesses.""" skill = InlineSkill(frontmatter=SkillFrontmatter(name="test-skill", description="Test"), instructions="Body") - first = skill.content - second = skill.content + first = (await skill.get_content()) + second = (await skill.get_content()) assert first is second # Same object (cached) assert "test-skill" in first @@ -5642,7 +5686,7 @@ class TestArrayStyleScriptArgs: async def test_tool_schema_accepts_array_args(self) -> None: """The run_skill_script tool schema accepts array-style args via oneOf.""" skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="s1", function=lambda: None)) + skill._scripts.append(InlineSkillScript(name="s1", function=lambda: None)) provider = SkillsProvider([skill]) await _init_provider(provider) @@ -5680,7 +5724,7 @@ class TestArrayStyleScriptArgs: async def test_run_skill_script_inline_with_list_args_returns_error(self) -> None: """Inline script called with list args through provider returns error (TypeError caught).""" skill = InlineSkill(frontmatter=SkillFrontmatter(name="my-skill", description="test"), instructions="body") - skill.scripts.append(InlineSkillScript(name="s1", function=lambda: "ok")) + skill._scripts.append(InlineSkillScript(name="s1", function=lambda: "ok")) provider = SkillsProvider([skill]) await _init_provider(provider) @@ -5689,7 +5733,7 @@ class TestArrayStyleScriptArgs: assert "Error" in result assert "Failed to run" in result - def test_file_skill_content_includes_scripts_block(self) -> None: + async def test_file_skill_content_includes_scripts_block(self) -> None: """FileSkill.content appends a block when scripts are present.""" script = FileSkillScript(name="run.py", full_path=f"{_ABS}/test/run.py") skill = FileSkill( @@ -5698,16 +5742,16 @@ class TestArrayStyleScriptArgs: path=f"{_ABS}/test", scripts=[script], ) - assert "" in skill.content - assert 'name="run.py"' in skill.content - assert "" in skill.content - assert '"type": "array"' in skill.content + assert "" in (await skill.get_content()) + assert 'name="run.py"' in (await skill.get_content()) + assert "" in (await skill.get_content()) + assert '"type": "array"' in (await skill.get_content()) - def test_file_skill_content_no_scripts_no_block(self) -> None: + async def test_file_skill_content_no_scripts_no_block(self) -> None: """FileSkill.content does not append a block when no scripts.""" skill = FileSkill( frontmatter=SkillFrontmatter(name="my-skill", description="test"), content="---\nname: my-skill\n---\nBody", path=f"{_ABS}/test", ) - assert "" not in skill.content + assert "" not in (await skill.get_content())