Skip to content

feat(memory): add tool annotations#3874

Merged
olaservo merged 2 commits into
modelcontextprotocol:mainfrom
ykshah1309:feat/memory-tool-annotations
May 30, 2026
Merged

feat(memory): add tool annotations#3874
olaservo merged 2 commits into
modelcontextprotocol:mainfrom
ykshah1309:feat/memory-tool-annotations

Conversation

@ykshah1309
Copy link
Copy Markdown
Contributor

Summary

Adds MCP tool annotations to all 9 tools in the memory server. The memory server is currently the only TypeScript reference server without tool annotations — this brings it in line with server-filesystem, server-git, server-time, and server-sequential-thinking.

Changes

  • Add readOnlyHint, destructiveHint, idempotentHint, and openWorldHint to all 9 registerTool definitions in src/memory/index.ts
  • Read tools (read_graph, search_nodes, open_nodes) marked as read-only and idempotent
  • Delete tools (delete_entities, delete_observations, delete_relations) marked as destructive and idempotent (deleting already-removed data is a safe no-op)
  • Write tools (create_entities, create_relations, add_observations) marked as non-read-only and non-destructive (they create/append, not overwrite)
  • All tools marked with openWorldHint: false (purely local JSONL file I/O, no network access)

Testing

Metadata-only change with no behavioral impact.

  • npm run build --workspace=src/memory compiles cleanly
  • npm test --workspace=src/memory — all 45 tests pass

Add MCP ToolAnnotations to all 9 tools in the memory server, matching
the annotation coverage already present in server-filesystem, server-git,
server-time, and server-sequential-thinking.

- Read tools (read_graph, search_nodes, open_nodes): read-only, idempotent
- Write tools (create_entities, create_relations, add_observations): non-destructive
- Delete tools (delete_entities, delete_observations, delete_relations): destructive, idempotent
- All tools: openWorldHint=false (local JSONL file operations only)
@cliffhall cliffhall added enhancement New feature or request server-memory Reference implementation for the Memory MCP server - src/memory labels Apr 20, 2026
@cliffhall
Copy link
Copy Markdown
Member

@claude review

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 21, 2026

Claude finished @cliffhall's task in 1m 57s —— View job


PR Review: feat(memory): add tool annotations

  • Gather context and read changed files
  • Compare annotation patterns with other servers
  • Verify annotation correctness against MCP spec
  • Post review feedback

Overall this is a clean, well-reasoned change. The annotations are correct and the PR achieves its stated goal of bringing the memory server in line with the other TS reference servers. A few observations below.

What's correct

Read tools (read_graph, search_nodes, open_nodes) — readOnlyHint: true, idempotentHint: true, destructiveHint: false, openWorldHint: false. All correct. These tools only read the JSONL file and have no side effects.

Delete tools (delete_entities, delete_observations, delete_relations) — destructiveHint: true, idempotentHint: true. Correct on both counts: deletes remove data permanently, and deleting an already-absent item is a safe no-op, making them genuinely idempotent.

openWorldHint: false on all 9 tools — Correct. The memory server exclusively performs local JSONL file I/O with no network calls.

Minor observations

idempotentHint: false on write tools is defensible but worth double-checking

create_entities and create_relations both deduplicate before writing (lines 121, 129–133), and add_observations similarly filters duplicate content (line 146). Calling any of these twice with identical arguments produces no additional state change — which technically satisfies the MCP spec's definition of idempotent ("calling the tool repeatedly with the same arguments will have no additional effect on the environment").

That said, add_observations can throw if the target entity has been deleted between calls (line 143–145), which breaks unconditional idempotency. And for create_entities/create_relations, the conservative false is a reasonable signal to clients that these tools are intended as creation operations. The false choice is defensible — just worth noting the nuance.

destructiveHint: false on read tools is redundant per spec but harmless

Per the MCP spec, destructiveHint is only meaningful when readOnlyHint: false. When readOnlyHint: true, clients already know no state is modified, so destructiveHint is implied false. Setting it explicitly doesn't hurt, and it's consistent with the sequentialthinking server's style (src/sequentialthinking/index.ts:96), so this is purely cosmetic.

Style divergence from filesystem server is fine

The filesystem server uses a minimal annotation style (only setting non-default values, e.g. annotations: { readOnlyHint: true }), while this PR uses the fully explicit 4-field style. Both are valid; the sequentialthinking server uses the same verbose style as this PR. No change needed.

Summary

The annotation choices are correct and the PR is consistent with the existing codebase patterns. The idempotentHint: false choice for write tools is the only mildly debatable point, but it's defensible and errs on the cautious side. LGTM.

@olaservo olaservo merged commit 64b1cb0 into modelcontextprotocol:main May 30, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request server-memory Reference implementation for the Memory MCP server - src/memory

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants