feat: surface MCP tool annotations to host code #110

2026-05-24T19:44:12-04:00

jasoncouture commented

2026-05-24 19:44:12 -04:00

(Migrated from github.com)

Summary

Sets the right MCP [McpServerTool] annotations on every internal tool — ReadOnly / Destructive / Idempotent / OpenWorld populated per the spec so clients can reason about side effects.
Adds ToolDescriptorAnnotations to ToolDescriptor and propagates them from the MCP client (tool.ProtocolTool.Annotations), so host code, UI, and future per-call confirmation gates can inspect destructive/idempotent/open-world/read-only without re-deriving from the schema.

Test plan

dotnet build clean
dotnet test — 566 / 566 passing
Spot-check that a workspace tool (e.g. file_read, file_write, shell_run) advertises the expected annotations to a connected MCP client

## Summary - Sets the right MCP `[McpServerTool]` annotations on every internal tool — `ReadOnly` / `Destructive` / `Idempotent` / `OpenWorld` populated per the spec so clients can reason about side effects. - Adds `ToolDescriptorAnnotations` to `ToolDescriptor` and propagates them from the MCP client (`tool.ProtocolTool.Annotations`), so host code, UI, and future per-call confirmation gates can inspect destructive/idempotent/open-world/read-only without re-deriving from the schema. ## Test plan - [x] `dotnet build` clean - [x] `dotnet test` — 566 / 566 passing - [ ] Spot-check that a workspace tool (e.g. `file_read`, `file_write`, `shell_run`) advertises the expected annotations to a connected MCP client

github-actions[bot] commented

2026-05-24 19:45:51 -04:00

(Migrated from github.com)

Package	Line Rate	Branch Rate	Complexity	Health
LlamaShears.Core.Eventing	91%	71%	90	✔
LlamaShears.Core.Abstractions	57%	37%	486	➖
LlamaShears.Api	29%	29%	494	❌
LlamaShears.Provider.Ollama	44%	28%	188	❌
LlamaShears.Core.Eventing.Extensions	100%	100%	1	✔
LlamaShears.Core	47%	43%	1854	❌
LlamaShears.Provider.Onnx.Embeddings	37%	38%	72	❌
LlamaShears.Provider.OpenAI	66%	65%	229	➖
LlamaShears.Api.Web	1%	1%	493	❌
LlamaShears.Hosting	33%	21%	27	❌
LlamaShears.Plugins	0%	100%	1	❌
LlamaShears.Core.Eventing	85%	61%	90	✔
LlamaShears	63%	47%	79	➖
LlamaShears.Plugins.Host	34%	24%	36	❌
LlamaShears.Core.Abstractions	46%	25%	486	❌
LlamaShears.Api	6%	2%	494	❌
LlamaShears.Provider.Ollama	3%	1%	188	❌
LlamaShears.IntegrationTests.Fixtures	73%	57%	64	➖
LlamaShears.Core.Eventing.Extensions	100%	100%	1	✔
StrangeSoft.Plugins.Host	20%	21%	87	❌
LlamaShears.Core	41%	27%	1854	❌
LlamaShears.Provider.Onnx.Embeddings	3%	0%	72	❌
LlamaShears.Provider.OpenAI	2%	0%	229	❌
LlamaShears.Api.Web	18%	7%	493	❌
LlamaShears.Hosting	26%	8%	27	❌
LlamaShears.Plugins	0%	100%	1	❌
LlamaShears.Core.Eventing	85%	61%	90	✔
LlamaShears	63%	47%	79	➖
LlamaShears.Plugins.Host	34%	24%	36	❌
LlamaShears.Core.Abstractions	36%	22%	486	❌
LlamaShears.Api	7%	3%	494	❌
LlamaShears.Provider.Ollama	3%	1%	188	❌
LlamaShears.IntegrationTests.Fixtures	70%	52%	64	➖
LlamaShears.Core.Eventing.Extensions	100%	100%	1	✔
StrangeSoft.Plugins.Host	20%	21%	87	❌
LlamaShears.Core	30%	18%	1854	❌
LlamaShears.Provider.Onnx.Embeddings	3%	0%	72	❌
LlamaShears.Provider.OpenAI	2%	0%	229	❌
LlamaShears.Api.Web	17%	6%	493	❌
LlamaShears.Hosting	26%	8%	27	❌
LlamaShears.Analyzers.CodeFixes	85%	69%	60	✔
LlamaShears.Analyzers	88%	76%	199	✔
Summary	46% (12557 / 37587)	36% (2827 / 11589)	12595	❌

Package | Line Rate | Branch Rate | Complexity | Health -------- | --------- | ----------- | ---------- | ------ LlamaShears.Core.Eventing | 91% | 71% | 90 | ✔ LlamaShears.Core.Abstractions | 57% | 37% | 486 | ➖ LlamaShears.Api | 29% | 29% | 494 | ❌ LlamaShears.Provider.Ollama | 44% | 28% | 188 | ❌ LlamaShears.Core.Eventing.Extensions | 100% | 100% | 1 | ✔ LlamaShears.Core | 47% | 43% | 1854 | ❌ LlamaShears.Provider.Onnx.Embeddings | 37% | 38% | 72 | ❌ LlamaShears.Provider.OpenAI | 66% | 65% | 229 | ➖ LlamaShears.Api.Web | 1% | 1% | 493 | ❌ LlamaShears.Hosting | 33% | 21% | 27 | ❌ LlamaShears.Plugins | 0% | 100% | 1 | ❌ LlamaShears.Core.Eventing | 85% | 61% | 90 | ✔ LlamaShears | 63% | 47% | 79 | ➖ LlamaShears.Plugins.Host | 34% | 24% | 36 | ❌ LlamaShears.Core.Abstractions | 46% | 25% | 486 | ❌ LlamaShears.Api | 6% | 2% | 494 | ❌ LlamaShears.Provider.Ollama | 3% | 1% | 188 | ❌ LlamaShears.IntegrationTests.Fixtures | 73% | 57% | 64 | ➖ LlamaShears.Core.Eventing.Extensions | 100% | 100% | 1 | ✔ StrangeSoft.Plugins.Host | 20% | 21% | 87 | ❌ LlamaShears.Core | 41% | 27% | 1854 | ❌ LlamaShears.Provider.Onnx.Embeddings | 3% | 0% | 72 | ❌ LlamaShears.Provider.OpenAI | 2% | 0% | 229 | ❌ LlamaShears.Api.Web | 18% | 7% | 493 | ❌ LlamaShears.Hosting | 26% | 8% | 27 | ❌ LlamaShears.Plugins | 0% | 100% | 1 | ❌ LlamaShears.Core.Eventing | 85% | 61% | 90 | ✔ LlamaShears | 63% | 47% | 79 | ➖ LlamaShears.Plugins.Host | 34% | 24% | 36 | ❌ LlamaShears.Core.Abstractions | 36% | 22% | 486 | ❌ LlamaShears.Api | 7% | 3% | 494 | ❌ LlamaShears.Provider.Ollama | 3% | 1% | 188 | ❌ LlamaShears.IntegrationTests.Fixtures | 70% | 52% | 64 | ➖ LlamaShears.Core.Eventing.Extensions | 100% | 100% | 1 | ✔ StrangeSoft.Plugins.Host | 20% | 21% | 87 | ❌ LlamaShears.Core | 30% | 18% | 1854 | ❌ LlamaShears.Provider.Onnx.Embeddings | 3% | 0% | 72 | ❌ LlamaShears.Provider.OpenAI | 2% | 0% | 229 | ❌ LlamaShears.Api.Web | 17% | 6% | 493 | ❌ LlamaShears.Hosting | 26% | 8% | 27 | ❌ LlamaShears.Analyzers.CodeFixes | 85% | 69% | 60 | ✔ LlamaShears.Analyzers | 88% | 76% | 199 | ✔ **Summary** | **46%** (12557 / 37587) | **36%** (2827 / 11589) | **12595** | ❌

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed

2026-05-24 19:47:35 -04:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull request overview

Surfaces MCP tool annotation metadata (read-only / destructive / idempotent / open-world) into host-visible abstractions by introducing ToolDescriptorAnnotations, plumbing it through the MCP client mapping, and annotating built-in MCP server tools accordingly.

Changes:

Add ToolDescriptorAnnotations and include it on ToolDescriptor so hosts/UIs can reason about tool side effects.
Map MCP ToolAnnotations from McpClientTool.ProtocolTool.Annotations into ToolDescriptor.Annotations.
Populate [McpServerTool] annotation flags on many built-in API tools and update affected unit tests/docs.

Reviewed changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/LlamaShears.UnitTests/Provider/OpenAI/OpenAILanguageModelTests.cs	Update `ToolDescriptor` construction to include annotations.
tests/LlamaShears.UnitTests/Provider/Ollama/OllamaLanguageModelToolFlatteningTests.cs	Update `ToolDescriptor` construction to include annotations.
tests/LlamaShears.UnitTests/Agent/Core/InferenceRunnerToolDispatchTests.cs	Update tool advertisement `ToolDescriptor` construction to include annotations.
src/public/LlamaShears.Core.Abstractions/Provider/ToolDescriptorAnnotations.cs	Introduce new public record describing tool behavior hints.
src/public/LlamaShears.Core.Abstractions/Provider/ToolDescriptor.cs	Add `Annotations` to the public `ToolDescriptor` record.
src/LlamaShears.Core/Tools/ModelContextProtocol/ModelContextProtocolClient.cs	Map MCP protocol tool annotations into `ToolDescriptorAnnotations`.
src/LlamaShears.Api/Tools/ModelContextProtocol/Todo/TodoTools.cs	Add `[McpServerTool]` flags for TODO tools.
src/LlamaShears.Api/Tools/ModelContextProtocol/Skills/SkillTools.cs	Add `[McpServerTool]` flags for skill tools.
src/LlamaShears.Api/Tools/ModelContextProtocol/Memory/StoreMemoryTool.cs	Add `[McpServerTool]` flags for memory store tool.
src/LlamaShears.Api/Tools/ModelContextProtocol/Memory/SearchMemoryTool.cs	Add `[McpServerTool]` flags for memory search tool.
src/LlamaShears.Api/Tools/ModelContextProtocol/Memory/IndexMemoryTool.cs	Add `[McpServerTool]` flags for memory index tool.
src/LlamaShears.Api/Tools/ModelContextProtocol/Filesystem/WriteFileTool.cs	Add `[McpServerTool]` flags for file write tool.
src/LlamaShears.Api/Tools/ModelContextProtocol/Filesystem/RegexReplaceFileTool.cs	Add `[McpServerTool]` flags for regex-replace tool.
src/LlamaShears.Api/Tools/ModelContextProtocol/Filesystem/ReadFileTool.cs	Add `[McpServerTool]` flags for file read tool.
src/LlamaShears.Api/Tools/ModelContextProtocol/Filesystem/MoveFileTool.cs	Add `[McpServerTool]` flags for file move tool.
src/LlamaShears.Api/Tools/ModelContextProtocol/Filesystem/ListFilesTool.cs	Add `[McpServerTool]` flags for file list tool.
src/LlamaShears.Api/Tools/ModelContextProtocol/Filesystem/GrepTool.cs	Add `[McpServerTool]` flags for grep tool.
src/LlamaShears.Api/Tools/ModelContextProtocol/Filesystem/DeleteFileTool.cs	Add `[McpServerTool]` flags for file delete tool.
src/LlamaShears.Api/Tools/ModelContextProtocol/Filesystem/AppendFileTool.cs	Add `[McpServerTool]` flags for file append tool.
src/LlamaShears.Api/Tools/ModelContextProtocol/Cron/TriggerCronTool.cs	Add `[McpServerTool]` flags for cron trigger tool.
src/LlamaShears.Api/Tools/ModelContextProtocol/Cron/ScheduleCronTool.cs	Add `[McpServerTool]` flags for cron schedule tool.
src/LlamaShears.Api/Tools/ModelContextProtocol/Cron/ListCronTool.cs	Add `[McpServerTool]` flags for cron list tool.
src/LlamaShears.Api/Tools/ModelContextProtocol/Cron/EditCronTool.cs	Add `[McpServerTool]` flags for cron edit tool.
src/LlamaShears.Api/Tools/ModelContextProtocol/Cron/CancelCronTool.cs	Add `[McpServerTool]` flags for cron cancel tool.
docs/api/LlamaShears.Core.Abstractions/LlamaShears/Core/Abstractions/Provider/ToolDescriptorAnnotations.md	Add generated API docs for `ToolDescriptorAnnotations`.
docs/api/LlamaShears.Core.Abstractions/LlamaShears/Core/Abstractions/Provider/ToolDescriptor.md	Update generated API docs for new `ToolDescriptor` parameter/property.
docs/api/LlamaShears.Core.Abstractions/LlamaShears/Core/Abstractions/Provider/index.md	Add `ToolDescriptorAnnotations` to provider namespace index.
docs/api/LlamaShears.Core.Abstractions/index.md	Add `ToolDescriptorAnnotations` to root API index.

Comments suppressed due to low confidence (5)

src/LlamaShears.Api/Tools/ModelContextProtocol/Filesystem/ReadFileTool.cs:33

file_read allows absolute paths "anywhere on disk the host can reach" (see parameter description and implementation), which is not a closed-domain tool. Marking OpenWorld = false seems inconsistent with the meaning documented for OpenWorld and could cause UIs/confirmation gates to under-warn. Either set OpenWorld = true here, or restrict the implementation to workspace-only reads so OpenWorld = false is accurate.

    [McpServerTool(Name = "file_read", Destructive = false, OpenWorld = false, ReadOnly = true)]
    [Description("Reads a file from the host filesystem starting at startLine. Returns a JSON object with the line range read, the content, the file's createdAt/modifiedAt timestamps (local time), and an endOfFile flag. A single call is capped by the shared response budget; when endOfFile is false, re-call with startLine = endLine + 1 to continue. On failure, the error field is populated and content is empty.")]
    public async Task<FileReadResult> ReadFile(
        [Description("Path to read. Relative paths are resolved against the agent's workspace; absolute paths are honored as-is, anywhere on disk the host can reach.")] string path,
        [Description("First line to return, 1-indexed. Defaults to 1 (start of file).")] int startLine = 1,

src/LlamaShears.Api/Tools/ModelContextProtocol/Filesystem/ListFilesTool.cs:33

file_list honors absolute paths as-is (outside the workspace), so it can enumerate arbitrary host directories. Setting OpenWorld = false understates the side-effect/scope profile for consumers relying on the hint. Consider setting OpenWorld = true or constraining listing to workspace-only so the hint matches behavior.

    [McpServerTool(Name = "file_list", Destructive = false, OpenWorld = false, ReadOnly = true)]
    [Description("Lists files and directories under the given path on the host filesystem. Returns a JSON object with the resolved path, the recursion flag, an array of entries (each carries name, isDirectory, and sizeBytes for files), the entry count, a truncation flag, and the cap applied. Entries are ordered: directories first, then files, both alphabetically.")]
    public async Task<FileListResult> ListFiles(
        [Description("Path to list. Relative paths resolve against the agent's workspace; absolute paths are honored as-is. Empty (default) lists the agent's workspace root.")] string path = "",
        [Description("If true, recurse into subdirectories. Defaults to false.")] bool recursive = false,

src/LlamaShears.Api/Tools/ModelContextProtocol/Filesystem/WriteFileTool.cs:36

file_write is marked Idempotent = true, but repeated calls with identical arguments are not generally no-ops: with overwrite=false the second call fails once the file exists, and with overwrite=true it still rewrites the file (timestamps/metadata change). Consider setting Idempotent = false (or adjusting semantics so identical calls become true no-ops).

    [McpServerTool(Name = "file_write", Idempotent = true, OpenWorld = false)]
    [Description("Writes the complete file content to a path within the agent's workspace. Returns a JSON object with the path, a written flag, bytesWritten, and whether an existing file was overwritten. By default, refuses if the file already exists; pass overwrite=true to replace it. Writes into the workspace's protected 'system/' subfolder, or any path matched by the workspace file-protection policy, are refused. Parent directories are created if missing. On failure the error field is populated and written=false.")]
    public async Task<FileWriteResult> WriteFile(
        [Description("Path to write. Relative paths resolve against the agent's workspace; absolute paths must still resolve inside the workspace.")] string path,
        [Description("Complete file contents to write. Hard-capped at 1 MiB.")] string content,
        [Description("If true, replace an existing file. Defaults to false (error if the file exists).")] bool overwrite = false,

src/LlamaShears.Api/Tools/ModelContextProtocol/Filesystem/DeleteFileTool.cs:33

file_delete is marked Idempotent = true, but after a successful delete a second identical call returns an error (Path not found). That makes the idempotent hint misleading for clients attempting safe retries. Consider setting Idempotent = false or changing the tool to treat missing paths as a successful no-op (Deleted=false, no error) so it is truly idempotent.

    [McpServerTool(Name = "file_delete", Idempotent = true, OpenWorld = false)]
    [Description("Deletes a file or directory inside the agent's workspace. Returns a JSON object with the path, a deleted flag, and a wasDirectory flag. Directories require recursive=true. Deletes inside the protected 'system/' subfolder, or any path matched by the workspace file-protection policy, are refused. On failure the error field is populated and deleted=false.")]
    public async Task<FileDeleteResult> DeleteFile(
        [Description("Path to delete. Relative paths resolve against the agent's workspace; absolute paths must still resolve inside the workspace.")] string path,
        [Description("If true, allow deleting a non-empty directory recursively. Defaults to false.")] bool recursive = false,
        CancellationToken cancellationToken = default)

src/LlamaShears.Api/Tools/ModelContextProtocol/Cron/CancelCronTool.cs:24

cron_cancel is marked Idempotent = true, but after a successful cancel a second identical call returns an error (job no longer exists). That makes the hint inaccurate for retry/confirmation behavior. Consider setting Idempotent = false or making cancel idempotent by returning Cancelled=false without an error when the job is already absent.

    [McpServerTool(Name = "cron_cancel", Idempotent = true, OpenWorld = false)]
    [Description("Cancels a cron job belonging to the calling agent. Returns a JSON object with the parsed jobId and a cancelled flag. Refuses jobs owned by other agents, unknown ids, or unparseable id strings.")]
    public async Task<CronCancelResult> CancelCron(
        [Description("Cron job id (GUID, format-D).")] string id,
        CancellationToken cancellationToken = default)

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

## Pull request overview Surfaces MCP tool annotation metadata (read-only / destructive / idempotent / open-world) into host-visible abstractions by introducing `ToolDescriptorAnnotations`, plumbing it through the MCP client mapping, and annotating built-in MCP server tools accordingly. **Changes:** - Add `ToolDescriptorAnnotations` and include it on `ToolDescriptor` so hosts/UIs can reason about tool side effects. - Map MCP `ToolAnnotations` from `McpClientTool.ProtocolTool.Annotations` into `ToolDescriptor.Annotations`. - Populate `[McpServerTool]` annotation flags on many built-in API tools and update affected unit tests/docs. ### Reviewed changes Copilot reviewed 28 out of 28 changed files in this pull request and generated 3 comments. <details> <summary>Show a summary per file</summary> | File | Description | | ---- | ----------- | | tests/LlamaShears.UnitTests/Provider/OpenAI/OpenAILanguageModelTests.cs | Update `ToolDescriptor` construction to include annotations. | | tests/LlamaShears.UnitTests/Provider/Ollama/OllamaLanguageModelToolFlatteningTests.cs | Update `ToolDescriptor` construction to include annotations. | | tests/LlamaShears.UnitTests/Agent/Core/InferenceRunnerToolDispatchTests.cs | Update tool advertisement `ToolDescriptor` construction to include annotations. | | src/public/LlamaShears.Core.Abstractions/Provider/ToolDescriptorAnnotations.cs | Introduce new public record describing tool behavior hints. | | src/public/LlamaShears.Core.Abstractions/Provider/ToolDescriptor.cs | Add `Annotations` to the public `ToolDescriptor` record. | | src/LlamaShears.Core/Tools/ModelContextProtocol/ModelContextProtocolClient.cs | Map MCP protocol tool annotations into `ToolDescriptorAnnotations`. | | src/LlamaShears.Api/Tools/ModelContextProtocol/Todo/TodoTools.cs | Add `[McpServerTool]` flags for TODO tools. | | src/LlamaShears.Api/Tools/ModelContextProtocol/Skills/SkillTools.cs | Add `[McpServerTool]` flags for skill tools. | | src/LlamaShears.Api/Tools/ModelContextProtocol/Memory/StoreMemoryTool.cs | Add `[McpServerTool]` flags for memory store tool. | | src/LlamaShears.Api/Tools/ModelContextProtocol/Memory/SearchMemoryTool.cs | Add `[McpServerTool]` flags for memory search tool. | | src/LlamaShears.Api/Tools/ModelContextProtocol/Memory/IndexMemoryTool.cs | Add `[McpServerTool]` flags for memory index tool. | | src/LlamaShears.Api/Tools/ModelContextProtocol/Filesystem/WriteFileTool.cs | Add `[McpServerTool]` flags for file write tool. | | src/LlamaShears.Api/Tools/ModelContextProtocol/Filesystem/RegexReplaceFileTool.cs | Add `[McpServerTool]` flags for regex-replace tool. | | src/LlamaShears.Api/Tools/ModelContextProtocol/Filesystem/ReadFileTool.cs | Add `[McpServerTool]` flags for file read tool. | | src/LlamaShears.Api/Tools/ModelContextProtocol/Filesystem/MoveFileTool.cs | Add `[McpServerTool]` flags for file move tool. | | src/LlamaShears.Api/Tools/ModelContextProtocol/Filesystem/ListFilesTool.cs | Add `[McpServerTool]` flags for file list tool. | | src/LlamaShears.Api/Tools/ModelContextProtocol/Filesystem/GrepTool.cs | Add `[McpServerTool]` flags for grep tool. | | src/LlamaShears.Api/Tools/ModelContextProtocol/Filesystem/DeleteFileTool.cs | Add `[McpServerTool]` flags for file delete tool. | | src/LlamaShears.Api/Tools/ModelContextProtocol/Filesystem/AppendFileTool.cs | Add `[McpServerTool]` flags for file append tool. | | src/LlamaShears.Api/Tools/ModelContextProtocol/Cron/TriggerCronTool.cs | Add `[McpServerTool]` flags for cron trigger tool. | | src/LlamaShears.Api/Tools/ModelContextProtocol/Cron/ScheduleCronTool.cs | Add `[McpServerTool]` flags for cron schedule tool. | | src/LlamaShears.Api/Tools/ModelContextProtocol/Cron/ListCronTool.cs | Add `[McpServerTool]` flags for cron list tool. | | src/LlamaShears.Api/Tools/ModelContextProtocol/Cron/EditCronTool.cs | Add `[McpServerTool]` flags for cron edit tool. | | src/LlamaShears.Api/Tools/ModelContextProtocol/Cron/CancelCronTool.cs | Add `[McpServerTool]` flags for cron cancel tool. | | docs/api/LlamaShears.Core.Abstractions/LlamaShears/Core/Abstractions/Provider/ToolDescriptorAnnotations.md | Add generated API docs for `ToolDescriptorAnnotations`. | | docs/api/LlamaShears.Core.Abstractions/LlamaShears/Core/Abstractions/Provider/ToolDescriptor.md | Update generated API docs for new `ToolDescriptor` parameter/property. | | docs/api/LlamaShears.Core.Abstractions/LlamaShears/Core/Abstractions/Provider/index.md | Add `ToolDescriptorAnnotations` to provider namespace index. | | docs/api/LlamaShears.Core.Abstractions/index.md | Add `ToolDescriptorAnnotations` to root API index. | </details> <details> <summary>Comments suppressed due to low confidence (5)</summary> **src/LlamaShears.Api/Tools/ModelContextProtocol/Filesystem/ReadFileTool.cs:33** * `file_read` allows absolute paths "anywhere on disk the host can reach" (see parameter description and implementation), which is not a closed-domain tool. Marking `OpenWorld = false` seems inconsistent with the meaning documented for OpenWorld and could cause UIs/confirmation gates to under-warn. Either set `OpenWorld = true` here, or restrict the implementation to workspace-only reads so `OpenWorld = false` is accurate. ``` [McpServerTool(Name = "file_read", Destructive = false, OpenWorld = false, ReadOnly = true)] [Description("Reads a file from the host filesystem starting at startLine. Returns a JSON object with the line range read, the content, the file's createdAt/modifiedAt timestamps (local time), and an endOfFile flag. A single call is capped by the shared response budget; when endOfFile is false, re-call with startLine = endLine + 1 to continue. On failure, the error field is populated and content is empty.")] public async Task<FileReadResult> ReadFile( [Description("Path to read. Relative paths are resolved against the agent's workspace; absolute paths are honored as-is, anywhere on disk the host can reach.")] string path, [Description("First line to return, 1-indexed. Defaults to 1 (start of file).")] int startLine = 1, ``` **src/LlamaShears.Api/Tools/ModelContextProtocol/Filesystem/ListFilesTool.cs:33** * `file_list` honors absolute paths as-is (outside the workspace), so it can enumerate arbitrary host directories. Setting `OpenWorld = false` understates the side-effect/scope profile for consumers relying on the hint. Consider setting `OpenWorld = true` or constraining listing to workspace-only so the hint matches behavior. ``` [McpServerTool(Name = "file_list", Destructive = false, OpenWorld = false, ReadOnly = true)] [Description("Lists files and directories under the given path on the host filesystem. Returns a JSON object with the resolved path, the recursion flag, an array of entries (each carries name, isDirectory, and sizeBytes for files), the entry count, a truncation flag, and the cap applied. Entries are ordered: directories first, then files, both alphabetically.")] public async Task<FileListResult> ListFiles( [Description("Path to list. Relative paths resolve against the agent's workspace; absolute paths are honored as-is. Empty (default) lists the agent's workspace root.")] string path = "", [Description("If true, recurse into subdirectories. Defaults to false.")] bool recursive = false, ``` **src/LlamaShears.Api/Tools/ModelContextProtocol/Filesystem/WriteFileTool.cs:36** * `file_write` is marked `Idempotent = true`, but repeated calls with identical arguments are not generally no-ops: with `overwrite=false` the second call fails once the file exists, and with `overwrite=true` it still rewrites the file (timestamps/metadata change). Consider setting `Idempotent = false` (or adjusting semantics so identical calls become true no-ops). ``` [McpServerTool(Name = "file_write", Idempotent = true, OpenWorld = false)] [Description("Writes the complete file content to a path within the agent's workspace. Returns a JSON object with the path, a written flag, bytesWritten, and whether an existing file was overwritten. By default, refuses if the file already exists; pass overwrite=true to replace it. Writes into the workspace's protected 'system/' subfolder, or any path matched by the workspace file-protection policy, are refused. Parent directories are created if missing. On failure the error field is populated and written=false.")] public async Task<FileWriteResult> WriteFile( [Description("Path to write. Relative paths resolve against the agent's workspace; absolute paths must still resolve inside the workspace.")] string path, [Description("Complete file contents to write. Hard-capped at 1 MiB.")] string content, [Description("If true, replace an existing file. Defaults to false (error if the file exists).")] bool overwrite = false, ``` **src/LlamaShears.Api/Tools/ModelContextProtocol/Filesystem/DeleteFileTool.cs:33** * `file_delete` is marked `Idempotent = true`, but after a successful delete a second identical call returns an error (`Path not found`). That makes the idempotent hint misleading for clients attempting safe retries. Consider setting `Idempotent = false` or changing the tool to treat missing paths as a successful no-op (Deleted=false, no error) so it is truly idempotent. ``` [McpServerTool(Name = "file_delete", Idempotent = true, OpenWorld = false)] [Description("Deletes a file or directory inside the agent's workspace. Returns a JSON object with the path, a deleted flag, and a wasDirectory flag. Directories require recursive=true. Deletes inside the protected 'system/' subfolder, or any path matched by the workspace file-protection policy, are refused. On failure the error field is populated and deleted=false.")] public async Task<FileDeleteResult> DeleteFile( [Description("Path to delete. Relative paths resolve against the agent's workspace; absolute paths must still resolve inside the workspace.")] string path, [Description("If true, allow deleting a non-empty directory recursively. Defaults to false.")] bool recursive = false, CancellationToken cancellationToken = default) ``` **src/LlamaShears.Api/Tools/ModelContextProtocol/Cron/CancelCronTool.cs:24** * `cron_cancel` is marked `Idempotent = true`, but after a successful cancel a second identical call returns an error (job no longer exists). That makes the hint inaccurate for retry/confirmation behavior. Consider setting `Idempotent = false` or making cancel idempotent by returning `Cancelled=false` without an error when the job is already absent. ``` [McpServerTool(Name = "cron_cancel", Idempotent = true, OpenWorld = false)] [Description("Cancels a cron job belonging to the calling agent. Returns a JSON object with the parsed jobId and a cancelled flag. Refuses jobs owned by other agents, unknown ids, or unparseable id strings.")] public async Task<CronCancelResult> CancelCron( [Description("Cron job id (GUID, format-D).")] string id, CancellationToken cancellationToken = default) ``` </details> --- 💡 <a href="/jasoncouture/llama-shears/new/main?filename=.github/instructions/*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.

src/LlamaShears.Api/Tools/ModelContextProtocol/Todo/TodoTools.cs

					
				@ -17,3 +17,3 @@

				    [McpServerTool(Name = "todo_add")]

				    [McpServerTool(Name = "todo_add", Destructive = false, OpenWorld = false)]

				    [Description("Appends a batch of items to the agent's TODO list. All items in the batch share the same 'done' flag; for a mixed batch, call twice. Pass an array of one to add a single item. Returns the full list as a JSON object (state, itemCount, items[]). The whole batch is rejected if any item is empty, contains newlines, or exceeds 200 characters.")]

PR description says "Sets the right MCP [McpServerTool] annotations on every internal tool", but shell_run still uses [McpServerTool(Name = "shell_run")] without any of the ReadOnly/Destructive/Idempotent/OpenWorld flags. Either update the PR description to scope out shell_run, or add the appropriate annotations for it as well (it appears to be OpenWorld=true and Destructive=true by default).

PR description says "Sets the right MCP [McpServerTool] annotations on every internal tool", but `shell_run` still uses `[McpServerTool(Name = "shell_run")]` without any of the ReadOnly/Destructive/Idempotent/OpenWorld flags. Either update the PR description to scope out `shell_run`, or add the appropriate annotations for it as well (it appears to be OpenWorld=true and Destructive=true by default).

src/LlamaShears.Core/Tools/ModelContextProtocol/ModelContextProtocolClient.cs

					
				@ -117,0 +128,4 @@

				            destructive,

				            idempotent,

				            openWorld,

				            readOnly);

When readOnlyHint is true but destructiveHint is absent, this mapping will still default Destructive to true, producing a contradictory combination (ReadOnly=true, Destructive=true) that consumers may not special-case. Consider normalizing annotations (e.g., if ReadOnly is true, force Destructive=false and possibly Idempotent=true) so the propagated hints are internally consistent.

When `readOnlyHint` is true but `destructiveHint` is absent, this mapping will still default `Destructive` to true, producing a contradictory combination (ReadOnly=true, Destructive=true) that consumers may not special-case. Consider normalizing annotations (e.g., if ReadOnly is true, force Destructive=false and possibly Idempotent=true) so the propagated hints are internally consistent.

src/public/LlamaShears.Core.Abstractions/Provider/ToolDescriptor.cs

					
				@ -16,3 +17,4 @@

				    string Description,

				    ImmutableArray<ToolParameter> Parameters,

				    ToolDescriptorAnnotations Annotations,

				    JsonElement Schema = default);

ToolDescriptor is a public abstraction and adding a new required positional parameter (Annotations) is a source-breaking change for any downstream callers constructing ToolDescriptor. If you want this to be non-breaking, consider making Annotations optional (nullable with a documented default, or a default value) and/or adding a secondary constructor/factory so existing call sites continue to compile while still surfacing annotations when available.

`ToolDescriptor` is a public abstraction and adding a new required positional parameter (`Annotations`) is a source-breaking change for any downstream callers constructing `ToolDescriptor`. If you want this to be non-breaking, consider making `Annotations` optional (nullable with a documented default, or a default value) and/or adding a secondary constructor/factory so existing call sites continue to compile while still surfacing annotations when available.

Sign in to join this conversation.

No reviewers