Commit Graph
100 Commits
Author SHA1 Message Date
Chris CoutinhoandClaude 6fe5596c13 feat: Implement BM25 hybrid search with native Qdrant RRF fusion
Replace custom keyword/fuzzy search algorithms with industry-standard BM25
sparse vectors, combined with dense semantic vectors using Qdrant's native
Reciprocal Rank Fusion (RRF). This consolidates search architecture and
improves relevance for both semantic and keyword queries.

Key changes:
- Add fastembed dependency for BM25 sparse vector generation
- Update Qdrant collection schema to support named vectors (dense + sparse)
- Create BM25SparseEmbeddingProvider using FastEmbed's Qdrant/bm25 model
- Implement BM25HybridSearchAlgorithm with native Qdrant RRF prefetch
- Update document processor to generate both dense and sparse embeddings
- Simplify nc_semantic_search() tool to use BM25 hybrid only
- Remove legacy keyword.py, fuzzy.py, and custom hybrid.py (736 lines)
- Update ADR-014 with implementation notes and test results

Benefits:
- Consolidated architecture (single Qdrant database)
- Native database-level RRF fusion (more efficient)
- Industry-standard BM25 (replaces brittle custom keyword search)
- Better relevance across semantic and keyword queries
- Simplified codebase (-285 net lines)

Tests: All 125 tests passing (118 unit, 7 integration)

Implements ADR-014: Replace Custom Keyword Search with BM25 Hybrid Search

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-16 06:59:44 +01:00
Chris Coutinho f5bc3e3bc3 docs: init ADR 2025-11-16 06:24:25 +01:00
Chris CoutinhoandClaude eaeb8eae28 feat: Normalize hybrid search RRF scores to 0-1 range
Improve user comprehension by scaling RRF scores to match the intuitive
0-1 range used by other search algorithms.

## Problem

RRF (Reciprocal Rank Fusion) scores had a drastically different scale
than semantic/keyword/fuzzy scores:

- Semantic similarity: 0.0 to 1.0 (typical: 0.5-0.9)
- RRF scores: 0.0 to ~0.016 (typical: 0.005-0.015)

This caused user confusion - a score of 0.0078 looked terrible but was
actually excellent (near theoretical maximum).

## Solution

Normalize RRF scores using the formula:
`normalized_score = rrf_score * (rrf_k + 1) / total_weight`

Where:
- rrf_k = 60 (RRF constant)
- total_weight = sum of algorithm weights (default: 1.0)

**Example transformation:**
- Before: 0.0078 (confusing)
- After: 0.477 (intuitive)

## Changes

**nextcloud_mcp_server/search/hybrid.py:**
- Store total_weight as instance variable (line 63)
- Calculate normalization factor in _reciprocal_rank_fusion() (line 209)
- Apply normalization to all RRF scores (line 217)
- Preserve raw RRF score in metadata for debugging (line 222)

## Impact

**User Experience:**
- Hybrid search scores now comparable with semantic/keyword/fuzzy
- Score of 0.5 indicates good match across all algorithms
- Consistent scale improves score threshold usability

**Backward Compatibility:**
- Raw RRF scores preserved in metadata["rrf_score_raw"]
- Result ordering unchanged (normalization is linear transformation)
- Breaking change: Existing score thresholds need adjustment

**Performance:**
- Negligible overhead (single multiplication per result)

## Testing

Verified with nc_semantic_search and nc_semantic_search_answer:
- Hybrid scores now 0.47-0.7 range (was 0.003-0.011)
- Semantic scores unchanged (0.75)
- Result ordering preserved

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-15 06:48:58 +01:00
Chris CoutinhoandClaude 42376483ab refactor: Optimize Nextcloud access verification with centralized filtering
Move access verification from individual search algorithms to final output
stage, eliminating redundant API calls and improving performance.

## Changes

**New:**
- `search/verification.py`: Centralized verification using anyio task groups
  - Deduplicates results by (doc_id, doc_type) before verification
  - Verifies all unique documents in parallel using structured concurrency
  - Filters out inaccessible documents in single pass

**Modified Search Algorithms:**
- `search/semantic.py`: Removed _deduplicate_and_verify() and _verify_document_access()
- `search/keyword.py`: Removed _verify_access() and parallel verification
- `search/fuzzy.py`: Removed _verify_access() and parallel verification
- `search/hybrid.py`: Removed nextcloud_client parameter passing

All algorithms now return unverified results from Qdrant payload.

**Modified Output Stages:**
- `server/semantic.py`: Added verify_search_results() call after search
- `auth/viz_routes.py`: Added verify_search_results() call after search

Both endpoints now verify access once at final stage with deduplication.

## Performance Impact

**Before:**
- Hybrid mode (limit=10): 30 API calls (10 per algorithm × 3 algorithms)
- Single algorithm: 10-20 API calls (with verification buffer)

**After:**
- Hybrid mode (limit=10): 10 API calls (deduplicated verification)
- Single algorithm: 10 API calls (deduplicated verification)

**Performance Gain:** 3x reduction in API calls for hybrid search

## Architecture Benefits

- **Separation of concerns**: Algorithms handle scoring, output stage handles security
- **Deduplication**: Each document verified exactly once
- **Parallel execution**: All verifications run concurrently via anyio task groups
- **Consistency**: Same verification logic across MCP tools and viz endpoints

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-15 06:21:06 +01:00
Chris CoutinhoandClaude ed0825e661 feat: Enhance vector visualization UI and parallelize search verification
Vector Visualization Improvements:
- Add interactive vector viz tab with Alpine.js and Plotly.js to user info page
- Refactor viz route CSS for better scoping and maintainability
- Remove unused nextcloud_host variable

Performance Optimizations:
- Parallelize access verification in fuzzy and keyword search algorithms
- Use asyncio.gather() to verify multiple documents concurrently
- Add exception handling with return_exceptions=True for resilience

Dependencies:
- Update third_party/oidc submodule to include RFC 9728 resource_url support

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-15 05:39:07 +01:00
Chris CoutinhoandClaude e3153822f7 perf: Exclude vector-sync status polling from distributed tracing
Skip tracing for /app/vector-sync/status to reduce noise from HTMX polling.
Metrics collection continues for this endpoint.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-15 05:19:35 +01:00
Chris Coutinho 2b35dd729f fix: Reorder tabs and fix viz pane session access
- Move Webhooks tab to the right (User Info | Vector Sync | Vector Viz | Webhooks)
- Use request.user.display_name instead of session for viz routes
- Fixes session middleware error when accessing via iframe
2025-11-15 02:41:42 +01:00
Chris Coutinho eb32bbbc6b feat: Add Vector Viz tab to app home page
- Add Vector Viz button to tab navigation
- Embed viz pane in iframe for seamless integration
- Only shown when vector sync is enabled
2025-11-15 02:38:05 +01:00
Chris Coutinho 916af1c8f3 feat: Add vector visualization pane with multi-select document types
- Add /app/vector-viz endpoint for interactive search testing
- Implement server-side PCA dimensionality reduction (768-dim → 2D)
- Support multi-select document type filter for cross-app search
- Support all search algorithms: semantic, keyword, fuzzy, hybrid
- Display 2D scatter plot of vector embeddings using Plotly
- Show search results with scores and document types
- Register viz routes in app.py
2025-11-15 02:32:10 +01:00
Chris Coutinho 9a62c8478f feat: Implement custom PCA to remove sklearn dependency
- Add custom PCA implementation using numpy eigendecomposition
- Replace sklearn.decomposition.PCA with custom implementation
- Maintains same API (fit, transform, fit_transform)
- Supports explained_variance_ratio_ for variance analysis
- Removes scikit-learn dependency from project
- Add type hints and assertion for type safety
2025-11-15 02:02:57 +01:00
Chris Coutinho 2a078093ed refactor!: Make all search algorithms query Qdrant payload, not Nextcloud
BREAKING CHANGE: Search algorithms now require Qdrant to be populated.
Vector sync must be enabled and documents indexed for search to work.

- Keyword and fuzzy search now query Qdrant scroll API for title/excerpt
- Remove inefficient Nextcloud API fetching pattern
- Add optional Nextcloud verification for security
- Deduplicate by (doc_id, doc_type) tuple, keeping chunk_index=0
- Align with document processor pattern that already stores text in Qdrant
2025-11-15 01:56:41 +01:00
Chris CoutinhoandClaude b5b03bfd78 feat: Add multi-document Protocol with cross-app search support
Implements NextcloudClientProtocol for multi-document type search following
user requirement that document types are not 1:1 with apps (e.g., Notes app
specializes in markdown, while Files/WebDAV handles multiple file types).

Key Changes:
- NextcloudClientProtocol: Generic protocol with app-specific client properties
- get_indexed_doc_types(): Query Qdrant for actually-indexed document types
- Document dispatch: All algorithms check Qdrant before attempting access
- Cross-type deduplication: Use (doc_id, doc_type) tuples in hybrid RRF

Search Algorithm Updates:
- Semantic: Added _verify_document_access() with dispatch to appropriate client
  - Deduplication by (doc_id, doc_type) tuple
  - Only "note" verification implemented, others return None with info log
- Keyword: Added _fetch_documents() dispatch method
  - Queries Qdrant for available types before fetching
  - Supports cross-app search when doc_type=None
- Fuzzy: Same pattern as keyword search
- Hybrid: Already uses (doc_id, doc_type) for deduplication (no changes needed)

Future-Proof Design:
- File/calendar verification stubs in place
- Clear logging when unsupported types found
- Easy to extend when processor indexes new document types

Currently Supported:
- "note" documents fully implemented and tested
- Other types gracefully handled (logged but skipped)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-15 01:19:29 +01:00
Chris CoutinhoandClaude f3bdb8b885 feat: Update nc_semantic_search tool with algorithm selection
Implements ADR-012 by adding multi-algorithm support to the MCP tool.

Key changes:
- Added algorithm parameter: "semantic"|"keyword"|"fuzzy"|"hybrid" (default: "hybrid")
- Added weight parameters for hybrid mode configuration
- Replaced direct Qdrant/embedding calls with search module abstractions
- Updated docstring to describe all four algorithms
- Simplified implementation: ~50 lines vs ~150 lines (67% reduction)
- Better error handling for missing vector sync

Algorithm selection:
- semantic: Pure vector similarity (requires VECTOR_SYNC_ENABLED=true)
- keyword: Token-based matching with weighted title/content scoring
- fuzzy: Character overlap for typo tolerance
- hybrid: RRF fusion with configurable weights (default: 0.5/0.3/0.2)

Backward compatibility:
- Tool name unchanged (nc_semantic_search)
- New parameters have sensible defaults
- Existing clients get hybrid search automatically (better than pure semantic)
- search_method field in response reflects actual algorithm used

Weight validation:
- Performed in HybridSearchAlgorithm constructor
- Must sum to ≤1.0 and all non-negative
- At least one weight must be > 0
- Clear error messages on validation failure

Next: Update viz pane to use same algorithms

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-15 00:25:55 +01:00
Chris CoutinhoandClaude 11e620f2d1 feat: Implement unified search algorithm module
Creates shared search module with four algorithms implementing ADR-012:
- Semantic search (vector similarity via Qdrant)
- Keyword search (token-based matching from ADR-001)
- Fuzzy search (character overlap matching)
- Hybrid search (RRF fusion from ADR-003)

Architecture:
- Base SearchAlgorithm interface for consistent API
- SearchResult dataclass for unified result format
- All algorithms async and independently testable
- Proper logging and error handling throughout

Semantic Search (search/semantic.py):
- Extracted from server/semantic.py
- Vector similarity using Qdrant query_points
- Dual-phase authorization (vector filter + API verification)
- Deduplication of document chunks
- Configurable score threshold (default: 0.7)

Keyword Search (search/keyword.py):
- Implements ADR-001 token-based matching
- Title matches weighted 3x higher than content
- Case-insensitive token matching
- Relevance scoring with normalization
- Excerpt extraction with context

Fuzzy Search (search/fuzzy.py):
- Simple character overlap calculation
- Configurable threshold (default: 70%)
- Typo-tolerant matching
- Fast and dependency-free

Hybrid Search (search/hybrid.py):
- Reciprocal Rank Fusion (RRF) from ADR-003
- Parallel execution of sub-algorithms
- Configurable weights per algorithm
- RRF constant k=60 (standard value)
- Weight validation (must sum ≤1.0)

All algorithms:
- Share NextcloudClient for document access
- Support user_id filtering (multi-tenant)
- Support doc_type filtering (currently notes only)
- Return consistent SearchResult objects
- Properly formatted with ruff and type-checked

Next steps: Update MCP tool to use these algorithms

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-15 00:10:19 +01:00
Chris CoutinhoandClaude 56bd85c0f7 docs: Emphasize server-side processing in ADR-012 viz pane
Updates ADR-012 to clarify that all search and filtering operations
must happen server-side, not in the browser.

Key changes:
- Enhanced viz pane data flow showing server-side processing
- Added performance benefits section (384x bandwidth reduction)
- Detailed server-side filtering approach:
  * Query execution via search/algorithms.py
  * User ID filtering (multi-tenant security)
  * Document type filtering
  * PCA reduction (768-dim → 2D) on server
  * Only 2D coordinates + metadata sent to client
- Updated Phase 3 implementation plan:
  * Remove ALL client-side search logic
  * Implement /app/vector-viz server endpoint
  * htmx form submission for queries
  * Performance optimizations (caching, streaming)

This ensures:
- Minimal bandwidth usage (only 2 floats per doc vs 768)
- Client handles only visualization, not computation
- Can visualize 10,000+ documents without client lag
- Raw vectors never leave server (security)
- Same search logic as MCP tool (consistency)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-15 00:02:54 +01:00
Chris CoutinhoandClaude 5e67277049 docs: Add architecture diagrams and viz pane UI to ADR-012
Enhances ADR-012 with detailed architecture visualization and UI mockup
for the vector visualization pane.

Added sections:
- Architecture diagram showing MCP tool and viz pane integration
- Data flow diagrams for both MCP requests and viz pane interactions
- Detailed UI mockup with ASCII art showing:
  * Search configuration controls
  * Algorithm selector with weight sliders
  * Interactive 2D scatter plot (Plotly.js)
  * Results panel with scores
  * Performance comparison table
- Technology stack details (htmx, Alpine.js, Plotly.js, Tailwind CSS)

The diagrams illustrate how the viz pane and MCP tool share the same
search algorithm implementations from search/algorithms.py, ensuring
consistency between user testing interface and programmatic API.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-15 00:00:40 +01:00
Chris CoutinhoandClaude 66a7109130 docs: Add ADR-012 for unified multi-algorithm search
Proposes unified search architecture with client-configurable algorithm
selection and weighting. Addresses the need for flexible search options
beyond pure semantic search.

Key features:
- Four algorithms: semantic, keyword, fuzzy, hybrid
- Client-configurable weights for hybrid search
- Shared implementation between viz pane and MCP tools
- Reciprocal Rank Fusion (RRF) for result combination
- Backward compatible with existing nc_semantic_search()

Implements designs from:
- ADR-003: Hybrid search with RRF (previously unimplemented)
- ADR-001: Token-based keyword search (previously unimplemented)

Supersedes ADR-011's placeholder for "ADR-013: Hybrid Search"

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-14 23:56:09 +01:00
Chris CoutinhoandClaude 00e72d24a6 feat: Enable SSE transport for mcp service and update test fixtures
Changes:
- Remove streamable-http transport override from mcp service in docker-compose.yml
- Service now uses CLI default SSE transport on /sse endpoint
- Add create_mcp_client_session_sse() helper for SSE connections
- Update nc_mcp_client fixture to use SSE transport
- Fix unpacking for SSE client (yields 2 values vs 3 for streamable-http)

Testing:
- All 4 smoke tests pass with SSE transport
- 32/34 affected tests pass (2 skipped for vector sync)
- OAuth services remain on streamable-http (unchanged)

Note: SSE transport is being deprecated in favor of streamable-http.
This enables minimal validation testing before deprecation.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-14 19:20:30 +01:00
Chris CoutinhoandClaude 26099d643d docs: Update ADR-011 to rejected status with Context Agent validation
After comprehensive research, the hybrid OAuth + AppAPI architecture is NOT
being implemented due to fundamental architectural incompatibilities.

Key updates:
- Status: Proposed → Not Planned
- Added validation from Nextcloud Context Agent project
- Context Agent (official NC ExApp with MCP) faces IDENTICAL limitations
- Proves constraints are architectural, not implementation-specific

Context Agent findings:
- ExApp with MCP server endpoint (~28 tools exposed)
- Uses Task Processing API for confirmations (NOT MCP elicitation)
- Works around AppAPI proxy limitations by changing protocol
- MCP endpoint is secondary feature with documented constraints
- Primary use: In-app Assistant integration, not external MCP clients

Critical features impossible through AppAPI proxy:
-  MCP sampling (eliminates RAG/LLM features)
-  MCP elicitation (user prompts)
-  Real-time progress updates
-  Bidirectional streaming
- Validated by Context Agent facing same limitations

Decision rationale:
- MCP requires multi-turn nested interactions
- AppAPI provides stateless request/response proxy only
- No implementation effort can bridge this fundamental gap
- Would require complete AppAPI redesign (WebSocket, message routing)
- Even official Nextcloud projects work around these limitations

Alternative considered for future:
- Register as Task Processing provider (different product)
- Use Nextcloud Assistant UI (not external MCP clients)
- Accept different capabilities (no sampling, custom flows)

OAuth mode remains sole solution for external MCP client integration.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-13 23:30:14 +01:00
Chris Coutinho 39259ef282 ci: Run smoke tests only in ci 2025-11-13 22:06:07 +01:00
Chris CoutinhoandClaude 3648d478f1 fix: return all notes when search query is empty
Previously, an empty query string to nc_notes_search_notes would return
zero results due to an early return when no query tokens were present.

This was counterintuitive - users expect an empty query to list all
notes, not return nothing.

Changes:
- Modified NotesSearchController.search_notes() to return all notes
  when query is empty
- Added documentation to clarify this behavior
- Empty query results have _score: None (no relevance scoring)
- Non-empty query results continue to have relevance scores

Fixes behavior where listing all notes was impossible via the search tool.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-13 21:57:14 +01:00
Chris CoutinhoandClaude 14a59fdff3 fix: Use NEXTCLOUD_OIDC_CLIENT_ID/SECRET env vars consistently
Fixes #296

The application code was looking for OIDC_CLIENT_ID and OIDC_CLIENT_SECRET
(without NEXTCLOUD_ prefix), but the Helm chart, documentation, and CLI
all use NEXTCLOUD_OIDC_CLIENT_ID and NEXTCLOUD_OIDC_CLIENT_SECRET.

This mismatch caused OAuth deployments via Helm to fail with crashloops
because the credentials weren't being found.

Changes:
- app.py: Use NEXTCLOUD_OIDC_CLIENT_ID/SECRET in setup_oauth_config()
- config.py: Use NEXTCLOUD_OIDC_CLIENT_ID/SECRET in get_settings()
- Updated documentation comments and error messages

This aligns with the documented naming convention where all Nextcloud-related
environment variables use the NEXTCLOUD_ prefix.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-13 21:48:58 +01:00
Chris Coutinho c3023d2cc3 feat: Complete Phase 5 - Instrument all 93 MCP tools
Applied @instrument_tool decorator to all 86 remaining tools
across 8 server files.

Instrumented files:
- calendar.py: 16 tools
- contacts.py: 7 tools
- deck.py: 25 tools
- webdav.py: 11 tools
- tables.py: 6 tools
- sharing.py: 5 tools
- cookbook.py: 13 tools
- semantic.py: 3 tools

Total: 93 tools instrumented (7 in notes.py + 86 in other files)

These metrics populate:
- MCP Tool Calls panel (by tool name and status)
- MCP Tool Duration panel (histogram)
- MCP Tool Errors panel (by tool name and error type)

This completes PR #295 - All 5 phases of metrics instrumentation done:
 Phase 1: Queue size metrics (2 locations)
 Phase 2: Health checks (1 location)
 Phase 3: Database operations (3 methods)
 Phase 4: OAuth token metrics (3 locations)
 Phase 5: MCP tool metrics (93 tools)

All 34 dashboard panels now have data sources.
2025-11-13 16:58:44 +01:00
Chris Coutinho 6253faee19 feat: Add instrumentation decorator and apply to notes tools (Phase 5)
Created @instrument_tool decorator for automatic MCP tool metrics collection.
Applied to all 7 tools in notes.py.

Changes:
- observability/metrics.py:
  * New instrument_tool() decorator for automatic timing and error tracking
  * Compatible with @mcp.tool() and @require_scopes() decorators
  * Records tool_name, duration, and success/error status

- server/notes.py:
  * Applied @instrument_tool to all 7 tool functions
  * nc_notes_create_note, nc_notes_update_note, nc_notes_append_content
  * nc_notes_search_notes, nc_notes_get_note, nc_notes_get_attachment
  * nc_notes_delete_note

These metrics will populate the MCP Tool Calls dashboard panels.

Part of PR #295 - Complete metrics instrumentation (Phase 5)
Remaining: 86 tools across 8 server files
2025-11-13 16:40:56 +01:00
Chris Coutinho c97f12d47e feat: Add OAuth token and database metrics (Phases 3-4)
Complete Prometheus instrumentation for OAuth token operations
and additional database operations to populate empty dashboard panels.

OAuth Token Metrics (Phase 4):
- unified_verifier.py:
  * Token validation cache hits/misses
  * JWT verification success/failure/error
  * Introspection validation results
  * Audience validation failures
- context_helper.py:
  * Token exchange cache hits/misses
  * RFC 8693 exchange success/error

Database Metrics (Phase 3 completion):
- storage.py:
  * get_refresh_token() with timing
  * delete_refresh_token() with timing
  * All operations record duration and success/error status

These metrics populate the following dashboard panels:
- Token Validations (by method and result)
- Token Cache Hit Rate
- Token Exchange Operations
- Database Operations (refresh token CRUD)
- Database Operation Duration

Part of PR #295 - Complete metrics instrumentation
2025-11-13 16:23:00 +01:00
Chris CoutinhoandClaude a667d7c59c feat: Add metrics instrumentation for queue, health, and database operations
Implement Prometheus metrics to populate empty Grafana dashboard panels.

## Phase 1: Queue Size Metrics 
**File**: `processor.py`
- Track vector sync queue depth in real-time
- Update metric after receiving and processing each document
- Update metric during timeout (empty queue)
- Enables: "Processing Queue Depth" panel

## Phase 2: Health Check Metrics 
**File**: `app.py`
- Add Nextcloud connectivity check with timing
- Add Qdrant health check with timing
- Record dependency health status (up/down)
- Record health check duration
- Enables: 4 health status panels + health check duration panel

## Phase 3: Database Operation Metrics (Partial) 
**File**: `storage.py`
- Instrument `store_refresh_token()` method
- Track SQLite INSERT operation timing and success/error status
- Enables: Partial data for database operation latency panel

## Metrics Now Exposed

### Queue Metrics:
- `mcp_vector_sync_queue_size` - Real-time queue depth

### Health Metrics:
- `mcp_dependency_health{dependency="nextcloud"}` - UP/DOWN status
- `mcp_dependency_health{dependency="qdrant"}` - UP/DOWN status
- `mcp_dependency_check_duration_seconds{dependency}` - Health check latency

### Database Metrics:
- `mcp_db_operations_total{db="sqlite",operation="insert"}` - Operation count
- `mcp_db_operation_duration_seconds{db="sqlite",operation="insert"}` - Operation latency

## Dashboard Impact

**Panels Now Populated** (7/34 panels):
-  Processing Queue Depth
-  Nextcloud Health
-  Qdrant Health
-  Health Check Duration
-  Database Operation Latency (partial)
-  Vector sync panels (already working from PR #292)

**Panels Still Empty** (remaining work):
-  OAuth panels (4): Token validations, exchanges, cache hit rate, refresh ops
-  MCP tool panels (3): Call volume, error rates, execution duration
-  Database panel: Needs more SQLite operations instrumented (~29 remaining)

## Testing

Verified metric definitions exist and will be recorded on next deployment.

## Next Steps

Phase 4: OAuth token metrics (unified_verifier.py, context_helper.py, storage.py)
Phase 5: MCP tool metrics (all server/*.py files with @mcp.tool())
Phase 3 completion: Remaining 29 database operations in storage.py

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-13 16:14:38 +01:00
Chris CoutinhoandClaude ff3123a190 docs: Add ADR-011 for hybrid OAuth + AppAPI deployment architecture
This ADR documents the architectural decision to support both OAuth and
AppAPI (ExApp) deployment modes in a single codebase with 90%+ code sharing.

Key additions:
- Comprehensive analysis of AppAPI limitations and challenges
- Feature parity matrix comparing OAuth vs AppAPI modes
- Resolution of critical open questions via research:
  * Non-browser client authentication (app passwords/OAuth)
  * Streaming transport compatibility (buffered, not real-time)
  * Callbacks/webhooks (MCP notifications not possible in AppAPI)
- Detailed implementation plan with 4 phases (10 days)
- Mode-aware architecture with abstraction layer

Critical findings:
- AppAPI mode does NOT support MCP sampling (RAG features)
- No real-time progress updates (use Nextcloud notifications)
- Buffered streaming only (Streamable HTTP works, WebSocket doesn't)
- Requires app password support in AppAPI proxy

Deployment mode selection:
- OAuth: Multi-tenant, external clients, sampling/RAG, real-time updates
- AppAPI: Single-tenant, simplified install, native UI, admin-controlled

Related to investigation of ~/Software/app_api/ and ~/Software/nc_py_api/
for AppAPI integration patterns.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-13 13:10:21 +01:00
Chris CoutinhoandClaude 4e43d15153 fix: Move grafana_folder from labels to annotations
Fixes Kubernetes label validation error when deploying dashboard ConfigMap.

Problem:
- Kubernetes labels cannot contain spaces (validation regex: [A-Za-z0-9][-A-Za-z0-9_.]*[A-Za-z0-9])
- Previous implementation had grafana_folder: "Nextcloud MCP" as a label
- Deployment failed with: "Invalid value: 'Nextcloud MCP'"

Solution:
- Move grafana_folder from labels to annotations (annotations allow spaces)
- Keep grafana_dashboard="1" as label for ConfigMap discovery
- Grafana sidecar reads folder name from folderAnnotation parameter

Changes:
- dashboard-configmap.yaml: Move grafana_folder to annotations section
- dashboards/README.md: Fix kubectl commands to use annotations
- values.yaml: Update comments to clarify annotation usage

This follows the standard kube-prometheus-stack pattern where:
- Labels are used for ConfigMap discovery (strict validation)
- Annotations are used for metadata like folder names (relaxed validation)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-13 13:08:45 +01:00
Chris CoutinhoandClaude 4ea5ed72d4 feat: Add Grafana dashboard and vector sync metric instrumentation
Implement comprehensive observability for vector database synchronization
with Grafana dashboard and Prometheus metrics.

## Part 1: Grafana Dashboard

Created all-in-one operations dashboard with 7 rows and 34 panels:

### Dashboard Structure:
- **Overview Row**: Request rate, error rate, P95 latency, active requests
- **HTTP Metrics (RED)**: Request/error rates by endpoint, latency percentiles
- **MCP Tools**: Call volume, error rates, execution duration by tool
- **Nextcloud API**: API calls/latency by app, retry patterns
- **OAuth & Authentication**: Token validations, exchanges, cache hit rate
- **Dependencies & Health**: Status for Nextcloud/Qdrant/Keycloak/Unstructured
- **Vector Sync**: Processing throughput, queue depth, Qdrant operations

### Helm Chart Integration:
- Added dashboard-configmap.yaml template for automatic provisioning
- Configured Grafana sidecar auto-discovery (label: grafana_dashboard="1")
- Added dashboards configuration section in values.yaml (opt-in)
- Updated Chart.yaml with dashboard annotations
- Enhanced NOTES.txt with dashboard deployment instructions
- Comprehensive documentation in dashboards/README.md

Dashboard supports dynamic filtering via variables:
- datasource: Prometheus data source selection
- namespace: Filter by Kubernetes namespace
- pod: Multi-select pod filtering
- interval: Query interval (1m/5m/10m/30m/1h)

## Part 2: Vector Sync Metric Instrumentation

Implemented metric recording throughout vector sync pipeline:

### metrics.py:
Added convenience functions:
- record_vector_sync_scan() - Track documents per scan
- record_vector_sync_processing() - Track processing duration/status
- record_qdrant_operation() - Track database operations
- update_vector_sync_queue_size() - Track queue depth

### scanner.py:
- Record number of documents found in each scan
- Enables monitoring of scan throughput

### processor.py:
- Record processing duration for each document
- Track success/failure status with timing
- Record Qdrant upsert/delete operations
- Handle all code paths (success, deletion, error)

### semantic.py:
- Wrap Qdrant query_points with try/except
- Record search operation success/failure

## Metrics Exposed:

- mcp_vector_sync_documents_scanned_total
- mcp_vector_sync_documents_processed_total{status}
- mcp_vector_sync_processing_duration_seconds (histogram)
- mcp_vector_sync_queue_size (gauge)
- mcp_qdrant_operations_total{operation,status}

This enables monitoring of:
- Scan and processing throughput
- Processing latency (P50/P95/P99)
- Error rates for processing and Qdrant operations
- Queue depth trends
- Complete observability of vector sync pipeline

## Testing:

Verified locally that metrics are recorded correctly:
- 36 documents scanned
- 3 documents processed (avg 7.5s each)
- 3 successful Qdrant upsert operations
- Search operations tracked

## Deployment:

Enable dashboard provisioning in Helm values:
```yaml
dashboards:
  enabled: true
  grafanaFolder: "Nextcloud MCP"
```

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-13 11:49:20 +01:00
Chris Coutinho 619ba5684d build: Add ./worktrees to .gitignore 2025-11-12 08:27:33 +01:00
Chris CoutinhoandClaude 6812e1aca7 fix: add dynamic dimension detection for Ollama embedding models
This fixes dimension mismatch errors when using embedding models with
non-standard dimensions (e.g., qwen3-embedding:4b produces 2560-dim
vectors instead of the hardcoded 768).

Changes:
- OllamaEmbeddingProvider: Detect dimensions dynamically by generating
  test embedding instead of hardcoding to 768
- qdrant_client: Call dimension detection before collection creation
- app.py: Initialize Qdrant collection before starting background tasks
  in streamable-http transport path
- tests: Fix integration tests to properly mock EmbeddingService wrapper

Fixes dimension mismatch error:
"could not broadcast input array from shape (2560,) into shape (768,)"

All integration tests passing (6/6).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-12 02:46:30 +01:00
Chris Coutinho 7e93097137 feat(ollama): Pull model on startup if not available in ollama 2025-11-12 00:37:26 +01:00
Chris Coutinho 0eae33a918 ci: Fix logging warning and cli mock 2025-11-11 23:42:00 +01:00
Chris Coutinho 3430b2409d build: Set default logging to text 2025-11-11 23:19:37 +01:00
Chris CoutinhoandClaude adde0e5623 fix: improve webapp tab UI with CSS Grid and viewport-filling container
Fixes layout issues on the webhooks admin tab:
- Add min-height to container to fill viewport consistently
- Use CSS Grid to overlay tab panes without jumpiness
- Add smooth htmx fade transitions for content swaps
- Adjust vector sync polling interval from 3s to 10s
- Add .playwright-mcp/ to gitignore for test screenshots

The CSS Grid approach allows tabs to overlay without absolute positioning,
preventing content cutoff while maintaining smooth transitions without
container resizing jumps.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-11 23:07:44 +01:00
Chris CoutinhoandClaude 12c96af819 feat: add dynamic vector sync status updates with htmx polling
Implement real-time vector sync status updates in the /app UI without
requiring page refreshes. The status (indexed documents, pending
documents, sync state) now updates automatically every 3 seconds.

Changes:
- Add vector_sync_status_fragment() endpoint that returns HTML fragment
  with current vector sync status
- Modify user_info_html() to use htmx loading for vector sync section
  with hx-trigger="load" on initial render
- Status fragment includes hx-trigger="every 3s" for continuous polling
- Add /app/vector-sync/status route to browser_routes

The implementation uses htmx (already loaded on page) to poll the status
endpoint, providing near real-time updates with minimal overhead. The
endpoint queries Qdrant for indexed count and reads from memory streams
for pending count, returning only the status HTML fragment.

Pattern follows existing webhook management UI which also uses htmx
for dynamic loading.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-11 21:04:31 +01:00
Chris CoutinhoandClaude d86a185e04 refactor: move webapp from /user/page to /app
Simplified the webapp routing structure by consolidating the admin UI
to a single clean endpoint.

Changes:
- Moved webapp from /user/page to /app (root of mount)
- Removed /user JSON endpoint (no longer needed)
- Updated mount point from /user to /app in app.py
- Updated all route path checks (3 locations)
- Updated OAuth redirects to point to /app
- Updated all HTMX endpoint references
- Updated documentation (ADR-007, CHANGELOG)
- Added redirect from /app to /app/ for trailing slash handling

New Route Structure:
- /app - Main webapp (HTML UI with tabs)
- /app/revoke - Revoke background access
- /app/webhooks - Webhook management UI
- /app/webhooks/enable/{preset_id} - Enable webhook preset
- /app/webhooks/disable/{preset_id} - Disable webhook preset

Breaking Change: Existing bookmarks to /user or /user/page will no longer work.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-11 20:53:43 +01:00
Chris CoutinhoandClaude f4759e424d feat: add webhook management UI and BeforeNodeDeletedEvent support
Added comprehensive webhook management capabilities including:

Webhook Client & API:
- Added WebhooksClient for Nextcloud webhooks API integration
- Create, list, update, and delete webhooks programmatically
- Support for event filters in webhook registration

Webhook Presets:
- Added preset system for common webhook configurations
- notes_sync: BeforeNodeDeletedEvent for Notes file operations
- calendar_sync: Calendar events (create, update, delete)
- deck_sync: Deck card operations
- files_sync: File system changes
- forms_sync: Form submissions (conditional)
- Filter presets by installed apps

Admin UI:
- Added multi-pane app view with tabs (User Info, Vector Sync, Webhooks)
- Webhooks tab for admin users only
- Enable/disable preset webhooks via UI
- View currently registered webhooks
- Uses htmx for dynamic loading and Alpine.js for tab state
- Admin permission checking via OCS API

CLI Improvements:
- Refactored CLI to separate module (cli.py)
- Updated entry point in pyproject.toml

BeforeNodeDeletedEvent Fix:
- Updated ADR-010 to document NodeDeletedEvent issue
- BeforeNodeDeletedEvent includes node.id before deletion
- NodeDeletedEvent lacks node.id (file already deleted)
- Implemented per Nextcloud maintainer recommendation

Testing:
- Added comprehensive webhook client tests
- Added webhook preset filtering tests
- Added admin permission tests

Configuration:
- Updated docker-compose.yml Qdrant settings

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-11 20:35:08 +01:00
Chris CoutinhoandClaude 1bced88c97 refactor: consolidate database storage for webhooks and OAuth tokens
Refactored the storage system to use a unified SQLite database for both
webhook tracking and OAuth token storage, available in both BasicAuth
and OAuth modes.

Changes:
- Renamed refresh_token_storage.py → storage.py
- Made TOKEN_ENCRYPTION_KEY optional (only required for OAuth token ops)
- Added registered_webhooks table with schema versioning
- Added webhook storage methods (store, get, delete, list, clear)
- Initialize storage in both BasicAuth and OAuth modes
- Updated webhook routes to persist registrations in database
- Database-first pattern for webhook status checks (performance)
- Updated all imports across codebase

Storage Behavior:
- Database created automatically at startup if needed
- Existing databases detected and reused
- Server fails fast if database initialization fails
- No migrations needed (OAuth feature is experimental)

Testing:
- Added 13 comprehensive unit tests for webhook storage
- All 118 unit tests pass
- All 5 smoke tests pass
- Verified fail-fast behavior on initialization errors

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-11 20:01:49 +01:00
Chris Coutinho b58e7238ae feat: validate Nextcloud webhook schemas and document findings
Manual testing of Nextcloud webhook_listeners app to validate webhook
payloads against ADR-010 expected schemas and document implementation
requirements for webhook-based vector synchronization.

## Changes

- Add test webhook endpoint at /webhooks/nextcloud in app.py
  - Captures and logs webhook payloads for analysis
  - Returns 200 OK immediately for webhook delivery confirmation

- Create webhook-testing-findings.md with comprehensive test results
  - Captured payloads for 5/6 webhook event types
  - Critical findings: missing node.id in deletions, type mismatches
  - Implementation recommendations with code examples

- Update ADR-010 with Appendix A: Manual Webhook Testing Results
  - Document actual vs expected webhook behavior
  - Update event mapping table with tested webhook status
  - Add 6 specific implementation recommendations
  - Include testing implications for future development

## Testing Results

 NodeCreatedEvent - fires correctly, includes node.id (integer)
 NodeWrittenEvent - fires correctly, includes node.id (integer)
 NodeDeletedEvent - fires but missing node.id field (path only)
 CalendarObjectCreatedEvent - fires correctly with full iCal
 CalendarObjectUpdatedEvent - fires correctly with full iCal
 CalendarObjectDeletedEvent - does not fire (potential NC bug)

## Key Findings

1. NodeDeletedEvent missing node.id field - requires path-based fallback
2. node.id returns integer not string - needs casting for consistency
3. Multiple webhooks fire per operation - needs deduplication logic
4. Calendar deletion webhooks don't fire - reported as issue #53497
5. Calendar webhooks include full iCal content - enables rich parsing

## GitHub Issues

- Created issue #56371: NodeDeletedEvent missing node.id field
- Commented on issue #53497: CalendarObjectDeletedEvent not firing

Closes #283

---

_This commit was generated with the help of AI, and reviewed by a Human_
2025-11-11 12:13:20 +01:00
Chris CoutinhoandClaude a6e5f3d8ff refactor: simplify OpenTelemetry tracing configuration
Simplifies the OpenTelemetry tracing setup by removing the redundant
OTEL_ENABLED flag and using the presence of OTEL_EXPORTER_OTLP_ENDPOINT
to determine if tracing should be enabled. This follows the standard
OpenTelemetry environment variable conventions more closely.

Changes:
- Remove OTEL_ENABLED/tracing_enabled flag in favor of checking if
  OTEL_EXPORTER_OTLP_ENDPOINT is set
- Add OTEL_EXPORTER_VERIFY_SSL configuration option for OTLP endpoints
  with self-signed certificates (defaults to false for development)
- Move HTTPXClientInstrumentor initialization to module level to ensure
  httpx calls are traced across all Nextcloud API requests
- Add tracing spans to vector sync operations (scan_user_documents)
- Fix authorization header logging to only warn about missing headers
  in OAuth mode (BasicAuth mode doesn't use Authorization headers)
- Update observability documentation to reflect simplified configuration
- Refactor Dockerfile to use --no-editable flag for uv sync

Breaking changes:
- OTEL_ENABLED environment variable is removed
- Tracing is now automatically enabled when OTEL_EXPORTER_OTLP_ENDPOINT
  is set

Migration guide:
- Remove OTEL_ENABLED=true from environment configuration
- Tracing will be enabled automatically if OTEL_EXPORTER_OTLP_ENDPOINT
  is configured

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-10 22:48:37 +01:00
Chris Coutinho c787abf2f3 fix: add retry logic for ETag conflicts in category change test
The test_attachments_category_change_handling test was failing in CI with
HTTP 412 Precondition Failed errors. This is caused by the background vector
scanner (runs every 10 seconds) modifying notes between when the test fetches
the ETag and when it attempts to update the category.

Solution: Added retry logic (up to 3 attempts) that refetches the latest ETag
and retries the update operation when encountering 412 errors. This handles
the race condition gracefully while still catching genuine errors.
2025-11-10 07:41:02 +01:00
Chris CoutinhoandClaude b32324cb76 feat: skip tracing for health and metrics endpoints
Health check and metrics endpoints are frequently polled and don't
provide meaningful trace data. This change skips OpenTelemetry span
creation for:
- /health/* (liveness, readiness checks)
- /metrics (Prometheus metrics)

These endpoints still record Prometheus metrics (request count, latency,
in-flight requests) but no longer create trace spans, reducing tracing
noise and storage costs.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-10 07:24:27 +01:00
Chris CoutinhoandClaude 640a7818f9 fix: optimize Notes API pagination with pruneBefore parameter
The Nextcloud Notes API intentionally returns all note IDs (with only 'id'
field) in the last chunk to enable deletion detection. Without using the
pruneBefore parameter, this causes duplicates - all notes appear with full
data in chunks, then again with minimal data in the last chunk.

This commit implements proper pruneBefore support:
- NotesClient.get_all_notes() now accepts prune_before timestamp parameter
- Scanner calculates max(indexed_at) from Qdrant to use as prune threshold
- Only notes modified after this timestamp are sent with full data
- Deduplication logic handles the API's deletion detection pattern
- Significantly reduces data transfer for incremental syncs

The behavior is documented in Notes API v1 spec - this is not an API bug,
but a feature we weren't utilizing correctly.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-10 07:19:26 +01:00
Chris CoutinhoandClaude 3a41860d27 docs: Add ADR-010 for webhook-based vector sync
Add architecture decision record for integrating Nextcloud webhooks
into the vector database synchronization system.

Key features:
- Webhook endpoint at /webhooks/nextcloud receives push notifications
- Complements existing polling (ADR-007) without replacing it
- Optional authentication via WEBHOOK_SECRET
- Simple architecture: webhooks are just another DocumentTask producer
- Administrators can reduce polling frequency when webhooks are configured

Benefits:
- Reduced latency: seconds to minutes instead of up to 1 hour
- Lower API load: ~95% reduction when polling frequency is increased
- Better scalability: only process changed documents
- No changes required to scanner or processor components

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-10 05:28:36 +01:00
Chris CoutinhoandClaude d80e54ff97 feat(helm): Add document chunking configuration
Add support for configurable document chunking parameters to Helm chart
to match docker-compose and application capabilities.

Changes:
1. values.yaml:
   - Add documentChunking section with chunkSize (512) and chunkOverlap (50)
   - Include comprehensive comments explaining chunking strategies
   - Positioned between vectorSync and qdrant sections

2. templates/deployment.yaml:
   - Add DOCUMENT_CHUNK_SIZE and DOCUMENT_CHUNK_OVERLAP env vars
   - Always set (not conditional), used by vector sync processor
   - Environment variables follow same pattern as config.py defaults

3. README.md:
   - Add documentChunking parameter table in Vector Search section
   - Document chunking strategies (small/medium/large chunks)
   - Explain overlap recommendations (10-20% of chunk size)

Validation:
- helm lint: Passes
- helm template: Environment variables correctly generated
- Custom values: Work as expected (tested with chunkSize=1024)
- Always present: Not conditional on vectorSync.enabled

This maintains feature parity between Helm and docker-compose deployments,
allowing users to tune chunking for their embedding models and use cases.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-10 03:34:16 +01:00
Chris CoutinhoandClaude 157e433d65 fix: Support in-memory Qdrant for CI testing
Changes to make tests work without external qdrant/ollama dependencies:

1. docker-compose.yml (mcp service):
   - Switch from QDRANT_URL (network mode) to QDRANT_LOCATION=":memory:"
   - Comment out QDRANT_URL and QDRANT_API_KEY (not needed for in-memory)
   - Keep OLLAMA_BASE_URL commented out (use SimpleEmbeddingProvider fallback)

2. nextcloud_mcp_server/vector/qdrant_client.py:
   - Fix collection creation bug in in-memory mode
   - Previously: All ValueError exceptions were re-raised
   - Now: Only dimension mismatch ValueError is re-raised
   - Allows "Collection not found" ValueError to trigger auto-creation

3. tests/integration/test_sampling.py:
   - Update test to handle all sampling unsupported cases
   - Check for multiple fallback search_method values
   - Skip test gracefully when sampling unavailable

This configuration enables:
- CI testing without external services (qdrant, ollama)
- In-memory vector database (ephemeral but sufficient for tests)
- SimpleEmbeddingProvider for embeddings (feature hashing, 384 dims)
- Automatic collection creation on first use

Test result: test_semantic_search_answer_successful_sampling now passes
(skipped with appropriate message when sampling unsupported)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-10 03:21:27 +01:00
Chris Coutinho 94d16092c0 ci: Add qdrant profile to docker compose up command 2025-11-10 03:09:50 +01:00
Chris CoutinhoandClaude cb39b3fca4 feat(vector): Add configurable chunk size and overlap for document embedding
Enable users to tune document chunking parameters to match their embedding
model and content type by adding DOCUMENT_CHUNK_SIZE and DOCUMENT_CHUNK_OVERLAP
environment variables.

- **config.py**: Added `document_chunk_size` (default: 512) and
  `document_chunk_overlap` (default: 50) configuration fields with validation:
  - Ensures overlap < chunk_size
  - Warns if chunk_size < 100 words
  - Prevents negative overlap values

- **processor.py**: Updated DocumentChunker instantiation to use config
  settings instead of hardcoded values (line 174-177)

- **tests/unit/test_config.py**: Added TestChunkConfigValidation class with
  9 tests covering:
  - Default values
  - Valid configurations
  - Validation errors (overlap >= chunk_size, negative overlap)
  - Warning for small chunk sizes
  - Environment variable loading

- **docs/configuration.md**: Added comprehensive "Document Chunking
  Configuration" section with:
  - Chunk size selection guidance (256-384 vs 512 vs 768-1024 words)
  - Overlap recommendations (10-20% of chunk size)
  - Configuration examples for different use cases
  - Added env vars to reference table

- **docs/semantic-search-architecture.md**: Added "Document Chunking Strategy"
  section with:
  - Chunking process explanation
  - Example showing sliding window behavior
  - Search behavior with chunks
  - Tuning recommendations

- **env.sample**: Added complete "Semantic Search & Vector Sync Configuration"
  section with:
  - Vector sync settings
  - Qdrant configuration (3 modes)
  - Ollama embedding service
  - Document chunking configuration

- **docker-compose.yml**: Added commented examples for DOCUMENT_CHUNK_SIZE and
  DOCUMENT_CHUNK_OVERLAP with usage notes

\`\`\`bash
DOCUMENT_CHUNK_SIZE=512

DOCUMENT_CHUNK_OVERLAP=50
\`\`\`

1. \`overlap\` must be less than \`chunk_size\`
2. \`overlap\` cannot be negative
3. Warning issued if \`chunk_size\` < 100 words

**Precise matching** (small notes, specific queries):
\`\`\`bash
DOCUMENT_CHUNK_SIZE=256
DOCUMENT_CHUNK_OVERLAP=25
\`\`\`

**Balanced** (default, general purpose):
\`\`\`bash
DOCUMENT_CHUNK_SIZE=512
DOCUMENT_CHUNK_OVERLAP=50
\`\`\`

**Contextual** (long documents, broader topics):
\`\`\`bash
DOCUMENT_CHUNK_SIZE=1024
DOCUMENT_CHUNK_OVERLAP=100
\`\`\`

 **User control** - Tune chunking to match embedding model capabilities
 **Experimentation** - Test different chunk sizes for optimal results
 **Model alignment** - Match chunk size to embedding context window
 **Backward compatible** - Defaults maintain existing behavior
 **Well validated** - Comprehensive tests prevent misconfiguration

All 22 config validation tests pass (9 new tests for chunking):
- Default values work correctly
- Validation prevents invalid configurations
- Environment variables load properly
- Warning system works as expected

With configurable chunk sizes, users can now experiment with different Ollama
embedding models and tune chunk parameters for optimal semantic search quality.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-10 02:47:57 +01:00
Chris Coutinho f3050e9b45 chore: Remove /health and /metrics endpoints from logging 2025-11-10 02:07:45 +01:00
Chris CoutinhoandClaude e575c8e57b feat(vector): Support multiple embedding models with auto-generated collection names
This PR enables safe switching between embedding models and multi-server
deployments by implementing auto-generated Qdrant collection names based on
deployment ID and model name.

## Problem

Previously, all deployments used a single hardcoded collection name
"nextcloud_content", which caused two critical issues:

1. **Dimension mismatches when switching models**: Changing
   OLLAMA_EMBEDDING_MODEL (e.g., nomic-embed-text at 768D → all-minilm at
   384D) would cause runtime errors as vectors couldn't be inserted into a
   collection with incompatible dimensions.

2. **Collection collisions in multi-server setups**: Multiple MCP servers
   sharing a single Qdrant instance would overwrite each other's data,
   making horizontal scaling impossible.

## Solution

### Auto-Generated Collection Naming

Collections are now automatically named using the pattern:
\`{deployment-id}-{model-name}\`

**Deployment ID**: Uses \`OTEL_SERVICE_NAME\` if configured (and not default
value), otherwise falls back to \`hostname\` for simple Docker deployments.

**Model Name**: From \`OLLAMA_EMBEDDING_MODEL\` with path separators sanitized.

**Examples**:
- \`my-mcp-server-nomic-embed-text\` (with OTEL_SERVICE_NAME=my-mcp-server)
- \`mcp-container-all-minilm\` (simple Docker, hostname=mcp-container)

**Override**: Users can still set \`QDRANT_COLLECTION\` explicitly to bypass
auto-generation for backward compatibility.

### Dimension Validation

Added startup validation that checks collection dimensions match the
embedding service. If a mismatch is detected, the server fails fast with a
clear error message explaining:
- Expected vs actual dimensions
- Likely cause (model change)
- Solutions (delete collection, use different name, or revert model)

### Improved Sampling Error Handling

Enhanced MCP sampling rejection handling to treat user rejections as normal
behavior rather than errors:

- **User rejections** ("rejected", "denied") → INFO log, no traceback
- **Unsupported clients** → INFO log, no traceback
- **Other MCP errors** → WARNING log, no traceback
- **Unexpected errors** → ERROR log WITH traceback

This aligns with the MCP specification where clients SHOULD prompt users for
approval/denial of sampling requests.

## Changes

### Core Implementation

- **nextcloud_mcp_server/config.py**: Added \`get_collection_name()\` method
  with deployment ID detection and model name sanitization
- **nextcloud_mcp_server/vector/qdrant_client.py**: Dimension validation on
  collection open with helpful error messages
- **nextcloud_mcp_server/vector/{scanner,processor}.py**: Updated to use
  \`get_collection_name()\`
- **nextcloud_mcp_server/auth/userinfo_routes.py**: Vector sync status uses
  \`get_collection_name()\`
- **nextcloud_mcp_server/server/semantic.py**:
  - Updated semantic search tools to use \`get_collection_name()\`
  - Improved sampling rejection error handling (McpError vs Exception)

### Documentation

- **docs/semantic-search-architecture.md**: New comprehensive architecture
  document (557 lines) covering background sync, semantic search flow, RAG
  implementation, and deployment modes
- **docs/configuration.md**: Added detailed "Qdrant Collection Naming"
  section with examples and multi-server deployment guidance
- **docker-compose.yml**: Added comments explaining collection naming behavior
- **README.md**: Updated semantic search descriptions to clarify
  experimental status, Notes-only support, and infrastructure requirements

## Migration Guide

**For existing single-server deployments:**

Option 1 (Recommended): Use explicit collection name for continuity
\`\`\`bash
QDRANT_COLLECTION=nextcloud_content  # Keep existing collection
\`\`\`

Option 2: Allow auto-generation and re-embed
\`\`\`bash
# Remove QDRANT_COLLECTION override
# New collection will be created based on deployment ID + model
# Requires re-embedding all documents (may take time)
\`\`\`

**For new multi-server deployments:**

Set unique OTEL service names per server:
\`\`\`bash
# Server 1
OTEL_SERVICE_NAME=mcp-prod
OLLAMA_EMBEDDING_MODEL=nomic-embed-text
# → Collection: "mcp-prod-nomic-embed-text"

# Server 2
OTEL_SERVICE_NAME=mcp-staging
OLLAMA_EMBEDDING_MODEL=nomic-embed-text
# → Collection: "mcp-staging-nomic-embed-text"
\`\`\`

## Benefits

 **Safe model switching**: Each model gets its own collection, preventing
   dimension mismatch errors
 **Multi-server support**: Multiple MCP servers can share one Qdrant
   instance without conflicts
 **Clear ownership**: Collection names show which deployment and model owns
   the data
 **Better error messages**: Dimension validation provides actionable
   guidance
 **Backward compatible**: Existing deployments can continue using
   \`QDRANT_COLLECTION\` override

## Testing

Validated with:
- Single-server deployments (default hostname-based naming)
- Multi-server deployments (OTEL service name-based naming)
- Model switching scenarios (dimension validation)
- Collection override scenarios (backward compatibility)

Next steps: Testing various Ollama embedding models to investigate optimal
chunk sizes and performance characteristics.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-10 01:18:30 +01:00
Chris Coutinho 4a6c60113b fix(helm): Set default strategy to Recreate 2025-11-09 19:27:55 +01:00
Chris CoutinhoandClaude 4e89e92b65 fix(observability): isolate metrics endpoint to dedicated port
Security fix: Move Prometheus metrics endpoint from main HTTP port to
dedicated port 9090 to prevent external exposure of metrics data.

Changes:
- Use prometheus_client.start_http_server() for dedicated metrics server
- Remove /metrics route from main application routes
- Metrics now only accessible on port 9090 (configurable via METRICS_PORT)
- Main application port no longer serves /metrics endpoint

This follows security best practice of isolating monitoring endpoints
from application traffic.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-09 09:53:36 +01:00
Chris CoutinhoandClaude 5e4667a643 fix(readiness): Only check external Qdrant in network mode
The readiness probe incorrectly tried to connect to an external Qdrant service
even when using memory or persistent mode (embedded Qdrant). This caused pods
to never become ready in Kubernetes deployments using the default configuration.

Root cause:
- In memory/persistent modes, QDRANT_URL env var is NOT set
- Readiness check used default 'http://qdrant:6333' anyway
- Tried to connect to non-existent service
- Connection failed -> 503 -> pod stuck in not-ready state

Fix:
- Only check external Qdrant health if QDRANT_URL is explicitly set (network mode)
- For embedded modes (memory/persistent), report status as 'embedded' without blocking
- Background scanner tasks don't block readiness (already non-blocking via anyio.start_soon)

This allows pods to become ready immediately when using embedded Qdrant,
while still validating external Qdrant connectivity in network mode.

Fixes: Kubernetes pods failing readiness check with default Qdrant configuration

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-09 09:28:09 +01:00
Chris CoutinhoandClaude 093ac5b5ba feat(helm): Add observability support with ServiceMonitor and Grafana dashboard
Add comprehensive observability configuration to Helm chart:

**Helm Values:**
- Add observability configuration section for metrics, tracing, and logging
- Add serviceMonitor configuration (disabled by default)
- Add prometheusRule configuration (disabled by default)

**Templates:**
- Update deployment to include observability environment variables
- Update deployment to expose metrics port (9090)
- Update service to expose metrics port
- Add ServiceMonitor template for Prometheus Operator
- Add PrometheusRule template with critical and warning alerts

**Dashboards:**
- Add comprehensive Grafana dashboard JSON with 6 panels:
  - Request Rate (by method and endpoint)
  - Error Rate (5xx errors percentage)
  - Request Latency (P50/P95 by endpoint)
  - Top MCP Tools (by invocation volume)
  - Nextcloud API Latency (by app)
  - Vector Sync Queue Size
- Add dashboard README with import instructions

**Alert Rules:**
- Critical: Server down, high error rate (>5%), high latency (>1s), dependency down
- Warning: Token validation errors (>1%), vector sync queue high (>100), Qdrant slow (>500ms)

All features are opt-in via values.yaml configuration.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-09 09:10:11 +01:00
Chris CoutinhoandClaude 7be40a33e1 fix(vector): Handle missing 'modified' field in notes gracefully
The vector scanner crashed when encountering notes without a 'modified' field,
causing KeyError and preventing initial sync from completing.

Changes:
- Use dict.get() with fallback value (0) instead of direct key access
- Log warnings for notes missing 'modified' field
- Apply fix to both initial sync and incremental sync code paths

This ensures the scanner continues processing all notes even if some have
missing metadata fields, preventing scanner crashes that could affect
deployment readiness.

Fixes: Notes without 'modified' field causing scanner crash and readiness check failure

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-09 09:03:05 +01:00
Chris CoutinhoandClaude 578de4d7d6 feat(observability): Add comprehensive monitoring with Prometheus and OpenTelemetry
- Add Prometheus metrics for HTTP, MCP tools, Nextcloud API, OAuth, vector sync, and DB operations
- Add OpenTelemetry distributed tracing with OTLP export
- Add structured JSON logging with trace context correlation
- Add ObservabilityMiddleware for automatic HTTP instrumentation
- Add app_name attribute to all client classes for per-app metrics
- Add configuration for metrics, tracing, and logging via environment variables
- Add documentation in docs/observability.md
- Fix graceful degradation when tracing is disabled (default state)
- Fix uvicorn logging configuration to use observability formatters

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-09 08:54:04 +01:00
Chris Coutinho f8a2935c22 fix(ci): Use helm dependency build instead of update to use Chart.lock 2025-11-09 07:52:00 +01:00
Chris CoutinhoandClaude 725ac65e6a fix(helm): update Qdrant dependency condition to match new mode structure
The Qdrant subchart was being included by default even in memory/persistent
modes. Changed the dependency condition from `qdrant.enabled` to
`qdrant.networkMode.deploySubchart` to align with the three-mode structure.

Now the Qdrant subchart is ONLY deployed when:
- qdrant.mode: "network"
- qdrant.networkMode.deploySubchart: true

Verified all three modes:
- Memory mode (:memory:): No subchart, QDRANT_LOCATION=:memory:
- Persistent mode (path): No subchart, QDRANT_LOCATION=/app/data/qdrant, PVC created
- Network mode (subchart): Qdrant subchart deployed, QDRANT_URL=http://...:6333
- Network mode (external): No subchart, QDRANT_URL=<external-url>

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-09 07:45:06 +01:00
Chris CoutinhoandClaude 50ba6ccc88 fix(ci): add Helm repository setup to chart release workflow
The chart-releaser was failing because it couldn't resolve the
dependencies (Qdrant and Ollama subcharts) when packaging.

Changes:
- Add azure/setup-helm action to install Helm v3.16.0
- Add step to add Qdrant and Ollama Helm repositories
- Run helm dependency update before chart-releaser runs

This fixes the error:
"Error: no repository definition for https://qdrant.github.io/qdrant-helm, https://otwld.github.io/ollama-helm"

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-09 07:21:17 +01:00
Chris CoutinhoandClaude 167e49788e feat(helm): add Qdrant local mode support with three deployment options [skip ci]
Add support for three Qdrant deployment modes in Helm chart:
1. In-memory mode (:memory:) - Default, zero-config, ephemeral storage
2. Persistent local mode (path-based) - File-based storage with PVC
3. Network mode (URL-based) - Dedicated Qdrant service or external instance

Changes:
- Restructured qdrant configuration in values.yaml with mode selector
- Added conditional environment variable logic in deployment.yaml
- Created PVC template for persistent local mode with optional existingClaim
- Added qdrantPvcName helper template in _helpers.tpl
- Updated README.md with Helm registry URL (https://cbcoutinho.github.io/nextcloud-mcp-server)

Breaking change: Default changed from requiring qdrant.enabled to using
in-memory mode (:memory:) when no Qdrant configuration is provided.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-09 07:14:19 +01:00
Chris CoutinhoandClaude 857d8f2152 feat: add Qdrant local mode support with in-memory and persistent storage
Adds flexible Qdrant deployment modes to reduce infrastructure requirements
for local development and smaller deployments:

**Configuration Changes:**
- Add QDRANT_LOCATION environment variable (mutually exclusive with QDRANT_URL)
- Three modes: network (URL), in-memory (:memory:, default), persistent (file path)
- Settings dataclass validation via __post_init__ ensures mutual exclusivity
- API key warning when set in local mode (ignored, only for network mode)

**Client Initialization:**
- Auto-detect mode: network (url + api_key) vs local (:memory: or path=)
- In-memory: AsyncQdrantClient(":memory:") - zero config default
- Persistent: AsyncQdrantClient(path="/app/data/qdrant") - file storage
- Network: AsyncQdrantClient(url, api_key) - production mode

**Docker Compose Updates:**
- Qdrant service moved to optional profile (--profile qdrant)
- MCP service uses QDRANT_LOCATION=:memory: by default
- Added mcp-data volume for persistent storage (/app/data)
- No hard dependency on qdrant service

**Documentation:**
- Comprehensive configuration guide in docs/configuration.md
- All three modes documented with pros/cons
- Docker Compose examples for each mode
- Environment variable reference table

**Tests:**
- 13 new config validation tests (mutual exclusivity, defaults, warnings)
- Persistent mode integration test (create, close, reopen, verify persistence)
- All 82 unit tests + 5 smoke tests pass

**Breaking Change:**
- Default changed from QDRANT_URL=http://qdrant:6333 to QDRANT_LOCATION=:memory:
- Simplifies local development (no external service needed)
- Production deployments: explicitly set QDRANT_URL or QDRANT_LOCATION

Related: ADR-007 background vector sync implementation

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-09 07:07:07 +01:00
Chris CoutinhoandClaude 72232f937a refactor: migrate vector sync from asyncio.Queue to anyio memory object streams
Replace asyncio.Queue with anyio.create_memory_object_stream() throughout
the vector sync system for better library consistency and improved shutdown
semantics.

## Changes Made

**scanner.py**:
- Changed parameter type from `asyncio.Queue` to `MemoryObjectSendStream[DocumentTask]`
- Replaced all `await document_queue.put()` calls with `await send_stream.send()`
- Wrapped scanner loop in `async with send_stream:` context manager for automatic cleanup
- Updated log messages: "Queued" → "Sent"
- Removed `import asyncio` (no longer needed)

**processor.py**:
- Changed parameter type from `asyncio.Queue` to `MemoryObjectReceiveStream[DocumentTask]`
- Replaced `asyncio.wait_for(document_queue.get(), timeout=1.0)` with `anyio.fail_after(1.0)` + `await receive_stream.receive()`
- Removed all `document_queue.task_done()` calls (not needed with streams)
- Added `anyio.EndOfStream` exception handling for graceful shutdown when scanner closes
- Removed `import asyncio` (no longer needed)

**app.py**:
- Removed `import asyncio` from top-level imports
- Added `from anyio.streams.memory import MemoryObjectReceiveStream, MemoryObjectSendStream`
- Updated AppContext dataclass:
  - Replaced `document_queue: Optional[asyncio.Queue]` with:
    - `document_send_stream: Optional[MemoryObjectSendStream]`
    - `document_receive_stream: Optional[MemoryObjectReceiveStream]`
- Updated `app_lifespan_basic()`:
  - Replaced `asyncio.Queue(maxsize=...)` with `anyio.create_memory_object_stream(max_buffer_size=...)`
  - Pass `send_stream` to scanner_task
  - Pass `receive_stream.clone()` to each processor_task (enables multiple consumers)
  - Updated AppContext yield to include both streams
- Updated `starlette_lifespan()`:
  - Same changes as app_lifespan_basic for streamable-http transport
  - Removed `import asyncio as asyncio_module` (no longer needed)
  - Updated app.state storage to use send_stream and receive_stream

**semantic.py**:
- Updated `nc_get_vector_sync_status()` tool:
  - Access `document_receive_stream` instead of `document_queue` from lifespan context
  - Use `stream_stats.current_buffer_used` instead of `queue.qsize()` for pending count
  - More reliable metrics (qsize() was not guaranteed accurate)

## Benefits

1. **Library Consistency**: Pure anyio throughout codebase (was mixing asyncio.Queue with anyio.Event and anyio.create_task_group)
2. **Graceful Shutdown**: `async with send_stream:` automatically closes stream on exit, signaling EndOfStream to all processors
3. **Better Timeout Handling**: `anyio.fail_after()` is more idiomatic than `asyncio.wait_for()`
4. **Stream Cloning**: Easy to add multiple consumers via `receive_stream.clone()`
5. **Better Statistics**: `.statistics()` provides accurate buffer metrics (qsize() was unreliable)
6. **Type Safety**: Separate send/receive types prevent accidental misuse
7. **No task_done() tracking**: Streams handle completion automatically

## Testing

-  All 69 unit tests passing
-  All 5 smoke tests passing
-  No regressions in functionality
-  Graceful shutdown behavior improved

## References

- https://anyio.readthedocs.io/en/stable/why.html#queue-fix
- https://anyio.readthedocs.io/en/stable/streams.html#memory-object-streams

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-09 06:43:44 +01:00
Chris CoutinhoandClaude 4b026e9aa0 feat: implement ADR-009 - refactor semantic search to use generic semantic:read scope
This implements ADR-009, which documents the decision to use a generic
`semantic:read` OAuth scope instead of requiring all app-specific scopes
for semantic search functionality.

Changes:
- Created new `nextcloud_mcp_server/models/semantic.py` with semantic search models
  - SemanticSearchResult (with new doc_type field for multi-app support)
  - SemanticSearchResponse
  - SamplingSearchResponse
  - VectorSyncStatusResponse

- Created new `nextcloud_mcp_server/server/semantic.py` with semantic search tools
  - nc_semantic_search (renamed from nc_notes_semantic_search)
  - nc_semantic_search_answer (renamed from nc_notes_semantic_search_answer)
  - nc_get_vector_sync_status (renamed from nc_notes_get_vector_sync_status)
  - All tools now use @require_scopes("semantic:read") instead of "notes:read"

- Updated `nextcloud_mcp_server/server/notes.py`
  - Removed semantic search tools (moved to semantic.py)
  - Removed semantic search model imports
  - Removed unused MCP imports (ModelHint, ModelPreferences, etc.)

- Updated `nextcloud_mcp_server/models/notes.py`
  - Removed semantic search models (moved to semantic.py)

- Updated `nextcloud_mcp_server/app.py`
  - Import configure_semantic_tools
  - Register semantic tools when VECTOR_SYNC_ENABLED=true

- Updated `nextcloud_mcp_server/server/__init__.py`
  - Export configure_semantic_tools

- Updated tests
  - tests/integration/test_sampling.py: Use new tool names
  - tests/unit/test_response_models.py: Import from semantic.py, add doc_type field

Architecture:
- Semantic search is now a cross-app feature, not tied to Notes
- Uses dual-phase authorization: semantic:read scope + per-document verification
- Supports future multi-app indexing (notes, calendar, deck, files, contacts)

Test results:
- All 69 unit tests passing
- All 5 smoke tests passing

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-09 05:53:53 +01:00
Chris CoutinhoandClaude 31799ffd9a docs: remove VECTOR_SYNC_ENABLED_APPS env var, use per-user database settings
Replace static VECTOR_SYNC_ENABLED_APPS environment variable with per-user
database storage for which apps to index. This allows each user to control
their own indexing preferences (e.g., enable notes and calendar but not
deck or files).

Rationale:
- Nextcloud doesn't support granular OAuth scopes at the app level
- Per-user settings provide flexibility for multi-user deployments
- Users control app enablement via nc_enable_vector_sync MCP tool
- Aligns with OAuth architecture where users manage their own settings

Changes:
- ADR-007: Remove VECTOR_SYNC_ENABLED_APPS from configuration section
- ADR-007: Update scanner implementation to read from database
- ADR-007: Add explanation of per-user app enablement mechanism
- ADR-007: Clarify that nc_enable_vector_sync tool manages this setting

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-09 05:11:56 +01:00
Chris CoutinhoandClaude 5cc598e1b1 docs: refactor semantic search from notes-specific to multi-app architecture
Update ADRs to reflect that vector database and semantic search support
multiple Nextcloud apps (notes, calendar, deck, files, contacts) rather
than being notes-specific. Introduce semantic:read/write OAuth scopes
to replace app-specific scope requirements for cross-app search.

Changes:
- ADR-007: Add plugin architecture (DocumentScanner, DocumentProcessor,
  DocumentVerifier) for multi-app vector sync
- ADR-008: Rename tools from nc_notes_semantic_* to nc_semantic_*, update
  scope from notes:read to semantic:read
- ADR-009: NEW - Document decision to use generic semantic:read scope
  with dual-phase authorization instead of requiring all app scopes
- oauth-architecture.md: Add semantic:read/write scope documentation
- README.md: Move semantic search to dedicated section separate from Notes

This is a breaking change that correctly positions semantic search as a
cross-app capability before broader adoption. Existing deployments will
need to re-authenticate with the new semantic:read scope.

Relates to user request to decouple vector database from notes-only model
and establish proper OAuth scope boundaries for multi-app semantic search.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-09 04:47:20 +01:00
Chris Coutinho a6c76c5cc1 chore: Add openid scope to nc_notes_get_vector_sync_status 2025-11-09 03:27:17 +01:00
Chris CoutinhoandClaude a854656d3c fix: implement deletion grace period and vector sync status tool
This commit addresses issues with vector database synchronization that
were causing test failures:

1. **Deletion Grace Period** (scanner.py)
   - Fixed premature deletion of documents due to pagination cursor
     inconsistencies in Notes API
   - Implemented 2-scan verification with 1.5x scan interval grace period
     (15 seconds default)
   - Documents must be missing for 2 consecutive scans before deletion
   - Documents that reappear are removed from deletion tracking
   - Prevents false deletions during concurrent note creation/indexing

2. **Vector Sync Status Tool** (server/notes.py, models/notes.py)
   - Added nc_notes_get_vector_sync_status MCP tool
   - Returns indexed_count, pending_count, status, and enabled fields
   - Enables tests and clients to wait for vector sync completion
   - Uses lifespan context to access document queue and Qdrant client

3. **Test Improvements** (test_sampling.py, conftest.py)
   - Added temporary_note_factory fixture for creating multiple test notes
   - Updated all sampling tests to wait for vector sync completion
   - Adjusted score_threshold to 0.0 for SimpleEmbeddingProvider
     (feature hashing produces low-quality embeddings)
   - Fixed CallToolResult extraction (removed ["result"] key access)
   - Removed invalid @pytest.mark.asyncio markers (anyio mode)

All integration tests now pass successfully.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-09 03:11:39 +01:00
Chris CoutinhoandClaude bb5d4f464f feat: implement MCP sampling for semantic search RAG (ADR-008)
Add nc_notes_semantic_search_answer tool that combines semantic search
with MCP sampling to generate natural language answers from retrieved
Nextcloud Notes. This enables Retrieval-Augmented Generation (RAG)
patterns without requiring a server-side LLM.

Key features:
- Client-side LLM generation via ctx.session.create_message()
- Graceful fallback when sampling unavailable
- Proper source citations in generated answers
- No results optimization (skips sampling when no docs found)
- Comprehensive unit and integration tests

Implementation details:
- SamplingSearchResponse model with generated_answer and sources
- Fixed prompt template with document context and citation instructions
- Model preferences hint Claude Sonnet for balanced performance
- Falls back to returning documents without answer on sampling failure

Updates:
- Add ADR-008 documenting sampling architecture decision
- Add MCP sampling pattern guidance to CLAUDE.md
- Update README.md and docs/notes.md (7 → 9 tools)
- Add 4 unit tests and 6 integration tests

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-09 01:00:18 +01:00
Chris CoutinhoandClaude e32c8f4aec feat: add optional vector database and semantic search to helm chart
Add support for deploying Qdrant vector database and Ollama embedding
service as optional helm chart dependencies. Enables semantic search
capabilities for Nextcloud content with flexible deployment options.

Chart Dependencies:
- Add Qdrant v0.9.0 from qdrant/qdrant-helm (conditional)
- Add Ollama v1.33.0 from otwld/ollama-helm (conditional)
- Both dependencies only deploy when enabled

Configuration (values.yaml):
- vectorSync: Background sync settings (interval, workers, queue size)
- qdrant: Subchart config with persistence, resources, clustering
- ollama: Subchart config with model pull, persistence, resources
  - Support for external Ollama via ollama.url (no subchart deployment)
- openai: Alternative embedding provider (OpenAI or compatible API)

Environment Variables (deployment.yaml):
- VECTOR_SYNC_* variables when vectorSync.enabled
- QDRANT_URL, QDRANT_COLLECTION when qdrant.enabled
- OLLAMA_BASE_URL, OLLAMA_EMBEDDING_MODEL when ollama enabled or URL set
- OPENAI_API_KEY when openai.enabled

Documentation:
- README: New "Vector Search & Semantic Capabilities" section
- README: Example 5 showing three deployment patterns
- NOTES.txt: Conditional guidance when vector features enabled
- Secret template for OpenAI API key management

All features disabled by default for backward compatibility.
Tested with helm template and helm lint.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-09 00:03:51 +01:00
Chris CoutinhoandClaude ee183e1c1c feat: add vector sync processing status to /user/page endpoint
Add real-time processing status display to the browser UI at /user/page
showing indexed document count, pending queue size, and sync status.
Implements the status display described in ADR-007 lines 280-298.

Changes:
- Store document_queue and related state in app.state for route access
- Add _get_processing_status() helper to query Qdrant and check queue
- Display status section in user_info_html() with indexed/pending counts
- Show color-coded status badge (green "Idle" or orange "Syncing")
- Only displays when VECTOR_SYNC_ENABLED=true

Status appears in both BasicAuth and OAuth modes, positioned after
session info but before logout buttons. Numbers are formatted with
commas for readability.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-08 23:59:18 +01:00
Chris Coutinho 1a57f97d3a refactor: update to Qdrant query_points API and fix Playwright Keycloak login
- Replace deprecated qdrant_client.search() with query_points() API
- Update semantic search implementation in notes.py
- Update all integration tests to use query_points()
- Fix Keycloak login in test_keycloak_dcr.py to use form.submit() instead of button click
- Remove unnecessary popup handler code
- Simplify consent screen logging
2025-11-08 22:41:14 +01:00
Chris CoutinhoandClaude e96c02e4d4 fix: remove unnecessary urllib3<2.0 constraint
The urllib3<2.0 constraint was added unnecessarily during troubleshooting.
urllib3 2.x works perfectly fine with qdrant-client. The import path for
urllib3.util.Url and parse_url remains the same across 1.x and 2.x versions.

Changes:
- Remove urllib3<2.0 constraint from pyproject.toml
- Upgrade to urllib3 2.5.0 (latest)
- All integration tests pass with urllib3 2.x

Verified:
- from urllib3.util import Url, parse_url works in 2.5.0
- All 6 semantic search integration tests pass
- qdrant-client 1.15.1 works correctly with urllib3 2.5.0

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-08 22:18:31 +01:00
Chris CoutinhoandClaude 7b8c3f93a8 test: add integration tests for semantic search with in-process embeddings
Adds comprehensive integration tests for vector database semantic search that
work without external dependencies (Ollama), making them suitable for CI/CD.

Changes:
- Add SimpleEmbeddingProvider: in-process TF-IDF-like embeddings using feature hashing
- Make Ollama optional: embedding service now falls back to SimpleEmbeddingProvider
- Add 6 integration tests covering semantic search, filtering, and batch operations
- Downgrade urllib3 to 1.26.x for qdrant-client compatibility
- Update docker-compose.yml to comment out Ollama configuration (optional)

The SimpleEmbeddingProvider generates deterministic, normalized embeddings
suitable for testing semantic similarity without requiring external services.
Tests validate that similar texts have higher cosine similarity and that
semantic search correctly ranks results by relevance.

Test coverage:
- Deterministic embedding generation
- Semantic similarity between texts
- Full search flow with Qdrant (in-memory)
- Category filtering
- Empty result handling
- Batch embedding generation

All tests pass and can run in GitHub CI without Ollama infrastructure.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-08 22:13:33 +01:00
Chris CoutinhoandClaude fdd82f59e2 feat: implement semantic search tool and fix vector sync issues (ADR-007 Phase 3)
Completes the ADR-007 implementation by adding user-facing semantic search
functionality. Previous phases implemented scanner and processor for background
indexing; this adds the query interface.

Changes:
- Add nc_notes_semantic_search MCP tool for natural language queries
- Fix Qdrant point IDs to use UUIDs instead of strings (was causing 400 errors)
- Reduce scan interval default from 1 hour to 5 minutes for faster updates
- Add SemanticSearchResult and SemanticSearchNotesResponse models
- Implement dual-phase authorization (Qdrant filter + Nextcloud API verification)

The semantic search enables finding notes by meaning rather than exact keywords,
using vector embeddings to understand query intent. Point ID fix resolves
critical bug where all document indexing failed with "invalid point ID" errors.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-08 21:51:12 +01:00
Chris CoutinhoandClaude 4dbb2eb468 fix: integrate vector sync tasks with Starlette lifespan for streamable-http
Fixes background task startup for streamable-http transport by integrating
vector sync initialization into the Starlette lifespan context manager.

Starlette Lifespan Integration:
- Moved background task startup from FastMCP lifespan to Starlette lifespan
- FastMCP lifespan only triggers on MCP session establishment
- Starlette lifespan runs on server startup (correct timing)
- Fixed module scoping issues with local imports (anyio_module, asyncio_module)
- Added conditional startup based on oauth_enabled flag

Scanner Fixes:
- Fixed NotesClient method: list_notes() → get_all_notes()
- Properly handle AsyncIterator with list comprehension
- Collects all notes before processing

Verified Working:
- Background tasks start successfully on server startup
- Scanner fetches notes from Nextcloud API
- Processor pool (3 workers) ready for document processing
- Health endpoint reports Qdrant status
- No startup errors

Phase 3 Complete:
- BasicAuth mode with vector sync fully functional
- Background tasks integrate cleanly with streamable-http transport
- Graceful shutdown with coordinated task cancellation

Related: ADR-007 Background Vector Database Synchronization

🤖 Generated with Claude Code (https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-08 21:20:26 +01:00
Chris CoutinhoandClaude 8f45e996e8 feat: implement vector sync scanner and processor (ADR-007 Phase 2)
Implements background vector database synchronization using anyio
TaskGroups for BasicAuth mode with single-user credentials.

Scanner Implementation:
- Periodic document discovery (hourly, configurable)
- Timestamp-based change detection (Nextcloud vs Qdrant)
- Wake event for immediate scanning on-demand
- Supports both initial sync (all docs) and incremental sync (changes only)
- Detects deleted documents and queues for removal

Processor Implementation:
- Concurrent document processing pool (3 workers default)
- I/O-bound embedding generation via Ollama API
- Retry logic with exponential backoff (3 retries)
- Document chunking (512 words, 50-word overlap)
- Handles both index and delete operations
- Upserts vectors to Qdrant with rich metadata

App Lifespan Integration:
- Extended AppContext with background task state
- Modified app_lifespan_basic() to start tasks via anyio TaskGroups
- Graceful shutdown with coordinated task cancellation
- Only activates when VECTOR_SYNC_ENABLED=true

Embedding Service:
- OllamaEmbeddingProvider with TLS support
- Singleton pattern for shared client instances
- Batch embedding support for efficiency
- Auto-detects embedding dimension (768 for nomic-embed-text)

Qdrant Client:
- Async client wrapper with singleton pattern
- Auto-creates collection on first use
- COSINE distance metric for semantic similarity
- Integrates with embedding service for dimension detection

Health Check Enhancement:
- Added Qdrant status check to /health/ready endpoint
- Only checks when VECTOR_SYNC_ENABLED=true
- 2-second timeout for health probe
- Reports connection errors with details

Configuration:
- VECTOR_SYNC_ENABLED: Enable background sync
- VECTOR_SYNC_SCAN_INTERVAL: Scanner frequency (3600s default)
- VECTOR_SYNC_PROCESSOR_WORKERS: Concurrent processors (3 default)
- QDRANT_URL, QDRANT_API_KEY, QDRANT_COLLECTION: Vector DB config
- OLLAMA_BASE_URL, OLLAMA_EMBEDDING_MODEL: Embedding service config

Dependencies Added:
- qdrant-client>=1.7.0: Vector database client

Docker Compose:
- Added Qdrant service with health check
- Exposed ports 6333 (REST) and 6334 (gRPC)
- Configured MCP service with vector sync environment
- Added qdrant-data volume for persistence

Known Issue:
- FastMCP lifespan not triggering for streamable-http transport
- Background tasks will start once lifespan integration is complete
- Lifespan triggers on MCP session establishment, not server startup

Related: ADR-007 Background Vector Database Synchronization

🤖 Generated with Claude Code (https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-08 21:14:38 +01:00
Chris CoutinhoandClaude dc93da2ea0 docs: add ADR-007 for background vector database synchronization
Add comprehensive ADR-007 documenting background vector database
synchronization architecture using anyio TaskGroups for in-process
concurrency. This supersedes ADR-003's conceptual background worker.

Key decisions:
- In-process architecture using anyio TaskGroups (not Celery)
- Scanner task runs hourly, detects changes via timestamp comparison
- In-memory asyncio.Queue for pending documents
- Pool of 3 concurrent processor tasks for I/O-bound embedding workloads
- Qdrant metadata as single source of truth for indexing state
- Simple user controls: enable/disable with status visibility

Benefits:
- Single container deployment (was 3: mcp, celery-worker, celery-beat)
- No distributed task queue infrastructure
- Shared process state (no volume coordination)
- Sufficient throughput for I/O-bound embedding APIs
- Simpler debugging and deployment

Update ADR-003 status to "Superseded by ADR-007" with reference link.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-08 20:32:49 +01:00
Chris CoutinhoandClaude a766f4be32 test: enhance elicitation callback logging and error handling
Improve debugging capabilities for OAuth flow in elicitation callback:
- Add detailed logging for consent screen handling
- Capture screenshots when consent screen not detected or fails
- Replace networkidle wait with explicit callback URL polling
- Add 2-second grace period for server-side callback processing
- Log page title and current URL for debugging

This helps diagnose issues like expired OAuth clients or authorization failures during real elicitation testing.

Test results: All 4 elicitation integration tests passing
- test_check_logged_in_with_real_elicitation_callback ✓
- test_elicitation_callback_url_extraction ✓
- test_elicitation_stores_refresh_token ✓
- test_second_check_logged_in_does_not_elicit ✓

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-07 23:49:58 +01:00
Chris Coutinho ee053d559c chore: Remove tests 2025-11-07 22:59:57 +01:00
Chris CoutinhoandClaude 71326384da feat: add real elicitation integration test with python-sdk MCP client
This commit adds proper integration testing of the login elicitation flow
(ADR-006) using python-sdk's MCP client with actual elicitation callback
support, and fixes user_id extraction to support both JWT and opaque tokens.

## Changes

### 1. Enhanced create_mcp_client_session helper (tests/conftest.py)
- Added `elicitation_callback` parameter to function signature
- Pass callback to ClientSession constructor
- Added necessary imports: RequestContext, ElicitRequestParams,
  ElicitResult, ErrorData from mcp package
- Allows fixtures to provide custom elicitation handlers

### 2. New fixture: nc_mcp_oauth_client_with_elicitation (tests/conftest.py)
- Creates MCP client with Playwright-based elicitation callback
- Callback implementation:
  - Extracts OAuth URL from elicitation message using regex
  - Uses Playwright browser to complete OAuth flow automatically
  - Handles Nextcloud login form (username/password)
  - Handles consent screen if present
  - Waits for OAuth callback completion
  - Returns ElicitResult(action="accept") on success
- Function-scoped to allow independent test state
- Tracks elicitation invocations via session.elicitation_triggered

### 3. Fixed user_id extraction for opaque tokens (oauth_tools.py)
- Created extract_user_id_from_token() helper to handle both JWT and
  opaque tokens by calling userinfo endpoint when needed
- Fixed check_logged_in to use helper instead of broken ctx.authorization
- Fixed revoke_nextcloud_access to use helper instead of ctx.context.get()
- Both tools now properly extract user_id from access tokens

### 4. Enhanced integration tests (test_elicitation_integration.py)
- Updated tests to revoke refresh tokens via MCP tool
- All 4 tests now pass:
  - test_check_logged_in_with_real_elicitation_callback: Complete flow
  - test_elicitation_callback_url_extraction: URL extraction validation
  - test_elicitation_stores_refresh_token: Token persistence verification
  - test_second_check_logged_in_does_not_elicit: No redundant elicitations

### 5. Added diagnostic logging (oauth_routes.py)
- Track user_id extraction from ID tokens during OAuth callbacks
- Log refresh token storage with user_id and flow_type

## Test Results
 4/4 tests pass

The test suite successfully validates:
- Elicitation callback is triggered when no refresh token exists
- Playwright automation completes OAuth flow
- Refresh token is stored after OAuth with correct user_id
- Tool returns "yes" after successful login
- Already-logged-in users don't get redundant elicitations

## Why This Matters
Previous tests (test_login_elicitation.py) only validated response
formats and acknowledged they couldn't test real elicitation protocol.

This test exercises the REAL MCP elicitation protocol end-to-end:
1. MCP server calls ctx.elicit()
2. python-sdk ClientSession invokes custom callback
3. Callback completes OAuth via Playwright
4. Client returns acceptance to server
5. Tool proceeds with authenticated state

This proves the python-sdk MCP client can handle elicitation in
production environments with both JWT and opaque tokens.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-07 22:55:49 +01:00
Chris CoutinhoandClaude 11cdab475f feat: unify session architecture and enhance login status visibility
This commit addresses the "Login not detected" issue after completing
OAuth login via elicitation by unifying the session architecture and
adding comprehensive visibility into background session status.

## Changes

### 1. Enhanced check_logged_in with comprehensive logging (oauth_tools.py)
- Added detailed logging at each step of token lookup
- Implemented fallback strategy: first search by provisioning_client_id,
  then fall back to user_id lookup
- This allows detection of refresh tokens created via any flow
  (elicitation or browser login)
- Log messages include flow_type, provisioned_at, and provisioning_client_id
  for debugging

### 2. Unified session architecture (browser_oauth_routes.py)
- Browser login now stores provisioning_client_id=state when saving
  refresh token
- This makes browser and elicitation flows consistent - both can be
  found by the same state parameter
- Treats Flow 2 (elicitation) and browser login as the same "background
  session"

### 3. Enhanced /user/page with session status (userinfo_routes.py)
- Added comprehensive background access section showing:
  - Background Access: Granted/Not Granted (with visual indicators)
  - Flow Type: browser/flow2/hybrid
  - Provisioned At: timestamp
  - Token Audience: nextcloud/mcp
  - Scopes: detailed scope list
- Status displayed regardless of which flow created the session
  (browser login or elicitation)

### 4. Added revoke functionality (userinfo_routes.py, app.py)
- New POST endpoint: /user/revoke
- Allows users to revoke background access (delete refresh token)
- Browser session cookie remains valid for UI access
- Confirmation dialog before revocation
- Success page with auto-redirect back to /user/page
- Registered route in app.py browser_routes

## Testing
All tests pass:
- 6/6 login elicitation tests pass
- 21/21 core OAuth tests pass
- Comprehensive logging helps debug future issues

## Fixes
Resolves: "Login not detected. Please ensure you completed the login
at the provided URL before clicking OK."

The issue occurred because elicitation and browser login created
separate sessions. Now they are unified under the same architecture.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-07 21:50:55 +01:00
Chris CoutinhoandClaude 281d28c7cd test: Add comprehensive elicitation URL and refresh token validation
Enhanced test suite to validate:
1. Elicitation URL format and Flow 2 endpoint routing
2. Server-side refresh token validation via check_provisioning_status API
3. Proper separation of concerns - tests use MCP server API, not direct storage access

The refresh token validation test validates server responses:
- is_provisioned=true: Server has valid refresh token
- is_provisioned=false: No token or invalid token
- Error response: Token validation failed

This ensures the MCP server properly validates refresh tokens internally
and reports status correctly through its public API.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-07 21:21:58 +01:00
Chris CoutinhoandClaude 0c9a9ea24d fix: Consolidate OAuth callbacks and implement PKCE for all flows
This PR fixes multiple OAuth-related issues:

## Unified OAuth Callback
- Consolidated `/oauth/callback-nextcloud` and `/oauth/login-callback` into single `/oauth/callback` endpoint
- Flow type determined by session lookup via state parameter (no query params in redirect_uri)
- Fixes redirect_uri validation issues with IdPs requiring exact match
- Legacy endpoints kept as aliases for backwards compatibility

## PKCE Implementation
- Implemented PKCE (RFC 7636) for Flow 2 (resource provisioning)
  - Generate code_verifier and code_challenge
  - Store code_verifier in session storage
  - Retrieve and use in token exchange
- Fixed PKCE for browser login (integrated mode)
  - Previously only worked for external IdP (Keycloak)
  - Now works for both Nextcloud OIDC and external IdP

## Login Elicitation Fixes (ADR-006)
- Fixed elicitation URL to route through MCP server endpoint
  - Changed from direct Nextcloud URL to `/oauth/authorize-nextcloud`
  - Ensures PKCE is properly handled by server
- Fixed login detection after OAuth flow completes
  - Look up refresh token by state parameter instead of user_id
  - Works even when Flow 1 token not present
- Added `get_refresh_token_by_provisioning_client_id()` method

## Session Authentication
- Fixed `/user/page` redirect loop
  - Shared oauth_context with mounted browser_app
  - SessionAuthBackend can now validate sessions correctly

## Tests
- Added comprehensive login elicitation test suite
- Updated scope authorization test expectations
- All 43 OAuth tests passing

## Files Changed
- `app.py`: Shared oauth_context, unified callback route
- `oauth_routes.py`: Unified callback, PKCE for Flow 2
- `browser_oauth_routes.py`: PKCE for integrated mode
- `oauth_tools.py`: Fixed elicitation URL generation
- `refresh_token_storage.py`: Added lookup by provisioning_client_id
- `test_login_elicitation.py`: New test suite

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-07 21:08:55 +01:00
Chris CoutinhoandClaude 659087e4c7 fix: Implement proper OAuth resource parameters and PRM-based discovery
This commit completes the OAuth audience validation implementation per RFC 7519,
RFC 8707 (Resource Indicators), and RFC 9728 (Protected Resource Metadata).

## Key Changes

### OAuth Resource Parameters (RFC 8707)
- Add `resource` parameter to Flow 1 (MCP client auth) with MCP server audience
- Add `resource` parameter to Flow 2 (Nextcloud access) with Nextcloud audience
- Add `nextcloud_resource_uri` to oauth_context configuration
- Fix undefined variable error in starlette_lifespan

### PRM-Based Resource Discovery (RFC 9728)
- Update tests to fetch resource identifier from PRM endpoint
- Add fallback to hardcoded value if PRM fetch fails
- Demonstrate correct OAuth client implementation pattern

### ADR-005 Documentation Updates
- Update to reflect simplified RFC 7519 compliant implementation
- Document that MCP validates only its own audience (not Nextcloud's)
- Add section on OAuth resource parameters and PRM discovery
- Update implementation checklist to show completed items
- Mark status as "Implemented" with update date

## Implementation Details

The solution follows RFC 7519 Section 4.1.3: resource servers validate only
their own presence in the audience claim. This simplifies the logic while
maintaining security:

- MCP server validates MCP audience only
- Nextcloud independently validates its own audience
- No dual validation required at MCP layer
- Token reuse is allowed per RFC 8707 for multi-audience tokens

## Test Results
 test_mcp_oauth_server_connection - PASSED
 test_deck_board_view_permissions - PASSED
 test_prm_endpoint - PASSED

All OAuth flows now properly specify target resources, resulting in correct
audience validation throughout the system.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-05 23:19:03 +01:00
Chris CoutinhoandClaude bdb1ba2051 refactor: Eliminate duplicate validation logic in UnifiedTokenVerifier
Since both multi-audience and exchange modes now validate the same thing
(MCP audience only per RFC 7519), consolidated the duplicate methods:

- Removed duplicate verification methods (_verify_multi_audience_token
  and _verify_mcp_audience_only)
- Created single _verify_mcp_audience() method for all validation
- Removed duplicate helper (_validate_multi_audience), kept _has_mcp_audience
- Mode only affects logging and what happens AFTER verification

The mode distinction is now purely about post-verification behavior:
- Multi-audience mode: Use token directly (Nextcloud validates its own)
- Exchange mode: Exchange for Nextcloud-audience token via RFC 8693

This makes the code cleaner and clearer about what's actually happening -
both modes do identical validation, they just differ in how the validated
token is used.

All tests pass: unit (65), OAuth integration confirmed working.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-05 21:58:52 +01:00
Chris CoutinhoandClaude 7d9ab5559c fix: Simplify token verifier to be RFC 7519 compliant
Per RFC 7519 Section 4.1.3, resource servers should only validate their
own presence in the audience claim, not check for other resource servers.

Changes:
- UnifiedTokenVerifier now validates only MCP audience (not Nextcloud's)
- Nextcloud independently validates its own audience when receiving API calls
- This is NOT token passthrough (we validate tokens before use)
- This IS token reuse which is explicitly allowed by RFC 8707

Updates:
- Simplified _validate_multi_audience() to follow OAuth spec
- Updated docstrings and comments to clarify RFC 7519 compliance
- Fixed unit tests that expected dual-audience validation
- Updated ADR-005 to document the correct OAuth interpretation
- All tests pass: unit (65), smoke (5), OAuth integration

This makes the implementation simpler, more maintainable, and properly
aligned with OAuth 2.0 specifications while maintaining security.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-05 21:44:04 +01:00
Chris CoutinhoandClaude 877c4c91e0 fix: Use Keycloak client ID for NEXTCLOUD_RESOURCE_URI in token exchange
Fix external IdP token exchange by using the correct audience identifier
for Keycloak.

Keycloak uses client IDs as audience identifiers, not URLs. The token
exchange was failing with "Audience not found" because it was requesting
audience "http://localhost:8080" but Keycloak only knows about the
"nextcloud" client ID.

Changes:
- Update mcp-keycloak service NEXTCLOUD_RESOURCE_URI from
  "http://localhost:8080" to "nextcloud"
- Matches Keycloak's client ID convention for resource identifiers
- Token exchange now requests audience "nextcloud" which matches the
  Keycloak resource server client configuration

Note: mcp-oauth service keeps URL-based resource URI because Nextcloud's
integrated OIDC app expects URLs, not client IDs. Different IdPs have
different conventions for audience/resource identifiers.

Test result: test_external_idp_token_validation now passes

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-05 19:18:10 +01:00
Chris CoutinhoandClaude 5deb3132c3 fix: Correct OAuth token audience validation for multi-audience mode
Fix two issues preventing OAuth tests from passing:

1. Set oidc_client_id and oidc_client_secret on Settings object
   - These were being read from environment but not propagated to the
     UnifiedTokenVerifier settings instance

2. Use client_issuer instead of issuer for JWT validation
   - client_issuer accounts for NEXTCLOUD_PUBLIC_ISSUER_URL override
   - Fixes "Invalid issuer" errors when public URL differs from internal

3. Accept resource URL with /mcp path in audience validation
   - During DCR, resource_url is registered as "{mcp_server_url}/mcp"
   - Tokens correctly include this full path as audience
   - Verifier now accepts both "http://localhost:8001" and
     "http://localhost:8001/mcp" as valid MCP audiences

These changes restore OAuth functionality while maintaining ADR-005
security requirements for proper audience validation.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-05 19:03:35 +01:00
Chris CoutinhoandClaude 9fab6cb550 feat: Implement ADR-005 unified token verifier to eliminate token passthrough vulnerability
Replace two non-compliant token verifiers (NextcloudTokenVerifier and
ProgressiveConsentTokenVerifier) with a single UnifiedTokenVerifier that properly
validates token audiences per MCP Security Best Practices specification.

The previous implementation had a critical security vulnerability where tokens
intended for the MCP server were passed directly to Nextcloud APIs without
proper audience validation (token passthrough anti-pattern). This violates
OAuth 2.0 security principles and the MCP specification.

Changes:
- Add UnifiedTokenVerifier supporting two compliant modes:
  * Multi-audience mode (default): Validates tokens contain BOTH MCP and
    Nextcloud audiences, enabling direct use without exchange
  * Token exchange mode (opt-in): Validates MCP audience only, exchanges
    for Nextcloud tokens via RFC 8693 with caching to minimize latency

- Remove token passthrough vulnerability from context.py and context_helper.py
- Implement token exchange caching (5-minute TTL default) to reduce network calls
- Add required environment variables for audience validation:
  * NEXTCLOUD_MCP_SERVER_URL - MCP server URL (used as audience)
  * NEXTCLOUD_RESOURCE_URI - Nextcloud resource identifier
  * TOKEN_EXCHANGE_CACHE_TTL - Cache TTL for exchanged tokens

- Update docker-compose.yml with resource URI configuration for both OAuth modes
- Add comprehensive test suite (29 tests) covering both authentication modes
- Remove legacy NextcloudTokenVerifier and ProgressiveConsentTokenVerifier

Security improvements:
- Eliminates token passthrough anti-pattern
- Enforces proper audience separation between MCP and Nextcloud
- Complies with MCP Security Best Practices and RFC 8707/8693
- Maintains performance with token exchange caching

Test results: 65/65 unit tests passed, 5/5 smoke tests passed

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-05 18:53:14 +01:00
Chris CoutinhoandClaude 28c2debf3e docs: Add ADR-005 for unified token verifier architecture
This ADR addresses the critical token passthrough vulnerability identified
in Issue #261 by proposing a unified token verifier that eliminates the
security issue while maintaining flexibility.

Key changes:
- Consolidates two non-compliant verifiers into single UnifiedTokenVerifier
- Implements two-layer architecture (verification + exchange)
- Supports multi-audience mode (default) and token exchange mode (opt-in)
- Removes all token passthrough paths to comply with MCP security spec
- Works within python-sdk constraints using proper separation of concerns

The solution provides:
- Single source of truth for token validation
- MCP specification compliance
- Minimal performance impact (1-2% of LLM request time)
- Clear migration path for existing deployments

BREAKING CHANGE: All OAuth deployments must be reconfigured to specify
resource URIs (NEXTCLOUD_MCP_SERVER_URL and NEXTCLOUD_RESOURCE_URI) and
choose between multi-audience or token exchange mode.

Related: #261
Supersedes: Token passthrough mode in ADR-004

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-05 18:34:43 +01:00
Chris Coutinho 3485b55e2d ci: Update oidc app 2025-11-05 15:58:40 +01:00
Chris Coutinho 4adb9de5f0 chore: fix typo 2025-11-05 15:36:50 +01:00
Chris Coutinho bfa944d0e8 ci: Rename pre-commit hook [skip ci] 2025-11-05 15:31:52 +01:00
Chris Coutinho 01569497d7 ci: Add pre-commit hook for ty [skip ci] 2025-11-05 15:26:00 +01:00
Chris Coutinho 6cccd92b3b build: Add type checking 2025-11-05 15:19:55 +01:00
Chris Coutinho 9dcda0cd6a test: Update config 2025-11-05 09:53:23 +01:00
Chris Coutinho 7c2f39930a ci: Update oidc app config 2025-11-05 07:13:46 +01:00
Chris Coutinho 205c3b013c build: Update oidc submodule 2025-11-05 06:57:12 +01:00
Chris CoutinhoandClaude 8983f25eaf fix: add missing await for get_nextcloud_client in capabilities resource
Fix nc_get_capabilities resource handler that was missing await when
calling get_nextcloud_client(ctx), causing error:
'coroutine' object has no attribute 'capabilities'

Root cause:
- get_nextcloud_client() is an async function (context.py:9)
- Returns a coroutine that must be awaited
- app.py:737 called it without await

Solution:
- Add await: client = await get_nextcloud_client(ctx)
- The handler is already async, so can await the call

Test fixed:
- test_mcp_resources_access now passes

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-04 10:22:50 +01:00