feat: upgrade fastmcp 2.12.3 to 3.0.1 with tool annotations#598
feat: upgrade fastmcp 2.12.3 to 3.0.1 with tool annotations#598phernandez merged 3 commits intomainfrom
Conversation
Upgrades from pinned fastmcp==2.12.3 (pinned due to tool visibility regression in 2.14.x, issue #463) to fastmcp>=3.0.1,<4. Breaking changes addressed: - Remove `.fn` from all tool callsites (~540 occurrences across 53 files) In v3, @mcp.tool() returns the original function directly - Make context.get_state()/set_state() async with await (7 callsites) - Serialize Pydantic models via model_dump()/model_validate() for JSON-serializable context state persistence New features adopted: - Add MCP tool annotations to all 26 tools (readOnlyHint, destructiveHint, idempotentHint, openWorldHint) enabling smarter MCP client UI decisions like skipping confirmation prompts for read-only operations 🧪 All 1946 unit tests + 281 integration tests pass (SQLite) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
|
Claude finished @phernandez's task in 3m 3s —— View job PR Review: feat/upgrade-fastmcp-3.0Tasks
SummaryThis is a well-scoped, mechanical upgrade with one genuinely tricky behavioral change (async state serialization). The bulk of the diff (~540 Code Quality & Standards
Testing & Documentation
Basic Memory Architecture
Security & Performance
Compatibility
Issues Found1. Annotation Semantic Accuracy —
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 816aa54826
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| @mcp.tool( | ||
| description="Edit an existing markdown note using various operations like append, prepend, find_replace, or replace_section.", | ||
| annotations={"destructiveHint": False, "idempotentHint": True, "openWorldHint": False}, |
There was a problem hiding this comment.
Mark edit_note as non-idempotent
edit_note supports append and prepend, which are not idempotent: repeating the same request adds duplicate content and changes the note again. Advertising idempotentHint: True can cause MCP clients that trust this hint to auto-retry the tool on transient failures, resulting in duplicated edits and user-visible data corruption in exactly those retry scenarios.
Useful? React with 👍 / 👎.
With .fn removed, pyright now sees the real return types (str | dict) of tools that support output_format. Fix by: - Adding str() narrowing where callers always use text mode - Passing output_format="json" explicitly in CLI search (was implicit) - Providing defaults for depth/timeframe instead of passing None Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
|
Claude finished @phernandez's task in 2m 15s —— View job PR Review: feat/upgrade-fastmcp-3.0Tasks
Updated Review — Commit
|
| Issue | Status |
|---|---|
edit_note idempotentHint: True is inaccurate for append/prepend |
Still open |
move_note destructiveHint: False may be inaccurate (can overwrite) |
Still open (lower priority) |
| No tests asserting annotation values per tool | Still open |
Sync def functions cloud_info / release_notes with fastmcp 3.x |
Still open (verify only) |
The highest-impact unresolved item remains edit_note idempotentHint: True, flagged independently by both this review and the Codex reviewer. MCP clients that trust idempotentHint: True may auto-retry append/prepend on transient failures, producing duplicate content.
Fix edit_note idempotentHint →
Checklist
Code Quality & Standards
- Follows Basic Memory coding conventions
- Python 3.12+ type annotations and async patterns
- N/A — no SQLAlchemy changes
- FastAPI/Typer conventions followed
- 100-character line length maintained
- No commented-out code blocks
Testing & Documentation
- Pyright fixes are covered by existing tests (no new paths introduced)
- CLI tests in
test_cli_tool_json_output.pyupdated in prior commit - [
⚠️ ] No annotation contract tests (open from first review)
Basic Memory Architecture
- No architecture changes in this commit
- MCP tool composability unaffected
- Local-first principles preserved
Security & Performance
- No new security concerns
-
json.dumpswith explicitoutput_format="json"is more correct than priordefault=strfallback
Compatibility
- No file path comparison changes
- No emoji/unicode added
Recommendation
The latest commit (31c145e) is clean and correct — approve as-is for the pyright fixes. The outstanding item worth addressing before merge is edit_note idempotentHint: True → False.
edit_note supports append/prepend operations which are not idempotent — repeating the same call adds duplicate content. Marking it idempotent could cause MCP clients to auto-retry on transient failures, producing duplicated edits. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
|
Claude finished @phernandez's task in 1m 49s —— View job PR Review: feat/upgrade-fastmcp-3.0Tasks
Updated Review — Commit
|
| Issue | Status |
|---|---|
edit_note idempotentHint: True is inaccurate for append/prepend |
✅ Fixed in 2a458c8 |
move_note destructiveHint: False may be inaccurate (can overwrite) |
|
| No tests asserting annotation values per tool | |
Sync def functions cloud_info / release_notes with fastmcp 3.x |
✅ Non-issue — all 2227 tests pass, fastmcp 3.x handles sync functions correctly |
Annotation Consistency Check (Full Review)
Reviewing all tool annotations in the codebase:
| Tool | Annotations | Assessment |
|---|---|---|
read_note, view_note, read_content, build_context, recent_activity, list_directory, search_notes, search, schema_*, list_memory_projects, list_workspaces, chatgpt fetch/search, cloud_info, release_notes |
readOnlyHint: True |
✅ Correct |
write_note |
destructiveHint: False, idempotentHint: True |
✅ Correct — upsert semantics make it genuinely idempotent |
canvas |
destructiveHint: False, idempotentHint: True |
✅ Correct — same args always produce the same canvas file |
edit_note |
destructiveHint: False |
✅ Fixed — idempotentHint correctly absent |
delete_note, delete_project |
destructiveHint: True |
✅ Correct |
create_memory_project |
destructiveHint: False |
✅ Correct |
move_note |
destructiveHint: False |
The one remaining annotation concern is move_note. If the destination path already exists, the move can silently overwrite the target file. Setting destructiveHint: True would be more conservative and accurate per spec intent ("may perform destructive updates"). That said, this is a low-priority judgment call — the tool's core purpose is a non-destructive restructuring operation.
Checklist
Code Quality & Standards
- Follows Basic Memory coding conventions
- Python 3.12+ type annotations and async patterns
- N/A — no SQLAlchemy changes
- FastAPI/Typer conventions followed
- 100-character line length maintained
- No commented-out code blocks
Testing & Documentation
- No new code paths introduced — this is a one-line annotation fix
- Existing tests unaffected
- [
⚠️ ] No annotation contract tests (open from prior reviews, nice-to-have)
Basic Memory Architecture
- No architecture changes
- MCP tool composability unaffected
- Local-first principles preserved
Security & Performance
- No new security concerns
- No behavioral changes — annotation-only fix
Compatibility
- No file path comparison changes
- No emoji/unicode added
Recommendation
Approve. The primary blocker raised by both reviews — edit_note advertising itself as idempotent while supporting non-idempotent append/prepend — is now fixed. The remaining open items are low-priority (annotation contract tests) or a minor judgment call (move_note destructive hint). The core fastmcp 3.0 migration is mechanically sound, all tests pass, and the codebase is in a clean state.
If the team wants to address the remaining items before merge:
move_notedestructiveHint→ setdestructiveHint: Truesince it can overwrite existing files at the destination- Add annotation contract tests → to guard against annotation regressions
Summary
fastmcpfrom pinned 2.12.3 to>=3.0.1,<4, resolving the tool visibility regression (issue Pin FastMCP to 2.12.3 - Version 2.14.x breaks MCP tools visibility #463).fnfrom ~540 tool callsites across 53 files (v3 returns the original function from@mcp.tool())context.get_state()/set_state()async withmodel_dump()/model_validate()for JSON-serializable statereadOnlyHint,destructiveHint,idempotentHint,openWorldHint) enabling smarter client UI decisionsTest plan
just run-inspector)🤖 Generated with Claude Code