Repository navigation
fix(mcp): advertise MCP tool annotations so clients stop treating every tool as destructive - #683
Conversation
…ry tool as destructive tools/list sent no annotations, so MCP clients applied the spec defaults (readOnlyHint: false, destructiveHint: true) and showed all 22 tools, memory.read included, as write + destructive. Each catalog entry now declares readOnlyHint, destructiveHint, idempotentHint and openWorldHint explicitly, and toWireTool passes them through to the edge server and the local stdio server. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
|
The dashboard preview for this PR — redeployed on each push that changes the web app.
Preview always points at this PR's latest commit; Deployment is the immutable build of |
✅ No issues foundThe follow-up commit adds Checked: 1 of 1 changed file read · 0 of 4 dependent files traced · 2 possible issues → 0 confirmed → 0 posted Progress: open review threads 1 → 0 across the last 2 reviews What this change reaches — 4 changed exports · 4 dependent files · 0 checked · 4 not checked · 1 changed file imported by 3 filesflowchart LR
subgraph pr["Changed in this PR"]
s1["MCP_TOOL_DEFS<br/>body changed"]:::changed
s2["MCP_TOOLS<br/>body changed"]:::changed
s3["renderTool<br/>body changed"]:::changed
s4["McpToolAnnotations<br/>added"]:::changed
m1["supabase/functions/_shared/schemas/tool-catalog.ts<br/>file changed"]:::changed
end
s1c1["packages/cli/src/commands/mcp-server.mjs<br/>? not checked"]:::unknown
s1c2["scripts/smoke/smoke-mcp-tools.mjs<br/>? not checked"]:::unknown
s2c1["packages/schemas/src/llms/ · 2 files<br/>? not checked"]:::unknown
s3c1["packages/schemas/src/llms/render.spec.ts<br/>? not checked"]:::unknown
s4c1["packages/schemas/src/llms/render.ts<br/>? not checked"]:::unknown
m1i1["supabase/functions/mcp/ · 2 files<br/>import this file"]:::imports
m1i2["supabase/functions/_shared/schemas/memory.ts<br/>imports this file"]:::imports
s1 -.-> s1c1
s1 -.-> s1c2
s2 -.-> s2c1
s3 -.-> s3c1
s4 -.-> s4c1
m1 -.-> m1i1
m1 -.-> m1i2
classDef changed fill:#eef2ff,stroke:#6366f1,color:#1e1b4b
classDef ok fill:#e7f6ec,stroke:#16a34a,color:#14532d
classDef partial fill:#fef9c3,stroke:#ca8a04,color:#713f12
classDef unknown fill:#f3f4f6,stroke:#9ca3af,stroke-dasharray:4 3,color:#374151
classDef bad fill:#fde8e8,stroke:#dc2626,color:#7f1d1d
classDef warn fill:#fff4e5,stroke:#d97706,color:#78350f
classDef imports fill:#f8fafc,stroke:#64748b,stroke-dasharray:4 3,color:#1e293b
Review details
Found Quality — produced 2 → posted inline 0 · cleared 0 · carried forward 0 · deferred 0 · below-bar 0 Run incremental · 9 lines in delta · tier standard · depth checkout · finders and verifier in-context · consumer trace limited to delta exports (none changed; dependents last traced at be26c3e) Nothing to report — optimality (skipped), integrations (not activated), severity, 0 files skipped.
|
…-operation guide Addresses the pr-reviewer bot's comment: #683 (comment) Refs: #683 Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
Summary
tools/listsent noannotations, so MCP clients applied the spec defaults (readOnlyHint: false,destructiveHint: true,idempotentHint: false,openWorldHint: true) and showed all 22 LoreKit tools,memory.readandmemory.searchincluded, as write + destructive. Every catalog entry now declares all four hints explicitly, and the wire projection passes them through to both the edge server and the locallorekit mcpstdio server.Changes
tool-catalog.ts: new requiredannotations: McpToolAnnotationsfield onMcpToolDoc(all four hints required, so no tool can fall back to a default), aREAD_ONLYconstant and awriteHints({ destructive, idempotent })helper.toWireToolnow emitsannotations.memory.read,memory.list,memory.search,memory.scopes,memory.list_archived,org.list,policy.list,groom.previewmemory.archive,memory.restore,memory.protect,groom.run,org.create,policy.creatememory.write(upsert overwrites the value in place),memory.delete(force: truehard-deletes),memory.purge,memory.purge_expired,org.rename,org.delete,policy.update,policy.deleteopenWorldHint: falseon all of them, since every tool acts only on the LoreKit store.idempotentHint: falseonly formemory.write(bumpsseen_count),org.createandpolicy.create.tool-catalog-parity.spec.ts: assertsreadOnlyHint === (toolRequires(name) === 'read'), that all four hints are present booleans on every tool, that read-only tools are never destructive, and pins the destructive set by name so moving a tool in or out of it has to be done on purpose.surfaces.generated.mjsandllms.txt(each tool block now gets a "MCP annotations: …" line). Added a check to the smoke suite and to the stdio server test. Documented the classification indocs/mcp-tools.md, and added the now-requiredannotationsfield to step 1 and the worked example ofdocs/adding-an-operation.md.Context
2024-11-05.annotationswas added to the spec in2025-03-26, and this PR sends the field without bumping the negotiated version. Clients that read annotations regardless of version will pick it up. A client that only reads them on a newer negotiated version would still need that bump, which is left out of this PR because it changes transport semantics.memory.archive,groom.run) is marked non-destructive becausememory.restorereverses it.org.renameandpolicy.updateare marked destructive because they overwrite in place.mcp-coresrc/mcp-guards/*,src/edge/*,permissions.spec.ts; allschemasspecs;tsc --noEmitforschemasandmcp-core;clitest/mcp-server.test.mjs. CI coversdeno checkand lint.