Address all reviewer comments from PR #387:
1. ✅ Add unit tests for annotations (tests/server/test_annotations.py)
- 10 comprehensive test functions validating all annotation patterns
- Tests for titles, read-only, destructive, idempotent operations
- Validates specific ADR-017 decisions (webdav write, semantic search)
- Cross-category consistency checks
2. ✅ Fix nc_webdav_write_file idempotency classification
- Changed from idempotentHint=False to idempotentHint=True
- Rationale: Uses HTTP PUT without version control
- Writing same content to same path = same end state (idempotent)
3. ✅ Fix semantic search openWorldHint inconsistency
- Changed from openWorldHint=False to openWorldHint=True
- Rationale: Consistent with other Nextcloud tools
- Nextcloud is external to MCP server (indexed data is implementation detail)
4. ✅ Update ADR-017 with resolved decisions
- Converted Open Questions to Resolved Questions
- Added detailed rationale for webdav write and semantic search
- Updated status from Proposed to Implemented
- Added decision timeline with dates
5. ✅ Add MCP Tool Annotations guidelines to CLAUDE.md
- Comprehensive section with code examples for all patterns
- Key principles documented (idempotency, destructive, open world)
- References ADR-017 for detailed rationale
All OAuth tools verified to have proper annotations (oauth_tools.py lines 686-751).
The Smithery scanner was reporting "0 tools" despite the server returning
valid tool definitions. Root cause: the server was returning SSE-formatted
responses (event: message\ndata: {...}) which the scanner couldn't parse.
Changes:
- Add json_response=True to FastMCP for Smithery stateless mode
- Clean up verbose docstring examples in semantic.py and webdav.py
The MCP spec allows both SSE and plain JSON responses for HTTP transport.
Setting json_response=True returns Content-Type: application/json with
plain JSON-RPC instead of text/event-stream with SSE format.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Resolves the token exchange implementation gap where get_session_client()
was implemented but never used by tools. Unifies token acquisition into a
single async get_client() method that handles both pass-through and token
exchange modes transparently.
Core Changes:
- Make get_client() async and merge token exchange logic into it
- Remove scopes parameter from token exchange (Nextcloud doesn't support OAuth scopes)
- Update all 8 tool modules to use await get_client(ctx)
- Fix provisioning decorator to skip checks in BasicAuth mode
Token Acquisition Modes:
1. BasicAuth: Returns shared client (no token operations)
2. OAuth pass-through (default): Verifies and passes Flow 1 token to Nextcloud
3. OAuth token exchange (opt-in): Exchanges Flow 1 token for ephemeral token via RFC 8693
Key Architectural Clarifications:
- Progressive Consent (Flow 1/2) = Authorization architecture
- Token Exchange = Token acquisition pattern during tool execution
- Refresh tokens from Flow 2 are NEVER used for tool calls (only background jobs)
- Nextcloud scopes are "soft-scopes" enforced by MCP server, not IdP
Documentation Updates:
- ADR-004: Added comprehensive token acquisition patterns section
- CRITICAL-TOKEN-EXCHANGE-PATTERN.md: Updated to reflect implementation status
- CLAUDE.md: Updated architectural patterns with async get_client()
Testing:
- All 36 unit tests passing
- All 4 smoke tests passing (BasicAuth mode)
- Linting issues fixed (ruff)
Configuration:
ENABLE_TOKEN_EXCHANGE=false (default) - pass-through mode
ENABLE_TOKEN_EXCHANGE=true (opt-in) - token exchange mode
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Chris Coutinho